fix(file-service): restore share notification publishing - #262
Open
devin-ai-integration[bot] wants to merge 3 commits into
Open
devin-ai-integration[bot] wants to merge 3 commits into
devin-ai-integration[bot] wants to merge 3 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
Author
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
Co-Authored-By: bot_apk <apk@cognition.ai>
Co-Authored-By: bot_apk <apk@cognition.ai>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Symptom and blast radius. New file shares returned HTTP 500 while publishing
file_shared, so recipients missed notifications and the UI reported that sharing failed. Any deployment inheriting the file-service image default was affected.Root cause (config/env defect). The image baked
FILE_SHARE_EVENT_ALWAYS_FAIL=true, causingEventPublisher::file_sharedto replace the configuredSNS_TOPIC_ARNwith a nonexistent-v2topic and surface the SNS rejection.Fix. Remove the enabled image default so the runtime keeps
SnsConfig's safefalsedefault and publishes to the configured topic. Addimage_does_not_force_share_event_failuresto reject enabled DockerENVdefaults, including multi-assignment and backslash-continued declarations.Verification. Rebuilt the image and recreated file-service without Compose's masking override; the image and running container contain no
FILE_SHARE_EVENT_ALWAYS_FAIL.cargo fmt --check, all 25 tests, andcargo clippy --all-targets -- -D warningspass. In the browser, sharingTeam Offsite Agenda.txtwithadmin@otterworks.devreturned HTTP 201, persisted recipient access after reload, publishedfile_sharedto the configured topic, and produced the recipient notification through LocalStack SNS/SQS.Post-fix browser recording
Recipient-side UI was not exercised; delivery was verified through the notification store and consumer logs. The fork's Helm default is intentionally unchanged because live tenant deployments use the upstream-owned runner chart and do not pass this key; changing it here would not affect the incident deployment path.
Related independent remediations remain untouched, including #245, #248, and #251–#259.
Link to Devin session: https://app.devin.ai/sessions/3f77dd83f4924148824c1480b580983b
Devin Review