Fix Desktop bridge backpressure, startup deadlines, and approval ownership

.changeset/potato-mustfix-stack.md

@@ -0,0 +1,5 @@
+---
+"grok-bot-cli": patch
+---
+
+Stack the must-fix set from the Codex reject of the Act-On-PR bridge and the unfinished follow-up accept bar: commit stdout BEFORE writing (dirty stdout never exits 1 / never fail-opens to stock Codex) with bounded stdout writes and termination, keep healthy idle reads untimed (separate absolute handshake/first-RPC deadline from mid-session write/lock budgets), clear the init timer only on the matching initialize response/error (notifications never satisfy it), stay silent on foreign serverRequest approvals (deferring turn-scoped ones until the turn id is known), require strict HTTP/1.1 101 plus accept, flush EOF-tail data before the WS Close, and keep the wrapper hardening (self-fallback refusal, bridge+python3+preflight gating, uninstall reporting, Linux status wording with shell-quoted path).

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 committed to stdout: 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 preflight and upgrade leave stdin untouched. Reads after a successful init block with no timeout so healthy idle sessions survive silence; every mid-session socket send and stdout write runs under its own write budget (`CODEX_BRIDGE_IO_TIMEOUT`, default 30 s), so a wedged or non-reading peer cannot hang Desktop. The commit point is byte-precise on input OR output, committed before the stdout write: the bridge counts every stdin byte read (buffered readahead included, even an unforwarded blank line) and marks stdout used before writing any forwarded daemon payload via a bounded write, 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 dirty 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: requests without both matching thread and turn IDs remain unanswered; a Codex client must handle those requests. 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-silent, so `gbot` cannot reject Desktop's approval. A request naming a turn that arrives before the acknowledgment supplies `gbot`'s turn id waits instead of being classified foreign, and is answered only on a match. Requests with missing thread or turn IDs remain unanswered because ownership cannot be established. `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,43 @@ 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,
+      // Approvals that name a turn while our own turn id is still unknown
+      // (arriving before — or in the same socket batch as — the turn/start
+      // acknowledgment) wait here instead of being classified foreign: once
+      // the acknowledgment supplies the turn id, matching ones are answered
+      // and the rest stay silent.
+      deferred: [],
+      // Adopt the acknowledged turn id, answering deferred requests that
+      // prove to be ours. Runs inside the connection closure so it can send.
+      _adoptTurn(turnId) {
+        client.expectedTurnId = turnId;
+        for (const entry of client.deferred.splice(0)) {
+          const params = entry.params && typeof entry.params === "object" && !Array.isArray(entry.params)
+            ? entry.params
+            : null;
+          const threadId = params ? (params.threadId ?? params.thread_id) : null;
+          const namedTurnId = params
+            ? (params.turnId ?? params.turn_id ?? (params.turn && params.turn.id))
+            : null;
+          if (threadId === client.expectedThreadId && namedTurnId === turnId) {
+            entry.answered = true;
+            sendJson({
+              jsonrpc: "2.0",
+              id: entry.id,
+              error: { code: -32601, message: "gbot codex does not answer " + entry.method + "; configure approval_policy on the daemon" },
+            });
+          }
+        }
+      },
       request(method, params) {
         const id = nextId++;
         return new Promise((res, rej) => {
@@ -327,8 +359,29 @@ export function connectCodexAppServer(path, { timeoutMs = 15000 } = {}) {
 
     const onMessage = (msg) => {
       if (msg.id != null && msg.method) {
-        const answered = client.answerServerRequests;
-        refused.push({ id: msg.id, method: msg.method, params: msg.params, answered });
+        let answered = client.answerServerRequests;
+        let defer = false;
+        // Approval ownership: only answer requests for the turn gbot itself
+        // started. A request naming another thread — or naming a turn that is
+        // not ours — is recorded foreign-silent, never rejected. A request
+        // naming a turn while our turn id is still unknown is deferred, not
+        // classified: the turn/start acknowledgment decides it.
+        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 && params.turn.id);
+            if (threadId !== client.expectedThreadId || turnId == null) answered = false;
+            else if (client.expectedTurnId == null) { answered = false; defer = true; }
+            else if (turnId !== client.expectedTurnId) answered = false;
+          } else answered = false;
+        }
+        const entry = { id: msg.id, method: msg.method, params: msg.params, answered };
+        refused.push(entry);
+        if (defer) {
+          client.deferred.push(entry);
+          return;
+        }
         if (!answered) return;
         sendJson({
           jsonrpc: "2.0",
@@ -893,11 +946,15 @@ 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. Requests naming a turn
+    // that arrives before the acknowledgment supplies our turn id wait in
+    // client.deferred and are adopted only on a match. Requests missing
+    // thread or turn IDs remain unanswered because ownership is unknown.
+    client.expectedThreadId = threadId;
+    client.expectedTurnId = null;
     client.answerServerRequests = true;
     let turn;
     try {
@@ -924,8 +981,18 @@ async function sendToCodexThreadInner(threadId, text, { env, envelope, whenBusy
         { delivery: "unknown", reason: "bad-response", threadId, envelope },
       );
     }
+    client._adoptTurn(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 && 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(