Skip to content

fix(core): fall back to a direct write when link() is denied with EACCES - #7474

Merged
miguel-heygen merged 2 commits into
mainfrom
fix/atomic-create-eacces
Nov 11, 2026
Merged

miguel-heygen merged 2 commits into
mainfrom
fix/atomic-create-eacces

Conversation

@miguel-heygen

@miguel-heygen miguel-heygen commented Nov 10, 2026 •

Copy link
Copy Markdown
Collaborator

What

createFileAtomically() (core) and the studio history owner lock (ownerLock.ts) now treat EACCES from link() the same way they treat EPERM: as "this folder doesn't allow hard links". Both then use their existing exclusive direct write (wx).

Why

On Android app data (Termux, SELinux enforcing), link(2) is denied with EACCES even in a folder where the temp file was just written. Both sites treated it as fatal:

  • hyperframes init aborted halfway. index.html and meta.json were written, but hyperframes.json and package.json were not.
  • Every render logged Post-render step 'recordRenderSuccess' failed … link …/video-state-lock/owner.pid-…tmp -> owner.pid.

Related work

Fixes #7473 (thanks @rdsdevcoorp for the exact diagnosis, including the second bundled copy).

How

Rebased on current main. The change is now 4 files, +5 -5:

  • packages/core/src/atomicFile.ts: add EACCES to NO_HARD_LINKS, and name Android app data in the comment above it.
  • packages/core/src/atomicFile.test.ts: add EACCES to the no-hard-links cases.
  • packages/studio-server/src/history/ownerLock.ts: add EACCES to its own NO_HARD_LINKS. This is the lock path from the report's second error.
  • packages/studio-server/src/history/ownerLock.test.ts: add EACCES to its no-hard-links cases.

The share lock (packages/share/src/projectLock.mjs) and its test already treat EACCES as no hard links on main, so this PR no longer touches them.

Both callers have just written the temp file or draft into the same folder, so an EACCES from link() means links are refused there, not that the folder is unwritable. The direct path stays exclusive: an existing file still raises EEXIST, and a truly unwritable folder still raises EACCES from open.

I checked every other linkSync site. videoFrameExtractor and distributed/plan already fall back to a copy on any error. engine/src/utils/urlDownloader.ts:986 has no fallback for any no-hard-link code. That is a separate, pre-existing gap and is left for a follow-up: losing callers there reuse any non-empty file, so a direct write needs its own design. The no-hard-link code lists have also drifted apart; merging them into one export from core is a separate follow-up.

Size

Under 100 lines: one word (EACCES) added to two code lists, plus the two tests that list the same codes.

Test plan

  • Unit tests updated: EACCES is added to the no-hard-links cases in the core and studio-server suites.
  • Fails without the fix: with EACCES removed from core's NO_HARD_LINKS, packages/core/src/atomicFile.test.ts fails writes directly on a volume without hard links (EACCES) (1 failed, 37 passed, 1 skipped). Restored, it passes.
  • On the rebased head: atomicFile.test.ts 38 passed (1 skipped), ownerLock.test.ts 22/22, and the share suite's kernel.test.mjs 31/31.
  • Manual testing on Termux: not done here. The reporter verified the same one-line change by patching both dist chunks.
  • Comments say why. The Android note is folded into the existing comment in core.

@github-actions

github-actions Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Edit accuracy: accurate 267 (base branch 267), smooth 103 of those

The gate passes.
Smoothness is reported in the artifact, not gated. A case fails only if it fails 2 of 3 runs.

Quarantined, measured but not gated (8)

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Independent review at 174d183cd99bf694bdace253931615ed3236f5fc. This is a COMMENT review because this account opened the PR.

