From daee45bee91c01ed4a6d323d99c79b5822f3afad Mon Sep 17 00:00:00 2001 From: Tang Bohao Date: Sun, 27 Sep 2026 00:32:30 +0800 Subject: [PATCH 1/9] feat: localized plugin meta and icon Add the locale/en.json + locale/zh.json meta pair and the plugin icon, and declare both through package.json (exports ./locale/*.json, top-level icon, files locale + icon.svg). The host plugin manager resolves the per-UI-language title/description for the plugin detail page and the component row from these files; the icon ships the official-gradient plugin mark. --- icon.svg | 18 ++++++++++++++++++ locale/en.json | 6 ++++++ locale/zh.json | 6 ++++++ package.json | 4 ++++ 4 files changed, 34 insertions(+) create mode 100644 icon.svg create mode 100644 locale/en.json create mode 100644 locale/zh.json diff --git a/icon.svg b/icon.svg new file mode 100644 index 0000000..7298689 --- /dev/null +++ b/icon.svg @@ -0,0 +1,18 @@ + + + + + + + + + + + + + + + + + + diff --git a/locale/en.json b/locale/en.json new file mode 100644 index 0000000..01c6fe8 --- /dev/null +++ b/locale/en.json @@ -0,0 +1,6 @@ +{ + "meta": { + "title": "Advisor", + "description": "Configure the advisor reviewer model that observes the primary transcript and injects severity-ranked advice." + } +} diff --git a/locale/zh.json b/locale/zh.json new file mode 100644 index 0000000..cd887e6 --- /dev/null +++ b/locale/zh.json @@ -0,0 +1,6 @@ +{ + "meta": { + "title": "顾问", + "description": "配置顾问审阅模型：它观察主会话记录，并注入按严重程度排序的建议。" + } +} diff --git a/package.json b/package.json index 3add29b..0897eee 100644 --- a/package.json +++ b/package.json @@ -13,6 +13,7 @@ ], "main": "./lib/index.js", "types": "./lib/index.d.ts", + "icon": "./icon.svg", "exports": { ".": { "types": "./lib/index.d.ts", @@ -22,10 +23,13 @@ "types": "./lib/client.d.ts", "default": "./lib/client.js" }, + "./locale/*.json": "./locale/*.json", "./package.json": "./package.json" }, "files": [ "lib", + "locale", + "icon.svg", "cordis.patch.yml", "scripts" ], From 2fb2627137143121d97ef2dd43363cf918866310 Mon Sep 17 00:00:00 2001 From: Tang Bohao Date: Sun, 27 Sep 2026 01:02:08 +0800 Subject: [PATCH 2/9] feat!: remove the config-level enabled switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin-row enable/disable toggle is the master switch (2026-09-26 user ruling): a config-level `enabled` key is redundant with it and is removed across the whole chain, strictly and without a compatibility layer. Host side: - Config schema / CONFIG_KEYS / AdvisorConfig drop `enabled`; the S4 gate now keys purely on the pair — provider AND model non-empty -> enabled:true, otherwise disabled-with-reason (ResolvedAdvisorConfig.enabled stays as the post-gate flag). A stored profile still carrying `enabled:` is rejected as an unknown config key (actionable disabledReason on the row). - AdvisorSessionOverrides drops the configEnabled seed + setConfigEnabled: the effective session switch is `override ?? true` (a running row is on); only /advisor off suppresses a session. - The wiring's safeEffective applies the session override AFTER the gate instead of baking it into the raw config; the onChange re-apply no longer mirrors a fallback switch. - The TUI /settings section drops the enabled boolean field (four keys left) and its provider/model hints state the requirement unconditionally. Client side: - The store draft drops `enabled`; the client-form gate requires the pair unconditionally; setEnabled is gone; patchFor always-keys drop 'enabled'; AdvisorConfigView.enabled stays as a read-only wire fact. Tests updated to the new contract, including pins that a patch or entry carrying `enabled` is rejected as an unknown key. --- src/client/advisor-store.ts | 48 +++--- src/commands.ts | 32 ++-- src/config.ts | 61 ++++---- src/index.ts | 65 ++++---- src/settings.ts | 9 +- src/tui-settings.ts | 45 +++--- tests/advisor-runtime.test.ts | 7 +- tests/advisor-store.test.ts | 214 ++++++++++++++------------- tests/commands.test.ts | 45 +++--- tests/config.test.ts | 101 ++++++------- tests/gateway-session.test.ts | 13 +- tests/gateway.test.ts | 62 +++++--- tests/integration.test.ts | 105 +++++++------ tests/session-model-override.test.ts | 36 ++--- tests/settings-live.test.ts | 71 +++++---- tests/settings.test.ts | 20 +-- tests/tui-client.test.ts | 1 - tests/tui-settings.test.ts | 24 ++- 18 files changed, 476 insertions(+), 483 deletions(-) diff --git a/src/client/advisor-store.ts b/src/client/advisor-store.ts index 0224c2d..0cb0e56 100644 --- a/src/client/advisor-store.ts +++ b/src/client/advisor-store.ts @@ -82,7 +82,12 @@ export interface AdvisorStoreRemote { * simply missing from the JSON), so every optional key reads as undefined. */ export interface AdvisorConfigView { - /** Master switch; the resolved value (false while the gate blocks). */ + /** + * Read-only wire fact — the post-gate switch (false while the gate blocks). + * NOT a draft field: there is no config-level `enabled` key (2026-09-26; + * the plugin-row toggle is the switch), so the draft cannot write it and + * the card renders it nowhere. + */ enabled: boolean /** Provider route; absent when unset. */ provider?: string @@ -128,13 +133,11 @@ export type ModelsEmptyReason = /** No profile models; the catalog has no group for this provider. */ | 'unavailable' -/** The user-layer draft the form edits (all six config keys). */ +/** The user-layer draft the form edits (provider/model/systemPrompt/immuneTurns/maxDeltaMessages). */ export interface AdvisorDraft { - /** Master switch; default false. */ - enabled: boolean - /** Provider route; required (non-empty) when enabled. */ + /** Provider route; required (non-empty). */ provider?: string - /** Model id; required (non-empty) when enabled. */ + /** Model id; required (non-empty). */ model?: string /** Optional system prompt override; '' = built-in reviewer prompt. */ systemPrompt: string @@ -183,16 +186,16 @@ export interface AdvisorSettingsState { * resolved `config === undefined` (qc1 S-2 fix wave). The snapshot cannot * tell a "refresh of a degraded card" from a "first mount not yet settled" * while `status === 'loading'` (both read loading + advisorPresent=false), - * so the card derives its degraded disclosure from this latch during - * loading — keeping the AC-3 notice visible through a background refresh + * so the card derives its degraded notice from this latch during loading — + * keeping the config-channel notice visible through a background refresh * of a degraded card. Set ONLY in the ready update (the load-error path - * leaves it alone — the error state has its own always-open branch). + * leaves it alone — the error state has its own always-on branch). */ degraded: boolean /** - * Whether the draft holds edits a save would write — the "unsaved" pill - * and the save/discard disabled semantics (upstream CardShell.dirty). - * Derived as `patchFor(draft)` non-empty (KD-U2, plan + * Whether the draft holds edits a save would write — the save/discard + * disabled semantics (upstream CardShell.dirty). Derived as + * `patchFor(draft)` non-empty (KD-U2, plan * dsh-advisor-plugin-config-card-ux task 2), recomputed on load (seed * settled, real config resolved only), every draft mutation, discard and a * successful apply — always inside the same `store.update` callback that @@ -207,7 +210,7 @@ export interface AdvisorSettingsState { /** The schema-defaulted advisor config used when no config resolves. */ function defaultDraft(): AdvisorDraft { - return { enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 } + return { systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 } } /** A non-empty string field (whitespace-only reads as absent, mirroring the hard gate). */ @@ -227,7 +230,6 @@ function numberField(value: unknown, fallback: number): number { */ function draftOfConfig(config: AdvisorConfigView | undefined): AdvisorDraft { return { - enabled: config?.enabled === true, provider: stringField(config?.provider), model: stringField(config?.model), systemPrompt: typeof config?.systemPrompt === 'string' ? config.systemPrompt : '', @@ -236,9 +238,8 @@ function draftOfConfig(config: AdvisorConfigView | undefined): AdvisorDraft { } } -/** KD-S4 client-form gate: enabled requires a non-empty provider and model. */ +/** KD-S4 client-form gate: a non-empty provider and model are both required. */ function gateFailure(draft: AdvisorDraft): 'provider' | 'model' | undefined { - if (!draft.enabled) return undefined if (draft.provider === undefined) return 'provider' if (draft.model === undefined) return 'model' return undefined @@ -259,8 +260,8 @@ export class AdvisorSettingsStore { namespaces: {}, advisorPresent: false, // qc1 S-2: the degraded latch defaults false — a first mount / healthy - // card is never degraded, so the healthy card stays collapsed through its - // first load (the latch only flips on a settled degraded ready state). + // card is never degraded, so a healthy first load renders nothing (the + // latch only flips on a settled degraded ready state). degraded: false, // KD-U2: dirty derives from the patch diff against the seed // (recomputeDirty below — patchFor non-empty), recomputed on @@ -429,8 +430,8 @@ export class AdvisorSettingsStore { s.advisorPresent = config !== undefined // qc1 S-2: the degraded latch mirrors advisorPresent on every SETTLED // ready update — during a subsequent refresh (status 'loading') the - // card derives its degraded disclosure from this latch, so the AC-3 - // notice never collapses for the refresh window. + // card derives its degraded notice from this latch, so the config- + // channel notice never disappears for the refresh window. s.degraded = config === undefined s.modelsByProvider = {} s.modelsEmptyReason = {} @@ -538,11 +539,6 @@ export class AdvisorSettingsStore { } } - /** Set the enabled switch (gate fields become required while on). */ - setEnabled(enabled: boolean): void { - this.setField('enabled', enabled) - } - /** Set or clear the provider ('' clears); switches invalidate the chosen model. */ setProvider(provider: string): void { const trimmed = provider.trim() @@ -714,7 +710,7 @@ export class AdvisorSettingsStore { */ private patchFor(draft: AdvisorDraft): Record { const patch: Record = {} - const always = ['enabled', 'systemPrompt', 'immuneTurns', 'maxDeltaMessages'] as const + const always = ['systemPrompt', 'immuneTurns', 'maxDeltaMessages'] as const for (const key of always) { const next = draft[key] // A cleared number input (undefined) means "leave the stored value diff --git a/src/commands.ts b/src/commands.ts index 6a87901..67f0544 100644 --- a/src/commands.ts +++ b/src/commands.ts @@ -180,7 +180,11 @@ export type AdvisorModelSource = 'session' | 'global' * The per-session overrides consulted by the runtime gate and the effective * resolver (`src/index.ts`): * - * - enable: `override ?? config.enabled` (`/advisor on|off|toggle`); + * - enable: `override ?? true` (`/advisor on|off|toggle`) — there is no + * config-level switch anymore (2026-09-26: the config `enabled` key was + * removed; a running plugin row IS enabled, and the row toggle turns every + * session off at once), so the default is ON and only an explicit + * `/advisor off` override suppresses a session; * - model: the complete session pair ?? the composed global pair — the pair * is atomic, so the two levels are never merged (spec §5.3); * - generation: a per-session counter that fences async model work — a newer @@ -199,21 +203,9 @@ export class AdvisorSessionOverrides { private readonly models = new Map() private readonly generations = new Map() - constructor(private configEnabled: boolean) {} - - /** Effective switch for one session: `override ?? config.enabled`. */ + /** Effective switch for one session: `override ?? true` (a running row is on). */ effective(sessionId: string): boolean { - return this.enables.get(sessionId) ?? this.configEnabled - } - - /** - * Update the config-level fallback switch (live config — settings onChange, - * plan dsh-advisor-settings-n2 T1). Sessions with an explicit override keep - * it; every other session follows the new switch, so a Settings-page edit - * takes effect for new sessions without touching the override mechanism. - */ - setConfigEnabled(enabled: boolean): void { - this.configEnabled = enabled + return this.enables.get(sessionId) ?? true } /** Set the enable override for one session. */ @@ -295,7 +287,7 @@ export class AdvisorSessionOverrides { * the override state. */ export interface AdvisorSessionStatus { - /** Effective switch for this session (`override ?? config.enabled`). */ + /** Effective switch for this session (`override ?? true` — row on = on). */ readonly enabled: boolean /** * Present iff the session is effectively enabled but the S4 explicit gate @@ -600,7 +592,8 @@ export const MODEL_USAGE = [ * "Enabled" outcome text — mentions the S4 gate when it blocks model calls. * Callers pass the status AFTER the override flip, so the caveat appears when * the flip itself is what trips the gate (qc2 W-2 / qc3 I-2 — the pre-flip - * status cannot know the gate yet: the gate only fires when enabled). + * status cannot know the gate yet: the gate only fires on an incomplete + * provider/model pair). */ function enableText(status: AdvisorSessionStatus): string { if (status.disabledReason === undefined) return 'Advisor on for this session.' @@ -635,9 +628,8 @@ function createAdvisorCommandHandler(controller: AdvisorCommandController) { } controller.setEnabled(sessionId, true, invocation.agent.session.seq) // Reply from the POST-flip status: when the override flip trips the - // S4 gate (config-off + missing provider/model), the reply must say - // the advisor did not start and why, not a bare "Advisor on" (qc2 - // W-2 / qc3 I-2). + // S4 gate (missing provider/model), the reply must say the advisor + // did not start and why, not a bare "Advisor on" (qc2 W-2 / qc3 I-2). return { kind: 'success', text: enableText(controller.getStatus(sessionId)) } } case 'off': { diff --git a/src/config.ts b/src/config.ts index 4bab526..5a16127 100644 --- a/src/config.ts +++ b/src/config.ts @@ -2,21 +2,25 @@ * dsh-advisor plugin configuration contract (spec §5 / S4). * * The exported schemastery `Config` schema is what the cordis Loader uses to - * validate the plugin row config: it applies defaults (`enabled` false, - * `immuneTurns` 3, `maxDeltaMessages` 60, `systemPrompt` "") and enforces - * types/bounds (integers ≥ 0). All six live fields are declared `.volatile()`: + * validate the plugin row config: it applies defaults (`immuneTurns` 3, + * `maxDeltaMessages` 60, `systemPrompt` "") and enforces types/bounds + * (integers ≥ 0). All five live fields are declared `.volatile()`: * the Loader commits edits to them into the running fiber's references WITHOUT * a remount (dsh 0.1.7-rc.1 — the settings.yaml user layer is gone), so * `apply` receives each field as a `{ get() }` reference and every read must * unwrap it first — {@link unwrapAdvisorConfig}, which tolerates plain values * too (integration harnesses pass plain objects). * - * `resolveAdvisorConfig(raw)` additionally enforces the explicit model gate: - * when `enabled` is true but `provider` or `model` is missing or empty, it - * resolves to a disabled-with-reason config — the advisor never starts a model - * call (hard gate, not a warning). The volatile unwrap happens BEFORE the gate: - * an unwrapped reference object is truthy, so the enabled/provider/model reads - * would silently pass the gate on the reference objects themselves. + * `resolveAdvisorConfig(raw)` additionally enforces the explicit model gate. + * There is NO config-level `enabled` key (2026-09-26 user ruling: the + * plugin-row enable/disable toggle IS the switch, so a config key would be + * redundant) — the gate keys purely on the pair: `provider` AND `model` + * non-empty → enabled; otherwise it resolves to a disabled-with-reason config + * — the advisor never starts a model call (hard gate, not a warning). The + * volatile unwrap happens BEFORE the gate: an unwrapped reference object is + * truthy, so the provider/model reads would silently pass the gate on the + * reference objects themselves. `enabled` arriving in a raw config (a stored + * profile predating the removal) is rejected like any other unknown key. * * @module dsh-advisor/config */ @@ -25,11 +29,9 @@ import z from '@deepseek-ai/schemastery' /** Raw plugin row config after Loader defaults — spec §5.1. */ export interface AdvisorConfig { - /** Master switch; default false. */ - readonly enabled: boolean - /** Provider route; REQUIRED (non-empty) when enabled. */ + /** Provider route; REQUIRED (non-empty) for the advisor to run. */ readonly provider?: string - /** Model id; REQUIRED (non-empty) when enabled. */ + /** Model id; REQUIRED (non-empty) for the advisor to run. */ readonly model?: string /** Optional system prompt override; "" = built-in reviewer prompt (T4). */ readonly systemPrompt: string @@ -41,6 +43,11 @@ export interface AdvisorConfig { /** Config after the explicit model gate (spec §5.2) — consumed by T4/T6. */ export interface ResolvedAdvisorConfig { + /** + * Post-gate switch: true iff the pair is complete — NOT a config key (the + * row toggle is the switch; a session `/advisor off` override flips this + * to false downstream, `src/index.ts` `safeEffective`). + */ readonly enabled: boolean readonly provider?: string readonly model?: string @@ -58,7 +65,6 @@ export interface ResolvedAdvisorConfig { * — same pattern as `resolveSessionTitleLlmConfig` in the dsh repo. */ const CONFIG_KEYS: ReadonlySet = new Set([ - 'enabled', 'provider', 'model', 'systemPrompt', @@ -69,8 +75,10 @@ const CONFIG_KEYS: ReadonlySet = new Set([ /** * Loader schema (strict): defaults + type/bounds validation for the plugin * row config. The explicit gate is intentionally NOT here — `provider`/`model` - * stay optional so an enabled-without-pair config validates and then resolves - * to disabled-with-reason instead of failing to load. + * stay optional so a pairless config validates and then resolves to + * disabled-with-reason instead of failing to load. There is no `enabled` + * field: the plugin-row enable/disable toggle is the master switch, and a + * stored `enabled:` key is rejected as unknown by the strict resolver. * * Volatile (dsh 0.1.7-rc.1): every field is a LIVE field. The Loader commits * edits into the running fiber's references without remounting the plugin @@ -86,7 +94,6 @@ const CONFIG_KEYS: ReadonlySet = new Set([ * exported contracts plain-valued. */ export const Config = z.object({ - enabled: z.boolean().default(false).volatile(), provider: z.string().volatile(), model: z.string().volatile(), systemPrompt: z.string().default('').volatile(), @@ -111,7 +118,6 @@ function isNonEmptyString(value: string | undefined): value is string { * Loader). */ export interface VolatileAdvisorConfig { - readonly enabled: unknown readonly provider?: unknown readonly model?: unknown readonly systemPrompt: unknown @@ -147,10 +153,9 @@ function unwrapReference(value: unknown): T { * config `resolveAdvisorConfig` consumes. */ export function unwrapAdvisorConfig(raw: VolatileAdvisorConfig): AdvisorConfig { - const { enabled, provider, model, systemPrompt, immuneTurns, maxDeltaMessages, ...rest } = raw + const { provider, model, systemPrompt, immuneTurns, maxDeltaMessages, ...rest } = raw return { ...rest, - enabled: unwrapReference(enabled), provider: unwrapReference(provider) ?? undefined, model: unwrapReference(model) ?? undefined, systemPrompt: unwrapReference(systemPrompt), @@ -163,9 +168,12 @@ export function unwrapAdvisorConfig(raw: VolatileAdvisorConfig): AdvisorConfig { * Resolve the raw config into the runtime contract. * * - Rejects unknown keys (strict schema, spec §5.2) and non-object input. - * - Applies the explicit model gate (S4): `enabled: true` with `provider` or - * `model` missing/empty → disabled-with-reason, never throws, no model call. - * - `provider`/`model` are ignored while disabled. + * `enabled` is an unknown key since its 2026-09-26 removal — a stored + * profile still carrying it is rejected with an actionable message (the row + * toggle replaces it; no compatibility layer). + * - Applies the explicit model gate (S4): `provider` or `model` missing/empty + * → disabled-with-reason, never throws, no model call. Both present → + * `enabled: true` (the post-gate flag). * * The volatile unwrap happens FIRST (before any gate read): a reference * object is truthy regardless of the value behind it, so gating on the raw @@ -183,13 +191,12 @@ export function resolveAdvisorConfig(raw: unknown): ResolvedAdvisorConfig { // Config(raw) resolves each `.volatile()` field to its reference; unwrap // into the plain contract the gate (and every consumer) reads. const normalized = unwrapAdvisorConfig(Config(raw)) - if (!normalized.enabled) return normalized const missing: string[] = [] if (!isNonEmptyString(normalized.provider)) missing.push('provider') if (!isNonEmptyString(normalized.model)) missing.push('model') - if (missing.length === 0) return normalized + if (missing.length === 0) return { ...normalized, enabled: true } const disabledReason = missing.length === 2 - ? 'enabled but provider and model are missing — configure both to enable the advisor' - : `enabled but ${missing[0]} is missing or empty — configure provider and model` + ? 'provider and model are missing — configure both to enable the advisor' + : `${missing[0]} is missing or empty — configure provider and model` return { ...normalized, enabled: false, disabledReason } } diff --git a/src/index.ts b/src/index.ts index 73bdf0d..4abdf14 100644 --- a/src/index.ts +++ b/src/index.ts @@ -31,11 +31,14 @@ * (`ctx.get('tuiSettingsSections') !== undefined`), so the hint names the TUI * `/settings` Advisor section as a write path exactly while the seam is * mounted; the readback itself stays session-less and read-only. - * Settings (plan dsh-advisor-settings-n2): the plugin-row entry config's six + * Settings (plan dsh-advisor-settings-n2): the plugin-row entry config's five * live fields are schema-volatile (`src/config.ts`), read live through the * bridge source (`src/settings.ts`); committed volatile edits (Loader * `loader/volatile-update`) re-apply derived state (immuneTurns / - * maxDeltaMessages / per-session runtimes) without a restart. + * maxDeltaMessages / per-session runtimes) without a restart. There is no + * config-level `enabled` key (2026-09-26): the plugin-row enable/disable + * toggle IS the master switch, and the per-session `/advisor` override + * defaults to ON (`override ?? true`). * Config gateway (plan dsh-advisor-settings-gateway-n5): `apply` also * registers the host-side `AdvisorConfigGateway` (`src/gateway.ts`) — the * `/api/advisor/get` + `/api/advisor/set` endpoints (explicit @@ -169,7 +172,7 @@ export function apply(ctx: Context, config: AdvisorConfig) { // plugin row at load; the gate resolves to disabled-with-reason instead. // // T1-settings (plan dsh-advisor-settings-n2): the plugin-row entry config's - // six live fields are schema-volatile (0.1.7-rc.1). The runtime reads the + // five live fields are schema-volatile (0.1.7-rc.1). The runtime reads the // LIVE values through the bridge source (per-field volatile reference // unwrap); the Loader commits edits into those references without a remount // and announces them via `loader/volatile-update`. The hard gate is applied @@ -263,21 +266,23 @@ export function apply(ctx: Context, config: AdvisorConfig) { } // Spec §5.3 — the ONE effective resolver: (1) the global shape is validated // first (a malformed entry still throws → safeFallback: fail closed for - // every session); (2) enable = session enable override ?? global; (3) route - // = the session's COMPLETE modelOverride pair ?? the composed global pair — - // the pair is atomic, so the levels are never merged; (4) the explicit pair - // gate (§5.2) applies AFTER this resolution on the effective values: a - // complete session pair satisfies a pairless global default, while an - // invalid global schema can never be bypassed. + // every session); (2) enable = session enable override ?? true — a running + // row is enabled (no config-level switch since its 2026-09-26 removal), so + // only an explicit `/advisor off` override forces `enabled: false` on the + // POST-gate resolution; (3) route = the session's COMPLETE modelOverride + // pair ?? the composed global pair — the pair is atomic, so the levels are + // never merged; (4) the explicit pair gate (§5.2) applies on the effective + // values: a complete session pair satisfies a pairless global default, while + // an invalid global schema can never be bypassed. const safeEffective = (sessionId: string): ResolvedAdvisorConfig => { try { const source = sourceConfig() const pair = overrides.model(sessionId) - return resolveAdvisorConfig({ + const resolved = resolveAdvisorConfig({ ...source, - enabled: effectiveEnabled(sessionId), ...(pair === undefined ? {} : { provider: pair.provider, model: pair.model }), }) + return effectiveEnabled(sessionId) ? resolved : { ...resolved, enabled: false } } catch (error) { return safeFallback(error instanceof Error ? error.message : String(error)) } @@ -288,18 +293,16 @@ export function apply(ctx: Context, config: AdvisorConfig) { }) // T7: the per-session override mechanism — `/advisor on|off|toggle` write - // here and the runtime gate consults `override ?? config.enabled`, so the + // here and the runtime gate consults `override ?? true` (a running row is + // enabled; no config-level switch since its 2026-09-26 removal), so the // commands start/stop per-session runtimes WITHOUT touching the persisted // config (spec §4 mapping — omp `/advisor` semantics). Ephemeral: entries - // are cleared on `agent/disposed` / `session/disposed` below. Seeded with - // the LIVE config switch read through the bridge (the raw `config` fields - // are volatile references on a Loader composition — an unwrapped snapshot - // shares the source with `safeResolved`): a config-enabled-but-gate-blocked - // session (enabled without provider/model) then re-derives the - // disabled-with-reason through the resolver, so `/advisor status` shows the - // reason (spec §5.2; qc3 I-1) — the gate itself still blocks every runtime - // (the resolver is the SSOT for the gate). - const overrides = new AdvisorSessionOverrides(bridge.source().enabled) + // are cleared on `agent/disposed` / `session/disposed` below. A session + // enabled without a usable pair re-derives the disabled-with-reason through + // the resolver, so `/advisor status` shows the reason (spec §5.2; qc3 I-1) + // — the gate itself still blocks every runtime (the resolver is the SSOT + // for the gate). + const overrides = new AdvisorSessionOverrides() const effectiveEnabled = (sessionId: string): boolean => overrides.effective(sessionId) // Live-path alias: every consumer reads the effective config through the // safe wrapper (qc2 W-1 — a throwing resolver must not break the @@ -548,24 +551,15 @@ export function apply(ctx: Context, config: AdvisorConfig) { // (qc3 W-1 / qc1 W-2: an immuneTurns/maxDeltaMessages-only edit must not // abort in-flight advisor calls or drop backlogs). The S4 gate is // re-applied by the resolver on every read, so a config edit can never - // start a gated model call (SSOT unchanged); the config-level fallback - // switch follows the live source so new sessions pick up an enabled edit - // immediately. An entry config the resolver rejects (qc2 W-1 — unknown - // key) stops the advisor without wedging the re-apply path, and the - // last-good latches stay until the config is repaired. + // start a gated model call (SSOT unchanged). An entry config the resolver + // rejects (qc2 W-1 — unknown key) stops the advisor without wedging the + // re-apply path, and the last-good latches stay until the config is + // repaired. bridge.onChange(() => { let next: ResolvedAdvisorConfig try { next = resolveAdvisorConfig(sourceConfig()) } catch (error) { - // The raw source is still readable for the switch even when the - // resolver rejects the composed value; if even that fails, keep the - // current config-level switch (the gate below still blocks runtimes). - try { - overrides.setConfigEnabled(sourceConfig().enabled) - } catch { - // unreadable source — the effective switch stays as-is - } for (const sessionId of [...runtimes.keys()]) disposeRuntime(sessionId) ctx.logger('advisor').warn('settings change: invalid advisor config — advisor stopped', { disabledReason: error instanceof Error ? error.message : String(error), @@ -574,7 +568,6 @@ export function apply(ctx: Context, config: AdvisorConfig) { } delivery.setImmuneTurns(next.immuneTurns) observer.setMaxDeltaMessages(next.maxDeltaMessages) - overrides.setConfigEnabled(sourceConfig().enabled) // Candidates = live runtimes ∪ sessions holding an override. The second // set covers an `/advisor on`-enabled session whose runtime is ABSENT // because it was gate-blocked: when the defaults become usable (a global @@ -905,7 +898,7 @@ export function apply(ctx: Context, config: AdvisorConfig) { // T1 (plan dsh-advisor-tui-settings-n9): the dsh-tui settings-section seam — // the `tuiSettingsSections` "Advisor" section (editable `/settings` screen - // fields: enabled/provider/model/immuneTurns/maxDeltaMessages). Runs AFTER + // fields: provider/model/immuneTurns/maxDeltaMessages). Runs AFTER // the single-reviewer claim like `installTuiClient`, so the section // registers at most once per process (duplicate-ns registration is // contained inside the module). The inject is conditional: profiles without diff --git a/src/settings.ts b/src/settings.ts index ed335be..5bf0453 100644 --- a/src/settings.ts +++ b/src/settings.ts @@ -7,7 +7,7 @@ * service `SettingsForms` over profile entries). A plugin's editable config is * now its OWN entry config: the fields the schema declares `.volatile()` are * committed by the cordis Loader into the running fiber's references WITHOUT a - * remount (`src/config.ts` marks all six live fields volatile). + * remount (`src/config.ts` marks all five live fields volatile). * * This module is the read side of that pipeline. `apply` receives the entry * config with each volatile field as a `{ get() }` reference; the bridge @@ -27,7 +27,10 @@ * * The hard gate is untouched: the source returns the RAW config and every * consumer passes it through `resolveAdvisorConfig` — the SSOT for the - * enabled-without-pair disabled-with-reason resolution (no model call). + * pairless-config disabled-with-reason resolution (no model call). There is + * no config-level `enabled` key (2026-09-26): the plugin-row toggle is the + * master switch, so the entry's volatile fields are the four §5.1 keys plus + * the system prompt. * * The write side rides the same entry id: the gateway (`src/gateway.ts`) * writes through `settings.update(ADVISOR_SETTINGS_NAMESPACE, ...)` — the @@ -85,7 +88,7 @@ interface VolatileUpdateContext { * Wire the live source over the entry config's volatile references. * * `entry` is the config object `apply` received — on a Loader composition the - * six schema-declared fields are `{ get() }` references; plain-object entries + * five schema-declared fields are `{ get() }` references; plain-object entries * (integration harnesses) work identically. The bridge holds the ENTRY, not a * snapshot, so every `source()` call reads the references' CURRENT values and * the returned config is live from the first read. `onChange` fires on every diff --git a/src/tui-settings.ts b/src/tui-settings.ts index 8b18830..8b4b3e4 100644 --- a/src/tui-settings.ts +++ b/src/tui-settings.ts @@ -30,17 +30,18 @@ * cast goes through `unknown`) and could cause host-side misrender rather than * a compile error — re-verify against the dsh-TUI repo when bumping. * - * Field subset (grill-me locked): the section covers the five SAFE §5.1 keys - * (`enabled` / `provider` / `model` / `immuneTurns` / `maxDeltaMessages`). - * `systemPrompt` is intentionally NOT a field — the TUI `text` control is - * single-line, and editing a multi-line prompt there would truncate/replace - * it (data loss). It stays editable via the web card or the profile's - * `cordis.patch.yml`. The TUI seam has no cross-field validation - * (upstream behavior, recorded): a save may set `enabled: true` with empty - * `provider`/`model`, which the S4 explicit model gate (spec §5.2) resolves - * to disabled-with-reason at runtime; the settings-service schema - * re-validation on mutate is the value-level backstop (a non-schema value - * fails the whole save before persist). + * Field subset (2026-09-26): the section covers the four §5.1 keys + * (`provider` / `model` / `immuneTurns` / `maxDeltaMessages`). `enabled` is + * gone with the config-level switch it edited (the plugin-row toggle is the + * master switch), and `systemPrompt` is intentionally NOT a field — the TUI + * `text` control is single-line, and editing a multi-line prompt there would + * truncate/replace it (data loss). It stays editable via the web card or the + * profile's `cordis.patch.yml`. The TUI seam has no cross-field validation + * (upstream behavior, recorded): a save may leave `provider`/`model` empty, + * which the S4 explicit model gate (spec §5.2) resolves to + * disabled-with-reason at runtime; the settings-service schema re-validation + * on mutate is the value-level backstop (a non-schema value fails the whole + * save before persist). * * @module dsh-advisor/tui-settings */ @@ -129,9 +130,9 @@ export const TUI_SETTINGS_SECTIONS = 'tuiSettingsSections' export const ADVISOR_TUI_SETTINGS_NS = 'advisor' /** The declared "Advisor" section for the dsh-tui `/settings` screen: the - * five safe §5.1 keys (enabled/provider/model/immuneTurns/maxDeltaMessages) - * with zh/en labels + hints. Field paths are single-element §5.1 flat keys, - * so staged edits map 1:1 onto the namespace `mutate` paths. */ + * four §5.1 keys (provider/model/immuneTurns/maxDeltaMessages) with zh/en + * labels + hints. Field paths are single-element §5.1 flat keys, so staged + * edits map 1:1 onto the namespace `mutate` paths. */ export const ADVISOR_TUI_SETTINGS_SECTION: TuiSettingsSection = { ns: ADVISOR_SETTINGS_NAMESPACE, title: 'Advisor', @@ -140,21 +141,13 @@ export const ADVISOR_TUI_SETTINGS_SECTION: TuiSettingsSection = { en: 'Advisor settings', }, fields: [ - { - path: ['enabled'], - kind: 'boolean', - label: 'Enabled', - descriptions: { zh: '启用', en: 'Enabled' }, - hint: 'Master switch for the advisor.', - hintDescriptions: { zh: '顾问总开关。', en: 'Master switch for the advisor.' }, - }, { path: ['provider'], kind: 'text', label: 'Provider', descriptions: { zh: 'Provider', en: 'Provider' }, - hint: 'Provider route; required (non-empty) when enabled.', - hintDescriptions: { zh: 'Provider 路由；启用时必须非空。', en: 'Provider route; required (non-empty) when enabled.' }, + hint: 'Provider route; required (non-empty).', + hintDescriptions: { zh: 'Provider 路由；必须非空。', en: 'Provider route; required (non-empty).' }, placeholder: 'e.g. deepseek-official', }, { @@ -162,8 +155,8 @@ export const ADVISOR_TUI_SETTINGS_SECTION: TuiSettingsSection = { kind: 'text', label: 'Model', descriptions: { zh: 'Model', en: 'Model' }, - hint: 'Model id; required (non-empty) when enabled.', - hintDescriptions: { zh: '模型 ID；启用时必须非空。', en: 'Model id; required (non-empty) when enabled.' }, + hint: 'Model id; required (non-empty).', + hintDescriptions: { zh: '模型 ID；必须非空。', en: 'Model id; required (non-empty).' }, placeholder: 'e.g. deepseek-flash', }, { diff --git a/tests/advisor-runtime.test.ts b/tests/advisor-runtime.test.ts index f51e3b4..30e1eea 100644 --- a/tests/advisor-runtime.test.ts +++ b/tests/advisor-runtime.test.ts @@ -963,13 +963,16 @@ describe('AdvisorRuntime — composed with a real LlmRuntime + registered adapte expect('purpose' in adapter.requests[0]!).toBe(false) }) - it('never starts a model call when the config is disabled (explicit gate, S4)', async () => { + it('never starts a model call when the pair gate blocks (explicit gate, S4)', async () => { const ctx = new Context() await ctx.plugin(LlmRuntime) const adapter = new RecordingAdapter([...textReply('{"note":"should never be called"}')]) ctx.llm.registerAdapter(['test-provider'], adapter) - apply(ctx, { enabled: false } as AdvisorConfig) + // No config-level switch anymore (2026-09-26 — the row toggle is the + // switch): a pairless entry resolves to disabled-with-reason, and the + // gate drops every delta before a call could start. + apply(ctx, { systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) // A full stepped turn would otherwise render a delta and dispatch a call. const events = buildEvents(minimalTurn()) diff --git a/tests/advisor-store.test.ts b/tests/advisor-store.test.ts index 23f82c6..af0a565 100644 --- a/tests/advisor-store.test.ts +++ b/tests/advisor-store.test.ts @@ -12,8 +12,9 @@ * ② model options: profile-declared `models` win; else the * `session.modelCatalog` group for that provider; neither → empty options * + a reason. - * ③ Apply gate (KD-S4): enabled + missing provider/model → blocked with the - * gate failure; disabled → provider/model may be empty and the apply lands. + * ③ Apply gate (KD-S4): a missing provider/model blocks the apply with the + * gate failure — the gate is UNCONDITIONAL now (no config-level `enabled` + * switch since 2026-09-26; the pair is the on/off state). * ④ gateway patch semantics: the advisor config is read/written over * `rpc.call('/api', 'advisor/get'|'advisor/set')` — a minimal patch * (changed keys only) against the last-read config; a cleared @@ -242,7 +243,6 @@ describe('providers join (KD-S2 configured determination)', () => { const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() expect(draftOf(store)).toEqual({ - enabled: true, provider: 'deepseek-official', model: 'ds-a', systemPrompt: '', @@ -254,13 +254,13 @@ describe('providers join (KD-S2 configured determination)', () => { it('seeds the draft from the effective config regardless of layer origin (base+user already folded by the host)', async () => { // The gateway returns the RESOLVED config — the composition base / user // layer split is host-side and invisible here: the form shows the - // effective values so the toggle is not off while the advisor is running. + // effective values so the fields never lie about what runs. The wire + // `enabled` stays a read-only fact (the draft cannot write it). const { remote, rpc } = scriptedApi({ config: { enabled: true, provider: 'deepseek-official', model: 'ds-a', systemPrompt: 'entry', immuneTurns: 7, maxDeltaMessages: 20 }, }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - expect(draftOf(store).enabled).toBe(true) expect(draftOf(store).provider).toBe('deepseek-official') expect(draftOf(store).model).toBe('ds-a') expect(draftOf(store).systemPrompt).toBe('entry') @@ -278,7 +278,6 @@ describe('providers join (KD-S2 configured determination)', () => { await store.load() expect(draftOf(store).provider).toBeUndefined() expect(draftOf(store).model).toBeUndefined() - expect(draftOf(store).enabled).toBe(false) }) }) @@ -372,12 +371,11 @@ describe('model options (KD-S2 profile-first, catalog fallback)', () => { }) }) -describe('apply gate (KD-S4 required-when-enabled)', () => { - it('blocks Apply when enabled with no provider, naming the gate failure', async () => { +describe('apply gate (KD-S4 — the pair is required, unconditionally)', () => { + it('blocks Apply with no provider, naming the gate failure', async () => { const { remote, rpc, set } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) await store.apply() const { applyState } = store.store.getSnapshot() expect(applyState.kind).toBe('error') @@ -388,11 +386,10 @@ describe('apply gate (KD-S4 required-when-enabled)', () => { expect(set).not.toHaveBeenCalled() }) - it('blocks Apply when enabled with a provider but no model', async () => { + it('blocks Apply with a provider but no model', async () => { const { remote, rpc, set } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) store.setProvider('deepseek-official') await store.apply() const { applyState } = store.store.getSnapshot() @@ -403,10 +400,12 @@ describe('apply gate (KD-S4 required-when-enabled)', () => { expect(set).not.toHaveBeenCalled() }) - it('allows empty provider/model while disabled and lands the apply', async () => { + it('lands the apply once the pair is complete', async () => { const { remote, rpc, set } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() + store.setProvider('deepseek-official') + store.setModel('ds-a') store.setImmuneTurns(5) await store.apply() expect(set).toHaveBeenCalledTimes(1) @@ -429,44 +428,45 @@ describe('apply patch + seed (gateway channel semantics)', () => { }) }) - it('overrides a config-pinned provider/model with explicit empty values when cleared', async () => { - // The seed pins the values through the effective config; the explicit '' - // override is used because the gateway merge cannot express an unset (the - // host resolver treats '' as absent — the clear stays stable). - const { remote, rpc, call } = scriptedApi({ + it('blocks a cleared pair the seed pins at the gate (no clear write from the card)', async () => { + // The seed pins the values; the unconditional KD-S4 gate (no enable switch + // since 2026-09-26) refuses an incomplete pair, so the '' override the + // patch layer supports is a DIRTY-DERIVATION fact, never a card-issued + // write: clearing the provider marks the form dirty (a save would be + // needed) and the apply is refused until the pair is complete again. + const { remote, rpc, set } = scriptedApi({ config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - // The KD-S4 gate forbids Apply while enabled with empty provider/model, - // so the clear path is exercised with the switch off (values are then - // ignored by the host gate). - store.setEnabled(false) - store.setProvider('') + store.setProvider('') // clears the provider AND the invalidated model + expect(store.store.getSnapshot().dirty).toBe(true) await store.apply() - const payload = call.mock.calls.find(callArgs => callArgs[1] === 'advisor/set')?.[2] as { args: { patch: Record } } - expect(payload.args.patch).toEqual({ enabled: false, provider: '', model: '' }) + const { applyState } = store.store.getSnapshot() + expect(applyState.kind).toBe('error') + if (applyState.kind === 'error' && applyState.failure.kind === 'gate') { + expect(applyState.failure.reason).toBe('provider') + } + expect(set).not.toHaveBeenCalled() }) - it('keeps a base-clearing override stable across a second apply (no churn)', async () => { - // Apply 1 stores the explicit '' override; a later apply with a DIFFERENT - // edit must not re-emit the cleared keys — after the reload the get - // returns the stored '' (the wire may carry it), but the resolver treats - // '' as absent and the client reads it as missing, so the seed no longer - // pins provider/model and the second patch carries neither. + it('keeps earlier writes stable across a second apply (no churn)', async () => { + // Apply 1 stores the systemPrompt edit; a later apply with a DIFFERENT + // edit must not re-emit the already-stored key — after the reload the get + // returns the stored value, the seed no longer differs, and the second + // patch carries only the new key. const { remote, rpc, call } = scriptedApi({ - config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: 'entry', immuneTurns: 3, maxDeltaMessages: 60 }, }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(false) - store.setProvider('') - await store.apply() // Apply 1: stores the provider '' / model '' overrides + store.setSystemPrompt('edited') + await store.apply() // Apply 1: stores the systemPrompt edit let payload = call.mock.calls.find(callArgs => callArgs[1] === 'advisor/set')?.[2] as { args: { patch: Record } } - expect(payload.args.patch.provider).toBe('') + expect(payload.args.patch.systemPrompt).toBe('edited') - // Apply 2 with a different edit (immuneTurns) carries no provider/model - // key: nothing pins them anymore, so nothing is written. + // Apply 2 with a different edit (immuneTurns) carries no systemPrompt + // key: nothing differs anymore, so nothing is re-written. store.setImmuneTurns(9) await store.apply() const setCalls = call.mock.calls.filter(callArgs => callArgs[1] === 'advisor/set') @@ -474,23 +474,28 @@ describe('apply patch + seed (gateway channel semantics)', () => { expect(payload.args.patch).toEqual({ immuneTurns: 9 }) }) - it('omits the patch for a cleared provider/model nothing pins (nothing stored → no op)', async () => { + it('a cleared provider nothing pins stays clean (nothing stored → no op) and the gate still refuses the apply', async () => { // The config does not pin provider/model at all: clearing them writes // nothing — there is no stored value to remove (the old unset branch is // unreachable through the gateway: the returned config IS the effective - // view). + // view). The unconditional KD-S4 gate then refuses the pairless apply. const { remote, rpc, set } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() store.setProvider('') + expect(store.store.getSnapshot().dirty).toBe(false) await store.apply() expect(set).not.toHaveBeenCalled() - expect(store.store.getSnapshot().applyState.kind).toBe('saved') + const { applyState } = store.store.getSnapshot() + expect(applyState.kind).toBe('error') + if (applyState.kind === 'error' && applyState.failure.kind === 'gate') { + expect(applyState.failure.reason).toBe('provider') + } }) it('omits a cleared number field from the patch (empty input = leave unchanged)', async () => { const { remote, rpc, call } = scriptedApi({ - config: { enabled: false, systemPrompt: '', immuneTurns: 5, maxDeltaMessages: 60 }, + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 5, maxDeltaMessages: 60 }, }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() @@ -502,7 +507,9 @@ describe('apply patch + seed (gateway channel semantics)', () => { }) it('reports saved without a gateway call when the patch is empty', async () => { - const { remote, rpc, set } = scriptedApi() + const { remote, rpc, set } = scriptedApi({ + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() await store.apply() // no edits at all → nothing to write @@ -538,7 +545,6 @@ describe('apply patch + seed (gateway channel semantics)', () => { set.mockReturnValueOnce(Promise.resolve(failResult('host refused'))) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) store.setProvider('deepseek-official') store.setModel('ds-a') await store.apply() @@ -562,7 +568,6 @@ describe('apply patch + seed (gateway channel semantics)', () => { // The Once is registered AFTER load so it targets the set call, not the // load's get call. call.mockRejectedValueOnce(new Error('transport down')) - store.setEnabled(true) store.setProvider('deepseek-official') store.setModel('ds-a') await store.apply() @@ -572,7 +577,6 @@ describe('apply patch + seed (gateway channel semantics)', () => { expect(applyState.failure.message).toBe('transport down') } // The in-progress draft survives (nothing was written, nothing re-seeded). - expect(draft.enabled).toBe(true) expect(draft.provider).toBe('deepseek-official') expect(draft.model).toBe('ds-a') expect(describe).toHaveBeenCalledTimes(1) @@ -722,13 +726,13 @@ describe('gateway availability (KD-G5 — advisorPresent)', () => { expect(state.advisorPresent).toBe(false) // The pristine (unseeded) draft stays the schema defaults — NOT marked // seeded, so the next successful load can still seed. - expect(state.draft).toEqual({ enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) + expect(state.draft).toEqual({ systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) // Gateway recovers: the next load seeds the ACTUAL config. await store.load() state = store.store.getSnapshot() expect(state.advisorPresent).toBe(true) expect(state.draft).toEqual({ - enabled: true, provider: 'deepseek-official', model: 'ds-a', + provider: 'deepseek-official', model: 'ds-a', systemPrompt: 'entry', immuneTurns: 7, maxDeltaMessages: 20, }) }) @@ -736,7 +740,9 @@ describe('gateway availability (KD-G5 — advisorPresent)', () => { describe('post-apply reload failure (qc3 N-1)', () => { it('keeps the saved feedback when the reload after a successful set fails', async () => { - const { remote, rpc, describe, set } = scriptedApi() + const { remote, rpc, describe, set } = scriptedApi({ + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + }) // First describe (initial load) resolves; the post-apply reload fails. describe.mockReturnValueOnce(Promise.resolve(ok({ writable: true, @@ -766,14 +772,12 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() store.setProvider('openai') // a provider switch invalidates the chosen model - store.setEnabled(false) store.setSystemPrompt('edited') expect(draftOf(store).provider).toBe('openai') - expect(draftOf(store).enabled).toBe(false) store.discard() // The draft is exactly the seed again — every edited key rewound. expect(draftOf(store)).toEqual({ - enabled: true, provider: 'deepseek-official', model: 'ds-a', + provider: 'deepseek-official', model: 'ds-a', systemPrompt: 'entry', immuneTurns: 7, maxDeltaMessages: 20, }) // Discard is a client-side rewind — no advisor/set call ever happened. @@ -789,7 +793,6 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(false) store.setProvider('openai') store.setSystemPrompt('edited') store.discard() @@ -798,7 +801,7 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { expect(store.store.getSnapshot().applyState.kind).toBe('saved') // The empty apply leaves the rewound draft untouched (no re-seed, no re-write). expect(draftOf(store)).toEqual({ - enabled: true, provider: 'deepseek-official', model: 'ds-a', + provider: 'deepseek-official', model: 'ds-a', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, }) }) @@ -807,7 +810,7 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { const { remote, rpc } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) // enabled without provider/model → the KD-S4 gate blocks Apply + await store.apply() // no provider/model → the KD-S4 gate blocks Apply await store.apply() expect(store.store.getSnapshot().applyState.kind).toBe('error') store.discard() @@ -815,7 +818,9 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { }) it('clears the saved feedback back to idle after a landed apply (discard is a no-op on values)', async () => { - const { remote, rpc } = scriptedApi() + const { remote, rpc } = scriptedApi({ + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() store.setImmuneTurns(5) @@ -859,11 +864,11 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { await store.load() expect(store.store.getSnapshot().advisorPresent).toBe(false) store.discard() - expect(draftOf(store)).toEqual({ enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) + expect(draftOf(store)).toEqual({ systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) await store.load() expect(store.store.getSnapshot().advisorPresent).toBe(true) expect(draftOf(store)).toEqual({ - enabled: true, provider: 'deepseek-official', model: 'ds-a', + provider: 'deepseek-official', model: 'ds-a', systemPrompt: 'entry', immuneTurns: 7, maxDeltaMessages: 20, }) }) @@ -871,7 +876,9 @@ describe('discard (card draft rewind — T2 store add, T3 review)', () => { describe('dirty derivation (plan dsh-advisor-plugin-config-card-ux, task 2 — KD-U2)', () => { it('tracks the dirty lifecycle: clean → edit dirty → discard clean → edit → apply success clean', async () => { - const { remote, rpc } = scriptedApi() + const { remote, rpc } = scriptedApi({ + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + }) const store = new AdvisorSettingsStore(remote, rpc, schema) // The store default is clean — no edits staged against any seed. expect(store.store.getSnapshot().dirty).toBe(false) @@ -911,20 +918,19 @@ describe('dirty derivation (plan dsh-advisor-plugin-config-card-ux, task 2 — K const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() expect(store.store.getSnapshot().dirty).toBe(false) - // The KD-S4 gate forbids Apply while enabled with an empty provider/model, - // so the clear path is exercised with the switch off (values are then - // ignored by the host gate) — the dirty derivation itself does not care. - store.setEnabled(false) + // The unconditional KD-S4 gate refuses to APPLY a cleared pair, but the + // dirty derivation itself only diffs the draft against the seed: the + // clear is a staged edit (a save would be needed once the pair is + // complete again) and must read dirty. store.setProvider('') // patchFor emits provider: '' → a real write → dirty expect(store.store.getSnapshot().dirty).toBe(true) }) - it("derives dirty from the '' provider override alone — no enabled toggle needed (M-6 isolation)", async () => { - // The sibling test calls setEnabled(false) first, which alone makes the - // patch non-empty ({ enabled: false }) — this variant isolates the - // ''-provider semantic: NO enabled toggle, only the provider clear, and - // the seed pins no model, so the resulting patch is exactly - // { provider: '' } → dirty derives true from that alone. + it("derives dirty from the '' provider override alone (M-6 isolation)", async () => { + // The sibling test clears a provider whose seed also pins a model; this + // variant isolates the ''-provider semantic: the seed pins NO model, so + // the resulting patch is exactly { provider: '' } → dirty derives true + // from that alone. const { remote, rpc } = scriptedApi({ config: { enabled: false, provider: 'deepseek-official', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, }) @@ -976,7 +982,6 @@ describe('dirty derivation (plan dsh-advisor-plugin-config-card-ux, task 2 — K set.mockReturnValueOnce(Promise.resolve(failResult('host refused'))) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) store.setProvider('deepseek-official') store.setModel('ds-a') expect(store.store.getSnapshot().dirty).toBe(true) @@ -1007,61 +1012,59 @@ describe('dirty derivation (plan dsh-advisor-plugin-config-card-ux, task 2 — K expect(state.dirty).toBe(false) // host matches the draft → clean }) - it('keeps the draft dirty when the client gate blocks apply (the force-down patch is unreachable from the card) (S-3 pin)', async () => { - // qc2 S-3 (deferred — plan Risks, 2026-08-12): the host force-down - // (resolved enabled:false + disabledReason) is reachable only if a patch - // the client gate would block still reaches the host. This pin documents - // that such a patch cannot be produced from the card: enabled without - // provider/model is blocked by the gate BEFORE any write, dirty stays - // true for the user to complete, and advisor/set is never called. + it('keeps the draft dirty when the client gate blocks apply (an incomplete pair never writes) (S-3 pin)', async () => { + // qc2 S-3 (deferred — plan Risks, 2026-08-12), restated for the + // unconditional gate: the host force-down (resolved enabled:false + + // disabledReason) is reachable only if a patch the client gate would + // block still reaches the host. This pin documents that such a patch + // cannot be produced from the card: an incomplete provider/model pair is + // blocked by the gate BEFORE any write, dirty stays true for the user to + // complete, and advisor/set is never called. const { remote, rpc, set } = scriptedApi() const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - store.setEnabled(true) // enabled + no provider/model → the KD-S4 gate blocks apply + expect(store.store.getSnapshot().dirty).toBe(false) + store.setProvider('deepseek-official') // provider set, model still missing → gate blocks expect(store.store.getSnapshot().dirty).toBe(true) await store.apply() const state = store.store.getSnapshot() expect(state.applyState.kind).toBe('error') if (state.applyState.kind === 'error' && state.applyState.failure.kind === 'gate') { - expect(state.applyState.failure.reason).toBe('provider') + expect(state.applyState.failure.reason).toBe('model') } expect(state.dirty).toBe(true) // nothing written, nothing re-seeded — the form stays expect(set).not.toHaveBeenCalled() }) - it('recomputes dirty in the empty-patch apply branch — a stale seed cannot leave the pill lit (qc3 S-2)', async () => { + it('recomputes dirty in the empty-patch apply branch — an immediate re-apply after a landed save stays clean (qc3 S-2 restated)', async () => { // qc3 S-2 belt-and-braces: the empty-patch apply branch must recompute - // dirty like every other apply outcome. In every UI-reachable flow the - // patch is empty only when the draft already equals the seed (dirty - // false), but the M-7 degraded window can leave the SNAPSHOT dirty=true - // against a stale seed: a get-failure refresh clobbers `this.seed` to - // defaults while skipping the dirty recompute (config undefined). A - // programmatic apply in that window would diff EMPTY against the - // defaulted seed and report saved while the pill stayed lit. + // dirty like every other apply outcome. The original M-7 stale window (a + // get-failure refresh clobbers the seed to defaults while skipping the + // dirty recompute) can no longer reach this branch — the defaulted seed + // pairs with a pairless draft, which the unconditional KD-S4 gate refuses + // before the patch diff. The surviving reachable path: a draft exactly + // equal to a pair-pinning seed (right after a landed save), where the + // empty branch must still recompute and stay clean. const { remote, rpc, get, set } = scriptedApi({ - config: { enabled: true, provider: 'deepseek-official', model: 'ds-a', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, + config: { enabled: true, provider: 'x', model: 'y', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }, }) const store = new AdvisorSettingsStore(remote, rpc, schema) await store.load() - // Edit the draft back to the schema defaults (enabled off + cleared pair). - store.setEnabled(false) - store.setProvider('') // clears the provider AND the invalidated model - expect(draftOf(store)).toEqual({ enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 }) - expect(store.store.getSnapshot().dirty).toBe(true) // still differs from the pinned seed - // The degraded refresh: get fails → `seed` clobbers to defaults and the - // dirty recompute is skipped → the stale window (snapshot dirty=true). - get.mockImplementationOnce(() => Promise.resolve(failResult('advisor gateway is not ready'))) - await store.load() - let state = store.store.getSnapshot() - expect(state.advisorPresent).toBe(false) - expect(state.dirty).toBe(true) // stale — the recompute was skipped - // The programmatic apply diff-cleans against the (defaulted) seed → empty - // patch → saved WITHOUT a call, and the recompute clears the stale pill. + // Edit the draft and apply — the write lands and the returned config + // (with the pair) is adopted as the seed. + store.setSystemPrompt('edited') await store.apply() - state = store.store.getSnapshot() - expect(set).not.toHaveBeenCalled() + expect(set).toHaveBeenCalledTimes(1) + expect(store.store.getSnapshot().applyState.kind).toBe('saved') + expect(store.store.getSnapshot().dirty).toBe(false) + // An immediate re-apply: the draft equals the adopted seed → EMPTY patch → + // saved WITHOUT a call, and the branch recomputes dirty (stays clean). + await store.apply() + expect(set).toHaveBeenCalledTimes(1) + const state = store.store.getSnapshot() expect(state.applyState.kind).toBe('saved') expect(state.dirty).toBe(false) + expect(draftOf(store).provider).toBe('x') // the draft was never touched }) }) @@ -1109,13 +1112,12 @@ describe('card scenario (store-level load/save over the gateway channel)', () => await store.load() expect(store.store.getSnapshot().advisorPresent).toBe(true) - // Card edit: enable + pick provider/model → apply writes the minimal patch. - store.setEnabled(true) + // Card edit: pick provider/model → apply writes the minimal patch. store.setProvider('deepseek-official') store.setModel('ds-b') await store.apply() const payload = call.mock.calls.find(callArgs => callArgs[1] === 'advisor/set')?.[2] as { args: { patch: Record } } - expect(payload.args.patch).toEqual({ enabled: true, provider: 'deepseek-official', model: 'ds-b' }) + expect(payload.args.patch).toEqual({ provider: 'deepseek-official', model: 'ds-b' }) expect(get).toHaveBeenCalledTimes(2) // initial load + post-apply reload // Edit again, then discard: the draft rewinds to the post-apply seed. diff --git a/tests/commands.test.ts b/tests/commands.test.ts index 312e01f..8a3a5dc 100644 --- a/tests/commands.test.ts +++ b/tests/commands.test.ts @@ -228,21 +228,20 @@ describe('parseAdvisorCommand (parse of the text after /advisor)', () => { // AdvisorSessionOverrides — the override mechanism // --------------------------------------------------------------------------- -describe('AdvisorSessionOverrides (per-session override ?? config.enabled)', () => { - it('defaults to the config switch when no override is set', () => { - expect(new AdvisorSessionOverrides(false).effective('s1')).toBe(false) - expect(new AdvisorSessionOverrides(true).effective('s1')).toBe(true) +describe('AdvisorSessionOverrides (per-session override ?? the running-row default on)', () => { + it('defaults to ON when no override is set (a running row is enabled — no config switch since 2026-09-26)', () => { + expect(new AdvisorSessionOverrides().effective('s1')).toBe(true) }) it('an override flips the effective switch for that session only', () => { - const overrides = new AdvisorSessionOverrides(false) - overrides.set('s1', true) - expect(overrides.effective('s1')).toBe(true) - expect(overrides.effective('s2')).toBe(false) // other sessions untouched + const overrides = new AdvisorSessionOverrides() + overrides.set('s1', false) + expect(overrides.effective('s1')).toBe(false) + expect(overrides.effective('s2')).toBe(true) // other sessions keep the default }) - it('clear removes the override, falling back to the config switch', () => { - const overrides = new AdvisorSessionOverrides(true) + it('clear removes the override, falling back to the running-row default (on)', () => { + const overrides = new AdvisorSessionOverrides() overrides.set('s1', false) expect(overrides.effective('s1')).toBe(false) overrides.clear('s1') @@ -704,7 +703,7 @@ describe('apply wiring — /advisor config tuiSettingsAvailable reflects the tui /** Full plugin-row config shape for the apply wiring test. */ function entryConfig(): AdvisorConfig { - return { enabled: true, provider: 'openai', model: 'gpt-4o', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 } + return { provider: 'openai', model: 'gpt-4o', systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60 } } /** Minimal apply()-shaped ctx that ACTIVATES the `commands` inject child @@ -816,7 +815,7 @@ describe('parseModelSetArgs (atomic-pair validation, spec §5.3)', () => { describe('AdvisorSessionOverrides — model pair API (spec §5.3)', () => { it('absence inherits; setModel commits an atomic pair; clearModel deletes', () => { - const overrides = new AdvisorSessionOverrides(false) + const overrides = new AdvisorSessionOverrides() expect(overrides.model('s1')).toBeUndefined() overrides.setModel('s1', { provider: 'p', model: 'm' }) expect(overrides.model('s1')).toEqual({ provider: 'p', model: 'm' }) @@ -827,7 +826,7 @@ describe('AdvisorSessionOverrides — model pair API (spec §5.3)', () => { }) it('generations fence model work: begin bumps, clear resets to 0', () => { - const overrides = new AdvisorSessionOverrides(false) + const overrides = new AdvisorSessionOverrides() expect(overrides.modelGeneration('s1')).toBe(0) const g1 = overrides.beginModelGeneration('s1') expect(g1).toBe(1) @@ -838,39 +837,41 @@ describe('AdvisorSessionOverrides — model pair API (spec §5.3)', () => { }) it('generations are per-session', () => { - const overrides = new AdvisorSessionOverrides(false) + const overrides = new AdvisorSessionOverrides() expect(overrides.beginModelGeneration('s1')).toBe(1) expect(overrides.beginModelGeneration('s2')).toBe(1) expect(overrides.modelGeneration('s1')).toBe(1) }) it('clear wipes enable + pair + generation together (dispose)', () => { - const overrides = new AdvisorSessionOverrides(false) - overrides.set('s1', true) + const overrides = new AdvisorSessionOverrides() + overrides.set('s1', false) overrides.setModel('s1', { provider: 'p', model: 'm' }) overrides.beginModelGeneration('s1') overrides.clear('s1') - expect(overrides.effective('s1')).toBe(false) + // The enable override is wiped with the rest: the effective switch falls + // back to the running-row default (on). + expect(overrides.effective('s1')).toBe(true) expect(overrides.model('s1')).toBeUndefined() expect(overrides.modelGeneration('s1')).toBe(0) }) it('disposeAll wipes every session (owner teardown invalidates all fences)', () => { - const overrides = new AdvisorSessionOverrides(false) - overrides.set('s1', true) + const overrides = new AdvisorSessionOverrides() + overrides.set('s1', false) overrides.setModel('s1', { provider: 'p', model: 'm' }) overrides.beginModelGeneration('s1') overrides.beginModelGeneration('s2') overrides.disposeAll() - expect(overrides.effective('s1')).toBe(false) + expect(overrides.effective('s1')).toBe(true) expect(overrides.model('s1')).toBeUndefined() expect(overrides.modelGeneration('s1')).toBe(0) expect(overrides.modelGeneration('s2')).toBe(0) }) it('overrideSessionIds enumerates sessions holding enable and/or pair state', () => { - const overrides = new AdvisorSessionOverrides(false) - overrides.set('a', true) + const overrides = new AdvisorSessionOverrides() + overrides.set('a', false) overrides.setModel('b', { provider: 'p', model: 'm' }) expect([...overrides.overrideSessionIds()].sort()).toEqual(['a', 'b']) overrides.clearModel('b') diff --git a/tests/config.test.ts b/tests/config.test.ts index 4588108..c40b291 100644 --- a/tests/config.test.ts +++ b/tests/config.test.ts @@ -3,16 +3,20 @@ * * Contract under test: * - The exported schemastery `Config` schema (the cordis Loader path) applies - * defaults (`enabled` false, `immuneTurns` 3, `maxDeltaMessages` 60, - * `systemPrompt` "") and enforces types/bounds (int ≥ 0). All six live - * fields are `.volatile()`: `Config(raw)` resolves them to `{ get() }` - * references (the loader's no-remount edit channel), and the reads below go - * through `unwrapAdvisorConfig` — the same unwrapping the runtime does - * before the gate. - * - `resolveAdvisorConfig(raw)` never throws for the gate scenario: when - * `enabled` is true but `provider`/`model` is missing or empty it resolves - * to a disabled-with-reason config (no model call). - * - Unknown config keys are rejected (strict schema). + * defaults (`immuneTurns` 3, `maxDeltaMessages` 60, `systemPrompt` "") and + * enforces types/bounds (int ≥ 0). All five live fields are `.volatile()`: + * `Config(raw)` resolves them to `{ get() }` references (the loader's + * no-remount edit channel), and the reads below go through + * `unwrapAdvisorConfig` — the same unwrapping the runtime does before the + * gate. + * - `resolveAdvisorConfig(raw)` never throws for the gate scenario: there is + * no config-level `enabled` key (2026-09-26 — the plugin-row toggle is the + * switch), so when `provider`/`model` is missing or empty it resolves to a + * disabled-with-reason config (no model call); a complete pair resolves + * enabled. + * - Unknown config keys are rejected (strict schema) — including `enabled` + * itself, whose removal is the 2026-09-26 breaking change (a stored profile + * still carrying it is rejected with an actionable message). */ import { describe, expect, it } from 'vitest' @@ -24,14 +28,13 @@ describe('schema defaults (cordis Loader path, spec §5.1)', () => { // fiber's references. A resolved field must duck-type as `{ get() }` — // every runtime read (unwrapAdvisorConfig → the gate) depends on it. const resolved: Record = Config({}) - for (const key of ['enabled', 'provider', 'model', 'systemPrompt', 'immuneTurns', 'maxDeltaMessages']) { + for (const key of ['provider', 'model', 'systemPrompt', 'immuneTurns', 'maxDeltaMessages']) { expect(typeof (resolved[key] as { get?: unknown }).get, key).toBe('function') } }) it('applies defaults for an empty config', () => { expect(unwrapAdvisorConfig(Config({}))).toEqual({ - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -40,14 +43,12 @@ describe('schema defaults (cordis Loader path, spec §5.1)', () => { it('keeps explicit values over defaults', () => { expect(unwrapAdvisorConfig(Config({ - enabled: true, provider: 'deepseek', model: 'deepseek-chat', systemPrompt: 'custom reviewer prompt', immuneTurns: 5, maxDeltaMessages: 10, }))).toEqual({ - enabled: true, provider: 'deepseek', model: 'deepseek-chat', systemPrompt: 'custom reviewer prompt', @@ -56,9 +57,8 @@ describe('schema defaults (cordis Loader path, spec §5.1)', () => { }) }) - it('rejects non-boolean / non-number / non-string values', () => { + it('rejects non-number / non-string values', () => { // `as never` — inputs are intentionally invalid; runtime must reject them. - expect(() => Config({ enabled: 'yes' as never })).toThrow() expect(() => Config({ immuneTurns: '3' as never })).toThrow() expect(() => Config({ systemPrompt: 42 as never })).toThrow() expect(() => Config({ provider: 7 as never })).toThrow() @@ -66,7 +66,6 @@ describe('schema defaults (cordis Loader path, spec §5.1)', () => { it('treats null as absent (schemastery nullable input → default)', () => { expect(unwrapAdvisorConfig(Config({ maxDeltaMessages: null })).maxDeltaMessages).toBe(60) - expect(unwrapAdvisorConfig(Config({ enabled: null })).enabled).toBe(false) }) it('enforces integer ≥ 0 bounds; 0 = unbounded is allowed', () => { @@ -80,62 +79,57 @@ describe('schema defaults (cordis Loader path, spec §5.1)', () => { }) describe('explicit model gate (S4 / spec §5.2)', () => { - it('is disabled by default without provider/model (no reason)', () => { + it('resolves to disabled-with-reason by default (no provider/model)', () => { + // No config-level switch since 2026-09-26: the gate keys purely on the + // pair, so the defaulted config is disabled-with-reason, not silently off. const resolved = resolveAdvisorConfig({}) expect(resolved.enabled).toBe(false) - expect(resolved.disabledReason).toBeUndefined() + expect(resolved.disabledReason).toBeTruthy() expect(resolved.systemPrompt).toBe('') expect(resolved.immuneTurns).toBe(3) expect(resolved.maxDeltaMessages).toBe(60) }) - it('resolves to disabled-with-reason when enabled without provider/model', () => { - const resolved = resolveAdvisorConfig({ enabled: true }) - expect(resolved.enabled).toBe(false) - expect(resolved.disabledReason).toBeTruthy() - }) - it('resolves to disabled-with-reason when only provider is set', () => { - const resolved = resolveAdvisorConfig({ enabled: true, provider: 'deepseek' }) + const resolved = resolveAdvisorConfig({ provider: 'deepseek' }) expect(resolved.enabled).toBe(false) expect(resolved.disabledReason).toBeTruthy() }) it('resolves to disabled-with-reason when only model is set', () => { - const resolved = resolveAdvisorConfig({ enabled: true, model: 'deepseek-chat' }) + const resolved = resolveAdvisorConfig({ model: 'deepseek-chat' }) expect(resolved.enabled).toBe(false) expect(resolved.disabledReason).toBeTruthy() }) it('treats empty provider or model as missing (gate requires both)', () => { - expect(resolveAdvisorConfig({ enabled: true, provider: '', model: 'm' }).enabled).toBe(false) - expect(resolveAdvisorConfig({ enabled: true, provider: 'p', model: '' }).enabled).toBe(false) - expect(resolveAdvisorConfig({ enabled: true, provider: '', model: '' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: '', model: 'm' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: 'p', model: '' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: '', model: '' }).enabled).toBe(false) }) it('treats whitespace-only provider/model as missing (trim before the gate, qc2 W-3 / qc3 I-3)', () => { - expect(resolveAdvisorConfig({ enabled: true, provider: ' ', model: 'm' }).enabled).toBe(false) - expect(resolveAdvisorConfig({ enabled: true, provider: 'p', model: ' ' }).enabled).toBe(false) - expect(resolveAdvisorConfig({ enabled: true, provider: ' \t ', model: ' ' }).enabled).toBe(false) - const resolved = resolveAdvisorConfig({ enabled: true, provider: ' ', model: 'm' }) + expect(resolveAdvisorConfig({ provider: ' ', model: 'm' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: 'p', model: ' ' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: ' \t ', model: ' ' }).enabled).toBe(false) + const resolved = resolveAdvisorConfig({ provider: ' ', model: 'm' }) expect(resolved.disabledReason).toBeTruthy() }) it('treats null provider/model as missing (normalized before the gate)', () => { - expect(resolveAdvisorConfig({ enabled: true, provider: null, model: 'm' }).enabled).toBe(false) - expect(resolveAdvisorConfig({ enabled: true, provider: null, model: null }).enabled).toBe(false) - const resolved = resolveAdvisorConfig({ enabled: true, provider: null, model: 'm' }) + expect(resolveAdvisorConfig({ provider: null, model: 'm' }).enabled).toBe(false) + expect(resolveAdvisorConfig({ provider: null, model: null }).enabled).toBe(false) + const resolved = resolveAdvisorConfig({ provider: null, model: 'm' }) expect(resolved.disabledReason).toBeTruthy() }) it('never throws for the gate scenario', () => { - expect(() => resolveAdvisorConfig({ enabled: true })).not.toThrow() - expect(() => resolveAdvisorConfig({ enabled: true, provider: 'p' })).not.toThrow() + expect(() => resolveAdvisorConfig({})).not.toThrow() + expect(() => resolveAdvisorConfig({ provider: 'p' })).not.toThrow() }) it('resolves enabled when both provider and model are present', () => { const resolved = resolveAdvisorConfig({ - enabled: true, provider: 'deepseek', model: 'deepseek-chat', }) @@ -147,7 +141,6 @@ describe('explicit model gate (S4 / spec §5.2)', () => { it('preserves defaults and explicit values in the resolved config', () => { expect(resolveAdvisorConfig({ - enabled: true, provider: 'p', model: 'm', systemPrompt: 'custom', @@ -162,27 +155,29 @@ describe('explicit model gate (S4 / spec §5.2)', () => { maxDeltaMessages: 0, }) }) - - it('ignores provider/model while disabled (gate not applied)', () => { - const resolved = resolveAdvisorConfig({ enabled: false, provider: 'p', model: 'm' }) - expect(resolved.enabled).toBe(false) - expect(resolved.disabledReason).toBeUndefined() - expect(resolved.provider).toBe('p') - expect(resolved.model).toBe('m') - }) }) describe('strict schema — unknown keys rejected (spec §5.2)', () => { - it('rejects unknown keys when disabled', () => { - expect(() => resolveAdvisorConfig({ enabled: false, bogus: 1 })) + it('rejects unknown keys on a pairless config', () => { + expect(() => resolveAdvisorConfig({ bogus: 1 })) .toThrow(/unknown config key "bogus"/) }) - it('rejects unknown keys when enabled with a valid pair', () => { - expect(() => resolveAdvisorConfig({ enabled: true, provider: 'p', model: 'm', extra: true })) + it('rejects unknown keys on a config with a valid pair', () => { + expect(() => resolveAdvisorConfig({ provider: 'p', model: 'm', extra: true })) .toThrow(/unknown config key "extra"/) }) + it('rejects the removed `enabled` key (2026-09-26 breaking change — no compat layer)', () => { + // The migration surface: a stored profile still carrying `enabled:` is + // rejected like any unknown key (the row toggle replaces it). The host + // surfaces the message as the row's disabledReason. + expect(() => resolveAdvisorConfig({ enabled: true })) + .toThrow(/unknown config key "enabled"/) + expect(() => resolveAdvisorConfig({ enabled: false })) + .toThrow(/unknown config key "enabled"/) + }) + it('rejects non-object config input', () => { expect(() => resolveAdvisorConfig('nope')).toThrow() expect(() => resolveAdvisorConfig(null)).toThrow() diff --git a/tests/gateway-session.test.ts b/tests/gateway-session.test.ts index b2fd017..c8f1703 100644 --- a/tests/gateway-session.test.ts +++ b/tests/gateway-session.test.ts @@ -44,7 +44,6 @@ beforeEach(() => { /** Full entry (plugin-row) config shape, merged over the schema defaults. */ function entryConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -142,10 +141,10 @@ interface Harness { } /** - * Compose the real plugin over a pairless-but-enabled global default: the - * entry gate blocks (enabled: true without provider/model), which is exactly - * the state where a session pin must satisfy the gate and a reset must report - * the blocked state truthfully. + * Compose the real plugin over a pairless global default: the entry gate + * blocks (no provider/model — no config-level switch since 2026-09-26), which + * is exactly the state where a session pin must satisfy the gate and a reset + * must report the blocked state truthfully. */ async function compose(): Promise { const ctx = new Context() @@ -155,7 +154,7 @@ async function compose(): Promise { const resolveModelInfo = vi.fn(async () => ({ provider: 'deepseek', model: 'deepseek-chat' })) ctx.provide('agents', { get: (id: string) => (id === 'sess-1' ? agentDouble(id) : undefined) } as never) ctx.provide('llm', { stream: async () => {}, resolveModelInfo } as never) - const entry = new MemoryEntryConfig(entryConfig({ enabled: true })) + const entry = new MemoryEntryConfig(entryConfig()) await ctx.plugin(harnessAdvisorPlugin(), entry.config) await vi.waitFor(() => { expect(ctx.reflect.props['advisor']).toEqual({ type: 'service' }) @@ -274,7 +273,7 @@ describe('composed session endpoints (apply wiring)', () => { ctx.provide('agents', { get: (id: string) => (id === 'sess-1' ? agentDouble(id) : undefined) } as never) const resolveModelInfo = vi.fn(async () => { throw new Error('unknown route') }) ctx.provide('llm', { stream: async () => {}, resolveModelInfo } as never) - const entry = new MemoryEntryConfig(entryConfig({ enabled: true })) + const entry = new MemoryEntryConfig(entryConfig()) await ctx.plugin(harnessAdvisorPlugin(), entry.config) await vi.waitFor(() => expect(ctx.reflect.props['advisor']).toEqual({ type: 'service' })) diff --git a/tests/gateway.test.ts b/tests/gateway.test.ts index db6ae89..b960d98 100644 --- a/tests/gateway.test.ts +++ b/tests/gateway.test.ts @@ -16,8 +16,9 @@ * patch rides the in-process write channel (the wire-level exposed- * namespace check only guards the apiproxy path). * ③ `set` with an unknown key is rejected by the `Config` schema - * (unknown-key rejection unchanged) and nothing is persisted. - * ④ Hard gate regression: enabled without provider/model still resolves to + * (unknown-key rejection unchanged) and nothing is persisted — the removed + * `enabled` key (2026-09-26) is rejected the same way. + * ④ Hard gate regression: a pairless config still resolves to * disabled-with-reason (no model call — SSOT unchanged). * ⑤ Endpoint claims: the explicit typert registration (the same * `ctx.typert.local` store `claimsEndpoint` checks) claims @@ -57,7 +58,6 @@ beforeEach(() => { /** Full entry (plugin-row) config shape, merged over the schema defaults. */ function entryConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -123,7 +123,7 @@ async function waitCaptured(ctx: Context, gateway: AdvisorConfigGateway): Promis describe('no settings service (entry fallback)', () => { it('get returns the entry composed value; the gateway is a registered service', () => { const ctx = new Context() - const entry = entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 }) + const entry = entryConfig({ provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 }) const gateway = new AdvisorConfigGateway(ctx, installAdvisorSettings(ctx, entry)) expect(ctx.reflect.props['advisor']).toEqual({ type: 'service' }) @@ -141,7 +141,7 @@ describe('no settings service (entry fallback)', () => { it('get unwraps a volatile-reference entry (the loader-resolved shape)', () => { const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat' })) const gateway = new AdvisorConfigGateway(ctx, installAdvisorSettings(ctx, entry.config)) expect(gateway.get().config.enabled).toBe(true) expect(gateway.get().config.provider).toBe('deepseek') @@ -150,7 +150,7 @@ describe('no settings service (entry fallback)', () => { it('set fails cleanly when no settings service is composed (KD-G5 error path)', async () => { const ctx = new Context() const gateway = new AdvisorConfigGateway(ctx, installAdvisorSettings(ctx, entryConfig())) - await expect(gateway.set({ enabled: true })).rejects.toThrow(/settings service is unavailable/) + await expect(gateway.set({ maxDeltaMessages: 10 })).rejects.toThrow(/settings service is unavailable/) }) it('a second gateway on the same context fails loud (multi-fiber dedupe relies on this)', () => { @@ -175,7 +175,7 @@ describe('with a settings service (set writes the entry config)', () => { await provideSettingsDouble(ctx, entry) await waitCaptured(ctx, gateway) - const result = await gateway.set({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' }) + const result = await gateway.set({ provider: 'deepseek', model: 'deepseek-chat' }) // The write rode the settings channel keyed by the ENTRY id. const composed: ResolvedAdvisorConfig = { @@ -201,8 +201,8 @@ describe('with a settings service (set writes the entry config)', () => { const settings = await provideSettingsDouble(ctx, entry) await waitCaptured(ctx, gateway) - await gateway.set({ enabled: true }) - expect(settings.update).toHaveBeenCalledWith('advisor', { enabled: true }) + await gateway.set({ maxDeltaMessages: 10 }) + expect(settings.update).toHaveBeenCalledWith('advisor', { maxDeltaMessages: 10 }) }) it('a patch changing only one key leaves the other entry values intact', async () => { @@ -220,6 +220,7 @@ describe('with a settings service (set writes the entry config)', () => { systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 10, + disabledReason: expect.any(String), }, }) }) @@ -232,11 +233,11 @@ describe('with a settings service (set writes the entry config)', () => { await provideSettingsDouble(ctx, entry) await waitCaptured(ctx, gateway) - await gateway.set({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' }) + await gateway.set({ provider: 'deepseek', model: 'deepseek-chat' }) await gateway.set({ maxDeltaMessages: 10 }) - // The entry config keeps ALL four keys written across the two calls — a - // replace-semantics write would have dropped the earlier trio. + // The entry config keeps ALL three keys written across the two calls — a + // replace-semantics write would have dropped the earlier pair. expect(gateway.get()).toEqual({ config: { enabled: true, @@ -251,7 +252,7 @@ describe('with a settings service (set writes the entry config)', () => { it('an empty patch is a no-op: returns the current composed value without a write (S2)', async () => { const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat' })) const bridge = installAdvisorSettings(ctx, entry.config) const gateway = new AdvisorConfigGateway(ctx, bridge) const settings = await provideSettingsDouble(ctx, entry) @@ -269,7 +270,7 @@ describe('with a settings service (set writes the entry config)', () => { // read; the raw entry config must not store it either — the null key is // dropped before the write, so the pinned provider survives. const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat' })) const bridge = installAdvisorSettings(ctx, entry.config) const gateway = new AdvisorConfigGateway(ctx, bridge) await provideSettingsDouble(ctx, entry) @@ -288,7 +289,7 @@ describe('with a settings service (set writes the entry config)', () => { it('an all-null patch is a no-op: nothing written, composed value unchanged', async () => { const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat' })) const bridge = installAdvisorSettings(ctx, entry.config) const gateway = new AdvisorConfigGateway(ctx, bridge) const settings = await provideSettingsDouble(ctx, entry) @@ -315,7 +316,7 @@ describe('with a settings service (set writes the entry config)', () => { ctx.registry.delete(MemorySettingsDouble) await vi.waitFor(() => expect(settingsOf(gateway)).toBeUndefined()) - await expect(gateway.set({ enabled: true })).rejects.toThrow(/settings service is unavailable/) + await expect(gateway.set({ maxDeltaMessages: 10 })).rejects.toThrow(/settings service is unavailable/) }) }) @@ -355,17 +356,26 @@ describe('set validation (Config schema, unknown-key rejection unchanged)', () = // --------------------------------------------------------------------------- describe('hard gate regression (resolveAdvisorConfig stays the SSOT)', () => { - it('set-enabled without provider/model still resolves to disabled-with-reason', async () => { + it('a patch carrying the removed `enabled` key is rejected (unknown key — 2026-09-26 removal, no compat layer)', async () => { const ctx = new Context() const entry = new MemoryEntryConfig(entryConfig()) const bridge = installAdvisorSettings(ctx, entry.config) const gateway = new AdvisorConfigGateway(ctx, bridge) - await provideSettingsDouble(ctx, entry) + const settings = await provideSettingsDouble(ctx, entry) await waitCaptured(ctx, gateway) - // The schema accepts an enabled-without-pair patch (the gate is a READ - // resolution, not a write gate — the user may configure in stages). - await gateway.set({ enabled: true }) + // The config-level switch is gone (the row toggle is the switch): a + // stored profile's `enabled:` lands at `set` like any unknown key and + // nothing is written. + await expect(gateway.set({ enabled: true } as never)).rejects.toThrow(/unknown config key "enabled"/) + expect(settings.update).not.toHaveBeenCalled() + }) + + it('a pairless entry resolves to disabled-with-reason through get', async () => { + const ctx = new Context() + const entry = new MemoryEntryConfig(entryConfig()) + const gateway = new AdvisorConfigGateway(ctx, installAdvisorSettings(ctx, entry.config)) + const config = gateway.get().config expect(config.enabled).toBe(false) expect(config.disabledReason).toMatch(/provider and model are missing/) @@ -382,7 +392,9 @@ describe('hard gate regression (resolveAdvisorConfig stays the SSOT)', () => { await provideSettingsDouble(ctx, entry) await waitCaptured(ctx, gateway) - await gateway.set({ enabled: true, provider: '', model: '' }) + // The gate is a READ resolution, not a write gate — the user may + // configure in stages; the empty pair simply resolves disabled. + await gateway.set({ provider: '', model: '' }) const config = gateway.get().config expect(config.enabled).toBe(false) expect(config.disabledReason).toBeTruthy() @@ -482,13 +494,14 @@ describe('typertGateway endpoint claims + payload contract', () => { systemPrompt: 'entry prompt', immuneTurns: 5, maxDeltaMessages: 60, + disabledReason: expect.any(String), }, }, }) const setResult = await connection.handler!( 'advisor/set', - { args: { patch: { enabled: true, provider: 'deepseek', model: 'deepseek-chat' } } }, + { args: { patch: { provider: 'deepseek', model: 'deepseek-chat' } } }, signal, ) expect(setResult.ok).toBe(true) @@ -590,6 +603,7 @@ describe('composed plugin (apply wires the gateway)', () => { systemPrompt: 'entry prompt', immuneTurns: 5, maxDeltaMessages: 60, + disabledReason: expect.any(String), }) // The set child may activate a tick after the plugin loads; the waitFor @@ -598,7 +612,7 @@ describe('composed plugin (apply wires the gateway)', () => { const result = await ctx.typertGateway.invoke({ namespace: 'advisor', method: 'set', - args: { patch: { enabled: true, provider: 'deepseek', model: 'deepseek-chat' } }, + args: { patch: { provider: 'deepseek', model: 'deepseek-chat' } }, }) as { config: ResolvedAdvisorConfig } expect(result.config.enabled).toBe(true) expect(result.config.provider).toBe('deepseek') diff --git a/tests/integration.test.ts b/tests/integration.test.ts index ac80214..7505808 100644 --- a/tests/integration.test.ts +++ b/tests/integration.test.ts @@ -20,8 +20,8 @@ * `agent.steer` with a message whose source is the `plugin` arm tagged * `plugin: 'advisor'`. * 2. A nit routes to `agent.inject`, never `steer`. - * 3. The explicit model gate (S4): `enabled: true` without `provider`/`model` - * starts zero model calls. + * 3. The explicit model gate (S4): a config without `provider`/`model` + * (no config-level switch since 2026-09-26) starts zero model calls. * 4. `/advisor` commands register only when a `commands` registry is composed * (conditional child activation — T7 ⚠️). * 5. A `compact/*` event and a `user/message` surface replace both reset the @@ -121,7 +121,6 @@ class StubAdapter extends LlmAdapter { /** Merge test config over the schema defaults (full `AdvisorConfig` shape). */ function fullConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -399,7 +398,7 @@ async function registerCommands(ctx: Context): Promise { it('drives user → primary → turn/end → delta → advisor call → guard → steer with a stub adapter', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"extract the helper","severity":"concern"}')]], ) const { agent, steer, inject } = makeFakeAgent('s1') @@ -436,7 +435,7 @@ describe('integration — full advisor loop (spec §7)', () => { it('routes a nit to agent.inject, never steer', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"add a unit test","severity":"nit"}')]], ) const { agent, steer, inject } = makeFakeAgent('s1') @@ -453,8 +452,10 @@ describe('integration — full advisor loop (spec §7)', () => { expect(adapter.requests).toHaveLength(1) }) - it('starts zero model calls when enabled without provider/model (explicit gate, S4)', async () => { - const { ctx, adapter } = await composeHarness({ enabled: true }, []) + it('starts zero model calls without provider/model (explicit gate, S4)', async () => { + // No config-level switch since 2026-09-26: the pairless entry resolves to + // disabled-with-reason and the gate drops every delta. + const { ctx, adapter } = await composeHarness({}, []) const { agent } = makeFakeAgent('s1') ctx.emit('agent/created', { agent, source: 'startup' }) const { session, log } = makeSession('s1') @@ -464,18 +465,6 @@ describe('integration — full advisor loop (spec §7)', () => { await flush() expect(adapter.requests).toEqual([]) // no runtime → no model call, ever }) - - it('starts zero model calls when the config switch is off (enabled: false)', async () => { - const { ctx, adapter } = await composeHarness({ enabled: false }, []) - const { agent } = makeFakeAgent('s1') - ctx.emit('agent/created', { agent, source: 'startup' }) - const { session, log } = makeSession('s1') - - feed(ctx, session, log, simpleTurn(1, 'do the thing', 'done')) - - await flush() - expect(adapter.requests).toEqual([]) - }) }) // --------------------------------------------------------------------------- @@ -495,7 +484,7 @@ describe('integration — full advisor loop (spec §7)', () => { describe('integration — agentic reply-complete gate drives the loop without turn/end (KD-N4-5)', () => { it('harness stream → advisor calls per round, nit→inject / concern→steer, immuneTurns fence decays', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model', immuneTurns: 3 }, + { provider: 'stub', model: 'stub-model', immuneTurns: 3 }, [ [...textReply('{"note":"concern one","severity":"concern"}')], [...textReply('{"note":"concern two","severity":"concern"}')], @@ -590,7 +579,7 @@ describe('integration — agentic reply-complete gate drives the loop without tu it('routes a nit note to agent.inject in a harness stream', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"add a unit test","severity":"nit"}')]], ) const { agent, steer, inject } = makeFakeAgent('s1') @@ -613,7 +602,7 @@ describe('integration — agentic reply-complete gate drives the loop without tu it('mode latch: after a reviewable turn/end the new gate stays dormant', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"extract the helper","severity":"concern"}')]], ) const { agent, steer } = makeFakeAgent('s1') @@ -648,7 +637,7 @@ describe('integration — agentic reply-complete gate drives the loop without tu describe('integration — advisor self-delivery never re-triggers the review gate (C-1)', () => { it('a re-emitted advisor inbox splice during delivery produces exactly one review per round', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"nit one","severity":"nit"}')], [...textReply('{"note":"nit two","severity":"nit"}')], @@ -721,7 +710,7 @@ describe('integration — advisor self-delivery never re-triggers the review gat describe('integration — /advisor commands conditional activation (T7)', () => { it('runs a full cycle without a commands registry, then registers when one is composed', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"watch the loop bound","severity":"concern"}')]], ) const { agent, steer } = makeFakeAgent('s1') @@ -747,14 +736,17 @@ describe('integration — /advisor commands conditional activation (T7)', () => expect(typeof definitions[0]!.handler).toBe('function') }) - it('the registered /advisor on handler starts the live runtime (KD-5 seed-on-enable)', async () => { + it('the registered /advisor on handler restarts an off session with the KD-5 seed-on-enable', async () => { const { ctx, adapter } = await composeHarness( - // Config switch off; provider/model present so the S4 gate passes once - // the per-session override flips on. - { enabled: false, provider: 'stub', model: 'stub-model' }, - [[...textReply('{"note":"after enabling","severity":"concern"}')]], + // Provider/model present so the S4 gate passes once the per-session + // override flips on; the session is paused with /advisor off below. + { provider: 'stub', model: 'stub-model' }, + [ + [...textReply('{"note":"pre pause","severity":"nit"}')], + [...textReply('{"note":"after enabling","severity":"concern"}')], + ], ) - const { agent, steer } = makeFakeAgent('s1') + const { agent, steer, inject } = makeFakeAgent('s1') ctx.emit('agent/created', { agent, source: 'startup' }) const { session, log } = makeSession('s1') @@ -769,10 +761,24 @@ describe('integration — /advisor commands conditional activation (T7)', () => } as never) await vi.waitFor(() => expect(definitions).toHaveLength(1)) - // Config off → a completed turn produces no model call. + // A running row is on: a completed turn is reviewed (nit → inject). feed(ctx, session, log, simpleTurn(1, 'history turn', 'old reply')) + await vi.waitFor(() => expect(inject).toHaveBeenCalledTimes(1)) + + // /advisor off with the REAL handler: the runtime is disposed and a + // completed turn produces no model call. + const off = definitions[0]!.handler({ + commandId: CommandId('cmd-t8'), + agent: { id: 's1', session: { id: 's1', seq: log.length } } as unknown as Agent, + rawInput: ' off', + attachments: [], + signal: new AbortController().signal, + }) + if (off instanceof Promise) throw new Error('test: /advisor handler must be synchronous') + expect(off.text).toContain('Advisor off') + feed(ctx, session, log, simpleTurn(2, 'paused work', 'paused reply')) await flush() - expect(adapter.requests).toEqual([]) + expect(adapter.requests).toHaveLength(1) // disabled: no call // /advisor on with the REAL handler: flips the override, seeds the cursor // to the current transcript length, and creates/resumes the runtime. @@ -788,13 +794,14 @@ describe('integration — /advisor commands conditional activation (T7)', () => // The next completed turn is reviewed — incrementally, without replaying // the pre-enable history (KD-5 seed-on-enable). - feed(ctx, session, log, simpleTurn(2, 'new work', 'new reply')) + feed(ctx, session, log, simpleTurn(3, 'new work', 'new reply')) await vi.waitFor(() => expect(steer).toHaveBeenCalledTimes(1)) - expect(adapter.requests).toHaveLength(1) - const delta = deltaTextOf(adapter.requests[0]!) + expect(adapter.requests).toHaveLength(2) + const delta = deltaTextOf(adapter.requests[1]!) expect(delta).toContain('**user**: new work') expect(delta).not.toContain('history turn') + expect(delta).not.toContain('paused work') expect(steer.mock.calls[0]![0]).toMatchObject({ role: 'user', source: { kind: 'advisor' }, @@ -809,7 +816,7 @@ describe('integration — /advisor commands conditional activation (T7)', () => describe('integration — compact / surface-replace reset the composed observer + guard (KD-5)', () => { it('a compact/* rewrite triggers a full replay AND a fresh emission-guard history', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"extract the helper","severity":"concern"}')], [...textReply('{"note":"extract the helper","severity":"concern"}')], @@ -847,7 +854,7 @@ describe('integration — compact / surface-replace reset the composed observer it('a user/message surface replace (no compact events) also triggers the replay', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"first nit","severity":"nit"}')], [...textReply('{"note":"second nit","severity":"nit"}')], @@ -883,16 +890,16 @@ describe('integration — compact / surface-replace reset the composed observer // --------------------------------------------------------------------------- describe('integration — /advisor recovery + S4 gate reporting wiring (QC fix wave 1)', () => { - it('config-enabled-but-gate-blocked: status shows the S4 reason and /advisor on says no model call can start (qc3 I-1/I-2)', async () => { - const { ctx, adapter } = await composeHarness({ enabled: true }, []) + it('pairless-gate-blocked: status shows the S4 reason and /advisor on says no model call can start (qc3 I-1/I-2)', async () => { + const { ctx, adapter } = await composeHarness({}, []) const { agent } = makeFakeAgent('s1') ctx.emit('agent/created', { agent, source: 'startup' }) const { session, log } = makeSession('s1') const handler = await registerCommands(ctx) // `/advisor status` must show the disabled-with-reason (spec §5.2) — the - // gate reason previously vanished because the overrides were seeded with - // the POST-gate switch. + // reason is re-derived through the resolver's post-gate resolution on + // every read. const status = invokeHandler(handler, ' status', session) expect(status.kind).toBe('success') expect(status.text).toContain('Reason:') @@ -910,7 +917,7 @@ describe('integration — /advisor recovery + S4 gate reporting wiring (QC fix w it('/advisor on resumes a quota-paused session advisor (KD-5 manual resume; qc1/qc2/qc3 W-1/I-4)', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...errorReply(quotaFailure())], // turn 1 → quota_exhausted pause [...textReply('{"note":"back after resume","severity":"concern"}')], // the retained batch, after resume @@ -938,7 +945,7 @@ describe('integration — /advisor recovery + S4 gate reporting wiring (QC fix w it('/advisor on rebuilds a halted session advisor after a permanent model error (qc1/qc2/qc3 W-1/I-4)', async () => { const { ctx, adapter } = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...errorReply(permanentFailure())], // turn 1 → permanent → halted [...textReply('{"note":"fresh start","severity":"concern"}')], // the rebuilt runtime @@ -1002,7 +1009,7 @@ describe('integration — root-llm resolution from an isolated child scope (qc1 } as never) child.provide('sessions', {} as never) child.provide('agents', { get: () => undefined } as never) - await child.plugin(advisorPlugin, fullConfig({ enabled: true, provider: 'stub', model: 'stub-model' })) + await child.plugin(advisorPlugin, fullConfig({ provider: 'stub', model: 'stub-model' })) const { agent, steer } = makeFakeAgent('s1') child.emit('agent/created', { agent, source: 'startup' }) @@ -1029,7 +1036,7 @@ describe('integration — root-llm resolution from an isolated child scope (qc1 describe('single-reviewer guard (n4 QC F-6)', () => { it('a second apply on the same process does not wire a second reviewer (one model call per round)', async () => { const first = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('{"note":"first round"}')], ) const { session, log } = makeSession() @@ -1047,7 +1054,7 @@ describe('single-reviewer guard (n4 QC F-6)', () => { // Now compose a SECOND instance of the same plugin module in the same process. const second = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('{"note":"second round"}')], ) // The guard is process-global: the second apply must not have claimed the @@ -1072,7 +1079,7 @@ describe('single-reviewer guard (n4 QC F-6)', () => { it('releases the reviewer claim when the reviewer fiber is disposed — a later instance wires (qc1 W-4)', async () => { const first = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('{"note":"first round"}')], ) const { session, log } = makeSession() @@ -1087,7 +1094,7 @@ describe('single-reviewer guard (n4 QC F-6)', () => { // A later instance in the same process can now claim the role and wire. const second = await composeHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('{"note":"second round"}')], ) expect((globalThis as Record)['__dshAdvisorReviewer__']).toBe(true) @@ -1108,7 +1115,7 @@ describe('single-reviewer guard (n4 QC F-6)', () => { const ctx = new Context() ctx.provide('sessions', {} as never) ctx.provide('agents', { get: () => undefined } as never) - expect(() => advisorPlugin.apply(ctx, { enabled: true, bogus: 1 } as never)) + expect(() => advisorPlugin.apply(ctx, { bogus: 1 } as never)) .toThrow(/unknown config key "bogus"/) expect((globalThis as Record)['__dshAdvisorReviewer__']).toBeUndefined() }) diff --git a/tests/session-model-override.test.ts b/tests/session-model-override.test.ts index 44224a4..7b0f61f 100644 --- a/tests/session-model-override.test.ts +++ b/tests/session-model-override.test.ts @@ -161,7 +161,6 @@ class HangingResolveAdapter extends GatedStubAdapter { /** Merge test config over the schema defaults (full `AdvisorConfig` shape). */ function fullConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -347,7 +346,7 @@ function flush(): Promise { describe('session model override — A/B isolation + inheritor tracking (spec §5.3)', () => { it('a pin affects only its own session; global edits reach inheritors, never the pinned session', async () => { const { ctx, adapter, entry, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('a1'), textReply('b1'), textReply('a2'), textReply('b2')], ) publishAgent(ctx, agents, 'A') @@ -376,7 +375,7 @@ describe('session model override — A/B isolation + inheritor tracking (spec § it('readback labels: /advisor config stays GLOBAL; /advisor status + /advisor model report the effective route', async () => { const { ctx, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) publishAgent(ctx, agents, 's1') @@ -412,8 +411,9 @@ describe('session model override — A/B isolation + inheritor tracking (spec § describe('session model override — explicit gate applies AFTER session resolution (spec §5.2+§5.3)', () => { it('a complete session pair satisfies a pairless-but-valid global default', async () => { const { ctx, adapter, handler, agents } = await composeOverrideHarness( - // Enabled globally WITHOUT a pair: gate-blocked (no model call anywhere). - { enabled: true }, + // Globally WITHOUT a pair: gate-blocked (no model call anywhere — no + // config-level switch since 2026-09-26, the gate keys on the pair). + {}, [textReply('unblocked')], ) publishAgent(ctx, agents, 's1') @@ -436,7 +436,7 @@ describe('session model override — explicit gate applies AFTER session resolut it('a malformed global config cannot be bypassed — the pin records but the advisor stays blocked', async () => { const { ctx, adapter, entry, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('x')], ) publishAgent(ctx, agents, 's1') @@ -473,7 +473,7 @@ describe('session model override — explicit gate applies AFTER session resolut describe('session model override — enable-switch independence (spec §5.3)', () => { it('model set never toggles enabled; off retains the pin for a later on', async () => { const { ctx, adapter, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('after on')], ) publishAgent(ctx, agents, 's1') @@ -499,7 +499,7 @@ describe('session model override — enable-switch independence (spec §5.3)', ( it('an equal-default pin records without restarting (in-flight call survives) and protects against later default edits', async () => { const { ctx, adapter, entry, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('in flight'), textReply('after edit')], ) const { inject } = publishAgent(ctx, agents, 's1') @@ -539,7 +539,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('the 60s deadline fires against a hung lookup even when the adapter ignores cancellation', async () => { const { ctx, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -568,7 +568,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('cancelling the invoking command aborts the lookup with NO retry; previous selection untouched', async () => { const { ctx, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -591,7 +591,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('a newer set supersedes unresolved older work; the final pin is the newer pair', async () => { const { ctx, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -614,7 +614,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('reset during validation supersedes the pending set and re-inherits', async () => { const { ctx, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -636,7 +636,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('a session dispose during validation cannot be undone by the delayed completion', async () => { const { ctx, agents, handler } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -660,7 +660,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { it('owner unload during validation aborts the lookup and drops the pin commit', async () => { const { ctx, fiber, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) const hanging = new HangingResolveAdapter() @@ -684,7 +684,7 @@ describe('session model override — validation fencing (spec §5.3)', () => { describe('session model override — route change effect (spec §5.3)', () => { it('aborts the old call, drops the backlog, re-seeds (no replay), and serves the next delta on the new route', async () => { const { ctx, adapter, handler, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [textReply('old route note'), textReply('new route note')], ) const { inject } = publishAgent(ctx, agents, 's1') @@ -727,7 +727,7 @@ describe('session model override — route change effect (spec §5.3)', () => { describe('session model override — lifetime (spec §5.3 + KD-5)', () => { it('agent dispose clears the pin: a cold-resumed or forked session inherits global defaults', async () => { const { ctx, agents, handler } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) publishAgent(ctx, agents, 's1') @@ -757,7 +757,7 @@ describe('session model override — lifetime (spec §5.3 + KD-5)', () => { describe('session model override — single elected owner (spec §5.3)', () => { it('a second plugin fiber stays inert: still one command registration, the owner keeps serving', async () => { const { ctx, handler, definitions, agents } = await composeOverrideHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], ) publishAgent(ctx, agents, 's1') @@ -765,7 +765,7 @@ describe('session model override — single elected owner (spec §5.3)', () => { // Compose a second fiber (the host's observed multi-fiber composition): // the single-reviewer guard makes it return before any commands wiring. - const secondEntry = new MemoryEntryConfig(fullConfig({ enabled: true, provider: 'stub', model: 'stub-model' })) + const secondEntry = new MemoryEntryConfig(fullConfig({ provider: 'stub', model: 'stub-model' })) await ctx.plugin(harnessAdvisorPlugin(), secondEntry.config) await flush() expect(definitions).toHaveLength(1) // no duplicate registration diff --git a/tests/settings-live.test.ts b/tests/settings-live.test.ts index 1f4e61e..fb7c092 100644 --- a/tests/settings-live.test.ts +++ b/tests/settings-live.test.ts @@ -11,7 +11,7 @@ * volatile references, and every edit is driven the way the Loader drives it * — write the references, then emit `loader/volatile-update` — so the full * wiring — volatile commit → bridge source → `onChange` → re-apply - * (setImmuneTurns / setMaxDeltaMessages / setConfigEnabled / dispose+ensure + * (setImmuneTurns / setMaxDeltaMessages / dispose+ensure * per-session runtimes) — is exercised end to end. `commit` writes the values * BEFORE emitting, so the synchronous re-apply has settled when the call * returns — that is the settle point for every write below. @@ -32,9 +32,10 @@ * - maxDeltaMessages = 20: one turn appending 22 messages to a live renderer * is truncated to the last 20 with the marker (the pre-edit bound of 60 * would render all 22). - * - hard gate: an entry edit that disables the advisor (and a re-enable with - * an empty provider/model pair) still blocks runtime creation — no model - * call can ever start through the live source (the resolver stays the SSOT). + * - hard gate: an entry edit that empties the provider/model pair still + * blocks runtime creation — no model call can ever start through the live + * source (the resolver stays the SSOT; there is no config-level switch + * since 2026-09-26). * * Events are synthetic but shaped exactly like dsh emits (same builders as the * T8 integration suite); `feed` mirrors the cordis `session/event` listener. @@ -162,7 +163,6 @@ interface AdapterProbe { /** Merge test config over the schema defaults (full `AdvisorConfig` shape). */ function fullConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -349,7 +349,7 @@ async function registerCommands(ctx: Context): Promise { it('re-applies immuneTurns and rebuilds the session runtime with the edited provider/model/systemPrompt', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"pre write nit","severity":"nit"}')], // Eight distinct concern replies: one steer (arms the fence) + six @@ -375,7 +375,7 @@ describe('settings live re-apply — latched config + runtime rebuild (Important // Committed volatile edit: values written, then `loader/volatile-update` // — the synchronous onChange re-apply (setImmuneTurns(7) / - // setMaxDeltaMessages(20) / setConfigEnabled(true) / dispose + ensure for + // setMaxDeltaMessages(20) / dispose + ensure for // every live session runtime) has fully run when this call returns. entry.commit(ctx, { provider: 'other', @@ -424,7 +424,7 @@ describe('settings live re-apply — latched config + runtime rebuild (Important describe('settings live re-apply — observer maxDeltaMessages (Important-1)', () => { it('applies the edited delta window to an already-live per-session renderer', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"first nit","severity":"nit"}')], [...textReply('{"note":"bulk nit","severity":"nit"}')], @@ -476,9 +476,9 @@ describe('settings live re-apply — observer maxDeltaMessages (Important-1)', ( // --------------------------------------------------------------------------- describe('settings live re-apply — hard gate through the live source (Important-1)', () => { - it('a settings edit that disables the advisor still blocks runtime creation; re-enabling without a provider/model pair is gated too', async () => { + it('a settings edit that empties the pair still blocks runtime creation (the live gate re-applies)', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"first nit","severity":"nit"}')]], ) const { agent } = makeFakeAgent('s1') @@ -489,20 +489,15 @@ describe('settings live re-apply — hard gate through the live source (Importan feed(ctx, session, log, simpleTurn(1, 'first request', 'first reply')) await vi.waitFor(() => expect(adapter.requests).toHaveLength(1)) - // Settings-page switch off: the live gate (effectiveEnabled) drops every - // delta — no model call, and the onChange rebuild loop disposes the runtime. - entry.commit(ctx, { enabled: false }) + // An edit that empties the provider/model pair: the live gate (the + // resolver, re-applied by onChange) resolves disabled-with-reason and the + // rebuild loop disposes the runtime — every later delta is dropped, no + // model call (no config-level switch since 2026-09-26; the pair IS the + // on/off state). + entry.commit(ctx, { provider: '', model: '' }) feed(ctx, session, log, simpleTurn(2, 'second request', 'second reply')) await flush() expect(adapter.requests).toHaveLength(1) - - // Even re-enabled, an empty provider/model pair trips the S4 gate through - // the live source (resolveAdvisorConfig stays the SSOT) — an edit can - // never start a gated model call. - entry.commit(ctx, { enabled: true, provider: '', model: '' }) - feed(ctx, session, log, simpleTurn(3, 'third request', 'third reply')) - await flush() - expect(adapter.requests).toHaveLength(1) }) }) @@ -518,7 +513,7 @@ describe('settings live re-apply — conditional runtime rebuild (qc3 W-1 / qc1 const note = '{"note":"same note","severity":"nit"}' const gated = new GatedAdapter([...textReply(note)]) const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [], gated, ) @@ -552,7 +547,7 @@ describe('settings live re-apply — conditional runtime rebuild (qc3 W-1 / qc1 it('a systemPrompt-only edit rebuilds the runtime: the next call carries the new prompt', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [ [...textReply('{"note":"before edit","severity":"nit"}')], [...textReply('{"note":"after edit","severity":"nit"}')], @@ -584,7 +579,7 @@ describe('settings live re-apply — conditional runtime rebuild (qc3 W-1 / qc1 describe('settings live re-apply — unknown-key user layer containment (qc2 W-1)', () => { it('an unknown key never wedges live reads: no throw, no model call, /advisor status shows disabled-with-reason', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model' }, + { provider: 'stub', model: 'stub-model' }, [[...textReply('{"note":"never delivered","severity":"nit"}')]], ) const handler = await registerCommands(ctx) @@ -614,26 +609,27 @@ describe('settings live re-apply — unknown-key user layer containment (qc2 W-1 }) // --------------------------------------------------------------------------- -// 6. Commit-time switch re-apply: a committed volatile edit re-derives the -// config-level fallback switch, so sessions created AFTER the commit pick -// it up (the old attach-ordering case rode the inject child's microtask -// activation — gone with the 0.1.7-rc.1 registration-free model). +// 6. Commit-time re-apply: a committed volatile edit completes the pair, so +// a gate-blocked session runs from the FIRST turn without a restart (the +// old attach-ordering case rode the inject child's microtask activation — +// gone with the 0.1.7-rc.1 registration-free model). No fallback switch +// anymore (removed 2026-09-26): the pair completion IS the unblock. // --------------------------------------------------------------------------- -describe('settings live re-apply — config switch follows the committed entry', () => { - it('boots with the entry switch off: after a committed enable, the FIRST turn already runs a session runtime without any further edit', async () => { +describe('settings live re-apply — a committed pair unblocks the gate', () => { + it('boots pairless (gate-blocked): after a committed provider/model pair, the FIRST turn already runs a session runtime without any further edit', async () => { const { ctx, adapter, entry } = await composeLiveHarness( - { enabled: false }, // entry switch off at load + {}, // pairless entry: gate-blocked at load [[...textReply('{"note":"boot nit","severity":"nit"}')]], ) const { agent, inject } = makeFakeAgent('s1') ctx.emit('agent/created', { agent, source: 'startup' }) const { session, log } = makeSession('s1') - // One committed enable + provider/model pair: the onChange re-apply - // flipped overrides.setConfigEnabled(true), so the first turn runs a - // session runtime and calls the model — no restart, no second commit. - entry.commit(ctx, { enabled: true, provider: 'stub', model: 'stub-model' }) + // One committed provider/model pair: the onChange re-apply re-derives the + // post-gate resolution, so the first turn runs a session runtime and calls + // the model — no restart, no second commit. + entry.commit(ctx, { provider: 'stub', model: 'stub-model' }) feed(ctx, session, log, simpleTurn(1, 'first request', 'first reply')) await vi.waitFor(() => expect(adapter.requests).toHaveLength(1)) expect(adapter.requests[0]!.provider).toBe('stub') @@ -653,7 +649,7 @@ describe('settings live re-apply — config switch follows the committed entry', describe('/advisor config — session-less composed readback (T2)', () => { it('reports the composed entry config and ignores the per-session override', async () => { const { ctx, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model', systemPrompt: 'custom prompt\nsecond line' }, + { provider: 'stub', model: 'stub-model', systemPrompt: 'custom prompt\nsecond line' }, [], ) const handler = await registerCommands(ctx) @@ -698,7 +694,6 @@ describe('/advisor config — session-less composed readback (T2)', () => { const longFirstLine = `line-one-${'x'.repeat(100)}` // 109 chars const { ctx, entry } = await composeLiveHarness( { - enabled: true, provider: 'stub', model: 'stub-model', systemPrompt: `${longFirstLine}\nsecond line must never appear`, @@ -722,7 +717,7 @@ describe('/advisor config — session-less composed readback (T2)', () => { // seed immuneTurns/maxDeltaMessages/systemPrompt from it (web-card // readConfig S1 parity) instead of the hardcoded 3/60/'' defaults. const { ctx, entry } = await composeLiveHarness( - { enabled: true, provider: 'stub', model: 'stub-model', immuneTurns: 5, maxDeltaMessages: 20, systemPrompt: 'keep me' }, + { provider: 'stub', model: 'stub-model', immuneTurns: 5, maxDeltaMessages: 20, systemPrompt: 'keep me' }, [], ) const handler = await registerCommands(ctx) diff --git a/tests/settings.test.ts b/tests/settings.test.ts index 4a6af98..d80c180 100644 --- a/tests/settings.test.ts +++ b/tests/settings.test.ts @@ -13,7 +13,7 @@ * emitted — the loader double in `support/memory-settings.ts`) is * reflected in `source()` and fires `onChange`; the event carries the * changed paths and the listener sees the ALREADY-committed values. - * ④ Hard gate regression: enabled without provider/model still resolves to + * ④ Hard gate regression: a pairless entry still resolves to * disabled-with-reason (no model call). * ⑤ Unknown config keys ride along and are still rejected by the hard gate; * the entry id stays the exact `advisor` literal. @@ -29,7 +29,6 @@ import type { AdvisorConfig } from '../src/config' /** Full entry (plugin-row) config shape, merged over the schema defaults. */ function entryConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -52,7 +51,7 @@ function emitVolatileUpdate(ctx: Context, paths: string[][]): void { describe('plain-value entry (integration harness form, behavior identical to today)', () => { it('bridge.source() is exactly the entry config', () => { const ctx = new Context() - const entry = entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 }) + const entry = entryConfig({ provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 }) const bridge = installAdvisorSettings(ctx, entry) expect(bridge.source()).toEqual(entry) // The source still passes through the hard gate — the SSOT is unchanged. @@ -82,10 +81,9 @@ describe('plain-value entry (integration harness form, behavior identical to tod describe('volatile-reference entry (source() unwraps { get() } references)', () => { it('source() returns the plain-valued config behind the references', () => { const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat', immuneTurns: 5 })) const bridge = installAdvisorSettings(ctx, entry.config) expect(bridge.source()).toEqual({ - enabled: true, provider: 'deepseek', model: 'deepseek-chat', systemPrompt: '', @@ -100,7 +98,6 @@ describe('volatile-reference entry (source() unwraps { get() } references)', () const ctx = new Context() const provider = mutableReference(undefined) const bridge = installAdvisorSettings(ctx, { - enabled: false, provider: provider, systemPrompt: '', immuneTurns: 3, @@ -128,12 +125,11 @@ describe('loader/volatile-update commit (source reflects the write, onChange fir // Loader-style commit: values are written BEFORE the event dispatches, so // the listener observes the committed state (not a pending one). - entry.commit(ctx, { enabled: true, provider: 'deepseek', model: 'deepseek-chat' }) + entry.commit(ctx, { provider: 'deepseek', model: 'deepseek-chat' }) expect(listener).toHaveBeenCalledTimes(1) expect(second).toHaveBeenCalledTimes(1) expect(bridge.source()).toEqual({ - enabled: true, provider: 'deepseek', model: 'deepseek-chat', // entry values the patch did not touch are kept @@ -177,13 +173,13 @@ describe('loader/volatile-update commit (source reflects the write, onChange fir // --------------------------------------------------------------------------- describe('hard gate regression (resolveAdvisorConfig stays the SSOT)', () => { - it('a volatile-enabled entry without provider/model still resolves to disabled-with-reason', () => { + it('a pairless volatile entry resolves to disabled-with-reason', () => { const ctx = new Context() const entry = new MemoryEntryConfig(entryConfig()) const bridge = installAdvisorSettings(ctx, entry.config) - entry.commit(ctx, { enabled: true }) - + // No config-level switch since 2026-09-26: the gate keys purely on the + // pair, so the pairless entry resolves disabled-with-reason. const resolved = resolveAdvisorConfig(bridge.source()) expect(resolved.enabled).toBe(false) expect(resolved.disabledReason).toMatch(/provider and model are missing/) @@ -191,7 +187,7 @@ describe('hard gate regression (resolveAdvisorConfig stays the SSOT)', () => { it('a committed empty provider trips the gate (no model call)', () => { const ctx = new Context() - const entry = new MemoryEntryConfig(entryConfig({ enabled: true, provider: 'deepseek', model: 'deepseek-chat' })) + const entry = new MemoryEntryConfig(entryConfig({ provider: 'deepseek', model: 'deepseek-chat' })) const bridge = installAdvisorSettings(ctx, entry.config) // The edit overrides the provider with an empty value — the gate must diff --git a/tests/tui-client.test.ts b/tests/tui-client.test.ts index 2ba9a57..7612e11 100644 --- a/tests/tui-client.test.ts +++ b/tests/tui-client.test.ts @@ -34,7 +34,6 @@ import type { AdvisorConfig } from '../src/config' /** Full plugin-row config shape (the `apply` wiring test only needs a valid entry). */ function entryConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, diff --git a/tests/tui-settings.test.ts b/tests/tui-settings.test.ts index 1ec12df..d7c03a6 100644 --- a/tests/tui-settings.test.ts +++ b/tests/tui-settings.test.ts @@ -9,16 +9,16 @@ * `ADVISOR_SETTINGS_NAMESPACE` ('advisor'); title + zh/en descriptions * are non-empty strings; the disposer returned by the inject child is * exactly the stub registry's `register` return value (no wrapping). - * ② The section's fields: the five expected kinds in display order - * (`enabled` boolean, `provider`/`model` text, - * `immuneTurns`/`maxDeltaMessages` number), each with a non-empty `path`, - * `label`, and zh/en `hint`/`hintDescriptions`; `systemPrompt` is NOT - * among the field paths. + * ② The section's fields: the four expected kinds in display order + * (`provider`/`model` text, `immuneTurns`/`maxDeltaMessages` number), + * each with a non-empty `path`, `label`, and zh/en `hint`/`hintDescriptions`; + * `systemPrompt` is NOT among the field paths. * ③ Field-path ↔ §5.1 schema alignment (regression pin): every field `path` * is a single-element array whose key is a §5.1 `AdvisorConfig` key, and - * the exact allowed set is {enabled, provider, model, immuneTurns, + * the exact allowed set is {provider, model, immuneTurns, * maxDeltaMessages} — `systemPrompt` is the only §5.1 key intentionally - * absent (single-line TUI text input would truncate a multi-line prompt). + * absent (single-line TUI text input would truncate a multi-line prompt); + * `enabled` is gone with the config-level switch (2026-09-26). * ④ No `tuiSettingsSections` service → `installTuiSettingsSection` completes * without error and registers nothing. * ⑤ A duplicate-ns registration is contained: debug log + no-op disposer, @@ -54,7 +54,6 @@ import type { AdvisorConfig } from '../src/config' /** Full plugin-row config shape (the `apply` wiring test only needs a valid entry). */ function entryConfig(overrides: Partial = {}): AdvisorConfig { return { - enabled: false, systemPrompt: '', immuneTurns: 3, maxDeltaMessages: 60, @@ -67,10 +66,9 @@ function entryConfig(overrides: Partial = {}): AdvisorConfig { * order. The `keyof AdvisorConfig` annotation is the compile-time regression * pin: a field key drifting off the schema stops typechecking; the runtime * assertions below pin the exact allowed set (`systemPrompt` intentionally - * absent). + * absent; `enabled` removed from the schema — the row toggle is the switch). */ const TUI_FIELD_KEYS: readonly (keyof AdvisorConfig)[] = [ - 'enabled', 'provider', 'model', 'immuneTurns', @@ -194,7 +192,7 @@ describe('installTuiSettingsSection — registration (AC-1)', () => { // ② + ③ the section's fields: kinds, display order, zh/en copy, schema pins // --------------------------------------------------------------------------- -describe('section fields — five §5.1 keys, display order, zh/en copy (AC-1)', () => { +describe('section fields — four §5.1 keys, display order, zh/en copy (AC-1)', () => { function registeredSection(): TuiSettingsSection { const sections = new StubSettingsSections() const { ctx } = activateCtx({ tuiSettingsSections: sections }) @@ -203,11 +201,11 @@ describe('section fields — five §5.1 keys, display order, zh/en copy (AC-1)', return sections.sections[0]! } - it('declares the five fields with the expected kinds in display order', () => { + it('declares the four fields with the expected kinds in display order', () => { const fields = registeredSection().fields expect(fields.map((field) => field.path)).toEqual(TUI_FIELD_KEYS.map((key) => [key])) - expect(fields.map((field) => field.kind)).toEqual(['boolean', 'text', 'text', 'number', 'number']) + expect(fields.map((field) => field.kind)).toEqual(['text', 'text', 'number', 'number']) }) it('every field carries a non-empty path, label, and zh/en hint + hintDescriptions; systemPrompt is absent', () => { From d21bf7dae2a17bf26361bfe2a7903428535031c5 Mon Sep 17 00:00:00 2001 From: Tang Bohao Date: Sun, 27 Sep 2026 01:08:48 +0800 Subject: [PATCH 3/9] feat: flat advisor settings card MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rebuild the advisor card to the official settings-page language: the fields tile directly under the page's plugin title/description — no collapsible box, no header button, no chevron, no dirty pill, no cardOpen disclosure state. The chrome mirrored the retired upstream PluginCard and existed to gate the form behind the config-level `enabled` switch; both are gone. - provider/model selects render unconditionally (the model select was previously hidden behind the enable checkbox); the KD-S4 gate blocks Save until the pair is complete, unconditionally. - the system-prompt textarea's placeholder IS DEFAULT_ADVISOR_SYSTEM_PROMPT (same-package import from src/prompts.ts — one SSOT; the client tsconfig widens its rootDir to src/ to include it), and the leave-empty contract moves to a hint line under the field. - the CSS module drops the chrome classes and aligns the field language with the official settings-form values (12px field padding, 0.5px hairline separators, 13px/500 labels, 34px inputs, dark solid Save, 16px footer padding), every color through a --dsw-alias-* token. - locales: title/intro move to the locale/*.json plugin meta (T1); collapse/expand/unsaved/enabled keys die with the chrome; providerRequired/modelRequired are unconditional; the namespaceUnavailable example config drops `enabled`. - package.json drops the @deepseek-ai/dsh-client-ui-primitives peer (the chevron icon value-import was the last one; build-client CLIENT_EXTERNALS stays as the frozen platform-table mirror). - client-build test pins move from the chrome classes to the flat ones, add not-to-contain pins for the primitives require and the inlined reviewer prompt; the card spec asserts the flat layout (no checkbox, always-rendered selects, prompt placeholder, unchanged disabled semantics). --- package.json | 1 - src/client/advisor-card.module.css | 389 ++++++++-------------- src/client/advisor-card.tsx | 506 +++++++++++++---------------- src/client/locales.ts | 35 +- tests/advisor-card.spec.tsx | 373 +++++++-------------- tests/client-build.test.ts | 34 +- tsconfig.client.json | 12 +- 7 files changed, 519 insertions(+), 831 deletions(-) diff --git a/package.json b/package.json index 0897eee..8850348 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,6 @@ "@deepseek-ai/dsh-client-ui-conversation": "^0.1.7-rc.2", "@deepseek-ai/dsh-client-store": "^0.1.7-rc.2", "@deepseek-ai/dsh-client-ui-plugin-manager": "^0.1.7-rc.2", - "@deepseek-ai/dsh-client-ui-primitives": "^0.1.7-rc.2", "@deepseek-ai/dsh-client-ui-renderer": "^0.1.7-rc.2", "@deepseek-ai/dsh-client-ui-settings": "^0.1.7-rc.2", "@deepseek-ai/dsh-client-ui-slots": "^0.1.7-rc.2", diff --git a/src/client/advisor-card.module.css b/src/client/advisor-card.module.css index a5871f9..cf29f4e 100644 --- a/src/client/advisor-card.module.css +++ b/src/client/advisor-card.module.css @@ -1,281 +1,85 @@ -/* Advisor settings card — chrome aligned to the upstream PluginCard - * (packages/client/ui-plugin-config/src/client/PluginCard.module.css, shape - * snapshot 5d49d016), form-field styles kept from the card-form design - * language. Every color resolves through a `--dsw-alias-*` token (light/dark - * adaptive); bare `--border` / `--surface` / `--text-*` names, which nothing - * in this app defines, would render the light-mode literals written as their - * fallbacks and stay light under the dark theme. - * - * The card is the whole body of its bundle's configuration section on the - * Plugins page (`plugins.bundle.config`; the owner owns the column width); the - * chrome is the upstream contract: a bordered box whose header is a button - * (name over description, dirty pill, rotating chevron) and whose open state - * shows a divided body — divider, readOnly / form content, footer divider with - * the failed message + Discard/Save. */ - -/* The card box. */ -.card { - border: 1px solid var(--dsw-alias-border-l2); - border-radius: 12px; - background: var(--dsw-alias-bg-layer-3); - transition: border-color .16s, background .16s; -} - -.card:hover { - border-color: var(--dsw-alias-label-dimmed); -} - -/* An open card reads as the one being worked on, not merely taller. */ -.cardOpen { - background: var(--dsw-alias-bg-layer-2); - border-color: var(--dsw-alias-label-dimmed); -} - -/* The disclosure header: a full-width button stacking name over description. */ -.header { - width: 100%; - appearance: none; - border: 0; - background: none; - font: inherit; - color: inherit; - text-align: left; - cursor: pointer; - display: flex; - align-items: center; - gap: 12px; - padding: 14px 16px; - border-radius: 12px; -} - -.header:focus-visible { - outline: 2px solid var(--dsw-alias-brand-primary); - outline-offset: -2px; -} - -/* Name over description: the description is what tells two plugins apart, so - it gets its own line rather than trailing the name. */ -.headText { - flex: 1; - min-width: 0; - display: flex; - flex-direction: column; - gap: 4px; -} - -.name { - font-size: 15px; - font-weight: 600; - line-height: 1.4; - color: var(--dsw-alias-label-primary); -} - -.description { - font-size: 13px; - line-height: 1.5; - color: var(--dsw-alias-label-tertiary); -} - -.chevron { - flex: none; - color: var(--dsw-alias-label-tertiary); - transition: transform .16s; -} - -.chevronOpen { - transform: rotate(180deg); -} - -/* The open body: a divider under the header, then the card content. */ -.body { - border-top: 1px solid var(--dsw-alias-border-l2); - margin: 0 16px; - padding-bottom: 8px; -} - +/* Advisor settings card — flat alignment to the official settings-form + * language (ui-primitives settings-form: fields.module.css + + * SettingsForm.module.css, values snapshot 2026-09): the fields tile directly + * under the page's plugin title/description — NO collapsible box, no header + * chrome (the former PluginCard-mirror chrome died with the config-level + * `enabled` switch, plan dsh-advisor-web-config-flat-n10). Every color + * resolves through a `--dsw-alias-*` token (light/dark adaptive); bare + * `--border` / `--surface` / `--text-*` names, which nothing in this app + * defines, would render the light-mode literals written as their fallbacks + * and stay light under the dark theme. */ + +/* Body-level notices: tertiary for read-only, warn-toned for the + config-channel notice, success for the saved feedback, error tone for the + load error. Flat and always-on — there is no derived-open surface left to + hide them behind. */ .readOnly { - margin: 12px 0 0; + margin: 0 0 12px; font-size: 12px; line-height: 1.5; color: var(--dsw-alias-label-tertiary); } -/* Carried on the header so a collapsed card still says it holds edits. */ -.pending { - flex: none; - border-radius: 999px; - padding: 1px 8px; - font-size: 11px; - line-height: 17px; - font-weight: 500; - white-space: nowrap; - background: var(--dsw-alias-bg-module-platform); - color: var(--dsw-alias-label-secondary); -} - -.footer { - display: flex; - align-items: center; - justify-content: flex-end; - gap: 8px; - padding: 12px 0 4px; - border-top: 1px solid var(--dsw-alias-border-l2); -} - -.failed { - flex: 1; - min-width: 0; - margin: 0; - font-size: 12px; - line-height: 1.5; - color: var(--dsw-alias-label-error); -} - -.discard, -.save { - appearance: none; - border: 1px solid transparent; - border-radius: 8px; - padding: 5px 14px; - font: inherit; - font-size: 13px; - line-height: 1.5; - cursor: pointer; -} - -.discard { - border-color: var(--dsw-alias-border-l2); - background: none; - color: var(--dsw-alias-label-secondary); -} - -.discard:hover:not(:disabled) { - color: var(--dsw-alias-label-primary); - border-color: var(--dsw-alias-label-dimmed); -} - -.save { - background: var(--dsw-alias-label-primary); - color: var(--dsw-alias-bg-layer-3); -} - -.discard:disabled, -.save:disabled { - opacity: 0.4; - cursor: default; -} - -.discard:focus-visible, -.save:focus-visible { - outline: 2px solid var(--dsw-alias-brand-primary); - outline-offset: 1px; -} - -/* Body-level notices (KD-U3: degraded/error states keep the card chrome and - put the notice/error in the body): warn-toned for the config-channel - notice, success for the saved feedback, error tone for the load error. */ .notice { - margin: 12px 0 0; + margin: 0 0 12px; font-size: 12px; - line-height: 18px; + line-height: 1.5; color: var(--dsw-alias-state-warn-label); } .savedNotice { - margin: 12px 0 0; + margin: 0 0 12px; font-size: 12px; - line-height: 18px; + line-height: 1.5; color: var(--dsw-alias-state-success-primary); } .error { - margin: 12px 0 0; + margin: 0 0 12px; font-size: 12px; - line-height: 18px; + line-height: 1.5; color: var(--dsw-alias-state-error-primary); } -/* The form fields sit directly in the card body (the upstream cards stack - their controls in the body); the container only paces them below the - divider. */ +/* The fields tile directly; the container only stacks them (the official + form carries no extra padding — each field paces itself). */ .form { display: flex; flex-direction: column; - gap: 12px; - padding: 12px 0 0; -} - -/* The enabled switch: a real checkbox, tinted with the brand accent so it - matches the rest of the panel while staying a native control. */ -.checkboxRow { - display: flex; - align-items: center; - gap: 8px; -} - -.checkLabel { - font-size: 14px; - line-height: 22px; - font-weight: 500; - color: var(--dsw-alias-label-primary); -} - -.checkbox { - width: 16px; - height: 16px; - margin: 0; - accent-color: var(--dsw-alias-brand-primary); - cursor: pointer; -} - -.checkbox:disabled { - opacity: 0.4; - cursor: default; -} - -.checkbox:focus-visible { - outline: 2px solid var(--dsw-alias-border-l3); - outline-offset: 2px; -} - -/* The provider/model group is a fieldset (no legend: the enabled toggle - above it is the group's question). */ -.fieldset { - margin: 0; - padding: 0; - border: none; - display: flex; - flex-direction: column; - gap: 12px; } +/* One settings field: 12px vertical padding, hairline separator between + adjacent fields (official fields.module.css). Scoped to direct children so + the paired number fields inside .numberFields stay separator-free. */ .field { display: flex; flex-direction: column; gap: 6px; + padding: 12px 0; +} + +.form > .field + .field { + border-top: 0.5px solid var(--dsw-alias-border-l2); } .fieldLabel { - display: inline-flex; - align-items: center; - gap: 10px; - font-size: 12px; - line-height: 18px; + font-size: 13px; font-weight: 500; - color: var(--dsw-alias-label-secondary); + line-height: 1.5; + color: var(--dsw-alias-label-primary); } .input { box-sizing: border-box; width: 100%; - height: 32px; - padding: 0 10px; - border: 1px solid var(--dsw-alias-border-l2); - border-radius: 8px; + height: 34px; + padding: 0 12px; + border: 0.5px solid var(--dsw-alias-border-l4); + border-radius: var(--dsw-radius-md); font: inherit; - font-size: 14px; - line-height: 22px; - background: var(--dsw-alias-bg-layer-1); + font-size: 13px; + line-height: 1.5; + background: var(--dsw-alias-bg-layer-3); color: var(--dsw-alias-label-primary); } @@ -286,9 +90,9 @@ select.input { cursor: pointer; } -.input:focus { +.input:focus-visible { outline: none; - border-color: var(--dsw-alias-brand-primary); + border-color: var(--dsw-alias-state-business-primary); } .input::placeholder { @@ -296,7 +100,7 @@ select.input { } .input:disabled { - opacity: 0.6; + color: var(--dsw-alias-label-tertiary); cursor: default; } @@ -315,52 +119,125 @@ select.input { } /* The system prompt: same field chrome as .input, but a multiline surface — - taller by default, vertically resizable. */ + taller by default, vertically resizable. The placeholder is the built-in + reviewer prompt, so the dimmed placeholder color keeps it readable as + ghost text rather than content. */ .textarea { box-sizing: border-box; width: 100%; min-height: 96px; - padding: 8px 10px; - border: 1px solid var(--dsw-alias-border-l2); - border-radius: 8px; + padding: 8px 12px; + border: 0.5px solid var(--dsw-alias-border-l4); + border-radius: var(--dsw-radius-md); font: inherit; - font-size: 14px; - line-height: 22px; - background: var(--dsw-alias-bg-layer-1); + font-size: 13px; + line-height: 1.5; + background: var(--dsw-alias-bg-layer-3); color: var(--dsw-alias-label-primary); resize: vertical; } -.textarea:focus { +.textarea:focus-visible { outline: none; - border-color: var(--dsw-alias-brand-primary); + border-color: var(--dsw-alias-state-business-primary); +} + +.textarea::placeholder { + color: var(--dsw-alias-label-dimmed); } .textarea:disabled { - opacity: 0.6; + color: var(--dsw-alias-label-tertiary); cursor: default; } /* The two short numeric fields (immune turns / max delta messages) sit side - by side, each keeping a full-width field of its own column. */ + by side, each keeping a full-width field of its own column; the group + closes the field run, so it carries the hairline separator itself. */ .numberFields { display: grid; grid-template-columns: repeat(auto-fit, minmax(160px, 1fr)); gap: 12px; + padding: 12px 0; + border-top: 0.5px solid var(--dsw-alias-border-l2); } -/* Inline guidance under a field: tertiary for stale-value / no-option hints, - warn for the required-when-enabled gate. */ +.numberFields .field { + padding: 0; +} + +/* Inline guidance under a field: tertiary for hints, warn for the required + pair gate (official 12px hint). */ .hint { margin: 0; font-size: 12px; - line-height: 18px; + line-height: 1.5; color: var(--dsw-alias-label-tertiary); } .warnHint { margin: 0; font-size: 12px; - line-height: 18px; + line-height: 1.5; color: var(--dsw-alias-state-warn-label); } + +/* Footer: the failed message flexes wide, then the action pair (official + SettingsForm.module.css — 16px top padding, no divider: the field padding + above provides the air). */ +.footer { + display: flex; + align-items: center; + gap: 8px; + padding-top: 16px; +} + +.failed { + flex: 1; + min-width: 0; + margin: 0; + font-size: 12px; + line-height: 1.5; + color: var(--dsw-alias-label-error); +} + +.discard, +.save { + appearance: none; + border: 1px solid transparent; + border-radius: var(--dsw-radius-md); + padding: 5px 14px; + font: inherit; + font-size: 13px; + line-height: 1.5; + cursor: pointer; +} + +.discard { + border-color: var(--dsw-alias-border-l2); + background: none; + color: var(--dsw-alias-label-secondary); +} + +.discard:hover:not(:disabled) { + color: var(--dsw-alias-label-primary); + border-color: var(--dsw-alias-label-dimmed); +} + +/* Save is the dark solid primary action (official SettingsForm). */ +.save { + background: var(--dsw-alias-label-primary); + color: var(--dsw-alias-bg-layer-3); +} + +.discard:disabled, +.save:disabled { + opacity: 0.4; + cursor: default; +} + +.discard:focus-visible, +.save:focus-visible { + outline: 2px solid var(--dsw-alias-brand-primary); + outline-offset: 1px; +} diff --git a/src/client/advisor-card.tsx b/src/client/advisor-card.tsx index 0f551d4..95fc87a 100644 --- a/src/client/advisor-card.tsx +++ b/src/client/advisor-card.tsx @@ -1,40 +1,44 @@ /** - * Advisor settings card (plan dsh-advisor-plugin-config-card-ux, task 1): the - * card registered into the Plugins page's `plugins.bundle.config` keyed slot - * (key `dsh-advisor` — the bundle's package name the page dispatches). It keeps - * the n5 gateway channel — the store - * reads/writes the advisor config through `/api/advisor/get` + - * `/api/advisor/set` (KD-G3) — while the card chrome is rebuilt to replicate - * the upstream `PluginCard` contract (self-drawn: the upstream client value - * face exports no reusable card). The chrome: a collapsible box whose header - * is a button stacking the plugin name over its description, with a dirty - * "unsaved" pill and a rotating chevron (`IconChevronDownOutlineRegular` from - * ui-primitives — 0.1.7-rc.1 moved the rendered size out of the icon name - * into the `size` prop; the chevron's drawn size is still 14), - * `aria-expanded`/`aria-label` like the upstream header; a - * divider under the header; then the form content; then a footer with the - * failed message + Discard/Save carrying the upstream disabled semantics — - * save = `!dirty || invalid || saving`, discard = `!dirty || saving` (KD-U1, - * Global Constraints). Save additionally carries `!writable` and the store - * refuses writes outright in read-only environments (W-1, qc2 fix wave) — - * see the disabled-term comment in the ready branch. Disclosure is - * card-local state: which card a user has open is a reading gesture, and - * staged edits outlive collapsing — the pill rides the header (upstream - * rationale). + * Advisor settings card (plan dsh-advisor-plugin-config-card-ux, task 1; + * flat rebuild 2026-09-26 — plan dsh-advisor-web-config-flat-n10): the card + * registered into the Plugins page's `plugins.bundle.config` keyed slot (key + * `dsh-advisor` — the bundle's package name the page dispatches). It keeps + * the n5 gateway channel — the store reads/writes the advisor config through + * `/api/advisor/get` + `/api/advisor/set` (KD-G3) — while the layout is the + * official settings-page language: NO collapsible box. The page already + * renders the plugin title/description above the card (the `locale/*.json` + * meta files the host resolves per UI language), so the fields tile directly + * beneath it: provider select, model select (ALWAYS rendered — there is no + * enable checkbox to gate them; the config-level `enabled` switch was removed + * and the plugin-row toggle is the master switch), system-prompt textarea, + * and the paired `immuneTurns`/`maxDeltaMessages` numbers; then the footer + * with the failed message + Discard/Save carrying the upstream disabled + * semantics — save = `!dirty || invalid || saving`, discard = `!dirty || + * saving` (KD-U1, Global Constraints). Save additionally carries `!writable` + * and the store refuses writes outright in read-only environments (W-1, qc2 + * fix wave) — see the disabled-term comment in the ready branch. The former + * collapsible chrome (header button, rotating chevron, dirty "unsaved" pill, + * card-local disclosure state) is gone with the switch it mirrored: nothing + * is hidden, so nothing needs disclosure state, and the readOnly / saved / + * error / namespaceUnavailable notices are flat and always-on (the derived- + * open semantics they used to ride — AC-1/AC-3 — have no surface left). * - * The full form is unchanged from the card-form plan: `enabled` switch - * (default off), `provider`/`model` select boxes limited to the - * system-configured providers and their models (KD-S2), the - * required-when-enabled gate (KD-S4, also enforced in the store), the - * `systemPrompt` textarea, the `immuneTurns`/`maxDeltaMessages` number - * inputs, and Save writing the advisor config through the gateway channel - * (store → `connection.rpc.call('/api', 'advisor/set', { patch })`). Discard - * rewinds the draft to the last-known host config (client-side only — no - * gateway write). + * The form behavior is unchanged from the card-form plan: provider/model + * selects limited to the system-configured providers and their models + * (KD-S2), the required-pair gate (KD-S4, also enforced in the store), and + * Save writing the advisor config through the gateway channel (store → + * `connection.rpc.call('/api', 'advisor/set', { patch })`). Discard rewinds + * the draft to the last-known host config (client-side only — no gateway + * write). The textarea's placeholder IS the built-in reviewer prompt + * (`DEFAULT_ADVISOR_SYSTEM_PROMPT` — same-package import, a pure string + * constant the bundler inlines; SSOT stays `src/prompts.ts`), so the field + * shows exactly what an empty prompt inherits; the "leave empty" hint rides + * below it. * - * Presentation follows `PluginCard.module.css` shape via - * `advisor-card.module.css` — every color resolves through a `--dsw-alias-*` - * token so the card adapts to the light/dark theme. + * Presentation follows the official settings-form values via + * `advisor-card.module.css` (12px field padding, 0.5px hairline separators, + * 34px inputs, dark solid Save) — every color resolves through a + * `--dsw-alias-*` token so the card adapts to the light/dark theme. * * A stored provider/model that is no longer among the current options * surfaces warning copy (`staleProvider`/`staleModel`) instead of blocking @@ -43,17 +47,11 @@ * Clearing a number input leaves the field empty; the store then omits that * key from the apply patch (the stored value stays unchanged). * - * Degraded/error/loading states keep the same card chrome (KD-U3): the - * header always renders title+description+chevron, and the body carries the - * config-channel notice or the load error + retry (AC-3 — the documented - * divergence from upstream, whose unavailable card renders nothing). A card - * that cannot render its form keeps the notice/error body ALWAYS visible - * (derived open — the header cannot collapse it away), while a healthy card - * is collapsed until the user expands it (AC-1). The notice also stays - * visible through a background refresh of a degraded card: while - * `status === 'loading'` the open derivation falls back to the store's - * latched `degraded` (qc1 S-2 fix wave), so the refresh window never - * collapses the AC-3 notice. + * Degraded/error states keep the flat layout (KD-U3): the config-channel + * notice or the load error + retry render as always-on blocks — a card that + * cannot render its form shows that state on every render, including through + * a background refresh of a degraded card (the store's latched `degraded` + * keeps the notice up while `status === 'loading'`, qc1 S-2 fix wave). * When the last load could not reach the `advisor.get` gateway endpoint (the * gateway channel is down or not ready on this host), the form is replaced * by the `namespaceUnavailable` notice and Save is never offered, so the @@ -64,11 +62,11 @@ * unreachability, not the no-settings-service case. */ -import { useState, type ReactNode } from 'react' -import { IconChevronDownOutlineRegular } from '@deepseek-ai/dsh-client-ui-primitives' +import type { ReactNode } from 'react' import type { InjectFace, PropsLocale, PropsRuntime } from '@deepseek-ai/dsh-client-ui-slots' import type { SnapshotStore } from '@deepseek-ai/dsh-client-store' import type { ApplyFailure, AdvisorSettingsState, AdvisorSettingsStore } from './advisor-store.ts' +import { DEFAULT_ADVISOR_SYSTEM_PROMPT } from '../prompts.ts' import styles from './advisor-card.module.css' /** Injected dependencies of {@link AdvisorCard} (slot `inject`). The `hooks` @@ -86,10 +84,9 @@ export interface AdvisorCardInjected { /** * Props the renderer binds for the card: the `plugins.bundle.config` runtime * share (the owner passes the `view` the page asks for — this seat is - * `page`-only, so the self-chromed card needs no branch for it), the - * framework-synthesized `t` seat for the declared `settings.advisor` namespace - * (KD-1 — `t` is NOT part of the inject face), and the registrant's business - * face. + * `page`-only), the framework-synthesized `t` seat for the declared + * `settings.advisor` namespace (KD-1 — `t` is NOT part of the inject face), + * and the registrant's business face. */ export type AdvisorCardProps = PropsRuntime<'plugins.bundle.config'> @@ -106,34 +103,14 @@ function failureCopy(failure: ApplyFailure, t: AdvisorCardProps['t']): string | /** * Render the advisor card inside its bundle's configuration section on the - * Plugins page, replicating the upstream PluginCard chrome (KD-U1): a - * collapsible block with a header button (name over description, dirty pill, - * rotating chevron, aria) and, when open, a divided body holding the readOnly - * notice, the form, and the footer (failed message + Discard/Save). + * Plugins page — flat: the notices, the form, and the footer tile directly + * under the page's plugin title/description, with no chrome of our own. * @param props - slot-delivered injected dependencies and the synthesized t seat. * @returns the card. */ export function AdvisorCard(props: AdvisorCardProps): ReactNode { const { controller, useSnapshot, t } = props const state = useSnapshot(snapshot => snapshot) - // Disclosure is card-local USER state (upstream rationale): the healthy - // card starts collapsed and opens on the header click only. The degraded - // (advisorPresent=false) and error cards render their notice/error body - // ALWAYS visible (AC-3 — the notice must appear without interaction), so - // `open` is DERIVED from the current snapshot — never from a mount-time - // snapshot read and never through a useEffect (I-1, T1 fix wave): the - // mount-time snapshot is the store default ('idle', advisorPresent=false), - // so a mount-time read would wrongly start the healthy card open. - const [userOpen, setUserOpen] = useState(false) - // `degraded` is the derived notion (qc1 S-2): while ready it IS - // `!advisorPresent`; while loading/error it falls back to the store's - // LATCHED last-settled degraded state. A background refresh flips status - // to 'loading' while advisorPresent keeps its stale value, so the latch is - // what keeps the AC-3 notice visible through the refresh window — and it - // is false on a first mount, so the healthy card still starts (and stays) - // collapsed through its first load. - const degraded = state.status === 'ready' ? !state.advisorPresent : state.degraded - const open = userOpen || state.status === 'error' || degraded // Load-on-mount (KD-3): the Plugins page mounts the card lazily when the // user opens the bundle's page, so the first mount triggers the first @@ -148,252 +125,213 @@ export function AdvisorCard(props: AdvisorCardProps): ReactNode { // remount, changing the load-once semantics. if (state.status === 'idle') void controller.load() - const title = t('title') - const header = ( - - ) - - let body: ReactNode if (state.status === 'error') { // A post-apply reload failure must not mask a landed write: the saved // feedback renders alongside the error + retry. - body = ( -
+ return ( + <> {state.applyState.kind === 'saved' ?

{t('saved')}

: null}

{`${t('loadFailed')}: ${state.error ?? ''}`}

{/* Retry reuses the `.discard` (secondary/outline) button look — the - module's only secondary-button style, mirroring upstream - PluginCard's single secondary action; intentional reuse (qc1 + module's only secondary-button style; intentional reuse (qc1 N-2). */}
-
+ ) - } else if (degraded) { + } + // `degraded` is the derived notion (qc1 S-2): while ready it IS + // `!advisorPresent`; while loading it falls back to the store's LATCHED + // last-settled degraded state. A background refresh flips status to + // 'loading' while advisorPresent keeps its stale value, so the latch is + // what keeps the config-channel notice visible through the refresh window — + // and it is false on a first mount, so a healthy first load renders + // nothing yet. + const degraded = state.status === 'ready' ? !state.advisorPresent : state.degraded + if (degraded) { // KD-G5 (the n2-era C-1 mitigation): when the last load could not reach // the `advisor.get` gateway endpoint (gateway not ready / channel down), // the form would present defaults + a writable-looking Save that can only - // fail with a host refusal — render the explicit notice in the card body - // instead and never offer Save. The branch is the DERIVED `degraded` (not - // the raw `!advisorPresent`) so it also covers a refresh in flight: while - // `status === 'loading'` on a degraded card the latch keeps the AC-3 - // notice visible through the refresh window (qc1 S-2). qc3 N-1 mirrors - // here too: a post-apply reload that loses the gateway must not mask a - // landed write — the saved line renders alongside the notice. - body = ( -
+ // fail with a host refusal — render the explicit notice instead and never + // offer Save. qc3 N-1 mirrors the error branch here too: a post-apply + // reload that loses the gateway must not mask a landed write — the saved + // line renders alongside the notice. + return ( + <> {state.applyState.kind === 'saved' ?

{t('saved')}

: null}

{t('namespaceUnavailable')}

{/* Retry reuses the `.discard` (secondary/outline) button look — the - module's only secondary-button style, mirroring upstream - PluginCard's single secondary action; intentional reuse (qc1 + module's only secondary-button style; intentional reuse (qc1 N-2). */}
-
+ ) - } else if (state.status === 'ready') { - const { draft, providers, writable, applyState } = state - const providerEmpty = draft.provider === undefined - const modelEmpty = draft.model === undefined - // KD-S4: enabled + missing provider/model blocks Save and shows the hints. - const gateFailed = draft.enabled && (providerEmpty || modelEmpty) - const saving = applyState.kind === 'saving' - const busy = !writable || saving - const selectedModels = draft.provider === undefined - ? [] - : state.modelsByProvider[draft.provider] ?? [] - const modelsEmpty = draft.provider !== undefined && Object.hasOwn(state.modelsEmptyReason, draft.provider) - // Stored values that are no longer among the current options: warn instead - // of silently dropping them; Save stays enabled once the draft is dirty - // (keep or reselect). - const providerStale = draft.provider !== undefined - && !providers.some(option => option.provider === draft.provider) - const modelStale = !providerStale && draft.model !== undefined - && selectedModels.length > 0 - && !selectedModels.some(option => option.id === draft.model) - const errorText = applyState.kind === 'error' ? failureCopy(applyState.failure, t) : undefined - // Upstream disabled semantics (Global Constraints): save = !dirty || - // invalid || saving; discard = !dirty || saving. In a read-only - // environment the fields are disabled, so the draft cannot become dirty - // and both actions stay disabled through the !dirty term. W-1 (qc2 fix - // wave): the dirty-implies-writable assumption does NOT hold for this - // in-place-draft store — a mid-session invalidation refresh can return - // writable=false while staged edits survive (dirty stays true), so Save - // additionally carries `!writable` (restoring the pre-plan Apply, which - // was always disabled when !writable). - const saveDisabled = !state.dirty || gateFailed || saving || !writable - // Discard KEEPS `!dirty || saving` BY DESIGN: it is a pure client-side - // revert to the last-known seed (no gateway write) — disabling it in - // read-only would strand staged edits the user cannot clear, and the - // store-side writable guard (advisor-store.ts apply()) makes a read-only - // write fail cleanly even if it were reached. - const discardDisabled = !state.dirty || saving - body = ( -
- {!writable ?

{t('readOnly')}

: null} - {applyState.kind === 'saved' ?

{t('saved')}

: null} -
-
- + } + if (state.status !== 'ready') { + // Loading (or the idle→loading transition): nothing yet — the flat card + // has no header chrome to hold the space (the former empty body died + // with the chrome). + return null + } + const { draft, providers, writable, applyState } = state + const providerEmpty = draft.provider === undefined + const modelEmpty = draft.model === undefined + // KD-S4: a missing provider/model blocks Save and shows the hints. The + // gate is UNCONDITIONAL now — the config-level `enabled` switch that used + // to make it conditional is gone (the row toggle is the switch), so the + // form can only save a complete pair. + const gateFailed = providerEmpty || modelEmpty + const saving = applyState.kind === 'saving' + const busy = !writable || saving + const selectedModels = draft.provider === undefined + ? [] + : state.modelsByProvider[draft.provider] ?? [] + const modelsEmpty = draft.provider !== undefined && Object.hasOwn(state.modelsEmptyReason, draft.provider) + // Stored values that are no longer among the current options: warn instead + // of silently dropping them; Save stays enabled once the draft is dirty + // (keep or reselect). + const providerStale = draft.provider !== undefined + && !providers.some(option => option.provider === draft.provider) + const modelStale = !providerStale && draft.model !== undefined + && selectedModels.length > 0 + && !selectedModels.some(option => option.id === draft.model) + const errorText = applyState.kind === 'error' ? failureCopy(applyState.failure, t) : undefined + // Upstream disabled semantics (Global Constraints): save = !dirty || + // invalid || saving; discard = !dirty || saving. In a read-only + // environment the fields are disabled, so the draft cannot become dirty + // and both actions stay disabled through the !dirty term. W-1 (qc2 fix + // wave): the dirty-implies-writable assumption does NOT hold for this + // in-place-draft store — a mid-session invalidation refresh can return + // writable=false while staged edits survive (dirty stays true), so Save + // additionally carries `!writable` (restoring the pre-plan Apply, which + // was always disabled when !writable). + const saveDisabled = !state.dirty || gateFailed || saving || !writable + // Discard KEEPS `!dirty || saving` BY DESIGN: it is a pure client-side + // revert to the last-known seed (no gateway write) — disabling it in + // read-only would strand staged edits the user cannot clear, and the + // store-side writable guard (advisor-store.ts apply()) makes a read-only + // write fail cleanly even if it were reached. + const discardDisabled = !state.dirty || saving + return ( + <> + {!writable ?

{t('readOnly')}

: null} + {applyState.kind === 'saved' ?

{t('saved')}

: null} +
+
+ + + {providers.length === 0 ?

{t('noProviders')}

: null} + {providerEmpty ?

{t('providerRequired')}

: null} + {providerStale ?

{t('staleProvider')}

: null} +
+
+ + + {modelsEmpty ?

{t('noModels')}

: null} + {modelStale ?

{t('staleModel')}

: null} + {!providerEmpty && modelEmpty ?

{t('modelRequired')}

: null} +
+
+ +