fix(system-agent): apply approved proposal exactly once (#119389)

Co-authored-by: Chris Davidson <chris.davidson.47@gmail.com>

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 () => {

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<SystemAgentChatReply> {
     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;

src/system-agent/operator-approval.ts

@@ -24,9 +24,9 @@ export function resolvePendingOperatorProposal(
 export async function resolveOperatorApprovalDecision<T>(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<T>;
+  apply: (operation: SystemAgentOperation) => Promise<T>;
   denied: () => T;
 }): Promise<T | null> {
   const proposal = params.getProposal();
@@ -37,7 +37,8 @@ export async function resolveOperatorApprovalDecision<T>(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);
 }