make test silently runs the suite N+1 times when a git worktree exists inside the checkout #25

Open
opened 2026-08-09 08:04:44 +02:00 by clawbot · 1 comment
Collaborator

Problem

Found by accident while verifying main immediately after merging #23. make check reported:

Test Files  90 passed (90)
     Tests  1050 passed (1050)

The real suite is 18 files and 210 tests. There were four git worktrees under
.claude/worktrees/, each a full checkout of this repo. 18 + (4 x 18) = 90, and
210 + (4 x 210) = 1050. vitest had globbed every worktree and run the entire suite five times.

vitest's default exclude covers node_modules, dist, .idea, .git and .cache. It does
not cover .claude/, and there is no vitest.config.ts in this repo to add it. .gitignore
lists .claude/, but vitest does not consult .gitignore for test discovery, so ignoring it for
version control does nothing here.

Why this matters more than it looks

  1. It silently invalidates timing measurements, and this repo gates on timing. script/test
    hard-caps at 30 seconds and the README claims a 20-second budget. A polluted run inflates the
    number by a factor of N+1, so a suite that is actually fine can blow the cap and fail the
    build, and a genuine regression can hide inside noise. A branch on this repo has already
    failed once by exceeding that cap.
  2. A stale worktree can fail make check on a clean main. The worktrees present during that
    run happened to hold passing code. Had any held a broken or mid-rework tree, make check would
    have failed on main for reasons entirely absent from main, and the obvious next move —
    re-running it — would not have explained anything.
  3. It is invisible. Nothing in the output says "these tests came from somewhere else". The
    only symptom is a test count that a reader has to already know is wrong.
  4. It affects exactly the workflow this repo uses, where implementers and reviewers work in
    temporary worktrees created inside the checkout.

Definition of done

  1. make test discovers only the tests in the checkout it was invoked from, regardless of how
    many worktrees or nested checkouts exist beneath it. A vitest.config.ts adding .claude/**
    (and any other nested-checkout location) to exclude is the obvious fix; prefer excluding by
    an explicit list over anything clever.
  2. A test or check that fails if the exclusion is removed — this repo has a standing problem with
    guarantees that nothing enforces, so demonstrate it rather than assert it. Creating a throwaway
    nested directory containing a test file and showing the suite count does not change is
    sufficient.
  3. make lint and make fmt-check are verified against the same hazard and fixed if they share
    it. Prettier 3 does read .gitignore by default so it is likely already safe, but confirm
    rather than assume; eslint's flat config needs checking on its own terms.
  4. The observed counts before and after are recorded in the PR body.

Suggested addition

make clean currently removes build output only. Consider having it also prune worktrees under
.claude/worktrees/, or add a separate target, so the cleanup is a documented operation rather
than tribal knowledge.

Not a 1.0.0 blocker

It does not affect shipped behaviour — it affects the reliability of the gate. Worth doing early
regardless, because every measurement taken while it is unfixed is suspect.

## Problem Found by accident while verifying `main` immediately after merging #23. `make check` reported: ``` Test Files 90 passed (90) Tests 1050 passed (1050) ``` The real suite is 18 files and 210 tests. There were four git worktrees under `.claude/worktrees/`, each a full checkout of this repo. 18 + (4 x 18) = 90, and 210 + (4 x 210) = 1050. vitest had globbed every worktree and run the entire suite five times. vitest's default `exclude` covers `node_modules`, `dist`, `.idea`, `.git` and `.cache`. It does **not** cover `.claude/`, and there is no `vitest.config.ts` in this repo to add it. `.gitignore` lists `.claude/`, but vitest does not consult `.gitignore` for test discovery, so ignoring it for version control does nothing here. ## Why this matters more than it looks 1. **It silently invalidates timing measurements**, and this repo gates on timing. `script/test` hard-caps at 30 seconds and the README claims a 20-second budget. A polluted run inflates the number by a factor of N+1, so a suite that is actually fine can blow the cap and fail the build, and a genuine regression can hide inside noise. A branch on this repo has already failed once by exceeding that cap. 2. **A stale worktree can fail `make check` on a clean `main`.** The worktrees present during that run happened to hold passing code. Had any held a broken or mid-rework tree, `make check` would have failed on `main` for reasons entirely absent from `main`, and the obvious next move — re-running it — would not have explained anything. 3. **It is invisible.** Nothing in the output says "these tests came from somewhere else". The only symptom is a test count that a reader has to already know is wrong. 4. It affects exactly the workflow this repo uses, where implementers and reviewers work in temporary worktrees created inside the checkout. ## Definition of done 1. `make test` discovers only the tests in the checkout it was invoked from, regardless of how many worktrees or nested checkouts exist beneath it. A `vitest.config.ts` adding `.claude/**` (and any other nested-checkout location) to `exclude` is the obvious fix; prefer excluding by an explicit list over anything clever. 2. A test or check that fails if the exclusion is removed — this repo has a standing problem with guarantees that nothing enforces, so demonstrate it rather than assert it. Creating a throwaway nested directory containing a test file and showing the suite count does not change is sufficient. 3. `make lint` and `make fmt-check` are verified against the same hazard and fixed if they share it. Prettier 3 does read `.gitignore` by default so it is likely already safe, but confirm rather than assume; eslint's flat config needs checking on its own terms. 4. The observed counts before and after are recorded in the PR body. ## Suggested addition `make clean` currently removes build output only. Consider having it also prune worktrees under `.claude/worktrees/`, or add a separate target, so the cleanup is a documented operation rather than tribal knowledge. ## Not a 1.0.0 blocker It does not affect shipped behaviour — it affects the reliability of the gate. Worth doing early regardless, because every measurement taken while it is unfixed is suspect.
clawbot self-assigned this 2026-08-09 08:04:44 +02:00
Author
Collaborator

Still live, and now confirmed by direct reproduction rather than by accident. During review of #31 a nested tree was planted under .claude/worktrees/ containing real test files: host make test picked it up (23 files instead of 22, exit 2).

Two things that change the shape of this issue:

  • The lint half is now closed. #30 moved eslint and prettier into a container whose build context excludes .claude/, and the same planted tree provably did not reach them. Point 3 of the definition of done above is therefore satisfied for lint and fmt-check — prettier 3 reads .gitignore and .dockerignore mirrors it, so host and container agree. What remains is the test path only.
  • CI is not exposed; only a developer's local make check is. That lowers the urgency but not the value, since local runs are exactly where the misleading measurements get taken.
Still live, and now confirmed by direct reproduction rather than by accident. During review of https://git.eeqj.de/sneak/quak/pulls/31 a nested tree was planted under `.claude/worktrees/` containing real test files: host `make test` picked it up (23 files instead of 22, exit 2). Two things that change the shape of this issue: - The **lint half is now closed**. https://git.eeqj.de/sneak/quak/issues/30 moved eslint and prettier into a container whose build context excludes `.claude/`, and the same planted tree provably did not reach them. Point 3 of the definition of done above is therefore satisfied for lint and fmt-check — prettier 3 reads `.gitignore` and `.dockerignore` mirrors it, so host and container agree. What remains is the **test** path only. - **CI is not exposed**; only a developer's local `make check` is. That lowers the urgency but not the value, since local runs are exactly where the misleading measurements get taken.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/quak#25