Make Engine Settings Consistent

[jaywedgeworth22]

Fixes #1111.

Summary

Engine settings and the capability matrix now present one shared, neutral set of engine facts instead of per-engine marketing copy and special-case callouts. The engine matrix also drops the ASCII.dev Box row, uses provider brand marks, and can be scoped to engines that have live instances, while Settings → Engines keeps Box out of its list and hides the built-in OpenAI-compatible row until it is actually configured.

Changes

Shared engine facts (EngineCallout)

• Replaced the collapsible "More Info" disclosure (headline, prose, expand/collapse state, aria-expanded) with an always-visible, neutral facts block showing Access, Billing, and Models.
• Added a models prop so callers can pass the live instance model ids; when omitted, the registry's default models are used.
• Model lists are capped at six ids with a +N more suffix, and fall back to a "Discovered from the configured endpoint" message when no ids are available.
• Removed the instanceId and defaultOpen props and the disclosure/aria-controls wiring.

Settings → Engines (EnginesSettings)

• Every engine row now renders the shared EngineCallout with the instance's configured model options, replacing the hardcoded driver-kind list, the Box-specific notice, and the custom-engine callout.
• The built-in openai-compat row is hidden unless openaiCompat.configured or openaiCompat.url is set; custom instances are always shown.
• Rows gained top/bottom separator styling.
• User-facing wording changed from "Delete"/"Delete Custom Engine?"/"Delete Engine" to "Remove"/"Remove Added Connection?"/"Remove Connection", with the dialog body clarifying that built-in engines cannot be removed.

Engine availability (engine-status)

• listedInEnginesSettings now always excludes boxAgent, regardless of configuration, so ASCII.dev Box stays in Computer settings rather than the Engines list. The corresponding test was updated accordingly.

Capability matrix (EngineCapabilitiesMatrix)

• The ASCII.dev Box engine is no longer rendered as a matrix row.
• Engine rows and the active-cell detail strip now show a ProviderMark brand icon plus the display name, replacing the colored uppercase text badge.
• Added an engineMarkKind mapping so driver-kind ids (e.g. deepseek-harness → dshAgent, opencode → opencodeGo) resolve to the correct provider mark.
• When instances are supplied, the matrix filters its rows to engines that have a matching live instance.

Tests

• EngineCallout.test.tsx was rewritten for the new neutral-facts behavior, model-list handling, truncation, and driver-kind resolution.
• EngineCapabilitiesMatrix.test.tsx now iterates a MATRIX_ENGINE_IDS set that excludes box, asserts Box is absent, and checks that ProviderMark is used instead of capabilityBadgeColor.

---

Summary

Makes the engine settings display consistent by always showing the full set of supported engines and adding proper icon/mark mappings for additional engine types.

Changes

Engine Capabilities Matrix (src/components/EngineCapabilitiesMatrix.tsx)

• Added mark kind mappings for three engine IDs: qwen → qwenAgent, hermes → hermesAgent, and pi → piAgent. These engines now render with their dedicated marks instead of falling back to the generic entry.id value.

• Removed instance-based filtering of the engine list. Previously, when props.instances was provided, the matrix only displayed engines that had a corresponding listed instance, and the now-unused listedIds lookup was computed for that purpose. With this change, the matrix always renders the full set of engines from ENGINE_DISPLAY_ORDER/ENGINE_CAPABILITIES (excluding box), so the settings view no longer varies based on which engines the user has installed or configured.

Functional Impact

Engine settings are now consistent for all users: every supported engine appears in the capabilities matrix regardless of local instances, and the newly mapped engines (Qwen, Hermes, Pi) display their correct identifying marks rather than a generic fallback.

---

Summary

Makes the visibility rule for the built-in OpenAI-compatible engine consistent by extracting it into a single shared helper, and updates the visual test harness/spec so that it exercises that visibility behavior (hidden until configured, shown once an endpoint URL or configuration flag exists) rather than the previous More Info callout layout shapes.

Changes

• src/components/EnginesSettings.tsx

  • Adds an exported helper showBuiltInOpenAiCompatible(configured, url) that returns Boolean(configured || url).
  • Replaces the inline filter condition in EnginesSettings with a call to that helper, so the built-in OpenAI-compatible instance is only listed when it is configured or has a URL. The helper is now the single source of truth for this rule.

