fix(utils): throw AppError from app-log-files and verified-file guards

[thymikee]

Summary

Four file-identity guards in src/utils threw plain Error, so an agent hitting one saw UNKNOWN with the generic "unexpected internal error" hint, and tests could only pin them by message regex (ADR 0010 §1; #1781 B4):

Site	Was	Now
app-log-files.ts — app.log path is a symlink	Error("App-log file must not be a symbolic link: …")	AppError('COMMAND_FAILED', same message, { hint: NOT_REGULAR_FILE_HINT })
verified-file.ts — final path is not a regular file	Error("Final path must be a regular file: …")	COMMAND_FAILED + NOT_REGULAR_FILE_HINT
verified-file.ts — identity changed between open and post-open lstat	Error("Final file identity changed …")	COMMAND_FAILED + concurrent-replacement hint
verified-file.ts — create lost the identity race twice	Error("… could not be opened without an identity race")	COMMAND_FAILED + concurrent-replacement hint

COMMAND_FAILED because these are runtime failures of well-formed requests on daemon-derived paths (not user input → not INVALID_ARGS; not a capability gap or contention code). Hints answer "what next": remove the symlink/special file at this path and retry / stop the concurrent writer, then retry. The two "not a regular file" guards share one exported hint constant (NOT_REGULAR_FILE_HINT in verified-file.ts) — same failure mode seen from two sides, per ADR 0010 §3.

Small structural side-effect: both lstat wrappers used to throw from inside the try whose catch keys on error.code === 'ENOENT'; an AppError also carries .code, so the guard now sits outside that catch (lstatIfPresent, shared by both files) rather than relying on 'COMMAND_FAILED' !== 'ENOENT'.

Callers checked: app-log.ts, durable-capture-resource-store.ts (isMissingFile keys on errno, unaffected), platform-runtime-app-log-process.ts / platform-runtime-network-host.ts (error instanceof Error ? error.message — AppError extends Error). None depend on the plain-Error shape.

Closes #1792.

Validation

• src/daemon/__tests__/app-log.test.ts (3 cases), src/utils/__tests__/app-log-files.test.ts (1), src/utils/__tests__/verified-file.test.ts (3) tightened from toThrow(<string>) to assertThrowsAppError(…, { code: 'COMMAND_FAILED', message }).
• Two new tests reach the race guards, which the filesystem alone cannot: the post-open lstat is answered with a different file (identity changed), and openSync keeps returning EEXIST (identity race exhausted).
• Red against pre-fix code (git stash the two production files, run the three test files):

  Failed Tests 9
   FAIL app-log.test.ts > rejects a final app.log symlink before metadata touches its target
     AssertionError: expected AppError, got Error: Error: Final path must be a regular file: …/session/app.log
   FAIL app-log.test.ts > … before mark …            expected AppError, got Error: Error: App-log file must not be a symbolic link: …
   FAIL app-log.test.ts > … before clear …
   FAIL app-log-files.test.ts > rotation rejects a final app.log symlink …
   FAIL verified-file.test.ts > rejects a final symlink before read|append|truncate … (3)
   FAIL verified-file.test.ts > reports a typed failure when the file is swapped while it is being opened
   FAIL verified-file.test.ts > reports a typed failure when a create keeps losing the identity race
   Tests  9 failed | 5 passed (14)
  

  Restored: Tests 14 passed (14).
• pnpm check:affected --run: 166 affected test files / 923 tests green, lint/typecheck/format/layering clean.

Touched files: 5 (2 production, 3 tests). Scope stayed on the two utils. No docs/skills change: no command surface moved; error code/hint reach every surface through normalizeError.

[github-actions]

Size Report

Metric	Base	Current	Diff
JS raw	2.32 MB	2.32 MB	+412 B
JS gzip	761.4 kB	761.6 kB	+186 B
npm tarball	884.6 kB	884.7 kB	+123 B
npm unpacked	3.08 MB	3.08 MB	+412 B

Startup median (7 runs, lower is better):

Scenario	Base	Current	Diff
CLI --version	28.9 ms	29.1 ms	+0.2 ms
CLI --help	70.3 ms	71.8 ms	+1.5 ms

Top changed chunks:

Chunk	Raw diff	Gzip diff
dist/src/internal/daemon.js	0 B	+3 B
dist/src/session2.js	0 B	-2 B

[thymikee]

P1 regression gap: #1792/ADR 0010 require actionable recovery hints, but all tightened/new tests call assertThrowsAppError with only code/message. Removing or corrupting the four new hints leaves all 14 targeted tests green. Extend the helper or explicitly inspect AppError.hint and assert the shared not-regular-file hint and concurrent-replacement hint for every relevant guard, with planted-red proof. The implementation/error classification otherwise looks sound. PR remains draft with required CI running.

[thymikee]

CI note: Smoke Tests (iOS simulator lane, run 32164277510) failed at wait text Agent Device Tester with reason: wait_capture_stalled, readableCaptures: 0 in smoke:automation-input — an iOS capture stall on the fixture home, before any app-log/verified-file path is exercised. Same lane failed on refactor/inspected-use-token at 15:58 and passed on its rerun at 17:09; it passed on sibling PR #1852 (same base). This diff touches only src/utils/{app-log-files,verified-file}.ts error types (AppError vs Error, same messages/paths). Needs a gh run rerun 32164277510 --failed; I could not issue it from this environment.

[thymikee]

Out of draft. CI is green on the actual head 84e46327: 28/28 checks pass, including the Smoke Tests iOS lane whose earlier failure was the wait_capture_stalled flake noted above (full-workflow rerun passed; --failed was avoided because it loses the fixture-app artifact).

Review nits recorded but deliberately not fixed here: verified-file.ts still lets a raw ELOOP escape when a symlink is swapped in between the lstat and the openSync — that is a fourth throw site outside the three this issue enumerates, and typing it would widen the diff past #1792's scope.

[thymikee]

Re-reviewed exact head 84e46327: still blocked on the prior regression-proof finding. Every tightened/new assertThrowsAppError assertion supplies only code and message, and the helper does not inspect hint; deleting or corrupting NOT_REGULAR_FILE_HINT or CONCURRENT_REPLACEMENT_HINT leaves all 14 claimed targeted tests green even though the recovery hint is #1792’s core behavior. Assert the exact/shared hints for every relevant guard and include planted-red proof. Error classification otherwise looks sound, and all 28 exact-head checks are green (the earlier iOS capture stall passed on rerun).

[thymikee]

Blocker accepted — the finding was correct, and the proof was vacuous exactly where the issue's value is. Fixed in 8bf8695f (rebased onto current origin/main).

What was wrong: every assertion supplied code + message, and neither helper looked at the hint. NOT_REGULAR_FILE_HINT and CONCURRENT_REPLACEMENT_HINT could be deleted or reworded with all 14 targeted tests green — so the half of #1792 that ADR 0010 is actually about ("errors say how to recover") had no pin at all.

Fix:

1. assertThrowsAppError / assertRejectsAppError accept hint?: string | RegExp, checked against normalizeError(error).hint rather than details.hint. That is deliberate: a throw site that drops its hint does not produce undefined, it silently inherits defaultHintForCode('COMMAND_FAILED') — so asserting the raw detail would still pass on a guard that had degraded to the generic default. The assertion sees what a CLI/SDK/MCP caller sees.
2. Each of the six guards pins its exact hint text — as a literal in the test file, not the imported constant. Importing it would compare the constant to itself and stay green under precisely the "corrupt the constant" mutation you named. The literals carry a comment saying so, so nobody helpfully refactors them back into an import.

Planted-red proof (three separate plants, each restored afterwards):

(a) drop NOT_REGULAR_FILE_HINT from the not-a-regular-file guard — 5 tests across two files fail, and the failure shows the exact degradation the reviewer predicted:

FAIL verified-file.test.ts > rejects a final symlink before read|append|truncate … (3)
FAIL app-log.test.ts     > rejects a final app.log symlink before metadata|clear … (2)
AssertionError: Expected values to be strictly equal:
+ actual - expected
+ 'Retry with --debug and inspect diagnostics log for details.'
- 'agent-device only reads and writes regular files at this path. Remove the symbolic link or special file there and retry.'
  ❯ assertAppError src/__tests__/test-utils/app-error.ts:23:51
Tests  5 failed | 6 passed (11)

(b) reword CONCURRENT_REPLACEMENT_HINT, removing only the recovery step ("Stop the concurrent writer, then retry." → "Retry."):

FAIL verified-file.test.ts > reports a typed failure when the file is swapped while it is being opened
FAIL verified-file.test.ts > reports a typed failure when a create keeps losing the identity race
+ 'Another process replaced the file at this path while it was being opened. Retry.'
- 'Another process replaced the file at this path while it was being opened. Stop the concurrent writer, then retry.'

(c) drop the hint from the app-log symlink guard — app-log.test.ts > … before mark … fails with the same generic-default diff.

Restored: Tests 14 passed (14), and pnpm check:affected --run is green post-rebase (2,010 tests).

Still not fixed, deliberately: the raw ELOOP that escapes verified-file.ts when a symlink is swapped in between the lstat and the openSync. It is a fourth throw site outside the three #1792 enumerates, and typing it is a behaviour change to the open path rather than an error-classification fix. Happy to take it as a follow-up issue if you'd rather it not wait.

[thymikee]

Re-reviewed exact head 8bf8695f: the prior recovery-hint regression gap is fixed. The shared assertion helper now checks caller-visible normalizeError(...).hint, every affected guard pins a literal expected hint, and the planted mutations prove dropped/reworded hints fail rather than silently inheriting the generic COMMAND_FAILED default. Code review is clean. Exact-head CI is currently green except Linux/iOS Smoke still running. Before merge, refresh the PR body to stamp evidence at 8bf8695f, update affected-gate evidence to 2,010 tests, and correct touched files from 5 to 6.

[github-actions]

PR Preview Action v1.8.1
Preview removed because the pull request was closed.
2026-08-19 11:55 UTC