fix(dispatch): #1589 — phantom dispatch (cwd, observability, status filter)#1599
Conversation
…ilter) `genie work <slug>` advanced PG state to `in_progress` but never spawned a real worker. Three architectural gaps allowed the bug to ship: 1. **Spawn cwd silently overridden.** `dispatch.ts` forwarded `team` but not `cwd` to `handleWorkerSpawn`. `agents.ts:2393` then clobbered `agent.repoPath` with `teamConfig.worktreePath`, so wish dispatches landed in the team's default worktree (e.g. `dogfood-session`) instead of the wish worktree. The agent ran in the wrong repo and never saw the wish files. 2. **Readiness probe warned and proceeded.** `awaitAgentReadiness` `console.log`'d on timeout and returned successfully — making silent spawn failures invisible to dispatch and to operators. 3. **Wish-status surfaced unrelated team-mates.** `printWishExecutors` listed every team agent with a current executor and only relabeled the task column based on `assignment.wishSlug`. The dog-fooder's long-running `engineer` appeared under every wish's "Active Executors", masking phantom dispatches. Fixes: - Add `cwd` forwarding to all 4 `handleWorkerSpawn` callsites in dispatch.ts (brainstorm, wish, work, review). Gate the team-worktree override on `!options.cwd` so explicit cwd from dispatch is honored. - Emit a `wish.dispatch.work` audit event from `runWorkDispatch` BEFORE `handleWorkerSpawn` so dispatch is observable in `genie events list` even when spawn no-ops or auto-resumes a stale executor. - Convert `awaitAgentReadiness` timeout from warn-and-proceed to throwing `AgentReadinessTimeoutError` in strict mode (default). Callers can opt back into warn-and-proceed via `SpawnOptions.tolerateReadinessTimeout: true`. - Filter "Active Executors" in `state.ts:printWishExecutors` and `task/status.ts:printActiveExecutors` to skip executors whose `assignment.wishSlug !== slug` (drop the cross-wish bleed-through). Tests: - 7 new regression-guard tests in `dispatch.test.ts` covering all four cwd callsites, the override gate, the dispatch event ordering, the readiness error class, the SpawnOptions/SpawnCtx flag, and both status-display filters. Total: 77 pass (70 existing + 7 new), 0 fail. - Full `bun test src/term-commands` 579 pass, 0 fail. - `bun run typecheck` clean (only pre-existing TUI keymap errors). - `bun run lint` clean (only pre-existing `team-manager.ts` and `buildSpawnParams` complexity, both untouched). Closes #1589 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f98bc550fc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| console.error(` ✗ Agent readiness timeout (${Math.round(result.elapsedMs / 1000)}s) — failing strict mode`); | ||
| throw new AgentReadinessTimeoutError(role, paneId, result.elapsedMs); |
There was a problem hiding this comment.
Transition out of spawning before throwing timeout
The new strict timeout path throws AgentReadinessTimeoutError, but this exception is raised before launchTmuxSpawn runs the state transitions that move executor/agent rows out of spawning (see the transition block immediately after the readiness await). In timeout or false-negative cases, handleWorkerSpawn now exits with rows left in spawning, which can mislead status/recovery flows and leave stale in-progress workers. Catch this path and mark executor/agent as a terminal/error state before rethrowing.
Useful? React with 👍 / 👎.
| if (assignment?.wishSlug !== slug) continue; | ||
| const taskLabel = `Group ${assignment.groupNumber ?? '?'}`; |
There was a problem hiding this comment.
Keep active executors visible when assignment is missing
This filter drops any executor without an active assignment row, but spawn flow does not create assignment records synchronously, so getActiveAssignment(executor.id) can be null even for legitimately running workers. After this change, genie status/genie task status can show no “Active Executors” right after dispatch (and indefinitely where assignments are not written), reducing observability for live work. Preserve visibility for unassigned executors (or fall back to agent metadata) instead of unconditional continue.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Code Review
This pull request addresses the 'phantom dispatch' issue (#1589) by ensuring wish dispatches use the correct working directory, making agent readiness timeouts fatal by default to surface failures, and filtering the active executors list by wish slug to prevent cross-wish bleed-through. Regression tests have been added to verify these fixes. Feedback was provided to improve the documentation of the CWD resolution logic and to fix a typo in a validation command.
|
|
||
| **Acceptance Criteria:** | ||
| - [ ] `SpawnOptions.cwd` is optional and documented in the type definition. | ||
| - [ ] `agents.ts` cwd resolution prefers `options.cwd` → `agent.entry.dir` → `teamConfig.worktreePath` (in that order). |
There was a problem hiding this comment.
The description of the cwd resolution order is a bit simplified. The implementation in agents.ts has a more nuanced precedence, including agent.entry.repo and process.cwd() as fallbacks. For improved clarity and to prevent future confusion, consider updating this acceptance criterion to reflect the full resolution logic:
options.cwdagent.entry.dirteamConfig.worktreePathagent.entry.repoprocess.cwd()
|
|
||
| **Validation:** | ||
| ```bash | ||
| cd /home/genie/workspace/repos/genie && bun test src/term-commands/state.test.ts 2>/dev/null || bun test src/term-commands/state |
There was a problem hiding this comment.
There appears to be a typo in the validation command. The fallback command bun test src/term-commands/state is likely incorrect as state is not a directory of tests. It should probably be bun test src/term-commands/state.test.ts to match the primary command.
| cd /home/genie/workspace/repos/genie && bun test src/term-commands/state.test.ts 2>/dev/null || bun test src/term-commands/state | |
| cd /home/genie/workspace/repos/genie && bun test src/term-commands/state.test.ts 2>/dev/null || bun test src/term-commands/state.test.ts |
Summary
Closes #1589 —
genie workadvanced PG state but never spawned a real worker. Three architectural gaps allowed the bug to ship through4.260430.20–.23:dispatch.tsforwardedteambut notcwdtohandleWorkerSpawn.agents.ts:2393then clobberedagent.repoPathwithteamConfig.worktreePath, so wish dispatches landed in the team's default worktree (e.g.dogfood-session), never the wish worktree.awaitAgentReadinessconsole.log'd on timeout and returned successfully, masking silent spawn failures.printWishExecutorslisted every team agent with a current executor and only relabeled the task column based onassignment.wishSlug— the dog-fooder's long-runningengineerappeared under every wish's "Active Executors", masking phantom dispatches.Fixes
cwd: process.cwd()to all 4handleWorkerSpawncallsites (brainstorm/wish/work/review). Emitwish.dispatch.workaudit event beforehandleWorkerSpawnso dispatch is observable ingenie events listeven when spawn no-ops or auto-resumes a stale executor.!options.cwdso explicit cwd from dispatch is honored. AddAgentReadinessTimeoutErrorand convertawaitAgentReadinessto throw in strict mode (default). AddtolerateReadinessTimeout?: booleanopt-out toSpawnOptions+SpawnCtxfor legacy fire-and-forget callers.if (assignment?.wishSlug !== slug) continuefilter on the executor list so only the wish's actual workers appear under "Active Executors".Test plan
bun test src/term-commands/dispatch.test.ts— 77 pass (70 existing + 7 new regression-guards), 0 fail.bun test src/term-commands— 579 pass, 0 fail.bun run typecheck— clean (afterbun installfor missing@opentui/keymap).bun run lint— clean (only pre-existingteam-manager.ts:617andbuildSpawnParamscomplexity warnings, both untouched).bun run wishes:lint—fix-genie-work-phantom-dispatchwish structurally clean.bun run lint:emit— pass.GENIE_TEAM=genie, rungenie work tui-bottom-bar-opentuifrom~/.genie/worktrees/tui-bottom-bar-opentui. Expected:pgrep -af engineer-1returns a live PID;ls -la /proc/<pid>/cwdshows the wish worktree;genie events list --since 5mshows awish.dispatch.workevent;genie wish status tui-bottom-bar-opentui"Active Executors" shows only the dispatched engineer (no dog-fooder bleed).Files
Authority
.23repro), 4356521047 (cross-wish bleed verification)🤖 Generated with Claude Code