feat(usai-provider): normalize and vendor USAi model catalog

[wz-gsa]

Summary

Adds the usai-provider kit's host-side USAi model-catalog normalizer and a small, explicitly non-authoritative metadata snapshot. The prior guest-written provider-facts mechanism is not part of this PR; it was removed because an agent with passwordless sudo can forge guest-owned state.

The normalizer now fails closed before writing output: each source entry must be an object with a non-empty id; present token limits must be positive integers; and present recognized cost fields must be non-negative finite numbers. Optional metadata may remain absent, zero price remains valid, and unknown source fields are deliberately excluded from the neutral shape.

What Changed

• Add scripts/normalize-catalog.mjs, a local host-side transform with no network or credential access.
• Vendor a four-model metadata-only snapshot at files/home/usai-config/model-catalog-snapshot.json.
• Add drift and metadata-only shape gates for the snapshot.
• Add malformed-input regressions for null/non-object records, invalid IDs, invalid/fractional limits, malformed costs, and non-finite or negative prices.

Security Impact

No authentication, authorization, routing, credential-pointer, or outbound-network behavior is added. The shipped snapshot contains model IDs, limits, and costs only; tests prevent routing and credential fields from being added.

Verification

node --check integrations/isolation/acq-kits/usai-provider/scripts/normalize-catalog.mjs  pass
node --test integrations/isolation/acq-kits/usai-provider/tests/normalize-catalog.test.mjs  12 pass / 0 fail
make test-kits  112 pass / 0 fail
CLI normalization of checked-in catalog  pass (20 models)
git diff --check  clean

make validate and make test were attempted but blocked by this checkout's missing Python development tools (jsonschema, PyYAML, and pytest); no validation failure was masked or bypassed. CI remains the authoritative full-environment verification.

Review

An independent adversarial review found and the branch now fixes fractional token-limit acceptance. Final re-review found no actionable findings.

AI-assisted (OpenCode [gpt-5.6-terra]) on operator instruction. Human review and merge required.

Rollback

Revert commit b3c18f8 to remove the validation hardening without altering the earlier catalog-normalizer implementation.

[mogul]

I think you should close this and start again based on these changes:
#423 (comment)

[mogul]

I think you should close this and start again based on these changes: [...]

@wz-gsa what's your plan for this one?

[wz-gsa]

You're right about the mechanism, and I'm going to act on it — but I did the rebase first to see exactly what conflicts, and it's narrower than "start again." Proposing a rescope rather than a close. AI-assisted analysis (OpenCode), advisory; I'm the author here, so this is a proposal, not a review.

The rebase is clean — so the conflict isn't with main

$ git diff --name-only <merge-base>..origin/main -- \
    integrations/isolation/acq-kits/usai-provider/ integrations/providers/usai/
(empty)
$ git merge-tree <merge-base> HEAD origin/main | rg -c 'CONFLICT|<<<<<<<'
(no conflict markers)

Nothing in main touches this kit. The conflict is with the redesigned ADR-0003 in #423 (ecfa36a → 051db58), which landed after this PR opened. I read your rationale in #423 and it's correct on the substance: the in-sandbox agent has passwordless sudo, so a root-owned guest path is not a trust boundary. This PR implements the old shape at spec.yaml:117-136 — a phase: startup / user: "0" command that cat >s the facts file into the guest. That's exactly what the new Layer 1 rejects.

But 80 of 473 lines conflict, not 473

File	+lines	Survives the redesign?
spec.yaml	80	No — the guest-write startup command
scripts/normalize-catalog.mjs	152	Yes — host-side authoring tool; ADR unchanged on it
tests/normalize-catalog.test.mjs	95	Yes
tests/fixtures/normalize-catalog-source.json	64	Yes
files/home/usai-config/model-catalog-snapshot.json	55	Yes — the new ADR wants this, just delivered via the :ro mount
scripts/verify (1b env-ownership)	27	Partial — the check moves host-side, the logic is reusable

