diff --git a/.github/workflows/skill-card-worker.yml b/.github/workflows/skill-card-worker.yml index 513c8bfd..a368ab04 100644 --- a/.github/workflows/skill-card-worker.yml +++ b/.github/workflows/skill-card-worker.yml @@ -67,7 +67,7 @@ jobs: if ! command -v codex >/dev/null 2>&1; then npm install -g @openai/codex@0.142.3 fi - python3 -m pip install --user 'jinja2==3.1.6' + python3 -m pip install --user --retries 5 --timeout 60 'jinja2==3.1.6' codex --version - name: Authenticate Codex CLI diff --git a/scripts/security/run-codex-scan-worker.test.ts b/scripts/security/run-codex-scan-worker.test.ts index d3b2dc87..7156f239 100644 --- a/scripts/security/run-codex-scan-worker.test.ts +++ b/scripts/security/run-codex-scan-worker.test.ts @@ -14,10 +14,12 @@ import { buildPrompt, claimBatchDrainedQueue, claimFailuresAreFatal, + minimumClaimWindowMs, normalizeSkillSpectorAnalysis, processJob, resolveSkillSpectorScanInput, resolveSkillSpectorScanInputs, + shouldClaimSecurityScanBatch, writeArtifactWorkspace, writeJobDiagnostic, } from "./run-codex-scan-worker"; @@ -74,6 +76,17 @@ describe("run-codex-scan-worker diagnostics", () => { expect(claimBatchDrainedQueue(0, 4, 4)).toBe(false); }); + it("keeps enough wall-clock room for one Codex scan plus cleanup before claiming", () => { + expect(minimumClaimWindowMs(8 * 60_000, 4 * 60_000)).toBe(7 * 60_000); + expect(minimumClaimWindowMs(5 * 60_000, 4 * 60_000)).toBe(5 * 60_000); + }); + + it("always allows the first claim even when the configured runtime is short", () => { + expect(shouldClaimSecurityScanBatch(0, 1, 5 * 60_000)).toBe(true); + expect(shouldClaimSecurityScanBatch(1, 1, 5 * 60_000)).toBe(false); + expect(shouldClaimSecurityScanBatch(1, 5 * 60_000, 5 * 60_000)).toBe(true); + }); + it("keeps successful claims when a parallel claim request fails", async () => { const claimCodexScanJobBatch = ( codexScanWorker as typeof codexScanWorker & { @@ -734,7 +747,11 @@ describe("run-codex-scan-worker diagnostics", () => { }, undefined, ), - ).resolves.toBe(false); + ).resolves.toEqual({ + completed: false, + hardFailed: true, + retryableFailed: false, + }); expect(client.action).toHaveBeenCalledWith( expect.anything(), @@ -768,7 +785,7 @@ describe("run-codex-scan-worker diagnostics", () => { process.env.GITHUB_ACTIONS = "true"; const stdoutWrite = vi.spyOn(process.stdout, "write").mockImplementation(() => true); const client = { - action: vi.fn(async (..._args: unknown[]) => ({ retry: false })), + action: vi.fn(async (..._args: unknown[]) => ({ retry: true })), }; await expect( @@ -799,7 +816,11 @@ describe("run-codex-scan-worker diagnostics", () => { }, undefined, ), - ).resolves.toBe(false); + ).resolves.toEqual({ + completed: false, + hardFailed: false, + retryableFailed: true, + }); const failArgs = client.action.mock.calls[0]?.[1] as { error?: unknown } | undefined; const error = String(failArgs?.error); diff --git a/scripts/security/run-codex-scan-worker.ts b/scripts/security/run-codex-scan-worker.ts index 0460f6d4..ed30a80d 100644 --- a/scripts/security/run-codex-scan-worker.ts +++ b/scripts/security/run-codex-scan-worker.ts @@ -109,9 +109,16 @@ type JobDiagnosticInput = { type CodexScanWorkerClient = Pick; +type ProcessJobResult = { + completed: boolean; + hardFailed: boolean; + retryableFailed: boolean; +}; + const DEFAULT_BATCH_LIMIT = 4; const DEFAULT_MAX_RUNTIME_MS = 40 * 60 * 1000; const DEFAULT_CODEX_SCAN_TIMEOUT_MS = 20 * 60 * 1000; +const CLAIM_WINDOW_SHUTDOWN_BUFFER_MS = 3 * 60 * 1000; const MAX_DIAGNOSTIC_TEXT_CHARS = 20_000; const MAX_STORED_SKILLSPECTOR_ISSUES = 25; const MAX_STORED_SKILLSPECTOR_TEXT_CHARS = 2_000; @@ -1134,6 +1141,18 @@ function codexScanTimeoutMs() { return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_CODEX_SCAN_TIMEOUT_MS; } +export function minimumClaimWindowMs(maxRuntimeMs: number, scanTimeoutMs: number) { + return Math.min(maxRuntimeMs, scanTimeoutMs + CLAIM_WINDOW_SHUTDOWN_BUFFER_MS); +} + +export function shouldClaimSecurityScanBatch( + totalClaimed: number, + remainingRuntimeMs: number, + minClaimWindowMs: number, +) { + return totalClaimed === 0 || remainingRuntimeMs >= minClaimWindowMs; +} + async function runCodex( job: ClaimedJob, workspace: string, @@ -1205,7 +1224,7 @@ export async function processJob( token: string, job: ClaimedJob, diagnosticsRoot: string | undefined, -): Promise { +): Promise { const workspace = await mkdtemp(join(tmpdir(), `clawhub-codex-scan-${basename(job.job._id)}-`)); const startedAt = Date.now(); const codex: CodexCommandDiagnostic = {}; @@ -1245,7 +1264,7 @@ export async function processJob( }, "security scan job completed", ); - return true; + return { completed: true, hardFailed: false, retryableFailed: false }; } catch (error) { errorMessage = sanitizeWorkerErrorMessage( error instanceof Error ? error.message : String(error), @@ -1268,7 +1287,11 @@ export async function processJob( }, "security scan job failed", ); - return false; + return { + completed: false, + hardFailed: !failResult?.retry, + retryableFailed: Boolean(failResult?.retry), + }; } finally { try { await writeJobDiagnostic({ @@ -1350,10 +1373,11 @@ async function main() { }:${process.env.CODEX_SECURITY_SCAN_SHARD ?? process.env.GITHUB_JOB ?? "0"}`; const startedAt = Date.now(); const claimDeadline = startedAt + maxRuntimeMs; - const minClaimWindowMs = Math.min(maxRuntimeMs, codexScanTimeoutMs() + 60_000); + const minClaimWindowMs = minimumClaimWindowMs(maxRuntimeMs, codexScanTimeoutMs()); let totalClaimed = 0; let totalCompleted = 0; let totalFailed = 0; + let totalRetryableFailed = 0; let totalClaimFailures = 0; logger.info( @@ -1363,7 +1387,7 @@ async function main() { while (Date.now() < claimDeadline) { const remainingRuntimeMs = claimDeadline - Date.now(); - if (remainingRuntimeMs < minClaimWindowMs) { + if (!shouldClaimSecurityScanBatch(totalClaimed, remainingRuntimeMs, minClaimWindowMs)) { logger.info( { event: "security_scan_claim_window_closed", @@ -1410,8 +1434,9 @@ async function main() { const results = await Promise.all( jobs.map((job) => processJob(client, token, job, diagnosticsRoot)), ); - totalCompleted += results.filter(Boolean).length; - totalFailed += results.filter((ok) => !ok).length; + totalCompleted += results.filter((result) => result.completed).length; + totalFailed += results.filter((result) => result.hardFailed).length; + totalRetryableFailed += results.filter((result) => result.retryableFailed).length; if (claimBatchDrainedQueue(claimFailures, jobs.length, claimLimit)) break; } @@ -1424,6 +1449,7 @@ async function main() { totalClaimFailures, totalCompleted, totalFailed, + totalRetryableFailed, workerId, }, "security scan worker summary", diff --git a/scripts/skill-cards/skill-card-worker-workflow.test.ts b/scripts/skill-cards/skill-card-worker-workflow.test.ts index 1efbe743..18fcb579 100644 --- a/scripts/skill-cards/skill-card-worker-workflow.test.ts +++ b/scripts/skill-cards/skill-card-worker-workflow.test.ts @@ -86,7 +86,9 @@ describe("skill-card-worker workflow", () => { (step) => step.name === "Install Codex CLI and renderer dependencies", )?.run; expect(dependenciesInstall).toContain("npm install -g @openai/codex@0.142.3"); - expect(dependenciesInstall).toContain("python3 -m pip install --user 'jinja2==3.1.6'"); + expect(dependenciesInstall).toContain( + "python3 -m pip install --user --retries 5 --timeout 60 'jinja2==3.1.6'", + ); expect(dependenciesInstall).not.toContain("@latest"); expect(dependenciesInstall).not.toMatch(/pip install --user jinja2(?:\s|$)/); });