Repository navigation
fix(cli): history lists its entries even when this run cannot write the history - #6061
Conversation
…he history history --json, show and the other read-only subcommands open the history as its owner, which writes a lock and files pending changes. On a full disk or an unwritable cache folder that write failed the whole read. A read now falls back to the history as last recorded and says why on stderr; undo, restore and other writes still fail.
jrusso1020
left a comment
There was a problem hiding this comment.
Requesting changes at aba07bf.
P2: withReader classifies the error by errno alone (historyOwner.ts:277-278). EACCES/EPERM are in CANNOT_WRITE, but opening the owner can raise them from a read as well as a write. Repro on a real filesystem:
chmod 0111the existing blobs directory, leaving the home and log readable and writable.withOwnerrejects withEACCES,syscall: 'scandir'. That is a read failure.withReadertreats it as unwritable and falls back to the read view. It returns successfully and prints "Could not write this project's history", which is the wrong diagnosis.
Fix: take write provenance into account, either the syscall (open with write flags, mkdir, write, rename, ...) or a marker set by the write stage, rather than the code alone. Add a test where an opening-stage read fails, so a read EACCES propagates (or at least isn't reported as a write failure).
Checked and fine: write commands still go through withOwner, and the ran guard is sound. Once the task has started, its errors are rethrown rather than retried on the read view.
Review by Rames (hf-review)
Edit accuracy: accurate 271 (base branch 271), smooth 162 of thoseThe gate passes. Quarantined, measured but not gated (0) Unstable (1)
|
|
Fixed at 1ae5f8e. The fallback now keys on which call failed, not the error code alone.
|
jrusso1020
left a comment
There was a problem hiding this comment.
Re-reviewed at 1ae5f8e. The earlier read-side finding is fixed: real scandir and read-open failures now propagate instead of being reported as cache-write failures. The ran guard is still sound, and the write commands still use withOwner unchanged.
P2: a failed write-open on an existing log still blocks reading saved history (packages/cli/src/utils/historyOwner.ts:292). The new check treats an open as a write only when the file doesn't exist yet. Repro on a real filesystem:
- Create some history.
chmod 0444itslog.jsonl(still readable), leaving the directories writable.- Change
index.html. - Run
history list.
On startup the CLI records the external change through saveRecord, which opens the existing log with O_WRONLY | O_APPEND (packages/studio-server/src/history/historyLog.ts:100). That open raises EACCES. Because the log exists, absent(log) is false, so withReader rejects instead of showing the readable saved entries. Fix: track that an open was for writing for existing files too, not only when creating one. Keep the new tests for real read errors.
Seven focused filesystem checks ran against the current source, as uid 1000 with real permissions. Only project resolution and preview discovery were stubbed, and dependencies were borrowed from another checkout.
- Read denials on directories, the owner file and sources propagate.
- Write denials on history records and blobs fall back to the read view.
- A task error runs the task only once.
- The existing-log write-open failure above reproduces.
The full history suite was not re-run after the earlier dependency collection failure. No full-disk or Windows case was tested.
Review by Rames (hf-review)
|
Fixed at 501ad61. A denied
|
jrusso1020
left a comment
There was a problem hiding this comment.
Re-reviewed at 501ad61. Both earlier blockers are fixed:
- Directory and read-open permission errors still surface as errors.
- When appending to an existing, readable log is denied, the read now shows the recorded history instead of failing.
packages/cli/src/utils/historyOwner.ts:287-309 now decides whether an open failure was a write by checking whether the file is readable or missing, and still filters by syscall. The guard on the task having started is still there. undo, restore, pin, begin and end still use the unchanged withOwner write path.
Seven focused filesystem checks pass on Linux against the current withReader and history-engine source, with real permissions as uid 1000:
- An unreadable directory, owner file or source still fails with an error.
- A denied write to a history record or blob falls back to the read-only view.
- A task that throws runs only once.
- With
log.jsonlread-only and the project changed afterwards, the previously recorded entries are still shown.
Only project resolution and preview discovery were stubbed, and dependencies were borrowed from another checkout. The full history suite was not re-run: it failed to load earlier because of a parser missing from the borrowed dependencies. There was no Windows, native macOS or real full-disk run.
Review by Rames (hf-review)
What
hyperframes history(the list, with or without--json, plusshowandpeek) now lists the project's recorded changes even when this run cannot write the history folder, for example on a full disk or an unwritable cache folder. It prints the history as last recorded, plus one line on stderr naming the cause:Commands that change the project through the history (
undo,restore,pin,begin,end) still fail as before. Their job is the write.Why
Reading the history opens it as its owner. That writes the owner lock, the project record and copies of any changed files. If any of those writes failed, the read failed with it, and the user lost sight of every recorded change. Users on macOS hit this on 0.8.183:
ENOSPC: no space left on device, open '<history>/<id>/owner.pid-<pid>-<uuid>.tmp'(full disk)EPERM opening <history>/<project>/<file>.tmp(cache folder it may not write), twice from one installNone of those writes is needed to read.
readProjectHistoryalready reads the log without taking the lock or writing anything, andwithReaderalready falls back to it when another process holds the history. This PR uses the same fallback when opening fails because a write failed (ENOSPC,EDQUOT,EACCES,EPERMorEROFSfrom a write). A read that fails while opening, such as a blobs folder it may not list or an owner file it may not read, still fails with its own error.What the user gives up in that case: changes made since the last successful open are not filed into the history on this run. They couldn't be written anyway, and they get filed on the next open that can write.
Related work
None.
How
withReaderinpackages/cli/src/utils/historyOwner.ts, which every read-only history subcommand goes through.mkdir,write,rename,unlink,link,copyfileand the like, or a deniedopenof a file that is missing (creating it) or that this process can read (so the denial was for writing, for example appending to a read-only log). Node's error does not carry the open flags; the file's state answers which kind of open failed. Everything else propagates, includingscandirand anENOSPCfrom a file watcher.Test plan
history.test.ts, seven new tests. They use real read-only folders, not mocks, and skip on Windows and as root, where permissions don't block the write:mainEACCES ... open '<root>/<id>/project.json.<x>.tmp'chmod 0444 log.jsonl)EACCES ... open '<root>/<id>/log.jsonl'EACCES ... open '<root>/<id>/blobs/incoming-<uuid>'chmod 0111blobs) fails withEACCESfromscandir, not "could not write"EACCESfromopen, not "could not write"tsc --noEmitinpackages/cliexit 0; oxlint, oxfmt, fallow, comment ratchet and citations pass.openas a write: the owner-file test fails;openas a write: the two "lists" tests fail;EACCES: permission denied, open '<history>/<id>/project.json.<x>.tmp';chmod 0111: exit 1,EACCES: permission denied, scandir '<history>/<id>/blobs', no fallback.Not exercised: a real full disk (ENOSPC is the same fallback, keyed on the error code), and Windows, where
EPERM/EBUSYwhile another process holds the lock is still retried as busy first.