refactor(runtime-host)!: remove unused Session catalog filters (#3071) - #3165
Conversation
|
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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review. 📝 WalkthroughSummaryThis PR removes unused The change updates the protocol, coordinator, Desktop IPC, runtime client, and SQLite query construction. Pagination cursors no longer carry filter data. The compatibility epoch increases to 24. Source of truthThis PR extends the existing Session metadata and catalog paths. It does not create a parallel path. Labels, archive state, and flag state remain readable, mutable, and included in catalog projections. Migration v26 removes label replica tables, triggers, indexes, and synchronization code. Existing Session metadata remains in the primary metadata store. Scope and complexityThe change is the smallest coherent solution for removing the unused wire filters. It removes related validation, cursor state, SQL predicates, replica maintenance, and obsolete tests while preserving child-Session filtering and metadata behavior. The compatibility epoch and schema migration are necessary because the wire contract and SQLite schema change. Tests that only verify removed filtered queries were deleted or simplified. Regression coverage remains for unfiltered pagination, revision handling, rejected filters and legacy cursors, metadata preservation, label and flag state, archive state, and catalog projections. Further deletion should not remove coverage for strict filter rejection, legacy cursor rejection, pagination, or migration metadata preservation. Risks and validation
Review-relevant risksThe current diff has a material public wire-contract and release-compatibility effect through the removed filters and compatibility epoch change. Material changes in this area require independent human review under repository policy. The current diff has a material data-migration effect through schema migration v26 and removal of label replica tables. Material changes in this area require independent human review under repository policy. The person performing the merge reviews the final diff. A maintainer makes the final determination. WalkthroughThe session catalog no longer supports archive, flag, or label filters. Storage removes label projections and related maintenance. Runtime Host cursors and protocol inputs are simplified. Desktop reads the complete catalog and retains local subagent-parent filtering. ChangesSession Catalog Filter Removal
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to The change removes unused catalog filters while preserving local subagent filtering and readable session status and labels; stale filter requests are rejected rather than silently misapplied. No actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
PR Summary by QodoRemove unused Session catalog filters from runtime host contract
AI Description
Diagram
High-Level Assessment
Files changed (15)
|
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9651c409-3cae-41d5-a608-fe9170152f75
📒 Files selected for processing (15)
apps/desktop/src/main/__tests__/runtime-host-client-operations.test.tsapps/desktop/src/main/runtime-host-client.tsapps/desktop/src/main/runtime-host-session-catalog-ipc-main.tspackages/core/src/runtime-inputs.tspackages/runtime-host/src/__tests__/session-catalog-coordinator.test.tspackages/runtime-host/src/__tests__/session-catalog-protocol.test.tspackages/runtime-host/src/__tests__/session-catalog-two-client-uds.test.tspackages/runtime-host/src/client/catalog-reader.tspackages/runtime-host/src/protocol/index.tspackages/runtime-host/src/protocol/session-catalog.tspackages/runtime-host/src/server/session-catalog-coordinator.tspackages/storage/src/__tests__/sqlite-session-metadata-store.test.tspackages/storage/src/sqlite-session-catalog-query.tspackages/storage/src/sqlite-session-metadata-schema.tspackages/storage/src/sqlite-session-metadata-store.ts
💤 Files with no reviewable changes (4)
- packages/core/src/runtime-inputs.ts
- packages/runtime-host/src/tests/session-catalog-two-client-uds.test.ts
- packages/runtime-host/src/client/catalog-reader.ts
- packages/storage/src/sqlite-session-metadata-store.ts
Included review availability: Your plan includes up to 3 reviews per rolling hour; 1 remains after this review.
There was a problem hiding this comment.
Pull request overview
Removes unused Session catalog filtering capabilities (archived/flagged/label) across the Runtime Host wire protocol, coordinator/client call sites, Desktop IPC passthrough, and storage query/schema machinery, while preserving subagentParentSessionId support via local filtering and storage-level predicates.
Changes:
- Runtime Host protocol/coordinator/client:
session.catalog.queryis now filter-free; paging cursors no longer include filters; compatibility epoch bumped. - Storage: removes label replica/projection tables and associated triggers/indexes; removes filter predicate construction for archived/flagged/label; adds schema v25 migration and updates tests.
- Desktop: runtime-host session listing no longer forwards removed filters;
subagentParentSessionIdremains supported via local filtering.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/storage/src/sqlite-session-metadata-store.ts | Removes label-replica maintenance and archived/flagged/label filtering in list predicates. |
| packages/storage/src/sqlite-session-metadata-schema.ts | Bumps schema to v25; migration drops label projection tables/triggers and related indexes; adjusts catalog update trigger. |
| packages/storage/src/sqlite-session-catalog-query.ts | Simplifies catalog page query to remove archived/flagged/label filters and label-join ordering. |
| packages/storage/src/tests/sqlite-session-metadata-store.test.ts | Updates tests to assert removed tables and to validate label/flag/archive readability without server-side filtering. |
| packages/runtime-host/src/server/session-catalog-coordinator.ts | Removes filter canonicalization/continuation matching; removes filter from cursor encoding/decoding and page generation. |
| packages/runtime-host/src/protocol/session-catalog.ts | Removes SessionCatalogFilter from the wire contract and decoder requirements. |
| packages/runtime-host/src/protocol/index.ts | Bumps RUNTIME_HOST_COMPATIBILITY_EPOCH (22 → 23). |
| packages/runtime-host/src/client/catalog-reader.ts | Makes readRuntimeHostSessions filter-free to match protocol changes. |
| packages/runtime-host/src/tests/session-catalog-two-client-uds.test.ts | Removes coverage that relied on filtered session catalog queries. |
| packages/runtime-host/src/tests/session-catalog-protocol.test.ts | Adds coverage ensuring filtered inputs are rejected by the protocol decoder. |
| packages/runtime-host/src/tests/session-catalog-coordinator.test.ts | Adds coverage rejecting legacy cursors that carry filters. |
| packages/core/src/runtime-inputs.ts | Removes archived/flagged/labelSlug from SessionListFilter, preserving subagentParentSessionId. |
| apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts | Stops forwarding removed filters to Runtime Host; keeps local subagentParentSessionId filtering. |
| apps/desktop/src/main/runtime-host-client.ts | Updates Desktop runtime-host client to list sessions without filters. |
| apps/desktop/src/main/tests/runtime-host-client-operations.test.ts | Updates expectations for filter-free session.catalog.query requests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Code Review by Qodo🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0)
Great, no issues found!Qodo reviewed your code and found no material issues that require reviewTip of the day💡 Did you know, you can route each action level your way: inline, summary, both, or drop |
31d4899 to
d7f6477
Compare
|
@coderabbitai please re-review. The inline concern is addressed by the latest commit: |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --stat --oneline d7f647722
git show --format=fuller --find-renames --find-copies -- d7f647722 -- \
apps/desktop packages/runtime-host | sed -n '1,500p'
printf '\n--- sessions:list definitions and tests ---\n'
rg -n -C 5 --glob '*.{ts,tsx}' \
'(sessions:list|isArchived|isFlagged|labelSlug)' \
apps/desktop packages/runtime-host | sed -n '1,800p'Length of output: 99274
|
d7f6477 to
3e9b484
Compare
3e9b484 to
41c86b7
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Recent commits on this PR branch ---'
git log --oneline -8
printf '%s\n' '--- Diff from first parent of HEAD ---'
git diff --stat HEAD^ HEAD
git diff --find-renames HEAD^ HEAD -- apps/desktop packages/runtime-host | sed -n '1,900p'
printf '%s\n' '--- Candidate files ---'
fd -t f -e ts . apps/desktop packages/runtime-host \
| rg '(runtime-host-session-catalog-ipc-main|session-catalog.*test|two-client-uds|runtime-host-client-operations)'
printf '%s\n' '--- IPC filter validation and tests ---'
rg -n -C 8 --glob '*.{ts,tsx}' \
'(sessions:list|isArchived|isFlagged|labelSlug|subagentParentSessionId)' \
apps/desktop/src/main packages/runtime-host/src/__tests__ \
| sed -n '1,1200p'Length of output: 149005 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Storage module outlines ---'
ast-grep outline packages/storage/src/sqlite-session-metadata-schema.ts --items all
ast-grep outline packages/storage/src/sqlite-session-catalog-query.ts --items all
ast-grep outline packages/storage/src/sqlite-session-metadata-store.ts --items all
printf '%s\n' '--- Schema migration and catalog query diff ---'
git diff HEAD^ HEAD -- \
packages/storage/src/sqlite-session-metadata-schema.ts \
packages/storage/src/sqlite-session-catalog-query.ts \
packages/storage/src/sqlite-session-metadata-store.ts \
packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts \
| sed -n '1,1400p'
printf '%s\n' '--- SessionListFilter definitions and list call sites ---'
rg -n -C 5 --glob '*.{ts,tsx}' \
'(SessionListFilter|listSessions\s*\(|listCatalogPage\s*\()' \
packages apps \
| sed -n '1,1400p'Length of output: 140832 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- Schema bootstrap and migration executor ---'
sed -n '20,260p' packages/storage/src/sqlite-session-metadata-schema.ts
sed -n '930,1020p' packages/storage/src/sqlite-session-metadata-schema.ts
printf '%s\n' '--- Catalog and label schema definitions before migration 25 ---'
rg -n -C 10 \
'(session_catalog_projection|session_catalog_after_update|session_catalog_label_after_|session_metadata_labels|session_catalog_label_projection)' \
packages/storage/src/sqlite-session-metadata-schema.ts \
| sed -n '1,1000p'
printf '%s\n' '--- Migration test setup and migration-specific assertions ---'
rg -n -C 12 \
'(schemaVersion|migrat|SQLITE_SESSION_METADATA_SCHEMA_VERSION|session_metadata_labels|session_catalog_label_projection)' \
packages/storage/src/__tests__/sqlite-session-metadata-store.test.ts \
| sed -n '1,1000p'
printf '%s\n' '--- Preload forwarding behavior ---'
sed -n '640,680p' apps/desktop/src/preload/preload.tsLength of output: 51159
The Runtime Host protocol also rejects I found no new issues in the reviewed changes. I did not execute the test suite. ✏️ Learnings added
You are interacting with an AI system. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 16 out of 16 changed files in this pull request and generated no new comments.
Suppressed comments (1)
apps/desktop/src/main/runtime-host-session-catalog-ipc-main.ts:272
normalizeSessionListFilterreadsrecord.subagentParentSessionIdwithout an own-property check. BecauseObject.keys(record)ignores inherited properties, a prototype-polluted object could smugglesubagentParentSessionIdvia the prototype chain and bypass the key whitelist.
const record = value as Record<string, unknown>;
if (Object.keys(record).some((key) => key !== 'subagentParentSessionId')) {
throw new Error('Invalid Session list filter keys');
}
return {
|
Thanks for the cleanup — the zero-consumer claim holds up (I re-verified on the PR head: no renderer/CLI/TUI caller passes these filters; renderer filters the projection locally via Conclusion: PASS with one P2 (coordination, not a code defect). P2 — schema v25 is now taken: #3159 merged first and owns v25; this PR and #3074 must renumber. #3159 ( P3 (optional): the base schema still contains the full label-copy DDL (migrations 1 and 16 create AI-assisted review disclosure: this review was produced with AI assistance (two pi review subagents on 中文摘要（AI 辅助审查）结论：PASS（1 个 P2，合并协调类，非代码缺陷）。零消费者结论成立（renderer/CLI/TUI 无调用方传 filter，renderer 本地过滤 projection），删除彻底（协议类型、coordinator 游标编解码、client、IPC、storage 谓词、replaceLabels、failpoint 同步删除，无孤儿 import），breaking change 处理正确（epoch 22→23 双向握手严格拒绝、requireExactRecord 拒绝 filter 键、旧游标 exact-keys 拒绝、IPC 边界显式报错而非静默返回未过滤目录），持久化数据不受影响。P2：与 #3074（同为 OPEN、同改 schema v25/epoch/coordinator/protocol）撞车——先后合入会强制冲突，若保留两个 [25,...] 条目会静默丢弃第一个迁移（已跑过 25 的库永不执行第二个迁移），epoch 只生效一次。迁移编号与 epoch 是全局单调资源，合并前需协调（后合者重编号 26 + epoch 顺延），合并后跑 v24→v25 升级测试。P3：基础 schema 仍保留 label 副本 DDL（append-only 正确但描述"tables removed"仅运行时成立）、sessions:list(null) 从静默未过滤变抛错（有意收紧未记录）、PR 缺 AI 披露与 Conventional Commits trailer。 |
41c86b7 to
01fa427
Compare
|
Thanks for the coordination check. Resolved on the current head (rebased onto main at 2de6d6d):
P3 notes: historical label DDL remains in migrations 1 and 16 by append-only design while the runtime schema is clean; strict |
2e2d132 to
8b1fd7f
Compare
|
The e2e failure is not related to this PR: both failing tests are known flakes already tracked upstream.
Neither test touches the Session catalog filter paths changed here ( |
|
Update: the re-run passed. e2e is green (3m27s) and all other required checks (typecheck, test_runtime_host, test_workspaces, windows lanes) pass on the current head. The earlier failure was the known flake. |
|
LGTM. Could you state the AI assistance part and add it on both commit and PR body? |
移除 Session catalog 协议、coordinator、Desktop IPC 与 storage 中未使用的 isArchived/isFlagged/labelSlug 过滤能力，保留 subagentParentSessionId 子会话过滤；新增 schema v26 迁移，清理 label 查询副本表及 archive/flag 过滤索引。labels、isFlagged、isArchived 的读写与 catalog 投影行为保持不变。 Desktop 的 sessions:list 现在只接受 subagentParentSessionId，并对 isArchived/isFlagged/labelSlug 等未知键显式报错，避免旧调用方静默拿到未过滤目录。 BREAKING CHANGE: session.catalog.query 不再接受 isArchived/isFlagged/labelSlug 过滤参数，兼容性 epoch 提升至 24。 当前 main 的兼容性 epoch 为 23、schema 为 v25；本提交对应 schema v26 / epoch 24。 Closes apache#3071 Generated-by: Codex
8b1fd7f to
cf7f679
Compare
|
@Astro-Han done. The PR body now includes the standard AI use section (Codex, with tool and scope) and the commit carries |
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
`goal.arm` is a new wire operation, so a client that has it and a Host that does not are no longer safely interoperable. Without a new epoch the pair completes the handshake, the client offers Set Goal, and the first arm reaches an older decoder as an unknown operation — a protocol error that tears the connection down instead of refusing the pair up front and routing the user to the Host upgrade path. The epoch is the boundary the repository already uses for this: apache#3145 advanced it for a new operation and apache#3165 advanced it merely for removing catalog filters. Handshake fixtures name deliberately stale epochs to exercise rejection, so they stay as they are. Generated-by: Claude Code
Closes #3071
Summary
Removes the unused
isArchived/isFlagged/labelSlugfilters from the Session catalog wire contract, coordinator, Desktop IPC passthrough, and storage query construction.subagentParentSessionIdfiltering is preserved.Changes
session.catalog.querylist_start/list_continue no longer accept filters; pagination cursors no longer carry a filter;RUNTIME_HOST_COMPATIBILITY_EPOCHbumped 22 -> 24 (renumbered after fix(session): migrate legacy session statuses #3159 took epoch 23).readRuntimeHostSessionsandDesktopRuntimeHostClient.listSessionsare filter-free.sessions:liststill supportssubagentParentSessionIdvia local filtering. The IPC boundary now strictly accepts only that key and rejects staleisArchived/isFlagged/labelSlugfilter keys instead of silently returning the unfiltered catalog.session_metadata_labelsandsession_catalog_label_projectiontables, their triggers/indexes, andreplaceLabelsmaintenance are removed; archive/flag filter indexes dropped; schema v26 migration added on top of #3159s v25 migration.isFlagged,isArchivedremain readable/mutable and are still projected in the catalog.Verification
sessions:listboundary.AI use
Select exactly one:
Tool(s) and scope: Codex (OpenAI) — implementation, tests, and schema migration for removing the unused Session catalog filters; the contributor reviewed the output and owns the final result. The commit carries
Generated-by: Codex.Breaking change
A protocol client can no longer ask Runtime Host to filter the catalog by archive state, flag state, or label; clients must read the catalog and filter locally. No shipped client depends on the removed capability.