Skip to content

Require gitPullRequest's environment and drop a duplicate restore-name test - #509

Merged
pondorasti merged 1 commit into
mainfrom
codevisor/test-audit-core-pkgs-3
Oct 14, 2026
Merged

pondorasti merged 1 commit into
mainfrom
codevisor/test-audit-core-pkgs-3

Conversation

@pondorasti

@pondorasti pondorasti commented Oct 14, 2026 •

Copy link
Copy Markdown
Member

What

A test audit of packages/worktrees, rebased onto the comment-free main.

Removed:

  • The process.env fallback in gitPullRequest(dir, env?), and its test "uses the process environment when none is given".
    • The only production caller, cachedPullRequest in apps/server/src/routes/git-review.ts:61, always passes the server's env. That means the fallback branch ran only in this test, which had to stub PATH to reach it.
    • env is now required on gitPullRequest and on its private gh helper.
  • The pure test chooseRestoreName "gives up rather than looping forever when the space is exhausted".
    • It built the same -2…-99 taken set as the restoreWorktree boundary test "fails loudly rather than restoring under an unrelated name".
    • The boundary test also checks the user-facing error and that the snapshot survives.

Strengthened:

  • The surviving fake-gh test. It now checks that gh runs with GH_PROMPT_DISABLED=1 on top of the caller's environment. I confirmed it fails when that variable is dropped. Without the check, a regression could leave gh blocked on an auth prompt.

Candidate evidence

Candidate Non-test callers Remaining proof Coverage
gitPullRequest env fallback git-review.ts:61 passes env "asks gh about the current branch…", "is null when gh has no pull request…", "is null on a detached HEAD…" Neutral, because the ?? branch is gone
chooseRestoreName exhaustion test n/a restoreWorktree name exhaustion 100% holds

Retained false positives

  • chooseRestoreName "prefers the original name and falls back to readable suffixes", and the export it needs.
    • Its -3 case (original and -2 both taken) is the only check that the suffix search moves past -2. Code review pointed this out.
  • git.test.ts "resolves no start point for a repo without an origin/main ref".
    • It is the only direct check of the "undefined, so use HEAD" contract.
  • The ≥500-name uniqueness tests, and "deletes snapshots idempotently".
    • Both guard real invariants.

Code review

The review found no bugs in this diff. Two of its findings were pre-existing bugs in restoreWorktree:

  • It calls worktreeStartPoint without env.
  • Its branch-name scan runs without env.

Those go in a separate PR at the owner. I didn't take the suggestion to keep env optional for consistency with siblings: a test-only default is exactly what this audit removes.

Validation

  • The pre-commit bun run check ran format:check, lint, comments:check, ratchet:check, and boundaries, plus typecheck and coverage for worktrees and everything that depends on it, including the server. All passed.
  • git diff --check passed.

LOC (git diff --numstat)

  • Production: +3 / −3 (git-pull-request.ts)
  • Tests: +8 / −35

🤖 Generated with Claude Code

@pondorasti
pondorasti force-pushed the codevisor/test-audit-core-pkgs-3 branch from 7ef2d2e to 04bf6da Compare October 14, 2026 19:22
@pondorasti pondorasti changed the title Require gitPullRequest's environment and stop exporting chooseRestoreName Require gitPullRequest's environment and drop a duplicate restore-name test Oct 14, 2026
…e test

The only production caller of gitPullRequest always passes the server's
environment, so the process.env fallback ran only in a test. The pure
chooseRestoreName exhaustion test repeated the restoreWorktree exhaustion
test, which also asserts the user-facing error. The surviving gh test now
checks that gh runs with prompts disabled.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pondorasti
pondorasti force-pushed the codevisor/test-audit-core-pkgs-3 branch from 04bf6da to 3a40902 Compare October 14, 2026 19:26
@pondorasti
pondorasti merged commit 773809c into main Oct 14, 2026
@pondorasti
pondorasti deleted the codevisor/test-audit-core-pkgs-3 branch October 14, 2026 19:26
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.

1 participant