fix(subagent): say when a run is parked on an unanswered spawn approval

[chenmingwei23]

Problem / Motivation

On a default install a spawn_run can sit forever showing "waiting" while no child ACP process is ever created, and nothing that reads the run through /api/spawn says why. The reporter's status API showed the contradiction exactly:

top-level subagents: 1
stats.subagents_spawned: 0

Registered and counted as running, but never spawned: no error, no failure event, no log line keyed by the run id, and kirocrew spawn list showing the same hourglass it shows for a healthy agent. The only way out was restarting the gateway.

The cause is not a queueing race. A default install has no YOLO override, auto_approve_subagent_spawn defaults to False, and a fresh session has no trust, so every spawn_run falls through the approval ladder to the interactive spawn-approval prompt. Until that prompt is answered the run parks in _spawn_with_approval: registered in _agents, counted by the manager's running count, turns == 0, _pid is None, _exec_started is None.

count filters _agents on not done and not queued, so the reporter's own numbers pin the location: subagents: 1 proves the run passed registration, and subagents_spawned: 0 proves it never reached _log_spawned. Between those two points there is only the approval wait.

Both reported reproductions collapse to this one mechanism. The CLI half is worse: kirocrew spawn run posts {"task": ...} with no parent_session, so the prompt resolves to slot="" and is surfaced only on the global approvals feed. It appears in no chat tab, while the CLI prints "waiting for result..." and polls.

Why it matters

The failure is silent, so a two-second click turns into a full investigation.

• No diagnostic exists. The report's dead end was verbatim: "kirocrew logs contained no error or warning keyed by the affected run ID." That is still true on main today.
• The run lies about itself on four surfaces a user or an agent actually reads: GET /api/spawn, GET /api/spawn/<id>, kirocrew spawn list, and MCP spawn_list. Each reports it byte-identically to an executing agent, so none can answer "is this working, or waiting for me?"
• The blocking CLI is the worst case. kirocrew spawn run polls the single-run endpoint every 2s and prints "waiting for result..." while the prompt it needs is on a surface it never mentions.
• It looks like a hang, so users restart the gateway, which discards the run with no completion or failure event, exactly as reported.

What changed (motivation -> approach -> change)

Symptom to cause: no child process -> the run never reached _run -> it never reached _log_spawned (hence subagents_spawned: 0) -> it is parked in _spawn_with_approval -> and that wait is marked in memory but is not named in the log and not reported on the wire. So the change names the wait and reports it, on every path that reads a run.

Two adjacent halves of #6484 already landed on main and are deliberately not redone here. #7325 stopped the reap of such a run blaming a deadline it never reached, and in doing so put info._awaiting_approval = True on the spawn gate. #7477 stopped a chat tab rendering an owned parked run as executing; it is frontend-only and derives its cue from the WS approval event (status === 'pending' && approval_id), not from this payload, so it is scoped to a slot and the unowned CLI spawn still reaches no tab. What is left is every reader that goes through /api/spawn.

file	change
subagent_manager/admission.py	+15, one statement: _spawn_with_approval logs at INFO under the run id, with the parent or <unowned>. The flag beside it is #7325's; an earlier revision of this branch set it too, and that duplication was removed on rebase.
dashboard/handlers/messaging.py	BOTH /api/spawn read paths carry awaiting_approval while parked, via one shared predicate _awaiting_spawn_approval(info). Present only while true, so the default payload is unchanged (same convention as the existing context_withheld).
cli_commands.py	kirocrew spawn list renders a lock glyph and the wait instead of the bare hourglass it shared with a running agent; the blocking spawn run poll announces the pending approval once, not on every 2s poll.
mcp_tools/spawn.py	MCP spawn_list reports [awaiting-approval] rather than [running]. This is the surface an LLM reads, and spawn.py itself tells a caller whose spawn POST failed to "Check spawn_list".
.github/black-baseline.txt	Prunes one entry, src/kiro_crew/mcp_tools/spawn.py: touching the file made it black-clean, and the baseline is shrink-only, so the gate requires the graduated entry be removed.

