diff --git a/extensions/gentle-ai.ts b/extensions/gentle-ai.ts index ef869ff57..0aaf1b288 100644 --- a/extensions/gentle-ai.ts +++ b/extensions/gentle-ai.ts @@ -2122,7 +2122,21 @@ function readGlobalEffectiveModelConfig(cwd: string): AgentModelConfig { async function readEffectiveModelConfigAsync(cwd: string): Promise { const pinned = pinnedEffectiveModelConfig(cwd); if (pinned) return pinned; - const effective = cloneModelConfig(await readModelConfigAsync(cwd)); + return readGlobalEffectiveModelConfigFromAsync(cwd, await readModelConfigAsync(cwd)); +} + +/** + * The saved global routing merged with the materialized stores of every + * discoverable agent it is silent about — the same effective view + * `readEffectiveModelConfigAsync` builds, but starting from an already-read + * saved routing so callers that must distinguish an unreadable authority can + * keep that distinction while still seeing materialized routes. + */ +async function readGlobalEffectiveModelConfigFromAsync( + cwd: string, + base: AgentModelConfig, +): Promise { + const effective = cloneModelConfig(base); const profilesByPath = new Map>(); for (const agent of await listDiscoverableAgentsAsync(cwd)) { if (isProviderReviewRole(agent.name) || agent.name in effective) continue; @@ -4148,6 +4162,89 @@ async function runProfilesPanelAction( } const normalized = normalizeModelConfig(file.profiles[result.name]) ?? {}; const orchestratorEntry = readProfileOrchestrator(normalized); + const hasAgentRoutes = Object.keys(normalized).some((name) => !isProfileOrchestratorKey(name)); + // Every global apply confirms before anything is written: applying replaces + // the whole routing map in models.json and clears materialized routes the + // profile omits, so the dialog must name the concrete diff. The current + // routing is read from the same authority every other consumer uses; an + // unreadable config is tolerated as empty, exactly like readModelConfigAsync. + const savedRouting = await readModelRoutingAuthorityAsync( + modelConfigPath(ctx.cwd), + legacyProjectModelConfigPath(ctx.cwd), + ); + // The apply pads omitted discoverable agents with clear entries, so the + // diff must run against the effective current routing: the saved global + // routing plus the materialized routes (frontmatter, subagents.json) of + // agents the saved routing is silent about. models.json alone would hide + // materialized-only routes the approval actually clears. + const currentRouting = savedRouting.status === "valid" + ? await readGlobalEffectiveModelConfigFromAsync(ctx.cwd, savedRouting.config) + : {}; + const agentNames = [ + ...new Set([ + ...Object.keys(currentRouting).filter((name) => !isProfileOrchestratorKey(name)), + ...Object.keys(normalized).filter((name) => !isProfileOrchestratorKey(name)), + ]), + ].sort(); + const replacedRoutes: string[] = []; + const clearedRoutes: string[] = []; + const addedRoutes: string[] = []; + for (const name of agentNames) { + const from = currentRouting[name]; + const to = normalized[name]; + if (to === undefined) { + clearedRoutes.push(`${name}: ${formatOrchestratorSelection(from)} → inherit (cleared)`); + } else if (from === undefined) { + addedRoutes.push(`${name}: ${formatOrchestratorSelection(to)} (added)`); + } else if (from.model !== to.model || from.thinking !== to.thinking) { + replacedRoutes.push(`${name}: ${formatOrchestratorSelection(from)} → ${formatOrchestratorSelection(to)}`); + } + } + if (!hasAgentRoutes) { + // A genuinely empty profile and an orchestrator-only profile both wipe + // every materialized agent route, but they read very differently to the + // user: the orchestrator entry survives the apply and reconfigures the + // orchestrator, so the dialog must not promise an entirely empty config. + const [confirmTitle, confirmMessage] = orchestratorEntry !== undefined + ? [ + "Apply orchestrator-only profile?", + `Profile "${result.name}" has an orchestrator entry but no agent routing entries. Applying it will replace global routing in ${sanitizeTerminalText(modelConfigPath(ctx.cwd))} with the orchestrator entry alone, clear every materialized agent route so every agent returns to inherit its default model, set the configured orchestrator entry in settings.json, and attempt to switch this session to that orchestrator model. Continue?`, + ] + : [ + "Apply empty profile?", + `Profile "${result.name}" has no routing entries. Applying it will replace global routing in ${sanitizeTerminalText(modelConfigPath(ctx.cwd))} with an empty configuration and return every agent to inherit its default model. Continue?`, + ]; + const approved = await ctx.ui.confirm(confirmTitle, confirmMessage); + if (!approved) return file; + } else { + // A populated profile keeps the same abort semantics as the empty and + // orchestrator-only dialogs: declining leaves every surface untouched. + // When the routing authority is unreadable the diff above was computed + // against an empty map, so the dialog must disclose the unreadable + // routing and the replace/clear-to-inherit effect instead of presenting + // existing routes as merely "(added)" (the #1349 wipe-bug class). + // When the profile carries an orchestrator entry, approval also writes + // settings.json and tries to move the live session, so both variants + // must disclose those effects in the same words the orchestrator-only + // dialog uses. An unconditional "will switch" is never claimed: the + // registry/auth can refuse the live move. + const orchestratorEffects = orchestratorEntry !== undefined + ? ", set the configured orchestrator entry in settings.json, and attempt to switch this session to that orchestrator model" + : ""; + let confirmMessage: string; + if (savedRouting.status !== "valid") { + const modelsPath = sanitizeTerminalText(modelConfigPath(ctx.cwd)); + confirmMessage = `Profile "${result.name}" has agent routing entries, but the current global routing in ${modelsPath} could not be read, so existing routes are not listed. Applying it will replace global routing in ${modelsPath} with this profile's routes, so every existing agent route may be replaced or cleared back to inherit${orchestratorEffects}. Continue?`; + } else { + const changes = [...replacedRoutes, ...clearedRoutes, ...addedRoutes]; + const changeSummary = changes.length > 0 + ? changes.join("; ") + : "its agent routes already match the current global routing"; + confirmMessage = `Profile "${result.name}" has agent routing entries. Applying it will replace global routing in ${sanitizeTerminalText(modelConfigPath(ctx.cwd))} with this profile's routes: ${changeSummary}${orchestratorEffects}. Continue?`; + } + const approved = await ctx.ui.confirm(`Apply profile "${result.name}"?`, confirmMessage); + if (!approved) return file; + } // Applying spans three files — the store, models.json, and Pi's global // settings.json — and there is no cross-file rename, so order the writes to // keep the store truthful and compensate on failure: claim the profile in diff --git a/odd/tasks/pr-1384-populated-apply-confirm.md b/odd/tasks/pr-1384-populated-apply-confirm.md new file mode 100644 index 000000000..d79d11b20 --- /dev/null +++ b/odd/tasks/pr-1384-populated-apply-confirm.md @@ -0,0 +1,94 @@ +# PR #1384 Populated-Apply Confirmation + +## Objective + +Close the maintainer-flagged gap on PR #1384: applying a *populated* profile +globally still replaces every materialized route without confirmation +(`Ctrl+S` → `enter` sequence from #1349). Global applies now always confirm, +naming what changes before anything is written. + +## Decisions (operator, 2026-09-30) + +- Behavior: confirm **every** global profile apply. Populated profiles get a + dialog naming replaced routes, cleared agents, and added agents. The + empty / orchestrator-only dialogs from commit `efc7b52a` stay specialized. +- Repo-pinned applies stay silent (repo-scoped, no global state touched). +- Delivery: local only. No push, no PR comment unless the operator later + authorizes it. + +## Scope + +- `extensions/gentle-ai.ts`: extend the `case "apply"` guard — read the + current global routing via `readModelRoutingAuthorityAsync` before the + prompt, compute a diff (replaced / cleared / added agent routes), prompt on + every global apply, abort on decline before any write or live switch. +- `tests/gentle-ai.test.ts`: flip the "nonempty global apply never asks for + confirmation" assertion; add populated-apply regressions (decline preserves + all four surfaces byte-identically; confirm applies and reports). + +## Out of scope + +- #1557 key rebinding (`a`) reconciliation — follow-up when that branch lands. +- Snapshot/undo of successful applies (jonathanludena's general case). +- `odd/tasks/pr-1384-review-fixes.md` cleanup — operator decision at delivery. + +## Tasks + +- [x] T1 — RED: populated-apply regression (prompt shown, decline preserves + models.json/subagents.json/store/settings byte-identically and no live + switch). Observed: `confirmCalls.length 0 !== 1` on both new tests and on + the flipped orchestrator-persistence assertion. +- [x] T2 — Implement the universal confirmation with route diff. +- [x] T3 — GREEN: focused profile tests (37 pass), full + `tests/gentle-ai.test.ts` (96 pass), typecheck (187 diagnostics, baseline + unchanged), `git diff --check` clean. +- [ ] T4 — Work-unit commit (local only) and delivery report. + +## CodeRabbit closure round (2026-10-02, both Major threads) + +- Finding 1 (materialized-only routes): the populated dialog now diffs against + the effective current routing — saved global routing merged with materialized + routes via the new `readGlobalEffectiveModelConfigFromAsync` helper (reuses + `listDiscoverableAgentsAsync` / `readMaterializedRoutingEntryAsync`, same as + `readEffectiveModelConfigAsync`, which now delegates to it). `models.json` + alone hid routes the approval actually clears. Unreadable-authority + disclosure branch unchanged and still authoritative. +- Finding 2 (orchestrator effects): when a populated profile has an + `orchestrator` entry, both the readable and unreadable populated messages + append "set the configured orchestrator entry in settings.json, and attempt + to switch this session to that orchestrator model" (same wording as the + orchestrator-only dialog; never an unconditional switch promise). No + orchestrator entry → messages unchanged. +- T7 — RED observed: 4 new tests fail — dialog claimed "its agent routes + already match the current global routing" while approval would clear + helper's materialized route (2 tests); readable and unreadable populated + messages lacked the settings.json / live-switch disclosure (2 tests). +- T8 — GREEN: focused profile tests 43 pass, full `tests/gentle-ai.test.ts` + 102 pass, typecheck 187 diagnostics (baseline unchanged), `git diff --check` + clean. Confirm-path regression added (materialized-only route cleared on + approval); existing message assertions untouched. + +## QA R4 closure (2026-09-30) + +- Finding: with an invalid routing authority the apply guard fell back to + `currentRouting = {}`, so the populated dialog labeled every profile route + `(added)` and never disclosed the replace/clear-to-inherit effect. +- Fix: when `readModelRoutingAuthorityAsync` is not `valid` at prompt time, the + populated dialog uses an alternative message disclosing the unreadable + current routing and that applying replaces global routing so every existing + agent route may be replaced or cleared back to inherit. Guard structure, + decline semantics, empty/orchestrator-only dialogs, and repo-pinned silence + unchanged. +- T5 — RED observed: `AssertionError ... the dialog must disclose that the + current global routing is unreadable` with actual message ending + `worker: openai/alpha (added). Continue?` on both new tests. +- T6 — GREEN: focused profile tests 39 pass, full `tests/gentle-ai.test.ts` + 98 pass, typecheck 187 diagnostics (baseline), `git diff --check` clean. + +## Constraints + +- Strict TDD: observe RED before changing production code. +- Technical artifacts in English; dialog copy stays consistent with the + existing `efc7b52a` style. +- Allowed edit surfaces: `extensions/gentle-ai.ts`, `tests/gentle-ai.test.ts` + (plus this task doc). diff --git a/odd/tasks/pr-1384-review-fixes.md b/odd/tasks/pr-1384-review-fixes.md new file mode 100644 index 000000000..e5405ade4 --- /dev/null +++ b/odd/tasks/pr-1384-review-fixes.md @@ -0,0 +1,46 @@ +# PR #1384 Review Fixes + +## Objective + +Close the verified CodeRabbit review finding on PR #1384 (`#1349` empty profile wipe guard) by requiring confirmation when applying orchestrator-only profiles that lack agent routes. + +## Problem + +Commit `b0a590be` checked `Object.keys(normalized).length === 0` to prompt for confirmation. However, a profile that contains only an orchestrator entry (`orchestrator: { model: "..." }`) has `Object.keys(normalized).length === 1`. Applying it skipped confirmation, replaced global routing with the orchestrator-only config, and wiped materialized routes for all omitted agents in `subagents.json`. + +## Scope + +- In `extensions/gentle-ai.ts`, check whether any agent routes exist (excluding the reserved orchestrator key) before applying a profile. +- Add regression coverage in `tests/gentle-ai.test.ts` for orchestrator-only profiles. +- Verify typecheck and tests. + +## Constraints + +- Keep the patch minimal and limited to PR #1384 review findings. +- Technical artifacts remain in English. +- Do not commit, push, or merge without explicit user direction. +- Strict TDD discipline: RED test confirmed before implementation fix. + +## Tasks + +- [x] **T1 — Guard orchestrator-only profiles against unconfirmed agent route wipes.** Check for absence of agent routes (e.g. `!Object.keys(normalized).some((name) => !isProfileOrchestratorKey(name))`) in the profile apply flow. +- [x] **T2 — Regression tests and typecheck verification.** Verify orchestrator-only profile application prompts for confirmation and aborts when declined. +- [x] **T3 — Correct confirmation copy and strengthen safety assertions.** Implementation and independent nan QA complete: dialog distinguishes empty/orchestrator-only profiles and explains settings plus attempted live-session change. Decline preserves models, subagents, profile store, settings and live switches; confirmation clears omitted routes; nonempty/repo-pinned applies do not prompt. Work-unit commit pending explicit user authorization. +- [ ] **T4 — Validate the correction candidate and report delivery readiness.** Run candidate assessment and native review under the enabled user-owned RDD switch. Commit, push and PR merge remain pending explicit user direction. + +## QA follow-up (2026-09-30) + +The operator accepted independent nan QA findings on target `03ea59771bb7d18adc28f8702890b306bf7d1069`. The confirmation works, but the orchestrator-only message incorrectly promises an empty config and omits settings/live-session effects. Existing tests do not explicitly assert preservation of every affected surface or confirmed omitted-agent clearing. + +- Allowed source surfaces: `extensions/gentle-ai.ts`, `tests/gentle-ai.test.ts`. +- Route: delegated `gentle-ai-worker`; multi-file writer trigger (two nontrivial source/test files). +- TDD: strict, retained from this feature document's Constraints; observe failing copy regression before changing production code, then GREEN. +- Focused runner: `node --experimental-strip-types --test --test-name-pattern="profile" tests/gentle-ai.test.ts`. +- Closure checks: `node --experimental-strip-types --test tests/gentle-ai.test.ts`; `node scripts/check-types.mjs`; `git diff --check`. +- Environment: target requires pi-coding-agent 0.99.1; parent dependencies previously contained 0.85.1. Use matching dependencies in an isolated test worktree, without installing or modifying operator routing. +- Delivery strategy: ask-on-risk; forecast approximately 80–160 additional authored lines, one cohesive correction unit; no size-driven split required yet. +- Local branch `pr-1384` fast-forwarded to updated PR head `03ea5977`; no history rewritten. +- Progress: T3 verified locally. Writer (glm-4.7/high) observed RED for title and conditional live-switch wording, then GREEN. Independent nan/mimo-v2.6-flash QA observed focused 35/35 and full extension 94/94 tests, typecheck 187 baseline diagnostics/no regressions, and full-tree diff check exit 0 using pi 0.99.1 dependencies. Temporary test worktrees removed; parent dependencies and operator config unchanged. +- T4 blocked: native assessment returned schema-incompatible/unassessable, requiring independent verification (completed). Native inspect then stopped with managed_assets_outdated, offering `/home/jbarbat/.local/share/gentle-ai/main/gentle-ai sync --agent pi`. No lineage started and no review approval obtained. Synchronizing installed assets is outside this source-only correction and awaits operator direction. +- Remaining checks: manual TUI Escape/cancel smoke and installed packed-package check not performed; no new CI run because corrections remain local. +- Delivery: no correction commit, push or PR merge executed. Engram mirror unavailable (session already ended). Next step: operator decides whether to sync managed assets for native review and authorize delivery separately. diff --git a/tests/gentle-ai.test.ts b/tests/gentle-ai.test.ts index cb030f993..91300e010 100644 --- a/tests/gentle-ai.test.ts +++ b/tests/gentle-ai.test.ts @@ -406,6 +406,8 @@ function routingConsumerFixture(t: test.TestContext, agents = ["worker"]) { { provider: "openai", id: "beta" }, { provider: "nan", id: "glm5.3" }, ]; + let onConfirm: (title: string, message: string) => Promise = async () => true; + const confirmCalls: Array<[string, string]> = []; const ctx = { cwd: root, hasUI: true, @@ -414,6 +416,10 @@ function routingConsumerFixture(t: test.TestContext, agents = ["worker"]) { find: (provider: string, id: string) => registryModels.find((model) => model.provider === provider && model.id === id), }, ui: { + confirm: async (title: string, message: string) => { + confirmCalls.push([title, message]); + return onConfirm(title, message); + }, notify(message: string, severity: string) { notifications.push({ message, severity }); }, input: async (title: string, placeholder?: string) => { inputPrompts.push({ title, placeholder }); @@ -445,6 +451,8 @@ function routingConsumerFixture(t: test.TestContext, agents = ["worker"]) { }, refuseSetModel() { setModelResult = false; }, rejectThinkingLevel() { thinkingRejects = true; }, + confirmCalls, + onConfirm(handler: (title: string, message: string) => Promise) { onConfirm = handler; }, onPanel(action: typeof onPanel) { onPanel = action; }, onInput(action: (panel: RoutingConsumerPanel) => void) { onInput = action; }, answerInputs(...answers: Array) { inputAnswers.push(...answers); }, @@ -2016,6 +2024,9 @@ test("applying a profile persists its orchestrator and never leaks the key into worker: { model: "openai/alpha" }, }, }); + // Every global apply now confirms, so this populated apply approves the + // dialog explicitly before running. + fixture.onConfirm(async () => true); applyOnce(fixture); await fixture.run("gentle:profiles"); @@ -2041,6 +2052,8 @@ test("applying a profile persists its orchestrator and never leaks the key into { kind: "model", provider: "nan", id: "glm5.3" }, { kind: "thinking", level: "max" }, ], "the live session switches to the profile's orchestrator"); + assert.equal(fixture.confirmCalls.length, 1, "every global apply asks for confirmation"); + assert.match(fixture.confirmCalls[0]?.[0] ?? "", /Apply profile/); }); test("applying a profile whose orchestrator model is unknown to the registry persists the default and says the session did not switch", async (t) => { @@ -2106,6 +2119,440 @@ test("a profile store entry with only the orchestrator key counts zero roles", a assert.match(applied, /Orchestrator set to nan\/glm5\.3 · high/); }); +test("applying an empty profile asks for confirmation and aborts when declined", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + writeStore({ + empty: {}, + team: { worker: { model: "openai/alpha" } }, + }, "team"); + const before = { + models: readFileSync(fixture.globalPath, "utf8"), + subagents: readFileSync(subagentsPath, "utf8"), + store: readFileSync(storePath, "utf8"), + settings: readFileSync(settingsPath, "utf8"), + }; + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "confirm dialog must be displayed when applying an empty profile"); + const [title, message] = fixture.confirmCalls[0]; + assert.equal(title, "Apply empty profile?"); + assert.match(message, /has no routing entries/); + assert.match(message, /replace global routing/); + assert.match(message, /empty configuration/); + + assert.equal(readFileSync(fixture.globalPath, "utf8"), before.models, "declined apply must preserve models.json byte-identically"); + assert.equal(readFileSync(subagentsPath, "utf8"), before.subagents, "declined apply must preserve subagents.json byte-identically"); + assert.equal(readFileSync(storePath, "utf8"), before.store, "declined apply must preserve the profiles store byte-identically"); + assert.equal(readFileSync(settingsPath, "utf8"), before.settings, "declined apply must preserve settings.json byte-identically"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); + const models = JSON.parse(before.models); + assert.deepEqual(models, { worker: { model: "openai/alpha" } }, "global routing must NOT be wiped when declined"); + + const store = JSON.parse(before.store); + assert.equal(store.active, "team", "active profile must not change when declined"); +}); + +test("applying an empty profile with explicit confirmation replaces global routing with empty config", async (t) => { + const { fixture, storePath, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + writeStore({ + empty: {}, + team: { worker: { model: "openai/alpha" } }, + }, "team"); + + fixture.onConfirm(async () => true); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "confirm dialog must be displayed when applying an empty profile"); + const models = JSON.parse(readFileSync(fixture.globalPath, "utf8")); + assert.deepEqual(models, {}, "global routing must be replaced with empty config when confirmed"); + + const store = JSON.parse(readFileSync(storePath, "utf8")); + assert.equal(store.active, "empty", "active profile must be set to empty when confirmed"); + + const subagents = JSON.parse(readFileSync(subagentsPath, "utf8")); + assert.deepEqual(subagents, {}, "confirmed empty apply clears the seeded omitted worker route (shared clearing semantics drop the empty key)"); +}); + +test("applying an orchestrator-only profile asks for confirmation and aborts when declined", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + writeStore({ + orchOnly: { orchestrator: { model: "nan/glm5.3", thinking: "high" } }, + team: { worker: { model: "openai/alpha" } }, + }, "team"); + const before = { + models: readFileSync(fixture.globalPath, "utf8"), + subagents: readFileSync(subagentsPath, "utf8"), + store: readFileSync(storePath, "utf8"), + settings: readFileSync(settingsPath, "utf8"), + }; + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "confirm dialog must be displayed when applying an orchestrator-only profile"); + const [title, message] = fixture.confirmCalls[0]; + assert.equal(title, "Apply orchestrator-only profile?"); + assert.match(message, /has an orchestrator entry but no agent routing entries/); + assert.match(message, /clear every materialized agent route/); + assert.match(message, /inherit/); + assert.match(message, /orchestrator entry/); + assert.match(message, /settings\.json/); + assert.match(message, /attempt to switch this session/, "the dialog must not promise an unconditional live switch; registry/auth refusal can keep the session model"); + assert.doesNotMatch(message, /empty configuration/, "orchestrator-only must not promise an entirely empty config"); + + assert.equal(readFileSync(fixture.globalPath, "utf8"), before.models, "declined apply must preserve models.json byte-identically"); + assert.equal(readFileSync(subagentsPath, "utf8"), before.subagents, "declined apply must preserve subagents.json byte-identically"); + assert.equal(readFileSync(storePath, "utf8"), before.store, "declined apply must preserve the profiles store byte-identically"); + assert.equal(readFileSync(settingsPath, "utf8"), before.settings, "declined apply must preserve settings.json byte-identically"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); + + const store = JSON.parse(before.store); + assert.equal(store.active, "team", "active profile must not change when declined"); +}); + +test("applying an orchestrator-only profile with explicit confirmation updates orchestrator and active profile", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + writeStore({ + orchOnly: { orchestrator: { model: "nan/glm5.3", thinking: "high" } }, + team: { worker: { model: "openai/alpha" } }, + }, "team"); + + fixture.onConfirm(async () => true); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "confirm dialog must be displayed when applying an orchestrator-only profile"); + const models = JSON.parse(readFileSync(fixture.globalPath, "utf8")); + assert.deepEqual(models, { orchestrator: { model: "nan/glm5.3", thinking: "high" } }, "global routing must hold only orchestrator"); + + const store = JSON.parse(readFileSync(storePath, "utf8")); + assert.equal(store.active, "orchOnly", "active profile must be set to orchOnly when confirmed"); + + const settings = JSON.parse(readFileSync(settingsPath, "utf8")); + assert.equal(settings.defaultProvider, "nan"); + assert.equal(settings.defaultModel, "glm5.3"); + assert.equal(settings.defaultThinkingLevel, "high"); + + const subagents = JSON.parse(readFileSync(subagentsPath, "utf8")); + assert.deepEqual(subagents, {}, "confirmed orchestrator-only apply clears the seeded omitted worker route (shared clearing semantics drop the empty key)"); + assert.equal("orchestrator" in subagents, false, "the orchestrator key never materializes as an agent route"); + assert.deepEqual(fixture.liveSwitches, [ + { kind: "model", provider: "nan", id: "glm5.3" }, + { kind: "thinking", level: "high" }, + ], "the live session switches to the profile's orchestrator"); +}); + +test("applying a populated profile asks for confirmation naming the diff and aborts when declined", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + // Current global routing: "worker" will be replaced, "helper" will be + // cleared back to inherit, and "tracer" only exists in the profile. + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha", thinking: "high" }, helper: { model: "openai/beta" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/alpha", effort: "high" }, helper: { model: "openai/beta" } } }, null, 2)}\n`); + const helperPath = join(fixture.root, ".pi", "agents", "helper.md"); + writeMarkdown(helperPath, "---\nname: helper\ndescription: Helper\nmodel: openai/beta\n---\nbody\n"); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/beta", thinking: "max" }, + tracer: { model: "openai/alpha" }, + }, + }); + const before = { + models: readFileSync(fixture.globalPath, "utf8"), + subagents: readFileSync(subagentsPath, "utf8"), + store: readFileSync(storePath, "utf8"), + settings: readFileSync(settingsPath, "utf8"), + }; + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [title, message] = fixture.confirmCalls[0]; + assert.equal(title, 'Apply profile "team"?'); + assert.match(message, /replace global routing/, "the dialog must name the file whose routing is replaced"); + assert.match(message, /models\.json/); + assert.match(message, /worker: openai\/alpha · high → openai\/beta · max/, "the replaced route must name both the old and the new route"); + assert.match(message, /helper: openai\/beta → inherit/, "the cleared agent must be named with its return to inherit"); + assert.match(message, /tracer: openai\/alpha \(added\)/, "the added agent must be named"); + + assert.equal(readFileSync(fixture.globalPath, "utf8"), before.models, "declined apply must preserve models.json byte-identically"); + assert.equal(readFileSync(subagentsPath, "utf8"), before.subagents, "declined apply must preserve subagents.json byte-identically"); + assert.equal(readFileSync(storePath, "utf8"), before.store, "declined apply must preserve the profiles store byte-identically"); + assert.equal(readFileSync(settingsPath, "utf8"), before.settings, "declined apply must preserve settings.json byte-identically"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); +}); + +test("applying a populated profile names materialized-only routes it would clear before asking", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + // The saved global routing matches the profile exactly: "helper" is routed + // only where the runtime actually resolves it (agent frontmatter and + // subagents.json model profiles), and the apply clears it because the profile + // omits it. models.json alone never speaks for "helper". + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { helper: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + const helperPath = join(fixture.root, ".pi", "agents", "helper.md"); + writeMarkdown(helperPath, "---\nname: helper\ndescription: Helper\nmodel: openai/beta\n---\nbody\n"); + writeStore({ + team: { worker: { model: "openai/alpha" } }, + }); + const before = { + models: readFileSync(fixture.globalPath, "utf8"), + subagents: readFileSync(subagentsPath, "utf8"), + store: readFileSync(storePath, "utf8"), + settings: readFileSync(settingsPath, "utf8"), + }; + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [title, message] = fixture.confirmCalls[0]; + assert.equal(title, 'Apply profile "team"?'); + assert.match(message, /helper: openai\/beta · high → inherit/, "the materialized-only route the apply clears must be named"); + assert.doesNotMatch(message, /already match/, "routes matching the saved routing must not hide a cleared materialized-only route"); + + assert.equal(readFileSync(fixture.globalPath, "utf8"), before.models, "declined apply must preserve models.json byte-identically"); + assert.equal(readFileSync(subagentsPath, "utf8"), before.subagents, "declined apply must preserve subagents.json byte-identically"); + assert.equal(readFileSync(storePath, "utf8"), before.store, "declined apply must preserve the profiles store byte-identically"); + assert.equal(readFileSync(settingsPath, "utf8"), before.settings, "declined apply must preserve settings.json byte-identically"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); +}); + +test("applying a populated profile clears the materialized-only route it named when confirmed", async (t) => { + const { fixture, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { helper: { model: "openai/beta", effort: "high" } } }, null, 2)}\n`); + const helperPath = join(fixture.root, ".pi", "agents", "helper.md"); + writeMarkdown(helperPath, "---\nname: helper\ndescription: Helper\nmodel: openai/beta\n---\nbody\n"); + writeStore({ + team: { worker: { model: "openai/alpha" } }, + }); + + fixture.onConfirm(async () => true); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [, message] = fixture.confirmCalls[0]; + assert.match(message, /helper: openai\/beta · high → inherit/, "the dialog must name the materialized-only route before it is cleared"); + const subagents = JSON.parse(readFileSync(subagentsPath, "utf8")); + assert.equal("helper" in subagents.model_profiles, false, "confirming the apply clears the materialized-only route the dialog named"); + assert.doesNotMatch(readFileSync(helperPath, "utf8"), /^model:/m, "the materialized-only frontmatter route is dropped"); +}); + +test("applying a populated profile with an orchestrator entry discloses the settings and live-session effects", async (t) => { + const { fixture, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha" } }, null, 2)}\n`); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/beta" }, + }, + }); + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [, message] = fixture.confirmCalls[0]; + assert.match(message, /replace global routing/, "the readable variant must keep naming the routing diff"); + assert.match(message, /settings\.json/, "the dialog must disclose that the orchestrator entry is set in settings.json"); + assert.match(message, /attempt to switch this session/, "the dialog must disclose the attempted live-session switch"); + assert.doesNotMatch(message, /will switch this session/, "the dialog must not promise an unconditional live switch"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); +}); + +test("applying a populated profile with an orchestrator entry discloses the effects when the routing is unreadable", async (t) => { + const { fixture, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + // Malformed models.json keeps the unreadable-authority disclosure branch in + // charge: the orchestrator effects must be added there, not swap it out. + writeFileSync(fixture.globalPath, "{ not json\n"); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/alpha" }, + }, + }); + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [, message] = fixture.confirmCalls[0]; + assert.match(message, /could not be read/, "the unreadable-authority disclosure must stay authoritative"); + assert.match(message, /settings\.json/, "the dialog must disclose that the orchestrator entry is set in settings.json"); + assert.match(message, /attempt to switch this session/, "the dialog must disclose the attempted live-session switch"); + assert.doesNotMatch(message, /will switch this session/, "the dialog must not promise an unconditional live switch"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); +}); + +test("applying a populated profile with an unreadable routing authority discloses the replace effect instead of added labels", async (t) => { + const { fixture, storePath, writeStore, writeSettings, settingsPath } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + // Malformed JSON makes readModelRoutingAuthorityAsync report the global + // models.json as invalid: real routes exist on disk but cannot be diffed. + writeFileSync(fixture.globalPath, "{ not json\n"); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/alpha", effort: "high" } } }, null, 2)}\n`); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/alpha" }, + }, + }); + const before = { + models: readFileSync(fixture.globalPath, "utf8"), + subagents: readFileSync(subagentsPath, "utf8"), + store: readFileSync(storePath, "utf8"), + settings: readFileSync(settingsPath, "utf8"), + }; + + fixture.onConfirm(async () => false); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [title, message] = fixture.confirmCalls[0]; + assert.equal(title, 'Apply profile "team"?'); + assert.match(message, /could not be read/, "the dialog must disclose that the current global routing is unreadable"); + assert.match(message, /replace global routing/, "the dialog must name the global replacement"); + assert.match(message, /inherit/, "the dialog must disclose the clear-back-to-inherit effect"); + assert.doesNotMatch(message, /\(added\)/, "unread current routes must not be presented as merely added"); + + assert.equal(readFileSync(fixture.globalPath, "utf8"), before.models, "declined apply must preserve models.json byte-identically"); + assert.equal(readFileSync(subagentsPath, "utf8"), before.subagents, "declined apply must preserve subagents.json byte-identically"); + assert.equal(readFileSync(storePath, "utf8"), before.store, "declined apply must preserve the profiles store byte-identically"); + assert.equal(readFileSync(settingsPath, "utf8"), before.settings, "declined apply must preserve settings.json byte-identically"); + assert.deepEqual(fixture.liveSwitches, [], "declined apply must not switch the live session"); +}); + +test("applying a populated profile with an unreadable routing authority still applies when confirmed", async (t) => { + const { fixture, storePath, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, "{ not json\n"); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/alpha" }, + }, + }); + + fixture.onConfirm(async () => true); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const [, message] = fixture.confirmCalls[0]; + assert.match(message, /could not be read/, "the dialog must disclose that the current global routing is unreadable"); + const models = JSON.parse(readFileSync(fixture.globalPath, "utf8")); + assert.deepEqual(models, { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/alpha" }, + }, "confirmed apply proceeds as before despite the unreadable authority"); + const store = JSON.parse(readFileSync(storePath, "utf8")); + assert.equal(store.active, "team", "active profile must be set to team when confirmed"); +}); + +test("applying a populated profile with explicit confirmation applies the routing diff", async (t) => { + const { fixture, storePath, writeStore, writeSettings } = profilesStoreFixture(t); + writeSettings(); + mkdirSync(fixture.configHome, { recursive: true }); + writeFileSync(fixture.globalPath, `${JSON.stringify({ worker: { model: "openai/alpha", thinking: "high" }, helper: { model: "openai/beta" } }, null, 2)}\n`); + const subagentsPath = join(fixture.root, ".pi", "subagents.json"); + writeFileSync(subagentsPath, `${JSON.stringify({ model_profiles: { worker: { model: "openai/alpha", effort: "high" }, helper: { model: "openai/beta" } } }, null, 2)}\n`); + const helperPath = join(fixture.root, ".pi", "agents", "helper.md"); + writeMarkdown(helperPath, "---\nname: helper\ndescription: Helper\nmodel: openai/beta\n---\nbody\n"); + const tracerPath = join(fixture.root, ".pi", "agents", "tracer.md"); + writeMarkdown(tracerPath, "---\nname: tracer\ndescription: Tracer\n---\nbody\n"); + writeStore({ + team: { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/beta", thinking: "max" }, + tracer: { model: "openai/alpha" }, + }, + }); + + fixture.onConfirm(async () => true); + + applyOnce(fixture); + await fixture.run("gentle:profiles"); + + assert.equal(fixture.confirmCalls.length, 1, "a populated global apply must ask for confirmation"); + const models = JSON.parse(readFileSync(fixture.globalPath, "utf8")); + assert.deepEqual(models, { + orchestrator: { model: "nan/glm5.3", thinking: "max" }, + worker: { model: "openai/beta", thinking: "max" }, + tracer: { model: "openai/alpha" }, + }, "confirmed apply replaces global routing with the profile"); + const subagents = JSON.parse(readFileSync(subagentsPath, "utf8")); + assert.equal("helper" in subagents.model_profiles, false, "the cleared agent loses its materialized route"); + assert.equal(subagents.model_profiles.worker?.model, "openai/beta", "the replaced route is materialized"); + assert.equal(subagents.model_profiles.tracer?.model, "openai/alpha", "the added agent is materialized"); + assert.doesNotMatch(readFileSync(helperPath, "utf8"), /^model:/m, "the cleared agent's frontmatter route is dropped"); + const store = JSON.parse(readFileSync(storePath, "utf8")); + assert.equal(store.active, "team", "active profile must be set to team when confirmed"); + assert.deepEqual(fixture.liveSwitches, [ + { kind: "model", provider: "nan", id: "glm5.3" }, + { kind: "thinking", level: "max" }, + ], "the live session switches to the profile's orchestrator"); +}); + test("applying a profile replaces materialized routing for agents the profile omits", async (t) => { const { fixture, writeStore, writeSettings } = profilesStoreFixture(t); writeSettings(); @@ -2781,6 +3228,7 @@ test("applying a profile in a pinned repository re-pins the clone and writes no assert.equal(readFileSync(fixture.globalPath, "utf8"), modelsBefore, "no global routing was written"); assert.equal(readFileSync(settingsPath, "utf8"), settingsBefore, "no orchestrator was written"); assert.equal(existsSync(join(fixture.root, ".pi", "subagents.json")), false, "no materialized routing was written"); + assert.equal(fixture.confirmCalls.length, 0, "a repo-pinned apply never asks for confirmation"); assert.match(fixture.notifications.at(-1)?.message ?? "", /repo-scoped/); });