Skip to content

feat(vnext): port the configured-server inventory capture - #18

Merged
mohit-gupta-glean merged 1 commit into
mainfrom
mohit/port-inventory-hook
Aug 20, 2026
Merged

feat(vnext): port the configured-server inventory capture#18
mohit-gupta-glean merged 1 commit into
mainfrom
mohit/port-inventory-hook

Conversation

@mohit-gupta-glean

Copy link
Copy Markdown
Contributor

Ports glean-plugins-vnext#53, continuing where #17 left off.

What

configuredServers was the last unreported field of the negotiation request — inventory() returned {source: "unavailable"} unconditionally. A SessionStart hook now runs claude mcp list once per session and leaves the result for the server to read.

Claude Code only, and this repo expresses that better than vnext does

Codex was wired identically in vnext — its codex mcp list --json parser worked against real output — and it never invokes the hook. Codex's own trace log, at DEBUG level and ~700k rows, mentioning "glean" 550 times so it is plainly tracking the plugin, contains zero occurrences of SessionStart, the hook filename, or the manifest, across a restart and four sessions. Not a sandbox and not a path problem. Both wirings were tried: the explicit hooks pointer in the plugin manifest, and the default ./hooks.json at the plugin root that appears as a literal in the Codex binary.

In vnext that meant tearing out Codex wiring by hand. Here the overrides/ layout already says it: the hook lives in overrides/claude/glean/hooks/, and pluginpack build puts it only in dist/claudedist/cursor and dist/codex get no hooks directory at all. Verified on a real build.

The Codex parser is preserved on mohit/inventory-codex-followup in vnext. The measurement that codex mcp list --json does not spawn servers also means an in-process background capture is safe there, needing no hook — the likelier follow-up.

Why a hook rather than a shell-out

claude mcp list health-checks every server, which spawns each stdio one — including this plugin — and the spawned copy goes on to serve a full tools/list with a live remote fetch. Calling the CLI from inventory() would recurse without bound, one process and one backend call per level. Confirmed by watching the spawned copy's own log.

So policy/inventory-cache.ts only ever reads a file, and its tests assert that rather than trusting the comment: every child_process entry point is replaced with one that records and throws, because loadCachedInventory catches everything to fail open and would otherwise swallow an attempt into a plausible unavailable.

The filter is the part to review hardest

Its failure mode is disclosing a customer's estate, not returning a wrong answer.

  • Glean's own domain only. Deriving the allowed set from the configured server URL would also cover white-labeled deployments, but matching a host is not fine-grained enough — several MCP servers may share one host under different paths, which is ordinary behind a corporate gateway, so it would report the customer's unrelated servers, origin and path.
  • stdio servers always withheld, including this plugin's own. They expose no URL, so they cannot be confirmed. Ours could be identified by launch path, but reporting it adds nothing: plugin.id and plugin.version ride the same request. Matching on the name stays rejected — a customer directory named for Glean would satisfy it.
  • URLs reduced to origin+path; withheld servers counted, never named.

Both rules are mutation-verified: widening exact-host to registrable-domain, and swapping the URL rule for a name check, each fail exactly the tests that name them.

Consequence worth deciding deliberately: with stdio withheld, a user whose only Glean MCP is the plugin gets servers: [] every time. The field carries Glean information only when a remote Glean server is separately configured — confirmed both ways on a real machine.

unavailable carries a reason

Fourteen paths led to unavailable with no diagnostic, so a fleet reporting mostly unavailable was unreadable. Four coarse codes now ride the request: capture-pending, cli-unavailable, cli-output-invalid, capture-invalid. Coarser than the branches behind them, since fourteen wire codes would freeze the implementation's shape into the contract; the detail goes to the local log. A closed set, never free text — an exec error would ship an absolute binary path, which on a normal install contains the user's name.

