Skip to content

fix(runtime-host): project authoritative live run state - #3079

Merged
Astro-Han merged 6 commits into
apache:mainfrom
Sun-GLiang:fix/3059-runtime-host-live-run-state
Aug 18, 2026
Merged

fix(runtime-host): project authoritative live run state#3079
Astro-Han merged 6 commits into
apache:mainfrom
Sun-GLiang:fix/3059-runtime-host-live-run-state

Conversation

@Sun-GLiang

Copy link
Copy Markdown
Contributor

Summary

  • project authoritative live-run state from Runtime Host session queries
  • preserve the absent versus known-empty contract across protocol, Desktop IPC, and renderer state merges
  • make session-list running indicators reflect remote runs while retaining local optimistic streaming state
  • add protocol, coordinator, IPC, renderer, UI, integration, and Storybook coverage

Fixes #3059

Verification

  • npm run lint
  • npm run format:check
  • npm run build
  • npm run typecheck
  • Runtime Host tests: 937 passed
  • Runtime tests: 2,832 passed and 12 skipped, excluding the unrelated macOS-only failure in node-pty-write-lifecycle.test.js caused by its exact file-descriptor reuse assumption
  • Desktop tests: 861 passed
  • UI tests: 169 passed
  • Storybook build and smoke tests: 131 stories passed
  • git diff --check

Visual evidence: Storybook LiveRunAuthorityStates covers local streaming, remote running, known-empty, and unknown fallback states.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the protocol, Runtime Host, Desktop, renderer, UI, Storybook, and test changes; it also ran verification and code review. The human contributor reviewed the issue scope and chose to submit the result.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions
github-actions Bot requested a lite review from Copilot August 15, 2026 15:12
@coderabbitai

coderabbitai Bot commented Aug 15, 2026

Copy link
Copy Markdown

Review 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 2234378e-8a27-44e3-bbdf-7d90d66efe61

📥 Commits

Reviewing files that changed from the base of the PR and between 49f8675 and e609776.

📒 Files selected for processing (21)
  • apps/desktop/src/main/__tests__/app-shell-session-ui-state.test.ts
  • apps/desktop/src/main/__tests__/runtime-host-session-catalog-ipc-main.test.ts
  • apps/desktop/src/main/__tests__/runtime-host-session-catalog-running-turns.test.ts
  • apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts
  • apps/desktop/src/renderer/session-status-presentation.ts
  • apps/desktop/src/renderer/settled-session-transients.ts
  • apps/desktop/src/renderer/use-app-shell-session-list.ts
  • packages/core/src/session.ts
  • packages/runtime-host/src/__tests__/authenticated-websocket.test.ts
  • packages/runtime-host/src/__tests__/protocol.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-protocol.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-two-client-uds.test.ts
  • packages/runtime-host/src/protocol/index.ts
  • packages/runtime-host/src/protocol/session-catalog.ts
  • packages/runtime-host/src/server/session-catalog-coordinator.ts
  • packages/runtime/src/__tests__/session-manager.test.ts
  • packages/runtime/src/session-manager.ts
  • packages/ui/src/__tests__/session-history-row-actions.test.tsx
  • packages/ui/src/session-history-list.tsx
  • packages/ui/stories/session-list-panel.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (17)
  • packages/runtime-host/src/protocol/index.ts
  • apps/desktop/src/renderer/use-app-shell-session-list.ts
  • packages/runtime/src/session-manager.ts
  • packages/ui/src/tests/session-history-row-actions.test.tsx
  • packages/ui/stories/session-list-panel.stories.tsx
  • apps/desktop/src/main/tests/runtime-host-session-catalog-ipc-main.test.ts
  • apps/desktop/src/main/tests/app-shell-session-ui-state.test.ts
  • packages/runtime-host/src/tests/protocol.test.ts
  • packages/core/src/session.ts
  • packages/ui/src/session-history-list.tsx
  • apps/desktop/src/renderer/settled-session-transients.ts
  • packages/runtime-host/src/server/session-catalog-coordinator.ts
  • packages/runtime-host/src/tests/session-catalog-two-client-uds.test.ts
  • packages/runtime-host/src/tests/session-catalog-protocol.test.ts
  • packages/runtime-host/src/protocol/session-catalog.ts
  • packages/runtime-host/src/tests/session-catalog-coordinator.test.ts
  • packages/runtime/src/tests/session-manager.test.ts

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.


