Skip to content

fix(file-service): restore share notification publishing - #258

Open
devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787627355-share-notification-sns
Open

devin-ai-integration[bot] wants to merge 1 commit into
mainfrom
devin/1787627355-share-notification-sns

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 25, 2026

Copy link
Copy Markdown

Summary

file-service images forced FILE_SHARE_EVENT_ALWAYS_FAIL=true, so every file_shared publish substituted the configured SNS topic with a nonexistent -v2 ARN. Shares returned errors, were not reliably recorded, and recipients missed notifications.

  • Remove the image-level override so SnsConfig::from_env() retains its safe default and EventPublisher::file_shared() publishes to SNS_TOPIC_ARN.
  • Add a regression guard that rejects enabled Dockerfile defaults across supported ENV syntax variants.
  • Automated verification: cargo fmt --check, all 26 Rust tests, and cargo clippy --all-targets -- -D warnings.
  • Runtime verification: rebuilt/recreated file-service without the override; browser share returned success and persisted after reload, the API returned 201, the share row persisted, and notification-service consumed the file_shared event from the configured otterworks-events topic. Logs contained no -v2 ARN or publish failures.
  • End-to-end screen recording

This is an independent reproduction and fix from main; related investigations: #253 and #254. This run adds syntax-aware regression coverage for both key/value and assignment-form Dockerfile environment declarations.

Link to Devin session: https://app.devin.ai/sessions/eca7fa9335614cbda9ebe95da1757522


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Devin Review (Staging)
Open in Devin Review

Co-Authored-By: bot_apk <apk@cognition.ai>
@devin-ai-integration
devin-ai-integration Bot requested a review from a team August 25, 2026 03:11
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment on lines +151 to +178
fn dockerfile_enables_share_event_failure(dockerfile: &str) -> bool {
let dockerfile = dockerfile.replace("\\\r\n", " ").replace("\\\n", " ");

dockerfile
.lines()
.map(str::trim)
.filter_map(|line| {
let (instruction, rest) = line.split_once(char::is_whitespace)?;
instruction
.eq_ignore_ascii_case("ENV")
.then_some(rest.trim_start())
})
.any(|env| {
if env.contains('=') {
env.split_whitespace().any(|assignment| {
assignment
.strip_prefix("FILE_SHARE_EVENT_ALWAYS_FAIL=")
.is_some_and(|value| parse_bool(value.trim_matches(['"', '\'']), false))
})
} else {
env.strip_prefix("FILE_SHARE_EVENT_ALWAYS_FAIL")
.is_some_and(|value| {
value.starts_with(char::is_whitespace)
&& parse_bool(value.trim().trim_matches(['"', '\'']), false)
})
}
})
}

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Regression guard covers the relevant ENV forms

dockerfile_enables_share_event_failure joins continuations and parses both ENV KEY=value and ENV KEY value forms, handling quotes, case, comments, and prefix collisions like FILE_SHARE_EVENT_ALWAYS_FAIL_OTHER. The primary reintroduction form is covered.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration

Copy link
Copy Markdown
Author

Runtime verification

Sharing Annual Planning 2027.txt with admin@otterworks.dev succeeded in the browser and persisted after reload:

Share succeeded with green toast

Share persisted after reload

file-service returned 201 and published file_shared; notification-service consumed it and stored the recipient's in-app notification. Full-log checks found no Failed to publish file_shared or -v2 ARN. A negative-control container with the old enabled override returned 500 against otterworks-events-v2.

Screen recording

Pre-existing local noise observed outside this path: unrelated queued messages with null sharedWithUserId, and unverified local SES sender email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants