From d258129d527757d15f8f7ae69995543f6570549a Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:09:29 -0300 Subject: [PATCH 1/3] fix(release): predict the release commit from staged-blob truth The dry-run tail promised a commit unconditionally while the executed path detects an empty cached diff and tags existing HEAD instead, so a re-cut preview named an action the release would not take. The manifest preview was worse than mislabelled: it compared canonical bytes against canonical bytes, so its formatting-drift label named an unreachable state while a noncanonical manifest at the same version really did stage a diff and commit. Both now decide the one question the executed path decides - would `git add` stage anything - through `git hash-object --path --stdin` against `HEAD:`, which applies the same clean filters and writes no object. The dry run consults `ensureCleanTree`, reports the refusal the real release raises, and predicts no landing on a dirty tree; the executed re-cut sentence moves behind a shared constant byte for byte, so preview and execution cannot drift. The mutation matrix that proved these behaviours load-bearing also exposed the reason the old reporting survived: the fake-adapter release contracts and the dirty-tree comparison were reachable only from a flag-gated scenario that nothing schedules. Both now run in the default suite, and the adapter's two fail-loud predicates are asserted against real git. The shim passes through only the three exact read-only argument vectors and refuses `-w`, index, and ref writes. --- scripts/AGENTS.md | 4 +- scripts/lib/plugin-release.mjs | 105 ++++++-- scripts/tests/ci-plugin-targeting.mjs | 334 +++++++++++++++++++++++++- 3 files changed, 418 insertions(+), 25 deletions(-) diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index 8a487996..0320cfe2 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -26,7 +26,7 @@ The repo hosts **multiple plugins** (`docks`, `plan-lifecycle`, `effect-kit`) un | `transformGuard` | run `transform-guard.mjs` (curated transformers) | | `release` | Closed, data-only release policy. Every plugin declares exactly `{ kind: 'generic', install }`. No callbacks, commands, safety gates, or ordering belong in descriptors. | -`lib/plugin-release.mjs` owns ordinary release ordering behind `runGenericPluginRelease({ argv, repo, plugins, io })`. Its IO value is an exact closed adapter of filesystem, Git, Claude, GitHub, selected-CI, and logging operations; production composes those operations in `release.mjs`, while descriptors remain inert policy data. The engine validates every policy before touching IO and enforces dry-run no-mutation itself rather than trusting an adapter. +`lib/plugin-release.mjs` owns ordinary release ordering behind `runGenericPluginRelease({ argv, repo, plugins, io })`. Its IO value is an exact closed adapter that carries fifteen filesystem, Git, Claude, GitHub, selected-CI, and logging operations. `release.mjs` composes the production operations. Descriptors remain inert policy data. The fifteenth operation, `wouldStageChange`, answers whether `git add` of release bytes would stage anything different from HEAD. Production hashes the proposed bytes with `git hash-object --path --stdin`. It compares that hash with `git rev-parse --quiet --verify HEAD:`. `--path` applies the same clean filters that `git add` applies. The probe never passes `-w`, so it never writes an object. The engine validates every policy before touching IO. It enforces dry-run no-mutation itself rather than trusting an adapter. `ci.mjs` is **registry-driven**. A full invocation runs repo-wide checks once (workflow YAML, both marketplace catalogs, tree/guard, durable anchors, author tooling, unit tests, and CI targeting), then selects every present plugin's shell hooks, repository author suites, and capability-driven `gatePlugin` work. `--plugin ` skips repo-wide sections and runs only the named plugin's owned author checks, target-derived shell lint, and plugin validation. When Docks plan author checks apply, CI runs `scripts/tests/plan-cli.mjs` plus `scripts/tests/plan-skill-phases.mjs` with the `bounded-workflows` and `plan-workspace-template` cases. Trigger-collision checks audit Docks and Effect Kit together once. @@ -184,6 +184,8 @@ final implementation tree → node scripts/ci.mjs --plugin (LAYER 1 — └── tag-CI fails → exits non-zero, prints recovery ``` +The release tag, not the manifest number, is the fact that a version was released. When manifests are already at this version, a re-cut stages nothing. The release tags existing HEAD instead of creating a commit. A dry run consults the clean-tree gate. On a dirty tree, it reports the refusal instead of forecasting a landing it cannot predict. + The positional flow above is preserved for docks/effect-kit/plan-lifecycle, including its existing bump resolution, local and tag CI gates, commit/push/tag behavior, release notes, and read-only dry run. GitHub pull requests resolve their diff into a shard set and run `node scripts/ci.mjs --lane ` for each, then require the unchanged `validate` join status. `resolve-shards` maps changed paths onto plugin roots from `lib/plugins.mjs` and emits the matrix; the `repo` shard always runs, `core` runs when the diff implicates a plugin it owns, and every resolution failure — unresolvable base, empty diff, non-pull-request event, or a path outside every plugin root — falls open to both shards. `workflow_dispatch` runs one untargeted `node scripts/ci.mjs` full invocation. A release-tag push strictly resolves the tag's plugin identity, rejects malformed or unknown targets, and runs `node scripts/ci.mjs --plugin ` as the authoritative selected-plugin gate; PR sharding never touches that path. The `repo` shard owns the repo-wide workflow, standalone catalog, tree/durable-anchor, and CI-targeting sections, so a plugin shard runs only the selected plugins' owned author checks, shell-hook lint, and plugin gates, including marketplace/version coherence. Targeted `--plugin` CI skips the repo-wide sections entirely. The Bun dependency cache only reduces repeated download work. Its contents are never validation evidence: the frozen lockfile, release preflight, and `ci.mjs` result remain authoritative. diff --git a/scripts/lib/plugin-release.mjs b/scripts/lib/plugin-release.mjs index 746ffc09..73227404 100644 --- a/scripts/lib/plugin-release.mjs +++ b/scripts/lib/plugin-release.mjs @@ -2,6 +2,11 @@ import { spawnSync } from 'node:child_process'; import fs from 'node:fs'; import path from 'node:path'; +// Preview and execution share these reasons so a dry run cannot drift from the release +// decision it forecasts. +const RECUT_TAG_REASON = 'manifests already at this version — tagging existing HEAD'; +const DIRTY_TREE_REASON = 'working tree dirty — commit/stash first'; + const IO_KEYS = Object.freeze([ 'commit', 'createRelease', @@ -16,6 +21,7 @@ const IO_KEYS = Object.freeze([ 'resolveTagCommit', 'runSelectedCi', 'waitForTagCi', + 'wouldStageChange', 'writeJson', ]); @@ -223,16 +229,27 @@ function formattedJson(value) { return `${JSON.stringify(value, null, 2)}\n`; } -function previewJsonWrite({ original, after, file, repo, io }) { - const originalLines = original.split('\n'); - const output = formattedJson(after); - const changed = output - .split('\n') - .filter((line, index) => line !== originalLines[index]) - .map((line) => line.trim()); - io.log( - ` [dry-run] would write ${path.relative(repo, file)} (changed: ${changed.join(' | ') || 'none — formatting drift!'})`, - ); +// Preview of the manifest writes a real run would make. A dirty tree is never compared: +// the probe below is only meaningful against a clean index, and the executed release +// refuses on a dirty tree, so no staging decision exists to forecast. +function previewJsonWrites({ repo, io, version, versionChanged, cleanTree }) { + const staged = []; + const preview = (file, after) => { + const relative = path.relative(repo, file); + if (!cleanTree) { + io.log(` [dry-run] ${relative}: not compared (working tree dirty)`); + return; + } + const wouldStage = io.wouldStageChange(file, formattedJson(after)); + const action = !wouldStage + ? `unchanged (already at ${version})` + : versionChanged + ? `would write version → ${version}` + : `would rewrite formatting only (already at ${version})`; + io.log(` [dry-run] ${relative}: ${action}`); + staged.push(wouldStage); + }; + return { preview, staged }; } function reportedFailure(message) { @@ -268,7 +285,13 @@ export async function runGenericPluginRelease({ argv, repo, plugins, io }) { if (!dryRun && !io.ensureTool('claude')) throw new Error('claude is required'); if (!io.fileExists(pluginJson)) throw new Error(`plugin.json not found at ${pluginJson}`); if (!io.fileExists(marketplaceJson)) throw new Error(`marketplace.json not found at ${marketplaceJson}`); - if (!dryRun && !io.ensureCleanTree()) throw new Error('working tree dirty — commit/stash first'); + const cleanTree = io.ensureCleanTree(); + let dryRunBlocked = false; + if (!cleanTree) { + if (!dryRun) throw new Error(DIRTY_TREE_REASON); + io.log(` [dry-run] refused: ${DIRTY_TREE_REASON}`); + dryRunBlocked = true; + } io.log(`Running local ci.mjs for ${plugin.name}...`); const ciResult = await io.runSelectedCi(plugin, ['-q', '--plugin', plugin.name]); @@ -288,25 +311,30 @@ export async function runGenericPluginRelease({ argv, repo, plugins, io }) { throw new Error(`already released: ${plugin.name} v${newVersion} (tag ${plugin.name}--v${newVersion} exists)`); } io.log(`Bumping ${plugin.name}: ${currentVersion} → ${newVersion}`); + const versionChanged = newVersion !== currentVersion; + const { preview, staged: wouldStageChanges } = previewJsonWrites({ + repo, + io, + version: newVersion, + versionChanged, + cleanTree, + }); - const pluginOriginal = dryRun ? formattedJson(pluginManifest) : null; pluginManifest.version = newVersion; - if (dryRun) previewJsonWrite({ original: pluginOriginal, after: pluginManifest, file: pluginJson, repo, io }); + if (dryRun) preview(pluginJson, pluginManifest); else await io.writeJson(pluginJson, pluginManifest); const marketplace = await io.readJson(marketplaceJson); - const marketplaceOriginal = dryRun ? formattedJson(marketplace) : null; const marketplacePlugin = marketplace.plugins.find((candidate) => candidate.name === plugin.name); if (marketplacePlugin) marketplacePlugin.version = newVersion; - if (dryRun) previewJsonWrite({ original: marketplaceOriginal, after: marketplace, file: marketplaceJson, repo, io }); + if (dryRun) preview(marketplaceJson, marketplace); else await io.writeJson(marketplaceJson, marketplace); const codexFiles = []; if (plugin.codex && io.fileExists(codexPluginJson)) { const codexManifest = await io.readJson(codexPluginJson); - const codexOriginal = dryRun ? formattedJson(codexManifest) : null; codexManifest.version = newVersion; - if (dryRun) previewJsonWrite({ original: codexOriginal, after: codexManifest, file: codexPluginJson, repo, io }); + if (dryRun) preview(codexPluginJson, codexManifest); else await io.writeJson(codexPluginJson, codexManifest); codexFiles.push(path.relative(repo, codexPluginJson)); } @@ -314,8 +342,19 @@ export async function runGenericPluginRelease({ argv, repo, plugins, io }) { const addFiles = [`${plugin.root}/.claude-plugin/plugin.json`, '.claude-plugin/marketplace.json', ...codexFiles]; const tag = `${plugin.name}--v${newVersion}`; if (dryRun) { + // The executed release refuses before writing on a dirty tree, so there is no + // commit, push, tag, or release outcome to forecast. + if (dryRunBlocked) { + io.log(' [dry-run] no commit, push, tag, or release would run'); + io.log('\n[dry-run] BLOCKED — the release would refuse; no changes written, no tag, no release.'); + return true; + } io.log(` [dry-run] git add ${addFiles.join(' ')}`); - io.log(` [dry-run] git commit -m "chore(release): ${plugin.name} v${newVersion}"`); + if (wouldStageChanges.some(Boolean)) { + io.log(` [dry-run] git commit -m "chore(release): ${plugin.name} v${newVersion}"`); + } else { + io.log(` [dry-run] ${RECUT_TAG_REASON}`); + } io.log(' [dry-run] git push origin HEAD'); io.log(` [dry-run] claude plugin tag --push --message "${plugin.name} plugin %s" ${pluginPath}`); io.log(` [dry-run] wait for tag-CI on ${tag}, then gh release create (gated on CI green)`); @@ -398,7 +437,8 @@ export function createGenericPluginReleaseIo({ repo, plugins }) { // recovery path after a run that bumped, pushed, and then failed tag-CI — not an error, so // tag the existing HEAD rather than failing on an empty commit. if (capture('git', ['diff', '--cached', '--quiet', '--', ...files]).status === 0) { - process.stdout.write(' manifests already at this version — tagging existing HEAD\n'); + // Keep the executed re-cut sentence byte-identical to the dry-run reason. + process.stdout.write(` ${RECUT_TAG_REASON}\n`); return; } run('git', ['commit', '-m', message]); @@ -413,7 +453,12 @@ export function createGenericPluginReleaseIo({ repo, plugins }) { return pushedAt; }, ensureCleanTree() { - return capture('git', ['status', '--porcelain']).stdout.trim() === ''; + // A failed `git status` says nothing about the tree; reporting it as clean would + // let a dry run forecast a landing the real release refuses. + const status = capture('git', ['status', '--porcelain']); + if ((status.status ?? 1) !== 0) + throw new Error(`git status --porcelain failed: ${status.stderr?.trim() || `exit ${status.status}`}`); + return status.stdout.trim() === ''; }, ensureTool(tool) { const result = spawnSync(tool, ['--version'], { stdio: 'ignore' }); @@ -521,6 +566,26 @@ export function createGenericPluginReleaseIo({ repo, plugins }) { createdAt: identified.createdAt, }; }, + wouldStageChange(file, content) { + const relative = path.relative(repo, file); + // `git add` compares the filter-normalized blob with HEAD, not the worktree + // bytes. Hash through the path without `-w` to reproduce that decision without + // writing either the index or the object database. + const staged = spawnSync('git', ['hash-object', '--path', file, '--stdin'], { + cwd: repo, + encoding: 'utf8', + input: content, + }); + if ((staged.status ?? 1) !== 0) { + throw new Error(`git hash-object failed for ${relative}: ${staged.stderr?.trim() || `exit ${staged.status}`}`); + } + const committed = capture('git', ['rev-parse', '--quiet', '--verify', `HEAD:${relative}`]); + // Callers probe only on a clean tree, where every managed file is tracked. A path + // missing from HEAD is therefore an inconsistency, not a new file to guess about. + if ((committed.status ?? 1) !== 0) + throw new Error(`${relative} is missing from HEAD; refusing to guess whether a release would commit it`); + return (staged.stdout ?? '').trim() !== (committed.stdout ?? '').trim(); + }, writeJson(file, value) { fs.writeFileSync(file, `${JSON.stringify(value, null, 2)}\n`); }, diff --git a/scripts/tests/ci-plugin-targeting.mjs b/scripts/tests/ci-plugin-targeting.mjs index e3a0b999..4d8d12b0 100755 --- a/scripts/tests/ci-plugin-targeting.mjs +++ b/scripts/tests/ci-plugin-targeting.mjs @@ -308,8 +308,24 @@ import fs from 'node:fs'; const tool = ${JSON.stringify(name)}; const args = process.argv.slice(2); fs.appendFileSync(process.env.DOCKS_RELEASE_CALL_LOG, JSON.stringify({ tool, args }) + '\\n'); -if (tool === 'node') { - const child = spawnSync(process.env.DOCKS_RELEASE_REAL_NODE, args, { +// Only the three read-only invocations the dry run is allowed to make reach real git, matched +// argument for argument. A looser match keyed on the subcommand alone would let a mutating +// form such as \`hash-object -w\` through the harness that exists to prove nothing mutates. +const readOnlyGit = [ + ['status', '--porcelain'], + ['hash-object', '--path', null, '--stdin'], + ['rev-parse', '--quiet', '--verify', null], +]; +const passThroughGit = + tool === 'git' && + readOnlyGit.some( + (shape) => + shape.length === args.length && shape.every((token, index) => token === null || token === args[index]), + ) && + !args.includes('-w'); +if (tool === 'node' || passThroughGit) { + const command = tool === 'node' ? process.env.DOCKS_RELEASE_REAL_NODE : tool; + const child = spawnSync(command, args, { stdio: 'inherit', env: { ...process.env, PATH: process.env.DOCKS_RELEASE_REAL_PATH }, }); @@ -766,7 +782,8 @@ function genericReleaseIo(repo, options = {}) { }, ensureCleanTree() { record('ensureCleanTree'); - return true; + if (typeof options.cleanTree === 'function') return options.cleanTree(); + return options.cleanTree ?? true; }, ensureTool(tool) { record('ensureTool', [tool]); @@ -806,6 +823,12 @@ function genericReleaseIo(repo, options = {}) { record('waitForTagCi', [tag, commit, pushed]); return tagCiResult; }, + wouldStageChange(file, content) { + const relative = relativePath(file); + record('wouldStageChange', [relative, content]); + if (typeof options.wouldStageChange === 'function') return options.wouldStageChange(relative, content); + return options.wouldStageChange ?? true; + }, writeJson(file, value) { record('writeJson', [relativePath(file), value]); }, @@ -862,6 +885,8 @@ async function testGenericReleaseModuleContract( const currentVersion = JSON.parse( fs.readFileSync(path.join(ROOT, plugin.root, '.claude-plugin/plugin.json'), 'utf8'), ).version; + const [major, minor, patchVersion] = currentVersion.split('.').map(Number); + const targetVersion = `${major}.${minor}.${patchVersion + 1}`; const fixture = genericReleaseIo(ROOT); await runGenericPluginRelease({ argv: ['--dry-run', '--plugin', plugin.name, 'patch'], @@ -889,6 +914,17 @@ async function testGenericReleaseModuleContract( fixture.output.join('\n'), new RegExp(`Bumping ${plugin.name}: ${currentVersion.replaceAll('.', '\\.')} →`), ); + const dryRunOutput = fixture.output.join('\n'); + assert.ok( + dryRunOutput.includes( + ` [dry-run] ${plugin.root}/.claude-plugin/plugin.json: would write version → ${targetVersion}`, + ), + `${plugin.name} real bump preview must name the target version`, + ); + assert.ok( + dryRunOutput.includes(` [dry-run] git commit -m "chore(release): ${plugin.name} v${targetVersion}"`), + `${plugin.name} real bump preview must predict the release commit`, + ); assert.equal( fixture.calls.some(({ tool }) => ['writeJson', 'commit', 'push', 'createTag', 'waitForTagCi', 'createRelease'].includes(tool), @@ -899,6 +935,169 @@ async function testGenericReleaseModuleContract( } const generic = ordinaryPlugins[0]; + const recutPlugin = ordinaryPlugins[2]; + const recutVersion = JSON.parse( + fs.readFileSync(path.join(ROOT, recutPlugin.root, '.claude-plugin/plugin.json'), 'utf8'), + ).version; + const recutManifest = `${recutPlugin.root}/.claude-plugin/plugin.json`; + + const unchangedRecut = genericReleaseIo(ROOT, { wouldStageChange: false }); + await runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', recutPlugin.name, recutVersion], + repo: ROOT, + plugins: PLUGINS, + io: unchangedRecut.io, + }); + const unchangedRecutOutput = unchangedRecut.output.join('\n'); + assert.ok( + unchangedRecutOutput.includes(` [dry-run] ${recutManifest}: unchanged (already at ${recutVersion})`), + 'unchanged re-cut preview must identify a manifest that would not stage', + ); + assert.ok( + unchangedRecutOutput.includes(' [dry-run] manifests already at this version — tagging existing HEAD'), + 'unchanged re-cut preview must predict tagging existing HEAD', + ); + assert.equal( + unchangedRecut.output.some((line) => line.includes('git commit')), + false, + 'unchanged re-cut preview must not predict a release commit', + ); + + const formattingOnlyRecut = genericReleaseIo(ROOT, { wouldStageChange: true }); + await runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', recutPlugin.name, recutVersion], + repo: ROOT, + plugins: PLUGINS, + io: formattingOnlyRecut.io, + }); + const formattingOnlyOutput = formattingOnlyRecut.output.join('\n'); + assert.ok( + formattingOnlyOutput.includes( + ` [dry-run] ${recutManifest}: would rewrite formatting only (already at ${recutVersion})`, + ), + 'formatting-only re-cut preview must distinguish canonicalization from a version bump', + ); + assert.ok( + formattingOnlyOutput.includes(` [dry-run] git commit -m "chore(release): ${recutPlugin.name} v${recutVersion}"`), + 'formatting-only re-cut preview must predict the release commit', + ); + assert.ok( + formattingOnlyRecut.calls.some( + ({ tool, args: callArgs }) => + tool === 'wouldStageChange' && + callArgs[0] === recutManifest && + typeof callArgs[1] === 'string' && + callArgs[1].includes(`"version": "${recutVersion}"`), + ), + 'formatting-only re-cut fixture must record the staged path and candidate content', + ); + + const dirtyTree = genericReleaseIo(ROOT, { cleanTree: false }); + await runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', recutPlugin.name, 'patch'], + repo: ROOT, + plugins: PLUGINS, + io: dirtyTree.io, + }); + const dirtyTreeOutput = dirtyTree.output.join('\n'); + for (const relative of [ + recutManifest, + '.claude-plugin/marketplace.json', + `${recutPlugin.root}/.codex-plugin/plugin.json`, + ]) { + assert.ok( + dirtyTreeOutput.includes(` [dry-run] ${relative}: not compared (working tree dirty)`), + `dirty-tree dry-run refusal must decline the per-manifest comparison for ${relative}`, + ); + } + assert.equal( + dirtyTree.calls.some(({ tool }) => tool === 'wouldStageChange'), + false, + 'dirty-tree dry run must not run a stage comparison it cannot trust', + ); + assert.ok( + dirtyTreeOutput.includes(' [dry-run] refused: working tree dirty — commit/stash first'), + 'dirty-tree dry-run refusal must name the clean-tree gate', + ); + assert.ok( + dirtyTreeOutput.includes(' [dry-run] no commit, push, tag, or release would run'), + 'dirty-tree dry-run refusal must suppress every landing action', + ); + assert.ok( + dirtyTreeOutput.includes('[dry-run] BLOCKED — the release would refuse; no changes written, no tag, no release.'), + 'dirty-tree dry-run refusal must end with the blocked closer', + ); + for (const forbidden of ['git add', 'git commit', 'git push', 'plugin tag']) { + assert.equal( + dirtyTree.output.some((line) => line.includes(forbidden)), + false, + `dirty-tree dry-run refusal must not print ${forbidden}`, + ); + } + + const landingForecastFragments = ['git add', 'git commit', 'git push', 'plugin tag']; + const cleanTreeFailureMessage = 'fixture clean-tree check failed'; + const cleanTreeFailure = genericReleaseIo(ROOT, { + cleanTree() { + throw new Error(cleanTreeFailureMessage); + }, + }); + await assert.rejects( + runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', generic.name, 'patch'], + repo: ROOT, + plugins: PLUGINS, + io: cleanTreeFailure.io, + }), + { message: cleanTreeFailureMessage }, + 'clean-tree check failure must surface unchanged', + ); + assert.equal( + cleanTreeFailure.output.some((line) => landingForecastFragments.some((fragment) => line.includes(fragment))), + false, + 'clean-tree check failure must not print a landing forecast', + ); + + const stageProbeFailureMessage = 'fixture staged-content check failed'; + const stageProbeFailure = genericReleaseIo(ROOT, { + wouldStageChange() { + throw new Error(stageProbeFailureMessage); + }, + }); + await assert.rejects( + runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', generic.name, 'patch'], + repo: ROOT, + plugins: PLUGINS, + io: stageProbeFailure.io, + }), + { message: stageProbeFailureMessage }, + 'staged-content check failure must surface unchanged', + ); + assert.ok( + stageProbeFailure.calls.some(({ tool }) => tool === 'wouldStageChange'), + 'staged-content check failure must reach the stage probe', + ); + assert.equal( + stageProbeFailure.output.some((line) => landingForecastFragments.some((fragment) => line.includes(fragment))), + false, + 'staged-content check failure must not print a landing forecast', + ); + + const missingStageProbe = genericReleaseIo(ROOT); + const missingStageProbeIo = { ...missingStageProbe.io }; + delete missingStageProbeIo.wouldStageChange; + await assert.rejects( + runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', generic.name, 'patch'], + repo: ROOT, + plugins: PLUGINS, + io: missingStageProbeIo, + }), + /generic release IO must be the exact closed adapter/i, + 'missing stage probe must fail closed-adapter validation', + ); + assert.deepEqual(missingStageProbe.calls, [], 'missing stage probe validation must fail before IO'); await expectReleasePolicyRefusal( runGenericPluginRelease, generic, @@ -1123,7 +1322,6 @@ async function testDryRunReleaseSafety() { /fixture and report environment variables must both be non-empty/i, ); } - await testGenericReleaseModuleContract(dispatchPluginRelease, runGenericPluginRelease, resolveGenericReleaseIo); const before = gitSnapshot(); assert.equal(before.status, '', 'dry-run safety requires a clean checkout'); const fixtureRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-dry-run-')); @@ -1165,6 +1363,14 @@ async function testDryRunReleaseSafety() { ), 'fixture must intercept and preserve the targeted Docks preflight', ); + assert.ok( + calls.some(({ tool, args: callArgs }) => tool === 'git' && callArgs[0] === 'hash-object'), + 'dry-run fixture must pass the staged-content hash probe through to real git', + ); + assert.ok( + calls.some(({ tool, args: callArgs }) => tool === 'git' && callArgs[0] === 'rev-parse'), + 'dry-run fixture must pass the HEAD comparison through to real git', + ); assert.equal( calls.some(({ tool, args: callArgs }) => tool === 'git' && callArgs[0] === 'push'), false, @@ -1180,6 +1386,19 @@ async function testDryRunReleaseSafety() { false, 'dry-run must not invoke gh release create', ); + assert.equal( + calls.some(({ tool, args: callArgs }) => tool === 'git' && callArgs.includes('-w')), + false, + 'dry-run must never ask git to write an object', + ); + assert.equal( + calls.some( + ({ tool, args: callArgs }) => + tool === 'git' && ['update-index', 'write-tree', 'update-ref'].includes(callArgs[0]), + ), + false, + 'dry-run must never write the index or a ref', + ); assert.equal( calls.some( ({ tool, args: callArgs }) => @@ -2273,3 +2492,110 @@ assert.equal(integrityStep('verify registry signatures').run, 'npm audit signatu assert.equal(integrityStep('verify registry signatures')['continue-on-error'], undefined); console.log('workflow targeting and integrity separation contracts passed'); + +// The fake-adapter release contracts need neither real git nor a clean checkout, so they +// belong in the default run. Only the shim scenario above requires the gated clean tree. +const releaseModule = await import('../lib/plugin-release.mjs'); +await testGenericReleaseModuleContract( + releaseModule.dispatchPluginRelease, + releaseModule.runGenericPluginRelease, + releaseModule.resolveGenericReleaseIo, +); +console.log('generic release module contract and dry-run manifest previews passed'); + +async function testReleaseAdapterGitContracts(createReleaseIo) { + const runGit = (repo, gitArgs) => { + const result = spawnSync('git', gitArgs, { cwd: repo, encoding: 'utf8' }); + if ((result.status ?? 1) !== 0) { + throw new Error( + `git ${gitArgs.join(' ')} fixture setup failed: ${result.stderr?.trim() || `exit ${result.status}`}`, + ); + } + }; + + const notRepository = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-no-git-')); + try { + const io = createReleaseIo({ repo: notRepository, plugins: PLUGINS }); + assert.throws( + () => io.ensureCleanTree(), + /git status --porcelain failed/, + 'a failed status probe must never be reported as a clean release tree', + ); + } finally { + fs.rmSync(notRepository, { recursive: true, force: true }); + } + + const cleanComparison = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-compare-')); + try { + const manifest = path.join(cleanComparison, 'manifest.json'); + const committedBytes = '{"version":"1.0.0"}\n'; + runGit(cleanComparison, ['init', '-q']); + fs.writeFileSync(manifest, committedBytes); + runGit(cleanComparison, ['add', 'manifest.json']); + runGit(cleanComparison, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); + + const io = createReleaseIo({ repo: cleanComparison, plugins: PLUGINS }); + assert.equal( + io.wouldStageChange(manifest, committedBytes), + false, + 'the exact committed bytes must predict no release commit', + ); + assert.equal( + io.wouldStageChange(manifest, '{"version":"1.0.1"}\n'), + true, + 'changed manifest data must predict a release commit', + ); + assert.equal( + io.wouldStageChange(manifest, `${JSON.stringify({ version: '1.0.0' }, null, 2)}\n`), + true, + 'different serialization bytes must predict a release commit', + ); + } finally { + fs.rmSync(cleanComparison, { recursive: true, force: true }); + } + + const missingFromHead = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-missing-')); + try { + const committed = path.join(missingFromHead, 'manifest.json'); + const absent = path.join(missingFromHead, 'never-committed.json'); + runGit(missingFromHead, ['init', '-q']); + fs.writeFileSync(committed, '{"version":"1.0.0"}\n'); + runGit(missingFromHead, ['add', 'manifest.json']); + runGit(missingFromHead, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); + + const io = createReleaseIo({ repo: missingFromHead, plugins: PLUGINS }); + assert.throws( + () => io.wouldStageChange(absent, '{}\n'), + /missing from HEAD/, + 'an uncommitted path must make the release prediction refuse', + ); + } finally { + fs.rmSync(missingFromHead, { recursive: true, force: true }); + } + + const brokenFilter = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-filter-')); + try { + const manifest = path.join(brokenFilter, 'manifest.json'); + const committedBytes = '{"version":"1.0.0"}\n'; + runGit(brokenFilter, ['init', '-q']); + fs.writeFileSync(manifest, committedBytes); + runGit(brokenFilter, ['add', 'manifest.json']); + runGit(brokenFilter, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); + fs.writeFileSync(path.join(brokenFilter, '.gitattributes'), '*.json filter=broken\n'); + runGit(brokenFilter, ['config', 'filter.broken.clean', 'exit 3']); + runGit(brokenFilter, ['config', 'filter.broken.required', 'true']); + + const io = createReleaseIo({ repo: brokenFilter, plugins: PLUGINS }); + // Hashing through --path applies its clean filter; if that fails, the dry run must refuse rather than guess. + assert.throws( + () => io.wouldStageChange(manifest, committedBytes), + /git hash-object failed for /, + 'a required clean-filter failure must make the release prediction refuse', + ); + } finally { + fs.rmSync(brokenFilter, { recursive: true, force: true }); + } +} + +await testReleaseAdapterGitContracts(releaseModule.createGenericPluginReleaseIo); +console.log('release adapter git predicates refuse to guess'); From 88e2cb534a5b731af114c0b5aa9938dae9d3721c Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 21 Aug 2026 10:30:46 -0300 Subject: [PATCH 2/3] test(release): count objects and isolate the real-git fixtures Round-1 review found the safety scenario could certify a false negative. The shim passes nested `node` argv through with the real PATH, because the scenario exists to run a real plugin gate, and an object write is the one mutation such a child could make that neither status nor refs reveal. Restricting the argv would make the harness brittle without closing the hole, so the before/after snapshot now counts objects: injecting one `hash-object -w` into that exact branch fails the scenario at the comparison, 2845 objects to 2846, while the real gate leaves the count byte-identical. The real-git fixtures inherited ambient Git configuration, so a developer's `commit.gpgSign` or `core.hooksPath` could fail or alter fixtures the test calls hermetic. Fixture spawns now null out global and system config and disable signing, hooks, and prompts, without ever mutating `process.env`; a hostile injected config failed the suite before this change and passes after it. The fixture whose purpose is a required-but-broken clean filter keeps its repository-local filter. The dirty-tree negative list stopped at add, commit, push, and tag, so moving the workflow or release forecast above the blocked return would still have passed. One list now covers every line the unblocked tail can print. --- scripts/tests/ci-plugin-targeting.mjs | 37 ++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/scripts/tests/ci-plugin-targeting.mjs b/scripts/tests/ci-plugin-targeting.mjs index 4d8d12b0..a0ce4073 100755 --- a/scripts/tests/ci-plugin-targeting.mjs +++ b/scripts/tests/ci-plugin-targeting.mjs @@ -297,6 +297,10 @@ function gitSnapshot() { return { status: run(['status', '--porcelain=v1', '--untracked-files=all']), refs: run(['show-ref']), + // The shim passes nested `node` argv through with the real PATH, so a Node child can reach + // real git outside the call log. Objects are the one write such a child could make that + // neither status nor refs would show, so the safety claim has to count them too. + objects: run(['count-objects', '-v']), manifests: manifests.map((file) => fs.readFileSync(path.join(ROOT, file), 'base64')), }; } @@ -1027,7 +1031,18 @@ async function testGenericReleaseModuleContract( dirtyTreeOutput.includes('[dry-run] BLOCKED — the release would refuse; no changes written, no tag, no release.'), 'dirty-tree dry-run refusal must end with the blocked closer', ); - for (const forbidden of ['git add', 'git commit', 'git push', 'plugin tag']) { + // Every line the unblocked tail can print, so moving one above the blocked return is caught + // rather than passing because the list stopped at the first four. + const landingForecastFragments = [ + 'git add', + 'git commit', + 'git push', + 'plugin tag', + 'wait for tag-CI', + 'gh release create', + 'already at this version', + ]; + for (const forbidden of landingForecastFragments) { assert.equal( dirtyTree.output.some((line) => line.includes(forbidden)), false, @@ -1035,7 +1050,6 @@ async function testGenericReleaseModuleContract( ); } - const landingForecastFragments = ['git add', 'git commit', 'git push', 'plugin tag']; const cleanTreeFailureMessage = 'fixture clean-tree check failed'; const cleanTreeFailure = genericReleaseIo(ROOT, { cleanTree() { @@ -2504,14 +2518,28 @@ await testGenericReleaseModuleContract( console.log('generic release module contract and dry-run manifest previews passed'); async function testReleaseAdapterGitContracts(createReleaseIo) { + const gitEnv = { + ...process.env, + GIT_CONFIG_GLOBAL: os.devNull, + GIT_CONFIG_NOSYSTEM: '1', + GIT_TERMINAL_PROMPT: '0', + }; const runGit = (repo, gitArgs) => { - const result = spawnSync('git', gitArgs, { cwd: repo, encoding: 'utf8' }); + const result = spawnSync('git', ['-c', 'commit.gpgSign=false', '-c', `core.hooksPath=${os.devNull}`, ...gitArgs], { + cwd: repo, + encoding: 'utf8', + env: gitEnv, + }); if ((result.status ?? 1) !== 0) { throw new Error( `git ${gitArgs.join(' ')} fixture setup failed: ${result.stderr?.trim() || `exit ${result.status}`}`, ); } }; + const configureFixtureRepo = (repo) => { + runGit(repo, ['config', '--local', 'core.hooksPath', os.devNull]); + runGit(repo, ['config', '--local', 'core.attributesFile', os.devNull]); + }; const notRepository = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-no-git-')); try { @@ -2530,6 +2558,7 @@ async function testReleaseAdapterGitContracts(createReleaseIo) { const manifest = path.join(cleanComparison, 'manifest.json'); const committedBytes = '{"version":"1.0.0"}\n'; runGit(cleanComparison, ['init', '-q']); + configureFixtureRepo(cleanComparison); fs.writeFileSync(manifest, committedBytes); runGit(cleanComparison, ['add', 'manifest.json']); runGit(cleanComparison, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); @@ -2559,6 +2588,7 @@ async function testReleaseAdapterGitContracts(createReleaseIo) { const committed = path.join(missingFromHead, 'manifest.json'); const absent = path.join(missingFromHead, 'never-committed.json'); runGit(missingFromHead, ['init', '-q']); + configureFixtureRepo(missingFromHead); fs.writeFileSync(committed, '{"version":"1.0.0"}\n'); runGit(missingFromHead, ['add', 'manifest.json']); runGit(missingFromHead, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); @@ -2578,6 +2608,7 @@ async function testReleaseAdapterGitContracts(createReleaseIo) { const manifest = path.join(brokenFilter, 'manifest.json'); const committedBytes = '{"version":"1.0.0"}\n'; runGit(brokenFilter, ['init', '-q']); + configureFixtureRepo(brokenFilter); fs.writeFileSync(manifest, committedBytes); runGit(brokenFilter, ['add', 'manifest.json']); runGit(brokenFilter, ['-c', 'user.email=a@b', '-c', 'user.name=a', 'commit', '-qm', 'x']); From 1fccf71eeeec411abdf1fd004fba35143e2951c4 Mon Sep 17 00:00:00 2001 From: Eduardo Marquez <55303379+DocksDocks@users.noreply.github.com> Date: Fri, 21 Aug 2026 11:12:24 -0300 Subject: [PATCH 3/3] fix(release): keep ambient git config out of the release tests The dry-run truth work added real-git fixtures, and two of them let the developer's environment decide the result. `releaseTagExists` called `git ls-remote` directly, so the default suite path reached the network: `node scripts/ci.mjs` failed with "cannot reach origin to check whether plan-lifecycle--v0.6.0 is already released" whenever origin was unreachable. The consult is now the sixteenth closed-set operation, `tagPublished`, keeping the local-tag-then-origin order and the refusal text byte for byte, with the fixture stubbing it false. The same-version consult and the already-released refusal are now asserted instead of inherited. `git help config` documents that `GIT_CONFIG_KEY_` pairs override every configuration file, a fixture's own `--local` settings included, so pointing `GIT_CONFIG_GLOBAL` at devNull was not isolation. The adapter spawns git with this process's environment and must keep doing so, because in production it has to predict what the operator's `git add` would stage; the boundary is therefore the test process, which now drops the pairs around the adapter contracts and restores every prior value. The shim matrix carries the same zero pair count. --- scripts/AGENTS.md | 2 +- scripts/lib/plugin-release.mjs | 42 ++++--- scripts/tests/ci-plugin-targeting.mjs | 174 +++++++++++++++++++++++--- 3 files changed, 184 insertions(+), 34 deletions(-) diff --git a/scripts/AGENTS.md b/scripts/AGENTS.md index 0320cfe2..3901d272 100644 --- a/scripts/AGENTS.md +++ b/scripts/AGENTS.md @@ -26,7 +26,7 @@ The repo hosts **multiple plugins** (`docks`, `plan-lifecycle`, `effect-kit`) un | `transformGuard` | run `transform-guard.mjs` (curated transformers) | | `release` | Closed, data-only release policy. Every plugin declares exactly `{ kind: 'generic', install }`. No callbacks, commands, safety gates, or ordering belong in descriptors. | -`lib/plugin-release.mjs` owns ordinary release ordering behind `runGenericPluginRelease({ argv, repo, plugins, io })`. Its IO value is an exact closed adapter that carries fifteen filesystem, Git, Claude, GitHub, selected-CI, and logging operations. `release.mjs` composes the production operations. Descriptors remain inert policy data. The fifteenth operation, `wouldStageChange`, answers whether `git add` of release bytes would stage anything different from HEAD. Production hashes the proposed bytes with `git hash-object --path --stdin`. It compares that hash with `git rev-parse --quiet --verify HEAD:`. `--path` applies the same clean filters that `git add` applies. The probe never passes `-w`, so it never writes an object. The engine validates every policy before touching IO. It enforces dry-run no-mutation itself rather than trusting an adapter. +`lib/plugin-release.mjs` owns ordinary release ordering behind `runGenericPluginRelease({ argv, repo, plugins, io })`. Its IO value is an exact closed adapter that carries sixteen filesystem, Git, Claude, GitHub, selected-CI, and logging operations. `release.mjs` composes the production operations. Descriptors remain inert policy data. The fifteenth operation, `wouldStageChange`, answers whether `git add` of release bytes would stage anything different from HEAD. Production hashes the proposed bytes with `git hash-object --path --stdin`. It compares that hash with `git rev-parse --quiet --verify HEAD:`. `--path` applies the same clean filters that `git add` applies. The probe never passes `-w`, so it never writes an object. The sixteenth operation, `tagPublished`, answers whether a release tag is already published: it checks the local ref, then asks origin, and refuses to guess when origin is unreachable. It is an adapter operation because reaching origin is IO. A caller that cannot stub it puts the network inside every test of the surrounding decision. The engine validates every policy before touching IO. It enforces dry-run no-mutation itself rather than trusting an adapter. `ci.mjs` is **registry-driven**. A full invocation runs repo-wide checks once (workflow YAML, both marketplace catalogs, tree/guard, durable anchors, author tooling, unit tests, and CI targeting), then selects every present plugin's shell hooks, repository author suites, and capability-driven `gatePlugin` work. `--plugin ` skips repo-wide sections and runs only the named plugin's owned author checks, target-derived shell lint, and plugin validation. When Docks plan author checks apply, CI runs `scripts/tests/plan-cli.mjs` plus `scripts/tests/plan-skill-phases.mjs` with the `bounded-workflows` and `plan-workspace-template` cases. Trigger-collision checks audit Docks and Effect Kit together once. diff --git a/scripts/lib/plugin-release.mjs b/scripts/lib/plugin-release.mjs index 73227404..1efe5372 100644 --- a/scripts/lib/plugin-release.mjs +++ b/scripts/lib/plugin-release.mjs @@ -20,6 +20,7 @@ const IO_KEYS = Object.freeze([ 'readReleaseNotes', 'resolveTagCommit', 'runSelectedCi', + 'tagPublished', 'waitForTagCi', 'wouldStageChange', 'writeJson', @@ -196,24 +197,6 @@ function parseGenericArgs(argv, plugins) { return { dryRun, plugin, versionArgument }; } -function releaseTagExists(tag, repo) { - if ( - spawnSync('git', ['rev-parse', '-q', '--verify', `refs/tags/${tag}`], { cwd: repo, stdio: 'ignore' }).status === 0 - ) - return true; - // Local tags are themselves only a proxy: nothing in this flow fetches, so a clone with stale - // refs would let an already-published version through. origin holds the fact. An unreachable - // origin must not read as "never released" — this flow pushes, tags, and creates a GitHub - // release, so it cannot proceed offline anyway, and refusing to guess costs nothing. - const remote = spawnSync('git', ['ls-remote', '--tags', 'origin', `refs/tags/${tag}`], { - cwd: repo, - encoding: 'utf8', - }); - if ((remote.status ?? 1) !== 0) - throw new Error(`cannot reach origin to check whether ${tag} is already released — refusing to guess`); - return (remote.stdout ?? '').trim() !== ''; -} - function nextVersion(current, requested) { const match = /^(\d+)\.(\d+)\.(\d+)$/.exec(current || ''); if (!match) throw new Error(`current version not semver: ${current}`); @@ -307,7 +290,7 @@ export async function runGenericPluginRelease({ argv, repo, plugins, io }) { // tagged. The manifest is a proxy for "already released", not the fact: a run that bumps the // manifest and then fails CI leaves the number written but unpublished, and the recovery this // tool itself prints is to re-cut exactly that number. - if (newVersion === currentVersion && releaseTagExists(`${plugin.name}--v${newVersion}`, repo)) { + if (newVersion === currentVersion && io.tagPublished(`${plugin.name}--v${newVersion}`)) { throw new Error(`already released: ${plugin.name} v${newVersion} (tag ${plugin.name}--v${newVersion} exists)`); } io.log(`Bumping ${plugin.name}: ${currentVersion} → ${newVersion}`); @@ -492,6 +475,27 @@ export function createGenericPluginReleaseIo({ repo, plugins }) { runSelectedCi(_plugin, ciArgs) { return spawnSync('node', [path.join(repo, 'scripts/ci.mjs'), ...ciArgs], { stdio: 'inherit' }); }, + // Reaching origin is IO, so it belongs in the closed set: a caller that cannot stub it + // drags the network into every test of the surrounding decision. Behaviour is unchanged. + tagPublished(tag) { + if ( + spawnSync('git', ['rev-parse', '-q', '--verify', `refs/tags/${tag}`], { cwd: repo, stdio: 'ignore' }).status === + 0 + ) + return true; + // Local tags are themselves only a proxy: nothing in this flow fetches, so a clone with + // stale refs would let an already-published version through. origin holds the fact. An + // unreachable origin must not read as "never released" — this flow pushes, tags, and + // creates a GitHub release, so it cannot proceed offline anyway, and refusing to guess + // costs nothing. + const remote = spawnSync('git', ['ls-remote', '--tags', 'origin', `refs/tags/${tag}`], { + cwd: repo, + encoding: 'utf8', + }); + if ((remote.status ?? 1) !== 0) + throw new Error(`cannot reach origin to check whether ${tag} is already released — refusing to guess`); + return (remote.stdout ?? '').trim() !== ''; + }, // `headBranch` carries the pushed ref's short name, so for a tag push it is the tag // itself (verified against `gh run list --json headBranch` on this repository: every // push run reports its `--v` tag). Commit plus event alone cannot diff --git a/scripts/tests/ci-plugin-targeting.mjs b/scripts/tests/ci-plugin-targeting.mjs index a0ce4073..6d489092 100755 --- a/scripts/tests/ci-plugin-targeting.mjs +++ b/scripts/tests/ci-plugin-targeting.mjs @@ -297,10 +297,8 @@ function gitSnapshot() { return { status: run(['status', '--porcelain=v1', '--untracked-files=all']), refs: run(['show-ref']), - // The shim passes nested `node` argv through with the real PATH, so a Node child can reach - // real git outside the call log. Objects are the one write such a child could make that - // neither status nor refs would show, so the safety claim has to count them too. - objects: run(['count-objects', '-v']), + // Belt and braces only: the shim below is what makes mutation impossible, because every + // git call the release makes has to match one of three read-only shapes to run at all. manifests: manifests.map((file) => fs.readFileSync(path.join(ROOT, file), 'base64')), }; } @@ -313,8 +311,11 @@ const tool = ${JSON.stringify(name)}; const args = process.argv.slice(2); fs.appendFileSync(process.env.DOCKS_RELEASE_CALL_LOG, JSON.stringify({ tool, args }) + '\\n'); // Only the three read-only invocations the dry run is allowed to make reach real git, matched -// argument for argument. A looser match keyed on the subcommand alone would let a mutating -// form such as \`hash-object -w\` through the harness that exists to prove nothing mutates. +// argument for argument, which alone refuses every write form: a real \`hash-object -w\` carries +// a different argument count, and \`--path -w\` consumes the flag as its path operand and writes +// nothing. The \`-w\` rejection below is defence in depth for a future widened shape, and it is +// observable exactly there: matching by subcommand alone keeps the write form refused while the +// read-only near misses start passing. const readOnlyGit = [ ['status', '--porcelain'], ['hash-object', '--path', null, '--stdin'], @@ -327,9 +328,21 @@ const passThroughGit = shape.length === args.length && shape.every((token, index) => token === null || token === args[index]), ) && !args.includes('-w'); -if (tool === 'node' || passThroughGit) { - const command = tool === 'node' ? process.env.DOCKS_RELEASE_REAL_NODE : tool; - const child = spawnSync(command, args, { +// The release runs the selected-plugin gate through \`node scripts/ci.mjs\`. Executing it here +// would spawn a descendant with the real PATH, and every git call that descendant makes would +// escape this shim and its log, so the one Node argv the release is allowed to use is stubbed +// green instead. The gate is covered by the gate's own CI job. With no descendant left, the +// three shapes above are the only way any git command runs at all, and anything else exits 97. +const ciStub = ['-q', '--plugin']; +const isSelectedCiGate = + tool === 'node' && + args.length === 4 && + args[0].endsWith('/scripts/ci.mjs') && + args[1] === ciStub[0] && + args[2] === ciStub[1]; +if (isSelectedCiGate) process.exit(0); +if (passThroughGit) { + const child = spawnSync(tool, args, { stdio: 'inherit', env: { ...process.env, PATH: process.env.DOCKS_RELEASE_REAL_PATH }, }); @@ -823,6 +836,12 @@ function genericReleaseIo(repo, options = {}) { assert.equal(plugin.name, ciArgs.at(-1)); return { status: 0, stdout: '', stderr: '' }; }, + // Stubbed false by default: the production adapter reaches origin, and a fixture that + // could not override it would put the network inside every same-version contract case. + tagPublished(tag) { + record('tagPublished', [tag]); + return options.tagPublished ?? false; + }, waitForTagCi(tag, commit, pushed) { record('waitForTagCi', [tag, commit, pushed]); return tagCiResult; @@ -966,6 +985,30 @@ async function testGenericReleaseModuleContract( false, 'unchanged re-cut preview must not predict a release commit', ); + // The re-cut is legal only while the version was never published, so the same-version path + // must consult origin through the closed set rather than reaching it directly. + assert.deepEqual( + unchangedRecut.calls.filter(({ tool }) => tool === 'tagPublished').map(({ args }) => args), + [[`${recutPlugin.name}--v${recutVersion}`]], + 'a same-version run must ask exactly once whether that tag is already published', + ); + const publishedRecut = genericReleaseIo(ROOT, { wouldStageChange: false, tagPublished: true }); + await assert.rejects( + () => + runGenericPluginRelease({ + argv: ['--dry-run', '--plugin', recutPlugin.name, recutVersion], + repo: ROOT, + plugins: PLUGINS, + io: publishedRecut.io, + }), + new RegExp(`already released: ${recutPlugin.name} v${recutVersion}`), + 'a published tag must refuse the re-cut instead of previewing it', + ); + assert.equal( + publishedRecut.calls.some(({ tool }) => ['writeJson', 'commit', 'push', 'createTag'].includes(tool)), + false, + 'the already-released refusal must precede every write', + ); const formattingOnlyRecut = genericReleaseIo(ROOT, { wouldStageChange: true }); await runGenericPluginRelease({ @@ -1353,7 +1396,6 @@ async function testDryRunReleaseSafety() { ...process.env, PATH: `${shimDir}${path.delimiter}${process.env.PATH ?? ''}`, DOCKS_RELEASE_CALL_LOG: callLog, - DOCKS_RELEASE_REAL_NODE: process.execPath, DOCKS_RELEASE_REAL_PATH: process.env.PATH ?? '', }, }); @@ -1377,6 +1419,14 @@ async function testDryRunReleaseSafety() { ), 'fixture must intercept and preserve the targeted Docks preflight', ); + // The structural safety claim: the gate argv is stubbed green, so no descendant process + // exists with the real PATH, and therefore every git call the release makes had to match + // one of the three read-only shapes to run at all. A second Node argv would break that. + assert.deepEqual( + calls.filter(({ tool }) => tool === 'node').map(({ args: callArgs }) => callArgs.slice(1).join(' ')), + ['-q --plugin docks'], + 'the release may run exactly one Node argv, the stubbed selected-plugin gate', + ); assert.ok( calls.some(({ tool, args: callArgs }) => tool === 'git' && callArgs[0] === 'hash-object'), 'dry-run fixture must pass the staged-content hash probe through to real git', @@ -1442,7 +1492,6 @@ async function testDryRunReleaseSafety() { ...process.env, PATH: `${shimDir}${path.delimiter}${process.env.PATH ?? ''}`, DOCKS_RELEASE_CALL_LOG: callLog, - DOCKS_RELEASE_REAL_NODE: process.execPath, DOCKS_RELEASE_REAL_PATH: process.env.PATH ?? '', }, }, @@ -2518,17 +2567,35 @@ await testGenericReleaseModuleContract( console.log('generic release module contract and dry-run manifest previews passed'); async function testReleaseAdapterGitContracts(createReleaseIo) { - const gitEnv = { - ...process.env, + // The adapter spawns git with this process's environment, and must keep doing so: in + // production it has to predict what the operator's own `git add` would stage. That makes the + // test process the isolation boundary. `GIT_CONFIG_KEY_`/`VALUE_` pairs override every + // configuration file, a fixture's `--local` settings included, so a hostile ambient pair can + // only be dropped here. Every key is restored below. + const isolation = { GIT_CONFIG_GLOBAL: os.devNull, + GIT_CONFIG_SYSTEM: os.devNull, GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_COUNT: '0', GIT_TERMINAL_PROMPT: '0', }; + const inherited = Object.fromEntries(Object.keys(isolation).map((key) => [key, process.env[key]])); + Object.assign(process.env, isolation); + try { + await runReleaseAdapterGitContracts(createReleaseIo); + } finally { + for (const [key, value] of Object.entries(inherited)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } +} + +async function runReleaseAdapterGitContracts(createReleaseIo) { const runGit = (repo, gitArgs) => { const result = spawnSync('git', ['-c', 'commit.gpgSign=false', '-c', `core.hooksPath=${os.devNull}`, ...gitArgs], { cwd: repo, encoding: 'utf8', - env: gitEnv, }); if ((result.status ?? 1) !== 0) { throw new Error( @@ -2539,6 +2606,10 @@ async function testReleaseAdapterGitContracts(createReleaseIo) { const configureFixtureRepo = (repo) => { runGit(repo, ['config', '--local', 'core.hooksPath', os.devNull]); runGit(repo, ['config', '--local', 'core.attributesFile', os.devNull]); + runGit(repo, ['config', '--local', 'core.autocrlf', 'false']); + runGit(repo, ['config', '--local', 'core.eol', 'lf']); + runGit(repo, ['config', '--local', 'core.safecrlf', 'false']); + runGit(repo, ['config', '--local', 'commit.gpgSign', 'false']); }; const notRepository = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-adapter-no-git-')); @@ -2630,3 +2701,78 @@ async function testReleaseAdapterGitContracts(createReleaseIo) { await testReleaseAdapterGitContracts(releaseModule.createGenericPluginReleaseIo); console.log('release adapter git predicates refuse to guess'); + +function testReleaseShimRefusalMatrix() { + // The dry-run safety scenario trusts the shim to be the only route to git. That claim is a + // property of the shim itself, so it is proven here on the default path rather than only + // behind `--dry-run-release-safety`, which needs a clean checkout and so cannot always run. + const shimRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'docks-release-shim-')); + try { + const callLog = path.join(shimRoot, 'calls.jsonl'); + fs.writeFileSync(callLog, '', { mode: 0o600 }); + for (const name of ['node', 'git', 'claude', 'gh']) writeReleaseShim(shimRoot, name); + const invoke = (tool, args, options = {}) => + spawnSync(path.join(shimRoot, tool), args, { + cwd: ROOT, + encoding: 'utf8', + env: { + ...process.env, + DOCKS_RELEASE_CALL_LOG: callLog, + DOCKS_RELEASE_REAL_PATH: process.env.PATH ?? '', + // The permitted shapes reach real git, so ambient configuration decides their exit + // status. Disable global and system config here as the adapter fixtures do, and drop + // any inherited `GIT_CONFIG_KEY_` pairs, which would otherwise override both files. + GIT_CONFIG_GLOBAL: os.devNull, + GIT_CONFIG_SYSTEM: os.devNull, + GIT_CONFIG_NOSYSTEM: '1', + GIT_CONFIG_COUNT: '0', + GIT_TERMINAL_PROMPT: '0', + }, + ...options, + }); + const manifest = 'plugins/docks/.claude-plugin/plugin.json'; + const allowed = [ + ['git', ['status', '--porcelain'], {}], + ['git', ['hash-object', '--path', manifest, '--stdin'], { input: '{}\n' }], + ['git', ['rev-parse', '--quiet', '--verify', `HEAD:${manifest}`], {}], + // The selected-plugin gate is stubbed green: executing it would spawn a descendant with + // the real PATH, whose git calls would bypass this shim entirely. The script path below + // must not exist, so a regression back to executing this argv fails to load and is caught + // here rather than quietly reintroducing the descendant. + ['node', [path.join(ROOT, 'no-such-directory/scripts/ci.mjs'), '-q', '--plugin', 'docks'], {}], + ]; + for (const [tool, args, options] of allowed) { + const result = invoke(tool, args, options); + assert.equal(result.status, 0, `${tool} ${args.join(' ')} must be permitted: ${result.stderr}`); + } + const refused = [ + ['git', ['hash-object', '-w', '--stdin']], + ['git', ['tag', 'docks--v9.9.9']], + ['git', ['commit', '-m', 'release']], + ['git', ['update-ref', 'refs/heads/main', 'HEAD']], + // A near miss must refuse too: the shapes match argument for argument, not by subcommand. + ['git', ['status']], + ['node', ['-e', 'process.exit(0)']], + ['node', [path.join(ROOT, 'scripts/ci.mjs'), '--plugin', 'docks']], + ['claude', ['plugin', 'tag', '--push']], + ['gh', ['release', 'create', 'docks--v9.9.9']], + ]; + for (const [tool, args] of refused) { + assert.equal(invoke(tool, args).status, 97, `${tool} ${args.join(' ')} must be refused`); + } + // Refusal stays observable: the log records the attempt before the decision, which is how + // the safety scenario can assert on calls the release was never allowed to make. + const logged = fs + .readFileSync(callLog, 'utf8') + .trim() + .split('\n') + .filter(Boolean) + .map((line) => JSON.parse(line)); + assert.equal(logged.length, allowed.length + refused.length, 'every attempt must be logged'); + } finally { + fs.rmSync(shimRoot, { recursive: true, force: true }); + } +} + +testReleaseShimRefusalMatrix(); +console.log('release shim refuses every argv but the read-only three and the stubbed gate');