feat: add versioned SpawnSpec axes - #394
Conversation
Issue stable agent identities at spawn, separate authority/function/placement, hard-reject ambiguous Claude jobs, and support plain terminal spawns. Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_673fa45b-4383-457c-83c1-5bfcde51915a) |
📝 WalkthroughWalkthroughThe PR adds SpawnSpec v1 metadata for agent authority, function, and placement. It adds plain-terminal spawning, removes legacy role inference, adds role-based defaults, and keeps agent IDs stable during session capture. ChangesSpawnSpec v1
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant spawn_agent
participant SpawnAxisNormalizer
participant AgentEngine
participant TerminalSurface
Caller->>spawn_agent: Submit SpawnSpec v1
spawn_agent->>SpawnAxisNormalizer: Normalize spawn axes
SpawnAxisNormalizer-->>spawn_agent: Return canonical metadata
alt Agent spawn
spawn_agent->>AgentEngine: Persist and launch agent
AgentEngine-->>spawn_agent: Return stable agent ID and parent ID
else Terminal spawn
spawn_agent->>TerminalSurface: Create terminal and apply cwd
TerminalSurface-->>spawn_agent: Return terminal metadata
end
spawn_agent-->>Caller: Return expanded spawn response
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e2c24c048f
ℹ️ 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".
| const explicitRole = normalizeExplicitRole(agent.role); | ||
| if (explicitRole) return explicitRole; | ||
| throw new AgentRoleInferenceError({ role: agent.role, cli: agent.cli }); |
There was a problem hiding this comment.
When an older persisted record has no role, crash recovery still selects it as eligible, increments respawn_attempts, and then calls inferRecordRole() at src/agent-engine.ts:3140; this new unconditional error therefore consumes an attempt on every sweep until the record is permanently marked exhausted. Since normalizePersistedAgentRecord() explicitly permits a missing role, upgrading can strand otherwise resumable sessions instead of merely quarantining their placement. Resolve or quarantine the role before incrementing recovery attempts so later repair can still resume the agent.
AGENTS.md reference: AGENTS.md:L29-L34
Useful? React with 👍 / 👎.
| const created = await client.newSplit("right", { | ||
| ...(workspace ? { workspace } : {}), | ||
| focus: args.focus, |
There was a problem hiding this comment.
When cwd is supplied while focus is omitted (and therefore false), this creates an unfocused tab and immediately sends cd. The established lifecycle invariant in src/agent-engine.ts:4931-4933 says a newly created unfocused tab does not initialize its terminal, so the default terminal-spawn path can fail or lose the command while returning the requested cwd as successful. Temporarily focus the created surface before the shell I/O, then restore the origin when focus is false.
Useful? React with 👍 / 👎.
| .enum(["orchestrator", "worker", "implementor", "reviewer", "gatherer"]) | ||
| .catch((context) => context.input as AgentRole); |
There was a problem hiding this comment.
The .catch() makes every invalid role value pass schema parsing, not just a supported legacy alias. For example, role:"reviewr" reaches normalizeSpawnAxes(), is silently converted to implementor, and even satisfies the explicit-role check for Claude, causing a successfully spawned agent with different semantics from the request. Accept the intended legacy literal explicitly and let all other values fail validation.
Useful? React with 👍 / 👎.
| if ( | ||
| args.role !== undefined || | ||
| args.authority !== undefined || | ||
| args.placement !== undefined || | ||
| args.worktree !== undefined |
There was a problem hiding this comment.
For type:"terminal", this incompatibility check omits prompt and boot_prompt_path, and the handler returns from the terminal branch before either delivery path runs. A caller can therefore receive ok:true after supplying a task instruction that was never sent. Reject these agent-only fields for terminal specs, or explicitly implement their terminal-delivery semantics.
Useful? React with 👍 / 👎.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| agent.repo, | ||
| identity.session_id, | ||
| ); | ||
| if (!updated.agent_id.includes("-pending-")) { |
There was a problem hiding this comment.
🟠 High src/agent-engine.ts:2337
The substring check updated.agent_id.includes("-pending-") false-positives when the repo name itself contains -pending- (e.g. repo foo-pending-bar produces a golem name like foo-pending-barClaude-<uuid>). For such repos, finalizeCapturedSession wrongly falls through to the rename logic and renames a stable ID to the session-derived ID, breaking the new stable-public-ID behavior.
Use isPendingAgentId (the canonical predicate used throughout agent-registry.ts) instead of a raw substring test. This requires adding it to the import from ./agent-types.js and replacing !updated.agent_id.includes("-pending-") with !isPendingAgentId(updated.agent_id).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/agent-engine.ts around line 2337:
The substring check `updated.agent_id.includes("-pending-")` false-positives when the repo name itself contains `-pending-` (e.g. repo `foo-pending-bar` produces a golem name like `foo-pending-barClaude-<uuid>`). For such repos, `finalizeCapturedSession` wrongly falls through to the rename logic and renames a stable ID to the session-derived ID, breaking the new stable-public-ID behavior.
Use `isPendingAgentId` (the canonical predicate used throughout `agent-registry.ts`) instead of a raw substring test. This requires adding it to the import from `./agent-types.js` and replacing `!updated.agent_id.includes("-pending-")` with `!isPendingAgentId(updated.agent_id)`.
| ...(workspace ? { workspace } : {}), | ||
| focus: args.focus, | ||
| }); | ||
| if (args.cwd) { |
There was a problem hiding this comment.
🟠 High src/server.ts:9790
If client.send or client.sendKey fails after client.newSplit succeeds in the type: "terminal" branch, the exception propagates to the outer catch which returns err(e) without any surface identity. The caller receives only an error and has no way to identify or clean up the orphaned terminal pane (and potentially a newly created new:<name> workspace). The created handle is scoped inside the if (args.type === "terminal") block and is invisible to the outer catch.
Wrap the cwd initialization (client.send + client.sendKey) in its own try/catch so that when it fails, the response still includes surface_id and workspace_id (e.g. with an added cwd_error field), allowing the caller to address the already-created surface.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 9790:
If `client.send` or `client.sendKey` fails after `client.newSplit` succeeds in the `type: "terminal"` branch, the exception propagates to the outer `catch` which returns `err(e)` without any surface identity. The caller receives only an error and has no way to identify or clean up the orphaned terminal pane (and potentially a newly created `new:<name>` workspace). The `created` handle is scoped inside the `if (args.type === "terminal")` block and is invisible to the outer catch.
Wrap the `cwd` initialization (`client.send` + `client.sendKey`) in its own try/catch so that when it fails, the response still includes `surface_id` and `workspace_id` (e.g. with an added `cwd_error` field), allowing the caller to address the already-created surface.
There was a problem hiding this comment.
🟡 Medium src/agent-engine.ts:5003
When spawnParams.authority or spawnParams.placement is explicitly supplied by the caller, the value is persisted without checking that it agrees with the resolved role. A direct engine caller can pass role: "worker", authority: "lead", placement: "left" and produce a durable record where the SpawnSpec axes contradict the role used for surface placement — downstream consumers see a worker-placed surface that claims lead authority on the left column.
Since the comment above declares role authoritative, authority and placement should always be derived from role rather than accepting arbitrary caller overrides.
- authority:
- spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker"),
+ authority: role === "orchestrator" ? "lead" : "worker",
function: spawnParams.function ?? "implementor",
- placement:
- spawnParams.placement ?? (role === "orchestrator" ? "left" : "right"),
+ placement: role === "orchestrator" ? "left" : "right",🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/agent-engine.ts around lines 5003-5007:
When `spawnParams.authority` or `spawnParams.placement` is explicitly supplied by the caller, the value is persisted without checking that it agrees with the resolved `role`. A direct engine caller can pass `role: "worker", authority: "lead", placement: "left"` and produce a durable record where the SpawnSpec axes contradict the role used for surface placement — downstream consumers see a worker-placed surface that claims lead authority on the left column.
Since the comment above declares role authoritative, `authority` and `placement` should always be derived from `role` rather than accepting arbitrary caller overrides.
Keep Gemini and Kiro discovery compatible with fresh installs, and reject invalid spawn role axes before any surface mutation. Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_14448727-27e1-4e3a-a7f7-a38374403834) |
| } catch (error) { | ||
| if (isAgentRoleInferenceError(error)) { | ||
| return inferAgentRole({ cli: input.cli }); | ||
| if (input.cli === "gemini" || input.cli === "kiro") { | ||
| return "worker"; | ||
| } | ||
| } | ||
| throw error; | ||
| } |
There was a problem hiding this comment.
🟡 Medium src/agent-registry.ts:313
When inferAgentRole throws an AgentRoleInferenceError and the CLI is gemini or kiro, roleFromSeatOrLauncher returns "worker" instead of rethrowing. This silently assigns worker to a recovered surface whose actual role could not be determined from its seat_role, launcher name, or title, so repairCandidateForSurface persists it into the wrong lane. Other unresolved CLIs fail closed by rethrowing; Gemini and Kiro fail open with a guessed role.
} catch (error) {
if (isAgentRoleInferenceError(error)) {
- if (input.cli === "gemini" || input.cli === "kiro") {
- return "worker";
- }
}
throw error;
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/agent-registry.ts around lines 313-320:
When `inferAgentRole` throws an `AgentRoleInferenceError` and the CLI is `gemini` or `kiro`, `roleFromSeatOrLauncher` returns `"worker"` instead of rethrowing. This silently assigns `worker` to a recovered surface whose actual role could not be determined from its `seat_role`, launcher name, or title, so `repairCandidateForSurface` persists it into the wrong lane. Other unresolved CLIs fail closed by rethrowing; Gemini and Kiro fail open with a guessed role.
| `Invalid placement=${JSON.stringify(input.placement)}; expected left or right`, | ||
| ); | ||
| } | ||
| const legacyRaw = |
There was a problem hiding this comment.
🟡 Medium src/server.ts:545
normalizeSpawnAxes silently picks role over placement when both carry conflicting legacy authority aliases, silently flipping the spawned topology. For example, role: "worker", placement: "orchestrator" returns a worker/right spawn, but before this change placement took precedence and spawned an orchestrator/left. Instead of silently changing the meaning of a contradictory request, it should reject it. The same silent override happens with explicit authority: role: "orchestrator", authority: "worker" is accepted and returned as a worker/right spawn because input.authority wins at line 566, even though the orchestrator alias denotes lead authority. Consider rejecting these contradictory combinations rather than silently picking one side.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 545:
`normalizeSpawnAxes` silently picks `role` over `placement` when both carry conflicting legacy authority aliases, silently flipping the spawned topology. For example, `role: "worker", placement: "orchestrator"` returns a worker/right spawn, but before this change `placement` took precedence and spawned an orchestrator/left. Instead of silently changing the meaning of a contradictory request, it should reject it. The same silent override happens with explicit `authority`: `role: "orchestrator", authority: "worker"` is accepted and returned as a worker/right spawn because `input.authority` wins at line 566, even though the `orchestrator` alias denotes lead authority. Consider rejecting these contradictory combinations rather than silently picking one side.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e291e32ac
ℹ️ 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".
| agent.repo, | ||
| identity.session_id, | ||
| ); | ||
| if (!updated.agent_id.includes("-pending-")) { |
There was a problem hiding this comment.
When the repository name itself contains -pending- (for example, api-pending-migrations), the newly generated stable UUID-based ID also contains this substring, so session capture incorrectly treats it as a legacy provisional ID and renames it to a session-derived ID. For spawns that return before capture, the public ID then works only through an in-memory alias and is lost across restart, preventing later resume by the originally returned ID; use the existing full legacy-pending pattern rather than a substring check.
AGENTS.md reference: AGENTS.md:L29-L34
Useful? React with 👍 / 👎.
| const authority = | ||
| input.authority ?? | ||
| (legacy?.role === "orchestrator" | ||
| ? "lead" |
There was a problem hiding this comment.
When a caller supplies a compatibility alias together with a conflicting new axis, such as role:"orchestrator", authority:"worker", the explicit authority silently wins and the spawn succeeds as a right-side worker. Because orchestrator is advertised as a supported compatibility role, silently reversing its placement makes pane routing depend on which spelling the caller happened to combine; validate that legacy role/placement aliases agree with explicit authority instead.
AGENTS.md reference: AGENTS.md:L20-L25
Useful? React with 👍 / 👎.
Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_eb816d7b-431c-49bc-9cef-1f995a8d22f2) |
| args.placement ?? args.role, | ||
| selectedRoleField, | ||
| ); | ||
| if (args.type === "terminal") { |
There was a problem hiding this comment.
🟡 Medium src/server.ts:9784
When spawn_agent is called with type="terminal" and a prompt or boot_prompt_path, the terminal branch returns ok: true without reading, validating, or delivering either field. A caller can pass a nonexistent boot_prompt_path (which is not preflight-checked on this path) and still receive a success response, even though no prompt was ever submitted to the terminal. Consider rejecting prompt and boot_prompt_path for terminal spawns alongside the existing role/authority/placement/worktree guard.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 9784:
When `spawn_agent` is called with `type="terminal"` and a `prompt` or `boot_prompt_path`, the terminal branch returns `ok: true` without reading, validating, or delivering either field. A caller can pass a nonexistent `boot_prompt_path` (which is not preflight-checked on this path) and still receive a success response, even though no prompt was ever submitted to the terminal. Consider rejecting `prompt` and `boot_prompt_path` for terminal spawns alongside the existing `role`/`authority`/`placement`/`worktree` guard.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 641fe3bef6
ℹ️ 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".
| if (explicitRole) return explicitRole; | ||
| throw new AgentRoleInferenceError({ role: agent.role, cli: agent.cli }); |
There was a problem hiding this comment.
For an upgraded roleless Claude record that reaches done, this change makes inferRecordRoleOrNull() return null, but assessHarvestability() only excludes role === "orchestrator"; the unresolved record therefore enters the worker-only artifact checks and can become closeable, causing the sweep to emit a worker completion notification. Require role === "worker" before evaluating worker harvestability so unknown roles remain quarantined rather than being silently reclassified.
AGENTS.md reference: AGENTS.md:L11-L16
Useful? React with 👍 / 👎.
Preserve SpawnSpec contracts alongside delivery, watch, and Codex model changes. Co-Authored-By: cmuxlayerCodex running gpt-5.6-sol <noreply@anthropic.com>
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_804cdbcf-6a17-4958-b23c-f10d34d04733) |
| new Set<string>(), | ||
| { role: "worker" }, | ||
| ); | ||
| const created = |
There was a problem hiding this comment.
🟡 Medium src/server.ts:9953
When spawn_agent creates a terminal (type: "terminal") and chooseAgentSpawnPlacement returns kind: "surface", the code calls client.newSurface and never calls focusSurface, so focus: true is silently ignored. The split branch passes focus: args.focus to client.newSplit, but the surface branch has no equivalent — the newly created tab is never focused even when the caller explicitly requests it. Consider calling client.focusSurface(created.surface, { workspace: created.workspace ?? workspace }) after newSurface when args.focus is true, matching the split branch's behavior.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/server.ts around line 9953:
When `spawn_agent` creates a terminal (`type: "terminal"`) and `chooseAgentSpawnPlacement` returns `kind: "surface"`, the code calls `client.newSurface` and never calls `focusSurface`, so `focus: true` is silently ignored. The `split` branch passes `focus: args.focus` to `client.newSplit`, but the `surface` branch has no equivalent — the newly created tab is never focused even when the caller explicitly requests it. Consider calling `client.focusSurface(created.surface, { workspace: created.workspace ?? workspace })` after `newSurface` when `args.focus` is true, matching the `split` branch's behavior.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 894a826918
ℹ️ 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".
| await client.send( | ||
| created.surface, | ||
| `cd -- ${shellQuote(args.cwd)}`, | ||
| { workspace: created.workspace ?? workspace }, | ||
| ); |
There was a problem hiding this comment.
When cwd is nonexistent, inaccessible, or uses an unexpanded form such as ~/repo, these calls only submit cd and Return; they never inspect the shell command's exit status. The handler subsequently returns ok:true and echoes the requested cwd, even though the terminal remains in its original directory, so callers receive incorrect routing state. Validate the directory before creation or verify the terminal's working directory after the command before reporting success.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/server.ts (1)
10356-10362: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueRemove the unreachable
topologyRolefallback.
versionis fixed at1and defaults to1, so both response objects always usenormalizedRole.function. Remove the unusedtopologyRolecomputation at lines 10356-10362. The topology-role values in the deprecated spawn tools are intentional legacy contracts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server.ts` around lines 10356 - 10362, Remove the unused topologyRole computation near the response construction, including the currentAgent role fallback and inferAgentRole call. Keep both response objects using normalizedRole.function, and do not alter the intentional topology-role values in the deprecated spawn tools.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/agent-engine.ts`:
- Around line 5444-5448: Update the spawn parameter resolution in the engine so
the default placement is derived from the resolved authority rather than
independently from role. Reuse a shared lead/worker placement mapping, ensuring
an explicit placement remains honored while authority "lead" defaults to "left"
and other authority values default to "right", preserving the invariant enforced
by normalizeSpawnAxes.
In `@src/layout-policy.ts`:
- Around line 313-318: Update the crash-recovery flow that calls inferRecordRole
so persisted records without an explicit role are handled before
respawn_attempts is incremented. Skip or quarantine these roleless records, or
apply an explicit recovery fallback, while preserving normal recovery and retry
accounting for records whose role can be inferred.
In `@src/server.ts`:
- Around line 590-624: Update the legacy-role normalization and authority
validation in the function containing legacyRaw, legacy, and authority so an
explicit input.authority that contradicts legacy.role is rejected. In
particular, throw for legacy.role "worker" with authority "lead" and legacy.role
"orchestrator" with authority "worker", matching the existing placement-conflict
handling and preventing contradictory aliases from producing an inconsistent
role/placement.
- Around line 9933-9964: The terminal spawn flow in src/server.ts lines
9933-9964 must reuse the vetted placement helpers: resolve the workspace with
resolvePlacementWorkspace, focus the target before creation, capture and restore
post-creation focus, and apply args.focus via focusCreatedSurface for
newSurface. In src/server.ts lines 9965-9974, call waitForLaunchShellReady for
the created surface before sending the cd command.
In `@src/spawn-response.ts`:
- Around line 16-20: Update the ESSENTIAL_FIELDS allowlist in spawn-response.ts
to include the "function" axis so lean SpawnSpec responses retain it for default
non-verbose tool calls, and add a test covering this response contract.
---
Outside diff comments:
In `@src/server.ts`:
- Around line 10356-10362: Remove the unused topologyRole computation near the
response construction, including the currentAgent role fallback and
inferAgentRole call. Keep both response objects using normalizedRole.function,
and do not alter the intentional topology-role values in the deprecated spawn
tools.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 51d0e655-b696-40aa-925b-181ab3f78e08
📒 Files selected for processing (13)
src/agent-engine.tssrc/agent-registry.tssrc/agent-types.tssrc/layout-policy.tssrc/server.tssrc/spawn-response.tstests/agent-engine.test.tstests/agent-registry.test.tstests/agent-types.test.tstests/layout-policy.test.tstests/server-agent-tools.test.tstests/server.test.tstests/sidebar-sync.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Macroscope - Correctness Check
🧰 Additional context used
🧠 Learnings (33)
📓 Common learnings
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-08-02T16:01:45.986Z
Learning: PR `#345` preserves created resource identities in `src/server.ts` spawn-related tool failure responses. The follow-up structural prevention work is tracked in GitHub issue `#348`.
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-09T13:21:30.476Z
Learning: Applies to <AGENTS.md> : If spawning fails after cmuxlayer creates a worktree, remove both that worktree and its newly created branch; never roll back a reused worktree.
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T18:04:24.095Z
Learning: In the cmuxlayer project (src/layout-policy.ts), for `chooseAgentSpawnPlacement`, orchestrator/lead agents should tab into the leftmost lead column (leftPane) unless that pane qualifies as a worker dock via `isWorkerDockPane()` (requires: orchestratorCount=0, icCount=0, workerCount>0, workerCount>nonRoleCount — i.e. workers are strict majority with no leads/ICs). A stale registry role such as ic or worker on a Claude-LEAD tab, or a non-agent shell tab in the left lead pane, is contamination and must NOT force a new left split. Worker placement (docking) remains owned by the rightmost worker pane logic using the same `isWorkerDockPane` predicate.
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-03-16T22:37:27.796Z
Learning: In the cmuxlayer project (src/agent-registry.ts), orphan reparenting is NOT part of V1. When a parent agent crashes, children intentionally keep their parent_agent_id pointing to the dead parent (orphan survival). Reparenting children to root (setting parent_agent_id to null) is a V2 design feature that will be introduced in a dedicated future PR with its own tests. Do not flag missing reparenting logic in agent-registry.ts until the V2 reparenting PR lands.
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-07-14T17:32:22.637Z
Learning: In cmuxlayer's `src/agent-engine.ts`, `runCloseForensicsBestEffort` treats both `tab_close` and `workspace_teardown` close-forensics event origins as terminal operator intent for a matching managed surface, persisting that intent before absence reconciliation can treat it as a recoverable crash (as of commit cd1ac43). Previously only `tab_close` was treated this way. Genuine PTY-death recovery (respawn with attempt limits) is a separate code path and remains unaffected by this origin allowlist.
📚 Learning: 2026-07-18T01:46:21.272Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 338
File: tests/self-registration.test.ts:775-935
Timestamp: 2026-07-18T01:46:21.272Z
Learning: In the self-registration feature, `tests/self-registration.test.ts` intentionally contains fully mocked `AgentEngine` composition/boot-capture cases. These feature-level acceptance tests cover the contract between `src/self-registration.ts` and `src/agent-engine.ts`; do not require moving them to `tests/agent-engine.test.ts` solely to mirror source layout.
Applied to files:
tests/agent-types.test.tstests/layout-policy.test.tstests/agent-registry.test.tstests/sidebar-sync.test.tstests/server.test.tssrc/agent-types.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📚 Learning: 2026-07-04T23:37:37.595Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 220
File: tests/agent-engine.test.ts:2977-2977
Timestamp: 2026-07-04T23:37:37.595Z
Learning: In tests/agent-engine.test.ts for the cmuxlayer project, the general guideline of using 1-second timeouts for `waitFor` in agent-engine tests does not apply to positive/ready-resolution test cases that depend on consecutive-match accumulation across multiple poll/sweep ticks (e.g., "codex-pending-ready", "gemini-identity-screen-ready"). In `AgentEngine.waitFor`, the elapsed time is checked against timeoutMs before the next evidence poll, so a 1s budget can cause a false timeout for these multi-poll cases. These specific tests intentionally use longer timeouts (e.g., 1500ms/2500ms) and this is verified/expected behavior, not a violation to flag.
Applied to files:
tests/agent-types.test.tstests/sidebar-sync.test.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.test.{ts,tsx} : Agents must have comprehensive unit tests covering success and failure paths
Applied to files:
tests/agent-types.test.tstests/agent-registry.test.tstests/server.test.tssrc/agent-types.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Use the Agent interface/base class for creating new agents
Applied to files:
tests/agent-types.test.tssrc/agent-registry.tstests/agent-registry.test.tssrc/agent-types.tstests/agent-engine.test.tssrc/agent-engine.tssrc/server.ts
📚 Learning: 2026-03-16T22:37:27.455Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-03-16T22:37:27.455Z
Learning: In the cmuxlayer project (src/agent-engine.ts / src/agent-types.ts), the inconsistency between `buildLaunchCommand` (throws on `/` in repo names for shell arg safety) and `generateAgentId` (sanitizes `/` to `-` for key safety) is intentional and tracked for follow-up. Do not flag this mismatch as a bug. Both approaches are valid for their respective contexts.
Applied to files:
tests/agent-types.test.tssrc/agent-registry.tstests/layout-policy.test.tssrc/agent-types.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Agent implementations must be written in TypeScript
Applied to files:
tests/agent-types.test.tssrc/agent-types.tssrc/server.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Use type definitions for agent inputs, outputs, and configuration
Applied to files:
tests/agent-types.test.tssrc/agent-registry.tstests/agent-registry.test.tssrc/agent-types.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Document agent purpose and usage in agent implementation files
Applied to files:
tests/agent-types.test.tssrc/agent-registry.tstests/agent-registry.test.tssrc/agent-types.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-03-15T10:46:40.958Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/sidebar-sync.test.ts:18-77
Timestamp: 2026-03-15T10:46:40.958Z
Learning: In the cmuxlayer project, each test file (e.g., tests/sidebar-sync.test.ts, tests/quality-tracking.test.ts, tests/agent-hierarchy.test.ts) is intentionally self-contained. All mock setup helpers (makeMockClient, makeSurface, makeRecord) are defined locally within each test file rather than in shared fixtures. This is a deliberate design choice so that when a test fails, all context is in one file. Shared fixtures are avoided to prevent coupling between test suites. Minor drift in mock fields across files (e.g., listStatus present in one file but not another) is acceptable — it only matters when a test explicitly calls that method. Do not flag duplicated test helpers or suggest extracting them into shared fixture modules.
Applied to files:
tests/agent-types.test.tstests/sidebar-sync.test.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📚 Learning: 2026-03-15T10:42:35.917Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/quality-tracking.test.ts:171-200
Timestamp: 2026-03-15T10:42:35.917Z
Learning: In tests/quality-tracking.test.ts for the cmuxlayer project, ensure that at or above 80% context quality degradation, behavior depends on depth: depth-0 agents receive a /compact command; depth > 0 agents are killed and logged (kill + log). Respawn of non-root agents is out of scope for v1. Treat the design doc quality tracking section as the authoritative source for this behavior, and align test expectations accordingly.
Applied to files:
tests/agent-types.test.tstests/layout-policy.test.tstests/agent-registry.test.tstests/sidebar-sync.test.tstests/server.test.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📚 Learning: 2026-06-05T17:19:12.114Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T17:19:12.114Z
Learning: In the cmuxlayer project (src/server.ts / spawn lifecycle), readiness timeouts during agent launch are non-terminal for lifecycle state. A `BootPromptTimeoutError` should NOT transition the agent to `error` — the agent stays in `booting` with no `error` set. A timeout can mean the CLI chrome changed or the PTY is still healthy but not yet matched; transitioning to error ("poisoning the registry") would block `send_to_agent` and inbox wake. Only actual boot-prompt delivery failures (non-timeout) are terminal, because partial delivery can leave the receiver in an unreliable state.
Applied to files:
src/agent-registry.tstests/sidebar-sync.test.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-06-05T17:16:47.571Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T17:16:47.571Z
Learning: In cmuxlayer layout policy (src/layout-policy.ts or equivalent), terminal surface titles with repoGolem launcher labels are an intentional role fallback when the lifecycle registry or role overrides do not classify a surface. Browser surfaces must NOT be classified via launcher-label title matching. For B1 (role=orchestrator), the correct placement is to tab into the leftmost non-worker lead pane, even when stale IC records or non-role tabs are present there. The layout invariant is: leads as tabs in the left column, workers as tabs in the right column.
Applied to files:
src/agent-registry.tstests/layout-policy.test.tssrc/layout-policy.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-06-05T18:04:24.095Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T18:04:24.095Z
Learning: In the cmuxlayer project (src/layout-policy.ts), for `chooseAgentSpawnPlacement`, orchestrator/lead agents should tab into the leftmost lead column (leftPane) unless that pane qualifies as a worker dock via `isWorkerDockPane()` (requires: orchestratorCount=0, icCount=0, workerCount>0, workerCount>nonRoleCount — i.e. workers are strict majority with no leads/ICs). A stale registry role such as ic or worker on a Claude-LEAD tab, or a non-agent shell tab in the left lead pane, is contamination and must NOT force a new left split. Worker placement (docking) remains owned by the rightmost worker pane logic using the same `isWorkerDockPane` predicate.
Applied to files:
src/agent-registry.tstests/layout-policy.test.tstests/server.test.tssrc/agent-types.tssrc/layout-policy.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-07-14T17:32:22.637Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-07-14T17:32:22.637Z
Learning: In cmuxlayer's `src/agent-engine.ts`, `runCloseForensicsBestEffort` treats both `tab_close` and `workspace_teardown` close-forensics event origins as terminal operator intent for a matching managed surface, persisting that intent before absence reconciliation can treat it as a recoverable crash (as of commit cd1ac43). Previously only `tab_close` was treated this way. Genuine PTY-death recovery (respawn with attempt limits) is a separate code path and remains unaffected by this origin allowlist.
Applied to files:
src/agent-registry.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-04-01T16:08:15.301Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-04-01T16:08:15.301Z
Learning: In the cmuxlayer project (src/agent-engine.ts), the switch statement in `buildLaunchCommand` has no `default` case by design. `CliType` is a compile-time exhaustive union (`'claude' | 'codex' | 'gemini' | 'kiro' | 'cursor'`); TypeScript enforces that all cases are covered, so adding a new CLI to the union without a corresponding case is a compile error. Do not flag the missing default case.
Applied to files:
src/agent-registry.tstests/layout-policy.test.tssrc/layout-policy.tssrc/server.ts
📚 Learning: 2026-07-14T17:22:29.038Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-07-14T17:22:29.038Z
Learning: In cmuxlayer's `src/server.ts` `close_surface` tool handler, the UUID-less legacy-record fallback match (`record.surface_id === args.surface` for records without `surface_uuid`) must only be applied when `observedSurfaceUuid === undefined` (i.e., the live topology genuinely cannot resolve a UUID for the closed ref). This mirrors the invariant in `resolveAgentIoRoute` (`src/agent-engine.ts`), which only permits UUID-less ref-based terminal I/O when a complete fresh topology proves zero UUID coverage. Without this guard, a recycled mutable ref could be mistakenly attributed to a stale legacy record even when a live UUID is observed for that ref (belonging to a different, current owner).
Applied to files:
src/agent-registry.tstests/agent-registry.test.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-03-16T22:37:27.796Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-03-16T22:37:27.796Z
Learning: In the cmuxlayer project (src/agent-registry.ts), orphan reparenting is NOT part of V1. When a parent agent crashes, children intentionally keep their parent_agent_id pointing to the dead parent (orphan survival). Reparenting children to root (setting parent_agent_id to null) is a V2 design feature that will be introduced in a dedicated future PR with its own tests. Do not flag missing reparenting logic in agent-registry.ts until the V2 reparenting PR lands.
Applied to files:
src/agent-registry.tstests/agent-registry.test.tssrc/agent-types.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-08-02T16:01:45.986Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-08-02T16:01:45.986Z
Learning: PR `#345` preserves created resource identities in `src/server.ts` spawn-related tool failure responses. The follow-up structural prevention work is tracked in GitHub issue `#348`.
Applied to files:
src/spawn-response.tstests/sidebar-sync.test.tstests/server.test.tssrc/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-06-05T17:26:08.862Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T17:26:08.862Z
Learning: In the cmuxlayer project, `wait_for(done)` must NOT trust registry state alone. It must require terminal output evidence — either a persisted `task_done_detected_at` timestamp or a current parser-confirmed completion (`parseScreen` reporting `status === "done"`) — because registry state can be stale, manually mutated, or poisoned by prior launch/readiness failures (e.g. `BootPromptTimeoutError`). Completion signals must be accepted only from the current screen tail / chrome-adjacent area so echoed prompt instructions like `R2_WORKER_DONE` do not mark active work as done.
Applied to files:
tests/sidebar-sync.test.tstests/agent-engine.test.tstests/server-agent-tools.test.ts
📚 Learning: 2026-03-15T10:42:36.027Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/sidebar-sync.test.ts:79-279
Timestamp: 2026-03-15T10:42:36.027Z
Learning: In the cmuxlayer project, tests/sidebar-sync.test.ts should cover only the implemented channels: set-status, set-progress, and log. The rename-workspace and report_meta_block channels are intentionally deferred (per phase5-v2-cmux-sidebar-research.md) and must not be considered as missing test coverage. Do not flag or require tests for these two channels in this file.
Applied to files:
tests/sidebar-sync.test.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Use logging for agent actions and state transitions
Applied to files:
src/agent-types.tssrc/server.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx,json,yaml,yml} : Agent configuration should be externalized and not hardcoded
Applied to files:
src/agent-types.tssrc/server.ts
📚 Learning: 2026-03-15T10:42:41.158Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 1
File: tests/quality-tracking.test.ts:171-200
Timestamp: 2026-03-15T10:42:41.158Z
Learning: In the cmuxlayer project (src/agent-engine.ts), quality degradation at ≥80% context behaves differently by depth: depth-0 agents receive a /compact command; depth>0 agents are killed and the event is logged (kill+log). Respawn of non-root agents is intentionally out of scope for v1. The design doc quality tracking section is the authoritative source for this behavior.
Applied to files:
src/agent-types.tssrc/layout-policy.ts
📚 Learning: 2026-07-11T13:51:37.614Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 285
File: src/control-health.ts:183-208
Timestamp: 2026-07-11T13:51:37.614Z
Learning: In the `cmuxlayer` repository, `src/monitor-registry.ts` internals (e.g., `readMonitorRegistry`, `queryMonitorRegistryForGates`) are considered out of scope for modification in PR `#285`; its public API only accepts registry file paths, not pre-parsed snapshots. A true single-snapshot read-validate-count fix for `collectSelfHealHealth` in `src/control-health.ts` (avoiding a TOCTOU risk from reopening the file twice) requires adding a new public snapshot parser/query API to `monitor-registry.ts`. This is tracked in issue `#286` (EtanHey/cmuxlayer) with concrete acceptance criteria; until then, the mitigation is size-bounded reads and fail-safe (unavailable) handling of malformed/invalid registries, relying on the registry writer's atomic-rename contract.
Applied to files:
src/layout-policy.tstests/agent-engine.test.ts
📚 Learning: 2026-08-09T13:21:30.476Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-09T13:21:30.476Z
Learning: Applies to <AGENTS.md> : If spawning fails after cmuxlayer creates a worktree, remove both that worktree and its newly created branch; never roll back a reused worktree.
Applied to files:
src/layout-policy.tstests/agent-engine.test.tssrc/agent-engine.tstests/server-agent-tools.test.tssrc/server.ts
📚 Learning: 2026-04-01T20:31:10.910Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: site/CLAUDE.md:0-0
Timestamp: 2026-04-01T20:31:10.910Z
Learning: Applies to site/**/*agent*.{ts,tsx} : Agents must implement error handling for all external service calls
Applied to files:
tests/agent-engine.test.tssrc/server.ts
📚 Learning: 2026-04-01T16:08:15.301Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-04-01T16:08:15.301Z
Learning: In the cmuxlayer project (src/agent-engine.ts), `buildLaunchCommand` explicitly rejects `.` and `..` as repo names (throws "Invalid repo name") to prevent path traversal where `cd ~/Gits/..` would escape to the parent directory. This guard was added in commit fe41149.
Applied to files:
src/agent-engine.tssrc/server.ts
📚 Learning: 2026-06-05T17:53:58.548Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-06-05T17:53:58.548Z
Learning: In the cmuxlayer project (src/harness-session.ts), `parseCodexSessionMeta` parses JSONL line-by-line looking for a `session_meta` entry with `payload.id`. Both the `catch` block (malformed JSON line) and the case where `session_meta` is found but `payload.id` is null/missing should `continue` to the next line, NOT `return null`. Returning null early would stop scanning on any partial write or missing-id entry.
Applied to files:
src/agent-engine.ts
📚 Learning: 2026-08-09T13:21:30.476Z
Learnt from: CR
Repo: EtanHey/cmuxlayer PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-08-09T13:21:30.476Z
Learning: Applies to <AGENTS.md> : For managed worktree spawns, treat `repo` as selecting the repoGolem registration and naming the worker; do not infer a repository path such as `~/Gits/<repo>`.
Applied to files:
src/agent-engine.tssrc/server.ts
📚 Learning: 2026-08-03T12:44:07.210Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 351
File: src/model-policy.ts:253-273
Timestamp: 2026-08-03T12:44:07.210Z
Learning: In `src/model-policy.ts`, repoGolem requires `REPOGOLEM_ALLOW_MODEL=1` for the Claude `opus` and `haiku` model aliases. The Claude `sonnet` alias is available without this environment variable through repoGolem’s `-S`/`--sonnet` launcher path. Therefore, the ungated accepted Claude model set is `claude-opus-5[1m], sonnet`.
Applied to files:
tests/server-agent-tools.test.ts
📚 Learning: 2026-08-02T15:25:29.749Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 345
File: src/server.ts:8413-8414
Timestamp: 2026-08-02T15:25:29.749Z
Learning: In `src/server.ts`, `boot_prompt_timeout_ms` is an intentional cross-phase override for `spawn_agent` and `new_worktree_split`. When supplied, it controls initial shell readiness, agent launch readiness, post-update relaunch readiness, and boot-prompt readiness. When omitted, the phases retain independent defaults: 10 seconds for shell readiness, 15 seconds for agent launch readiness, and 60 seconds for boot-prompt readiness.
Applied to files:
src/server.ts
📚 Learning: 2026-04-01T16:08:15.301Z
Learnt from: EtanHey
Repo: EtanHey/cmuxlayer PR: 0
File: :0-0
Timestamp: 2026-04-01T16:08:15.301Z
Learning: In the cmuxlayer project (src/agent-engine.ts), `buildLaunchCommand` intentionally does NOT use Zod for input validation. The function is internal (called only from `spawnAgent`), and upstream Zod schema validation already occurs in server.ts around lines 884-886. Adding Zod at this layer is considered redundant. The regex + explicit `.`/`..` path-traversal rejection is the sufficient sanitization boundary.
Applied to files:
src/server.ts
🪛 OpenGrep (1.26.0)
src/server.ts
[WARNING] 9779-9780: Sequelize.literal() with dynamic input can lead to SQL injection. Use parameterized queries or model methods instead.
(coderabbit.sql-injection.sequelize-literal)
🔇 Additional comments (15)
src/agent-engine.ts (3)
56-58: LGTM!Also applies to: 228-230, 243-245, 5594-5594
2509-2511: LGTM!
5332-5337: LGTM!src/agent-types.ts (1)
5-5: LGTM!Also applies to: 20-22, 64-67, 429-429
tests/agent-types.test.ts (1)
117-121: LGTM!tests/layout-policy.test.ts (1)
8-8: LGTM!Also applies to: 290-310, 349-355
src/server.ts (3)
95-97: LGTM!Also applies to: 525-532
9777-9791: LGTM!Also applies to: 9807-9812, 9856-9869
9983-10001: LGTM!Also applies to: 10070-10080, 10118-10120
tests/server-agent-tools.test.ts (1)
547-936: LGTM!Also applies to: 960-966, 1203-1204, 2171-2171, 2725-2729, 2998-2998, 3007-3011, 4250-4250, 5257-5258, 5338-5339, 11880-11880
src/agent-registry.ts (1)
315-317: LGTM!Also applies to: 2301-2302
tests/agent-engine.test.ts (1)
156-156: LGTM!Also applies to: 409-409, 468-471, 637-640, 4637-4637, 4657-4680, 4773-4778, 5627-5627
tests/agent-registry.test.ts (1)
46-46: LGTM!Also applies to: 1666-1697
tests/server.test.ts (1)
435-435: LGTM!Also applies to: 467-474
tests/sidebar-sync.test.ts (1)
154-154: LGTM!Also applies to: 965-965
| authority: | ||
| spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker"), | ||
| function: spawnParams.function ?? "implementor", | ||
| placement: | ||
| spawnParams.placement ?? (role === "orchestrator" ? "left" : "right"), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Derive placement from the resolved authority.
authority and placement are computed from role independently. A direct engine caller can pass authority: "lead" without a placement, and the record then persists authority: "lead" with placement: "right". That contradicts the lead=left / worker=right invariant that normalizeSpawnAxes enforces in src/server.ts. Only the MCP tool validates the combination today, so the engine can persist contradictory axes.
♻️ Proposed fix to keep the persisted axes consistent
+ const authority: AgentAuthority =
+ spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker");Place the constant next to the role resolution (near line 5335), then apply:
- authority:
- spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker"),
+ authority,
function: spawnParams.function ?? "implementor",
- placement:
- spawnParams.placement ?? (role === "orchestrator" ? "left" : "right"),
+ placement:
+ spawnParams.placement ?? (authority === "lead" ? "left" : "right"),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| authority: | |
| spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker"), | |
| function: spawnParams.function ?? "implementor", | |
| placement: | |
| spawnParams.placement ?? (role === "orchestrator" ? "left" : "right"), | |
| authority, | |
| function: spawnParams.function ?? "implementor", | |
| placement: | |
| spawnParams.placement ?? (authority === "lead" ? "left" : "right"), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/agent-engine.ts` around lines 5444 - 5448, Update the spawn parameter
resolution in the engine so the default placement is derived from the resolved
authority rather than independently from role. Reuse a shared lead/worker
placement mapping, ensuring an explicit placement remains honored while
authority "lead" defaults to "left" and other authority values default to
"right", preserving the invariant enforced by normalizeSpawnAxes.
| export function inferRecordRole( | ||
| agent: Pick<AgentRecord, "role" | "cli" | "repo">, | ||
| ): AgentRole { | ||
| return ( | ||
| normalizeExplicitRole(agent.role) ?? | ||
| inferAgentRole({ | ||
| cli: agent.cli, | ||
| launcherName: launcherNameForCli(agent.repo, agent.cli), | ||
| }) | ||
| ); | ||
| const explicitRole = normalizeExplicitRole(agent.role); | ||
| if (explicitRole) return explicitRole; | ||
| throw new AgentRoleInferenceError({ role: agent.role, cli: agent.cli }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find inferRecordRole call sites and show surrounding error handling.
set -euo pipefail
rg -nP --type=ts -C 8 '\binferRecordRole\s*\(' src tests
rg -nP --type=ts -C 4 '\bisAgentRoleInferenceError\s*\(' srcRepository: EtanHey/cmuxlayer
Length of output: 6747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- crash-recovery implementation ---'
sed -n '3230,3375p' src/agent-engine.ts
printf '%s\n' '--- createAgentSurface error handling ---'
sed -n '2025,2105p' src/agent-engine.ts
printf '%s\n' '--- role inference definitions and tests ---'
sed -n '210,360p' src/layout-policy.ts
rg -n -C 5 'inferRecordRole|respawn_attempts|Crash recovery failed|crash recovery' tests src/agent-engine.tsRepository: EtanHey/cmuxlayer
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- remainder of recoverCrashedAgents ---'
sed -n '3360,3450p' src/agent-engine.ts
printf '%s\n' '--- recovery failure helper ---'
sed -n '3050,3150p' src/agent-engine.ts
printf '%s\n' '--- relevant role-policy tests ---'
sed -n '270,320p' tests/layout-policy.test.ts
printf '%s\n' '--- recovery-related tests mentioning roleless records ---'
rg -n -i -C 6 'roleless|role.*undefined|inferRecordRole|legacy.*recover|recover.*legacy|crash.recover.*role' tests srcRepository: EtanHey/cmuxlayer
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '6170,6220p' src/agent-engine.ts
rg -n -C 8 'function isCrashRecoveryEligible|const isCrashRecoveryEligible|isCrashRecoveryEligible' src testsRepository: EtanHey/cmuxlayer
Length of output: 6569
Handle roleless records before incrementing respawn_attempts.
inferRecordRole(agent) throws for roleless persisted records by design. The recovery catch persists Crash recovery failed: ... after respawn_attempts was incremented, so recovery fails and consumes retry budget without creating a surface. Skip or quarantine roleless records without incrementing the counter, or define an explicit recovery fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/layout-policy.ts` around lines 313 - 318, Update the crash-recovery flow
that calls inferRecordRole so persisted records without an explicit role are
handled before respawn_attempts is incremented. Skip or quarantine these
roleless records, or apply an explicit recovery fallback, while preserving
normal recovery and retry accounting for records whose role can be inferred.
| const legacyRaw = | ||
| raw === "orchestrator" || raw === "worker" || raw === "ic" | ||
| ? raw | ||
| : input.placement === "orchestrator" || | ||
| input.placement === "worker" || | ||
| input.placement === "ic" | ||
| ? input.placement | ||
| : undefined; | ||
| const legacy = | ||
| legacyRaw !== undefined | ||
| ? normalizeToolAgentRole( | ||
| legacyRaw, | ||
| legacyRaw === input.role ? "role" : "placement", | ||
| ) | ||
| : null; | ||
| const jobFunction: AgentFunction = | ||
| raw === "reviewer" || raw === "gatherer" || raw === "implementor" | ||
| ? raw | ||
| : "implementor"; | ||
| const defaultAuthority: AgentAuthority = "worker"; | ||
| const authority = | ||
| input.authority ?? | ||
| (legacy?.role === "orchestrator" | ||
| ? "lead" | ||
| : legacy?.role === "worker" | ||
| ? "worker" | ||
| : defaultAuthority); | ||
| if ( | ||
| (jobFunction === "reviewer" || jobFunction === "gatherer") && | ||
| authority !== "worker" | ||
| ) { | ||
| throw new Error( | ||
| `${jobFunction} is a worker function and cannot claim lead authority`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Reject a legacy role alias that contradicts explicit authority.
legacy and input.authority can disagree. With role: "worker" and authority: "lead", legacy.role is "worker" but the explicit authority wins, so the function returns role: "orchestrator" and placement: "left". The caller asked for a worker and receives a lead. The same function already rejects the analogous placement conflict at lines 631-635, so the handling is asymmetric.
🐛 Proposed fix to fail closed on a contradictory legacy alias
const authority =
input.authority ??
(legacy?.role === "orchestrator"
? "lead"
: legacy?.role === "worker"
? "worker"
: defaultAuthority);
+ if (
+ input.authority &&
+ legacy?.role &&
+ ((legacy.role === "orchestrator" && input.authority !== "lead") ||
+ (legacy.role === "worker" && input.authority !== "worker"))
+ ) {
+ throw new Error(
+ `Legacy role=${JSON.stringify(raw)} conflicts with authority=${input.authority}; pass a job function instead`,
+ );
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const legacyRaw = | |
| raw === "orchestrator" || raw === "worker" || raw === "ic" | |
| ? raw | |
| : input.placement === "orchestrator" || | |
| input.placement === "worker" || | |
| input.placement === "ic" | |
| ? input.placement | |
| : undefined; | |
| const legacy = | |
| legacyRaw !== undefined | |
| ? normalizeToolAgentRole( | |
| legacyRaw, | |
| legacyRaw === input.role ? "role" : "placement", | |
| ) | |
| : null; | |
| const jobFunction: AgentFunction = | |
| raw === "reviewer" || raw === "gatherer" || raw === "implementor" | |
| ? raw | |
| : "implementor"; | |
| const defaultAuthority: AgentAuthority = "worker"; | |
| const authority = | |
| input.authority ?? | |
| (legacy?.role === "orchestrator" | |
| ? "lead" | |
| : legacy?.role === "worker" | |
| ? "worker" | |
| : defaultAuthority); | |
| if ( | |
| (jobFunction === "reviewer" || jobFunction === "gatherer") && | |
| authority !== "worker" | |
| ) { | |
| throw new Error( | |
| `${jobFunction} is a worker function and cannot claim lead authority`, | |
| ); | |
| } | |
| const legacyRaw = | |
| raw === "orchestrator" || raw === "worker" || raw === "ic" | |
| ? raw | |
| : input.placement === "orchestrator" || | |
| input.placement === "worker" || | |
| input.placement === "ic" | |
| ? input.placement | |
| : undefined; | |
| const legacy = | |
| legacyRaw !== undefined | |
| ? normalizeToolAgentRole( | |
| legacyRaw, | |
| legacyRaw === input.role ? "role" : "placement", | |
| ) | |
| : null; | |
| const jobFunction: AgentFunction = | |
| raw === "reviewer" || raw === "gatherer" || raw === "implementor" | |
| ? raw | |
| : "implementor"; | |
| const defaultAuthority: AgentAuthority = "worker"; | |
| const authority = | |
| input.authority ?? | |
| (legacy?.role === "orchestrator" | |
| ? "lead" | |
| : legacy?.role === "worker" | |
| ? "worker" | |
| : defaultAuthority); | |
| if ( | |
| input.authority && | |
| legacy?.role && | |
| ((legacy.role === "orchestrator" && input.authority !== "lead") || | |
| (legacy.role === "worker" && input.authority !== "worker")) | |
| ) { | |
| throw new Error( | |
| `Legacy role=${JSON.stringify(raw)} conflicts with authority=${input.authority}; pass a job function instead`, | |
| ); | |
| } | |
| if ( | |
| (jobFunction === "reviewer" || jobFunction === "gatherer") && | |
| authority !== "worker" | |
| ) { | |
| throw new Error( | |
| `${jobFunction} is a worker function and cannot claim lead authority`, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server.ts` around lines 590 - 624, Update the legacy-role normalization
and authority validation in the function containing legacyRaw, legacy, and
authority so an explicit input.authority that contradicts legacy.role is
rejected. In particular, throw for legacy.role "worker" with authority "lead"
and legacy.role "orchestrator" with authority "worker", matching the existing
placement-conflict handling and preventing contradictory aliases from producing
an inconsistent role/placement.
| const requestedWorkspace = args.workspace; | ||
| const callerWorkspace = await currentSafetyCallerWorkspace(); | ||
| const createsWorkspace = requestedWorkspace?.startsWith("new:"); | ||
| await assertWorkspaceMutationAllowed( | ||
| "spawn_agent", | ||
| createsWorkspace | ||
| ? callerWorkspace | ||
| : requestedWorkspace ?? callerWorkspace, | ||
| ); | ||
| const workspace = createsWorkspace | ||
| ? (await client.createWorkspace(requestedWorkspace!.slice(4))) | ||
| .workspace | ||
| : requestedWorkspace ?? callerWorkspace; | ||
| const panes = await client.listPanes({ workspace }); | ||
| const placement = chooseAgentSpawnPlacement( | ||
| panes.panes, | ||
| [], | ||
| new Set<string>(), | ||
| { role: "worker" }, | ||
| ); | ||
| const created = | ||
| placement.kind === "surface" | ||
| ? await client.newSurface({ | ||
| pane: placement.pane, | ||
| ...(workspace ? { workspace } : {}), | ||
| type: "terminal", | ||
| }) | ||
| : await client.newSplit(placement.direction, { | ||
| ...(workspace ? { workspace } : {}), | ||
| ...(placement.pane ? { pane: placement.pane } : {}), | ||
| focus: args.focus, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
The terminal spawn branch reimplements surface creation instead of reusing the vetted spawn helpers. Agent spawns resolve the workspace with resolvePlacementWorkspace, focus the target before creating the pane, restore the origin focus, and wait for shell readiness before any command. The new terminal branch skips all four steps, so a terminal can land in the focused workspace, strand focus, ignore focus: true, and lose the cd command.
src/server.ts#L9933-L9964: resolve the workspace throughresolvePlacementWorkspace, wrap creation withfocusTargetBeforeSplit/capturePostCreationFocus/restoreFocusAfterRender, and honorargs.focuson thenewSurfacebranch throughfocusCreatedSurface.src/server.ts#L9965-L9974: callwaitForLaunchShellReadyfor the created surface before sendingcd -- <cwd>.
📍 Affects 1 file
src/server.ts#L9933-L9964(this comment)src/server.ts#L9965-L9974
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/server.ts` around lines 9933 - 9964, The terminal spawn flow in
src/server.ts lines 9933-9964 must reuse the vetted placement helpers: resolve
the workspace with resolvePlacementWorkspace, focus the target before creation,
capture and restore post-creation focus, and apply args.focus via
focusCreatedSurface for newSurface. In src/server.ts lines 9965-9974, call
waitForLaunchShellReady for the created surface before sending the cd command.
| "authority", | ||
| "placement", | ||
| "parent_agent_id", | ||
| "version", | ||
| "type", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Return the SpawnSpec function axis.
Lean responses drop function because ESSENTIAL_FIELDS does not include it. This breaks the SpawnSpec response contract for default, non-verbose tool calls. Add "function" to this allowlist and add a lean-response test.
Proposed fix
"role",
"authority",
+ "function",
"placement",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "authority", | |
| "placement", | |
| "parent_agent_id", | |
| "version", | |
| "type", | |
| "authority", | |
| "function", | |
| "placement", | |
| "parent_agent_id", | |
| "version", | |
| "type", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/spawn-response.ts` around lines 16 - 20, Update the ESSENTIAL_FIELDS
allowlist in spawn-response.ts to include the "function" axis so lean SpawnSpec
responses retain it for default non-verbose tool calls, and add a test covering
this response contract.
1 participant
Summary
new:<name>workspacesVerification
npm run typechecknpm run buildgit diff --checkLive MCP/cmux verification is intentionally left to the lead-routed review lane required by the Phase 3 brief.
Refs #378 and #383.
— cmuxlayerCodex (worker) · codex/gpt-5.6-sol
Note
High Risk
Changes core spawn API semantics, agent ID stability, and role inference across engine, MCP tools, and registry—breaking for callers that relied on CLI-derived roles or pending ID renames.
Overview
Introduces SpawnSpec v1 for
spawn_agent: spawn-time authority (lead/worker), job function (implementor/reviewer/gatherer), and placement (left/right), validated bynormalizeSpawnAxes(e.g. reviewer/gatherer cannot be lead; placement must match authority). Legacyorchestrator/workeronrole/placementare still accepted and mapped. Agent identity no longer derives from CLI—engine spawns default to worker when role is omitted; Claude v1 spawns without an explicit job role fail withROLE_REQUIREDbefore any surface mutation.Stable public
agent_ids replace*-pending-*provisional IDs: pre-session IDs use a UUID suffix and stay fixed after transcript capture (no rename/alias). Responses addparent_agent_id,version,type, and the new axes; lean spawn payloads include the same fields.Adds
type: terminalspawns (optionalcwd, parent-workspace inheritance,new:<name>workspaces) that return routing fields only—no agent lifecycle fields. Registry/layout stop inferring persisted roles from CLI alone; unresolved records surface as null, with gemini/kiro repair defaulting to worker for list merge.Purging and duplicate-lane checks now skip agents with unresolved roles (not only orchestrators).
Reviewed by Cursor Bugbot for commit 894a826. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add SpawnSpec v1 axes (authority, function, placement) to the
spawn_agenttoolspawn_agentwithauthority(lead/worker),function(implementor/reviewer/gatherer), andplacement(left/right) axes, validated and normalized via a newnormalizeSpawnAxesfunction in server.ts.spawn_agentwithtype: terminalcreates a terminal surface, setscwd, and returns no agent fields.authority,function, andplacementinAgentRecord; spawn responses includeparent_agent_id,version, andtypeas essential fields.-pending-<ts>agent IDs with stable short-UUID-based IDs for agents spawned without a known session.inferAgentRoleandinferRecordRolethrow without explicit or launcher/title evidence, and duplicate-lane detection usesinferRecordRoleOrNull.Macroscope summarized 894a826.
Summary by CodeRabbit
New Features
Bug Fixes