Porting notes

  • Most of the change landed on files identical to vnext at the feat(vnext): port capability policy and Cursor HITL updates #17 merge point, so they applied cleanly. src/index.ts and src/tools/run-tool.ts have diverged here, so only the data-dir change was merged into each by hand.
  • policy/enforce.ts is deliberately untouched, because this repo names the meta tool find_skills_and_tools rather than find_skills. A file copy would have silently reverted that.
  • Also ports the data-dir deduplication: <env> || ~/.glean appeared seven times across src/ in two variants that look identical and are not — server-only state keys off PLUGIN_DATA_DIR, hook-shared state must key off CLAUDE_PLUGIN_DATA, because a hook never sees the former. Both now live in src/data-dir.ts.
  • The hook test suite is skipped on win32, with the reason stated in the file. It spawns the real hook against a stubbed CLI and the stub is a /bin/sh script; the hook itself is cross-platform, and check:no-shell still passes because the guard reads first lines and the suite is a .ts file. CI runs ubuntu, so this only spares a Windows developer a failure that would look like a real defect.

Testing

typecheck:bundle, 321 tests, check:no-shell, build, and validate on all three targets. The built dist/claude/plugins/glean/hooks/ carries the hook and a hooks.json with both SessionStart and the existing PreToolUse; dist/cursor and dist/codex carry neither.

Verified end to end on a real Claude Code session in vnext, one process moving from miss to hit without restarting:

08:15:31 [18287] inventory.unavailable {"reason":"capture-pending", …}
08:15:34 [18289] inventory-hook  {"outcome":"host-cli","servers":1,"withheld":2}
08:21:36 [18287] inventory.resolved {"servers":1,"withheld":2}

Capture written at 0600 inside a 0700 directory with no filesystem path recorded, and the same pid changing its answer shows the cache is re-read per request rather than memoized.

Ports gleanwork/glean-plugins-vnext#53, continuing where #17 left off. `configuredServers`
was the last unreported field of the negotiation request: inventory() returned
{source: "unavailable"} unconditionally. A SessionStart hook now runs `claude mcp list` once
per session and leaves the result for the server to read.

Claude Code only, and that is not an omission. Codex was wired identically in vnext and its
`codex mcp list --json` parser worked against real output, but Codex never invokes the hook:
its own trace log -- DEBUG level, ~700k rows, mentioning "glean" 550 times -- contains zero
occurrences of SessionStart, the hook filename or the manifest, across a restart and four
sessions. Both wirings were tried, the explicit `hooks` pointer and the default
./hooks.json at the plugin root. This repo's overrides/ layout expresses that cleanly: the
hook lives under overrides/claude/ and the build puts it only in dist/claude, where cursor
and codex get no hooks directory at all.

Why a hook rather than a shell-out: `claude mcp list` health-checks every server, which
spawns each stdio one -- including this plugin -- and the spawned copy goes on to serve a
full tools/list with a live remote fetch. Calling the CLI from inventory() would recurse
without bound, one process and one backend call per level. So inventory-cache.ts only ever
reads a file, and its tests assert that rather than trusting the comment.

The filter is the part to review hardest, because its failure mode is disclosing a
customer's estate: Glean's own domain only (deriving the allowed set from the configured URL
would admit anything else fronted off the same host under a different path, which is
ordinary behind a corporate gateway), stdio servers always withheld including this plugin's
own, URLs reduced to origin+path, withheld servers counted and never named. Both rules are
mutation-verified.

`unavailable` now carries one of four coarse reasons so an expected absence is
distinguishable from a broken capture -- a closed set, never free text, since an exec error
would ship a binary path containing the user's name.

Porting notes:

- Most of the diff landed on files identical to vnext at the #52 merge, so those applied
  cleanly. index.ts and tools/run-tool.ts have diverged in this repo, so only the data-dir
  change was merged into each by hand.
- policy/enforce.ts is untouched, which matters because this repo names the meta tool
  find_skills_and_tools rather than find_skills -- a file copy would have reverted that.
- The hook test suite is skipped on win32 with a stated reason. It spawns the real hook
  against a stubbed CLI, and the stub is a /bin/sh script; the hook itself is
  cross-platform. CI runs ubuntu, so this only spares a Windows developer a failure that
  would look like a real defect.

Verified end to end on a real Claude Code session in vnext: one process moved from
capture-pending to inventory.resolved {"servers":1,"withheld":2} without restarting, with
the capture at 0600 inside a 0700 directory and no filesystem path recorded. Here:
typecheck, 321 tests, check:no-shell, build, and validate on all three targets.
@mohit-gupta-glean
mohit-gupta-glean merged commit 46587b7 into main Aug 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants