From cc1ffb59fa740647f68940e41e54415bb05ecf2c Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 5 Jul 2026 12:41:39 +0200 Subject: [PATCH] docs(pi): revise dual-mode gate eval per review (P1/P2) - present() = presentBlockingWidget: transport + no-limbo loop + response validation; both branches share validateUiResponse; the id invariant must be validated explicitly in the TUI branch (emitAndWait no longer runs there) - TUI renderer uses numbered options + index parsing, never label mapping (frontend already answers with ids); artifact-gate editor is viewer-only, returned content ignored - guards resolved: both TUI-only notices now ctx.mode === "tui" - interactive-render.js as two layers (renderTuiDescriptor + validateSyntheticResponse); add negative test cases + a real TUI smoke Co-Authored-By: Claude Opus 4.8 --- .../2026-07-04-dual-mode-gate-evaluation.md | 64 ++++++++++++------- 1 file changed, 42 insertions(+), 22 deletions(-) diff --git a/docs/superpowers/specs/2026-07-04-dual-mode-gate-evaluation.md b/docs/superpowers/specs/2026-07-04-dual-mode-gate-evaluation.md index 7c224fb8..1eb19a2e 100644 --- a/docs/superpowers/specs/2026-07-04-dual-mode-gate-evaluation.md +++ b/docs/superpowers/specs/2026-07-04-dual-mode-gate-evaluation.md @@ -76,14 +76,21 @@ id-match in `emitAndWait` ([tht-gate.js:312](../../../harness/.pi/extensions/tht - **Invariati**: `gate/builders.js` (costruzione descrittori) e i classificatori di esito. Sono il contratto condiviso tra le due modalità. -- **Unica modifica**: sostituire `emitAndWait(ctx, descriptor)` con un - `present(ctx, descriptor): Promise` **mode-aware**: +- **Unica modifica**: sostituire `emitAndWait(ctx, descriptor)` con + `presentBlockingWidget(ctx, descriptor): Promise` **mode-aware**. Il confine + **non** è solo il trasporto: è **presentazione + loop no-limbo + validazione della + risposta** — oggi `emitAndWait` fa già trasporto → `JSON.parse` → check `cancel` → + id-match → re-present su risposta invalida ([tht-gate.js:298](../../../harness/.pi/extensions/tht-gate.js)). + Entrambi i rami confluiscono in un **unico validatore condiviso** + `validateUiResponse(descriptor, resp)`: - `if (ctx.mode !== "tui")` → percorso ATTUALE (RPC/json/print): - `ctx.ui.input(JSON.stringify(descriptor))` + `JSON.parse` + id-match. Nessun - cambiamento nel frontend. - - `else` (`ctx.mode === "tui"`, interattivo) → renderizza il descrittore con le - primitive native e **sintetizza** un `UiResponse` con lo stesso `id` e la stessa - forma, così i classificatori a valle non cambiano di una riga. + `ctx.ui.input(JSON.stringify(descriptor))` + `JSON.parse` → `validateUiResponse`. + Nessun cambiamento nel frontend. + - `else` (`ctx.mode === "tui"`) → `renderTuiDescriptor` con le primitive native → + **sintetizza** un `UiResponse` → **stesso** `validateUiResponse`. ⚠️ **Critico**: in TUI + l'id-match di `emitAndWait` **non gira più** (present lo sostituisce), quindi + l'invariante `resp.id === descriptor.id` va **validato esplicitamente** qui — non è più + protetto in automatico. I **3 gate bloccanti** che passano da `emitAndWait` sono l'intera superficie da rendere: `select` ([tht-gate.js:492](../../../harness/.pi/extensions/tht-gate.js)), `multiselect` (576), @@ -92,9 +99,9 @@ nativa ritorna `undefined` (Esc / cancel / timeout / `signal`): | widget | render interattivo | no-limbo (nativo → `undefined`) | UiResponse sintetizzato | |---|---|---|---| -| `select` | `ctx.ui.select(title, [...labels, "Go back", "Exit", "Other — specify"])`; "Other"→`ctx.ui.input` | re-present una volta; se ancora `undefined` → `{control:"exit"}` | `{id, choices:[optId]}` o `{control:"back"/"exit"/"freetext", text}` | -| `multiselect` | `ctx.ui.custom` (checkbox list); fallback low-cost: `input` con indici separati da virgola | re-present una volta; poi `{control:"exit"}` | `{id, choices:[TUTTI gli id scelti]}` — vedi nota reviewer_decide sotto | -| `artifact-gate` | artefatto via `ctx.ui.editor(title, testo)` (multi-linea, meglio di `notify`) + `ctx.ui.confirm`/`select` approve/reject/other | re-present (advance solo su approve/reject **esplicito**, mai implicito) | `{id, choices:["approve"/"reject"]}` o control | +| `select` | opzioni **numerate** (`1. label … / b) Torna / e) Esci / o) Altro`); parsing **per indice → id**, MAI per label | re-present una volta; se ancora `undefined` → `{control:"exit"}` | `{id, choices:[optId]}` o `{control:"back"/"exit"/"freetext", text}` | +| `multiselect` | `ctx.ui.custom` (checkbox) **oppure** `ctx.ui.input` **indicizzato** (`1,3,4`) → dedupe + ordine stabile + **rifiuto indici invalidi** | re-present una volta; poi `{control:"exit"}` | `{id, choices:[TUTTI gli id scelti]}` — vedi nota reviewer_decide sotto | +| `artifact-gate` | artefatto mostrato con `ctx.ui.editor(title, testo)` **come viewer** (il contenuto ritornato è **ignorato** — il gate approva/rifiuta, non modifica) + scelta approve/reject **numerata** | re-present (advance solo su approve/reject **esplicito**, mai implicito) | `{id, choices:["approve"/"reject"]}` o control | **Non sono widget-descriptor** (nessuna riga `present()` dedicata): - **`freetext`** non è un widget a sé: è il `control:"freetext"` restituito dall'opzione @@ -108,8 +115,11 @@ nativa ritorna `undefined` (Esc / cancel / timeout / `signal`): ## Vincoli del runtime Pi verificati (0.80.3) - **`ctx.ui.select` ritorna la label** (string), non l'id — `options: string[]` → - `Promise` (types.d.ts:69). Il renderer deve rimappare - label→`option.id`. `custom` evita il problema ma costa di più. + `Promise` (types.d.ts:69). **NON rimappare per label** (label + duplicate/localizzate collidono): opzioni **numerate** + parsing **per indice → id**. Il + contratto ThothII è sugli id, e il frontend risponde già sempre con id, non label + ([SelectWidget:26](../../../frontend/src/widgets/SelectWidget.tsx), + [MultiselectWidget:66](../../../frontend/src/widgets/MultiselectWidget.tsx)). - **Nessuna multiselect nativa**: `ExtensionUIContext` non la espone → `custom` (checkbox, livello A) o fallback a input indicizzato (livello C). - **Dismiss / no-limbo nativo**: `select`/`confirm`/`input`/`editor` ritornano @@ -145,9 +155,10 @@ gli si dà un renderer, invece di derivare silenziosamente dal percorso RPC. - `harness/.pi/extensions/tht-gate.js`: introdurre `present(ctx, descriptor)` e sostituire le 3 chiamate a `emitAndWait` (reviewer_select/decide/confirm) con essa; il ramo RPC è l'attuale `emitAndWait`. -- Nuovo modulo puro es. `harness/.pi/extensions/gate/interactive-render.js` - (descriptor → prompt testuale/native + parse risposta → `UiResponse`), così è - L1-testabile in isolamento come `builders.js`. +- Nuovo modulo puro `harness/.pi/extensions/gate/interactive-render.js` a **due strati**: + `renderTuiDescriptor(descriptor)` (descriptor → prompt numerato/native) e + `validateSyntheticResponse(descriptor, resp)` (= `validateUiResponse`, **condiviso** dal + ramo TUI e dal ramo RPC dopo `JSON.parse`). Entrambi L1-testabili come `builders.js`. - Nessun cambiamento a `builders.js`, ai classificatori, al backend o al frontend. ## Note implementative (checklist prima di scrivere present()) @@ -161,11 +172,13 @@ gli si dà un renderer, invece di derivare silenziosamente dal percorso RPC. fire-and-forget verso il frontend; in TUI diventerebbero warning a terminale. Decidere per-notify se appartiene a entrambe le modalità — in particolare la **651** è dentro il loop di re-present del confirm gate. -- **Guardie `ctx.hasUI` da rivedere (regresso di migrazione).** 319/397 erano "solo TUI" con - la semantica 0.73.1 (`hasUI=false` in RPC). Su 0.80.3 `hasUI=true` anche in RPC, quindi ora - scattano in RPC (es. riga 321 *"Esc non chiude il gate…"*, priva di senso nel browser). - L'intento va espresso con `ctx.mode === "tui"`. **Indipendente dal dual-mode**, ma da - sistemare comunque. +- **Guardie modo-dipendenti (risolte).** Entrambi i messaggi TUI-only sono ora guardati da + `ctx.mode === "tui"`: reLoop anti-limbo + ([tht-gate.js:322](../../../harness/.pi/extensions/tht-gate.js)) e steering-durante-lock + ([tht-gate.js:400](../../../harness/.pi/extensions/tht-gate.js)). In RPC il testo libero + durante il lock resta comunque `{action:"handled"}` (silenziato) — cambia solo che il warning + da terminale non viene più inviato al browser. Nessuna guardia `ctx.hasUI` residua per messaggi + TUI-only. ## Test (il vero guardrail contro la deriva) @@ -181,8 +194,15 @@ gli si dà un renderer, invece di derivare silenziosamente dal percorso RPC. `opts.filter(o => (resp.choices ?? []).includes(o.id))`). Il test di non-regressione deve quindi verificare che la sintesi interattiva del multiselect popoli **tutti** gli id scelti (non solo il primo), altrimenti in TUI si perderebbero selezioni. -- **Invariante id-match**: la sintesi interattiva deve impostare `id = descriptor.id` - (altrimenti `emitAndWait` scarta la risposta, tht-gate.js:312). +- **Invariante id-match**: in TUI `emitAndWait` non gira più → `validateUiResponse` deve + verificare `resp.id === descriptor.id` **esplicitamente** (non basta che la sintesi imposti + l'id: va validato, come faceva tht-gate.js:312). +- **Casi negativi** (oltre al round-trip): id sbagliato, scelta inesistente, **label + duplicate**, multiselect con **duplicati**, `allow_empty=false` con input vuoto, `undefined` + ripetuto (no-limbo), `control:"cancel"`. +- **Smoke TUI reale su Pi 0.80.3**: i property test coprono la *deriva del contratto* ma + **non** il comportamento delle primitive native (`select`/`input`/`editor`/`custom`) → serve + almeno uno smoke interattivo vero, come già fatto per il percorso RPC. - **RPC invariato**: backend bridge + 115 test frontend + 53 test harness restano da rigirare per non-regressione; il percorso descriptor non cambia.