fix: address Codex review on PR #3683

electron/bridges/ai/ptyExec.test.cjs

@@ -967,6 +967,124 @@ test("posix wrapper keeps cd contained in the subshell (documented trade-off)",
   }
 });
 
+// Issue #3485: interactive bash 4.4+ expands PS0 after reading a command and
+// before executing it, so a user-defined PS0 hook (commonly a timestamp from
+// ~/.bashrc; root usually has none) prints extra output inside the hidden
+// wrapper's echo window. The wrapper must silence it for its own reads and
+// restore the original value afterwards.
+test("posix wrapper saves PS0 before wrapping and restores it after execution", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo PS0_HOOK_TEST", "posix", marker);
+  assert.match(wrapped, /\[ -n "\$\{PS0-\}" \]&&\{ __NCMCP_PS0_VAL=\$\{PS0-\}; __NCMCP_PS0_OWN=1; PS0=; \}; printf/);
+  assert.match(
+    wrapped,
+    /\[ "\$\{__NCMCP_PS0_OWN-\}" = 1 \]&&\{ PS0=\$\{__NCMCP_PS0_VAL-\}; unset __NCMCP_PS0_OWN __NCMCP_PS0_VAL; \}; \(exit \$__NCMCP_rc\); \}/,
+  );
+
+  const result = spawnSync(
+    "bash",
+    ["--norc", "-c", 'PS0="[hook] "\n' + wrapped + 'printf "RESTORED_PS0=%s\\n" "$PS0"'],
+    { encoding: "utf8" },
+  );
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  assert.match(result.stdout, /PS0_HOOK_TEST/);
+  // The user's PS0 value is back once the wrapper group finishes, so their
+  // own subsequent commands still run the hook.
+  assert.match(result.stdout, /RESTORED_PS0=\[hook\] /);
+});
+
+// Codex review: a session whose bashrc defines its own `__NCMCP_PS0` (even a
+// readonly one) must keep that variable intact — the wrapper's scratch state
+// is namespaced and gated on its own ownership flag, so a pre-existing
+// `__NCMCP_PS0` is neither overwritten on save nor mistaken for saved wrapper
+// state (assigned to PS0 and deleted) on restore.
+test("posix wrapper does not clobber a user-defined __NCMCP_PS0 variable", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo USER_VAR_TEST", "posix", marker);
+  const result = spawnSync(
+    "bash",
+    ["--norc", "-c", '__NCMCP_PS0="user-data"\n' + wrapped + 'printf "USER_V=[%s]\\n" "${__NCMCP_PS0-}"'],
+    { encoding: "utf8" },
+  );
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  assert.match(result.stdout, /USER_VAR_TEST/);
+  const userVarLine = result.stdout.split("\n").find((line) => line.startsWith("USER_V="));
+  assert.equal(userVarLine, "USER_V=[user-data]");
+});
+
+test("posix wrapper survives a readonly user __NCMCP_PS0 without restore errors", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo READONLY_USER_TEST", "posix", marker);
+  const result = spawnSync(
+    "bash",
+    ["--norc", "-c", 'PS0="[hook] "\nreadonly __NCMCP_PS0="pinned"\n' + wrapped + 'printf "PIN=[%s] PS0=[%s]\\n" "${__NCMCP_PS0-}" "${PS0-}"'],
+    { encoding: "utf8" },
+  );
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  assert.match(result.stdout, /READONLY_USER_TEST/);
+  assert.equal(result.stderr, "");
+  const pinLine = result.stdout.split("\n").find((line) => line.startsWith("PIN="));
+  assert.equal(pinLine, "PIN=[pinned] PS0=[[hook] ]");
+});
+
+// An interrupted earlier run leaves its ownership flag and saved value behind
+// with PS0 cleared; the next wrapper run must restore the previously saved
+// value and clear the bookkeeping instead of leaving PS0 silenced forever.
+test("posix wrapper heals PS0 state left behind by an interrupted earlier run", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo HEAL_TEST", "posix", marker);
+  const result = spawnSync(
+    "bash",
+    ["--norc", "-c", 'unset PS0\n__NCMCP_PS0_VAL="[hook] "\n__NCMCP_PS0_OWN=1\n' + wrapped + 'printf "HEALED=[%s] OWN=[%s]\\n" "${PS0-}" "${__NCMCP_PS0_OWN-}"'],
+    { encoding: "utf8" },
+  );
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  assert.match(result.stdout, /HEAL_TEST/);
+  const healedLine = result.stdout.split("\n").find((line) => line.startsWith("HEALED="));
+  assert.equal(healedLine, "HEALED=[[hook] ] OWN=[]");
+});
+
+test("posix wrapper leaves PS0 alone when the user has none set", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo NO_PS0_TEST", "posix", marker);
+  const result = spawnSync(
+    "bash",
+    ["--norc", "-c", `unset PS0\n${wrapped}printf "PS0_SET=[%s]\\n" "\${PS0-}"`],
+    { encoding: "utf8" },
+  );
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  // Nothing was saved and nothing is restored: PS0 stays unset afterwards.
+  assert.match(result.stdout, /NO_PS0_TEST/);
+  const ps0Line = result.stdout.split("\n").find((line) => line.startsWith("PS0_SET="));
+  assert.equal(ps0Line, "PS0_SET=[]");
+});
+
+test("posix wrapper silencing still works under sh (dash) where PS0 does not exist", () => {
+  const marker = "__NCMCP_TEST__";
+  const wrapped = buildWrappedCommand("echo DASH_TEST", "posix", marker);
+  const result = spawnSync("sh", ["-c", `${wrapped}printf 'SH_ALIVE\\n'`], { encoding: "utf8" });
+
+  assert.equal(result.error, undefined);
+  assert.equal(result.status, 0);
+  assert.match(result.stdout, new RegExp(`${marker}_E:0`));
+  assert.match(result.stdout, /SH_ALIVE/);
+});
+
 // Issue #3265: with plain HISTCONTROL the wrapper line is recorded in bash
 // history; arrow-up recall then redraws the marker text, the preload __NCMCP_
 // filter suppresses the redraw fragments containing it, and readline's row

