From d91f2be782142efd12e31af08146b98d10054e17 Mon Sep 17 00:00:00 2001 From: 4ndreello <4ndreello@users.noreply.github.com> Date: Tue, 15 Sep 2026 14:01:03 -0300 Subject: [PATCH] fix(setup): Honor active profile when saving configuration Make interactive and batch setup target the active profile by default so the configuration setup matches runtime resolution. Fail clearly when the active profile is missing and expose the selected profile in doctor output. --- README.md | 2 +- src/cli/commands/doctor.ts | 30 +++++++++--- src/cli/commands/setup.ts | 95 +++++++++++++++++++++++--------------- tests/doctor-roles.test.ts | 14 ++++++ tests/profiles.test.ts | 33 +++++++++++-- tests/setup-wizard.test.ts | 13 ++++++ 6 files changed, 139 insertions(+), 48 deletions(-) diff --git a/README.md b/README.md index 8a7d9c3..9da6657 100644 --- a/README.md +++ b/README.md @@ -200,7 +200,7 @@ npx codedeck run "task" --profile max --bg npx codedeck open reviewer --profile max ``` -`use` sets the default. `--profile` overrides it for one launch, so two profiles run side by side with no switching. An unknown name fails loud instead of launching on the wrong setup. +`use` sets the default. `setup` edits that active profile when one is selected, while `setup --profile max` edits an explicit profile. With no active profile, `setup` edits the base config. `--profile` overrides the active profile for one launch, so two profiles run side by side with no switching. An unknown name fails loud instead of launching on the wrong setup. ## Session diff --git a/src/cli/commands/doctor.ts b/src/cli/commands/doctor.ts index 3222246..3a9e85a 100644 --- a/src/cli/commands/doctor.ts +++ b/src/cli/commands/doctor.ts @@ -73,9 +73,9 @@ export function resolveRoleReadiness(config: RunAgentConfig): RoleReadiness[] { * and nobody could see why: the bindings lived in a JSON file no command * printed. Reading them costs nothing, so `doctor` reads them. */ -export function renderRolesSection(rows: RoleReadiness[]): string { +export function renderRolesSection(rows: RoleReadiness[], activeProfile?: string): string { return [ - "Roles", + activeProfile === undefined ? "Roles" : `Roles (active profile: ${activeProfile})`, ...rows.map(({ role, harness, model, fallback }) => ` ${check(role, harness !== undefined, harness ? `${harness} / ${model}` : `unbound, runs on ${fallback}`)}`, ), @@ -109,16 +109,29 @@ export function registerDoctorCommand(program: Command): void { } const loaded = loadConfig(); + const activeProfile = typeof loaded.activeProfile === "string" && loaded.activeProfile.trim() !== "" + ? loaded.activeProfile.trim() + : undefined; // Readiness follows the active profile. A dangling pointer still gets - // a report: fall back to the base config instead of failing the check. + // a report, but the error is shown instead of hiding it behind the base + // bindings. let effective = loaded; + let activeProfileError: string | null = null; try { effective = resolveEffectiveConfig(loaded); - } catch {} + } catch (error) { + activeProfileError = error instanceof Error ? error.message : String(error); + } const roles = resolveRoleReadiness(effective); if (opts.json) { - console.log(JSON.stringify({ ...result, power: resolvePowerInfo(result), roles }, null, 2)); + console.log(JSON.stringify({ + ...result, + power: resolvePowerInfo(result), + activeProfile: activeProfile ?? null, + activeProfileError, + roles, + }, null, 2)); return; } @@ -172,7 +185,12 @@ export function registerDoctorCommand(program: Command): void { console.log(renderPowerSection(resolvePowerInfo(result))); console.log(""); - console.log(renderRolesSection(roles)); + console.log("Profile"); + console.log(` active ${activeProfile ?? "base config"}`); + if (activeProfileError !== null) console.log(` error ${activeProfileError}`); + console.log(""); + + console.log(renderRolesSection(roles, activeProfile)); console.log(""); const paths = getPaths(); diff --git a/src/cli/commands/setup.ts b/src/cli/commands/setup.ts index e69a5bc..0438582 100644 --- a/src/cli/commands/setup.ts +++ b/src/cli/commands/setup.ts @@ -624,20 +624,7 @@ function watchResize(listener: () => void): () => void { export async function runModelSetupWizard(options: ModelWizardOptions = {}): Promise { const loaded = options.config ?? loadConfig(); - const profile = options.profile !== undefined ? parseProfileName(options.profile) : undefined; - // --profile edits one saved setup, shown exactly as saved. A name nobody - // saved yet starts from the base setup with an empty agent map: role - // screens show no inherited "atual", while sandbox/autocompact/orchestrator - // keep mirroring what the profile would inherit at launch (blanking them - // would silently downgrade the base values on confirm). Without --profile - // the base config is edited and the active snapshot is left untouched - // (see runSetupBatch below). - const snapshot = profile === undefined ? undefined : getProfileSnapshot(loaded, profile); - const config = profile === undefined - ? loaded - : snapshot === undefined - ? { ...baseConfig(loaded), agents: {} } - : { ...baseConfig(loaded), ...snapshot }; + const { profile, config } = resolveSetupTarget(loaded, options.profile); if (!(options.isTTY ?? isInteractiveTerminal())) return config; const output = options.output ?? process.stdout; @@ -836,6 +823,36 @@ export class SetupUsageError extends Error { } } +interface SetupTarget { + profile?: string; + config: RunAgentConfig; +} + +function resolveSetupTarget(loaded: RunAgentConfig, explicitProfile?: string): SetupTarget { + const isExplicit = explicitProfile !== undefined; + const profile = isExplicit + ? parseProfileName(explicitProfile) + : loaded.activeProfile === undefined || loaded.activeProfile.trim() === "" + ? undefined + : parseProfileName(loaded.activeProfile); + const snapshot = profile === undefined ? undefined : getProfileSnapshot(loaded, profile); + + if (!isExplicit && profile !== undefined && snapshot === undefined) { + throw new SetupUsageError( + `Active profile "${profile}" does not exist. Choose an existing profile with "codedeck profile use " or pass --profile .`, + ); + } + + return { + ...(profile === undefined ? {} : { profile }), + config: profile === undefined + ? loaded + : snapshot === undefined + ? { ...baseConfig(loaded), agents: {} } + : { ...baseConfig(loaded), ...snapshot }, + }; +} + function invalidBindMessage(value: string): string { return `Invalid --bind "${value}": expected role=harness:model[:effort] (role: general|orchestrator|reviewer|auditor; harness: claude|codex|opencode|omp; model: non-empty and without whitespace, control characters or '='; effort: ${REASONING_EFFORTS.join("|")})`; } @@ -1371,20 +1388,17 @@ export async function runSetupBatch( } const current: RunAgentConfig = { ...DEFAULT_CONFIG, ...(read.config ?? {}) }; - // --profile edits one saved setup instead of the active one. An existing - // name loads base plus its snapshot; a name nobody saved yet starts from - // the base setup with an empty agent map, so root agents never leak into - // the new profile as inherited "atual" while sandbox/autocompact/ - // orchestrator keep their inherited values. Without --profile the base - // config is edited and saved profiles (including the active one) are left - // untouched. - const profile = options.profile; - const snapshot = profile === undefined ? undefined : getProfileSnapshot(current, profile); - const target: RunAgentConfig = profile === undefined - ? current - : snapshot === undefined - ? { ...baseConfig(current), agents: {} } - : { ...baseConfig(current), ...snapshot }; + let profile: string | undefined; + let target: RunAgentConfig; + try { + const resolved = resolveSetupTarget(current, options.profile); + profile = resolved.profile; + target = resolved.config; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + writeLine(stderr, message); + return errorResult(read, null, notNeededCatalog(), [], [], 14, message); + } const lastByRole = new Map(); options.binds.forEach((binding, index) => lastByRole.set(binding.role, index)); const winning = options.binds.filter((binding, index) => lastByRole.get(binding.role) === index); @@ -1565,16 +1579,21 @@ export async function executeSetupAction( writeError(message); return { code: 1 }; } - await runModelSetupWizard({ - registry: dependencies.registry, - input: dependencies.input, - output: dependencies.stdout, - refresh: parsed.options.refresh, - discoverModels: dependencies.wizardDiscoverModels, - save: dependencies.saveConfig, - isTTY: tty, - ...(parsed.options.profile === undefined ? {} : { profile: parsed.options.profile }), - }); + try { + await runModelSetupWizard({ + registry: dependencies.registry, + input: dependencies.input, + output: dependencies.stdout, + refresh: parsed.options.refresh, + discoverModels: dependencies.wizardDiscoverModels, + save: dependencies.saveConfig, + isTTY: tty, + ...(parsed.options.profile === undefined ? {} : { profile: parsed.options.profile }), + }); + } catch (error) { + writeError(error instanceof Error ? error.message : String(error)); + return { code: 1 }; + } return { code: 0 }; } diff --git a/tests/doctor-roles.test.ts b/tests/doctor-roles.test.ts index d9b388f..09698f7 100644 --- a/tests/doctor-roles.test.ts +++ b/tests/doctor-roles.test.ts @@ -65,4 +65,18 @@ describe("doctor roles section", () => { ); expect(lines.find((line) => line.includes("auditor"))).toContain("✗ unbound, runs on claude"); }); + + it("labels the active profile when showing effective roles", () => { + const lines = strip( + renderRolesSection( + resolveRoleReadiness({ + defaultAgent: "claude", + agents: { reviewer: { harness: "codex", model: "gpt-5" } }, + }), + "default", + ), + ).split("\n"); + + expect(lines[0]).toBe("Roles (active profile: default)"); + }); }); diff --git a/tests/profiles.test.ts b/tests/profiles.test.ts index 0fd3ae2..66bbc5e 100644 --- a/tests/profiles.test.ts +++ b/tests/profiles.test.ts @@ -299,7 +299,7 @@ describe("setup --profile batch", () => { expect(written.agents?.general).toEqual({ harness: "claude", model: "base" }); }); - it("setup without --profile edits the base and does not clobber the active snapshot", async () => { + it("setup without --profile edits the active profile and leaves the base alone", async () => { const file = getPaths().configFile; const activeSnapshot = { agents: { general: { harness: "codex", model: "from-a" } } } as const; const effective = { @@ -330,11 +330,38 @@ describe("setup --profile batch", () => { expect(result.code).toBe(0); const written = save.mock.calls[0][0] as RunAgentConfig; expect(written.activeProfile).toBe("a"); - expect(written.profiles?.a).toEqual(activeSnapshot); - expect(written.agents?.reviewer).toEqual({ harness: "codex", model: "gpt-5" }); + expect(written.profiles?.a?.agents?.reviewer).toEqual({ harness: "codex", model: "gpt-5" }); + expect(written.profiles?.a?.agents?.general).toEqual(activeSnapshot.agents.general); + expect(written.agents?.reviewer).toBeUndefined(); expect(written.agents?.general).toEqual({ harness: "claude", model: "base" }); }); + it("refuses setup when the active profile is missing", async () => { + const { store, deps } = batchWorld(); + const read = store.read as ReturnType; + const file = getPaths().configFile; + const config = { + ...DEFAULT_CONFIG, + activeProfile: "missing", + agents: { general: { harness: "claude", model: "base" } }, + profiles: {}, + } satisfies RunAgentConfig; + read.mockReturnValueOnce({ + status: "ok", + source: "canonical", + path: file, + config, + raw: serializeConfig(config), + message: null, + }); + + const result = await runSetupBatch(setupOptions(["--bind", "reviewer=codex:gpt-5"]), deps); + + expect(result.code).toBe(14); + expect(result.envelope.resultado.message).toContain('Active profile "missing" does not exist'); + expect(store.save).not.toHaveBeenCalled(); + }); + it("rejects a bad profile name at parse time", () => { expect(parseSetupArgs(["--profile", "bad name"])).toMatchObject({ ok: false }); }); diff --git a/tests/setup-wizard.test.ts b/tests/setup-wizard.test.ts index 4c97217..324caf2 100644 --- a/tests/setup-wizard.test.ts +++ b/tests/setup-wizard.test.ts @@ -202,6 +202,19 @@ describe("runModelSetupWizard", () => { expect(existing.agents?.general).toEqual({ harness: "codex", model: "from-a" }); }); + it("uses the active profile when no --profile is given", async () => { + const config: RunAgentConfig = { + ...base().config, + agents: { general: { harness: "claude", model: "base-model" } }, + activeProfile: "a", + profiles: { a: { agents: { general: { harness: "omp", model: "from-a" } } } }, + }; + + const result = await runModelSetupWizard({ config, isTTY: false }); + + expect(result.agents?.general).toEqual({ harness: "omp", model: "from-a" }); + }); + it("keeps the inherited sandbox and autocompact values when creating a profile", async () => { const { input, output } = io(); const save = vi.fn();