feat: task retry re-resolves against current configuration (ADR-0014)

ARCHITECTURE.md

@@ -1,6 +1,6 @@
 # Architecture
 
-This document is the orientation map for contributors. The authoritative design lives in `docs/design/` (10 documents) and `docs/adr/` (13 decision records); this file tells you what exists, where it lives, and which invariants hold it together.
+This document is the orientation map for contributors. The authoritative design lives in `docs/design/` (10 documents) and `docs/adr/` (14 decision records); this file tells you what exists, where it lives, and which invariants hold it together.
 
 ## Bird's-eye view
 

CHANGELOG.md

@@ -13,7 +13,7 @@ All notable changes to Agrippa are documented here. The format follows
   - *Provider catalog.* `PROVIDER_CATALOG` in `@agrippa/core` declares per-wire-protocol default endpoints and an auth policy per provider — `dashscope` requires a project credential (no legitimate env fallback), `anthropic`/`openai` keep worker env as the deployment default. Qwen models (`qwen3.7-max/plus`, `qwen3.6-flash`) are seeded; claude-agent-sdk serves `dashscope` (claude-only for now — Codex CLI ≥0.122 removed the chat wire API Bailian's compatible mode speaks; ADR-0013 amendment).
   - *Executor injection, project wins.* The engine materializes the project's key fresh per step (decrypted in the worker, redactor-registered before use, so rotation applies at the next step) and executors overlay it by replacing the wire protocol's entire auth-var family — Bailian lands as bearer `ANTHROPIC_AUTH_TOKEN` + gateway URL on claude; explicit `openai` proxy overrides land as a synthesized `-c model_providers` entry with a per-run `CODEX_HOME` on codex (so ambient `auth.json` can't outrank the project key). Codex now registers on a CLI probe alone; env auth at boot is no longer required.
   - *Base-URL overrides are policy-checked, and endpoint + key travel together.* A credential's baseUrl is where the worker sends the decrypted key, so overrides must be https, carry no userinfo/query, point at a public DNS name (never an IP literal or localhost — and the worker requires every resolved address to be global-unicast at each step; for IPv6 that is allowlist-first from `2000::/3`, so unallocated space never passes), and stay within the provider's pinned hosts (dashscope → the exact API hosts + `.maas.aliyuncs.com`, excluding customer-controlled OSS bucket domains) — rejected as `base_url_invalid` otherwise. Permanent non-resolution is the same actionable configuration failure; transient resolver errors remain queue-retryable. Changing the endpoint requires re-entering the key in the same request, so an existing write-only key can never be redirected; clearing back to the catalog default needs no key.
-  - *Role-scoped, single-provider slot resolution.* Slots resolve only the model roles their steps (and subagents) reference, and provider-constrained slots resolve all of them from one provider (credentialed provider first, then cheapest, deterministic) — a step's base URL is process-wide, so mixed-provider slots could never execute. A slot blocked only by a missing credential fails submission with the new `provider_credential_required` error — retries re-assert the same gate against the frozen resolution, and a credential deleted mid-run fails the run with the same code before the executor is invoked; demo/`fake` resolution is unchanged.
+  - *Role-scoped, single-provider slot resolution.* Slots resolve only the model roles their steps (and subagents) reference, and provider-constrained slots resolve all of them from one provider (credentialed provider first, then cheapest, deterministic) — a step's base URL is process-wide, so mixed-provider slots could never execute. A slot blocked only by a missing credential fails submission with the new `provider_credential_required` error — retries enforce the same gate through full re-resolution against current configuration (ADR-0014 superseded the original frozen-resolution re-check), and a credential deleted mid-run fails the run with the same code before the executor is invoked; demo/`fake` resolution is unchanged.
   - *Keyless workers defer instead of failing.* Executors advertise which providers their worker env can authenticate (`envAuthProviders`); a run whose providers neither a project credential nor the claiming worker's env covers is declined before any status transition (the existing `run.deferred` heterogeneous-fleet path), so it waits for a capable worker rather than burning mid-run. The decline applies only to the pre-claim states the worker can actually re-enqueue (`queued`/`waiting_approval`) — a crash-recovered running run proceeds and, if its auth is truly unusable (including an env-policy credential deleted after claim on a keyless worker), fails actionably per-step with `provider_credential_required` instead of a generic error. Preflight checks only credential-row/secret presence; endpoint validation and decryption happen after claim, where bad configuration fails actionably and infrastructure errors retry.
 
 - **`gpt-5.6-sol` joins the seeded model catalog** — the first seeded OpenAI model that works under a ChatGPT-account Codex login (`CODEX_HOME`); the existing `gpt-*-codex` ids are API-key-only and the backend 400s them under account auth, which left ChatGPT-login deployments with no runnable reviewer model. Context window and pricing follow OpenAI's model catalog (1,050,000 tokens, $5.00/$30.00 per MTok); a new CONTRIBUTING recipe covers adding builtin models.
@@ -64,6 +64,12 @@ All notable changes to Agrippa are documented here. The format follows
 
 ### Changed
 
+- **Task retry re-resolves against current configuration** ([ADR-0014](docs/adr/0014-retry-re-resolves-configuration.md)) — retry used to copy the previous run's frozen bindings verbatim, so a configuration failure (a model the auth mode can't serve, a revoked grant, a deleted credential) could never be healed by fixing the configuration and retrying: the retried run re-billed the whole pipeline from checkout and died identically. Retry is now a re-submission of the pinned task:
+  - *Template version and params snapshot stay pinned; everything configuration-derived re-resolves.* Agent bindings, per-slot model resolution, the resource manifest, the budget, quota headroom, and repo-ref ownership all re-derive through the same helper submit uses — with unchanged configuration the result is deterministically identical, so transient-failure retries behave as before. Configuration that still cannot satisfy a slot fails fast at the endpoint with submit's error codes (now localized, including `executor_unavailable`), never mid-run.
+  - *Submit-time agent overrides persist.* The `agents` slot overrides picked on the submit form land on `tasks.agent_overrides` (migration 0008) and re-apply at retry, so a deliberate executor choice survives re-resolution; tasks predating the column retry with template defaults.
+  - *`assertResolutionCredentialed` is retired* — the narrow credential re-check (ADR-0013 amendment 1) is subsumed by the full resolution gate. Retry also gained submit's transactional insert and an audit payload naming the source run.
+  - *The Retry button says what it does.* A confirm dialog states that the new run starts over from checkout under current configuration and re-asks earlier answers/approvals; rejections surface as localized toasts; the run-page actions are hidden from viewers, who could only collect a 403.
+  - *Review-round corrections (ADR-0014 amendment).* Status is configuration too: disabled MCP servers, skills without an active version satisfying their range (new `skill_version_unavailable`, matching via a helper shared with the worker's materializer), and disabled default Fabri now fail resolution instead of the run. Concurrent retries serialize on the task row (the loser gets 409 `run_active`, not a unique-violation 500), and submit/retry audit rows commit inside their mutation's transaction. `repo_not_in_project` joins both locales' error catalogs (retry surfaces it in a toast when the original repo connection was deleted), and the admin manuals document the skill-version and disabled-MCP semantics.
 - **`approvals` table → `checkpoints`** (data-preserving rename) with `kind`, `iteration`, and a structured `response`; `run_steps` and `artifacts` gain `iteration`; `runs` gain `agent_bindings` and `work_branch`; new `run_comments` table (migration `0005_requirement-delivery`). Engine events rename `approval.required/decided` → `checkpoint.required/decided` and add `loop.*`, `branch.*`, `pr.opened`, `comment.added`.
 - **Demo mode hardening** — when `AGRIPPA_EXECUTOR=fake`, every agent slot binds to the demo executor (a demo install must never silently route a slot to a real, key-consuming engine), `AGRIPPA_SCM=fake` fabricates branch/push/PR, and the demo executor is loop-round-aware (questions/findings in round 1, clean in round 2) so the delivery workflow demos end-to-end without tokens.
 

apps/api/src/routes/execution.ts

@@ -32,9 +32,9 @@ import {
 } from "@agrippa/db";
 import {
   appendRunEvent as allocateRunEvent,
-  assertResolutionCredentialed,
   authorizeResources,
   buildParamsValidator,
+  type CompiledTemplate,
   decideCheckpoint,
   flattenPhases,
   resolveAgentBindings,
@@ -54,6 +54,51 @@ import { assertProjectRole, requireProjectRole } from "../middleware/rbac";
 
 const DEFAULT_EXECUTOR = process.env.AGRIPPA_EXECUTOR ?? "claude-agent-sdk";
 
+/**
+ * Everything a new run derives from CURRENT project configuration: the
+ * resource manifest, per-slot agent bindings and model resolution, and the
+ * budget off the pinned compiled spec. Shared by submit and retry so the two
+ * paths cannot drift — a retry is a re-submission of the pinned task
+ * (ADR-0014); only the template version and params snapshot stay pinned.
+ */
+async function resolveRunPlan(
+  db: AppEnv["Variables"]["db"],
+  projectId: string,
+  taskType: { defaultFaberId: string },
+  compiled: CompiledTemplate,
+  overrides: Record<string, { executorId?: string; faberId?: string }>,
+) {
+  const skillRows = await db.select({ id: skills.id, slug: skills.slug }).from(skills);
+  // a disabled server is not "registered" — pinning it would only defer the
+  // failure to worker materialization (skill versions are checked downstream
+  // in authorizeResources; the skills head table has no status column)
+  const mcpRows = await db
+    .select({ id: mcpServers.id, slug: mcpServers.slug })
+    .from(mcpServers)
+    .where(eq(mcpServers.status, "active"));
+  // required grants enforced; optional resources pinned only when granted
+  const resourceManifest = await authorizeResources(db, projectId, compiled, {
+    skillIdBySlug: new Map(skillRows.map((s) => [s.slug, s.id])),
+    mcpIdBySlug: new Map(mcpRows.map((m) => [m.slug, m.id])),
+  });
+  // every agent slot resolves to a concrete faber + executor (per-slot
+  // provider-filtered models, checked against the executors the deployment's
+  // workers actually registered) and freezes onto the run
+  const agentResolution = await resolveAgentBindings(
+    db,
+    projectId,
+    compiled,
+    { faberId: taskType.defaultFaberId, executorId: DEFAULT_EXECUTOR },
+    overrides,
+    { registeredExecutors: await liveExecutorIds(db) },
+  );
+  return {
+    resourceManifest,
+    agentResolution,
+    budget: compiled.spec.budgets as unknown as Record<string, unknown>,
+  };
+}
+
 async function loadRunScoped(
   c: { var: AppEnv["Variables"] },
   runId: string,
@@ -308,25 +353,12 @@ export const executionRoutes = new Hono<AppEnv>()
         // every repoRef must reference a connection owned by this project
         await verifyRepoRefs(db, projectId, compiled.spec.inputs, parsed.data);
 
-        const skillRows = await db.select({ id: skills.id, slug: skills.slug }).from(skills);
-        const mcpRows = await db
-          .select({ id: mcpServers.id, slug: mcpServers.slug })
-          .from(mcpServers);
-        // required grants enforced; optional resources pinned only when granted
-        const resourceManifest = await authorizeResources(db, projectId, compiled, {
-          skillIdBySlug: new Map(skillRows.map((s) => [s.slug, s.id])),
-          mcpIdBySlug: new Map(mcpRows.map((m) => [m.slug, m.id])),
-        });
-        // every agent slot resolves to a concrete faber + executor here (with
-        // per-slot provider-filtered models and a check against the executors
-        // the deployment's workers actually registered) and freezes onto the run
-        const agentResolution = await resolveAgentBindings(
+        const { resourceManifest, agentResolution, budget } = await resolveRunPlan(
           db,
           projectId,
+          taskType,
           compiled,
-          { faberId: taskType.defaultFaberId, executorId: DEFAULT_EXECUTOR },
           input.agents ?? {},
-          { registeredExecutors: await liveExecutorIds(db) },
         );
 
         const { task, run } = await db.transaction(async (tx) => {
@@ -338,6 +370,7 @@ export const executionRoutes = new Hono<AppEnv>()
               taskTypeId: taskType.id,
               title: input.title,
               params: parsed.data,
+              agentOverrides: input.agents ?? {},
               createdBy: user.id,
             })
             .returning();
@@ -355,22 +388,28 @@ export const executionRoutes = new Hono<AppEnv>()
               paramsSnapshot: parsed.data,
               modelResolution: agentResolution.modelResolution,
               resourceManifest,
-              budget: compiled.spec.budgets as unknown as Record<string, unknown>,
+              budget,
               createdBy: user.id,
             })
             .returning();
           if (!run) throw new Error("run insert failed");
           await tx.update(tasks).set({ latestRunId: run.id }).where(eq(tasks.id, task.id));
+          // in the tx: a committed mutation without its audit row would break
+          // the every-mutation-is-audited invariant (ADR-0013 amendment 1)
+          await audit(
+            c,
+            {
+              action: "task.submit",
+              resourceType: "task",
+              resourceId: task.id,
+              projectId,
+              payload: { taskTypeId: taskType.id, runId: run.id },
+            },
+            tx,
+          );
           return { task, run };
         });
 
-        await audit(c, {
-          action: "task.submit",
-          resourceType: "task",
-          resourceId: task.id,
-          projectId,
-          payload: { taskTypeId: taskType.id, runId: run.id },
-        });
         // post-commit send; singleton key + worker sweeper make this loss-proof
         await c.var.queue?.enqueueRun(run.id);
         return c.json({ taskId: task.id, runId: run.id }, 202);
@@ -441,49 +480,90 @@ export const executionRoutes = new Hono<AppEnv>()
       throw AppError.conflict("run_active", "The latest run has not finished");
     }
 
