Repository navigation
provider-catalog-sync: enumerateProxy trusts the /v1/models shape — a missing data field silently flags every model as a live vanishing, and an id-less entry crashes --apply #2827
Description
Problem
bin/provider-catalog-sync.mjs trusts the shape of a proxy's /v1/models response (and of --source-dir snapshot files) past "it parsed as JSON". enumerateProxy (lines 443–451) is:
return (data.data ?? []).map((m) => m.id);with no check that data is an object, that data.data is an array, or that the mapped ids are non-empty strings. The header contract (lines 22–25) promises the opposite failure shape: "A provider that fails to enumerate degrades to a per-provider note, never a failed run", and additions_validated is "the only class --apply will ever add" (line 8). Two verified consequences on current main (1281cb37):
- A parseable 200 body with no
datafield is silently treated as a live empty model list.data.data ?? []turns a shape drift (e.g.{}or{"unexpected": []}from a proxy whose response format changed) intolive_count: 0with NO error status/note, andclassifythen flags every model in the config as a vanishing withvanishings_basis: "live". A reader of the report (or a catalog PR body built from it) sees "the provider serves nothing" instead of "the provider's response shape changed". - An entry without a string
idpoisons--applyand crashes the whole run. An entry like{"name": "no-id"}maps toundefined, which lands inadditions_validated(serialized asnullin the report). That class is exactly what--applyacts on:applyAdditions→makeEntry(line 408) →prettifyModelName(line 352,modelId.split('/')) throwsTypeError: Cannot read properties of undefined (reading 'split')— outside the per-provider try/catch inrunSync(line 511), so one malformed entry from one proxy fails the entire sync instead of degrading that provider.
Other malformed shapes DO degrade correctly, which shows the gap is missing validation, not design: data: "oops" → not-enumerable ("(data.data ?? []).map is not a function"), data: [null] → not-enumerable ("Cannot read properties of null (reading 'id')"). Only the missing-data and missing-id shapes slip through silently / fatally.
Evidence (this run, laptop probe with the real module)
Probe: ~/git/fleet/probe-2026-10-30-laneB-catalogsync/ on the laptop — real bin/provider-catalog-sync.mjs (md5 521dfab189aa83e1202691ba48f195b6, identical on sandbox main 1281cb37 and the laptop copy), driven through the exported runSync with --source-dir-style snapshot files; config fixture has provider claude-code (port 18749) with models model-a/model-b and commandcode (18748) as a healthy control.
- Snapshot
18749.json = {"unexpected": []}→{"status":null, "live_count":0, "additions_validated":[], "vanishings":["model-a","model-b"], "basis":"live", "note":null}— silent false vanishings. - Snapshot
18749.json = {"data":[{"name":"no-id"}]}→{"live_count":1, "additions_validated":[null], "vanishings":["model-a","model-b"], "basis":"live"}; thenapplyAdditions(...)→THREW TypeError Cannot read properties of undefined (reading 'split'). - Snapshot
{"data":"oops"}→status:"not-enumerable"with explanatory note (intended degradation — control). - Snapshot
{"data":[null]}→status:"not-enumerable"with explanatory note (intended degradation — control). - Snapshot
{"data":[{"id":"model-a"},{"id":"model-b"}]}→live_count:2, no additions/vanishings (valid control).
The same code path serves the live HTTP enumeration (fetchJson → res.json() → same .map), so a live proxy returning either shape hits the same outcomes; snapshots are just the reproducible offline door the tool itself documents.
Why it matters
This tool's whole job is trustworthy drift classification for the provider catalog, and its safety story rests on two promises: enumeration failures degrade per-provider, and only validated additions are ever applied. A proxy whose /v1/models shape drifts (a real event in this fleet — the proxies are actively rewritten) currently either manufactures a full-catalog "vanishings" alarm with a live basis, or takes down the scheduled sync run entirely at --apply time with a stack that names neither the provider nor the offending entry.
Acceptance criteria
-
enumerateProxyvalidates the response shape: body is an object,datais present and an array, and every entry yields a non-empty stringid; any violation throws an enumeration error naming the port/shape problem, sorunSyncrecords that provider asnot-enumerablewith a note (the existing degradation path). - A missing or non-array
datafield is never classified as a live empty list (novanishings_basis: "live"vanishings from an unvalidated body). - Entries without a valid string
idnever reachadditions_validated, and--apply/applyAdditionscannot be crashed by an enumeration result. -
bin/provider-catalog-sync.test.mjsgains cases for: no-databody, non-arraydata, id-less entry, and null entry — each asserting the per-providernot-enumerableoutcome (or the pinned equivalent), plus the valid-list control unchanged. - Valid enumerations (live and snapshot) produce identical reports to today.
Activity
dezren39 commentedon Oct 30, 2026
🤖 Agent note · state: filed
databody silently flags all config models as live-basis vanishings, and an id-less entry lands in additions_validated and crashes --apply in prettifyModelName; string/null shapes correctly degrade to not-enumerable (controls).dezren39 commentedon Oct 31, 2026
🤖 Design-architect pass 26 (2026-10-31) — agent note: this issue is adopted as part of Phase 2 of design #3068 ("parsed is not validated"). Triage: do NOT re-file, re-scope or split it — record the phase structure instead. The design records the rule this phase lands under: validate the shape you deref at the boundary, before any side effect; absence of a relied-on field is a shape failure, not an empty collection; failure is the tool's own named error/diagnostic (naming file + key for config files, the module's own error type for response readers) with no partial work; one shared probe matrix (null, array, scalar, {}, each relied-on field wrong-typed). Phase-specific decisions are on #3068 and in files/designs/code-parsed-is-not-validated.md (goal workspace).
10 remaining items
dezren39 commentedon Nov 1, 2026
🤖 Agent note · picked up
ses_e5e6972b3d6ffec1coEHS8pnxg· queue task5433(small_fix)/v1/modelsshape validation inenumerateProxyand regression tests.dezren39 commentedon Nov 1, 2026
🤖 Agent note · done
ses_e5e6972b3d6ffec1coEHS8pnxg· queue task5433(small_fix)/v1/modelsresponse shapes before using model IDs.node --test bin/provider-catalog-sync.test.mjs— 21 passed;git diff --check— passed. GitHub currently reports the PR open with only a neutral CodeRabbit check; CI is not yet green.data, invalid entries, and empty/non-string IDs now flow to the existing per-providernot-enumerablepath; valid IDs/reporting are preserved.dezren39 commentedon Nov 3, 2026
agent-note:v1
state: picked-up
worker: repo-worker-code (lane c8b9355e-319a-4640-8096-7ad19480d151, openchamber scheduled lane)
task: 7979 (triage)
Verdict: CONFIRMED still unfixed at origin/main 4c75cc8 —
enumerateProxystill returns(data.data ?? []).map((m) => m.id)on both the snapshot and live paths (bin/provider-catalog-sync.mjs:496-503).Resuming the existing fix PR instead of re-doing it: PR #3487 (branch queue/5433-2827-catalogsync-shape) had the validation + 5 regression tests but no CI and was 5 commits behind main. Merged origin/main into the branch (clean except import unions in the test file), reran the suite locally:
node --test bin/provider-catalog-sync.test.mjs→ 35 pass / 0 fail, pushed as 275d47d. Verifying CI next; merge when the required basic check is green.Merge pull request #3487 from developing-today/queue/5433-2827-catalo…
dezren39 commentedon Nov 3, 2026
agent-note:v1
state: done (fixed, merged)
worker: repo-worker-code (lane c8b9355e-319a-4640-8096-7ad19480d151, openchamber scheduled lane)
task: 7979
Result: PR #3487 MERGED (merge commit f04a439, verified on origin/main:
modelIdsFromProxyResponseat bin/provider-catalog-sync.mjs:496 with both enumerateProxy paths using it).What landed (matches every acceptance criterion):
modelIdsFromProxyResponsevalidates the parsed body: non-null object, owndataproperty present,datais an array, every entry is an object with a non-empty stringid; each violation throws naming the index/field, sorunSync's existing per-provider try/catch recordsnot-enumerable+ note.datacan no longer be read as a live empty list, so novanishings_basis: "live"false alarm from a shape drift.additions_validated;applyAdditionscan no longer hitprettifyModelName(undefined).bin/provider-catalog-sync.test.mjsgained four negative snapshot cases (missing data, non-array data, entry without id, null entry) each assertingnot-enumerable+ no live basis, plus the valid-list control.Evidence:
node --test bin/provider-catalog-sync.test.mjs→ 35 pass / 0 fail (run locally at the merged head, after merging origin/main into the PR branch).CI note: no PR-tier check ran for this PR —
.github/workflows/nix-ci.yml(which ownsbasic: proxy port fallbacks) triggers onpush: branches: [main]andscheduleonly, despite the policy text saying it "runs on every PR", sobin/-only PRs get no basic-tier verification. Filed separately as a coverage gap rather than treating a missing check as green.developing-today commentedon Nov 3, 2026
agent-note:v1
state: triaged (coverage pass)
Coverage pass 2026-11-03: reviewed in the fleet-wide per-issue pass (drift / overlap / already-handled / stale / blockers / ordering). Verdict: still live; queued in repo-health-code-pilot as triage at priority 30 (priority/low). If this is already handled at current main, the triage closes it with evidence; if it needs splitting, the triage files the split.
Found via: coverage pass 2026-11-03