electron/bridges/ai/ptyExecHelpers.cjs

@@ -274,16 +274,32 @@ function buildPosixWrapperBody(command, marker, startFormat, { omitBashHistoryCl
     cmdAssign = `${marker}_cmd=$(${writes.join(`; \\\n: '${marker}'; `)})`;
   }
   const historyCleanup = omitBashHistoryCleanup ? [] : [buildBashHistoryCleanup(marker)];
+  // Issue #3485: interactive bash 4.4+ expands PS0 after reading a command
+  // and before executing it. A user-set PS0 hook (commonly a pre-exec
+  // timestamp/hook defined in ~/.bashrc — root usually has none) prints extra
+  // output inside the hidden wrapper's echo window, garbling the agent's
+  // command echo while manual typing renders natively. Save PS0 only while it
+  // is currently set (so an interrupted run cannot overwrite the saved value
+  // with empty), silence it for every wrapper read, and restore it after the
+  // wrapper group finishes. zsh/dash/fish have no PS0, so both guards are
+  // no-ops there.
+  // Ownership bookkeeping instead of a bare scratch value: a session that
+  // already defines its own `__NCMCP_PS0*` variables must never have them
+  // read into PS0, overwritten, or deleted, and the restore below only fires
+  // when this wrapper (or an interrupted earlier wrapper, which the next run
+  // then heals) actually saved a value.
+  const ps0Save = '[ -n "${PS0-}" ]&&{ __NCMCP_PS0_VAL=${PS0-}; __NCMCP_PS0_OWN=1; PS0=; }';
+  const ps0Restore = '[ "${__NCMCP_PS0_OWN-}" = 1 ]&&{ PS0=${__NCMCP_PS0_VAL-}; unset __NCMCP_PS0_OWN __NCMCP_PS0_VAL; }';
   // OpenWrt builds BusyBox with a 512-byte interactive line editor. Keep
   // every physical line below that limit, even for short user commands.
   // Emit a short standalone input marker before the shell reads any PS2
   // continuation. A leading newline separates it from concurrent PTY echo.
-  return `${marker}=0; printf '\\n%s\\n' '${marker}_I'\n` + [
+  return `${marker}=0; ${ps0Save}; printf '\\n%s\\n' '${marker}_I'\n` + [
     ` : '${marker}'; ${cmdAssign}`,
     `{ printf '${startFormat}' '${marker}_S'; trap ':' INT; ( ${noPager}eval "$${marker}_cmd" ); __NCMCP_rc=$?; trap - INT`,
     `printf '%s\\n' '${marker}_E:'"$__NCMCP_rc"`,
     ...historyCleanup,
-    `(exit $__NCMCP_rc); }`,
+    `${ps0Restore}; (exit $__NCMCP_rc); }`,
   ].join(`; \\\n: '${marker}'; `);
 }