fix(workflows): recover terminal artifact commits - #386
Conversation
|
CI status note: Node 22/24 completed this PR's repository checks and failed only at the final fresh-install I reproduced the packed install independently: npm now admits Pi 0.85.0 through OpenPI's published No dependency workaround is mixed into this feature PR. |
tt-a1i
left a comment
There was a problem hiding this comment.
需要改:「已提交」不要用整份 workflow.json 字节当身份
文档写 crash 在 unlink 之后是 harmless,因为 recovery 认得 already-committed manifest。实现用 整份 workflow.json 字节相等 判断 already-committed。
这不是无害窗口:persistWorkflowDeliveryState、#382 的 delivery owner、#379 的 resourceRefs 都会改同一份文件。下一轮 recoverPendingWorkflowCommit 会把收据里更旧的 manifest 写回去。
请用 run id + 终态 + artifact 引用(名字/digest)判断 already-committed,不要比整文件字节。和 #379/#382 叠在 artifacts.ts 上之前,先把这个身份修好。
tt-a1i
left a comment
There was a problem hiding this comment.
Exact-head follow-up on fd2842f: the prior already-committed identity blocker is addressed. The implementation compares runId/status/artifact references after checking artifact size/digests, rather than requiring whole-file manifest equality.
Verification in an isolated exact-head worktree: artifacts tests 18/18, dashboard tests 25/25, plus a direct probe confirming that delivery/resourceRefs survive a leftover receipt, a pre-manifest crash is recovered, and an incomplete receipt does not publish references. All commands exited 0. No new P0/P1 was confirmed in this bounded review.
Please update the architecture text below to match the corrected identity rule. This PR changes recovery writes reached through persisted workflow reads, so this comment is not final integration/runtime acceptance: current-main integration and its full required checks remain outstanding. No branch update or merge was performed in this review.
f787df3 to
68774e1
Compare
|
@tt-a1i 已修复:already-committed 改为校验 runId、terminal state、artifact refs/digest,不再使用整份 workflow.json 字节相等。 |
|
@tt-a1i 已同步 architecture record 与 source boundary,说明已改为 runId + terminal state + artifact refs/digest;当前 head CI 已通过。 |
Problem
Workflow terminal persistence writes
result.json,transcripts.json, optionaljournal.json, and the final referencingworkflow.jsonas separate atomic replacements. A process crash after the side artifacts but before the final manifest can leave the earlier manifest without references and the terminal evidence orphaned.Current
mainalready publishes a reference-free terminal manifest first, which prevents a known terminal run from remainingrunning, but it cannot prove that an otherwise orphaned artifact belongs to a complete terminal commit.Closes #110.
Value
Restart recovery can now distinguish a fully prepared terminal artifact set from partial, stale, substituted, or malformed files. Exact terminal results remain discoverable after the crash window without treating an arbitrary
result.jsonas completion evidence.Approach
.workflow-commit.jsonreceipt before the terminal side-artifact sequence. It records the exact final manifest plus the fixed artifact names, byte counts, and SHA-256 digests.workflow.jsoncanonical: publish the existing reference-free terminal projection first, prepare every side artifact, atomically commit the exact recorded manifest last, then remove the receipt.result.jsonalone never proves completion.docs/architecture/WORKFLOW_ARTIFACT_COMMIT.md, linked to workflows: terminal artifact set (result.json + workflow.json) is not committed atomically #110.Validation
bun run check— passed.bun run test— Node tests: 1247 passed, 0 failed, 1 skipped; Vitest: 30 passed.Impact
.workflow-commit.json; successful writes remove it, and legacy artifacts remain readable.writeFileAtomic.