Skip to content

fix(stats): split Windows cwds on the backslash in attributeRepo - #858

Merged
jeff-r2026 merged 2 commits into
Tencent:mainfrom
ydflow:fix/repo-attribution-windows-paths
Sep 28, 2026
Merged

jeff-r2026 merged 2 commits into
Tencent:mainfrom
ydflow:fix/repo-attribution-windows-paths

Conversation

@ydflow

@ydflow ydflow commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Summary

attributeRepo (src/utils/repo-attribution.ts) split a filesystem cwd on / only, so on Windows a path never got past its first segment:

cwd before after
C:\Users\dev\work\teamai-cli C:\Users\dev\work\teamai-cli teamai-cli
D:\src\teamai-cli D:\src\teamai-cli teamai-cli
\srv\share\new-api \srv\share\new-api new-api
C:\Users\dev\home C:\Users\dev\home no_repo
C:\src\data C:\src\data no_repo
C:\ C: no_repo

Two consequences on Windows:

  • stats --by-repo, the dashboard and session save labelled every session with its whole absolute path, so no two sessions ever shared a repo row and the per-project breakdown was unusable.
  • NON_REPO_LEAVES (home, root, users, tmp, workspace, srv, mnt, data, opt) never fired, because the "leaf" was the entire path. A session in C:\src\data opened its own row where /src/data merges into no_repo.

