P0: add agent installer workflow#144
Conversation
lzehrung
force-pushed
the
p0/explore-facade
branch
from
5c1aa8e to
d19ac54
Compare
lzehrung
force-pushed
the
p0/agent-installer
branch
from
86a7083 to
5d8fca4
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| const detectedIds = detections | ||
| .filter((detection) => detection.detected) | ||
| .map((detection, index) => { | ||
| const definition = TARGET_DEFINITIONS[index]!; |
There was a problem hiding this comment.
CRITICAL: Wrong array index after filter() in auto-detection
detections.filter(...).map((detection, index) => ...) uses the index of the filtered array, not the original detections array. If some targets are not detected, TARGET_DEFINITIONS[index] will map to the wrong target. For example, if codex and cursor are detected but claude is not, the filtered index 1 maps to TARGET_DEFINITIONS[1] (claude) instead of TARGET_DEFINITIONS[2] (cursor).
| const definition = TARGET_DEFINITIONS[index]!; | |
| const definition = TARGET_DEFINITIONS[detections.indexOf(detection)]!; |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in 5eac588. Auto-detect now keeps detections aligned with their original target objects instead of indexing after filter(), and the regression covers detected Cursor with missing Claude.
| if (existing === null) return change(definition.id, "unchanged", configPath, dryRun); | ||
| const next = removeCodegraphConfig(definition, existing, configPath); | ||
| if (existing === next) return change(definition.id, "unchanged", configPath, dryRun); | ||
| const action = next.trim() ? "update" : "delete"; |
There was a problem hiding this comment.
WARNING: Empty JSON config files left behind after uninstall
For JSON configs, removeCodegraphConfig returns "{}\n" when the file only contained a Codegraph entry. next.trim() is "{}" (truthy), so the action is "update" instead of "delete". This leaves behind an empty config file instead of removing it, which is inconsistent with TOML uninstall behavior.
| const action = next.trim() ? "update" : "delete"; | |
| const action = next.trim() === "{}" ? "delete" : next.trim() ? "update" : "delete"; |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
There was a problem hiding this comment.
Fixed in 5eac588. JSON uninstall now renders an empty root object as empty content so removeConfig deletes the file, with a regression for Cursor-only config removal.
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous Review Summaries (7 snapshots, latest commit 20cc49e)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 20cc49e)Status: No Issues Found | Recommendation: Merge Files Reviewed (4 files)
Previous review (commit 932b406)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 6fa7ec4)Status: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous review (commit 3697365)Status: No Issues Found | Recommendation: Merge Files Reviewed (12 files)
Previous review (commit 733a0f9)Status: No Issues Found | Recommendation: Merge Files Reviewed (1 file)
Previous review (commit 5eac588)Status: No Issues Found | Recommendation: Merge Resolved Issues
Files Reviewed (3 files)
Previous review (commit 5d8fca4)Status: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Files Reviewed (12 files)
Reviewed by kimi-k2.6-20260420 · Input: 55.2K · Output: 2.5K · Cached: 102.3K |
There was a problem hiding this comment.
Pull request overview
Adds a new top-level installer workflow (codegraph install / codegraph uninstall) that can detect supported agent clients, write/remove Codegraph-owned MCP configuration + skill marker files, and documents the workflow across CLI help and docs.
Changes:
- Introduces an installer registry that models install targets (Codex/Claude/Cursor/Gemini/OpenCode/Agents), supports detect/print-config/dry-run/idempotent upserts, and scoped uninstall behavior.
- Wires new
install/uninstallcommands into the CLI (schemas, command dispatch, help text). - Adds end-to-end installer tests and updates documentation + bundled skill guidance.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/installer.test.ts | New tests covering detect, print-config, dry-run, idempotent install, scoped uninstall, and malformed JSON handling. |
| src/installer/registry.ts | Core installer target registry + read/merge/remove logic for TOML and JSON-based configs, plus skill marker handling. |
| src/cli/options.ts | Adds install/uninstall command schemas and registers --print-config as a value option. |
| src/cli/install.ts | CLI handler for install/uninstall implementing detect/print-config/install/uninstall flows. |
| src/cli/help.ts | Adds help text + examples for install/uninstall and registers them as known commands. |
| src/cli.ts | Routes install/uninstall to the new handler. |
| README.md | Documents the new installer workflow in “Agent setup”. |
| docs/mcp.md | Adds installer documentation to MCP docs. |
| docs/installation.md | Adds “Agent client setup” section describing installer usage. |
| docs/cli.md | Adds installer examples + a dedicated “Agent client installer” section. |
| docs/agent-workflows.md | Documents installer usage in agent workflows. |
| codegraph-skill/codegraph/SKILL.md | Updates bundled skill guidance to reference installer workflow. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const parsed = parseConfigJson(existing, configPath); | ||
| if (definition.kind === "json-opencode-mcp") { | ||
| parsed.mcp = mergeRecord(readRecordProperty(parsed, "mcp"), { | ||
| codegraph: { | ||
| type: "local", | ||
| enabled: true, | ||
| command: ["codegraph", "mcp", "serve", "--root", ".", "--stdio"], | ||
| }, | ||
| }); | ||
| } else { | ||
| parsed.mcpServers = mergeRecord(readRecordProperty(parsed, "mcpServers"), { | ||
| codegraph: { | ||
| type: "stdio", | ||
| command: "codegraph", | ||
| args: ["mcp", "serve", "--root", ".", "--stdio"], | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fixed in 5eac588. JSON install now preserves an existing non-matching codegraph entry instead of overwriting it, and the regression keeps custom command/args/env intact.
| function detectTarget(definition: TargetDefinition, options: InstallOptions): TargetDetection { | ||
| const settings = installerSettings(options); | ||
| const configPath = definition.configPath?.(settings); | ||
| const skillTargetDir = getSkillTargetDirForAgent(definition.id, settings.homeDir, settings.env); |
There was a problem hiding this comment.
Fixed in 5eac588. getSkillTargetDirForAgent("opencode") now honors XDG_CONFIG_HOME, so detection uses the same base path as the OpenCode config path.
| const settings = installerSettings(options); | ||
| const dryRun = options.dryRun ?? false; | ||
| const changes: InstallChange[] = []; | ||
| const skillTargetDir = getSkillTargetDirForAgent(definition.id, settings.homeDir, settings.env); |
There was a problem hiding this comment.
Fixed in 5eac588. OpenCode install now writes CODEGRAPH_INSTALLED under XDG_CONFIG_HOME/opencode/skills/codegraph when XDG_CONFIG_HOME is set; the regression asserts no marker is written under ~/.config.
| const settings = installerSettings(options); | ||
| const dryRun = options.dryRun ?? false; | ||
| const changes: InstallChange[] = []; | ||
| const skillTargetDir = getSkillTargetDirForAgent(definition.id, settings.homeDir, settings.env); |
There was a problem hiding this comment.
Fixed in 5eac588. OpenCode uninstall now removes the marker from the same XDG_CONFIG_HOME-derived path used during install; the regression covers the install/uninstall round trip.
| if (Object.keys(servers).length) { | ||
| parsed[property] = servers; | ||
| } else { | ||
| delete parsed[property]; | ||
| } | ||
| return `${JSON.stringify(parsed, null, 2)}\n`; | ||
| } |
There was a problem hiding this comment.
Fixed in 5eac588. Removing the last Codegraph-owned JSON entry now returns empty content, so the config file is deleted instead of left as {}; covered by a new uninstall regression.
| configPath: ({ env, homeDir }) => | ||
| path.join(env.XDG_CONFIG_HOME ?? path.join(homeDir, ".config"), "opencode", "opencode.json"), | ||
| }, |
There was a problem hiding this comment.
Addressed in 3697365. OpenCode install now trims XDG_CONFIG_HOME and falls back to ~/.config/opencode/opencode.json when it is unset or blank; the regression test covers the blank-env case.
| function assertWriteAllowed(options: InstallOptions): void { | ||
| if (options.dryRun) return; | ||
| if (options.yes) return; | ||
| throw new Error("Install writes require --yes. Use --dry-run or --print-config to inspect changes first."); |
There was a problem hiding this comment.
Addressed in 3697365. The shared write guard now reports the selected target path without referencing Claude-specific storage, so the message is correct for Codex, Gemini, OpenCode, and other targets.
| function isCodegraphJsonServer(value: unknown): boolean { | ||
| if (!isJsonRecord(value)) return false; | ||
| const command = value.command; | ||
| if (command === "codegraph") return true; | ||
| if (Array.isArray(command) && command[0] === "codegraph") return true; | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Addressed in 3697365. JSON-backed uninstall ownership now requires an exact installer-owned MCP server shape instead of a command prefix match, and the tests cover preserving user-modified commands, args, env, and non-object values.
| if (context.hasFlag("--detect")) { | ||
| context.writeJSONLine({ targets: await detectInstallTargets(options) }); | ||
| return; | ||
| } | ||
|
|
||
| if (context.command === "install") { | ||
| context.writeJSONLine(await installCodegraphTargets(options)); | ||
| return; |
There was a problem hiding this comment.
Addressed in 3697365. Installer commands no longer advertise or accept --json; help/docs/skill guidance now state installer reporting is text-only while analysis commands keep JSON output.
| export async function handleInstallerCommand(context: InstallerCommandContext): Promise<void> { | ||
| const printConfigTarget = context.getOpt("--print-config"); | ||
| const targetIds = parseInstallerTargets(context); | ||
| const options = { | ||
| ...(targetIds !== undefined ? { targetIds } : {}), | ||
| yes: context.hasFlag("--yes"), | ||
| dryRun: context.hasFlag("--dry-run"), | ||
| }; | ||
|
|
||
| if (printConfigTarget !== undefined) { | ||
| const targetId = parseInstallTargetId(printConfigTarget); | ||
| context.writeStdoutLine(printInstallConfig({ targetId }).trimEnd()); | ||
| return; | ||
| } | ||
|
|
||
| if (context.hasFlag("--detect")) { | ||
| context.writeJSONLine({ targets: await detectInstallTargets(options) }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Addressed in 6fa7ec4. install --print-config now fails fast with usage exit 2 when combined with --target, extra positional target selection, --detect, --yes, or --dry-run; tests cover each conflict.
| if (targetOpt !== undefined && positionalTarget !== undefined) { | ||
| throw new Error("Use either --target or a positional target, not both."); | ||
| } |
There was a problem hiding this comment.
Addressed in 6fa7ec4. Installer target-selection conflicts now go through the CLI usage-error path with stderr plus exit 2 instead of throwing; tests cover both install and uninstall with --target plus a positional target.
| const value = record[property]; | ||
| if (value === undefined) return {}; | ||
| if (isJsonRecord(value)) return value; | ||
| throw new Error(`Existing ${property} config must be a JSON object before codegraph can merge into it.`); |
There was a problem hiding this comment.
Addressed in 932b406. readRecordProperty now uses the generic message before codegraph can update it, which applies to both install merge and uninstall removal paths.
| function expectInstallerChange(changes: InstallChange[], expected: InstallChange): void { | ||
| expect(changes).toContainEqual(expected); | ||
| } |
There was a problem hiding this comment.
Addressed in 932b406. Installer test expectations now normalize expected InstallChange.path values to forward slashes before comparing, matching normalizePathForDisplay on Windows and POSIX.
| if (definition.kind === "skill-only") { | ||
| return `codegraph skill install --agent ${definition.id} --target ${normalizePathForDisplay(skillTargetDir)}\n`; | ||
| } |
There was a problem hiding this comment.
Addressed in 20cc49e. install --print-config agents now emits codegraph skill install --agent agents without --target, so the generated command no longer conflicts with skill install validation; a regression covers it.
| return { | ||
| detected: definition.kind === "skill-only" && pathExists(path.dirname(path.dirname(skillTargetDir))), | ||
| reason: `${definition.label} was not detected`, | ||
| ...(configPath !== undefined ? { configPath: normalizePathForDisplay(configPath) } : {}), | ||
| skillTargetDir: normalizePathForDisplay(skillTargetDir), | ||
| }; |
There was a problem hiding this comment.
Addressed in 20cc49e. Skill-only target detection now reports a positive base directory exists reason when ~/.agents makes the agents target detected; a regression asserts the reason is not contradictory.
| commandSchema(["--detect", "--dry-run", "--yes"], ["--print-config", "--target"], { | ||
| kind: "max", | ||
| max: 1, | ||
| usage: "Usage: codegraph install [target] [--target <ids>] [--yes | --dry-run] [--print-config <target>]", | ||
| }), |
There was a problem hiding this comment.
Addressed in 20cc49e. The install positional-argument usage string now includes --detect, and the installer regression covers the usage hint.
| commandSchema(["--detect", "--dry-run", "--yes"], ["--target"], { | ||
| kind: "max", | ||
| max: 1, | ||
| usage: "Usage: codegraph uninstall [target] [--target <ids>] [--yes | --dry-run]", | ||
| }), |
There was a problem hiding this comment.
Addressed in 20cc49e. The uninstall positional-argument usage string now includes --detect, with the same regression covering both install and uninstall usage hints.
| - `install` configures Codegraph-owned MCP entries, bundled skill payloads, and marker files for supported local agent clients: `codex`, `claude`, `cursor`, `gemini`, `opencode`, and `agents`. | ||
| - Writes require `--yes`; use `--dry-run` to preview changed file paths and actions or `--print-config <target>` to print a copyable MCP snippet without writing. | ||
| - `uninstall` removes only Codegraph-owned marker blocks, marker files, exact bundled skill payloads, or exact installer-owned MCP entries. | ||
| - `skill install` remains the lower-level primitive when you only want to copy the bundled skill directly without MCP config. |
There was a problem hiding this comment.
Addressed in 20cc49e. docs/cli.md now documents --detect in the Agent client installer section alongside --dry-run and --print-config.
| # Install or preview agent client integration | ||
| codegraph install --target codex,claude --dry-run | ||
| codegraph install --target codex,claude --yes | ||
| codegraph install --print-config codex | ||
| codegraph uninstall --target codex --yes | ||
| codegraph mcp --help |
There was a problem hiding this comment.
Addressed in 9b93153. codegraph mcp --help moved back under the MCP examples section; the installer examples section now ends with codegraph install --help.
Summary
installanduninstallcommands for supported agent targetsVerification
npm run checknode ./dist/cli.js review --base origin/main --head HEAD --summaryRebased onto
mainafter #143 merged.