fix(tasks): permanent failure on missing secrets; post-transition row from POST /api/tasks - #569
Merged
Merged
Conversation
…issing A repo task whose agent secret was never stored (e.g. claude-code with no ANTHROPIC_API_KEY) threw "Secret not found: NAME (scope: global)" during provisioning, which the error classifier marked retryable. The task-worker re-queued it on a 30s delay, and because the reconciler re-enqueues queued tasks without the provisioningRetryCount job field, the 3-retry cap reset every cycle: the task bounced between queued and provisioning forever with no terminal state and no actionable surface. Classify the exact "Secret not found: NAME" shape thrown by secret-service.retrieveSecret() as non-retryable, so the provisioning catch takes its permanent-failure branch: one worker pickup, a provisioning_permanent_failure transition, and terminal failed with the actionable message on the task row. The classification stays narrow; other auth errors keep their existing retry semantics. Covered by a dedicated e2e (provisioning-no-secret.e2e.test.ts) that boots the real API server with no secrets seeded and proves the task lands in failed with no provisioning_retry events, staying failed across reconciler cycles.
The repo-task branch of POST /api/tasks inserted the task, transitioned it to queued (or waiting_on_deps), enqueued the BullMQ job, and then responded with the stale row from createTask() - state "pending", a state the task had already left. Clients acting on the response immediately saw the wrong state. transitionTask() already returns the post-transition row, so respond with that instead of re-fetching. The e2e assertion that pinned the old behavior (repo-task.e2e.test.ts expected "pending" with a comment noting it was the PRE-transition row) now asserts "queued", and the route unit tests assert the response state for both the queued and waiting_on_deps paths.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two confirmed task-pipeline bugs, one fix each, both covered by tests.
Bug 1: missing agent secret made repo tasks retry provisioning forever
Failure scenario. Create a repo task (agentType
claude-code) with noANTHROPIC_API_KEYsecret stored. The task-worker's provisioning path throwsSecret not found: ANTHROPIC_API_KEY (scope: global)(fromsecret-service.retrieveSecret()viaresolveSecretsForTask). The provisioning catch intask-worker.tsconsultsclassifyError(), which markedSecret not foundasretryable: true, so the task took the recoverable branch: back toqueuedwith a 30s-delay requeue. The worker's own 3-retry cap never engaged in practice because the reconciler'sapplyRequeueForAgentre-enqueues queued tasks as bare{ taskId }jobs, resettingprovisioningRetryCountto 0 every cycle. Net effect: the task bouncedqueued<->provisioningindefinitely with no terminal state and no actionable surface.Fix.
packages/shared/src/error-classifier.ts: theSecret not found: NAMEpattern (the exact message shaperetrieveSecret()throws, nothing broader) is nowretryable: false. The provisioning catch already treats non-retryable classifications as permanent, so the first failure lands the task in terminalfailedviaprovisioning_permanent_failure, with the clearSecret not found: ...message onerrorMessageand an actionable remedy in the classified error. Other auth patterns (expired OAuth token, rate limits, etc.) keep their existing retry semantics.Bug 2: POST /api/tasks returned the pre-transition row
Failure scenario. The repo-task branch of
POST /api/tasksinserts the task (pending), transitions it toqueued(orwaiting_on_deps), enqueues the BullMQ job — and then responded with the stale insert row:state: "pending". Clients immediately saw a state the task was no longer in; the e2e suite had even pinned this with a comment ("the response carries the ORIGINAL createTask() row").Fix.
apps/api/src/routes/tasks.ts: respond with the row returned bytaskService.transitionTask()(it already returns the post-transition row — no extra query). The response now reportsqueued/waiting_on_deps. The Zod response schema (task: z.record(z.unknown())) is unaffected; the OpenAPI description notes the response carries the post-transition row.Tests
apps/api/e2e/provisioning-no-secret.e2e.test.ts: boots the real API server with no secrets seeded, creates a repo task, and proves it reaches terminalfailedwithSecret not found: ANTHROPIC_API_KEYinerrorMessage, exactly oneworker_pickup, oneprovisioning_permanent_failureevent (provisioning->failed), zeroprovisioning_retryevents — and stays that way after several reconciler/stall-check cycles (no resurrection loop). Also asserts the POST response state isqueued(bug 2 in the same server).apps/api/e2e/repo-task.e2e.test.ts: the pinnedexpect(body.task.state).toBe("pending")assertion now asserts"queued".packages/shared/src/error-classifier.test.ts: missing-secret classifications now asserted non-retryable.apps/api/src/routes/tasks.test.ts:transitionTaskmocks return the post-transition rows; response state asserted for both thequeuedandwaiting_on_depspaths.Verification
apps/apiandpackages/sharedtsc --noEmit: cleanapps/apiunit suite: 123 files / 2167 tests pass;packages/shared: 19 files / 440 tests pass (full workspaceturbo testalso ran green in the pre-push hook)pnpm format:check: clean