diff --git a/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js b/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js new file mode 100644 index 00000000..8acc417a --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_stringified_params.test.js @@ -0,0 +1,48 @@ +// #2 robustness: some models send object params (Type.Object / Type.Any) as +// stringified JSON, which fails validation before execute() and makes the model +// loop. prepareReviewerArguments must coerce them back; the anti-bypass hook must +// block the model from editing the gate code itself. +const test = require("node:test"); +const assert = require("node:assert"); +const { + jsonObjectOrSelf, + prepareReviewerArguments, + GATE_CODE_FILES, +} = require("../../tht-gate.js"); + +test("jsonObjectOrSelf parses a stringified object/array, leaves plain strings intact", () => { + assert.deepEqual(jsonObjectOrSelf('{"a":1}'), { a: 1 }); + assert.deepEqual(jsonObjectOrSelf('[1,2]'), [1, 2]); + assert.equal(jsonObjectOrSelf("SELECT 1"), "SELECT 1"); // not JSON -> unchanged + assert.equal(jsonObjectOrSelf('"just a string"'), '"just a string"'); // JSON string primitive -> unchanged + const obj = { kind: "text" }; + assert.equal(jsonObjectOrSelf(obj), obj); // already an object -> same reference +}); + +test("prepareReviewerArguments coerces a stringified artifact into an object", () => { + const out = prepareReviewerArguments({ + session: "s", + artifact: JSON.stringify({ kind: "text", data: { recap: "x" }, version: 1 }), + }); + assert.equal(typeof out.artifact, "object"); + assert.equal(out.artifact.kind, "text"); + assert.equal(out.artifact.data.recap, "x"); +}); + +test("prepareReviewerArguments leaves an object artifact and parses stringified options", () => { + const art = { kind: "text", data: {} }; + const out = prepareReviewerArguments({ + artifact: art, + options: JSON.stringify([{ id: "a", label: "A" }]), + }); + assert.deepEqual(out.artifact, art); + assert.ok(Array.isArray(out.options)); + assert.equal(out.options[0].id, "a"); +}); + +test("GATE_CODE_FILES blocks writes to gate extensions, not session artifacts", () => { + assert.ok(GATE_CODE_FILES.test("harness/.pi/extensions/tht-gate.js")); + assert.ok(GATE_CODE_FILES.test(".pi/extensions/reserved-labels.mjs")); + assert.equal(GATE_CODE_FILES.test("sessions/s1/schema_linking.json"), false); + assert.equal(GATE_CODE_FILES.test("sessions/s1/sql_final.sql"), false); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 2ff2fa2d..83f78ca9 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -34,7 +34,20 @@ import { isReserved } from "./reserved-labels.mjs"; // --- prepareArguments: parse stringified arrays (workaround for models that send // arrays as JSON strings -- same pattern as pi-core's edit tool prepareEditArguments) -function prepareReviewerArguments(input) { +// Coerce a value that may arrive as a JSON-encoded string back into an object/array. +// Models sometimes stringify object params (Type.Object / Type.Any); the parsed value +// is returned only when it is genuinely an object, so plain strings pass through intact. +export function jsonObjectOrSelf(value) { + if (typeof value !== "string") return value; + try { + const parsed = JSON.parse(value); + return parsed && typeof parsed === "object" ? parsed : value; + } catch { + return value; + } +} + +export function prepareReviewerArguments(input) { if (!input || typeof input !== "object") return input; const args = { ...input }; // Parse stringified JSON arrays (workaround for models that send arrays as strings) @@ -54,6 +67,10 @@ function prepareReviewerArguments(input) { /* not JSON */ } } + // reviewer_confirm's `artifact` is a Type.Object; some models send it as a + // stringified JSON object, which fails validation BEFORE execute() and makes the + // model loop. Coerce it back to an object so the gate proceeds. + if (args.artifact !== undefined) args.artifact = jsonObjectOrSelf(args.artifact); return args; } @@ -65,6 +82,10 @@ const FORBIDDEN = [ ]; const PROTECTED_FILES = /(review_decisions\.jsonl|session_manifest\.yaml|cte_plan\.json)/; +// The model orchestrates the workflow; it must never rewrite the gate/extension code +// itself. pi's write/edit tools are otherwise unrestricted, so a confused model can +// (and did) patch tht-gate.js mid-loop. Block any write under the extensions dir. +export const GATE_CODE_FILES = /\.pi[\\/]extensions[\\/]/; // --- kickoff payloads (verbatim from source L184-212, load-bearing model prose) - const NUOVA_DOMANDA_KICKOFF = @@ -332,6 +353,14 @@ export default function (pi) { } if (event.toolName === "write" || event.toolName === "edit") { const path = event.input?.path ?? event.input?.file_path ?? ""; + if (GATE_CODE_FILES.test(path)) { + return { + block: true, + reason: + "Il codice del gate (.pi/extensions) non va modificato: sei l'orchestratore " + + "del workflow, non uno sviluppatore del gate. Usa i tool del gate.", + }; + } if (PROTECTED_FILES.test(path)) { return { block: true, @@ -758,7 +787,10 @@ export default function (pi) { }), async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; - const { session, schema_linking } = params; + const { session } = params; + // schema_linking is Type.Any(): a stringified JSON object passes validation + // but would be double-encoded here and rejected by the CLI. Normalize first. + const schema_linking = jsonObjectOrSelf(params.schema_linking); try { tht( ctx,