(1) Is EACCES safe to treat as "no hard links" at both sites? Yes.

  • Both sites write their source into the same folder just before linking:
    • createFileAtomically writes its temp sibling (atomicFile.ts:135).
    • claim writes ${file}-<pid>-<uuid>.tmp beside the lock (ownerLock.ts:114-115).
  • So an EACCES from link() there can't be the usual "no write or search permission on the folder". On Linux that case has already passed, and protected_hardlinks answers EPERM, not EACCES. What's left is a policy denial, which is what SELinux on Termux does (issue #7473: ln a b → Permission denied).
  • Exclusivity doesn't depend on the error code:
    • Both fallbacks open with wx (O_CREAT|O_EXCL): createDirectly at atomicFile.ts:85, and createExclusively at ownerLock.ts:133.
    • An existing file still raises EEXIST. In claim, EEXIST from link() is still checked first, at :121.
    • A folder that really is unwritable fails at the wx open with its own EACCES, so the person still sees the error.
  • What the fallback gives up is the same as for EPERM and ENOTSUP today: a reader can briefly see a partial file. ownerLock already covers this with beingTaken (a size-0 file younger than TAKING_MS).
  • Windows: WINDOWS_BUSY includes EACCES, but busyOnWindows only wraps the outer retry at :227. A Windows EACCES from link() now goes through createExclusively, which either takes the lock, returns false on EEXIST, or throws back into the same busy retry. So that behaviour doesn't change.
  • Both of the issue's failures go through these two sites:
    • init writes hyperframes.json via createFileAtomically;
    • recordRenderSuccess writes owner.pid via takeHistoryOwnership (videoState.ts:113).

(2) The other hard-link sites

  • engine/src/services/videoFrameExtractor.ts:2010 and producer/src/services/distributed/plan.ts:1158: a bare catch falls back to copyFileSync. Fine on Termux.

  • producer/src/services/distributed/planV2Publisher.ts:36-40: canFallbackToCopy already includes EACCES. Fine.

  • share/src/projectLock.mjs:64-70 is the same contract as these two sites (link a candidate, on a no-hard-links code open(lock, "wx")), and its list lacks EACCES. Important for consistency. Nothing in this repo imports @hyperframes/share on Android, so it doesn't reach the issue's path. But the lists have already drifted: core has EXDEV and EISDIR, ownerLock has ENOSYS, and share has EINVAL. One exported NO_HARD_LINKS in core, used by all three, would stop the next code from landing in only one place.

  • engine/src/utils/urlDownloader.ts:986: only EEXIST is handled, so on Termux any render with an http(s) media source fails at publish with EACCES. The same is already true on FAT and exFAT with EPERM. Not blocking for this PR, but worth an issue. It can't take this PR's one-line fix:

    • its publish has to be atomic and no-clobber, because the losing caller trusts any existing non-empty file (race_reused, :999-1000);
    • a direct wx write would let a concurrent caller reuse a half-written download.

    A fallback there needs its own design, for example wx to a sibling plus a completion marker, or a lock around a copy.

(3) Do the tests pin the change? Yes.

  • With EACCES removed from atomicFile.ts, only writes directly on a volume without hard links (EACCES) fails (1 of 39).
  • With EACCES removed from ownerLock.ts, only takes the lock when linking fails with EACCES fails (1 of 22).
  • Locally the full files pass: atomicFile.test.ts 38 passed / 1 skipped, ownerLock.test.ts 22/22.
  • The EEXIST and live-owner refusal tests use other codes. The fallback path doesn't depend on the code, so that coverage carries over to EACCES.

CI: important.

  • The Comments check fails: comment-ratchet.mjs reports that the comment share rose in both files (atomicFile.ts from 17/205 to 18/206, ownerLock.ts from 12/244 to 13/245). It isn't in main's required list, but it's red and the PR body doesn't explain it.
  • A fix that adds no comment line:
    • in atomicFile.ts, fold Android into the existing line 10 ("…FAT, exFAT, some network shares, Android app data…");
    • in ownerLock.ts, drop the new line, since the commit message and the tests already carry the reason.
  • CodeQL is neutral (a missing configuration, not an alert).
  • At 01:32 UTC: 78 success, 11 skipped, 3 still running, and Comments is the only failure. The required Build, Typecheck, Test, Test reachability, Test: runtime contract, Render on windows-latest, Studio and player captures, Studio: edit accuracy gate and Semantic PR title all pass. Skills: manifest in sync is skipped. regression and Tests on windows-latest were still running.

Verdict: COMMENT (would approve once the Comments check is green)
Reasoning: EACCES is safe to treat as "no hard links" at both sites, because the wx fallback keeps exclusivity and the tests pin both additions. The red comment-ratchet check is the only thing to fix here. The share lock list and urlDownloader are follow-ups.

@miguel-heygen miguel-heygen left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Re-review at 05bd36e902ce5f5234aa5155052d8dbed00fd968 (one commit on top of 174d183cd, which I reviewed above). This is a COMMENT review because this account opened the PR. I'd approve this head.

What changed since my last review, and checks

  • packages/core/src/atomicFile.ts:10: the Android note is now part of the existing comment line instead of a new one. ownerLock.ts loses its added comment, and the EACCES entry stays at ownerLock.ts:19. That was the cause of the red "Comments" check; it is green at this head.
  • packages/share/src/projectLock.mjs:67: EACCES is added to the no-hard-links list. This was my consistency note. The fallback is still exclusive: open(lock, "wx") at :72, and an EEXIST from link() is still not in the list, so it is still thrown.
  • packages/share/tests/kernel.test.mjs:338: EACCES is added to the fallback loop. I removed EACCES from projectLock.mjs and exactly the new case failed (exclusive-create fallback … with EACCES, 5/6 pass). With it restored, kernel.test.mjs passes 22/22.
  • urlDownloader.ts:986 and merging the three code lists into one are listed in the body as follow-ups. That works for me; neither blocks this PR.

CI

  • The current run at this head is green: 86 success, 13 skipped, 0 failures. That includes Comments, Build, Typecheck, Lint, Format, CLI smoke (required), Test (cli 1/2, 2/2, runtime, studio, support), regression (all 10 shards), and Tests on windows-latest (all 5 lanes).
  • The one red Test at this head is in an earlier run that was cancelled when the new one started. It failed only because its Detect changes step was cancelled; it does not reflect the code.

Verdict: APPROVE
Reasoning: The new commit fixes the one red check and closes the sibling-site gap from my last review. The new share case is pinned by a test that fails without the change, and the remaining items are tracked as follow-ups.

On Android app data (Termux, SELinux enforcing), link(2) fails with
EACCES in a folder the temp file was just written to. createFileAtomically()
and the studio history owner lock treated that as fatal, so
`hyperframes init` left a half-scaffolded project and every render logged
a recordRenderSuccess failure. Both now take their existing exclusive
direct-write path, as they already do for EPERM.

Fixes #7473
Same Termux/SELinux link() denial as core and the studio history lock:
the share lock's link-then-exclusive-create fallback now covers it too.
Folds the Android note into the existing comment so no file's comment
share rises.
@miguel-heygen
miguel-heygen force-pushed the fix/atomic-create-eacces branch from 05bd36e to 60d723d Compare November 11, 2026 07:47

@heygengenesis heygengenesis Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Approve at 60d723dd. Nothing blocks this.

  • The fails without the fix: line is present: atomicFile.test.ts › writes directly on a volume without hard links (EACCES). The studio-server side has its own case pinning the ownerLock.ts entry.
  • It's safe to treat EACCES as "no hard links" at both sites:
    • Each site writes its temp or draft file into the same folder just before link(), so an EACCES there can't be a plain directory-permission error.
    • Both fallbacks still open with wx (atomicFile.ts:84, ownerLock.ts:132), so exclusivity is kept and an existing file still raises EEXIST.
    • In claim, EEXIST from link() is still checked before the list (ownerLock.ts:120).
    • A folder that really can't be written still fails at the wx open.
  • Windows behaviour is unchanged. WINDOWS_BUSY only wraps the outer retry, and the wx path either takes the lock, returns false, or throws back into that retry.
  • The rebase drops the share change correctly. projectLock.mjs:136 on main already lists EACCES. The branch is 0 behind main.
  • Every check on this head passes, across both CI runs: CI, regression, Windows render, Comments, CodeQL, captures, perf.
  • The urlDownloader.ts:986 gap and merging the three code lists into one remain tracked follow-ups.

@miguel-heygen
miguel-heygen added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit f36463e Nov 11, 2026
141 checks passed
@miguel-heygen
miguel-heygen deleted the fix/atomic-create-eacces branch November 11, 2026 08:36
@miguel-heygen miguel-heygen mentioned this pull request Nov 11, 2026
3 of 4 tasks
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.

createFileAtomically throws EACCES on Android/Termux: hard links are denied, so init and render state writes abort

1 participant