Compare commits
5 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 3440292abb | |||
| 585a5d21cc | |||
| fe2746198c | |||
| dc4dff98da | |||
| a0746dcc27 |
@@ -0,0 +1,96 @@
|
||||
import { readFileSync } from "node:fs";
|
||||
import { dirname, resolve } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
|
||||
// The GHA `post:` hook runs `node action/entryPost.ts` directly against the
|
||||
// rsynced action checkout, which deliberately excludes `node_modules`. Any
|
||||
// non-relative / non-`node:` import in entryPost.ts (or in its transitive
|
||||
// imports) crashes the post-step with `ERR_MODULE_NOT_FOUND` AFTER the agent
|
||||
// already exited 0, flipping the workflow to `failure`. see #834.
|
||||
//
|
||||
// This test parses the static-import graph rooted at entryPost.ts and refuses
|
||||
// any specifier that isn't one of:
|
||||
// - node:* (stdlib)
|
||||
// - ./* or ../* (relative)
|
||||
//
|
||||
// Any other specifier (`@actions/core`, `pullfrog`, `zod`, etc.) means the
|
||||
// post-hook will need a `node_modules` tree the rsync drops.
|
||||
|
||||
const ENTRY_FILE = resolve(import.meta.dirname, "entryPost.ts");
|
||||
|
||||
const IMPORT_RE = /^\s*(?:import|export)(?:\s+(?:type\s+)?[\s\S]*?)?\s+from\s+["']([^"']+)["']/gm;
|
||||
const SIDE_EFFECT_RE = /^\s*import\s+["']([^"']+)["']/gm;
|
||||
// `import.meta.glob` and friends are not used in entryPost.ts; the simple
|
||||
// regex above is sufficient here. expand if a transitive dep starts using
|
||||
// dynamic imports for stdlib-only logic.
|
||||
|
||||
function extractImports(filePath: string): string[] {
|
||||
const source = readFileSync(filePath, "utf8");
|
||||
const specs: string[] = [];
|
||||
for (const re of [IMPORT_RE, SIDE_EFFECT_RE]) {
|
||||
re.lastIndex = 0;
|
||||
for (const m of source.matchAll(re)) specs.push(m[1]);
|
||||
}
|
||||
return specs;
|
||||
}
|
||||
|
||||
function isAllowed(spec: string): boolean {
|
||||
return spec.startsWith("node:") || spec.startsWith("./") || spec.startsWith("../");
|
||||
}
|
||||
|
||||
type WalkResult = {
|
||||
visited: Set<string>;
|
||||
violations: { file: string; spec: string }[];
|
||||
};
|
||||
|
||||
function walk(start: string): WalkResult {
|
||||
const visited = new Set<string>();
|
||||
const violations: WalkResult["violations"] = [];
|
||||
const queue: string[] = [start];
|
||||
|
||||
while (queue.length > 0) {
|
||||
const file = queue.shift()!;
|
||||
if (visited.has(file)) continue;
|
||||
visited.add(file);
|
||||
|
||||
for (const spec of extractImports(file)) {
|
||||
if (!isAllowed(spec)) {
|
||||
violations.push({ file, spec });
|
||||
continue;
|
||||
}
|
||||
if (spec.startsWith("node:")) continue;
|
||||
const resolved = resolve(dirname(file), spec);
|
||||
const candidate = resolved.endsWith(".ts") ? resolved : `${resolved}.ts`;
|
||||
try {
|
||||
readFileSync(candidate, "utf8");
|
||||
queue.push(candidate);
|
||||
} catch {
|
||||
// non-.ts (e.g. JSON `with { type: "json" }`) — already classified
|
||||
// as relative-allowed above. nothing further to walk.
|
||||
}
|
||||
}
|
||||
}
|
||||
return { visited, violations };
|
||||
}
|
||||
|
||||
describe("entryPost.ts stdlib-only invariant (#834)", () => {
|
||||
it("only imports node: builtins and relative siblings (no node_modules deps)", () => {
|
||||
const result = walk(ENTRY_FILE);
|
||||
expect(result.violations, JSON.stringify(result.violations, null, 2)).toEqual([]);
|
||||
});
|
||||
|
||||
it("walks the full transitive graph (entryPost + 3 utils)", () => {
|
||||
const result = walk(ENTRY_FILE);
|
||||
expect(result.visited.size).toBeGreaterThanOrEqual(4);
|
||||
});
|
||||
|
||||
it("matches the modules entryPost actually imports today", () => {
|
||||
const direct = extractImports(ENTRY_FILE).sort();
|
||||
expect(direct).toEqual([
|
||||
"./utils/codexRefreshDetect.ts",
|
||||
"./utils/ghaCore.ts",
|
||||
"./utils/postApiFetch.ts",
|
||||
"node:fs",
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -197,6 +197,12 @@ const TRANSIENT_PATTERNS: RegExp[] = [
|
||||
/returned error: 5\d\d/i,
|
||||
/HTTP 429/,
|
||||
/returned error: 429/i,
|
||||
// github installation tokens can 401 for seconds after minting while
|
||||
// replicating (@octokit/auth-app retries the same class). git push
|
||||
// surfaces it as "Invalid username or token", distinct from 403
|
||||
// permission denied — safe to backoff-retry with the same token.
|
||||
/Invalid username or token/,
|
||||
/Authentication failed for 'https:\/\/github\.com\//,
|
||||
];
|
||||
|
||||
export function classifyPushError(msg: string): PushErrorKind {
|
||||
|
||||
+1
-310
@@ -1,10 +1,7 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
buildCommentableMap,
|
||||
type CommentableLines,
|
||||
clearStrandedPendingReview,
|
||||
commentableLinesForFile,
|
||||
createReviewWithStrandedRecovery,
|
||||
type DroppedComment,
|
||||
duplicateReviewDecision,
|
||||
formatDroppedCommentsNote,
|
||||
@@ -13,7 +10,6 @@ import {
|
||||
reviewSkipDecision,
|
||||
validateInlineComments,
|
||||
} from "./review.ts";
|
||||
import type { ToolContext } from "./server.ts";
|
||||
|
||||
describe("commentableLinesForFile", () => {
|
||||
it("returns empty sets for missing patches (binary or no changes)", () => {
|
||||
@@ -163,95 +159,6 @@ describe("validateInlineComments", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("buildCommentableMap", () => {
|
||||
it("returns the cached snapshot when toolState matches PR and checkoutSha", async () => {
|
||||
// simulates checkout_pr having pre-populated the cache. the cache pins the
|
||||
// commentable lines to checkoutSha so review-time validation matches what
|
||||
// GitHub anchors to, even if the PR is updated mid-run.
|
||||
const cached = buildMap([["src/foo.ts", "@@ -1,1 +1,2 @@\n ctx\n+new"]]);
|
||||
const paginate = vi.fn();
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listFiles: {} } } },
|
||||
repo: { owner: "o", name: "r" },
|
||||
toolState: {
|
||||
commentableLinesByFile: cached,
|
||||
commentableLinesPullNumber: 42,
|
||||
commentableLinesCheckoutSha: "sha1",
|
||||
checkoutSha: "sha1",
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
|
||||
const result = await buildCommentableMap(ctx, 42);
|
||||
|
||||
expect(result).toBe(cached);
|
||||
expect(paginate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("ignores the cached snapshot when it was built for a different PR", async () => {
|
||||
// without this guard, checkout_pr(B) followed by review(A) would validate
|
||||
// A's inline comments against B's diff — silently dropping valid anchors.
|
||||
const cached = buildMap([["src/foo.ts", "@@ -1,1 +1,2 @@\n ctx\n+new"]]);
|
||||
const freshFile = { filename: "src/bar.ts", patch: "@@ -1,1 +1,2 @@\n ctx\n+added" };
|
||||
const paginate = vi.fn().mockResolvedValue([freshFile]);
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listFiles: {} } } },
|
||||
repo: { owner: "o", name: "r" },
|
||||
toolState: {
|
||||
commentableLinesByFile: cached,
|
||||
commentableLinesPullNumber: 99,
|
||||
commentableLinesCheckoutSha: "sha1",
|
||||
checkoutSha: "sha1",
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
|
||||
const result = await buildCommentableMap(ctx, 42);
|
||||
|
||||
expect(paginate).toHaveBeenCalledTimes(1);
|
||||
expect(result).not.toBe(cached);
|
||||
expect(result.get("src/bar.ts")?.RIGHT.has(2)).toBe(true);
|
||||
});
|
||||
|
||||
it("ignores the cached snapshot when checkoutSha has moved since it was built", async () => {
|
||||
// simulates a second checkout_pr(42) that bumped checkoutSha but failed
|
||||
// before repopulating the cache (e.g., listFiles rate-limited). without
|
||||
// the sha guard, review would reuse the stale snapshot against the new
|
||||
// anchor and either drop valid comments or let invalid ones through.
|
||||
const cached = buildMap([["src/foo.ts", "@@ -1,1 +1,2 @@\n ctx\n+new"]]);
|
||||
const freshFile = { filename: "src/bar.ts", patch: "@@ -1,1 +1,2 @@\n ctx\n+added" };
|
||||
const paginate = vi.fn().mockResolvedValue([freshFile]);
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listFiles: {} } } },
|
||||
repo: { owner: "o", name: "r" },
|
||||
toolState: {
|
||||
commentableLinesByFile: cached,
|
||||
commentableLinesPullNumber: 42,
|
||||
commentableLinesCheckoutSha: "sha-old",
|
||||
checkoutSha: "sha-new",
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
|
||||
const result = await buildCommentableMap(ctx, 42);
|
||||
|
||||
expect(paginate).toHaveBeenCalledTimes(1);
|
||||
expect(result).not.toBe(cached);
|
||||
});
|
||||
|
||||
it("falls back to listFiles when no cache exists", async () => {
|
||||
const file = { filename: "src/bar.ts", patch: "@@ -1,1 +1,2 @@\n ctx\n+added" };
|
||||
const paginate = vi.fn().mockResolvedValue([file]);
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listFiles: {} } } },
|
||||
repo: { owner: "o", name: "r" },
|
||||
toolState: {},
|
||||
} as unknown as ToolContext;
|
||||
|
||||
const result = await buildCommentableMap(ctx, 42);
|
||||
|
||||
expect(paginate).toHaveBeenCalledTimes(1);
|
||||
expect(result.get("src/bar.ts")?.RIGHT.has(2)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("formatDroppedCommentsNote", () => {
|
||||
it("renders single-line dropped entries with `path:line`", () => {
|
||||
const dropped: DroppedComment[] = [
|
||||
@@ -321,222 +228,6 @@ describe("formatDroppedCommentsNote", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("clearStrandedPendingReview", () => {
|
||||
function pendingReviewError(status: number, message: string): Error {
|
||||
const err = new Error(message) as Error & { status: number };
|
||||
err.status = status;
|
||||
return err;
|
||||
}
|
||||
|
||||
const baseParams = { owner: "o", repo: "r", pull_number: 42 };
|
||||
|
||||
it("rethrows the original error when status is not 422", async () => {
|
||||
const err = pendingReviewError(500, "server exploded");
|
||||
const ctx = {
|
||||
octokit: {
|
||||
paginate: vi.fn(),
|
||||
rest: { pulls: { listReviews: {}, deletePendingReview: vi.fn() } },
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
await expect(clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })).rejects.toBe(
|
||||
err
|
||||
);
|
||||
expect(ctx.octokit.paginate).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rethrows the original error when 422 does not mention pending review", async () => {
|
||||
// a 422 from an unrelated validation (e.g., invalid anchor) must not
|
||||
// trigger a destructive delete of the user's own draft.
|
||||
const err = pendingReviewError(422, "pull_request_review_thread is not part of the diff");
|
||||
const deletePendingReview = vi.fn();
|
||||
const ctx = {
|
||||
octokit: {
|
||||
paginate: vi.fn(),
|
||||
rest: { pulls: { listReviews: {}, deletePendingReview } },
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
await expect(clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })).rejects.toBe(
|
||||
err
|
||||
);
|
||||
expect(ctx.octokit.paginate).not.toHaveBeenCalled();
|
||||
expect(deletePendingReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rethrows the original error when no PENDING review is found", async () => {
|
||||
// 422 claimed a pending exists but listReviews returns only SUBMITTED —
|
||||
// likely a transient GitHub inconsistency. retry won't help; surface the
|
||||
// original error so the caller sees why createReview failed.
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockResolvedValue([{ id: 1, state: "COMMENTED" } as unknown as never]);
|
||||
const deletePendingReview = vi.fn();
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })).rejects.toBe(
|
||||
err
|
||||
);
|
||||
expect(paginate).toHaveBeenCalledTimes(1);
|
||||
expect(deletePendingReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("deletes the leftover PENDING review and resolves on success", async () => {
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockResolvedValue([
|
||||
{ id: 100, state: "COMMENTED" },
|
||||
{ id: 101, state: "PENDING" },
|
||||
] as unknown as never);
|
||||
const deletePendingReview = vi.fn().mockResolvedValue({ status: 204 });
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(
|
||||
clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })
|
||||
).resolves.toBeUndefined();
|
||||
expect(deletePendingReview).toHaveBeenCalledWith({
|
||||
owner: "o",
|
||||
repo: "r",
|
||||
pull_number: 42,
|
||||
review_id: 101,
|
||||
});
|
||||
});
|
||||
|
||||
it("swallows a 404 from deletePendingReview (raced with another cleanup)", async () => {
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockResolvedValue([{ id: 101, state: "PENDING" }] as unknown as never);
|
||||
const deletePendingReview = vi.fn().mockRejectedValue(pendingReviewError(404, "not found"));
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(
|
||||
clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("swallows a 422 from deletePendingReview (draft submitted by a concurrent caller)", async () => {
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockResolvedValue([{ id: 101, state: "PENDING" }] as unknown as never);
|
||||
const deletePendingReview = vi
|
||||
.fn()
|
||||
.mockRejectedValue(pendingReviewError(422, "review has already been submitted"));
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(
|
||||
clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("rethrows the ORIGINAL 422 when listReviews fails so the real blocker isn't masked", async () => {
|
||||
// if listReviews throws a transient 502 during cleanup, we must surface
|
||||
// the pending-review 422 — not the 502 — so the caller sees the actual
|
||||
// reason createReview failed and can retry the cleanup. masking the 422
|
||||
// with a 502 previously sent agents chasing phantom server errors.
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockRejectedValue(pendingReviewError(502, "bad gateway"));
|
||||
const deletePendingReview = vi.fn();
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })).rejects.toBe(
|
||||
err
|
||||
);
|
||||
expect(deletePendingReview).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rethrows non-404/422 errors from deletePendingReview so the real cause surfaces", async () => {
|
||||
const err = pendingReviewError(422, "User already has a pending review for this pull request");
|
||||
const paginate = vi.fn().mockResolvedValue([{ id: 101, state: "PENDING" }] as unknown as never);
|
||||
const cleanupErr = pendingReviewError(500, "internal server error");
|
||||
const deletePendingReview = vi.fn().mockRejectedValue(cleanupErr);
|
||||
const ctx = {
|
||||
octokit: { paginate, rest: { pulls: { listReviews: {}, deletePendingReview } } },
|
||||
} as unknown as ToolContext;
|
||||
await expect(clearStrandedPendingReview(ctx, { ...baseParams, originalErr: err })).rejects.toBe(
|
||||
cleanupErr
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("createReviewWithStrandedRecovery", () => {
|
||||
function pendingReviewError(status: number, message: string): Error {
|
||||
const err = new Error(message) as Error & { status: number };
|
||||
err.status = status;
|
||||
return err;
|
||||
}
|
||||
|
||||
const params = {
|
||||
owner: "o",
|
||||
repo: "r",
|
||||
pull_number: 42,
|
||||
event: "COMMENT" as const,
|
||||
};
|
||||
|
||||
it("returns createReview result directly when no stranded draft exists", async () => {
|
||||
const response = { data: { id: 1, node_id: "n1" } };
|
||||
const createReview = vi.fn().mockResolvedValue(response);
|
||||
const ctx = {
|
||||
octokit: {
|
||||
paginate: vi.fn(),
|
||||
rest: { pulls: { createReview, listReviews: {}, deletePendingReview: vi.fn() } },
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
await expect(createReviewWithStrandedRecovery(ctx, params)).resolves.toBe(response);
|
||||
expect(createReview).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("clears a stranded PENDING draft and retries on pending-review 422 — covers the no-body path", async () => {
|
||||
// regression: the no-body review path (approve-with-no-feedback,
|
||||
// comments-only) used to call createReview directly. a prior body-path run
|
||||
// that crashed between createReview(PENDING) and submitReview would leave
|
||||
// a stranded PENDING draft; every subsequent no-body review would 422
|
||||
// with "already has a pending review" until a body-path run happened to
|
||||
// clear it. this test exercises the recovery: first createReview 422s,
|
||||
// clearStranded deletes the leftover, and the retry succeeds.
|
||||
const stranded = pendingReviewError(
|
||||
422,
|
||||
"User already has a pending review for this pull request"
|
||||
);
|
||||
const response = { data: { id: 2, node_id: "n2" } };
|
||||
const createReview = vi.fn().mockRejectedValueOnce(stranded).mockResolvedValueOnce(response);
|
||||
const paginate = vi.fn().mockResolvedValue([{ id: 77, state: "PENDING" }] as unknown as never);
|
||||
const deletePendingReview = vi.fn().mockResolvedValue({ status: 204 });
|
||||
const ctx = {
|
||||
octokit: {
|
||||
paginate,
|
||||
rest: { pulls: { createReview, listReviews: {}, deletePendingReview } },
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
await expect(createReviewWithStrandedRecovery(ctx, params)).resolves.toBe(response);
|
||||
expect(createReview).toHaveBeenCalledTimes(2);
|
||||
expect(deletePendingReview).toHaveBeenCalledWith({
|
||||
owner: "o",
|
||||
repo: "r",
|
||||
pull_number: 42,
|
||||
review_id: 77,
|
||||
});
|
||||
});
|
||||
|
||||
it("rethrows non-pending 422s without retrying — avoids masking a real validation error", async () => {
|
||||
// if the 422 is unrelated to a stranded draft (e.g. body too long, bad
|
||||
// anchor), clearStrandedPendingReview rethrows and we must not retry
|
||||
// blindly — a retry would just hit the same validation and double the
|
||||
// GitHub API traffic for nothing.
|
||||
const err = pendingReviewError(422, "body is too long");
|
||||
const createReview = vi.fn().mockRejectedValue(err);
|
||||
const paginate = vi.fn();
|
||||
const deletePendingReview = vi.fn();
|
||||
const ctx = {
|
||||
octokit: {
|
||||
paginate,
|
||||
rest: { pulls: { createReview, listReviews: {}, deletePendingReview } },
|
||||
},
|
||||
} as unknown as ToolContext;
|
||||
await expect(createReviewWithStrandedRecovery(ctx, params)).rejects.toBe(err);
|
||||
expect(createReview).toHaveBeenCalledTimes(1);
|
||||
expect(deletePendingReview).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("reviewSkipDecision", () => {
|
||||
// GitHub 422s `event: "COMMENT"` reviews with no body + no comments
|
||||
// ("{\"message\":\"Unprocessable Entity\",\"errors\":[\"\"]}"). verified
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "pullfrog",
|
||||
"version": "0.1.12",
|
||||
"version": "0.1.14",
|
||||
"type": "module",
|
||||
"bin": {
|
||||
"pullfrog": "dist/cli.mjs",
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { execFileSync } from "node:child_process";
|
||||
import { accessSync, constants, existsSync } from "node:fs";
|
||||
import { accessSync, constants, existsSync, mkdtempSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { delimiter, dirname, isAbsolute, join, resolve, sep } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import actionPackageJson from "./package.json" with { type: "json" };
|
||||
@@ -125,9 +126,22 @@ function createRuntimeContext(): RuntimeContext {
|
||||
};
|
||||
}
|
||||
|
||||
// $GITHUB_WORKSPACE is the customer's repo. running `npx --yes pullfrog@…`
|
||||
// there makes npm read THEIR `package.json` first, which on npm v11+ enforces
|
||||
// `devEngines.packageManager` and aborts the bootstrap with EBADDEVENGINES
|
||||
// before the agent ever boots. our bootstrap doesn't need anything from the
|
||||
// customer's tree — a freshly-created tmpdir is package.json-free and
|
||||
// parent-less, so npm walks up to `/` finding nothing. see #837.
|
||||
//
|
||||
// `mkdtempSync` (vs raw `tmpdir()`): `$TMPDIR` is overridable from a prior
|
||||
// `$GITHUB_ENV` step, and a customer-authored or compromised prior step
|
||||
// could plant `node_modules/pullfrog/` in the resolved tmpdir to hijack
|
||||
// `npx --yes pullfrog@<version>` resolution. a fresh per-invocation
|
||||
// subdirectory is mode 0700 and not pre-writable by anything earlier in
|
||||
// the job.
|
||||
function runCommand(params: { context: RuntimeContext; command: string; args: string[] }): void {
|
||||
execFileSync(params.command, params.args, {
|
||||
cwd: process.env.GITHUB_WORKSPACE || params.context.actionRoot,
|
||||
cwd: mkdtempSync(join(tmpdir(), "pullfrog-bootstrap-")),
|
||||
stdio: "inherit",
|
||||
env: params.context.env,
|
||||
});
|
||||
|
||||
+19
-9
@@ -1,9 +1,14 @@
|
||||
/**
|
||||
* git authentication via GIT_ASKPASS.
|
||||
*
|
||||
* a localhost HTTP server serves tokens via single-use UUID codes.
|
||||
* each $git() call writes a unique askpass script with the server
|
||||
* port+code baked into the file body — no secrets in subprocess env.
|
||||
* a localhost HTTP server serves tokens via UUID codes whose lifetime is
|
||||
* bounded by the parent $git() invocation: register() makes the code active,
|
||||
* the script (and any sibling subprocess — e.g. git-lfs pre-push) can fetch
|
||||
* the token any number of times, and $git()'s finally calls revoke() to
|
||||
* close the window. each $git() call writes a unique askpass script with
|
||||
* the server port+code baked into the file body — no secrets in subprocess
|
||||
* env. a replay of a revoked code trips a 409 and revokes the underlying
|
||||
* github installation token.
|
||||
*
|
||||
* see wiki/askpass.md for full security documentation.
|
||||
*/
|
||||
@@ -88,9 +93,13 @@ export function setGitAuthServer(server: GitAuthServer): void {
|
||||
* a remote and need credentials. working-tree operations (checkout, merge)
|
||||
* use $() from shell.ts which has no token.
|
||||
*
|
||||
* per call: registers a one-time code with the auth server, writes a
|
||||
* unique askpass script with port+code baked in, spawns git with
|
||||
* GIT_ASKPASS pointing to the script, and deletes the script in finally.
|
||||
* per call: registers a code with the auth server (valid for the lifetime
|
||||
* of this invocation), writes a unique askpass script with port+code baked
|
||||
* in, spawns git with GIT_ASKPASS pointing to the script. on completion,
|
||||
* revokes the code and deletes the script in finally. multiple sibling
|
||||
* askpass calls within one invocation (e.g. git itself + git-lfs pre-push)
|
||||
* all see a valid code; replay attempts after finally trip a 409 and the
|
||||
* server revokes the underlying github token as a tamper signal.
|
||||
*
|
||||
* @example
|
||||
* await $git("fetch", ["origin", "main"], { token });
|
||||
@@ -149,8 +158,8 @@ export async function $git(
|
||||
});
|
||||
|
||||
if (result.stderr.includes("askpass-compromised")) {
|
||||
log.info("askpass code was already consumed — token has been revoked");
|
||||
throw new Error("git auth failed — askpass code was already consumed, token revoked");
|
||||
log.info("askpass code was replayed after revoke — token has been revoked");
|
||||
throw new Error("git auth failed — askpass code was replayed after revoke, token revoked");
|
||||
}
|
||||
|
||||
if (result.exitCode !== 0) {
|
||||
@@ -175,10 +184,11 @@ export async function $git(
|
||||
stderr: result.stderr.trim(),
|
||||
};
|
||||
} finally {
|
||||
authServer.revoke(code);
|
||||
try {
|
||||
unlinkSync(scriptPath);
|
||||
} catch {
|
||||
// script may have self-deleted already
|
||||
// script may already be gone (e.g. tmpdir cleanup raced us)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -75,8 +75,23 @@ describe("token delivery", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("single-use enforcement (tamper detection)", () => {
|
||||
it("returns 409 on second use of same code", async () => {
|
||||
describe("code lifecycle (tamper detection)", () => {
|
||||
it("returns the token on repeated use while the code is active", async () => {
|
||||
// a single $git() call can produce multiple legitimate askpass requests:
|
||||
// git itself (username + password), git-lfs pre-push hook, custom hooks.
|
||||
// they must all succeed until $git()'s finally calls revoke().
|
||||
const tmp = makeTmpdir();
|
||||
server = await startGitAuthServer(tmp);
|
||||
const code = server.register("ghs_active_test");
|
||||
|
||||
for (let i = 0; i < 5; i++) {
|
||||
const res = await fetch(`http://127.0.0.1:${server.port}/${code}`);
|
||||
expect(res.status).toBe(200);
|
||||
expect(await res.text()).toBe("ghs_active_test");
|
||||
}
|
||||
});
|
||||
|
||||
it("returns 409 after revoke (replay-after-call trap)", async () => {
|
||||
const tmp = makeTmpdir();
|
||||
server = await startGitAuthServer(tmp);
|
||||
const code = server.register("ghs_tamper_test");
|
||||
@@ -84,10 +99,17 @@ describe("single-use enforcement (tamper detection)", () => {
|
||||
const first = await fetch(`http://127.0.0.1:${server.port}/${code}`);
|
||||
expect(first.status).toBe(200);
|
||||
|
||||
const second = await fetch(`http://127.0.0.1:${server.port}/${code}`);
|
||||
expect(second.status).toBe(409);
|
||||
const body = await second.text();
|
||||
expect(body).toBe("compromised");
|
||||
server.revoke(code);
|
||||
|
||||
const replay = await fetch(`http://127.0.0.1:${server.port}/${code}`);
|
||||
expect(replay.status).toBe(409);
|
||||
expect(await replay.text()).toBe("compromised");
|
||||
});
|
||||
|
||||
it("revoke() on an unknown code is a no-op", async () => {
|
||||
const tmp = makeTmpdir();
|
||||
server = await startGitAuthServer(tmp);
|
||||
expect(() => server!.revoke("nonexistent")).not.toThrow();
|
||||
});
|
||||
|
||||
it("each register() call produces an independent code", async () => {
|
||||
|
||||
+50
-33
@@ -1,12 +1,17 @@
|
||||
/**
|
||||
* ASKPASS-based git authentication server.
|
||||
*
|
||||
* serves tokens via a localhost HTTP server with single-use UUID codes.
|
||||
* serves tokens via a localhost HTTP server with per-$git()-call UUID codes.
|
||||
* each $git() call gets a unique askpass script with the port+code baked in.
|
||||
* the token never appears in subprocess env — only the script file path.
|
||||
*
|
||||
* tamper-evident: if a code is used twice, the second request triggers
|
||||
* immediate token revocation via the GitHub API as a precaution.
|
||||
* lifetime: the code is valid for as long as the $git() invocation is
|
||||
* running. multiple askpass calls within one invocation (e.g. git's own
|
||||
* fetch/push + a git-lfs pre-push hook that also authenticates) all
|
||||
* succeed. $git() calls revoke(code) in finally; subsequent requests for
|
||||
* a revoked code trigger immediate token revocation via the GitHub API
|
||||
* as a tamper-evidence precaution (an agent replaying the code after the
|
||||
* legitimate window has closed is the realistic attack we still catch).
|
||||
*/
|
||||
|
||||
import { randomUUID } from "node:crypto";
|
||||
@@ -15,20 +20,25 @@ import { createServer } from "node:http";
|
||||
import { join } from "node:path";
|
||||
import { log } from "./cli.ts";
|
||||
|
||||
type CodeState = "pending" | "consumed";
|
||||
type CodeState = "active" | "revoked";
|
||||
|
||||
type PendingCode = {
|
||||
type CodeEntry = {
|
||||
token: string;
|
||||
state: CodeState;
|
||||
timeout: NodeJS.Timeout;
|
||||
// only present once the entry is revoked — bounds the replay-trap window.
|
||||
// active entries have no timer because $git() can take arbitrarily long
|
||||
// (large LFS pushes, slow networks, `activityTimeout: 0` on the spawn);
|
||||
// any wall-clock TTL here would re-introduce the original LFS bug at
|
||||
// a different boundary. revoke() is the only way out for an active code.
|
||||
timeout?: NodeJS.Timeout;
|
||||
};
|
||||
|
||||
const CODE_TTL_MS = 5 * 60 * 1000;
|
||||
const TAMPER_WINDOW_MS = 60_000;
|
||||
const REVOKED_TRAP_MS = 60_000;
|
||||
|
||||
export type GitAuthServer = {
|
||||
port: number;
|
||||
register: (token: string) => string;
|
||||
revoke: (code: string) => void;
|
||||
writeAskpassScript: (code: string) => string;
|
||||
close: () => Promise<void>;
|
||||
[Symbol.asyncDispose]: () => Promise<void>;
|
||||
@@ -49,7 +59,7 @@ function revokeGitHubToken(token: string): void {
|
||||
}
|
||||
|
||||
export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer> {
|
||||
const codes = new Map<string, PendingCode>();
|
||||
const codes = new Map<string, CodeEntry>();
|
||||
|
||||
const server = createServer((req, res) => {
|
||||
if (req.method !== "GET") {
|
||||
@@ -69,21 +79,20 @@ export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer>
|
||||
return;
|
||||
}
|
||||
|
||||
if (entry.state === "pending") {
|
||||
// first use — return token, keep entry for tamper detection
|
||||
entry.state = "consumed";
|
||||
clearTimeout(entry.timeout);
|
||||
entry.timeout = setTimeout(() => codes.delete(code), TAMPER_WINDOW_MS);
|
||||
entry.timeout.unref();
|
||||
if (entry.state === "active") {
|
||||
// legitimate caller (git, git-lfs, or any subprocess of the running
|
||||
// $git() call). hand back the token without consuming the code —
|
||||
// revoke() in $git's finally is what closes the window.
|
||||
res.writeHead(200, { "Content-Type": "text/plain" });
|
||||
res.end(entry.token);
|
||||
return;
|
||||
}
|
||||
|
||||
// second request for same code — revoke token as a precaution
|
||||
log.info("askpass code used twice — revoking token");
|
||||
// request for a revoked code — the $git() window has closed, so this
|
||||
// is an agent replaying the code. revoke the token as a precaution.
|
||||
log.info("askpass code used after revoke — revoking token");
|
||||
revokeGitHubToken(entry.token);
|
||||
clearTimeout(entry.timeout);
|
||||
if (entry.timeout) clearTimeout(entry.timeout);
|
||||
codes.delete(code);
|
||||
res.writeHead(409, { "Content-Type": "text/plain" });
|
||||
res.end("compromised");
|
||||
@@ -104,25 +113,34 @@ export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer>
|
||||
|
||||
function register(token: string): string {
|
||||
const code = randomUUID();
|
||||
const timeout = setTimeout(() => {
|
||||
codes.delete(code);
|
||||
log.debug(`git auth code expired: ${code.slice(0, 8)}...`);
|
||||
}, CODE_TTL_MS);
|
||||
timeout.unref();
|
||||
codes.set(code, { token, state: "pending", timeout });
|
||||
codes.set(code, { token, state: "active" });
|
||||
return code;
|
||||
}
|
||||
|
||||
function revoke(code: string): void {
|
||||
const entry = codes.get(code);
|
||||
if (!entry) return;
|
||||
entry.state = "revoked";
|
||||
// keep the entry around briefly so a replay attempt trips the trap
|
||||
// (token revocation) instead of returning an opaque 404.
|
||||
entry.timeout = setTimeout(() => codes.delete(code), REVOKED_TRAP_MS);
|
||||
entry.timeout.unref();
|
||||
}
|
||||
|
||||
function writeAskpassScript(code: string): string {
|
||||
const scriptId = randomUUID();
|
||||
const scriptName = `askpass-${scriptId}.js`;
|
||||
const scriptPath = join(tmpdir, scriptName);
|
||||
|
||||
// standalone node script — no project dependencies.
|
||||
// git calls this twice: once for "Username for ..." and once for "Password for ...".
|
||||
// username: return "x-access-token" locally (no server call).
|
||||
// password: fetch token from auth server, self-delete, return token.
|
||||
// 409 = code was already consumed by another process (tamper detected).
|
||||
// git invokes this once per credential prompt — separate process spawn
|
||||
// per prompt: one for "Username for ...", one for "Password for ...".
|
||||
// sibling subprocesses (git-lfs pre-push, custom auth-bound hooks)
|
||||
// invoke it independently for their own auth, also one spawn per prompt.
|
||||
// all succeed as long as the parent $git() is still running, which is
|
||||
// why neither the script nor the code is single-use. cleanup happens
|
||||
// in $git()'s finally.
|
||||
// 409 = code was already revoked by $git()'s finally (replay attempt).
|
||||
const content = [
|
||||
`#!/usr/bin/env node`,
|
||||
`var a=process.argv[2]||"";`,
|
||||
@@ -132,10 +150,8 @@ export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer>
|
||||
`if(r.statusCode===409){process.stderr.write("askpass-compromised\\n");process.exit(1)}`,
|
||||
`if(r.statusCode!==200){process.exit(1)}`,
|
||||
`var d="";r.on("data",function(c){d+=c});`,
|
||||
`r.on("end",function(){`,
|
||||
`process.stdout.write(d+"\\n");`,
|
||||
`try{require("fs").unlinkSync("${scriptPath.replace(/\\/g, "\\\\")}")}catch(e){}`,
|
||||
`})}).on("error",function(){process.exit(1)})}`,
|
||||
`r.on("end",function(){process.stdout.write(d+"\\n")})`,
|
||||
`}).on("error",function(){process.exit(1)})}`,
|
||||
].join("\n");
|
||||
|
||||
writeFileSync(scriptPath, content, { mode: 0o700 });
|
||||
@@ -144,7 +160,7 @@ export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer>
|
||||
|
||||
async function close(): Promise<void> {
|
||||
for (const entry of codes.values()) {
|
||||
clearTimeout(entry.timeout);
|
||||
if (entry.timeout) clearTimeout(entry.timeout);
|
||||
}
|
||||
codes.clear();
|
||||
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||
@@ -154,6 +170,7 @@ export async function startGitAuthServer(tmpdir: string): Promise<GitAuthServer>
|
||||
return {
|
||||
port,
|
||||
register,
|
||||
revoke,
|
||||
writeAskpassScript,
|
||||
close,
|
||||
[Symbol.asyncDispose]: close,
|
||||
|
||||
@@ -1,130 +0,0 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { executeLifecycleHook } from "./lifecycle.ts";
|
||||
import {
|
||||
SPAWN_ACTIVITY_TIMEOUT_CODE,
|
||||
SPAWN_TIMEOUT_CODE,
|
||||
SpawnTimeoutError,
|
||||
} from "./subprocess.ts";
|
||||
|
||||
// mock the spawn call so we don't run real subprocesses. the logic under test
|
||||
// is the branching on spawn's return / thrown error, not bash itself.
|
||||
vi.mock("./subprocess.ts", async (importOriginal) => {
|
||||
const actual = await importOriginal<typeof import("./subprocess.ts")>();
|
||||
return {
|
||||
...actual,
|
||||
spawn: vi.fn(),
|
||||
};
|
||||
});
|
||||
|
||||
const { spawn } = await import("./subprocess.ts");
|
||||
const mockedSpawn = vi.mocked(spawn);
|
||||
|
||||
describe("executeLifecycleHook", () => {
|
||||
beforeEach(() => {
|
||||
mockedSpawn.mockReset();
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("returns empty result when no script is configured", async () => {
|
||||
const result = await executeLifecycleHook({ event: "setup", script: null });
|
||||
expect(result).toEqual({});
|
||||
expect(mockedSpawn).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("returns empty result when script exits 0", async () => {
|
||||
mockedSpawn.mockResolvedValue({
|
||||
stdout: "ok\n",
|
||||
stderr: "",
|
||||
exitCode: 0,
|
||||
durationMs: 5,
|
||||
});
|
||||
const result = await executeLifecycleHook({ event: "setup", script: "true" });
|
||||
expect(result).toEqual({});
|
||||
});
|
||||
|
||||
it("returns a warning with stderr content and retry-if-flaky guidance on non-zero exit", async () => {
|
||||
mockedSpawn.mockResolvedValue({
|
||||
stdout: "",
|
||||
stderr: "npm ERR! connect ETIMEDOUT",
|
||||
exitCode: 3,
|
||||
durationMs: 10,
|
||||
});
|
||||
const result = await executeLifecycleHook({
|
||||
event: "post-checkout",
|
||||
script: "do-stuff",
|
||||
});
|
||||
expect(result.warning).toMatch(/post-checkout/);
|
||||
expect(result.warning).toMatch(/exit code 3/);
|
||||
expect(result.warning).toMatch(/npm ERR! connect ETIMEDOUT/);
|
||||
expect(result.warning).toMatch(/retry the operation if the failure looks flaky/);
|
||||
expect(result.warning).toMatch(/do NOT retry/);
|
||||
});
|
||||
|
||||
it("falls back to stdout when stderr is empty", async () => {
|
||||
mockedSpawn.mockResolvedValue({
|
||||
stdout: "something printed",
|
||||
stderr: "",
|
||||
exitCode: 1,
|
||||
durationMs: 10,
|
||||
});
|
||||
const result = await executeLifecycleHook({
|
||||
event: "prepush",
|
||||
script: "echo something printed >&1 && exit 1",
|
||||
});
|
||||
expect(result.warning).toContain("something printed");
|
||||
});
|
||||
|
||||
it("prints '(empty)' when both streams are blank", async () => {
|
||||
mockedSpawn.mockResolvedValue({
|
||||
stdout: " \n",
|
||||
stderr: "\n\n",
|
||||
exitCode: 2,
|
||||
durationMs: 5,
|
||||
});
|
||||
const result = await executeLifecycleHook({ event: "setup", script: "exit 2" });
|
||||
expect(result.warning).toContain("(empty)");
|
||||
});
|
||||
|
||||
it("emits a do-NOT-retry warning when spawn reports an overall timeout", async () => {
|
||||
// SPAWN_TIMEOUT_CODE is the code we must distinguish. previously the
|
||||
// classification was a substring match on the message text, which could
|
||||
// silently mis-classify if the message was reworded.
|
||||
mockedSpawn.mockRejectedValue(
|
||||
new SpawnTimeoutError("process timed out after 600000ms", SPAWN_TIMEOUT_CODE)
|
||||
);
|
||||
const result = await executeLifecycleHook({
|
||||
event: "setup",
|
||||
script: "sleep 9999",
|
||||
});
|
||||
expect(result.warning).toMatch(/timed out after \d+min/);
|
||||
expect(result.warning).toMatch(/do NOT retry/);
|
||||
expect(result.warning).not.toMatch(/transient/);
|
||||
});
|
||||
|
||||
it("treats an activity-timeout error the same as an overall timeout", async () => {
|
||||
mockedSpawn.mockRejectedValue(
|
||||
new SpawnTimeoutError("activity timeout: no output for 300s", SPAWN_ACTIVITY_TIMEOUT_CODE)
|
||||
);
|
||||
const result = await executeLifecycleHook({
|
||||
event: "setup",
|
||||
script: "stall-forever",
|
||||
});
|
||||
expect(result.warning).toMatch(/timed out/);
|
||||
expect(result.warning).toMatch(/do NOT retry/);
|
||||
});
|
||||
|
||||
it("emits a transient-retry warning on a non-timeout spawn failure (e.g. ENOENT)", async () => {
|
||||
mockedSpawn.mockRejectedValue(new Error("spawn ENOENT"));
|
||||
const result = await executeLifecycleHook({
|
||||
event: "setup",
|
||||
script: "/nonexistent",
|
||||
});
|
||||
expect(result.warning).toMatch(/failed to spawn/);
|
||||
expect(result.warning).toMatch(/spawn ENOENT/);
|
||||
expect(result.warning).toMatch(/transient/);
|
||||
expect(result.warning).not.toMatch(/do NOT retry/);
|
||||
});
|
||||
});
|
||||
@@ -1,6 +1,8 @@
|
||||
import {
|
||||
detectProviderError,
|
||||
extractProviderId,
|
||||
findProviderErrorMatch,
|
||||
isProviderBillingExhausted,
|
||||
isRouterKeylimitExhaustedError,
|
||||
} from "./providerErrors.ts";
|
||||
|
||||
@@ -103,6 +105,15 @@ describe("detectProviderError", () => {
|
||||
it("classifies bare 'Insufficient balance' as billing exhausted", () => {
|
||||
expect(detectProviderError("error: Insufficient balance")).toBe("provider billing exhausted");
|
||||
});
|
||||
|
||||
it("classifies Anthropic 'credit balance is too low' as billing exhausted (#835)", () => {
|
||||
// Anthropic-direct BYOK returns this string verbatim when the user's
|
||||
// Anthropic console credit balance can't cover the request. distinct
|
||||
// wording from "Insufficient balance" used by DeepSeek / OpenCode Zen.
|
||||
const stderr =
|
||||
"APIError: 400 Your credit balance is too low to access the Anthropic API. Please go to Plans & Billing to upgrade or purchase credits.";
|
||||
expect(detectProviderError(stderr)).toBe("provider billing exhausted");
|
||||
});
|
||||
});
|
||||
|
||||
describe("real provider errors", () => {
|
||||
@@ -213,6 +224,47 @@ describe("findProviderErrorMatch", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isProviderBillingExhausted (#835)", () => {
|
||||
it("matches DeepSeek 'Insufficient Balance' payloads", () => {
|
||||
expect(isProviderBillingExhausted("AI_APICallError: Insufficient Balance")).toBe(true);
|
||||
});
|
||||
|
||||
it("matches Anthropic 'credit balance is too low' payloads", () => {
|
||||
expect(
|
||||
isProviderBillingExhausted("Your credit balance is too low to access the Anthropic API")
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it("matches OpenCode Zen CreditsError / FreeUsageLimitError", () => {
|
||||
expect(isProviderBillingExhausted("CreditsError: out of credit")).toBe(true);
|
||||
expect(isProviderBillingExhausted("FreeUsageLimitError: limit hit")).toBe(true);
|
||||
});
|
||||
|
||||
it("returns false for unrelated provider errors", () => {
|
||||
expect(isProviderBillingExhausted('{"statusCode": 401}')).toBe(false);
|
||||
expect(isProviderBillingExhausted("rate_limit_exceeded")).toBe(false);
|
||||
expect(isProviderBillingExhausted("just some log noise")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("extractProviderId", () => {
|
||||
it("parses providerID= from OpenCode harness logs", () => {
|
||||
expect(
|
||||
extractProviderId(
|
||||
'ERROR providerID=deepseek modelID=deepseek-v4-pro error={"name":"AI_APICallError"}'
|
||||
)
|
||||
).toBe("deepseek");
|
||||
});
|
||||
|
||||
it("lowercases the captured slug", () => {
|
||||
expect(extractProviderId("providerID=Anthropic modelID=claude")).toBe("anthropic");
|
||||
});
|
||||
|
||||
it("returns null when providerID is absent", () => {
|
||||
expect(extractProviderId("APIError: Insufficient Balance")).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("isRouterKeylimitExhaustedError", () => {
|
||||
it("matches the canonical OpenRouter mid-run error", () => {
|
||||
expect(
|
||||
|
||||
+36
-8
@@ -1,5 +1,8 @@
|
||||
type ProviderErrorPattern = { regex: RegExp; label: string };
|
||||
|
||||
/** Stable label for the BYOK provider-billing-exhausted classification. */
|
||||
export const PROVIDER_BILLING_EXHAUSTED_LABEL = "provider billing exhausted";
|
||||
|
||||
// status codes are only treated as provider errors when they are adjacent to
|
||||
// a recognised status key. this rejects commit SHAs that happen to contain
|
||||
// "429", version strings, file hashes, etc.
|
||||
@@ -9,14 +12,16 @@ const PROVIDER_ERROR_PATTERNS: ProviderErrorPattern[] = [
|
||||
// billing-payload patterns come BEFORE bare status-code patterns. providers
|
||||
// commonly return 401 / 429 for billing/quota exhaustion (OpenCode Zen
|
||||
// `CreditsError` / `FreeUsageLimitError`, Gemini `RESOURCE_EXHAUSTED` +
|
||||
// "spending cap", Anthropic "Insufficient balance"). these are non-retryable
|
||||
// and require user-billing action — distinct from a transient auth error or
|
||||
// rate-limit. status-code patterns would otherwise win and surface
|
||||
// "auth error (401)" / "rate limited (429)" with no billing hint. see #778.
|
||||
{ regex: /\bCreditsError\b/, label: "provider billing exhausted" },
|
||||
{ regex: /\bFreeUsageLimitError\b/, label: "provider billing exhausted" },
|
||||
{ regex: /Insufficient balance/i, label: "provider billing exhausted" },
|
||||
{ regex: /spending cap/i, label: "provider billing exhausted" },
|
||||
// "spending cap", Anthropic "Insufficient balance" / "credit balance is
|
||||
// too low"). these are non-retryable and require user-billing action —
|
||||
// distinct from a transient auth error or rate-limit. status-code patterns
|
||||
// would otherwise win and surface "auth error (401)" / "rate limited (429)"
|
||||
// with no billing hint. see #778, #835.
|
||||
{ regex: /\bCreditsError\b/, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||||
{ regex: /\bFreeUsageLimitError\b/, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||||
{ regex: /Insufficient balance/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||||
{ regex: /credit balance is too low/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||||
{ regex: /spending cap/i, label: PROVIDER_BILLING_EXHAUSTED_LABEL },
|
||||
// auth patterns must come BEFORE rate-limit patterns. OpenRouter 401 error
|
||||
// payloads carry `x-ratelimit-*` response headers in the dump, and the
|
||||
// free-form rate-limit regex below would otherwise win on word-boundary
|
||||
@@ -142,3 +147,26 @@ const ROUTER_KEYLIMIT_EXHAUSTED_PATTERN =
|
||||
export function isRouterKeylimitExhaustedError(text: string): boolean {
|
||||
return ROUTER_KEYLIMIT_EXHAUSTED_PATTERN.test(text);
|
||||
}
|
||||
|
||||
/**
|
||||
* BYOK billing-exhausted: provider rejected the request because the user's
|
||||
* provider wallet is empty (DeepSeek "Insufficient Balance", Anthropic
|
||||
* "credit balance is too low", OpenCode Zen `CreditsError` /
|
||||
* `FreeUsageLimitError`, Gemini "spending cap"). Distinct from
|
||||
* `isRouterKeylimitExhaustedError` — that's Pullfrog's Router wallet, this
|
||||
* is the user's own provider account.
|
||||
*/
|
||||
export function isProviderBillingExhausted(text: string): boolean {
|
||||
return findProviderErrorMatch(text)?.label === PROVIDER_BILLING_EXHAUSTED_LABEL;
|
||||
}
|
||||
|
||||
/**
|
||||
* Extract `providerID=foo` from agent error logs (OpenCode emits this on
|
||||
* `provider error detected (...)` lines). Returns the lowercase provider
|
||||
* slug, or null when absent. Used to render a provider-specific dashboard
|
||||
* link in the BYOK billing-exhausted summary.
|
||||
*/
|
||||
export function extractProviderId(text: string): string | null {
|
||||
const match = text.match(/\bproviderID=([a-z0-9_-]+)/i);
|
||||
return match ? match[1].toLowerCase() : null;
|
||||
}
|
||||
|
||||
@@ -12,6 +12,11 @@ export interface HandleAgentResultParams {
|
||||
|
||||
export async function handleAgentResult(ctx: HandleAgentResultParams): Promise<MainResult> {
|
||||
if (!ctx.result.success) {
|
||||
// rendering + posting for the `!success` branch lives in
|
||||
// `finalizeSuccessRun` (called immediately before this function) so the
|
||||
// BYOK billing-exhausted, hang, and api-key bodies land on a single
|
||||
// surface — both for runs with a pre-existing progress comment AND for
|
||||
// silent triggers via `createIfMissing`. see #835.
|
||||
return {
|
||||
success: false,
|
||||
error: ctx.result.error || "Agent execution failed",
|
||||
|
||||
@@ -3,6 +3,58 @@ import { renderRunError } from "./runErrorRenderer.ts";
|
||||
|
||||
const repo = { owner: "acme", name: "widget" };
|
||||
|
||||
describe("renderRunError BYOK provider billing exhausted (#835)", () => {
|
||||
const deepseekRaw =
|
||||
'» provider error detected (provider billing exhausted): ERROR providerID=deepseek modelID=deepseek-v4-pro error={"name":"AI_APICallError","message":"Insufficient Balance"}';
|
||||
|
||||
const anthropicRaw =
|
||||
"APIError: Your credit balance is too low to access the Anthropic API. Please go to Plans & Billing to upgrade or purchase credits.";
|
||||
|
||||
const opencodeZenRaw = "CreditsError: account out of free usage";
|
||||
|
||||
it("renders DeepSeek billing-exhausted with provider-specific dashboard link", () => {
|
||||
const result = renderRunError({
|
||||
errorMessage: deepseekRaw,
|
||||
repo,
|
||||
agentDiagnostic: undefined,
|
||||
});
|
||||
expect(result.summary).toContain("`deepseek` account is out of credit");
|
||||
expect(result.summary).toContain("https://platform.deepseek.com/top_up");
|
||||
expect(result.summary).toContain("### ❌ Pullfrog failed");
|
||||
expect(result.comment).toContain("`deepseek` account is out of credit");
|
||||
expect(result.comment).not.toContain("### ❌ Pullfrog failed");
|
||||
});
|
||||
|
||||
it("matches Anthropic 'credit balance is too low' (#835 Anthropic case)", () => {
|
||||
const result = renderRunError({
|
||||
errorMessage: anthropicRaw,
|
||||
repo,
|
||||
agentDiagnostic: undefined,
|
||||
});
|
||||
expect(result.comment).toContain("out of credit");
|
||||
});
|
||||
|
||||
it("matches OpenCode Zen CreditsError shape", () => {
|
||||
const result = renderRunError({
|
||||
errorMessage: opencodeZenRaw,
|
||||
repo,
|
||||
agentDiagnostic: undefined,
|
||||
});
|
||||
expect(result.comment).toContain("out of credit");
|
||||
});
|
||||
|
||||
it("falls through to a generic CTA when providerID cannot be parsed", () => {
|
||||
const result = renderRunError({
|
||||
errorMessage: "Insufficient balance — provider response with no providerID tag",
|
||||
repo,
|
||||
agentDiagnostic: undefined,
|
||||
});
|
||||
expect(result.comment).toContain("Your provider account is out of credit");
|
||||
expect(result.comment).not.toContain("Your your");
|
||||
expect(result.comment).toContain("Top up your provider account");
|
||||
});
|
||||
});
|
||||
|
||||
describe("renderRunError ProviderModelNotFoundError (#816)", () => {
|
||||
const staleFreeRaw =
|
||||
'ProviderModelNotFoundError: {"providerID":"opencode","modelID":"retired-free-model","suggestions":["deepseek-v4-flash-free"]}';
|
||||
|
||||
@@ -3,22 +3,37 @@
|
||||
* pair of user-facing markdown bodies — one for the GitHub Actions job
|
||||
* summary tab, one for the PR progress comment.
|
||||
*
|
||||
* Four classifications, in priority order:
|
||||
* Classifications, in dispatch order (first match wins; the api-key
|
||||
* branch additionally folds in the activity-timeout hang body as a
|
||||
* sub-source so a hang masking an api-key error still surfaces the api-key
|
||||
* CTA):
|
||||
*
|
||||
* 1. `BillingError` — either the proxy-token mint already threw one (402
|
||||
* handled inline) or the agent runtime surfaced an OpenRouter
|
||||
* "key budget exhausted" string mid-run. Both render via
|
||||
* `formatBillingErrorSummary` so the user sees actionable copy.
|
||||
*
|
||||
* 2. Activity-timeout hang — `errorMessage` starts with
|
||||
* `"activity timeout"` or `"agent still pending"`. The harness keeps
|
||||
* structured diagnostic state on `toolState.agentDiagnostic`;
|
||||
* `formatAgentHangBody` renders that as a markdown block.
|
||||
* 2. BYOK provider billing-exhausted (#835) — DeepSeek "Insufficient
|
||||
* Balance", Anthropic "credit balance is too low", OpenCode Zen
|
||||
* `CreditsError`, Gemini "spending cap". Checked before api-key auth
|
||||
* because billing-exhausted responses often carry 401 status codes
|
||||
* that `isApiKeyAuthError` would otherwise mis-classify.
|
||||
*
|
||||
* 3. API-key auth error — `isApiKeyAuthError` sniffs the raw error string;
|
||||
* `formatApiKeyErrorSummary` renders provider + console-link copy.
|
||||
* 3. API-key auth error — `isApiKeyAuthError` sniffs the raw error string
|
||||
* (or the activity-timeout hang body when present, since that's where
|
||||
* the underlying provider error often lands); `formatApiKeyErrorSummary`
|
||||
* renders provider + console-link copy.
|
||||
*
|
||||
* 4. Default — a generic `❌ Pullfrog failed` block with the raw error
|
||||
* 4. ProviderModelNotFoundError — stale free-fallback model id no longer
|
||||
* in the OpenCode catalog; renders a nudge to add a BYOK key.
|
||||
*
|
||||
* 5. Activity-timeout hang — `errorMessage` starts with
|
||||
* `"activity timeout"` or `"agent still pending"` AND none of the
|
||||
* above matched. The harness keeps structured diagnostic state on
|
||||
* `toolState.agentDiagnostic`; `formatAgentHangBody` renders that as
|
||||
* a markdown block.
|
||||
*
|
||||
* 6. Default — a generic `❌ Pullfrog failed` block with the raw error
|
||||
* message in a fenced code block. Same body for both surfaces.
|
||||
*
|
||||
* The hang body and the API-key body diverge between the two surfaces only
|
||||
@@ -31,7 +46,11 @@ import type { AgentDiagnostic } from "./agentHangReport.ts";
|
||||
import { formatAgentHangBody } from "./agentHangReport.ts";
|
||||
import { formatApiKeyErrorSummary, isApiKeyAuthError } from "./apiKeys.ts";
|
||||
import { BillingError, formatBillingErrorSummary } from "./billingErrors.ts";
|
||||
import { isRouterKeylimitExhaustedError } from "./providerErrors.ts";
|
||||
import {
|
||||
extractProviderId,
|
||||
isProviderBillingExhausted,
|
||||
isRouterKeylimitExhaustedError,
|
||||
} from "./providerErrors.ts";
|
||||
|
||||
export type RenderedRunError = {
|
||||
summary: string;
|
||||
@@ -42,6 +61,60 @@ function isProviderModelNotFoundError(message: string): boolean {
|
||||
return message.includes("ProviderModelNotFoundError");
|
||||
}
|
||||
|
||||
/**
|
||||
* Best-known billing top-up URL per provider. Conservative list: only
|
||||
* providers we've actually classified billing-exhaustion shapes for in
|
||||
* `providerErrors.ts`. Unknown providers fall through to a generic CTA.
|
||||
*/
|
||||
const PROVIDER_BILLING_URLS: Record<string, string> = {
|
||||
deepseek: "https://platform.deepseek.com/top_up",
|
||||
anthropic: "https://console.anthropic.com/settings/billing",
|
||||
openai: "https://platform.openai.com/account/billing",
|
||||
google: "https://aistudio.google.com/usage",
|
||||
opencode: "https://opencode.ai/zen",
|
||||
};
|
||||
|
||||
/**
|
||||
* `extractProviderId` only fires when the harness emits `providerID=...`
|
||||
* (OpenCode log shape). Direct-provider errors (e.g. Anthropic SDK throwing
|
||||
* `"Your credit balance is too low to access the Anthropic API"`) carry no
|
||||
* such tag, so map their distinctive copy to a provider id here so the
|
||||
* dashboard link is reachable.
|
||||
*
|
||||
* Pattern is intentionally tight (Anthropic-specific phrasing only) to
|
||||
* avoid mis-tagging non-Anthropic billing-exhausted errors that happen to
|
||||
* mention `"Anthropic API"` in passing — the broader phrase appears in
|
||||
* fallback-chain agent prompt text and OpenCode harness logs.
|
||||
*/
|
||||
function detectProviderId(message: string): string | null {
|
||||
const harnessId = extractProviderId(message);
|
||||
if (harnessId) return harnessId;
|
||||
if (/credit balance is too low/i.test(message)) return "anthropic";
|
||||
return null;
|
||||
}
|
||||
|
||||
function formatProviderBillingExhausted(input: { errorMessage: string }): string {
|
||||
const providerId = detectProviderId(input.errorMessage);
|
||||
const dashboardUrl = providerId ? PROVIDER_BILLING_URLS[providerId] : undefined;
|
||||
|
||||
const headline = providerId
|
||||
? `**Your \`${providerId}\` account is out of credit.**`
|
||||
: "**Your provider account is out of credit.**";
|
||||
const cta = dashboardUrl
|
||||
? `[Top up \`${providerId}\` →](${dashboardUrl})`
|
||||
: "Top up your provider account, then re-trigger Pullfrog.";
|
||||
|
||||
return [
|
||||
headline,
|
||||
"",
|
||||
"Pullfrog detected a billing-exhausted response from your provider — the agent stopped before completing this run.",
|
||||
"",
|
||||
cta,
|
||||
"",
|
||||
`\`\`\`\n${input.errorMessage}\n\`\`\``,
|
||||
].join("\n");
|
||||
}
|
||||
|
||||
function formatProviderModelNotFoundSummary(input: {
|
||||
owner: string;
|
||||
name: string;
|
||||
@@ -87,6 +160,21 @@ export function renderRunError(input: {
|
||||
})
|
||||
: null;
|
||||
|
||||
// BYOK provider billing-exhausted (DeepSeek "Insufficient Balance",
|
||||
// Anthropic "credit balance is too low", OpenCode Zen `CreditsError` /
|
||||
// `FreeUsageLimitError`, Gemini "spending cap"). distinct from the Router
|
||||
// billing branches above — Router uses `BillingError`, this uses the agent
|
||||
// log payload classified by `isProviderBillingExhausted`. see #835.
|
||||
//
|
||||
// checked BEFORE api-key auth: providers commonly return 401 (DeepSeek,
|
||||
// Gemini) or include `"API Error: 401"` in the error body for billing
|
||||
// exhaustion, which `isApiKeyAuthError` would otherwise match — surfacing
|
||||
// a "rotate your key" CTA when the actual fix is "top up credits".
|
||||
if (isProviderBillingExhausted(input.errorMessage)) {
|
||||
const body = formatProviderBillingExhausted({ errorMessage: input.errorMessage });
|
||||
return { summary: `### ❌ Pullfrog failed\n\n${body}`, comment: body };
|
||||
}
|
||||
|
||||
const apiKeySource = hangBody ?? input.errorMessage;
|
||||
const apiKeyErrorSummary = isApiKeyAuthError(apiKeySource)
|
||||
? formatApiKeyErrorSummary({
|
||||
|
||||
+33
-13
@@ -81,12 +81,13 @@ export async function finalizeSuccessRun(input: {
|
||||
await persistRunArtifacts(input.toolContext);
|
||||
|
||||
// shared rendering for the !success branch — same classifier as the
|
||||
// outer catch path (BillingError reclassify → hang → api-key → generic),
|
||||
// so a harness-returned `{success: false}` lands an actionable error
|
||||
// block in the job summary alongside the matching body in the progress
|
||||
// comment. hang and generic get the `### ❌ Pullfrog failed` H3 banner;
|
||||
// BillingError and api-key render their own provider-specific framing
|
||||
// (no banner). renders once; reused for both surfaces below.
|
||||
// outer catch path (BillingError reclassify → hang → BYOK billing →
|
||||
// api-key → generic), so a harness-returned `{success: false}` lands an
|
||||
// actionable error block in the job summary alongside the matching body
|
||||
// in the progress comment. hang and generic get the `### ❌ Pullfrog
|
||||
// failed` H3 banner; BillingError, BYOK billing, and api-key render
|
||||
// their own provider-specific framing (no banner). renders once; reused
|
||||
// for both surfaces below.
|
||||
const rendered = !input.result.success
|
||||
? renderRunError({
|
||||
errorMessage: input.result.error || "agent run failed",
|
||||
@@ -95,12 +96,20 @@ export async function finalizeSuccessRun(input: {
|
||||
})
|
||||
: null;
|
||||
|
||||
if (rendered && input.toolState.progressComment) {
|
||||
await reportErrorToComment({ toolState: input.toolState, error: rendered.comment }).catch(
|
||||
(error) => {
|
||||
log.debug(`failure error report failed: ${error}`);
|
||||
}
|
||||
);
|
||||
// `createIfMissing: true` is load-bearing for silent triggers
|
||||
// (IncrementalReview / pull_request_synchronize / auto-label) that have
|
||||
// no progress comment to update — without it, terminal failures like
|
||||
// BYOK billing exhaustion land only in the GH job summary, which most
|
||||
// users never open. `reportErrorToComment` no-ops when both progress
|
||||
// comment AND issue context are absent. see #835.
|
||||
if (rendered) {
|
||||
await reportErrorToComment({
|
||||
toolState: input.toolState,
|
||||
error: rendered.comment,
|
||||
createIfMissing: true,
|
||||
}).catch((error) => {
|
||||
log.debug(`failure error report failed: ${error}`);
|
||||
});
|
||||
}
|
||||
|
||||
// create_pull_request_review owns its own deletion (see mcp/review.ts), so
|
||||
@@ -141,6 +150,13 @@ export async function finalizeSuccessRun(input: {
|
||||
*
|
||||
* `lastProgressBody` and the usage table are appended to the summary so the
|
||||
* partial work the agent did before failing isn't lost.
|
||||
*
|
||||
* `createIfMissing: true` is symmetric with `finalizeSuccessRun` — silent
|
||||
* triggers (IncrementalReview / pull_request_synchronize / auto-label) that
|
||||
* throw past `finalizeSuccessRun` (e.g. timeout race kills the agent
|
||||
* mid-billing-exhausted-retry) reach this catch path with no progress
|
||||
* comment to update, and without `createIfMissing` the terminal error
|
||||
* lands only in the GH job summary that most users never open. see #835.
|
||||
*/
|
||||
export async function writeRunErrorOutputs(input: {
|
||||
rendered: RenderedRunError;
|
||||
@@ -155,7 +171,11 @@ export async function writeRunErrorOutputs(input: {
|
||||
} catch {}
|
||||
|
||||
try {
|
||||
await reportErrorToComment({ toolState: input.toolState, error: input.rendered.comment });
|
||||
await reportErrorToComment({
|
||||
toolState: input.toolState,
|
||||
error: input.rendered.comment,
|
||||
createIfMissing: true,
|
||||
});
|
||||
} catch {
|
||||
// error reporting failed, but don't let it mask the original error
|
||||
}
|
||||
|
||||
@@ -1,220 +0,0 @@
|
||||
import { performance } from "node:perf_hooks";
|
||||
import * as cli from "./cli.ts";
|
||||
import { ThinkingTimer, Timer } from "./timer.ts";
|
||||
|
||||
describe("Timer", () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(cli.log, "debug");
|
||||
// Mock performance.now() to have predictable timestamps
|
||||
vi.spyOn(performance, "now");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("constructor", () => {
|
||||
it("should initialize with current timestamp", () => {
|
||||
const mockTime = 1000000;
|
||||
vi.mocked(performance.now).mockReturnValueOnce(mockTime).mockReturnValueOnce(mockTime);
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("test");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledWith(expect.stringContaining("test"));
|
||||
});
|
||||
});
|
||||
|
||||
describe("checkpoint", () => {
|
||||
it("should log duration from initial timestamp on first checkpoint", () => {
|
||||
const startTime = 1000000;
|
||||
const checkpointTime = startTime + 100;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // constructor
|
||||
.mockReturnValueOnce(checkpointTime); // checkpoint
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("first");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledWith("» first: 100ms");
|
||||
});
|
||||
|
||||
it("should log duration from last checkpoint on subsequent checkpoints", () => {
|
||||
const startTime = 1000000;
|
||||
const firstCheckpointTime = startTime + 50;
|
||||
const secondCheckpointTime = firstCheckpointTime + 75;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // constructor
|
||||
.mockReturnValueOnce(firstCheckpointTime) // first checkpoint
|
||||
.mockReturnValueOnce(secondCheckpointTime); // second checkpoint
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("first");
|
||||
timer.checkpoint("second");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledTimes(2);
|
||||
expect(cli.log.debug).toHaveBeenNthCalledWith(1, "» first: 50ms");
|
||||
expect(cli.log.debug).toHaveBeenNthCalledWith(2, "» second: 75ms");
|
||||
});
|
||||
|
||||
it("should handle multiple checkpoints correctly", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // constructor
|
||||
.mockReturnValueOnce(startTime + 10) // step1
|
||||
.mockReturnValueOnce(startTime + 25) // step2
|
||||
.mockReturnValueOnce(startTime + 45); // step3
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("step1");
|
||||
timer.checkpoint("step2");
|
||||
timer.checkpoint("step3");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledTimes(3);
|
||||
expect(cli.log.debug).toHaveBeenNthCalledWith(1, "» step1: 10ms");
|
||||
expect(cli.log.debug).toHaveBeenNthCalledWith(2, "» step2: 15ms");
|
||||
expect(cli.log.debug).toHaveBeenNthCalledWith(3, "» step3: 20ms");
|
||||
});
|
||||
|
||||
it("should handle zero duration correctly", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // constructor
|
||||
.mockReturnValueOnce(startTime); // checkpoint
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("immediate");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledWith("» immediate: 0ms");
|
||||
});
|
||||
|
||||
it("should handle custom checkpoint names", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // constructor
|
||||
.mockReturnValueOnce(startTime + 200); // checkpoint
|
||||
|
||||
const timer = new Timer();
|
||||
timer.checkpoint("Custom Checkpoint Name");
|
||||
|
||||
expect(cli.log.debug).toHaveBeenCalledWith("» Custom Checkpoint Name: 200ms");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe("ThinkingTimer", () => {
|
||||
beforeEach(() => {
|
||||
vi.spyOn(cli.log, "info");
|
||||
vi.spyOn(performance, "now");
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe("markToolResult", () => {
|
||||
it("should store the current timestamp", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 5000); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("markToolCall", () => {
|
||||
it("should not log if markToolResult was never called", () => {
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should not log if elapsed time is below threshold (3000ms)", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 2999); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("should log if elapsed time equals threshold (3000ms)", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 3000); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalledWith("» thought for 3 seconds");
|
||||
});
|
||||
|
||||
it("should log if elapsed time exceeds threshold", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 5500); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalledWith("» thought for 5.5 seconds");
|
||||
});
|
||||
|
||||
it("should format large durations correctly", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 15000); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalledWith("» thought for 15 seconds");
|
||||
});
|
||||
|
||||
it("should handle multiple markToolCall invocations", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 4000) // first markToolCall
|
||||
.mockReturnValueOnce(startTime + 5000); // second markToolCall
|
||||
|
||||
const timer = new ThinkingTimer();
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalledTimes(2);
|
||||
expect(cli.log.info).toHaveBeenNthCalledWith(1, "» thought for 4 seconds");
|
||||
expect(cli.log.info).toHaveBeenNthCalledWith(2, "» thought for 5 seconds");
|
||||
});
|
||||
|
||||
it("routes log lines through the optional formatLine for per-session prefixing", () => {
|
||||
const startTime = 1000000;
|
||||
vi.mocked(performance.now)
|
||||
.mockReturnValueOnce(startTime) // markToolResult
|
||||
.mockReturnValueOnce(startTime + 4000); // markToolCall
|
||||
|
||||
const timer = new ThinkingTimer((line) => `[lens:security] ${line}`);
|
||||
timer.markToolResult();
|
||||
timer.markToolCall();
|
||||
|
||||
expect(cli.log.info).toHaveBeenCalledWith("[lens:security] » thought for 4 seconds");
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user