Skip to content

fix(team): change sendToWorker fallback from fail-open to fail-closed#2175

Merged
Yeachan-Heo merged 1 commit intoYeachan-Heo:devfrom
jonckr:fix/sendtoworker-fail-closed
Apr 5, 2026
Merged

fix(team): change sendToWorker fallback from fail-open to fail-closed#2175
Yeachan-Heo merged 1 commit intoYeachan-Heo:devfrom
jonckr:fix/sendtoworker-fail-closed

Conversation

@jonckr
Copy link
Copy Markdown

@jonckr jonckr commented Apr 4, 2026

Summary

  • sendToWorker returns true after all retry rounds exhaust without confirming the message left the pane buffer
  • All callers (notifyPaneWithRetry, idle-nudge, api-interop) treat true as delivery success, so no retry is attempted
  • The worker silently misses its task assignment and the dispatch system marks it as notified

Fix

Change the final return true to return false at line 863. The callers already handle false correctly (retry or record failure), as confirmed by existing tests (idle-nudge.test.ts:313-315, runtime-v2.dispatch.test.ts:240).

Failure scenario

  1. sendToWorker sends message text via tmux send-keys -l
  2. 6 submit rounds + adaptive retry + 4 more rounds all fail to clear the text from the pane
  3. Function returns true -- caller believes delivery succeeded
  4. Worker never receives the task, leader never retries

Test plan

  • All 63 existing team tests pass (tmux-session, tmux-comm, idle-nudge, runtime-v2.dispatch)
  • idle-nudge.test.ts:313 already tests the sendToWorker → false path and verifies pane is not counted as nudged
  • runtime-v2.dispatch.test.ts:240 already tests the sendToWorker → false path and verifies retry behavior

After all retry rounds (6 submit rounds + adaptive retry + 4 more rounds)
fail to confirm message left the pane buffer, sendToWorker returned true.
All callers (notifyPaneWithRetry, idle-nudge, api-interop) treat true as
delivery success, so no retry was attempted and the worker silently missed
its task assignment.

Return false instead so callers can retry or record failure, which they
already handle correctly.
@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@Yeachan-Heo
Copy link
Copy Markdown
Owner

Reviewed via OMC session. Approve - critical reliability fix for silent failure mode in team mode. 63 team tests pass.\n\n—\n*[repo owner'''s gaebal-gajae (clawdbot) 🦞]*

@Yeachan-Heo Yeachan-Heo merged commit 8773fc0 into Yeachan-Heo:dev Apr 5, 2026
Yeachan-Heo added a commit that referenced this pull request Apr 5, 2026
This restores the pre-jonckr runtime, hook, and team behavior by
reverting the five owner-scoped merge commits on top of origin/dev.
The rollback stays narrowly focused on the affected source/script
surfaces so reviewers can audit the history against the original PRs.

Constraint: Owner-directed rollback to pre-jonckr runtime behavior after CI/base-red fallout
Constraint: Must revert merge commits #2175 #2176 #2177 #2178 #2179 with mainline parent 1 semantics
Rejected: Manual file-by-file rollback | weaker audit trail and easier to miss behavior edges
Rejected: Partial rollback that keeps #2177-#2179 fixes | conflicts with the requested rollback scope
Confidence: medium
Scope-risk: moderate
Reversibility: clean
Directive: Reassess the reverted hook contract, fd-cleanup, and macOS version-sync behavior before reintroducing any of these changes
Tested: npx vitest run src/team/__tests__/tmux-session.test.ts
Tested: npx vitest run src/__tests__/pre-tool-enforcer.test.ts src/__tests__/bedrock-lm-suffix-hook.test.ts
Tested: npx vitest run src/hooks/persistent-mode/stop-hook-blocking.test.ts
Tested: node version consistency check for package/plugin/marketplace versions
Tested: npx tsc --noEmit
Tested: npm run lint
Tested: npm run build
Tested: npm pack + local prefix install smoke (`omc --version`, `omc --help`)
Not-tested: Full `npm test -- --run` remains red on `src/__tests__/hooks-command-escaping.test.ts` with an unrelated Windows-style plugin-root path failure outside the reverted files
Related: PR #2175
Related: PR #2176
Related: PR #2177
Related: PR #2178
Related: PR #2179
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