test(catalog): pin the routed reasoning joint contract (#1100) + bug-grade triage unit #1119
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,129 @@ | ||
| # 000 — Plan: bug-labelled backlog remediation | ||
|
|
||
| Unit for the loop that converts the 2026-08-06 bug-grade triage into landed, | ||
| evidence-backed remediation. Base `origin/dev` = `ef1317871`, release `v2.10.2` | ||
| is published and is an ancestor of `origin/main` (npm `latest` = 2.10.2). | ||
|
|
||
| ## Objective | ||
|
|
||
| Every `bug`-labelled open item reaches a **stated disposition**: fixed by us with | ||
| a genuine regression test, reviewed with `file:line` and the required change, or | ||
| explicitly parked with the named evidence that would unpark it. No item is closed | ||
| on suspicion and no PR is merged inside this loop. | ||
|
|
||
| Being precise about the limit, because the amended audit caught the original | ||
| wording overselling it: this loop authors code for **one** defect (#1100's joint | ||
| contract). The other eleven G1 issues get a disposition, not a fix — several are | ||
| upstream, and several are waiting on a reporter capture no amount of local work | ||
| can substitute for. | ||
|
|
||
| ## Where the triage landed (25 items) | ||
|
|
||
| Twelve issues and thirteen PRs carry the `bug` label. **No item was fake** — every | ||
| reported defect reproduces in code. What separates them is whether the *fix* is | ||
| sound. | ||
|
|
||
| | Grade | Meaning | Items | | ||
| |---|---|---| | ||
| | G1 | Real defect, nobody is fixing it yet | issues #1102 #1100 #1024 #1017 #994 #904 #796 #1059 #418 #417 #241 #92 | | ||
| | G2 | Fix is real and correct | PRs #1095 #1085 (merge-ready) · #1111 #1056 #1047 (small change) | | ||
| | G3 | Bug real, fix dangerous | PRs #1036 #1092 #978 #1068 #557 | | ||
| | G4 | Premise or contract missing | PRs #1093 · #997 (borderline: fix right, proof absent) | | ||
|
Comment on lines
+20
to
+31
Contributor
There was a problem hiding this comment. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Reconcile the 25-item total. The supplied records enumerate 12 issue items and 12 PR rows, not 25 total items. One bug-labelled item therefore has no grade, evidence, or disposition.
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
|
|
||
| `001_grade_matrix.md` carries the per-item evidence anchors. | ||
|
|
||
| ## Constraints | ||
|
|
||
| - Bun-native TypeScript. `bun run typecheck` and the touched test files must be | ||
| green before any completion claim. | ||
| - Review language is English (repo review guideline), regardless of issue language. | ||
| - Every new regression test needs a red ablation: break the mechanism, watch the | ||
| test fail, restore, watch it pass. A green suite alone proves nothing. | ||
|
|
||
| ## Scope | ||
|
|
||
| **IN** — this unit's docs; `tests/` additions we author; `src/` changes only for | ||
| defects we own; GitHub review comments and labels. | ||
|
|
||
| **OUT (escalate, do not do)** — merging any PR; `git push`; closing contributor | ||
| PRs; weakening any security or capability gate; implementing the #1102 | ||
| loopback-peer exemption (a policy decision the maintainer owns); releases. | ||
|
|
||
| ## Work-phase map | ||
|
|
||
| Dependency graph: **wp0 → wp1 is serial; wp2–wp5 are parallel lanes; wp6 | ||
| aggregates their settled outcomes.** The catalog contract runs first because it | ||
| is the only phase that changes shipped behavior, and it produces the vocabulary | ||
| ("advertised capability is not proven capability") the review lanes reuse. | ||
|
|
||
| | WP | Doc | Deliverable | | ||
| |---|---|---| | ||
| | wp0 | `000`, `001` | This roadmap, at diff-level | | ||
| | wp1 | `010` | #1100 catalog joint-contract regression test | | ||
| | wp2 | `020` | #1092 + #978 capability-gate reversal reviews | | ||
| | wp3 | `030` | #1036 + #1068 correctness-hazard reviews | | ||
| | wp4 | `040` | #557 + #1093 + #997 dispositions | | ||
| | wp5 | `050` | G2 merge-readiness dispositions | | ||
| | wp6 | `060` | G1 issue dispositions (the 11 issues wp1 does not fix) | | ||
|
|
||
| wp1 consumes wp0's verified ordering finding, so that edge is serial. wp2–wp5 | ||
| have no dependency on each other and may run in any order; they are separate | ||
| phases because they fail for different reasons, not because one feeds the next. | ||
| wp6 is the aggregation barrier — it cannot close until the PR lanes settle, | ||
| because several G1 issues are dispositioned by pointing at a PR reviewed in | ||
| wp2/wp3 (#1017 → #1036, #994 → #1068). | ||
|
|
||
| Running wp2/wp3 as one phase would blur why each PR is blocked: wp2 items reverse | ||
| a deliberate fail-closed decision, wp3 items delete or bypass an existing | ||
| protection. | ||
|
|
||
| ## The finding that shapes wp1 | ||
|
|
||
| `normalizeRoutedCatalogEntry` deletes `supports_reasoning_summaries` from every | ||
| routed row (`src/codex/catalog/parsing.ts:353`) and `ensureStrictCatalogFields` | ||
| then defaults it to `false` (`:266`). Codex gates construction of the entire | ||
| Responses `reasoning` object on that flag, so a routed model advertises a full | ||
| effort ladder while the wire carries no effort — issue #1100. | ||
|
|
||
| A direct experiment settles what is actually broken: | ||
|
|
||
| ``` | ||
| plain/ladder-model levels=[low,high,max,ultra] default=high summaries=false | ||
| optin/ladder-model levels=[low,high,max,ultra] default=high summaries=true | ||
| ``` | ||
|
Comment on lines
+90
to
+93
Contributor
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Add language identifiers to every fenced block. The supplied Markdown lint results report MD040 for these fences.
🧰 Tools🪛 markdownlint-cli2 (0.23.2)[warning] 90-90: Fenced code blocks should have a language specified (MD040, fenced-code-language) 📍 Affects 2 files
🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| The per-model opt-in the `:352` comment says should exist **already exists** | ||
| (`modelSupportsReasoningSummaries`, resolved at | ||
| `src/codex/catalog/provider-fetch.ts:545`) and survives the delete, because | ||
| `applyCatalogModelMetadata` runs at `src/codex/catalog/sync.ts:268`, one line | ||
| *after* `normalizeRoutedCatalogEntry`. | ||
|
|
||
| So the defect is narrower and more honest than "we strip the flag": the ordering | ||
| works, the escape hatch works, and **nothing asserts the two stay consistent**. | ||
| A future reordering of those two calls would silently disable effort propagation | ||
| for every routed provider that opted in, and no test would notice. That absent | ||
| joint contract is what wp1 buys, and it is squarely ours regardless of how the | ||
|
Comment on lines
+95
to
+105
Contributor
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | sed -n '1,200p' | rg 'devlog/_plan/260806_bug_grade_remediation/000_plan.md|devlog/_plan/260806_bug_grade_remediation/010_wp1_catalog_joint_contract.md|src/codex/catalog'
echo "== plan snippets =="
for f in devlog/_plan/260806_bug_grade_remediation/000_plan.md devlog/_plan/260806_bug_grade_remediation/010_wp1_catalog_joint_contract.md; do
echo "--- $f lines 85-110 / 14-35"
sed -n '85,110p;14,35p' "$f" | cat -n
done
echo "== sync outline =="
ast-grep outline src/codex/catalog/sync.ts --view expanded || true
echo "== sync relevant lines =="
sed -n '240,320p;1,80p' src/codex/catalog/sync.ts | cat -n
echo "== provider-fetch metadata restore locations =="
rg -n "modelSupportsReasoningSummaries|applyCatalogModelMetadata|normalizeRoutedCatalogEntry|applyJawcodeCatalogMetadata|applyReasoningLevels" src/codex/catalog -SRepository: lidge-jun/opencodex Length of output: 169 🏁 Script executed: #!/bin/bash
set -u
echo "== repo root files =="
git ls-files | sed -n '1,220p'
echo "== target file availability =="
for f in \
"devlog/_plan/260806_bug_grade_remediation/000_plan.md" \
"devlog/_plan/260806_bug_grade_remediation/010_wp1_catalog_joint_contract.md" \
"src/codex/catalog/sync.ts"
do
printf '%s: ' "$f"
test -f "$f" && echo exists || echo missing
done
if [ -f "devlog/_plan/260806_bug_grade_remediation/000_plan.md" ]; then
echo "== 000_plan relevant excerpts =="
sed -n '85,110p;14,35p' devlog/_plan/260806_bug_grade_remediation/000_plan.md | cat -n
fi
if [ -f "devlog/_plan/260806_bug_grade_remediation/010_wp1_catalog_joint_contract.md" ]; then
echo "== 010_wp1 relevant excerpts =="
sed -n '85,110p;14,35p' devlog/_plan/260806_bug_grade_remediation/010_wp1_catalog_joint_contract.md | cat -n
fi
if [ -f "src/codex/catalog/sync.ts" ]; then
echo "== sync.ts call sites around 240-320 =="
sed -n '240,320p' src/codex/catalog/sync.ts | cat -n
echo "== sync.ts call sites around template/fallback =="
sed -n '1,80p' src/codex/catalog/sync.ts | cat -n
fi
echo "== search symbols =="
rg -n "modelSupportsReasoningSummaries|applyCatalogModelMetadata|normalizeRoutedCatalogEntry|applyJawcodeCatalogMetadata|applyReasoningLevels" . -S || trueRepository: lidge-jun/opencodex Length of output: 50377 Correct the ordered
📍 Affects 2 files
🤖 Prompt for AI Agents |
||
| upstream Codex gate behaves. | ||
|
|
||
| wp1 deliberately does NOT flip the default to `true`. Advertising OpenAI-only | ||
| summary delivery for arbitrary providers is the exact overclaim the `:352` | ||
| comment refuses, and #1092/#978 are blocked in wp2/wp3 for the same class of | ||
| mistake. Fixing our own bug by committing the neighbours' bug would be incoherent. | ||
|
|
||
| ## Accept criteria | ||
|
|
||
| - `c0` unit exists with `000`, `001` and one diff-level decade doc per phase. | ||
| - `c1` #1100 joint-contract test green, with recorded red-ablation output. | ||
| - `c2` #1092 and #978 reviews posted, each quoting the fail-closed source. | ||
| - `c3` #1036 and #1068 reviews posted with a reproducible hazard scenario. | ||
| - `c4` #557, #1093, #997 dispositions posted; #557 cites real failing tests. | ||
| - `c5` every G2 PR has a readiness verdict; all remain open and unmerged. | ||
| - `c6` each of the 11 remaining G1 issues carries one concrete disposition: | ||
| fix-owner named, reporter evidence requested, upstream handoff recorded, or | ||
| parked with the specific unblocking condition. | ||
|
|
||
| ## Terminal outcomes | ||
|
|
||
| `DONE` all six criteria met with fresh evidence. `NEEDS_HUMAN` for any merge, | ||
| push, or the #1102 policy call. `BLOCKED` where a contributor or reporter must | ||
| act first. | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| # 001 — Grade matrix (25 bug-labelled items) | ||
|
|
||
| Research record. No diffs here (LEXICO-SPLIT-01). Verified against | ||
| `origin/dev` = `ef1317871`; two independent read-only review lanes produced the | ||
| PR rows and the main session re-verified the load-bearing claims by reading the | ||
| dev-side source directly. | ||
|
|
||
| ## Issues (12) — G1: real, unfixed | ||
|
|
||
| | Issue | What breaks | Evidence anchor | What would move it | | ||
| |---|---|---|---| | ||
| | #1102 | Direct-spawned `codex app-server` never receives `OPENCODEX_API_AUTH_TOKEN`; 401 before any SSE frame | shim covers `app-server` (`src/codex/shim.ts:42,386,416`) but a direct spawn bypasses it; admission reads only the header (`src/server/auth-cors.ts:256,369`) | a maintainer policy decision on an opt-in loopback-peer exemption | | ||
| | #1100 | Routed models advertise an effort ladder; the wire carries no effort | `src/codex/catalog/parsing.ts:353,266`; `requestedEffort` set from parsed input at `src/server/responses/core.ts:1379` | one sanitized ingress body from the reporter | | ||
| | #1024 | Text-only models still receive images on one route | Zen path fixed (`a6f3b2fc2`, in `v2.10.2`); reporter's `TR` provider is not a built-in | the reporter's redacted `TR` config | | ||
| | #1017 | Cursor emits malformed `apply_patch` payloads | no structured-edit translation on dev (`src/adapters/cursor/protobuf-events.ts:314-348`) | PR #1036, once its hijack defect is fixed | | ||
| | #994 | Claude Code 400: `reasoning_content` must be passed back | replay metadata absent for `opencode-zen` (`src/providers/registry.ts:1753-1762`) | reporter's provider/model + wire capture | | ||
| | #904 | U+FFFD when writing Korean files | `eeef7a32a` shipped; tests cover astral surrogates, not the reported Hangul case | the original failing capture | | ||
| | #796 | Volcengine Ark 400 on tool turns | `d3abf4345` shipped with `tests/volcengine-ark-assistant-content.test.ts:66`; no live Ark validation | a live Ark retest | | ||
| | #1059 | ~207 Windows failures; leg is dispatch-only | `.github/workflows/ci.yml:371-400` | a Windows runner burn-down | | ||
| | #418 #417 #241 #92 | upstream Codex behavior | tracked upstream; relay transparency covered locally | upstream resolution | | ||
|
|
||
| #1100 and #1102 received maintainer RCA comments on 2026-08-06 | ||
| (`5201923105`, `5201927225`). | ||
|
Comment on lines
+22
to
+23
Contributor
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Escape issue references that start with
Also applies to: 56-59 🧰 Tools🪛 markdownlint-cli2 (0.23.2)[warning] 22-22: No space after hash on atx style heading (MD018, no-missing-space-atx) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| ## PRs (13) | ||
|
|
||
| ### G2 — fix is real and correct | ||
|
|
||
| | PR | Author | Readiness | Remaining blocker | | ||
| |---|---|---|---| | ||
| | #1095 | baileyh8 | merge-ready | removes forced non-streaming for `deepseek-v4-flash` (`src/providers/registry.ts:1254-1257`); synthesizes a terminal only after a complete item lifecycle; real upstream terminals stay authoritative | | ||
| | #1085 | n3wr1ch | merge-ready | replaces the unset `$OPENCODEX_API_KEY` reference that made Pi hide the provider; needs the stated security review | | ||
| | #1111 | Simon-Opopeee | small change | Copilot stream normalization is sound; the 64 MiB native-history bound is an unrelated concern and should be split | | ||
| | #1056 | WZBbiao | small change | opt-in native-alias workaround for #241, correctly bounded; split the unrelated drain-server timing edit | | ||
| | #1047 | baileyh8 | small change | syncs vision replacements into `_rawBody`, which passthrough actually serializes (`src/adapters/openai-responses.ts:1144-1188`) | | ||
|
|
||
| ### G3 — bug real, fix dangerous | ||
|
|
||
| | PR | Hazard | Dev-side proof it is deliberate | | ||
| |---|---|---| | ||
| | #1036 | converts every call named `edit_file`/`multi_edit`, so a genuine client/MCP tool with that name is silently reinterpreted as `apply_patch` | the PR's own tool builder avoids shadowing those names, then the translator ignores that distinction | | ||
| | #1092 | injects a combo default effort for targets whose ladder is UNKNOWN | dev fails closed and logs `capability: unknown|unsupported` (`src/combos/request.ts:43-55`) | | ||
| | #978 | sends `thinkingLevel` to any model with a configured ladder | dev restricts it to two direct Flash ids (`src/adapters/google.ts:313-326`); a configured ladder is a UI contract, not a wire capability | | ||
| | #1068 | drops `OPENCODE_ZEN_TEXT_ONLY_MODELS`, which protects six models (`src/providers/registry.ts:372-379`), and uses obsolete generic DeepSeek constants superseded by per-model ladders (`:380-434`) | | ||
| | #557 | skips the preflight entirely on Windows; the PR claims "7000 pass / 0 fail" while the recorded Windows run has two failing update tests | | ||
|
Comment on lines
+37
to
+45
Contributor
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Repair the G3 evidence table. Line 42 contains
Proposed table correction-| `#1092` | injects a combo default effort for targets whose ladder is UNKNOWN | dev fails closed and logs `capability: unknown|unsupported` (`src/combos/request.ts:43-55`) |
+| `#1092` | injects a combo default effort for targets whose ladder is UNKNOWN | dev fails closed and logs `capability: unknown or unsupported` (`src/combos/request.ts:43-55`) |
-| `#557` | skips the preflight entirely on Windows; the PR claims "7000 pass / 0 fail" while the recorded Windows run has two failing update tests |
+| `#557` | skips the preflight entirely on Windows | the PR claims "7000 pass / 0 fail", but the recorded Windows run has two failing update tests; add the exact test names or log anchor |🧰 Tools🪛 markdownlint-cli2 (0.23.2)[warning] 42-42: Spaces inside code span elements (MD038, no-space-in-code) [warning] 42-42: Table column count (MD056, table-column-count) [warning] 44-44: Table column count (MD056, table-column-count) [warning] 45-45: Table column count (MD056, table-column-count) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||
|
|
||
| ### G4 — contract or proof missing | ||
|
|
||
| | PR | Gap | | ||
| |---|---| | ||
| | #1093 | accepts any client-supplied `x-opencodex-ingress-span` matching a regex; no issuer is identified, and no policy for failures before adapter resolution. Also adds fields to persisted usage and `/api/logs` while claiming no contract change | | ||
| | #997 | fixes a real hazard (tests writing into the developer's real `~/.opencodex`, `src/config.ts:545-552`) but adds no test asserting the real home was untouched | | ||
|
|
||
| ## Cross-cutting observation | ||
|
|
||
| #1092, #978 and #1068 are the same mistake wearing three costumes: each treats | ||
| *advertised* capability as *proven* capability, or removes a guard that encodes | ||
| the difference. The reviews in wp2/wp3 should name that shared principle rather | ||
| than reading as three unrelated nitpicks. | ||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These rows are presented as the final triage result but conflict with the phase ledgers added by the same commit:
020_wp2_capability_gate_reviews.mdsays the dangerous-fix grading for #978 was wrong and its disposition is not blocked, while050_wp5_grade2_dispositions.mdsays #1095 is a draft 341 commits behind and explicitly not merge-ready. Leaving the overview and001_grade_matrix.mdunchanged can cause maintainers to block a sound change or treat an unready PR as mergeable; update both summaries to reflect the corrected verdicts.Useful? React with 👍 / 👎.