Repository navigation
feat(api): flag a pinned account on /v1/combos steps without leaking the id - #11076
Conversation
…the id
`projectComboStep` strips `connectionId` on purpose, so two steps that pin
different accounts of the same provider project to identical objects:
{ "kind": "model", "model": "jina-ai/some-embed", "providerId": "jina-ai" }
{ "kind": "model", "model": "jina-ai/some-embed", "providerId": "jina-ai" }
A client reading that catalog cannot tell a two-account failover from the same
step listed twice, so an importing integration concludes the combo has no
failover at all.
Add `accountPinned`, derived from the same `connectionId` that stays stripped —
a boolean, not a prefix. It is set on every `model` step, so an absent field
means an older server rather than an unpinned step; a `combo-ref` routes through
another combo and has no account of its own, so it carries no flag.
The shape test is the one `providerId` already gets. `cleanupComboConnectionRefs`
deletes the key when the connection is deleted, so a step whose pinned account is
gone reports false rather than pointing at nothing.
Closes cheaper-inference#10968.
57ee123 to
8a3161d
Compare
|
Note on the one red check, since it is not from this PR and it is not only this PR. Merge integrity fails on its second half. The changelog half passes:
It traces to the base: So #11050 changed a generator input and the generated file was never rebuilt, and nothing since has rebuilt it. Any PR rebased onto that commit inherits the red — which is the exact scenario the comment in I rebased onto For completeness, the job is |
|
Same story for the other red check, No new ESLint warnings — different cause, also on the base. I ran the gate's own command locally, which writes the full inventory to One file, four errors, and it is not one of mine — this PR's diff is two files: Worth noting these are errors, not warnings, so they are not specific to the That accounts for both red checks on this PR, and both point at the base rather than the diff:
Both also fail identically on #11072 and #11074, while #11053 — cut before these landed — is green on both. Happy to send either fix as its own PR if useful; I have kept both out of this one. |
e06f8b7
into
cheaper-inference:release/v3.8.50
|
Final CI note, then I will stop adding to this thread. The four unit shards went red too, so here is the direct evidence that this diff is not in any of it. This PR's own tests run in shard 3, and all fourteen pass there: What shard 3 fails on instead: The webhook ones are the same root as the skills drift I flagged above — #11050 removed three events from Consistent across the board:
#11053 was cut before those commits landed and is green on all three. Nothing here is specific to this PR, and nothing here is specific to fork PRs either. Green on this PR: Vitest, Docs Gates, Build, dast-smoke, semgrep ×2, Change Classification. |
…the id (cheaper-inference#11076) ⭐5 — /v1/combos strip connectionId de propósito; dois passos pinando contas DIFERENTES do mesmo provider viram objetos byte-idênticos e um cliente conclui que não há failover. Adiciona accountPinned (boolean derivado do connectionId, nunca vaza o id) em cada passo model; sempre true/false, nunca em combo-ref. Sem rota/schema/mudança de caller. TDD. Fecha cheaper-inference#10968.
Summary
GET /v1/combosstripsconnectionIdon purpose, and must keep doing so. The side effect is that two steps pinning different accounts of the same provider project to byte-identical objects:A client reading that cannot tell a two-account failover from the same step listed twice, so an importing integration concludes the combo has no failover.
This adds
accountPinnedto each model step — a boolean derived from theconnectionIdthat still never leaves the server, not a prefix of it.Three decisions worth calling out, since the issue left them open:
pinnedAccountCounton the combo. The count says a combo has pins somewhere; the boolean says which of the duplicate-looking steps they are, which is the question a client is actually stuck on.modelsteps,trueorfalse. An absent field then means an older server rather than an unpinned step, so a client can tell "not pinned" from "cannot tell".combo-ref. That step routes through another combo and has no account of its own, sofalsewould be a claim rather than a fact.The flag uses the same shape test
providerIdalready gets.cleanupComboConnectionRefsdeletes the key when a connection is deleted, so a step whose pinned account is gone reportsfalserather than pointing at nothing.No route, schema, or caller changes — the three consumers (
/v1/combos, and both VS Code combo routes) pass the projection straight through, and/v1/combosis not described indocs/openapi.yaml, so nothing there needed updating.Related Issues
Validation
npm run lint—eslintclean on both changed filesrelease/v3.8.50); focused checks rerun afterwardMutation-checked, after confirming each edit actually applied:
So the new cases pin the derivation itself rather than passing incidentally — in particular the second mutation is what keeps
accountPinned: falsefrom being a value nothing asserts.Tests Added Or Updated
tests/unit/v1-combos-projection.test.ts— six new cases: a pinned step (asserting the id does not appear in the serialized output), an unpinned step, empty/non-stringconnectionIdvalues,combo-refcarrying no flag, malformed steps still returningnull, and the issue's own reproduction — two pinned accounts plus one unpinned step of the same provider and model, which now read as[true, true, false].#2300assertions gained the new key. The connectionId-drop assertions themselves are unchanged and still green, which is the point of the pair.Coverage Notes
src/app/api/v1/combos/projectCombo.tsis covered by the file above. The added branch has both outcomes asserted directly and both directions mutation-checked.Reviewer Notes
accountPinnednow appears in/v1/combos, the VS Code combo route, and the raw VS Code combo route alike, since all three shareprojectCombo. That seemed right — the same ambiguity exists on all three — but say the word if you want it scoped to/v1/combosonly.