[CreedTech]

Summary

Preserve existing Claude hookify guards when setup runs again. New guards are still created, and setup now reports each existing guard as skipped so hand edits are not silently replaced.

The guard writes use exclusive creation (wx), which also prevents an existing file from being replaced between a check and the write. The setup README describes the rerun behavior.

Related Issue

Closes #481

Validation

• Tests added or updated: the regression failed before the fix, then passed with this change. The setup package suite passed 130 tests with the live socket case excluded.
• Documentation updated for the behavior change.
• Security impact considered: existing approval guards remain intact, and other write errors still fail setup.
• Trust boundary preserved: this changes only setup file creation, not the executor.
• CI passes: awaiting GitHub CI.

The full package run had one failure: a reachable externally managed daemon makes a skipped install complete. That same test fails on untouched main in this macOS environment because the socket probe reports the local test socket as unreachable. python3 scripts/check_evidence_claims.py and git diff --check passed. No Rust files changed; per CONTRIBUTING.md, I used the setup package suite and evidence check for this JavaScript-only diff.

Notes for Reviewers

The change was prepared with Codex assistance. The complete diff and reported test results were checked before opening this PR. No packaged defaults are applied to existing hookify files; users can update those manually.

[vladimirrott]

Reviewed at 952a46b9. Thanks for this. Choosing { flag: 'wx' } over an existsSync check is the better fix: the kernel answers "does it exist" and "create it" in one call, so nothing can slip in between. It also closes a gap #481 never asked about. The old plain writeFileSync followed a symlink planted at a guard's path; with wx, a dangling symlink there comes back EEXIST and setup leaves it alone:

$ node wx-symlink.mjs
wx on dangling symlink: EEXIST
plain write: target exists = true

I approve it. Nothing is blocking.

The diff runs as npm test, so I ran it in a container (node:24-slim, no network) instead of on my machine. Full suite on main and on this head, then once more with packages/setup/index.js put back to main and your test left in place:

main                       # tests 130  # pass 129  # fail 1
952a46b9                   # tests 131  # pass 130  # fail 1
952a46b9, index.js reverted  # tests 131  # pass 128  # fail 3
  not ok 92 - rerunning setup preserves existing hookify guards and reports what it did

Your new test goes red without the fix, which is what I look for first. The one failure in every run is a reachable externally managed daemon makes a skipped install complete. It fails the same way on main inside my container, so your note about it on macOS holds up and it is not yours to fix. Your count checks out too: 131 tests, 130 passing once that one is set aside.

Optional, take it or leave it. The body says other write errors still fail setup, and the code does that, but no test covers it. I deleted if (error.code !== 'EEXIST') throw error; and the suite stayed at 130 of 131. Making .claude a regular file gives you an ENOTDIR that is easy to provoke. With the rethrow deleted, setup prints Skipped .claude/... (already exists) for a file that was never written and exits 3, so you get a misleading log line and a setup that looks normal. I checked that this probe passes on your head and fails on that mutation:

test('a non-EEXIST hookify write error still fails setup', () => {
  const cwd = fs.mkdtempSync(path.join(os.tmpdir(), 'sysknife-setup-enotdir-'));
  try {
    fs.writeFileSync(path.join(cwd, '.claude'), 'not a directory\n');
    const result = runWizard({ cwd });
    const output = `${result.stdout}${result.stderr}`;
    assert.doesNotMatch(output, /Skipped \.claude/);
    assert.notEqual(result.status, 0);
    assert.notEqual(result.status, 3);
  } finally {
    fs.rmSync(cwd, { recursive: true, force: true });
  }
});

952a46b9               status=1 skipped=false enotdir=true   ok 1
952a46b9, rethrow cut  status=3 skipped=true  enotdir=false  not ok 1

If you add it, I'll re-check the new head. If you'd rather not, I'll merge this as it is.

CI hasn't run yet. GitHub holds workflow runs on a first PR until I approve them, and that part is on me. I've assigned #481 to you so it shows up on your dashboard. Once this lands, ask on the thread for the next one in the setup wizard and I'll point you at it.

If you run Linux boxes yourself, sysknife "list services that are failing" is a read-only first try: it plans a query and changes nothing on the host.

[vladimirrott]

Merged, thank you @CreedTech. { flag: 'wx' } was the right call: it skips an existing guard and closes the check-then-write window in one move, and the test runs the real wizard over two hand-edited guards and a missing third.

Before merging I removed the wx flag and re-ran your test. It failed, so the test guards the fix. The CHANGELOG entry will say reruns keep hand-edited hookify guards and report them as skipped.

For a next one in the same file: #562. sysknife-setup decides whether .mcp.json is gitignored with a substring match, so a comment in .gitignore that merely names the file hides the API-key warning. It's small, it's in packages/setup/index.js, and the setup test harness you just used is the place to pin it. I'll offer it on the issue thread.

If you run Linux machines yourself, I'd like to hear how SysKnife behaves on one. The read-only commands (sysknife doctor, sysknife history, sysknife audit verify) change nothing on the host, and a report from a real box is worth more than anything our tests can produce. No pressure either way.