Skip to content

Serialize flaky Claude CLI platform-gating cases#2311

Merged
steipete merged 2 commits into
steipete:mainfrom
Chipagosfinest:agent/isolate-platform-gating-tests
Jul 18, 2026
Merged

Serialize flaky Claude CLI platform-gating cases#2311
steipete merged 2 commits into
steipete:mainfrom
Chipagosfinest:agent/isolate-platform-gating-tests

Conversation

@Chipagosfinest

@Chipagosfinest Chipagosfinest commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • serialize only the two parameterized Claude CLI platform-gating cases
  • preserve the per-mode assertion that logged-out .auto and .cli runs invoke only auth status --json
  • keep the containment test-only; no production concurrency behavior changes

Fixes #2309.

This flake surfaced in #2292's unrelated Linux x64 shard. Both parameterized cases started together and failed the same invocation assertion even though each already uses UUID-scoped binary and log paths. The deeper shared runtime boundary is not proven, so this PR takes the narrowest safe containment.

Validation

  • swift test --filter PlatformGatingTests — 10/10 passed
  • make check — passed; 0 SwiftFormat changes and 0 SwiftLint violations across 1,572 files
  • make test — passed all 716 selections in 60/60 groups, no retries or timeouts
  • git diff --check — passed

Linux x64 CI is the authoritative regression gate for the originally observed flake.

Exact-head Linux proof

Both jobs retained the full Linux test step and CLI smoke test. The remaining macOS jobs are queued and unchanged by this TestsLinux-only diff.

@Chipagosfinest

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 2723057453

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 6:52 PM ET / 22:52 UTC.

Summary
The PR adds Swift Testing’s .serialized trait to the two parameterized logged-out Claude CLI platform-gating cases in TestsLinux/PlatformGatingTests.swift.

Reproducibility: no. high-confidence deterministic current-main reproduction is provided; the report and CI logs identify the concurrent test path and failing assertion, making the source path reproducible but the underlying interference mechanism unproven.

Review metrics: 1 noteworthy metric.

  • Diff scope: 1 line changed in 1 test file. The patch is tightly limited to scheduling two Linux test invocations and leaves production code untouched.

Merge readiness
Overall: 🦞 diamond lobster
Proof: 🦞 diamond lobster ✨ media proof bonus
Patch quality: 🦞 diamond lobster
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Risk before merge

  • [P1] Serializing these cases removes their incidental concurrent coverage, so it does not determine whether a deeper Claude planner/auth/subprocess race exists outside this test harness.

Maintainer options:

  1. Accept narrow test containment (recommended)
    Merge the serialization as the documented flake containment while retaining a separate path to investigate any reproducible deeper runtime race.
  2. Preserve explicit concurrency coverage
    Pause this PR and first add a deterministic isolated concurrency harness if maintainers need these two modes to remain parallel-tested.

Next step before merge

  • [P2] No mechanical repair is identified; a maintainer should decide whether the documented reduction in incidental concurrency coverage is acceptable for this narrow flake containment.

Security
Cleared: The diff only changes a test scheduling trait and introduces no dependency, secret, workflow, permission, or production execution-path change.

Review details

Best possible solution:

Land the narrow test serialization if maintainers accept the bounded coverage trade-off, then investigate the deeper shared runtime boundary separately only if production evidence or a deterministic concurrency reproducer emerges.

Do we have a high-confidence way to reproduce the issue?

No high-confidence deterministic current-main reproduction is provided; the report and CI logs identify the concurrent test path and failing assertion, making the source path reproducible but the underlying interference mechanism unproven.

Is this the best way to solve the issue?

Yes for the currently evidenced test flake: serializing only these two cases is the narrowest maintainable containment and avoids changing production behavior while the deeper shared boundary remains unknown.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against ac7b2b5e3606.

Label changes

Label changes:

  • add merge-risk: 🚨 other: The intentional serialization prevents the observed flake but reduces concurrent coverage without proving whether a deeper shared runtime boundary exists.
  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P2: This is a bounded CI-reliability fix for a nondeterministic Linux test failure rather than a user-facing runtime outage.
  • merge-risk: 🚨 other: The intentional serialization prevents the observed flake but reduces concurrent coverage without proving whether a deeper shared runtime boundary exists.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🦞 diamond lobster and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (linked_artifact): The PR supplies linked successful Linux x64 and arm64 CI runs for the test-bearing commit plus focused and full-suite validation; this is appropriate real runtime evidence for a Linux test-only scheduling change.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR supplies linked successful Linux x64 and arm64 CI runs for the test-bearing commit plus focused and full-suite validation; this is appropriate real runtime evidence for a Linux test-only scheduling change.
