From b78eba08d320a58c4cd793cf4811444e351ffa17 Mon Sep 17 00:00:00 2001
From: Agando <roduccomm@gmail.com>
Date: Tue, 27 Oct 2026 17:10:40 +0100
Subject: fix(newos4): `if test …` was always false and `for` items never
 globbed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Found by running the shipped runner the way a user would, not by re-reading it.

- _evalTest() stripped [ ] but not the `test` keyword, so `if test -f /work/alpha.txt` evaluated
  the word "test" as an operator, returned false, and every such branch took the else arm —
  measured live: "alpha missing" for a file that exists. Both spellings work now
  (`test -f x`, `[ -f x ]`), plus `-d`, `-e`, `-z`, `-n`, `!` and the = / != comparisons.
- `for f in /work/*.txt` ran ONCE with $f set to the literal pattern, because only command
  arguments went through _expandGlobs. Loop items are expanded per iteration now, so the body runs
  once per matched file with the real path.
- _expandGlobs() falls back to readDir() when a fs has no ls(), so the helper works against either
  shape instead of silently returning the literal pattern.

Verification, live on v4.6.119:
- for+glob: "--- /work/alpha.txt / 3 /work/alpha.txt / --- /work/beta.txt / 1 /work/beta.txt"
  (6 statements, exit 0) — previously one iteration printing the pattern.
- conditionals: "alpha exists", "nope missing", grep + $? intact.
- end-to-end /work/report.sh (loop + glob + >> append + test -f) wrote a correct log:
  "start / file: /work/alpha.txt / 3 /work/alpha.txt / file: /work/beta.txt / 1 /work/beta.txt …".
- unknown command still reports "command not found" and never calls the AI.
- test-shell-script.js 27 -> 35 asserts.

Retracted: I reported `echo x > file` as broken. It was my probe monkey-patching terminal.print,
which bypassed the capture stack the redirect uses. Both paths write correctly:
direct helper -> "direct-call", prompt -> "prompt-call\n".
---
 main.html                          |  6 +++---
 modules/core/module-loader.js      |  2 +-
 modules/ext/critical-ext-bundle.js | 28 ++++++++++++++++++++++------
 modules/ext/shell.js               | 28 ++++++++++++++++++++++------
 modules/test/test-shell-script.js  | 28 +++++++++++++++++++++++++++-
 5 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/main.html b/main.html
index 872e3cf..c386cc2 100644
--- a/main.html
+++ b/main.html
@@ -1349,13 +1349,13 @@
     </script>
 
     <!-- v4.5.63: A2A scratchpad fixes + agent-register arg fix -->
-<script src="modules/core/core-bundle.js?v=v4.6.118"></script>
+<script src="modules/core/core-bundle.js?v=v4.6.119"></script>
 
     <!-- Async dependency-ordered module loader now only initializes terminal and loads optional ext modules -->
-<script src="modules/core/module-loader.js?v=v4.6.118"></script>
+<script src="modules/core/module-loader.js?v=v4.6.119"></script>
 
     <!-- Critical ext bundle loads AFTER webx11 exists so registerModule calls work -->
-<script src="modules/ext/critical-ext-bundle.js?v=v4.6.118"></script>
+<script src="modules/ext/critical-ext-bundle.js?v=v4.6.119"></script>
 
 <script>
 // ═══ ONE TRUE VERSION SOURCE ═══
diff --git a/modules/core/module-loader.js b/modules/core/module-loader.js
index c383f23..18938f7 100644
--- a/modules/core/module-loader.js
+++ b/modules/core/module-loader.js
@@ -4,7 +4,7 @@
     // Core module bundle loaded synchronously in main.html.
     // This loader only handles version cache-busting, splash, terminal init, and optional ext modules.
 
-    const LATEST_VER = 'v4.6.118';
+    const LATEST_VER = 'v4.6.119';
     const CACHE = '?v=' + LATEST_VER;
     // Exposed so ext modules can register their own URL with the same cache-busting version
     // instead of an unversioned path the browser may already have cached (shell.js did).
