Repository navigation
feat(core): resolve executor slots and run workflow agent sessions - #72
Conversation
Zpc001
left a comment
There was a problem hiding this comment.
Request changes
会话落地、绑定冻结、授权按槽位校验这几块的做法我都认可;但这份 head 现在合不进 main,而且有一处和已批准的决策相反。
1. 合到 main 上编译不过(同名声明 + 参数个数)
main(#65,df507f0)已经带了槽位校验,本 PR 的 internal/core/workflow_slots.go 又声明了一遍。我把这个分支和上一个 PR 合到 df507f0 上,go build ./internal/core/ 直接失败:
internal/core/workflow_slots.go:25:6: rejectWith redeclared in this block
internal/core/store.go:72:6: other declaration of rejectWith
internal/core/workflow_slots.go:96:6: validateWorkflowSlots redeclared in this block
internal/core/workflow_graph.go:243:6: other declaration of validateWorkflowSlots
internal/core/workflow.go:31:25: not enough arguments in call to validateWorkflowSlots
have (Object)
want (Object, []any)
internal/core/workflow.go:115:24: not enough arguments in call to validateWorkflowSlots
internal/core/workflow.go:134:24: not enough arguments in call to validateWorkflowSlots
main 上是 validateWorkflowSlots(graph Object, nodes []any)(workflow_graph.go:243,由 requireValidWorkflowGraph 调用),本 PR 新写的是单参数版本。合并前请 rebase 到 df507f0:rejectWith 和槽位校验都用 main 已有的那两份,只保留本 PR 真正新增的部分(workflowGraphSlots、冻结、绑定)。
2. 同一笔创建事务里现在有两道槽位门禁
createWorkflowRun 合起来以后是这样的:main 的 admitWorkflowRun → requireWorkflowExecutorSlots(对任何命名槽位都报 slot_not_configured,因为个人映射还没实现),紧接着同一个事务里本 PR 的 freezeWorkflowRunExecutors 用新映射解析同一个图并冻结绑定。同一个请求会被两道语义不同的门禁判断,拒绝码也不同(workflow_executor_unconfigured / slot_not_configured 与新的 workflow_slots_unresolved)。这两道门禁只能留一道:本 PR 既然实现了映射,就应当把旧的那道一起删掉,并在 PR 描述里写明这次替换是对外的行为变化(specs 的 test-cases/cloud/model-access/executor-slot-mappings.md 现在按 df507f0 把 slot_not_configured 记成了门禁的直接证据,这份 PR 会让那几行证据换指向,需要同步)。
3. 保存路径上的槽位校验和已批准的决策相反
// internal/core/workflow.go graphText
normalized := normalizeWorkflowGraph(graph)
validateWorkflowSlots(normalized) // ← 每次保存都校验graphText 是保存路径。工作流执行根决策 D2 允许保存未完成的草稿,publishWorkflow 里的注释也写着"保存故意不设门禁"(internal/core/workflow.go:99,requireValidWorkflowGraph 只被发布调用),owner 在 specs#106 里把槽位决策 D2 的措辞改成的是"引用未声明的槽位,图非法,发布时拒绝……保存草稿不拒绝"。现在这样会让一份引用还没声明的槽位的草稿保存不了,与决策相反。请把校验只留在 publishWorkflow(restoreWorkflowSnapshot 是恢复成已发布过的快照,那处保留或去掉都行,但要说清楚)。normalizeWorkflowGraph 的 D5 剥离(保存时丢掉旧 agentConfig.executor)没有这个问题,那正是决策要求的。
4. 迁移编号撞了
internal/core/migrations/0036_executor_slot_bindings.sql 和另一个还开着的 PR #71 新加的 0036_workflow_invocation_reference.sql 用了同一个编号。
5. 冻结绑定只有直接运行一个入口
freezeWorkflowRunExecutors 的非测试调用点只有 internal/core/workflow_run.go:131,落在 createWorkflowRun 的 else(非模拟器)分支里,而 createWorkflowRun 的唯一调用点是 internal/core/public.go:320(POST .../runs)。会话侧是按绑定取执行器的(internal/core/workflow_run_session.go:202 查 run_model_bindings,取不到就报永久缺口),所以今天没问题——workflow_runs 的 INSERT 也只有这一处。但 manual IssueRun、@ Confirm、定时任务这几个入口接上时,必须都走同一个开始函数、在同一笔事务里冻结绑定,否则那些运行会在每个 agent 节点上报"永久缺口"。请确认落地顺序里这几条入口是这么接的(根决策 D3 的"三个入口共用一个开始函数")。
6. CI 两个 job 都 fail
check 和 frontend 都是 fail。frontend 挂在 "Generated client matches api/openapi.json"(frontend.yml:40 跑 npm run api:generate 后要求 git status --porcelain -- src/api 为空):本 PR 改了 api/openapi.json(+557)但 diff 里没有任何 frontend/src/api 文件,生成结果自然对不上。提交前跑一次生成、把 src/api 的产物一起提交即可。check 那边同一批 gofumpt 与 golangci-lint 的问题(public.go、model_slot_mappings_db_test.go、workflow_run_gate_db_test.go、workflow_slots_test.go、workflow_run_identity_db_test.go、workflow_scheduler_test.go)也请一并跑:
$ go run ./cmd/checkformat
internal/core/model_slot_mappings_db_test.go
internal/core/public.go
internal/core/workflow_run_gate_db_test.go
internal/core/workflow_run_identity_db_test.go
internal/core/workflow_scheduler_test.go
internal/core/workflow_slots_test.go
exit status 1
Refs ora-space/circus#141
🤖 Generated with Claude Code
73acb42 to
99f0a1e
Compare
5083f6b to
dd2c135
Compare
a84e435 to
fc944ca
Compare
Zpc001
left a comment
There was a problem hiding this comment.
复审结论:Request changes
复审对象:head fc944ca(git diff 43f5505..fc944ca,34 个自有文件)。已执行 go build ./...、go run ./cmd/checkformat、go tool golangci-lint run ./internal/core/...(0 issues),以及 session/gate/slot-mappings/control-projection 的针对性 PostgreSQL 用例,全部通过。
上一轮 6 条均已处理:
workflow_slots.go已不存在,rejectWith、validateWorkflowSlots各只保留一处。- 单一解析器
resolveWorkflowSlotExecutor与单一 409workflow_executor_unconfigured,缺口一次收集。 - 保存路径不再校验图,只有发布调用
requireValidWorkflowGraph,符合 D2。 - 迁移编号 0036/0038/0039 无冲突,0037 留给 #71。
- 冻结只挂在直接运行的创建事务上;#71 的
insertWorkflowRun接缝在本分支不可达,但 #71 review 时必须强制核对该半段。 - CI 两个 job 均恢复,生成的前端 client 已提交。
阻塞项
1. P1:节点可兑换同一运行内其它槽位的模型凭据 — internal/core/model_grants.go:86-101
liveWorkflowModelBinding 只要求绑定属于同一个 WorkflowRun(b.workflow_run_id=r.id),没有要求绑定就是这次会话执行被派发时冻结的那个。bindingId 来自 Node 请求体(internal/modelgateway/grants.go:68),模型 mTLS 证书只绑定 tenant/workspace/generation,不绑定槽位或绑定 id;同一运行里每个槽位各有一行 run_model_bindings(workflow_run_gate.go:29)。因此持有运行工作区证书的节点 A(槽 fast)可以用自己的 executionId 加节点 B(槽 slow)的 bindingId 换取 B 的凭据和模型,直接违反 ADR D4 与不变量 4。
请在 WHERE 中加上 AND e.input->>'modelBindingId' = b.id(并校验 e.input->>'kind'='agent_session'),再补一条负向用例:同一运行中 A 的 executionId + B 的 bindingId 必须 403。
2. 会话派发链在 ClaimWork 处断开 — internal/controlgrpc/executions.go:31
本 PR 第一次让 enqueueExecutionWork 真正承担 workflow run,但 ClaimWork 仍投影 runId。workflow 行的 run_id 为 NULL,返回的 OperationId 是空串,dispatchRunExecution 的 (run_id=$1 OR workflow_run_id=$1) 匹配不到,永久 409 dispatch_conflict。现有 session DB 用例直接调用 nodeDispatch(workflow_run_session_db_test.go:152,329),绕过了 ClaimWork。请改成 coalesce(run_id, workflow_run_id) 投影,并加一条 ClaimWork → RecordDispatch 的集成用例。
3. workflowSessionSpec 静默省略 modelBindingId,与本函数注释矛盾 — internal/core/workflow_run_session.go:217
绑定行缺失时不返回 named reason,而是继续启动一个没有模型绑定的会话,失败点被推迟到推理阶段;函数注释写的是“A permanent gap returns a named reason”。请在绑定缺失时让节点以 workflow_executor_unconfigured(或同义的有界原因)失败,不要启动无绑定的会话。
4. 0036/0038 迁移未固定 eol=lf — .gitattributes
本次新增 0036_executor_slot_bindings.sql 与 0038_workflow_execution_node.sql,只把 0039 加进了 allowlist。Migrate 对嵌入字节取 sha256,Windows 检出的 CRLF 会得到不同校验和;注释里“已应用迁移不改”的理由对这两个尚未被任何已发布版本应用的文件不成立。请一并固定。
5. slot mappings 写接口的 status 恒为 0 — internal/core/model_slot_mappings.go:12,34-65
命名返回值 status 在 PUT/DELETE 分支从未赋值,DELETE 还把它写进了 personal_model_idempotency.status。当前只因 Gin 把 0 当 200 才没暴露;重放命中时回放的是 0,任何直接使用该值的日志/度量/重放路径都会得到错误的成功码。请显式设置 200/201/204,与 OpenAPI 一致。
6. ADR 状态与核心用例证据未同步
PR 描述没有声明 20261015-executor-slots-replace-agent-model-ids.md 与 20261015-executor-slot-mappings-and-per-slot-run-bindings.md 的状态,也没有对应的 specs PR。相关用例仍停在实现前:executor-slots.md 4 处 Missing、executor-slot-mappings.md 8 处 Missing,且 executor-slots.md:49、executor-slot-mappings.md:31,50、publish-and-start-admission.md:48 仍把 409 描述为 slot_not_configured,而本 PR 已改为 workflow_executor_unconfigured + reason。按 cloud/AGENTS.md 和 circus 合并规则,请在合并前把证据与错误码同步并提交 specs PR。
非阻塞说明
- force stop 后永久悬挂:
workflowAgentSessionOutcome(workflow_run_session.go:63-87)对“已派发、result 为 NULL”的执行永久等待,而runtime_force_stop.go:70只写terminated_by_force_stop_id不写result。对正在运行的 agent 节点执行 force stop 后,运行无法进入终态、workspace 不释放。ADR 把 G-026 取消写入者放在 #75,本 PR 不必实现,但请在 #141 记录并确保 #75 的验收覆盖这一条,避免遗漏。 - OpenAPI 幂等键作用域描述:
internal/contract/openapi.go的启发式只特判/join/与/me/model-,新的/api/v1/me/executor-slot-mappings/:slotDELETE 被描述为“Scoped to tenant and user”,实际是纯用户作用域。属于契约漂移,建议顺手修正。 workflowSessionSpec的target参数完全未使用,建议删除或用于上述一致性校验。
已核实无问题
槽位解析不回退到 default/hint、冻结绑定唯一性与回滚、0036 的 owner 排他 CHECK 与部分唯一索引、0038 的每节点会话隔离与 Controller 投影排除 workflow_node_id、循环图在 publish 与 advance 双门拒绝、版本 CAS 重读与 finished_at COALESCE、插件版本冻结、workspace 释放、租户/owner 作用域、凭据只以引用形式存在、删除连接时按 connection_id 吊销全部冻结绑定。
fc944ca to
80cb401
Compare
…d stop silent gaps Review round on ora-space#72. Six fixes, each with the reason it is not cosmetic. The model grant's workflow branch matched a binding by its run alone. That is exact for an IssueRun, which owns one binding (0033's `run_id ... UNIQUE`), but a WorkflowRun owns one per executor slot since 0036 dropped that uniqueness, so the same test admitted every slot's binding at once. The binding id is the caller's to state — a model mTLS certificate is bound to a tenant, workspace and runtime generation, never to a slot — so node A holding `fast` could present node B's `slow` binding and be handed B's credential and model. The binding must now be the one this execution was dispatched with, read from `e.input->>'modelBindingId'`, which Cloud writes at enqueue time and no request body can restate. The new test proves the theft is refused and that the fix is what refuses it. `workflowSessionSpec` started a session even when the node's slot had no frozen binding. The run-creation gate freezes one per referenced slot, so a missing row is a run whose freeze never happened, not a state the next tick repairs — and starting anyway handed the Node a session with no model to run on, the silent downgrade the gate exists to prevent. The node now fails under the gate's own reason, `workflow_executor_unconfigured`. Its unused `target` parameter went with it. `.gitattributes` left 0036 and 0038 unpinned, on a comment that argued a database had already recorded their bytes. That was true of 0034, which is on main, and false of these: `origin/main` stops at 0034, so nothing released has ever applied them and this is the last moment pinning them is free. They are pinned now, and the comment says which migration the old argument actually applies to. `personalSlotMappingsPublic` recorded a zero status into the idempotency ledger: the named result was never assigned on the PUT and DELETE paths, so a replayed write answered 0. It is set to 200 before the switch, the way the connection routes do it. The migration READMEs now describe 0036, 0038 and 0039, which they did not. Co-Authored-By: Claude Code <noreply@anthropic.com>
…rite it is Review round on ora-space#72, non-blocking item. The idempotency-key heuristic names the personal routes as user-scoped by prefix, and `/api/v1/me/model-` was the whole list. `/api/v1/me/executor-slot-mappings/:slot` is the same kind of route — a (user_id, slot_name) row, with no tenant anywhere in the handler — but it missed the prefix and was documented as "Scoped to tenant and user", a boundary nothing checks. Regenerated; the one line above is the entire diff. Co-Authored-By: Claude Code <noreply@anthropic.com>
80cb401 to
54bd882
Compare
…d stop silent gaps Review round on ora-space#72. Six fixes, each with the reason it is not cosmetic. The model grant's workflow branch matched a binding by its run alone. That is exact for an IssueRun, which owns one binding (0033's `run_id ... UNIQUE`), but a WorkflowRun owns one per executor slot since 0036 dropped that uniqueness, so the same test admitted every slot's binding at once. The binding id is the caller's to state — a model mTLS certificate is bound to a tenant, workspace and runtime generation, never to a slot — so node A holding `fast` could present node B's `slow` binding and be handed B's credential and model. The binding must now be the one this execution was dispatched with, read from `e.input->>'modelBindingId'`, which Cloud writes at enqueue time and no request body can restate. The new test proves the theft is refused and that the fix is what refuses it. `workflowSessionSpec` started a session even when the node's slot had no frozen binding. The run-creation gate freezes one per referenced slot, so a missing row is a run whose freeze never happened, not a state the next tick repairs — and starting anyway handed the Node a session with no model to run on, the silent downgrade the gate exists to prevent. The node now fails under the gate's own reason, `workflow_executor_unconfigured`. Its unused `target` parameter went with it. `.gitattributes` left 0036 and 0038 unpinned, on a comment that argued a database had already recorded their bytes. That was true of 0034, which is on main, and false of these: `origin/main` stops at 0034, so nothing released has ever applied them and this is the last moment pinning them is free. They are pinned now, and the comment says which migration the old argument actually applies to. `personalSlotMappingsPublic` recorded a zero status into the idempotency ledger: the named result was never assigned on the PUT and DELETE paths, so a replayed write answered 0. It is set to 200 before the switch, the way the connection routes do it. The migration READMEs now describe 0036, 0038 and 0039, which they did not. Co-Authored-By: Claude Code <noreply@anthropic.com>
…rite it is Review round on ora-space#72, non-blocking item. The idempotency-key heuristic names the personal routes as user-scoped by prefix, and `/api/v1/me/model-` was the whole list. `/api/v1/me/executor-slot-mappings/:slot` is the same kind of route — a (user_id, slot_name) row, with no tenant anywhere in the handler — but it missed the prefix and was documented as "Scoped to tenant and user", a boundary nothing checks. Regenerated; the one line above is the entire diff. Co-Authored-By: Claude Code <noreply@anthropic.com>
Rebase 完成、CI 已绿 — 请复审
堆叠顺序仍是 #67 → #68 → #72。冲突与解决、迁移编号的处理、与未改动基线逐条对照的测试证据,都在描述开头的「Rebase 到 一处需要留意的是 另一处是 请基于新 head 复审。 |
Zpc001
left a comment
There was a problem hiding this comment.
复审结论:Approve
复审对象:head 54bd882,本 PR 自身净改动用 git diff 51d31d4..54bd882 复核。上一轮的阻塞项逐条核对:
- 跨 slot 凭证盗用已修:
liveWorkflowModelBinding现在要求e.input->>'modelBindingId' = b.id::text,并补了反例用例TestWorkflowLiveModelBindingRefusesAnotherSlotsBinding; - 静默无绑定启动已消除:
workflowSessionSpec缺冻结的pluginVersion返回agent_plugin_version_unfrozen,缺 slot 绑定返回workflow_executor_unconfigured(reasonslot_not_configured),与 specs#119 的措辞一致; - ClaimWork 的 workflow 工作项带
claimedRunId,不再投影空 runId; - 0036 / 0038 / 0039 的
eol=lf已加,migrations README 已补; - slot mappings 的写响应统一 200,此前
status恒 0 的问题已修,version CAS、DELETE 幂等与视图available/unavailableReason都有 DB 用例覆盖; - 调度器侧:环检测、agent 节点 settle 先行、commit 后重读运行版本、仅终态写
finished_at都已落实,双节点/超时/无插件版本/无绑定的端到端用例齐全。
本地验证(真实 PostgreSQL 127.0.0.1:55432,REQUIRE_POSTGRES=1,在 #72 head 的 worktree 上):
go test ./internal/core -run 'TestWorkflow|TestExecutorSlotMappings|TestMigration|TestControllerExecutionProjection' -count=1
ok github.com/wanglongan587/cloud/internal/core 9.369s
CI 的 check(8m53s)与 frontend(2m12s)也通过。
非阻塞(已有归属):
注意:本 Approve 只针对 #72 自身增量。合并顺序仍是 #67 → #68 → #72,且 #68 复审提出的 projectId 契约不一致需要修正后随栈 rebase 进入本 PR 再合并;届时本 PR 的 diff 会变,若改动超出 #68 的修正范围请再叫我确认。
…d stop silent gaps Review round on ora-space#72. Six fixes, each with the reason it is not cosmetic. The model grant's workflow branch matched a binding by its run alone. That is exact for an IssueRun, which owns one binding (0033's `run_id ... UNIQUE`), but a WorkflowRun owns one per executor slot since 0036 dropped that uniqueness, so the same test admitted every slot's binding at once. The binding id is the caller's to state — a model mTLS certificate is bound to a tenant, workspace and runtime generation, never to a slot — so node A holding `fast` could present node B's `slow` binding and be handed B's credential and model. The binding must now be the one this execution was dispatched with, read from `e.input->>'modelBindingId'`, which Cloud writes at enqueue time and no request body can restate. The new test proves the theft is refused and that the fix is what refuses it. `workflowSessionSpec` started a session even when the node's slot had no frozen binding. The run-creation gate freezes one per referenced slot, so a missing row is a run whose freeze never happened, not a state the next tick repairs — and starting anyway handed the Node a session with no model to run on, the silent downgrade the gate exists to prevent. The node now fails under the gate's own reason, `workflow_executor_unconfigured`. Its unused `target` parameter went with it. `.gitattributes` left 0036 and 0038 unpinned, on a comment that argued a database had already recorded their bytes. That was true of 0034, which is on main, and false of these: `origin/main` stops at 0034, so nothing released has ever applied them and this is the last moment pinning them is free. They are pinned now, and the comment says which migration the old argument actually applies to. `personalSlotMappingsPublic` recorded a zero status into the idempotency ledger: the named result was never assigned on the PUT and DELETE paths, so a replayed write answered 0. It is set to 200 before the switch, the way the connection routes do it. The migration READMEs now describe 0036, 0038 and 0039, which they did not. Co-Authored-By: Claude Code <noreply@anthropic.com>
…rite it is Review round on ora-space#72, non-blocking item. The idempotency-key heuristic names the personal routes as user-scoped by prefix, and `/api/v1/me/model-` was the whole list. `/api/v1/me/executor-slot-mappings/:slot` is the same kind of route — a (user_id, slot_name) row, with no tenant anywhere in the handler — but it missed the prefix and was documented as "Scoped to tenant and user", a boundary nothing checks. Regenerated; the one line above is the entire diff. Co-Authored-By: Claude Code <noreply@anthropic.com>
54bd882 to
a9b9fd7
Compare
EricWvi
left a comment
There was a problem hiding this comment.
main 已合入 cloud#85:docs/ 按领域整体重建为中英双语,文档地图在 docs/README.md,旧的 docs/*.md 已经全部删除。同时 cloud#62 已合入迁移 0038_agent_workspace_blocked_deadline.sql。请先合并或 rebase 最新 main,再做以下适配:
- 迁移编号:main 上最新的迁移已经是
0038。请把本 PR 新增的迁移顺延到合并时 main 最新编号之后,同步改测试里的文件名、测试名和升级基线,并在internal/core/migrations/README(.en).md末尾追加对应小节(两个文件现在和 main 冲突)。(本 PR 堆叠在 #68 之上,新增0038–0041,请按堆叠的合并顺序依次顺延)。 - 文档:
docs/workflows/workflows(.en).md现在写的是"Agent 节点目前会让运行以明确的原因失败",执行器槽位一节只写了解析规则,本 PR 落地了 executor slot 的解析和 agent 节点的真实会话,请按本 PR 的行为改写,并写明相关 ADR 的状态。涉及 IssueRun 或 Thread 共用路径的部分,同步检查docs/agent-runs/issue-runs(.en).md;新增配置写进docs/operations/configuration(.en).md。 - 文档规则见
AGENTS.md的 Documentation 一节:中英两版同步修改;只写当前代码能核验的事实;接口字段以api/openapi.json为准;进度和验收记录写在 issue 里,不进仓库。
…control plane specs#99 D3/D8 step 4 (run identity): the WorkflowRun becomes an execution aggregate the control plane accepts, while session execution stays unwired - the scheduler still fails runs on unwired agent nodes until the executor-slot resolution bridge lands, per the slots decision's landing order (cloud behavior is unchanged). - migration 0035: workflow_runs freezes created_by/project_id; workspaces bind workflow_run_id exclusively with issue_run_id; execution_work carries the same optional ownership under an exactly-one constraint with one partial unregistered index per owner; runtime_controls.maintenance_run_id drops its IssueRun FK, since the live run of either kind is validated where it is granted on. - createWorkflowRun freezes created_by from the authenticated Identity. - createWorkflowRunWorkspace/deleteWorkflowRunWorkspace: the WorkflowRun counterpart of the IssueRun workspace lifecycle, keyed on the frozen identity (project and actor required; idempotent; busy semantics shared). - the six public-visibility filters and the document key strip exclude workflow-run workspaces exactly as they exclude issue-run ones. - enqueue, dispatch and runtime permit accept either run kind; the three settlement hooks (workspace settled/deleted, thread events) accept a WorkflowRun and write nothing - the evidence-driven scheduler reads terminal facts directly. Co-Authored-By: Claude Code <noreply@anthropic.com>
specs#99 D3, run identity: workflow_runs.project_id had no production writer at all. Migration 0035 added the column and createWorkflowRunWorkspace already refuses a run without one, but the only creation path that exists — the editor's Run button — never wrote one, so every direct run was identity-incomplete and could only ever fail at its first agent node with workflow_run_project_required. Nothing ever named the Project a run provisions in. - createWorkflowRun accepts an optional projectId from the request body, freezes it in the same INSERT as created_by, and authorizes it through the same `project` helper every project-scoped route uses: an unknown, malformed, other tenant's or archived Project is one 404, and the frozen value is the row's own id rather than the caller's string. No hardcoded or default Project — a workflow document belongs to a tenant, not to a Project, so there is nothing to default to. - A Project already `deleting` is refused (409 resource_unavailable, the rule the rest of the Project API applies). Freezing one would commit a run whose Workspace can never be created — createWorkflowRunWorkspace reads a deleting Project as busy and a deleted one as gone — leaving the run `running` for good, one failed advance per tick and no terminal state. A `provisioning` Project is still accepted: that state is transient, and the scheduler already waits for it the way it waits for any other admission. - Omitting projectId stays legal: a graph with no agent node needs no Workspace, and the editor's Run button has no Project to offer. Such a run stays identity-incomplete and fails honestly at its first agent node rather than guessing. - The route's field list, api/openapi.json and the generated client carry the new optional field. Tests (integration/workflow_run_project_test.go) drive the real HTTP path rather than seeding rows. The freeze test proves the creation transaction wrote the value — read off the row, not off the response, since the run resource carries no project field — and that deleting the Project afterwards cannot change what an already-created run was created against. The refusal test proves no run row survives a rejected Project, and the `deleting` case proves the hang guard. Both were confirmed failing before this change: the old router rejects the field outright (400 unknown_field field:projectId), and the `deleting` case answered 400 instead of 409. Co-Authored-By: Claude Code <noreply@anthropic.com>
Owner ruling on ora-space#141 (2026-10-17): `project_id` is half of a run's frozen identity (specs#99 D3), so a direct run that names no Project is refused at creation rather than persisted. The refusal reuses the code createWorkflowRunWorkspace already raises for the same condition (`workflow_run_project_required`, 409) instead of inventing a second one. - `resolveWorkflowRunProject` requires a non-empty `projectId`. The gate lives on the resolver rather than on its one caller so that every entry which creates a run — the editor today, and the @ confirm / manual run once they are linked to the same creation point — makes one shared decision inside the creation transaction instead of each writing its own "no Project" rule. This is the single creation point the owner asked for; today the INSERT exists only here. - This reverses the previous "omitting it stays legal" contract. That reading treated a Project-less run as merely incomplete and deferred the failure to the run's first agent node. It is not recoverable there: the run can never provision the Workspace those nodes need, so the honest place to report the gap is before the row exists. - A graph that genuinely needs no Workspace still names one. The rule is unconditional, which is the part a "only graphs that provision" reading would get wrong, so a start -> input graph is the case the new test pins. - `createWorkflowRunWorkspace`'s guard is kept: it now guards rows written before this gate, or by a future entry that bypasses the shared creation point. Tests (integration/workflow_run_project_test.go) drive the real HTTP path: - `TestWorkflowRunWithoutProjectIsRefused` — a start -> input graph is refused with `workflow_run_project_required`, leaves no run row, and runs once the caller names a Project, so the refusal is about the missing identity and not about the graph shape. - `TestWorkflowRunRefusesUnreachableProject` — its trailing "omitting stays accepted" case is now the missing-Project refusal, asserted against the same run-count baseline the unreachable-Project table uses. - The tests that start runs legitimately (`TestWorkflowRunLifecycle`, `TestWorkflowRunTenantIsolation`, the admission and snapshot-scope cases) now name a Project through the new `runProject` helper. A missing Project is checked after admission and input validation, so the cases asserting a snapshot, input or slot refusal are unchanged. Co-Authored-By: Claude Code <noreply@anthropic.com>
`ClaimWork` projected `OperationId` from `run_id` alone. A workflow work item carries `workflow_run_id` and leaves `run_id` NULL by the mutual-exclusion CHECK 0035 added, so the Controller was handed the empty string; the dispatch that echoes it back then matched no row in `dispatchRunExecution` and was refused as `dispatch_conflict` forever. The whole session dispatch chain was dead for workflow runs, and the existing seam test could not see it because it calls `nodeDispatch` directly instead of claiming first. The claim now projects `coalesce(run_id, workflow_run_id)` under its own name. The alias is deliberate: rows are serialized with `row_to_json`, which keeps both copies of a repeated column name, and which one the decoder keeps is not something this query should depend on. New DB test drives the whole chain through the real control actions — `clone_claim` -> `clone_dispatch` -> `node_executions` — and asserts the claim projects the workflow run id and that the dispatch it authorizes binds both the execution and the work item. Against the old projection it fails with the empty string, which is the bug. Co-Authored-By: Claude Code <noreply@anthropic.com>
…rerun Review round on ora-space#68. The migration shipped without the two things every other migration in this directory has: an entry in the READMEs and a fresh/upgrade/ rerun test. `TestMigration0035WorkflowRunIdentityAppliesFreshAndUpgrades` asserts the four columns, both exactly-one-owner CHECKs, both partial unique indexes, and — the one this migration does by deletion rather than addition — that runtime_controls' IssueRun foreign key is gone while the column it constrained stays. A polymorphic run reference cannot be a foreign key, so the liveness check moved into the code that grants on it, and that is worth a test rather than a comment. The migration is pinned to LF at the same time. It is introduced by this PR and no released database has applied it, so this is the last moment the pin is free: after merge the bytes are in a ledger this change could not reach. 0034, which is on main, keeps its platform-dependent newline for exactly that reason, and 0035_model_attempt_statistics.sql is left to the PR that owns it. Co-Authored-By: Claude Code <noreply@anthropic.com>
Rebasing onto main surfaced three conflicts the textual merge could not see: 0037 (cloud#75, already on main) moved a Project's repository out of the projects table into repository_bindings, and this branch still read it where it used to live. - createWorkflowRunWorkspace selected the projects row whole and read defaultBranch from it. That column is gone, so every run Workspace was refused with default_branch_required. It now reads through projectView, exactly as its IssueRun counterpart in the same file does. - TestWorkflowRunRefusesUnreachableProject and the dispatcherDB seed inserted projects(repository_url, default_branch). Both now insert the Project and its Binding together, the idiom every other direct seed in this repository uses. Behavior is unchanged: the same branch name, read from the row that now holds it. Co-Authored-By: Claude Code <noreply@anthropic.com>
The runtime refuses a run that names no Project with 409 `workflow_run_project_required`, but the contract still described the opposite: `projectId` sat in `optionalField`'s `/runs` branch, so the published schema left it out of `required`, and its description promised that a run without one "fails honestly at its first agent node". Neither holds — a run that names none is never created, so it never reaches a node. `resolveWorkflowRunProject`'s own doc comment contradicted itself the same way, calling the Project optional two paragraphs above "Naming one is mandatory, not optional." `projectId` is now required on the run-creation route and nowhere else, the description states the refusal, and the doc comment says what the body does. The same change is carried through `api/openapi.json` and `frontend/src/api`, so the generated client stops offering a request the service always rejects. That makes the editor's create mutation unable to build its body at all: it has no Project to state, and the run form that picks one is not built yet. The mutation refuses such a run before sending it rather than inventing a placeholder Project to fill the field — the service answers the same code, and no run is frozen against a Project nobody chose. `useCreateWorkflowRun` is where that gate lives, so the form that lands later states a Project and the gate opens; the toolbar tests pin the refusal on both sides of the app's real routes. Co-Authored-By: Claude Code <noreply@anthropic.com>
Main now carries 0038_agent_workspace_blocked_deadline.sql (cloud#62), so the run-identity migration moves one further along: 0038_workflow_run_identity.sql becomes 0039, with the test name, upgrade baseline (from 0038), migrations README entries (zh and en) and the eol pin following it. Main keeps its own 0038 section and file untouched. Co-Authored-By: Claude Code <noreply@anthropic.com>
…ership main rebuilt docs/ into bilingual per-domain documents (cloud#85), so the behavior this PR adds is documented there: the workflow run section gains the mandatory projectId and the created_by/project_id frozen at creation, and the run Workspace ownership (workspaces.workflow_run_id, mutually exclusive with issue_run_id) is stated where the Workspace aggregates and the runtime control reservation are described. Both languages change together, per the Documentation rules. Co-Authored-By: Claude Code <noreply@anthropic.com>
specs#99 D5 + the executor-slot decisions (20261015-executor-slots, 20261015-executor-slot-mappings), landing order steps 1/2/4 and the session wiring: a WorkflowRun's agent nodes now execute as real Node sessions against the invoker's own frozen per-slot bindings. Circus#70, run issue circus#141. The graph schema and the admission gate are upstream's (ora-space#65). This branch replays onto it and drops the duplicate it had carried, so the publish gate, validateWorkflowGraph and requireWorkflowExecutorSlots stay one implementation. What is added here is the half that was missing: the runner's own slot mappings, and a run that freezes what admission admitted. Mappings API (model_slot_mappings.go): the verified owner's slot name -> executor resolution table under /api/v1/me/executor-slot-mappings, with CAS writes (PUT upsert on version, DELETE existence-then-CAS behind an Idempotency-Key) and a view that computes today's availability so a mapping survives a broken connection and shows the reason instead of vanishing. `default` is not a mapping name - it IS the personal default model. Resolution and freezing (workflow_run_gate.go): one resolver (resolveWorkflowSlotExecutor) answers "can this user supply this slot" for both halves of the gate, in the same transaction, so a slot cannot be admitted by one policy and frozen by another. `default` still resolves through defaultExecutorGap - now itself the same policy a mapped pair is judged by (personalExecutorGap) - and a named slot resolves through the user's own mapping, never through `default`, the author's hint or another user's configuration. A mapping that has since broken names which part broke, so requireWorkflowExecutorSlots keeps its 409 workflow_executor_unconfigured and its params.slots shape and now fills in a fixed slot's description and hint. Freezing (migration 0036): each referenced slot - including the implicit `default` - writes one run_model_bindings row inside the run's creation transaction, carrying the connection's facts as copied values, so editing the connection afterwards cannot rewrite what an already-created run was authorized against. Any gap rolls the whole creation back. The table's uniqueness becomes one (run, slot) key per owner kind and the owner a discriminator pair, so a WorkflowRun carries several bindings while a plain OpenCode IssueRun keeps its single one under `default`. Session execution (workflow_run_session.go): a ready agent node provisions the run Workspace, waits for a connected Node, builds the session spec from the node's frozen slot binding and the run project's plugin version, commits `running` under the version CAS and releases the work item. Sessions serialize per run; the pass holds a dispatched session until terminal evidence commits, and a session that never dispatches fails its node honestly once the 15-minute startup deadline passes. The end reason is the evidence: agent_failed and interrupted fail, cancelled cancels, idle_timeout and user_ended succeed. A WorkflowRun has no Thread, so settleSessionEnded proves the event is this run's own session work and acks it, leaving the terminal commit to the scheduler. Also: liveModelBinding resolves a WorkflowRun binding through its own branch (no Thread vocabulary) for the model gateway; the run detail reports the frozen per-slot bindings; one pass that settles a session then continues its downstream wave now CASes the second commit against the version the first one left behind - a stale version made the final settle a silent no-op and left the run in `running`; and every document-level assertion in workflow_graph.go goes through asWorkflowObject, because Object is a named type and a jsonb-decoded row is a plain map[string]any, so the direct assertion silently skipped it. Co-Authored-By: Claude Code <noreply@anthropic.com>
A run's agent nodes execute serially through the run's single Workspace (D4), so a run
has as many agent sessions in its life as it has agent nodes. Two run-scoped identities
assumed the opposite and together made every node after the first unreachable:
* workflowAgentSessionOutcome resolved the running node's terminal evidence as "the
newest agent_session result for this run", so the second node inherited the first
node's result and was committed succeeded with the first node's reason without ever
executing — a fabricated success, and a violation of invariant 5;
* dispatchRunExecution refused any agent_session for a run that already had one, and
0025's one_session_per_run index refused it at the storage layer, so the second
node's dispatch could never register.
The identity chain graph node -> work item -> execution is now explicit: the scheduler
records the graph node when it releases a node's session work, the dispatch copies it
onto the execution it registers, and every evidence lookup is scoped to the node being
settled. The lifetime uniqueness splits by owner at the granularity the design intends:
an IssueRun still runs one session in its life, a WorkflowRun one per agent node.
Duplicate dispatch stays bounded without the per-run check: a node's work item is
consumed by its dispatch, a replayed execution is the same fact, and a second execution
against a consumed work item is refused. TestWorkflowSessionDispatchIsNotRepeatable
guards all three, including that a settled node's spec cannot be replayed onto the next
node's released work.
Co-Authored-By: Claude Code <noreply@anthropic.com>
The first phase executes a DAG and nothing else (ora-space#67 结论 4, D1 条件④), but nothing enforced it. A cyclic document published fine, and a run of one settled `succeeded`: a cycle reachable from the start went round once and stopped with its back edge silently dropped, and a cycle nothing reached left its nodes unresolved with no state at all — the run reported success for a document whose author drew a loop, and for nodes that produced no result (invariant 5). Both layers are closed by one walk, so a graph Cloud refuses to publish and a graph Cloud refuses to run are the same graph: - `workflowGraphCycle` (workflow_scheduler.go) is a three-colour depth-first walk over the same reader the run uses. It names the source of the back edge it found, which is on a cycle by construction — not a node merely downstream of one, and not a node between two cycles, which is where a peel-based detector lands. - The publish gate refuses with `graph_cycle` naming that node. - The advance pass settles such a run `failed` with `graph has a cycle` before the first node runs, so a snapshot frozen before the gate existed neither executes as a DAG it is not nor settles succeeded. The no-start refusal beside the new one now finishes through `finishWorkflowRun` like every other terminal settle, so a terminal run never holds live resources (D7) even when it is a run that was already in flight when the check landed. Regression evidence — new tests against the pre-fix implementation: - `TestValidateWorkflowGraph`: `problems = []`, want `[graph_cycle]` for a self-loop, a cycle behind the start, a cycle with nodes downstream of it, and a node between two cycles. - `TestWorkflowAdvanceFailsACyclicSnapshot` and `TestWorkflowAdvanceFailsAHistoricalCycleBehindTheStart`: `run settled "succeeded" error ""`, want `failed/graph has a cycle`. Co-Authored-By: Claude Code <noreply@anthropic.com>
The run's execution identity is frozen at creation — snapshot, input, initiator,
Project, Workspace binding, per-slot model bindings (D3). The plugin version was
the one member missing. `workflowSessionSpec` read the space's *current* version
at session start, so an upgrade between create and dispatch changed what an
already-created run executed, while its own record said nothing about either
version. The gate's comment claimed this matched the IssueRun path; it did not —
`agent_run_settle.go` pins agentPluginVersion in the run input at create and
never re-reads it. This makes the two paths agree.
The freeze is the same read moved earlier, not a new rule: it reads the space's
desired state, so an install still converging freezes the version it is
converging to. It runs in the creation transaction beside resolveWorkflowRunProject,
because the plugin lives in the Project's space.
0039 adds a nullable plugin_version and backfills nothing. A pre-existing row was
never frozen and there is no recoverable answer for it — the space's current
version is not what such a run was created against, so writing it would record a
fact that never held. Such a run fails at its first agent node with the new
reason `agent_plugin_version_unfrozen` rather than guessing; the reason is its
own code so those rows stay countable. Whether they should instead be backfilled
or refused is an open owner decision and this change records no answer to it.
Refusing at creation when no version can be frozen is likewise left to the owner:
the executor-slot gate beside it does refuse, but that would turn a request the
API accepts today into a 400.
Old-code failure evidence (both cases, against the pre-fix live read):
spec = map[agentPluginId:official/ora-space.opencode agentPluginVersion:2.0.0 ...]
— an in-flight upgrade to 2.0.0 changed the session spec of a run created
against 1.0.0
run settled "running" error ""; want failed/agent_plugin_version_unfrozen
— a run with no frozen version proceeded to session start instead of failing
Tests: TestWorkflowRunFreezesPluginVersionAgainstUpgrade and
TestWorkflowSessionRefusesAnUnfrozenPluginVersion in
workflow_run_session_db_test.go, plus the existing scene now freezes the same
values the creation transaction does.
go test ./internal/core/ -run 'TestWorkflow' -count=1
go test ./internal/core/ -count=1 (36.4s, pass)
go test ./integration/... (only the known
TestGitServerAuthorizesDeliveryPushFromCloudState, which needs git http-backend)
Also pins 0039 to eol=lf in .gitattributes: CheckSchema re-hashes the embedded
bytes on every boot, so an unpinned migration has a platform-dependent checksum.
0034-0038 are left unpinned deliberately — pinning them would change bytes a
database has already recorded.
Co-Authored-By: Claude Code <noreply@anthropic.com>
The D7 release was asserted on the succeeded path only. A run that fails or is
cancelled while holding a live Workspace is the case where a missing release
matters most — the clone, the sandbox and the Project's operation slot stay
occupied with nothing left to notice — and neither the agent_failed path nor the
startup-deadline path checked that the deletion was declared.
assertRunWorkspaceReleased collects both halves of the release (no unregistered
work survives; exactly one delete_workspace operation is declared) and the three
terminal-status tests call it, so the failure and cancelled paths are now held to
the same contract as the succeeded one. The `cancelled` status had no coverage at
all; TestWorkflowSessionCancelledReleasesTheRunWorkspace adds it, because a run
that is cancelled is not a run that failed and the two settle through different
branches.
Failure evidence for the new assertions — with finishWorkflowRunCleanup's
deleteWorkflowRunWorkspace call removed, all four tests fail:
--- FAIL: TestWorkflowSessionRunsEndToEnd
workflow_run_session_db_test.go:221: 0 workspace deletions declared for a terminal run; want 1
--- FAIL: TestWorkflowSessionFailureFailsRun
workflow_run_session_db_test.go:391: 0 workspace deletions declared for a terminal run; want 1
--- FAIL: TestWorkflowSessionCancelledReleasesTheRunWorkspace
workflow_run_session_db_test.go:415: 0 workspace deletions declared for a terminal run; want 1
--- FAIL: TestWorkflowSessionStartDeadlineFailsNode
workflow_run_session_db_test.go:579: 0 workspace deletions declared for a terminal run; want 1
Tests: go test ./internal/core/ -count=1 (35.4s, pass)
This does NOT close F. A session that dispatched and never reports still holds its
run `running` forever, and no evidence-based fix exists for it: the only committed
signal a workflow run's execution can carry is terminated_by_force_stop_id, and
the sole writer of that column is reachable only through POST
/workspaces/:wid/force-stop, which resolves the Workspace through store.go's
workspace() — and that excludes workflow_run_id IS NOT NULL, so a run Workspace
404s there and can never be force-stopped. The remaining real-world trigger is a
Node that vanishes, and reconcileRuntimeControls already rules out inferring
death from it ("A missing connection, an empty new table or elapsed time alone
proves neither"), while the IssueRun sibling bounds exactly that case with
AgentRestoreTimeout because a disconnected Node may legitimately restore. Bounding
an in-flight session is therefore a timeout-policy decision, not a defect fix, and
is reported to the owner rather than invented here.
Co-Authored-By: Claude Code <noreply@anthropic.com>
settleWorkflowRunStates wrote finished_at=now() on every settle, but it is also
how a pass reports progress: committing an agent node `running`, and settling one
so the next wave starts. A run whose session had just been dispatched therefore
carried the moment it "finished" while it was still executing.
The projection is what a client reads — a non-empty finishedAt is how the run
viewer renders a run as over — and the contract already declared the field
nullable (WorkflowRun's `startedAt finishedAt` optional list), so the
implementation was the side that disagreed with its own contract. Only a terminal
status writes the timestamp now, and COALESCE keeps the first terminal write so a
later pass re-declaring a terminal status cannot move the moment the run stopped.
started_at is untouched: it is set once where the run leaves `pending`.
Old-code failure evidence — with the committed settle restored, the regression
test fails on the first progress settle:
--- FAIL: TestWorkflowRunningCarriesNoFinishedAt
workflow_run_session_db_test.go:447: a run whose session just started
carries finished_at "2026-10-17T03:19:50Z"; want none
TestWorkflowRunningCarriesNoFinishedAt also asserts the terminal write lands, and
that a later pass cannot move it.
Tests: go test ./internal/core/ -run 'TestWorkflow' -count=1 (8.3s, pass)
go test ./internal/core/ -count=1 (pass)
Co-Authored-By: Claude Code <noreply@anthropic.com>
…ives
A run's nodeStates carries an `error` field — the published OpenAPI description
names it, run-node-state.ts types it as `error?: string`, and the run viewer
renders it in its own Error section keyed on it (run-inspector.tsx) — but
nothing on the server ever wrote it, and the simulator did not either. A failed
node reached the viewer with an empty Error section while its reason sat inside
Output.reason, so the one place the UI looks was always blank.
The implementation was the side disagreeing with its own contract, so it moves:
workflowNodeState gains Error, and the three writers that can fail a node set it
— the session terminal commit (the session's end reason), failWorkflowAgentNode,
and the never-dispatched startup deadline. A cancelled node carries no error: it
was cancelled, not broken, and `cancelled` is a distinct terminal status the
viewer renders as itself. An idle or succeeded node has nothing to report.
Old-code failure evidence — with the committed commitWorkflowAgentTerminal
restored, the regression test fails on the first failed node:
failed agent error = ""; want agent_failed
failed agent error = ""; want session_start_deadline_exceeded
The description said only "status, displayName, startedAt, finishedAt, output",
which is why the field read as optional-in-practice; it now says what is emitted
and when. Regenerated through the project's flow (task openapi, then
npm run api:generate) and verified idempotent with diff --strip-trailing-cr. The
change is additive inside an additionalProperties object, so no client breaks.
Tests: go test ./internal/core/ -count=1 (pass)
npm run typecheck && npx vitest run src/features/workflows/run (pass)
Co-Authored-By: Claude Code <noreply@anthropic.com>
Migration 0038 added node_executions.workflow_node_id, and every Controller-facing read of that table was `SELECT *`. t.one/t.list serialise a row through row_to_json, so the new column appeared in response bodies as `workflowNodeId` — and the NodeExecution schema is closed. Two integration tests have been failing since 0038, not since any change in this branch: Post "http://127.0.0.1:.../internal/v1/operations/claim": OpenAPI response mismatch ...: Error at "/pluginExecutions/0": property "workflowNodeId" is unsupported The Controller is the side that is right, and the ADR says so twice over: the first gRPC contract covers only the lease and clone closure, so node_* work keeps being served by JSON /internal/v1 alone, whose view fields correspond verbatim to the desktop's interface (controller-integration D2, D7). The proto ExecutionRecord declares no such field either — it is Cloud-internal routing, denormalised onto the row so the one_session_per_workflow_node partial index has a predicate and so the scheduler can attribute a result without joining through execution_work. Nothing in Cloud reads it back off a row; every reader queries it in SQL. So the rows that reach a response body are projected explicitly through nodeExecutionColumns — the table minus that column — the way workflowRunSummary already projects a run row, which is why 0039's new column never reached the wire. Widening the schema was the alternative and is not taken: it would change a contract the Controller owns to describe a field the Controller has no use for. TestControllerExecutionProjectionIsExplicit pins the projection against information_schema, so a column added later must be classified deliberately — onto the wire, or into nodeExecutionInternalColumns — instead of drifting onto one or off the other by omission. Its teeth, with last_event_sequence removed from the projection: the Controller-facing projection drifted from the table: projected: [created_at dispatched_epoch execution_id input kind node_id ...] table minus internal: [created_at dispatched_epoch execution_id input kind last_event_sequence node_id ...] Tests: go test ./integration/ -run 'TestStartPluginPlanSurvivesSelectionChangesAndExecutionFailures' -count=1 (pass, was failing) go test ./internal/core/ -run 'TestControllerExecutionProjectionIsExplicit' -count=1 (pass) Co-Authored-By: Claude Code <noreply@anthropic.com>
TestRevisionUpgradePreservesPublishedWorkflowSchema compares the workflows,
workflow_snapshots and workflow_runs rows a published database carries against the
same rows after the later migrations run. It projects the run row with
`to_jsonb(r) - ARRAY['created_by','project_id']` — the columns 0035 added, which
land NULL on published rows: an intended shape change, not a data change. 0039
adds plugin_version the same way, so the unextended array compared two different
shapes and the test failed.
revision_upgrade_test.go:112: Revision upgrade changed published workflow data
or migration identities
This extends the array and the comment beside it. It is the test's own documented
extension point, not a relaxed assertion: the row data it guards is still compared
in full, and removing an entry from the array still fails it.
This failure came in with 0039 and was missed. The commit that added 0039 reported
the integration suite as failing only on the known git http-backend test; it
actually had three failures, of which this is one and
TestStartPluginPlanSurvivesSelectionChangesAndExecutionFailures (fixed by the
projection commit before this) is another.
Tests: go test ./integration/ -run
'TestRevisionUpgradePreservesPublishedWorkflowSchema' -count=1
(pass, was failing)
Co-Authored-By: Claude Code <noreply@anthropic.com>
`go run ./cmd/checkformat` and `go tool golangci-lint run` are both CI gates on every PR, and this branch failed both. None of it was the CRLF trap the repo has been bitten by before: `gofumpt -d` named real rewrites. - gofumpt wanted three things restored: the key alignment in control.go's operation snapshot, where adding the longer pluginExecutions line broke the block around it; spaces around the concatenation in pendingExecutions, where the line crossed the length at which the no-space form reads worse; and the multi-line call arguments in workflow_graph_test.go closed on their own line. - misspell: "colours" in the cycle walk's doc comment. - gocritic hugeParam: workflowNodeState is 88 bytes now that a failed node carries its reason, so object() takes a pointer. The one caller renders a range variable, so nothing changes but the copy. Co-Authored-By: Claude Code <noreply@anthropic.com>
Rebasing onto the new ora-space#68 head made `resolveWorkflowRunPluginVersion`'s empty -Project branch unreachable: creation now refuses a run that names no Project, so the Project this function reads the space from is always present. The branch and the comment describing it ("a run that named none has no space to read from and freezes nothing") are removed rather than left as a guard for a case the creation gate no longer admits; the value is passed straight to the query. The rest of the function is unchanged: a missing plugin still freezes nothing and is still reported later as `agent_plugin_version_unfrozen`, which remains an owner decision rather than something to change here. Co-Authored-By: Claude Code <noreply@anthropic.com>
…d stop silent gaps Review round on ora-space#72. Six fixes, each with the reason it is not cosmetic. The model grant's workflow branch matched a binding by its run alone. That is exact for an IssueRun, which owns one binding (0033's `run_id ... UNIQUE`), but a WorkflowRun owns one per executor slot since 0036 dropped that uniqueness, so the same test admitted every slot's binding at once. The binding id is the caller's to state — a model mTLS certificate is bound to a tenant, workspace and runtime generation, never to a slot — so node A holding `fast` could present node B's `slow` binding and be handed B's credential and model. The binding must now be the one this execution was dispatched with, read from `e.input->>'modelBindingId'`, which Cloud writes at enqueue time and no request body can restate. The new test proves the theft is refused and that the fix is what refuses it. `workflowSessionSpec` started a session even when the node's slot had no frozen binding. The run-creation gate freezes one per referenced slot, so a missing row is a run whose freeze never happened, not a state the next tick repairs — and starting anyway handed the Node a session with no model to run on, the silent downgrade the gate exists to prevent. The node now fails under the gate's own reason, `workflow_executor_unconfigured`. Its unused `target` parameter went with it. `.gitattributes` left 0036 and 0038 unpinned, on a comment that argued a database had already recorded their bytes. That was true of 0034, which is on main, and false of these: `origin/main` stops at 0034, so nothing released has ever applied them and this is the last moment pinning them is free. They are pinned now, and the comment says which migration the old argument actually applies to. `personalSlotMappingsPublic` recorded a zero status into the idempotency ledger: the named result was never assigned on the PUT and DELETE paths, so a replayed write answered 0. It is set to 200 before the switch, the way the connection routes do it. The migration READMEs now describe 0036, 0038 and 0039, which they did not. Co-Authored-By: Claude Code <noreply@anthropic.com>
…rite it is Review round on ora-space#72, non-blocking item. The idempotency-key heuristic names the personal routes as user-scoped by prefix, and `/api/v1/me/model-` was the whole list. `/api/v1/me/executor-slot-mappings/:slot` is the same kind of route — a (user_id, slot_name) row, with no tenant anywhere in the handler — but it missed the prefix and was documented as "Scoped to tenant and user", a boundary nothing checks. Regenerated; the one line above is the entire diff. Co-Authored-By: Claude Code <noreply@anthropic.com>
0037 (cloud#75, already on main) moved a Project's repository into repository_bindings, so the session test's direct seed no longer matched the schema. It now inserts the Project and its Binding together, the idiom every other direct seed in this repository uses. Co-Authored-By: Claude Code <noreply@anthropic.com>
The stacked PR below now takes 0039 (cloud#62 landed its own 0038 on main), so this branch shifts one further: 0040_executor_slot_bindings.sql, 0041_workflow_execution_node.sql and 0042_workflow_run_plugin_version.sql, in the same order. Migration README entries (zh and en), the eol pins, the run_model_bindings/execution_work comments and the upgrade comparison all follow the new numbers; the identity columns the comparison strips stay 0039. Co-Authored-By: Claude Code <noreply@anthropic.com>
…essions The workflow documents said named slot mappings were unimplemented and that an Agent node fails the run with a reason; both are now the code this PR lands, so the slot section points at the personal mapping endpoints and the scheduling section describes the session path (run Workspace, frozen per-slot binding, one session at a time, bounded startup, workspace deletion at the terminal state). model-access records that one run may carry several bindings. Both languages change together, per the Documentation rules. Co-Authored-By: Claude Code <noreply@anthropic.com>
a9b9fd7 to
31c9ca7
Compare
Resolving this branch's rebase onto the run-identity PR kept both sides of one conflict in the revision upgrade test, so `documents` was declared twice — a redeclaration the typecheck gate rejects before any test runs. Keep the later migration's query, whose comment names the identity migration as 0039, and drop the stray blank lines the same resolution left in `.gitattributes` and the two migrations READMEs. Co-Authored-By: Claude Code <noreply@anthropic.com>
Zpc001
left a comment
There was a problem hiding this comment.
复审结论:Approve
复审对象:head a6b703b(相对 #6873a8189 的净改动),针对 @EricWvi 2026-10-18 的 request changes 逐条核对:
- 迁移顺延为
0040/0041/0042:文件名、migrations README 中英三节、.gitattributes的eol=lf、代码注释里的编号、升级比较里的0042引用均已改到,0039(#68 的运行身份列)不动,顺序 = 合并顺序。 - 文档中英同步:
docs/workflows/workflows(.en).md把「Agent 节点会让运行以明确的原因失败」换成真实会话路径(运行 Workspace、冻结绑定、按运行串行、有界启动、终态后删除 Workspace),执行器槽位一节改为写明个人映射接口;docs/models/model-access(.en).md补「一次运行多份绑定、每节点只取用它引用槽位的那条」。 docs/operations/configuration(.en).md无需改动:净改动不含任何配置项或环境变量(无cmd/、配置包或配置文件变更),15 分钟启动上限是代码常量而非可配项,PR 描述已说明。- rebase 残留已修:
integration/revision_upgrade_test.go里重复的documents查询只保留一份,注释身份迁移号同步为0039。 - ADR 状态:根决策保持
approved;两个 slot 决策由本 PR 补齐实现,应改implemented,随 specs#119 按 owner 结论在 #68/#72 合并后逐行重核。 - CI:head
a6b703b上check+frontend全绿,mergeable。
Approve。合并顺序仍须 #67 → #68 → #72。
非阻塞观察:0040/0041/0042 目前只有 fresh 库测试(*_db_test.go),没有 TestMigrationXXXXAppliesFreshAndUpgrades 式的 upgrade 测试;其 SQL 纯前向新增(旧行新列均为 NULL/default,天然满足新约束),且 0039 已覆盖 polymorphic drop 的 upgrade 场景,故不阻塞,是否补交由 owner 定夺。
本 PR 做什么
执行器槽位抵达运行时:个人槽位映射、按槽位冻结的运行绑定、以及 agent 节点的真实会话执行。这是 specs 根决策 D1 落地顺序第 3 步,加上两个已批准决策的落地:
20261015-executor-slots-replace-agent-model-ids.md与20261015-executor-slot-mappings-and-per-slot-run-bindings.md。堆叠关系:base 只能是
main(同 fork 分支不能作 base),本 PR 叠在 #68 之上,合并必须按 #67 → #68 → #72 的顺序;本 PR 自己的净改动是「净改动文件」一节。迁移
0040_executor_slot_bindings.sql:run_model_bindings的所有者变为判别对(run_id与workflow_run_id恰有其一),整表UNIQUE(run_id)拆成每种所有者一条(run, slot)唯一索引;新增executor_slot_mappings(槽位名 → 执行器,调用者私有;default被显式排除,它就是个人默认模型本身)。0041_workflow_execution_node.sql:execution_work与node_executions各加workflow_node_id,约束保证工作项所属节点属于该工作项所在运行的图;它是 Cloud 内部路由,刻意不进 Controller 投影。0042_workflow_run_plugin_version.sql:workflow_runs.plugin_version并入创建事务冻结的运行身份;可空且不回填,旧行在首个 agent 节点以agent_plugin_version_unfrozen失败而不是猜测。会话执行
workflow_executor_unconfigured回滚,参数逐个列出槽位、节点与原因,不回退到default/hint。agent_session工作项;会话终态就是节点终态。会话按运行串行(每运行至多一个未注册工作项),启动有 15 分钟上限。liveWorkflowModelBinding要求e.input->>'modelBindingId'就是这次执行派发时冻结的那一条(反例用例TestWorkflowLiveModelBindingRefusesAnotherSlotsBinding)。契约
/me/executor-slot-mappings/:slot的读写接口(PUT 200/201、DELETE 204,视图带available/unavailableReason),api/openapi.json与frontend/src/api已重新生成,无手改。本轮改动(按 @EricWvi 2026-10-18 的 request changes)
0038的最新 main)。0040/0041/0042:文件名、migrations README 中英三节、.gitattributes的eol=lf固定、代码注释里的编号、升级比较里「0042的插件版本列」一起改;0039(feat(core): bind WorkflowRun identity and workspace lifecycle to the control plane #68 的运行身份列)不动。docs/workflows/workflows(.en).md:执行器槽位一节改为写明个人映射接口;调度一节把「Agent 节点会让运行以明确的原因失败」换成真实会话路径(运行 Workspace、冻结绑定、按运行串行、有界启动、终态后删除 Workspace);docs/models/model-access(.en).md:一次运行可以有多份绑定,每个节点的会话只取用它引用的槽位那一条。docs/operations/configuration(.en).md无需改动:本 PR 的净改动不含任何配置项或环境变量(无cmd/、配置包或配置文件变更),会话启动上限 15 分钟是代码里的常量而不是可配项。integration/revision_upgrade_test.go里同一条查询被保留了两份(documents重复声明,typecheck 直接失败),.gitattributes与两个 migrations README 里也多了空行;现只保留后一次迁移的那条查询,注释里的身份迁移号同步为0039。ADR 状态
20261015-executor-slots-replace-agent-model-ids.md与20261015-executor-slot-mappings-and-per-slot-run-bindings.md由本 PR 补齐实现,应改为implemented;状态变更与核心用例证据一起放在 specs#119(作者按 owner 结论只留证据表,等 #68/#72 合并后按最终 main 逐行重核,再请复审)。根决策0-cloud-side-scheduler-owns-workflow-execution.md保持approved。门禁
CI 在 head
a6b703b上全绿:check、frontend。本地 Go 工具链在 UNC 路径上不可用,task check无法在本机复跑,以 CI 为准。净改动文件
Refs ora-space/circus#141
🤖 Generated with Claude Code