Compare commits
8 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 5abb3072c7 | |||
| 74b7329f64 | |||
| ba7f5a0b89 | |||
| b9383bbcfd | |||
| 8d6460da1c | |||
| 1f4c3031be | |||
| 4ad649ebb9 | |||
| 2960d51493 |
+5
-5
@@ -21,7 +21,7 @@ import { BEDROCK_MODEL_ID_ENV, isBedrockAnthropicId } from "../models.ts";
|
|||||||
import { getIdleMs, markActivity } from "../utils/activity.ts";
|
import { getIdleMs, markActivity } from "../utils/activity.ts";
|
||||||
import { formatJsonValue, log } from "../utils/cli.ts";
|
import { formatJsonValue, log } from "../utils/cli.ts";
|
||||||
import { installFromNpmTarball } from "../utils/install.ts";
|
import { installFromNpmTarball } from "../utils/install.ts";
|
||||||
import { detectProviderError } from "../utils/providerErrors.ts";
|
import { findProviderErrorMatch } from "../utils/providerErrors.ts";
|
||||||
import { addSkill, installBundledSkills } from "../utils/skills.ts";
|
import { addSkill, installBundledSkills } from "../utils/skills.ts";
|
||||||
import {
|
import {
|
||||||
DEFAULT_MAX_RETAINED_BYTES,
|
DEFAULT_MAX_RETAINED_BYTES,
|
||||||
@@ -639,10 +639,10 @@ export async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
|
|||||||
recentStderr.push(trimmed);
|
recentStderr.push(trimmed);
|
||||||
if (recentStderr.length > MAX_STDERR_LINES) recentStderr.shift();
|
if (recentStderr.length > MAX_STDERR_LINES) recentStderr.shift();
|
||||||
|
|
||||||
const providerError = detectProviderError(trimmed);
|
const match = findProviderErrorMatch(trimmed);
|
||||||
if (providerError) {
|
if (match) {
|
||||||
lastProviderError = providerError;
|
lastProviderError = match.label;
|
||||||
log.info(`» provider error detected (${providerError}): ${trimmed.substring(0, 500)}`);
|
log.info(`» provider error detected (${match.label}): ${match.excerpt}`);
|
||||||
} else {
|
} else {
|
||||||
log.debug(trimmed);
|
log.debug(trimmed);
|
||||||
}
|
}
|
||||||
|
|||||||
+59
-28
@@ -17,10 +17,12 @@ import { join } from "node:path";
|
|||||||
import { performance } from "node:perf_hooks";
|
import { performance } from "node:perf_hooks";
|
||||||
import { pullfrogMcpName } from "../external.ts";
|
import { pullfrogMcpName } from "../external.ts";
|
||||||
import { BEDROCK_MODEL_ID_ENV, modelAliases } from "../models.ts";
|
import { BEDROCK_MODEL_ID_ENV, modelAliases } from "../models.ts";
|
||||||
|
import type { ToolState } from "../toolState.ts";
|
||||||
import { getIdleMs, markActivity } from "../utils/activity.ts";
|
import { getIdleMs, markActivity } from "../utils/activity.ts";
|
||||||
|
import { type AgentDiagnostic, formatAgentHangBody } from "../utils/agentHangReport.ts";
|
||||||
import { formatJsonValue, log } from "../utils/cli.ts";
|
import { formatJsonValue, log } from "../utils/cli.ts";
|
||||||
import { installFromNpmTarball } from "../utils/install.ts";
|
import { installFromNpmTarball } from "../utils/install.ts";
|
||||||
import { detectProviderError } from "../utils/providerErrors.ts";
|
import { findProviderErrorMatch } from "../utils/providerErrors.ts";
|
||||||
import { addSkill, installBundledSkills } from "../utils/skills.ts";
|
import { addSkill, installBundledSkills } from "../utils/skills.ts";
|
||||||
import {
|
import {
|
||||||
DEFAULT_MAX_RETAINED_BYTES,
|
DEFAULT_MAX_RETAINED_BYTES,
|
||||||
@@ -307,6 +309,20 @@ interface OpenCodeStepFinishEvent {
|
|||||||
[key: string]: unknown;
|
[key: string]: unknown;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* tool-part state, mirroring opencode's `ToolState` (anomalyco/opencode
|
||||||
|
* `session/message-v2.ts`). error parts carry the reason on `error`,
|
||||||
|
* completed parts on `output` — reading the wrong field is what caused
|
||||||
|
* the silent `(no error message)` log in #662.
|
||||||
|
*
|
||||||
|
* Named `ToolPartState` locally (not `ToolState`) so it doesn't shadow the
|
||||||
|
* action-wide `ToolState` imported above.
|
||||||
|
*/
|
||||||
|
type ToolPartState =
|
||||||
|
| { status: "pending" | "running"; input?: unknown }
|
||||||
|
| { status: "completed"; input?: unknown; output: string }
|
||||||
|
| { status: "error"; input?: unknown; error: string };
|
||||||
|
|
||||||
interface OpenCodeToolUseEvent {
|
interface OpenCodeToolUseEvent {
|
||||||
type: "tool_use";
|
type: "tool_use";
|
||||||
timestamp?: number;
|
timestamp?: number;
|
||||||
@@ -315,7 +331,7 @@ interface OpenCodeToolUseEvent {
|
|||||||
id?: string;
|
id?: string;
|
||||||
callID?: string;
|
callID?: string;
|
||||||
tool?: string;
|
tool?: string;
|
||||||
state?: { status?: string; input?: unknown; output?: string };
|
state?: ToolPartState;
|
||||||
};
|
};
|
||||||
[key: string]: unknown;
|
[key: string]: unknown;
|
||||||
}
|
}
|
||||||
@@ -324,7 +340,7 @@ interface OpenCodeToolResultEvent {
|
|||||||
type: "tool_result";
|
type: "tool_result";
|
||||||
timestamp?: number;
|
timestamp?: number;
|
||||||
sessionID?: string;
|
sessionID?: string;
|
||||||
part?: { callID?: string; state?: { status?: string; output?: string } };
|
part?: { callID?: string; state?: ToolPartState };
|
||||||
tool_id?: string;
|
tool_id?: string;
|
||||||
status?: "success" | "error";
|
status?: "success" | "error";
|
||||||
output?: string;
|
output?: string;
|
||||||
@@ -409,6 +425,7 @@ type RunParams = {
|
|||||||
args: string[];
|
args: string[];
|
||||||
cwd: string;
|
cwd: string;
|
||||||
env: Record<string, string | undefined>;
|
env: Record<string, string | undefined>;
|
||||||
|
toolState: ToolState;
|
||||||
todoTracker?: TodoTracker | undefined;
|
todoTracker?: TodoTracker | undefined;
|
||||||
onActivityTimeout?: (() => void) | undefined;
|
onActivityTimeout?: (() => void) | undefined;
|
||||||
onToolUse?: ((event: { toolName: string; input: unknown }) => void) | undefined;
|
onToolUse?: ((event: { toolName: string; input: unknown }) => void) | undefined;
|
||||||
@@ -703,11 +720,9 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
// status="error" through the same `tool_use` event the CLI's run-loop
|
// status="error" through the same `tool_use` event the CLI's run-loop
|
||||||
// (and our injected plugin for subagent parts) emits — without this
|
// (and our injected plugin for subagent parts) emits — without this
|
||||||
// branch the only signal in the user's logs is `» <tool>(...)` with
|
// branch the only signal in the user's logs is `» <tool>(...)` with
|
||||||
// no indication the call failed. error info lives in `state.output`
|
// no indication the call failed.
|
||||||
// (an error string set by the tool layer).
|
|
||||||
if (event.part?.state?.status === "error") {
|
if (event.part?.state?.status === "error") {
|
||||||
const errorMsg = event.part.state.output ?? "(no error message)";
|
log.info(withLabel(label, `» tool call failed: ${event.part.state.error}`));
|
||||||
log.info(withLabel(label, `» tool call failed: ${errorMsg}`));
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// agent's explicit MCP report_progress takes priority over todo tracking
|
// agent's explicit MCP report_progress takes priority over todo tracking
|
||||||
@@ -723,8 +738,14 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
},
|
},
|
||||||
tool_result: (event: OpenCodeToolResultEvent) => {
|
tool_result: (event: OpenCodeToolResultEvent) => {
|
||||||
const toolId = event.part?.callID || event.tool_id;
|
const toolId = event.part?.callID || event.tool_id;
|
||||||
const status = event.part?.state?.status || event.status || "unknown";
|
const state = event.part?.state;
|
||||||
const output = event.part?.state?.output || event.output;
|
const status = state?.status ?? event.status ?? "unknown";
|
||||||
|
const payload =
|
||||||
|
state?.status === "completed"
|
||||||
|
? state.output
|
||||||
|
: state?.status === "error"
|
||||||
|
? state.error
|
||||||
|
: event.output;
|
||||||
const label = eventLabel(event);
|
const label = eventLabel(event);
|
||||||
|
|
||||||
timerFor(label).markToolResult();
|
timerFor(label).markToolResult();
|
||||||
@@ -743,12 +764,12 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
if (taskDispatchByCallID.size > 0 || pendingTaskDispatches.length > 0) {
|
if (taskDispatchByCallID.size > 0 || pendingTaskDispatches.length > 0) {
|
||||||
if (toolId && taskDispatchByCallID.has(toolId)) {
|
if (toolId && taskDispatchByCallID.has(toolId)) {
|
||||||
const dispatch = taskDispatchByCallID.get(toolId);
|
const dispatch = taskDispatchByCallID.get(toolId);
|
||||||
if (dispatch) emitSubagentFinished(dispatch, status, output, "exact");
|
if (dispatch) emitSubagentFinished(dispatch, status, payload, "exact");
|
||||||
} else {
|
} else {
|
||||||
const callIDIsKnownNonTask = toolId ? knownNonTaskCallIDs.has(toolId) : false;
|
const callIDIsKnownNonTask = toolId ? knownNonTaskCallIDs.has(toolId) : false;
|
||||||
if (!callIDIsKnownNonTask && pendingTaskDispatches.length > 0) {
|
if (!callIDIsKnownNonTask && pendingTaskDispatches.length > 0) {
|
||||||
const dispatch = pendingTaskDispatches[0]!;
|
const dispatch = pendingTaskDispatches[0]!;
|
||||||
emitSubagentFinished(dispatch, status, output, "fifo");
|
emitSubagentFinished(dispatch, status, payload, "fifo");
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -765,13 +786,8 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
`» ${params.label} tool_result${stepContext}: id=${toolId}, status=${status}, duration=${Math.round(toolDuration)}ms`
|
`» ${params.label} tool_result${stepContext}: id=${toolId}, status=${status}, duration=${Math.round(toolDuration)}ms`
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
if (output) {
|
if (payload) {
|
||||||
log.debug(
|
log.debug(withLabel(label, ` output: ${payload}`));
|
||||||
withLabel(
|
|
||||||
label,
|
|
||||||
` output: ${typeof output === "string" ? output : JSON.stringify(output)}`
|
|
||||||
)
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
if (toolDuration > 5000) {
|
if (toolDuration > 5000) {
|
||||||
log.info(
|
log.info(
|
||||||
@@ -784,11 +800,9 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
if (status === "error") {
|
if (status === "error") {
|
||||||
const errorMsg = typeof output === "string" ? output : JSON.stringify(output);
|
log.info(withLabel(label, `» tool call failed: ${payload ?? "(no error message)"}`));
|
||||||
log.info(withLabel(label, `» tool call failed: ${errorMsg}`));
|
} else if (payload) {
|
||||||
} else if (output) {
|
log.debug(withLabel(label, `tool output: ${payload}`));
|
||||||
const outputStr = typeof output === "string" ? output : JSON.stringify(output);
|
|
||||||
log.debug(withLabel(label, `tool output: ${outputStr}`));
|
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
error: (event: OpenCodeErrorEvent) => {
|
error: (event: OpenCodeErrorEvent) => {
|
||||||
@@ -928,6 +942,19 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
let lastProviderError: string | null = null;
|
let lastProviderError: string | null = null;
|
||||||
let agentErrorEvent: OpenCodeErrorEvent | null = null;
|
let agentErrorEvent: OpenCodeErrorEvent | null = null;
|
||||||
|
|
||||||
|
// shared with main.ts via toolState. updated in place as events stream and
|
||||||
|
// stderr accumulates so the outer activity-timeout catch sees the same
|
||||||
|
// context the harness's own catch path uses to format `result.error`.
|
||||||
|
// recentStderr is shared by reference; the scalar fields are mirrored on
|
||||||
|
// each update below.
|
||||||
|
const diagnostic: AgentDiagnostic = {
|
||||||
|
label: params.label,
|
||||||
|
recentStderr,
|
||||||
|
lastProviderError: undefined,
|
||||||
|
eventCount: 0,
|
||||||
|
};
|
||||||
|
params.toolState.agentDiagnostic = diagnostic;
|
||||||
|
|
||||||
// capped accumulator for the agent's narration. used as a post-run fallback
|
// capped accumulator for the agent's narration. used as a post-run fallback
|
||||||
// when `finalOutput` (the orchestrator's final assistant message) is empty.
|
// when `finalOutput` (the orchestrator's final assistant message) is empty.
|
||||||
// unbounded `output += text` previously grew to ~1 GiB on multi-lens Reviews
|
// unbounded `output += text` previously grew to ~1 GiB on multi-lens Reviews
|
||||||
@@ -986,6 +1013,7 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
eventCount++;
|
eventCount++;
|
||||||
|
diagnostic.eventCount = eventCount;
|
||||||
log.debug(JSON.stringify(event, null, 2));
|
log.debug(JSON.stringify(event, null, 2));
|
||||||
|
|
||||||
const timeSinceLastActivity = getIdleMs();
|
const timeSinceLastActivity = getIdleMs();
|
||||||
@@ -1024,10 +1052,11 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
recentStderr.push(trimmed);
|
recentStderr.push(trimmed);
|
||||||
if (recentStderr.length > MAX_STDERR_LINES) recentStderr.shift();
|
if (recentStderr.length > MAX_STDERR_LINES) recentStderr.shift();
|
||||||
|
|
||||||
const providerError = detectProviderError(trimmed);
|
const match = findProviderErrorMatch(trimmed);
|
||||||
if (providerError) {
|
if (match) {
|
||||||
lastProviderError = providerError;
|
lastProviderError = match.label;
|
||||||
log.info(`» provider error detected (${providerError}): ${trimmed.substring(0, 500)}`);
|
diagnostic.lastProviderError = match.label;
|
||||||
|
log.info(`» provider error detected (${match.label}): ${match.excerpt}`);
|
||||||
} else {
|
} else {
|
||||||
log.debug(trimmed);
|
log.debug(trimmed);
|
||||||
}
|
}
|
||||||
@@ -1158,10 +1187,11 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
|
|||||||
`» recent stderr (last ${Math.min(recentStderr.length, 10)} lines):\n${stderrContext}`
|
`» recent stderr (last ${Math.min(recentStderr.length, 10)} lines):\n${stderrContext}`
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const body = formatAgentHangBody({ diagnostic, isHang: isActivityTimeout, errorMessage });
|
||||||
return {
|
return {
|
||||||
success: false,
|
success: false,
|
||||||
output: finalOutput || output.toString(),
|
output: finalOutput || output.toString(),
|
||||||
error: `${errorMessage} [${diagnosis}]`,
|
error: body ?? `${errorMessage} [${diagnosis}]`,
|
||||||
usage: buildUsage(),
|
usage: buildUsage(),
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
@@ -1254,6 +1284,7 @@ export const opencode = agent({
|
|||||||
cliPath,
|
cliPath,
|
||||||
cwd: repoDir,
|
cwd: repoDir,
|
||||||
env,
|
env,
|
||||||
|
toolState: ctx.toolState,
|
||||||
todoTracker: ctx.todoTracker,
|
todoTracker: ctx.todoTracker,
|
||||||
onActivityTimeout: ctx.onActivityTimeout,
|
onActivityTimeout: ctx.onActivityTimeout,
|
||||||
onToolUse: ctx.onToolUse,
|
onToolUse: ctx.onToolUse,
|
||||||
|
|||||||
@@ -16,6 +16,7 @@ import {
|
|||||||
DEFAULT_ACTIVITY_TIMEOUT_MS,
|
DEFAULT_ACTIVITY_TIMEOUT_MS,
|
||||||
} from "./utils/activity.ts";
|
} from "./utils/activity.ts";
|
||||||
import { resolveAgent, resolveModel } from "./utils/agent.ts";
|
import { resolveAgent, resolveModel } from "./utils/agent.ts";
|
||||||
|
import { formatAgentHangBody } from "./utils/agentHangReport.ts";
|
||||||
import { apiFetch } from "./utils/apiFetch.ts";
|
import { apiFetch } from "./utils/apiFetch.ts";
|
||||||
import {
|
import {
|
||||||
formatApiKeyErrorSummary,
|
formatApiKeyErrorSummary,
|
||||||
@@ -1123,12 +1124,32 @@ export async function main(): Promise<MainResult> {
|
|||||||
? new BillingError(errorMessage, { code: "router_keylimit_exhausted" })
|
? new BillingError(errorMessage, { code: "router_keylimit_exhausted" })
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
|
// when the activity-timeout watchdog wins the race against the agent
|
||||||
|
// harness's own catch, the bare timer reject reason ("activity timeout:
|
||||||
|
// no output for 302s") tells the user nothing actionable. the harness
|
||||||
|
// keeps a structured diagnostic on toolState as it runs — recent stderr,
|
||||||
|
// last provider-error label, event count — and `formatAgentHangBody`
|
||||||
|
// renders that as a markdown body suitable for both the job summary tab
|
||||||
|
// and the PR progress comment.
|
||||||
|
//
|
||||||
|
// gated on isHang because the harness sets `agentDiagnostic` on entry,
|
||||||
|
// so any non-hang throw that hits the outer catch (e.g. the post-success
|
||||||
|
// output_schema validator, or a late cleanup throw after the run already
|
||||||
|
// succeeded) would otherwise render "Pullfrog failed" with stale event
|
||||||
|
// counts and silently drop the real `errorMessage`.
|
||||||
|
const isHang =
|
||||||
|
errorMessage.startsWith("activity timeout") || errorMessage.startsWith("agent still pending");
|
||||||
|
const hangBody = isHang
|
||||||
|
? formatAgentHangBody({ diagnostic: toolState.agentDiagnostic, isHang: true, errorMessage })
|
||||||
|
: null;
|
||||||
|
|
||||||
|
const apiKeySource = hangBody ?? errorMessage;
|
||||||
const apiKeyErrorSummary =
|
const apiKeyErrorSummary =
|
||||||
!billingError && isApiKeyAuthError(errorMessage)
|
!billingError && isApiKeyAuthError(apiKeySource)
|
||||||
? formatApiKeyErrorSummary({
|
? formatApiKeyErrorSummary({
|
||||||
owner: runContext.repo.owner,
|
owner: runContext.repo.owner,
|
||||||
name: runContext.repo.name,
|
name: runContext.repo.name,
|
||||||
raw: errorMessage,
|
raw: apiKeySource,
|
||||||
})
|
})
|
||||||
: null;
|
: null;
|
||||||
|
|
||||||
@@ -1136,7 +1157,10 @@ export async function main(): Promise<MainResult> {
|
|||||||
try {
|
try {
|
||||||
const errorSummary = billingError
|
const errorSummary = billingError
|
||||||
? formatBillingErrorSummary(billingError, runContext.repo.owner)
|
? formatBillingErrorSummary(billingError, runContext.repo.owner)
|
||||||
: (apiKeyErrorSummary ?? `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``);
|
: (apiKeyErrorSummary ??
|
||||||
|
(hangBody
|
||||||
|
? `### ❌ Pullfrog failed\n\n${hangBody}`
|
||||||
|
: `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``));
|
||||||
const usageSummary = formatUsageSummary(toolState.usageEntries);
|
const usageSummary = formatUsageSummary(toolState.usageEntries);
|
||||||
const parts = [errorSummary, toolState.lastProgressBody, usageSummary].filter(Boolean);
|
const parts = [errorSummary, toolState.lastProgressBody, usageSummary].filter(Boolean);
|
||||||
await writeSummary(parts.join("\n\n"));
|
await writeSummary(parts.join("\n\n"));
|
||||||
@@ -1145,7 +1169,7 @@ export async function main(): Promise<MainResult> {
|
|||||||
try {
|
try {
|
||||||
const commentBody = billingError
|
const commentBody = billingError
|
||||||
? formatBillingErrorSummary(billingError, runContext.repo.owner)
|
? formatBillingErrorSummary(billingError, runContext.repo.owner)
|
||||||
: (apiKeyErrorSummary ?? errorMessage);
|
: (apiKeyErrorSummary ?? hangBody ?? errorMessage);
|
||||||
await reportErrorToComment({ toolState, error: commentBody });
|
await reportErrorToComment({ toolState, error: commentBody });
|
||||||
} catch {
|
} catch {
|
||||||
// error reporting failed, but don't let it mask the original error
|
// error reporting failed, but don't let it mask the original error
|
||||||
|
|||||||
+234
-174
@@ -5,7 +5,7 @@ import type { Octokit, RestEndpointMethodTypes } from "@octokit/rest";
|
|||||||
import { type } from "arktype";
|
import { type } from "arktype";
|
||||||
import { log } from "../utils/cli.ts";
|
import { log } from "../utils/cli.ts";
|
||||||
import { countLines, createDiffCoverageState } from "../utils/diffCoverage.ts";
|
import { countLines, createDiffCoverageState } from "../utils/diffCoverage.ts";
|
||||||
import { $git } from "../utils/gitAuth.ts";
|
import { $git, $gitFetchWithDeepen } from "../utils/gitAuth.ts";
|
||||||
import { executeLifecycleHook } from "../utils/lifecycle.ts";
|
import { executeLifecycleHook } from "../utils/lifecycle.ts";
|
||||||
import { computeIncrementalDiff } from "../utils/rangeDiff.ts";
|
import { computeIncrementalDiff } from "../utils/rangeDiff.ts";
|
||||||
import { retry } from "../utils/retry.ts";
|
import { retry } from "../utils/retry.ts";
|
||||||
@@ -259,10 +259,10 @@ async function ensureBeforeShaReachable(params: EnsureBeforeShaParams): Promise<
|
|||||||
sha: params.sha,
|
sha: params.sha,
|
||||||
ref: tempBranch,
|
ref: tempBranch,
|
||||||
});
|
});
|
||||||
await $git(
|
await $gitFetchWithDeepen(
|
||||||
"fetch",
|
|
||||||
["--no-tags", ...(params.isShallow ? ["--depth=1"] : []), "origin", tempBranch],
|
["--no-tags", ...(params.isShallow ? ["--depth=1"] : []), "origin", tempBranch],
|
||||||
{ token: params.gitToken }
|
{ token: params.gitToken },
|
||||||
|
`before_sha temp branch ${tempBranch}`
|
||||||
);
|
);
|
||||||
log.debug(`» fetched before_sha via temp branch ${tempBranch}`);
|
log.debug(`» fetched before_sha via temp branch ${tempBranch}`);
|
||||||
return true;
|
return true;
|
||||||
@@ -410,9 +410,17 @@ export async function checkoutPrBranch(
|
|||||||
toolState.checkoutSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
|
toolState.checkoutSha = $("git", ["rev-parse", "HEAD"], { log: false }).trim();
|
||||||
const alreadyOnBranch = toolState.checkoutSha === pr.headSha;
|
const alreadyOnBranch = toolState.checkoutSha === pr.headSha;
|
||||||
|
|
||||||
// fetch base branch so origin/<base> exists for diff operations
|
// fetch base branch so origin/<base> exists for diff operations.
|
||||||
|
// wrap with deepen-retry: on shallow clones (the actions/checkout default
|
||||||
|
// is depth=1), repos with deep PR ancestry can't reach the baseRef tip in
|
||||||
|
// a single round trip, surfacing as `Could not read <sha>` / `remote did
|
||||||
|
// not send all necessary objects` (issue #656).
|
||||||
log.debug(`» fetching base branch (${pr.baseRef})...`);
|
log.debug(`» fetching base branch (${pr.baseRef})...`);
|
||||||
await $git("fetch", ["--no-tags", "origin", pr.baseRef], { token: gitToken });
|
await $gitFetchWithDeepen(
|
||||||
|
["--no-tags", "origin", pr.baseRef],
|
||||||
|
{ token: gitToken },
|
||||||
|
`base branch ${pr.baseRef}`
|
||||||
|
);
|
||||||
|
|
||||||
// alreadyOnBranch only matches for repeated checkout_pr calls for the same PR in one session
|
// alreadyOnBranch only matches for repeated checkout_pr calls for the same PR in one session
|
||||||
// (without the tip moving), or if an external setup already checked out the PR head.
|
// (without the tip moving), or if an external setup already checked out the PR head.
|
||||||
@@ -426,14 +434,21 @@ export async function checkoutPrBranch(
|
|||||||
// -B creates or resets the branch to match origin/baseBranch
|
// -B creates or resets the branch to match origin/baseBranch
|
||||||
$("git", ["checkout", "-B", pr.baseRef, `origin/${pr.baseRef}`], { log: false });
|
$("git", ["checkout", "-B", pr.baseRef, `origin/${pr.baseRef}`], { log: false });
|
||||||
|
|
||||||
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs)
|
// fetch PR branch using pull/{n}/head refspec (works for both fork and same-repo PRs).
|
||||||
|
// two transient classes wrap this fetch:
|
||||||
|
// - shallow-unreachable (`Could not read <sha>` etc.) — handled by the
|
||||||
|
// inner `$gitFetchWithDeepen` deepen-retry (one shot, see issue #656)
|
||||||
|
// - pull/N/head webhook race (`couldn't find remote ref pull/N/head`) —
|
||||||
|
// handled by the outer retry below (see issue #591)
|
||||||
log.debug(`» fetching PR #${pr.number} (${localBranch})...`);
|
log.debug(`» fetching PR #${pr.number} (${localBranch})...`);
|
||||||
await retry(
|
await retry(
|
||||||
async () => {
|
async () => {
|
||||||
try {
|
try {
|
||||||
await $git("fetch", ["--no-tags", "origin", `+pull/${pr.number}/head:${localBranch}`], {
|
await $gitFetchWithDeepen(
|
||||||
token: gitToken,
|
["--no-tags", "origin", `+pull/${pr.number}/head:${localBranch}`],
|
||||||
});
|
{ token: gitToken },
|
||||||
|
`PR #${pr.number}`
|
||||||
|
);
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
// on the webhook race, check whether the PR still matches what we
|
// on the webhook race, check whether the PR still matches what we
|
||||||
// dispatched on. if it's been closed/merged or the head SHA moved,
|
// dispatched on. if it's been closed/merged or the head SHA moved,
|
||||||
@@ -588,7 +603,192 @@ export async function checkoutPrBranch(
|
|||||||
return { hookWarning: postCheckoutHook.warning };
|
return { hookWarning: postCheckoutHook.warning };
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* dedupes concurrent `checkout_pr` calls for the same PR. agents (notably
|
||||||
|
* Sonnet/Claude) occasionally emit duplicate parallel tool_use blocks for the
|
||||||
|
* same args in one turn; without this, both invocations race
|
||||||
|
* `checkoutPrBranch` against the same `.git/shallow.lock` and one fails with
|
||||||
|
* `File exists` (issue #642). cleared in `finally` so subsequent same-PR
|
||||||
|
* calls re-do the work normally.
|
||||||
|
*/
|
||||||
|
const inFlightCheckouts = new Map<number, Promise<CheckoutPrResult>>();
|
||||||
|
|
||||||
export function CheckoutPrTool(ctx: ToolContext) {
|
export function CheckoutPrTool(ctx: ToolContext) {
|
||||||
|
const runCheckout = async (pull_number: number): Promise<CheckoutPrResult> => {
|
||||||
|
const prResponse = await ctx.octokit.rest.pulls.get({
|
||||||
|
owner: ctx.repo.owner,
|
||||||
|
repo: ctx.repo.name,
|
||||||
|
pull_number,
|
||||||
|
});
|
||||||
|
|
||||||
|
const headRepo = prResponse.data.head.repo;
|
||||||
|
if (!headRepo) {
|
||||||
|
throw new Error(`PR #${pull_number} source repository was deleted`);
|
||||||
|
}
|
||||||
|
|
||||||
|
const pr: PrData = {
|
||||||
|
number: pull_number,
|
||||||
|
headSha: prResponse.data.head.sha,
|
||||||
|
headRef: prResponse.data.head.ref,
|
||||||
|
headRepoFullName: headRepo.full_name,
|
||||||
|
baseRef: prResponse.data.base.ref,
|
||||||
|
baseRepoFullName: prResponse.data.base.repo.full_name,
|
||||||
|
maintainerCanModify: prResponse.data.maintainer_can_modify,
|
||||||
|
};
|
||||||
|
|
||||||
|
const checkoutResult = await checkoutPrBranch(pr, {
|
||||||
|
octokit: ctx.octokit,
|
||||||
|
owner: ctx.repo.owner,
|
||||||
|
name: ctx.repo.name,
|
||||||
|
gitToken: ctx.gitToken,
|
||||||
|
toolState: ctx.toolState,
|
||||||
|
shell: ctx.payload.shell,
|
||||||
|
postCheckoutScript: ctx.postCheckoutScript,
|
||||||
|
beforeSha: ctx.toolState.beforeSha,
|
||||||
|
});
|
||||||
|
|
||||||
|
const tempDir = process.env.PULLFROG_TEMP_DIR;
|
||||||
|
if (!tempDir) {
|
||||||
|
throw new Error(
|
||||||
|
"PULLFROG_TEMP_DIR not set - checkout_pr must run in pullfrog action context"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
const headShort = ctx.toolState.checkoutSha!.slice(0, 7);
|
||||||
|
|
||||||
|
// compute incremental diff if we have a beforeSha to compare against
|
||||||
|
let incrementalDiffPath: string | undefined;
|
||||||
|
if (ctx.toolState.beforeSha && ctx.toolState.checkoutSha) {
|
||||||
|
const beforeShort = ctx.toolState.beforeSha.slice(0, 7);
|
||||||
|
const incremental = computeIncrementalDiff({
|
||||||
|
baseBranch: pr.baseRef,
|
||||||
|
beforeSha: ctx.toolState.beforeSha,
|
||||||
|
headSha: ctx.toolState.checkoutSha,
|
||||||
|
});
|
||||||
|
if (incremental) {
|
||||||
|
incrementalDiffPath = join(
|
||||||
|
tempDir,
|
||||||
|
`pr-${pull_number}-${beforeShort}-${headShort}-incremental.diff`
|
||||||
|
);
|
||||||
|
writeFileSync(incrementalDiffPath, incremental);
|
||||||
|
log.info(
|
||||||
|
`» incremental diff computed (${incremental.length} bytes) → ${incrementalDiffPath}`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// fetch PR files and format with line numbers
|
||||||
|
const formatResult = await fetchAndFormatPrDiff(ctx, pull_number);
|
||||||
|
const diffPreview = formatResult.content.split("\n").slice(0, 100).join("\n");
|
||||||
|
log.debug(`formatted diff preview (first 100 lines):\n${diffPreview}`);
|
||||||
|
const diffPath = join(tempDir, `pr-${pull_number}-${headShort}.diff`);
|
||||||
|
writeFileSync(diffPath, formatResult.content);
|
||||||
|
log.debug(`wrote diff to ${diffPath} (${formatResult.content.length} bytes)`);
|
||||||
|
ctx.toolState.diffCoverage = createDiffCoverageState({
|
||||||
|
diffPath,
|
||||||
|
totalLines: countLines({ content: formatResult.content }),
|
||||||
|
toc: formatResult.toc,
|
||||||
|
previous: ctx.toolState.diffCoverage,
|
||||||
|
});
|
||||||
|
log.debug(
|
||||||
|
`» diff coverage initialized: diffPath=${diffPath}, totalLines=${ctx.toolState.diffCoverage.totalLines}, tocEntries=${ctx.toolState.diffCoverage.tocEntries.length}`
|
||||||
|
);
|
||||||
|
|
||||||
|
// cache commentable-lines snapshot so review-time validation matches what
|
||||||
|
// GitHub will anchor to (commit_id=checkoutSha), even if the PR is updated
|
||||||
|
// between checkout and review.
|
||||||
|
const cached = new Map<string, ReturnType<typeof commentableLinesForFile>>();
|
||||||
|
for (const file of formatResult.files) {
|
||||||
|
cached.set(file.filename, commentableLinesForFile(file.patch));
|
||||||
|
}
|
||||||
|
ctx.toolState.commentableLinesByFile = cached;
|
||||||
|
ctx.toolState.commentableLinesPullNumber = pull_number;
|
||||||
|
ctx.toolState.commentableLinesCheckoutSha = ctx.toolState.checkoutSha;
|
||||||
|
|
||||||
|
const incrementalInstructions = incrementalDiffPath
|
||||||
|
? ` IMPORTANT: incrementalDiffPath contains ONLY the changes since the last reviewed version ` +
|
||||||
|
`(computed via range-diff). you MUST read incrementalDiffPath FIRST to understand what changed, ` +
|
||||||
|
`then use diffPath for full PR context. do NOT skip the incremental diff.`
|
||||||
|
: "";
|
||||||
|
|
||||||
|
// commit metadata relative to the PR base (e.g. main). use origin/<base>
|
||||||
|
// because the local base ref may not exist after a shallow fetch. cap
|
||||||
|
// the log so a PR with thousands of commits doesn't blow up the tool
|
||||||
|
// response. if the base ref can't be resolved (e.g. shallow fetch that
|
||||||
|
// didn't pull down origin/<base>), degrade gracefully rather than
|
||||||
|
// failing the whole checkout_pr call over metadata.
|
||||||
|
const COMMIT_LOG_MAX = 200;
|
||||||
|
const baseRange = `origin/${pr.baseRef}..HEAD`;
|
||||||
|
let commitCount = 0;
|
||||||
|
let commitLog = "";
|
||||||
|
let commitLogUnavailable = false;
|
||||||
|
try {
|
||||||
|
commitCount = parseInt(
|
||||||
|
$("git", ["rev-list", "--count", baseRange], { log: false }).trim() || "0",
|
||||||
|
10
|
||||||
|
);
|
||||||
|
commitLog = $("git", ["log", "--oneline", `--max-count=${COMMIT_LOG_MAX}`, baseRange], {
|
||||||
|
log: false,
|
||||||
|
});
|
||||||
|
} catch (err) {
|
||||||
|
commitLogUnavailable = true;
|
||||||
|
log.debug(
|
||||||
|
`» unable to compute commit metadata for ${baseRange}: ${err instanceof Error ? err.message : String(err)}`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
const commitLogTruncated = commitCount > COMMIT_LOG_MAX;
|
||||||
|
|
||||||
|
const hookWarningInstructions = checkoutResult.hookWarning
|
||||||
|
? ` HOOK WARNING: the post-checkout lifecycle hook reported a non-fatal failure (see hookWarning). ` +
|
||||||
|
`decide whether to retry based on the guidance in that field before proceeding.`
|
||||||
|
: "";
|
||||||
|
|
||||||
|
const commitLogInstructions = commitLogUnavailable
|
||||||
|
? ` NOTE: commit metadata is partial (base ref unreachable, likely a shallow fetch). ` +
|
||||||
|
`commitCount/commitLog may be 0/empty or incomplete; treat them as "unknown" rather than "no commits", ` +
|
||||||
|
`and use \`git log\` directly if you need the full history.`
|
||||||
|
: commitLogTruncated
|
||||||
|
? ` NOTE: commitLog was capped at ${COMMIT_LOG_MAX} entries out of ${commitCount} commits; ` +
|
||||||
|
`use \`git log\` directly if you need the full history.`
|
||||||
|
: "";
|
||||||
|
|
||||||
|
return {
|
||||||
|
success: true,
|
||||||
|
number: prResponse.data.number,
|
||||||
|
title: prResponse.data.title,
|
||||||
|
body: prResponse.data.body,
|
||||||
|
base: pr.baseRef,
|
||||||
|
localBranch: `pr-${pull_number}`,
|
||||||
|
remoteBranch: `refs/heads/${pr.headRef}`,
|
||||||
|
isFork: pr.headRepoFullName !== pr.baseRepoFullName,
|
||||||
|
maintainerCanModify: pr.maintainerCanModify,
|
||||||
|
url: prResponse.data.html_url,
|
||||||
|
headRepo: pr.headRepoFullName,
|
||||||
|
diffPath,
|
||||||
|
incrementalDiffPath,
|
||||||
|
toc: formatResult.toc,
|
||||||
|
commitCount,
|
||||||
|
commitLog,
|
||||||
|
commitLogTruncated,
|
||||||
|
commitLogUnavailable,
|
||||||
|
hookWarning: checkoutResult.hookWarning,
|
||||||
|
instructions:
|
||||||
|
`the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. ` +
|
||||||
|
`use the TOC line ranges as your checklist and read specific files from the diff instead of reading the entire file. ` +
|
||||||
|
`for example, if the TOC says "src/foo.ts → lines 5-42", read lines 5-42 from diffPath to see that file's changes. ` +
|
||||||
|
`review files selectively based on relevance rather than reading everything sequentially. ` +
|
||||||
|
`to inspect the PR's changed files, use diffPath — do NOT run \`git diff <base>..<head>\` to re-derive what's already in diffPath. the formatted diff with line numbers is authoritative. ` +
|
||||||
|
`\`git log\` and \`git diff --stat\` are fine for commit-range overview, and \`git diff\` / \`git diff --cached\` are fine for inspecting *your own* uncommitted changes — but PR review content MUST come from diffPath. ` +
|
||||||
|
`before your review is submitted, a one-time coverage pre-flight may error listing unread TOC regions. ` +
|
||||||
|
`retry the same create_pull_request_review call to proceed — optionally after reading the listed ranges. the pre-flight will not block again this session. ` +
|
||||||
|
`the local branch is 'localBranch' (pr-{number}), not the remote branch name. ` +
|
||||||
|
`when pushing, omit branchName to use the current branch. do not use remoteBranch as a local branch name.` +
|
||||||
|
incrementalInstructions +
|
||||||
|
hookWarningInstructions +
|
||||||
|
commitLogInstructions,
|
||||||
|
} satisfies CheckoutPrResult;
|
||||||
|
};
|
||||||
|
|
||||||
return tool({
|
return tool({
|
||||||
name: "checkout_pr",
|
name: "checkout_pr",
|
||||||
description:
|
description:
|
||||||
@@ -599,178 +799,38 @@ export function CheckoutPrTool(ctx: ToolContext) {
|
|||||||
"If the error mentions `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, that's a stale lock from a prior timed-out fetch — remove it via the shell tool (`rm -f .git/shallow.lock .git/index.lock`) and retry.",
|
"If the error mentions `.git/shallow.lock: File exists` or `.git/index.lock: File exists`, that's a stale lock from a prior timed-out fetch — remove it via the shell tool (`rm -f .git/shallow.lock .git/index.lock`) and retry.",
|
||||||
parameters: CheckoutPr,
|
parameters: CheckoutPr,
|
||||||
execute: execute(async ({ pull_number }) => {
|
execute: execute(async ({ pull_number }) => {
|
||||||
const prResponse = await ctx.octokit.rest.pulls.get({
|
const inFlight = inFlightCheckouts.get(pull_number);
|
||||||
owner: ctx.repo.owner,
|
if (inFlight) {
|
||||||
repo: ctx.repo.name,
|
log.info(`» checkout_pr({pull_number:${pull_number}}) already in flight — sharing result`);
|
||||||
pull_number,
|
return inFlight;
|
||||||
});
|
|
||||||
|
|
||||||
const headRepo = prResponse.data.head.repo;
|
|
||||||
if (!headRepo) {
|
|
||||||
throw new Error(`PR #${pull_number} source repository was deleted`);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const pr: PrData = {
|
// refuse to clobber an active checkout if the working tree is dirty —
|
||||||
number: pull_number,
|
// forces the agent to commit/push or discard before switching contexts
|
||||||
headSha: prResponse.data.head.sha,
|
// instead of silently overwriting uncommitted work. `issueNumber`
|
||||||
headRef: prResponse.data.head.ref,
|
// tracks any issue/PR the agent has touched (issues and PRs share
|
||||||
headRepoFullName: headRepo.full_name,
|
// GitHub's number space); the guard fires only when the agent is
|
||||||
baseRef: prResponse.data.base.ref,
|
// switching to a *different* number with a dirty tree, which captures
|
||||||
baseRepoFullName: prResponse.data.base.repo.full_name,
|
// the legitimate "stop, you have unsaved work" case regardless of
|
||||||
maintainerCanModify: prResponse.data.maintainer_can_modify,
|
// whether the prior number was an issue or a PR.
|
||||||
};
|
const current = ctx.toolState.issueNumber;
|
||||||
|
if (current !== undefined && current !== pull_number) {
|
||||||
const checkoutResult = await checkoutPrBranch(pr, {
|
const dirty = $("git", ["status", "--porcelain"], { log: false }).trim();
|
||||||
octokit: ctx.octokit,
|
if (dirty) {
|
||||||
owner: ctx.repo.owner,
|
throw new Error(
|
||||||
name: ctx.repo.name,
|
`cannot checkout PR #${pull_number} while the working tree has uncommitted changes. ` +
|
||||||
gitToken: ctx.gitToken,
|
`commit, push, or discard them before switching. dirty paths:\n${dirty}`
|
||||||
toolState: ctx.toolState,
|
|
||||||
shell: ctx.payload.shell,
|
|
||||||
postCheckoutScript: ctx.postCheckoutScript,
|
|
||||||
beforeSha: ctx.toolState.beforeSha,
|
|
||||||
});
|
|
||||||
|
|
||||||
const tempDir = process.env.PULLFROG_TEMP_DIR;
|
|
||||||
if (!tempDir) {
|
|
||||||
throw new Error(
|
|
||||||
"PULLFROG_TEMP_DIR not set - checkout_pr must run in pullfrog action context"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
|
|
||||||
const headShort = ctx.toolState.checkoutSha!.slice(0, 7);
|
|
||||||
|
|
||||||
// compute incremental diff if we have a beforeSha to compare against
|
|
||||||
let incrementalDiffPath: string | undefined;
|
|
||||||
if (ctx.toolState.beforeSha && ctx.toolState.checkoutSha) {
|
|
||||||
const beforeShort = ctx.toolState.beforeSha.slice(0, 7);
|
|
||||||
const incremental = computeIncrementalDiff({
|
|
||||||
baseBranch: pr.baseRef,
|
|
||||||
beforeSha: ctx.toolState.beforeSha,
|
|
||||||
headSha: ctx.toolState.checkoutSha,
|
|
||||||
});
|
|
||||||
if (incremental) {
|
|
||||||
incrementalDiffPath = join(
|
|
||||||
tempDir,
|
|
||||||
`pr-${pull_number}-${beforeShort}-${headShort}-incremental.diff`
|
|
||||||
);
|
|
||||||
writeFileSync(incrementalDiffPath, incremental);
|
|
||||||
log.info(
|
|
||||||
`» incremental diff computed (${incremental.length} bytes) → ${incrementalDiffPath}`
|
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// fetch PR files and format with line numbers
|
const promise = runCheckout(pull_number);
|
||||||
const formatResult = await fetchAndFormatPrDiff(ctx, pull_number);
|
inFlightCheckouts.set(pull_number, promise);
|
||||||
const diffPreview = formatResult.content.split("\n").slice(0, 100).join("\n");
|
|
||||||
log.debug(`formatted diff preview (first 100 lines):\n${diffPreview}`);
|
|
||||||
const diffPath = join(tempDir, `pr-${pull_number}-${headShort}.diff`);
|
|
||||||
writeFileSync(diffPath, formatResult.content);
|
|
||||||
log.debug(`wrote diff to ${diffPath} (${formatResult.content.length} bytes)`);
|
|
||||||
ctx.toolState.diffCoverage = createDiffCoverageState({
|
|
||||||
diffPath,
|
|
||||||
totalLines: countLines({ content: formatResult.content }),
|
|
||||||
toc: formatResult.toc,
|
|
||||||
previous: ctx.toolState.diffCoverage,
|
|
||||||
});
|
|
||||||
log.debug(
|
|
||||||
`» diff coverage initialized: diffPath=${diffPath}, totalLines=${ctx.toolState.diffCoverage.totalLines}, tocEntries=${ctx.toolState.diffCoverage.tocEntries.length}`
|
|
||||||
);
|
|
||||||
|
|
||||||
// cache commentable-lines snapshot so review-time validation matches what
|
|
||||||
// GitHub will anchor to (commit_id=checkoutSha), even if the PR is updated
|
|
||||||
// between checkout and review.
|
|
||||||
const cached = new Map<string, ReturnType<typeof commentableLinesForFile>>();
|
|
||||||
for (const file of formatResult.files) {
|
|
||||||
cached.set(file.filename, commentableLinesForFile(file.patch));
|
|
||||||
}
|
|
||||||
ctx.toolState.commentableLinesByFile = cached;
|
|
||||||
ctx.toolState.commentableLinesPullNumber = pull_number;
|
|
||||||
ctx.toolState.commentableLinesCheckoutSha = ctx.toolState.checkoutSha;
|
|
||||||
|
|
||||||
const incrementalInstructions = incrementalDiffPath
|
|
||||||
? ` IMPORTANT: incrementalDiffPath contains ONLY the changes since the last reviewed version ` +
|
|
||||||
`(computed via range-diff). you MUST read incrementalDiffPath FIRST to understand what changed, ` +
|
|
||||||
`then use diffPath for full PR context. do NOT skip the incremental diff.`
|
|
||||||
: "";
|
|
||||||
|
|
||||||
// commit metadata relative to the PR base (e.g. main). use origin/<base>
|
|
||||||
// because the local base ref may not exist after a shallow fetch. cap
|
|
||||||
// the log so a PR with thousands of commits doesn't blow up the tool
|
|
||||||
// response. if the base ref can't be resolved (e.g. shallow fetch that
|
|
||||||
// didn't pull down origin/<base>), degrade gracefully rather than
|
|
||||||
// failing the whole checkout_pr call over metadata.
|
|
||||||
const COMMIT_LOG_MAX = 200;
|
|
||||||
const baseRange = `origin/${pr.baseRef}..HEAD`;
|
|
||||||
let commitCount = 0;
|
|
||||||
let commitLog = "";
|
|
||||||
let commitLogUnavailable = false;
|
|
||||||
try {
|
try {
|
||||||
commitCount = parseInt(
|
return await promise;
|
||||||
$("git", ["rev-list", "--count", baseRange], { log: false }).trim() || "0",
|
} finally {
|
||||||
10
|
inFlightCheckouts.delete(pull_number);
|
||||||
);
|
|
||||||
commitLog = $("git", ["log", "--oneline", `--max-count=${COMMIT_LOG_MAX}`, baseRange], {
|
|
||||||
log: false,
|
|
||||||
});
|
|
||||||
} catch (err) {
|
|
||||||
commitLogUnavailable = true;
|
|
||||||
log.debug(
|
|
||||||
`» unable to compute commit metadata for ${baseRange}: ${err instanceof Error ? err.message : String(err)}`
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
const commitLogTruncated = commitCount > COMMIT_LOG_MAX;
|
|
||||||
|
|
||||||
const hookWarningInstructions = checkoutResult.hookWarning
|
|
||||||
? ` HOOK WARNING: the post-checkout lifecycle hook reported a non-fatal failure (see hookWarning). ` +
|
|
||||||
`decide whether to retry based on the guidance in that field before proceeding.`
|
|
||||||
: "";
|
|
||||||
|
|
||||||
const commitLogInstructions = commitLogUnavailable
|
|
||||||
? ` NOTE: commit metadata is partial (base ref unreachable, likely a shallow fetch). ` +
|
|
||||||
`commitCount/commitLog may be 0/empty or incomplete; treat them as "unknown" rather than "no commits", ` +
|
|
||||||
`and use \`git log\` directly if you need the full history.`
|
|
||||||
: commitLogTruncated
|
|
||||||
? ` NOTE: commitLog was capped at ${COMMIT_LOG_MAX} entries out of ${commitCount} commits; ` +
|
|
||||||
`use \`git log\` directly if you need the full history.`
|
|
||||||
: "";
|
|
||||||
|
|
||||||
return {
|
|
||||||
success: true,
|
|
||||||
number: prResponse.data.number,
|
|
||||||
title: prResponse.data.title,
|
|
||||||
body: prResponse.data.body,
|
|
||||||
base: pr.baseRef,
|
|
||||||
localBranch: `pr-${pull_number}`,
|
|
||||||
remoteBranch: `refs/heads/${pr.headRef}`,
|
|
||||||
isFork: pr.headRepoFullName !== pr.baseRepoFullName,
|
|
||||||
maintainerCanModify: pr.maintainerCanModify,
|
|
||||||
url: prResponse.data.html_url,
|
|
||||||
headRepo: pr.headRepoFullName,
|
|
||||||
diffPath,
|
|
||||||
incrementalDiffPath,
|
|
||||||
toc: formatResult.toc,
|
|
||||||
commitCount,
|
|
||||||
commitLog,
|
|
||||||
commitLogTruncated,
|
|
||||||
commitLogUnavailable,
|
|
||||||
hookWarning: checkoutResult.hookWarning,
|
|
||||||
instructions:
|
|
||||||
`the diff file at diffPath contains a table of contents (TOC) at the top listing every changed file with its line range. ` +
|
|
||||||
`use the TOC line ranges as your checklist and read specific files from the diff instead of reading the entire file. ` +
|
|
||||||
`for example, if the TOC says "src/foo.ts → lines 5-42", read lines 5-42 from diffPath to see that file's changes. ` +
|
|
||||||
`review files selectively based on relevance rather than reading everything sequentially. ` +
|
|
||||||
`to inspect the PR's changed files, use diffPath — do NOT run \`git diff <base>..<head>\` to re-derive what's already in diffPath. the formatted diff with line numbers is authoritative. ` +
|
|
||||||
`\`git log\` and \`git diff --stat\` are fine for commit-range overview, and \`git diff\` / \`git diff --cached\` are fine for inspecting *your own* uncommitted changes — but PR review content MUST come from diffPath. ` +
|
|
||||||
`before your review is submitted, a one-time coverage pre-flight may error listing unread TOC regions. ` +
|
|
||||||
`retry the same create_pull_request_review call to proceed — optionally after reading the listed ranges. the pre-flight will not block again this session. ` +
|
|
||||||
`the local branch is 'localBranch' (pr-{number}), not the remote branch name. ` +
|
|
||||||
`when pushing, omit branchName to use the current branch. do not use remoteBranch as a local branch name.` +
|
|
||||||
incrementalInstructions +
|
|
||||||
hookWarningInstructions +
|
|
||||||
commitLogInstructions,
|
|
||||||
} satisfies CheckoutPrResult;
|
|
||||||
}),
|
}),
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|||||||
+2
-32
@@ -2,7 +2,7 @@ import { regex } from "arkregex";
|
|||||||
import { type } from "arktype";
|
import { type } from "arktype";
|
||||||
import type { StoredPushDest } from "../toolState.ts";
|
import type { StoredPushDest } from "../toolState.ts";
|
||||||
import { log } from "../utils/cli.ts";
|
import { log } from "../utils/cli.ts";
|
||||||
import { $git } from "../utils/gitAuth.ts";
|
import { $git, $gitFetchWithDeepen } from "../utils/gitAuth.ts";
|
||||||
import { executeLifecycleHook } from "../utils/lifecycle.ts";
|
import { executeLifecycleHook } from "../utils/lifecycle.ts";
|
||||||
import { $ } from "../utils/shell.ts";
|
import { $ } from "../utils/shell.ts";
|
||||||
import type { ToolContext } from "./server.ts";
|
import type { ToolContext } from "./server.ts";
|
||||||
@@ -510,21 +510,6 @@ const GitFetch = type({
|
|||||||
depth: type.number.describe("Fetch depth (for shallow clones)").optional(),
|
depth: type.number.describe("Fetch depth (for shallow clones)").optional(),
|
||||||
});
|
});
|
||||||
|
|
||||||
// when an agent-supplied depth is too shallow to reach the merge base, git
|
|
||||||
// surfaces "Could not read <sha>" and "remote did not send all necessary
|
|
||||||
// objects". detect both wordings so a single deepen retry can recover before
|
|
||||||
// the error reaches the agent (issue #564). git emits the full OID via
|
|
||||||
// oid_to_hex, so the bound is 40 (SHA-1) or 64 (SHA-256).
|
|
||||||
const SHALLOW_UNREACHABLE_PATTERNS: RegExp[] = [
|
|
||||||
/Could not read [a-f0-9]{40,64}/,
|
|
||||||
/remote did not send all necessary objects/,
|
|
||||||
];
|
|
||||||
|
|
||||||
// large enough to clear the merge base on most real-world PRs without
|
|
||||||
// downloading the full history; matches the fallback used by checkoutPrBranch
|
|
||||||
// when the compare API is unavailable.
|
|
||||||
const DEEPEN_RETRY_DEPTH = 1000;
|
|
||||||
|
|
||||||
export function GitFetchTool(ctx: ToolContext) {
|
export function GitFetchTool(ctx: ToolContext) {
|
||||||
return tool({
|
return tool({
|
||||||
name: "git_fetch",
|
name: "git_fetch",
|
||||||
@@ -538,22 +523,7 @@ export function GitFetchTool(ctx: ToolContext) {
|
|||||||
if (params.depth !== undefined) {
|
if (params.depth !== undefined) {
|
||||||
fetchArgs.push(`--depth=${params.depth}`);
|
fetchArgs.push(`--depth=${params.depth}`);
|
||||||
}
|
}
|
||||||
try {
|
await $gitFetchWithDeepen(fetchArgs, { token: ctx.gitToken }, "git_fetch");
|
||||||
await $git("fetch", fetchArgs, { token: ctx.gitToken });
|
|
||||||
} catch (err) {
|
|
||||||
const msg = err instanceof Error ? err.message : String(err);
|
|
||||||
const isShallowUnreachable = SHALLOW_UNREACHABLE_PATTERNS.some((p) => p.test(msg));
|
|
||||||
const isShallow =
|
|
||||||
isShallowUnreachable &&
|
|
||||||
$("git", ["rev-parse", "--is-shallow-repository"], { log: false }).trim() === "true";
|
|
||||||
if (!isShallow) throw err;
|
|
||||||
log.info(
|
|
||||||
`» git_fetch hit shallow-unreachable error, retrying with --deepen=${DEEPEN_RETRY_DEPTH}`
|
|
||||||
);
|
|
||||||
await $git("fetch", [`--deepen=${DEEPEN_RETRY_DEPTH}`, "--no-tags", "origin", params.ref], {
|
|
||||||
token: ctx.gitToken,
|
|
||||||
});
|
|
||||||
}
|
|
||||||
return { success: true, ref: params.ref };
|
return { success: true, ref: params.ref };
|
||||||
}),
|
}),
|
||||||
});
|
});
|
||||||
|
|||||||
+22
-2
@@ -176,6 +176,23 @@ function getTempDir(): string {
|
|||||||
return tempDir;
|
return tempDir;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** chars of shell output kept inline in the agent reply. anything past this
|
||||||
|
* blows the agent's context budget on commands that dump big logs (test
|
||||||
|
* runners, build tools, grep on large trees), so the overflow is spilled
|
||||||
|
* to a tempfile the agent can re-read selectively (cat/tail/grep). */
|
||||||
|
export const MAX_OUTPUT_CHARS = 5000;
|
||||||
|
|
||||||
|
/** if `output` exceeds `MAX_OUTPUT_CHARS`, persist the full body to a
|
||||||
|
* tempfile and return the last `MAX_OUTPUT_CHARS` prefixed with a sentinel
|
||||||
|
* pointing at the saved path. otherwise return as-is. */
|
||||||
|
function capOutput(output: string): string {
|
||||||
|
if (output.length <= MAX_OUTPUT_CHARS) return output;
|
||||||
|
const fullPath = join(getTempDir(), `shell-${randomUUID().slice(0, 8)}.log`);
|
||||||
|
writeFileSync(fullPath, output);
|
||||||
|
const elided = output.length - MAX_OUTPUT_CHARS;
|
||||||
|
return `... [${elided} chars truncated; full output saved to ${fullPath}] ...\n${output.slice(-MAX_OUTPUT_CHARS)}`;
|
||||||
|
}
|
||||||
|
|
||||||
/** detect git as a command invocation (not as part of another word like .gitignore) */
|
/** detect git as a command invocation (not as part of another word like .gitignore) */
|
||||||
function isGitCommand(command: string): boolean {
|
function isGitCommand(command: string): boolean {
|
||||||
const trimmed = command.trim();
|
const trimmed = command.trim();
|
||||||
@@ -196,6 +213,8 @@ Use this tool to:
|
|||||||
- Execute build tools (npm, pnpm, cargo, make, etc.)
|
- Execute build tools (npm, pnpm, cargo, make, etc.)
|
||||||
- Run tests and linters
|
- Run tests and linters
|
||||||
|
|
||||||
|
Output is capped at ${MAX_OUTPUT_CHARS} chars: if exceeded, only the tail is returned and the full body is saved to a tempfile (path included in the response). Re-read the tempfile with cat/tail/grep when you need more.
|
||||||
|
|
||||||
Do NOT use this tool for git commands — use the dedicated git tools instead.`,
|
Do NOT use this tool for git commands — use the dedicated git tools instead.`,
|
||||||
parameters: ShellParams,
|
parameters: ShellParams,
|
||||||
execute: execute(async (params) => {
|
execute: execute(async (params) => {
|
||||||
@@ -301,13 +320,14 @@ Do NOT use this tool for git commands — use the dedicated git tools instead.`,
|
|||||||
: `[timed out after ${timeout}ms]`;
|
: `[timed out after ${timeout}ms]`;
|
||||||
|
|
||||||
const finalExitCode = exitCode ?? (timedOut ? 124 : -1);
|
const finalExitCode = exitCode ?? (timedOut ? 124 : -1);
|
||||||
|
const trimmed = output.trim();
|
||||||
if (finalExitCode !== 0) {
|
if (finalExitCode !== 0) {
|
||||||
log.info(`shell command failed with exit code ${finalExitCode}: ${params.command}`);
|
log.info(`shell command failed with exit code ${finalExitCode}: ${params.command}`);
|
||||||
if (output) log.info(`output: ${output.trim()}`);
|
if (trimmed) log.info(`output: ${trimmed}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
output: output.trim(),
|
output: capOutput(trimmed),
|
||||||
exit_code: finalExitCode,
|
exit_code: finalExitCode,
|
||||||
timed_out: timedOut,
|
timed_out: timedOut,
|
||||||
};
|
};
|
||||||
|
|||||||
+1
-1
@@ -1,6 +1,6 @@
|
|||||||
{
|
{
|
||||||
"name": "pullfrog",
|
"name": "pullfrog",
|
||||||
"version": "0.1.7",
|
"version": "0.1.8",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"bin": {
|
"bin": {
|
||||||
"pullfrog": "dist/cli.mjs",
|
"pullfrog": "dist/cli.mjs",
|
||||||
|
|||||||
@@ -96,4 +96,10 @@ export const test: TestRunnerOptions = {
|
|||||||
repoSetup,
|
repoSetup,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic", "security"],
|
tags: ["agnostic", "security"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/gitAuth.ts",
|
||||||
|
"action/utils/gitAuthServer.ts",
|
||||||
|
"action/mcp/git.ts",
|
||||||
|
"action/mcp/checkout.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -104,4 +104,10 @@ export const test: TestRunnerOptions = {
|
|||||||
agentEnv,
|
agentEnv,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic"],
|
tags: ["agnostic"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/gitAuth.ts",
|
||||||
|
"action/utils/gitAuthServer.ts",
|
||||||
|
"action/mcp/git.ts",
|
||||||
|
"action/mcp/checkout.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -92,4 +92,5 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic", "security"],
|
tags: ["agnostic", "security"],
|
||||||
|
coverage: ["action/mcp/dependencies.ts", "action/utils/install.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -62,4 +62,10 @@ export const test: TestRunnerOptions = {
|
|||||||
agentEnv,
|
agentEnv,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic"],
|
tags: ["agnostic"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/gitAuth.ts",
|
||||||
|
"action/utils/gitAuthServer.ts",
|
||||||
|
"action/mcp/git.ts",
|
||||||
|
"action/mcp/checkout.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -74,4 +74,10 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic"],
|
tags: ["agnostic"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/gitAuth.ts",
|
||||||
|
"action/utils/gitAuthServer.ts",
|
||||||
|
"action/mcp/git.ts",
|
||||||
|
"action/mcp/checkout.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -67,4 +67,10 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic"],
|
tags: ["agnostic"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/gitAuth.ts",
|
||||||
|
"action/utils/gitAuthServer.ts",
|
||||||
|
"action/mcp/git.ts",
|
||||||
|
"action/mcp/checkout.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -29,4 +29,11 @@ export const test: TestRunnerOptions = {
|
|||||||
expectFailure: true,
|
expectFailure: true,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
tags: ["agnostic"],
|
tags: ["agnostic"],
|
||||||
|
coverage: [
|
||||||
|
"action/utils/timer.ts",
|
||||||
|
"action/utils/subprocess.ts",
|
||||||
|
"action/utils/exitHandler.ts",
|
||||||
|
"action/utils/activity.ts",
|
||||||
|
"action/mcp/selectMode.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -1,70 +0,0 @@
|
|||||||
#!/usr/bin/env bash
|
|
||||||
# determines which agents need testing based on changed files.
|
|
||||||
# reads changed file paths from stdin (JSON array or newline-delimited).
|
|
||||||
# outputs a JSON array of agent names to stdout.
|
|
||||||
#
|
|
||||||
# only agents whose harness file changed AND are exported from index.ts are included.
|
|
||||||
# shared.ts/index.ts/postRun.ts and other non-harness action changes fall back to opencode as a canary.
|
|
||||||
set -euo pipefail
|
|
||||||
|
|
||||||
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
|
||||||
AGENTS_INDEX="$SCRIPT_DIR/../agents/index.ts"
|
|
||||||
|
|
||||||
# build the set of active agents from index.ts imports (portable, no -P)
|
|
||||||
active_agents=()
|
|
||||||
while IFS= read -r line; do
|
|
||||||
[[ -n "$line" ]] && active_agents+=("$line")
|
|
||||||
done < <(sed -n 's/.*from "\.\/\([^"]*\)\.ts".*/\1/p' "$AGENTS_INDEX" | grep -v shared)
|
|
||||||
|
|
||||||
# read stdin - auto-detect JSON array vs newline-delimited
|
|
||||||
input=$(cat)
|
|
||||||
if echo "$input" | jq -e 'type == "array"' > /dev/null 2>&1; then
|
|
||||||
files=$(echo "$input" | jq -r '.[]')
|
|
||||||
else
|
|
||||||
files="$input"
|
|
||||||
fi
|
|
||||||
|
|
||||||
is_active_agent() {
|
|
||||||
local name="$1"
|
|
||||||
for a in "${active_agents[@]}"; do
|
|
||||||
[[ "$a" == "$name" ]] && return 0
|
|
||||||
done
|
|
||||||
return 1
|
|
||||||
}
|
|
||||||
|
|
||||||
# find which agent harness files changed
|
|
||||||
changed_agents=()
|
|
||||||
has_non_agent_change=false
|
|
||||||
|
|
||||||
while IFS= read -r file; do
|
|
||||||
[[ -z "$file" ]] && continue
|
|
||||||
case "$file" in
|
|
||||||
action/agents/shared.ts|action/agents/index.ts|action/agents/postRun.ts)
|
|
||||||
has_non_agent_change=true
|
|
||||||
;;
|
|
||||||
action/agents/*.ts)
|
|
||||||
agent_name="$(basename "$file" .ts)"
|
|
||||||
if is_active_agent "$agent_name"; then
|
|
||||||
changed_agents+=("$agent_name")
|
|
||||||
else
|
|
||||||
# legacy/inactive agent file changed — treat as non-agent change
|
|
||||||
has_non_agent_change=true
|
|
||||||
fi
|
|
||||||
;;
|
|
||||||
action/*)
|
|
||||||
has_non_agent_change=true
|
|
||||||
;;
|
|
||||||
esac
|
|
||||||
done <<< "$files"
|
|
||||||
|
|
||||||
# output agents based on change type.
|
|
||||||
# non-agent action changes always include opencode as a canary.
|
|
||||||
if $has_non_agent_change; then
|
|
||||||
changed_agents+=("opencode")
|
|
||||||
fi
|
|
||||||
|
|
||||||
if [[ ${#changed_agents[@]} -gt 0 ]]; then
|
|
||||||
printf '%s\n' "${changed_agents[@]}" | sort -u | jq -R . | jq -sc .
|
|
||||||
else
|
|
||||||
echo '[]'
|
|
||||||
fi
|
|
||||||
+20
-51
@@ -1,4 +1,3 @@
|
|||||||
import { execFileSync } from "node:child_process";
|
|
||||||
import { readdirSync, readFileSync } from "node:fs";
|
import { readdirSync, readFileSync } from "node:fs";
|
||||||
import { dirname, join } from "node:path";
|
import { dirname, join } from "node:path";
|
||||||
import { fileURLToPath } from "node:url";
|
import { fileURLToPath } from "node:url";
|
||||||
@@ -16,7 +15,7 @@ type WorkflowJob = {
|
|||||||
"runs-on": string;
|
"runs-on": string;
|
||||||
"timeout-minutes"?: number;
|
"timeout-minutes"?: number;
|
||||||
permissions?: WorkflowPermissions;
|
permissions?: WorkflowPermissions;
|
||||||
strategy?: { "fail-fast": boolean; matrix: Record<string, string[]> };
|
strategy?: { "fail-fast": boolean; matrix: Record<string, unknown> };
|
||||||
env?: Record<string, string>;
|
env?: Record<string, string>;
|
||||||
steps?: unknown[];
|
steps?: unknown[];
|
||||||
};
|
};
|
||||||
@@ -57,7 +56,6 @@ const expectedAgents = Object.keys(agents).sort();
|
|||||||
const crossagentTests = getTestNamesFromDir("crossagent");
|
const crossagentTests = getTestNamesFromDir("crossagent");
|
||||||
const agnosticTests = getTestNamesFromDir("agnostic");
|
const agnosticTests = getTestNamesFromDir("agnostic");
|
||||||
const adhocTests = getTestNamesFromDir("adhoc");
|
const adhocTests = getTestNamesFromDir("adhoc");
|
||||||
const dynamicAgentsExpression = "$" + "{{ fromJSON(needs.changes.outputs.agents) }}";
|
|
||||||
|
|
||||||
// all provider API key names + GITHUB_TOKEN + model overrides
|
// all provider API key names + GITHUB_TOKEN + model overrides
|
||||||
const expectedAgentEnvVars = [
|
const expectedAgentEnvVars = [
|
||||||
@@ -83,53 +81,22 @@ describe("ci workflow consistency", () => {
|
|||||||
const rootJob = rootWorkflow.jobs["action-agents"];
|
const rootJob = rootWorkflow.jobs["action-agents"];
|
||||||
const actionJob = actionWorkflow.jobs.agents;
|
const actionJob = actionWorkflow.jobs.agents;
|
||||||
|
|
||||||
it("root agent matrix uses dynamic output from changes job", () => {
|
it("root agents matrix is wired to the dynamic matrix output", () => {
|
||||||
expect(rootJob.strategy!.matrix.agent).toBe(dynamicAgentsExpression);
|
const include = rootJob.strategy?.matrix.include;
|
||||||
});
|
expect(typeof include).toBe("string");
|
||||||
|
expect(include as string).toContain("fromJSON(needs.changes.outputs.matrix).agents");
|
||||||
it("changed-agents.sh falls back to opencode when shared agent code changed", () => {
|
|
||||||
const input = JSON.stringify(["action/agents/shared.ts"]);
|
|
||||||
const output = execFileSync("bash", [join(__dirname, "changed-agents.sh")], {
|
|
||||||
input,
|
|
||||||
encoding: "utf-8",
|
|
||||||
});
|
|
||||||
expect(JSON.parse(output)).toEqual(["opencode"]);
|
|
||||||
});
|
|
||||||
|
|
||||||
it("changed-agents.sh falls back to opencode for non-agent action changes", () => {
|
|
||||||
const output = execFileSync("bash", [join(__dirname, "changed-agents.sh")], {
|
|
||||||
input: JSON.stringify(["action/mcp/server.ts"]),
|
|
||||||
encoding: "utf-8",
|
|
||||||
});
|
|
||||||
expect(JSON.parse(output)).toEqual(["opencode"]);
|
|
||||||
});
|
|
||||||
|
|
||||||
it("changed-agents.sh includes opencode canary alongside changed agents", () => {
|
|
||||||
const output = execFileSync("bash", [join(__dirname, "changed-agents.sh")], {
|
|
||||||
input: JSON.stringify(["action/agents/opencode.ts", "action/mcp/server.ts"]),
|
|
||||||
encoding: "utf-8",
|
|
||||||
});
|
|
||||||
expect(JSON.parse(output)).toEqual(["opencode"]);
|
|
||||||
});
|
|
||||||
|
|
||||||
it("changed-agents.sh treats legacy agent files as non-agent changes", () => {
|
|
||||||
const output = execFileSync("bash", [join(__dirname, "changed-agents.sh")], {
|
|
||||||
input: JSON.stringify(["action/agents/codex.ts", "action/agents/gemini.ts"]),
|
|
||||||
encoding: "utf-8",
|
|
||||||
});
|
|
||||||
expect(JSON.parse(output)).toEqual(["opencode"]);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("action agent matrix matches agents map", () => {
|
it("action agent matrix matches agents map", () => {
|
||||||
expect([...actionJob.strategy!.matrix.agent].sort()).toEqual(expectedAgents);
|
expect((actionJob.strategy?.matrix.agent as string[])?.slice().sort()).toEqual(
|
||||||
});
|
expectedAgents
|
||||||
|
);
|
||||||
it("root test matrix matches crossagent/ directory", () => {
|
|
||||||
expect([...rootJob.strategy!.matrix.test].sort()).toEqual(crossagentTests);
|
|
||||||
});
|
});
|
||||||
|
|
||||||
it("action test matrix matches crossagent/ directory", () => {
|
it("action test matrix matches crossagent/ directory", () => {
|
||||||
expect([...actionJob.strategy!.matrix.test].sort()).toEqual(crossagentTests);
|
expect((actionJob.strategy?.matrix.test as string[])?.slice().sort()).toEqual(
|
||||||
|
crossagentTests
|
||||||
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("permissions match between root and action", () => {
|
it("permissions match between root and action", () => {
|
||||||
@@ -149,8 +116,8 @@ describe("ci workflow consistency", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("fail-fast is enabled in both", () => {
|
it("fail-fast is enabled in both", () => {
|
||||||
expect(rootJob.strategy!["fail-fast"]).toBe(true);
|
expect(rootJob.strategy?.["fail-fast"]).toBe(true);
|
||||||
expect(actionJob.strategy!["fail-fast"]).toBe(true);
|
expect(actionJob.strategy?.["fail-fast"]).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -158,12 +125,14 @@ describe("ci workflow consistency", () => {
|
|||||||
const rootJob = rootWorkflow.jobs["action-agnostic"];
|
const rootJob = rootWorkflow.jobs["action-agnostic"];
|
||||||
const actionJob = actionWorkflow.jobs.agnostic;
|
const actionJob = actionWorkflow.jobs.agnostic;
|
||||||
|
|
||||||
it("root test matrix matches agnostic/ directory", () => {
|
it("root agnostic matrix is wired to the dynamic matrix output", () => {
|
||||||
expect([...rootJob.strategy!.matrix.test].sort()).toEqual(agnosticTests);
|
const include = rootJob.strategy?.matrix.include;
|
||||||
|
expect(typeof include).toBe("string");
|
||||||
|
expect(include as string).toContain("fromJSON(needs.changes.outputs.matrix).agnostic");
|
||||||
});
|
});
|
||||||
|
|
||||||
it("action test matrix matches agnostic/ directory", () => {
|
it("action test matrix matches agnostic/ directory", () => {
|
||||||
expect([...actionJob.strategy!.matrix.test].sort()).toEqual(agnosticTests);
|
expect((actionJob.strategy?.matrix.test as string[])?.slice().sort()).toEqual(agnosticTests);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("permissions match between root and action", () => {
|
it("permissions match between root and action", () => {
|
||||||
@@ -183,8 +152,8 @@ describe("ci workflow consistency", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("fail-fast is enabled in both", () => {
|
it("fail-fast is enabled in both", () => {
|
||||||
expect(rootJob.strategy!["fail-fast"]).toBe(true);
|
expect(rootJob.strategy?.["fail-fast"]).toBe(true);
|
||||||
expect(actionJob.strategy!["fail-fast"]).toBe(true);
|
expect(actionJob.strategy?.["fail-fast"]).toBe(true);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -0,0 +1,112 @@
|
|||||||
|
/**
|
||||||
|
* shared coverage / glob plumbing for the matrix builder.
|
||||||
|
*
|
||||||
|
* every test (`crossagent/`, `agnostic/`) and every provider entry
|
||||||
|
* (`providers.ts`) declares a `coverage` array of repo-relative globs. on a PR
|
||||||
|
* push, the `changes` job feeds the changed-file list into `matrix.ts`, which
|
||||||
|
* intersects each entry's globs against the diff and emits only the entries
|
||||||
|
* that need to run.
|
||||||
|
*
|
||||||
|
* `ALWAYS_RUN_ALL` is the escape hatch: any change to a file matched here
|
||||||
|
* forces the full matrix (every test, every flagship, every alias). it
|
||||||
|
* captures cross-cutting infrastructure where fan-out is unpredictable —
|
||||||
|
* agent loader, MCP server boot, test runner itself. if a per-test glob
|
||||||
|
* goes stale, this list and the on-`main`-full-matrix policy are the safety
|
||||||
|
* nets — there's no completeness lint.
|
||||||
|
*
|
||||||
|
* `coverage` is optional on tests/providers; missing = always run (treat as
|
||||||
|
* "any code change touches me"). default to defensive — opt into precision
|
||||||
|
* by adding globs.
|
||||||
|
*/
|
||||||
|
|
||||||
|
/** patterns that, when matched by any changed file, force the full matrix. */
|
||||||
|
export const ALWAYS_RUN_ALL: string[] = [
|
||||||
|
// agent loader + cross-agent shared code
|
||||||
|
"action/agents/shared.ts",
|
||||||
|
"action/agents/index.ts",
|
||||||
|
"action/agents/postRun.ts",
|
||||||
|
// test harness — changing these can affect every test
|
||||||
|
"action/test/run.ts",
|
||||||
|
"action/test/utils.ts",
|
||||||
|
"action/test/matrix.ts",
|
||||||
|
"action/test/coverage.ts",
|
||||||
|
"action/test/providers.ts",
|
||||||
|
// boot + lifecycle
|
||||||
|
"action/main.ts",
|
||||||
|
"action/index.ts",
|
||||||
|
"action/cli.ts",
|
||||||
|
"action/utils/setup.ts",
|
||||||
|
"action/utils/lifecycle.ts",
|
||||||
|
"action/utils/install.ts",
|
||||||
|
"action/utils/docker.ts",
|
||||||
|
"action/utils/globals.ts",
|
||||||
|
// MCP orchestrator (every test runs through it)
|
||||||
|
"action/mcp/server.ts",
|
||||||
|
"action/mcp/shared.ts",
|
||||||
|
// dependency graph
|
||||||
|
"action/package.json",
|
||||||
|
"action/pnpm-lock.yaml",
|
||||||
|
// workflow itself
|
||||||
|
".github/workflows/test.yml",
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* expand a single brace group like `{a,b,c}` into an array of patterns.
|
||||||
|
*
|
||||||
|
* intentionally minimal: nested braces (`{a,{b,c}}`) and escaped braces are
|
||||||
|
* NOT supported — coverage globs in this repo only need flat brace groups
|
||||||
|
* (`{claude,opencode}.ts`). add complexity if a real use case emerges.
|
||||||
|
*/
|
||||||
|
function expandBraces(pattern: string): string[] {
|
||||||
|
const m = pattern.match(/\{([^{}]+)\}/);
|
||||||
|
if (!m || m.index === undefined) return [pattern];
|
||||||
|
const before = pattern.slice(0, m.index);
|
||||||
|
const after = pattern.slice(m.index + m[0].length);
|
||||||
|
const opts = m[1].split(",");
|
||||||
|
return opts.flatMap((opt) => expandBraces(`${before}${opt}${after}`));
|
||||||
|
}
|
||||||
|
|
||||||
|
/** convert a glob pattern to a regex anchored at start + end. */
|
||||||
|
function globToRegex(pattern: string): RegExp {
|
||||||
|
const DSTAR = "\u0000DSTAR\u0000";
|
||||||
|
let s = pattern.replace(/\*\*/g, DSTAR);
|
||||||
|
s = s.replace(/[.+^$()|[\]\\]/g, "\\$&");
|
||||||
|
s = s.replace(/\*/g, "[^/]*");
|
||||||
|
s = s.replace(/\?/g, "[^/]");
|
||||||
|
s = s.replaceAll(DSTAR, ".*");
|
||||||
|
return new RegExp(`^${s}$`);
|
||||||
|
}
|
||||||
|
|
||||||
|
/** does any path in `paths` match any glob in `patterns`? */
|
||||||
|
export function anyMatch(paths: string[], patterns: string[]): boolean {
|
||||||
|
if (patterns.length === 0) return false;
|
||||||
|
const regexes = patterns.flatMap((p) => expandBraces(p)).map(globToRegex);
|
||||||
|
return paths.some((path) => regexes.some((r) => r.test(path)));
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* decide whether an entry runs given changed files + its coverage globs.
|
||||||
|
*
|
||||||
|
* three short-circuits:
|
||||||
|
* 1. `full` flag (e.g. main pushes, workflow_dispatch) → always run
|
||||||
|
* 2. any changed file matches `ALWAYS_RUN_ALL` → run everything
|
||||||
|
* 3. coverage missing or empty on the entry → run (defensive default)
|
||||||
|
*
|
||||||
|
* otherwise: run iff any changed file matches the entry's coverage globs.
|
||||||
|
*
|
||||||
|
* `coverage: []` is treated identically to `coverage: undefined` to avoid the
|
||||||
|
* footgun where a future test author intends "skip on PRs" by passing an
|
||||||
|
* empty array — silently skipping CI on every PR is worse than always running.
|
||||||
|
*/
|
||||||
|
export type ShouldRunInput = {
|
||||||
|
changedFiles: string[];
|
||||||
|
coverage: string[] | undefined;
|
||||||
|
full: boolean;
|
||||||
|
};
|
||||||
|
|
||||||
|
export function shouldRun(input: ShouldRunInput): boolean {
|
||||||
|
if (input.full) return true;
|
||||||
|
if (anyMatch(input.changedFiles, ALWAYS_RUN_ALL)) return true;
|
||||||
|
if (input.coverage === undefined || input.coverage.length === 0) return true;
|
||||||
|
return anyMatch(input.changedFiles, input.coverage);
|
||||||
|
}
|
||||||
@@ -43,4 +43,6 @@ export const test: TestRunnerOptions = {
|
|||||||
},
|
},
|
||||||
repoSetup:
|
repoSetup:
|
||||||
'mkdir -p /tmp/pullfrog-mcp-secret && printf "%s" "$PULLFROG_MCP_SECRET" > /tmp/pullfrog-mcp-secret/secret.txt',
|
'mkdir -p /tmp/pullfrog-mcp-secret && printf "%s" "$PULLFROG_MCP_SECRET" > /tmp/pullfrog-mcp-secret/secret.txt',
|
||||||
|
// any MCP-layer change can affect repo-MCP merging; agents own MCP wiring.
|
||||||
|
coverage: ["action/mcp/**", "action/agents/{claude,opencode}.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -43,4 +43,5 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
agentEnv,
|
agentEnv,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
|
coverage: ["action/mcp/shell.ts", "action/agents/{claude,opencode}.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -52,4 +52,9 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
agentEnv,
|
agentEnv,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
|
coverage: [
|
||||||
|
"action/utils/normalizeEnv.ts",
|
||||||
|
"action/mcp/shell.ts",
|
||||||
|
"action/agents/{claude,opencode}.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -44,4 +44,5 @@ export const test: TestRunnerOptions = {
|
|||||||
PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1",
|
PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1",
|
||||||
PULLFROG_MODEL: "anthropic/claude-sonnet-4-6",
|
PULLFROG_MODEL: "anthropic/claude-sonnet-4-6",
|
||||||
},
|
},
|
||||||
|
coverage: ["action/agents/claude.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -44,4 +44,5 @@ export const test: TestRunnerOptions = {
|
|||||||
PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1",
|
PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1",
|
||||||
PULLFROG_MODEL: "anthropic/claude-sonnet-4-6",
|
PULLFROG_MODEL: "anthropic/claude-sonnet-4-6",
|
||||||
},
|
},
|
||||||
|
coverage: ["action/agents/opencode.ts", "action/agents/opencodePlugin.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -29,4 +29,7 @@ export const test: TestRunnerOptions = {
|
|||||||
fixture,
|
fixture,
|
||||||
validator,
|
validator,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
|
// canary: any agent harness change runs the smoke. shared MCP set_output
|
||||||
|
// surface is also captured.
|
||||||
|
coverage: ["action/agents/{claude,opencode}.ts", "action/mcp/output.ts"],
|
||||||
};
|
};
|
||||||
|
|||||||
@@ -58,4 +58,9 @@ export const test: TestRunnerOptions = {
|
|||||||
validator,
|
validator,
|
||||||
agentEnv,
|
agentEnv,
|
||||||
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
env: { PULLFROG_DISABLE_SECURITY_INSTRUCTIONS: "1" },
|
||||||
|
coverage: [
|
||||||
|
"action/utils/normalizeEnv.ts",
|
||||||
|
"action/mcp/shell.ts",
|
||||||
|
"action/agents/{claude,opencode}.ts",
|
||||||
|
],
|
||||||
};
|
};
|
||||||
|
|||||||
+48
-41
@@ -1,7 +1,7 @@
|
|||||||
/**
|
/**
|
||||||
* emits a JSON array of { slug, agent, name } entries for one of two CI matrix
|
* emits a JSON array of { slug, agent, name } entries for one of two CI matrix
|
||||||
* jobs. `agent` mirrors the harness the runtime would pick in production
|
* jobs. `agent` mirrors the harness the runtime would pick in production
|
||||||
* (anthropic/* → claude-code, everything else → opencode).
|
* (anthropic/* → claude, everything else → opencode).
|
||||||
*
|
*
|
||||||
* MODE=aliases (default) — every alias minus pruned passthroughs. consumed by
|
* MODE=aliases (default) — every alias minus pruned passthroughs. consumed by
|
||||||
* `models-live`, which runs the cheap top-level CLI smoke per alias
|
* `models-live`, which runs the cheap top-level CLI smoke per alias
|
||||||
@@ -10,7 +10,8 @@
|
|||||||
* MODE=flagships — one standard-tier model per provider. consumed by
|
* MODE=flagships — one standard-tier model per provider. consumed by
|
||||||
* `providers-live`, which runs the full harness smoke
|
* `providers-live`, which runs the full harness smoke
|
||||||
* (`pnpm runtest smoke <agent>`) to validate provider-class tool-calling
|
* (`pnpm runtest smoke <agent>`) to validate provider-class tool-calling
|
||||||
* (e.g. Gemini schema sanitizer, OpenAI tool-call format).
|
* (e.g. Gemini schema sanitizer, OpenAI tool-call format). flagship slugs
|
||||||
|
* live in `providers.ts` alongside their per-provider coverage globs.
|
||||||
*
|
*
|
||||||
* passthrough pruning (aliases mode): openrouter/* aliases and keyed opencode/*
|
* passthrough pruning (aliases mode): openrouter/* aliases and keyed opencode/*
|
||||||
* aliases are routing-layer wrappers around models we already smoke-test
|
* aliases are routing-layer wrappers around models we already smoke-test
|
||||||
@@ -24,29 +25,15 @@
|
|||||||
* MODE=flagships node action/test/list-aliases.ts
|
* MODE=flagships node action/test/list-aliases.ts
|
||||||
* MATRIX_FILTER=gemini node action/test/list-aliases.ts
|
* MATRIX_FILTER=gemini node action/test/list-aliases.ts
|
||||||
* INCLUDE_PASSTHROUGHS=1 node action/test/list-aliases.ts
|
* INCLUDE_PASSTHROUGHS=1 node action/test/list-aliases.ts
|
||||||
|
*
|
||||||
|
* NOTE: the per-PR-precision matrix lives in `matrix.ts`, which calls into
|
||||||
|
* this file. raw invocation here emits the unfiltered matrix.
|
||||||
*/
|
*/
|
||||||
import { modelAliases } from "../models.ts";
|
import { modelAliases } from "../models.ts";
|
||||||
|
import { providers } from "./providers.ts";
|
||||||
|
|
||||||
const ROUTING_CANARIES = new Set(["openrouter/claude-sonnet", "opencode/claude-sonnet"]);
|
const ROUTING_CANARIES = new Set(["openrouter/claude-sonnet", "opencode/claude-sonnet"]);
|
||||||
|
|
||||||
// hand-picked "standard good model" per provider — not the pro/opus tier (too
|
|
||||||
// expensive for per-push) and not the free/experimental tier (too flaky). these
|
|
||||||
// aliases anchor the harness smoke job that catches provider-class regressions
|
|
||||||
// like Gemini schema sanitization or OpenAI tool-call format drift. the
|
|
||||||
// assertion below catches slug-drift loudly, but adding a NEW provider without
|
|
||||||
// an entry here silently omits it from `providers-live` — see
|
|
||||||
// wiki/models-catalog.md "To add a provider".
|
|
||||||
const FLAGSHIPS = [
|
|
||||||
"anthropic/claude-sonnet",
|
|
||||||
"openai/gpt",
|
|
||||||
"google/gemini-pro",
|
|
||||||
"xai/grok",
|
|
||||||
"deepseek/deepseek-pro",
|
|
||||||
"moonshotai/kimi-k2",
|
|
||||||
"opencode/big-pickle",
|
|
||||||
"openrouter/claude-sonnet",
|
|
||||||
];
|
|
||||||
|
|
||||||
function isPrunablePassthrough(alias: (typeof modelAliases)[number]): boolean {
|
function isPrunablePassthrough(alias: (typeof modelAliases)[number]): boolean {
|
||||||
if (ROUTING_CANARIES.has(alias.slug)) return false;
|
if (ROUTING_CANARIES.has(alias.slug)) return false;
|
||||||
if (alias.provider === "openrouter") return true;
|
if (alias.provider === "openrouter") return true;
|
||||||
@@ -59,7 +46,13 @@ function isPrunablePassthrough(alias: (typeof modelAliases)[number]): boolean {
|
|||||||
return alias.provider === "opencode" && !alias.isFree;
|
return alias.provider === "opencode" && !alias.isFree;
|
||||||
}
|
}
|
||||||
|
|
||||||
function toMatrixEntry(alias: (typeof modelAliases)[number]) {
|
export type MatrixEntry = {
|
||||||
|
slug: string;
|
||||||
|
agent: string;
|
||||||
|
name: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
function toMatrixEntry(alias: (typeof modelAliases)[number]): MatrixEntry {
|
||||||
return {
|
return {
|
||||||
slug: alias.slug,
|
slug: alias.slug,
|
||||||
agent: alias.slug.startsWith("anthropic/") ? "claude" : "opencode",
|
agent: alias.slug.startsWith("anthropic/") ? "claude" : "opencode",
|
||||||
@@ -68,25 +61,14 @@ function toMatrixEntry(alias: (typeof modelAliases)[number]) {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
const mode = process.env.MODE === "flagships" ? "flagships" : "aliases";
|
|
||||||
const filter = process.env.MATRIX_FILTER?.trim().toLowerCase() ?? "";
|
|
||||||
const includePassthroughs = process.env.INCLUDE_PASSTHROUGHS === "1";
|
|
||||||
|
|
||||||
const aliasBySlug = new Map(modelAliases.map((a) => [a.slug, a]));
|
const aliasBySlug = new Map(modelAliases.map((a) => [a.slug, a]));
|
||||||
const matrix = (() => {
|
|
||||||
if (mode === "flagships") {
|
export function buildAliasMatrix(opts: {
|
||||||
return FLAGSHIPS.map((slug) => {
|
filter?: string;
|
||||||
const alias = aliasBySlug.get(slug);
|
includePassthroughs?: boolean;
|
||||||
if (!alias) {
|
}): MatrixEntry[] {
|
||||||
throw new Error(
|
const filter = opts.filter ?? "";
|
||||||
`list-aliases: flagship "${slug}" missing from modelAliases — update FLAGSHIPS`
|
const includePassthroughs = opts.includePassthroughs ?? false;
|
||||||
);
|
|
||||||
}
|
|
||||||
return alias;
|
|
||||||
})
|
|
||||||
.filter((alias) => !filter || alias.slug.toLowerCase().includes(filter))
|
|
||||||
.map(toMatrixEntry);
|
|
||||||
}
|
|
||||||
return modelAliases
|
return modelAliases
|
||||||
.filter((alias) => {
|
.filter((alias) => {
|
||||||
if (filter && !alias.slug.toLowerCase().includes(filter)) return false;
|
if (filter && !alias.slug.toLowerCase().includes(filter)) return false;
|
||||||
@@ -94,6 +76,31 @@ const matrix = (() => {
|
|||||||
return true;
|
return true;
|
||||||
})
|
})
|
||||||
.map(toMatrixEntry);
|
.map(toMatrixEntry);
|
||||||
})();
|
}
|
||||||
|
|
||||||
process.stdout.write(JSON.stringify(matrix));
|
export function buildFlagshipMatrix(opts: { filter?: string }): MatrixEntry[] {
|
||||||
|
const filter = opts.filter ?? "";
|
||||||
|
return providers
|
||||||
|
.map((p) => {
|
||||||
|
const alias = aliasBySlug.get(p.flagship);
|
||||||
|
if (!alias) {
|
||||||
|
throw new Error(
|
||||||
|
`list-aliases: flagship "${p.flagship}" missing from modelAliases — update providers.ts`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
return alias;
|
||||||
|
})
|
||||||
|
.filter((alias) => !filter || alias.slug.toLowerCase().includes(filter))
|
||||||
|
.map(toMatrixEntry);
|
||||||
|
}
|
||||||
|
|
||||||
|
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||||
|
const mode = process.env.MODE === "flagships" ? "flagships" : "aliases";
|
||||||
|
const filter = process.env.MATRIX_FILTER?.trim().toLowerCase() ?? "";
|
||||||
|
const includePassthroughs = process.env.INCLUDE_PASSTHROUGHS === "1";
|
||||||
|
const matrix =
|
||||||
|
mode === "flagships"
|
||||||
|
? buildFlagshipMatrix({ filter })
|
||||||
|
: buildAliasMatrix({ filter, includePassthroughs });
|
||||||
|
process.stdout.write(JSON.stringify(matrix));
|
||||||
|
}
|
||||||
|
|||||||
+232
@@ -0,0 +1,232 @@
|
|||||||
|
/**
|
||||||
|
* unified CI matrix builder. emits the four matrices consumed by
|
||||||
|
* `.github/workflows/test.yml`:
|
||||||
|
*
|
||||||
|
* - agents: crossagent tests × eligible agents (fan-out)
|
||||||
|
* - agnostic: agnostic infrastructure tests (run with opencode)
|
||||||
|
* - flagships: one harness smoke per provider (providers-live)
|
||||||
|
* - aliases: one CLI smoke per model alias (models-live)
|
||||||
|
*
|
||||||
|
* input: a JSON array of repo-relative changed paths on stdin (the
|
||||||
|
* `paths-filter` action's `*_files` output). PR pushes pass the diff;
|
||||||
|
* `main` pushes and `workflow_dispatch` set FULL=1 to skip filtering and
|
||||||
|
* emit every entry.
|
||||||
|
*
|
||||||
|
* each test/provider declares its own `coverage` globs colocated with the
|
||||||
|
* test (`crossagent/`, `agnostic/`) or provider (`providers.ts`). the matrix
|
||||||
|
* builder intersects coverage against the diff. a top-level `ALWAYS_RUN_ALL`
|
||||||
|
* (see `coverage.ts`) bypasses filtering when test-harness or cross-cutting
|
||||||
|
* agent code changes — keeps stale globs from silently skipping critical
|
||||||
|
* tests on test runner / shared.ts churn.
|
||||||
|
*
|
||||||
|
* usage:
|
||||||
|
* echo '["action/agents/opencode.ts"]' | node action/test/matrix.ts
|
||||||
|
* FULL=1 node action/test/matrix.ts < /dev/null
|
||||||
|
* MATRIX_FILTER=gemini FULL=1 node action/test/matrix.ts < /dev/null
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { existsSync, readdirSync, readFileSync } from "node:fs";
|
||||||
|
import { dirname, join } from "node:path";
|
||||||
|
import { fileURLToPath } from "node:url";
|
||||||
|
import { shouldRun } from "./coverage.ts";
|
||||||
|
import { buildAliasMatrix, buildFlagshipMatrix } from "./list-aliases.ts";
|
||||||
|
import { providers } from "./providers.ts";
|
||||||
|
|
||||||
|
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||||
|
|
||||||
|
type AgentEntry = { agent: string; test: string; name: string };
|
||||||
|
type AgnosticEntry = { test: string; name: string };
|
||||||
|
type SlugEntry = { slug: string; agent: string; name: string };
|
||||||
|
|
||||||
|
type MatrixOutput = {
|
||||||
|
agents: AgentEntry[];
|
||||||
|
agnostic: AgnosticEntry[];
|
||||||
|
flagships: SlugEntry[];
|
||||||
|
aliases: SlugEntry[];
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* extracted test metadata. parsed via regex from the test source — see
|
||||||
|
* `parseTestFile`. dynamic-import is intentionally avoided: the GHA `changes`
|
||||||
|
* job runs without `pnpm install`, and the real test modules transitively
|
||||||
|
* import `@actions/core` etc. parsing keeps `matrix.ts` zero-dep.
|
||||||
|
*/
|
||||||
|
type ParsedTest = {
|
||||||
|
name: string;
|
||||||
|
agents: string[] | undefined;
|
||||||
|
coverage: string[] | undefined;
|
||||||
|
};
|
||||||
|
|
||||||
|
const STRING_LITERAL = /"((?:\\.|[^"\\])*)"/g;
|
||||||
|
|
||||||
|
function extractStringLiterals(source: string): string[] {
|
||||||
|
const out: string[] = [];
|
||||||
|
STRING_LITERAL.lastIndex = 0;
|
||||||
|
let m: RegExpExecArray | null;
|
||||||
|
// biome-ignore lint/suspicious/noAssignInExpressions: idiomatic regex iteration
|
||||||
|
while ((m = STRING_LITERAL.exec(source))) {
|
||||||
|
out.push(m[1]);
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* extract a `key: [...]` array literal of strings from a test object. matches
|
||||||
|
* line-leading indented `key:` to avoid colliding with the same word inside
|
||||||
|
* prompts / template literals.
|
||||||
|
*/
|
||||||
|
function extractStringArray(source: string, key: string): string[] | undefined {
|
||||||
|
const re = new RegExp(`^\\s+${key}:\\s*\\[([\\s\\S]*?)\\]`, "m");
|
||||||
|
const m = source.match(re);
|
||||||
|
if (!m) return undefined;
|
||||||
|
return extractStringLiterals(m[1]);
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseTestFile(source: string): ParsedTest | null {
|
||||||
|
// strip line comments — `//` inside string literals is rare in test files,
|
||||||
|
// and the static parser doesn't need to be perfect (defensive default of
|
||||||
|
// "missing coverage = always run" covers parse misses).
|
||||||
|
const stripped = source.replace(/\/\/[^\n]*$/gm, "");
|
||||||
|
const nameMatch = stripped.match(/^\s+name:\s*"([^"]+)"/m);
|
||||||
|
if (!nameMatch) return null;
|
||||||
|
return {
|
||||||
|
name: nameMatch[1],
|
||||||
|
agents: extractStringArray(stripped, "agents"),
|
||||||
|
coverage: extractStringArray(stripped, "coverage"),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function loadDir(dir: string): ParsedTest[] {
|
||||||
|
const dirPath = join(__dirname, dir);
|
||||||
|
if (!existsSync(dirPath)) return [];
|
||||||
|
const files = readdirSync(dirPath).filter((f) => f.endsWith(".ts"));
|
||||||
|
const out: ParsedTest[] = [];
|
||||||
|
for (const file of files) {
|
||||||
|
const source = readFileSync(join(dirPath, file), "utf8");
|
||||||
|
const parsed = parseTestFile(source);
|
||||||
|
if (parsed) out.push(parsed);
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* derive the active agent list from `agents/index.ts` so adding a new harness
|
||||||
|
* file automatically wires it into the matrix. avoids dynamic-import
|
||||||
|
* (transitively pulls `@actions/core` etc. — would explode in the no-install
|
||||||
|
* `changes` job) by regex-parsing the imports the same way `parseTestFile`
|
||||||
|
* handles tests.
|
||||||
|
*/
|
||||||
|
function loadAgents(): string[] {
|
||||||
|
const indexPath = join(__dirname, "..", "agents", "index.ts");
|
||||||
|
const source = readFileSync(indexPath, "utf8");
|
||||||
|
const out: string[] = [];
|
||||||
|
const re = /^\s*import\s+\{\s*(\w+)\s*\}\s+from\s+"\.\/(\w+)\.ts"/gm;
|
||||||
|
let m: RegExpExecArray | null;
|
||||||
|
// biome-ignore lint/suspicious/noAssignInExpressions: idiomatic regex iteration
|
||||||
|
while ((m = re.exec(source))) {
|
||||||
|
if (m[2] === "shared") continue;
|
||||||
|
out.push(m[1]);
|
||||||
|
}
|
||||||
|
return out.sort();
|
||||||
|
}
|
||||||
|
|
||||||
|
function readChangedFiles(): string[] {
|
||||||
|
const raw = readFileSync(0, "utf8").trim();
|
||||||
|
if (!raw) return [];
|
||||||
|
const parsed: unknown = JSON.parse(raw);
|
||||||
|
if (!Array.isArray(parsed)) {
|
||||||
|
throw new Error("matrix: stdin must be a JSON array of changed paths");
|
||||||
|
}
|
||||||
|
return parsed.map((p) => {
|
||||||
|
if (typeof p !== "string") {
|
||||||
|
throw new Error(`matrix: non-string entry in changed paths: ${JSON.stringify(p)}`);
|
||||||
|
}
|
||||||
|
return p;
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildAgentsMatrix(input: { changedFiles: string[]; full: boolean }): AgentEntry[] {
|
||||||
|
const tests = loadDir("crossagent");
|
||||||
|
const allAgents = loadAgents();
|
||||||
|
const out: AgentEntry[] = [];
|
||||||
|
for (const t of tests) {
|
||||||
|
if (!shouldRun({ changedFiles: input.changedFiles, coverage: t.coverage, full: input.full })) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
const agents = t.agents ?? allAgents;
|
||||||
|
for (const agent of agents) {
|
||||||
|
out.push({ agent, test: t.name, name: `${t.name}-${agent}` });
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildAgnosticMatrix(input: { changedFiles: string[]; full: boolean }): AgnosticEntry[] {
|
||||||
|
const tests = loadDir("agnostic");
|
||||||
|
const out: AgnosticEntry[] = [];
|
||||||
|
for (const t of tests) {
|
||||||
|
if (!shouldRun({ changedFiles: input.changedFiles, coverage: t.coverage, full: input.full })) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
out.push({ test: t.name, name: t.name });
|
||||||
|
}
|
||||||
|
return out;
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildFlagshipsMatrix(input: {
|
||||||
|
changedFiles: string[];
|
||||||
|
full: boolean;
|
||||||
|
filter: string;
|
||||||
|
}): SlugEntry[] {
|
||||||
|
const all = buildFlagshipMatrix({ filter: input.filter });
|
||||||
|
const byName = new Map(providers.map((p) => [p.flagship, p]));
|
||||||
|
return all.filter((entry) => {
|
||||||
|
const provider = byName.get(entry.slug);
|
||||||
|
return shouldRun({
|
||||||
|
changedFiles: input.changedFiles,
|
||||||
|
coverage: provider?.coverage,
|
||||||
|
full: input.full,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function buildAliasesMatrix(input: {
|
||||||
|
changedFiles: string[];
|
||||||
|
full: boolean;
|
||||||
|
filter: string;
|
||||||
|
includePassthroughs: boolean;
|
||||||
|
}): SlugEntry[] {
|
||||||
|
const all = buildAliasMatrix({
|
||||||
|
filter: input.filter,
|
||||||
|
includePassthroughs: input.includePassthroughs,
|
||||||
|
});
|
||||||
|
const coverageByProvider = new Map(providers.map((p) => [p.name, p.coverage]));
|
||||||
|
return all.filter((entry) => {
|
||||||
|
const provider = entry.slug.split("/")[0];
|
||||||
|
return shouldRun({
|
||||||
|
changedFiles: input.changedFiles,
|
||||||
|
coverage: coverageByProvider.get(provider),
|
||||||
|
full: input.full,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function main(): void {
|
||||||
|
const full = process.env.FULL === "1";
|
||||||
|
const filter = process.env.MATRIX_FILTER?.trim().toLowerCase() ?? "";
|
||||||
|
const includePassthroughs = process.env.INCLUDE_PASSTHROUGHS === "1";
|
||||||
|
const changedFiles = full ? [] : readChangedFiles();
|
||||||
|
|
||||||
|
const output: MatrixOutput = {
|
||||||
|
agents: buildAgentsMatrix({ changedFiles, full }),
|
||||||
|
agnostic: buildAgnosticMatrix({ changedFiles, full }),
|
||||||
|
flagships: buildFlagshipsMatrix({ changedFiles, full, filter }),
|
||||||
|
aliases: buildAliasesMatrix({ changedFiles, full, filter, includePassthroughs }),
|
||||||
|
};
|
||||||
|
|
||||||
|
process.stdout.write(JSON.stringify(output));
|
||||||
|
}
|
||||||
|
|
||||||
|
if (import.meta.url === `file://${process.argv[1]}`) {
|
||||||
|
main();
|
||||||
|
}
|
||||||
@@ -0,0 +1,88 @@
|
|||||||
|
/**
|
||||||
|
* provider catalog — the source of truth for `providers-live` (full harness
|
||||||
|
* smoke per provider) and the per-provider coverage globs that scope `models-live`
|
||||||
|
* (per-alias CLI smoke).
|
||||||
|
*
|
||||||
|
* each entry pins one standard-tier flagship slug per provider — not the
|
||||||
|
* pro/opus tier (too expensive for per-push) and not the free/experimental
|
||||||
|
* tier (too flaky). these flagships catch provider-class regressions like
|
||||||
|
* Gemini schema sanitization or OpenAI tool-call format drift that the cheap
|
||||||
|
* per-alias CLI smoke can't see.
|
||||||
|
*
|
||||||
|
* `coverage` lists the source files that, when changed, should rerun this
|
||||||
|
* provider's flagship + every alias of this provider. `action/models.ts` is
|
||||||
|
* included on every entry — touching the resolution table reruns all model
|
||||||
|
* tests (simple model; matches the per-PR-precision answer from planning).
|
||||||
|
*
|
||||||
|
* adding a new provider:
|
||||||
|
* 1. add an entry here with the flagship slug, agent harness, coverage globs
|
||||||
|
* 2. add a row to wiki/models-catalog.md "To add a provider"
|
||||||
|
* 3. CI picks it up automatically — no workflow change
|
||||||
|
*/
|
||||||
|
|
||||||
|
export type ProviderEntry = {
|
||||||
|
name: string;
|
||||||
|
/** flagship slug for `providers-live` full-harness smoke. */
|
||||||
|
flagship: string;
|
||||||
|
/** harness used by the runtime for this provider's models. */
|
||||||
|
agent: "claude" | "opencode";
|
||||||
|
/** repo-relative globs that invalidate this provider's matrix entries. */
|
||||||
|
coverage: string[];
|
||||||
|
};
|
||||||
|
|
||||||
|
const SHARED_OPENCODE_COVERAGE = [
|
||||||
|
"action/models.ts",
|
||||||
|
"action/agents/opencode.ts",
|
||||||
|
"action/agents/opencodePlugin.ts",
|
||||||
|
];
|
||||||
|
|
||||||
|
export const providers: ProviderEntry[] = [
|
||||||
|
{
|
||||||
|
name: "anthropic",
|
||||||
|
flagship: "anthropic/claude-sonnet",
|
||||||
|
agent: "claude",
|
||||||
|
coverage: ["action/models.ts", "action/agents/claude.ts"],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "openai",
|
||||||
|
flagship: "openai/gpt",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "google",
|
||||||
|
flagship: "google/gemini-pro",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: [...SHARED_OPENCODE_COVERAGE, "action/mcp/geminiSanitizer.ts"],
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "xai",
|
||||||
|
flagship: "xai/grok",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "deepseek",
|
||||||
|
flagship: "deepseek/deepseek-pro",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "moonshotai",
|
||||||
|
flagship: "moonshotai/kimi-k2",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "opencode",
|
||||||
|
flagship: "opencode/big-pickle",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "openrouter",
|
||||||
|
flagship: "openrouter/claude-sonnet",
|
||||||
|
agent: "opencode",
|
||||||
|
coverage: SHARED_OPENCODE_COVERAGE,
|
||||||
|
},
|
||||||
|
];
|
||||||
@@ -328,6 +328,10 @@ export interface TestRunnerOptions {
|
|||||||
// - "agnostic": runs with opencode only, excluded when filtering by agent
|
// - "agnostic": runs with opencode only, excluded when filtering by agent
|
||||||
// - "adhoc": excluded from default runs, must be explicitly requested
|
// - "adhoc": excluded from default runs, must be explicitly requested
|
||||||
tags?: TestTag[];
|
tags?: TestTag[];
|
||||||
|
// repo-relative globs of source files that, when changed in a PR, should
|
||||||
|
// trigger this test in CI. omit to opt out of filtering (test always runs
|
||||||
|
// — the defensive default). see action/test/coverage.ts.
|
||||||
|
coverage?: string[];
|
||||||
}
|
}
|
||||||
|
|
||||||
export type TestTag = "adhoc" | "agnostic" | "security";
|
export type TestTag = "adhoc" | "agnostic" | "security";
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import type { AgentUsage } from "./agents/shared.ts";
|
import type { AgentUsage } from "./agents/shared.ts";
|
||||||
import type { PrepResult } from "./prep/types.ts";
|
import type { PrepResult } from "./prep/types.ts";
|
||||||
|
import type { AgentDiagnostic } from "./utils/agentHangReport.ts";
|
||||||
import { log } from "./utils/cli.ts";
|
import { log } from "./utils/cli.ts";
|
||||||
import type { DiffCoverageState } from "./utils/diffCoverage.ts";
|
import type { DiffCoverageState } from "./utils/diffCoverage.ts";
|
||||||
import {
|
import {
|
||||||
@@ -157,6 +158,12 @@ export interface ToolState {
|
|||||||
model?: string | undefined;
|
model?: string | undefined;
|
||||||
todoTracker?: TodoTracker | undefined;
|
todoTracker?: TodoTracker | undefined;
|
||||||
diffCoverage?: DiffCoverageState | undefined;
|
diffCoverage?: DiffCoverageState | undefined;
|
||||||
|
// mutable handle the agent harness writes to as a run progresses (recent
|
||||||
|
// stderr ring buffer reference, last provider-error label, event count).
|
||||||
|
// read by main.ts's outer catch so a watchdog-fired activity timeout still
|
||||||
|
// surfaces the same agent-side context the harness's own catch path returns
|
||||||
|
// via `result.error`. see `utils/agentHangReport.ts`.
|
||||||
|
agentDiagnostic?: AgentDiagnostic | undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
interface InitToolStateParams {
|
interface InitToolStateParams {
|
||||||
|
|||||||
@@ -0,0 +1,115 @@
|
|||||||
|
const MAX_STDERR_BYTES = 3000;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* mutable per-run handle the agent harness writes to as a run progresses.
|
||||||
|
* the action's outer try/catch in `main.ts` reads this off `toolState` when
|
||||||
|
* the activity-timeout watchdog wins the race against the harness's own
|
||||||
|
* catch — the bare timer reject reason ("activity timeout: no output for
|
||||||
|
* 302s") tells the user nothing actionable, but `recentStderr` +
|
||||||
|
* `lastProviderError` together usually point straight at the upstream cause.
|
||||||
|
*
|
||||||
|
* `recentStderr` is shared by reference with the harness's bounded ring
|
||||||
|
* buffer, so the diagnostic always reflects the latest captured tail.
|
||||||
|
*/
|
||||||
|
export type AgentDiagnostic = {
|
||||||
|
/** display label for the agent, e.g. "Pullfrog". used in the headline. */
|
||||||
|
label: string;
|
||||||
|
/** shared reference to the harness's bounded stderr ring buffer. */
|
||||||
|
recentStderr: string[];
|
||||||
|
/** most-recent provider-error label from `detectProviderError`, if any. */
|
||||||
|
lastProviderError: string | undefined;
|
||||||
|
/** count of stdout events successfully parsed before the failure. */
|
||||||
|
eventCount: number;
|
||||||
|
};
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Build a user-facing markdown body for an agent hang or failure.
|
||||||
|
*
|
||||||
|
* Rendered into both the PR progress comment and the GitHub Actions job
|
||||||
|
* summary. Returns `null` when no diagnostic is available, which signals to
|
||||||
|
* the caller to fall back to its bare-error rendering.
|
||||||
|
*
|
||||||
|
* `errorMessage` is the underlying timer / spawn reject string (e.g.
|
||||||
|
* `activity timeout: no output for 301s`). The idle seconds are parsed out
|
||||||
|
* of it for the hang explanation — total runtime would overstate the stall
|
||||||
|
* for runs that streamed for a long time before going quiet.
|
||||||
|
*/
|
||||||
|
export function formatAgentHangBody(input: {
|
||||||
|
diagnostic: AgentDiagnostic | undefined;
|
||||||
|
isHang: boolean;
|
||||||
|
errorMessage: string;
|
||||||
|
}): string | null {
|
||||||
|
if (!input.diagnostic) return null;
|
||||||
|
|
||||||
|
const verb = input.isHang ? "stalled" : "failed";
|
||||||
|
const cause = input.diagnostic.lastProviderError
|
||||||
|
? ` — likely cause: \`${input.diagnostic.lastProviderError}\``
|
||||||
|
: "";
|
||||||
|
const headline = `**${input.diagnostic.label} ${verb}**${cause}`;
|
||||||
|
|
||||||
|
const explanation = formatExplanation({
|
||||||
|
isHang: input.isHang,
|
||||||
|
errorMessage: input.errorMessage,
|
||||||
|
});
|
||||||
|
const parts = [headline, "", `${explanation} ${formatEventsPart(input.diagnostic)}`];
|
||||||
|
|
||||||
|
const tail = renderStderrTail(input.diagnostic.recentStderr);
|
||||||
|
if (tail) {
|
||||||
|
// pick a fence longer than any backtick run in the body so a stderr line
|
||||||
|
// containing ``` (provider error JSON occasionally embeds it) can't
|
||||||
|
// terminate the fence early and corrupt the rest of the markdown.
|
||||||
|
const fence = pickFence(tail);
|
||||||
|
parts.push(
|
||||||
|
"",
|
||||||
|
"<details><summary>Recent agent stderr</summary>",
|
||||||
|
"",
|
||||||
|
fence,
|
||||||
|
tail,
|
||||||
|
fence,
|
||||||
|
"",
|
||||||
|
"</details>"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
return parts.join("\n");
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatExplanation(input: { isHang: boolean; errorMessage: string }): string {
|
||||||
|
if (!input.isHang) return `The agent exited unexpectedly: ${input.errorMessage}`;
|
||||||
|
const idleSec = parseIdleSec(input.errorMessage);
|
||||||
|
if (idleSec === undefined) {
|
||||||
|
return "The agent stopped emitting events and was killed by the activity-timeout watchdog.";
|
||||||
|
}
|
||||||
|
return `The agent stopped emitting events for ${idleSec}s and was killed by the activity-timeout watchdog.`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function parseIdleSec(message: string): number | undefined {
|
||||||
|
const match = /no output for (\d+)s/.exec(message);
|
||||||
|
return match ? Number(match[1]) : undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
function formatEventsPart(diagnostic: AgentDiagnostic): string {
|
||||||
|
if (diagnostic.eventCount > 0) {
|
||||||
|
return `${diagnostic.eventCount} events were processed before the failure.`;
|
||||||
|
}
|
||||||
|
// when the provider-error label already names the cause in the headline,
|
||||||
|
// the reachability nudge below contradicts it (e.g. an immediate 401 also
|
||||||
|
// produces zero events but isn't a reachability problem). suppress it.
|
||||||
|
if (diagnostic.lastProviderError) return "No events were emitted before the failure.";
|
||||||
|
return "No events were emitted — check whether the model provider is reachable.";
|
||||||
|
}
|
||||||
|
|
||||||
|
function renderStderrTail(lines: readonly string[]): string {
|
||||||
|
if (lines.length === 0) return "";
|
||||||
|
const joined = lines.join("\n");
|
||||||
|
if (joined.length <= MAX_STDERR_BYTES) return joined;
|
||||||
|
return `... (older lines truncated)\n${joined.slice(-MAX_STDERR_BYTES)}`;
|
||||||
|
}
|
||||||
|
|
||||||
|
function pickFence(content: string): string {
|
||||||
|
let max = 0;
|
||||||
|
for (const match of content.matchAll(/`+/g)) {
|
||||||
|
if (match[0].length > max) max = match[0].length;
|
||||||
|
}
|
||||||
|
return "`".repeat(Math.max(3, max + 1));
|
||||||
|
}
|
||||||
@@ -14,6 +14,7 @@ import { readFileSync, realpathSync, unlinkSync } from "node:fs";
|
|||||||
import { log } from "./cli.ts";
|
import { log } from "./cli.ts";
|
||||||
import type { GitAuthServer } from "./gitAuthServer.ts";
|
import type { GitAuthServer } from "./gitAuthServer.ts";
|
||||||
import { filterEnv } from "./secrets.ts";
|
import { filterEnv } from "./secrets.ts";
|
||||||
|
import { $ } from "./shell.ts";
|
||||||
import { spawn } from "./subprocess.ts";
|
import { spawn } from "./subprocess.ts";
|
||||||
|
|
||||||
type SafeGitSubcommand = "fetch" | "push";
|
type SafeGitSubcommand = "fetch" | "push";
|
||||||
@@ -181,3 +182,56 @@ export async function $git(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* shallow-clone unreachable: when an existing local depth is too shallow for
|
||||||
|
* git to traverse to the requested ref's ancestry, the remote walk fails with
|
||||||
|
* one of these wordings (git emits the full OID via oid_to_hex, so the bound
|
||||||
|
* is 40 for SHA-1 or 64 for SHA-256). detecting both lets a single deepen
|
||||||
|
* retry recover before the error reaches the agent — see issue #564 for the
|
||||||
|
* original `git_fetch` precedent and #656 for the `checkout_pr` follow-up.
|
||||||
|
*/
|
||||||
|
export const SHALLOW_UNREACHABLE_PATTERNS: RegExp[] = [
|
||||||
|
/Could not read [a-f0-9]{40,64}/,
|
||||||
|
/remote did not send all necessary objects/,
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* large enough to clear the merge base on most real-world PRs without
|
||||||
|
* downloading the full history; matches the fallback used by
|
||||||
|
* `checkoutPrBranch` when the GitHub compare API is unavailable.
|
||||||
|
*/
|
||||||
|
export const DEEPEN_RETRY_DEPTH = 1000;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* authenticated `git fetch` that recovers from shallow-unreachable errors
|
||||||
|
* by retrying once with `--deepen=1000`. callers pass the same args they
|
||||||
|
* would to `$git("fetch", ...)`; on shallow-unreachable failures in a
|
||||||
|
* shallow repo, the second attempt prepends `--deepen=N` and strips any
|
||||||
|
* caller-supplied `--depth=` (the two flags are mutually exclusive, and
|
||||||
|
* the caller's depth is what got us into this mess).
|
||||||
|
*
|
||||||
|
* non-shallow-unreachable errors and non-shallow repos rethrow unchanged,
|
||||||
|
* so this is safe to wrap any fetch without changing fast-path behavior.
|
||||||
|
*/
|
||||||
|
export async function $gitFetchWithDeepen(
|
||||||
|
args: string[],
|
||||||
|
options: GitAuthOptions,
|
||||||
|
label?: string
|
||||||
|
): Promise<GitResult> {
|
||||||
|
try {
|
||||||
|
return await $git("fetch", args, options);
|
||||||
|
} catch (err) {
|
||||||
|
const msg = err instanceof Error ? err.message : String(err);
|
||||||
|
const isShallowUnreachable = SHALLOW_UNREACHABLE_PATTERNS.some((p) => p.test(msg));
|
||||||
|
if (!isShallowUnreachable) throw err;
|
||||||
|
const isShallow =
|
||||||
|
$("git", ["rev-parse", "--is-shallow-repository"], { log: false }).trim() === "true";
|
||||||
|
if (!isShallow) throw err;
|
||||||
|
log.info(
|
||||||
|
`» ${label ?? "git fetch"} hit shallow-unreachable error, retrying with --deepen=${DEEPEN_RETRY_DEPTH}`
|
||||||
|
);
|
||||||
|
const retryArgs = args.filter((a) => !a.startsWith("--depth="));
|
||||||
|
return await $git("fetch", [`--deepen=${DEEPEN_RETRY_DEPTH}`, ...retryArgs], options);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -1,4 +1,8 @@
|
|||||||
import { detectProviderError, isRouterKeylimitExhaustedError } from "./providerErrors.ts";
|
import {
|
||||||
|
detectProviderError,
|
||||||
|
findProviderErrorMatch,
|
||||||
|
isRouterKeylimitExhaustedError,
|
||||||
|
} from "./providerErrors.ts";
|
||||||
|
|
||||||
describe("detectProviderError", () => {
|
describe("detectProviderError", () => {
|
||||||
describe("false positives previously seen in production", () => {
|
describe("false positives previously seen in production", () => {
|
||||||
@@ -116,6 +120,64 @@ describe("detectProviderError", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("findProviderErrorMatch", () => {
|
||||||
|
// regression for issue #703: when stderr arrives as a multi-KB buffer
|
||||||
|
// (mcp tool-schema dump + the actual error message), the old
|
||||||
|
// `chunk.substring(0, 500)` excerpt showed the head of the buffer
|
||||||
|
// (schema) instead of the matched error text. the windowed excerpt
|
||||||
|
// must center on the matched line.
|
||||||
|
it("excerpt centers on the matched line, not the head of the buffer", () => {
|
||||||
|
const schemaDump =
|
||||||
|
"{".repeat(2000) +
|
||||||
|
'"name":"pullfrog_create_pull_request_review","description":"Submit a review..."';
|
||||||
|
const errorLine = "ERROR 2026-05-13 service=session error=rate_limit_exceeded retry-after=30";
|
||||||
|
const chunk = `${schemaDump}\n${errorLine}\ncaller stack at handler.ts:42`;
|
||||||
|
|
||||||
|
const match = findProviderErrorMatch(chunk);
|
||||||
|
expect(match).not.toBeNull();
|
||||||
|
expect(match?.label).toBe("rate limited");
|
||||||
|
expect(match?.excerpt).toContain("rate_limit_exceeded");
|
||||||
|
expect(match?.excerpt).toContain("retry-after=30");
|
||||||
|
expect(match?.excerpt).not.toContain("pullfrog_create_pull_request_review");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("includes a small surrounding-line window for stack-trace context", () => {
|
||||||
|
const chunk =
|
||||||
|
"» about to call session.processor\n" +
|
||||||
|
"ERROR rate_limit_exceeded for key=abc\n" +
|
||||||
|
"at handler.ts:42\n" +
|
||||||
|
"at runtime.ts:88";
|
||||||
|
const match = findProviderErrorMatch(chunk);
|
||||||
|
expect(match?.excerpt).toContain("about to call session.processor");
|
||||||
|
expect(match?.excerpt).toContain("rate_limit_exceeded");
|
||||||
|
expect(match?.excerpt).toContain("handler.ts:42");
|
||||||
|
expect(match?.excerpt).toContain("runtime.ts:88");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to the matched line alone when adjacent lines are huge", () => {
|
||||||
|
const giantPrefix = "x".repeat(5000);
|
||||||
|
const errorLine = '"statusCode": 429, "message": "slow down"';
|
||||||
|
const giantSuffix = "y".repeat(5000);
|
||||||
|
const chunk = `${giantPrefix}\n${errorLine}\n${giantSuffix}`;
|
||||||
|
|
||||||
|
const match = findProviderErrorMatch(chunk);
|
||||||
|
expect(match?.label).toBe("rate limited (429)");
|
||||||
|
expect(match?.excerpt).toBe(errorLine);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("head-truncates the matched line if it alone exceeds the byte cap", () => {
|
||||||
|
const padding = "z".repeat(700);
|
||||||
|
const chunk = `${padding} "statusCode": 429 ${padding}`;
|
||||||
|
const match = findProviderErrorMatch(chunk);
|
||||||
|
expect(match?.label).toBe("rate limited (429)");
|
||||||
|
expect(match?.excerpt.length).toBeLessThanOrEqual(600);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns null when no pattern matches", () => {
|
||||||
|
expect(findProviderErrorMatch("just some normal log line\nnothing wrong here")).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("isRouterKeylimitExhaustedError", () => {
|
describe("isRouterKeylimitExhaustedError", () => {
|
||||||
it("matches the canonical OpenRouter mid-run error", () => {
|
it("matches the canonical OpenRouter mid-run error", () => {
|
||||||
expect(
|
expect(
|
||||||
|
|||||||
+60
-2
@@ -41,13 +41,71 @@ const PROVIDER_ERROR_PATTERNS: ProviderErrorPattern[] = [
|
|||||||
{ regex: /["']?\blimit\b["']?\s*:\s*0\b/, label: "zero quota" },
|
{ regex: /["']?\blimit\b["']?\s*:\s*0\b/, label: "zero quota" },
|
||||||
];
|
];
|
||||||
|
|
||||||
export function detectProviderError(text: string): string | null {
|
/**
|
||||||
|
* Result of a provider-error scan: the classification label plus a
|
||||||
|
* human-readable excerpt centered on the matched line. The excerpt is what
|
||||||
|
* gets surfaced in `» provider error detected (...)` log lines — see
|
||||||
|
* `extractExcerpt` for the windowing/byte-cap policy.
|
||||||
|
*/
|
||||||
|
export type ProviderErrorMatch = {
|
||||||
|
label: string;
|
||||||
|
excerpt: string;
|
||||||
|
};
|
||||||
|
|
||||||
|
// roughly half a wide terminal line by 4–5 lines of context; large enough
|
||||||
|
// to capture a structured error payload (request id, retry-after, model)
|
||||||
|
// plus its immediate stack/headers, small enough to not flood the log.
|
||||||
|
const EXCERPT_MAX_BYTES = 600;
|
||||||
|
const LINES_BEFORE = 1;
|
||||||
|
const LINES_AFTER = 2;
|
||||||
|
|
||||||
|
export function findProviderErrorMatch(text: string): ProviderErrorMatch | null {
|
||||||
for (const entry of PROVIDER_ERROR_PATTERNS) {
|
for (const entry of PROVIDER_ERROR_PATTERNS) {
|
||||||
if (entry.regex.test(text)) return entry.label;
|
const m = entry.regex.exec(text);
|
||||||
|
if (!m) continue;
|
||||||
|
return { label: entry.label, excerpt: extractExcerpt(text, m.index) };
|
||||||
}
|
}
|
||||||
return null;
|
return null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function detectProviderError(text: string): string | null {
|
||||||
|
return findProviderErrorMatch(text)?.label ?? null;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Slice a context window around `matchIndex`: the matched line plus
|
||||||
|
* `LINES_BEFORE`/`LINES_AFTER` neighbours. If the windowed slice exceeds
|
||||||
|
* `EXCERPT_MAX_BYTES` (giant adjacent lines, e.g. JSON tool-schema dumps),
|
||||||
|
* fall back to the matched line alone, head-truncated if still too long.
|
||||||
|
* Replaces the old `chunk.substring(0, 500)` head-anchored excerpt which
|
||||||
|
* surfaced whatever happened to be at the front of the stderr buffer
|
||||||
|
* instead of the error itself. See issue #703.
|
||||||
|
*/
|
||||||
|
function extractExcerpt(text: string, matchIndex: number): string {
|
||||||
|
const lineStart = text.lastIndexOf("\n", matchIndex - 1) + 1;
|
||||||
|
const lineEndRaw = text.indexOf("\n", matchIndex);
|
||||||
|
const lineEnd = lineEndRaw === -1 ? text.length : lineEndRaw;
|
||||||
|
|
||||||
|
let start = lineStart;
|
||||||
|
for (let i = 0; i < LINES_BEFORE && start > 0; i++) {
|
||||||
|
const prev = text.lastIndexOf("\n", start - 2);
|
||||||
|
start = prev < 0 ? 0 : prev + 1;
|
||||||
|
}
|
||||||
|
|
||||||
|
let end = lineEnd;
|
||||||
|
for (let i = 0; i < LINES_AFTER && end < text.length; i++) {
|
||||||
|
const next = text.indexOf("\n", end + 1);
|
||||||
|
end = next < 0 ? text.length : next;
|
||||||
|
}
|
||||||
|
|
||||||
|
let excerpt = text.slice(start, end);
|
||||||
|
if (excerpt.length > EXCERPT_MAX_BYTES) {
|
||||||
|
excerpt = text.slice(lineStart, lineEnd);
|
||||||
|
if (excerpt.length > EXCERPT_MAX_BYTES) excerpt = excerpt.slice(0, EXCERPT_MAX_BYTES);
|
||||||
|
}
|
||||||
|
return excerpt.trim();
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* OpenRouter's response when the per-run key's remaining budget can't cover
|
* OpenRouter's response when the per-run key's remaining budget can't cover
|
||||||
* the agent's `max_tokens` reservation. Distinct from a generic provider error
|
* the agent's `max_tokens` reservation. Distinct from a generic provider error
|
||||||
|
|||||||
Reference in New Issue
Block a user