fix(harness): prevent Pi crash on unhandled throw in reviewer tool execute
The last live session crashed during reviewer_schema_linking: an uncaught exception (likely from execFileSync in currentPhase/phaseMeta or from cat.columns being undefined) rejected the async execute() Promise. Pi does not catch rejected tool Promises — Node.js treats them as unhandled rejections and kills the process. Fix: - Wrap all four reviewer tool execute bodies (select/decide/confirm/ schema_linking) in a top-level try/catch → returns a textResult on any unexpected error instead of crashing Pi. - reviewer_schema_linking: defensively re-parse `tables` if still a string (belt-and-suspenders over prepareArguments), guard cat.columns before .map(). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -727,6 +727,7 @@ export default function (pi) {
|
||||
prepareArguments: prepareReviewerArguments,
|
||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||
lockActive = true;
|
||||
try {
|
||||
const { session, title, options: opts, intro, advance } = params;
|
||||
const typeErr = validateDecisionTypes(ctx, opts, session);
|
||||
if (typeErr) return textResult(typeErr);
|
||||
@@ -745,14 +746,12 @@ export default function (pi) {
|
||||
});
|
||||
const resp = await emitAndWait(ctx, widget);
|
||||
const outcome = resolveSelectOutcome(opts, resp);
|
||||
// control responses (back/exit/other) are surfaced as text for the model to act on.
|
||||
if (outcome.kind === "freetext")
|
||||
return textResult(`Altro (reviewer): ${outcome.text}`);
|
||||
if (outcome.kind === "back")
|
||||
return textResult("Il reviewer vuole tornare indietro.");
|
||||
if (outcome.kind === "exit")
|
||||
return textResult("Il reviewer vuole uscire.");
|
||||
// concrete choice carrying a decision -> auto-confirm: persist directly, no second gate.
|
||||
if (outcome.kind === "decision") {
|
||||
const err = relayIfThtFails(
|
||||
ctx,
|
||||
@@ -765,10 +764,13 @@ export default function (pi) {
|
||||
`Decisione registrata (${outcome.decision.type}): ${outcome.option.label}.`,
|
||||
);
|
||||
}
|
||||
// bare choice (no decision payload) -> ask-only, non-persisting.
|
||||
return textResult(
|
||||
`Scelta del reviewer: ${outcome.option ? outcome.option.label : outcome.choice}`,
|
||||
);
|
||||
} catch (fatal) {
|
||||
const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
|
||||
return textResult(`[reviewer_select ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
@@ -801,6 +803,7 @@ export default function (pi) {
|
||||
prepareArguments: prepareReviewerArguments,
|
||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||
lockActive = true;
|
||||
try {
|
||||
const { session, title, options: opts, advance } = params;
|
||||
const typeErr = validateDecisionTypes(ctx, opts, session);
|
||||
if (typeErr) return textResult(typeErr);
|
||||
@@ -888,12 +891,14 @@ export default function (pi) {
|
||||
const msg = "Nessuna decisione registrata (il reviewer non ha selezionato opzioni di merito).";
|
||||
return textResult(adv.advanced ? msg + " Fase avanzata automaticamente." : msg);
|
||||
}
|
||||
// Decisions were recorded AND advance requested: the reviewer's selection IS the
|
||||
// approval — force-advance without a separate reviewer_confirm gate.
|
||||
const adv = advance ? forceAdvance(ctx, session) : { advanced: false };
|
||||
const parts = [`Registrate ${toAdd.length} decisioni: ${toAdd.map((d) => d.type).join(", ")}.`];
|
||||
if (adv.advanced) parts.push("Fase avanzata automaticamente — nessun gate aggiuntivo necessario.");
|
||||
return textResult(parts.join(" "));
|
||||
} catch (fatal) {
|
||||
const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
|
||||
return textResult(`[reviewer_decide ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
@@ -925,7 +930,12 @@ export default function (pi) {
|
||||
prepareArguments: prepareReviewerArguments,
|
||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||
lockActive = true;
|
||||
const { session, title, tables, advance } = params;
|
||||
try {
|
||||
const { session, title, tables: rawTables, advance } = params;
|
||||
const tables = Array.isArray(rawTables) ? rawTables : (() => {
|
||||
try { const p = JSON.parse(rawTables); return Array.isArray(p) ? p : []; } catch { return []; }
|
||||
})();
|
||||
if (!tables.length) return textResult("Parametro `tables` vuoto o non parsabile. Riprova con un array JSON di tabelle.");
|
||||
const phase = phaseId(ctx, currentPhase(ctx, session));
|
||||
|
||||
// Enrich each table with its full catalog columns (deterministic source).
|
||||
@@ -940,6 +950,9 @@ export default function (pi) {
|
||||
`Tabella '${t.name}' non caricabile dal catalogo (${msg}). Proponi solo tabelle presenti nel catalogo (usa 'tht schema render' / 'tht search' per verificarne i nomi).`,
|
||||
);
|
||||
}
|
||||
if (!Array.isArray(cat.columns)) {
|
||||
return textResult(`Catalogo per '${t.name}' non contiene colonne valide. Verifica con 'tht schema columns ${t.name} --json'.`);
|
||||
}
|
||||
const suggested = new Set(t.suggested_columns ?? []);
|
||||
enriched.push({
|
||||
id: t.id,
|
||||
@@ -1003,6 +1016,10 @@ export default function (pi) {
|
||||
const parts = [`Schema linking registrato dal reviewer (${n} tabelle + colonne curate). schema_linking.json scritto.`];
|
||||
if (adv.advanced) parts.push("Fase avanzata automaticamente — nessun gate aggiuntivo necessario.");
|
||||
return textResult(parts.join(" "));
|
||||
} catch (fatal) {
|
||||
const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
|
||||
return textResult(`[reviewer_schema_linking ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
@@ -1033,6 +1050,7 @@ export default function (pi) {
|
||||
prepareArguments: prepareReviewerArguments,
|
||||
async execute(_id, params, _signal, _onUpdate, ctx) {
|
||||
lockActive = true;
|
||||
try {
|
||||
const { session, kind, title } = params;
|
||||
let artifact = params.artifact;
|
||||
const curNum = currentPhase(ctx, session);
|
||||
@@ -1257,6 +1275,10 @@ export default function (pi) {
|
||||
return textResult(`SQL approvato (sessione ${session}).`);
|
||||
}
|
||||
return textResult(`Approvato (kind=${kind}, sessione ${session}).`);
|
||||
} catch (fatal) {
|
||||
const msg = (fatal.stderr || fatal.message || String(fatal)).toString().trim();
|
||||
return textResult(`[reviewer_confirm ERRORE INTERNO] ${msg}. Riprova o usa un approccio diverso.`);
|
||||
}
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user