fix(gate): reviewer_confirm requires explicit approve; free-text is actionable, not accidental approve
This commit is contained in:
@@ -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");
|
||||
});
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user