The normalizer was already host-side-only and already emits acq-neutral-model-catalog/v1. The trust-boundary change doesn't touch it.

Proposal: rescope this PR, don't close it

1. Drop the self-registration startup command from spec.yaml entirely. This PR stops claiming to deliver Layer 1.
2. Keep the normalizer + tests + fixture + vendored snapshot, retitled to what they are: the provider's own catalog normalizer and vendored fallback.
3. Layer 1 lands as a follow-up on top of quickstart #504, as a static facts file plus host-side validation.

That last point is a sequencing constraint worth naming: Mechanism 2 (the :ro host-materialized mount) is implemented in quickstart #504, which is still open and BEHIND. Layer 1 can't be built correctly until it merges. Rewriting this PR now would mean writing against a mechanism that doesn't exist yet.

If you'd rather have one PR after #504 lands, say so and I'll close this — but I'd rather not leave 393 working lines rotting on a branch for however long #504 takes.

Separately: the facts shape needs revision, independent of the mechanism

While I had the thing open I surveyed how this is actually modeled in the wild — models.dev, OpenCode, LiteLLM, Vercel AI SDK, Cline, Continue — and then measured against live models.dev/api.json (223 providers, 8,173 models) rather than trusting the survey. Our draft is {schemaVersion, providerId, host, baseUrl, modelsUrl, keyEnv}. Findings that matter for a contract meant to carry many providers:

