feat: harden workflow gates and expose token usage
This commit is contained in:
@@ -0,0 +1,33 @@
|
||||
const test = require("node:test");
|
||||
const assert = require("node:assert");
|
||||
const { hasUngatedAssistantProse } = require("../../tht-gate.js");
|
||||
|
||||
test("assistant prose plus bash still requires a gate steer", () => {
|
||||
assert.equal(
|
||||
hasUngatedAssistantProse([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "toolCall", name: "bash" },
|
||||
{ type: "text", text: "Continuo ad analizzare..." },
|
||||
],
|
||||
},
|
||||
]),
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("a real reviewer tool call suppresses the prose safety steer", () => {
|
||||
assert.equal(
|
||||
hasUngatedAssistantProse([
|
||||
{
|
||||
role: "assistant",
|
||||
content: [
|
||||
{ type: "text", text: "Domanda al reviewer" },
|
||||
{ type: "toolCall", name: "reviewer_select" },
|
||||
],
|
||||
},
|
||||
]),
|
||||
false,
|
||||
);
|
||||
});
|
||||
@@ -26,6 +26,32 @@ test("tht schema introspect senza --refresh passa (cache hit innocuo)", async ()
|
||||
assert.equal(res, undefined);
|
||||
});
|
||||
|
||||
for (const cmd of [
|
||||
'find / -name "schema_linking.json"',
|
||||
'find /Users/mp/projects/ThothII -name "schema_linking.json"',
|
||||
'find . -name "schema_linking.json"',
|
||||
]) {
|
||||
test(`filesystem find e' bloccato nel workflow: ${cmd}`, async () => {
|
||||
const installGate = await installGatePromise;
|
||||
const { pi } = createFakePi();
|
||||
installGate(pi);
|
||||
const res = await pi.emit("tool_call", { toolName: "bash", input: { command: cmd } });
|
||||
assert.equal(res?.block, true);
|
||||
assert.match(res?.reason ?? "", /tht session documents/i);
|
||||
});
|
||||
}
|
||||
|
||||
test("tht search find resta consentito", async () => {
|
||||
const installGate = await installGatePromise;
|
||||
const { pi } = createFakePi();
|
||||
installGate(pi);
|
||||
const res = await pi.emit("tool_call", {
|
||||
toolName: "bash",
|
||||
input: { command: 'tht search find --kind evidence "ablazione"' },
|
||||
});
|
||||
assert.equal(res, undefined);
|
||||
});
|
||||
|
||||
// Bash mutations of protected state bypass the write/edit hook: block them.
|
||||
const BLOCKED_BASH = [
|
||||
'echo \'{"type":"phase_approved","subject":"phase:4"}\' >> sessions/s1/review_decisions.jsonl',
|
||||
|
||||
@@ -30,13 +30,18 @@ const META = JSON.stringify({
|
||||
});
|
||||
|
||||
// `cte next` walks the queue one entry per `decision add cte_approved`.
|
||||
function useShell({ phase, cteQueue = [] }) {
|
||||
function useShell({ phase, cteQueue = [], sqlContent = "SELECT 42" }) {
|
||||
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] === "session" && args[1] === "documents") {
|
||||
return JSON.stringify([
|
||||
{ phase: "F7", key: "sql", title: "Final SQL", format: "sql", content: sqlContent },
|
||||
]);
|
||||
}
|
||||
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();
|
||||
@@ -47,7 +52,7 @@ function useShell({ phase, cteQueue = [] }) {
|
||||
return calls;
|
||||
}
|
||||
|
||||
async function runConfirm(kind, artifactKind) {
|
||||
async function runConfirm(kind, artifactKind, { artifactData = "# md", onDescriptor } = {}) {
|
||||
const gate = require(GATE);
|
||||
const { createFakePi } = require("./fake_pi_runtime.js");
|
||||
const { pi, ctx, tools } = createFakePi();
|
||||
@@ -57,11 +62,12 @@ async function runConfirm(kind, artifactKind) {
|
||||
await pi.emit("session_start", {});
|
||||
ctx.ui.input = async (title) => {
|
||||
const d = JSON.parse(title);
|
||||
onDescriptor?.(d);
|
||||
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" } },
|
||||
{ session: "s1", kind, title: "t", artifact: { kind: artifactKind, data: artifactData } },
|
||||
null,
|
||||
null,
|
||||
ctx,
|
||||
@@ -104,3 +110,30 @@ test("approving the SQL closes F7 automatically (no separate phase gate)", async
|
||||
);
|
||||
assert.ok(!calls.some((c) => c.startsWith("session finalize")), "F7 must not finalize");
|
||||
});
|
||||
|
||||
test("SQL approval hydrates the gate from persisted sql_final instead of trusting empty model data", async () => {
|
||||
useShell({ phase: 7, sqlContent: "SELECT persisted_sql" });
|
||||
let descriptor;
|
||||
|
||||
await runConfirm("sql", "sql", {
|
||||
artifactData: {},
|
||||
onDescriptor: (value) => { descriptor = value; },
|
||||
});
|
||||
|
||||
assert.equal(descriptor.artifact.kind, "sql");
|
||||
assert.equal(descriptor.artifact.data, "SELECT persisted_sql");
|
||||
});
|
||||
|
||||
test("SQL approval refuses to show a gate when persisted sql_final is empty", async () => {
|
||||
const calls = useShell({ phase: 7, sqlContent: "" });
|
||||
let shown = false;
|
||||
|
||||
const result = await runConfirm("sql", "sql", {
|
||||
artifactData: {},
|
||||
onDescriptor: () => { shown = true; },
|
||||
});
|
||||
|
||||
assert.equal(shown, false);
|
||||
assert.match(result.content[0].text, /sql_final.*vuoto/i);
|
||||
assert.ok(!calls.some((c) => c.includes("decision add")));
|
||||
});
|
||||
|
||||
@@ -21,6 +21,8 @@ test("the resume kickoff injects the bootstrap steps and forces in-turn action",
|
||||
assert.match(text, /<tht-sessione-skill>/);
|
||||
assert.match(text, /# Thoth session workflow \(phases 1-8\)/);
|
||||
assert.match(text, /non esplorare il repository/i);
|
||||
assert.match(text, /tht session documents <id> --json/);
|
||||
assert.match(text, /non cercare i file fisici con `find`/i);
|
||||
assert.doesNotMatch(text, /Carica la skill leggendo/);
|
||||
});
|
||||
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
const test = require("node:test");
|
||||
const assert = require("node:assert");
|
||||
const { resolveSelectOutcome, decisionAddArgs } = require("../../tht-gate.js");
|
||||
const {
|
||||
resolveSelectOutcome,
|
||||
decisionAddArgs,
|
||||
decisionRecordedResultText,
|
||||
} = require("../../tht-gate.js");
|
||||
|
||||
// Workstream F — single-select answers auto-confirm. A concrete reviewer_select choice
|
||||
// that carries a `decision` payload IS the confirmation: the gate persists it directly
|
||||
@@ -46,3 +50,15 @@ test("decisionAddArgs omits optional detail/rationale when absent", () => {
|
||||
["decision", "add", "--session", "s1", "--type", "t", "--subject", "sub"],
|
||||
);
|
||||
});
|
||||
|
||||
test("a persisted select decision immediately directs the model to the next gate tool", () => {
|
||||
const text = decisionRecordedResultText(
|
||||
{ type: "concept_clarified" },
|
||||
{ label: "Procedure invasive" },
|
||||
);
|
||||
|
||||
assert.match(text, /Decisione registrata \(concept_clarified\): Procedure invasive\./);
|
||||
assert.match(text, /tht session show/);
|
||||
assert.match(text, /prossimo tool reviewer_/);
|
||||
assert.match(text, /Non scrivere analisi o spiegazioni visibili/);
|
||||
});
|
||||
|
||||
@@ -165,6 +165,14 @@ export function isProtectedBashMutation(cmd) {
|
||||
return PROTECTED_PATH_TOKEN.test(cmd) && BASH_MUTATION.test(cmd);
|
||||
}
|
||||
|
||||
// Session artifacts are exposed by deterministic `tht session documents`; shell
|
||||
// discovery is both unnecessary and dangerous (a model once escalated to `find /`
|
||||
// and wedged the whole RPC turn). Match shell command boundaries so the supported
|
||||
// `tht search find` subcommand remains available.
|
||||
export function isFilesystemFind(cmd) {
|
||||
return /(?:^|[;&|\n])\s*(?:(?:sudo|command)\s+)?find(?:\s|$)/.test(cmd);
|
||||
}
|
||||
|
||||
// --- kickoff payloads (verbatim from source L184-212, load-bearing model prose) -
|
||||
const NUOVA_DOMANDA_KICKOFF =
|
||||
"AZIONE IMMEDIATA OBBLIGATORIA: la skill canonica è già inclusa integralmente nel " +
|
||||
@@ -480,6 +488,35 @@ export function decisionAddArgs(session, d) {
|
||||
return args;
|
||||
}
|
||||
|
||||
// Keep the model inside the gate workflow immediately after a reviewer selection.
|
||||
// The agent_end prose safety net is too late for providers that keep streaming a
|
||||
// single, very long assistant turn: put the continuation contract in the tool result
|
||||
// that unlocks the model after the reviewer response.
|
||||
export function decisionRecordedResultText(decision, option) {
|
||||
return (
|
||||
`Decisione registrata (${decision.type}): ${option.label}. ` +
|
||||
"Ora rileggi lo stato persistito con `tht session show` e invoca immediatamente " +
|
||||
"il prossimo tool reviewer_ richiesto dal workflow. " +
|
||||
"Non scrivere analisi o spiegazioni visibili."
|
||||
);
|
||||
}
|
||||
|
||||
export function hasUngatedAssistantProse(messages) {
|
||||
const last = messages?.[messages.length - 1];
|
||||
if (!last || last.role !== "assistant" || !Array.isArray(last.content))
|
||||
return false;
|
||||
const hasText = last.content.some(
|
||||
(block) => block.type === "text" && (block.text ?? "").trim().length > 0,
|
||||
);
|
||||
const hasReviewerTool = last.content.some(
|
||||
(block) =>
|
||||
block.type === "toolCall" &&
|
||||
typeof block.name === "string" &&
|
||||
block.name.startsWith("reviewer_"),
|
||||
);
|
||||
return hasText && !hasReviewerTool;
|
||||
}
|
||||
|
||||
// Workstream F: classifies a reviewer_select response into the action the gate takes.
|
||||
// A concrete choice that carries a `decision` payload auto-confirms (persist directly,
|
||||
// no second gate); a bare choice stays ask-only; back/exit/Other never persist.
|
||||
@@ -618,6 +655,15 @@ export default function (pi) {
|
||||
lastSteered = false;
|
||||
return;
|
||||
}
|
||||
if (isFilesystemFind(cmd)) {
|
||||
return {
|
||||
block: true,
|
||||
reason:
|
||||
"Non cercare file con `find`: gli artefatti persistiti della sessione " +
|
||||
"si leggono con `tht session documents <id> --json`; per catalogo ed " +
|
||||
"evidence usa `tht schema render` / `tht search find`.",
|
||||
};
|
||||
}
|
||||
if (FORBIDDEN.some((re) => re.test(cmd))) {
|
||||
return {
|
||||
block: true,
|
||||
@@ -739,17 +785,7 @@ export default function (pi) {
|
||||
// tool call (and the lock is active), nudge it back to the gate tools.
|
||||
pi.on("agent_end", async (event) => {
|
||||
if (!lockActive) return;
|
||||
const msgs = event.messages ?? [];
|
||||
const last = msgs[msgs.length - 1];
|
||||
const isProse =
|
||||
last &&
|
||||
last.role === "assistant" &&
|
||||
Array.isArray(last.content) &&
|
||||
last.content.some(
|
||||
(b) => b.type === "text" && (b.text ?? "").trim().length > 0,
|
||||
) &&
|
||||
!last.content.some((b) => b.type === "toolCall");
|
||||
if (isProse && !lastSteered) {
|
||||
if (hasUngatedAssistantProse(event.messages ?? []) && !lastSteered) {
|
||||
lastSteered = true;
|
||||
await pi.sendUserMessage(
|
||||
"Le risposte del reviewer arrivano solo dai widget del gate. Riproponi la " +
|
||||
@@ -832,7 +868,7 @@ export default function (pi) {
|
||||
if (err) return err;
|
||||
if (advance) advanceIfReady(ctx, session);
|
||||
return textResult(
|
||||
`Decisione registrata (${outcome.decision.type}): ${outcome.option.label}.`,
|
||||
decisionRecordedResultText(outcome.decision, outcome.option),
|
||||
);
|
||||
}
|
||||
return textResult(
|
||||
@@ -1159,6 +1195,36 @@ export default function (pi) {
|
||||
const curNum = currentPhase(ctx, session);
|
||||
const phase = phaseId(ctx, curNum);
|
||||
|
||||
// F7's review target is the persisted artifact, never the model-authored
|
||||
// descriptor payload. A model can (and did) call reviewer_confirm with
|
||||
// `artifact.data: {}` even though write_final_sql had already persisted a
|
||||
// non-empty sql_final.sql; trusting that payload rendered a blank approval
|
||||
// dialog. Hydrate from the repository boundary and fail closed before any
|
||||
// widget is shown if the artifact is unavailable or empty.
|
||||
if (kind === "sql") {
|
||||
let docs;
|
||||
try {
|
||||
docs = JSON.parse(tht(ctx, ["session", "documents", session, "--json"]));
|
||||
} catch (e) {
|
||||
const msg = (e.stderr || e.message || String(e)).toString().trim();
|
||||
return textResult(
|
||||
`Impossibile leggere sql_final.sql dalla sessione ${session}: ${msg}. ` +
|
||||
"Verifica l'artefatto persistito e riprova.",
|
||||
);
|
||||
}
|
||||
const sqlDoc = Array.isArray(docs)
|
||||
? docs.find((doc) => doc && doc.key === "sql" && doc.format === "sql")
|
||||
: null;
|
||||
const sql = typeof sqlDoc?.content === "string" ? sqlDoc.content : "";
|
||||
if (!sql.trim()) {
|
||||
return textResult(
|
||||
`sql_final.sql assente o vuoto per la sessione ${session}: ` +
|
||||
"scrivi un SQL finale valido prima di ripresentare il gate F7.",
|
||||
);
|
||||
}
|
||||
artifact = { ...artifact, kind: "sql", data: sql };
|
||||
}
|
||||
|
||||
// Sessione gia' finalizzata: non c'e' piu' nulla da approvare — mai
|
||||
// ripresentare il phase-gate (la form "approve") a workflow chiuso.
|
||||
if (kind === "phase" && curNum >= phaseMeta(ctx).max_phase) {
|
||||
|
||||
@@ -151,9 +151,13 @@ persisted state is your only context. Bootstrap before doing anything else:
|
||||
|
||||
1. `tht session show <id> --json` → read `phase` (the current phase N), `status`, and
|
||||
the manifest (`question`, `database`, `schema`).
|
||||
2. Load the artifacts produced so far, as needed for phase N: `question.md` (revised
|
||||
question), `schema_linking.json` (F4 output), `ctes/*.sql` + `cte_tests.json` (F6),
|
||||
2. Load the artifacts produced so far with `tht session documents <id> --json`, which
|
||||
returns their keys and contents directly: `question.md` (revised question),
|
||||
`schema_linking.json` (F4 output), `ctes/*.sql` + `cte_tests.json` (F6),
|
||||
`sql_final.sql` (F7). The decision ledger is summarized by `tht session show`.
|
||||
**Non cercare i file fisici con `find`, `ls`, `cat` o il generic read tool**: the
|
||||
session repository may live outside the checkout and the documents command is the
|
||||
canonical read boundary.
|
||||
3. **Resume at phase N reviewing the existing artifacts** (same discipline as rollback,
|
||||
§Disciplines 11). Do NOT restart from Phase 1, do NOT re-run `tht` commands for
|
||||
artifacts that already exist and are valid, and do NOT treat this as a new question.
|
||||
|
||||
Reference in New Issue
Block a user