Skip to content

fix(workflows): recover terminal artifact commits - #386

Open
testikun wants to merge 3 commits into
openpi-dev:mainfrom
testikun:codex/issue-110-artifact-commit
Open

fix(workflows): recover terminal artifact commits#386
testikun wants to merge 3 commits into
openpi-dev:mainfrom
testikun:codex/issue-110-artifact-commit

Conversation

@testikun

@testikun testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Problem

Workflow terminal persistence writes result.json, transcripts.json, optional journal.json, and the final referencing workflow.json as 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 main already publishes a reference-free terminal manifest first, which prevents a known terminal run from remaining running, 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.json as completion evidence.

Approach

  • Write one bounded, versioned .workflow-commit.json receipt before the terminal side-artifact sequence. It records the exact final manifest plus the fixed artifact names, byte counts, and SHA-256 digests.
  • Keep workflow.json canonical: publish the existing reference-free terminal projection first, prepare every side artifact, atomically commit the exact recorded manifest last, then remove the receipt.
  • Recover at the persisted Workflow read boundary and before delivery-state mutation. Promotion requires an exact run-directory identity, known terminal status, fixed path whitelist, matching manifest references, regular non-symlink files, byte budgets, lengths, and digests.
  • Leave incomplete or invalid receipts fail-closed; a normal retry replaces the single fixed receipt. Legacy runs without receipts retain their old behavior, and orphan result.json alone never proves completion.
  • Document the ownership, sequence, recovery outcomes, compatibility, and fsync limitation in 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.
  • Focused artifact and dashboard tests — 42 passed, 0 failed.
  • bun run test — Node tests: 1247 passed, 0 failed, 1 skipped; Vitest: 30 passed.
  • Regression cases include complete crash recovery, missing artifacts, same-size digest substitution, already-committed idempotence, delivery update after recovery, dashboard/startup hydration, normal cleanup, and dependent-write failure.

Impact

  • User-visible behavior: no UI changes.
  • Model-visible context/tools: none.
  • Runtime/lifecycle: terminal Workflow artifact persistence gains a recoverable commit receipt at the existing filesystem boundary.
  • Persisted config/data: no configuration changes. New terminal writes may briefly contain .workflow-commit.json; successful writes remove it, and legacy artifacts remain readable.
  • Compatibility/risk: filenames and canonical manifest references remain unchanged. Recovery fails closed on unknown versions, identities, paths, states, sizes, symlinks, or hashes. The guarantee covers process-crash recovery at the existing atomic-rename boundary, not power-loss durability beyond writeFileAtomic.

@github-actions github-actions Bot added documentation Improvements or additions to documentation area:workflows Workflow engine, capability, skills, or tests labels Sep 4, 2026
@testikun

testikun commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

CI status note: Node 22/24 completed this PR's repository checks and failed only at the final fresh-install Smoke-test packed standalone Web CLI step, where the child exits before the test can read its URL. The same failure now occurs on unrelated PRs #385, #386, and #387.

I reproduced the packed install independently: npm now admits Pi 0.85.0 through OpenPI's published peerDependencies: "*"; the CLI then exits with Cannot find module '@earendil-works/pi-server'. The checkout/lock-based bun run check and full test suite remain green. I recorded the shared packaging regression and suggested peer-range follow-up in #328: #328 (comment)

No dependency workaround is mixed into this feature PR.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

需要改:「已提交」不要用整份 workflow.json 字节当身份

文档写 crash 在 unlink 之后是 harmless,因为 recovery 认得 already-committed manifest。实现用 整份 workflow.json 字节相等 判断 already-committed。

这不是无害窗口:persistWorkflowDeliveryState#382 的 delivery owner、#379resourceRefs 都会改同一份文件。下一轮 recoverPendingWorkflowCommit 会把收据里更旧的 manifest 写回去。

请用 run id + 终态 + artifact 引用(名字/digest)判断 already-committed,不要比整文件字节。和 #379/#382 叠在 artifacts.ts 上之前,先把这个身份修好。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:workflows Workflow engine, capability, skills, or tests documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

workflows: terminal artifact set (result.json + workflow.json) is not committed atomically

2 participants