functionalscript · sergey-shandar · Aug 14, 2026 · Aug 14, 2026 · Aug 14, 2026 · Aug 14, 2026
diff --git a/changelog/unreleased/1559.md b/changelog/unreleased/1559.md
@@ -0,0 +1,4 @@
+- `effects/node/virtual`: `readFile`/`readBytesOp` narrow via
+  `assert(Array.isArray(file), …)` instead of an unreachable defensive
+  `return`; `rmOp` drops its unreachable "is a directory" guard entirely
+  [#1559](https://github.com/functionalscript/functionalscript/pull/1559)
diff --git a/fjs/effects/node/virtual/module.f.mjs b/fjs/effects/node/virtual/module.f.mjs
@@ -10,7 +10,7 @@
  * @import { Dir, State, _Entity } from './types.ts'
  */
 
-import { todo } from '../../../asserts/module.f.mjs'
+import { assert, todo } from '../../../asserts/module.f.mjs'
 import { isProperPrefix, join, parse } from '../../../path/module.f.mjs'
 import { utf8ToString } from '../../../text/module.f.mjs'
 import { empty, length, maxLengthBytes, msb, vec } from '../../../types/bit_vec/module.f.mjs'
@@ -92,8 +92,11 @@ const readFile = readOperation((dir, path) => {
     const file = dir[path[0]]
     if (typeof file === 'function') { throw new Error(`'${path[0]}' is a JsModule; readFile not supported`) }
     if (file === undefined) { return enoent }
-    if (!Array.isArray(file)) { return error(`'${path[0]}' is not a file`) }
-    const chunks = /** @type {readonly Vec[]} */ (file)
+    // `operation`'s wrapper descends into every plain-object (`Dir`) entry
+    // before this op ever runs, and the `JsModule` case already threw above,
+    // so `file` here is always a `Vec[]` — never a bare `Dir`.
+    assert(Array.isArray(file), `'${path[0]}' is not a file`)
+    const chunks = file
     const capBits = maxLengthBytes * 8n
     let result = empty
     for (const chunk of chunks) {
@@ -167,7 +170,11 @@ const rmOp = (dir, path) => {
     const [name] = path
     const entry = dir[name]
     if (entry === undefined) { return [dir, error('no such file')] }
-    if (!Array.isArray(entry) && typeof entry === 'object') { return [dir, error('is a directory')] }
+    // No "is a directory" guard here: `operation`'s wrapper descends into
+    // every plain-object (`Dir`) entry before this op ever runs, so `entry`
+    // is always a `Vec[]` or a `JsModule` — never a bare `Dir` — and rm can
+    // always proceed. (`rm` on a genuinely non-empty directory instead hits
+    // `path.length !== 1` above, once the wrapper has descended into it.)
     const { [name]: _, ...rest } = dir
     return [rest, okVoid]
 }
@@ -195,7 +202,13 @@ const extractEntity = (dir, path) => {
 
 /** @type {(dir: Dir, path: readonly string[], entity: _Entity) => readonly [Dir, IoResult<void>]} */
 const insertEntityAt = (dir, path, entity) => {
-    if (path.length === 0) { return [dir, error('cannot insert at root')] }
+    // `insertEntityAt`'s only external caller, `rename`, always rejects an
+    // empty `dst` earlier — `isProperPrefix([], srcParsed)` is true whenever
+    // `srcParsed` is non-empty, so renaming onto root is already caught as
+    // "onto an ancestor" before this function runs. The recursive self-calls
+    // below never pass an empty path either (they only recurse when
+    // `path.length > 1`, with a non-empty remainder).
+    assert(path.length > 0, 'cannot insert at root')
     if (path.length === 1) {
         const [name] = path
         const existing = dir[name]
@@ -250,13 +263,16 @@ const readBytesOp = (path, offset, size) => readOperation((dir, p) => {
     const file = dir[p[0]]
     if (typeof file === 'function') { throw new Error(`'${p[0]}' is a JsModule; readBytes not supported`) }
     if (file === undefined) { return enoent }
-    if (!Array.isArray(file)) { return error(`'${p[0]}' is not a file`) }
+    // `operation`'s wrapper descends into every plain-object (`Dir`) entry
+    // before this op ever runs, and the `JsModule` case already threw above,
+    // so `file` here is always a `Vec[]` — never a bare `Dir`.
+    assert(Array.isArray(file), `'${p[0]}' is not a file`)
     if (!Number.isInteger(offset)) { return error(`Offset ${offset} is not an integer`) }
     if (!Number.isInteger(size)) { return error(`Chunk size ${size} is not an integer`) }
     if (offset < 0) { return error(`Offset ${offset} is negative`) }
     if (size < 0) { return error(`Chunk size ${size} is negative`) }
     if (BigInt(size) > maxLengthBytes) { return error(`Chunk size ${size} exceeds maximum allowed size of ${maxLengthBytes} bytes`) }
-    const chunks = /** @type {readonly Vec[]} */ (file)
+    const chunks = file
     let toSkip = BigInt(offset) * 8n
     let toRead = BigInt(size) * 8n
     let result = empty

diff --git a/fjs/effects/node/virtual/proof.f.mjs b/fjs/effects/node/virtual/proof.f.mjs
@@ -3,8 +3,8 @@
  */
 
 import { assert, assertEq } from '../../../asserts/module.f.mjs'
-import { access, awaitIfPromise, fetch, rm, writeFile, readFile, readdir, import_, rename, readBytes, writeBytes, stat } from '../module.f.mjs'
-import { maxLengthBytes, vec, vec8 } from '../../../types/bit_vec/module.f.mjs'
+import { access, awaitIfPromise, fetch, rm, writeFile, readFile, readdir, import_, rename, readBytes, writeBytes, stat, createExclusive } from '../module.f.mjs'
+import { empty, length, maxLengthBytes, vec, vec8 } from '../../../types/bit_vec/module.f.mjs'
 import { emptyState, virtual } from './module.f.mjs'
 
 export const proof = {
@@ -19,7 +19,10 @@ export const proof = {
             const [, result] = virtual(emptyState)(rm('notexist.txt'))
             assert(result[0] === 'error')
         },
-        isDirectory: () => {
+        onDirectory: () => {
+            // `operation`'s wrapper descends into 'mydir' (a plain object),
+            // so rmOp itself runs with an empty remaining path and rejects
+            // via its `path.length !== 1` guard, not a directory-specific one.
             /** @type {Dir} */
             const inner = {}
             /** @type {Dir} */
@@ -110,6 +113,132 @@ export const proof = {
             const root = { 'a.f.ts': /** @type {JsModule} */ (() => ({})) }
             virtual({ ...emptyState, root })(readFile('a.f.ts'))
         },
+        readBytesOnJsModule: () => {
+            // readBytes on a JsModule path covers typeof file === 'function' branch
+            /** @type {Dir} */
+            const root = { 'a.f.ts': /** @type {JsModule} */ (() => ({})) }
+            virtual({ ...emptyState, root })(readBytes('a.f.ts', 0, 1))
+        },
+    },
+    readFileSkipsEmptyChunk: () => {
+        // A file stored with a zero-length chunk ahead of real data: readFile's
+        // loop must skip it (`chunkLen === 0n`) rather than concatenating it.
+        /** @type {Dir} */
+        const root = { 'f': [empty, vec8(0x42n)] }
+        const [, result] = virtual({ ...emptyState, root })(readFile('f'))
+        assert(result[0] === 'ok', result)
+        assertEq(length(result[1]), 8n)
+    },
+    readdirSkipsUndefinedEntry: () => {
+        // `Dir`'s index signature is optional (`{[name]?: _Entity}`), so an
+        // entry can legitimately be present with value `undefined` (e.g. after
+        // a rename leaves a stale key in some future refactor). readdir's loop
+        // must skip such entries rather than reporting them.
+        /** @type {Dir} */
+        const root = { 'd': { 'a': undefined, 'b': [vec8(0x42n)] } }
+        const [, result] = virtual({ ...emptyState, root })(readdir('d', {}))
+        assert(result[0] === 'ok', result)
+        assertEq(result[1].length, 1)
+    },
+    renameEmptySrc: () => {
+        // rename('', dst): src parses to the root path itself.
+        const [, result] = virtual(emptyState)(rename('', 'dst'))
+        assert(result[0] === 'error')
+        assertEq(result[1], 'cannot extract root')
+    },
+    renameSrcThroughFile: () => {
+        // rename('a/b', dst) where 'a' is a file, not a directory: the
+        // intermediate segment can't be descended into.
+        /** @type {Dir} */
+        const root = { 'a': [vec8(0x42n)] }
+        const [, result] = virtual({ ...emptyState, root })(rename('a/b', 'dst'))
+        assert(result[0] === 'error')
+    },
+    renameSrcThreeLevelsMissing: () => {
+        // rename('a/b/c', dst) where 'a/b' exists but 'c' doesn't: the error
+        // from the deepest extractEntity call propagates through two levels
+        // of recursion.
+        /** @type {Dir} */
+        const root = { 'a': { 'b': {} } }
+        const [, result] = virtual({ ...emptyState, root })(rename('a/b/c', 'dst'))
+        assert(result[0] === 'error')
+    },
+    renameDstMissingIntermediate: () => {
+        // rename(src, 'missingdir/x'): the destination's parent doesn't exist.
+        /** @type {Dir} */
+        const root = { 'src': [vec8(0x42n)] }
+        const [, result] = virtual({ ...emptyState, root })(rename('src', 'missingdir/x'))
+        assert(result[0] === 'error')
+    },
+    renameDstThroughFile: () => {
+        // rename(src, 'blocker/x') where 'blocker' is a file, not a directory.
+        /** @type {Dir} */
+        const root = { 'src': [vec8(0x42n)], 'blocker': [vec8(0x1n)] }
+        const [, result] = virtual({ ...emptyState, root })(rename('src', 'blocker/x'))
+        assert(result[0] === 'error')
+        assertEq(result[1], 'not a directory')
+    },
+    renameDstNestedError: () => {
+        // rename(src, 'a/b/c') where 'a/b' is a file: insertEntityAt's error
+        // one level down propagates through the outer recursive call.
+        /** @type {Dir} */
+        const root = { 'src': [vec8(0x1n)], 'a': { 'b': [vec8(0x2n)] } }
+        const [, result] = virtual({ ...emptyState, root })(rename('src', 'a/b/c'))
+        assert(result[0] === 'error')
+        assertEq(result[1], 'not a directory')
+    },
+    createExclusiveNestedMissing: () => {
+        // createExclusive('a/b') where 'a' doesn't exist: the operation
+        // wrapper falls through with the full remaining path. Start from a
+        // non-empty root and check it survives untouched, so a mutant that
+        // returns the right error tag alongside a wiped dir would be caught.
+        /** @type {Dir} */
+        const root = { keep: [vec8(0x1n)] }
+        const [state, result] = virtual({ ...emptyState, root })(createExclusive('a/b'))
+        assert(result[0] === 'error')
+        assertEq(Object.keys(state.root).length, 1)
+    },
+    writeBytesNestedMissing: () => {
+        // writeBytes('a/b', ...) where 'a' doesn't exist. Non-empty root, as above.
+        /** @type {Dir} */
+        const root = { keep: [vec8(0x1n)] }
+        const [state, result] = virtual({ ...emptyState, root })(writeBytes('a/b', 0, vec8(0x1n)))
+        assert(result[0] === 'error')
+        assertEq(Object.keys(state.root).length, 1)
+    },
+    writeBytesMissingFile: () => {
+        // writeBytes on a path that doesn't exist at all: writeBytes never
+        // creates. Non-empty root, as above.
+        /** @type {Dir} */
+        const root = { keep: [vec8(0x1n)] }
+        const [state, result] = virtual({ ...emptyState, root })(writeBytes('missing', 0, vec8(0x1n)))
+        assert(result[0] === 'error')
+        assertEq(Object.keys(state.root).length, 1)
+    },
+    writeBytesOnJsModule: () => {
+        // writeBytes on a JsModule entry covers the `!Array.isArray(file)`
+        // branch (unlike readFile/readBytes, writeBytes has no separate throw
+        // for JsModule, so this is the reachable way to hit "not a file").
+        /** @type {Dir} */
+        const root = { 'a.f.ts': /** @type {JsModule} */ (() => ({})) }
+        const [, result] = virtual({ ...emptyState, root })(writeBytes('a.f.ts', 0, vec8(0x1n)))
+        assert(result[0] === 'error')
+    },
+    writeBytesNegativeOffset: () => {
+        /** @type {Dir} */
+        const root = { 'file': [vec8(0x1n)] }
+        const [, result] = virtual({ ...emptyState, root })(writeBytes('file', -1, vec8(0x2n)))
+        assert(result[0] === 'error')
+        assertEq(result[1], 'Offset -1 is invalid')
+    },
+    statNestedMissing: () => {
+        // stat('a/b') where 'a' doesn't exist.
+        const [, result] = virtual(emptyState)(stat('a/b'))
+        assert(result[0] === 'error')
+    },
+    statMissingFile: () => {
+        const [, result] = virtual(emptyState)(stat('missing'))
+        assert(result[0] === 'error')
     },
     renameSamePath: () => {
         // rename('a', 'a') should succeed as a no-op, not reject