test: port June upstream test-hardening fixes (#729, #738) - #124
Merged
Conversation
Hand-ports the scrub-stability poll from upstream G-Research/bobbit PR G-Research#729 (unmerged upstream) against current tests/e2e/ui/tree-cost-rollup.spec.ts, which has diverged since June: - quiesceAutoStartedChildTeam's initial wait now polls for a populated `teamLeadSessionId` instead of just `getTeamState()` truthiness, so we don't race ahead of a team-lead session id being assigned. - the goalId cost wipe after teardown becomes a stableZeroPolls loop keyed off CostTracker.getGeneration()/getGoalCost() (both present on the current CostTracker) instead of a single-shot removeSession sweep, to catch a final mock-agent usage event landing just after teardown returns. Verified: `npm run check`, `npm run test:unit -- tests/e2e/ui/tree-cost-rollup.spec.ts` would route to the browser/file:// suite so this was instead validated directly with `npx playwright test -c playwright-e2e.config.ts tests/e2e/ui/tree-cost-rollup.spec.ts --retries=0` — all 4 tests pass. Upstream: G-Research#729
Hand-ports the genuine-robustness hunk from upstream G-Research/bobbit PR G-Research#738 (unmerged upstream, "harden unit flakes under full-suite load") against current tests/verification-harness-timeout.test.ts. TREE_PAYLOAD no longer pipes the grandchild's stdout through the parent and waits for an async stdout chunk before printing PARENT_PID — under full-suite load on slow Windows workers that round-trip could lose the race against a short timeout, failing the test before it captured both PIDs. The parent now reports PARENT_PID and CHILD_PID synchronously, immediately after spawn (`child.pid` is available as soon as the OS child exists) — removing the race entirely rather than widening a margin around it. Skipped from the same upstream PR (see PR body for the full table): - The three `poll(..., <n>ms)` timeout-widenings in this same file (3000/5000/3000 -> 10_000) are pure margin-widening with no correctness change; our program has since moved flaky timing tests toward by-construction proofs rather than bigger margins, so these were left alone. One of the three (the cancellation-test reap poll) is already superseded here by an even better fix already in this file (10000ms + explicit 100ms poll interval, with a comment explaining the Windows taskkill scheduling contention it accounts for). - tests/goal-push-safety-regression.test.ts: the PR's `10_000ms -> 60_000ms` deadline widening targets a `waitForPublishedUpstream` helper/function that no longer exists in this fork — that test file has since been rewritten wholesale to synchronous by-construction assertions over git upstream-tracking config, with no polling loop left to widen. - tests/worktree-pool.test.ts: the PR's fix for the racy `pool/_pool-*` branch-existence assertion (capture the pooled branch name before claim, assert only that branch is gone rather than the prefix's global absence) is already present in this fork, independently landed with equivalent logic. Verified: `npm run check` and `npm run test:unit -- tests/verification-harness-timeout.test.ts tests/worktree-pool.test.ts tests/goal-push-safety-regression.test.ts` — 28/28 pass. Upstream: G-Research#738
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
Post-program housekeeping: ports the LOW-RISK test-hardening subset of not-landed June 2026 upstream (
G-Research/bobbit) PR content into this fork, perJUNE-PRS-REPORT.md"Residual risk" section. Each hunk was hand-verified against current code (files have diverged since June) rather than blindly applied.stableZeroPolls/getGeneration()scrub-stability poll intotests/e2e/ui/tree-cost-rollup.spec.ts.tests/verification-harness-timeout.test.ts.Port / skip table
tests/e2e/ui/tree-cost-rollup.spec.tsstableZeroPolls/getGeneration()/getGoalCost()all exist on the currentCostTracker. Validated with 4/4 Playwright passes.tests/verification-harness-timeout.test.tsPARENT_PID; under full-suite load that round-trip could lose the race against a short timeout. New payload reports both PIDs synchronously right after spawn. Confirmed still-applicable — the racy async version was still present in this fork.tests/verification-harness-timeout.test.tstests/goal-push-safety-regression.test.tswaitForPublishedUpstreampolling helper that no longer exists — this file has since been rewritten wholesale to synchronous by-construction assertions over git upstream-tracking config. Nothing left to widen.tests/worktree-pool.test.tsclaim()and asserts only that specific branch is gone (rather than the racy global-prefix-absence check) — equivalent logic already present, independently landed.Out of scope (explicitly, per instructions)
G-Research#762 (multi-gateway, feature), G-Research#716 (Podman runtime, feature), G-Research#786 (prompt placeholders, separate lane), G-Research#715 (local config cleanup, touches gateway state), G-Research#724 (superseded), and G-Research#708/nurse-cap-equivalents — none touched.
Test plan
npm run check— passes clean.npm run test:unit -- tests/verification-harness-timeout.test.ts tests/worktree-pool.test.ts tests/goal-push-safety-regression.test.ts— 28/28 pass.npx playwright test -c playwright-e2e.config.ts tests/e2e/ui/tree-cost-rollup.spec.ts --retries=0— 4/4 pass.Upstream references (read-only, not pushed to): G-Research#729, G-Research#738
🤖 Generated with Claude Code