Skip to content

feat: add versioned SpawnSpec axes #394

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EtanHey merged 4 commits into from
Aug 12, 2026
Merged

feat: add versioned SpawnSpec axes #394

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e2c24c0
feat: add versioned SpawnSpec axes
EtanHey Aug 11, 2026
7e291e3
fix: harden SpawnSpec role recovery
EtanHey Aug 12, 2026
641fe3b
fix: dock terminal spawns in worker column
EtanHey Aug 12, 2026
894a826
Merge origin/main into fix/stability-p3-spawnspec
EtanHey Aug 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 20 additions & 13 deletions src/agent-engine.ts
View file
Open in desktop

@macroscopeapp macroscopeapp Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,9 @@ import {
MAX_RESPAWN_ATTEMPTS,
type AgentRoute,
type AgentRecord,
type AgentAuthority,
type AgentFunction,
type AgentPlacement,
type AgentRole,
type AgentState,
type CliType,
Expand Down Expand Up @@ -222,6 +225,9 @@ export interface SpawnAgentParams {
worktree_branch?: string;
parent_agent_id?: string;
role?: AgentRole;
authority?: AgentAuthority;
function?: AgentFunction;
placement?: AgentPlacement;
auto_archive_on_done?: boolean;
max_cost_per_agent?: number;
crash_recover?: boolean;
Expand All @@ -236,6 +242,7 @@ export interface SpawnAgentParams {

export interface SpawnAgentResult {
agent_id: string;
parent_agent_id: string | null;
surface_id: string;
workspace_id?: string;
state: AgentState;
Expand Down Expand Up @@ -2499,6 +2506,9 @@ export class AgentEngine {
agent.repo,
identity.session_id,
);
if (!updated.agent_id.includes("-pending-")) {

@macroscopeapp macroscopeapp Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 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)`.

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Detect legacy pending IDs with the full format

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 👍 / 👎.

return updated;
}
if (updated.agent_id === finalAgentId) {
return updated;
}
Expand Down Expand Up @@ -5319,21 +5329,12 @@ export class AgentEngine {
parentAgent = parent;
}

// Job role is authoritative. When no role was declared, spawn context is
// stronger evidence than the CLI: a worker's child is worker work even if
// the selected harness is Claude. CLI/launcher inference is the final
// compatibility fallback only (#378).
// Job role is authoritative. The versioned tool rejects missing agent
// axes; direct legacy engine callers default to worker without consulting
// the selected harness.
const role = spawnParams.role !== undefined
? inferAgentRole({ role: spawnParams.role })
: parentAgent && inferRecordRoleOrNull(parentAgent) === "worker"
? "worker"
: inferAgentRole({
cli: spawnParams.cli,
launcherName: launcherNameForCli(
spawnParams.repo,
spawnParams.cli,
),
});
: "worker";

this.spawnGuard.check(spawnParams.workspace);

Expand Down Expand Up @@ -5440,6 +5441,11 @@ export class AgentEngine {
parent_agent_id: parentAgentId,
spawn_depth: spawnDepth,
role,
authority:
spawnParams.authority ?? (role === "orchestrator" ? "lead" : "worker"),
function: spawnParams.function ?? "implementor",
placement:
spawnParams.placement ?? (role === "orchestrator" ? "left" : "right"),
Comment on lines +5444 to +5448

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ 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.

Suggested change
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.

auto_archive_on_done: spawnParams.auto_archive_on_done,
deletion_intent: false,
quality: "unknown",
Expand Down Expand Up @@ -5585,6 +5591,7 @@ export class AgentEngine {
this.schedulePostSpawnLivenessAssertion(agentId);
return {
agent_id: agentId,
parent_agent_id: parentAgentId,
surface_id: surface.surface,
workspace_id: surface.workspace,
state: "booting",
Expand Down
7 changes: 5 additions & 2 deletions src/agent-registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,9 @@ function roleFromSeatOrLauncher(input: {
});
} catch (error) {
if (isAgentRoleInferenceError(error)) {
return inferAgentRole({ cli: input.cli });
if (input.cli === "gemini" || input.cli === "kiro") {
return "worker";
}
}
throw error;
}
Comment on lines 313 to 320

@macroscopeapp macroscopeapp Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Expand Down Expand Up @@ -2296,7 +2298,8 @@ export class AgentRegistry {
if (shouldRetainCrashRecoveryError(agent)) {
continue;
}
if (inferRecordRoleOrNull(agent) === "orchestrator") {
const role = inferRecordRoleOrNull(agent);
if (role === null || role === "orchestrator") {
continue;
}
if (this.matchingLiveSurface(agent, surfaces)) {
Expand Down
12 changes: 9 additions & 3 deletions src/agent-types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* Agent lifecycle types — flat, SQLite-importable schema.
* Every field is a primitive (string | number | null).
*/
import { randomUUID } from "node:crypto";

export type AgentState =
| "creating"
Expand All @@ -16,6 +17,9 @@ export type CliType = "claude" | "codex" | "gemini" | "kiro" | "cursor";

export type AgentQuality = "unknown" | "verified" | "suspect" | "degraded";
export type AgentRole = "orchestrator" | "worker";
export type AgentAuthority = "lead" | "worker";
export type AgentFunction = "implementor" | "reviewer" | "gatherer";
export type AgentPlacement = "left" | "right";
export type SurfaceProvenance = "cmuxlayer_spawn" | "unknown";
export type SeatIdentityStatus = "ok" | "mismatch" | "unknown";

Expand Down Expand Up @@ -57,6 +61,10 @@ export interface AgentRecord {
parent_agent_id: string | null;
spawn_depth: number;
role?: AgentRole;
/** SpawnSpec v1 axes. `role` remains a persisted compatibility field. */
authority?: AgentAuthority;
function?: AgentFunction;
placement?: AgentPlacement;
auto_archive_on_done?: boolean;
task_done_candidate_at?: string | null;
task_done_detected_at?: string | null;
Expand Down Expand Up @@ -418,7 +426,5 @@ export function generateAgentId(
if (sessionId) {
return `${golemName}-${sessionIdPrefix(sessionId)}`;
}
const ts = Math.floor(Date.now() / 1000);
const rand = Math.random().toString(36).slice(2, 6);
return `${golemName}-pending-${ts}-${rand}`;
return `${golemName}-${randomUUID().slice(0, SESSION_ID_PREFIX_LENGTH)}`;
}
30 changes: 4 additions & 26 deletions src/layout-policy.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,20 +257,6 @@ function roleFromLauncherLabel(label: string | undefined): AgentRole | null {
return null;
}

function roleFromCli(cli: string | undefined): AgentRole | null {
switch (cli) {
case "claude":
return "orchestrator";
case "codex":
case "cursor":
case "gemini":
case "kiro":
return "worker";
default:
return null;
}
}

function normalizeExplicitRole(
role: AgentRole | "ic" | undefined,
): AgentRole | undefined {
Expand All @@ -286,8 +272,7 @@ export function canInferAgentRole(input: {
return Boolean(
input.role ||
roleFromLauncherLabel(input.launcherName) ||
roleFromLauncherLabel(input.title) ||
roleFromCli(input.cli),
roleFromLauncherLabel(input.title),
);
}

Expand All @@ -305,9 +290,6 @@ export function inferAgentRole(input: {
roleFromLauncherLabel(input.title);
if (launcherRole) return launcherRole;

const cliRole = roleFromCli(input.cli);
if (cliRole) return cliRole;

throw new AgentRoleInferenceError(input);
}

Expand All @@ -331,13 +313,9 @@ export function launcherNameForCli(repo: string, cli: CliType): string {
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 });
Comment on lines +316 to +318

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 11, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Preserve recovery attempts for roleless persisted agents

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 👍 / 👎.

Comment on lines +317 to +318

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Exclude unresolved roles from worker harvestability

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 👍 / 👎.

Comment on lines 313 to +318

@coderabbitai coderabbitai Bot Aug 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 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*\(' src

Repository: 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.ts

Repository: 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 src

Repository: 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 tests

Repository: 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.

}

export function inferRecordRoleOrNull(
Expand Down
Loading
Loading