Skip to content

fix(bridge): stop replaying delivered output files on follow-up turns - #352

Open
Mor-Li wants to merge 2 commits into
xvirobotics:mainfrom
Mor-Li:fix/outputs-resend-across-turns
Open

fix(bridge): stop replaying delivered output files on follow-up turns#352
Mor-Li wants to merge 2 commits into
xvirobotics:mainfrom
Mor-Li:fix/outputs-resend-across-turns

Conversation

@Mor-Li

@Mor-Li Mor-Li commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Problem

Every follow-up message within 5 minutes of a task that produced output files replays the entire outputs directory to the chat — say "thanks" after receiving report.pdf, and the bot sends report.pdf again (and repeats the oversized-file notice too).

Three pieces line up to cause it:

  1. OutputsManager.prepareDir intentionally keeps files younger than the 5-minute retention window (RETENTION_MS),
  2. scanOutputs is a stateless directory walk with no delivered-file filter,
  3. sentPaths in OutputHandler.sendOutputFiles is method-local — it only dedupes within a single call, with zero memory across turns.

Fix

Bind delivery to deletion: once a file is sent, text-fallbacked, or accounted for via the oversized notice, unlink it so the next turn's scan doesn't see it. Files whose send threw are deliberately kept on disk (delivered stays false), so the follow-up turn retries them — the retention window still garbage-collects them eventually.

Tests (red/green verified)

tests/output-handler.test.ts gains a "resend prevention" suite; the two resend cases fail on the unfixed code:

× does not resend a delivered file on the next turn within the retention window
× does not repeat the oversized notice on the next turn
✓ keeps a file whose send threw, so the next turn can retry it   (semantics lock)

With the fix: file suite 9/9, full root suite 63 files / 665 tests green.

🤖 Generated with Claude Code

prepareDir keeps files younger than the 5-minute retention window and
scanOutputs is a stateless directory walk, so every follow-up message
inside that window resent the entire outputs directory ("thanks" →
here is report.pdf again).

Bind delivery to deletion: files that were sent, text-fallbacked, or
accounted for via the oversized notice are unlinked after handling;
files whose send threw stay on disk so the next turn retries them.

The two resend tests fail on the unfixed code (bug reproduction); the
retry test locks the keep-on-throw semantics.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@floodsung

Copy link
Copy Markdown
Contributor

This is the right delivery-bound direction and should supersede #336, but an additional failure-path probe shows files are still deleted when sendImageFile/sendLocalFile returns false. Please mark delivered only on a true send result or successful text fallback; retain files on false or throw, and cover both image/file false returns plus notice failure. Keeping open for the corrected head.

Review follow-up: sendImageFile/sendLocalFile resolve false on
non-throwing send failures, but delivery-bound deletion treated every
non-throwing path as delivered and unlinked files the user never
received.

- image sends: delivered = the actual boolean send result
- file sends: delivered on true, or on a successful text fallback;
  a false return with no applicable fallback retains the file
- oversized files: deletion deferred until the coalesced notice
  actually reached the user; a failed notice retains them so the
  next turn re-raises it
- tests: image false-return, file false-return without fallback,
  text-fallback delivery, notice failure + re-raise

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@Mor-Li

Mor-Li commented Jul 26, 2026

Copy link
Copy Markdown
Contributor Author

@floodsung Thanks for the failure-path probe — fixed in ae0aa44. Delivery is now bound to proof of delivery rather than "didn't throw":

  • Image sends: delivered takes the actual sendImageFile boolean; a false return retains the file for the next turn to retry.
  • File sends: delivered on a true sendLocalFile result, or when the text fallback resolves; a false return with no applicable fallback (non-text or ≥30KB) retains the file.
  • Oversized files: deletion is deferred until the coalesced notice actually reached the user (sendOversizedNotice now reports success); a failed notice keeps the files so the next turn re-raises it. Fallback-scan images (outside the outputs dir) are never deleted.

New tests cover all three probes: image false-return, file false-return without fallback, and notice failure + next-turn re-raise — plus one locking text-fallback delivery → deletion. All 13 output-handler tests pass.

🤖 Generated with Claude Code

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.

2 participants