From 1a1422210e2daba67aff17975244966e7d4c24a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:05:11 +0800 Subject: [PATCH 01/22] feat(scheduled): persist the business verdict of a scheduled run 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. --- internal/controlgrpc/execution_codec.go | 50 ++++- internal/controlgrpc/scheduled_codec_test.go | 111 +++++++++ internal/core/agent_run_session_settle.go | 6 + internal/core/agent_run_session_start.go | 6 + .../migrations/0062_scheduled_run_results.sql | 31 +++ internal/core/migrations/README.en.md | 2 + internal/core/migrations/README.md | 2 + internal/core/scheduled_run_result.go | 80 +++++++ internal/core/scheduled_run_result_db_test.go | 211 ++++++++++++++++++ 9 files changed, 497 insertions(+), 2 deletions(-) create mode 100644 internal/controlgrpc/scheduled_codec_test.go create mode 100644 internal/core/migrations/0062_scheduled_run_results.sql create mode 100644 internal/core/scheduled_run_result.go create mode 100644 internal/core/scheduled_run_result_db_test.go diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index 120b34ee..d1015e00 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -5,6 +5,7 @@ import ( "net/url" "strings" "time" + "unicode/utf8" "google.golang.org/protobuf/types/known/timestamppb" @@ -166,6 +167,15 @@ func sessionObject(spec *controlpb.AgentSessionSpec) (core.Object, error) { } out["workspaceDirectory"] = directory } + // The admission is part of the frozen input: a dispatch that disagrees with the work item about + // whether this run is a scheduled occurrence conflicts like any other changed field. + switch spec.GetTrigger() { + case controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_UNSPECIFIED: + case controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_SCHEDULED: + out["trigger"] = "scheduled" + default: + return nil, invalidInput("invalid_dispatch") + } return out, nil } @@ -205,7 +215,17 @@ func sessionMessage(o core.Object) *controlpb.AgentSessionSpec { InitialTurn: &controlpb.UserTurn{TurnId: turn.S("turnId"), Content: contentMessages(rows(turn["content"]))}, PriorRevision: priorMessage(o.O("priorRevision")), WorkspaceDirectory: directoryMessages(rows(o["workspaceDirectory"])), + Trigger: sessionTriggerMessage(o), + } +} + +// sessionTriggerMessage is the admission Cloud froze for this session. An absent field is a manual +// IssueRun, which is also how every session released before the field existed reads. +func sessionTriggerMessage(o core.Object) controlpb.AgentSessionTrigger { + if o.S("trigger") == "scheduled" { + return controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_SCHEDULED } + return controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_UNSPECIFIED } func contentObjects(blocks []*controlpb.ContentBlock) []core.Object { @@ -272,8 +292,25 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { out["outcome"], out["reason"] = "plugins_failed", shortEnum(outcome.PluginsFailed.GetReason().String(), "PLUGINS_FAILURE_REASON_") case *controlpb.ExecutionResult_AgentSessionEnded: out["outcome"], out["reason"] = "agent_session_ended", shortEnum(outcome.AgentSessionEnded.GetReason().String(), "AGENT_SESSION_END_REASON_") - if outcome.AgentSessionEnded.Detail != nil { - out["detail"] = outcome.AgentSessionEnded.GetDetail() + ended := outcome.AgentSessionEnded + if ended.Detail != nil { + out["detail"] = ended.GetDetail() + } + // The business verdict stays a structured value Cloud only validates: it is never derived + // from the Thread, the end reason or the execution status, and it travels only when the + // executor reported it (an unspecified verdict is a malformed report, not "not reported"). + if result := ended.GetScheduledResult(); result != nil { + if result.GetVerdict() == controlpb.ScheduledVerdict_SCHEDULED_VERDICT_UNSPECIFIED { + return nil, invalidInput("invalid_result") + } + scheduled := core.Object{"verdict": shortEnum(result.GetVerdict().String(), "SCHEDULED_VERDICT_")} + if result.VerdictDetail != nil { + if utf8.RuneCountInString(result.GetVerdictDetail()) > core.MaxScheduledRunVerdictDetailRunes { + return nil, invalidInput("invalid_result") + } + scheduled["verdictDetail"] = result.GetVerdictDetail() + } + out["scheduledResult"] = scheduled } case *controlpb.ExecutionResult_RevisionDelivered: out["outcome"] = "revision_delivered" @@ -337,6 +374,15 @@ func result(o core.Object) *controlpb.ExecutionResult { if detail := o.S("detail"); detail != "" { ended.Detail = &detail } + // A stored verdict returns to the Controller as the same structured value it reported, so a + // recovered execution is queried with the business result it actually carried. + if scheduled := o.O("scheduledResult"); scheduled != nil { + reported := &controlpb.ScheduledRunResult{Verdict: controlpb.ScheduledVerdict(enum(controlpb.ScheduledVerdict_value, "SCHEDULED_VERDICT_", scheduled.S("verdict")))} + if detail := scheduled.S("verdictDetail"); detail != "" { + reported.VerdictDetail = &detail + } + ended.ScheduledResult = reported + } out.Outcome = &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: ended} case "revision_delivered": out.Outcome = &controlpb.ExecutionResult_RevisionDelivered{RevisionDelivered: &controlpb.RevisionDelivered{FinalCommit: o.S("finalCommit"), BaseCommit: o.S("baseCommit"), RevisionRef: o.S("revisionRef"), History: storedMessage(o.O("history"))}} diff --git a/internal/controlgrpc/scheduled_codec_test.go b/internal/controlgrpc/scheduled_codec_test.go new file mode 100644 index 00000000..b7b41c31 --- /dev/null +++ b/internal/controlgrpc/scheduled_codec_test.go @@ -0,0 +1,111 @@ +package controlgrpc + +import ( + "encoding/json" + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/wanglongan587/cloud/internal/controlpb" + "github.com/wanglongan587/cloud/internal/core" +) + +// A scheduled session crosses the contract as the admission Cloud froze: the trigger belongs to the +// durable input, so a dispatch that drops or changes it conflicts with the work item instead of +// silently running a scheduled occurrence as a manual session. +func TestScheduledSessionTriggerRoundTrip(t *testing.T) { + const scheduled = `{"agentPluginId":"official/a","agentPluginVersion":"1.0.0","checkoutExecutionId":"checkout",` + + `"gitIdentity":{"email":"ada@example.invalid","name":"Ada"},` + + `"initialTurn":{"content":[{"text":"review"}],"turnId":"turn-1"},` + + `"kind":"agent_session","trigger":"scheduled"}` + const manual = `{"agentPluginId":"official/a","agentPluginVersion":"1.0.0","checkoutExecutionId":"checkout",` + + `"gitIdentity":{"email":"ada@example.invalid","name":"Ada"},` + + `"initialTurn":{"content":[{"text":"review"}],"turnId":"turn-1"},"kind":"agent_session"}` + + for name, tc := range map[string]struct { + frozen string + trigger controlpb.AgentSessionTrigger + }{ + "scheduled": {frozen: scheduled, trigger: controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_SCHEDULED}, + "manual": {frozen: manual, trigger: controlpb.AgentSessionTrigger_AGENT_SESSION_TRIGGER_UNSPECIFIED}, + } { + t.Run(name, func(t *testing.T) { + var frozen core.Object + if err := json.Unmarshal([]byte(tc.frozen), &frozen); err != nil { + t.Fatal(err) + } + message := sessionMessage(frozen) + if message.GetTrigger() != tc.trigger { + t.Fatalf("trigger = %v, want %v", message.GetTrigger(), tc.trigger) + } + back, err := sessionObject(message) + if err != nil { + t.Fatal(err) + } + text, err := json.Marshal(back) + if err != nil { + t.Fatal(err) + } + // The dispatch equality gate compares this exact encoding, so the round trip must + // reproduce the frozen input byte for byte. + if string(text) != tc.frozen { + t.Fatalf("round trip must reproduce the frozen input:\n got %s\nwant %s", text, tc.frozen) + } + }) + } +} + +// The business verdict crosses as a structured value Cloud validates and stores verbatim; an +// unspecified verdict or an over-long detail is a malformed result rather than "not reported". +func TestScheduledRunResultCrossesAsAStructuredValue(t *testing.T) { + node := &controlpb.NodeIdentity{NodeId: "node-1", NodeIncarnationId: "incarnation-1"} + endedResult := func(detail *string, verdict controlpb.ScheduledVerdict) *controlpb.ExecutionResult { + return &controlpb.ExecutionResult{ + Node: node, + Outcome: &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: &controlpb.AgentSessionEnded{ + Reason: controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_IDLE_TIMEOUT, + ScheduledResult: &controlpb.ScheduledRunResult{Verdict: verdict, VerdictDetail: detail}, + }}, + } + } + + stored, err := resultObject(endedResult(proto.String("no comparison base"), controlpb.ScheduledVerdict_SCHEDULED_VERDICT_RANGE_UNRESOLVED)) + if err != nil { + t.Fatal(err) + } + if want := (core.Object{"verdict": "range_unresolved", "verdictDetail": "no comparison base"}); !objectEqual(stored.O("scheduledResult"), want) { + t.Fatalf("stored scheduled result = %+v, want %+v", stored.O("scheduledResult"), want) + } + // A stored result returns to the Controller with the business facts it carried, so a recovered + // dispatch is queried exactly as it reported. + back := result(stored).GetAgentSessionEnded().GetScheduledResult() + if back.GetVerdict() != controlpb.ScheduledVerdict_SCHEDULED_VERDICT_RANGE_UNRESOLVED || back.GetVerdictDetail() != "no comparison base" { + t.Fatalf("stored verdict did not survive the round trip: %v", back) + } + // A run that reported no verdict stores no field, which reads as "not reported". + bare, err := resultObject(&controlpb.ExecutionResult{ + Node: node, + Outcome: &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: &controlpb.AgentSessionEnded{ + Reason: controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_CANCELLED, + }}, + }) + if err != nil { + t.Fatal(err) + } + if bare["scheduledResult"] != nil { + t.Fatalf("an unreported verdict must store no field, got %+v", bare["scheduledResult"]) + } + if _, err := resultObject(endedResult(nil, controlpb.ScheduledVerdict_SCHEDULED_VERDICT_UNSPECIFIED)); err == nil { + t.Fatal("an unspecified verdict must be refused") + } + if _, err := resultObject(endedResult(proto.String(string(make([]byte, core.MaxScheduledRunVerdictDetailRunes+1))), controlpb.ScheduledVerdict_SCHEDULED_VERDICT_FAIL)); err == nil { + t.Fatal("an over-long verdict detail must be refused") + } +} + +// objectEqual compares two durable objects by their canonical encoding. +func objectEqual(got, want core.Object) bool { + gotText, gotErr := json.Marshal(got) + wantText, wantErr := json.Marshal(want) + return gotErr == nil && wantErr == nil && string(gotText) == string(wantText) +} diff --git a/internal/core/agent_run_session_settle.go b/internal/core/agent_run_session_settle.go index 4bc79bdc..589da1a6 100644 --- a/internal/core/agent_run_session_settle.go +++ b/internal/core/agent_run_session_settle.go @@ -135,6 +135,12 @@ func (s *Store) settleSessionEnded(t *transaction, runID, executionID string, en t.execRows(` UPDATE thread_entries SET status='discarded' WHERE run_id=$1 AND source='user' AND status='queued'`, runID) + // The business verdict of a scheduled run lands in the same transaction as its terminal state and + // the delivery release (scheduled-task verdict/artifacts ADR D1/D2): it is a run-level fact, so it + // neither waits for the delivery settlement nor derives from the status that settlement writes. + if err := recordScheduledRunResult(t, runID, ended); err != nil { + return err + } if ended.S("reason") == "agent_failed" { appendSessionFailure(t, o, ended.S("detail")) } diff --git a/internal/core/agent_run_session_start.go b/internal/core/agent_run_session_start.go index ccfde78f..611a7572 100644 --- a/internal/core/agent_run_session_start.go +++ b/internal/core/agent_run_session_start.go @@ -176,6 +176,12 @@ func (s *Store) sessionStartSpec(t *transaction, o, snap Object, turnID, content spec["priorRevision"] = prior } spec["workspaceDirectory"] = workspaceDirectory(t, wid) + // Only a scheduled occurrence may report a business verdict or run artifacts, and the executor + // must know which admission it runs under before it starts. A manual run leaves the field out: + // that is the shape every session released before this field existed already has. + if scheduledTaskRun(t, runID) { + spec["trigger"] = "scheduled" + } return spec, nil } diff --git a/internal/core/migrations/0062_scheduled_run_results.sql b/internal/core/migrations/0062_scheduled_run_results.sql new file mode 100644 index 00000000..966fdde9 --- /dev/null +++ b/internal/core/migrations/0062_scheduled_run_results.sql @@ -0,0 +1,31 @@ +-- Cloud 定时运行的业务 verdict(specs decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md, +-- approved 的 D1/D2)。verdict 与执行层状态分开存放:只有定时触发且正常收尾的运行才有结果行,写入者是 +-- Node 执行端经会话终态上报的结构化值,Cloud 只校验枚举、detail 长度与运行身份,没有任何从 +-- issue_runs.status 或 Thread 文本推导的路径。 +-- +-- issue_run_id 是结果的唯一身份(一个运行至多一条,单次写入、重放幂等);task_id 与 scheduled_at 是写入时 +-- 从 occurrence 冻结的不可变任务归属与计划时间事实,随后不随 occurrence 的 100 条历史裁剪改变。 +-- occurrence 引用只作审计:裁剪删除 occurrence 行时置空,不级联删除运行级 verdict。 + +-- occurrence 原本只以 (task_id, scheduled_at) 为身份。结果与被拒清单需要一条可空、可置空的审计引用, +-- 这里给它一个代理 id;不复制复合键,否则 ON DELETE SET NULL 会连冻结的 task_id 一起置空。 +ALTER TABLE scheduled_task_occurrences ADD COLUMN id uuid; +UPDATE scheduled_task_occurrences SET id = gen_random_uuid() WHERE id IS NULL; +-- 既有接纳事务不写 id,所以列必须自带默认值,否则需要在接纳路径上再多传一个与业务无关的字段。 +ALTER TABLE scheduled_task_occurrences ALTER COLUMN id SET DEFAULT gen_random_uuid(); +ALTER TABLE scheduled_task_occurrences ALTER COLUMN id SET NOT NULL; +ALTER TABLE scheduled_task_occurrences ADD CONSTRAINT scheduled_task_occurrences_id_key UNIQUE (id); + +CREATE TABLE scheduled_run_results ( + issue_run_id uuid PRIMARY KEY REFERENCES issue_runs(id), + tenant_id uuid NOT NULL REFERENCES tenants(id), + task_id uuid NOT NULL REFERENCES scheduled_tasks(id), + occurrence_id uuid REFERENCES scheduled_task_occurrences(id) ON DELETE SET NULL, + scheduled_at timestamptz NOT NULL, + verdict text NOT NULL CHECK(verdict IN ('pass','warn','fail','blocked','idle','range_unresolved')), + verdict_detail text CHECK(verdict_detail IS NULL OR char_length(verdict_detail) <= 2000), + verdict_at timestamptz NOT NULL, + created_at timestamptz NOT NULL DEFAULT now() +); +-- 每任务的保留与历史读取都按冻结的 scheduled_at 从新到旧排序,不依赖可能已被裁剪的 occurrence 行。 +CREATE INDEX scheduled_run_results_task ON scheduled_run_results(task_id, scheduled_at DESC); diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index 56b343ba..f366cc59 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -73,6 +73,7 @@ There are currently 62 SQL files. | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | Named unconfirmed-stop evidence without changing residual responsibility or reclaim authority | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Plugin model-adapter capability declared by a Node instance at registration | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | Scheduled task definitions, idempotent admission ledger and deadline-cancellation facts | +| [0062_scheduled_run_results.sql](0062_scheduled_run_results.sql) | Scheduled-run business verdict with its frozen task identity and occurrence audit reference | 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. @@ -97,6 +98,7 @@ There are currently 62 SQL files. - 0059 only adds `stop_evidence`, initially empty for existing rows; no historical stop observation is invented. Covered by `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`. - 0060 adds `model_adapter` to `node_instances` (a jsonb object, default `{}`) recording the plugin model-adapter capability a Node instance declares at registration. Nothing is backfilled: existing instances count as having no capability and receive no session that needs an adapter until they register again with a newer version. Fresh and upgrade coverage is in `TestMigration0060ModelAdapterFreshAndUpgrade`. - 0061 adds `scheduled_tasks` (frozen prompt, agent ref, `schedule` jsonb, IANA timezone, unattended consent and soft-delete tombstone) and `scheduled_task_occurrences` (an idempotent admission ledger keyed by `(task_id, scheduled_at)`, holding the deadline instant and deadline-cancellation fact); it backfills no historical row. Admission creates an agent IssueRun under the bound issue in the same transaction, and the plugin version is snapshotted from the Space at admission rather than frozen on the definition. Semantics are in the specs root decision `decisions/cloud/scheduled-task`; fresh/upgrade/repeat integration coverage lands with the Web and end-to-end acceptance commit. +- 0062 gives `scheduled_task_occurrences` a surrogate `id` (with a default, so the existing admission transaction needs no change) and adds `scheduled_run_results`: `issue_run_id` is the identity, `verdict` comes from the closed set, `verdict_detail` is at most 2000 characters, `task_id` and `scheduled_at` are the task ownership and planned instant frozen at write time, and `occurrence_id` is an audit reference only, nulled when the occurrence is trimmed. No historical run is backfilled. Semantics are in the specs decision `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`; coverage is in `TestSessionEndedWritesTheScheduledVerdict`, `TestScheduledRunResultIsWrittenOncePerRun`, `TestScheduledRunResultRefusesUnattributableReports` and `TestScheduledRunResultSurvivesOccurrenceTrim`. - 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 78e714b3..e5373505 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -73,6 +73,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0059_agent_restore_stop_evidence.sql](0059_agent_restore_stop_evidence.sql) | 停止未确认的命名证据,不改变残留责任和回收授权 | | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Node 实例登记的插件模型 adapter 能力 | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | 定时任务定义、幂等接纳台账与截止取消事实 | +| [0062_scheduled_run_results.sql](0062_scheduled_run_results.sql) | 定时运行的业务 verdict、冻结的任务归属与 occurrence 审计引用 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 @@ -97,6 +98,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 - 0059 只增 `stop_evidence`,存量为空对象,不追认历史未知停止;覆盖见 `TestMigration0059RestoreStopEvidenceFreshUpgradeAndRepeat`。 - 0060 给 `node_instances` 增加 `model_adapter`(jsonb 对象,默认 `{}`),记录 Node 实例登记时声明的插件模型 adapter 能力;不回填,既有实例按空能力处理,不会被派发需要 adapter 的会话,直到以新版本重新登记。空库与升级覆盖见 `TestMigration0060ModelAdapterFreshAndUpgrade`。 - 0061 增加 `scheduled_tasks`(冻结 Prompt、agent ref、`schedule` jsonb、IANA 时区、无人值守同意与软删除墓碑)和 `scheduled_task_occurrences`(`(task_id, scheduled_at)` 唯一的幂等接纳台账,含截止时刻与截止取消事实);不回填历史行。接纳由工作线程在同一事务里创建绑定 issue 下的 agent IssueRun,插件版本在接纳时按 Space 当时值快照而非定义上冻结。语义见 specs 根决策 `decisions/cloud/scheduled-task`;空库、升级与重复应用的集成覆盖随 Web 与端到端验收 commit 一并补齐。 +- 0062 给 `scheduled_task_occurrences` 补一个代理 `id`(默认值,既有接纳事务不用改),并增加 `scheduled_run_results`:以 `issue_run_id` 为唯一身份,`verdict` 取自 Close 集、`verdict_detail` 至多 2000 字符,`task_id` 与 `scheduled_at` 是写入时冻结的任务归属与计划时间事实,`occurrence_id` 只作审计并在裁剪 occurrence 时置空。不回填任何历史运行。语义见 specs `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`;覆盖见 `TestSessionEndedWritesTheScheduledVerdict`、`TestScheduledRunResultIsWrittenOncePerRun`、`TestScheduledRunResultRefusesUnattributableReports` 与 `TestScheduledRunResultSurvivesOccurrenceTrim`。 - 新增迁移须验证空库、前一 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/scheduled_run_result.go b/internal/core/scheduled_run_result.go new file mode 100644 index 00000000..ba60b237 --- /dev/null +++ b/internal/core/scheduled_run_result.go @@ -0,0 +1,80 @@ +package core + +import ( + "fmt" + "unicode/utf8" +) + +// Cloud 定时运行的业务 verdict 落点(specs decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md, +// approved 的 D1/D2)。verdict 与执行层状态相互独立:completed 的运行可以携带 fail,两者都不从对方推导。 +// +// 可信写入者是 Node 执行端:它在会话收尾时从最终 Assistant 轮解析哨兵文本,把结构化值放进 +// AgentSessionEnded.scheduled_result 上报。Cloud 在这里只做最后一道校验(枚举、detail 长度、运行身份), +// 不读取也不解析 Thread、消息或工具业务字段。 + +// MaxScheduledRunVerdictDetailRunes 是 ADR D1 冻结的 detail 上限,按 Unicode 字符计。控制面与核心共用同一个 +// 定义,避免两处各自写死一个数字后允许的值不一致。 +const MaxScheduledRunVerdictDetailRunes = 2000 + +// scheduledRunVerdicts 是 Cloud 与 local 共用的 verdict 闭集,DTO 与列都使用同一组稳定词。 +var scheduledRunVerdicts = map[string]bool{ + "pass": true, + "warn": true, + "fail": true, + "blocked": true, + "idle": true, + "range_unresolved": true, +} + +// scheduledRunResultIsReportable reports whether a session end reason may carry business facts. +// Only an end Cloud itself asked for (`user_ended` / `idle_timeout`) completed the turn whose +// conclusion the verdict describes; a cancelled, failed or interrupted session has none. +func scheduledRunResultIsReportable(reason string) bool { + return reason == "user_ended" || reason == "idle_timeout" +} + +// recordScheduledRunResult writes the business verdict this session reported, at most once. +// +// It runs inside the caller's terminal-event takeover transaction, so the verdict commits and rolls +// back with the Thread terminal state and the delivery release. A run that is not a scheduled +// occurrence has no authoritative task identity to freeze a result against and must not have +// reported one, so a scheduled_result on such a run is refused rather than dropped: it would mean the +// executor was told to report business facts Cloud cannot attribute. +// +// A replay of the same terminal event finds the row already written and leaves it untouched; a later +// event or any other caller can neither overwrite nor clear it (`ON CONFLICT DO NOTHING`). No row is +// written when the executor reported nothing, which the reader projects as "not reported". +func recordScheduledRunResult(t *transaction, runID string, ended Object) error { + result := ended.O("scheduledResult") + if result == nil { + return nil + } + if !scheduledRunResultIsReportable(ended.S("reason")) { + return nil + } + verdict := result.S("verdict") + if !scheduledRunVerdicts[verdict] { + return fmt.Errorf("sessionEnded: run %s reported verdict %q, outside the closed set", runID, verdict) + } + detail := result.S("verdictDetail") + if utf8.RuneCountInString(detail) > MaxScheduledRunVerdictDetailRunes { + return fmt.Errorf("sessionEnded: run %s reported a verdict detail over %d characters", runID, MaxScheduledRunVerdictDetailRunes) + } + occurrence := t.one(` + SELECT id, tenant_id, task_id, scheduled_at FROM scheduled_task_occurrences + WHERE run_id=$1 AND outcome='admitted'`, runID) + if occurrence == nil { + return fmt.Errorf("sessionEnded: run %s reported a scheduled verdict but is not an admitted scheduled run", runID) + } + var stored any + if detail != "" { + stored = detail + } + t.exec(` + INSERT INTO scheduled_run_results + (issue_run_id, tenant_id, task_id, occurrence_id, scheduled_at, verdict, verdict_detail, verdict_at) + VALUES($1,$2,$3,$4,$5,$6,$7,now()) + ON CONFLICT (issue_run_id) DO NOTHING`, + runID, occurrence.S("tenantId"), occurrence.S("taskId"), occurrence.S("id"), occurrence.S("scheduledAt"), verdict, stored) + return nil +} diff --git a/internal/core/scheduled_run_result_db_test.go b/internal/core/scheduled_run_result_db_test.go new file mode 100644 index 00000000..8b3f2f83 --- /dev/null +++ b/internal/core/scheduled_run_result_db_test.go @@ -0,0 +1,211 @@ +package core + +import ( + "context" + "strings" + "testing" +) + +// DB tests for the scheduled-run business verdict (specs decisions/cloud/scheduled-task/ +// 20261022-run-verdict-and-artifacts.md D1/D2). The value only ever arrives through the executor's +// terminal session result, so these tests drive the shipped settlement and the persistence helper +// against a real isolated PostgreSQL schema rather than waiting on a Node. + +// stageScheduledOccurrence makes a seeded run an admitted scheduled occurrence, which is the one +// authoritative origin fact that lets it carry a business verdict. The definition row is derived +// from the run's own tenant and issue so the ledger's foreign keys stay real. +func stageScheduledOccurrence(t *testing.T, store *Store, runID string) string { + t.Helper() + var taskID string + if err := store.Pool.QueryRow(` + WITH run AS (SELECT tenant_id, issue_id FROM issue_runs WHERE id=$1) + INSERT INTO scheduled_tasks + (id,tenant_id,issue_id,creator_user_id,agent_id,name,prompt,schedule,timezone,revision,version,enabled) + SELECT gen_random_uuid(), run.tenant_id, run.issue_id, + (SELECT id FROM users ORDER BY id LIMIT 1), + (SELECT id FROM space_agents ORDER BY id LIMIT 1), + 'verdict-test','review the change','{"kind":"daily","hour":0,"minute":0}'::jsonb,'UTC',1,1,false + FROM run + RETURNING id`, runID).Scan(&taskID); err != nil { + t.Fatalf("stage scheduled definition: %v", err) + } + if _, err := store.Pool.Exec(` + INSERT INTO scheduled_task_occurrences(tenant_id,task_id,scheduled_at,outcome,task_revision,observed_at,run_id) + SELECT tenant_id,$2,now(),'admitted',1,now(),$1 FROM issue_runs WHERE id=$1`, runID, taskID); err != nil { + t.Fatalf("stage scheduled occurrence: %v", err) + } + return taskID +} + +// scheduledResultObject is the structured value an executor reports for one concluded run. +func scheduledResultObject(verdict, detail string) Object { + result := Object{"verdict": verdict} + if detail != "" { + result["verdictDetail"] = detail + } + return result +} + +// recordedResult reads the stored verdict of one run, or nil when it reported none. +func recordedResult(t *testing.T, store *Store, runID string) Object { + t.Helper() + var verdict, detail string + var occurrence *string + var scheduledAt string + err := store.Pool.QueryRow(` + SELECT verdict, COALESCE(verdict_detail,''), occurrence_id::text, scheduled_at::text + FROM scheduled_run_results WHERE issue_run_id=$1`, runID).Scan(&verdict, &detail, &occurrence, &scheduledAt) + if err != nil { + return nil + } + return Object{"verdict": verdict, "verdictDetail": detail, "occurrenceId": occurrence, "scheduledAt": scheduledAt} +} + +// The terminal session result of a scheduled run writes its verdict in the same transaction as the +// Thread's terminal state. +// +// The store carries no business hooks here: the delivery work item this same transaction declares is +// not what this test is about, and with object storage unconfigured the hook is what settles it. The +// verdict write must not depend on that settlement, which is exactly what the assertion shows. +func TestSessionEndedWritesTheScheduledVerdict(t *testing.T) { + store := &Store{Pool: coreFixturePool(t, func(cleanup func() error) { + t.Cleanup(func() { + if err := cleanup(); err != nil { + t.Error(err) + } + }) + })} + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + ended := Object{ + "outcome": "agent_session_ended", + "reason": "idle_timeout", + "scheduledResult": scheduledResultObject("fail", "two critical defects"), + } + var settleErr error + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + settleErr = store.settleSessionEnded(tx, scene.run, scene.execution, ended) + return Object{} + }); err != nil { + t.Fatalf("settle transaction: %v", err) + } + if settleErr != nil { + t.Fatalf("settle the scheduled session end: %v", settleErr) + } + recorded := recordedResult(t, store, scene.run) + if recorded == nil { + t.Fatal("the reported verdict was not written") + } + // A delivering run and a failing business result coexist: the verdict never derives from, or + // rewrites, the execution-layer fact. + if recorded.S("verdict") != "fail" || recorded.S("verdictDetail") != "two critical defects" { + t.Fatalf("stored verdict = %+v, want fail with its detail", recorded) + } + var phase string + if err := store.Pool.QueryRow(`SELECT phase FROM issue_runs WHERE id=$1`, scene.run).Scan(&phase); err != nil || phase != "delivering" { + t.Fatalf("run phase = %q (err %v), want delivering", phase, err) + } +} + +// A verdict is written at most once: a second report for the same run cannot overwrite the first. +func TestScheduledRunResultIsWrittenOncePerRun(t *testing.T) { + store := dispatcherDB(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + if e := recordScheduledRunResult(tx, scene.run, Object{"reason": "idle_timeout", "scheduledResult": scheduledResultObject("fail", "first")}); e != nil { + t.Fatalf("write the first verdict: %v", e) + } + if e := recordScheduledRunResult(tx, scene.run, Object{"reason": "idle_timeout", "scheduledResult": scheduledResultObject("pass", "second")}); e != nil { + t.Fatalf("repeat the verdict report: %v", e) + } + return Object{} + }); err != nil { + t.Fatalf("transaction: %v", err) + } + recorded := recordedResult(t, store, scene.run) + if recorded == nil || recorded.S("verdict") != "fail" || recorded.S("verdictDetail") != "first" { + t.Fatalf("stored verdict = %+v, want the first report to win", recorded) + } + var count int + if err := store.Pool.QueryRow(`SELECT count(*) FROM scheduled_run_results WHERE issue_run_id=$1`, scene.run).Scan(&count); err != nil || count != 1 { + t.Fatalf("result rows = %d (err %v), want exactly one", count, err) + } +} + +// Only an admitted scheduled run that Cloud ended as asked may carry a verdict. An unattributable +// report is refused, and an end reason that never completed a turn stores nothing. +func TestScheduledRunResultRefusesUnattributableReports(t *testing.T) { + store := dispatcherDB(t) + scene := seedTakeoverScene(t, store) + longDetail := strings.Repeat("x", MaxScheduledRunVerdictDetailRunes+1) + report := func(t *testing.T, reason string, result Object) error { + t.Helper() + var writeErr error + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + writeErr = recordScheduledRunResult(tx, scene.run, Object{"reason": reason, "scheduledResult": result}) + return Object{} + }); err != nil { + t.Fatalf("transaction: %v", err) + } + return writeErr + } + + // A manual run has no authoritative task identity, so a reported verdict cannot be attributed. + if err := report(t, "idle_timeout", scheduledResultObject("idle", "")); err == nil { + t.Fatal("a verdict on a manual run must be refused") + } + if recordedResult(t, store, scene.run) != nil { + t.Fatal("a refused report must store nothing") + } + + stageScheduledOccurrence(t, store, scene.run) + for name, tc := range map[string]struct { + reason string + result Object + wantError bool + }{ + "cancelled end": {reason: "cancelled", result: scheduledResultObject("idle", "")}, + "failed end": {reason: "agent_failed", result: scheduledResultObject("idle", "")}, + "unknown verdict": {reason: "idle_timeout", result: scheduledResultObject("banana", ""), wantError: true}, + "over-long detail": {reason: "idle_timeout", result: scheduledResultObject("fail", longDetail), wantError: true}, + } { + t.Run(name, func(t *testing.T) { + err := report(t, tc.reason, tc.result) + if (err != nil) != tc.wantError { + t.Fatalf("report error = %v, want an error: %v", err, tc.wantError) + } + if recordedResult(t, store, scene.run) != nil { + t.Fatal("nothing may be stored for this report") + } + }) + } +} + +// The frozen task identity survives the 100-entry history trim: clearing the occurrence reference +// keeps task_id, scheduled_at and the run. +func TestScheduledRunResultSurvivesOccurrenceTrim(t *testing.T) { + store := dispatcherDB(t) + scene := seedTakeoverScene(t, store) + taskID := stageScheduledOccurrence(t, store, scene.run) + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + if e := recordScheduledRunResult(tx, scene.run, Object{"reason": "idle_timeout", "scheduledResult": scheduledResultObject("blocked", "environment offline")}); e != nil { + t.Fatalf("write the verdict: %v", e) + } + return Object{} + }); err != nil { + t.Fatalf("transaction: %v", err) + } + if _, err := store.Pool.Exec(`DELETE FROM scheduled_task_occurrences WHERE run_id=$1`, scene.run); err != nil { + t.Fatalf("trim the occurrence: %v", err) + } + var storedTask, storedAt string + var occurrence *string + if err := store.Pool.QueryRow(`SELECT task_id::text, scheduled_at::text, occurrence_id::text + FROM scheduled_run_results WHERE issue_run_id=$1`, scene.run).Scan(&storedTask, &storedAt, &occurrence); err != nil { + t.Fatalf("the verdict must survive the trim: %v", err) + } + if storedTask != taskID || occurrence != nil || storedAt == "" { + t.Fatalf("after trim task=%q occurrence=%v scheduledAt=%q; want the frozen task and no occurrence reference", storedTask, occurrence, storedAt) + } +} From ae07bfc17376b5ca56e5d28e01636aa1d9d1ce2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:19:19 +0800 Subject: [PATCH 02/22] feat(scheduled): persist scheduled artifact declarations and grant their 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. --- internal/controlgrpc/agent_runs.go | 27 +++ internal/controlgrpc/execution_codec.go | 32 +++ internal/core/agent_run_session_settle.go | 3 + internal/core/clone.go | 2 + internal/core/execution_work.go | 6 +- .../0063_scheduled_run_artifacts.sql | 64 ++++++ internal/core/scheduled_artifact.go | 185 ++++++++++++++++++ internal/core/scheduled_artifact_delivery.go | 88 +++++++++ internal/core/scheduled_artifact_grants.go | 68 +++++++ 9 files changed, 473 insertions(+), 2 deletions(-) create mode 100644 internal/core/migrations/0063_scheduled_run_artifacts.sql create mode 100644 internal/core/scheduled_artifact.go create mode 100644 internal/core/scheduled_artifact_delivery.go create mode 100644 internal/core/scheduled_artifact_grants.go diff --git a/internal/controlgrpc/agent_runs.go b/internal/controlgrpc/agent_runs.go index 5c44a7bf..ce8de518 100644 --- a/internal/controlgrpc/agent_runs.go +++ b/internal/controlgrpc/agent_runs.go @@ -3,6 +3,7 @@ package controlgrpc import ( "context" "encoding/json" + "math" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -100,6 +101,32 @@ func (s *agentRunService) GrantRevisionUpload(ctx context.Context, req *controlp return &controlpb.GrantRevisionUploadResponse{Grants: grants}, nil } +// GrantScheduledArtifactUpload issues fresh grants for the artifacts a scheduled run declared. Each +// requested entry must name a declared index whose size and digest Cloud persisted; a request that +// tries to describe an object Cloud did not freeze is refused instead of granted. +func (s *agentRunService) GrantScheduledArtifactUpload(ctx context.Context, req *controlpb.GrantScheduledArtifactUploadRequest) (*controlpb.GrantScheduledArtifactUploadResponse, error) { + artifacts := make([]core.Object, 0, len(req.GetArtifacts())) + for _, artifact := range req.GetArtifacts() { + if artifact.GetSize() > math.MaxInt64 { + return nil, invalidInput("invalid_upload_checksum") + } + artifacts = append(artifacts, core.Object{ + "index": int64(artifact.GetIndex()), "size": int64(artifact.GetSize()), "sha256": artifact.GetSha256(), + }) + } + out, e := control(ctx, s.store, "grant_scheduled_artifact_upload", "", "", core.Object{ + "epoch": req.GetEpoch(), "executionId": req.GetExecutionId(), "artifacts": artifacts, + }) + if e != nil { + return nil, e + } + grants := make([]*controlpb.UploadGrant, 0) + for _, row := range rows(out["grants"]) { + grants = append(grants, grantMessage(row)) + } + return &controlpb.GrantScheduledArtifactUploadResponse{Grants: grants}, nil +} + func threadCommand(row core.Object) *controlpb.ThreadCommand { cmd := &controlpb.ThreadCommand{ CommandId: row.S("commandId"), RunId: row.S("runId"), ExecutionId: row.S("executionId"), diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index d1015e00..a1cb0e59 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -31,6 +31,8 @@ func inputObject(in *controlpb.ExecutionInput) (core.Object, error) { return sessionObject(spec.AgentSession) case *controlpb.ExecutionInput_DeliverRevision: return deliveryObject(spec.DeliverRevision) + case *controlpb.ExecutionInput_DeliverScheduledArtifacts: + return scheduledArtifactsObject(spec.DeliverScheduledArtifacts) default: return nil, invalidInput("invalid_dispatch") } @@ -46,6 +48,8 @@ func input(o core.Object) *controlpb.ExecutionInput { return &controlpb.ExecutionInput{Spec: &controlpb.ExecutionInput_AgentSession{AgentSession: sessionMessage(o)}} case "deliver_revision": return &controlpb.ExecutionInput{Spec: &controlpb.ExecutionInput_DeliverRevision{DeliverRevision: deliveryMessage(o)}} + case "deliver_scheduled_artifacts": + return &controlpb.ExecutionInput{Spec: &controlpb.ExecutionInput_DeliverScheduledArtifacts{DeliverScheduledArtifacts: &controlpb.DeliverScheduledArtifactsSpec{SessionExecutionId: o.S("sessionExecutionId")}}} default: return &controlpb.ExecutionInput{Spec: &controlpb.ExecutionInput_Clone{Clone: cloneMessage(o)}} } @@ -179,6 +183,16 @@ func sessionObject(spec *controlpb.AgentSessionSpec) (core.Object, error) { return out, nil } +// scheduledArtifactsObject is the durable form of the artifact-upload input: it names the ended +// session whose declaration Cloud persisted and never a location. Object keys belong to Cloud and +// reach the Node only through an upload grant. +func scheduledArtifactsObject(spec *controlpb.DeliverScheduledArtifactsSpec) (core.Object, error) { + if spec.GetSessionExecutionId() == "" { + return nil, invalidInput("invalid_dispatch") + } + return core.Object{"kind": "deliver_scheduled_artifacts", "sessionExecutionId": spec.GetSessionExecutionId()}, nil +} + // priorObject is the durable form of a PriorRevision. A session spec's names where the Node fetches // it and what it was based on; a delivery spec's carries only the identity and final commit. func priorObject(prior *controlpb.PriorRevision) core.Object { @@ -322,6 +336,16 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { unchanged := outcome.RevisionUnchanged out["finalCommit"], out["baseCommit"], out["revisionRef"] = unchanged.GetFinalCommit(), unchanged.GetBaseCommit(), unchanged.GetRevisionRef() out["history"] = storedObject(unchanged.GetHistory()) + case *controlpb.ExecutionResult_ScheduledArtifactsDelivered: + out["outcome"] = "scheduled_artifacts_delivered" + artifacts := outcome.ScheduledArtifactsDelivered.GetArtifacts() + uploaded := make([]core.Object, 0, len(artifacts)) + for _, artifact := range artifacts { + uploaded = append(uploaded, storedObject(artifact)) + } + out["artifacts"] = uploaded + case *controlpb.ExecutionResult_ScheduledArtifactsFailed: + out["outcome"], out["reason"] = "scheduled_artifacts_failed", shortEnum(outcome.ScheduledArtifactsFailed.GetReason().String(), "SCHEDULED_ARTIFACTS_FAILURE_REASON_") case *controlpb.ExecutionResult_RevisionFailed: out["outcome"], out["reason"] = "revision_failed", shortEnum(outcome.RevisionFailed.GetReason().String(), "REVISION_FAILURE_REASON_") default: @@ -390,6 +414,14 @@ func result(o core.Object) *controlpb.ExecutionResult { out.Outcome = &controlpb.ExecutionResult_RevisionUnchanged{RevisionUnchanged: &controlpb.RevisionUnchanged{FinalCommit: o.S("finalCommit"), BaseCommit: o.S("baseCommit"), RevisionRef: o.S("revisionRef"), History: storedMessage(o.O("history"))}} case "revision_failed": out.Outcome = &controlpb.ExecutionResult_RevisionFailed{RevisionFailed: &controlpb.RevisionFailed{Reason: controlpb.RevisionFailureReason(enum(controlpb.RevisionFailureReason_value, "REVISION_FAILURE_REASON_", o.S("reason")))}} + case "scheduled_artifacts_delivered": + delivered := &controlpb.ScheduledArtifactsDelivered{} + for _, artifact := range rows(o["artifacts"]) { + delivered.Artifacts = append(delivered.Artifacts, storedMessage(artifact)) + } + out.Outcome = &controlpb.ExecutionResult_ScheduledArtifactsDelivered{ScheduledArtifactsDelivered: delivered} + case "scheduled_artifacts_failed": + out.Outcome = &controlpb.ExecutionResult_ScheduledArtifactsFailed{ScheduledArtifactsFailed: &controlpb.ScheduledArtifactsFailed{Reason: controlpb.ScheduledArtifactsFailureReason(enum(controlpb.ScheduledArtifactsFailureReason_value, "SCHEDULED_ARTIFACTS_FAILURE_REASON_", o.S("reason")))}} default: failed := &controlpb.CloneFailed{Reason: controlpb.CloneFailureReason(controlpb.CloneFailureReason_value[o.S("reason")])} if retained, ok := o["retainedPath"].(string); ok { diff --git a/internal/core/agent_run_session_settle.go b/internal/core/agent_run_session_settle.go index 589da1a6..86f131fe 100644 --- a/internal/core/agent_run_session_settle.go +++ b/internal/core/agent_run_session_settle.go @@ -141,6 +141,9 @@ func (s *Store) settleSessionEnded(t *transaction, runID, executionID string, en if err := recordScheduledRunResult(t, runID, ended); err != nil { return err } + if err := recordScheduledArtifacts(t, runID, executionID, ended); err != nil { + return err + } if ended.S("reason") == "agent_failed" { appendSessionFailure(t, o, ended.S("detail")) } diff --git a/internal/core/clone.go b/internal/core/clone.go index 73e8d5c1..62af2bd1 100644 --- a/internal/core/clone.go +++ b/internal/core/clone.go @@ -91,6 +91,8 @@ func cloneCommand(t *transaction, r *ControlRequest) Object { return submitted(t, r, func() Object { return recordThreadCommandDelivered(t, r) }) case "grant_revision_upload": return revisionGrants(t, r.Body.S("executionId"), r.Body.O("checksums")) + case "grant_scheduled_artifact_upload": + return scheduledArtifactGrants(t, r.Body.S("executionId"), objectRows(r.Body["artifacts"])) default: reject(404, "not_found") } diff --git a/internal/core/execution_work.go b/internal/core/execution_work.go index ab5fc411..dd611f1e 100644 --- a/internal/core/execution_work.go +++ b/internal/core/execution_work.go @@ -36,10 +36,12 @@ func enqueueExecutionWork(t *transaction, runID, kind string, input, target Obje } require(t.one("SELECT id FROM revisions WHERE run_id=$1", runID) == nil, 409, "delivery_already_settled") } - require(kind == "agent_session" || kind == "deliver_revision", 400, "invalid_dispatch") + require(kind == "agent_session" || kind == "deliver_revision" || kind == "deliver_scheduled_artifacts", 400, "invalid_dispatch") require(input.S("kind") == kind, 400, "invalid_dispatch") require(target.S("workspaceId") != "" && target.S("sandboxInstanceId") != "" && target.S("nodeId") != "", 400, "invalid_dispatch") - if existing := t.one("SELECT * FROM execution_work WHERE run_id=$1 AND execution_id IS NULL", runID); existing != nil { + // One unregistered item per run *and kind*: a scheduled run holds its Revision delivery and its + // artifact upload at the same time, and dispatch still finds each of them by `kind`. + if existing := t.one("SELECT * FROM execution_work WHERE run_id=$1 AND kind=$2 AND execution_id IS NULL", runID, kind); existing != nil { if kind == "deliver_revision" { input = deliveryWorkInput(t, run, existing.S("id"), input) } diff --git a/internal/core/migrations/0063_scheduled_run_artifacts.sql b/internal/core/migrations/0063_scheduled_run_artifacts.sql new file mode 100644 index 00000000..96d9acf1 --- /dev/null +++ b/internal/core/migrations/0063_scheduled_run_artifacts.sql @@ -0,0 +1,64 @@ +-- Cloud 定时运行的产物交付、登记与被拒清单(specs +-- decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md,approved 的 D3/D4/D5)。 +-- +-- 产物声明本身留在会话终态的 node_executions.result 里(不可变),这里只存交付状态与已经核对过的登记索引: +-- 对象键由 Cloud 生成、登记前按 size/SHA-256 复核,所以 scheduled_run_artifacts 里的每一行都代表一个 +-- 存在且已核对的对象,而不是 Node 的声明。 +-- +-- task_id 与 scheduled_at 在写入时从 occurrence 冻结,occurrence_id 只作审计:100 条历史裁剪把它置空, +-- 不级联删除运行级事实。每任务的 10 个运行淘汰按冻结的 scheduled_at 从新到旧,不依赖 occurrence 行。 + +-- 一个运行现在可以同时持有一个 Revision 交付工作项和一个产物上传工作项(ADR D3 要求产物是独立工作项)。 +-- 未登记工作的唯一性因此从"每运行一个"收紧为"每运行每类一个";RecordDispatch 本来就按 +-- input->>'kind' 定位未登记项,领取仍是先到先领,所以这条索引是唯一的改动点。工作流侧保持不变。 +DROP INDEX one_unregistered_execution_work; +CREATE UNIQUE INDEX one_unregistered_execution_work ON execution_work(run_id, kind) + WHERE execution_id IS NULL AND workflow_run_id IS NULL; + +CREATE TABLE scheduled_run_artifact_deliveries ( + issue_run_id uuid PRIMARY KEY REFERENCES issue_runs(id), + tenant_id uuid NOT NULL REFERENCES tenants(id), + task_id uuid NOT NULL REFERENCES scheduled_tasks(id), + occurrence_id uuid REFERENCES scheduled_task_occurrences(id) ON DELETE SET NULL, + scheduled_at timestamptz NOT NULL, + state text NOT NULL CHECK(state IN ('delivering','delivered','partial','failed','skipped')), + -- 以运行身份唯一、不可续期:登记时刻 + 10 分钟,重启、重放与重派都不重算。 + deadline_at timestamptz NOT NULL, + settled_at timestamptz, + -- 收敛原因:skipped 记 object_store_unconfigured,failed 记窗口到期或上传失败。 + settled_reason text, + created_at timestamptz NOT NULL DEFAULT now() +); +-- 到期结算扫描只需要仍在交付中的行。 +CREATE INDEX scheduled_run_artifact_deliveries_due ON scheduled_run_artifact_deliveries(deadline_at) + WHERE state='delivering'; + +CREATE TABLE scheduled_run_artifacts ( + tenant_id uuid NOT NULL REFERENCES tenants(id), + issue_run_id uuid NOT NULL REFERENCES issue_runs(id), + task_id uuid NOT NULL REFERENCES scheduled_tasks(id), + occurrence_id uuid REFERENCES scheduled_task_occurrences(id) ON DELETE SET NULL, + scheduled_at timestamptz NOT NULL, + artifact_index integer NOT NULL CHECK(artifact_index >= 0 AND artifact_index < 64), + object_key text NOT NULL, + display_name text NOT NULL, + kind text NOT NULL CHECK(kind IN ('markdown','json','text','log','xml','html')), + size_bytes bigint NOT NULL CHECK(size_bytes > 0 AND size_bytes <= 10485760), + sha256 text NOT NULL CHECK(sha256 ~ '^[0-9a-f]{64}$'), + created_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (issue_run_id, artifact_index) +); +CREATE INDEX scheduled_run_artifacts_task ON scheduled_run_artifacts(task_id, scheduled_at DESC); + +CREATE TABLE scheduled_run_artifact_rejections ( + tenant_id uuid NOT NULL REFERENCES tenants(id), + issue_run_id uuid NOT NULL REFERENCES issue_runs(id), + task_id uuid NOT NULL REFERENCES scheduled_tasks(id), + occurrence_id uuid REFERENCES scheduled_task_occurrences(id) ON DELETE SET NULL, + scheduled_at timestamptz NOT NULL, + display_name text NOT NULL, + reason text NOT NULL, + created_at timestamptz NOT NULL DEFAULT now(), + PRIMARY KEY (issue_run_id, display_name) +); +CREATE INDEX scheduled_run_artifact_rejections_task ON scheduled_run_artifact_rejections(task_id, scheduled_at DESC); diff --git a/internal/core/scheduled_artifact.go b/internal/core/scheduled_artifact.go new file mode 100644 index 00000000..c6d3ab5b --- /dev/null +++ b/internal/core/scheduled_artifact.go @@ -0,0 +1,185 @@ +package core + +import ( + "fmt" + "strings" + "time" + "unicode/utf8" +) + +// Cloud 定时运行的产物交付(specs decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md, +// approved 的 D3/D4/D5)。 +// +// 声明由 Node 执行端在会话终态里上报,随后不可变(留在 node_executions.result)。Cloud 在这里校验上限与 +// 路径规则、为每个被接受的条目生成对象键、冻结不可续期的交付截止,并把被拒候选登记下来。产物本身在 +// Workspace 释放前经上传授权写入对象存储;登记只发生在 Cloud 用 HEAD 复核 size/SHA-256 之后,因此 +// scheduled_run_artifacts 里的每一行都是已核对的对象,而不是声明。 +// +// 产物失败、超时或迟到都不改变 IssueRun 状态、Revision 交付与回收语义:delivering 等产物收敛或窗口到期, +// 然后照常释放。 + +const ( + // scheduledArtifactDeadline 是 ADR D3 冻结的产物上传窗口:登记时刻 + 10 分钟,以运行身份唯一且不可续期。 + scheduledArtifactDeadline = 10 * time.Minute + // 单产物 10 MiB、单运行 64 个、单运行总计 64 MiB:Node 与 Cloud 两侧都校验,以云端上限为准。 + maxScheduledArtifactBytes = 10 * 1024 * 1024 + maxScheduledArtifactItems = 64 + maxScheduledArtifactTotalBytes = 64 * 1024 * 1024 + // 展示用相对路径的上限;它不参与对象寻址,只约束存储与展示。 + maxScheduledArtifactNameBytes = 1024 +) + +// scheduledArtifactKinds 是 ADR D3 冻结的类型白名单闭集。 +var scheduledArtifactKinds = map[string]bool{ + "markdown": true, "json": true, "text": true, "log": true, "xml": true, "html": true, +} + +// scheduledArtifactRejectionReasons 是执行端可以上报的拒绝原因闭集。Cloud 自己判定的失败 +// (upload_failed、verification_failed)不使用这组词,它们只出现在 Cloud 结算时写入的行里。 +var scheduledArtifactRejectionReasons = map[string]bool{ + "unsupported_type": true, "path_unsafe": true, "symlink": true, "hard_link": true, + "too_large": true, "too_many": true, "total_too_large": true, "read_failed": true, +} + +// scheduledArtifact 是校验通过的一条声明,带有 Cloud 生成的对象键。 +type scheduledArtifact struct { + Index int + Key string + DisplayName string + Kind string + Size int64 + Sha256 string +} + +// scheduledArtifactRejection 是一条被拒候选:执行端拒绝的,或 Cloud 结算时判定失败的。 +type scheduledArtifactRejection struct { + DisplayName string + Reason string +} + +// scheduledArtifactDeclaration 是一次声明在 Cloud 侧的权威视图。 +type scheduledArtifactDeclaration struct { + Artifacts []scheduledArtifact + Rejections []scheduledArtifactRejection +} + +// scheduledArtifactKey 生成一个条目的对象键。键只取决于运行身份与声明序号,绝不使用用户文件名,所以 +// 同一运行的同一序号在重派、重启与重放里始终指向同一个对象。 +func scheduledArtifactKey(tenantID, runID string, index int) string { + return fmt.Sprintf("scheduled-runs/%s/%s/%d", tenantID, runID, index) +} + +// parseScheduledArtifactManifest 校验上报的清单并交付 Cloud 自己的对象键。 +// +// 这里拒绝的是任何会破坏存储或读取语义的声明:超过三项上限、序号重复或越界、类型不在白名单、大小或摘要 +// 不合规、展示路径不是安全相对路径。执行端本应已经在采集时拦下这些,所以一旦出现就是矛盾的上报, +// 拒绝整份声明而不是挑选一部分——半个清单会让运行看起来有产物而事实不全。 +func parseScheduledArtifactManifest(tenantID, runID string, manifest Object) (scheduledArtifactDeclaration, error) { + declaration := scheduledArtifactDeclaration{} + seen := map[string]bool{} + var total int64 + entries := objectRows(manifest["items"]) + if len(entries) > maxScheduledArtifactItems { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %d artifacts, over the %d limit", runID, len(entries), maxScheduledArtifactItems) + } + for _, entry := range entries { + index := entry.N("index") + 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")] { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %q twice", runID, entry.S("displayName")) + } + seen[entry.S("displayName")] = true + name := entry.S("displayName") + if !scheduledArtifactPath(name) { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared unsafe path %q", runID, name) + } + kind := entry.S("kind") + if !scheduledArtifactKinds[kind] { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared kind %q outside the whitelist", runID, kind) + } + size := entry.N("sizeBytes") + if size <= 0 || size > maxScheduledArtifactBytes { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %d bytes, outside 1..%d", runID, size, maxScheduledArtifactBytes) + } + digest := entry.S("sha256") + if !lowerHexDigest(digest) { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared an invalid SHA-256", runID) + } + total += size + if total > maxScheduledArtifactTotalBytes { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared more than %d bytes", runID, maxScheduledArtifactTotalBytes) + } + declaration.Artifacts = append(declaration.Artifacts, scheduledArtifact{ + Index: int(index), + Key: scheduledArtifactKey(tenantID, runID, int(index)), + DisplayName: name, + Kind: kind, + Size: size, + Sha256: digest, + }) + } + for _, entry := range objectRows(manifest["rejections"]) { + name, reason := entry.S("displayName"), entry.S("reason") + if !scheduledArtifactPath(name) || !scheduledArtifactRejectionReasons[reason] { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s reported an invalid rejection (%q, %q)", runID, name, reason) + } + if seen[name] { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s both declared and rejected %q", runID, name) + } + declaration.Rejections = append(declaration.Rejections, scheduledArtifactRejection{DisplayName: name, Reason: reason}) + } + return declaration, nil +} + +// scheduledArtifactPath reports whether one declared name is a safe forward-slash relative path. +// +// The name only ever reaches a human reader, but a value that escapes its directory or carries a +// Windows alternate data stream would still be misread by whatever opens it next, so the same shape +// the executor collects is enforced here again. +func scheduledArtifactPath(name string) bool { + if name == "" || len(name) > maxScheduledArtifactNameBytes || strings.HasPrefix(name, "/") || strings.ContainsAny(name, ":\\") { + return false + } + if !utf8.ValidString(name) || strings.ContainsFunc(name, func(r rune) bool { return r < 0x20 || r == 0x7f }) { + return false + } + for _, segment := range strings.Split(name, "/") { + if segment == "" || segment == "." || segment == ".." { + return false + } + } + return true +} + +// lowerHexDigest accepts the digest spelling Cloud verifies uploads against. +func lowerHexDigest(value string) bool { + if len(value) != 64 { + return false + } + for _, c := range value { + if (c < '0' || c > '9') && (c < 'a' || c > 'f') { + return false + } + } + return true +} + +// objectRows reads a JSON array of objects out of a decoded durable object. +func objectRows(v any) []Object { + switch typed := v.(type) { + case []Object: + return typed + case []any: + rows := make([]Object, 0, len(typed)) + for _, entry := range typed { + if object, ok := entry.(Object); ok { + rows = append(rows, object) + } + } + return rows + default: + return nil + } +} diff --git a/internal/core/scheduled_artifact_delivery.go b/internal/core/scheduled_artifact_delivery.go new file mode 100644 index 00000000..69d5b3e2 --- /dev/null +++ b/internal/core/scheduled_artifact_delivery.go @@ -0,0 +1,88 @@ +package core + +import ( + "fmt" + "time" +) + +// The artifact-delivery half of a scheduled run: what the executor declared is persisted and given a +// delivery window in the session-terminal transaction, and the upload work item is released there. +// +// Cloud does not scan the Workspace and does not copy files: the Node uploads each declared artifact +// to a key Cloud chose, and a row only reaches `scheduled_run_artifacts` after Cloud verified the +// stored object's size and SHA-256. What is stored here is therefore the delivery state, the refused +// candidates and the frozen window - never the declaration itself, which stays in the immutable +// session terminal result. + +// scheduledArtifactSkippedObjectStore is the settlement reason of a deployment without object +// storage: the run keeps its facts and releases, because no attempt could ever succeed. +const scheduledArtifactSkippedObjectStore = "object_store_unconfigured" + +// recordScheduledArtifacts persists the artifacts a scheduled run declared, freezes the run's +// non-renewable delivery window and releases the upload work item. +// +// It runs in the caller's terminal-event transaction, so the declaration, the window and the work +// item commit or roll back with the Thread's terminal state. A run that is not an admitted scheduled +// occurrence, or a declaration outside the frozen bounds, is refused rather than stored: the executor +// was told to report artifacts Cloud cannot attribute or verify. +func recordScheduledArtifacts(t *transaction, runID, executionID string, ended Object) error { + manifest := ended.O("artifactManifest") + if manifest == nil || !scheduledRunResultIsReportable(ended.S("reason")) { + return nil + } + occurrence := t.one(` + SELECT id, tenant_id, task_id, scheduled_at FROM scheduled_task_occurrences + WHERE run_id=$1 AND outcome='admitted'`, runID) + if occurrence == nil { + return fmt.Errorf("sessionEnded: run %s reported scheduled artifacts but is not an admitted scheduled run", runID) + } + run := t.one("SELECT workspace_id FROM issue_runs WHERE id=$1", runID) + if run == nil { + return fmt.Errorf("sessionEnded: run %s disappeared while recording its artifacts", runID) + } + declaration, err := parseScheduledArtifactManifest(occurrence.S("tenantId"), runID, manifest) + if err != nil { + return err + } + if len(declaration.Artifacts) == 0 && len(declaration.Rejections) == 0 { + return nil + } + // No object storage means no upload could ever succeed: the delivery converges as skipped in the + // same transaction instead of holding the Workspace against a limit that can never be reached. + state, reason := "delivering", "" + if t.objectStore == nil { + state, reason = "skipped", scheduledArtifactSkippedObjectStore + } + var settledAt any + if state == "skipped" { + settledAt = time.Now() + } + // The window is fixed here and never renewed: a restart, a replay or a re-dispatch reuses the same + // row and therefore the same deadline. + t.exec(` + INSERT INTO scheduled_run_artifact_deliveries + (issue_run_id, tenant_id, task_id, occurrence_id, scheduled_at, state, deadline_at, settled_at, settled_reason) + VALUES($1,$2,$3,$4,$5,$6, now() + make_interval(secs => $7), $8, NULLIF($9,'')) + ON CONFLICT (issue_run_id) DO NOTHING`, + runID, occurrence.S("tenantId"), occurrence.S("taskId"), occurrence.S("id"), occurrence.S("scheduledAt"), + state, scheduledArtifactDeadline.Seconds(), settledAt, reason) + for _, rejection := range declaration.Rejections { + t.exec(` + INSERT INTO scheduled_run_artifact_rejections + (tenant_id, issue_run_id, task_id, occurrence_id, scheduled_at, display_name, reason) + VALUES($1,$2,$3,$4,$5,$6,$7) + ON CONFLICT (issue_run_id, display_name) DO NOTHING`, + occurrence.S("tenantId"), runID, occurrence.S("taskId"), occurrence.S("id"), occurrence.S("scheduledAt"), + rejection.DisplayName, rejection.Reason) + } + if len(declaration.Artifacts) == 0 || state == "skipped" { + return nil + } + // The upload is its own work item: it settles independently of the Revision delivery, and the run + // stays `delivering` until both converge or this window expires. + enqueueExecutionWork(t, runID, "deliver_scheduled_artifacts", Object{ + "kind": "deliver_scheduled_artifacts", + "sessionExecutionId": executionID, + }, sessionStartTarget(t, run.S("workspaceId")), "", time.Time{}) + return nil +} diff --git a/internal/core/scheduled_artifact_grants.go b/internal/core/scheduled_artifact_grants.go new file mode 100644 index 00000000..fe1881eb --- /dev/null +++ b/internal/core/scheduled_artifact_grants.go @@ -0,0 +1,68 @@ +package core + +import ( + "errors" + "time" + + "github.com/wanglongan587/cloud/internal/objectstore" +) + +// errScheduledDeclarationUnavailable reports a delivery execution whose session has no stored +// declaration: an internal contradiction rather than anything a Node could have caused. +var errScheduledDeclarationUnavailable = errors.New("scheduled artifact declaration is unavailable") + +// scheduledArtifactGrants signs one upload URL per declared artifact of a scheduled run. +// +// A grant names a key Cloud already chose and carries the declared checksum into the signature, so +// the Node can neither pick a location nor store different bytes than it declared. Nothing about the +// URL is written down: the signature is returned to the caller and forgotten. +// +// The absolute delivery deadline is enforced where it matters - at registration, on the database's +// own clock - so a grant issued just before the window closes may still be used, and whatever it +// writes past the deadline is refused there and cleaned up by the prefix GC. +func scheduledArtifactGrants(t *transaction, executionID string, requested []Object) Object { + e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND kind='deliver_scheduled_artifacts'", executionID) + require(e != nil, 404, "not_found") + require(len(e.O("result")) == 0 && e["terminatedByForceStopId"] == nil, 409, "dispatch_conflict") + require(t.objectStore != nil, 409, "object_store_unconfigured") + delivery := t.one("SELECT * FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) + require(delivery != nil && delivery.S("state") == "delivering", 409, "dispatch_conflict") + open := t.one("SELECT now() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) + require(open.B("open"), 409, "artifact_delivery_expired") + declaration, err := scheduledArtifactDeclarationOf(t, delivery.S("tenantId"), e) + require(err == nil, 400, "invalid_upload_checksum") + grants := make([]Object, 0, len(requested)) + for _, want := range requested { + artifact := declaration.artifact(int(want.N("index"))) + require(artifact != nil, 400, "invalid_upload_checksum") + require(want.N("size") == artifact.Size && want.S("sha256") == artifact.Sha256, 400, "invalid_upload_checksum") + signed, presignErr := objectstore.PresignPUTChecksum(t.objectStore, artifact.Key, artifact.Sha256, time.Now()) + require(presignErr == nil, 400, "invalid_upload_checksum") + grants = append(grants, Object{ + "objectKey": artifact.Key, "url": signed.URL, "method": signed.Method, + "headers": signed.Headers, "expiresAt": signed.Expires, + }) + } + return Object{"grants": grants} +} + +// scheduledArtifactDeclarationOf reads what the run declared, from the immutable session terminal +// result of the delivery execution's own session, and re-derives Cloud's object keys. +func scheduledArtifactDeclarationOf(t *transaction, tenantID string, deliveryExecution Object) (scheduledArtifactDeclaration, error) { + sessionExecution := deliveryExecution.O("input").S("sessionExecutionId") + session := t.one("SELECT result FROM node_executions WHERE execution_id=$1 AND kind='agent_session'", sessionExecution) + if session == nil { + return scheduledArtifactDeclaration{}, errScheduledDeclarationUnavailable + } + return parseScheduledArtifactManifest(tenantID, deliveryExecution.S("operationId"), session.O("result").O("artifactManifest")) +} + +// artifact returns the declared entry at one index, or nil when the run did not declare it. +func (d scheduledArtifactDeclaration) artifact(index int) *scheduledArtifact { + for i := range d.Artifacts { + if d.Artifacts[i].Index == index { + return &d.Artifacts[i] + } + } + return nil +} From 19320b90f2d27c089f5e4d55933eb4a8b2d203fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:30:06 +0800 Subject: [PATCH 03/22] refactor(scheduled): split the run's delivery outcome from its release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- internal/core/agent_run_delivery.go | 6 +-- internal/core/agent_run_release.go | 83 +++++++++++++++++++++++------ 2 files changed, 71 insertions(+), 18 deletions(-) diff --git a/internal/core/agent_run_delivery.go b/internal/core/agent_run_delivery.go index 6e36ef9d..5bf4a281 100644 --- a/internal/core/agent_run_delivery.go +++ b/internal/core/agent_run_delivery.go @@ -147,13 +147,13 @@ func (s *Store) settleDelivery(t *transaction, runID, executionID string, settle if !validID(revisionID) { return fmt.Errorf("deliverySettled: run %s execution %s reports %s without a registered Revision", runID, executionID, outcome) } - return s.releaseAfterDelivery(t, o, outcome, revisionID) + return s.settleRevisionForRun(t, o, outcome, revisionID) case deliveryStateSkipped: // No object store or Git Server is configured, so no delivery attempt was ever declared and none ever will // be: the control plane's enqueue recorded the skip instead of a work item. Release with // deliveryState=skipped, which is D4's own spelling for "no delivery happened", rather than // waiting on a give-up window that no attempt is running against. - return s.releaseAfterDelivery(t, o, deliveryStateSkipped, "") + return s.settleRevisionForRun(t, o, deliveryStateSkipped, "") } // A failure. The reason is either the wire's closed set or Cloud's own `verification_failed` @@ -166,7 +166,7 @@ func (s *Store) settleDelivery(t *transaction, runID, executionID string, settle return fmt.Errorf("deliverySettled: run %s execution %s carries failure reason %q, outside the closed set", runID, executionID, reason) } if s.deliveryGivenUp(t, o) { - return s.releaseAfterDelivery(t, o, deliveryStateFailed, "") + return s.settleRevisionForRun(t, o, deliveryStateFailed, "") } // D5: the failure releases a new delivery execution and the run keeps `delivering` — the sandbox // stays running because the Revision is not registered yet. The retry is a fresh work item whose diff --git a/internal/core/agent_run_release.go b/internal/core/agent_run_release.go index e76e5f79..25f897d9 100644 --- a/internal/core/agent_run_release.go +++ b/internal/core/agent_run_release.go @@ -77,7 +77,73 @@ func runReleaseStatus(reason string) string { // writing a comment would mean guessing a foreign schema. The conflict is left as an open // question for the ADR rather than resolved here by a silent choice. func (s *Store) releaseAfterDelivery(t *transaction, o Object, state, revisionID string) error { + recordDeliveryOutcome(t, o, state, revisionID) + return s.finishRelease(t, o) +} + +// recordDeliveryOutcome writes the run's delivery conclusion into its result without leaving +// `delivering`. +// +// The conclusion and the phase are two facts, and a scheduled run needs them apart: its artifacts +// may still be uploading when the Revision has settled, so the run states what the Revision decided +// first and finishes releasing once the artifacts converge or their window expires. Writing it here +// keeps one authority over the run's outcome, and `finishRelease` reads it back instead of taking it +// from a caller that may have decided it in another transaction. +// +// `revisionId` is written beside `deliveryState` (IssueRun D4 spells the run's result as +// `{revisionId | null, deliveryState}`): an absent key would be indistinguishable from a Revision +// whose id Cloud failed to record, so a delivery that saved nothing writes an explicit JSON null. +// Both parameters are cast explicitly because `jsonb_build_object` is variadic `"any"`: the server +// cannot infer a bare parameter's type from it, so `$3` alone is rejected at parse time with +// SQLSTATE 42P18. The `revisionId` cast also makes the empty string mean NULL, which is what a +// failed delivery passes. +func recordDeliveryOutcome(t *transaction, o Object, state, revisionID string) { + t.exec(` + UPDATE issue_runs + SET result = COALESCE(result, '{}'::jsonb) || jsonb_build_object('revisionId', NULLIF($3::text,'')::uuid, 'deliveryState', $2::text), + version=version+1, updated_at=now() + WHERE id=$1`, o.S("id"), state, revisionID) +} + +// deliveryOutcomeRecorded reports whether the run already states what its Revision delivery decided. +// It is the precondition for releasing a run that also awaits artifacts. +func deliveryOutcomeRecorded(t *transaction, runID string) bool { + o := t.one("SELECT result->>'deliveryState' AS delivery_state FROM issue_runs WHERE id=$1", runID) + return o != nil && o.S("deliveryState") != "" +} + +// artifactDeliveryPending reports whether one run's artifact delivery is still inside its window. +func artifactDeliveryPending(t *transaction, runID string) bool { + o := t.one("SELECT state FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", runID) + return o != nil && o.S("state") == "delivering" +} + +// settleRevisionForRun records what the Revision delivery decided and releases the run only when no +// artifact delivery still holds it. +// +// A scheduled run keeps `delivering` until both converge (scheduled-task verdict/artifacts ADR D3): +// the artifact upload is registered in its own work item and settles independently, so recording the +// Revision outcome here and leaving the release to whichever side settles last is what keeps a +// released Workspace from cutting an upload off. A run without artifacts, and the two later calls +// (artifact settlement or window expiry) that find the outcome already recorded, release exactly as +// before. +func (s *Store) settleRevisionForRun(t *transaction, o Object, state, revisionID string) error { + recordDeliveryOutcome(t, o, state, revisionID) + if artifactDeliveryPending(t, o.S("id")) { + return nil + } + return s.finishRelease(t, o) +} + +// finishRelease performs the `delivering → releasing` transition from the outcome the run already +// recorded. +func (s *Store) finishRelease(t *transaction, o Object) error { runID := o.S("id") + recorded := t.one("SELECT result->>'deliveryState' AS delivery_state FROM issue_runs WHERE id=$1", runID) + state := recorded.S("deliveryState") + if state == "" { + return fmt.Errorf("release agent run: run %s has no recorded delivery outcome", runID) + } sess := runSessionExecution(t, runID) if sess == nil { return fmt.Errorf("release agent run: run %s has no settled agent_session execution", runID) @@ -94,24 +160,11 @@ func (s *Store) releaseAfterDelivery(t *transaction, o Object, state, revisionID // source phase is `delivering` and nothing else: the other two ways into `releasing` (the // provisioning failure and the no-session cancel) already ran their own write, and a second // entry from here would be a second authority over the same transition. - // - // `result` gains `revisionId` beside the deliveryState: D4 spells the run's result as - // `{revisionId | null, deliveryState}`, and an absent key would be indistinguishable from a - // Revision whose id Cloud failed to record. A delivery that saved nothing writes an explicit JSON - // null, so "no Revision" is a value rather than a missing field. - // - // Both parameters are cast explicitly because `jsonb_build_object` is variadic `"any"`: the server - // cannot infer a bare parameter's type from it, so `$3` alone is rejected at parse time with - // SQLSTATE 42P18. The `revisionId` cast also makes the empty string mean NULL, which is what a - // failed delivery passes: `NULLIF($4::text,'')::uuid` cannot turn a Revision id into an invalid - // uuid, because the only other value ever passed is one the control plane just generated. if t.execRows(` UPDATE issue_runs - SET phase='releasing', status=$2, completed_at=now(), - result = COALESCE(result, '{}'::jsonb) || jsonb_build_object('revisionId', NULLIF($4::text,'')::uuid, 'deliveryState', $3::text), - version=version+1, updated_at=now() + SET phase='releasing', status=$2, completed_at=now(), version=version+1, updated_at=now() WHERE id=$1 AND executor_type='agent' AND deleted_at IS NULL AND phase='delivering'`, - runID, status, state, revisionID) != 1 { + runID, status) != 1 { now := t.one("SELECT phase, status FROM issue_runs WHERE id=$1", runID) return fmt.Errorf("release agent run: run %s was not moved to releasing (phase=%q status=%q)", runID, now.S("phase"), now.S("status")) } From e4529b872d330c0e4328b3cb1b472b5571dc7816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:42:56 +0800 Subject: [PATCH 04/22] feat(scheduled): register verified artifacts, settle the window and converge 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. --- cmd/server/main.go | 7 + internal/core/agent_run_hooks.go | 8 ++ internal/core/business_hooks.go | 10 ++ internal/core/control.go | 12 ++ internal/core/node_execution.go | 35 ++++- internal/core/scheduled_artifact_db_test.go | 117 ++++++++++++++++ internal/core/scheduled_artifact_delivery.go | 2 +- internal/core/scheduled_artifact_expiry.go | 63 +++++++++ internal/core/scheduled_artifact_settle.go | 140 +++++++++++++++++++ internal/core/scheduled_artifact_verify.go | 80 +++++++++++ internal/core/scheduled_run_result.go | 2 +- internal/core/store.go | 19 ++- 12 files changed, 486 insertions(+), 9 deletions(-) create mode 100644 internal/core/scheduled_artifact_db_test.go create mode 100644 internal/core/scheduled_artifact_expiry.go create mode 100644 internal/core/scheduled_artifact_settle.go create mode 100644 internal/core/scheduled_artifact_verify.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 8ecd6462..3eaaafff 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -282,6 +282,13 @@ func run() (runErr error) { defer syncGroup.Done() pluginmarket.RunSyncLoop(ctx, store.GiveUpStaleWorkspaceReleasesOnce, agentDeliveryInterval, pluginmarket.ContextSleep, log) }() + syncGroup.Add(1) + go func() { + defer syncGroup.Done() + // A scheduled run's artifact window is fixed when its declaration is stored, so expiry is + // the same clock-driven pass as the others: one short transaction per run, no policy. + pluginmarket.RunSyncLoop(ctx, store.ExpireScheduledArtifactDeliveriesOnce, agentDeliveryInterval, pluginmarket.ContextSleep, log) + }() } // Residual recovery remains independent of a run's business terminal state. Cloud consumes // existing, identity-bound facts; a missing fact leaves its ledger open rather than treating diff --git a/internal/core/agent_run_hooks.go b/internal/core/agent_run_hooks.go index 35f5ee10..898f805a 100644 --- a/internal/core/agent_run_hooks.go +++ b/internal/core/agent_run_hooks.go @@ -42,3 +42,11 @@ func deliverySettled(t *transaction, run, execution string, settled Object) { } } } + +func scheduledArtifactsSettled(t *transaction, run, execution string, settled Object) { + if t.onScheduledArtifactsSettled != nil { + if err := t.onScheduledArtifactsSettled(t.ctx, t.tx, run, execution, settled); err != nil { + reject(409, "business_hook_failed") + } + } +} diff --git a/internal/core/business_hooks.go b/internal/core/business_hooks.go index f14ebd97..c08ecfd5 100644 --- a/internal/core/business_hooks.go +++ b/internal/core/business_hooks.go @@ -59,6 +59,7 @@ func BindBusinessHooks(s *Store) { s.OnRunWorkspaceSettled = onRunWorkspaceSettled s.OnRunWorkspaceDeleted = onRunWorkspaceDeleted s.OnDeliverySettled = onDeliverySettled + s.OnScheduledArtifactsSettled = onScheduledArtifactsSettled } // onThreadEvents stores one batch of a session execution's history records as Thread entries and @@ -127,3 +128,12 @@ func onDeliverySettled(ctx context.Context, tx *sql.Tx, run, execution string, s t := businessTransaction(ctx, tx) return t.store.settleDelivery(t, run, execution, settled) } + +// onScheduledArtifactsSettled registers the artifacts Cloud verified and finishes the run's release +// once its Revision delivery has also settled (scheduled-task verdict/artifacts ADR D3). The settled +// object is the control plane's own verdict: per-object verification and the absolute-deadline +// comparison, both computed outside the transaction and bound to the frozen execution input. +func onScheduledArtifactsSettled(ctx context.Context, tx *sql.Tx, run, execution string, settled Object) error { + t := businessTransaction(ctx, tx) + return t.store.settleScheduledArtifacts(t, run, execution, settled) +} diff --git a/internal/core/control.go b/internal/core/control.go index c025bf59..f5cca693 100644 --- a/internal/core/control.go +++ b/internal/core/control.go @@ -15,6 +15,7 @@ type ControlRequest struct { Service *Claims Identity *Claims verification *revisionVerification + artifacts *artifactVerification } // Control is the finite internal command API. Controllers have no table-write or SQL interface. @@ -32,6 +33,17 @@ func (s *Store) Control(ctx context.Context, r *ControlRequest) (Object, error) copyRequest.verification = prepared r = ©Request } + // A scheduled run's artifacts are checked the same way: the object store is asked between two + // short transactions, and the answer travels bound to the frozen execution input. + if r.Action == "clone_takeover" && r.Body.O("result").S("outcome") == "scheduled_artifacts_delivered" { + prepared, err := s.prepareArtifactVerification(ctx, r) + if err != nil { + return nil, err + } + copyRequest := *r + copyRequest.artifacts = prepared + r = ©Request + } out, err := s.transact(ctx, func(t *transaction) (response Object) { defer func() { if t.residualFactConflict && response != nil { diff --git a/internal/core/node_execution.go b/internal/core/node_execution.go index 5a0777ea..5a2cf930 100644 --- a/internal/core/node_execution.go +++ b/internal/core/node_execution.go @@ -140,7 +140,7 @@ func nodeExecutionResult(t *transaction, r *ControlRequest, withReceipt bool) Ob } t.exec("UPDATE node_executions SET result=$2,updated_at=now() WHERE execution_id=$1", execution, jsonText(result)) } - if withReceipt || e.S("kind") == "agent_session" || e.S("kind") == "deliver_revision" { + if withReceipt || e.S("kind") == "agent_session" || e.S("kind") == "deliver_revision" || e.S("kind") == "deliver_scheduled_artifacts" { require(withReceipt, 400, "invalid_receipt") // Plugin queries settle the result without an ACK receipt. Only their first real // terminal event may add that missing receipt; later terminal replays, sessions and @@ -160,6 +160,9 @@ func nodeExecutionResult(t *transaction, r *ControlRequest, withReceipt bool) Ob if fresh && e.S("kind") == "deliver_revision" && revisionSuccess(result) { settleVerifiedRevision(t, r, e, lateDelivery) } + if fresh && e.S("kind") == "deliver_scheduled_artifacts" { + settleVerifiedArtifacts(t, r, operation, execution, result) + } return t.one("SELECT "+nodeExecutionColumns+" FROM node_executions WHERE execution_id=$1", execution) } @@ -173,6 +176,22 @@ func validateNodeResult(e, result Object) { case "agent_session": require(result.S("outcome") == "agent_session_ended", 409, "result_conflict") require(sessionEndReasons[result.S("reason")], 400, "invalid_result") + case "deliver_scheduled_artifacts": + // A scheduled run reports either what it uploaded, as objects Cloud verifies itself, or why it + // could not. The declared set is never taken from this payload: Cloud reads it from the + // session's immutable terminal result. + switch result.S("outcome") { + case "scheduled_artifacts_delivered": + uploaded := objectRows(result["artifacts"]) + require(len(uploaded) <= maxScheduledArtifactItems, 400, "invalid_result") + for _, object := range uploaded { + require(object.S("key") != "" && object.N("size") > 0 && lowerHexDigest(object.S("sha256")), 400, "invalid_result") + } + case "scheduled_artifacts_failed": + require(scheduledArtifactsFailureReasons[result.S("reason")], 400, "invalid_result") + default: + reject(409, "result_conflict") + } default: switch result.S("outcome") { case "revision_delivered", "revision_unchanged": @@ -189,6 +208,20 @@ var sessionEndReasons = map[string]bool{ "user_ended": true, "idle_timeout": true, "cancelled": true, "agent_failed": true, "interrupted": true, } +// settleVerifiedArtifacts hands a scheduled run's artifact delivery to the business side. A reported +// failure carries no verification (there is nothing to verify), while a reported upload must come +// with the object-store verdict prepared outside this transaction. +func settleVerifiedArtifacts(t *transaction, r *ControlRequest, runID, executionID string, result Object) { + if result.S("outcome") == "scheduled_artifacts_failed" { + scheduledArtifactsSettled(t, runID, executionID, Object{"outcome": "failed", "reason": result.S("reason")}) + return + } + require(r.artifacts != nil, 409, "invalid_delivery_verification") + scheduledArtifactsSettled(t, runID, executionID, Object{ + "outcome": "delivered", "input": r.artifacts.input, "objects": r.artifacts.objects, + }) +} + // revisionFailureReasons is the closed set a Node may report. The retired bundle_failed value is // reserved on the wire, so a result carrying it decodes to a reason outside this set and is refused. var revisionFailureReasons = map[string]bool{ diff --git a/internal/core/scheduled_artifact_db_test.go b/internal/core/scheduled_artifact_db_test.go new file mode 100644 index 00000000..4438c6fe --- /dev/null +++ b/internal/core/scheduled_artifact_db_test.go @@ -0,0 +1,117 @@ +package core + +import ( + "context" + "strings" + "testing" +) + +// DB tests for the scheduled-run artifact delivery (specs decisions/cloud/scheduled-task/ +// 20261022-run-verdict-and-artifacts.md D3). They drive the shipped settlement on a real isolated +// schema; the object-store side is covered where a deployment can be given one. + +// scheduledManifest is the declaration an executor reports for one run. +func scheduledManifest() Object { + return Object{ + "items": []Object{{ + "index": 0, "displayName": "reports/review.md", "kind": "markdown", + "sizeBytes": 5, "sha256": strings.Repeat("ab", 32), + }}, + "rejections": []Object{{"displayName": "notes.pdf", "reason": "unsupported_type"}}, + } +} + +// A deployment without object storage cannot upload anything, so the delivery converges as skipped +// in the session-terminal transaction: the declaration is still recorded, the refused candidate stays +// visible, no upload work item is declared, and the run releases rather than holding its Workspace +// against a limit that can never be reached. +func TestScheduledArtifactsSkipWithoutObjectStorage(t *testing.T) { + store := dispatcherDB(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + ended := Object{ + "outcome": "agent_session_ended", "reason": "idle_timeout", + "node": Object{"nodeId": scene.node}, + "artifactManifest": scheduledManifest(), + } + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + // The real takeover: it persists the session's terminal result, which is what the release + // path reads its end reason from, and it runs the business hooks in this transaction. + executionResult(tx, &ControlRequest{Action: "clone_takeover", Body: Object{ + "operationId": scene.run, "executionId": scene.execution, + "sequence": int64(1), "event": "scheduled-artifacts-ended", "result": ended, + }}, true) + return Object{} + }); err != nil { + t.Fatalf("settle transaction: %v", err) + } + + var state, reason string + var settled bool + if err := store.Pool.QueryRow(` + SELECT state, COALESCE(settled_reason,''), settled_at IS NOT NULL + FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1`, scene.run).Scan(&state, &reason, &settled); err != nil { + t.Fatalf("read the delivery: %v", err) + } + if state != "skipped" || reason != scheduledArtifactSkippedObjectStore || !settled { + t.Fatalf("delivery = %q/%q settled=%v, want skipped/%s", state, reason, settled, scheduledArtifactSkippedObjectStore) + } + // The refused candidate is a fact the reader must see, whatever happened to the uploads. + var rejections int + if err := store.Pool.QueryRow(`SELECT count(*) FROM scheduled_run_artifact_rejections + WHERE issue_run_id=$1 AND display_name='notes.pdf' AND reason='unsupported_type'`, scene.run).Scan(&rejections); err != nil { + t.Fatalf("count rejections: %v", err) + } + if rejections != 1 { + t.Fatalf("rejections = %d, want the declared refusal recorded", rejections) + } + // Nothing was uploaded, so nothing may be registered, and no Node may be asked to try. + var registered, work int + if err := store.Pool.QueryRow(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1`, scene.run).Scan(®istered); err != nil { + t.Fatalf("count artifacts: %v", err) + } + if registered != 0 { + t.Fatalf("registered = %d, want none without object storage", registered) + } + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work + WHERE run_id=$1 AND kind='deliver_scheduled_artifacts'`, scene.run).Scan(&work); err != nil { + t.Fatalf("count work: %v", err) + } + if work != 0 { + t.Fatalf("upload work items = %d, want none without object storage", work) + } + var phase string + if err := store.Pool.QueryRow(`SELECT phase FROM issue_runs WHERE id=$1`, scene.run).Scan(&phase); err != nil || phase != "releasing" { + t.Fatalf("run phase = %q (err %v), want releasing", phase, err) + } +} + +// A declaration outside the frozen bounds is refused instead of stored: half a manifest would let a +// run look like it had artifacts while the facts were incomplete. +func TestScheduledArtifactManifestIsValidatedBeforeStorage(t *testing.T) { + for name, manifest := range map[string]Object{ + "unsafe path": {"items": []Object{{ + "index": 0, "displayName": "../escape.md", "kind": "markdown", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32), + }}}, + "unknown kind": {"items": []Object{{ + "index": 0, "displayName": "report.sh", "kind": "shell", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32), + }}}, + "over the size bound": {"items": []Object{{ + "index": 0, "displayName": "report.md", "kind": "markdown", "sizeBytes": maxScheduledArtifactBytes + 1, + "sha256": strings.Repeat("ab", 32), + }}}, + "declared twice": {"items": []Object{ + {"index": 0, "displayName": "report.md", "kind": "markdown", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32)}, + {"index": 1, "displayName": "report.md", "kind": "markdown", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32)}, + }}, + "declared and rejected": {"items": []Object{{ + "index": 0, "displayName": "report.md", "kind": "markdown", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32), + }}, "rejections": []Object{{"displayName": "report.md", "reason": "too_large"}}}, + } { + t.Run(name, func(t *testing.T) { + if _, err := parseScheduledArtifactManifest("4b3c2d1e-0f9a-4b8c-9d7e-6f5a4b3c2d1e", "5d4c3b2a-1f0e-4d8c-9b7a-6f5e4d3c2b1a", manifest); err == nil { + t.Fatal("an invalid declaration must be refused") + } + }) + } +} diff --git a/internal/core/scheduled_artifact_delivery.go b/internal/core/scheduled_artifact_delivery.go index 69d5b3e2..92960e7c 100644 --- a/internal/core/scheduled_artifact_delivery.go +++ b/internal/core/scheduled_artifact_delivery.go @@ -27,7 +27,7 @@ const scheduledArtifactSkippedObjectStore = "object_store_unconfigured" // was told to report artifacts Cloud cannot attribute or verify. func recordScheduledArtifacts(t *transaction, runID, executionID string, ended Object) error { manifest := ended.O("artifactManifest") - if manifest == nil || !scheduledRunResultIsReportable(ended.S("reason")) { + if len(manifest) == 0 || !scheduledRunResultIsReportable(ended.S("reason")) { return nil } occurrence := t.one(` diff --git a/internal/core/scheduled_artifact_expiry.go b/internal/core/scheduled_artifact_expiry.go new file mode 100644 index 00000000..70b9654a --- /dev/null +++ b/internal/core/scheduled_artifact_expiry.go @@ -0,0 +1,63 @@ +package core + +import "context" + +// Expiry of scheduled artifact deliveries (scheduled-task verdict/artifacts ADR D3). +// +// The window is fixed when the declaration is stored and never renewed, so expiry needs no policy of +// its own: a row still `delivering` at or past its deadline is settled as failed, no object is +// registered, and the run is released if its Revision delivery has already settled. Uploaded objects +// that never got registered are cleaned up by the prefix GC, which is the only cleanup that does not +// depend on a receipt arriving at all. + +// scheduledArtifactBatchSize bounds one expiry pass so a single tick settles at most this many runs. +const scheduledArtifactBatchSize = 100 + +// ExpireScheduledArtifactDeliveriesOnce settles the artifact deliveries whose window has closed. +// +// The scan is read-only and the decision is taken again inside each short transaction against the +// database's own clock, so a delivery that settled (or was released by another tick) in between is a +// no-op rather than an error. +func (s *Store) ExpireScheduledArtifactDeliveriesOnce(ctx context.Context) error { + rows, err := s.Pool.QueryContext(ctx, ` + SELECT issue_run_id FROM scheduled_run_artifact_deliveries + WHERE state='delivering' AND deadline_at <= now() + ORDER BY deadline_at + LIMIT $1`, scheduledArtifactBatchSize) + if err != nil { + return err + } + defer rows.Close() + var expired []string + for rows.Next() { + var runID string + if err := rows.Scan(&runID); err != nil { + return err + } + expired = append(expired, runID) + } + if err := rows.Err(); err != nil { + return err + } + for _, runID := range expired { + if _, err := s.transact(ctx, func(t *transaction) Object { + delivery := t.one(` + SELECT * FROM scheduled_run_artifact_deliveries + WHERE issue_run_id=$1 AND state='delivering' AND deadline_at <= now()`, runID) + if delivery == nil { + return Object{} + } + o := t.one("SELECT * FROM issue_runs WHERE id=$1 AND deleted_at IS NULL", runID) + if o == nil { + return Object{} + } + if err := s.closeArtifactDelivery(t, o, "failed", scheduledArtifactDeadlineExpired); err != nil { + panic(databaseFailure{err}) + } + return Object{} + }); err != nil { + return err + } + } + return nil +} diff --git a/internal/core/scheduled_artifact_settle.go b/internal/core/scheduled_artifact_settle.go new file mode 100644 index 00000000..51e062dc --- /dev/null +++ b/internal/core/scheduled_artifact_settle.go @@ -0,0 +1,140 @@ +package core + +import "fmt" + +// The artifact-delivery settlement behind the control plane's OnScheduledArtifactsSettled hook +// (scheduled-task verdict/artifacts ADR D3). +// +// It runs inside the transaction that takes the delivery execution's terminal event over, and the +// object verification it consumes was computed outside that transaction: the control plane asks the +// object store whether every reported object exists with the declared size and digest, binds the +// answer to the frozen execution input, and this function refuses an answer bound to any other input. +// Nothing here performs external I/O. +// +// Two rules from the ADR decide the outcome, and neither depends on the order things arrive in: +// +// deadline the database's own clock decides. A result that lands at or after `deadline_at` +// confirms the receipt, registers nothing and settles the delivery as failed, no +// matter whether the expiry pass has already run. +// per item each declared artifact is registered only when its own object verified. Successes are +// registered, everything else becomes a visible rejection, and the run's aggregate state +// says whether all, some or none of the declared artifacts made it. + +const scheduledArtifactDeadlineExpired = "deadline_expired" + +// scheduledArtifactsFailureReasons is the closed set an executor may report. Cloud's own +// verification and deadline verdicts are not part of it: they never come from the wire. +var scheduledArtifactsFailureReasons = map[string]bool{ + "workspace_unavailable": true, "upload_failed": true, + "declaration_unavailable": true, "interrupted": true, +} + +// settleScheduledArtifacts registers one scheduled run's verified artifacts and finishes its release +// when the Revision delivery has also settled. +func (s *Store) settleScheduledArtifacts(t *transaction, runID, executionID string, settled Object) error { + if !validID(runID) { + return fmt.Errorf("scheduledArtifactsSettled: invalid run id %q", runID) + } + o := t.one("SELECT * FROM issue_runs WHERE id=$1 AND deleted_at IS NULL", runID) + if o == nil { + return fmt.Errorf("scheduledArtifactsSettled: issue_run %s not found", runID) + } + e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND operation_id=$2", executionID, runID) + if e == nil { + return fmt.Errorf("scheduledArtifactsSettled: execution %s is not registered for run %s", executionID, runID) + } + if e.S("kind") != "deliver_scheduled_artifacts" { + return fmt.Errorf("scheduledArtifactsSettled: execution %s is kind=%q, not deliver_scheduled_artifacts", executionID, e.S("kind")) + } + delivery := t.one("SELECT * FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", runID) + if delivery == nil { + return fmt.Errorf("scheduledArtifactsSettled: run %s has no artifact delivery", runID) + } + // A replay, or a receipt that lands after the window already settled, decides nothing: the + // receipt itself was committed by the takeover, which is what the Node needs to stop replaying. + if delivery.S("state") != "delivering" { + return nil + } + switch outcome := settled.S("outcome"); outcome { + case "delivered": + expected := settled.S("input") + if expected == "" || expected != jsonText(e.O("input")) { + return fmt.Errorf("scheduledArtifactsSettled: run %s execution %s carries a verification for another input", runID, executionID) + } + case "failed": + if reason := settled.S("reason"); reason != "" && !scheduledArtifactsFailureReasons[reason] { + return fmt.Errorf("scheduledArtifactsSettled: run %s carries failure reason %q, outside the closed set", runID, reason) + } + case "expired": + default: + return fmt.Errorf("scheduledArtifactsSettled: run %s carries unknown outcome %q", runID, settled.S("outcome")) + } + // 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) + if !open.B("open") { + return s.closeArtifactDelivery(t, o, "failed", scheduledArtifactDeadlineExpired) + } + if settled.S("outcome") != "delivered" { + return s.closeArtifactDelivery(t, o, "failed", settled.S("reason")) + } + declaration, err := scheduledArtifactDeclarationOf(t, delivery.S("tenantId"), e) + if err != nil { + return err + } + verified := map[string]bool{} + for _, object := range objectRows(settled["objects"]) { + verified[object.S("key")] = object.B("verified") + } + registered := 0 + for _, artifact := range declaration.Artifacts { + reason := "upload_failed" + if _, reported := verified[artifact.Key]; reported { + reason = "verification_failed" + } + if verified[artifact.Key] { + t.exec(` + INSERT INTO scheduled_run_artifacts + (tenant_id, issue_run_id, task_id, occurrence_id, scheduled_at, artifact_index, object_key, display_name, kind, size_bytes, sha256) + SELECT $1,$2,$3,$4,$5,$6,$7,$8,$9,$10,$11 + ON CONFLICT (issue_run_id, artifact_index) DO NOTHING`, + delivery.S("tenantId"), runID, delivery.S("taskId"), delivery.S("occurrenceId"), delivery.S("scheduledAt"), + artifact.Index, artifact.Key, artifact.DisplayName, artifact.Kind, artifact.Size, artifact.Sha256) + registered++ + continue + } + rejectScheduledArtifact(t, delivery, runID, artifact.DisplayName, reason) + } + state, reason := "failed", "upload_failed" + switch { + case registered == len(declaration.Artifacts) && registered > 0: + state, reason = "delivered", "" + case registered > 0: + state, reason = "partial", "upload_failed" + } + return s.closeArtifactDelivery(t, o, state, reason) +} + +// closeArtifactDelivery settles the delivery row and releases the run when its Revision delivery has +// already settled as well. +func (s *Store) closeArtifactDelivery(t *transaction, o Object, state, reason string) error { + t.exec(` + UPDATE scheduled_run_artifact_deliveries + SET state=$2, settled_at=now(), settled_reason=NULLIF($3,'') + WHERE issue_run_id=$1 AND state='delivering'`, o.S("id"), state, reason) + if !deliveryOutcomeRecorded(t, o.S("id")) { + return nil + } + return s.finishRelease(t, o) +} + +// rejectScheduledArtifact records one declared artifact that did not become a registered object. +func rejectScheduledArtifact(t *transaction, delivery Object, runID, displayName, reason string) { + t.exec(` + INSERT INTO scheduled_run_artifact_rejections + (tenant_id, issue_run_id, task_id, occurrence_id, scheduled_at, display_name, reason) + VALUES($1,$2,$3,$4,$5,$6,$7) + ON CONFLICT (issue_run_id, display_name) DO NOTHING`, + delivery.S("tenantId"), runID, delivery.S("taskId"), delivery.S("occurrenceId"), delivery.S("scheduledAt"), + displayName, reason) +} diff --git a/internal/core/scheduled_artifact_verify.go b/internal/core/scheduled_artifact_verify.go new file mode 100644 index 00000000..d29a1461 --- /dev/null +++ b/internal/core/scheduled_artifact_verify.go @@ -0,0 +1,80 @@ +package core + +import ( + "context" + + "github.com/wanglongan587/cloud/internal/objectstore" +) + +// artifactVerification is the object-store check of one artifact delivery. +// +// It is computed between two short transactions, exactly like the Revision delivery's verification: +// Cloud asks the object store whether every object the Node reported exists with the declared size +// and digest, and hands the answer to the takeover transaction bound to the frozen execution input, +// so a verdict can never be replayed against another execution. Definitive negative evidence is a +// rejection; an unreachable object store stays retryable with no takeover and no ACK. +type artifactVerification struct { + input string + objects []Object +} + +// prepareArtifactVerification authenticates the takeover, captures the frozen input and the declared +// artifacts of the session the delivery names, then checks every reported object outside any +// transaction. +func (s *Store) prepareArtifactVerification(ctx context.Context, r *ControlRequest) (*artifactVerification, error) { + expected, err := s.transact(ctx, func(t *transaction) Object { + require(r.Service != nil && r.Service.Role == "controller", 403, "service_forbidden") + leaseValid(t, r) + require(len(r.SubmissionID) <= 200, 400, "invalid_submission") + if old := t.one("SELECT * FROM control_submissions WHERE submission_id=$1", r.SubmissionID); old != nil { + require(old.S("requestHash") == requestHash(r.Action, r.Service.Subject, r.Body), 409, "submission_conflict") + return nil + } + e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND operation_id=$2 AND kind='deliver_scheduled_artifacts'", r.Body.S("executionId"), r.Body.S("operationId")) + require(e != nil, 404, "not_found") + // A committed result needs no external check: the replay returns the stored fact. + if len(e.O("result")) > 0 { + require(r.Body.O("result").O("node").S("nodeId") == e.S("nodeId"), 409, "result_conflict") + validateNodeResult(e, r.Body.O("result")) + return nil + } + validateNodeResult(e, r.Body.O("result")) + require(r.Body.N("sequence") == e.N("lastEventSequence")+1 && r.Body.S("event") != "", 409, "receipt_conflict") + delivery := t.one("SELECT * FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) + require(delivery != nil && delivery.S("state") == "delivering", 409, "dispatch_conflict") + declaration, declarationErr := scheduledArtifactDeclarationOf(t, delivery.S("tenantId"), e) + require(declarationErr == nil, 409, "dispatch_conflict") + declared := Object{} + for _, artifact := range declaration.Artifacts { + declared[artifact.Key] = Object{"size": artifact.Size, "sha256": artifact.Sha256} + } + return Object{"input": jsonText(e.O("input")), "declared": declared} + }) + if err != nil || expected == nil { + return nil, err + } + if s.ObjectStore == nil { + return nil, &Fault{Status: 503, Code: "object_store_unavailable", Params: Object{}} + } + declared := expected.O("declared") + verified := &artifactVerification{input: expected.S("input")} + for _, object := range objectRows(r.Body.O("result")["artifacts"]) { + key := object.S("key") + want := declared.O(key) + if len(want) == 0 { + // Reporting an object Cloud never declared is not a verification failure to retry: the + // item simply cannot be registered, and the settlement records the rejection. + verified.objects = append(verified.objects, Object{"key": key, "verified": false}) + continue + } + matched, verifyErr := objectstore.Verify(ctx, s.ObjectStore, key, want.N("size"), want.S("sha256")) + if verifyErr != nil { + return nil, &Fault{Status: 503, Code: "object_store_unavailable", Params: Object{}} + } + verified.objects = append(verified.objects, Object{ + "key": key, + "verified": matched && object.N("size") == want.N("size") && object.S("sha256") == want.S("sha256"), + }) + } + return verified, nil +} diff --git a/internal/core/scheduled_run_result.go b/internal/core/scheduled_run_result.go index ba60b237..4056723c 100644 --- a/internal/core/scheduled_run_result.go +++ b/internal/core/scheduled_run_result.go @@ -46,7 +46,7 @@ func scheduledRunResultIsReportable(reason string) bool { // written when the executor reported nothing, which the reader projects as "not reported". func recordScheduledRunResult(t *transaction, runID string, ended Object) error { result := ended.O("scheduledResult") - if result == nil { + if len(result) == 0 { return nil } if !scheduledRunResultIsReportable(ended.S("reason")) { diff --git a/internal/core/store.go b/internal/core/store.go index 4491ccfa..585d6fe4 100644 --- a/internal/core/store.go +++ b/internal/core/store.go @@ -114,6 +114,10 @@ type Store struct { // OnDeliverySettled receives saved/unchanged with revisionId and Revision metadata, failed with // a safe reason, or skipped when storage is unconfigured. It runs after receipt/Revision writes. OnDeliverySettled func(context.Context, *sql.Tx, string, string, Object) error + // OnScheduledArtifactsSettled receives a scheduled run's verified artifact delivery: the objects + // Cloud confirmed, per-object results and the absolute deadline verdict, computed outside the + // transaction. It runs after the receipt is recorded. + OnScheduledArtifactsSettled func(context.Context, *sql.Tx, string, string, Object) error // Collaboration ports (consuming-side seams; see collaboration.go). NewStore wires the real // workflow-backed Directory and Forms, because Cloud owns the workflow document and needs no @@ -256,12 +260,14 @@ type transaction struct { onRunWorkspaceSettled func(context.Context, *sql.Tx, string, string) error onRunWorkspaceDeleted func(context.Context, *sql.Tx, string) error onDeliverySettled func(context.Context, *sql.Tx, string, string, Object) error - queued []string - workSignals []string - commandSignals []string - events []SpaceEvent - residualFactConflict bool - residualFactSource Object + // onScheduledArtifactsSettled receives the verified artifact delivery of one scheduled run. + onScheduledArtifactsSettled func(context.Context, *sql.Tx, string, string, Object) error + queued []string + workSignals []string + commandSignals []string + events []SpaceEvent + residualFactConflict bool + residualFactSource Object } func (t *transaction) exec(q string, args ...any) { @@ -434,6 +440,7 @@ func (s *Store) transactLocked(ctx context.Context, lock func(*transaction), cla }() t := &transaction{tx: tx, ctx: ctx, store: s, pluginExecution: s.PluginExecution, directory: s.Directory, contextBuilder: s.Context, forms: s.Forms, assist: s.Assist, simulator: s.Simulator, objectStore: s.ObjectStore, onThreadEvents: s.OnThreadEvents, onSessionEnded: s.OnSessionEnded} t.onRunWorkspaceSettled, t.onRunWorkspaceDeleted, t.onDeliverySettled = s.OnRunWorkspaceSettled, s.OnRunWorkspaceDeleted, s.OnDeliverySettled + t.onScheduledArtifactsSettled = s.OnScheduledArtifactsSettled // Publish this transaction to the B-side hooks, which the seams below invoke with the *sql.Tx // alone. Without it a hook could only wrap the handle in a second transaction view, and every // enqueue, Thread entry and post-commit hint it produced would be collected in a set that is From abac4ef041a8e7488b6d089e4ca84297ea9ceca7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:45:05 +0800 Subject: [PATCH 05/22] feat(objectstore): delete objects Cloud owns through the private endpoint 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. --- internal/objectstore/delete.go | 66 +++++++++++++++++++++++++++++ internal/objectstore/delete_test.go | 44 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 internal/objectstore/delete.go create mode 100644 internal/objectstore/delete_test.go diff --git a/internal/objectstore/delete.go b/internal/objectstore/delete.go new file mode 100644 index 00000000..10efcf4d --- /dev/null +++ b/internal/objectstore/delete.go @@ -0,0 +1,66 @@ +package objectstore + +import ( + "context" + "net/http" + "time" +) + +// DeleteError is a bounded classification that never carries a signed URL or transport details. +type DeleteError string + +// Error returns the safe object-delete classification. +func (e DeleteError) Error() string { return string(e) } + +const ( + // ErrDeleteUnavailable means the store did not confirm the deletion; a later pass may retry it. + ErrDeleteUnavailable DeleteError = "object delete unavailable" +) + +// Delete removes one object Cloud owns, through the private S3 endpoint, with the same local +// presigning the uploads use: the signed URL is a server-side capability for exactly this key and +// method, and it is fetched from the private endpoint rather than the browser-facing one so a +// deployment that publishes a CDN can never route a deletion through it. +// +// An object that is already gone is success: the callers are cleanup passes that must converge on +// "not there" rather than distinguish "we deleted it" from "nothing was left to delete". Redirects +// are refused, and the caller must not hold a database transaction here. +func Delete(ctx context.Context, cfg *Config, key string) error { + if err := ctx.Err(); err != nil { + return err + } + if cfg == nil { + return ErrDeleteUnavailable + } + private := *cfg + private.PublicEndpoint = "" + grant, err := presign(&private, key, http.MethodDelete, map[string]string{}, time.Now()) + if err != nil { + return ErrDeleteUnavailable + } + req, err := http.NewRequestWithContext(ctx, grant.Method, grant.URL, http.NoBody) + if err != nil { + return ErrDeleteUnavailable + } + for name, value := range grant.Headers { + req.Header.Set(name, value) + } + client := &http.Client{ + Timeout: 30 * time.Second, + CheckRedirect: func(_ *http.Request, _ []*http.Request) error { return http.ErrUseLastResponse }, + } + resp, err := client.Do(req) + if err != nil { + if ctx.Err() != nil { + return ctx.Err() + } + return ErrDeleteUnavailable + } + defer func() { _ = resp.Body.Close() }() + switch resp.StatusCode { + case http.StatusNoContent, http.StatusOK, http.StatusNotFound: + return nil + default: + return ErrDeleteUnavailable + } +} diff --git a/internal/objectstore/delete_test.go b/internal/objectstore/delete_test.go new file mode 100644 index 00000000..75be0ef8 --- /dev/null +++ b/internal/objectstore/delete_test.go @@ -0,0 +1,44 @@ +package objectstore + +import ( + "errors" + "net/http" + "net/http/httptest" + "testing" +) + +func TestDeleteSignsThePrivateKeyAndTreatsMissingObjectsAsDone(t *testing.T) { + for name, status := range map[string]int{"deleted": http.StatusNoContent, "already gone": http.StatusNotFound} { + t.Run(name, func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete || r.URL.Path != "/revisions/scheduled-runs/tenant/run/0" || r.URL.Query().Get("X-Amz-Signature") == "" { + t.Error("delete did not bind the private key, DELETE signature and path") + } + w.WriteHeader(status) + })) + defer server.Close() + if err := Delete(t.Context(), readConfig(server.URL), "scheduled-runs/tenant/run/0"); err != nil { + t.Fatalf("delete = %v, want success", err) + } + }) + } +} + +func TestDeleteReportsUncomfirmedAndRedirectedDeletes(t *testing.T) { + failing := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusInternalServerError) + })) + defer failing.Close() + if err := Delete(t.Context(), readConfig(failing.URL), "scheduled-runs/tenant/run/0"); !errors.Is(err, ErrDeleteUnavailable) { + t.Fatalf("delete = %v, want the bounded unavailable classification", err) + } + + // A redirect must not be followed: the signed capability names one endpoint and one key. + redirecting := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, "http://elsewhere.invalid/other", http.StatusTemporaryRedirect) + })) + defer redirecting.Close() + if err := Delete(t.Context(), readConfig(redirecting.URL), "scheduled-runs/tenant/run/0"); !errors.Is(err, ErrDeleteUnavailable) { + t.Fatalf("redirected delete = %v, want the bounded unavailable classification", err) + } +} From ebe579430b094df1b5a5dded1411640921c1be30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:58:19 +0800 Subject: [PATCH 06/22] feat(scheduled): keep ten artifact runs per task and sweep unregistered 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. --- cmd/server/main.go | 14 ++ .../0063_scheduled_run_artifacts.sql | 10 + internal/core/migrations/README.en.md | 1 + internal/core/migrations/README.md | 2 + .../scheduled_artifact_cleanup_db_test.go | 144 +++++++++++++ internal/core/scheduled_artifact_retention.go | 202 ++++++++++++++++++ 6 files changed, 373 insertions(+) create mode 100644 internal/core/scheduled_artifact_cleanup_db_test.go create mode 100644 internal/core/scheduled_artifact_retention.go diff --git a/cmd/server/main.go b/cmd/server/main.go index 3eaaafff..f2fde010 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -289,6 +289,20 @@ func run() (runErr error) { // the same clock-driven pass as the others: one short transaction per run, no policy. pluginmarket.RunSyncLoop(ctx, store.ExpireScheduledArtifactDeliveriesOnce, agentDeliveryInterval, pluginmarket.ContextSleep, log) }() + syncGroup.Add(1) + go func() { + defer syncGroup.Done() + // Retention applies D5's ten-run cost boundary per task; the object-store deletions happen + // between transactions, exactly like the reads. + pluginmarket.RunSyncLoop(ctx, store.PruneScheduledRunArtifactsOnce, agentDeliveryInterval, pluginmarket.ContextSleep, log) + }() + syncGroup.Add(1) + go func() { + defer syncGroup.Done() + // The prefix sweep is the lifetime fallback for objects a presigned PUT wrote after Cloud + // stopped granting them: deterministic keys, no bucket listing, no receipt needed. + pluginmarket.RunSyncLoop(ctx, store.CollectScheduledArtifactObjectsOnce, agentDeliveryInterval, pluginmarket.ContextSleep, log) + }() } // Residual recovery remains independent of a run's business terminal state. Cloud consumes // existing, identity-bound facts; a missing fact leaves its ledger open rather than treating diff --git a/internal/core/migrations/0063_scheduled_run_artifacts.sql b/internal/core/migrations/0063_scheduled_run_artifacts.sql index 96d9acf1..43794f85 100644 --- a/internal/core/migrations/0063_scheduled_run_artifacts.sql +++ b/internal/core/migrations/0063_scheduled_run_artifacts.sql @@ -14,6 +14,10 @@ DROP INDEX one_unregistered_execution_work; CREATE UNIQUE INDEX one_unregistered_execution_work ON execution_work(run_id, kind) WHERE execution_id IS NULL AND workflow_run_id IS NULL; +-- The kind itself is a closed set; the artifact upload is the third kind of IssueRun work. +ALTER TABLE execution_work DROP CONSTRAINT execution_work_kind_check; +ALTER TABLE execution_work ADD CONSTRAINT execution_work_kind_check + CHECK (kind IN ('agent_session','deliver_revision','deliver_scheduled_artifacts')); CREATE TABLE scheduled_run_artifact_deliveries ( issue_run_id uuid PRIMARY KEY REFERENCES issue_runs(id), @@ -27,11 +31,17 @@ CREATE TABLE scheduled_run_artifact_deliveries ( settled_at timestamptz, -- 收敛原因:skipped 记 object_store_unconfigured,failed 记窗口到期或上传失败。 settled_reason text, + -- 前缀清理扫过这一刻之后就不再重扫:清理按确定性对象键删除已上传未登记的对象, + -- 不需要列举桶,但需要记住"这个运行已经清过",否则每一轮都会重删。 + artifacts_gc_at timestamptz, created_at timestamptz NOT NULL DEFAULT now() ); -- 到期结算扫描只需要仍在交付中的行。 CREATE INDEX scheduled_run_artifact_deliveries_due ON scheduled_run_artifact_deliveries(deadline_at) WHERE state='delivering'; +-- 前缀清理只关心尚未清过、且已经收敛的交付。 +CREATE INDEX scheduled_run_artifact_deliveries_gc ON scheduled_run_artifact_deliveries(settled_at) + WHERE state IN ('failed','partial','skipped') AND artifacts_gc_at IS NULL; CREATE TABLE scheduled_run_artifacts ( tenant_id uuid NOT NULL REFERENCES tenants(id), diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index f366cc59..04f305df 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -99,6 +99,7 @@ There are currently 62 SQL files. - 0060 adds `model_adapter` to `node_instances` (a jsonb object, default `{}`) recording the plugin model-adapter capability a Node instance declares at registration. Nothing is backfilled: existing instances count as having no capability and receive no session that needs an adapter until they register again with a newer version. Fresh and upgrade coverage is in `TestMigration0060ModelAdapterFreshAndUpgrade`. - 0061 adds `scheduled_tasks` (frozen prompt, agent ref, `schedule` jsonb, IANA timezone, unattended consent and soft-delete tombstone) and `scheduled_task_occurrences` (an idempotent admission ledger keyed by `(task_id, scheduled_at)`, holding the deadline instant and deadline-cancellation fact); it backfills no historical row. Admission creates an agent IssueRun under the bound issue in the same transaction, and the plugin version is snapshotted from the Space at admission rather than frozen on the definition. Semantics are in the specs root decision `decisions/cloud/scheduled-task`; fresh/upgrade/repeat integration coverage lands with the Web and end-to-end acceptance commit. - 0062 gives `scheduled_task_occurrences` a surrogate `id` (with a default, so the existing admission transaction needs no change) and adds `scheduled_run_results`: `issue_run_id` is the identity, `verdict` comes from the closed set, `verdict_detail` is at most 2000 characters, `task_id` and `scheduled_at` are the task ownership and planned instant frozen at write time, and `occurrence_id` is an audit reference only, nulled when the occurrence is trimmed. No historical run is backfilled. Semantics are in the specs decision `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`; coverage is in `TestSessionEndedWritesTheScheduledVerdict`, `TestScheduledRunResultIsWrittenOncePerRun`, `TestScheduledRunResultRefusesUnattributableReports` and `TestScheduledRunResultSurvivesOccurrenceTrim`. +- 0063 adds the three scheduled-artifact tables: the delivery state with its non-renewable ten-minute window (and its prefix-cleanup marker), the verified object index and the rejection list; all three freeze `task_id`/`scheduled_at` and keep `occurrence_id` as an audit reference only. It also narrows `execution_work`'s unregistered uniqueness from "one per run" to "one per run and kind" and adds `deliver_scheduled_artifacts` to the kind set: a scheduled run holds its Revision delivery and its artifact upload at the same time. Coverage is in `TestScheduledArtifactsSkipWithoutObjectStorage`, `TestScheduledArtifactManifestIsValidatedBeforeStorage`, `TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` and `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`. - 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 e5373505..814d64d7 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -74,6 +74,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Node 实例登记的插件模型 adapter 能力 | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | 定时任务定义、幂等接纳台账与截止取消事实 | | [0062_scheduled_run_results.sql](0062_scheduled_run_results.sql) | 定时运行的业务 verdict、冻结的任务归属与 occurrence 审计引用 | +| [0063_scheduled_run_artifacts.sql](0063_scheduled_run_artifacts.sql) | 定时产物的交付窗口、已核对登记、被拒清单,以及每运行每类一个未登记工作项 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 @@ -99,6 +100,7 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 - 0060 给 `node_instances` 增加 `model_adapter`(jsonb 对象,默认 `{}`),记录 Node 实例登记时声明的插件模型 adapter 能力;不回填,既有实例按空能力处理,不会被派发需要 adapter 的会话,直到以新版本重新登记。空库与升级覆盖见 `TestMigration0060ModelAdapterFreshAndUpgrade`。 - 0061 增加 `scheduled_tasks`(冻结 Prompt、agent ref、`schedule` jsonb、IANA 时区、无人值守同意与软删除墓碑)和 `scheduled_task_occurrences`(`(task_id, scheduled_at)` 唯一的幂等接纳台账,含截止时刻与截止取消事实);不回填历史行。接纳由工作线程在同一事务里创建绑定 issue 下的 agent IssueRun,插件版本在接纳时按 Space 当时值快照而非定义上冻结。语义见 specs 根决策 `decisions/cloud/scheduled-task`;空库、升级与重复应用的集成覆盖随 Web 与端到端验收 commit 一并补齐。 - 0062 给 `scheduled_task_occurrences` 补一个代理 `id`(默认值,既有接纳事务不用改),并增加 `scheduled_run_results`:以 `issue_run_id` 为唯一身份,`verdict` 取自 Close 集、`verdict_detail` 至多 2000 字符,`task_id` 与 `scheduled_at` 是写入时冻结的任务归属与计划时间事实,`occurrence_id` 只作审计并在裁剪 occurrence 时置空。不回填任何历史运行。语义见 specs `decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md`;覆盖见 `TestSessionEndedWritesTheScheduledVerdict`、`TestScheduledRunResultIsWrittenOncePerRun`、`TestScheduledRunResultRefusesUnattributableReports` 与 `TestScheduledRunResultSurvivesOccurrenceTrim`。 +- 0063 增加定时产物的三张表:交付状态与不可续期 10 分钟窗口(含前缀清理标记)、已核对对象索引、被拒清单;三者都冻结 `task_id`/`scheduled_at` 并把 `occurrence_id` 只作审计。同时把 `execution_work` 的未登记唯一性从"每运行一个"改为"每运行每类一个",并在 kind 闭集里加入 `deliver_scheduled_artifacts`——定时运行要同时持 Revision 交付项和产物上传项。覆盖见 `TestScheduledArtifactsSkipWithoutObjectStorage`、`TestScheduledArtifactManifestIsValidatedBeforeStorage`、`TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys` 与 `TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns`。 - 新增迁移须验证空库、前一 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/scheduled_artifact_cleanup_db_test.go b/internal/core/scheduled_artifact_cleanup_db_test.go new file mode 100644 index 00000000..b08c554a --- /dev/null +++ b/internal/core/scheduled_artifact_cleanup_db_test.go @@ -0,0 +1,144 @@ +package core + +import ( + "context" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/wanglongan587/cloud/internal/objectstore" +) + +// cleanupObjectStore is a private-endpoint stand-in that records every deletion Cloud asks for. +func cleanupObjectStore(t *testing.T) (*objectstore.Config, func() []string) { + t.Helper() + var mu sync.Mutex + var paths []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodDelete || r.URL.Query().Get("X-Amz-Signature") == "" { + t.Error("cleanup must delete through a signed private request") + } + mu.Lock() + paths = append(paths, r.URL.Path) + mu.Unlock() + w.WriteHeader(http.StatusNoContent) + })) + t.Cleanup(server.Close) + config := &objectstore.Config{ + Endpoint: server.URL, Region: "us-east-1", Bucket: "scheduled", PathStyle: true, + AccessKeyID: "test-access", SecretAccessKey: "test-secret", + } + return config, func() []string { + mu.Lock() + defer mu.Unlock() + return append([]string(nil), paths...) + } +} + +// A delivery that settled without registering its declaration leaves "uploaded but unregistered" +// objects behind. The sweep deletes exactly those deterministic keys - never a registered one - and +// remembers that it ran, so a second pass asks the store for nothing. +func TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys(t *testing.T) { + store := dispatcherDB(t) + config, deleted := cleanupObjectStore(t) + store.ObjectStore = config + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + ended := Object{ + "outcome": "agent_session_ended", "reason": "idle_timeout", + "node": Object{"nodeId": scene.node}, + "artifactManifest": scheduledManifest(), + } + if _, err := store.transact(context.Background(), func(tx *transaction) Object { + executionResult(tx, &ControlRequest{Action: "clone_takeover", Body: Object{ + "operationId": scene.run, "executionId": scene.execution, + "sequence": int64(1), "event": "artifacts-declared", "result": ended, + }}, true) + return Object{} + }); err != nil { + t.Fatalf("declare the artifacts: %v", err) + } + // Wind the delivery forward to the state the sweep reacts to, with nothing registered. + if _, err := store.Pool.Exec(` + UPDATE scheduled_run_artifact_deliveries + SET state='failed', settled_at=now(), settled_reason='upload_failed' + WHERE issue_run_id=$1`, scene.run); err != nil { + t.Fatalf("settle the delivery: %v", err) + } + var tenantID string + if err := store.Pool.QueryRow(`SELECT tenant_id FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1`, scene.run).Scan(&tenantID); err != nil { + t.Fatalf("read the tenant: %v", err) + } + + if err := store.CollectScheduledArtifactObjectsOnce(context.Background()); err != nil { + t.Fatalf("collect: %v", err) + } + want := scheduledArtifactKey(tenantID, scene.run, 0) + if paths := deleted(); len(paths) != 1 || !strings.HasSuffix(paths[0], want) { + t.Fatalf("deleted = %v, want only the unregistered key %s", paths, want) + } + var swept bool + if err := store.Pool.QueryRow(`SELECT artifacts_gc_at IS NOT NULL FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1`, scene.run).Scan(&swept); err != nil || !swept { + t.Fatalf("swept = %v (err %v), want the pass to remember it ran", swept, err) + } + // The second pass has nothing left to ask about. + if err := store.CollectScheduledArtifactObjectsOnce(context.Background()); err != nil { + t.Fatalf("second collect: %v", err) + } + if paths := deleted(); len(paths) != 1 { + t.Fatalf("deletions = %v, want the second pass to delete nothing", paths) + } +} + +// Retention keeps the ten newest artifact-bearing runs of a task and drops the rest, objects first. +func TestPruneScheduledRunArtifactsKeepsTheTenNewestRuns(t *testing.T) { + store := dispatcherDB(t) + config, deleted := cleanupObjectStore(t) + store.ObjectStore = config + taskID := "" + var runs []string + for index := 0; index <= scheduledArtifactRunsPerTask; index++ { + seed := seedDispatchScene(t, store.Pool, false) + runs = append(runs, seed.run) + var tenantID, issueID string + if err := store.Pool.QueryRow(`SELECT tenant_id, issue_id FROM issue_runs WHERE id=$1`, seed.run).Scan(&tenantID, &issueID); err != nil { + t.Fatalf("read the run: %v", err) + } + if taskID == "" { + if err := store.Pool.QueryRow(` + INSERT INTO scheduled_tasks + (id,tenant_id,issue_id,creator_user_id,agent_id,name,prompt,schedule,timezone,revision,version,enabled) + SELECT gen_random_uuid(), $1, $2, (SELECT id FROM users ORDER BY id LIMIT 1), + (SELECT id FROM space_agents ORDER BY id LIMIT 1), + 'retention','review','{"kind":"daily","hour":0,"minute":0}'::jsonb,'UTC',1,1,false + RETURNING id`, tenantID, issueID).Scan(&taskID); err != nil { + t.Fatalf("stage the task: %v", err) + } + } + if _, err := store.Pool.Exec(` + INSERT INTO scheduled_run_artifacts + (tenant_id, issue_run_id, task_id, scheduled_at, artifact_index, object_key, display_name, kind, size_bytes, sha256) + VALUES($1,$2,$3, now() - make_interval(secs => $4), 0, $5, 'report.md', 'markdown', 5, $6)`, + tenantID, seed.run, taskID, (scheduledArtifactRunsPerTask-index)*60, scheduledArtifactKey(tenantID, seed.run, 0), + strings.Repeat("ab", 32)); err != nil { + t.Fatalf("stage the artifact: %v", err) + } + } + + if err := store.PruneScheduledRunArtifactsOnce(context.Background()); err != nil { + t.Fatalf("prune: %v", err) + } + // The oldest run is gone; the ten newest still hold their index. + var remaining int + if err := store.Pool.QueryRow(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1`, runs[0]).Scan(&remaining); err != nil || remaining != 0 { + t.Fatalf("oldest run artifacts = %d (err %v), want pruned", remaining, err) + } + if err := store.Pool.QueryRow(`SELECT count(*) FROM scheduled_run_artifacts WHERE task_id=$1`, taskID).Scan(&remaining); err != nil || remaining != scheduledArtifactRunsPerTask { + t.Fatalf("task artifacts = %d (err %v), want the ten newest runs", remaining, err) + } + if paths := deleted(); len(paths) != 1 { + t.Fatalf("deleted = %v, want the pruned run's object", paths) + } +} diff --git a/internal/core/scheduled_artifact_retention.go b/internal/core/scheduled_artifact_retention.go new file mode 100644 index 00000000..e2b7a0e4 --- /dev/null +++ b/internal/core/scheduled_artifact_retention.go @@ -0,0 +1,202 @@ +package core + +import ( + "context" + + "github.com/wanglongan587/cloud/internal/objectstore" +) + +// Retention and prefix cleanup of scheduled-run artifacts (scheduled-task verdict/artifacts ADR D5). +// +// Both passes follow the repository's bounded-pass shape: a read-only scan outside the advisory lock, +// the object-store calls between transactions, then one short transaction per run. Neither makes a +// business decision the durable state has not already made - retention only applies the ten-run cost +// boundary, and the prefix sweep only removes objects the index does not back. + +const ( + // scheduledArtifactRunsPerTask is D5's cost boundary: ten artifact-bearing runs per task, newest + // first by the frozen `scheduled_at`, never by an occurrence row the history trim may have removed. + scheduledArtifactRunsPerTask = 10 + // scheduledArtifactCleanupBatch bounds one pass so a single tick advances at most this many runs. + scheduledArtifactCleanupBatch = 100 +) + +// PruneScheduledRunArtifactsOnce drops the artifacts of the runs beyond a task's retention boundary. +// +// Objects are deleted before their index rows, and outside the transaction: a crash between the two +// leaves rows that the next pass repeats, and re-deleting a missing object is success. The row is +// re-checked under the transaction so a delivery that started registering in between is not pruned +// mid-flight. +func (s *Store) PruneScheduledRunArtifactsOnce(ctx context.Context) error { + rows, err := s.Pool.QueryContext(ctx, ` + SELECT issue_run_id FROM ( + SELECT issue_run_id, + row_number() OVER (PARTITION BY task_id ORDER BY scheduled_at DESC, issue_run_id DESC) AS position + FROM (SELECT DISTINCT issue_run_id, task_id, scheduled_at FROM scheduled_run_artifacts) covering + ) ranked + WHERE position > $1 + ORDER BY issue_run_id + LIMIT $2`, scheduledArtifactRunsPerTask, scheduledArtifactCleanupBatch) + if err != nil { + return err + } + defer rows.Close() + var runs []string + for rows.Next() { + var runID string + if err := rows.Scan(&runID); err != nil { + return err + } + runs = append(runs, runID) + } + if err := rows.Err(); err != nil { + return err + } + for _, runID := range runs { + keys, err := s.scheduledArtifactKeys(ctx, runID) + if err != nil { + return err + } + for _, key := range keys { + if err := objectstore.Delete(ctx, s.ObjectStore, key); err != nil { + return err + } + } + if _, err := s.transact(ctx, func(t *transaction) Object { + if !scheduledRunBeyondRetention(t, runID) { + return Object{} + } + t.exec("DELETE FROM scheduled_run_artifacts WHERE issue_run_id=$1", runID) + t.exec("DELETE FROM scheduled_run_artifact_rejections WHERE issue_run_id=$1", runID) + return Object{} + }); err != nil { + return err + } + } + return nil +} + +// scheduledRunBeyondRetention re-reads the retention position of one run inside the pruning +// transaction, and refuses to prune a delivery that is still inside its window. +func scheduledRunBeyondRetention(t *transaction, runID string) bool { + row := t.one(` + WITH ranked AS ( + SELECT issue_run_id, + row_number() OVER (PARTITION BY task_id ORDER BY scheduled_at DESC, issue_run_id DESC) AS position + FROM (SELECT DISTINCT issue_run_id, task_id, scheduled_at FROM scheduled_run_artifacts) covering + ) + SELECT position FROM ranked WHERE issue_run_id=$1`, runID) + if row == nil || row.N("position") <= scheduledArtifactRunsPerTask { + return false + } + return !artifactDeliveryPending(t, runID) +} + +// CollectScheduledArtifactObjectsOnce deletes the objects of a delivery that settled without +// registering them. +// +// Keys are Cloud's own and deterministic (`scheduledArtifactKey`), so the sweep needs no bucket +// listing: it can name every object the run could have written and delete the ones the index does not +// back. That is the lifetime fallback the ADR asks for - it depends on no receipt, because a +// presigned PUT stays valid after Cloud stopped granting them. +func (s *Store) CollectScheduledArtifactObjectsOnce(ctx context.Context) error { + rows, err := s.Pool.QueryContext(ctx, ` + SELECT issue_run_id, tenant_id FROM scheduled_run_artifact_deliveries + WHERE state IN ('failed','partial','skipped') AND artifacts_gc_at IS NULL + ORDER BY settled_at + LIMIT $1`, scheduledArtifactCleanupBatch) + if err != nil { + return err + } + defer rows.Close() + type delivery struct{ runID, tenantID string } + var pending []delivery + for rows.Next() { + var entry delivery + if err := rows.Scan(&entry.runID, &entry.tenantID); err != nil { + return err + } + pending = append(pending, entry) + } + if err := rows.Err(); err != nil { + return err + } + for _, entry := range pending { + unregistered, err := s.unregisteredScheduledArtifactKeys(ctx, entry.tenantID, entry.runID) + if err != nil { + return err + } + for _, key := range unregistered { + if err := objectstore.Delete(ctx, s.ObjectStore, key); err != nil { + return err + } + } + if _, err := s.transact(ctx, func(t *transaction) Object { + t.exec(`UPDATE scheduled_run_artifact_deliveries SET artifacts_gc_at=now() + WHERE issue_run_id=$1 AND state IN ('failed','partial','skipped') AND artifacts_gc_at IS NULL`, entry.runID) + return Object{} + }); err != nil { + return err + } + } + return nil +} + +// scheduledArtifactKeys lists the object keys one run's index backs. +func (s *Store) scheduledArtifactKeys(ctx context.Context, runID string) ([]string, error) { + rows, err := s.Pool.QueryContext(ctx, ` + SELECT object_key FROM scheduled_run_artifacts WHERE issue_run_id=$1 ORDER BY artifact_index`, runID) + if err != nil { + return nil, err + } + defer rows.Close() + var keys []string + for rows.Next() { + var key string + if err := rows.Scan(&key); err != nil { + return nil, err + } + keys = append(keys, key) + } + return keys, rows.Err() +} + +// unregisteredScheduledArtifactKeys derives the keys a run declared but never registered. +func (s *Store) unregisteredScheduledArtifactKeys(ctx context.Context, tenantID, runID string) ([]string, error) { + declared, err := s.Pool.QueryContext(ctx, ` + SELECT (entry.item->>'index')::int + FROM scheduled_run_artifact_deliveries d + JOIN node_executions e ON e.operation_id = d.issue_run_id AND e.kind = 'agent_session' + CROSS JOIN LATERAL jsonb_array_elements(COALESCE(e.result->'artifactManifest'->'items','[]'::jsonb)) AS entry(item) + WHERE d.issue_run_id=$1`, runID) + if err != nil { + return nil, err + } + defer declared.Close() + var keys []string + for declared.Next() { + var index int + if err := declared.Scan(&index); err != nil { + return nil, err + } + keys = append(keys, scheduledArtifactKey(tenantID, runID, index)) + } + if err := declared.Err(); err != nil { + return nil, err + } + registered, err := s.scheduledArtifactKeys(ctx, runID) + if err != nil { + return nil, err + } + backed := map[string]bool{} + for _, key := range registered { + backed[key] = true + } + orphans := make([]string, 0, len(keys)) + for _, key := range keys { + if !backed[key] { + orphans = append(orphans, key) + } + } + return orphans, nil +} From bb9b3d3c4f82443179fc3051392e577793c87009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 12:04:57 +0800 Subject: [PATCH 07/22] fix(controlgrpc): annotate the bounded artifact size conversion for gosec --- internal/controlgrpc/agent_runs.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/controlgrpc/agent_runs.go b/internal/controlgrpc/agent_runs.go index ce8de518..b6a697fd 100644 --- a/internal/controlgrpc/agent_runs.go +++ b/internal/controlgrpc/agent_runs.go @@ -111,6 +111,7 @@ func (s *agentRunService) GrantScheduledArtifactUpload(ctx context.Context, req return nil, invalidInput("invalid_upload_checksum") } artifacts = append(artifacts, core.Object{ + // #nosec G115 -- sizes above MaxInt64 were refused just above. "index": int64(artifact.GetIndex()), "size": int64(artifact.GetSize()), "sha256": artifact.GetSha256(), }) } From cda3a537d5dbff2bd0f59f830bef10527754aa6d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:18:43 +0800 Subject: [PATCH 08/22] fix(scheduled): route the artifact upload grant through the execution 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. --- internal/core/clone.go | 12 +++++++++++- internal/core/clone_test.go | 24 ++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 internal/core/clone_test.go diff --git a/internal/core/clone.go b/internal/core/clone.go index 62af2bd1..db642c0b 100644 --- a/internal/core/clone.go +++ b/internal/core/clone.go @@ -151,6 +151,16 @@ func cloneResult(t *transaction, r *ControlRequest, withReceipt bool) Object { // isCloneAction reports whether a control action belongs to the execution registry, including the // Agent session methods that share its lease and submission rules. +// +// The upload grants are listed explicitly rather than matched by prefix: a grant is issued against a +// registered execution under the same lease, so it must reach the same handler the Node's claim does, +// and a typo in a new action name must not silently fall through to the operation command path where +// it would be answered by a different handler with a different error. func isCloneAction(action string) bool { - return strings.HasPrefix(action, "clone_") || strings.HasPrefix(action, "thread_") || action == "grant_revision_upload" + switch action { + case "grant_revision_upload", "grant_scheduled_artifact_upload": + return true + default: + return strings.HasPrefix(action, "clone_") || strings.HasPrefix(action, "thread_") + } } diff --git a/internal/core/clone_test.go b/internal/core/clone_test.go new file mode 100644 index 00000000..57993281 --- /dev/null +++ b/internal/core/clone_test.go @@ -0,0 +1,24 @@ +package core + +import "testing" + +// Both upload grants are issued against a registered execution, so they must reach the execution +// registry's handler rather than the operation command path. A new grant action that stops being +// routed is a silent capability outage: the RPC answers with a different handler's error. +func TestIsCloneActionRoutesBothUploadGrants(t *testing.T) { + for _, action := range []string{ + "grant_revision_upload", + "grant_scheduled_artifact_upload", + "clone_takeover", + "thread_events", + } { + if !isCloneAction(action) { + t.Fatalf("isCloneAction(%q) = false, want the execution registry to answer it", action) + } + } + for _, action := range []string{"grant", "grant_", "operation_command", ""} { + if isCloneAction(action) { + t.Fatalf("isCloneAction(%q) = true, want it to stay on the operation command path", action) + } + } +} From f79676cdc2737b59536061ddeb7a6dbb16bf8a16 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:21:51 +0800 Subject: [PATCH 09/22] fix(scheduled): carry the declared artifacts across the session result 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. --- internal/controlgrpc/execution_codec.go | 73 ++++++++++++++++++++ internal/controlgrpc/scheduled_codec_test.go | 62 +++++++++++++++++ 2 files changed, 135 insertions(+) diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index a1cb0e59..223d01c6 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -326,6 +326,37 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { } out["scheduledResult"] = scheduled } + // The declaration is the executor's report of what it will upload; it travels with the + // terminal event so Cloud can freeze the window and hand out grants. Dropping it here would + // lose the run's artifacts silently - the settlement would see an empty manifest and stop - + // so the boundary either carries it whole or refuses it. + if declared := ended.GetArtifactManifest(); declared != nil { + items := make([]core.Object, 0, len(declared.GetItems())) + for _, item := range declared.GetItems() { + if item.GetSizeBytes() > math.MaxInt64 { + return nil, invalidInput("invalid_result") + } + kind := shortEnum(item.GetKind().String(), "SCHEDULED_ARTIFACT_KIND_") + if kind == "" { + return nil, invalidInput("invalid_result") + } + items = append(items, core.Object{ + // The durable object stores plain integers: N() reads float64/int64, so a uint32 or + // uint64 kept as-is would read back as zero and silently lose the index or size. + "index": int64(item.GetIndex()), "displayName": item.GetDisplayName(), "kind": kind, + "sizeBytes": int64(item.GetSizeBytes()), "sha256": item.GetSha256(), + }) + } + rejections := make([]core.Object, 0, len(declared.GetRejections())) + for _, rejection := range declared.GetRejections() { + reason := shortEnum(rejection.GetReason().String(), "SCHEDULED_ARTIFACT_REJECTION_REASON_") + if reason == "" { + return nil, invalidInput("invalid_result") + } + rejections = append(rejections, core.Object{"displayName": rejection.GetDisplayName(), "reason": reason}) + } + out["artifactManifest"] = core.Object{"items": items, "rejections": rejections} + } case *controlpb.ExecutionResult_RevisionDelivered: out["outcome"] = "revision_delivered" delivered := outcome.RevisionDelivered @@ -407,6 +438,25 @@ func result(o core.Object) *controlpb.ExecutionResult { } ended.ScheduledResult = reported } + // A stored declaration returns to the Controller as the same structure it reported, so a + // recovered session is queried with the artifacts it actually carried. + if declared := o.O("artifactManifest"); len(declared) > 0 { + manifest := &controlpb.ScheduledArtifactManifest{} + for _, item := range objectsOf(declared["items"]) { + manifest.Items = append(manifest.Items, &controlpb.ScheduledArtifactItem{ + Index: uint32(item.N("index")), DisplayName: item.S("displayName"), + Kind: controlpb.ScheduledArtifactKind(enum(controlpb.ScheduledArtifactKind_value, "SCHEDULED_ARTIFACT_KIND_", item.S("kind"))), + SizeBytes: uint64(item.N("sizeBytes")), Sha256: item.S("sha256"), + }) + } + for _, rejection := range objectsOf(declared["rejections"]) { + manifest.Rejections = append(manifest.Rejections, &controlpb.ScheduledArtifactRejection{ + DisplayName: rejection.S("displayName"), + Reason: controlpb.ScheduledArtifactRejectionReason(enum(controlpb.ScheduledArtifactRejectionReason_value, "SCHEDULED_ARTIFACT_REJECTION_REASON_", rejection.S("reason"))), + }) + } + ended.ArtifactManifest = manifest + } out.Outcome = &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: ended} case "revision_delivered": out.Outcome = &controlpb.ExecutionResult_RevisionDelivered{RevisionDelivered: &controlpb.RevisionDelivered{FinalCommit: o.S("finalCommit"), BaseCommit: o.S("baseCommit"), RevisionRef: o.S("revisionRef"), History: storedMessage(o.O("history"))}} @@ -503,3 +553,26 @@ func boundedSchemaVersion(value int64) uint32 { } return uint32(value) // #nosec G115 -- bounded above. } + +// objectsOf decodes an array of objects out of a stored result. The value arrives either as the typed +// slice the codec just built or as the generic form jsonb returns, so both are accepted rather than +// making one caller reshape what the other produced. +func objectsOf(value any) []core.Object { + switch rows := value.(type) { + case []core.Object: + return rows + case []any: + out := make([]core.Object, 0, len(rows)) + for _, row := range rows { + switch item := row.(type) { + case core.Object: + out = append(out, item) + case map[string]any: + out = append(out, core.Object(item)) + } + } + return out + default: + return nil + } +} diff --git a/internal/controlgrpc/scheduled_codec_test.go b/internal/controlgrpc/scheduled_codec_test.go index b7b41c31..e5f97681 100644 --- a/internal/controlgrpc/scheduled_codec_test.go +++ b/internal/controlgrpc/scheduled_codec_test.go @@ -2,6 +2,7 @@ package controlgrpc import ( "encoding/json" + "strings" "testing" "google.golang.org/protobuf/proto" @@ -103,6 +104,67 @@ func TestScheduledRunResultCrossesAsAStructuredValue(t *testing.T) { } } +// The declared artifacts cross the contract in both directions. Losing them at this seam is silent: +// Cloud would freeze an empty window and never ask the Node to upload anything. +func TestScheduledArtifactManifestRoundTripsThroughTheResult(t *testing.T) { + declared := &controlpb.ScheduledArtifactManifest{ + Items: []*controlpb.ScheduledArtifactItem{ + {Index: 0, DisplayName: "reports/review.md", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, SizeBytes: 5, Sha256: strings.Repeat("ab", 32)}, + {Index: 1, DisplayName: "reports/result.json", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_JSON, SizeBytes: 7, Sha256: strings.Repeat("cd", 32)}, + }, + Rejections: []*controlpb.ScheduledArtifactRejection{ + {DisplayName: "notes.pdf", Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE}, + }, + } + stored, err := resultObject(&controlpb.ExecutionResult{ + Node: &controlpb.NodeIdentity{NodeId: "node-1", NodeIncarnationId: "incarnation-1"}, + Outcome: &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: &controlpb.AgentSessionEnded{ + Reason: controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_IDLE_TIMEOUT, + ArtifactManifest: declared, + }}, + }) + if err != nil { + t.Fatal(err) + } + manifest := stored.O("artifactManifest") + if items := objectsOf(manifest["items"]); len(items) != 2 || + items[0].S("displayName") != "reports/review.md" || items[0].S("kind") != "markdown" || + items[0].N("sizeBytes") != 5 || items[0].S("sha256") != strings.Repeat("ab", 32) || + items[1].N("index") != 1 || items[1].S("kind") != "json" { + t.Fatalf("stored manifest items = %+v, want both declared items verbatim", manifest["items"]) + } + if rejections := objectsOf(manifest["rejections"]); len(rejections) != 1 || + rejections[0].S("displayName") != "notes.pdf" || rejections[0].S("reason") != "unsupported_type" { + t.Fatalf("stored manifest rejections = %+v, want the refusal recorded", manifest["rejections"]) + } + // The stored declaration returns to the Controller unchanged, including after a jsonb round trip. + var decoded core.Object + encoded, err := json.Marshal(stored) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatal(err) + } + back := result(decoded).GetAgentSessionEnded().GetArtifactManifest() + if !proto.Equal(back, declared) { + t.Fatalf("manifest did not survive the round trip:\n got %v\nwant %v", back, declared) + } + // A run without artifacts carries no manifest, which is what "declared nothing" looks like. + bare, err := resultObject(&controlpb.ExecutionResult{ + Node: &controlpb.NodeIdentity{NodeId: "node-1", NodeIncarnationId: "incarnation-1"}, + Outcome: &controlpb.ExecutionResult_AgentSessionEnded{AgentSessionEnded: &controlpb.AgentSessionEnded{ + Reason: controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_USER_ENDED, + }}, + }) + if err != nil { + t.Fatal(err) + } + if bare["artifactManifest"] != nil { + t.Fatalf("a run that declared nothing must store no manifest, got %+v", bare["artifactManifest"]) + } +} + // objectEqual compares two durable objects by their canonical encoding. func objectEqual(got, want core.Object) bool { gotText, gotErr := json.Marshal(got) From fb4dc933c48264b859c748cdc8a5d136036e6812 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:36:50 +0800 Subject: [PATCH 10/22] fix(scheduled): register independent artifact delivery executions --- internal/core/agent_run_residual_fact.go | 2 +- ...0064_scheduled_artifact_execution_kind.sql | 14 +++++ internal/core/node_execution.go | 13 +++-- .../scheduled_artifact_dispatch_db_test.go | 54 +++++++++++++++++++ 4 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 internal/core/migrations/0064_scheduled_artifact_execution_kind.sql create mode 100644 internal/core/scheduled_artifact_dispatch_db_test.go diff --git a/internal/core/agent_run_residual_fact.go b/internal/core/agent_run_residual_fact.go index 247d4dfa..394f750e 100644 --- a/internal/core/agent_run_residual_fact.go +++ b/internal/core/agent_run_residual_fact.go @@ -211,7 +211,7 @@ func residualExecutionIdentity(t *transaction, e Object) Object { } w := t.one("SELECT * FROM workspaces WHERE id=$1", row.S("workspaceId")) require(w != nil && w.S("tenantId") == row.S("tenantId") && w.S("projectId") == row.S("projectId") && w.S("issueRunId") == row.S("runId"), 409, "residual_identity_conflict") - if e.S("kind") == "agent_session" || e.S("kind") == "deliver_revision" { + if e.S("kind") == "agent_session" || e.S("kind") == "deliver_revision" || e.S("kind") == "deliver_scheduled_artifacts" { require(e.S("operationId") == row.S("runId"), 409, "residual_identity_conflict") work := t.one("SELECT * FROM execution_work WHERE id=$1 AND run_id=$2", e.S("workId"), row.S("runId")) require(work != nil && work.S("executionId") == e.S("executionId") && work.S("kind") == e.S("kind"), 409, "residual_identity_conflict") diff --git a/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql b/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql new file mode 100644 index 00000000..bfbbceff --- /dev/null +++ b/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql @@ -0,0 +1,14 @@ +-- The scheduled artifact upload is a run-scoped Node execution like the session and the Revision +-- delivery, so it belongs to the same closed kind set. + +ALTER TABLE node_executions DROP CONSTRAINT node_executions_kind_check; +ALTER TABLE node_executions ADD CONSTRAINT node_executions_kind_check + CHECK (kind IN ('install_plugins','remove_plugins','agent_session','deliver_revision','deliver_scheduled_artifacts')); + +-- One unfinished execution per run *and* kind. A scheduled run holds its Revision delivery and its +-- artifact upload at the same time, so the previous one-per-run rule would have refused the upload +-- while the delivery was still running, which is exactly when it must be allowed. +DROP INDEX one_pending_run_execution; +CREATE UNIQUE INDEX one_pending_run_execution ON node_executions(operation_id, kind) + WHERE kind IN ('agent_session','deliver_revision','deliver_scheduled_artifacts') + AND result IS NULL AND terminated_by_force_stop_id IS NULL; diff --git a/internal/core/node_execution.go b/internal/core/node_execution.go index 5a2cf930..23bd5f3e 100644 --- a/internal/core/node_execution.go +++ b/internal/core/node_execution.go @@ -16,7 +16,7 @@ const nodeExecutionColumns = `execution_id,kind,operation_id,work_id,workspace_i func nodeDispatch(t *transaction, r *ControlRequest) Object { operation, execution, node := r.Body.S("operationId"), r.Body.S("executionId"), r.Body.S("nodeId") input := r.Body.O("input") - if input.S("kind") == "agent_session" || input.S("kind") == "deliver_revision" { + if input.S("kind") == "agent_session" || input.S("kind") == "deliver_revision" || input.S("kind") == "deliver_scheduled_artifacts" { require(t.one("SELECT run_id FROM agent_run_residuals WHERE run_id=$1", operation) == nil, 409, "dispatch_conflict") } require(execution != "" && node != "" && len(input) > 0, 400, "invalid_dispatch") @@ -28,7 +28,7 @@ func nodeDispatch(t *transaction, r *ControlRequest) Object { switch input.S("kind") { case "install_plugins", "remove_plugins": return dispatchPluginExecution(t, r, operation, execution, node, input) - case "agent_session", "deliver_revision": + case "agent_session", "deliver_revision", "deliver_scheduled_artifacts": return dispatchRunExecution(t, r, operation, execution, node, input) default: reject(400, "invalid_dispatch") @@ -93,10 +93,15 @@ func dispatchRunExecution(t *transaction, r *ControlRequest, runID, execution, n require(t.one("SELECT execution_id FROM node_executions WHERE operation_id=$1 AND kind='agent_session'", runID) == nil, 409, "dispatch_conflict") } } - if input.S("kind") == "deliver_revision" { + if input.S("kind") == "deliver_revision" || input.S("kind") == "deliver_scheduled_artifacts" { require(t.one("SELECT execution_id FROM node_executions WHERE execution_id=$1 AND operation_id=$2 AND workspace_id=$3 AND kind='agent_session' AND result->>'outcome'='agent_session_ended'", input.S("sessionExecutionId"), runID, wid) != nil, 409, "dispatch_conflict") } - require(t.one("SELECT execution_id FROM node_executions WHERE operation_id=$1 AND kind IN ('agent_session','deliver_revision') AND result IS NULL AND terminated_by_force_stop_id IS NULL", runID) == nil, 409, "dispatch_conflict") + // Both deliveries may run together after their session ended. A session still excludes all + // run executions, and each delivery excludes another pending execution of its own kind. + require(t.one(`SELECT execution_id FROM node_executions WHERE operation_id=$1 + AND kind IN ('agent_session','deliver_revision','deliver_scheduled_artifacts') + AND (kind='agent_session' OR kind=$2 OR $2='agent_session') + AND result IS NULL AND terminated_by_force_stop_id IS NULL`, runID, input.S("kind")) == nil, 409, "dispatch_conflict") t.exec(`INSERT INTO node_executions(execution_id,kind,operation_id,work_id,workspace_id,node_id,node_operation_id,input,dispatched_epoch,workflow_node_id) VALUES($1,$2,$3,$4,$5,$6,$1,$7,$8,NULLIF($9,''))`, execution, input.S("kind"), runID, work.S("id"), wid, node, jsonText(input), r.Body.N("epoch"), work.S("workflowNodeId")) diff --git a/internal/core/scheduled_artifact_dispatch_db_test.go b/internal/core/scheduled_artifact_dispatch_db_test.go new file mode 100644 index 00000000..ff60a418 --- /dev/null +++ b/internal/core/scheduled_artifact_dispatch_db_test.go @@ -0,0 +1,54 @@ +package core + +import ( + "context" + "testing" + "time" +) + +func TestScheduledArtifactUploadIsClaimedAndDispatched(t *testing.T) { + store := dispatcherDB(t) + store.ObjectStore, _ = cleanupObjectStore(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + _, err := store.Control(context.Background(), &ControlRequest{ + Action: "clone_takeover", SubmissionID: "declare-artifacts", Service: scene.claims, + Body: Object{ + "epoch": 1, "operationId": scene.run, "executionId": scene.execution, + "sequence": 1, "event": "declared", "result": Object{ + "node": Object{"nodeId": scene.node}, "outcome": "agent_session_ended", + "reason": "idle_timeout", "artifactManifest": scheduledManifest(), + }, + }, + }) + if err != nil { + t.Fatal(err) + } + work := claimAgentWork(t, store, scene.claims) + if work.S("kind") != "deliver_scheduled_artifacts" { + t.Fatalf("claimed %q, want artifact delivery", work.S("kind")) + } + execution := "upload-execution" + first, err := dispatchAgentWork(t, store, scene.claims, work, execution, "", nil) + if err != nil { + t.Fatal(err) + } + replayed, err := dispatchAgentWork(t, store, scene.claims, work, execution, "", nil) + if err != nil || jsonText(first) != jsonText(replayed) { + t.Fatalf("dispatch replay must return the original execution: %v", err) + } + pending, err := store.Control(context.Background(), &ControlRequest{ + Action: "clone_pending", Service: scene.claims, Body: Object{"nodeId": scene.node}, + }) + if err != nil || len(objectRows(pending["executions"])) != 1 { + t.Fatalf("recovery must locate the pending upload: %v, %v", pending, err) + } + // A new work item cannot admit a second concurrent upload of the same run. + _, err = store.EnqueueExecutionWork(context.Background(), scene.run, work.S("kind"), work.O("input"), work.O("target"), time.Time{}) + if err != nil { + t.Fatal(err) + } + if _, err := dispatchAgentWork(t, store, scene.claims, work, "second-upload", "", nil); err == nil { + t.Fatal("a second pending upload must be refused") + } +} From 744ff1002e413bc68b191a028863b08f61e3ebc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:36:50 +0800 Subject: [PATCH 11/22] fix(scheduled): preserve optional facts through durable JSON round trips --- internal/controlgrpc/execution_codec.go | 2 +- internal/controlgrpc/scheduled_codec_test.go | 3 +++ internal/core/scheduled_artifact.go | 5 ++++- internal/core/scheduled_artifact_db_test.go | 22 ++++++++++++++++++++ 4 files changed, 30 insertions(+), 2 deletions(-) diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index 223d01c6..98a9f220 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -431,7 +431,7 @@ func result(o core.Object) *controlpb.ExecutionResult { } // A stored verdict returns to the Controller as the same structured value it reported, so a // recovered execution is queried with the business result it actually carried. - if scheduled := o.O("scheduledResult"); scheduled != nil { + if scheduled := o.O("scheduledResult"); len(scheduled) > 0 { reported := &controlpb.ScheduledRunResult{Verdict: controlpb.ScheduledVerdict(enum(controlpb.ScheduledVerdict_value, "SCHEDULED_VERDICT_", scheduled.S("verdict")))} if detail := scheduled.S("verdictDetail"); detail != "" { reported.VerdictDetail = &detail diff --git a/internal/controlgrpc/scheduled_codec_test.go b/internal/controlgrpc/scheduled_codec_test.go index e5f97681..5b7be33b 100644 --- a/internal/controlgrpc/scheduled_codec_test.go +++ b/internal/controlgrpc/scheduled_codec_test.go @@ -96,6 +96,9 @@ func TestScheduledRunResultCrossesAsAStructuredValue(t *testing.T) { if bare["scheduledResult"] != nil { t.Fatalf("an unreported verdict must store no field, got %+v", bare["scheduledResult"]) } + if result(bare).GetAgentSessionEnded().GetScheduledResult() != nil { + t.Fatal("an unreported verdict must also remain absent in a recovery query") + } if _, err := resultObject(endedResult(nil, controlpb.ScheduledVerdict_SCHEDULED_VERDICT_UNSPECIFIED)); err == nil { t.Fatal("an unspecified verdict must be refused") } diff --git a/internal/core/scheduled_artifact.go b/internal/core/scheduled_artifact.go index c6d3ab5b..6f7fbf73 100644 --- a/internal/core/scheduled_artifact.go +++ b/internal/core/scheduled_artifact.go @@ -174,8 +174,11 @@ func objectRows(v any) []Object { case []any: rows := make([]Object, 0, len(typed)) for _, entry := range typed { - if object, ok := entry.(Object); ok { + switch object := entry.(type) { + case Object: rows = append(rows, object) + case map[string]any: + rows = append(rows, Object(object)) } } return rows diff --git a/internal/core/scheduled_artifact_db_test.go b/internal/core/scheduled_artifact_db_test.go index 4438c6fe..a5f343de 100644 --- a/internal/core/scheduled_artifact_db_test.go +++ b/internal/core/scheduled_artifact_db_test.go @@ -2,6 +2,8 @@ package core import ( "context" + "encoding/json" + "reflect" "strings" "testing" ) @@ -21,6 +23,26 @@ func scheduledManifest() Object { } } +func TestScheduledArtifactManifestParsesDecodedJSON(t *testing.T) { + manifest := scheduledManifest() + encoded, err := json.Marshal(manifest) + if err != nil { + t.Fatal(err) + } + var decoded Object + if err := json.Unmarshal(encoded, &decoded); err != nil { + t.Fatal(err) + } + want, err := parseScheduledArtifactManifest("tenant", "run", manifest) + if err != nil { + t.Fatal(err) + } + got, err := parseScheduledArtifactManifest("tenant", "run", decoded) + if err != nil || !reflect.DeepEqual(got, want) { + t.Fatalf("decoded declaration = %+v, %v; want %+v", got, err, want) + } +} + // A deployment without object storage cannot upload anything, so the delivery converges as skipped // in the session-terminal transaction: the declaration is still recorded, the refused candidate stays // visible, no upload work item is declared, and the run releases rather than holding its Workspace From 06d8ae416a5260ba0f6ecc8771b2c03f817f1220 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:37:20 +0800 Subject: [PATCH 12/22] fix(controlgrpc): bound artifact manifest integer conversions --- internal/controlgrpc/execution_codec.go | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index 98a9f220..96859b57 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -344,7 +344,7 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { // The durable object stores plain integers: N() reads float64/int64, so a uint32 or // uint64 kept as-is would read back as zero and silently lose the index or size. "index": int64(item.GetIndex()), "displayName": item.GetDisplayName(), "kind": kind, - "sizeBytes": int64(item.GetSizeBytes()), "sha256": item.GetSha256(), + "sizeBytes": int64(item.GetSizeBytes()), "sha256": item.GetSha256(), // #nosec G115 -- values above MaxInt64 are refused above. }) } rejections := make([]core.Object, 0, len(declared.GetRejections())) @@ -443,10 +443,17 @@ func result(o core.Object) *controlpb.ExecutionResult { if declared := o.O("artifactManifest"); len(declared) > 0 { manifest := &controlpb.ScheduledArtifactManifest{} for _, item := range objectsOf(declared["items"]) { + index, size := item.N("index"), item.N("sizeBytes") + if index < 0 || index > math.MaxUint32 { + index = 0 + } + if size < 0 { + size = 0 + } manifest.Items = append(manifest.Items, &controlpb.ScheduledArtifactItem{ - Index: uint32(item.N("index")), DisplayName: item.S("displayName"), + Index: uint32(index), DisplayName: item.S("displayName"), // #nosec G115 -- bounded above. Kind: controlpb.ScheduledArtifactKind(enum(controlpb.ScheduledArtifactKind_value, "SCHEDULED_ARTIFACT_KIND_", item.S("kind"))), - SizeBytes: uint64(item.N("sizeBytes")), Sha256: item.S("sha256"), + SizeBytes: uint64(size), Sha256: item.S("sha256"), // #nosec G115 -- nonnegative above. }) } for _, rejection := range objectsOf(declared["rejections"]) { From 015172252f13c6519a4ef26d4666e0a542f34445 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:37:55 +0800 Subject: [PATCH 13/22] fix(scheduled): acknowledge expired artifact receipts without storage probes --- internal/core/scheduled_artifact_verify.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/internal/core/scheduled_artifact_verify.go b/internal/core/scheduled_artifact_verify.go index d29a1461..cd339f78 100644 --- a/internal/core/scheduled_artifact_verify.go +++ b/internal/core/scheduled_artifact_verify.go @@ -32,16 +32,21 @@ func (s *Store) prepareArtifactVerification(ctx context.Context, r *ControlReque } e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND operation_id=$2 AND kind='deliver_scheduled_artifacts'", r.Body.S("executionId"), r.Body.S("operationId")) require(e != nil, 404, "not_found") + require(r.Body.O("result").O("node").S("nodeId") == e.S("nodeId"), 409, "result_conflict") // A committed result needs no external check: the replay returns the stored fact. if len(e.O("result")) > 0 { - require(r.Body.O("result").O("node").S("nodeId") == e.S("nodeId"), 409, "result_conflict") validateNodeResult(e, r.Body.O("result")) return nil } validateNodeResult(e, r.Body.O("result")) require(r.Body.N("sequence") == e.N("lastEventSequence")+1 && r.Body.S("event") != "", 409, "receipt_conflict") - delivery := t.one("SELECT * FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) - require(delivery != nil && delivery.S("state") == "delivering", 409, "dispatch_conflict") + delivery := t.one("SELECT *, now() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) + require(delivery != nil, 409, "dispatch_conflict") + // Closed windows only take over the original event and its receipt. Return the frozen + // input so settlement can fence it without contacting storage or registering objects. + if delivery.S("state") != "delivering" || !delivery.B("open") { + return Object{"input": jsonText(e.O("input"))} + } declaration, declarationErr := scheduledArtifactDeclarationOf(t, delivery.S("tenantId"), e) require(declarationErr == nil, 409, "dispatch_conflict") declared := Object{} @@ -53,6 +58,9 @@ func (s *Store) prepareArtifactVerification(ctx context.Context, r *ControlReque if err != nil || expected == nil { return nil, err } + if expected["declared"] == nil { + return &artifactVerification{input: expected.S("input")}, nil + } if s.ObjectStore == nil { return nil, &Fault{Status: 503, Code: "object_store_unavailable", Params: Object{}} } From 81f8558305b640b53ea43db3ee9f2b7a43044e0f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:37:56 +0800 Subject: [PATCH 14/22] fix(scheduled): cap grants and repeat orphan cleanup after late PUTs --- internal/core/README.en.md | 12 ++ internal/core/README.md | 9 ++ .../0065_scheduled_artifact_recurring_gc.sql | 8 ++ .../scheduled_artifact_cleanup_db_test.go | 12 ++ internal/core/scheduled_artifact_grants.go | 21 ++- .../core/scheduled_artifact_late_put_test.go | 125 ++++++++++++++++++ internal/core/scheduled_artifact_retention.go | 14 +- 7 files changed, 190 insertions(+), 11 deletions(-) create mode 100644 internal/core/migrations/0065_scheduled_artifact_recurring_gc.sql create mode 100644 internal/core/scheduled_artifact_late_put_test.go diff --git a/internal/core/README.en.md b/internal/core/README.en.md index 9e671cd8..65b726c3 100644 --- a/internal/core/README.en.md +++ b/internal/core/README.en.md @@ -24,6 +24,18 @@ This package owns business state, authorization, transactions and recovery decis | `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) | +## Scheduled-run artifact delivery + +`scheduled_artifact*.go` persists declarations, signs grants, verifies objects and settles delivery under +the [verdict/artifact ADR](https://github.com/ora-space/specs/blob/main/decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md). +Revision and artifact deliveries dispatch independently, with at most one unfinished execution of +each kind; both must settle before the run Workspace releases. Upload grants never outlive the run's +fixed deadline. Expired receipts still acknowledge the original execution without HEAD or registration. +Object verification and deletion happen outside transactions. Cleanup derives deterministic keys and +deletes objects absent from the index. Settled runs become eligible for another sweep after one minute, +including successful deliveries whose index retention has pruned. `artifacts_gc_at` records the last +sweep, never permanent completion. + ## Store and transaction boundaries - `NewStore` obtains `*sql.DB` from injected GORM and wires the real Workflow Directory/Forms; it neither connects nor migrates nor creates a global database. diff --git a/internal/core/README.md b/internal/core/README.md index 8bcbe441..d2a42724 100644 --- a/internal/core/README.md +++ b/internal/core/README.md @@ -24,6 +24,15 @@ | `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) | +## 定时运行的产物交付 + +`scheduled_artifact*.go` 保存声明、签发授权、核验对象并收敛交付,依据 +[verdict 与产物 ADR](https://github.com/ora-space/specs/blob/main/decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md)。 +Revision 与产物可以分别派发,每类最多一个未完成执行,两者均收敛后才释放运行 Workspace。 +上传授权不超过运行的固定截止;到期回执仍按原执行身份确认,但不请求 HEAD、不登记对象。 +对象核验与删除都在事务外执行。清理按确定性键删除索引未覆盖的对象,已结算运行每分钟可再次扫描, +包含已被保留策略淘汰索引的成功交付;`artifacts_gc_at` 表示上次扫描时间,不表示永久完成。 + ## Store 与事务边界 - `NewStore` 从注入的 GORM 获取 `*sql.DB`,装配真实 Workflow Directory/Forms;不连接、迁移或建立全局数据库。 diff --git a/internal/core/migrations/0065_scheduled_artifact_recurring_gc.sql b/internal/core/migrations/0065_scheduled_artifact_recurring_gc.sql new file mode 100644 index 00000000..497b4b81 --- /dev/null +++ b/internal/core/migrations/0065_scheduled_artifact_recurring_gc.sql @@ -0,0 +1,8 @@ +-- A sweep is an observation, not proof that an admitted PUT can no longer complete. Keep every +-- settled delivery eligible, including delivered runs whose index retention can prune later. +DROP INDEX scheduled_run_artifact_deliveries_gc; +CREATE INDEX scheduled_run_artifact_deliveries_gc + ON scheduled_run_artifact_deliveries(artifacts_gc_at NULLS FIRST, settled_at, issue_run_id) + WHERE state IN ('delivered','failed','partial','skipped'); +COMMENT ON COLUMN scheduled_run_artifact_deliveries.artifacts_gc_at IS + 'Last successful sweep; settled deliveries are checked again for late PUTs and pruned objects'; diff --git a/internal/core/scheduled_artifact_cleanup_db_test.go b/internal/core/scheduled_artifact_cleanup_db_test.go index b08c554a..27d761fe 100644 --- a/internal/core/scheduled_artifact_cleanup_db_test.go +++ b/internal/core/scheduled_artifact_cleanup_db_test.go @@ -90,6 +90,18 @@ func TestCollectScheduledArtifactObjectsRemovesOnlyUnregisteredKeys(t *testing.T if paths := deleted(); len(paths) != 1 { t.Fatalf("deletions = %v, want the second pass to delete nothing", paths) } + // Delivered runs also remain eligible after retention removes their index. A PUT that finished + // after pruning must not become permanently unreachable to the collector. + if _, err := store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET state='delivered', artifacts_gc_at=now()-interval '2 minutes' WHERE issue_run_id=$1`, scene.run); err != nil { + t.Fatal(err) + } + if err := store.CollectScheduledArtifactObjectsOnce(context.Background()); err != nil { + t.Fatal(err) + } + if paths := deleted(); len(paths) != 2 || !strings.HasSuffix(paths[1], want) { + t.Fatalf("a later pass must revisit a pruned delivered run: %v", paths) + } } // Retention keeps the ten newest artifact-bearing runs of a task and drops the rest, objects first. diff --git a/internal/core/scheduled_artifact_grants.go b/internal/core/scheduled_artifact_grants.go index fe1881eb..d38e1b27 100644 --- a/internal/core/scheduled_artifact_grants.go +++ b/internal/core/scheduled_artifact_grants.go @@ -17,9 +17,8 @@ var errScheduledDeclarationUnavailable = errors.New("scheduled artifact declarat // the Node can neither pick a location nor store different bytes than it declared. Nothing about the // URL is written down: the signature is returned to the caller and forgotten. // -// The absolute delivery deadline is enforced where it matters - at registration, on the database's -// own clock - so a grant issued just before the window closes may still be used, and whatever it -// writes past the deadline is refused there and cleaned up by the prefix GC. +// Grants expire no later than the run's frozen deadline. An already-started PUT can still finish +// later; registration fences it on the database clock and the recurring GC removes its object. func scheduledArtifactGrants(t *transaction, executionID string, requested []Object) Object { e := t.one("SELECT * FROM node_executions WHERE execution_id=$1 AND kind='deliver_scheduled_artifacts'", executionID) require(e != nil, 404, "not_found") @@ -27,8 +26,18 @@ func scheduledArtifactGrants(t *transaction, executionID string, requested []Obj require(t.objectStore != nil, 409, "object_store_unconfigured") delivery := t.one("SELECT * FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) require(delivery != nil && delivery.S("state") == "delivering", 409, "dispatch_conflict") - open := t.one("SELECT now() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) - require(open.B("open"), 409, "artifact_delivery_expired") + clock := t.one("SELECT clock_timestamp() AS now") + at, timeErr := time.Parse(time.RFC3339Nano, clock.S("now")) + require(timeErr == nil, 409, "dispatch_conflict") + deadline, timeErr := time.Parse(time.RFC3339Nano, delivery.S("deadlineAt")) + require(timeErr == nil, 409, "dispatch_conflict") + config := *t.objectStore + if config.UploadGrantTTL <= 0 { + config.UploadGrantTTL = 15 * time.Minute + } + config.UploadGrantTTL = min(config.UploadGrantTTL, deadline.Sub(at)).Truncate(time.Second) + // SigV4 has whole-second precision. Never round a sub-second remainder up past the deadline. + require(config.UploadGrantTTL >= time.Second, 409, "artifact_delivery_expired") declaration, err := scheduledArtifactDeclarationOf(t, delivery.S("tenantId"), e) require(err == nil, 400, "invalid_upload_checksum") grants := make([]Object, 0, len(requested)) @@ -36,7 +45,7 @@ func scheduledArtifactGrants(t *transaction, executionID string, requested []Obj artifact := declaration.artifact(int(want.N("index"))) require(artifact != nil, 400, "invalid_upload_checksum") require(want.N("size") == artifact.Size && want.S("sha256") == artifact.Sha256, 400, "invalid_upload_checksum") - signed, presignErr := objectstore.PresignPUTChecksum(t.objectStore, artifact.Key, artifact.Sha256, time.Now()) + signed, presignErr := objectstore.PresignPUTChecksum(&config, artifact.Key, artifact.Sha256, at) require(presignErr == nil, 400, "invalid_upload_checksum") grants = append(grants, Object{ "objectKey": artifact.Key, "url": signed.URL, "method": signed.Method, diff --git a/internal/core/scheduled_artifact_late_put_test.go b/internal/core/scheduled_artifact_late_put_test.go new file mode 100644 index 00000000..3c2ee3f8 --- /dev/null +++ b/internal/core/scheduled_artifact_late_put_test.go @@ -0,0 +1,125 @@ +package core + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "github.com/wanglongan587/cloud/internal/objectstore" +) + +// The object store can admit a PUT before expiry and finish writing after Cloud's first DELETE. +// Synchronize that exact ordering; a successful first sweep is never permanent proof of absence. +func TestScheduledArtifactGCRemovesPUTThatFinishesAfterFirstSweep(t *testing.T) { + store := dispatcherDB(t) + started, finish := make(chan struct{}), make(chan struct{}) + var release sync.Once + var mu sync.Mutex + exists := false + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodPut: + close(started) + <-finish + mu.Lock() + exists = true + mu.Unlock() + w.WriteHeader(http.StatusOK) + case http.MethodDelete: + mu.Lock() + exists = false + mu.Unlock() + w.WriteHeader(http.StatusNoContent) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } + })) + t.Cleanup(server.Close) + t.Cleanup(func() { release.Do(func() { close(finish) }) }) + store.ObjectStore = &objectstore.Config{ + Endpoint: server.URL, Region: "us-east-1", Bucket: "artifacts", PathStyle: true, + AccessKeyID: "test-access", SecretAccessKey: "test-secret", + } + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + _, err := store.Control(context.Background(), &ControlRequest{ + Action: "clone_takeover", Service: scene.claims, Body: Object{ + "epoch": 1, "operationId": scene.run, "executionId": scene.execution, "sequence": 1, "event": "declared", + "result": Object{"node": Object{"nodeId": scene.node}, "outcome": "agent_session_ended", "reason": "idle_timeout", "artifactManifest": scheduledManifest()}, + }, + }) + if err != nil { + t.Fatal(err) + } + work := claimAgentWork(t, store, scene.claims) + if _, err := dispatchAgentWork(t, store, scene.claims, work, "late-put-upload", "", nil); err != nil { + t.Fatal(err) + } + granted, err := store.Control(context.Background(), &ControlRequest{ + Action: "grant_scheduled_artifact_upload", Service: scene.claims, Body: Object{ + "epoch": 1, "executionId": "late-put-upload", + "artifacts": []Object{{"index": 0, "size": 5, "sha256": strings.Repeat("ab", 32)}}, + }, + }) + if err != nil { + t.Fatal(err) + } + grant := objectRows(granted["grants"])[0] + req, err := http.NewRequestWithContext(context.Background(), http.MethodPut, grant.S("url"), strings.NewReader("hello")) + if err != nil { + t.Fatal(err) + } + headers, ok := grant["headers"].(map[string]string) + if !ok { + t.Fatal("grant must carry the signed headers") + } + for name, value := range headers { + req.Header.Set(name, value) + } + done := make(chan error, 1) + go func() { + resp, err := server.Client().Do(req) + if err == nil { + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + err = fmt.Errorf("upload status %d", resp.StatusCode) + } + } + done <- err + }() + <-started + if _, err := store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries SET state='failed', + settled_at=now(), deadline_at=now()-interval '1 second' WHERE issue_run_id=$1`, scene.run); err != nil { + t.Fatal(err) + } + if err := store.CollectScheduledArtifactObjectsOnce(context.Background()); err != nil { + t.Fatal(err) + } + release.Do(func() { close(finish) }) + if err := <-done; err != nil { + t.Fatal(err) + } + mu.Lock() + lateObject := exists + mu.Unlock() + if !lateObject { + t.Fatal("the admitted PUT must finish after the first sweep") + } + // Make the next periodic pass due without sleeps or wall-clock races. + if _, err := store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET artifacts_gc_at=now()-interval '2 minutes' WHERE issue_run_id=$1`, scene.run); err != nil { + t.Fatal(err) + } + if err := store.CollectScheduledArtifactObjectsOnce(context.Background()); err != nil { + t.Fatal(err) + } + mu.Lock() + defer mu.Unlock() + if exists { + t.Fatal("a later sweep must remove the object written after the first DELETE") + } +} diff --git a/internal/core/scheduled_artifact_retention.go b/internal/core/scheduled_artifact_retention.go index e2b7a0e4..b95551d0 100644 --- a/internal/core/scheduled_artifact_retention.go +++ b/internal/core/scheduled_artifact_retention.go @@ -97,13 +97,17 @@ func scheduledRunBeyondRetention(t *transaction, runID string) bool { // // Keys are Cloud's own and deterministic (`scheduledArtifactKey`), so the sweep needs no bucket // listing: it can name every object the run could have written and delete the ones the index does not -// back. That is the lifetime fallback the ADR asks for - it depends on no receipt, because a -// presigned PUT stays valid after Cloud stopped granting them. +// back. Settled runs remain eligible for recurring sweeps: a PUT admitted before grant expiry can +// finish after an earlier DELETE, and retention can remove an index after a successful delivery. func (s *Store) CollectScheduledArtifactObjectsOnce(ctx context.Context) error { + if s.ObjectStore == nil { + return nil + } rows, err := s.Pool.QueryContext(ctx, ` SELECT issue_run_id, tenant_id FROM scheduled_run_artifact_deliveries - WHERE state IN ('failed','partial','skipped') AND artifacts_gc_at IS NULL - ORDER BY settled_at + WHERE state IN ('delivered','failed','partial','skipped') + AND (artifacts_gc_at IS NULL OR artifacts_gc_at <= now() - interval '1 minute') + ORDER BY artifacts_gc_at NULLS FIRST, settled_at, issue_run_id LIMIT $1`, scheduledArtifactCleanupBatch) if err != nil { return err @@ -133,7 +137,7 @@ func (s *Store) CollectScheduledArtifactObjectsOnce(ctx context.Context) error { } if _, err := s.transact(ctx, func(t *transaction) Object { t.exec(`UPDATE scheduled_run_artifact_deliveries SET artifacts_gc_at=now() - WHERE issue_run_id=$1 AND state IN ('failed','partial','skipped') AND artifacts_gc_at IS NULL`, entry.runID) + WHERE issue_run_id=$1 AND state IN ('delivered','failed','partial','skipped')`, entry.runID) return Object{} }); err != nil { return err From fc13acb6c9095c1cc02e3521db4ba826e1671285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:37:56 +0800 Subject: [PATCH 15/22] test(scheduled): cover artifact delivery through real gRPC control paths --- .../scheduled_artifact_control_test.go | 243 ++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 integration/scheduled_artifact_control_test.go diff --git a/integration/scheduled_artifact_control_test.go b/integration/scheduled_artifact_control_test.go new file mode 100644 index 00000000..b5e87017 --- /dev/null +++ b/integration/scheduled_artifact_control_test.go @@ -0,0 +1,243 @@ +package integration + +import ( + "context" + "net/url" + "strconv" + "testing" + "time" + + "google.golang.org/grpc/codes" + "google.golang.org/protobuf/proto" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +// scheduledArtifactScene crosses the shipped gRPC takeover, claim and dispatch boundaries. Only the +// admission precondition is staged; no artifact work or execution rows are inserted by the test. +func scheduledArtifactScene(t *testing.T, f *fixture) (liveThreadScene, string) { + t.Helper() + scene := seedLiveThreadScene(t, f) + var taskID string + must(t, f.store.Pool.QueryRow(`INSERT INTO scheduled_tasks + (id,tenant_id,issue_id,creator_user_id,agent_id,name,prompt,schedule,timezone,revision,version,enabled) + SELECT gen_random_uuid(),tenant_id,issue_id,$2,executor_id,'artifact-test','review', + '{"kind":"daily","hour":0,"minute":0}'::jsonb,'UTC',1,1,false FROM issue_runs WHERE id=$1 + RETURNING id`, scene.runID, f.uid).Scan(&taskID)) + _, err := f.store.Pool.Exec(`INSERT INTO scheduled_task_occurrences + (tenant_id,task_id,scheduled_at,outcome,task_revision,observed_at,run_id) + VALUES($1,$2,now(),'admitted',1,now(),$3)`, scene.tenantID, taskID, scene.runID) + must(t, err) + scene.start(t, f) + ended := sessionEndResult(scene, controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_IDLE_TIMEOUT, "") + ended.GetAgentSessionEnded().ArtifactManifest = &controlpb.ScheduledArtifactManifest{ + Items: []*controlpb.ScheduledArtifactItem{{ + Index: 3, DisplayName: "report.md", + Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, + SizeBytes: revisionHistorySize, Sha256: revisionHistorySHA, + }}, + Rejections: []*controlpb.ScheduledArtifactRejection{{ + DisplayName: "notes.pdf", + Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE, + }}, + } + _, err = f.executions.TakeOverNodeEvent(asController("ctrl-a"), &controlpb.TakeOverNodeEventRequest{ + Epoch: 1, SubmissionId: "declare-artifacts", OperationId: scene.runID, ExecutionId: scene.executionID, + Sequence: 1, Event: []byte("declare-artifacts"), Result: ended, + }) + must(t, err) + queried, err := f.executions.GetDispatch(asController("ctrl-a"), &controlpb.GetDispatchRequest{ExecutionId: scene.executionID}) + must(t, err) + if !proto.Equal(queried.GetRecord().GetResult(), ended) { + t.Fatalf("session recovery got %v; want %v", queried.GetRecord().GetResult(), ended) + } + var uploadExecution string + // Revision and artifact deliveries are independently dispatchable while the other is pending. + for i := 0; i < 2; i++ { + claimed, err := f.executions.ClaimWork(asController("ctrl-a"), &controlpb.ClaimWorkRequest{Epoch: 1}) + must(t, err) + item := claimed.GetItem() + if item == nil || item.GetOperationId() != scene.runID { + t.Fatal("session end must release both delivery work items") + } + execution := "scheduled-revision" + if item.GetInput().GetDeliverScheduledArtifacts() != nil { + execution = "scheduled-upload" + uploadExecution = execution + } + _, err = f.executions.RecordDispatch(asController("ctrl-a"), &controlpb.RecordDispatchRequest{ + Epoch: 1, SubmissionId: execution, OperationId: scene.runID, ExecutionId: execution, + NodeId: item.GetTarget().GetNodeId(), Input: item.GetInput(), + }) + must(t, err) + } + if uploadExecution == "" { + t.Fatal("artifact upload work was not dispatched") + } + return scene, uploadExecution +} + +func scheduledGrantRequest(execution string) *controlpb.GrantScheduledArtifactUploadRequest { + return &controlpb.GrantScheduledArtifactUploadRequest{ + Epoch: 1, ExecutionId: execution, + Artifacts: []*controlpb.ScheduledArtifactUpload{{Index: 3, Size: revisionHistorySize, Sha256: revisionHistorySHA}}, + } +} + +func scheduledUploadResult(scene liveThreadScene, key string) *controlpb.ExecutionResult { + return &controlpb.ExecutionResult{ + Node: &controlpb.NodeIdentity{NodeId: scene.nodeID, NodeIncarnationId: "inc-" + scene.nodeID[:8]}, + Outcome: &controlpb.ExecutionResult_ScheduledArtifactsDelivered{ScheduledArtifactsDelivered: &controlpb.ScheduledArtifactsDelivered{ + Artifacts: []*controlpb.StoredObject{{Key: key, Size: revisionHistorySize, Sha256: revisionHistorySHA}}, + }}, + } +} + +func TestScheduledArtifactGRPCDispatchGrantAndRegistration(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + client := controlpb.NewAgentRunServiceClient(f.controlConn) + req := scheduledGrantRequest(execution) + _, err := client.GrantScheduledArtifactUpload(asController("another-controller"), req) + if err == nil { + t.Fatal("another controller must not obtain an upload capability") + } + bad := proto.Clone(req).(*controlpb.GrantScheduledArtifactUploadRequest) + bad.Artifacts[0].Index++ + _, err = client.GrantScheduledArtifactUpload(asController("ctrl-a"), bad) + expectStatus(t, err, codes.InvalidArgument, controlpb.ErrorCode_ERROR_CODE_INVALID_INPUT) + granted, err := client.GrantScheduledArtifactUpload(asController("ctrl-a"), req) + must(t, err) + if len(granted.Grants) != 1 { + t.Fatal("exactly one declared artifact must be granted") + } + grant := granted.Grants[0] + var deadline time.Time + must(t, f.store.Pool.QueryRow(`SELECT deadline_at FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1`, scene.runID).Scan(&deadline)) + if grant.GetExpiresAt().AsTime().After(deadline) { + t.Fatal("upload grant outlives the frozen delivery deadline") + } + _, err = f.deliveryTerminal(scene, execution, "uploaded-artifact", 1, scheduledUploadResult(scene, grant.GetObjectKey()), "uploaded-artifact") + must(t, err) + if f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1 AND artifact_index=3`, scene.runID) != 1 { + t.Fatal("verified artifact was not registered through the real gRPC path") + } + if f.scalar(`SELECT count(*) FROM scheduled_run_artifact_rejections WHERE issue_run_id=$1`, scene.runID) != 1 { + t.Fatal("the executor's rejected candidate must remain visible") + } + if len(f.objects.probed()) != 1 { + t.Fatal("registration must verify exactly the declared object") + } + finishScheduledRevision(t, f, scene) + if f.scalar(`SELECT count(*) FROM issue_runs WHERE id=$1 AND phase='releasing'`, scene.runID) != 1 { + t.Fatal("the last delivery must release the run") + } +} + +func finishScheduledRevision(t *testing.T, f *fixture, scene liveThreadScene) { + t.Helper() + in := f.deliveryInputOf("scheduled-revision") + f.push(in, in.S("baseCommit")) + _, err := f.deliveryTerminal(scene, "scheduled-revision", "scheduled-revision-finished", 1, + unchangedResult(scene, in), "scheduled-revision-finished") + must(t, err) +} + +func TestScheduledArtifactGRPCRevisionSettlesFirst(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + finishScheduledRevision(t, f, scene) + if f.scalar(`SELECT count(*) FROM issue_runs WHERE id=$1 AND phase='delivering'`, scene.runID) != 1 { + t.Fatal("a settled Revision must wait for the independent artifact delivery") + } + key := "scheduled-runs/" + scene.tenantID + "/" + scene.runID + "/3" + _, err := f.deliveryTerminal(scene, execution, "artifacts-last", 1, scheduledUploadResult(scene, key), "artifacts-last") + must(t, err) + if f.scalar(`SELECT count(*) FROM issue_runs WHERE id=$1 AND phase='releasing'`, scene.runID) != 1 { + t.Fatal("artifact settlement must release a run whose Revision already settled") + } +} + +func TestScheduledArtifactGRPCGrantLifetime(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + client := controlpb.NewAgentRunServiceClient(f.controlConn) + for _, tc := range []struct { + name string + remaining string + ttl time.Duration + maximum time.Duration + }{ + {"remaining-deadline", "30 seconds", 15 * time.Minute, 30 * time.Second}, + {"configured-ttl", "10 minutes", 20 * time.Second, 20 * time.Second}, + {"subsecond-expiry", "0.5 seconds", 15 * time.Minute, 0}, + } { + t.Run(tc.name, func(t *testing.T) { + f.objects.config.UploadGrantTTL = tc.ttl + _, err := f.store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET deadline_at=clock_timestamp()+$2::interval WHERE issue_run_id=$1`, scene.runID, tc.remaining) + must(t, err) + granted, err := client.GrantScheduledArtifactUpload(asController("ctrl-a"), scheduledGrantRequest(execution)) + if tc.maximum == 0 { + if err == nil { + t.Fatal("a subsecond remainder must not round up into a new upload capability") + } + return + } + must(t, err) + grant := granted.Grants[0] + var deadline time.Time + must(t, f.store.Pool.QueryRow(`SELECT deadline_at FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1`, scene.runID).Scan(&deadline)) + u, err := url.Parse(grant.GetUrl()) + must(t, err) + issued, err := time.Parse("20060102T150405Z", u.Query().Get("X-Amz-Date")) + must(t, err) + seconds, err := strconv.Atoi(u.Query().Get("X-Amz-Expires")) + must(t, err) + lifetime := time.Duration(seconds) * time.Second + if lifetime > tc.maximum || lifetime <= 0 || issued.Add(lifetime).After(deadline) || + !issued.Add(lifetime).Equal(grant.GetExpiresAt().AsTime()) { + t.Fatal("the signed expiry must agree with both the response and the frozen deadline") + } + }) + } +} + +func TestScheduledArtifactGRPCLateSuccessACK(t *testing.T) { + for _, scanFirst := range []bool{false, true} { + name := "receipt-before-scan" + if scanFirst { + name = "scan-before-receipt" + } + t.Run(name, func(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + _, err := f.store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries SET deadline_at=now()-interval '1 second' WHERE issue_run_id=$1`, scene.runID) + must(t, err) + if scanFirst { + must(t, f.store.ExpireScheduledArtifactDeliveriesOnce(context.Background())) + } + key := "scheduled-runs/" + scene.tenantID + "/" + scene.runID + "/3" + result := scheduledUploadResult(scene, key) + // An unavailable object store must not prevent acknowledgement after the deadline. + f.store.ObjectStore = nil + wrongNode := proto.Clone(result).(*controlpb.ExecutionResult) + wrongNode.Node.NodeId = "unrelated-node" + _, err = f.deliveryTerminal(scene, execution, "wrong-node", 1, wrongNode, "late-upload") + if err == nil { + t.Fatal("expiry must not bypass the original execution's Node identity") + } + for _, submission := range []string{"late-upload", "late-upload", "late-upload-replay"} { + _, err := f.deliveryTerminal(scene, execution, submission, 1, result, "late-upload") + must(t, err) + } + if f.scalar(`SELECT count(*) FROM node_event_receipts WHERE execution_id=$1`, execution) != 1 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1`, scene.runID) != 0 { + t.Fatal("late success must ACK one receipt and register no objects") + } + if len(f.objects.probed()) != 0 { + t.Fatal("late success must not HEAD an object that cannot be registered") + } + }) + } +} From de7bd669cb0475ae9253a2a91d4a12dbe8581aea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 14:46:33 +0800 Subject: [PATCH 16/22] fix(scheduled): preserve the session and Revision concurrency fence --- .../scheduled_artifact_migration_test.go | 44 +++++++++++++++++++ ...0064_scheduled_artifact_execution_kind.sql | 11 ++--- internal/core/migrations/README.en.md | 3 ++ internal/core/migrations/README.md | 2 + 4 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 integration/scheduled_artifact_migration_test.go diff --git a/integration/scheduled_artifact_migration_test.go b/integration/scheduled_artifact_migration_test.go new file mode 100644 index 00000000..0a124cca --- /dev/null +++ b/integration/scheduled_artifact_migration_test.go @@ -0,0 +1,44 @@ +package integration + +import ( + "context" + "strings" + "testing" +) + +func TestMigration0064And0065ScheduledArtifactFreshUpgradeAndRepeat(t *testing.T) { + for _, mode := range []string{"fresh", "upgrade-from-0063"} { + t.Run(mode, func(t *testing.T) { + pool, _ := testSchema(t, "test_artifact_upgrade_") + if mode == "upgrade-from-0063" { + applyMigrationsUpTo(t, pool, migrationsBefore(t, "0064_scheduled_artifact_execution_kind.sql")) + _, err := pool.Exec(`INSERT INTO node_executions + (execution_id,kind,operation_id,node_id,node_operation_id,input,dispatched_epoch) + VALUES('legacy-plugin','install_plugins',gen_random_uuid(),'node','legacy-op','{}',1)`) + must(t, err) + } + store := newStoreOnSchema(t, pool) + must(t, store.Migrate(context.Background())) + must(t, store.Migrate(context.Background())) + must(t, store.CheckSchema(context.Background())) + // Keep plugin executions and admit independent deliveries of the same run together. + _, err := pool.Exec(`INSERT INTO node_executions + (execution_id,kind,operation_id,node_id,node_operation_id,input,dispatched_epoch) + SELECT 'new-'||kind,kind,'11111111-1111-4111-8111-111111111111'::uuid,'node','op-'||kind,'{}',1 + FROM unnest(ARRAY['install_plugins','deliver_revision','deliver_scheduled_artifacts']) AS kind`) + must(t, err) + _, err = pool.Exec(`INSERT INTO node_executions + (execution_id,kind,operation_id,node_id,node_operation_id,input,dispatched_epoch) + VALUES('duplicate','deliver_scheduled_artifacts','11111111-1111-4111-8111-111111111111','node','duplicate','{}',1)`) + if err == nil { + t.Fatal("the migrated index must reject a second pending upload for the same run") + } + var index string + must(t, pool.QueryRow(`SELECT indexdef FROM pg_indexes + WHERE schemaname=current_schema() AND indexname='scheduled_run_artifact_deliveries_gc'`).Scan(&index)) + if !strings.Contains(index, "delivered") || strings.Contains(index, "artifacts_gc_at IS NULL") { + t.Fatal("GC index must retain already-swept and pruned successful deliveries") + } + }) + } +} diff --git a/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql b/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql index bfbbceff..004110ae 100644 --- a/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql +++ b/internal/core/migrations/0064_scheduled_artifact_execution_kind.sql @@ -5,10 +5,7 @@ ALTER TABLE node_executions DROP CONSTRAINT node_executions_kind_check; ALTER TABLE node_executions ADD CONSTRAINT node_executions_kind_check CHECK (kind IN ('install_plugins','remove_plugins','agent_session','deliver_revision','deliver_scheduled_artifacts')); --- One unfinished execution per run *and* kind. A scheduled run holds its Revision delivery and its --- artifact upload at the same time, so the previous one-per-run rule would have refused the upload --- while the delivery was still running, which is exactly when it must be allowed. -DROP INDEX one_pending_run_execution; -CREATE UNIQUE INDEX one_pending_run_execution ON node_executions(operation_id, kind) - WHERE kind IN ('agent_session','deliver_revision','deliver_scheduled_artifacts') - AND result IS NULL AND terminated_by_force_stop_id IS NULL; +-- Keep the existing session/Revision mutual exclusion. Artifact uploads have their own pending +-- uniqueness, allowing a Revision and an upload together without weakening the historical fence. +CREATE UNIQUE INDEX one_pending_scheduled_artifact_execution ON node_executions(operation_id) + WHERE kind='deliver_scheduled_artifacts' AND result IS NULL AND terminated_by_force_stop_id IS NULL; diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index 04f305df..613650b9 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -74,6 +74,9 @@ There are currently 62 SQL files. | [0060_node_model_adapter.sql](0060_node_model_adapter.sql) | Plugin model-adapter capability declared by a Node instance at registration | | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | Scheduled task definitions, idempotent admission ledger and deadline-cancellation facts | | [0062_scheduled_run_results.sql](0062_scheduled_run_results.sql) | Scheduled-run business verdict with its frozen task identity and occurrence audit reference | +| [0063_scheduled_run_artifacts.sql](0063_scheduled_run_artifacts.sql) | Artifact delivery windows, verified objects, rejections and one unregistered work item per run and kind | +| [0064_scheduled_artifact_execution_kind.sql](0064_scheduled_artifact_execution_kind.sql) | Artifact execution registration and one pending execution per run and kind, preserving plugin kinds | +| [0065_scheduled_artifact_recurring_gc.sql](0065_scheduled_artifact_recurring_gc.sql) | Recurring sweeps of settled runs for late PUTs and pruned successful deliveries | 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. diff --git a/internal/core/migrations/README.md b/internal/core/migrations/README.md index 814d64d7..cd245161 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -75,6 +75,8 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0061_scheduled_tasks.sql](0061_scheduled_tasks.sql) | 定时任务定义、幂等接纳台账与截止取消事实 | | [0062_scheduled_run_results.sql](0062_scheduled_run_results.sql) | 定时运行的业务 verdict、冻结的任务归属与 occurrence 审计引用 | | [0063_scheduled_run_artifacts.sql](0063_scheduled_run_artifacts.sql) | 定时产物的交付窗口、已核对登记、被拒清单,以及每运行每类一个未登记工作项 | +| [0064_scheduled_artifact_execution_kind.sql](0064_scheduled_artifact_execution_kind.sql) | 产物执行登记闭集与每运行每类一个未完成执行,保留插件执行类型 | +| [0065_scheduled_artifact_recurring_gc.sql](0065_scheduled_artifact_recurring_gc.sql) | 已结算运行持续重扫,清理迟到 PUT 与被淘汰的成功交付 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 From beaf8c3c3a7ca93dc2efa5079b6aa970068e882f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:30:28 +0800 Subject: [PATCH 17/22] feat(control): preserve artifact manifest completeness on the wire --- .../scheduled_manifest_contract_test.go | 27 +++++++++++++ internal/controlpb/agent_executions.pb.go | 39 +++++++++++++++---- .../cloud/internal/v1/agent_executions.proto | 6 +++ 3 files changed, 65 insertions(+), 7 deletions(-) create mode 100644 internal/controlgrpc/scheduled_manifest_contract_test.go diff --git a/internal/controlgrpc/scheduled_manifest_contract_test.go b/internal/controlgrpc/scheduled_manifest_contract_test.go new file mode 100644 index 00000000..1ecdaf80 --- /dev/null +++ b/internal/controlgrpc/scheduled_manifest_contract_test.go @@ -0,0 +1,27 @@ +package controlgrpc + +import ( + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +func TestScheduledManifestCompletenessWireRoundTrip(t *testing.T) { + want := &controlpb.ScheduledArtifactManifest{ + Rejections: []*controlpb.ScheduledArtifactRejection{{DisplayName: ".ora-scheduled-artifacts", Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT}}, + OmittedRejections: 448, WalkTruncated: true, + } + encoded, err := proto.Marshal(want) + if err != nil { + t.Fatal(err) + } + got := &controlpb.ScheduledArtifactManifest{} + if err := proto.Unmarshal(encoded, got); err != nil { + t.Fatal(err) + } + if !proto.Equal(got, want) { + t.Fatalf("manifest completeness lost on wire: got %v, want %v", got, want) + } +} diff --git a/internal/controlpb/agent_executions.pb.go b/internal/controlpb/agent_executions.pb.go index 8d5ed243..f234508c 100644 --- a/internal/controlpb/agent_executions.pb.go +++ b/internal/controlpb/agent_executions.pb.go @@ -268,6 +268,8 @@ const ( ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_MANY ScheduledArtifactRejectionReason = 6 ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_TOTAL_TOO_LARGE ScheduledArtifactRejectionReason = 7 ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED ScheduledArtifactRejectionReason = 8 + // The bounded directory walk stopped before examining every candidate. + ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT ScheduledArtifactRejectionReason = 9 ) // Enum value maps for ScheduledArtifactRejectionReason. @@ -282,6 +284,7 @@ var ( 6: "SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_MANY", 7: "SCHEDULED_ARTIFACT_REJECTION_REASON_TOTAL_TOO_LARGE", 8: "SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED", + 9: "SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT", } ScheduledArtifactRejectionReason_value = map[string]int32{ "SCHEDULED_ARTIFACT_REJECTION_REASON_UNSPECIFIED": 0, @@ -293,6 +296,7 @@ var ( "SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_MANY": 6, "SCHEDULED_ARTIFACT_REJECTION_REASON_TOTAL_TOO_LARGE": 7, "SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED": 8, + "SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT": 9, } ) @@ -1431,9 +1435,13 @@ func (x *ScheduledArtifactRejection) GetReason() ScheduledArtifactRejectionReaso // Everything one scheduled run declared about its artifacts, in one message. type ScheduledArtifactManifest struct { - state protoimpl.MessageState `protogen:"open.v1"` - Items []*ScheduledArtifactItem `protobuf:"bytes,1,rep,name=items,proto3" json:"items,omitempty"` - Rejections []*ScheduledArtifactRejection `protobuf:"bytes,2,rep,name=rejections,proto3" json:"rejections,omitempty"` + state protoimpl.MessageState `protogen:"open.v1"` + Items []*ScheduledArtifactItem `protobuf:"bytes,1,rep,name=items,proto3" json:"items,omitempty"` + Rejections []*ScheduledArtifactRejection `protobuf:"bytes,2,rep,name=rejections,proto3" json:"rejections,omitempty"` + // Refused candidates whose details did not fit in the bounded rejection list. + OmittedRejections uint32 `protobuf:"varint,3,opt,name=omitted_rejections,json=omittedRejections,proto3" json:"omitted_rejections,omitempty"` + // The walk stopped at its candidate budget; an unknown number was never examined. + WalkTruncated bool `protobuf:"varint,4,opt,name=walk_truncated,json=walkTruncated,proto3" json:"walk_truncated,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -1482,6 +1490,20 @@ func (x *ScheduledArtifactManifest) GetRejections() []*ScheduledArtifactRejectio return nil } +func (x *ScheduledArtifactManifest) GetOmittedRejections() uint32 { + if x != nil { + return x.OmittedRejections + } + return 0 +} + +func (x *ScheduledArtifactManifest) GetWalkTruncated() bool { + if x != nil { + return x.WalkTruncated + } + return false +} + type DeliverRevisionSpec struct { state protoimpl.MessageState `protogen:"open.v1"` // The ended session whose Workspace and history are saved. @@ -2152,12 +2174,14 @@ const file_ora_cloud_internal_v1_agent_executions_proto_rawDesc = "" + "\x06sha256\x18\x05 \x01(\tR\x06sha256\"\x90\x01\n" + "\x1aScheduledArtifactRejection\x12!\n" + "\fdisplay_name\x18\x01 \x01(\tR\vdisplayName\x12O\n" + - "\x06reason\x18\x02 \x01(\x0e27.ora.cloud.internal.v1.ScheduledArtifactRejectionReasonR\x06reason\"\xb2\x01\n" + + "\x06reason\x18\x02 \x01(\x0e27.ora.cloud.internal.v1.ScheduledArtifactRejectionReasonR\x06reason\"\x88\x02\n" + "\x19ScheduledArtifactManifest\x12B\n" + "\x05items\x18\x01 \x03(\v2,.ora.cloud.internal.v1.ScheduledArtifactItemR\x05items\x12Q\n" + "\n" + "rejections\x18\x02 \x03(\v21.ora.cloud.internal.v1.ScheduledArtifactRejectionR\n" + - "rejections\"\xb5\x03\n" + + "rejections\x12-\n" + + "\x12omitted_rejections\x18\x03 \x01(\rR\x11omittedRejections\x12%\n" + + "\x0ewalk_truncated\x18\x04 \x01(\bR\rwalkTruncated\"\xb5\x03\n" + "\x13DeliverRevisionSpec\x120\n" + "\x14session_execution_id\x18\x01 \x01(\tR\x12sessionExecutionId\x122\n" + "\x15checkout_execution_id\x18\x02 \x01(\tR\x13checkoutExecutionId\x12\x1f\n" + @@ -2232,7 +2256,7 @@ const file_ora_cloud_internal_v1_agent_executions_proto_rawDesc = "" + "\x1cSCHEDULED_ARTIFACT_KIND_TEXT\x10\x03\x12\x1f\n" + "\x1bSCHEDULED_ARTIFACT_KIND_LOG\x10\x04\x12\x1f\n" + "\x1bSCHEDULED_ARTIFACT_KIND_XML\x10\x05\x12 \n" + - "\x1cSCHEDULED_ARTIFACT_KIND_HTML\x10\x06*\xfd\x03\n" + + "\x1cSCHEDULED_ARTIFACT_KIND_HTML\x10\x06*\xb6\x04\n" + " ScheduledArtifactRejectionReason\x123\n" + "/SCHEDULED_ARTIFACT_REJECTION_REASON_UNSPECIFIED\x10\x00\x128\n" + "4SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE\x10\x01\x123\n" + @@ -2242,7 +2266,8 @@ const file_ora_cloud_internal_v1_agent_executions_proto_rawDesc = "" + "-SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_LARGE\x10\x05\x120\n" + ",SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_MANY\x10\x06\x127\n" + "3SCHEDULED_ARTIFACT_REJECTION_REASON_TOTAL_TOO_LARGE\x10\a\x123\n" + - "/SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED\x10\b*\xde\x03\n" + + "/SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED\x10\b\x127\n" + + "3SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT\x10\t*\xde\x03\n" + "\x15RevisionFailureReason\x12'\n" + "#REVISION_FAILURE_REASON_UNSPECIFIED\x10\x00\x12/\n" + "+REVISION_FAILURE_REASON_SESSION_NOT_SETTLED\x10\x01\x120\n" + diff --git a/proto/ora/cloud/internal/v1/agent_executions.proto b/proto/ora/cloud/internal/v1/agent_executions.proto index 2add470e..da25fd7e 100644 --- a/proto/ora/cloud/internal/v1/agent_executions.proto +++ b/proto/ora/cloud/internal/v1/agent_executions.proto @@ -200,6 +200,8 @@ enum ScheduledArtifactRejectionReason { SCHEDULED_ARTIFACT_REJECTION_REASON_TOO_MANY = 6; SCHEDULED_ARTIFACT_REJECTION_REASON_TOTAL_TOO_LARGE = 7; SCHEDULED_ARTIFACT_REJECTION_REASON_READ_FAILED = 8; + // The bounded directory walk stopped before examining every candidate. + SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT = 9; } message ScheduledArtifactRejection { @@ -212,6 +214,10 @@ message ScheduledArtifactRejection { message ScheduledArtifactManifest { repeated ScheduledArtifactItem items = 1; repeated ScheduledArtifactRejection rejections = 2; + // Refused candidates whose details did not fit in the bounded rejection list. + uint32 omitted_rejections = 3; + // The walk stopped at its candidate budget; an unknown number was never examined. + bool walk_truncated = 4; } message DeliverRevisionSpec { From b661c1fb00a2918ceaac0aa389b69080af595c5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:40:45 +0800 Subject: [PATCH 18/22] fix(scheduled): align artifact admission with the Node collector --- .../scheduled_artifact_control_test.go | 23 ++--- integration/scheduled_empty_artifact_test.go | 86 +++++++++++++++++++ .../0066_scheduled_empty_artifacts.sql | 5 ++ internal/core/node_execution.go | 2 +- internal/core/scheduled_artifact.go | 33 +++++-- .../scheduled_artifact_boundaries_test.go | 28 ++++++ 6 files changed, 158 insertions(+), 19 deletions(-) create mode 100644 integration/scheduled_empty_artifact_test.go create mode 100644 internal/core/migrations/0066_scheduled_empty_artifacts.sql create mode 100644 internal/core/scheduled_artifact_boundaries_test.go diff --git a/integration/scheduled_artifact_control_test.go b/integration/scheduled_artifact_control_test.go index b5e87017..7847e8fb 100644 --- a/integration/scheduled_artifact_control_test.go +++ b/integration/scheduled_artifact_control_test.go @@ -16,6 +16,17 @@ import ( // scheduledArtifactScene crosses the shipped gRPC takeover, claim and dispatch boundaries. Only the // admission precondition is staged; no artifact work or execution rows are inserted by the test. func scheduledArtifactScene(t *testing.T, f *fixture) (liveThreadScene, string) { + t.Helper() + return scheduledArtifactSceneForManifest(t, f, &controlpb.ScheduledArtifactManifest{ + Items: []*controlpb.ScheduledArtifactItem{{ + Index: 3, DisplayName: "report.md", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, + SizeBytes: revisionHistorySize, Sha256: revisionHistorySHA, + }}, + Rejections: []*controlpb.ScheduledArtifactRejection{{DisplayName: "notes.pdf", Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE}}, + }) +} + +func scheduledArtifactSceneForManifest(t *testing.T, f *fixture, manifest *controlpb.ScheduledArtifactManifest) (liveThreadScene, string) { t.Helper() scene := seedLiveThreadScene(t, f) var taskID string @@ -30,17 +41,7 @@ func scheduledArtifactScene(t *testing.T, f *fixture) (liveThreadScene, string) must(t, err) scene.start(t, f) ended := sessionEndResult(scene, controlpb.AgentSessionEndReason_AGENT_SESSION_END_REASON_IDLE_TIMEOUT, "") - ended.GetAgentSessionEnded().ArtifactManifest = &controlpb.ScheduledArtifactManifest{ - Items: []*controlpb.ScheduledArtifactItem{{ - Index: 3, DisplayName: "report.md", - Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, - SizeBytes: revisionHistorySize, Sha256: revisionHistorySHA, - }}, - Rejections: []*controlpb.ScheduledArtifactRejection{{ - DisplayName: "notes.pdf", - Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE, - }}, - } + ended.GetAgentSessionEnded().ArtifactManifest = manifest _, err = f.executions.TakeOverNodeEvent(asController("ctrl-a"), &controlpb.TakeOverNodeEventRequest{ Epoch: 1, SubmissionId: "declare-artifacts", OperationId: scene.runID, ExecutionId: scene.executionID, Sequence: 1, Event: []byte("declare-artifacts"), Result: ended, diff --git a/integration/scheduled_empty_artifact_test.go b/integration/scheduled_empty_artifact_test.go new file mode 100644 index 00000000..0ac6d4dc --- /dev/null +++ b/integration/scheduled_empty_artifact_test.go @@ -0,0 +1,86 @@ +package integration + +import ( + "encoding/base64" + "encoding/hex" + "io" + "net/http" + "net/http/httptest" + "sync/atomic" + "testing" + + "github.com/wanglongan587/cloud/internal/controlpb" + "github.com/wanglongan587/cloud/internal/objectstore" +) + +func TestScheduledEmptyArtifactUploadsAndRegisters(t *testing.T) { + const digest = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + f := setup(t) + scene, execution := scheduledArtifactSceneForManifest(t, f, &controlpb.ScheduledArtifactManifest{ + Items: []*controlpb.ScheduledArtifactItem{{Index: 0, DisplayName: "empty.log", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_LOG, SizeBytes: 0, Sha256: digest}}, + }) + if f.scalar(`SELECT count(*) FROM node_event_receipts WHERE execution_id=$1`, scene.executionID) != 1 { + t.Fatal("an empty file must not roll back the session terminal receipt") + } + checksum, err := hex.DecodeString(digest) + must(t, err) + encoded := base64.StdEncoding.EncodeToString(checksum) + var uploaded atomic.Bool + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.Method { + case http.MethodPut: + body, err := io.ReadAll(r.Body) + if err != nil || len(body) != 0 || r.Header.Get("X-Amz-Checksum-Sha256") != encoded { + t.Error("empty PUT must contain zero bytes and the signed empty-content checksum") + w.WriteHeader(http.StatusBadRequest) + return + } + uploaded.Store(true) + w.WriteHeader(http.StatusOK) + case http.MethodHead: + if !uploaded.Load() { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Length", "0") + w.Header().Set("X-Amz-Checksum-Sha256", encoded) + w.WriteHeader(http.StatusOK) + default: + w.WriteHeader(http.StatusMethodNotAllowed) + } + })) + t.Cleanup(server.Close) + f.store.ObjectStore = &objectstore.Config{Endpoint: server.URL, Region: "us-east-1", Bucket: "artifacts", PathStyle: true, AccessKeyID: "test-access", SecretAccessKey: "test-secret"} + granted, err := controlpb.NewAgentRunServiceClient(f.controlConn).GrantScheduledArtifactUpload(asController("ctrl-a"), &controlpb.GrantScheduledArtifactUploadRequest{ + Epoch: 1, ExecutionId: execution, Artifacts: []*controlpb.ScheduledArtifactUpload{{Index: 0, Size: 0, Sha256: digest}}, + }) + must(t, err) + grant := granted.Grants[0] + req, err := http.NewRequestWithContext(t.Context(), http.MethodPut, grant.GetUrl(), http.NoBody) + must(t, err) + for key, value := range grant.GetHeaders() { + req.Header.Set(key, value) + } + resp, err := server.Client().Do(req) + must(t, err) + must(t, resp.Body.Close()) + if resp.StatusCode != http.StatusOK { + t.Fatal("empty artifact PUT failed") + } + result := &controlpb.ExecutionResult{ + Node: &controlpb.NodeIdentity{NodeId: scene.nodeID, NodeIncarnationId: "inc-" + scene.nodeID[:8]}, + Outcome: &controlpb.ExecutionResult_ScheduledArtifactsDelivered{ScheduledArtifactsDelivered: &controlpb.ScheduledArtifactsDelivered{ + Artifacts: []*controlpb.StoredObject{{Key: grant.GetObjectKey(), Size: 0, Sha256: digest}}, + }}, + } + _, err = f.deliveryTerminal(scene, execution, "empty-upload", 1, result, "empty-upload") + must(t, err) + if f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1 AND size_bytes=0 AND sha256=$2`, scene.runID, digest) != 1 { + t.Fatal("verified empty artifact must be registered") + } + f.store.ObjectStore = f.objects.config + finishScheduledRevision(t, f, scene) + if f.scalar(`SELECT count(*) FROM issue_runs WHERE id=$1 AND phase='releasing'`, scene.runID) != 1 { + t.Fatal("empty artifact must not prevent Revision delivery and Workspace release") + } +} diff --git a/internal/core/migrations/0066_scheduled_empty_artifacts.sql b/internal/core/migrations/0066_scheduled_empty_artifacts.sql new file mode 100644 index 00000000..9255d2f0 --- /dev/null +++ b/internal/core/migrations/0066_scheduled_empty_artifacts.sql @@ -0,0 +1,5 @@ +-- Empty regular files are valid Node artifacts. Keep the upper bound while accepting their +-- measured zero length throughout the persisted registration contract. +ALTER TABLE scheduled_run_artifacts DROP CONSTRAINT scheduled_run_artifacts_size_bytes_check; +ALTER TABLE scheduled_run_artifacts ADD CONSTRAINT scheduled_run_artifacts_size_bytes_check + CHECK (size_bytes >= 0 AND size_bytes <= 10485760); diff --git a/internal/core/node_execution.go b/internal/core/node_execution.go index 23bd5f3e..1c4ed684 100644 --- a/internal/core/node_execution.go +++ b/internal/core/node_execution.go @@ -190,7 +190,7 @@ func validateNodeResult(e, result Object) { uploaded := objectRows(result["artifacts"]) require(len(uploaded) <= maxScheduledArtifactItems, 400, "invalid_result") for _, object := range uploaded { - require(object.S("key") != "" && object.N("size") > 0 && lowerHexDigest(object.S("sha256")), 400, "invalid_result") + require(object.S("key") != "" && object.N("size") >= 0 && lowerHexDigest(object.S("sha256")), 400, "invalid_result") } case "scheduled_artifacts_failed": require(scheduledArtifactsFailureReasons[result.S("reason")], 400, "invalid_result") diff --git a/internal/core/scheduled_artifact.go b/internal/core/scheduled_artifact.go index 6f7fbf73..742fe736 100644 --- a/internal/core/scheduled_artifact.go +++ b/internal/core/scheduled_artifact.go @@ -2,6 +2,7 @@ package core import ( "fmt" + "math" "strings" "time" "unicode/utf8" @@ -39,6 +40,7 @@ var scheduledArtifactKinds = map[string]bool{ var scheduledArtifactRejectionReasons = map[string]bool{ "unsupported_type": true, "path_unsafe": true, "symlink": true, "hard_link": true, "too_large": true, "too_many": true, "total_too_large": true, "read_failed": true, + "candidate_limit": true, } // scheduledArtifact 是校验通过的一条声明,带有 Cloud 生成的对象键。 @@ -59,8 +61,10 @@ type scheduledArtifactRejection struct { // scheduledArtifactDeclaration 是一次声明在 Cloud 侧的权威视图。 type scheduledArtifactDeclaration struct { - Artifacts []scheduledArtifact - Rejections []scheduledArtifactRejection + Artifacts []scheduledArtifact + Rejections []scheduledArtifactRejection + OmittedRejections int64 + WalkTruncated bool } // scheduledArtifactKey 生成一个条目的对象键。键只取决于运行身份与声明序号,绝不使用用户文件名,所以 @@ -75,8 +79,13 @@ func scheduledArtifactKey(tenantID, runID string, index int) string { // 不合规、展示路径不是安全相对路径。执行端本应已经在采集时拦下这些,所以一旦出现就是矛盾的上报, // 拒绝整份声明而不是挑选一部分——半个清单会让运行看起来有产物而事实不全。 func parseScheduledArtifactManifest(tenantID, runID string, manifest Object) (scheduledArtifactDeclaration, error) { - declaration := scheduledArtifactDeclaration{} + omitted := manifest.N("omittedRejections") + if omitted < 0 || omitted > math.MaxUint32 { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: invalid omitted rejection count") + } + declaration := scheduledArtifactDeclaration{OmittedRejections: omitted, WalkTruncated: manifest.B("walkTruncated")} seen := map[string]bool{} + indices := map[int64]bool{} var total int64 entries := objectRows(manifest["items"]) if len(entries) > maxScheduledArtifactItems { @@ -87,6 +96,10 @@ func parseScheduledArtifactManifest(tenantID, runID string, manifest Object) (sc if index < 0 || index >= maxScheduledArtifactItems { return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared index %d outside 0..%d", runID, index, maxScheduledArtifactItems-1) } + if indices[index] { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared index %d twice", runID, index) + } + indices[index] = true if seen[entry.S("displayName")] { return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %q twice", runID, entry.S("displayName")) } @@ -100,8 +113,8 @@ func parseScheduledArtifactManifest(tenantID, runID string, manifest Object) (sc return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared kind %q outside the whitelist", runID, kind) } size := entry.N("sizeBytes") - if size <= 0 || size > maxScheduledArtifactBytes { - return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %d bytes, outside 1..%d", runID, size, maxScheduledArtifactBytes) + if size < 0 || size > maxScheduledArtifactBytes { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s declared %d bytes, outside 0..%d", runID, size, maxScheduledArtifactBytes) } digest := entry.S("sha256") if !lowerHexDigest(digest) { @@ -120,9 +133,15 @@ func parseScheduledArtifactManifest(tenantID, runID string, manifest Object) (sc Sha256: digest, }) } - for _, entry := range objectRows(manifest["rejections"]) { + rejections := objectRows(manifest["rejections"]) + if len(rejections) > maxScheduledArtifactItems { + return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: too many rejection details") + } + for _, entry := range rejections { name, reason := entry.S("displayName"), entry.S("reason") - if !scheduledArtifactPath(name) || !scheduledArtifactRejectionReasons[reason] { + // A rejected path can be unsafe by definition. It is bounded display text, never a key or + // a filesystem path Cloud opens, so retain it instead of rejecting the session terminal. + if name == "" || len(name) > maxScheduledArtifactNameBytes || !utf8.ValidString(name) || strings.ContainsRune(name, 0) || !scheduledArtifactRejectionReasons[reason] { return scheduledArtifactDeclaration{}, fmt.Errorf("artifactManifest: run %s reported an invalid rejection (%q, %q)", runID, name, reason) } if seen[name] { diff --git a/internal/core/scheduled_artifact_boundaries_test.go b/internal/core/scheduled_artifact_boundaries_test.go new file mode 100644 index 00000000..fa7f71a1 --- /dev/null +++ b/internal/core/scheduled_artifact_boundaries_test.go @@ -0,0 +1,28 @@ +package core + +import ( + "strings" + "testing" +) + +const emptyArtifactSHA256 = "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + +func TestScheduledArtifactManifestAcceptsEmptyFile(t *testing.T) { + manifest := Object{"items": []Object{{"index": 0, "displayName": "empty.txt", "kind": "text", "sizeBytes": 0, "sha256": emptyArtifactSHA256}}} + declared, err := parseScheduledArtifactManifest("tenant", "run", manifest) + if err != nil || len(declared.Artifacts) != 1 || declared.Artifacts[0].Size != 0 { + t.Fatalf("empty file declaration = %+v, %v", declared, err) + } +} + +func TestScheduledArtifactManifestRejectsDuplicateIndex(t *testing.T) { + for _, digest := range []string{strings.Repeat("ab", 32), strings.Repeat("cd", 32)} { + manifest := Object{"items": []Object{ + {"index": 0, "displayName": "first.txt", "kind": "text", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32)}, + {"index": 0, "displayName": "second.txt", "kind": "text", "sizeBytes": 5, "sha256": digest}, + }} + if _, err := parseScheduledArtifactManifest("tenant", "run", manifest); err == nil { + t.Fatal("different names must not share an artifact index, regardless of digest") + } + } +} From 84dcd87ff25d6a5398c0ae09586ff6105244a2dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:40:45 +0800 Subject: [PATCH 19/22] feat(scheduled): persist artifact manifest completeness --- .../scheduled_artifact_migration_test.go | 16 ++++- .../scheduled_manifest_completeness_test.go | 24 +++++++ internal/controlgrpc/execution_codec.go | 10 ++- internal/controlgrpc/scheduled_codec_test.go | 1 + internal/core/README.en.md | 5 ++ internal/core/README.md | 4 ++ .../0067_scheduled_manifest_completeness.sql | 7 ++ internal/core/migrations/README.en.md | 2 + internal/core/migrations/README.md | 2 + internal/core/scheduled_artifact_delivery.go | 12 ++-- .../scheduled_manifest_completeness_test.go | 66 +++++++++++++++++++ 11 files changed, 140 insertions(+), 9 deletions(-) create mode 100644 integration/scheduled_manifest_completeness_test.go create mode 100644 internal/core/migrations/0067_scheduled_manifest_completeness.sql create mode 100644 internal/core/scheduled_manifest_completeness_test.go diff --git a/integration/scheduled_artifact_migration_test.go b/integration/scheduled_artifact_migration_test.go index 0a124cca..6d365479 100644 --- a/integration/scheduled_artifact_migration_test.go +++ b/integration/scheduled_artifact_migration_test.go @@ -7,11 +7,15 @@ import ( ) func TestMigration0064And0065ScheduledArtifactFreshUpgradeAndRepeat(t *testing.T) { - for _, mode := range []string{"fresh", "upgrade-from-0063"} { + for _, mode := range []string{"fresh", "upgrade-from-0063", "upgrade-from-0065"} { t.Run(mode, func(t *testing.T) { pool, _ := testSchema(t, "test_artifact_upgrade_") - if mode == "upgrade-from-0063" { - applyMigrationsUpTo(t, pool, migrationsBefore(t, "0064_scheduled_artifact_execution_kind.sql")) + if mode != "fresh" { + next := "0064_scheduled_artifact_execution_kind.sql" + if mode == "upgrade-from-0065" { + next = "0066_scheduled_empty_artifacts.sql" + } + applyMigrationsUpTo(t, pool, migrationsBefore(t, next)) _, err := pool.Exec(`INSERT INTO node_executions (execution_id,kind,operation_id,node_id,node_operation_id,input,dispatched_epoch) VALUES('legacy-plugin','install_plugins',gen_random_uuid(),'node','legacy-op','{}',1)`) @@ -21,6 +25,12 @@ func TestMigration0064And0065ScheduledArtifactFreshUpgradeAndRepeat(t *testing.T must(t, store.Migrate(context.Background())) must(t, store.Migrate(context.Background())) must(t, store.CheckSchema(context.Background())) + var sizeConstraint string + must(t, pool.QueryRow(`SELECT pg_get_constraintdef(oid) FROM pg_constraint + WHERE conrelid='scheduled_run_artifacts'::regclass AND conname='scheduled_run_artifacts_size_bytes_check'`).Scan(&sizeConstraint)) + if !strings.Contains(sizeConstraint, "size_bytes >= 0") { + t.Fatal("fresh and upgraded schemas must allow empty artifacts") + } // Keep plugin executions and admit independent deliveries of the same run together. _, err := pool.Exec(`INSERT INTO node_executions (execution_id,kind,operation_id,node_id,node_operation_id,input,dispatched_epoch) diff --git a/integration/scheduled_manifest_completeness_test.go b/integration/scheduled_manifest_completeness_test.go new file mode 100644 index 00000000..d270acc3 --- /dev/null +++ b/integration/scheduled_manifest_completeness_test.go @@ -0,0 +1,24 @@ +package integration + +import ( + "testing" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +func TestScheduledManifestCompletenessSurvivesTakeoverAndRegistration(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactSceneForManifest(t, f, &controlpb.ScheduledArtifactManifest{ + Items: []*controlpb.ScheduledArtifactItem{{Index: 3, DisplayName: "report.md", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, SizeBytes: revisionHistorySize, Sha256: revisionHistorySHA}}, + Rejections: []*controlpb.ScheduledArtifactRejection{{DisplayName: ".ora-scheduled-artifacts", Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_CANDIDATE_LIMIT}}, + OmittedRejections: 448, WalkTruncated: true, + }) + key := "scheduled-runs/" + scene.tenantID + "/" + scene.runID + "/3" + _, err := f.deliveryTerminal(scene, execution, "complete-upload", 1, scheduledUploadResult(scene, key), "complete-upload") + must(t, err) + if f.scalar(`SELECT count(*) FROM scheduled_run_artifact_deliveries + WHERE issue_run_id=$1 AND omitted_rejections=448 AND walk_truncated`, scene.runID) != 1 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifact_rejections WHERE issue_run_id=$1 AND reason='candidate_limit'`, scene.runID) != 1 { + t.Fatal("manifest completeness must remain queryable after delivery settlement") + } +} diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index 96859b57..de837d3e 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -356,6 +356,12 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { rejections = append(rejections, core.Object{"displayName": rejection.GetDisplayName(), "reason": reason}) } out["artifactManifest"] = core.Object{"items": items, "rejections": rejections} + if declared.GetOmittedRejections() != 0 { + out.O("artifactManifest")["omittedRejections"] = int64(declared.GetOmittedRejections()) + } + if declared.GetWalkTruncated() { + out.O("artifactManifest")["walkTruncated"] = true + } } case *controlpb.ExecutionResult_RevisionDelivered: out["outcome"] = "revision_delivered" @@ -441,7 +447,9 @@ func result(o core.Object) *controlpb.ExecutionResult { // A stored declaration returns to the Controller as the same structure it reported, so a // recovered session is queried with the artifacts it actually carried. if declared := o.O("artifactManifest"); len(declared) > 0 { - manifest := &controlpb.ScheduledArtifactManifest{} + manifest := &controlpb.ScheduledArtifactManifest{ + OmittedRejections: boundedSchemaVersion(declared.N("omittedRejections")), WalkTruncated: declared.B("walkTruncated"), + } for _, item := range objectsOf(declared["items"]) { index, size := item.N("index"), item.N("sizeBytes") if index < 0 || index > math.MaxUint32 { diff --git a/internal/controlgrpc/scheduled_codec_test.go b/internal/controlgrpc/scheduled_codec_test.go index 5b7be33b..3219fcc0 100644 --- a/internal/controlgrpc/scheduled_codec_test.go +++ b/internal/controlgrpc/scheduled_codec_test.go @@ -111,6 +111,7 @@ func TestScheduledRunResultCrossesAsAStructuredValue(t *testing.T) { // Cloud would freeze an empty window and never ask the Node to upload anything. func TestScheduledArtifactManifestRoundTripsThroughTheResult(t *testing.T) { declared := &controlpb.ScheduledArtifactManifest{ + OmittedRejections: 448, WalkTruncated: true, Items: []*controlpb.ScheduledArtifactItem{ {Index: 0, DisplayName: "reports/review.md", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, SizeBytes: 5, Sha256: strings.Repeat("ab", 32)}, {Index: 1, DisplayName: "reports/result.json", Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_JSON, SizeBytes: 7, Sha256: strings.Repeat("cd", 32)}, diff --git a/internal/core/README.en.md b/internal/core/README.en.md index 65b726c3..5e0bed9e 100644 --- a/internal/core/README.en.md +++ b/internal/core/README.en.md @@ -36,6 +36,11 @@ deletes objects absent from the index. Settled runs become eligible for another including successful deliveries whose index retention has pruned. `artifacts_gc_at` records the last sweep, never permanent completion. +Zero-length files may be declared, uploaded and registered; indices and names are deduplicated +independently. The original session manifest and delivery row preserve `omitted_rejections` and +`walk_truncated`, so a bounded refusal list never implies a complete walk. A declaration containing +only refusals or truncation facts settles immediately without an empty upload or deadline wait. + ## Store and transaction boundaries - `NewStore` obtains `*sql.DB` from injected GORM and wires the real Workflow Directory/Forms; it neither connects nor migrates nor creates a global database. diff --git a/internal/core/README.md b/internal/core/README.md index d2a42724..77a50f4c 100644 --- a/internal/core/README.md +++ b/internal/core/README.md @@ -33,6 +33,10 @@ Revision 与产物可以分别派发,每类最多一个未完成执行,两 对象核验与删除都在事务外执行。清理按确定性键删除索引未覆盖的对象,已结算运行每分钟可再次扫描, 包含已被保留策略淘汰索引的成功交付;`artifacts_gc_at` 表示上次扫描时间,不表示永久完成。 +零字节文件可声明、上传并登记;每个条目的 index 和名称分别去重。会话原始清单及交付行保留 +`omitted_rejections` 和 `walk_truncated`,不会把有界拒绝明细当成完整遍历结果。只有拒绝或截断信息、 +没有可上传文件时,保留这些事实并直接收敛,不派发空上传或等待截止。 + ## Store 与事务边界 - `NewStore` 从注入的 GORM 获取 `*sql.DB`,装配真实 Workflow Directory/Forms;不连接、迁移或建立全局数据库。 diff --git a/internal/core/migrations/0067_scheduled_manifest_completeness.sql b/internal/core/migrations/0067_scheduled_manifest_completeness.sql new file mode 100644 index 00000000..f17a4dd7 --- /dev/null +++ b/internal/core/migrations/0067_scheduled_manifest_completeness.sql @@ -0,0 +1,7 @@ +-- Keep the bounded collector's completeness facts independently of the listed refusal details. +-- The immutable session result retains the original declaration; these columns support run reads +-- after occurrence trimming without interpreting a truncated list as a complete one. +ALTER TABLE scheduled_run_artifact_deliveries + ADD COLUMN omitted_rejections bigint NOT NULL DEFAULT 0 + CHECK (omitted_rejections >= 0 AND omitted_rejections <= 4294967295), + ADD COLUMN walk_truncated boolean NOT NULL DEFAULT false; diff --git a/internal/core/migrations/README.en.md b/internal/core/migrations/README.en.md index 613650b9..fd86ed14 100644 --- a/internal/core/migrations/README.en.md +++ b/internal/core/migrations/README.en.md @@ -77,6 +77,8 @@ There are currently 62 SQL files. | [0063_scheduled_run_artifacts.sql](0063_scheduled_run_artifacts.sql) | Artifact delivery windows, verified objects, rejections and one unregistered work item per run and kind | | [0064_scheduled_artifact_execution_kind.sql](0064_scheduled_artifact_execution_kind.sql) | Artifact execution registration and one pending execution per run and kind, preserving plugin kinds | | [0065_scheduled_artifact_recurring_gc.sql](0065_scheduled_artifact_recurring_gc.sql) | Recurring sweeps of settled runs for late PUTs and pruned successful deliveries | +| [0066_scheduled_empty_artifacts.sql](0066_scheduled_empty_artifacts.sql) | Accept zero-length artifacts in verified registration | +| [0067_scheduled_manifest_completeness.sql](0067_scheduled_manifest_completeness.sql) | Persist omitted rejection counts and walk truncation facts | 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. diff --git a/internal/core/migrations/README.md b/internal/core/migrations/README.md index cd245161..7454c5e4 100644 --- a/internal/core/migrations/README.md +++ b/internal/core/migrations/README.md @@ -77,6 +77,8 @@ SQL 通过 `core/store.go` 的 `embed.FS` 编入二进制。`Store.Migrate` 在 | [0063_scheduled_run_artifacts.sql](0063_scheduled_run_artifacts.sql) | 定时产物的交付窗口、已核对登记、被拒清单,以及每运行每类一个未登记工作项 | | [0064_scheduled_artifact_execution_kind.sql](0064_scheduled_artifact_execution_kind.sql) | 产物执行登记闭集与每运行每类一个未完成执行,保留插件执行类型 | | [0065_scheduled_artifact_recurring_gc.sql](0065_scheduled_artifact_recurring_gc.sql) | 已结算运行持续重扫,清理迟到 PUT 与被淘汰的成功交付 | +| [0066_scheduled_empty_artifacts.sql](0066_scheduled_empty_artifacts.sql) | 产物大小下界改为零,允许空文件登记 | +| [0067_scheduled_manifest_completeness.sql](0067_scheduled_manifest_completeness.sql) | 保存未列明的拒绝数和遍历截断事实 | 0050 不复制个人连接或 Key,不回填历史运行;清除配置保留 revision,运行选项独立于共享运行输入保存。 diff --git a/internal/core/scheduled_artifact_delivery.go b/internal/core/scheduled_artifact_delivery.go index 92960e7c..476b0f00 100644 --- a/internal/core/scheduled_artifact_delivery.go +++ b/internal/core/scheduled_artifact_delivery.go @@ -44,7 +44,7 @@ func recordScheduledArtifacts(t *transaction, runID, executionID string, ended O if err != nil { return err } - if len(declaration.Artifacts) == 0 && len(declaration.Rejections) == 0 { + if len(declaration.Artifacts) == 0 && len(declaration.Rejections) == 0 && declaration.OmittedRejections == 0 && !declaration.WalkTruncated { return nil } // No object storage means no upload could ever succeed: the delivery converges as skipped in the @@ -52,20 +52,22 @@ func recordScheduledArtifacts(t *transaction, runID, executionID string, ended O state, reason := "delivering", "" if t.objectStore == nil { state, reason = "skipped", scheduledArtifactSkippedObjectStore + } else if len(declaration.Artifacts) == 0 { + state, reason = "failed", "no_accepted_artifacts" } var settledAt any - if state == "skipped" { + if state != "delivering" { settledAt = time.Now() } // The window is fixed here and never renewed: a restart, a replay or a re-dispatch reuses the same // row and therefore the same deadline. t.exec(` INSERT INTO scheduled_run_artifact_deliveries - (issue_run_id, tenant_id, task_id, occurrence_id, scheduled_at, state, deadline_at, settled_at, settled_reason) - VALUES($1,$2,$3,$4,$5,$6, now() + make_interval(secs => $7), $8, NULLIF($9,'')) + (issue_run_id, tenant_id, task_id, occurrence_id, scheduled_at, state, deadline_at, settled_at, settled_reason, omitted_rejections, walk_truncated) + VALUES($1,$2,$3,$4,$5,$6, now() + make_interval(secs => $7), $8, NULLIF($9,''),$10,$11) ON CONFLICT (issue_run_id) DO NOTHING`, runID, occurrence.S("tenantId"), occurrence.S("taskId"), occurrence.S("id"), occurrence.S("scheduledAt"), - state, scheduledArtifactDeadline.Seconds(), settledAt, reason) + state, scheduledArtifactDeadline.Seconds(), settledAt, reason, declaration.OmittedRejections, declaration.WalkTruncated) for _, rejection := range declaration.Rejections { t.exec(` INSERT INTO scheduled_run_artifact_rejections diff --git a/internal/core/scheduled_manifest_completeness_test.go b/internal/core/scheduled_manifest_completeness_test.go new file mode 100644 index 00000000..0b1a8be8 --- /dev/null +++ b/internal/core/scheduled_manifest_completeness_test.go @@ -0,0 +1,66 @@ +package core + +import ( + "context" + "strings" + "testing" +) + +func TestScheduledManifestCompletenessWithoutAcceptedFiles(t *testing.T) { + for _, manifest := range []Object{ + {"items": []Object{}, "rejections": []Object{}, "omittedRejections": 12, "walkTruncated": true}, + {"rejections": []Object{{"displayName": "unsafe:name.txt", "reason": "path_unsafe"}, {"displayName": ".ora-scheduled-artifacts", "reason": "candidate_limit"}}, "omittedRejections": 448, "walkTruncated": true}, + } { + store := dispatcherDB(t) + store.ObjectStore, _ = cleanupObjectStore(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + _, err := store.Control(context.Background(), &ControlRequest{Action: "clone_takeover", Service: scene.claims, Body: Object{ + "epoch": 1, "operationId": scene.run, "executionId": scene.execution, "sequence": 1, "event": "truncated", + "result": Object{"node": Object{"nodeId": scene.node}, "outcome": "agent_session_ended", "reason": "idle_timeout", "artifactManifest": manifest}, + }}) + if err != nil { + t.Fatal(err) + } + var omitted int64 + var truncated bool + var state string + if err := store.Pool.QueryRow(`SELECT omitted_rejections,walk_truncated,state FROM scheduled_run_artifact_deliveries + WHERE issue_run_id=$1`, scene.run).Scan(&omitted, &truncated, &state); err != nil { + t.Fatal(err) + } + if omitted != manifest.N("omittedRejections") || !truncated || state != "failed" { + t.Fatalf("completeness facts = %d/%v/%s", omitted, truncated, state) + } + var pending int + if err := store.Pool.QueryRow(`SELECT count(*) FROM execution_work WHERE run_id=$1 AND kind='deliver_scheduled_artifacts'`, scene.run).Scan(&pending); err != nil || pending != 0 { + t.Fatalf("a declaration without accepted files must not wait for an upload: %d, %v", pending, err) + } + } +} + +func TestScheduledDuplicateIndexRollsBackSessionTakeover(t *testing.T) { + store := dispatcherDB(t) + store.ObjectStore, _ = cleanupObjectStore(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + manifest := Object{"items": []Object{ + {"index": 0, "displayName": "first.txt", "kind": "text", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32)}, + {"index": 0, "displayName": "second.txt", "kind": "text", "sizeBytes": 5, "sha256": strings.Repeat("ab", 32)}, + }} + _, err := store.Control(context.Background(), &ControlRequest{Action: "clone_takeover", Service: scene.claims, Body: Object{ + "epoch": 1, "operationId": scene.run, "executionId": scene.execution, "sequence": 1, "event": "invalid-index", + "result": Object{"node": Object{"nodeId": scene.node}, "outcome": "agent_session_ended", "reason": "idle_timeout", "artifactManifest": manifest}, + }}) + if err == nil { + t.Fatal("duplicate index must be refused before freezing the declaration") + } + var changes int + if err := store.Pool.QueryRow(`SELECT + (SELECT count(*) FROM node_event_receipts WHERE execution_id=$1) + + (SELECT count(*) FROM node_executions WHERE execution_id=$1 AND result IS NOT NULL) + + (SELECT count(*) FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$2) + + (SELECT count(*) FROM execution_work WHERE run_id=$2 AND kind IN ('deliver_revision','deliver_scheduled_artifacts'))`, scene.execution, scene.run).Scan(&changes); err != nil || changes != 0 { + t.Fatalf("invalid declaration must leave no partial terminal, receipt or deliveries: %d, %v", changes, err) + } +} From e0fb03f4992ca7676c16e0ecc4c149d0325a46bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 15:54:09 +0800 Subject: [PATCH 20/22] fix(controlgrpc): distinguish empty artifacts from overflowing wire sizes --- internal/controlgrpc/execution_codec.go | 3 +++ .../scheduled_artifact_size_test.go | 25 +++++++++++++++++++ 2 files changed, 28 insertions(+) create mode 100644 internal/controlgrpc/scheduled_artifact_size_test.go diff --git a/internal/controlgrpc/execution_codec.go b/internal/controlgrpc/execution_codec.go index de837d3e..ebaee230 100644 --- a/internal/controlgrpc/execution_codec.go +++ b/internal/controlgrpc/execution_codec.go @@ -378,6 +378,9 @@ func resultObject(res *controlpb.ExecutionResult) (core.Object, error) { artifacts := outcome.ScheduledArtifactsDelivered.GetArtifacts() uploaded := make([]core.Object, 0, len(artifacts)) for _, artifact := range artifacts { + if artifact == nil || artifact.GetSize() > math.MaxInt64 { + return nil, invalidInput("invalid_result") + } uploaded = append(uploaded, storedObject(artifact)) } out["artifacts"] = uploaded diff --git a/internal/controlgrpc/scheduled_artifact_size_test.go b/internal/controlgrpc/scheduled_artifact_size_test.go new file mode 100644 index 00000000..60e081b3 --- /dev/null +++ b/internal/controlgrpc/scheduled_artifact_size_test.go @@ -0,0 +1,25 @@ +package controlgrpc + +import ( + "math" + "strings" + "testing" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +func TestScheduledArtifactResultDistinguishesEmptyFromOverflow(t *testing.T) { + object := &controlpb.StoredObject{Key: "artifact", Size: math.MaxUint64, Sha256: strings.Repeat("a", 64)} + message := &controlpb.ExecutionResult{ + Node: &controlpb.NodeIdentity{NodeId: "node", NodeIncarnationId: "incarnation"}, + Outcome: &controlpb.ExecutionResult_ScheduledArtifactsDelivered{ScheduledArtifactsDelivered: &controlpb.ScheduledArtifactsDelivered{Artifacts: []*controlpb.StoredObject{object}}}, + } + if _, err := resultObject(message); err == nil { + t.Fatal("an overflowing wire size must not become a legitimate zero-byte object") + } + object.Size = 0 + got, err := resultObject(message) + if err != nil || rows(got["artifacts"])[0].N("size") != 0 { + t.Fatalf("a real zero-byte size must survive: %v, %v", got, err) + } +} From bd7af3b98ee0168f5b91a7f9105d88791ae658fa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:28:17 +0800 Subject: [PATCH 21/22] fix: fence artifact registration after lock acquisition --- .../scheduled_artifact_deadline_test.go | 98 +++++++++++++++++++ internal/core/scheduled_artifact_expiry.go | 4 +- internal/core/scheduled_artifact_settle.go | 7 +- internal/core/scheduled_artifact_verify.go | 2 +- 4 files changed, 105 insertions(+), 6 deletions(-) create mode 100644 integration/scheduled_artifact_deadline_test.go diff --git a/integration/scheduled_artifact_deadline_test.go b/integration/scheduled_artifact_deadline_test.go new file mode 100644 index 00000000..8eeb4f4d --- /dev/null +++ b/integration/scheduled_artifact_deadline_test.go @@ -0,0 +1,98 @@ +package integration + +import ( + "context" + "net/http" + "net/http/httptest" + "sync" + "testing" + "time" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +// HEAD finishes before the takeover waits on the real control-plane lock. Move the fixture's +// deadline between that transaction's start and lock acquisition, using PostgreSQL time only. +// No elapsed sleep or guessed request latency decides which side of the deadline was exercised. +func TestScheduledArtifactGRPCDeadlineWhileWaitingForLock(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + ctx, cancel := context.WithTimeout(asController("ctrl-a"), 30*time.Second) + defer cancel() + headStarted, resumeHEAD := make(chan struct{}), make(chan struct{}) + var resume sync.Once + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + close(headStarted) + select { + case <-resumeHEAD: + f.objects.head(w, r) + case <-ctx.Done(): + http.Error(w, "test canceled", http.StatusServiceUnavailable) + } + })) + defer server.Close() + defer resume.Do(func() { close(resumeHEAD) }) + config := *f.objects.config + config.Endpoint = server.URL + f.store.ObjectStore = &config + key := "scheduled-runs/" + scene.tenantID + "/" + scene.runID + "/3" + done := make(chan error, 1) + go func() { + _, err := f.executions.TakeOverNodeEvent(ctx, &controlpb.TakeOverNodeEventRequest{ + Epoch: 1, SubmissionId: "wait-across-deadline", OperationId: scene.runID, ExecutionId: execution, + Sequence: 1, Event: []byte("uploaded"), Result: scheduledUploadResult(scene, key), + }) + done <- err + }() + select { + case <-headStarted: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + blocker, err := f.store.Pool.BeginTx(ctx, nil) + must(t, err) + defer blocker.Rollback() + var blockerPID int + must(t, blocker.QueryRowContext(ctx, "SELECT pg_backend_pid() FROM pg_advisory_xact_lock(67420911)").Scan(&blockerPID)) + resume.Do(func() { close(resumeHEAD) }) + ticker := time.NewTicker(10 * time.Millisecond) + defer ticker.Stop() + var crossed bool + for !crossed { + // The deadline is changed only after the final transaction demonstrably waits for our + // lock. Its now() remains before this instant even after our commit releases it. + must(t, f.store.Pool.QueryRowContext(ctx, `WITH waiter AS ( + SELECT xact_start FROM pg_stat_activity WHERE datname=current_database() + AND $2::integer=ANY(pg_blocking_pids(pid)) + ), changed AS ( + UPDATE scheduled_run_artifact_deliveries SET deadline_at=clock_timestamp() + WHERE issue_run_id=$1 AND EXISTS (SELECT 1 FROM waiter) + RETURNING deadline_at + ) SELECT EXISTS (SELECT 1 FROM changed, waiter WHERE deadline_at > xact_start)`, scene.runID, blockerPID).Scan(&crossed)) + if !crossed { + select { + case err := <-done: + t.Fatalf("takeover returned before the lock wait: %v", err) + case <-ticker.C: + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + } + } + must(t, blocker.Commit()) + select { + case err := <-done: + must(t, err) + case <-ctx.Done(): + t.Fatal(ctx.Err()) + } + if f.scalar(`SELECT count(*) FROM node_event_receipts WHERE execution_id=$1`, execution) != 1 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1`, scene.runID) != 0 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifact_deliveries + WHERE issue_run_id=$1 AND state='failed' AND settled_reason='deadline_expired'`, scene.runID) != 1 { + t.Fatal("a takeover that acquires the lock after its deadline must ACK without registration and settle as expired") + } + if len(f.objects.probed()) != 1 { + t.Fatal("the object must have verified before the final transaction waited") + } +} diff --git a/internal/core/scheduled_artifact_expiry.go b/internal/core/scheduled_artifact_expiry.go index 70b9654a..b30ea217 100644 --- a/internal/core/scheduled_artifact_expiry.go +++ b/internal/core/scheduled_artifact_expiry.go @@ -21,7 +21,7 @@ const scheduledArtifactBatchSize = 100 func (s *Store) ExpireScheduledArtifactDeliveriesOnce(ctx context.Context) error { rows, err := s.Pool.QueryContext(ctx, ` SELECT issue_run_id FROM scheduled_run_artifact_deliveries - WHERE state='delivering' AND deadline_at <= now() + WHERE state='delivering' AND deadline_at <= clock_timestamp() ORDER BY deadline_at LIMIT $1`, scheduledArtifactBatchSize) if err != nil { @@ -43,7 +43,7 @@ func (s *Store) ExpireScheduledArtifactDeliveriesOnce(ctx context.Context) error if _, err := s.transact(ctx, func(t *transaction) Object { delivery := t.one(` SELECT * FROM scheduled_run_artifact_deliveries - WHERE issue_run_id=$1 AND state='delivering' AND deadline_at <= now()`, runID) + WHERE issue_run_id=$1 AND state='delivering' AND deadline_at <= clock_timestamp()`, runID) if delivery == nil { return Object{} } diff --git a/internal/core/scheduled_artifact_settle.go b/internal/core/scheduled_artifact_settle.go index 51e062dc..7d8a17e6 100644 --- a/internal/core/scheduled_artifact_settle.go +++ b/internal/core/scheduled_artifact_settle.go @@ -70,8 +70,9 @@ func (s *Store) settleScheduledArtifacts(t *transaction, runID, executionID stri return fmt.Errorf("scheduledArtifactsSettled: run %s carries unknown outcome %q", runID, settled.S("outcome")) } // 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) + // expiry pass has done so far. now() is the transaction start, before waiting for the control- + // plane lock, so only the actual clock after acquiring that lock can fence registration. + open := t.one("SELECT clock_timestamp() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", runID) if !open.B("open") { return s.closeArtifactDelivery(t, o, "failed", scheduledArtifactDeadlineExpired) } @@ -120,7 +121,7 @@ func (s *Store) settleScheduledArtifacts(t *transaction, runID, executionID stri func (s *Store) closeArtifactDelivery(t *transaction, o Object, state, reason string) error { t.exec(` UPDATE scheduled_run_artifact_deliveries - SET state=$2, settled_at=now(), settled_reason=NULLIF($3,'') + SET state=$2, settled_at=clock_timestamp(), settled_reason=NULLIF($3,'') WHERE issue_run_id=$1 AND state='delivering'`, o.S("id"), state, reason) if !deliveryOutcomeRecorded(t, o.S("id")) { return nil diff --git a/internal/core/scheduled_artifact_verify.go b/internal/core/scheduled_artifact_verify.go index cd339f78..089167d5 100644 --- a/internal/core/scheduled_artifact_verify.go +++ b/internal/core/scheduled_artifact_verify.go @@ -40,7 +40,7 @@ func (s *Store) prepareArtifactVerification(ctx context.Context, r *ControlReque } validateNodeResult(e, r.Body.O("result")) require(r.Body.N("sequence") == e.N("lastEventSequence")+1 && r.Body.S("event") != "", 409, "receipt_conflict") - delivery := t.one("SELECT *, now() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) + delivery := t.one("SELECT *, clock_timestamp() < deadline_at AS open FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1", e.S("operationId")) require(delivery != nil, 409, "dispatch_conflict") // Closed windows only take over the original event and its receipt. Return the frozen // input so settlement can fence it without contacting storage or registering objects. From 5ae392eeac05504c4e59002b1ad068663bc7be2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E9=BE=99=E5=AE=89?= <297888591+wanglongan587@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:30:19 +0800 Subject: [PATCH 22/22] fix: persist artifact rejections for failed batches --- .../scheduled_artifact_settlement_test.go | 107 ++++++++++++++++++ internal/core/README.en.md | 4 + internal/core/README.md | 2 + .../core/scheduled_artifact_expiry_db_test.go | 47 ++++++++ internal/core/scheduled_artifact_settle.go | 20 ++++ 5 files changed, 180 insertions(+) create mode 100644 integration/scheduled_artifact_settlement_test.go create mode 100644 internal/core/scheduled_artifact_expiry_db_test.go diff --git a/integration/scheduled_artifact_settlement_test.go b/integration/scheduled_artifact_settlement_test.go new file mode 100644 index 00000000..ad448f2e --- /dev/null +++ b/integration/scheduled_artifact_settlement_test.go @@ -0,0 +1,107 @@ +package integration + +import ( + "reflect" + "testing" + + "google.golang.org/protobuf/proto" + + "github.com/wanglongan587/cloud/internal/controlpb" +) + +func TestScheduledArtifactGRPCBatchFailureRejections(t *testing.T) { + for _, order := range []string{"failure", "receipt-before-scan", "scan-before-receipt", "failure-after-expiry"} { + t.Run(order, func(t *testing.T) { + f := setup(t) + scene, execution := scheduledArtifactScene(t, f) + result := scheduledUploadResult(scene, "scheduled-runs/"+scene.tenantID+"/"+scene.runID+"/3") + reason := "deadline_expired" + if order == "failure" || order == "failure-after-expiry" { + result.Outcome = &controlpb.ExecutionResult_ScheduledArtifactsFailed{ScheduledArtifactsFailed: &controlpb.ScheduledArtifactsFailed{ + Reason: controlpb.ScheduledArtifactsFailureReason_SCHEDULED_ARTIFACTS_FAILURE_REASON_UPLOAD_FAILED, + }} + } + if order == "failure" { + reason = "upload_failed" + } else { + _, err := f.store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET deadline_at=clock_timestamp()-interval '1 second' WHERE issue_run_id=$1`, scene.runID) + must(t, err) + } + if order == "scan-before-receipt" { + must(t, f.store.ExpireScheduledArtifactDeliveriesOnce(t.Context())) + assertScheduledArtifactRejections(t, f, scene.runID, [][2]string{{"notes.pdf", "unsupported_type"}, {"report.md", reason}}) + } + for _, submission := range []string{"batch-terminal", "batch-terminal", "batch-replay"} { + _, err := f.deliveryTerminal(scene, execution, submission, 1, result, "batch-terminal") + must(t, err) + if order == "failure" { + _, err := f.store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET deadline_at=clock_timestamp()-interval '1 second' WHERE issue_run_id=$1`, scene.runID) + must(t, err) + } + must(t, f.store.ExpireScheduledArtifactDeliveriesOnce(t.Context())) + assertScheduledArtifactRejections(t, f, scene.runID, [][2]string{{"notes.pdf", "unsupported_type"}, {"report.md", reason}}) + } + if f.scalar(`SELECT count(*) FROM node_event_receipts WHERE execution_id=$1`, execution) != 1 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1`, scene.runID) != 0 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifact_deliveries + WHERE issue_run_id=$1 AND state='failed' AND settled_reason=$2`, scene.runID, reason) != 1 { + t.Fatal("batch failure must preserve one ACK, zero objects and its first settlement") + } + if len(f.objects.probed()) != 0 { + t.Fatal("failed or expired batches need no object verification") + } + }) + } +} + +func TestScheduledArtifactGRPCPartialSettlementPreservesItemFacts(t *testing.T) { + f := setup(t) + manifest := &controlpb.ScheduledArtifactManifest{ + Rejections: []*controlpb.ScheduledArtifactRejection{{DisplayName: "notes.pdf", Reason: controlpb.ScheduledArtifactRejectionReason_SCHEDULED_ARTIFACT_REJECTION_REASON_UNSUPPORTED_TYPE}}, + } + for i, name := range []string{"report.md", "wrong.md", "missing.md"} { + manifest.Items = append(manifest.Items, &controlpb.ScheduledArtifactItem{ + Index: uint32(i + 3), DisplayName: name, Kind: controlpb.ScheduledArtifactKind_SCHEDULED_ARTIFACT_KIND_MARKDOWN, + SizeBytes: revisionHistorySize, Sha256: revisionHistorySHA, + }) + } + scene, execution := scheduledArtifactSceneForManifest(t, f, manifest) + prefix := "scheduled-runs/" + scene.tenantID + "/" + scene.runID + "/" + result := scheduledUploadResult(scene, prefix+"3") + wrong := proto.Clone(result.GetScheduledArtifactsDelivered().Artifacts[0]).(*controlpb.StoredObject) + wrong.Key = prefix + "4" + result.GetScheduledArtifactsDelivered().Artifacts = append(result.GetScheduledArtifactsDelivered().Artifacts, wrong) + f.objects.breakObject(wrong.Key) + for _, submission := range []string{"partial", "partial", "partial-replay"} { + _, err := f.deliveryTerminal(scene, execution, submission, 1, result, "partial") + must(t, err) + assertScheduledArtifactRejections(t, f, scene.runID, [][2]string{ + {"missing.md", "upload_failed"}, {"notes.pdf", "unsupported_type"}, {"wrong.md", "verification_failed"}, + }) + } + if f.scalar(`SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1 AND artifact_index=3`, scene.runID) != 1 || + f.scalar(`SELECT count(*) FROM scheduled_run_artifact_deliveries WHERE issue_run_id=$1 AND state='partial'`, scene.runID) != 1 || + len(f.objects.probed()) != 2 { + t.Fatal("settlement must preserve its verified success and original item reasons without probing on replay") + } +} + +func assertScheduledArtifactRejections(t *testing.T, f *fixture, runID string, want [][2]string) { + t.Helper() + rows, err := f.store.Pool.Query(`SELECT display_name, reason FROM scheduled_run_artifact_rejections + WHERE issue_run_id=$1 ORDER BY display_name`, runID) + must(t, err) + defer rows.Close() + var got [][2]string + for rows.Next() { + var item [2]string + must(t, rows.Scan(&item[0], &item[1])) + got = append(got, item) + } + must(t, rows.Err()) + if !reflect.DeepEqual(got, want) { + t.Fatalf("rejections = %v; want collection refusals and unregistered upload failures %v", got, want) + } +} diff --git a/internal/core/README.en.md b/internal/core/README.en.md index 5e0bed9e..04e1b84c 100644 --- a/internal/core/README.en.md +++ b/internal/core/README.en.md @@ -31,6 +31,10 @@ the [verdict/artifact ADR](https://github.com/ora-space/specs/blob/main/decision Revision and artifact deliveries dispatch independently, with at most one unfinished execution of each kind; both must settle before the run Workspace releases. Upload grants never outlive the run's fixed deadline. Expired receipts still acknowledge the original execution without HEAD or registration. +Final registration checks the actual database clock after acquiring the lock, rather than the +transaction start before the lock wait. Batch failure or expiry atomically records rejections for +unregistered files in the frozen manifest, preserving existing collection/verification rejections +and registered successes without adding facts on replay. Object verification and deletion happen outside transactions. Cleanup derives deterministic keys and deletes objects absent from the index. Settled runs become eligible for another sweep after one minute, including successful deliveries whose index retention has pruned. `artifacts_gc_at` records the last diff --git a/internal/core/README.md b/internal/core/README.md index 77a50f4c..8cf120b0 100644 --- a/internal/core/README.md +++ b/internal/core/README.md @@ -30,6 +30,8 @@ [verdict 与产物 ADR](https://github.com/ora-space/specs/blob/main/decisions/cloud/scheduled-task/20261022-run-verdict-and-artifacts.md)。 Revision 与产物可以分别派发,每类最多一个未完成执行,两者均收敛后才释放运行 Workspace。 上传授权不超过运行的固定截止;到期回执仍按原执行身份确认,但不请求 HEAD、不登记对象。 +最终登记在持锁后以数据库实际当前时间检查截止,不能沿用等待锁之前的事务开始时间。 +整批失败或到期时,同事务为冻结清单中未登记的文件补齐拒绝事实;既有采集/核验拒绝与成功登记保持不变,重放不增记。 对象核验与删除都在事务外执行。清理按确定性键删除索引未覆盖的对象,已结算运行每分钟可再次扫描, 包含已被保留策略淘汰索引的成功交付;`artifacts_gc_at` 表示上次扫描时间,不表示永久完成。 diff --git a/internal/core/scheduled_artifact_expiry_db_test.go b/internal/core/scheduled_artifact_expiry_db_test.go new file mode 100644 index 00000000..d998aa98 --- /dev/null +++ b/internal/core/scheduled_artifact_expiry_db_test.go @@ -0,0 +1,47 @@ +package core + +import "testing" + +// The deadline applies even when no Node has claimed the upload. Rejections must come from the +// frozen work/session binding, not depend on a delivery execution or any terminal receipt existing. +func TestScheduledArtifactExpiryBeforeDispatchRecordsRejections(t *testing.T) { + store := dispatcherDB(t) + store.ObjectStore, _ = cleanupObjectStore(t) + scene := seedTakeoverScene(t, store) + stageScheduledOccurrence(t, store, scene.run) + _, err := store.Control(t.Context(), &ControlRequest{ + Action: "clone_takeover", Service: scene.claims, Body: Object{ + "epoch": 1, "operationId": scene.run, "executionId": scene.execution, "sequence": 1, "event": "declared", + "result": Object{"node": Object{"nodeId": scene.node}, "outcome": "agent_session_ended", "reason": "idle_timeout", "artifactManifest": scheduledManifest()}, + }, + }) + if err != nil { + t.Fatal(err) + } + _, err = store.Pool.Exec(`UPDATE scheduled_run_artifact_deliveries + SET deadline_at=clock_timestamp()-interval '1 second' WHERE issue_run_id=$1`, scene.run) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 2; i++ { + if err := store.ExpireScheduledArtifactDeliveriesOnce(t.Context()); err != nil { + t.Fatal(err) + } + var refusals, work, executions, artifacts int + if err := store.Pool.QueryRow(`SELECT + (SELECT count(*) FROM scheduled_run_artifact_rejections WHERE issue_run_id=$1), + (SELECT count(*) FROM execution_work WHERE run_id=$1 AND kind='deliver_scheduled_artifacts' AND execution_id IS NULL), + (SELECT count(*) FROM node_executions WHERE operation_id=$1 AND kind='deliver_scheduled_artifacts'), + (SELECT count(*) FROM scheduled_run_artifacts WHERE issue_run_id=$1)`, scene.run).Scan(&refusals, &work, &executions, &artifacts); err != nil { + t.Fatal(err) + } + if refusals != 2 || work != 1 || executions != 0 || artifacts != 0 { + t.Fatalf("rejections/work/executions/artifacts = %d/%d/%d/%d; want 2/1/0/0", refusals, work, executions, artifacts) + } + var reason string + if err := store.Pool.QueryRow(`SELECT reason FROM scheduled_run_artifact_rejections + WHERE issue_run_id=$1 AND display_name='reports/review.md'`, scene.run).Scan(&reason); err != nil || reason != scheduledArtifactDeadlineExpired { + t.Fatalf("undeployed upload rejection = %q, %v; want deadline_expired", reason, err) + } + } +} diff --git a/internal/core/scheduled_artifact_settle.go b/internal/core/scheduled_artifact_settle.go index 7d8a17e6..05797221 100644 --- a/internal/core/scheduled_artifact_settle.go +++ b/internal/core/scheduled_artifact_settle.go @@ -119,6 +119,26 @@ func (s *Store) settleScheduledArtifacts(t *transaction, runID, executionID stri // closeArtifactDelivery settles the delivery row and releases the run when its Revision delivery has // already settled as well. func (s *Store) closeArtifactDelivery(t *transaction, o Object, state, reason string) error { + if state == "failed" || state == "partial" { + // A whole-batch failure (including expiry before dispatch) has no per-object result. + // The work item's frozen session binding still names the complete accepted manifest. + // Preserve collection/verification refusals and registered successes; fill only missing + // failure facts in the same transaction as the first settlement and release. + t.exec(` + INSERT INTO scheduled_run_artifact_rejections + (tenant_id, issue_run_id, task_id, occurrence_id, scheduled_at, display_name, reason) + SELECT d.tenant_id, d.issue_run_id, d.task_id, d.occurrence_id, d.scheduled_at, + item->>'displayName', COALESCE(NULLIF($2,''),'upload_failed') + FROM scheduled_run_artifact_deliveries d + JOIN execution_work w ON w.run_id=d.issue_run_id AND w.kind='deliver_scheduled_artifacts' + JOIN node_executions e ON e.execution_id=w.input->>'sessionExecutionId' + AND e.operation_id=d.issue_run_id AND e.kind='agent_session' + CROSS JOIN LATERAL jsonb_array_elements(e.result->'artifactManifest'->'items') AS item + WHERE d.issue_run_id=$1 AND d.state='delivering' + AND NOT EXISTS (SELECT 1 FROM scheduled_run_artifacts a + WHERE a.issue_run_id=d.issue_run_id AND a.artifact_index=(item->>'index')::int) + ON CONFLICT (issue_run_id, display_name) DO NOTHING`, o.S("id"), reason) + } t.exec(` UPDATE scheduled_run_artifact_deliveries SET state=$2, settled_at=clock_timestamp(), settled_reason=NULLIF($3,'')