• src/components/EngineCalloutVisualFixture.tsx

  • Rewrites the fixture from the two EngineCalloutBody layout shapes (Clutch with provider marks, MiniMax with none) to two Engine Settings states:
    • openai-hidden-state: no OpenAI-compatible endpoint configured; only Codex is rendered.
    • openai-configured-state: an endpoint URL is configured; an OpenAI-Compatible row with two models is rendered.
  • Uses the new showBuiltInOpenAiCompatible helper to drive whether the OpenAI-Compatible row appears in each section, plus EngineCallout and ProviderMark for rendering the rows.

• tests/e2e/engine-callout.visual.spec.ts

  • Replaces the two callout-shape tests with two visibility tests: OpenAI-compatible stays hidden until configured, and the configured state shows neutral endpoint facts (no "More Info" text).
  • Tightens the screenshot options to maxDiffPixels: 0, maxDiffPixelRatio: 0, threshold: 0 (previously maxDiffPixelRatio: 0.02, threshold: 0.2).
  • Enlarges the viewport to 720x620 and renames the snapshot files to engine-settings-openai-hidden.png and engine-settings-openai-configured.png.

Review findings (from automated review)

• high — tests/e2e/engine-callout.visual.spec.ts:46: the toHaveScreenshot call uses stableShot without a mask option, leaving the dynamic model-list region of the configured OpenAI-compatible block unmasked.
• medium — showBuiltInOpenAiCompatible is exported without the Vitest coverage required by rule 2 in the same diff.

---

Summary

Aligns how engines are presented across settings surfaces, so the Box (ASCII.dev) engine and locally injected models are only surfaced where they belong.

Changes

• Engine Settings no longer lists the Box engine. The ASCII.dev Box agent is now excluded from Engine Settings, since it is configured under Computer settings instead. The corresponding test suite was updated: the previous assertions that validated Box's engine callout copy inside Engine Settings were replaced with a single test asserting that the Box driver kind (boxAgent) is not referenced in the Engine Settings source.

• Locally injected models are hidden from the "Why this engine" callout. In ModelPicker.tsx, the model list passed to the EngineCallout is now filtered to exclude injected local models before mapping to model IDs. This keeps the callout's engine/model listing consistent with the models users can actually select.

Impact

Users will no longer see the Box engine presented as a selectable engine within Engine Settings, and the engine callout will no longer reference injected local models. This reduces confusion between where the Box agent is configured (Computer settings) and where standard engines are configured (Engine Settings), and prevents internal/injected models from leaking into user-facing engine explanations.

---

Summary

Makes engine/model settings consistent by allowing instance configuration patches to update or clear the configured model list through the same patch flow.

Changes

• Extended patchInstanceConfig to accept an optional models field containing model definitions with id and label.
• When a non-empty models array is provided, it replaces nextConfig.userModels.
• When an empty models array is provided, userModels is removed from the instance configuration.

Impact

Callers can now update or clear an instance’s model list alongside other engine settings such as CLI, full-auto, enabled, key, and external credential options. Empty model lists correctly clear the stored model configuration instead of leaving stale values.

---

Make Engine Settings Consistent

Summary

Adds the ability to register custom model entries for an engine instance from Engine Settings, and moves the built-in OpenAI-compatible visibility helper into the shared engine-status module so all surfaces answer consistently.

Changes

Custom user models per instance

• New server/instance-models.ts defines a shared validation schema for user-supplied model entries: each entry has an id and label, both trimmed, non-empty, at most 200 characters, and free of newlines; the list is capped at 100 entries and duplicate IDs are rejected.
• The instance PATCH endpoint now accepts an optional models array, validated against that schema, and returns a 400 with the message "models must contain at most 100 unique IDs with valid labels" when validation fails.
• The provider registry reads the stored userModels from an instance's config and merges them into the live model options list, tagging each with custom: true and overwriting any existing option with the same ID.

Engine Settings UI

• Added an "Add Model…" control to each engine row, disabled when the engine is not enabled, busy, or being deleted.
• Added an AddModelForm that collects a model slug and display name, blocks duplicates against the engine's current model list, submits the merged list of existing custom models plus the new entry, and surfaces validation or save errors inline.
• The patch callback and patch types in EnginesSettings and EngineRow now carry the optional models payload.

Engine status helper

• Moved showBuiltInOpenAiCompatible from EnginesSettings.tsx into src/lib/engine-status.ts (behavior unchanged: visible when either the configured flag or a URL is present), updated EngineCalloutVisualFixture to import it from the shared module, and added unit tests covering the unconfigured, empty-URL, configured, and URL-only cases.

Tests

• Added server/instance-models.test.ts covering trimming, stripping of unknown fields, accepting an explicitly cleared catalog, and rejection of malformed, empty, multiline, over-length, duplicate, and over-100-entry inputs.

Review Findings

The automated review reported 2 findings on this change:

