From 4d02bc82b3de9cba642086955506dfdd848d15a2 Mon Sep 17 00:00:00 2001 From: Avocado Date: Wed, 28 Oct 2026 23:32:03 +1100 Subject: [PATCH] fix(setup): disclose manual connector steps before approval Fixes #7692 Supersedes #7792 Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com> --- docs/getting-started/installation.md | 5 +- src/application/Setup.planning.test.ts | 39 +++++++++++++ src/application/Setup.ts | 3 + src/application/SetupTypes.ts | 2 + src/cli/interactiveSetup.ts | 4 ++ tests/boundary/cli/cliSetup.test.ts | 76 ++++++++++++++++++++++++-- 6 files changed, 123 insertions(+), 6 deletions(-) diff --git a/docs/getting-started/installation.md b/docs/getting-started/installation.md index 764e22256f..0f14a3ba13 100644 --- a/docs/getting-started/installation.md +++ b/docs/getting-started/installation.md @@ -51,7 +51,10 @@ installation or connection and requires its own explicit approval. It can also save verified paths for an existing Ghidra installation. After selection, review the plan's exact paths and changes and approve before -REA writes files or installs Hopper. You can cancel at any prompt. +REA writes files or installs Hopper. The approval view also shows known manual +completion steps for selected clients, such as Grok Bot's hosted connector. +Approving the local changes does not complete those manual steps. You can cancel +at any prompt. Before applying changes, REA checks your current configuration. The plan lists: diff --git a/src/application/Setup.planning.test.ts b/src/application/Setup.planning.test.ts index 7e58a9c434..1299f6e4a4 100644 --- a/src/application/Setup.planning.test.ts +++ b/src/application/Setup.planning.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import { describe, expect, it } from "vitest"; import { runSetup } from "./Setup.js"; +import { manualRegistrationRemediation } from "./SupportedClients.js"; import type { SetupProgressEvent } from "./SetupTypes.js"; import { FakeSetupHost, options } from "./Setup.fixture.js"; import { SetupSkillInstallError } from "./SetupSkill.js"; @@ -868,3 +869,41 @@ it("records verified skill installation while its staging cleanup remains incomp }); expect(result.remediation).toContain("close failed"); }); + +describe("manual registration approval", () => { + it.each([true, false])( + "discloses manual registration before any write (apply: %s)", + async (approved) => { + const host = new FakeSetupHost(); + host.hopper = "/fixture/Hopper"; + host.doctorHealthy = true; + host.clients = [ + { + name: "grok_bot", + displayName: "Grok Bot", + registrationProfile: supportedClientRegistrationProfile("grok_bot"), + configPath: "/home/user/.grokbot", + format: "unsupported", + }, + ]; + const result = await runSetup( + { ...options(false), clientIds: ["grok_bot"], structured: false }, + host, + async (actions, context) => { + expect(context?.stage).toBe("confirm"); + expect(context).toHaveProperty( + "remediation", + manualRegistrationRemediation("grok_bot"), + ); + expect(actions.map(({ kind }) => kind)).toEqual(["install_skill"]); + expect(host.skillInstalls).toBe(0); + expect(host.configurations).toBe(0); + return approved; + }, + ); + expect(result.status).toBe(approved ? "needs_human" : "cancelled"); + expect(host.skillInstalls).toBe(approved ? 1 : 0); + expect(host.configurations).toBe(0); + }, + ); +}); diff --git a/src/application/Setup.ts b/src/application/Setup.ts index 6586876974..9c4ffdcf5f 100644 --- a/src/application/Setup.ts +++ b/src/application/Setup.ts @@ -403,6 +403,9 @@ const runSetupWorkflow = async ( if (!approved && confirm !== undefined && !options.structured) { const decision = await confirm?.(plannedActions, { stage: "confirm", + ...(manualRemediation === undefined + ? {} + : { remediation: manualRemediation }), clientStates, selectedClientIds, clientSelectionAllowed: false, diff --git a/src/application/SetupTypes.ts b/src/application/SetupTypes.ts index 3615f081ae..b45819cc98 100644 --- a/src/application/SetupTypes.ts +++ b/src/application/SetupTypes.ts @@ -288,6 +288,8 @@ export interface SetupConfirmationContext { readonly clientStates: readonly SetupClientState[]; readonly selectedClientIds: readonly string[]; readonly clientSelectionAllowed: boolean; + /** Known manual completion required after the approved local changes. */ + readonly remediation?: string; } /** Adapter-owned selection and confirmation for the setup workflow. */ diff --git a/src/cli/interactiveSetup.ts b/src/cli/interactiveSetup.ts index 5e79ae74c2..d06b9da163 100644 --- a/src/cli/interactiveSetup.ts +++ b/src/cli/interactiveSetup.ts @@ -59,6 +59,10 @@ export const confirmInteractiveSetup = async ( } if (actions.length === 0) return { approved: true, selectedActionIds: [] }; renderPreflight(actions); + if (context?.remediation !== undefined) { + writeLine("│"); + writeLine(`│ ${context.remediation}`); + } const approved = await confirm({ ...promptStreams, ...(signal === undefined ? {} : { signal }), diff --git a/tests/boundary/cli/cliSetup.test.ts b/tests/boundary/cli/cliSetup.test.ts index 98fe26ee3f..e94881e498 100644 --- a/tests/boundary/cli/cliSetup.test.ts +++ b/tests/boundary/cli/cliSetup.test.ts @@ -16,6 +16,7 @@ import { describe, expect, it, onTestFinished } from "vitest"; import { z } from "zod"; import { parse as parseToml } from "smol-toml"; +import { manualRegistrationRemediation } from "../../../src/application/SupportedClients.js"; import type { SetupResult } from "../../../src/application/SetupTypes.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { cliTest, type TestCli } from "../../support/cli/cliFixture.js"; @@ -225,13 +226,15 @@ cliTest( }, ); -const runSetupDryRunInTty = async ( +const runSetupInTty = async ( home: string, arguments_: readonly string[], + approval?: boolean, + environment: NodeJS.ProcessEnv = {}, ): Promise<{ readonly exitCode: number; readonly output: string }> => { const terminal = spawn( process.execPath, - ["scripts/rea.mjs", "setup", "--dry-run", ...arguments_], + ["scripts/rea.mjs", "setup", ...arguments_], { cwd: process.cwd(), env: { @@ -241,6 +244,7 @@ const runSetupDryRunInTty = async ( XDG_CONFIG_HOME: join(home, ".config"), REA_LOG_LEVEL: "silent", NO_COLOR: "1", + ...environment, }, name: "xterm-256color", cols: 120, @@ -248,13 +252,22 @@ const runSetupDryRunInTty = async ( }, ); let output = ""; + let answered = false; terminal.onData((data) => { output += data; + if ( + approval !== undefined && + !answered && + output.includes("Apply this change?") + ) { + answered = true; + terminal.write(approval ? "\r" : "\u001b[C\r"); + } }); const exitCode = await new Promise((resolvePromise, reject) => { const timeout = setTimeout(() => { terminal.kill(); - reject(new Error(`TTY dry run timed out after output:\n${output}`)); + reject(new Error(`TTY setup timed out after output:\n${output}`)); }, 15_000); terminal.onExit(({ exitCode: code }) => { clearTimeout(timeout); @@ -264,10 +277,62 @@ const runSetupDryRunInTty = async ( return { exitCode, output }; }; +describe("manual connector approval", () => { + it.skipIf(process.platform === "win32").each([true, false])( + "discloses the manual step before approval (apply: %s)", + async (approved) => { + const home = await createTestTempDirectory("rea-cli-setup-manual-"); + const bin = join(home, "bin"); + await mkdir(bin); + const hopper = join(bin, "hopper"); + await writeFile( + hopper, + "#!/bin/sh\necho 'Hopper Disassembler v6.0.0'\n", + { mode: 0o755 }, + ); + for (const runner of ["npx", "npm", "bun"]) + await writeFile(join(bin, runner), "#!/bin/sh\nexit 99\n", { + mode: 0o755, + }); + const { output, exitCode } = await runSetupInTty( + home, + ["--client", "grok_bot"], + approved, + { + PATH: `${bin}:${process.env.PATH ?? ""}`, + HOPPER_LAUNCHER_PATH: hopper, + DSH_HOME: join(home, ".dsh"), + DSH_AGENTS_HOME: join(home, ".dsh-agents"), + XDG_DATA_HOME: join(home, ".local/share"), + XDG_CACHE_HOME: join(home, ".cache"), + XDG_STATE_HOME: join(home, ".local/state"), + }, + ); + const prompt = output.indexOf("Apply this change?"); + const remediation = manualRegistrationRemediation("grok_bot"); + expect(remediation).toBeDefined(); + expect(prompt, output).toBeGreaterThan(0); + expect(output.slice(0, prompt)).toContain(remediation); + const skill = join( + home, + ".agents/skills/reverse-engineer-anything/SKILL.md", + ); + if (approved) { + expect(exitCode, output).toBe(1); + expect(await readFile(skill, "utf8")).not.toBe(""); + } else { + expect(exitCode, output).toBe(0); + await expect(access(skill)).rejects.toMatchObject({ code: "ENOENT" }); + } + }, + ); +}); + describe("TTY setup dry run", () => { it("prints the plan and no-mutation notice for a TTY dry run", async () => { const home = await createTestTempDirectory("rea-cli-setup-dry-run-"); - const { exitCode, output } = await runSetupDryRunInTty(home, [ + const { exitCode, output } = await runSetupInTty(home, [ + "--dry-run", "--client", "codex", "--skill=false", @@ -298,7 +363,8 @@ describe("TTY setup dry run", () => { process.platform === "win32" ? "junction" : "dir", ); - const { exitCode, output } = await runSetupDryRunInTty(home, [ + const { exitCode, output } = await runSetupInTty(home, [ + "--dry-run", "--client", "claude_code", ]);