fix(hooks): resolve project name to repo basename, not raw cwd#475
Open
efenex wants to merge 5 commits into
Open
fix(hooks): resolve project name to repo basename, not raw cwd#475efenex wants to merge 5 commits into
efenex wants to merge 5 commits into
Conversation
|
@efenex is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds centralized resolveProject(cwd?) (env | git toplevel basename | dirname basename), applies basename-aware projectMatches() in context lesson/session selection, updates hooks/scripts to send resolved project and cwd, and adds tests for resolution and matching. ChangesProject Resolution and Basename-aware Lesson Matching
Sequence Diagram(s)sequenceDiagram
participant HookScript
participant Resolve as resolveProject
participant Git as git
participant API as /agentmemory/observe
HookScript->>Resolve: resolveProject(cwd)
Resolve->>Git: git rev-parse --show-toplevel (timeout)
Git-->>Resolve: git-toplevel-path or error
Resolve-->>HookScript: project (env | git-toplevel-basename | dir-basename)
HookScript->>API: POST { project, cwd, ... }
API-->>HookScript: 200/ack
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/functions/context.ts (1)
26-31: ⚡ Quick winRemove WHAT-style block comments and rely on naming/tests instead.
These comments describe behavior/history in detail; per repo rules for
src/**/*.ts, prefer clear naming and tests over explanatory WHAT comments in code.As per coding guidelines, "Do not use code comments explaining WHAT — use clear naming instead".
🤖 Prompt for 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. In `@src/functions/context.ts` around lines 26 - 31, Remove the WHAT-style explanatory block comment in src/functions/context.ts and instead rely on clearer naming and tests: delete the multi-line comment that explains legacy `project` values and hook history, ensure any logic referencing the `project` variable (and strings like `memory_lesson_save` / `mem::replay::import-jsonl`) keeps its current behavior, and add/update unit tests that assert matching both basename and full-cwd forms so the intent is captured by test names rather than an in-code narrative comment.src/hooks/_project.ts (1)
24-25: ⚡ Quick winStreamline environment variable handling.
The current code calls
trim()twice. Using optional chaining is more idiomatic and avoids the redundant call.♻️ Proposed refactor
- const explicit = process.env["AGENTMEMORY_PROJECT_NAME"]; - if (explicit && explicit.trim()) return explicit.trim(); + const explicit = process.env["AGENTMEMORY_PROJECT_NAME"]?.trim(); + if (explicit) return explicit;🤖 Prompt for 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. In `@src/hooks/_project.ts` around lines 24 - 25, The code redundantly calls trim() twice when reading AGENTMEMORY_PROJECT_NAME; change the logic to read the env var once using optional chaining and trim it once (e.g., assign process.env["AGENTMEMORY_PROJECT_NAME"]?.trim() to explicit) and then return explicit only if truthy; update references to the explicit variable in the function in src/hooks/_project.ts so the value is trimmed a single time.
🤖 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 `@src/hooks/_project.ts`:
- Line 27: The current assignment to dir uses the untrimmed cwd when cwd
contains whitespace (const dir = cwd && cwd.trim() ? cwd : process.cwd()), so
update it to use the trimmed value: capture const trimmed = cwd?.trim(); then
set dir to trimmed when non-empty, otherwise fallback to process.cwd();
reference the cwd variable and dir constant in src/hooks/_project.ts and ensure
any subsequent basename(dir) calls receive the trimmed path.
In `@src/hooks/notification.ts`:
- Around line 37-38: Guard the untrusted data.cwd before passing to
resolveProject: check that data.cwd is a non-empty string and otherwise fall
back to process.cwd(), e.g. validate typeof data.cwd === "string" and truthiness
before assigning cwd, then call resolveProject(cwd); update the assignment that
currently reads const cwd = (data.cwd as string) || process.cwd() to perform
this runtime type check so resolveProject and the subsequent observe call always
receive a valid string.
In `@test/hook-project.test.ts`:
- Around line 2-5: Replace the shelling out that uses execSync to create
directories with Node's fs.mkdirSync(..., { recursive: true }): locate where
execSync is invoked to create a path (reference execSync usage in
test/hook-project.test.ts), change that call to use mkdirSync(targetPath, {
recursive: true }) and add an import for mkdirSync from "node:fs"; also remove
the execSync import if it becomes unused to keep imports clean.
---
Nitpick comments:
In `@src/functions/context.ts`:
- Around line 26-31: Remove the WHAT-style explanatory block comment in
src/functions/context.ts and instead rely on clearer naming and tests: delete
the multi-line comment that explains legacy `project` values and hook history,
ensure any logic referencing the `project` variable (and strings like
`memory_lesson_save` / `mem::replay::import-jsonl`) keeps its current behavior,
and add/update unit tests that assert matching both basename and full-cwd forms
so the intent is captured by test names rather than an in-code narrative
comment.
In `@src/hooks/_project.ts`:
- Around line 24-25: The code redundantly calls trim() twice when reading
AGENTMEMORY_PROJECT_NAME; change the logic to read the env var once using
optional chaining and trim it once (e.g., assign
process.env["AGENTMEMORY_PROJECT_NAME"]?.trim() to explicit) and then return
explicit only if truthy; update references to the explicit variable in the
function in src/hooks/_project.ts so the value is trimmed a single time.
🪄 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
Run ID: 4c00c45e-c901-470a-86be-ab3a70226412
📒 Files selected for processing (13)
.env.examplesrc/functions/context.tssrc/hooks/_project.tssrc/hooks/notification.tssrc/hooks/post-tool-failure.tssrc/hooks/post-tool-use.tssrc/hooks/prompt-submit.tssrc/hooks/session-start.tssrc/hooks/subagent-start.tssrc/hooks/subagent-stop.tssrc/hooks/task-completed.tstest/context-lessons.test.tstest/hook-project.test.ts
Contributor
Author
|
Thanks for the review — pushed 3827fc7 addressing all three nits:
Tests pass ( |
Owner
|
Hey @efenex — solid fix for #515 (Codex worktrees fragmenting context). The Three minor things CodeRabbit flagged that are worth tightening before merge:
The Push the two fixes and this is good to merge. |
Follow-up to rohitg00#473. The Claude Code SessionStart hook (and seven other hooks that POST observations) computed `project = data.cwd || process.cwd()` — i.e. the absolute path of the working directory. But every other code path that tags records with a project uses the basename: - mem::replay::import-jsonl preserves whatever the JSONL file contains, which is the basename Claude Code itself stores in transcripts - humans calling memory_lesson_save naturally pass the short name ("gitops-assistant", not "/Users/me/work/foo/gitops-assistant") Result: the auto-inject context block at session start ran a project-equality filter that matched zero of the bulk of the operator's data. A 4351-lesson, 52-summary store rendered as a 3-block context payload with no Lessons Learned section at all (just session summaries from the rare records tagged with the full cwd). Two-sided fix: 1. Hooks now resolve project via a shared `src/hooks/_project.ts` helper. Resolution order: - AGENTMEMORY_PROJECT_NAME env var (operator escape hatch, settable per-repo via Claude Code's .claude/settings.json `env` block) - basename(`git rev-parse --show-toplevel`) — handles sessions started in a subdirectory of the repo, survives moving the repo - basename(cwd) — final fallback for non-git directories All eight hooks that POST a project field were updated (session-start, prompt-submit, post-tool-use, post-tool-failure, notification, subagent-start, subagent-stop, task-completed). 2. mem::context tolerates the mixed-vintage corpus that this regression has already produced. The project-match for both lessons and sessions now uses a basename-equality fallback: exact match wins, then basename(stored) === basename(requested). Means existing records tagged with the full cwd continue to surface for callers passing the basename, and vice versa, without requiring re-import. Documented trade-off: same-named repos in different paths now collapse to one logical project at retrieval time (e.g. /work/foo and /personal/foo). Operators who need to keep them separate set AGENTMEMORY_PROJECT_NAME explicitly per repo. Behavior is asserted in test/context-lessons.test.ts to make any future tightening a known break, not an accidental one. Tests: 17/17 in the directly impacted files. Test layout: - test/hook-project.test.ts: AGENTMEMORY_PROJECT_NAME precedence, whitespace handling, git-toplevel basename when inside a repo, cwd basename when not in a repo, subdir-of-repo handling - test/context-lessons.test.ts: full-path-caller matches basename-tagged lesson, basename-caller matches full-path-tagged lesson, same-basename-different-path collapse The remaining 10 failures in the full suite are pre-existing on main (embedding-provider / auto-compress / fetch-timeout env-pollution tests that fail when ~/.agentmemory/.env has API keys set); unrelated to this change. Filed against rohitg00#474.
- _project.ts: use cwd?.trim() || process.cwd() so a whitespace-only
cwd doesn't pass through and produce a basename like " /path ".
- notification.ts: runtime-guard data.cwd with typeof === "string"
before passing to resolveProject; data is parsed from untyped JSON.
- hook-project.test.ts: replace execSync("mkdir -p ...") with
mkdirSync(sub, { recursive: true }) for platform portability.
efenex
force-pushed
the
fix/hook-project-name
branch
from
3827fc7 to
b243dd1
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (1)
src/hooks/session-start.ts (1)
56-58: ⚡ Quick winDrop the added WHAT-comment block here.
The variable/function naming already communicates this behavior.
As per coding guidelines, "Avoid code comments explaining WHAT — use clear naming instead".
🤖 Prompt for 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. In `@src/hooks/session-start.ts` around lines 56 - 58, Remove the added "WHAT"-style explanatory comment block that describes the project name behavior (the lines mentioning "Project name is the short identifier..." and references to imported sessions, memory_lesson_save, memory_lesson_recall and ./_project.ts); the variable and function names already convey this, so delete that comment from src/hooks/session-start.ts to comply with the guideline "Avoid code comments explaining WHAT — use clear naming instead."
🤖 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 `@src/functions/context.ts`:
- Around line 32-36: projectMatches currently compares raw project and requested
strings (and their basenames) which fails if either has incidental whitespace;
update projectMatches to trim() both inputs before comparison and when calling
basename (i.e., call project = project.trim() after the undefined check and
requested = requested.trim()), then perform equality and basename comparisons on
the trimmed values so leading/trailing spaces do not prevent matches.
In `@src/hooks/post-tool-failure.ts`:
- Around line 37-38: The value assigned to cwd currently trusts (data.cwd as
string) which can be a non-string at runtime and crash resolveProject; before
calling resolveProject, validate that data.cwd is a non-empty string (e.g.,
typeof data.cwd === 'string' and not just whitespace) and only use it when
valid, otherwise fall back to process.cwd(); update the cwd assignment and any
callers of resolveProject to use this validated cwd so resolveProject always
receives a proper string.
In `@src/hooks/post-tool-use.ts`:
- Around line 36-37: data.cwd is untyped JSON and casting with (data.cwd as
string) lets non-string values through, causing resolveProject(cwd) to throw
when it expects a string; fix by validating/coercing cwd before calling
resolveProject: check typeof data.cwd === "string" (and optionally .trim()
non-empty) and fall back to process.cwd() if it isn't a string (or use
String(data.cwd) only if you intentionally want to coerce), then pass that safe
cwd into resolveProject; update the code around the cwd assignment (the cwd
variable and the call to resolveProject) to perform this guard.
In `@src/hooks/session-start.ts`:
- Around line 55-59: Guard against non-string cwd by validating data.cwd before
asserting its type: check typeof data.cwd === "string" and only use it as cwd if
true, otherwise fall back to process.cwd(); then call resolveProject(cwd).
Update references to the resulting variable (project) if needed and remove or
replace the verbose inline comment about project usage with a clearer, concise
name or short summary that encodes intent (e.g., rename variable if helpful) so
the code documents purpose instead of explaining behavior.
In `@src/hooks/subagent-start.ts`:
- Around line 44-45: The code in src/hooks/subagent-start.ts casts data.cwd to
string then calls resolveProject(cwd) which may receive non-strings and crash in
resolveProject (e.g., when it calls .trim()); change the cwd assignment to
validate the input at the boundary: check typeof data.cwd === "string" (and
optionally that it's non-empty after trimming) and only then pass it to
resolveProject, otherwise fall back to process.cwd(); update the variable used
in resolveProject(cwd) to the validated value so resolveProject and the
_project.ts .trim() call always receive a real string.
In `@src/hooks/subagent-stop.ts`:
- Around line 36-37: The current code assigns cwd = (data.cwd as string) ||
process.cwd() which does no runtime type checking; validate data.cwd before
using it by checking typeof data.cwd === "string" and that it's non-empty (e.g.,
trim() produces length > 0), then set cwd to that validated string, otherwise
fall back to process.cwd(); update the assignment near the declaration of cwd in
subagent-stop.ts and ensure resolveProject(cwd) is only called with the
validated cwd to avoid resolveProject calling .trim() on non-strings.
In `@src/hooks/task-completed.ts`:
- Around line 36-37: The code casts data.cwd to string without verifying its
runtime type so non-string truthy values can reach resolveProject and trigger
.trim() errors; update the cwd assignment (used by resolveProject) to first
check typeof data.cwd === "string" and that it's non-empty (optionally .trim()
length > 0) and only then use it, otherwise fallback to process.cwd(); ensure
resolveProject always receives a bona fide string and remove the unsafe cast.
---
Nitpick comments:
In `@src/hooks/session-start.ts`:
- Around line 56-58: Remove the added "WHAT"-style explanatory comment block
that describes the project name behavior (the lines mentioning "Project name is
the short identifier..." and references to imported sessions,
memory_lesson_save, memory_lesson_recall and ./_project.ts); the variable and
function names already convey this, so delete that comment from
src/hooks/session-start.ts to comply with the guideline "Avoid code comments
explaining WHAT — use clear naming instead."
🪄 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
Run ID: bba6b55a-3d96-4e76-af94-7e9454387dfd
📒 Files selected for processing (13)
.env.examplesrc/functions/context.tssrc/hooks/_project.tssrc/hooks/notification.tssrc/hooks/post-tool-failure.tssrc/hooks/post-tool-use.tssrc/hooks/prompt-submit.tssrc/hooks/session-start.tssrc/hooks/subagent-start.tssrc/hooks/subagent-stop.tssrc/hooks/task-completed.tstest/context-lessons.test.tstest/hook-project.test.ts
Comment on lines
+32
to
+36
| function projectMatches(project: string | undefined, requested: string): boolean { | ||
| if (!project) return false; | ||
| if (project === requested) return true; | ||
| return basename(project) === basename(requested); | ||
| } |
Contributor
There was a problem hiding this comment.
Trim project values before matching.
projectMatches compares raw strings and raw basenames. If either side contains incidental whitespace, matching fails and relevant lessons/sessions are skipped.
Proposed fix
function projectMatches(project: string | undefined, requested: string): boolean {
- if (!project) return false;
- if (project === requested) return true;
- return basename(project) === basename(requested);
+ const left = project?.trim();
+ const right = requested.trim();
+ if (!left || !right) return false;
+ if (left === right) return true;
+ return basename(left) === basename(right);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function projectMatches(project: string | undefined, requested: string): boolean { | |
| if (!project) return false; | |
| if (project === requested) return true; | |
| return basename(project) === basename(requested); | |
| } | |
| function projectMatches(project: string | undefined, requested: string): boolean { | |
| const left = project?.trim(); | |
| const right = requested.trim(); | |
| if (!left || !right) return false; | |
| if (left === right) return true; | |
| return basename(left) === basename(right); | |
| } |
🤖 Prompt for 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.
In `@src/functions/context.ts` around lines 32 - 36, projectMatches currently
compares raw project and requested strings (and their basenames) which fails if
either has incidental whitespace; update projectMatches to trim() both inputs
before comparison and when calling basename (i.e., call project = project.trim()
after the undefined check and requested = requested.trim()), then perform
equality and basename comparisons on the trimmed values so leading/trailing
spaces do not prevent matches.
4 tasks
Hook stdin is untyped JSON, so `data.cwd` may arrive as a non-string (object/number/null/undefined) — `cwd?.trim()` would throw on those. Switch the parameter type to `unknown` and check `typeof cwd === "string"` before trimming. Non-string and whitespace-only values now fall through to `process.cwd()` cleanly. Addresses CodeRabbit review on rohitg00#475: - src/hooks/_project.ts:27 (whitespace handling) - src/hooks/{notification,post-tool-failure,post-tool-use,prompt-submit, session-start,subagent-start,subagent-stop,task-completed}.ts (runtime cwd guard at hook layer) — fixed at the resolver layer instead so the check lives in one place. - src/functions/context.ts:36 (projectMatches whitespace) — separate layer, not changed here; existing matching already normalizes basenames via _project. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
efenex
added a commit
to efenex/agentmemory
that referenced
this pull request
May 20, 2026
Two requested tightenings from the maintainer review:
1. Normalize cwd type-narrowing across all hooks. notification.ts
already had the proper guard:
const cwd =
typeof data.cwd === "string" && data.cwd.length > 0
? data.cwd
: process.cwd();
The 7 other hooks (post-tool-failure, post-tool-use, prompt-submit,
task-completed, subagent-start, subagent-stop, session-start) used
the lazier `(data.cwd as string) || process.cwd()`. That compiles
but silently accepts non-string objects via TypeScript's `as`
escape hatch — the resolveProject layer defends against it, but
the cast pattern is sloppy. Match the typeof-narrow pattern so all
hooks read identically.
2. Strengthen test/hook-project.test.ts:
- Same-basename collision: assert that two distinct ancestor paths
ending in the same leaf directory BOTH resolve to that exact
basename, not just to equal-but-arbitrary strings. Covers the
property that makes lessons + sessions cross-reference correctly
across Codex worktrees with shared leaf names.
- Non-string cwd guard: assert resolveProject({}/42/null) falls
back to process.cwd() rather than throwing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two requested tightenings from the maintainer review:
1. Normalize cwd type-narrowing across all hooks. notification.ts
already had the proper guard:
const cwd =
typeof data.cwd === "string" && data.cwd.length > 0
? data.cwd
: process.cwd();
The 7 other hooks (post-tool-failure, post-tool-use, prompt-submit,
task-completed, subagent-start, subagent-stop, session-start) used
the lazier `(data.cwd as string) || process.cwd()`. That compiles
but silently accepts non-string objects via TypeScript's `as`
escape hatch — the resolveProject layer defends against it, but
the cast pattern is sloppy. Match the typeof-narrow pattern so all
hooks read identically.
2. Strengthen test/hook-project.test.ts:
- Same-basename collision: assert that two distinct ancestor paths
ending in the same leaf directory BOTH resolve to that exact
basename, not just to equal-but-arbitrary strings. Covers the
property that makes lessons + sessions cross-reference correctly
across Codex worktrees with shared leaf names.
- Non-string cwd guard: assert resolveProject({}/42/null) falls
back to process.cwd() rather than throwing.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
efenex
force-pushed
the
fix/hook-project-name
branch
from
2119101 to
f9114b3
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/hooks/post-tool-failure.ts`:
- Around line 37-40: The current assignment of cwd accepts whitespace-only
strings; trim data.cwd first and use the trimmed value when non-empty, otherwise
fall back to process.cwd(); specifically, compute a trimmedCwd from data.cwd (if
it's a string), check trimmedCwd.length > 0 and assign cwd = trimmedCwd, else
cwd = process.cwd(), updating the logic around the cwd/data.cwd handling in
post-tool-failure so stored/sent payloads never contain only-whitespace cwd.
In `@src/hooks/session-start.ts`:
- Around line 55-58: The current assignment for cwd treats strings containing
only whitespace as valid; update the logic in session-start (the cwd assignment
using data.cwd) to first trim data.cwd and then use the trimmed value if its
length > 0, otherwise fall back to process.cwd(); ensure you don't pass the raw
data.cwd into sdk.trigger()—use the validated/whitelisted trimmed cwd variable
when emitting events or calling SDKs.
🪄 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
Run ID: f5a105e4-9ac0-42b7-8dd1-48284f9abf2e
📒 Files selected for processing (17)
plugin/scripts/_project--Krf34Q5.mjsplugin/scripts/notification.mjsplugin/scripts/post-tool-failure.mjsplugin/scripts/post-tool-use.mjsplugin/scripts/prompt-submit.mjsplugin/scripts/session-start.mjsplugin/scripts/subagent-start.mjsplugin/scripts/subagent-stop.mjsplugin/scripts/task-completed.mjssrc/hooks/post-tool-failure.tssrc/hooks/post-tool-use.tssrc/hooks/prompt-submit.tssrc/hooks/session-start.tssrc/hooks/subagent-start.tssrc/hooks/subagent-stop.tssrc/hooks/task-completed.tstest/hook-project.test.ts
CodeRabbit's re-review of f9114b3 caught that `length > 0` accepts whitespace-only strings — `data.cwd = " "` passes the guard and gets sent in the /observe payload as-is. The resolveProject layer was already robust (its own typeof+trim guard falls back to process.cwd()), so this only affected the cwd VALUE written to the observation, not the project derivation. But sending malformed values in payloads is still bad form. Switch to "trim first, then check non-empty" across all 8 hooks that use the typeof-narrow pattern (notification, post-tool-failure, post-tool-use, prompt-submit, task-completed, subagent-start, subagent-stop, session-start). The new pattern is: const trimmedCwd = typeof data.cwd === "string" ? data.cwd.trim() : ""; const cwd = trimmedCwd || process.cwd(); Same shape across all hooks — easy to scan. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #473. The Claude Code SessionStart hook (and seven other hooks that POST observations) computed
project = data.cwd || process.cwd()— i.e. the absolute path of the working directory. But every other code path that tags records with a project uses the basename:mem::replay::import-jsonlpreserves whatever the JSONL file contains, which is the basename Claude Code itself stores in transcriptsmemory_lesson_savenaturally pass the short name ("gitops-assistant", not"/Users/me/work/foo/gitops-assistant")Result: the auto-inject context block at session start ran a project-equality filter that matched zero of the bulk of the operator's data. In my repro on a real corpus, a 4,351-lesson / 52-summary store rendered as a 3-block context payload with no Lessons Learned section at all — only session summaries from the rare records tagged with the full cwd (which happen to be the ones the broken hook itself created in earlier sessions).
Filed against #474.
Two-sided fix
A) Hook side — resolve project to basename
Hooks now resolve project via a shared
src/hooks/_project.tshelper. Resolution order:AGENTMEMORY_PROJECT_NAMEenv var — operator escape hatch, settable per-repo via Claude Code's.claude/settings.jsonenvblock (which Claude Code inherits into hook subprocesses)basename(git rev-parse --show-toplevel)— handles sessions started in a subdirectory of the repo, survives moving the repobasename(cwd)— final fallback for non-git directoriesAll eight hooks that POST a project field were updated:
session-start,prompt-submit,post-tool-use,post-tool-failure,notification,subagent-start,subagent-stop,task-completed.B) Server side —
mem::contextbasename fallbackTolerates the mixed-vintage corpus this regression has already produced. Project-match for both lessons and sessions now uses a basename-equality fallback: exact match wins, then
basename(stored) === basename(requested). Means existing records tagged with the full cwd continue to surface for callers passing the basename, and vice versa, without requiring re-import.Documented trade-off
Same-named repos in different paths now collapse to one logical project at retrieval time (e.g.
/work/fooand/personal/foo). Operators who need to keep them separate setAGENTMEMORY_PROJECT_NAMEexplicitly per repo. Behavior is asserted intest/context-lessons.test.tsas a passing test, so any future tightening is a known break, not an accidental one.Tests
17/17 in the directly impacted files. Layout:
test/hook-project.test.ts(new)AGENTMEMORY_PROJECT_NAMEprecedence, whitespace handling, git-toplevel basename, cwd fallback when not in a repo, subdir-of-repo handlingtest/context-lessons.test.tsFiles
src/hooks/_project.ts(new) — sharedresolveProject()helpersrc/hooks/{session-start,prompt-submit,post-tool-use,post-tool-failure,notification,subagent-start,subagent-stop,task-completed}.ts— useresolveProject()src/functions/context.ts—projectMatches()helper, applied to both the lessons filter and the sessions filtertest/hook-project.test.ts(new) — 6 casestest/context-lessons.test.ts— 3 added cases.env.example— documentAGENTMEMORY_PROJECT_NAMEWhy this matters
This is the difference between agentmemory's auto-inject doing something useful for a Claude Code session and silently doing nothing. In my repro the operator had 1,405 lessons in their
gitops-assistantcorpus and the SessionStart hook was filtering all of them out. After this fix, the same store renders a 4,539-token context block with the full Lessons Learned section.Reproduces with: import any JSONL into agentmemory (
mem::replay::import-jsonl), start a new Claude Code session in that project, observe that no## Lessons Learnedblock lands.Summary by CodeRabbit
New Features
Tests