From 08f002979322fae30c69e9ffdf68138be084e5b7 Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 12 Jul 2026 14:26:48 +0200 Subject: [PATCH] fix: finalize session after memory promotion --- .claude/launch.json | 11 ++ .../src/shell/AppShell.session-mgmt.test.tsx | 25 +++++ frontend/src/shell/AppShell.tsx | 21 +++- .../__tests__/gate_promote_autoclose.test.js | 87 +++++++++++++++ harness/.pi/extensions/tht-gate.js | 100 +++++++++++++----- harness/.pi/skills/tht-sessione/SKILL.md | 27 +++-- 6 files changed, 232 insertions(+), 39 deletions(-) create mode 100644 .claude/launch.json create mode 100644 harness/.pi/extensions/gate/__tests__/gate_promote_autoclose.test.js diff --git a/.claude/launch.json b/.claude/launch.json new file mode 100644 index 00000000..61be3335 --- /dev/null +++ b/.claude/launch.json @@ -0,0 +1,11 @@ +{ + "version": "0.0.1", + "configurations": [ + { + "name": "replay", + "runtimeExecutable": "node", + "runtimeArgs": ["tools/replay/server.mjs"], + "port": 5333 + } + ] +} diff --git a/frontend/src/shell/AppShell.session-mgmt.test.tsx b/frontend/src/shell/AppShell.session-mgmt.test.tsx index a5a2ed51..c3ae218e 100644 --- a/frontend/src/shell/AppShell.session-mgmt.test.tsx +++ b/frontend/src/shell/AppShell.session-mgmt.test.tsx @@ -125,6 +125,31 @@ test("WIP icon toggles the Model activity panel and shows the streamed text", as expect(screen.getByText(/FIRSTLINE/)).toBeInTheDocument(); }); +test("session finalization shows the completion banner and returns to landing", async () => { + useSessionStore.getState().resetSession(); + let finalized = false; + server.use( + http.post("http://localhost:8787/sessions/:id/resume", () => new HttpResponse(null, { status: 204 })), + http.get("http://localhost:8787/sessions", () => + HttpResponse.json(finalized ? [{ ...LIST[0], status: "finalized" }] : LIST)), + ); + wrap(); + await userEvent.click(await screen.findByText("Attiva uno")); + await userEvent.click(await screen.findByRole("button", { name: /resume/i })); + // The final turn ends: the session is now finalized on disk and agent_end + // arrives — the shell must refetch immediately (not wait for the 10s poll) + // and surface the new-session invite. + finalized = true; + act(() => { + useSessionStore.getState().applyEvent({ type: "system_event", event: "agent_end" } as any); + }); + const cta = await screen.findByRole("button", { name: /start a new question/i }); + expect(screen.getByText(/session completed and finalized/i)).toBeInTheDocument(); + await userEvent.click(cta); + // Landing state: the composer invites a brand-new question. + expect(await screen.findByText(/type your question/i)).toBeInTheDocument(); +}); + test("renaming a group reassigns its members via setSessionGroup", async () => { const groupSets: Array<{ id: string; group: string }> = []; server.use( diff --git a/frontend/src/shell/AppShell.tsx b/frontend/src/shell/AppShell.tsx index fb14de05..263ef669 100644 --- a/frontend/src/shell/AppShell.tsx +++ b/frontend/src/shell/AppShell.tsx @@ -177,9 +177,13 @@ export function AppShell() { // stopSession already does, and this effect must stay side-effect-free on the // backend if the session is already inactive. useEffect(() => { - if (lastSystemEvent?.type === "system_event" && (lastSystemEvent as any).event === "session_exit") { - stopSession(); - } + if (lastSystemEvent?.type !== "system_event") return; + const ev = (lastSystemEvent as any).event; + if (ev === "session_exit") stopSession(); + // The final workflow turn ends with the session already finalized on disk: + // refetch now instead of waiting for the 10s poll, so the completed state + // (and the new-session invite below the transcript) appears immediately. + if (ev === "agent_end") refresh(); // eslint-disable-next-line react-hooks/exhaustive-deps }, [lastSystemEvent]); @@ -241,6 +245,17 @@ export function AppShell() { <> + {finalized && !agentActive && ( +
+

+ Session completed and finalized — the SQL and all phase + documents are saved. +

+ +
+ )} ) : ( diff --git a/harness/.pi/extensions/gate/__tests__/gate_promote_autoclose.test.js b/harness/.pi/extensions/gate/__tests__/gate_promote_autoclose.test.js new file mode 100644 index 00000000..098015da --- /dev/null +++ b/harness/.pi/extensions/gate/__tests__/gate_promote_autoclose.test.js @@ -0,0 +1,87 @@ +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"); + +// F8 auto-close: reviewer_memory_promote is the last human interaction of the +// workflow — after recording the promotion the gate must advance F8 and finalize +// the session itself (no trailing reviewer_confirm "approve" form). These tests +// drive the real tool execute against a fake `tht` CLI placed on PATH. + +// tht-gate.js calls a bare `require` (Pi's runtime provides it); under node --test +// the ESM import has none, so expose the CJS require globally for the gate module. +globalThis.require = require; + +const installGatePromise = import("../../tht-gate.js").then((m) => m.default); + +function setupFakeTht(t, { phase, status = "open" }) { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "fake-tht-")); + const log = path.join(dir, "calls.log"); + fs.writeFileSync(log, ""); + const script = `#!/bin/bash +echo "$@" >> "${log}" +case "$1 $2" in + "phase show") echo "Fase corrente: ${phase}";; + "phase meta") echo '{"max_phase":8,"phases":[{"num":2,"id":"F2","emits":[]},{"num":8,"id":"F8","emits":[]}]}';; + "memory promote") echo "[]";; + "session show") echo '{"status":"${status}"}';; + *) echo "OK";; +esac +`; + const bin = path.join(dir, "tht"); + fs.writeFileSync(bin, script, { 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") }; +} + +async function runPromote(t, { phase }) { + const fake = setupFakeTht(t, { phase }); + const installGate = await installGatePromise; + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = fake.dir; // execFileSync needs an existing cwd; no .env here is fine + installGate(pi); + const { def } = tools.get("reviewer_memory_promote"); + const res = await def.execute("t1", { session: "s1" }, null, null, ctx); + return { res, calls: fake.calls() }; +} + +test("F8 + zero candidati: il gate avanza la fase e finalizza da solo", async (t) => { + const { res, calls } = await runPromote(t, { phase: 8 }); + const text = res.content[0].text; + assert.match(text, /sessione finalizzata \(s1\)/); + assert.match(text, /NON presentare altri gate/); + assert.match(calls, /^phase advance --session s1$/m); + assert.match(calls, /^session finalize s1$/m); +}); + +test("fuori dall'ultima fase non chiude nulla (comportamento precedente)", async (t) => { + const { res, calls } = await runPromote(t, { phase: 2 }); + assert.match(res.content[0].text, /prosegui con la chiusura della sessione/); + assert.doesNotMatch(calls, /phase advance/); + assert.doesNotMatch(calls, /session finalize/); +}); + +test("phase-gate su sessione gia' finalizzata: nessun widget, il modello viene fermato", async (t) => { + setupFakeTht(t, { phase: 8, status: "finalized" }); + const installGate = await installGatePromise; + const { pi, ctx, tools } = createFakePi(); + ctx.cwd = os.tmpdir(); + installGate(pi); + const { def } = tools.get("reviewer_confirm"); + const res = await def.execute( + "t2", + { session: "s1", kind: "phase", title: "F8", artifact: { kind: "phase", data: {} } }, + null, + null, + ctx, + ); + assert.match(res.content[0].text, /gia' finalizzata/); + assert.equal(ctx.uiCalls.length, 0); // la form "approve" NON viene ripresentata +}); diff --git a/harness/.pi/extensions/tht-gate.js b/harness/.pi/extensions/tht-gate.js index 2d6f7bfb..78784aa6 100644 --- a/harness/.pi/extensions/tht-gate.js +++ b/harness/.pi/extensions/tht-gate.js @@ -832,6 +832,24 @@ export default function (pi) { const curNum = currentPhase(ctx, session); const phase = phaseId(ctx, curNum); + // 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) { + try { + const show = JSON.parse(tht(ctx, ["session", "show", session, "--json"])); + if (show?.status === "finalized") { + lockActive = false; + return textResult( + `La sessione ${session} e' gia' finalizzata: il workflow e' completo. ` + + "Comunica al reviewer il riepilogo finale e termina il turno; NON " + + "presentare altri gate reviewer_*.", + ); + } + } catch { + // show non leggibile: prosegui col gate normale + } + } + // --- v2 payload construction (WS2) ------------------------------------- // The gate builds/validates the structured payload BEFORE showing the // widget, so the reviewer approves a deterministic artifact (not the raw @@ -948,26 +966,9 @@ export default function (pi) { return textResult("Il reviewer vuole uscire."); // 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 - // and exits 6 with the missing items, which relayIfThtFails surfaces. - const err = relayIfThtFails( - ctx, - ["phase", "advance", "--session", session], - PHASE_RECOVERY, - ); - if (err) return err; - // Auto-finalize after advancing the LAST phase: the model may not - // follow through (token budget, turn end) leaving the session open. - const meta = phaseMeta(ctx); - if (curNum >= meta.max_phase) { - const fErr = relayIfThtFails( - ctx, - ["session", "finalize", session], - "Fase approvata ma finalizzazione fallita: esegui manualmente " + - `\`tht session finalize ${session}\`.`, - ); - if (fErr) return fErr; + const r = advancePhaseAndFinalize(ctx, session, curNum); + if (r.err) return r.err; + if (r.finalized) { lockActive = false; lastSteered = false; return textResult( @@ -1053,6 +1054,46 @@ export default function (pi) { }, }); + // Explicit human approval: advance unconditionally except for unmet + // prerequisites. Plain `phase advance` (no --auto) enforces advance_problems + // and exits 6 with the missing items, which relayIfThtFails surfaces. + // Auto-finalize after advancing the LAST phase: the model may not follow + // through (token budget, turn end) leaving the session open. + function advancePhaseAndFinalize(ctx, session, curNum) { + const err = relayIfThtFails( + ctx, + ["phase", "advance", "--session", session], + PHASE_RECOVERY, + ); + if (err) return { err }; + if (curNum < phaseMeta(ctx).max_phase) return { finalized: false }; + const fErr = relayIfThtFails( + ctx, + ["session", "finalize", session], + "Fase approvata ma finalizzazione fallita: esegui manualmente " + + `\`tht session finalize ${session}\`.`, + ); + if (fErr) return { err: fErr }; + return { finalized: true }; + } + + // La promozione memorie e' l'ultima interazione umana di F8: chiudere qui + // (advance + finalize) evita il reviewer_confirm finale a workflow gia' + // deciso (la form "approve" ridondante a fine sessione). Fuori dall'ultima + // fase ritorna null e il chiamante mantiene il comportamento precedente. + function closeAfterPromotion(ctx, session, curNum, summary) { + if (curNum < phaseMeta(ctx).max_phase) return null; + const r = advancePhaseAndFinalize(ctx, session, curNum); + if (r.err) return r.err; + lockActive = false; + lastSteered = false; + return textResult( + `${summary} Fase ${curNum} approvata e sessione finalizzata (${session}): ` + + "il workflow e' completo. Comunica al reviewer il riepilogo finale " + + "della sessione e termina il turno; NON presentare altri gate reviewer_*.", + ); + } + pi.registerTool({ name: "reviewer_memory_promote", label: "Promozione memorie riusabili (reviewer)", @@ -1062,14 +1103,17 @@ export default function (pi) { "table_promoted/table_excluded, max 5, esclusi i gia' promossi/rifiutati). Le selezioni " + "vengono salvate nel vectordb (tht memory save-one) e registrate come memory_promoted; " + "le deselezioni come memory_promotion_declined (non riproposte). Nessun parametro oltre " + - "alla sessione: i candidati sono deterministici, NON li scrivi tu.", + "alla sessione: i candidati sono deterministici, NON li scrivi tu. Registrata la " + + "promozione, in F8 il gate chiude la fase e finalizza la sessione da solo: NON " + + "presentare un reviewer_confirm dopo.", parameters: Type.Object({ session: Type.String(), }), async execute(_id, params, _signal, _onUpdate, ctx) { lockActive = true; const { session } = params; - const phase = phaseId(ctx, currentPhase(ctx, session)); + const curNum = currentPhase(ctx, session); + const phase = phaseId(ctx, curNum); let candidates; try { candidates = JSON.parse( @@ -1084,6 +1128,10 @@ export default function (pi) { "Nessuna decisione riusabile da promuovere in memoria per questa sessione.", "info", ); + const closed = closeAfterPromotion( + ctx, session, curNum, "Nessun candidato di promozione.", + ); + if (closed) return closed; return textResult( "Nessun candidato di promozione: prosegui con la chiusura della sessione.", ); @@ -1128,10 +1176,12 @@ export default function (pi) { }), ""); if (err) return err; } - return textResult( + const summary = `Promozione registrata: ${saved} memorie salvate nel vectordb, ` + - `${decline.length} candidati scartati.`, - ); + `${decline.length} candidati scartati.`; + const closed = closeAfterPromotion(ctx, session, curNum, summary); + if (closed) return closed; + return textResult(summary); }, }); diff --git a/harness/.pi/skills/tht-sessione/SKILL.md b/harness/.pi/skills/tht-sessione/SKILL.md index 2cb3f3db..6bef71cb 100644 --- a/harness/.pi/skills/tht-sessione/SKILL.md +++ b/harness/.pi/skills/tht-sessione/SKILL.md @@ -41,7 +41,7 @@ substantive decisions. | 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"` | -| F8 datamart | — | `reviewer_confirm kind:"phase"` | +| 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) @@ -387,19 +387,24 @@ Prerequisite: Phase 7 closed. 1. Ask the reviewer whether they want a datamart (`reviewer_select` yes/no). 2. If yes: `tht datamart generate` (stub — raises NotImplementedError for now). Tell the reviewer that dbt generation is not implemented yet. -3. **Memory promotion.** Call `reviewer_memory_promote` with ONLY the session id: the - gate computes the candidates itself (`tht memory promote --preview` — the 3 - reusable types, already excluding promoted/declined ones) and shows the reviewer a - pre-selected checklist. Selected → saved to the vectordb + `memory_promoted`; - deselected → `memory_promotion_declined` (never re-proposed). If the gate reports - zero candidates, move on — do not retry. -4. Close with `reviewer_confirm kind:"phase"`. The gate auto-finalizes the session - after advancing the last phase — you do NOT need to call `tht session finalize` - yourself. If auto-finalize fails, the error message tells you the recovery command. +3. **Memory promotion closes the session.** Call `reviewer_memory_promote` with ONLY + the session id: the gate computes the candidates itself (`tht memory promote + --preview` — the 3 reusable types, already excluding promoted/declined ones) and + shows the reviewer a pre-selected checklist. Selected → saved to the vectordb + + `memory_promoted`; deselected → `memory_promotion_declined` (never re-proposed). + After recording the promotion (even with zero candidates) the gate advances F8 and + finalizes the session itself — do NOT present a `reviewer_confirm kind:"phase"` + afterwards: there is nothing left to approve. When the gate answers "sessione + finalizzata", give the reviewer the final summary and end the turn. +4. If the gate reports an error instead (e.g. the datamart decision is missing), + fix the prerequisite and call `reviewer_memory_promote` again. Only if the gate + says the session is still open, close with `reviewer_confirm kind:"phase"` as a + fallback — it auto-finalizes after advancing the last phase too. ## Session end -When Phase 8 is approved, the gate calls `tht session finalize` automatically. +When the promotion gate (or, as fallback, the F8 phase gate) closes Phase 8, the +gate calls `tht session finalize` automatically. Finalize also indexes the question→SQL pair in the vectordb (kind `solved_question`, best-effort — on failure recover with `tht memory solved-index `). The persisted state (ledger `review_decisions.jsonl` + artifacts) is the truth: what is not