feat(vnext): port the configured-server inventory capture - #18
Merged
Conversation
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.
eshwar-sundar-glean
approved these changes
Aug 20, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports glean-plugins-vnext#53, continuing where #17 left off.
What
configuredServerswas the last unreported field of the negotiation request —inventory()returned{source: "unavailable"}unconditionally. ASessionStarthook now runsclaude mcp listonce 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 --jsonparser 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 ofSessionStart, 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 explicithookspointer in the plugin manifest, and the default./hooks.jsonat 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 inoverrides/claude/glean/hooks/, andpluginpack buildputs it only indist/claude—dist/cursoranddist/codexget no hooks directory at all. Verified on a real build.The Codex parser is preserved on
mohit/inventory-codex-followupin vnext. The measurement thatcodex mcp list --jsondoes 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 listhealth-checks every server, which spawns each stdio one — including this plugin — and the spawned copy goes on to serve a fulltools/listwith a live remote fetch. Calling the CLI frominventory()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.tsonly ever reads a file, and its tests assert that rather than trusting the comment: everychild_processentry point is replaced with one that records and throws, becauseloadCachedInventorycatches everything to fail open and would otherwise swallow an attempt into a plausibleunavailable.The filter is the part to review hardest
Its failure mode is disclosing a customer's estate, not returning a wrong answer.
plugin.idandplugin.versionride the same request. Matching on the name stays rejected — a customer directory named for Glean would satisfy it.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.unavailablecarries a reasonFourteen paths led to
unavailablewith no diagnostic, so a fleet reporting mostlyunavailablewas 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
src/index.tsandsrc/tools/run-tool.tshave diverged here, so only the data-dir change was merged into each by hand.policy/enforce.tsis deliberately untouched, because this repo names the meta toolfind_skills_and_toolsrather thanfind_skills. A file copy would have silently reverted that.<env> || ~/.gleanappeared seven times acrosssrc/in two variants that look identical and are not — server-only state keys offPLUGIN_DATA_DIR, hook-shared state must key offCLAUDE_PLUGIN_DATA, because a hook never sees the former. Both now live insrc/data-dir.ts./bin/shscript; the hook itself is cross-platform, andcheck:no-shellstill passes because the guard reads first lines and the suite is a.tsfile. 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, andvalidateon all three targets. The builtdist/claude/plugins/glean/hooks/carries the hook and ahooks.jsonwith bothSessionStartand the existingPreToolUse;dist/cursoranddist/codexcarry neither.Verified end to end on a real Claude Code session in vnext, one process moving from miss to hit without restarting:
Capture written at
0600inside a0700directory with no filesystem path recorded, and the same pid changing its answer shows the cache is re-read per request rather than memoized.