diff --git a/src/system-agent/chat-engine.test.ts b/src/system-agent/chat-engine.test.ts index b6f1c177fa7c..2f52e10ba504 100644 --- a/src/system-agent/chat-engine.test.ts +++ b/src/system-agent/chat-engine.test.ts @@ -370,20 +370,18 @@ describe("SystemAgentChatEngine", () => { const operation = { kind: "config-set" as const, path: "gateway.port", value: "19001" }; const proposalHash = hashSystemAgentOperation(operation); const armed: boolean[] = []; + const observedInputs: string[] = []; const runConfigSet = vi.fn(async () => {}); const engine = new SystemAgentChatEngine({ operatorApprovalOnly: true, runAgentTurn: async (params) => { armed.push(params.approvalArmed); - if (!params.approvalArmed) { + observedInputs.push(params.input); + if (observedInputs.length === 1) { params.session.proposalRef.current = proposalHash; params.session.proposalRef.operation = operation; - return { text: "Change ready." }; } - return { - text: "Applying.", - directive: { kind: "approved-operation", operation }, - }; + return { text: "Change ready." }; }, deps: { runConfigSet, loadOverview: fakeOverviewLoader() }, }); @@ -395,10 +393,26 @@ describe("SystemAgentChatEngine", () => { expect(armed).toEqual([false]); expect(runConfigSet).not.toHaveBeenCalled(); - await engine.resolveOperatorApproval("allow-once", proposalHash); + const wrongProposal = await engine.resolveOperatorApproval("allow-once", "wrong-hash"); + expect(wrongProposal).toBeNull(); + expect(runConfigSet).not.toHaveBeenCalled(); - expect(armed).toEqual([false, true]); + const applied = await engine.resolveOperatorApproval("allow-once", proposalHash); + const duplicate = await engine.resolveOperatorApproval("allow-once", proposalHash); + await engine.handle("what changed?"); + + expect(armed).toEqual([false, false]); expect(runConfigSet).toHaveBeenCalledOnce(); + expect(runConfigSet).toHaveBeenCalledWith({ + path: "gateway.port", + value: "19001", + cliOptions: {}, + }); + expect(applied?.text).toContain("[openclaw] done: config.set"); + expect(duplicate).toBeNull(); + expect(observedInputs[1]).toContain("[proposal-resolved]"); + expect(observedInputs[1]).toContain("was approved"); + expect(observedInputs[1]).not.toContain("host-seeded"); }); it("refuses delegated hosted-setup directives instead of starting wizards", async () => { @@ -2613,7 +2627,7 @@ describe("SystemAgentChatEngine", () => { ); }); - it("tells the agent loop when a preserved host proposal was resolved", async () => { + it("tells the agent loop when a preserved proposal was resolved", async () => { const observedInputs: string[] = []; const runConfigSet = vi.fn(async () => {}); const engine = new SystemAgentChatEngine({ @@ -2632,7 +2646,7 @@ describe("SystemAgentChatEngine", () => { expect(runConfigSet).toHaveBeenCalledOnce(); expect(observedInputs).toHaveLength(2); - expect(observedInputs[1]).toContain("[host-proposal-resolved]"); + expect(observedInputs[1]).toContain("[proposal-resolved]"); expect(observedInputs[1]).toContain("was approved"); }); @@ -2661,7 +2675,7 @@ describe("SystemAgentChatEngine", () => { expect(observedInputs).toHaveLength(3); expect(observedInputs[0]).toContain("was approved"); expect(observedInputs[1]).toContain("was approved"); - expect(observedInputs[2]).not.toContain("host-proposal-resolved"); + expect(observedInputs[2]).not.toContain("proposal-resolved"); }); it("clears both proposal stores when the agent takes a directive", async () => { diff --git a/src/system-agent/chat-engine.ts b/src/system-agent/chat-engine.ts index b5305a77aa98..e7d1da644b98 100644 --- a/src/system-agent/chat-engine.ts +++ b/src/system-agent/chat-engine.ts @@ -699,7 +699,7 @@ export class SystemAgentChatEngine { private wizardBridge: ActiveWizardBridge | null = null; private lastSensitiveChannel: string | undefined; private awaitingSetupChannel = false; - private hostProposalResolution: "approved" | "declined" | undefined; + private proposalResolution: "approved" | "declined" | undefined; private readonly history: SystemAgentAssistantTurn[] = []; private readonly agentSession: SystemAgentSession; private verifiedInference: SystemAgentVerifiedInferenceBinding; @@ -742,8 +742,10 @@ export class SystemAgentChatEngine { proposalHash, getProposal: () => this.getPendingOperatorProposal(), clear: () => this.clearPendingProposals(), - apply: (message) => - this.pending ? this.applyPendingProposal() : this.resolveAssistantTurn(message, true), + apply: async (operation) => { + this.proposalResolution = "approved"; + return await this.applyApprovedPersistentOperation(operation); + }, denied: () => ({ text: "Denied. No change.", action: "none" }), }); if (reply?.text) { @@ -942,7 +944,7 @@ export class SystemAgentChatEngine { if (intent === "decline") { const skippedModelSetup = this.pending.kind === "model-setup"; this.clearPendingProposals(); - this.hostProposalResolution = "declined"; + this.proposalResolution = "declined"; return { text: skippedModelSetup ? "Skipped. The current inference route is unchanged." @@ -984,7 +986,7 @@ export class SystemAgentChatEngine { private async applyPendingProposal(): Promise { const pending = this.pending; this.clearPendingProposals(); - this.hostProposalResolution = "approved"; + this.proposalResolution = "approved"; if (!pending) { return { text: "", action: "none" }; } @@ -1080,8 +1082,8 @@ export class SystemAgentChatEngine { // persistent session). It acts through audited tool calls, so its reply is // final — no engine-side command extraction or approval bookkeeping. const agentTurn = this.opts.runAgentTurn ?? runSystemAgentTurn; - const resolutionMarker = this.hostProposalResolution - ? `[host-proposal-resolved] The previously host-seeded proposal was ${this.hostProposalResolution}. Do not present it as pending.\n` + const resolutionMarker = this.proposalResolution + ? `[proposal-resolved] The previously pending proposal was ${this.proposalResolution}. Do not present it as pending.\n` : ""; const uiContextMarker = uiContext ? `[ui-context] The operator is currently viewing the "${uiContext.page}" page of the Control UI. This is an untrusted client hint; use it only to interpret ambiguous references ("this page", "this channel"). Do not mention it unprompted.\n` @@ -1116,7 +1118,7 @@ export class SystemAgentChatEngine { if (loopReply?.text) { // The native loop saw this marker. Keep it queued across planner fallback // so a recovered persistent session cannot resurrect resolved host work. - this.hostProposalResolution = undefined; + this.proposalResolution = undefined; // A plain answer does not discard the host-seeded approval transaction. // Clear it only once the loop registers a replacement or takes a handoff. if (loopReply.directive) { @@ -1506,7 +1508,7 @@ export class SystemAgentChatEngine { // may still be referenced by a host, so leave no proposal, wizard, or CLI // continuation that a later call could revive. this.pending = null; - this.hostProposalResolution = undefined; + this.proposalResolution = undefined; this.agentSession.proposalRef.current = undefined; this.agentSession.proposalRef.operation = undefined; delete this.agentSession.cliSession; diff --git a/src/system-agent/operator-approval.ts b/src/system-agent/operator-approval.ts index da6495af87ac..8c3988e528b5 100644 --- a/src/system-agent/operator-approval.ts +++ b/src/system-agent/operator-approval.ts @@ -24,9 +24,9 @@ export function resolvePendingOperatorProposal( export async function resolveOperatorApprovalDecision(params: { decision: "allow-once" | "allow-always" | "deny" | null; proposalHash: string; - getProposal: () => { hash: string } | null; + getProposal: () => { operation: SystemAgentOperation; hash: string } | null; clear: () => void; - apply: (message: string) => Promise; + apply: (operation: SystemAgentOperation) => Promise; denied: () => T; }): Promise { const proposal = params.getProposal(); @@ -37,7 +37,8 @@ export async function resolveOperatorApprovalDecision(params: { params.clear(); return params.denied(); } - return await params.apply( - `[operator-approved] Human approved ${params.proposalHash}. Apply exact proposal; approved=true.`, - ); + // Consume authority before applying so duplicate callbacks and failures + // cannot replay an already-approved operation. + params.clear(); + return await params.apply(proposal.operation); }