Repository navigation
docs(wish): perfect-spawn-hierarchy — plan ghost-approval deadlock fix - #1133
Conversation
Authoritative wish for our worst active bug — native-team spawns persist `leadSessionId: "pending"`, permission_requests pile up in an unreadable inbox (44 unanswered since 2026-03-26), teammates see "The user doesn't want to proceed" and silently give up. Plan covers: - Kill every `'pending'` literal in production spawn paths (team-auto-spawn.ts:144, session.ts:77) - New helper `resolveSpawnerSessionId(cwd)` — fail loud, no fake fallbacks - Spawner-as-leader in every spawn path (genie spawn, team-auto-spawn, session, protocol-router-spawn) - Three-layer hierarchy via nearest-live-ancestor lookup so master doesn't drown when task-leads spawn underlings - `genie doctor` Team Health section (4 checks) + `--fix` auto-remediation - Auto-approver daemon with trust-on-team-membership + audit log - Migration backfill for existing "pending" teams - Regression tests incl. end-to-end 3-layer test + anti-regression lint sentinel Third instance of this bug class in 8 days (7c21301, #1094, this one). Architecture fix, not another patch. Status: DRAFT — pending /review plan gate.
|
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive plan to resolve the 'Ghost-Approval Deadlock' in Claude Code by establishing a robust three-layer spawn hierarchy (master → task-lead → underlings). The changes involve eliminating 'pending' session placeholders, implementing a real-time parent session resolution helper, and adding an auto-approver daemon for non-TUI leaders. Additionally, it enhances 'genie doctor' with team health checks and automated remediation. Feedback focuses on correcting the environment variable precedence in session resolution to ensure correct hierarchy routing, clarifying the relationship between new and existing team configuration fields, and suggesting liveness checks for session heuristics to prevent selecting crashed sessions.
| - Priority 3: `resolveSpawnerSessionId()` (the new helper). | ||
| - Priority 4: **throw** — no fake string fallback. | ||
| 3. `GENIE_PARENT_SESSION_ID` env var is set on every spawned subprocess (`buildTeamLeadCommand`, `buildSpawnParams`, and any other launcher) so grandchildren can read it for their own resolution. | ||
| 4. `resolveSpawnerSessionId()` checks `GENIE_PARENT_SESSION_ID` before it checks `CLAUDE_CODE_SESSION_ID` — this is how grandchildren find their direct parent instead of the root. |
There was a problem hiding this comment.
There is a logic error in the proposed resolution order. If resolveSpawnerSessionId checks GENIE_PARENT_SESSION_ID before CLAUDE_CODE_SESSION_ID, a task-lead (which has both env vars) will return its own parent's ID instead of its own. This would cause the spawnee to route to the master session, defeating the "three-layer hierarchy" goal where underlings should route to their immediate task-lead. CLAUDE_CODE_SESSION_ID must take precedence to ensure the immediate spawner becomes the parent.
| 4. `resolveSpawnerSessionId()` checks `GENIE_PARENT_SESSION_ID` before it checks `CLAUDE_CODE_SESSION_ID` — this is how grandchildren find their direct parent instead of the root. | |
| 4. resolveSpawnerSessionId() checks CLAUDE_CODE_SESSION_ID before it checks GENIE_PARENT_SESSION_ID — this is how grandchildren find their direct parent (the task-lead) instead of the root master session. |
| **Goal:** Make every production spawn path resolve a real session ID before calling `ensureNativeTeam()`. Eliminate the `'pending'` literal. | ||
|
|
||
| **Deliverables:** | ||
| 1. New exported function `resolveSpawnerSessionId(cwd?: string): Promise<string>` in `src/lib/claude-native-teams.ts`. Strategy: `CLAUDE_CODE_SESSION_ID` env var → newest `.jsonl` in `~/.claude/projects/<sanitized-cwd>/` → throw `GhostLeaderError` with an actionable message. (Do NOT fall back to `"genie-${team}"` or any other synthetic string.) |
There was a problem hiding this comment.
The strategy for resolveSpawnerSessionId relies on the "newest" .jsonl file. However, if the newest session is dead (e.g., crashed) and an older session is still alive, this heuristic will pick the dead one, potentially leading to another ghost-approval deadlock. Consider adding a liveness check (e.g., verifying the associated tmux pane is still active) before selecting the session ID.
| **Deliverables:** | ||
| 1. When the inbox-watcher or any process resolves "who is the team-lead for this team", it walks the hierarchy by reading team configs and finding the nearest ancestor whose session is still alive (JSONL exists and was modified recently, AND its tmux pane is alive per `isPaneAlive`). | ||
| 2. New helper `resolveNearestLiveAncestor(childSessionId: string): Promise<string>` in `src/lib/claude-native-teams.ts`. Walks `parentSessionId` → `grandparentSessionId` until it finds one that's alive. Throws if none are. | ||
| 3. Team config schema gains an optional `taskLeadSessionId` field. When set, the inbox-watcher treats THAT as the effective leader for the team, not the master. Set by the spawn layer whenever a task-lead is introduced (i.e. whenever a spawned worker itself spawns workers). |
There was a problem hiding this comment.
Group 3 introduces taskLeadSessionId, but Group 2 and the existing TeamConfig (in src/lib/team-manager.ts) use nativeTeamParentSessionId. It is unclear if these are intended to be distinct fields or if taskLeadSessionId is a rename. Clarifying the schema change here will prevent implementation drift during Wave 2.
| 3. Team config schema gains an optional `taskLeadSessionId` field. When set, the inbox-watcher treats THAT as the effective leader for the team, not the master. Set by the spawn layer whenever a task-lead is introduced (i.e. whenever a spawned worker itself spawns workers). | |
| 3. Team config schema gains an optional taskLeadSessionId field (distinct from the root nativeTeamParentSessionId). When set, the inbox-watcher treats THAT as the effective leader for the team, not the master. Set by the spawn layer whenever a task-lead is introduced (i.e. whenever a spawned worker itself spawns workers). |
| 4. **Default-on for non-TUI leaders.** A leader is "non-TUI" if its session JSONL does NOT contain a `custom-title` entry or its agent-type is not `"team-lead"`. TUI sessions surface prompts to the user via Claude Code's built-in UI and must not be overridden. | ||
| 5. **Config toggle:** `genie config set autoApprove.enabled false` disables it globally. Default: `true`. | ||
| 6. **Audit log** at `~/.genie/auto-approve.log` — one line per decision: timestamp, team, request_id, agent_id, decision (approved/skipped), reason. | ||
| 7. **Rate limit:** no more than 100 approvals per minute per team (guardrail against runaway loops). |
There was a problem hiding this comment.
The rate limit of 100 approvals per minute is a good safety measure, but the plan doesn't specify where the rate-limit state is persisted. If it is purely in-memory, a daemon restart or multiple watchers could bypass it. Consider specifying if this should be persisted or if in-memory is sufficient for the expected failure modes.
| 7. **Rate limit:** no more than 100 approvals per minute per team (guardrail against runaway loops). | |
| 7. Rate limit: no more than 100 approvals per minute per team (in-memory counter; guardrail against runaway loops). |
docs(wish): perfect-spawn-hierarchy — plan ghost-approval deadlock fix
All six wishes have their deliverables merged to dev via the PRs listed below. Moving them from active tracking into _archive/ keeps the wish directory scoped to in-flight and draft work. | Wish | Shipped via | |-----------------------------|------------------------| | velocity-dashboard | #1142 + #1143 (hotfix) | | fix-ghost-approval-p0 | #1134 | | perfect-spawn-hierarchy | #1133 | | brain-help-passthrough | #1129 | | scaffold-auto-memory | #1128 | | unify-bridge-revamp-skills | #1125 + #1126 + #1127 | No code changes — purely a directory reorganization. Wish artifacts remain readable at .genie/wishes/_archive/<slug>/WISH.md for historical reference. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds an optional `team` field to `DirectoryEntry` and populates it in `resolve()` via a direct lookup against the `agent_templates` PG row. This makes the template-pinned team available to spawn-time resolvers without requiring them to re-query the templates table themselves. This is tier 2 of the tui-spawn-dx Group 1 four-tier team-resolution precedence. `agent.entry.team` is an authoritative PG lookup — NOT a synthetic tmux/env fallback — so it does not violate the UUID-first principle established by the perfect-spawn-hierarchy wish (PR #1133 merge 8a78346, PR #1134 merge 6921574, commits 6389f39, d44a6b9, 0b98bec, 4100d29). Refs: tui-spawn-dx (Group 1)
…AndResume
Replaces the two-tier fallback (`options.team || discoverTeamName()`)
with an explicit four-tier precedence that restores the canonical-UUID-
per-agent invariant when spawning from a tmux session that does not
match the agent's home team.
Precedence (highest wins):
1. `options.team` — the `--team` flag. Only tier that flips
`teamWasExplicit` to true.
2. `agent.entry?.team` — template-pinned team from the
`agent_templates` PG row. Authoritative PG lookup, NOT a synthetic
tmux/env fallback.
3. `process.env.GENIE_TEAM` — session-scoped env var (short-circuit
before the heavier discovery path).
4. `await discoverTeamName()` — retained PR #1164 fallback (JSONL
leadSessionId match → tmux session name). Fires only when every
higher-priority tier is empty.
Extracts the pure precedence logic into `resolveTeamName(opts)` and
makes it exported for unit testing. `resolveTeamAndResume` now takes
the resolved agent as a parameter so `agent.entry.team` is in scope.
Reproducer now fixed: running `genie spawn simone` from inside the
`genie` tmux session (where `discoverTeamName` would return 'genie'
via PR #1164's tmux fallback) now resolves to `simone` because the
template pins it. Full regression test covers this case.
Authority: perfect-spawn-hierarchy wish — PR #1133 merge 8a78346,
PR #1134 merge 6921574 (commits 6389f39 kill-pending,
d44a6b9 remove-hardcoded-genie-from-team-flag,
0b98bec remove-genie-literal-from-dispatch,
4100d29 remove-genie-literal-from-resume).
Refs: tui-spawn-dx (Group 1)
Adds the three-branch state machine that gates canonical vs parallel row creation based on pane liveness — no new flags, semantics are invisible to the caller. State machine (in handleWorkerSpawn, after resolveTeamAndResume): - No row with id=<name> → create canonical (workerId=<name>) - Alive canonical → create parallel (workerId=<name>-<sN>) - Dead canonical → findDeadResumable handles it (unchanged) Parallels mint their OWN fresh Claude session UUID and derive a deterministic short-id (<sN>) from that UUID's hex prefix — never a random or minted token. On collision, the slice extends one char at a time (s5, s6, …) of the SAME UUID until unique. This keeps row ids in lockstep with session UUIDs and makes parallels resumable only by their full id. Canonical rows are never clobbered by parallel creation: parallels get their own row with their own UUID, preserving the perfect-spawn-hierarchy canonical-UUID invariant (PR #1133/#1134 merge 6921574). Helpers introduced (exported for unit testing): - resolveSpawnIdentity(name, team, uuidFactory?, isAliveFn?) - pickParallelShortId(baseName, team, uuid) — rejects non-UUID input - findDeadResumable now exported (for off-auto-resume-path tests) rejectDuplicateRole is retained for the legacy explicit --role path; for single-verb spawns it no longer fires (alive collision = parallel). Authority: tui-spawn-dx wish, Group 2. Builds on Wave 1 (merge 9321dd6, commits 79cbe06 + 898c219). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
Authoritative plan for our worst active bug: native-team spawns persist
leadSessionId: "pending", permission_requests pile up in unreadable inboxes (44 unanswered since 2026-03-26), teammates see"The user doesn't want to proceed with this tool use"and silently give up.This PR is a DRAFT wish — no code changes. Awaiting
/reviewplan-gate approval before/workdispatches execution groups.What this wish delivers
'pending'literal in production spawn paths (team-auto-spawn.ts:144,session.ts:77)resolveSpawnerSessionId(cwd)— single source of truth for caller's real Claude Code session ID. Fail loud, no fake"genie-${team}"fallbacks.genie spawn,team-auto-spawn,session,protocol-router-spawn) — the caller automatically becomes the spawnee's team-lead at spawn timegenie doctorTeam Health — 4 new checks (stale leadSessionId, inbox backlog, teammateMode, pane liveness) +--fixauto-remediation~/.genie/auto-approve.logleadSessionId: "pending"(idempotent)Why a wish (not a direct fix)
Third instance of this bug class in 8 days — commit
7c21301a6(2026-04-02teammateModebypass), issue #1094 (2026-04-09claude-sdk.tsspread override), and this new one all share the "ghost/self approval" symptom through different code paths. We keep patching individual leaks while the architecture leaks. This wish is the architecture fix.Full trace + primary/secondary/tertiary root causes + 8 decisions with rationale are documented in the WISH.md.
Execution plan
3 waves, 8 groups — Wave 1 has 3 independent streams that can start in parallel (Groups 1, 4, 8). Waves 2-3 build on the foundation. Review gate between waves.
See
.genie/wishes/perfect-spawn-hierarchy/WISH.mdExecution Strategy section.Related
7c21301a6(2026-04-02) — partialensureTeammateBypassPermissions()fixclaude-sdk.tsspread-override variantTest plan
This PR only adds a markdown planning document — no code changes. CI should be green on the existing full suite (2436 tests).
bun run check— 2436 pass / 0 fail/wishskill template/reviewplan-gate approval before moving to/work