feat(#1451): wire gsd capability install/update/remove/list/disable/enable management CLI (#1457)

* feat(#1451): wire gsd capability install/update/remove/list/disable/enable CLI

ADR-1244 D5/D6: the management command was built as a library (capability-lifecycle.cjs
install/upgrade/remove + capability-ledger.cjs) across Phases 3-5 but never wired to a
user-facing command — gsd-tools.cjs 'capability' only handled state/set. This adds the
six subcommands, dispatching to the existing lifecycle/ledger:

- install <spec> [--integrity] [--scope global|project] [--yes] [--shared-file <rel>]…
- update [<id>|--all] [--scope] [--yes] [--shared-file]  (re-resolves recorded source)
- remove <id> [--purge-data] [--scope]  (first-party rejected)
- list [--json]  (first-party + overlay, both scopes, JSON array)
- disable|enable <id>  (activation-state alias of capability set --off/--on)

Scope→runtimeDir mapping matches capability-loader exactly (global=$GSD_HOME||home,
project=project root; caps at <root>/.gsd/capabilities/<id>, ledger at <root>/.gsd-capabilities.json).
Consent is non-interactive: --yes grants; without it an executable install aborts after
printing the disclosure and writes nothing. Best-effort reconcile before each mutation.

Tests: tests/capability-cli.test.cjs (20 behavioral, real resolver via local specs,
GSD_HOME-sandboxed) — install consent/block/usage matrix, list, update round-trip,
remove round-trip + first-party guard, disable/enable, unknown subcommand.
Docs: docs/reference/gsd-capability-command.md reconciled to the real surface
(ledger paths, --shared-file, consent model, disable mechanism, outdated marked planned);
docs/COMMANDS.md gains the gsd capability entry.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(#1451): resolve adversarial-review findings + root-cause the --raw silent-output bug

Adversarial-review (Codex) fixes:
- capReadStrict passes a malformed strict_known_registries value THROUGH so the trust gate
  fail-closes on it (was silently downgrading to permissive)
- installCapability/upgradeCapability gain an expectedId guard + first-party-id rejection
  (capability-lifecycle.cts): an overlay can't shadow a first-party id, and 'update <id>' can't
  act on a different id if the recorded source was retargeted
- capability update: prints the consent disclosure, exits non-zero on --all partial failure,
  no longer masks the resolved id
- capability remove: ledger-first ordering so an overlay is removable even if it shadows a
  first-party name; first-party guard only fires for ids not in the ledger
- gsd-capability-command.md: disable/enable doc corrected (registry-known ids; overlay toggle
  not yet wired through this path)

Silent-output bug (root cause, not waved off as pre-existing):
- captureStdoutSyncWrites buffered fd-1 output and DISCARDED it on the throw path — any --raw
  command that emitted a result/error envelope then threw (to set a non-zero exit) lost ALL of
  stdout. Now it flushes the captured buffer before re-throwing (exit code preserved).
- cmdCapabilitySet threw via process.exit() (bypassing the capture wrapper entirely); now throws
  ExitError so the wrapper flushes — matches the repo's no-process-exit architecture.
- Regression test: capability disable <unknown> --raw must emit the JSON error envelope on stdout.

Verified: capability suite 165/165, @file/json-errors/phase 183/183, lint clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(#1451): address adversarial-review R2 — shared-file confinement, MCP no-clobber, config fail-closed

- confinedSharedFile(): realpath-confine every shared-config write/strip to the scope root (mirrors
  safeRmUnder), so a --shared-file whose parent is a symlink escaping the scope can't write outside it.
- mcpServers shared edits: never overwrite an UNOWNED entry — a name collision with the user's (or
  another capability's) server is skipped, so install/remove can't silently clobber user MCP config
  (hooks already append; the map-keyed mcpServers path was the gap).
- capReadStrict: a PRESENT-but-unparseable .planning/config.json now fails CLOSED (lockdown) instead
  of silently downgrading the strict_known_registries policy to permissive.
- Tests: symlink-escape shared-file writes nothing outside scope; colliding user mcpServers entry
  preserved; unparseable config blocks an external install. capability suite 83/83, lint clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* fix(#1451): address code-review — aborted-status robustness + coverage + project-scoped strict doc

- install/update: handle an 'aborted' result independently of the requiresConsent flag so it can never
  fall through to the generic 'blocked: unknown reason' arm (aborted always means consent-needed per
  the lifecycle contract; latent today, hardened for future status additions).
- Clarify capResolveScope comment (project scope === already-resolved cwd) and document that
  strict_known_registries is a PROJECT-scoped policy (read regardless of --scope; no machine-wide
  allowlist) in gsd-capability-command.md.
- Tests: update --all over an empty ledger returns an empty result set (exit 0); a flag value that
  looks like another flag (--integrity --scope) is rejected, not swallowed. CLI suite 33/33, lint clean.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(#1451): FEATURES.md entry #147 + Added/Fixed changesets for the capability CLI

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(#1451): backfill changeset PR number → #1457

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-06-19 12:02:36 -04:00
committed by GitHub
parent 1abebbf4fd
commit 34bc096ec2
9 changed files with 989 additions and 105 deletions

View File

@@ -109,6 +109,12 @@ interface LifecycleOptions {
execOverrides?: Record<string, unknown>;
/** Also delete CAPABILITY_DATA on remove (default false — data is preserved/prompted). */
removeData?: boolean;
/**
* When set, the resolved capability id MUST equal this or the operation is refused with NO writes.
* `gsd capability update <id>` passes the requested id so a source that has been retargeted or
* hand-edited to a different manifest id cannot silently act on (and overwrite) another capability.
*/
expectedId?: string;
/**
* Test seam: override the source resolver. Must honor promote:false semantics — return a
* staged dir (left on disk for the caller to promote/clean). Defaults to the real resolver.
@@ -276,6 +282,34 @@ function safeRmUnder(runtimeDir: string, rel: string): boolean {
}
}
/**
* Resolve a shared-config file path RELATIVE to runtimeDir, confined to the scope root by realpath
* (mirrors safeRmUnder). Rejects absolute paths, `..`, and any relFile whose existing parent
* directory is a symlink escaping runtimeDir — so `--shared-file evil/x.json`, where `evil` is a
* pre-planted symlink pointing outside the scope, can never write outside it. Returns the safe
* absolute path, or null when the path is unsafe.
*/
function confinedSharedFile(runtimeDir: string, relFile: unknown): string | null {
if (typeof relFile !== 'string' || !relFile || path.isAbsolute(relFile) || relFile.split(/[/\\]/).includes('..')) {
return null;
}
let realRoot: string;
try { realRoot = fs.realpathSync(runtimeDir); } catch { return null; }
const target = path.resolve(realRoot, relFile);
const parentDir = path.dirname(target);
let realParent: string;
try {
realParent = fs.realpathSync(parentDir);
} catch {
// Parent does not exist yet (created inside the scope on write): a non-existent path cannot be a
// symlink escaping the root, so a lexical containment check is sufficient.
if (parentDir !== realRoot && !parentDir.startsWith(realRoot + path.sep)) return null;
return target;
}
if (realParent !== realRoot && !realParent.startsWith(realRoot + path.sep)) return null;
return path.join(realParent, path.basename(target));
}
// ---------------------------------------------------------------------------
// Atomic directory promotion (stage -> swap, backup retained for the caller)
// ---------------------------------------------------------------------------
@@ -393,10 +427,8 @@ function applyCapabilitySharedEdits(args: {
if (hooks.length === 0 && mcpEntries.length === 0) return records;
for (const relFile of sharedFiles) {
if (typeof relFile !== 'string' || !relFile || path.isAbsolute(relFile) || relFile.split(/[/\\]/).includes('..')) {
continue;
}
const file = path.join(runtimeDir, relFile);
const file = confinedSharedFile(runtimeDir, relFile);
if (file === null) continue; // unsafe path (absolute / .. / symlink escaping the scope root)
const settings = readJsonFile(file) ?? {};
let touched = false;
@@ -427,6 +459,14 @@ function applyCapabilitySharedEdits(args: {
: {};
for (const { name, config } of mcpEntries) {
if (!name || isUnsafeKey(name)) continue;
// Marker isolation for the map-keyed mcpServers shape: only (re)write an entry we already own
// or a brand-new name. A collision with an UNOWNED entry (the user's, or another capability's)
// is SKIPPED so user config is never clobbered — hooks are arrays and append, but mcpServers is
// keyed by name, so a blind overwrite would silently destroy the existing server config.
const existing = mcpObj[name];
const ownedByUs = typeof existing === 'object' && existing !== null
&& (existing as Record<string, unknown>)[CAP_MARKER] === capId;
if (existing !== undefined && !ownedByUs) continue;
const stamped = (typeof config === 'object' && config !== null && !Array.isArray(config))
? { ...(config as Record<string, unknown>), [CAP_MARKER]: capId }
: { value: config, [CAP_MARKER]: capId };
@@ -458,8 +498,8 @@ function stripCapabilitySharedEdits(args: {
let stripped = 0;
for (const edit of sharedEdits) {
const relFile = edit && typeof edit.file === 'string' ? edit.file : '';
if (!relFile || path.isAbsolute(relFile) || relFile.split(/[/\\]/).includes('..')) continue;
const file = path.join(runtimeDir, relFile);
const file = confinedSharedFile(runtimeDir, relFile);
if (file === null) continue; // unsafe path (absolute / .. / symlink escaping the scope root)
const settings = readJsonFile(file);
if (settings === null) continue; // missing/unparseable — nothing to strip
let changed = false;
@@ -502,6 +542,22 @@ function stripCapabilitySharedEdits(args: {
return stripped;
}
/**
* Is `id` a first-party capability id (present in the committed registry)? First-party always wins,
* so an overlay reusing one of these ids — even a non-reserved name like "ui" — must be refused at
* install (the loader would skip it at load anyway; rejecting here avoids writing an inert, shadowing
* bundle). Fail-open to `false` if the registry cannot be read (the reserved-prefix gate still applies).
*/
function isFirstPartyCapabilityId(id: string): boolean {
try {
// eslint-disable-next-line @typescript-eslint/no-require-imports
const reg = require('./capability-registry.cjs') as { capabilities?: Record<string, unknown> };
return !!(reg && reg.capabilities && Object.prototype.hasOwnProperty.call(reg.capabilities, id));
} catch {
return false;
}
}
// ---------------------------------------------------------------------------
// Install
// ---------------------------------------------------------------------------
@@ -559,6 +615,12 @@ async function installCapability(spec: string, opts: LifecycleOptions): Promise<
if (manifest === null) {
return { status: 'blocked', blockReasons: ['staged capability.json is missing or invalid'] };
}
if (opts.expectedId && resolved.id !== opts.expectedId) {
return { status: 'blocked', id: resolved.id, blockReasons: [`source resolved to capability id "${resolved.id}" but "${opts.expectedId}" was expected; refusing`] };
}
if (isFirstPartyCapabilityId(resolved.id)) {
return { status: 'blocked', id: resolved.id, blockReasons: [`"${resolved.id}" is a first-party capability id and cannot be overridden by a third-party overlay`] };
}
const verdict = trustMod.evaluateInstallTrust({
parsed: parsedPre,
@@ -697,6 +759,9 @@ async function upgradeCapability(spec: string, opts: LifecycleOptions): Promise<
if (!lock) {
return { status: 'blocked', id: resolved.id, blockReasons: ['another capability operation is in progress'] };
}
if (opts.expectedId && resolved.id !== opts.expectedId) {
return { status: 'blocked', id: resolved.id, blockReasons: [`source for "${opts.expectedId}" now resolves to a different capability id "${resolved.id}"; refusing to upgrade`] };
}
const existing = ledgerMod.readLedger(runtimeDir);
const prior = existing && Object.prototype.hasOwnProperty.call(existing.entries, resolved.id)
? existing.entries[resolved.id]

View File

@@ -24,7 +24,14 @@
// eslint-disable-next-line @typescript-eslint/no-require-imports
import ioMod = require('./io.cjs');
const { output: coreOutput, error: coreError } = ioMod;
const { output: coreOutput } = ioMod;
// ExitError (NOT process.exit) is how every gsd-tools command signals a non-zero exit: runMain
// translates it to process.exitCode so buffered stdout flushes first. Calling process.exit() here
// truncates a just-written --raw JSON payload before the reader sees it (a real silent-output bug).
// eslint-disable-next-line @typescript-eslint/no-require-imports
import cliExitMod = require('./cli-exit.cjs');
const { ExitError } = cliExitMod;
// eslint-disable-next-line @typescript-eslint/no-require-imports
import capabilityStateMod = require('./capability-state.cjs');
@@ -444,7 +451,8 @@ function cmdCapabilitySet(
// Do NOT print human stderr lines — raw consumers parse the JSON.
coreOutput({ capabilities: result.capabilities, warnings: result.warnings, errors: result.errors }, true);
if (result.errors.length > 0) {
process.exit(1);
// Throw (don't process.exit) so the JSON written just above flushes before the process ends.
throw new ExitError(1);
}
return;
}
@@ -457,10 +465,12 @@ function cmdCapabilitySet(
process.stderr.write(`capability set: error: ${e}\n`);
}
// Exit non-zero if any errors (hard failures — requested action was not realized).
// Exit non-zero if any errors (hard failures — requested action was not realized). The per-error
// lines were already written to stderr above; signal the exit code via ExitError (not process.exit)
// so any pending stdout/stderr flushes — runMain maps it to process.exitCode.
if (result.errors.length > 0) {
coreError(`capability set: ${String(result.errors.length)} error(s) — see above`);
return; // unreachable — coreError calls process.exit(1)
process.stderr.write(`Error: capability set: ${String(result.errors.length)} error(s) — see above\n`);
throw new ExitError(1);
}
// Human-readable summary: focus on the target capability