Skip to content

feat(usai-catalog): select default models in the catalog pipeline - #495

Open
wz-gsa wants to merge 1 commit into
mainfrom
feat/365-catalog-default-models
Open

wz-gsa wants to merge 1 commit into
mainfrom
feat/365-catalog-default-models

Conversation

@wz-gsa

@wz-gsa wz-gsa commented Oct 13, 2026

Copy link
Copy Markdown
Contributor

Closes #365.

Problem

Default model selection for OpenCode (model, small_model, agent.compaction.model) lives only inside the kit-specific sync-usai-models.mjs writer, driven by a hardcoded ROLE_PRIORITY constant. The harness-neutral catalog carries no role data at all, so the neutral pipeline cannot produce a complete config and every future harness would have to re-derive the same decision.

This is the prerequisite the epic's Phase-B cutover (#363) is blocked on.

Approach

Move role selection into the catalog, keeping the catalog harness-neutral and the OpenCode vocabulary confined to the OpenCode emitter.

catalog.json gains a defaults block with functional tier names, not harness key names:

"defaults": {
  "primary": "claude-sonnet-5",
  "small": "claude_4_5_haiku",
  "compaction": "gpt-5.6-terra"
}
  • catalog.schema.json declares defaults with primary/small required, optional compaction and reasoning, and a strict ^[A-Za-z0-9_.:-]+$ charset.
  • build-catalog.mjs resolves each role from an ordered candidate list on both the live-feed and bootstrap paths, preferring an existing valid value when no candidate is present, and throwing when nothing resolves.
  • validateCatalog rejects unknown roles, missing required roles, provider-prefixed values, and any default that does not reference a real models[] id — so a dangling default cannot ship.
  • describeDrift reports defaults block changed, and KEY_ORDER keeps serialization deterministic.
  • emitters/opencode.mjs adds emitOpenCodeDefaults() (neutral roles to usai/-prefixed OpenCode values) and patchOpenCodeConfig() (models block plus defaults in one pass).

emitOpenCodeBlockFromCatalog is unchanged, so the existing byte-exact round-trip guarantee is untouched.

Security hardening

An independent security review of this change returned CHANGES REQUESTED. Every finding is fixed, with a regression test for each:

  • Blocker — runaway regex. The compaction pattern used [\s\S]*?, which crosses closing braces. On a template with a top-level "compaction": { "auto": true } and no agent.compaction.model, it would match from there to the next "model" anywhere in the file and replace everything between, destroying unrelated config. Now scoped with [^{}]*? inside "agent", and a test asserts neighbouring keys survive.
  • Should-fix — backslash breakout. The id charset permitted \, so an id ending in a backslash would escape the closing quote and leave the JSON string open. Charset is now ^[A-Za-z0-9_.:-]+$.
  • Should-fix — brace expansion. Unchecked {/} could reach OpenCode's {env:VAR} / {file:PATH} expansion. Now rejected.
  • Should-fix — unanchored match. "model" could match inside a comment. Both patterns are now line-anchored.
  • Nit — schema/emitter divergence. The schema pattern previously allowed characters the emitter refused; both now agree, and the schema constraint is enforced through validateStringFromSchema.

Replacements use callback form throughout, so $-bearing values cannot trigger $&/$1 expansion.

Verification

providers/usai tests (sandbox, network off):  63/63 pass
acq-kits/usai-provider tests:                 76/76 pass
Python suite (sandbox):                      442/442 pass
repository + strict kit validators:           pass
git diff --check:                             clean

New coverage includes byte-exact patchOpenCodeConfig round-trips against both shipped opencode.jsonc copies, defaults structural/referential validation, $-in-id safety, and the runaway-regex regression.

Scope

This PR only adds the neutral defaults concept and the OpenCode mapping. It does not cut sync-usai-models.mjs over to the neutral pipeline; that remains #363 and is now unblocked.

AI-assisted implementation (OpenCode). Requires independent human review; the author identity will not self-approve.

Add a harness-neutral defaults block to the catalog so role selection is derived once from live model data instead of hardcoded in a kit-specific writer.

Resolve primary, small, and compaction from priority candidates, fail closed when no candidate resolves, and reject dangling or unsafe default references during validation.

Add an OpenCode defaults emitter and a config patcher that reproduces both shipped configs byte-for-byte, with scoped replacements that cannot escape their target keys.

Closes #365

Co-authored-by: OpenCode Agent <william.zujkowski@gsa.gov>
@wz-gsa
wz-gsa requested a review from a team as a code owner October 13, 2026 21:58

@basiliskus basiliskus left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: request changes — de0dfbb

  1. integrations/providers/usai/scripts/build-catalog.mjs:940 — buildCatalog() never passes existing defaults to resolveCatalogDefaults(), so the promised stability fallback is unreachable. If the feed still contains the configured model but none of the priority candidates, regeneration throws instead of preserving the valid current default. Please load/pass the existing catalog defaults on both build paths and add regression coverage.
  2. integrations/providers/usai/emitters/opencode.mjs:314 — patchOpenCodeConfig() only finds agent.compaction when it is the first nested object under agent, because [^{}]*? admits no earlier nested sibling. A valid template with summary or title first silently keeps the stale compaction model. Please parse JSONC or use an object-aware replacement that handles sibling order, and fail if a requested role cannot be patched.
  3. integrations/providers/usai/catalog.schema.json:59 — the closed issue explicitly covers agent.summary and agent.title, but the neutral schema and OpenCode emitter omit both, leaving those defaults hardcoded outside the split pipeline. Please model and emit those roles, or keep the issue open and document a separately tracked scope decision.

Assisted-by: OpenCode [gpt-5.6-sol-latest-guardrails-defaultv2] skill:change-review

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.

USAi catalog (later): default-model selection in the split pipeline

2 participants