Skip to content

fix(hooks): repair a stale SessionEnd hook instead of only installing once - #7

Merged
lupuletic merged 1 commit into
mainfrom
fix/stale-hook-migration
Jul 25, 2026
Merged

fix(hooks): repair a stale SessionEnd hook instead of only installing once#7
lupuletic merged 1 commit into
mainfrom
fix/stale-hook-migration

Conversation

@lupuletic

Copy link
Copy Markdown
Owner

Problem

The SessionEnd hook command embeds an absolute path to the binary, so renaming the project (claude-recallcode-recall) left existing installs pointing at a command that no longer exists. The hook then failed silently on every session end:

ModuleNotFoundError: No module named 'claude_recall.cli'

The migration for exactly this case was already present — LEGACY_COMMAND_NAMES and _hook_mentions_app — but it was unreachable:

is_first_run = not args.db.exists()
if is_first_run and not HOOKS_MARKER.exists():   # false for anyone with an index
    _auto_install_hooks()

Anyone past their first run could never reach the rewrite branch, so a broken hook stayed broken.

Changes

  • Split repair from install. An existing hook is refreshed whenever it differs from the desired config; a missing hook is only added on first run, so one the user deleted is not resurrected. Writes happen only on an actual change, so ordinary runs leave settings.json untouched.
  • Stop destroying settings.json when it cannot be parsed. The read error was swallowed with pass and the resulting empty dict written straight back, replacing every unrelated key with just hooks. Verified against the pre-fix code: a truncated settings file was left with only ['hooks'], losing model and permissions.
  • Atomic writes via temp file + rename, so an interrupted write cannot truncate the file.

Notes

_cmd_index does not call _first_run_setup, so the hook's own index --quiet never rewrites settings — repair rides on interactive commands only. It has to: a hook pointing at a missing binary never executes and so cannot repair itself.

Testing

11 new tests (339 total, all passing), plus compileall and check_version.py. Confirmed as genuine regression tests by running the same scenario against main: old code leaves the stale command in place, new code repairs it while preserving unrelated keys.

🤖 Generated with Claude Code

… once

The hook command embeds an absolute path to the binary, so renaming the
project (claude-recall -> code-recall) left existing installs pointing at a
command that no longer exists. The migration for exactly this case was
already here — LEGACY_COMMAND_NAMES and _hook_mentions_app — but it sat
behind `is_first_run`, which is false for anyone who has ever built an
index. The rewrite branch was unreachable, so a broken hook stayed broken
and failed silently on every session end.

Split the two concerns: refresh an existing hook whenever it differs from
the desired config, and only append a new one on first run, so a hook the
user deleted is not resurrected. Writes happen only on an actual change,
so ordinary runs still leave settings.json alone.

Also stop destroying settings.json when it cannot be parsed. The read error
was swallowed with `pass` and the resulting empty dict written straight
back, replacing every unrelated key with just `hooks`. Bail out instead,
and write through a temp file and atomic rename so an interrupted write
cannot truncate the file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lupuletic
lupuletic merged commit 59b1aad into main Jul 25, 2026
10 checks passed
@lupuletic
lupuletic deleted the fix/stale-hook-migration branch July 25, 2026 16:19

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2fd9b75d32

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/code_recall/cli.py
# Auto-install hooks on first run
if is_first_run and not HOOKS_MARKER.exists():
_auto_install_hooks()
# Repair a stale hook on every run, but only add a missing one on first run.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Bump the package version for this hook repair

This changes installed CLI behavior by repairing hooks during ordinary invocations, but pyproject.toml, src/code_recall/__init__.py, and uv.lock remain at version 0.2.5. Bump all three version records so installed copies and update checks can distinguish this bug fix.

AGENTS.md reference: AGENTS.md:L3-L11

Useful? React with 👍 / 👎.

Comment thread src/code_recall/cli.py
Comment on lines +234 to +236
with open(tmp_path, "w") as f:
json.dump(settings, f, indent=2)
os.replace(tmp_path, settings_path)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve settings.json permissions during atomic replacement

When an existing settings.json has restrictive permissions, writing a newly created .tmp file and replacing the original discards its mode; under a common 022 umask, a 0600 settings file becomes 0644. Because Claude settings can contain private configuration, preserve the original mode on the temporary file before os.replace.

Useful? React with 👍 / 👎.

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.

1 participant