From 29e402991757899b936a8c8606aa41c6cd688ff5 Mon Sep 17 00:00:00 2001 From: UnbreakableMJ Date: Tue, 25 Aug 2026 22:42:04 +0300 Subject: [PATCH] Reach projects under dotted paths, and let --cwd name the scope MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects that both made `ingest` lie quietly rather than fail. `mangle_cwd` replaced only `/`. The harness replaces every character outside `[A-Za-z0-9\-_]` — its own function is `e.replace(/[^a-zA-Z0-9\-_]/g,"-")` with `""` mapping to `unknown` — so a dot becomes a dash too. Engram therefore looked for `…construct-.claude-worktrees-x` where Claude Code had written `…construct--claude-worktrees-x`, and reported NOT_FOUND, which reads as "no such session" rather than "I spelled the directory wrong". Every worktree Claude Code creates lives under `.claude/worktrees/`, so this grew with use. The test planters carried a copy of the same wrong rule, which mattered more than it looks: a TempDir path contains `.tmpXXXX`, so after the fix they would have planted into a directory the reader no longer reads. Both now call one helper that mirrors production, with a comment saying why. `ingest --cwd` chose which transcripts to read but never the scope, which still resolved from the process's directory. Importing thirty-three projects from one terminal filed every one of them under whichever scope that terminal was in, and the response said `scope_origin: "git-root"` each time — correct-looking output for silently mis-filed data. `rules::resolve_scope_in` takes the directory as an argument; `resolve_scope` now delegates to it. Explicit `--scope` still wins. Verified on the real machine: the worktree that returned NOT_FOUND now yields 12 turns, and `--cwd /spacecraft-software/vault` files into `vault`. Gates: fmt, clippy -D warnings, 264 tests, REUSE 3.3, makeinfo clean. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016i16R4GhdSffsboRYq97Fs --- AGENTS.md | 8 ++- CHANGELOG.md | 10 ++++ src/main.rs | 8 +-- src/rules.rs | 37 +++++++++--- src/transcript/mod.rs | 80 +++++++++++++++++++++++--- tests/cli.rs | 127 +++++++++++++++++++++++++++++++++++++++++- 6 files changed, 247 insertions(+), 23 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index dd083c4..cbb1dcc 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -83,7 +83,11 @@ engram rule sync [--scope S] [--file PATH]... [--dry-run] ``` - **Scope cascade:** `--scope` → `ENGRAM_SCOPE` → git working-tree basename → cwd - basename. Returned as `scope_origin` on every response. Under MCP this resolves + basename. Returned as `scope_origin` on every response. `rules::resolve_scope` + resolves against the process's directory; `rules::resolve_scope_in` takes the + directory as an argument, which is what `ingest --cwd` uses — reading another + project's transcripts while filing them under the caller's scope was silent + mis-filing, and the response said `git-root` while doing it. Under MCP this resolves against the *server process's* cwd — pass `scope` explicitly for a shared server. - **`sync` is the delivery step.** `rule add` only writes SQLite; nothing reads SQLite. Rendering into `AGENTS.md`/`CLAUDE.md` is what makes a rule take effect. @@ -254,7 +258,7 @@ Two readers exist: `claude_code` and `codex`. Adding a third means adding a `Rea - **Codex session ids are per-rollout, NOT `session_meta.session_id`.** That field is *not unique* — resuming a session writes a new file reusing the same id, and three files sharing one id exist on this machine. Since `turn_id` derives from the session id, reusing it would collide turns at the same line index across rollouts and `INSERT OR IGNORE` would silently drop them. Engram therefore keys on the file name minus `rollout-` (unique, sortable, still contains the uuid). There is a test for exactly this. - **Codex records carry no per-record id**, so `source_uuid` is `{line_index}:{v5 digest of the text}`. The index alone would suffice for an append-only log; folding in the content means an inserted line does not renumber every later turn into a new identity. - **`--max-bytes` is not theoretical.** A 114 MB rollout exists on this machine; the 64 MiB default refuses it with a structured error naming the override. Both readers stream line by line. -- **Claude Code layout:** `~/.claude/projects//.jsonl`. `mangle_cwd` replaces every `/` with `-` (so the leading slash becomes a leading dash) and **preserves case** — `-spacecraft-software-Majestic` and `…-majestic` are different directories. **Forward-only by construction**: a literal `-` in a path is indistinguishable from a separator in the result, so no inverse is exported. Sibling `/subagents/` transcripts are deliberately not read — a subagent is a different conversation and folding it in would interleave two narratives by timestamp. +- **Claude Code layout:** `~/.claude/projects//.jsonl`. `mangle_cwd` replaces **every character outside `[A-Za-z0-9_-]`** with `-` (so the leading slash becomes a leading dash, and a dot becomes one too), **preserves case** — `-spacecraft-software-Majestic` and `…-majestic` are different directories — and maps an empty result to `unknown`. The harness's own function is the authority: `e.replace(/[^a-zA-Z0-9\-_]/g,"-")`. Replacing only `/` was wrong for any dotted path and silently so: `/…/construct/.claude/worktrees/x` is written `…construct--claude-worktrees-x`, engram looked for `…construct-.claude-worktrees-x`, and the session read as `NOT_FOUND` rather than as a mangling failure — and *every* worktree Claude Code creates lives under `.claude/worktrees/`. The test helper in `tests/cli.rs` must mirror the rule exactly; a `TempDir` path contains `.tmpXXXX`, so a planter using the old rule writes a directory the reader no longer looks in. **Forward-only by construction**: a literal `-` in a path is indistinguishable from a replaced character in the result, so no inverse is exported. Sibling `/subagents/` transcripts are deliberately not read — a subagent is a different conversation and folding it in would interleave two narratives by timestamp. - **Filtering is the feature, not a detail.** Measured on a real 1.7 MB session: 935 records in, **46 turns out** — 140 `tool_use`, 139 `tool_result`, 52 `thinking`, 226 non-message, 331 empty. Tool payloads and thinking are excluded **by default**; even with `--include-tools` a tool result is summarized to its byte size and the payload is *never* stored, because payloads are where file contents, command output, and credentials live. Every drop is counted in `filtered` and reported. - **Never guess, two rules.** (a) Anything a read cannot turn into a turn is counted rather than skipped silently, in **three separate counters**, because the three mean different things and call for different responses. `unknown_record` is an unrecognized record `type` — a format change in a file engram does not own, fixed by extending an allowlist; it earned its keep by surfacing three Codex tool types (`web_search_call`, `tool_search_call`, `tool_search_output`) that the first implementation miscategorized. `torn_line` is an interrupted write, which lands mid-file and not only at EOF; nothing in engram is wrong, and it is *transient* when a transcript is read while its harness is still appending. `missing_uuid` is a conversation record with no `uuid` — the only one of the three where a real turn was lost. These shared one counter until 2026-08-08, which made every torn line read as a format change and sent a reader chasing a harness that had not moved: one session reported 56 "unknown records" that were all complete lines minutes later. A signal that cries wolf two times in three stops being read, which costs exactly the early warning the counter exists to give. (b) An unparseable timestamp is an **error**, never a substitution of now: `recall_inner` orders by `created_at`, so a wall-clock fallback would collapse a whole conversation into one instant and destroy reading order invisibly. - **`created_at` is the transcript's timestamp**, and `valid_from` is set to match. This bends the documented "`created_at` is transaction time" reading, and has to, for the ordering reason above. diff --git a/CHANGELOG.md b/CHANGELOG.md index f0bbd64..ae05a13 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,16 @@ follows [Keep a Changelog](https://keepachangelog.com/); versions follow ### Fixed +- **A project under a dotted directory was unreachable.** `mangle_cwd` replaced + only `/`, but the harness replaces every character outside `[A-Za-z0-9_-]`, so + anything beneath `.claude/worktrees/` — every worktree Claude Code creates — + resolved to a directory that does not exist and reported `NOT_FOUND` as though + the session were missing. +- **`ingest --cwd` now sets the scope.** It selected which transcripts to read + while scope still resolved from the process's own directory, so importing many + projects from one terminal filed them all under one scope and reported + `scope_origin: "git-root"` while doing it. An explicit `--scope` still wins. + - **A skill description containing a colon silently broke the whole skill.** `description: Save this conversation: capture ...` is invalid YAML, so Antigravity loaded two of engram's three commands and reported nothing. diff --git a/src/main.rs b/src/main.rs index ae46a55..d946f66 100644 --- a/src/main.rs +++ b/src/main.rs @@ -979,10 +979,10 @@ fn handle_ingest( ); } - let resolved = match rules::resolve_scope(scope.as_deref()) { - Ok(r) => r, - Err(e) => return fail(scope_error(e), mode), - }; + // Resolve the scope against the directory whose transcripts are being read, + // not the one the process was started in. `--cwd` names another project; + // filing its history under the caller's scope would be silent mis-filing. + let resolved = rules::resolve_scope_in(scope.as_deref(), &cwd); let capture = match transcript::capture(store, spec, &selected, &resolved.name, &opts, dry_run) { diff --git a/src/rules.rs b/src/rules.rs index 9d4852f..af92b7d 100644 --- a/src/rules.rs +++ b/src/rules.rs @@ -97,41 +97,62 @@ pub struct ResolvedScope { /// Returns an error if the current working directory cannot be read. pub fn resolve_scope(explicit: Option<&str>) -> std::io::Result { let cwd = std::env::current_dir()?; + Ok(resolve_scope_in(explicit, &cwd)) +} + +/// Resolves the scope as [`resolve_scope`] does, but relative to `cwd`. +/// +/// Every step of the cascade below the explicit argument is a question about a +/// *directory* — which git tree encloses it, what it is called — so a command +/// that already knows which directory it is acting on must be able to say so. +/// +/// `ingest --cwd` is the case that forced this. It reads another project's +/// transcripts, and while it did that, scope still resolved from the process's +/// own directory: importing thirty-three projects from one terminal filed all +/// of them under whichever scope that terminal happened to be in, reporting +/// `scope_origin: "git-root"` each time, which looked entirely correct. The +/// mis-filing was invisible in the output and expensive to undo. +/// +/// Infallible by construction — the caller has already produced the directory, +/// so there is no `current_dir()` call left to fail. +#[must_use] +pub fn resolve_scope_in(explicit: Option<&str>, cwd: &Path) -> ResolvedScope { + let cwd = cwd.to_path_buf(); let git_root = managed_file::find_git_root(&cwd); let root = git_root.clone().unwrap_or_else(|| cwd.clone()); if let Some(name) = explicit.map(str::trim).filter(|s| !s.is_empty()) { - return Ok(ResolvedScope { + return ResolvedScope { name: name.to_string(), root, origin: ScopeOrigin::Explicit, - }); + }; } if let Some(name) = std::env::var("ENGRAM_SCOPE") .ok() .map(|s| s.trim().to_string()) .filter(|s| !s.is_empty()) { - return Ok(ResolvedScope { + return ResolvedScope { name, root, origin: ScopeOrigin::Env, - }); + }; } if let Some(dir) = git_root { let name = basename(&dir); - return Ok(ResolvedScope { + return ResolvedScope { name, root: dir, origin: ScopeOrigin::GitRoot, - }); + }; } let name = basename(&cwd); - Ok(ResolvedScope { + ResolvedScope { name, root, origin: ScopeOrigin::Cwd, - }) + } } fn basename(path: &Path) -> String { diff --git a/src/transcript/mod.rs b/src/transcript/mod.rs index 3dd0819..9358227 100644 --- a/src/transcript/mod.rs +++ b/src/transcript/mod.rs @@ -255,16 +255,46 @@ pub fn turn_id(harness: &str, session_id: &str, source_uuid: &str) -> String { /// Maps a working directory to Claude Code's project-directory name. /// /// `/spacecraft-software/engram` becomes `-spacecraft-software-engram`: every -/// `/` becomes `-`, which turns the leading slash into a leading dash, and -/// **case is preserved** (`-spacecraft-software-Majestic` and -/// `-spacecraft-software-majestic` are different directories on disk). +/// Every character outside `[A-Za-z0-9_-]` becomes `-`, which turns the +/// leading slash into a leading dash, and **case is preserved** +/// (`-spacecraft-software-Majestic` and `-spacecraft-software-majestic` are +/// different directories on disk). An empty result becomes `unknown`. +/// +/// This mirrors the harness's own function, which is the authority — engram +/// does not get to choose the spelling of a directory somebody else creates: +/// +/// ```js +/// function tl(e){ let t=e.replace(/[^a-zA-Z0-9\-_]/g,"-"); return t===""?"unknown":t } +/// ``` +/// +/// Replacing only `/` was wrong for any path containing a dot, and silently +/// so: `/spacecraft-software/construct/.claude/worktrees/x` mangles to +/// `…construct--claude-worktrees-x`, engram looked for +/// `…construct-.claude-worktrees-x`, and the session came back `NOT_FOUND` as +/// though it did not exist. Claude Code puts every worktree it creates under +/// `.claude/worktrees/`, so the blind spot grows with use. /// /// Forward-only by construction. A literal `-` in a directory name is -/// indistinguishable from a separator in the result, so the inverse mapping -/// does not exist and is deliberately not offered — callers always start from -/// a working directory they already know. +/// indistinguishable from a replaced character in the result, so the inverse +/// mapping does not exist and is deliberately not offered — callers always +/// start from a working directory they already know. pub fn mangle_cwd(cwd: &Path) -> String { - cwd.to_string_lossy().replace('/', "-") + let mangled: String = cwd + .to_string_lossy() + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '-' + } + }) + .collect(); + if mangled.is_empty() { + "unknown".to_owned() + } else { + mangled + } } /// Normalizes a transcript timestamp to ISO 8601 UTC with a `Z` suffix. @@ -539,6 +569,42 @@ mod tests { assert_eq!(mangle_cwd(Path::new("/a-b/c")), "-a-b-c"); } + /// A dot is replaced too, and an underscore is not. + /// + /// Replacing only `/` made every worktree under `.claude/worktrees/` + /// unreadable — engram looked for a directory the harness never wrote and + /// reported `NOT_FOUND`, which reads as "no such session" rather than "I + /// mangled the name". The underscore case pins the other half of the + /// harness's character class: `_` is *kept*, so mapping it to `-` would + /// break paths that a naive "replace punctuation" rule would mangle. + #[test] + fn mangle_cwd_replaces_dots_and_keeps_underscores() { + assert_eq!( + mangle_cwd(Path::new( + "/spacecraft-software/construct/.claude/worktrees/peaceful-jones" + )), + "-spacecraft-software-construct--claude-worktrees-peaceful-jones" + ); + assert_eq!( + mangle_cwd(Path::new("/home/mj/my_project")), + "-home-mj-my_project" + ); + // Anything else outside the class collapses to a dash as well. + assert_eq!(mangle_cwd(Path::new("/a b/c.d")), "-a-b-c-d"); + // One `char`, one dash — the mapping is per-`char`, not per-byte, so a + // multi-byte character does not become a run of dashes. + assert_eq!(mangle_cwd(Path::new("/v1.2/café")), "-v1-2-caf-"); + } + + /// An empty path is `unknown`, not the empty string. + /// + /// The harness names that directory `unknown`; a bare `""` would make + /// engram join the sessions root itself and list every project at once. + #[test] + fn mangle_cwd_of_an_empty_path_is_unknown() { + assert_eq!(mangle_cwd(Path::new("")), "unknown"); + } + #[test] fn turn_id_is_stable_and_distinct() { let a = turn_id("claude-code", "sess", "rec"); diff --git a/tests/cli.rs b/tests/cli.rs index 601e4f4..9d40d7f 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1711,8 +1711,29 @@ fn which_makeinfo() -> Result { /// The directory name is produced by mangling `project` forward — `/` becomes /// `-`, case preserved — exactly as the reader does. Nothing here reverses a /// mangled name, because that mapping does not exist. +/// Mangles a path the way the harness does, for planting a fake transcript. +/// +/// This must mirror `transcript::mangle_cwd` exactly: every character outside +/// `[A-Za-z0-9_-]` becomes `-`. Replacing only `/` is not equivalent, and the +/// difference bites here in particular — a `TempDir` path contains `.tmpXXXX`, +/// so a test that planted with the old rule would write a directory the reader +/// no longer looks in, and every ingest test would fail for a reason that has +/// nothing to do with what it is testing. +fn mangle_like_the_harness(path: &Path) -> String { + path.to_string_lossy() + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '-' + } + }) + .collect() +} + fn plant_claude_transcript(home: &Path, project: &Path, session_id: &str) -> std::path::PathBuf { - let mangled = project.to_string_lossy().replace('/', "-"); + let mangled = mangle_like_the_harness(project); let dir = home.join(".claude").join("projects").join(mangled); std::fs::create_dir_all(&dir).expect("create fake projects dir"); let src = Path::new(env!("CARGO_MANIFEST_DIR")) @@ -3189,7 +3210,7 @@ fn plant_openclaude_transcript( project: &Path, session_id: &str, ) -> std::path::PathBuf { - let mangled = project.to_string_lossy().replace('/', "-"); + let mangled = mangle_like_the_harness(project); let dir = home.join(".openclaude").join("projects").join(mangled); std::fs::create_dir_all(&dir).expect("create fake projects dir"); let src = Path::new(env!("CARGO_MANIFEST_DIR")) @@ -3494,3 +3515,105 @@ fn install_warns_when_two_harnesses_share_a_skills_directory() { .clone(); assert!(codex["warning"].is_null(), "codex: {:?}", codex["warning"]); } + +/// A project under a dotted directory is reachable. +/// +/// `mangle_cwd` replaced only `/`, so `.claude/worktrees/x` was looked for as +/// `-…-.claude-worktrees-x` while the harness had written +/// `-…--claude-worktrees-x`. Ingest reported `NOT_FOUND` — indistinguishable +/// from "that session does not exist" — and every worktree Claude Code creates +/// lives under exactly that path, so the blind spot grew with use. +#[test] +fn ingest_reads_a_project_under_a_dotted_directory() { + let tmp = TempDir::new().expect("tempdir"); + let db = tmp.path().join("test.db"); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).expect("create fake home"); + + // The shape Claude Code creates for a worktree. + let project = tmp.path().join(".claude/worktrees/peaceful-jones"); + std::fs::create_dir_all(&project).expect("create worktree dir"); + std::fs::write(project.join(".git"), "gitdir: elsewhere\n").expect("pin the git root"); + plant_claude_transcript(&home, &project, "sess-dotted"); + + let assert = ingest( + &db, + &home, + &project, + &["--harness", "claude-code", "--session", "all"], + ) + .assert() + .success(); + let data = parse_single_line_json(&assert.get_output().stdout)["data"].clone(); + + assert_eq!(data["sessions"].as_array().expect("sessions").len(), 1); + assert!( + data["inserted"].as_u64().expect("inserted") > 0, + "a dotted path must not read as an empty harness: {data}" + ); +} + +/// `--cwd` names the project, so it must name the scope too. +/// +/// Reading another project's transcripts while filing them under the caller's +/// own scope is silent mis-filing: the response said `scope_origin: "git-root"` +/// and looked entirely correct. Importing many projects from one terminal put +/// all of them in one scope. +#[test] +fn ingest_cwd_sets_the_scope_and_explicit_scope_still_wins() { + let tmp = TempDir::new().expect("tempdir"); + let db = tmp.path().join("test.db"); + let home = tmp.path().join("home"); + std::fs::create_dir_all(&home).expect("create fake home"); + + // Two projects: one we stand in, one we point at. + let here = pinned_project(&tmp); + let elsewhere = tmp.path().join("other-project"); + std::fs::create_dir_all(&elsewhere).expect("create other project"); + std::fs::write(elsewhere.join(".git"), "gitdir: elsewhere\n").expect("pin the git root"); + plant_claude_transcript(&home, &elsewhere, "sess-elsewhere"); + + let cwd = elsewhere.to_string_lossy().into_owned(); + let assert = ingest( + &db, + &home, + &here, + &[ + "--harness", + "claude-code", + "--cwd", + &cwd, + "--session", + "all", + "--dry-run", + ], + ) + .assert() + .success(); + let data = parse_single_line_json(&assert.get_output().stdout)["data"].clone(); + assert_eq!(data["scope"], "other-project", "scope must follow --cwd"); + assert_eq!(data["scope_origin"], "git-root"); + + // An explicit scope still beats the inferred one. + let assert = ingest( + &db, + &home, + &here, + &[ + "--harness", + "claude-code", + "--cwd", + &cwd, + "--scope", + "pinned", + "--session", + "all", + "--dry-run", + ], + ) + .assert() + .success(); + let data = parse_single_line_json(&assert.get_output().stdout)["data"].clone(); + assert_eq!(data["scope"], "pinned"); + assert_eq!(data["scope_origin"], "explicit"); +}