Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions .claude/handoffs/20260409-auto-memory-interop-merged.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,111 @@
# Handoff: Auto-memory interop (assist/takeover modes)

**Date:** 2026-04-09
**Branch:** worktree-auto-memory-interop (merged to main)
**Working directory:** /home/jim/workspace/github.com/jim80net/memex-claude/.claude/worktrees/auto-memory-interop

## Objective

Implement two operating modes (`assist` and `takeover`) so memex-claude coexists cleanly with Claude Code's built-in auto-memory feature, preventing double-injection of memory content.

## Session Summary

Responded to PR #45 review feedback. Two review comments from the claude[bot] reviewer were addressed:

1. **Path centralization review** β€” `getAutoMemoryWatermarkPath()` was defined inline in `session-start.ts`, violating the CLAUDE.md convention that all Claude-specific paths must be centralized in `src/core/paths.ts`. Fixed by moving it to the `ClaudePaths` type alongside `cronWatermarkPath`.

2. **Concurrency safety review** β€” Both `writeAutoMemoryWatermark()` and `writeCronWatermark()` wrote to shared state (`~/.claude/cache/`) without advisory file locks, violating the CLAUDE.md convention. Fixed by wrapping both in `withFileLock()` consistent with the registry write pattern.

The PR was already merged before these review fixes were pushed, so the two fix commits (258e918, 9dc4fe6) landed on the branch after merge.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Incorrect chronology β€” these commits landed before the merge, not after.

Both 258e918 and 9dc4fe6 were committed ~2.5 hours before PR #45 was merged and were included in the merge:

The same incorrect statement is repeated in the Gotchas section (line 105). This inverted timeline could mislead a future session into thinking the review fixes were not included in the merged PR β€” when in fact they were already merged.

See:

The PR was already merged before these review fixes were pushed, so the two fix commits (258e918, 9dc4fe6) landed on the branch after merge.


## Completed Work

### PR #45 β€” Auto-memory interop with assist/takeover modes

**PR:** #45 β€” https://github.com/jim80net/memex-claude/pull/45
**Status:** MERGED (merged at 2026-04-09T10:27:55Z)
**Problem:** Memex and Claude Code auto-memory both inject memory content at session start, causing duplicate context delivery.
**Fix:** Added `autoMemoryMode` config key (default: `assist`) that controls interop behavior:
- `assist` mode: memex filters `memory`/`session-learning` types from UserPromptSubmit β€” auto-memory is authoritative
- `takeover` mode: memex injects a memory-creation rule at session start and warns (once, via watermark) if auto-memory is still enabled
**Files changed:**
- `src/core/config.ts` β€” `AutoMemoryMode` type, config field, merge logic, `isAutoMemoryEnabled()` utility
- `src/core/paths.ts` β€” added `autoMemoryWatermarkPath` to `ClaudePaths` type (review fix)
- `src/hooks/user-prompt.ts` β€” filters memory types in assist mode before `index.search()`
- `src/hooks/session-start.ts` β€” takeover warning + memory-creation rule injection, combined with sleep schedule; `getPluginRoot()` fix for Bun-compiled binaries; both watermark writes wrapped in `withFileLock` (review fix)
- `skills/memory-creation/SKILL.md` β€” new bundled skill
- `test/user-prompt.test.ts` β€” 3 new tests for assist-mode filtering
- `test/session-start.test.ts` β€” 5 new tests for warning, rule injection, watermarking, combined output
- `CLAUDE.md` β€” documents `autoMemoryMode`
**Tests:** 37/37 pass
**Review feedback addressed:**
1. Centralized `autoMemoryWatermarkPath` in `ClaudePaths` type (commit 258e918)
2. Wrapped both watermark writes in `withFileLock` for concurrency safety (commit 9dc4fe6)

### Key Decisions

