diff --git a/src/agents/pi-embedded-runner/run/attempt.ts b/src/agents/pi-embedded-runner/run/attempt.ts index e83c3ae4a..622bdb7f4 100644 --- a/src/agents/pi-embedded-runner/run/attempt.ts +++ b/src/agents/pi-embedded-runner/run/attempt.ts @@ -52,6 +52,7 @@ import { import { DEFAULT_BOOTSTRAP_FILENAME } from "../../workspace.js"; import { buildSystemPromptReport } from "../../system-prompt-report.js"; import { resolveDefaultModelForAgent } from "../../model-selection.js"; +import { sanitizeToolUseResultPairing } from "../../session-transcript-repair.js"; import { isAbortError } from "../abort.js"; import { buildEmbeddedExtensionPaths } from "../extensions.js"; @@ -535,9 +536,11 @@ export async function runEmbeddedAttempt( validated, getDmHistoryLimitFromSessionKey(params.sessionKey, params.config), ); - cacheTrace?.recordStage("session:limited", { messages: limited }); - if (limited.length > 0) { - activeSession.agent.replaceMessages(limited); + // Fix: Repair tool_use/tool_result pairings AFTER truncation (issue #4367) + const repaired = sanitizeToolUseResultPairing(limited); + cacheTrace?.recordStage("session:limited", { messages: repaired }); + if (repaired.length > 0) { + activeSession.agent.replaceMessages(repaired); } } catch (err) { sessionManager.flushPendingToolResults?.(); diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index 444726efc..46d902e80 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -192,6 +192,41 @@ async function maybeQueueSubagentAnnounce(params: { return "none"; } +/** + * Build a descriptive error status label from outcome data. + * Includes error type, message, and hint if available. + */ +function buildErrorStatusLabel(outcome: SubagentRunOutcome): string { + const parts: string[] = []; + + // Start with "failed" + parts.push("failed"); + + // Add error type context + if (outcome.errorType) { + const typeLabel: Record = { + model: "API error", + tool: "tool error", + network: "network error", + config: "configuration error", + timeout: "timeout", + }; + const label = typeLabel[outcome.errorType] || "error"; + parts.push(`(${label}):`); + } + + // Add error message + const errorMsg = outcome.error || "unknown error"; + parts.push(errorMsg); + + // Add hint if available + if (outcome.errorHint) { + parts.push(`— ${outcome.errorHint}`); + } + + return parts.join(" "); +} + async function buildSubagentStatsLine(params: { sessionKey: string; startedAt?: number; @@ -299,6 +334,8 @@ export function buildSubagentSystemPrompt(params: { export type SubagentRunOutcome = { status: "ok" | "error" | "timeout" | "unknown"; error?: string; + errorType?: "model" | "tool" | "network" | "config" | "timeout" | "unknown"; + errorHint?: string; }; export async function runSubagentAnnounceFlow(params: { @@ -380,7 +417,7 @@ export async function runSubagentAnnounceFlow(params: { : outcome.status === "timeout" ? "timed out" : outcome.status === "error" - ? `failed: ${outcome.error || "unknown error"}` + ? buildErrorStatusLabel(outcome) : "finished with unknown status"; // Build instructional message for main agent diff --git a/src/agents/subagent-registry.ts b/src/agents/subagent-registry.ts index d325e40e2..dca685b72 100644 --- a/src/agents/subagent-registry.ts +++ b/src/agents/subagent-registry.ts @@ -184,7 +184,16 @@ function ensureListener() { entry.endedAt = endedAt; if (phase === "error") { const error = typeof evt.data?.error === "string" ? (evt.data.error as string) : undefined; - entry.outcome = { status: "error", error }; + const errorType = + typeof evt.data?.errorType === "string" ? (evt.data.errorType as string) : undefined; + const errorHint = + typeof evt.data?.errorHint === "string" ? (evt.data.errorHint as string) : undefined; + entry.outcome = { + status: "error", + error, + errorType: errorType as SubagentRunOutcome["errorType"], + errorHint, + }; } else { entry.outcome = { status: "ok" }; } diff --git a/src/auto-reply/reply/agent-runner-execution.ts b/src/auto-reply/reply/agent-runner-execution.ts index 21732f49f..36305bc68 100644 --- a/src/auto-reply/reply/agent-runner-execution.ts +++ b/src/auto-reply/reply/agent-runner-execution.ts @@ -51,6 +51,85 @@ export type AgentRunLoopResult = } | { kind: "final"; payload: ReplyPayload }; +/** + * Categorize errors to provide better error messages to users. + * Returns error message, type, and optional hint for remediation. + */ +function categorizeError(err: unknown): { + message: string; + type: "model" | "tool" | "network" | "config" | "timeout" | "unknown"; + hint?: string; +} { + const message = err instanceof Error ? err.message : String(err); + + // File system errors + if (message.includes("ENOENT") || message.includes("ENOTDIR")) { + return { message, type: "tool", hint: "File or directory not found" }; + } + if (message.includes("EACCES") || message.includes("EPERM")) { + return { message, type: "tool", hint: "Permission denied" }; + } + if (message.includes("EISDIR")) { + return { message, type: "tool", hint: "Expected file but found directory" }; + } + + // API/Model errors + if (message.includes("rate limit") || message.includes("429")) { + return { message, type: "model", hint: "Rate limit exceeded - retry in a few moments" }; + } + if ( + message.includes("401") || + message.includes("unauthorized") || + message.includes("authentication") + ) { + return { message, type: "config", hint: "Check API credentials and permissions" }; + } + if (message.includes("403") || message.includes("forbidden")) { + return { message, type: "config", hint: "Access denied - check permissions" }; + } + if (message.includes("400") || message.includes("invalid request")) { + return { message, type: "model", hint: "Invalid request parameters" }; + } + if (message.includes("500") || message.includes("503")) { + return { message, type: "model", hint: "API service error - try again later" }; + } + if (message.includes("quota") || message.includes("billing")) { + return { message, type: "config", hint: "Check billing and API quota limits" }; + } + + // Network errors + if (message.includes("ECONNREFUSED") || message.includes("ETIMEDOUT")) { + return { message, type: "network", hint: "Connection failed - check network connectivity" }; + } + if (message.includes("ENOTFOUND") || message.includes("DNS") || message.includes("EAI_AGAIN")) { + return { message, type: "network", hint: "DNS resolution failed - check hostname" }; + } + if (message.includes("ENETUNREACH") || message.includes("EHOSTUNREACH")) { + return { message, type: "network", hint: "Network unreachable - check connection" }; + } + + // Timeout errors + if ( + message.toLowerCase().includes("timeout") || + message.toLowerCase().includes("timed out") || + message.includes("ETIMEDOUT") + ) { + return { message, type: "timeout", hint: "Operation took too long - try increasing timeout" }; + } + + // Context/memory errors + if (message.includes("context") && message.includes("too large")) { + return { message, type: "model", hint: "Conversation too long - try clearing history" }; + } + + // Missing environment/config + if (message.includes("missing") && (message.includes("key") || message.includes("token"))) { + return { message, type: "config", hint: "Missing required configuration or credentials" }; + } + + return { message, type: "unknown" }; +} + export async function runAgentTurnWithFallback(params: { commandBody: string; followupRun: FollowupRun; @@ -204,6 +283,7 @@ export async function runAgentTurnWithFallback(params: { return result; }) .catch((err) => { + const { message, type, hint } = categorizeError(err); emitAgentEvent({ runId, stream: "lifecycle", @@ -211,7 +291,9 @@ export async function runAgentTurnWithFallback(params: { phase: "error", startedAt, endedAt: Date.now(), - error: err instanceof Error ? err.message : String(err), + error: message, + errorType: type, + errorHint: hint, }, }); throw err; diff --git a/src/cli/cron-cli/shared.test.ts b/src/cli/cron-cli/shared.test.ts new file mode 100644 index 000000000..17cd1fd8b --- /dev/null +++ b/src/cli/cron-cli/shared.test.ts @@ -0,0 +1,105 @@ +import { describe, expect, it, vi } from "vitest"; +import type { CronJob } from "../../cron/types.js"; +import { parseDurationMs, printCronList } from "./shared.js"; + +// Minimal CronJob factory +function makeCronJob(overrides: Partial & Record = {}): CronJob { + return { + id: "test-id-000", + name: "test-job", + enabled: true, + createdAtMs: Date.now(), + updatedAtMs: Date.now(), + schedule: { kind: "every", everyMs: 60_000 }, + sessionTarget: "isolated", + wakeMode: "prompt", + payload: { prompt: "hello" }, + state: {}, + ...overrides, + } as CronJob; +} + +describe("printCronList", () => { + const captureLog = () => { + const lines: string[] = []; + const runtime = { + log: vi.fn((msg: string) => lines.push(msg)), + error: vi.fn(), + }; + return { lines, runtime }; + }; + + it("prints 'No cron jobs.' for empty list", () => { + const { runtime } = captureLog(); + printCronList([], runtime as never); + expect(runtime.log).toHaveBeenCalledWith("No cron jobs."); + }); + + it("handles job with standard id property", () => { + const { lines, runtime } = captureLog(); + const job = makeCronJob({ id: "abc-123" }); + printCronList([job], runtime as never); + // Header + 1 job line + expect(lines).toHaveLength(2); + expect(lines[1]).toContain("abc-123"); + }); + + it("handles job with jobId instead of id (Gateway API variant)", () => { + const { lines, runtime } = captureLog(); + // Simulate Gateway returning jobId instead of id + const job = makeCronJob({ id: undefined as unknown as string, jobId: "gateway-456" } as never); + printCronList([job], runtime as never); + expect(lines).toHaveLength(2); + expect(lines[1]).toContain("gateway-456"); + }); + + it("prefers jobId over id when both are present", () => { + const { lines, runtime } = captureLog(); + const job = makeCronJob({ id: "fallback-id", jobId: "preferred-jobid" } as never); + printCronList([job], runtime as never); + expect(lines).toHaveLength(2); + expect(lines[1]).toContain("preferred-jobid"); + expect(lines[1]).not.toContain("fallback-id"); + }); + + it("does not crash when both id and jobId are missing", () => { + const { lines, runtime } = captureLog(); + const job = makeCronJob({ id: undefined as unknown as string }); + printCronList([job], runtime as never); + // Should not throw and should still print a line + expect(lines).toHaveLength(2); + }); + + it("prints multiple jobs", () => { + const { lines, runtime } = captureLog(); + const jobs = [ + makeCronJob({ id: "job-1", name: "first" }), + makeCronJob({ id: "job-2", name: "second" }), + ]; + printCronList(jobs, runtime as never); + // Header + 2 job lines + expect(lines).toHaveLength(3); + }); +}); + +describe("parseDurationMs", () => { + it.each([ + ["1s", 1000], + ["5m", 300_000], + ["2h", 7_200_000], + ["1d", 86_400_000], + ["500ms", 500], + ["1.5s", 1500], + ])("parses %s → %d", (input, expected) => { + expect(parseDurationMs(input)).toBe(expected); + }); + + it("returns null for empty string", () => { + expect(parseDurationMs("")).toBeNull(); + }); + + it("returns null for invalid input", () => { + expect(parseDurationMs("abc")).toBeNull(); + expect(parseDurationMs("-5s")).toBeNull(); + }); +}); diff --git a/src/cli/cron-cli/shared.ts b/src/cli/cron-cli/shared.ts index 5e5efc81a..2e3ddaa61 100644 --- a/src/cli/cron-cli/shared.ts +++ b/src/cli/cron-cli/shared.ts @@ -141,7 +141,9 @@ export function printCronList(jobs: CronJob[], runtime = defaultRuntime) { const now = Date.now(); for (const job of jobs) { - const idLabel = pad(job.id, CRON_ID_PAD); + // Gateway may return either 'id' or 'jobId' depending on API version + const id = (job as CronJob & { jobId?: string }).jobId ?? job.id ?? ""; + const idLabel = pad(id, CRON_ID_PAD); const nameLabel = pad(truncate(job.name, CRON_NAME_PAD), CRON_NAME_PAD); const scheduleLabel = pad( truncate(formatSchedule(job.schedule), CRON_SCHEDULE_PAD),