feat(agent-graph): browse historical graph runs - #2991
Conversation
da55cfe to
1b61baa
Compare
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 3 included reviews per hour; 1 remains after this review. 📝 WalkthroughSummaryThis PR adds bounded historical Agent Graph reads through the Runtime Host. Desktop and TUI clients can inspect prior Graph epochs in read-only mode without direct SQLite access or an Agent Turn. The Runtime Host validates that epoch directories and graph snapshots belong to the requested root Session. Desktop follows the current epoch by default and prevents stale refreshes from replacing newer selections. TUI adds Design and scopeThe PR extends the existing epoch store, Runtime Host protocol, coordinator, and client paths. It does not create a parallel persistence or client-read path. Pagination, graph-ID validation, and protocol plumbing are necessary for bounded reads, epoch transitions, and root-Session isolation. The implementation appears to be the smallest coherent solution for the requested behavior. No code or tests can be clearly deleted or simplified without weakening behavior or regression coverage. The tests cover graph identity, pagination, truncation, read-only behavior, stale selection recovery, notification refreshes, disposal, and the absence of an Agent Turn. Complexity delta
Maintenance complexity increases. The increase is justified by the requested behavior and its session-boundary and refresh-consistency requirements. ValidationThe PR summary reports build, typecheck, 146 Agent Graph/Desktop/TUI tests, Biome checks, and Direct current check-result records are not available. Therefore, the final status of required checks remains unverified. Review-relevant risks
WalkthroughThe change adds paginated Agent Graph epoch history and graph-specific snapshot access. Desktop and CLI interfaces can select historical epochs, retrieve snapshots, and render read-only views. It also adds refresh scheduling and remote project-directory selection. ChangesAgent Graph history and project selection
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: ⚪ Minimal · up to The PR adds read-only browsing of historical Agent Graph runs without changing active execution behavior, and no actionable merge-blocking risk remains beyond normal checks and review. Sequence Diagram(s)sequenceDiagram
participant User
participant AgentGraphPanel
participant DesktopPreload
participant RuntimeHost
participant AgentGraphCoordinator
User->>AgentGraphPanel: Select historical epoch
AgentGraphPanel->>DesktopPreload: graphs.listEpochs(rootSessionId)
DesktopPreload->>RuntimeHost: graphs:listEpochs
RuntimeHost->>AgentGraphCoordinator: agent.graph.epochs.query
AgentGraphCoordinator-->>RuntimeHost: epoch summaries
RuntimeHost-->>AgentGraphPanel: epoch summaries
AgentGraphPanel->>DesktopPreload: graphs.getSnapshot(rootSessionId, graphId)
DesktopPreload->>RuntimeHost: agent.graph.query with graphId
RuntimeHost->>AgentGraphCoordinator: getGraphSnapshot
AgentGraphCoordinator-->>AgentGraphPanel: read-only historical snapshot
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Qodo is busy workingCheck back in a few minutes. Qodo's code review agents are on it. |
PR Summary by QodoAdd Runtime Host-backed browsing of historical Agent Graph runs
AI Description
Diagram
High-Level Assessment
Files changed (30)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts (1)
216-218: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the dismiss control behavior instead of its CSS class.
Line 218 couples the test to
.maka-agent-graph-dismiss. Assert that no control with the dismiss accessible label exists. This keeps the test valid if styling classes change.As per path instructions, flag tests that assert implementation details.
Source: Path instructions
packages/storage/src/sqlite-session-metadata-store.ts (1)
2760-2764: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReuse the declared epoch page types.
AgentGraphEpochPageRequestandAgentGraphEpochPagealready exist inpackages/core/src/agent-graph-epoch.tsand type this method onAgentGraphEpochStore. The inline shape here restates that contract and can drift from it without a type error.♻️ Proposed change
- async listAgentGraphEpochPage(request: { - rootSessionId: string; - beforeEpoch?: number; - limit: number; - }): Promise<{ epochs: AgentGraphEpochBinding[]; nextBeforeEpoch: number | null }> { + async listAgentGraphEpochPage( + request: AgentGraphEpochPageRequest, + ): Promise<AgentGraphEpochPage> {
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: fda2e3a7-beeb-448f-9efe-9fd21d497c66
📒 Files selected for processing (30)
apps/desktop/src/main/__tests__/agent-graph-panel.test.tsapps/desktop/src/main/__tests__/runtime-host-session-domains-ipc-main.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-session-domains-ipc-main.tsapps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/agent-graph-panel.tsxapps/desktop/src/renderer/app-shell.tsxapps/desktop/src/renderer/locales/shell-copy.tsapps/desktop/src/renderer/styles/agent-graph.csspackages/cli/src/__tests__/pi-tui-runner.test.tspackages/cli/src/pi-tui-runner.tspackages/cli/src/runtime-host-tui-command.tspackages/cli/src/runtime-host-tui-context.tspackages/core/src/__tests__/graph-command.test.tspackages/core/src/agent-graph-epoch.tspackages/core/src/graph-command.tspackages/runtime-host/src/__tests__/agent-graph-coordinator.test.tspackages/runtime-host/src/__tests__/agent-graph-protocol.test.tspackages/runtime-host/src/__tests__/agent-graph-reader.test.tspackages/runtime-host/src/__tests__/agent-graph-two-client-uds.test.tspackages/runtime-host/src/client/agent-graph-reader.tspackages/runtime-host/src/client/index.tspackages/runtime-host/src/protocol/agent-graph.tspackages/runtime-host/src/protocol/operations.tspackages/runtime-host/src/server/agent-graph-coordinator.tspackages/runtime/src/__tests__/stream-graph-coordinator.test.tspackages/runtime/src/stream-graph-coordinator.tspackages/storage/src/__tests__/agent-graph-epochs.test.tspackages/storage/src/sqlite-session-metadata-store.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 0 remain after this review.
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
I reviewed exact head a520f2c1128b64413d21508708d6c17376b1a6f4, including the full Storage → Runtime → Runtime Host → Desktop/TUI path, current CI, and all prior feedback.
The ownership model is sound. Durable epoch bindings remain in Storage, Runtime Host is the sole historical graph reader, exact graph reads are checked against the root Session, and clients consume bounded read-only projections rather than opening a parallel persistence path. The earlier atomic page/current observation, large-history truncation, selected-history eviction, and TUI shutdown findings are fixed on this head.
One P2 remains in the Desktop refresh lifecycle.
refresh() advances refreshGeneration before checking whether a request is already in flight. A normal graphs:changed notification therefore both queues a follow-up refresh and invalidates the current response, even though the session and selected graph have not changed. Runtime activity can publish these notifications continuously; if one arrives during each listEpochs() / getSnapshot() sequence, every valid snapshot is discarded and the panel stays blank or stale until the graph becomes quiet.
A coalesced notification should request one follow-up read while allowing the current read for the same session and selection to commit. Advance the fence only when the session or selected graph changes, or rely on an explicit selected-graph identity token. Please add a deferred-request regression that emits an invalidation while refresh is in flight.
Non-blocking simplification: the unused RuntimeHostCapabilityProviderService export can be deleted. If historical operator drill-down is not shipping in this slice, its currently unconsumed public protocol expansion can also wait for a real consumer.
The PR should remain one vertical slice; splitting it would only create an unusable half-contract.
Disclosure: Codex performed the read-only source, lifecycle, protocol, test, CI, and prior-feedback analysis. The human contributor remains responsible for verifying the finding and deciding whether to merge.
中文
历史 graph 的 owner 方向正确，既有四条反馈已修。当前 P2 是普通 invalidation 与用户 selection 共用 generation fence：活跃 graph 可能让每轮 snapshot 都被丢弃，直到 graph 静默。普通通知应只排队 follow-up。
|
The P2 refresh-lifecycle finding is fixed on The refresh scheduling now lives in a pure
Regression coverage in On the non-blocking note: |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for making historical graph runs inspectable across Host, CLI, and Desktop. The durable epoch directory plus explicit graphId reads is the right shape: Host remains the read authority, historical snapshots are immutable/read-only, and the UI follows current until the user intentionally pins history. Independent reviewer passes and a separate read-only DeepSeek V4 Flash high-effort pass reviewed this head.
The remaining defects come from splitting selection authority between refs and the last rendered snapshot. Immediately after choosing history, the rendered snapshot still represents current, so the current-only Stop action remains live and can stop a different graph than the one the user just selected. Separately, a disposed read can still clear the loading state owned by a newer session/effect.
From first principles, one state tuple—{rootSessionId, selectedGraphId, requestGeneration}—should own both the rendered snapshot and all actions. Update the selected graph synchronously on selection, disable current-only actions until the matching snapshot commits, and let only the current request generation settle loading/error. That is simpler than keeping parallel ref/snapshot interpretations and closes both races at the same seam.
I did not run a local test suite; this review is based on the latest head, focused static tracing, current green CI, independent @reviewer passes, and a read-only ollama-cloud/deepseek-v4-flash high-effort review. AI-assisted review; I verified the UI race paths.
中文评论
感谢把历史 Graph run 的浏览能力贯通到 Host、CLI 和 Desktop。持久 epoch directory 配合显式 graphId 读取的方向正确：Host 保持读取 authority，历史 snapshot 只读，UI 默认跟随 current，用户选择后才固定历史。
剩余问题来自 selection authority 被拆在 ref 和最后一次渲染的 snapshot 中。选择历史后，界面暂时仍把 current snapshot 当成所选对象，因此“停止 Graph”仍可点击，并会停止另一个 graph；同时旧 session 已 dispose 的请求仍可清除新 session 的 loading。
更符合第一性原理和奥卡姆剃刀的方案，是用同一个 {rootSessionId, selectedGraphId, requestGeneration} 状态元组同时拥有 snapshot 与 action：选择时立即更新 selected graph，匹配 snapshot 提交前禁用 current-only action，只有当前 generation 能结算 loading/error。这样在一个 seam 同时消除两条竞态。
本次未运行本地测试；结论来自最新 head 静态追踪、当前绿色 CI、独立 reviewer 和一次只读的 DeepSeek V4 Flash high-effort 审查。AI 辅助审查；我已人工复核 UI 竞态路径。
83371c4 to
6212969
Compare
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Historical graph browsing belongs in the Runtime Host graph authority, and this PR generally preserves that boundary: pages are bounded, snapshots are read-only, and Desktop selection fencing avoids acting on historical state. Two validation gaps remain at the new epoch-list operation. The simplest fix is to validate the root once at the authority boundary and make cursor normalization/rejection happen before a page reaches the strict client assertion.
test_workspaces and storybook are still running on the current head.
AI-assisted review disclosure: Codex verified the findings against the current head and traced Runtime session ownership, epoch storage transactions, Host projection, protocol assertions, and client connection failure handling. Two independent reviewer-agent passes and an OpenCode Go DeepSeek V4 Flash (high) adversarial pass were used as inputs. No local tests were run.
中文复核
历史 Graph 浏览应由 Runtime Host 的 Graph 权威负责，这个 PR 基本守住了边界：分页有界、历史 snapshot 只读，Desktop selection fencing 也避免了对历史状态执行操作。新 epoch-list operation 还缺两个校验：root Session 所有权/存在性，以及越过 current epoch 的 cursor。最小修复是在 authority boundary 统一验证 root，并在 page 进入严格 client assertion 前规范化或拒绝 cursor。当前 test_workspaces 与 storybook 仍在运行。
本次为 AI 辅助审查：Codex 在最新 head 上追踪 Runtime Session ownership、epoch storage transaction、Host projection、协议 assertion 与 client connection failure；另使用两次独立 reviewer 及一次 OpenCode Go DeepSeek V4 Flash（high）对抗审查。未运行本地测试。
me2seeks
left a comment
me2seeks
left a comment
There was a problem hiding this comment.
Publishing the verified inline fix responses.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The core design is well chosen: Runtime Host remains the only historical Graph reader, epoch paging and the current marker share one storage observation, graph ownership is checked against the root Session, and Desktop/TUI history is read-only. The exact head is green and merges cleanly with current main.\n\nTwo UI authority races and one avoidable refresh cost remain below. The first-principles fix is to bind mutable UI actions and commits to the graph/session identity the user actually saw, while keeping the Host operation as the final authority. No broader redesign is needed.\n\nReviewed with Codex using three independent reviewer agents and OpenCode Go DeepSeek V4 Flash (high); I verified the exact head, current-main merge tree, protocol/storage boundaries, UI refresh paths, focused tests, and live CI.\n\n
中文
\n\n核心设计方向正确：Runtime Host 仍是历史 Graph 的唯一读取权威；epoch 分页与 current marker 来自同一次 storage observation；graph ownership 按 root Session 校验；Desktop/TUI 历史保持只读。当前精确 head 检查全绿，也可与最新 main 干净合并。\n\n目前还剩下面两个 UI 权威竞态和一个可避免的刷新成本。按第一性原理，修复应把可变 UI action/commit 绑定到用户实际看到的 graph/session identity，并继续让 Host operation 做最终权威校验；不需要更大的重构。\n\n本次由 Codex 配合三个独立 reviewer agent，以及 OpenCode Go DeepSeek V4 Flash（high）审查；我核验了精确 head、当前 main 的 merge tree、协议/存储边界、UI 刷新路径、聚焦测试和实时 CI。\n\n
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
apps/desktop/src/main/__tests__/agent-graph-panel.test.ts (1)
138-155: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
createdAtin the fake directory is inverted relative toepoch.The sort puts the current graph first, so the current epoch receives
createdAt: 1and the oldest epoch receives the largestcreatedAt. The real directory orders newest last-created highest. The panel ignorescreatedAttoday, so no test fails now. A future panel change that sorts or filters bycreatedAtwould pass against this inverted fixture.Derive
createdAtfrom the epoch number to keep the fixture faithful.♻️ Proposed fixture correction
epochs: entries.map((entry, index) => ({ epoch: entries.length - index, graphId: entry.graphId, - createdAt: index + 1, + createdAt: entries.length - index, current: currentGraphIds.get(sessionId) === entry.graphId, })),apps/desktop/src/main/__tests__/runtime-host-session-domains-ipc-main.test.ts (1)
730-730: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd
listCurrentAgentGraphEpochsto the unavailable defaults.
domainClientexists so that any un-stubbed domain operation fails withUnexpected domain operation.listCurrentAgentGraphEpochsis part of the client contract but is missing here. The object is castas DomainClient, so the gap is invisible to the type checker. A future test that invokesgraphs:listCurrentEpochswithout an override gets aTypeErroron an undefined call instead of the intended failure message.Disposition: optional.
♻️ Proposed fix
listAgentGraphEpochs: unavailable, + listCurrentAgentGraphEpochs: unavailable, listTasks: unavailable,
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c14e66cf-1057-48bb-b25d-6a23cac4ae24
📒 Files selected for processing (22)
apps/desktop/src/main/__tests__/agent-graph-panel.test.tsapps/desktop/src/main/__tests__/runtime-host-session-domains-ipc-main.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-session-domains-ipc-main.tsapps/desktop/src/preload/bridge-contract.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/agent-graph-panel.tsxapps/desktop/src/renderer/app-shell.tsxapps/desktop/src/renderer/locales/shell-copy.tspackages/cli/src/__tests__/pi-tui-runner.test.tspackages/cli/src/pi-tui-runner.tspackages/cli/src/runtime-host-tui-command.tspackages/runtime-host/src/__tests__/agent-graph-coordinator.test.tspackages/runtime-host/src/__tests__/agent-graph-protocol.test.tspackages/runtime-host/src/__tests__/agent-graph-two-client-uds.test.tspackages/runtime-host/src/__tests__/execution-composition.test.tspackages/runtime-host/src/protocol/agent-graph.tspackages/runtime-host/src/server/agent-graph-coordinator.tspackages/runtime-host/src/server/execution-composition.tspackages/runtime/src/__tests__/stream-graph-coordinator.test.tspackages/runtime/src/stream-graph-coordinator.tspackages/storage/src/sqlite-session-metadata-store.ts
Included review availability: Your plan provides up to 3 included reviews per hour; 2 remain after this review.
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for the substantial follow-up work—the storage/runtime/host/UI authority boundaries are now coherent, and the earlier epoch, refresh-fence, expected-graph, shutdown, and truncation issues are addressed.
Two things prevent approval of this exact head. First, GitHub currently reports it as conflicting with current main, which now includes #2992's cross-epoch graph-result semantics across many of the same files; the rebase must preserve those semantics and then receive a fresh review. Second, I left one new P2 inline for Stop state leaking across a root-session switch.
Please rebase onto current main, retain #2992's result authority, fence/reset the Stop request state by session/request generation, and add the focused A→B deferred-stop regression. Then rerun CI and request review on the new head.
AI-assisted review disclosure: two Codex reviewers independently reviewed exact head d30cd39, current merge state, prior fixes, and the Desktop session-switch path.
Fence stale session reads before they can mutate panel selection, reuse cached epoch history for activity refreshes, and bind stop requests to the graph rendered by the operator UI. Generated-by: Maka
Keep current graph actions mounted during background reads, fence historical TUI results after shutdown, and cover both lifecycle boundaries. Generated-by: Maka
Keep deferred stop completion and failure state scoped to the root session and request that initiated them. Generated-by: Maka
d30cd39 to
2445683
Compare
|
Can We have a before after comparison screenshot? |
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
The current patch correctly binds the protocol request to expectedGraphId, keeps Runtime Host as the sole graph authority, and prevents a deferred stop result from leaking across root sessions. Two remaining identity gaps need to be closed before approval: the server can still stop a newly rolled graph supervisor, and renderer feedback is scoped only to the session rather than the selected graph.
E2E, Storybook, and the root test job are still running/queued.
AI-assisted review disclosure: Codex reviewed exact head 2445683, the current three-commit PR patch against its live base, current CI, and resolved thread history, with two independent reviewer passes.
中文说明
当前 patch 已把 stop 请求绑定到 expectedGraphId，Runtime Host 仍是唯一权威，也修复了跨 root session 的异步状态泄漏。但身份约束还没有贯穿完整路径：server 在第一次校验后仍可能重新解析并停止刚 rollover 的新 graph supervisor；renderer 的 pending/error 也只按 session 隔离，会泄漏到同 session 的新 graph 或历史视图。相关 CI 仍在运行。
|
Added the requested Before/After comparison to the PR body. It contrasts the current-only panel with a selected historical run, including the read-only label and absence of the current-run Stop action. |
Generated-by: Maka
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Approved on exact head d42bf37ee6986ae4375d35651df08e5c05e1cbf1.
Historical Graph browsing stays on the existing Runtime Host authority. Epoch pagination, root-Session ownership, cursor validation, graph-identity checks, and Desktop/TUI stale-result fences keep historical selection read-only and prevent it from acting on the current graph. The current-page refresh path also avoids rereading complete history on every activity notification.
No remaining P0-P2 findings. Current CI is green, mergeability is CLEAN, and there are no unresolved review threads. The requested before/after screenshots are present, accessible, and match the current read-only historical state. The PR body and all four substantive commits carry consistent Maka AI-use disclosure.
AI-assisted review disclosure: Codex reviewed the exact-head Host/CLI/Desktop paths, concurrency and identity fences, focused tests, live CI, thread state, screenshots, and provenance metadata. No external model was used. Astro-Han authorized this review campaign.
中文说明
历史 Graph 浏览继续复用 Runtime Host 单一权威；分页 epoch、root Session、cursor、graph identity，以及 Desktop/TUI 的异步 stale-result fence 都完整，历史选择保持只读，也不会误操作当前 Graph。
当前没有剩余 P0-P2，CI 全绿、CLEAN、无未解决线程。before/after 截图与当前实现一致，PR body 和 4 个 commit 的 AI disclosure 也完整。
|
Human merge confirmation: Astro-Han reviewed the final diff at |
2 participants
Summary
/graph historyto the TUI without starting an agent TurnThe Runtime Host remains the only historical graph reader: clients request an epoch directory and an exact graph snapshot, while the Host validates that the requested graph belongs to the same root Session. Desktop follows the current epoch by default and fences stale refreshes when the user switches runs.
This is the historical-read slice of #2588 and builds on #2969.
Refs #2588
Ready for review.
Verification
npm run build:testnpm run typecheckgit diff --checkChecklist
Does this PR entail a change in behavior?
中文说明
摘要
/graph history，查看历史不会启动 Agent Turn客户端不会直接读取 SQLite。Runtime Host 会验证指定 graph 确实属于同一个 root Session，再返回 epoch 目录和精确历史快照。Desktop 默认跟随当前 epoch，并通过 generation fence 防止较慢的旧请求覆盖用户刚选择的新页面。
这是 #2588 的历史读取部分，建立在 #2969 的 epoch 生命周期边界之上。
已转为 Ready，请求 review。
AI disclosure / AI 披露
This PR was implemented by Codex under me2seeks’s direction and review. / 本 PR 由 Codex 在 me2seeks 的指导与审核下完成。
Visual evidence
Representative Desktop capture using the PR copy and graph states. The historical selection is explicitly read-only and does not expose the current-run Stop action.