From d7f20695c7ea468b741537809ebe9ee11204add1 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 10:41:08 +0800 Subject: [PATCH 01/14] feat(workflows): retry agent nodes under their frozen policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the agent-node retry contract from specs decisions/cloud/workflow/ 20261021-node-retry-and-resume-contract.md (D1–D6, D8): - publishing validates agentConfig.retry with four named gate codes; absent or null keeps the default policy, and frozen snapshots are never re-gated - a retryable agent_failed session commits `retrying` with a database-clock retryAt, keeps its evidence and the run's serial session slot, while the run stays running - the scheduler starts the next attempt only when retryAt is due, through the ordinary session path and under the run's version compare-and-swap - migration 0062 gives work items, executions and commands a per-attempt identity, re-keying the session and end-request uniqueness so attempt 2 can register without reusing attempt 1 evidence or end request - cancellation wins over a waiting retry; budget exhaustion, a disabled policy and every non-agent_failed reason fail the node and release the run - the run UI shows retrying, the attempt number and the next retry instant, and offers no resume entry Refs ora-space/circus#469 Co-Authored-By: Claude Code --- .../workflows/run/run-inspector.test.tsx | 16 + .../features/workflows/run/run-inspector.tsx | 8 + .../workflows/run/run-node-state.test.ts | 30 ++ .../features/workflows/run/run-node-state.ts | 13 + .../workflows/run/run-status-mark.test.tsx | 8 + .../workflows/run/run-status-mark.tsx | 2 +- .../runtime/run-status-style.test.ts | 4 +- .../workflows/runtime/run-status-style.ts | 16 +- .../src/features/workflows/runtime/types.ts | 2 +- .../src/features/workflows/translations.ts | 6 + integration/migration_upgrade_path_test.go | 77 ++++ integration/workflow_admission_test.go | 7 +- ...ent_run_controlled_reclaim_race_db_test.go | 4 +- internal/core/agent_run_delivery.go | 2 +- internal/core/agent_run_session_settle.go | 2 +- internal/core/agent_run_session_start.go | 2 +- internal/core/control_projection_db_test.go | 8 +- internal/core/execution_work.go | 27 +- .../migrations/0062_workflow_node_attempt.sql | 39 ++ internal/core/node_execution.go | 17 +- internal/core/thread_commands.go | 45 +- internal/core/workflow_graph.go | 48 +- internal/core/workflow_graph_test.go | 107 +++++ internal/core/workflow_retry.go | 168 +++++++ internal/core/workflow_retry_db_test.go | 417 ++++++++++++++++++ internal/core/workflow_retry_test.go | 163 +++++++ internal/core/workflow_run_cancel.go | 30 +- internal/core/workflow_run_session.go | 88 +++- internal/core/workflow_run_session_db_test.go | 23 +- internal/core/workflow_scheduler.go | 72 ++- 30 files changed, 1364 insertions(+), 87 deletions(-) create mode 100644 internal/core/migrations/0062_workflow_node_attempt.sql create mode 100644 internal/core/workflow_retry.go create mode 100644 internal/core/workflow_retry_db_test.go create mode 100644 internal/core/workflow_retry_test.go diff --git a/frontend/src/features/workflows/run/run-inspector.test.tsx b/frontend/src/features/workflows/run/run-inspector.test.tsx index 1e5ddbc1..d6fde100 100644 --- a/frontend/src/features/workflows/run/run-inspector.test.tsx +++ b/frontend/src/features/workflows/run/run-inspector.test.tsx @@ -37,6 +37,22 @@ describe('RunInspector', () => { expect(within(output).getByText(/"exit": 2/)).toBeInTheDocument() }) + it('states the attempt and the next retry instant for a retrying node', () => { + const entry: RunNodeEntry = { + status: 'retrying', + attempt: 2, + startedAt: '2026-09-20T02:00:00.000Z', + retryAt: '2026-09-20T02:00:30.000Z', + error: 'agent_failed', + } + renderWithProviders() + + const execution = screen.getByRole('region', { name: '执行信息' }) + expect(within(execution).getByText('第几次尝试')).toBeInTheDocument() + expect(within(execution).getByText('2')).toBeInTheDocument() + expect(within(execution).getByText('下次重试')).toBeInTheDocument() + }) + it('reveals a trace error in a destructive block', () => { const entry: RunNodeEntry = { ...FINISHED_ENTRY, error: '命令退出码 2' } renderWithProviders() diff --git a/frontend/src/features/workflows/run/run-inspector.tsx b/frontend/src/features/workflows/run/run-inspector.tsx index 73fdc0f9..1c956c56 100644 --- a/frontend/src/features/workflows/run/run-inspector.tsx +++ b/frontend/src/features/workflows/run/run-inspector.tsx @@ -125,9 +125,17 @@ export function InspectorExecution({ entry }: { entry: RunNodeEntry | undefined return null } const rows: Array<[string, string]> = [] + if (entry.attempt !== undefined) { + rows.push([t('workflows.run.inspector.attempt'), String(entry.attempt)]) + } if (entry.startedAt !== undefined) { rows.push([t('workflows.run.inspector.startedAt'), formatClock(entry.startedAt)]) } + if (entry.retryAt !== undefined) { + // The retry instant is the only forward-looking row: it tells the reader when the + // node will try again instead of leaving a failed attempt looking final. + rows.push([t('workflows.run.inspector.retryAt'), formatClock(entry.retryAt)]) + } if (entry.finishedAt !== undefined) { rows.push([t('workflows.run.inspector.finishedAt'), formatClock(entry.finishedAt)]) } diff --git a/frontend/src/features/workflows/run/run-node-state.test.ts b/frontend/src/features/workflows/run/run-node-state.test.ts index 468474b5..21f70aa9 100644 --- a/frontend/src/features/workflows/run/run-node-state.test.ts +++ b/frontend/src/features/workflows/run/run-node-state.test.ts @@ -26,6 +26,36 @@ describe('parseRunNodeStates', () => { } satisfies RunNodeEntry) }) + it('reads the attempt and retry instant of a retrying node', () => { + const states = parseRunNodeStates({ + 'a-1': { + status: 'retrying', + attempt: 2, + startedAt: '2026-09-20T10:00:00+08:00', + retryAt: '2026-09-20T10:00:30+08:00', + error: 'agent_failed', + }, + }) + expect(states['a-1']).toEqual({ + status: 'retrying', + attempt: 2, + startedAt: '2026-09-20T10:00:00+08:00', + retryAt: '2026-09-20T10:00:30+08:00', + error: 'agent_failed', + } satisfies RunNodeEntry) + }) + + it('drops an attempt that is not a positive integer and a blank retry instant', () => { + const states = parseRunNodeStates({ + 'a-1': { status: 'retrying', attempt: 0, retryAt: '' }, + 'a-2': { status: 'running', attempt: 1.5 }, + 'a-3': { status: 'running', attempt: 'two' }, + }) + expect(states['a-1']).toEqual({ status: 'retrying' } satisfies RunNodeEntry) + expect(states['a-2']).toEqual({ status: 'running' } satisfies RunNodeEntry) + expect(states['a-3']).toEqual({ status: 'running' } satisfies RunNodeEntry) + }) + it('keeps an explicitly falsy output when the row carries one', () => { const states = parseRunNodeStates({ 'a-1': { status: 'failed', output: 0 } }) expect(states['a-1']?.output).toBe(0) diff --git a/frontend/src/features/workflows/run/run-node-state.ts b/frontend/src/features/workflows/run/run-node-state.ts index c6244207..4a0e443a 100644 --- a/frontend/src/features/workflows/run/run-node-state.ts +++ b/frontend/src/features/workflows/run/run-node-state.ts @@ -13,6 +13,10 @@ export interface RunNodeEntry { displayName?: string startedAt?: string finishedAt?: string + /** The 1-based attempt this row describes; absent for a node that never started. */ + attempt?: number + /** When the next attempt becomes due; present only while the node is retrying. */ + retryAt?: string error?: string /** Whatever the node produced; rendered as text by the inspector. */ output?: unknown @@ -25,6 +29,7 @@ const NODE_STATUSES = new Set([ 'succeeded', 'failed', 'cancelled', + 'retrying', ]) /** @@ -71,6 +76,14 @@ function readNodeEntry(value: unknown): RunNodeEntry | undefined { if (finishedAt !== undefined) { entry.finishedAt = finishedAt } + const attempt = value['attempt'] + if (typeof attempt === 'number' && Number.isInteger(attempt) && attempt >= 1) { + entry.attempt = attempt + } + const retryAt = readTimestamp(value['retryAt']) + if (retryAt !== undefined) { + entry.retryAt = retryAt + } const error = value['error'] if (typeof error === 'string') { entry.error = error diff --git a/frontend/src/features/workflows/run/run-status-mark.test.tsx b/frontend/src/features/workflows/run/run-status-mark.test.tsx index 03f4c7a1..a41ade1b 100644 --- a/frontend/src/features/workflows/run/run-status-mark.test.tsx +++ b/frontend/src/features/workflows/run/run-status-mark.test.tsx @@ -13,6 +13,7 @@ type RunStatus = GraphWorkflowRunStatus | GraphWorkflowNodeStatus const ZHU_LABELS: ReadonlyArray<[RunStatus, string]> = [ ['pending', '等待中'], ['running', '运行中'], + ['retrying', '重试中'], ['succeeded', '成功'], ['failed', '失败'], ['cancelled', '已取消'], @@ -42,6 +43,13 @@ describe('RunStatusMark', () => { unmount() }) + it('spins a working cue while a node waits for its next retry', () => { + const { unmount } = renderWithProviders() + expect(svgCount()).toBe(1) + unmount() + }) + + it('keeps a plain dot for a live-worthy-looking but non-working status', () => { const { unmount } = renderWithProviders() expect(svgCount()).toBe(0) diff --git a/frontend/src/features/workflows/run/run-status-mark.tsx b/frontend/src/features/workflows/run/run-status-mark.tsx index 61f550d5..9ecaaccb 100644 --- a/frontend/src/features/workflows/run/run-status-mark.tsx +++ b/frontend/src/features/workflows/run/run-status-mark.tsx @@ -44,7 +44,7 @@ export function RunStatusMark({ className={cn( 'inline-flex shrink-0 items-center justify-center rounded-full text-white', ICON_BOX, - 'bg-sky-500', + tone.dot, className, )} aria-hidden diff --git a/frontend/src/features/workflows/runtime/run-status-style.test.ts b/frontend/src/features/workflows/runtime/run-status-style.test.ts index ef798a2a..3035ce26 100644 --- a/frontend/src/features/workflows/runtime/run-status-style.test.ts +++ b/frontend/src/features/workflows/runtime/run-status-style.test.ts @@ -7,8 +7,9 @@ import { } from '@/features/workflows/runtime/types' describe('isNodeWorking', () => { - it('marks the executing status as the only live one', () => { + it('marks executing and retrying statuses as live', () => { expect(isNodeWorking('running')).toBe(true) + expect(isNodeWorking('retrying')).toBe(true) }) it('is quiet for every awaited or settled status', () => { @@ -25,6 +26,7 @@ describe('runStatusTone', () => { [ ['pending', 'workflows.run.status.pending'], ['running', 'workflows.run.status.running'], + ['retrying', 'workflows.run.status.retrying'], ['succeeded', 'workflows.run.status.succeeded'], ['failed', 'workflows.run.status.failed'], ['cancelled', 'workflows.run.status.cancelled'], diff --git a/frontend/src/features/workflows/runtime/run-status-style.ts b/frontend/src/features/workflows/runtime/run-status-style.ts index 767b75d9..ca1b9b54 100644 --- a/frontend/src/features/workflows/runtime/run-status-style.ts +++ b/frontend/src/features/workflows/runtime/run-status-style.ts @@ -3,9 +3,12 @@ import type { GraphWorkflowRunStatus, } from '@/features/workflows/runtime/types' -/** Node is actively executing — the only spinner-worthy state. */ +/** + * Node owns the run's live progress: executing, or waiting for its next retry. These are the + * only spinner-worthy states — the run has not moved on from them. + */ export function isNodeWorking(status: GraphWorkflowRunStatus | GraphWorkflowNodeStatus): boolean { - return status === 'running' + return status === 'running' || status === 'retrying' } /** Shared run/node status chrome — color is never the only signal (dot + label + tone). */ @@ -23,6 +26,15 @@ export function runStatusTone(status: GraphWorkflowRunStatus | GraphWorkflowNode badge: 'border-sky-500/30 bg-sky-500/10 text-sky-800 dark:text-sky-300', labelKey: 'workflows.run.status.running', } + case 'retrying': + // Orange reads as a transient warning distinct from the amber of `pending`: the node + // failed an attempt, but the run will carry on by itself. + return { + dot: 'bg-orange-500', + ring: 'border-orange-500/50 ring-orange-500/20', + badge: 'border-orange-500/35 bg-orange-500/10 text-orange-900 dark:text-orange-200', + labelKey: 'workflows.run.status.retrying', + } case 'succeeded': return { dot: 'bg-emerald-500', diff --git a/frontend/src/features/workflows/runtime/types.ts b/frontend/src/features/workflows/runtime/types.ts index ebfe67fb..253036cf 100644 --- a/frontend/src/features/workflows/runtime/types.ts +++ b/frontend/src/features/workflows/runtime/types.ts @@ -351,7 +351,7 @@ export function isTerminalRunStatus(status: GraphWorkflowRunStatus): boolean { /** Per-node execution status overlaid on a frozen definition snapshot. */ export type GraphWorkflowNodeStatus = - 'idle' | 'inactive' | 'running' | 'succeeded' | 'failed' | 'cancelled' + 'idle' | 'inactive' | 'running' | 'succeeded' | 'failed' | 'cancelled' | 'retrying' /** HITL timeout policy; MVP mock always waits (`wait`) until submit. */ export type HitlTimeoutPolicy = 'fail' | 'skip' | 'wait' diff --git a/frontend/src/features/workflows/translations.ts b/frontend/src/features/workflows/translations.ts index 08a89c8a..3fcb2855 100644 --- a/frontend/src/features/workflows/translations.ts +++ b/frontend/src/features/workflows/translations.ts @@ -383,6 +383,7 @@ export const workflowTranslations = { // Run viewer: run-level lifecycle and the read-only Overview surfaces. 'workflows.run.status.pending': '等待中', 'workflows.run.status.running': '运行中', + 'workflows.run.status.retrying': '重试中', 'workflows.run.status.succeeded': '成功', 'workflows.run.status.failed': '失败', 'workflows.run.status.cancelled': '已取消', @@ -425,7 +426,9 @@ export const workflowTranslations = { 'workflows.run.inspector.config': '配置', 'workflows.run.inspector.unconfigured': '该节点没有可展示的配置。', 'workflows.run.inspector.execution': '执行信息', + 'workflows.run.inspector.attempt': '第几次尝试', 'workflows.run.inspector.startedAt': '开始', + 'workflows.run.inspector.retryAt': '下次重试', 'workflows.run.inspector.finishedAt': '结束', 'workflows.run.inspector.output': '输出', 'workflows.run.inspector.noOutput': '该节点没有可展示的输出。', @@ -814,6 +817,7 @@ export const workflowTranslations = { // Run viewer: run-level lifecycle and the read-only Overview surfaces. 'workflows.run.status.pending': 'Pending', 'workflows.run.status.running': 'Running', + 'workflows.run.status.retrying': 'Retrying', 'workflows.run.status.succeeded': 'Succeeded', 'workflows.run.status.failed': 'Failed', 'workflows.run.status.cancelled': 'Cancelled', @@ -859,7 +863,9 @@ export const workflowTranslations = { 'workflows.run.inspector.config': 'Configuration', 'workflows.run.inspector.unconfigured': 'This node has no recorded configuration.', 'workflows.run.inspector.execution': 'Execution', + 'workflows.run.inspector.attempt': 'Attempt', 'workflows.run.inspector.startedAt': 'Started', + 'workflows.run.inspector.retryAt': 'Next retry', 'workflows.run.inspector.finishedAt': 'Finished', 'workflows.run.inspector.output': 'Output', 'workflows.run.inspector.noOutput': 'This node has no recorded output.', diff --git a/integration/migration_upgrade_path_test.go b/integration/migration_upgrade_path_test.go index b2f96d25..1320edb5 100644 --- a/integration/migration_upgrade_path_test.go +++ b/integration/migration_upgrade_path_test.go @@ -1735,3 +1735,80 @@ func TestMigration0055WorkflowRunWorkspaceReleaseAppliesFreshAndUpgrades(t *test refuse(`UPDATE workflow_runs SET workspace_release_state='abandoned', workspace_release_error=' ' WHERE id=$1`, ids["running"]) refuse(`UPDATE workflow_runs SET workspace_release_state='settled' WHERE id=$1`, ids["running"]) } + +// 0062 gives workflow node sessions and end requests a per-attempt identity (retry decision D4). The +// columns are NOT NULL with default 1 because every row that exists today belongs to its node's first +// attempt; nothing is backfilled. The uniqueness of 0041/0049 is re-keyed on the attempt, so a second +// attempt of the same node can register while the same attempt still cannot register twice. A fresh +// database and one that stopped at 0061 must reach the same shape, and rerunning the migration must +// change nothing. +func TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades(t *testing.T) { + tables := []string{"execution_work", "node_executions", "thread_commands"} + columnShape := func(t *testing.T, pool *sql.DB, table string) (int, string) { + t.Helper() + var n int + var nullable, dflt sql.NullString + must(t, pool.QueryRow(`SELECT count(*), max(is_nullable), max(column_default) + FROM information_schema.columns WHERE table_schema=current_schema() + AND table_name=$1 AND column_name='workflow_node_attempt'`, table).Scan(&n, &nullable, &dflt)) + return n, nullable.String + "|" + dflt.String + } + assertAttemptColumns := func(t *testing.T, pool *sql.DB, stage string) { + t.Helper() + for _, table := range tables { + n, shape := columnShape(t, pool, table) + if n != 1 || shape != "NO|1" { + t.Fatalf("%s: %s.workflow_node_attempt columns %d nullable/default %q; want one NOT NULL DEFAULT 1", stage, table, n, shape) + } + } + // The uniqueness is re-keyed on the attempt, and stays partial: IssueRun sessions (no graph + // node) keep the lifetime rule of their own index, and IssueRun end commands keep none. + for _, index := range []string{"one_session_per_workflow_node", "one_workflow_session_end"} { + var def string + must(t, pool.QueryRow(`SELECT indexdef FROM pg_indexes + WHERE schemaname=current_schema() AND indexname=$1`, index).Scan(&def)) + if !strings.Contains(def, "workflow_node_attempt") || !strings.Contains(def, "WHERE") { + t.Fatalf("%s: %s = %q; want a partial unique index keyed on the attempt", stage, index, def) + } + } + } + + freshPool, _ := testSchema(t, "test_wfna62f_") + migratedTwice(t, newStoreOnSchema(t, freshPool)) + assertAttemptColumns(t, freshPool, "fresh") + + pool, _ := testSchema(t, "test_wfna62_") + applyMigrationsUpTo(t, pool, migrationsBefore(t, "0062_workflow_node_attempt.sql")) + for _, table := range tables { + if n, _ := columnShape(t, pool, table); n != 0 { + t.Fatalf("before 0062: %s.workflow_node_attempt already exists", table) + } + } + // One session from before the migration. Its result is committed because 0024's pending-execution + // uniqueness admits only one open agent_session per operation; two rows coexist only as terminal + // facts. + operation := uuid.NewString() + insertSession := func(execution, attempt string) error { + _, e := pool.Exec(`INSERT INTO node_executions(execution_id,kind,operation_id,node_id,node_operation_id,input,result,dispatched_epoch,workflow_node_id,workflow_node_attempt) + VALUES($1,'agent_session',$2,'node','node-op','{}','{"outcome":"agent_session_ended","reason":"agent_failed"}',1,'a',$3)`, execution, operation, attempt) + return e + } + _, err := pool.Exec(`INSERT INTO node_executions(execution_id,kind,operation_id,node_id,node_operation_id,input,result,dispatched_epoch,workflow_node_id) + VALUES('legacy-session','agent_session',$1,'node','node-op','{}','{"outcome":"agent_session_ended","reason":"agent_failed"}',1,'a')`, operation) + must(t, err) + + migratedTwice(t, newStoreOnSchema(t, pool)) + assertAttemptColumns(t, pool, "upgrade") + var legacyAttempt int64 + must(t, pool.QueryRow(`SELECT workflow_node_attempt FROM node_executions WHERE execution_id='legacy-session'`).Scan(&legacyAttempt)) + if legacyAttempt != 1 { + t.Fatalf("legacy session reads attempt %d; want the default 1", legacyAttempt) + } + // A retry can register its own session for the same node, and the same attempt still cannot. + if err := insertSession("second-attempt", "2"); err != nil { + t.Fatalf("attempt 2 of the same node was refused: %v", err) + } + if err := insertSession("duplicate-attempt", "1"); err == nil { + t.Fatal("a second session for attempt 1 was accepted; want the re-keyed uniqueness to refuse it") + } +} diff --git a/integration/workflow_admission_test.go b/integration/workflow_admission_test.go index 73b4314e..db44aa5e 100644 --- a/integration/workflow_admission_test.go +++ b/integration/workflow_admission_test.go @@ -186,11 +186,11 @@ func TestWorkflowPublishRefusesUnusedAgentConfiguration(t *testing.T) { "name": "Configured flow", "graph": agentRunGraph("", false, nil), }, "adm-cfg-create", 200).O("resource") - // The graph the editor would write for an author who configured retry and skills: both are in the - // canonical schema, and neither is in the first phase's executable subset. + // The graph the editor would write for an author who configured skills: it is in the canonical + // schema but not in Cloud's executable subset. Retry is no longer part of this set — it is + // executed (retry decision D1) and its shape has its own gate tests. configured := agentRunGraph("", false, nil) config := configured["nodes"].([]any)[1].(core.Object).O("data").O("agentConfig") - config["retry"] = core.Object{"max": 3} config["skills"] = []any{"review"} live := f.call("GET", f.path("/workflows/"+draft.S("id")), nil, "", 200) f.call("PUT", f.path("/workflows/"+draft.S("id")), core.Object{ @@ -200,7 +200,6 @@ func TestWorkflowPublishRefusesUnusedAgentConfiguration(t *testing.T) { refused := f.call("POST", f.path("/workflows/"+draft.S("id")+"/publish"), core.Object{}, "adm-cfg-pub", 400) want := []core.Object{ {"code": "node_config_unsupported", "nodeId": "agent-1", "field": "skills"}, - {"code": "node_config_unsupported", "nodeId": "agent-1", "field": "retry"}, } if got := problems(t, refused); !reflect.DeepEqual(got, want) { t.Fatalf("publish problems = %v, want %v", got, want) diff --git a/internal/core/agent_run_controlled_reclaim_race_db_test.go b/internal/core/agent_run_controlled_reclaim_race_db_test.go index 226750fc..19d91449 100644 --- a/internal/core/agent_run_controlled_reclaim_race_db_test.go +++ b/internal/core/agent_run_controlled_reclaim_race_db_test.go @@ -240,7 +240,7 @@ func TestControlledResidualRaceNoTargetProofAndDispatchBothCommitOrders(t *testi return controlledForceConfirm(tx, &ControlRequest{Action: "force_confirm", Body: Object{"epoch": 1, "version": current.N("version")}}, current) } producer := func(tx *transaction) Object { - return enqueueExecutionWork(tx, scene.run, "agent_session", Object{"kind": "agent_session"}, Object{"workspaceId": scene.workspace, "sandboxInstanceId": newID(), "nodeId": "late-producer"}, "", time.Time{}) + return enqueueExecutionWork(tx, scene.run, "agent_session", Object{"kind": "agent_session"}, Object{"workspaceId": scene.workspace, "sandboxInstanceId": newID(), "nodeId": "late-producer"}, "", 0, time.Time{}) } first, second := producer, proof if proofFirst { @@ -274,7 +274,7 @@ func TestControlledResidualRaceNoTargetProofAndDispatchBothCommitOrders(t *testi r := seedControlledRequest(scene, "legacy-dispatch-race") controlled := func(tx *transaction) Object { return manageResidualCommand(tx, &r) } producer := func(tx *transaction) Object { - return enqueueExecutionWork(tx, scene.run, "agent_session", Object{"kind": "agent_session"}, Object{"workspaceId": scene.workspace, "sandboxInstanceId": scene.effect, "runtimeGeneration": 1, "nodeId": "legacy-node"}, "", time.Time{}) + return enqueueExecutionWork(tx, scene.run, "agent_session", Object{"kind": "agent_session"}, Object{"workspaceId": scene.workspace, "sandboxInstanceId": scene.effect, "runtimeGeneration": 1, "nodeId": "legacy-node"}, "", 0, time.Time{}) } first, second := producer, controlled if controlledFirst { diff --git a/internal/core/agent_run_delivery.go b/internal/core/agent_run_delivery.go index 5bf4a281..ca961a73 100644 --- a/internal/core/agent_run_delivery.go +++ b/internal/core/agent_run_delivery.go @@ -187,6 +187,6 @@ func (s *Store) settleDelivery(t *transaction, runID, executionID string, settle JOIN execution_work w ON w.id = e.work_id WHERE w.run_id=$1 AND w.kind='deliver_revision'`, runID).N("n") availableAt := time.Now().UTC().Add(deliveryRetryBackoff(int(attempts))) - enqueueExecutionWork(t, runID, "deliver_revision", spec, sessionStartTarget(t, o.S("workspaceId")), "", availableAt) + enqueueExecutionWork(t, runID, "deliver_revision", spec, sessionStartTarget(t, o.S("workspaceId")), "", 0, availableAt) return nil } diff --git a/internal/core/agent_run_session_settle.go b/internal/core/agent_run_session_settle.go index 86f131fe..e4e8b19c 100644 --- a/internal/core/agent_run_session_settle.go +++ b/internal/core/agent_run_session_settle.go @@ -164,7 +164,7 @@ func (s *Store) settleSessionEnded(t *transaction, runID, executionID string, en if err != nil { return err } - enqueueExecutionWork(t, runID, "deliver_revision", spec, sessionStartTarget(t, o.S("workspaceId")), "", time.Time{}) + enqueueExecutionWork(t, runID, "deliver_revision", spec, sessionStartTarget(t, o.S("workspaceId")), "", 0, time.Time{}) // Last, so lastSeq is the high-water mark this transaction is about to commit. `ended` is a // state-only change with a cleared idleSince, and the discards change per-turn status: both diff --git a/internal/core/agent_run_session_start.go b/internal/core/agent_run_session_start.go index 611a7572..3036223c 100644 --- a/internal/core/agent_run_session_start.go +++ b/internal/core/agent_run_session_start.go @@ -92,7 +92,7 @@ func (s *Store) startAgentSession(t *transaction, runID string) error { if err != nil { return err } - enqueueExecutionWork(t, runID, "agent_session", spec, sessionStartTarget(t, wid), "", time.Time{}) + enqueueExecutionWork(t, runID, "agent_session", spec, sessionStartTarget(t, wid), "", 0, time.Time{}) return nil } diff --git a/internal/core/control_projection_db_test.go b/internal/core/control_projection_db_test.go index c0c6e9cd..0526dfe6 100644 --- a/internal/core/control_projection_db_test.go +++ b/internal/core/control_projection_db_test.go @@ -11,11 +11,13 @@ import ( // // workflow_node_id is the graph node a session ran for. It is denormalized onto the row (migration // 0041) so the one_session_per_workflow_node partial index has a predicate to test and so the -// scheduler can attribute a result without joining through execution_work. No Controller contract -// declares it: the proto ExecutionRecord has no such field, and the JSON binding's NodeExecution +// scheduler can attribute a result without joining through execution_work. workflow_node_attempt is +// the node's attempt (migration 0061), denormalized the same way for the same reasons. No Controller +// contract declares either: the proto ExecutionRecord has none, and the JSON binding's NodeExecution // schema is closed. var nodeExecutionInternalColumns = map[string]bool{ - "workflow_node_id": true, + "workflow_node_id": true, + "workflow_node_attempt": true, } // TestControllerExecutionProjectionIsExplicit is the regression guard for the contract break diff --git a/internal/core/execution_work.go b/internal/core/execution_work.go index dd611f1e..1490a7e0 100644 --- a/internal/core/execution_work.go +++ b/internal/core/execution_work.go @@ -15,18 +15,20 @@ const executionWorkLiveRuns = "SELECT id FROM issue_runs WHERE deleted_at IS NUL // Repeating the same unregistered input returns the original item. A different input conflicts. func (s *Store) EnqueueExecutionWork(ctx context.Context, runID, kind string, input, target Object, availableAt time.Time) (Object, error) { return s.transact(ctx, func(t *transaction) Object { - return enqueueExecutionWork(t, runID, kind, input, target, "", availableAt) + return enqueueExecutionWork(t, runID, kind, input, target, "", 0, availableAt) }) } // graphNode names the workflow graph node a WorkflowRun's session work belongs to, so the scheduler -// can read that node's own execution back (workflowAgentSessionOutcome). IssueRun work has no graph -// and passes "". -func enqueueExecutionWork(t *transaction, runID, kind string, input, target Object, graphNode string, availableAt time.Time) Object { +// can read that node's own execution back (workflowAgentSessionOutcome), and graphAttempt names the +// node's attempt: one node has one work item per attempt, so the attempt is part of the item's +// identity the same way the node id is (retry decision D4). IssueRun work has no graph and passes +// "" and zero; a workflow item without a node (the public seam) is attempt 1. +func enqueueExecutionWork(t *transaction, runID, kind string, input, target Object, graphNode string, graphAttempt int64, availableAt time.Time) Object { require(validID(runID), 400, "invalid_dispatch") run := t.one("SELECT * FROM issue_runs WHERE id=$1 AND deleted_at IS NULL", runID) if run == nil { - return enqueueWorkflowExecutionWork(t, runID, kind, input, target, graphNode, availableAt) + return enqueueWorkflowExecutionWork(t, runID, kind, input, target, graphNode, graphAttempt, availableAt) } require(t.one("SELECT run_id FROM agent_run_residuals WHERE run_id=$1", runID) == nil, 409, "dispatch_conflict") requireNoControlledReclamation(t, target.S("workspaceId"), "") @@ -84,22 +86,27 @@ func enqueueExecutionWork(t *transaction, runID, kind string, input, target Obje // running at a time but several over its life (D4), and the scheduler reads the node's terminal // evidence back through this link. Repeating the same unregistered input returns the original item; // a different input conflicts. -func enqueueWorkflowExecutionWork(t *transaction, runID, kind string, input, target Object, graphNode string, availableAt time.Time) Object { +func enqueueWorkflowExecutionWork(t *transaction, runID, kind string, input, target Object, graphNode string, graphAttempt int64, availableAt time.Time) Object { require(t.one("SELECT id FROM workflow_runs WHERE id=$1 AND deleted_at IS NULL", runID) != nil, 404, "not_found") require(kind == "agent_session", 400, "invalid_dispatch") require(input.S("kind") == kind, 400, "invalid_dispatch") require(target.S("workspaceId") != "" && target.S("sandboxInstanceId") != "" && target.S("nodeId") != "", 400, "invalid_dispatch") + attempt := graphAttempt + if attempt < 1 { + attempt = 1 + } if existing := t.one("SELECT * FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL", runID); existing != nil { - require(existing.S("kind") == kind && existing.S("workflowNodeId") == graphNode && jsonText(existing.O("input")) == jsonText(input) && jsonText(existing.O("target")) == jsonText(target), 409, "dispatch_conflict") + require(existing.S("kind") == kind && existing.S("workflowNodeId") == graphNode && existing.N("workflowNodeAttempt") == attempt && jsonText(existing.O("input")) == jsonText(input) && jsonText(existing.O("target")) == jsonText(target), 409, "dispatch_conflict") return existing } id := newID() // Work released without a graph node (the public seam) stores NULL, not the empty string: the - // column's CHECK and the scheduler's node-scoped lookups both read "no node" as NULL. + // column's CHECK and the scheduler's node-scoped lookups both read "no node" as NULL. The attempt + // is written explicitly and never derived from the wire: it is the scheduler's own counter. if availableAt.IsZero() { - t.exec("INSERT INTO execution_work(id,workflow_run_id,kind,input,target,workflow_node_id) VALUES($1,$2,$3,$4,$5,NULLIF($6,''))", id, runID, kind, jsonText(input), jsonText(target), graphNode) + t.exec("INSERT INTO execution_work(id,workflow_run_id,kind,input,target,workflow_node_id,workflow_node_attempt) VALUES($1,$2,$3,$4,$5,NULLIF($6,''),$7)", id, runID, kind, jsonText(input), jsonText(target), graphNode, attempt) } else { - t.exec("INSERT INTO execution_work(id,workflow_run_id,kind,input,target,workflow_node_id,available_at) VALUES($1,$2,$3,$4,$5,NULLIF($6,''),$7)", id, runID, kind, jsonText(input), jsonText(target), graphNode, availableAt) + t.exec("INSERT INTO execution_work(id,workflow_run_id,kind,input,target,workflow_node_id,workflow_node_attempt,available_at) VALUES($1,$2,$3,$4,$5,NULLIF($6,''),$7,$8)", id, runID, kind, jsonText(input), jsonText(target), graphNode, attempt, availableAt) } t.workSignals = append(t.workSignals, runID) return t.one("SELECT * FROM execution_work WHERE id=$1", id) diff --git a/internal/core/migrations/0062_workflow_node_attempt.sql b/internal/core/migrations/0062_workflow_node_attempt.sql new file mode 100644 index 00000000..ae05dba6 --- /dev/null +++ b/internal/core/migrations/0062_workflow_node_attempt.sql @@ -0,0 +1,39 @@ +-- Per-attempt identity for workflow graph nodes (specs decisions/cloud/workflow +-- 20261021-node-retry-and-resume-contract.md D4). +-- +-- Retrying an agent node means one (run, node) pair owns several agent_session executions over the +-- run's life. Two identities assumed one execution per node and would refuse every attempt after the +-- first, or serve the first attempt's end request to the second attempt's session: +-- +-- * one_session_per_workflow_node (0041) allowed one agent_session per (run, node) for the run's +-- lifetime, so attempt 2's dispatch could never register; +-- * one_workflow_session_end (0049) allowed one end_session command per (run, node), so attempt 2 +-- could neither release its own end request nor be sure the first attempt's request would not be +-- delivered to it. +-- +-- Both are re-keyed on the attempt. The attempt travels the same path workflow_node_id already +-- travels (0041): the scheduler records it on the work item, the dispatch copies it onto the +-- execution, and the command names the attempt whose session it addresses. It is Cloud-internal +-- routing like the node id: the Controller wire shape is unchanged (root decision D8). +-- +-- Existing rows all default to 1 — every execution and end request that exists today belongs to a +-- node's first attempt, and IssueRun rows have no attempts at all. Nothing is backfilled, and the +-- columns are NOT NULL with that default so a writer that does not name one keeps the old meaning. +-- Additive and forward-only. + +ALTER TABLE execution_work ADD COLUMN workflow_node_attempt integer NOT NULL DEFAULT 1 + CHECK (workflow_node_attempt >= 1); +ALTER TABLE node_executions ADD COLUMN workflow_node_attempt integer NOT NULL DEFAULT 1 + CHECK (workflow_node_attempt >= 1); +ALTER TABLE thread_commands ADD COLUMN workflow_node_attempt integer NOT NULL DEFAULT 1 + CHECK (workflow_node_attempt >= 1); + +DROP INDEX one_session_per_workflow_node; +CREATE UNIQUE INDEX one_session_per_workflow_node + ON node_executions(operation_id, workflow_node_id, workflow_node_attempt) + WHERE kind='agent_session' AND workflow_node_id IS NOT NULL; + +DROP INDEX one_workflow_session_end; +CREATE UNIQUE INDEX one_workflow_session_end + ON thread_commands(workflow_run_id, workflow_node_id, workflow_node_attempt) + WHERE kind='end_session' AND workflow_run_id IS NOT NULL; diff --git a/internal/core/node_execution.go b/internal/core/node_execution.go index 1c4ed684..7188e00d 100644 --- a/internal/core/node_execution.go +++ b/internal/core/node_execution.go @@ -1,10 +1,11 @@ package core // nodeExecutionColumns is the Controller-facing projection of node_executions: every column of the -// table except workflow_node_id. That column is Cloud-internal routing — which graph node a session -// ran for — and no Controller contract has a field for it: the proto ExecutionRecord declares none, -// and the JSON binding's NodeExecution schema is closed, so a `SELECT *` that carried the column -// failed the response contract the moment migration 0041 added it. +// table except workflow_node_id and workflow_node_attempt. Those columns are Cloud-internal routing — +// which graph node a session ran for, and which attempt of it — and no Controller contract has a field +// for either: the proto ExecutionRecord declares none, and the JSON binding's NodeExecution schema is +// closed, so a `SELECT *` that carried the column failed the response contract the moment migration +// 0041 added it. // // So a row that reaches a response body is projected explicitly, the way workflowRunSummary projects // a run row, rather than read whole. A column added to the table therefore does not reach the wire by @@ -102,9 +103,11 @@ func dispatchRunExecution(t *transaction, r *ControlRequest, runID, execution, n AND kind IN ('agent_session','deliver_revision','deliver_scheduled_artifacts') AND (kind='agent_session' OR kind=$2 OR $2='agent_session') AND result IS NULL AND terminated_by_force_stop_id IS NULL`, runID, input.S("kind")) == nil, 409, "dispatch_conflict") - t.exec(`INSERT INTO node_executions(execution_id,kind,operation_id,work_id,workspace_id,node_id,node_operation_id,input,dispatched_epoch,workflow_node_id) - VALUES($1,$2,$3,$4,$5,$6,$1,$7,$8,NULLIF($9,''))`, - execution, input.S("kind"), runID, work.S("id"), wid, node, jsonText(input), r.Body.N("epoch"), work.S("workflowNodeId")) + // The attempt is copied from the claimed work item, never from the wire: it is Cloud's routing + // identity for the execution (retry decision D4), and the Controller neither knows nor needs it. + t.exec(`INSERT INTO node_executions(execution_id,kind,operation_id,work_id,workspace_id,node_id,node_operation_id,input,dispatched_epoch,workflow_node_id,workflow_node_attempt) + VALUES($1,$2,$3,$4,$5,$6,$1,$7,$8,NULLIF($9,''),$10)`, + execution, input.S("kind"), runID, work.S("id"), wid, node, jsonText(input), r.Body.N("epoch"), work.S("workflowNodeId"), work.N("workflowNodeAttempt")) t.exec("UPDATE execution_work SET execution_id=$2 WHERE id=$1", work.S("id"), execution) return t.one("SELECT "+nodeExecutionColumns+" FROM node_executions WHERE execution_id=$1", execution) } diff --git a/internal/core/thread_commands.go b/internal/core/thread_commands.go index 3630dc76..52c66771 100644 --- a/internal/core/thread_commands.go +++ b/internal/core/thread_commands.go @@ -25,22 +25,27 @@ func enqueueThreadCommand(t *transaction, runID, kind string, body Object) Objec return t.one("SELECT * FROM thread_commands WHERE id=$1", id) } -// enqueueWorkflowThreadCommand releases one session command for a WorkflowRun's graph node. It is -// the WorkflowRun counterpart of enqueueThreadCommand: the row registers against workflow_runs -// (0049's discriminator), and the graph node scopes the command to the session it addresses — a run -// executes one agent node at a time but several over its life (D4). Repeating the same request -// returns the original row; a different request for the same node conflicts. -func enqueueWorkflowThreadCommand(t *transaction, runID, nodeID, kind string, body Object) Object { +// enqueueWorkflowThreadCommand releases one session command for a WorkflowRun's graph node and +// attempt. It is the WorkflowRun counterpart of enqueueThreadCommand: the row registers against +// workflow_runs (0049's discriminator), and the node scopes the command to the session it addresses — +// a run executes one agent node at a time but several over its life (D4). The attempt scopes it +// further, to that node's one session per attempt (retry decision D4): attempt 1's end request must +// never be reused for, or delivered to, attempt 2. Repeating the same request returns the original +// row; a different request for the same node and attempt conflicts. +func enqueueWorkflowThreadCommand(t *transaction, runID, nodeID string, attempt int64, kind string, body Object) Object { require(validID(runID), 400, "invalid_command") require(t.one("SELECT id FROM workflow_runs WHERE id=$1 AND deleted_at IS NULL", runID) != nil, 404, "not_found") require(kind == "end_session", 400, "invalid_command") validateThreadCommand(kind, body) - if existing := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND kind=$3", runID, nodeID, kind); existing != nil { + if attempt < 1 { + attempt = 1 + } + if existing := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind=$4", runID, nodeID, attempt, kind); existing != nil { require(jsonText(existing.O("body")) == jsonText(body), 409, "dispatch_conflict") return existing } id := newID() - t.exec("INSERT INTO thread_commands(id,workflow_run_id,workflow_node_id,kind,body) VALUES($1,$2,$3,$4,$5)", id, runID, nodeID, kind, jsonText(body)) + t.exec("INSERT INTO thread_commands(id,workflow_run_id,workflow_node_id,workflow_node_attempt,kind,body) VALUES($1,$2,$3,$4,$5,$6)", id, runID, nodeID, attempt, kind, jsonText(body)) t.commandSignals = append(t.commandSignals, runID) return t.one("SELECT * FROM thread_commands WHERE id=$1", id) } @@ -68,18 +73,21 @@ func claimThreadCommands(t *transaction, limit int64) Object { // rather than `c.*`: row_to_json keeps both copies of a repeated column name, and which one the // decoder ends up with is not something this query should depend on. // - // A workflow command is scoped to the graph node whose session it addresses. The join therefore - // carries the node scope: one operation id has one live agent_session execution per graph node - // over the run's life (D4), and a command must never be claimable against another node's - // session. Ordering is by queue_sequence alone — it is the insertion order the Controller's - // "a skipped head blocks later commands of that run" rule relies on — because ordering by the - // owner column would sort every workflow command after every IssueRun command. + // A workflow command is scoped to the graph node and attempt whose session it addresses. The join + // therefore carries both: one operation id has one live agent_session execution per graph node + // over the run's life (D4), and one more per attempt since retries (retry decision D4), so a + // command must never be claimable against another node's session or another attempt's. IssueRun + // commands carry no node, and their executions keep the default attempt, so the node branch is + // where the attempt predicate belongs. Ordering is by queue_sequence alone — it is the insertion + // order the Controller's "a skipped head blocks later commands of that run" rule relies on — + // because ordering by the owner column would sort every workflow command after every IssueRun + // command. rows := t.list(`SELECT c.id AS command_id,COALESCE(c.run_id,c.workflow_run_id) AS run_id,c.kind,c.body, e.execution_id,e.node_id,e.workspace_id,s.id AS sandbox_instance_id FROM thread_commands c JOIN node_executions e ON e.operation_id=COALESCE(c.run_id,c.workflow_run_id) AND e.kind='agent_session' AND e.result IS NULL AND e.terminated_by_force_stop_id IS NULL - AND (c.workflow_node_id IS NULL OR e.workflow_node_id=c.workflow_node_id) + AND (c.workflow_node_id IS NULL OR (e.workflow_node_id=c.workflow_node_id AND e.workflow_node_attempt=c.workflow_node_attempt)) JOIN workspaces w ON w.id=e.workspace_id JOIN sandbox_instances s ON s.workspace_id=w.id AND s.generation=w.runtime_generation AND s.terminated_at IS NULL WHERE c.delivered_at IS NULL @@ -99,10 +107,11 @@ func recordThreadCommandDelivered(t *transaction, r *ControlRequest) Object { } e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND operation_id=$2 AND kind='agent_session'", execution, owner) require(e != nil, 409, "dispatch_conflict") - // A WorkflowRun command names the graph node whose session it addresses; an execution of the - // same run under another node is not this command's destination. + // A WorkflowRun command names the graph node and attempt whose session it addresses; an execution + // of the same run under another node, or under the same node's other attempt, is not this + // command's destination (retry decision D4). if node := cmd.S("workflowNodeId"); node != "" { - require(e.S("workflowNodeId") == node, 409, "dispatch_conflict") + require(e.S("workflowNodeId") == node && e.N("workflowNodeAttempt") == cmd.N("workflowNodeAttempt"), 409, "dispatch_conflict") } if cmd.S("deliveredExecutionId") != "" { require(cmd.S("deliveredExecutionId") == execution, 409, "dispatch_conflict") diff --git a/internal/core/workflow_graph.go b/internal/core/workflow_graph.go index ca333fc2..ba929b41 100644 --- a/internal/core/workflow_graph.go +++ b/internal/core/workflow_graph.go @@ -36,8 +36,8 @@ var workflowExecutableNodeKinds = map[string]bool{ // believes a setting is in effect when no runner will ever apply it. // // They are listed rather than derived so the refusal stays a decision about *this* field, and in this -// order so the problem list a client renders is deterministic. `executorSlot`, `roleId` and `prompt` -// are the first phase's agent configuration and are deliberately absent here; the pre-slot +// order so the problem list a client renders is deterministic. `executorSlot`, `roleId`, `prompt` and +// `retry` are executed agent configuration and are deliberately absent here; the pre-slot // `agentConfig.executor` is absent too, because specs#98 D5 makes it a field to accept and drop // rather than refuse. var workflowUnsupportedAgentConfigFields = []string{ @@ -45,7 +45,6 @@ var workflowUnsupportedAgentConfigFields = []string{ "mcps", "interactive", "outputContract", - "retry", "promptInactivity", } @@ -414,16 +413,18 @@ func validateWorkflowNodes(nodes []any, globals []Object) []workflowGraphProblem } // validateAgentConfig refuses the agent-node configuration the first phase does not execute, naming -// both the node and the field (#67 D2). The alternative — ignoring the field — is the one outcome the -// decision rules out: an author who set `retry` would read a publish that succeeded as "my retry -// policy is in effect", and nothing would ever apply it. +// both the node and the field (#67 D2), and checks the shape of the configuration it does execute. +// The alternative — ignoring an unsupported field — is the one outcome the decision rules out: an +// author who set `interactive` would read a publish that succeeded as "my setting is in effect", and +// nothing would ever apply it. // // A key present with a JSON null is not a refusal, and neither is the value the editor writes when // nothing is enabled: an empty `skills`/`mcps` list or `interactive: false`. The decision's reason // for refusing these fields is that they change execution behavior, and a null changes none, exactly // like an empty list or a false flag; refusing them would be the same mistake in the other // direction, rejecting a document for a value that asks for nothing. The same reading is what -// `validSlotHint` applies to an absent-or-null `hint`. +// `validSlotHint` applies to an absent-or-null `hint`. `retry` is no longer refused — it is executed +// (retry decision D1) — so it is validated for shape instead of rejected. func validateAgentConfig(nodeID string, data Object) []workflowGraphProblem { problems := []workflowGraphProblem{} config := data.O("agentConfig") @@ -434,6 +435,39 @@ func validateAgentConfig(nodeID string, data Object) []workflowGraphProblem { } problems = append(problems, workflowGraphProblem{Code: "node_config_unsupported", NodeID: nodeID, Field: field}) } + return append(problems, validateAgentRetryConfig(nodeID, config)...) +} + +// validateAgentRetryConfig checks `agentConfig.retry` against the policy shape the scheduler executes +// (retry decision D1). Absent and null mean the default policy, so neither is a problem. A present +// value must be an object carrying all three fields, each with its own type and range, because a +// partially filled object would make the author execute a policy they never wrote: the missing fields +// would silently fall back to defaults, so "I only wanted the delay" would also change the enabled +// flag or the budget. The object's shape is validated as a unit and every invalid field is reported, +// so one publish answers with the whole list the author has to fix. +// +// Unknown keys are ignored, like everywhere else in the graph document: the schema may grow fields +// Cloud does not execute yet, and refusing them here would make publishing fail for a newer editor. +func validateAgentRetryConfig(nodeID string, config Object) []workflowGraphProblem { + raw, present := config["retry"] + if !present || raw == nil { + return nil + } + retry, ok := asWorkflowObject(raw) + if !ok { + return []workflowGraphProblem{{Code: "agent_retry_not_an_object", NodeID: nodeID, Field: "agentConfig.retry"}} + } + problems := []workflowGraphProblem{} + if _, ok := retry["enabled"].(bool); !ok { + problems = append(problems, workflowGraphProblem{Code: "agent_retry_enabled_invalid", NodeID: nodeID, Field: "agentConfig.retry.enabled"}) + } + if n, ok := workflowRetryInteger(retry["maxRetries"]); !ok || n < workflowRetryMinMaxRetries || n > workflowRetryMaxMaxRetries { + problems = append(problems, workflowGraphProblem{Code: "agent_retry_max_retries_invalid", NodeID: nodeID, Field: "agentConfig.retry.maxRetries"}) + } + if n, ok := workflowRetryInteger(retry["initialDelaySeconds"]); !ok || + n < workflowRetryMinInitialDelaySeconds || n > workflowRetryMaxInitialDelaySeconds { + problems = append(problems, workflowGraphProblem{Code: "agent_retry_initial_delay_invalid", NodeID: nodeID, Field: "agentConfig.retry.initialDelaySeconds"}) + } return problems } diff --git a/internal/core/workflow_graph_test.go b/internal/core/workflow_graph_test.go index 1d547a25..272eb848 100644 --- a/internal/core/workflow_graph_test.go +++ b/internal/core/workflow_graph_test.go @@ -443,6 +443,113 @@ func TestValidateWorkflowGraph(t *testing.T) { } } +// The publish gate validates the retry policy Cloud executes (retry decision D1). Absent and null are +// the default policy and are admitted; a present value must be a complete object, because a partial +// one would silently run defaults for the fields the author left out. Every invalid field is reported +// at once, and unknown keys are ignored like everywhere else in the document, so a newer editor's +// extra key cannot block publishing. The runtime parser is deliberately more forgiving +// (TestWorkflowRetryPolicyOf): a frozen snapshot is never re-gated. +func TestWorkflowPublishValidatesAgentRetry(t *testing.T) { + valid := func(enabled, maxRetries, delay any) Object { + return Object{"enabled": enabled, "maxRetries": maxRetries, "initialDelaySeconds": delay} + } + cases := []struct { + name string + absent bool + retry any + want []any + }{ + {name: "no retry policy", absent: true}, + {name: "retry null", retry: nil}, + {name: "a complete policy", retry: valid(true, 2, 10)}, + {name: "the lower bounds", retry: valid(false, 0, 0)}, + {name: "the upper bounds", retry: valid(true, 5, 300)}, + { + name: "a policy with an unknown key", + retry: Object{"enabled": true, "maxRetries": 2, "initialDelaySeconds": 10, "future": 1}, + }, + { + name: "retry not an object", + retry: true, + want: []any{Object{"code": "agent_retry_not_an_object", "nodeId": "a", "field": "agentConfig.retry"}}, + }, + { + name: "retry a list", + retry: []any{true, 2, 10}, + want: []any{Object{"code": "agent_retry_not_an_object", "nodeId": "a", "field": "agentConfig.retry"}}, + }, + { + // A partial object is refused as a whole rather than filled from the defaults: "I only + // changed the delay" must not also change the enabled flag or the budget. + name: "an empty object reports every missing field", + retry: Object{}, + want: []any{ + Object{"code": "agent_retry_enabled_invalid", "nodeId": "a", "field": "agentConfig.retry.enabled"}, + Object{"code": "agent_retry_max_retries_invalid", "nodeId": "a", "field": "agentConfig.retry.maxRetries"}, + Object{"code": "agent_retry_initial_delay_invalid", "nodeId": "a", "field": "agentConfig.retry.initialDelaySeconds"}, + }, + }, + { + name: "enabled of the wrong type", + retry: valid("true", 2, 10), + want: []any{Object{"code": "agent_retry_enabled_invalid", "nodeId": "a", "field": "agentConfig.retry.enabled"}}, + }, + { + name: "maxRetries below the range", + retry: valid(true, -1, 10), + want: []any{Object{"code": "agent_retry_max_retries_invalid", "nodeId": "a", "field": "agentConfig.retry.maxRetries"}}, + }, + { + name: "maxRetries above the range", + retry: valid(true, 6, 10), + want: []any{Object{"code": "agent_retry_max_retries_invalid", "nodeId": "a", "field": "agentConfig.retry.maxRetries"}}, + }, + { + name: "a fractional maxRetries", + retry: valid(true, 1.5, 10), + want: []any{Object{"code": "agent_retry_max_retries_invalid", "nodeId": "a", "field": "agentConfig.retry.maxRetries"}}, + }, + { + name: "initialDelaySeconds below the range", + retry: valid(true, 2, -1), + want: []any{Object{"code": "agent_retry_initial_delay_invalid", "nodeId": "a", "field": "agentConfig.retry.initialDelaySeconds"}}, + }, + { + name: "initialDelaySeconds above the range", + retry: valid(true, 2, 301), + want: []any{Object{"code": "agent_retry_initial_delay_invalid", "nodeId": "a", "field": "agentConfig.retry.initialDelaySeconds"}}, + }, + { + name: "every field invalid", + retry: Object{"enabled": 1, "maxRetries": "two", "initialDelaySeconds": false}, + want: []any{ + Object{"code": "agent_retry_enabled_invalid", "nodeId": "a", "field": "agentConfig.retry.enabled"}, + Object{"code": "agent_retry_max_retries_invalid", "nodeId": "a", "field": "agentConfig.retry.maxRetries"}, + Object{"code": "agent_retry_initial_delay_invalid", "nodeId": "a", "field": "agentConfig.retry.initialDelaySeconds"}, + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + extra := Object{} + if !tc.absent { + extra["retry"] = tc.retry + } + graph := workflowGraph( + []any{graphStart("s"), graphAgentConfig("a", extra)}, + []any{graphEdge("s", "a")}, nil, + ) + got := problemObjects(validateWorkflowGraph(decodeGraph(t, graph))) + if tc.want == nil { + tc.want = []any{} + } + if !reflect.DeepEqual(got, tc.want) { + t.Fatalf("problems = %v, want %v", got, tc.want) + } + }) + } +} + // The publish gate refuses every condition configuration Cloud cannot evaluate the way desktop's // condition.rs does (circus#202 A 条件 2). Each one is a place the evaluator would otherwise answer // silently — a no-match, an `and`, an else branch, a whole node output, a literal where desktop diff --git a/internal/core/workflow_retry.go b/internal/core/workflow_retry.go new file mode 100644 index 00000000..368986e7 --- /dev/null +++ b/internal/core/workflow_retry.go @@ -0,0 +1,168 @@ +package core + +import ( + "encoding/json" + "math" + "time" +) + +// Workflow agent-node retry: the desktop-compatible `agentConfig.retry` policy and the scheduler's +// retry decision (specs decisions/cloud/workflow 20261021-node-retry-and-resume-contract.md D1–D3). +// +// The publish gate validates the document (validateAgentRetryConfig in workflow_graph.go); this file +// is the runtime read of the same field. The two share the shape and the bounds so a graph the gate +// accepts always parses here. The parser is total because a frozen snapshot is never re-gated: an +// imported or pre-gate document may carry a value the gate would refuse, and the scheduler must +// still reach a decision rather than fail a run it cannot interpret. + +const ( + // The default policy every agent node executes with when it declares none: enabled, two extra + // attempts, ten seconds before the first retry. These are desktop's AgentRetryPolicy::default() + // values (D1), and the defaults are also what a historical snapshot without `retry` reads. + workflowRetryDefaultMaxRetries = 2 + workflowRetryDefaultInitialDelaySeconds = 10 + + // The ranges the publish gate accepts (D1). They bound the runtime parser too: a value outside + // them is not a policy the gate could have frozen, so it reads as the default policy. + workflowRetryMinMaxRetries = 0 + workflowRetryMaxMaxRetries = 5 + workflowRetryMinInitialDelaySeconds = 0 + workflowRetryMaxInitialDelaySeconds = 300 + workflowRetryMaxWaitSeconds = 600 + + // workflowRetryableSessionReason is the one session end reason a failed attempt is retried for. + // The set is closed (D2): `interrupted` is the host/Node interrupt desktop never retries, + // `cancelled` is the user's decision, and Cloud's own deadline and resource reasons are already + // bounded waits whose retry would only multiply the ceiling. Extending this table is an ADR + // change, not an implementation detail. + workflowRetryableSessionReason = "agent_failed" +) + +// workflowRetryPolicy is the parsed retry configuration of one agent node. It is a plain value: the +// policy is frozen in the snapshot graph and read fresh on every pass, so nothing about the wait is +// carried in memory between ticks. +type workflowRetryPolicy struct { + Enabled bool + MaxRetries int64 + InitialDelaySeconds int64 +} + +// defaultWorkflowRetryPolicy is the policy an absent or unusable `agentConfig.retry` reads as. +func defaultWorkflowRetryPolicy() workflowRetryPolicy { + return workflowRetryPolicy{ + Enabled: true, + MaxRetries: workflowRetryDefaultMaxRetries, + InitialDelaySeconds: workflowRetryDefaultInitialDelaySeconds, + } +} + +// workflowRetryPolicyOf reads the frozen node's `agentConfig.retry`. Absent and null are the default +// policy (D1), and so is a value the publish gate would refuse: snapshots are not re-gated, and a +// document Cloud imported before this field was executable has no policy to honor — the default is +// the one reading that never invents a wait or a budget out of a malformed value. +func workflowRetryPolicyOf(data Object) workflowRetryPolicy { + raw, present := data.O("agentConfig")["retry"] + if !present || raw == nil { + return defaultWorkflowRetryPolicy() + } + config, ok := asWorkflowObject(raw) + if !ok { + return defaultWorkflowRetryPolicy() + } + enabled, enabledOK := config["enabled"].(bool) + maxRetries, maxOK := workflowRetryInteger(config["maxRetries"]) + initialDelay, delayOK := workflowRetryInteger(config["initialDelaySeconds"]) + if !enabledOK || !maxOK || !delayOK || + maxRetries < workflowRetryMinMaxRetries || maxRetries > workflowRetryMaxMaxRetries || + initialDelay < workflowRetryMinInitialDelaySeconds || initialDelay > workflowRetryMaxInitialDelaySeconds { + return defaultWorkflowRetryPolicy() + } + return workflowRetryPolicy{Enabled: enabled, MaxRetries: maxRetries, InitialDelaySeconds: initialDelay} +} + +// workflowRetryInteger reads one integral JSON number of a retry policy. The document reaches Cloud +// through either the request decoder (json.Number) or a jsonb column (float64), so both are accepted +// together with the Go integer types hand-built documents use. A fractional value is refused rather +// than truncated: 1.5 retries is not a policy anyone wrote on purpose. +func workflowRetryInteger(value any) (int64, bool) { + switch v := value.(type) { + case json.Number: + n, err := v.Int64() + return n, err == nil + case float64: + if math.IsNaN(v) || math.IsInf(v, 0) || v != math.Trunc(v) { + return 0, false + } + return int64(v), true + case int: + return int64(v), true + case int64: + return v, true + } + return 0, false +} + +// workflowRetryWait is the wait before the attempt following failedAttempt: the policy's initial +// delay doubled per retry (`initialDelaySeconds × 2^(n-1)`, n the 1-based retry number, which equals +// the number of the attempt that failed), capped at 600 seconds. Both bounds are small — at most six +// attempts and 300 seconds — so the shift cannot overflow. +func workflowRetryWait(policy workflowRetryPolicy, failedAttempt int64) time.Duration { + retry := failedAttempt + if retry < 1 { + retry = 1 + } + if retry > workflowRetryMaxMaxRetries+1 { + retry = workflowRetryMaxMaxRetries + 1 + } + seconds := policy.InitialDelaySeconds << (retry - 1) + if seconds > workflowRetryMaxWaitSeconds { + seconds = workflowRetryMaxWaitSeconds + } + return time.Duration(seconds) * time.Second +} + +// workflowRetryDecision reports whether a failed attempt is replaced by a committed wait, and for how +// long. The reason is read from the session result before it is mapped to a node status (D2): only +// `agent_failed` retries, so a reason outside the closed set settles the node failed even when it +// also maps to failed. The budget counts the failed attempt: attempt a may be followed by a+1 only +// while a <= maxRetries, so one run makes at most maxRetries+1 attempts (D1/D6). +func workflowRetryDecision(policy workflowRetryPolicy, failedAttempt int64, reason string) (time.Duration, bool) { + if reason != workflowRetryableSessionReason || !policy.Enabled || policy.MaxRetries < 1 { + return 0, false + } + if failedAttempt > policy.MaxRetries { + return 0, false + } + return workflowRetryWait(policy, failedAttempt), true +} + +// workflowRetryingAgentNode returns the id of the node committed `retrying`, if any. A run has at +// most one: a waiting attempt keeps the run's serial agent slot (D3), so no other agent node of the +// same run can be waiting at the same time. +func workflowRetryingAgentNode(states map[string]workflowNodeState) (string, bool) { + for id, state := range states { + if state.Status == "retrying" { + return id, true + } + } + return "", false +} + +// workflowRetryAt reads the next attempt's earliest start from the database clock. The instant is +// computed and rendered by PostgreSQL so a process clock skew can neither shorten nor extend the +// committed wait, exactly as the session windows are measured; the text is RFC 3339 in UTC so the +// run viewer reads it like every other timestamp in `node_states`. +func workflowRetryAt(t *transaction, wait time.Duration) string { + row := t.one(`SELECT to_char((clock_timestamp() + make_interval(secs => $1)) AT TIME ZONE 'UTC', + 'YYYY-MM-DD"T"HH24:MI:SS.MS"Z"') AS retry_at`, wait.Seconds()) + return row.S("retryAt") +} + +// workflowRetryDue reports whether the committed retry instant has been reached. It reads the run +// row rather than the pass's decoded copy and compares on the database clock, so every replica, +// restart and replay judges the same instant (D3, invariant 5). +func workflowRetryDue(t *transaction, runID, nodeID string) bool { + return t.one(`SELECT id FROM workflow_runs + WHERE id=$1 AND COALESCE(node_states->$2::text->>'retryAt','') <> '' + AND (node_states->$2::text->>'retryAt')::timestamptz <= clock_timestamp()`, runID, nodeID) != nil +} diff --git a/internal/core/workflow_retry_db_test.go b/internal/core/workflow_retry_db_test.go new file mode 100644 index 00000000..97bdc34b --- /dev/null +++ b/internal/core/workflow_retry_db_test.go @@ -0,0 +1,417 @@ +package core + +import ( + "context" + "testing" + "time" +) + +// DB tests for workflow agent-node retry (specs decisions/cloud/workflow +// 20261021-node-retry-and-resume-contract.md D1–D6). +// +// The decision's obligations are proved on the shipped seams, not around them: the session result +// arrives through `executionResult`, the scheduler wakes through `AdvanceWorkflowRunsOnce`, and the +// cancellation request goes through `Store.Public` exactly as the router drives it. Only two +// preconditions are staged directly — a settled run Workspace (the control plane's asynchronous +// create) and, where a case needs the committed due instant already passed, the `retryAt` value +// itself, because the database clock is the only authority the scheduler reads and a test must not +// sleep for it. + +// workflowRetryGraph is the linear start -> agent -> output graph with an explicit retry policy on +// the agent node. The policy is written where the published gate reads it, so the scheduler parses +// exactly what an author would have published. +func workflowRetryGraph(enabled bool, maxRetries, initialDelaySeconds int64) Object { + return wfGraph( + []Object{ + wfNode("s", "start", ""), + wfAgentNode("a", Object{ + "prompt": "Do the thing", + "retry": Object{ + "enabled": enabled, "maxRetries": maxRetries, "initialDelaySeconds": initialDelaySeconds, + }, + }), + wfNode("o", "output", ""), + }, + [][3]string{{"s", "a", ""}, {"a", "o", ""}}, + ) +} + +// readWorkflowNodeState reads one node's committed state from the run row. +func readWorkflowNodeState(t *testing.T, store *Store, runID, nodeID string) Object { + t.Helper() + _, _, states, _ := readWorkflowRun(t, store, runID) + state, _ := asWorkflowObject(states[nodeID]) + if state == nil { + t.Fatalf("run %s carries no state for node %s", runID, nodeID) + } + return state +} + +// requireRetrying asserts the committed state of a node waiting for its next attempt (D3): the +// attempt that failed, its reason, the kept start instant and no finish instant, plus the committed +// due instant. +func requireRetrying(t *testing.T, state Object, attempt int64) time.Time { + t.Helper() + if state.S("status") != "retrying" || state.N("attempt") != attempt { + t.Fatalf("state = %v; want retrying attempt %d", state, attempt) + } + if state.S("retryAt") == "" { + t.Fatalf("state = %v; a retrying node must carry retryAt", state) + } + if state.S("finishedAt") != "" { + t.Fatalf("state = %v; a retrying node has not finished", state) + } + if state.S("startedAt") == "" { + t.Fatalf("state = %v; the failed attempt's start instant must be kept", state) + } + if state.S("error") != "agent_failed" || state.O("output").S("reason") != "agent_failed" { + t.Fatalf("state = %v; want the failed attempt's reason kept as agent_failed", state) + } + at, err := time.Parse(time.RFC3339, state.S("retryAt")) + if err != nil { + t.Fatalf("retryAt %q is not RFC 3339: %v", state.S("retryAt"), err) + } + return at +} + +// makeWorkflowRetryDue moves the committed retry instant into the past. The scheduler judges only +// `retryAt <= clock_timestamp()`, so this is the one boundary a test crosses to advance the database +// clock without sleeping. +func makeWorkflowRetryDue(t *testing.T, store *Store, runID, nodeID string) { + t.Helper() + if _, err := store.Pool.Exec(`UPDATE workflow_runs + SET node_states = jsonb_set(node_states, ARRAY[$2::text,$3::text], to_jsonb(clock_timestamp() - interval '1 second')) + WHERE id=$1`, runID, nodeID, "retryAt"); err != nil { + t.Fatalf("move retryAt into the past: %v", err) + } +} + +// dispatchSessionAttempt dispatches an attempt's released session work and passes the scheduler once, +// which holds the committed running node until terminal evidence arrives. The execution id must be +// unique per attempt: the same id with a different input is a conflict, and the re-keyed per-attempt +// uniqueness is exactly what lets a second attempt register at all. +func dispatchSessionAttempt(t *testing.T, store *Store, runID, graphNode, nodeService, execution string, spec Object) { + t.Helper() + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + nodeDispatch(tx, &ControlRequest{Body: Object{"operationId": runID, "executionId": execution, "nodeId": nodeService, "input": spec, "epoch": int64(1)}}) + return Object{} + }); err != nil { + t.Fatalf("dispatch node %s attempt %s: %v", graphNode, execution, err) + } + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after dispatching %s: %v", execution, err) + } + state := readWorkflowNodeState(t, store, runID, graphNode) + if state.S("status") != "running" { + t.Fatalf("dispatched session held: %v", state) + } +} + +// sessionWorkAttempt reads the attempt recorded on a node's unregistered work item — Cloud's own +// routing identity, which the dispatch copies onto the execution. +func sessionWorkAttempt(t *testing.T, store *Store, runID, graphNode string) int64 { + t.Helper() + var attempt int64 + if err := store.Pool.QueryRow(`SELECT workflow_node_attempt FROM execution_work + WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND execution_id IS NULL`, runID, graphNode).Scan(&attempt); err != nil { + t.Fatalf("read session work attempt for %s: %v", graphNode, err) + } + return attempt +} + +// runWorkspaceReleaseState reads the run's release fact; `pending` is what a non-terminal run +// carries, because no release was declared yet. +func runWorkspaceReleaseState(t *testing.T, store *Store, runID string) string { + t.Helper() + var state string + if err := store.Pool.QueryRow(`SELECT COALESCE(workspace_release_state,'') FROM workflow_runs WHERE id=$1`, runID).Scan(&state); err != nil { + t.Fatalf("read workspace release state: %v", err) + } + return state +} + +// A retryable failure commits `retrying`, not a terminal state: the run stays `running`, the node +// keeps the failed attempt's evidence and start instant, and the committed wait is the database +// clock's instant. Until that instant passes a pass writes nothing. Once it is due, the next attempt +// starts through the ordinary session path with the next attempt number, and the run settles from +// that attempt's own result — a lookup scoped to attempt 2 must not read attempt 1's failure. +func TestWorkflowRetryWaitsThenStartsTheNextAttempt(t *testing.T) { + store := dispatcherDB(t) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 2, 300)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec1 := startSessionNode(t, store, runID, wsID, "a") + firstStarted := readWorkflowNodeState(t, store, runID, "a").S("startedAt") + execution1 := "exec-a-attempt-1-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution1, spec1) + reportSessionEnd(t, store, runID, execution1, nodeService, "agent_failed") + + status, runErr, _, _ := readWorkflowRun(t, store, runID) + state := readWorkflowNodeState(t, store, runID, "a") + retryAt := requireRetrying(t, state, 1) + if status != "running" || runErr != "" { + t.Fatalf("run = %q error %q; a waiting retry keeps the run running", status, runErr) + } + if state.S("startedAt") != firstStarted { + t.Fatalf("startedAt = %q; want the failed attempt's %q", state.S("startedAt"), firstStarted) + } + // The wait is the policy's 300 seconds measured on the database clock. + if wait := time.Until(retryAt); wait < 295*time.Second || wait > 305*time.Second { + t.Fatalf("retryAt = %s (in %s); want about 300s from now", state.S("retryAt"), wait) + } + if got := runWorkspaceReleaseState(t, store, runID); got != "pending" { + t.Fatalf("workspace release state = %q; a waiting retry is not terminal", got) + } + var unregistered int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items released while the retry waits; none should be", unregistered) + } + + // Before the committed instant the pass is a no-op: no new attempt, no work item. + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance before due: %v", err) + } + if again := readWorkflowNodeState(t, store, runID, "a"); again.S("status") != "retrying" || again.N("attempt") != 1 || again.S("retryAt") != state.S("retryAt") { + t.Fatalf("state before due = %v; want the unchanged waiting state", again) + } + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items released before the retry was due", unregistered) + } + + // Once due, the next attempt starts with the next attempt number and a fresh state. + makeWorkflowRetryDue(t, store, runID, "a") + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance at due: %v", err) + } + started := readWorkflowNodeState(t, store, runID, "a") + if started.S("status") != "running" || started.N("attempt") != 2 { + t.Fatalf("state after due = %v; want running attempt 2", started) + } + if started.S("retryAt") != "" || started.S("finishedAt") != "" || started.S("error") != "" || started["output"] != nil { + t.Fatalf("state after due = %v; the new attempt starts clean", started) + } + if attempt := sessionWorkAttempt(t, store, runID, "a"); attempt != 2 { + t.Fatalf("released work item names attempt %d; want 2", attempt) + } + + // The second attempt settles the run from its own result. + spec2 := sessionWorkSpec(t, store, runID, "a") + execution2 := "exec-a-attempt-2-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution2, spec2) + reportSessionEnd(t, store, runID, execution2, nodeService, "idle_timeout") + + status, runErr, states, _ := readWorkflowRun(t, store, runID) + if status != "succeeded" || runErr != "" { + t.Fatalf("run settled %q error %q states=%v; want succeeded from attempt 2", status, runErr, states) + } + agent, _ := asWorkflowObject(states["a"]) + outs, _ := asWorkflowObject(states["o"]) + if agent == nil || agent.S("status") != "succeeded" || agent.N("attempt") != 2 || agent.O("output").S("reason") != "idle_timeout" { + t.Fatalf("agent state = %v; want succeeded attempt 2 with its own result", states["a"]) + } + if outs == nil || outs.S("status") != "succeeded" { + t.Fatalf("output state = %v; want succeeded after the retried node", states["o"]) + } + + // Two sessions registered for one node, each against its own attempt: the lifetime uniqueness was + // re-keyed on the attempt (D4), and the dispatch copied it off the work item. + rows, err := store.Pool.Query(`SELECT workflow_node_attempt, execution_id FROM node_executions + WHERE operation_id=$1 AND workflow_node_id='a' AND kind='agent_session' ORDER BY workflow_node_attempt`, runID) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + type session struct { + attempt int64 + execution string + } + var sessions []session + for rows.Next() { + var s session + if err := rows.Scan(&s.attempt, &s.execution); err != nil { + t.Fatal(err) + } + sessions = append(sessions, s) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + want := []session{{1, execution1}, {2, execution2}} + if len(sessions) != len(want) || sessions[0] != want[0] || sessions[1] != want[1] { + t.Fatalf("registered sessions = %v; want one per attempt %v", sessions, want) + } + assertRunWorkspaceReleased(t, store, runID) +} + +// The budget is `maxRetries + 1` attempts (D1/D6). After the last allowed attempt fails, the node +// keeps that attempt's reason and finishes failed, and the run fails and releases its Workspace — +// there is no third attempt and no unregistered work left behind. +func TestWorkflowRetryBudgetExhaustionFailsTheRun(t *testing.T) { + store := dispatcherDB(t) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 1, 0)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec1 := startSessionNode(t, store, runID, wsID, "a") + execution1 := "exec-budget-1-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution1, spec1) + reportSessionEnd(t, store, runID, execution1, nodeService, "agent_failed") + requireRetrying(t, readWorkflowNodeState(t, store, runID, "a"), 1) + + // maxRetries=1 allows exactly one more attempt. + makeWorkflowRetryDue(t, store, runID, "a") + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance to attempt 2: %v", err) + } + started := readWorkflowNodeState(t, store, runID, "a") + if started.S("status") != "running" || started.N("attempt") != 2 { + t.Fatalf("state after due = %v; want running attempt 2", started) + } + spec2 := sessionWorkSpec(t, store, runID, "a") + execution2 := "exec-budget-2-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution2, spec2) + reportSessionEnd(t, store, runID, execution2, nodeService, "agent_failed") + + status, runErr, states, _ := readWorkflowRun(t, store, runID) + if status != "failed" || runErr != "agent_failed" { + t.Fatalf("run settled %q error %q states=%v; want failed with the last attempt's reason", status, runErr, states) + } + agent, _ := asWorkflowObject(states["a"]) + if agent == nil || agent.S("status") != "failed" || agent.N("attempt") != 2 || agent.S("error") != "agent_failed" { + t.Fatalf("agent state = %v; want failed attempt 2 keeping its reason", states["a"]) + } + if agent.S("retryAt") != "" || agent.S("finishedAt") == "" { + t.Fatalf("agent state = %v; a failed node has finished and schedules nothing", agent) + } + var unregistered int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items left behind by a failed run", unregistered) + } + // A further pass does not invent a third attempt. + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after failure: %v", err) + } + status, _, states, _ = readWorkflowRun(t, store, runID) + agent, _ = asWorkflowObject(states["a"]) + if status != "failed" || agent == nil || agent.N("attempt") != 2 { + t.Fatalf("a terminal run moved: status=%q agent=%v", status, states["a"]) + } + assertRunWorkspaceReleased(t, store, runID) +} + +// A failure that must not retry settles the node and the run immediately: a disabled policy, a zero +// budget, and a reason outside D2's closed set are all decided before the node status is mapped, so +// none of them leaves a committed wait. `interrupted` is the reason desktop never retries. +func TestWorkflowRetryOnlyRetriesAgentFailedWithinBudget(t *testing.T) { + cases := []struct { + name string + enabled bool + budget int64 + reason string + }{ + {name: "a disabled policy", enabled: false, budget: 2, reason: "agent_failed"}, + {name: "a zero budget", enabled: true, budget: 0, reason: "agent_failed"}, + {name: "an interrupted session", enabled: true, budget: 2, reason: "interrupted"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + store := dispatcherDB(t) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(tc.enabled, tc.budget, 0)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec := startSessionNode(t, store, runID, wsID, "a") + execution := "exec-no-retry-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution, spec) + reportSessionEnd(t, store, runID, execution, nodeService, tc.reason) + + status, runErr, states, _ := readWorkflowRun(t, store, runID) + if status != "failed" || runErr != tc.reason { + t.Fatalf("run settled %q error %q states=%v; want failed with %q and no retry", status, runErr, states, tc.reason) + } + agent, _ := asWorkflowObject(states["a"]) + if agent == nil || agent.S("status") != "failed" || agent.N("attempt") != 1 || agent.S("retryAt") != "" || agent.S("error") != tc.reason { + t.Fatalf("agent state = %v; want failed attempt 1 with %q and no retryAt", states["a"], tc.reason) + } + var unregistered int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items released by a failure that must not retry", unregistered) + } + assertRunWorkspaceReleased(t, store, runID) + }) + } +} + +// Cancellation wins over a waiting retry (D5): the node settles cancelled without waiting for the +// committed due instant, the run settles cancelled, the Workspace is released — and even a due +// instant that has already passed starts no attempt. The cancelled node keeps the attempt it failed +// on and carries no retryAt, so no later pass mistakes it for scheduled work. +func TestWorkflowRunCancelWhileRetryingStartsNoAttempt(t *testing.T) { + store := dispatcherDB(t) + runID, tid, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 2, 300)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec := startSessionNode(t, store, runID, wsID, "a") + execution := "exec-cancel-retry-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution, spec) + reportSessionEnd(t, store, runID, execution, nodeService, "agent_failed") + requireRetrying(t, readWorkflowNodeState(t, store, runID, "a"), 1) + + // The due instant passes before the request is recorded: the request still decides the run. + makeWorkflowRetryDue(t, store, runID, "a") + wid := workflowRunWorkflowID(t, store, runID) + mustCancelWorkflowRun(t, store, tid, wid, runID, "wf-cancel-while-retrying", cancelCaller(t, store, tid)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after the request: %v", err) + } + + status, runErr, states, _ := readWorkflowRun(t, store, runID) + if status != "cancelled" || runErr != "" { + t.Fatalf("run settled %q error %q states=%v; want cancelled", status, runErr, states) + } + agent, _ := asWorkflowObject(states["a"]) + if agent == nil || agent.S("status") != "cancelled" || agent.N("attempt") != 1 { + t.Fatalf("agent state = %v; want cancelled keeping attempt 1", states["a"]) + } + if agent.S("retryAt") != "" || agent.S("finishedAt") == "" || agent.S("error") != "" || agent.O("output").S("reason") != "cancelled" { + t.Fatalf("agent state = %v; a cancelled wait is finished, unscheduled and carries no error", agent) + } + var unregistered, sessions int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if err := store.Pool.QueryRow(`SELECT count(*) FROM node_executions WHERE operation_id=$1 AND workflow_node_id='a' AND kind='agent_session'`, runID).Scan(&sessions); err != nil { + t.Fatal(err) + } + if unregistered != 0 || sessions != 1 { + t.Fatalf("cancelled run holds %d work items and %d sessions; want none and the one that failed", unregistered, sessions) + } + // A later pass changes nothing. + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after cancellation: %v", err) + } + status, _, states, _ = readWorkflowRun(t, store, runID) + agent, _ = asWorkflowObject(states["a"]) + if status != "cancelled" || agent == nil || agent.S("retryAt") != "" { + t.Fatalf("a terminal run moved: status=%q agent=%v", status, states["a"]) + } + assertRunWorkspaceReleased(t, store, runID) +} diff --git a/internal/core/workflow_retry_test.go b/internal/core/workflow_retry_test.go new file mode 100644 index 00000000..c2abdadb --- /dev/null +++ b/internal/core/workflow_retry_test.go @@ -0,0 +1,163 @@ +package core + +import ( + "encoding/json" + "math" + "testing" + "time" +) + +// The retry policy's pure core: reading the frozen `agentConfig.retry`, computing the wait and +// deciding whether a failed attempt is replaced by a committed wait (specs decisions/cloud/workflow +// 20261021-node-retry-and-resume-contract.md D1–D3). The database-facing halves — where the wait +// instant is written and when it becomes due — are covered by workflow_retry_db_test.go. + +// retryNodeData wraps a retry value the way a frozen agent node carries it. +func retryNodeData(retry any) Object { + return Object{"agentConfig": Object{"retry": retry}} +} + +// retryPolicy is the full policy object hand-built documents and the editor write. +func retryPolicy(enabled, maxRetries, initialDelaySeconds any) Object { + return Object{"enabled": enabled, "maxRetries": maxRetries, "initialDelaySeconds": initialDelaySeconds} +} + +// The parser is total because a frozen snapshot is never re-gated: absent, null and every value the +// publish gate would refuse read as the default policy rather than making the scheduler unable to +// decide. The accepted numeric forms are the three the document can reach Go as: json.Number from +// the request decoder, float64 from a jsonb column, and the Go integer types hand-built documents +// use. +func TestWorkflowRetryPolicyOf(t *testing.T) { + def := defaultWorkflowRetryPolicy() + cases := []struct { + name string + data Object + want workflowRetryPolicy + }{ + {name: "an agent node with no agentConfig", data: Object{}, want: def}, + {name: "an agentConfig without retry", data: Object{"agentConfig": Object{"prompt": "x"}}, want: def}, + {name: "retry null", data: retryNodeData(nil), want: def}, + {name: "retry not an object", data: retryNodeData(true), want: def}, + {name: "retry an empty object", data: retryNodeData(Object{}), want: def}, + {name: "a partial object", data: retryNodeData(Object{"enabled": true}), want: def}, + { + // Present with every field, but maxRetries is outside the gate's range: a snapshot imported + // before the gate existed has no policy Cloud can honor, and inventing a budget from it is + // worse than the documented default. + name: "maxRetries past the gate's range", data: retryNodeData(retryPolicy(true, 6, 10)), want: def, + }, + {name: "a negative initial delay", data: retryNodeData(retryPolicy(true, 2, -1)), want: def}, + {name: "a fractional maxRetries", data: retryNodeData(retryPolicy(true, 1.5, 10)), want: def}, + {name: "enabled of the wrong type", data: retryNodeData(retryPolicy("yes", 2, 10)), want: def}, + { + name: "a complete policy", + data: retryNodeData(retryPolicy(false, int64(4), int64(25))), + want: workflowRetryPolicy{Enabled: false, MaxRetries: 4, InitialDelaySeconds: 25}, + }, + { + // A document that traveled through jsonb carries float64 numbers; an integral one is the + // same policy. + name: "float64 values from jsonb", + data: retryNodeData(retryPolicy(true, float64(0), float64(300))), + want: workflowRetryPolicy{Enabled: true, MaxRetries: 0, InitialDelaySeconds: 300}, + }, + { + name: "json.Number values from the decoder", + data: retryNodeData(retryPolicy(true, json.Number("5"), json.Number("0"))), + want: workflowRetryPolicy{Enabled: true, MaxRetries: 5, InitialDelaySeconds: 0}, + }, + { + // The schema may grow fields Cloud does not execute yet; an unknown key is not a reason to + // ignore the policy written beside it. + name: "a policy with an unknown key", + data: retryNodeData(Object{"enabled": true, "maxRetries": int64(1), "initialDelaySeconds": int64(5), "future": "value"}), + want: workflowRetryPolicy{Enabled: true, MaxRetries: 1, InitialDelaySeconds: 5}, + }, + { + // A non-finite value cannot be written to jsonb, but a hand-built document can carry one; + // it must not be truncated into a policy. + name: "a non-finite initial delay", data: retryNodeData(retryPolicy(true, 2, math.Inf(1))), want: def, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := workflowRetryPolicyOf(tc.data); got != tc.want { + t.Fatalf("policy = %+v, want %+v", got, tc.want) + } + }) + } +} + +// D1's wait is `initialDelaySeconds × 2^(n-1)` before retry n, capped at 600 seconds, where n is the +// 1-based retry number — the number of the attempt that failed. Both bounds are small, so a table can +// state the complete curve instead of asserting an arithmetic identity. +func TestWorkflowRetryWait(t *testing.T) { + cases := []struct { + name string + delay int64 + failedAttempt int64 + want time.Duration + }{ + {name: "first retry", delay: 10, failedAttempt: 1, want: 10 * time.Second}, + {name: "second retry doubles", delay: 10, failedAttempt: 2, want: 20 * time.Second}, + {name: "third retry doubles again", delay: 10, failedAttempt: 3, want: 40 * time.Second}, + {name: "fourth retry", delay: 10, failedAttempt: 4, want: 80 * time.Second}, + {name: "fifth retry", delay: 10, failedAttempt: 5, want: 160 * time.Second}, + {name: "sixth retry", delay: 10, failedAttempt: 6, want: 320 * time.Second}, + {name: "a large delay is also capped", delay: 300, failedAttempt: 3, want: 600 * time.Second}, + {name: "zero delay means due at once", delay: 0, failedAttempt: 1, want: 0}, + {name: "an attempt below one reads as the first", delay: 10, failedAttempt: 0, want: 10 * time.Second}, + {name: "an attempt past the curve is clamped to the last retry", delay: 10, failedAttempt: 99, want: 320 * time.Second}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + policy := workflowRetryPolicy{Enabled: true, MaxRetries: 5, InitialDelaySeconds: tc.delay} + if got := workflowRetryWait(policy, tc.failedAttempt); got != tc.want { + t.Fatalf("wait after attempt %d with delay %ds = %s, want %s", tc.failedAttempt, tc.delay, got, tc.want) + } + }) + } +} + +// The retry set is closed (D2): only `agent_failed` retries, and a reason outside the set settles the +// node terminal even when it maps to failed. The budget counts the failed attempt: attempt a may be +// followed by a+1 only while a <= maxRetries, so the run makes at most maxRetries+1 attempts (D1/D6). +func TestWorkflowRetryDecision(t *testing.T) { + enabled := workflowRetryPolicy{Enabled: true, MaxRetries: 2, InitialDelaySeconds: 10} + disabled := workflowRetryPolicy{Enabled: false, MaxRetries: 2, InitialDelaySeconds: 10} + noBudget := workflowRetryPolicy{Enabled: true, MaxRetries: 0, InitialDelaySeconds: 10} + cases := []struct { + name string + policy workflowRetryPolicy + state workflowNodeState + reason string + want bool + }{ + {name: "agent_failed within budget", policy: enabled, state: workflowNodeState{Attempt: 1}, reason: "agent_failed", want: true}, + {name: "agent_failed on the last allowed attempt", policy: enabled, state: workflowNodeState{Attempt: 2}, reason: "agent_failed", want: true}, + {name: "agent_failed past the budget", policy: enabled, state: workflowNodeState{Attempt: 3}, reason: "agent_failed", want: false}, + {name: "an absent attempt reads as the first", policy: enabled, state: workflowNodeState{}, reason: "agent_failed", want: true}, + {name: "a disabled policy", policy: disabled, state: workflowNodeState{Attempt: 1}, reason: "agent_failed", want: false}, + {name: "a zero budget", policy: noBudget, state: workflowNodeState{Attempt: 1}, reason: "agent_failed", want: false}, + {name: "interrupted is not retryable", policy: enabled, state: workflowNodeState{Attempt: 1}, reason: "interrupted", want: false}, + {name: "a user end is not retryable", policy: enabled, state: workflowNodeState{Attempt: 1}, reason: "user_ended", want: false}, + {name: "a cancelled session is not retryable", policy: enabled, state: workflowNodeState{Attempt: 1}, reason: "cancelled", want: false}, + {name: "an empty reason is not retryable", policy: enabled, state: workflowNodeState{Attempt: 1}, reason: "", want: false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + wait, retry := workflowRetryDecision(tc.policy, tc.state.attemptNumber(), tc.reason) + if retry != tc.want { + t.Fatalf("retry(%q at attempt %d) = %v, want %v", tc.reason, tc.state.attemptNumber(), retry, tc.want) + } + if retry { + want := workflowRetryWait(tc.policy, tc.state.attemptNumber()) + if wait != want { + t.Fatalf("wait = %s, want %s", wait, want) + } + } else if wait != 0 { + t.Fatalf("a non-retry returned a non-zero wait: %s", wait) + } + }) + } +} diff --git a/internal/core/workflow_run_cancel.go b/internal/core/workflow_run_cancel.go index f212ba19..20d1aec0 100644 --- a/internal/core/workflow_run_cancel.go +++ b/internal/core/workflow_run_cancel.go @@ -106,6 +106,10 @@ func cancellableWorkflowRun(run Object) bool { // settles the run `cancelled` without dispatching another node, and it keeps whatever evidence the // run already committed: // +// - A node waiting to retry: the wait is not a session, so there is nothing to end. The node +// settles cancelled, the run settles cancelled and the Workspace is released — the request does +// not wait for the committed due instant and does not start the attempt that was waiting for it +// (retry decision D5, invariant 6). // - No in-flight agent session: nothing to stop, so the request settles the run now. This is also // the whole story of a run cancelled while `pending`: it never claims, never stamps started_at, // never validates its graph and never provisions a Workspace — the unregistered work, if any, @@ -130,21 +134,33 @@ func cancellableWorkflowRun(run Object) bool { // work and declares its Workspace deletion like any other terminal run (D7). func advanceCancelledWorkflowRun(t *transaction, run Object, version int64, states map[string]workflowNodeState, unreachableAfter time.Duration) { runID := run.S("id") + // The retrying branch comes first: a waiting retry starts no session, so cancellation must not + // wait for its due instant, and the request wins even when retryAt has already passed (D5). + if nodeID, retrying := workflowRetryingAgentNode(states); retrying { + cancelWorkflowAgentNode(states, nodeID) + finishWorkflowRun(t, runID, version, "cancelled", "", states) + return + } nodeID, running := workflowRunningAgentNode(states) if !running { finishWorkflowRun(t, runID, version, "cancelled", "", states) return } + // Evidence is scoped to the attempt in flight: a cancelled node may have an earlier attempt's + // result committed on its own execution row, and reading it here would settle the cancel with + // that stale outcome instead of stopping the current session (retry decision D4). + nodeState := states[nodeID] + attempt := nodeState.attemptNumber() // The session's own terminal evidence is the node's state before the request is: it is what // actually ran. Reading it first also ends the session cleanly, without a pointless end request. if e := t.one(`SELECT result FROM node_executions - WHERE operation_id=$1 AND workflow_node_id=$2 AND kind='agent_session' AND result IS NOT NULL`, runID, nodeID); e != nil { + WHERE operation_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='agent_session' AND result IS NOT NULL`, runID, nodeID, attempt); e != nil { applyWorkflowAgentTerminal(states, nodeID, e.O("result")) finishWorkflowRun(t, runID, version, "cancelled", "", states) return } exec := t.one(`SELECT execution_id FROM node_executions - WHERE operation_id=$1 AND workflow_node_id=$2 AND kind='agent_session'`, runID, nodeID) + WHERE operation_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='agent_session'`, runID, nodeID, attempt) if exec == nil { // Committed running but never dispatched: there is no session to stop, and waiting for the // startup deadline would hold a run the user already asked to stop. @@ -158,9 +174,9 @@ func advanceCancelledWorkflowRun(t *transaction, run Object, version int64, stat finishWorkflowRun(t, runID, version, "cancelled", "", states) return } - cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND kind='end_session'", runID, nodeID) + cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='end_session'", runID, nodeID, attempt) if cmd == nil { - cmd = enqueueWorkflowThreadCommand(t, runID, nodeID, "end_session", Object{"reason": "cancelled"}) + cmd = enqueueWorkflowThreadCommand(t, runID, nodeID, attempt, "end_session", Object{"reason": "cancelled"}) } window := startupDuration(t.store.AgentEndConfirmTimeout, 5*time.Minute) if t.one(`SELECT id FROM thread_commands WHERE id=$1 @@ -180,6 +196,8 @@ func applyWorkflowAgentTerminal(states map[string]workflowNodeState, nodeID stri reason := workflowAgentTerminalReason(result) state := states[nodeID] state.Status = workflowSessionEndReason(result.S("reason")) + state.Attempt = state.attemptNumber() + state.RetryAt = "" state.FinishedAt = nowRFC3339() state.Output = Object{"reason": reason} if state.Status == "failed" { @@ -206,6 +224,10 @@ func workflowAgentTerminalReason(result Object) string { func cancelWorkflowAgentNode(states map[string]workflowNodeState, nodeID string) { state := states[nodeID] state.Status = "cancelled" + state.Attempt = state.attemptNumber() + // A cancelled wait has no next attempt, so the committed due instant must not survive into the + // terminal state: a later pass reading it would see a cancelled node that still looks scheduled. + state.RetryAt = "" state.FinishedAt = nowRFC3339() state.Error = "" state.Output = Object{"reason": "cancelled"} diff --git a/internal/core/workflow_run_session.go b/internal/core/workflow_run_session.go index b4cdf921..b78800db 100644 --- a/internal/core/workflow_run_session.go +++ b/internal/core/workflow_run_session.go @@ -74,15 +74,21 @@ func workflowRunningAgentNode(states map[string]workflowNodeState) (string, bool // dispatched fails the node once the startup deadline passes. The run advances on the next tick // after any commit. // -// Every lookup is scoped to the node being settled. A run has several agent sessions over its life -// (D4 executes its agent nodes serially through one Workspace), so evidence read at run scope — -// "the newest agent_session result for this run" — would let the second node inherit the first -// node's result and be recorded succeeded without ever executing (invariant 5). -func workflowAgentSessionOutcome(t *transaction, runID string, version int64, nodeID string, states map[string]workflowNodeState, unreachableAfter time.Duration) (map[string]workflowNodeState, bool) { +// Every lookup is scoped to the node being settled and to its current attempt. A run has several +// agent sessions over its life (D4 executes its agent nodes serially through one Workspace), so +// evidence read at run scope — "the newest agent_session result for this run" — would let the second +// node inherit the first node's result and be recorded succeeded without ever executing (invariant +// 5). The same argument applies inside one node once it retries: attempt 1's committed result stays +// on its own execution row, and a lookup that does not name the current attempt would settle the +// in-flight attempt 2 with it (retry decision D4). The attempt comes from the committed state, so a +// restart, a replica change and a replay all scope to the same row. +func workflowAgentSessionOutcome(t *transaction, runID string, version int64, node workflowNodeView, states map[string]workflowNodeState, unreachableAfter time.Duration) (map[string]workflowNodeState, bool) { + nodeID, nodeState := node.ID, states[node.ID] + attempt := nodeState.attemptNumber() e := t.one(`SELECT result FROM node_executions - WHERE operation_id=$1 AND workflow_node_id=$2 AND kind='agent_session' AND result IS NOT NULL`, runID, nodeID) + WHERE operation_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='agent_session' AND result IS NOT NULL`, runID, nodeID, attempt) if e != nil { - return commitWorkflowAgentTerminal(t, runID, version, nodeID, states, e.O("result")) + return commitWorkflowAgentTerminal(t, runID, version, node, states, e.O("result")) } // Dispatched but not yet terminal: the Node owns the session until it reports — and a session may // legitimately run for hours, so elapsed time is not evidence of anything. The Node is. When the @@ -99,7 +105,7 @@ func workflowAgentSessionOutcome(t *transaction, runID string, version int64, no // Workspace before it commits the node `running` — and the binding is never cleared, only // soft-deleted, which the live-sandbox half of the judgement already covers. if e := t.one(`SELECT execution_id FROM node_executions - WHERE operation_id=$1 AND workflow_node_id=$2 AND kind='agent_session'`, runID, nodeID); e != nil { + WHERE operation_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='agent_session'`, runID, nodeID, attempt); e != nil { if w := t.one("SELECT id FROM workspaces WHERE workflow_run_id=$1", runID); w != nil && runWorkspaceNodeUnknown(t, runID, w.S("id"), unreachableAfter) { state := states[nodeID] @@ -130,7 +136,7 @@ func workflowAgentSessionOutcome(t *transaction, runID string, version int64, no finishWorkflowRun(t, runID, version, "failed", workflowSessionTurnDeadlineExceeded, states) return states, false } - return workflowSessionEndRequest(t, runID, version, nodeID, executionID, turnEnded, states) + return workflowSessionEndRequest(t, runID, version, nodeID, attempt, executionID, turnEnded, states) } // Never dispatched: the startup deadline applies. A node committed running with no registered // execution past the never dispatched fails honestly — never recorded as succeeded (invariant 5). @@ -161,13 +167,15 @@ func workflowAgentSessionOutcome(t *transaction, runID string, version int64, no // never claims the command would leave the run `running` forever — the same permanent stall this // path exists to remove. The deadline is measured from the command's own `created_at`, so retries // and replicas all read the same window. -func workflowSessionEndRequest(t *transaction, runID string, version int64, nodeID, executionID string, turnEnded bool, states map[string]workflowNodeState) (map[string]workflowNodeState, bool) { - cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND kind='end_session'", runID, nodeID) +func workflowSessionEndRequest(t *transaction, runID string, version int64, nodeID string, attempt int64, executionID string, turnEnded bool, states map[string]workflowNodeState) (map[string]workflowNodeState, bool) { + // The end request is one per (run, node, attempt): 0061 re-keys 0049's uniqueness on the attempt, + // so attempt 1's outstanding request is neither reused nor delivered to attempt 2's session. + cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='end_session'", runID, nodeID, attempt) if cmd == nil { if !turnEnded { return states, false } - cmd = enqueueWorkflowThreadCommand(t, runID, nodeID, "end_session", Object{"reason": "idle_timeout"}) + cmd = enqueueWorkflowThreadCommand(t, runID, nodeID, attempt, "end_session", Object{"reason": "idle_timeout"}) } window := startupDuration(t.store.AgentEndConfirmTimeout, 5*time.Minute) // The comparison runs on the database clock, the same clock `created_at` came from: a process @@ -236,11 +244,38 @@ func workflowSessionEndReason(reason string) string { } } -// commitWorkflowAgentTerminal commits a running agent node's terminal state from the session -// execution's result document. A failed or cancelled node finishes the run (the wave resolves -// downstream casualties on the next pass); a succeeded node commits and reports that the run can -// continue advancing in the same pass — the next wave executes downstream of it. -func commitWorkflowAgentTerminal(t *transaction, runID string, version int64, nodeID string, states map[string]workflowNodeState, result Object) (map[string]workflowNodeState, bool) { +// commitWorkflowAgentTerminal commits a running agent node's state from the session execution's +// result document. A retryable failure becomes a committed `retrying` state and the pass stops — the +// run stays running and the attempt is retried after its committed wait. Otherwise a failed or +// cancelled node finishes the run (the wave resolves downstream casualties on the next pass), and a +// succeeded node commits and reports that the run can continue advancing in the same pass — the next +// wave executes downstream of it. +// +// The retry decision is made from result.reason before the reason is mapped (D2). The mapping sends +// several unrelated reasons to `failed` — interrupted, Cloud's own deadlines, node_unavailable — +// and treats every unknown reason as success; deriving the retry from the mapped status would both +// retry what the ADR forbids and skip the decision for reasons that map to success. +func commitWorkflowAgentTerminal(t *transaction, runID string, version int64, node workflowNodeView, states map[string]workflowNodeState, result Object) (map[string]workflowNodeState, bool) { + nodeID := node.ID + state := states[nodeID] + state.Attempt = state.attemptNumber() + states[nodeID] = state + if wait, retry := workflowRetryDecision(workflowRetryPolicyOf(node.Data), state.attemptNumber(), result.S("reason")); retry { + state.Status = "retrying" + state.RetryAt = workflowRetryAt(t, wait) + // The attempt that failed keeps its start instant and its reason; it has no finish instant, + // because the run has not finished with this node — it is waiting. error/output are the failed + // attempt's evidence and stay readable while it waits (D3). + state.FinishedAt = "" + state.Error = result.S("reason") + state.Output = Object{"reason": result.S("reason")} + states[nodeID] = state + // A waiting node is not terminal: the run keeps status running with an empty error, retains + // its Workspace and releases no work. The state and the run row commit together under the + // existing version CAS, so every later pass wakes against exactly this retryAt. + settleWorkflowRunStates(t, runID, version, "running", "", states) + return states, false + } applyWorkflowAgentTerminal(states, nodeID, result) reason := workflowAgentTerminalReason(result) switch states[nodeID].Status { @@ -400,16 +435,27 @@ func startWorkflowAgentSession(t *transaction, run Object, node workflowNodeView failWorkflowAgentNode(t, runID, version, states, node, reason) return } - states[node.ID] = workflowNodeState{Status: "running", DisplayName: workflowNodeDisplayName(node), StartedAt: nowRFC3339()} + // The attempt follows the committed state: the first start is attempt 1 and the start after a + // committed `retrying` is the attempt it was waiting for plus one (D3). Nothing about the counter + // lives in process memory, so a restart between the wait and this start reads the same number. + // The fresh state deliberately drops the previous attempt's retryAt, error and finishedAt. + attempt := int64(1) + if prior, started := states[node.ID]; started { + attempt = prior.attemptNumber() + 1 + } + states[node.ID] = workflowNodeState{Status: "running", DisplayName: workflowNodeDisplayName(node), StartedAt: nowRFC3339(), Attempt: attempt} if !settleWorkflowRunStates(t, runID, version, "running", "", states) { return } - enqueueExecutionWork(t, runID, "agent_session", spec, target, node.ID, time.Time{}) + enqueueExecutionWork(t, runID, "agent_session", spec, target, node.ID, attempt, time.Time{}) } -// failWorkflowAgentNode commits one agent node failed with a named reason and finishes the run. +// failWorkflowAgentNode commits one agent node failed with a named reason and finishes the run. It +// keeps the attempt the state already names — the attempt whose start could not proceed — and reads +// an absent state as the first attempt; the budget is history, not something a failure resets (D6). func failWorkflowAgentNode(t *transaction, runID string, version int64, states map[string]workflowNodeState, node workflowNodeView, reason string) { - states[node.ID] = workflowNodeState{Status: "failed", DisplayName: workflowNodeDisplayName(node), StartedAt: nowRFC3339(), FinishedAt: nowRFC3339(), Error: reason, Output: Object{"reason": reason}} + prior := states[node.ID] + states[node.ID] = workflowNodeState{Status: "failed", DisplayName: workflowNodeDisplayName(node), StartedAt: nowRFC3339(), FinishedAt: nowRFC3339(), Attempt: prior.attemptNumber(), Error: reason, Output: Object{"reason": reason}} finishWorkflowRun(t, runID, version, "failed", reason, states) } diff --git a/internal/core/workflow_run_session_db_test.go b/internal/core/workflow_run_session_db_test.go index ac33fa96..afe199c7 100644 --- a/internal/core/workflow_run_session_db_test.go +++ b/internal/core/workflow_run_session_db_test.go @@ -21,6 +21,24 @@ func workflowSessionGraph() Object { return g } +// workflowSessionNoRetryGraph is workflowSessionGraph with retry explicitly disabled on its agent +// node. Tests that assert how a failed attempt settles without a committed wait seed this graph so +// the default policy — enabled (D1) — does not turn their failure into a `retrying` node. The +// retry-enabled settlement has its own tests in workflow_retry_db_test.go. +func workflowSessionNoRetryGraph() Object { + return wfGraph( + []Object{ + wfNode("s", "start", ""), + wfAgentNode("a", Object{ + "prompt": "Do the thing", + "retry": Object{"enabled": false, "maxRetries": 0, "initialDelaySeconds": 0}, + }), + wfNode("o", "output", ""), + }, + [][3]string{{"s", "a", ""}, {"a", "o", ""}}, + ) +} + // workflowTwoAgentGraph is start -> agent a1 -> agent a2 -> output: one run with two agent nodes, // which D4 executes serially through the run's single Workspace. func workflowTwoAgentGraph() Object { @@ -429,7 +447,10 @@ func TestWorkflowSessionDispatchIsNotRepeatable(t *testing.T) { // decides, not a policy — and the terminal cleanup still runs. func TestWorkflowSessionFailureFailsRun(t *testing.T) { store := dispatcherDB(t) - runID, _, _, _, _ := seedWorkflowSessionScene(t, store) + // Retry is disabled on this graph: the test is about a failure that settles the node and run + // immediately. The retry-enabled outcome (a committed `retrying`, the run still running) is + // asserted in workflow_retry_db_test.go. + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowSessionNoRetryGraph()) if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { t.Fatalf("advance 1: %v", err) } diff --git a/internal/core/workflow_scheduler.go b/internal/core/workflow_scheduler.go index 26de7b41..714ec48a 100644 --- a/internal/core/workflow_scheduler.go +++ b/internal/core/workflow_scheduler.go @@ -239,20 +239,46 @@ type workflowNodeState struct { DisplayName string StartedAt string FinishedAt string + // Attempt is the 1-based attempt this state describes (retry decision D4). It is committed on + // every agent-node state — running, retrying and terminal — because the scheduler reads the + // budget and the evidence scope back from it after a restart, a replica change or a replay; the + // attempt counter is not kept anywhere else. Non-agent nodes have no attempts and leave it zero. + Attempt int64 + // RetryAt is the earliest instant at which the next attempt may start (retry decision D3). Only + // a `retrying` node carries it, and it is the database clock's instant, so every pass judges the + // same wake-up. A terminal or running state has none. + RetryAt string // Error is the node's failure reason, and only a failed node has one: a cancelled node was // cancelled, not broken, and an idle or succeeded one has nothing to report. The run viewer // renders it in its own Error section, which is why it is a field of the state rather than // something the viewer has to dig out of Output — a node's Output is what it produced, and a - // failed node produced nothing. + // failed node produced nothing. A `retrying` node keeps the failed attempt's reason: the retry + // replaces a failure, it does not erase it. Error string Output Object } +// attemptNumber is the state's 1-based attempt, reading an absent attempt as the first one. Only +// agent nodes carry attempts; a state written before attempts existed, or by a non-agent node, +// belongs to attempt 1. +func (s *workflowNodeState) attemptNumber() int64 { + if s.Attempt < 1 { + return 1 + } + return s.Attempt +} + // object renders the state in the shape the run viewer reads (and the simulator writes). The receiver // is a pointer because the state is large enough that copying it per render is waste, and its one // caller already holds it in a range variable. func (s *workflowNodeState) object() Object { out := Object{"status": s.Status, "displayName": s.DisplayName, "startedAt": s.StartedAt, "finishedAt": s.FinishedAt} + if s.Attempt >= 1 { + out["attempt"] = s.Attempt + } + if s.RetryAt != "" { + out["retryAt"] = s.RetryAt + } if s.Error != "" { out["error"] = s.Error } @@ -278,6 +304,8 @@ func workflowStatesFromJSON(raw Object) map[string]workflowNodeState { DisplayName: node.S("displayName"), StartedAt: node.S("startedAt"), FinishedAt: node.S("finishedAt"), + Attempt: node.N("attempt"), + RetryAt: node.S("retryAt"), Error: node.S("error"), } if output, ok := asWorkflowObject(node["output"]); ok { @@ -510,12 +538,52 @@ func advanceWorkflowRun(t *transaction, runID string, unreachableAfter time.Dura // the user globals frozen in the snapshot. They are derived from the run row on every pass, so a // retry, recovery or re-dispatch reads the same frozen values the run was created with. ambient := workflowAmbientStates(run, run.O("graph")) + // A committed `retrying` node owns the run's serial agent slot (D3): until its committed due + // instant the pass writes nothing, and once due it starts the next attempt through the same path + // a fresh node takes. The branch has to sit here — after the cancel branch and before the + // running-session settlement — and it always returns: `retrying` is neither failed nor ready, so + // falling through would let the wave loop's "no failure left" rule settle the run succeeded while + // the node is still waiting (invariant 3's converse). + if nodeID, retrying := workflowRetryingAgentNode(states); retrying { + node, known := graph.ByNode[nodeID] + if !known { + // The state names a node the frozen graph does not contain. No session can be started for + // it and no wake-up can ever do anything, so the run fails with Cloud's own explanation + // rather than waiting forever — the treatment a snapshot with no start node gets. + state := states[nodeID] + state.Status = "failed" + state.RetryAt = "" + state.FinishedAt = nowRFC3339() + state.Error = "graph node missing" + states[nodeID] = state + finishWorkflowRun(t, runID, version, "failed", "graph has no retrying node", states) + return + } + if !workflowRetryDue(t, runID, nodeID) { + return + } + startWorkflowAgentSession(t, run, node, version, states) + return + } // A committed running agent node settles from its session execution's terminal evidence before // anything else runs: sessions serialize per run (one unregistered agent_session work item per // run), and the next wave waits for the terminal commit. The pass returns after any outcome — // a settled node lets the next tick execute its downstream wave. if nodeID, running := workflowRunningAgentNode(states); running { - if _, cont := workflowAgentSessionOutcome(t, runID, version, nodeID, states, unreachableAfter); !cont { + node, known := graph.ByNode[nodeID] + if !known { + // Same corruption as the retrying branch: the node's session was built from this graph, so + // a missing entry means the snapshot cannot explain its own state. Fail honestly instead of + // looking for evidence that can never settle. + state := states[nodeID] + state.Status = "failed" + state.FinishedAt = nowRFC3339() + state.Error = "graph node missing" + states[nodeID] = state + finishWorkflowRun(t, runID, version, "failed", "graph has no running node", states) + return + } + if _, cont := workflowAgentSessionOutcome(t, runID, version, node, states, unreachableAfter); !cont { return } // A settled session commits its terminal state and continues the pass; that commit bumped From 501838676361acec77d5aac6840a14c2ce7d6586 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 10:42:00 +0800 Subject: [PATCH 02/14] feat(workflows): register the run resume contract as unavailable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implement the resume half of decisions/cloud/workflow/ 20261021-node-retry-and-resume-contract.md (D7–D8): - POST /api/v1/tenants/:tid/workflows/:wfid/runs/:rid/resume is authorized like cancel and validated strictly: an unknown field and a non-string, null or unknown mode are 400, while a valid keep/node_files/checkpoint request is always 501 workflow_run_resume_unavailable - a refused request writes no run state and records no idempotency key, so a replay receives the same refusal; the UI has no resume entry - the OpenAPI document and both workflow guides describe the retrying node shape, the attempt and retryAt fields, and the unavailable endpoint Refs ora-space/circus#469 Co-Authored-By: Claude Code --- docs/workflows/workflows.en.md | 22 +++ docs/workflows/workflows.md | 14 ++ integration/workflow_run_resume_test.go | 201 ++++++++++++++++++++++++ internal/api/router/router.go | 9 ++ internal/contract/openapi.go | 33 +++- internal/core/public.go | 7 + internal/core/workflow_run_resume.go | 48 ++++++ 7 files changed, 332 insertions(+), 2 deletions(-) create mode 100644 integration/workflow_run_resume_test.go create mode 100644 internal/core/workflow_run_resume.go diff --git a/docs/workflows/workflows.en.md b/docs/workflows/workflows.en.md index 9f174559..1b123beb 100644 --- a/docs/workflows/workflows.en.md +++ b/docs/workflows/workflows.en.md @@ -113,6 +113,15 @@ node states, session or Workspace; the scheduler settles it on its next pass (se bindings frozen on the run: on a bound run only the initiator and the tenant's administrators may cancel, and any other member answers `403 workflow_run_cancel_forbidden`; an unbound run keeps the Issue Thread's member rule. +**The resume endpoint is registered but unavailable in this phase.** `POST .../workflows/:wfid/runs/:rid/resume` +validates the request against the final contract: `mode` is optional and is one of `keep` (the default), `node_files` +and `checkpoint`; an unknown field is `400 unknown_field` and an invalid value is `400 invalid_field_value`. +Authorization is cancellation's: on a bound run only the initiator and the tenant's administrators may call it, and +any other member answers `403 workflow_run_resume_forbidden`. Every valid request answers +`501 workflow_run_resume_unavailable` and writes nothing — no run state and no idempotency record — so a repeat is +the same refusal rather than a replay of a recorded success. A failed run's Workspace is already declared deleted, +leaving `keep` nothing to resume onto, so the frontend offers no resume entry point. + ## Scheduling WorkflowRuns are carried forward by a scheduling loop in the Cloud server (`AdvanceWorkflowRunsOnce`, every 10 @@ -160,6 +169,18 @@ seconds): of its Workspace. In the first phase this session has no Git Server access (reads and writes get `403`), and its Workspace is not listed in an IssueRun session's Workspace directory; see [Git Server](../repositories/git-server.en.md). +- **Agent-node retry is decided by the `agentConfig.retry` frozen on the node.** A session retries only when it ends + as `agent_failed`, the policy is enabled and the budget is not spent: the budget is `maxRetries + 1` attempts + (`maxRetries` ranges 0–5 and defaults to 2), and the next attempt starts after `initialDelaySeconds × 2^(n-1)` + (`initialDelaySeconds` ranges 0–300 and defaults to 10, with a 600-second cap per wait). While it waits, the node + commits `retrying`: the run stays `running`, the node keeps the failed attempt's `startedAt`, `error` and + `output.reason`, records `retryAt` from the database clock and clears `finishedAt`; it keeps the run's single + session slot until `retryAt` arrives. The new attempt has its own execution identity (`workflow_node_attempt`), so + the previous attempt's result and end request are never reused. A spent budget, a disabled policy or a reason + outside `agent_failed` fails the node with the last reason, and the run reaches a terminal state and releases its + Workspace. At publish, `agentConfig.retry` must be a complete object; absent or `null` means the default policy, + and unknown keys are ignored. A frozen historical snapshot is never re-gated, and a policy that cannot be parsed + runs as the default. - **A workflow session is single-turn, and Cloud decides when it ends.** Once the initial turn's `turnEnded` receipt arrives (with a `stop_reason` other than `cancelled`), the scheduler ends the session through the existing `EndSession` command relay; the session result the Node reports remains the node's only terminal evidence. If the @@ -231,6 +252,7 @@ rewrites imported documents or published versions. - [Workflows are executed by a Cloud-side scheduler: WorkflowRun is the execution aggregate](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/0-cloud-side-scheduler-owns-workflow-execution.md) (approved) - [Workflow agent nodes reference executors by executor slot; graphs no longer store Agent CLIs or model IDs](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261015-executor-slots-replace-agent-model-ids.md) (approved) - [A workflow session ends after its initial turn by the Cloud scheduler, with a bounded wait on the end request](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261019-workflow-session-end-after-initial-turn.md) +- [Workflow agent nodes retry under a retry policy; the resume endpoint is registered against its final contract but unavailable in the first phase](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261021-node-retry-and-resume-contract.md) (approved) - [Plugin asset import produces published snapshots: the asset lifecycle never rewrites snapshots, and runs freeze only the snapshotId](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261016-asset-import-and-published-snapshots.md) - [Workflow plugin assets are fetched by Cloud and imported document by document as editable copies](https://github.com/ora-space/specs/blob/main/decisions/cloud/plugin-marketplace/20261016-workflow-plugin-assets.md) - [Personal slot mappings resolve a workflow's executors; each run pins one model binding per slot](https://github.com/ora-space/specs/blob/main/decisions/cloud/model-access/20261015-executor-slot-mappings-and-per-slot-run-bindings.md) (approved) diff --git a/docs/workflows/workflows.md b/docs/workflows/workflows.md index 7b2a2f14..fdedda3e 100644 --- a/docs/workflows/workflows.md +++ b/docs/workflows/workflows.md @@ -81,6 +81,12 @@ Project 缺失时统一返回 `409 workflow_run_project_required`,不可用、 `pending` 和 `running` 的运行接受请求;已经进入终态的运行返回 `409 run_not_cancellable`。授权按运行上冻结的模型绑定判断:带绑定的运行只有发起人和 租户管理员可以取消,其他成员返回 `403 workflow_run_cancel_forbidden`;没有绑定的运行沿用 Issue 线程的成员规则。 +**恢复接口已注册,但本阶段不可用。** `POST .../workflows/:wfid/runs/:rid/resume` 按最终契约校验请求体:`mode` 可选,取值 +`keep`(默认)、`node_files`、`checkpoint`,未知字段返回 `400 unknown_field`,非法取值返回 `400 invalid_field_value`;授权与取消相同, +带冻结绑定的运行只有发起人和租户管理员可以调用,其他成员返回 `403 workflow_run_resume_forbidden`。任何合法请求都返回 +`501 workflow_run_resume_unavailable`,不改动运行状态,也不写幂等记录——重复请求得到同样的拒绝,而不是重放一条记录下来的成功。运行的 +Workspace 在失败时已经声明删除,`keep` 没有可恢复的状态,所以前端不提供恢复入口。 + ## 调度 WorkflowRun 由 Cloud server 里的调度循环推进(`AdvanceWorkflowRunsOnce`,每 10 秒一轮): @@ -107,6 +113,13 @@ WorkflowRun 由 Cloud server 里的调度循环推进(`AdvanceWorkflowRunsOnce 已提交的结果按它自己的原因结算,Node 不可达仍报 `node_unavailable`,两者都不成立时才轮到窗口;窗口把运行带入终态之后才到达的结果 保留在执行行上,不改写已经终态的运行。运行进入终态后,Cloud 声明删除这次运行的 Workspace。第一阶段这个会话没有 Git Server 访问(读写都返回 `403`),它的 Workspace 也不进 IssueRun 会话的 Workspace 目录,见 [Git Server](../repositories/git-server.md)。 +- **Agent 节点重试由冻结在节点上的 `agentConfig.retry` 决定。** 只有会话以 `agent_failed` 结束、策略启用且预算未用完时才重试:预算是 + `maxRetries + 1` 次尝试(`maxRetries` 取值 0–5,默认 2),下一次尝试在 `initialDelaySeconds × 2^(n-1)` 之后开始(`initialDelaySeconds` + 取值 0–300,默认 10,单次等待上限 600 秒)。等待期间节点提交 `retrying`:运行仍是 `running`,节点保留失败尝试的 `startedAt`、 + `error` 和 `output.reason`,写入数据库时钟计算的 `retryAt` 并清空 `finishedAt`;它继续占用运行唯一的会话位置,直到 `retryAt` 到达才启动 + 下一次尝试。新尝试使用自己的执行身份(`workflow_node_attempt`),上一次尝试的结果和结束请求不会被它复用。预算用完、策略关闭或原因不在 + `agent_failed` 内时,节点以最后一次的原因失败,运行进入终态并释放 Workspace。发布时 `agentConfig.retry` 必须是完整对象,缺失或为 `null` + 表示默认策略,未知键忽略;已冻结的历史快照不重新校验,无法解析的策略按默认值执行。 - **工作流会话是单回合的,结束由 Cloud 决定。** 初始回合的 `turnEnded` 回执(`stop_reason` 不是 `cancelled`)到达后,调度器通过既有的 `EndSession` 命令让 Node 结束会话;Node 上报的会话执行结果仍是节点终态的唯一证据。结束请求放出后超过确认窗口仍没有会话终态时,节点以 `session_end_deadline_exceeded` 失败,运行进入终态并释放 Workspace——不会再有永久 `running` 的运行。 @@ -151,6 +164,7 @@ WorkflowRun 的实际状态更新成功后,在同一事务里将状态、时 - [工作流由 Cloud 侧调度器执行:WorkflowRun 是执行聚合](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/0-cloud-side-scheduler-owns-workflow-execution.md)(approved) - [工作流 agent 节点按执行器槽位引用执行器,图里不再存 agent CLI 与模型 ID](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261015-executor-slots-replace-agent-model-ids.md)(approved) - [工作流会话在初始回合结束后由 Cloud 调度器结束,结束请求有界等待](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261019-workflow-session-end-after-initial-turn.md) +- [工作流 agent 节点按重试策略重试,恢复接口按最终契约注册但在首期不可用](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261021-node-retry-and-resume-contract.md)(approved) - [插件资产导入产出已发布快照:资产生命周期不改写快照,运行只冻结 snapshotId](https://github.com/ora-space/specs/blob/main/decisions/cloud/workflow/20261016-asset-import-and-published-snapshots.md) - [Workflow 插件资产由 Cloud 自行取包、逐文档导入为可编辑副本](https://github.com/ora-space/specs/blob/main/decisions/cloud/plugin-marketplace/20261016-workflow-plugin-assets.md) - [个人槽位映射为工作流解析执行器,每次运行按槽位锁定多组模型绑定](https://github.com/ora-space/specs/blob/main/decisions/cloud/model-access/20261015-executor-slot-mappings-and-per-slot-run-bindings.md)(approved) diff --git a/integration/workflow_run_resume_test.go b/integration/workflow_run_resume_test.go new file mode 100644 index 00000000..0c4e13fb --- /dev/null +++ b/integration/workflow_run_resume_test.go @@ -0,0 +1,201 @@ +package integration + +// Acceptance for the workflow-run resume endpoint (specs decisions/cloud/workflow +// 20261021-node-retry-and-resume-contract.md D7; circus#460). +// +// The final contract resumes a terminal failed run inside the same run identity. This phase ships the +// endpoint's shape and nothing else: authorization identical to cancellation, the final request +// vocabulary validated at the HTTP boundary, and a stable 501 for every valid request. These cases +// drive the shipped route, because the obligations are about what a client observes: +// +// - every valid request is answered `501 workflow_run_resume_unavailable`, regardless of the run's +// status; +// - the refusal writes nothing — no run state, no Workspace or execution rows, and no idempotency +// record, so a repeat under the same key is the same refusal rather than a replay; +// - an unknown body field is the router's `400 unknown_field`, and an invalid `mode` is +// `400 invalid_field_value`; +// - only the run's initiator and the tenant's administrators may resume a run carrying frozen +// executor bindings, and a run outside this tenant/workflow, a soft-deleted run and a non-id are +// all `404 not_found`. + +import ( + "context" + "testing" + + "github.com/golang-jwt/jwt/v5" + + "github.com/wanglongan587/cloud/internal/core" +) + +func workflowRunResumePath(tenant, workflow, run string) string { + return "/api/v1/tenants/" + tenant + "/workflows/" + workflow + "/runs/" + run + "/resume" +} + +// resumeWorkflowRun issues the production resume request as the fixture's verified user. +func (f *fixture) resumeWorkflowRun(wfid, runID, key string, body core.Object, want int, code string) core.Object { + f.t.Helper() + return f.resumeWorkflowRunAs(f.user, f.uid, f.tid, wfid, runID, key, body, want, code) +} + +// resumeWorkflowRunAs issues the request as a specific caller and asserts the exact status and stable +// public code. +func (f *fixture) resumeWorkflowRunAs(user core.Claims, uid, tenant, wfid, runID, key string, body core.Object, want int, code string) core.Object { + f.t.Helper() + out, status, err := f.client.Call(context.Background(), "POST", + workflowRunResumePath(tenant, wfid, runID), "gateway", + core.Claims{RegisteredClaims: jwt.RegisteredClaims{Subject: "gateway-a"}}, &user, key, body) + must(f.t, err) + if status != want { + f.t.Fatalf("POST workflow run/resume: want %d got %d %v", want, status, out) + } + if code != "" && out.S("code") != code { + f.t.Fatalf("POST workflow run/resume: want code %q got %v", code, out) + } + return out +} + +// resumeRecordCount reads whether a key left a durable idempotency record. The 501 is a refusal, so +// it must not: the recorded-response path is for committed successes. +func (f *fixture) resumeRecordCount(uid, key string) int { + f.t.Helper() + return f.scalar(`SELECT count(*) FROM idempotency_records WHERE tenant_id=$1 AND user_id=$2 AND key=$3`, f.tid, uid, key) +} + +// resumeSideEffects counts the rows a resume could plausibly write if it were implemented by accident +// or left a partial write behind. +func (f *fixture) resumeSideEffects(runID string) (workspaces, work, executions, commands int) { + f.t.Helper() + return f.scalar(`SELECT count(*) FROM workspaces WHERE workflow_run_id=$1`, runID), + f.scalar(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1`, runID), + f.scalar(`SELECT count(*) FROM node_executions WHERE operation_id=$1`, runID), + f.scalar(`SELECT count(*) FROM thread_commands WHERE workflow_run_id=$1`, runID) +} + +// Every valid request — each documented mode and an absent mode (the `keep` default) — is refused +// with the published 501. The run row is byte-for-byte unchanged, no Workspace or execution rows +// appear, and no idempotency record is written: the same key repeated is refused the same way, not +// replayed as a recorded success. +func TestWorkflowRunResumeIsUnavailableForEveryValidRequest(t *testing.T) { + f := setup(t) + wfid, runID := stageWorkflowRun(t, f, "resume-valid", "failed", f.uid) + before := f.workflowRunFacts(runID) + + bodies := []core.Object{ + {"mode": "keep"}, + {"mode": "node_files"}, + {"mode": "checkpoint"}, + {}, // mode defaults to keep + } + for i, body := range bodies { + key := "wf-resume-valid-" + string(rune('a'+i)) + out := f.resumeWorkflowRun(wfid, runID, key, body, 501, "workflow_run_resume_unavailable") + if out.S("code") != "workflow_run_resume_unavailable" { + t.Fatalf("body %v: want workflow_run_resume_unavailable, got %v", body, out) + } + if after := f.workflowRunFacts(runID); after != before { + t.Fatalf("body %v changed the run row:\nbefore %s\nafter %s", body, before, after) + } + if n := f.resumeRecordCount(f.uid, key); n != 0 { + t.Fatalf("body %v left %d idempotency records", body, n) + } + if worked, items, executions, commands := f.resumeSideEffects(runID); worked != 0 || items != 0 || executions != 0 || commands != 0 { + t.Fatalf("body %v wrote side effects: workspaces=%d work=%d executions=%d commands=%d", body, worked, items, executions, commands) + } + } + + // A repeat under the first key is the same 501, because there is no recorded response to replay. + f.resumeWorkflowRun(wfid, runID, "wf-resume-valid-a", core.Object{"mode": "keep"}, 501, "workflow_run_resume_unavailable") + if after := f.workflowRunFacts(runID); after != before { + t.Fatalf("a repeated refusal changed the run row:\nbefore %s\nafter %s", before, after) + } + if n := f.resumeRecordCount(f.uid, "wf-resume-valid-a"); n != 0 { + t.Fatalf("a repeated refusal recorded %d idempotency rows", n) + } +} + +// The HTTP boundary validates the final request vocabulary before availability is decided: a field +// the contract does not define is the router's unknown_field, and a mode outside the closed set is +// invalid_field_value. Neither is answered as unavailable, and neither writes anything. +func TestWorkflowRunResumeRejectsAnUnknownFieldAndAnInvalidMode(t *testing.T) { + f := setup(t) + wfid, runID := stageWorkflowRun(t, f, "resume-invalid", "failed", f.uid) + before := f.workflowRunFacts(runID) + + f.resumeWorkflowRun(wfid, runID, "wf-resume-unknown", core.Object{"mode": "keep", "future": true}, 400, "unknown_field") + for _, mode := range []any{nil, "everything", 3, false, []any{"keep"}} { + f.resumeWorkflowRun(wfid, runID, "wf-resume-mode", core.Object{"mode": mode}, 400, "invalid_field_value") + } + if after := f.workflowRunFacts(runID); after != before { + t.Fatalf("a rejected body changed the run row:\nbefore %s\nafter %s", before, after) + } + for _, key := range []string{"wf-resume-unknown", "wf-resume-mode"} { + if n := f.resumeRecordCount(f.uid, key); n != 0 { + t.Fatalf("rejected request %q left %d idempotency records", key, n) + } + } + if workspaces, items, executions, commands := f.resumeSideEffects(runID); workspaces != 0 || items != 0 || executions != 0 || commands != 0 { + t.Fatalf("rejected bodies wrote side effects: workspaces=%d work=%d executions=%d commands=%d", workspaces, items, executions, commands) + } +} + +// Authorization is cancellation's rule: on a run carrying frozen bindings only the initiator and the +// tenant's administrators reach the 501, and an ordinary member is refused before learning whether +// the operation is available. An unbound run keeps the member rule. +func TestWorkflowRunResumeAuthorizesTheInitiatorOrAnAdmin(t *testing.T) { + f := setup(t) + stranger := joinUser("wf-resume-stranger") + callAs(t, f, stranger, "GET", "/api/v1/me", "", nil, 200) // account, no membership + member, memberID := f.addUser(t, "wf-resume-member", "Member") + + // Bound to the fixture's administrator: the member is neither initiator nor admin. + boundByAdmin, boundByAdminRun := stageWorkflowRun(t, f, "resume-bound-admin", "failed", f.uid) + f.bindWorkflowModelToRun(boundByAdminRun, f.uid) + f.resumeWorkflowRunAs(stranger, f.uid, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-stranger", core.Object{}, 403, "membership_required") + f.resumeWorkflowRunAs(member, memberID, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-member-refused", core.Object{}, 403, "workflow_run_resume_forbidden") + f.resumeWorkflowRunAs(f.user, f.uid, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-admin", core.Object{}, 501, "workflow_run_resume_unavailable") + // The stranger's record is keyed by the stranger's own user id, which this fixture does not read + // back; a key-only count in the tenant is enough to prove nothing was written. + if n := f.scalar(`SELECT count(*) FROM idempotency_records WHERE tenant_id=$1 AND key=$2`, f.tid, "wf-resume-stranger"); n != 0 { + t.Fatalf("a refused caller left %d idempotency records", n) + } + if n := f.resumeRecordCount(memberID, "wf-resume-member-refused"); n != 0 { + t.Fatalf("a refused member left %d idempotency records", n) + } + + // Bound to the member: the initiator reaches the 501. + boundByMember, boundByMemberRun := stageWorkflowRun(t, f, "resume-bound-member", "failed", memberID) + f.bindWorkflowModelToRun(boundByMemberRun, memberID) + f.resumeWorkflowRunAs(member, memberID, f.tid, boundByMember, boundByMemberRun, "wf-resume-initiator", core.Object{}, 501, "workflow_run_resume_unavailable") + + // No binding, no run-level rule: an active member reaches the 501 too. + unbound, unboundRun := stageWorkflowRun(t, f, "resume-unbound", "failed", f.uid) + f.resumeWorkflowRunAs(member, memberID, f.tid, unbound, unboundRun, "wf-resume-unbound", core.Object{}, 501, "workflow_run_resume_unavailable") +} + +// Scope is the workflow routes' answer, unchanged: a run that is not this tenant's, not this +// workflow's, not a live row, or not an id is the same not-found a caller gets for a run that does +// not exist. +func TestWorkflowRunResumeIsNotFoundOutsideItsScope(t *testing.T) { + f := setup(t) + wfid, runID := stageWorkflowRun(t, f, "resume-scope", "failed", f.uid) + otherWorkflow, _ := stageWorkflowRun(t, f, "resume-scope-other", "failed", f.uid) + otherTenant := f.moreTenant() + + for _, tc := range []struct{ name, tenant, workflow, run string }{ + {"cross tenant", otherTenant, wfid, runID}, + {"cross workflow", f.tid, otherWorkflow, runID}, + {"unknown run", f.tid, wfid, newTestID()}, + {"run id is not an id", f.tid, wfid, "not-a-uuid"}, + } { + t.Run(tc.name, func(t *testing.T) { + f.resumeWorkflowRunAs(f.user, f.uid, tc.tenant, tc.workflow, tc.run, "wf-resume-scope-"+tc.name, core.Object{}, 404, "not_found") + }) + } + + t.Run("a soft-deleted run is gone", func(t *testing.T) { + _, gone := stageWorkflowRun(t, f, "resume-scope-deleted", "failed", f.uid) + _, err := f.store.Pool.Exec(`UPDATE workflow_runs SET deleted_at=now() WHERE id=$1`, gone) + must(t, err) + f.resumeWorkflowRun(f.workflowOf(gone), gone, "wf-resume-deleted", core.Object{}, 404, "not_found") + }) +} diff --git a/internal/api/router/router.go b/internal/api/router/router.go index 4a310ec3..78820a1d 100644 --- a/internal/api/router/router.go +++ b/internal/api/router/router.go @@ -150,6 +150,10 @@ func Routes() []Route { {"POST", "/api/v1/tenants/:tid/workflows/:wfid/runs", "", []string{"name", "snapshotId", "input", "projectId"}}, {"GET", "/api/v1/tenants/:tid/workflows/:wfid/runs/:rid", "", nil}, {"POST", "/api/v1/tenants/:tid/workflows/:wfid/runs/:rid/cancel", "", nil}, + // Resume is registered with its final request shape and refuses every valid request with a + // named 501 (retry-and-resume decision D7). Registering it lets clients tell "unavailable" + // from "wrong path" while the implementation lands in a later phase. + {"POST", "/api/v1/tenants/:tid/workflows/:wfid/runs/:rid/resume", "", []string{"mode"}}, {"GET", WorkflowStatisticsPath, "", nil}, {"GET", WorkflowModelCallsPath, "", nil}, {"GET", "/api/v1/tenants/:tid/issue-views", "", nil}, @@ -729,6 +733,11 @@ func validField(name string, value any) bool { case "idle", "initialized", "impactConfirmed", "enabled": _, ok := value.(bool) return ok + case "mode": + // The workflow-run resume request owns this vocabulary: every present value that is not one + // of the three mode strings — including null and a non-string — is the route's own 400 + // invalid_field_value, so the value must reach core instead of becoming invalid_field_type. + return true case "result": o, ok := value.(map[string]any) if !ok { diff --git a/internal/contract/openapi.go b/internal/contract/openapi.go index 0156d3f0..61a0ca40 100644 --- a/internal/contract/openapi.go +++ b/internal/contract/openapi.go @@ -137,7 +137,7 @@ func Document() map[string]any { workflowRunProps["status"] = enumeration("pending", "running", "succeeded", "failed", "cancelled") workflowRunProps["pluginVersion"] = optional(obj{"type": "string", "description": "The agent plugin version frozen into the run at creation: the frozen identity the run's agent sessions execute with. Null only when the space declared no plugin at creation (or the run predates migration 0042); the first agent node then fails with `agent_plugin_version_unfrozen`."}) workflowRunProps["input"] = obj{"type": "object", "additionalProperties": true, "description": "The run's input object: each form value keyed by variable name, plus the reserved `input` key carrying the kickoff text the run submitted. A run that submitted no kickoff evaluates against the Start node's authored instruction instead."} - workflowRunProps["nodeStates"] = obj{"type": "object", "additionalProperties": true, "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed."} + workflowRunProps["nodeStates"] = obj{"type": "object", "additionalProperties": true, "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts."} workflowRunProps["rounds"] = array(obj{"type": "object", "additionalProperties": true}) // definitionSnapshot lives only on a detail read (the history list trips without it); // it is optional so one schema serves both list rows and the composed run page. @@ -459,7 +459,13 @@ func Document() map[string]any { } description := description(r) response, status := responseSchema(r) - responses := obj{status: obj{"description": "Successful command or resource response", "content": obj{"application/json": obj{"schema": response}}}} + responseDescription := "Successful command or resource response" + if status == "501" { + // An operation whose documented answer is its own unavailability states the refusal + // here instead of calling it a success. + responseDescription = errorDescription(status) + } + responses := obj{status: obj{"description": responseDescription, "content": obj{"application/json": obj{"schema": response}}}} if r.Method == "POST" && strings.HasSuffix(r.Path, "/repository/syncs") { responses["200"] = responses[status] } @@ -842,6 +848,13 @@ func responseSchema(r router.Route) (schema obj, status string) { if strings.HasSuffix(r.Path, "/restore") { return ref("Workflow"), "200" } + if strings.HasSuffix(r.Path, "/resume") { + // The run-resume request has no success representation in this phase: every valid + // request is refused with the named 501, so that refusal is the operation's only + // response instead of a success arm no caller can obtain. The arm must come before + // /runs for the same reason the core dispatcher matches it first. + return ref("Error"), "501" + } if strings.HasSuffix(r.Path, "/cancel") { // The workflow-run cancellation request answers the recorded instant, exactly like the // IssueRun request; the branch must come before the /runs arm for the same reason the @@ -1044,6 +1057,9 @@ func optionalField(name string, r router.Route) bool { case "values": // Confirm must state what it is confirming; assist may be asked with a still-empty form. return strings.HasSuffix(r.Path, "/assist") + case "mode": + // Resume defaults to keep: an omitted mode and an explicit keep ask for the same thing. + return strings.Contains(r.Path, "/workflows") && strings.HasSuffix(r.Path, "/resume") } return name == "version" && r.Method == "POST" && strings.HasSuffix(r.Path, "/plugins") || name == "credentialRefId" || name == "role" && strings.HasSuffix(r.Path, "/members/huawei") || name == "version" && r.Method == "PUT" || name == "epoch" && r.Action == "access" || name == "workspaceId" && r.Action == "plan" || name == "externalId" && r.Action == "effect_result" } @@ -1075,6 +1091,11 @@ func inputSchema(name string, r router.Route) obj { return obj{"type": "object", "additionalProperties": obj{"oneOf": []any{str(), obj{"type": "number"}, boolean()}}} case "retrySeconds": return obj{"type": "integer", "minimum": 1, "maximum": 3600} + case "mode": + // The closed set the run-resume contract defines; the endpoint validates it now even + // though every accepted value ends in the same named 501. An absent mode means `keep`; + // a present value outside this set is the route's own invalid_field_value. + return enumeration("keep", "node_files", "checkpoint") case "protocolVersion": return obj{"type": "integer", "enum": []int{1}} case "initialized", "idle", "impactConfirmed", "acknowledgeLostWork": @@ -1284,6 +1305,12 @@ func description(r router.Route) string { // request above because the run kinds differ in what a request means and who may make one. base += "Records the user's cancellation request on a live workflow run and answers 202 with the instant it was recorded, so the run page can show that a cancellation is pending. The body is an empty JSON object and any field is 400 unknown_field. Cancellation is a request and not a transition: this endpoint never changes the run's status or node states and never touches its session or Workspace — the Cloud-side scheduler reacts on its next pass, settling the run cancelled without dispatching another node, stopping an in-flight session through the existing EndSession relay within the same bounded confirmation window an idle session gets, and declaring the Workspace deletion (see the workflow scheduling documentation). Accepted only while the run is pending or running; any other status answers 409 run_not_cancellable. Repeating the request is a no-op that returns the instant the first one recorded, whether or not the run has since settled, so the recorded instant is never replaced. Authorized for the run's initiator and the tenant's administrators on a run carrying frozen executor bindings; an active tenant member who is neither answers 403 workflow_run_cancel_forbidden. A missing or mismatched tenant, workflow or run, and a soft-deleted run are all 404 not_found. " } + if strings.HasSuffix(r.Path, "/runs/:rid/resume") && strings.Contains(r.Path, "/workflows") { + // The run-resume contract is registered before its implementation exists. This paragraph + // states the whole request vocabulary now so a client can integrate against the final + // shape and can tell "not available" apart from "wrong path". + base += "Resumes a terminal failed workflow run inside the same run identity. The route is registered against its final request contract, but no resume mode can run in this phase: every valid request answers 501 workflow_run_resume_unavailable, changes no run, node or Workspace state, and records no idempotency entry, so repeating the request returns the same refusal instead of a replay. mode selects how much Workspace state rolls back — keep (the default when omitted), node_files or checkpoint — and any other value is 400 invalid_field_value; the body carries only mode, and any other field is 400 unknown_field. Authorization is exactly the cancellation request's: the run's initiator and the tenant's administrators on a run carrying frozen executor bindings, and an active tenant member who is neither answers 403 workflow_run_resume_forbidden. A missing or mismatched tenant, workflow or run, and a soft-deleted run are all 404 not_found. " + } if strings.HasSuffix(r.Path, "/thread") { // The Thread GET's own fault vocabulary and cursor semantics, stated here rather than in the // shared per-status descriptions that unrelated routes also carry. @@ -1337,6 +1364,8 @@ func errorDescription(code string) string { return "Version/idempotency conflict, resource_in_use, closed admission, stale epoch/Node/sandbox, incomplete effect, invalid transition, unconfirmed termination/idle, last_admin, space_last_owner, space_slug_conflict, or default_space_protected" case "428": return "Version precondition required" + case "501": + return "The request is valid but the operation is not available in this deployment (e.g. workflow_run_resume_unavailable)" case "503": return "External capability not wired in this deployment (a port is Unavailable), e.g. form_descriptor_unavailable or assist_unavailable" default: diff --git a/internal/core/public.go b/internal/core/public.go index fa5a991d..777fdf36 100644 --- a/internal/core/public.go +++ b/internal/core/public.go @@ -402,6 +402,13 @@ func (s *Store) Public(ctx context.Context, r *PublicRequest) (Object, int, erro require(r.Method == "POST", 404, "not_found") out = cancelWorkflowRunByUser(t, r, uid) status = 202 + case strings.HasSuffix(r.Path, "/resume"): + // D7: resume is registered against its final request shape and refuses every valid + // request with a named 501. Matched before /runs for the same reason as /cancel. The + // handler rejects through a Fault, so this transaction rolls back and the idempotency + // record below is never written. + require(r.Method == "POST", 404, "not_found") + resumeWorkflowRun(t, r, uid) case strings.HasSuffix(r.Path, "/runs"): out = Object{"resource": createWorkflowRun(t, r, uid)} case strings.HasSuffix(r.Path, "/publish"): diff --git a/internal/core/workflow_run_resume.go b/internal/core/workflow_run_resume.go new file mode 100644 index 00000000..e4e7ee9a --- /dev/null +++ b/internal/core/workflow_run_resume.go @@ -0,0 +1,48 @@ +package core + +// WorkflowRun resume: the public request registered by specs decisions/cloud/workflow +// 20261021-node-retry-and-resume-contract.md D7, and explicitly unavailable in the first phase. +// +// The final contract resumes a terminal `failed` run inside the same run identity: the failed node +// and its descendants are cleared, the surviving state is reprojected, and the scheduler dispatches +// from there, with `keep` / `node_files` / `checkpoint` selecting how much Workspace state rolls +// back. None of that can run yet — a failed run's Workspace is already declared deleted, so `keep` +// has nothing to resume onto, and the checkpoint infrastructure does not exist. What this phase +// ships is the endpoint's shape: authorization identical to cancellation, request validation against +// the final contract, and a stable named refusal. The alternative owner rejected — not registering +// the route — leaves clients unable to tell "the server cannot do this" from "the path is wrong". + +// workflowResumeModes is the closed set of rollback modes the final contract defines (D7). It is +// validated now so a client can integrate against the final shape and a request that will never be +// valid is answered as malformed rather than as unavailable. +var workflowResumeModes = map[string]bool{"keep": true, "node_files": true, "checkpoint": true} + +// resumeWorkflowRun is the public request behind +// `POST .../workflows/:wfid/runs/:rid/resume`. It checks the same things cancellation checks, in the +// same order — resource scope, then authorization — and then refuses with the published code. +// +// The refusal is a Fault, so the transaction rolls back: no run, node or Workspace state changes, +// and no idempotency record is written (Public writes one only after the handler returns). A caller +// that repeats the request therefore gets the same answer every time rather than a replay of a +// recorded success. +// +// Authorization runs before the availability check so an unauthorized caller never learns whether +// the operation is available, and resource lookup before both so a run of another tenant or workflow +// is indistinguishable from a missing one. +func resumeWorkflowRun(t *transaction, r *PublicRequest, uid string) Object { + workflow(t, r.TenantID, r.WorkflowID) + require(validID(r.RunID), 404, "not_found") + run := t.one(`SELECT * FROM workflow_runs + WHERE id=$1 AND tenant_id=$2 AND workflow_id=$3 AND deleted_at IS NULL`, r.RunID, r.TenantID, r.WorkflowID) + require(run != nil, 404, "not_found") + require(mayControlWorkflowRun(t, run, uid), 403, "workflow_run_resume_forbidden") + // `mode` defaults to keep only when the field is absent. Every present value must be one of the + // three mode strings: a null, a non-string or an unknown string is the final contract's + // invalid_field_value, because the field was given but names no mode. + if mode, present := r.Body["mode"]; present { + text, ok := mode.(string) + require(ok && workflowResumeModes[text], 400, "invalid_field_value") + } + reject(501, "workflow_run_resume_unavailable") + return nil +} From 07c7d7f5aa60cb8a79516339de7b6222f7accde1 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 10:46:02 +0800 Subject: [PATCH 03/14] docs(migrations): list 0062 workflow node attempt Record the new migration in both migration indexes, including its upgrade path and coverage, so the schema history stays complete after upstream took 0061 for scheduled tasks. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/migrations/README.en.md | 4 +++- internal/core/migrations/README.md | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index aa2fb29b..56eae4b1 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -6,7 +6,7 @@ SQL is embedded in the binary through `embed.FS` in `core/store.go`. `Store.Migr ## File index -There are currently 62 SQL files. +There are currently 65 SQL files. | SQL file | Schema change | |---|---| @@ -73,6 +73,7 @@ There are currently 62 SQL files. | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | Named unconfirmed-stop evidence without changing residual responsibility or reclaim authority | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Plugin model-adapter capability declared by a Node instance at registration | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | Scheduled task definitions, idempotent admission ledger and deadline-cancellation facts | +| [0062_workflow_node_attempt.sql](0062_workflow_node_attempt.sql) | Per-attempt identity for workflow node work items, executions and end requests | | [0062_workflow_run_plugin_release.sql](0062_workflow_run_plugin_release.sql) | Frozen run plugin release package | | [0063_scheduled_run_results.sql](0063_scheduled_run_results.sql) | Scheduled-run business verdict with its frozen task identity and occurrence audit reference | | [0064_scheduled_run_artifacts.sql](0064_scheduled_run_artifacts.sql) | Artifact delivery windows, verified objects, rejections and one unregistered work item per run and kind | @@ -104,6 +105,7 @@ There are currently 62 SQL files. - 0059 only adds `stop_evidence`, initially empty for existing rows; no historical stop observation is invented. Covered by `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`. - 0060 adds `model_adapter` to `node_instances` (a jsonb object, default `{}`) recording the plugin model-adapter capability a Node instance declares at registration. Nothing is backfilled: existing instances count as having no capability and receive no session that needs an adapter until they register again with a newer version. Fresh and upgrade coverage is in `TestMigration0060ModelAdapterFreshAndUpgrade`. - 0061 adds `scheduled_tasks` (frozen prompt, agent ref, `schedule` jsonb, IANA timezone, unattended consent and soft-delete tombstone) and `scheduled_task_occurrences` (an idempotent admission ledger keyed by `(task_id, scheduled_at)`, holding the deadline instant and deadline-cancellation fact); it backfills no historical row. Admission creates an agent IssueRun under the bound issue in the same transaction, and the plugin version is snapshotted from the Space at admission rather than frozen on the definition. Semantics are in the specs root decision `decisions/cloud/scheduled-task`; fresh/upgrade/repeat integration coverage lands with the Web and end-to-end acceptance commit. +- 0062 adds `workflow_node_attempt` to `execution_work`, `node_executions` and `thread_commands` (integer, NOT NULL, default 1) and re-keys the `one_session_per_workflow_node` and `one_workflow_session_end` partial unique indexes on the attempt, so retrying an agent node can register a second session and its own end request without reusing the first attempt's evidence. Every existing row belongs to its node's first attempt, so nothing is backfilled. Fresh, 0061-upgrade, repeated application and constraint coverage is in `TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades`. - 0062 adds a nullable `plugin_release` (jsonb) to `workflow_runs` recording the Agent plugin release package the creation transaction froze; it is written only when the Space's `selected_release` version equals the frozen `plugin_version`, and stays NULL otherwise. The run Workspace's `plugin` step uses it in place of the Space's current selection for that plugin; a plugin the Space already removed is not appended, and the start gate fails the run as `agent_plugin_unavailable`. Semantics are in [workflows](../../../docs/workflows/workflows.en.md#scheduling); coverage is `internal/core/workflow_run_plugin_db_test.go`. - 0063 gives `scheduled_task_occurrences` a surrogate `id` (with a default, so the existing admission transaction needs no change) and adds `scheduled_run_results`: `issue_run_id` is the identity, `verdict` comes from the closed set, `verdict_detail` is at most 2000 characters, `task_id` and `scheduled_at` are the task ownership and planned instant frozen at write time, and `occurrence_id` is an audit reference only, nulled when the occurrence is trimmed. No historical run is backfilled. Semantics are in the specs decision `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`; coverage is in `TestSessionEndedWritesTheScheduledVerdict`, `TestScheduledRunResultIsWrittenOncePerRun`, `TestScheduledRunResultRefusesUnattributableReports` and `TestScheduledRunResultSurvivesOccurrenceTrim`. - 0064 adds the three scheduled-artifact tables: the delivery state with its non-renewable ten-minute window (and its prefix-cleanup marker), the verified object index and the rejection list; all three freeze `task_id`/`scheduled_at` and keep `occurrence_id` as an audit reference only. It also narrows `execution_work`'s unregistered uniqueness from "one per run" to "one per run and kind" and adds `deliver_scheduled_artifacts` to the kind set: a scheduled run holds its Revision delivery and its artifact upload at the same time. Coverage is in `TestScheduledArtifactsSkipWithoutObjectStorage`, `TestScheduledArtifactManifestIsValidatedBeforeStorage`, `TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` and `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`. diff --git a/internal/core/migrations/README.md b/internal/core/migrations/README.md index a350c42c..849e918f 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -6,7 +6,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 ## 文件索引 -当前共 62 个 SQL 文件。 +当前共 65 个 SQL 文件。 | SQL 文件 | schema 变化 | |---|---| @@ -73,6 +73,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | 停止未确认的命名证据,不改变残留责任和回收授权 | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Node 实例登记的插件模型 adapter 能力 | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | 定时任务定义、幂等接纳台账与截止取消事实 | +| [0062_workflow_node_attempt.sql](0062_workflow_node_attempt.sql) | 工作流节点工作项、执行与结束请求的尝试序号 | | [0062_workflow_run_plugin_release.sql](0062_workflow_run_plugin_release.sql) | 冻结运行插件 release 包 | | [0063_scheduled_run_results.sql](0063_scheduled_run_results.sql) | 定时运行的业务 verdict、冻结的任务归属与 occurrence 审计引用 | | [0064_scheduled_run_artifacts.sql](0064_scheduled_run_artifacts.sql) | 定时产物的交付窗口、已核对登记、被拒清单,以及每运行每类一个未登记工作项 | @@ -104,6 +105,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 - 0059 只增 `stop_evidence`,存量为空对象,不追认历史未知停止;覆盖见 `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`。 - 0060 给 `node_instances` 增加 `model_adapter`(jsonb 对象,默认 `{}`),记录 Node 实例登记时声明的插件模型 adapter 能力;不回填,既有实例按空能力处理,不会被派发需要 adapter 的会话,直到以新版本重新登记。空库与升级覆盖见 `TestMigration0060ModelAdapterFreshAndUpgrade`。 - 0061 增加 `scheduled_tasks`(冻结 Prompt、agent ref、`schedule` jsonb、IANA 时区、无人值守同意与软删除墓碑)和 `scheduled_task_occurrences`(`(task_id, scheduled_at)` 唯一的幂等接纳台账,含截止时刻与截止取消事实);不回填历史行。接纳由工作线程在同一事务里创建绑定 issue 下的 agent IssueRun,插件版本在接纳时按 Space 当时值快照而非定义上冻结。语义见 specs 根决策 `decisions/cloud/scheduled-task`;空库、升级与重复应用的集成覆盖随 Web 与端到端验收 commit 一并补齐。 +- 0062 给 `execution_work`、`node_executions` 和 `thread_commands` 增加 `workflow_node_attempt`(整数,NOT NULL,默认 1),并把 `one_session_per_workflow_node`、`one_workflow_session_end` 两个部分唯一索引按尝试重键,使 agent 节点重试时可以登记第二次会话及其自己的结束请求,而不复用第一次尝试的证据。既有行都属于各自节点的第一次尝试,不回填。空库、0061 升级、重复应用与约束覆盖见 `TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades`。 - 0062 给 `workflow_runs` 增加可空 `plugin_release`(jsonb),记录创建事务冻结的 Agent 插件 release 包;仅当 Space 的 `selected_release` 版本等于冻结的 `plugin_version` 时写入,其余为 NULL,不回填。运行 Workspace 的 `plugin` 步骤用它替换 Space 当前选择里的 Agent 插件条目;Space 已移除该插件时不追加,交由启动判定以 `agent_plugin_unavailable` 失败。语义见[工作流](../../../docs/workflows/workflows.md#调度),覆盖见 `internal/core/workflow_run_plugin_db_test.go`。 - 0063 给 `scheduled_task_occurrences` 补一个代理 `id`(默认值,既有接纳事务不用改),并增加 `scheduled_run_results`:以 `issue_run_id` 为唯一身份,`verdict` 取自 Close 集、`verdict_detail` 至多 2000 字符,`task_id` 与 `scheduled_at` 是写入时冻结的任务归属与计划时间事实,`occurrence_id` 只作审计并在裁剪 occurrence 时置空。不回填任何历史运行。语义见 specs `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`;覆盖见 `TestSessionEndedWritesTheScheduledVerdict`、`TestScheduledRunResultIsWrittenOncePerRun`、`TestScheduledRunResultRefusesUnattributableReports` 与 `TestScheduledRunResultSurvivesOccurrenceTrim`。 - 0064 增加定时产物的三张表:交付状态与不可续期 10 分钟窗口(含前缀清理标记)、已核对对象索引、被拒清单;三者都冻结 `task_id`/`scheduled_at` 并把 `occurrence_id` 只作审计。同时把 `execution_work` 的未登记唯一性从"每运行一个"改为"每运行每类一个",并在 kind 闭集里加入 `deliver_scheduled_artifacts`——定时运行要同时持 Revision 交付项和产物上传项。覆盖见 `TestScheduledArtifactsSkipWithoutObjectStorage`、`TestScheduledArtifactManifestIsValidatedBeforeStorage`、`TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` 与 `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`。 From 7e1f64f090c70533a5f49414fc4d4a7ff6032c1a Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 10:48:24 +0800 Subject: [PATCH 04/14] build(contract): regenerate OpenAPI and frontend client for retry and resume The retrying node shape with its attempt and retryAt, the resume request and its 501 response are part of the published contract. The artifacts are produced by task frontend:generate on the current base and contain no hand edits. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- api/openapi.json | 171 +++++++++++++++++++++++- frontend/src/api/generated.schemas.ts | 17 ++- frontend/src/api/workflows/workflows.ts | 72 ++++++++++ 3 files changed, 257 insertions(+), 3 deletions(-) diff --git a/api/openapi.json b/api/openapi.json index 84bedaeb..e65d2c7e 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -5913,7 +5913,7 @@ }, "nodeStates": { "additionalProperties": true, - "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed.", + "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts.", "type": "object" }, "pluginVersion": { @@ -31197,6 +31197,175 @@ ] } }, + "/api/v1/tenants/{tid}/workflows/{wfid}/runs/{rid}/resume": { + "post": { + "description": "Public requests require a gateway service credential plus a caller-bound user credential. Active tenant membership is checked before lookup. Shared projects expose safe runtime summaries; runtime content and use require the verified creator or current tenant administrator. Conflicting mutations additionally require an effective server-confirmed control session. Workflows are tenant-owned graph documents. `graph` is the authored document (nodes, edges, viewport, editor annotations, global variables), stored and returned whole — the editor is its only reader, so no field inside it is validated or indexed here. A live workflow name is unique per tenant; archiving one frees its name. Resumes a terminal failed workflow run inside the same run identity. The route is registered against its final request contract, but no resume mode can run in this phase: every valid request answers 501 workflow_run_resume_unavailable, changes no run, node or Workspace state, and records no idempotency entry, so repeating the request returns the same refusal instead of a replay. mode selects how much Workspace state rolls back — keep (the default when omitted), node_files or checkpoint — and any other value is 400 invalid_field_value; the body carries only mode, and any other field is 400 unknown_field. Authorization is exactly the cancellation request's: the run's initiator and the tenant's administrators on a run carrying frozen executor bindings, and an active tenant member who is neither answers 403 workflow_run_resume_forbidden. A missing or mismatched tenant, workflow or run, and a soft-deleted run are all 404 not_found. Mutation version conflicts return 409; a missing required version returns 428. Unknown fields are rejected. Lists use ascending UUID pagination.", + "operationId": "post_api_v1_tenants_tid_workflows_wfid_runs_rid_resume", + "parameters": [ + { + "in": "path", + "name": "tid", + "required": true, + "schema": { + "format": "uuid", + "type": "string" + } + }, + { + "in": "path", + "name": "wfid", + "required": true, + "schema": { + "format": "uuid", + "type": "string" + } + }, + { + "in": "path", + "name": "rid", + "required": true, + "schema": { + "format": "uuid", + "type": "string" + } + }, + { + "description": "Scoped to tenant and user. Same key and canonical method/path/body returns the original response before version validation; changed request is 409.", + "in": "header", + "name": "Idempotency-Key", + "required": true, + "schema": { + "maxLength": 200, + "minLength": 1, + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "additionalProperties": false, + "properties": { + "mode": { + "enum": [ + "keep", + "node_files", + "checkpoint" + ], + "type": "string" + } + }, + "type": "object" + } + } + }, + "required": true + }, + "responses": { + "400": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Invalid JSON/field/input, missing idempotency key, invalid pagination or evidence" + }, + "401": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Invalid, forged, expired, wrong-audience, untrusted, or caller-mismatched credential" + }, + "403": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Disabled user, inactive/missing membership, wrong service role, or admin required" + }, + "404": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Resource absent or outside authorized tenant/owner scope" + }, + "409": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Version/idempotency conflict, resource_in_use, closed admission, stale epoch/Node/sandbox, incomplete effect, invalid transition, unconfirmed termination/idle, last_admin, space_last_owner, space_slug_conflict, or default_space_protected" + }, + "428": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Version precondition required" + }, + "500": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "Internal error; no SQL or secret details are exposed" + }, + "501": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "The request is valid but the operation is not available in this deployment (e.g. workflow_run_resume_unavailable)" + }, + "503": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + }, + "description": "External capability not wired in this deployment (a port is Unavailable), e.g. form_descriptor_unavailable or assist_unavailable" + } + }, + "security": [ + { + "serviceCredential": [], + "userCredential": [] + } + ], + "summary": "POST /api/v1/tenants/:tid/workflows/:wfid/runs/:rid/resume", + "tags": [ + "workflows" + ] + } + }, "/api/v1/tenants/{tid}/workflows/{wfid}/runs/{rid}/statistics": { "get": { "description": "Read-only active-member model statistics for a WorkflowRun. Covers all recorded model-gateway HTTP attempts owned by this WorkflowRun, independently of request-page pagination. The run must belong to the named workflow and be live; any active tenant member who can read the run can read its statistics. This is not a historical request backfill or billing ledger. Each metric exposes its own known/total coverage; absent values remain null, including a run with no observations. Input/cache semantics follow the source protocol; reasoning is an output subset. No query parameters accepted; private, no-store.", diff --git a/frontend/src/api/generated.schemas.ts b/frontend/src/api/generated.schemas.ts index d81d0f4e..6fc09cec 100644 --- a/frontend/src/api/generated.schemas.ts +++ b/frontend/src/api/generated.schemas.ts @@ -2620,7 +2620,7 @@ export type WorkflowRunDefinitionSnapshot = { [key: string]: unknown } | null; export type WorkflowRunInput = { [key: string]: unknown }; /** - * Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed. + * Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. */ export type WorkflowRunNodeStates = { [key: string]: unknown }; @@ -2666,7 +2666,7 @@ export interface WorkflowRun { */ modelBindings?: WorkflowRunModelBinding[] | null; name: string; - /** Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed. */ + /** Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. */ nodeStates: WorkflowRunNodeStates; /** * The agent plugin version frozen into the run at creation: the frozen identity the run's agent sessions execute with. Null only when the space declared no plugin at creation (or the run predates migration 0042); the first agent node then fails with `agent_plugin_version_unfrozen`. @@ -4469,6 +4469,19 @@ export type GetApiV1TenantsTidWorkflowsWfidRunsRidModelCalls200 = { nextCursor: string; }; +export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBodyMode = typeof PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBodyMode[keyof typeof PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBodyMode]; + + +export const PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBodyMode = { + keep: 'keep', + node_files: 'node_files', + checkpoint: 'checkpoint', +} as const; + +export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBody = { + mode?: PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBodyMode; +}; + export type GetApiV1TenantsTidWorkflowsWfidSnapshotsParams = { /** * @minimum 1 diff --git a/frontend/src/api/workflows/workflows.ts b/frontend/src/api/workflows/workflows.ts index 1a0361a3..0e4a25c4 100644 --- a/frontend/src/api/workflows/workflows.ts +++ b/frontend/src/api/workflows/workflows.ts @@ -44,6 +44,7 @@ import type { PostApiV1TenantsTidWorkflowsWfidRunsBody, PostApiV1TenantsTidWorkflowsWfidRunsRidCancel202, PostApiV1TenantsTidWorkflowsWfidRunsRidCancelBody, + PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBody, PutApiV1TenantsTidWorkflowsWfidBody, PutApiV1TenantsTidWorkflowsWfidSnapshotsSnapshotIdRestoreBody, SessionModelStatistics, @@ -1028,6 +1029,77 @@ export function useGetApiV1TenantsTidWorkflowsWfidRunsRidModelCalls,signal?: AbortSignal +) => { + + + return customInstance( + {url: `/api/v1/tenants/${tid}/workflows/${wfid}/runs/${rid}/resume`, method: 'POST', + headers: {'Content-Type': 'application/json', }, + data: postApiV1TenantsTidWorkflowsWfidRunsRidResumeBody, signal + }, + options); + } + + + + +export const getPostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationKey = () => ['postApiV1TenantsTidWorkflowsWfidRunsRidResume'] as const; + +export const getPostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationOptions = , + TContext = unknown>(options?: { mutation?:UseMutationOptions>, TError,PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables, TContext>, request?: SecondParameter} +): UseMutationOptions>, TError,PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables, TContext> => { + +const mutationKey = getPostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationKey(); +const {mutation: mutationOptions, request: requestOptions} = options ? + options.mutation && 'mutationKey' in options.mutation && options.mutation.mutationKey ? + options + : {...options, mutation: {...options.mutation, mutationKey}} + : {mutation: { mutationKey, }, request: undefined}; + + + + + const mutationFn: MutationFunction>, PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables> = (props) => { + const {tid,wfid,rid,data} = props ?? {}; + + return postApiV1TenantsTidWorkflowsWfidRunsRidResume(tid,wfid,rid,data,requestOptions) + } + + + + + + + return { mutationFn, ...mutationOptions }} + + export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationResult = NonNullable>> + export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationBody = PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBody + export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationError = ErrorType + export type PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables = {tid: string;wfid: string;rid: string;data: PostApiV1TenantsTidWorkflowsWfidRunsRidResumeBody} + + /** + * @summary POST /api/v1/tenants/:tid/workflows/:wfid/runs/:rid/resume + */ +export const usePostApiV1TenantsTidWorkflowsWfidRunsRidResume = , + TContext = unknown>(options?: { mutation?:UseMutationOptions>, TError,PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables, TContext>, request?: SecondParameter} + , queryClient?: QueryClient): UseMutationResult< + Awaited>, + TError, + PostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationVariables, + TContext + > => { + return useMutation(getPostApiV1TenantsTidWorkflowsWfidRunsRidResumeMutationOptions(options), queryClient); + } + /** * Read-only active-member model statistics for a WorkflowRun. Covers all recorded model-gateway HTTP attempts owned by this WorkflowRun, independently of request-page pagination. The run must belong to the named workflow and be live; any active tenant member who can read the run can read its statistics. This is not a historical request backfill or billing ledger. Each metric exposes its own known/total coverage; absent values remain null, including a run with no observations. Input/cache semantics follow the source protocol; reasoning is an output subset. No query parameters accepted; private, no-store. * @summary GET /api/v1/tenants/:tid/workflows/:wfid/runs/:rid/statistics */ From b8c1e4b7a6fbe3c6e95d524f5bd1f8ee9b8db4a3 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 11:00:49 +0800 Subject: [PATCH 05/14] test(workflows): replay a terminal receipt while the node waits to retry A Controller resending an already recorded agent_session result must be a replay, not a second settlement: the retrying state, its retryAt and the run version stay exactly as committed, and the replay releases no work. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/workflow_retry_db_test.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/internal/core/workflow_retry_db_test.go b/internal/core/workflow_retry_db_test.go index 97bdc34b..54f95056 100644 --- a/internal/core/workflow_retry_db_test.go +++ b/internal/core/workflow_retry_db_test.go @@ -148,7 +148,7 @@ func TestWorkflowRetryWaitsThenStartsTheNextAttempt(t *testing.T) { dispatchSessionAttempt(t, store, runID, "a", nodeService, execution1, spec1) reportSessionEnd(t, store, runID, execution1, nodeService, "agent_failed") - status, runErr, _, _ := readWorkflowRun(t, store, runID) + status, runErr, _, version := readWorkflowRun(t, store, runID) state := readWorkflowNodeState(t, store, runID, "a") retryAt := requireRetrying(t, state, 1) if status != "running" || runErr != "" { @@ -172,6 +172,25 @@ func TestWorkflowRetryWaitsThenStartsTheNextAttempt(t *testing.T) { t.Fatalf("%d work items released while the retry waits; none should be", unregistered) } + // Replaying the same terminal receipt — the Controller resending an already recorded result — is + // idempotent: the waiting state, its instant and the run version stay exactly as committed, and + // the replay releases no work. This is the late/duplicate callback half of the acceptance; the + // repeated wake below covers the other half. + reportSessionEnd(t, store, runID, execution1, nodeService, "agent_failed") + if replayed := readWorkflowNodeState(t, store, runID, "a"); jsonText(replayed) != jsonText(state) { + t.Fatalf("state after replaying the receipt = %v; want the unchanged waiting state %v", replayed, state) + } + if replayedStatus, replayedErr, _, replayedVersion := readWorkflowRun(t, store, runID); replayedStatus != status || replayedErr != runErr || replayedVersion != version { + t.Fatalf("run after replaying the receipt = %q error %q version %d; want %q error %q version %d", + replayedStatus, replayedErr, replayedVersion, status, runErr, version) + } + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items released by the replayed receipt; none should be", unregistered) + } + // Before the committed instant the pass is a no-op: no new attempt, no work item. if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { t.Fatalf("advance before due: %v", err) From ac796d0bc42706587dc1840b244b62a9adfcfcdc Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 11:00:49 +0800 Subject: [PATCH 06/14] docs(core): correct the migration number in retry comments The per-attempt identity is migration 0062; upstream took 0061 for scheduled tasks. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/control_projection_db_test.go | 2 +- internal/core/workflow_run_session.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/core/control_projection_db_test.go b/internal/core/control_projection_db_test.go index 0526dfe6..d732eb80 100644 --- a/internal/core/control_projection_db_test.go +++ b/internal/core/control_projection_db_test.go @@ -12,7 +12,7 @@ import ( // workflow_node_id is the graph node a session ran for. It is denormalized onto the row (migration // 0041) so the one_session_per_workflow_node partial index has a predicate to test and so the // scheduler can attribute a result without joining through execution_work. workflow_node_attempt is -// the node's attempt (migration 0061), denormalized the same way for the same reasons. No Controller +// the node's attempt (migration 0062), denormalized the same way for the same reasons. No Controller // contract declares either: the proto ExecutionRecord has none, and the JSON binding's NodeExecution // schema is closed. var nodeExecutionInternalColumns = map[string]bool{ diff --git a/internal/core/workflow_run_session.go b/internal/core/workflow_run_session.go index b78800db..4e5f5adb 100644 --- a/internal/core/workflow_run_session.go +++ b/internal/core/workflow_run_session.go @@ -168,7 +168,7 @@ func workflowAgentSessionOutcome(t *transaction, runID string, version int64, no // path exists to remove. The deadline is measured from the command's own `created_at`, so retries // and replicas all read the same window. func workflowSessionEndRequest(t *transaction, runID string, version int64, nodeID string, attempt int64, executionID string, turnEnded bool, states map[string]workflowNodeState) (map[string]workflowNodeState, bool) { - // The end request is one per (run, node, attempt): 0061 re-keys 0049's uniqueness on the attempt, + // The end request is one per (run, node, attempt): 0062 re-keys 0049's uniqueness on the attempt, // so attempt 1's outstanding request is neither reused nor delivered to attempt 2's session. cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='end_session'", runID, nodeID, attempt) if cmd == nil { From 41f5941ab12c588c83b1611b26421ca6c43f4337 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 11:26:22 +0800 Subject: [PATCH 07/14] test(workflows): settle the plugin-detail interaction with retry Upstream #147 derives the public reason agent_plugin_unavailable from a session's agent_failed plus detail. D2's retry set is keyed on the session's result.reason, and widening that set is an ADR change rather than an implementation detail, so the retry decision still reads agent_failed; the lifted reason is applied only at terminal settlement. The public-row test keeps its subject by seeding the no-retry graph, and a new retry test pins the reason/detail split through budget exhaustion. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/workflow_retry_db_test.go | 67 +++++++++++++++++++ internal/core/workflow_run_session.go | 5 +- internal/core/workflow_run_session_db_test.go | 6 +- 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/internal/core/workflow_retry_db_test.go b/internal/core/workflow_retry_db_test.go index 54f95056..d38dec50 100644 --- a/internal/core/workflow_retry_db_test.go +++ b/internal/core/workflow_retry_db_test.go @@ -377,6 +377,73 @@ func TestWorkflowRetryOnlyRetriesAgentFailedWithinBudget(t *testing.T) { } } +// D2's closed set is keyed on the session's result.reason, not on the public reason derived from +// its detail. The Controller reports a pinned-plugin startup failure as `agent_failed` with +// `detail=agent_plugin_unavailable`; the decision therefore retries it, and the waiting state keeps +// the raw `agent_failed`. Only the terminal state lifts the detail (the mapping issue-runs share), +// so an exhausted budget still reports the precise public cause. Widening the set to exclude the +// detail is an ADR change, not an implementation detail. +func TestWorkflowRetryDecidesOnTheSessionReasonNotItsDetail(t *testing.T) { + store := dispatcherDB(t) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 1, 0)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec := startSessionNode(t, store, runID, wsID, "a") + reportPluginFailure := func(execution string) { + t.Helper() + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + executionResult(tx, &ControlRequest{Body: Object{ + "operationId": runID, "executionId": execution, "sequence": int64(1), "event": "ended", + "result": Object{ + "outcome": "agent_session_ended", "reason": "agent_failed", + "detail": "agent_plugin_unavailable", "node": Object{"nodeId": nodeService}, + }, + }}, true) + return Object{} + }); err != nil { + t.Fatalf("report session end: %v", err) + } + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after result: %v", err) + } + } + execution1 := "exec-plugin-detail-1-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution1, spec) + reportPluginFailure(execution1) + requireRetrying(t, readWorkflowNodeState(t, store, runID, "a"), 1) + + // The single retry runs and fails the same way; the budget is then spent, and the terminal row + // carries the public reason lifted from the detail. + makeWorkflowRetryDue(t, store, runID, "a") + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance at due: %v", err) + } + spec2 := sessionWorkSpec(t, store, runID, "a") + execution2 := "exec-plugin-detail-2-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution2, spec2) + reportPluginFailure(execution2) + + status, runErr, states, _ := readWorkflowRun(t, store, runID) + if status != "failed" || runErr != "agent_plugin_unavailable" { + t.Fatalf("run settled %q error %q states=%v; want failed/agent_plugin_unavailable", status, runErr, states) + } + agent, _ := asWorkflowObject(states["a"]) + if agent == nil || agent.S("status") != "failed" || agent.N("attempt") != 2 || agent.S("retryAt") != "" || + agent.S("error") != "agent_plugin_unavailable" || agent.O("output").S("reason") != "agent_plugin_unavailable" { + t.Fatalf("agent state = %v; want failed attempt 2 with the lifted reason and no retryAt", states["a"]) + } + var unregistered int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE workflow_run_id=$1 AND execution_id IS NULL`, runID).Scan(&unregistered); err != nil { + t.Fatal(err) + } + if unregistered != 0 { + t.Fatalf("%d work items released after the budget was spent", unregistered) + } + assertRunWorkspaceReleased(t, store, runID) +} + // Cancellation wins over a waiting retry (D5): the node settles cancelled without waiting for the // committed due instant, the run settles cancelled, the Workspace is released — and even a due // instant that has already passed starts no attempt. The cancelled node keeps the attempt it failed diff --git a/internal/core/workflow_run_session.go b/internal/core/workflow_run_session.go index 4e5f5adb..023e922b 100644 --- a/internal/core/workflow_run_session.go +++ b/internal/core/workflow_run_session.go @@ -254,7 +254,10 @@ func workflowSessionEndReason(reason string) string { // The retry decision is made from result.reason before the reason is mapped (D2). The mapping sends // several unrelated reasons to `failed` — interrupted, Cloud's own deadlines, node_unavailable — // and treats every unknown reason as success; deriving the retry from the mapped status would both -// retry what the ADR forbids and skip the decision for reasons that map to success. +// retry what the ADR forbids and skip the decision for reasons that map to success. The decision +// reads the session's reason and not its detail: a pinned-plugin failure is agent_failed with +// detail=agent_plugin_unavailable, so D2's closed set keeps it retryable; the lifted public reason +// is applied only when the node settles. func commitWorkflowAgentTerminal(t *transaction, runID string, version int64, node workflowNodeView, states map[string]workflowNodeState, result Object) (map[string]workflowNodeState, bool) { nodeID := node.ID state := states[nodeID] diff --git a/internal/core/workflow_run_session_db_test.go b/internal/core/workflow_run_session_db_test.go index afe199c7..dd67c213 100644 --- a/internal/core/workflow_run_session_db_test.go +++ b/internal/core/workflow_run_session_db_test.go @@ -750,6 +750,10 @@ func TestWorkflowSessionRefusesUnusablePinnedPlugin(t *testing.T) { // `detail=agent_plugin_unavailable`. A WorkflowRun has no Thread, so the node and run rows are the // only public place the cause can appear; `agent_plugin_unavailable` is lifted out of the detail and // every other detail stays out (the same bounded set IssueRun threads use). +// +// Retry is disabled on this graph: the test is about which detail becomes public, so it asserts the +// terminal rows directly. The retry-enabled settlement — the decision reads the raw `agent_failed`, +// and the exhausted terminal row still lifts the detail — is asserted in workflow_retry_db_test.go. func TestWorkflowSessionPluginFailureReachesThePublicRow(t *testing.T) { for _, tc := range []struct { name, detail, want string @@ -759,7 +763,7 @@ func TestWorkflowSessionPluginFailureReachesThePublicRow(t *testing.T) { } { t.Run(tc.name, func(t *testing.T) { store := dispatcherDB(t) - runID, _, _, _, _ := seedWorkflowSessionScene(t, store) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowSessionNoRetryGraph()) if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { t.Fatalf("advance to workspace create: %v", err) } From 7459ba64fff7bc1689e9fb4fa573f895240057e4 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 12:01:18 +0800 Subject: [PATCH 08/14] test(workflows): prove a due retry wakes one attempt across concurrent passes The wake runs under the run's exclusive transaction, so the pass that loses reads the winning attempt as running and releases nothing. Two replicas advance the same due retry under a synchronized start, and the test asserts the complete permitted outcome: attempt 2 running, exactly one released session work item, and no retryAt left behind. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/workflow_retry_db_test.go | 66 +++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/internal/core/workflow_retry_db_test.go b/internal/core/workflow_retry_db_test.go index d38dec50..c2aa24dd 100644 --- a/internal/core/workflow_retry_db_test.go +++ b/internal/core/workflow_retry_db_test.go @@ -2,6 +2,7 @@ package core import ( "context" + "sync" "testing" "time" ) @@ -270,6 +271,71 @@ func TestWorkflowRetryWaitsThenStartsTheNextAttempt(t *testing.T) { assertRunWorkspaceReleased(t, store, runID) } +// The wake is serialized by the run's exclusive transaction (D3): two replicas advancing the same +// due retry at the same time release exactly one next attempt. The pass that loses reads the +// winning attempt as running and releases nothing, so the run cannot end up with two session work +// items for the same attempt or skip an attempt number. +func TestWorkflowRetryDueStartsOneAttemptUnderConcurrentPasses(t *testing.T) { + store := dispatcherDB(t) + replica := secondReplica(t, store) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 2, 0)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec := startSessionNode(t, store, runID, wsID, "a") + execution := "exec-concurrent-due-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution, spec) + reportSessionEnd(t, store, runID, execution, nodeService, "agent_failed") + requireRetrying(t, readWorkflowNodeState(t, store, runID, "a"), 1) + makeWorkflowRetryDue(t, store, runID, "a") + + start := make(chan struct{}) + errs := make(chan error, 2) + var wg sync.WaitGroup + for _, s := range []*Store{store, replica} { + wg.Add(1) + go func(s *Store) { + defer wg.Done() + <-start + errs <- s.AdvanceWorkflowRunsOnce(context.Background()) + }(s) + } + close(start) + wg.Wait() + close(errs) + for err := range errs { + if err != nil { + t.Fatalf("concurrent pass: %v", err) + } + } + + state := readWorkflowNodeState(t, store, runID, "a") + if state.S("status") != "running" || state.N("attempt") != 2 || state.S("retryAt") != "" { + t.Fatalf("state after concurrent passes = %v; want running attempt 2 with no retryAt", state) + } + rows, err := store.Pool.Query(`SELECT workflow_node_attempt FROM execution_work + WHERE workflow_run_id=$1 AND workflow_node_id='a' AND execution_id IS NULL ORDER BY workflow_node_attempt`, runID) + if err != nil { + t.Fatal(err) + } + defer rows.Close() + var released []int64 + for rows.Next() { + var attempt int64 + if err := rows.Scan(&attempt); err != nil { + t.Fatal(err) + } + released = append(released, attempt) + } + if err := rows.Err(); err != nil { + t.Fatal(err) + } + if len(released) != 1 || released[0] != 2 { + t.Fatalf("released session work attempts = %v; want exactly [2]", released) + } +} + // The budget is `maxRetries + 1` attempts (D1/D6). After the last allowed attempt fails, the node // keeps that attempt's reason and finishes failed, and the run fails and releases its Workspace — // there is no third attempt and no unregistered work left behind. From 8bb45cc5e637667818fc54c9cea0c852843b9d57 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 12:34:15 +0800 Subject: [PATCH 09/14] test(workflows): scope end requests and late results to the attempt Attempt identity covers the whole session handshake, not just the session row: while attempt 2 runs, attempt 1's undelivered end request is neither claimable by the controller nor accepted as delivered to attempt 2, and delivering it there is refused with dispatch_conflict. Attempt 2's own turn end releases a fresh request, and replaying attempt 1's terminal result is a receipt replay that leaves the node and run on attempt 2. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- internal/core/workflow_retry_db_test.go | 94 +++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/internal/core/workflow_retry_db_test.go b/internal/core/workflow_retry_db_test.go index c2aa24dd..ef038958 100644 --- a/internal/core/workflow_retry_db_test.go +++ b/internal/core/workflow_retry_db_test.go @@ -271,6 +271,100 @@ func TestWorkflowRetryWaitsThenStartsTheNextAttempt(t *testing.T) { assertRunWorkspaceReleased(t, store, runID) } +// Attempt identity scopes the session's end request and its terminal evidence, not only the session +// itself (D4). Once the node has retried, attempt 1's outstanding end request is neither claimable +// for attempt 2's session nor accepted as delivered to it, attempt 2 gets its own request, and +// replaying attempt 1's terminal result cannot settle the in-flight attempt 2. +func TestWorkflowRetryKeepsAttemptOneRequestAndResultOutOfAttemptTwo(t *testing.T) { + store := dispatcherDB(t) + runID, _, _, _, _ := seedWorkflowSessionSceneGraph(t, store, workflowRetryGraph(true, 1, 0)) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance 1: %v", err) + } + wsID := settleSessionWorkspace(t, store, runID) + nodeService, spec1 := startSessionNode(t, store, runID, wsID, "a") + execution1 := "exec-attempt-scope-1-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution1, spec1) + + // The initial turn ends, so Cloud releases attempt 1's end request; the session then fails + // before the request is ever delivered (the Controller may not have claimed it), which leaves + // the row undelivered while the node waits for its retry. + takeOverWorkflowEvents(t, store, runID, execution1, []Object{threadEvent(1, turnEndedRecord("end_turn"), "")}) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after turn end: %v", err) + } + first := claimCommands(t, store) + if len(first) != 1 || first[0].S("executionId") != execution1 || first[0].S("kind") != "end_session" { + t.Fatalf("attempt 1 commands = %v; want one end_session addressed to %s", first, execution1) + } + reportSessionEndAt(t, store, runID, execution1, nodeService, "agent_failed", 2) + requireRetrying(t, readWorkflowNodeState(t, store, runID, "a"), 1) + + // The retry starts attempt 2 and registers its own session, which stays in flight. + makeWorkflowRetryDue(t, store, runID, "a") + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance at due: %v", err) + } + spec2 := sessionWorkSpec(t, store, runID, "a") + execution2 := "exec-attempt-scope-2-" + runID[:8] + dispatchSessionAttempt(t, store, runID, "a", nodeService, execution2, spec2) + + // Attempt 1's request names attempt 1: it is not claimable against attempt 2's live session + // (whose own turn has not ended), and naming that execution on delivery is refused. + if got := claimCommands(t, store); len(got) != 0 { + t.Fatalf("attempt 2 claimed attempt 1's request: %v", got) + } + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + recordThreadCommandDelivered(tx, &ControlRequest{Body: Object{"commandId": first[0].S("commandId"), "executionId": execution2}}) + return Object{} + }); err == nil || ErrorCode(err).Code != "dispatch_conflict" { + t.Fatalf("cross-attempt delivery: err=%v; want dispatch_conflict", err) + } + var attempt int64 + var node, deliveredExecution string + if err := store.Pool.QueryRow(`SELECT workflow_node_attempt, workflow_node_id, COALESCE(delivered_execution_id,'') + FROM thread_commands WHERE id=$1`, first[0].S("commandId")).Scan(&attempt, &node, &deliveredExecution); err != nil { + t.Fatalf("read attempt 1's request: %v", err) + } + if attempt != 1 || node != "a" || deliveredExecution != "" { + t.Fatalf("attempt 1's request names node %q attempt %d, delivered to %q; want a/1 delivered to nothing", node, attempt, deliveredExecution) + } + + // Replaying attempt 1's terminal result is a receipt replay, not evidence for attempt 2: its + // lookup names the current attempt, so the committed node stays the running attempt 2. + reportSessionEndAt(t, store, runID, execution1, nodeService, "agent_failed", 2) + if state := readWorkflowNodeState(t, store, runID, "a"); state.S("status") != "running" || state.N("attempt") != 2 || state.S("retryAt") != "" { + t.Fatalf("state after replaying attempt 1's result = %v; want running attempt 2", state) + } + if status, runErr, _, _ := readWorkflowRun(t, store, runID); status != "running" || runErr != "" { + t.Fatalf("run after replaying attempt 1's result = %q error %q; want running", status, runErr) + } + + // Attempt 2's own turn end releases a fresh request addressed to attempt 2 — attempt 1's row + // neither suppresses it nor gets reused for it — and delivering that request is accepted. + takeOverWorkflowEvents(t, store, runID, execution2, []Object{threadEvent(1, turnEndedRecord("end_turn"), "")}) + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatalf("advance after attempt 2 turn end: %v", err) + } + second := claimCommands(t, store) + if len(second) != 1 || second[0].S("executionId") != execution2 || second[0].S("commandId") == first[0].S("commandId") { + t.Fatalf("attempt 2 commands = %v; want a new request addressed to %s", second, execution2) + } + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + recordThreadCommandDelivered(tx, &ControlRequest{Body: Object{"commandId": second[0].S("commandId"), "executionId": execution2}}) + return Object{} + }); err != nil { + t.Fatalf("deliver attempt 2's request: %v", err) + } + if got := claimCommands(t, store); len(got) != 0 { + t.Fatalf("commands after delivery: %v; want none", got) + } + reportSessionEndAt(t, store, runID, execution2, nodeService, "idle_timeout", 2) + if status, runErr, _, _ := readWorkflowRun(t, store, runID); status != "succeeded" || runErr != "" { + t.Fatalf("run settled %q error %q; want succeeded from attempt 2", status, runErr) + } +} + // The wake is serialized by the run's exclusive transaction (D3): two replicas advancing the same // due retry at the same time release exactly one next attempt. The pass that loses reads the // winning attempt as running and releases nothing, so the run cannot end up with two session work From 5346089ded3b00acb6fcd132b1c55a75c48a886f Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 15:57:26 +0800 Subject: [PATCH 10/14] docs(contract): describe a retrying node's start and finish accurately A node that retries twice has a retrying state whose startedAt is the failed attempt's start, not the first attempt's, because every attempt begins a fresh start instant and the failed attempt's value is kept. finishedAt is always emitted; a retrying state carries an empty string, not an absent key. Regenerate the OpenAPI document and the frontend client from the corrected description. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- api/openapi.json | 2 +- frontend/src/api/generated.schemas.ts | 4 ++-- internal/contract/openapi.go | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/api/openapi.json b/api/openapi.json index e65d2c7e..72488736 100644 --- a/api/openapi.json +++ b/api/openapi.json @@ -5913,7 +5913,7 @@ }, "nodeStates": { "additionalProperties": true, - "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts.", + "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is the failed attempt's start, `finishedAt` is empty, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts.", "type": "object" }, "pluginVersion": { diff --git a/frontend/src/api/generated.schemas.ts b/frontend/src/api/generated.schemas.ts index 6fc09cec..69a9679f 100644 --- a/frontend/src/api/generated.schemas.ts +++ b/frontend/src/api/generated.schemas.ts @@ -2620,7 +2620,7 @@ export type WorkflowRunDefinitionSnapshot = { [key: string]: unknown } | null; export type WorkflowRunInput = { [key: string]: unknown }; /** - * Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. + * Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is the failed attempt's start, `finishedAt` is empty, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. */ export type WorkflowRunNodeStates = { [key: string]: unknown }; @@ -2666,7 +2666,7 @@ export interface WorkflowRun { */ modelBindings?: WorkflowRunModelBinding[] | null; name: string; - /** Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. */ + /** Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is the failed attempt's start, `finishedAt` is empty, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts. */ nodeStates: WorkflowRunNodeStates; /** * The agent plugin version frozen into the run at creation: the frozen identity the run's agent sessions execute with. Null only when the space declared no plugin at creation (or the run predates migration 0042); the first agent node then fails with `agent_plugin_version_unfrozen`. diff --git a/internal/contract/openapi.go b/internal/contract/openapi.go index 61a0ca40..3865530d 100644 --- a/internal/contract/openapi.go +++ b/internal/contract/openapi.go @@ -137,7 +137,7 @@ func Document() map[string]any { workflowRunProps["status"] = enumeration("pending", "running", "succeeded", "failed", "cancelled") workflowRunProps["pluginVersion"] = optional(obj{"type": "string", "description": "The agent plugin version frozen into the run at creation: the frozen identity the run's agent sessions execute with. Null only when the space declared no plugin at creation (or the run predates migration 0042); the first agent node then fails with `agent_plugin_version_unfrozen`."}) workflowRunProps["input"] = obj{"type": "object", "additionalProperties": true, "description": "The run's input object: each form value keyed by variable name, plus the reserved `input` key carrying the kickoff text the run submitted. A run that submitted no kickoff evaluates against the Start node's authored instruction instead."} - workflowRunProps["nodeStates"] = obj{"type": "object", "additionalProperties": true, "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is kept from the first attempt, `finishedAt` is absent, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts."} + workflowRunProps["nodeStates"] = obj{"type": "object", "additionalProperties": true, "description": "Per-node execution state keyed by node id: status, displayName, startedAt, finishedAt, output, and error — the reason, present only when the node failed or is retrying. An agent node can be `retrying`, which means its last attempt failed with a retryable reason and the run is waiting for the next one: `retryAt` is the database-clock instant that attempt becomes due, `error` and `output.reason` name the failed attempt's reason, `startedAt` is the failed attempt's start, `finishedAt` is empty, and the run itself stays `running`. `attempt` is the 1-based attempt the state describes, present whenever the node has started at least once, so a terminal node states which attempt ended it; a node's retry budget is `maxRetries + 1` attempts."} workflowRunProps["rounds"] = array(obj{"type": "object", "additionalProperties": true}) // definitionSnapshot lives only on a detail read (the history list trips without it); // it is optional so one schema serves both list rows and the composed run page. From e65c87fe5b3fe6cd766ec426f5c77f17d2fcee51 Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 15:57:27 +0800 Subject: [PATCH 11/14] test(integration): drop the unused caller id from the resume helper resumeWorkflowRunAs takes the request identity from the verified claims, so its uid parameter only repeated a value the helper never read. Refs ora-space/circus#469 Co-Authored-By: Claude Code --- integration/workflow_run_resume_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/integration/workflow_run_resume_test.go b/integration/workflow_run_resume_test.go index 0c4e13fb..2f2fe253 100644 --- a/integration/workflow_run_resume_test.go +++ b/integration/workflow_run_resume_test.go @@ -34,12 +34,12 @@ func workflowRunResumePath(tenant, workflow, run string) string { // resumeWorkflowRun issues the production resume request as the fixture's verified user. func (f *fixture) resumeWorkflowRun(wfid, runID, key string, body core.Object, want int, code string) core.Object { f.t.Helper() - return f.resumeWorkflowRunAs(f.user, f.uid, f.tid, wfid, runID, key, body, want, code) + return f.resumeWorkflowRunAs(f.user, f.tid, wfid, runID, key, body, want, code) } // resumeWorkflowRunAs issues the request as a specific caller and asserts the exact status and stable // public code. -func (f *fixture) resumeWorkflowRunAs(user core.Claims, uid, tenant, wfid, runID, key string, body core.Object, want int, code string) core.Object { +func (f *fixture) resumeWorkflowRunAs(user core.Claims, tenant, wfid, runID, key string, body core.Object, want int, code string) core.Object { f.t.Helper() out, status, err := f.client.Call(context.Background(), "POST", workflowRunResumePath(tenant, wfid, runID), "gateway", @@ -150,9 +150,9 @@ func TestWorkflowRunResumeAuthorizesTheInitiatorOrAnAdmin(t *testing.T) { // Bound to the fixture's administrator: the member is neither initiator nor admin. boundByAdmin, boundByAdminRun := stageWorkflowRun(t, f, "resume-bound-admin", "failed", f.uid) f.bindWorkflowModelToRun(boundByAdminRun, f.uid) - f.resumeWorkflowRunAs(stranger, f.uid, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-stranger", core.Object{}, 403, "membership_required") - f.resumeWorkflowRunAs(member, memberID, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-member-refused", core.Object{}, 403, "workflow_run_resume_forbidden") - f.resumeWorkflowRunAs(f.user, f.uid, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-admin", core.Object{}, 501, "workflow_run_resume_unavailable") + f.resumeWorkflowRunAs(stranger, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-stranger", core.Object{}, 403, "membership_required") + f.resumeWorkflowRunAs(member, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-member-refused", core.Object{}, 403, "workflow_run_resume_forbidden") + f.resumeWorkflowRunAs(f.user, f.tid, boundByAdmin, boundByAdminRun, "wf-resume-admin", core.Object{}, 501, "workflow_run_resume_unavailable") // The stranger's record is keyed by the stranger's own user id, which this fixture does not read // back; a key-only count in the tenant is enough to prove nothing was written. if n := f.scalar(`SELECT count(*) FROM idempotency_records WHERE tenant_id=$1 AND key=$2`, f.tid, "wf-resume-stranger"); n != 0 { @@ -165,11 +165,11 @@ func TestWorkflowRunResumeAuthorizesTheInitiatorOrAnAdmin(t *testing.T) { // Bound to the member: the initiator reaches the 501. boundByMember, boundByMemberRun := stageWorkflowRun(t, f, "resume-bound-member", "failed", memberID) f.bindWorkflowModelToRun(boundByMemberRun, memberID) - f.resumeWorkflowRunAs(member, memberID, f.tid, boundByMember, boundByMemberRun, "wf-resume-initiator", core.Object{}, 501, "workflow_run_resume_unavailable") + f.resumeWorkflowRunAs(member, f.tid, boundByMember, boundByMemberRun, "wf-resume-initiator", core.Object{}, 501, "workflow_run_resume_unavailable") // No binding, no run-level rule: an active member reaches the 501 too. unbound, unboundRun := stageWorkflowRun(t, f, "resume-unbound", "failed", f.uid) - f.resumeWorkflowRunAs(member, memberID, f.tid, unbound, unboundRun, "wf-resume-unbound", core.Object{}, 501, "workflow_run_resume_unavailable") + f.resumeWorkflowRunAs(member, f.tid, unbound, unboundRun, "wf-resume-unbound", core.Object{}, 501, "workflow_run_resume_unavailable") } // Scope is the workflow routes' answer, unchanged: a run that is not this tenant's, not this @@ -188,7 +188,7 @@ func TestWorkflowRunResumeIsNotFoundOutsideItsScope(t *testing.T) { {"run id is not an id", f.tid, wfid, "not-a-uuid"}, } { t.Run(tc.name, func(t *testing.T) { - f.resumeWorkflowRunAs(f.user, f.uid, tc.tenant, tc.workflow, tc.run, "wf-resume-scope-"+tc.name, core.Object{}, 404, "not_found") + f.resumeWorkflowRunAs(f.user, tc.tenant, tc.workflow, tc.run, "wf-resume-scope-"+tc.name, core.Object{}, 404, "not_found") }) } From d5c4087a157a227e4f9a1aa34016bdac8de99fae Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 16:33:57 +0800 Subject: [PATCH 12/14] style(workflows): drop a stray blank line in the retry status test Co-Authored-By: Claude Code --- frontend/src/features/workflows/run/run-status-mark.test.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/src/features/workflows/run/run-status-mark.test.tsx b/frontend/src/features/workflows/run/run-status-mark.test.tsx index a41ade1b..530484b6 100644 --- a/frontend/src/features/workflows/run/run-status-mark.test.tsx +++ b/frontend/src/features/workflows/run/run-status-mark.test.tsx @@ -49,7 +49,6 @@ describe('RunStatusMark', () => { unmount() }) - it('keeps a plain dot for a live-worthy-looking but non-working status', () => { const { unmount } = renderWithProviders() expect(svgCount()).toBe(0) From a17ff65b55a9e357e8635d7f6bf6a43984876c8e Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 16:39:34 +0800 Subject: [PATCH 13/14] fix(core): renumber the node attempt migration to 0069 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit main gained 0062–0068 while this branch was in review; the node attempt migration takes the next free number and its test, comments and migration README follow it. Co-Authored-By: Claude Code --- integration/migration_upgrade_path_test.go | 14 +++++++------- internal/core/control_projection_db_test.go | 2 +- ..._attempt.sql => 0069_workflow_node_attempt.sql} | 0 internal/core/migrations/README.en.md | 6 +++--- internal/core/migrations/README.md | 6 +++--- internal/core/workflow_run_session.go | 2 +- 6 files changed, 15 insertions(+), 15 deletions(-) rename internal/core/migrations/{0062_workflow_node_attempt.sql => 0069_workflow_node_attempt.sql} (100%) diff --git a/integration/migration_upgrade_path_test.go b/integration/migration_upgrade_path_test.go index 1320edb5..3ac91087 100644 --- a/integration/migration_upgrade_path_test.go +++ b/integration/migration_upgrade_path_test.go @@ -1736,13 +1736,13 @@ func TestMigration0055WorkflowRunWorkspaceReleaseAppliesFreshAndUpgrades(t *test refuse(`UPDATE workflow_runs SET workspace_release_state='settled' WHERE id=$1`, ids["running"]) } -// 0062 gives workflow node sessions and end requests a per-attempt identity (retry decision D4). The +// 0069 gives workflow node sessions and end requests a per-attempt identity (retry decision D4). The // columns are NOT NULL with default 1 because every row that exists today belongs to its node's first // attempt; nothing is backfilled. The uniqueness of 0041/0049 is re-keyed on the attempt, so a second // attempt of the same node can register while the same attempt still cannot register twice. A fresh -// database and one that stopped at 0061 must reach the same shape, and rerunning the migration must +// database and one that stopped at 0068 must reach the same shape, and rerunning the migration must // change nothing. -func TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades(t *testing.T) { +func TestMigration0069WorkflowNodeAttemptAppliesFreshAndUpgrades(t *testing.T) { tables := []string{"execution_work", "node_executions", "thread_commands"} columnShape := func(t *testing.T, pool *sql.DB, table string) (int, string) { t.Helper() @@ -1773,15 +1773,15 @@ func TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades(t *testing.T) { } } - freshPool, _ := testSchema(t, "test_wfna62f_") + freshPool, _ := testSchema(t, "test_wfna69f_") migratedTwice(t, newStoreOnSchema(t, freshPool)) assertAttemptColumns(t, freshPool, "fresh") - pool, _ := testSchema(t, "test_wfna62_") - applyMigrationsUpTo(t, pool, migrationsBefore(t, "0062_workflow_node_attempt.sql")) + pool, _ := testSchema(t, "test_wfna69_") + applyMigrationsUpTo(t, pool, migrationsBefore(t, "0069_workflow_node_attempt.sql")) for _, table := range tables { if n, _ := columnShape(t, pool, table); n != 0 { - t.Fatalf("before 0062: %s.workflow_node_attempt already exists", table) + t.Fatalf("before 0069: %s.workflow_node_attempt already exists", table) } } // One session from before the migration. Its result is committed because 0024's pending-execution diff --git a/internal/core/control_projection_db_test.go b/internal/core/control_projection_db_test.go index d732eb80..2d2226b0 100644 --- a/internal/core/control_projection_db_test.go +++ b/internal/core/control_projection_db_test.go @@ -12,7 +12,7 @@ import ( // workflow_node_id is the graph node a session ran for. It is denormalized onto the row (migration // 0041) so the one_session_per_workflow_node partial index has a predicate to test and so the // scheduler can attribute a result without joining through execution_work. workflow_node_attempt is -// the node's attempt (migration 0062), denormalized the same way for the same reasons. No Controller +// the node's attempt (migration 0069), denormalized the same way for the same reasons. No Controller // contract declares either: the proto ExecutionRecord has none, and the JSON binding's NodeExecution // schema is closed. var nodeExecutionInternalColumns = map[string]bool{ diff --git a/internal/core/migrations/0062_workflow_node_attempt.sql b/internal/core/migrations/0069_workflow_node_attempt.sql similarity index 100% rename from internal/core/migrations/0062_workflow_node_attempt.sql rename to internal/core/migrations/0069_workflow_node_attempt.sql diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index 56eae4b1..8ba6fda4 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -6,7 +6,7 @@ SQL is embedded in the binary through `embed.FS` in `core/store.go`. `Store.Migr ## File index -There are currently 65 SQL files. +There are currently 71 SQL files. | SQL file | Schema change | |---|---| @@ -73,7 +73,6 @@ There are currently 65 SQL files. | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | Named unconfirmed-stop evidence without changing residual responsibility or reclaim authority | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Plugin model-adapter capability declared by a Node instance at registration | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | Scheduled task definitions, idempotent admission ledger and deadline-cancellation facts | -| [0062_workflow_node_attempt.sql](0062_workflow_node_attempt.sql) | Per-attempt identity for workflow node work items, executions and end requests | | [0062_workflow_run_plugin_release.sql](0062_workflow_run_plugin_release.sql) | Frozen run plugin release package | | [0063_scheduled_run_results.sql](0063_scheduled_run_results.sql) | Scheduled-run business verdict with its frozen task identity and occurrence audit reference | | [0064_scheduled_run_artifacts.sql](0064_scheduled_run_artifacts.sql) | Artifact delivery windows, verified objects, rejections and one unregistered work item per run and kind | @@ -81,6 +80,7 @@ There are currently 65 SQL files. | [0066_scheduled_artifact_recurring_gc.sql](0066_scheduled_artifact_recurring_gc.sql) | Recurring sweeps of settled runs for late PUTs and pruned successful deliveries | | [0067_scheduled_empty_artifacts.sql](0067_scheduled_empty_artifacts.sql) | Accept zero-length artifacts in verified registration | | [0068_scheduled_manifest_completeness.sql](0068_scheduled_manifest_completeness.sql) | Persist omitted rejection counts and walk truncation facts | +| [0069_workflow_node_attempt.sql](0069_workflow_node_attempt.sql) | Per-attempt identity for workflow node work items, executions and end requests | 0050 copies no personal connection or key and backfills no historical run. Clearing configuration retains its revision; run options are stored separately from shared run input. @@ -105,10 +105,10 @@ There are currently 65 SQL files. - 0059 only adds `stop_evidence`, initially empty for existing rows; no historical stop observation is invented. Covered by `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`. - 0060 adds `model_adapter` to `node_instances` (a jsonb object, default `{}`) recording the plugin model-adapter capability a Node instance declares at registration. Nothing is backfilled: existing instances count as having no capability and receive no session that needs an adapter until they register again with a newer version. Fresh and upgrade coverage is in `TestMigration0060ModelAdapterFreshAndUpgrade`. - 0061 adds `scheduled_tasks` (frozen prompt, agent ref, `schedule` jsonb, IANA timezone, unattended consent and soft-delete tombstone) and `scheduled_task_occurrences` (an idempotent admission ledger keyed by `(task_id, scheduled_at)`, holding the deadline instant and deadline-cancellation fact); it backfills no historical row. Admission creates an agent IssueRun under the bound issue in the same transaction, and the plugin version is snapshotted from the Space at admission rather than frozen on the definition. Semantics are in the specs root decision `decisions/cloud/scheduled-task`; fresh/upgrade/repeat integration coverage lands with the Web and end-to-end acceptance commit. -- 0062 adds `workflow_node_attempt` to `execution_work`, `node_executions` and `thread_commands` (integer, NOT NULL, default 1) and re-keys the `one_session_per_workflow_node` and `one_workflow_session_end` partial unique indexes on the attempt, so retrying an agent node can register a second session and its own end request without reusing the first attempt's evidence. Every existing row belongs to its node's first attempt, so nothing is backfilled. Fresh, 0061-upgrade, repeated application and constraint coverage is in `TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades`. - 0062 adds a nullable `plugin_release` (jsonb) to `workflow_runs` recording the Agent plugin release package the creation transaction froze; it is written only when the Space's `selected_release` version equals the frozen `plugin_version`, and stays NULL otherwise. The run Workspace's `plugin` step uses it in place of the Space's current selection for that plugin; a plugin the Space already removed is not appended, and the start gate fails the run as `agent_plugin_unavailable`. Semantics are in [workflows](../../../docs/workflows/workflows.en.md#scheduling); coverage is `internal/core/workflow_run_plugin_db_test.go`. - 0063 gives `scheduled_task_occurrences` a surrogate `id` (with a default, so the existing admission transaction needs no change) and adds `scheduled_run_results`: `issue_run_id` is the identity, `verdict` comes from the closed set, `verdict_detail` is at most 2000 characters, `task_id` and `scheduled_at` are the task ownership and planned instant frozen at write time, and `occurrence_id` is an audit reference only, nulled when the occurrence is trimmed. No historical run is backfilled. Semantics are in the specs decision `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`; coverage is in `TestSessionEndedWritesTheScheduledVerdict`, `TestScheduledRunResultIsWrittenOncePerRun`, `TestScheduledRunResultRefusesUnattributableReports` and `TestScheduledRunResultSurvivesOccurrenceTrim`. - 0064 adds the three scheduled-artifact tables: the delivery state with its non-renewable ten-minute window (and its prefix-cleanup marker), the verified object index and the rejection list; all three freeze `task_id`/`scheduled_at` and keep `occurrence_id` as an audit reference only. It also narrows `execution_work`'s unregistered uniqueness from "one per run" to "one per run and kind" and adds `deliver_scheduled_artifacts` to the kind set: a scheduled run holds its Revision delivery and its artifact upload at the same time. Coverage is in `TestScheduledArtifactsSkipWithoutObjectStorage`, `TestScheduledArtifactManifestIsValidatedBeforeStorage`, `TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` and `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`. +- 0069 adds `workflow_node_attempt` to `execution_work`, `node_executions` and `thread_commands` (integer, NOT NULL, default 1) and re-keys the `one_session_per_workflow_node` and `one_workflow_session_end` partial unique indexes on the attempt, so retrying an agent node can register a second session and its own end request without reusing the first attempt's evidence. Every existing row belongs to its node's first attempt, so nothing is backfilled. Fresh, 0068-upgrade, repeated application and constraint coverage is in `TestMigration0069WorkflowNodeAttemptAppliesFreshAndUpgrades`. - New migrations must verify fresh databases, upgrades from the preceding schema, repeated application and key constraints. See [migration_upgrade_path_test.go](../../../integration/migration_upgrade_path_test.go) and domain upgrade tests in integration. See [Persistence](../../../docs/core/persistence.en.md) for ownership/transactions, and [Workflows](../../../docs/workflows/workflows.en.md) / [External Git line](../../../docs/repositories/external-git.en.md) for runtime semantics. diff --git a/internal/core/migrations/README.md b/internal/core/migrations/README.md index 849e918f..4c0b43fc 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -6,7 +6,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 ## 文件索引 -当前共 65 个 SQL 文件。 +当前共 71 个 SQL 文件。 | SQL 文件 | schema 变化 | |---|---| @@ -73,7 +73,6 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | 停止未确认的命名证据,不改变残留责任和回收授权 | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Node 实例登记的插件模型 adapter 能力 | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | 定时任务定义、幂等接纳台账与截止取消事实 | -| [0062_workflow_node_attempt.sql](0062_workflow_node_attempt.sql) | 工作流节点工作项、执行与结束请求的尝试序号 | | [0062_workflow_run_plugin_release.sql](0062_workflow_run_plugin_release.sql) | 冻结运行插件 release 包 | | [0063_scheduled_run_results.sql](0063_scheduled_run_results.sql) | 定时运行的业务 verdict、冻结的任务归属与 occurrence 审计引用 | | [0064_scheduled_run_artifacts.sql](0064_scheduled_run_artifacts.sql) | 定时产物的交付窗口、已核对登记、被拒清单,以及每运行每类一个未登记工作项 | @@ -81,6 +80,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0066_scheduled_artifact_recurring_gc.sql](0066_scheduled_artifact_recurring_gc.sql) | 已结算运行持续重扫,清理迟到 PUT 与被淘汰的成功交付 | | [0067_scheduled_empty_artifacts.sql](0067_scheduled_empty_artifacts.sql) | 产物大小下界改为零,允许空文件登记 | | [0068_scheduled_manifest_completeness.sql](0068_scheduled_manifest_completeness.sql) | 保存未列明的拒绝数和遍历截断事实 | +| [0069_workflow_node_attempt.sql](0069_workflow_node_attempt.sql) | 工作流节点工作项、执行与结束请求的尝试序号 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 @@ -105,10 +105,10 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 - 0059 只增 `stop_evidence`,存量为空对象,不追认历史未知停止;覆盖见 `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`。 - 0060 给 `node_instances` 增加 `model_adapter`(jsonb 对象,默认 `{}`),记录 Node 实例登记时声明的插件模型 adapter 能力;不回填,既有实例按空能力处理,不会被派发需要 adapter 的会话,直到以新版本重新登记。空库与升级覆盖见 `TestMigration0060ModelAdapterFreshAndUpgrade`。 - 0061 增加 `scheduled_tasks`(冻结 Prompt、agent ref、`schedule` jsonb、IANA 时区、无人值守同意与软删除墓碑)和 `scheduled_task_occurrences`(`(task_id, scheduled_at)` 唯一的幂等接纳台账,含截止时刻与截止取消事实);不回填历史行。接纳由工作线程在同一事务里创建绑定 issue 下的 agent IssueRun,插件版本在接纳时按 Space 当时值快照而非定义上冻结。语义见 specs 根决策 `decisions/cloud/scheduled-task`;空库、升级与重复应用的集成覆盖随 Web 与端到端验收 commit 一并补齐。 -- 0062 给 `execution_work`、`node_executions` 和 `thread_commands` 增加 `workflow_node_attempt`(整数,NOT NULL,默认 1),并把 `one_session_per_workflow_node`、`one_workflow_session_end` 两个部分唯一索引按尝试重键,使 agent 节点重试时可以登记第二次会话及其自己的结束请求,而不复用第一次尝试的证据。既有行都属于各自节点的第一次尝试,不回填。空库、0061 升级、重复应用与约束覆盖见 `TestMigration0062WorkflowNodeAttemptAppliesFreshAndUpgrades`。 - 0062 给 `workflow_runs` 增加可空 `plugin_release`(jsonb),记录创建事务冻结的 Agent 插件 release 包;仅当 Space 的 `selected_release` 版本等于冻结的 `plugin_version` 时写入,其余为 NULL,不回填。运行 Workspace 的 `plugin` 步骤用它替换 Space 当前选择里的 Agent 插件条目;Space 已移除该插件时不追加,交由启动判定以 `agent_plugin_unavailable` 失败。语义见[工作流](../../../docs/workflows/workflows.md#调度),覆盖见 `internal/core/workflow_run_plugin_db_test.go`。 - 0063 给 `scheduled_task_occurrences` 补一个代理 `id`(默认值,既有接纳事务不用改),并增加 `scheduled_run_results`:以 `issue_run_id` 为唯一身份,`verdict` 取自 Close 集、`verdict_detail` 至多 2000 字符,`task_id` 与 `scheduled_at` 是写入时冻结的任务归属与计划时间事实,`occurrence_id` 只作审计并在裁剪 occurrence 时置空。不回填任何历史运行。语义见 specs `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`;覆盖见 `TestSessionEndedWritesTheScheduledVerdict`、`TestScheduledRunResultIsWrittenOncePerRun`、`TestScheduledRunResultRefusesUnattributableReports` 与 `TestScheduledRunResultSurvivesOccurrenceTrim`。 - 0064 增加定时产物的三张表:交付状态与不可续期 10 分钟窗口(含前缀清理标记)、已核对对象索引、被拒清单;三者都冻结 `task_id`/`scheduled_at` 并把 `occurrence_id` 只作审计。同时把 `execution_work` 的未登记唯一性从"每运行一个"改为"每运行每类一个",并在 kind 闭集里加入 `deliver_scheduled_artifacts`——定时运行要同时持 Revision 交付项和产物上传项。覆盖见 `TestScheduledArtifactsSkipWithoutObjectStorage`、`TestScheduledArtifactManifestIsValidatedBeforeStorage`、`TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` 与 `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`。 +- 0069 给 `execution_work`、`node_executions` 和 `thread_commands` 增加 `workflow_node_attempt`(整数,NOT NULL,默认 1),并把 `one_session_per_workflow_node`、`one_workflow_session_end` 两个部分唯一索引按尝试重键,使 agent 节点重试时可以登记第二次会话及其自己的结束请求,而不复用第一次尝试的证据。既有行都属于各自节点的第一次尝试,不回填。空库、0068 升级、重复应用与约束覆盖见 `TestMigration0069WorkflowNodeAttemptAppliesFreshAndUpgrades`。 - 新增迁移须验证空库、前一 schema 升级、重复应用与关键约束。覆盖见 [migration_upgrade_path_test.go](../../../integration/migration_upgrade_path_test.go) 和 integration 的领域升级测试。 数据归属与事务规则见 [持久化](../../../docs/core/persistence.md),运行语义见 [工作流](../../../docs/workflows/workflows.md) 和 [外部 Git 线](../../../docs/repositories/external-git.md)。 diff --git a/internal/core/workflow_run_session.go b/internal/core/workflow_run_session.go index 023e922b..830ffb2b 100644 --- a/internal/core/workflow_run_session.go +++ b/internal/core/workflow_run_session.go @@ -168,7 +168,7 @@ func workflowAgentSessionOutcome(t *transaction, runID string, version int64, no // path exists to remove. The deadline is measured from the command's own `created_at`, so retries // and replicas all read the same window. func workflowSessionEndRequest(t *transaction, runID string, version int64, nodeID string, attempt int64, executionID string, turnEnded bool, states map[string]workflowNodeState) (map[string]workflowNodeState, bool) { - // The end request is one per (run, node, attempt): 0062 re-keys 0049's uniqueness on the attempt, + // The end request is one per (run, node, attempt): 0069 re-keys 0049's uniqueness on the attempt, // so attempt 1's outstanding request is neither reused nor delivered to attempt 2's session. cmd := t.one("SELECT * FROM thread_commands WHERE workflow_run_id=$1 AND workflow_node_id=$2 AND workflow_node_attempt=$3 AND kind='end_session'", runID, nodeID, attempt) if cmd == nil { From 4a1efb26759c8f40bb21f066ecdb4d755a2d823a Mon Sep 17 00:00:00 2001 From: Kyber5323 Date: Fri, 23 Oct 2026 17:43:57 +0800 Subject: [PATCH 14/14] fix(core): pass the graph attempt when enqueuing scheduled artifact work The retried base gives enqueueExecutionWork a node attempt between the graph node and the availability instant. Scheduled artifact delivery has no graph node and is IssueRun work, so it passes an empty node and attempt zero like the other non-graph callers. Co-Authored-By: Claude Code --- internal/core/scheduled_artifact_delivery.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/core/scheduled_artifact_delivery.go b/internal/core/scheduled_artifact_delivery.go index 476b0f00..92235ed8 100644 --- a/internal/core/scheduled_artifact_delivery.go +++ b/internal/core/scheduled_artifact_delivery.go @@ -85,6 +85,6 @@ func recordScheduledArtifacts(t *transaction, runID, executionID string, ended O enqueueExecutionWork(t, runID, "deliver_scheduled_artifacts", Object{ "kind": "deliver_scheduled_artifacts", "sessionExecutionId": executionID, - }, sessionStartTarget(t, run.S("workspaceId")), "", time.Time{}) + }, sessionStartTarget(t, run.S("workspaceId")), "", 0, time.Time{}) return nil }