Skip to content

fix(combo): treat a pin-only step as implicit connection allowlist - #12697

Merged
diegosouzapw merged 14 commits into
cheaper-inference:release/v3.8.51from
HouMinXi:fix/combo-pin-implicit-allowlist
Sep 7, 2026
Merged

diegosouzapw merged 14 commits into
cheaper-inference:release/v3.8.51from
HouMinXi:fix/combo-pin-implicit-allowlist

Conversation

@HouMinXi

@HouMinXi HouMinXi commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

A combo step with connectionId and no allowedConnectionIds was only a first-attempt pin. After 502/429, resolveForcedConnectionForCredentialPool dropped forcedConnectionId and getProviderCredentials scanned the whole provider pool.

Live effect: a combo pinned to one Claude Max account could retry on a sibling Pro account that is not in the combo.

This treats a pin-only step as implicit allowlist [connectionId]. An explicit allowlist is kept. Header-forced pins (x-omniroute-connection, not a combo step) still allow sibling fallback.

a11930ec4 already stops account rotation inside handleSingleModel when a pin is present. That does not populate allowedConnectionIds at resolve time, so the credential pool after pin-drop is still the whole provider. These are complementary.

Files

  • open-sse/services/combo/comboStructure.ts — pin-only / empty allowlist → [connectionId]
  • open-sse/services/combo/fingerprintExpansion.ts — rewrite composite pin ids to the real row
  • src/lib/combos/steps.ts — comboPinAllowlist / implicitPinAllowlist
  • src/shared/validation/schemas/combo.ts — keep allowedConnectionIds on PATCH
  • src/sse/handlers/chat.ts — apply implicit allowlist on the combo execution path

Test plan

DISABLE_SQLITE_AUTO_BACKUP=true node --max-old-space-size=8192 \
  --import tsx/esm --import ./open-sse/utils/setupPolyfill.ts \
  --import ./tests/_setup/isolateDataDir.ts --test --test-force-exit \
  tests/unit/combo-pin-implicit-allowlist.test.ts

10/10 pass on upstream/release/v3.8.51 @ 488f57e9d.

Covers: pin-only, empty list + pin, explicit allowlist kept, unpinned step unchanged, combo dispatch forwards the list, fingerprint unwrap, Zod parse, header-forced pins do not invent an allowlist.

Not in this PR

  • Combo executeTarget split (feat/combo-execute-target-split)
  • Changing a11930ec4 rotation gate

@HouMinXi
HouMinXi force-pushed the fix/combo-pin-implicit-allowlist branch from cf3f0e6 to c73528d Compare September 4, 2026 07:13
@HouMinXi

HouMinXi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Technical review (LOCAL, review-default, baseline c3945a724) on 4817b05da.

HOLD, 0 CONFIRMED, 7 UNCERTAIN. I am not restarting rounds on the same fingerprints.

Adjudication:

  • implicitPinAllowlist + explicit [] with a pin: by design, fail-closed. steps.ts:219-222 returns [pin] whenever the allowlist is missing or empty and a pin is present. tests/unit/combo-pin-implicit-allowlist.test.ts asserts allowedConnectionIds: [] + pin → ["20x-account"]. The comment on lines 210-212 is about the no-pin case.
  • hasForcedConnection / forcedConnectionId in handleSingleModelChat: same boolean, trimmed. Empty/whitespace pins become "" then length > 0 is false, matching the old typeof === "string" && trim().length > 0. forcedConnectionId || null vs ?? null only differs on "", which the old path already treated as unforced.
  • checkModelAvailable no longer has a local hasForcedConnection: true, unused after the pin-allowlist call. Preflight now goes through comboPinAllowlist(true, …).
  • comboStructure toTrimmedString(connectionId): whitespace pins were already broken (step.connectionId || null kept " " and later lookups miss). Trim is the implicit-allowlist input.
  • rewriteAllowlistIds mapping |fp| entries to realConnectionId: needed so a fingerprint-expanded target keeps the pin allowlist on the real row id. Dedup via Set. Not a silent drop of a second fingerprint; expansion already splits those into separate targets.

chat.ts own-growth frozen at 2458 after inlining the two comboPinAllowlist calls (_rebaseline_2026_09_04_12697_combo_pin_allowlist). Helper stays in src/lib/combos/steps.ts.

