Universal Search Palette#709
Conversation
…ic ranking, terminal chunking Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p main + ade serve daemon + preload bridge Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… enqueue Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…palette Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…sor-chat classification, unlimited backfill enumeration, primary-lane branch dedup) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…se guard, torn-tail safety, title-tier candidate union, shared host wiring factory, palette search module extraction Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…eads, totals consistency on title union Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tests, docs feature page, CLI help/README parity Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
@copilot review but do not make fixes |
|
Warning Review limit reached
Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds universal search across desktop and ADE CLI, including shared search contracts, indexing/query logic, runtime wiring, CLI/TUI integration, desktop command-palette support, and docs. ChangesUniversal search feature
Estimated code review effort: 5 (Critical) | ~120 minutes Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Review complete (no fixes applied). I found one meaningful issue:
Repro example: If you want, I can follow up with a surgical fix in a separate pass. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
…palette-527927bf # Conflicts: # docs/ARCHITECTURE.md
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
apps/desktop/src/main/services/search/searchService.ts (2)
1225-1300: 🧹 Nitpick | 🔵 TrivialOffset-based pagination is not isolated from concurrent background re-indexing.
query()recomputes the full ranked candidate list on every call and pages via a plain offset. Since ingestion runs on a debounced background queue that can insert/update docs between two page fetches for the same UI search session, a user paging through results while chat/terminal activity is being indexed could see duplicate or skipped items across pages. This is a reasonable trade-off for a local, single-user index, but worth being aware of if "next page" correctness during active sessions becomes a support issue — a keyset-based cursor (e.g.(tier, bm25/updatedAt, docId)) would be more resilient but is a larger change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/search/searchService.ts` around lines 1225 - 1300, The pagination in query() uses a plain offset cursor over a freshly re-ranked result set, so concurrent re-indexing can cause duplicates or skipped items between pages. If exact next-page stability matters for searchService.ts, switch the cursor logic in decodeCursor/encodeCursor and the query() paging flow to a keyset-based cursor using stable sort keys from rankCandidates (for example tier, score or updatedAt, and docId) instead of offset.
377-397: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUse
run().lastInsertRowidinstead of a follow-up SELECT.Node's
StatementSync.run()already returns{ changes, lastInsertRowid }, so the extraSELECT id FROM docs WHERE doc_id = ?after the INSERT is an avoidable round-trip on a hot ingestion path (backfill of large chat/terminal histories calls this many times).⚡ Proposed refactor
- db.prepare( + const inserted = db.prepare( `INSERT INTO docs (doc_id, kind, lane_id, lane_name, session_id, title, rank_title, snippet_source, deep_link, updated_at) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ).run( doc.docId, doc.kind, doc.laneId, doc.laneName, doc.sessionId, doc.title, doc.rankTitle, doc.snippetSource, doc.deepLink, doc.updatedAt ); - const inserted = db.prepare("SELECT id FROM docs WHERE doc_id = ?").get(doc.docId) as { id: number }; db.prepare("INSERT INTO docs_fts (rowid, rank_title, body) VALUES (?, ?, ?)").run( - inserted.id, + inserted.lastInsertRowid, doc.rankTitle ?? "", clampBody(doc.body) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/search/searchService.ts` around lines 377 - 397, The docs insertion path in searchService should avoid the extra lookup after inserting into docs. Update the code that performs the INSERT and then queries by docId so it uses the result from StatementSync.run() and reads lastInsertRowid directly, then pass that rowid into the docs_fts insert. Keep the change localized around the docs insert/fts insert flow in searchService to preserve the existing behavior while removing the follow-up SELECT.apps/desktop/src/main/services/search/searchService.test.ts (1)
52-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider extracting the duplicated transcript-writing test helpers.
writeChatLine/writeTerminalOutputare re-declared identically across threedescribeblocks in this file. A shared test-utils module would reduce duplication as more search-service test suites are added.Also applies to: 321-328
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/search/searchService.test.ts` around lines 52 - 65, The transcript-writing helpers are duplicated across multiple search service test blocks, so extract `writeChatLine` and `writeTerminalOutput` into a shared test utility and reuse them from each `describe` in `searchService.test.ts`. Keep the helper behavior the same, but centralize the filesystem setup and append logic so future search-service tests can import the same helpers instead of redeclaring them.apps/desktop/src/main/services/search/searchIndexDb.ts (1)
66-74: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winRecreate path can stamp a mismatched schema as "current" if file removal silently fails.
If
tryRemoveDbFilescan't actually delete the old db file (e.g. locked on Windows),create()re-runsCREATE TABLE/INDEX IF NOT EXISTSagainst the same, still-mismatched file and then unconditionally writes the newschemaVersionintometa— leaving stale/incompatible table structure marked as healthy. Consider havingcreate()explicitlyDROP TABLE/INDEX IF EXISTSbefore recreating, rather than relying solely on file deletion succeeding.♻️ Proposed defense-in-depth fix
const create = (): DatabaseSyncType => { const db = openAt(dbPath); + db.exec(` + DROP TABLE IF EXISTS docs_fts; + DROP TABLE IF EXISTS docs; + DROP TABLE IF EXISTS sources; + DROP TABLE IF EXISTS meta; + `); db.exec(DDL); db.prepare("INSERT OR REPLACE INTO meta (key, value) VALUES ('schemaVersion', ?)").run( String(SEARCH_INDEX_SCHEMA_VERSION) ); return db; };Also applies to: 106-134
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/main/services/search/searchIndexDb.ts` around lines 66 - 74, The recreate flow can leave an old SQLite file in place and then mark it current with a new schemaVersion, so update create() to actively reset the schema instead of relying on tryRemoveDbFiles succeeding. In the create() path around the existing CREATE TABLE/INDEX setup, first DROP TABLE/INDEX IF EXISTS for the affected objects, then recreate them and only after that write the new value into meta. Use tryRemoveDbFiles only as a best-effort cleanup, not as the only safeguard against a mismatched schema.apps/ade-cli/src/cli.ts (1)
5976-5983: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
--rebuildalways exits 0, even when the rebuild didn't start.Unlike the query path (which sets
exitCodeFromResultto fail on empty results), the--rebuildbranch has noexitCodeFromResult, so aSearchRebuildResultof{ started: false }(e.g. non-CTO caller, or already running) still produces a0exit code. Given the feature's "script-friendly" exit-code pitch, callers doingade search --rebuild && ...can't detect a rejected/no-op rebuild.♻️ Suggested fix
if (readFlag(args, ["--rebuild", "--rebuild-index"])) { return { kind: "execute", label: "search rebuild", formatter: "search-status", steps: [actionStep("result", "search", "rebuildIndex", {})], + exitCodeFromResult: (result) => + isRecord(result) && result.started === false ? 1 : 0, }; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/ade-cli/src/cli.ts` around lines 5976 - 5983, The --rebuild branch in cli.ts always returns a successful execute action even when rebuildIndex does not start, so add an exitCodeFromResult mapping for the search rebuild path in the same place that builds the action object. Use the SearchRebuildResult from actionStep("result", "search", "rebuildIndex", {}) and make it return a non-zero exit code when started is false, matching the script-friendly behavior already used in the query path.apps/desktop/src/renderer/components/app/CommandPalette.tsx (1)
2221-2270: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEntity "visible/show-more" slicing is computed twice (here and in
useUniversalSearch'sflatEntities).This render block recomputes
expanded/visible/showMore/hiddenCountfromentitySectionsindependently of the identical computation already done inflatEntities(commandPaletteSearch.tsx). They currently agree, butactivateFlat/keyboard selection rely onflatEntities's ordering matching this render's flat index exactly — any future edit to one copy without the other would silently desync selection from what's displayed. Consider havinguseUniversalSearchreturn pre-sliced, render-ready rows (e.g. addvisibleRows/showMore/hiddenCountontoEntitySection) so both the index count and the render consume one source of truth.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/desktop/src/renderer/components/app/CommandPalette.tsx` around lines 2221 - 2270, The entity section rendering in CommandPalette is duplicating the visible/show-more slicing logic already computed in useUniversalSearch/flatEntities, which can cause the displayed rows and flat selection indices to drift apart. Update the search data model so EntitySection (or the useUniversalSearch result) carries pre-sliced, render-ready fields like visibleRows, showMore, and hiddenCount, and make this render loop consume those values directly. Keep activateFlat, flatEntities, and SearchResultRow/ShowMoreRow ordering sourced from the same single source of truth.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 5960-5967: `buildSearchPlan` already handles the `--actions` flag,
but `HELP_BY_COMMAND.search` does not advertise it. Update the search help text
to mention `--actions`, or align the search command with the existing
service-command pattern by switching to the positional `actions --text`
convention; make sure the documented usage matches the behavior in
`buildSearchPlan` and related search command helpers.
In `@apps/desktop/src/main/main.ts`:
- Around line 4643-4647: Move the search service teardown in the shutdown flow
so it happens after the event-producing disposals; in the main cleanup function
in main.ts, keep ctx.ptyService?.disposeAll(), ctx.processService?.disposeAll(),
and ctx.agentChatService?.disposeAll() ahead of ctx.searchService?.dispose(),
and ensure searchServiceHolder is cleared as part of that final cleanup so no
later events can reach an already-disposed search service.
In `@apps/desktop/src/main/services/search/searchIndexDb.ts`:
- Around line 103-147: The openSearchIndexDb initialization path always executes
the FTS5 DDL, so on runtimes where node:sqlite lacks SQLITE_ENABLE_FTS5 the
create/open retry loop in openSearchIndexDb and create will fail repeatedly.
Update the search index setup to detect FTS5 support before running DDL, and
skip or disable search-index creation when the capability is unavailable;
alternatively switch to a SQLite build that includes FTS5. Keep the fallback
logic in openSearchIndexDb from retrying the same unsupported CREATE VIRTUAL
TABLE path.
In `@apps/desktop/src/main/services/search/searchServiceWiring.ts`:
- Around line 142-162: The `dispose()` implementation in
`createProjectSearchService` is leaving the `sessionService.onChanged` and
`agentChatService.subscribeToEvents` listeners attached, so each project switch
leaks handlers that keep calling a disposed search service. Capture the
unsubscribe handles returned by those two registrations in
`searchServiceWiring.ts`, store them alongside the `backfillTimer`, and invoke
them inside the returned `dispose()` before calling `searchService.dispose()`.
In `@apps/desktop/src/renderer/components/app/CommandPalette.tsx`:
- Around line 1122-1138: Guard the chat/terminal branch in CommandPalette so it
does not navigate when item.sessionId is missing. In the switch case handling
"chat" and "terminal", check sessionId before dispatching
"ade:work:select-session" and calling navigate; if it is falsy, skip both
actions or return early so /work?sessionId= is never created. Use the existing
sessionId, item.laneId, and navigate logic in this branch to place the guard
cleanly.
In `@apps/desktop/src/renderer/components/app/commandPaletteSearch.tsx`:
- Around line 361-378: The “Show N more” label in the command palette section
preview is using the backend’s total count instead of the number of rows
actually available in `section.rows`, so the displayed hidden count can
overpromise. Update the `flatEntities` memo in `commandPaletteSearch.tsx` to
derive `hiddenCount` from the fetched/visible data for each `section.kind` and
clamp it to what is actually present in `section.rows` (and the preview limit),
so expanding a section only advertises rows that can really be revealed without
another fetch.
---
Nitpick comments:
In `@apps/ade-cli/src/cli.ts`:
- Around line 5976-5983: The --rebuild branch in cli.ts always returns a
successful execute action even when rebuildIndex does not start, so add an
exitCodeFromResult mapping for the search rebuild path in the same place that
builds the action object. Use the SearchRebuildResult from actionStep("result",
"search", "rebuildIndex", {}) and make it return a non-zero exit code when
started is false, matching the script-friendly behavior already used in the
query path.
In `@apps/desktop/src/main/services/search/searchIndexDb.ts`:
- Around line 66-74: The recreate flow can leave an old SQLite file in place and
then mark it current with a new schemaVersion, so update create() to actively
reset the schema instead of relying on tryRemoveDbFiles succeeding. In the
create() path around the existing CREATE TABLE/INDEX setup, first DROP
TABLE/INDEX IF EXISTS for the affected objects, then recreate them and only
after that write the new value into meta. Use tryRemoveDbFiles only as a
best-effort cleanup, not as the only safeguard against a mismatched schema.
In `@apps/desktop/src/main/services/search/searchService.test.ts`:
- Around line 52-65: The transcript-writing helpers are duplicated across
multiple search service test blocks, so extract `writeChatLine` and
`writeTerminalOutput` into a shared test utility and reuse them from each
`describe` in `searchService.test.ts`. Keep the helper behavior the same, but
centralize the filesystem setup and append logic so future search-service tests
can import the same helpers instead of redeclaring them.
In `@apps/desktop/src/main/services/search/searchService.ts`:
- Around line 1225-1300: The pagination in query() uses a plain offset cursor
over a freshly re-ranked result set, so concurrent re-indexing can cause
duplicates or skipped items between pages. If exact next-page stability matters
for searchService.ts, switch the cursor logic in decodeCursor/encodeCursor and
the query() paging flow to a keyset-based cursor using stable sort keys from
rankCandidates (for example tier, score or updatedAt, and docId) instead of
offset.
- Around line 377-397: The docs insertion path in searchService should avoid the
extra lookup after inserting into docs. Update the code that performs the INSERT
and then queries by docId so it uses the result from StatementSync.run() and
reads lastInsertRowid directly, then pass that rowid into the docs_fts insert.
Keep the change localized around the docs insert/fts insert flow in
searchService to preserve the existing behavior while removing the follow-up
SELECT.
In `@apps/desktop/src/renderer/components/app/CommandPalette.tsx`:
- Around line 2221-2270: The entity section rendering in CommandPalette is
duplicating the visible/show-more slicing logic already computed in
useUniversalSearch/flatEntities, which can cause the displayed rows and flat
selection indices to drift apart. Update the search data model so EntitySection
(or the useUniversalSearch result) carries pre-sliced, render-ready fields like
visibleRows, showMore, and hiddenCount, and make this render loop consume those
values directly. Keep activateFlat, flatEntities, and
SearchResultRow/ShowMoreRow ordering sourced from the same single source of
truth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b3e9da64-e212-4f2d-be47-4a61506365f3
⛔ Files ignored due to path filters (7)
AGENTS.mdis excluded by!*.mddocs/ARCHITECTURE.mdis excluded by!docs/**docs/PRD.mdis excluded by!docs/**docs/README.mdis excluded by!docs/**docs/features/chat/README.mdis excluded by!docs/**docs/features/search/README.mdis excluded by!docs/**docs/features/terminals-and-sessions/README.mdis excluded by!docs/**
📒 Files selected for processing (29)
apps/ade-cli/README.mdapps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/ade-cli/src/bootstrap.tsapps/ade-cli/src/cli.test.tsapps/ade-cli/src/cli.tsapps/ade-cli/src/tuiClient/app.tsxapps/desktop/resources/ade-cli-help.txtapps/desktop/resources/agent-skills/ade-search/SKILL.mdapps/desktop/scripts/regen-ade-cli-help.cjsapps/desktop/src/main/main.tsapps/desktop/src/main/services/adeActions/registry.test.tsapps/desktop/src/main/services/adeActions/registry.tsapps/desktop/src/main/services/ipc/registerIpc.tsapps/desktop/src/main/services/search/searchIndexDb.tsapps/desktop/src/main/services/search/searchQueryParser.test.tsapps/desktop/src/main/services/search/searchQueryParser.tsapps/desktop/src/main/services/search/searchRanking.tsapps/desktop/src/main/services/search/searchService.test.tsapps/desktop/src/main/services/search/searchService.tsapps/desktop/src/main/services/search/searchServiceWiring.tsapps/desktop/src/main/services/search/terminalChunking.tsapps/desktop/src/preload/global.d.tsapps/desktop/src/preload/preload.tsapps/desktop/src/renderer/components/app/CommandPalette.tsxapps/desktop/src/renderer/components/app/commandPaletteSearch.tsxapps/desktop/src/shared/adeCliGuidance.tsapps/desktop/src/shared/types/index.tsapps/desktop/src/shared/types/search.ts
| sessionService.onChanged((event) => { | ||
| searchService.notifySessionChanged( | ||
| event.sessionId, | ||
| event.reason === "deleted" ? "deleted" : "meta-updated" | ||
| ); | ||
| }); | ||
| args.agentChatService?.subscribeToEvents?.((envelope) => { | ||
| searchService.notifyChatEvent(envelope.sessionId); | ||
| }); | ||
|
|
||
| const backfillTimer = setTimeout(() => searchService.startBackfill(), args.backfillDelayMs); | ||
| backfillTimer.unref?.(); | ||
|
|
||
| return { | ||
| ...searchService, | ||
| dispose: () => { | ||
| clearTimeout(backfillTimer); | ||
| searchService.dispose(); | ||
| } | ||
| }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Listener leak: dispose() never unsubscribes session/chat event hooks.
sessionService.onChanged and agentChatService.subscribeToEvents are typed to return unknown (implying an unsubscribe handle) but the return values are discarded. dispose() only clears the backfill timer and calls searchService.dispose() — the outer subscriptions stay wired. Since createProjectSearchService runs again on every project switch (not just app restart), each switch leaves the previous listeners attached, invoking methods on an already-disposed search service on subsequent session/chat events.
🔧 Proposed fix
- sessionService.onChanged((event) => {
- searchService.notifySessionChanged(
- event.sessionId,
- event.reason === "deleted" ? "deleted" : "meta-updated"
- );
- });
- args.agentChatService?.subscribeToEvents?.((envelope) => {
- searchService.notifyChatEvent(envelope.sessionId);
- });
+ const unsubscribeSessionChanged = sessionService.onChanged((event) => {
+ searchService.notifySessionChanged(
+ event.sessionId,
+ event.reason === "deleted" ? "deleted" : "meta-updated"
+ );
+ });
+ const unsubscribeChatEvents = args.agentChatService?.subscribeToEvents?.((envelope) => {
+ searchService.notifyChatEvent(envelope.sessionId);
+ });
const backfillTimer = setTimeout(() => searchService.startBackfill(), args.backfillDelayMs);
backfillTimer.unref?.();
return {
...searchService,
dispose: () => {
clearTimeout(backfillTimer);
+ if (typeof unsubscribeSessionChanged === "function") unsubscribeSessionChanged();
+ if (typeof unsubscribeChatEvents === "function") unsubscribeChatEvents();
searchService.dispose();
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| sessionService.onChanged((event) => { | |
| searchService.notifySessionChanged( | |
| event.sessionId, | |
| event.reason === "deleted" ? "deleted" : "meta-updated" | |
| ); | |
| }); | |
| args.agentChatService?.subscribeToEvents?.((envelope) => { | |
| searchService.notifyChatEvent(envelope.sessionId); | |
| }); | |
| const backfillTimer = setTimeout(() => searchService.startBackfill(), args.backfillDelayMs); | |
| backfillTimer.unref?.(); | |
| return { | |
| ...searchService, | |
| dispose: () => { | |
| clearTimeout(backfillTimer); | |
| searchService.dispose(); | |
| } | |
| }; | |
| } | |
| const unsubscribeSessionChanged = sessionService.onChanged((event) => { | |
| searchService.notifySessionChanged( | |
| event.sessionId, | |
| event.reason === "deleted" ? "deleted" : "meta-updated" | |
| ); | |
| }); | |
| const unsubscribeChatEvents = args.agentChatService?.subscribeToEvents?.((envelope) => { | |
| searchService.notifyChatEvent(envelope.sessionId); | |
| }); | |
| const backfillTimer = setTimeout(() => searchService.startBackfill(), args.backfillDelayMs); | |
| backfillTimer.unref?.(); | |
| return { | |
| ...searchService, | |
| dispose: () => { | |
| clearTimeout(backfillTimer); | |
| if (typeof unsubscribeSessionChanged === "function") unsubscribeSessionChanged(); | |
| if (typeof unsubscribeChatEvents === "function") unsubscribeChatEvents(); | |
| searchService.dispose(); | |
| } | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/main/services/search/searchServiceWiring.ts` around lines
142 - 162, The `dispose()` implementation in `createProjectSearchService` is
leaving the `sessionService.onChanged` and `agentChatService.subscribeToEvents`
listeners attached, so each project switch leaks handlers that keep calling a
disposed search service. Capture the unsubscribe handles returned by those two
registrations in `searchServiceWiring.ts`, store them alongside the
`backfillTimer`, and invoke them inside the returned `dispose()` before calling
`searchService.dispose()`.
| case "chat": | ||
| case "terminal": { | ||
| const sessionId = item.sessionId ?? ""; | ||
| // The Work tab stays mounted (keep-alive), so its select-session | ||
| // listener focuses the target; the navigate switches the visible tab. | ||
| window.dispatchEvent( | ||
| new CustomEvent("ade:work:select-session", { | ||
| detail: { sessionId, laneId: item.laneId ?? undefined }, | ||
| }), | ||
| ); | ||
| navigate( | ||
| `/work?sessionId=${encodeURIComponent(sessionId)}${ | ||
| item.laneId ? `&laneId=${encodeURIComponent(item.laneId)}` : "" | ||
| }`, | ||
| ); | ||
| break; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard against a missing sessionId before navigating.
If item.sessionId is falsy, sessionId defaults to "", producing /work?sessionId= — a degenerate URL that won't focus any session.
🔧 Suggested guard
case "chat":
case "terminal": {
- const sessionId = item.sessionId ?? "";
+ const sessionId = item.sessionId;
+ if (!sessionId) break;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| case "chat": | |
| case "terminal": { | |
| const sessionId = item.sessionId ?? ""; | |
| // The Work tab stays mounted (keep-alive), so its select-session | |
| // listener focuses the target; the navigate switches the visible tab. | |
| window.dispatchEvent( | |
| new CustomEvent("ade:work:select-session", { | |
| detail: { sessionId, laneId: item.laneId ?? undefined }, | |
| }), | |
| ); | |
| navigate( | |
| `/work?sessionId=${encodeURIComponent(sessionId)}${ | |
| item.laneId ? `&laneId=${encodeURIComponent(item.laneId)}` : "" | |
| }`, | |
| ); | |
| break; | |
| } | |
| case "chat": | |
| case "terminal": { | |
| const sessionId = item.sessionId; | |
| if (!sessionId) break; | |
| // The Work tab stays mounted (keep-alive), so its select-session | |
| // listener focuses the target; the navigate switches the visible tab. | |
| window.dispatchEvent( | |
| new CustomEvent("ade:work:select-session", { | |
| detail: { sessionId, laneId: item.laneId ?? undefined }, | |
| }), | |
| ); | |
| navigate( | |
| `/work?sessionId=${encodeURIComponent(sessionId)}${ | |
| item.laneId ? `&laneId=${encodeURIComponent(item.laneId)}` : "" | |
| }`, | |
| ); | |
| break; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/app/CommandPalette.tsx` around lines
1122 - 1138, Guard the chat/terminal branch in CommandPalette so it does not
navigate when item.sessionId is missing. In the switch case handling "chat" and
"terminal", check sessionId before dispatching "ade:work:select-session" and
calling navigate; if it is falsy, skip both actions or return early so
/work?sessionId= is never created. Use the existing sessionId, item.laneId, and
navigate logic in this branch to place the guard cleanly.
| const flatEntities = useMemo<FlatEntity[]>(() => { | ||
| const out: FlatEntity[] = []; | ||
| for (const section of sections) { | ||
| const expanded = expandedKinds.has(section.kind); | ||
| const visible = expanded | ||
| ? section.rows | ||
| : section.rows.slice(0, ENTITY_SECTION_PREVIEW); | ||
| for (const item of visible) out.push({ type: "result", item }); | ||
| if (!expanded && section.rows.length > ENTITY_SECTION_PREVIEW) { | ||
| out.push({ | ||
| type: "showMore", | ||
| kind: section.kind, | ||
| hiddenCount: section.total - ENTITY_SECTION_PREVIEW, | ||
| }); | ||
| } | ||
| } | ||
| return out; | ||
| }, [sections, expandedKinds]); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
"Show N more" count can exceed what's actually available to render.
hiddenCount is derived from section.total (backend's true per-kind total), but visible/section.rows are capped by the shared SEARCH_QUERY_LIMIT across all kinds. When a kind's true total exceeds what fit in that shared page, expanding the section can reveal fewer new rows than the "Show N more" label promised, since expand doesn't trigger an additional fetch.
🔧 Suggested fix: clamp hiddenCount to what's actually fetched
- if (!expanded && section.rows.length > ENTITY_SECTION_PREVIEW) {
- out.push({
- type: "showMore",
- kind: section.kind,
- hiddenCount: section.total - ENTITY_SECTION_PREVIEW,
- });
- }
+ if (!expanded && section.rows.length > ENTITY_SECTION_PREVIEW) {
+ out.push({
+ type: "showMore",
+ kind: section.kind,
+ hiddenCount: Math.min(section.total, section.rows.length) - ENTITY_SECTION_PREVIEW,
+ });
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const flatEntities = useMemo<FlatEntity[]>(() => { | |
| const out: FlatEntity[] = []; | |
| for (const section of sections) { | |
| const expanded = expandedKinds.has(section.kind); | |
| const visible = expanded | |
| ? section.rows | |
| : section.rows.slice(0, ENTITY_SECTION_PREVIEW); | |
| for (const item of visible) out.push({ type: "result", item }); | |
| if (!expanded && section.rows.length > ENTITY_SECTION_PREVIEW) { | |
| out.push({ | |
| type: "showMore", | |
| kind: section.kind, | |
| hiddenCount: section.total - ENTITY_SECTION_PREVIEW, | |
| }); | |
| } | |
| } | |
| return out; | |
| }, [sections, expandedKinds]); | |
| const flatEntities = useMemo<FlatEntity[]>(() => { | |
| const out: FlatEntity[] = []; | |
| for (const section of sections) { | |
| const expanded = expandedKinds.has(section.kind); | |
| const visible = expanded | |
| ? section.rows | |
| : section.rows.slice(0, ENTITY_SECTION_PREVIEW); | |
| for (const item of visible) out.push({ type: "result", item }); | |
| if (!expanded && section.rows.length > ENTITY_SECTION_PREVIEW) { | |
| out.push({ | |
| type: "showMore", | |
| kind: section.kind, | |
| hiddenCount: Math.min(section.total, section.rows.length) - ENTITY_SECTION_PREVIEW, | |
| }); | |
| } | |
| } | |
| return out; | |
| }, [sections, expandedKinds]); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/desktop/src/renderer/components/app/commandPaletteSearch.tsx` around
lines 361 - 378, The “Show N more” label in the command palette section preview
is using the backend’s total count instead of the number of rows actually
available in `section.rows`, so the displayed hidden count can overpromise.
Update the `flatEntities` memo in `commandPaletteSearch.tsx` to derive
`hiddenCount` from the fetched/visible data for each `section.kind` and clamp it
to what is actually present in `section.rows` (and the preview limit), so
expanding a section only advertises rows that can really be revealed without
another fetch.
… filters, listener unsubscribe on dispose, dispose ordering, palette sessionId guard + show-more clamp, --actions help Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 120708db0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| removeSessionDocs(sessionId); | ||
| return; | ||
| } | ||
| if (isChatToolType(session.toolType)) return; |
There was a problem hiding this comment.
When a legacy chat row has a chat JSONL transcript but a non-chat toolType such as other, processChatSession treats it as chat via isChatSession, but this guard lets the same session continue through terminal indexing. Because the terminal meta doc is written before checking for a .log, those legacy chats show up under kind:terminal and inflate terminal results even when no terminal transcript exists.
Useful? React with 👍 / 👎.
|
|
||
| const queryArgs: JsonObject = { query }; | ||
| if (kinds && kinds.length) queryArgs.kinds = kinds; | ||
| if (laneId) queryArgs.laneId = laneId; |
There was a problem hiding this comment.
The CLI help advertises --lane as accepting an id or name, but this stores the raw flag value in laneId; resolveLaneFilter returns args.laneId immediately and only resolves names for inline lane:<name> filters. As a result, ade search "auth" --lane fix-login filters FTS rows by d.lane_id = 'fix-login' and searches files with workspace id fix-login, so normal lane names return no lane-scoped matches unless they happen to equal the internal id.
Useful? React with 👍 / 👎.
| const root = project?.rootPath ?? null; | ||
| if (relative && root) { | ||
| const separator = root.includes("\\") ? "\\" : "/"; | ||
| const absolute = `${root}${ | ||
| root.endsWith(separator) ? "" : separator | ||
| }${relative}`; |
There was a problem hiding this comment.
For command-palette searches that include lane:<name>, the backend delegates file search to that lane's workspace, but activation re-roots the repo-relative result under the primary project.rootPath. In worktree lanes this opens the primary checkout's file (or a missing path) instead of the file that was actually matched in the lane; route the Files tab with the lane/workspace context rather than constructing a primary-root external path.
Useful? React with 👍 / 👎.
…als, lane-name resolution for --lane, lane-scoped file activation opens the lane worktree, since: RangeError guard Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b632559247
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| filters.push("(d.kind NOT IN ('chat', 'terminal') OR d.session_id = ?)"); | ||
| filterParams.push(scopeChatSessionId); |
There was a problem hiding this comment.
When a non-CTO caller has a chatSessionId, this predicate only keeps terminal docs whose indexed session_id equals that chat id. Attached terminals store their own terminal row id as session_id during indexing, while the owning chat is held on TerminalSessionSummary.chatSessionId, and direct terminal reads authorize that owner path. In chats that launch separate shell/app-control terminals, scoped search.query therefore hides the caller's own terminal output; index and filter the owning chat id as well.
Useful? React with 👍 / 👎.
| if (event.type === "prs-updated") { | ||
| for (const pr of event.prs) searchServiceHolder.current?.notifyPrChanged(pr.id); |
There was a problem hiding this comment.
This only refreshes PR search docs for prs-updated events, but the background polling path in this same file reports changed PR rows through createPrPollingService's onPullRequestsChanged callback rather than emitPrEvent. After the initial backfill, PR titles/bodies/comments updated by polling remain stale in universal search until some separate webhook or manual PR event happens; call notifyPrChanged for changedPrs in that polling callback too.
Useful? React with 👍 / 👎.
…hat's scoped caller (schema v2 owner_session_id), PR polling notifies search in both hosts Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55a5e9b633
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // an unbound agent/orchestrator/evaluator — which chat.readTranscript | ||
| // would deny outright — gets no session content at all. | ||
| if (session.identity.role === "external") return searchArgs; | ||
| return { ...searchArgs, callerScope: { excludeSessionContent: true } }; |
There was a problem hiding this comment.
For a normal ade search launched from a shell, the CLI initializes as role agent by default and usually has no ADE_CHAT_SESSION_ID, so this branch injects excludeSessionContent and strips all chat and terminal hits. That makes the documented user CLI search miss the primary indexed content unless users know to pass --role external; distinguish unbound human CLI callers from unbound agents or have the search command initialize as an external/user caller.
Useful? React with 👍 / 👎.
| delegated.push(...(await delegatedLaneCandidates(parsed, matchAll))); | ||
| } | ||
| if (kinds.includes("file")) { | ||
| delegated.push(...(await delegatedFileCandidates(parsed, matchAll, laneId))); |
There was a problem hiding this comment.
When the query includes since:..., the FTS, lane, and artifact paths apply parsed.sinceIso, but this still delegates file search and returns file hits with updatedAt: "". A default query like ade search "TODO since:7d" can therefore be filled with file matches that are not known to be recent, violating the recency filter; skip or timestamp file results when parsed.sinceIso is set.
Useful? React with 👍 / 👎.
… search (plain ade defaults to role agent; bound workers stay scoped), since: filter omits timestamp-less file results Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/ade-cli/src/adeRpcServer.ts`:
- Around line 2350-2358: The unbound search behavior in adeRpcServer now allows
whole-project results, but the shared callerScope contract still documents
unbound non-external agent roles as excluding session content. Update the shared
type/comment in search.ts to match the new unbound behavior, or change the
searchArgs handling in the callerScope path to keep excludeSessionContent
enforced; keep the contract and the implementation aligned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9945c2f0-f96d-40c2-b337-cc0d75827a61
📒 Files selected for processing (4)
apps/ade-cli/src/adeRpcServer.test.tsapps/ade-cli/src/adeRpcServer.tsapps/desktop/src/main/services/search/searchService.test.tsapps/desktop/src/main/services/search/searchService.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/main/services/search/searchService.test.ts
- apps/desktop/src/main/services/search/searchService.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78b445c359
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| rebuildIndex(): SearchRebuildResult { | ||
| if (disposed) return { started: false }; | ||
| queue.clear(); | ||
| clearSearchIndex(ensureDb().db); |
There was a problem hiding this comment.
When search.rebuildIndex runs while the background workChain is already processing a source, this clears the DB immediately instead of waiting for that in-flight processor to finish. A chat/terminal processor that read an old cursor can then resume after the clear, write only the later transcript chunk, and persist an advanced cursor; the new backfill is chained behind it and will see that cursor, so earlier transcript bytes stay missing from the rebuilt index until another idle rebuild.
Useful? React with 👍 / 👎.
|
|
||
| const startBackfill = (): void => { | ||
| if (backfillStarted || disposed) return; | ||
| backfillStarted = true; |
There was a problem hiding this comment.
If any awaited source list in the delayed startup backfill throws once (for example sessions/lanes/PRs are temporarily unavailable during project startup), backfillStarted remains true after the catch logs the failure. Later startBackfill() calls then return immediately, so any sources not enqueued before the failure are never indexed unless the user explicitly runs a rebuild.
Useful? React with 👍 / 👎.
…exing, backfill retry after startup failure, callerScope contract comment sync Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 223688722c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // --------------------------------------------------------------------- | ||
|
|
||
| const effectiveKinds = (args: SearchQueryArgs, parsed: ParsedSearchQuery): SearchDocKind[] => { | ||
| const fromArgs = (args.kinds ?? []).filter(isSearchDocKind); |
There was a problem hiding this comment.
When search.query is invoked through the ADE action bridge or preload with a malformed kinds array, this line drops every invalid value; if none remain, effectiveKinds falls through to the default all-kind set. For example, ade actions run search.query --input-json '{"query":"secret","kinds":["termnal"]}' returns chat/PR/file/etc. hits instead of an empty or invalid-kind response, so a caller's intended narrowing can silently broaden results. Preserve whether args.kinds was supplied and reject or return no results when any requested kind is not valid.
Useful? React with 👍 / 👎.
| title: session.title, | ||
| rankTitle: null, | ||
| snippetSource: sanitized.slice(0, 240), | ||
| deepLink: withQueryParam(baseLink, "event", seq), |
There was a problem hiding this comment.
These event deep links are advertised as opening the exact chat message, and terminal chunks add the same kind of offset parameter, but the shared ADE deeplink parser only preserves lane for ade://session/... before dispatching a Work navigation target. In the ade search --kind chat --json / ade open <deepLink> flow, the app therefore opens the session but drops the matching message/scrollback position, so the result cannot navigate to the hit. Extend the session deeplink target/navigation payload to carry these parameters or avoid emitting unsupported anchors.
Useful? React with 👍 / 👎.
| laneId, | ||
| laneName: null, | ||
| sessionId: null, | ||
| deepLink: `ade://files?path=${encodeURIComponent(item.path)}`, |
There was a problem hiding this comment.
File search hits return ade://files?..., but the shared ADE deeplink parser/dispatcher only recognizes lane, session, branch/repo, PR, and Linear issue targets. In the CLI/skill flow (ade search --kind file --json followed by ade open <deepLink>), these links parse as unknown_type and do nothing, so file results cannot be opened from their advertised deepLink. Add a supported file target to the deeplink parser/navigation path or return an existing openable route.
Useful? React with 👍 / 👎.
|
Merging via ship loop (force-finalize at iteration cap; ci-pass green on 2236887). Three P2 polish items from the final Codex round are deliberately deferred as follow-ups:
🤖 Generated with Claude Code |
|
All three deferred items are fixed in PR #713: (1) supplied-but-all-invalid |
…main The lane-branch-reset incident left three files rebuilt from a pre-#710 base, which read to git as intentional reverts of relay-everywhere code (Greptile P1s: relay URL missing from handshake, non-host runtime owning the relay). 3-way remerge (base=#709, ours=main, theirs=branch) so main's relay behavior returns while the deeplink additions stay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* search: reject supplied-but-all-invalid kinds arrays instead of broadening A query whose kinds array contains only invalid entries (e.g. ["termnal"]) previously fell back to the default all-kind set — silently broadening the search. Mirror the inline kind:bogus reject: supplied + nothing valid left means empty results. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * deeplinks: session anchors, file target, portable envelopes + resolution ladder (desktop) - Shared contract: session links carry event=<seq> (chat) / offset=<bytes> (terminal) anchors; new file target (repo-relative path + line + lane); new commit/artifact targets; portable envelope (repo/branch/pr/linear query params) on machine-local targets, parsed leniently. - Anchors consumed end-to-end: chat list scrolls to the anchored event (envelope.sequence match, ordinal fallback on full history, bounded older-history paging, highlight); terminal replay scrolls by byte fraction when the whole transcript is loaded; Files editor reveals the anchored line. Search emits envelope.sequence in chat links and canonical ade://file links (index schema 2->3, disposable rebuild). - Resolution ladder for session/lane links: local -> open exactly; repo matches another known project (new project.findForRepo IPC + repoProjectResolver over recents' git configs) -> switch-project card; foreign machine -> fallback-only card (create lane from branch, open PR on GitHub, open Linear issue). InboundDeeplinkModal generalized; no request-access path by design. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * deeplinks(ios+web): router parity, Universal Links, envelope-aware Send-to-Mac, link minting - https /open session links navigate locally on the phone; file/commit/ artifact shapes validate then route to Send-to-Mac; session anchors carried on WorkSessionNavigationRequest (no scroll hook yet on iOS). - Universal Links: applinks:ade-app.dev entitlement, NSUserActivity routing, AASA served from apps/web/.well-known (appID VQ372F39G6.com.ade.ios, /open* and /pair* only) with JSON content-type. - Envelope-aware Swift link builder; lane/session/PR copy-link actions mint https links with envelopes; Send-to-Mac card offers GitHub PR / Linear fallbacks from the envelope. Web /open page describes the new target types. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * deeplinks: commit/artifact dispatch, ade link targets, envelope auto-embed sweep - commit target dispatches to the owning lane (commitSha param) with the same resolution ladder as sessions/lanes; foreign fallback offers the GitHub commit URL. artifact links open the history surface (no focused- artifact mechanism exists yet). Palette commit results deep-route too. - ade link gains file/commit/artifact targets plus mint-time envelope enrichment (lane branch, PR number, Linear issue) with --no-envelope; round-trip re-emit handles all new forms. ade-deeplinks skill doc updated. - Auto-embed: Linear lane cards and desktop copy-link affordances (lane context menu, session copy, TUI Ctrl+Y) now mint envelope-carrying links. Search emits canonical commit/artifact links and repo/branch envelopes on session links via an optional repoSlug dep (index schema 3->4). - Recovered from an out-of-process lane-branch reset that wiped the working tree mid-task: restored A/B content (kinds guard, sequence anchors, canonical file links, palette anchor pass, regression tests) and re-added the named findForRepo types. Backup refs: backup/pre-incident, backup/c-snapshot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(deeplinks): new targets, anchors, portable envelopes, resolution ladder, iOS universal links Also restores the contract-file doc comments and the parser edge-case tests (traversal normalization, malformed anchors, envelope leniency). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * quality: traversal guard on RPC file targets, anchor re-apply, commit-sha retry, envelope/link hygiene Dual-review synthesis fixes: - app/navigate RPC file targets now validated with the shared repo-relative rules (exported isValidRepoRelativePath/isValidCommitSha) + defense-in-depth check in renderer dispatch — an RPC caller could previously smuggle ../../ paths past the URL parser into openExternalPath. - Work URL anchor apply-once guard keyed by session AND event/offset so re-opening the same session at a different anchor re-positions. - Commit deeplink keeps laneId+commitSha through switch-project retries instead of dropping to a bare /lanes. - Lane search results now carry the portable envelope (envelopeForLane). - ade link refuses to mint links the shared parser would reject (round-trip gate). - project.findForRepo IPC rewired through the tested mtime-cached resolver (was an inline reimplementation spawning git per recent project). - Parser branch/pr assembly unified into shared helpers across both URL forms. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: regression pins, folder consolidation, parity residuals - Pin the quality gate: app/navigate file-traversal + malformed-sha rejection (adeRpcServer), ade link round-trip refusal (CLI), lane search envelope (searchService). - Consolidate services/deeplinks to one test file (dispatch contract merged into protocolHandler.test.ts). - Parity residuals: search README deepLink prose, ios-companion universal-link notes, ade-cli README CLI-surface inventory. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ship: dedupe AASA header after main merge Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ship: restore #710 relay code accidentally reverted in bootstrap/cli/main The lane-branch-reset incident left three files rebuilt from a pre-#710 base, which read to git as intentional reverts of relay-everywhere code (Greptile P1s: relay URL missing from handshake, non-host runtime owning the relay). 3-way remerge (base=#709, ours=main, theirs=branch) so main's relay behavior returns while the deeplink additions stay. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ship: address Codex P2s — repo-qualified PR links hand off to Mac on iOS; validate ade link file paths before building - iOS: https pr links carrying a repo param no longer resolve a local PR by bare number (numbers are only unique per repo and the snapshot has no repo identity) — they route to Send-to-Mac where the desktop ladder verifies. - ade link file validates the input path with the shared repo-relative rule BEFORE building: the ade:// path form URL-normalizes dot segments, so the post-build round-trip gate alone would accept ../secret as a link to a different in-repo path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * ship: sweep stale-state dispatch class — latest-dispatcher ref, no root fallback for lane file links - The switch-project card now re-dispatches through a ref that always points at the latest dispatcher, so post-switch retries see the new project's lanes/root instead of the closure captured before switchToPath. - laneById / active-repo / findForRepo comparisons read via refs (same class). - File deeplinks with an explicit lane never degrade to the project root: refresh + bounded wait for the lane list, then open the Files tab bare (or report unhandled during suppressed retries) instead of composing the same relative path in the wrong worktree. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Universal search + global command palette
Makes everything in ADE searchable — lanes, chats including full message content, CLI sessions including terminal scrollback, PRs, commits, branches, files, Linear issues, and proof artifacts — through one deterministic search service, surfaced in the desktop ⌘K palette, an
ade searchCLI command for agents, and theade codeTUI palette.Search service (
apps/desktop/src/main/services/search/).ade/cache/search-index.db— never insideade.db(FTS virtual tables can't be cr-sqlite CRRs, the index must not sync, and a rebuild is just a file drop). Schema-versioned: mismatch or corruption drops and rebuilds.updatedAtdesc then docId; message/chunk docs rank body-only so a chat's title doesn't promote every message. A title-scoped candidate union keeps the ranking spec true even when body matches saturate the candidate window."quoted phrases",kind:/lane:/session:/since:filters.searchaction domainsearch.query/search.indexStatus(all roles) andsearch.rebuildIndex(CTO-only), registered through the daemon action registry and constructed in both desktop main and theade servedaemon via a shared wiring factory — no preload/in-process bypass (runtime-backed null-services bug class). Caller scoping mirrors the chat/terminal read policy: session-bound agents see only their own session's chat/terminal content; unbound agent roles get no session content; user CLI/desktop/CTO keep whole-project search.Surfaces
ade search "<query>" [--kind a,b] [--lane <id|name>] [--limit N] [--cursor c] [--json|--text], plus--status/--rebuild; exit 1 on no results. Newade-searchagent skill + AGENTS.md line + bundled skill advertisement.Verification
🤖 Generated with Claude Code
Summary by CodeRabbit
ade searchacross CLI, desktop command palette, and TUI with coverage for chats, terminals, lanes, PRs, commits/branches, files/artifacts, and Linear.--kind/--lane,--limit/--cursorpagination,--status/--rebuild, and--text/--jsonoutput with ranked snippets and keyboard navigation (including “show more”).ade-searchskill guidance.Greptile Summary
This PR adds a universal search layer across ADE — chat transcripts, terminal scrollback, PRs, commits, branches, lanes, files, Linear issues, and proof artifacts — surfaced through a desktop ⌘K palette, an
ade searchCLI command, and the TUI Ctrl+K palette. The search backend is a schema-versioned FTS5 SQLite cache with a debounced ingestion queue, deterministic ranking, and caller-scoped result filtering.searchService.ts,searchIndexDb.ts,searchQueryParser.ts,searchRanking.ts,terminalChunking.ts): FTS5 index in a disposable.ade/cache/search-index.db; five FTS kinds (chat, terminal, PR, commit, branch) with ANSI-stripped chunking and cursor-resumable ingestion; lanes, files, artifacts, and Linear delegated at query time;callerScopeenforcement mirrors the existing chat/terminal read scoping.ade searchCLI adds--kind/--lane/--limit/--cursorflags and script-friendly exit codes; TUI palette merges server-ranked hits with local matches.search.query/search.indexStatus(all roles) andsearch.rebuildIndex(CTO-only) registered through the shared action registry;scopeSearchAdeActionArgslimits session-bound agents to their own session's content.Confidence Score: 4/5
Safe to merge with one fix: the Show N more count in the palette renders the backend pre-limit total rather than the count of rows actually fetched, so clicking the button can promise hundreds of additional results while only revealing a handful.
The ingestion queue, ranking, scoping, FTS schema, and CLI are well-constructed. One concrete display bug in CommandPalette.tsx makes Show N more advertise a count drawn from totalByKind rather than the fetched page size, which misleads users when a kind has many total matches but few rows in the 60-result response window.
apps/desktop/src/renderer/components/app/CommandPalette.tsx — the hiddenCount computation needs the same Math.min cap that commandPaletteSearch.tsx already applies in flatEntities.
Important Files Changed
hiddenCountusessection.total(backend pre-limit count) instead of capping bysection.rows.length, causing the "Show N more" button to advertise far more results than are actually available.laneIdparameter.parseSinceguards against out-of-range Date values.useUniversalSearchhook,SearchResultRow,ShowMoreRow, and highlight helpers. Generation-guarded debounce is correct;flatEntitiescorrectly capshiddenCountby fetched rows.ade searchcommand with--kind,--lane,--limit,--cursor,--status,--rebuildflags. Exit-code-from-result pattern is clean.scopeSearchAdeActionArgsscoping session-bound callers to their own session's content. Unbound callers intentionally keep whole-project search.connectionRef.currentinside the debounce timer. Session-based deduplication against local items is correct.Reviews (5): Last reviewed commit: "search: round-5 review fixes — serialize..." | Re-trigger Greptile