Skip to content

fix: point Setup hook to smart-install.js instead of deleted setup.sh (#1268)#1358

Open
ousamabenyounes wants to merge 2 commits intothedotmack:mainfrom
ousamabenyounes:fix/setup-hook-1268
Open

fix: point Setup hook to smart-install.js instead of deleted setup.sh (#1268)#1358
ousamabenyounes wants to merge 2 commits intothedotmack:mainfrom
ousamabenyounes:fix/setup-hook-1268

Conversation

@ousamabenyounes
Copy link
Contributor

Summary

  • The Setup hook in hooks.json referenced scripts/setup.sh which was deleted in commit 74d94aa
  • After a Claude Code update, the Setup hook failed silently, causing claude-mem to appear disconnected
  • Fix: point the Setup hook to smart-install.js which already handles all setup tasks (Bun/uv installation, dependency checks, CLI install)

Fixes #1268

Test plan

  • 3 new tests: Setup hook references existing script, doesn't reference setup.sh, references smart-install.js
  • All 1119 tests pass (0 failures)
  • Verified plugin/scripts/smart-install.js exists and handles all setup tasks previously in setup.sh

🤖 Coded by Claude, vibe-coded by Ousama Ben Younes

🤖 Generated with Claude Code

…thedotmack#1268)

The Setup hook referenced scripts/setup.sh which was deleted in commit
74d94aa but the hooks.json reference was never updated. This caused
the Setup hook to fail silently on Claude Code updates, making claude-mem
appear disconnected. smart-install.js already handles all the same setup
tasks (Bun/uv installation, dependency checks, CLI install).

Adds 3 tests verifying the Setup hook references an existing script.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@xkonjin
Copy link

xkonjin commented Mar 13, 2026

Code Review

Overview

This PR fixes a setup hook issue by pointing the Setup hook to smart-install.js instead of the deleted setup.sh script. The fix includes tests to verify the hook references the correct script.

Strengths

  1. Targeted Fix: Addresses the specific issue (Claude code update = no claude-mem #1268) with minimal changes

  2. Comprehensive Tests: Added three test cases that verify:

    • The referenced script exists
    • The deleted setup.sh is not referenced
    • smart-install.js handles all setup tasks
  3. Good Test Structure: Tests are clear, well-named, and follow the existing test patterns

Issues & Concerns

None identified. This is a clean, well-tested fix.

Security Concerns

None identified.

Potential Bugs

None identified.

Test Coverage

Good test coverage for the fix:

  • Tests verify the hook references an existing script
  • Tests ensure the deleted script is not referenced
  • Tests confirm smart-install.js is used

Recommendations

Consider adding a comment in hooks.json explaining why smart-install.js is used instead of setup.sh, for future maintainers who might not be aware of the issue history.

Summary

This is a straightforward, well-tested fix that addresses a setup hook issue. The tests provide good coverage and the change is minimal and targeted.

Overall Assessment: Approve

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ousamabenyounes
Copy link
Contributor Author

Thanks for the review! Added the recommended comment in commit 5d15c62:

"_comment": "Uses smart-install.js (not setup.sh which was deleted in 74d94aa2). See #1268."

All 1119 tests pass, 0 failures.

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.

Claude code update = no claude-mem

2 participants