Skip to content

fix(task-bridge): belt-suspenders guard against non-task .txt fallthrough (closes #1035) - #1036

Merged
sonichi merged 2 commits into
mainfrom
fix/task-bridge-belt-suspenders-1035
May 22, 2026
Merged

fix(task-bridge): belt-suspenders guard against non-task .txt fallthrough (closes #1035)#1036
sonichi merged 2 commits into
mainfrom
fix/task-bridge-belt-suspenders-1035

Conversation

@sonichi

@sonichi sonichi commented May 22, 2026

Copy link
Copy Markdown
Owner

Closes #1035. Follow-up to PR #1033 (per-channel pull path for discord-voice + phone).

Background

PR #1033 introduced results/<channel-key>.task-{id}.txt as a new filename namespace for the per-channel pull path used by discord-voice + phone. Per-consumer audit in #1033's PR body confirmed no existing consumer claims those filenames — except for one fragile spot Maddy flagged in review:

src/task-bridge.ts's result-watcher has an unconditional fallthrough that, when voice-agent's client is connected, fires onResult() for any non-empty .txt file past the voice-* / dedup / offline-forward branches. Without a guard, a <key>.task-foo.txt file could race the per-channel scanner inside discord-voice / phone and also be injected into the live voice session. PR #1033's mitigation is the scanner's read-and-delete usually winning the race — but the race exists, and it was filed as out-of-scope for #1033's narrow design.

Change

Two-part, ~5 LOC of behavior change plus a comment + helper + test:

  1. src/task-bridge.ts — add _shouldFallthrough(file) predicate near _isVoiceTask returning true only for task-* / voice-* prefixes; gate the unconditional fallthrough with if (!_shouldFallthrough(file)) continue; immediately before the existing if (result) block.

  2. tests/task-bridge-fallthrough-guard.test.ts — new test covering canonical-task / voice-push / channel-pull-namespace / proactive / unknown-prefix cases. Anchored to the start of the filename (rejects prefix-task- and xtask-).

The predicate is exported (_-prefixed convention, same as _isVoiceTask) because the watcher's setInterval body is awkward to exercise in isolation; testing the predicate directly is cleaner and locks in the contract.

Behavioral impact

Filename pattern Before After
task-*.txt falls through → onResult unchanged
voice-*.txt short-circuits earlier → spoken unchanged
proactive-*.txt hit the fallthrough as a latent duplicate-delivery hazard (poll_proactive in discord-bridge also reads it) now skipped cleanly; sole consumer is poll_proactive, which matches its design
<channel-key>.task-*.txt (PR #1033 new namespace) could race per-channel scanner and reach onResult rejected — race closed
Any other unknown prefix could reach onResult if any tool ever drops it in results/ rejected

The proactive-* change is a side-effect cleanup, not a regression: poll_proactive was always the intended consumer, and task-bridge reaching onResult on those files was the bug.

Verification

  • npm run typecheck clean
  • 268 existing TS tests pass + 6 new tests pass
  • No other consumer modified

🤖 Generated with Claude Code

sonichi and others added 2 commits May 22, 2026 15:29
@sonichi @claude
fix(task-bridge): belt-suspenders guard against non-task .txt fallthr…
f532d67 
…ough (closes #1035)

PR #1033 introduced `results/<channel-key>.task-{id}.txt` as a new filename
namespace for the per-channel pull path used by discord-voice + phone. The
existing consumers don't pick those up, except for one fragile spot:
`src/task-bridge.ts`'s result-watcher has an unconditional fallthrough that,
when voice-agent's client is connected, fires `onResult()` for any non-empty
`.txt` file past the `voice-*` / dedup / offline-forward branches. Without a
guard, a `<key>.task-foo.txt` file could race the per-channel scanner and
*also* be injected into voice. PR #1033 accepted the race (scanner's read-and-
delete usually wins); this guard closes it.

Change:
  - Add `_shouldFallthrough(file)` predicate near `_isVoiceTask` — returns true
    only for `task-*` / `voice-*` prefixes. Rejects the new-namespace files,
    `proactive-*` (discord-bridge's poll_proactive handles those), and any
    unfamiliar prefix.
  - Gate the fallthrough at the result-watcher with
    `if (!_shouldFallthrough(file)) continue;` before the existing `if (result)`
    block.
  - Test: `tests/task-bridge-fallthrough-guard.test.ts` exercises the predicate
    across canonical-task / voice-push / channel-pull-namespace / proactive /
    unknown-prefix cases. Anchored to the start of the filename — `prefix-task-`
    and `xtask-` are correctly rejected.

No behavioral change for the existing `task-*` / `voice-*` paths (both still
fall through). `proactive-*` was previously hitting the fallthrough as a latent
duplicate-delivery hazard (discord-bridge's `poll_proactive` also consumes it);
the guard now skips it cleanly, matching its actual intended consumer.

Verified:
  - `npm run typecheck` clean
  - 268 existing TS tests pass + 6 new tests pass
  - No other consumer modified

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonichi @claude
fix(task-bridge): allow proactive-* through the fallthrough guard
0ba77aa 
The #1035 belt-suspenders guard rejected `proactive-*.txt`, framed as
"closing a duplicate-delivery hazard." That framing was wrong — per the
long-standing proactive-voice rule, `proactive-*` files are spoken by
the voice agent when the client is connected. That delivery has no
explicit handler upstream in startResultWatcher; the unconditional
fallthrough IS the path. Blocking it silently disables voice-spoken
proactive messages.

Add `proactive-` to the `_shouldFallthrough` allowlist so the original
delivery path survives. The guard still rejects `<channel-key>.task-*`
(the actual #1035 target). Flip the proactive test case from "rejects"
to "allows", with a docstring explaining why both consumers coexist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sonichi
sonichi merged commit d16526a into main May 22, 2026
2 checks passed
@sonichi
sonichi deleted the fix/task-bridge-belt-suspenders-1035 branch May 22, 2026 22:41
This was referenced May 23, 2026
Merged
Merged
Merged
liususan091219 pushed a commit to liususan091219/sutando-fork that referenced this pull request May 28, 2026
@sonichi @claude
fix(task-bridge): belt-suspenders guard against non-task .txt fallthr…
ef256a9 
…ough (closes sonichi#1035) (sonichi#1036)

* fix(task-bridge): belt-suspenders guard against non-task .txt fallthrough (closes sonichi#1035)

PR sonichi#1033 introduced `results/<channel-key>.task-{id}.txt` as a new filename
namespace for the per-channel pull path used by discord-voice + phone. The
existing consumers don't pick those up, except for one fragile spot:
`src/task-bridge.ts`'s result-watcher has an unconditional fallthrough that,
when voice-agent's client is connected, fires `onResult()` for any non-empty
`.txt` file past the `voice-*` / dedup / offline-forward branches. Without a
guard, a `<key>.task-foo.txt` file could race the per-channel scanner and
*also* be injected into voice. PR sonichi#1033 accepted the race (scanner's read-and-
delete usually wins); this guard closes it.

Change:
  - Add `_shouldFallthrough(file)` predicate near `_isVoiceTask` — returns true
    only for `task-*` / `voice-*` prefixes. Rejects the new-namespace files,
    `proactive-*` (discord-bridge's poll_proactive handles those), and any
    unfamiliar prefix.
  - Gate the fallthrough at the result-watcher with
    `if (!_shouldFallthrough(file)) continue;` before the existing `if (result)`
    block.
  - Test: `tests/task-bridge-fallthrough-guard.test.ts` exercises the predicate
    across canonical-task / voice-push / channel-pull-namespace / proactive /
    unknown-prefix cases. Anchored to the start of the filename — `prefix-task-`
    and `xtask-` are correctly rejected.

No behavioral change for the existing `task-*` / `voice-*` paths (both still
fall through). `proactive-*` was previously hitting the fallthrough as a latent
duplicate-delivery hazard (discord-bridge's `poll_proactive` also consumes it);
the guard now skips it cleanly, matching its actual intended consumer.

Verified:
  - `npm run typecheck` clean
  - 268 existing TS tests pass + 6 new tests pass
  - No other consumer modified

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(task-bridge): allow proactive-* through the fallthrough guard

The sonichi#1035 belt-suspenders guard rejected `proactive-*.txt`, framed as
"closing a duplicate-delivery hazard." That framing was wrong — per the
long-standing proactive-voice rule, `proactive-*` files are spoken by
the voice agent when the client is connected. That delivery has no
explicit handler upstream in startResultWatcher; the unconditional
fallthrough IS the path. Blocking it silently disables voice-spoken
proactive messages.

Add `proactive-` to the `_shouldFallthrough` allowlist so the original
delivery path survives. The guard still rejects `<channel-key>.task-*`
(the actual sonichi#1035 target). Flip the proactive test case from "rejects"
to "allows", with a docstring explaining why both consumers coexist.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

task-bridge: belt-suspenders guard for non-task .txt files (narrow-scope-followup from #1033)

1 participant

@sonichi