📝 Walkthrough

Summary

This PR fixes stale and incomplete session run indicators. Runtime Host projects authoritative live-run state through the versioned protocol, Desktop IPC, renderer merges, and the session-list UI.

It extends the existing session catalog source of truth. It does not create a parallel execution-state path. Local renderer streaming remains an optimistic signal for local runs, while Runtime Host state identifies remote runs.

The solution is the smallest coherent path across the affected layers. Known-empty and unknown states distinguish “no run exists” from “run state is unavailable.” Renderer merge helpers preserve live-run state when metadata mutation responses omit it. Protocol limits and validation protect the protocol boundary.

No safe code or test deletion is apparent. Removing the state distinctions, merge behavior, or coverage would weaken required behavior or regression protection.

Complexity delta

  • Authorities: Runtime Host becomes authoritative for execution state. Local renderer streaming remains an optimistic supplementary signal.
  • States: The protocol and UI now distinguish active, known-empty, and unknown live-run state.
  • Branches: Renderer, Desktop, and UI logic adds merge and fallback branches for omitted, empty, and populated runningTurnIds.
  • Configuration and public surface: The compatibility epoch increases from 24 to 25. The session catalog adds versioned liveRunState data with a 64-item limit and validation.
  • Tests: Coverage expands across protocol validation, Runtime Host projection, Desktop IPC, renderer merging, UI indicators, integration scenarios, and Storybook.

The added complexity is required to preserve the authority boundary and the distinction between known-empty and unknown state. Total maintenance complexity stays justified by the required behavior and regression coverage.

Validation

The change includes coverage for:

  • Protocol versioning and live-run schema validation.
  • Runtime Host catalog projection and coordinator behavior.
  • Desktop IPC conversion and running-turn merging.
  • Renderer normalization and list merging.
  • Local streaming, remote running, known-empty, and unknown states.
  • Stale persisted running status settlement.
  • UI indicators and Storybook states.
  • Two-client and response-loss scenarios.

The supplied objectives report lint, formatting, build, typecheck, Runtime Host, Runtime, Desktop, UI, Storybook, and diff checks. Direct results for the current PR are not available here. Required check status remains unverified.

Review-relevant risks

  • The compatibility epoch changes from 24 to 25, and the session catalog protocol adds versioned liveRunState data. This affects a public protocol contract and release compatibility. Material changes in this area require independent human review under repository policy.
  • Session-list indicators now show remote runs and clear stale persisted running status when Runtime Host reports known-empty state. This changes user-visible behavior. Material changes in this area require independent human review under repository policy.
  • No security, licensing, or governance effect was identified in the current diff.
  • The person performing the merge reviews the final diff. A maintainer makes the final determination.

Walkthrough

The change adds versioned live-run state to Runtime Host session catalog projections. Runtime reports running turn IDs. Desktop preserves known state during metadata updates. Renderer and UI logic normalize and display session activity.

Changes

Live-run state authority

