Skip to content

fix(ingest): serialize qoder checkpoint writers - #1

Closed
sun-970 wants to merge 1 commit into
mainfrom
codex/fix-139-cross-process-checkpoint
Closed

sun-970 wants to merge 1 commit into
mainfrom
codex/fix-139-cross-process-checkpoint

Conversation

@sun-970

@sun-970 sun-970 commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Tracking

Refs bytefolk#139

Supersedes bytefolk#140 (rebased onto latest main to resolve CHANGELOG.md conflict).

Consumed requirement revision: #139 R1. This PR must not auto-close the issue; merge and product acceptance remain separate decisions.

Scope

Repairs only the local Qoder checkpoint persistence path on current main. It adds a persistent per-checkpoint OS advisory-lock sidecar, merges competing writes under that lock without allowing LastLine or associated diagnostic state to regress, and replaces the shared *.tmp name with a unique private staging file plus sync and atomic rename.

Traceability

Requirement Acceptance criterion Implementation / evidence
REQ-001 AC-001 Lock-held reread preserves the complete higher checkpoint; deterministic independent processes submit 12 then stale 4 and final state remains the full 12 checkpoint.
REQ-002 AC-001, AC-002 Retained sidecar inode uses OS-backed locking (flock / AIX fcntl / Windows LockFileEx); staging uses same-directory CreateTemp; an owner process exits through os.Exit while holding the lock and a waiting writer proceeds after kernel release.
REQ-003 AC-003 JSON schema, caller interface, and normal load behavior are unchanged; only private .lock and transient private temp files are added beside local state.

Validation

  • Focused child-process tests and race tests pass.
  • go test ./... and go test -race -p 1 ./... pass.
  • gofmt, go vet ./..., and build pass.

Risk / rollback

The lock is advisory and applies only to participating mem checkpoint writers on a filesystem with normal OS locking semantics. Revert this PR to restore the prior writer. No remote API, transcript format, release artifact, dependency, or repository-setting change is included.

@sun-970 sun-970 closed this Sep 1, 2026
sun-970 pushed a commit that referenced this pull request Sep 7, 2026
spawnSync returns null status when npm is killed by signal or not found
on PATH. Three return sites in runWithRetry passed null through to
process.exit, which Node treats as 0 — failing the audit gate silently.

Add `?? 1` at the two unguarded sites (:67, :85) and remove the
redundant 4th spawnSync after loop exhaustion (review finding #1),
reusing the last loop result with the same `?? 1` guard. All three
sites now aligned.
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