fix: use parent project name for worktree observation writes#1820
Merged
thedotmack merged 3 commits intothedotmack:mainfrom Apr 15, 2026
Merged
Conversation
…ack#1819) Observations and sessions from git worktrees were stored under basename(cwd) instead of the parent repo name because write paths called getProjectName() (not worktree-aware) instead of getProjectContext() (worktree-aware). This is the same bug as thedotmack#1081, thedotmack#1317, and thedotmack#1500 — it regressed because the two functions coexist and new code reached for the simpler one. Fix: getProjectContext() now returns parentProjectName as primary when in a worktree, and all four write-path call sites now use getProjectContext().primary instead of getProjectName(). Includes regression test that creates a real worktree directory structure and asserts primary === parentProjectName.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Summary by CodeRabbit
WalkthroughThe PR standardizes project resolution to use Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (session-init)
participant Context as ContextService
participant API as Server (SessionRoutes)
participant DB as Observations DB
CLI->>Context: getProjectContext(cwd)
Context-->>CLI: { primary, parent, allProjects }
CLI->>API: POST /api/sessions/init (project=primary)
API->>Context: getProjectContext(cwd)
Context-->>API: { primary, parent, allProjects }
API->>DB: createSDKSession(contentSessionId, project=primary) / queueObservation(project=primary,...)
DB-->>API: ack
API-->>CLI: session init response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (3)
src/services/context/ContextBuilder.ts (1)
132-136: Consider defaulting tocontext.allProjectswheninput.projectsis not provided.With Line 132 now resolving to parent repo in worktrees, using
context.allProjectsas the default fallback can keep context reads compatible with legacy worktree-labeled records during transition.♻️ Suggested adjustment
- const project = getProjectContext(cwd).primary; + const context = getProjectContext(cwd); + const project = context.primary; const platformSource = input?.platform_source; // Use provided projects array (for worktree support) or fall back to single project - const projects = input?.projects || [project]; + const projects = input?.projects ?? context.allProjects;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/context/ContextBuilder.ts` around lines 132 - 136, Change the fallback for projects so that when input?.projects is undefined you default to using context.allProjects rather than [project]; update the assignment that currently builds projects (where getProjectContext(cwd).primary and platformSource are used) to prefer input.projects and otherwise use context.allProjects to preserve compatibility with legacy worktree-labeled records; ensure the symbol names involved are getProjectContext, project, projects, input.projects and context.allProjects so the change is applied in the same expression that constructs the projects array.src/utils/project-name.ts (1)
91-95: RefreshProjectContextdocs to match the new canonicalprimarybehavior.Line 94 now sets
primaryto the parent repo in worktrees, but the interface/JSDoc above still read likeprimarymay be the worktree name. Tightening those comments will prevent future caller misuse.📝 Suggested doc-only update
export interface ProjectContext { - /** The current project name (worktree or main repo) */ + /** Canonical project name for writes/primary queries (parent repo in worktrees) */ primary: string; /** Parent project name if in a worktree, null otherwise */ parent: string | null; /** True if currently in a worktree */ isWorktree: boolean; - /** All projects to query: [primary] for main repo, [parent, primary] for worktree */ + /** All projects to query: [primary] for main repo, [parentRepo, worktreeName] for worktree */ allProjects: string[]; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/project-name.ts` around lines 91 - 95, Update the ProjectContext JSDoc/interface to reflect that in worktrees the canonical "primary" is the parent repository name (not the worktree name): modify the comment for ProjectContext and the "primary" field to state it will be the parent project when worktreeInfo is present, and clarify "parent" remains the parent project name as well; locate the ProjectContext declaration/JSDoc near the top of src/utils/project-name.ts and update wording so callers understand primary references the main repo in worktree scenarios (mention worktreeInfo.parentProjectName and the functions that return it, e.g., where primary is assigned).tests/utils/project-name.test.ts (1)
100-133: Add one integration regression test to protect write-path call sites.This test validates resolver behavior well, but the historical regressions came from write callers using the wrong helper. Add one integration test that performs a worktree write flow (e.g.,
/api/sessions/initor observation path) and asserts the persistedprojectis the parent repo name.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/services/context/ContextBuilder.ts`:
- Around line 132-136: Change the fallback for projects so that when
input?.projects is undefined you default to using context.allProjects rather
than [project]; update the assignment that currently builds projects (where
getProjectContext(cwd).primary and platformSource are used) to prefer
input.projects and otherwise use context.allProjects to preserve compatibility
with legacy worktree-labeled records; ensure the symbol names involved are
getProjectContext, project, projects, input.projects and context.allProjects so
the change is applied in the same expression that constructs the projects array.
In `@src/utils/project-name.ts`:
- Around line 91-95: Update the ProjectContext JSDoc/interface to reflect that
in worktrees the canonical "primary" is the parent repository name (not the
worktree name): modify the comment for ProjectContext and the "primary" field to
state it will be the parent project when worktreeInfo is present, and clarify
"parent" remains the parent project name as well; locate the ProjectContext
declaration/JSDoc near the top of src/utils/project-name.ts and update wording
so callers understand primary references the main repo in worktree scenarios
(mention worktreeInfo.parentProjectName and the functions that return it, e.g.,
where primary is assigned).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 984c7a38-6477-437b-84a9-9620bca61017
📒 Files selected for processing (6)
src/cli/handlers/session-init.tssrc/services/context/ContextBuilder.tssrc/services/transcripts/processor.tssrc/services/worker/http/routes/SessionRoutes.tssrc/utils/project-name.tstests/utils/project-name.test.ts
…h test - ContextBuilder: default projects to context.allProjects for legacy worktree-labeled record compatibility - ProjectContext: clarify JSDoc that primary is canonical (parent repo in worktrees) - Tests: add write-path regression test mirroring session-init/SessionRoutes pattern; refactor worktree fixture into beforeAll/afterAll
Contributor
Author
|
Addressed all three nitpicks in 2089c75:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/utils/project-name.ts (1)
91-98: Consider a small readability hardening in the worktree return path.This is functionally correct, but renaming the local
primary(Line 81) tocwdProjectNameand dedupingallProjectswould make future regressions less likely.♻️ Proposed refactor
export function getProjectContext(cwd: string | null | undefined): ProjectContext { - const primary = getProjectName(cwd); + const cwdProjectName = getProjectName(cwd); if (!cwd) { - return { primary, parent: null, isWorktree: false, allProjects: [primary] }; + return { primary: cwdProjectName, parent: null, isWorktree: false, allProjects: [cwdProjectName] }; } @@ if (worktreeInfo.isWorktree && worktreeInfo.parentProjectName) { @@ + const allProjects = Array.from(new Set([worktreeInfo.parentProjectName, cwdProjectName])); return { primary: worktreeInfo.parentProjectName, parent: worktreeInfo.parentProjectName, isWorktree: true, - allProjects: [worktreeInfo.parentProjectName, primary] + allProjects }; } - return { primary, parent: null, isWorktree: false, allProjects: [primary] }; + return { primary: cwdProjectName, parent: null, isWorktree: false, allProjects: [cwdProjectName] }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/project-name.ts` around lines 91 - 98, The worktree return uses a local variable named primary which is confusing; rename that local variable to cwdProjectName (the value coming from the current working dir) and update the return to use cwdProjectName for the cwd-specific slot and worktreeInfo.parentProjectName for the parent slot, then dedupe the allProjects array (e.g., build allProjects from [worktreeInfo.parentProjectName, cwdProjectName] and remove duplicates) so duplicate entries are not returned; update references to primary in this function accordingly (look for primary and worktreeInfo in this module).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/utils/project-name.ts`:
- Around line 91-98: The worktree return uses a local variable named primary
which is confusing; rename that local variable to cwdProjectName (the value
coming from the current working dir) and update the return to use cwdProjectName
for the cwd-specific slot and worktreeInfo.parentProjectName for the parent
slot, then dedupe the allProjects array (e.g., build allProjects from
[worktreeInfo.parentProjectName, cwdProjectName] and remove duplicates) so
duplicate entries are not returned; update references to primary in this
function accordingly (look for primary and worktreeInfo in this module).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e6abbe31-075c-499a-8330-19bbc7543de4
📒 Files selected for processing (3)
src/services/context/ContextBuilder.tssrc/utils/project-name.tstests/utils/project-name.test.ts
✅ Files skipped from review due to trivial changes (1)
- tests/utils/project-name.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/services/context/ContextBuilder.ts
…Projects Addresses final CodeRabbit nitpick: disambiguates the local variable from the returned `primary` field, and dedupes allProjects via Set in case parent and cwd resolve to the same name.
thedotmack
added a commit
that referenced
this pull request
Apr 16, 2026
…t cross worktrees Revert of #1820 behavior. Each worktree now gets its own bucket: - In a worktree, primary = `parent/worktree` (e.g. `claude-mem/dar-es-salaam`) - In a main repo, primary = basename (unchanged) - allProjects is always `[primary]` — strict isolation at query time Includes a one-off maintenance script (scripts/worktree-remap.ts) that retroactively reattributes past sessions to their worktree using path signals in observations and user prompts. Two-rule inference keeps the remap high-confidence: 1. The worktree basename in the path matches the session's current plain project name (pre-#1820 era; trusted). 2. Or all worktree path signals converge on a single (parent, worktree) across the session. Ambiguous sessions are skipped. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced Apr 16, 2026
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.
Fixes #1819
Summary
getProjectContext().primarynow returnsparentProjectNamewhen in a worktree (instead ofbasename(cwd))getProjectName()togetProjectContext().primaryprimary === parentProjectNameProblem
Observations and sessions from git worktrees are stored under
basename(cwd)(e.g.modest-burnell) instead of the parent repo name (e.g.my-app). This is the same root cause as #1081, #1317, and #1500 — it regresses becausegetProjectName()(not worktree-aware) andgetProjectContext()(worktree-aware) coexist, and write paths use the wrong one.Root Cause
getProjectName(cwd)getProjectName(cwd)getProjectContext(cwd)Changes
src/utils/project-name.ts—getProjectContext()now setsprimary: worktreeInfo.parentProjectNamewhen a worktree is detected, instead ofprimary: basename(cwd).4 call site migrations (all
getProjectName(cwd)→getProjectContext(cwd).primary):src/cli/handlers/session-init.ts— session init hooksrc/services/worker/http/routes/SessionRoutes.ts— observation handlersrc/services/context/ContextBuilder.ts— context generationsrc/services/transcripts/processor.ts— transcript processingtests/utils/project-name.test.ts— regression test that creates a temporary worktree directory structure (.gitfile withgitdir:pointer) and asserts:ctx.isWorktree === truectx.primary === 'main-repo'(parent name, not worktree name)ctx.allProjects === ['main-repo', 'my-worktree']Why this keeps regressing
Each prior fix patched individual call sites, but
getProjectName()still exists as the simpler, easier-to-reach function. New code naturally gravitates toward it. This PR addresses the core issue:getProjectContext().primarynow returns the correct name for worktrees, so even if callers use either function path, the worktree-aware one gives the right answer. The regression test provides a safety net against future changes.Test plan
bun test tests/utils/project-name.test.ts— 16/16 pass (including new worktree regression test)