* test(#2329): fail-first tests for opencode commands/ (plural) command dir Red phase, empirically probed: global/local install lands in command/ (singular) with 71 gsd-*.md files and no commands/; the manifest records 71 keys under command/ and zero under commands/; all four declaring sites report 'command'. Migration coverage is black-box (two sequential install runs against one configDir) so it holds regardless of how the fix implements cleanup. The Kilo guard passes today by design — a forward-looking no-collateral check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * fix(#2329): write opencode commands to commands/ (plural), migrate legacy command/ OpenCode discovers slash commands from commands/ (plural); the installer wrote them to command/ (singular), so none of the ~71 /gsd-* commands appeared in the TUI. Five sites declared the directory and all had to agree: - capabilities/opencode/capability.json: both artifactLayout destSubpath entries (global + local) and hostBehaviors.flatCommandDir - bin/install.js: the manifest prefix was a SEPARATE hardcoded 'command/' literal, so the manifest would have diverged from the descriptor even after a rename. It now derives from _hostBehaviors(runtime).flatCommandDir. - src/install-engine.cts installOpencodeFamilyArtifacts: the actual write target, which bypasses resolveRuntimeArtifactLayout via combinedFamilyInstall. This was a fifth site the issue did not list — without it the descriptor change alone would not have moved a single file. Migration: an upgrade over a pre-fix install removes only manifest-proven GSD-managed files from the legacy command/ dir and rmdirs it once empty. Unmanifested user files are preserved, never deleted. Kilo shares the opencode family install path and is explicitly unaffected — pinned by a no-collateral test. Note on the tests: the migration cases originally built their legacy fixture by running the installer and relying on it to produce command/ — i.e. they depended on the bug to set up the fixture, and became unsatisfiable the moment it was fixed (block 1 requires command/ to be absent after a fresh install). They now fabricate the legacy layout explicitly, including rewriting the manifest keys to the command/ prefix — which is load-bearing, since the migration only removes manifest-proven files and an unrewritten fixture would silently no-op and pass even against a broken migration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * test(#2329): regenerate opencode install golden after rebase onto next The golden conflicted on rebase because #2322 also regenerated it. Resolved by regenerating from the merged source rather than hand-merging a generated file; the only delta is the 71 command/gsd-*.md -> commands/gsd-*.md key renames. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * test(#2329): update stale tests that pinned opencode's singular command/ dir Seven tests encoded the old contract (opencode: command/gsd-help.md exists, the descriptor's flatCommandDir, the install-integration contract, and the resolveRuntimeArtifactLayout golden). They passed in the red phase precisely because they pinned the buggy singular dir; the fix intentionally changes that contract, so these are stale-test corrections, not regressions. Kilo shares the opencode family install path and is deliberately NOT changing — it stays on command/ (singular). The shared opencode/kilo test is now split via an explicit per-runtime dir map so the two cannot be conflated, and Kilo's own layout test is untouched. tests/opencode-command-dir-plural.test.cjs independently pins Kilo unchanged end-to-end. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * docs(#2329): changeset for opencode commands/ dir fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * docs(#2329): backfill PR number 2354 into changeset Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * docs(#2329): correct the changeset — do not assert opencode ignores command/ The changeset repeated the issue's stated mechanism ("OpenCode discovers them from commands/ ... a clean install produced no usable commands in the TUI at all"). OpenCode's source contradicts that: packages/core/src/v1/config/command.ts globs {command,commands}/**/*.md, so BOTH names resolve, and its own skill doc still calls .opencode/command/ typical. Shipping that claim as a release note would document a mechanism that does not exist. The change is still right, for the stronger reason: OpenCode's config docs list plural as the convention and singular as backwards compatibility, so GSD was shipping on the alias the vendor may withdraw. Reworded to describe it as the alignment it is, decided on OpenCode's source and docs rather than on bug reports in either repo. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * fix(#2329): baseline opencode's commands/ surface — closes a data-loss path this PR opened Not a bookkeeping gap. Moving opencode's command dir to commands/ moved the install destination to a surface the first-time baseline scan does not cover: 000-first-time-baseline's RUNTIME_SURFACES.opencode lists ['gsd-core','command', 'skills','agents'] — no 'commands'. installOpencodeFamilyCommands unconditionally unlinks every gsd-*.md under its destination before writing the fresh set (install-engine.cts:870-873), with zero manifest or migration involvement. The only thing that protects a pre-existing file is assertInstallerMigrationsUnblocked, which runs before materialization and halts when the baseline scan flags an unknown file at a KNOWN surface. Probed: a pre-existing commands/gsd-plan.md is silently destroyed (install exits 0). The identical file under the legacy, already-baselined command/ surface correctly halts the install with "installer migration blocked pending user choice". So this PR would have traded a protected surface for an unprotected one. Fixed with a NEW fix-forward migration rather than editing 000, per docs/installer-migrations.md:131-134 — an applied migration never re-runs, so editing 000 would only protect fresh installs and leave every existing machine exposed. A new id runs for both populations and drifts no shipped checksum; adding its entry to EXPECTED_CHECKSUMS is the case that test explicitly sanctions. All five pre-existing shipped checksums verified byte-identical. Kilo is excluded by the migration's runtimes filter and keeps command/. This was previously deferred as a PR-body note claiming "low impact — nothing else acts on baseline-scan misses". That claim was never probed and was wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA * docs(#2329): drop the parenthetical product description from the changeset The product-name purity guard (#1777) rejects "Kilo (which still uses command/)" — fragment prose renders verbatim into CHANGELOG.md, so a product name must not carry a parenthetical. Reworded to a plain sentence; the meaning is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SLufH5sDuqA1AiEGu45cuA --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
@@ -27,6 +27,7 @@ import runtimeArtifactLayout = require('./runtime-artifact-layout.cjs');
|
||||
import runtimeArtifactInstallPlan = require('./runtime-artifact-install-plan.cjs');
|
||||
import runtimeNamePolicy = require('./runtime-name-policy.cjs');
|
||||
import installProfiles = require('./install-profiles.cjs');
|
||||
import installerMigrations = require('./installer-migrations.cjs');
|
||||
import { posixNormalize } from './shell-command-projection.cjs';
|
||||
|
||||
const { processAttribution } = runtimeArtifactConversion;
|
||||
@@ -620,6 +621,10 @@ function installRuntimeArtifacts(
|
||||
// previously lived inline in bin/install.js.
|
||||
const behaviors = _hostBehaviors(runtime);
|
||||
if (behaviors.combinedFamilyInstall) {
|
||||
// #2329: combined-family runtimes (OpenCode/Kilo) bypass
|
||||
// _runLegacyInstallMigrations below entirely (early return), so their
|
||||
// legacy-directory cleanup needs its own pre-materialization hook here.
|
||||
_migrateLegacyOpencodeCommandDir(runtime, configDir, behaviors);
|
||||
installOpencodeFamilyArtifacts(runtime, configDir, scope, resolvedProfile, resolveAttribution, behaviors);
|
||||
return;
|
||||
}
|
||||
@@ -937,6 +942,76 @@ function _installNativePluginIfDeclared(
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// _migrateLegacyOpencodeCommandDir
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/**
|
||||
* #2329: migrate a pre-fix OpenCode install's legacy singular `command/`
|
||||
* command directory into the current descriptor-driven destination (plural
|
||||
* `commands/` for OpenCode — the dir OpenCode actually discovers slash
|
||||
* commands from; unaffected for Kilo, whose descriptor still declares
|
||||
* `command`, so `currentName === LEGACY_NAME` short-circuits below).
|
||||
*
|
||||
* Runs BEFORE materialization writes the fresh command set to the new
|
||||
* location (mirroring `_runLegacyInstallMigrations`'s ordering for the
|
||||
* generic branch, which combined-family runtimes otherwise skip entirely).
|
||||
*
|
||||
* Ownership safety mirrors installer-migrations 003
|
||||
* (rename-get-shit-done-to-gsd-core): only files present, and unchanged or
|
||||
* locally modified, in the PRIOR install manifest under the legacy
|
||||
* `command/<file>` key are removed here — the materialization call
|
||||
* immediately following writes the current command set fresh into the new
|
||||
* location, so removing the stale copies is safe. Anything not proven
|
||||
* manifest-managed (unrelated user content someone dropped into `command/`)
|
||||
* is left untouched, never deleted. The emptied legacy directory is removed
|
||||
* only once nothing else is left inside it.
|
||||
*
|
||||
* Implemented as inline pre-materialization cleanup rather than a
|
||||
* `src/installer-migrations/*.cts` record: the formal migrations framework
|
||||
* only ever DELETES individual files (never directories, and never a
|
||||
* relocate/move primitive — see docs/installer-migrations.md's Action
|
||||
* Types), so the empty-directory removal below would need this same
|
||||
* hand-written glue regardless. It also intentionally is NOT reachable via
|
||||
* combinedFamilyInstall's early return above `_runLegacyInstallMigrations`,
|
||||
* matching the existing precedent that OpenCode/Kilo's bespoke install path
|
||||
* owns its own legacy cleanup rather than routing through the generic
|
||||
* layout-driven migrations hook.
|
||||
*/
|
||||
function _migrateLegacyOpencodeCommandDir(runtime: string, configDir: string, behaviors: any): void {
|
||||
const LEGACY_NAME = 'command';
|
||||
const currentName = behaviors.flatCommandDir || LEGACY_NAME;
|
||||
if (currentName === LEGACY_NAME) return; // e.g. Kilo — legacy IS the current location; nothing to migrate
|
||||
const legacyDir = path.join(configDir, LEGACY_NAME);
|
||||
if (!fs.existsSync(legacyDir)) return;
|
||||
// Never follow a symlinked legacy dir out of configDir.
|
||||
if (fs.lstatSync(legacyDir).isSymbolicLink()) return;
|
||||
|
||||
const manifest = installerMigrations.readInstallManifest(configDir);
|
||||
let entries: fs.Dirent[];
|
||||
try {
|
||||
entries = fs.readdirSync(legacyDir, { withFileTypes: true });
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
for (const entry of entries) {
|
||||
// command/ is a flat directory of gsd-*.md files; skip anything that
|
||||
// isn't a plain file (nested dirs, symlinks) rather than guess intent.
|
||||
if (!entry.isFile()) continue;
|
||||
const relPath = `${LEGACY_NAME}/${entry.name}`;
|
||||
const { classification } = installerMigrations.classifyArtifact(configDir, relPath, manifest);
|
||||
if (classification === 'managed-pristine' || classification === 'managed-modified') {
|
||||
try { fs.unlinkSync(path.join(legacyDir, entry.name)); } catch { /* best-effort */ }
|
||||
}
|
||||
// 'unknown' (not manifest-tracked) is left untouched — GSD cannot prove
|
||||
// ownership, so it must never be deleted as collateral damage.
|
||||
}
|
||||
|
||||
try {
|
||||
if (fs.readdirSync(legacyDir).length === 0) fs.rmdirSync(legacyDir);
|
||||
} catch { /* best-effort — a non-empty or otherwise-busy dir is left in place */ }
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// installOpencodeFamilyArtifacts
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -981,7 +1056,17 @@ function installOpencodeFamilyArtifacts(
|
||||
homeDir: posixNormalize(os.homedir()),
|
||||
});
|
||||
|
||||
const commandDir = runtimeArtifactInstallPlan.assertDestWithinConfigHome(configDir, 'command');
|
||||
// #2329: destDir is derived from the SAME hostBehaviors.flatCommandDir
|
||||
// descriptor value read by writeManifest's manifest-key prefix and by
|
||||
// resolveRuntimeArtifactLayout's commands-kind destSubpath — a hardcoded
|
||||
// literal here would silently diverge from the descriptor the moment either
|
||||
// is edited (Generative Fix Divergence guard). OpenCode uses 'commands'
|
||||
// (plural, the dir OpenCode actually discovers slash commands from); Kilo
|
||||
// keeps its own descriptor value ('command', singular) unchanged.
|
||||
const commandDir = runtimeArtifactInstallPlan.assertDestWithinConfigHome(
|
||||
configDir,
|
||||
behaviors.flatCommandDir || 'command',
|
||||
);
|
||||
installOpencodeFamilyCommands(runtime, commandDir, rawCommandsDir, pathPrefix, resolveAttribution);
|
||||
installOpencodeFamilySkills(runtime, configDir, rawCommandsDir, pathPrefix, resolveAttribution);
|
||||
|
||||
|
||||
187
src/installer-migrations/005-opencode-baseline-commands-dir.cts
Normal file
187
src/installer-migrations/005-opencode-baseline-commands-dir.cts
Normal file
@@ -0,0 +1,187 @@
|
||||
/**
|
||||
* Installer migration 005: baseline pre-existing OpenCode commands/ (plural)
|
||||
* files during the first-time installer migration baseline scan (#2329
|
||||
* follow-up).
|
||||
*
|
||||
* Background: #2329 moved OpenCode's slash-command install target from the
|
||||
* legacy singular `command/` alias to the documented plural `commands/`
|
||||
* convention. The first-time baseline scan migration
|
||||
* (2026-05-11-first-time-baseline-scan, 000-first-time-baseline.cts) is a
|
||||
* SHIPPED migration body — docs/installer-migrations.md#state-files requires
|
||||
* shipped migration bodies stay immutable so an already-applied migration's
|
||||
* checksum never drifts for a user who ran it before this fix (issue #670).
|
||||
* Its RUNTIME_SURFACES.opencode list still only names the legacy `command`
|
||||
* directory, so the baseline scan never classified pre-existing files under
|
||||
* `commands/` before this fix.
|
||||
*
|
||||
* Consequence proven by probe (see tests/installer-migrations.test.cjs): a
|
||||
* pre-existing, unmanifested file at `commands/gsd-<name>.md` that predates
|
||||
* any GSD install is silently destroyed by ordinary OpenCode command
|
||||
* materialization (which unconditionally removes every `gsd-*.md` file under
|
||||
* its destination before writing the fresh set) with a clean exit code — no
|
||||
* report, no backup, no prompt. The identical scenario under the
|
||||
* already-covered legacy `command/` surface instead halts the install with a
|
||||
* blocked `prompt-user` action, exactly as designed. This migration closes
|
||||
* that gap for `commands/` without editing 000's shipped body.
|
||||
*
|
||||
* This is a NEW fix-forward migration id (per
|
||||
* docs/installer-migrations.md#state-files) rather than an edit to 000: an
|
||||
* already-applied migration never re-runs, so editing 000 would only protect
|
||||
* fresh installs and leave every machine that already applied 000
|
||||
* permanently unprotected for `commands/`. A new id runs for both
|
||||
* populations (installs that never ran a baseline scan AND installs that
|
||||
* already applied the original 000 scan) and drifts no checksum.
|
||||
*
|
||||
* Scope: OpenCode only. Kilo shares the same combined-family install path,
|
||||
* but its command directory descriptor is still the singular `command/`
|
||||
* (already covered by 000's RUNTIME_SURFACES.kilo), so this migration must
|
||||
* never touch Kilo installs — enforced by the `runtimes: ['opencode']`
|
||||
* scoping below.
|
||||
*
|
||||
* Classification mirrors 000-first-time-baseline.cts exactly (record-baseline
|
||||
* for manifest-proven files, prompt-user for stale-GSD-looking unmanifested
|
||||
* files, baseline-preserve-user for everything else) — this migration only
|
||||
* widens the surface scanned, not the classification policy.
|
||||
*/
|
||||
|
||||
import fs from 'node:fs';
|
||||
import path from 'node:path';
|
||||
|
||||
const SURFACE = 'commands';
|
||||
|
||||
interface ClassifiedArtifact {
|
||||
classification: string;
|
||||
originalHash?: string | null;
|
||||
currentHash?: string | null;
|
||||
[key: string]: unknown;
|
||||
}
|
||||
|
||||
interface BaselineAction {
|
||||
type: string;
|
||||
relPath: string;
|
||||
reason: string;
|
||||
classification?: string;
|
||||
originalHash?: string | null;
|
||||
currentHash?: string | null;
|
||||
prompt?: string;
|
||||
choices?: string[];
|
||||
}
|
||||
|
||||
interface PlanContext {
|
||||
configDir: string;
|
||||
baselineScan?: boolean;
|
||||
classifyArtifact: (relPath: string) => ClassifiedArtifact;
|
||||
}
|
||||
|
||||
interface InstallerMigration {
|
||||
id: string;
|
||||
title: string;
|
||||
description: string;
|
||||
introducedIn: string;
|
||||
runtimes: string[];
|
||||
scopes: string[];
|
||||
destructive: boolean;
|
||||
plan: (ctx: PlanContext) => BaselineAction[];
|
||||
}
|
||||
|
||||
function normalizeRelPath(relPath: string): string {
|
||||
return relPath.replace(/\\/g, '/').replace(/^\/+/, '');
|
||||
}
|
||||
|
||||
function walkFiles(root: string, relDir: string, files: Set<string>): void {
|
||||
const dir = path.join(root, relDir);
|
||||
if (!fs.existsSync(dir)) return;
|
||||
const entries = fs.readdirSync(dir, { withFileTypes: true });
|
||||
for (const entry of entries) {
|
||||
const relPath = path.posix.join(relDir, entry.name);
|
||||
if (entry.isDirectory()) {
|
||||
walkFiles(root, relPath, files);
|
||||
} else if (entry.isFile()) {
|
||||
files.add(normalizeRelPath(relPath));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function scanCommandsSurface(configDir: string): string[] {
|
||||
const relPaths = new Set<string>();
|
||||
const fullPath = path.join(configDir, SURFACE);
|
||||
if (!fs.existsSync(fullPath)) return [];
|
||||
const stat = fs.statSync(fullPath);
|
||||
if (stat.isDirectory()) {
|
||||
walkFiles(configDir, SURFACE, relPaths);
|
||||
} else if (stat.isFile()) {
|
||||
relPaths.add(SURFACE);
|
||||
}
|
||||
return [...relPaths];
|
||||
}
|
||||
|
||||
function isStaleGsdLookingPath(relPath: string): boolean {
|
||||
return /^gsd[-_]/.test(path.posix.basename(relPath));
|
||||
}
|
||||
|
||||
function baselineActionRank(action: BaselineAction): number {
|
||||
if (action.type === 'record-baseline') return 0;
|
||||
if (action.type === 'baseline-preserve-user') return 1;
|
||||
return 2;
|
||||
}
|
||||
|
||||
const migration: InstallerMigration = {
|
||||
id: '2026-07-17-opencode-baseline-commands-dir',
|
||||
title: "Baseline OpenCode's commands/ directory in the first-time scan (#2329 follow-up)",
|
||||
description:
|
||||
"Classify pre-existing files under OpenCode's commands/ (plural) directory during the first-time installer " +
|
||||
'migration baseline scan. #2329 moved OpenCode command materialization from the legacy command/ alias to ' +
|
||||
"commands/, but the shipped 000-first-time-baseline.cts RUNTIME_SURFACES.opencode list is immutable and still " +
|
||||
'only names command/; this fix-forward migration widens the scanned surface without editing the shipped body.',
|
||||
introducedIn: '1.7.0',
|
||||
runtimes: ['opencode'],
|
||||
scopes: ['global', 'local'],
|
||||
destructive: false,
|
||||
plan: ({ configDir, baselineScan, classifyArtifact }: PlanContext): BaselineAction[] => {
|
||||
if (!baselineScan) return [];
|
||||
|
||||
const actions: BaselineAction[] = [];
|
||||
for (const relPath of scanCommandsSurface(configDir)) {
|
||||
const artifact = classifyArtifact(relPath);
|
||||
if (artifact.classification === 'managed-pristine' || artifact.classification === 'managed-modified') {
|
||||
actions.push({
|
||||
type: 'record-baseline',
|
||||
relPath,
|
||||
reason: 'existing manifest-managed OpenCode commands/ file included in first-time migration baseline',
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
const currentHash = artifact.currentHash ?? null;
|
||||
if (isStaleGsdLookingPath(relPath)) {
|
||||
actions.push({
|
||||
type: 'prompt-user',
|
||||
relPath,
|
||||
reason: 'GSD-looking file is not proven manifest-managed and needs explicit user choice',
|
||||
classification: 'stale-gsd-looking',
|
||||
originalHash: artifact.originalHash ?? null,
|
||||
currentHash,
|
||||
prompt: 'Choose whether to remove this stale-looking GSD artifact or keep it as user-owned.',
|
||||
choices: ['keep', 'remove'],
|
||||
});
|
||||
continue;
|
||||
}
|
||||
|
||||
actions.push({
|
||||
type: 'baseline-preserve-user',
|
||||
relPath,
|
||||
reason: 'unknown OpenCode commands/ file preserved by first-time migration baseline',
|
||||
classification: artifact.classification,
|
||||
originalHash: artifact.originalHash ?? null,
|
||||
currentHash,
|
||||
});
|
||||
}
|
||||
|
||||
return actions.sort(
|
||||
(left, right) =>
|
||||
baselineActionRank(left) - baselineActionRank(right) || left.relPath.localeCompare(right.relPath)
|
||||
);
|
||||
},
|
||||
};
|
||||
|
||||
export = migration;
|
||||
Reference in New Issue
Block a user