[Zpc001]

What

A run's kickoff text now lives on the run record and reaches the Start variable pool.

• workflow_runs.input keeps the run-form values and now carries the reserved input key with the kickoff text the caller submitted. No schema change: the column already stores the object. Runs created before this change were not asked for a kickoff, so the key is absent in the common case and the fallback reproduces what they executed; a historical run whose caller happened to submit a string input now reads it as the kickoff (the reserved selector), while a non-string one falls back to the authored instruction.
• workflowStartOutput resolves {start}.input from that key, falling back to the frozen Start node's authored instruction. Historical runs therefore keep evaluating with the text their snapshot froze, with no backfill.
• A Start variable named input is shadowed by the reserved selector, matching desktop: the kickoff overwrites the declared variable, so a graph declaring input as a non-string type stays runnable.
• With neither source the root is omitted, matching desktop's unset {start}.input; exists / not_exists and empty / not_empty answer as desktop does for unset versus present-empty.
• A present non-string reserved input is refused with invalid_field_value, matching desktop's string-typed kickoff_input.
• The development simulator resolves the kickoff through the same helper.

Scope

This covers the direct-run entry. The manual workflow IssueRun and @ Confirm entries do not create a WorkflowRun yet; they follow once #71 merges, tracked in ora-space/circus#289.

Evidence

