mirror of
https://github.com/morluto/rea.git
synced 2026-11-14 18:52:48 +00:00
fix(setup): disclose manual connector steps before approval
Fixes #7692 Supersedes #7792 Co-Authored-By: gensokyo-dev <342580529+gensokyo-dev@users.noreply.github.com>
This commit is contained in:
1 parent
4e8e1f5346
commit
4d02bc82b3
6 files changed
+123
-6
No files matched your search
@@ -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:
|
||||
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -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,
|
||||
|
||||
@@ -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. */
|
||||
|
||||
@@ -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 }),
|
||||
|
||||
@@ -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<number>((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",
|
||||
]);
|
||||
|
||||
Reference in new issue
Block a user