fix(codex): remove legacy hooks json update hook

.changeset/fix-3357-codex-legacy-hooks-json.md

@@ -0,0 +1,5 @@
+---
+type: Fixed
+pr: 3364
+---
+**Codex installs now clean up legacy GSD-managed `hooks.json` update hooks after writing the TOML SessionStart hook** — reinstalling no longer leaves duplicate GSD update hooks across `hooks.json` and `config.toml`, while user-owned JSON hooks are preserved. (#3357)

bin/install.js

@@ -759,6 +759,76 @@ function rewriteLegacyCodexHookBlock(content, absoluteRunner) {
   return { content: updated, changed };
 }
 
+function isManagedCodexHookCommand(command, targetDir) {
+  if (typeof command !== 'string') return false;
+  if (typeof targetDir !== 'string' || targetDir.length === 0) return false;
+  const normalizedCommand = command.replace(/\\/g, '/');
+  const managedHooksDir = `${path.join(targetDir, 'hooks').replace(/\\/g, '/')}/`;
+  if (!normalizedCommand.includes(managedHooksDir)) return false;
+  return /(^|[\\/\s"'])(gsd-check-update\.js|gsd-update-check\.js)(?=$|[\s"'])/.test(normalizedCommand);
+}
+
+function pruneGsdManagedHooksJsonValue(value, targetDir) {
+  if (Array.isArray(value)) {
+    let changed = false;
+    const next = [];
+    for (const item of value) {
+      const pruned = pruneGsdManagedHooksJsonValue(item, targetDir);
+      if (pruned.changed) changed = true;
+      if (!isStructurallyEmpty(pruned.value)) next.push(pruned.value);
+      else changed = true;
+    }
+    return { value: next, changed };
+  }
+
+  if (value && typeof value === 'object') {
+    if (isManagedCodexHookCommand(value.command, targetDir)) {
+      return { value: null, changed: true };
+    }
+
+    let changed = false;
+    const next = {};
+    for (const [key, child] of Object.entries(value)) {
+      const pruned = pruneGsdManagedHooksJsonValue(child, targetDir);
+      if (pruned.changed) changed = true;
+      if (!isStructurallyEmpty(pruned.value)) next[key] = pruned.value;
+      else changed = true;
+    }
+    return { value: next, changed };
+  }
+
+  return { value, changed: false };
+}
+
+function isStructurallyEmpty(value) {
+  if (value === null || value === undefined) return true;
+  if (Array.isArray(value)) return value.length === 0;
+  return typeof value === 'object' && Object.keys(value).length === 0;
+}
+
+function cleanupLegacyCodexHooksJson(targetDir) {
+  const hooksPath = path.join(targetDir, 'hooks.json');
+  if (!fs.existsSync(hooksPath)) return { changed: false, removedFile: false };
+
+  let parsed;
+  try {
+    parsed = JSON.parse(fs.readFileSync(hooksPath, 'utf8'));
+  } catch {
+    return { changed: false, removedFile: false, skipped: 'invalid_json' };
+  }
+
+  const pruned = pruneGsdManagedHooksJsonValue(parsed, targetDir);
+  if (!pruned.changed) return { changed: false, removedFile: false };
+
+  if (isStructurallyEmpty(pruned.value)) {
+    fs.unlinkSync(hooksPath);
+    return { changed: true, removedFile: true };
+  }
+
+  atomicWriteFileSync(hooksPath, JSON.stringify(pruned.value, null, 2) + '\n', 'utf8');
+  return { changed: true, removedFile: false };
+}
+
 /**
  * Build a hook command path using forward slashes for cross-platform compatibility.
  * On Windows, $HOME is not expanded by cmd.exe/PowerShell, so we use the actual path.
@@ -8586,6 +8656,10 @@ function install(isGlobal, runtime = 'claude') {
         throw wrapped;
       }
       console.log(`  ${green}✓${reset} Configured Codex hooks (SessionStart)`);
+      const legacyHooksCleanup = cleanupLegacyCodexHooksJson(targetDir);
+      if (legacyHooksCleanup.changed) {
+        console.log(`  ${green}✓${reset} Removed legacy GSD hooks.json entries`);
+      }
     } catch (e) {
       // #2760 — schema-validation and write failures must be loud and fatal
       // so the user is never left with a config Codex refuses to load (or no
@@ -10613,6 +10687,7 @@ if (process.env.GSD_TEST_MODE) {
     rewriteLegacyManagedNodeHookCommands,
     buildCodexHookBlock,
     rewriteLegacyCodexHookBlock,
+    cleanupLegacyCodexHooksJson,
   };
 } else {
 

tests/bug-3357-codex-legacy-hooks-json-migration.test.cjs

@@ -0,0 +1,107 @@
+/**
+ * Regression test for bug #3357.
+ *
+ * Older Codex installs used hooks.json for SessionStart hooks. Current Codex
+ * installs write config.toml hooks. Reinstalling must remove only GSD-managed
+ * legacy hooks.json entries so users do not end up with duplicate GSD hooks.
+ */
+
+'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 { install, parseTomlToObject } = require('../bin/install.js');
+const { createTempDir, cleanup } = require('./helpers.cjs');
+
+function withCodexHome(codexHome, fn) {
+  const previousCodexHome = process.env.CODEX_HOME;
+  process.env.CODEX_HOME = codexHome;
+  try {
+    return fn();
+  } finally {
+    if (previousCodexHome == null) delete process.env.CODEX_HOME;
+    else process.env.CODEX_HOME = previousCodexHome;
+  }
+}
+
+function legacyGsdHook(codexHome) {
+  return {
+    hooks: [{
+      type: 'command',
+      command: `node "${path.join(codexHome, 'hooks', 'gsd-check-update.js')}"`,
+    }],
+  };
+}
+
+function userHook() {
+  return {
+    hooks: [{
+      type: 'command',
+      command: 'node "/Users/example/bin/user-hook.js"',
+    }],
+  };
+}
+
+function tomlGsdHookCount(codexHome) {
+  const parsed = parseTomlToObject(fs.readFileSync(path.join(codexHome, 'config.toml'), 'utf8'));
+  const sessionStart = parsed.hooks?.SessionStart ?? [];
+  return sessionStart
+    .flatMap((entry) => Array.isArray(entry.hooks) ? entry.hooks : [])
+    .filter((hook) => typeof hook.command === 'string' && hook.command.includes('gsd-check-update.js'))
+    .length;
+}
+
+describe('#3357 — Codex install removes legacy GSD hooks.json entries', { concurrency: false }, () => {
+  let tmpRoot;
+  let codexHome;
+
+  beforeEach(() => {
+    tmpRoot = createTempDir('gsd-3357-');
+    codexHome = path.join(tmpRoot, '.codex');
+    fs.mkdirSync(codexHome, { recursive: true });
+  });
+
+  afterEach(() => {
+    cleanup(tmpRoot);
+  });
+
+  test('removes hooks.json when it only contained the legacy GSD SessionStart hook', () => {
+    fs.writeFileSync(
+      path.join(codexHome, 'hooks.json'),
+      JSON.stringify({ SessionStart: [legacyGsdHook(codexHome)] }, null, 2),
+    );
+
+    withCodexHome(codexHome, () => install(true, 'codex'));
+
+    assert.equal(fs.existsSync(path.join(codexHome, 'hooks.json')), false);
+    assert.equal(tomlGsdHookCount(codexHome), 1);
+  });
+
+  test('preserves user hooks.json entries while removing the legacy GSD hook', () => {
+    const userOwnedSameBasenameHook = {
+      hooks: [{
+        type: 'command',
+        command: 'node "/Users/example/bin/gsd-check-update.js"',
+      }],
+    };
+    fs.writeFileSync(
+      path.join(codexHome, 'hooks.json'),
+      JSON.stringify({ SessionStart: [legacyGsdHook(codexHome), userHook(), userOwnedSameBasenameHook] }, null, 2),
+    );
+
+    withCodexHome(codexHome, () => install(true, 'codex'));
+
+    const hooksJson = JSON.parse(fs.readFileSync(path.join(codexHome, 'hooks.json'), 'utf8'));
+    const commands = hooksJson.SessionStart.flatMap((entry) => entry.hooks).map((hook) => hook.command);
+    assert.deepEqual(commands, [
+      'node "/Users/example/bin/user-hook.js"',
+      'node "/Users/example/bin/gsd-check-update.js"',
+    ]);
+    assert.equal(tomlGsdHookCount(codexHome), 1);
+  });
+});