Skip to content

fix: harden Windows state persistence retries - #6

Merged
ByBrawe merged 2 commits into
mainfrom
fix/state-write-retry-hardening
Aug 8, 2026
Merged

ByBrawe merged 2 commits into
mainfrom
fix/state-write-retry-hardening

Conversation

@ByBrawe

@ByBrawe ByBrawe commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

Follow-up hardening for #5.

  • Retry transient Windows rename failures before falling back to non-atomic copy/overwrite.
  • Retry transient/partial JSON reads before treating session state as corrupt and returning an empty job list.
  • Add deterministic regressions for simulated EPERM rename locks and partial JSON reads.
  • Remove the duplicated Example progress.md heading introduced in fix: Windows-safe atomic session state writes (EPERM) #5.

Verification

  • node --check src/index.js
  • node --check scripts/comprehensive-test.mjs
  • Targeted Windows state persistence regression tests passed locally.

This keeps the fallback from unnecessarily widening the non-atomic write window and prevents a transient partial read from looking like jobs: [].

Copilot AI lite review requested due to automatic review settings August 8, 2026 14:37
@ByBrawe
ByBrawe merged commit 17098c8 into main Aug 8, 2026
1 check passed

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens Windows session state persistence by adding retries around transient filesystem and partial-read failures, reducing the chance that temporary locks or partial reads cause dropped jobs or unnecessary non-atomic writes.

Changes:

  • Retry readState on transient/partial JSON reads before treating state as corrupt.
  • Retry rename on transient Windows lock errors before falling back to non-atomic copyFile/overwrite in writeFileAtomically.
  • Add deterministic comprehensive regressions for rename-lock retries and partial JSON read retries; remove duplicated README heading.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/index.js Adds retry loops for transient state reads and for Windows rename failures before falling back to copy/overwrite.
scripts/comprehensive-test.mjs Adds regressions simulating EPERM destination locks and partial JSON reads; exposes stateFile via the harness.
README.md Removes a duplicated “Example progress.md” heading section.
Suppressed comments (1)

scripts/comprehensive-test.mjs:552

  • This test can become flaky if loop-status (or related internals) reads the state file more than once; that would make partialReads exceed 3 even though the retry behavior is correct. Use a >= assertion so the test only enforces that retries happened before treating state as corrupt.
    await h.command("loop-status")
    assert.equal(partialReads, 3, "partial JSON should be retried before treating state as corrupt")
    assert.equal(h.records.toasts.at(-1)?.message, "1 loop job(s).")

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +529 to +533
const state = await h.readState()
assert.equal(state.jobs[0]?.name, "retry-safe")
assert.equal(renameAttempts, 3, "transient EPERM should retry rename")
assert.equal(fallbackCopies, 0, "successful rename retry should avoid non-atomic copy fallback")
} finally {
@github-actions
github-actions Bot deleted the fix/state-write-retry-hardening branch September 21, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants