Skip to content

fix(editor): prevent concurrent str_replace_editor corruption - #52

Merged
osovv merged 1 commit into
mainfrom
fix/str-replace-editor-concurrent-writes
Nov 10, 2026
Merged

osovv merged 1 commit into
mainfrom
fix/str-replace-editor-concurrent-writes

Conversation

@osovv

@osovv osovv commented Nov 10, 2026

Copy link
Copy Markdown
Owner

Upstream editor reference · Upstream filesystem coordination

Why the change

Concurrent str_replace_editor calls could read the same original file, independently rewrite it, and all report success while overwriting each other's changes. Concurrent truncating writes could also leave NUL bytes or stale tails.

Before the fix, a local diagnostic through the native handler with real temporary files produced three success responses in all 50 parallel trials, but never the complete expected result; one trial contained NUL bytes. The sequential control was byte-exact in all 20 trials. This did not require a model or an OpenCode host process.

The DeepSeek Harness editor delegates to a filesystem with version-guarded writes, canonical-target coordination, and staged atomic publication. Our adapter used an uncoordinated writeFile; the fix restores the relevant safety properties without importing the upstream filesystem or expanding the tool API.

Special things to note

  • The existing four commands and exact-match rules stay unchanged. Independent calls are not a batch transaction: a failed call does not roll back earlier successful calls.
  • Coordination applies to native str_replace_editor calls within one process, not host-owned editors, shell commands, other processes, or crash/power-loss durability.
  • Text remains UTF-8; this does not introduce arbitrary encoding support or repair already damaged files by stripping NUL bytes.
  • Ordinary permission bits, POSIX uid/gid, and existing symlink targets are preserved. Multi-hardlink targets and files with setuid/setgid/sticky mode bits are refused instead of silently changing their semantics; ownership refusal has an actionable diagnostic. Atomic publication needs a writable parent directory. Extended ACLs/xattrs and Windows security metadata are not preserved; removing a restrictive ACL can widen access, so files using those metadata need a metadata-preserving editor.
  • Local verification passed: bun run check (2338 pass, 7 live-host skips), bun run pack:check, grace lint --path ., and grace status --path . (zero integrity errors/warnings). The original three-call real-file diagnostic was exact in 50/50 trials, without NUL bytes. Full OpenCode-host execution and Windows behavior were not exercised.
  • Independent code review passed; three initial minor findings were corrected and a focused final review passed without findings. The final corrections have fresh targeted coverage (207 pass, 1 live-host skip) and full repository/package checks.

Change outline

Before: independent full-file writes

  call A ─ read original ─ replace A ─ truncate/write ─ success
  call B ─ read original ─ replace B ─ truncate/write ─ success
  call C ─ read original ─ replace C ─ truncate/write ─ success
                                      │
                                      └─ lost changes / mixed bytes

After: one complete operation at a time per canonical target

  native resource permission
    → canonical-target queue (different files remain independent)
    → freshness check + read + exact-match validation
    → private staging file + byte verification
    → ownership/mode preservation + cancellation/change recheck
    → atomic publication + snapshot update
    → success

Regression coverage pins concurrent disjoint replacements, growing/shrinking and empty payloads, exact UTF-8 bytes without NUL or duplicate tails, alias and session behavior, create conflicts, cancellation/failure recovery, and unrelated-file progress. A smoke through the built public plugin also applied three concurrent replacements with exact Unicode/BOM/CRLF bytes and no NUL or staging residue.

Native reads and editor views use the same session-isolated canonical freshness records; a later handler instance or another path alias cannot bypass an existing observation. Handler disposal is idempotent and releases the shared cache after its last owner.

Fault-injection tests also cover partially written staging files, corrupted staged bytes, cancellation during staging, external changes during staging, ownership refusal, and a competing creator winning publication. These cases were exercised independently of the scheduler-dependent stress diagnostic.

@osovv
osovv merged commit 91b7a2c into main Nov 10, 2026
1 check passed
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.

1 participant