CI remaining on this head: merge-integrity (changelog.d/fixes/reset-aware-model-family.md missing - on the release tip), API Route Typecheck (glm.ts TS2554 on tip; only #12711 drops the extra positional), FQG mutation + open-sse-typecheck. Same fingerprint as #12732 and the other open PRs that do not touch those files. Eslint on this PR is green.

Contributor only — not merging.

@HouMinXi
HouMinXi force-pushed the fix/combo-pin-implicit-allowlist branch from 4817b05 to 6987910 Compare September 5, 2026 14:33
@HouMinXi

HouMinXi commented Sep 5, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/release/v3.8.51 @ 9d1a896c6 (#12670).

range-diff every unique commit =. All rewritten commits GPG G. behind=0 vs tip.

Own tests re-run on the new base. --force-with-lease to the previous fork SHA.

⚠️ base-red inherited: #12732 (FQG / unit shards / changelog-integrity / glm arity until #12770 lands). Files this PR does not touch.

@HouMinXi

HouMinXi commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto release/v3.8.51 tip f9a1cc8a9 (#12682 / #12691 / #12834).

698791076 → 5ac5072c8. range-diff 3 commits =. GPG G. No file overlap with the three landed commits.

A combo step with connectionId and no allowedConnectionIds was only
a first-attempt pin. After 502/429 the credential loop dropped
forcedConnectionId and scanned the whole provider pool, so
offical-fable leaked from a 20X account onto a sibling Pro.

Treat a pin-only step as implicit allowlist [connectionId], keep
that list through fingerprint-pin unwrap, and stop Zod from
stripping allowedConnectionIds on combo PATCH.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Preflight caches selected creds. A pin without allowlist scanned the
provider pool and could stash a sibling for dispatch.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
Inline comboPinAllowlist at the two credential-pool call sites and drop
the local pinAllowlist temps. chat.ts 2454->2458 is own growth at the
existing chokepoint; helper stays in steps.ts.

Signed-off-by: Minxi Hou <houminxi@gmail.com>
@HouMinXi
HouMinXi force-pushed the fix/combo-pin-implicit-allowlist branch from 5ac5072 to b6e11ae Compare September 7, 2026 02:34
@HouMinXi

HouMinXi commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto upstream/release/v3.8.51 b345c7f6c (#12870 OpenCode v2 plugin).

  • 5ac5072c8e3 → b6e11aec26c
  • range-diff identity (eq=3); all commits GPG G
  • 3 unique commit(s) ahead, 0 behind
  • --force-with-lease to HouMinXi:fix/combo-pin-implicit-allowlist

diegosouzapw and others added 11 commits September 7, 2026 08:35
…ibution (cheaper-inference#12772)

* chore(ci): guard commit identity in pre-commit to stop author misattribution

Two windows of commits in this checkout were signed with the wrong identity,
both caused by an identity override left behind by an automated session:
2026-08-13..26 (name "Xiangzhe" + @backryun's e-mail, 237 commits) and
2026-08-29..09-02 (name "Markus Hartung" + the maintainer's e-mail, 59 commits).
The .mailmap repairs the record after the fact; this gate stops the next window.

The gate is opt-in per machine via omniroute.expectedName / expectedEmail — with
no config it exits 0, so contributors who clone the repo are never affected. It
blocks three things: a committer that is not this machine's identity (which is
what BOTH windows looked like — in August neither the name nor the e-mail was
the maintainer's, so checking only their e-mail would have missed it), an author
carrying the maintainer's e-mail under someone else's name, and any address
listed in omniroute.legacyEmail.

Crediting a contributor with `git commit --author="Name <their@email>"` keeps
working, since the rule targets the committer and the maintainer's own address.

* test(ci): isolate the identity gate's test from the ambient git config

The "stays inert when the machine has not opted in" case read the real
global config, so on a machine that HAS opted in (omniroute.expectedEmail
set — the maintainer's own boxes, where this gate matters most) the gate
correctly refused a synthetic contributor identity and the test failed.
It only passed on a clean CI runner.

Neutralising GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM makes the opt-in state
come solely from what the test injects, so the suite is deterministic on
both an opted-in and a clean machine.
…nce#12770)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

O `65536` era o 16º posicional de um helper com 15 parâmetros — `TS2554` vivo no tip (`open-sse/executors/glm.ts:244`, confirmado aqui antes do board). O teste de guarda de aridade é o que impede a reincidência: ele checa a assinatura do helper e o call site, não o comportamento, que é exatamente onde o erro morava.

Obrigado por isolar isso do cheaper-inference#12711 em vez de deixar o `glm.ts` viajar junto com pin/combo-split/moonshot.
…heaper-inference#12711)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Além do bug do toast, esta PR foi a que derrubou os três base-reds vivos do tip: o fragmento `changelog.d/fixes/reset-aware-model-family.md` sem o `- ` inicial, o registro do `tests/unit/reset-aware-request-scope-12600.test.ts` no `stryker.conf.json` e o `TS2554` do glm. O `check-changelog-integrity` voltou a passar aqui por causa dela.

O diagnóstico do MouseEvent é o que dá o valor: `onConfirm` chegava como handler de clique nativo e `handleBatchDeleteConfirm` tratava qualquer primeiro argumento truthy como callback. O cinto (`typeof`) e o suspensório (o wrap no ConfirmModal) juntos estão certos — só um dos dois deixaria a porta aberta para o próximo caller.
…ce#12868)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

O `nodeMap` lido do closure de `RuntimePageClient` por uma função de nível de módulo é uma bomba-relógio silenciosa: só explode quando um monitor entra em error/exhausted/alerting, e o teste existente só alimentava listas vazias. Tirar o arquivo do exclude do vitest vale tanto quanto o fix — confirmei aqui que `tests/unit/ui/runtime-page-client.test.tsx` agora roda na `test:vitest:ui` e passa.

A anotação sobre o "内部服务器错误" ser o catálogo RSC da página, e não o crash, poupou o próximo a caçar fantasma.
…cheaper-inference#12733)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Available 0% com Voucher 100% e Cash 100% não é estado de carteira que exista — foi o sinal certo para puxar o fio. Tratar leftover como booleano só para Available e cravar 100% nos outros dois buckets é o tipo de defeito que passa despercebido enquanto a conta tem saldo.

Os dois testes cobrem os dois lados: o produtor e o caminho até `getQuotaRemainingPercentage` com `isCredits` + CNY.
…eaper-inference#12767)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

A separação entre o que é do service worker e o que é do Caddy está certa e é o que torna a PR mergeável: o `respondWith` em navegação é defeito nosso, o `Alt-Svc` mentindo h3 é config de proxy reverso e não tem o que fazer aqui.

O `/dashboardfoo` casando com `startsWith("/dashboard")` é um achado à parte, e o bump de cache v2→v3 é o que faz o worker antigo sair do ar nos clientes que já estão presos.
…gets (cheaper-inference#12475) (cheaper-inference#12926)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

A busca do sufixo mais longo primeiro (`-xhigh` antes de `-high`) é o detalhe que faz a herança funcionar em vez de quase-funcionar. Manter `getResolvedModelContextOverride` fora do escopo, com o teste existente registrando que aquele caminho continua sem herança, deixa a fronteira explícita.
… member (cheaper-inference#12899)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

Regressão de v3.8.50 vinda do cheaper-inference#9057, com o sintoma mais enganoso possível: `attempted: 0`. A política já tinha admitido o combo e a checagem era refeita em cada membro interno.

Manter o filtro por prefixo de provider e o `disableNonPublicModels` intactos é o que impede o short-circuit de virar um buraco na allow-list.
…ts (cheaper-inference#12805)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

A decodificação dos campos aninhados 10/20/30 do `GetRemainingResets` ao vivo (último commit) é o que separa isto de um palpite sobre o formato do frame. Mostrar zero em vez de esconder a linha é a escolha certa: crédito zerado é informação, ausência de linha é ambiguidade.
…nference#12803)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes.

`blockExtraUsage: false` significa "pode usar crédito extra", nunca "esconda a conta antes de despachar" — a conta saía da rota justamente quando o crédito extra existia para ser usado. Os 32/32 cobrem os quatro pontos onde a mesma decisão era tomada, e revalidei após o merge da base (32/32 de novo).

Nota de integração: o seu `isQuotaExhaustedForRequest` colidiu com o placeholder `_providerSpecificData` do cheaper-inference#12789 na worktree combinada. Ficou a sua implementação, que é a que de fato usa o parâmetro. A base foi mergeada na branch para resolver o `file-size-baseline.json` (aditivo, JSON revalidado).
@diegosouzapw
diegosouzapw merged commit 1b97f42 into cheaper-inference:release/v3.8.51 Sep 7, 2026
3 of 7 checks passed
@HouMinXi
HouMinXi deleted the fix/combo-pin-implicit-allowlist branch September 7, 2026 13:27
muhamadgalihsaputra pushed a commit to niyatna/NiyatnaRoute that referenced this pull request Sep 27, 2026
…heaper-inference#12697)

Validado numa worktree combinada com as 16 PRs desta leva sobre `release/v3.8.51`: typecheck:core limpo, check-file-size e check-changelog-integrity OK, complexity 2788/3218 e cognitive 1261/1437 (ambos sob a baseline), ESLint 0 erros nos 152 arquivos alterados, 771 testes unitários focados, 49 de integração e a suíte vitest:ui completa (2149) verdes. Após o merge da base nesta branch, os 11/11 do `combo-pin-implicit-allowlist` foram revalidados.

A distinção entre pin de step de combo e pin forçado por header (`x-omniroute-connection`) é o que salva a PR de virar uma restrição ampla demais — o header continua permitindo fallback para conexões irmãs, o step não.

Apontar que o `8f3f72591` para a rotação dentro do `handleSingleModel` mas não popula `allowedConnectionIds` no resolve foi a peça que explicou por que os dois são complementares e não redundantes. Sem isso a PR pareceria duplicar um gate que já existia.
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.

2 participants