Skip to content

fix(hooks): never wipe agent settings when installing notification hooks - #675

Merged
Zeus-Deus merged 3 commits into
mainfrom
fix/hook-config-safe-writes
Nov 13, 2026
Merged

Zeus-Deus merged 3 commits into
mainfrom
fix/hook-config-safe-writes

Conversation

@Zeus-Deus

Copy link
Copy Markdown
Owner

Problem

At every start, Codemux merges its notification hooks into Claude Code's ~/.claude/settings.json, Codex's ~/.codex/hooks.json and Gemini's ~/.gemini/settings.json.

  • Data loss: if a file couldn't be read or parsed, the merge treated it as {} and wrote it back non-atomically. A settings file with a syntax error, or one caught mid-write, lost every other setting.
  • Crash: a valid file with "hooks": [] crashed the merge.
  • OpenCode errors: the generated OpenCode plugins were rewritten at every start. OpenCode 2.x rejects the older singular plugin/codemux-notify.js file and logs a loader error each time. That has happened about 200 times on one machine.

Fix

  • Fail closed: an unreadable or invalid file, an unexpected root, or a malformed event Codemux owns now skips the write and logs it. Events Codemux doesn't touch are left alone.

  • Atomic writes: writes use a unique sibling temp file and an atomic replace, keep the existing file mode, follow an existing symlink to its target once, and skip read-only files.

  • No-op starts: identical content is never rewritten, so the bytes and mtime stay the same.

  • Hook matching: Codemux's own generated commands are matched exactly before the old heuristic. Hooks are no longer duplicated when the home path contains &, ;, |, <, > or spaces, and existing duplicates collapse to one entry. User hook entries, including mixed ones, are kept. Entries whose only owned hooks were removed are dropped only when they hold nothing else.

  • Notifier script and plugins: notify.sh and the plugin files are written only when they changed, through temp file and rename. A hook firing mid-write can no longer read a truncated script.

  • OpenCode: plugins/codemux/tui.js is always installed. The detected version decides only the legacy singular file:

    • 1.x keeps it;
    • 2.x retires it only if it is byte-for-byte the file Codemux generated;
    • an unknown or future version leaves it alone.

    The install runs in the background, so a slow or missing opencode never delays startup or terminal restore.

Hooks stay global. notify.sh already exits immediately outside Codemux terminals.

Verification

  • cargo check.
  • cargo test --lib hooks::tests: 31 passed, 1 ignored. The tests cover invalid JSON left untouched, hooks: [], mixed user entries, home paths with metacharacters, no-op byte and mtime identity, mode retention, read-only skip, symlink targets, and idempotent plugin writes.
  • hooks::tests::opencode_loader_smoke_isolated_home against real OpenCode 2.2.5 in an isolated HOME. It reproduces the singular-file loader error and confirms the plural plugin loads cleanly.
  • OpenCode 1.19.0 probe in an isolated HOME: the nested plural file is not discovered, so 1.x behaviour is unchanged.

Known limits: atomic replacement breaks an existing hard link to the settings file. Leftover v0.23/v0.24 plugin files are not retired, because they don't match the exact generated template.

Implemented with Codex (GPT-6.1 Sol) and reviewed with Claude Code (Opus 5.5).

🤖 Generated with Claude Code

@mario-reviewer mario-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of 65b8820

No blocking findings. Reviewed the full diff and exact-head context for settings validation, mixed-hook preservation, atomic file replacement, symlink and permission handling, and OpenCode plugin migration. No prior review findings were present.

Validation: static review only. No repository code was built, installed, or executed. The added regression tests were inspected, not run.

CI: changes and ci-gate failed; web, Rust, and WASM jobs were skipped. The changes annotation says the job could not start because the account is locked due to a billing issue. This is an infrastructure blocker, not a demonstrated code defect.

Verdict: APPROVE

🤖 Automated review by Mario (AI). Findings may be wrong; a human makes the final call.

@mario-reviewer mario-reviewer left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-review of 1a07eff

No blocking findings. Reviewed the new commit since 65b8820 against exact-head repository context. The Windows replacement retry preserves the staged file, bounds retries, and rechecks the original bytes before each attempt. The added tests cover readers temporarily blocking replacement, timeout preservation and cleanup, and intervening edits. The concurrent-reader test now stops and joins its reader before reporting a failed write.

The previous review had no unresolved findings. The whole-PR verdict remains unchanged.

Validation: static review only. No repository code was built, installed, or executed. Tests were inspected, not run.

CI: changes passed; Ubuntu and Windows web/Rust checks are in progress; iroh-wasm was skipped. Pending CI is not a code defect.

Verdict: APPROVE

🤖 Automated review by Mario (AI). Findings may be wrong; a human makes the final call.

@Zeus-Deus
Zeus-Deus merged commit 9127773 into main Nov 13, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants