test(gate,ai,rate-limit): recover coverage work orphaned by the #9501/#9504/#9505 merges - #9509
Merged
Merged
Conversation
Mirrors the sibling visual-capture-satisfied fail-safe test: a rejecting clear must not throw out of the pass, since a failed clear only means the gate stays deferred until the head moves. (cherry picked from commit efa6472)
…sive head-SHA guard (#9462) screenshotTableEvidenceUnresolved is computed as a plain boolean, so the undefined arm of its conditional spread was dead code -- pass it directly. The head-SHA guard in the exhausted branch is genuinely defensive (a recapture-preview job is only minted for a PR that had one), matching the sibling mark write, so it carries a reason rather than a contrived test. (cherry picked from commit 4f59499)
…age (#9479) rm with force:true does not throw for a missing path, so the catch needs a filesystem-level failure no unit test can portably induce; the unset-cwd guard is only reachable when mkdtemp itself threw, i.e. no directory was ever created. Both annotated with their reason, matching the file's existing convention. (cherry picked from commit bf3efe6)
…l guard (#9476, #9479) The EPIPE regression uses the existing real-subprocess harness rather than a stubbed spawn, so the stdin error listener is genuinely exercised: a fake CLI exits immediately without draining stdin, and the test asserts no uncaughtException reaches the process. Verified to fail without the listener (it captures an uncaught EPIPE), which is precisely the crash that took down every in-flight queue job in the container. Also covers isStalledNoOutput rejecting a non-Error throw -- a provider adapter can reject with a string, and misclassifying that as a deadline signal would silently skip the retry budget. (cherry picked from commit c770cc1)
Contributor
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-28 02:00:10 UTC
Review summary Nits — 3 non-blocking
Concerns raised — review before merging
📋 Copy for AI agents — paste into your coding agentDecision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. Decision record
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Contributor
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9509 +/- ##
==========================================
- Coverage 89.55% 88.65% -0.90%
==========================================
Files 843 843
Lines 110106 110101 -5
Branches 26203 26200 -3
==========================================
- Hits 98601 97610 -991
- Misses 10239 11520 +1281
+ Partials 1266 971 -295
Flags with carried forward coverage won't be shown. Click here to find out more.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Recovers coverage work for #9460, #9462, #9476, #9479 and #9494 that was orphaned when PRs #9501, #9504 and #9505 merged while the follow-up commits were still in flight.
No behaviour change beyond two dead-code removals — this is the test and annotation work those three PRs were missing when they landed, plus the
codecov/patchgap that was open on each at merge time.What happened
#9501, #9503, #9504 and #9505 merged at 00:27–00:48 UTC. I had pushed additional coverage commits to those branches shortly before and after, and the squash-merges took the branch state as of the merge, so five commits never reached
main:test(gate): fail-safe marker-clear pathrefactor(gate): unreachable spread arm + defensive guard annotationtest(ai): exclude two unreachable cleanup armstest(ai): real-subprocess EPIPE + non-Error stall guardtest(rate-limit): inline-retry break for an over-budget Retry-AfterVerified by content rather than SHA (squash-merge rewrites SHAs):
grepingorigin/mainfor each change's distinguishing marker. The metrics registration and the marker-clear-on-exhaustion test did land and are not re-applied here.Each commit is cherry-picked with
-x, so the original SHA is recorded in its message.Contents
#9462 — fail-safe marker clear. A rejecting
clearPullRequestVisualCaptureRetryPendingmust not throw out of the pass; a failed clear only means the gate stays deferred until the head moves. Mirrors the sibling visual-capture-satisfied fail-safe test.#9462 — dead code.
screenshotTableEvidenceUnresolvedis computed as a plain boolean, so the!== undefinedarm of its conditional spread was unreachable; it is passed directly now. The head-SHA guard in the budget-exhausted branch is genuinely defensive (arecapture-previewjob is only minted for a PR that had one, and the sibling mark write carries the identical guard), so it gets a documented reason rather than a contrived test.#9479 — EPIPE, as a real regression. This one matters: it drives the real subprocess via the existing fake-CLI-on-PATH harness rather than a stubbed spawn, so the
child.stdinerror listener is genuinely exercised. A fake CLI exits immediately without draining stdin, and the test asserts nouncaughtExceptionreaches the process. Verified to fail without the listener — it captures an uncaughtEPIPE, which is exactly the crash that took down every in-flight queue job in the container.#9476 — non-Error stall guard.
isStalledNoOutputmust reject a non-Error value rather than throwing on.message; a provider adapter can reject with a string, and misclassifying that as a deadline signal would silently skip the retry budget.#9479 — two unreachable cleanup arms annotated with their reasons:
rmwithforce: truedoes not throw for a missing path, and the unset-cwd guard is only reachable whenmkdtempitself threw.#9494 — inline-retry break. Asserts the attempt count, not just the classification: a 60s
Retry-Afteryields exactly one attempt instead of four, while a short one still uses the full inline budget so the break stays narrow.Validation
npx tsc --noEmit -p tsconfig.json— cleanqueue-3,selfhost-ai,ai-review,github-app,agent-actionsNote for future PRs in this series
The lesson is recorded rather than just fixed: pushing a follow-up commit to a branch whose PR is eligible to merge races the merge, and the squash takes whatever the branch held at that instant. For the remaining PRs in this series I'm verifying landed content by grepping
origin/mainfor a distinguishing marker rather than trusting that a push to the branch implies a change inmain.