Repository navigation
test: pin conditions from the 10-16 evening fixes - #2661
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Pin every link modifier and the middle button, the credential veto's case, use the real runtime read timeout shape for the stale failing status read, and note the synthetic boundary rows.
|
#2660 has landed. I checked the overlap in One small integration detail: the new |
|
Codex review: needs changes before merge. What this changesThis PR adds regression tests for startup-probe retry rules, console polling, loading transitions, recovery links, and discarded Slack drafts. Example: A superseded runtime-status request returns HTTP 504 after a newer successful response.
Review scores
ProductKind: Test only · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Needs changes before merge - 1 item remains Keep open for maintainer triage: this maintainer-authored PR adds useful regression coverage, with one low-impact test-integrity correction. Priority: P3 Before merge
Findings
Tests
Agent review detailsHow this fits togetherBrowser tests exercise the console and fixture-backed controller routes; startup conformance tests execute the generated runtime entrypoint with scripted native-process results and inspect readiness and failure diagnostics. flowchart LR
BrowserTests[Browser scenarios] --> Console[Console modules]
Console --> Controller[Fixture controller API]
Controller --> BrowserResults[Visible state assertions]
StartupTests[Startup scenarios] --> Entrypoint[Generated runtime entrypoint]
Entrypoint --> StartupResults[Readiness and diagnostic assertions]
Technical reviewBest possible solution: Retain the focused regression coverage and replace or drop the unreachable numeric-stdout scenario. Do we have a high-confidence way to reproduce the issue? This is a coverage PR rather than a current product-bug report; source inspection establishes the unsupported fixture input without executing target code. Is this the best way to solve the issue? Extending existing scenarios is appropriate, while the numeric-stdout case should use a reachable failure or be removed. Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 0dc13f25a432. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 16, 2026, 8:54 PM ET / October 17, 2026, 00:54 UTC. |
#2660 moved console-agent-overview to one Chromium per file; the new patchChildren case now takes it too.
Problem
A mutation sweep of the rules that landed on 10-16 evening found conditions their own tests did not check. Each mutant below survived the owning test files on main (product file restored after every mutant):
codex-native-startup:auth,forbidden,credentialor the bare401/403alternative, or the veto's case-insensitivity, changed nothing;invalid_request_errorbody was covered) and the top of the range (499);connection reset,connection closedandconnection failed(the onlyfailedcase also saiderror sending request), and the markers' case-insensitivity;now + 1000 < deadlinevs<=);-attempt2mark on a cause with no detail (aWRAPPER_ERRORafter a retry would carryundefined-attempt2).console-agent-overview);patchChildrenkeeping stale attributes, keeping children a shorter render no longer has, rewriting unchanged text, and patching a node in place when its role or role class changes (no direct test);console-retained-revalidationcovered only the reads path);200was covered,console-agent-logs);channels/shared-ui.mjs; the surviving mutant from theconsole-agent-savesconsolidation).Change
Test-only. Rows and cases next to the existing tests, no new helpers or files:
tests/conformance/codex-native-startup.test.mjs: 3 transient rows (each runs twice: recovers, then held as-attempt2), 7 held-at-once rows (400 with status, 499, one credential marker each, the API key in mixed case), and 3 scenarios (exact one-second budget, transient then timeout in one attempt, wrapper error after a retry).tests/browser/console-agent-overview.test.mjs: the deploy-poll test also checks the patched row losesmuted, and that Refresh pressed during a held poll shows busy and reads after it; a new case drivespatchChildrendirectly; the reduced-motion recovery-link test checks each modified click and a middle click are left to the browser.tests/browser/console-agent-logs.test.mjs: a third supersede case where the stale status read fails with 504.tests/browser/console-retained-revalidation.test.mjs: a fresh load whose session check is held past 400 ms keeps "Reading this Agent." for 299 ms after it appears and shows the Agent at 300 ms.tests/browser/console-agent-saves.test.mjs: after a declined carried Slack draft, leaving for the Agents list and opening the Agent again shows no note and no editor.Verification
taskset, 2 cores) on 1516c27, 20/20 each: codex-native-startup (121 tests), and the 14 touched browser tests (overview 4, logs 3, retained-revalidation 5 with the four existing fresh-load cases, saves 2). After merging main (test(console): speed up the Agent overview tests #2660 moved the overview file to one shared browser; the new patch case now takes it), the four touched overview tests again 20/20 on a982961, and the patch, Refresh-queue and link-modifier cases still fail on their mutants.prettier --checkandeslint --max-warnings 0on the five files.unauthori[sz]ed(always matched byauth);>= 400/< 600on the 5xx check (a 4xx already returned, statuses are[45]xx);result.status === nullwith a kill signal;transient === truevs truthy;"-attempt" + attemptvs"-attempt2"(two attempts at most);includesvsendsWithfor the cause suffix (no detail token contains-attempt2; the runtime appends it once, at the end); a duplicate reveal timer or an uncleared one after abort (the late call finds the stand-in gone and its steps check the page is current).invalid_request_error|content_filter|certificate|Name does not resolve|…; the pinned Codex build prints these markers in fixed case); thens_pattern's 64-character cap and character class (a sanity cap; real IDs arens_plus a UUID); the timeout retry's own budget boundary (predates these fixes; the transient one is pinned).Internal review: approve with nits, all applied (every link modifier and the middle button, the credential veto's case, the real
RUNTIME_LOGS_TIMEOUTshape for the stale failing read, comments on the synthetic rows).Risks
Test-only. The browser additions use the existing paused-clock and held-request helpers; the new waits are bounded by the suite's test timeout.
console-agent-saveschanges stay clear of #2593's lines (git merge-treeclean), andcodex-native-startupchanges stay clear of #2487's.