diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a6fd755..d653ec7 100644 --- a/ARCHITECTURE.md +++ b/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 diff --git a/CHANGELOG.md b/CHANGELOG.md index fb801da..f868fca 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/apps/api/src/routes/execution.ts b/apps/api/src/routes/execution.ts index 47e6d84..a396c2a 100644 --- a/apps/api/src/routes/execution.ts +++ b/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, +) { + 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, + }; +} + async function loadRunScoped( c: { var: AppEnv["Variables"] }, runId: string, @@ -308,25 +353,12 @@ export const executionRoutes = new Hono() // 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() 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() paramsSnapshot: parsed.data, modelResolution: agentResolution.modelResolution, resourceManifest, - budget: compiled.spec.budgets as unknown as Record, + 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() 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, - latest.agentBindings as Record | 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 ──────────────────────────────────────────────────────────────────── diff --git a/apps/api/src/test/execution.integration.test.ts b/apps/api/src/test/execution.integration.test.ts index 9144c6e..34e91e8 100644 --- a/apps/api/src/test/execution.integration.test.ts +++ b/apps/api/src/test/execution.integration.test.ts @@ -1,6 +1,6 @@ import { beforeAll, describe, expect, it } from "bun:test"; import type { RunQueue } from "@agrippa/core"; -import { repoConnections, runs } from "@agrippa/db"; +import { auditLogs, fabri, mcpServers, repoConnections, runs, skillVersions } from "@agrippa/db"; import { FakeExecutor, type FakeStepBehavior } from "@agrippa/executor-core"; import { type EngineDeps, @@ -11,7 +11,7 @@ import { InProcessEventBus, silentLogger, } from "@agrippa/orchestration"; -import { eq } from "drizzle-orm"; +import { desc, eq } from "drizzle-orm"; import type { App } from "../app"; import { createApp } from "../app"; import { freshTestDb, jsonOf, postgresAvailable, signUp, type TestClient } from "./helpers"; @@ -422,4 +422,210 @@ describe.skipIf(!dbUp)("execution api (submit → engine → approve → artifac ); expect(task.runs.map((r) => r.number)).toEqual([2, 1]); }); + + it("retry re-resolves against current configuration (ADR-0014)", async () => { + // no worker in this fixture — mark queued runs terminal so retry is legal + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + const [prev] = await db + .select() + .from(runs) + .where(eq(runs.taskId, taskId)) + .orderBy(desc(runs.number)) + .limit(1); + const flatten = (resolution: Record | null | undefined) => + Object.values(resolution ?? {}).flatMap((slot) => + Object.values(slot as Record), + ); + // every run so far resolved anthropic — no dashscope credential existed + expect(flatten(prev?.modelResolution).some((e) => e.provider === "anthropic")).toBe(true); + + // adding a credential makes dashscope the ranked provider (ADR-0013's + // credentialed-first consequence) — a retry must pick that up + await admin.request(`/api/v1/projects/${projectId}/providers`, { + method: "POST", + json: { provider: "dashscope", apiKey: "sk-bailian-retry-test" }, + }); + const res = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(res.status).toBe(202); + const { runId: rerunId } = await jsonOf<{ runId: string }>(res); + const [rerun] = await db.select().from(runs).where(eq(runs.id, rerunId)); + + // pinned: template version and params snapshot; re-derived: the resolution + expect(rerun?.templateVersionId).toBe(prev?.templateVersionId as string); + expect(rerun?.paramsSnapshot).toEqual(prev?.paramsSnapshot ?? {}); + const entries = flatten(rerun?.modelResolution); + expect(entries.length).toBeGreaterThan(0); + for (const entry of entries) expect(entry.provider).toBe("dashscope"); + + await admin.request(`/api/v1/projects/${projectId}/providers/dashscope`, { + method: "DELETE", + }); + }); + + it("retry preserves the user's submit-time agent overrides", async () => { + const types = await jsonOf>( + await admin.request("/api/v1/scenarios/software-development/task-types"), + ); + const rdTypeId = types.find((t) => t.slug === "requirement-delivery")?.id as string; + const submitted = await admin.request(`/api/v1/projects/${projectId}/tasks`, { + method: "POST", + json: { + taskTypeId: rdTypeId, + title: "Override survives retry", + params: { requirement: "Do the thing", repo: { repoConnectionId } }, + // the template pins codex-cli for the reviewer; the user chose otherwise + agents: { reviewer: { executorId: "claude-agent-sdk" } }, + }, + }); + expect(submitted.status).toBe(202); + const { taskId: rdTaskId } = await jsonOf<{ taskId: string }>(submitted); + + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, rdTaskId)); + const retried = await admin.request(`/api/v1/tasks/${rdTaskId}/retry`, { method: "POST" }); + expect(retried.status).toBe(202); + const { runId: rerunId } = await jsonOf<{ runId: string }>(retried); + const [rerun] = await db.select().from(runs).where(eq(runs.id, rerunId)); + expect(rerun?.agentBindings.reviewer?.executorId).toBe("claude-agent-sdk"); + }); + + it("retry fails fast when current configuration cannot satisfy the template", async () => { + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + + // strip every grant — the manifest re-check fires before model resolution + await admin.request(`/api/v1/projects/${projectId}/grants`, { method: "PUT", json: [] }); + const ungranted = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(ungranted.status).toBe(400); + expect((await jsonOf<{ code: string }>(ungranted)).code).toBe("skill_not_granted"); + + const models = await jsonOf>(await admin.request("/api/v1/models")); + const skillRows = await jsonOf>(await admin.request("/api/v1/skills")); + await admin.request(`/api/v1/projects/${projectId}/grants`, { + method: "PUT", + json: [ + ...models.map((m) => ({ resourceType: "model", resourceId: m.id })), + ...skillRows.map((s) => ({ resourceType: "skill", resourceId: s.id })), + ], + }); + + // a retry burns budget like any run — the quota hard-stop applies + await admin.request(`/api/v1/projects/${projectId}/quota`, { + method: "PUT", + json: { tokenLimit: 1, hardStop: true }, + }); + const blocked = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(blocked.status).toBe(400); + expect((await jsonOf<{ code: string }>(blocked)).code).toBe("quota_exhausted"); + await admin.request(`/api/v1/projects/${projectId}/quota`, { + method: "PUT", + json: { tokenLimit: null, hardStop: true }, + }); + }); + + it("retry rejects a default faber that was disabled since submit", async () => { + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + const [latest] = await db + .select({ faberId: runs.faberId }) + .from(runs) + .where(eq(runs.taskId, taskId)) + .orderBy(desc(runs.number)) + .limit(1); + await db + .update(fabri) + .set({ status: "disabled" }) + .where(eq(fabri.id, latest?.faberId as string)); + const res = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(res.status).toBe(400); + expect((await jsonOf<{ code: string }>(res)).code).toBe("faber_unknown"); + await db + .update(fabri) + .set({ status: "active" }) + .where(eq(fabri.id, latest?.faberId as string)); + }); + + it("retry rejects a required skill whose versions were all deprecated", async () => { + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + await db.update(skillVersions).set({ status: "deprecated" }); + const res = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(res.status).toBe(400); + expect((await jsonOf<{ code: string }>(res)).code).toBe("skill_version_unavailable"); + await db.update(skillVersions).set({ status: "active" }); + }); + + it("concurrent retries serialize: one 202, one run_active conflict, audited", async () => { + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + const [first, second] = await Promise.all([ + admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }), + admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }), + ]); + const statuses = [first.status, second.status].sort(); + expect(statuses).toEqual([202, 409]); + const winner = first.status === 202 ? first : second; + const loser = first.status === 202 ? second : first; + expect((await jsonOf<{ code: string }>(loser)).code).toBe("run_active"); + + // the audit row commits with the retry (every mutation is audited) + const { runId: newRunId } = await jsonOf<{ runId: string }>(winner); + const rows = await db.select().from(auditLogs).where(eq(auditLogs.resourceId, newRunId)); + const entry = rows.find((r) => r.action === "task.retry"); + expect(entry).toBeDefined(); + const payload = entry?.payload as { fromRunId?: string } | undefined; + expect(payload?.fromRunId).toBeDefined(); + }); + + it("retry drops an optional MCP server disabled since submit", async () => { + // bug-localize-fix references MCP 'github' as optional — register + grant it + const [mcp] = await db + .insert(mcpServers) + .values({ + slug: "github", + nameI18n: { en: "GitHub", "zh-CN": "GitHub" }, + transport: "http", + config: { url: "https://mcp.example.com" }, + }) + .returning(); + const models = await jsonOf>(await admin.request("/api/v1/models")); + const skillRows = await jsonOf>(await admin.request("/api/v1/skills")); + await admin.request(`/api/v1/projects/${projectId}/grants`, { + method: "PUT", + json: [ + ...models.map((m) => ({ resourceType: "model", resourceId: m.id })), + ...skillRows.map((s) => ({ resourceType: "skill", resourceId: s.id })), + { resourceType: "mcp_server", resourceId: mcp?.id }, + ], + }); + + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + const withMcp = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(withMcp.status).toBe(202); + const { runId: withId } = await jsonOf<{ runId: string }>(withMcp); + const [runWith] = await db.select().from(runs).where(eq(runs.id, withId)); + expect(runWith?.resourceManifest.mcpServers).toContain("github"); + + // disabled → treated as unregistered; the optional ref silently drops + await db + .update(mcpServers) + .set({ status: "disabled" }) + .where(eq(mcpServers.id, mcp?.id as string)); + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + const withoutMcp = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(withoutMcp.status).toBe(202); + const { runId: withoutId } = await jsonOf<{ runId: string }>(withoutMcp); + const [runWithout] = await db.select().from(runs).where(eq(runs.id, withoutId)); + expect(runWithout?.resourceManifest.mcpServers).not.toContain("github"); + }); + + it("retry rejects params referencing a deleted repo connection", async () => { + const [conn] = await db + .select() + .from(repoConnections) + .where(eq(repoConnections.id, repoConnectionId)); + await db.delete(repoConnections).where(eq(repoConnections.id, repoConnectionId)); + await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, taskId)); + + const res = await admin.request(`/api/v1/tasks/${taskId}/retry`, { method: "POST" }); + expect(res.status).toBe(400); + expect((await jsonOf<{ code: string }>(res)).code).toBe("repo_not_in_project"); + + if (conn) await db.insert(repoConnections).values(conn); + }); }); diff --git a/apps/api/src/test/providers.integration.test.ts b/apps/api/src/test/providers.integration.test.ts index 19885d2..58843db 100644 --- a/apps/api/src/test/providers.integration.test.ts +++ b/apps/api/src/test/providers.integration.test.ts @@ -260,11 +260,12 @@ describe.skipIf(!dbUp)("provider credentials (project settings → submit gate)" gatedTaskId = taskId; }); - it("retry re-checks the credential gate against the frozen resolution", async () => { + it("retry re-checks the credential gate against current configuration", async () => { // no worker in this fixture — mark the queued run terminal so retry is legal await db.update(runs).set({ status: "failed" }).where(eq(runs.taskId, gatedTaskId)); - // deleting the credential must block retry, not fail mid-run on auth + // retry re-resolves in full (ADR-0014); with dashscope-only grants the + // deleted credential must block retry here, not fail mid-run on auth await admin.request(`/api/v1/projects/${gatedProjectId}/providers/dashscope`, { method: "DELETE", }); diff --git a/apps/web/src/pages/RunDetailPage.tsx b/apps/web/src/pages/RunDetailPage.tsx index dd6b6d0..86890c0 100644 --- a/apps/web/src/pages/RunDetailPage.tsx +++ b/apps/web/src/pages/RunDetailPage.tsx @@ -4,6 +4,7 @@ import { useNavigate, useParams } from "@tanstack/react-router"; import { ChevronDownIcon, RotateCcwIcon, XIcon } from "lucide-react"; import { useTranslation } from "react-i18next"; import { ArtifactPreview, isPreviewable } from "@/components/artifacts/ArtifactPreview"; +import { ConfirmDialog } from "@/components/ConfirmDialog"; import { FaberAvatar } from "@/components/FaberAvatar"; import { DetailSkeleton } from "@/components/LoadingSkeletons"; import { PageHeader } from "@/components/PageHeader"; @@ -22,6 +23,7 @@ import { useMe } from "../features/me"; import { useRunEvents } from "../features/useRunEvents"; import { api } from "../lib/api"; import { formatCost, formatDuration, formatTime, lt } from "../lib/format"; +import { toastApiError } from "../lib/toast"; import type { Artifact, Run, RunStep } from "../lib/types"; function ArtifactRow({ artifact }: { artifact: Artifact }) { @@ -100,6 +102,8 @@ export function RunDetailPage() { params: { projectId, runId: result.runId }, }); }, + // re-resolution can reject like submit (grants, credentials, quota…) + onError: toastApiError, }); if (run.isError) return void run.refetch()} />; @@ -146,16 +150,20 @@ export function RunDetailPage() { } actions={ - isTerminalRunStatus(current.status) ? ( - + // both endpoints require the member role — hide them from viewers + !canRespond ? null : isTerminalRunStatus(current.status) ? ( + + + {t("actions.retry")} + + } + title={t("actions.retryTitle")} + description={t("actions.retryWarning")} + confirmLabel={t("actions.retryConfirm")} + onConfirm={() => retry.mutate()} + /> ) : (