Skip to content

fix(ci): clear the orphan base-reds on release/v3.8.51 — the #12867 pipeline extraction, a legacy-DB boot abort and the catalog readers - #13349

Merged
diegosouzapw merged 22 commits into
release/v3.8.51from
fix/release-v3.8.51-basereds-orphans
Sep 14, 2026
Merged

diegosouzapw merged 22 commits into
release/v3.8.51from
fix/release-v3.8.51-basereds-orphans

Conversation

@diegosouzapw

Copy link
Copy Markdown
Collaborator

Clears the orphan half of the base-red on release/v3.8.51 (#12732) — the failures that no other open PR was covering. Eleven test files that are red on the clean tip af49d497 are green here.

⚠️ base-red inherited: #12732 — No new ESLint warnings (owned by #13215), Fast Quality Gates → pack-policy (owned by #12929) and open-sse-typecheck (owned by #13216), plus the redaction family owned by #12945 / #13036 / #13227. None of those are this branch's defect and none are touched here.

The dominant root cause is #12867, not #12506

Four of the five production fixes trace to the same commit: #12867 (d6f31501) extracted the provider execution leg out of chatCore.ts into open-sse/handlers/chatCore/providerExecutionPipeline.ts and re-implemented the non-2xx branch with an inline JSON.parse instead of delegating to parseUpstreamError(). Four behaviors went missing, silently, for every request routed through the new pipeline:

lost behavior user-visible effect
parseUpstreamError() message extraction a plain-text 429 surfaced as the empty statusText — upstream error — instead of the upstream wording
body-derived retryAfterMs Please retry after 20s never drained the runtime reservoir, so the limiter was never locked
recordRateLimitBody (plumbed through PipelineStateHooks, wired at two call sites, never called) updateFromResponseBody() stopped running entirely on this path
upstream error.code / error.type gates keying on the pair — isAntigravityMissingProjectError — could never fire, so a config-class 422 degraded into a generic 5s account cooldown

The same extraction also moved the post-response block behind if (stream), which dropped the 401/403 credential refresh and the failure-state persistence for the non-streaming leg: a Copilot token was never retried after a 401, and per-model quota locks lost their helper references. Both legs now drive one shared refresh.

Everything else

  • src/lib/db/core.ts — a real boot bug. fix(db): call_logs provider stats read true on empty and legacy data #12832 declared CREATE INDEX idx_cl_request_provider ON call_logs(request_type, provider) inside SCHEMA_SQL, which runs before ensureCallLogsColumns() heals a legacy table. On any install predating request_type the whole db.exec(SCHEMA_SQL) aborts with no such column and the server never opens the database. The index moved next to the other request_type indexes, after the healing.
  • autoCombo/virtualFactory.ts iterated getCustomModels() raw; one null row in the operator-writable blob killed every auto/* pool with a TypeError.
  • activeSyncedCatalog.ts — fix(catalog): union picker customModels into the dispatch-time live catalog (#12597) #12934 unioned customModels into getAllActiveSyncedModels() alongside the dispatch readers it actually needed. That reader is not a dispatch reader, and /v1/models has its own custom-model pass right after, so a custom embedding came out as a parentless jina/<id> primary with the registry's jina-ai/<id> demoted to its child. The dispatch trio keeps the union; only this reader loses it.
  • imageGeneration.ts — codex image errors crashed reading fields off a body that fix(security): harden public error boundaries #12506's sanitization had already rewritten.

Guards were re-pointed, never relaxed

Three source-scanning guards broke because the code they read moved, not because the invariant broke. Each was rewritten to follow the code and each came out stricter:

kiro-tool-call-validation was the one genuinely stale test: #12506 made an upstream error frame terminate the stream after forwarding response.failed, a contract its sibling stream-passthrough-error-redaction already encodes for nine cases. The harness was switched to a reader; all four content assertions are intact and one was added.

Validation

file tip af49d497 this branch
antigravity-missing-project-chat 1 fail 2/2
chat-rate-limit-body-lock 2 fail 2/2
chatcore-hierarchical-admission 1 fail 2/2
chatcore-translation-paths 2 fail 74/74
combo-provider-cooldown-sibling 1 fail 9/9
db-core-init 1 fail 14/14
hard-session-lease-bypass-inventory 2 fail 3/3
image-generation-handler 3 fail 47/47
kiro-tool-call-validation 1 fail 6/6
models-catalog-route 10 fail 44/44
provider-execution-pipeline — 14/14
non-streaming-provider-leg — 39/39

typecheck:core, typecheck:noimplicit:core and check:cycles are clean. A 2664-test battery over every chatcore-*, chat-*, pipeline, db-* and catalog file was run on this branch and on the clean tip for comparison: eight failures are fixed here, and the deltas that only appear on this branch all pass in isolation — they are the CATALOG_BUILD_TIMEOUT_MS cold-build flake this PR documents (an 8s bound against a 10-13s tsx cold build under load), not regressions. Repo-wide npm run lint still reports the three inherited no-explicit-any errors owned by #13215 and nothing from these files.

Two production fixes ship with new TDD regression tests that fail on the base with the original error.

Merge-order notes

Refs #12732

…vider pipeline

#12867 lifted the non-2xx branch out of chatCore.ts into
providerExecutionPipeline.ts::toOutcome, but reimplemented it instead of
delegating, dropping three behaviors the non-streaming failure path relies on:

1. A non-JSON upstream body fell into the inline JSON.parse catch and surfaced
   as the (empty) statusText — "upstream error" — discarding the text the
   client needs. Now parseUpstreamError() supplies the message, as chatCore did.
2. The body-derived retry-after ("Please retry after 20s") was never parsed:
   retryAfterMs was hard-coded null into applyStatusRestatement/createErrorResult.
3. recordRateLimitBody was plumbed through PipelineStateHooks and wired at both
   chatCore call sites, but never called — so updateFromResponseBody(), which
   drains the runtime reservoir on a body-derived 429, silently stopped running
   for every request routed through this pipeline.

restatement rules also now match against the real upstream payload rather than
the request body that transformedBody carried whenever the parse failed.

Regression guard: tests/unit/chat-rate-limit-body-lock.test.ts (2 base-red
failures on the release/v3.8.51 tip) — both green; provider-execution-pipeline
13/13; typecheck:core clean.
#12832 declared `idx_cl_request_provider ON call_logs(request_type, provider)`
inside SCHEMA_SQL. That block runs before ensureCallLogsColumns() heals a legacy
call_logs table, so on any lineage predating the request_type column the CREATE
INDEX aborted the whole schema exec with "no such column: request_type" and the
server never finished opening the database.

Move the index next to the other request_type/combo indexes in
ensureCallLogsColumns(), which runs after the ALTER TABLE healing (and is also
called on the in-memory path), so both fresh and upgraded databases get it.

Proven by tests/unit/db-core-init.test.ts, "legacy call_logs schemas are upgraded
before combo target indexes are created" — failing on the release tip, green now.
… pools

prepareVirtualAutoComboInputs() reads getCustomModels(providerId) straight into
`for (const m of customModels) if (m.id ...)`. That blob is operator-writable and
is returned as raw parsed JSON, so a null / non-object row threw "Cannot read
properties of null (reading 'id')" and EVERY auto/* combo failed to materialize
("[catalog] Could not materialize built-in auto model auto/<id>"), silently
degrading the whole zero-setup routing surface to its minimal catalog entries.

Filter the rows to objects first, the same way the /v1/models custom-model pass
already does. Regression test in combo-auto-pool-visible-only.test.ts fails with
the original TypeError before the guard.
#12934 unioned customModels into getAllActiveSyncedModels() alongside the
dispatch-time readers it was actually fixing (#12597: getActiveSyncedCatalog,
reconcileProvidersWithActiveSyncedCatalog, getActiveProvidersWithSyncedModel).

getAllActiveSyncedModels() is not a dispatch reader. Its three consumers read it
as "what the provider's live sync reported": /v1/models feeds its synced-emission
loop (and has a separate custom-model pass right after, which owns the
specialty-registry dedupe, hidePaid and the vision overrides), /api/models uses it
to decide whether an exclusive-listing provider suppresses a static row, and
getSyncedAutoAliases derives tier aliases from it. Blurring custom rows into
"synced" made a custom embedding/rerank model on jina-ai come out as the alias
row `jina/<id>` (parentless primary) with the registry's canonical
`jina-ai/<id>` demoted to its child — the inverse of the identity every other
specialty model of that provider carries, and it also dropped the registry's
`dimensions`.

Drop the union there only; the dispatch trio keeps it, so #12597's tests and the
400-on-picker-model fix are untouched. Locked by a new case in
custom-models-live-catalog-12597.test.ts and by both jina cases in
models-catalog-route.test.ts ("does not duplicate imported/custom Jina specialty
models"), which encode the two identities side by side.
… modules

#12746 (executeTarget → gates/attempt/loop) and #12811 (round-robin) moved the
combo dispatcher out of open-sse/services/combo.ts. The #5976 invariant moved
with it intact — executeTargetAttempt.ts:1169 and roundRobinCombo.ts:1049 both
still exclude a 500/429 on a per-model-quota provider before recording a provider
cooldown — but the guard only read combo.ts and went red on an empty file.

Scan the whole combo surface instead of one filename and require the skip in
EVERY module that calls recordProviderCooldown(), with the pattern matched on
whitespace-normalized source so a prettier rewrap cannot silently disarm it. The
invariant asserted is unchanged; the guard is now stronger than the single
substring check it replaces.
…traction

Four drifts, all from PRs merged into the tip on 2026-09-07:

- #12867 extracted chatCore.ts's streaming execution loop into
  chatCore/providerExecutionPipeline.ts. Its two getProviderCredentials() sites
  now go through the injected `connection.getProviderCredentials` handle, which
  the bare-identifier AST scan never saw — the sites would have left the
  inventory unnoticed. Count property-access calls too and inventory the new file.
- #12867 also re-expressed the codex 429 managed-lease fence: the inline
  `provider === "codex" && !managedLease` became `allowAccountRotation:
  !managedLease && …` in chatCore.ts, gated in the pipeline as `canRotateAccount`.
  Assert both halves of that seam instead of the vanished inline form.
- #12746 moved combo.ts's getProviderConnectionById into
  combo/executeTargetGates.ts (class B, unchanged).
- #12805 added the Grok Build reset-credit path: src/lib/usage/grokResetCredits.ts
  (class B — same isConnectionUnavailableToAuxiliaryActivity fence as its codex
  sibling, so it also joins auxiliaryIsolationSources) and
  src/app/api/usage/codex-reset-credit/route.ts (class C — resolves the
  connection's provider to pick a library, never to serve a request).

Also pin CATALOG_BUILD_TIMEOUT_MS in models-catalog-route.test.ts: #12627's 8s
cold-build bound is sized for a warm production process, and a tsx test runner
building the full catalog from a fresh SQLite file crosses it (10-13s observed),
returning a `catalog_build_timeout` error body with no `data` array. The bound's
own behavior stays covered by 12627-catalog-inflight-timeout.test.ts.
…eline

parseUpstreamError() already returns errorCode/errorType; the pipeline was
dropping both on the floor. Gates that key on the pair — notably
isAntigravityMissingProjectError — never fired, so a config-class 422 turned
into a generic account cooldown instead of a fail-closed answer.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…racts

Regression guards for the two defects above: the pipeline must surface the
upstream error code/type, and a Kiro error frame now terminates the stream
after forwarding response.failed (the #12506 contract the sibling
stream-passthrough suite already encodes).

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…mage errors

Three more regressions from the #12867 pipeline extraction, all red on the
base tip:

- the non-streaming leg stopped refreshing credentials after a 401 and
  stopped persisting failure state, so a Copilot token was never retried
  and per-model quota locks lost their helper references;
- handleImageGeneration (codex) crashed reading fields off a body that
  #12506 now sanitizes before it reaches the classifier;
- the pipeline discarded the raw upstream body, which provider-error
  classification needs (quota vs rate-limit vs ban) — surfaced as
  rawMessage/upstreamBody, both internal to the classifier.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
…l-arg fix

#13009 taught the CLI registry parser to read positionals declared with
.addArgument() but did not re-run the generator, so check:agent-skills-sync
has been red on the base tip ever since. Regenerated with the official
generator (--apply --only=cli-tunnel); the only delta is 'tunnel create'
gaining its [type] positional.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Two new base-reds appeared on the tip while this branch was being built, both reproduced on the clean tip af49d497 before anything here touched them:

  1. Docs Gates (fast-path) — the EURouter (feat(providers): add EURouter as an OpenAI-compatible gateway #13025) and GreenPT (feat(providers): add GreenPT as an OpenAI-compatible provider #13024) merges took the provider count from 356 to 358 without updating docs/reference/PROVIDER_REFERENCE.md and six SVGs. Already owned by docs: sync provider count to 358 #13347 (@maheswari-pinneti), which covers exactly those seven files. Not touched here.

  2. Merge integrity → check:agent-skills-sync — orphaned, so it is fixed here in 480f4a2b. fix(skills): read positionals declared with .addArgument() #13009 taught the CLI registry parser to read positionals declared with .addArgument() but never re-ran the generator, so skills/cli-tunnel/SKILL.md has been stale on the base ever since. Regenerated with the official generator (--apply --only=cli-tunnel); the whole delta is tunnel create gaining its [type] positional.

⚠️ Operator attention: that commit touches skills/**/SKILL.md, which AGENTS.md classifies as an agent-instruction surface requiring explicit approval before merge. It is machine-generated and two lines, but please eyeball it rather than waving it through with the rest.

Also confirmed on the clean tip, so still not this branch's defect: No new ESLint warnings is the three no-explicit-any errors owned by #13215, and Fast Quality Gates is pack-policy (#12929) plus open-sse-typecheck (#13216).

Four base-reds that appeared on the tip today, none owned by any open PR:

- providers-constants-split / provider-node-reserved-prefix: GreenPT (#13024)
  and EURouter (#13025) move the totals 238 -> 240 and 408 -> 410.
- provider-translate-path-golden: same two providers, regenerated with
  UPDATE_GOLDEN=1; the snapshot delta is purely additive.
- glm-sse-transform-arity: #12925 declared the buffer-size slot GLM had been
  filling since #12179. The invariant is unchanged — the value must land in a
  real parameter, never a dropped positional — so the guard now checks it from
  the helper side and requires the named constant at the call site instead of
  a magic literal.
- injection-guard-scan-bound-3932: #13104 split the 16 KB budget between head
  and tail, so 'outside the window' is now the middle of an oversized body,
  not its end. The 16 KB ceiling these cases protect is untouched.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

Copy link
Copy Markdown
Collaborator Author

Another four base-reds surfaced on the tip from today's merges, none of them owned by an open PR, so they are fixed here in 555d97f1. All four were reproduced on the clean tip first.

guard what changed under it fix
providers-constants-split GreenPT (#13024) + EURouter (#13025) 238 → 240
provider-node-reserved-prefix same two providers 408 → 410
provider-translate-path-golden same two providers regenerated with UPDATE_GOLDEN=1; the snapshot delta is purely additive, no existing provider moved
glm-sse-transform-arity #12925 declared the buffer-size slot GLM had been filling since #12179 guard re-pointed
injection-guard-scan-bound-3932 #13104 split the 16 KB budget between head and tail cases moved to the middle

The last two are worth a second look because they invert what the old assertion said, so I kept each invariant explicitly rather than relaxing it:

  • GLM — the property was never "no 16th argument", it was "the buffer size must land in a real parameter instead of being dropped as an extra positional". The guard now checks that from the helper side (the slot is declared, and declared last) and requires the call site to pass the named constant, so a magic literal cannot creep back and hide the next mismatch.
  • Injection scan — fix(security): scan both ends of an oversized body, not just the front #13104 reshaped the window from head-only to head-plus-tail, so "outside the window" is now the middle of an oversized body, not its end. Both cases moved the marker to the middle. The 16 KB ceiling these cases exist to protect is untouched, and the export assertion on MAX_INJECTION_SCAN_BYTES is unchanged.

Still not fixed here and still red: check-docs-counts-sync and Docs Gates, both the same seven 356 → 358 drifts owned by #13347 (@maheswari-pinneti).

…d sweep

- stryker.conf.json: register three covering tests that drifted out of
  tap.testFiles on the base tip (combo-predicates-epoch-cooldown,
  opencode-400-model-unavailable, chat-correlation-id-exhaustion). Reproduced
  on the clean tip, so this is inherited drift, not this branch's.
- file-size: models-catalog-route.test.ts was already at its frozen ceiling, so
  the one-line CATALOG_BUILD_TIMEOUT_MS pin cannot be absorbed. Comment trimmed
  to a single trailing note and testFrozen bumped 1652 -> 1653 with the
  rationale recorded in the baseline.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
@diegosouzapw

diegosouzapw commented Sep 11, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status of the quality gates on this branch, after b1d58c51.

Cleared here:

  • mutation-test-coverage — three covering tests had drifted out of tap.testFiles on the base tip (combo-predicates-epoch-cooldown, opencode-400-model-unavailable, chat-correlation-id-exhaustion). Reproduced on the clean tip first, so it is inherited drift, not this branch's. Registered.
  • file-size — models-catalog-route.test.ts was already sitting exactly on its frozen ceiling, so the one-line CATALOG_BUILD_TIMEOUT_MS pin could not be absorbed. The explanatory comment was collapsed into a single trailing note and testFrozen bumped 1652 → 1653, with the full rationale recorded in file-size-baseline.json alongside the existing precedents.

Needs your call — complexity-ratchets, and it is genuinely this branch's:

open-sse/handlers/chatCore.ts goes 10 → 12 cyclomatic violations and 3 → 4 cognitive. The cause is structural, not substantive. Restoring the failure-state persistence for the non-streaming leg required naming the block so both legs could call it, and the ratchet counts violations per function — so a block that cost nothing while inline costs three once it has a name. The same change made the file measurably simpler:

tip this branch
handleChatCore cyclomatic 1032 971
handleChatCore cognitive 1245 856

I tried three ways out and rejected all three, so you are not re-treading them:

  1. Move it to its own module — I built it, and it does clear the gate (57 vs 57). But the block closes over roughly twenty locals of handleChatCore (currentModel, lockModel, hasPerModelQuota, coreOwnedAntigravityLockout, markAccountSemaphoreBlocked, …), and the extraction came back with 19 TypeScript errors that typecheck:core does not catch because it does not cover the open-sse/ workspace. Reverted rather than shipped.
  2. Suppress it inline — eslint.complexity-ratchets.config.mjs sets noInlineConfig: true, so the gate ignores disable comments by design. Correctly so.
  3. Split the ladder into small handlers — it is nine branches over PROVIDER_ERROR_TYPES, one of them 135 lines; it would take six or seven functions to get every piece under the 80-line and complexity-15 limits. That is a real refactor of the error path, with no new test coverage behind it, and not something I will slip into a base-red fix.

So the options are yours: accept the +2/+1 and re-ratchet at release, or land this and track the decomposition under #3501. Reverting the fix instead would put chatCore refreshes GitHub credentials after 401 and chatCore locks per-model quota failures back to red with no owner.

…e-v3.8.51-basereds-orphans

# Conflicts:
#	config/quality/file-size-baseline.json
…ough added

#12653 made /v1/images/edits fall through to the next combo target, and each
per-target attempt resolves credentials of its own — one more classified site
in that route (5 -> 6). Drift from the base tip, not from this branch.

Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
# Conflicts:
#	config/quality/file-size-baseline.json
#	open-sse/handlers/chatCore.ts
#	open-sse/handlers/chatCore/providerExecutionPipeline.ts
#	src/lib/db/models/activeSyncedCatalog.ts
#	tests/unit/hard-session-lease-bypass-inventory.test.ts
#	tests/unit/kiro-tool-call-validation.test.ts
… to what the tip still lacks

Reconciled against release/v3.8.51. The non-streaming credential refresh and
body-derived rate-limit lock this PR restored landed first via #13069, and the
codex image-generation stringify via #12945, so those halves are dropped in
favour of the tip's implementation. What remains is what the tip did not have:
the upstream error code/type (lost in the #12867 extraction, which let a
config-class Antigravity 422 degrade into an account cooldown), the legacy
call_logs boot abort, the malformed custom-models row guard, and the realigned
test guards.
@diegosouzapw
diegosouzapw merged commit 8786c17 into release/v3.8.51 Sep 14, 2026
9 of 11 checks passed
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…inference#12867 pipeline extraction, a legacy-DB boot abort and the catalog readers (cheaper-inference#13349)

Merged after a real reconciliation — this PR and cheaper-inference#13069 fixed the same cheaper-inference#12867 regression (the non-streaming leg losing failure classification and credential refresh) with different strategies, and cheaper-inference#13069 landed first. Rather than stacking two implementations, the tip's was kept and this PR was trimmed to what the tip still lacked.

**Dropped, already on the tip:**
- non-streaming credential refresh and failure-state persistence → cheaper-inference#13069 (`applyProviderFailureClassification`); the `persistProviderFailureState` hook this PR added would have been dead code
- body-derived rate-limit lock and non-JSON body message → already in the tip's pipeline (`chat-rate-limit-body-lock` passes there)
- codex image-generation stringify of a sanitized error body → cheaper-inference#12945 (`stringifyImageErrorForLog`, which would otherwise be declared twice — TS2393)
- the two test realignments (`hard-session-lease` inventory wording, kiro stream reader) → cheaper-inference#12945

**Kept, missing on the tip:**
- **upstream error `code`/`type` in the pipeline error outcome** — ported onto the tip's implementation. Running this PR's own test against the tip returned `errorCode: undefined` instead of `missing_project_id`, so a config-class Antigravity 422 still degraded into an account cooldown.
- legacy `call_logs` boot abort (index created after column healing)
- malformed operator custom-models row no longer kills every `auto/*` pool (`virtualFactory.ts` 1219 → 1230, annotated)
- realigned guards

**Evidence on the reconciled tree**
- 104 of 106 focused assertions. The 2 red (`models-catalog-route`, `provider-node-reserved-prefix`) fail identically on the pure tip. Against the tip, this PR turns 7 previously red cases green.
- ESLint, `typecheck:core`, `check:open-sse-typecheck` (0 errors), `check:changelog-integrity`: all clean
- changelog fragment rewritten to claim only what this PR still delivers

Unblocking this also surfaced a repo-wide red: the eight llm.txt mirrors added by cheaper-inference#13660 kept the pre-cheaper-inference#13216/cheaper-inference#13248 counts, which failed the pre-commit `docs-sync` gate for everyone. Fixed separately in cheaper-inference#13674.

`skills/cli-tunnel/SKILL.md` needed no change — cheaper-inference#13216 already landed the identical edit.

⚠️ base-red inherited: cheaper-inference#12732
@diegosouzapw
diegosouzapw deleted the fix/release-v3.8.51-basereds-orphans branch October 18, 2026 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant