From 6714ae94e89e510e7401bde832c18019009cd19e Mon Sep 17 00:00:00 2001 From: ozymandiashh <234437643+ozymandiashh@users.noreply.github.com> Date: Fri, 17 Jul 2026 03:00:02 +0300 Subject: [PATCH] fix(yield): group attribution by canonical repo root, not path string The single-owner attribution from #692 keyed repo groups by the raw project.projectPath string while getCommitsInRange runs git log --all repo-wide, so a monorepo (sessions launched from different subdirectories) or linked worktrees formed multiple groups that each pulled the identical history and the same commit was credited once per group. - repo groups are keyed by git rev-parse --show-toplevel, resolved once per path and cached - linked worktrees unify through --git-common-dir, with both sides realpath- canonicalized before comparison (symlinks, platform case); when git or realpath fails the session degrades to the existing cwd fallback, and a comment documents the accepted residual for repos moved after worktree add - regression tests: two sessions from two subdirectories of one repo, and a linked-worktree pair - both mutation-checked to fail if the grouping or the common-dir merge is removed; exactly one productive owner and commitCount 1 in each Closes #713 --- src/yield.ts | 70 +++++++++++++++------ tests/yield-overlap-attribution.test.ts | 82 ++++++++++++++++++++++++- 2 files changed, 132 insertions(+), 20 deletions(-) diff --git a/src/yield.ts b/src/yield.ts index eeee66dc..e5b61540 100644 --- a/src/yield.ts +++ b/src/yield.ts @@ -1,4 +1,6 @@ import { execFileSync } from 'child_process' +import { realpathSync } from 'node:fs' +import { resolve } from 'node:path' import { parseAllSessions } from './parser.js' import type { DateRange, SessionSummary } from './types.js' @@ -60,8 +62,31 @@ function runGit(args: string[], cwd: string): string | null { } } -function isGitRepo(dir: string): boolean { - return runGit(['rev-parse', '--is-inside-work-tree'], dir) === 'true' +type RepoResolution = { + readonly root: string + readonly commonDir: string +} + +function canonicalizePath(path: string): string { + try { + return realpathSync(path) + } catch { + return path + } +} + +function resolveRepo(dir: string): RepoResolution | null { + // Accepted residual: moving the main repo after worktree add leaves stale Git metadata; Git fails and sessions fall back to cwd. + const root = runGit(['rev-parse', '--show-toplevel'], dir) + if (!root) return null + + const commonDir = runGit(['rev-parse', '--git-common-dir'], root) + return { + root, + // Linked worktrees have different top-levels but `git log --all` reads + // the same shared repository history, so they must share attribution. + commonDir: canonicalizePath(commonDir ? resolve(root, commonDir) : root), + } } function getMainBranch(cwd: string): string { @@ -252,40 +277,47 @@ export async function computeYield(range: DateRange, cwd: string, provider: stri details: [], } - // Get all commits in the date range for correlation - const commits = isGitRepo(cwd) - ? getCommitsInRange(cwd, range.start, range.end, getMainBranch(cwd)) - : [] + // Resolve each recorded path once. Multiple project entries can carry the + // same path, and each uncached lookup would otherwise spawn git again. + const repoResolutionCache = new Map() + const resolveRepoCached = (dir: string): RepoResolution | null => { + if (!repoResolutionCache.has(dir)) repoResolutionCache.set(dir, resolveRepo(dir)) + return repoResolutionCache.get(dir) ?? null + } + const cwdRepo = resolveRepoCached(cwd) - // Group sessions by resolved repo before attributing: every project entry - // whose projectPath is missing (or not a git repo) falls back to the same - // cwd and shares one commit list, so attribution must run once per repo or - // two such entries could each award the same commit to one of their sessions. + // Canonical top-levels are the primary keys. A secondary common-dir index + // aliases linked worktree roots to one group because --all sees the same + // shared history from each worktree. Missing/non-git paths still use cwd. type RepoGroup = { commits: CommitInfo[]; sessions: SessionSummary[]; projectNames: string[] } const repoGroups = new Map() + const commonDirGroups = new Map() + const uniqueRepoGroups = new Set() for (const project of projects) { - const projectCwd = project.projectPath && isGitRepo(project.projectPath) - ? project.projectPath - : cwd + const repo = (project.projectPath && resolveRepoCached(project.projectPath)) || cwdRepo + const projectCwd = repo?.root ?? cwd + const groupKey = repo?.root ?? cwd - let group = repoGroups.get(projectCwd) + let group = repoGroups.get(groupKey) ?? (repo ? commonDirGroups.get(repo.commonDir) : undefined) if (!group) { group = { - commits: projectCwd === cwd - ? commits - : getCommitsInRange(projectCwd, range.start, range.end, getMainBranch(projectCwd)), + commits: repo + ? getCommitsInRange(projectCwd, range.start, range.end, getMainBranch(projectCwd)) + : [], sessions: [], projectNames: [], } - repoGroups.set(projectCwd, group) + uniqueRepoGroups.add(group) } + repoGroups.set(groupKey, group) + if (repo) commonDirGroups.set(repo.commonDir, group) for (const session of project.sessions) { group.sessions.push(session) group.projectNames.push(project.project) } } - for (const group of repoGroups.values()) { + for (const group of uniqueRepoGroups) { const attributions = attributeCommits(group.sessions, group.commits) for (const [index, session] of group.sessions.entries()) { const attribution = attributions[index] diff --git a/tests/yield-overlap-attribution.test.ts b/tests/yield-overlap-attribution.test.ts index f1dc37df..518f656b 100644 --- a/tests/yield-overlap-attribution.test.ts +++ b/tests/yield-overlap-attribution.test.ts @@ -1,5 +1,5 @@ import { execFileSync } from 'node:child_process' -import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' @@ -52,12 +52,16 @@ function makeSession(overrides: Partial): SessionSummary { describe('yield attribution for overlapping sessions (issue #641)', () => { let repoDir: string + let worktreeDir: string + let worktreeParentDir: string beforeAll(async () => { repoDir = await mkdtemp(join(tmpdir(), 'codeburn-yield-overlap-')) git(repoDir, ['init', '-b', 'main']) git(repoDir, ['config', 'user.email', 'test@example.com']) git(repoDir, ['config', 'user.name', 'Test']) + await mkdir(join(repoDir, 'packages', 'a'), { recursive: true }) + await mkdir(join(repoDir, 'packages', 'b'), { recursive: true }) await writeFile(join(repoDir, 'file.txt'), 'hello\n') git(repoDir, ['add', '.']) // One commit on main at 10:30, inside both sessions' time windows. @@ -65,9 +69,15 @@ describe('yield attribution for overlapping sessions (issue #641)', () => { GIT_AUTHOR_DATE: '2026-01-01T10:30:00Z', GIT_COMMITTER_DATE: '2026-01-01T10:30:00Z', }) + worktreeParentDir = await mkdtemp(join(tmpdir(), 'codeburn-yield-worktree-')) + worktreeDir = join(worktreeParentDir, 'linked') + git(repoDir, ['worktree', 'add', '--detach', worktreeDir]) + await mkdir(join(worktreeDir, 'packages', 'linked'), { recursive: true }) }) afterAll(async () => { + git(repoDir, ['worktree', 'remove', '--force', worktreeDir]) + await rm(worktreeParentDir, { recursive: true, force: true }) await rm(repoDir, { recursive: true, force: true }) }) @@ -195,4 +205,74 @@ describe('yield attribution for overlapping sessions (issue #641)', () => { expect(productive[0]!.commitCount).toBe(1) expect(summary.details.find(d => d.sessionId === 'proj2-session')!.category).toBe('ambiguous') }) + + it('credits one owner across project paths inside the same repository (issue #713)', async () => { + const sessionA = makeSession({ + sessionId: 'monorepo-a', + project: 'package-a', + firstTimestamp: '2026-01-01T10:15:00.000Z', + lastTimestamp: '2026-01-01T10:45:00.000Z', + totalCostUSD: 2, + }) + const sessionB = makeSession({ + sessionId: 'monorepo-b', + project: 'package-b', + firstTimestamp: '2026-01-01T10:00:00.000Z', + lastTimestamp: '2026-01-01T11:00:00.000Z', + totalCostUSD: 3, + }) + parseAllSessionsMock.mockResolvedValue([ + { project: 'package-a', projectPath: join(repoDir, 'packages', 'a'), sessions: [sessionA] } as ProjectSummary, + { project: 'package-b', projectPath: join(repoDir, 'packages', 'b'), sessions: [sessionB] } as ProjectSummary, + ]) + + const summary = await computeYield( + { start: new Date('2026-01-01T00:00:00.000Z'), end: new Date('2026-01-02T00:00:00.000Z') }, + repoDir, + ) + + const productive = summary.details.filter(detail => detail.category === 'productive') + expect(productive.map(detail => detail.sessionId)).toEqual(['monorepo-a']) + expect(productive[0]!.commitCount).toBe(1) + expect(summary.details.find(detail => detail.sessionId === 'monorepo-b')).toMatchObject({ + category: 'ambiguous', + commitCount: 0, + }) + expect(summary.productive.sessions).toBe(1) + }) + + it('credits one owner across a main-repo path and linked worktree (issue #713)', async () => { + const mainSession = makeSession({ + sessionId: 'worktree-main', + project: 'main-package', + firstTimestamp: '2026-01-01T10:15:00.000Z', + lastTimestamp: '2026-01-01T10:45:00.000Z', + totalCostUSD: 2, + }) + const linkedSession = makeSession({ + sessionId: 'worktree-linked', + project: 'linked-package', + firstTimestamp: '2026-01-01T10:00:00.000Z', + lastTimestamp: '2026-01-01T11:00:00.000Z', + totalCostUSD: 3, + }) + parseAllSessionsMock.mockResolvedValue([ + { project: 'main-package', projectPath: join(repoDir, 'packages', 'a'), sessions: [mainSession] } as ProjectSummary, + { project: 'linked-package', projectPath: join(worktreeDir, 'packages', 'linked'), sessions: [linkedSession] } as ProjectSummary, + ]) + + const summary = await computeYield( + { start: new Date('2026-01-01T00:00:00.000Z'), end: new Date('2026-01-02T00:00:00.000Z') }, + repoDir, + ) + + const productive = summary.details.filter(detail => detail.category === 'productive') + expect(productive.map(detail => detail.sessionId)).toEqual(['worktree-main']) + expect(productive[0]!.commitCount).toBe(1) + expect(summary.details.find(detail => detail.sessionId === 'worktree-linked')).toMatchObject({ + category: 'ambiguous', + commitCount: 0, + }) + expect(summary.productive.sessions).toBe(1) + }) })