fix(senpi-task): apply maintained OpenAI-only registry recommendations at resolution time - #7251
Conversation
…s at resolution time The OpenAI-only recommendation catalog lived only in the OpenCode CLI installer (openai-only-model-catalog.ts via model-fallback.ts), so OMO Native never applied it: with an authenticated OpenAI-only live registry, artistry/writing/ quick dead-chained to model_unavailable and visual-engineering resolved its generic openai rung at medium instead of the maintained high. - delegate-core: add harness-neutral OPENAI_ONLY_CATEGORY/AGENT_RECOMMENDATIONS mirrored from the maintained catalog (architect deliberately excluded so its required-model gate stays authoritative) - senpi-task: add optional getUpstreamModelId to SenpiModelRegistryPort so an explicitly mapped provider alias is identified safely; wire-protocol compatibility alone never implies OpenAI identity - senpi-task: compile the overlay from the live registry in resolveCategory and resolveAgent - applied only with no explicit user entry, a safely OpenAI-only inventory, and the exact recommended model present; user entries always win, user config files are never rewritten Fixes code-yeongyu#6813
github-actions
Bot
added
senpi-task
… markdown local path failures)
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
code-yeongyu
left a comment
code-yeongyu
left a comment
There was a problem hiding this comment.
[sisyphus-bot] REQUEST CHANGES. This branch is CONFLICTING/DIRTY against current dev (62 commits behind and 5 commits ahead), so it must be rebased before these changes can be evaluated or merged; the touched resolver surfaces have moved and the claimed behavior needs re-validation on the rebased tree. More importantly, the new fail-closed boundary is not actually fail-closed, and the live registry is sampled inconsistently, allowing resolution to diverge from the authenticated availability snapshot. The OpenCode audit is weakened globally, and the supposed shared policy remains hand-duplicated, guaranteeing future drift.
Most serious findings:
- [P1] Reject the entire inventory when any entry is malformed; silently dropping malformed entries can classify a partial/poisoned registry as OpenAI-only and activate recommendations.
- [P1] Take one stable registry snapshot per resolution and use it for identity, availability, and lookup; the current category and agent paths can authorize a model that was not in the authenticated snapshot.
- [P1] Preserve the receiver when invoking getUpstreamModelId; extracting a class method can throw and make every alias fail closed (or hide a real registry bug).
- [P2] Do not exclude the entire .opencode tree from the markdown audit just to accommodate new .omo evidence.
- [P2] Put the catalog in one shared source of truth (or add a real cross-edition equality test); the current tests only prove the duplicate matches itself.
| if (!Array.isArray(available)) return undefined | ||
| const entries: ParsedInventoryEntry<TModel>[] = [] | ||
| for (const candidate of available) { | ||
| if (typeof candidate !== "object" || candidate === null || hasSecretLikeModelField(candidate)) continue |
There was a problem hiding this comment.
[sisyphus-bot] P1: This is not fail-closed for malformed inventories. Invalid/non-object/secret-bearing entries are silently skipped, so [openai valid model, null] (or a valid OpenAI model plus a malformed entry) leaves a non-empty parsed list and can activate the OpenAI-only overlay. The comment and PR description explicitly promise malformed registries fail closed; reject the whole inventory (or carry an invalid flag) when any entry cannot be parsed, and add that mixed-malformed regression test.
| registry: SenpiModelRegistryPort<TModel>, | ||
| model: TModel, | ||
| ): string | undefined { | ||
| const getUpstreamModelId = registry.getUpstreamModelId |
There was a problem hiding this comment.
[sisyphus-bot] P1: Extracting the optional method loses its receiver: const getUpstreamModelId = registry.getUpstreamModelId followed by a bare call invokes a normal class/prototype method with this === undefined. The concrete senpi registry is specifically the consumer this seam is meant to support, and a method implementation that reads registry state will throw or misbehave, causing all aliases to fail closed. Call registry.getUpstreamModelId(model) directly (inside the try) or bind it explicitly, and exercise the real registry method in a test.
| // entry for this category, the live inventory is safely identified as OpenAI-only, and the exact | ||
| // recommended model exists. Any explicit categories.<name> entry above already won. | ||
| const overlay = userConfig === undefined | ||
| ? compileOpenAiOnlyOverlay(OPENAI_ONLY_CATEGORY_RECOMMENDATIONS, categoryName, senpiModelRegistry) |
There was a problem hiding this comment.
[sisyphus-bot] P1: resolveCategory already parsed one availability result above, then this helper calls getAvailable() again. A live registry can change between those calls; the overlay may be classified from snapshot B and select a model absent from snapshot A, while resolveModelForDelegateTask accepts the injected userModel and find() can still return it from the broader catalog. That bypasses the authenticated availability gate and makes resolution nondeterministic. Pass the already parsed/snapshotted entries into the compiler and ensure the selected target is in that same snapshot; add a changing-getAvailable regression test.
| // OpenAI-only, and the exact recommended model exists. An explicit definition.model/models entry | ||
| // is a user decision and always wins through the directModels loop above. | ||
| if (availableModels !== undefined && definition.model === undefined && (definition.models?.length ?? 0) === 0) { | ||
| const overlay = compileOpenAiOnlyOverlay(OPENAI_ONLY_AGENT_RECOMMENDATIONS, name, registry) |
There was a problem hiding this comment.
[sisyphus-bot] P1: The agent path has the same TOCTOU/auth bug as categories: availableModels is captured from one getAvailable() call, but compileOpenAiOnlyOverlay calls getAvailable() again and findExactAgentModel() is not checked against the original available set. If the second snapshot exposes luna-fast while the first does not, an otherwise unavailable model can be selected from the full catalog. Compile and resolve against one availability snapshot, and pin this with a registry whose getAvailable changes between calls.
| .trim() | ||
| .split("\n") | ||
| .filter(Boolean) | ||
| .filter((filePath) => !filePath.startsWith(".omo/") && !filePath.startsWith(".opencode/")) |
There was a problem hiding this comment.
[sisyphus-bot] P2: This broadens the workaround from the newly added .omo/ evidence to the entire .opencode/ tree. Any malformed local or absolute markdown link under .opencode/ now disappears from this repository-wide audit without a replacement check, weakening an existing security/quality gate unrelated to this PR. Exclude only the exact generated evidence paths (or make the audit understand generated artifacts) and add a test proving unrelated .opencode markdown is still scanned.
| // Maintained OpenAI-only model recommendations, mirrored from | ||
| // packages/omo-opencode/src/cli/openai-only-model-catalog.ts (the OpenCode installer applies that | ||
| // catalog at config-generation time; OMO Native compiles these into runtime category/agent | ||
| // resolution against the authenticated live model registry). Hand-mirrored on purpose: senpi-task |
There was a problem hiding this comment.
[sisyphus-bot] P2: Hand-mirrored on purpose contradicts the stated goal of a single maintained policy: the OpenCode catalog remains a separate literal, so a future catalog update can silently leave Native stale. The added catalog tests only assert these literals and cannot detect drift. Move the data to a shared module consumed by both editions, or add a cross-package test that imports the actual OpenCode catalog and compares the machine-consumed values.
2 participants
What
OMO Native now compiles the maintained OpenAI-only model recommendations from the authenticated live senpi model registry at task-resolution time:
artistry->gpt-5.6-sol(xhigh),writing->gpt-5.6-sol(medium),visual-engineering->gpt-5.6-sol(high),quick->gpt-5.6-luna-fast, curatedexplore/librarian->gpt-5.6-luna-fast(low).packages/delegate-core/src/openai-only-recommendations.tsmirrors the maintained catalog frompackages/omo-opencode/src/cli/openai-only-model-catalog.ts(single maintained policy, consumed by both editions;architectdeliberately absent so its required-model gate stays authoritative).packages/senpi-task/src/category/openai-only-overlay.tsapplies a recommendation only when (1) the target has no explicit user entry, (2) the inventory is safely identified as OpenAI-only, and (3) the exact recommended model exists in the registry.SenpiModelRegistryPortgains optionalgetUpstreamModelId(model)so an explicitly mapped provider alias (senpiupstreamModelId) is identified safely; an OpenAI-compatible wire protocol alone never implies OpenAI identity, and unparseable/empty registries fail closed.resolveCategoryinjects the compiled recommendation ahead of static-chain evaluation (bypassing the dead-chain short-circuit);resolveAgentdoes the same for agents whose definition carries no explicit model choice. Explicit usercategories.<name>/ agent model entries always win; user config files are never rewritten.Why
The OpenAI-only catalog was applied only by the OpenCode CLI installer at config-generation time (
model-fallback.ts), so OMO Native never saw it. With a working OpenAI-only endpoint registered as the canonical senpiopenaiprovider:artistry,writing, andquickdead-chained tomodel_unavailable(their generic chains have no direct openai rung), andvisual-engineeringresolved its generic rung atmediuminstead of the maintainedhigh. Connecting a working endpoint was not enough to receive the maintained automatic category tuning.Verified
Failing-first regression tests (co-located, given/when/then):
.omo/evidence/20260824-6813-registry-recs/red-state.log.bun test packages/delegate-core packages/senpi-task: 1773 pass / 0 fail (green-senpi-task-delegate-core.log). Guard tests pin user precedence, disabled-stays-disabled, architect gate preservation, mixed-inventory static-chain behavior, alias fail-closed withoutgetUpstreamModelId, unrelated compatible endpoints rejected, malformed registries fail closed, and configured agent tuning outranks the maintained variant.bun test packages/omo-senpi: 2215 pass / 11 fail - the 11 are generated-artifact-dependent suites (skills-sync, installer refresh, ulw-loop probe) that reproduce identically on base with my changes reverted (preexisting-failures-on-base.log); their build is blocked by the pre-existingshared-skills/upstreams/*submodule fetch failure that also failsbun installprepare (documented, harmless to deps/tests).tsgo --noEmitgreen for delegate-core, senpi-task, omo-senpi.Risk
Low. The overlay activates only on a safely identified OpenAI-only inventory with the exact recommended model present; every existing resolution path (user entries, gates, chains, mixed inventories) is pinned by tests. Listing metadata (
availableCategories) intentionally unchanged since spawn-timeresolveCategoryremains the sole authoritative gate. Live-harness QA drivers were not run here because the packed plugin build is blocked by the same pre-existing submodule issue; noted in the evidenceOMITTEDsection.Fixes #6813
Summary by cubic
Applies the maintained OpenAI-only model recommendations at runtime during category and agent resolution, and refreshes the committed plugin extension bundles from the merged Senpi compatibility base. Previously OMO Native applied the catalog only at install time, causing OpenAI-only inventories to dead-chain (artistry/writing/quick) or pick the wrong variant (visual-engineering=sol/medium); now the resolver injects the recommended model when the inventory is safely OpenAI-only and the exact model exists, while user config still wins. Addresses #6813.
Review notes
packages/delegate-coreaddsopenai-only-recommendations.tsexporting the maintained catalog andOPENAI_ONLY_RECOMMENDED_MODEL_IDS;architectis intentionally excluded and re-exported from@oh-my-opencode/delegate-core.packages/senpi-taskaddscategory/openai-only-overlay.tsand integrates it incategory/resolver.tsandagents/resolve-agent.ts; the overlay applies only without an explicit user entry and sets the catalog's variant when present.SenpiModelRegistryPortgains optionalgetUpstreamModelId(model)to recognize explicit provider aliases; OpenAI compatibility alone does not imply identity, and malformed/empty registries or mixed providers fail closed.packages/omo-senpi/plugin/extensions/*.jsbundles are rebuilt from the merged compatibility base; no behavior change..omo/and.opencode/paths so the new evidence files don't fail local-path checks.getUpstreamModelIdin custom registries to enable alias recognition.Written for commit 0b6baeb. Summary will update on new commits.