fix(daemon): tell Fullstack QA Loop Coder to escalate no-code tasks via Runtime Execution Contract - #2298
fix(daemon): tell Fullstack QA Loop Coder to escalate no-code tasks via Runtime Execution Contract#2298lsm wants to merge 4 commits into
Conversation
…o space-agent The Fullstack QA Loop Review handoff is guarded by a PR-ready hook, so validation-only cycles (no code changes) were blocked when the Coder tried to send a pr_url handoff. Mirror the Coding Workflow behavior on dev: instruct the Coder to message space-agent for re-routing instead of creating an empty PR. Add a prompt-drift patch variant so existing seeded spaces are restamped with the new guidance.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1959955f43
ℹ️ 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".
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by glm-5.1[1m] (GLM)
Model: glm-5.1[1m] | Client: NeoKai | Provider: GLM
Recommendation: REQUEST_CHANGES
The change is small, correct, and well-tested. I independently verified the patch-variant semantics, confirmed space-agent is reachable from the Fullstack Coder, confirmed parity with the regular Coding Workflow, and re-ran the targeted suite locally (300 pass, 0 fail). One finding blocks approval.
P1 — Don't hardcode the space-agent literal in a slot prompt (built-in-workflows.ts:399)
CLAUDE.md ("Space agent user message") requires slot prompts to be behavioral only — peers/channels/gate-IDs/reviewer-chrome are injected centrally; restating them creates drift. FULLSTACK_CODING_NOCHANGE_GUIDANCE hardcodes the literal space-agent target, but that target is centrally maintained: AgentMessageRouter resolves it as a reserved escalation target (agent-message-router.ts:686) and adds it to every agent's reachable-targets list (agent-message-router.ts:726), so it already appears in the Runtime Execution Contract's permitted targets. The slot literal is therefore duplication that goes stale if the escalation address ever changes.
Suggested fix: keep the behavior, drop the literal — e.g. "escalate via send_message to the escalation target listed in your Runtime Execution Contract, then stop and wait for re-routing." Since the contract already names it, the slot only needs the behavior.
Context: this is a pre-existing pattern — the regular CODING_WORKFLOW (built-in-workflows.ts:458) and the guidance at L1550 hardcode the same literal. Ideally fix all three together via central injection; at minimum, don't perpetuate it in the new constant. Flagged P1 because the slot-prompt drift system (the very BUILT_IN_PROMPT_PATCH_VARIANTS mechanism this PR extends) exists to prevent exactly this class of drift.
Verified (no findings)
- Patch-variant entry
[[FULLSTACK_CODING_NOCHANGE_GUIDANCE, '']]synthesizes "template minus guidance" = the stale persisted form, whichpatchKnownBuiltInPromptDriftthen recognizes and restamps. Correct, load-bearing, correctly ordered in the chain. space-agentis reachable from the Fullstack Coding node (central router bypass; not a declared channel). The guidance is actionable.- "Mirror Coding Workflow" claim is accurate — regular CODING_WORKFLOW has substantively identical guidance.
bun test ./tests/unit/5-space/workflow/built-in-workflows.test.ts→ 300 pass, 0 fail (local).
Remove the literal escalation target from the Fullstack QA Loop and Coding Workflow no-code guidance so the prompts do not duplicate runtime topology. The Runtime Execution Contract already lists the permitted escalation target; agents should escalate to that target without hard-coding its name.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ada728c4b
ℹ️ 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".
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by glm-5.1[1m] (GLM)
Model: glm-5.1[1m] | Client: NeoKai | Provider: GLM
Recommendation: REQUEST_CHANGES
The literal space-agent is now removed from all three slot-prompt sites, and the focused suite passes (300/300, re-run locally). But the fix is incomplete: the reference that replaces the literal is not backed by the contract, so the no-PR escape path this PR exists to fix can still stall. Two findings remain — both independently flagged by Codex on this revision, and verified in source here.
P1 — The "escalation target listed in your Runtime Execution Contract" does not exist there (built-in-workflows.ts:399, :457)
The slot prompts now tell the Coder to use "the escalation target listed in your Runtime Execution Contract." But buildNodeExecutionRuntimeContract (task-agent-manager.ts:2424-2488) renders only tools, gated channels, and the generic line "Only contact the task-agent via send_message if you are blocked." It never names space-agent and never designates an escalation target. space-agent surfaces only as a tool result from list_reachable_agents / list_peers (node-agent-tools.ts:855, :1344) — i.e., the agent must call a discovery tool first; the contract does not list it. So the slot points at a contract entry that is not there, and an agent that does not think to call list_reachable_agents can stall at exactly the no-PR escape path this change is meant to fix.
This is the central-injection half of the original Codex suggestion, and it was not done. Fix: inject the escalation target into the contract once, centrally — e.g. add to buildNodeExecutionRuntimeContract a line like - Escalation: send_message({ target: "space-agent", message }) requests human/space-level judgment (use for misrouted no-code tasks or hard blockers). Putting it in the contract is correct — the contract IS the central injection point CLAUDE.md L167-170 calls for (not a slot prompt); the slot prompts can then reference it truthfully.
P2 — No restamp variant for the immediate-predecessor Coding wording (built-in-workflows.ts:1551)
The CODING_WORKFLOW step-7 wording changed from "send a message to space-agent" to the runtime-contract reference, but no BUILT_IN_PROMPT_PATCH_VARIANTS entry was added for that predecessor. Spaces seeded from the parent revision still carry the space-agent literal; patchKnownBuiltInPromptDrift only recognizes retired forms synthesized from BUILT_IN_PROMPT_PATCH_VARIANTS, and the existing no-change entry maps the new text to the much older "Validation Complete" wording — not the immediate predecessor. So those seeded spaces keep the hardcoded topology indefinitely. Fix: define the predecessor (space-agent) step text as a RETIRED constant and add [[CURRENT_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT, RETIRED_..._SPACEAGENT_STEP_PROMPT]] — parallel to the Fullstack variant ([[FULLSTACK_CODING_NOCHANGE_GUIDANCE, '']]) that already handles the Fullstack side.
Verified (no findings)
- Targeted suite: 300 pass, 0 fail (local).
- Patch-variant mechanism and candidate-chaining re-confirmed; the Fullstack variant is load-bearing.
space-agentreachability from the Coder (central router bypass) unchanged.
…estamp Coding predecessor - Inject WORKFLOW_ESCALATION_TARGET into task-agent-manager's fallback and workflow Runtime Execution Contract so slot prompts can truthfully refer to a centrally listed escalation target. - Add RETIRED_PREVIOUS_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT and patch variant so spaces seeded with the hard-coded space-agent literal restamp to the runtime-contract reference. - Add task-agent-manager-contract.test.ts and a Coding restamp test.
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by glm-5.1[1m] (GLM)
Model: glm-5.1[1m] | Client: NeoKai | Provider: GLM
Recommendation: APPROVE
Both round-2 findings are resolved correctly and tested; no new findings.
P1 resolved — escalation target centrally injected (task-agent-manager.ts:133, :2404, :2487)
WORKFLOW_ESCALATION_TARGET = 'space-agent' is now injected into both the fallback and the workflow Runtime Execution Contract as Escalation: send_message({ target: "space-agent", message }) requests human/space-level judgment (use for misrouted no-code tasks or hard blockers). The contract now actually lists the target, so the slot prompts' behavioral reference ("the escalation target listed in your Runtime Execution Contract") is truthful. The target is named once, centrally — the CLAUDE.md L167-170 pattern — so a future rename is a single-constant change, and the slot prompts never hardcode it. Verified the line renders for both no-workflow and in-workflow sessions via the new task-agent-manager-contract.test.ts.
P2 resolved — predecessor restamp variant added (built-in-workflows.ts:1554, :1684)
RETIRED_PREVIOUS_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT captures the immediate space-agent predecessor, and [[CURRENT_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT, RETIRED_PREVIOUS_...]] restamps seeded spaces carrying that literal to the runtime-contract wording. The new restamp test confirms a persisted space-agent step-7 prompt is rewritten and no longer contains the literal. Candidate-chaining means both the older Validation-Complete form and this predecessor are now recognized.
Verified
bun test task-agent-manager-contract.test.ts built-in-workflows.test.ts→ 303 pass, 0 fail (local).- Slot prompts at built-in-workflows.ts:399 and :457 reference the contract behaviorally; no remaining hardcoded
space-agentin any slot prompt. - Fullstack side remains covered by its own variant; no regressions.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a116b11e6e
ℹ️ 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".
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by glm-5.1[1m] (GLM)
Model: glm-5.1[1m] | Client: NeoKai | Provider: GLM
Recommendation: REQUEST_CHANGES — approval on hold
My round-3 diff review found zero issues and I posted APPROVE, but Codex then surfaced a valid P2 on this head that I concur with, so approval is on hold until it is addressed. (The QA handoff is gated on a Codex +1, which will not land while this is open.)
P2 — Add the Fullstack predecessor restamp variant (built-in-workflows.ts:1706-1708)
The Fullstack no-code guidance underwent the same space-agent → runtime-contract wording change as CODING_WORKFLOW step 7 (in commit 5ada728). The Coding side got a predecessor variant ([[CURRENT_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT, RETIRED_PREVIOUS_...]]), but the Fullstack side only has [[FULLSTACK_CODING_NOCHANGE_GUIDANCE, '']], which recognizes the guidance entirely absent — not the prior space-agent wording. So a persisted Fullstack prompt carrying the space-agent guidance paragraph is not recognized by patchKnownBuiltInPromptDrift and will not restamp.
Fix (parallel to the Coding predecessor): define the prior Fullstack guidance (the commit-1959955 space-agent wording) as a RETIRED constant and add [[FULLSTACK_CODING_NOCHANGE_GUIDANCE, RETIRED_PRIOR_FULLSTACK_NOCHANGE_GUIDANCE]], plus a restamp test.
Impact note: lower than the Coding predecessor — the Fullstack space-agent wording only existed in this PR's first commit (never on dev), so the affected population is spaces seeded from that in-flight revision, if any. But having just added the Coding predecessor for the identical class of change, the Fullstack side should be symmetric.
Still verified (unchanged from round 3)
- 303 pass / 0 fail across both test files (local).
- P1 (central contract injection) and the Coding P2 (Coding predecessor restamp) remain correctly resolved.
Define RETIRED_PREVIOUS_FULLSTACK_CODING_NOCHANGE_GUIDANCE and a BUILT_IN_PROMPT_PATCH_VARIANTS entry so persisted Fullstack prompts carrying the prior hard-coded space-agent literal restamp to the runtime-contract reference. Add a matching restamp test.
lsm
left a comment
There was a problem hiding this comment.
🤖 Review by glm-5.1[1m] (GLM)
Model: glm-5.1[1m] | Client: NeoKai | Provider: GLM
Recommendation: APPROVE
Round-3 P2 resolved correctly and tested; no new findings. Restamp coverage is now symmetric across Coding and Fullstack.
P2 resolved — Fullstack predecessor restamp variant added (built-in-workflows.ts:402, :1714)
RETIRED_PREVIOUS_FULLSTACK_CODING_NOCHANGE_GUIDANCE captures the commit-1959955 space-agent wording exactly, and [[FULLSTACK_CODING_NOCHANGE_GUIDANCE, RETIRED_PREVIOUS_...]] restamps persisted Fullstack prompts carrying that literal to the runtime-contract reference. The new restamp test confirms a space-agent Fullstack prompt is rewritten and no longer contains the literal. The Fullstack side is now symmetric with the Coding predecessor.
Cumulative state (all rounds)
- P1 (round 2): central
WORKFLOW_ESCALATION_TARGETinjection into both contract paths — resolved. - P2 (round 2): Coding predecessor restamp variant — resolved.
- P2 (round 3): Fullstack predecessor restamp variant — resolved.
- Slot prompts are behavioral; the escalation target is named once, centrally; all predecessor restamps (Coding step-7, Fullstack absent, Fullstack space-agent) are present.
Verified
bun test task-agent-manager-contract.test.ts built-in-workflows.test.ts→ 304 pass, 0 fail (local).RETIRED_PREVIOUS_FULLSTACK_CODING_NOCHANGE_GUIDANCEmatches the original commit-1959955 wording exactly (strict-equality restamp will match).- No remaining hardcoded
space-agentin any slot prompt.
Problem\n\nThe Fullstack QA Loop Review handoff is guarded by a
pr_readyhook that requires an open, mergeable PR with no unresolved review threads. For validation-only tasks that produce no code changes (e.g. diagnostics, goal check-ins), the Coder had no PR to hand off, sosend_messageto Review was blocked.\n\n## Fix\n\nMirror the current Coding Workflow behavior ondev: add explicit guidance to the Fullstack Coder prompt that a no-code task is misrouted in this workflow. Instead of creating an empty commit/PR, the Coder should escalate viasend_messageto the escalation target listed in the Runtime Execution Contract and wait for re-routing.\n\n- AddedFULLSTACK_CODING_NOCHANGE_GUIDANCEinpackages/daemon/src/lib/space/workflows/built-in-workflows.ts.\n- Injected a centralWORKFLOW_ESCALATION_TARGETinto the Runtime Execution Contract assembled inpackages/daemon/src/lib/space/runtime/task-agent-manager.tsso the slot prompt reference is backed by the contract.\n- Removed the hard-codedspace-agentliteral from the Coding Workflow inline step 7 andCURRENT_CODING_WORKFLOW_NOCHANGE_STEP_PROMPT.\n- AddedRETIRED_PREVIOUS_CODING_WORKFLOW_NOCHANGE_STEP_PROMPTand aBUILT_IN_PROMPT_PATCH_VARIANTSentry so existing seeded spaces carrying the old literal are restamped to the runtime-contract reference.\n- AddedRETIRED_PREVIOUS_FULLSTACK_CODING_NOCHANGE_GUIDANCEand a matchingBUILT_IN_PROMPT_PATCH_VARIANTSentry for the same Fullstack predecessor.\n\n## Tests\n\n- AddedFULLSTACK_QA_LOOP_WORKFLOW coder prompt instructs runtime escalation for no-code tasks.\n- AddedpatchKnownBuiltInPromptDrift rewrites persisted Fullstack Coder prompt missing no-code guidance.\n- AddedpatchKnownBuiltInPromptDrift rewrites persisted Fullstack Coder prompt with space-agent literal to runtime contract.\n- AddedpatchKnownBuiltInPromptDrift rewrites persisted Coding coder step 7 (space-agent literal -> runtime contract).\n- Addedpackages/daemon/tests/unit/5-space/runtime/task-agent-manager-contract.test.tsverifying the escalation target line appears in both fallback and workflow Runtime Execution Contracts.\n\n## Verification\n\n-bun test --preload=./tests/unit/setup.ts ./tests/unit/5-space/workflow/built-in-workflows.test.ts→ 302 pass, 0 fail.\n-bun test --preload=./tests/unit/setup.ts ./tests/unit/5-space/runtime/task-agent-manager-contract.test.ts→ 2 pass, 0 fail.\n-./scripts/test-daemon.sh→ all daemon unit shards pass except two pre-existing, unrelated failures inprovider-service.test.tsandspace-mcp-handlers.test.ts(also fail ondevin this environment).\n- Typecheck and lint pass.