diff --git a/CHANGELOG.md b/CHANGELOG.md index d2fe77e..1aca646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +### Fixed + +- **A git worktree no longer gets its own project database under Claude Code.** `Configuration#project_dir` read `CLAUDE_PROJECT_DIR` and returned it directly, so `git_main_repo_root` — added in 20c2731 precisely to stop "duplicate project databases when using git worktrees" — never ran in the environment it was written for. Claude Code sets that variable on every hook and MCP subprocess, pointing at the directory the session started in, so the worktree logic was unreachable outside a bare CLI run, and so was `CLAUDE_MEMORY_ISOLATE_WORKTREES`, which lives behind it. The two are now composed rather than one shadowing the other: the variable says which directory the session is in, git says which repo that directory belongs to, and `CLAUDE_MEMORY_ISOLATE_WORKTREES` remains the way to ask for per-worktree databases. A `CLAUDE_PROJECT_DIR` outside any repo is returned unchanged. Measured on one workspace: 58 of 70 redundant databases, 466 MiB and 704 facts, were worktrees duplicating their main repo. Resolution also no longer inherits `GIT_DIR`/`GIT_INDEX_FILE`, which git exports to its own hooks and which outrank `-C`, so a claude-memory command run from inside a git hook no longer resolves every project to whatever repository invoked the hook. +- **SessionStart no longer reports a spurious hook error on fresh sessions.** The plugin's SessionStart `ingest` hook runs before the new session's transcript file exists, and `transcript_not_found` fell into the catch-all skip branch that exits `WARNING` (1) — so every session started with "SessionStart:startup hook error ... No stderr output" (the skip message went to stdout, which Claude Code doesn't surface on failure). A missing transcript at SessionStart is the expected state, not a warning: it now exits `SUCCESS`, and the remaining catch-all skip reasons write to stderr so any future non-zero exit shows a real message. +- **The sweep no longer aborts on its own pruning step, so databases stop growing without bound.** `ingestion_metrics.content_item_id` was created (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` — which escaped the sweeper, so one aged row aborted the entire sweep and nothing was ever pruned. Migration v22 rebuilds the table with `ON DELETE CASCADE`. Observed on a real workspace as a 507 MiB project DB holding 39 facts, where 340 content items were blocking the prune. +- **`sqlite-vec` no longer silently reports itself unavailable on installed gems.** extralite 2.15 has no `Database#load_extension`, so `SqliteVec.load(conn)` raised `NoMethodError`; `VectorIndex#load_extension!` rescued `StandardError` and returned false, which every caller renders as "sqlite-vec not available". The gemspec allowed `~> 2.14` (admitting 2.15) while `Gemfile.lock` pinned 2.14, so the suite and `bundle exec` never saw it and only real installs lost vector recall. 2.15 is the only affected release, since 2.14 and 3.x both have the method, so the dependency is now `>= 2.14, != 2.15, < 3.0` — excluding the one bad version rather than everything above it. 3.x is held back separately: it requires Ruby >= 3.4 where this gem allows >= 3.2, and it turns foreign keys on by default. The probe also keeps its failure reason in `VectorIndex#load_error`, so an unusable extension no longer looks like an absent one. + ## [0.13.2] - 2026-06-27 Theme: **Robustness from a real bug report (issue #7) + a 28MB→606K gem.** Closes [#7](https://github.com/codenamev/claude_memory/issues/7) — three independent failure modes around embeddings, FTS, and process lifecycle — plus two backlog items surfaced alongside it. No schema changes, no breaking changes. diff --git a/Gemfile.lock b/Gemfile.lock index 342ea37..648c2c5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -2,7 +2,7 @@ PATH remote: . specs: claude_memory (0.13.2) - extralite (~> 2.14) + extralite (>= 2.14, < 3.0, != 2.15) sequel (~> 5.102) sqlite-vec (~> 0.1, >= 0.1.9) webrick (~> 1.8) diff --git a/claude_memory.gemspec b/claude_memory.gemspec index 60c31d3..aae2d6c 100644 --- a/claude_memory.gemspec +++ b/claude_memory.gemspec @@ -36,7 +36,13 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] spec.add_dependency "sequel", "~> 5.102" - spec.add_dependency "extralite", "~> 2.14" + # 2.15 alone has no Extralite::Database#load_extension, so SqliteVec.load + # raises NoMethodError and VectorIndex reports "sqlite-vec not available" + # for what is really a broken dependency. 2.14 and 3.x both have it, so + # this excludes the one bad release rather than everything above it. + # Held below 3.0 separately: it requires Ruby >= 3.4 (we allow >= 3.2) + # and turns foreign keys on by default. + spec.add_dependency "extralite", ">= 2.14", "!= 2.15", "< 3.0" spec.add_dependency "sqlite-vec", "~> 0.1", ">= 0.1.9" spec.add_dependency "webrick", "~> 1.8" end diff --git a/db/migrations/022_cascade_ingestion_metrics.rb b/db/migrations/022_cascade_ingestion_metrics.rb new file mode 100644 index 0000000..f8e1541 --- /dev/null +++ b/db/migrations/022_cascade_ingestion_metrics.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +# Migration v22: cascade ingestion_metrics when its content item is deleted. +# +# ingestion_metrics.content_item_id was created (v7) as a plain +# `foreign_key ..., null: false` with no ON DELETE action, unlike its +# sibling tool_calls which cascades. Sweep::Maintenance#prune_old_content +# deletes aged content items that carry no provenance, and any such item +# that still has a metrics row raises ForeignKeyConstraintViolation. The +# raise escapes the sweeper, so a single aged row aborts the whole sweep +# and no pruning ever runs — databases grow without bound. +# +# SQLite cannot alter a foreign key in place, so rebuild the table. +Sequel.migration do + up do + # Drop rows whose content item is already gone; they would block the + # copy into a table that enforces the reference. + run <<~SQL + DELETE FROM ingestion_metrics + WHERE content_item_id NOT IN (SELECT id FROM content_items) + SQL + + create_table(:ingestion_metrics_new) do + primary_key :id + foreign_key :content_item_id, :content_items, null: false, on_delete: :cascade + Integer :input_tokens + Integer :output_tokens + Integer :facts_extracted + String :created_at, null: false + end + + run <<~SQL + INSERT INTO ingestion_metrics_new + (id, content_item_id, input_tokens, output_tokens, facts_extracted, created_at) + SELECT id, content_item_id, input_tokens, output_tokens, facts_extracted, created_at + FROM ingestion_metrics + SQL + + drop_table(:ingestion_metrics) + rename_table(:ingestion_metrics_new, :ingestion_metrics) + + run "CREATE INDEX IF NOT EXISTS idx_ingestion_metrics_content_item ON ingestion_metrics(content_item_id)" + run "CREATE INDEX IF NOT EXISTS idx_ingestion_metrics_created_at ON ingestion_metrics(created_at)" + end + + down do + create_table(:ingestion_metrics_old) do + primary_key :id + foreign_key :content_item_id, :content_items, null: false + Integer :input_tokens + Integer :output_tokens + Integer :facts_extracted + String :created_at, null: false + end + + run <<~SQL + INSERT INTO ingestion_metrics_old + (id, content_item_id, input_tokens, output_tokens, facts_extracted, created_at) + SELECT id, content_item_id, input_tokens, output_tokens, facts_extracted, created_at + FROM ingestion_metrics + SQL + + drop_table(:ingestion_metrics) + rename_table(:ingestion_metrics_old, :ingestion_metrics) + + run "CREATE INDEX IF NOT EXISTS idx_ingestion_metrics_content_item ON ingestion_metrics(content_item_id)" + run "CREATE INDEX IF NOT EXISTS idx_ingestion_metrics_created_at ON ingestion_metrics(created_at)" + end +end diff --git a/lib/claude_memory/commands/hook_command.rb b/lib/claude_memory/commands/hook_command.rb index d05b3c8..d23c6ee 100644 --- a/lib/claude_memory/commands/hook_command.rb +++ b/lib/claude_memory/commands/hook_command.rb @@ -137,9 +137,13 @@ def hook_ingest(handler, payload) when "session_excluded" stdout.puts "Session excluded by privacy marker" Hook::ExitCodes::SUCCESS - else - # transcript_not_found or other skipped reasons + when "transcript_not_found" + # Expected at SessionStart: a fresh session has no transcript yet stdout.puts "Skipped ingestion: #{result[:reason]}" + Hook::ExitCodes::SUCCESS + else + # Claude Code only surfaces stderr for non-zero exits + stderr.puts "Skipped ingestion: #{result[:reason]}" Hook::ExitCodes::WARNING end else diff --git a/lib/claude_memory/commands/index_command.rb b/lib/claude_memory/commands/index_command.rb index 71c2cb5..02577ba 100644 --- a/lib/claude_memory/commands/index_command.rb +++ b/lib/claude_memory/commands/index_command.rb @@ -200,7 +200,7 @@ def vec_backfill(opts) vec_index = store.vector_index unless vec_index.available? - stderr.puts "#{label.capitalize}: sqlite-vec not available, cannot backfill" + stderr.puts "#{label.capitalize}: sqlite-vec not available, cannot backfill#{" (#{vec_index.load_error})" if vec_index.load_error}" next end diff --git a/lib/claude_memory/configuration.rb b/lib/claude_memory/configuration.rb index 760bb4c..e523c42 100644 --- a/lib/claude_memory/configuration.rb +++ b/lib/claude_memory/configuration.rb @@ -9,6 +9,16 @@ module ClaudeMemory class Configuration attr_reader :env + # Environment overrides that clear git's per-invocation variables. A Hash + # passed to Process.spawn is merged into the caller's environment, so a + # filtered copy of ENV does not remove anything - only an explicit nil + # unsets a variable. + GIT_ENV_SCRUB = %w[ + GIT_DIR GIT_WORK_TREE GIT_INDEX_FILE GIT_COMMON_DIR + GIT_OBJECT_DIRECTORY GIT_ALTERNATE_OBJECT_DIRECTORIES + GIT_PREFIX GIT_CEILING_DIRECTORIES + ].to_h { |k| [k, nil] }.freeze + # @param env [Hash] environment variables (default: ENV) def initialize(env = ENV) @env = env @@ -21,7 +31,10 @@ def home_dir # @return [String] project root directory (resolves git worktrees) def project_dir - env["CLAUDE_PROJECT_DIR"] || resolve_project_dir + base = env["CLAUDE_PROJECT_DIR"] || Dir.pwd + return base if env["CLAUDE_MEMORY_ISOLATE_WORKTREES"] + + git_main_repo_root(base) || base end # @return [String] Claude config directory (default: ~/.claude) @@ -128,38 +141,45 @@ def settings_json_path private - def resolve_project_dir - return Dir.pwd if env["CLAUDE_MEMORY_ISOLATE_WORKTREES"] - - git_main_repo_root || Dir.pwd - end - - # Resolve main repository root, even when running inside a git worktree. - # Uses --git-common-dir which returns the shared .git directory across - # all worktrees, preventing duplicate project databases per worktree. - def git_main_repo_root - common_dir = git_command("rev-parse --git-common-dir") + # Resolve the main repository root for +dir+, even when +dir+ is a git + # worktree. --git-common-dir returns the shared .git directory across all + # worktrees, so every worktree of a repo maps to one project database. + # + # CLAUDE_PROJECT_DIR is resolved through here rather than returned as-is. + # Claude Code sets it on every hook and MCP subprocess, pointing at the + # directory the session started in, so reading it directly would skip this + # entirely and mint a database per worktree. Asking for that is what + # CLAUDE_MEMORY_ISOLATE_WORKTREES is for, and it still does exactly that. + def git_main_repo_root(dir) + common_dir = git_command(dir, "rev-parse", "--git-common-dir") return nil unless common_dir if common_dir == ".git" - git_command("rev-parse --show-toplevel") + git_command(dir, "rev-parse", "--show-toplevel") else - # Worktree - common_dir is absolute path to main repo's .git dir - # (or .git/worktrees parent). Resolve to repo root. - File.dirname(File.realpath(common_dir)) + # Worktree - common_dir points at the main repo's .git dir. It can be + # relative, and git resolves it against +dir+, not the process cwd. + File.dirname(File.realpath(File.absolute_path(common_dir, dir))) end - rescue Errno::ENOENT - # git not available or path doesn't exist + rescue Errno::ENOENT, Errno::ENOTDIR + # git not available, or the path does not exist nil end - # Run a git command and return stripped output, or nil on failure - def git_command(args) - output, status = Open3.capture2("git #{args}", err: File::NULL) + # Run a git command in +dir+ and return stripped output, or nil on failure. + # Array form, so a directory containing spaces or shell metacharacters is + # passed through as one argument rather than reparsed by a shell. + # + # GIT_ENV_SCRUB is passed so -C decides the repository. git exports GIT_DIR + # and GIT_INDEX_FILE to its own hooks, and those outrank -C, so a + # claude-memory command invoked from a git hook would otherwise resolve + # every project to whatever repository ran the hook. + def git_command(dir, *args) + output, status = Open3.capture2(GIT_ENV_SCRUB, "git", "-C", dir.to_s, *args, err: File::NULL) return nil unless status.success? stripped = output.strip stripped.empty? ? nil : stripped - rescue Errno::ENOENT + rescue Errno::ENOENT, Errno::ENOTDIR nil end end diff --git a/lib/claude_memory/index/vector_index.rb b/lib/claude_memory/index/vector_index.rb index 7ba2c1a..8c40865 100644 --- a/lib/claude_memory/index/vector_index.rb +++ b/lib/claude_memory/index/vector_index.rb @@ -26,6 +26,10 @@ def available? @available = load_extension! end + # Why the last extension probe failed, or nil when it succeeded. + # @return [String, nil] + attr_reader :load_error + # Insert (or replace) a fact's embedding into the vec0 virtual table. # Also sets vec_indexed_at on the fact row. # @param fact_id [Integer] @@ -203,8 +207,13 @@ def load_extension! @db.synchronize do |conn| SqliteVec.load(conn) end + @load_error = nil true - rescue LoadError, StandardError + rescue LoadError, StandardError => e + # Keep the reason: a silently-false probe here reads as "no vector + # support configured" when it is really a broken dependency, and an + # unusable extension is indistinguishable from an absent one. + @load_error = "#{e.class}: #{e.message}" false end diff --git a/lib/claude_memory/store/schema_manager.rb b/lib/claude_memory/store/schema_manager.rb index e1dd94b..d9ad4ce 100644 --- a/lib/claude_memory/store/schema_manager.rb +++ b/lib/claude_memory/store/schema_manager.rb @@ -5,7 +5,7 @@ module Store # Schema migration and version management for SQLiteStore. # Handles Sequel migrations, legacy version syncing, and initial setup. module SchemaManager - SCHEMA_VERSION = 21 + SCHEMA_VERSION = 22 private diff --git a/spec/claude_memory/cli_spec.rb b/spec/claude_memory/cli_spec.rb index 4c50da5..7f5e6ea 100644 --- a/spec/claude_memory/cli_spec.rb +++ b/spec/claude_memory/cli_spec.rb @@ -235,7 +235,7 @@ def run_cli_with_stdin(*args, input:) code = run_cli_with_stdin("hook", "ingest", "--db", db_path, input: payload) - expect(code).to eq(ClaudeMemory::Hook::ExitCodes::WARNING) + expect(code).to eq(ClaudeMemory::Hook::ExitCodes::SUCCESS) expect(stdout.string).to include("Skipped ingestion: transcript_not_found") end end diff --git a/spec/claude_memory/commands/hook_command_spec.rb b/spec/claude_memory/commands/hook_command_spec.rb index c00b59a..618ad14 100644 --- a/spec/claude_memory/commands/hook_command_spec.rb +++ b/spec/claude_memory/commands/hook_command_spec.rb @@ -68,7 +68,7 @@ expect(stdout.string).to include("excluded") end - it "returns WARNING (1) for skipped ingest (missing file)" do + it "returns SUCCESS (0) for missing transcript (fresh session at SessionStart)" do payload = { "session_id" => "sess-123", "transcript_path" => "/nonexistent/file.txt" @@ -77,7 +77,8 @@ exit_code = command.call(["ingest", "--db", db_path]) - expect(exit_code).to eq(ClaudeMemory::Hook::ExitCodes::WARNING) + expect(exit_code).to eq(ClaudeMemory::Hook::ExitCodes::SUCCESS) + expect(stdout.string).to include("transcript_not_found") end it "returns ERROR (2) for invalid JSON payload" do diff --git a/spec/claude_memory/configuration_spec.rb b/spec/claude_memory/configuration_spec.rb index 518035f..48787ba 100644 --- a/spec/claude_memory/configuration_spec.rb +++ b/spec/claude_memory/configuration_spec.rb @@ -1,6 +1,9 @@ # frozen_string_literal: true require "spec_helper" +require "tmpdir" +require "fileutils" +require "open3" RSpec.describe ClaudeMemory::Configuration do describe "#initialize" do @@ -29,32 +32,115 @@ end describe "#project_dir" do - it "returns CLAUDE_PROJECT_DIR when set" do + it "returns CLAUDE_PROJECT_DIR when it is not inside a git repo" do config = described_class.new({"CLAUDE_PROJECT_DIR" => "/path/to/project"}) + allow(config).to receive(:git_command).and_return(nil) + expect(config.project_dir).to eq("/path/to/project") end it "resolves git repo root for regular repos" do config = described_class.new({}) - allow(config).to receive(:git_command).with("rev-parse --git-common-dir").and_return(".git") - allow(config).to receive(:git_command).with("rev-parse --show-toplevel").and_return("/repo/root") + allow(config).to receive(:git_command).with(Dir.pwd, "rev-parse", "--git-common-dir").and_return(".git") + allow(config).to receive(:git_command).with(Dir.pwd, "rev-parse", "--show-toplevel").and_return("/repo/root") expect(config.project_dir).to eq("/repo/root") end it "resolves main repo root when inside a git worktree" do config = described_class.new({}) - allow(config).to receive(:git_command).with("rev-parse --git-common-dir").and_return("/main/repo/.git") + allow(config).to receive(:git_command).with(Dir.pwd, "rev-parse", "--git-common-dir").and_return("/main/repo/.git") + allow(File).to receive(:realpath).with("/main/repo/.git").and_return("/main/repo/.git") + + expect(config.project_dir).to eq("/main/repo") + end + + # Claude Code sets CLAUDE_PROJECT_DIR on every hook and MCP subprocess, to + # the directory the session started in. Returning it as-is skipped the + # worktree resolution entirely and gave each worktree its own database. + it "resolves CLAUDE_PROJECT_DIR through the git root when it names a worktree" do + config = described_class.new({"CLAUDE_PROJECT_DIR" => "/wt/feature"}) + allow(config).to receive(:git_command).with("/wt/feature", "rev-parse", "--git-common-dir").and_return("/main/repo/.git") allow(File).to receive(:realpath).with("/main/repo/.git").and_return("/main/repo/.git") expect(config.project_dir).to eq("/main/repo") end + it "resolves CLAUDE_PROJECT_DIR up to the repo root when it names a subdirectory" do + config = described_class.new({"CLAUDE_PROJECT_DIR" => "/repo/root/backend"}) + allow(config).to receive(:git_command).with("/repo/root/backend", "rev-parse", "--git-common-dir").and_return(".git") + allow(config).to receive(:git_command).with("/repo/root/backend", "rev-parse", "--show-toplevel").and_return("/repo/root") + + expect(config.project_dir).to eq("/repo/root") + end + it "uses Dir.pwd when CLAUDE_MEMORY_ISOLATE_WORKTREES is set" do config = described_class.new({"CLAUDE_MEMORY_ISOLATE_WORKTREES" => "1"}) expect(config.project_dir).to eq(Dir.pwd) end + it "keeps CLAUDE_PROJECT_DIR unresolved when CLAUDE_MEMORY_ISOLATE_WORKTREES is set" do + config = described_class.new({ + "CLAUDE_PROJECT_DIR" => "/wt/feature", + "CLAUDE_MEMORY_ISOLATE_WORKTREES" => "1" + }) + expect(config).not_to receive(:git_command) + + expect(config.project_dir).to eq("/wt/feature") + end + + context "against a real git worktree" do + let(:tmp) { Dir.mktmpdir("cm-worktree") } + let(:repo) { File.join(tmp, "repo") } + let(:worktree) { File.join(tmp, "wt") } + + # git exports GIT_DIR, GIT_INDEX_FILE and friends to its own hooks, so a + # suite run from pre-commit inherits them and every git call below acts + # on the wrong repository. Scrub them, and raise on failure instead of + # discarding stderr - a fixture that silently fails to build breaks a + # later expectation, somewhere that does not name the cause. + def git!(*args) + out, status = Open3.capture2e(ClaudeMemory::Configuration::GIT_ENV_SCRUB, "git", *args) + raise "git #{args.join(" ")} failed: #{out}" unless status.success? + out + end + + before do + FileUtils.mkdir_p(repo) + git!("init", "-q", repo) + git!("-C", repo, "config", "user.email", "t@example.com") + git!("-C", repo, "config", "user.name", "t") + File.write(File.join(repo, "README"), "x") + git!("-C", repo, "add", "README") + git!("-C", repo, "commit", "-qm", "init") + git!("-C", repo, "worktree", "add", "-q", worktree, "-b", "feature") + raise "worktree fixture was not created" unless File.directory?(worktree) + end + + after { FileUtils.remove_entry(tmp) } + + it "maps a worktree named by CLAUDE_PROJECT_DIR back to the main repo" do + config = described_class.new({"CLAUDE_PROJECT_DIR" => worktree}) + expect(File.realpath(config.project_dir)).to eq(File.realpath(repo)) + end + + it "gives the worktree and the main repo the same project database" do + from_worktree = described_class.new({"CLAUDE_PROJECT_DIR" => worktree}).project_db_path + from_repo = described_class.new({"CLAUDE_PROJECT_DIR" => repo}).project_db_path + + expect(File.realpath(File.dirname(from_worktree, 2))) + .to eq(File.realpath(File.dirname(from_repo, 2))) + end + + it "still isolates the worktree when CLAUDE_MEMORY_ISOLATE_WORKTREES is set" do + config = described_class.new({ + "CLAUDE_PROJECT_DIR" => worktree, + "CLAUDE_MEMORY_ISOLATE_WORKTREES" => "1" + }) + expect(config.project_dir).to eq(worktree) + end + end + it "falls back to Dir.pwd when git is not available" do config = described_class.new({}) allow(config).to receive(:git_command).and_return(nil) @@ -64,7 +150,7 @@ it "falls back to Dir.pwd when not in a git repo" do config = described_class.new({}) - allow(config).to receive(:git_command).with("rev-parse --git-common-dir").and_return(nil) + allow(config).to receive(:git_command).with(Dir.pwd, "rev-parse", "--git-common-dir").and_return(nil) expect(config.project_dir).to eq(Dir.pwd) end diff --git a/spec/claude_memory/sweep/maintenance_spec.rb b/spec/claude_memory/sweep/maintenance_spec.rb index a02b705..d54ec87 100644 --- a/spec/claude_memory/sweep/maintenance_spec.rb +++ b/spec/claude_memory/sweep/maintenance_spec.rb @@ -152,6 +152,19 @@ def create_content(days_ago:) create_content(days_ago: 10) expect(maintenance.prune_old_content).to eq(0) end + + it "prunes aged content that still carries an ingestion metric" do + content_id = create_content(days_ago: 35) + store.record_ingestion_metrics( + content_item_id: content_id, + input_tokens: 10, + output_tokens: 5, + facts_extracted: 1 + ) + + expect(maintenance.prune_old_content).to eq(1) + expect(store.ingestion_metrics.where(content_item_id: content_id).count).to eq(0) + end end describe "#backfill_vec_index" do