fix: merge-time follow-ups for #400, #401, #362 and #388

Each item is from the pre-merge review of the PR it names, applied on master
rather than by pushing to a contributor branch.

#400 (response viewer, shenlvkang-collab)
- The brief view opened at `scrollTop = 0`, right when it was a single card
  holding the last row. Now that it renders the whole turn, the top is the
  turn's first narration line and the answer can be screens below it, while
  loadFullContext already scrolls to the bottom of the same turn. A multi-row
  turn now opens at its newest text; a single card still opens at the top.

#401 (loopback links as web tabs, shenlvkang-collab)
- Drop `*.localhost` from the auto-route set. Every other member is an address
  literal that can only mean this box; a `*.localhost` DNS name is not one, and
  a resolver with a search domain retries `evil.localhost` as
  `evil.localhost.<search domain>`. The link source is agent-written terminal
  output, so that set is the whole confinement on a tap that makes Codeman
  fetch a URL server-side and persist it. The page-side test stays broader
  (`isOnBoxHostname`), where a false positive only declines to proxy.
- A link to the origin root navigated nothing: the path was flattened to '',
  which openWebview reads as "no deep link", leaving an open frame where it was.
- `this.webviews` being set does not mean it is loaded. initWebviews() assigns a
  truthy empty map and only then awaits the list, so a tap during page load
  found nothing to reuse and POSTed a duplicate record. Join the in-flight
  refresh instead.
- One dashboard per dev server rather than per host spelling, which is what the
  method's own comment already promised.
- Toast on the auto-create: it writes webviews.json, broadcasts over SSE and
  adds a Run-dropdown row on every signed-in device, with a new tab as its only
  previous signal.

#362 (remote omp continuation, timkjr)
- Accept the allowlisted `mode === 'omp'` arm as-is; a blanket registry render
  would hand deepseek a locally-resolved --profile and bypass claude's own
  overlay. A registry-declared switch is the follow-up if a third mode needs it.
- Revert the whole-file Prettier reformat of docs/remote-sessions.md (docs/ is
  hand-formatted and outside `npm run format`), keeping only the two new
  sections.
