Repository navigation
fix(search): keep legacy host search and negotiate agent support - #25009
Conversation
Keep older request and reply parsers usable while current peers retain all supported history.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review. 📝 WalkthroughWalkthroughThe changes update session-search agent negotiation and fallback behavior. They add OpenCode 1.x TUI plugin registration and status reporting, plus session identity and launch-token handling for local and relay hooks. Relay build configurations and test bundles use a shared JSONC parser ESM alias. The preview-arm driver can wait for a post-action CSP report. Priority: ⬇️ Low Merge Risk: 🔵 Low · up to An invalid OpenCode TUI configuration can stop plugin installation or refresh before the server plugin is written, leaving the integration absent or stale until the configuration is corrected. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to No authentication bypass or expanded data access was established. However, a new configuration-registration failure can prevent an existing reporter from being installed or updated, leaving reporting components at different versions. Recovery and ownership propagation are not fully established. Retained concerns
Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
021d1d51-f23b-4ba2-8ca4-fded49b213a4
📒 Files selected for processing (13)
docs/reference/remote-wire-compatibility.mdsrc/main/ai-vault-search/session-search-scope-entry-points.test.tssrc/main/ai-vault-search/session-search-scope-service.test.tssrc/main/ai-vault-search/session-search-service-registry.test.tssrc/main/ai-vault-search/session-search-service-registry.tssrc/main/ipc/ai-vault-search.test.tssrc/main/runtime/rpc/methods/ai-vault-search.test.tssrc/shared/ai-vault-search-agent-negotiation.tssrc/shared/ai-vault-search-client.tssrc/shared/ai-vault-search-contract.tssrc/shared/ai-vault-search-qoder-client.test.tstests/e2e/cross-version-wire/qoder-history-search-downgrade.unit.test.tstests/e2e/cross-version-wire/session-search-agent-set-downgrade.unit.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The negotiation itself looks correct, but three unchanged test suites assert the pre-negotiation wire shape and now fail (7 tests). CI will be red until they are updated.
Reviewed changes
- Agent negotiation helper. New
src/shared/ai-vault-search-agent-negotiation.tsdefines the frozenLEGACY_SESSION_SEARCH_AGENTSvocabulary andsessionSearchAgentsForPeer, which intersects a peer's advertised list withAI_VAULT_AGENTSand otherwise falls back to the legacy set (plusqoderwhensupportsQoderHistoryis set). - Contract. Optional
supportedAgents: string[]added to both the search request and the search status schemas, so a peer without the field stays parseable. - Host narrowing.
searchSessionServicefilters requested agents to the peer's set before search; an empty intersection keeps the narrow filter and retrieves under a no-match scope, returning an empty page rather than widening.sessionSearchServiceStatusadvertises the build's full list. - Client negotiation. For any non-IPC request carrying an agent filter, the client reads
searchStatus, narrows the filter to the host's advertised list, and returnsunsupported-agentwhen nothing is shared. Every request now carriessupportsQoderHistory: trueandsupportedAgents: [...AI_VAULT_AGENTS]. - Docs and tests.
remote-wire-compatibility.mdgains a section, and new/updated cross-version controls pin the b49 request and response parsers.
Verified while reviewing: b49 (pinned pre-Jcode) has aiVault.searchStatus, so the client's new unconditional status call does not break it, and its non-strict request/status schemas strip the added fields. b49's unavailable.reason enum lacks unsupported-agent, and the host never emits it (it returns an empty page instead), so old clients stay parseable.
⚠️ Three unchanged suites assert the pre-negotiation wire shape and now fail
The shared client now always appends supportedAgents, and the host now withholds Jcode from peers that do not advertise a list — but three test files that pin the old shape were not updated. I ran them on this branch: src/cli/handlers/search.test.ts (5 failures), src/renderer/src/web/preload-api/web-ai-vault-search-compat.test.ts (1), and src/relay/session-search-transport.test.ts (1). Each uses an exact-argument assertion that no longer matches, so CI cannot go green as-is.
Technical details
# Unchanged tests pin the pre-negotiation search wire shape
## Affected sites
- `src/cli/handlers/search.test.ts:105` and `:164` — `toHaveBeenCalledWith('aiVault.searchSessions', { query, limit, supportsQoderHistory: true })`; the actual call also carries `supportedAgents: [...AI_VAULT_AGENTS]`.
- `src/renderer/src/web/preload-api/web-ai-vault-search-compat.test.ts:21` — `toHaveBeenCalledExactlyOnceWith(... { query, limit, supportsQoderHistory: true })`; same missing `supportedAgents`.
- `src/relay/session-search-transport.test.ts:56` — expects `filters: { agents: AI_VAULT_AGENTS.filter((agent) => agent !== 'qoder') }` (which includes `jcode`); a request without `supportedAgents` is now narrowed to `LEGACY_SESSION_SEARCH_AGENTS`, which drops `jcode` too.
## Required outcome
- The three suites should assert the new wire shape so CI passes.
- The relay case should expect `LEGACY_SESSION_SEARCH_AGENTS` (the intended behavior change: a peer that does not advertise a list cannot receive Jcode), not the old "everything but qoder" set.
## Suggested approach
- Import `LEGACY_SESSION_SEARCH_AGENTS` (and/or `AI_VAULT_AGENTS`) into the two client-transport suites and include `supportedAgents: [...AI_VAULT_AGENTS]` in the expected params.
- In `session-search-transport.test.ts`, replace the inline `AI_VAULT_AGENTS.filter(...)` expectation with `[...LEGACY_SESSION_SEARCH_AGENTS]`.deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Join current per-agent negotiation while retaining empty-peer no-match scope and consent precedence.
Join current per-agent negotiation while retaining empty-peer no-match scope and consent precedence.
nwparker
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
3ff31216-a632-4b0d-973a-98fad8787229
📒 Files selected for processing (49)
config/build-plugins/jsonc-parser-esm.tsconfig/scripts/build-relay.mjsdocs/reference/remote-wire-compatibility.mdsrc/main/agent-hooks/server-opencode-tui-identity.test.tssrc/main/agent-hooks/server/server-ingest-normalization.tssrc/main/agent-hooks/server/server-runtime-env.tssrc/main/ai-vault-search/session-search-agent-compatibility.test.tssrc/main/ai-vault-search/session-search-service-registry.test.tssrc/main/ai-vault-search/session-search-service-registry.tssrc/main/opencode/hook-plugin-legacy-tui-ownership.test.tssrc/main/opencode/hook-service-legacy-acl.test.tssrc/main/opencode/hook-service.test.tssrc/main/opencode/opencode-legacy-plugin-loader-fixture.tssrc/main/opencode/opencode-legacy-tui-fixture.tssrc/main/opencode/status-plugin-delivery-source.tssrc/main/opencode/status-plugin-endpoint-source.tssrc/main/opencode/status-plugin-legacy-tui-source.tssrc/main/opencode/status-plugin-post-source.tssrc/main/opencode/status-plugin-runtime-state-source.tssrc/main/opencode/status-plugin-shared-server-capability.test.tssrc/main/opencode2/status-plugin-setup-source.tssrc/main/opencode2/status-plugin-tui-source.tssrc/main/runtime/rpc/methods/ai-vault-search.test.tssrc/main/ssh/relay-native-dependency-coverage.test.tssrc/relay/agent-hook-endpoint-coordinates.tssrc/relay/agent-hook-opencode-tui-identity.test.tssrc/relay/agent-hook-request.tssrc/relay/agent-hook-server-contract.tssrc/relay/agent-hook-server.tssrc/relay/agent-hook-spool-ingest.tssrc/relay/pty-handler-agent-launch-token.test.tssrc/relay/pty-handler.tssrc/relay/relay-agent-hook-runtime.tssrc/relay/relay-endpoint-credential-publication.test.tssrc/relay/skill-upload-multi-relay.integration.test.tssrc/relay/subprocess.test.tssrc/shared/agent-hook-listener-opencode-tui-identity.test.tssrc/shared/agent-hook-listener-relay-dependency.test.tssrc/shared/agent-hook-listener.tssrc/shared/agent-hook-listener/endpoint-publication.tssrc/shared/agent-hook-listener/opencode-session-registry.tssrc/shared/agent-hook-spool.tssrc/shared/ai-vault-search-client.tssrc/shared/ai-vault-search-status-fallback.test.tssrc/shared/opencode-tui-config-registration.test.tssrc/shared/opencode-tui-config-registration.tssrc/shared/opencode-tui-plugin-install.tstests/e2e/cross-version-wire/qoder-history-search-downgrade.unit.test.tstests/e2e/cross-version-wire/session-search-agent-set-downgrade.unit.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/cross-version-wire/session-search-agent-set-downgrade.unit.test.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.
There was a problem hiding this comment.
Important
CI will be red: the client's new unconditional status probe (ai-vault-search-client.ts:55) breaks src/cli/handlers/search.test.ts "sends filters", whose call mock still answers every method with a search-results envelope.
Reviewed changes
- Adopted main's compatibility policy. The join drops the first commit's parallel
ai-vault-search-agent-negotiation.tshelper and reuses main's existingai-vault-search-agent-compatibility.ts(Qoder/Jcode flags, frozen legacy vocabulary). - Client negotiates every remote agent filter. Any non-IPC request with agent filters now probes
aiVault.searchStatus(newreadSessionSearchStatus), narrows the filter to the peer's advertised list, and returnsunavailable/unsupported-agenton an empty intersection. A missing status method still searches the conservative legacy subset; other status errors propagate. - Host returns an empty page on an empty intersection.
searchSessionServicekeeps the narrow filter, retrieves under the existing no-match scope{kind:'resolved', paths:['']}, and publishes{hits:[], page:{cursor:null,hasMore:false}}instead ofunavailable/unsupported-agent, preservingdisabled/not-readyandwithin-unknownprecedence. - Coverage and docs. Updated CLI/web/relay suites pin the new wire shape; added
ai-vault-search-status-fallback.test.tsand the cross-versionsession-search-agent-set-downgrade.unit.test.ts; documented the negotiation inremote-wire-compatibility.md.
The prior review's one finding (three suites pinning the old wire shape) is now mostly resolved — the CLI/web expectations include the added fields and the relay suite expects the legacy subset — but one CLI case still fails, flagged inline.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The delta since the prior pullfrog review (583650d225) is merge commit f323450345, which brings main up to date and fixes the one CLI fixture the previous review flagged. Production search code is byte-identical to the previously reviewed source.
- CLI
runSearchfixture routes by RPC method. Thecallmock now answersaiVault.searchStatuswith a valid status envelope and every other method with the configured search result, so the unconditional status probe no longer throws aZodError. sends filtersasserts the probe-then-search sequence. It now expects two calls, checks theaiVault.searchStatusprobe ({}) runs first, and still assertsaiVault.searchSessionscarries the filter plussupportedAgents,supportsQoderHistory, andsupportsJcodeHistory.
Verified locally: pnpm test src/cli/handlers/search.test.ts (19/19) and the compatibility + status-fallback suites (32/32). The client thread from the prior review (ai-vault-search-client.ts:55) is resolved.
deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏
Preserve every accepted component postimage and all unowned main entries. Retain the public component ancestry and prior source-qualified proof.
1 participant
ELI5
Search works with older Orca servers even when they do not have the newer search-status method. Current peers keep their supported providers, while older peers receive only filters and results they understand.
What Changed
The client now treats an explicit missing status method as a legacy server and continues the supported search. Authentication, connection, and malformed-response failures still propagate. It uses the existing compatibility policy, including the Qoder and Jcode capabilities already on main.
The host keeps empty provider intersections narrow using its existing no-match scope. Normal consent, readiness, and unknown-workspace responses retain their precedence, and empty results have no next-page cursor. This join retains the newer main implementation and the original PR's valid coverage and ancestry.
Why
Status and search were introduced independently. Treating a missing status probe as missing search incorrectly hid a working service. Negotiating every explicit remote filter also respects an advertised empty or narrower catalog. Reusing the existing policy avoids a second provider list that could drift.
Linked Issue
Addresses the missing-status review in this PR. No additional issue was opened.
Visual Proof
N/A — this corrects backend request compatibility and has no layout or rendered UI change. Before/after production-dispatcher and SQLite controls are retained.
Testing
On macOS, the real RPC dispatcher reproduced failure with only the status method absent and passed after the fix. Actual pinned older request/response parsers, SQLite retrieval, remote error propagation, and runtime/relay/IPC boundaries pass: 215 focused tests and 103 cross-version tests (300 distinct). Node, CLI, and web typechecks, formatting, strict lint, and changed-code gates pass. No fresh native Windows or OpenCode UI claim is made for this contract correction.
Review
Filtered remote searches make one status probe. Unknown-method fallback is narrow and each execution host negotiates separately; this adds no client cache or path interpretation.
Agent skill upstream boundary
Checklist
The branch now includes main
94f5669inb1dcb04. All 16 reviewed file decisions, including two intentional deletions, match the preceding qualified source, and every other main file is preserved. Independent full-source and artifact reviews pass. Fresh Node/CLI/web typechecks pass. The focused checks cover 133 distinct cases across eight files: 115 unaffected cases from the initial run, plus 18 cases rerun after restoring the exact existing release tag in the private test repository. The initial five setup failures are retained. Earlier quality, dispatcher, SQLite and runtime receipts remain tied to their tested sources; fresh public CI covers this composition before merge.