From 1b1554b3536888c114c8717a52668e95bc484375 Mon Sep 17 00:00:00 2001 From: User Date: Tue, 14 Jul 2026 13:51:39 +0200 Subject: [PATCH] fix: auto-approve phase 3 rewrite --- .../2026-07-14-f3-rewrite-auto-approval.md | 44 ++++++++++ ...6-07-14-f3-rewrite-auto-approval-design.md | 15 ++++ .../gate_rewrite_auto_approval.test.js | 83 +++++++++++++++++++ harness/.pi/extensions/tht-gate.js | 25 +++++- harness/.pi/skills/tht-sessione/SKILL.md | 15 ++-- 5 files changed, 169 insertions(+), 13 deletions(-) create mode 100644 docs/superpowers/plans/2026-07-14-f3-rewrite-auto-approval.md create mode 100644 docs/superpowers/specs/2026-07-14-f3-rewrite-auto-approval-design.md create mode 100644 harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js diff --git a/docs/superpowers/plans/2026-07-14-f3-rewrite-auto-approval.md b/docs/superpowers/plans/2026-07-14-f3-rewrite-auto-approval.md new file mode 100644 index 00000000..2d690685 --- /dev/null +++ b/docs/superpowers/plans/2026-07-14-f3-rewrite-auto-approval.md @@ -0,0 +1,44 @@ +# F3 Rewrite Auto-Approval Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Remove the F3 reviewer confirmation while preserving the persisted rewrite and normal phase transition. + +**Architecture:** `rewrite_question` owns the ordered document write, decision recording, and phase advance. The F3 skill calls it once, without a reviewer widget. + +**Tech Stack:** JavaScript Pi extension, Python `tht` CLI, Node test runner, pytest. + +## Global Constraints + +- Preserve the eight phase numbers, `question.md`, and `question_rewritten` ledger type. +- Never write decisions or advance phases outside F3. +- Use `tht` commands with `--session` after the command group. + +--- + +### Task 1: Test and implement automatic F3 completion + +**Files:** +- Modify: `harness/.pi/extensions/tht-gate.js` +- Test: `harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js` + +- [ ] Add a fake Pi integration test that invokes the registered `rewrite_question` tool at F3 and asserts these calls in order: `session set-question`, `decision add question_rewritten`, `phase advance`. +- [ ] Run `cd harness && npm test -- --test-name-pattern="rewrite question"` and observe failure because the current tool only writes the document. +- [ ] Make `rewrite_question` reject non-F3 sessions, record the decision after a successful write, advance after a successful decision, and return a no-widget completion message. +- [ ] Re-run the focused test and then `cd harness && npm test`. + +### Task 2: Remove the F3 reviewer instruction + +**Files:** +- Modify: `harness/.pi/skills/tht-sessione/SKILL.md` + +- [ ] Replace the F3 `reviewer_decide` and `reviewer_confirm` procedure with one direct `rewrite_question` call and an explicit prohibition on reviewer widgets. + +### Task 3: Verify workflow compatibility + +**Files:** +- Test: `harness/tests/test_workflow.py` +- Test: `harness/tests/test_phase_effective.py` + +- [ ] Run `cd harness && .venv/bin/pytest -q tests/test_workflow.py tests/test_phase_effective.py`. +- [ ] Run `cd harness && .venv/bin/pytest -q`. diff --git a/docs/superpowers/specs/2026-07-14-f3-rewrite-auto-approval-design.md b/docs/superpowers/specs/2026-07-14-f3-rewrite-auto-approval-design.md new file mode 100644 index 00000000..12028e09 --- /dev/null +++ b/docs/superpowers/specs/2026-07-14-f3-rewrite-auto-approval-design.md @@ -0,0 +1,15 @@ +# F3 Rewrite Auto-Approval Design + +## Goal + +Remove the redundant human confirmation titled "Conferma riscrittura della domanda" while retaining Phase 3, its deterministic `question.md` artifact, and its audit trail. + +## Design + +The existing privileged `rewrite_question` tool becomes the owner of F3 completion. It writes `question.md`, records `question_rewritten` with the full rewritten question, then advances F3. It refuses calls outside F3. The orchestration skill calls this tool directly and must not emit `reviewer_decide` or `reviewer_confirm` in F3. + +If the write fails, it records nothing. If decision recording fails, it does not advance. If the advance fails, the persisted artifact and decision remain for a safe retry. Phase numbers, workflow YAML, artifact paths, decision type, and old sessions remain compatible. + +## Tests + +Gate coverage will verify ordered CLI calls and F3-only protection. The existing Python phase tests will verify the unchanged workflow invariant. diff --git a/harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js b/harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js new file mode 100644 index 00000000..25abc582 --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_rewrite_auto_approval.test.js @@ -0,0 +1,83 @@ +const test = require("node:test"); +const assert = require("node:assert"); +const fs = require("node:fs"); +const os = require("node:os"); +const path = require("node:path"); +const { createFakePi } = require("./fake_pi_runtime.js"); + +globalThis.require = require; + +const installGatePromise = import("../../tht-gate.js").then((m) => m.default); + +function setupFakeTht(t, phase) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fake-tht-rewrite-")); + const log = path.join(dir, "calls.log"); + fs.writeFileSync(log, ""); + const bin = path.join(dir, "tht"); + fs.writeFileSync( + bin, + `#!/bin/bash +echo "$@" >> "${log}" +case "$1 $2" in + "phase show") echo "Fase corrente: ${phase}" ;; + "phase meta") echo '{"max_phase":8,"phases":[{"num":3,"id":"F3","emits":["question_rewritten"]}]}' ;; + *) echo "OK" ;; +esac +`, + { mode: 0o755 }, + ); + const oldPath = process.env.PATH; + process.env.PATH = `${dir}:${oldPath}`; + t.after(() => { + process.env.PATH = oldPath; + fs.rmSync(dir, { recursive: true, force: true }); + }); + return { dir, calls: () => fs.readFileSync(log, "utf8").trim().split("\n").filter(Boolean) }; +} + +test("rewrite question completes F3 without a reviewer widget", async (t) => { + const fake = setupFakeTht(t, 3); + const installGate = await installGatePromise; + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = fake.dir; + installGate(pi); + + const { def } = tools.get("rewrite_question"); + const result = await def.execute( + "rewrite-1", + { session: "s1", question: "pazienti con ablazione", assumptions: ["anno 2025"] }, + null, + null, + ctx, + ); + + assert.deepEqual(fake.calls(), [ + "phase show --session s1", + "session set-question s1 --question pazienti con ablazione --assumption anno 2025", + "decision add --session s1 --type question_rewritten --subject domanda --detail pazienti con ablazione", + "phase advance --session s1", + "phase meta --json", + ]); + assert.equal(ctx.uiCalls.length, 0); + assert.match(result.content[0].text, /Fase 3 completata/); +}); + +test("rewrite question refuses calls outside F3", async (t) => { + const fake = setupFakeTht(t, 4); + const installGate = await installGatePromise; + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = fake.dir; + installGate(pi); + + const { def } = tools.get("rewrite_question"); + const result = await def.execute( + "rewrite-2", + { session: "s1", question: "pazienti con ablazione" }, + null, + null, + ctx, + ); + + assert.deepEqual(fake.calls(), ["phase show --session s1"]); + assert.match(result.content[0].text, /solo in Fase 3/); +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 375074d5..bc029ed8 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -1266,10 +1266,11 @@ export default function (pi) { pi.registerTool({ name: "rewrite_question", - label: "Riscrittura domanda (deterministica)", + label: "Riscrittura domanda e chiusura F3 (deterministica)", description: - "Scrive deterministicamente question.md via tht session set-question (evita il tool " + - "di edit unreliable). assumptions puo' essere array o stringa JSON.", + "In Fase 3 scrive deterministicamente question.md, registra question_rewritten e " + + "chiude la fase senza chiedere conferma al reviewer. assumptions puo' essere array " + + "o stringa JSON.", parameters: Type.Object({ session: Type.String(), question: Type.String(), @@ -1278,6 +1279,10 @@ export default function (pi) { async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; const { session, question, assumptions } = params; + const curNum = currentPhase(ctx, session); + if (curNum !== 3) { + return textResult("La riscrittura automatica e' disponibile solo in Fase 3."); + } let assumps = assumptions; if (typeof assumps === "string") { try { @@ -1295,7 +1300,19 @@ export default function (pi) { } const err = relayIfThtFails(ctx, args, ""); if (err) return err; - return textResult(`Domanda riscritta per la sessione ${session}.`); + const decisionErr = relayIfThtFails( + ctx, + decisionAddArgs(session, { + type: "question_rewritten", + subject: "domanda", + detail: question, + }), + "", + ); + if (decisionErr) return decisionErr; + const advanced = advancePhaseAndFinalize(ctx, session, curNum); + if (advanced.err) return advanced.err; + return textResult(`Domanda riscritta e Fase 3 completata (sessione ${session}).`); }, }); diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 0b31c750..be7e46b7 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -243,15 +243,12 @@ Phase 3 (CLI exit 5). terms, each condition as a separate numbered clause, ambiguous terms replaced with the concepts clarified in Phase 1 citing the defining evidence, expected output made explicit). -2. Present in **a single** `reviewer_decide(advance:false, allow_other:true)`. The - "Confirm rewriting" option is `recommended:true` with - `{type:"question_rewritten", subject:"domanda", detail:""}`. -3. **Order matters:** (a) the `reviewer_decide` records `question_rewritten` → (b) call the - gate's `rewrite_question` tool, which runs `tht session set-question` to write - `question.md` (regenerates question + an "## Assunzioni" section; never edit it by hand) - → (c) close the phase with `reviewer_confirm kind:"phase"`. F3 does NOT auto-advance: - the `question_rewritten` decision alone does not move the phase. -4. "Altro" iterates (re-propose a new `reviewer_decide`). "Torna indietro" reopens F1. +2. Call `rewrite_question` once with the completed rewritten question and assumptions. The + gate writes `question.md` (including `## Assunzioni`), records `question_rewritten`, and + closes F3 automatically. +3. Do **not** call `reviewer_decide` or `reviewer_confirm` in F3: the rewrite is assumed + approved. Continue at F4 only after `rewrite_question` reports success. To revise the + rewrite, use "Torna indietro" to reopen F1. ## Phase 4 — Schema linking