Repository navigation
fix(quality): drain the 2026-09-22 release-tip base-reds - #14520
Closed
diegosouzapw wants to merge 18 commits into
Closed
diegosouzapw wants to merge 18 commits into
diegosouzapw wants to merge 18 commits into
Conversation
Four gates were red on the pure release tip, so every PR opened against it inherited them: - Docs counts: the migration count moved to 179 (#13222's open-wa seed and #12814's proxy_logs_proxy_name landed) but README.md, AGENTS.md and llm.txt (+ 65 i18n mirrors) still said 178. Bumped in place — these are protected agent-instruction surfaces, so contributor PRs must not carry the change. - Mutation coverage: eight covering unit tests across four mutated modules were missing from stryker.conf.json tap.testFiles, so their mutant kills did not count. Registered additively (six new entries; two were already present). - open-sse/executors/auggie.ts (TS2769 x4, TS18047 x4): buildAuggieSpawnOptions typed its stdio parameter as "readonly string[]", which matches none of spawn()'s overloads — and without the tuple overload TypeScript treats child.stdout / child.stdin as possibly null at every use. Constrained the generic to the 3-tuple all four call sites actually pass. - src/lib/combos/intelligentRouting.ts (TS2698): applyIntelligentRoutingConfigPatch kept only isRecord()'s boolean result, so "patch.weights" was still "unknown" at the spread. Keeps the narrowed value instead. No suppressions, no baseline widening. Validated: check-docs-counts-sync clean, check:mutation-test-coverage "No drift", tsconfig.typecheck-api.json reports no auggie errors, intelligent-routing-weight-edit.test.ts 4/4.
…e-v3.8.51-basereds-0922
…lidated (Refs #13866) Both tests were red on the pure release tip and both pin behavior that a merged PR deliberately changed, so the snapshot is the stale side: - tests/snapshots/provider/translate-path.json: #14316 moved DeepSeek off the Responses API back to Chat Completions (format openai-responses -> openai, /responses -> /chat/completions). Regenerated with UPDATE_GOLDEN=1; the diff is exactly those three lines, no other provider moved. - tests/unit/combo-builder-options-route.test.ts: #14143 (fixes #14135) makes a provider node with a prefix alias qualify its models as "<alias>/<model>", because the raw internal node id never routed. The test still expected the node id. Aligned to "gd/gpt-custom", matching the "oc/" assertion the test right above already makes for opencode. Validated: provider-translate-path-golden 3/3, combo-builder-options-route 3/3.
#14252 ("strip _omniroute* markers at shared pre-executor boundary") wired stripInternalBodyFields() into normalizeAttemptBody(). That helper removes two classes of marker: the `_omniroute*` prefix class (consumed by routing before dispatch — the ones #14252 was actually about) AND the four keys in INTERNAL_BODY_FIELDS, which are consumed by the EXECUTORS inside transformRequest(): _nativeCodexPassthrough CodexExecutor.transformRequest _nativeXaiResponsesPassthrough XaiExecutor.transformRequest _nativeOpenAICompatibleResponsesPassthrough passthrough dispatch _claudeCodeRequiresLowercaseToolNames BaseExecutor normalizeAttemptBody() runs BEFORE transformRequest(), so every native Responses passthrough was silently demoted to the translated path: CodexExecutor no longer saw its marker, fell through to the RESPONSES_API_ALLOWLIST, and dropped the client's top-level `metadata` (plus the passthrough instructions handling). No error — just a quietly different upstream payload. Red on the release tip: "chatCore keeps Responses-native Codex payloads in native passthrough mode". Fix: split out stripInternalOmnirouteMarkers() (prefix-only) and call that at the pre-executor boundary, preserving #14252's intent. The executor-consumed markers keep being removed at the executor-egress boundary (base.ts, dario.ts, ninerouter.ts, applyFingerprint), which runs after transformRequest has read them. stripInternalBodyFields() itself is unchanged in behavior. Tests: two regressions in strip-internal-omniroute-markers.test.ts — the helper level (prefix strip keeps the executor markers) and the real boundary (prepareUpstreamBody preserves _nativeCodexPassthrough and the client metadata while still dropping _omnirouteSkipContextRelay).
#14316 (5f67e11, "fix(deepseek): default to OpenAI Chat Completions instead of Responses API") flipped the DeepSeek registry entry from format: "openai-responses" / baseUrl .../responses to format: "openai" / baseUrl .../chat/completions, because DeepSeek's public API is Chat Completions and the Responses path 400s on multi-turn tool calls that do not echo reasoning_text. The change was deliberate and correct; it moved "openai-responses" into `alternateFormats` ("Responses-compatible") so an operator can still select it per connection, and it updated alternate-formats.test.ts — but not these five fixtures, which still assumed the old default and so went red on the release tip: chatcore-translation-paths.test.ts - carries Chat reasoning_content into official DeepSeek Responses input - replays nonstream DeepSeek Responses reasoning across a Chat tool turn - replays streamed DeepSeek Responses reasoning across a Chat tool turn chat-route-coverage.test.ts - applies task-aware routing when a semantic override is enabled - defaults a Combo's incompatible reasoning fallback to drop All five are regressions about the DeepSeek *Responses* wire path (reasoning replay, Responses-shaped upstream `input`, opaque-reasoning drop), so they now select that protocol the way an operator does — providerSpecificData.targetFormat = "openai-responses", resolved by resolveAlternateFormat() — instead of leaning on a default that no longer exists. Every assertion is unchanged: the tests still prove the same behavior, just on the connection shape that actually reaches it.
…Refs #13866) `provider-node-reserved-prefix` has been red on the release tip since #12474 merged. That PR registered Lyceum and annotated the running tally as "413 -> 414", but the set was ALREADY 414 at its parent commit, so adding "lyceum" moved it to 415 while the assertion stayed at 414. Verified by dumping RESERVED_PROVIDER_PREFIXES at 2a33528~1 (414 members) and at the tip (415) and diffing the two: "lyceum" is the one and only member added, and nothing was removed. So the count is right and the annotation's starting point was wrong — no provider is missing from the walk. Validated: provider-node-reserved-prefix 21/21.
`qwen38-max-bare-id-alias.test.ts` asserted `MODEL_SPECS["qwen3.8-max"] === undefined`, guarding against a second source of truth for what was then the SAME model published under two ids (the preview row carried `aliases: ["qwen3.8-max"]`). That premise expired in #14181/#14242 (83a6e9a, "stop rewriting GA qwen3.8-max to preview on opencode-go"): OpenCode Go ships a GA `qwen3.8-max` that answers 401 on the `-preview` id, so the two ids are now two DISTINCT models and the commit deliberately gave the GA one its own MODEL_SPECS row while dropping the alias from the preview row. The production change is correct; the assertion is the stale side. Replaced it with the invariant this file actually exists to protect — NEITHER id may fall through to contextManager's `default: 128000` — plus a guard that the preview row does not re-declare the bare id as an alias, which is what would silently collapse the two rows back into one. No assertion was removed or weakened. ℹ pass 5 / ℹ fail 0 (node --import tsx/esm --test)
…og suites Three `/v1` catalog suites answered 503 `catalog_build_timeout` instead of 200 and died before evaluating a single catalog-shape assertion: - specialty-model-catalog-routes.test.ts — `listedIds()` aborts on `assert.equal(response.status, 200)` - image-model-not-in-chat-catalog-6457.test.ts — same status check - model-token-limit-catalog.test.ts — the 503 body has no `data`, so `getModel()` throws `Cannot read properties of undefined (reading 'find')` Root cause is not a catalog defect: every case in these files resets the catalog cache (or bumps its generation via an override write), so each request is a COLD build of the full catalog — 460 rows across 42 image providers — and the cold path is bounded by `CATALOG_BUILD_TIMEOUT_MS` (#12627, default 8000ms). A cold tsx build already costs ~7s on an idle box; measured here it takes 11.7–22.9s, so the bound trips deterministically under runner contention. Verified this is not #14216's `parseImageModel()` call in the image loop: 20 full passes over all 268 image models cost 105ms total. Pinned the bound out of the way exactly like the three sibling suites that hit this before — 9147-catalog-eventloop-yield, 12058-models-catalog-canonical-self-aliased and models-catalog-route — each carrying the same rationale. No assertion was changed, removed or weakened; on the contrary, the assertions now actually run instead of being short-circuited by the status check.
…og ids #14216 (4970f5f, "separate Codex GPT-5.6 image catalog ids") gave the three Codex GPT-5.6 image models a `catalogId` — `gpt-5.6-sol-image`, `gpt-5.6-terra-image`, `gpt-5.6-luna-image` — because the callable upstream id collided with the chat surface of the same model. `imageProviderCatalogEntries()` now publishes the `catalogId`, while `parseImageModel()`/`findImageModelConfig()` map it back to the callable `gpt-5.6-sol` on dispatch and for the supported/hidden checks. The production change is deliberate and correct; three suites still asserted the old shared ids: - specialty-model-catalog-routes.test.ts — the image-route catalog listed `codex/gpt-5.6-{sol,terra,luna}-image`, not the bare ids. - image-model-not-in-chat-catalog-6457.test.ts — the #7004 case required the image row "under the same model id"; the two rows now live under their own ids. - model-token-limit-catalog.test.ts — `getModel("codex/gpt-5.6-sol")` now resolves the CHAT row (`type` undefined), so `assert.equal(canonical?.type, "image")` failed on a row that is no longer the specialty one. Updated each to the new public ids and kept every invariant they were written for. Two assertions were added rather than removed: the image row must NOT reappear under the bare chat id (6457), and a context override targeting the chat id must not bleed into the image row (token-limit) — both are the point of the #14216 split.
Both were red on the pure release tip and both pin behavior a merged PR changed on purpose. - opencode-executor "omits accept header when stream is false": #14230 routed the whole opencode-zen GPT-5.6 family to the Responses API (targetFormat:"openai-responses"), and #12633 established that Zen's /v1/responses endpoint authenticates with `x-api-key` rather than Bearer — unlike /chat/completions on the same host. The header therefore moved legitimately; the absent Accept header this case is actually about is unchanged. - domain-branch-hardening "quotaCache covers empty quotas…" (2 assertions): #14276 made a window whose fraction the upstream never reported — `total: 0`, or an Infinity percentage — UNKNOWN instead of "0% remaining", so it can no longer reach the exhaustion threshold. That fix exists because Vertex spend telemetry was locking whole accounts out. The percentages still read 0/100 (the documented placeholder) and both branches the case exists to cover are still exercised. No assertion was dropped. Validated: opencode-executor 59/59, domain-branch-hardening 6/6, quota-cache-unknown-limits 6/6.
…2' into fix/basered-singles-0922
…efs #13866) `check-api-typecheck` has been red on the release tip since #14271 merged: ✗ src/app/api/v1/combos/projectCombo.ts TS2459 (baseline 0, live 2) ✗ src/app/api/v1/combos/projectCombo.ts TS2724 (baseline 0, live 1) #14271 pulled `ComboCollectionLike`, `ComboLike` and `ResolvedComboTarget` from `combo/comboStructure.ts`, which only `import type`s them from `combo/types.ts` and never re-exported them. The value import (`resolveNestedComboTargets`) is correct and stays; only the three types move to `combo/types.ts`. Nothing runtime changes — the types were erased either way, which is why the PR merged green under `typecheck:core`: that project does not cover `open-sse/`, so the unresolved type references only surfaced in the API-route typecheck.
…2' into fix/release-v3.8.51-basereds-0922
`check:env-doc-sync` (part of `check:docs-all`, blocking) has been red on the release tip since #14236: ✗ In code but missing from .env.example: 1 - DEEP_HEALTH_CHECK_ENABLED #14236 added the opt-in deep health probe behind that flag (`src/app/api/monitoring/health/route.ts`) but did not document it. Added to .env.example and docs/reference/ENVIRONMENT.md, stating both halves of the gate: the flag must be exactly "1", and an anonymous caller never triggers a probe with or without it. Validated: check:env-doc-sync in sync, check:docs-all exit 0.
…2' into fix/basered-singles-0922
Merged
5 tasks done
… (Refs #13866) All three reproduce on the pure release tip; none is this PR's defect except the last, which is this PR's own measured growth. - check:deps — "@opencode/plugin" is not in the dependency allowlist. #14370 ported the opencode plugin to the v2 contract, which lives under a DIFFERENT scope from the already-approved "@opencode-ai/plugin". That scope change is exactly the slopsquatting shape this gate exists to catch, so it was verified before being allowlisted: npm shows both packages under the same maintainer (thdxr <d@ironbay.co>, who also publishes @opencode-ai/plugin), with @opencode/plugin created 2026-09-02. Legitimate v2 scope of the same publisher. The verification is recorded in the allowlist's _justifications entry. - hard-session-lease-bypass-inventory — #14213's bounded empty-turn retry added a credential-resolution site in open-sse/handlers/chatCore.ts that the frozen inventory did not list. Inventoried as class B: it resolves through the normal getProviderCredentials path (its own log line says so) and does not reach past the lease, so it is inventoried rather than exempted. - check:file-size — tests/unit/chatcore-translation-paths.test.ts 3546 -> 3564, this PR's own growth from the #14316 realignment: one shared credentials const plus the comment recording why the connection shape changed and that no assertion did. Rebaselined with that attribution; nothing was condensed away. Validated: check-deps 13/13, hard-session-lease-bypass-inventory 3/3, check:file-size OK (both halves).
This was referenced Sep 24, 2026
Merged
Owner
Author
|
Status check from the base-red sweep (2026-09-24): every item this PR carries now has another home, and the branch conflicts with
Leaving it open for the maintainer to decide; nothing was pushed here. |
Owner
Author
|
Fechando como coberta pela #14511, mergeada na release/v3.8.51 (drenou as mesmas base-reds: genérico de stdio do auggie, imports do projectCombo, strip de _native*Passthrough e os guards de catálogo/prefixo/qwen/opencode/quota). O conteúdo que só esta PR tinha já entrou pelas #14501, #14503, #14508 e #14516. (merge-batch 2026-09-24) |
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
1 participant
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.
Drains the gates that were red on the pure
release/v3.8.51tip, so PRs opened against it stop inheriting failures they did not cause.Every red here was reproduced on
origin/release/v3.8.51before being touched (merge-gates §3 discriminator), and for each one the commit that introduced it was identified before deciding whether the test or the production code was the stale side. None of these come from a boarded PR.Static gates
check:docs-countsREADME.md,AGENTS.md,llm.txt+ 66 i18n mirrorscheck:mutation-test-coveragetap.testFilescheck-api-typecheckopen-sse/executors/auggie.tsspawn()'s overloads key oncheck-api-typechecksrc/lib/combos/intelligentRouting.tsisRecord()'s narrowed value, not just its booleancheck-api-typechecksrc/app/api/v1/combos/projectCombo.tsimport types them; import fromcombo/types.tsdocs-syncdocs/i18n/bs/llm.txtwas born saying 178Unit reds
One production regression, found by chasing a red instead of adjusting it:
#14252plugged the fullstripInternalBodyFields()into the shared pre-executor boundary. That helper removes two classes of marker — the_omniroute*ones (already consumed by routing, the PR's legitimate target) and the four inINTERNAL_BODY_FIELDS, which are read by the executors themselves insidetransformRequest(). Since the boundary runs beforetransformRequest, every native Responses passthrough was silently demoted to the translated path: Codex lost the client'smetadataand took the Responses allowlist, and the same applied to_nativeXaiResponsesPassthroughand_claudeCodeRequiresLowercaseToolNames. No error, just a different upstream payload.Split into
stripInternalOmnirouteMarkers()(prefix-only) for the pre-executor boundary; the full strip stays at executor egress (base.ts:1356,dario.ts,ninerouter.ts), which runs after the markers are read. Two regression tests added.Tests that pinned behavior a merged PR deliberately changed — each realigned to still prove what it was written for, with the causing PR cited in a comment. No assertion was dropped or weakened; several cases gained assertions:
provider-translate-path-goldenchatcore-translation-paths×3,chat-route-coverage×2openai-responsesalternate per connection, as an operator doescombo-builder-options-routegd/), not the raw node idprovider-node-reserved-prefixspecialty-model-catalog-routes,image-model-not-in-chat-catalog-6457,model-token-limit-catalogcatalogId; + a new assertion that the image row must not reappear under the chat idqwen38-max-bare-id-aliasdefault: 128000) + that the preview must not re-alias the bare idopencode-executor/v1/responses, which authenticates withx-api-keydomain-branch-hardening×2Three catalog suites also pin
CATALOG_BUILD_TIMEOUT_MS, exactly as9147-catalog-eventloop-yield,12058-models-catalog-canonical-self-aliasedandmodels-catalog-routealready do: every case there resets the cache, so each request is a cold full-catalog build, and the 8s production bound answers 503catalog_build_timeouton a loaded runner before a single catalog-shape assertion runs. Build latency is not what those tests cover.Not included
check-translation-driftis still red on the tip (8 drifted sources, all from merged PRs). The refresh is running against the translation backend and will land separately — it is 528 files and does not belong in this PR.No suppressions added, no baseline widened.
Three more found by this PR's own CI (all reproduce on the pure tip)
check:deps—@opencode/pluginis not in the dependency allowlist. fix(opencode-plugin-v2): port catalog publishing to stable provider contract #14370 ported the opencode plugin to the v2 contract, which lives under a different scope from the already-approved@opencode-ai/plugin. A scope change is exactly the slopsquatting shape that gate exists to catch, so it was verified before allowlisting: npm shows both under the same maintainer (thdxr <d@ironbay.co>, who also publishes@opencode-ai/plugin),@opencode/plugincreated 2026-09-02. Legitimate v2 scope of the same publisher; the verification is recorded in the allowlist's_justificationsentry.hard-session-lease-bypass-inventory— fix(sse): retry empty translated stream turns through the normal path #14213's bounded empty-turn retry added a credential-resolution site inopen-sse/handlers/chatCore.tsthe frozen inventory did not list. Inventoried as class B: it resolves through the normalgetProviderCredentialspath and does not reach past the lease, so it is inventoried rather than exempted.check:file-size—tests/unit/chatcore-translation-paths.test.ts3546 → 3564. This one is this PR's own growth: the sharedDEEPSEEK_RESPONSES_CREDENTIALSconst plus the comment recording why the connection shape changed and that no assertion did. Rebaselined with that attribution rather than condensing the explanation away.i18n drift — resolved elsewhere
While this PR was in flight another session landed #14532, refreshing the mirrors of the same 8 drifted sources. The refresh this session had running was therefore stopped and its output discarded rather than pushed on top. One source (
docs/reference/FEATURE_FLAGS.md) has drifted since, anddocs/reference/ENVIRONMENT.mdre-drifts when this PR's env-var row lands — both are follow-up refreshes, not blockers for this drain.