Evidence reviewed

What I checked:

  • Narrow test-only diff: The branch changes one annotation on the parameterized Claude CLI logged-out fallback test from @Test(arguments: ...) to @Test(.serialized, arguments: ...); it does not touch production concurrency behavior. (TestsLinux/PlatformGatingTests.swift:88, 27230574533f)
  • Observed flake and proposed containment: The linked report documents both .auto and .cli cases starting together and failing the same per-case invocation-log assertion; its stated acceptance criteria explicitly prefer serializing only this parameterized test while the deeper shared boundary remains unproven. (TestsLinux/PlatformGatingTests.swift:88, 27230574533f)
  • After-change Linux proof: The PR body links successful Linux x64 and arm64 jobs for the test-bearing commit, alongside a focused PlatformGatingTests run and complete local make check and make test results. The current branch head is a later merge of origin/main, not a further edit to this test change. (TestsLinux/PlatformGatingTests.swift:88, 9c0ab0ff3273)
  • Current implementation candidate: This PR formally targets the linked open flake report, so the report should remain open until this branch is merged rather than being treated as already implemented. (TestsLinux/PlatformGatingTests.swift:88, 27230574533f)

Likely related people:

  • Chipagosfinest: Authored the focused serialization commit and the linked flake report; adjacent merged work exposed the Linux shard failure that this isolated test change addresses. (role: recent test-area contributor; confidence: medium; commits: 27230574533f, 6ba3d19575fd; files: TestsLinux/PlatformGatingTests.swift)
  • steipete: Authored the branch merge that brought current main into the review branch, making this the closest available repository-history signal for the current review head. (role: recent branch integrator; confidence: low; commits: 9c0ab0ff3273; files: TestsLinux/PlatformGatingTests.swift)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (3 earlier review cycles)
  • reviewed 2026-07-18T20:09:05.642Z sha 2723057 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-18T20:19:23.276Z sha 2723057 :: needs maintainer review before merge. :: none
  • reviewed 2026-07-18T21:01:22.049Z sha 2723057 :: needs maintainer review before merge. :: none

@Chipagosfinest

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 18, 2026

Copy link
Copy Markdown

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. labels Jul 18, 2026
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Jul 18, 2026
@Chipagosfinest

Copy link
Copy Markdown
Contributor Author

Fresh independent reproduction from unrelated share-card PR #2307:

This is a second real CI occurrence of the race #2311 narrowly contains. It strengthens the case for merging this one-line test-only serialization before treating downstream unrelated PR CI as product regressions.

@steipete
steipete merged commit ad7ee2f into steipete:main Jul 18, 2026
25 checks passed
@steipete

Copy link
Copy Markdown
Owner

Merged as ad7ee2f807dbee82a2f0b998bdd45303364fe59f after verifying exact head 9c0ab0ff3273f9a7383d2455fcb1d2d1af6a4806.

Verification:

  • Reviewed the full test-only diff and the failing Linux run from Prevent concurrent PlatformGatingTests flake on Linux #2309. The failed .cli and .auto cases began within the same millisecond and both missed the invocation-log assertion. Their fixture paths are UUID-scoped, and the relevant resolver/status/auth overrides are @TaskLocal; I found no mutable production singleton that justified a production-code change. Serializing these two subprocess-driven parameter cases is the narrow containment, with negligible suite cost.
  • swift test --filter PlatformGatingTests: passed, 10 tests in 1 suite.
  • make check: passed; SwiftFormat changed 0/1577 files and SwiftLint reported 0 violations in 1576 files.
  • Structured autoreview against origin/main: no actionable findings; patch correct (0.98 confidence).
  • Exact-head CI run 29664159644, attempt 1: all required checks green, including Linux x64 job 88131684043, Linux arm64, both macOS shards, lint, aggregate, and GitGuardian.
  • Targeted Linux x64 rerun attempt 2: job 88135126555 passed.
  • Targeted Linux x64 rerun attempt 3: job 88135779801 passed.
  • Final pre-merge state: MERGEABLE / CLEAN, exact head unchanged, 8 required checks successful and musl intentionally skipped.
  • Post-merge: the PR head is contained in origin/main; issue Prevent concurrent PlatformGatingTests flake on Linux #2309 auto-closed with the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Prevent concurrent PlatformGatingTests flake on Linux

2 participants