Compare commits

...

8 Commits

Author SHA1 Message Date
Colin McDonnell 17b610e1a1 bump action version to 0.1.1 2026-05-08 21:32:06 +00:00
Colin McDonnell ca913c76ea spawn: kill process group + heartbeat subagent activity (#631)
* spawn: kill process group + heartbeat subagent activity

two compounding bugs produced zombie agent runs that stalled until the
GitHub-Actions job-level timeout (observed on PR #622, run 25577068620).

1. SIGKILL hit the wrong process. node_modules/opencode-ai/bin/opencode
   is a Node shim that spawnSyncs the native opencode-<plat>-<arch>
   binary with stdio:"inherit". our spawn() ran without detached, so
   child.kill("SIGKILL") killed only the shim. the native binary was
   reparented to PID 1, kept holding our stdout pipe via inherited fds,
   and child.on("close") never fired — leaving the agent promise
   pending past the 5min outer safety-net timer ("agent still pending
   5min after inner activity kill — forcing exit") and the grandchild
   running until the runner timed out.

   fix: SpawnOptions gains killGroup; when set, we spawn detached and
   route all kill paths (timeout, activity timeout, ctrl-c) through
   process.kill(-pid, signal). opencode + claude opt in.

2. inner activity timer false-fired during long task subagents.
   opencode's `task` tool encapsulates subagent execution in-process —
   subagent-internal events don't reach the parent NDJSON stream — so
   the parent looked idle for the full subagent duration even when
   real work was happening, and the 5min DEFAULT_ACTIVITY_TIMEOUT_MS
   would fire mid-subagent.

   fix: SpawnOptions gains externalActivitySource; the timer fires on
   min(local stdout idle, external idle). opencode passes getIdleMs()
   from the global activity tracker and runs a 30s heartbeat
   (markActivity()) while at least one task dispatch is in flight.

action/utils/subprocess.test.ts covers both: a bash+sleep grandchild
that proves close fires <10s with killGroup, and externalActivitySource
keeping the timer armed during 8s of stdout silence.

* opencode: suspend activity timer instead of heartbeat during subagent runs

addresses review on prior commit: replace the 30s markActivity()
heartbeat with a boolean isPausedExternally predicate keyed off
opencode's existing taskDispatchByCallID + pendingTaskDispatches.
no fake activity, no race window between a 30s tick and a subagent
that finishes between ticks.

while the predicate returns true, spawn's activity check skips the
kill decision *and* advances lastActivityTime so a clean unpause
can't fire on a stale baseline. tests cover both the suspended case
(8s of stdout silence + activityTimeout=1s but paused → process
exits cleanly) and the resume case (paused for 500ms then unpaused
→ 30s sleep gets killed by activity timeout as normal).
2026-05-08 21:29:22 +00:00
Colin McDonnell 20d4b12522 bump action version to 0.1.0
document direct-to-main exceptions in AGENTS.md (version bumps and
other release-trigger commits when the user explicitly says "push to
main").
2026-05-08 21:26:53 +00:00
Colin McDonnell ec43c0e0d1 router: fix bugs from PR #616 review (#625)
Three real defects flagged in the post-merge review of #616, plus one cheap
hardening:

1. OpenCode `limit.output` override was a silent no-op on opencode-ai@1.1.56.
   Top-level `limit.output` has no read site in OpenCode (verified against
   the v1.1.56 source: `OUTPUT_TOKEN_MAX = Flag.OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX
   || 32_000` in session/llm.ts; per-model `model.limit.output` has its own
   scope). Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX=5000` env var
   on the OpenCode spawn instead. Drops dead `OpenCodeConfig.limit?` type
   field and the corresponding config write in `buildSecurityConfig`. This
   was the headline mechanism of #616 — without the env var, the upfront
   `max_tokens` reservation stayed at 32_000 and low-wallet runs continued
   failing the way #616 was supposed to prevent.

2. Phantom auto-reload buffer for detached-card accounts. DELETE
   /payment-method clears `stripeCustomerId` but leaves `autoReloadEnabled`
   intact, so an account with welcome-credit residue and a detached card
   could mint a key with `keyLimitCents = balance + autoReloadAmountCents`
   ($50 default, schema-cap $100K) of free spend headroom we have no way
   to bill. Conjunctive `account.autoReloadEnabled && hasCard` in the
   buffer selection closes this. Defense-in-depth follow-up worth doing:
   clear `autoReloadEnabled` in the card-detach handler.

3. The autoReloadEnabled 402 branch fired for phase-1 noop paths
   (`!stripeCustomerId`, `reloadAmountCents < 50`, `balance >= threshold`)
   where `result.failure == null`, returning `"insufficient balance"` with
   no actionable code. Gated on `result.status === "failed"` so non-charge
   paths fall through to the `hasCard` / no-card branches and emit
   `router_balance_exhausted` / `router_requires_card` instead.

4. (cheap) `ROUTER_KEYLIMIT_EXHAUSTED_PATTERN` now uses `/is` instead of
   `/i` so `.*?` crosses newlines. Defends the BillingError reclassification
   against any upstream layer that wraps the OpenRouter error onto multiple
   lines. Trivial.

Test plan: 488/488 unit tests pass (1 new test for newline regex behavior).
2026-05-08 21:02:38 +00:00
Colin McDonnell 93cc7b1a44 show effective model in agent comment/review footers (#618)
`toolState.model` was set only to `payload.model` (the stored slug, often
undefined for router/oss runs that derive the target from `proxyModel`).
the footer's "Using `…`" segment is gated on a truthy model, so router
runs on repos without an explicit model setting shipped reviews/comments
with no model badge — e.g. PR #614's review showed no model despite
running `openrouter/anthropic/claude-opus-4.7` via proxy.

now mirror the priority used by `resolveModelForLog` and `isGeminiRouted`:
`payload.proxyModel ?? resolvedModel ?? payload.model`. also reverse-look
up by `resolve`/`openRouterResolve` in `formatModelLabel` so a proxy
target like "openrouter/anthropic/claude-opus-4.7" still renders as
"Claude Opus".
2026-05-08 20:59:09 +00:00
pullfrog[bot] 851e49e2d7 action: retry transient GitHub 422 "internal error" on review submission (#610)
* action: retry transient GitHub 422 "internal error" on review submission

GitHub sometimes 422s POST /pulls/{n}/reviews with body
"An internal error occurred, please try again." — a server-side hiccup
that the existing 422 handler framed with the generic
"likely causes (1)(2)(3)" prompt listing affected comments. the agent
dutifully refetched the diff, dropped comments, and resubmitted, hitting
the same transient error on a shifting affected-comments list until
GitHub accepted. some runs logged 8+ spurious retries with ~11 minutes
of wall-clock, dropping valid inline comments along the way.

detect the transient body explicitly, retry in-tool twice with 1s/3s
backoff, and surface a distinct error on exhaustion that tells the agent
this is a GitHub-side issue — do not modify inline comments, wait and
retry or fall back to a body-only review. closes #584.

* action: use retry util for transient review 422, drop isTransientReviewError tests

---------

Co-authored-by: pullfrog[bot] <226033991+pullfrog[bot]@users.noreply.github.com>
2026-05-08 20:33:01 +00:00
Colin McDonnell 4101df566b router: decouple per-run key budget from wallet, add overdraft buffer (#616)
Replaces today's `keyLimitUsd = min(walletBalance, $25)` with population-aware
buffers so users can use 100% of their credits before being paywalled, and
opaque mid-run "more credits" failures (e.g. https://github.com/pullfrog/app/actions/runs/25531633203)
get a clear PR comment instead of a generic stack-trace dump.

Policy matrix:
- Auto-reload accounts: `wallet + autoReloadAmountCents` (default $50, no cap)
- Card + no-autoreload: `wallet + $5` overdraft buffer
- No card: `wallet` (no buffer; existing zero-balance 402 stays)
- OSS: `$10` (unchanged)

Removes the $25 per-run cap entirely. Long Build runs at high-balance
accounts no longer silently cap at $25.

Other changes:
- Classify mid-run OpenRouter "requires more credits, or fewer max_tokens"
  errors as `router_keylimit_exhausted` BillingError so users get an
  actionable PR comment.
- Override OpenCode `max_tokens: 32000` default to `5000` via
  OpenCodeConfig.limit.output. Drops Opus per-call upfront budget reservation
  from ~$2.40 to ~$0.38 — what makes low-wallet runs viable at all.
- Switch `findInitialComment` and `findExistingPaywallComment` to GraphQL
  `issueOrPullRequest(number:) { comments(last: 100) }` (single round trip,
  actually returns newest-100; REST listComments doesn't support sort/direction).
  Also fixes a latent `comments.find()` returning the OLDEST match instead
  of the most recent — now selects max(databaseId).
- Wrap `syncAccountUsage` in `prisma.$transaction` with `SELECT ... FOR UPDATE`
  on the account row. Pre/post-balance reads inside the transaction enable
  deterministic low-balance edge detection (currently logs; will push the
  outreach.low_balance task once #592 lands).

Plan: .cursor/plans/router-low-balance-paywall.plan.md (in companion wiki-billing branch)
2026-05-08 20:15:47 +00:00
Colin McDonnell 9d04cad360 drop legacy summaryCommentNodeId column (#617)
Was retained on `workflow_runs` after PR #568 replaced the comment-based
summary path with the snapshot architecture, with a "kept for backfill of
pre-snapshot runs" annotation. No backfill is planned: pre-snapshot summary
comments were written in the user-facing PR_SUMMARY_FORMAT (TL;DR + key
changes blockquote + before/after sections), not the agent-context
functional-summary format the snapshot now expects. Backfilling them would
prime new runs with the wrong shape and pollute the agent context. Old
comments stay on github.com as historical artifacts; the column on the DB
row is dead weight.

Strips the field from:
- prisma schema + new migration `20260508190000_drop_summary_comment_node_id`
- `app/api/workflow-run/[runId]/route.ts` STRING_FIELDS allowlist
- `action/utils/patchWorkflowRunFields.ts` type union + STRING_KEYS
- `utils/db/selectActiveWorkflowRuns.ts` select clause
- `utils/github/enrichWorkflowRunsWithArtifactUrls.ts` node-id type, URL
  resolution, collectUniqueNodeIds + urlsForRun
- `utils/webhooks/handleWorkflowRunWebhook.ts` two select clauses, the
  hasRecordedArtifact param, and the orphaned-leaping-comment alert text
- `components/RunArtifactPills.tsx` ArtifactKey union + ARTIFACT_KEYS +
  switch cases (drops the "View summary" chip from the workflow run list)

Verified: pnpm typecheck clean, pnpm lint clean (537 files), action build
clean. Dev DB reset against production parent and the migration applied
cleanly — column is gone from the workflow_runs table.
2026-05-08 19:47:38 +00:00
12 changed files with 390 additions and 23 deletions
+6
View File
@@ -407,6 +407,12 @@ async function runClaude(params: RunParams): Promise<ClaudeRunResult> {
activityTimeout: 300_000,
onActivityTimeout: params.onActivityTimeout,
stdio: ["ignore", "pipe", "pipe"],
// run claude in its own process group so SIGKILL on activity timeout /
// outer cancellation reaches any subprocesses it spawns (rg, file
// watchers, mcp transports, etc). claude itself is a node bundle so
// there's no shim-orphan issue like opencode-ai/bin/opencode, but
// detached + killGroup is the right default for any agent runtime.
killGroup: true,
onStdout: async (chunk) => {
const text = chunk.toString();
output += text;
+41
View File
@@ -59,6 +59,22 @@ type OpenCodeConfig = {
[key: string]: unknown;
};
/**
* Per-inference `max_tokens` reservation the agent sends to the upstream
* model. OpenCode's default is 32_000 (sized for long-running TUI sessions
* where a human user might want big outputs). Pullfrog runs are headless and
* short — typical outputs are 1-3K tokens — so we cap at 5_000. This
* drastically reduces the upfront budget reservation OpenRouter requires per
* call (~$0.38 vs ~$2.40 for Opus), which is what lets low-wallet runs
* actually start.
*
* Plumbed via `OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX` env var rather than the
* config JSON. OpenCode's `OUTPUT_TOKEN_MAX` (session/llm.ts) is sourced
* exclusively from this env var; top-level `limit.output` in the config
* has no read site and is silently dropped on merge.
*/
const PULLFROG_OPENCODE_OUTPUT_LIMIT = 5000;
function buildSecurityConfig(ctx: AgentRunContext, model: string | undefined): string {
const config: OpenCodeConfig = {
permission: {
@@ -352,6 +368,16 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
// mismatched callID" signal).
const knownNonTaskCallIDs = new Set<string>();
// a subagent is "in flight" while at least one `task` tool_use is awaiting
// its tool_result. used to suspend spawn's activity timer (which only sees
// opencode's stdout) so a long-running subagent — whose internal events
// don't surface on the parent NDJSON stream — can't trip the 5min timeout.
// boolean state instead of a heartbeat so there's no race window between a
// periodic tick and a subagent that finishes between ticks.
function isSubagentInFlight(): boolean {
return taskDispatchByCallID.size > 0 || pendingTaskDispatches.length > 0;
}
function emitSubagentFinished(
dispatch: TaskDispatch,
status: string,
@@ -693,6 +719,20 @@ async function runOpenCode(params: RunParams): Promise<AgentResult> {
activityTimeout: 300_000,
onActivityTimeout: params.onActivityTimeout,
stdio: ["ignore", "pipe", "pipe"],
// node_modules/opencode-ai/bin/opencode is a Node shim that spawnSyncs
// the native opencode-<plat>-<arch> binary with stdio:"inherit". without
// a process-group kill, SIGKILL hits only the shim, the native binary
// is reparented to PID 1, holds our stdout pipe open, and `child.close`
// never fires — producing zombie runs. detached + killGroup nukes the
// whole tree.
killGroup: true,
// suspend the inner activity timer while a `task` subagent is in flight.
// opencode's task tool encapsulates subagent execution in-process — the
// subagent's internal events don't surface on the parent NDJSON stream,
// so without this the 5min timeout would falsely fire mid-subagent.
// suspend/resume is preferable to a heartbeat because there's no race
// between a periodic tick and a subagent finishing between ticks.
isPausedExternally: isSubagentInFlight,
onStdout: async (chunk) => {
const text = chunk.toString();
output += text;
@@ -929,6 +969,7 @@ export const opencode = agent({
...homeEnv,
OPENCODE_CONFIG_CONTENT: buildSecurityConfig(ctx, model),
OPENCODE_PERMISSION: permissionOverride,
OPENCODE_EXPERIMENTAL_OUTPUT_TOKEN_MAX: PULLFROG_OPENCODE_OUTPUT_LIMIT.toString(),
GOOGLE_GENERATIVE_AI_API_KEY:
process.env.GOOGLE_GENERATIVE_AI_API_KEY || process.env.GEMINI_API_KEY,
};
+53 -2
View File
@@ -35,6 +35,7 @@ import { executeLifecycleHook } from "./utils/lifecycle.ts";
import { normalizeEnv } from "./utils/normalizeEnv.ts";
import { aggregateUsage, patchWorkflowRunFields } from "./utils/patchWorkflowRunFields.ts";
import { resolvePayload, resolvePromptInput } from "./utils/payload.ts";
import { isRouterKeylimitExhaustedError } from "./utils/providerErrors.ts";
import { readSummaryFile, seedSummaryFile } from "./utils/prSummary.ts";
import { postReviewCleanup } from "./utils/reviewCleanup.ts";
import { handleAgentResult } from "./utils/run.ts";
@@ -186,6 +187,14 @@ function billingConsoleUrl(owner: string, anchor: "billing" | "model-access"): s
* - `router_requires_card`: user is on Router mode with no card AND no
* wallet balance. Lead with the carrot ($20 free credit), link to
* `#model-access` where the Add Card flow lives.
* - `router_balance_exhausted`: user has a card on file but auto-reload is
* disabled and they've spent past their $5 overdraft buffer. Frame as
* "balance ran out" and surface both remediation paths (top up, or flip
* on auto-reload).
* - `router_keylimit_exhausted`: OpenRouter rejected mid-run because the
* per-run key budget was exhausted while the agent was working. The
* wallet is now negative; same remediation as `router_balance_exhausted`
* but framed for the after-the-fact case ("this run was cut short").
* - `needsReauthentication`: issuer requires 3DS on every off-session
* charge. Re-adding the card won't help — the only escape is a manual
* top-up where 3DS runs interactively in Stripe Checkout.
@@ -205,6 +214,26 @@ function formatBillingErrorSummary(error: BillingError, owner: string): string {
].join("\n");
}
if (error.code === "router_balance_exhausted") {
return [
"**Your Pullfrog Router balance is exhausted.**",
"",
"You have a card on file but auto-reload is disabled, so runs paused once your balance went past the overdraft buffer.",
"",
`[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`,
].join("\n");
}
if (error.code === "router_keylimit_exhausted") {
return [
"**This run was cut short — your Pullfrog Router balance ran out mid-run.**",
"",
"OpenRouter stopped the agent because the per-run budget was exhausted. Your wallet is now negative; top up or enable auto-reload to keep runs flowing.",
"",
`[Top up balance →](${billingConsoleUrl(owner, "billing")}) · [Enable auto-reload →](${billingConsoleUrl(owner, "model-access")})`,
].join("\n");
}
if (error.needsReauthentication) {
const code = error.declineCode ?? "authentication_required";
return [
@@ -549,6 +578,12 @@ export async function main(): Promise<MainResult> {
const resolvedModel = payload.proxyModel ? undefined : resolveModel({ slug: payload.model });
const agent = resolveAgent({ model: resolvedModel });
// surface the effective model in comment/review footers. payload.model is
// just the stored slug (often undefined for router/oss runs that derive
// the target from proxyModel). matching priority with resolveModelForLog
// so the "Using `…`" badge reflects what actually ran.
toolState.model = payload.proxyModel ?? resolvedModel ?? payload.model;
validateAgentApiKey({
agent,
model: payload.proxyModel ?? resolvedModel ?? payload.model,
@@ -887,16 +922,32 @@ export async function main(): Promise<MainResult> {
killTrackedChildren();
log.error(errorMessage);
// Reclassify OpenRouter "key budget exhausted" mid-run errors as
// BillingError. The agent runtime surfaces this as a generic APIError,
// but it's a Pullfrog billing concern — the user's Router wallet ran
// out partway through the run. Route through the same formatBillingErrorSummary
// path as proxy-token 402s so the user gets actionable copy + a top-up
// CTA on both the job summary and the PR progress comment, instead of
// a generic "❌ Pullfrog failed" stack-trace dump.
const billingError = isRouterKeylimitExhaustedError(errorMessage)
? new BillingError(errorMessage, { code: "router_keylimit_exhausted" })
: null;
// best-effort summary — write the error so it's visible in the Actions summary tab
try {
const errorSummary = `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``;
const errorSummary = billingError
? formatBillingErrorSummary(billingError, runContext.repo.owner)
: `### ❌ Pullfrog failed\n\n\`\`\`\n${errorMessage}\n\`\`\``;
const usageSummary = formatUsageSummary(toolState.usageEntries);
const parts = [errorSummary, toolState.lastProgressBody, usageSummary].filter(Boolean);
await writeSummary(parts.join("\n\n"));
} catch {}
try {
await reportErrorToComment({ toolState, error: errorMessage });
const commentBody = billingError
? formatBillingErrorSummary(billingError, runContext.repo.owner)
: errorMessage;
await reportErrorToComment({ toolState, error: commentBody });
} catch {
// error reporting failed, but don't let it mask the original error
}
+65 -7
View File
@@ -11,6 +11,7 @@ import {
} from "../utils/diffCoverage.ts";
import { fixDoubleEscapedString } from "../utils/fixDoubleEscapedString.ts";
import { patchWorkflowRunFields } from "../utils/patchWorkflowRunFields.ts";
import { retry } from "../utils/retry.ts";
import { deleteProgressComment } from "./comment.ts";
import type { ToolContext } from "./server.ts";
import { execute, tool } from "./shared.ts";
@@ -21,6 +22,29 @@ function getHttpStatus(err: unknown): number | undefined {
return typeof status === "number" ? status : undefined;
}
/**
* detect GitHub's generic server-side 422 ("An internal error occurred,
* please try again.") that sometimes fires on `POST /pulls/{n}/reviews`.
*
* the body is stable across occurrences and distinct from every other 422
* cause we care about (anchor validation, body length, malformed suggestion
* blocks) — those all cite the specific problem. treating this as a
* transient server error unlocks bounded in-tool retry instead of surfacing
* it to the agent with the generic "likely causes (1)(2)(3)" prompt, which
* induces whack-a-mole comment dropping on content that was never the issue.
*/
export function isTransientReviewError(err: unknown): boolean {
if (getHttpStatus(err) !== 422) return false;
const msg = err instanceof Error ? err.message : String(err);
return /internal error occurred, please try again/i.test(msg);
}
// backoff schedule for transient GitHub 422 "internal error" responses on the
// reviews endpoint. 3 attempts total (initial + 2 retries) with 1s/3s delays
// — most transient GH errors clear within a few seconds, and longer delays
// push review submission past agent-perceived responsiveness.
export const TRANSIENT_REVIEW_RETRY_DELAYS_MS = [1_000, 3_000];
type PullFile = RestEndpointMethodTypes["pulls"]["listFiles"]["response"]["data"][number];
export type CommentableLines = { RIGHT: Set<number>; LEFT: Set<number> };
@@ -483,16 +507,50 @@ export function CreatePullRequestReviewTool(ctx: ToolContext) {
// no body → single-step createReview (no footer needed)
// has body → pending + submit so we can build footer with Fix links using review ID
//
// wrap the submission in `retry` so GitHub's transient 422 "internal
// error" body (distinct from anchor / body-length / suggestion 422s,
// which all cite the specific cause) clears on its own instead of
// surfacing through the generic 422 handler — that framing sent the
// agent dropping valid inline comments chasing a non-issue.
// `shouldRetry` scopes retries to the transient body only, so real
// validation 422s still fail fast.
let result;
try {
result = body
? await createAndSubmitWithFooter(ctx, params, {
body,
approved: approved ?? false,
hasComments: (params.comments?.length ?? 0) > 0,
})
: await createReviewWithStrandedRecovery(ctx, params);
result = await retry(
() =>
body
? createAndSubmitWithFooter(ctx, params, {
body,
approved: approved ?? false,
hasComments: (params.comments?.length ?? 0) > 0,
})
: createReviewWithStrandedRecovery(ctx, params),
{
delaysMs: TRANSIENT_REVIEW_RETRY_DELAYS_MS,
shouldRetry: isTransientReviewError,
label: "review submission",
}
);
} catch (err: unknown) {
// GitHub's transient 422 "internal error" is distinct from anchor /
// body-length / suggestion validation failures — framing it with the
// generic "likely causes (1)(2)(3)" prompt sends the agent dropping
// comments that were never the problem. after bounded in-tool retry
// we surface a dedicated message that tells the agent to wait-and-
// retry or fall back to a body-only review.
if (isTransientReviewError(err)) {
const rawMsg = err instanceof Error ? err.message : String(err);
throw new Error(
`GitHub returned a transient 422 "internal error" on the reviews endpoint after ${TRANSIENT_REVIEW_RETRY_DELAYS_MS.length + 1} attempts. ` +
`This is a GitHub-side issue, not a problem with your review content. ` +
`Do NOT modify or drop inline comments — their content is not the cause. ` +
`Wait ~30 seconds and call this tool once more with the SAME arguments. ` +
`If it still fails, submit a body-only review (move all inline feedback into \`body\` as text) so nothing is lost. ` +
`GitHub said: ${rawMsg}`,
{ cause: err }
);
}
if (getHttpStatus(err) !== 422 || !params.comments?.length) throw err;
const details = params.comments.map((c) => {
+1 -1
View File
@@ -1,6 +1,6 @@
{
"name": "pullfrog",
"version": "0.0.205",
"version": "0.1.1",
"type": "module",
"bin": {
"pullfrog": "dist/cli.mjs",
+8 -2
View File
@@ -1,4 +1,4 @@
import { resolveDisplayAlias } from "../models.ts";
import { modelAliases, resolveDisplayAlias } from "../models.ts";
export const PULLFROG_DIVIDER = "<!-- PULLFROG_DIVIDER_DO_NOT_REMOVE_PLZ -->";
@@ -28,7 +28,13 @@ export interface BuildPullfrogFooterParams {
function formatModelLabel(slug: string): string {
// walk the fallback chain so a deprecated stored slug shows the model the
// run actually executed against (e.g. "GPT", not "GPT Codex").
const alias = resolveDisplayAlias(slug);
const alias =
resolveDisplayAlias(slug) ??
// reverse-lookup: when the caller passes an effective model (proxy or
// resolved target like "openrouter/anthropic/claude-opus-4.7") instead of
// a stored alias slug, find the alias whose resolve target matches so we
// still render a friendly display name.
modelAliases.find((a) => a.resolve === slug || a.openRouterResolve === slug);
if (!alias) return `\`${slug}\``;
return alias.isFree ? `\`${alias.displayName}\` (free)` : `\`${alias.displayName}\``;
}
-2
View File
@@ -14,7 +14,6 @@ export type WorkflowRunArtifactPatchKey =
| "issueNodeId"
| "reviewNodeId"
| "planCommentNodeId"
| "summaryCommentNodeId"
| "summarySnapshot";
/**
@@ -38,7 +37,6 @@ const STRING_KEYS: WorkflowRunArtifactPatchKey[] = [
"issueNodeId",
"reviewNodeId",
"planCommentNodeId",
"summaryCommentNodeId",
"summarySnapshot",
];
+48 -1
View File
@@ -1,4 +1,4 @@
import { detectProviderError } from "./providerErrors.ts";
import { detectProviderError, isRouterKeylimitExhaustedError } from "./providerErrors.ts";
describe("detectProviderError", () => {
describe("false positives previously seen in production", () => {
@@ -78,3 +78,50 @@ describe("detectProviderError", () => {
});
});
});
describe("isRouterKeylimitExhaustedError", () => {
it("matches the canonical OpenRouter mid-run error", () => {
expect(
isRouterKeylimitExhaustedError(
"APIError: This request requires more credits, or fewer max_tokens. " +
"You requested up to 32000 tokens, but can only afford 22800. " +
"To increase, visit https://openrouter.ai/settings/keys and create a key with a higher total limit"
)
).toBe(true);
});
it("matches the 'requires more credits' phrasing on its own", () => {
expect(
isRouterKeylimitExhaustedError("This request requires more credits, or fewer max_tokens.")
).toBe(true);
});
it("matches the 'requested up to ... can only afford' phrasing on its own", () => {
expect(
isRouterKeylimitExhaustedError("You requested up to 8000 tokens but can only afford 1234")
).toBe(true);
});
it("does not match generic out-of-credit text", () => {
expect(isRouterKeylimitExhaustedError("Your account has insufficient credits")).toBe(false);
expect(isRouterKeylimitExhaustedError("rate_limit_exceeded")).toBe(false);
expect(isRouterKeylimitExhaustedError('{"limit": 0}')).toBe(false);
});
it("does not match unrelated mentions of max_tokens", () => {
expect(isRouterKeylimitExhaustedError("max_tokens parameter must be a positive integer")).toBe(
false
);
});
it("matches across newlines (defends against upstream wrapping/reformatting)", () => {
expect(
isRouterKeylimitExhaustedError(
"APIError: This request requires more credits, or\nfewer max_tokens. You requested up to 32000 tokens"
)
).toBe(true);
expect(
isRouterKeylimitExhaustedError("You requested up to 32000 tokens,\nbut can only afford 22800")
).toBe(true);
});
});
+26
View File
@@ -36,3 +36,29 @@ export function detectProviderError(text: string): string | null {
}
return null;
}
/**
* 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
* because it's a Pullfrog billing concern, not an upstream outage — the user's
* Router wallet ran out (or the key budget was undersized at mint time and the
* agent ran out of headroom partway through).
*
* Match must be specific to this exact OpenRouter error class. Generic "credits"
* or "limit" text shows up in unrelated errors and would mis-classify them.
*
* Sample:
* `APIError: This request requires more credits, or fewer max_tokens.
* You requested up to 32000 tokens, but can only afford 22800.`
*/
// `/s` (dotAll) lets `.*?` cross newlines so we still detect the error if any
// upstream layer reformats the message onto multiple lines. Without it, a
// single inserted `\n` would silently bypass the BillingError reclassification
// and the user would see the generic `❌ Pullfrog failed` dump instead of the
// actionable top-up CTA.
const ROUTER_KEYLIMIT_EXHAUSTED_PATTERN =
/requires more credits.*?fewer max_tokens|requested up to \d+ tokens.*?can only afford/is;
export function isRouterKeylimitExhaustedError(text: string): boolean {
return ROUTER_KEYLIMIT_EXHAUSTED_PATTERN.test(text);
}
+14 -3
View File
@@ -4,6 +4,12 @@ import { log } from "./cli.ts";
export type RetryOptions = {
maxAttempts?: number;
delayMs?: number;
/**
* explicit delay schedule — one entry per retry (length N ⇒ N+1 attempts).
* when set, overrides `maxAttempts` and `delayMs`. e.g. `[1_000, 3_000]`
* means up to 3 attempts, sleeping 1s before retry 2 and 3s before retry 3.
*/
delaysMs?: readonly number[];
shouldRetry?: (error: unknown) => boolean;
label?: string;
};
@@ -20,10 +26,15 @@ const defaultShouldRetry = (error: unknown): boolean => {
};
export async function retry<T>(fn: () => Promise<T>, options: RetryOptions = {}): Promise<T> {
const maxAttempts = options.maxAttempts ?? 3;
const delayMs = options.delayMs ?? 1000;
const shouldRetry = options.shouldRetry ?? defaultShouldRetry;
const label = options.label ?? "operation";
const delays = options.delaysMs
? Array.from(options.delaysMs)
: Array.from(
{ length: (options.maxAttempts ?? 3) - 1 },
(_, i) => (options.delayMs ?? 1000) * (i + 1)
);
const maxAttempts = delays.length + 1;
let lastError: unknown;
@@ -37,7 +48,7 @@ export async function retry<T>(fn: () => Promise<T>, options: RetryOptions = {})
throw error;
}
const delay = delayMs * attempt;
const delay = delays[attempt - 1]!;
log.info(`» ${label} failed (attempt ${attempt}/${maxAttempts}), retrying in ${delay}ms...`);
await sleep(delay);
}
+74
View File
@@ -1,3 +1,4 @@
import { performance } from "node:perf_hooks";
import { describe, expect, it } from "vitest";
import { spawn } from "./subprocess.ts";
@@ -48,6 +49,79 @@ describe("spawn error path", () => {
expect(afterHandles).toBeLessThanOrEqual(beforeHandles);
});
it("killGroup: true propagates SIGKILL to grandchildren so close fires promptly", async () => {
// regression: node_modules/opencode-ai/bin/opencode is a Node shim that
// spawnSyncs the native binary with stdio:"inherit". without killGroup,
// child.kill("SIGKILL") hit only the shim — the native binary was
// reparented to PID 1, kept holding our stdout pipe via the inherited
// fds, and `child.on("close")` never fired (because pipes stayed open).
// a 5-min outer safety-net timer eventually rejected the agent promise,
// but the grandchild kept running until the GitHub Actions job-level
// timeout. this test replicates the shape with bash + a backgrounded
// sleep grandchild: with killGroup, close fires promptly after SIGKILL;
// without it, the parent would wait for sleep to exit (30s).
//
// the activity-check interval is fixed at 5s so the earliest the kill
// can fire is ~5s after start. budget 15s end-to-end.
const before = performance.now();
const result = await spawn({
cmd: "bash",
args: ["-c", "sleep 30 & wait"],
env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
activityTimeout: 1000,
killGroup: true,
}).catch((err) => err);
const elapsed = performance.now() - before;
expect(result).toBeInstanceOf(Error);
// 10s ceiling: 5s activity-check tick + signal delivery. a regression
// here (no killGroup) would hang for the full 30s sleep.
expect(elapsed).toBeLessThan(10_000);
}, 20_000);
it("isPausedExternally suspends the activity timer while it returns true", async () => {
// opencode's `task` tool encapsulates subagent execution in-process —
// subagent-internal events don't reach our parent NDJSON stream. this
// means the child's stdout falls silent for the full subagent duration
// even when real work is happening. opencode passes a predicate keyed
// off its in-flight task dispatch tracker; while it returns true, the
// activity timer must skip the kill decision and reset its baseline.
const result = await spawn({
cmd: "bash",
args: ["-c", "sleep 8; echo done"],
env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
activityTimeout: 1000,
isPausedExternally: () => true,
});
expect(result.exitCode).toBe(0);
expect(result.stdout).toContain("done");
}, 20_000);
it("isPausedExternally fires normally once it returns false again", async () => {
// verifies suspend/resume rather than unconditional bypass: the predicate
// flips to false after a few hundred ms, so the timer should resume from
// a fresh baseline (advanced during the paused window) and fire after
// activityTimeout idle time. critical that lastActivityTime is reset
// while paused — otherwise the resume tick would see a stale baseline
// and kill instantly.
let paused = true;
setTimeout(() => {
paused = false;
}, 500);
const result = await spawn({
cmd: "sleep",
args: ["30"],
env: { PATH: process.env.PATH ?? "", HOME: process.env.HOME ?? "" },
activityTimeout: 1000,
isPausedExternally: () => paused,
}).catch((err) => err);
expect(result).toBeInstanceOf(Error);
expect(String(result)).toMatch(/activity timeout/i);
}, 20_000);
it("reports signal-killed subprocesses as failures, not success", async () => {
// regression: before the fix, `child.on("close", (exitCode) => ...)`
// discarded the signal parameter and `exitCode || 0` coerced the
+54 -5
View File
@@ -106,6 +106,25 @@ export interface SpawnOptions {
stdio?: ("pipe" | "ignore" | "inherit")[];
onStdout?: (chunk: string) => void;
onStderr?: (chunk: string) => void;
// when true, spawn the child detached (its own process group) and route all
// kill paths (timeout, activity timeout, ctrl-c) through `process.kill(-pid, ...)`
// so signals reach grandchildren too. critical for binaries that fork through
// a shim (e.g. node_modules/opencode-ai/bin/opencode is a Node shim that
// spawnSync's the native binary; without killGroup, SIGKILL only hits the
// shim and the native binary is reparented to PID 1, holds our stdout pipe
// open, keeps emitting NDJSON, and `child.on("close")` never fires —
// producing zombie runs that hang until the GitHub Actions job timeout).
killGroup?: boolean;
// optional pause predicate consulted on every activity check. when it
// returns true the activity timer skips the kill decision *and* resets
// its idle baseline so a clean unpause can't immediately fire on a stale
// lastActivityTime. opencode uses this because its `task` tool encapsulates
// subagent execution in-process — subagent-internal events don't surface
// on the parent's NDJSON stream, so the local stdout-only signal would
// falsely fire mid-subagent. preferred over fake-activity heartbeats
// because there's no race window between a heartbeat tick and a subagent
// that finishes between ticks.
isPausedExternally?: () => boolean;
}
export interface SpawnResult {
@@ -127,6 +146,8 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
let stdoutBuffer = "";
let stderrBuffer = "";
const killGroup = options.killGroup ?? false;
return new Promise((resolve, reject) => {
// security: caller must provide complete env object, not merged with process.env
const child = nodeSpawn(options.cmd, options.args, {
@@ -136,10 +157,28 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
},
stdio: options.stdio || ["pipe", "pipe", "pipe"],
cwd: options.cwd || process.cwd(),
detached: killGroup,
});
// sends `signal` to the entire process group when killGroup is set, so
// grandchildren (e.g. the native opencode binary spawned by the
// opencode-ai Node shim) die with the parent. falls back to a direct
// child kill if the process-group send fails (common when the child
// already exited or was never made a process group leader).
const killSelf = (signal: NodeJS.Signals): void => {
if (killGroup && child.pid) {
try {
process.kill(-child.pid, signal);
return;
} catch {
// fall through to direct kill
}
}
child.kill(signal);
};
// track child for cleanup on Ctrl+C
trackChild({ child });
trackChild({ child, killGroup });
let timeoutId: NodeJS.Timeout | undefined;
let sigkillEscalatorId: NodeJS.Timeout | undefined;
@@ -157,7 +196,7 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
if (options.timeout) {
timeoutId = setTimeout(() => {
isTimedOut = true;
child.kill("SIGTERM");
killSelf("SIGTERM");
// track the escalator so a graceful SIGTERM response (close fires
// before the 5s elapses) can clear it. without capture, this timer
@@ -165,7 +204,7 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
// past a timed-out subprocess's clean exit.
sigkillEscalatorId = setTimeout(() => {
if (!child.killed) {
child.kill("SIGKILL");
killSelf("SIGKILL");
}
}, 5000);
}, options.timeout);
@@ -177,6 +216,16 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
`spawn activity timer: pid=${child.pid} cmd=${options.cmd} timeout=${activityTimeoutMs}ms`
);
activityCheckIntervalId = setInterval(() => {
// when an external pause predicate says we're suspended (e.g.
// opencode in a long-running task subagent whose internal events
// don't surface on stdout), advance lastActivityTime to "now" so a
// clean unpause doesn't immediately fire on a stale baseline, and
// skip the kill decision for this tick.
if (options.isPausedExternally?.()) {
lastActivityTime = performance.now();
log.debug(`spawn activity check: pid=${child.pid} paused externally`);
return;
}
const idleMs = performance.now() - lastActivityTime;
log.debug(
`spawn activity check: pid=${child.pid} idle=${Math.round(idleMs)}ms / ${activityTimeoutMs}ms`
@@ -186,9 +235,9 @@ export async function spawn(options: SpawnOptions): Promise<SpawnResult> {
killedAtIdleMs = idleMs;
const idleSec = Math.round(idleMs / 1000);
log.info(
`no output for ${idleSec}s from pid=${child.pid} (${options.cmd}), killing process`
`no output for ${idleSec}s from pid=${child.pid} (${options.cmd}), killing process${killGroup ? " group" : ""}`
);
child.kill("SIGKILL");
killSelf("SIGKILL");
clearInterval(activityCheckIntervalId);
try {
options.onActivityTimeout?.();