fix(newos4): four-model review — lazy-load race, restore order, a11y, key handling

Reviewed the shipped change set with kimi-k2.7-code, deepseek-v4-pro, glm-5.3 and minimax-m3.
Findings below are the ones that survived verification against file:line; the rest are in
.hermes/plans/2026-11-07_163420-newos4-six-ui-modes.review-log.md

Accepted and fixed:
- apply() had no generation token: a lazy mode load that finished after the user switched to
  another mode force-applied the abandoned one (`apply(want,{force:true})` in the .then).
  Every apply() now bumps `_loadSeq` and the callback acts only while its token is newest.
  Same token also removes a double mount: registerMode() already applies the mode as it
  registers, so the callback was mounting every lazy mode twice (teardown+cleanup twice).
- _restoreStock() used the recorded `nextSibling` as insert anchor. Every real mode moves
  SEVERAL stock nodes (terminal, input, status bar), so the recorded sibling is inside the
  mode container and every insert fell through to append. Now anchors on the next stock node
  that is already back home. A raw child index also fails (elements removed shift the
  indices) — measured order terminal,input,status-bar,quick-actions.
- _abortToClassic() left the broken mode persisted and `_pending` set, so the next reload
  retried the same failure with no way out. It now clears both and stores 'classic'.
- ?ui= reached classList.add('wx11-ui-' + id) unvalidated: a space or colon throws a
  DOMException and boot dies. Validated against /^[a-z][a-z0-9-]{0,15}$/ and no longer
  persisted (the comment always claimed it was a request, not a preference).
- Modes hide .prompt-label, so #command-input had no accessible name at all. The shell sets
  aria-label once when a mode reparents it.
- cockpit: preventDefault on all F1-F8 killed help, the address bar and devtools. F1/F12 are
  left to the browser and nothing is swallowed when focus is in a text field.
- commander: Ctrl/Cmd+K opened the palette even while typing in the composer. Ignored from
  inside editable elements now.
- picker: role=dialog + aria-modal + aria-labelledby + focus on the first card.
- api.capture(): strip the quoting characters from argv (capturePipeOutput takes argv).
- stale `wx11-ui-*` body class removed on switch instead of accumulating.

Rejected with evidence:
- "loadModuleFromURL may not return a boolean" — modules/core/module-manager.js:56 returns
  true on success and :60 false in the catch. Verified by reading the function.
- "_abortToClassic does not clear _pending" — it did not; that one is accepted above. The
  sibling claim that it "leaves the broken mode persisted" was verified by reading the
  function: it never touched localStorage, so also accepted.
- "mm.LATEST_VER / webx11_cache_ver are never set, so the mode URL falls back to ?v=0" —
  modules/core/module-loader.js:12 sets window.webx11.moduleManager.LATEST_VER and :14-15
  writes webx11_cache_ver. Verified live: the mode fetch carries ?v=v4.6.152.
- "moduleManager.modules may be a Map" — it is a plain object (terminal.moduleManager.modules
  .CryptoGzipModule is the documented access path), so ui-commander's Object.keys works.

Verified: node modules/test/test-ui-shell.js -> 25/25 (the test now moves TWO stock nodes; a
raw-index restore fails it with "terminal-container,input-area,status-bar,quick-actions");
probe-ui-modes.py --cycle against production -> ALL PASS, 13 checks.

1 files changed, 53 insertions, 11 deletions