repoLabel masked most of this on Windows — its repo-directory branch calls repoName, which goes through path.basename and handles both separators. The gap is visible wherever the path is not a live repo directory: a worktree removed after the fact (#810), a session recorded outside git, or a dashboard event whose anchor no longer exists. A drive root (C:\) also left the bare drive letter as the leaf, where / on POSIX yields no_repo.

This also fixes a pre-existing Windows failure in the same suite: repoLabel's "names a repo after its directory even when that is a word like workspace" test builds its directories with path.join, so on Windows they reach attributeRepo with backslashes and ...\plain\data came back as data instead of no_repo.

Changes

1. attributeRepo splits on both separators (src/utils/repo-attribution.ts)

-  const segs = raw.replace(/\/+$/, '').split('/').filter(Boolean);
+  const segs = raw.replace(/[/\]+$/, '').split(/[/\]/).filter(Boolean);
   if (segs.length === 0) return 'no_repo';
   const leaf = segs[segs.length - 1];
+  // A drive root (`C:\`) leaves the bare drive letter as the leaf; it names no
+  // project, like `/` on POSIX.
+  if (/^[a-z]:$/i.test(leaf)) return 'no_repo';
   if (NON_REPO_LEAVES.has(leaf.toLowerCase())) return 'no_repo';
   return leaf;

canonicalRepo is untouched: its input is a remote form (github.com/o/r, git@host:o/r), which never contains a backslash, and splitting one on the backslash would only change a value that cannot occur.

2. Retry the #810 e2e sandbox removal (src/__tests__/e2e/deleted-worktree-scope-810.test.ts)

The E2E (fork-safe, no credentials) check was failing this suite on Linux:

 FAIL  src/__tests__/e2e/deleted-worktree-scope-810.test.ts > hook events after their worktree is deleted (#810)
Error: ENOTEMPTY: directory not empty, rmdir '/tmp/teamai-issue810-e2e-BAysX3/home/.teamai'
 Test Files  1 failed | 56 passed | 3 skipped (60)
      Tests  356 passed | 26 skipped (382)

All 12 tests passed; the suite-level failure is its afterAll. Every hook here goes through hook-dispatch, which spawns a detached child for the background-only handlers (SessionEnd, Stop) — spawnPlainDetached in src/hook-dispatch-cli.ts. The suite waits for the events that child writes and never for the child to exit, so the removal runs while it may still be creating a file under .teamai, and one rmdir loses the race.

-  const cleanup = () => fs.rmSync(sandbox, { recursive: true, force: true });
+  const cleanup = () => fs.rmSync(sandbox, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 });

This is the retry git-kind-learnings.test.ts:33 already uses for the same git gc --auto race, documented there in a comment. Node's rm retries EBUSY, EMFILE, ENFILE, ENOTEMPTY and EPERM with a linear backoff, which covers the Windows EPERM this suite also hits locally.

This second change is independent of the first: the suite never calls attributeRepo, and the diff is an identity transform on every POSIX-shaped cwd it produces — verified over 35 inputs, including each teamai-issue810-e2e-* path the suite builds.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • This change requires a documentation update

No signature or output-contract change on POSIX: the existing attributeRepo expectations (/home/u/new-api, /opt/teamai-cli/, /home, /root, /opt, '', undefined) are unchanged and still pass. No docs describe the --by-repo label rule, so there is no wording to sync; CHANGELOG.md is generated by standard-version from commit messages.

Test Plan

  • npx tsc --noEmit passes (exit 0)
  • npm run lint passes (exit 0)
  • npx vitest run — affected files pass (see below)
  • Added tests for the change

New tests, red on origin/main (a8ab8e0), verified by copying only the test file onto a clean origin/main checkout and re-running:

attributeRepo   splits Windows paths on the backslash too                                     red on main
attributeRepo   maps a Windows path whose leaf is not a project to no_repo                    red on main
attributeByRepo labels Windows cwds by their project directory, non-project dirs → no_repo    red on main
repoLabel        names a repo after its directory even when that is a word like workspace      red on main  (pre-existing Windows failure, same root cause)

npx vitest run src/__tests__/session-analytics.test.ts src/__tests__/session-collector.test.ts src/__tests__/dashboard-collector.test.ts src/__tests__/dashboard-ui.test.ts src/__tests__/session-trends.test.ts: 5 files, exit 0, all passed (was 1 failed / 7 passed on origin/main for the same set, the failure being the repoLabel one above).

End-to-end (real CLI): npm run build, then npx vitest run --config vitest.e2e.config.ts src/__tests__/e2e/repo-attribution-worktrees-809.test.ts — 6 passed (15.5 s), covering stats --by-repo showing one row per repo across worktrees, labelling two same-named repos apart, session save recording the repo as Project, import --dir / codebase --extract wiki slugs, and a removed worktree's sessions staying in the repo's dashboard workspace.

The #810 suite, before and after the retry, on this Windows host:

origin/main this branch
suite result 12 tests | 1 failed | 11 skipped 12 tests | 1 failed | 11 skipped (identical)
EPERM on cleanup 5 5

The remaining failure is a pre-existing Windows-only issue, unchanged by this PR: git worktree remove --force raises EPERM on this host and CI on Linux never hits it. It reproduces identically on a clean origin/main checkout. It persists after the retry because it is a persistent condition, not a transient one — the retry covers the transient ENOTEMPTY CI actually reported, and does not make the Windows case worse.

Not verified on this machine: the full npx vitest run, and the E2E job's Linux behaviour end to end. On Windows the vitest/tinypool worker pool dies with ERR_IPC_CHANNEL_CLOSED part-way through (reproduced on a clean origin/main checkout too, so it predates this branch), and dashboard-report-scope.test.ts cannot complete even alone on origin/main (timed out at 110 s). CI runs the full suite on Linux; the files that consume attributeRepo are covered individually above.

`attributeRepo` split a filesystem cwd on `/` only, so on Windows every
`cwd` stayed one segment: `C:\Users\dev\work\teamai-cli` was attributed to
a repo literally named `C:\Users\dev\work\teamai-cli` instead of
`teamai-cli`. The `NON_REPO_LEAVES` check (`home`, `data`, `tmp`, ...)
never fired either, so a session started in `C:\src\data` opened its own
row instead of merging into `no_repo` like `/src/data` does.

`repoLabel` masked most of it on Windows, because `path.basename` handles
both separators and the repo-directory branch of `nameOf` runs first. The
gap shows wherever the path is not a live repo directory: a worktree
removed after the fact (Tencent#810), a session recorded outside git, or a
dashboard event whose anchor no longer exists. A drive root (`C:\`) also
left the bare drive letter as the leaf, where `/` on POSIX yields
`no_repo`.

Split on both separators and treat a bare drive letter as no project, so
Windows cwds attribute the way POSIX ones already do. This also fixes
`repoLabel`'s "a directory named like workspace is still no_repo" test on
Windows, which failed on `origin/main` for the same reason.
@jeff-r2026 jeff-r2026 self-assigned this Sep 27, 2026
@github-actions

Copy link
Copy Markdown
  • [P3 nit] Preserve valid POSIX backslashes — src/utils/repo-attribution.ts:61 unconditionally treats \ as a separator. On POSIX, a cwd such as /tmp/acme\backend is valid and should retain basename acme\backend, but now becomes backend, potentially merging unrelated repositories. Apply backslash splitting only to Windows-shaped drive/UNC paths.

The PR description includes a representative real-CLI end-to-end verification, so its testing record is sufficient.

… child

`E2E (fork-safe, no credentials)` fails this suite on Linux with

    Error: ENOTEMPTY: directory not empty, rmdir '/tmp/teamai-issue810-e2e-*/home/.teamai'

after all 12 tests pass, so the run reports `1 failed | 56 passed` suites with
`356 passed | 26 skipped` tests and exit code 1.

Every hook here goes through `hook-dispatch`, which spawns a detached child for
the background-only handlers (SessionEnd, Stop) — `spawnPlainDetached` in
src/hook-dispatch-cli.ts. The suite waits for the events that child writes, and
never for the child itself to exit, so `afterAll`'s removal runs while it may
still be creating a file under `.teamai`. One `rmdir` then loses the race and
the suite is reported failed with nothing actually wrong.

Add the retry options `git-kind-learnings.test.ts` already uses for the same
`git gc --auto` race. Node's rm retries EBUSY, EMFILE, ENFILE, ENOTEMPTY and
EPERM with a linear backoff, which covers both this and the Windows `EPERM`
this suite also hits locally.

Unrelated to the attributeRepo change in the parent commit: the suite never
calls it, and the diff is an identity transform on every POSIX-shaped cwd the
suite produces (verified over 35 inputs, including each `teamai-issue810-e2e-*`
path). The Windows `EPERM` failure is pre-existing — it reproduces identically
on a clean origin/main checkout — and persists here because it is a persistent
condition, not a transient one; this change does not make it worse.
@github-actions

Copy link
Copy Markdown
  • [P2 non-blocking] Remove the unrelated teardown hardening — src/__tests__/e2e/deleted-worktree-scope-810.test.ts:227 changes a separate E2E suite that never exercises attributeRepo. The review rules require surgical changes and explicitly flag unrelated drive-by edits; this should be a separate PR.
  • [P3 nit] The earlier POSIX-backslash finding remains unresolved — src/utils/repo-attribution.ts:61 treats every \ as a separator. On POSIX, /tmp/acme\backend is a valid path whose project name is acme\backend, but this returns backend, potentially merging it with /tmp/backend. Split on backslashes only for Windows-shaped drive or UNC paths.

The PR description includes a representative real-CLI end-to-end verification, so its testing record is sufficient.

@jeff-r2026
jeff-r2026 merged commit e936870 into Tencent:main Sep 28, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants