Repository navigation
Conversation
A scheduled run's business conclusion has to survive independently of its execution result: a completed run can report `fail`, and neither value may be derived from the other. Cloud is not allowed to read the verdict out of the Thread, so the only credible writer is the executor, which now reports it in its terminal session result. - The session input a scheduled occurrence releases carries `trigger: scheduled`; a manual run leaves it out, so every existing input stays byte-identical. It takes part in the dispatch equality gate like any other frozen field. - `recordScheduledRunResult` writes the reported verdict once, in the same transaction as the Thread terminal state and the delivery release. It refuses an unattributable report (no admitted occurrence) and a value outside the closed set, ignores an end reason that never completed a turn, and `ON CONFLICT DO NOTHING` makes a replay leave the first fact intact. - Migration 0062 adds `scheduled_run_results`. `issue_run_id` is the identity, `task_id` and `scheduled_at` are frozen from the occurrence, and `occurrence_id` is an audit reference only: the history trim nulls it and keeps the run-level fact, so it gains a surrogate id instead of copying a composite key whose `ON DELETE SET NULL` would also clear `task_id`. - The control codec carries the trigger and the verdict in both directions, and refuses an unspecified verdict or a detail over the ADR's 2000 characters instead of silently dropping them. Coverage: `internal/core` runs green against the local PostgreSQL 17 with the new DB tests (write-once, unattributable reports, occurrence trim), and the codec round-trip tests run without a database.
…eir uploads A scheduled run's artifacts must leave the one-shot Workspace before it is released, but Cloud never scans it: the executor declares what it left behind, Cloud owns every object key, and a row only counts as an artifact after Cloud verified the stored bytes. - Migration 0063 adds the delivery state with its non-renewable 10-minute window, the verified per-run artifact index and the rejection list. All three freeze `task_id`/`scheduled_at` and keep `occurrence_id` as an audit reference the history trim nulls. - Unregistered work becomes unique per run *and* kind: a scheduled run holds its Revision delivery and its artifact upload at the same time, and dispatch already finds each of them by `kind`. - `recordScheduledArtifacts` validates the declaration against the frozen bounds, freezes the window in the session-terminal transaction and releases the upload work item; a deployment without object storage converges the delivery as `skipped` instead of holding the Workspace. - `GrantScheduledArtifactUpload` signs one checksum-bound URL per declared artifact and refuses an index, size or digest Cloud did not persist. Registering the uploaded objects, the deadline settlement and the `delivering` convergence follow in the next commit of this change.
A scheduled run keeps `delivering` until both its Revision delivery and its artifact upload
converge or the artifact window expires, so the run has to state what the Revision decided
before it may leave the phase. The release path therefore becomes two facts written by two
different settlers:
- `recordDeliveryOutcome` writes `{revisionId, deliveryState}` into the run's result without
changing its phase; it is the single authority over that outcome and `finishRelease` reads it
back instead of trusting a caller that may have decided it in another transaction.
- `finishRelease` performs the same `delivering → releasing` CAS, activity and delete intent as
before, so a run without artifacts releases exactly as it did.
- `settleRevisionForRun` records the outcome and releases only when no artifact delivery is
still open. Artifact settlement and the window-expiry pass call the same release path, which
makes whichever side settles last the one that releases.
No behavior changes for runs without artifacts: the same rows are written in the same
transaction, and the release/delivery tests stay green.
…onverge the run The artifact declaration now has a counterpart: what the executor uploaded is checked and registered, and the run releases only after both deliveries have converged. - `prepareArtifactVerification` asks the object store about every reported object between two short transactions and binds the answer to the frozen execution input, exactly as the Revision delivery does. An unchanging outcome is registered one item at a time; anything unverified, unreported or reported against an undeclared key becomes a visible rejection. - `settleScheduledArtifacts` refuses a verdict bound to another input, judges the absolute deadline on the database's own clock in the same transaction (registering nothing at or past it, whatever the expiry pass has done), and settles `delivered`/`partial`/`failed` from the per-item results. - `ExpireScheduledArtifactDeliveriesOnce` settles a window that closed without a receipt, in the same bounded-pass shape as the other recovery loops, and the server runs it beside them. - The sixth business hook carries the settlement across the control-plane seam; the terminal result of the new kind is validated (closed failure set, declared-shape objects) and requires its receipt like the other terminal events. Coverage: the skip path runs end to end through the real takeover on a local PostgreSQL 17 - delivery settled `skipped`, the refused candidate recorded, no upload work item declared, and the run released; the declaration bound checks are unit-tested.
…oint Retention and prefix cleanup have to remove bytes, and until now Cloud could only write, read and verify objects. Deletion reuses the same local presigning as the uploads: the signed URL is a server-side capability for exactly one key and one method, fetched from the private endpoint so a deployment that publishes a CDN can never route a deletion through it. An object that is already gone is success. Every caller is a cleanup pass that must converge on "not there", and distinguishing "we deleted it" from "there was nothing left" would only make a retried pass report a failure it does not have. Redirects are refused, and the caller must not hold a database transaction, exactly like the read path.
…ed objects Two cleanup passes close the artifact lifetime, both in the repository's bounded-pass shape: a read-only scan outside the advisory lock, object-store calls between transactions, then one short transaction per run. - `PruneScheduledRunArtifactsOnce` applies D5's ten-run cost boundary per task, ordered by the frozen `scheduled_at` so a trimmed occurrence changes nothing. Objects are deleted before their index rows, and the position is re-read inside the pruning transaction so a delivery that started registering in between is not pruned mid-flight. - `CollectScheduledArtifactObjectsOnce` is the prefix fallback: a delivery that settled without registering its declaration leaves "uploaded but unregistered" objects, and because Cloud derives every key itself the sweep can name them without a bucket listing. It deletes only keys the index does not back and records that it ran, so the next tick asks the store for nothing. - `objectstore.Delete` is the deletion primitive: the private endpoint, a signed capability for one key and one method, and "already gone" treated as success. - Migration 0063 also widens `execution_work`'s kind set to the new work item - the check constraint would otherwise reject the upload work item at runtime - and documents the tables it adds.
HappyAuratus
left a comment
There was a problem hiding this comment.
审查 commit bb9b3d3。当前存在以下阻断项,Request changes。
-
[P1] 会话产物清单没有跨过真实 gRPC 边界。internal/controlgrpc/execution_codec.go:307–328 的 AgentSessionEnded 分支只转换 scheduled_result,没有读取 artifact_manifest,反向 result() 同样没有转交。正常 Node 上报的清单会静默丢失,recordScheduledArtifacts 因空 manifest 直接返回,整个上传/登记链不会启动。请补齐 items/rejections 双向转换以及真实入口 round-trip 测试。本地增加最小测试,resultObject 输入带一条 manifest,输出仅 node/outcome/reason,测试失败。
-
[P1] 新工作项无法登记成 Node 执行。execution_work 已支持 deliver_scheduled_artifacts,但 node_execution.go:28–34 的 nodeDispatch switch 仍只把 agent_session/deliver_revision 交给 dispatchRunExecution,新类型固定返回 invalid_dispatch;0024 建立的 node_executions_kind_check 也没有任何后续迁移扩充。请同时补齐派发路由、数据库 kind 约束与相关身份/恢复分支,并通过 ClaimWork → RecordDispatch 的真实数据库测试证明可执行,不能只测直接插入工作项。
-
[P1] 上传授权 action 没有接入 Control 路由。clone.go:94–95 虽新增 handler,但 isCloneAction(同文件:154–155)仍不识别 grant_scheduled_artifact_upload,Control 会落入 operationCommand 而非 cloneCommand。合法 GrantScheduledArtifactUpload RPC 无法签发授权。最小 isCloneAction 测试已复现 false;请补路由并从 RPC/Control 入口验证授权成功及身份拒绝场景。
-
[P2] 到期扫描先执行时,迟到成功回执无法 ACK。scheduled_artifact_verify.go:43–44 在接管事务之前强制 state=delivering;ExpireScheduledArtifactDeliveriesOnce 将状态结算为 failed 后,尚未落库的 ScheduledArtifactsDelivered 会固定得到 dispatch_conflict,永远进不到 settleScheduledArtifacts 的已结算 no-op/receipt 提交逻辑。这违反 ADR D3 的截止后只确认回执语义,并让 Node 持续重放。请让已结算/已过截止的合法事件跳过 HEAD,仍按原执行身份接管并确认,不登记对象;覆盖扫描先到、回执先到和重复回执。
-
[P1] 一次性 GC 无法兜底迟到 PUT。scheduled_artifact_grants.go:39 使用完整 UploadGrantTTL(默认 15 分钟),没有按 ADR D3 限制为 min(TTL, 剩余截止)。交付失败/到期后,scheduled_artifact_retention.go:134–136 首轮 DELETE 即永久写 artifacts_gc_at,后续不再扫描;仍有效的授权可在该轮之后写入同一键,形成永久孤对象。即便限制授权截止,也必须处理截止前启动、清扫后才完成的 PUT。请冻结授权上界并提供可重扫/生命周期兜底,验证“首轮 GC → 迟到 PUT 完成 → 后续清理”确实删除对象。
验证范围:完整检查本次新增产物实现、Control/gRPC 消费链、迁移、释放/到期与保留路径,核对 approved ADR D1–D5。现有 go test ./internal/controlgrpc/... ./internal/objectstore/... -count=1 通过(首次被沙箱回环 socket 权限阻止,解除限制重跑通过);新增的 manifest 与 action 路由最小测试均按预期失败。没有在本机运行 PostgreSQL 全套/真实栈验收;当前 head CI 全绿,不能替代上述缺失入口的行为覆盖。仅审本 Cloud 切片,desktop 后续交付不在本结论内。
… registry Control decides between the execution registry and the operation command path by action name, and `grant_scheduled_artifact_upload` was not in that set: the handler existed but nothing reached it, so a legitimate `GrantScheduledArtifactUpload` call was answered by the operation path instead of issuing a grant. The set is now a closed switch rather than a prefix test, because a grant is only meaningful against one registered execution: an unlisted action must fail loudly instead of being answered by a handler that knows nothing about the lease or the execution identity. Coverage: both grant actions and the two prefix families are asserted, together with names that look like grants but must stay on the operation path; the Control-level authorisation case rides along with the codec round-trip test that shares its fixture.
…t codec The takeover codec translated `scheduled_result` but not `artifact_manifest`, in both directions. A Node that reported its artifacts therefore lost them at the seam: the settlement saw an empty manifest, returned early, and the whole upload-and-register chain never started - silently, because an empty declaration is also what a run that declared nothing looks like. - Forward: every item crosses as index, display name, kind, size and digest, and every refusal as its name and reason; an unspecified kind or rejection reason is refused at the boundary instead of being stored as an empty string, and an unsigned size that would not fit the durable integer is refused before conversion. - Reverse: the stored declaration returns to the Controller unchanged, including after the result has round-tripped through jsonb, so a recovered execution is queried with what it actually reported. - The durable object stores plain integers on purpose: the object accessors read float64/int64, so a uint32 index or uint64 size kept as-is would read back as zero and silently lose the index or the size - which is exactly how the first version of this fix broke the round trip. Coverage: two items and one refusal survive resultObject -> result byte-for-byte, including through a jsonb encode/decode, and a run that declared nothing stores no manifest at all.
HappyAuratus
left a comment
There was a problem hiding this comment.
重新审查 commit f79676c,结论仍为 Request changes。
与上一轮 bb9b3d3 相比,新增 cda3a53(授权 action 路由)和 f79676c(artifact_manifest 双向转换与 JSON round-trip 测试),已核对两项修复 diff;本轮未重新运行测试,也不把这些单元覆盖视为完整入口验收。
上一轮第 2、4、5 项仍未修改,继续阻断:
- P1:nodeDispatch 路由和 node_executions kind 约束未扩充,产物工作项仍不能派发。
- P2:到期扫描先结算后,迟到成功回执仍被 prepareArtifactVerification 拒绝,无法 ACK。
- P1:上传授权未截到交付截止,且 GC 一次清理后停止,迟到 PUT 仍可能留下永久孤对象。
这些文件不在本轮两个修复提交的变更范围中;具体依据与修复验收要求见上一轮 review。作者已在运行事项确认五项问题并正在分项修复。本轮针对上述固定 SHA,不覆盖后续 push;待剩余修复、入口/数据库及到期/GC 测试补齐后再重新送审。
HappyAuratus
left a comment
There was a problem hiding this comment.
复审 de7bd66:上一轮五项修复及新增 gRPC/PostgreSQL、迁移与迟到 PUT 测试已核对;六项 CI 全绿,本机 controlgrpc/objectstore 全包重跑通过。仍有以下两项清单边界问题,均有本机最小测试复现。未配置本机 PostgreSQL,未独立重跑数据库全套或真实 Node/S3 栈。本轮仅针对 Cloud 切片。
| return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared kind %q outside the whitelist", runID, kind) | ||
| } | ||
| size := entry.N("sizeBytes") | ||
| if size <= 0 || size > maxScheduledArtifactBytes { |
There was a problem hiding this comment.
[P1] 接受 Node 正常采集的零字节产物。desktop#735 的 apps/ora-node/src/session/scheduled/artifacts.rs::accept 仅拒绝超过大小上限的文件,空 .txt/.log 会以 size_bytes=0 和合法空内容 SHA-256 上报;ADR D3 也没有最小大小限制。这里返回错误,经 recordScheduledArtifacts → settleSessionEnded 使整个会话终态接管事务回滚,终态、回执和 Revision 交付都无法提交。最小测试把 scheduledManifest 第一项大小设为 0、摘要设为空内容 SHA,稳定得到 outside 1..10485760。请允许零字节文件通过整条链,同步 node_execution.go 的 object.size > 0 校验和数据库 size_bytes > 0 约束(按迁移规则处理),补真实终态→上传→登记测试。
| if index < 0 || index >= maxScheduledArtifactItems { | ||
| return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared index %d outside 0..%d", runID, index, maxScheduledArtifactItems-1) | ||
| } | ||
| if seen[entry.S("displayName")] { |
There was a problem hiding this comment.
[P2] 对 artifact index 单独去重。seen 只检查 displayName,不同名称、相同 index 的两项会被接纳;最小测试已确认两条声明均指向 scheduled-runs/tenant/run/0。授权按 index 只取第一项;结算以相同 key 查 verified,两项大小/摘要相同时都会 registered++,但主键 (issue_run_id, artifact_index) 的 ON CONFLICT DO NOTHING 只保存第一行,最终仍标成 delivered,第二项静默丢失且没有拒绝记录。请在冻结声明前拒绝重复 index,补同 index 不同名称(包括相同和不同摘要)的回归测试。
HappyAuratus
left a comment
There was a problem hiding this comment.
已按当前 head 8fef243 复审。上一轮零字节与重复 index 的修复、溢出拒绝、完整性字段双向转换和持久化、0063–0068 迁移顺延均已核对。当前仍有以下两项需修复。验证:本机 controlgrpc、objectstore 全包和 core 空文件/重复 index 定向测试通过,diff --check 通过,当前 SHA 六项 CI 全绿;本机未配置 PostgreSQL,下面两项来自代码路径核对,未声称本机完成数据库复现或完整 race。
| } | ||
| // The absolute deadline is judged on the database's clock, in this transaction, whatever the | ||
| // expiry pass has done so far. | ||
| open := t.one("SELECT now() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", runID) |
There was a problem hiding this comment.
[P1] 用实际数据库当前时间判断绝对截止。这里的 now() 是 PostgreSQL 事务开始时间,而 Store.transactLocked 在 BeginTx 后才等待全局 pg_advisory_xact_lock。若回执完成 HEAD 后在截止前开启接管事务,但被另一事务持锁阻塞至 deadline_at 之后,取锁后此查询仍返回 open=true,随后登记对象并标为 delivered,违反截止后只 ACK、不登记的契约。请在持锁后的最终登记判据使用 clock_timestamp()(并核对验证/到期路径的一致性),补一个持锁跨越截止再放行的同步 PostgreSQL 测试,断言回执成功但索引为空、交付按超时结算。
There was a problem hiding this comment.
| if !open.B("open") { | ||
| return s.closeArtifactDelivery(t, o, "failed", scheduledArtifactDeadlineExpired) | ||
| } | ||
| if settled.S("outcome") != "delivered" { |
There was a problem hiding this comment.
[P2] 整批失败和到期结算也要登记逐项拒绝事实。ScheduledArtifactsFailed 在这里直接 closeArtifactDelivery;截止分支及 ExpireScheduledArtifactDeliveriesOnce 也直接走同一关闭函数。该函数只更新交付行并释放运行,没有读取冻结清单或插入拒绝行,所以一个声明了 report.md、最终 upload_failed 或超时的运行会留下零个产物、零条该文件拒绝明细;只有 delivered 消息中缺项/HEAD 不匹配才会执行下方 rejectScheduledArtifact。ADR D3 要求失败项在 scheduled_run_artifact_rejections 中可见,后续历史的拒绝数量也依赖这些事实。请在首次失败/到期结算同事务为尚未登记的声明项幂等记录原因,保留已有采集拒绝,覆盖显式失败、扫描先到、回执先到和重放。
There was a problem hiding this comment.
已在 5ae392e 修复:closeArtifactDelivery 在首次失败/部分成功结算的同一事务中,沿 execution_work 绑定的不可变会话清单为未登记条目补拒绝事实,ON CONFLICT 保留既有采集/核验原因,并排除成功索引。真实 PostgreSQL/gRPC 回归覆盖显式失败、两种扫描/回执顺序、迟到失败、重复及新 submission 重放、部分成功;另覆盖尚未派发即到期,不依赖上传 execution 存在。原实现缺少文件拒绝记录的回归已转绿;完整检查与 race 完成后统一回写 #536。
HappyAuratus
left a comment
There was a problem hiding this comment.
复审 commit 5ae392e,未发现新的阻断问题,Approve。
已逐项确认上一轮两项 Request changes:
- 最终登记在控制面锁获取后用 clock_timestamp() 比较绝对截止,预检与到期扫描采用一致判据;截止后仅 ACK,不登记。同步 gRPC/PostgreSQL 测试确实让最终事务在锁上跨过截止,且断言零登记、deadline_expired 和原始回执。
- closeArtifactDelivery 在交付终态与释放的同一事务中,沿 execution_work 的 sessionExecutionId 读取不可变 manifest,为未登记条目补齐拒绝事实;已有采集/核验原因和成功登记保留,交付状态与唯一约束保证重放不增记。测试覆盖显式失败、扫描/回执两种顺序、截止后失败、重放、部分成功以及未派发即到期。
核对全部 PR 文件列表为交付物,并复核本轮增量及其完整调用链;当前 main 6e302ef 的最新迁移为 0062,PR 新迁移 0063–0068 未改已有迁移。依据 specs main 45559e3 的 approved Scheduled Task 根决策与 verdict/产物 ADR。
验证:本地 controlgrpc/objectstore 全包及 core 产物清单纯逻辑定向测试通过,git diff --check 通过。当前 SHA 六项 CI 全绿;已从日志核实锁等待跨截止、整批失败四种顺序、部分成功及未派发到期回归均实际 PASS,CI 使用 PostgreSQL 17 与 race。本机未配置 TEST_DATABASE_URL,相关本地 DB 测试跳过;未独立重跑完整 task check/race 或真实 Node/S3 跨栈验收。
本结论仅覆盖 Cloud 实现切片;契约 #158 按既定顺序先合并,Desktop 接线、读取/API/UI、Space 失效提示和真实栈/specs 验收仍待后续交付。ADR 保持 approved,合并由 owner 决定。
Refs ora-space/circus#536
Summary
Persist executor-reported Scheduled Run verdicts once and deliver artifacts independently of Revision.
The original session result and delivery row retain
omitted_rejectionsandwalk_truncated;candidate_limitremains a distinct refusal reason. Rejected unsafe names are bounded display text,not upload paths. Manifests containing only refusals/truncation facts settle without empty work or
a ten-minute wait. Empty files use the normal signed PUT, verification and registration path. Whole-batch failures and
expiry persist each missing item from the frozen session/work binding, including expiry before dispatch;
existing collection/verification refusals and registered successes remain unchanged on replay.
Migrations 0063–0068 add results, artifacts, independent upload uniqueness, recurring cleanup,
zero-length registrations and manifest completeness. Existing session/Revision mutual exclusion
and plugin execution kinds remain intact.
The additive wire slice is #158; merge it first so
Controller/Node consumers can then pin the merged Cloud contract. Its identical generated additions
are included here for integration verification.
ADR: Cloud write/delivery/retention portions of
specs/decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md(D1–D5), still approved.Desktop integration, history/download API and UI, Space invalidation and real cross-stack acceptance
remain separate slices. The landing specs PR records the final implementation and
triggerfield 10deviation once all obligations are complete.
Evidence
could share a key while being counted as separate delivered objects; collection completeness was
not representable across the control contract.
After: regressions cover empty-file terminal takeover → signed HTTP PUT → HEAD → registration
→ Revision release, duplicate-index rejection before any terminal/receipt/delivery commits, and
completeness round trips through gRPC, durable results and delivery rows.
storage, grant bounds, fresh/upgrade/repeat migrations, and an admitted PUT completing after the
first GC sweep and being removed by the next one.
and register using the stale transaction-start
now(). Whole-batch failures/expiry omitted allaccepted items from the rejection facts.
After: a synchronized PostgreSQL lock-wait regression proves successful verification followed
by expired takeover is ACKed without registration. gRPC tests cover explicit failure, both
receipt/expiry orders, failure after expiry, replay, partial-success preservation and expiry
before upload dispatch.
5ae392e, Linuxtask checkand the completetask test:raceboth pass with realPostgreSQL 17. Check: integration 344.358s / core 196.964s; race: integration 441.684s /
core 248.644s. No data races are reported. Windows targeted PostgreSQL regressions also pass.
head, including race-enabled package tests and all four PostgreSQL integration shards.
does not claim real Node/S3 stack acceptance.
Merge Danger
Door: one-way for schema migration; run
cloudctl migratebefore deploying the server.Main already owns 0062; this PR follows with 0063–0068. Development databases migrated from earlier
PR numbering must be recreated.
Blast Radius: scheduled-runs
Workspaces wait for two independent deliveries. Cleanup makes bounded recurring passes over settled
runs and deletes only deterministic keys absent from the verified index. Schema additions preserve
historical rows and legacy completeness defaults.