diff --git a/harness/.pi/extensions/gate/__tests__/gate_confirm_autoclose.test.js b/harness/.pi/extensions/gate/__tests__/gate_confirm_autoclose.test.js new file mode 100644 index 00000000..27f5c7ce --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_confirm_autoclose.test.js @@ -0,0 +1,106 @@ +// F6/F7 auto-close: where completeness is machine-detectable the LAST substantive +// approval closes the phase itself — approving the final CTE of the plan (F6) and +// approving the SQL (F7) must run `tht phase advance` without a separate +// reviewer_confirm kind:"phase"; a non-final CTE must NOT advance. +// +// HARNESS NOTE: the gate's ESM `import { execFileSync }` is snapshotted from the CJS +// namespace at first require — later reassignments of cp.execFileSync are NOT seen. +// Install ONE dispatcher before requiring the gate; tests swap its `current` handler. +const test = require("node:test"); +const assert = require("node:assert"); +const cp = require("node:child_process"); +const { createRequire } = require("node:module"); +const path = require("node:path"); + +const GATE = path.join(__dirname, "..", "..", "tht-gate.js"); +if (typeof globalThis.require === "undefined") { + globalThis.require = createRequire(GATE); +} + +const shell = { current: () => "" }; +cp.execFileSync = (file, args, opts) => shell.current(file, args, opts); + +const META = JSON.stringify({ + max_phase: 8, + phases: [ + { num: 6, id: "F6" }, + { num: 7, id: "F7" }, + { num: 8, id: "F8" }, + ], +}); + +// `cte next` walks the queue one entry per `decision add cte_approved`. +function useShell({ phase, cteQueue = [] }) { + const calls = []; + const queue = [...cteQueue]; + shell.current = (file, args) => { + calls.push(args.join(" ")); + if (args[0] === "phase" && args[1] === "meta") return META; + if (args[0] === "phase" && args[1] === "show") return `Fase corrente: ${phase}\n`; + if (args[0] === "cte" && args[1] === "next") return queue.length ? `${queue[0]}\n` : ""; + if (args[0] === "decision" && args[1] === "add" && args.includes("cte_approved")) { + queue.shift(); + return ""; + } + return ""; + }; + return calls; +} + +async function runConfirm(kind, artifactKind) { + const gate = require(GATE); + const { createFakePi } = require("./fake_pi_runtime.js"); + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = "/nonexistent-thothii-test-cwd"; + gate.default(pi); + // Reset the module-level phase-meta cache captured by a previous test's handler. + await pi.emit("session_start", {}); + ctx.ui.input = async (title) => { + const d = JSON.parse(title); + return JSON.stringify({ id: d.id, kind: "artifact-gate", choices: ["approve"] }); + }; + return tools.get("reviewer_confirm").def.execute( + "call-1", + { session: "s1", kind, title: "t", artifact: { kind: artifactKind, data: "# md" } }, + null, + null, + ctx, + ); +} + +test("approving a NON-final CTE records it and does NOT advance the phase", async () => { + const calls = useShell({ phase: 6, cteQueue: ["cte_a", "cte_b"] }); + const result = await runConfirm("cte_result", "cte_result"); + assert.match(result.content[0].text, /cte_a.*approvato/i); + assert.match(result.content[0].text, /cte_b/); + assert.ok(!calls.some((c) => c.startsWith("phase advance")), `no advance in: ${calls}`); +}); + +test("approving the LAST CTE of the plan closes F6 automatically", async () => { + const calls = useShell({ phase: 6, cteQueue: ["cte_finale"] }); + const result = await runConfirm("cte_result", "cte_result"); + assert.match(result.content[0].text, /cte_finale.*approvato/i); + assert.match(result.content[0].text, /chiusa automaticamente/); + assert.match(result.content[0].text, /NON presentare reviewer_confirm/); + assert.ok( + calls.some((c) => c === "phase advance --session s1"), + `expected phase advance in: ${calls}`, + ); + assert.ok(!calls.some((c) => c.startsWith("session finalize")), "F6 must not finalize"); +}); + +test("approving the SQL closes F7 automatically (no separate phase gate)", async () => { + const calls = useShell({ phase: 7 }); + const result = await runConfirm("sql", "sql"); + assert.match(result.content[0].text, /SQL approvato/); + assert.match(result.content[0].text, /chiusa automaticamente/); + assert.ok( + calls.some((c) => c.includes("decision add --session s1 --type sql_approved")), + `expected sql_approved in: ${calls}`, + ); + assert.ok( + calls.some((c) => c === "phase advance --session s1"), + `expected phase advance in: ${calls}`, + ); + assert.ok(!calls.some((c) => c.startsWith("session finalize")), "F7 must not finalize"); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 599469ce..fc43906d 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -1299,7 +1299,28 @@ export default function (pi) { "", ); if (err) return err; - return textResult(`CTE '${cteName}' approvato (sessione ${session}).`); + // Deterministic completeness: when no CTE remains in the plan the reviewer + // has approved everything F6 can ask — close the phase here instead of + // echoing a reviewer_confirm kind:"phase" that decides nothing new. + const remaining = tht(ctx, ["cte", "next", "--session", session]).trim(); + if (remaining) { + return textResult( + `CTE '${cteName}' approvato (sessione ${session}). Prossimo CTE del piano: '${remaining}'.`, + ); + } + const closed = advancePhaseAndFinalize(ctx, session, curNum); + if (closed.err) return closed.err; + if (closed.finalized) { + lockActive = false; + lastSteered = false; + return textResult( + `CTE '${cteName}' approvato — era l'ultimo del piano: fase chiusa e sessione finalizzata (${session}).`, + ); + } + return textResult( + `CTE '${cteName}' approvato — era l'ultimo del piano: Fase ${curNum} chiusa automaticamente. ` + + 'NON presentare reviewer_confirm kind:"phase" per questa fase; prosegui con la successiva.', + ); } if (kind === "sql") { const err = relayIfThtFails( @@ -1317,7 +1338,19 @@ export default function (pi) { "", ); if (err) return err; - return textResult(`SQL approvato (sessione ${session}).`); + // F7's only prerequisite IS sql_approved (workflow.yaml): the phase gate + // after it decides nothing new — close the phase here. + const closed = advancePhaseAndFinalize(ctx, session, curNum); + if (closed.err) return closed.err; + if (closed.finalized) { + lockActive = false; + lastSteered = false; + return textResult(`SQL approvato — fase chiusa e sessione finalizzata (${session}).`); + } + return textResult( + `SQL approvato — Fase ${curNum} chiusa automaticamente. ` + + 'NON presentare reviewer_confirm kind:"phase" per questa fase; prosegui con la successiva (datamart/memorie).', + ); } return textResult(`Approvato (kind=${kind}, sessione ${session}).`); } catch (fatal) { diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 77431a13..24ef3773 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -26,13 +26,16 @@ about a domain term, ask the reviewer. ## Phase map (advance cheat-sheet) A phase advances ONLY when a `phase_approved:phase:N` decision is recorded for the current -phase — written by `reviewer_confirm kind:"phase"`. (F7 is two-step: `kind:"sql"` records -`sql_approved`, then `kind:"phase"` advances.) A `reviewer_decide`/ +phase. THE RULE: where completeness is machine-detectable, the LAST substantive approval +closes the phase itself; a `reviewer_confirm kind:"phase"` summary gate exists only where +completeness is a human judgment (F1, F2 with recorded memories, F5). A `reviewer_decide`/ `reviewer_select` choice records its OWN decision but does NOT advance the phase. `advance:true` on `reviewer_decide` auto-advances only F2 (empty memory) and F6 (skipped/empty) — never a -phase that recorded substantive decisions. Exactly three gates close their phase themselves, -because there the human interaction IS the phase approval: `rewrite_question` (F3), -`reviewer_schema_linking` with `advance:true` (F4), and `reviewer_memory_promote` (F8). +phase that recorded substantive decisions. FIVE gates close their phase themselves, because +there the human interaction IS the phase approval: `rewrite_question` (F3), +`reviewer_schema_linking` with `advance:true` (F4), the LAST `reviewer_confirm +kind:"cte_result"` of the plan (F6), `reviewer_confirm kind:"sql"` (F7), and +`reviewer_memory_promote` (F8). | Phase | Artifact out | Advance / close by | |-------|--------------|--------------------| @@ -41,8 +44,8 @@ because there the human interaction IS the phase approval: `rewrite_question` (F | F3 riscrittura | `question.md` | `rewrite_question` records approval and advances automatically | | F4 schema_linking | `schema_linking.json` | `reviewer_schema_linking` with `advance:true` closes the phase itself (the curation IS the approval; `reviewer_confirm kind:"phase"` only as fallback if it reports an error). Promoted columns are the reviewer-approved OUTPUT columns — project exactly those in the final SELECT. | | F5 sintesi | — | `reviewer_confirm kind:"phase"` (after `tht session check`) | -| F6 cte | `cte_plan.json`, `ctes/`, `cte_tests.json` | approve each CTE `kind:"cte_result"`, then `reviewer_confirm kind:"phase"` | -| F7 sql_finale | `sql_final.sql` | `kind:"sql"` records `sql_approved`, then `reviewer_confirm kind:"phase"` | +| F6 cte | `cte_plan.json`, `ctes/`, `cte_tests.json` | approve each CTE with `kind:"cte_result"`; approving the LAST CTE of the plan closes the phase automatically (`kind:"phase"` only as fallback if the auto-close reports an error) | +| F7 sql_finale | `sql_final.sql` | `kind:"sql"` records `sql_approved` AND closes the phase automatically (`kind:"phase"` only as fallback if it reports an error) | | F8 datamart | — | auto: the `reviewer_memory_promote` gate advances F8 and finalizes the session itself (`reviewer_confirm kind:"phase"` only as fallback if it reports an error) | ## Disciplines (hold in every phase) @@ -51,17 +54,17 @@ because there the human interaction IS the phase approval: `rewrite_question` (F command (you invoke `tht ...` via the shell tool). NEVER run `tht phase advance` or `tht decision add` from the shell — they are blocked by the gate's anti-bypass hook; the gate extension records every decision via the `reviewer_*` tools. -2. **The choice records; the phase gate advances.** A `reviewer_decide`, or a +2. **The choice records; the closing gate advances.** A `reviewer_decide`, or a `reviewer_select` whose chosen option carries a `decision`, PERSISTS that decision — it - does NOT by itself advance the phase. To move to the next phase you MUST issue - `reviewer_confirm kind:"phase"` (for F7, first `kind:"sql"` to record `sql_approved`, then - `kind:"phase"` to advance), the deliberate "this phase is - done" gate. The `advance:true` flag on `reviewer_decide` is a shortcut that auto-advances - ONLY F2 when the memory phase recorded nothing and F6 when it is skipped/empty; everywhere - else it is a silent no-op, so never rely on it to advance. The self-closing gates are the - three listed above (F3 `rewrite_question`, F4 `reviewer_schema_linking` `advance:true`, - F8 `reviewer_memory_promote`) — after one of those, do NOT add a `reviewer_confirm` that - merely echoes it; the phase is already closed. + does NOT by itself advance the phase. In F1, F2-with-memories and F5 the phase closes + with the deliberate `reviewer_confirm kind:"phase"` summary gate. The `advance:true` + flag on `reviewer_decide` is a shortcut that auto-advances ONLY F2 when the memory phase + recorded nothing and F6 when it is skipped/empty; everywhere else it is a silent no-op, + so never rely on it to advance. The self-closing gates are the five listed above + (F3 `rewrite_question`, F4 `reviewer_schema_linking` `advance:true`, F6 last + `kind:"cte_result"`, F7 `kind:"sql"`, F8 `reviewer_memory_promote`) — after one of + those, do NOT add a `reviewer_confirm kind:"phase"` that merely echoes it; the phase is + already closed. 3. **Single pick vs multi-answer.** For a single-pick clarification or decision, use `reviewer_select` and attach a `decision` payload (`{type, subject, detail?, rationale?}`) to each concrete option: picking it persists that decision directly —