Potato follow-up: Astra blockers (bounded sends, first-response, output commit, approval ownership, strict 101)

.changeset/desktop-shim-followup.md

@@ -0,0 +1,5 @@
+---
+"grok-bot-cli": patch
+---
+
+Harden the Desktop shim bridge per follow-up review: bound every send/lock with the monotonic session budget (no pong/cleanup hang on a non-reading peer), track the first daemon response instead of any notification for the first-RPC deadline, commit on input or output (never fall back to stock on used stdout), answer approvals only for the turn gbot started (foreign thread/Desktop turn stays silent), require HTTP/1.1 101 plus accept, and flush EOF-tail data before the WS Close.

README.md

@@ -87,7 +87,9 @@ gbot codex desktop-shim uninstall  # removes wrapper/bridge/LaunchAgent; Desktop
 
 Install copies the scripts out of the package into durable `~/.codex/bin/` paths, so uninstalling the npm package never leaves Desktop pointing into a deleted checkout. The wrapper exports install-time `CODEX_HOME` and derives the socket from it at runtime (`CODEX_APP_SERVER_SOCK`), so custom `CODEX_HOME` layouts work; `gbot` itself resolves the socket the same way (`CODEX_APP_SERVER_SOCK` wins, else `CODEX_HOME`), and `status` reports the effective path and which variable won.
 
-Fail-open runs only before any stdin byte is consumed: the wrapper preflights the daemon socket (3 s deadline) and runs the bridge as a child — never `exec` — falling through to the real standalone `codex` for every non-`app-server` spawn, every preflight failure, and every pre-session bridge failure (exit 1), always on pristine stdio. The bridge's connect + WebSocket-upgrade handshake runs under one absolute monotonic deadline (`CODEX_BRIDGE_CONNECT_TIMEOUT`, default 10 s) with the 101 status and `Sec-WebSocket-Accept` validated, and a first-message deadline (`CODEX_BRIDGE_FIRST_MESSAGE_TIMEOUT`, default 30 s) bounds connect-to-first-daemon-message covering the first RPC — preflight (3 s) + upgrade (10 s) + first RPC (30 s) stays far below a daemon-lock hang, and none of these stages reads stdin. The commit point is byte-precise: the bridge counts every stdin byte read (buffered readahead included), so even an unforwarded blank line counts as committed. A mid-session bridge failure (exit 2+) makes the wrapper exit promptly so Desktop reconnects; the fallback never runs on half-consumed stdin. The wrapper never starts the daemon on the spawn path (a wedged daemon lock must not block Desktop) — upkeep belongs to the LaunchAgent login script and install, which share one `CODEX_HOME` for the GUI domain and the daemon they start; if the socket is absent, Desktop simply runs stock Codex until the daemon is started.
+Fail-open runs only before any stdin byte is consumed and no daemon payload was forwarded: the wrapper preflights the daemon socket (3 s deadline) and runs the bridge as a child — never `exec` — falling through to the real standalone `codex` for every non-`app-server` spawn, every preflight failure, and every pre-session bridge failure (exit 1), always on pristine stdio+stdout. The bridge's connect + WebSocket-upgrade handshake runs under one absolute monotonic deadline (`CODEX_BRIDGE_CONNECT_TIMEOUT`, default 10 s) requiring `HTTP/1.1 101` plus a matching `Sec-WebSocket-Accept` (a `200` fails even with a valid hash), and a first-response deadline (`CODEX_BRIDGE_FIRST_MESSAGE_TIMEOUT`, default 30 s) bounds connect-to-matching-daemon-response covering the first RPC — an unrelated notification or foreign response never satisfies it, only the matching response/error (or timeout) clears it — preflight (3 s) + upgrade (10 s) + first RPC (30 s) stays far below a daemon-lock hang, and none of these stages reads stdin. Every send/lock shares the same monotonic session budget (remaining first-RPC budget, else the connect timeout), so a non-reading peer cannot wedge pong writes or timeout cleanup; timeouts close and exit. The commit point is byte-precise on input OR output: the bridge counts every stdin byte read (buffered readahead included, even an unforwarded blank line) and marks stdout used on any forwarded daemon payload, so the fallback only ever runs on truly pristine stdio+stdout. Buffered EOF-tail data flushes before the WS Close. A mid-session bridge failure (exit 2+) makes the wrapper exit promptly so Desktop reconnects; the fallback never runs on half-consumed stdin or already-used stdout. The wrapper never starts the daemon on the spawn path (a wedged daemon lock must not block Desktop) — upkeep belongs to the LaunchAgent login script and install, which share one `CODEX_HOME` for the GUI domain and the daemon they start; if the socket is absent, Desktop simply runs stock Codex until the daemon is started.
+
+Residual risks, stated honestly: a concurrent foreign approval carrying no thread/turn ids inside `gbot`'s own `turn/start` window is indistinguishable from `gbot`'s and will still be refused — don't send to threads Desktop is actively driving. A daemon that speaks framing-valid but semantically unexpected JSON-RPC (unknown methods, id-less responses) is treated as transport; pins are to app-server schema 0.154.0. The bridge trusts the local control socket; a malicious local daemon could hold the session up to the stated budgets, not past them.
 
 Permanent tradeoff, stated plainly: Desktop's app-tools MCP (`-c` overrides on its spawn line) is not applied to the already-running managed daemon, and no config/`mcpServer`/`reload` path imports Desktop's `-c` flags — Desktop app-tools stay degraded while pointed at the shared daemon. Fully quit and relaunch ChatGPT.app after install (or login) so it inherits `CODEX_CLI_PATH`. `status` reads the macOS GUI-domain value via `launchctl getenv` (what Desktop actually inherits) alongside the calling shell's value. LaunchAgent persistence is macOS-first; elsewhere install still writes the wrapper and bridge but leaves `CODEX_CLI_PATH` for you to export. `~/.codex/bin` holds scripts only — there is no extra revert note to clean up; revert is `gbot codex desktop-shim uninstall` plus this section.
 
