Skip to content

test: separate reftable semantics from production latency budgets - #13185

Closed
teamleaderleo wants to merge 2 commits into
mainfrom
test-isolate-git-plumbing-fixture
Closed

teamleaderleo wants to merge 2 commits into
mainfrom
test-isolate-git-plumbing-fixture

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 20, 2026 •

Copy link
Copy Markdown
Collaborator

Reftable semantic tests inherit the production two-second Git-read budget while running real subprocesses alongside the rest of the package. The full package reproduces a failure in metadataUsesGitResolvedWorktreeBranchAndWatchesReftableStorage even though the suite passes in isolation. Temporary local instrumentation showed a successful rev-parse starting with 14 ms remaining and taking 33 ms; the reader correctly rejected it as incomplete after the deadline.

Give these real-process semantic tests an explicit 15-second budget through existing injection points. For the ambient-repository test, use the executable that successfully created the reftable fixture, so environment isolation is tested independently of backend fallback. Preserve branch, commit, watch-path, unborn-branch, and fallback assertions, and add a direct check that an expired reference read remains unreadable without starting Git. Production code and its deadlines are unchanged.

Validation: baseline full-package runs failed locally; the test-first commit also fails the existing linked-worktree test (208 tests, 1 issue). With the fix, the full package passes, including the deadline/process-termination tests. A separate full run with Git 2.53 preferred for fixture creation also passes 208 tests. Builds use two compiler jobs locally, without launching the app. No hosted diagnostic run was dispatched.

This is a scoped reliability fix, not a claim that the original CI failure is conclusively explained: #13163 failed plumbingIgnoresAmbientRepositorySelection, which still passes in isolation locally, and CI's Homebrew Git is 2.55 rather than the local versions. Hosted confirmation remains pending.

Related: #13095, #13163.


Summary by cubic

Reftable semantic tests were inheriting the production two-second Git-read budget, so real-process assertions could time out when the full package runs in parallel. They now use an explicit 15-second wall-time budget; production code and deadlines are unchanged.

Changes

  • Adds a direct assertion that an expired reference read returns .unreadable without starting Git.
  • Uses the executable that created the reftable fixture for the ambient-repository test so environment isolation is tested independently of backend fallback.
  • Preserves the existing branch, commit, watch-path, unborn-branch, and fallback assertions.

Validation

Written for commit 23b61fe. Summary will update on new commits.

Review in cubic

@coderabbitai

coderabbitai Bot commented Sep 20, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 1 minute.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 05b44388-1467-4124-9aae-7e30bca4b737

📥 Commits

Reviewing files that changed from the base of the PR and between 1cd76eb and 23b61fe.

📒 Files selected for processing (1)
  • Packages/macOS/CmuxGit/Tests/CmuxGitTests/ReftableGitMetadataTests.swift

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@greptile-apps

greptile-apps Bot commented Sep 20, 2026

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

The PR appears safe to merge because it only adjusts test configuration and adds focused deadline coverage without changing production behavior.

Summary

This PR separates reftable semantic test execution from the production Git-read latency budget without changing production behavior.

  • Gives real-process reftable tests an explicit 15-second bound.
  • Pins the ambient-repository isolation test to the Git executable that created its fixture.
  • Adds coverage proving an already-expired reference deadline returns unreadable metadata without launching Git.

Reviews (1) · Last reviewed commit: "test: isolate reftable semantics from pr..."

@teamleaderleo

Copy link
Copy Markdown
Collaborator Author

Duplicate of #13186, which merged. Both fixed the same reftable test flake in the same file.

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