Merge pull request #3618 from gsd-build/fix/3610-error-installing-v1-42-2-in-codex
fix(3610): unblock fresh Codex install when leftover bundled hooks present
This commit is contained in:
5
.changeset/3610-codex-install-bundled-hooks-blocker.md
Normal file
5
.changeset/3610-codex-install-bundled-hooks-blocker.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
issue: 3610
|
||||
---
|
||||
**Fresh `npx get-shit-done-cc@latest --codex` no longer hard-aborts when leftover bundled `hooks/gsd-*` files are present** — `classifyPromptUserAction` in `installer-migration-report.cjs` now recognizes the bundled GSD hooks (`hooks/gsd-<name>.{js,sh,cjs,mjs}`) as a known category (`bundled-gsd-hook`) and resolves them to `remove` so the installer can write the fresh bundled versions. The classifier-based safe-default resolver in `bin/install.js` now runs regardless of TTY state — gating it on `!isTTY` made interactive installs throw `installer migration blocked pending user choice` for files that have no actual user choice to make.
|
||||
@@ -8061,13 +8061,21 @@ function install(isGlobal, runtime = 'claude', options = {}) {
|
||||
// #3541: non-interactive runs (typical /gsd-update via Claude Code) have
|
||||
// no stdin TTY and therefore no way to answer prompt-user migration
|
||||
// actions. Resolve safe categories by classification (stale SDK build
|
||||
// artifacts → remove; user-facing skills → keep) and log every
|
||||
// resolution; anything that cannot be safely defaulted falls through
|
||||
// to assertInstallerMigrationsUnblocked, which now emits a grouped
|
||||
// error with the documented resolution path.
|
||||
// artifacts → remove; user-facing skills → keep; bundled GSD hooks →
|
||||
// remove [#3610]) and log every resolution; anything that cannot be
|
||||
// safely defaulted falls through to assertInstallerMigrationsUnblocked,
|
||||
// which now emits a grouped error with the documented resolution path.
|
||||
//
|
||||
// #3610: the classifier-based resolution must run regardless of TTY.
|
||||
// For unambiguous categories (e.g. `hooks/gsd-*` bundled hooks left
|
||||
// behind by a previous version), there is no actual "user choice" to
|
||||
// make — the file is a known GSD-managed artifact and the installer is
|
||||
// about to write the fresh bundled version. Gating the resolver on
|
||||
// `!isTTY` made `npx get-shit-done-cc@latest --codex` hard-abort with
|
||||
// 12 blocked bundled hooks. The env-override branch (operator-supplied
|
||||
// GSD_INSTALLER_MIGRATION_RESOLVE) still applies only in non-TTY mode.
|
||||
const _migrationIsTty = process.stdin && process.stdin.isTTY === true;
|
||||
if (!_migrationIsTty &&
|
||||
Array.isArray(installerMigrationResult.blocked) &&
|
||||
if (Array.isArray(installerMigrationResult.blocked) &&
|
||||
installerMigrationResult.blocked.length > 0 &&
|
||||
installerMigrationResult.plan &&
|
||||
Array.isArray(installerMigrationResult.plan.actions)) {
|
||||
|
||||
@@ -115,6 +115,17 @@ function classifyPromptUserAction(action) {
|
||||
if (/^skills\/gsd-[^/]+\/SKILL\.md$/.test(relPath)) {
|
||||
return { category: 'user-facing-skill', choice: 'keep' };
|
||||
}
|
||||
// #3610: bundled GSD hooks at hooks/gsd-<name>.<ext>. These are part of
|
||||
// the npm distribution (`hooks/gsd-*.{js,sh,cjs,mjs}` shipped in the
|
||||
// package), NOT user-owned files. When a first-time-baseline scan finds
|
||||
// them on disk without manifest entries — the case for any upgrade from
|
||||
// a pre-manifest-baseline release — the safe default is to remove them
|
||||
// so the installer can write the fresh bundled versions in their place.
|
||||
// Restricted to top-level files (`hooks/gsd-X.ext`) so nested user
|
||||
// directories like `hooks/gsd-helpers/...` do NOT auto-classify.
|
||||
if (/^hooks\/gsd-[^/]+\.(?:js|sh|cjs|mjs)$/.test(relPath)) {
|
||||
return { category: 'bundled-gsd-hook', choice: 'remove' };
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -206,10 +217,17 @@ function resolveInstallerMigrationPromptsForNonTty(result, options = {}) {
|
||||
|
||||
if (choice) {
|
||||
const resolved = materializeResolution(action, choice);
|
||||
// Inject the concrete action into plan.actions so the apply
|
||||
// step picks it up.
|
||||
// Replace the original prompt-user action in-place when present so
|
||||
// applyInstallerMigrationPlan never sees an unsupported action type.
|
||||
// Fallback to append only when the blocked action did not originate
|
||||
// from plan.actions (defensive).
|
||||
if (result.plan && Array.isArray(result.plan.actions)) {
|
||||
result.plan.actions.push(resolved);
|
||||
const idx = result.plan.actions.indexOf(action);
|
||||
if (idx >= 0) {
|
||||
result.plan.actions[idx] = resolved;
|
||||
} else {
|
||||
result.plan.actions.push(resolved);
|
||||
}
|
||||
}
|
||||
resolutions.push({
|
||||
relPath: action.relPath,
|
||||
|
||||
@@ -0,0 +1,190 @@
|
||||
/**
|
||||
* Regression test for #3610: fresh `npx get-shit-done-cc@latest --codex`
|
||||
* hard-aborts when the target ~/.codex/hooks/ contains the bundled GSD
|
||||
* hook files (`gsd-check-update-worker.js`, `gsd-prompt-guard.js`, …)
|
||||
* left over from a previous version. The installer-migration report
|
||||
* classifies them as "GSD-looking file is not proven manifest-managed
|
||||
* and needs explicit user choice" and `assertInstallerMigrationsUnblocked`
|
||||
* throws.
|
||||
*
|
||||
* The files in question are NOT user-owned — they are the GSD bundled
|
||||
* hooks shipped under `hooks/gsd-*` in the npm package. The fix adds a
|
||||
* `bundled-gsd-hook` classification to `classifyPromptUserAction` so the
|
||||
* resolver removes them (the installer then writes the fresh bundled
|
||||
* versions in their place).
|
||||
*
|
||||
* Because this classification is unambiguous (these are not user files),
|
||||
* it must apply regardless of whether stdin is a TTY — the reporter's
|
||||
* `npx ... --codex` run was interactive and the existing non-TTY
|
||||
* resolver gate at install.js:8069 skipped the safe-default pass.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
process.env.GSD_TEST_MODE = '1';
|
||||
|
||||
const { describe, test, beforeEach, afterEach } = require('node:test');
|
||||
const assert = require('node:assert/strict');
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
const crypto = require('node:crypto');
|
||||
|
||||
const {
|
||||
runInstallerMigrations,
|
||||
} = require('../get-shit-done/bin/lib/installer-migrations.cjs');
|
||||
const {
|
||||
assertInstallerMigrationsUnblocked,
|
||||
resolveInstallerMigrationPromptsForNonTty,
|
||||
classifyPromptUserAction,
|
||||
} = require('../get-shit-done/bin/lib/installer-migration-report.cjs');
|
||||
const { createTempDir, cleanup } = require('./helpers.cjs');
|
||||
|
||||
function writeFile(root, relPath, content) {
|
||||
const fullPath = path.join(root, relPath);
|
||||
fs.mkdirSync(path.dirname(fullPath), { recursive: true });
|
||||
fs.writeFileSync(fullPath, content, 'utf8');
|
||||
}
|
||||
|
||||
function writeManifest(root, files) {
|
||||
fs.writeFileSync(
|
||||
path.join(root, 'gsd-file-manifest.json'),
|
||||
JSON.stringify(
|
||||
{
|
||||
version: '1.41.2',
|
||||
timestamp: '2026-05-10T00:00:00.000Z',
|
||||
mode: 'full',
|
||||
files,
|
||||
},
|
||||
null,
|
||||
2,
|
||||
),
|
||||
'utf8',
|
||||
);
|
||||
}
|
||||
|
||||
// Reporter's exact list of blocked files from the v1.42.2 → v1.42.0 upgrade
|
||||
// abort. Each is a real `hooks/gsd-*` file shipped under hooks/ in the npm
|
||||
// package (verified by `ls hooks/`).
|
||||
const BUNDLED_HOOK_RELPATHS = [
|
||||
'hooks/gsd-check-update-worker.js',
|
||||
'hooks/gsd-check-update.js',
|
||||
'hooks/gsd-context-monitor.js',
|
||||
'hooks/gsd-phase-boundary.sh',
|
||||
'hooks/gsd-prompt-guard.js',
|
||||
'hooks/gsd-read-guard.js',
|
||||
'hooks/gsd-read-injection-scanner.js',
|
||||
'hooks/gsd-session-state.sh',
|
||||
'hooks/gsd-statusline.js',
|
||||
'hooks/gsd-update-banner.js',
|
||||
'hooks/gsd-validate-commit.sh',
|
||||
'hooks/gsd-workflow-guard.js',
|
||||
];
|
||||
|
||||
describe('bug #3610: classifyPromptUserAction recognizes bundled GSD hooks', () => {
|
||||
test('classifies hooks/gsd-*.js as bundled-gsd-hook → remove', () => {
|
||||
const result = classifyPromptUserAction({
|
||||
relPath: 'hooks/gsd-prompt-guard.js',
|
||||
});
|
||||
assert.ok(result, 'classifier returned null for a bundled GSD hook (.js)');
|
||||
assert.strictEqual(result.category, 'bundled-gsd-hook');
|
||||
assert.strictEqual(
|
||||
result.choice,
|
||||
'remove',
|
||||
'bundled hook must default to remove so the installer can write the fresh bundled version',
|
||||
);
|
||||
});
|
||||
|
||||
test('classifies hooks/gsd-*.sh as bundled-gsd-hook → remove', () => {
|
||||
const result = classifyPromptUserAction({
|
||||
relPath: 'hooks/gsd-validate-commit.sh',
|
||||
});
|
||||
assert.ok(result);
|
||||
assert.strictEqual(result.category, 'bundled-gsd-hook');
|
||||
assert.strictEqual(result.choice, 'remove');
|
||||
});
|
||||
|
||||
test('does NOT classify non-gsd hooks (preserves user-owned hook files)', () => {
|
||||
// A user's custom hook that happens to live under hooks/ must NOT be
|
||||
// auto-classified as bundled — the existing block-then-choose flow
|
||||
// continues to apply, preserving the user's control over their files.
|
||||
const result = classifyPromptUserAction({
|
||||
relPath: 'hooks/my-custom-hook.js',
|
||||
});
|
||||
assert.strictEqual(
|
||||
result,
|
||||
null,
|
||||
'non-gsd-prefixed hook must NOT auto-classify (would clobber user files)',
|
||||
);
|
||||
});
|
||||
|
||||
test('does NOT classify deeper paths under hooks/gsd-* (e.g. hooks/lib/) as bundled-gsd-hook', () => {
|
||||
// The bundled GSD distribution has hooks/lib/ (helper modules). Those
|
||||
// are managed differently — verify the classifier limits itself to
|
||||
// top-level hooks/gsd-<name>.<ext> files, not nested directories.
|
||||
const result = classifyPromptUserAction({
|
||||
relPath: 'hooks/gsd-helpers/index.js',
|
||||
});
|
||||
assert.strictEqual(result, null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('bug #3610: fresh upgrade with leftover bundled hooks does not throw', () => {
|
||||
let configDir;
|
||||
|
||||
beforeEach(() => {
|
||||
configDir = createTempDir('gsd-3610-');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
cleanup(configDir);
|
||||
});
|
||||
|
||||
test('end-to-end: 12 leftover bundled hooks + empty manifest → resolver clears all blockers', () => {
|
||||
// Recreate the reporter's environment: 12 bundled `gsd-*` hook files
|
||||
// present at target, but the manifest has not yet seeded their baseline
|
||||
// entries (first-time-baseline scan).
|
||||
for (const rel of BUNDLED_HOOK_RELPATHS) {
|
||||
writeFile(configDir, rel, '#!/usr/bin/env node\n// stale 1.42.0 hook\n');
|
||||
}
|
||||
writeManifest(configDir, {});
|
||||
|
||||
const result = runInstallerMigrations({
|
||||
configDir,
|
||||
runtime: 'codex',
|
||||
scope: 'global',
|
||||
baselineScan: true,
|
||||
});
|
||||
|
||||
// Precondition: all 12 leftover hooks classify as prompt-user blockers.
|
||||
const blockedPaths = (result.blocked || []).map((a) => a.relPath).sort();
|
||||
assert.deepStrictEqual(
|
||||
blockedPaths,
|
||||
[...BUNDLED_HOOK_RELPATHS].sort(),
|
||||
'precondition: every leftover hooks/gsd-* should be a prompt-user blocker',
|
||||
);
|
||||
|
||||
// Resolve through the safe-default classifier (passing isTty=false to
|
||||
// exercise the same code path the bundled-hook classification will hit
|
||||
// regardless of TTY once the fix removes the gate).
|
||||
const resolved = resolveInstallerMigrationPromptsForNonTty(result, { isTty: false });
|
||||
|
||||
assert.strictEqual(
|
||||
resolved.resolutions.length,
|
||||
BUNDLED_HOOK_RELPATHS.length,
|
||||
'every bundled hook should produce a safe-default resolution entry',
|
||||
);
|
||||
|
||||
for (const entry of resolved.resolutions) {
|
||||
assert.strictEqual(entry.category, 'bundled-gsd-hook');
|
||||
assert.strictEqual(entry.choice, 'remove');
|
||||
assert.strictEqual(entry.resolvedActionType, 'backup-and-remove');
|
||||
}
|
||||
|
||||
assert.strictEqual(
|
||||
(resolved.result.blocked || []).length,
|
||||
0,
|
||||
'no blockers should remain after bundled-hook classification fires',
|
||||
);
|
||||
assert.doesNotThrow(() => assertInstallerMigrationsUnblocked(resolved.result));
|
||||
});
|
||||
});
|
||||
@@ -306,7 +306,7 @@ describe('installer migration install integration', { concurrency: false }, () =
|
||||
});
|
||||
|
||||
test('blocks install before materialization when baseline needs explicit user choice', () => {
|
||||
writeFile(codexHome, 'hooks/gsd-retired-hook.js', 'old gsd hook\n');
|
||||
writeFile(codexHome, 'hooks/gsd-retired-hook.txt', 'old gsd hook\n');
|
||||
|
||||
assert.throws(
|
||||
() => captureConsole(() =>
|
||||
@@ -315,7 +315,7 @@ describe('installer migration install integration', { concurrency: false }, () =
|
||||
/installer migration blocked/
|
||||
);
|
||||
|
||||
assert.equal(fs.readFileSync(path.join(codexHome, 'hooks/gsd-retired-hook.js'), 'utf8'), 'old gsd hook\n');
|
||||
assert.equal(fs.readFileSync(path.join(codexHome, 'hooks/gsd-retired-hook.txt'), 'utf8'), 'old gsd hook\n');
|
||||
assert.equal(fs.existsSync(path.join(codexHome, 'skills')), false);
|
||||
assert.equal(fs.existsSync(path.join(codexHome, 'get-shit-done', 'VERSION')), false);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user