Repository navigation
Conversation
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>
basiliskus
reviewed
Oct 14, 2026
basiliskus
left a comment
Contributor
There was a problem hiding this comment.
Verdict: request changes — de0dfbb
integrations/providers/usai/scripts/build-catalog.mjs:940—buildCatalog()never passes existing defaults toresolveCatalogDefaults(), 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.integrations/providers/usai/emitters/opencode.mjs:314—patchOpenCodeConfig()only findsagent.compactionwhen it is the first nested object underagent, because[^{}]*?admits no earlier nested sibling. A valid template withsummaryortitlefirst 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.integrations/providers/usai/catalog.schema.json:59— the closed issue explicitly coversagent.summaryandagent.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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #365.
Problem
Default model selection for OpenCode (
model,small_model,agent.compaction.model) lives only inside the kit-specificsync-usai-models.mjswriter, driven by a hardcodedROLE_PRIORITYconstant. 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.jsongains adefaultsblock with functional tier names, not harness key names:catalog.schema.jsondeclaresdefaultswithprimary/smallrequired, optionalcompactionandreasoning, and a strict^[A-Za-z0-9_.:-]+$charset.build-catalog.mjsresolves 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.validateCatalogrejects unknown roles, missing required roles, provider-prefixed values, and any default that does not reference a realmodels[]id — so a dangling default cannot ship.describeDriftreportsdefaults block changed, andKEY_ORDERkeeps serialization deterministic.emitters/opencode.mjsaddsemitOpenCodeDefaults()(neutral roles tousai/-prefixed OpenCode values) andpatchOpenCodeConfig()(models block plus defaults in one pass).emitOpenCodeBlockFromCatalogis 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:
[\s\S]*?, which crosses closing braces. On a template with a top-level"compaction": { "auto": true }and noagent.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.\, 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_.:-]+$.{/}could reach OpenCode's{env:VAR}/{file:PATH}expansion. Now rejected."model"could match inside a comment. Both patterns are now line-anchored.validateStringFromSchema.Replacements use callback form throughout, so
$-bearing values cannot trigger$&/$1expansion.Verification
New coverage includes byte-exact
patchOpenCodeConfiground-trips against both shippedopencode.jsonccopies, 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.mjsover 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.