fix(tools): atomic writes and ACP/compaction file-tool continuity

docs/tools/read.md

@@ -81,7 +81,9 @@ URL selectors are parsed separately in `packages/coding-agent/src/tools/fetch.ts
    - `#readSqlite()` dispatches on `parseSqliteSelector()`.
 6. Otherwise it treats the input as a local filesystem path.
    - `resolveReadPath()` expands `~`, resolves relative to session cwd, treats bare `/` as session cwd, and retries macOS screenshot/NFD/curly-quote variants.
-   - If the path does not exist, `findUniqueSuffixMatch()` does a workspace glob-based unique suffix lookup (skipped for remote mounts).
+   - If the path does not exist on disk and an ACP `readTextFile` bridge is present, the editor buffer is tried before suffix lookup so a just-written client buffer is not reported as missing.
+   - Bridge failures fail closed by default: only an explicit `transport_unavailable` or `bridge_unavailable` code authorizes falling back to the agent host's disk. Structured denials (`permission_denied`, `-32001`) and raw OS errno values (`EPERM`, `EACCES`, …) do **not** fall back, because an errno at this boundary is ambiguous and reading the path locally would bypass a remote client's access decision.
+   - If the path still does not exist, `findUniqueSuffixMatch()` does a workspace glob-based unique suffix lookup (skipped for remote mounts).
 7. Directories go through `#readDirectory()`.
 8. Non-directories branch by content type:
    - image metadata / inline image

docs/tools/write.md

@@ -11,6 +11,7 @@
   - `packages/coding-agent/src/lsp/index.ts` — format-on-write and diagnostics writethrough.
   - `packages/coding-agent/src/tools/auto-generated-guard.ts` — block overwriting generated files.
   - `packages/coding-agent/src/tools/fs-cache-invalidation.ts` — invalidate shared FS scan caches after writes.
+  - `packages/coding-agent/src/tools/atomic-file-write.ts` — sibling temp + rename so a failed write never leaves a 0-byte destination.
   - `packages/coding-agent/src/tools/plan-mode-guard.ts` — resolve paths and enforce plan-mode write policy.
 
 ## Inputs
@@ -52,9 +53,9 @@ Single-shot result.
 1. `WriteTool.execute()` in `packages/coding-agent/src/tools/write.ts` strips `LINE+ID|` hashline prefixes from `content` when the session is in hashline display mode.
 2. It calls `#resolveArchiveWritePath()` first. That uses `parseArchivePathCandidates()` from `packages/coding-agent/src/tools/archive-reader.ts`, checks candidate archive files on disk, and falls back to the longest matching archive suffix even when the archive file does not exist yet.
 3. Archive writes call `enforcePlanModeWrite(..., { op: exists ? "update" : "create" })`, then `#writeArchiveEntry()`.
-   - The parent directory of the archive file is created with `fs.mkdir(..., { recursive: true })`.
-   - `.zip` archives are read with `fflate.unzipSync()`, the target entry is replaced in an in-memory map, and the archive is rewritten with `fflate.zipSync()` + `Bun.write()`.
-   - `.tar`, `.tar.gz`, and `.tgz` archives are read with `Bun.Archive`, existing entries are copied into an object map, the target entry is replaced, and `Bun.Archive.write()` rewrites the archive.
+   - `.zip` archives are read with `fflate.unzipSync()`, the target entry is replaced in an in-memory map, and the complete archive is reconstructed with `fflate.zipSync()` and published through the same guarded sibling-temp atomic writer as plain files.
+   - `.tar`, `.tar.gz`, and `.tgz` archives are read with `Bun.Archive`, existing entries are copied into an object map, the target entry is replaced, and `Bun.Archive.bytes()` reconstructs the archive before atomic publication.
+   - The reconstructed ZIP/TAR bytes are published through `writeFileAtomically()`, which creates parents only after trust-boundary validation. A failed reconstruction or publication leaves an existing archive byte-identical and removes the owned staging file.
    - `invalidateFsScanAfterWrite()` runs on the archive file path.
 4. If the path is not treated as an archive, `execute()` calls `#resolveSqliteWritePath()`. That uses `parseSqlitePathCandidates()` and `isSqliteFile()` from `packages/coding-agent/src/tools/sqlite-reader.ts`. Existing non-SQLite files suppress the SQLite path interpretation.
 5. SQLite writes call `enforcePlanModeWrite(..., { op: "update" })`, then `#writeSqliteRow()`.
@@ -66,15 +67,17 @@ Single-shot result.
 6. Otherwise the tool treats `path` as a plain filesystem file.
    - `enforcePlanModeWrite(..., { op: "create" })` runs before path resolution.
    - Existing files are checked by `assertEditableFile()` to block overwriting detected generated files.
-   - The session’s writethrough callback writes content. With LSP enabled and `lsp.formatOnWrite` / `lsp.diagnosticsOnWrite` settings on, `createLspWritethrough()` may format content, sync it through LSP servers, save it, and collect diagnostics. Otherwise `writethroughNoop()` writes directly with `Bun.write()` or `file.write()`.
-   - `invalidateFsScanAfterWrite()` runs on the file path.
+   - The session’s writethrough callback writes content. With LSP enabled and `lsp.formatOnWrite` / `lsp.diagnosticsOnWrite` settings on, `createLspWritethrough()` may format content, sync it through LSP servers, save it, and collect diagnostics. Otherwise `writethroughNoop()` writes through `writeFileAtomically()` (sibling temp, then rename). Permission errors (`EACCES`/`EPERM`/`EROFS`) become a `ToolError` that says the original file was left unchanged.
+   - `invalidateFsScanAfterWrite()` and `fileReadCache.invalidate()` run on the file path.
 7. The tool returns a text result and optional diagnostics metadata.
 
 ## Modes / Variants
 ### Plain file path
 - Target is any path that does not resolve as an archive selector and does not resolve as an existing-or-new SQLite selector.
 - Existing files are overwritten.
-- `write.ts` does not call `fs.mkdir()` on this path; parent-directory creation is only implemented in the archive branch.
+- Parent directories are created by `writeFileAtomically()`. A failed write never truncates an existing destination to 0 bytes.
+- Existing referents must be writable and are checked before publication. Hard-linked regular files are rejected rather than silently leaving aliases with stale bytes; this preserves the no-truncate guarantee instead of switching to an unsafe in-place fallback.
+- On Windows, a writable file held with write sharing but without delete sharing falls back to a rollback-capable in-place update after bounded rename retries, preserving the existing inode and editability.
 
 Example:
 
@@ -88,7 +91,7 @@ content: "hello\n"
 - Supported archive suffixes come from `parseArchivePathCandidates()`: `.tar`, `.tar.gz`, `.tgz`, `.zip`.
 - The inner path is normalized to `/`, strips empty and `.` segments, rejects `..`, and rejects directory targets ending in `/`.
 - Rewrites the whole archive file after replacing one entry.
-- Creates the parent directory for the archive file if needed.
+- Creates the parent directory only inside the guarded atomic publication path after destination trust validation.
 
 Example:
 
@@ -130,11 +133,21 @@ path: "data/app.sqlite:users:42"
 content: ""
 ```
 
+## Publication contract
+
+Plain-file writes stage to a sibling temp and publish with a same-directory `rename(2)`. A write that fails at any point before that rename leaves the destination byte-identical to its prior contents -- a failed write never truncates the target or leaves a 0-byte file -- and the staging file it created is removed. Successful writes leave no residue beside the destination.
+
+The staged bytes are fsynced before publication, but the parent directory is not, so publication is **not** crash-durable: a rename can be lost across a system crash.
+
+Overwrites are **last-writer-wins**. Destination identity is revalidated immediately before the rename, which rejects a target that was replaced or retargeted while staging, but the rename commits against the pathname. A concurrent writer that publishes a successor between that check and the rename is overwritten rather than detected.
+
+Destination symlinks are followed: the referent is replaced and the link is preserved. Hard-linked targets are rejected, because replacement would split the link group.
+
 ## Side Effects
 - Filesystem
   - Creates or overwrites plain files.
-  - Rewrites entire archive files when writing an archive entry.
-  - Creates parent directories for archive files only.
+  - Reconstructs and atomically publishes entire archive files when writing an archive entry; failed publication preserves an existing archive and cleans owned staging residue.
+  - Creates parent directories for plain files and archive files only after the destination boundary has been validated.
   - Mutates existing SQLite databases; never creates a new SQLite DB.
 - Subprocesses / native bindings
   - Uses Bun SQLite bindings via `bun:sqlite`.
@@ -171,7 +184,7 @@ content: ""
 ## Notes
 - Archive path detection runs before SQLite detection. A path that matches an archive selector is never treated as SQLite.
 - SQLite detection declines when an existing file with a `.sqlite` / `.db` suffix is present but does not have SQLite magic bytes; then the path falls back to a plain file write.
-- ZIP entry content is encoded with `new TextEncoder().encode(content)` in `#writeArchiveEntry()`. Non-ZIP archive writes pass the string directly to `Bun.Archive.write()`.
+- ZIP entry content is encoded with `new TextEncoder().encode(content)` in `#writeArchiveEntry()`. Non-ZIP archive entries are reconstructed with `Bun.Archive.bytes()` and both formats publish through `writeFileAtomically()`.
 - The prompt forbids two common anti-patterns: using `write` for routine edits that should use `edit`, and creating `*.md` / `README` files unless explicitly requested. It also forbids emojis unless requested.
 - Plain file writes report byte count using `cleanContent.length`, which is UTF-16 code units in JS, not an on-disk byte measurement.
 - `stripWriteContent()` only removes hashline prefixes when the session’s file display mode has `hashLines` enabled; otherwise content is written unchanged.

packages/coding-agent/CHANGELOG.md

@@ -19,6 +19,9 @@
 - Coordinator event journal rows can now be pushed to one opt-in webhook (#4706). External orchestrators that cannot stay attached to `gjc_coordinator_watch_events` long-poll (a 300s `await_turn` timeout is not session death) had no push of **existing** journal rows; they can now set `GJC_COORDINATOR_MCP_EVENT_WEBHOOK_URL` to receive each row as an authenticated POST whose body is the exact native `watch_events` record — same `seq`, same stable `id`, at-least-once so sinks dedupe on `id`. The feature is env-only and default-off (no MCP tool can set or read it), destinations are allowlisted (`https:` anywhere, `http:` loopback only, no redirects), the bearer token comes from a secret file path rather than env, an optional session-id scope restricts delivery to authorized sessions, and delivery runs through a durable per-row outbox off the journal append path with bounded attempts, exponential backoff, and a bounded request timeout — a dead sink never delays or rewrites terminal turn/session persistence. The five `GJC_COORDINATOR_MCP_EVENT_WEBHOOK_*` variables resolve through the trusted credential environment (`$credentialEnv`, the same provenance as the crash-relay DSN) rather than raw `process.env`, so a checkout's `.env` cannot select the egress destination or the token file. `watch_events` long-poll is unchanged and remains the source of truth; `gjc coordinator doctor` reports the resolved webhook state.
 - Extension activation is now transactional. `pi.registerFlag(..., { default })` and `pi.registerProvider(...)` used to mutate the shared `ExtensionRuntime` state directly with no rollback, so a factory that threw midway was discarded while its side effects leaked: the flag default stayed readable via `getFlag`/`getFlagValues` and the provider registration stayed queued for the ModelRegistry drain in `sdk/session.ts` and `runListModelsCommand`, activating providers from an extension that never activated. Each factory invocation now stages its shared-state writes in an `ExtensionActivationScope` (stage → factory completes without throwing → commit into the shared runtime); rollback discards the staged writes so a failed extension leaves no flag default and no provider registration behind, and earlier extensions' committed state is untouched. After commit the shared runtime is authoritative for `getFlag`, so runtime-side writes (CLI flag overrides, a later extension's committed default) stay observable to retained extension API objects exactly as before the transaction (#4718). Commit itself is transactional: prior flag entries and the provider-queue length are journaled before publication, so a throw partway through commit is undone before it escapes and the scope only becomes terminal once publication fully succeeds — a failed extension leaves nothing behind even when the failure happens during publication.
 - A content-free Anthropic capacity overload no longer ends the turn under the default retry configuration. Anthropic can answer with its typed `overloaded_error` as a statusless stream error, and session retry already classifies that as transient, but the bare-default admission list only covered watchdog timeouts and the Codex `server_is_overloaded` event — so the turn surfaced the raw provider envelope and went idle, leaving the operator to resend or switch models by hand for a failure the provider says to retry. The admission now also accepts Anthropic's own overload code, recognized by parsing the error envelope and requiring both the outer `type` and the nested `error.type` to match exactly. Nothing else changes: the attempt must still carry no assistant text, thinking, or tool call and no conflicting transport facts (a status-bearing or otherwise typed failure keeps failing closed), overload prose alone can never authorize a replay, and the existing capped exponential backoff and `retry.enabled: false` opt-out are untouched.
+- File tools no longer lose a just-written path or leave a 0-byte target when a write fails (#4734). `writethroughNoop` and LSP writethrough now publish through a sibling temp + rename (`writeFileAtomically`) so a permission/IO error cannot truncate the destination; `EACCES`/`EPERM`/`EROFS` surface as an actionable `ToolError` that says the original file is unchanged. Read tries the ACP `readTextFile` bridge when disk stat misses, and bridge failures fail closed: only an explicit `transport_unavailable`/`bridge_unavailable` code falls back to the agent host's disk, while structured denials and raw OS errno values such as `EPERM` do not, so a local read cannot bypass a remote client's access decision. Successful writes invalidate `fileReadCache`. Compaction-state now lists recent successful `write`/`edit`/`apply_patch`/`ast_edit` paths so a long-session compact does not silently drop in-flight file-tool context. This is independent of Windows directory-fsync `EPERM` (#4457) and of workflow-validation compaction (#4560).
+- Closed exact-head review findings on the #4734 atomic write path: the session-local trust boundary is now validated **before** any parent directory is created (a dangling symlink inside a trusted root resolves outside it, so creating parents first materialized an attacker-selected tree outside the sandbox before publication was refused, and the boundary is re-checked after `mkdir -p` follows existing symlinked ancestors); the publication parent is pinned by device/inode rather than realpath string, so a parent unlinked and replaced by a different directory at the same path is detected instead of published into; and the Windows in-place sharing fallback revalidates destination inode identity before mutating by pathname, refusing with `destUnchanged: true`/`not_published` when a concurrent writer substituted a successor during rename backoff. Read's ACP bridge fail-closed policy is now documented accurately: only explicit `transport_unavailable`/`bridge_unavailable` codes fall back to disk, never structured denials or raw OS errno.
+- Hardened the #4734 atomic write path after review: LSP writethrough awaited its `BunFile` write again (an unawaited call escaped the surrounding `try`, so a rejecting write was recorded as published and surfaced as an unhandled rejection), the Windows in-place sharing fallback now writes replacement and rollback bytes at absolute position 0 (`handle.writeFile()` resumes from the handle offset, so a partially accepted replacement left interleaved bytes while the result still reported `destUnchanged: true`), and the module contract no longer claims crash atomicity it does not provide. Publication is documented as last-writer-wins in `docs/tools/write.md`: identity is revalidated before the rename, but `rename(2)` commits against the pathname.
 - Runtime skill discovery now scans `skills.customDirectories`. Session startup already loaded those directories through `loadSkills`, but `discoverRuntimeSkills` and `findRuntimeSkillByName` searched only the canonical project and user roots, so a configured custom skill was invocable by exact name yet absent from every `skill_discovery` search -- usable only by someone who already knew it existed. Both discovery entry points now scan the configured directories at user level (so project-scoped queries exclude them), deduplicated and tilde-expanded the same way `loadSkills` does. Naming a directory is explicit consent, so custom directories are not gated on `skills.trustUserSkills` -- matching the startup rule -- while the `skills.enabled` master switch still suppresses them.
 - A broker that cannot retain its own publication now names the object that withheld authority. The native layer opens `sdk`, `sdk/broker.lock`, `sdk/broker.lock/owner.json`, and `sdk/broker.json` no-follow and reports every refusal as one opaque `Retained broker publication authority is unavailable.`, so `gjc sdk` died with nothing to act on and the precondition could only be learned from the native source — a shared multi-account layout that symlinks the agent directory's `sdk` entry crashed every broker start this way. The failure is still fatal and still rolls back its publication; it now appends the first obstruction (missing entry, symlinked entry, wrong file kind, unreadable entry, or a non-fixed-width `heartbeatAt`) ahead of a bounded agent directory, so the named object survives the 512-character startup-failure reason, and stays verbatim when every precondition holds so a named condition is never invented. Each object is probed with the native's own access mode — the lock record read-only, only the published record read/write — and a file kind is only ever named through the open the native itself refuses, so a layout the native accepts is never reported as an obstruction; the published record is read through the descriptor the no-follow open already verified, never reopened by name. When rollback fails too, the aggregate message now carries the acquisition diagnostic, since the durable startup-failure marker persists only that message.
 - Retained broker publication probing now opens POSIX objects non-blocking, diagnoses exact-buffer malformed records, escapes control and bidi characters in persisted agent-directory diagnostics, and covers native-rejected wrong-kind objects without inventing a condition the native layer accepts.