Re-aim the CLI on a standalone local stack report - #10
Conversation
The scanner knew where a tool was configured (source path) but not which
AI-coding tool it belonged to, so nothing downstream could group by client.
Adds a `client` field alongside the existing local-only `source`/`scope`
metadata, set by each detector to a fixed literal.
`client` is never read out of a config file, so it cannot carry user data,
and it never reaches /api/sync — the payload is still {type, name} only.
The manifest-only-sync security test's key allowlist is updated to match.
Co-authored-by: omnigent <noreply@omnigent.ai>
`npx devcat-cli` with no arguments now scans this machine and prints the MCP servers and plugins installed across Claude Code, Codex, and Cursor, grouped by client with per-type counts. Local only: no network, no auth, no credentials touched, and an empty result exits 0 rather than erroring. `--markdown` emits the same scan as a "My AI stack" snippet for a README or gist. Both renderers are pure string transforms over DetectResult so the layout is asserted directly. Sync keeps its own subcommand and every flag it had; it just no longer holds the default slot. Co-authored-by: omnigent <noreply@omnigent.ai>
devcat.dev is being rebuilt, so sync cannot complete. Rather than open a browser, run a device flow, and fail deep inside an HTTP call, runSync now returns immediately with a single human-readable line naming the working alternative. No stack trace, no retries, exit 1 (and a sync.error event under --json). Nothing is deleted: the full device-flow and sync path below is untouched and still compiles. DEVCAT_SYNC_ENABLED=1 runs it — which is how the five existing sync integration suites keep covering the live path, and how a self-hosted DEVCAT_API_URL still works. Removing the gate restores sync. Co-authored-by: omnigent <noreply@omnigent.ai>
Rewrites the README around what the tool does without a backend: see your whole AI-coding stack in one command, with a one-line npx quickstart. The sample outputs are copied from real runs against a fixture home, so they match the code byte for byte. Says plainly that profile sync to devcat.dev is paused while the site is rebuilt and that it is coming back, rather than leading with a command that cannot currently work. Also documents two behaviours that were undocumented and surprise people: the report changes with the directory you run it in, and a tool configured twice is listed once. npm description and keywords follow the same positioning. Co-authored-by: omnigent <noreply@omnigent.ai>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
The report listed MCP servers and plugins but not the two things people actually accumulate most: Claude Code skills and subagent personas. Both are folders rather than config keys, so they need a directory scan. dirScan.ts keeps that scan narrow. It reads a known config root and its immediate children and never recurses, so it cannot wander out of the root by following a link. Symlinks are resolved to one canonical path — the ~/.claude/skills link farm is entirely symlinks — and that resolved path dedupes aliases pointing at the same skill. Broken links, unreadable directories, and entries that vanish mid-scan are skipped, and entry count per root is capped. Names come from the folder; no file is opened. Both Claude Code subagent shapes are handled: <name>.md and <name>/<name>.md. These are report-only. syncableTools() narrows detections to the mcp and plugin types the catalog matches, so the /api/sync payload is unchanged — and because the narrowed type is what postSync accepts, handing it an unfiltered list is now a compile error rather than a runtime surprise. Also fixes a mislabel this exposed: $HOME is an ancestor of most working directories, so the upward project walk was "finding" ~/.claude/skills, ~/.codex/config.toml and ~/.cursor/mcp.json and reporting the entire user shelf as project-scoped. Those paths already have a user-scope reader, so the project pass now skips them — nothing is detected less, it is just attributed correctly. ~/.mcp.json has no user-scope reader and is left as it was. Co-authored-by: omnigent <noreply@omnigent.ai>
`devcat --json` printed the human report, which broke anyone piping it to jq. It now emits one JSON object mirroring the report: totals, the project/user split, per-client groups with per-type names, and every path checked. One object rather than the newline-delimited event stream sync emits — this is a result, not a sequence of events. Sync's --json behaviour is untouched. --json wins when --markdown is passed too: a caller asking for machine-readable output is scripting, and a markdown document would break their parser. Co-authored-by: omnigent <noreply@omnigent.ai>
Sample outputs regenerated from a real run against a fixture home, so the report, the markdown snippet, and the JSON object all match the code byte for byte again. Documents what the directory scan does and does not do — shallow, no recursion, symlinks resolved, names from folder names, no file contents — plus the new $HOME-is-not-a-project rule, and states plainly that skills and subagents never enter the sync payload. Co-authored-by: omnigent <noreply@omnigent.ai>
The previous commit described this guard as covering all three detectors but only staged the Claude Code half. Without it, a walk started anywhere under $HOME still "finds" ~/.codex/config.toml and ~/.cursor/mcp.json and labels the user's own config project-scoped. Both paths are read by the user-scope pass already, so skipping them in the project pass detects nothing less — it attributes correctly. Co-authored-by: omnigent <noreply@omnigent.ai>
~/.codex/skills goes through the same bounded scanSkills pass as the Claude Code shelf: root plus immediate children, no recursion, symlinks resolved and aliases collapsed by canonical path, broken links and unreadable roots skipped, entry count capped. User scope only — Codex has no project skills root. On a machine where both shelves are link farms into one shared directory, two passes collapse the overlap and both are deterministic: aliases within a root collapse by resolved path, and across clients dedupe() keeps the first occurrence in a fixed scan order (project before user, Claude Code before Codex before Cursor). A skill on both shelves is therefore listed once under Claude Code, the same way on every run; a skill only Codex carries still appears under Codex. Co-authored-by: omnigent <noreply@omnigent.ai>
Adds ~/.codex/skills to what the CLI reads, and states the two-pass dedupe explicitly: aliases collapse by resolved symlink target within a directory, and across clients the first occurrence wins in a fixed scan order. That is what makes a shared shelf list once, under Claude Code, identically on every run. Co-authored-by: omnigent <noreply@omnigent.ai>
Review finding 1. syncableTools() filtered the runtime path, but postSync
still accepted the wide API ToolType — which includes 'skill' — so the
claimed compile-time guarantee did not exist. A future caller passing a raw
detection list would have compiled fine.
Adds SyncableToolType ('mcp' | 'plugin') and types the whole request path
with it: postSync and computeManifestHash. Passing detect().tools now fails
to compile, naming 'skill' as the culprit.
The security test no longer rebuilds the payload by hand. It runs the real
runSync against the fixture machine with msw intercepting, and asserts on
the bytes that actually reached the wire: no planted secrets, keys exactly
{manifest_hash, tools}, each entry exactly {type, name} — no source, scope,
client, or canonicalPath — and no skill or subagent anywhere in it.
Co-authored-by: omnigent <noreply@omnigent.ai>
Review finding 2. Canonical paths were resolved inside a single root and then thrown away; the global pass keyed on (type, name). That was wrong in both directions — two differently-named aliases of one canonical skill both survived, and two genuinely different skills sharing a name collapsed into one. Entries that are a thing on disk now carry canonicalPath, and dedupe() uses it as identity when present, falling back to (type, name) for config keys that have no path of their own. The two key spaces are prefixed so a path can never collide with a name. First-occurrence order is unchanged, so the documented scan order still decides attribution. Real effect on the development machine: tldraw-offline exists on both the Claude and Codex shelves as separate directories, not links to one target. Name identity collapsed them into a single entry; path identity correctly reports both. Co-authored-by: omnigent <noreply@omnigent.ai>
Review findings 3 and 4. The cap was applied before sorting, so which entries survived a large root depended on the order the filesystem happened to return — the cap bounded the work but not the result. Sorting now happens first, so the kept subset is the alphabetical head and is identical on every run. readdir still materializes the listing, which is unavoidable: choosing deterministically requires knowing every candidate name first. That is now stated in the code rather than left implicit, and the expensive per-entry work is what the cap actually limits. The folder-shaped subagent match accepted any visible .md inside a directory, so agents/reviewer/README.md became a subagent called 'reviewer'. The contract is <name>/<name>.md and it is now enforced exactly — which also removes the uncapped second readdir that check used, replacing it with a single stat for a known filename. Co-authored-by: omnigent <noreply@omnigent.ai>
Review finding 5. The tests called runReport() directly and faked argv, so commander never parsed anything — the default-command dispatch and the flag wiring were both untested, and `devcat --json` in particular depended on reading process.argv behind commander's back. Extracts the program into cli.ts so tests can run the real parser over real argv arrays. bin/devcat.ts is now the shim that turns the returned code into process.exit, the one part that cannot run in a test process. Actions set an exit code rather than calling process.exit themselves, and exitOverride() routes commander's own exits — bad flag, --help, --version — through the same return path instead of killing the process. The report command now takes `json` as an argument from commander rather than sniffing process.argv, so the flag arrives the same way whether it is given to the program or the subcommand. Covers all three modes end to end plus `report`, both --json positions, --json winning over --markdown, an unknown flag, and --help/--version. Co-authored-by: omnigent <noreply@omnigent.ai>
Review findings 7, 8, and 9. Names come from config keys and folder names, both of which can legally hold control characters — a directory named with an ANSI sequence could repaint the terminal report. Control characters are now stripped from the terminal and markdown renderings, and backticks additionally from markdown where they would break out of the inline code span. The JSON path needs nothing: stringify escapes them, and a test asserts the raw name survives there intact. pathsScanned holds directories as well as files now, so "config files checked" mislabelled them — it reads "locations checked". Adds the missing $HOME-guard regressions for Codex and Cursor. Only Claude Code had one, which is why that guard's other half once shipped in a commit that claimed to cover all three. Each asserts both halves: the user-level path is not claimed as project-scoped, and a genuine project-level config under $HOME still is. Co-authored-by: omnigent <noreply@omnigent.ai>
Review finding 6. "Config files and directory names only — never file contents" was false: the JSON and TOML configs are read and parsed in full. What is true is narrower and worth stating precisely, so the section is now a table of what is read where and how. - Config files ARE read and parsed; only the names inside them are kept. - Directory scans open nothing — not even the SKILL.md, whose presence is all that is checked. - Names come from the folder for skills and folder-shaped subagents, but from the FILE for bare <name>.md subagents, which the old blanket "names come from the folder name" got wrong. - The subagent folder shape requires a matching inner filename. The security bullets and the dedupe rules are restated to match the code as it now stands, and the sample output is regenerated from a real run. Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker A. Path-backed entries keyed on `path::${canonicalPath}` alone, so
one directory that legitimately represents two tools of different types —
a folder holding both a SKILL.md and a matching <name>.md, linked into the
skills root and the agents root — collapsed to a single entry and silently
lost the second.
Key is now `path::${type}::${canonicalPath}`. Same-path entries of the same
type still collapse; same-path entries of different types both survive.
The existing different-type test used path-less entries, so it exercised
the (type, name) fallback rather than this key. Adds one with two
path-backed entries sharing canonicalPath and differing in type, plus a
companion asserting same-type collapse still works.
Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker B, both halves.
The scan still called readdir(), which materializes an entire directory
before the 500 cap could apply — the cap bounded the result, not the work,
so a pathological root was unbounded in practice. Reading now streams via
opendir and stops at a READ_CEILING of 10,000 entries; the remainder is
never read. Dot-entries count toward the ceiling too, so a root full of
them cannot spin either. Selection stays deterministic: what was read is
sorted, then the first 500 get the per-entry filesystem work.
Neither bound may fire silently. A known-valid 501st tool missing while
every output claims completeness is precisely the claims-versus-reality
failure this review exists to end. When either bites, the scan returns a
RootTruncation, and it surfaces everywhere:
- stderr: one line per root, naming it with entries read and examined,
and saying when the read ceiling was the cause. Emitted in --json mode
too, so stdout stays parseable while the operator still learns.
- terminal and markdown: a short footnote that the list is incomplete.
- --json: a `truncated` flag plus a `truncations` array carrying root,
entries_seen, entries_kept, and hit_read_ceiling per root.
scanSkills/scanSubagents take an optional readCeiling so tests can reach
the ceiling path without creating ten thousand directories; production
always uses the default.
Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker C. The README said paths "never enter the report, the markdown,
the JSON" while renderStackJson deliberately emits paths_checked — visible
in the README's own JSON example — and entries carry source, scope, client,
and canonicalPath locally.
Re-audited every absolute claim against the code and narrowed each to what
is true:
- What is discarded is everything inside a parsed config except the
tool's name. That claim holds.
- Scan provenance — where the CLI looked — IS recorded and IS present in
local output: paths_checked, the roots named in truncation warnings,
and the locations listed in the empty-state report. Now stated plainly
rather than denied.
- The only thing transmitted remains {type, name} for mcp and plugin, by
sync alone, which the wire test asserts.
- "$HOME is never double-counted" was overbroad: it holds for the four
locations that have a user-scope reader, and ~/.mcp.json is genuinely
still found by the upward walk. Both stated.
Also documents the two scan bounds, their values, and the fact that
hitting either is disclosed rather than silent.
Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 1. The guard nulled the upward hit, but the project pass then fell back to join(cwd, '.claude', 'skills'|'agents') — which, run from the home directory, is that same guarded root. It got scanned as project scope, and project-first dedupe kept every entry that way. `cd ~ && npx devcat-cli` misattributed the entire user shelf. The fallback now carries the same guard as the hit, and the comparison is canonical rather than textual: realpath both sides, so `cd ~`, a symlinked $HOME, and a cwd reached through a symlink all resolve to the same place instead of slipping past string equality. isUserLevelPath is async for that reason; Codex and Cursor were audited and now use the same comparison. Neither had the fallback hole — both return early without reading — but both shared the string-equality weakness. A location skipped because it is the user root is not reported among the locations checked either; the user pass already names it. The existing tests all used directories nested BENEATH $HOME, so none of them could reach this. New ones run with cwd === homedir() for skills and subagents separately, through a symlinked route, across the whole scan, and for Codex and Cursor — plus one asserting a genuine project shelf inside $HOME is still project-scoped. Co-authored-by: omnigent <noreply@omnigent.ai>
…erently Blocker 2 plus residuals R1 and R2. `for await` fetches the next entry and only then runs the body, so a top-of-body ceiling check had already read one entry past the limit — the README's "reading stops there, the rest is never read" was false by one. The loop now drives the iterator manually and checks before each pull, then calls return() to close the handle. Off-by-one gone, promise now true. R1: entriesSeen counted only non-dot candidates while the ceiling counts every entry, so a dot-heavy root could report "0 entries read" beside "ceiling reached". RootTruncation now carries entriesRead — what the ceiling actually bounds — alongside entriesSeen, and the warning quotes entriesRead whenever the ceiling is the reason. R2: a directory erroring partway was swallowed, returning a short list with no truncation metadata at all, which reads exactly like a complete scan. That now sets readFailed and produces a truncation regardless of how few names were collected. The ceiling test no longer infers behaviour from output counts. It hands the scanner a fake directory whose async iterator counts next() calls and asserts the count exactly — the only way to tell "stopped at N" from "read N+1 and kept N". Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 3. Both renderers early-returned on "no tools" before reaching their footnote branch, so a truncated scan that surfaced nothing announced "No AI tooling detected" with no hint that it had stopped looking. That is the worst place to lose the notice: an empty result is exactly where a user concludes there is nothing installed. The empty state now carries the same footnote in terminal and markdown. Also updates the warning text for the richer counts: it quotes entriesRead when the ceiling fired, says so plainly when a read failed, and reports candidates and examined separately so the numbers cannot contradict. Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 4. The shim called process.exit() the moment runCli resolved. process.exit terminates immediately and discards whatever is still queued in stdout — and stdout is a pipe when piped, pipes are asynchronous, and a large --json report does not fit in one write. `devcat --json | jq` could therefore receive JSON cut mid-object together with exit 0. It now sets process.exitCode and returns, letting Node drain the stream and exit with the same code. Nothing holds the loop open, so there is no cost. The error path does the same rather than hard-exiting after a write. A true pipe test would tie the suite to dist/ existing and to build ordering, so instead the real shim module is imported with runCli mocked and asserted directly: process.exit is never called, exitCode carries the returned code including on rejection, and the whole payload reaches stdout before the shim resolves. The reasoning is recorded in the test file. Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 5 and residual R3. "every path checked" overstated the data. findUpwardDir stats a candidate at every ancestor while walking up, but only the resolved location is recorded — the intermediate probes are not. Kept the field name, since it is accurate for what it holds, and made the prose exact: paths_checked is the location each detector resolved to, one per config file or directory consulted, including candidates that turned out not to exist, and explicitly not a trace of the walk. R3: "the same machine always produces the same report" is only true below the read ceiling. Above it, which 10,000 entries were read is the directory's enumeration order, which the CLI does not control — sorting happens after. Now qualified, with the note that a run in that state says so rather than implying stability it does not have. Also documents the two new truncation fields and that a directory erroring partway is reported as truncated. Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 1. Attribution was already right, but the project pass still named the user file among the locations it checked: after the guard nulled the hit, scannedPath fell back to join(cwd, ...), which running from $HOME is that same user file. The user pass names it too and detect() concatenates without dedupe, so one location appeared twice — and the README's "one per config file or directory" was false. Both detectors now test the candidate rather than the hit, with the same canonical comparison, and bow out entirely when it resolves to the user location: no tools, no reported path. That mirrors what Claude's detector already does. An ordinary miss still reports the candidate it looked for, which is what the empty-state message is built from. Tests assert pathsScanned exactly, not just tools: the project pass reports nothing from $HOME for both detectors, a real project config is still reported, a miss still names its candidate, and a full scan from $HOME lists every location exactly once. Claude's detector is pinned by the same whole-scan assertion. Verified on this machine: run from $HOME, 9 locations, zero duplicates. Co-authored-by: omnigent <noreply@omnigent.ai>
Blocker 2 and residual R2.
Troubleshooting still told users the empty report "lists every path it
checked", contradicting the corrected paragraph above it that says the
upward walk's intermediate probes are not recorded. It now says what the
list is — the locations each detector resolved to — and points at that
paragraph.
Then a pass over every remaining every/always/never/all in the file,
checked against the code rather than against memory:
- "no file is opened, not even the SKILL.md" — true, presence is stat'd
- "directory scans open nothing at all" — true
- "every other field dropped at the parser" — true
- "a broken .mcp.json never fails the scan" — true
- "the rest is never read" — true since the ceiling gates before the pull
- "never double-counted as project config" — true, and now also of the
reported locations
- "never enter the sync payload" — true, enforced by type and wire test
- "makes no network request at all" — true
- "skill and subagent files are never opened at all" — true
Two did not survive intact. "stats a candidate at every ancestor directory"
overstated a walk that stops after 64 levels — now "each directory the walk
passes through". And R2's "far above any real config directory" traded an
unnecessary absolute for the actual figures.
Co-authored-by: omnigent <noreply@omnigent.ai>
Residual R1. cli.ts and cli.entrypoint.test.ts both still described bin/devcat.ts as turning the returned code into a process.exit call. It has assigned process.exitCode since the truncation fix; the comments now say so, and say why it matters — a hard exit after writing stdout can cut a piped report. Also records the one place the ceiling is deliberately conservative: a root holding exactly READ_CEILING entries is flagged truncated although nothing was missed. Detecting that would cost the extra read the gate exists to prevent, and over-disclosing never hides a tool. Co-authored-by: omnigent <noreply@omnigent.ai>
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb4307217f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { runSync } from './commands/sync.js'; | ||
| import { runLogout } from './commands/logout.js'; |
There was a problem hiding this comment.
Load authentication commands lazily for the local report
When the native @napi-rs/keyring binding cannot load—for example, on an unsupported Linux architecture or an installation where optional platform binaries were omitted—these eager imports load sync.ts/logout.ts and their keyring dependency before Commander can dispatch the new local-only report. Consequently even plain npx devcat-cli, which does not use authentication, exits during module initialization; dynamically importing the auth-backed handlers inside their command actions would keep the standalone report usable in those environments.
Useful? React with 👍 / 👎.
What
npx devcat-cliwith no arguments used to runsync, which needs devcat.dev — a site that currently 404s. The default is now a local scan of everything this machine has installed across Claude Code, Codex, and Cursor: MCP servers, plugins, skills, and subagents.No account, no sign-in, no network call. Three renderings of one scan: terminal,
--markdown,--json. On the development machine the report went from 52 tools to 120.Round-4 review — both blockers and both residuals fixed
1 — duplicate user location in
pathsScannedfrom$HOME. Attribution was already correct, but after the guard nulled the hit,scannedPathfell back tojoin(cwd, ...)— which, run from$HOME, is that same user file. The user pass named it too anddetect()concatenates without dedupe, so one location appeared twice and the README's "one per config file or directory" was false. Both detectors now test the candidate rather than the hit, with the same canonical comparison, and return no tools and no path when it resolves to the user location — mirroring what Claude's detector already did. An ordinary miss still reports the candidate it looked for, which is what the empty-state message is built from.Verified on this machine: run from
$HOME, 9 locations, zero duplicates (previously 11 with 2 dupes).2 — last contradictory README line. Troubleshooting said the empty report "lists every path it checked", contradicting the corrected paragraph above it. Reworded to name what the list actually is, pointing at that paragraph.
Then a full pass over every remaining
every/always/never/all, checked against code:SKILL.md"stat'd.mcp.jsonnever fails the scan"Two did not survive: "stats a candidate at every ancestor directory" overstated a walk that stops after 64 levels (now "each directory the walk passes through"), and R2's "far above any real config directory" traded an unnecessary absolute for the actual figures.
R1 — stale comments.
cli.tsandcli.entrypoint.test.tsstill described the shim as callingprocess.exit. It has assignedprocess.exitCodesince the truncation fix; both now say so and why.Left as directed: the conservative flag on an exactly-
READ_CEILINGroot (now carrying a one-line comment explaining why over-disclosing is the safe direction), and the "source is public" line.Skills and subagents never reach the server
Enforced by the type system —
postSynctakesSyncableToolType, so a raw detection list is a compile error naming'skill'. Proven on the wire: secrets planted throughout a fixture machine including inside aSKILL.mdbody and a subagent file, realrunSyncagainst an interceptor, assertions on received bytes.Tests
227 green (+116 over the branch point, +5 this round).
New:
neither REPORTS the user location from the project pass,a real project config under $HOME is still reported as a scanned location,a missing project config still reports the candidate it looked for,a full scan from $HOME lists every location exactly once,lists each location once from a directory nested under $HOME too.Gates
pwdasserted andpackage.jsonconfirmed asdevcat-clibefore every command:npm run lintclean ·npm run buildclean ·npm test227/227 ·npm pack --dry-run67 files,dist+ README + LICENSE, zerosrc/ortest/lines, no tarball written. Real-machine run from$HOME: 120 tools, 0 project-scoped, 9 locations, no duplicates. Not published — no version bump, no tag, no visibility change.Known gaps
SKILL.mdfrontmatter.Branch re-frozen for review as of this update — no further scope changes.