Conversation
`context add` writes to ~/.context/packages from a separate process, but a running `serve` reads that directory once at startup. A long-lived stdio server therefore kept serving the package list it saw when it launched, and `get_docs` reported a package as missing when it was already installed. The only way out was to reconnect the client. The mechanism to avoid that already existed and was wired to a single trigger. refreshGetDocsTool rebuilds the tool's `library` enum and calls sendToolListChanged, which is the MCP notification telling a client to re-fetch the tool list; it was called only from the download_package handler, so packages that arrived any other way were invisible. This adds a second trigger rather than a second mechanism. - watch.ts: a small debounced directory watcher. Debounced because one install is several filesystem events (a temp file, then a rename), which would otherwise rebuild the schema three or four times. The watcher is unref'd so it never keeps the process alive on its own, and a callback that throws cannot tear it down. - serve: watches the data directory, reloads the store and refreshes the tool. Skipped when --libs is set, because that flag pins the session to a fixed library set on purpose and picking up new packages would defeat it. - refreshGetDocsTool is now public, since the trigger lives outside the class. - loadPackages now syncs rather than only adding. It is called repeatedly now, so a package removed from disk has to leave the store too, which the add-only version could not express. HTTP transport needed no change: it builds a fresh ContextServer per session over the same store, so a session started after an install already sees it. Only the long-lived stdio server needed the live notification. Tests cover the watcher directly: a single change fires once, a burst collapses to one call, stopping prevents further calls, a throwing callback does not kill the watcher, and a missing directory is a no-op. Verified discriminating by removing the debounce, which fails three of the five. Local `pnpm test` shows 40 pre-existing failures in this environment, identical before and after this change: better-sqlite3 11.10.0 does not build against Node v26, so every sqlite-backed test errors on the missing bindings. The five new tests pass (181 to 186 passing). pnpm lint and pnpm build are clean.
🦋 Changeset detectedLatest commit: f093bce The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. **The per-class OCCT reference manual IS indexed**, as `occt-refman`, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. Those routes remain, scoped to the case that needs them: a class genuinely absent after an OCCT bump, before the package is rebuilt. **`context query` is now a lookup step in its own right.** `get_docs` takes its `library` from an enum fixed when the MCP server connected, and the server rebuilds that list only for packages arriving through its own download_package tool. Anything installed by `context add` is therefore invisible to `get_docs` for the rest of the session while `context query` sees it immediately. The failure reads as "package missing", which an agent takes at face value and escalates on, so the policy now says plainly that it means "not in the list I was handed". Fixed upstream in neuledge/context#117. Also records that the `ecosystem` package is indexed, so this standard and every shared policy are themselves queryable, and what a version in the cache means: a package is pinned to a release and does not follow its repo. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
Two corrections, one of which was actively costing time. The per-class OCCT reference manual IS indexed, as occt-refman, 37,008 sections. This copy said it was not, which sent an agent to the bundled headers or to WebFetch as a first resort rather than a last one. And context query is now a lookup step in its own right: get_docs takes its library from an enum fixed when the MCP server connected, so anything installed by context add is invisible to it for the rest of the session while the CLI sees it immediately. Fixed upstream in neuledge/context#117. Copied verbatim from SecondMouseAU/ecosystem okf/policies/context-first.md.
|
Thanks for this — the mechanism is right and the debounce, the 1. A package that's merely unreadable gets evicted as if deleted.
That would be a narrow race, except if (existsSync(outputPath)) unlinkSync(outputPath); // removes the installed package
const db = openDatabase(outputPath); // rebuilds in placeSo Worse, a half-built DB passes Fix: evict only on real absence. for (const pkg of store.list()) {
if (onDisk.has(packageKey(pkg))) continue;
if (existsSync(pkg.path)) continue; // unreadable != gone
store.remove(packageKey(pkg));
}2. In-flight downloads get advertised. Both download paths write if (!file.endsWith(".db") || file.startsWith(".")) continue;3. Two more worth doing: On the tests — I mutation-tested them. Removing the Happy to push these myself if you'd rather not — say the word and I'll do it, keeping you as author. Otherwise take your time; nothing here is far off. Generated by Claude Code |
moshest
left a comment
There was a problem hiding this comment.
Still at f093bce with no new commits since my Aug 25 review, so the three blockers are open. One thing that should help: #127 bumped better-sqlite3 to 13.x, so the missing-bindings failure that stopped you running the sqlite-backed suites on Node 26 should be gone if you sync with main — the branch still merges clean against it.
My earlier offer stands: I'm happy to push the three fixes myself and keep you as author, just say the word. It's been four weeks though, so if there's no reply by early October I'll close this as stale and we can reopen it whenever you're ready.
Generated by Claude Code
There was a problem hiding this comment.
Changes requested — three blockers remain: the live store can lose an installed package, temporary downloads can be indexed as complete, and first-run serve never starts a real watcher.
Data & state — high risk, blocker
Behavior — high risk, blocker
Docs & rules — low risk, comment
Maintainability — low risk, comment
Simplicity — low risk, comment (combined with the HTTP-scope finding below)
Unanchored data finding (packages/context/src/cli.ts:521–524, 692; packages/context/src/download.ts:79): the .db filter also admits .downloading-*.db. A slow download can be read and stored under its temporary path; once renamed, get_docs can fail to open the stale path. Exclude dotfiles or .downloading-* before loading.
Unanchored test gap (packages/context/src/watch.test.ts:24–93): these tests cover watchDirectory alone, not serve → store reload → MCP tool-list notification. Add an integration test for that flow.
This codebase is managed by Human0.
|
|
||
| for (const pkg of store.list()) { | ||
| const key = packageKey(pkg); | ||
| if (!onDisk.has(key)) store.remove(key); |
There was a problem hiding this comment.
Data & state · blocker (state corruption): this removes a package whenever readPackageInfo failed, because only successful reads are recorded in onDisk. Reachable sequence: while serve watches, another process rebuilds an installed package and unlinks its DB; if the debounced reload runs during that gap (or reads a partially built DB), this removes the still-installed package or replaces it with partial metadata. Remove a store entry only after confirming the package file is absent; keep the old entry on read failure.
This codebase is managed by Human0.
| * since nothing has been installed yet in that case. | ||
| */ | ||
| export function watchDirectory(dir: string, onChange: () => void): () => void { | ||
| if (!existsSync(dir)) return () => {}; |
There was a problem hiding this comment.
Behavior · blocker: on a fresh HOME, DATA_DIR does not exist, so the initial package scan skips it and this watcher returns a no-op. If another process then creates the directory and installs a package, no event reloads the store or refreshes get_docs—the exact missing-package case this PR says it fixes. Ensure the directory before loading/watching, or watch/retry its parent.
This codebase is managed by Human0.
| // would otherwise serve the package list it read at startup until the | ||
| // client reconnected. Skipped under --libs, which pins the session to a | ||
| // fixed library set on purpose. | ||
| if (!allowedLibraries) { |
There was a problem hiding this comment.
Behavior · comment: this starts the watcher for HTTP serve as well as stdio. HTTP sessions use their own MCP server, but the callback refreshes only the root server, which is not connected to a session; it still mutates the shared store without sending existing sessions a tool-list update. This also conflicts with the description that HTTP needs no change. Scope this watcher to stdio or refresh the actual HTTP session servers. The unnecessary HTTP watcher is the simplicity concern too.
This codebase is managed by Human0.
|
|
||
| let pending: NodeJS.Timeout | undefined; | ||
|
|
||
| const watcher = watch(dir, () => { |
There was a problem hiding this comment.
Behavior · comment: watch() is created without an error listener or retry. An emitted filesystem watcher error (for example, resource exhaustion) is unhandled and can terminate the server; deleting and recreating the directory can also leave the watch attached to the old inode with no later events. Handle watcher errors and reattach or watch the parent directory.
This codebase is managed by Human0.
| }); | ||
|
|
||
| // What one install looks like: a temp file, then a rename into place. | ||
| writeFileSync(join(dir, ".downloading-demo.db"), "x"); |
There was a problem hiding this comment.
Maintainability · comment: these synchronous writes in one tick do not establish debounce behavior; filesystem event coalescing can make the test pass even if the debounce is removed. Space events across the debounce window and assert only one callback after the final event.
This codebase is managed by Human0.
| throw new Error("refresh failed"); | ||
| }); | ||
|
|
||
| writeFileSync(join(dir, "one.db"), "x"); |
There was a problem hiding this comment.
Maintainability · comment: this only checks that a later filesystem event invokes the callback after the first callback throws; it does not assert that the exception was contained. Add an assertion that proves the thrown callback error is handled while the watcher remains active.
This codebase is managed by Human0.
| @@ -0,0 +1,5 @@ | |||
| --- | |||
| "@neuledge/context": patch | |||
There was a problem hiding this comment.
Docs & rules · comment: refreshGetDocsTool is now public on the package API, which is an additive API feature; the repository convention maps new features to a minor bump, but this changeset declares patch. Please bump it to minor, or clarify why this method is not part of the supported package surface.
This codebase is managed by Human0.
context addwrites to~/.context/packagesfrom a separate process, but a runningservereads that directory once at startup. A long-lived stdio server therefore keeps serving the package list it saw when it launched, soget_docsreports a package as missing when it is already installed, and the only way out is to reconnect the client.I hit this adding a locally-built docs package while a Claude Code session was open:
context querysaw it instantly,get_docscould not see it at all.The mechanism already existed
refreshGetDocsToolrebuilds the tool'slibraryenum and callssendToolListChanged, the MCP notification that tells a client to re-fetch the tool list. It had exactly one caller, thedownload_packagehandler, so a package arriving any other way was invisible.This adds a second trigger, not a second mechanism.
The change
watch.ts, a small debounced directory watcher. Debounced because one install is several filesystem events (a temp file, then a rename into place), which would otherwise rebuild the schema three or four times for one logical change. The watcher isunref'd so it never holds the process open by itself, and a callback that throws cannot tear it down.servewatches the data directory, reloads the store, refreshes the tool. Skipped when--libsis set, since that flag pins the session to a fixed library set on purpose and picking up new packages would defeat it.refreshGetDocsToolis now public, because the trigger lives outside the class. That is the only public surface change; happy to bump the changeset tominorif you would rather treat it as one.loadPackagesnow syncs rather than only adding. It is called repeatedly now, so a package removed from disk has to leave the store too, which the add-only version could not express.HTTP needed no change. It builds a fresh
ContextServerper session over the same store, so a session started after an install already sees it. Only the long-lived stdio server needed the live notification.Tests
Five, against the watcher directly: a single change fires once, a burst collapses to one call, stopping prevents further calls, a throwing callback does not kill the watcher, and a missing directory is a no-op.
Verified discriminating rather than merely passing: removing the debounce fails three of the five.
The watcher went into its own module partly so it could be tested at all.
cli.tsbuilds acommanderprogram at module scope, so importing it from a test would run the CLI.About the local test run
pnpm lintandpnpm buildare clean.pnpm testreports 40 failures in my environment, identical before and after this change (I ran the baseline on a stash to be sure). They are allbetter-sqlite3@11.10.0failing to build against Node v26, so every sqlite-backed test errors on missing bindings:My five new tests pass, taking the suite from 181 to 186 passing. I could not verify the sqlite-backed suites locally, so those are worth a look on your CI rather than taking my word for it.
Housekeeping
Followed the
CLAUDE.mdconventions: searched open and recently closed PRs and/.plans/first (nothing related), and added a changeset. No plan file to delete, since there was none to claim.