[WRONG BRANCH] release: promote main to preview after 2.58.0 - #4917
Merged
Merged
Conversation
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…state (#4832) * docs(devlog): record the 2.57.0 release and its registry propagation state Evidence for every step of the train, the two judgment calls (red dev, CodeQL new-alert count), and the pending registry read that must not become a republish. * docs(devlog): the registry confirmed 2.57.0 eight minutes after publish --------- Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…4825) Last release blocker for 2.57.0. The PR aggregate ci check is green at this exact head, and the lane=all dispatch 35134620067 on the same head passed all 24 real jobs including every one of the six Windows shards individually, with only the optional macos control job outstanding. Windows shard failures had moved between shards across five dispatches while always landing on the same management-auth cleanup, which is what said the shard number was noise and the held handle was the defect. The chain was a killed subprocess abandoned before it died, then two real Windows timeouts being paid on the startup path, then a failed-start native-main release that was not awaitable. All three are fixed at the cause. Host-owned merge decision; no local suite, typecheck, build, or install was run.
…ill clock (#4833) The shutdown drain measured its deadlines with `Date.now()` while the ACL harden work those deadlines budget runs on the injectable spill clock. A test could therefore freeze the clock, believe it had removed wall time from the case, and still lose its reserve to real elapsed time on a loaded runner. That is what turned `shutdown fallback prices the job-owned superseded generation before publishing` red on macOS 2/2 in run 35137850114 at e2304ce: the case freezes both clocks and sets an 80 ms reserve, and its own comment says the budget must not race a loaded shard's wall clock, but the freeze never reached `fallbackPendingResponseSpills`. It failed with `Response spill shutdown fallback budget exhausted` (ETIMEDOUT) instead of reaching the cap assertion it exists to make. `responseSpillNow()` is now exported from the spill store and used at all six former `Date.now()` sites in the drain, so the budget and the work it budgets read one clock. Production behaviour is unchanged: with no override installed it is `Date.now()`. The new test pins both directions. Real wall time burned inside a synchronous icacls runner cannot exhaust a frozen reserve, and advancing only the injected clock past the reserve still produces the ETIMEDOUT close-out - so the contract is "the budget reads the injected clock", not "the budget was removed". No local suite, focused test, typecheck, build, or install was run.
…sable port (#4834) PR #4830 raised SPAWN_BUDGET_MS on win32 from 45s to 90s to fix one test case. 31 test files read that constant and nine hand it to setDefaultTimeout, so the edit reached 339 Windows cases: 315 went 45s to 90s, 22 more that multiply it went 90s to 180s, and one derivation chain in codex-sync-api reached 265s. The detectors that now report twice as late are the contention ones -- codex-write-lock, the cross-process history-lock exclusions, the shim process cases -- and several of the affected files never spawn anything. The measurement behind #4830 was also contaminated. The child published its port through atomicWriteFile, the production SECRET writer, which on Windows runs hardenSecretPath(..., required: true) twice, each able to spawn PowerShell for SID resolution and several 30s-budgeted icacls passes. That ACL ceremony ran inside the window the parent measures as "time to reach a port" -- on a disposable port number. - SPAWN_BUDGET_MS returns to 45s on every platform. - COLD_SPAWN_BUDGET_MS (90s, win32 only) is consumed exactly once, by the readiness wait of the first proxy child in native-profile-startup. - The child publishes its port and settled markers with a plain temp-file rename, preserving the #1061 no-partial-read contract without the ACL ceremony. - The child logs child-entry, start-server-begin, start-server-end and port-published, so the next slow run names its own phase. - codex-sync-api owns its bounds instead of deriving them, which returns that case to 130s and makes it immune to the next edit of a shared constant. It also reports its measured preparation window on green runs. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…them (#4835) * test(ci): make two fail-open environment skips hard preconditions in CI model-metadata-sync skipped its entire drift gate when scripts/model-metadata.source.json was absent. That snapshot is tracked in this repository, so an absent input is a broken checkout, not an environment variation, and the skip silently removed the only check that the committed src/generated/model-metadata.ts still matches its source. The precondition is now asserted. server-startup-reconcile-resilience probed Bun.serve and skipped four startup cases whenever the probe failed. That is right in a sandboxed agent environment that denies Bun.serve outright, and wrong in hosted CI, where a runner that cannot bind loopback is a broken runner and four assertions disappeared with no trace. The probe now suppresses cases only outside CI, and a CI-only guard case asserts the bind capability so a genuinely unbindable runner names itself. * test(storage): re-enable the isolate worker-teardown cases on Linux and macOS Four Worker-spawning cases were skipped everywhere except win32. The stated cause was real: Bun 1.3.14 segfaulted at 0xFFFFFFFFFFFFFFF8 mid-file with a balanced workers_spawned/workers_terminated count (exit 133 on macOS Silicon in run 30691129351, exit 132 on ubuntu GHA in run 30700011812), which is a runtime defect our JavaScript teardown cannot close. Bun 1.4.0, the version this repository pins, contains the upstream fix: worker threads are parent-owned and joined before the parent VM is destroyed, bun:sqlite and other native resources are torn down before JSC, and a termination gate keeps native callbacks out of a stopping worker (oven-sh/bun#37075, #38299). oven-sh/bun#38519 reproduces this exact class and records 3/3 crashes on 1.3.14 against 3 x 400 clean terminate cycles on 1.4.0. The skip is deleted rather than re-scoped, and the churn count is a single 8 on every platform: the one-cycle macOS and two-cycle Linux caps were crash avoidance, and a one-cycle 'repeated spawn/reset' case does not test what its name claims. The meta-test that pinned those per-platform caps goes with them. The OS-join settle in src/storage/worker-lifecycle.ts is unchanged. * fix(codex): carry the history busy timeout into the Worker and unskip the restore-busy case tests/codex-integration/codex-composed-acceptance.test.ts declared the restore-busy envelope a platform-independent contract and then skipped it on win32. The comment was right and the skip was wrong. Run 32344670867 shows what actually happened: 'CLI watchdog: ocx restore --json' at 45197 ms on a shard where neighbouring cases took 54-106 s. No envelope, no SQLite error, no failed assertion - the child was still waiting out production's own busy budget (5 s per attempt, two attempts, 500 ms apart) inside a real CLI process. That wait is not the assertion, so it is shortened rather than budgeted for. In-process history tests already do this with setHistoryDbBusyTimeoutForTests; a child process could not be reached that way, and neither could the history Worker, which is a separate module realm that starts from the provider's default. The run message now carries the parent realm's busy timeout the same way it already carries the homes, validated and refused when malformed, and the Worker adopts it before its first state_5.sqlite open. Production sends the same codex-rs-matching 5 s the Worker would have used on its own, so the happy path is unchanged. The test spawns that one child with a --preload that applies the knob only when OCX_TEST_HISTORY_BUSY_TIMEOUT_MS is set on its environment; no production module reads that variable. The lock, the two-attempt retry, the exit code, the exact JSON envelope, the byte-identity check, the release, and the convergence assertion are untouched.
…r-the-fact dispatch (#4840) The release record claimed the candidate run covered all six Windows shards. It did not: platform-windows is dispatch-only, so both the candidate push run 35131181996 and the release-SHA push run 35133242171 skipped it, and the aggregate ci check accepts a skipped producer as a pass. The green Windows evidence that existed belonged to 1504caa, the #4825 lane head, not to the commit 2.57.0 was published from. Dispatch 35139132889 has now run lane=all at 44de45d, the exact published SHA, and all six shards passed individually. The release is sound; the record was not. No local suite, typecheck, build, or install was run.
…en (#4837) Four paths in this workflow turned a real failure into a green check. scripts/ci/run-bun-test-batches.sh classified a batch timeout separately, re-ran the batch one file per process, retried each singleton once more, and returned success when that sweep passed. Singleton isolation removes exactly the conditions that produce the failure -- batch concurrency, shared process state, resource pressure -- so the sweep was always going to pass. Run 35087572377 job 104766021341 logged a segfault in batches 11, 15, 18, 19, 22 and 26 and reported green. The macOS shard, the macOS control and the Windows shard each carried `for attempt in 1 2`: a second execution that happens not to die does not un-kill the first. A timeout or a crash now fails its job on the first occurrence. The one-file-per -process sweep stays, renamed to attribute_batch_file_by_file, and runs only after the shard has already failed, so a human still learns which files were in the batch. The shared classifier still runs in every lane; it now chooses the error message rather than the outcome. The aggregate `ci` gate accepted `skipped` from any job unconditionally, so it could not tell "this event did not ask for the job" from "this event asked and the job never started". It now derives what the event requested from the same conditions the jobs carry, and requires success from each requested job and skipped from the rest. On a lane=all dispatch it also reads the run's own job list and requires six concrete successful `windows N/6` results, because a matrix rollup reports success for five successes and one skipped leg -- the shape of run 35112645195, where job 104850075282 was skipped with zero steps while aggregate job 104855808104 concluded success. tests/ci-workflows/ci-bun-crash-classifier.test.ts asserted shell source text and never executed anything; one of its cases pinned the mask in place. The disposition contract moves to tests/ci-workflows/ci-crash-disposition.test.ts, which executes the real classifier against synthesized exit statuses and log fixtures, and the real batch runner against a fake bun and a fake timeout that reproduce a crash, a hang and an assertion failure on demand.
Each ACL-hardened write on Windows ran the icacls sequence twice against the same file. atomicWriteFile hardens the temp while it is still empty, so it is never readable by another principal, and hardens it again before the rename. The content written in between moves the success memo's freshness component (ctimeNs, which libuv reports from the NTFS ChangeTime), so the second call missed the memo and reapplied the ACL the file already had: /grant:r, /inheritance:r, /remove:g, plus up to three /findsid probes. After the content write the writer now re-asserts that the path still resolves to the object its descriptor holds, then re-attributes the memo to that same object, so the pre-rename harden resolves through the memo instead of repeating the mutation. Both calls stay required: true and still fail the write closed; re-attribution moves only the freshness, and only for an unchanged object. Windows also stops taking the chmod on that path. There it sets the read-only attribute rather than the DACL, so it is not the secret boundary, and its ChangeTime bump is what retired the memo a line later. POSIX keeps it. A cold ocx start writes ocx.pid and runtime-port.json through this writer, so those two writes go from four icacls sequences to two.
…it holds (#4849) * fix(lib): let a killed child actually die before anyone deletes what it holds Windows has been failing one shard per lane=all dispatch for weeks, and the shard number kept moving while the case family did not. The repository already wrote the right conclusion down once - the blame moved, the cause did not - and #4825 was the attempt at it. It did not land the fix. #4825 replaced "kill and resolve in the same tick" with "kill, wait up to two seconds, then abandon". That is the same false ownership contract on a slower clock. When the grace expired the caller was told its child was gone, cleanup removed the directory, and Windows returned EPERM because icacls still held it: EPERM: operation not permitted, rm '...\Temp\opencodex-test-vwyZZv\tmp\ocx-management-auth-kgvXCT' (fail) codex app-server restart routes ride the management gate > the data-plane token does not authorize the restart route [15367.90ms] A handle-bearing caller now has no second deadline after the kill. The child's actual exit is the only release signal, because it is the only thing that is true. Only the icacls runner takes that path; windows-user-principal passes 0 and still abandons immediately, since its PowerShell lookup sits on the startup critical path and holds no path anyone removes. The icacls runner's own outer watchdog still bounds the caller, so nothing can wait forever - the reap continues in the background and teardown drains it through flushRequiredSubprocessReapsForTests. The regression test no longer sleeps. A manual deadline seam orders kill against exit directly, so it proves the sequence rather than waiting for it. The Desktop copy-coherence probes are the same mechanism seen from the other end. Those scenarios perform different numbers of secure writes, each able to launch PowerShell or icacls on Windows, and that contention was spending a deadline the probe shares. The generated child now installs the existing synthetic principal and ACL runner seams before it loads any client module. Files, SQLite, locking, rotation, recovery and Desktop projection stay real; only the ACL subprocesses become synthetic, and an ACL descendant can no longer outlive the parent's kill holding a temp path. Causation was checked before writing any of this. Across 48 lane=all dispatches that completed all six Windows shards without PR #4836, the EPERM class appears in 3 and the probe-timeout class in 1, so both predate that PR and neither is attributable to it. No local suite, focused test, typecheck, build, or install was run. * fix(windows): separate "may I stop waiting" from "is it safe to delete" The first attempt made `waitForSubprocessExit` wait for a killed child to really exit, and Windows still failed the same way. The abandonment had simply moved up a layer. `awaitAsyncIcaclsRunner`'s belt fires at timeoutMs + kill grace + 250ms. That margin was sized against the old bounded 2-second grace, so once the inner wait became unbounded the belt was once again the deadline: on a stalled icacls it resolved timedOut while the child was alive, the caller logged "continuing without it", and teardown deleted a directory icacls.exe still held. [opencodex] ACL hardening timed out (ETIMEDOUT) - transient icacls stall EPERM: operation not permitted, rm '...\tmp\ocx-management-auth-Ge98cW' (fail) codex app-server restart routes ride the management gate > both routes reject an unauthenticated caller and a cross-origin caller [15261.56ms] The 15.26s there is removeTreeWithRetry's ladder exhausting, not a probe deadline. These are two different questions and they now have two different answers. The belt still releases its caller, so a genuinely stuck child cannot hang startup, shutdown or uninstall - that bound is the whole reason the belt exists. But when it gives up it registers the outstanding reap against the target path, and code that is about to REMOVE something must settle that separately: a test tree awaits flushWindowsSecretAclReapsBeforeRemoval, the async writer leaves a residual temp rather than racing an unlink against a live handle, and uninstall refuses promptly rather than waiting, because handing a stuck child the power to hang `ocx uninstall` would give back the bound we just protected. The holder was confirmed rather than assumed: the failing proxy is in-process and auth rejection stops the restart child from spawning, so icacls.exe is the only child holding the fixture path. removeTreeWithRetry stays. It is a fair accommodation of a short filesystem release race; it should just never have been absorbing a live child. No local suite, focused test, typecheck, build, or install was run.
…4846) * test: assert these contracts by behaviour instead of by source shape Five test files asserted what the source LOOKS LIKE — regexes over src/ read through repoPath()/Bun.file() — rather than what it does. A test like that survives any regression that keeps the shape and breaks on a harmless rename, so it reports the opposite of what a reader assumes. The audit that found them rated this family High for the auth, send-budget, abort and lifecycle cases specifically, because those are the ones whose silent failure a user pays for. Each file keeps every contract it previously pinned; the assertions moved to the observable consequence of that contract. - responses-preview-main-read-fence: the fence forbids READING the physical main token, so the oracle is whether auth.json is opened. Both preview sites and final auth are now driven for real - a thread_spawn carrying a forwardable caller bearer, a main-only denial cache, and an encrypted-recovery re-preview through a noncanonical route. The file's own comment said this fixture was more fragile than the divergence it would catch; it is built here and it is not. The deliberate asymmetry is preserved: nativeMainSelectionOnly still derives from the drain alone in both files, now proven by routing rather than by regex. - transient-budget-scope-source and execution-budget-permits: one physical send is one charge, and a reservation the budget hands back releases its tokens instead of booking spend. Both are now read off the ledger and the observer. - cancel-body-on-abort: the teardown ORDER is the contract, so a fake records fetch.abort -> body.cancel -> reader.cancel -> releaseLock and the sequence is asserted. Four dispatch paths additionally prove each original body is consumed exactly once. - probe-lease-dispatch-wiring: "the production limiter is reachable" is precisely the claim a regex answers badly, since an import can exist while the call site is dead. A real dispatch now has to move the shared limiter's counter. No production file changed and no test seam was added - every observable already existed. Ordering is done with deferred promises, not wall-clock sleeps. No local suite, focused test, typecheck, build, or install was run. * test: narrow the two preview cases to the read they actually fence Hosted CI on the first attempt reported two real auth.json reads where the case expected none, and a 404 where it expected the encrypted-recovery path. Both were fixture faults, and chasing them found something worth recording. The reads are genuine request-path reads, not setup noise: prepareResponsesRequest's explicit account preview reaches auth.json through previewCodexAccountForRequest -> pickPriorityPreemption -> getEligiblePoolAccounts -> isCodexAccountUsable -> isMainAccountCredentialUsable, and applySubagentModelFallback then runs poolAccountPreview over the same path a second time. So a request owning its own credential still consults physical-main LIVENESS twice, through pool eligibility. That is a wider gap than the fence these cases were converted from, which guards the denial-cache credential validation in model-entitlements.ts. Rather than quietly widen the assertion to today's number, the two cases now say what they check - main is excluded from denial-cache credential validation - and assert it by recording a stack at every read and requiring none to pass through model-entitlements.ts. Removing the preview ownership exclusion still turns them red, and now the failure names the path. The 404 was native-model reservation: a bare gpt-5.6-sol needs a provider configured under the name openai, and defaultProvider routed is never consulted for a bare native model. The pool-liveness reads are left as a finding for a separate change; widening the fence to cover them is an auth-boundary change and does not belong in a test conversion. * test: reach encrypted recovery through the response, and name what it covers The previous attempt configured a noncanonical baseUrl under the provider named openai. That provider's endpoint is fixed, so the setting was discarded with a warning and the recovery path never ran - recoveryCalls stayed 0. Recovery is driven by the upstream RESPONSE, not by the URL, so the fixture now provokes it the way the existing combo recovery suite does: canonical OpenAI receives the encrypted assignment and answers 401, the routed backup is ineligible while the ciphertext is unreadable, combo failover invokes assignment recovery, and the decrypted plaintext is replayed to backup.example. The case asserts one recovery call, one backup request carrying the recovered assignment, and zero physical-main reads after the recovery boundary. It is renamed to say what it actually covers. Combo children set comboAttempt, so this response-driven flow does not execute the direct recovery re-preview at request-prepare.ts:713. Calling it that would be the same false confidence this whole change removes, so the name is narrower than the one it replaces. No local suite, focused test, typecheck, build, or install was run. * test: drain what the new combo path leaves open The previous commit made this file provoke real combo failover and encrypted recovery. That reached a code path the earlier cases never did, and it left process resources behind: postSpawn never drained the response body, and handleResponses holds its translator budget until EOF or cancellation, so the stream lifecycle stayed open past the case. The completed recovery path can also schedule a response-state debounce, and combo selection state and cooldowns survived teardown. macOS shards run every file in one Bun process with bounded parallelism - --isolate rebuilds globals and module graphs per file but does not reclaim outstanding process resources - so the cost landed on siblings. Two unrelated cases in other files timed out at 30.09s and 20.77s where they had taken 8.36s and 8.77s on this branch's own baseline. postSpawn now drains every body it returns, and teardown clears combo selection, combo cooldowns and response-state memory and persistence. The rest of the cleanup - fetch, Date.now, the filesystem spy, native-main recovery blocking, account state, recovery-cache flights and timers - was already correct. Worth recording: those two cases were already marginal. A recent dev run had the cancellation case at 20.58s against its 30s bound and the memory case at 11.79s against 20s. This change removes the burden that pushed them over, but the margin itself is a separate finding. No local suite, focused test, typecheck, build, or install was run.
…each legible (#4851) * ci(windows): restore the margin the six-shard leg lost, and make a breach legible Every Windows dispatch had become a coin flip against the 30-minute job wall. Measured wall time per shard over the last seven lane=all dispatches, in minutes: run 35168946544 30.2 CANCELLED 20.8 20.3 21.3 13.4 21.0 run 35164979005 23.6 22.5 20.4 23.3 13.8 16.6 run 35161399172 23.5 24.7 23.7 26.8 17.2 17.3 run 35152226272 16.5 18.6 20.3 13.7 20.4 24.8 run 35148850553 18.6 20.0 24.8 14.2 23.9 22.8 run 35139132889 21.7 18.8 23.8 18.9 24.7 20.5 run 35134620067 20.5 20.3 19.9 16.8 24.0 28.3 13.4 to 30.2 against a 30-minute ceiling. A shard killed at the wall reports cancelled - neither a pass nor a fail, and with no indication of which file was running when it died. This is the third time this leg has grown into its ceiling; ci.yml already records the first two. One leg reached 30 minutes and died in cleanup, four shards then ran 17-25 minutes with a green 3/4 cancelled at 25m12s, and six were chosen to put each leg at two-thirds of that. Six has now done the same, helped by a suite that keeps growing and by #4835 re-enabling a family that had been skipped. Nine shards, arithmetic in the workflow: total observed work is about 133 minutes, so nine legs project to 26.5 minutes including the ~1.43 slowest-shard skew and the 25% run-to-run variance this file already documents; eight projects to 29.8, which is not margin. The ceiling stays 30 minutes, because raising it is the masking answer and the number is supposed to mean something. The cost is three more concurrent Windows runners and their fixed setup. Cutting work per shard buys time but does not make a wedge readable, so this leg now runs through the same batch runner Linux uses: at-most-12-file processes with a 120-second bound. A timeout or crash fixes the shard red immediately and names the batch; the singleton sweep that follows is diagnosis only and cannot turn it green, exactly as #4837 established. scope=all keeps all 1327 Windows files - Linux alone excludes the storage-policy and api-usage families because separate jobs own them. The aggregate gate counts the nine legs by name through the Actions API. A matrix rolls up to success when a leg never starts, so counting is the only way to know the dispatch produced the evidence it was run to produce. No local suite, focused test, typecheck, build, or install was run. * ci(windows): size the batch bound from Windows data, not Linux's The first attempt gave this leg Linux's batch settings unchanged - 12 files, 120 seconds - and 7 of 9 shards went red on dispatch 35171877721. The runner reported it precisely: "batch 5 timeout failure (exit 124)" followed by "every file passed alone, so the timeout lives in multi-file process state". That second line is the report you get when a bound is simply too small, not when something is wedged. Windows is the slowest hardware on the board, which is the whole reason this leg needed nine shards; a bound copied from the fastest one was never going to hold. Measured across 58 completed batches in that dispatch: median 39.1s, p90 92.6s, p95 100.1s, max 105.8s, and seven batches reached the 120s ceiling. The bound sat at roughly the mean, so about half of all batches were always going to breach it. Six files per batch with a 480-second bound. The sizing case is one naturally slow file: codex-inject-integration.test.ts passes in 312.0s and 317.6s in green runs, so its six-file batch projects to 337.4s, and 421.8s with the 25% run-to-run variance this workflow already documents. 480 leaves 58.2s over that. Six-file attribution halves topped out at 148.0s, so every other batch has an enormous margin. Linux keeps 12 files and 120 seconds. That number is correctly sized for that hardware and sharing one constant across two very different machines is what caused this. The two numbers are independent. Batch size and bound decide how quickly a wedge is named; the nine-shard split decides total wall time. Six-file batches add 12 processes per shard at a measured 0.106-0.168s of wrapper overhead each, about 2.1s per shard, so the margin arithmetic in the shard comment is unchanged. A real wedge now fails within eight minutes naming at most six files, with singleton attribution after the shard is already red. No local suite, focused test, typecheck, build, or install was run. * test(ci): stop the batch oracle from discarding a one-file primary batch The new scope=all case failed expecting three batches and seeing two, and the interesting part is that the runner was right and the test was wrong. batchCalls() classified every invocation beginning with "1|" as singleton attribution. Seven fixture files at batch size three is a valid primary sequence of 3, 3, 1 - so the oracle threw away the last real batch and then reported the count it had just corrupted. A test that miscounts and then asserts its own miscount is the same false confidence this branch has been removing elsewhere, so the fix is the oracle, not the number. It now asserts the exact primary sequence 3, 3, 1, checks the runner's own summary line for seven files in three processes, and still requires the dedicated file to appear. Windows coverage was verified independently rather than assumed, because a scope that silently dropped the dedicated families would be exactly the silent loss this round exists to prevent. From dispatch 35174148018: 1327 test files in the repository, 1320 in general scope, 7 dedicated; the Windows legs ran 148x4 + 147x5 = 1327, and the logs show all seven - tests/server/api-usage.test.ts and the six storage-policy files - executing across shards 3 through 8. That dispatch also carried the calibration result: nine Windows shards, all green, at 10.3 12.2 12.5 13.1 13.6 13.8 15.0 15.1 17.1 minutes against the 30-minute wall, against a six-shard spread of 13.4 to 30.2. No local suite, focused test, typecheck, build, or install was run.
…nal (#4859) `a contender with a deadline waits for the holder instead of failing immediately` went red on dev (run 35177450461, job 105062310488) with the HOLDER reporting busy, which should have been impossible: the parent had already seen its hold marker. Both facts were true. The marker was written from inside the lock callback, so it proved the child had ENTERED the section, not that it finished holding it. The child's coordination database lived under the ambient OPENCODEX_HOME, which every file in the same CI batch shares, so another test reading it could turn the holder's COMMIT into SQLITE_BUSY. With timeoutMs 0 the child had no retry, the acquisition rolled back, and it returned busy after having already published the marker the parent was waiting on. The 150ms sleep was the second half of the same problem. It was standing in for "the waiter is now actually waiting", and nothing made that true - on a loaded runner the parent could release before the contention it exists to measure had begun, which would also have made the waitedMs > 0 assertion a coin flip. So: the child's database moves to the per-test temp root, which removes the cross-file contention entirely; the waiter runs as its own child and publishes a wait marker only after confirming its lock promise did not settle synchronously; and the parent releases the holder only once it has seen that marker. waitedMs now comes back from the child, so the property the case exists to prove is guaranteed by construction rather than by timing. No other blind sequencing remains in this file - every other wait is the existing waitFor, which polls an observable condition. No local suite, focused test, typecheck, build, or install was run.
…igits "502" (#4860) Windows shard 4/9 of run 35180376537 went red on a turn that had succeeded perfectly. The relay stamps every chunk with a random chatcmpl-<hex> id, and this run drew chatcmpl-05021785ecf5440c96ca31be. expect(text).not.toContain("502") searched the whole stream, found those three characters inside the id, and failed. tests/images/loop.test.ts already retired the identical assertion for "504" and measured it: roughly one id in 137 contains a given three-digit string, which reddened about one run in 69 for no reason at all. The assertion could not do its job either. This relay's failure mode carries an error frame and ends the turn; the number 502 never appears in the body, so a real failure would have slipped straight past it. It was simultaneously flaky and blind. Assert the terminal shape instead: the stream reached [DONE] and carried no error frame. That detects the failure the case was written for and cannot be moved by a random identifier. No local suite, focused test, typecheck, build, or install was run.
…one flag (#4862) `ocx config show` timed out at its 40-second bound on Windows shard 6/9 of run 35182806140. The same case took 879ms and 1673ms in the two preceding dispatches, so this was a 25-45x outlier rather than a chronic cost, and a bigger budget would have been the wrong answer twice over. The obvious suspect was Windows ACL hardening, and it is not that: `config show` reads through readConfigDiagnostics and never calls loadConfig, the failing job carries no ACL diagnostic line, and the ACL-free `config get` case beside it also took 16.5 seconds. What both share is cold module loading. The command was importing the whole connect, lifecycle and catalog graph for one thing: deciding whether the `_remoteHub` annotation should say connected. On a cold Windows process that import is most of the command's cost, and it can drag the lifecycle recovery path in behind it. It now derives that annotation from the validated client record and the bounded service-token reader, without importing ./connect, without catalog readiness work and without entering lifecycle recovery. Nothing about the security boundary changes: no writer moved, and no `required: true` ACL call was touched. If the read-path reasoning is wrong the worst case is an inaccurate `_remoteHub.connected` display during an unusual recovery state - secret permissions and persisted bytes are unaffected. No local suite, focused test, typecheck, build, or install was run.
* test(web-search): drive the connect deadline on a virtual clock `fast headers plus raw byte progress can outlive connectTimeoutMs` timed out at 1002.37ms against its own 1000ms ceiling on macOS 2/2 of run 35186268515. Its siblings in the same describe run in 2.58, 3.48, 2.57, 3.06 and 14.65ms, so this case was three orders of magnitude slower than everything around it and sitting 2.37ms outside a bound it could not reliably clear. It was spending that second on five real 12ms waits used to walk a real deadline forward. Real sleeps and a real ceiling race the same scheduler, so runner load decides the outcome - which is how a case about timeout SEMANTICS became a case about how busy the machine was. It now drives the existing clearableDeadline seam with a virtual elapsed counter. The first byte lands at a virtual 26ms against a 25ms deadline, which is the exact condition the case exists to describe: headers that arrive fast must clear the deadline before the body is consumed, and continuing byte progress must not re-arm it. Body, cancellation, status and completion assertions are unchanged. The 1000ms ceiling stays exactly where it was. Nothing was widened; the wall-clock wait was removed instead. Ablation: leave the deadline armed, or move its clear to first-byte progress, and the abort lands before "a" is enqueued, so the body and completion assertions fail. No local suite, focused test, typecheck, build, or install was run. * test(web-search): fit the virtual-clock rewrite inside the ratchet cap The rewrite took this file to 2857 lines against a 2823 baseline, so the repository file-size ratchet failed and took test 1/4 and macos 2/2 with it. Compacted to exactly 2823. Every assertion, the virtual deadline semantics, the ablation and the 1000ms ceiling are unchanged; only the expression is tighter. The baseline itself is untouched. Editing the cap to fit a change is the same move as widening a timeout to fit a slow test. No local suite, focused test, typecheck, build, or install was run.
…nst itself (#4876) dev went red on windows 4/9 of dispatch 35191675127, and the cause is mine. #4851 replaced the Windows leg's single `bun test` invocation with 25 sequential batch invocations. tests/preload.ts takes a user-scoped machine-local lock, and line 101 makes it win32-only, so Linux and macOS have run this same batch runner unqueued for a long time while Windows had never batched at all. The lock joins workers that share a run ID and blocks anything with a different one, so each batch now queued behind the previous batch's stragglers: [test] bare Bun worker 2548 is waiting for test run pid 7272 to release the user lock. ##[warning]Bun test process timed out after 480s in shard 4/9 batch 6/25. ##[error]Shard 4/9 batch 6: every file passed alone The first file of that batch then ran in 11.25s during attribution. The eight minutes were queue, not work, which is why a larger bound would only have hidden it for longer. scripts/test-run-lock.ts already names this case in its own timeout message: set OCX_TEST_NO_QUEUE=1 only when overlapping test runners are intentional. A dedicated CI job running its own batches back to back is one logical test run, so the batch step now sets it. What the queue protects against - an unrelated second suite stacking load on a developer's machine - cannot happen in that job, and each batch still creates its isolated home and arms the live-home and service-manager guards before it would have reached the lock. The lock's own unit tests now pin an explicitly queued environment, so the workflow bypass can never silently turn their acquisitions into no-ops. That was the real risk in disabling a guard by environment variable. The 480-second batch bound and the 30-minute job ceiling are unchanged. No local suite, focused test, typecheck, build, or install was run.
…act (#4882) Record the parent-relative delta of each stage in the four-deep native\nWebSocket control stack, state the stages as one checkable contract,\ndetermine in code whether the native-main read fence gates them, and set\nthe order in which activation should be decided.
Six narrow contract defects that share no code path, one pull request each. Records what each unit may not change, and two repository gates checked against the tree rather than assumed: the file-size ratchet pins gui/src/pages/Models.tsx at its current 2792 lines, so the carried custom-model validation cannot add a net line, and enforce-target requires a GUI screenshot this lane cannot produce.
The reference entry opens with "Key-auth openai-chat providers only", which an operator reasonably reads as "every other adapter gets no transient retry". That is not what happens. A provider whose adapter is openai-responses never reaches transientRetryPolicyFor at all. createResponsesPassthroughAdapter declares passthrough: true, and core.ts returns into executePassthroughResponse on that flag before the adapter dispatch path is built. The passthrough lane then applies its own transient ladder from a fixed constant, so such a provider gets replay the setting can neither enable nor tune. Both halves of that mismatch mislead: the option looks broader than it is, and the untouched lane looks quieter than it is. One sentence in each locale states the boundary. Documentation only. No runtime behaviour changes.
…udget (#4865) * fix(adapters): charge adapter-owned retry sends to the request send budget mimo-free's 401 JWT replay, command-code's reasoning-effort repair, and the google-http transient loop each issued bare fetches that never touched ctx.sendBudget, so a request holding only its final recovery permit still dispatched and a refused retry still paid the backoff sleep. Route each physical send through createAdapterPhysicalSend: admission precedes pacing, backoff and superseded-response cancellation, a credential hop's pending permit pays for the first send exactly once, and a refused retry returns the real upstream response instead of a synthetic error. Follow-up to #4621. * test(layout): map physical-send.test.ts to the adapters domain * fix(mimo-free): drain the 401 body before the JWT refresh can reject The 401 replay moved its drain behind admission so that a budget-refused replay can still return that same response with a readable body. Inside beforeDispatch it ran last, after resetMimoJwtCache and getMimoJwt. getMimoJwt issues its own bootstrap request and rejects on a failed or oversized response. When it did, fetchResponse threw and the 401 body was never released - a leak the pre-change code did not have, because it cancelled first and refreshed second. Draining first WITHIN beforeDispatch keeps both properties: it is still after admission, so a refusal returns the untouched response, and it no longer depends on the refresh succeeding. --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: JUN <bitkyc08@gmail.com>
…4881) * docs(devlog): plan the L3 retry, admission and combo-recovery unit * docs(devlog): quote the mimo drain line with its comment intact The hygiene scanner reads added lines in every changed file, markdown included, and the abbreviated snippet carried a literal empty catch. Quoting the real source line keeps the point and clears the rule.
…ird-party upstreams (#4884) Codex 0.155 attaches a top-level access_programs object gated on ChatGPT auth rather than on the destination URL, and loopback injection keeps the client's openai provider identity, so the field rode along to every routed destination. A gateway that validates its top-level schema rejects the request before inference, which broke every turn on OpenCode-Go's muse-spark models. Add CANONICAL_ONLY_TOP_LEVEL_FIELDS beside CANONICAL_ONLY_TOOL_FIELDS and apply it at the existing private-field boundary for destinations OpenCodex does not operate. Scoped by destination rather than by the canonical surface so the native /responses/compact path, which spreads the raw body and is offered only to OpenAI-operated destinations, cannot disagree with /responses for the same provider. codex_output_schema is not listed: in codex-rs it is the name of the text.format JSON schema, not a top-level key. The table stays limited to keys a client is observed to send so this does not become an unknown-parameter sanitizer. Closes #4853
The Astro toolchain had no pull-request build gate. ci.yml contained no docs-site reference and built no docs, and deploy-docs.yml triggers only on push to main, which is after promotion. The first machine that could discover a broken docs build was the deploy, so a dependency bump under docs-site/ could only be backed by an author's local run. Adds a docs filter to the existing changes job and one job selected by it. docs-site/** is deliberately not added to the ci filter: a prose edit has no business starting the cross-platform matrix, it only has to build. A separate filter output keeps those two questions apart. The workflow file itself is in the docs filter so an edit to the job verifies itself, without which this change's own pull request would skip the job it adds. One Linux leg. The site is static output from a Node/Bun toolchain with no OS-specific behaviour to promise, so another platform would spend queue time without buying coverage. --frozen-lockfile carries as much of the value as the build does, because it fails on a manifest and lockfile that disagree, which is the shape a hand-edited override introduces. The aggregate ci gate is event-aware, so the job is declared in the four places that have to agree: needs, CHANGES_DOCS, GATED_JOBS, and expected_for. No existing job's sharding, timeout, or runner selection changes, no Windows leg is added, and permissions stay contents: read.
…view outcomes (#4892) The native /responses/compact path spreads the caller's raw body without passing through the Responses adapter, and is offered only to OpenAI-operated destinations. Scoping the top-level strip on the canonical surface would have made openai-apikey behave differently on its two endpoints, so the boundary is the destination instead. Records why, since the document argued the opposite. Also records what the two reviews found: the adjacency flag was doing double duty and would have inserted placeholder tool turns into Kimi conversations, and the ollama-native replay change has no correctness defect.
* docs(devlog): plan safe teardown and honest settings application [skip ci] Opens unit 260917_l2_safe_teardown for issues #4812 and #4809. #4812: the Codex history preflight answers "may I rewrite conversation history?" and four restore call sites use that answer to decide "may I remove OpenCodex routing from config.toml?". Since the refusal triggers on the mere presence of a history_mode column, which every current Codex build has, the config half is permanently unreachable and uninstall can remove the proxy while leaving routing pointed at it. #4809: the two Desktop switches persist and report success without rewriting the injected config.toml, and report the configured value rather than the effective one on a bind where the flag is dropped. The plan records the upstream provider-resolution facts that make a degraded "strip root routing, retain the provider table" restore safe, and that make the opposite ordering fatal to config load. No product code changes here. * fix(codex): restore routing without waiting on paginated history Closes #4812. preflightCodexHistoryInjection answers one question — may conversation history be rewritten — and four restore call sites used that answer to decide a different one: may OpenCodex routing come out of config.toml. The refusal triggers on the mere presence of a history_mode column, which every current Codex build has, so the config half was permanently unreachable. ocx uninstall removed the proxy and left config.toml pointed at it, and every subsequent codex invocation failed on a dead port with no supported recovery. The guard is right about history and is unchanged. It was being asked the wrong question. On history_paginated_requires_native_writer, and only that reason, restore now removes every OpenCodex root routing key, retains [model_providers.opencodex] verbatim including its ownership marker, and skips the history relabel rather than attempting it. Every other refusal reason keeps its hard refusal and compensating rollback untouched. Retention is what makes the split safe. In codex-rs, Config::load resolves model_provider_id as (thread override) ?? cfg.model_provider ?? "openai" and returns NotFound on a miss, which fails the entire config load rather than one thread; a resumed thread supplies its persisted provider as that override. Keeping the table therefore leaves tagged conversations openable — their requests fail against the stopped proxy with an ordinary connection error — while plain codex returns to the built-in provider. Removing the table while the root selector survived would break every codex invocation, so the table is captured before the transform and re-appended into the same buffer, reaching disk as one write. The capture cannot reuse removeOcxSection's scan. That one opens a section on any line carrying the ownership marker, which is safe only because the root openai_base_url marker has already been consumed by the time it runs. Capture reads the untouched file, so it anchors on the provider header instead and pulls in an immediately preceding marker as its comment. A stand-down can also appear during the write, after the pre-write check said otherwise. The table is therefore captured unconditionally and re-attached once after the write, and the history decision is re-asked at the same point so a Worker whose own preflight is now guaranteed to refuse is never spawned. Reported as artifacts.config.state "partial" with action routing-restored-provider-retained, carrying the retained lines and the follow-up command. historyPreflightRefusal stays unset: it still means nothing was attempted, and a stop obligation that was discharged must release its receipt. restore, stop, uninstall, the service subcommands, the stop API and status all name the retained table and the skipped relabel, so a partial outcome is never printed as a plain success. ocx restore --remove-codex-provider-table removes the table for a user who accepts that tagged conversations stop opening. Nothing selects it implicitly. Verification: hosted CI only. The local suite, typecheck, build and the ocx binary were deliberately not run for this change. * fix(codex): keep the new teardown helpers off the inject facade The file-size ratchet caught src/codex/inject.ts at 998 lines against a committed cap of 987: the previous commit had added nine passthrough re-exports and a comment to it. That cap is the right answer here rather than an obstacle. The new helpers are teardown reporting, owned by inject/remove.ts and inject/restore.ts, and routing them through the facade added nothing except a second name for each. The three consumers import from the owning modules instead, and the dangling comment left behind when HISTORY_RELABEL_STANDS_DOWN moved to history-provider.ts is removed with it. No behavior change. * fix(codex): let an explicit provider-table removal past the history preflight CI caught it: "paginated sync/async restore removes every root route and honors removeProviderTable" failed with success false. removeCodexConfig re-derived the history question with its own preflight and keyed the refusal on whether the table was being retained, so the one path that legitimately removes the table on a paginated home — the explicit --remove-codex-provider-table opt-in — was refused by a check its caller had already answered. Retention and the refusal are the same decision seen from two sides, so they are now one option instead of two booleans. The caller passes down what it resolved: refuse-on-any, stand-down-retain, or stand-down-remove. A direct caller that passes nothing keeps the original refusal on every reason, and only the stand-down reason can be resolved this way — every other reason still means the history state itself is wrong. * test(stop): pin the restore success branch by shape, not by one line CI caught it: the source-oracle assertion in grok-lifecycle.test.ts expected the literal text "if (result.success) console.log", and that branch grew a body when a degraded restore had to report the provider table it retained. Pinned as two assertions instead, plus the preflight-refusal conjunction that decides whether a stop receipt stays owed. A degraded restore discharged its config obligation and must not be read as a deferral, so that condition is now part of what this test holds rather than something it happened to cover.
…ad fence (#4883) * docs(devlog): plan the L1 preview read fence and dependency audit lane Records the roadmap for two independent safety units: closing pool eligibility inside the caller-owned preview read fence (#4850), and reviewing the existing dependency audit override PR (#4873). * fix(codex): close pool eligibility inside the caller-owned preview read fence A `thread_spawn` authenticating with the caller's own forwardable Codex bearer still opened the operator's physical native-main `auth.json` during request preview, twice per request. `request-prepare.ts` already computed the ownership fence and honoured it for quota priming, entitlement discovery, and the denied-model cache. It did not reach pool eligibility: `previewSelectionOptions` carried no `isMainAccountTokenLive`, so `codexAccountUnusableReason` fell through to its default `isMainAccountCredentialUsable()` and read the file. The second read came from subagent fallback re-entering the preview through its callback with the same options object, and the post-decryption recovery re-preview had the same omission. No token was disclosed, refreshed, or sent: final authentication passes its own synthetic liveness and never selected physical main for these requests. What leaked across the boundary was influence -- operator-main liveness, cached quota, and plan state could score into the decision to rewrite a subagent's model for a request that owns its credential. ADR-0086 already rejected reading the physical main token for identity. Use the existing `CodexAccountUsabilityOptions.isMainAccountTokenLive` seam, scoped to `previewRequestScopedMainCredential`, and give it the value final authentication computes rather than a constant. An effective main pin means the request really is served by its own main credential, so preview must score main eligible; without the pin final authentication drops main from pool eligibility and preview must drop it too. The predicate moves into one exported `requestOwnedMainPinState` that both callers share, because two copies of this fence is how the gap appeared. Regression coverage asserts the read count rather than the outcome. "The right credential was eventually sent" was already true while the defect existed, so only an unfiltered zero-read assertion distinguishes a closed fence from a lucky result. A no-bearer request is asserted to still read and serve physical main, so the fix cannot be satisfied by making main globally ineligible, and the #3166 main-pin path is asserted to stay on main and stay read-free. Closes #4850 * test(responses): order and scope the new read-fence cases correctly Three corrections from the first hosted run, all in the new tests rather than in the fix. The case that proves the fence itself -- zero `auth.json` reads for a caller-owned spawn -- passed and is unchanged. Both cases that let a request reach native main now run last. Observing a main credential writes module state in `main-account-cache.ts` that nothing in this file resets: `beforeEach` rebuilds `OPENCODEX_HOME` and the read counters, not that cache. Running them earlier made the existing recovery/drain case see three reads it does not make on its own. The no-bearer case no longer selects and materializes main. It asserts what it was actually there to assert -- that a preview owning no credential still probes physical main liveness, so the seam cannot have been applied to every preview -- without depending on which account ends up serving. The main-pin case drops its read-count assertion and says so. The pin path does reach the physical credential elsewhere in the request; the guarantee under test is that preview and final authentication agree on the pin, which the resolved context and the untouched model already establish.
… CVEs (#4873) * fix(deps): bump hono, astro, and docs-site overrides to resolve audit CVEs * fix(deps): exact-pin hono override to 4.13.8
…4899) * fix(settings): apply the Desktop switches and report effective state Closes #4809. --desktop-authless and --client-compaction only mean anything through the injected config.toml, so persisting them was never applying them. PUT /api/settings persisted and then converged the catalog, under a comment asserting that the injector re-read config and rewrote the form. convergeCodexCatalog rejects any scope but "catalog" and never reaches injectCodexConfig, so the injected shape stayed as it was until a separate ocx sync. The CLI discarded the response body and printed a fixed "System settings updated.", so nothing said a second step was owed. A quieter failure sat beside it. On a non-loopback bind without unauthenticatedLoopbackListener, standaloneCodexRoutingTarget drops the authless flag, while both GET and PUT reported the configured true. The user read back the value they set and got the behavior they did not. The route now runs the real injection, after catalog convergence and after the config mutation transaction has closed. That ordering is not incidental: coordinated Codex writes take the Codex write lock before the config mutation lock, so awaiting the injector inside that transaction would invert it, and the injected model_catalog_json should point at a catalog that has settled. Each switch is reported as three separate facts — the stored value, the effective value this bind and role will actually produce, and whether config.toml was applied, with the reason and whether a retry is worth trying. Effective values come from isEffectiveCodexDesktopAuthless and a new isEffectiveCodexClientCompaction beside it, rather than a second copy of the predicate, because the reporting answer and the injection answer diverging is the defect itself. The report also states the auth-source consequence. The flag decides requires_openai_auth in the injected provider table, which upstream reads to decide whether to ask the user to sign in at all, so flipping it changes whose identity is in use. The user is told while they are making the change. CodexInjectResult gains a declared retryable field. codexInjectLockOutcome has always set it for a busy write lock and the type has always omitted it, so a caller wanting to separate "someone else is writing right now" from "this will fail the same way forever" had to reach for an undeclared field. history_paginated_requires_native_writer is not a failure on this path: apply already stands the relabel down and writes the config half, so a paginated home applies normally. The pre-existing top-level codexDesktopAuthless and codexClientCompaction booleans keep reporting the configured value; the report is additive. The stale comment is rewritten to describe what the code does. Verification: hosted CI only. The local suite, typecheck, build and the ocx binary were deliberately not run for this change. * test(settings): send the Host header the management origin check requires CI caught it: the new write-lock-busy test got 403 instead of 200, so the settings handler was never reached. managementRequestOrigin derives the allowed origin from the Host header, and an in-process `new Request` carries none, so isAllowedManagementOrigin rejected it as cross-origin before any route ran. The passing in-process pattern in settings-main-account-hard-lock.test.ts sets `host` explicitly; this now matches it. * test(settings): call repoRoot instead of passing the function as cwd CI caught it: four isolated-home settings tests died with ERR_INVALID_ARG_TYPE, "Received function repoRoot", before the subprocess ever started. repoRoot is a function in tests/helpers/repo-root.ts; spawnSync needs its return value. * test(settings): send Host from the isolated-home settings subprocess too CI caught it: the four isolated-home cases still returned 403 once the cwd bug was out of the way, for the same reason the in-process case did. managementRequestOrigin derives the allowed origin from the Host header, a constructed Request carries none, and isAllowedManagementOrigin rejects the request before any route runs. The in-process case was fixed one commit ago; this is the same fix inside the subprocess script. * test(status): guard the human status probe the way the json one already is The stale-record fixture spawns the CLI twice and knew the hazard: a /healthz probe can abort without ECONNREFUSED while the port is genuinely empty, which leaves staleProcessState false with nothing having taken the port. It retried on that for the --json run and not for the human run. The asymmetry was the defect. The human run makes the identical probe and can abort the identical way, so a human probe that timed out instead of being refused printed no stale line and was read as a lost signal — the same misreading the existing comment describes, one run later. This is what failed test 2/4 on a branch that touches no status code, and it is a dev defect rather than one this branch introduced. The guard is now symmetric, and it stays conditioned on an observed abort rather than on the assertion outcome. After the human run the structured verdict is re-sampled under the conditions that run just saw: true means the probe path was reaching a refusal, so the human output is a valid sample and the assertions judge it, and a human path that genuinely stopped reporting the stale line still fails. False while the port is still refusing is the documented abort, so that attempt is discarded. All three assertions are unchanged.
…eout (#4902) * test(windows): size the first-child readiness wait for cold start, and report dead children Two Windows shards failed on two different files during heavy queue contention, and both are the same defect: an in-test deadline that has to cover a Windows cold first child start, sized for an idle runner. run 35211904734 windows 3/9 failed at waitFor(holdMarker), not at the contention assertion -- that line was never reached, and holdMs was never approached. run 35210400258 windows 7/9 failed at the first wait of its case, for "listening", with no events at all. dev at 6d19a07 passed all nine Windows shards on an idle queue (run 35215552842), so neither is a defect in the code under test. This is not a number raised to make red go away. The deadlines were smaller than the range this repository has already measured for the wait they bound. COLD_SPAWN_BUDGET_MS records a first child publishing at 50.7s while the next spawn in the same file was ready in 1.76s, with surviving readiness waits from 2.0s to 19.7s; codex-write-lock.test.ts says in its own comment that a holder child boots in 8-19s on a loaded shard, and bounded that wait at 15s. A case the file calls normal is slower than the deadline it has to meet. Each file now spends that named ceiling exactly once, on its first child, as the constant's own contract requires; every later wait keeps the ordinary bound. The second defect is that neither wait observed the child. A child that died and one that was merely slow produced the same message, so CI could not tell them apart, and the two want opposite fixes. Both waits now race the exit and report the code and stderr immediately. That is also what makes the native-main evidence readable: its stderr promise resolves at EOF, so an empty stderr in that message means the child had already exited silently rather than that it was still booting. The timeout branch in the owner harness also awaited that same EOF promise while claiming the child was still running, which could never settle. It is bounded now. Closes #4901 * test(windows): drop the readiness-budget change, keep the child-death diagnostics The evidence this PR's budget argument rested on turned out to be an artifact of how it was produced. Both evidence branches were dispatched from refs whose merge-base predates #4876, and workflow_dispatch reads the workflow from the dispatched ref, so those runs used a ci.yml without OCX_TEST_NO_QUEUE and reproduced exactly the batch-serialization bug #4876 had already fixed. The batch-4 log shows it directly: one line saying a bare Bun worker is waiting for another test run to release the user lock, then eight minutes with no (pass) at all. Confirmed on the branches: OCX_TEST_NO_QUEUE appears 0 times in codex/ci-evidence-4875's ci.yml and once in dev's. So whether these deadlines are actually too small is undetermined again, and the COLD_SPAWN_BUDGET_MS use is withdrawn. Both files keep their original bounds. What does not depend on that evidence stays. Neither wait observed its child, so a child that died and one that was merely slow produced the same message; both now race the exit and report the code and stderr immediately, re-checking the awaited signal first so a child that signalled and then exited is not misreported. And the owner harness's timeout branch awaited a promise that only resolves at EOF while describing a child that is still running, which could never settle; it is bounded now. Those are diagnostic defects on their own terms, and they are what would have made the original evidence readable in the first place.
… dropping it (#4890) * fix(models): reject an invalid custom-model context window instead of dropping it Carries PR #4863 onto current dev. Typing a k-suffixed value such as 350k into the Custom Model dialog produced NaN, so the field was dropped on add or sent as null on edit while the dialog closed with a success toast. Co-authored-by: liangbo <liangbo.yejc@bytedance.com> * fix(gui): keep custom context validation within size cap * Keep the rationale for the custom-model context guard inline The guard rewrite that fit the file-size ratchet dropped the note explaining why an invalid draft must not read as "field omitted / override cleared". Restore it as a trailing comment, which costs no line. --------- Co-authored-by: liangbo <liangbo.yejc@bytedance.com>
…4875) * fix(cursor): remint conversation after incomplete tool-call streams (#4874) Cursor fail-closes a truncated client-tool stream but kept the same conversation id, so the next turn resumed a session left waiting for mcpResult. Remint after the streamed error, persist the thread override, and synthesize a missing tool_result on native Composer replay. Isolated helper turns stay fail-closed. * fix(cursor): bound incomplete-tool conversation remints Keep compaction and isolated turns outside incomplete-tool recovery, cap rotations per retained thread scope, and share the producer message prefix with classification. Preserve the blob-test size ratchet by relocating the existing replay regression. Co-authored-by: MerryEcho <xx59623633@163.com> --------- Co-authored-by: JUN <bitkyc08@gmail.com>
…threads (#4871) * fix(xai): seed Responses tool-result adjacency for interrupted Codex threads Grok 4.6/4.5 OAuth Responses replays Codex tool history. After a mid-stream 502/reset the client can resend a function_call without its output, or with hook-injected developer context between the pair. Google already synthesizes a missing tool_result; xAI did not, so later turns in the same thread 400. Reuse requiresAdjacentResponsesToolResults (Kimi #4726, DeepSeek #1292) and run the existing orphan-call placeholder for non-forward adjacency providers. Do not set statelessResponses: xAI stores responses for 30 days. Closes #4870 * fix(responses): separate tool-result pairing from adjacency Gating the missing-output synthesis on requiresAdjacentResponsesToolResults enrolled kimi and kimi-code in it too. Both carry that flag because their parser rejects a hook-split pair, but the same report (#4726) shows a call left without any result is accepted, so they would have started receiving placeholder tool turns for a shape they never rejected. Adjacency reorders items the upstream would accept in some order. Pairing synthesizes an item the client never sent, which is a larger claim about what happened in the conversation, so it gets its own capability: requiresPairedResponsesToolResults, seeded on xai only. statelessResponses still implies it, which is how DeepSeek already had the repair. Stateful behavior is untouched: store and previous_response_id survive, and forward auth still suppresses synthesis. * test(xai): pin the dangling custom tool call through the lowering it actually takes The previous case declared no custom tool, so collectRoutedCustomToolNames found nothing and the lowering never ran; it proved only that the repair indexes custom calls. xAI sets supportsResponsesCustomTools: false, so the production shape is a declared custom tool that gets lowered, and that is what this now asserts end to end. * docs(structure): note the xAI-only pairing capability on the provider page providers/xai-grok.md co-owns src/providers/ and says the surface retains its existing behavior, which stopped being true once a capability was seeded for xAI alone. Points at chat-compat.md rather than restating the contract in two places. * fix(server): classify the new pairing capability in the provider field policy PROVIDER_CONFIG_FIELD_POLICY is satisfies Record<keyof OcxProviderConfig, ...>, so adding a field to OcxProviderConfig without a policy row fails the whole typecheck from a different file. That is what broke gates and the production adapter contract test on Linux and Windows. editor matches the sibling wire capabilities (statelessResponses, requiresAdjacentResponsesToolResults, annotateEmptyToolOutputs) and matches what the policy means: the field is user-authorable through the leaf validator schema and is seeded from the registry only when the user has not set it. It carries no credential and is not a runtime observation. --------- Co-authored-by: JUN <bitkyc08@gmail.com>
… they now measure (#4843) The first round added the instrumentation; this one uses it. Run 35141541461 measured the Windows preparation window at 2740ms and the Linux/macOS one at 423-575ms, against a reserve that had been guessing at 52.7s. Boot drops 40s to 30s and the Windows preparation reserve 80s to 55s, which takes the case from 265s to 95s while still leaving room for the 52.7s outlier the reserve was written for: that much preparation still leaves the flip its full boot budget and its reap. The same run also showed every readiness wait in native-profile-startup at 2.0-4.9s on all six Windows shards, first child included, confirming the 50.7s observation behind #4830 was the ACL ceremony rather than a cold start. COLD_SPAWN_BUDGET_MS is kept as a bound against the part one run cannot rule out, with a note to delete rather than raise it. Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…f refusing the replay (#4848) Codex writes mid-turn context items inside a single turn: a PostToolUse hook verdict from hooks.json, a context notice. One of them routinely lands between the assistant tool_calls message and that call's own tool result, so buildNativeMessages saw a conversational message while the batch was open, flushed it, found a call without a result, and threw: ollama-native tool call call_x is missing its tool result; refusing interrupted replay Codex surfaces that local validation failure as 502 Provider unreachable, and the item order is part of the persisted thread history, so every later turn of the affected thread failed the same way and the task could not be resumed. The chat adapter already repairs this shape: src/adapters/openai-chat/messages.ts defers barrier messages until the tool round completes, reattaches the real results to their original call occurrence, and answers an unresolvable call with an explicit "no tool result was recorded" tool message. The native transport now does both, request-locally. The strict pair checks are untouched: an orphan result, a duplicate result for the same call, and a result naming another tool still throw. structure/providers/chat-compat.md records that the native wire now carries the chat wire's deferred-barrier contract, so the two cannot drift apart silently. Reported in #4842.
) * feat(responses): add opt-in native WebSocket steering * fix(responses): preserve steering continuations and register bounded owners * fix(responses): address native steering review findings Clear unstarted and superseded channel ownership, append bounded replay without argument spreading, and add six red-to-green regressions. Consolidate architecture notes into short links and document native control helpers. Preserve the maintainer-integrated dev branch without rewriting its history. --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…y after cooling (#4824) * fix(combos): allow single-target combo with waitForCooldownMs to retry after cooling * fix(combos): bound single-target cooldown retry and document loop termination * fix(combos): name single-target cooldown retry and document the wait contract * fix(combos): correct waitForCooldownMs terminal response in docs
Co-authored-by: 87003697 <870036797@qq.ocm> Co-authored-by: lidge-jun <lidge-jun@users.noreply.github.com>
…4908) The file-size ratchet fails on dev: tests/server/server-combo-failover-e2e.test.ts is 4207 lines against a cap of 4166. Neither contributing change was over the cap on its own branch. #4824 took the file from 4100 to 4153 and #4817 added 54 lines that computed to 4154 against the pre-#4824 file, so both were honestly green; the sum only crossed the cap once both were on dev. updateBaseline() stores Math.min(cap, lines), so the tool lowers a cap and never raises one. A GREW offence cannot be cleared by regenerating the baseline, and raising the number by hand is the one move the ratchet exists to prevent. Move the newest case into a sibling file instead, as d3ca552 did for the same file. The test body is moved verbatim. The new file carries only the part of the parent fixture this case uses: loopback upstreams, an isolated home, and the combo and request-log state that leaks between tests. It mocks no module, because this case drives the real openai-responses adapter. tests/server/server-combo-failover-e2e.test.ts returns to 4153 lines and the repository scan reports no offender. Co-authored-by: 404Unkown <52745108+87003697@users.noreply.github.com> Co-authored-by: agentHits <140916359+agentHits@users.noreply.github.com>
…4858) * feat(responses): relay native multi-agent function-result injection Follow up on #4782 with a separate default-off injection owner, bounded serial acknowledgements, same-account caller continuations, accepted-result replay and regression coverage. * refactor(responses): name the shared native control field nativeControl The field now holds either a steering or an injection owner, so the generic name matches the NativeResponseControl contract. Move the response-ownership markers next to the shared interface. Behavior is unchanged. * fix(responses): fail the turn on a native-control attach conflict instead of falling back to HTTP * test(responses): move the attach-conflict regression into ws-failure-stage --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com> Co-authored-by: luvs01 <luvs01@users.noreply.github.com>
…ed output (#4861) * feat(responses): support typed native result continuations and preserve hosted output Extend #4858 with rich/custom results and explicit approval continuations, execution-mode selection, structural replay matching and lossless sparse-terminal reconciliation. Keep unsupported inject and mixed-mode operations fail-closed. * fix(responses): reject continuations that omit pinned injection settings * fix(responses): refund exact injection batch bytes and use site-relative docs link * test(responses): follow the nativeControl field rename in continuation assertions --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
… replay output (#4911) * fix(responses): bound steering confirmation waits and preserve sparse replay output Separate monotonic acknowledgement, successor and tool deadlines; reconcile steering replay with completed wire items without weakening ownership or retry guards. Follow up on #4861. * fix(server): validate native control settings before superseding the active turn A malformed response.create frame cancelled the live turn before its steering channel was constructed, so a rejected frame could discard active work without recording a replacement. Build the channel first; only cancel after it validates. * fix(responses): reject malformed output_index in steering replay A response.output_item.done frame with a non-safe-integer index matched no branch and was silently dropped from retained output. Validate inside the branch and throw, matching the injection replay observer. * perf(responses): cache the parsed base frame across steering continuations sendControl re-parsed the full original frameText for every response.create continuation; a full-replay frame runs to megabytes. Hoist the parse and reuse the immutable base. * docs: name the canonical steering route and drop duplicated policy text State that native steering requires the canonical ChatGPT forward route, describe control deadlines as fixed rather than inactivity-based, and reduce the server reference paragraphs to a scope summary with the canonical guide links. --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
… and executable probes (#4912) * feat(responses): complete safe steering overrides, API transport and executable probe * docs: clarify steering routes and consolidate control guidance --------- Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
[WRONG BRANCH] release: promote dev to main for 2.58.0
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Contributor
|
✅ Deterministic PR hygiene checks passed. |
github-actions
Bot
changed the title
Contributor
⏳ DRAFT
What to do
Its title has been prefixed with |
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (233)
📝 WalkthroughWalkthroughThis PR bundles several independent lanes: experimental native WebSocket steering and function-result injection, Cursor adapter text-toolcall quarantine and remint budgets, Codex preview-credential-fence and safe-teardown fixes, adapter send-budget/retry hardening, provider registry/model-discovery updates, Windows ACL/subprocess reliability, CI workflow restructuring, and small contract fixes. ChangesCI Workflow and Test Infrastructure
Planning and Release Documentation
Native WebSocket Steering and Function-Result Injection
Cursor Adapter Stabilization
Codex Config Safety: Preview Fence and Safe Teardown
Adapter Send-Budget, Retry, and Combo Hardening
Provider Registry, Model Discovery, and Capacity Fields
Windows ACL Hardening and Subprocess Reliability
Small Contract and Quality Fixes
Estimated code review effort: 5 (Critical) | ~240 minutes ✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Owner
Author
리뷰 · 우선순위 78 / 80이 PR은 기능을 새로 넣는 요청이 아니다. 2.58.0 릴리스 트레인의 preview 프로모션이다. 이미 머지 후 트리를 다시 맞춰 봤다. 태그 현재 파일 수가 235개(+16163/-1454)로 크게 보이는 이유는 preview가 2.57.0대에 머물러 있던 것을 라인 단위로 “이 줄이 틀렸다”고 찍을 제품 패치는 없다. 아래는 경로·게이트·전제 위주의 점검이다. 경로 release/preview-2.58.0 → preview - 프로모션 base/head가 맞고, 이미 MERGED( 메인테이너의 판단이 필요한 지점
너의 추천 이 댓글은 grok-bot이 작성했습니다 |
3 tasks
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
6 participants
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
Promotes
maintopreviewafter the 2.58.0 release, matching the promotion that followed 2.57.0.git diff origin/main HEADis empty, sopreviewends up byte-identical to the published tree; the merge carried no conflict this time becausepreviewhad not diverged beyond the version line.Verification
The release SHA
6fe4cd0de8already carries a successfulCross-platform CIpush run onmain, which is the gaterelease.ymlitself required before publishing. The published artifact was produced from that exact commit:+ @bitkyc08/opencodex@2.58.0with a signed provenance statement, and tagv2.58.0points at the same SHA.Empty diff against
mainis the substantive check for a promotion, and it holds.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation