fix(3610): unblock fresh Codex install when leftover bundled hooks present

`npx get-shit-done-cc@latest --codex` aborted with
"installer migration blocked pending user choice" listing 12 hooks/gsd-*
files. Those files are part of the GSD npm distribution
(hooks/gsd-prompt-guard.js, hooks/gsd-context-monitor.js, etc.), not
user-owned content, so asking the user to choose between keep/remove for
them was a UX bug, not a real choice. The installer is about to write
the fresh bundled versions in their place.

Root cause: `classifyPromptUserAction` in
get-shit-done/bin/lib/installer-migration-report.cjs knew two
unambiguous categories (`stale-sdk-build-artifact`, `user-facing-skill`)
but had no rule for the bundled GSD hooks. The first-time-baseline scan
classified them as `stale-gsd-looking` prompt-user blockers, and
`assertInstallerMigrationsUnblocked` threw.

A second gate compounded the bug: the safe-default resolver in
bin/install.js was wrapped in `if (!_migrationIsTty)`, so even with a
correct classification rule, TTY runs (every `npx get-shit-done-cc`
invocation) skipped the resolver and went straight to the hard throw.

Fix:
1) Add `hooks/gsd-<name>.(js|sh|cjs|mjs)` to `classifyPromptUserAction`
   as `bundled-gsd-hook` → `remove`. The regex is anchored at the
   top-level `hooks/` directory so nested paths like
   `hooks/gsd-helpers/index.js` (or any user-owned helper directory) do
   NOT auto-classify.
2) Remove the `!_migrationIsTty` gate from the resolver call in
   bin/install.js. The classifier-based path is unambiguous and must
   apply regardless of TTY; the env-override branch
   (GSD_INSTALLER_MIGRATION_RESOLVE) still applies only when isTty=false
   inside the resolver, preserving the #3541 semantic.

Regression test added
(tests/bug-3610-installer-migration-bundled-hooks-classification.test.cjs):

- Positive: hooks/gsd-*.{js,sh} → category=bundled-gsd-hook, choice=remove.
- Counter-test: hooks/my-custom-hook.js → classifier returns null
  (user files are preserved).
- Boundary: hooks/gsd-helpers/index.js → classifier returns null
  (nested directories don't auto-classify).
- End-to-end: 12 reporter-exact bundled hooks + empty manifest →
  resolver clears every blocker, assertInstallerMigrationsUnblocked
  does not throw.

Test exercises the real installer-migration code path
(`runInstallerMigrations` + `resolveInstallerMigrationPromptsForNonTty`
+ `assertInstallerMigrationsUnblocked`) — no source-grep, no raw text
matching on outputs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Tom Boucher
2026-05-15 22:13:34 -04:00
parent 38c35a608d
commit f422a05450
4 changed files with 220 additions and 6 deletions

View 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.

View File

@@ -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)) {

View File

@@ -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;
}

View File

@@ -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));
});
});