• Provider-level fields across all 223 are exactly {id, env, npm, name, doc, models, api}. No host, no modelsUrl, no keyEnv, no schemaVersion anywhere.
• env is an array on all 223, and 14 need more than one variable — ["CLOUDFLARE_ACCOUNT_ID","CLOUDFLARE_API_KEY"], ["DATABRICKS_HOST","DATABRICKS_TOKEN"], ["SNOWFLAKE_ACCOUNT","SNOWFLAKE_CORTEX_PAT"]. A scalar keyEnv cannot express Azure, Bedrock, or Vertex at all.
• providerId → id everywhere (models.dev, Cline, LiteLLM's litellm_provider). Nobody uses providerId at provider level.
• host has zero precedent in any of the six. It's new URL(baseUrl).host, and it becomes a lie the moment an operator overrides baseUrl. If it exists for egress allowlisting, that's a different field with a different name on a different layer.
• We're missing the API-dialect field, and every single surveyed tool has one — models.dev provider.shape: responses|completions, Cline protocol (6 values), OpenCode npm (182/223 are @ai-sdk/openai-compatible), LiteLLM custom_llm_provider. It is not inferable from baseUrl, and Azure needs it per model under one base URL. Without it a consumer can't form a request.
• baseUrl needs to be a template. 5 providers embed ${ENV_VAR} in api — https://${DATABRICKS_HOST}/ai-gateway/mlflow/v1. A plain string can't express them.
• modelsUrl should be relative to baseUrl. Ours is literally baseUrl + "/models", which desynchronizes the moment someone points baseUrl at a proxy. Cline rebases it deliberately. Listing-response shapes also disagree — Ollama is /api/tags with a different body — so a bare URL with no declared shape is a guess.

The one I'd most want to fix before v1 freezes

cost.context_over_200k is on 485 live models.dev entries, and their own contributor docs now call it "legacy output-only — schema rejects it on write, the generator may still emit it for old consumers." They had to add cost.tiers[] (567 entries) alongside it and keep emitting the old field forever. LiteLLM has the same wound three times (_above_200k_tokens, _above_272k_tokens, _above_128k_tokens). OpenCode v2 is actively normalizing it away.

A magic number in a field name is permanent. Making cost tier-capable now is free; retrofitting it isn't. Same for the unit — models.dev is per-million, LiteLLM is per-token, Vercel is per-token-as-string. A bare cost is a 1000× error waiting to happen, so the unit belongs in the field name.

And a forward-compat rule worth adopting explicitly

LiteLLM's schema states it in-band: additionalProperties: true, one required field, and a description telling consumers to ignore unknown fields rather than reject them. Four years and ~191 fields later it has never broken. OpenCode sets additionalProperties: false 29 times including on ProviderConfig, so a validator from version N rejects a config from N+1. Continue shipped a closed capability enum, broke consumers, and patched it with z.union([...literals, z.string()]) — the code comment cites the PR number.

Suggestion: strict on authoring (catch a kit author's typo in validate-kits.py CI), permissive on consuming (never reject a newer producer). None of the six does both cleanly; it costs one schema file and two validation profiles.

One thing worth keeping from this PR either way: the normalizer leaves an absent price absent. Cline's equivalent does model.cost?.input ?? 0, which turns "price unknown" into "free" — precisely the degraded-path defect this repo's own rules exist to prevent.

Three questions, all scope not direction

1. Does the shape revision belong as an amendment to your ADR-0003, or a separate ADR?
2. Is the rescope above acceptable, or do you want one PR after #504?
3. Who owns chore(isolation): formalize JSON Schema for acq-provider-facts/v1 and acq-neutral-model-catalog/v1 #435 (the formal JSON Schema)? Worth settling before two of us write one. My read: the survey above is the consumer evidence chore(isolation): formalize JSON Schema for acq-provider-facts/v1 and acq-neutral-model-catalog/v1 #435 was waiting for — we now know what 223 real providers need, so it no longer has to be guesswork against zero consumers.

Full measurements and the sequencing plan are written up locally; happy to paste any part of it here if useful.

[mogul]

Address now

1. Blocking: integrations/isolation/acq-kits/usai-provider/spec.yaml:117-136 still writes provider facts from inside the sandbox at startup as root. That is the mechanism the current PR discussion identifies as invalid for the redesigned provider-discovery trust boundary: the in-sandbox agent has passwordless sudo, so a root-owned guest path is not a trustworthy source of provider facts. Leaving this in means a future consumer would be encouraged to trust /var/lib/acq/models/providers/usai.json even though that file is materialized inside the mutable guest, not from the host-side/static :ro mechanism discussed as the replacement. Please remove or rescope the self-registration startup command and the dependent claims/checks before merging.

2. Blocking follow-on to the same issue: integrations/isolation/acq-kits/usai-provider/scripts/verify:77-102 validates the obsolete guest-written keyEnv heredoc by grepping spec.yaml. Once the facts file is no longer authored inside spec.yaml, this check either becomes inert or continues to bless the wrong mechanism. Please move this validation to the host-side/static facts source when that source exists, or drop it from this PR if this PR is rescoped to only the normalizer/snapshot.

Track later

1. The normalizer and tests look broadly reasonable, but the new neutral catalog shape is still unschematized. The existing follow-up GSA-TTS/agentic-coding-patterns#435 should remain the tracked place to define size limits, required fields, unknown-field behavior, and defensive parsing expectations for the first real consumer.

2. There is a pre-existing durable comment reference in integrations/isolation/acq-kits/usai-provider/spec.yaml:172 using quickstart#235. It is outside this PR's added hunk, so I am not treating it as a blocker here, but it still conflicts with the repository's durable-reference guidance and should be converted to prose or a fully qualified reference when that area is next touched.

Checks run

• Confirmed local HEAD matches GitHub PR head SHA a7dec19caab22974d7491ad207625abe3c33f583.
• Reviewed PR timeline/comments/reviews with gh pr view 436 -R GSA-TTS/agentic-coding-patterns --comments --json ..., gh api repos/GSA-TTS/agentic-coding-patterns/issues/436/timeline --paginate, gh api repos/GSA-TTS/agentic-coding-patterns/pulls/436/reviews, and gh pr diff 436 -R GSA-TTS/agentic-coding-patterns.
• node --check integrations/isolation/acq-kits/usai-provider/scripts/normalize-catalog.mjs passed.
• node --test integrations/isolation/acq-kits/usai-provider/tests/normalize-catalog.test.mjs passed: 8/8 tests.
• git diff --check passed.
• python3 integrations/isolation/acq-kits/validate-kits.py --strict integrations/isolation/acq-kits/usai-provider was attempted but blocked locally by missing jsonschema; CI already reports the kit validation job passing on this head.

Decision: request changes until the obsolete guest-written provider-facts mechanism is removed or replaced with the redesigned host-side/static mechanism.

[wz-gsa]

Rescoped to the half that's sound. Both blocking items addressed in b0f8980, CI 7/7. Follow-up filed as #456.

Blocker 1 — the facts file is gone (58 lines)

I verified your reasoning rather than taking it on trust, and it holds at a sharper source than the ADR:

quickstart origin/main:acq.backends/msb.sh
  printf "agent ALL=(ALL) NOPASSWD:ALL\n" > /etc/sudoers.d/90-acq-agent

acq writes that rule itself — unrestricted root, not an allowlist — and docs/BACKEND_GUIDE.md:473 states the same guarantee as the sbx base-image contract.

So the kit's own comment was wrong, and that's worth naming separately from the mechanism objection:

"the agent (uid 1000) cannot write concurrently (root-owned, agent has no write access to /var/lib/acq/)"

DAC-true, false as a security property — sudo tee defeats it. And because the file advertises keyEnv, a credential pointer, an agent that rewrites it can redirect a consumer at a different env var. That's a confused-deputy primitive shipping with a comment licensing consumers to trust it. Your "not a trustworthy source" framing was if anything understated.

Rather than delete silently, spec.yaml's header now records what was tried, why it was wrong, and what the replacement requires — so the next person doesn't reinvent it.

Blocker 2 — verify check 1b is gone

$ git diff origin/main -- integrations/isolation/acq-kits/usai-provider/scripts/verify
(empty)

Byte-identical to main — exactly this PR's addition removed, none of the pre-existing script touched.

Two things about that check worth recording, independent of the coupling you identified:

• It claimed to prove "the kit declares this env var itself" by counting occurrences of the value across the whole spec.yaml and requiring >= 2. The only two mentions were a header comment (:23) and the heredoc itself (:133). So a check the PR called "security-load-bearing" passed entirely on the strength of one comment line.
• It used grep -c, which counts matching lines, not occurrences. Verified: AAA AAA AAA on one line gives 1, not 3. So its own ($occurrences occurrences) message was wrong whenever two mentions shared a line.

Both are mine, both now deleted.

Two additions beyond the strict rescope

A reviewer on the consensus panel found two gaps I'd missed. Both are closed.

1. The snapshot had no drift gate. Nothing asserted model-catalog-snapshot.json matched normalizeCatalog(providers/usai/catalog.json) — a second hand-maintained copy free to diverge from its stated source. It's a deliberate 5-of-21 subset, so the new gate allows a subset but requires every kept entry to be byte-equal, and asserts both sides are non-empty first — otherwise an emptied snapshot makes the comparison pass vacuously.

Degraded paths exercised, not assumed:

mutated cost      -> FAILS "snapshot model 'claude_4_5_sonnet' has drifted from the normalizer output"
emptied models[]  -> FAILS "snapshot has no models[] -- nothing to compare, so this gate cannot pass"

2. The snapshot's comment was the same trust invitation one directory over — with weaker ownership. It said a consumer "has SOMETHING to read", and it ships to /home/agent/, which the agent owns — forging it needs no escalation at all. It's now documented as not authoritative, and a second test asserts the shape stays metadata-only (no host/baseUrl/modelsUrl/keyEnv/apiKey at envelope or model level), so a routing field can't be added later without a test failing.

On your "close and start again"

I kept the branch rather than restarting. The conflicting surface was 58 of 473 lines, all in spec.yaml plus the 26-line verify check — the normalizer, its tests, the fixture and the snapshot were unaffected. Restarting would have discarded this thread and your two line-anchored reviews for a byte-identical result. If you'd still rather see it as a fresh PR, say so and I'll open one.

Verification

node --test tests/normalize-catalog.test.mjs  -> 10 pass, 0 fail (was 8)
make test-kits                                -> 80 pass, 0 fail
validate-kits.py --root . --strict            -> All 7 acq-kits valid
python3 scripts/validate_repo.py --root .     -> exit 0
python3 scripts/scan_unsafe_shell.py .        -> 0 errors, 0 warnings
markdownlint-cli2                             -> 0 issues
git diff --check                              -> clean
gh pr checks                                  -> 7/7 pass

Track-later items, left as you scoped them

• chore(isolation): formalize JSON Schema for acq-provider-facts/v1 and acq-neutral-model-catalog/v1 #435 stays the tracked place for the catalog schema. Flagging that it's now load-bearing: acq-neutral-model-catalog/v1 here coexists with usai-model-catalog/v1 already shipped in integrations/providers/usai/ — two schemas for one job. I raised the same overlap on docs(isolation): add ADR 0004 for neutral model-provider discovery #423.
• The quickstart#235 durable-reference nit at the old spec.yaml:172 is outside this change; left alone.

One thing I found in the pre-existing verify script

Not this PR's doing, not fixed here, but you should know. scripts/verify on main is structurally two-state — ok()/bad() only, no "could not measure" reporter — and its verdict is [ "$fail" -eq 0 ]. I probed the shape in isolation:

pass=0 fail=0  ->  "All checks passed."  exit 0

Zero checks run reports success. Concretely on main: the missing-secret branch (:69-72) and the API-probe elif [ -z "$status" ] branch (:150-151) both write a NOTE to stderr and increment neither counter, and in_sbx() (:58) discards both stderr and exit status via 2>/dev/null || true, so "predicate false" and "sbx exec failed" are indistinguishable. A run with no secret and no network exits 0 saying all checks passed.

Happy to file that separately if useful — I didn't want to widen this PR to fix it.

---

AI-assisted (OpenCode), prepared on operator instruction. Advisory; human review and merge still required.

[wz-gsa]

@mogul — this is CI-green now (rebased onto `main` post-#471/#548/#478, so it picks up both the markdownlint-cli migration and the katex/smol-toml override fix). Your prior CHANGES_REQUESTED review was dismissed on Oct 5; per the Oct 1 thread both original blockers were addressed in `b0f8980` (guest-written provider facts dropped entirely rather than redesigned, follow-up filed as #456) and I think that's still accurate — just needed the CI catch-up pushed through today. Could you take another look when you have a chance?

[mogul]

Adversarial Review Report

Finding

1. High: the normalizer accepts malformed model records and produces invalid neutral catalogs
   Location: integrations/isolation/acq-kits/usai-provider/scripts/normalize-catalog.mjs:86-108

   normalizeCatalog() only checks that sourceCatalog.models is an array. normalizeModel() then creates { id: model.id } without validating that each item is an object or that id is a non-empty string. A malformed but parseable source therefore succeeds and writes an invalid artifact instead of failing closed:

   $ node --input-type=module -e "import { normalizeCatalog } from "./scripts/normalize-catalog.mjs"; console.log(JSON.stringify(normalizeCatalog({ models: [{}] })))"
   {"schemaVersion":"acq-neutral-model-catalog/v1","providerId":"usai","models":[{}]}

   null entries instead throw an unhelpful property-access TypeError. This tool is described as the regeneration path for a shipped catalog, so it needs explicit source validation before generating output: require each model to be a plain object with a non-empty string id; reject non-finite/negative numeric metadata and malformed cost values according to the source contract; and cover these rejected-input paths in tests. Otherwise a damaged or changed upstream catalog can silently replace the snapshot with unusable records.

Review Scope

• Reviewed the actual current head 7a4dc650af519ef837854bfcc2ef6023c4396ef3, not the stale PR summary. The guest-written provider-facts mechanism and its verifier check are no longer present. The live change is the host-side normalizer plus an explicitly non-authoritative, agent-writable metadata snapshot.
• No suspicious persistence, hidden execution, obfuscation, credential handling, or outbound-network behavior was introduced in the changed files. The snapshot excludes routing and credential-pointer fields, and its drift test confirms each shipped entry matches the normalizer output.
• The PR description should be updated before merge: it still claims self-registration and an env-var-ownership verification section, and says the snapshot contains five models, while the current implementation deliberately ships no provider facts and the snapshot has four models.

Verification Transcript

Command	Result
git diff --check origin/main...HEAD	PASS
node --check scripts/normalize-catalog.mjs	PASS
node --test tests/normalize-catalog.test.mjs	PASS: 10/10
make test-kits	PASS: all suites, including 57 USAi-provider tests
CLI normalization with an explicit fixture and /tmp/opencode output	PASS
malformed-entry adversarial probes	FAIL as described above
python3 scripts/validate_repo.py --root .	BLOCKED: local environment lacks jsonschema and YAML Python modules before repo-specific validators can run

Decision: request changes for the normalizer validation gap.

AI-assisted review by OpenCode; human review and merge remain required.

[wz-gsa]

Thanks for b3c18f8 — the polarity change is the right call. The old typeof x === "number" checks silently dropped bad values, so contextWindow: -5 passed the check and emitted a negative context window into the neutral catalog; now it throws with the index and the offending value. 59/59 tests pass, CI 7/7 green.

I probed 14 cases your test list doesn't cover. Nine are correctly rejected without being explicitly tested (contextWindow: true, NaN, -Infinity; cost.input: NaN/true/null) — Number.isInteger/Number.isFinite handle those. Of the five accepted, four are defensible:

• __proto__ as a cost key → dropped, because the explicit ["input","output","cacheRead","cacheWrite"] allowlist never reads it. Worth noting that allowlist is doing real work here.
• cost: {} → cost omitted entirely, via the Object.keys(cost).length > 0 guard. Correct.
• contextWindow: 2**53 → accepted; Number.isInteger is true and the source schema sets no maximum.
• id: "a\nb" → accepted; schema says type: string with no pattern, so this is consistent with the contract.

Two notes, neither blocking.

1. Whitespace-only id is accepted. model.id.length === 0 catches "" but not " ":

normalizeCatalog({ models: [{ id: "   " }] })  ->  [{"id":"   "}]

That reaches the neutral catalog as a usable model identifier, and catalog.schema.json:88 only constrains type: string, so nothing downstream rejects it either. model.id.trim().length === 0 closes it.

2. Duplicate model ids are accepted.

normalizeCatalog({ models: [{ id: "dup" }, { id: "dup" }] })  ->  [{"id":"dup"},{"id":"dup"}]

The source schema has no uniqueItems, so this isn't a contract violation — raising it because it's the same shape as a finding on basiliskus's validator in #476 (two kits could declare one name: because a set() collapsed the duplicates). There the fix was worth making since the registry treats the name as an identity. Open question whether the catalog wants the same guard or whether a duplicate id is harmless to the consumer.

I'm the author of record here so I can't approve this; flagging both for you to take or leave.

[mogul]

Validation update

The catalog-entry validation repair is committed and pushed in b3c18f8 (fix(usai-provider): validate normalized catalog entries).

• Added fail-closed checks for model object shape, non-empty IDs, positive integer token limits, finite non-negative costs, and object-valued cost metadata.
• Added regressions for fractional, null, string, zero/negative, infinite, and malformed values.
• Confirmed the current checked-in USAi catalog normalizes successfully (20 models).
• Local verification: npm test in integrations/isolation/acq-kits/usai-provider passed, 59/59 tests.
• GitHub CI is green: Pattern Validation, acq-kits Tests, Python lint/test and compatibility, Markdown Lint, Link Check, and PR title checks.

An adversarial read-only re-review found the fractional-limit issue resolved and no remaining actionable findings. The change is ready for the repository-required human review and approval; no additional review cycle is proposed unless a reviewer identifies a concern.

AI assistance was used for implementation and review; the commit includes the required attribution.

[mogul]

(I exercised Maintainer's Privilege to merge without another review round, since this has been pretty thoroughly thrashed in previous cycles.)