From 57f8bfba63eee8b8fc7db96ff64ec70eb4ef44cc Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:04:28 +0330 Subject: [PATCH 1/6] delegate.js: classify transient vs permanent Gemini errors instead of always suggesting resume callGenerateContentOnce/callGenerateContent already tag network-layer failures with err.status (429/503 are the documented transient cases -- see client.js's own model-cascade, which already only retries on 429). Everything else -- a malformed request (400), auth/config problems (401/403, or no status at all e.g. "GEMINI_API_KEY is not set"), or "Gemini returned no candidates" from a safety/recitation block -- will reproduce identically on retry. The per-step failure message previously treated all of these the same way and always told the caller to resume, which is actively misleading for the non-transient cases. Added isTransientGeminiError() and branched the message accordingly. --- connectors/gemini/delegate.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/connectors/gemini/delegate.js b/connectors/gemini/delegate.js index 4753c3b..1448162 100644 --- a/connectors/gemini/delegate.js +++ b/connectors/gemini/delegate.js @@ -34,6 +34,18 @@ import { DEFAULT_OWNER } from "../../config.js"; const HARD_MAX_STEPS = 20; +// 429 (rate limit) and 503 (overloaded/high demand) are the only cases +// documented as transient -- see client.js's own model-fallback cascade, +// which deliberately only retries a different model on a 429 for the same +// reason. Everything else (400 malformed request, 401/403 auth, 404 unknown +// model, or no err.status at all -- e.g. "GEMINI_API_KEY is not set" thrown +// locally in client.js, or "Gemini returned no candidates" from a +// safety/recitation block) is a config or request problem that will +// reproduce identically on a resume, not something retrying fixes. +function isTransientGeminiError(err) { + return err?.status === 429 || err?.status === 503; +} + // Minimal line-based diff (LCS backtrace) -- good enough for investigation // summaries, not a full unified-diff implementation. Capped so a huge file // pair can't blow up the O(n*m) table. From db61b82f25b728d07b45b6b001cc3f44393628ed Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:04:40 +0330 Subject: [PATCH 2/6] delegate.js: use transient/permanent classification in the geminiChat failure message; guard err.message for non-Error throws --- connectors/gemini/delegate.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/connectors/gemini/delegate.js b/connectors/gemini/delegate.js index 1448162..7a5acdf 100644 --- a/connectors/gemini/delegate.js +++ b/connectors/gemini/delegate.js @@ -879,8 +879,12 @@ export async function runInvestigation({ task, max_steps = 6, resume_run_id }) { // iteration, but cheap and safe) and hand the caller everything they // need to resume instead of restarting. await saveCheckpoint(runId, { contents, transcript, stepsDone: step - 1, task: effectiveTask }); + const errMessage = err?.message ?? String(err); + const resumeHint = isTransientGeminiError(err) + ? ` ${transcript.length} tool call(s) already completed this run are saved. Call gemini_investigate again with resume_run_id: "${runId}" to continue from here instead of starting over. Checkpoint expires in 1 hour.` + : ` This does not look like a transient error (not a 429/503) -- resuming with resume_run_id: "${runId}" will likely reproduce the same failure, so check the underlying cause (e.g. GEMINI_API_KEY, request format, safety/recitation block) before retrying. The ${transcript.length} tool call(s) already completed are still saved if you want to resume anyway.`; return { - answer: `(Gemini call failed on step ${step}: ${err.message} -- ${transcript.length} tool call(s) already completed this run are saved. Call gemini_investigate again with resume_run_id: "${runId}" to continue from here instead of starting over. Checkpoint expires in 1 hour.)`, + answer: `(Gemini call failed on step ${step}: ${errMessage} --${resumeHint})`, steps: step - 1, transcript, runId, From e6e261cbd92ba49489259f27200ef94207f97d79 Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:05:05 +0330 Subject: [PATCH 3/6] delegate.js: don't silently discard a checkpoint when resuming with max_steps below steps already completed Previously, resuming with a max_steps ceiling lower than checkpoint.stepsDone meant startStep > cappedSteps, so the loop body never ran -- it fell straight through to deleteCheckpoint() + the generic "stopped after reaching the step cap" message, discarding a checkpoint that had real completed work in it without ever surfacing that work or explaining what happened. Added an explicit guard right after the checkpoint state is established: if there's nothing new to do because the requested ceiling is already met/exceeded, leave the checkpoint alone (still resumable with a higher max_steps) and return the existing transcript with a message that says so plainly. --- connectors/gemini/delegate.js | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/connectors/gemini/delegate.js b/connectors/gemini/delegate.js index 7a5acdf..b6c9c1a 100644 --- a/connectors/gemini/delegate.js +++ b/connectors/gemini/delegate.js @@ -860,6 +860,28 @@ export async function runInvestigation({ task, max_steps = 6, resume_run_id }) { startStep = 1; } + // Resuming with a max_steps ceiling that's already been met or exceeded + // by the checkpoint's own stepsDone (e.g. a checkpoint has 5 completed + // steps and the caller resumes with max_steps: 2) -- there's no budget + // left to take even one more step. Don't fall into the loop-and-fall- + // through path below: that unconditionally deletes the checkpoint via + // deleteCheckpoint(runId) once the loop exits, which would throw away a + // still-good, still-resumable checkpoint for no reason (the loop body + // simply never executes when startStep > cappedSteps), and the generic + // step-cap message doesn't explain that anything was actually completed. + // Leave the checkpoint alone -- it's still resumable with a higher + // max_steps -- and say so explicitly instead. + if (checkpoint && startStep > cappedSteps) { + return { + answer: `(This run already completed ${startStep - 1} step(s), which meets or exceeds the requested max_steps of ${cappedSteps} -- no new steps were taken this call. The checkpoint has NOT been discarded. Call gemini_investigate again with resume_run_id: "${runId}" and a higher max_steps to continue, or treat the ${transcript.length} tool call(s) below as the result so far.)`, + steps: startStep - 1, + transcript, + runId, + task: effectiveTask, + failed: true, + }; + } + for (let step = startStep; step <= cappedSteps; step++) { // On the final allowed step, withhold the function-calling tools // entirely instead of just reminding the model to wrap up: a text-only From 959d866028092223e8b60605e0e9e75d7efe9643 Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:05:28 +0330 Subject: [PATCH 4/6] delegate.js: guard the function-execution/transcript block so an unexpected throw doesn't lose checkpointed progress Unlike the geminiChat call (which has its own try/catch that checkpoints and returns a resumable failure), the loop over functionCalls -- fn.execute(), building the transcript entry, JSON.stringify(args) -- had no equivalent guard. Every current FUNCTIONS[].execute() happens to return a string today, so resultText.length never throws in practice, but nothing enforced that contract; a future function returning non-string (or a JSON.stringify throw on unexpected args shape) would escape runInvestigation entirely uncaught, land in tools.js's generic catch, and lose the run's runId/resume info even though the model's turn (and possibly several prior steps) had already completed. Added a defensive typeof guard on resultText and wrapped the whole per-call loop in a try/catch that mirrors the geminiChat failure path: checkpoint what's done and return a resumable failure instead of throwing past this function. --- connectors/gemini/delegate.js | 65 ++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/connectors/gemini/delegate.js b/connectors/gemini/delegate.js index b6c9c1a..6064403 100644 --- a/connectors/gemini/delegate.js +++ b/connectors/gemini/delegate.js @@ -933,25 +933,56 @@ export async function runInvestigation({ task, max_steps = 6, resume_run_id }) { contents.push({ role: "model", parts }); const responseParts = []; - for (const part of functionCalls) { - const { name, args, id } = part.functionCall; - const fn = FUNCTIONS.find((f) => f.name === name); - let resultText; - if (!fn) { - resultText = `Error: unknown function "${name}".`; - } else { - try { - resultText = await fn.execute(args || {}); - } catch (err) { - resultText = `Error: ${err.message}`; + try { + for (const part of functionCalls) { + const { name, args, id } = part.functionCall; + const fn = FUNCTIONS.find((f) => f.name === name); + let resultText; + if (!fn) { + resultText = `Error: unknown function "${name}".`; + } else { + try { + resultText = await fn.execute(args || {}); + } catch (err) { + resultText = `Error: ${err?.message ?? String(err)}`; + } + } + // Defensive: every FUNCTIONS[].execute() is expected to return a + // string. Guard against a future one accidentally returning + // something else (object, undefined, etc.) so this can't throw + // mid-transcript and take down the whole step -- see the outer + // catch below for why that matters. + if (typeof resultText !== "string") { + resultText = `Error: ${name} returned a non-string result (${typeof resultText}); this is a bug in the function's execute().`; } + transcript.push(`[step ${step}] ${name}(${JSON.stringify(args || {})}) -> ${resultText.length > 300 ? resultText.slice(0, 300) + "…" : resultText}`); + // Gemini 3 (current generateContent contract, verified 2026-07-25): function-result + // turns go back with role "user" (NOT "function" -- that was the older doc convention + // and is rejected by Gemini 3 models), and functionResponse.id echoes the model's + // original functionCall.id so the API can thread multi-call turns correctly. + responseParts.push({ functionResponse: { name, id, response: { result: resultText } } }); } - transcript.push(`[step ${step}] ${name}(${JSON.stringify(args || {})}) -> ${resultText.length > 300 ? resultText.slice(0, 300) + "…" : resultText}`); - // Gemini 3 (current generateContent contract, verified 2026-07-25): function-result - // turns go back with role "user" (NOT "function" -- that was the older doc convention - // and is rejected by Gemini 3 models), and functionResponse.id echoes the model's - // original functionCall.id so the API can thread multi-call turns correctly. - responseParts.push({ functionResponse: { name, id, response: { result: resultText } } }); + } catch (err) { + // Belt-and-suspenders: nothing inside the loop above should throw past + // its own per-call try/catch or the typeof guard anymore, but if + // something still does (a bug in a future function, an unexpected + // JSON.stringify(args) failure on a circular/exotic args shape, etc.), + // don't let it escape runInvestigation and land in tools.js's generic + // catch, which has no runId to offer -- that would silently lose this + // step's (and any prior steps') completed work. Checkpoint what's + // already done (this step's model turn was already pushed to + // `contents` above) and return the same resumable-failure shape as a + // geminiChat failure. + await saveCheckpoint(runId, { contents, transcript, stepsDone: step - 1, task: effectiveTask }); + const errMessage = err?.message ?? String(err); + return { + answer: `(Unexpected error while processing step ${step}'s function calls: ${errMessage} -- ${transcript.length} tool call(s) already completed this run are saved. Call gemini_investigate again with resume_run_id: "${runId}" to continue from here instead of starting over. Checkpoint expires in 1 hour.)`, + steps: step - 1, + transcript, + runId, + task: effectiveTask, + failed: true, + }; } // Step-budget reminder (added after the 2026-07-26 resume-truncation // bug): SYSTEM_PREAMBLE and the task's own formatting instructions only From eb273e021462025d02dd0784f59bc1c4b452381f Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:05:48 +0330 Subject: [PATCH 5/6] tools.js: validate max_steps >= 1 and guard err.message for non-Error throws max_steps had no floor check -- passing 0 or a negative number produced a nonsensical "(Investigation stopped after reaching the step cap of 0/-N without a final answer...)" instead of Gemini ever being called, with no indication the input itself was invalid. Zod's z.number().optional() accepts any finite number including 0/negatives/non-integers, so the guard has to be a runtime check. Also broadened the existing task-required guard's error message to mention this new check, and hardened `err.message` references to fall back to String(err) in case anything ever throws a non-Error value. --- connectors/gemini/tools.js | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/connectors/gemini/tools.js b/connectors/gemini/tools.js index 80368e7..4278dec 100644 --- a/connectors/gemini/tools.js +++ b/connectors/gemini/tools.js @@ -116,11 +116,24 @@ export function register(server) { }; } + // max_steps has no floor in its Zod type (z.number().optional() accepts + // 0, negatives, and non-integers), but runInvestigation's loop is a + // `for (step = startStep; step <= cappedSteps; ...)` that simply never + // executes when cappedSteps < startStep -- silently "succeeding" with + // zero Gemini calls made and a confusing "reached the step cap of 0" + // answer instead of surfacing that the input itself was invalid. + if (max_steps !== undefined && (!Number.isInteger(max_steps) || max_steps < 1)) { + return { + content: [{ type: "text", text: `Invalid max_steps: ${max_steps}. Must be a positive integer (at least 1); the hard cap is 20 regardless of a larger value.` }], + isError: true, + }; + } + let result; try { result = await runInvestigation({ task, max_steps, resume_run_id }); } catch (err) { - return { content: [{ type: "text", text: `Investigation failed: ${err.message}` }], isError: true }; + return { content: [{ type: "text", text: `Investigation failed: ${err?.message ?? String(err)}` }], isError: true }; } // On a resumed run, `task` may be undefined here (a fresh run always has From 8e04a39db0e3d053b38c71ef7a68a2b97f311ef5 Mon Sep 17 00:00:00 2001 From: allocsys <225476909+allocsys@users.noreply.github.com> Date: Sun, 26 Jul 2026 14:05:58 +0330 Subject: [PATCH 6/6] tools.js: guard err.message in Delegate_web_fetch's catches too, for consistency with delegate_gemini's hardening --- connectors/gemini/tools.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/connectors/gemini/tools.js b/connectors/gemini/tools.js index 4278dec..7def7a2 100644 --- a/connectors/gemini/tools.js +++ b/connectors/gemini/tools.js @@ -51,7 +51,7 @@ export function register(server) { try { fetched = await fetchUrl(url); } catch (err) { - return { content: [{ type: "text", text: `Fetch failed: ${err.message}` }], isError: true }; + return { content: [{ type: "text", text: `Fetch failed: ${err?.message ?? String(err)}` }], isError: true }; } let sourceText = fetched.contentType.includes("text/html") ? htmlToText(fetched.text) : fetched.text; @@ -68,7 +68,7 @@ export function register(server) { try { answer = await geminiGenerate(prompt); } catch (err) { - return { content: [{ type: "text", text: `Gemini call failed: ${err.message}` }], isError: true }; + return { content: [{ type: "text", text: `Gemini call failed: ${err?.message ?? String(err)}` }], isError: true }; } let notionNote = "";