diff --git a/modules/core/module-uishell.js b/modules/core/module-uishell.js
index ac14b20..9914ec0 100644
--- a/modules/core/module-uishell.js
+++ b/modules/core/module-uishell.js
@@ -18,6 +18,7 @@ class UIShellModule {
         this._root = null;
         this._cleanup = null;
         this._where = null;       // [{ el, parent, next }] captured on first mount
+        this._loadSeq = 0;        // bumps on every apply(): a stale lazy load must never win
         this._terminalEl = null;  // identity of the one real output node
         this._cssIds = [];
         this._pending = null;     // mode requested before its ext module loaded
@@ -49,9 +50,13 @@ class UIShellModule {
         let wanted = localStorage.getItem(this.key) || 'classic';
         try {
             const q = new URLSearchParams(location.search).get('ui');
-            if (q) { wanted = q; this._fromQuery = true; }
+            // Validate before it reaches classList.add('wx11-ui-' + id): a space or a colon in the
+            // query throws a DOMException there and boot dies with no interface at all.
+            if (q && /^[a-z][a-z0-9-]{0,15}$/.test(q)) { wanted = q; this._fromQuery = true; }
         } catch (e) { /* no URLSearchParams: ignore the request */ }
-        const applied = this.apply(wanted, { silent: true, persist: this._fromQuery });
+        // persist:false — a shared link requests a view, it must not rewrite the user's own default
+        // (the code comment already claimed this; the flag did the opposite).
+        const applied = this.apply(wanted, { silent: true, persist: false });
         if (!applied) this._pending = wanted;    // ext modules register ~800 ms later
 
         // A requested/stored mode that never registers (404, parse error) is forgotten rather than
@@ -103,6 +108,9 @@ class UIShellModule {
     apply(id, opts) {
         const o = opts || {};
         const want = id || 'classic';
+        // Every apply supersedes an in-flight lazy load. Without this token a load that finishes
+        // after the user moved on force-applies the abandoned mode (two reviewers, same finding).
+        const token = ++this._loadSeq;
         // A user-initiated switch cancels a pending boot restore, or a slow ext module would later
         // yank the interface back to whatever boot asked for.
         if (!o.silent) { this._pending = null; this._closePicker(); }
@@ -117,12 +125,15 @@ class UIShellModule {
             this._pending = want;
             if (this.terminal) this.toast('Loading ' + want + '…', 1500);
             this._loadMode(want).then((ok) => {
+                // registerMode() applies the mode the moment it registers — which already bumped this
+                // token — and so does a user switch. Only act while this load is still the newest.
+                if (this._loadSeq !== token) return;
                 if (ok) this.apply(want, { force: true, retried: true, persist: o.persist });
                 else if (this.terminal) {
                     this.terminal.print(`UI mode '${want}' could not be loaded — available: ` +
                         (['classic'].concat(Object.keys(this.modes)).join(', ')), 'warning');
                 }
-            });
+            }).catch((e) => this._loadFail(want, 'load rejected: ' + (e && e.message)));
             return false;
         }
         if (want === this.current && !o.force) {
@@ -137,6 +148,11 @@ class UIShellModule {
         this._teardown();
         this.current = want;
         document.body.dataset.uiMode = want;
+        // Drop the previous mode's class: two wx11-ui-* classes at once keep any rule keyed on the
+        // old one matching, and the next switch then has to clean up after two modes.
+        Array.from(document.body.classList)
+            .filter((c) => c.indexOf('wx11-ui-') === 0 && c !== 'wx11-ui-' + want)
+            .forEach((c) => document.body.classList.remove(c));
         document.body.classList.add('wx11-ui-' + want);
         if (o.persist !== false) localStorage.setItem(this.key, want);
 
@@ -162,6 +178,9 @@ class UIShellModule {
             return false;
         }
         this._dockFooter();
+        // Every mode hides .prompt-label, so without this the real input has no accessible name.
+        const inp = document.getElementById('command-input');
+        if (inp && !inp.getAttribute('aria-label')) inp.setAttribute('aria-label', 'Terminal command input');
         document.dispatchEvent(new CustomEvent('wx11:ui-changed', { detail: { mode: want } }));
         return true;
     }
@@ -170,6 +189,10 @@ class UIShellModule {
         if (this.terminal) this.terminal.print(`UI mode '${id}' ${why} — reverted to classic.`, 'error');
         try { if (this._cleanup) this._cleanup(); } catch (e) { /* mode is already broken */ }
         this._cleanup = null;
+        this._pending = null;                       // a sweep or an in-flight load must not re-apply it
+        // Roll the stored preference back: leaving the broken mode persisted means every reload fails
+        // the same way, with no way out except devtools.
+        try { localStorage.setItem(this.key, 'classic'); } catch (e) { /* private mode */ }
         this._restoreStock();
         if (this._root) this._root.innerHTML = '';
         this.current = 'classic';
@@ -248,16 +271,25 @@ class UIShellModule {
     _snapshotStock() {
         if (this._where) return;
         this._terminalEl = document.getElementById('terminal-container');
-        this._where = this._stockEls().map(el => ({ el, parent: el.parentNode, next: el.nextSibling || null }));
+        this._where = this._stockEls().map(el => ({
+            el,
+            parent: el.parentNode,
+            index: Array.prototype.indexOf.call(el.parentNode.children, el),
+        }));
     }
 
     _restoreStock() {
         if (!this._where) return;
-        for (const w of this._where) {
-            if (!w.parent) continue;
-            const ref = w.next && w.next.parentNode === w.parent ? w.next : null;
-            try { w.parent.insertBefore(w.el, ref); } catch (e) { /* detached parent */ }
-        }
+        // Ascending original index, anchored on the next stock node that is ALREADY home. Neither
+        // `nextSibling` (a mode moves several nodes, so the recorded sibling lives inside the mode)
+        // nor a raw child index (removing nodes shifts every later index) reproduces the order —
+        // measured: `body.children[idx]` gave terminal,input,status-bar,quick-actions.
+        const list = this._where.slice().sort((a, b) => a.index - b.index);
+        list.forEach((w) => {
+            if (!w.parent || w.el.parentNode === w.parent) return;     // already in place
+            const anchor = list.find((c) => c.index > w.index && c.el.parentNode === w.parent);
+            try { w.parent.insertBefore(w.el, anchor ? anchor.el : null); } catch (e) { /* detached */ }
+        });
     }
 
     /* ---------- chrome ---------- */
@@ -391,7 +423,9 @@ class UIShellModule {
                 // Do NOT use terminal._parseArgs — it does not exist on the terminal instance; every
                 // panel rendered "unavailable: term._parseArgs is not a function" (measured 2026-11-07).
                 const raw = String(line).trim();
-                const parts = raw.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) || [];
+                // Strip the quoting characters: capturePipeOutput receives argv, not shell words.
+                const parts = (raw.match(/(?:[^\s"']+|"[^"]*"|'[^']*')+/g) || [])
+                    .map((a) => a.replace(/^["']/, '').replace(/["']$/, ''));
                 const cmd = parts[0];
                 if (!cmd) return Promise.resolve('');
                 return term.capturePipeOutput(cmd, parts.slice(1), raw);
@@ -445,15 +479,23 @@ class UIShellModule {
         const self = this;
         const wrap = document.createElement('div');
         wrap.id = 'wx11-ui-picker';
+        // A full-screen overlay that is not announced as a dialog is a keyboard trap in reverse: Tab
+        // walks straight into the page behind it. (Found by review.)
+        wrap.setAttribute('role', 'dialog');
+        wrap.setAttribute('aria-modal', 'true');
+        wrap.setAttribute('aria-labelledby', 'wx11-ui-picker-title');
         wrap.style.cssText = 'position:fixed;inset:0;z-index:9998;background:rgba(0,0,0,.86);' +
             'display:flex;flex-direction:column;justify-content:center;padding:20px;overflow:auto;gap:10px;';
         wrap.innerHTML = '<div style="max-width:720px;margin:0 auto;width:100%">' +
-            '<h2 style="margin:0 0 4px;font-size:1.15em">How do you want newOS4 to look?</h2>' +
+            '<h2 id="wx11-ui-picker-title" style="margin:0 0 4px;font-size:1.15em">How do you want newOS4 to look?</h2>' +
             '<p class="wx11-dim" style="margin:0 0 14px">Pick one — you can switch any time with <code>ui</code>.</p>' +
             '<div id="wx11-ui-picker-list" style="display:grid;gap:10px"></div>' +
             '<button class="wx11-btn" id="wx11-ui-picker-skip" style="margin-top:12px;width:100%;text-align:center">' +
             'Keep the classic terminal</button></div>';
         document.body.appendChild(wrap);
+        // Focus the first card so Escape/Tab/Enter work without a pointer.
+        const firstCard = wrap.querySelector('#wx11-ui-picker-list button');
+        if (firstCard && firstCard.focus) firstCard.focus();
         const list = wrap.querySelector('#wx11-ui-picker-list');
         // Fewer than 4 registered means the ext modules are still in flight — use the static list so
         // the user never sees a one-card chooser (the card handler re-checks and warns if missing).