From 220414682e782061ee303a45705e53b18a754477 Mon Sep 17 00:00:00 2001 From: Yann Roberto <165009858+shoyann@users.noreply.github.com> Date: Fri, 23 Oct 2026 02:23:58 +0800 Subject: [PATCH] fix(setup): preserve explicit clients with all-detected selection (#5508) * fix(setup): preserve explicit clients with all-detected selection * test(filesystem): align staged cancellation cleanup assertions --------- Co-authored-by: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/installation.md | 8 +- src/application/Setup.planning.test.ts | 55 +++++++++++++ src/application/Setup.ts | 8 +- src/cli/setupCommands.ts | 3 +- .../setup/repeatedClientSetup.test.ts | 78 +++++++++++++++++++ .../filesystem/jsonOutputOwnership.test.ts | 20 +++-- 6 files changed, 157 insertions(+), 15 deletions(-) diff --git a/docs/installation.md b/docs/installation.md index dc331ac712..3cafa9a797 100644 --- a/docs/installation.md +++ b/docs/installation.md @@ -104,9 +104,11 @@ your investigation. Failed steps and diagnostics remain in terminal history. Select exact clients in scripts with repeatable `--client` flags. Each explicit client skips interactive selection. Use `--all-detected` only when you intend -to configure every detected supported client. `--skill=false` omits the -workflow, and `--dry-run` returns a read-only plan with status `planned` and -exit code 0: +to configure every detected supported client. Combining `--client` with +`--all-detected` configures both the explicitly selected and detected clients, +even when an explicit client is not detected. Each client is configured once. +`--skill=false` omits the workflow, and `--dry-run` returns a read-only plan +with status `planned` and exit code 0: ```bash rea setup --client codex --client cursor --skill=false --dry-run diff --git a/src/application/Setup.planning.test.ts b/src/application/Setup.planning.test.ts index 51917ba6a8..abef189bd4 100644 --- a/src/application/Setup.planning.test.ts +++ b/src/application/Setup.planning.test.ts @@ -224,6 +224,61 @@ describe("setup workflow", () => { }); describe("setup client selection", () => { + it.each([ + { + name: "explicit clients before detected clients without duplicates", + selection: { clientIds: ["cursor", "codex", "cursor"] }, + expected: ["cursor", "codex"], + }, + { + name: "readiness-scope clients when no explicit list is supplied", + selection: { readinessScope: { clients: ["cursor"] } }, + expected: ["cursor", "codex"], + }, + { + name: "only detected clients without an explicit selection", + selection: {}, + expected: ["codex"], + }, + ])("configures and verifies $name", async ({ selection, expected }) => { + const host = new FakeSetupHost(); + host.hopper = "/Applications/Hopper"; + host.skill = "unchanged"; + const codex = { + name: "codex", + configPath: "/codex.toml", + format: "toml" as const, + }; + host.clients = [codex]; + host.availableClients = [ + { name: "cursor", configPath: "/cursor.json" }, + codex, + ]; + + const result = await runSetup( + { ...options(true), ...selection, allDetectedClients: true }, + host, + ); + + expect(result.status).toBe("ready"); + expect(result.plannedActions.map(({ id }) => id)).toEqual( + expected.map((id) => `configure_client:${id}`), + ); + expect(result.appliedActions).toEqual( + expected.map((id) => `configured_${id}`), + ); + expect(host.configurations).toBe(expected.length); + expect(host.skillDirectoryValidations).toEqual([expected, expected]); + expect(host.doctorScopes.at(-1)).toEqual({ + clients: expected, + providers: [], + skill: true, + skillClients: expected, + }); + expect(host.hopperInstalls).toBe(0); + expect(host.skillInstalls).toBe(0); + }); + it.each([true, false])( "handles repeated clients once in first-selected order (dry run: %s)", async (dryRun) => { diff --git a/src/application/Setup.ts b/src/application/Setup.ts index b1b674737a..3a5d99e6c4 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -128,13 +128,13 @@ const runSetupWorkflow = async ( options.installSkill === undefined; const interactiveSelection = confirm !== undefined && !options.structured && clientSelectionAllowed; + const requestedClientIds = + options.clientIds ?? options.readinessScope?.clients; let selectedClientIds = [ ...new Set( options.allDetectedClients - ? discovery.detectedClientIds - : (options.clientIds ?? - options.readinessScope?.clients ?? - discovery.defaultClientIds), + ? [...(requestedClientIds ?? []), ...discovery.detectedClientIds] + : (requestedClientIds ?? discovery.defaultClientIds), ), ]; let selectedSkillClientIds = diff --git a/src/cli/setupCommands.ts b/src/cli/setupCommands.ts index 947937bfca..a1dacaddce 100644 --- a/src/cli/setupCommands.ts +++ b/src/cli/setupCommands.ts @@ -255,7 +255,6 @@ const setupRunOptions = ( options.skillClient.length > 0; const hasSelectedClients = hasExplicitScope && - !options.allDetected && (options.client.length > 0 || options.skillClient.length > 0); return { approved: options.yes && !options.dryRun, @@ -272,7 +271,7 @@ const setupRunOptions = ( ? { installSkill: options.skill ?? agentIntegrationSelected } : {}), ...(interactive ? { onProgress: renderSetupProgress } : {}), - ...(hasSelectedClients + ...(hasSelectedClients && !options.allDetected ? { readinessScope: { clients: options.client, diff --git a/tests/acceptance/setup/repeatedClientSetup.test.ts b/tests/acceptance/setup/repeatedClientSetup.test.ts index e1a66f3f2d..def26a75bb 100644 --- a/tests/acceptance/setup/repeatedClientSetup.test.ts +++ b/tests/acceptance/setup/repeatedClientSetup.test.ts @@ -81,6 +81,84 @@ cliTest( }, ); +cliTest( + "combines explicitly selected undetected clients with all detected clients", + async ({ cli }) => { + const home = await createTestTempDirectory( + "rea-explicit-detected-clients-", + ); + await mkdir(join(home, ".codex")); + const run = (selection: readonly string[], mode: string) => + cli.run({ + arguments: ["setup", ...selection, "--skill=false", "--json", mode], + cwd: home, + environment: { + HOME: home, + USERPROFILE: home, + TMPDIR: home, + XDG_CONFIG_HOME: join(home, ".config"), + XDG_DATA_HOME: join(home, ".local", "share"), + XDG_CACHE_HOME: join(home, ".cache"), + XDG_STATE_HOME: join(home, ".local", "state"), + }, + }); + const configurations = [ + join(home, ".cursor", "mcp.json"), + join(home, ".codex", "config.toml"), + ]; + const combined = [ + "--client", + "cursor", + "--client", + "cursor", + "--all-detected", + ]; + for (const [selection, expected] of [ + [["--client", "cursor"], ["cursor"]], + [["--all-detected"], ["codex"]], + [combined, ["cursor", "codex"]], + ] as const) { + const preview = await run(selection, "--dry-run"); + expect(preview.exitCode, preview.stdout).toBe(0); + expect(preview.json).toMatchObject({ + status: "planned", + plannedActions: expected.map((id) => ({ + id: `configure_client:${id}`, + })), + appliedActions: [], + }); + for (const path of configurations) + await expect(access(path)).rejects.toMatchObject({ code: "ENOENT" }); + } + + const applied = await run(combined, "--yes"); + expect(applied.exitCode, applied.stdout).toBe(0); + expect(applied.json).toMatchObject({ + status: "ready", + plannedActions: [ + { id: "configure_client:cursor" }, + { id: "configure_client:codex" }, + ], + appliedActions: ["configured_cursor", "configured_codex"], + clients: { + cursor: { status: "configured" }, + codex: { status: "configured" }, + }, + doctor: { + healthy: true, + scope: { + mode: "explicit", + clients: ["codex", "cursor"], + providers: [], + skill: false, + }, + }, + }); + for (const path of configurations) + expect(await readFile(path, "utf8")).toContain("rea"); + }, +); + cliTest( "uninstall preserves user edits made after an earlier setup", async ({ cli }) => { diff --git a/tests/boundary/filesystem/jsonOutputOwnership.test.ts b/tests/boundary/filesystem/jsonOutputOwnership.test.ts index cd83e0b6ac..8535836ef9 100644 --- a/tests/boundary/filesystem/jsonOutputOwnership.test.ts +++ b/tests/boundary/filesystem/jsonOutputOwnership.test.ts @@ -200,12 +200,14 @@ it("preserves cancellation when staging cleanup also fails", async () => { const root = await createTestTempDirectory("rea-json-cancel-"); const resources = new ArtifactResourceScope(); const controller = new AbortController(); + let unexpected: string | undefined; try { function* cancelledParts(): Generator { const directories = requireStages(root); if (directories.length !== 1) throw new Error("Expected one staging directory"); - writeFileSync(join(root, directories[0]!, "unexpected"), "retain this"); + unexpected = join(root, directories[0]!, "unexpected"); + writeFileSync(unexpected, "retain this"); controller.abort(); yield "must not publish"; } @@ -223,12 +225,16 @@ it("preserves cancellation when staging cleanup also fails", async () => { // directory is reported as cleanup state, not as the cancellation cause. expect(result.error.cause).toBe(controller.signal.reason); expect(result.error.cleanupPrimary).toBeInstanceOf(AnalysisCancelledError); - expect(result.error.cleanupResources).toEqual([ - expect.stringContaining(".rea-write-"), - ]); + expect(result.error.cleanupPrimary).toMatchObject({ + operation: "export_evidence_bundle", + }); + expect(result.error.cleanupPrimary?.cause).toBe(controller.signal.reason); + const stages = requireStages(root); + expect(stages).toHaveLength(1); + expect(result.error.cleanupResources).toEqual([join(root, stages[0]!)]); + expect(await fs.readFile(unexpected!, "utf8")).toBe("retain this"); await expect(fs.access(join(root, "cancelled.json"))).rejects.toThrow(); - const stage = requireStages(root)[0]; - if (stage !== undefined) await fs.rm(join(root, stage, "unexpected")); + await fs.rm(unexpected!); await resources.close(); expect(requireStages(root)).toEqual([]); expect(result.error.cleanupIncomplete).toBe(false); @@ -239,7 +245,9 @@ it("preserves cancellation when staging cleanup also fails", async () => { }); expect(released.details).not.toHaveProperty("resources"); } finally { + if (unexpected !== undefined) await fs.rm(unexpected, { force: true }); await resources.close(); + expect(requireStages(root)).toEqual([]); await rm(root, { recursive: true, force: true }); } });