chore: simplify pkg/devicescan#6603
Draft
njhale wants to merge 1 commit into
Draft
Conversation
The package was ~2,900 lines of orchestration machinery sitting on top
of ~1,000 lines of actual per-client knowledge. Two interfaces
(ClientScanner, PluginScanner), a scanState struct, a multi-phase
pipeline, options-struct helpers, and field-by-field map round-tripping
between parsed JSON and typed specs. This rewrite preserves the wire
shape (apiclient/types/devicescan.go is unchanged) but collapses the
internals into a flat orchestrator over per-client values.
Shape:
* One file per client, each exporting a single `var <client> = client{...}`
with name, presence signals, directRules, walkRules, walkSkipPrefixes.
* Two rule kinds:
- directRules: absolute paths stat'd once at scan start; parse runs
if the target exists (file or dir).
- walkRules: home-relative path suffixes matched during a single
filepath.WalkDir under $HOME; matches dispatch to parse.
* Parse functions are pure: (path) -> parseResult. No shared mutable
state, no scanState methods, no second interface for plugins.
* mergeResults() is a pure terminal step that dedups files, merges
clients, computes has_* rollups, synthesises orphan rows, and sorts.
* The orchestrator builds a set of directRule targets and uses it to
suppress walk re-dispatch when a walkRule's suffix would also match
a global config path (e.g. ~/.cursor/mcp.json).
Deleted machinery:
* scanner.go (ClientScanner/PluginScanner interfaces)
* walk.go (walkProject + projectHit dispatch)
* state.go (scanState, addFile, addClient)
* plugins.go (emitPlugin opts-struct, decodeMCPServerSpec round-trip,
pluginMCPServersBlock magic-field detection)
* hash.go (folded into mcp.go)
* frontmatter.go (replaced by a small local parser in skills.go)
* scanners_test.go, hermes_test.go, openclaw_test.go (replaced by a
focused devicescan_test.go covering codex/cursor/highest-version-dir)
Signed-off-by: Nick Hale <4175918+njhale@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR rewrites pkg/devicescan from interface/state-based scanners into a flat rule-driven orchestrator that scans real OS paths, parses per-client configs/plugins/skills, and merges results into the existing device scan manifest shape.
Changes:
- Replaces scanner interfaces/state helpers with
client,parseRule,parseResult, andmergeResults. - Reimplements per-client parsers for MCP configs, plugins, skills, presence, and file reading around absolute filesystem paths.
- Replaces broad scanner tests with focused parser/version tests and updates the CLI to call the new
devicescan.Scan(ctx, maxDepth)API.
Reviewed changes
Copilot reviewed 30 out of 30 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
pkg/devicescan/devicescan.go |
Adds the new orchestrator, client registry, walk logic, merge logic, and shared helpers. |
pkg/devicescan/claudecode.go |
Rewrites Claude Code config, project MCP, plugin, and skill scanning. |
pkg/devicescan/claudedesktop.go |
Rewrites Claude Desktop extension/config parsing and connector plugin emission. |
pkg/devicescan/codex.go |
Rewrites Codex config/plugin parsing and semver version selection. |
pkg/devicescan/cursor.go |
Rewrites Cursor MCP and plugin cache parsing. |
pkg/devicescan/gitconfig.go |
Switches git origin parsing to OS paths. |
pkg/devicescan/goose.go |
Rewrites Goose YAML config parsing. |
pkg/devicescan/hermes.go |
Rewrites Hermes YAML config parsing. |
pkg/devicescan/mcp.go |
Consolidates MCP spec, transport normalization, and hashing. |
pkg/devicescan/openclaw.go |
Replaces OpenClaw scanner with a presence-only client value. |
pkg/devicescan/opencode.go |
Rewrites OpenCode config, local plugin, and npm plugin parsing. |
pkg/devicescan/plugin.go |
Adds shared plugin manifest, nested MCP, and artifact scanning helpers. |
pkg/devicescan/presence.go |
Rewrites client presence detection around client values. |
pkg/devicescan/read.go |
Rewrites file and JSON/YAML/TOML reading helpers for OS paths. |
pkg/devicescan/skills.go |
Rewrites skill discovery, frontmatter parsing, and attribution. |
pkg/devicescan/vscode.go |
Rewrites VS Code MCP parsing. |
pkg/devicescan/windsurf.go |
Rewrites Windsurf MCP parsing. |
pkg/devicescan/zed.go |
Rewrites Zed settings and extension parsing. |
pkg/devicescan/walk.go |
Deletes the old glob-based project walker. |
pkg/devicescan/state.go |
Deletes the old shared scan state implementation. |
pkg/devicescan/scanner.go |
Deletes the old scanner interfaces and registry. |
pkg/devicescan/plugins.go |
Deletes the old plugin emission helpers. |
pkg/devicescan/frontmatter.go |
Deletes the old frontmatter parser. |
pkg/devicescan/hash.go |
Deletes the old hash helper after moving hashing into mcp.go. |
pkg/devicescan/scanners_test.go |
Deletes broad scanner smoke tests. |
pkg/devicescan/hermes_test.go |
Deletes Hermes-specific tests. |
pkg/devicescan/openclaw_test.go |
Deletes OpenClaw-specific presence tests. |
pkg/devicescan/devicescan_test.go |
Adds focused tests for version selection, Codex parsing, and Cursor parsing. |
pkg/cli/scan.go |
Updates CLI scan command to use the new devicescan API. |
go.mod |
Updates dependency classification after removing glob-based walking. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+41
to
+46
| // Empty when the stdlib call fails — paths built on top stat to ENOENT | ||
| // and silently skip. | ||
| var ( | ||
| homeDir, _ = os.UserHomeDir() | ||
| configDir, _ = os.UserConfigDir() | ||
| macAppsDir = "/Applications" |
Comment on lines
+162
to
+166
| if !dirExists(install.InstallPath) { | ||
| continue | ||
| } | ||
| manifestRel := path.Join(installRel, claudePluginManifestSub) | ||
| if !fileExists(s.fsys, manifestRel) { | ||
|
|
||
| manifestPath := filepath.Join(install.InstallPath, ".claude-plugin/plugin.json") |
| // maxDepth caps how deep the project walk descends from the home | ||
| // root. Direct paths are not subject to maxDepth — they always run | ||
| // regardless. | ||
| func Scan(ctx context.Context, maxDepth int) (types.DeviceScanManifest, error) { |
Comment on lines
+47
to
+57
| // Nil args collapses to []; map keys are sorted by json.Marshal. | ||
| func mcpConfigHash(name, transport, command string, args []string, url string) string { | ||
| if args == nil { | ||
| args = []string{} | ||
| } | ||
| data, _ := json.Marshal(map[string]any{ | ||
| "name": name, | ||
| "type": transport, | ||
| "command": command, | ||
| "args": args, | ||
| "url": url, |
| File: path, | ||
| Name: name, | ||
| Transport: "streamable-http", | ||
| URL: e.URL, |
| File: path, | ||
| Name: name, | ||
| Transport: "sse", | ||
| URL: e.URL, |
Comment on lines
16
to
20
| type mcpServerSpec struct { | ||
| Type string `json:"type" yaml:"type"` | ||
| Transport string `json:"transport" yaml:"transport"` | ||
| Command string `json:"command" yaml:"command"` | ||
| Args []string `json:"args" yaml:"args"` |
Comment on lines
+41
to
+45
| // configPath: first directRules target that exists. Provides a | ||
| // presence signal for GUI apps that aren't in $PATH (e.g. Claude | ||
| // Desktop on Linux). | ||
| var configPath string | ||
| for _, r := range c.directRules { |
Comment on lines
+3
to
+7
| // openclaw is presence-only: OpenClaw has no public config or plugin | ||
| // format we scan today. Presence is signalled by the `openclaw` binary | ||
| // in $PATH or the macOS app bundle. | ||
| var openclaw = client{ | ||
| name: "openclaw", |
Comment on lines
+369
to
+374
| out := types.DeviceScanManifest{ | ||
| Files: make([]types.DeviceScanFile, 0, len(files)), | ||
| Clients: make([]types.DeviceScanClient, 0, len(clients)), | ||
| MCPServers: mcps, | ||
| Skills: skills, | ||
| Plugins: plugins, |
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.
The package was ~2,900 lines of orchestration machinery sitting on top
of ~1,000 lines of actual per-client knowledge. Two interfaces
(ClientScanner, PluginScanner), a scanState struct, a multi-phase
pipeline, options-struct helpers, and field-by-field map round-tripping
between parsed JSON and typed specs. This rewrite preserves the wire
shape (apiclient/types/devicescan.go is unchanged) but collapses the
internals into a flat orchestrator over per-client values.
Shape:
var <client> = client{...}with name, presence signals, directRules, walkRules, walkSkipPrefixes.
if the target exists (file or dir).
filepath.WalkDir under $HOME; matches dispatch to parse.
state, no scanState methods, no second interface for plugins.
clients, computes has_* rollups, synthesises orphan rows, and sorts.
suppress walk re-dispatch when a walkRule's suffix would also match
a global config path (e.g. ~/.cursor/mcp.json).
Deleted machinery:
pluginMCPServersBlock magic-field detection)
focused devicescan_test.go covering codex/cursor/highest-version-dir)