Skip to content

refactor(agent-status): delete the runtime's retained row store (PR 1b) - #19785

Merged
brennanb2025 merged 19 commits into
mainfrom
brennanb2025/agent-status-1b-drop-row-store
Sep 11, 2026
Merged

brennanb2025 merged 19 commits into
mainfrom
brennanb2025/agent-status-1b-drop-row-store

Conversation

@brennanb2025

@brennanb2025 brennanb2025 commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 38 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1779 $\color{#cf222e}{\Huge{\mathbf{−}}}$​291 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1488
Prod 63 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​1469 $\color{#cf222e}{\Huge{\mathbf{−}}}$​703 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​766

Summary

This change removes the runtime's duplicate in-memory agent-status row store. The hook server is now the single execution-host owner of agent-status rows, and desktop, worktree ps, mobile, and fleet projections all read from that one source.

In plain English

Previously, the same terminal could have one status row in the hook server and a second copy in the runtime. Those copies could disagree about the terminal identity, freshness, or dismissal state. A row dismissed in one place could therefore remain visible elsewhere.

Now the hook server records the terminal handle and lifecycle identity when status arrives. Readers select the current row from the hook-server snapshot, matching by the stable terminal handle when a pane key changes. This removes the split-brain state instead of adding another precedence rule.

Why there was a second store before

The runtime store filled a real integration gap at the time: runtime consumers such as mobile and worktree ps needed the latest OSC row, a PTY-based fallback when pane bindings moved, and a change signal for status-only session-tab updates. The runtime therefore retained its own copy while the hook server retained the same status for hook delivery and persistence, and readers reconciled the two copies.

The gap is now closed at the authoritative boundary. OSC status, hook posts, relays, and structured sessions all write to the hook-server store; those rows carry the optional runtime terminal handle and the store emits row-mutation notifications. Readers can resolve the current row by pane key or terminal handle and receive dismissals, reconnects, and teardown from the same owner. The runtime copy, PTY-id fallback, and reader-side precedence logic no longer provide unique information, so keeping them would only recreate the split-brain state.

What changed

  • Deleted RuntimeAgentRowStore, which duplicated hook-server status retention.
  • Stamped optional runtime-only terminal handles onto enriched hook status events.
  • Added handle-based row ownership and lifecycle fencing for pane moves, reconnects, stale events, and teardown.
  • Added event-driven row-mutation notifications so dismissals and identity transfers reach all projections consistently.
  • Updated orcad startup and teardown wiring so the hook store is attached after PTY and restored-worker recovery.
  • Replaced retained-store reads with focused hook-snapshot selectors for explicit and mobile status projections.
  • Added regression coverage for identity recovery, dismissal, startup failure cleanup, SSH/WSL relay behavior, mobile projection, and the shared test harness.

User-visible behavior

  • Dismissing an agent row removes it from desktop worktree ps and mobile projections together.
  • Reconnecting or moving a terminal preserves the correct agent identity instead of creating a stale duplicate row.
  • Late status from an ended or replaced session cannot resurrect the old row.
  • The change does not add a required wire field or persist terminal handles; the new identity fields remain optional and runtime-only.

Scope

This is focused on agent-status ownership and lifecycle consistency. No unrelated product behavior was changed, and no native mobile source was modified.

Validation

  • Focused suites: 89 passed, 10 skipped.
  • Readiness-focused suites: 74 passed.
  • pnpm tc:node passed.
  • Changed-code quality reported zero findings.
  • git diff --check passed.
  • Hosted static analysis, typecheck, Windows packaging, Node 24 test shards, SSH suites, and the E2E build passed.

The Linux package job has encountered Ubuntu Docker mirror apt-get timeouts, and the changed-E2E job has encountered Electron renderer/GPU and watcher-process crashes in the hosted runner. Those failures occur outside the changed code path; the relevant local and hosted functional suites otherwise pass. The Electron/CDP manual-validation path was successfully attached using a free CDP port and an alternate renderer port after the default renderer port was occupied by another worktree; the exact branch identity and rendered Orca surface were verified. In that isolated target worktree, the real Agents sidebar launched authenticated Claude, Codex, and OpenCode sessions and rendered their lifecycle rows through working to Done; the full-app screenshot is included below. Focused automated suites cover the remaining teardown, reconnect, dismissal, SSH/WSL, and mobile projection cases.

Visual proof

@brennanb2025
brennanb2025 force-pushed the brennanb2025/agent-status-1b-drop-row-store branch from d65010e to 3bbd8d9 Compare September 10, 2026 08:11
@brennanb2025
brennanb2025 marked this pull request as ready for review September 10, 2026 18:27
Merge Sim added 5 commits September 10, 2026 11:29
Names the five RuntimeAgentRowStore call sites and what each becomes, why
terminalHandle has to be stamped before the store can go, and the one
intended behavior change.
The runtime's retained row store carried the pty binding two readers need. Put
that fact on the row that already owns the pane instead, resolved through the
same lookup the renderer-facing IPC boundary runs, so the two surfaces cannot
disagree about which terminal a pane is.

Carried forward when a later write resolves no handle (only main's OSC parse
can), and never persisted: a handle belongs to the runtime that issued it.
`retain()` was not only a duplicate store: its boolean return was the signal
that republished `session.tabs` for a status-only transition, which no title
change covers (#7970). `hook-status-session-tabs-invalidation.ts` already
mirrors that change set plus hook restore provenance, so route the signal off
the store rather than keep a second comparator.

Adds the status-drop arm a user dismissal emits, which the pane-clear fan-out
deliberately skips — now load-bearing, because a dismissed row leaves the
listing at once.

Installed on both hosts. orcad had neither the OSC producer nor this signal, so
its runtime observed agent status and published it nowhere; deleting the
retained copy without wiring it would list no PTY agents there at all.
…tore

`RuntimeAgentRowStore` held the same payload the hook server already holds, so
the same pane could legitimately read differently in the sidebar, in
`worktree ps`, and on the phone. Both of its readers move onto the store's
snapshot in `runtime-hook-agent-row-selection.ts`, and
`collectRuntimeWorktreePtyAgentSources` loses the retained-versus-hook
reconciliation that only existed because two stores could disagree.

`ConnectedPtyEvidence` trades its flat pty-id set for `ptyIdByTerminalHandle`,
which is how a row still resolves the connected PTY behind it — the
working-terminal rollup's match key, and the last rescue for a row whose pane
binding a controller incarnation nulled under it.

The one intended behavior change: a row the user dismisses on the desktop
leaves `worktree ps` and mobile at once instead of lingering until the pty
exits. One store means one dismissal.

The suites written against the retained store are rewired to a real
AgentHookServer rather than deleted, so each still asserts the listing
behavior it named.
Past tense, plus two corrections to the plan: `terminalHandle` is not the pty
id (they are different identifiers, and the explicit-status reader was already
comparing against a real handle), and the legacy numeric pane key is a
consequence the plan did not name.
@brennanb2025
brennanb2025 force-pushed the brennanb2025/agent-status-1b-drop-row-store branch from 3bbd8d9 to 0f2bc5d Compare September 10, 2026 18:31
@brennanb2025
brennanb2025 changed the base branch from brennanb2025/agent-status-store to main September 10, 2026 18:31
@coderabbitai

coderabbitai Bot commented Sep 10, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

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: f0c51aec-0b71-4a70-8dfc-8826ab524e98

📥 Commits

Reviewing files that changed from the base of the PR and between 66f4ed9 and 8f1b23e.

📒 Files selected for processing (21)
  • src/main/agent-awake-service-platform-assertions.test.ts
  • src/main/agent-awake-service.test.ts
  • src/main/agent-awake-service.ts
  • src/main/agent-awake-status-lease.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.test.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.ts
  • src/main/agent-hooks/server-start-failure-lifecycle.test.ts
  • src/main/agent-hooks/server.ts
  • src/main/agent-hooks/server/server-authority-fences.ts
  • src/main/agent-hooks/server/server-lifecycle.ts
  • src/main/agent-hooks/server/server-listeners.ts
  • src/main/agent-hooks/server/server-state.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/orcad/orcad-entry.ts
  • src/main/runtime/agent-status-store-wiring.test-fixture.ts
  • src/main/runtime/mobile-session-tabs-agent-status-heartbeat.test.ts
  • src/main/runtime/mobile-session-tabs-agent-status-heartbeat.ts
  • src/main/runtime/orca-runtime-touch-mobile-session-tabs-for-worktree.ts
  • src/main/startup/main-process-observers.ts
  • src/main/startup/main-process-state.ts
💤 Files with no reviewable changes (1)
  • src/main/startup/main-process-state.ts

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


📝 Walkthrough

Walkthrough

The runtime now records terminal handles on agent-status events and keeps them runtime-only. The retained-row store was removed. Hook snapshots now drive explicit and mobile row selection, with terminal-handle PTY evidence. Session-tab republishing now uses one installer for status-row mutations. Orcad startup and teardown now use lifecycle wiring and status-store dependencies. Tests and documentation cover terminal-handle identity, row selection, dismissal, compatibility, and republishing.

Priority: ➖ Normal

Merge Risk: 🟡 Moderate · up to 8f1b2

Startup failures can prevent immediate Orcad retry, while certain legacy dismissals and multi-handle mobile views can show stale status. Resolve these before merge.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.51% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 89 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ⚠️ Warning The description clearly explains the change, rationale, behavior impact, scope, validation, and visual proof. However, it omits the required Linked Issue entry and does not include the template's chec… Add a linked issue in the required Fixes #<issue-number> format. Complete or explicitly mark the remaining template sections, including AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist.
✅ Passed checks (3 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 identifies the primary change: deleting the runtime's retained agent-status row store.
Full details: Description check

Explanation

The description clearly explains the change, rationale, behavior impact, scope, validation, and visual proof. However, it omits the required Linked Issue entry and does not include the template's checklist and other review sections.

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 2d77ecd7-609b-4ece-a190-0fa78f9c31f5

📥 Commits

Reviewing files that changed from the base of the PR and between 26f9fd8 and 0f2bc5d.

📒 Files selected for processing (43)
  • docs/reference/agent-status-store.md
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.ts
  • src/main/agent-hooks/server/server-ingest-terminal.ts
  • src/main/agent-hooks/server/server-persistence.ts
  • src/main/agent-hooks/server/server-status-identity.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/agent-hooks/terminal-handle-row-identity.test.ts
  • src/main/orcad/orcad-entry.ts
  • src/main/runtime/agent-status-store-wiring.test-fixture.ts
  • src/main/runtime/orca-runtime-apply-tracked-pty-title.ts
  • src/main/runtime/orca-runtime-create-terminal-side-effect-command-code-detector.ts
  • src/main/runtime/orca-runtime-fit-override-listeners.ts
  • src/main/runtime/orca-runtime-get-worktree-ps.ts
  • src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts
  • src/main/runtime/orca-runtime-on-pty-data.ts
  • src/main/runtime/orca-runtime-on-pty-exit.ts
  • src/main/runtime/orca-runtime-prune-mobile-session-tab-group-layout.ts
  • src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
  • src/main/runtime/orca-runtime-serialize-agent-prompt-submission.ts
  • src/main/runtime/orca-runtime-tests/agent-status-and-waits.spec.ts
  • src/main/runtime/orca-runtime-tests/headless-snapshots.spec.ts
  • src/main/runtime/orca-runtime-tests/mobile-summaries-part-02.spec.ts
  • src/main/runtime/orca-runtime-tests/mobile-summaries-part-03.spec.ts
  • src/main/runtime/orca-runtime-tests/mobile-summaries.spec.ts
  • src/main/runtime/orca-runtime-tests/terminal-output-and-worker-recovery.spec.ts
  • src/main/runtime/orca-runtime-tests/worktree-ps-agent-row-dismissal.spec.ts
  • src/main/runtime/orca-runtime.test.ts
  • src/main/runtime/rpc/methods/orchestration/worker/agent-status-producer-census.test.ts
  • src/main/runtime/runtime-agent-row-store.ts
  • src/main/runtime/runtime-hook-agent-row-selection.test.ts
  • src/main/runtime/runtime-hook-agent-row-selection.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/runtime-worktree-agent-rows-structured.test.ts
  • src/main/runtime/runtime-worktree-agent-rows.ts
  • src/main/runtime/runtime-worktree-agent-sources.test.ts
  • src/main/runtime/runtime-worktree-ps-activity.ts
  • src/main/runtime/runtime-worktree-pty-agent-sources.ts
  • src/main/runtime/runtime-worktree-structured-agent-rows-liveness.test.ts
  • src/main/runtime/terminal-interactive-wait-visibility.test.ts
  • src/main/startup/main-process-observers.ts
  • src/shared/agent-hook-listener/listener-event.ts
💤 Files with no reviewable changes (5)
  • src/main/runtime/orca-runtime-apply-tracked-pty-title.ts
  • src/main/runtime/runtime-agent-row-store.ts
  • src/main/runtime/orca-runtime-on-pty-exit.ts
  • src/main/runtime/orca-runtime-fit-override-listeners.ts
  • src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts

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

Comment thread src/main/orcad/orcad-entry.ts Resolved

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

The terminalHandle-based rejoining this PR introduces drops the ptyId rescue for leaf-connected terminals whose handle was never pre-allocated — a case the old verbatim ptyId on retainedSnapshots bridged. See the inline comment on orca-runtime-get-worktree-ps.ts.

Reviewed changes

  • Deletes RuntimeAgentRowStore and rewrites its five readers over the hook server's snapshot via the new runtime-hook-agent-row-selection.ts (selectFreshExplicitAgentStatus, selectFreshAgentRowForMobileTab).
  • Stamps terminalHandle on OSC-ingested rows (getAgentStatusTerminalHandleForPaneKey in the detector), carries it forward in applyNormalizedStatus, strips it on persistence, and publishes it via toAgentStatusIpcPayload.
  • Consolidates the session.tabs republish signal into installHookStatusSessionTabsRepublish — now covering enriched status, pane clear, and the new status-drop tap — wired into both main-process-observers.ts and orcad-entry.ts.
  • Swaps ConnectedPtyEvidence.ptyIds for ptyIdByTerminalHandle and drops the retained-versus-hook reconciliation from collectRuntimeWorktreePtyAgentSources.
  • Rewires tests through makeAgentStatusStoreWiring, plus new terminal-handle-row-identity and worktree-ps-agent-row-dismissal characterization specs; also binds orcad's OSC producer so a headless host still lists PTY agents.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ℹ️ Re-review of the rebase delta: no new code. The branch was rebased onto main after #19683 merged (base brennanb2025/agent-status-store → main); all five commits are identical in subject and content, only their SHAs changed.

Reviewed changes

  • Base rebase only. docs/, src/ content in this PR is unchanged from the prior review; orca-runtime-get-worktree-ps.ts:99-100 and runtime-worktree-pty-agent-sources.ts:36-38 are byte-identical, so the unresolved finding from the prior review about terminalHandle-based ptyId rejoining for non-pre-allocated leaf terminals still stands as-is.

Pullfrog  | Fix it ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important

The mobile projection still rejoins rows on issuePtyHandle, while the row carries the leaf handle — so the worktree ps rescue this commit delivered via getExistingTerminalHandlesForPtyId is still a silent drop on the phone. See inline.

Reviewed changes

  • Restored the worktree ps ptyId rescue — replaced getTerminalHandleForPty (which only resolved pty:-prefixed handles) with getExistingTerminalHandlesForPtyId, resolving leaf (handleByLeafKey), pre-allocated (handleByPtyId), and incarnation (handleByPtyIncarnation) handles; collectRuntimeWorktreePtyAgentSources now rejoins a row through any of them.
  • Hardened single-store teardown — certified PTY exit, provider-generation reset, and PTY-record pruning evict status rows (including handle-joined and dismissed remnants) via collectAgentStatusPaneKeysForPty → reconcileAgentStatusForEndedProcess.
  • Admitted runtime-owned legacy numeric panes without opening HTTP/relay ingress, gated on ptyId + terminalHandle + matching tabId.
  • Refactored orcad startup/stop into startOrcadWithLifecycle (idempotent cleanup, startup-failure rollback) and bound onTerminalAgentStatus so a headless host lists PTY agents.
  • Tightened liveness — providerSessionOnly and stale evidenceObservedAt now read as dead in selectFresh*, collectRuntimeWorktreePtyAgentSources, and the mobile projection (agentStatusAuthorityObservedAt).
  • Added coverage for handle rejoin after pane ownership clears (leaf + incarnation), certified-exit/provider-generation/prune eviction, cross-connection terminalHandle gating, and legacy numeric pane admission.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/runtime/runtime-mobile-agent-status-projection.ts (1)

132-141: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Order selectFreshAgentRowForMobileTab by authority observation time.

The helper filters rows by evidenceObservedAt ?? receivedAt but selects them by receivedAt. A replayed row can therefore replace a newer authority observation. Compare evidenceObservedAt ?? receivedAt first, and use receivedAt only as a tie-breaker while preserving pane-key priority.


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: c9e02a5b-5701-4d10-b1dc-9eaa34cdfc18

📥 Commits

Reviewing files that changed from the base of the PR and between 0f2bc5d and c3cc86c.

📒 Files selected for processing (43)
  • docs/reference/agent-status-store.md
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.ts
  • src/main/agent-hooks/server-ingest-terminal-status.test.ts
  • src/main/agent-hooks/server-status-listener-fanout.test.ts
  • src/main/agent-hooks/server/server-ingest-terminal.ts
  • src/main/agent-hooks/server/server-status-disposition.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-tab-cleanup.ts
  • src/main/agent-hooks/terminal-handle-row-identity.test.ts
  • src/main/ipc/agent-hooks.test.ts
  • src/main/ipc/agent-status-row-teardown-ipc.ts
  • src/main/orcad/orcad-entry.ts
  • src/main/orcad/orcad-launch-contract.test.ts
  • src/main/orcad/orcad-lifecycle.ts
  • src/main/runtime/agent-status-store-wiring.test-fixture.ts
  • src/main/runtime/mobile-agent-status-permission-renewal.test.ts
  • src/main/runtime/orca-runtime-apply-tracked-pty-title.ts
  • src/main/runtime/orca-runtime-get-orchestration-dispatch-authority.ts
  • src/main/runtime/orca-runtime-get-worktree-ps.ts
  • src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts
  • src/main/runtime/orca-runtime-on-pty-exit.ts
  • src/main/runtime/orca-runtime-prune-mobile-session-tab-group-layout.ts
  • src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
  • src/main/runtime/orca-runtime-stop-exact-terminals-for-worktree.ts
  • src/main/runtime/orca-runtime-tests/mobile-session-tabs-part-08.spec.ts
  • src/main/runtime/orca-runtime-tests/worktree-ps-agent-row-dismissal.spec.ts
  • src/main/runtime/runtime-hook-agent-row-selection.test.ts
  • src/main/runtime/runtime-hook-agent-row-selection.ts
  • src/main/runtime/runtime-mobile-agent-status-builder.ts
  • src/main/runtime/runtime-mobile-agent-status-projection.ts
  • src/main/runtime/runtime-mobile-session-projection-contract.ts
  • src/main/runtime/runtime-mobile-session-projection.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/runtime-worktree-agent-sources.test.ts
  • src/main/runtime/runtime-worktree-ps-activity.ts
  • src/main/runtime/runtime-worktree-pty-agent-sources.ts
  • src/main/ssh/ssh-relay-session-agent-hooks.integration.test.ts
  • src/main/ssh/ssh-relay-session.ts
  • src/shared/orchestration-fleet-agent-status-evidence.ts
  • tests/e2e/session-tabs-decorative-title-fanout.unit.test.ts
  • tests/e2e/session-tabs-rich-status-boundaries.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/main/agent-hooks/hook-status-session-tabs-republish.ts
  • src/main/runtime/runtime-terminal-contracts.ts
  • src/main/runtime/runtime-hook-agent-row-selection.test.ts
  • src/main/runtime/runtime-hook-agent-row-selection.ts
  • docs/reference/agent-status-store.md

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

if (typeof paneKey !== 'string' || !isValidPaneKey(paneKey)) {
if (
typeof paneKey !== 'string' ||
(!isValidPaneKey(paneKey) && parseLegacyNumericPaneKey(paneKey) === null)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use the normalized legacy pane key.

Line 33 accepts whitespace-padded legacy keys because parseLegacyNumericPaneKey() trims them. Lines 42-43 then use the untrimmed value, so no stored row matches and the dismissal has no effect. Store the parser result and pass legacy?.paneKey ?? paneKey to both operations. Add a regression case for ' tab-1:0 '.

Proposed fix
   ipcMain.on('agentStatus:drop', (_event, paneKey: unknown) => {
+    const legacyPaneKey = parseLegacyNumericPaneKey(paneKey)
     if (
       typeof paneKey !== 'string' ||
-      (!isValidPaneKey(paneKey) && parseLegacyNumericPaneKey(paneKey) === null)
+      (!isValidPaneKey(paneKey) && legacyPaneKey === null)
     ) {
       return
     }
+    const resolvedPaneKey = legacyPaneKey?.paneKey ?? paneKey
     try {
-      agentHookServer.dropStatusEntry(paneKey)
-      clearMigrationUnsupportedPtysForPaneKey(paneKey)
+      agentHookServer.dropStatusEntry(resolvedPaneKey)
+      clearMigrationUnsupportedPtysForPaneKey(resolvedPaneKey)

instanceLock.release()
throw error
}
return startOrcadWithLifecycle(

Copy link
Copy Markdown
Contributor

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

Release the instance lock when browser-provider setup fails.

startOrcadWithLifecycle starts after resolveOrcadBrowserProvider() runs. If browser-provider resolution rejects after acquireOrcadInstanceLock() succeeds, the lifecycle catch does not run and instanceLock.release() is skipped. A caller that catches this startup error cannot restart Orcad in the same process because lock acquisition fails.

Start lifecycle management before the first fallible operation after lock acquisition, or release the lock in a setup-failure path.

(entry.providerSessionOnly !== true ||
(entry.agentType === 'pi' && entry.providerSession != null)) &&
entry.receivedAt >= freshAfter &&
(entry.evidenceObservedAt ?? entry.receivedAt) >= freshAfter &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use authority observation time for candidate ordering.

These conditions admit rows according to evidenceObservedAt, but Lines 133 and 142 still choose the candidate with the newest receivedAt.

If an older observation is replayed after a newer observation, the replay receives a later delivery timestamp and wins selection. Mobile can then publish an older agent state or prompt.

Order agent and live by authority observation time. Use receivedAt only to resolve equal observation timestamps.

Also applies to: 141-141

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 44d6e473-9143-4d8b-bc50-a1af632a20af

📥 Commits

Reviewing files that changed from the base of the PR and between c3cc86c and 9471b34.

📒 Files selected for processing (4)
  • src/main/runtime/orca-runtime-hook-agent-status-projection.test.ts
  • src/main/runtime/orca-runtime-prune-mobile-session-tab-group-layout.ts
  • src/main/runtime/orca-runtime-refresh-floating-workspace-pty-liveness.ts
  • src/main/runtime/orca-runtime-tests/worktree-ps-agent-row-dismissal.spec.ts

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

Comment on lines +152 to +153
if (handleMatch) {
return handleMatch

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Select the newest matching handle row.

getExistingTerminalHandlesForPtyId combines leaf, runtime, and incarnation handles, then sorts them lexically. selectFreshAgentRowForMobileTab selects the newest row only for the current handle. When multiple live rows exist for different handles, the loop returns the first lexical match and can expose an older status. Compare all handle matches by updatedAt before returning, and add a regression test with two fresh rows on different handles.

Proposed fix
+    let newestHandleMatch: RuntimeAgentRowSnapshot | null = null
     for (const terminalHandle of this.getExistingTerminalHandlesForPtyId(pty.ptyId)) {
       const handleMatch = selectFreshAgentRowForMobileTab({
         paneKey,
         terminalHandle,
         hookRows: getRows(paneKey, terminalHandle)
       })
-      if (handleMatch) {
-        return handleMatch
+      if (
+        handleMatch &&
+        (!newestHandleMatch || handleMatch.updatedAt > newestHandleMatch.updatedAt)
+      ) {
+        newestHandleMatch = handleMatch
       }
     }
-    return null
+    return newestHandleMatch

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Reworked the mobile rejoin in getFreshRetainedAgentStatusForMobileTab (orca-runtime-prune-mobile-session-tab-group-layout.ts:130) — it now resolves a pane-key match first (with terminalHandle: null), then falls back to rejoining through getExistingTerminalHandlesForPtyId (the same non-minting leaf / pre-allocated / incarnation inventory worktree ps uses) instead of minting issuePtyHandle against the row's carried handle.
  • Added a mobile rejoin characterization test (rejoins mobile status through the incarnation handle after a tab id remint in worktree-ps-agent-row-dismissal.spec.ts:140) that drives the real OSC byte path, remints the tab id, and asserts the still-live row resolves to the mobile tab.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Gated status eviction on certified process death in dropDisconnectedPtyRecord (orca-runtime-refresh-floating-workspace-pty-liveness.ts:120) — reconcileAgentStatusForEndedProcess now runs only when connectionId === null (a local PTY) or the pty liveness verdict is exited, so pruning a disconnected remote record keeps the host-owned row unverifiable instead of evicting it.
  • Added a prune-path test (keeps an unverifiable remote row when its disconnected PTY record is pruned in orca-runtime-hook-agent-status-projection.test.ts) mirroring the existing local-row eviction test.

This is the consistent third seam in the SSH boundary work: remote disconnect is unverifiable, not exited, so the row survives until certified exit (matching ssh-relay-session.ts and the relay integration test).

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 4


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 224a5495-239d-48e0-a3db-b2e7f078f63b

📥 Commits

Reviewing files that changed from the base of the PR and between 9471b34 and 66f4ed9.

📒 Files selected for processing (36)
  • src/main/agent-hooks/hook-provider-session-invalidation.test.ts
  • src/main/agent-hooks/hook-provider-session-invalidation.ts
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.test.ts
  • src/main/agent-hooks/hook-status-session-tabs-republish.ts
  • src/main/agent-hooks/server-ingest-terminal-status.test.ts
  • src/main/agent-hooks/server-start-failure-lifecycle.test.ts
  • src/main/agent-hooks/server.ts
  • src/main/agent-hooks/server/server-authority-aliases.ts
  • src/main/agent-hooks/server/server-authority-fences.ts
  • src/main/agent-hooks/server/server-cleanup.ts
  • src/main/agent-hooks/server/server-ingest-terminal.ts
  • src/main/agent-hooks/server/server-lifecycle.ts
  • src/main/agent-hooks/server/server-reaping.ts
  • src/main/agent-hooks/server/server-row-ownership.ts
  • src/main/agent-hooks/server/server-state.ts
  • src/main/agent-hooks/server/server-status-inference.ts
  • src/main/agent-hooks/server/server-status-update.ts
  • src/main/agent-hooks/server/server-tab-cleanup.ts
  • src/main/agent-hooks/server/server-types.ts
  • src/main/agent-hooks/terminal-handle-row-identity.test.ts
  • src/main/orcad/orcad-entry.ts
  • src/main/runtime/agent-status-store-wiring.test-fixture.ts
  • src/main/runtime/orca-runtime-bind-pty-incarnation-handle.ts
  • src/main/runtime/orca-runtime-build-pty-terminal-summary.ts
  • src/main/runtime/orca-runtime-get-pty-record-for-pane-key.ts
  • src/main/runtime/orca-runtime-has-terminals-for-worktree.ts
  • src/main/runtime/orca-runtime-stop-requested-pty-ids.ts
  • src/main/runtime/orca-runtime-tests/terminal-handles-part-02.spec.ts
  • src/main/runtime/orca-runtime-tests/terminal-handles.spec.ts
  • src/main/runtime/orca-runtime-tests/worktree-ps-agent-row-dismissal.spec.ts
  • src/main/startup/headless-pty-hydration-ordering.test.ts
  • src/main/startup/main-process-observers.ts
  • src/main/startup/main-process-runtime-service.ts
  • src/main/startup/main-process-state.ts
💤 Files with no reviewable changes (6)
  • src/main/agent-hooks/hook-provider-session-invalidation.ts
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.test.ts
  • src/main/agent-hooks/hook-status-session-tabs-invalidation.ts
  • src/main/startup/main-process-runtime-service.ts
  • src/main/startup/main-process-state.ts
  • src/main/agent-hooks/hook-provider-session-invalidation.test.ts

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

Comment on lines +143 to +146
clearedStatusRows.set(
entry.stablePaneKey,
this.state.lastStatusByPaneKey.get(entry.stablePaneKey) as EnrichedAgentHookEventPayload
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🔎 Supported by static analysis

🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/conventions

Length of output: 38707


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- changed file and directly related definitions ---'
for f in \
  src/main/agent-hooks/server/server-authority-fences.ts \
  src/main/agent-hooks/server/server-tab-cleanup.ts
do
  if [ -f "$f" ]; then
    printf '\n--- %s ---\n' "$f"
    wc -l "$f"
    sed -n '1,240p' "$f"
  fi
done
printf '\n--- directly related symbols ---\n'
rg -n -S \
  'lastStatusByPaneKey|clearedStatusRows|statusRowsToClear|paneCacheKeyMatchesTab|legacy|stablePaneKey|statusPaneKeysToClear|mutation-only|republish' \
  src/main/agent-hooks src/main 2>/dev/null | head -n 300

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- cache clear and key identity contracts ---'
rg -n -C 12 \
  'function clearPaneCacheState|const clearPaneCacheState|export .*clearPaneCacheState|function paneCacheKeyMatchesTab|export .*paneCacheKeyMatchesTab' \
  src/shared src/main/agent-hooks/server

printf '\n--- alias status movement and registration ---\n'
sed -n '1,250p' src/main/agent-hooks/server/server-authority-aliases.ts
sed -n '1,180p' src/main/agent-hooks/server/server-hydration.ts
sed -n '1,130p' src/main/agent-hooks/server/server-status-application.ts
sed -n '1,115p' src/main/agent-hooks/server/server-ingest-terminal.ts

printf '\n--- focused tests for legacy status cleanup ---\n'
rg -n -C 8 \
  'clears hydrated stable statuses|lastStatusByPaneKey.*legacy|legacy.*status|dropStatusEntriesByTabPrefix|clearPaneKeyAliasesForPty|commitStatusRowMutation|emitPaneStatusCleared' \
  src/main/agent-hooks/*test.ts src/main/agent-hooks/server/*test.ts 2>/dev/null | head -n 500

Repository: stablyai/orca

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -eu
printf '%s\n' '--- exact cache deletion semantics ---'
sed -n '1,140p' src/shared/agent-hook-listener/listener-state.ts

printf '\n--- mutation emission contract and republisher ---\n'
rg -n -C 14 \
  'commitStatusRowMutation|mutation-only|status row mutation|statusRowMutation|republish' \
  src/main/agent-hooks/server src/main/agent-hooks/hook-status-session-tabs-republish.ts \
  src/main/agent-hooks/*test.ts | head -n 450

printf '\n--- direct legacy-row acceptance and cleanup assertions ---\n'
rg -n -C 18 \
  'runtimeOwnedLegacyPane|paneKey: .*legacy|legacyPaneKey.*lastStatusByPaneKey|lastStatusByPaneKey.*set\\(.*legacy|clearPaneKeyAliasesForPty\\(' \
  src/main/agent-hooks/*test.ts src/main/agent-hooks/server/*test.ts | head -n 450

Repository: stablyai/orca

Length of output: 50369


Capture every status row before bulk cleanup.

clearPaneCacheState removes lastStatusByPaneKey entries for each cleared key. Runtime-owned legacy pane keys can also create status rows. Capture the legacy and stable rows that each cleanup path removes before calling clearPaneCacheState; otherwise the mutation-only republisher receives no deletion event.

  • server-authority-fences.ts: capture the legacy row as well as the stable row.
  • server-tab-cleanup.ts: expand paneKeysToClear first, then collect rows and cleared keys from the complete set.
📍 Affects 2 files
  • src/main/agent-hooks/server/server-authority-fences.ts#L143-L146 (this comment)
  • src/main/agent-hooks/server/server-tab-cleanup.ts#L12-L22

Comment on lines +94 to +97
const retained = this.handleByPtyIncarnation.get(pty.ptyId)
if (retained?.incarnationId === pty.incarnationId) {
return retained.handle
}

Copy link
Copy Markdown
Contributor

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

🔎 Supported by static analysis

🏁 Script executed:

#!/bin/bash
set -euo pipefail

file="$(fd -a '^orca-runtime-bind-pty-incarnation-handle\.ts$' . | head -n1)"
ast-grep outline "$file" --items all --match 'issuePtyHandle|reconcilePtyIncarnationHandles'

rg -n -C8 --type=ts '\bissuePtyHandle\s*\(' .
rg -n -C8 --type=ts \
  'beginGraphReload|graphStatus|captureReadyGraphEpoch|assertStableReadyGraph|reconcilePtyIncarnationHandles' \
  src/main/runtime

Repository: stablyai/orca

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

target="src/main/runtime/orca-runtime-bind-pty-incarnation-handle.ts"
printf '%s\n' '--- handle binding implementation ---'
sed -n '1,180p' "$target"

printf '%s\n' '--- lifecycle symbols in runtime sources ---'
rg -n --glob '*.ts' \
  'beginGraphReload|reconcilePtyIncarnationHandles|assertStableReadyGraph|captureReadyGraphEpoch|graphStatus' \
  src/main/runtime --glob '!**/*.test.ts' --glob '!**/*.spec.ts'

printf '%s\n' '--- non-test issuePtyHandle callers ---'
rg -n --glob '*.ts' --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
  '\bissuePtyHandle\s*\(' src/main/runtime

Repository: stablyai/orca

Length of output: 14688


🤖 get_repo_knowledge executed:

get_repo_knowledge stablyai/orca /tmp/coderabbit-repo-knowledge/stablyai-orca-89dc44e4/learnings

Length of output: 34003


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- handle map declarations and mutations ---'
rg -n --glob '*.ts' --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
  'handleByPtyIncarnation|handleByPtyId|handles\.(clear|set|delete|get)|handles\s*=' \
  src/main/runtime | head -n 240

Repository: stablyai/orca

Length of output: 13521


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- graph reload lifecycle ---'
sed -n '1,125p' src/main/runtime/orca-runtime-has-terminals-for-worktree.ts
sed -n '150,195p' src/main/runtime/orca-runtime-has-terminals-for-worktree.ts

printf '%s\n' '--- graph synchronization lifecycle ---'
sed -n '55,245p' src/main/runtime/orca-runtime-sync-window-graph.ts

printf '%s\n' '--- terminal summary handle lookup ---'
sed -n '1,115p' src/main/runtime/orca-runtime-build-pty-terminal-summary.ts

printf '%s\n' '--- direct caller context ---'
sed -n '110,155p' src/main/runtime/orca-runtime-stop-exact-terminals-for-worktree.ts
sed -n '215,255p' src/main/runtime/orca-runtime-create-terminal.ts

Repository: stablyai/orca

Length of output: 23560


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- buildPtyTerminalSummary callers ---'
rg -n -C10 --glob '*.ts' --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
  '\bbuildPtyTerminalSummary\s*\(' src/main/runtime

printf '%s\n' '--- projection and inventory entry points ---'
rg -n -C8 --glob '*.ts' --glob '!**/*.test.ts' --glob '!**/*.spec.ts' \
  'getTerminalHandlesForPtyId|buildPtyTerminalSummary|RuntimeAgentOrchestrationProjection|project\(' \
  src/main/runtime | head -n 260

Repository: stablyai/orca

Length of output: 29276


Restore the retained handle record before returning it.

beginGraphReload clears handles but preserves non-null entries in handleByPtyIncarnation. The matching branch in issuePtyHandle returns that handle without restoring its TerminalHandleRecord. Terminal-summary and projection paths call issuePtyHandle directly, so a call during reload can return a handle that later lookups cannot resolve.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Consolidated the session.tabs republish onto a single mutation stream. Deleted hook-status-session-tabs-invalidation and hook-provider-session-invalidation, rewiring installHookStatusSessionTabsRepublish onto the new subscribeStatusRowMutations and deduping on a semantic row hash (semanticRowJson) that ignores only the volatile timing/observation fields.
  • Owned the PTY row lifecycle in the hook server. The new AgentHookServerRowOwnership mixin keeps an in-memory paneKeyByTerminalHandle index, and every lastStatusByPaneKey mutation site (applyNormalizedStatus, refreshTerminalStatusEvidence, transferPaneAuthority, clearPaneState, reaping, fences, cleanup) now runs through commitStatusRowMutation, which maintains the index and emits only on a semantic change.
  • Rejoined a reminted pane through the handle index. OSC ingest looks up the incoming terminalHandle, transfers a still-owned row via transferPaneAuthority (emitStatusRowMutation: false), and emits one net mutation so a PTY row and its resume identity follow a tab-id remint.
  • Carried terminalHandle across the WSL relay boundary. sameTerminalOwner treats a local pane and its matching-distro relay connection as one owner and refuses cross-worktree carry-forward.
  • Hardened startup. A failed listen now rolls back via stop() and retries end-to-end, covered by the new server-start-failure-lifecycle.test.ts.

ℹ️ Nitpicks

  • subscribeProviderSessionChanges / getProviderSessionIdentities (on AgentHookServer) and the runtime's touchMobileSessionTabsForPane are now dead in production — provider-session republish moved onto the mutation stream, but the old subscription surface remains and notifyStatusChangeListeners still computes providerSessions for no consumer. Harmless; fine to retire in a later cleanup step.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 66f4ed9 is the single fix(agent-status): preserve state and renew freshness commit:

  • Extracted the computer-awake freshness lease. AgentAwakeStatusLease now owns stale-timer scheduling and AgentAwakeService.observeStatusFreshness renews a per-pane lease without rescanning, so a continuously-working OSC agent no longer loses its sleep blocker after two hours of semantically-duplicate evidence.
  • Emitted status freshness from the hook server's OSC refresh path. refreshTerminalStatusEvidence fires emitStatusFreshnessObservation, and the new subscribeStatusFreshness fan-out reaches both the awake service and the session-tabs republisher.
  • Added a worktree-level mobile status heartbeat. The heartbeat FIFO now tracks directObservation alongside per-pty entries, and scheduleMobileSessionTabsAgentStatusHeartbeatForWorktree republishes session.tabs on a bounded cadence so a working agent doesn't read stale on mobile.
  • Refined startup rollback to preserve owner state. A failed listen now rolls back only the transport via rollbackTransportStart, keeping ownerStateInitialized set so a retry skips re-hydration instead of clobbering in-process rows with stale disk state.

The lease's earliest-expiry timer, the heartbeat FIFO's throttle/cleanup, and the retry-preserves-owner-state semantics are all pinned by focused tests, which pass.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at f2808f3 is a single merge commit (8f1b23e4, "Merge origin/main into agent-status review branch") bringing main up to ecd7b19.

  • Merged main and reconciled orcad-entry.ts — the only file both sides touched. Main's DesktopPushService / resolvePushGatewayOrigin wiring now coexists with the PR's registerCleanup teardown and installHookStatusSessionTabsRepublish wiring, and the runtimeUserDataPath / initOrcaProfilePaths() lines are retained in place.
  • No regression to the PR's structural change — runtime-agent-row-store.ts is still deleted; runtime-hook-agent-row-selection.ts, server-row-ownership.ts, and agent-awake-status-lease.ts are all intact. No conflict markers anywhere.
  • Main's additive shared edits are compatible — agent-status-types.ts adding the 'unverifiable' subagent state and subagentObservation, and main-process-state.ts adding desktopPushService, don't collide with the PR's usage of toAgentStatusIpcPayload/terminalHandle or its publishProviderSessionChanges removal.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 8f1b23e4 is the single fix(agent-status): ignore freshness for dismissed identity rows commit:

  • Gated freshness emission on provider-session-only rows. refreshTerminalStatusEvidence now returns early after commitStatusRowMutation / scheduleStatusPersist when refreshed.providerSessionOnly === true, so a dismissed row that retains only its provider resume identity (whose preserved payload can still read working) no longer renews the awake lease or the mobile heartbeat.
  • Added a focused regression test (does not renew freshness from a provider-session-only dismissal remnant) that dismisses a row, re-ingests duplicate OSC evidence, and asserts subscribeStatusFreshness never fires.

The guard sits at the only emitStatusFreshnessObservation call site (reached solely from the unchanged-OSC ingest path), placed after the row-ownership index and persistence bookkeeping but before the awake/status-change fan-out — so index maintenance and persistence are unaffected while both the awake service and the mobile heartbeat are suppressed. The test would fail without the guard, so it is real coverage, not theatre.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 22dddc2 is two commits, both centered on orcad-entry.ts:

  • Fenced orcad observed identities at ingest. orcad-entry.ts now instantiates AgentStatusObservedPaneIdentities, records each enriched status's pane identity via recordObservedAgentStatusPaneIdentity, and serves it back through readObservedAgentStatusPaneIdentity — matching the desktop's fleet stale-row fencing (main-process-runtime-service.ts) so a headless host no longer remints a cached row under whichever process owns the pane now. The subscription registers after runtime construction and before registerHeadlessPtyRuntime, so the captured runtime is in scope and no OSC ingest precedes it. A source-order characterization test pins the ordering.
  • Removed the dead daemonStarted flag. Cleanup now always calls stopOrcadDaemon(); disconnectDaemon() is null-safe (adapter?.disconnectOnly()), and the daemon adapter is only ever installed by startOrcadDaemon(), so the unconditional call is a safe no-op before the daemon starts.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 49ae9ed is the single fix(agent-status): cover remint and headless lifecycle edges commit:

  • Emitted enriched status on pane remint. refreshTerminalStatusEvidence gained an emitEnrichedStatus flag (default false); the duplicate-OSC unchanged gate now passes mutationBefore !== undefined so a handle-authority transfer publishes the replacement pane identity to enriched subscribers, instead of leaving the reminted key permanently unobserved in the fleet fencing.
  • Buffered orcad observed identities through startup. New AgentStatusObservedPaneIdentityCapture subscribes before agentHookServer.start() (so no spool observation is dropped — the tap is no-replay) and replays buffered observations only after registerHeadlessPtyRuntime recovers terminal identities, closing both the missed-spool and the unresolved-identity gaps in the prior fencing wiring.
  • Carried provider-session identity through the mobile rejoin. selectFreshAgentRowForMobileTab now returns providerSession, and the mobile builder falls back to retained.providerSession when the pane-key-selected hookRow has none, so a handle-rejoined row keeps its transcript address on the phone.
  • Preserved the launch error when rollback fails. startOrcadWithLifecycle wraps its cleanup() call in a try/catch, logging [orcad] startup cleanup failed: while rethrowing the original startup failure as the supervisor-facing verdict.

Each change is pinned by a test that fails without it (remint enriched-status emission, capture buffering, provider-session rejoin, and rollback-error preservation), so the coverage is real rather than theatre.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 6b352d5 is two commits closing edges in the remint/fencing work:

  • Closed the orcad observed-identity recovery gap. orcad-entry.ts now subscribes observedStatusCapture.observe to subscribeEnrichedStatus unconditionally (moved out of the isAgentStatusHooksEnabled gate) and defers observedStatusCapture.attach(runtime) until after refreshRestoredOrchestrationAuthority() + reconcileLegacyWorkerTerminals(), so startup-spool observations resolve against the recovered dispatch identity instead of buffering a kind: 'pane' fallback with dispatchId: null.
  • Suppressed the duplicate Claude child-only row mutation. The claudeLeadBoundaryChildOnly OSC branch in ingestTerminalStatus now commits the mutation and emits enriched status only when a pane-authority transfer actually occurred (mutationBefore !== undefined), eliminating a spurious before: null "create" mutation and its session.tabs republish on every duplicate OSC for a child-only row.

Both are pinned by tests that fail without the change (publishes only the remint observation for a Claude child-only row, plus the source-order assertions in orcad-launch-contract.test.ts and headless-pty-hydration-ordering.test.ts).

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

Merge Sim added 2 commits September 11, 2026 02:03
…atus-1b-drop-row-store

# Conflicts:
#	src/main/runtime/terminal-interactive-wait-visibility.test.ts
@greptile-apps

greptile-apps Bot commented Sep 11, 2026

Copy link
Copy Markdown

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

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

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes — the delta since the prior pullfrog review at 752e65e is a main-sync merge plus one test-infrastructure commit:

  • Merged origin/main (up to 94c2f96). A 622-file main-sync. Clean: git diff --check reports no conflict markers, the PR's structural changes survive (runtime-agent-row-store.ts still deleted; runtime-hook-agent-row-selection.ts, server-row-ownership.ts, and agent-awake-status-lease.ts all intact), and main's additive edits reconcile with the PR's usage without conflict — notably the codex-trust-workspace → agent-trust-workspace legacy-alias rename, which the PR's terminal-interactive-wait-visibility.test.ts now asserts consistently.
  • Threaded the hook-store wiring through the shared transcript-pane harness. createTranscriptPane gains an optional runtimeDeps param (backward-compatible — the Antigravity readiness suite still calls it bare), so terminal-interactive-wait-visibility.test.ts keeps exercising the production OSC status path via makeAgentStatusStoreWiring().deps instead of a bare runtime. The refactor extracts the exact wiring previously inline in that spec, preserving behavior.

The delta has no production-behavior surface; both prior Pullfrog threads remain resolved.

Pullfrog  | View workflow run | Using DeepSeek Pro (free via Pullfrog for OSS) | 𝕏

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Review status for final SHA a2b8c70462c50e765c83beb7c8bda6ae7ca6a881

  • Conflicts are resolved and the branch matches the remote. The only local change outside the PR is the pre-existing user-owned mobile/pnpm-lock.yaml, left untouched.
  • Same-model adversarial review completed clean; the final readiness checklist is PASS.
  • The fix is focused on removing duplicate runtime agent-status ownership, preserving terminal-handle identity/fencing, and keeping lifecycle and hook-store wiring correct. No unrelated product scope was added.
  • Local validation passed: focused suites (89 passed, 10 skipped), readiness-focused suites (74 passed), pnpm tc:node, changed-code quality with zero findings, and git diff --check.
  • Hosted validation passed for static analysis, typecheck, managed hooks on Node 18, Windows packaging, all Node 24 shards, localhost SSH, Docker SSH watcher isolation, and the E2E build.
  • The Linux package job failed twice on Ubuntu archive/Docker apt-fetch timeouts before any Orca package assertion ran. The changed-E2E job failed twice with renderer/GPU/network-service and watcher crashes, SIGKILLs, relay reconnects, and the same worktree-switch assertion; no in-scope code regression was reproduced. GitHub does not permit another rerun while the workflow is active, and the remaining red status is infrastructure-driven.
  • Electron hidden-CDP startup did not become available in this environment, so that manual gap remains explicitly unverified. No mobile source changed, so mobile emulator QA was not applicable.

Visual evidence:

Code review verdict: clean and ready to land once the infrastructure-only hosted failures are rerun or waived.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Updated the PR description to explain both sides of the store change:

  • Why RuntimeAgentRowStore existed: runtime consumers needed a retained OSC row, a PTY fallback across pane-binding changes, and a status-only session-tab change signal while runtime and hook-server paths were separate.
  • Why it is no longer needed: OSC, hook, relay, and structured-session producers now converge in the hook-server store, which carries the runtime terminal handle and emits row-mutation notifications. Readers can use one authoritative snapshot, so the duplicate copy and reader-side precedence logic add no unique capability.
  • Electron validation is described accurately: the $electron CDP path was attempted, but this environment never exposed a usable hidden-CDP target, so that interaction remains unverified here.

Final CI rerun status for a2b8c70462c50e765c83beb7c8bda6ae7ca6a881:

  • Package: passed.
  • Changed E2E: failed with 20 passed and one worktree-switch-first-paint failure. The logs again show renderer/network/GPU crashes, watcher interruptions, relay reconnects, and SIGKILLs around the failure, matching the earlier infrastructure failure pattern.

The code review remains clean; the PR is mergeable but still marked unstable by the hosted changed-E2E check.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Correction to the earlier Electron note: I reran the $electron flow directly using the documented wrapper and alternate-port procedure. The default renderer port was occupied by another worktree; launching this branch on renderer 5176 with CDP 9335 succeeded. Playwright attached, verified the exact branch/worktree identity, and captured the rendered Orca surface without revealing or focusing the window.

The isolated profile had no workspaces, so the agent-status row lifecycle was not exercised interactively. The focused automated suites remain the coverage for that behavior. The PR description has been corrected accordingly.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Fresh Electron/CDP visual proof from target SHA a2b8c70462c50e765c83beb7c8bda6ae7ca6a881 (child worktree, exact identity verified): the real Agents sidebar shows authenticated Claude, Codex, and OpenCode sessions as Done, with full app context visible.

Electron agents done

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Additional fresh visual-proof captures from the exact target worktree (a2b8c70462c50e765c83beb7c8bda6ae7ca6a881), all full-app and uncropped:

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Final QA status for target SHA a2b8c70462c50e765c83beb7c8bda6ae7ca6a881:

  • Review/readiness: same-model adversarial review is clean; final readiness is PASS on code findings. Focused tests (89 passed, 10 skipped), readiness suites (74 passed), pnpm tc:node, changed-code quality, and git diff --check pass.
  • Electron: CDP attached to the exact child worktree using ORCA_BACKGROUND_LAUNCH=1, free CDP port 9335, and alternate renderer port 5174; window.api.app.getIdentity() matched the target branch/worktree. Through the real Agents sidebar, authenticated Claude, Codex, and OpenCode sessions reached working then Done; dismissing the Claude row removed it from the sidebar while the remaining Done rows stayed visible.
  • Provider coverage: the complete catalog has 37 IDs; 9 local providers were detected (Claude, Claude Agent Teams, Codex, OpenCode, OMP, Gemini, Antigravity, Cursor, Grok). Claude/Codex/OpenCode had usable auth and were lifecycle-tested. Gemini and Cursor stopped at workspace/auth trust gates, Grok returned 402 Payment Required, and the remaining detected providers were not counted as lifecycle-verified; no SSH/WSL target was configured.
  • Fresh uncropped screenshots: working https://github.com/user-attachments/assets/b3779d4e-c41a-4f3f-a538-ed872accd893 · done https://github.com/user-attachments/assets/dcf18470-5e14-4286-b052-fa2ec0ee9ded · dismissed https://github.com/user-attachments/assets/fc745ce8-67cd-4a3b-b5ab-3e9982258508
  • Hosted CI: package and other required suites pass; changed E2E remains red after reruns with 20 passes and one worktree-switch-first-paint failure amid renderer/GPU/network/watcher crashes and SIGKILLs. Linux package attempts timed out during Ubuntu mirror apt-get before package assertions. These are infrastructure failures; no in-scope regression was reproduced.

Verdict: code is clean and the PR is ready from a correctness/review standpoint; merge remains contingent on rerunning or waiving the infrastructure-only hosted failures.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Electron QA (branch brennanb2025/agent-status-1b-drop-row-store)

Validated commit a2b8c70462c50e765c83beb7c8bda6ae7ca6a881 in an isolated ORCA_BACKGROUND_LAUNCH=1 Electron profile via CDP (playwright-cli -s=qa19785, CDP 9335, renderer 5174). window.api.app.getIdentity() matched this branch/worktree.

Provider matrix

Provider Result / evidence
Claude Tested — real prompt reached Working → Done (QA_CLAUDE), hook row + provider session/terminal handle present.
Claude Agent Teams Tested — real orca claude-teams launch reached Done (QA_TEAMS).
Codex Tested — real prompt reached Working → Done (QA_CODEX), gpt-6-astra, handle/session present.
OpenCode Tested — real prompt reached Working → Done (QA_OPENCODE), handle/session present.
Antigravity Tested — real agy prompt reached Working → Done (QA_ANTIGRAVITY), conversation/handle present.
OMP Unavailable — v17.0.5 TUI reported No models available / No model selected; no transition.
Gemini Unavailable — real auth flow ended Authentication consent could not be obtained; no transition.
Cursor Installed but blocked — real Workspace Trust Required stayed at Trusting workspace… >1 minute; prompt queued, no turn.
Grok Unavailable — real UI returned 402 Payment Required: Grok Build usage balance exhausted; row settled Done/error.
Other 27 catalog IDs Unavailable — command -v absent for openclaude autohand ante traecli mimo pi prime-agent aider goose amp kilo kiro-cli crush auggie cline codebuff command-code cn droid kimi vibe mistral-vibe qwen rovo hermes openclaw copilot devin.

The complete 36-ID TUI_AGENT_CONFIG catalog and command/auth evidence are in the attached report path below.

Single-store paths

  • Full desktop sidebar rows observed from real hooks (Working → Done) and real UI dismissal.
  • Dismissed Claude row disappeared; its pane key was absent from both agentStatusByPaneKey and retainedAgentsByPaneKey.
  • worktree.ps via runtime RPC returned only remaining Codex/OpenCode rows after dismissal/teardown.
  • Codex split pane right/close preserved the same status pane key and terminal handle while PTY leaf IDs changed.
  • No SSH/WSL/runtime target was configured (hostPlatform=darwin, empty target maps), so those paths are unverified.

Screenshots (uncropped full app, 1728×994)

  • Working: opencode-working.png
  • Done: claude-done.png
  • Dismissed: dismissed.png
  • Error/unavailable: grok-failed.png

Automated checks

  • terminal-handle-row-identity.test.ts: 10 passed
  • orca-runtime-hook-agent-status-projection.test.ts: 24 passed
  • server-status-listener-fanout.test.ts + server-start-failure-lifecycle.test.ts: 16 passed
  • worktree-ps-agent-row-dismissal.spec.ts is excluded by default; forced run was 1 passed/5 failed because fixture timestamps 1/2/3 are filtered as stale (no production edits).

Full evidence report: /tmp/orca-qa-evidence/PR19785-electron-qa-report.md (local to QA worktree).

Verdict: local authenticated providers and single-store UI/projection/dismissal/pane-handle paths behaved as intended. Remaining gaps are unavailable provider accounts/binaries, no SSH/WSL target, the Grok row recording agentType: claude despite Grok title, and the timestamp-sensitive forced-spec failures.

@brennanb2025

Copy link
Copy Markdown
Contributor Author

Final review update for a2b8c70462c50e765c83beb7c8bda6ae7ca6a881:

  • Same-model adversarial review and readiness checks are clean; no P0/P1/P2 or meaningful in-scope regression remains.
  • The canonical runtime aggregate entrypoint passes (1,267 passed, 1 skipped); the earlier forced standalone dismissal-spec failure was an invocation artifact, so no test workaround was added.
  • The Grok row showing Claude labeling is a known compatibility path (Claude transport can carry Grok) and is outside this PR's routing/normalization changes; no regression was proven.
  • Fresh $electron CDP validation attached to the exact target worktree produced full-app Working, Done, and Dismissed screenshots, which are linked in the PR description and QA comments.
  • Hosted red status remains limited to infrastructure: changed E2E renderer/GPU/network/watcher crashes and Linux package Docker/Ubuntu mirror timeouts. All other required checks pass.

The branch is conflict-free and matches origin; only the pre-existing user-owned mobile/pnpm-lock.yaml is modified locally.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant