diff --git a/docs/collaboration/interactions.en.md b/docs/collaboration/interactions.en.md index 84985a274..066fe12c4 100644 --- a/docs/collaboration/interactions.en.md +++ b/docs/collaboration/interactions.en.md @@ -69,7 +69,7 @@ Cloud's own background loops, see [Agent IssueRuns](../agent-runs/issue-runs.en. A workflow has exactly one execution boundary: an explicit **confirm**. ```text -select workflow ──▶ read form descriptor ──▶ fill in (optional AI assist) ──▶ confirm ──▶ IssueRun +select workflow ──▶ read form descriptor ──▶ fill in (optional AI assist) ──▶ confirm ──▶ WorkflowRun + IssueRun no execution no execution no execution, no run the only execution boundary ``` @@ -77,6 +77,9 @@ select workflow ──▶ read form descriptor ──▶ fill in (optional AI as Issue-facing rendering description: `formRef`, title, description and a list of fields. It is not the workflow's schema and binds to no schema technology. Fields come in three bands: platform fields, the variables declared by the workflow's Start node, and the graph's global variables. + Opening the form initially reads the current document. Selecting a published version in the platform + version selector reloads the chosen snapshot's form through + `GET /tenants/:tid/workflows/:wfid/snapshots/:snapshotId/form?issueId=…`. - **Platform fields** are injected only when the form is opened for an Issue: repository URL and branch (defaulting to the repository and default branch of the Issue's Project; required), a prompt, the published version to run `snapshotId`, and additional context references `context_refs`. When the @@ -90,13 +93,18 @@ select workflow ──▶ read form descriptor ──▶ fill in (optional AI as - Frontend validation is only for user experience. On confirm Cloud resolves the target **again** and validates the submitted values against the form of **the published version being confirmed**; a workflow with no published version cannot be confirmed, because an unpublished draft cannot run. - - The form the user saw comes from the current document; if its fields no longer match the confirmed - version's, the confirm is refused with the changed fields listed - (`409 workflow_version_fields_changed`), rather than validating the user's values against a schema - they never saw. + - Cloud still compares the live draft's fields with the selected published version's fields. A + difference rejects confirm and lists the changed fields (`409 workflow_version_fields_changed`), + even when the frontend has rendered the selected snapshot's form. The frontend asks the user to + publish the current draft or choose another version; reading the snapshot's form does not remove + this check. - The submitted values are kept as the interaction's input and the executed version is frozen into the run input, so the run still knows which version it executed after the workflow is edited or published again. + - One WorkflowRun and one IssueRun are created in the same transaction, and the IssueRun's + `workflowInvocationRef` points at that WorkflowRun (a unique composite foreign key with the tenant). + Direct runs, manual workflow IssueRuns and confirm share one creation function. A linked IssueRun is + never handed to `ExecutionDispatcher`. - The interaction is claimed with a compare-and-set on `run_id IS NULL`; a repeated confirm returns `409 interaction_already_confirmed`, and confirming a non-form interaction returns `409 interaction_not_confirmable`. @@ -112,12 +120,18 @@ defining what those capabilities are internally: | `FormDescriptorProvider` | Produce the form descriptor for a `formRef` | Forms projected from workflow documents | | `InputAssistProvider` | AI assist for forms | Not wired (503) | | `ContextBuilder` | Assemble a run's context snapshot | Not wired; falls back to a deterministic snapshot of task, form values, target and raw context references | -| `ExecutionDispatcher`, `ExecutionObserver` | Hand queued runs to an external executor and observe their progress | Not wired: team runs and workflow runs confirmed from an Issue stay `queued` in production | +| `ExecutionDispatcher`, `ExecutionObserver` | Hand queued runs to an external executor and observe their progress | Not wired: team runs stay `queued` in production; workflow IssueRuns bypass it and execute through their linked WorkflowRun | Agent runs do not go through `ExecutionDispatcher`; Cloud's own scheduling carries them (see [Agent IssueRuns](../agent-runs/issue-runs.en.md)). Runs started directly from a workflow are a different aggregate, WorkflowRun; see [Workflows](../workflows/workflows.en.md). +For a linked workflow IssueRun, actual WorkflowRun status updates advance it in the same transaction: +updates affecting no rows do not project, `awaiting_input` leaves it unchanged, and contract conflicts +roll back the transaction. Output summaries are `system` `run.message` activities bounded to 20000 +bytes of valid UTF-8, never workflow-authored comments. Direct workflow runs without a linked IssueRun +create none of these collaboration records. + **Development fixtures.** `collaboration.development_fixtures: true` (environment variable `CLOUD_COLLABORATION_DEVELOPMENT_FIXTURES`) installs the in-process doubles from `internal/collab` on the Store: Agent and team targets with fixed IDs, a deterministic context builder (at most the 10 latest diff --git a/docs/collaboration/interactions.md b/docs/collaboration/interactions.md index a752b8b93..db7a668cf 100644 --- a/docs/collaboration/interactions.md +++ b/docs/collaboration/interactions.md @@ -54,12 +54,14 @@ Agent 名册(`space_agents`,由已安装的 Agent 插件形成,见 [插件 工作流的执行边界只有一个:显式的**确认**。 ```text -选中工作流 ──▶ 读取表单描述 ──▶ 填写(可选 AI 辅助)──▶ 确认 ──▶ IssueRun +选中工作流 ──▶ 读取表单描述 ──▶ 填写(可选 AI 辅助)──▶ 确认 ──▶ WorkflowRun + IssueRun 不执行 不执行 不执行、不建运行 唯一的执行边界 ``` - **表单描述**(`GET /tenants/:tid/collaboration/forms/:formRef?issueId=…`)是面向 Issue 的渲染描述:`formRef`、标题、说明和 字段列表。它不是工作流的 schema,也不绑定任何 schema 技术。字段分三段:平台字段、工作流 Start 节点声明的变量、图的全局变量。 + 初次打开时读取当前文档;在平台版本选择器中选择已发布版本后,前端通过 + `GET /tenants/:tid/workflows/:wfid/snapshots/:snapshotId/form?issueId=…` 重新读取所选快照的表单。 - **平台字段**只在带着 Issue 打开表单时注入:仓库地址和分支(默认取 Issue 关联 Project 的仓库与默认分支,必填)、提示词、要执行 的已发布版本 `snapshotId`、附加的上下文引用 `context_refs`。工作流自己声明了同名变量时,以工作流的声明为准。 - **AI 辅助**(`POST .../issues/:iid/collaboration/assist`)只返回字段级的建议补丁和建议的上下文引用:从不创建运行,也不写入 @@ -67,9 +69,10 @@ Agent 名册(`space_agents`,由已安装的 Agent 插件形成,见 [插件 - **确认**(`POST .../issues/:iid/interactions/:ixid/confirm`)是唯一把工作流交互变成执行意图的转换: - 前端校验只是体验,Cloud 在确认时**重新**解析目标,并按**被确认的那个已发布版本**的表单校验提交的值;没有任何已发布版本的 工作流不能确认,因为不能执行未发布的草稿。 - - 用户看到的表单来自当前文档;如果它与被确认版本的字段已经不一致,确认被拒绝并列出变化的字段 - (`409 workflow_version_fields_changed`),而不是用用户没见过的 schema 校验他填的值。 + - Cloud 仍比较当前活稿与所选发布版本的字段;即使前端已展示所选快照的表单,两者不一致时仍拒绝确认并列出变化的字段 + (`409 workflow_version_fields_changed`)。前端提示先发布当前活稿或选择其他版本;按快照读取表单并未取消这条检查。 - 提交的值作为交互的输入保存,被执行的版本冻结进运行输入;运行之后工作流再被编辑或发布,它仍然知道自己执行的是哪个版本。 + - 同一事务里创建一个 WorkflowRun 和一个 IssueRun,IssueRun 的 `workflowInvocationRef` 指向该 WorkflowRun(与租户一起的复合外键,且唯一);直接运行、manual 工作流 IssueRun 与确认共用同一个创建函数。已关联的 IssueRun 不交给 `ExecutionDispatcher`。 - 用 `run_id IS NULL` 的比较并交换认领交互,重复确认返回 `409 interaction_already_confirmed`;对非表单交互确认返回 `409 interaction_not_confirmable`。 @@ -83,11 +86,15 @@ Issue 协作通过一组端口向外部能力请求"做一件事",而不定义 | `FormDescriptorProvider` | 按 `formRef` 给出表单描述 | 工作流文档投影成的表单 | | `InputAssistProvider` | 表单的 AI 辅助 | 未接入(503) | | `ContextBuilder` | 组装运行的上下文快照 | 未接入,退回到"任务、表单值、目标、原始上下文引用"的确定性快照 | -| `ExecutionDispatcher`、`ExecutionObserver` | 把排队的运行交给外部执行者,并观察其进度 | 未接入:团队运行和从 Issue 确认的工作流运行在生产中保持 `queued` | +| `ExecutionDispatcher`、`ExecutionObserver` | 把排队的运行交给外部执行者,并观察其进度 | 未接入:团队运行在生产中保持 `queued`;工作流 IssueRun 不经过这里,由关联的 WorkflowRun 执行 | Agent 运行不走 `ExecutionDispatcher`,由 Cloud 自己的调度推进(见 [Agent IssueRun](../agent-runs/issue-runs.md))。直接从工作流 发起的运行是另一个聚合 WorkflowRun,见 [工作流](../workflows/workflows.md)。 +关联工作流的 IssueRun 由 WorkflowRun 的实际状态更新在同一事务内推进:未更新到行时不投影,`awaiting_input` 不改变 IssueRun, +契约冲突使事务回滚。输出摘要写成 `system` 的 `run.message` 活动,最多 20000 字节且保持合法 UTF-8,不创建工作流作者评论。 +没有关联 IssueRun 的直接工作流运行不产生这些协作记录。 + **开发夹具。** `collaboration.development_fixtures: true`(环境变量 `CLOUD_COLLABORATION_DEVELOPMENT_FIXTURES`)在 Store 上装入 `internal/collab` 的进程内替身:固定 ID 的 Agent 和团队目标、确定性的上下文构建器(最近至多 10 条评论,不用 AI)、给出固定输出的 派发器和 AI 辅助。工作流目标仍然来自数据库。这个开关只用于开发和演示,生产默认关闭;它与认证完全独立:开启 GitHub 登录不会打开 diff --git a/docs/workflows/workflows.en.md b/docs/workflows/workflows.en.md index 4956f59ed..b2304fdc3 100644 --- a/docs/workflows/workflows.en.md +++ b/docs/workflows/workflows.en.md @@ -62,8 +62,8 @@ Workflows have three start entries, all sharing one admission gate: | Entry | Produces | |---|---| | `POST .../workflows/:wfid/runs` | A WorkflowRun (`workflow_runs`) | -| `POST .../issues/:iid/runs` with `executorType = workflow` | A workflow IssueRun | -| Confirming an @workflow form on an Issue | A workflow IssueRun (see [Issue collaboration](../collaboration/interactions.en.md#form-mode-workflow)) | +| `POST .../issues/:iid/runs` with `executorType = workflow` | A WorkflowRun and the workflow IssueRun linked to it | +| Confirming an @workflow form on an Issue | A WorkflowRun and the workflow IssueRun linked to it (see [Issue collaboration](../collaboration/interactions.en.md#form-mode-workflow)) | **Admission** is decided once, in the caller's transaction: @@ -77,8 +77,10 @@ Workflows have three start entries, all sharing one admission gate: If any check fails, no run, claimed interaction or activity is left behind. The executed version is frozen onto the run, so the run still knows which version it executed after the workflow is edited or published again. -**A run's identity is frozen at creation.** A direct run must name its `projectId`: a missing one is `409 -workflow_run_project_required`, an unusable, archived or foreign-space Project is refused as `404`, and one being +**A run's identity is frozen at creation.** All three entries share one creation function: a direct +start takes the Project from the request body's `projectId`, while Confirm/manual take it from the +Issue's `project_ref`. A missing Project is `409 workflow_run_project_required` for every entry; +an unusable, archived or foreign-space Project is refused as `404`, and one being deleted is `409 resource_unavailable`. The run writes the initiating user (`created_by`) and that Project (`project_id`) into its row and never changes them — a workflow document belongs to a tenant, not to a Project, so there is nothing to default to, and the run's Workspace, its clone and its whole checkout land in the Project that @@ -151,7 +153,12 @@ seconds): Development fixtures (`collaboration.development_fixtures`) install a deterministic run simulator that fills in the execution trace at run creation and says in its output that nothing really ran; production never installs it. -Workflow IssueRuns currently have no executor in production and stay `queued` (see +A workflow IssueRun is linked through `workflow_invocation_ref` to the WorkflowRun created in the same transaction, +and the scheduling loop above advances that run. After a WorkflowRun status update affects a row, its state, +timestamps and errors are projected onto the linked IssueRun in that same transaction. An update affecting no rows +does not project; contract conflicts roll back the transaction. `awaiting_input` leaves the IssueRun unchanged, +and direct runs without a linked IssueRun create no collaboration records. Output summaries are `system` +`run.message` activities, bounded to 20000 bytes of valid UTF-8, never workflow-authored comments (see [Issue collaboration](../collaboration/interactions.en.md#ports-issues-own-only-the-consuming-side-contract)). ## Plugin asset import diff --git a/docs/workflows/workflows.md b/docs/workflows/workflows.md index 4d628886b..62f26883a 100644 --- a/docs/workflows/workflows.md +++ b/docs/workflows/workflows.md @@ -46,8 +46,8 @@ Agent 节点不在图里写死 Agent CLI 或模型 ID,而是引用一个**执 | 入口 | 产生 | |---|---| | `POST .../workflows/:wfid/runs` | 一个 WorkflowRun(`workflow_runs`) | -| `POST .../issues/:iid/runs`,`executorType = workflow` | 一个工作流 IssueRun | -| 在 Issue 上确认 @工作流 的表单 | 一个工作流 IssueRun(见 [Issue 协作](../collaboration/interactions.md#form-mode工作流)) | +| `POST .../issues/:iid/runs`,`executorType = workflow` | 一个 WorkflowRun 和关联它的工作流 IssueRun | +| 在 Issue 上确认 @工作流 的表单 | 一个 WorkflowRun 和关联它的工作流 IssueRun(见 [Issue 协作](../collaboration/interactions.md#form-mode工作流)) | **准入**在调用方的事务里一次判定: @@ -58,7 +58,8 @@ Agent 节点不在图里写死 Agent CLI 或模型 ID,而是引用一个**执 任何一项不通过都不会留下运行、已认领的交互或活动。被执行的版本冻结在运行上,运行之后工作流再被编辑或发布,它仍然知道自己执行的是哪个版本。 -**运行的身份在创建时冻结。** 直接创建运行时 `projectId` 必填:缺失返回 `409 workflow_run_project_required`,不可用、已归档或属于其他空间的 Project +**运行的身份在创建时冻结。** 三个入口共用创建函数:直接启动的 Project 来自请求体 `projectId`,Confirm/manual 来自 Issue 的 `project_ref`。 +Project 缺失时统一返回 `409 workflow_run_project_required`,不可用、已归档或属于其他空间的 Project 按 `404` 拒绝,正在删除的返回 `409 resource_unavailable`。运行把发起人(`created_by`)和这个 Project(`project_id`)写进运行行,之后不再变化——工作流文档 属于租户而不是某个 Project,没有可以默认的对象,而运行的 Workspace、它的 clone 和整份检出都落在被指定的 Project 里。运行 Workspace 归属于创建它的那次运行: `workspaces.workflow_run_id` 与 IssueRun 的 `issue_run_id` 互斥,运行时控制里为它预留的 `maintenance` 同样按运行身份记录(见 @@ -101,7 +102,10 @@ WorkflowRun 由 Cloud server 里的调度循环推进(`AdvanceWorkflowRunsOnce 保留在执行行上,不改写已经终态的运行。 开发夹具(`collaboration.development_fixtures`)会装入一个确定性的运行模拟器,在创建运行时就填好执行轨迹,并在输出里说明没有真实执行;生产环境 -不装它。工作流 IssueRun 在生产中目前没有执行端,保持 `queued`(见 [Issue 协作](../collaboration/interactions.md#端口issue-只拥有消费侧的契约))。 +不装它。工作流 IssueRun 通过 `workflow_invocation_ref` 关联同一事务里创建的 WorkflowRun,执行由上面的调度循环推进。 +WorkflowRun 的实际状态更新成功后,在同一事务里将状态、时间和错误投影回关联的 IssueRun;未更新到行时不投影,契约冲突使事务回滚。 +`awaiting_input` 不改变 IssueRun,未关联 IssueRun 的直接运行也不产生协作记录。输出摘要作为 `system` 的 `run.message` 活动写入, +最多 20000 字节且保持合法 UTF-8,不创建工作流作者评论(见 [Issue 协作](../collaboration/interactions.md#端口issue-只拥有消费侧的契约))。 ## 插件资产导入 diff --git a/frontend/src/features/issues/README.en.md b/frontend/src/features/issues/README.en.md index 4f06c88f5..225d78398 100644 --- a/frontend/src/features/issues/README.en.md +++ b/frontend/src/features/issues/README.en.md @@ -34,3 +34,9 @@ MSW stands in for the real Cloud API; under `onUnhandledRequest: 'error'` every Recovery reloads and property saves are mutually exclusive: properties and the reload button remain locked until the original save error has been handled. Failed reloads retain the error; a later save uses the loaded version and reports its own failure independently. Detail tests use SessionProvider and CurrentSpaceProvider to cover versioned association/removal, retained values and GET reload after conflicts, delayed GET/PUT exclusion and error visibility, ordinary comments creating no Run, and the Thread of a Run created through Task Mode. Component evidence never substitutes for real-provider multi-turn, terminal-state and resource-release acceptance. + +`IssueRun.workflowInvocationRef` retains the server's WorkflowRun reference for activity status and navigation; the API test checks that list reads preserve it. Workflow confirmation exclusion, retry and detail-link interaction tests live in `components/`. + +`useWorkflowSnapshotForm` reuses the generated snapshot-form GET with tenant/workflow/snapshot/issue-scoped query keys. The API test varies every scope so versions and Issues cannot share the wrong field defaults. + +`useWorkflowPublishedSnapshotIds` caches all published ids by tenant/workflow, following the generated list endpoint's cursors with a page limit of 100 and the caller's abort signal; it stores no graph copies. The activity form uses those ids to identify published-version options. API tests cover the second-page cursor, complete id list and tenant/workflow isolation. diff --git a/frontend/src/features/issues/README.md b/frontend/src/features/issues/README.md index 73e9208dd..ecbd1060a 100644 --- a/frontend/src/features/issues/README.md +++ b/frontend/src/features/issues/README.md @@ -30,3 +30,9 @@ MSW 模拟真实 Cloud 接口;`onUnhandledRequest: 'error'` 下每个测试都 恢复重读与属性保存互斥:重读期间锁住属性和重读按钮,处理原保存错误后才解锁;失败重读保留错误,后续保存仍以读取到的版本提交并独立显示失败。 详情测试经过 SessionProvider 和 CurrentSpaceProvider,验证关联/取消关联的版本号、冲突后保留及 GET 重读、延迟 GET/PUT 的互斥与错误可见性,以及普通评论不创建 Run、Task Mode 创建的同 Run Thread 可见。组件证据不替代真实供应商多轮、终态和资源释放的联合验收。 + +`IssueRun.workflowInvocationRef` 保留服务端返回的 WorkflowRun 引用,供活动栏读取真实运行状态并导航;API 测试检查列表没有丢失该关联。Workflow 确认去重、失败重试和详情链接的交互测试位于 `components/`。 + +`useWorkflowSnapshotForm` 复用生成的 snapshot form GET,查询键包含 tenant/workflow/snapshot/issue;API 测试逐项切换这些范围,避免不同版本或 Issue 共用表单默认值。 + +`useWorkflowPublishedSnapshotIds` 按 tenant/workflow 缓存全部已发布版本 id,使用生成的列表接口逐页读取(每页 100,传递取消信号),不缓存图副本。活动表单用它判断下拉选项是否都属于已发布版本;API 测试验证第二页游标、完整 id 列表及租户/Workflow 隔离。 diff --git a/frontend/src/features/issues/api.test.tsx b/frontend/src/features/issues/api.test.tsx index 414ee2a13..46584c844 100644 --- a/frontend/src/features/issues/api.test.tsx +++ b/frontend/src/features/issues/api.test.tsx @@ -4,7 +4,16 @@ import { http, HttpResponse } from 'msw' import { describe, expect, it } from 'vitest' import { server } from '@/test/msw-server' import type { FormDescriptor, Issue } from './types' -import { useCreateIssue, useFormDescriptor, useIssues, useMoveIssue, useUpdateIssue } from './api' +import { + useCreateIssue, + useFormDescriptor, + useIssues, + useMoveIssue, + useRuns, + useUpdateIssue, + useWorkflowPublishedSnapshotIds, + useWorkflowSnapshotForm, +} from './api' function wrapper(queryClient: QueryClient) { return function Wrapper({ children }: { children: React.ReactNode }) { @@ -47,6 +56,28 @@ describe('useIssues', () => { }) }) +describe('useRuns', () => { + it('preserves the WorkflowRun reference for confirmed interaction navigation', async () => { + const linkedRun = { + id: 'r1', + executorType: 'workflow', + executorId: 'wf1', + workflowInvocationRef: 'wr1', + status: 'queued', + } + server.use( + http.get('/api/v1/tenants/t1/issues/i1/runs', () => + HttpResponse.json({ items: [linkedRun], nextCursor: '' }), + ), + ) + const { result } = renderHook(() => useRuns('t1', 'i1'), { + wrapper: wrapper(new QueryClient()), + }) + + await waitFor(() => expect(result.current.data).toEqual([linkedRun])) + }) +}) + describe('useUpdateIssue', () => { it('sends the version alongside the patch and caches the returned issue', async () => { let body: unknown @@ -180,3 +211,128 @@ describe('useMoveIssue', () => { expect(queryClient.getQueryData(['issue', 't1', 'i1'])).toEqual(moved) }) }) + +describe('useWorkflowSnapshotForm', () => { + it.each([ + { + scope: 'tenant', + change: { tenant: 't2' }, + path: '/tenants/t2/workflows/wf1/snapshots/s1/form?issueId=i1', + }, + { + scope: 'workflow', + change: { workflow: 'wf2' }, + path: '/tenants/t1/workflows/wf2/snapshots/s1/form?issueId=i1', + }, + { + scope: 'snapshot', + change: { snapshot: 's2' }, + path: '/tenants/t1/workflows/wf1/snapshots/s2/form?issueId=i1', + }, + { + scope: 'issue', + change: { issue: 'i2' }, + path: '/tenants/t1/workflows/wf1/snapshots/s1/form?issueId=i2', + }, + ])('separates the descriptor cache when $scope changes', async ({ change, path }) => { + const queryClient = new QueryClient() + const requests: string[] = [] + server.use( + http.get( + '/api/v1/tenants/:tid/workflows/:wfid/snapshots/:snapshotId/form', + ({ request, params }) => { + requests.push(new URL(request.url).pathname + new URL(request.url).search) + return HttpResponse.json({ + formRef: params['wfid'], + snapshotId: params['snapshotId'], + fields: [], + }) + }, + ), + ) + const initial = { tenant: 't1', workflow: 'wf1', snapshot: 's1', issue: 'i1' } + const { result, rerender } = renderHook( + (props: typeof initial) => + useWorkflowSnapshotForm(props.tenant, props.workflow, props.snapshot, props.issue), + { wrapper: wrapper(queryClient), initialProps: initial }, + ) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + rerender({ ...initial, ...change }) + await waitFor(() => expect(requests).toHaveLength(2)) + await waitFor(() => expect(result.current.isSuccess).toBe(true)) + expect(requests).toEqual([ + '/api/v1/tenants/t1/workflows/wf1/snapshots/s1/form?issueId=i1', + `/api/v1${path}`, + ]) + expect(queryClient.getQueryCache().getAll()).toHaveLength(2) + }) +}) + +describe('useWorkflowPublishedSnapshotIds', () => { + it.each([ + { scope: 'tenant', tenant: 't2', workflow: 'wf1' }, + { scope: 'workflow', tenant: 't1', workflow: 'wf2' }, + ])('follows all pages and isolates ids when $scope changes', async ({ tenant, workflow }) => { + const requests: { path: string; limit: string | null; cursor: string | null }[] = [] + server.use( + http.get('/api/v1/tenants/:tid/workflows/:wfid/snapshots', ({ request, params }) => { + const url = new URL(request.url) + const cursor = url.searchParams.get('cursor') + requests.push({ path: url.pathname, limit: url.searchParams.get('limit'), cursor }) + const prefix = `${String(params['tid'])}/${String(params['wfid'])}` + return HttpResponse.json({ + items: [{ id: `${prefix}/${cursor ? 'second' : 'first'}` }], + nextCursor: cursor ? '' : 'page-two', + }) + }), + ) + const cache = new QueryClient() + const { result, rerender } = renderHook( + (props: { tenant: string; workflow: string }) => + useWorkflowPublishedSnapshotIds(props.tenant, props.workflow), + { wrapper: wrapper(cache), initialProps: { tenant: 't1', workflow: 'wf1' } }, + ) + await waitFor(() => expect(result.current.data).toEqual(['t1/wf1/first', 't1/wf1/second'])) + rerender({ tenant, workflow }) + await waitFor(() => + expect(result.current.data).toEqual([ + `${tenant}/${workflow}/first`, + `${tenant}/${workflow}/second`, + ]), + ) + expect(requests).toEqual([ + { path: '/api/v1/tenants/t1/workflows/wf1/snapshots', limit: '100', cursor: null }, + { path: '/api/v1/tenants/t1/workflows/wf1/snapshots', limit: '100', cursor: 'page-two' }, + { + path: `/api/v1/tenants/${tenant}/workflows/${workflow}/snapshots`, + limit: '100', + cursor: null, + }, + { + path: `/api/v1/tenants/${tenant}/workflows/${workflow}/snapshots`, + limit: '100', + cursor: 'page-two', + }, + ]) + expect( + cache + .getQueryCache() + .getAll() + .map((query) => query.queryKey), + ).toEqual([ + ['workflow-published-snapshot-ids', 't1', 'wf1'], + ['workflow-published-snapshot-ids', tenant, workflow], + ]) + }) + + it.each([ + { tenant: '', workflow: 'wf1' }, + { tenant: 't1', workflow: undefined }, + ])('does not fetch until tenant and workflow ids are present: %j', ({ tenant, workflow }) => { + const { result } = renderHook(() => useWorkflowPublishedSnapshotIds(tenant, workflow), { + wrapper: wrapper(new QueryClient()), + }) + expect(result.current.fetchStatus).toBe('idle') + expect(result.current.data).toBeUndefined() + }) +}) diff --git a/frontend/src/features/issues/api.ts b/frontend/src/features/issues/api.ts index 4378dee3f..e52dd90ba 100644 --- a/frontend/src/features/issues/api.ts +++ b/frontend/src/features/issues/api.ts @@ -1,4 +1,8 @@ import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' +import { + getApiV1TenantsTidWorkflowsWfidSnapshots, + getApiV1TenantsTidWorkflowsWfidSnapshotsSnapshotIdForm, +} from '@/api/workflows/workflows' import { AXIOS_INSTANCE } from '@/lib/api-client' import type { AssistSuggestion, @@ -312,6 +316,51 @@ export function useFormDescriptor(tid: string, formRef: string | undefined, issu }) } +/** Lists every published snapshot id so version-selector checks do not depend on UUID page order. */ +export function useWorkflowPublishedSnapshotIds(tid: string, workflowId: string | undefined) { + return useQuery({ + queryKey: ['workflow-published-snapshot-ids', tid, workflowId ?? ''], + queryFn: ({ signal }) => { + const ids: string[] = [] + async function collectPage(cursor = ''): Promise { + const page = await getApiV1TenantsTidWorkflowsWfidSnapshots( + tid, + workflowId ?? '', + { limit: 100, ...(cursor ? { cursor } : {}) }, + undefined, + signal, + ) + ids.push(...page.items.map((item) => item.id)) + return page.nextCursor ? collectPage(page.nextCursor) : ids + } + return collectPage() + }, + enabled: !!tid && !!workflowId, + }) +} + +/** Reads a chosen frozen version's form, isolated by tenant, workflow, snapshot and issue context. */ +export function useWorkflowSnapshotForm( + tid: string, + workflowId: string, + snapshotId: string | undefined, + issueId: string, +) { + return useQuery({ + queryKey: ['workflow-snapshot-form', tid, workflowId, snapshotId ?? '', issueId], + queryFn: ({ signal }) => + getApiV1TenantsTidWorkflowsWfidSnapshotsSnapshotIdForm( + tid, + workflowId, + snapshotId ?? '', + { issueId }, + undefined, + signal, + ), + enabled: !!tid && !!workflowId && !!snapshotId && !!issueId, + }) +} + /** * Asks for AI suggestions for a Workflow form that has **not** been confirmed yet. The form is a draft * until the user confirms it, so this route is deliberately stateless: no interaction needs to exist, diff --git a/frontend/src/features/issues/components/README.en.md b/frontend/src/features/issues/components/README.en.md index 6244393eb..d80c008a7 100644 --- a/frontend/src/features/issues/components/README.en.md +++ b/frontend/src/features/issues/components/README.en.md @@ -29,6 +29,16 @@ Depends on `features/issues/api`, `features/issues/types`, `features/issues/pres Creation drafts remount by tenant/space; activity drafts by tenant/space/issue, including navigation to cached scopes. Project association follows server results; versioned updates disable property edits until settled and keep stored values on refusal. Project guidance targets the `issue-project` selector. Targets come only from the real directory; the server remains authoritative for space compatibility. +Workflow confirmation locks duplicate submissions, editing and removal from interaction creation onward; a rejected confirm reuses that interaction on retry, including after a version switch. Confirmed rows use `features/workflows/api` to read the WorkflowRun referenced by `IssueRun.workflowInvocationRef`; detail links combine `executorId`, that reference and the route's workspace slug. + +The initial form reads the live descriptor by `formRef + issueId`. A `snapshotId` select is recognized as the platform version selector only when every option belongs to this workflow's published snapshots; the form waits for that list and allows retry after a failed read. An author's same-named text field or select with custom options remains ordinary input. + +Changing the platform version selector loads the existing snapshot-form route with a tenant/workflow/snapshot/issue-scoped key, clears the old field draft and initializes the selected version's defaults. The selector retains the echoed snapshotId rather than its newest-version default. Loading or failed reads cannot review stale fields; failed reads can retry. Reading a form is not run admission; Confirm's `workflow_version_fields_changed` gate is unchanged. That refusal explains the selected version differs from the current draft and directs the user to publish the draft or pick another version. + +Historical runs without a WorkflowRun reference retain their IssueRun status, make no WorkflowRun request and show no detail link. Linked runs always use the WorkflowRun's authoritative status. + ## Testing Component tests use MSW; properties and Task Mode→Thread are covered by `issue-detail-page.test.tsx`. Loading, empty, refused and retry states are distinct; creation, ordinary-comment and target failures retain drafts, while scope changes clear them. + +Workflow tests cover double-click exclusion, rejected confirms reusing an interaction across version switches, snapshot fields/defaults/echoes, an author's same-named select making no snapshot-form request, actionable field-divergence feedback, pending and failed reads, and WorkflowRun status and detail links. diff --git a/frontend/src/features/issues/components/README.md b/frontend/src/features/issues/components/README.md index 5583364f9..9ed788ee9 100644 --- a/frontend/src/features/issues/components/README.md +++ b/frontend/src/features/issues/components/README.md @@ -29,6 +29,16 @@ 创建草稿按 tenant/space 重挂,活动草稿按 tenant/space/issue 重挂;切到已缓存的其他作用域也不能继承草稿。关联项目以服务端结果为准,版本化更新等待期间禁止再改属性,拒绝保留原值。缺项目提示定位到 `issue-project` 选择器;目标必须来自真实目录,空间合法性仍由服务端校验。 +Workflow 确认从创建 interaction 开始锁住重复提交、返回修改与取消;确认失败后仍用同一 interaction 重试,包括切换版本后的重试。确认行通过 `features/workflows/api` 读取 `IssueRun.workflowInvocationRef` 对应的 WorkflowRun 状态,详情链接使用 `executorId`、该引用和当前路由的空间 slug。 + +初始表单按 `formRef + issueId` 读取活稿。只有 `snapshotId` select 的全部选项都属于该 Workflow 的已发布快照时,才识别为平台版本选择器;列表读取完成前不开放表单,失败可重试。作者自定义的同名文本字段或含自定义选项的 select 仍是普通输入。 + +用户切换平台版本选择器时,按 tenant/workflow/snapshot/issue 读取既有 snapshot form 接口,清空旧字段草稿并初始化新版本默认值;选择器保留响应回显的 snapshotId,不采用它仍指向最新版本的默认值。加载或失败期间不能审阅旧字段,失败可重试。读取表单不代表服务端准入通过,Confirm 的 `workflow_version_fields_changed` 门禁保持不变;收到该拒绝时,界面提示所选版本字段与当前草稿不一致,并引导先发布草稿或选择其他版本。 + +历史运行没有 WorkflowRun 引用时,确认行仍显示 IssueRun 状态,不请求 WorkflowRun,也不显示运行详情链接;有关联的运行始终以 WorkflowRun 状态为准。 + ## 测试 组件测试用 MSW;属性栏和 Task Mode→Thread 经 `issue-detail-page.test.tsx` 覆盖。测试区分加载、空列表、拒绝、重试;创建、普通评论和目标提交失败保留草稿,作用域切换则清空草稿。 + +Workflow 测试覆盖双击互斥、确认失败后复用 interaction(含切换版本)、版本字段/default/回显切换、作者同名 select 不发快照表单请求、字段差异拒绝的操作提示、读取等待与失败,以及 WorkflowRun 状态和详情链接。 diff --git a/frontend/src/features/issues/components/activity-panel.test.tsx b/frontend/src/features/issues/components/activity-panel.test.tsx index f248619e6..be81242fa 100644 --- a/frontend/src/features/issues/components/activity-panel.test.tsx +++ b/frontend/src/features/issues/components/activity-panel.test.tsx @@ -4,7 +4,7 @@ import { http, HttpResponse } from 'msw' import { describe, expect, it, vi } from 'vitest' import { useState } from 'react' import { useQueryClient } from '@tanstack/react-query' -import { renderWithProviders } from '@/test/render' +import { renderAtRoute, renderWithProviders } from '@/test/render' import { server } from '@/test/msw-server' import { installSignedInSession } from '@/test/cloud-handlers' import type { @@ -266,6 +266,74 @@ describe('ActivityPanel', () => { expect(screen.queryByRole('button', { name: '审阅' })).not.toBeInTheDocument() }) + it('reads WorkflowRun status and links its identity within the current workspace', async () => { + servePanel([], [interaction({ runId: 'r1' })]) + server.use( + http.get('/api/v1/tenants/t1/issues/i1/runs', () => + HttpResponse.json({ + items: [ + { + id: 'r1', + executorId: 'wf-executor', + executorType: 'workflow', + status: 'queued', + workflowInvocationRef: 'workflow-run-1', + }, + ], + nextCursor: '', + }), + ), + http.get('/api/v1/tenants/t1/workflows/wf-executor/runs/workflow-run-1', () => + HttpResponse.json({ id: 'workflow-run-1', status: 'succeeded' }), + ), + ) + renderAtRoute( + '/w/:workspaceSlug/issues/:issueId', + , + '/w/team-space/issues/i1', + ) + + expect(await screen.findByText('已确认 · 运行 succeeded')).toBeInTheDocument() + expect(screen.queryByText(/运行 queued/)).not.toBeInTheDocument() + expect(screen.getByRole('link', { name: '运行详情' })).toHaveAttribute( + 'href', + '/w/team-space/workflows/wf-executor/runs/workflow-run-1', + ) + }) + + it.each([null, undefined])( + 'keeps historical IssueRun status without a detail link when the workflow reference is %s', + async (workflowInvocationRef) => { + servePanel([], [interaction({ runId: 'legacy-run' })]) + const workflowRequest = vi.fn<() => void>() + server.use( + http.get('/api/v1/tenants/t1/issues/i1/runs', () => + HttpResponse.json({ + items: [ + { + id: 'legacy-run', + executorId: 'wf1', + executorType: 'workflow', + status: 'failed', + workflowInvocationRef, + }, + ], + nextCursor: '', + }), + ), + http.get('/api/v1/tenants/t1/workflows/wf1/runs/:runId', () => { + workflowRequest() + return HttpResponse.json({ status: 'succeeded' }) + }), + ) + renderWithProviders() + + expect(await screen.findByText('已确认 · 运行 failed')).toBeInTheDocument() + expect(screen.queryByRole('link', { name: '运行详情' })).not.toBeInTheDocument() + expect(workflowRequest).not.toHaveBeenCalled() + }, + ) + it('renders a workflow progress/message activity with its human-readable detail boxed', async () => { servePanel([ comment(), diff --git a/frontend/src/features/issues/components/activity-panel.tsx b/frontend/src/features/issues/components/activity-panel.tsx index c7a882c15..32e7e10a1 100644 --- a/frontend/src/features/issues/components/activity-panel.tsx +++ b/frontend/src/features/issues/components/activity-panel.tsx @@ -1,4 +1,5 @@ import { useState } from 'react' +import { Link, useParams } from 'react-router-dom' import { useCurrentSpace } from '@/features/spaces/current-space' import { AgentRequestFailure } from './agent-request-failure' import { ActorAvatar, type AvatarActor } from '@/components/common/actor-avatar' @@ -16,6 +17,8 @@ import { import { PendingTargets } from '@/features/issues/components/pending-targets' import { TargetPicker } from '@/features/issues/components/target-picker' import { memberNameById } from '@/features/issues/present' +import { useWorkflowRun } from '@/features/workflows/api' +import { workspacePaths } from '@/lib/paths' import { cn } from '@/lib/utils' import { ConversationMarkdown } from '@/features/conversation-content/markdown' import type { @@ -130,20 +133,40 @@ function ActivityItem({ } /** A confirmed workflow interaction: it is history now, and never offers a second configuration. */ -function ConfirmedWorkflowRow({ name, run }: { name: string; run: IssueRun | undefined }) { +function ConfirmedWorkflowRow({ + slug, + name, + run, +}: { + slug: string + name: string + run: IssueRun | undefined +}) { + const { workspaceSlug } = useParams<{ workspaceSlug: string }>() + const runId = run?.workflowInvocationRef ?? undefined + const workflow = useWorkflowRun(slug, run?.executorId, runId) + const status = runId ? workflow.data?.status : run?.status + const path = workspacePaths(workspaceSlug ?? slug) return (
Workflow {name} - 已确认{run ? ` · 运行 ${run.status}` : ''} + 已确认{status ? ` · 运行 ${status}` : ''} + {workflow.isError ? 运行状态加载失败 : null} + {run && runId ? ( + + 运行详情 + + ) : null}
) } /** Confirmed workflow interactions, shown as status rows next to the history. */ function ConfirmedWorkflows(props: { + slug: string interactions: IssueInteraction[] runs: IssueRun[] catalog: CollaborationTargetSummary[] @@ -155,6 +178,7 @@ function ConfirmedWorkflows(props: { {confirmed.map((item) => ( candidate.id === item.runId)} /> @@ -242,7 +266,12 @@ function IssueActivity({ slug, issueId, members, onOpenRun, onTaskStarted }: Act )} - +
diff --git a/frontend/src/features/issues/components/workflow-interaction-composer.tsx b/frontend/src/features/issues/components/workflow-interaction-composer.tsx index e92c4f8a7..a173ecb38 100644 --- a/frontend/src/features/issues/components/workflow-interaction-composer.tsx +++ b/frontend/src/features/issues/components/workflow-interaction-composer.tsx @@ -1,5 +1,6 @@ import { ChevronDown, ChevronUp, Sparkles, Workflow, X } from 'lucide-react' -import { useEffect, useRef, useState } from 'react' +import { useMutation } from '@tanstack/react-query' +import { useRef, useState } from 'react' import { Badge } from '@/components/ui/badge' import { Button } from '@/components/ui/button' import { Skeleton } from '@/components/ui/skeleton' @@ -8,6 +9,8 @@ import { useAssistWorkflow, useConfirmWorkflow, useFormDescriptor, + useWorkflowPublishedSnapshotIds, + useWorkflowSnapshotForm, } from '@/features/issues/api' import { AssistSuggestions } from '@/features/issues/components/assist-suggestions' import { ConfirmReview } from '@/features/issues/components/confirm-review' @@ -20,18 +23,37 @@ import type { FormField, FormValues, } from '@/features/issues/types' +import { faultCode } from '@/lib/api-client' /** Seeds the editable values from the descriptor's declared defaults. */ -function seedValues(descriptor: FormDescriptor): FormValues { +function seedValues(descriptor: FormDescriptor, snapshotId?: string): FormValues { const values: FormValues = {} for (const field of descriptor.fields) { if (field.defaultValue !== undefined && field.defaultValue !== null) { values[field.key] = field.defaultValue } } + if (snapshotId) values['snapshotId'] = snapshotId return values } +// An author's snapshotId field suppresses platform injection, so its name and type are insufficient. +function isSnapshotSelector(field: FormField, publishedIds: string[]): boolean { + return ( + field.key === 'snapshotId' && + field.type === 'select' && + !!field.options?.length && + field.options.every((option) => publishedIds.includes(option.value)) + ) +} + +function confirmFailure(error: unknown): string { + if (faultCode(error) === 'workflow_version_fields_changed') { + return '所选版本的字段与当前草稿不一致,请先发布当前草稿或选择其他版本。' + } + return '确认失败,请重试。' +} + /** Required fields the user has not filled yet — the only client-side gate on Review (Issue collaboration D9). */ function missingRequired(descriptor: FormDescriptor, values: FormValues): FormField[] { return descriptor.fields.filter((field) => { @@ -96,18 +118,11 @@ function mergeContextRefs(applied: ContextRefRef[], picked: ContextRefRef[]): Co } /** Editable form state for one Workflow interaction. Pure UI state — nothing here is persisted. */ -function useWorkflowForm(descriptor: FormDescriptor | undefined) { - const [values, setValues] = useState({}) +function useWorkflowForm(descriptor: FormDescriptor, snapshotId?: string) { + const [values, setValues] = useState(() => seedValues(descriptor, snapshotId)) const [contextRefs, setContextRefs] = useState([]) const [suggestion, setSuggestion] = useState(null) const [appliedKeys, setAppliedKeys] = useState([]) - const seeded = useRef(null) - - useEffect(() => { - if (!descriptor || seeded.current === descriptor.formRef) return - seeded.current = descriptor.formRef - setValues(seedValues(descriptor)) - }, [descriptor]) function setField(key: string, next: unknown) { setValues((current) => ({ ...current, [key]: next })) @@ -174,11 +189,16 @@ function WorkflowFormBody(props: { missing: FormField[] onAssist: () => void onReview: () => void + onFieldChange: (key: string, next: unknown) => void }) { const { descriptor, form } = props return (
- + {form.suggestion ? ( void onRemove: () => void }) { @@ -255,6 +277,7 @@ function ComposerHeader({ type="button" className="ml-auto rounded p-0.5 hover:bg-muted" onClick={onRemove} + disabled={pending} aria-label="取消" > @@ -273,11 +296,12 @@ function ComposerBody(props: { assistPending: boolean confirmPending: boolean assistFailed: boolean - confirmFailed: boolean + confirmError: string | undefined onAssist: () => void onReview: () => void onBack: () => void onConfirm: () => void + onFieldChange: (key: string, next: unknown) => void }) { if (props.reviewing) { return ( @@ -292,8 +316,10 @@ function ComposerBody(props: { onBack={props.onBack} onConfirm={props.onConfirm} /> - {props.confirmFailed ? ( -

确认失败,请重试。

+ {props.confirmError ? ( +

+ {props.confirmError} +

) : null} ) @@ -307,6 +333,7 @@ function ComposerBody(props: { missing={props.missing} onAssist={props.onAssist} onReview={props.onReview} + onFieldChange={props.onFieldChange} /> {props.assistFailed ? (

AI 建议当前不可用。

@@ -315,6 +342,53 @@ function ComposerBody(props: { ) } +/** Keeps one interaction across retries and locks before React can render a second click. */ +function useWorkflowSubmission({ + slug, + issueId, + target, + onConfirmed, +}: { + slug: string + issueId: string + target: CollaborationTargetSummary + onConfirmed: () => void +}) { + const interactionId = useRef(null) + const locked = useRef(false) + const confirm = useConfirmWorkflow(slug, issueId) + const submission = useMutation({ + mutationFn: async (input: { values: FormValues; contextRefs?: ContextRefRef[] }) => { + if (!interactionId.current) { + const interaction = await openWorkflowInteraction( + slug, + issueId, + target.id, + `@${target.displayName}`, + ) + interactionId.current = interaction.id + } + return confirm.mutateAsync({ interactionId: interactionId.current, ...input }) + }, + onSuccess: onConfirmed, + onError: () => { + locked.current = false + }, + }) + + function submit(input: { values: FormValues; contextRefs?: ContextRefRef[] }) { + if (locked.current) return + locked.current = true + submission.mutate(input) + } + + return { + submit, + isPending: submission.isPending || submission.isSuccess, + errorMessage: submission.isError ? confirmFailure(submission.error) : undefined, + } +} + /** * The Form Mode configuration panel for a workflow the user has selected but not yet submitted. It is * a **draft**: the comment, the interaction and the run are all created only when 确认执行 is pressed, @@ -334,23 +408,83 @@ export function WorkflowInteractionComposer({ onRemove: () => void }) { const formRef = target.interactionDescriptor.formRef ?? undefined - const descriptorQuery = useFormDescriptor(slug, formRef, issueId) - const assist = useAssistWorkflow(slug, issueId) - const confirm = useConfirmWorkflow(slug, issueId) - const [reviewing, setReviewing] = useState(false) - const [collapsed, setCollapsed] = useState(false) + const [snapshotId, setSnapshotId] = useState() + const live = useFormDescriptor(slug, formRef, issueId) + const snapshot = useWorkflowSnapshotForm(slug, target.id, snapshotId, issueId) + const descriptorQuery = snapshotId ? snapshot : live + const confirm = useWorkflowSubmission({ slug, issueId, target, onConfirmed }) const descriptor = descriptorQuery.data - const form = useWorkflowForm(descriptor) + const candidate = descriptor?.fields.find( + (field) => field.key === 'snapshotId' && field.type === 'select', + ) + const published = useWorkflowPublishedSnapshotIds(slug, candidate ? target.id : undefined) - if (descriptorQuery.isPending) return - if (descriptorQuery.isError || !descriptor) { + if (descriptorQuery.isPending || (candidate && published.isPending)) { + return + } + if (descriptorQuery.isError || !descriptor || (candidate && published.isError)) { return ( -

- 当前部署无法加载该 Workflow 的表单(form descriptor 不可用)。 -

+
+

+ 当前部署无法加载该 Workflow 的表单(form descriptor 不可用)。 +

+ +
) } + return ( + + isSnapshotSelector(field, published.data ?? []), + )} + snapshotId={snapshot.data?.snapshotId} + loading={descriptorQuery.isFetching} + confirm={confirm} + onSnapshotChange={setSnapshotId} + onRemove={onRemove} + /> + ) +} + +/** A fresh field draft per selected version; submission identity stays with the outer composer. */ +function WorkflowInteractionForm(props: { + slug: string + issueId: string + target: CollaborationTargetSummary + descriptor: FormDescriptor + versionField: FormField | undefined + snapshotId: string | undefined + loading: boolean + confirm: ReturnType + onSnapshotChange: (id: string) => void + onRemove: () => void +}) { + const { slug, issueId, target, descriptor, confirm, onRemove } = props + const assist = useAssistWorkflow(slug, issueId) + const [reviewing, setReviewing] = useState(false) + const [collapsed, setCollapsed] = useState(false) + const form = useWorkflowForm(descriptor, props.versionField ? props.snapshotId : undefined) + + function setField(key: string, next: unknown) { + if (typeof next === 'string' && key === props.versionField?.key) { + props.onSnapshotChange(next) + } else { + form.setField(key, next) + } + } + const contextRefs = mergeContextRefs( form.contextRefs, contextRefsFromField(form.values[CONTEXT_REFS_KEY]), @@ -358,22 +492,12 @@ export function WorkflowInteractionComposer({ const confirmInput = contextRefs.length > 0 ? { values: form.values, contextRefs } : { values: form.values } - /** Confirm is the first moment anything reaches the server: comment -> interaction -> run. */ - async function submit() { - const interaction = await openWorkflowInteraction( - slug, - issueId, - target.id, - `@${target.displayName}`, - ) - confirm.mutate({ interactionId: interaction.id, ...confirmInput }, { onSuccess: onConfirmed }) - } - return (
setCollapsed((current) => !current)} onRemove={onRemove} /> @@ -381,7 +505,8 @@ export function WorkflowInteractionComposer({ {!collapsed && descriptor.description ? (

{descriptor.description}

) : null} - {!collapsed && ( + {props.loading && } + {!collapsed && !props.loading && ( assist.mutate( { targetId: target.id, values: form.values }, @@ -400,9 +525,8 @@ export function WorkflowInteractionComposer({ } onReview={() => setReviewing(true)} onBack={() => setReviewing(false)} - onConfirm={() => { - void submit() - }} + onConfirm={() => confirm.submit(confirmInput)} + onFieldChange={setField} /> )}
diff --git a/frontend/src/features/issues/components/workflow-interaction.test.tsx b/frontend/src/features/issues/components/workflow-interaction.test.tsx index 187322ccb..b75fddfd6 100644 --- a/frontend/src/features/issues/components/workflow-interaction.test.tsx +++ b/frontend/src/features/issues/components/workflow-interaction.test.tsx @@ -1,4 +1,4 @@ -import { screen, waitFor, within } from '@testing-library/react' +import { fireEvent, screen, waitFor, within } from '@testing-library/react' import userEvent from '@testing-library/user-event' import { http, HttpResponse } from 'msw' import { describe, expect, it, vi } from 'vitest' @@ -180,6 +180,9 @@ function serveComposer(overrides: { descriptor?: unknown; suggestion?: AssistSug http.get('/api/v1/tenants/t1/collaboration/forms/security-review', () => HttpResponse.json(overrides.descriptor ?? descriptor), ), + http.get('/api/v1/tenants/t1/workflows/wf1/snapshots', () => + HttpResponse.json({ items: [{ id: 'snap-3' }, { id: 'snap-2' }], nextCursor: '' }), + ), http.post('/api/v1/tenants/t1/issues/i1/collaboration/assist', async ({ request }) => { assist(await request.json()) return HttpResponse.json(overrides.suggestion ?? suggestion) @@ -238,6 +241,44 @@ async function reviewAndConfirm(user: ReturnType) { await user.click(await screen.findByRole('button', { name: '确认执行' })) } +/** Renders the composer and confirms once against a server that answers the field-divergence 409. */ +async function confirmIntoFieldsChanged() { + server.use( + http.post('/api/v1/tenants/t1/issues/i1/interactions/ix1/confirm', () => + HttpResponse.json({ code: 'workflow_version_fields_changed' }, { status: 409 }), + ), + ) + const user = userEvent.setup() + const { onConfirmed } = renderComposer() + await screen.findByLabelText(/提示词/) + await reviewAndConfirm(user) + return { user, onConfirmed } +} + +function serveOldSnapshot() { + const read = vi.fn<(issueId: string | null) => void>() + server.use( + http.get('/api/v1/tenants/t1/workflows/wf1/snapshots/snap-2/form', ({ request }) => { + read(new URL(request.url).searchParams.get('issueId')) + return HttpResponse.json({ + ...launchDescriptor, + snapshotId: 'snap-2', + fields: [ + ...launchDescriptor.fields.filter((field) => field.key !== 'prompt'), + { + key: 'oldInput', + label: 'Old input', + type: 'text', + required: true, + defaultValue: 'old default', + }, + ], + }) + }), + ) + return read +} + describe('WorkflowInteractionComposer', () => { it('renders exactly the fields the descriptor declares', async () => { serveComposer() @@ -391,6 +432,92 @@ describe('WorkflowInteractionComposer', () => { ) }) + it('locks both confirm clicks while the first interaction is still being created', async () => { + const { comment, confirm } = serveComposer({ descriptor: platformDescriptor }) + let release = vi.fn<() => void>() + const gate = new Promise((resolve) => { + release = vi.fn<() => void>(resolve) + }) + server.use( + http.post('/api/v1/tenants/t1/issues/i1/comments', async ({ request }) => { + comment(await request.json()) + await gate + return HttpResponse.json({ resource: { id: 'c1' } }) + }), + ) + const user = userEvent.setup() + const { onConfirmed } = renderComposer() + await user.click(await screen.findByRole('button', { name: '审阅' })) + const button = screen.getByRole('button', { name: '确认执行' }) + fireEvent.click(button) + fireEvent.click(button) + + try { + await waitFor(() => expect(comment).toHaveBeenCalledTimes(1)) + expect(screen.getByRole('button', { name: '确认中…' })).toBeDisabled() + expect(screen.getByRole('button', { name: '返回修改' })).toBeDisabled() + expect(screen.getByRole('button', { name: '取消' })).toBeDisabled() + expect(confirm).not.toHaveBeenCalled() + } finally { + release() + } + await waitFor(() => expect(onConfirmed).toHaveBeenCalledTimes(1)) + expect(comment).toHaveBeenCalledTimes(1) + expect(confirm).toHaveBeenCalledTimes(1) + }) + + it('reuses its interaction after a rejected confirm, including edits before retry', async () => { + const { comment, confirm } = serveComposer({ descriptor: platformDescriptor }) + let attempts = 0 + server.use( + http.post('/api/v1/tenants/t1/issues/i1/interactions/ix1/confirm', async ({ request }) => { + confirm(await request.json()) + attempts += 1 + return attempts === 1 + ? HttpResponse.json({ code: 'invalid_input' }, { status: 400 }) + : HttpResponse.json({ resource: run() }) + }), + ) + const user = userEvent.setup() + const { onConfirmed } = renderComposer() + await screen.findByLabelText(/仓库地址/) + await reviewAndConfirm(user) + expect(await screen.findByText('确认失败,请重试。')).toBeInTheDocument() + expect(onConfirmed).not.toHaveBeenCalled() + + await user.click(screen.getByRole('button', { name: '返回修改' })) + await user.type(screen.getByLabelText('Notes'), 'retry notes') + await reviewAndConfirm(user) + + await waitFor(() => expect(onConfirmed).toHaveBeenCalledTimes(1)) + expect(comment).toHaveBeenCalledTimes(1) + expect(confirm).toHaveBeenCalledTimes(2) + expect(confirm).toHaveBeenLastCalledWith({ + values: { + repository: 'https://github.com/ora/cloud', + prompt: 'Review the diff', + notes: 'retry notes', + }, + }) + }) + + it('shows interaction creation failures and unlocks the form for a retry', async () => { + serveComposer({ descriptor: platformDescriptor }) + server.use( + http.post('/api/v1/tenants/t1/issues/i1/comments', () => + HttpResponse.json({ code: 'unavailable' }, { status: 503 }), + ), + ) + const user = userEvent.setup() + renderComposer() + await screen.findByLabelText(/仓库地址/) + await reviewAndConfirm(user) + + expect(await screen.findByText('确认失败,请重试。')).toBeInTheDocument() + expect(screen.getByRole('button', { name: '确认执行' })).toBeEnabled() + expect(screen.getByRole('button', { name: '返回修改' })).toBeEnabled() + }) + it('carries the ticked context references, once each even when Assist suggested one too', async () => { const { confirm } = serveComposer({ descriptor: launchDescriptor, @@ -426,4 +553,150 @@ describe('WorkflowInteractionComposer', () => { }), ) }) + + it('renders the selected snapshot defaults and retains its echo instead of the newest default', async () => { + const { confirm } = serveComposer({ descriptor: launchDescriptor }) + const read = serveOldSnapshot() + const user = userEvent.setup() + renderComposer() + await user.type(await screen.findByLabelText(/提示词/), ' obsolete draft') + await user.click(selectTrigger('运行版本')) + await user.click(await screen.findByRole('option', { name: 'v2 · 发布 2' })) + + expect(await screen.findByLabelText(/Old input/)).toHaveValue('old default') + expect(screen.queryByLabelText(/提示词/)).not.toBeInTheDocument() + expect(selectTrigger('运行版本')).toHaveTextContent('v2 · 发布 2') + expect(read).toHaveBeenCalledWith('i1') + await reviewAndConfirm(user) + await waitFor(() => + expect(confirm).toHaveBeenCalledWith({ + values: { + repository: 'https://github.com/ora/cloud', + branch: 'main', + snapshotId: 'snap-2', + oldInput: 'old default', + }, + }), + ) + }) + + it('blocks stale review while a selected snapshot loads and after a read failure', async () => { + serveComposer({ descriptor: launchDescriptor }) + let release = vi.fn<() => void>() + const gate = new Promise((resolve) => { + release = vi.fn<() => void>(resolve) + }) + server.use( + http.get('/api/v1/tenants/t1/workflows/wf1/snapshots/snap-2/form', async () => { + await gate + return HttpResponse.json({ code: 'not_found' }, { status: 404 }) + }), + ) + const user = userEvent.setup() + renderComposer() + await user.click(await screen.findByLabelText(/运行版本/)) + await user.click(await screen.findByRole('option', { name: 'v2 · 发布 2' })) + try { + expect(screen.queryByLabelText(/提示词/)).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: '审阅' })).not.toBeInTheDocument() + } finally { + release() + } + expect(await screen.findByText(/当前部署无法加载/)).toBeInTheDocument() + expect(screen.queryByRole('button', { name: '审阅' })).not.toBeInTheDocument() + const read = serveOldSnapshot() + await user.click(screen.getByRole('button', { name: '重试加载表单' })) + expect(await screen.findByLabelText(/Old input/)).toHaveValue('old default') + expect(read).toHaveBeenCalledTimes(1) + }) + + it('keeps an authors text variable named snapshotId as ordinary input', async () => { + const { confirm } = serveComposer({ + descriptor: { + formRef: 'security-review', + fields: [ + { key: 'snapshotId', label: 'Author snapshot name', type: 'text', required: true }, + ], + }, + }) + const user = userEvent.setup() + renderComposer() + await user.type(await screen.findByLabelText(/Author snapshot name/), 'my snapshot') + await reviewAndConfirm(user) + await waitFor(() => + expect(confirm).toHaveBeenCalledWith({ values: { snapshotId: 'my snapshot' } }), + ) + }) + + it('keeps an authors select named snapshotId as ordinary input when any option is not published', async () => { + const { confirm } = serveComposer({ + descriptor: { + formRef: 'security-review', + fields: [ + { + key: 'snapshotId', + label: 'Author snapshot choice', + type: 'select', + required: true, + defaultValue: 'snap-3', + options: [ + { value: 'snap-3', label: 'Author default' }, + { value: 'custom-choice', label: 'Author custom choice' }, + ], + }, + ], + }, + }) + const formRead = vi.fn<() => void>() + server.use( + http.get('/api/v1/tenants/t1/workflows/wf1/snapshots/:id/form', () => { + formRead() + return HttpResponse.json({ code: 'not_found' }, { status: 404 }) + }), + ) + const user = userEvent.setup() + renderComposer() + await user.click(await screen.findByLabelText(/Author snapshot choice/)) + await user.click(await screen.findByRole('option', { name: 'Author custom choice' })) + expect(selectTrigger('Author snapshot choice')).toHaveTextContent('Author custom choice') + await reviewAndConfirm(user) + await waitFor(() => + expect(confirm).toHaveBeenCalledWith({ values: { snapshotId: 'custom-choice' } }), + ) + expect(formRead).not.toHaveBeenCalled() + }) + + it('explains how to resolve workflow version field divergence after confirm', async () => { + serveComposer({ descriptor: launchDescriptor }) + const { onConfirmed } = await confirmIntoFieldsChanged() + expect(await screen.findByRole('alert')).toHaveTextContent( + '所选版本的字段与当前草稿不一致,请先发布当前草稿或选择其他版本。', + ) + expect(screen.queryByText('workflow_version_fields_changed')).not.toBeInTheDocument() + expect(screen.getByRole('button', { name: '返回修改' })).toBeEnabled() + expect(onConfirmed).not.toHaveBeenCalled() + }) + + it('keeps the same interaction when a rejected confirm is retried after switching snapshot', async () => { + const { comment, confirm } = serveComposer({ descriptor: launchDescriptor }) + serveOldSnapshot() + const { user, onConfirmed } = await confirmIntoFieldsChanged() + await screen.findByText('所选版本的字段与当前草稿不一致,请先发布当前草稿或选择其他版本。') + await user.click(screen.getByRole('button', { name: '返回修改' })) + await user.click(selectTrigger('运行版本')) + await user.click(await screen.findByRole('option', { name: 'v2 · 发布 2' })) + await screen.findByLabelText(/Old input/) + server.use( + http.post('/api/v1/tenants/t1/issues/i1/interactions/ix1/confirm', async ({ request }) => { + confirm(await request.json()) + return HttpResponse.json({ resource: run() }) + }), + ) + await reviewAndConfirm(user) + await waitFor(() => expect(onConfirmed).toHaveBeenCalledTimes(1)) + expect(comment).toHaveBeenCalledTimes(1) + expect(confirm).toHaveBeenCalledWith( + expect.objectContaining({ values: expect.objectContaining({ snapshotId: 'snap-2' }) }), + ) + }) }) diff --git a/frontend/src/features/issues/types.ts b/frontend/src/features/issues/types.ts index b5c3ea452..bc92019be 100644 --- a/frontend/src/features/issues/types.ts +++ b/frontend/src/features/issues/types.ts @@ -88,6 +88,8 @@ export interface IssueRun { issueId: string executorType: 'agent' | 'team' | 'workflow' executorId: string + /** Workflow runs: the linked WorkflowRun id, whose status is authoritative for execution. */ + workflowInvocationRef?: string | null /** Comment identity behind an Agent Task Mode run; absent for direct/manual runs. */ triggerEvidenceRefId?: string | null input: Record diff --git a/integration/workflow_admission_test.go b/integration/workflow_admission_test.go index b01ab60cc..73b4314e3 100644 --- a/integration/workflow_admission_test.go +++ b/integration/workflow_admission_test.go @@ -384,7 +384,7 @@ func manualWorkflowRun(t *testing.T, f *fixture, iid, executorID, key string, in // be used to start a run the other two entries would refuse. func TestManualWorkflowRunResolvesItsExecutor(t *testing.T) { f := setup(t) - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Manual workflow run"}, "adm-manual-issue", 200).O("resource") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Manual workflow run", "projectRef": runProject(t, f, "adm-manual-project")}, "adm-manual-issue", 200).O("resource") iid := iss.S("id") // An id that names no workflow of this tenant, published or not. diff --git a/integration/workflow_asset_invocation_test.go b/integration/workflow_asset_invocation_test.go new file mode 100644 index 000000000..814cadaf4 --- /dev/null +++ b/integration/workflow_asset_invocation_test.go @@ -0,0 +1,218 @@ +package integration + +import ( + "encoding/json" + "reflect" + "testing" + + "github.com/wanglongan587/cloud/internal/core" + "github.com/wanglongan587/cloud/internal/workflowasset" +) + +type importedWorkflowEvidence struct { + f *fixture + spaceID string + project string + workflow string + snapshot core.Object +} + +// importWorkflowForInvocation reuses the real .orax/HTTPS/marketplace harness. No workflow or +// snapshot is seeded: the plugin's desired state must produce both through the import worker. +func importWorkflowForInvocation(t *testing.T) *importedWorkflowEvidence { + t.Helper() + f := setup(t) + // These tests prove persisted invocation identity, never a simulated node trace. + f.store.Simulator = nil + document := startPromptGraph([]any{core.Object{ + "name": "releaseName", "displayName": "Imported release", "fieldType": "text-input", + "valueType": "string", "required": true, "value": "package-default", + }}, "Ship the imported release") + document["name"] = "Imported Release Evidence" + document["version"] = 47 + document["globalVariables"] = []any{core.Object{"name": "release.mode", "valueType": "string", "value": "safe"}} + encoded, err := json.Marshal(document) + must(t, err) + pkg := workflowPackage(t, map[string]string{"release.json": string(encoded)}) + packages := newWorkflowPackages(t, map[string][]byte{"/packages/invocation-1.0.0.orax": pkg}) + repo := workflowMarketplace(t, f.root) + publishWorkflowRelease(t, repo, "invocation-kit", "1.0.0", packages.server.URL+"/packages/invocation-1.0.0.orax", pkg) + f.syncMarketplace(t, repo) + f.store.WorkflowAssets = workflowasset.NewFetcher(nil, packages.server.Client().Transport) + sid := f.defaultSpaceID() + f.installWorkflowPlugin(t, sid, "official/invocation-kit", "") + must(t, f.importWorkflowAssets(t)) + item := f.assetImport(t, sid, "official/invocation-kit") + if item == nil || item.S("state") != "completed" { + t.Fatalf("import did not complete: %v", item) + } + imported := assetDocuments(item)["assets/workflows/release.json"] + if imported.S("outcome") != "imported" || imported.S("workflowId") == "" || imported.N("snapshotVersion") != 1 { + t.Fatalf("document was not imported and published: %v", imported) + } + wid := imported.S("workflowId") + snapshots := issueItems(f.call("GET", f.path("/workflows/"+wid+"/snapshots"), nil, "", 200)) + if len(snapshots) != 1 || snapshots[0].S("id") == "" || snapshots[0].N("version") != 1 { + t.Fatalf("import must leave exactly one Cloud-assigned snapshot: %v", snapshots) + } + var expected core.Object + must(t, json.Unmarshal(encoded, &expected)) + if !reflect.DeepEqual(snapshots[0].O("graph"), expected) { + t.Fatalf("published graph differs from the package document: %v", snapshots[0]) + } + return &importedWorkflowEvidence{f: f, spaceID: sid, project: runProject(t, f, "asset-inv-project"), workflow: wid, snapshot: snapshots[0]} +} + +func importedInvocationInput() core.Object { + return core.Object{"releaseName": "release-from-package", "release.mode": "strict"} +} + +// assertRun checks both the public frozen definition and the real PostgreSQL binding. +// IDs and timestamps differ by invocation; the returned projection names only run semantics. +func (e *importedWorkflowEvidence) assertRun(t *testing.T, user core.Claims, id string, snapshot core.Object) core.Object { + t.Helper() + run := callAs(t, e.f, user, "GET", e.f.path("/workflows/"+e.workflow+"/runs/"+id), "", nil, 200) + if run.S("snapshotId") != snapshot.S("id") || run.S("workflowId") != e.workflow || run.S("status") != "pending" { + t.Fatalf("run did not bind a pending imported invocation: %v", run) + } + if !reflect.DeepEqual(run.O("definitionSnapshot"), snapshot.O("graph")) || !reflect.DeepEqual(run.O("input"), importedInvocationInput()) { + t.Fatalf("run did not freeze the selected graph and input: %v", run) + } + if n := e.f.scalar("SELECT count(*) FROM workflow_runs WHERE id=$1 AND tenant_id=$2 AND workflow_id=$3 AND snapshot_id=$4", id, e.f.tid, e.workflow, snapshot.S("id")); n != 1 { + t.Fatalf("stored WorkflowRun binding = %d rows, want 1", n) + } + return core.Object{ + "workflowId": run.S("workflowId"), "snapshotId": run.S("snapshotId"), + "status": run.S("status"), "input": run.O("input"), "definitionSnapshot": run.O("definitionSnapshot"), + } +} + +func TestImportedWorkflowDirectoryAndRunsIgnoreImportTriggerUser(t *testing.T) { + e := importWorkflowForInvocation(t) + f := e.f + trigger := f.user + other, otherID := f.addUser(t, "asset-evidence-admin", "Other asset administrator") + f.call("PUT", f.path("/members/"+otherID), core.Object{"role": "admin", "status": "active", "version": 1}, "", 200) + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Discover imported workflow", "projectRef": e.project}, "asset-directory-issue", 200).O("resource").S("id") + formPath := f.path("/workflows/" + e.workflow + "/snapshots/" + e.snapshot.S("id") + "/form?issueId=" + iid) + initialForm := f.call("GET", formPath, nil, "", 200) + liveFormPath := f.path("/collaboration/forms/" + e.workflow + "?issueId=" + iid) + if live := f.call("GET", liveFormPath, nil, "", 200); !reflect.DeepEqual(live["fields"], initialForm["fields"]) { + t.Fatalf("the imported live copy and initial published form disagree: live=%v snapshot=%v", live, initialForm) + } + // Distinguish the selected snapshot's form from the mutable live form. This is the same + // snapshot-form endpoint the @ composer reads when the user selects a published version. + live := f.call("GET", f.path("/workflows/"+e.workflow), nil, "", 200) + f.call("PUT", f.path("/workflows/"+e.workflow), core.Object{ + "version": live.N("version"), "graph": startPromptGraph([]any{invocationVariable("replacement")}, "Draft-only prompt"), + }, "", 200) + changed := formFields(t, f.call("GET", liveFormPath, nil, "", 200)) + if changed["replacement"] == nil || changed["releaseName"] != nil { + t.Fatalf("test did not give the live draft a different form: %v", changed) + } + project := func(user core.Claims) core.Object { + t.Helper() + // This is the public directory used by useCollaborationTargets in the Issue @ picker. + directory := callAs(t, f, user, "GET", f.path("/collaboration/targets"), "", nil, 200) + var target core.Object + for _, item := range issueItems(directory) { + if item.S("type") == "workflow" && item.S("id") == e.workflow { + target = item + } + } + if target == nil || target.S("displayName") != "Imported Release Evidence" { + t.Fatalf("imported copy is absent from the @ directory: %v", directory) + } + interaction := target.O("interactionDescriptor") + if interaction.S("mode") != "form" || interaction.B("requiresTask") || interaction.S("formRef") != e.workflow { + t.Fatalf("imported target does not advertise its workflow form: %v", target) + } + form := callAs(t, f, user, "GET", formPath, "", nil, 200) + fields := formFields(t, form) + if form.S("snapshotId") != e.snapshot.S("id") || form.S("formRef") != interaction.S("formRef") || + fields["releaseName"].S("label") != "Imported release" || fields["releaseName"].S("defaultValue") != "package-default" || + !fields["releaseName"].B("required") || fields["replacement"] != nil || + fields["prompt"].S("defaultValue") != "Ship the imported release" || fields["release.mode"].S("defaultValue") != "safe" { + t.Fatalf("selected form was not projected from the imported snapshot: %v", form) + } + if !reflect.DeepEqual(form, initialForm) { + t.Fatalf("editing the live copy changed its published form: initial=%v selected=%v", initialForm, form) + } + workflow := callAs(t, f, user, "GET", f.path("/workflows/"+e.workflow), "", nil, 200) + return core.Object{"target": target, "form": form, "workflow": workflow} + } + start := func(user core.Claims, key string) core.Object { + t.Helper() + run := callAs(t, f, user, "POST", f.path("/workflows/"+e.workflow+"/runs"), key, core.Object{ + "snapshotId": e.snapshot.S("id"), "input": importedInvocationInput(), "projectId": e.project, + }, 200).O("resource") + return e.assertRun(t, user, run.S("id"), e.snapshot) + } + baseline := project(trigger) + baselineRun := start(trigger, "asset-author-trigger-run") + if otherProjection, otherRun := project(other), start(other, "asset-author-other-run"); !reflect.DeepEqual(otherProjection, baseline) || !reflect.DeepEqual(otherRun, baselineRun) { + t.Fatalf("changing administrator changed imported projection/run: projection=%v run=%v", otherProjection, otherRun) + } + // Global account disable has no public admin endpoint; change the actual authorization row, + // then prove the old identity is refused over HTTP before comparing the surviving admin. + _, err := f.store.Pool.Exec("UPDATE users SET status='disabled' WHERE id=$1", f.uid) + must(t, err) + callAs(t, f, trigger, "GET", f.path("/collaboration/targets"), "", nil, 403) + if afterProjection, afterRun := project(other), start(other, "asset-author-after-disable-run"); !reflect.DeepEqual(afterProjection, baseline) || !reflect.DeepEqual(afterRun, baselineRun) { + t.Fatalf("deactivating the import trigger changed the retained copy: projection=%v run=%v", afterProjection, afterRun) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE workflow_id=$1 AND snapshot_id=$2", e.workflow, e.snapshot.S("id")); n != 3 { + t.Fatalf("expected the same snapshot for all three administrators/account states, got %d", n) + } +} + +func TestImportedWorkflowSnapshotBindsConfirmAndDirectRuns(t *testing.T) { + e := importWorkflowForInvocation(t) + f, wid, snapshot := e.f, e.workflow, e.snapshot + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Use imported snapshot", "projectRef": e.project}, "asset-inv-issue", 200).O("resource").S("id") + interaction := workflowComment(t, f, wid, iid, "asset-inv-comment", "@ imported release") + values := importedInvocationInput() + values["repository"], values["branch"], values["snapshotId"] = "https://example.invalid/repo.git", "main", snapshot.S("id") + issueRun := f.call("POST", confirmPath(f, iid, interaction.S("id")), core.Object{"values": values}, "asset-inv-confirm", 200).O("resource") + ref := issueRun.S("workflowInvocationRef") + if ref == "" || issueRun.O("input").S("workflowSnapshotId") != snapshot.S("id") { + t.Fatalf("Confirm did not link the imported snapshot: %v", issueRun) + } + e.assertRun(t, f.user, ref, snapshot) + if n := f.scalar(`SELECT count(*) FROM issue_interactions ix + JOIN issue_runs ir ON ir.id=ix.run_id + JOIN workflow_runs wr ON wr.id=ir.workflow_invocation_ref AND wr.tenant_id=ir.tenant_id + WHERE ix.id=$1 AND ir.id=$2 AND wr.id=$3 AND wr.snapshot_id=$4`, interaction.S("id"), issueRun.S("id"), ref, snapshot.S("id")); n != 1 { + t.Fatalf("imported Confirm did not persist the interaction/run pair: %d", n) + } + create := func(key string) string { + t.Helper() + return f.call("POST", f.path("/workflows/"+wid+"/runs"), core.Object{ + "snapshotId": snapshot.S("id"), "input": importedInvocationInput(), "projectId": e.project, + }, key, 200).O("resource").S("id") + } + first := create("asset-inv-direct") + e.assertRun(t, f.user, first, snapshot) + var frozen string + must(t, f.store.Pool.QueryRow("SELECT graph::text FROM workflow_snapshots WHERE id=$1", snapshot.S("id")).Scan(&frozen)) + live := f.call("GET", f.path("/workflows/"+wid), nil, "", 200) + draft := startPromptGraph([]any{invocationVariable("replacement")}, "Changed draft prompt") + f.call("PUT", f.path("/workflows/"+wid), core.Object{"graph": draft, "version": live.N("version")}, "", 200) + // Direct admission judges the selected snapshot even when the draft has a different schema. + // Confirm's separate live-form compatibility gate is deliberately not bypassed here. + second := create("asset-inv-after-edit") + if first == second || second == ref || first == ref { + t.Fatal("distinct starts must create distinct WorkflowRun records") + } + e.assertRun(t, f.user, second, snapshot) + e.assertRun(t, f.user, first, snapshot) + e.assertRun(t, f.user, ref, snapshot) + var after string + must(t, f.store.Pool.QueryRow("SELECT graph::text FROM workflow_snapshots WHERE id=$1", snapshot.S("id")).Scan(&after)) + if after != frozen { + t.Fatal("editing the live draft rewrote the imported snapshot") + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE workflow_id=$1 AND snapshot_id=$2", wid, snapshot.S("id")); n != 3 { + t.Fatalf("expected Confirm and two direct starts bound to the imported snapshot, got %d", n) + } +} diff --git a/integration/workflow_asset_retained_test.go b/integration/workflow_asset_retained_test.go new file mode 100644 index 000000000..a2d98d5f9 --- /dev/null +++ b/integration/workflow_asset_retained_test.go @@ -0,0 +1,58 @@ +package integration + +import ( + "testing" + + "github.com/wanglongan587/cloud/internal/core" +) + +// TestImportedWorkflowRemainsPublishableAndRunnableAfterUninstall proves an imported document is +// retained user data: removing its plugin neither removes its history nor prevents a new invocation. +func TestImportedWorkflowRemainsPublishableAndRunnableAfterUninstall(t *testing.T) { + e := importWorkflowForInvocation(t) + f, wid, old := e.f, e.workflow, e.snapshot + readOldSnapshot := func() (string, string) { + t.Helper() + var graph, row string + must(t, f.store.Pool.QueryRow(`SELECT graph::text, row_to_json(s)::text FROM workflow_snapshots s + WHERE id=$1 AND tenant_id=$2 AND workflow_id=$3`, old.S("id"), f.tid, wid).Scan(&graph, &row)) + return graph, row + } + frozenGraph, frozenRow := readOldSnapshot() + plugin := f.spacePlugin(e.spaceID, "official/invocation-kit") + if plugin == nil { + t.Fatal("imported workflow has no installed plugin selection to remove") + } + f.call("DELETE", f.pluginSpacePath(e.spaceID)+"/plugins", core.Object{ + "identifier": "official/invocation-kit", "version": plugin.N("version"), + }, "asset-retained-uninstall", 200) + if removed := f.spacePlugin(e.spaceID, "official/invocation-kit"); removed == nil || removed.S("desiredState") != "removed" { + t.Fatalf("plugin uninstall did not persist its removed intent: %v", removed) + } + if retained := f.call("GET", f.path("/workflows/"+wid), nil, "", 200); retained.S("id") != wid { + t.Fatalf("uninstall did not retain the imported workflow: %v", retained) + } + if graph, row := readOldSnapshot(); graph != frozenGraph || row != frozenRow { + t.Fatal("uninstall changed the imported snapshot row or graph") + } + + newSnapshot := publishWorkflow(t, f, wid, "asset-retained-publish") + if newSnapshot.S("id") == "" || newSnapshot.S("id") == old.S("id") || newSnapshot.N("version") != old.N("version")+1 { + t.Fatalf("retained workflow did not publish its next distinct snapshot: old=%v new=%v", old, newSnapshot) + } + run := f.call("POST", f.path("/workflows/"+wid+"/runs"), core.Object{ + "snapshotId": newSnapshot.S("id"), "input": importedInvocationInput(), "projectId": e.project, + }, "asset-retained-run", 200).O("resource") + e.assertRun(t, f.user, run.S("id"), newSnapshot) + if graph, row := readOldSnapshot(); graph != frozenGraph || row != frozenRow { + t.Fatal("publishing or running the retained workflow changed its old snapshot") + } + snapshots := issueItems(f.call("GET", f.path("/workflows/"+wid+"/snapshots"), nil, "", 200)) + seen := map[string]int64{} + for _, snapshot := range snapshots { + seen[snapshot.S("id")] = snapshot.N("version") + } + if len(snapshots) != 2 || seen[old.S("id")] != old.N("version") || seen[newSnapshot.S("id")] != newSnapshot.N("version") { + t.Fatalf("retained workflow lost its old or new snapshot: %v", snapshots) + } +} diff --git a/integration/workflow_interaction_test.go b/integration/workflow_interaction_test.go index 7a1f94dfe..ff3db3dc6 100644 --- a/integration/workflow_interaction_test.go +++ b/integration/workflow_interaction_test.go @@ -726,7 +726,7 @@ func TestAuthorDeclaredSnapshotIDIsNotTheVersionCursor(t *testing.T) { core.Object{"name": "scope", "displayName": "Scope", "fieldType": "text-input", "valueType": "string", "required": true}, }) published := publishWorkflow(t, f, wid, "wf-version-owned-pub") - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Release it"}, "wf-version-issue", 200).O("resource") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Release it", "projectRef": runProject(t, f, "wf-version-project")}, "wf-version-issue", 200).O("resource") // The form the author's declaration produces: their own control under that key, holding nothing — // and no version selector, which is what makes the key theirs to begin with. @@ -776,7 +776,8 @@ func TestWorkflowConfirmReadsThePreRenameVersionKey(t *testing.T) { if newest.S("id") == older.S("id") { t.Fatal("the two publishes are the same snapshot, so a wrong version could pass as the right one") } - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Run the old form"}, "wf-rename-issue", 200).O("resource") + pid := runProject(t, f, "wf-rename-project") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Run the old form", "projectRef": pid}, "wf-rename-issue", 200).O("resource") // The submission a client that rendered the pre-rename form sends: the selection under the old key, // and no `snapshotId` at all. @@ -830,7 +831,7 @@ func TestWorkflowConfirmReadsThePreRenameVersionKey(t *testing.T) { } // A second issue, because a confirmed interaction is spent: `workflowComment` reads back the issue's // form interaction and would hand back the one just confirmed. - ownedIss := f.call("POST", f.path("/issues"), core.Object{"title": "Run the old form again"}, "wf-rename-issue-2", 200).O("resource") + ownedIss := f.call("POST", f.path("/issues"), core.Object{"title": "Run the old form again", "projectRef": pid}, "wf-rename-issue-2", 200).O("resource") it = workflowComment(t, f, owned, ownedIss.S("id"), "wf-rename-owned-1", "run it") created = f.call("POST", confirmPath(f, ownedIss.S("id"), it.S("id")), core.Object{"values": core.Object{ "repository": "ora-space/cloud", "branch": "main", "version": ownedOlder.S("id"), "snapshotId": "1.2.3", @@ -843,6 +844,10 @@ func TestWorkflowConfirmReadsThePreRenameVersionKey(t *testing.T) { if stored.S("snapshotId") != "1.2.3" { t.Fatalf("the author's variable was overwritten by the version selection: %v", stored) } + linked := f.call("GET", f.path("/workflows/"+owned+"/runs/"+run.S("workflowInvocationRef")), nil, "", 200) + if linked.O("input").S("snapshotId") != "1.2.3" || linked.S("snapshotId") != ownedOlder.S("id") { + t.Fatalf("the invocation confused the author's variable with the selected snapshot: %v", linked) + } if _, kept := stored["version"]; kept { t.Fatalf("the pre-rename key was stored: %v", stored) } @@ -868,7 +873,7 @@ func TestWorkflowAssistReadsThePreRenameVersionKey(t *testing.T) { if newest.S("id") == older.S("id") { t.Fatal("the two publishes are the same snapshot, so the test cannot tell which one was read") } - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Fill the old form"}, "wf-assist-rename-issue", 200).O("resource") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Fill the old form", "projectRef": runProject(t, f, "wf-assist-rename-project")}, "wf-assist-rename-issue", 200).O("resource") // The submission a client that rendered the pre-rename form sends — the selection under the old key // and no `snapshotId` at all — driven through both entries in turn. Assist comes first so the 200 @@ -961,9 +966,10 @@ func TestWorkflowConfirmRefusesAVersionTheLiveFormHasLeft(t *testing.T) { "graph": startInputGraph(variables), "version": live.N("version"), }, "", 200) } + pid := runProject(t, f, "wf-diverge-project") newIssue := func(key string) string { t.Helper() - return f.call("POST", f.path("/issues"), core.Object{"title": "Diverging form"}, key, 200).O("resource").S("id") + return f.call("POST", f.path("/issues"), core.Object{"title": "Diverging form", "projectRef": pid}, key, 200).O("resource").S("id") } rendered := func(iid string) []string { t.Helper() @@ -1049,7 +1055,7 @@ func TestFormDescriptorUnavailableAndInvalid(t *testing.T) { f := setup(t) wid := reviewWorkflow(t, f) publishWorkflow(t, f, wid, "wf-prov-pub") - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Provider states"}, "wf-prov", 200).O("resource") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Provider states", "projectRef": runProject(t, f, "wf-prov-project")}, "wf-prov", 200).O("resource") it := workflowComment(t, f, wid, iss.S("id"), "wf-prov-1", "configure") original := f.store.Forms @@ -1134,8 +1140,8 @@ func TestWorkflowAssistIsSideEffectFree(t *testing.T) { func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { f := setup(t) wid := reviewWorkflow(t, f) - publishWorkflow(t, f, wid, "wf-confirm-pub") - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Confirm me", "description": "desc"}, "wf-confirm-issue", 200).O("resource") + published := publishWorkflow(t, f, wid, "wf-confirm-pub") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Confirm me", "description": "desc", "projectRef": runProject(t, f, "wf-confirm-project")}, "wf-confirm-issue", 200).O("resource") it := workflowComment(t, f, wid, iss.S("id"), "wf-confirm-1", "configure the review") // Idempotency-Key is mandatory for POST, like every other mutation. @@ -1163,8 +1169,7 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { } created := f.call("POST", confirmPath(f, iss.S("id"), it.S("id")), core.Object{"values": validValues()}, "wf-confirm-ok", 200).O("resource") - // The confirm response is the committed enqueue: dispatch is post-commit, so execution has not been - // observed yet. The frontend re-reads the run/timeline afterwards. + // Confirm commits the linked execution intent; the workflow scheduler owns later advancement. if created.S("status") != "queued" || created.S("executorType") != "workflow" || created.S("executorId") != wid { t.Fatalf("workflow run wrong at confirm time: %v", created) } @@ -1172,10 +1177,18 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { if created.S("triggerEvidenceKind") != "interaction" || created.S("triggerEvidenceRefId") != it.S("id") { t.Fatalf("workflow run provenance wrong: %v", created) } - // Re-read: the mock dispatcher drove it to completion through the observer seam. + // The WorkflowRun owns execution. The legacy mock dispatcher must not fabricate a separate + // execution or advance this IssueRun through its unrelated observer lifecycle. run := f.call("GET", f.path("/issues/"+iss.S("id")+"/runs/"+created.S("id")), nil, "", 200) - if run.S("status") != "completed" || run.S("externalExecutionId") == "" { - t.Fatalf("workflow run did not complete through the observer: %v", run) + if run.S("status") != "queued" || run.S("workflowInvocationRef") == "" || run.S("externalExecutionId") != "" { + t.Fatalf("workflow run was not linked or still used legacy dispatch: %v", run) + } + linked := f.call("GET", f.path("/workflows/"+wid+"/runs/"+run.S("workflowInvocationRef")), nil, "", 200) + if linked.S("snapshotId") != published.S("id") || run.O("input").S("workflowSnapshotId") != linked.S("snapshotId") { + t.Fatalf("linked workflow run does not execute the frozen snapshot: issue=%v workflow=%v", run, linked) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE id=$1 AND tenant_id=$2", run.S("workflowInvocationRef"), f.tid); n != 1 { + t.Fatalf("confirm created %d linked WorkflowRuns", n) } // Exactly one run, and the interaction is now confirmed with its own persisted input. @@ -1193,7 +1206,7 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { t.Fatalf("the descriptor must never be stored as interaction input") } - // The run snapshot is the effective execution-time input, and it equals what the dispatcher got. + // The IssueRun snapshot preserves the effective invocation input and its issue context. input := run.O("input") if input.S("task") != "" || input.O("interactionValues").S("repository") != "ora-space/cloud" { t.Fatalf("run snapshot wrong: %v", input) @@ -1205,7 +1218,7 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { t.Fatalf("run snapshot lost the recent-comment window: %v", input) } - // Timeline: comment, enqueued, started, progress, system message, completed — and NO workflow-authored comment. + // Without scheduler advancement, only enqueue is visible; legacy dispatch adds no fake execution. tl := issueItems(f.call("GET", f.path("/issues/"+iss.S("id")+"/timeline"), nil, "", 200)) actions := []string{} for _, e := range tl { @@ -1213,7 +1226,7 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { actions = append(actions, e.S("action")) } } - want := []string{"run.enqueued", "run.started", "run.progress", "run.message", "run.completed"} + want := []string{"run.enqueued"} if len(actions) != len(want) { t.Fatalf("workflow timeline actions = %v, want %v", actions, want) } @@ -1227,11 +1240,6 @@ func TestWorkflowConfirmValidatesAndExecutes(t *testing.T) { t.Fatalf("a workflow must never author content: %v", e) } } - message := tl[4] - if message.S("authorType") != "system" || message.O("details").S("executorType") != "workflow" || - message.O("details").S("executorId") != wid || message.O("details").S("message") == "" { - t.Fatalf("workflow message activity wrong: %v", message) - } // seq is strictly increasing and unique across the merged timeline. for i := 1; i < len(tl); i++ { if tl[i].N("seq") <= tl[i-1].N("seq") { @@ -1246,7 +1254,7 @@ func TestWorkflowConfirmIdempotencyAndDoubleConfirm(t *testing.T) { f := setup(t) wid := reviewWorkflow(t, f) publishWorkflow(t, f, wid, "wf-idem-pub") - iss := f.call("POST", f.path("/issues"), core.Object{"title": "Idempotent"}, "wf-idem-issue", 200).O("resource") + iss := f.call("POST", f.path("/issues"), core.Object{"title": "Idempotent", "projectRef": runProject(t, f, "wf-idem-project")}, "wf-idem-issue", 200).O("resource") it := workflowComment(t, f, wid, iss.S("id"), "wf-idem-1", "configure") first := f.call("POST", confirmPath(f, iss.S("id"), it.S("id")), core.Object{"values": validValues()}, "wf-idem-confirm", 200).O("resource") diff --git a/integration/workflow_invocation_identity_test.go b/integration/workflow_invocation_identity_test.go new file mode 100644 index 000000000..af24a4c81 --- /dev/null +++ b/integration/workflow_invocation_identity_test.go @@ -0,0 +1,120 @@ +package integration + +import ( + "testing" + + "github.com/wanglongan587/cloud/internal/core" +) + +// Issue invocations freeze the confirming actor, not the Issue/workflow author. The default fixture +// simulator deliberately remains installed: neither entry may use it to skip executor freezing or +// fabricate a completed WorkflowRun. +func TestWorkflowInvocationFreezesVerifiedIdentity(t *testing.T) { + for _, entry := range []string{"confirm", "manual"} { + t.Run(entry, func(t *testing.T) { + f := setup(t) + if f.store.Simulator == nil { + t.Fatal("this scenario requires the direct-run simulator to remain installed") + } + pid := runProject(t, f, "inv-identity-project") + actor, actorID := f.addUser(t, "inv-actor", "Invocation actor") + author := f.user + f.user = actor + connection := seedPersonalModel(t, f) + f.call("PUT", "/api/v1/me/executor-slot-mappings/strong", core.Object{ + "agentCli": "official/ora-space.opencode", "connectionId": connection.S("id"), + "modelId": fixtureModelID, "version": 0, + }, "", 200) + f.user = author + graph := agentRunGraph("strong", false, []any{core.Object{"name": "strong", "description": "Review"}}) + wid := f.call("POST", f.path("/workflows"), core.Object{ + "name": "Identity workflow", "graph": graph, + }, "inv-identity-wf", 200).O("resource").S("id") + publishWorkflow(t, f, wid, "inv-identity-publish") + _, err := f.store.Pool.Exec(`INSERT INTO space_plugins(space_id,tenant_id,source_namespace,identifier,desired_state,desired_version,observed_state) + VALUES($1,$2,'official','ora-space.opencode','installed','1.7.0','pending')`, f.defaultSpaceID(), f.tid) + must(t, err) + iid := f.call("POST", f.path("/issues"), core.Object{ + "title": "Another member starts this", "projectRef": pid, + }, "inv-identity-issue", 200).O("resource").S("id") + var run core.Object + if entry == "confirm" { + it := workflowComment(t, f, wid, iid, "inv-identity-comment", "review") + run = callAs(t, f, actor, "POST", confirmPath(f, iid, it.S("id")), "inv-identity-confirm", core.Object{ + "values": core.Object{"repository": "ora-space/cloud", "branch": "main"}, + }, 200).O("resource") + } else { + run = callAs(t, f, actor, "POST", f.path("/issues/"+iid+"/runs"), "inv-identity-manual", core.Object{ + "executorType": "workflow", "executorId": wid, "input": core.Object{}, + }, 200).O("resource") + } + rid := run.S("workflowInvocationRef") + if rid == "" || run.S("status") != "queued" { + t.Fatalf("invocation did not return a queued linked IssueRun: %v", run) + } + assertFrozen := func() { + t.Helper() + var creator, project, plugin, status string + must(t, f.store.Pool.QueryRow(`SELECT created_by,project_id,plugin_version,status FROM workflow_runs WHERE id=$1`, rid). + Scan(&creator, &project, &plugin, &status)) + if creator != actorID || creator == f.uid || project != pid || plugin != "1.7.0" || status != "pending" { + t.Fatalf("frozen identity creator=%q project=%q plugin=%q status=%q; want verified actor, Issue project, 1.7.0, pending", creator, project, plugin, status) + } + if n := f.scalar("SELECT count(*) FROM run_model_bindings WHERE workflow_run_id=$1", rid); n != 1 { + t.Fatalf("invocation froze %d executor bindings, want exactly one", n) + } + var slot, userID, connectionID, credentialID, name, protocol, baseURL, authMode, modelID string + var connectionVersion int64 + must(t, f.store.Pool.QueryRow(`SELECT slot_name,user_id,connection_id,connection_version,credential_id,connection_name,protocol,base_url,auth_mode,model->>'id' + FROM run_model_bindings WHERE workflow_run_id=$1`, rid). + Scan(&slot, &userID, &connectionID, &connectionVersion, &credentialID, &name, &protocol, &baseURL, &authMode, &modelID)) + var expectedCredential string + must(t, f.store.Pool.QueryRow("SELECT credential_id FROM personal_model_connections WHERE id=$1", connection.S("id")).Scan(&expectedCredential)) + if slot != "strong" || userID != actorID || connectionID != connection.S("id") || connectionVersion != connection.N("version") || + credentialID != expectedCredential || name != "Personal" || protocol != "openai-completions" || + baseURL != "https://models.example.invalid/v1" || authMode != "bearer" || modelID != fixtureModelID { + t.Fatalf("frozen executor differs from actor's admitted configuration: slot=%q user=%q connection=%q version=%d name=%q protocol=%q baseURL=%q authMode=%q model=%q", slot, userID, connectionID, connectionVersion, name, protocol, baseURL, authMode, modelID) + } + if n := f.scalar(`SELECT count(*) FROM workflow_runs WHERE id=$1 AND started_at IS NULL AND finished_at IS NULL AND node_states='{}'::jsonb`, rid); n != 1 { + t.Fatal("Issue invocation fabricated a simulator execution trace") + } + } + assertFrozen() + // Later mutable configuration is not the invocation's execution identity. + _, err = f.store.Pool.Exec("UPDATE space_plugins SET desired_version='2.0.0' WHERE space_id=$1 AND identifier='ora-space.opencode'", f.defaultSpaceID()) + must(t, err) + _, err = f.store.Pool.Exec("UPDATE personal_model_connections SET name='Changed after creation',version=version+1 WHERE id=$1", connection.S("id")) + must(t, err) + assertFrozen() + }) + } +} + +func TestWorkflowInvocationWithoutIssueProjectRollsBack(t *testing.T) { + for _, entry := range []string{"confirm", "manual"} { + t.Run(entry, func(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "No Project workflow", "inv-no-project-wf", nil) + publishWorkflow(t, f, wid, "inv-no-project-publish") + iid := f.call("POST", f.path("/issues"), core.Object{"title": "No Project"}, "inv-no-project-issue", 200).O("resource").S("id") + it := workflowComment(t, f, wid, iid, "inv-no-project-comment", "run") + activities := f.scalar("SELECT count(*) FROM issue_activities WHERE issue_id=$1", iid) + var refusal core.Object + if entry == "confirm" { + refusal = f.call("POST", confirmPath(f, iid, it.S("id")), core.Object{ + "values": core.Object{"repository": "ora-space/cloud", "branch": "main"}, + }, "inv-no-project-confirm", 409) + } else { + status, body := manualWorkflowRun(t, f, iid, wid, "inv-no-project-manual", core.Object{}) + if status != 409 { + t.Fatalf("manual missing-project status=%d, want 409: %v", status, body) + } + refusal = body + } + if refusal.S("code") != "workflow_run_project_required" { + t.Fatalf("missing-project code=%q, want workflow_run_project_required", refusal.S("code")) + } + assertUnclaimedWorkflowInvocation(t, f, iid, it.S("id"), activities) + }) + } +} diff --git a/integration/workflow_invocation_migration_test.go b/integration/workflow_invocation_migration_test.go new file mode 100644 index 000000000..f5e93a49b --- /dev/null +++ b/integration/workflow_invocation_migration_test.go @@ -0,0 +1,171 @@ +package integration + +import ( + "database/sql" + "testing" + + "github.com/google/uuid" +) + +const workflowInvocationMigration = "0052_workflow_invocation_reference.sql" + +// seedInvocationWorkflowRun creates real rows for the composite reference under test. The graph is +// deliberately minimal: these tests exercise database integrity rather than graph admission. +func seedInvocationWorkflowRun(t *testing.T, pool *sql.DB, tenantID string) string { + t.Helper() + wid, sid, rid := uuid.NewString(), uuid.NewString(), uuid.NewString() + _, err := pool.Exec(`INSERT INTO workflows(id,tenant_id,name) VALUES($1,$2,$3)`, wid, tenantID, "Invocation "+wid) + must(t, err) + _, err = pool.Exec(`INSERT INTO workflow_snapshots(id,tenant_id,workflow_id,version,name,graph) VALUES($1,$2,$3,1,'Snapshot','{}')`, sid, tenantID, wid) + must(t, err) + _, err = pool.Exec(`INSERT INTO workflow_runs(id,tenant_id,workflow_id,snapshot_id,name) VALUES($1,$2,$3,$4,'Invocation')`, rid, tenantID, wid, sid) + must(t, err) + return rid +} + +func insertInvocationIssueRun(pool *sql.DB, seed skeletonSeed, executorType string, reference any) (string, error) { + id := uuid.NewString() + _, err := pool.Exec(`INSERT INTO issue_runs(id,tenant_id,issue_id,executor_type,executor_id,workflow_invocation_ref) + VALUES($1,$2,$3,$4,$5,$6)`, id, seed.tenantID, seed.issueID, executorType, uuid.NewString(), reference) + return id, err +} + +// assertInvocationConstraintValidation requires the same validated schema for fresh and upgraded data. +func assertInvocationConstraintValidation(t *testing.T, pool *sql.DB) { + t.Helper() + for _, constraint := range []struct { + table, name string + validated bool + }{ + {"issue_runs", "issue_runs_workflow_invocation_shape", true}, + {"issue_runs", "issue_runs_workflow_invocation_fk", true}, + {"issue_runs", "issue_runs_workflow_invocation_uniq", true}, + {"workflow_runs", "workflow_runs_id_tenant_uniq", true}, + } { + var validated bool + must(t, pool.QueryRow(`SELECT convalidated FROM pg_constraint WHERE conrelid=$1::regclass AND conname=$2`, constraint.table, constraint.name).Scan(&validated)) + if validated != constraint.validated { + t.Fatalf("%s validation = %v, want %v", constraint.name, validated, constraint.validated) + } + } +} + +func TestWorkflowInvocationMigrationFreshConstraints(t *testing.T) { + pool, _ := testSchema(t, "test_invocation_fresh_") + store := newStoreOnSchema(t, pool) + migratedTwice(t, store) + seed := seedOwnershipChain(t, pool) + assertInvocationConstraintValidation(t, pool) + + // A valid same-tenant reference succeeds and remains a durable one-to-one link. + ref := seedInvocationWorkflowRun(t, pool, seed.tenantID) + id, err := insertInvocationIssueRun(pool, seed, "workflow", ref) + must(t, err) + var linked string + must(t, pool.QueryRow(`SELECT workflow_invocation_ref FROM issue_runs WHERE id=$1`, id).Scan(&linked)) + if linked != ref { + t.Fatalf("valid reference = %q, want %q", linked, ref) + } + _, err = insertInvocationIssueRun(pool, seed, "workflow", nil) + wantPGError(t, err, "23514") + _, err = insertInvocationIssueRun(pool, seed, "workflow", uuid.NewString()) + wantPGError(t, err, "23503") + _, err = insertInvocationIssueRun(pool, seed, "workflow", ref) + wantPGError(t, err, "23505") + _, err = insertInvocationIssueRun(pool, seed, "agent", seedInvocationWorkflowRun(t, pool, seed.tenantID)) + wantPGError(t, err, "23514") + + // The referenced UUID exists, but belongs to another tenant. The composite FK must reject it. + foreignTenant := uuid.NewString() + tx, err := pool.Begin() + must(t, err) + defer func() { _ = tx.Rollback() }() + _, err = tx.Exec(`INSERT INTO tenants(id,name,status) VALUES($1,'Foreign tenant','active')`, foreignTenant) + must(t, err) + _, err = tx.Exec(`INSERT INTO tenant_memberships(tenant_id,user_id,role,status) VALUES($1,$2,'admin','active')`, foreignTenant, seed.userID) + must(t, err) + must(t, tx.Commit()) + foreignRef := seedInvocationWorkflowRun(t, pool, foreignTenant) + _, err = insertInvocationIssueRun(pool, seed, "workflow", foreignRef) + wantPGError(t, err, "23503") +} + +func TestWorkflowInvocationMigrationUpgradePreservesHistory(t *testing.T) { + pool, _ := testSchema(t, "test_invocation_upgrade_") + applyMigrationsUpTo(t, pool, migrationsBefore(t, workflowInvocationMigration)) + seed := seedOwnershipChain(t, pool) + ids := map[string]string{} + for _, status := range []string{"queued", "dispatched", "running", "deferred", "completed", "failed", "cancelled"} { + id := uuid.NewString() + ids[status] = id + _, err := pool.Exec(`INSERT INTO issue_runs(id,tenant_id,issue_id,executor_type,executor_id,status,input,failure_reason) + VALUES($1,$2,$3,'workflow',$4,$5,'{"topic":"historical"}','existing-history')`, id, seed.tenantID, seed.issueID, uuid.NewString(), status) + must(t, err) + } + rowJSON := func(id string) string { + t.Helper() + var out string + must(t, pool.QueryRow(`SELECT row_to_json(ir)::text FROM issue_runs ir WHERE id=$1`, id).Scan(&out)) + return out + } + terminalBefore := map[string]string{} + for _, status := range []string{"completed", "failed", "cancelled"} { + terminalBefore[status] = rowJSON(ids[status]) + } + agentBefore := rowJSON(seed.runID) + store := newStoreOnSchema(t, pool) + migratedTwice(t, store) + assertInvocationConstraintValidation(t, pool) + + for _, before := range []string{"queued", "dispatched", "running", "deferred"} { + var status, reason, topic string + var reference sql.NullString + var completed bool + var version int + must(t, pool.QueryRow(`SELECT status,failure_reason,input->>'topic',workflow_invocation_ref,completed_at IS NOT NULL,version + FROM issue_runs WHERE id=$1`, ids[before]).Scan(&status, &reason, &topic, &reference, &completed, &version)) + if status != "failed" || reason != "workflow_execution_unavailable_before_invocation_linkage" || topic != "historical" || reference.Valid || !completed || version != 2 { + t.Fatalf("historical %s settlement: status=%q reason=%q topic=%q ref=%v completed=%v version=%d", before, status, reason, topic, reference, completed, version) + } + } + for status, before := range terminalBefore { + if got := rowJSON(ids[status]); got != before { + t.Fatalf("terminal %s workflow history changed: before=%s after=%s", status, before, got) + } + } + if got := rowJSON(seed.runID); got != agentBefore { + t.Fatalf("ordinary agent row changed: before=%s after=%s", agentBefore, got) + } + var issues, workflowRuns int + must(t, pool.QueryRow(`SELECT count(*) FROM issue_runs`).Scan(&issues)) + must(t, pool.QueryRow(`SELECT count(*) FROM workflow_runs`).Scan(&workflowRuns)) + if issues != 8 || workflowRuns != 0 { + t.Fatalf("upgrade lost history or invented execution: IssueRuns=%d WorkflowRuns=%d, want 8/0", issues, workflowRuns) + } + + // A queued invocation must link execution; historical terminal rows remain editable. + _, err := insertInvocationIssueRun(pool, seed, "workflow", nil) + wantPGError(t, err, "23514") + for _, id := range ids { + _, err = pool.Exec(`UPDATE issue_runs SET trigger_summary='retained history' WHERE id=$1`, id) + must(t, err) + var summary string + must(t, pool.QueryRow(`SELECT trigger_summary FROM issue_runs WHERE id=$1`, id).Scan(&summary)) + if summary != "retained history" { + t.Fatalf("historical row %s was not editable", id) + } + } + ref := seedInvocationWorkflowRun(t, pool, seed.tenantID) + validID, err := insertInvocationIssueRun(pool, seed, "workflow", ref) + must(t, err) + _, err = pool.Exec(`UPDATE issue_runs SET workflow_invocation_ref=NULL WHERE id=$1`, validID) + wantPGError(t, err, "23514") +} + +func TestWorkflowInvocationMigrationUpgradeWithoutLegacyValidates(t *testing.T) { + pool, _ := testSchema(t, "test_invocation_clean_upgrade_") + applyMigrationsUpTo(t, pool, migrationsBefore(t, workflowInvocationMigration)) + seedOwnershipChain(t, pool) + migratedTwice(t, newStoreOnSchema(t, pool)) + assertInvocationConstraintValidation(t, pool) +} diff --git a/integration/workflow_invocation_test.go b/integration/workflow_invocation_test.go new file mode 100644 index 000000000..cb83e357f --- /dev/null +++ b/integration/workflow_invocation_test.go @@ -0,0 +1,410 @@ +package integration + +import ( + "context" + "reflect" + "sort" + "strings" + "sync" + "testing" + + "github.com/google/uuid" + + "github.com/wanglongan587/cloud/internal/core" +) + +// TestWorkflowConfirmCreatesLinkedWorkflowRun checks the persisted execution identity across the +// public confirm boundary, rather than accepting a legacy IssueRun simulation as execution evidence. +func TestWorkflowConfirmCreatesLinkedWorkflowRun(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Linked workflow", "inv-link-wf", nil) + snapshot := publishWorkflow(t, f, wid, "inv-link-pub") + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Linked execution", "projectRef": runProject(t, f, "inv-link-project")}, "inv-link-issue", 200).O("resource").S("id") + interaction := workflowComment(t, f, wid, iid, "inv-link-comment", "run") + run := f.call("POST", confirmPath(f, iid, interaction.S("id")), core.Object{"values": core.Object{ + "repository": "ora-space/cloud", "branch": "main", "snapshotId": snapshot.S("id"), + }}, "inv-link-confirm", 200).O("resource") + if run.S("workflowInvocationRef") == "" { + t.Fatalf("confirmed workflow IssueRun has no linked WorkflowRun: %v", run) + } + workflowRun := f.call("GET", f.path("/workflows/"+wid+"/runs/"+run.S("workflowInvocationRef")), nil, "", 200) + if workflowRun.S("snapshotId") != snapshot.S("id") || run.O("input").S("workflowSnapshotId") != snapshot.S("id") { + t.Fatalf("linked runs disagree about the selected snapshot: issue=%v workflow=%v", run, workflowRun) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE workflow_id=$1", wid); n != 1 { + t.Fatalf("confirm created %d WorkflowRuns, want exactly one", n) + } +} + +// assertUnclaimedWorkflowInvocation checks the entire refused-start transaction, including activity +// and the interaction claim; a status assertion alone would miss a partially committed start. +func assertUnclaimedWorkflowInvocation(t *testing.T, f *fixture, iid, interactionID string, activities int) { + t.Helper() + if n := f.scalar("SELECT count(*) FROM issue_runs WHERE issue_id=$1", iid); n != 0 { + t.Fatalf("refused confirm left %d IssueRuns", n) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs"); n != 0 { + t.Fatalf("refused confirm left %d WorkflowRuns", n) + } + if n := f.scalar("SELECT count(*) FROM issue_activities WHERE issue_id=$1", iid); n != activities { + t.Fatalf("refused confirm changed activity count from %d to %d", activities, n) + } + if n := f.scalar("SELECT count(*) FROM issue_interactions WHERE id=$1 AND run_id IS NULL AND input='{}'::jsonb", interactionID); n != 1 { + t.Fatal("refused confirm claimed the interaction or changed its input") + } +} + +func invocationVariable(name string) core.Object { + return core.Object{"name": name, "displayName": name, "fieldType": "text-input", "valueType": "string", "required": true} +} + +func TestWorkflowConfirmAdmissionRollsBackBothRuns(t *testing.T) { + for _, scenario := range []string{"unpublished", "wrong-workflow-snapshot", "foreign-snapshot", "missing-input", "invalid-input-type"} { + t.Run(scenario, func(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Admission workflow", "inv-gate-wf", []any{invocationVariable("topic")}) + values := core.Object{"repository": "ora-space/cloud", "branch": "main", "topic": "auth"} + status, code := 400, "" + if scenario != "unpublished" { + values["snapshotId"] = publishWorkflow(t, f, wid, "inv-gate-pub").S("id") + } else { + code = "workflow_no_published_snapshot" + } + switch scenario { + case "wrong-workflow-snapshot": + other := createWorkflow(t, f, "Other workflow", "inv-gate-other", nil) + values["snapshotId"] = publishWorkflow(t, f, other, "inv-gate-other-pub").S("id") + status = 404 + case "foreign-snapshot": + other, err := f.store.Bootstrap(context.Background(), "Other tenant", "corp", "inv-other", "Other") + must(t, err) + tenant, subject := f.tid, f.user.Subject + f.tid, f.user.Subject = other.S("tenantId"), "inv-other" + foreign := createWorkflow(t, f, "Foreign workflow", "inv-gate-foreign", nil) + values["snapshotId"] = publishWorkflow(t, f, foreign, "inv-gate-foreign-pub").S("id") + f.tid, f.user.Subject = tenant, subject + status = 404 + case "missing-input": + delete(values, "topic") + code = "required_field_missing" + case "invalid-input-type": + values["topic"] = 42 + code = "invalid_field_value" + } + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Rejected start", "projectRef": runProject(t, f, "inv-gate-project")}, "inv-gate-issue", 200).O("resource").S("id") + it := workflowComment(t, f, wid, iid, "inv-gate-comment", "run") + activities := f.scalar("SELECT count(*) FROM issue_activities WHERE issue_id=$1", iid) + refusal := f.call("POST", confirmPath(f, iid, it.S("id")), core.Object{"values": values}, "inv-gate-confirm", status) + if code != "" && refusal.S("code") != code { + t.Fatalf("refused start code = %q, want %q: %v", refusal.S("code"), code, refusal) + } + assertUnclaimedWorkflowInvocation(t, f, iid, it.S("id"), activities) + }) + } +} + +func TestWorkflowConfirmFreezesHistoricalSchemaAndInput(t *testing.T) { + f := setup(t) + graph := func(variables []any) core.Object { + out := startInputGraph(variables) + out["globalVariables"] = []any{core.Object{"name": "review.mode", "valueType": "string", "value": "default"}} + return out + } + wid := f.call("POST", f.path("/workflows"), core.Object{ + "name": "Historical workflow", "graph": graph([]any{invocationVariable("topic")}), + }, "inv-old-wf", 200).O("resource").S("id") + older := publishWorkflow(t, f, wid, "inv-old-pub-1") + edit := func(variables []any, zoom float64) { + t.Helper() + live := f.call("GET", f.path("/workflows/"+wid), nil, "", 200) + updated := graph(variables) + updated["viewport"] = core.Object{"x": 0, "y": 0, "zoom": zoom} + f.call("PUT", f.path("/workflows/"+wid), core.Object{ + "graph": updated, "version": live.N("version"), + }, "", 200) + } + // A newer graph with the same fields keeps the live-form compatibility gate satisfied. + edit([]any{invocationVariable("topic")}, 2) + newer := publishWorkflow(t, f, wid, "inv-old-pub-2") + if reflect.DeepEqual(older.O("graph"), newer.O("graph")) { + t.Fatal("historical snapshot test needs distinct published definitions") + } + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Historical start", "projectRef": runProject(t, f, "inv-old-project")}, "inv-old-issue", 200).O("resource").S("id") + it := workflowComment(t, f, wid, iid, "inv-old-comment", "run old version") + created := f.call("POST", confirmPath(f, iid, it.S("id")), core.Object{"values": core.Object{ + "repository": "ora-space/cloud", "branch": "main", "snapshotId": older.S("id"), "topic": "auth", "review.mode": "strict", + }}, "inv-old-confirm", 200).O("resource") + ref := created.S("workflowInvocationRef") + if ref == "" { + t.Fatal("historical confirm did not link a WorkflowRun") + } + path := f.path("/workflows/" + wid + "/runs/" + ref) + before := f.call("GET", path, nil, "", 200) + if before.S("snapshotId") != older.S("id") || created.O("input").S("workflowSnapshotId") != older.S("id") { + t.Fatalf("historical snapshot was not selected: issue=%v workflow=%v", created, before) + } + if !reflect.DeepEqual(before.O("input"), core.Object{"topic": "auth", "review.mode": "strict"}) { + t.Fatalf("execution input must contain selected-schema variables only: %v", before.O("input")) + } + if !reflect.DeepEqual(before.O("definitionSnapshot"), older.O("graph")) { + t.Fatalf("definition does not match selected frozen document: %v", before) + } + edit([]any{invocationVariable("replacement")}, 3) + publishWorkflow(t, f, wid, "inv-old-pub-3") + after := f.call("GET", path, nil, "", 200) + for _, field := range []string{"snapshotId", "input", "definitionSnapshot"} { + if !reflect.DeepEqual(after[field], before[field]) { + t.Fatalf("editing and republishing changed frozen %s: before=%v after=%v", field, before[field], after[field]) + } + } +} + +func TestWorkflowConfirmInvocationIdempotency(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Replay workflow", "inv-idem-wf", nil) + publishWorkflow(t, f, wid, "inv-idem-pub") + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Replay start", "projectRef": runProject(t, f, "inv-idem-project")}, "inv-idem-issue", 200).O("resource").S("id") + it := workflowComment(t, f, wid, iid, "inv-idem-comment", "run") + body := core.Object{"values": core.Object{"repository": "ora-space/cloud", "branch": "main"}} + path := confirmPath(f, iid, it.S("id")) + first := f.call("POST", path, body, "inv-idem-confirm", 200) + replay := f.call("POST", path, body, "inv-idem-confirm", 200) + if !reflect.DeepEqual(first, replay) { + t.Fatalf("same-key confirm did not replay its original response: first=%v replay=%v", first, replay) + } + if first.O("resource").S("workflowInvocationRef") == "" { + t.Fatal("replayed response has no WorkflowRun identity") + } + refusal := f.call("POST", path, body, "inv-idem-second", 409) + if refusal.S("code") != "interaction_already_confirmed" { + t.Fatalf("second confirm fault = %v", refusal) + } + if n := f.scalar("SELECT count(*) FROM issue_runs WHERE issue_id=$1", iid); n != 1 { + t.Fatalf("replay created %d IssueRuns", n) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE workflow_id=$1", wid); n != 1 { + t.Fatalf("replay created %d WorkflowRuns", n) + } +} + +func TestWorkflowConfirmConcurrentInvocationCreatesExactlyOnePair(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Concurrent workflow", "inv-race-wf", nil) + publishWorkflow(t, f, wid, "inv-race-pub") + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Concurrent start", "projectRef": runProject(t, f, "inv-race-project")}, "inv-race-issue", 200).O("resource").S("id") + it := workflowComment(t, f, wid, iid, "inv-race-comment", "run") + path := confirmPath(f, iid, it.S("id")) + start := make(chan struct{}) + var wg sync.WaitGroup + bodies, statuses, errs := make([]core.Object, 2), make([]int, 2), make([]error, 2) + for i := range 2 { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + bodies[i], statuses[i], errs[i] = callRaw(f, "POST", path, core.Object{"values": core.Object{ + "repository": "ora-space/cloud", "branch": "main", + }}, "inv-race-"+uuid.NewString()) + }(i) + } + close(start) + wg.Wait() + var accepted core.Object + for i := range 2 { + must(t, errs[i]) + if statuses[i] == 200 { + accepted = bodies[i].O("resource") + } else if bodies[i].S("code") != "interaction_already_confirmed" { + t.Fatalf("concurrent refusal = %v", bodies[i]) + } + } + sort.Ints(statuses) + if !reflect.DeepEqual(statuses, []int{200, 409}) { + t.Fatalf("concurrent statuses = %v, want [200 409]", statuses) + } + if n := f.scalar("SELECT count(*) FROM issue_runs WHERE issue_id=$1", iid); n != 1 { + t.Fatalf("concurrent confirm created %d IssueRuns", n) + } + if n := f.scalar("SELECT count(*) FROM workflow_runs WHERE workflow_id=$1", wid); n != 1 { + t.Fatalf("concurrent confirm created %d WorkflowRuns", n) + } + if n := f.scalar("SELECT count(*) FROM issue_runs ir JOIN workflow_runs wr ON wr.id=ir.workflow_invocation_ref AND wr.tenant_id=ir.tenant_id JOIN issue_interactions ix ON ix.run_id=ir.id WHERE ix.id=$1 AND ir.id=$2 AND wr.id=$3", it.S("id"), accepted.S("id"), accepted.S("workflowInvocationRef")); n != 1 { + t.Fatal("winning confirm did not atomically persist the interaction and same-tenant run pair") + } +} + +func TestManualWorkflowInvocationCreatesLinkedRun(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Manual workflow", "inv-manual-wf", []any{invocationVariable("topic")}) + snapshot := publishWorkflow(t, f, wid, "inv-manual-pub") + iid := f.call("POST", f.path("/issues"), core.Object{"title": "Manual start", "projectRef": runProject(t, f, "inv-manual-project")}, "inv-manual-issue", 200).O("resource").S("id") + status, body := manualWorkflowRun(t, f, iid, wid, "inv-manual-start", core.Object{"topic": "auth"}) + if status != 200 { + t.Fatalf("manual start status = %d: %v", status, body) + } + run := body.O("resource") + if run.S("workflowInvocationRef") == "" { + t.Fatal("manual workflow IssueRun has no WorkflowRun identity") + } + linked := f.call("GET", f.path("/workflows/"+wid+"/runs/"+run.S("workflowInvocationRef")), nil, "", 200) + if linked.S("snapshotId") != snapshot.S("id") || run.O("input").S("workflowSnapshotId") != snapshot.S("id") { + t.Fatalf("manual run pair froze different snapshots: issue=%v workflow=%v", run, linked) + } + if linked.O("input").S("topic") != "auth" { + t.Fatalf("manual WorkflowRun lost its execution input: %v", linked) + } + if n := f.scalar("SELECT count(*) FROM issue_runs ir JOIN workflow_runs wr ON wr.id=ir.workflow_invocation_ref AND wr.tenant_id=ir.tenant_id WHERE ir.issue_id=$1", iid); n != 1 { + t.Fatalf("manual start created %d valid linked pairs, want 1", n) + } +} + +// confirmInvocationScene crosses the public comment/form boundary. Unlike the fixture's direct +// workflow-run POST, Confirm leaves execution pending for the real scheduler used by these tests. +func confirmInvocationScene(t *testing.T, f *fixture, graph core.Object, key string) (wid, iid string, confirmed core.Object) { + t.Helper() + wid = f.call("POST", f.path("/workflows"), core.Object{ + "name": "Scheduled invocation", "graph": graph, + }, key+"-workflow", 200).O("resource").S("id") + publishWorkflow(t, f, wid, key+"-publish") + iid = launchIssue(t, f, runProject(t, f, key+"-project"), "Scheduled invocation", key+"-issue").S("id") + interaction := workflowComment(t, f, wid, iid, key+"-comment", "run this workflow") + confirmed = f.call("POST", confirmPath(f, iid, interaction.S("id")), core.Object{"values": core.Object{ + "repository": "ora-space/cloud", "branch": "main", + }}, key+"-confirm", 200).O("resource") + if confirmed.S("status") != "queued" || confirmed.S("workflowInvocationRef") == "" { + t.Fatalf("Confirm must enqueue a linked invocation before scheduling: %v", confirmed) + } + return wid, iid, confirmed +} + +// assertInvocationProjection reads both public run resources and the issue timeline, so a scheduler +// state change without its IssueRun projection or lifecycle activity cannot satisfy the test. +func assertInvocationProjection(t *testing.T, f *fixture, wid, iid string, confirmed core.Object, workflowStatus, issueStatus string, actions []string) (core.Object, core.Object) { + t.Helper() + workflow := f.call("GET", f.path("/workflows/"+wid+"/runs/"+confirmed.S("workflowInvocationRef")), nil, "", 200) + var issueRun core.Object + for _, candidate := range issueItems(f.call("GET", f.path("/issues/"+iid+"/runs"), nil, "", 200)) { + if candidate.S("id") == confirmed.S("id") { + issueRun = candidate + break + } + } + if workflow.S("status") != workflowStatus || issueRun.S("status") != issueStatus { + t.Fatalf("scheduler projection disagrees: workflow=%v issue=%v; want %s/%s", workflow, issueRun, workflowStatus, issueStatus) + } + if issueRun.S("workflowInvocationRef") != workflow.S("id") || issueRun.S("executorId") != wid || issueRun.S("executorType") != "workflow" { + t.Fatalf("projected IssueRun lost its execution identity: %v", issueRun) + } + if workflow.S("startedAt") == "" || issueRun.S("startedAt") != workflow.S("startedAt") || issueRun.S("completedAt") != workflow.S("finishedAt") { + t.Fatalf("projected lifecycle timestamps disagree: workflow=%v issue=%v", workflow, issueRun) + } + if issueRun.S("failureReason") != workflow.S("error") || issueRun.S("error") != workflow.S("error") { + t.Fatalf("projected failure reason disagrees: workflow=%v issue=%v", workflow, issueRun) + } + var got []string + for _, item := range issueItems(f.call("GET", f.path("/issues/"+iid+"/timeline"), nil, "", 200)) { + details := item.O("details") + if item.S("kind") != "activity" || details.S("runId") != confirmed.S("id") { + continue + } + got = append(got, item.S("action")) + if details.S("executorType") != "workflow" || details.S("executorId") != wid { + t.Fatalf("activity lost its executor identity: %v", item) + } + if item.S("action") != "run.enqueued" && details.S("workflowInvocationRef") != workflow.S("id") { + t.Fatalf("scheduler activity lost its workflow invocation identity: %v", item) + } + if item.S("action") == "run.failed" && details.S("failureReason") != workflow.S("error") { + t.Fatalf("failure activity lost the authoritative reason: %v", item) + } + } + if !reflect.DeepEqual(got, actions) { + t.Fatalf("invocation lifecycle activities = %v, want %v", got, actions) + } + return workflow, issueRun +} + +func TestWorkflowConfirmSchedulerProjectsRunningOnce(t *testing.T) { + f := setup(t) + seedPersonalModel(t, f) + wid, iid, confirmed := confirmInvocationScene(t, f, runGraph(), "inv-scheduler-running") + // The Project is still provisioning: session startup waits without Node or cluster execution. + // A second advance exercises the running-state retry, which must not emit another run.started. + for range 2 { + must(t, f.store.AdvanceWorkflowRunsOnce(t.Context())) + } + workflow, _ := assertInvocationProjection(t, f, wid, iid, confirmed, "running", "running", []string{"run.enqueued", "run.started"}) + if len(workflow.O("nodeStates")) != 0 || workflow.S("finishedAt") != "" { + t.Fatalf("waiting invocation fabricated committed nodes or terminal evidence: %v", workflow) + } +} + +func TestWorkflowConfirmSchedulerCompletesAndAllowsNextInvocation(t *testing.T) { + f := setup(t) + graph := startInputGraph(nil) + graph["nodes"] = append(graph["nodes"].([]any), core.Object{ + "id": "output-1", "type": "workflow", "position": core.Object{"x": 300, "y": 0}, + "data": core.Object{"kind": "output", "title": "Done"}, + }) + graph["edges"] = []any{core.Object{"id": "output-edge", "source": "start-1", "target": "output-1"}} + wid, iid, confirmed := confirmInvocationScene(t, f, graph, "inv-scheduler-completed") + for range 2 { + must(t, f.store.AdvanceWorkflowRunsOnce(t.Context())) + } + workflow, _ := assertInvocationProjection(t, f, wid, iid, confirmed, "succeeded", "completed", []string{"run.enqueued", "run.started", "run.completed"}) + for _, node := range []string{"start-1", "output-1"} { + if workflow.O("nodeStates").O(node).S("status") != "succeeded" { + t.Fatalf("completed invocation did not execute node %s: %v", node, workflow) + } + } + if workflow.S("finishedAt") == "" { + t.Fatal("completed invocation has no terminal timestamp") + } + + // workflowComment returns the first form, so find the fresh unclaimed form explicitly on this + // same Issue. Terminal projection must release the pending-executor dedup for this Confirm. + f.call("POST", f.path("/issues/"+iid+"/comments"), core.Object{ + "body": "run this workflow again", "targets": []any{core.Object{"type": "workflow", "id": wid}}, + }, "inv-scheduler-completed-next-comment", 200) + var nextInteraction core.Object + for _, interaction := range issueItems(f.call("GET", f.path("/issues/"+iid+"/interactions"), nil, "", 200)) { + if interaction.S("mode") == "form" && interaction.S("runId") == "" { + nextInteraction = interaction + break + } + } + if nextInteraction == nil { + t.Fatal("second workflow comment did not create an unclaimed form") + } + next := f.call("POST", confirmPath(f, iid, nextInteraction.S("id")), core.Object{"values": core.Object{ + "repository": "ora-space/cloud", "branch": "main", + }}, "inv-scheduler-completed-next-confirm", 200).O("resource") + if next.S("status") != "queued" || next.S("id") == confirmed.S("id") || next.S("workflowInvocationRef") == "" || next.S("workflowInvocationRef") == confirmed.S("workflowInvocationRef") { + t.Fatalf("second Confirm did not create a new linked queued invocation: first=%v next=%v", confirmed, next) + } +} + +func TestWorkflowConfirmSchedulerProjectsConditionFailure(t *testing.T) { + f := setup(t) + graph := startInputGraph(nil) + // input is a declared Start root and is always a string. Publishing admits the selector, but + // traversing a nested object path on that string deterministically fails during real execution. + graph["nodes"] = append(graph["nodes"].([]any), core.Object{ + "id": "condition-1", "type": "workflow", "position": core.Object{"x": 300, "y": 0}, + "data": core.Object{"kind": "condition", "title": "Invalid nested input", "cases": []any{core.Object{ + "id": "matched", "logic": "and", "conditions": []any{core.Object{ + "variableSelector": []any{"start-1", "input", "missing"}, "operator": "equals", "value": "value", + }}, + }}}, + }) + graph["edges"] = []any{core.Object{"id": "condition-edge", "source": "start-1", "target": "condition-1"}} + wid, iid, confirmed := confirmInvocationScene(t, f, graph, "inv-scheduler-failed") + for range 2 { + must(t, f.store.AdvanceWorkflowRunsOnce(t.Context())) + } + workflow, _ := assertInvocationProjection(t, f, wid, iid, confirmed, "failed", "failed", []string{"run.enqueued", "run.started", "run.message", "run.failed"}) + failed := workflow.O("nodeStates").O("condition-1") + if failed.S("status") != "failed" || !strings.Contains(workflow.S("error"), "start-1.input.missing") || failed.S("error") != workflow.S("error") || workflow.S("finishedAt") == "" { + t.Fatalf("condition failure did not preserve its node reason and terminal evidence: %v", workflow) + } +} diff --git a/integration/workflow_run_timestamp_test.go b/integration/workflow_run_timestamp_test.go new file mode 100644 index 000000000..3006bf58e --- /dev/null +++ b/integration/workflow_run_timestamp_test.go @@ -0,0 +1,26 @@ +package integration + +import ( + "testing" + + "github.com/google/uuid" +) + +func TestPendingWorkflowRunDetailPreservesNullTimestamps(t *testing.T) { + f := setup(t) + wid := createWorkflow(t, f, "Pending timestamps", "pending-timestamps-wf", nil) + snapshot := publishWorkflow(t, f, wid, "pending-timestamps-pub") + rid := uuid.NewString() + _, err := f.store.Pool.Exec(`INSERT INTO workflow_runs(id,tenant_id,workflow_id,snapshot_id,name) + VALUES($1,$2,$3,$4,'Pending timestamps')`, rid, f.tid, wid, snapshot.S("id")) + must(t, err) + run := f.call("GET", f.path("/workflows/"+wid+"/runs/"+rid), nil, "", 200) + if run.S("status") != "pending" { + t.Fatalf("run status = %v, want pending", run["status"]) + } + for _, field := range []string{"startedAt", "finishedAt"} { + if value, present := run[field]; !present || value != nil { + t.Fatalf("pending run %s = %#v (present=%v), want JSON null", field, value, present) + } + } +} diff --git a/internal/core/README.en.md b/internal/core/README.en.md index 87887e72b..b6c98678c 100644 --- a/internal/core/README.en.md +++ b/internal/core/README.en.md @@ -18,7 +18,7 @@ This package owns business state, authorization, transactions and recovery decis | `revision*.go`, `session_*.go` | Revision registration, archives, session listing and statistics | [Revision](../../docs/agent-runs/revisions.en.md), [Session reads](../../docs/agent-runs/session-reads.en.md) | | `model_*.go` | Personal configuration, frozen bindings, grant digests and request observations; no API Key decryption | [Model access](../../docs/models/model-access.en.md) | | `plugins.go`, `plugin_*.go`, `workflow_asset_imports.go` | Catalog snapshots with Agent declaration copies, space selection, Node install plans with frozen declaration associations and asset import ledger | [Plugin marketplace](../../docs/plugins/marketplace.en.md) | -| `workflow*.go` | Graphs, published snapshots, admission, run identity and scheduler | [Workflows](../../docs/workflows/workflows.en.md) | +| `workflow*.go` | Graphs, published snapshots, admission, run identity and scheduler; WorkflowRun creation shared by Issue confirm/manual and direct starts, the `workflow_invocation_ref` link, and the scheduler's projection of WorkflowRun status onto the linked IssueRun | [Workflows](../../docs/workflows/workflows.en.md), [Issue collaboration](../../docs/collaboration/interactions.en.md) | | `hub.go`, `signals.go` | Post-commit invalidation and control hints; no authoritative state | [HTTP API](../../docs/core/http-api.en.md) | ## Store and transaction boundaries diff --git a/internal/core/README.md b/internal/core/README.md index 0a3ebee94..c8127ee3c 100644 --- a/internal/core/README.md +++ b/internal/core/README.md @@ -18,7 +18,7 @@ | `revision*.go`、`session_*.go` | Revision 登记、归档读取、会话列表与统计 | [Revision](../../docs/agent-runs/revisions.md)、[会话读取](../../docs/agent-runs/session-reads.md) | | `model_*.go` | 个人配置、冻结绑定、授权摘要与请求观察;不解密 API Key | [模型访问](../../docs/models/model-access.md) | | `plugins.go`、`plugin_*.go`、`workflow_asset_imports.go` | 含 Agent 声明副本的市场快照、空间选择、冻结声明关联的 Node 安装计划和资产导入账本 | [插件市场](../../docs/plugins/marketplace.md) | -| `workflow*.go` | 图、发布快照、准入、运行身份与调度器 | [工作流](../../docs/workflows/workflows.md) | +| `workflow*.go` | 图、发布快照、准入、运行身份、调度器;Issue Confirm/manual 与直接启动共用的 WorkflowRun 创建、`workflow_invocation_ref` 关联,以及调度器把 WorkflowRun 状态投影回关联 IssueRun | [工作流](../../docs/workflows/workflows.md)、[Issue 协作](../../docs/collaboration/interactions.md) | | `hub.go`、`signals.go` | 提交后失效通知和控制信号;不承载权威状态 | [HTTP API](../../docs/core/http-api.md) | ## Store 与事务边界 diff --git a/internal/core/issue_interactions.go b/internal/core/issue_interactions.go index 0a55ec19e..e7c8b4747 100644 --- a/internal/core/issue_interactions.go +++ b/internal/core/issue_interactions.go @@ -109,9 +109,9 @@ func applyCommentTargets(t *transaction, i Object, r *PublicRequest, uid, commen // confirmInteraction is the ONLY transition that turns a Workflow interaction into an execution intent // (Issue collaboration D9). It re-resolves the target server-side, re-validates the submitted values, persists them as -// the interaction's Issue-owned input, creates the initial run, and claims the interaction with a -// compare-and-set on `run_id IS NULL`. Dispatch is the caller's post-commit responsibility, so no -// external effect happens inside this transaction (Issue collaboration D9). +// the interaction's Issue-owned input, creates the WorkflowRun and linked IssueRun, and claims the +// interaction with a compare-and-set on `run_id IS NULL`. WorkflowRun owns execution; this path never +// hands the linked IssueRun to the legacy dispatcher or performs an external effect (Issue collaboration D9). // // The values are validated against the schema of the *version being confirmed* rather than the live // document the form was rendered from, and against the same admission gate the other two entries use. @@ -132,7 +132,7 @@ func applyCommentTargets(t *transaction, i Object, r *PublicRequest, uid, commen // client rendered, which is the live document's. When the two have diverged the confirm refuses with // `workflow_version_fields_changed` and names the fields, rather than validating values the user was // shown against a schema they were never shown — see workflowFieldDivergence. -func confirmInteraction(t *transaction, r *PublicRequest, uid string, dispatches *[]dispatchTarget) Object { +func confirmInteraction(t *transaction, r *PublicRequest, uid string) Object { it := interaction(t, r.TenantID, r.IssueID, r.InteractionID) require(it.S("mode") == "form", 409, "interaction_not_confirmable") require(it["runId"] == nil, 409, "interaction_already_confirmed") @@ -152,11 +152,9 @@ func confirmInteraction(t *transaction, r *PublicRequest, uid string, dispatches validateFormValues(versionDescriptor, values, true) input := buildRunContext(t, i, "", targetType, targetID, values, appliedContextRefs(r.Body)) - freezeWorkflowSnapshot(input, s.S("id")) - runObj := enqueueRun(t, r.TenantID, i.S("id"), targetType, targetID, input, "interaction", it.S("id"), "user", uid) + runObj := enqueueWorkflowInvocation(t, r.TenantID, i.S("id"), w, s, values, input, "interaction", it.S("id"), uid) claimed := t.execRows("UPDATE issue_interactions SET input=$3, run_id=$4 WHERE id=$1 AND tenant_id=$2 AND run_id IS NULL", it.S("id"), r.TenantID, jsonText(values), runObj.S("id")) require(claimed == 1, 409, "interaction_already_confirmed") - *dispatches = append(*dispatches, dispatchTarget{tenantID: r.TenantID, runID: runObj.S("id")}) return Object{"resource": runObj} } diff --git a/internal/core/issue_run_lifecycle.go b/internal/core/issue_run_lifecycle.go index 225b79c40..911e28fee 100644 --- a/internal/core/issue_run_lifecycle.go +++ b/internal/core/issue_run_lifecycle.go @@ -94,6 +94,12 @@ func enqueueRun(t *transaction, tid, iid, executorType, executorID string, input // enqueueAdmittedRun persists a Run only after the caller owns input admission. Direct creation // uses enqueueRun; the durable task worker has already validated its specific queue head. func enqueueAdmittedRun(t *transaction, tid, iid, executorType, executorID string, input Object, triggerKind string, triggerRef any, actorType, actorID string) Object { + return enqueueRunWithInvocation(t, tid, iid, executorType, executorID, input, triggerKind, triggerRef, actorType, actorID, "") +} + +// enqueueRunWithInvocation inserts the reference with the IssueRun itself so PostgreSQL can enforce +// the invocation shape immediately; no transient unlinked workflow IssueRun is ever inserted. +func enqueueRunWithInvocation(t *transaction, tid, iid, executorType, executorID string, input Object, triggerKind string, triggerRef any, actorType, actorID, workflowRunID string) Object { require(t.one("SELECT id FROM issue_runs WHERE issue_id=$1 AND executor_type=$2 AND executor_id=$3 AND status IN ('queued','dispatched') AND deleted_at IS NULL", iid, executorType, executorID) == nil, 409, "pending_run_exists") if input == nil { input = Object{} @@ -108,7 +114,7 @@ func enqueueAdmittedRun(t *transaction, tid, iid, executorType, executorID strin delete(input, "workflowSnapshotId") } id := newID() - t.exec("INSERT INTO issue_runs(id,tenant_id,issue_id,executor_type,executor_id,input,status,trigger_evidence_kind,trigger_evidence_ref_id) VALUES($1,$2,$3,$4,$5,$6,'queued',$7,$8)", id, tid, iid, executorType, executorID, jsonText(input), triggerKind, triggerRef) + t.exec("INSERT INTO issue_runs(id,tenant_id,issue_id,executor_type,executor_id,input,status,trigger_evidence_kind,trigger_evidence_ref_id,workflow_invocation_ref) VALUES($1,$2,$3,$4,$5,$6,'queued',$7,$8,$9)", id, tid, iid, executorType, executorID, jsonText(input), triggerKind, triggerRef, nullable(workflowRunID)) if executorType == "agent" { // Caller-supplied private bindings cannot authorize a model connection. The production // OpenCode path always creates its own owner-scoped binding in this transaction. @@ -136,6 +142,11 @@ func (s *Store) dispatchRun(ctx context.Context, tenantID, runID string) error { if o == nil || o.S("status") != "queued" { return Object{} } + if o.S("workflowInvocationRef") != "" { + // WorkflowRun owns execution. A legacy dispatcher (including dev doubles) must never + // create a second execution or advance its linked IssueRun independently. + return Object{} + } if o.S("executorType") == "agent" && isSpaceAgentRun(t, tenantID, o.S("executorId")) { // A real Space Agent run is claimed by AgentRunDispatcher (Claim + Busy), independent of // whether the legacy ExecutionDispatcher is wired (nil in production). This routing is the diff --git a/internal/core/issue_runs.go b/internal/core/issue_runs.go index c47f79298..f35adffba 100644 --- a/internal/core/issue_runs.go +++ b/internal/core/issue_runs.go @@ -97,8 +97,7 @@ func createRun(t *transaction, r *PublicRequest, uid string) Object { if executorType == "workflow" { w := workflow(t, r.TenantID, executorID) s := admitWorkflowRun(t, r.TenantID, w.S("id"), "", uid) - requireWorkflowRunInput(s.O("graph"), input) - freezeWorkflowSnapshot(input, s.S("id")) + return enqueueWorkflowInvocation(t, r.TenantID, i.S("id"), w, s, input, input, "", nil, uid) } if executorType == "agent" { // Authoritative Space Agent (IssueRun D1): reuses the same evidence resolver as the comment diff --git a/internal/core/migrations/0052_workflow_invocation_reference.sql b/internal/core/migrations/0052_workflow_invocation_reference.sql new file mode 100644 index 000000000..7229efe3f --- /dev/null +++ b/internal/core/migrations/0052_workflow_invocation_reference.sql @@ -0,0 +1,25 @@ +-- Retire execution intents accepted before WorkflowRun linkage existed. A status-only settlement +-- cannot invent the snapshot/input those historical rows would have executed. +UPDATE issue_runs +SET status = 'failed', + failure_reason = 'workflow_execution_unavailable_before_invocation_linkage', + completed_at = now(), updated_at = now(), version = version + 1 +WHERE executor_type = 'workflow' AND workflow_invocation_ref IS NULL + AND status NOT IN ('completed', 'failed', 'cancelled'); + +ALTER TABLE workflow_runs + ADD CONSTRAINT workflow_runs_id_tenant_uniq UNIQUE (id, tenant_id); + +ALTER TABLE issue_runs + ADD CONSTRAINT issue_runs_workflow_invocation_fk + -- workflow_invocation_ref had no writer before this migration, so dangling references are not expected and the FK fails closed. + FOREIGN KEY (workflow_invocation_ref, tenant_id) REFERENCES workflow_runs(id, tenant_id), + ADD CONSTRAINT issue_runs_workflow_invocation_uniq UNIQUE (workflow_invocation_ref), + ADD CONSTRAINT issue_runs_workflow_invocation_shape + CHECK ((executor_type <> 'workflow' AND workflow_invocation_ref IS NULL) + OR (executor_type = 'workflow' AND (workflow_invocation_ref IS NOT NULL + OR status IN ('completed', 'failed', 'cancelled')))) NOT VALID; + +-- Historical terminal rows remain editable without inventing executions. New invocations enqueue +-- non-terminal rows and must carry a reference. Fresh and upgraded databases validate the same shape. +ALTER TABLE issue_runs VALIDATE CONSTRAINT issue_runs_workflow_invocation_shape; diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index f4ae94338..3b069125d 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 53 SQL files. +There are currently 54 SQL files. | SQL file | Schema change | |---|---| @@ -63,6 +63,7 @@ There are currently 53 SQL files. | [0049_thread_commands_workflow_run.sql](0049_thread_commands_workflow_run.sql) | WorkflowRun session-command ownership and node scope | | [0050_personal_agent_configurations.sql](0050_personal_agent_configurations.sql) | Persistent personal Agent configuration revisions and private run snapshots | | [0051_workflow_run_cancel.sql](0051_workflow_run_cancel.sql) | WorkflowRun cancellation requests | +| [0052_workflow_invocation_reference.sql](0052_workflow_invocation_reference.sql) | Invocation reference from workflow IssueRuns to WorkflowRuns | 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. @@ -77,6 +78,7 @@ There are currently 53 SQL files. - 0046–0048 backfill no old messages, Threads, history positions or pause state; `visible_text` is stored separately from the raw `record`, and old tasks are not paused automatically. Fresh, upgrade and repeated application are covered by `TestMigration0046TaskConversationFreshAndUpgrade` and the integration migration tests. - 0049 lets `thread_commands` accept a WorkflowRun owner and requires a graph node on workflow commands; each node holds at most one `end_session`, so the scheduler can replay it safely. Every old row carries `run_id` and is not backfilled. Semantics are in [workflows](../../../docs/workflows/workflows.en.md); coverage is in the `TestWorkflowSessionEndsAfterSuccessfulTurn` DB tests. - 0051 adds the cancellation-request instant to `workflow_runs` and backfills no old row; the request records intent only and is settled by the scheduler on later passes. Semantics are in [workflows](../../../docs/workflows/workflows.en.md#scheduling); coverage is in the `TestWorkflowRunCancelBeforeStartNeverStarts` DB tests. +- 0052 first settles every unlinked non-terminal historical workflow IssueRun (including running and deferred) as failed with `workflow_execution_unavailable_before_invocation_linkage`, preserving its input and outcome without inventing an execution. It then adds `workflow_runs_id_tenant_uniq` (unique `(id, tenant_id)` on `workflow_runs`, which the composite foreign key requires), the `(workflow_invocation_ref, tenant_id)` → `workflow_runs(id, tenant_id)` composite foreign key, a unique reference, and a CHECK that non-terminal workflow rows carry a reference while other executors carry none, all unconditionally validated. Historical terminal workflow rows may keep a NULL reference. Fresh, seeded upgrade, repeated application and constraint validation coverage is in `integration/workflow_invocation_migration_test.go`. - 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 623b17cb2..a2d7993cc 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` 在 ## 文件索引 -当前共 53 个 SQL 文件。 +当前共 54 个 SQL 文件。 | SQL 文件 | schema 变化 | |---|---| @@ -63,6 +63,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0049_thread_commands_workflow_run.sql](0049_thread_commands_workflow_run.sql) | WorkflowRun 的会话命令归属与节点范围 | | [0050_personal_agent_configurations.sql](0050_personal_agent_configurations.sql) | 个人 Agent 配置的持续 revision 与私有运行快照 | | [0051_workflow_run_cancel.sql](0051_workflow_run_cancel.sql) | WorkflowRun 的取消请求 | +| [0052_workflow_invocation_reference.sql](0052_workflow_invocation_reference.sql) | workflow IssueRun 到 WorkflowRun 的调用引用 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 @@ -77,6 +78,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 - 0046–0048 不回填旧消息、旧 Thread、历史位置或暂停状态;`visible_text` 与原始 `record` 分开保存,旧任务不自动暂停。空库、升级与重复应用覆盖见 `TestMigration0046TaskConversationFreshAndUpgrade` 及 integration 的迁移测试。 - 0049 让 `thread_commands` 接受 WorkflowRun 归属,并要求工作流命令带图节点;每个节点至多一条 `end_session`,调度器可安全重放。旧行全部带 `run_id`,不回填。语义见 [工作流](../../../docs/workflows/workflows.md),覆盖见 `TestWorkflowSessionEndsAfterSuccessfulTurn` 等 DB 测试。 - 0051 给 `workflow_runs` 增加取消请求时刻,不回填旧行;请求只记录意图,由调度器在后续轮次结算。语义见[工作流](../../../docs/workflows/workflows.md#调度),覆盖见 `TestWorkflowRunCancelBeforeStartNeverStarts` 等 DB 测试。 +- 0052 先把没有引用的历史非终态 workflow IssueRun(含 running、deferred)结算为 failed(`workflow_execution_unavailable_before_invocation_linkage`),保留其输入和结果、不虚构执行;再给 `workflow_runs` 加复合外键所需的 `workflow_runs_id_tenant_uniq`(`(id, tenant_id)` 唯一),并加 `(workflow_invocation_ref, tenant_id)` → `workflow_runs(id, tenant_id)` 复合外键、唯一引用,以及「非终态 workflow 行必须有引用、其他执行器不得有引用」的 CHECK,无条件 VALIDATE。历史终态 workflow 行可保留 NULL 引用。空库、带历史数据升级、重复应用与约束验证状态见 `integration/workflow_invocation_migration_test.go`。 - 新增迁移须验证空库、前一 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/public.go b/internal/core/public.go index 223b4c4e5..1788f5248 100644 --- a/internal/core/public.go +++ b/internal/core/public.go @@ -310,7 +310,7 @@ func (s *Store) Public(ctx context.Context, r *PublicRequest) (Object, int, erro out = assistWorkflow(t, r) case strings.Contains(r.Path, "/interactions"): require(r.Method == "POST" && strings.HasSuffix(r.Path, "/confirm"), 404, "not_found") - out = confirmInteraction(t, r, uid, &dispatches) + out = confirmInteraction(t, r, uid) case strings.Contains(r.Path, "/comments"): switch r.Method { case "POST": diff --git a/internal/core/workflow_invocation.go b/internal/core/workflow_invocation.go new file mode 100644 index 000000000..ba0137cc7 --- /dev/null +++ b/internal/core/workflow_invocation.go @@ -0,0 +1,27 @@ +package core + +// workflowInvocationInput isolates the pending form-to-execution contract: only Start and global +// variables are execution input. Platform launch fields and Issue context remain on the IssueRun. +// A platform-named key declared by the author is still a workflow variable and must survive. +func workflowInvocationInput(graph, values Object) Object { + input := Object{} + fields := workflowInputFields(graph) + for index := range fields { + if value, ok := values[fields[index].Key]; ok { + input[fields[index].Key] = value + } + } + return input +} + +// enqueueWorkflowInvocation creates both sides of an invocation inside the caller's transaction. +// Snapshot selection/admission and verified initiator resolution remain at the entry point. Unlike +// the direct development simulator, an Issue invocation always freezes its real executor bindings. +func enqueueWorkflowInvocation(t *transaction, tid, iid string, workflow, snapshot, values, issueInput Object, triggerKind string, triggerRef any, uid string) Object { + executionInput := workflowInvocationInput(snapshot.O("graph"), values) + i := issue(t, tid, iid) + invocation := insertWorkflowRun(t, tid, workflow.S("id"), snapshot, executionInput, workflow.S("name"), uid, i.S("projectRef")) + freezeWorkflowRunExecutors(t, invocation.S("id"), uid, snapshot.O("graph")) + freezeWorkflowSnapshot(issueInput, snapshot.S("id")) + return enqueueRunWithInvocation(t, tid, iid, "workflow", workflow.S("id"), issueInput, triggerKind, triggerRef, "user", uid, invocation.S("id")) +} diff --git a/internal/core/workflow_issue_projection.go b/internal/core/workflow_issue_projection.go new file mode 100644 index 000000000..9931b4c5d --- /dev/null +++ b/internal/core/workflow_issue_projection.go @@ -0,0 +1,106 @@ +package core + +import ( + "strings" + "unicode/utf8" +) + +// workflowIssueOutputSummary selects existing output-node results or the run error. Callers bound +// the UTF-8 message before projection so execution settlement cannot fail on activity size alone. +func workflowIssueOutputSummary(graph, states Object, runErr string) string { + message := strings.TrimSpace(runErr) + if message == "" { + outputs := Object{} + for _, node := range readWorkflowGraph(graph).Nodes { + if state := states.O(node.ID); node.Kind == "output" && state.S("status") == "succeeded" && len(state.O("output")) > 0 { + outputs[node.ID] = state.O("output") + } + } + if len(outputs) > 0 { + message = jsonText(outputs) + } + } + if len(message) > 20000 { + end := 20000 + for !utf8.RuneStart(message[end]) { + end-- + } + message = message[:end] + } + return message +} + +// projectWorkflowRunToIssueRun mirrors the committed-in-this-transaction workflow state onto its +// optional IssueRun. Execution callers invoke it only after a successful state write, in the same +// Store.transact boundary, so a projection or activity failure rolls back the workflow transition. +// Direct workflow runs have no IssueRun and are a no-op. The caller supplies the execution's output +// summary because per-node output has no canonical summary field in workflow_runs. awaiting_input +// has no IssueRun counterpart; it leaves the current projection untouched without blocking the +// caller's workflow transition. +func projectWorkflowRunToIssueRun(t *transaction, tid, workflowRunID, outputSummary string) Object { + w := t.one("SELECT * FROM workflow_runs WHERE id=$1 AND tenant_id=$2 AND deleted_at IS NULL", workflowRunID, tid) + require(w != nil, 404, "not_found") + i := t.one("SELECT * FROM issue_runs WHERE workflow_invocation_ref=$1 AND tenant_id=$2 AND executor_type='workflow' AND deleted_at IS NULL FOR UPDATE", workflowRunID, tid) + if i == nil { + return nil + } + require(i.S("executorId") == w.S("workflowId"), 409, "workflow_run_projection_conflict") + var status, action string + switch w.S("status") { + case "pending": + status = "queued" + case "running": + status, action = "running", "run.started" + case "awaiting_input": + return run(t, tid, i.S("issueId"), i.S("id")) + case "succeeded": + status, action = "completed", "run.completed" + case "failed": + status, action = "failed", "run.failed" + case "cancelled": + status, action = "cancelled", "run.cancelled" + default: + require(false, 409, "workflow_run_state_not_projectable") + } + if i.S("status") == status { + return run(t, tid, i.S("issueId"), i.S("id")) + } + // A terminal IssueRun cannot be reopened or silently given a different outcome. Recovery must + // project the authoritative current WorkflowRun, never replay a stale transition argument. + require(i.S("status") == "queued" || i.S("status") == "dispatched" || i.S("status") == "running", 409, "workflow_run_projection_conflict") + require(status != "queued", 409, "workflow_run_projection_conflict") + terminal := status == "completed" || status == "failed" || status == "cancelled" + if status == "running" { + require(w.S("startedAt") != "", 409, "workflow_run_projection_incomplete") + } + if terminal { + require(w.S("finishedAt") != "", 409, "workflow_run_projection_incomplete") + } + message := strings.TrimSpace(outputSummary) + require(len(message) <= 20000, 400, "invalid_input") + reason := "" + if status == "failed" || status == "cancelled" { + reason = w.S("error") + } + t.exec(`UPDATE issue_runs i SET status=$3, started_at=w.started_at, + completed_at=w.finished_at, error=$4, failure_reason=$4, + version=i.version+1, updated_at=now() + FROM workflow_runs w WHERE i.id=$1 AND i.tenant_id=$2 + AND w.id=i.workflow_invocation_ref AND w.tenant_id=i.tenant_id`, i.S("id"), tid, status, reason) + details := runActivityDetails(i.S("id"), "workflow", i.S("executorId"), Object{"workflowInvocationRef": workflowRunID}) + // A scheduler may finish a workflow within one transaction, before an intermediate running + // projection exists. Only authoritative start evidence permits the missing lifecycle activity; + // cancellation before execution must never fabricate a start. + if terminal && (i.S("status") == "queued" || i.S("status") == "dispatched") && w.S("startedAt") != "" { + appendActivity(t, tid, i.S("issueId"), "system", nil, "run.started", details) + } + if terminal && message != "" { + messageDetails := runActivityDetails(i.S("id"), "workflow", i.S("executorId"), Object{"workflowInvocationRef": workflowRunID, "message": message}) + appendActivity(t, tid, i.S("issueId"), "system", nil, "run.message", messageDetails) + } + if status == "failed" { + details["failureReason"] = reason + } + appendActivity(t, tid, i.S("issueId"), "system", nil, action, details) + return run(t, tid, i.S("issueId"), i.S("id")) +} diff --git a/internal/core/workflow_issue_projection_test.go b/internal/core/workflow_issue_projection_test.go new file mode 100644 index 000000000..461f956b2 --- /dev/null +++ b/internal/core/workflow_issue_projection_test.go @@ -0,0 +1,272 @@ +package core + +import ( + "context" + "reflect" + "testing" +) + +type workflowProjectionScene struct { + tenant, issue, workflow, workflowRun, issueRun string +} + +// The scene stages a linked run only; scheduler invocation is deliberately outside this test's +// boundary. Every projection below consumes the real WorkflowRun row in its caller's transaction. +func seedWorkflowProjection(t *testing.T, store *Store) workflowProjectionScene { + t.Helper() + s := workflowProjectionScene{newID(), newID(), newID(), newID(), newID()} + _, err := store.transact(context.Background(), func(tx *transaction) Object { + user, snapshot := newID(), newID() + tx.exec("INSERT INTO users(id,display_name,status) VALUES($1,'projection','active')", user) + tx.exec("INSERT INTO tenants(id,name,status) VALUES($1,'projection','active')", s.tenant) + tx.exec("INSERT INTO tenant_memberships(tenant_id,user_id,role,status) VALUES($1,$2,'admin','active')", s.tenant, user) + tx.exec("INSERT INTO issues(id,tenant_id,creator_user_id,title,number) VALUES($1,$2,$3,'projection',1)", s.issue, s.tenant, user) + tx.exec("INSERT INTO workflows(id,tenant_id,name) VALUES($1,$2,'projection')", s.workflow, s.tenant) + tx.exec("INSERT INTO workflow_snapshots(id,tenant_id,workflow_id,version,name,graph) VALUES($1,$2,$3,1,'projection','{}')", snapshot, s.tenant, s.workflow) + tx.exec("INSERT INTO workflow_runs(id,tenant_id,workflow_id,snapshot_id,name) VALUES($1,$2,$3,$4,'projection')", s.workflowRun, s.tenant, s.workflow, snapshot) + tx.exec("INSERT INTO issue_runs(id,tenant_id,issue_id,executor_type,executor_id,workflow_invocation_ref) VALUES($1,$2,$3,'workflow',$4,$5)", s.issueRun, s.tenant, s.issue, s.workflow, s.workflowRun) + appendActivity(tx, s.tenant, s.issue, "user", user, "run.enqueued", runActivityDetails(s.issueRun, "workflow", s.workflow, nil)) + return Object{} + }) + if err != nil { + t.Fatalf("seed linked workflow: %v", err) + } + return s +} + +func projectWorkflowState(t *testing.T, store *Store, s workflowProjectionScene, state, summary string) Object { + t.Helper() + out, err := store.transact(context.Background(), func(tx *transaction) Object { + tx.exec(`UPDATE workflow_runs SET status=$2, + started_at=CASE WHEN $2='pending' THEN NULL ELSE '2026-10-15 10:00:00+00'::timestamptz END, + finished_at=CASE WHEN $2 IN ('succeeded','failed','cancelled') THEN '2026-10-15 10:01:00+00'::timestamptz ELSE NULL END, + error=CASE WHEN $2 IN ('failed','cancelled') THEN 'execution reason' ELSE '' END + WHERE id=$1 AND tenant_id=$3`, s.workflowRun, state, s.tenant) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, summary) + }) + if err != nil { + t.Fatalf("project %s: %v", state, err) + } + return out +} + +func workflowProjectionActivities(t *testing.T, store *Store, s workflowProjectionScene) []Object { + t.Helper() + out, err := store.transact(context.Background(), func(tx *transaction) Object { + return Object{"items": tx.list("SELECT * FROM issue_activities WHERE tenant_id=$1 AND issue_id=$2 ORDER BY seq", s.tenant, s.issue)} + }) + if err != nil { + t.Fatalf("activities: %v", err) + } + return out["items"].([]Object) +} + +func TestWorkflowIssueProjectionStatesAndActivities(t *testing.T) { + store := dispatcherDB(t) + for _, tc := range []struct{ state, status, action string }{ + {"succeeded", "completed", "run.completed"}, + {"failed", "failed", "run.failed"}, + {"cancelled", "cancelled", "run.cancelled"}, + } { + t.Run(tc.state, func(t *testing.T) { + s := seedWorkflowProjection(t, store) + pending := projectWorkflowState(t, store, s, "pending", "") + if pending.S("status") != "queued" || pending.N("version") != 1 { + t.Fatalf("pending must stay queued without a write: %v", pending) + } + running := projectWorkflowState(t, store, s, "running", "") + if running.S("status") != "running" || running.S("startedAt") == "" || running.S("completedAt") != "" { + t.Fatalf("running projection: %v", running) + } + repeatedRunning, err := store.transact(context.Background(), func(tx *transaction) Object { + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "") + }) + if err != nil || !reflect.DeepEqual(running, repeatedRunning) || len(workflowProjectionActivities(t, store, s)) != 2 { + t.Fatalf("running repeat changed row or activities: err=%v before=%v after=%v", err, running, repeatedRunning) + } + out := projectWorkflowState(t, store, s, tc.state, " execution output ") + if out.S("status") != tc.status { + t.Fatalf("status = %s, want %s", out.S("status"), tc.status) + } + var timestampsMatch bool + if err := store.Pool.QueryRow(`SELECT i.started_at=w.started_at AND i.completed_at=w.finished_at + FROM issue_runs i JOIN workflow_runs w ON w.id=i.workflow_invocation_ref WHERE i.id=$1`, s.issueRun).Scan(×tampsMatch); err != nil || !timestampsMatch { + t.Fatalf("timestamps must mirror source: match=%v err=%v", timestampsMatch, err) + } + wantReason := "execution reason" + if tc.state == "succeeded" { + wantReason = "" + } + if out.S("failureReason") != wantReason || out.S("error") != wantReason { + t.Fatalf("terminal reason: %v", out) + } + activities := workflowProjectionActivities(t, store, s) + var actions []string + for index, a := range activities { + actions = append(actions, a.S("action")) + if index == 0 { + continue + } + if a.S("actorType") != "system" || a["actorId"] != nil || a.O("details").S("workflowInvocationRef") != s.workflowRun || a.O("details").S("runId") != s.issueRun { + t.Fatalf("system activity provenance: %v", a) + } + } + if !reflect.DeepEqual(actions, []string{"run.enqueued", "run.started", "run.message", tc.action}) || activities[2].O("details").S("message") != "execution output" { + t.Fatalf("activity sequence or summary: %v", activities) + } + if tc.state == "failed" && activities[3].O("details").S("failureReason") != wantReason { + t.Fatalf("failed activity reason: %v", activities[3]) + } + var comments int + if err := store.Pool.QueryRow("SELECT count(*) FROM issue_comments WHERE issue_id=$1", s.issue).Scan(&comments); err != nil || comments != 0 { + t.Fatalf("workflow output must not author any comment: count=%d err=%v", comments, err) + } + repeated, err := store.transact(context.Background(), func(tx *transaction) Object { + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "a replay must not replace the first summary") + }) + if err != nil || !reflect.DeepEqual(out, repeated) || !reflect.DeepEqual(activities, workflowProjectionActivities(t, store, s)) { + t.Fatalf("repeat changed row or activities: err=%v before=%v after=%v", err, out, repeated) + } + }) + } +} + +func TestWorkflowIssueProjectionDirectTerminalIncludesStart(t *testing.T) { + store := dispatcherDB(t) + for _, initial := range []string{"queued", "dispatched"} { + for _, tc := range []struct{ state, action string }{ + {"succeeded", "run.completed"}, + {"failed", "run.failed"}, + {"cancelled", "run.cancelled"}, + } { + t.Run(initial+"_to_"+tc.state, func(t *testing.T) { + s := seedWorkflowProjection(t, store) + if _, err := store.Pool.Exec("UPDATE issue_runs SET status=$2 WHERE id=$1", s.issueRun, initial); err != nil { + t.Fatal(err) + } + out := projectWorkflowState(t, store, s, tc.state, "direct terminal output") + activities := workflowProjectionActivities(t, store, s) + var actions []string + for _, activity := range activities { + actions = append(actions, activity.S("action")) + } + if !reflect.DeepEqual(actions, []string{"run.enqueued", "run.started", "run.message", tc.action}) { + t.Fatalf("direct terminal must retain lifecycle ordering: %v", actions) + } + if activities[1].S("actorType") != "system" || activities[1]["actorId"] != nil || activities[1].O("details").S("workflowInvocationRef") != s.workflowRun { + t.Fatalf("supplemented start provenance: %v", activities[1]) + } + repeated, err := store.transact(context.Background(), func(tx *transaction) Object { + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "direct terminal output") + }) + if err != nil || !reflect.DeepEqual(out, repeated) || !reflect.DeepEqual(activities, workflowProjectionActivities(t, store, s)) { + t.Fatalf("direct terminal repeat changed row or activities: err=%v", err) + } + }) + } + } +} + +func TestWorkflowIssueProjectionAwaitingInputPreservesIssueRunAndCommits(t *testing.T) { + store := dispatcherDB(t) + for _, initial := range []string{"pending", "running"} { + t.Run(initial, func(t *testing.T) { + s := seedWorkflowProjection(t, store) + before := projectWorkflowState(t, store, s, initial, "") + activities := workflowProjectionActivities(t, store, s) + out, err := store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("UPDATE workflow_runs SET status='awaiting_input',version=version+1,input=$2 WHERE id=$1", s.workflowRun, jsonText(Object{"commitWitness": true})) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "no output activity while waiting") + }) + if err != nil || !reflect.DeepEqual(before, out) || !reflect.DeepEqual(activities, workflowProjectionActivities(t, store, s)) { + t.Fatalf("awaiting_input must preserve IssueRun and activity history: err=%v before=%v after=%v", err, before, out) + } + var status string + var committed bool + if err := store.Pool.QueryRow("SELECT status,input->>'commitWitness'='true' FROM workflow_runs WHERE id=$1", s.workflowRun).Scan(&status, &committed); err != nil || status != "awaiting_input" || !committed { + t.Fatalf("caller's workflow update did not commit: status=%s witness=%v err=%v", status, committed, err) + } + }) + } +} + +func TestWorkflowIssueProjectionCancellationBeforeStart(t *testing.T) { + store := dispatcherDB(t) + s := seedWorkflowProjection(t, store) + _, err := store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("UPDATE workflow_runs SET status='cancelled',finished_at=now(),error='cancelled before start' WHERE id=$1", s.workflowRun) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "") + }) + if err != nil { + t.Fatal(err) + } + activities := workflowProjectionActivities(t, store, s) + if len(activities) != 2 || activities[1].S("action") != "run.cancelled" { + t.Fatalf("cancellation must not fabricate a start or output: %v", activities) + } + var complete, unstarted bool + if err := store.Pool.QueryRow("SELECT completed_at IS NOT NULL,started_at IS NULL FROM issue_runs WHERE id=$1", s.issueRun).Scan(&complete, &unstarted); err != nil || !complete || !unstarted { + t.Fatalf("cancelled timestamps: complete=%v unstarted=%v err=%v", complete, unstarted, err) + } +} + +func TestWorkflowIssueProjectionActivityFailureRollsBack(t *testing.T) { + store := dispatcherDB(t) + s := seedWorkflowProjection(t, store) + if _, err := store.Pool.Exec("ALTER TABLE issue_activities ADD CONSTRAINT reject_projection_failure CHECK(action <> 'run.failed')"); err != nil { + t.Fatal(err) + } + _, err := store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("UPDATE workflow_runs SET status='failed',finished_at=now(),error='failure' WHERE id=$1", s.workflowRun) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "must roll back too") + }) + if err == nil { + t.Fatal("activity failure must abort caller transaction") + } + var workflowState, issueState string + var version int + if err := store.Pool.QueryRow(`SELECT w.status,i.status,i.version FROM workflow_runs w + JOIN issue_runs i ON i.workflow_invocation_ref=w.id WHERE w.id=$1`, s.workflowRun).Scan(&workflowState, &issueState, &version); err != nil { + t.Fatal(err) + } + if workflowState != "pending" || issueState != "queued" || version != 1 || len(workflowProjectionActivities(t, store, s)) != 1 { + t.Fatalf("partial transition escaped rollback: workflow=%s issue=%s version=%d", workflowState, issueState, version) + } +} + +func TestWorkflowIssueProjectionScopeAndInvalidStates(t *testing.T) { + store := dispatcherDB(t) + s := seedWorkflowProjection(t, store) + _, err := store.transact(context.Background(), func(tx *transaction) Object { + return projectWorkflowRunToIssueRun(tx, newID(), s.workflowRun, "") + }) + if ErrorCode(err).Code != "not_found" { + t.Fatalf("foreign tenant: %v", err) + } + for _, state := range []string{"running", "failed"} { + _, err := store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("UPDATE workflow_runs SET status=$2 WHERE id=$1", s.workflowRun, state) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "") + }) + if err == nil { + t.Fatalf("state %s without required timestamps must fail", state) + } + } + projectWorkflowState(t, store, s, "succeeded", "") + _, err = store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("UPDATE workflow_runs SET status='failed',error='different outcome' WHERE id=$1", s.workflowRun) + return projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "") + }) + if ErrorCode(err).Code != "workflow_run_projection_conflict" { + t.Fatalf("terminal rewrite: %v", err) + } + // A direct WorkflowRun remains valid without an IssueRun and must not emit issue activities. + _, err = store.transact(context.Background(), func(tx *transaction) Object { + tx.exec("DELETE FROM issue_runs WHERE id=$1", s.issueRun) + require(projectWorkflowRunToIssueRun(tx, s.tenant, s.workflowRun, "") == nil, 500, "internal_error") + return Object{} + }) + if err != nil { + t.Fatalf("unlinked direct run: %v", err) + } +} diff --git a/internal/core/workflow_run.go b/internal/core/workflow_run.go index 3b52e0eb8..4c5210524 100644 --- a/internal/core/workflow_run.go +++ b/internal/core/workflow_run.go @@ -122,20 +122,19 @@ func createWorkflowRun(t *transaction, r *PublicRequest, uid string) Object { } s := admitWorkflowRun(t, r.TenantID, r.WorkflowID, r.Body.S("snapshotId"), uid) input := r.Body.O("input") - requireWorkflowRunInput(s.O("graph"), input) - id := newID() // The invocation identity freezes here (D3): created_by is the aggregate's actor for the whole // run — the run Workspace lifecycle reads it instead of re-resolving who pressed Run. The // authenticated Identity, not a /users path parameter, is who that is. The project freezes in the // same statement, so the two halves of the identity can never disagree about who ran what where. require(r.Identity != nil, 401, "user_credential_required") createdBy := identityWithAlias(t, r.Identity).S("id") - projectID := resolveWorkflowRunProject(t, r, uid) - pluginVersion := resolveWorkflowRunPluginVersion(t, r.TenantID, projectID) - t.exec("INSERT INTO workflow_runs(id,tenant_id,workflow_id,snapshot_id,name,status,input,created_by,project_id,plugin_version) VALUES($1,$2,$3,$4,$5,$6,$7,$8,$9,$10)", id, r.TenantID, r.WorkflowID, s.S("id"), name, "pending", jsonText(input), createdBy, projectID, pluginVersion) + created := insertWorkflowRun(t, r.TenantID, r.WorkflowID, s, input, name, createdBy, r.Body.S("projectId")) + id := created.S("id") if t.simulator != nil { nodeStates, rounds, status := t.simulator.SimulateWorkflowRun(s.O("graph"), input) - t.exec("UPDATE workflow_runs SET node_states=$1,rounds=$2,status=$3,started_at=now(),finished_at=now(),version=version+1,updated_at=now() WHERE id=$4", jsonText(nodeStates), jsonText(rounds), status, id) + if t.execRows("UPDATE workflow_runs SET node_states=$1,rounds=$2,status=$3,started_at=now(),finished_at=now(),version=version+1,updated_at=now() WHERE id=$4", jsonText(nodeStates), jsonText(rounds), status, id) > 0 { + projectWorkflowRunToIssueRun(t, r.TenantID, id, workflowIssueOutputSummary(s.O("graph"), nodeStates, "")) + } } else { // The run-creation gate (mappings decision D2): every slot the snapshot references must // resolve against the invoker's personal configuration before the run exists, and each @@ -147,12 +146,25 @@ func createWorkflowRun(t *transaction, r *PublicRequest, uid string) Object { return composeWorkflowRun(t, t.one("SELECT * FROM workflow_runs WHERE id=$1 AND tenant_id=$2", id, r.TenantID)) } -// resolveWorkflowRunProject authorizes and returns the Project a direct run freezes (D3). +// insertWorkflowRun is the shared creation seam after admission. The caller owns the transaction +// and the admitted snapshot. Every entry freezes the verified creator, authorized Project and that +// Project's plugin version here. Callers freeze executor slots before committing; only the direct +// development simulator may instead fill a completed trace without bindings. +func insertWorkflowRun(t *transaction, tid, wid string, snapshot, input Object, name, uid, projectID string) Object { + require(snapshot.S("tenantId") == tid && snapshot.S("workflowId") == wid, 404, "not_found") + requireWorkflowRunInput(snapshot.O("graph"), input) + projectID = resolveWorkflowRunProject(t, tid, uid, projectID) + pluginVersion := resolveWorkflowRunPluginVersion(t, tid, projectID) + id := newID() + t.exec("INSERT INTO workflow_runs(id,tenant_id,workflow_id,snapshot_id,name,status,input,created_by,project_id,plugin_version) VALUES($1,$2,$3,$4,$5,'pending',$6,$7,$8,$9)", id, tid, wid, snapshot.S("id"), name, jsonText(input), uid, projectID, pluginVersion) + return t.one("SELECT * FROM workflow_runs WHERE id=$1 AND tenant_id=$2", id, tid) +} + +// resolveWorkflowRunProject authorizes and returns the Project a run freezes (D3). // The Project is the one thing that says where the run's Workspace may provision — // createWorkflowRunWorkspace refuses a run without one — and it is stated by the caller rather than -// inferred, because nothing else knows it: a workflow document belongs to a tenant, not to a Project, -// so there is no project to default to and defaulting to one would place a run's Workspace, its clone -// and its whole checkout in a Project nobody named. +// inferred: a direct run names it explicitly, while an Issue invocation uses its Issue's project_ref. +// A workflow document belongs to a tenant, not to a Project, so it supplies no default of its own. // // The value is authorized by the same helper every project-scoped route uses, so an unknown, foreign, // archived or foreign-space project is the same 404 and a run can never be created against a Project @@ -169,20 +181,19 @@ func createWorkflowRun(t *transaction, r *PublicRequest, uid string) Object { // needed one yet — it is an identity-incomplete aggregate that can never provision the Workspace its // agent nodes require, so accepting it would persist a row that is born unable to finish and would // only ever report the gap later, from inside an execution that cannot proceed. This is the same -// reason created_by is required one line up: project_id is the "where" half of the frozen identity, +// reason created_by is required: project_id is the "where" half of the frozen identity, // and freezing half an identity is not freezing one. A graph that genuinely needs no Workspace still // names the Project it would provision in, which costs its caller one choice and keeps every run's // identity whole. // // The gate lives on this resolver rather than on the one caller so that every entry which creates a -// run — the editor today, the @ confirm and the manual run once they are linked to the same creation -// point — makes one shared decision in the creation transaction instead of each writing its own +// run — the editor, @ confirm and manual run — makes one shared decision in the creation transaction +// instead of each writing its own // "no Project" rule. Refusal reuses the code createWorkflowRunWorkspace already raises for the same // condition, so a missing Project reads the same whether it is caught here or there. -func resolveWorkflowRunProject(t *transaction, r *PublicRequest, uid string) any { - pid := r.Body.S("projectId") +func resolveWorkflowRunProject(t *transaction, tid, uid, pid string) string { require(pid != "", 409, "workflow_run_project_required") - p := project(t, r.TenantID, uid, pid) + p := project(t, tid, uid, pid) require(p.S("lifecycle") != "deleting", 409, "resource_unavailable") return p.S("id") } diff --git a/internal/core/workflow_run_session.go b/internal/core/workflow_run_session.go index 6f286d124..97c851960 100644 --- a/internal/core/workflow_run_session.go +++ b/internal/core/workflow_run_session.go @@ -251,8 +251,7 @@ func commitWorkflowAgentTerminal(t *transaction, runID string, version int64, no return states, false default: // The node succeeded; downstream waves continue in this same pass. - settleWorkflowRunStates(t, runID, version, "running", "", states) - return states, true + return states, settleWorkflowRunStates(t, runID, version, "running", "", states) } } @@ -262,8 +261,9 @@ func commitWorkflowAgentTerminal(t *transaction, runID string, version int64, no // Declaring delete on a busy Project returns busy — a later pass re-declares, so a terminal run // is never left with a live Workspace forever. func finishWorkflowRun(t *transaction, runID string, version int64, status, runErr string, states map[string]workflowNodeState) { - settleWorkflowRunStates(t, runID, version, status, runErr, states) - finishWorkflowRunCleanup(t, runID) + if settleWorkflowRunStates(t, runID, version, status, runErr, states) { + finishWorkflowRunCleanup(t, runID) + } } // finishWorkflowRunCleanup deletes the run's unregistered execution work and re-declares the @@ -392,7 +392,9 @@ func startWorkflowAgentSession(t *transaction, run Object, node workflowNodeView return } states[node.ID] = workflowNodeState{Status: "running", DisplayName: workflowNodeDisplayName(node), StartedAt: nowRFC3339()} - settleWorkflowRunStates(t, runID, version, "running", "", states) + if !settleWorkflowRunStates(t, runID, version, "running", "", states) { + return + } enqueueExecutionWork(t, runID, "agent_session", spec, target, node.ID, time.Time{}) } diff --git a/internal/core/workflow_scheduler.go b/internal/core/workflow_scheduler.go index 87252b8ec..34043f7a5 100644 --- a/internal/core/workflow_scheduler.go +++ b/internal/core/workflow_scheduler.go @@ -495,6 +495,7 @@ func advanceWorkflowRun(t *transaction, runID string, unreachableAfter time.Dura WHERE id=$1 AND status='pending' AND deleted_at IS NULL`, runID) == 0 { return } + projectWorkflowRunToIssueRun(t, run.S("tenantId"), runID, "") run = t.one(` SELECT r.*, s.graph, (EXTRACT(EPOCH FROM r.created_at)*1000)::bigint AS created_at_ms FROM workflow_runs r @@ -619,25 +620,33 @@ func workflowInactiveStates(graph workflowGraphView, states map[string]workflowN // the first terminal write, so a later pass re-declaring a terminal status cannot move the moment // the run actually stopped. started_at is not this function's business: it is set once, where the // run first leaves `pending`. -func settleWorkflowRunStates(t *transaction, runID string, version int64, status, runErr string, states map[string]workflowNodeState) { +func settleWorkflowRunStates(t *transaction, runID string, version int64, status, runErr string, states map[string]workflowNodeState) bool { // The statuses a run never leaves. `pending`, `running` and `awaiting_input` are the others the // column admits, and all three are states a run is still in. terminal := status == "succeeded" || status == "failed" || status == "cancelled" + var applied int64 + encoded := Object{} if states == nil { - t.exec(`UPDATE workflow_runs SET status=$2, error=$3, + applied = t.execRows(`UPDATE workflow_runs SET status=$2, error=$3, finished_at=CASE WHEN $5 THEN COALESCE(finished_at,now()) ELSE finished_at END, version=version+1, updated_at=now() WHERE id=$1 AND version=$4 AND deleted_at IS NULL`, runID, status, runErr, version, terminal) - return + } else { + for id, state := range states { + encoded[id] = state.object() + } + applied = t.execRows(`UPDATE workflow_runs SET node_states=$2, status=$3, error=$4, + finished_at=CASE WHEN $6 THEN COALESCE(finished_at,now()) ELSE finished_at END, + version=version+1, updated_at=now() + WHERE id=$1 AND version=$5 AND deleted_at IS NULL`, runID, jsonText(encoded), status, runErr, version, terminal) } - encoded := Object{} - for id, state := range states { - encoded[id] = state.object() + if applied == 0 { + return false } - t.exec(`UPDATE workflow_runs SET node_states=$2, status=$3, error=$4, - finished_at=CASE WHEN $6 THEN COALESCE(finished_at,now()) ELSE finished_at END, - version=version+1, updated_at=now() - WHERE id=$1 AND version=$5 AND deleted_at IS NULL`, runID, jsonText(encoded), status, runErr, version, terminal) + run := t.one(`SELECT r.tenant_id,s.graph FROM workflow_runs r + JOIN workflow_snapshots s ON s.id=r.snapshot_id WHERE r.id=$1`, runID) + projectWorkflowRunToIssueRun(t, run.S("tenantId"), runID, workflowIssueOutputSummary(run.O("graph"), encoded, runErr)) + return true } // workflowRunVersion re-reads the row's version after a mid-pass settle bumped it: one pass commits diff --git a/internal/core/workflow_scheduler_db_test.go b/internal/core/workflow_scheduler_db_test.go index 4da3bfb32..368f56a1b 100644 --- a/internal/core/workflow_scheduler_db_test.go +++ b/internal/core/workflow_scheduler_db_test.go @@ -9,9 +9,12 @@ package core import ( "context" "encoding/json" + "reflect" "strings" + "sync" "testing" "time" + "unicode/utf8" "github.com/google/uuid" ) @@ -185,6 +188,173 @@ func TestWorkflowAdvanceConditionOnUnsetKickoffReadsAbsent(t *testing.T) { if branch := wfSelectedBranch(t, store, runID); branch != "absent" { t.Fatalf("condition selected %q; want absent — an unset kickoff read as present", branch) } + var activities, linked int + if err := store.Pool.QueryRow(`SELECT (SELECT count(*) FROM issue_activities), + (SELECT count(*) FROM issue_runs WHERE workflow_invocation_ref=$1)`, runID).Scan(&activities, &linked); err != nil || activities != 0 || linked != 0 { + t.Fatalf("direct run created collaboration records: activities=%d linked=%d err=%v", activities, linked, err) + } +} + +func TestWorkflowAdvanceProjectsConcurrentPassesOnce(t *testing.T) { + store := dispatcherDB(t) + s := seedWorkflowProjection(t, store) + graph := wfGraph([]Object{wfNode("s", "start", ""), wfNode("o", "output", "")}, [][3]string{{"s", "o", ""}}) + if _, err := store.Pool.Exec(`UPDATE workflow_snapshots SET graph=$2::jsonb + WHERE id=(SELECT snapshot_id FROM workflow_runs WHERE id=$1)`, s.workflowRun, jsonText(graph)); err != nil { + t.Fatal(err) + } + start := make(chan struct{}) + var wg sync.WaitGroup + errs := make([]error, 2) + for i := range errs { + wg.Add(1) + go func() { + defer wg.Done() + <-start + errs[i] = store.AdvanceWorkflowRunsOnce(context.Background()) + }() + } + close(start) + wg.Wait() + for _, err := range errs { + if err != nil { + t.Fatal(err) + } + } + if err := store.AdvanceWorkflowRunsOnce(context.Background()); err != nil { + t.Fatal(err) + } + var status string + var version int + if err := store.Pool.QueryRow("SELECT status,version FROM issue_runs WHERE id=$1", s.issueRun).Scan(&status, &version); err != nil || status != "completed" || version != 3 { + t.Fatalf("linked run: status=%s version=%d err=%v", status, version, err) + } + activities := workflowProjectionActivities(t, store, s) + actions := make([]string, len(activities)) + for i, activity := range activities { + actions[i] = activity.S("action") + } + if !reflect.DeepEqual(actions, []string{"run.enqueued", "run.started", "run.completed"}) { + t.Fatalf("concurrent/repeated passes emitted %v", actions) + } +} + +func TestWorkflowSettleLostCASDoesNotProject(t *testing.T) { + store := dispatcherDB(t) + for _, withStates := range []bool{false, true} { + name := "without-states" + if withStates { + name = "with-states" + } + t.Run(name, func(t *testing.T) { + s := seedWorkflowProjection(t, store) + // A concurrent writer won, but its IssueRun is deliberately still queued: calling + // projection after our lost CAS would now emit run.started from that winner's row. + if _, err := store.Pool.Exec("UPDATE workflow_runs SET status='running',started_at=now(),version=version+1 WHERE id=$1", s.workflowRun); err != nil { + t.Fatal(err) + } + var states map[string]workflowNodeState + if withStates { + states = map[string]workflowNodeState{"s": {Status: "succeeded"}} + } + _, err := store.transact(context.Background(), func(tx *transaction) Object { + if settleWorkflowRunStates(tx, s.workflowRun, 1, "failed", "stale failure", states) { + t.Fatal("stale version unexpectedly applied") + } + return Object{} + }) + if err != nil { + t.Fatal(err) + } + status, runErr, committed, version := readWorkflowRun(t, store, s.workflowRun) + if status != "running" || runErr != "" || len(committed) != 0 || version != 2 { + t.Fatalf("lost CAS changed winner: %s %q %v version=%d", status, runErr, committed, version) + } + var issueStatus string + if err := store.Pool.QueryRow("SELECT status FROM issue_runs WHERE id=$1", s.issueRun).Scan(&issueStatus); err != nil || issueStatus != "queued" { + t.Fatalf("lost CAS projected: status=%s err=%v", issueStatus, err) + } + if activities := workflowProjectionActivities(t, store, s); len(activities) != 1 { + t.Fatalf("lost CAS emitted activities: %v", activities) + } + }) + } +} + +func TestWorkflowSettleTruncatesIssueSummary(t *testing.T) { + store := dispatcherDB(t) + for _, source := range []string{"error", "output"} { + t.Run(source, func(t *testing.T) { + s := seedWorkflowProjection(t, store) + text := strings.Repeat("界", 7000) + status, runErr := "failed", text + var states map[string]workflowNodeState + if source == "output" { + graph := wfGraph([]Object{wfNode("s", "start", ""), wfNode("o", "output", "")}, [][3]string{{"s", "o", ""}}) + if _, err := store.Pool.Exec(`UPDATE workflow_snapshots SET graph=$2::jsonb + WHERE id=(SELECT snapshot_id FROM workflow_runs WHERE id=$1)`, s.workflowRun, jsonText(graph)); err != nil { + t.Fatal(err) + } + status, runErr = "succeeded", "" + states = map[string]workflowNodeState{"o": {Status: "succeeded", Output: Object{"text": text}}} + } + _, err := store.transact(context.Background(), func(tx *transaction) Object { + if !settleWorkflowRunStates(tx, s.workflowRun, 1, status, runErr, states) { + t.Fatal("settle did not apply") + } + return Object{} + }) + if err != nil { + t.Fatalf("oversized summary rolled back settlement: %v", err) + } + activities := workflowProjectionActivities(t, store, s) + if len(activities) != 3 || activities[1].S("action") != "run.message" { + t.Fatalf("terminal summary activities: %v", activities) + } + message := activities[1].O("details").S("message") + if len(message) < 19997 || len(message) > 20000 || !utf8.ValidString(message) { + t.Fatalf("summary bytes=%d valid UTF-8=%v", len(message), utf8.ValidString(message)) + } + actual, actualErr, _, version := readWorkflowRun(t, store, s.workflowRun) + if actual != status || actualErr != runErr || version != 2 { + t.Fatalf("settle did not preserve execution result: status=%s error bytes=%d version=%d", actual, len(actualErr), version) + } + var issueStatus string + wantIssueStatus := "failed" + if status == "succeeded" { + wantIssueStatus = "completed" + } + if err := store.Pool.QueryRow("SELECT status FROM issue_runs WHERE id=$1", s.issueRun).Scan(&issueStatus); err != nil || issueStatus != wantIssueStatus { + t.Fatalf("terminal IssueRun: status=%s err=%v", issueStatus, err) + } + }) + } +} + +func TestWorkflowAdvanceProjectionFailureRollsBackClaim(t *testing.T) { + store := dispatcherDB(t) + s := seedWorkflowProjection(t, store) + if _, err := store.Pool.Exec("ALTER TABLE issue_activities ADD CONSTRAINT reject_scheduler_start CHECK(action <> 'run.started')"); err != nil { + t.Fatal(err) + } + _, err := store.transact(context.Background(), func(tx *transaction) Object { + advanceWorkflowRun(tx, s.workflowRun, store.DeliveryUnreachableAfter) + return Object{} + }) + if err == nil { + t.Fatal("failed projection must abort the scheduler transaction") + } + status, _, _, version := readWorkflowRun(t, store, s.workflowRun) + if status != "pending" || version != 1 { + t.Fatalf("projection failure left claimed workflow: %s version=%d", status, version) + } + var issueStatus string + if err := store.Pool.QueryRow("SELECT status FROM issue_runs WHERE id=$1", s.issueRun).Scan(&issueStatus); err != nil || issueStatus != "queued" { + t.Fatalf("projection failure changed IssueRun: status=%s err=%v", issueStatus, err) + } + if activities := workflowProjectionActivities(t, store, s); len(activities) != 1 { + t.Fatalf("rolled-back claim emitted activities: %v", activities) + } } // A ready agent node on a run with no project settles the run failed with an explicit reason: the