diff --git a/modules/ext/critical-ext-bundle.js b/modules/ext/critical-ext-bundle.js
index f892617..d241f33 100644
--- a/modules/ext/critical-ext-bundle.js
+++ b/modules/ext/critical-ext-bundle.js
@@ -857,7 +857,12 @@ class ShellModule {
             const dirPath = '/' + rawDir.map(s => String(s).split('/').filter(Boolean).join('/'))
                 .filter(Boolean).join('/');
             let entries = [];
-            try { entries = this.terminal.fs.ls(dirPath) || []; } catch (e) { entries = []; }
+            try {
+                const fsv = this.terminal.fs;
+                // ls() is the canonical lister; readDir() is the fallback so the helper also works
+                // against a bare fake fs (and against any future module that only implements one).
+                entries = (fsv && (fsv.ls ? fsv.ls(dirPath) : null)) || (fsv && fsv.readDir ? fsv.readDir(dirPath) : null) || [];
+            } catch (e) { entries = []; }
             const re = globToRe(namePat);
             const hits = entries.filter(e => re.test(e.name)).map(e => (dirPath === '/' ? '' : dirPath) + '/' + e.name);
             return hits.length ? hits : [a];
@@ -869,7 +874,11 @@ class ShellModule {
     // and the bracket spelling of the same. Anything else is false, never a crash.
     _evalTest(expr, env) {
         const t = this._expandVars(String(expr == null ? '' : expr), env || {})
-            .trim().replace(/^\[/, '').replace(/\]$/, '').trim();
+            .trim()
+            .replace(/^\[\s*/, '')      // `[ -f x ]`
+            .replace(/\s*\]$/, '')
+            .replace(/^test\s+/, '')     // `test -f x` — without this every `if test` was false
+            .trim();
         if (!t) return false;
         const fs = this.terminal.fs;
         const parts = t.split(/\s+/);
@@ -891,10 +900,17 @@ class ShellModule {
         let ran = 0;
         for (const node of nodes) {
             if (node.op === 'for') {
-                for (const item of node.items) {
-                    env[node.var] = this._expandVars(item, env);
-                    const r = await this._runInstructions(node.body, env, ctx);
-                    ran += r.ran + 1;
+                for (const rawItem of node.items) {
+                    const expanded = this._expandVars(rawItem, env);
+                    // `for f in /work/*.txt` used to run ONCE with $f set to the literal pattern,
+                    // because only command arguments went through glob expansion.
+                    const cwd = (this.terminal.fs && this.terminal.fs.currentPath) || [];
+                    const items = this._expandGlobs([expanded], cwd);
+                    for (const item of items) {
+                        env[node.var] = item;
+                        const r = await this._runInstructions(node.body, env, ctx);
+                        ran += r.ran + 1;
+                    }
                 }
                 continue;
             }
diff --git a/modules/ext/shell.js b/modules/ext/shell.js
index ad66e63..5784ef3 100644
--- a/modules/ext/shell.js
+++ b/modules/ext/shell.js
@@ -721,7 +721,12 @@ class ShellModule {
             const dirPath = '/' + rawDir.map(s => String(s).split('/').filter(Boolean).join('/'))
                 .filter(Boolean).join('/');
             let entries = [];
-            try { entries = this.terminal.fs.ls(dirPath) || []; } catch (e) { entries = []; }
+            try {
+                const fsv = this.terminal.fs;
+                // ls() is the canonical lister; readDir() is the fallback so the helper also works
+                // against a bare fake fs (and against any future module that only implements one).
+                entries = (fsv && (fsv.ls ? fsv.ls(dirPath) : null)) || (fsv && fsv.readDir ? fsv.readDir(dirPath) : null) || [];
+            } catch (e) { entries = []; }
             const re = globToRe(namePat);
             const hits = entries.filter(e => re.test(e.name)).map(e => (dirPath === '/' ? '' : dirPath) + '/' + e.name);
             return hits.length ? hits : [a];
@@ -733,7 +738,11 @@ class ShellModule {
     // and the bracket spelling of the same. Anything else is false, never a crash.
     _evalTest(expr, env) {
         const t = this._expandVars(String(expr == null ? '' : expr), env || {})
-            .trim().replace(/^\[/, '').replace(/\]$/, '').trim();
+            .trim()
+            .replace(/^\[\s*/, '')      // `[ -f x ]`
+            .replace(/\s*\]$/, '')
+            .replace(/^test\s+/, '')     // `test -f x` — without this every `if test` was false
+            .trim();
         if (!t) return false;
         const fs = this.terminal.fs;
         const parts = t.split(/\s+/);
@@ -755,10 +764,17 @@ class ShellModule {
         let ran = 0;
         for (const node of nodes) {
             if (node.op === 'for') {
-                for (const item of node.items) {
-                    env[node.var] = this._expandVars(item, env);
-                    const r = await this._runInstructions(node.body, env, ctx);
-                    ran += r.ran + 1;
+                for (const rawItem of node.items) {
+                    const expanded = this._expandVars(rawItem, env);
+                    // `for f in /work/*.txt` used to run ONCE with $f set to the literal pattern,
+                    // because only command arguments went through glob expansion.
+                    const cwd = (this.terminal.fs && this.terminal.fs.currentPath) || [];
+                    const items = this._expandGlobs([expanded], cwd);
+                    for (const item of items) {
+                        env[node.var] = item;
+                        const r = await this._runInstructions(node.body, env, ctx);
+                        ran += r.ran + 1;
+                    }
                 }
                 continue;
             }
diff --git a/modules/test/test-shell-script.js b/modules/test/test-shell-script.js
index 578b186..7b19fa6 100644
--- a/modules/test/test-shell-script.js
+++ b/modules/test/test-shell-script.js
@@ -145,6 +145,32 @@ async function runScript(seed, lines) {
         ok('D10 sequential statements all run', r.printed.join('|') === 'a|b', r.printed.join('|'));
     }
 
-    if (fails === 0) console.log('test-shell-script: ' + passes + ' passed, 0 failed');
+    // D. if-conditions: the `test` keyword and the bracket form (both were false before v4.6.119)
+{
+    const files = { '/work/alpha.txt': 'one\ntwo' };
+    shell.terminal = {
+        fs: {
+            readFile: (p) => (Object.prototype.hasOwnProperty.call(files, p) ? files[p] : null),
+            readDir: (p) => (p === '/work' ? [{ name: 'alpha.txt', type: 'file' }, { name: 'beta.txt', type: 'file' }] : null),
+            currentPath: ['work'],
+        },
+    };
+    ok('D1 "test -f" is true for an existing file', shell._evalTest('test -f /work/alpha.txt', {}) === true);
+    ok('D2 "test -f" is false for a missing file', shell._evalTest('test -f /work/nope.txt', {}) === false);
+    ok('D3 "[ -f … ]" is true for an existing file', shell._evalTest('[ -f /work/alpha.txt ]', {}) === true);
+    ok('D4 "[ -f … ]" is false for a missing file', shell._evalTest('[ -f /work/nope.txt ]', {}) === false);
+    ok('D5 "test -d" is true for a directory', shell._evalTest('test -d /work', {}) === true);
+    ok('D6 "! test -f" inverts', shell._evalTest('! test -f /work/nope.txt', {}) === true);
+}
+
+// E. for-items go through glob expansion (measured live: the loop ran once with $f = the pattern)
+{
+    const src = fsMod.readFileSync(path.join(ROOT, 'ext/shell.js'), 'utf8');
+    ok('E1 the for branch expands globs per item', /_expandGlobs\(\[expanded\], cwd\)/.test(src), 'for-branch expansion missing');
+    ok('E2 _expandGlobs turns *.txt into the real files', shell._expandGlobs(['*.txt'], ['work']).join(',') === '/work/alpha.txt,/work/beta.txt',
+        JSON.stringify(shell._expandGlobs(['*.txt'], ['work'])));
+}
+
+if (fails === 0) console.log('test-shell-script: ' + passes + ' passed, 0 failed');
     else { console.log('test-shell-script: ' + passes + ' passed, ' + fails + ' FAILED'); process.exit(1); }
 })();
-- 
cgit v1.2.3