Layer / File(s) Summary
Live-run state protocol contract
packages/core/src/session.ts, packages/runtime-host/src/protocol/*, packages/runtime-host/src/__tests__/session-catalog-protocol.test.ts, packages/runtime-host/src/__tests__/protocol.test.ts
Defines versioned liveRunState, validates turn IDs and collection limits, and updates the compatibility epoch.
Runtime Host catalog projection
packages/runtime/src/session-manager.ts, packages/runtime-host/src/server/session-catalog-coordinator.ts, packages/runtime-host/src/__tests__/*catalog*, packages/runtime-host/src/__tests__/authenticated-websocket.test.ts
Projects empty or populated running-turn state in catalog responses and omits it from metadata-only responses.
Desktop display-state merging
apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts, apps/desktop/src/renderer/session-status-presentation.ts, apps/desktop/src/renderer/settled-session-transients.ts, apps/desktop/src/renderer/use-app-shell-session-list.ts, apps/desktop/src/main/__tests__/*
Preserves live state across summary updates, normalizes stale persisted status, and allows explicitly empty runtime state to settle sessions.
UI live-status rendering
packages/ui/src/session-history-list.tsx, packages/ui/src/__tests__/session-history-row-actions.test.tsx, packages/ui/stories/session-list-panel.stories.tsx
Uses Runtime Host running turns and local streaming state to render activity indicators and Storybook scenarios.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: 🟡 Moderate · up to e6097

The change can misrepresent an active session when its projected turn data exceeds the protocol limit, causing live-run indicators or session state to fall back incorrectly; merge should wait for this boundedness issue to be fixed or explicitly accepted by the owner.

Sequence Diagram(s)

sequenceDiagram
  participant Runtime
  participant RuntimeHost
  participant Desktop
  participant SessionHistory
  Runtime->>RuntimeHost: Report runningTurnIds
  RuntimeHost->>Desktop: Return liveRunState in catalog projection
  Desktop->>Desktop: Merge live state with incoming summaries
  Desktop->>SessionHistory: Provide normalized session summary
  SessionHistory->>SessionHistory: Resolve remote and local activity
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the primary change: projecting authoritative live-run state from Runtime Host.
Description check ✅ Passed The description covers the required summary, verification, AI use, checklist, behavior change, and linked issue.
Linked Issues check ✅ Passed The implementation satisfies issue #3059 by projecting versioned live-run state, preserving known-empty semantics, and merging omitted state safely.
Out of Scope Changes check ✅ Passed The protocol, Runtime Host, Desktop, renderer, UI, integration, and Storybook changes directly support the linked issue objectives.
Ai Use Disclosure ✅ Passed The PR selects generative use and names Codex with scope. All four PR-side commit messages contain standalone, consistent Generated-by: Codex trailers.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Visual verification: the Storybook state matrix covers local streaming, remote running, authoritative known-empty, and unknown fallback behavior.

issue-3059-live-run-authority

Copilot AI 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.

Pull request overview

This PR makes Runtime Host the authoritative source of “live run” state for sessions (including a strict distinction between unknown vs known-empty), and wires that through the protocol, Desktop IPC, and UI so running indicators reflect remote/bot/other-window runs while keeping renderer-local optimistic streaming.

Changes:

  • Add a versioned liveRunState projection to the Runtime Host session catalog protocol and bump compatibility epoch.
  • Project/merge live run state through Runtime Host coordinator → Desktop main IPC → renderer state, preserving “absent vs known-empty” semantics across mutation responses.
  • Update UI indicators (Host live runs OR local streaming) and add tests + Storybook coverage for the new states.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
packages/ui/stories/session-list-panel.stories.tsx Adds Storybook coverage for unknown/known-empty/remote-running/local-streaming states.
packages/ui/src/session-history-list.tsx Updates row signal logic to OR Host live runs with local streaming and treat persisted running as fallback only when live state is unknown.
packages/ui/src/tests/session-history-row-actions.test.tsx Adds UI tests covering Host live runs and known-empty behavior.
packages/runtime/src/session-manager.ts Ensures listSessions() projects runningTurnIds even when empty (known-empty).
packages/runtime/src/tests/session-manager.test.ts Adds regression test for preserving known-empty live run state from listSessions().
packages/runtime-host/src/server/session-catalog-coordinator.ts Projects liveRunState from Runtime authority into catalog query results.
packages/runtime-host/src/protocol/session-catalog.ts Defines liveRunState schema + bounds; decodes it without collapsing absent vs known-empty.
packages/runtime-host/src/protocol/index.ts Bumps RUNTIME_HOST_COMPATIBILITY_EPOCH for the new catalog live-run projection.
packages/runtime-host/src/tests/session-catalog-two-client-uds.test.ts Updates integration expectations to include known-empty liveRunState in catalog reads.
packages/runtime-host/src/tests/session-catalog-protocol.test.ts Adds protocol-level tests for live-run decoding + validation bounds.
packages/runtime-host/src/tests/session-catalog-coordinator.test.ts Verifies catalog queries project known-empty/running state and mutation responses omit live state.
packages/runtime-host/src/tests/protocol.test.ts Updates epoch assertion for the new compatibility epoch.
packages/runtime-host/src/tests/authenticated-websocket.test.ts Updates WS integration expectations to include known-empty liveRunState.
packages/core/src/session.ts Updates SessionSummary.runningTurnIds contract to distinguish known-empty vs unknown.
apps/desktop/src/renderer/use-app-shell-session-list.ts Preserves live-run state across list merges and mutation upserts in renderer state.
apps/desktop/src/renderer/settled-session-transients.ts Adjusts transient-settlement rules to respect known-empty vs unknown running state.
apps/desktop/src/renderer/session-status-presentation.ts Normalizes/merges session summaries to clear stale persisted running only when live state is known-empty.
apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts Maps Runtime Host liveRunState into Desktop SessionSummary.runningTurnIds without collapsing absent vs known-empty.
apps/desktop/src/main/tests/runtime-host-session-catalog-ipc-main.test.ts Adds unit coverage for the IPC mapping and absent vs known-empty semantics.
apps/desktop/src/main/tests/app-shell-session-ui-state.test.ts Adds tests for renderer merge/normalize semantics and updates settlement expectations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread packages/runtime-host/src/server/session-catalog-coordinator.ts Outdated
Comment thread packages/runtime/src/session-manager.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: cd8784e0-f5e8-4507-9368-2aea13e1c68d

📥 Commits

Reviewing files that changed from the base of the PR and between 62cded2 and 5b87f71.

📒 Files selected for processing (20)
  • apps/desktop/src/main/__tests__/app-shell-session-ui-state.test.ts
  • apps/desktop/src/main/__tests__/runtime-host-session-catalog-ipc-main.test.ts
  • apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts
  • apps/desktop/src/renderer/session-status-presentation.ts
  • apps/desktop/src/renderer/settled-session-transients.ts
  • apps/desktop/src/renderer/use-app-shell-session-list.ts
  • packages/core/src/session.ts
  • packages/runtime-host/src/__tests__/authenticated-websocket.test.ts
  • packages/runtime-host/src/__tests__/protocol.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-protocol.test.ts
  • packages/runtime-host/src/__tests__/session-catalog-two-client-uds.test.ts
  • packages/runtime-host/src/protocol/index.ts
  • packages/runtime-host/src/protocol/session-catalog.ts
  • packages/runtime-host/src/server/session-catalog-coordinator.ts
  • packages/runtime/src/__tests__/session-manager.test.ts
  • packages/runtime/src/session-manager.ts
  • packages/ui/src/__tests__/session-history-row-actions.test.tsx
  • packages/ui/src/session-history-list.tsx
  • packages/ui/stories/session-list-panel.stories.tsx

Comment thread packages/runtime-host/src/protocol/session-catalog.ts Outdated
Comment thread packages/runtime-host/src/server/session-catalog-coordinator.ts Outdated

Copilot AI 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.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Copilot AI 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.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

@hqhq1025

I’ve resolved the conflicts with the latest upstream/main and pushed the merge commit.

The failing request-review job was not caused by the changes in this PR. The workflow successfully authenticates COPILOT_REVIEW_TOKEN, but GitHub rejects the request to add the Copilot reviewer with HTTP 403 (Resource not accessible by personal access token). The same workflow has also failed on other PRs since #3139 was merged, indicating a repository-level token permission or authorization issue rather than a regression introduced by this PR.

Since this is a pull_request_target workflow loaded from the base branch, it cannot be fixed from this PR branch.

Copilot AI 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.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

@Astro-Han

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

Merge conflicts have been resolved by merging the latest upstream/main into this branch. The conflict resolutions preserve both the Runtime Host live-run state behavior and the updated Session Catalog semantics. GitHub now reports the PR as mergeable and clean.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the rework (and sorry for the slow response on the @-mention) — the architecture direction is right and I verified it: live-run truth belongs in the Runtime Host kernel (RuntimeKernel.activeRuns is the only authority; the durable header can be stale after crash, the desktop observer is a downstream projection, local streaming only covers one window), and projecting it as a versioned optional liveRunState into the existing catalog protocol (with known-empty vs unknown distinction) extends the single source of truth rather than building a parallel path. The epoch bump and the 64-cap are wired. Tests cover protocol decode (unknown/known-empty/running/malformed/sparse/duplicates), coordinator (projection/truncation/mutation-omission), session-manager, IPC mapping, renderer merge/normalize, and UI + Storybook matrix.

Conclusion: PASS with no P0/P1 — but the PR is currently DIRTY and needs rebase, and there are three P2s; two of them must be resolved as part of the rebase, not mechanically.

P2-1 — waiting_for_user/blocked sessions are now shown as "Responding" (pulsing active dot), masking "waiting on a person". session-history-list.tsx:742 ORs local streaming into runningTurnIds.length > 0 and pushes the active dot as signals[0] — but a run stays in activeRuns from reserveGenerationRun until finalize(), so a permission prompt (waiting_for_user via agent-run.ts:859) keeps runningTurnIds non-empty. Result: on the flagship remote/second-window scenario, the row pulses "Responding" while the system is actually blocked waiting for the user, and the attention dot (explicitly defined as "waiting on a person" in session-status-presentation.ts:43) gets demoted to signals[1]. This is a regression introduced by this PR. Fix: gate the OR on status !== 'waiting_for_user' && status !== 'blocked' (or add a UI test with {status:'waiting_for_user', runningTurnIds:['turn-x']} asserting the attention cue stays first).

P2-2 — dual authority on Desktop runningTurnIds with the already-merged #3189; the rebase must make a design decision, not a mechanical conflict resolution. #3189's toDesktopHostSessionListSummary projects observedRunningTurnIds (from the observer; omits when empty) into the same field this PR writes Host liveRunState into (which maps known-empty too). After merge the semantics become "observer wins when non-empty, Host known-empty only survives when the observer is empty" — and both settledSessionTransients readers consume it. The post-auto-merge code even keeps #3189's comment "this shared presentational component does not arbitrate those sources itself" while the logic now does. Please decide (and state) who owns this field — Host kernel vs Desktop observer — and add a "Host known-empty + observer non-empty" combination test. (The other conflicts are mechanical: epoch 23→24, sessionStatusdecodeSessionStatus from #3159, the page() filter signature from #3165 — take 24/>22 compatible.)

P2-3 — mutation-path merge can resurrect a finished live state. mergeSessionSummaryForDisplay keeps current.runningTurnIds when incoming.runningTurnIds === undefined, and setSessions applies that merge to every updater — so a rename/model/read-market mutation landing between turn end (next catalog read reports known-empty []) and the next refresh resurrects ['turn-live'], and the row keeps pulsing Responding until some handler triggers a refresh. Bounded today because most mutation handlers call refreshSessions(), but it's an unbounded window for any path that doesn't (e.g. markSessionReadLocally). Either test "current=['turn-live'] → mutation omits → not resurrected" or explicitly accept the window with a guarantee that every mutation refreshes.

P3 (optional): the new Set dedupe in projectCatalogLiveRunState is redundant (RuntimeKernel.runningTurnIds already dedupes) — protocol-boundary defense is fine but deletable; >64 unique running turns silently degrades the whole field to unknown (bounded, but "truncate vs omit" should be an explicit decision); the renderer-helper unit test lives in a main-process test file, and one test uses destructuring + double-assert instead of a clean named assertion; SessionManager.listSessions/listChildSessions known-empty projection barely affects production Desktop (already all-Host-protocol) but is good contract consistency.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which traced the run lifecycle (reserveGenerationRunfinalize, waiting_for_user mid-run), the #3189 observer path, the mutation-merge chain, and probed the rebase conflicts in a temporary worktree. P2-1/P2-3 are code-path consequences from the PR head; P2-2 is a merge-semantics prediction for the DIRTY state. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS(无 P0/P1)但当前 DIRTY 需 rebase,3 个 P2 里两个必须在 rebase 时做设计决策而非机械解冲突。方向正确:live-run 唯一权威是 Runtime Host 内核 activeRuns(持久化 header 可被 crash 残留、observer 是下游投影、本地 streaming 只覆盖单窗口),以版本化可选字段 liveRunState 投影进既有 catalog 协议(known-empty vs unknown 区分)是扩展现有单一事实源而非并行路径;epoch bump 与 64 上限已接线;测试覆盖面广。P2-1(本 PR 引入的 UI 回归):waiting_for_user/blocked 会话被标为 Responding(脉动 active dot 当 signals[0])——run 从 reserveGenerationRun 到 finalize 都在 activeRuns,权限等待(agent-run.ts:859)时 runningTurnIds 非空;主打场景(远程/第二窗口权限确认)下用户等输入却显示"正在处理",attention dot(明确定义"waiting on a person")被贬到 signals[1]。修复:OR 条件 gate status!=='waiting_for_user'&&!=='blocked' 或加 UI 测试。P2-2(关键):与已合并 #3189 在 Desktop runningTurnIds 双 authority——#3189 observer 投影 obsatruningTurnIds(空时省略)与本 PR 的 Host liveRunState(known-empty 也映射)写同一字段,合并后"observer 非空时覆盖 Host、known-empty 仅 observer 空时保留",settledSessionTransients 两处读取方都会消费;auto-merge 后还保留 #3189 注释"组件不仲裁来源"而逻辑已在仲裁——作者必须裁决该字段归属(Host 内核 vs observer)并补组合测试。其余冲突机械(epoch 23→24、#3159 的 decodeSessionStatus、#3165 的 page() 签名)。P2-3:mutation 路径 merge 会复活已结束 live 状态(incoming.runningTurnIds===undefined 时保留 current),turn 结束(下次 catalog 报 known-empty)与刷新之间任何 mutation(改名/改模型/read-marker)复活 ['turn-live'],markSessionReadLocally 类不走 refresh 的路径窗口无界——加"mutation 省略不复活"测试或显式接受窗口并保证每次 mutation 必刷。P3(可选):projectCatalogLiveRunState 的 Set 去重冗余(内核已去重,协议边界防御可留可删);>64 个唯一 running turns 时整字段静默降级 unknown(行为有界但"截断 vs 省略"应显式决定);renderer 单测放进了 main-process 测试文件;SessionManager 列表 known-empty 投影在生产 Desktop 基本不生效但契约一致性好。

@Astro-Han

Copy link
Copy Markdown
Contributor

Follow-up after your rebase — re-verified on head e60977613. Both remaining P2s from my first pass are now resolved:

  • P2-2 (dual authority) — resolved as a real design decision, not a mechanical merge. The conflict resolution rewrote fix(desktop): preserve live turns across session refreshes #3189's replace-semantics into a union (runtime-host-session-catalog-ipc-main.ts:366-373 [...new Set([...(summary.runningTurnIds ?? []), ...runningTurnIds])]), and the deps comment now states the ownership explicitly: the Host kernel owns known-empty/known-running semantics, the Desktop observer may only append non-empty local observations and can neither delete nor override Host state. Known-empty maps to [] and can only be union-appended, never collapsed. Locked by a stability-order union test plus the unknown/known-empty/running IPC tri-state tests. Good.
  • P2-3 (mutation resurrect) — handled as intent + tests. mergeSessionSummaryForDisplay keeps its preserve semantics deliberately, with 3 new tests pinning the boundaries (mutation doesn't resurrect or clear in-flight state; authoritative known-empty replaces stale running and clears status; list-boundary metadata replacement preserves live authority), and handleSessionChange refreshes on every sessions:changed which bounds the stale window. Acceptable.

But P2-1 is still open with no deferral statement, and it's user-visible: packages/ui/src/session-history-list.tsx:742 still ORs options.streaming into runningTurnIds.length > 0 with no status gate, so a waiting_for_user/blocked session (permission prompt — the flagship remote/second-window scenario this PR is about) shows a pulsing "Responding" dot while it's actually waiting on a person; the attention cue defined as "waiting on a person" (session-status-presentation.ts:43) is demoted to signals[1]. I rendered the real component with {status:'waiting_for_user', runningTurnIds:['turn-1']} and got aria-label="Responding". Pre-PR, the parked-waiting row showed the attention dot. This is a regression introduced by this PR.

Per the review contract this needs one of: (a) a one-line gate — status !== 'waiting_for_user' && status !== 'blocked' on the OR — plus a UI test with {status:'waiting_for_user', runningTurnIds:['turn-x']} asserting the attention cue stays first and the story 'status-waiting' gets runningTurnIds injected; or (b) an explicit statement in the PR that "showing Responding while waiting for user permission is accepted behavior" with a reason. Without either, I can't approve as-is.

Holding the merge on that one item — everything else is green (epoch 25 chain is clean, schema decode tests pass, 16/16 CI, CLEAN/mergeable).

P3 (optional): the 'status-waiting' story doesn't inject runningTurnIds (same gap as the UI test); no literal "Host known-empty [] + observer non-empty" combo test (semantics covered by the union branch, a literal case would be sturdier); new Set dedupe in projectCatalogLiveRunState is redundant (kernel already dedupes); >64 unique running turns silently degrades the whole field to unknown (bounded, but "truncate vs omit" deserves an explicit decision).

AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on opencode-go/deepseek-v4-flash), which built all affected workspaces on head, ran the affected suites, rendered the live component for the P2-1 repro, and traced the conflict-resolution diff. P2-1 is an observed render; P2-2/P2-3 conclusions are verification of your handling. Please weigh these findings with your own judgment.

中文摘要

复评(head e609776)结论:P2-2/P2-3 已解决,但 P2-1 未修且无显式延后——不能按现状合并。P2-2(双 authority):冲突解决把 #3189 的替换语义改写为并集(runtime-host-session-catalog-ipc-main.ts:366-373),deps 注释明确了归属:Host 内核拥有 known-empty/known-running 语义,Desktop observer 只能追加非空本地观测,不能删除或覆盖 Host 状态;known-empty 映射 [] 后只能被并集追加、永不会塌缩;有稳定性序并集测试 + unknown/known-empty/running IPC 三态测试锁定。P2-3(mutation 复活):保留语义是显式意图 + 3 个新测试钉死边界(mutation 不复活也不清除 in-flight;权威 known-empty 替换陈旧 running 并清 status;列表边界 metadata 替换保留 live authority),handleSessionChange 每次 sessions:changed 都刷新保证陈旧窗口有界——可接受。P2-1(等待用户被标 Responding)still open:session-history-list.tsx:742 仍把 streaming OR runningTurnIds.length>0 推为 signals[0] 无 status gate;权限等待(本 PR 主打场景)时行显示脉动"Responding"而实际在等用户,attention 语义("waiting on a person")被贬到 signals[1];我已用真实组件渲染 {status:'waiting_for_user', runningTurnIds:['turn-1']} 得到 aria-label="Responding";PR 前停车等待行显示 attention dot——这是本 PR 引入的回归。按契约需二选一:(a) 一行 gate(status!=='waiting_for_user'&&!=='blocked')+ UI 测试断言 attention cue 仍首位 + story 'status-waiting' 注入 runningTurnIds;或 (b) 在 PR 中显式声明"等待用户时显示 Responding 是接受的行为"及理由。两者皆无则不能 approve。其余全绿(epoch 25 链干净、schema decode 测试过、16/16 CI、CLEAN)。P3(可选):status-waiting story 未注入 runningTurnIds(与 UI 测试同缺口);缺字面"Host known-empty [] + observer 非空"组合测试(语义已被并集分支覆盖,字面用例更稳);projectCatalogLiveRunState 的 Set 去重冗余;>64 个唯一 running turns 整字段静默降级 unknown("截断 vs 省略"值得显式决定)。

@Sun-GLiang

Copy link
Copy Markdown
Contributor Author

@Astro-Han Resolved in 189d18de6. waiting_for_user and blocked now suppress the Responding signal even when Runtime Host live-run state or renderer-local streaming remains present, so the attention state stays primary while the run is parked. Added UI regression coverage for both statuses. Verification: UI 183/183, Biome check, and UI typecheck pass.

@Astro-Han Astro-Han 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.

Thanks for moving the session rail away from durable status: running as its primary execution signal. I reviewed the current head with two independent @reviewer slices plus a read-only ollama-cloud/deepseek-v4-flash:high pass.

The problem is correctly defined: Runtime live turns are ephemeral facts and must come from Runtime/Host state, while the persisted header is only a fallback when live state is unavailable. The implementation keeps that authority in RuntimeKernel.runningTurnIds, projects a known-empty list explicitly, and suppresses “Responding” when the session is waiting for the user or blocked.

I specifically rechecked the apparent Host/observer conflict. The Desktop observer is not a second execution authority inferred from durable state; it is another Host subscription snapshot used for the short synchronization window before the catalog refresh lands. The union is intentional and covered, while terminal attention states take precedence in the UI. I could not establish a concrete stale-state failure that survives the observer's terminal snapshot handling, so I am not promoting that concern to P2.

I found no concrete P0–P2 issue on this head, and current CI is green. A future simplification could version the two live snapshots or eliminate the Desktop supplement once catalog freshness is sufficient, but that is not required for this fix.

No local test suite was run during this review; conclusions are based on source, test, and current CI inspection. Codex coordinated the independent passes and performed the final adjudication; external-model output was treated as unverified until checked against the code.

中文摘要

感谢把 session rail 从持久化的 status: running 迁移到真正的 live execution fact。问题定义正确:Runtime live turns 是瞬时事实,应来自 Runtime/Host;持久化 header 只在 live state 不可用时作为 fallback。

我重点复核了 Host 与 Desktop observer 可能冲突的问题。Desktop observer 不是从 durable state 推断出的第二权威,而是另一个 Host subscription snapshot,用于 catalog refresh 到达前的短同步窗口;union 是有意设计且有测试,waiting/blocked 也会在 UI 中优先。没有找到能越过 terminal snapshot 清理并持续存在的具体 stale failure,因此不将其升级为 P2。

当前 head 未发现具体 P0–P2,CI 全绿。未来如果 catalog freshness 足够,可以给两份 snapshot 加版本或删除 Desktop supplement,但这不是本修复的必要条件。

本次未运行本地测试套件;结论来自源码、测试和当前 CI 检查。Codex 协调了独立审查并完成最终判断,外部模型输出在核对代码前均视为未验证输入。

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.

Runtime Host should project live-run state, distinguishing known-empty from unknown

3 participants