Skip to content

Clean up Claude Code hooks on disconnect#783

Open
hq8sb2w7xq-cell wants to merge 2 commits into
moorcheh-ai:mainfrom
hq8sb2w7xq-cell:codex/memanto-remove-cleanup
Open

Clean up Claude Code hooks on disconnect#783
hq8sb2w7xq-cell wants to merge 2 commits into
moorcheh-ai:mainfrom
hq8sb2w7xq-cell:codex/memanto-remove-cleanup

Conversation

@hq8sb2w7xq-cell

@hq8sb2w7xq-cell hq8sb2w7xq-cell commented Jun 24, 2026

Copy link
Copy Markdown

Related to #770

Flaw summary

memanto connect claude-code installs a Memanto-managed Claude Code SessionStart hook and the Bash(memanto:*) permission, but memanto disconnect / memanto remove only deleted Memanto metadata. The Claude configuration remained modified after disconnect, so future Claude Code sessions could still invoke Memanto hooks or keep stale Memanto permissions even after the user explicitly removed the integration.

This is a setup and UX roadblock for adoption because disconnecting an agent should leave the Claude Code config in the same state except for unrelated user-managed hooks and permissions.

Reproduction

  1. Use a temporary home directory with a Claude Code settings file.
  2. Run the connect flow for claude-code so Memanto adds its SessionStart hook and Bash(memanto:*) permission.
  3. Run memanto disconnect <agent-id> or the remove flow.
  4. Inspect .claude/settings.json.

Before this change, the Memanto hook and permission are still present even though the agent has been removed from Memanto metadata.

Fix

  • Remove only Memanto-managed SessionStart hook entries when removing a Claude Code agent.
  • Remove only the Memanto-managed Bash(memanto:*) permission.
  • Preserve unrelated user hooks, permissions, and other Claude settings.
  • Delete an empty Claude settings file after cleanup so a pure Memanto install leaves no stale JSON behind.

Validation

.venv/bin/python -m pytest tests/test_connect_engine.py -q
.venv/bin/python -m ruff check memanto/cli/connect/engine.py tests/test_connect_engine.py
.venv/bin/python -m mypy memanto/cli/connect/engine.py --ignore-missing-imports
git diff --check

Summary by CodeRabbit

  • Bug Fixes
    • Removing an agent now also cleans up related Claude Code hook settings and permissions.
    • Unrelated hooks and permissions are preserved; settings files are removed when they become empty.
  • Tests
    • Added coverage to verify hook and permission cleanup for Claude Code, including preservation of unrelated entries in both global and local settings.

@coderabbitai

coderabbitai Bot commented Jun 24, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 606e4fd4-1eab-4b6c-b5ae-dd5f016ec18d

📥 Commits

Reviewing files that changed from the base of the PR and between bb557b1 and 2ab2cd4.

📒 Files selected for processing (2)
  • memanto/cli/connect/engine.py
  • tests/test_connect_engine.py
🚧 Files skipped from review as they are similar to previous changes (2)
  • tests/test_connect_engine.py
  • memanto/cli/connect/engine.py

📝 Walkthrough

Walkthrough

remove_agent now also removes MEMANTO hook and permission entries from Claude Code settings, deletes empty JSON files or directories, and the tests cover both removal and preservation of unrelated config data.

Changes

Claude Code agent cleanup

Layer / File(s) Summary
Removal orchestration
memanto/cli/connect/engine.py, tests/test_connect_engine.py
remove_agent now conditionally removes hook and permission config, records step and error messages, and the temp-filesystem test checks the installed .claude files are removed.
Hook settings cleanup
memanto/cli/connect/engine.py
_remove_hooks filters MEMANTO SessionStart hook entries, prunes empty structures, and persists or deletes the hook settings file through _write_or_remove_json.
Permission cleanup
memanto/cli/connect/engine.py, tests/test_connect_engine.py
_remove_permissions removes matching permissions.allow entries, prunes empty permissions data, and the preservation test checks unrelated hook entries and remaining permission/env data stay intact.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A bunny hops through settings bright,
Nibbling hooks and permissions right. 🐇
Empty files go poof in the moonlit air,
While old stray bits stay safe in there.
Hop, hop—clean paths now shine!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.85% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: cleaning up Claude Code configuration when disconnecting/removing an agent.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@memanto/cli/connect/engine.py`:
- Around line 396-416: The hook removal logic in the session-start cleanup is
matching only on command, which can delete user-authored hooks that merely share
the same Memanto sync command. Update the filtering in engine.py around the
session_start hook loop so it only removes hooks whose full shape matches the
managed payload from agent.hook_config.hook_payload, using the existing
expected_commands logic as part of a stricter comparison against fields like
command, matcher, type, and timeout.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 839931f7-7140-48f6-9e25-ea9e44f5be1e

📥 Commits

Reviewing files that changed from the base of the PR and between 3c604d4 and bb557b1.

📒 Files selected for processing (2)
  • memanto/cli/connect/engine.py
  • tests/test_connect_engine.py

Comment thread memanto/cli/connect/engine.py
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