Not-Expected-Yet/dsh-recall
2
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 7 Wiki Activity

feat: index extra source roots beside the workspace, for the code you write against

Browse Source 
The workspace index cannot answer questions about an installed framework or
SDK, and in plugin development that is where most of the reading goes: 64 of
80 tool calls in one real session went to the harness checkout under
/usr/local/lib/node_modules to look up the API being implemented.

A root is entered whatever it is called, and its dependencies are not: a
path ending in node_modules/... is the normal way to name a package tree,
while a node_modules below the root is somebody else's code. A result
carries the label that says which tree it came from and an absolute
sourcePath, the only form a follow-up read can resolve.

The tool description now also claims the move this replaces - searching
before reading files to find out what a module contains.
This commit is contained in:
NotExpectedYet committed 2026-09-27 07:16:21 +01:00
1 parent bccd112ce5
commit de4d020dcb
8 files changed
+873 -22
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

README.md
+93 -1
View File
@@ -168,7 +168,99 @@ machine.
The tool description steers the model on when *not* to use it: prefer `grep` when
you already know the symbol, `read` when you need the whole file, and do not
follow up with a `read` of a range you were just handed.
follow up with a `read` of a range you were just handed. It also claims the one
move it exists to replace: **reaching for it before reading files to find out
what a module contains**, because that is a search — and one search that warms a
bounded slice of the index costs less than the first round of reads it avoids.
## Extra source roots: indexing the code you write *against*
A workspace index answers questions about the code you are writing. It cannot
answer questions about the library that code is written against — the framework,
the SDK, the installed platform — because none of that is under the workspace.
Measured on one real plugin-development session, **64 of 80 tool calls** went to
the installed harness checkout under `/usr/local/lib/node_modules` to look up the
contract being implemented: *"Read workspace slot contract"*, *"Locate
DirectoryListing definition"*, *"Read UiWorkspace service type"*. That is the only
authoritative copy of the API, and every one of those reads went into the
conversation the expensive way.
```yaml
# your profile's cordis.patch.yml
- id: dsh-recall
name: dsh-recall
config:
sourceRoots:
- /usr/local/lib/node_modules/@deepseek-ai/dsh/node_modules/@deepseek-ai
# or, with a chosen label and a workspace-relative path:
# - vendor=../packages/node_modules/@vendor
```
Registered through the settings form it is a comma-separated list, as every
list-valued setting is.
An entry is a path, optionally prefixed with `name=`:
- an **absolute** path is taken as given — what a deployment's installed tree needs;
- a **relative** path resolves against the workspace, which is what makes a
checked-in configuration portable;
- the **name** defaults to the directory's basename and becomes the first segment
of every result path from that root, so a result says which tree it came from.
### The root is entered; its dependencies are not
A root is usually *a directory of packages*, which is the one shape the workspace
walk handles worst: `node_modules` and friends are skipped at every depth, so the
interesting files are exactly the ones it refuses to enter. So the rule is
asymmetric, and the asymmetry is the feature:
- **the root itself is entered whatever it is called** — a path ending in
`node_modules/...` is the normal way to name a package-manager tree;
- **below the root** the usual skip list applies, so a `node_modules` inside one
of those packages is not indexed. A dependency is somebody else's code, and it
is the same code the root already holds.
Measured against the real harness packages (`@deepseek-ai`): **2,927 files** into
11,926 chunks, a **113 ms** walk, and **zero** indexed paths containing
`node_modules`. A query — *"how does a plugin register a settings section the
client renders"* — returned its top six hits inside `dsh-client-ui-settings*`, the
correct module, at 11 KB of excerpts against 373 KB across the 7 files they came
from.
### How a result from a root is addressed
The paths a search returns are what the caller hands to `read`, which resolves
them against the workspace. A file under an extra root is not under the workspace,
so no prefixing makes `@dsh/dsh-agent/lib/index.js` readable. Each such result
therefore carries **both**: the labelled path that says where it came from, and an
absolute `sourcePath`:
```
--- 3. @dsh/dsh-client-ui-settings/lib/types/client/contract/slots.d.ts:128-169 similarity 0.47
at /usr/local/lib/node_modules/@deepseek-ai/.../slots.d.ts
```
Reading that absolute path may still need a scope grant from the caller's own
policy — a facet guard, say. This plugin cannot grant one and does not pretend to.
An unusable entry — a typo, a path that is a file rather than a directory, two
roots claiming one name, or a name that collides with the active focus — is
**reported in the result** and skipped. One bad entry must not cost the other
roots, and it must not look like the feature doing nothing.
### Warming it before you need it
An extra root makes the index bigger, and a cold index is the one call that is
slow. `index` runs as a background job by default for exactly that reason — and
it is what makes the first search of a session cheap:
```
recall action=index
```
The job is owned by the session that started it, so its completion reaches that
conversation and its cancellation follows the session.
## Focus: letting a conversation see less
client/client.js
+10
View File
@@ -252,6 +252,16 @@ window.__ModuleLoader__.load({
label: 'Scope marker files',
hint: 'Comma separated filenames that make a directory its own scope. Empty uses the built-ins.',
},
{
key: 'sourceRoots',
kind: 'list',
group: 'Focus',
label: 'Extra source roots',
hint:
'Comma separated trees to index beside the workspace, for code that is not in it. ' +
'A path, or name=path to choose the label results carry. Relative paths resolve against the ' +
'workspace; absolute ones are taken as given, e.g. /usr/local/lib/node_modules/@deepseek-ai',
},
{
key: 'expansionEnabled',
index.js
+199 -15
View File
@@ -41,7 +41,7 @@
import { readFile, stat } from 'node:fs/promises'
import { createHash } from 'node:crypto'
import { basename, join, resolve } from 'node:path'
import { basename, join, resolve, sep } from 'node:path'
import { defineTool } from '@deepseek-ai/dsh-tools'
// A real dependency rather than a duck-typed `{ parse }`: the projection service
@@ -55,6 +55,7 @@ import { EmbeddingError, createEmbedder } from './lib/embed.js'
import { DEFAULT_FOCUS_MARKERS, discoverScopes, focusKey, resolveFocus } from './lib/focus.js'
import { indexFile, resolveDshHome, workspaceStateDir } from './lib/paths.js'
import { fuseRankings, rankOrder, selectFrom } from './lib/rank.js'
import { listRootFiles, resolveSourceRoots } from './lib/roots.js'
import { RecallIndex } from './lib/store.js'
import { createExpander } from './lib/expand.js'
import { createReranker } from './lib/rerank.js'
@@ -100,6 +101,7 @@ export const SETTING_KEYS = [
'stateDir',
'focus',
'focusMarkers',
'sourceRoots',
'expansionEnabled',
'expansionModel',
'expansionCount',
@@ -209,6 +211,38 @@ export const Config = Schema.object({
focus: Schema.string().default('').volatile(),
/** Filenames that mark a directory as its own scope. Empty means the built-ins. */
focusMarkers: Schema.array(String).default([]).volatile(),
/**
* Extra trees to index beside the workspace, for code that is not in it.
*
* The workspace index cannot answer questions about the code you are writing
* *against* — an installed framework, a platform SDK, a sibling checkout. In
* plugin development that is where most of the reading goes: the harness's own
* packages are the only authoritative copy of the API being implemented, and
* they live outside the workspace.
*
* Each entry is a path, optionally prefixed with `name=`:
*
* - an **absolute** path is taken as given — what a deployment's installed
* tree needs;
* - a **relative** path resolves against the workspace, which is what makes a
* checked-in configuration portable;
* - the **name** defaults to the directory's basename and becomes the first
* segment of every result path from that root (`@deepseek-ai/dsh-agent/…`),
* so a result says which tree it came from. Use `name=` when the basename is
* long or ambiguous.
*
* The root's *contents* are indexed, never its dependencies: `node_modules`,
* build output and VCS directories are skipped below the root while the root
* itself is always entered — which is what lets a root be the scoped-packages
* directory of a package manager. An unusable entry is reported in the result
* rather than being fatal, so one typo cannot cost the other roots.
*
* A result from a root carries an absolute `sourcePath` for the follow-up read,
* because the labelled path is relative to the root rather than the workspace.
* Reading it may still need a scope grant from the caller's own policy; this
* plugin cannot grant one.
*/
sourceRoots: Schema.array(String).default([]).volatile(),
/**
* Ask the local model for alternative phrasings and fuse the rankings.
*
@@ -575,6 +609,7 @@ export function apply(ctx, config) {
'oversized',
'unreadable',
'truncated',
'sourceRoots',
]
for (const key of fields) if (extra[key] !== undefined) out[key] = extra[key]
return out
@@ -655,8 +690,18 @@ export function apply(ctx, config) {
* knows exactly what is stale, so a partial index is described rather than
* guessed at.
*/
async function syncWorkspace(input) {
const { root, prefix = '', index, settings, budgetMs, signal, onProgress } = input
/**
* The candidate file list for one index: the workspace (or focus) tree plus
* every extra root.
*
* Split out from {@link syncWorkspace} because a search reports the absolute
* path of a result that came from an extra root, and the read phase is where
* that mapping is known. Both callers therefore see one list and one set of
* problems.
*/
async function walkIndex(input) {
const { root, prefix = '', settings, signal, sourceRoots = [] } = input
const walk = await listCodeFiles(root, {
...(settings.extensions?.length ? { extensions: settings.extensions } : {}),
skipDirs: DEFAULT_SKIP_DIRS,
@@ -672,8 +717,46 @@ export function apply(ctx, config) {
for (const file of walk.files) file.path = `${prefix}/${file.path}`
}
// Extra roots are walked as their own trees and merged into the same index,
// so one search ranks workspace code and library code together. Each root
// reports its own truncation: a root too large to index must say so rather
// than answer from its first few files.
const files = [...walk.files]
const problems = [...(input.sourceRootProblems ?? [])]
const summary = []
let truncated = walk.truncated
for (const sourceRoot of sourceRoots) {
const walkOfRoot = await listRootFiles({
name: sourceRoot.name,
path: sourceRoot.path,
...(settings.extensions?.length ? { extensions: settings.extensions } : {}),
exclude: settings.exclude ?? [],
maxFiles: Number(settings.maxFiles),
signal,
})
// A root labelled with the focus path itself would index two different
// files under one key — the second write silently replacing the first.
if (prefix !== '' && (prefix === sourceRoot.name || prefix.startsWith(`${sourceRoot.name}/`))) {
problems.push(
`the source root "${sourceRoot.name}" collides with the focus path "${prefix}"; ` +
'give it a different name= or search without that focus',
)
continue
}
files.push(...walkOfRoot.files)
summary.push({ name: sourceRoot.name, path: sourceRoot.path, files: walkOfRoot.files.length })
truncated = truncated || walkOfRoot.truncated
}
return { files, truncated, problems, summary }
}
async function syncWorkspace(input) {
const { index, settings, budgetMs, signal, onProgress } = input
const { files, truncated, problems: rootProblems, summary: rootSummary } = await walkIndex(input)
const known = index.fileHashes()
const present = new Set(walk.files.map((file) => file.path))
const present = new Set(files.map((file) => file.path))
const removed = index.prune([...known.keys()].filter((path) => !present.has(path)))
const maxBytes = Number(settings.maxFileBytes)
@@ -681,7 +764,7 @@ export function apply(ctx, config) {
let oversized = 0
let unreadable = 0
for (const file of walk.files) {
for (const file of files) {
if (signal?.aborted) throw signal.reason ?? new Error('aborted')
let content
try {
@@ -746,8 +829,8 @@ export function apply(ctx, config) {
const stats = index.stats()
return {
...stats,
scanned: walk.files.length,
truncated: walk.truncated,
scanned: files.length,
truncated,
removed,
embedded,
chunksWritten,
@@ -755,6 +838,13 @@ export function apply(ctx, config) {
stale: [...stale],
oversized,
unreadable,
sourceRoots: rootSummary,
sourceRootProblems: rootProblems,
// Not part of the model-facing contract — the schema is closed — but a
// search needs it: a result from an extra root is labelled relative to
// that root, so the absolute path is the only thing `read` can use, and
// only the walk knows it.
fileAbsolutes: new Map(files.map((file) => [file.path, file.absolute])),
}
}
@@ -790,6 +880,13 @@ export function apply(ctx, config) {
excerpt: { type: 'string', required: true },
truncated: { type: 'boolean', required: true },
stale: { type: 'boolean', required: true },
/**
* Absolute path, present only for a result that came from an extra
* index root. The labelled `path` of such a result is relative to
* its root rather than the workspace, so it is not what `read`
* resolves — this is.
*/
sourcePath: { type: 'string' },
},
},
},
@@ -808,8 +905,23 @@ export function apply(ctx, config) {
truncated: { type: 'boolean' },
database: { type: 'string' },
stateDir: { type: 'string' },
/** The extra roots this index holds, and how many files each contributed. */
sourceRoots: {
type: 'array',
items: {
type: 'object',
additionalProperties: false,
properties: {
name: { type: 'string', required: true },
path: { type: 'string', required: true },
files: { type: 'integer', required: true },
},
},
},
},
},
/** Entries of `sourceRoots` that could not be used, one message each. */
sourceRootProblems: { type: 'array', items: { type: 'string' } },
best: { type: 'number' },
endpoint: {
type: 'object',
@@ -880,12 +992,15 @@ export function apply(ctx, config) {
}
if (value.action !== 'search') {
const { files = 0, chunks = 0, pending = 0, scanned = 0, embedded = 0 } = value.index ?? {}
const refused = (value.sourceRootProblems ?? []).length === 0
? ''
: `\n\nRefused source roots: ${value.sourceRootProblems.join('; ')}.`
return [
{
type: 'text',
text:
`Index for ${value.workspace}: ${files} files, ${chunks} chunks. ` +
`Scanned ${scanned}, embedded ${embedded} this call, ${pending} still pending.`,
`Scanned ${scanned}, embedded ${embedded} this call, ${pending} still pending.${refused}`,
},
]
}
@@ -893,11 +1008,15 @@ export function apply(ctx, config) {
const results = value.results ?? []
if (results.length === 0) {
const suffix = (value.index?.pending ?? 0) > 0 ? ` ${value.index.pending} file(s) are still unindexed.` : ''
const refused =
(value.sourceRootProblems ?? []).length === 0
? ''
: ` Refused source roots: ${value.sourceRootProblems.join('; ')}.`
return [
{
type: 'text',
text:
`No match for ${JSON.stringify(value.query)}.${suffix} ` +
`No match for ${JSON.stringify(value.query)}.${suffix}${refused} ` +
'If you know the exact identifier, grep is the better tool for this query.',
},
]
@@ -912,9 +1031,14 @@ export function apply(ctx, config) {
const symbol = result.symbol === undefined ? '' : ` (${result.symbol})`
const stale = result.stale ? ' [stale — file changed since indexing]' : ''
const cut = result.truncated ? '\n… excerpt truncated; read the file for the rest' : ''
// A result from an extra root is labelled relative to that root, so the
// workspace-relative `path` is not something `read` can resolve. The
// absolute path is stated when it is the one that differs — a workspace
// result already has a usable path and does not need the noise.
const source = result.sourcePath === undefined ? '' : `\n at ${result.sourcePath}`
return (
`--- ${at + 1}. ${result.path}:${result.startLine}-${result.endLine}${symbol} ` +
`similarity ${result.similarity}${stale}\n${result.excerpt}${cut}`
`similarity ${result.similarity}${stale}${source}\n${result.excerpt}${cut}`
)
})
// A scope is a window, not a filter on relevance. When it holds nothing
@@ -938,10 +1062,22 @@ export function apply(ctx, config) {
'Search this workspace semantically with a local embedding model and return ranked code excerpts ' +
'with file paths and line ranges — without reading whole files into the conversation. Use it when ' +
'you do not know the exact identifier: "where is X handled", "which module does Y", "how does Z ' +
'recover from failure". Prefer grep when you already know the exact symbol, and read when you need ' +
'the whole file. Because excerpts are returned inline, do not follow up with a read of the same ' +
'range. Actions: "search" (default) answers a query and warms a bounded slice of the index; ' +
'"index" builds or refreshes the whole index, as a background job by default; "status" reports the ' +
'recover from failure". ' +
// The move this exists to replace: reading a module's files one by one to
// find out what is in it. That is a search, and this is cheaper than the
// reads it avoids — including on the very first call, because a search
// warms a bounded slice of the index as it answers.
'Reach for it before reading files to find out what a module contains or ' +
'which of several alike modules does the job: one search returns the relevant lines from wherever ' +
'they are, where reading to find them costs the whole file for each wrong guess and every one of ' +
'those lines is re-sent on every later step. ' +
'Prefer grep when you already know the exact symbol, and read when you need the whole file. ' +
'Because excerpts are returned inline, do not follow up with a read of the same ' +
'range. A result labelled with a leading name (for example "@dsh/…") came from an extra source ' +
'root rather than the workspace; such a result also carries an absolute `sourcePath`, which is the ' +
'one `read` can resolve. Actions: "search" (default) answers a query and warms a bounded slice of ' +
'the index; "index" builds or refreshes the whole index, as a background job by default, which is ' +
'what makes the first search of a session cheap; "status" reports the ' +
'index and endpoint state. The index is incremental and lives outside your repository.',
parameters: {
query: {
@@ -1040,6 +1176,15 @@ export function apply(ctx, config) {
})
const focusPath = focusKey(cwd, focus.root)
// Extra roots are resolved once per call against the live workspace, so
// an entry is judged where it is used rather than at load: a path that
// has not been installed yet is reported on the call that wanted it. A
// bad entry never aborts the call — it is reported beside the result, or
// the caller would lose the workspace index to one typo.
const resolvedRoots = resolveSourceRoots(settings.sourceRoots ?? [], { workspace: cwd })
const sourceRoots = resolvedRoots.roots
const sourceRootProblems = resolvedRoots.problems
let opened
try {
opened = indexFor(settings, cwd, focusPath)
@@ -1094,6 +1239,7 @@ export function apply(ctx, config) {
index: indexView(base.index, stats),
endpoint,
scopes: scopes.map((entry) => ({ path: entry.path, marker: entry.marker })),
...(sourceRootProblems.length === 0 ? {} : { sourceRootProblems }),
note:
`Index ${file} holds ${stats.files} files and ${stats.chunks} chunks, scoped to ` +
`${focusPath === '.' ? 'the whole workspace' : focusPath} (${focus.source}` +
@@ -1108,6 +1254,17 @@ export function apply(ctx, config) {
.slice(0, 12)
.map((entry) => entry.path)
.join(', ')}${scopes.length > 12 ? ` (+${scopes.length - 12} more)` : ''}. `) +
// Extra roots are named even when the index has not been rebuilt
// since they were configured, so their file counts are deliberately
// absent here — `stats` describes the index, not the configuration.
(sourceRoots.length === 0
? ''
: `Extra roots indexed beside the workspace: ${sourceRoots
.map((entry) => `${entry.name} (${entry.path})`)
.join(', ')}. `) +
(sourceRootProblems.length === 0
? ''
: `Source roots refused: ${sourceRootProblems.join('; ')}. `) +
`Embeddings come from "${settings.embeddingModel}" (${settings.embeddingDimensions} dims) at ` +
`${settings.embeddingApiBase} — ` +
(endpoint.ok === true
@@ -1193,6 +1350,8 @@ export function apply(ctx, config) {
settings,
budgetMs: Number.POSITIVE_INFINITY,
signal,
sourceRoots,
sourceRootProblems,
}),
)
@@ -1220,7 +1379,14 @@ export function apply(ctx, config) {
}
const result = await runIndex(exec.signal)
return { ...base, ok: true, index: indexView(base.index, result) }
return {
...base,
ok: true,
index: indexView(base.index, result),
...(result.sourceRootProblems.length === 0
? {}
: { sourceRootProblems: result.sourceRootProblems }),
}
}
// search
@@ -1250,6 +1416,8 @@ export function apply(ctx, config) {
settings,
budgetMs: Math.max(0, Number(settings.syncBudgetMs)),
signal: exec.signal,
sourceRoots,
sourceRootProblems,
}),
)
const matrix = index.matrix()
@@ -1342,6 +1510,18 @@ export function apply(ctx, config) {
const stale = new Set(sync.stale ?? [])
const best = results.length === 0 ? 0 : Math.max(...results.map((result) => result.similarity))
// Where the result actually is, for the cases where the label is not a
// workspace-relative path. A workspace result keeps its plain path: the
// caller resolves it against the workspace and has no use for an
// absolute one. A result from an extra root cannot be resolved that
// way, so it carries the absolute path beside its label.
const absolutes = sync.fileAbsolutes ?? new Map()
const workspacePrefix = resolve(cwd).endsWith(sep) ? resolve(cwd) : `${resolve(cwd)}${sep}`
const sourcePathOf = (result) => {
const absolute = absolutes.get(result.path)
if (absolute === undefined) return undefined
return absolute.startsWith(workspacePrefix) ? undefined : absolute
}
// The alternative to these excerpts: the whole of each file they came
// from. Sizes come from the index, which recorded them at embed time,
@@ -1377,11 +1557,15 @@ export function apply(ctx, config) {
excerpt: result.excerpt,
truncated: result.truncated,
stale: stale.has(result.path),
...(sourcePathOf(result) === undefined ? {} : { sourcePath: sourcePathOf(result) }),
})),
index: indexView(base.index, sync),
best,
weak: results.length === 0 || best < Number(settings.minSimilarity),
measure,
...(sync.sourceRootProblems.length === 0
? {}
: { sourceRootProblems: sync.sourceRootProblems }),
}
} catch (error) {
const code = error instanceof EmbeddingError ? error.code : 'RECALL_FAILED'
lib/roots.js
+254
View File
@@ -0,0 +1,254 @@
/**
* Extra index roots: code that is not in the workspace, indexed beside it.
*
* ## Why this exists
*
* A workspace index answers questions about the code you are writing. It does
* not answer questions about the code that code is written *against* — the
* library, the framework, the platform SDK. In plugin development that is not a
* side interest: measured on one real session, 64 of 80 tool calls went to the
* installed harness checkout under `/usr/local/lib/node_modules` to look up the
* contract being implemented, because that is the only authoritative copy of it.
* None of that is under the workspace, so none of it was indexed, and the agent
* read it into context the expensive way.
*
* An extra root is a second tree, indexed into the same index, whose results are
* distinguishable from the workspace's at a glance.
*
* ## The traversal rule is the whole feature
*
* An extra root is usually *a directory of packages* — the scoped-packages
* directory of a package manager, say — which is the one shape the workspace
* walk handles worst: it skips `node_modules` and friends at any depth, so the
* interesting files are exactly the ones it refuses to enter.
*
* So the rule is deliberately asymmetric, and it is worth stating precisely
* because the wrong version is silently wrong:
*
* - **Below the root**, `node_modules`, build output and VCS directories are
* skipped as usual. A dependency of a package is still somebody else's code,
* and it is the same code the root already holds.
* - **The root itself is entered whatever it is called.** A path that ends in
* `node_modules/...` is the normal way to name a package-manager tree, and a
* rule that skipped it would refuse the configuration this exists for.
*
* The cost of not applying the first half would be an index that embeds every
* dependency copy it can reach; the cost of not applying the second is that the
* feature cannot be configured at all. Only the root is exempt, never a nested
* directory that happens to share its name.
*
* ## Names, not just prefixes
*
* A result from an extra root is labelled `<name>/<path within the root>`. The
* name defaults to the root directory's basename — `@deepseek-ai` for a scope
* directory — and a deployment that wants something shorter says so with the
* `name=path` form, because `@deepseek-ai/dsh-agent/lib/index.js` is precise but
* long, and the label is on every result.
*
* ## Results from a root are not workspace-relative, and cannot pretend to be
*
* The paths a search returns are what the caller hands to `read`, which resolves
* against the workspace. A file under an extra root is not under the workspace,
* so no amount of prefixing makes `<name>/lib/index.js` readable. The honest
* answer is the absolute path, reported beside the relative label: the label
* says where the excerpt came from, the absolute path is what a follow-up read
* needs. Reading it may still need a scope grant from the caller's own policy —
* this module cannot grant it and does not pretend to.
*/
import { existsSync, statSync } from 'node:fs'
import { basename, isAbsolute, resolve } from 'node:path'
import { DEFAULT_SKIP_DIRS, listCodeFiles } from './walk.js'
/**
* Directories skipped inside an extra root.
*
* The workspace default minus `node_modules`: a root is normally a
* package-manager tree whose *top* level is the thing being indexed, while a
* `node_modules` further down is a dependency of one of those packages and is
* skipped by the depth rule in {@link listRootFiles}.
*
* `node_modules` stays IN the set: it is skipped below a root like anything else,
* and it is {@link skipsRootDir} — not a shorter list — that lets the root itself
* be one. Dropping it here as well would index every dependency of every package
* in a package-manager tree, which is the cost the depth rule exists to avoid.
*/
export const ROOT_SKIP_DIRS = Object.freeze([...DEFAULT_SKIP_DIRS])
/** Whether `path` exists and is a directory. */
function statIsDirectory(path) {
try {
return statSync(path).isDirectory()
} catch {
return false
}
}
/**
* Read one `sourceRoots` entry.
*
* Accepted forms, both orderings of the same two facts:
*
* ```
* /abs/path/to/packages # name defaults to the basename
* @dsh=/abs/path/to/packages # explicit name
* ```
*
* A relative path is resolved against the workspace, which is what makes a
* checked-in configuration portable; an absolute one is taken as given, which is
* what a deployment's own installed tree needs.
*
* @param {string} entry one configured root
* @param {string} workspace absolute workspace root
* @returns {{name: string, path: string, workspaceRelative: boolean}}
*/
export function parseSourceRoot(entry, workspace) {
const text = String(entry ?? '').trim()
if (text === '') throw new Error('an empty entry')
// `name=path` only when a name is actually present: a Windows path or a
// relative path with an `=` in it must not be split by accident.
let name = ''
let target = text
const equals = text.indexOf('=')
if (equals > 0) {
name = text.slice(0, equals).trim()
target = text.slice(equals + 1).trim()
}
if (target === '') throw new Error(`"${text}" names no path`)
if (name.includes('/') || name.includes('\\')) {
throw new Error(`"${name}" is not a usable name: it contains a path separator`)
}
const workspaceRelative = !isAbsolute(target)
const absolute = workspaceRelative ? resolve(workspace, target) : resolve(target)
const label = sanitizeRootName(name === '' ? basename(absolute) : name)
if (label === '') throw new Error(`"${text}" does not yield a usable name`)
return { name: label, path: absolute, workspaceRelative }
}
/**
* Make a name safe to use as a path prefix.
*
* The name becomes the first segment of every result path, so a name carrying a
* separator would forge a deeper path than the root holds, and a name of `.` or
* `..` would forge one outside it.
*/
export function sanitizeRootName(name) {
const cleaned = String(name ?? '')
.trim()
.replace(/[^A-Za-z0-9._@-]+/g, '-')
.replace(/^[-.]+|[-.]+$/g, '')
return cleaned === '.' || cleaned === '..' ? '' : cleaned
}
/**
* Resolve every configured root, reporting each unusable one by name.
*
* A bad entry is skipped rather than fatal, and said out loud: a typo in one
* root must not cost the caller the other roots, and it must not look like the
* feature doing nothing. This mirrors how an unusable `focus` is reported.
*
* @param {readonly string[]} entries configured `sourceRoots`
* @param {object} [options]
* @param {string} options.workspace absolute workspace root
* @param {(path: string) => boolean} [options.isDirectory] injection point for tests
* @returns {{roots: {name: string, path: string}[], problems: string[]}}
*/
export function resolveSourceRoots(entries, options = {}) {
const { workspace, isDirectory = statIsDirectory } = options
const roots = []
const problems = []
const seen = new Set()
for (const entry of entries ?? []) {
let parsed
try {
parsed = parseSourceRoot(entry, workspace)
} catch (error) {
problems.push(String(error?.message ?? error))
continue
}
if (!isDirectory(parsed.path)) {
problems.push(`"${String(entry).trim()}" is not a directory: ${parsed.path}`)
continue
}
if (seen.has(parsed.name)) {
problems.push(`"${String(entry).trim()}" duplicates the name "${parsed.name}", which was already indexed`)
continue
}
seen.add(parsed.name)
roots.push({ name: parsed.name, path: parsed.path })
}
return { roots, problems }
}
/**
* Whether a directory is skipped during a *root* walk.
*
* Exported because it is the rule the feature turns on, and a test that asserts
* a nested `node_modules` is skipped is a test of this function, not of a walk.
*
* @param {number} depth level below the root; 0 is the root's own entries
* @param {string} name a directory entry's name
* @param {ReadonlySet<string>} [skip] directory names skipped below the root
*/
export function skipsRootDir(depth, name, skip = new Set(ROOT_SKIP_DIRS)) {
// The root is entered whatever it is called — that is the whole point of an
// explicit root. Everything below it obeys the normal skip list.
if (depth === 0) return false
return skip.has(name)
}
/**
* Label a file found under an extra root.
*
* @param {string} rootName the root's label
* @param {string} relPath path within the root, `/`-separated
*/
export function rootRelativePath(rootName, relPath) {
return relPath === '' ? rootName : `${rootName}/${relPath}`
}
/**
* List the indexable files under one extra root.
*
* Keyed by `ROOT_SKIP_DIRS` membership *below* the root and by nothing at all at
* it, because `listCodeFiles` reports the walk root at depth 0 and a directory it
* descends into at depth + 1. So the root's own name never decides anything,
* which is what lets a root be a `node_modules` tree, and a `node_modules`
* *inside* one of its packages is still skipped.
*
* `maxFiles` bounds each root rather than the whole index, and the result reports
* truncation, so a root too large to index says so instead of quietly answering
* from its first few files.
*
* @param {object} input
* @param {string} input.name label for the root, prefixed onto every path
* @param {string} input.path absolute directory to walk
* @param {readonly string[]} [input.extensions] allowed extensions, without dots
* @param {readonly string[]} [input.exclude] glob patterns over the root-relative path
* @param {number} [input.maxFiles] stop after this many candidates
* @param {AbortSignal} [input.signal]
* @returns {Promise<{files: {path: string, absolute: string}[], truncated: boolean, skippedFiles: number}>}
*/
export async function listRootFiles(input) {
const { name, path, extensions, exclude = [], maxFiles, signal } = input
const skip = new Set(ROOT_SKIP_DIRS)
const walk = await listCodeFiles(path, {
...(extensions === undefined ? {} : { extensions }),
exclude,
...(maxFiles === undefined ? {} : { maxFiles }),
isSkipped: (depth, dirName) => skipsRootDir(depth, dirName, skip),
signal,
})
return {
...walk,
files: walk.files.map((file) => ({ path: rootRelativePath(name, file.path), absolute: file.absolute })),
}
}
/** One entry of the `sourceRoots` field, as the plugin's own docs describe it. */
export const SOURCE_ROOT_EXAMPLE = '/usr/local/lib/node_modules/@deepseek-ai'
lib/walk.js
+13 -1
View File
@@ -142,6 +142,11 @@ export function extensionOf(path) {
* @param {readonly string[]} [options.extensions] allowed extensions, without dots
* @param {readonly string[]} [options.skipDirs] directory names skipped at any depth
* @param {readonly string[]} [options.exclude] glob patterns over the relative path
* @param {(depth: number, name: string, absolute: string) => boolean} [options.isSkipped]
* decides directory traversal, replacing the `skipDirs` membership test. This is
* the seam an extra index root uses: its top level is a package-manager
* `node_modules` tree, which the built-in list exists to skip, so it must opt out
* for the root and only the root. Depth 0 is the root's own entries.
* @param {number} [options.maxFiles] stop after this many candidates
* @param {number} [options.maxDepth] deepest directory level to enter
* @param {AbortSignal} [options.signal]
@@ -152,6 +157,7 @@ export async function listCodeFiles(root, options = {}) {
extensions = DEFAULT_EXTENSIONS,
skipDirs = DEFAULT_SKIP_DIRS,
exclude = [],
isSkipped,
maxFiles = 20_000,
maxDepth = 24,
signal,
@@ -159,6 +165,12 @@ export async function listCodeFiles(root, options = {}) {
const wanted = new Set(extensions.map((value) => String(value).toLowerCase()))
const skip = new Set(skipDirs)
// Materialised once per walk rather than tested with `isSkipped ?? default` at
// every directory: the hot loop should not carry a branch that cannot change.
const shouldSkip =
isSkipped === undefined
? (depth, name) => skip.has(name)
: (depth, name, absolute) => isSkipped(depth, name, absolute)
const files = []
let truncated = false
let skippedFiles = 0
@@ -185,7 +197,7 @@ export async function listCodeFiles(root, options = {}) {
const absolute = join(dir, entry.name)
const relPath = relative(root, absolute).split(sep).join('/')
if (entry.isDirectory()) {
if (skip.has(entry.name)) continue
if (shouldSkip(depth, entry.name, absolute)) continue
if (depth >= maxDepth) continue
if (matchesAny(relPath, exclude)) continue
stack.push({ dir: absolute, depth: depth + 1 })
test/helpers.js
+8 -2
View File
@@ -102,8 +102,14 @@ export function makeContext(options = {}) {
}
/** An `exec` argument for one tool call. */
export function makeExec(cwd, signal) {
return { signal: signal ?? new AbortController().signal, agent: { session: { header: { cwd } } } }
export function makeExec(cwd, signal, sessionId = 'session-under-test') {
// `agent.id` is the SESSION id — the jobs registry resolves a background job's
// owner through `agents.get(id)` and refuses anything that is not that string.
// A fixture without it cannot catch a call that passes the whole handle.
return {
signal: signal ?? new AbortController().signal,
agent: { id: sessionId, session: { header: { cwd } } },
}
}
/** Write a map of relative path to content into a fresh temporary directory. */
test/plugin.test.js
+158 -3
View File
@@ -1,5 +1,5 @@
import { strict as assert } from 'node:assert'
import { readFile, rm, writeFile } from 'node:fs/promises'
import { mkdir, readFile, rm, writeFile } from 'node:fs/promises'
import { join } from 'node:path'
import { test } from 'node:test'
@@ -159,6 +159,7 @@ function configFor(stateDir, extra = {}) {
// the two lists from drifting apart again.
focus: '',
focusMarkers: [],
sourceRoots: [],
expansionEnabled: false,
expansionModel: '',
expansionCount: 4,
@@ -173,7 +174,7 @@ function configFor(stateDir, extra = {}) {
}
/** Apply the plugin to a fresh workspace with a stubbed embedding endpoint. */
async function setup(t, files = FILES, extra = {}, stubOptions = {}) {
async function setup(t, files = FILES, extra = {}, stubOptions = {}, services = {}) {
const workspace = await makeWorkspace(files)
const stateDir = await makeStateDir()
const stub = makeStubFetch(stubOptions)
@@ -185,7 +186,7 @@ async function setup(t, files = FILES, extra = {}, stubOptions = {}) {
await rm(stateDir, { recursive: true, force: true })
})
const { ctx, tools, dispose } = makeContext()
const { ctx, tools, dispose } = makeContext({ services })
t.after(() => dispose())
apply(ctx, configFor(stateDir, extra))
assert.equal(tools.length, 1, 'exactly one tool should be registered')
@@ -814,3 +815,157 @@ test('re-ranking degrades to the fused order when the model has no opinion', asy
)
assert.ok(reranked.results.length > 0, 'no opinion must not mean no results')
})
test('a background index hands the jobs registry a session id, not the agent handle', async (t) => {
// The jobs registry resolves `spec.owner` through `agents.get(owner)` and
// throws `session "[object Object]" has no live agent` for anything that is not
// the session id string. This test is a stand-in for that check: the bug it
// catches was invisible in-process and made the DEFAULT `index` call fail on a
// real deployment, where a throwing starter leaves the caller no job at all.
const started = []
const services = {
jobs: {
start(spec) {
started.push(spec)
return `recall-index-${started.length}`
},
},
}
const { tool, workspace } = await setup(t, FILES, {}, {}, services)
const value = await tool.execute({ action: 'index' }, makeExec(workspace, undefined, 'session-42'))
assert.deepEqual(schemaErrors(tool.output.schema, value), [])
assert.equal(value.jobId, 'recall-index-1')
assert.equal(started.length, 1)
assert.equal(
started[0].owner,
'session-42',
'owner must be the session id: the registry refuses an Agent handle outright',
)
assert.equal(typeof started[0].run, 'function')
// And the job the registry was handed actually indexes the workspace.
const outcome = await started[0].run().done
assert.equal(outcome.status, 'completed')
assert.match(outcome.output, /Indexed/)
const search = await tool.execute({ query: 'session expiry' }, makeExec(workspace))
assert.ok(search.results.length > 0, 'the background job must leave a searchable index behind')
})
/**
* A workspace with a library tree beside it, laid out the way a real one is.
*
* The library is a *scoped packages directory* — its own path runs through
* `node_modules` — because that is the shape the workspace walk exists to skip
* and the shape this feature has to enter. `root` is the parent of both trees, so
* the configured root can be given as a relative path and stays portable.
*
* @returns {Promise<{root: string, cleanup: () => Promise<void>}>}
*/
async function makeRootPair() {
const root = await makeWorkspace({})
const workspace = join(root, 'My Project')
const library = join(root, 'packages', 'node_modules', '@vendor')
await mkdir(join(library, 'widget', 'lib'), { recursive: true })
await mkdir(join(library, 'widget', 'node_modules', 'dep'), { recursive: true })
await mkdir(workspace, { recursive: true })
await writeFile(join(workspace, 'app.ts'), 'export const app = 1\n')
await writeFile(
join(library, 'widget', 'lib', 'index.js'),
'export function mountWidget(root, options) { return { root, options } }\n',
)
await writeFile(
join(library, 'widget', 'node_modules', 'dep', 'index.js'),
'export const vendored = "should never be indexed"\n',
)
return { root, workspace, library, cleanup: () => rm(root, { recursive: true, force: true }) }
}
/** Apply the plugin to a workspace the caller made itself. */
async function setupAt(t, workspace, extra = {}) {
const stateDir = await makeStateDir()
const stub = makeStubFetch()
const originalFetch = globalThis.fetch
globalThis.fetch = stub
t.after(async () => {
globalThis.fetch = originalFetch
await rm(stateDir, { recursive: true, force: true })
})
const { ctx, tools, dispose } = makeContext()
t.after(() => dispose())
apply(ctx, configFor(stateDir, extra))
return { tool: tools[0], workspace, stub }
}
test('an extra root is indexed beside the workspace, and its dependencies are not', async (t) => {
const { root, workspace, library } = await makeRootPair()
t.after(async () => rm(root, { recursive: true, force: true }))
// Relative to the workspace, and named, because the naming is what makes a
// result say which tree it came from.
const { tool } = await setupAt(t, workspace, { sourceRoots: ['vendor=../packages/node_modules/@vendor'] })
const index = await tool.execute({ action: 'index', run_in_background: false }, makeExec(workspace))
assert.deepEqual(schemaErrors(tool.output.schema, index), [])
assert.deepEqual(index.index.sourceRoots, [{ name: 'vendor', path: library, files: 1 }])
assert.equal(index.index.files, 2, 'the workspace file and the library file, and nothing vendored')
const search = await tool.execute({ query: 'mountWidget root options' }, makeExec(workspace))
assert.deepEqual(schemaErrors(tool.output.schema, search), [])
const hit = search.results.find((result) => result.path.startsWith('vendor/'))
assert.ok(hit, `no library result in ${search.results.map((result) => result.path).join(', ')}`)
// The label names the tree; the absolute path is the only thing `read` can use,
// because the label is relative to the root rather than the workspace.
assert.equal(hit.path, 'vendor/widget/lib/index.js')
assert.equal(hit.sourcePath, join(library, 'widget', 'lib', 'index.js'))
// A workspace result is unchanged: its own path already resolves.
const own = search.results.find((result) => result.path === 'app.ts')
if (own !== undefined) assert.equal(own.sourcePath, undefined)
// The nested dependency is absent from the index entirely, not merely unranked.
assert.equal(
search.results.some((result) => result.path.includes('node_modules')),
false,
)
})
test('an unusable source root is reported and never costs the workspace index', async (t) => {
const { root, workspace } = await makeRootPair()
t.after(async () => rm(root, { recursive: true, force: true }))
const { tool } = await setupAt(t, workspace, { sourceRoots: ['ghost=../nowhere', 'vendor=../packages/node_modules/@vendor'] })
const value = await tool.execute({ action: 'index', run_in_background: false }, makeExec(workspace))
assert.deepEqual(schemaErrors(tool.output.schema, value), [])
// The good root still indexed, and the bad one is named rather than swallowed.
assert.deepEqual(value.index.sourceRoots.map((entry) => entry.name), ['vendor'])
assert.equal(value.sourceRootProblems.length, 1)
assert.match(value.sourceRootProblems[0], /nowhere/)
assert.ok(value.index.files >= 2, 'the workspace and the surviving root must both be indexed')
const status = await tool.execute({ action: 'status' }, makeExec(workspace))
assert.deepEqual(schemaErrors(tool.output.schema, status), [])
assert.equal(status.sourceRootProblems.length, 1)
assert.ok(status.results === undefined, 'status returns no results')
})
test('a source root whose path is a file, not a directory, is refused', async (t) => {
const { root, workspace } = await makeRootPair()
t.after(async () => rm(root, { recursive: true, force: true }))
const { tool } = await setupAt(t, workspace, { sourceRoots: ['app=./app.ts'] })
const value = await tool.execute({ action: 'index', run_in_background: false }, makeExec(workspace))
assert.deepEqual(schemaErrors(tool.output.schema, value), [])
assert.equal(value.index.sourceRoots.length, 0)
assert.match(value.sourceRootProblems[0], /not a directory/)
})
test('a source root named after the focus path is refused as a collision', async (t) => {
// Two different files would be indexed under one key, the second write
// replacing the first — so this cannot be left to the walk to get right.
const root = await makeWorkspace({ 'vendor/app.ts': 'export const a = 1\n', 'lib/app.ts': 'export const b = 2\n' })
t.after(() => rm(root, { recursive: true, force: true }))
const { tool } = await setupAt(t, root, { sourceRoots: ['vendor=vendor'] })
const value = await tool.execute(
{ action: 'index', run_in_background: false, focus: 'vendor' },
makeExec(root),
)
assert.deepEqual(schemaErrors(tool.output.schema, value), [])
assert.equal(value.index.sourceRoots.length, 0)
assert.match(value.sourceRootProblems[0], /collides with the focus path/)
})
test/roots.test.js
+138
View File
@@ -0,0 +1,138 @@
import { strict as assert } from 'node:assert'
import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
import {
ROOT_SKIP_DIRS,
listRootFiles,
parseSourceRoot,
resolveSourceRoots,
rootRelativePath,
sanitizeRootName,
skipsRootDir,
} from '../lib/roots.js'
test('a root defaults its name to the directory basename', () => {
const parsed = parseSourceRoot('/usr/local/lib/node_modules/@deepseek-ai', '/repo')
assert.equal(parsed.name, '@deepseek-ai')
assert.equal(parsed.path, '/usr/local/lib/node_modules/@deepseek-ai')
assert.equal(parsed.workspaceRelative, false)
})
test('an explicit name is taken as written', () => {
const parsed = parseSourceRoot('dsh=/usr/local/lib/node_modules/@deepseek-ai', '/repo')
assert.equal(parsed.name, 'dsh')
assert.equal(parsed.path, '/usr/local/lib/node_modules/@deepseek-ai')
})
test('a relative root resolves against the workspace and says so', () => {
const parsed = parseSourceRoot('vendor/lib', '/repo')
assert.equal(parsed.path, '/repo/vendor/lib')
assert.equal(parsed.name, 'lib')
assert.equal(parsed.workspaceRelative, true)
})
test('a name containing a separator is refused, because it would forge a path', () => {
assert.throws(() => parseSourceRoot('a/b=/tmp/x', '/repo'), /path separator/)
})
test('an entry that names no path or yields no name is refused', () => {
assert.throws(() => parseSourceRoot('', '/repo'), /empty entry/)
assert.throws(() => parseSourceRoot('name=', '/repo'), /names no path/)
// A root that climbs to the filesystem root has no basename to be labelled
// with, and `resolve` collapsing the traversal is exactly why the check is on
// the resolved path rather than on the text of the entry.
assert.throws(() => parseSourceRoot('/../..', '/repo'), /does not yield a usable name/)
})
test('sanitizeRootName keeps the characters a path segment can hold', () => {
assert.equal(sanitizeRootName('@deepseek-ai'), '@deepseek-ai')
assert.equal(sanitizeRootName('my root'), 'my-root')
assert.equal(sanitizeRootName('..'), '')
assert.equal(sanitizeRootName('.'), '')
assert.equal(sanitizeRootName(' '), '')
})
test('rootRelativePath never emits a leading or trailing separator', () => {
assert.equal(rootRelativePath('dsh', 'lib/index.js'), 'dsh/lib/index.js')
assert.equal(rootRelativePath('dsh', ''), 'dsh')
})
test('resolveSourceRoots skips a bad entry loudly and keeps the rest', () => {
const result = resolveSourceRoots(['/good', 'missing=/nope', '/good'], {
workspace: '/repo',
isDirectory: (path) => path === '/good',
})
assert.deepEqual(result.roots, [{ name: 'good', path: '/good' }])
// A duplicate name is dropped rather than silently overwriting: two roots with
// one label would make every result from the second ambiguous.
assert.equal(result.problems.length, 2)
assert.match(result.problems[0], /not a directory/)
assert.match(result.problems[1], /duplicates the name/)
})
test('resolveSourceRoots tolerates no configuration at all', () => {
assert.deepEqual(resolveSourceRoots([], { workspace: '/repo' }), { roots: [], problems: [] })
assert.deepEqual(resolveSourceRoots(undefined, { workspace: '/repo' }), { roots: [], problems: [] })
})
test('the root itself is entered whatever it is called, and node_modules below it is not', () => {
// This asymmetry is the feature: a root is normally named `.../node_modules/...`,
// while a `node_modules` inside one of its packages is somebody else's code.
assert.equal(skipsRootDir(0, 'node_modules'), false, 'the root must not be skipped')
assert.equal(skipsRootDir(0, '.git'), false)
assert.equal(skipsRootDir(1, 'node_modules'), true, 'a nested dependency store must be skipped')
assert.equal(skipsRootDir(3, 'node_modules'), true)
assert.equal(skipsRootDir(1, 'dist'), true)
assert.equal(skipsRootDir(1, 'lib'), false)
assert.ok(ROOT_SKIP_DIRS.includes('node_modules'), 'the skip list must still name node_modules')
})
test('listRootFiles indexes a package tree and not its dependencies', async (t) => {
const root = await mkdtemp(join(tmpdir(), 'recall-root-'))
t.after(() => rm(root, { recursive: true, force: true }))
const packages = join(root, 'node_modules', '@scope')
await mkdir(join(packages, 'alpha', 'lib'), { recursive: true })
await mkdir(join(packages, 'alpha', 'node_modules', 'dep'), { recursive: true })
await mkdir(join(packages, 'beta', 'dist'), { recursive: true })
await writeFile(join(packages, 'alpha', 'lib', 'index.js'), 'export const a = 1\n')
await writeFile(join(packages, 'alpha', 'node_modules', 'dep', 'index.js'), 'module.exports = 0\n')
await writeFile(join(packages, 'beta', 'dist', 'bundle.js'), 'var x = 1\n')
await writeFile(join(packages, 'beta', 'index.js'), 'export const b = 2\n')
await writeFile(join(packages, 'README.md'), '# not code\n')
const walk = await listRootFiles({ name: 'dsh', path: packages })
assert.deepEqual(
walk.files.map((file) => file.path),
['dsh/alpha/lib/index.js', 'dsh/beta/index.js'],
'the root is entered, a dependency below it is not, and dist is not',
)
// Every path is labelled but resolvable: `read` needs the absolute one.
for (const file of walk.files) assert.ok(file.absolute.startsWith(packages), file.absolute)
})
test('listRootFiles does not follow a symlink out of the root', async (t) => {
const root = await mkdtemp(join(tmpdir(), 'recall-root-'))
const outside = await mkdtemp(join(tmpdir(), 'recall-outside-'))
t.after(async () => {
await rm(root, { recursive: true, force: true })
await rm(outside, { recursive: true, force: true })
})
await mkdir(join(root, 'pkg'), { recursive: true })
await writeFile(join(root, 'pkg', 'a.js'), 'export const a = 1\n')
await writeFile(join(outside, 'secret.js'), 'const secret = 1\n')
await symlink(outside, join(root, 'link'), 'dir')
const walk = await listRootFiles({ name: 'r', path: root })
assert.deepEqual(walk.files.map((file) => file.path), ['r/pkg/a.js'])
})
test('a file at the root of a root is labelled with the root name alone', async (t) => {
const root = await mkdtemp(join(tmpdir(), 'recall-root-'))
t.after(() => rm(root, { recursive: true, force: true }))
await writeFile(join(root, 'index.js'), 'export const root = 1\n')
const walk = await listRootFiles({ name: 'solo', path: root })
assert.deepEqual(walk.files.map((file) => file.path), ['solo/index.js'])
})