• Real PostgreSQL: the stored kickoff drives both a [start, input] comparison and a {{#start.input#}} expected-value template; a run without one falls back to the authored prompt; neither source leaves the root unset.
• HTTP: the run resource keeps the kickoff, the Start output reads it, and a non-string value is refused.
• Unit: a Start variable named input is admitted when the kickoff is a string and still refused when it is non-string, pinning that the reserved selector shadows the declared variable.
• task check passes.

ADR

Implements the Start kickoff source described in decisions/cloud/workflow/0-cloud-side-scheduler-owns-workflow-execution.md (status: approved). The specs evidence update and the remaining entry points are tracked in ora-space/circus#289; this PR does not finish the ADR's implementation, so its status is unchanged.

Refs ora-space/circus#289

🤖 Generated with Claude Code

[wanglongan587]

成员 review（非 #289 assignee）：实现与测试已核对，建议修正文档/历史行边界

我原是 #289 的检视人，作者在 15:03 把检视人改派给 @zhangshilong123 并把我 unassign；按通知要求以 issue 为准，review:lgtm / status:needs-author 由 @zhangshilong123 或 owner 决定。这份 review 作为成员意见供参考，不代表运行 issue 的 review 结论。

核对基准：PR head 3187d85（base upstream/main 2803c14）；desktop upstream/main 3fa5fc03（crates/application/src/workflow_run/handlers.rs 的 kickoff_input.or_else(...)、engine/graph.rs:339-341、engine/node_runtime/control.rs 的 StartRuntime）；specs upstream/main 0225447（ADR 仍为 approved，kickoff 来源的 specs 更新由 #287 跟踪）。

本地验证（PostgreSQL 17.11；临时 worktree 已清理）：

• go build ./...、task format:check、task lint（0 issues）通过；go run ./cmd/openapi 后 api/openapi.json 无漂移。
• go test ./internal/core -run 'TestWorkflowStartOutputMatchesDesktopVariableRoots|TestRequireWorkflowRunInput|TestWorkflowAdvanceSucceedsLinearRun|TestWorkflowAdvanceStartInputUsesTheStoredKickoff|TestWorkflowAdvanceConditionOnUnsetKickoffReadsAbsent|TestWorkflowAdvanceConditionOnEmptyAuthoredPrompt|TestWorkflowAdvanceConditionNotEmptyOnEmptyAuthoredPrompt' -count=1 通过。
• go test ./integration -run TestWorkflowRunLifecycle -count=1、go test ./internal/collab ./internal/contract -count=1 通过。
• CI check / frontend 已 pass；我没有重跑完整 task check。

已核对、没有分歧的部分

• 取值优先级：运行行 input.input（字符串）优先，缺失/null 回退冻结快照 Start 的 data.input；空串保留为值；两者都无时不写根。与 desktop kickoff_input.or_else(instruction) + mirror 的语义一致。
• 历史运行：workflowStartOutput 读的是快照图，不是 live 文档；旧行无保留键时回退 authored，符合「按保存时的文本求值」。
• 契约：非字符串保留键在共用入口以 400 invalid_field_value（field: input）拒绝，且拒绝发生在建行前（集成测试后面的 list 计数证明没有留下行）；无迁移、无新字段；api/openapi.json 与 generated.schemas.ts 只有描述变化，生成物与 task openapi 输出一致。
• simulator：与调度器共用 WorkflowStartKickoff，Start 输出 shape 不再漂移。

建议修改（按重要程度）

1. internal/core/workflow_admission.go:52 的函数注释与实现矛盾。 新代码在声明字段校验之前检查保留键 input，非字符串即 400；注释仍写「Only the declared variables are checked, and keys the graph does not declare are left alone」。这句是 admission 契约的摘要，现在不再成立，建议改成「声明变量 + 保留键 input」并点明 null 视作缺省、非字符串拒绝；保留行内注释。

2. internal/core/workflow_scheduler.go:693-696 的来源说明写混了。 「A run created before Cloud persisted the kickoff carries only the authored prompt in its input object」——历史运行的 input 对象里没有 authored prompt，authored prompt 在冻结快照的 Start 节点里。建议改成「旧行的 input 对象没有保留键；它按冻结快照里 authored 的 data.input 求值」。

3. 历史行边界表述过强，需要准确化。 workflow_runs.input 是调用方可写的对象（requireWorkflowRunInput 只校验声明的表单字段，额外键一直放行；旧 OpenAPI 描述还把这个对象称为 kickoff input），所以 PR 描述的「runs created before this change simply lack the key」和 OpenAPI 的「including every run created before the key was stored」不是可保证的事实：旧行若已有字符串 input 键，本 PR 之后它会被读作 kickoff（非字符串则回退 authored），这是对持久状态的重新解释。请二选一：

   • 改成准确表述（旧行通常没有这个键；已有字符串 input 键的旧行会被读作本次 kickoff；非字符串回退 authored），并在 specs 证据里登记这条边界；
   • 或说明该场景可接受/不可达，但不要写成「一定没有这个键」。

   我没有在代码里看到能区分旧行来源的标记，所以这更像描述/specs 层的边界，而不是代码要修。

非阻塞

4. frontend/src/features/workflows/api.ts:378 的手写注释仍是「Kickoff input delivered to the start node.」，与修正后的 OpenAPI 语义不一致；建议同步为「运行输入对象；保留键 input 是本次 kickoff」。

5. 调研里写「作者声明同名 inputVariables 时其值会被 kickoff 覆盖 … 需要补测试钉住」，但 TestRequireWorkflowRunInput 新增的三个用例都是「图没有声明 input」。建议补一条声明变量名为 input 的用例（number 声明 + 字符串被拒；string 声明 + 值同时作为 kickoff），钉住这条代价。

6. desktop 的 engine/graph.rs:339-341 对 Start 读 data.input.or(data.instruction)，即旧快照的 legacy instruction 也是 authored 来源；Cloud 服务端没有这层回退，只有前端 definition.ts 在读文档时迁移 legacy instruction。若 Cloud 的发布快照可能带着未迁移的 data.instruction（例如服务端资产导入），这里的 authored 回退会与 desktop 分叉。若认为 Cloud 的规范文档只认 data.input，请在注释里写明这个前提；否则考虑对齐 legacy 回退。

合并仍由 owner 决定；上面 1–3 改完（或说明取舍）后，这份实现本身我没有发现需要改的逻辑。

[zhangshilong123]

Review of 3187d85

I checked the value chain against desktop 3fa5fc03 (crates/application/src/workflow_run/handlers.rs:148, engine/graph.rs:340, engine/variable_pool.rs:282).

What checks out:

• Priority and unset semantics. WorkflowStartKickoff matches desktop's kickoff_input.or_else(instruction): the run's value wins, the frozen Start node's authored input is the fallback, null/absent is treated as absent, and "" stays a value. Omitting the root when neither source exists is correct for desktop's pool (declare + write only on Some), and TestWorkflowAdvanceStartInputUsesTheStoredKickoff really distinguishes stored from authored (the two conditions would take the other branch if the document were read), while TestWorkflowAdvanceConditionOnUnsetKickoffReadsAbsent pins absence rather than present-empty.
• Historical rows. No key → authored; neither → root unset; no backfill needed. Covered on real PostgreSQL.
• Contract boundary. The reserved key is refused as non-string in the shared admission gate, so both entry points that call requireWorkflowRunInput agree; no schema change, and the regenerated api/openapi.json / frontend/src/api/generated.schemas.ts are in sync (go test ./internal/contract, go run ./cmd/openapi produce no diff).
• Scope. Deferring @ Confirm and the manual workflow IssueRun until cloud#71 merges is honest: both #71 and specs#133 are still open, the PR names the remaining work, and the ADR stays approved with its partial status stated.
• Checks run. With real PostgreSQL (TEST_DATABASE_URL=…:55432): go test ./internal/core ./internal/collab ./internal/contract and go test ./integration -run TestWorkflow pass; golangci-lint and checkformat are clean.

Request changes: a declared inputVariables entry named input with a non-string type makes the workflow unrunnable

workflowInputFields keeps a Start variable named input as a normal declared field (internal/core/workflow_collaboration.go:194), while requireWorkflowRunInput now also treats the same key as a string-only reserved kickoff (internal/core/workflow_admission.go:67). For a document that declares input as, say, valueType: "number":

• {"input": 7} is refused by the reserved-type check;
• {"input": "some kickoff"} is refused by the declared-field type check.

Every run of that document is therefore rejected (I reproduced both with a temporary probe against the PR head). Before this PR the declared value was accepted and dropped, so the document was runnable, and desktop still runs it: declare_base_node_variables declares the reserved {start}.input first, VariablePool::declare keeps it, and the kickoff only overwrites the value — the declared variable is shadowed, not a hard error. The Cloud editor likewise allows the name (only blank, dot and duplicates are rejected, same rule as desktop), so this is a new Cloud-only dead end.

Please choose one:

1. Shadow it as desktop does: skip the declared-field check for key input (that value can never reach the pool anyway) and pin it with a test; or
2. Refuse the declaration at publish in workflow_graph.go, so the author learns at publish time instead of getting a document whose runs can never be created. This is a deliberate parity divergence and should be written down.

In either case, please add the test the research comment called for: a graph declaring an inputVariables entry named input, asserting the kickoff shadows it. The current tests only cover graphs without that declaration.

Minor

• Branch name: fix/287-… uses the parent issue; the convention is the run issue number (fix/289-…).
• In-flight historical rows: for a frozen Start node with no data.input, the pool changes from present-empty to unset, not only for old completed rows but for any pending/running row at deploy time. That is the intended desktop parity and is documented in the PR body; flagging it so it is a stated consequence rather than an implicit one.

[zhangshilong123]

Re-reviewed head f9731b1. The blocking issue is resolved: requireWorkflowRunInput now skips the declared-field check for a variable named input, matching workflowStartOutput and desktop's shadow semantics, and TestRequireWorkflowRunInputKickoffShadowsInputVariable pins both the admitted-string and refused-non-string directions. The doc-comment and OpenAPI/frontend wording corrections are in place.

Local check: go build ./... and go test ./internal/core ./internal/collab ./internal/contract pass.