@@ -114,7 +116,7 @@ Sends at `hop >= GROK_BOT_MAX_HOPS` (default 4) are refused with `reason: "hop-l
 - `busy` / `thread-error` / `unknown-status`: see above.
 - `route-not-allowed` / `hop-limit` / `experimental-disabled`: refused by operator policy, the relay bound, or the experimental-API gate.
 - `unsupported`: the daemon does not offer the (experimental) method `--when-busy queue` needs.
-- `approval-refused` (`delivery: "accepted"`): `gbot` never approves commands or file changes on your behalf. If Codex asks while `gbot` is still connected, `send` refuses the request, exits 1, and tells you the turn id. Refusal replies are armed only once `gbot`'s own `turn/start` is in flight — an approval outstanding from another client's turn (notably ChatGPT Desktop's) during resume, a busy reject, or a queue add is recorded but never answered, so `gbot` cannot reject Desktop's approval. Residual race, stated honestly: an approval from a concurrent foreign turn arriving inside `gbot`'s own `turn/start` window is indistinguishable from `gbot`'s and will be refused; don't send to threads Desktop is actively driving. `send` disconnects as soon as the turn starts, so later approval requests stay with the daemon for a Codex client to answer; for unattended sends set `approval_policy = "never"` in the daemon's `config.toml`.
+- `approval-refused` (`delivery: "accepted"`): `gbot` never approves commands or file changes on your behalf. If Codex asks while `gbot` is still connected, `send` refuses the request, exits 1, and tells you the turn id. Refusal replies are armed only once `gbot`'s own `turn/start` is in flight and only for requests naming that thread/turn — an approval outstanding from another client's turn (notably ChatGPT Desktop's) during resume, a busy reject, or a queue add is recorded but never answered, and a request naming another thread or Desktop turn inside the window is foreign-skipped, so `gbot` cannot reject Desktop's approval. Residual race, stated honestly: an approval from a concurrent foreign turn arriving inside `gbot`'s own `turn/start` window with no thread/turn ids is indistinguishable from `gbot`'s and will be refused; don't send to threads Desktop is actively driving. `send` disconnects as soon as the turn starts, so later approval requests stay with the daemon for a Codex client to answer; for unattended sends set `approval_policy = "never"` in the daemon's `config.toml`.
 - `transport` / `bad-response` (`delivery: "unknown"`): the connection dropped or the daemon answered off-schema after the request left.
 
 ## Talking to Grok Bot from Codex

src/core/codex-bridge.js

@@ -282,11 +282,15 @@ export function connectCodexAppServer(path, { timeoutMs = 15000 } = {}) {
     const client = {
       refused,
       // Server-initiated requests are only *answered* once our own turn/start
-      // is in flight: anything earlier belongs to another client's turn
-      // (notably Desktop's) and must never be rejected on their behalf. Early
-      // requests are still recorded; the connection closes right after, so a
-      // pending request simply dies with it instead of denying someone's approval.
+      // is in flight, and then only when they name our own thread/turn:
+      // anything earlier — or naming another thread or Desktop turn — belongs
+      // to another client and must never be rejected on their behalf. Early
+      // and foreign requests are still recorded; the connection closes right
+      // after, so a pending request simply dies with it instead of denying
+      // someone's approval.
       answerServerRequests: false,
+      expectedThreadId: null,
+      expectedTurnId: null,
       request(method, params) {
         const id = nextId++;
         return new Promise((res, rej) => {
@@ -327,7 +331,20 @@ export function connectCodexAppServer(path, { timeoutMs = 15000 } = {}) {
 
     const onMessage = (msg) => {
       if (msg.id != null && msg.method) {
-        const answered = client.answerServerRequests;
+        let answered = client.answerServerRequests;
+        // Approval ownership: only answer requests for the turn gbot itself
+        // started. A request naming another thread — or naming a turn that is
+        // not ours (including any named Desktop turn while our turn id is
+        // still unknown) — is recorded foreign-silent, never rejected.
+        if (answered) {
+          const params = msg.params && typeof msg.params === "object" && !Array.isArray(msg.params) ? msg.params : null;
+          if (params) {
+            const threadId = params.threadId ?? params.thread_id;
+            const turnId = params.turnId ?? params.turn_id ?? params.turn?.id;
+            if (threadId != null && threadId !== client.expectedThreadId) answered = false;
+            else if (turnId != null && turnId !== client.expectedTurnId) answered = false;
+          }
+        }
         refused.push({ id: msg.id, method: msg.method, params: msg.params, answered });
         if (!answered) return;
         sendJson({
@@ -893,11 +910,14 @@ async function sendToCodexThreadInner(threadId, text, { env, envelope, whenBusy
     // the daemon exposes no compare-and-start request. Upgrade path: thread/queue/add + thread/queue/start once stable.
     // Scope refusals to this turn: server requests from earlier calls belong to another context.
     const seenRefused = client.refused.length;
-    // Arm refusal replies only now that our own turn/start is in flight: an
-    // approval arriving in this window plausibly belongs to our turn. Anything
-    // recorded earlier (resume, busy reject, queue) stays unanswered so gbot
-    // never rejects another client's approval. Residual race: a concurrent
-    // foreign approval inside this window is indistinguishable from ours.
+    // Arm refusal replies only now that our own turn/start is in flight, scoped
+    // to our own thread/turn: anything recorded earlier (resume, busy reject,
+    // queue) — or naming another thread or Desktop turn — stays unanswered so
+    // gbot never rejects another client's approval. Residual race, stated in
+    // the README: a concurrent foreign approval with no thread/turn ids inside
+    // this window is indistinguishable from ours.
+    client.expectedThreadId = threadId;
+    client.expectedTurnId = null;
     client.answerServerRequests = true;
     let turn;
     try {
@@ -924,8 +944,18 @@ async function sendToCodexThreadInner(threadId, text, { env, envelope, whenBusy
         { delivery: "unknown", reason: "bad-response", threadId, envelope },
       );
     }
+    client.expectedTurnId = turnId;
     const freshRefused = client.refused.slice(seenRefused)
-      .filter((r) => r.answered && (!r.params || r.params.threadId == null || r.params.threadId === threadId));
+      .filter((r) => {
+        if (!r.answered) return false;
+        const params = r.params && typeof r.params === "object" && !Array.isArray(r.params) ? r.params : null;
+        if (!params) return true;
+        const refusedThreadId = params.threadId ?? params.thread_id;
+        const refusedTurnId = params.turnId ?? params.turn_id ?? params.turn?.id;
+        if (refusedThreadId != null && refusedThreadId !== threadId) return false;
+        if (refusedTurnId != null && refusedTurnId !== turnId) return false;
+        return true;
+      });
     if (freshRefused.length) {
       const methods = freshRefused.map((r) => r.method).join(", ");
       throw new CodexSendError(