From 0b7a7719429cb9570ef7395e22a7ee0bf1d2e112 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Sat, 22 Aug 2026 19:46:22 +0900 Subject: [PATCH] devlog: record the late #2362 review and what retirement cost The review lane for #2362 was retired under DISPATCH-RETIRE-01 after three silent wait cycles, and the PR was reviewed directly instead. The lane then returned with three resolver defects the direct review had missed, each since reproduced at the PR head: the canonical ChatGPT forward provider can opt into terminal repair, an invalid per-model grace falls through to the provider default instead of failing closed, and duplicate case-folded keys resolve by request casing. Retiring the lane was right; treating retirement as a verdict would not have been. Records the rule to re-read a late result against what was already concluded. --- .../090_wp9_new_pr_disposition.md | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/devlog/_plan/260822_backlog_disposition_program/090_wp9_new_pr_disposition.md b/devlog/_plan/260822_backlog_disposition_program/090_wp9_new_pr_disposition.md index 10e99330fa..5d32ec8d83 100644 --- a/devlog/_plan/260822_backlog_disposition_program/090_wp9_new_pr_disposition.md +++ b/devlog/_plan/260822_backlog_disposition_program/090_wp9_new_pr_disposition.md @@ -120,3 +120,63 @@ problems. Closing them would discard that over fixable gaps. Each gets its block restated on the PR with the exact evidence, so the author can finish the work — which is the outcome the repository actually wants. + +--- + +# AMENDMENT — #2362's reviewer returned late, and it found more than the main agent did + +The `xai/grok-4.6` lane for #2362 was retired under DISPATCH-RETIRE-01 after three +silent wait cycles, and the main agent reviewed the PR directly instead. **The lane then +returned**, with a stronger result than the direct review produced. This is recorded +rather than discarded, because the honest comparison is the useful part. + +## What the direct review found + +One structural gap: three new operator-facing config keys added to +`src/types/provider.ts` with neither `src/config.ts` nor `src/server/auth-cors.ts` in the +diff, benchmarked against `modelAdapters`, which is validated at both +`config.ts:1463` and `auth-cors.ts:615`. That finding stands. + +## What the retired lane found on top + +Three defects in the resolver itself, all **reproduced by the main agent** in a +throwaway worktree at the PR head before being accepted: + +``` +B2 canonical-openai: {"graceMs":500} +B4 invalid-falls-through: {"graceMs":750} +B3 My-Model: {"graceMs":500} my-model: {"graceMs":1500} MY-MODEL: {"graceMs":1500} +``` + +1. **The canonical ChatGPT forward provider can opt into repair.** `authMode: "forward"` + plus `responsesTerminalRepair` wraps the canonical SSE in the DeepSeek repair + machine, which #1809 rules out. `providerConfigSchema` is `.passthrough()`, so a + hand-edited `config.json` loads it even though management POST would reject it. + `isCanonicalOpenAiForwardProvider` already exists and is not consulted. +2. **An invalid per-model grace re-enables repair through the provider default.** + `{ foo: 0 }` reads as "disable this model" and instead falls through to + `responsesTerminalRepair: 750`. +3. **Duplicate case-folded keys resolve by request casing.** One model, two grace + windows, decided by how the caller spelled it. + +Plus: the effective-wire check reimplements a looser lookup than +`resolveWireProtocolOverride` actually uses; `graceMs` is uncapped +(`Number.MAX_SAFE_INTEGER` accepted); and two of the new "fail-closed" tests are +tautological — they assert `undefined`, which the old code already returned, so they +survive a revert of the source change. + +## The lesson worth keeping + +DISPATCH-RETIRE-01 exists so a silent lane cannot stall a loop, and retiring it was +correct — the phase would otherwise still be waiting. But **retirement is not a verdict**. +The main agent's fallback review was thinner than the lane's, and had the late result +been dropped on the grounds that the lane was already retired, three reproduced defects +in a config surface would have gone unrecorded. + +Practical rule for later phases: when a retired lane returns after its replacement work +is done, re-read it against what was already concluded. Cheap to check, and here it +changed the evidence on the PR. + +Findings posted to #2362 as comment `5379825296`. Disposition is unchanged — +**LEAVE OPEN** — but the blocker list is now materially longer and measured. +