Repository navigation
feat: add batch session metadata lookup - #1408
Merged
Merged
Conversation
slapoguzov
marked this pull request as draft
October 23, 2026 13:36
slapoguzov
changed the title
There was a problem hiding this comment.
🟡 Changes recommended
Targeted lookups can incorrectly populate the shared negative cwd cache, causing subsequent session lists to omit recoverable sessions.
1 open issue
What changed in this PR
Adds _session/batch_get so clients can refresh metadata for pinned and saved sessions without paging through the full session history.
Changes:
- Adds capability negotiation, request validation, and routing for batches of up to 100 session IDs.
- Reuses session-list metadata and row formatting, preserving request order and including archived sessions and worktrees.
- Documents the API and adds lookup, validation, and failure-handling tests.
| File | Description |
|---|---|
src/tests/session-index.test.ts |
Tests negotiation, routing, lookup behavior, and store failures. |
src/session-index/session-index.ts |
Adds targeted transcript lookup and shared candidate collection. |
src/session-index/service.ts |
Validates batch requests and formats response rows. |
src/session-index/project-dirs.ts |
Exports the shared path-existence helper. |
src/session-index/connection.ts |
Advertises and routes the batch extension. |
src/air-extension.ts |
Defines the sessionBatchGet capability. |
src/acp-agent.ts |
Exposes the batch lookup agent method. |
docs/air-extensions.md |
Documents batch semantics and subscription boundaries. |
Review effort: Balanced
Drop the 100-ID limit on `_session/batch_get`: the client is the only caller and chunking does not reduce total work. Look up IDs 100 at a time so the project directories × IDs paths in memory stay bounded for any request size. Copy selection and request order are unchanged.
benbrandt
pushed a commit
that referenced
this pull request
Oct 26, 2026
A foreground fork can stream a copied `tool_result` for the Agent call that launched it. That placeholder completed the call and removed its tracked state, so the real report arriving later was dropped. Ignore only results whose tool ID equals the message's parent tool ID, keeping the launching call open for its actual report. Keep other results in the same message and discard the message-level `tool_use_result` sidecar when its launch result was removed, so it cannot be attributed to a remaining tool result. Regression coverage exercises plain and Air clients, root and nested launch results, and a mixed message containing a genuine nested Agent result. Fixes #1389. Validation: - [Linux CI](https://github.com/agentclientprotocol/claude-agent-acp/actions/runs/40217593917) passes the full suite: 3,304 passed, 32 skipped (57 test files passed, 4 skipped). All PR checks are green. - `npm run check` and `npm run build` pass. - `vitest run src/tests/acp-agent.test.ts --maxWorkers=1`: 712 passed, 10 skipped. The four new regression cases fail on the original code and pass with the fix. - Eight actual-SDK runs against localhost fake gateways complete successfully without model calls: plain and Air foreground forks, native Air child sessions, nested forks with the background option omitted or false, the SDK's fork-inside-fork rejection, a background launch receipt, and history replay. Foreground reports arrive exactly once and no launch-result tracking warning remains. The local full-suite run encountered 19 failures in `session-index.test.ts` and then stopped progressing, so it was interrupted. A standalone recursive-file-watcher probe also emits `EMFILE` on this machine. The successful Linux CI run above provides the complete-suite verification. Review dependencies (updated 26 October): | Related proposal | Relationship to this PR | | --- | --- | | [#1408](#1408) — targeted session lookup | Independent. Both orders merge cleanly onto the checked main and produce the same tree. | | [#1422](#1422) — resumed subagents | Complementary, with no dependency on this PR. It now conflicts with upstream main in `src/acp-agent.ts`; its author needs to resolve that conflict, integrate the [separately shared AIR routing patch](#1422 (comment)) and complete its validation. | <details> <summary>Additional local integration validation — 25 October</summary> A local combination of the recorded main, #1408, #1431, #1422 and the already-reviewed routing patch passed 199 focused foreground/resume/native-subagent checks (647 other cases filtered or skipped), TypeScript build, and two replays of previously captured SDK frames for AIR and standard subagents. The two captured-frame replays were independently repeated. No new model calls were made. The combination is a private test tree, not a published change. It does not replace full CI for #1422 or live Air acceptance. The routing patch needed three-way application against that recorded main because its surrounding source had moved; it applied cleanly that way. The current-main conflict in #1422 means this historical result does not establish today’s combined compatibility. Checked main `739dde3`, #1408 `e3b3aa4`, #1431 `8f96f46`, #1422 `dd3bf2c`. The two orders of #1408 and #1431 produce tree `1716d7e`; the full local test tree is `61c0447`. Refresh this evidence if these inputs change. </details> Co-authored-by: Codex <codex@openai.com>
EugeneTheDev
requested changes
Oct 26, 2026
EugeneTheDev
left a comment
EugeneTheDev
left a comment
Collaborator
There was a problem hiding this comment.
Idea is solid, there are some implementation concerns, please check the comments
Also explain the case check of batch lookups on case-insensitive file systems and describe sessionIds as non-blank, as the parser checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
EugeneTheDev
approved these changes
Oct 26, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A session browser may load only the 50 most recent conversations, while pinned sessions and saved folders refer to much older ones. After a restart, the client still has those IDs but needs their current titles, locations and archive state to rebuild the saved views. Finding a handful of retained sessions by paging through the entire history becomes increasingly expensive as that history grows.
Add
_session/batch_getto retrieve metadata for known session IDs in one request; the client chooses how many IDs to send per request. Successful results use the same row format assession/list, including archived sessions and sessions in worktrees, so the client can merge them with its recent page and apply its usual display filters.The response contains the sessions whose metadata is available, in request order. Other IDs are simply omitted. The client can refresh the rows it receives while keeping its saved references, so pinned and folder views stay independent of the recent-list window.