Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions electron/bridges/ai/ptyExec.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ function startPtyJob(ptyStream, command, options) {
expectedPrompt,
typedInput = false,
echoCommand,
onEchoSuppressionPrime,
maxBufferedChars = 0,
normalizeFinalOutput = true,
enforceWallTimeout = false,
Expand Down Expand Up @@ -812,6 +813,21 @@ function startPtyJob(ptyStream, command, options) {
writeInput(`${buildPendingInputClearPrefix(resolvedShellKind)}${wrapped}`);
}

// Prime the renderer's display suppression before the first byte is typed
// (issue #3384). Shells whose line editor echoes input, such as BusyBox ash
// on OpenWrt, break long echoed lines at the terminal width with CR/LF; the
// wrapped fragments no longer contain the marker and would leak through the
// per-line echo filter as visible "variables" until the wrapper's own
// _I printf runs. Delivering the _I line over the data channel up front
// suppresses the whole echo; the _S output releases it, and finish() sends
// the _R reset (onProbeAborted) when the command never starts.
if (typeof onEchoSuppressionPrime === "function") {
try {
onEchoSuppressionPrime(marker);
} catch {
// Display suppression must never prevent the command from starting.
}
}
if (probingShell) {
writeInput(`${buildPendingInputClearPrefix(resolvedShellKind)}${buildLiveShellProbe(marker)}`);
} else {
Expand Down
41 changes: 41 additions & 0 deletions electron/bridges/ai/ptyExec.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1420,3 +1420,44 @@ test("startPtyJob types the wrapper one code point per write for strict bastions
assert.equal(result.ok, false);
assert.equal(result.error, "Cancelled");
});

test("echo suppression prime is delivered before the first typed input (#3384)", async () => {
for (const probeLiveShell of [false, true]) {
const pty = new EventEmitter();
const events = [];
pty.write = (data) => {
if (data === "\x03") return;
events.push(`write:${data}`);
};
const job = startPtyJob(pty, "echo prime-order", {
shellKind: "posix",
probeLiveShell,
timeoutMs: 1000,
onEchoSuppressionPrime: (marker) => events.push(`prime:${marker}`),
});
assert.equal(events[0], `prime:${job.marker}`, "prime precedes every typed byte");
assert.ok((events[1] || "").startsWith("write:"), "first write follows the prime");
assert.equal(events.filter((event) => event.startsWith("prime:")).length, 1);
if (probeLiveShell) {
pty.emit("data", `${job.marker}_P:sh\n${job.marker}_Q`);
}
pty.emit("data", `${job.marker}_S\ndone\n${job.marker}_E:0\n`);
const result = await job.resultPromise;
assert.equal(result.ok, true, JSON.stringify(result));
}
});

test("a throwing echo suppression prime callback still types the command (#3384)", () => {
const writes = [];
const pty = new EventEmitter();
pty.write = (data) => writes.push(String(data));
const job = startPtyJob(pty, "echo resilient", {
shellKind: "posix",
timeoutMs: 1000,
onEchoSuppressionPrime: () => {
throw new Error("Renderer closed");
},
});
assert.ok(writes.some((data) => data.includes("echo resilient")));
job.cancel();
});
7 changes: 6 additions & 1 deletion electron/bridges/aiBridge/cattyExecHandlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Module-level require on purpose: code inside registerCattyExecHandlers
// runs under `with (ctx)` where bare `require` resolves to ctx.require
// (based in electron/bridges/). Requiring here keeps the path unambiguous.
const { emitTerminalSessionData } = require("../emitTerminalSessionData.cjs");
const { formatSyntheticEcho } = require("../ai/shellUtils.cjs");
const { remoteDisallowsExecChannelProbe, ensureSessionShellKindForExec } = require("../ai/sessionShellKind.cjs");

Expand Down Expand Up @@ -180,7 +181,11 @@ function registerCattyExecHandlers(ctx) {
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
onProbeAborted: (marker) => {
const contents = electronModule?.webContents?.fromId?.(session.webContentsId);
safeSend(contents, "netcatty:data", { sessionId, data: `${marker}_R\n` });
emitTerminalSessionData(contents, sessionId, `${marker}_R\n`, { session });
},
onEchoSuppressionPrime: (marker) => {
const contents = electronModule?.webContents?.fromId?.(session.webContentsId);
emitTerminalSessionData(contents, sessionId, `${marker}_I\n`, { session });
},
chatSessionId,
expectedPrompt: getFreshIdlePrompt(session),
Expand Down
65 changes: 65 additions & 0 deletions electron/bridges/aiEchoSuppressionOrdering.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
const assert = require('node:assert/strict');
const { EventEmitter } = require('node:events');
const test = require('node:test');
const { configureTerminalSessionDataEmitter, emitTerminalSessionData } = require('./emitTerminalSessionData.cjs');
const { registerCattyExecHandlers } = require('./aiBridge/cattyExecHandlers.cjs');
const { execViaPty } = require('./ai/ptyExec.cjs');
const bridge = require('./mcpServerBridge.cjs');

for (const surface of ['catty', 'mcp-exec', 'mcp-job']) {
for (const hasPort of [true, false]) {
test(`${surface} primes and resets on the PTY output transport (port=${hasPort})`, async (t) => {
const deliveries = [];
const pty = new EventEmitter();
const session = { protocol: 'local', shellKind: 'posix', pty, webContentsId: 7 };
const contents = { isDestroyed: () => false, send: (_channel, payload) => {
if (!payload.syntheticEcho) deliveries.push(['ipc', payload.data]);
} };
const sessions = new Map([['session', session]]);
configureTerminalSessionDataEmitter({ getSession: id => sessions.get(id), outputChannel: {
send: (_id, data) => {
if (!hasPort) return false;
deliveries.push(['port', data]);
return true;
},
} });
t.after(() => { bridge.cleanup(); configureTerminalSessionDataEmitter(); });
const transport = hasPort ? 'port' : 'ipc';
let writes = 0;
pty.write = data => {
if (data === '\x03') return;
if (writes++ === 0) {
assert.equal(deliveries.length, 1, 'prime arrives before any typed bytes');
assert.equal(deliveries[0][0], transport);
assert.match(deliveries[0][1], /__NCMCP_.*_I\r?\n$/);
emitTerminalSessionData(contents, 'session', 'wrapped echo\r\n', { session });
}
};
const electronModule = { webContents: { fromId: () => contents } };
bridge.init({ sessions, electronModule });
bridge.setPermissionMode('auto');
bridge.updateSessionMetadata([{ sessionId: 'session', protocol: 'local', connected: true }], 'chat');
let pending;
if (surface === 'catty') {
const handlers = new Map();
registerCattyExecHandlers({ ipcMain: { handle: (key, fn) => handlers.set(key, fn) },
validateSender: () => true, sessions, mcpServerBridge: bridge, electronModule,
safeSend: (target, ...args) => target.send(...args), execViaPty, getFreshIdlePrompt: () => '' });
pending = handlers.get('netcatty:ai:exec')({ sender: contents }, { sessionId: 'session', command: 'echo test', chatSessionId: 'chat' });
} else {
pending = bridge.dispatchBuiltinRpc(surface === 'mcp-job' ? 'netcatty/jobStart' : 'netcatty/exec', {
sessionId: 'session', command: 'echo test', chatSessionId: 'chat',
});
}
await new Promise(resolve => setTimeout(resolve, 30));
assert.ok(writes > 0, 'execution must reach the PTY');
bridge.cancelAllPtyExecs();
pty.emit("close");
await pending;
assert.equal(deliveries[1][0], transport);
assert.equal(deliveries[1][1], 'wrapped echo\r\n');
assert.equal(deliveries.at(-1)[0], transport);
assert.match(deliveries.at(-1)[1], /__NCMCP_.*_R\r?\n$/);
});
}
}
5 changes: 5 additions & 0 deletions electron/bridges/mcpServerBridge.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const {
} = require("./ai/commandSafety.cjs");
const { execViaPty, startPtyJob, execViaChannel, execViaRawPty } = require("./ai/ptyExec.cjs");
const { safeSend } = require("./ipcUtils.cjs");
const { emitTerminalSessionData } = require("./emitTerminalSessionData.cjs");
const { getCliDiscoveryFilePath } = require("../cli/discoveryPath.cjs");
const { EXTERNAL_MCP_CHAT_SESSION_ID } = require("../cli/externalMcpDiscoveryPath.cjs");
const sftpBridge = require("./sftpBridge.cjs");
Expand Down Expand Up @@ -487,6 +488,10 @@ function shutdownHost({ preserveScopedMetadata = false } = {}) {
function echoCommandToSession(session, sessionId, command, { syntheticEcho = true } = {}) {
if (!electronModule || !session?.webContentsId || !command) return;
const contents = electronModule.webContents?.fromId?.(session.webContentsId);
if (!syntheticEcho) {
emitTerminalSessionData(contents, sessionId, formatSyntheticEcho(command), { session });
return;
}
safeSend(contents, "netcatty:data", {
sessionId,
data: formatSyntheticEcho(command),
Expand Down
2 changes: 2 additions & 0 deletions electron/bridges/mcpServerBridge/execHandlers.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ function createExecHandlerApi(ctx) {
probeLiveShell: true,
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
onProbeAborted: (marker) => echoCommandToSession(session, sessionId, `${marker}_R`, { syntheticEcho: false }),
onEchoSuppressionPrime: (marker) => echoCommandToSession(session, sessionId, `${marker}_I`, { syntheticEcho: false }),
expectedPrompt: getFreshIdlePrompt(session),
typedInput: true,
echoCommand: (rawCommand) => echoCommandToSession(session, sessionId, rawCommand),
Expand Down Expand Up @@ -319,6 +320,7 @@ function createExecHandlerApi(ctx) {
probeLiveShell: true,
bastionKeystrokes: remoteDisallowsExecChannelProbe(session.remoteSshVersion),
onProbeAborted: (marker) => echoCommandToSession(session, sessionId, `${marker}_R`, { syntheticEcho: false }),
onEchoSuppressionPrime: (marker) => echoCommandToSession(session, sessionId, `${marker}_I`, { syntheticEcho: false }),
chatSessionId,
expectedPrompt: getFreshIdlePrompt(session),
typedInput: true,
Expand Down
86 changes: 86 additions & 0 deletions electron/preloadDataBacklog.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,34 @@ test("OpenWrt bounded wrapper continuations stay hidden across fragmented echoes
}
});

test("primed suppression hides BusyBox ash echo wrapped mid-marker (#3384)", () => {
const preload = loadPreloadWithFakeElectron();
try {
const received = [];
const sessionId = 'ash-wrap';
const marker = '__NCMCP_mttikd5b_ccbc892e865a115a80c88afdc77b96a6__';
preload.api.onSessionData(sessionId, chunk => received.push(chunk));
// The exec bridge primes display suppression over the data channel before
// the wrapper is typed, mirroring onEchoSuppressionPrime in ptyExec.cjs.
preload.handlers.get('netcatty:data')({}, { sessionId, data: `${marker}_I\n` });
// BusyBox ash's line editor breaks the echoed first wrapper line at the
// terminal width, so the second PTY line carries no complete __NCMCP_
// marker and the per-line echo filter alone cannot drop it.
const firstLine = ` ${marker}=0; printf '\\n%s\\n' '${marker}_I'`;
const secondLine = ` : '${marker}'; ${marker}_cmd='echo visible-output'; \\`;
const echo = `${firstLine.slice(0, 55)}\r\n${firstLine.slice(55)}\r\n`
+ `${secondLine.slice(0, 70)}\r\n${secondLine.slice(70)}\r\n`
+ `> : '${marker}'; printf '%s\\n' '${marker}_S'\r\n`;
const data = `${echo}\n${marker}_S\r\nvisible-output\r\n${marker}_E:0\r\n`;
for (let offset = 0; offset < data.length; offset += 7) {
preload.handlers.get('netcatty:data')({}, { sessionId, data: data.slice(offset, offset + 7) });
}
assert.equal(received.join(''), 'visible-output\r\n');
} finally {
preload.cleanup();
}
});


test("ordinary text resembling an OpenWrt continuation is released", async () => {
const preload = loadPreloadWithFakeElectron();
Expand Down Expand Up @@ -1873,3 +1901,61 @@ test("aborted input releases custom prompts and ignores a late input marker", ()
assert.equal(received.join(""), "normal output\nlate prompt\n");
} finally { preload.cleanup(); }
});

test('real OpenWrt ash hides wrapped input and preserves output with priming (#3384)', {
skip: !process.env.NETCATTY_OPENWRT_SSH_PORT,
timeout: 60000,
}, async () => {
const { Client } = require('ssh2');
const { execViaPty } = require('./bridges/ai/ptyExec.cjs');
const client = new Client();
await new Promise((resolve, reject) => client.once('ready', resolve).once('error', reject).connect({
host: '127.0.0.1', port: Number(process.env.NETCATTY_OPENWRT_SSH_PORT), username: 'root', password: '',
}));
try {
for (const cols of [80, 120]) {
for (const probeLiveShell of [false, true]) {
const preload = loadPreloadWithFakeElectron();
const stream = await new Promise((resolve, reject) => client.shell({ term: 'xterm', cols, rows: 30 },
(error, value) => error ? reject(error) : resolve(value)));
try {
await new Promise((resolve, reject) => {
let output = '';
const timer = setTimeout(() => reject(new Error('OpenWrt prompt missing')), 5000);
const onData = data => {
output += data;
if (output.includes(':~# ')) {
clearTimeout(timer);
stream.removeListener('data', onData);
resolve();
}
};
stream.on('data', onData);
});
const received = [];
const sessionId = `openwrt-${cols}-${probeLiveShell}`;
preload.api.onSessionData(sessionId, chunk => received.push(chunk));
const deliver = data => preload.handlers.get('netcatty:data')({}, { sessionId, data: String(data) });
stream.on('data', deliver);
const result = await execViaPty(stream, "printf 'visible-output\\n'", {
shellKind: 'posix', probeLiveShell, typedInput: true, timeoutMs: 5000,
onEchoSuppressionPrime: marker => deliver(`${marker}_I\n`),
onProbeAborted: marker => deliver(`${marker}_R\n`),
});
await sleep(150);
assert.equal(result.ok, true, JSON.stringify(result));
assert.equal(result.stdout.trim(), 'visible-output');
const display = received.join('');
assert.match(display, /visible-output/);
assert.doesNotMatch(display, /__nc_|__NCMCP_|printf|eval|unset|_cmd=|_d=/,
JSON.stringify({ cols, probeLiveShell, display }));
} finally {
stream.close();
preload.cleanup();
}
}
}
} finally {
client.end();
}
});
12 changes: 12 additions & 0 deletions electron/terminalWorker/aiExec.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,12 @@ function createWorkerAiExecHandler({
data: `${marker}_R\n`,
});
},
onEchoSuppressionPrime: (marker) => {
event?.sender?.send?.("netcatty:data", {
sessionId,
data: `${marker}_I\n`,
});
},
chatSessionId,
expectedPrompt: getFreshIdlePrompt(session),
typedInput: true,
Expand Down Expand Up @@ -502,6 +508,12 @@ function createWorkerAiJobStartHandler({
data: `${marker}_R\n`,
});
},
onEchoSuppressionPrime: (marker) => {
event?.sender?.send?.("netcatty:data", {
sessionId,
data: `${marker}_I\n`,
});
},
chatSessionId,
expectedPrompt: getFreshIdlePrompt(session),
typedInput: true,
Expand Down
10 changes: 8 additions & 2 deletions electron/terminalWorker/aiExec.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,7 @@ test("worker AI background jobs start, poll, stop, and block overlapping exec",
assert.equal(started.command, "npm test");
assert.equal(started.status, "running");
assert.equal(started.outputMode, "foreground-mirrored");
const marker = await extractMarker(pty.writes);
assert.deepEqual(event.rendererMessages, [
{
channel: "netcatty:data",
Expand All @@ -154,9 +155,14 @@ test("worker AI background jobs start, poll, stop, and block overlapping exec",
syntheticEcho: true,
},
},
{
channel: "netcatty:data",
payload: {
sessionId: "ssh-1",
data: `${marker}_I\n`,
},
},
]);

const marker = await extractMarker(pty.writes);
pty.emit("data", `${marker}_S\r\nready\r\n`);
await nextTick();

Expand Down
Loading