[brennanb2025]

	Files	Added	Deleted	Net
Test	45	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​2200	$\color{#cf222e}{\Huge{\mathbf{−}}}$​2076	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​124
Prod	79	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​3268	$\color{#cf222e}{\Huge{\mathbf{−}}}$​1809	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​1459

ELI5

Opening a structured agent chat now gives the user a usable chat view immediately. Provider acquisition continues behind that view, drafts and sends remain durable, and a slow or refused structured launch never silently turns into a legacy terminal.

What Changed

• Fixed the root lifecycle bug where a provider root had exited but an unverified descendant snapshot made eviction fail forever.
• Release only the lease owned by a provider root after first-hand root-exit or processless proof; live and genuinely unproven descendants still fail closed.
• Create a provisional native-chat tab immediately, gate provider reads and mutations until publication, and keep the durable outbox available while acquisition is pending.
• Persist launch identity across renderer reloads, normalize interrupted launches to manual Retry, and tombstone cancelled sessions so late publication cannot resurrect a closed tab.
• Removed the renderer deadline and every structured-launch-to-terminal fallback, including definitive-refusal fallback. Unknown outcomes retain the existing visibilityUnknown/manual Retry path.
• Keep option discovery and prompt-cancel capability negotiation fenced to the published provider session.
• Retain one phase-timed, privacy-safe agentSession.create span with no branch, path, prompt, session content, or sensitive identifier attributes.

Why

#19154 records a Claude root that exited before producing output. Because the descendant snapshot was still unverifiable, eviction treated the dead lease owner like a live descendant and could not release the lease or host session.

The durable lease identifies the provider root PID and start time, so first-hand root death is enough to release that root-owned lease. This does not claim that unknown descendants exited; later cleanup still preserves that uncertainty.

The user-facing fix is architectural: the chat surface is available immediately and acquisition is background work. A timeout or refusal therefore cannot replace the structured route with a different product surface.

Linked Issue

Fixes #19154

Testing

• Targeted launch/lifecycle command: 38 test files passed (38), 344 tests passed (344); every path was supplied as a separate pnpm test argument.
• Post-refactor native-chat smoke: 4 test files passed (4), 26 tests passed (26).
• New behavior ablations went red when their implementation behavior was removed and were restored from SHA-256-verified copies.
• pnpm run check:code-quality:changed: passed with 0 findings across 103 changed files.
• git diff --check: passed; lockfile restored and clean.
• pnpm tc: reached TypeScript but failed on 73 pre-existing editor errors caused by mixed installed prosemirror-model 1.25.4/1.25.11 trees. No reported error references the native-chat changes.
• No full test suite, install, stash, checkout, rebase, or visible-window Electron run was performed.

Review

The branch was merged with current origin/main and pushed as 5d8692db1c. The current head also structurally splits the upstream message-rail windowing coverage to satisfy the repository max-lines rule without a lint suppression and keeps transport gating render-pure. Earlier automated comments about a startup label and 30-second terminal fallback describe superseded behavior; the current design intentionally removes both. Please review the immediate provisional-tab lifecycle, root-owned lease release, and cancellation/publication fencing on this head.

Notes

• fix(native-chat): wait for the runtime capability probe before resolving the creation launch route #19819 is merged and included; it fixes route capability timing, while this PR fixes the provider-root exit/eviction lifecycle and makes structured chat immediately usable during acquisition.
• Folder workspaces and git worktrees use the same structured launch contract.
• Linux, Windows, SSH, relay, paired web, and mobile manual runs were not performed locally; no platform-specific process call, wire field, or persisted schema was added.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

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: 9a2d59b8-f929-4f5a-acbf-668a84f8f0e6

📥 Commits

Reviewing files that changed from the base of the PR and between b4432ec and 361ee00.

📒 Files selected for processing (1)

• src/renderer/src/i18n/locales/en.json

💤 Files with no reviewable changes (1)

• src/renderer/src/i18n/locales/en.json

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

---

📝 Walkthrough

Walkthrough

The change adds agent-session creation tracing with phase timing and span attributes. Claude acquisition and cleanup now use centralized resolution and classified exit errors. Structured renderer launches no longer fall back to legacy terminal surfaces after refusal or deadline outcomes. Launch state moves to a registry, callers use structured settlement results, and pending creation and recovery messages identify the selected agent.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 361ee

A stalled structured launch can remain pending without a usable recovery surface. Add a bounded settlement or equivalent recovery path before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 22.03% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 47 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies the coding requirements in [#19154]. structured-agent-session-claude-root-exit.test.ts verifies that a Claude provider-root exit releases the captured lease, records `deathEvidence:…
Out of Scope Changes check	✅ Passed	The changed Claude cleanup, eviction, acquisition, launch-progress, recovery-message, settlement, observability, and test files support the objectives in [#19154]. The fallback and deadline changes pr…
Title check	✅ Passed	The title clearly identifies the primary fix: releasing native-chat sessions after the provider root exits.
Description check	✅ Passed	The description is detailed, on-topic, and includes the ELI5, changes, rationale, linked issue, testing, review, and notes sections. It omits several template sections, including Visual Proof, AI Disc…

Full details: Docstring Coverage

Explanation

Docstring coverage is 22.03% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 59 functions across 47 files. (1 skipped: 1 unsupported.)

---

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

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: dfc60991-5464-4acb-b58c-36978faf7487

📥 Commits

Reviewing files that changed from the base of the PR and between c6548b9 and 643dcf9.

📒 Files selected for processing (22)

• IMPLEMENTATION-NOTES.md
• src/main/claude/claude-structured-acquisition-launch.ts
• src/main/claude/claude-structured-session-acquisition.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-acquisition-options.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-acquisition.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-adapter.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-attach-flow.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-attach-orchestration.ts
• src/main/observability/agent-session-instrumentation.ts
• src/main/observability/instrumentation.test.ts
• src/renderer/src/lib/launch-work-item-direct-agent-routing.test.ts
• src/renderer/src/lib/launch-work-item-direct-agent-routing.ts
• src/renderer/src/lib/pending-worktree-creation.test.ts
• src/renderer/src/lib/pending-worktree-creation.ts
• src/renderer/src/lib/structured-agent-launch-settlement.test.ts
• src/renderer/src/lib/structured-agent-launch-settlement.ts
• src/renderer/src/lib/structured-agent-session-launch-callers.ts
• src/renderer/src/lib/structured-agent-session-launch-deadline-fallback.test.ts
• src/renderer/src/lib/structured-agent-session-launch-registry.ts
• src/renderer/src/lib/structured-agent-session-launch.ts
• src/renderer/src/lib/worktree-creation-structured-session.test.ts
• src/renderer/src/lib/worktree-creation-structured-session.ts

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

[pullfrog]

Important

The new deadline-then-legacy settlement is only understood by two of its consumers, so a slow launch now takes the wrong path in the full-create, folder-workspace, new-tab, and fork flows.

Reviewed changes

• Phase-timed agentSession.create tracing — new agent-session-instrumentation.ts wraps create-intent attaches once and threads a phase recorder through orchestration → attach flow → acquisition → adapter → Claude acquisition; attributes carry only closed phase names and timings.
• starting-chat pending phase — WorktreeCreationPhase gains starting-chat, worktree creation sets it immediately before launch, and getCreationProgressLabel names the pending agent.
• 30s deadline fallback — settleStructuredAgentLaunch races the launch against an injectable deadline that claims the shared fallback as deadline-then-legacy, keeps cancellation ahead of it, and retires a late structured publication.
• Generalized fallback claim — claimFallback replaces claimDefinitiveRefusalFallback (kept as a deprecated alias); the callers group only transitions from pending/unknown, and the worktree session retires late structured sessions.
• Registry extraction / Claude refactor — launch registry and status hooks move to structured-agent-session-launch-registry.ts; Claude launch resolution is extracted to stay under the line cap.

ℹ️ Nitpicks

• IMPLEMENTATION-NOTES.md is a 132-line agent work log added at the repo root. It reads like a scratch plan/ablation record rather than repo documentation; consider dropping it from the PR (or moving the durable parts into docs/reference/).

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

[pullfrog]

✅ No new issues found.

Reviewed changes

The delta since the prior pullfrog review (643dcf9) is the merge commit 348149f and a test-hygiene follow-up b053abe. It resolves the prior [!IMPORTANT] finding that deadline-then-legacy was only understood by two consumers.

• Centralized legacy-fallback interpretation — new structuredAgentLegacyFallbackFromSettlement normalizes refused-then-legacy and deadline-then-legacy into one result, and full-create, folder-workspace create, new-tab, fork, and direct-routing all consume it instead of branching on the refusal kind alone.
• Universal late retirement — the per-flow onStructuredLate hook is gone; the shared settlement loop calls the new retireStructuredAgentSessionSurface whenever the fallback ran, and registers it before the post-fallback cancellation check, so a structured publication that lands after the fallback is retired in every flow.
• Census ratchet — a new test asserts no production file outside the settlement module names the legacy kinds directly; confirmed by grep (only the settlement module and tests match).
• Cleanup — IMPLEMENTATION-NOTES.md is deleted and post-merge test hygiene restored the hoisted mocks and formatting.

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

[pullfrog]

✅ No new issues found.

Reviewed changes

Commit 04673918 ("Make structured fallback settlement exhaustive") landed after the previous review; it tightens the legacy-fallback API rather than changing behavior.

• Type-narrowing guard — added isStructuredAgentLegacyFallbackSettlement, and overloaded structuredAgentLegacyFallbackFromSettlement so a settlement narrowed by the guard returns a non-null result.
• Exhaustive consumers — launch-work-item-direct-agent-routing.ts and worktree-creation-structured-session.ts narrow first, then consume the fallback without optional chaining, and drop their default / merged failed branches. The remaining switches cover every non-legacy kind, so a future settlement kind becomes a compile error instead of a silent fallthrough.

  ^{｜ 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: dc85e667-0e6f-492d-a464-8a4feb2dc132

📥 Commits

Reviewing files that changed from the base of the PR and between 0467391 and b4432ec.

📒 Files selected for processing (35)

• src/main/claude/claude-structured-session-close.test.ts
• src/main/claude/claude-structured-session-close.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-claude-root-exit.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-eviction.test.ts
• src/main/native-chat/agent-session-wire/structured-agent-session-eviction.ts
• src/renderer/src/components/sidebar/folder-workspace-composer-submit.ts
• src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts
• src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts
• src/renderer/src/hooks/composer-state/full-creation-execution.ts
• src/renderer/src/hooks/composer-state/full-creation-structured-launch.test.ts
• src/renderer/src/hooks/composer-state/full-creation-structured-launch.ts
• src/renderer/src/i18n/locales/en.json
• src/renderer/src/lib/launch-agent-in-new-tab-structured.test.ts
• src/renderer/src/lib/launch-agent-in-new-tab-structured.ts
• src/renderer/src/lib/launch-agent-in-new-tab.ts
• src/renderer/src/lib/launch-agent-structured-chat-guard.test.ts
• src/renderer/src/lib/launch-work-item-direct-agent-routing.test.ts
• src/renderer/src/lib/launch-work-item-direct-agent-routing.ts
• src/renderer/src/lib/launch-work-item-direct.ts
• src/renderer/src/lib/onboarding-folder-agent-launch.ts
• src/renderer/src/lib/structured-agent-launch-no-terminal-fallback.test.ts
• src/renderer/src/lib/structured-agent-launch-settlement-caller-census.test.ts
• src/renderer/src/lib/structured-agent-launch-settlement.test.ts
• src/renderer/src/lib/structured-agent-launch-settlement.ts
• src/renderer/src/lib/structured-agent-session-launch-callers.ts
• src/renderer/src/lib/structured-agent-session-launch-failure-toast.ts
• src/renderer/src/lib/structured-agent-session-launch-refusal-fallback.test.ts
• src/renderer/src/lib/structured-agent-session-launch-registry.ts
• src/renderer/src/lib/structured-agent-session-launch.test.ts
• src/renderer/src/lib/structured-agent-session-launch.ts
• src/renderer/src/lib/worktree-creation-flow-execute.ts
• src/renderer/src/lib/worktree-creation-structured-recovery.ts
• src/renderer/src/lib/worktree-creation-structured-session.test.ts
• src/renderer/src/lib/worktree-creation-structured-session.ts
• src/renderer/src/lib/worktree-creation-structured-unknown-outcome.test.ts

💤 Files with no reviewable changes (5)

• src/renderer/src/components/terminal-pane/terminal-agent-session-fork.ts
• src/renderer/src/hooks/composer-state/full-creation-structured-launch.ts
• src/renderer/src/lib/structured-agent-launch-settlement-caller-census.test.ts
• src/renderer/src/lib/structured-agent-session-launch-refusal-fallback.test.ts
• src/renderer/src/components/terminal-pane/terminal-agent-session-fork.test.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• src/renderer/src/lib/structured-agent-session-launch-registry.ts

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

[brennanb2025]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[pullfrog]

ℹ️ No critical issues — one documentation note inline.

Reviewed changes

Commit b4432ecee2 reworks the PR into a root-cause fix and removes the deadline/terminal-fallback design the earlier reviews covered. That commit is the delta since the prior pullfrog review at 04673918c3; the rest of the branch is unchanged.

• Provider root-exit releases the lease — claude-structured-session-close.ts classifies an unproven published-session close and an unproven acquisition cancel through claudeAcquisitionCleanupError, throwing AgentSessionAcquisitionRootExitObservedError (root: exited, tree: unverifiable) or AgentSessionPreSpawnError (processless) instead of returning false. Only a genuinely unproven exit still returns false.
• Eviction continues on actionable verdicts — structured-agent-session-eviction.ts swallows those two errors and runs the remaining steps (drain → sink teardown → release-lease → forget); every other throw, including …ExitUnprovenError, still aborts so the next close is a real retry.
• Deadline and terminal fallback removed — structured-agent-launch-settlement.ts drops legacyFallback/claimFallback/STRUCTURED_AGENT_LAUNCH_DEADLINE_MS, the caller-group refusal settlement, the late-surface retirement module, and the terminal fallback in every renderer entrypoint; a definitive refusal now settles as failed.
• Agent-named unknown recovery — markStructuredWorktreeLaunchUnconfirmed takes the narrowed provider and the unknown toast interpolates {{value0}}; en.json and its tests were updated.
• Census ratchet — a new test asserts no production renderer file names the fallback kinds or the deadline constant.

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

[brennanb2025]

Independent hidden-CDP QA — structured chat starting visibility

Validated on worktree /Users/brennanbenson/orca/workspaces/orca/native-chat-launch-visibility at 6dae2fb44f (brennanb2025/native-chat-launch-visibility). App identity: Orca: brennanb2025/native-chat-launch-visibility. Hidden launch with ORCA_BACKGROUND_LAUNCH=1 and CDP (no show / focus / OS automation).

The production pending row and creation panel name the selected agent and keep the elapsed counter visible:

Claude — pending row + panel, elapsed 57s

Codex — pending row + panel, elapsed 11s

Renderer console: 0 errors (unrelated Tooltip / React Grab warnings only). No production files changed.

Method note: the starting-chat phase was held through the real store APIs (beginPendingWorktreeCreation with phase: 'starting-chat') so the elapsed counter could be photographed. A live Claude provider spawn was not used, because a healthy launch can finish in ~1.5s.

Local evidence: /tmp/orca-pr20502-qa-evidence/

[brennanb2025]

Review status

PASS WITH RISK — no valuable code fixes required.

• Same-model adversarial review completed; no proven regressions or meaningful in-scope fixes found.
• Final readiness checklist completed with no P0/P1/P2 findings.
• Validation passed: targeted changed-test suite (17 files, 151 tests), additional lifecycle/proof suites (47 + 52 + 66 tests), pnpm tc, pnpm tc:node, pnpm tc:web, changed-code quality (0 findings across 41 files), and git diff --check.
• Hidden Electron/CDP proof on the exact PR worktree confirmed the rendered pending row and creation panel show agent-specific “Starting Claude chat…” / “Starting Codex chat…” text with elapsed time and 0 renderer errors. Screenshots are attached in the QA comment above.
• No mobile-facing RPC/protocol/pairing surfaces were changed.

Residual risk: this session did not exercise a live provider-backed create, real SSH/relay/network-failure paths, Windows/WSL runtime, or the full local suite. GitHub’s static analysis check is red from three existing mobile test warnings unrelated to this diff; the changed-code quality gate is clean.

No commit or push was necessary.

[brennanb2025]

Merge conflicts resolved and pushed in commit 5c4d95146e (merged origin/main at 539d4d1f32).

Resolved conflicts in Claude structured acquisition and acquisition-options tests while preserving the PR's launch lifecycle/instrumentation changes and current-main fast-mode/session-option behavior. Focused tests (25) and pnpm tc pass; merge state is now MERGEABLE (UNSTABLE pending CI).

[greptile-apps]

Too many files changed for review (124 files, 100 file limit).

Bypass the limit by tagging @greptile-apps to review.

[pullfrog]

Important

This delta rebases the structured-chat launch/close lifecycle onto main's surface-based launch API. Two behavioral gaps come with it: a closed published chat can be re-mirrored because only provisional closes get a cancellation tombstone, and a caller coalescing onto a retained failed launch has its prompt silently dropped.

Reviewed changes

Incremental delta 7e7125dcf9..a145a79dab — the dc9c77c8e8 "fix native chat launch visibility" reconciliation (87 files) plus the a145a79dab merge of origin/main (b61a2347b9).

• Adopted main's { surface } launch result everywhere — new-tab, work-item direct, fork, quick launch, source-control recovery, onboarding, worktree creation, full-create, and folder-workspace entrypoints now consume LaunchAgentInNewTabResult.surface and thread beforeSurfaceOpen, so a workspace can reveal before its surface opens.
• Extracted the launch state machine — structured-agent-session-launch-registry.ts plus new structured-agent-session-launch-persistence.ts (localStorage launch records and session-scoped cancellation tombstones) and structured-agent-session-launch-reload.ts.
• Provisional launch — structured-agent-session-provisional-tab.ts and beginStructuredAgentSessionProvisionalLaunch open the chat tab and expose the durable sessionId before host acquisition settles; NativeChatLaunchRetry / NativeChatDeliveryRetry and useNativeChatProvisionalLaunch drive retry and transportEnabled.
• Reworked structured-chat tab close — closeUnifiedTab removes the local row synchronously and delegates host retirement to retireStructuredAgentSessionTab, with tombstone suppression wired into the snapshot/mirror path and silent cancellation during bulk worktree purges.
• Gated provider transport on launch lifecycle — new use-structured-agent-session-transport, …-transport-state, and …-options withhold provider hold/read/mutate and the option surface while a launch is pending, failed, or visibility-unknown.

ℹ️ Nitpicks

• useNativeChatProvisionalLaunch guards retry on worktreeId sourced from useNativeChatFileLinkContext; when that context is null the NativeChatLaunchRetry affordance still renders but the click is a no-op (src/renderer/src/components/native-chat/use-native-chat-provisional-launch.ts:11-16). Passing the worktree the pane overlay already holds would remove the dead end.

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

[pullfrog]

The cancellation tombstone is written only for provisional closes, but this delta also moved local row removal ahead of host retirement (tabs-close-actions.ts removes synchronously; this function fires agentSession.close and session.tabs.close asynchronously). A published session has no tombstone, so a host frame that still contains it — an unrelated worktree update during the close round-trip, or any frame at all when the close RPC rejects — re-mirrors the row through buildMirroredAgentTabs. On main the close awaited the host calls and then removed the row, and a rejection surfaced a toast; now a failed retirement is swallowed as a console warning with no retry, so the resurrected row can persist.

Technical details

# Published structured tabs have no close tombstone

## Affected sites
- `src/renderer/src/runtime/structured-agent-session-tab-retirement.ts:66-70` — tombstone only when `args.provisional`; `retireStructuredAgentSessionTab` swallows rejections (no `onError` from `tabs-close-actions.ts`).
- `src/renderer/src/store/slices/tabs/tabs-close-actions.ts:51-71` — `provisional` is false for a published session (`shouldRetainStructuredAgentSessionLaunchTab` covers only pending/visibility-unknown/failed), and the row is removed later in the same synchronous `set`.
- `src/renderer/src/runtime/web-session-tabs-sync/terminal-surfaces.ts:64-69` and `apply-preparation-browser.ts:177-182` — the mirror/suppression path only consults the tombstone, never `withLocalSessionTabCloseOwner` (that owner is consumed by `session-tab-ipc-bridge.ts` only).
- `src/renderer/src/components/tab-group/workspace-tab-close-commands.ts` — the pre-`dc9c77c` branch awaited `closeStructuredAgentSession`/`session.tabs.close` before `closeUnifiedTab` and toasted via `reportStructuredSessionCloseError`; that ordering was removed.

## Required outcome
- Closing a published structured chat must not re-add its tab from a host frame that predates the host acknowledging the close, and a failed host retirement must be retried or reported rather than silently lost.

## Suggested approach (optional)
- Record the cancellation tombstone for every user-initiated structured tab close (not only provisional), or restore the local row only after the host close resolves; pass an `onError` that reports and/or retries the host close.

[pullfrog]

When an existing state is failed or visibility-unknown, retrying strips prompt for every joining caller, not just a re-stage of the same prompt. A second launch for the same agent:worktreeId identity that carries a different prompt therefore has it neither staged in the outbox nor seeded into the composer, and settleStructuredAgentLaunchPrompt returns undefined — so no promptDeliveryResult is reported and the retained outbox entry delivers the earlier prompt in its place.

Technical details

# Retried launch drops a joining caller's prompt

## Affected sites
- `src/renderer/src/lib/structured-agent-session-launch.ts:181-190` — `text = retrying ? '' : outboxPromptText(joined)` plus `callerOptions = retrying ? joinedWithoutPrompt : joined` applies to any joiner, while the comment only justifies skipping a duplicate re-stage (`failed launches keep their draft/outbox`).
- `src/renderer/src/lib/structured-agent-session-launch-prompt.ts:97` — with no `prompt`, `settleStructuredAgentLaunchPrompt` returns `undefined`, so the caller's `promptDeliveryResult` is absent.
- Impact: `runSourceControlAgentActionStart` treats a missing `promptDeliveryResult` as success and fires `onLaunched`, so irreversible follow-up runs as if the prompt reached the provider.

## Required outcome
- A joiner with a prompt distinct from the retained entry must have it staged/delivered (and report a delivery result); only a re-staged duplicate may be skipped.

## Suggested approach (optional)
- Compare the joining prompt against the retained draft/outbox entry (or carry an explicit `reuseStagedPrompt` flag) and strip the prompt only on a match; stage it normally otherwise.

[brennanb2025]

PR #20502 Electron validation — native structured-chat launch visibility

Scenario

Hidden Electron validation of brennanb2025/native-chat-launch-visibility (PR #20502) with ORCA_BACKGROUND_LAUNCH=1 and CDP on 127.0.0.1:9333 / renderer 127.0.0.1:5176. Isolated profile seeded with completed onboarding and structured-chat settings enabled. Temp git repo /tmp/orca-pr-20502-validation/repo added as the workspace.

Launched a structured Claude chat from the real New tab → Claude menu. Sent a composer message after the ordinary chat/composer appeared. Launched a structured Codex chat from launchAgentInNewTab and inspected the same-turn provisional tab.

No production code was edited. Launch-policy was rebuilt by the electron-vite dev runner (main+preload) before the app started.

Identity

window.api.app.getIdentity():

• name: Orca: brennanb2025/native-chat-launch-visibility
• isDev: true
• devLabel / worktreeName: native-chat-launch-visibility
• devBranch: brennanb2025/native-chat-launch-visibility
• devRepoRoot: /Users/brennanbenson/orca/workspaces/orca/native-chat-launch-visibility

Visible evidence

Claude (New tab menu, immediately after click):

• Tab label Claude Chat, content type agent-session, ptyId: null
• Ordinary empty native chat: “Start a chat with Claude”
• Composer placeholder “Send a message…”
• No “Starting Claude chat…” interstitial
• Resource Manager: 0 terminal sessions

Claude after send (same tab/session id):

• User bubble: Reply with the single word pong and nothing else.
• Assistant: pong (“Worked for 2s”)
• Tab id stayed structured-agent-session-claude_e9305a11_db62_4912_8a22_3b05005c8fe4
• Session id stayed claude_e9305a11_db62_4912_8a22_3b05005c8fe4
• Still 0 terminal sessions

Codex (same-turn launch handle):

• elapsedMs: 2
• surface.kind: local-agent-session
• lifecycle: pending, status: pending
• Tab Codex Chat, ptyId: null, no starting-label
• After paint: “Start a chat with Codex” + composer, still no terminal fallback

Backing store evidence

• Settings: experimentalNativeChat, experimentalStructuredNativeChat, openAgentTabsInChatByDefault all true
• Runtime capabilities include agent-session.structured.v1
• Unified tabs: two agent-session rows (claude + codex), empty ptyIdsByTabId
• Claude launch registry retired to idle after publication (expected)
• Codex same-turn registry: pending before host publication
• Renderer error collector: []
• Playwright console: 0 errors, 3 warnings (React DevTools / Tooltip / React Grab) — none native-chat
• Electron log: no Structured agent launch failed / Error: matches for this run; one pre-existing MaxListenersExceededWarning on BrowserWindow

Screenshots

Uncropped 1728×994 hidden-renderer CDP captures:

Local paths (not committed): /tmp/orca-pr-20502-before.png, /tmp/orca-pr-20502-after.png, /tmp/orca-pr-20502-codex.png

Cleanup

Playwright session pr20502 closed. Validation Electron process group for run-electron-vite-dev.mjs -c /tmp/orca-electron-vite-5176.mjs terminated after command-line check. Isolated profile/repo under /tmp/orca-pr-20502-validation removed. Screenshots were uploaded via gh image and were not committed.

Remaining gaps

• window.api.runtime.call is contextBridge-frozen (writable:false, configurable:false); a renderer wrap could not deterministically delay or fail agentSession.create. Provider attach on this idle machine was fast (~2s Claude; Codex pending only in the same JS turn).
• NativeChatLaunchRetry / visibility-unknown was not exercised; no injected unknown RPC outcome.
• Close-before-attach and late-publication suppression were not exercised in this live run.
• Linux/Windows/SSH/relay/paired-web/mobile were not run.
• Dev chrome (React Grab toolbar) is visible in screenshots; it is not a product interstitial.

[brennanb2025]

Review complete on 1224a61f49.

• Same-model review/fix loop returned clean and pushed the meaningful fixes.
• Fixed the duplicate initial structured-chat prompt race by coordinating launch settlement with the mounted outbox and rechecking the persisted head before dispatch.
• Fixed the cancellation lifecycle race by retaining cancellation tombstones until any in-flight launch settles and a newer authoritative census can retire them.
• Updated close-path tests and the launch retry localization catalog.
• Validation passed: 102 focused tests in the reviewer loop (84 lifecycle-focused tests rerun locally), pnpm run check:code-quality:changed with 0 findings, pnpm tc:web, localization catalog/runtime/extraction/coverage checks, formatting, lint, and git diff --check.
• Hidden Electron/CDP validation passed with screenshots attached in the preceding comment. Claude and Codex opened directly into native chat with no terminal fallback or startup interstitial.

The current PR matrix is green for typecheck, test shards, package (macOS and Windows), changed E2E build/specs, and wire compatibility. The remaining red static analysis/verify result is from src/shared/agent-status-legacy-adapter.ts:95, an unrelated file from the current base branch that violates the newly enabled anti-slop rule; no PR file is implicated.

[pullfrog]

ℹ️ No new issues found in this commit. The two findings from the previous review remain open and unchanged by this delta, so approval is withheld.

Reviewed changes

Reviewed the incremental delta 5d8692db1c..f9054bc564 — commit f9054bc564 ("coordinate launch prompt settlement", 13 files).

• Shared launch-prompt dispatch — new shareStructuredAgentLaunchPromptDispatch / getStructuredAgentLaunchPromptDispatch keyed by sessionId:clientMessageId:fence, so the provisional-chat outbox hook and the launch settlement issue one agentSession.send for the same staged entry; launch entries gain source: 'launch' and a fence-agnostic prefix lookup lets a hook that mounts before the fence joins the in-flight admission.
• Outbox dispatch extraction — the RPC/disposition path moved to structured-agent-session-outbox-dispatch.ts; readOutbox gained recoverDispatching, and the mounted reader demotes an interrupted dispatching entry to unconfirmed only when no launch dispatch is in flight, so mirror reads no longer mutate state.
• Cancellation retirement fencing — new structured-agent-session-launch-cancellation.ts arms a tombstone's retirement at authoritativeInventorySequence + 1 when the cancelled launch settles; refreshLocalStructuredSessionTabs allocates the authoritative-inventory sequence before IPC and threads it through StructuredSessionSnapshotApplyOptions, so a reply that began before settlement can no longer retire the fence.
• Status hook extraction — useStructuredAgentLaunchStatus moved to structured-agent-session-launch-status.ts (re-exported from structured-agent-session-launch.ts) to keep the registry under the max-lines cap.

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

[brennanb2025]

Closing readiness pass completed at 1224a61f49.

• Verdict: pass with residual risk; no proven P0/P1/P2 defect in the PR diff.
• All changed-test batches passed (395 tests), plus pnpm tc, changed-code quality (0 findings), localization checks, diff checks, and the focused lifecycle suites.
• The review fixes remain scoped to native structured-chat launch visibility, outbox ownership, cancellation fencing, close-path expectations, and required retry copy.
• A narrow renderer-reload scenario remains unproven: a persisted cancellation tombstone has no in-memory launch-settlement fence after reload. This is low-impact residual risk with no release block and no additional scope added.
• The only red remote checks remain static analysis and aggregate verify because the synthetic merge includes the unrelated base-branch finding at src/shared/agent-status-legacy-adapter.ts:95 (Reflect.get).

[pullfrog]

✅ No new issues found in the two follow-up commits.

Reviewed changes

Reviewed the incremental delta f9054bc564..1224a61f49 — commits b25c9d3b3e ("align unified close ownership") and 1224a61f49 ("correct lifecycle imports and test typing"), 6 files.

• Fixed a broken lifecycle import — snapshot-apply.ts now imports beginStructuredAgentSessionAuthoritativeInventory from structured-agent-session-launch-cancellation instead of re-importing it from the registry, which never re-exported it.
• Aligned close tests with the unified owner — useTabGroupWorkspaceModel.focus.test.ts, terminal-workspace-keydown.test.ts, and useTabGroupTabCloseCommands.structured-session.test.ts now assert the close routes through closeUnifiedTab and that closeStructuredAgentSession / session.tabs.close / cancelStructuredAgentLaunch are not called directly.
• Test typo fix — declared initialProps explicitly in use-structured-agent-session-outbox.test.tsx to give the fence prop a stable number | null type.
• i18n copy — added the worktree.creation.flow.structured.launch.unknown string and the NativeChatLaunchRetry strings (failed / unknown / retry) to en-runtime-required.json and en.json; no duplicate keys.

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

[pullfrog]

ℹ️ No critical issues — one reachability note inline.

Reviewed changes

Incremental delta 1224a61f49..c516c10a10 — commit c516c10a10 ("fence restored launch cancellations", 4 files).

• Restored cancellation fences — restoreCancellationRetirementFences seeds a restored / retireAfterInventory: null entry for every persisted tombstone the process has not already seen, so a tombstone loaded after a reload is no longer retired by the next authoritative census.
• Host cleanup before retirement — claimStructuredAgentLaunchCancellationCleanups claims each restored tombstone once, refreshLocalStructuredSessionTabs issues a best-effort agentSession.close, and settleStructuredAgentLaunchCancellationCleanup arms the fence at authoritativeInventorySequence + 1 on success (or resets cleanupStarted for retry on failure).
• Tombstone id export — readStructuredAgentLaunchCancellationTombstoneSessionIds exposes the persisted ids to the fence restore.
• Test — a restored cancellation is drained across an authoritative then an ordinary inventory: the tombstone is retained until the close settles, exactly one agentSession.close is issued, and it retires on the following census.

The two inline findings from the prior review (structured-agent-session-launch.ts:181-190, structured-agent-session-tab-retirement.ts:66) are unchanged by this delta, so approval remains withheld.

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

[pullfrog]

result.authoritative can only be true for a client that advertises SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY, and the desktop renderer's fixed capability list omits it (src/main/ipc/runtime.ts:86-93). On native this branch therefore never fires, so a restored tombstone's host cleanup runs only when launch recovery or the retired-epoch repair lane issues an explicit authoritative: true refresh — not on the non-authoritative startup restore. Fail-closed either way (the tombstone keeps suppressing), so this is a deferred host-child close rather than a correctness gap; flagging it in case the intent was to drain restored tombstones at startup.

Technical details

# Restored-cancellation cleanup is unreachable on the desktop startup path

## Affected sites
- `src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts:82` — the `result.authoritative === true` cleanup gate.
- `src/main/ipc/runtime.ts:86-93` (and `:138-145`) — the desktop renderer's `clientCapabilities` list, which omits `SESSION_TABS_AUTHORITATIVE_INVENTORY_RUNTIME_CAPABILITY`.
- `src/main/runtime/rpc/methods/session-tabs-inventory.ts:20-26,58-61` — `projectInventory` strips `authoritative` unless the client advertised the capability.
- `src/renderer/src/runtime/local-structured-session-tabs-sync/inventory-refresh.ts:45-57` — `restoreLocalStructuredSessionTabsOnce` calls `refreshLocalStructuredSessionTabs(expectedGeneration)` with no `options`, so the pre-IPC cleanup also does not run at startup.

## Required outcome
- Confirm whether restored tombstones are intended to be drained on the native startup restore or only by the explicit-authoritative repair/recovery lanes. If the former, the startup restore needs to trigger the cleanup (e.g. pass `authoritative: true`, or invoke `startRestoredLaunchCancellationCleanup` for a labeled census in the `startLocalStructuredSessionTabsSync` path); if the latter, a brief comment noting the desktop label gap would prevent a future reader assuming startup drains them.

## Open questions for the human (optional)
- Is the deferred host-child close on native acceptable, given the tombstone suppresses the row until a later authoritative refresh runs?