- Correct three stale passages: architecture-invariants' `exec claude
  --dangerously-skip-permissions`, the `exec <cli>` paragraph (claude and omp
  now have their own arms, and the claude pane's PID is the login shell), and
  omp-integration's `-c 'omp'`. RemoteCommandMode gains deepseek and omp.
- Add the missing `_maybeCaptureOmpSessionId` remote-guard test; the sibling
  guard in `_pinOmpRespawnId` had one and this path runs earlier, on the first
  idle turn.

#388 (keyCode 229 recovery, aakhter)
- Gate notifyCanonicalData on shouldSuppressTerminalQueryResponse and
  isTerminalFocusOrMouseReport. onData also carries the DA/DSR/CPR/OSC replies
  xterm answers during Ink redraws and its SGR mouse and focus reports; any of
  those landing between the keydown and the candidate's resolution was read as
  "xterm spoke for this keystroke", standing the recovery down and leaving the
  character dropped, worst on a busy agent pane. Reached through
  window.CodemanTerminalInput: the predicates live in a module IIFE that closes
  long before this call site, so bare references would throw into the
  surrounding try/catch and stop the notify from ever running.

Every fix has a test that fails without it (verified by reverting each).
Full gate green on the combined tree: 358 files, 6849 tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
2026-09-12 05:15:41 +02:00
parent 9d664ffe01
commit 02b0e27898
11 changed files with 368 additions and 51 deletions
+6 -1
View File
@@ -2398,7 +2398,12 @@ class CodemanApp {
viewer.classList.add('visible');
backdrop.classList.add('visible');
body.scrollTop = 0;
// A multi-row turn opens at its NEWEST text, matching loadFullContext's
// "scroll to bottom (latest message)". `scrollTop = 0` was right when the
// brief view was a single card holding the last row; with the whole turn
// rendered, the top is the turn's first narration line and the answer the
// eye button exists to show can be several screens down.
body.scrollTop = turnMessages.length > 1 ? body.scrollHeight : 0;
} catch (err) {
console.error('Failed to load response:', err);
}
+14 -1
View File
@@ -1400,8 +1400,21 @@ Object.assign(CodemanApp.prototype, {
this.terminal.onData((data) => {
// Canonical xterm data. Telling the controller is what lets it know a
// keystroke was already delivered and needs no recovery.
//
// ⚠️ onData ALSO fires for output xterm produces on its own initiative:
// the DA/DSR/CPR/OSC replies it answers during Ink redraws, and the SGR
// mouse and focus reports (see the two predicates above, used for exactly
// this question at the send sites). Any one of those landing between the
// keydown and the candidate's zero-delay resolution would be read as
// "xterm spoke for this keystroke", standing the recovery down and
// leaving the character dropped, worst on a busy agent pane, which is
// the case this exists for. Narrowing the counter cannot cause a
// duplicate: it only ever makes the controller less sure it can stand down.
try {
this._keyCode229Recovery?.notifyCanonicalData?.();
const input = window.CodemanTerminalInput;
if (!input?.shouldSuppressTerminalQueryResponse(data) && !input?.isTerminalFocusOrMouseReport(data)) {
this._keyCode229Recovery?.notifyCanonicalData?.();
}
} catch {
// Bookkeeping must never block real input.
}
+72 -14
View File
@@ -30,18 +30,53 @@
const LOOPBACK_HOSTNAMES = new Set(['localhost', '0.0.0.0', '::1', '[::1]', '::', '[::]']);
/** `localhost`, `*.localhost`, 127.0.0.0/8, 0.0.0.0 and the IPv6 loopback forms. */
function isLoopbackHostname(hostname) {
const host = String(hostname || '')
function normalizeHostname(hostname) {
return String(hostname || '')
.trim()
.toLowerCase()
.replace(/\.$/, '');
}
/**
* `localhost`, 127.0.0.0/8, 0.0.0.0 and the IPv6 loopback forms: names that can
* only ever mean this box.
*
* ⚠️ `*.localhost` is deliberately NOT here. The link source is agent-written
* terminal output and response-viewer markdown, i.e. prompt-injectable, and
* this set is the whole confinement on a tap that makes Codeman fetch a URL
* server-side and persist it. Every other member is an address literal; a
* `*.localhost` DNS name is not one: on a resolver that does not synthesise it
* locally and has a search domain configured, `evil.localhost` NXDOMAINs as
* absolute and is retried as `evil.localhost.<search domain>`, which an
* attacker can control. A user who really runs `api.localhost` dev hosts can
* still save that dashboard by hand, which is an explicit action.
*/
function isLoopbackHostname(hostname) {
const host = normalizeHostname(hostname);
if (!host) return false;
if (LOOPBACK_HOSTNAMES.has(host) || host.endsWith('.localhost')) return true;
if (LOOPBACK_HOSTNAMES.has(host)) return true;
const ipv4 = /^(\d{1,3})\.\d{1,3}\.\d{1,3}\.\d{1,3}$/.exec(host);
return !!ipv4 && Number(ipv4[1]) === 127;
}
/**
* Whether the PAGE is being viewed on the box itself. Broader than the
* auto-route set on purpose, and safe in the opposite direction: a false
* positive here only ever DECLINES to proxy, leaving the caller's direct open.
*/
function isOnBoxHostname(hostname) {
const host = normalizeHostname(hostname);
return isLoopbackHostname(host) || host.endsWith('.localhost');
}
/**
* One key per dev server, so `localhost:5173` and `127.0.0.1:5173` reuse a
* single saved dashboard and a single tab instead of one per host spelling.
*/
function webTabOriginKey(url) {
return isLoopbackHostname(url.hostname) ? `${url.protocol}//loopback:${url.port}` : url.origin;
}
/**
* Whether a link should open through a proxied web tab rather than directly:
* an http(s) URL on a loopback host, viewed from a page that is NOT itself on
@@ -57,11 +92,11 @@ function linkNeedsWebTabProxy(rawUrl, pageHostname) {
}
if (url.protocol !== 'http:' && url.protocol !== 'https:') return false;
if (!isLoopbackHostname(url.hostname)) return false;
return !isLoopbackHostname(pageHostname);
return !isOnBoxHostname(pageHostname);
}
if (typeof window !== 'undefined') {
window.CodemanWebviewLinks = { isLoopbackHostname, linkNeedsWebTabProxy };
window.CodemanWebviewLinks = { isLoopbackHostname, isOnBoxHostname, webTabOriginKey, linkNeedsWebTabProxy };
}
Object.assign(CodemanApp.prototype, {
@@ -91,17 +126,24 @@ Object.assign(CodemanApp.prototype, {
} catch {
return;
}
if (!this.webviews) await this.refreshWebviews();
// ⚠️ "is `this.webviews` set" does NOT answer "is it loaded": initWebviews()
// assigns a truthy EMPTY map synchronously and only then awaits the list, so
// a tap during page load used to find nothing to reuse and POST a duplicate
// record for an origin that already exists server-side. Join an in-flight
// load; start one only when none has ever run.
if (this._webviewsRefresh) await this._webviewsRefresh;
else if (!this._webviewsLoaded) await this.refreshWebviews();
if (!this.webviews) {
this.showToast?.('Could not open URL', 'error');
return;
}
const wantedKey = webTabOriginKey(url);
let existing = null;
for (const webview of this.webviews.values()) {
if (webview.managed || (webview.embedMode ?? 'proxy') !== 'proxy') continue;
try {
if (new URL(webview.url).origin === url.origin) {
if (webTabOriginKey(new URL(webview.url)) === wantedKey) {
existing = webview;
break;
}
@@ -122,9 +164,17 @@ Object.assign(CodemanApp.prototype, {
}
await this.refreshWebviews();
id = created.id;
// The create is a persisted record: it writes webviews.json, broadcasts
// over SSE, adds a Run-dropdown row on every device this owner is signed
// in on and counts toward MAX_WEBVIEWS. Adding one by hand goes through a
// modal; a tap should not do all that with a new tab as its only signal.
this.showToast?.(`Saved ${url.host} as a web tab`, 'success');
}
// `/` is passed through rather than flattened to '': openWebview reads an
// empty path as "no deep link" and leaves an already-open frame on whatever
// page it was showing, so a link to the origin root did nothing visible.
const path = `${url.pathname}${url.search}${url.hash}`;
await this.openWebview(id, { path: path === '/' ? '' : path });
await this.openWebview(id, { path: path || '/' });
},
// ── State ─────────────────────────────────────────────────────────────────
@@ -152,11 +202,19 @@ Object.assign(CodemanApp.prototype, {
},
async refreshWebviews() {
const data = await this._apiJson('/api/webviews');
if (!data) return;
this.webviews = new Map((data.webviews || []).map((w) => [w.id, w]));
if (typeof data.maxLiveFrames === 'number') this._webviewMaxFrames = data.maxLiveFrames;
this.renderWebviewMenuItems();
const inFlight = this._apiJson('/api/webviews').then((data) => {
if (!data) return;
this.webviews = new Map((data.webviews || []).map((w) => [w.id, w]));
if (typeof data.maxLiveFrames === 'number') this._webviewMaxFrames = data.maxLiveFrames;
this._webviewsLoaded = true;
this.renderWebviewMenuItems();
});
this._webviewsRefresh = inFlight;
try {
await inFlight;
} finally {
if (this._webviewsRefresh === inFlight) this._webviewsRefresh = null;
}
},
/** SSE: the saved list changed (possibly on another device). */