• [high] server/instance-models.ts:6 — The instance-models object schema uses .strip() and should be .strict() to reject unknown keys at the HTTP boundary.
• [low] The rendered copy in the new Add Model form uses two ASCII spaces after a sentence-ending period where a real U+00A0 plus space is expected.

[kody-ai]

❌ Error processing validation

An error occurred while processing the system response. Please try again.

Details

• Analyzer execution failed: Timeout after 120000ms in business-rules-analyzer-attempt-1

---

💡 This validation runs automatically only on the first review of a pull request. To run it again, comment @kody -v business-logic.

[kody-ai]

Missing Vitest coverage for the newly exported showBuiltInOpenAiCompatible utility in src/components/EnginesSettings.tsx (lines 842-844) violates rule 2, which requires a co-located test for any new export in the same change. Add a describe('showBuiltInOpenAiCompatible', ...) block asserting the false (no flag, no url), true (configured flag), and url-only branches.

Kody rule violation: Every exported schema or utility ships with tests and must typecheck

[jaywedgeworth22]

Designer UI pass: approve with nits. The neutral Access / Billing / Models block is a clear win over the per-engine sales copy, and dropping Box from Engines and the matrix matches where it belongs (Computer settings). Small polish before merge:

1. Two spaces after colons. The facts labels render Access: CLI, Billing: …, Models: … with one space (</span> {value}). Use {" "} after each label.
2. One noun for custom rows. This page calls them engines, but delete now says "Remove this added connection", "Remove Added Connection?" and "Remove Connection". Suggest Remove Engine? / Remove Engine, and keep the body as "Remove “X”? This cannot be undone." The new "Built-in engines cannot be removed." line answers a question the user didn't ask in a confirm for a custom engine.
3. Model ID, not slug. The helper says "model identifier", the matrix says model ids, the placeholder says "Model slug". Use Model ID for the placeholder and aria-label, and "That model ID is already on this engine." for the duplicate warning.
4. Monospace fallback. "Discovered from the configured endpoint" sits inside the font-mono span, so a sentence reads like an id. Render the fallback in the sans face.
5. Disabled with no reason. Add Model… greys out when the engine is off; add a title such as "Turn on this engine to add models."
6. Enter key. Enter submits from Display Name but not from the Model ID field. Wire both so keyboard entry works either way.

None of these block. Approve once 1–3 land.

[kody-ai]

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

Kody Guide: Usage and Configuration

Interacting with Kody

• Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.

• Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.

• Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Bug	✅
Performance	✅
Security	✅
Business Logic	✅

Access your configuration settings here.

​

[jaywedgeworth22]

Landing repair at fbaa8a55b replaces the new manual model-edit checks and persisted-model assertions with one Zod contract, and moves the endpoint-visibility helper to its tested module.  Malformed, duplicate, oversized, multiline, and untrimmed model inputs have focused coverage.

Validation: 28 tests across four files passed; final pnpm typecheck and git diff --check passed; the lint baseline gate passed at 4895 warnings versus 4908 without changing the baseline.  Full exact-head hosted CI is running; local full-suite work was avoided because the shared Mac is severely overloaded.

The unmasked screenshot fixture is deterministic: its model IDs are literal fixture values, not live endpoint discovery.  The review thread has a concrete explanation and is resolved.

[kody-ai]

Double ASCII space on line 309 of src/components/EnginesSettings.tsx ('engine. The display name...') collapses during SwiftUI/HTML rendering, violating rule 49. Replace the double space with 'engine.\u{00A0} The display name...' (U+00A0 + ASCII space) to preserve the gap.

Kody rule violation: Sentence gap by surface: two ASCII spaces in files, U+00A0 plus a space in rendered copy and GitHub text

[kody-ai]

The z.object({...}) schema on lines 6–9 uses .strip(), silently discarding unknown keys and violating Rule 20's requirement for .strict(). This masks client/server contract drift when HTTP callers send extra fields, so replace .strip() with .strict() to reject unknown keys at the instance HTTP boundary.

Kody rule violation: Validate all boundary data with Zod schemas instead of type assertions

Prompt for LLM

File server/instance-models.ts:

Line 6 to 9:

The `z.object({...})` schema on lines 6–9 uses `.strip()`, silently discarding unknown keys and violating Rule 20's requirement for `.strict()`. This masks client/server contract drift when HTTP callers send extra fields, so replace `.strip()` with `.strict()` to reject unknown keys at the instance HTTP boundary.

_{Talk to Kody by mentioning @kody}

_{Was this suggestion helpful? React with 👍 or 👎 to help Kody learn from this interaction.}

​

​