From 644634d49205f0e91599d7788b9d9f002e9bcd62 Mon Sep 17 00:00:00 2001 From: mptyl Date: Thu, 2 Jul 2026 18:30:14 +0200 Subject: [PATCH] fix(gate): reviewer_confirm requires explicit approve; free-text is actionable, not accidental approve --- .../__tests__/gate_confirm_outcome.test.js | 27 ++++++++++++++ harness/.pi/extensions/tht-gate.js | 37 +++++++++++++++---- 2 files changed, 56 insertions(+), 8 deletions(-) create mode 100644 harness/.pi/extensions/gate/__tests__/gate_confirm_outcome.test.js diff --git a/harness/.pi/extensions/gate/__tests__/gate_confirm_outcome.test.js b/harness/.pi/extensions/gate/__tests__/gate_confirm_outcome.test.js new file mode 100644 index 00000000..51b2b9cb --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_confirm_outcome.test.js @@ -0,0 +1,27 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const { resolveConfirmOutcome } = require("../../tht-gate.js"); + +test("explicit approve choice resolves to approve", () => { + assert.deepEqual(resolveConfirmOutcome({ id: "u1", choices: ["approve"] }), { kind: "approve" }); +}); + +test("reject choice resolves to reject", () => { + assert.deepEqual(resolveConfirmOutcome({ id: "u1", choices: ["reject"] }), { kind: "reject" }); +}); + +test("freetext control is actionable, carries text, and is NOT approve", () => { + const out = resolveConfirmOutcome({ id: "u1", control: "freetext", text: "aggiungi la tabella X" }); + assert.equal(out.kind, "freetext"); + assert.equal(out.text, "aggiungi la tabella X"); +}); + +test("back/exit controls resolve to their kinds", () => { + assert.equal(resolveConfirmOutcome({ control: "back" }).kind, "back"); + assert.equal(resolveConfirmOutcome({ control: "exit" }).kind, "exit"); +}); + +test("an unrecognized response never approves (kind: unknown)", () => { + assert.equal(resolveConfirmOutcome({ id: "u1", control: "other" }).kind, "unknown"); + assert.equal(resolveConfirmOutcome({ id: "u1", choices: [] }).kind, "unknown"); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index c39c817b..66b4938b 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -253,6 +253,20 @@ export function resolveSelectOutcome(opts, resp) { return { kind: "choice", option, choice }; } +// Classifies a reviewer_confirm response. Advance happens ONLY on an explicit +// "approve" choice; a bare/unknown response resolves to {kind:"unknown"} and is +// re-presented (never an accidental approve). freetext is actionable feedback, +// distinct from an explicit "reject". +export function resolveConfirmOutcome(resp) { + if (resp?.control === "freetext") return { kind: "freetext", text: resp.text }; + if (resp?.control === "back") return { kind: "back" }; + if (resp?.control === "exit") return { kind: "exit" }; + const choice = selectedChoice(resp); + if (choice === "approve") return { kind: "approve" }; + if (choice === "reject") return { kind: "reject" }; + return { kind: "unknown", choice }; +} + export async function emitAndWait(ctx, descriptor) { for (;;) { const value = await ctx.ui.input(JSON.stringify(descriptor), ""); @@ -582,17 +596,24 @@ export default function (pi) { artifact, action: { kind: "approve_reject", prompt: "Approvi o rifiuti?" }, }); - const resp = await emitAndWait(ctx, widget); - if (resp.control === "freetext" || selectedChoice(resp) === "reject") { - return textResult( - `Rifiutato${resp.text ? ` (motivo: ${resp.text})` : ""}: rivedi e riprova.`, - ); + let outcome; + for (;;) { + const resp = await emitAndWait(ctx, widget); + outcome = resolveConfirmOutcome(resp); + if (outcome.kind !== "unknown") break; + await ctx.ui.notify("Scegli «Conferma e prosegui» o «Rifiuta».", "warning"); } - if (resp.control === "back") + if (outcome.kind === "freetext") + return textResult( + `Altro (reviewer): ${outcome.text}. Valuta e agisci, poi ri-presenta il gate.`, + ); + if (outcome.kind === "reject") + return textResult("Rifiutato: rivedi e riprova."); + if (outcome.kind === "back") return textResult("Il reviewer vuole tornare indietro."); - if (resp.control === "exit") + if (outcome.kind === "exit") return textResult("Il reviewer vuole uscire."); - // approved -> execute the privileged action via the CLI. + // outcome.kind === "approve" -> execute the privileged action via the CLI. if (kind === "phase") { // Explicit human approval: advance unconditionally except for unmet // prerequisites. Plain `phase advance` (no --auto) enforces advance_problems