Skip to content

Commit 5bdf560

Browse files
authored
Merge pull request #2186 from lidge-jun/codex/promote-2.28.0
[WRONG BRANCH] release: promote dev to main for 2.28.0
2 parents 8e01dd4 + 96f288d commit 5bdf560

140 files changed

Lines changed: 8877 additions & 407 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/workflows/ci.yml

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -516,7 +516,7 @@ jobs:
516516
if [ "$suite_status" -eq 0 ]; then
517517
exit 0
518518
fi
519-
if ! grep -Eqi 'oh no: Bun has crashed|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
519+
if ! grep -Eqi 'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
520520
echo "::error::macOS suite failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried."
521521
exit "$suite_status"
522522
fi
@@ -553,7 +553,16 @@ jobs:
553553
# Sharded like the Linux legs. The single-leg run reached 30 minutes on a
554554
# green suite and was killed in cleanup; four shards put each leg inside the
555555
# same budget the Linux shards already hold.
556-
timeout-minutes: 15
556+
#
557+
# 15 was that Linux budget, and on this leg it truncated the evidence rather
558+
# than bounding a hang: shard 1/4 of run 32340498394 was CANCELLED at exactly
559+
# 15m12s while still executing tests, so its result was neither pass nor fail
560+
# and the composed-acceptance cases it carries could not be read at all. The
561+
# other shards finished in 14-15 minutes, which is the wrong side of the
562+
# margin. 25 leaves the outer bound in place — a wedged shard still dies —
563+
# while making a completed shard the normal outcome. The crash-retry below can
564+
# double a shard's work, and this ceiling has to cover that second attempt too.
565+
timeout-minutes: 25
557566
strategy:
558567
fail-fast: false
559568
matrix:
@@ -611,7 +620,31 @@ jobs:
611620
# the only one left on Bun's 5s default, and it is the slowest hardware on the board.
612621
# Three of its failures were the default firing on tests that had not hung — the
613622
# composed-acceptance cases spawn a real `ocx start` and were still working at 41s.
614-
run: bun test --isolate --timeout 60000 tests --shard=${{ matrix.shard }}/4
623+
#
624+
# The retry is the same one the macOS leg already carries, for the same reason: a Bun
625+
# runtime panic is a crash in the interpreter, not a test result, and failing the shard
626+
# on it reports a defect this repository does not have (#2152). An ordinary assertion
627+
# failure returns its status immediately — only the crash signatures below are retried,
628+
# and only once, so a genuinely broken build cannot be retried into green.
629+
shell: bash
630+
run: |
631+
set +e
632+
set -uo pipefail
633+
suite_log="$(mktemp -t ocx-windows-suite.XXXXXX)"
634+
for attempt in 1 2; do
635+
bun test --isolate --timeout 60000 tests --shard=${{ matrix.shard }}/4 2>&1 | tee "$suite_log"
636+
suite_status="${PIPESTATUS[0]}"
637+
if [ "$suite_status" -eq 0 ]; then
638+
exit 0
639+
fi
640+
if ! grep -Eqi 'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then
641+
echo "::error::Windows shard ${{ matrix.shard }}/4 failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried."
642+
exit "$suite_status"
643+
fi
644+
echo "::warning::Bun runtime crash in Windows shard ${{ matrix.shard }}/4 (exit ${suite_status}, attempt ${attempt})."
645+
done
646+
echo "::error::Bun runtime crash repeated on Windows shard ${{ matrix.shard }}/4; failing after one retry."
647+
exit 1
615648
616649
- name: CLI help smoke
617650
run: bun run src/cli/index.ts help

devlog/_plan/260820_bug_pr_backlog_consolidation/000_research_inventory.md

