fix(install): comprehensive audit cleanup of hook copy, uninstall, and manifest (#1755) (#1768)

- Add chmod +x for .sh hooks during install (fixes #1755 permission denied)
- Fix Codex hook: wrong path (get-shit-done/hooks/) and inverted filename (gsd-update-check.js → gsd-check-update.js)
- Fix cache invalidation path from ~/cache/ to ~/.cache/gsd/
- Track .sh hooks in writeManifest so saveLocalPatches detects modifications
- Add gsd-workflow-guard.js to uninstall file cleanup list
- Add community hooks (session-state, validate-commit, phase-boundary) to uninstall settings.json cleanup
- Remove phantom gsd-check-update.sh from uninstall list
- Remove dead isCursor/isWindsurf branches in uninstall (already handled by combined branch)
- Warn when expected .sh hooks are missing after verifyInstalled
- Add 15 regression tests in install-hooks-copy.test.cjs
- Update codex-config.test.cjs assertions for corrected hook filename

Fixes #1755
This commit is contained in:
Jeremy McSpadden
2026-04-05 10:37:27 -05:00
committed by GitHub
parent c7d25b183a
commit 84de0cc760
4 changed files with 404 additions and 64 deletions

View File

@@ -756,7 +756,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('[features]\ncodex_hooks = true\n'), 'writes codex_hooks feature');
assert.ok(content.includes('# GSD Hooks\n[[hooks]]\nevent = "SessionStart"\n'), 'writes GSD SessionStart hook block');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'writes one codex_hooks key');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'writes one GSD update hook');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'writes one GSD update hook');
assertNoDraftRootKeys(content);
assertUsesOnlyEol(content, '\n');
});
@@ -838,7 +838,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('# user comment'), 'preserves user comment');
assert.ok(content.includes('[model]\nname = "o3"'), 'preserves model section');
assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'adds one GSD update hook');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook');
assertNoDraftRootKeys(content);
});
@@ -975,7 +975,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(!content.includes('codex_hooks = false'), 'removes false codex_hooks value');
assert.ok(content.includes('other_feature = true'), 'preserves other feature keys');
assert.ok(content.includes('command = "echo custom"'), 'preserves custom hook');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'does not duplicate GSD update hook');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'does not duplicate GSD update hook');
assertNoDraftRootKeys(content);
});
@@ -1017,7 +1017,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^"codex_hooks" = true$/gm), 1, 'normalizes the quoted codex_hooks key to true');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a second bare features table');
assert.ok(content.includes('other_feature = true'), 'preserves existing feature keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'keeps one GSD update hook');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'keeps one GSD update hook');
assertNoDraftRootKeys(content);
});
@@ -1038,7 +1038,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('[features."a#b"]\nenabled = true'), 'preserves the quoted nested features table');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'adds one real top-level features table');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'adds one codex_hooks key');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'remains idempotent for the GSD hook block');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block');
assertNoDraftRootKeys(content);
});
@@ -1058,7 +1058,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not add a [features] table');
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 1, 'adds one dotted codex_hooks key');
assert.ok(content.includes('features.other_feature = true'), 'preserves existing dotted features key');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'adds one GSD update hook for dotted codex_hooks and remains idempotent');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook for dotted codex_hooks and remains idempotent');
assertNoDraftRootKeys(content);
});
@@ -1078,7 +1078,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('features = { other_feature = true }'), 'preserves the root inline-table assignment');
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block');
assertNoDraftRootKeys(content);
});
@@ -1099,7 +1099,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('features = "disabled"'), 'preserves the root scalar assignment');
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append an invalid dotted codex_hooks key');
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 0, 'does not prepend a features table');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 0, 'does not add the GSD hook block when codex_hooks cannot be enabled safely');
assert.ok(content.includes('[agents.gsd-executor]'), 'still installs the managed agent block');
assertNoDraftRootKeys(content);
});
@@ -1122,7 +1122,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^features\."codex_hooks" = true$/gm), 1, 'normalizes the quoted dotted key to true');
assert.strictEqual(countMatches(content, /^features\.codex_hooks = true$/gm), 0, 'does not append a bare dotted duplicate');
assert.ok(content.includes('features.other_feature = true'), 'preserves other dotted features keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'adds one GSD update hook for quoted dotted codex_hooks and remains idempotent');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds one GSD update hook for quoted dotted codex_hooks and remains idempotent');
assertNoDraftRootKeys(content);
});
@@ -1230,7 +1230,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'replaces the multiline basic-string assignment with one true value');
assert.ok(!content.includes('multiline-basic-sentinel'), 'removes multiline basic-string continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'remains idempotent for the GSD hook block');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block');
assertNoDraftRootKeys(content);
});
@@ -1255,7 +1255,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'replaces the multiline literal-string assignment with one true value');
assert.ok(!content.includes('multiline-literal-sentinel'), 'removes multiline literal-string continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'remains idempotent for the GSD hook block');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block');
assertNoDraftRootKeys(content);
});
@@ -1281,7 +1281,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(!content.includes('array-sentinel-1'), 'removes multiline array continuation lines');
assert.ok(!content.includes('array-sentinel-2'), 'removes multiline array continuation lines');
assert.ok(content.includes('other_feature = true'), 'preserves following feature keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'remains idempotent for the GSD hook block');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'remains idempotent for the GSD hook block');
assertNoDraftRootKeys(content);
});
@@ -1326,7 +1326,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'keeps one codex_hooks = true');
assert.ok(content.includes('other_feature = true'), 'preserves other feature keys');
assert.strictEqual(countMatches(content, /echo custom-after-command/g), 1, 'preserves non-GSD hook exactly once');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'keeps one GSD update hook');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'keeps one GSD update hook');
assertUsesOnlyEol(content, '\r\n');
assertNoDraftRootKeys(content);
});
@@ -1349,7 +1349,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.strictEqual(countMatches(content, /^\[features\]\s*$/gm), 1, 'keeps one [features] section');
assert.strictEqual(countMatches(content, /^codex_hooks = true # keep me$/gm), 1, 'preserves the commented true value');
assert.ok(content.includes('other_feature = true'), 'preserves other feature keys');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'adds the GSD update hook once');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'adds the GSD update hook once');
assertNoDraftRootKeys(content);
});
@@ -1366,7 +1366,7 @@ describe('Codex install hook configuration (e2e)', () => {
assert.ok(content.includes('# GSD Hooks\n[[hooks]]\nevent = "SessionStart"\n'), 'writes the GSD hook block using the first newline style');
assert.ok(content.includes('[model]\r\nname = "o3"'), 'preserves the existing CRLF model lines');
assert.strictEqual(countMatches(content, /^codex_hooks = true$/gm), 1, 'remains idempotent on repeated installs');
assert.strictEqual(countMatches(content, /gsd-update-check\.js/g), 1, 'does not duplicate the GSD hook block');
assert.strictEqual(countMatches(content, /gsd-check-update\.js/g), 1, 'does not duplicate the GSD hook block');
assertNoDraftRootKeys(content);
});
});
@@ -1415,7 +1415,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
assert.ok(cleaned.includes('# keep me'), 'preserves user comments in [features]');
assert.ok(cleaned.includes('other_feature = true'), 'preserves other feature keys');
assert.strictEqual(countMatches(cleaned, /echo custom-after-command/g), 1, 'preserves non-GSD hooks');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes only the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes only the GSD update hook');
assert.strictEqual(countMatches(cleaned, /\[agents\.gsd-/g), 0, 'removes managed GSD agent sections');
assertUsesOnlyEol(cleaned, '\r\n');
});
@@ -1440,7 +1440,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
assert.strictEqual(countMatches(cleaned, /^features\.codex_hooks = true$/gm), 0, 'removes the dotted GSD codex_hooks key');
assert.strictEqual(countMatches(cleaned, /^\[features\]\s*$/gm), 0, 'does not leave behind a [features] table');
assert.strictEqual(countMatches(cleaned, /echo custom-after-command/g), 1, 'preserves non-GSD hooks');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes the GSD update hook');
});
test('install then uninstall preserves a pre-existing [features].codex_hooks = true', () => {
@@ -1459,7 +1459,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
const cleaned = stripGsdFromCodexConfig(readCodexConfig(codexHome));
assert.ok(cleaned.includes('[features]\ncodex_hooks = true\nother_feature = true'), 'preserves the user-authored codex_hooks assignment');
assert.strictEqual(countMatches(cleaned, /^codex_hooks = true$/gm), 1, 'keeps the pre-existing codex_hooks key');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /\[agents\.gsd-/g), 0, 'removes managed GSD agent sections');
});
@@ -1479,7 +1479,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
const cleaned = stripGsdFromCodexConfig(readCodexConfig(codexHome));
assert.ok(cleaned.includes('[features]\n"codex_hooks" = true\nother_feature = true'), 'preserves the user-authored quoted codex_hooks assignment');
assert.strictEqual(countMatches(cleaned, /^"codex_hooks" = true$/gm), 1, 'keeps the pre-existing quoted codex_hooks key');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /\[agents\.gsd-/g), 0, 'removes managed GSD agent sections');
});
@@ -1498,7 +1498,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
const cleaned = stripGsdFromCodexConfig(readCodexConfig(codexHome));
assert.ok(cleaned.includes('features.codex_hooks = true\nfeatures.other_feature = true'), 'preserves the user-authored dotted codex_hooks assignment');
assert.strictEqual(countMatches(cleaned, /^features\.codex_hooks = true$/gm), 1, 'keeps the pre-existing dotted codex_hooks key');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /\[agents\.gsd-/g), 0, 'removes managed GSD agent sections');
});
@@ -1537,7 +1537,7 @@ describe('Codex uninstall symmetry for hook-enabled configs', () => {
const cleaned = stripGsdFromCodexConfig(readCodexConfig(codexHome));
assert.ok(cleaned.includes('# first line wins\n[features]\r\nother_feature = true\r\n\r\n[model]\r\nname = "o3"'), 'preserves the original mixed-EOL user content');
assert.strictEqual(countMatches(cleaned, /^codex_hooks = true$/gm), 0, 'removes the injected codex_hooks key');
assert.strictEqual(countMatches(cleaned, /gsd-update-check\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /gsd-check-update\.js/g), 0, 'removes the GSD update hook');
assert.strictEqual(countMatches(cleaned, /\[agents\.gsd-/g), 0, 'removes managed GSD agent sections');
});
});