Skip to content

core: keep recordings when the Permission Model blocks fsync - #1907

Merged
simonklee merged 1 commit into
mainfrom
harden/407-audio-fsync-permission
Nov 1, 2026
Merged

simonklee merged 1 commit into
mainfrom
harden/407-audio-fsync-permission

Conversation

@simonklee

Copy link
Copy Markdown
Member

Since Node 26.9.0, FileHandle.sync() throws ERR_ACCESS_DENIED whenever the Permission Model is on (nodejs/node#65431). AudioRecorder finalizes a recording with sync() and close(), so under --permission every recording failed with "Audio recorder finalize failed" and was discarded. OpenTUI's own Node test lane runs with --permission and hits this as soon as CI moves past Node 26.7.

The recorder now closes the file without the sync when that call is denied, then publishes it as usual. Any other sync error still fails the recording and keeps the destination. The audio capture docs say that under --permission the file is not forced to disk before the rename.

Tested by rows in the existing sync-failure table (denied: the recording completes and the file is closed; another error code: it still fails) and by the core Node lane on Node 26.7.0 and 26.13.0.

Since Node 26.9.0, FileHandle.sync() throws ERR_ACCESS_DENIED whenever
the Permission Model is on (nodejs/node#65431). AudioRecorder treated
that as a finalize failure, so every recording under --permission was
discarded.

Close without the sync when it is denied; other sync errors still fail
the recording. The docs say the file is not forced to disk in that case.

Issue: 407
@simonklee
simonklee requested a review from kommander as a code owner October 31, 2026 20:37
@simonklee
simonklee merged commit 9b7b1ba into main Nov 1, 2026
27 checks passed
@simonklee
simonklee deleted the harden/407-audio-fsync-permission branch November 1, 2026 04:50
simonklee added a commit that referenced this pull request Nov 1, 2026
CI pins Node 26.7.0. On Windows, the Node run of the Yoga default-config
worker test sometimes crashed with an access violation at process exit.
A minidump showed the fault inside Node, not OpenTUI: after a worker
exits, Node 26.7.0 can write one byte into the freed worker Realm, and
here that byte broke V8's page pool, which then crashed in
`V8::Dispose`. No OpenTUI code was loaded at the time. Node fixed this
in nodejs/node#65778, released in 26.10.0.

This pins the current LTS, 26.13.0, in every workflow that pinned
26.7.0. It needed #1907 first: since Node 26.9.0, `FileHandle.sync()`
throws under the Permission Model, which the core Node lane uses, and
that broke audio recording. The `engines` floor stays at `>=26.4.0`.

Tested on Linux under 26.13.0 with the core and Solid Node lanes and the
core, Solid, SSH and Keymap packed-consumer tests; on Windows, this PR's
CI and later runs are the evidence. The crash hit about 1 in 175 Windows
jobs, so one green run proves little.
Dicklesworthstone added a commit to Dicklesworthstone/opentui that referenced this pull request Nov 9, 2026
Take 9b7b1ba (core: keep recordings when the Permission Model blocks
fsync, anomalyco#1907). Since Node 26.9.0, FileHandle.sync() throws
ERR_ACCESS_DENIED whenever the Permission Model is on
(nodejs/node#65431). AudioRecorder finalized a recording with sync() and
close(), so under --permission every recording failed with "Audio
recorder finalize failed" and was discarded. OpenTUI's own Node test lane
runs with --permission and hits this as soon as CI moves past Node 26.7.

The recorder now closes the file without the sync when that call is
denied, then publishes it as usual. Any other sync error still fails the
recording and keeps the destination. The audio capture docs say that
under --permission the file is not forced to disk before the rename.
packages/core/src/audio.ts, the existing sync-failure rows in
packages/core/src/tests/audio.test.ts, and
packages/docs/application-apis/audio-capture.mdx are the three files.
The incoming commit deletes nothing. The local side deletes nothing.
Merge-tree was clean. This does not push to anomalyco/opentui and does
not create origin/master.
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