| Decision | Rationale | Alternatives Rejected |
|----------|-----------|----------------------|
| Default to `assist` mode | Zero-disruption for existing installs; immediately stops double-injection | Default to `takeover` would require all users to set env var |
| Watermark-based one-time warning | Simple file existence check; no state management needed | Session-registry approach (overkill for a one-time nudge) |
| Combine takeover output with sleep schedule | Avoids multiple hook responses; both are session-start concerns | Separate responses would work but create unnecessary noise |
| Centralize `autoMemoryWatermarkPath` in paths.ts | Follows existing `cronWatermarkPath` pattern; CLAUDE.md convention | Keep as inline function (violates convention) |
| withFileLock for watermark writes | Follows CLAUDE.md convention; prevents race conditions from concurrent sessions | No lock (pre-existing gap; review caught it) |

## Current State

### Git
```
% git log --oneline -5
9dc4fe6 fix: wrap watermark writes in withFileLock for concurrency safety
258e918 fix: centralize autoMemoryWatermarkPath in paths.ts per review feedback
bbe9b22 fix: handle compiled binary path in getPluginRoot
a1caac7 fix: combine takeover + sleep schedule sections instead of early return
1194005 docs: document autoMemoryMode config key
Comment on lines +60 to +64
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Git log snapshot is stale β€” missing the actual PR #45 head commit.

The log shows 9dc4fe6 as HEAD, but the real headRefOid of PR #45 is 01019cd ("Update src/hooks/session-start.ts"), which sits on top of 9dc4fe6. This commit moved mkdir(dirname(watermarkPath), { recursive: true }) from inside the withFileLock() callback to before it in writeCronWatermark() β€” a meaningful ordering fix, since acquiring the lock requires the directory to already exist.

The Review feedback addressed section (lines 42-43) is also incomplete for the same reason β€” it lists only 258e918 and 9dc4fe6 but omits 01019cd.

See:

```
% git log --oneline -5
9dc4fe6 fix: wrap watermark writes in withFileLock for concurrency safety
258e918 fix: centralize autoMemoryWatermarkPath in paths.ts per review feedback
bbe9b22 fix: handle compiled binary path in getPluginRoot
a1caac7 fix: combine takeover + sleep schedule sections instead of early return
1194005 docs: document autoMemoryMode config key
% git status


% git status
On branch worktree-auto-memory-interop
nothing to commit, working tree clean

% git branch --show-current
worktree-auto-memory-interop
```

### Open PRs
None β€” #45 is merged.

## Remaining Work

### 1. Manual testing [medium priority]

**What:** The PR's test plan includes 4 manual test scenarios that were not executed in this session:
- Verify assist mode suppresses memory injection (check stderr for `memex: injected N skills` without memory counts)
- Verify takeover mode shows warning when auto-memory is still enabled
- Verify takeover mode injects only memory-creation rule when `CLAUDE_CODE_DISABLE_AUTO_MEMORY=1`
- Verify `getPluginRoot()` fix works in compiled binary (`bun run build.ts` then invoke binary)

**Where:** Requires running the actual binary with config changes
**Approach:** Build binary, set `autoMemoryMode` in `~/.claude/memex.json`, run hooks manually
**Verify:** Each scenario produces expected output

### 2. Runtime verification of withFileLock [low priority]

**What:** Verify that concurrent session starts don't produce duplicate watermark writes
**Where:** `~/.claude/cache/memex-automemory-warned` and `~/.claude/cache/memex-cron-watermark`
**Approach:** This is a theoretical race condition; the fix matches the existing registry write pattern. Low risk.

## Failed Approaches & Dead Ends

None β€” both review fixes were straightforward pattern-matching to existing conventions.

## Gotchas & Environment Notes

- `rtk` tool wraps some commands (git, gh) but doesn't understand `pnpm` β€” use `npx` directly for vitest/tsc
- The project uses `bun build --compile` for production binaries; `import.meta.url` resolves to a virtual `$bunfs` path in compiled mode, which is why `getPluginRoot()` needed fixing
- PR was merged before review feedback was pushed β€” the two fix commits are on the feature branch but after the merge point

## To Resume

1. Read this file: `cat .claude/handoffs/20260409-auto-memory-interop-merged.md`
2. `cd /home/jim/workspace/github.com/jim80net/memex-claude && gh pr list --state open` to check for new issues
3. Manual testing of the 4 scenarios listed in Remaining Work if desired
Loading