fix(backend): echo Pi's RPC id so reviewer gates unblock after answer
ctx.ui.input in `pi --mode rpc` correlates extension_ui_response on its own
top-level RPC id (crypto.randomUUID), not the descriptor id the gate carries
in `title`. SessionBridge replied with the descriptor id, so Pi silently
dropped the response and the model never resumed — every reviewer widget hung
after the human answered.
SessionBridge now stores Pi's top-level m.id (pendingPiId) and replies
extension_ui_response{ id: pendingPiId, value: <uiResponse> }; value still
carries the descriptor id so the gate's internal resp.id === descriptor.id
check still holds.
The fake-pi double had masked the bug by forcing m.id == descriptor.id; it now
mirrors real Pi (distinct randomUUID, correlate on it, drop unknown ids), with
a negative regression test. SKILL.md Phase 1 also now steers multi-answer
disambiguation to reviewer_decide (multiselect).
Tests: backend 67/67, tsc clean, fake-pi contract 2/2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -100,10 +100,21 @@ Prerequisite: you must already be in Phase 1.
|
||||
over real values) and `tht search find --kind evidence "<term>"`. The LSH exposes
|
||||
EVERY column where a value appears — it does not collapse to a single best match,
|
||||
so a value like "ablazione" may anchor on multiple columns.
|
||||
2. For each ambiguity (clinical term, population, time window, outcome), present a
|
||||
`reviewer_select` with the candidate interpretations (`recommended:true` on the
|
||||
best) + "Altro". When a clarification is settled, move on. Pass the FULL list of
|
||||
clarifications, not only the latest, when you close.
|
||||
2. For each ambiguity (clinical term, population, time window, outcome), present the
|
||||
candidate interpretations (`recommended:true` on the best) + "Altro". Pick the widget
|
||||
by the question's shape:
|
||||
- **Exactly one interpretation is correct** (mutually exclusive) → `reviewer_select`
|
||||
(single-pick; it only asks — then record the choice with a `reviewer_decide`
|
||||
`concept_clarified`).
|
||||
- **Several answers can be simultaneously true** (e.g. more than one valid population,
|
||||
procedure code, or time window) → do NOT use `reviewer_select`: single-pick buttons
|
||||
force one answer and mislead the reviewer. Use `reviewer_decide` directly (it emits a
|
||||
**multiselect checkbox** widget), one option per candidate, each carrying its own
|
||||
`concept_clarified` decision; the reviewer checks all that apply. Keep `advance:false`
|
||||
(Phase 1 still closes via the phase gate in step 3).
|
||||
|
||||
When a clarification is settled, move on. Pass the FULL list of clarifications, not
|
||||
only the latest, when you close.
|
||||
3. To close Phase 1: `reviewer_confirm kind:"phase"` (the deliberate "I'm done
|
||||
clarifying" gate). Do NOT add a separate `reviewer_confirm` after each individual
|
||||
clarification — those advance via `reviewer_decide` (`concept_clarified`), not via
|
||||
|
||||
@@ -1,8 +1,16 @@
|
||||
// harness/tests/fake_pi/fake_pi_rpc.mjs — scripted RPC test double (LF-only JSONL).
|
||||
import fs from "node:fs";
|
||||
import crypto from "node:crypto";
|
||||
const script = JSON.parse(fs.readFileSync(process.argv[2], "utf8"));
|
||||
const out = (evt) => process.stdout.write(JSON.stringify(evt) + "\n");
|
||||
|
||||
// Mirror real Pi (rpc-mode createDialogPromise): ctx.ui.input assegna un id RPC PROPRIO
|
||||
// (crypto.randomUUID), distinto dall'id interno del descriptor che viaggia opaco nel
|
||||
// `title`. La risposta si correla su quell'id RPC; un id sconosciuto viene scartato in
|
||||
// silenzio (esattamente cio' che provocava lo "stuck senza output" quando l'host
|
||||
// rispondeva con l'id del descriptor invece dell'id RPC).
|
||||
const pendingUi = new Map(); // piId (RPC) -> descriptor.id
|
||||
|
||||
let buf = "";
|
||||
process.stdin.on("data", (chunk) => {
|
||||
buf += chunk.toString("utf8");
|
||||
@@ -14,11 +22,17 @@ process.stdin.on("data", (chunk) => {
|
||||
for (const step of script.on_prompt ?? []) {
|
||||
if (step.ui_request_descriptor) {
|
||||
const d = step.ui_request_descriptor;
|
||||
out({ type: "extension_ui_request", id: d.id, method: "input", title: JSON.stringify(d) });
|
||||
const piId = crypto.randomUUID(); // id RPC proprio di Pi (≠ descriptor.id)
|
||||
pendingUi.set(piId, d.id);
|
||||
out({ type: "extension_ui_request", id: piId, method: "input", title: JSON.stringify(d) });
|
||||
} else { out(step); } // eventi non-UI (text_delta, agent_end, …) passano tali e quali
|
||||
}
|
||||
} else if (cmd.type === "extension_ui_response") {
|
||||
for (const evt of (script.on_response ?? {})[cmd.id] ?? []) out(evt);
|
||||
const descId = pendingUi.get(cmd.id); // correla SOLO sull'id RPC di Pi
|
||||
if (descId !== undefined) {
|
||||
pendingUi.delete(cmd.id);
|
||||
for (const evt of (script.on_response ?? {})[descId] ?? []) out(evt);
|
||||
}
|
||||
} else if (cmd.type === "get_available_models") {
|
||||
out({ type: "response", command: "get_available_models", id: cmd.id, success: true,
|
||||
data: { models: script.available_models ?? [] } });
|
||||
|
||||
@@ -6,6 +6,9 @@
|
||||
"options": [ {"id":"a","label":"interpretazione A"}, {"id":"b","label":"interpretazione B"} ],
|
||||
"reserved": ["back","exit","other"] } }
|
||||
],
|
||||
"on_response": { "u1": [ { "type": "agent_end" } ] },
|
||||
"on_response": { "u1": [
|
||||
{ "type": "message_update", "assistantMessageEvent": { "type": "text_delta", "contentIndex": 0, "delta": "Procedo." } },
|
||||
{ "type": "agent_end" }
|
||||
] },
|
||||
"available_models": [ {"provider":"zai","id":"glm-5.2"} ]
|
||||
}
|
||||
|
||||
@@ -4,7 +4,10 @@ import assert from "node:assert";
|
||||
import { spawn } from "node:child_process";
|
||||
import path from "node:path";
|
||||
|
||||
function drive(scriptPath, commands) {
|
||||
// Avvia il fake, manda il prompt e — quando arriva il widget — risponde con l'id RPC
|
||||
// indicato da `respondWith` (una funzione widget -> id). Mirror del vero Pi: la risposta
|
||||
// si correla sull'id RPC top-level, non sull'id interno del descriptor.
|
||||
function driveReactive(scriptPath, respondWith) {
|
||||
return new Promise((resolve) => {
|
||||
const fp = spawn("node", [path.join(import.meta.dirname, "fake_pi_rpc.mjs"), scriptPath]);
|
||||
const events = []; let buf = "";
|
||||
@@ -12,27 +15,45 @@ function drive(scriptPath, commands) {
|
||||
buf += c.toString("utf8");
|
||||
for (let nl; (nl = buf.indexOf("\n")) !== -1; ) {
|
||||
const line = buf.slice(0, nl).replace(/\r$/, ""); buf = buf.slice(nl + 1);
|
||||
if (line) events.push(JSON.parse(line));
|
||||
if (!line) continue;
|
||||
const evt = JSON.parse(line);
|
||||
events.push(evt);
|
||||
if (evt.type === "extension_ui_request" && evt.method === "input") {
|
||||
const desc = JSON.parse(evt.title);
|
||||
fp.stdin.write(JSON.stringify({
|
||||
type: "extension_ui_response",
|
||||
id: respondWith(evt),
|
||||
value: JSON.stringify({ id: desc.id, choices: ["a"], decision: { type: "concept_clarified" } }),
|
||||
}) + "\n");
|
||||
}
|
||||
}
|
||||
});
|
||||
fp.on("exit", () => resolve(events));
|
||||
for (const cmd of commands) fp.stdin.write(JSON.stringify(cmd) + "\n");
|
||||
fp.stdin.write(JSON.stringify({ type: "prompt", message: '/nuova-domanda "x"' }) + "\n");
|
||||
setTimeout(() => fp.stdin.end(), 300);
|
||||
});
|
||||
}
|
||||
|
||||
test("on prompt emette il widget F1; on response avanza", async () => {
|
||||
test("on prompt emette il widget F1 con id RPC distinto dal descriptor; rispondere con l'id RPC sblocca", async () => {
|
||||
const sp = path.join(import.meta.dirname, "scripts/f1_disambiguation.json");
|
||||
const events = await drive(sp, [
|
||||
{ type: "prompt", message: "/nuova-domanda \"x\"" },
|
||||
{ type: "extension_ui_response", id: "u1",
|
||||
value: JSON.stringify({ id: "u1", choices: ["a"], decision: { type: "concept_clarified" } }) },
|
||||
]);
|
||||
// Host corretto: risponde con l'id RPC top-level del widget.
|
||||
const events = await driveReactive(sp, (w) => w.id);
|
||||
const widget = events.find((e) => e.type === "extension_ui_request");
|
||||
assert.equal(widget.method, "input"); // shape nativa di Pi
|
||||
assert.equal(widget.id, "u1");
|
||||
const descriptor = JSON.parse(widget.title); // il descriptor viaggia nel title
|
||||
assert.equal(descriptor.widget, "select");
|
||||
assert.equal(descriptor.id, "u1");
|
||||
assert.notEqual(widget.id, descriptor.id); // l'id RPC di Pi NON e' l'id del descriptor
|
||||
// La risposta correlata sblocca il follow-up (testo + agent_end).
|
||||
assert.ok(events.some((e) => e.type === "message_update"));
|
||||
assert.ok(events.some((e) => e.type === "agent_end"));
|
||||
});
|
||||
|
||||
test("rispondere con l'id del descriptor (sbagliato) viene scartato: nessun follow-up", async () => {
|
||||
const sp = path.join(import.meta.dirname, "scripts/f1_disambiguation.json");
|
||||
// Host bug: risponde con l'id interno del descriptor invece dell'id RPC -> drop.
|
||||
const events = await driveReactive(sp, (w) => JSON.parse(w.title).id);
|
||||
assert.ok(events.some((e) => e.type === "extension_ui_request"));
|
||||
assert.ok(!events.some((e) => e.type === "agent_end")); // mai sbloccato
|
||||
assert.ok(!events.some((e) => e.type === "message_update"));
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user