Skip to content

Fix the sweep's own prune, sqlite-vec loading, and the SessionStart hook exit code - #18

Open
codenamev wants to merge 5 commits into
mainfrom
vs-sweep-and-vec-fixes
Open

codenamev wants to merge 5 commits into
mainfrom
vs-sweep-and-vec-fixes

Conversation

@codenamev

Copy link
Copy Markdown
Owner

Three independent fixes, each with the spec that catches it.

The sweep could never prune. ingestion_metrics.content_item_id was created in v7 as a plain foreign_key ..., null: false with no ON DELETE action, unlike the sibling tool_calls which cascades. Sweep::Maintenance#prune_old_content deletes aged content items carrying no provenance, and any such item that still had a metrics row raised ForeignKeyConstraintViolation. The raise escaped the sweeper, so a single aged row aborted the whole sweep and no pruning ever ran. Migration v22 rebuilds the table with ON DELETE CASCADE.

sqlite-vec reported itself unavailable. extralite 2.15 has no Database#load_extension, so SqliteVec.load(conn) raises NoMethodError; VectorIndex#load_extension! rescues StandardError and returns false, which every caller renders as "sqlite-vec not available". The gemspec allowed ~> 2.14, which admits 2.15, while Gemfile.lock pins 2.14. The suite and bundle exec therefore never saw it, and the failure only reaches gems installed from the gemspec. 2.15 is the only affected release; 2.14 and 3.0.1 both have the method. The constraint is now >= 2.14, != 2.15, < 3.0. 3.x is held back separately: it requires Ruby >= 3.4 where this gem allows >= 3.2, and per extralite's 3.0.0 changelog it turns foreign keys on by default. VectorIndex#load_error now carries the probe's failure reason, so an unusable extension stops looking like an absent one.

SessionStart reported a spurious hook error on every fresh session. The ingest hook runs before the new session's transcript exists, and transcript_not_found fell into the catch-all skip branch that exits WARNING. Claude Code only surfaces stderr on a non-zero exit and the skip message went to stdout, so sessions opened with "SessionStart:startup hook error ... No stderr output". A missing transcript at SessionStart is the expected state and now exits SUCCESS; remaining catch-all skip reasons write to stderr.

SessionStart runs the ingest hook before the new session has written a
transcript, so transcript_not_found fell into the catch-all skip branch
that exits WARNING (1). Claude Code only surfaces stderr on a non-zero
exit and the skip message went to stdout, so every session opened with
"SessionStart:startup hook error ... No stderr output".

A missing transcript at SessionStart is the expected state. It now exits
SUCCESS, and the remaining catch-all skip reasons write to stderr so a
future non-zero exit carries a real message.

cli_spec covers the same path as hook_command_spec and still asserted
WARNING, so it is updated alongside.
ingestion_metrics.content_item_id was created in v7 as a plain
foreign_key ..., null: false with no ON DELETE action, unlike the
sibling tool_calls which cascades.

Sweep::Maintenance#prune_old_content deletes aged content items that
carry no provenance. Any such item that still had a metrics row raised
ForeignKeyConstraintViolation, and the raise escaped the sweeper, so one
aged row aborted the whole sweep and no pruning ever ran. Databases grew
without bound: observed on a real workspace as a 507 MiB project DB
holding 39 facts, with 340 content items blocking the prune.

Migration v22 rebuilds the table with ON DELETE CASCADE, dropping rows
whose content item is already gone so the copy can enforce the
reference. The new spec reproduces the exact production failure when the
migration is removed.
extralite 2.15 has no Database#load_extension, so SqliteVec.load(conn)
raises NoMethodError. VectorIndex#load_extension! rescues StandardError
and returns false, which every caller renders as "sqlite-vec not
available" - indistinguishable from the extension simply not being
installed.

The gemspec allowed ~> 2.14, which admits 2.15, while Gemfile.lock pins
2.14. The suite and bundle exec therefore never saw it and only
installed gems lost vector recall. Every database on a real machine had
an empty vec0 index as a result.

2.15 is the only affected release: 2.14 has the method and so does
3.0.1, and extralite-bundle 2.15 has it too. So exclude that one
version rather than everything above it. 3.x stays out for its own
reasons - it requires Ruby >= 3.4 where this gem allows >= 3.2, and
3.0.0 turns foreign keys on by default.

Also keep the probe failure reason in VectorIndex#load_error so index
reports why rather than asserting an absence it did not check.
Configuration#project_dir read CLAUDE_PROJECT_DIR and returned it
directly, so resolve_project_dir never ran when that variable was set.
Claude Code sets it on every hook and MCP subprocess, pointing at the
directory the session started in, which means git_main_repo_root -
added in 20c2731 specifically to prevent "duplicate project databases
when using git worktrees" - was unreachable in the environment it was
written for. CLAUDE_MEMORY_ISOLATE_WORKTREES lives behind the same
branch, so the documented opt-in to per-worktree isolation did nothing
either, while its effect was the permanent default.

Compose the two instead of letting one shadow the other. The variable
says which directory this session is in; git says which repo that
directory belongs to. A CLAUDE_PROJECT_DIR naming a worktree, or a
subdirectory of a repo, now resolves to the repo root. One outside any
repo is returned unchanged. CLAUDE_MEMORY_ISOLATE_WORKTREES short
circuits before git and is now actually reachable.

git_main_repo_root and git_command take the directory to run in, so
neither depends on the process cwd, and git_command uses the array form
so a path with spaces is one argument rather than shell input.

The interaction was untested in both directions: every worktree spec
passed an empty env, and the one CLAUDE_PROJECT_DIR spec used a path
with no git involved. Added specs for both, including a real git
worktree on disk rather than stubs - the four that cover this fail
against the old precedence.

Measured on one workspace: 58 of 70 redundant databases, 466 MiB and
704 facts, were worktrees duplicating their main repo.

git_command also clears GIT_DIR, GIT_INDEX_FILE and the rest of git's
per-invocation variables, which git exports to its own hooks and which
outrank -C. Without that, a claude-memory command run from inside a git
hook resolves every project to whatever repository invoked the hook.
Note a Hash given to Process.spawn is merged into the caller's
environment, so only an explicit nil unsets a variable; a filtered copy
of ENV removes nothing.
Resolve CLAUDE_PROJECT_DIR through the git root so worktrees share one database
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