fix(auto-attach): skip bad disk attachments and cap message flooding - #229
Open
alex-mextner wants to merge 1 commit into
Open
fix(auto-attach): skip bad disk attachments and cap message flooding#229alex-mextner wants to merge 1 commit into
alex-mextner wants to merge 1 commit into
Conversation
A disk attachment (auto-detected from a path mention, or an explicit --photo/--file) that goes missing, is truncated to empty, or loses read permission between detection and send used to make Telegram's "file must be non-empty" rejection kill the WHOLE send, including the primary text. transmit() now re-validates every disk-sourced attachment right before the photos/text/documents sandwich runs (features/auto-attach/file-check.ts, pure checkAttachmentFile with injected stat/canRead), drops a bad one with a stderr warning naming the path and the reason, and still delivers the rest of the plan. The same check runs a second, earlier time in the `tg` entrypoint right before code-as-pdf/md-as-pdf conversion, so a bad original source can't be silently replaced by a generated PDF that only gets validated on its own merits. If every attachment is bad and no text remains, the send now refuses loudly (non-zero exit) instead of a silent no-op success. A message long enough to fragment into more than FLOOD_CAP_MAX_MESSAGES (6) separate Telegram sends is now refused up front, naming the exact character count and message count it would have produced, instead of silently flooding the recipient with dozens of fragments. New `flood-cap` feature flag (on by default); disable with --no-feature flood-cap or features.flood-cap: false. Rich messages (sendRichMessage) are exempt — they always send whole. Both fixes are covered by unit tests (file-check.test.ts, transmitter.test.ts) and an end-to-end wiring test that runs the real `tg` binary against a mock Bot API server (cli-attachment-flood-integration.test.ts), which is what caught a prior gap where the entrypoint could forget to wire checkFile/allowFlood through to transmit(). 53 tests pass across the three touched test files (145 expect() calls). Version bump 1.41.0 -> 1.42.0 (minor, not patch): the flood cap can refuse a send that previously succeeded, so this is a behavior change. Fixes #207 Fixes #208 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
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
1 participant
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
--photo/--file) that goes missing, gets truncated to empty, or loses read permission between detection and send is now skipped with a stderr warning naming the path and reason — the primary text still delivers. Previously Telegram's "file must be non-empty" rejection killed the WHOLE send. If every attachment is bad and no text remains, the send now refuses loudly (non-zero exit) instead of a silent no-op success. Fixes auto-attach fails the whole send when the body mentions a nonexistent or empty file path #207.flood-capfeature flag (on by default); override with--no-feature flood-cap. Rich messages (tables/headings/lists/formulas) are unaffected — they always send whole. Fixes tg silently fragments an oversized message into dozens of separate Telegram sends instead of warning or refusing #208.Test plan
bun test tests/transmitter.test.ts tests/file-check.test.ts tests/cli-attachment-flood-integration.test.ts— 53 pass, 0 fail, 145 expect() callsbun test, 2451 tests / 147 files) — 2442 pass, 1 skip, 8 fail, 1 error; all 8 failures + the 1 error are in unrelated pre-existing integration tests (ctl-service-cli,ctl-tasks-integration,run-text-hooks,ctl-daemon-integration,hooks-photo-integration— timing-sensitive tmux/daemon races, not touched by this diff). Verified by stashing this change and re-running those same files against clean HEAD: identical failures reproduce without the fix applied.tg --helpstill runs correctly and documents both new behaviors (missing/empty/unreadable attachments, message-flood cap)tgbinary — catches entrypoint wiring gaps the pure unit tests can't (e.g. forgetting to passcheckFile/allowFloodthrough totransmit())🤖 Generated with Claude Code