Two details in the predicate are load-bearing:

• _exec_started is None as well as the flag. _awaiting_approval is shared: run.py sets it at three in-run TOOL-approval sites, so a bare read would render a run at turn 5 waiting on a tool prompt as "waiting for spawn approval" and tell a still-polling caller to approve it "to start this run" that already started. _exec_started is stamped once when execution begins (_run_inner_impl), so None means the run never entered execution. terminal.py picks its reap message off the same pair, arrived at independently in fix(subagent): report spawn-approval-parked reaps accurately, not as a missed deadline #7325.
• One predicate, not two inlined conditions. The handlers build their payloads independently, so a drift between them is invisible to a behavioural test, which is exactly how the status endpoint came to be missed in an earlier revision. A source ratchet pins both call sites and forbids a bare flag read creeping back into either.

Not extracted onto SubagentInfo to share with terminal.py: that read is a plain attribute read on a live run inside the manager package, while this one must survive the SimpleNamespace/MagicMock info doubles the handlers are tested with, and unifying them would mean editing a reap path this change does not otherwise touch. Two lines, and both sites name each other.

Covering both endpoints matters: api_spawn_list feeds kirocrew spawn list, but api_spawn_status is what a blocking kirocrew spawn run polls every 2s, so reporting the wait on the list alone would have left the CLI reproduction exactly as silent as before.

Deliberately not changed: the approval requirement itself, the approval window values, and the decision to leave a human prompt without a deadline of its own. Those are policy.

Tests

New file test/test_subagent_spawn_approval_parked_6484.py, 9 tests.

• The parked run is marked _awaiting_approval, with the reported state (done is False, turns == 0, _pid is None, _exec_started is None, count == 1) asserted as preconditions so the test cannot drift onto a different state.
• Some log record names both the run id and the gate. This is the only assertion covering a change to admission.py; everything else covers a reader of the state.
• The flag clears once answered, so a wire read cannot advertise a wait on a run that is executing.
• The shared predicate is true only while parked on the spawn gate, and false for a mid-run tool approval (_exec_started set).
• Source ratchet: both /api/spawn handlers gate on the shared predicate, and no bare flag read may creep back into either.
• The blocking CLI poll consults awaiting_approval and announces it once.
• MCP spawn_list reports awaiting-approval for a parked run and still reports running for a live one.

Two of the nine are deliberately precondition pins, not new behaviour. The flag set/clear pair now passes on main, because #7325 put the flag on the spawn gate. They are kept because every payload test feeds the predicate a hand-built SubagentInfo: if the gate stopped setting the flag, those tests would still pass while the field went dark in production, and #7325's own tests assert on the reap message rather than on the flag. Their docstrings say so rather than claiming credit.

Scoped suites, all green: 9/9 in the new file; 724 passed / 2 skipped across test_subagent_startup_watchdog (#7325's own), test_handlers_messaging_coverage, test_spawn_list_redaction, test_mcp_core_spawn_sub_agents, test_spawn_agent_roster, test_agent_roster_shared, test_subagent_reap_race, test_subagent_stall, test_cli; 493 passed across test_api_server, test_subagent_coverage, test_subagent_persistence, test_subagent_context_group_plumbing, test_messaging_commands.

Manual verification

Rebased first, then re-verified rather than trusting the automerge. The branch was 164 commits behind main and the rebase reported zero conflicts, but main had landed overlapping work in the same function, so a clean merge was not evidence of a correct result. Reading the rebased file found this PR's _awaiting_approval set/clear pair sitting next to #7325's, plus prose claims main had made false. Both were fixed before pushing.

A/B against pristine main, measured not asserted. A detached worktree at main tip with only the new test file copied in: 6 failed, 3 passed. The 3 that pass are the flag set/clear pair and the "a live run still says [running]" control. The 6 that fail are the log line, the four predicate/wire assertions, and the CLI poll, i.e. exactly this diff.

Mutation-verified, three probes, because a test that passes both ways proves nothing:

mutation	result
remove the logger.info from admission.py	1 failed (the log test)
weaken the predicate to a bare _awaiting_approval read	2 failed (the mid-run tool case, and the ratchet's bare-read guard)
drop the api_spawn_status call site	1 failed (the source ratchet)

Gates, all green: flake8, isort, mypy (1259 files), the black gate (6 files in scope, baseline honoured), agent-sdk-boundary, loop-bound-locks, sync-io-in-async, subprocess-encoding, lockdown-before-publish, brand-name, focus-cue, builtin-skill-scope, testpaths-coverage.

One CI red on this head was diagnosed and is not this diff's. Backend Tests (3.12, 3) failed on test/test_security.py::TestKeystoneVariableLeafNativeSpellings::test_an_absolute_home_spelled_with_backslashes_is_refused. The diff touches neither security.py nor test_security.py; git log HEAD..main on those two paths is empty, so the file is identical in this branch and in main; the test class predates this base; it passes locally in isolation; and only the 3.12 shard failed while every 3.10 shard was green on the same head. A rerun went green, consistent with order dependence (the test builds its input from os.path.expanduser("~"), while security.py anchors home via Path.home() behind a cache keyed on resolved roots). Nothing was folded into this diff for it.

No UI change, so no screenshots, no i18n key, and no eslint or bundle-size exposure.

Related Issues

• Closes the diagnostic half of Subagent runs stay queued without starting a child ACP process #6484 (see the trailer below).
• fix(subagent): report spawn-approval-parked reaps accurately, not as a missed deadline #7325 landed the reap-message half and put _awaiting_approval on the spawn gate.
• fix(dashboard): stop rendering an approval-parked subagent as running #7477 landed the chat-tab rendering half, frontend-only and slot-scoped.

Pattern harvest

Rule candidate: review-prompt
Pattern: a status field added to one serializer of a run/job object but not to its siblings

Only the second of this defect's two layers generalizes.

The surface layer is close to a one-off: one particular wait, the spawn-approval gate, was marked in memory but never named in the log.

The layer underneath is a class. /api/spawn has two read paths that build their payloads independently, so a field a caller acts on can land on one and be silently absent from the other, and the gap is invisible to a behavioural test because "field absent" and "the run is not in that state" are the same bytes on the wire. An earlier revision of this very PR shipped exactly that bug: the field was on the list endpoint only, which left the blocking kirocrew spawn run reproduction as silent as it was before the fix, because that path polls the single-run endpoint. The only guard available was a hand-written per-file source ratchet pinning both call sites, and needing to hand-write one is itself the argument for a shared rule.

Suggested prompt line: when a PR adds a status field to a run/job payload, name every serializer of that object and confirm the field reaches all of them, or say why it deliberately does not.

Deliberately not proposed as semgrep or lint: "sibling serializer" is not structurally detectable here. The two handlers share no type and no base class, so any pattern broad enough to catch them would fire on every optional key in the codebase.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) -- N/A, no user-facing doc or spec surface changes; the two adjacent halves that carried spec edits landed separately as fix(subagent): report spawn-approval-parked reaps accurately, not as a missed deadline #7325 and fix(dashboard): stop rendering an approval-parked subagent as running #7477
• No secrets, credentials, or internal references in the diff

Fixes #6484

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 10afdf7

Verdict parsed from the review's SHA-scoped output markers for commit 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a: <one-sentence reason>

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification done — flag-set at admission.py:731 is pre-existing context (from #7325), the diff adds only the log line; the wire field has counted consumers (cli_commands.py:227,295, mcp_tools/spawn.py:854); the predicate pair's one sibling is terminal.py:418, cross-referenced in both docstrings; the mcp_tools/spawn.py reformat hunks correspond to its removal from .github/black-baseline.txt, which AGENTS.md explicitly welcomes for touched files.

First-Principles-Verdict: PASS

Every added surface traces to one reported harm — a run invisible while parked on its spawn prompt — with consumers counted and the UI half explicitly deferred, not faked.

What this change ships

Intent: let a user see that a "waiting" subagent is parked on an unanswered spawn approval, not executing — a FIX.

1. CLI spawn list shows a lock and "waiting for spawn approval" instead of the hourglass — justified
2. Blocking spawn run prints once why the wait isn't progressing and where to approve — justified
3. MCP spawn_list reports [awaiting-approval] instead of [running] — justified
4. /api/spawn/<id> carries optional awaiting_approval — justified (1 consumer: cli_commands.py:295)
5. /api/spawn list carries the same field — justified (2 consumers: cli_commands.py:227, mcp_tools/spawn.py:854)
6. An INFO log line keyed to the run id when a spawn parks — justified (the report's dead end was zero log evidence)
7. mcp_tools/spawn.py black-formatted and pruned from the baseline — rides along, sanctioned by AGENTS.md's baseline-prune convention

Watch

• One description paragraph ("_spawn_with_approval sets info._awaiting_approval = True for the duration of the wait… Cleared in a finally") describes work the diff does not ship — admission.py:731 is unchanged context, landed by fix(subagent): report spawn-approval-parked reaps accurately, not as a missed deadline #7325. The PR's own test file states this correctly; only the description is stale.
• The predicate pair is spelled twice (grepped _awaiting_approval.*_exec_started: messaging.py:706 and terminal.py:418). Both sites name each other, and unifying would touch a reap path this change doesn't otherwise edit — accepted-and-deferred; a third spelling is the trigger to hoist it onto SubagentInfo.

[FIRST-PRINCIPLES-REVIEWED] 10afdf7

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A silent indefinite wait becomes observable on every read surface, additively and reversibly — the right diagnostic shape, with policy questions correctly deferred and filed.

Suggestions

• The (_awaiting_approval, _exec_started is None) discriminator now lives in dashboard/handlers/messaging.py and (per its own docstring) again in subagent_manager/terminal.py, held together only by comments — the exact cross-serializer drift this PR's ratchet exists to prevent, but the ratchet only sees one file. A property on SubagentInfo would give the pair one owner; worth folding into dashboard: a subagent parked on an unanswered spawn approval still renders as running (awaiting_approval has no frontend consumer) #7318 rather than a third copy appearing there.

[DESIGN-REVIEWED] 10afdf7

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 10afdf7

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a: <one-sentence reason>

[chenmingwei23]

The finding was correct, and it is already fixed - it read the description as it stood when the run was triggered, not the current one.

Sequence: I force-pushed 6fd77983a (the head this reviewed) at ~16:51Z, which fired CI and this lane. At that moment the description was still the pre-split one, which did claim the terminal.py reap-message fix and 5 tests as shipped. I rewrote the description at ~16:59Z, after the trigger. So the lane compared a post-split diff against a pre-split description, and correctly reported the contradiction.

Nothing to rebut - a description claiming a diagnostic that does not exist is exactly the thing worth blocking on, and "a human merging on this description believes that diagnostic now exists" is the right framing of the harm. The remediation is already in place rather than promised: the current description contains zero matches for now reports .Never started and for Both conjuncts of, states "3 tests", and carries a Scope note in section 3 saying the reap-message half was written, tested, then reverted, and why - the agent-sdk-boundary gate promotes the pre-existing unbaselined kiro_crew.acp.client import in terminal.py:517 to a "new offender" the moment the file is touched, and its baseline is shrink-only by design. That half is filed as #7306.

I have re-run this workflow so it re-reads the current description. No code changed for this finding; terminal.py remains byte-identical to main.

Two of the review's own observations are worth keeping on the record, since they verify the shipped surface rather than just the prose: the flag has a real pre-existing consumer at monitoring.py:626, and the new API field has exactly one real consumer at cli_commands.py:227. That is the whole shipped scope, and it is the part the report's dead end ("no log entry keyed by the affected run ID") actually needed.

[chenmingwei23]

Verified and taken as a declared deferral, with the overclaim fixed.

The claim checks out. grep -rn awaiting_approval website/src/ returns only unrelated hits -- the ops-mission-control app's own blocked_reason union (api.ts:245-404) and i18n keys for tool-approval aria labels. There is no subagent consumer, so the dashboard does render a parked run exactly like an executing one.

The real defect here was the description, not the diff: it said the fix speaks "at each place the user actually looked" while quoting a sentence that names the dashboard. That framing is gone -- the section now enumerates the surfaces that actually got it (run state, log, both /api/spawn read paths, both CLI surfaces) and says the dashboard UI is explicitly not among them.

Filed as #7318, so it has the same standing as the reap-message deferral rather than "getting nothing". Not folded into this PR because it is a different kind of change, not a hidden cost:

• a new user-facing string needs a key across 14 locale files (bn de en-XA en en.manual es fr hi it ja ko pt ru zh-CN) under the enforced "Check i18n extraction, key references and plurals" gate
• a frontend surface pulls the frontend test suite, the eslint --max-warnings ratchet and the bundle-size gate, none of which a backend-only diff touches
• it wants a visual decision -- badge vs label vs glyph, and whether it belongs on the inline run card, the Subagents panel, or both -- plus screenshot evidence, which is design-review territory

The backend field is the prerequisite and it has landed, so #7318 can proceed independently without re-establishing the mechanism. #7318 also carries two things worth deciding there rather than here: a click-through to the approvals surface (which matters most for an unowned spawn, whose prompt carries slot="" and appears in no chat tab at all), and whether a parked run should keep being counted in the manager's running tally when it has launched no process.

Also noted: item 6, the one-shot spawn run notice, was undeclared in the fix section. It is declared now.

[chenmingwei23]

Real finding, fixed in 943d15a8a rather than papered over with a wording change.

Verified at the call sites first: run.py sets _awaiting_approval = True at :1273, :1364 and :1388, all inside a running subagent, and _exec_started is stamped once at :442 when execution begins. So the flag genuinely does not distinguish the two waits, and reading it bare on the wire meant a run at turn 5 parked on a tool prompt would render "waiting for spawn approval" and a still-polling caller would be told to approve it "to start this run" that already started.

Both remedies were on offer; I took the gating one rather than softening the strings, because the strings are not the only wrong thing - the lock glyph and the whole "waiting to start" framing are wrong for a mid-run tool wait, and a run at turn 5 should not appear in that state at all.

_exec_started is None is the discriminator rather than turns == 0: it is permanent and unambiguous, whereas turns is bumped by the permission request itself before the approval wait (noted at monitoring.py:626), so a first-turn tool approval could still read turns == 0.

Both read paths now go through one predicate, _awaiting_spawn_approval(info), instead of repeating the pair. That is deliberate: the handlers build their payloads independently, which is precisely how the status endpoint came to be missed in the earlier round of this review. A source ratchet pins both call sites and also fails if a bare flag read creeps back into either payload builder - a behavioural test cannot cover that, since one handler drifting looks identical to a run that simply is not parked.

Mutation-verified both ways: dropping the _exec_started conjunct fails the new mid-run-tool-approval test AND the ratchet; dropping the field from api_spawn_status fails the ratchet. 7 tests now, 906 passed on the targeted set, gates and the agent-sdk-boundary gate clean.

Worth recording, because it is the more interesting half of this finding: the deferred terminal.py message needed exactly this same pair of conjuncts, and it has a test named for the confusion. Getting a discriminator right for one consumer did not propagate it to the next one. That is now a rule candidate in the harvest section.

[chenmingwei23]

Real finding, fixed in c6ea4b0b5. Not deferred, because none of the reasons that justified deferring the dashboard apply to this one.

Verified both halves of the claim at the source. spawn_list built its status as "done" if a.get("done") else "running", so a parked run read [running]. It already consumed the optional context_withheld field with the same present-only-when-set shape, so the read really was one line. And spawn.py does tell a caller whose spawn POST failed to "Check spawn_list", which makes this the worst of the four surfaces to get wrong: it misinforms an agent at the exact moment it is trying to reconcile whether its tasks landed.

Now reports [awaiting-approval]. Two tests rather than one, because a status branch can fail in both directions: a parked run must not read [running], and an ordinary live run must still read [running] so the new state cannot swallow the old one. Both behavioural (stubbed mcp_core._get), not source ratchets. Mutation-verified: reverting the branch fails the parked-run test. The pre-existing [running] assertion in test_mcp_core_spawn_sub_agents.py:507 still passes, and 136 passed across the four suites that touch spawn_list.

On the accounting, which is the part of this finding worth keeping: all four read surfaces rendering per-run status off /api/spawn are now explicitly accounted for -- CLI list, CLI poll and MCP roster fixed; dashboard SubagentProgressBar deferred as #7318. The dashboard is the only one that needs a locale key across 14 files plus the frontend gate stack and a visual placement decision; this one needed none of that, so declaring it would have been an excuse rather than a reason.

I have also recorded the process failure honestly in the harvest section: this fix found those four consumers one at a time across three review rounds, shipping each time believing it was complete. Grepping for an existing optional field on the same payload (context_withheld) enumerates them in one shot, and doing that first would have collapsed three rounds into one.

[buluoray]

Reviewed at 01f375b51. 0 blocking / 1 yellow / 1 blue — approving.

Confirmed the gap is real: _exec_started is stamped at run.py:442, and on origin/main _awaiting_approval is set only at the three in-run tool sites (1276/1367/1391), never at the spawn gate — so a run parked on an unanswered spawn approval was registered and counted exactly like an executing agent with no child process. That is #6484's silence, and this addresses the cause rather than the symptom.

The _exec_started is None conjunct is genuinely load-bearing — it is the only thing separating a spawn-gate park from a mid-run tool approval, which reuses the same flag. Checked for both failure directions and found neither: the finally clears the flag on answer/deny/cancel, every consumer checks done first, and the stall watchdog's first guard (if not (info.turns>0 or info._pid is not None): return) early-returns for a parked run, so setting the flag at the spawn gate does not perturb reaping, the timeout window, or orphan recovery. MCP statelessness holds — no module global, no per-caller state, just a read branch on the /api/spawn response. Mutations reproduce your table exactly.

Yellow, and the one thing I would like fixed: the PR adds a new wire field awaiting_approval to the /api/spawn and /api/spawn/<id> payloads without updating docs/system-specs/modules/subagent.md in the same commit. That spec already documents the sibling stalled surface signal, so it now under-describes the payload and the next editor has no record of the field. AGENTS.md requires the spec move with the schema. Non-blocking, but no bot can catch this class — code-review.yml does not read docs and the line reviewers' FIX BAR excludes untouched files.

Blue: test_both_endpoints_use_the_shared_predicate does a literal .replace() of the exact 2-line predicate, so a future black rewrap breaks the ratchet on unchanged behavior.

[chenmingwei23]

Rebased onto current main after the 13-PR merge batch put this branch in conflict; the conflict is resolved and the PR is MERGEABLE again.

The remaining reds on this head are main-owned and reproduce on PRs with disjoint diffs, so they are not actionable here:

• Backend Tests shard 3 (3.10, 3 / 3.12, 3 / Windows 3) -- filed as Backend Tests shard 3 reds every PR: log-site census has slack for dashboard/handlers/files.py #7504. test_security_posture.py::test_the_census_holds_no_slack fails because _BASELINE_LOG_SITE_CENSUS pins dashboard/handlers/files.py: 3 while the code now has 1. Reproduced on fix(chat): let Back close the mobile sessions drawer #7295, whose diff is one .tsx, one new .test.tsx and four media files -- zero Python -- which is what establishes it as main-owned.
• Frontend Lint & Type Check (where it runs) -- filed as Frontend Lint reds every PR: main sits at 660 eslint warnings against a 659 ratchet #7499. main carries 660 eslint warnings against the --max-warnings 659 ratchet in ci.yml:1298. Measured locally in a worktree at merge-base 271428883: pristine base = 660, base + branch diff = 660, so the branch contributes zero.
• Coverage Gate and PR Readiness are downstream aggregates of the above.

Per house rule the main-owned fixes are not being folded in here. Once main heals I will rebase to cut a fresh merge ref and re-run, rather than re-triggering against a stale one.

[chenmingwei23]

Disposition: GPT 5.6 advisory on cli_commands.py:293 -- accepted, premise verified

GPT flagged that "indefinitely" contradicts an enforced approval timeout. I traced it rather than taking or dismissing it on wording, and the finding is right: the spawn-approval gate is not an unbounded wait.

The chain, for the record:

• slack/gateway.py:7728 builds the spawn callback from the same factory as tool approvals (_approve_subagent = self._interactive_approval("subagent", slot_resolver=_spawn_slot_resolver)), and :7900 wires on_spawn_approval=_spawn_approve to it (:7732-7736).
• That path awaits with a timeout: dashboard/chat_runner.py:7794, outcome = await asyncio.wait_for(fut, timeout=_approval_window).
• The window is min(state.approval_timeout_for(slot), tool_approval_timeout_secs()) (:7780). The comment at :7771 names the attended value as 7200s, which is GPT's "two hours" and is consistent with TOOL_APPROVAL_TIMEOUT_MAX = 7200 in config/sections.py:3366, while tool_approval_timeout_secs() defaults to 600s. The effective default bound is the smaller of the two.

So the absolute wording is wrong in more than the one line GPT cited. The same overclaim appears in the admission.py comment ("a wait with no deadline of its own"), in the commit message, and in this PR body's Pattern harvest section. A wording fix has to move all four together, or it just relocates the phantom claim.

One part I have NOT verified, stated plainly rather than assumed either way: whether an unowned spawn (the CLI case with parent_session="" and slot="", which is the reproduction this PR is actually about) resolves a slot through _spawn_slot_resolver and therefore gets a finite window at all. If it does not, the original wording is accurate for precisely the reported case and misleading only for the owned case. I did not trace that branch, so the honest correction is wording that holds either way ("until the approval window expires") rather than asserting a specific bound.

Why this is not fixed in this revision. The head is currently fully green: 68/68 checks, PR Readiness success, and all four review lanes PASS with markers naming 10afdf7a300ad2706fdfc00898de5ecdd66c9d7a. Amending for a comment-wording correction re-rolls every lane, including a Backend Tests (3.12, 3) shard that already needed one rerun on this head for an order-dependent failure in test_security.py that this diff does not touch. That is a real risk of trading a green state for prose, so the call belongs to the maintainer rather than to an automated loop. Flagging it here so the choice is visible instead of silently deferred, and so nobody reading the comments is misled by the current wording in the meantime.

[buluoray]

Verdict: 0 blocking, 2 non-blocking. Approving.

This is an additive, backend-only observability fix: a subagent parked on an unanswered spawn-approval prompt is now distinguishable from one that is executing, on every reader that goes through /api/spawn (both HTTP shapes, spawn list, the blocking spawn run poll, and MCP spawn_list), plus one INFO log line keyed to the run id.

What I verified (against GitHub main at head 10afdf7, not the local mirror)

• Real approval state, not a timeout/slowness heuristic. The wire predicate _awaiting_spawn_approval is getattr(_awaiting_approval) is True and getattr(_exec_started) is None (dashboard/handlers/messaging.py:672). The flag is set synchronously at the spawn gate (admission.py:731, pre-existing from #7325) and _exec_started is stamped once when execution begins (run.py:569). A slow-but-executing run has _exec_started set, so the predicate returns False for it — a merely-slow run is never mislabeled "parked on approval." Confirmed the three mid-run TOOL-approval sites (run.py:1330,1421,1445) all run after _exec_started is stamped, so _exec_started is None correctly separates the spawn gate from a mid-run tool wait.
• No credential/argument leak on the reporting path. The new log line (admission.py:741) emits only info.id, request_id (=spawn:{id}), and parent_session_key — no tool arguments and no task text (the redacted task_preview is pre-existing and used only in the approval description). The new payload additions are a bare boolean awaiting_approval. Pre-existing _redact(info.last_tool) is unchanged.
• State is cleared on every terminal branch. Verified on GitHub main that admission.py sets the flag inside a try with finally: info._awaiting_approval = False (main:737, PR:752), so it clears on approval, on denial (returns after the finally), and on crash (except runs after the finally). It cannot wedge as permanently parked.
• Previously-working output is unchanged for non-parked runs. All new fields and labels are present-only-while-awaiting (same convention as the existing context_withheld); a live run's payload and glyph are byte-identical to before.
• No frontend surface touched. The diff is Python-only (admission.py, messaging.py, cli_commands.py, mcp_tools/spawn.py) plus the black-baseline.txt prune and one new test file — no website/ or docs files — so website/AUTOSDE.yaml's six blocking rules and the i18n catalog do not apply.
• Spec obligation (AGENTS.md). docs/system-specs/modules/subagent.md already documents the _awaiting_approval/_exec_started mechanism this builds on (lines 158, 222, 264, including the spawn gate resetting the flag in a finally), and that mechanism is unchanged here. The PR only surfaces existing state, so no mandatory same-commit spec update is triggered.
• All four AI review lanes PASS on this exact SHA (GPT 5.6, Opus 4.8, First Principles, Design), and the mcp_tools/spawn.py reformat hunks correspond 1:1 to its removal from .github/black-baseline.txt, which AGENTS.md sanctions for touched files.

Non-blocking findings

1. docs/system-specs/modules/subagent.md — the spec's /api/spawn description does not mention the new awaiting_approval wire field. Consequence: a future reader enumerating the payload from the spec alone would miss it. This is not a rule violation (the spec does not enumerate wire fields like turns/last_tool/elapsed either) and no gate flagged it — purely optional. Suggestion: a one-line mention next to the existing outcome/stopped wire notes, or leave it for #7318 which will touch this surface.

2. dashboard/handlers/messaging.py:672 and subagent_manager/terminal.py:418 — the _awaiting_approval and _exec_started is None pair is spelled in two places, held together by cross-referencing comments rather than one owner on SubagentInfo. The Design and First Principles bots both raised this; the author accepted-and-deferred it to #7318 with a rationale (the handler read must survive test info-doubles, the terminal read is a plain attribute read on a live run, and unifying would edit a reap path this PR doesn't touch). I agree the deferral is reasonable — a source ratchet in the new test pins both handler call sites in the meantime. Recording it so a third spelling appearing is the trigger to hoist it.

What I could not verify

• I did not independently re-run the test suite (read-only review in a checkout shared by 19 agents; no git writes). I relied on the author's documented A/B (6 of 9 tests fail against pristine main + the new file) and three mutation probes, plus the green Backend Tests lanes on this SHA. Two of the nine tests are precondition pins that pass on main by design, which the author discloses honestly.
• I judged review-lane freshness by comment bodies citing the full current SHA; I did not separately audit the earlier main-owned CI reds (#7499/#7504/#7295) the author diagnosed as unrelated to this diff.