fix(agents): preserve accepted spawn terminal success#85054
Conversation
|
Codex review: needs maintainer review before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source level: current main can accept a child session but the incomplete-turn classifier only recognizes committed messaging delivery before emitting the false warning. I did not run the current-main repro in this read-only review. PR rating What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Risk before merge
Maintainer options:
Next step before merge Security Review detailsBest possible solution: Land this structured accepted-spawn evidence fix after normal maintainer and CI validation, keeping the terminal-success contract limited to accepted status with non-empty run and child session identifiers. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main can accept a child session but the incomplete-turn classifier only recognizes committed messaging delivery before emitting the false warning. I did not run the current-main repro in this read-only review. Is this the best way to solve the issue? Yes. Carrying a structured accepted-spawn fact through the runner is narrower than a config flag or broad tool-success exemption, and the PR preserves malformed/error spawn and timeout paths. Label changes:
Label justifications:
What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against b25a0d013b64. |
Signed-off-by: samzong <samzong.lu@gmail.com>
|
ClawSweeper PR egg ✨ Hatched: 🌱 uncommon Moonlit Shellbean Hatch commandComment Hatchability rules:
Rarity: 🌱 uncommon. What is this egg doing here?
|
0a1e304 to
0261fcd
Compare
|
@clawsweeper automerge |
|
🦞🔧
Draft PRs stay fix-only until GitHub marks them ready for review. Pause with Automerge progress:
|
|
ClawSweeper 🐠 reef update Thanks for the work on this. ClawSweeper did not have permission to update this branch directly, so it opened a narrow replacement PR instead. that's a branch access thing, not a knock on the contribution. Why replacement: ClawSweeper could not update the source PR branch directly; GitHub did not grant sufficient push rights to the bot for that branch.
fish notes: model gpt-5.5, reasoning high; reviewed against c1b3c30. |
Summary
sessions_spawnchild session could still be classified as an incomplete empty parent turn.sessions_spawnresults now requirestatus: "accepted", non-emptyrunId, and non-emptychildSessionKey; that evidence suppresses only the false incomplete-turn path and prevents duplicate/replay fallback paths.main: two qa-lab Codex lifecycle fixtures are now optional Knip unused-file entries so the dependency shard matches the scenario-driven fixture contract.Motivation
Agent couldn't generate a responseanyway.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Real behavior proof (required for external PRs)
sessions_spawnterminal success no longer trips incomplete-turn fallback, abandoned lifecycle state, replay/fallback retries, or duplicate child-session spawn paths.qa-channel+ qa-lab bus + real gateway child +mock-openaiprovider.Agent couldn't generate a responseor re-running the spawn.subagent-completion-direct-fallbackQA path; that broader path was not used as final proof because a separate announce empty-response timeout was observed outside this patch's changed surface.Root Cause (if applicable)
EmbeddedRunAttemptResultor final run results.sessions_spawn.Regression Test Plan (if applicable)
src/agents/pi-embedded-runner/run.incomplete-turn.test.ts,src/agents/pi-embedded-subscribe.handlers.tools.test.ts,src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts,src/agents/pi-embedded-runner/result-fallback-classifier.test.ts,src/agents/pi-embedded-runner/run/attempt-trajectory-status.test.ts.sessions_spawnis terminal outbound progress, malformed/error spawns are not, replay/fallback retries are blocked after acceptance, and prompt timeout still returns timeout payload.User-visible / Behavior Changes
Pure-relay subagent handoffs no longer show a false
Agent couldn't generate a responsemessage after a child session was accepted.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation: N/ARepro + Verification
Environment
mock-openai/gpt-5.5qa-channel+ qa-lab bus + real gateway childSteps
Passed: 1,Failed: 0.Expected
sessions_spawnis preserved as terminal evidence.Actual
Evidence
Human Verification (required)
codex review --commit HEADfound no actionable regressions after fixes.node scripts/run-vitest.mjs src/agents/pi-embedded-runner/run/attempt-trajectory-status.test.ts src/agents/pi-embedded-subscribe.handlers.tools.test.ts src/agents/pi-embedded-runner/run.incomplete-turn.test.ts src/agents/pi-embedded-subscribe.handlers.lifecycle.test.ts src/agents/pi-embedded-subscribe.subscribe-embedded-pi-session.subscribeembeddedpisession.test.ts src/agents/pi-embedded-runner/result-fallback-classifier.test.ts --runpassed: 10 files, 425 tests.pnpm exec tsgo -p test/tsconfig/tsconfig.core.test.json --noEmit --pretty falsepassed.pnpm exec oxfmt --check ...passed on all touched files.git diff --checkpassed.pnpm deadcode:dependencies,pnpm deadcode:unused-files, andpnpm deadcode:report:ci:ts-unusedpassed after rebasing to currentupstream/main.node scripts/run-vitest.mjs test/scripts/check-deadcode-unused-files.test.ts --runpassed: 1 file, 7 tests.Review Conversations
Compatibility / Migration
Risks and Mitigations
sessions_spawnas terminal evidence could hide unrelated timeout failures.