Repository navigation
chore(test): align Bun runner with the 1.4.4 security baseline - #7627
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Align Bun pins, validation, and promotion guidance package.json, tests/test-layout-tooling.test.ts, tests/ci-workflows/*, .github/actions/setup-project-bun/action.yml, .github/workflows/ci.yml, Dockerfile, CONTRIBUTING.md, docs-site/src/content/docs/contributing.md, structure/runtime.md |
The testRunnerBun pin changes from 1.4.3 to 1.4.4. Runner-selection tests cover matching and differing caller versions and an incorrect override. CI comments, tests, and documentation state that the runtime and test-runner roles are validated independently. They specify that promotion requires a successful full cross-platform lane=all run on the candidate head. |
Early TLS Response Upload Tests
| Layer / File(s) | Summary |
|---|---|
Exercise buffered uploads with early TLS responses tests/claude-integration/claude-picker-upload.test.ts |
The fixture adds an early-response TLS mode that records whether the request body is complete and whether the response flushes. A direct-origin test checks that the origin sends no reply before the first body byte, then returns a flushed 413 while the request remains incomplete. Buffered cases verify that the origin receives only part of the advertised 32 MiB body. |
Priority:
Estimated code review effort: 2 (Simple) | ~12 minutes
Change: Other
Possibly related PRs
- lidge-jun/opencodex#7606: This PR updates the
testRunnerBunpin to match the shipped Bun 1.4.4 runtime introduced in#7606. - lidge-jun/opencodex#7420: This PR adds the same early-FIN upload fixture behavior and checks for incomplete request bodies and flushed responses.
Merge Risk: 
Minimal · up to ba67d
-
No actionable merge-blocking risk is established by the reviewed changes.
Pre-merge checks: 
4 · 
1
-
Failed checks: 1 warning Passed checks: 4 passed -
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 @coderabbitai help to get the list of available commands.
|
Triage note (2026-10-24, against Needs fixes before this can land: resolve dev conflicts, preserve dependency repairs from #7642/#7644, then refresh required CI and lane=all on the resulting head. Obtain explicit workflow/dependency security review. Still needed on dev: partial — runtime 1.4.4 already landed in #7606 / Exact-head CI at triage time: Please refresh onto current |
luvs01
left a comment
There was a problem hiding this comment.
Follow-up to the triage request
Reviewed f011a94be1b468b7b785cb54976d4cb4a9b6a563 against dev@2494848f56e8a4f283722b2ebd816977e828c7af, which remains the current dev tip. The existing writer has already completed the normal dev merge and started fresh CI. No duplicate source change or workflow dispatch is needed.
Merge and dependency preservation
- The head has parents
a9c3d127d3728f50533df32504b787823b5220eeand2494848f56e8a4f283722b2ebd816977e828c7af, including the repairs from #7642 and #7644. bun.lock,gui/package.jsonandgui/bun.lockare byte-identical to current dev. The rootpackage.jsondiffers only intestRunnerBun: 1.4.3 → 1.4.4;zod 4.6.6, thebody-parser 2.4.0override/resolution, and GUIhappy-dom 21.0.4are preserved.- The resolved
tests/ci-workflows/install-scripts.test.tsretains both the runner1.4.4assertion and the zod4.6.6assertion. No lockfile regeneration or dependency-policy rollback was found.
Workflow/dependency security review
Security-focused static review found no newly introduced privilege, credential-handling or dependency-policy regression in the reviewed delta:
- Both changed
.githubfiles contain comment-only changes; their contents with comment lines removed match dev. Event triggers, job permissions, checkout credential persistence, immutable external action pins and frozen-lockfile installation commands are unchanged. Docker changes are also comments only, with the existing image digest retained. - Root dependencies, overrides, scripts and
trustedDependenciesmatch dev. The runner resolver and layout verifier are unchanged; wrong-version overrides still fail before test execution, and resolver probing does not download or install a runner. - The updated role fixture exercises matching/different callers and wrong-version refusal through the real resolver, retaining
--isolateand the working-directory assertions. - The buffered HTTP/1 fixture remains test-only and retains the 413/
earlyresponse, connection-close, incomplete-upload and subsequent-success assertions and original timeouts. Its writablefinishobservation is not packet-level FIN evidence. No production relay change was found.
This records author-side technical review evidence. Maintainer security acceptance remains outstanding. @lidge-jun @Ingwannu: please record explicit workflow/dependency security acceptance for this exact head; the existing reviewer requests remain in place.
Validation still pending on this head
- Required PR CI — 40051221633 and full-platform lane=all — 40051232056 both target
f011a94…and are queued/running. The latter has the Ubuntu four-shard, macOS three-shard plus control, and Windows nine-shard jobs. - The full-platform run skips its docs build; the required current-head docs site build is present in the PR run and still needs to finish.
- Source/lockfile equality does not establish a successful fresh installation. The earlier
a9c3d127full-platform pass and recorded Windows focused tests do not replace these current-head gates.
No additional corrective code change was identified by this follow-up review. Keep the current draft state until the outstanding full-platform validation and explicit security-review requirement are satisfied.
|
Independent source review (gpt-6.1-sol, dispatched by the triage manager, 2026-10-24) at head VERDICT: MUST-FIX Reviewed current head
No additional source defect identified. Explicit workflow/dependency static security review found no new credential, permission, or installation-policy regression: workflow/Docker changes are comments only, and dependency changes relative to the integrated base are limited to the runner pin. Maintainer security acceptance remains unrecorded. #7642/#7644 repairs and lockfiles are preserved. Existing test registrations remain valid; layout stays 1,999 lines, runtime documentation 600 lines, with no ratchet violation found. Regression assertions remain meaningful. PR template is complete; unresolved review threads: 0. Branch is mergeable, one commit behind live NOT RUN: tests, typecheck, builds, or repository gates—read-only review. This is a read-only source review; merging still requires the gates named above (exact-head CI, |
|
Exact-head validation update for |
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. |
Addendum: journal failure diagnostics at 8b4c34cOriginal FAIL remains recorded; these authorized diagnostic repetitions do not replace it.
Exact batch-026 command (same ordered 12 files for HEAD×3 and BASE×3): timeout --signal=TERM --kill-after=15s 300s bun test --isolate --parallel=1 --timeout 60000 tests/codex-integration/codex-history-lock.test.ts tests/codex-integration/codex-history-pending-wal.test.ts tests/codex-integration/codex-history-provider.test.ts tests/codex-integration/codex-history-writer.test.ts tests/codex-integration/codex-home-owner-lifecycle.test.ts tests/codex-integration/codex-integration-record.test.ts tests/codex-integration/codex-journal.test.ts tests/codex-integration/codex-legacy-config-keys.test.ts tests/codex-integration/codex-log-guard-coderabbit.test.ts tests/codex-integration/codex-log-guard-processes.test.ts tests/codex-integration/codex-log-guard-status-zero-write.test.ts tests/codex-integration/codex-main-rotation.test.tsHEAD-alone×3: timeout --signal=TERM --kill-after=15s 300s bun test --isolate --parallel=1 --timeout 60000 tests/codex-integration/codex-journal.test.tsUsed the original recorded batch environment unchanged: Failing assertion text in these nine runs: none. Original shard3 batch026 (234 pass/1 fail, exit1): Assessment: the original failure is intermittent/non-reproducible in this sample. Repeating the exact batch order passed3/3 at HEAD and3/3 on base; HEAD alone passed3/3. Fixed batch order alone did not reproduce it, and no deterministic source regression was demonstrated. The precise cause and dependence on earlier suite state remain unproven. The original failure did not expose the child stderr, so its exit1 cannot be diagnosed further from that log. Original comparison classification and overall FAIL remain unchanged. |
Local CI-equivalent (Windows) at 8b4c34cHistorical-head evidence: the PR moved to FAIL under the owner-approved real-machine rule. 14 BASE-IDENTICAL observed test failure(s); 0 INTRODUCED. Raw nonzero exits remain failed and are shown below. Host MINI, Windows_NT 10.0.26200 x64. Test-runner Bun 1.4.4, runtime Bun 1.4.4. The pull ref was re-fetched immediately before posting and is now Scope: Windows CI legs only; the owner supplied the already-passing exact-head Ubuntu results. Before each shard, fresh host-pressure and native-process checks admitted the job only with non-critical pressure and no other suite. No concurrent heavy suite was started by this verifier. Environment: Windows shards — counts are pass/fail/skip Command per shard:
Install/build commands
Each CLI-help entry is Other selected commands and base comparisons
npm-global, when selected, used the workflow Node 20 line (portable Node 20.20.2/npm 10.8.2), no external Bun on PATH, and a private global prefix/home/cache. The bundled dependency supplied Bun. This did not modify the user’s global installation. Failure classifications
Base comparisons use detached Runtime failures and interrupted output
Selected smoke failures
Each remaining batch (pass/fail/skip) Every row executes
Assertion details for the cases named above (shared assertions shown once):
The history recovery case fails on both heads with EICACLS at For the service-status preflight failure, both head and base expected Known remote-hub case: this head’s recorded result(s):
Shared-host observation: shard 8 was admitted at 22:46:40 UTC with normal pressure and zero other suites. M5 started another suite at 22:59:58 UTC during later successful remainder batches; all three shard-8 failing commands had ended by 22:57:24 UTC. At the next admission check (23:04:44 UTC), this verifier held all new heavy checks until the other suite ended. No passing or failing test was rerun because of this overlap. NOT RUN
Complete exact-command ledger and batch membership Brotli/base64 UTF-8 JSON below records every invocation, ordered file argument, exit code, counts, native attribution, deadlines and classification. SHA-256 of decompressed JSON: Full command/coverage ledgerRaw logs and JSON are retained outside removed scratch worktrees at Ledger codec: Brotli/base64 UTF-8 JSON; Node decoder: |
Addendum: macOS test-only follow-up at ba67ddbPASS for the requested scope. Full CI on this new head was NOT RUN.
Each focused run: timeout --signal=TERM --kill-after=15s 300s bun test --isolate --parallel=1 --timeout 60000 tests/claude-integration/claude-picker-upload.test.ts
Same CI command form for every full-shard batch, with its ordered FILES from the head’s script: timeout --signal=TERM --kill-after=15s 300s bun test --isolate --parallel=1 --timeout 60000 "${FILES[@]}"
Setup and smoke exits (no Bun test count unless shown):
Failures:
All full-shard batches: exit and pass/fail/skip
NOT RUN at this new SHA: macOS 1/3, 2/3, 1/1 control, macOS native/app jobs, Ubuntu/Windows and other full-CI gates; they were outside this explicitly requested follow-up. The earlier |
Windows addendum at ba67ddbPASS for the manager-requested additional scope. Historical full-run result: FAIL (record); its shard-7 cutoff remains unverified. This addendum covers the changed file 5x and its owning Windows shard once. It does not attest all nine legs at the new head. Host MINI; Windows_NT 10.0.26200 x64; runner/runtime Bun 1.4.4/1.4.4. Re-fetched PR head: Both fresh detached worktrees ran Requested file command:
Shard command:
The two remote-hub cases both hit Base NOT RUN: No batch in the requested shard or requested observation remains unexecuted. New-head Windows shards 1/2/4/5/6/7/8/9 and other jobs were not repeated under this bounded manager request. Every remaining batch: batch, exit, pass/fail/skipRaw logs and exact command JSON: |
|
Integration evidence (owner decision decision:e6895606, approved, including the explicit workflow/dependency security review): independent gpt-6.1-sol review at 8b4c34c found no source defect (action SHAs, permissions, frozen installs, Docker digest and trustedDependencies unchanged; #7642/#7644/#7647 pins preserved), and a delta review of 8b4c34c..ba67ddb (two test-only commits in tests/claude-integration/claude-picker-upload.test.ts) passed. Exact-head PR CI run 40140422865 and exact-head ci.yml lane=all run 40140464158 both succeeded. A real-machine macOS run at 8b4c34c recorded one codex-journal.test.ts:515 failure that did not reproduce in nine targeted reruns (batch x3 at head, x3 on dev, file alone x3). Static merge simulation on current dev 5ab431b (this Mac): dev=5ab431bec0f438c397cd5c424adbdcd38f2f07cf base=0aeaf0cb6b0a7c2f117d8c75f8fa396cf2ea6e39 pr_files= 11 overlap=[]; merge clean; install exit=0; gui install exit=0; tsc exit=0; tsc doctor exit=0; structure:check exit=0; privacy:scan exit=0; layout+ratchet guards exit=0 54 pass 0 fail ; Merged by the triage manager as maintainer integration (MAINTAINERS.md, dev only) with green exact-head PR CI, an independent review PASS, and the owner's approval of this bundle. |
…native-audit-utf8 # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…t-rejection-lifetime # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…ream-fragmented-buffer # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…-signature-persist-batches # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…owner-resource-lifetime # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…e-aggregation-bounds # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…upload-waiter-lifetime # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…ncremental-response-lifecycle # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…-connect-header-buffer # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…rsist-starvation # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…-unchanged-partial-tail # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…tation-rewrite-for-third-party-gateways # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…k-preflight-no-selection # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
…ws-close-grace # Conflicts: # tests/claude-integration/claude-picker-upload.test.ts
# Conflicts: # scripts/test-layout/layout-adapters.json # tests/claude-integration/claude-picker-upload.test.ts
# Conflicts: # structure/transports/byte-accounting.md # tests/claude-integration/claude-picker-upload.test.ts
Resolve tests/claude-integration/claude-picker-upload.test.ts to dev: the conflicting side came from pre-squash lidge-jun#7627 commits that dev already carries as 0929e6e.
Summary
Align the test-runner default with the Bun 1.4.4 security runtime introduced by #7606. Both runtime and test-runner roles select 1.4.4; role validation and CI trigger policy remain intact.
The layout test uses explicit caller/runner fixtures to check version rejection, isolation arguments and the working directory. Production resolvers are unchanged.
The buffered HTTP/1 upload fixture uses a draining TLS origin with explicit response framing. It now waits for the first request-body byte before sending its early reply, rather than accepting a header-only exchange as upload evidence. A separate TLS regression sends headers, waits for the origin to observe them, verifies that no reply was sent, then sends one byte and requires 413 / "early". The existing buffered test still requires incomplete upload, client closure and a successful subsequent request. Deadlines and production relay code are unchanged.
The recorded dev integration preserves zod 4.6.6, body-parser 2.4.0 and GUI happy-dom 21.0.4 security pins. No lockfile was regenerated.
Verification
ba67ddb501cc9ee2871025d8a6b812074afbdd08, two test-only follow-ups to8b4c34c7e6103314dcb061f0f82a8eaafdb4d386. Runtime and configured runner remain Bun 1.4.4.bun test --isolate --timeout 60000 tests/claude-integration/claude-picker-upload.test.ts --test-name-pattern "^(early-fin origin waits for the first body byte after separate headers|h1 early upstream response closes a buffered unfinished upload without losing the reply)$".8b4c34c7passed full-platform CI with 28 successful jobs / 6 conditional skips, and PR CI. These results support the unchanged transition code but do not qualify the new candidate. Skipped lanes are not counted as executed tests. Earlierf011a94belocal role/upload/layout checks passed 88 cases with 3 skips using an older installed dependency tree; hosted CI used frozen installs.8b4c34c7remains recorded. The maintainer's diagnostic addendum reports the ordered batch passing 3/3 on both HEAD and base, plus the journal file passing 3/3 alone. It is currently unreproduced; the precise cause and preceding-suite dependence remain unproven, and the original child diagnostics were not captured. The journal blob is unchanged at those revisions and this candidate. No additional repetition is requested.Checklist
Summary by CodeRabbit