Skip to content

refactor(jsonutil): centralize streaming atomic replacement - #2053

Open
MuskanPaliwal wants to merge 4 commits into
entireio:mainfrom
MuskanPaliwal:refactor-streaming-atomic-write
Open

refactor(jsonutil): centralize streaming atomic replacement#2053
MuskanPaliwal wants to merge 4 commits into
entireio:mainfrom
MuskanPaliwal:refactor-streaming-atomic-write

Conversation

@MuskanPaliwal

Copy link
Copy Markdown
Contributor

Depends on #1877. Until that PR merges, review the stacked range 3a39273b..7c62491a5.

This change moves OpenCode’s staged transcript publication into jsonutil.WriteFileAtomicStream. The previous implementation kept temporary-file creation, validation, rename retries, and recovery inside OpenCode even though most of that lifecycle is general atomic-write behavior. The shared implementation keeps publication ordered as produce, sync, close, validate, chmod, rename, and best-effort directory sync.

Core Changes

WriteFileAtomicStream now owns the staging file from creation through publication. Producer and validator errors are returned unchanged. Any failure before publication leaves the existing destination untouched and makes a best-effort attempt to remove the staging file. If a completed and validated file cannot be renamed into place, the helper returns a PublishError and transfers ownership of the retained staging path to the caller so the output can be recovered.

Windows replacement contention is handled inside jsonutil rather than OpenCode. Only access-denied and sharing-violation failures are retried, with five total attempts and cancellation-aware waits. The follow-up review found a cancellation window between retries: cancellation could occur after a wait, but the next rename could still replace the destination. The retry loop now checks the context before every rename attempt, retaining the validated staging file if cancellation wins.

OpenCode continues to translate publication failures into its existing actionable “export saved at …” message. Its package-specific staging, rename, and contention helpers have been removed because those responsibilities now belong to the shared atomic-write implementation.

API Notes

This adds the public jsonutil.WriteFileAtomicStream helper and the public jsonutil.PublishError ownership contract. A caller receiving PublishError owns the validated file at StagedPath and must either recover or remove it.

The existing jsonutil.WriteFileAtomic function uses the same private engine but keeps its previous behavior. Rename failures do not expose PublishError, and its staging files are still cleaned up instead of being retained.

Compatibility / Release Notes

OpenCode command invocation, timeout and error classification, mock export mode, transcript validation, attach behavior, transcript protocol, and declared capabilities are unchanged. Failed, partial, empty, invalid, and cancelled exports cannot replace an existing valid transcript.

No unrelated temporary-file or rename-based writers are migrated in this change because they may have different locking, permission, confinement, executable, or cross-filesystem requirements.

This branch is stacked on #1877 and should be rebased onto main after that PR merges.

Testing

  • mise run check
  • Formatting and lint passed with zero issues
  • Race-enabled unit and integration suites passed
  • Vogon deterministic canary passed: 56/56
  • Roger-Roger deterministic canary passed: 4/4
  • Focused race tests passed for jsonutil and OpenCode
  • Windows cross-compilation and vet passed for the affected packages
  • Real-agent E2E tests were not run because they make paid external calls and were not requested

@MuskanPaliwal
MuskanPaliwal marked this pull request as ready for review August 19, 2026 10:49
@MuskanPaliwal
MuskanPaliwal requested a review from a team as a code owner August 19, 2026 10:49
@MuskanPaliwal MuskanPaliwal changed the title refactor(jsonutil): centralize streaming atomic replacement refactor(jsonutil): centralize streaming atomic replacement Aug 19, 2026
@MuskanPaliwal
MuskanPaliwal force-pushed the refactor-streaming-atomic-write branch from 7c62491 to ffdb142 Compare August 24, 2026 12:04
@MuskanPaliwal

Copy link
Copy Markdown
Contributor Author

Hi @Soph , now that PR number #1877 is merged, I would highly appreciate your review on this PR. This was stacked on #1877 .

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant