Skip to content

fix(mountsync): preserve local files when the server denies the write#50

Merged
khaliqgant merged 2 commits intomainfrom
fix/mount-preserve-local-on-write-denied
Apr 20, 2026
Merged

fix(mountsync): preserve local files when the server denies the write#50
khaliqgant merged 2 commits intomainfrom
fix/mount-preserve-local-on-write-denied

Conversation

@khaliqgant
Copy link
Copy Markdown
Member

@khaliqgant khaliqgant commented Apr 20, 2026

Problem

relayfile-mount silently destroys local files in its --local-dir whenever the workspace server rejects the push with HTTP 403. Downstream impact observed in a live cloud orchestrator sandbox:

  • Workflow agents write deliverables (e.g. intro-<cli>.md, opencode.json) to /project.
  • Mount's next sync cycle (~1s default) attempts to push them → server returns 403 because the synced agent lacks write permission on the workspace ACL.
  • pushSingleFile at internal/mountsync/syncer.go:622-630 deletes the local file on 403 and removes it from sync state.
  • From the agent's perspective the file is gone. Workflow verifications like file_exists: intro-*.md fail even though the agent successfully wrote the file moments earlier.

Evidence from a reproducing run

```

/project/.relay/permissions-denied.log

[2026-04-20T09:16:30Z] WRITE_DENIED /opencode.json: agent does not have write permission
[2026-04-20T09:16:30Z] WRITE_DENIED /probe-project-1776676590.md: agent does not have write permission
...
```

Within 5 seconds of a deterministic shell step writing /project/probe-project-*.md, /project/.probe-stamp, and /project/opencode.json, all three were gone from disk. Files written to `$HOME` (outside the mount) survived. The mount is actively, aggressively destroying user data.

Fix

On 403 during a create (exists=false), keep the local file, record a `WriteDenied` + `DeniedHash` entry in the sync state, and short-circuit future push attempts for the same content.

```diff
var httpErr *HTTPError
if errors.As(err, &httpErr) && httpErr.StatusCode == http.StatusForbidden {
if !exists {

  •    s.logDenial(\"WRITE_DENIED\", remotePath, \"agent does not have write permission\")
    
  •    if removeErr := os.Remove(localPath); removeErr != nil && !errors.Is(removeErr, os.ErrNotExist) {
    
  •        return removeErr
    
  •    }
    
  •    delete(s.state.Files, remotePath)
    
  •    return nil
    
  •    s.logDenial(\"WRITE_DENIED\", remotePath, \"agent does not have write permission; local copy preserved\")
    
  •    s.state.Files[remotePath] = trackedFile{
    
  •        ContentType: snapshot.ContentType,
    
  •        Hash:        snapshot.Hash,
    
  •        WriteDenied: true,
    
  •        DeniedHash:  snapshot.Hash,
    
  •    }
    
  •    return nil
    
    }
    ```

Supporting changes

  • trackedFile gains `WriteDenied bool` and `DeniedHash string`. Distinct from the existing `Denied` (read-denied, local file removed) so future readers don't conflate semantics.
  • pushLocal (first loop) skips re-pushing a write-denied entry when `tracked.DeniedHash == snapshot.Hash`. Prevents log spam and redundant 403 round-trips.
  • pushLocal (second loop) excludes `WriteDenied` entries from the "state has it, remote doesn't → delete" reconcile path. Write-denied files were never on the remote; their absence is expected.
  • applyRemoteDelete returns early for `WriteDenied` entries for the same reason.
  • pushSingleFile revision selection: when re-trying a previously write-denied entry whose `Revision` is empty, fall back to the create-if-absent sentinel "0" instead of sending an empty revision.

Behavior after the fix

Scenario Before After
Agent writes new file, server 403s File deleted within 1s File preserved; state flags it WriteDenied
Same cycle, repeated Same as above Re-push skipped, no new denial log entry
User edits file N/A (gone) Hash differs from DeniedHash → retry; success or fresh denial log entry
Full reconcile / Reconcile() Would also delete file Excluded from delete path

Tests

  • New: `TestLocalCreatePreservedWhenServerDeniesWrite` — exercises all four invariants above against a mock server that 403s every write to a specific path.
  • Full mountsync suite passes (`go test ./internal/mountsync/...`).
  • Full repo: `go test ./...` green.

Follow-up (not in this PR)

The 403 itself is a server-side ACL decision. This PR makes the mount non-destructive when it happens — it does not change the ACL policy. If the orchestrator agent is expected to write back, the workspace ACL for that agent needs `fs:write` on the paths it pushes. That's a relayauth/relayfile ACL concern, tracked separately.

🤖 Generated with Claude Code


Open in Devin Review

khaliqgant and others added 2 commits April 20, 2026 11:30
Previously `pushSingleFile` deleted the local copy of a newly-created
file whenever the server returned HTTP 403 for its create. On a cloud
orchestrator sandbox where the workspace ACL denies writes for the
synced agent, this silently wiped every agent-written file in the
mount's `local-dir` within one sync cycle (~1s default). Agents that
wrote workflow deliverables (e.g. `intro-<cli>.md`, `opencode.json`)
saw their writes succeed on disk, then disappear mid-run.

Evidence from a live cloud run:

  denial.log: "WRITE_DENIED /opencode.json: agent does not have write
              permission"
  sandbox:    /project/opencode.json missing 5s after write
              /home/daytona/... (outside local-dir) preserved

Change: on 403 for a create, keep the local file on disk, write a
WRITE_DENIED entry to the denial log (now annotated "local copy
preserved"), and record a `WriteDenied` / `DeniedHash` entry in the
sync state. `pushLocal` and `applyRemoteDelete` now skip entries
flagged `WriteDenied` so:

  1. Subsequent sync cycles don't re-push identical content (no log
     spam, no redundant server round-trip).
  2. If the user edits the file (hash changes) we retry the push and
     either succeed or log a fresh denial — never silently skipped.
  3. The "tracked in state but absent from remote → delete local"
     reconcile path is disabled for write-denied entries, since their
     absence from the remote snapshot is expected.

Also tightens the revision selection in `pushSingleFile`: a state
entry with `Revision=""` (previously WriteDenied, now retrying)
falls back to the create-if-absent sentinel "0" instead of sending
an empty `baseRevision` on the next attempt.

Test: `TestLocalCreatePreservedWhenServerDeniesWrite` covers all four
invariants (file preserved, denial logged, reconcile doesn't re-spam
the log or re-delete, content edits retry the push).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The v0.2.0 release commit bumped every workspace package.json to 0.2.0
but didn't regenerate package-lock.json, leaving @relayfile/sdk pinned
to 0.1.13 in the lockfile. Every `npm ci` in CI (SDK Typecheck,
E2E, Contract) failed with:

  npm error Missing: @relayfile/sdk@0.1.13 from lock file

Regenerate the lockfile (npm install --package-lock-only) so all
workspace versions line up with package.json. No runtime dep changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ba985907e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +637 to +641
s.state.Files[remotePath] = trackedFile{
ContentType: snapshot.ContentType,
Hash: snapshot.Hash,
WriteDenied: true,
DeniedHash: snapshot.Hash,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Record a deletable revision for write-denied creates

This new state entry stores a write-denied file without any Revision, but pushSingleDelete later sends tracked.Revision as If-Match when the user removes/renames that local file. With an empty revision, the HTTP API returns 412 precondition_failed (handleDeleteFile rejects empty If-Match), so local delete events start failing and the stale write-denied record is never cleaned up. In practice, a file that was denied on create can become undeletable from sync state and future recreates on that path can behave incorrectly.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

@khaliqgant khaliqgant merged commit 432aade into main Apr 20, 2026
6 checks passed
@khaliqgant khaliqgant deleted the fix/mount-preserve-local-on-write-denied branch April 20, 2026 09:40
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