Lines changed: 289 additions & 0 deletions
Large diffs are not rendered by default.
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
> **SUPERSEDED IN PART — read `000_research_inventory.md` amendments 1 and 2 first.**
2+
>
3+
> Two things in this document are wrong and were corrected after it was written:
4+
>
5+
> 1. **It is NOT a stack layer and does NOT root on #2134.** No dependency edge exists;
6+
> the shipped PR (#2137) is based on `dev` as a sibling.
7+
> 2. **The substitution predicate is NOT "native ChatGPT pool".** Pool-only would exclude
8+
> `codexAccountMode: "direct"` and re-break #1686, whose Direct admission is only safe
9+
> BECAUSE substitution still runs. The shipped predicate is
10+
> `route.codexAccountMode !== undefined`, covering pool AND direct. Do not "correct" it back.
11+
12+
13+
# 010 — Layer 1 (stack bottom): fix issue #2132, bearer admission must not force a ChatGPT credential
14+
15+
Work-phase: wp2. Branch: `codex/fix-bearer-admission-2132`. Base: `codex/fix-subagent-roster-truncation` (PR #2134).
16+
Absorbs: nothing (no PR exists). Closes: #2132.
17+
18+
## Why this is the stack bottom
19+
20+
It is the highest-scoring item in the backlog (96) and it shares `src/server/responses/core.ts`
21+
with layer 2 (#2131). Layer 2 must be based on this, or the two edits to that file collide.
22+
23+
## Defect
24+
25+
Reported in #2132: after v2.23.0, a key-auth provider (Cloudflare/etc.) returns 401
26+
`No usable Codex main credential` when `~/.codex/auth.json` holds no ChatGPT token. Bearer
27+
admission sets `substituteMainCredential` unconditionally, so a route that needs no ChatGPT
28+
identity is still gated on one.
29+
30+
## P-phase re-verification required (stale check)
31+
32+
Before editing, confirm against the CURRENT tree — the lane read `dev`, not this branch:
33+
1. `rg -n "substituteMainCredential" src/` — enumerate every producer and consumer.
34+
2. Read `src/server/responses/core.ts`, `src/server/responses/compact.ts`,
35+
`src/codex/auth-context.ts` and establish where the flag is set and where it is read.
36+
3. Reproduce the admission decision in a unit context with a key-auth provider and an
37+
auth.json containing no ChatGPT token. If the current code does NOT reproduce, stop and
38+
amend this doc rather than writing a fix for a defect that is not there.
39+
40+
## Intended change
41+
42+
Make the substitution conditional on the resolved route actually requiring a native/ChatGPT
43+
credential. A key-auth routed provider carries its own credential and must be admitted
44+
without one. Exact call sites are fixed during the stale check above; the invariant is:
45+
`substituteMainCredential` is set only when the route's credential source is the native
46+
ChatGPT pool.
47+
48+
Out of scope: changing what happens once a native route legitimately lacks a credential,
49+
and any change to the pool/account selection itself.
50+
51+
## Test plan (must fail RED first)
52+
53+
New `tests/bearer-admission-key-auth.test.ts`:
54+
1. key-auth routed provider + auth.json with NO ChatGPT token -> request is admitted (no 401).
55+
2. native gpt route + no ChatGPT token -> still fails closed with the existing error.
56+
3. key-auth provider + ChatGPT token present -> unchanged behavior (no regression).
57+
58+
Drive the file against the unpatched tree first and record the failure output; a test that
59+
passes before the fix does not prove anything.
60+
61+
## Verification
62+
63+
`bun run typecheck`; `bun test --isolate` on the new file plus the existing responses/auth
64+
suites; full `bun test --isolate tests` before marking review-ready; `bun run privacy:scan`.
65+
66+
## Standalone thesis (DEV-STACK-03)
67+
68+
"A provider that carries its own key must not be gated on a ChatGPT credential." Builds and
69+
passes its own tests at its own tip, independent of layer 2.
70+
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
# 020 — Layer 2: absorb PR #2131, backfill missing Responses output ids
2+
3+
Work-phase: wp3. Branch: `codex/absorb-responses-id-backfill`. Base: `codex/fix-bearer-admission-2132` (layer 1).
4+
Absorbs: **PR #2131 by @bet4it**. Closes: PR #2131 as superseded, with attribution.
5+
6+
## Dependency edge (the only real one in this backlog)
7+
8+
#2131 adds `src/server/responses/responses-field-backfill.ts` and calls it from
9+
`src/server/responses/core.ts` — the same file layer 1 edits. This is why it stacks rather
10+
than sitting beside layer 1.
11+
12+
## Defect
13+
14+
Strict decoders (grok-build) reject Responses output items that omit `id` on
15+
`message` / `reasoning` / `function_call`. #1941 landed earlier but some relays still omit it.
16+
17+
## Change to carry over
18+
19+
@bet4it's implementation, preserved in substance: synthesize stable `msg_ocx_N` / `rs_ocx_N` /
20+
`fc_ocx_N` ids keyed on `output_index`, never overwriting an id that is already present.
21+
22+
## Correction to apply on top (audit finding, lane: quality)
23+
24+
An invalid or missing `output_index` collapses to `0`, so two unindexed items can both become
25+
`msg_ocx_0` — duplicate ids, which is the exact class of bug this fixes. Replace the
26+
collapse-to-zero fallback with a monotonic per-response counter so synthesized ids are unique
27+
even when `output_index` is absent or malformed. Add the regression test that pins it.
28+
29+
Docs: the locale files in #2131 are uneven (EN/FR rewritten, JA/KO/ZH/TR only first sentence).
30+
Carry only the EN change in this layer; locale parity is not this layer's thesis.
31+
32+
## Test plan (must fail RED first)
33+
34+
Carry @bet4it's tests (SSE `response.completed`, `output_item.done` via `output_index`, JSON
35+
passthrough, preserve-existing-id, inherited `toString` type) and ADD:
36+
- two items with missing `output_index` receive DISTINCT ids (fails on #2131 as written).
37+
38+
## Verification
39+
40+
Same gate as layer 1, plus explicit confirmation that layer 2's branch contains layer 1's
41+
commit (`git log --oneline <layer1>..<layer2>` shows only layer-2 commits) and that the PR
42+
base ref names layer 1's branch.
43+
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# 030 — Sibling A: consolidate prompt_cache_retention (issue #2092)
2+
3+
Work-phase: wp4. Branch: `codex/consolidate-prompt-cache-retention`. Base: **dev** (sibling, not a stack layer).
4+
Absorbs: **PR #2102 by @lilinxiong** (base implementation). Supersedes: **#2099 by @yzxcj797**, **#2091 by @luvs01**. Closes #2092.
5+
6+
## Why a sibling and not a layer
7+
8+
It touches only `src/adapters/openai-responses.ts`, which no other absorbed item touches. It has
9+
no dependency on layers 1-2, so stacking it would impose a false merge order (DEV-STACK-01).
10+
11+
## Chosen contract
12+
13+
@lilinxiong's #2102: strip `prompt_cache_retention` only when
14+
`forward && isCanonicalOpenAiForwardProvider(provider)` AND the model is `gpt-5.6` or
15+
`gpt-5.6-*`. This matches the issue's own correction — the reporter withdrew the "strip
16+
everywhere" claim, and some non-5.6 deployments still honor the field.
17+
18+
Rejected: #2091's blanket strip for every forward provider and every model (it inverts the
19+
existing gpt-5.5 preserve pin at tests/openai-responses-passthrough.test.ts:807).
20+
Rejected: #2099's `startsWith("gpt-5.6")`, which also matches `gpt-5.60`, and its stray
21+
package.json 2.24.2 -> 2.25.0 bump.
22+
23+
## Carried from the superseded PRs
24+
25+
From @yzxcj797's #2099: the `Fixes #2092` issue link and the repro-shaped fixture
26+
(`store:false`, streamed input array). From @luvs01's #2091: nothing — its key-auth preserve
27+
case is already covered by #2102.
28+
29+
## Tightening to apply
30+
31+
Replace the string-prefix family match with the catalog/native-slug predicate if one exists
32+
in the current tree (`rg -n "isGpt56NativeSlug|NATIVE_OPENAI_MODELS" src/`); otherwise keep
33+
the exact `gpt-5.6` / `gpt-5.6-*` match and pin `gpt-5.60` as a NON-match in tests.
34+
35+
## Test plan (must fail RED first)
36+
37+
Carry #2102's tests; add `gpt-5.60` non-match; keep the gpt-5.5 preserve pin intact.
38+
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
# 040 — Sibling B: routing capability + lab behavior evidence
2+
3+
Work-phase: wp5. Branch: `codex/absorb-capability-evidence`. Base: **dev**.
4+
Absorbs: **PR #2100 and PR #2077, both by @ntdatt812**. Closes both as superseded.
5+
6+
## Why these two together, and why a sibling
7+
8+
#2100 touches `src/routing/capability.ts`; #2077 touches
9+
`src/routing/compatibility/behavior.ts`. Disjoint files, one author, one thesis: *model-keyed
10+
lookups must use the same resolution rules the runtime uses*. Neither depends on layers 1-2.
11+
12+
Note: #2077 is Lab-adjacent. Verify `tests/core-lab-boundary.test.ts` stays green — the file
13+
already imports Lab types, so this must not newly puncture the boundary.
14+
15+
## Defects
16+
17+
#2100: bare map lookups made `gpt-oss:120b` inherit the provider-wide 8k window instead of the
18+
`gpt-oss` family's 131072, and `noVisionModels` was ignored.
19+
#2077: `map[modelId]` missed family/case overrides, and `constructor` resolved to
20+
`Object.prototype.constructor`, making `jcsStringify` throw and silently dropping Lab subjects.
21+
22+
## Change
23+
24+
Route both through `modelRecordValue` / `isModelTextOnly` as @ntdatt812 wrote them. Prototype-id
25+
safety (`constructor`, `toString`) is the load-bearing part; keep those tests verbatim.
26+
27+
## Test plan
28+
29+
Carry both test files. Confirm the exact-own maps (`modelPreferHostedTools`,
30+
`modelOpenRouterRouting`) still do NOT family-spread.
31+
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# 050 — Sibling C: K12 short-window quota (issue #2047)
2+
3+
Work-phase: wp6. Branch: `codex/absorb-k12-short-window`. Base: **dev**.
4+
Absorbs: **PR #2056 by @Ingwannu**. Supersedes: **#2062 by @yzxcj797**. Closes #2047.
5+
6+
## Chosen base
7+
8+
#2056 is a strict superset of #2062: `snapshotHasShort`, partial-snapshot preservation,
9+
`updateAccountQuota` carry, and the parse -> cache -> DTO path #2047 actually requires. #2062
10+
drops short on a later weekly/monthly partial snapshot and carries a stray version bump.
11+
12+
## Blocker to fix before this can land (raised by the maintainer on both PRs)
13+
14+
A short-only snapshot with `shortPercent: 0` scores `0` instead of `CODEX_UNKNOWN_USAGE_SCORE`,
15+
so `pickLowestUsageAmong` prefers an account whose long windows are unverified. Fix:
16+
include `shortPercent` in `computeCodexUsageScore` only when the plan's governing long window
17+
is finite; otherwise return `CODEX_UNKNOWN_USAGE_SCORE`. Add the short-only regression.
18+
19+
This blocker is why #2056 is absorbed-and-corrected rather than simply approved.
20+
21+
## Also close
22+
23+
**#2063 by @yzxcj797** — superseded by ALREADY-MERGED #2055 (`2648ffa87`), which classifies
24+
`detail.code` with a stricter own-property lookup. Close with attribution; fold nothing.
25+
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# 060 — Close-out: supersede operations and attribution
2+
3+
Work-phase: wp7. No code. GitHub state only.
4+
5+
## Ordering rule
6+
7+
A PR is closed ONLY after its replacement exists and is pushed. Never close first.
8+
9+
## Operations
10+
11+
| Close | Author | Replaced by | Carried over |
12+
|---|---|---|---|
13+
| #2131 | @bet4it | layer 2 (020) | full implementation + tests, plus unique-id correction |
14+
| #2099 | @yzxcj797 | sibling A (030) | issue link, repro fixture |
15+
| #2091 | @luvs01 | sibling A (030) | nothing; contract deliberately narrower |
16+
| #2100 | @ntdatt812 | sibling B (040) | full implementation + tests |
17+
| #2077 | @ntdatt812 | sibling B (040) | full implementation + tests |
18+
| #2102 | @lilinxiong | sibling A (030) | full implementation + tests (base) |
19+
| #2062 | @yzxcj797 | sibling C (050) | nothing; #2056 supersedes |
20+
| #2063 | @yzxcj797 | merged #2055 | nothing |
21+
| #2056 | @Ingwannu | sibling C (050) | full implementation + scorer correction |
22+
| #2029 | @yzxcj797 | maintainer PR #2130 | nothing; #2130 is a superset |
23+
24+
## Comment template
25+
26+
> Thanks for this, @<login> — closing as superseded by #<n>, which carries <what> from your
27+
> patch. <What changed and why.> Your work is credited in that PR's description.
28+
29+
## NOT closed, with reasons stated publicly
30+
31+
- **#2109 / #2110** (@drakonkat): unresolved security gap in the override gate; needs a human
32+
security pass (AGENTS.md security boundary).
33+
- **#2053** (@Ingwannu): C4 OAuth surface; MAINTAINERS.md mandates security review.
34+
- **#2101, #2040**: large (20 and 14 files); each needs its own PABCD cycle.
35+
- **#2115, #2082, #2027, #2067, #2054, #2032**: below the 60 threshold.
36+
- **#2104, #2075, #2127**: #2075/#2054 CONFLICTING; #2127 is an active draft by its author.
37+

0 commit comments

Comments
 (0)