fix: salvage truncated worker envelopes and log worker lifecycle
Models occasionally drop the closing tag of the result envelope while the JSON itself is complete; the strict parser rejected these and one repair attempt could not always recover, cascading into worker_error and dropping valid attachments. The parser now falls back to brace-balanced salvage when the closing tag is missing, and worker lifecycle events (start, resume, invalid envelope, repair, settle, worker_error) are logged.
This commit is contained in:
zenord
@@ -502,6 +502,39 @@ test("envelope parsers accept valid tails and reject invalid ones", () => {
|
||||
assert.equal(parseAssistantActions(`x\n<GORI_ASSISTANT_ACTION_V2>{"reply":"hi","actions":[{"type":"send_image"}]}</GORI_ASSISTANT_ACTION_V2>`), undefined);
|
||||
});
|
||||
|
||||
test("envelope parsers salvage envelopes missing only the closing tag", () => {
|
||||
// Complete JSON truncated at the closing tag is accepted, attachments included.
|
||||
assert.deepEqual(
|
||||
parseWorkerResult(`worker reply\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"s","attachments":[{"path":".gori-outbox/a.png"}]}`),
|
||||
{ status: "PENDING", summary: "s", question: undefined, workspaceDirty: undefined, attachments: [{ path: ".gori-outbox/a.png", mimeType: undefined }] }
|
||||
);
|
||||
// Braces and escapes inside strings do not confuse the balancing.
|
||||
assert.deepEqual(
|
||||
parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"use {a} and \\"}\\" end"}`),
|
||||
{ status: "PENDING", summary: "use {a} and \"}\" end", question: undefined, workspaceDirty: undefined, attachments: undefined }
|
||||
);
|
||||
// Trailing whitespace after the balanced JSON is fine, trailing content is not.
|
||||
assert.deepEqual(
|
||||
parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"s"}\n`),
|
||||
{ status: "PENDING", summary: "s", question: undefined, workspaceDirty: undefined, attachments: undefined }
|
||||
);
|
||||
// JSON cut off mid-object stays invalid.
|
||||
assert.equal(parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"cut off`), undefined);
|
||||
// Balanced JSON with trailing content (e.g. a closing tag plus more text) stays invalid.
|
||||
assert.equal(parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"s"}</GORI_WORKER_RESULT_V2> extra`), undefined);
|
||||
assert.equal(parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"s"} trailing`), undefined);
|
||||
// Salvage never accepts an invalid payload even when the JSON balances.
|
||||
assert.equal(parseWorkerResult(`r\n<GORI_WORKER_RESULT_V2>{"status":"SUCCESS","summary":"s"}`), undefined);
|
||||
|
||||
assert.deepEqual(
|
||||
parseAssistantActions(`ok\n<GORI_ASSISTANT_ACTION_V2>{"reply":"ok","actions":[]}`),
|
||||
{ reply: "ok", actions: [] }
|
||||
);
|
||||
assert.equal(parseAssistantActions(`ok\n<GORI_ASSISTANT_ACTION_V2>{"reply":"ok","actions":[{"type":"stop"}`), undefined);
|
||||
assert.equal(parseAssistantActions(`ok\n<GORI_ASSISTANT_ACTION_V2>{"reply":"ok","actions":[]}</GORI_ASSISTANT_ACTION_V2> more`), undefined);
|
||||
assert.equal(parseAssistantActions(`ok\n<GORI_ASSISTANT_ACTION_V2>{"reply":"ok","actions":[]} trailing`), undefined);
|
||||
});
|
||||
|
||||
test("an owner's own pending proposal blocks their start_next with an actionable correction", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
@@ -689,6 +722,57 @@ test("image attachments degrade to a text note when the agent has no image capab
|
||||
}
|
||||
});
|
||||
|
||||
test("a worker envelope truncated at the closing tag is salvaged without a repair round", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await harness.manager.prompt(request("create proposal: truncattach"));
|
||||
const proposal = harness.proposals.list()[0]!;
|
||||
await harness.manager.prompt(request("confirm"));
|
||||
await waitFor(() => harness.proposals.get(proposal.id)!.status === "pending");
|
||||
|
||||
const pending = harness.proposals.get(proposal.id)!.pending!;
|
||||
assert.equal(pending.summary, "made a pic");
|
||||
assert.equal(pending.attachments?.length, 1);
|
||||
assert.ok(pending.attachments![0]!.path.endsWith(path.join(".gori-outbox", "shot.png")));
|
||||
|
||||
const repairs = readLog(harness.logFile).filter((entry) => entry.method === "session/prompt"
|
||||
&& entry.text?.startsWith("Your previous response did not end with a valid GORI_WORKER_RESULT_V2"));
|
||||
assert.equal(repairs.length, 0);
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("a worker envelope with unbalanced JSON still goes through exactly one repair round", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
await harness.manager.prompt(request("create proposal: truncbad"));
|
||||
const proposal = harness.proposals.list()[0]!;
|
||||
await harness.manager.prompt(request("confirm"));
|
||||
await waitFor(() => harness.proposals.get(proposal.id)!.status === "pending");
|
||||
|
||||
assert.equal(harness.proposals.get(proposal.id)!.pending?.summary, "repaired");
|
||||
const repairs = readLog(harness.logFile).filter((entry) => entry.method === "session/prompt"
|
||||
&& entry.text?.startsWith("Your previous response did not end with a valid GORI_WORKER_RESULT_V2"));
|
||||
assert.equal(repairs.length, 1);
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("an assistant envelope truncated at the closing tag is salvaged without a repair round", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
const reply = await harness.manager.prompt(request("truncated assistant"));
|
||||
assert.equal(reply.text, "ok");
|
||||
const repairs = readLog(harness.logFile).filter((entry) => entry.method === "session/prompt"
|
||||
&& entry.text?.startsWith("Your previous response did not end with a valid GORI_ASSISTANT_ACTION_V2"));
|
||||
assert.equal(repairs.length, 0);
|
||||
} finally {
|
||||
await closeHarness(harness);
|
||||
}
|
||||
});
|
||||
|
||||
test("worker attachments are validated against the workspace and ride along with the owner event", async () => {
|
||||
const harness = await createHarness();
|
||||
try {
|
||||
|
||||
Vendored
+13
@@ -12,6 +12,7 @@ const logFile = process.env.FAKE_ACP_LOG;
|
||||
const log = (entry) => { if (logFile) fs.appendFileSync(logFile, `${JSON.stringify(entry)}\n`); };
|
||||
const assistantEnvelope = (reply, actions) => `${reply}\n<GORI_ASSISTANT_ACTION_V2>${JSON.stringify({ reply, actions })}</GORI_ASSISTANT_ACTION_V2>`;
|
||||
const workerEnvelope = (result) => `worker reply\n<GORI_WORKER_RESULT_V2>${JSON.stringify(result)}</GORI_WORKER_RESULT_V2>`;
|
||||
const workerEnvelopeTruncated = (result) => `worker reply\n<GORI_WORKER_RESULT_V2>${JSON.stringify(result)}`;
|
||||
|
||||
const app = acp.agent({ name: "fake-acp-agent" })
|
||||
.onRequest(acp.methods.agent.initialize, ({ params }) => {
|
||||
@@ -92,6 +93,7 @@ const app = acp.agent({ name: "fake-acp-agent" })
|
||||
else if (userText === "stop") response = assistantEnvelope("stopping", [{ type: "stop" }]);
|
||||
else if (userText === "cancel proposal") response = assistantEnvelope("cancelling", [{ type: "cancel" }]);
|
||||
else if (userText === "ack pending") response = assistantEnvelope("任务「Test proposal」还在等你确认。", []);
|
||||
else if (userText === "truncated assistant") response = `ok\n<GORI_ASSISTANT_ACTION_V2>${JSON.stringify({ reply: "ok", actions: [] })}`;
|
||||
else if (userText === "invalid assistant") response = "invalid without envelope";
|
||||
else response = assistantEnvelope("ok", []);
|
||||
await update(client, params.sessionId, response);
|
||||
@@ -121,6 +123,17 @@ const app = acp.agent({ name: "fake-acp-agent" })
|
||||
if (process.env.FAKE_ACP_WORKER_GATE_FILE) {
|
||||
while (!fs.existsSync(process.env.FAKE_ACP_WORKER_GATE_FILE)) await new Promise((resolve) => setTimeout(resolve, 5));
|
||||
}
|
||||
if (goal.includes("truncbad")) {
|
||||
await update(client, params.sessionId, `worker reply\n<GORI_WORKER_RESULT_V2>{"status":"PENDING","summary":"cut off mid json"`);
|
||||
return { stopReason: "end_turn" };
|
||||
}
|
||||
if (goal.includes("truncattach")) {
|
||||
const outbox = path.join(process.cwd(), ".gori-outbox");
|
||||
fs.mkdirSync(outbox, { recursive: true });
|
||||
fs.writeFileSync(path.join(outbox, "shot.png"), Buffer.concat([PNG_HEADER, Buffer.from("fake-png-payload-truncated")]));
|
||||
await update(client, params.sessionId, workerEnvelopeTruncated({ status: "PENDING", summary: "made a pic", attachments: [{ path: ".gori-outbox/shot.png", mimeType: "image/png" }] }));
|
||||
return { stopReason: "end_turn" };
|
||||
}
|
||||
if (goal.includes("attachbad")) {
|
||||
await update(client, params.sessionId, workerEnvelope({ status: "PENDING", summary: "made a pic", attachments: [{ path: "/tmp/evil.png" }, { path: "not-an-image.txt" }] }));
|
||||
return { stopReason: "end_turn" };
|
||||
|
||||
Reference in New Issue
Block a user