-    // the frozen resolution is copied verbatim below, so re-assert that its
-    // project-only provider credentials still exist — a credential deleted
-    // since the last run must fail here, not as an auth error mid-run
+    // deliberately no taskType.enabled check: disabling blocks new catalog
+    // submissions, not retries of a task that already exists (ADR-0014)
+    const [taskType] = await db.select().from(taskTypes).where(eq(taskTypes.id, task.taskTypeId));
+    if (!taskType) throw AppError.notFound("Task type");
+    const [version] = await db
+      .select()
+      .from(templateVersions)
+      .where(eq(templateVersions.id, latest.templateVersionId));
+    if (!version) throw AppError.notFound("Template version");
+    const compiled = upgradeCompiledTemplate(version.compiled);
+
+    // a retry burns budget like any run — same hard-stop as submit
+    await assertQuotaHeadroom(db, task.projectId);
+
     try {
-      await assertResolutionCredentialed(
+      // ADR-0014: the template version and params snapshot stay pinned;
+      // everything derived from project configuration re-resolves, so a
+      // config fix (grants, registry, credentials) heals the task here
+      // instead of re-billing a full run into the same failure
+      await verifyRepoRefs(db, task.projectId, compiled.spec.inputs, latest.paramsSnapshot);
+      const { resourceManifest, agentResolution, budget } = await resolveRunPlan(
         db,
         task.projectId,
-        latest.modelResolution as Record<string, unknown>,
-        latest.agentBindings as Record<string, { executorId: string }> | null,
-        latest.executorId,
+        taskType,
+        compiled,
+        task.agentOverrides ?? {},
       );
+
+      const run = await db.transaction(async (tx) => {
+        // serialize concurrent retries on the task row: two racers would both
+        // compute number N+1 and the loser would die on runs_task_number_uq
+        // as a 500 — locked, the second sees the first's queued run instead
+        // and gets the run_active conflict. The pre-tx read above only sourced
+        // the pinned snapshot/version, identical across all runs of a task.
+        await tx.select({ id: tasks.id }).from(tasks).where(eq(tasks.id, task.id)).for("update");
+        const [head] = await tx
+          .select({ id: runs.id, number: runs.number, status: runs.status })
+          .from(runs)
+          .where(eq(runs.taskId, task.id))
+          .orderBy(desc(runs.number))
+          .limit(1);
+        if (!head) throw AppError.notFound("Run");
+        if (!isTerminalRunStatus(head.status)) {
+          throw AppError.conflict("run_active", "The latest run has not finished");
+        }
+        const [run] = await tx
+          .insert(runs)
+          .values({
+            taskId: task.id,
+            projectId: task.projectId,
+            number: head.number + 1,
+            templateVersionId: latest.templateVersionId,
+            faberId: agentResolution.primary.faberId,
+            executorId: agentResolution.primary.executorId,
+            agentBindings: agentResolution.bindings,
+            paramsSnapshot: latest.paramsSnapshot,
+            modelResolution: agentResolution.modelResolution,
+            resourceManifest,
+            budget,
+            createdBy: c.var.user.id,
+          })
+          .returning();
+        if (!run) throw new Error("run insert failed");
+        await tx.update(tasks).set({ latestRunId: run.id }).where(eq(tasks.id, task.id));
+        // in the tx: the every-mutation-is-audited invariant (ADR-0013 am. 1)
+        await audit(
+          c,
+          {
+            action: "task.retry",
+            resourceType: "run",
+            resourceId: run.id,
+            projectId: task.projectId,
+            payload: { fromRunId: head.id, taskId: task.id },
+          },
+          tx,
+        );
+        return run;
+      });
+      await c.var.queue?.enqueueRun(run.id);
+      return c.json({ runId: run.id, number: run.number }, 202);
     } catch (err) {
       if (err instanceof SubmitError) throw new AppError(err.code, 400, err.message, err.details);
       throw err;
     }
-
-    const [run] = await db
-      .insert(runs)
-      .values({
-        taskId: task.id,
-        projectId: task.projectId,
-        number: latest.number + 1,
-        templateVersionId: latest.templateVersionId, // pinned — retries never re-resolve
-        faberId: latest.faberId,
-        executorId: latest.executorId,
-        agentBindings: latest.agentBindings,
-        paramsSnapshot: latest.paramsSnapshot,
-        modelResolution: latest.modelResolution,
-        resourceManifest: latest.resourceManifest,
-        budget: latest.budget,
-        createdBy: c.var.user.id,
-      })
-      .returning();
-    if (!run) throw new Error("run insert failed");
-    await db.update(tasks).set({ latestRunId: run.id }).where(eq(tasks.id, task.id));
-    await audit(c, {
-      action: "task.retry",
-      resourceType: "run",
-      resourceId: run.id,
-      projectId: task.projectId,
-    });
-    await c.var.queue?.enqueueRun(run.id);
-    return c.json({ runId: run.id, number: run.number }, 202);
   })
 
   // ── Runs ────────────────────────────────────────────────────────────────────