This commit is contained in:
oogway 2026-01-30 14:24:00 +03:00 committed by GitHub
commit 0ab664d66e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 245 additions and 7 deletions

View File

@ -52,6 +52,7 @@ import {
import { DEFAULT_BOOTSTRAP_FILENAME } from "../../workspace.js"; import { DEFAULT_BOOTSTRAP_FILENAME } from "../../workspace.js";
import { buildSystemPromptReport } from "../../system-prompt-report.js"; import { buildSystemPromptReport } from "../../system-prompt-report.js";
import { resolveDefaultModelForAgent } from "../../model-selection.js"; import { resolveDefaultModelForAgent } from "../../model-selection.js";
import { sanitizeToolUseResultPairing } from "../../session-transcript-repair.js";
import { isAbortError } from "../abort.js"; import { isAbortError } from "../abort.js";
import { buildEmbeddedExtensionPaths } from "../extensions.js"; import { buildEmbeddedExtensionPaths } from "../extensions.js";
@ -535,9 +536,11 @@ export async function runEmbeddedAttempt(
validated, validated,
getDmHistoryLimitFromSessionKey(params.sessionKey, params.config), getDmHistoryLimitFromSessionKey(params.sessionKey, params.config),
); );
cacheTrace?.recordStage("session:limited", { messages: limited }); // Fix: Repair tool_use/tool_result pairings AFTER truncation (issue #4367)
if (limited.length > 0) { const repaired = sanitizeToolUseResultPairing(limited);
activeSession.agent.replaceMessages(limited); cacheTrace?.recordStage("session:limited", { messages: repaired });
if (repaired.length > 0) {
activeSession.agent.replaceMessages(repaired);
} }
} catch (err) { } catch (err) {
sessionManager.flushPendingToolResults?.(); sessionManager.flushPendingToolResults?.();

View File

@ -192,6 +192,41 @@ async function maybeQueueSubagentAnnounce(params: {
return "none"; 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<string, string> = {
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: { async function buildSubagentStatsLine(params: {
sessionKey: string; sessionKey: string;
startedAt?: number; startedAt?: number;
@ -299,6 +334,8 @@ export function buildSubagentSystemPrompt(params: {
export type SubagentRunOutcome = { export type SubagentRunOutcome = {
status: "ok" | "error" | "timeout" | "unknown"; status: "ok" | "error" | "timeout" | "unknown";
error?: string; error?: string;
errorType?: "model" | "tool" | "network" | "config" | "timeout" | "unknown";
errorHint?: string;
}; };
export async function runSubagentAnnounceFlow(params: { export async function runSubagentAnnounceFlow(params: {
@ -380,7 +417,7 @@ export async function runSubagentAnnounceFlow(params: {
: outcome.status === "timeout" : outcome.status === "timeout"
? "timed out" ? "timed out"
: outcome.status === "error" : outcome.status === "error"
? `failed: ${outcome.error || "unknown error"}` ? buildErrorStatusLabel(outcome)
: "finished with unknown status"; : "finished with unknown status";
// Build instructional message for main agent // Build instructional message for main agent

View File

@ -184,7 +184,16 @@ function ensureListener() {
entry.endedAt = endedAt; entry.endedAt = endedAt;
if (phase === "error") { if (phase === "error") {
const error = typeof evt.data?.error === "string" ? (evt.data.error as string) : undefined; 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 { } else {
entry.outcome = { status: "ok" }; entry.outcome = { status: "ok" };
} }

View File

@ -51,6 +51,85 @@ export type AgentRunLoopResult =
} }
| { kind: "final"; payload: ReplyPayload }; | { 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: { export async function runAgentTurnWithFallback(params: {
commandBody: string; commandBody: string;
followupRun: FollowupRun; followupRun: FollowupRun;
@ -204,6 +283,7 @@ export async function runAgentTurnWithFallback(params: {
return result; return result;
}) })
.catch((err) => { .catch((err) => {
const { message, type, hint } = categorizeError(err);
emitAgentEvent({ emitAgentEvent({
runId, runId,
stream: "lifecycle", stream: "lifecycle",
@ -211,7 +291,9 @@ export async function runAgentTurnWithFallback(params: {
phase: "error", phase: "error",
startedAt, startedAt,
endedAt: Date.now(), endedAt: Date.now(),
error: err instanceof Error ? err.message : String(err), error: message,
errorType: type,
errorHint: hint,
}, },
}); });
throw err; throw err;

View File

@ -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<CronJob> & Record<string, unknown> = {}): 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();
});
});

View File

@ -141,7 +141,9 @@ export function printCronList(jobs: CronJob[], runtime = defaultRuntime) {
const now = Date.now(); const now = Date.now();
for (const job of jobs) { 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 nameLabel = pad(truncate(job.name, CRON_NAME_PAD), CRON_NAME_PAD);
const scheduleLabel = pad( const scheduleLabel = pad(
truncate(formatSchedule(job.schedule), CRON_SCHEDULE_PAD), truncate(formatSchedule(job.schedule), CRON_SCHEDULE_PAD),