Repository navigation
test(public-read,kubernetes): pin untested rules from the 10-16 reader, oauth seed and workspace fixes - #2570
Conversation
…der, oauth seed and workspace rules Mutant sweep of #2550/#2562/#2558, #2552, #2555 and #2559 left real rules untested: the refused-connection ceiling, close-on-client-close, the one-second close and the request drain in the public reader; the consumed seed Secret retry statuses and attempt cap, the already-consumed path, the Job 404 re-read, the Deployment ownership check and the legacy Pod wait; an OAuth home on an API-key Harness and a node subPath escape; two prior-PAT helper conditions.
|
🦞👀 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. |
There was a problem hiding this comment.
I verified that the new assertions in three files at 4aa5a937 distinguish working behavior from specific regressions. This is a partial review, not whole-PR approval; the fourth changed file still needs its separate review.
| New coverage | What I actually observed |
|---|---|
| Public GitHub reader, refused connection | Exact full test file: 13 passed. In an isolated negative control removing only socket.resume() from the refusal path, the new test failed at its 15-second deadline (rather than falsely passing). Real loopback sockets and a fixture provider; not a live GitHub or load-capacity claim. |
| Harness workspace volume, provider projection | Exact owning test passed. Separately permitting a suffix after the valid node directory name, or permitting an OAuth home in the API-key projection, made the new assertion fail with “Missing expected exception.” No cluster or volume was created. |
| First-Agent prior OpenShell PAT policy | Exact owning test passed. Separately removing the Codex-harness requirement or falsely accepting read-only as that historical policy made the new assertion fail (true !== false). This tests the policy predicate, not a live login. |
Node 24.20.0 on Linux, isolated local controls, no product changes requested from these results. Required CI passed; I saw Claw’s acknowledgment, not a final verdict, and did not request a duplicate run.
Post-merge check of the three files above. I verified their final 3c379506 bytes match squash commit 70698b4d and ran the revised tests: public-reader file 13 passed; the changed workspace-volume and first-Agent cases each passed, with no skips. In particular, I changed only refusing >= maxRefusing to refusing > maxRefusing in a private negative control. The new ninth-client assertion failed on the actual 429 Too Many Requests response versus the required empty response, rather than simply timing out. That rules out the revised already-closed-client branch masking an off-by-one regression in this case. These are local tests, not production traffic, a retrospective approval, or a claim about the fourth file. No contributor action or rerun requested.
…ases The legacy Pod wait now requires an empty Pod list before the Job starts; refused-socket reads resolve at once when the connection is already gone, so drain mutants fail on an assertion instead of the subtest timeout; the unowned Deployment case also checks the source stays unconsumed; the node subPath traversal row is pinned to its own message.
|
ClawSweeper status: review started. I am starting a fresh review of this pull request: test(public-read,kubernetes): pin untested rules from the 10-16 reader, oauth seed and workspace fixes This is item 1/1 in the current shard. Shard 0/1. This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment. Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted. |
Problem
A mutation sweep of the rules that landed on 10-16 with their own tests (#2550, #2562 and #2558 public GitHub reader connection caps; #2552 OAuth seed Secret delete-first; #2555 OpenShell network proxy drop and the first-Agent prior PAT helper; #2559 single Harness workspace claim) ran 90 mutants against the owning tests on main. 46 survived. 30 of those are real rules that no test pinned, including:
>or 1000), the one-second close of a refused socket that never closes (a 30 s or no-op timer), the refusal count leaking (refusing--dropped), the refused socket's error handler (a client reset would throw), draining the request (without it a refused socket stays open after its client closes), and closing only after the client reads (a destroy right after the write resets a client that is still sending).removeOAuthBootstrapretiring a legacy Deployment, the Job delete re-reading on 404, a non-conflict Job delete error being re-read, a foreign bootstrap Deployment being deleted (getinstead ofgetOwned), and the Job starting while a legacy Deployment's Pod still runs.subPathsuch asworkspace-node-…/../xpassed a loosened shape check.harness === "codex"condition could be dropped, and so could the exactworkspace-writematch (any non-full-access sandbox).Change
Test-only. New cases sit next to the existing ones:
tests/integration/public-github-read-http.test.mjs: one subtest, withsetTimeoutmocked so the one-second timer fires ontick. It checks that a refused connection closes when its client closes, that a request arriving after the answer is drained rather than reset, that a client reset leaves the reader running, that a lingering refused socket is closed at exactly 1000 ms, and that eight refused sockets can wait while a ninth closes unanswered until one of the eight frees its slot.tests/conformance/kubernetes-compute.test.mjs: four new tests and one added assertion. The new tests cover the seed delete retry table (timeout, 404, 409, 429, 500 then 503, and 403 not retried); three failed seed deletes, then an already-consumed pass that removes the seed before a failing Job delete and a later pass that clears a legacy Deployment; a Job already gone at its delete; a foreign bootstrap Deployment that is refused and kept, with the source left unconsumed; and a legacy Deployment's Pod being awaited before the Job is created, whether the Deployment is deleted or already gone. The added assertion counts Job deletes in the existing seed-first test, so a non-conflict error is not re-read.tests/conformance/kubernetes-harness-workspace-volume.test.mjs: one nodesubPathtraversal row, plus the API-key render with an OAuth home mount.tests/integration/first-agent-startup.test.mjs:read-onlysandbox andopenclawharness rows.Verification
endhandler (stream autoDestroy closes the socket once both sides end), theleft >= 0map entry, the undefined-address and no-listener guards, the only-first-listener mutant (Node registers a single connection listener), the final seed delete always running (a no-op after delete-first),Backgroundpropagation (Pods are awaited separately), the missing-home count (the name check refuses it),readOnlyshape strictness on home and node mounts (an undefined value is writable either way), the initializer path-only filter,priorRevisionId !== undefined(revision IDs are always set), and the prepare-sitenodeExpected(the render always adds the node mount when a node exists).first-agent.mjsreporting and printing the prior PAT warning, so those mutants were not run.prettier --checkandeslintpass on the four files. Review nits are applied: the Pod wait now requires an empty Pod list, and the drain mutants fail on an assertion rather than the subtest timeout.Risks
Test-only. Open PRs that touch
kubernetes-compute.test.mjs(#2518, #1982, #115) or other files here have the same conflict count against this branch as against main, so this change adds no new conflicts. The public reader subtest mocks onlysetTimeoutand resets it after the subtest. The reset case uses a half-open client:resetAndDestroy()after an auto-ended socket spun the event loop on Node 26.