[brennanb2025]

	Files	Added	Deleted	Net
Test	7	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​444	$\color{#cf222e}{\Huge{\mathbf{−}}}$​22	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​422
Prod	18	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​262	$\color{#cf222e}{\Huge{\mathbf{−}}}$​164	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​98

ELI5

When Orca starts an agent as a native chat, it makes up an id for the chat session: the conversation's identity. This PR lets the caller that starts the launch choose that id instead, so the launcher knows which conversation it created before the reply arrives, and a retry of the same launch refers to the same conversation. It also fixes a mismatch the move would otherwise expose: a launch that becomes a terminal ignored the model the user picked, while the same launch as a chat honoured it.

No shipped caller sends either field yet. Mobile is the only client of agent.launch today, and it sends neither sessionId nor sessionOptions. The one change a user can reach: a chat that mobile starts now gets a session id shaped like every other (claude_…, underscores) instead of a bare hyphenated one; nothing reads the shape back, and ids already saved are untouched.

What Changed

1. agent.launch accepts an optional sessionId for the chat session a structured launch creates.

• Absent: exactly today's behaviour. The host mints the id.
• Present: the structured create uses it, and the outcome's sessionId confirms it. It reaches the create on both routes: an existing workspace, and a chat created along with a new workspace.
• A launch that settles as a terminal creates no session and ignores it.
• Validated at the wire with the shared SessionId schema, and refused unless it is named for its agent (claude_… for a claude launch), followed by one token of letters, digits and underscores. This is checked on both agent.launch and agent.launchReplay. Only the part after the agent name is checked for the one-token shape, so an agent whose own name has a hyphen (mimo-code_…) is not refused at the wire; like any agent without a chat, its launch settles as a terminal and ignores the id.

2. A taken id is refused with its own code, agent_launch_session_already_exists. The host already refuses a new session under an existing id (agent_session_conflict, from the null-fence check in agent-session-reservation-admission.ts:185). This PR adds no second guard. It gives that refusal a stable code when the caller named the id, and makes sure it is not treated as a reason to fall back:

• No fallback to a terminal. agent_session_conflict is already outside the set of refusals that may fall back (agent-session-definitive-refusal.ts), and the new error is not a structured-refusal type at all. A caller that named a taken session has a bug to see, not an agent in a different surface.
• The ledger records a failure, for an existing workspace, the same way a live reserved pane and a worktree name collision do. Otherwise the row would stay claimed and every retry would get agent_session_operation_unknown. On create-worktree the workspace already exists by the time the chat is refused, so the row stays unknown, as it does for the pane refusal.
• The code passes through to the caller on agent.launch, and on agent.launchReplay for an existing workspace. On create-worktree, agent.launchReplay answers agent_session_operation_unknown, as it does for a live reserved pane, because the workspace was already created.
• A conflict on an id the host minted is left exactly as it was.

3. sessionId is in the replay fingerprint when present. A retry that minted a different session is a different request: replaying the first answer would hand the caller a conversation it did not mint. Absent keys are dropped by the canonical form, so every existing digest is unchanged.

4. A terminal launch now starts with the model, effort and mode the launch carried. sessionOptions were read by the structured create but dropped on the terminal route, so the same launch got a different model depending on which surface the host chose. Both terminal routes now pass them to the runtime as launch preferences, which the startup plan already ranks above configured args:

• existing workspace (and the structured-to-terminal fallback) → createTerminal({ launchPreferences })
• create-worktree → startupLaunchPreferences, a field the worktree create already had and orchestration already uses

5. One converter. toAgentLaunchPreferences moves from src/renderer/src/runtime/agent-session-create-operation.ts to src/shared/agent-launch-preferences.ts. The three renderer callers are repointed; no second copy is left behind.

6. Two small structural changes the above needed:

• The terminal routes' inputs (args, cwd, launch source, pane, options) are built once, in terminalLaunchInputs, instead of being repeated at the two call sites.
• The factory contracts the executor is handed (AgentLaunchSurfaceFactory, AgentLaunchWorkspaceFactory, the structured refusal error) move to agent-launch-surface-factories.ts. They are what each caller implements, not part of the sequencing the executor runs, and moving them keeps the executor under its line limit without trimming anything.

Why

Why the caller mints the chat session id. The session id is the conversation's identity, and a launcher that mints it knows which conversation it started without waiting for the reply. A retry carries the same id: under an operationId the host answers it from the ledger, and without one the host refuses a second session under that id instead of starting another chat. The desktop's direct chat launch already works this way: it mints the id and sends it on agentSession.create (launch-structured-agent-session.ts:94). This brings agent.launch in line and replaces its bare randomUUID().

This field is not how a caller places the chat's tab. A chat tab's id is currently derived from its session id, but that derivation is being removed in follow-up work: tabs get their own id, reserved by the caller through paneKey for both terminals and chats, and point at the session through entityId. Nothing in this PR keys placement on the session id.

Why the id's shape is enforced. The shared minting helper (createStructuredAgentSessionId) documents two properties of every session id: it is prefixed with its agent, so an id names its lane on sight, and it is underscore-only, so it stays one token inside tab ids and log keys. Nothing reads the prefix back today (searched; the one match is the writer itself), but session ids are persisted, so the shape is decided once. Letting callers mint codex_… for a claude launch, or a hyphenated id, would make that documented invariant false for exactly the ids that crossed the wire. The predicate (isStructuredAgentSessionIdFor) lives next to the writer.

The host's own fallback now uses the same helper. When no id is sent, agent.launch used to mint a bare hyphenated randomUUID(), the only structured-session id in the app without the shape. It now calls createStructuredAgentSessionId, so every id a launch creates, caller-minted or not, has one shape. Ids already persisted are untouched; the shared SessionId schema still accepts them.

Why the model belongs on the terminal route now. Once the desktop launcher calls agent.launch, it sends the user's remembered model in sessionOptions the way it builds its own startup plan today. If the host dropped it whenever the launch became a terminal, moving the desktop launcher onto agent.launch would silently lose the model on every terminal launch. The host building the preference, rather than the desktop spelling a --model flag into args, keeps the knowledge of how each CLI spells a model in one place: the shared startup plan.

Linked Issue

Part of the agent.launch series (#22291, #22116, #22108). No separate issue.

Visual Proof

N/A. No caller sends either field and no renderer behaviour changes; the three renderer edits only change where the converter is imported from. The checks that matter are that a launch sending nothing is untouched, that a reserved id reaches the create, and that a picked model reaches the terminal's real launch command. All are pinned below.

Testing

• Automated tests added/updated

pnpm tc clean, oxlint clean on every changed file, and 20,111 tests pass across 1,809 files in the affected suites (src/shared, the agent.launch RPC methods, the executor, the renderer runtime and lib directories, the pty-connection directory, and the orca-runtime.test.ts aggregator).

Two lineage tests (falls back to cwd lineage when the caller terminal handle is stale and keeps cwd-inferred lineage best-effort when the cwd parent cannot be resolved) failed in that combined local run, and a CI node shard failed the same family once on this PR's first head, all with Worktree created but not found in listing: /tmp/workspaces/…. This is a frequent existing flake, not this PR: this test family (the lineage tests and worktree scan cache: expires scans per repo without coupling sibling repos, which failed on this PR's third head with listWorktrees called 4 times instead of 3) fails in 14 of 37 recent failed PR Checks runs on unrelated branches, among them lead-status-pr-c, claude-init-deadline, chat-file-links-open, browser-capture-owns-paint-hold and structured-send-recovery. A probe confirmed this PR's new runtime tests list worktrees exactly once each and nothing after they return, so they are not a source of the extra call. The same shard passed on this PR's next head with the same test code, and the tests pass alone and in the full orca-runtime.test.ts aggregator on both this branch and main. The likely mechanism is in the test harness: those tests queue one-shot listWorktrees results with mockResolvedValueOnce, and the shared beforeEach re-arms listWorktrees with mockResolvedValue without mockReset, so a result left or stolen by another test shifts their queue. That is worth its own fix and is not part of this PR.

Against the real runtime (terminal-creation-and-readiness-part-12.spec.ts, a real OrcaRuntimeService):

• an agent terminal created with launchPreferences: { model: 'opus' } spawns with '--model' 'opus' in its command
• without preferences, the command carries no --model

At the agent.launch boundary (agent-launch-session-reservation.test.ts):

• the structured create uses the caller's session id, and the outcome reports it. The conflict tests stub the structured create and pin this PR's mapping of its refusal; the store's own refusal of a taken id is pre-existing and not re-tested here
• without one, the host mints its own
• the reservation reaches a chat created along with a new workspace
• a taken id is refused with agent_launch_session_already_exists, and no terminal is created in its place
• a conflict on a host-minted id is left as it was
• a launch that settles as a terminal creates no session
• five malformed ids are refused at the wire (too short, a character outside the id alphabet, surrounding space, named for another agent, hyphenated); a well-formed one is accepted
• an id for an agent whose own name has a hyphen (mimo-code_…) is accepted, and ignored on its terminal like any other agent without a chat; a hyphen after the agent name is still refused
• an id named for another agent is refused on agent.launchReplay too
• the host-minted fallback is named for its agent and is one token
• the fingerprint separates two different reservations, and is unchanged without one
• under an operationId, the refusal is recorded as a failed row, a retry is answered without touching the host, and agent.launchReplay answers with the refusal code

Model on the terminal route (agent-launch-terminal-preferences.test.ts):

• an existing-workspace terminal launch passes the picks as launchPreferences; none are added when the launch carried none; the picks survive a downgrade to a terminal; options that are not launch preferences are dropped
• a create-worktree terminal launch passes them as startupLaunchPreferences; none when the launch carried none; none when the new workspace gets a chat instead

Mobile. Making AgentLaunch a refined schema removed .omit from it, and mobile/src/tasks/mobile-agent-launch-architecture.test.ts builds an older host's params with AgentLaunch.omit({ operationId: true }). The unrefined base is now exported as AgentLaunchFields and that test uses it; every receiver still parses the refined AgentLaunch / AgentLaunchReplay. With the old line, 8 of that file's 13 tests fail; with the fix, all 13 pass. Mobile typecheck is clean and the mobile tests-typecheck ratchet passes.

Converter (agent-launch-preferences.test.ts): the existing case moved from the renderer test, plus the empty case.

Ablation, run at the final head. Each production change was removed one at a time, the suites above re-run, then restored:

Removed	Failing tests
sessionId in the agent.launch schema	8
the handler copying it onto the launch intent	5
the executor forwarding it to the structured create	5
the factory using the caller's id instead of minting (restored to randomUUID())	2
the factory turning a conflict on a caller's id into agent_launch_session_already_exists	3
recording that refusal as a failed ledger row	2
passing its code through to the caller	1 (agent.launchReplay answers with it)
sessionId in the fingerprint	1
launchPreferences on the existing-workspace terminal	2
options in the shared terminal inputs (both routes)	3
startupLaunchPreferences on create-worktree	1
the runtime reading launchPreferences into the startup plan (pre-existing)	1, the real-runtime --model test
the wire's id-shape check	3 (the other-agent, hyphenated and replay-method refusals)
checking the one-token alphabet on the whole id instead of only after the agent name	1 (the hyphenated-agent accept test)
the fallback minting through createStructuredAgentSessionId (restored to randomUUID())	1

Every row went red and was restored.

Notes

Wire compatibility. A new optional field on an existing method, and a new error code only that field can trigger. No new stream opcode. Old client → new host: sends nothing, no change. New client → old host: the field is stripped and the host mints its own id; the outcome's sessionId differs from the one the caller minted, so the caller learns from the outcome which conversation was really started, and the launch still creates exactly one chat. The outcome is the reconciliation: a caller must not rely on its own id being honoured before the reply arrives. No capability advertises support because no caller depends on it yet; a caller that needs the id before the reply will need one.

sessionOptions on the terminal route is a behaviour change for any client that sends them. Today that is no client (mobile sends none), and it is the direction the desktop migration needs. A future client that sends picks and lands on a terminal now gets those picks instead of the configured default args.

Performance. One conversion per launch that sends options. No extra IPC, render or hot-path work.

Cross-platform and SSH. No platform-specific code. The model preference goes through the same shared startup plan on every host, and the session id goes through the same structured create on local and paired hosts; SSH hosts do not support structured chat, so the session id never applies there.

Agent skill upstream boundary

• Not applicable.

Checklist

• This PR is small and focused
• I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
• Before/after screenshots or videos attached for UI changes, or N/A with reason
• Self-reviewed for correctness, security, and performance
• Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
• pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

[coderabbitai]

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 94178559-5f7f-4e58-b198-3594f9a84ea7

📥 Commits

Reviewing files that changed from the base of the PR and between 70a32d5 and 0a6a954.

📒 Files selected for processing (2)

• src/main/runtime/rpc/methods/agent-launch-session-reservation.test.ts
• src/shared/structured-agent-session-create.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

The launch RPC now accepts a caller-reserved session ID for structured launches, validates it against the selected agent, includes it in operation fingerprints, and returns a dedicated error when that ID already exists. Terminal launch options now pass supported model, effort, and mode preferences to terminal and worktree creation. Surface factory contracts and preference conversion are defined in shared modules. Tests cover session reservation, preference forwarding, and spawned model arguments.

Merge Risk: ⚪ Minimal · up to 0a6a9

Reserved IDs for agents with hyphenated names now pass validation. No outstanding issue in the supplied evidence prevents merge after normal checks.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 25 functions across 25 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely identifies both primary changes: caller-selected chat session IDs and terminal launches receiving session preferences.
Description check	✅ Passed	The description is detailed and covers the required explanation, changes, rationale, testing, compatibility, platform impact, and checklist items. It links related issues in the Linked Issue section, …

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 2

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

🛠️ Fix failing CI checks 💡

• Commit to this branch
• Create a new PR

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[pullfrog]

ℹ️ No critical issues — a few comment/coverage nits inline.

Reviewed changes

• Caller-reserved chat session id — agent.launch accepts an optional sessionId, threaded to the structured create on both the existing-workspace and create-worktree routes; the outcome reports the session that really exists.
• New conflict code — a taken caller-named id is refused with agent_launch_session_already_exists, deliberately not a definitive structured refusal, so the launch does not fall back to a terminal; the ledger records the failure for an existing workspace.
• Replay fingerprint — sessionId is included when present; absent keys are dropped, so existing digests are unchanged.
• Terminal session picks — both terminal routes now forward sessionOptions as launchPreferences/startupLaunchPreferences, so a terminal landing starts with the picked model/effort and no duplicate flags.
• Shared converter + factory extraction — toAgentLaunchPreferences moves to src/shared/agent-launch-preferences.ts (three renderer callers repointed), and the surface/workspace factory contracts move to agent-launch-surface-factories.ts.

ℹ️ Nitpicks

• src/shared/rpc-contract/agent-launch-params.ts:70 still describes sessionOptions as "Only the seedable string options a structured create accepts; a terminal launch ignores them." This PR makes the second clause false — the terminal routes now read them as model/effort/mode preferences. Worth updating alongside the AgentLaunchIntent doc that already was.
• The new agent-launch-session-reservation.test.ts covers an existing + taken id + operationId collision but not the create-worktree twin; the pane reservation test covers both target kinds. The behavior is correct by construction, but a regression to the targetKind === 'existing' guard in launchFailureWithoutEffectsCode would go unnoticed on the create-worktree lane.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

the host created nothing is only true of the session. On a create-worktree launch the worktree already exists by the time this is raised, and the code still reaches agent.launch callers (the executor rethrows the cause even though the row stays unknown). Consider wording it as "the host created no session" so it is not read as "no effects at all".

[pullfrog]

Caution

The new object-level refinement on AgentLaunch breaks an existing mobile test at runtime. AgentLaunch.omit({ operationId: true }) in mobile/src/tasks/mobile-agent-launch-architecture.test.ts:76 now throws Error: .omit() cannot be used on object schemas containing refinements (Zod 4.5.4). That file is excluded from mobile's test typecheck and from the root typecheck, so only the mobile verify job's pnpm test catches it — and this PR's src/shared/rpc-contract/** change triggers that job. Details inline.

Reviewed changes (since the prior review at 8be3f6e)

• Session id shape enforced at the wire — a caller-minted sessionId must now start with ${agent}_ and be one token, via the new isStructuredAgentSessionIdFor; checked on both agent.launch and agent.launchReplay.
• Host fallback minting — the executor now mints through createStructuredAgentSessionId instead of a bare randomUUID(), so every launch-created id has the same shape.
• AgentLaunch split — a module-private AgentLaunchFields plain object plus a refined AgentLaunch; AgentLaunchReplay is rebuilt from the base object (.required() on a refined object is forbidden in Zod 4).

ℹ️ Nitpicks

• The shape refinement is written out twice (AgentLaunch and AgentLaunchReplay) instead of derived from one helper. Previously AgentLaunchReplay = AgentLaunch.required(...) inherited any future refinement automatically; now a refinement added to AgentLaunch would silently not apply to the replay method. A shared withLaunchRefinements(schema) wrapper would keep them in step.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 3835a653-3961-40a4-93c1-f0e034776465

📥 Commits

Reviewing files that changed from the base of the PR and between a77f87e and 939bb00.

📒 Files selected for processing (23)

• src/main/agent-launch/agent-launch-executor.test.ts
• src/main/agent-launch/agent-launch-executor.ts
• src/main/agent-launch/agent-launch-prompt-delivery.ts
• src/main/agent-launch/agent-launch-surface-factories.ts
• src/main/runtime/orca-runtime-tests/terminal-creation-and-readiness-part-12.spec.ts
• src/main/runtime/rpc/errors.ts
• src/main/runtime/rpc/methods/agent-launch-session-reservation.test.ts
• src/main/runtime/rpc/methods/agent-launch-surfaces.ts
• src/main/runtime/rpc/methods/agent-launch-terminal-preferences.test.ts
• src/main/runtime/rpc/methods/agent-launch-worktree-creation.ts
• src/main/runtime/rpc/methods/agent-launch.ts
• src/renderer/src/components/terminal-pane/pty-connection/pty-input-recovery.ts
• src/renderer/src/lib/launch-agent-web-host-tab.ts
• src/renderer/src/lib/runtime-agent-background-create.ts
• src/renderer/src/runtime/agent-session-create-operation.ts
• src/renderer/src/runtime/agent-session-operation-id.test.ts
• src/shared/agent-launch-intent.ts
• src/shared/agent-launch-operation.ts
• src/shared/agent-launch-preferences.test.ts
• src/shared/agent-launch-preferences.ts
• src/shared/agent-launch-session-already-exists.ts
• src/shared/rpc-contract/agent-launch-params.ts
• src/shared/structured-agent-session-create.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review.

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Only check the session-id shape for agents that can create a structured session.

refuseSessionIdForAnotherAgent runs for every TuiAgent. For agents whose names contain -, such as claude-agent-teams, mimo-code, and qwen-code, isStructuredAgentSessionIdFor can never return true. Its regex /^[A-Za-z0-9_]+$/ rejects the - in the required ${agent}_ prefix. So any sessionId on those launches fails validation.

The field doc says the reservation is "Ignored when the launch settles as a terminal". Those agents always launch in a terminal. A caller that mints a sessionId on every launch, as it does for paneKey, would have those launches refused.

Choose one fix:

• Limit the check to structured providers, so the id stays ignored for other agents.
• Refuse sessionId for non-structured agents with an explicit message, and update the field doc to say so.

Proposed fix

 function refuseSessionIdForAnotherAgent(
   launch: { agent: string; sessionId?: string | undefined },
   ctx: z.RefinementCtx
 ): void {
-  if (launch.sessionId && !isStructuredAgentSessionIdFor(launch.agent, launch.sessionId)) {
+  const structured = launch.agent === 'claude' || launch.agent === 'codex'
+  if (structured && launch.sessionId && !isStructuredAgentSessionIdFor(launch.agent, launch.sessionId)) {

📝 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

	/** A caller-minted session id must be shaped like every id the host mints, so an id still names
	* its lane on sight. */
	function refuseSessionIdForAnotherAgent(
	launch: { agent: string; sessionId?: string | undefined },
	ctx: z.RefinementCtx
	): void {
	if (launch.sessionId && !isStructuredAgentSessionIdFor(launch.agent, launch.sessionId)) {
	ctx.addIssue({
	code: z.ZodIssueCode.custom,
	path: ['sessionId'],
	message: 'Launch session id must be named for its agent'
	})
	}
	}
	/** A caller-minted session id must be shaped like every id the host mints, so an id still names
	* its lane on sight. */
	function refuseSessionIdForAnotherAgent(
	launch: { agent: string; sessionId?: string | undefined },
	ctx: z.RefinementCtx
	): void {
	const structured = launch.agent === 'claude' || launch.agent === 'codex'
	if (structured && launch.sessionId && !isStructuredAgentSessionIdFor(launch.agent, launch.sessionId)) {
	ctx.addIssue({
	code: z.ZodIssueCode.custom,
	path: ['sessionId'],
	message: 'Launch session id must be named for its agent'
	})
	}
	}

[pullfrog]

ℹ️ No new issues in this delta — the earlier blocking concern is resolved.

Reviewed changes

• Mobile fixture unblocked — mobile/src/tasks/mobile-agent-launch-architecture.test.ts now builds its older-host params from the exported un-refined AgentLaunchFields base instead of AgentLaunch, so Zod 4's .omit() no longer throws while the legacy dispatcher is constructed. Verified locally: .omit() succeeds on AgentLaunchFields and still throws on the refined AgentLaunch, while the shape check still rejects a codex_… id for a claude launch on both AgentLaunch and AgentLaunchReplay.
• AgentLaunchFields exported — the un-refined base is re-exported from src/main/runtime/rpc/methods/agent-launch-schemas.ts with a doc comment scoping it to building an older host's shape in tests; every receiver still parses AgentLaunch or AgentLaunchReplay.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in this delta. The reserved-session-id wire check now validates only the token after the agent-name prefix. The still-open wording thread on agent-launch-session-already-exists.ts:4 is untouched by these commits, so this review is intentionally non-approving.

Reviewed changes

• Reserved-id shape check fixed — isStructuredAgentSessionIdFor now strips the ${agent}_ prefix and validates only the remaining token, so a host-shaped id for a hyphenated agent (mimo-code_…) is no longer wrongly refused at the wire, while a hyphen inside the token is still rejected.
• Wire tests added — one asserts a hyphenated-agent id is accepted, one asserts a hyphen after the agent name is still refused.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[brennanb2025]

Review status: ready to merge once shard 1 is re-run

Head: 0a6a9541f0. Four review loops ran; the last two came back clean. A separate architecture review found no reason to change the approach, and a readiness check before and after the review found nothing blocking. Electron QA on a hidden dev build is below.

What changed during review

Commit	Change
105d70f294	Comments no longer describe sessionId as a way to place a tab. It is the conversation's identity, and tab placement stays off the launch request. Also moved a doc comment in the executor back onto createTerminalSurface.
70a32d5e95	The sessionOptions doc on the request schema still said a terminal launch ignores them. It now says a terminal reads the model, effort and mode.
0a6a9541f0	The id-shape check now looks for the <agent>_ prefix and applies the one-token rule only to what follows it. Before, any id for an agent whose own name has a hyphen (for example mimo-code) was refused at the request, while every other agent without a chat just ignores the id on its terminal. That hard refusal would also have been fixed into every host at this version. Removing the fix makes exactly one test fail: the new test that accepts a hyphenated agent's id.

The PR body has been corrected to match: the fingerprint and wire-compatibility wording was still about placement, and the agent.launchReplay passthrough on create-worktree was described wrongly.

Architecture

• Kept: the caller chooses the id. The alternative is having the host derive the session id from the operation id. That was rejected for three reasons:
  • The caller could only know the id before the reply by recomputing the derivation itself, and version skew then yields a silently wrong id.
  • It can't work for launches without an operationId.
  • The desktop's direct agentSession.create path already has the caller choose the id.
• No second guard. A taken id is refused by the existing check inside the store transaction. The PR only gives that refusal its own code when the caller chose the id.
• One place builds the flags. Model, effort and mode become per-CLI flags in the shared startup plan. The one deliberate difference is that unknown model ids are passed through rather than rejected.

Electron QA (hidden dev build of this branch, agent.launch called through the renderer's runtime RPC)

Scenario	Result
Mobile-shaped launch (no sessionId, no sessionOptions)	Chat opens with id claude_dc348233_…. A message round-trips: the reply shown is a provider usage-limit error from the test account, not an app error.
Caller-chosen sessionId	Chat opens and the result reports the same id.
Same sessionId again	Refused with agent_launch_session_already_exists; no new tab or terminal.
codex_… or hyphenated id on a claude launch	Refused at the request (invalid_argument).
Terminal launch with sessionOptions: { model: "opus" }	Process started as claude --dangerously-skip-permissions --model opus.
Terminal launch without sessionOptions	claude --dangerously-skip-permissions, no --model.

What this QA did not cover:

• Launches with an operationId. The desktop renderer can't be the caller for these: an existing check that the caller has an identity refuses it with agent_session_identity_required, and this PR doesn't touch that check. Ledger replay with an operationId is covered by unit tests.
• Mobile emulator QA. Nothing in mobile changes. Mobile's host-side path was exercised with its exact params in the first row above.
• Windows and SSH. No platform-specific code changed, and SSH hosts don't support structured chat.

CI

Everything passes except tests node 24 1/8 (and verify, which depends on it). Its one failure is Sidebar.test.tsx > does not start a full worktree scan while the startup session is hydrating, which fails in repo-scan-failure.ts. That breakage is already on main, this PR doesn't touch the file, and the fix is #22564. Re-run shard 1 after #22564 lands. e2e / ssh docker watcher isolation was still pending when this was written.

Follow-ups (not in this PR)

• Once the desktop launcher moves onto agent.launch, it must read the chat's sessionId from the outcome and not assume its own id was used. An older host silently drops the field and chooses its own id.
• mode is honoured on a terminal but dropped when the launch starts a chat, and fastMode is the reverse. This mismatch already existed; it becomes visible once desktop sends picks.
• On create-worktree, a taken id leaves the new workspace in place without an agent. This is the same leftover already accepted for a live reserved pane, and it can only happen through a caller bug.