From 7fca34758be76adaf52c2ca7f7161be586f51318 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 15 May 2026 18:10:56 +0000 Subject: [PATCH 1/8] Add pluggable inbound-webhook framework with Granola meeting summarizer Introduces webhooks/ as a place to mount custom webhook listeners on the same Bun.serve instance. First source is Granola: when a meeting wraps, the emitter POSTs to /api/webhooks/granola and a Codex-generated summary is streamed into the configured Slack channel. - Shared X-Webhook-Secret auth and Redis NX/EX dedup per source - Handler returns 200 immediately and processes in the background - Failures post a :warning: notice to the same channel so issues are visible - Extracted streamCodex into lib/codex.ts so mentions and webhooks share it --- .env.example | 10 ++++- CLAUDE.md | 15 +++++++- index.ts | 59 +++++----------------------- lib/codex.ts | 48 +++++++++++++++++++++++ webhooks/granola.ts | 94 +++++++++++++++++++++++++++++++++++++++++++++ webhooks/types.ts | 12 ++++++ 6 files changed, 186 insertions(+), 52 deletions(-) create mode 100644 lib/codex.ts create mode 100644 webhooks/granola.ts create mode 100644 webhooks/types.ts diff --git a/.env.example b/.env.example index 4f077f3..ef174b2 100644 --- a/.env.example +++ b/.env.example @@ -5,5 +5,13 @@ SLACK_BOT_TOKEN=xoxb-your-token-here # Must have connections:write scope SLACK_APP_TOKEN=xapp-your-token-here -# Redis URL for state management +# Redis URL for state management (and inbound-webhook dedup) REDIS_URL=redis://localhost:6379 + +# Shared secret for inbound custom webhooks. Senders must include it as the +# `X-Webhook-Secret` header. Pick something long and random. +WEBHOOK_SECRET=change-me + +# Slack channel ID for #edison-os-updates (the target for meeting wrap-ups). +# Find it in Slack: channel name > "View channel details" > scroll to bottom > "Channel ID". +GRANOLA_SLACK_CHANNEL_ID= diff --git a/CLAUDE.md b/CLAUDE.md index 52ceeaa..4933dcd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,5 +4,18 @@ Slack bot using [chat-sdk](https://chat-sdk.dev) with `@chat-adapter/slack` (web - Runtime: Bun (auto-loads `.env`, no dotenv needed) - HTTP server: `Bun.serve()` on port 3123 -- State: local Redis on `localhost:6379` +- State: local Redis on `localhost:6379` (also used for inbound-webhook dedup via `Bun.redis`) - Slack setup and known issues: `docs/SLACK_INTEGRATION.md` + +## Inbound webhooks (custom, non-Slack) + +Pluggable webhook handlers live in `webhooks/`. Each module exports a factory `(ctx: { bot }) => { path, handler }`; `index.ts` mounts them under `Bun.serve({ routes })`. Add a source by creating `webhooks/.ts` and pushing the factory into the `webhooks` array in `index.ts`. + +Shared conventions: +- Auth: senders must include `X-Webhook-Secret: $WEBHOOK_SECRET`. +- Dedup: handlers `SET key NX EX 604800` in Redis; duplicates return 200 without re-processing. +- Async: handlers return 200 immediately and process in the background. Failures post a `:warning:` notice to the same Slack channel. +- Codex: reuse `lib/codex.ts#streamCodex` and pipe it into `bot.channel("slack:Cxxx").post(...)` for a streamed Slack reply. + +Current sources: +- `webhooks/granola.ts` — `POST /api/webhooks/granola`. Expects `{ meeting_id, title, transcript, notes?, attendees? }`. Posts a Codex summary to `GRANOLA_SLACK_CHANNEL_ID` (intended: `#edison-os-updates`). diff --git a/index.ts b/index.ts index 913fa0c..e1b9adf 100644 --- a/index.ts +++ b/index.ts @@ -1,58 +1,12 @@ import { Chat, toAiMessages } from "chat"; import { createSlackAdapter } from "@chat-adapter/slack"; import { createRedisState } from "@chat-adapter/state-redis"; +import { streamCodex } from "./lib/codex"; +import { granolaWebhook } from "./webhooks/granola"; +import type { WebhookRoute } from "./webhooks/types"; const slackAdapter = createSlackAdapter(); -async function* streamCodex(prompt: string): AsyncIterable { - const proc = Bun.spawn( - ["codex", "exec", "--ephemeral", "-s", "read-only", "--json", "-"], - { stdin: "pipe", stdout: "pipe", stderr: "ignore" }, - ); - proc.stdin.write(prompt); - proc.stdin.end(); - - const decoder = new TextDecoder(); - let buffer = ""; - let messageCount = 0; - - for await (const chunk of proc.stdout) { - buffer += decoder.decode(chunk, { stream: true }); - const lines = buffer.split("\n"); - buffer = lines.pop() ?? ""; - - for (const line of lines) { - if (!line.trim()) continue; - try { - const event = JSON.parse(line); - if ( - event.type === "item.completed" && - event.item?.type === "agent_message" && - event.item.text - ) { - if (messageCount > 0) yield "\n\n---\n\n"; - yield event.item.text; - messageCount++; - } - } catch {} - } - } - - if (buffer.trim()) { - try { - const event = JSON.parse(buffer); - if ( - event.type === "item.completed" && - event.item?.type === "agent_message" && - event.item.text - ) { - if (messageCount > 0) yield "\n\n---\n\n"; - yield event.item.text; - } - } catch {} - } -} - const bot = new Chat({ userName: "edison-bot", adapters: { @@ -76,6 +30,8 @@ bot.onNewMention(async (thread, message) => { await bot.initialize(); +const webhooks: WebhookRoute[] = [granolaWebhook({ bot })]; + const port = 3123; Bun.serve({ @@ -86,10 +42,13 @@ Bun.serve({ return slackAdapter.handleWebhook(req); }, }, + ...Object.fromEntries(webhooks.map((w) => [w.path, { POST: w.handler }])), }, fetch(req) { return new Response("Not found", { status: 404 }); }, }); -console.log("Bot is running! Webhook listening on http://localhost:3123/api/webhooks/slack"); +console.log(`Bot is running on http://localhost:${port}`); +console.log(` POST /api/webhooks/slack`); +for (const w of webhooks) console.log(` POST ${w.path}`); diff --git a/lib/codex.ts b/lib/codex.ts new file mode 100644 index 0000000..8692fb3 --- /dev/null +++ b/lib/codex.ts @@ -0,0 +1,48 @@ +export async function* streamCodex(prompt: string): AsyncIterable { + const proc = Bun.spawn( + ["codex", "exec", "--ephemeral", "-s", "read-only", "--json", "-"], + { stdin: "pipe", stdout: "pipe", stderr: "ignore" }, + ); + proc.stdin.write(prompt); + proc.stdin.end(); + + const decoder = new TextDecoder(); + let buffer = ""; + let messageCount = 0; + + for await (const chunk of proc.stdout) { + buffer += decoder.decode(chunk, { stream: true }); + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + + for (const line of lines) { + if (!line.trim()) continue; + try { + const event = JSON.parse(line); + if ( + event.type === "item.completed" && + event.item?.type === "agent_message" && + event.item.text + ) { + if (messageCount > 0) yield "\n\n---\n\n"; + yield event.item.text; + messageCount++; + } + } catch {} + } + } + + if (buffer.trim()) { + try { + const event = JSON.parse(buffer); + if ( + event.type === "item.completed" && + event.item?.type === "agent_message" && + event.item.text + ) { + if (messageCount > 0) yield "\n\n---\n\n"; + yield event.item.text; + } + } catch {} + } +} diff --git a/webhooks/granola.ts b/webhooks/granola.ts new file mode 100644 index 0000000..9f798e5 --- /dev/null +++ b/webhooks/granola.ts @@ -0,0 +1,94 @@ +import { redis } from "bun"; +import { streamCodex } from "../lib/codex"; +import type { WebhookContext, WebhookRoute } from "./types"; + +type GranolaPayload = { + meeting_id: string; + title: string; + transcript: string; + notes?: string; + attendees?: string[]; +}; + +const DEDUP_TTL_SECONDS = 60 * 60 * 24 * 7; + +export function granolaWebhook(ctx: WebhookContext): WebhookRoute { + const channelId = process.env.GRANOLA_SLACK_CHANNEL_ID; + const secret = process.env.WEBHOOK_SECRET; + + if (!channelId) { + console.warn("[granola webhook] GRANOLA_SLACK_CHANNEL_ID is not set; requests will 500"); + } + if (!secret) { + console.warn("[granola webhook] WEBHOOK_SECRET is not set; all requests will be rejected"); + } + + return { + path: "/api/webhooks/granola", + handler: async (req) => { + if (!secret || req.headers.get("x-webhook-secret") !== secret) { + return new Response("Unauthorized", { status: 401 }); + } + if (!channelId) { + return new Response("GRANOLA_SLACK_CHANNEL_ID not configured", { status: 500 }); + } + + let payload: GranolaPayload; + try { + payload = (await req.json()) as GranolaPayload; + } catch { + return new Response("Invalid JSON", { status: 400 }); + } + if (!payload.meeting_id || !payload.title || !payload.transcript) { + return new Response("Missing required fields: meeting_id, title, transcript", { status: 400 }); + } + + const dedupKey = `webhook:granola:${payload.meeting_id}`; + const claimed = await redis.send("SET", [dedupKey, "1", "NX", "EX", String(DEDUP_TTL_SECONDS)]); + if (claimed !== "OK") { + return new Response("Duplicate (already processed)", { status: 200 }); + } + + void processGranolaMeeting(ctx, channelId, payload); + + return new Response("OK", { status: 200 }); + }, + }; +} + +async function processGranolaMeeting( + ctx: WebhookContext, + channelId: string, + payload: GranolaPayload, +) { + const channel = ctx.bot.channel(`slack:${channelId}`); + const attendeesLine = payload.attendees?.length + ? `\nAttendees: ${payload.attendees.join(", ")}` + : ""; + + const prompt = `Write a Slack message summarizing the following meeting. + +Format: +- First line: "*Meeting wrap-up — ${payload.title}*" (Slack-style bold with single asterisks). +- Then 3-5 bullets starting with "• " covering what was discussed. +- If concrete action items were mentioned, add a blank line, then "*Action items:*", then bullets starting with "• ". + +No preamble, no markdown headers, no "Here is the summary" — emit only the formatted Slack message.${attendeesLine} + +Transcript: +${payload.transcript}${payload.notes ? `\n\n--- Notes ---\n${payload.notes}` : ""}`; + + try { + await channel.post(streamCodex(prompt)); + } catch (err) { + console.error("[granola webhook] processing failed:", err); + const msg = err instanceof Error ? err.message : String(err); + try { + await channel.post( + `:warning: Failed to summarize meeting "${payload.title}" (\`${payload.meeting_id}\`): ${msg}`, + ); + } catch (notifyErr) { + console.error("[granola webhook] failure notify also failed:", notifyErr); + } + } +} diff --git a/webhooks/types.ts b/webhooks/types.ts new file mode 100644 index 0000000..bf2775f --- /dev/null +++ b/webhooks/types.ts @@ -0,0 +1,12 @@ +import type { Chat } from "chat"; + +export type WebhookContext = { + bot: Chat; +}; + +export type WebhookRoute = { + path: string; + handler: (req: Request) => Promise; +}; + +export type WebhookFactory = (ctx: WebhookContext) => WebhookRoute; From dc6d91c3d82388ca74f05361a70a51208b71de28 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 16 May 2026 16:55:58 +0000 Subject: [PATCH 2/8] Harden webhook input validation and codex exit handling Addresses three review issues from cubic on PR #4: - webhooks/granola.ts: reject non-object JSON bodies (null, arrays, primitives) before field access, instead of letting a TypeError bubble up as a 500. - webhooks/granola.ts: normalize `attendees` with Array.isArray before calling .join, so a malformed (non-array, non-empty) value can no longer throw outside the surrounding try/catch in the background task. - lib/codex.ts: await the subprocess and throw on non-zero exit so silent codex failures surface as errors instead of empty summaries. Callers (webhook handler, onNewMention) now see the failure and can react. --- lib/codex.ts | 5 +++++ webhooks/granola.ts | 24 ++++++++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/lib/codex.ts b/lib/codex.ts index 8692fb3..c262328 100644 --- a/lib/codex.ts +++ b/lib/codex.ts @@ -45,4 +45,9 @@ export async function* streamCodex(prompt: string): AsyncIterable { } } catch {} } + + const exitCode = await proc.exited; + if (exitCode !== 0) { + throw new Error(`codex exited with code ${exitCode}`); + } } diff --git a/webhooks/granola.ts b/webhooks/granola.ts index 9f798e5..da54372 100644 --- a/webhooks/granola.ts +++ b/webhooks/granola.ts @@ -33,13 +33,24 @@ export function granolaWebhook(ctx: WebhookContext): WebhookRoute { return new Response("GRANOLA_SLACK_CHANNEL_ID not configured", { status: 500 }); } - let payload: GranolaPayload; + let raw: unknown; try { - payload = (await req.json()) as GranolaPayload; + raw = await req.json(); } catch { return new Response("Invalid JSON", { status: 400 }); } - if (!payload.meeting_id || !payload.title || !payload.transcript) { + if (typeof raw !== "object" || raw === null || Array.isArray(raw)) { + return new Response("Invalid payload: expected a JSON object", { status: 400 }); + } + const payload = raw as Partial; + if ( + typeof payload.meeting_id !== "string" || + typeof payload.title !== "string" || + typeof payload.transcript !== "string" || + !payload.meeting_id || + !payload.title || + !payload.transcript + ) { return new Response("Missing required fields: meeting_id, title, transcript", { status: 400 }); } @@ -49,7 +60,7 @@ export function granolaWebhook(ctx: WebhookContext): WebhookRoute { return new Response("Duplicate (already processed)", { status: 200 }); } - void processGranolaMeeting(ctx, channelId, payload); + void processGranolaMeeting(ctx, channelId, payload as GranolaPayload); return new Response("OK", { status: 200 }); }, @@ -62,8 +73,9 @@ async function processGranolaMeeting( payload: GranolaPayload, ) { const channel = ctx.bot.channel(`slack:${channelId}`); - const attendeesLine = payload.attendees?.length - ? `\nAttendees: ${payload.attendees.join(", ")}` + const attendees = Array.isArray(payload.attendees) ? payload.attendees : []; + const attendeesLine = attendees.length + ? `\nAttendees: ${attendees.join(", ")}` : ""; const prompt = `Write a Slack message summarizing the following meeting. From 0bb78145d83b65c2657c43578a29c6f8c6fc9e8c Mon Sep 17 00:00:00 2001 From: Edison Date: Fri, 10 Jul 2026 10:34:19 +0100 Subject: [PATCH 3/8] Document launchd deployment and add LaunchAgent template MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Capture the macOS deploy setup that was previously only tribal knowledge: - deploy/com.edison.assistant-bot.plist: LaunchAgent template (RunAtLoad + KeepAlive, direct bun exec, no secrets — env comes from .env). - docs/DEPLOY.md: install/manage commands plus two gotchas hit in practice: (1) a checkout on an external/removable volume needs Full Disk Access on the bun binary or launchd gets EPERM reading files (TCC); (2) the codex CLI hangs in dyld if its signing cert is revoked — upgrade to a clean build. - CLAUDE.md: link to the new deploy doc. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 1 + deploy/com.edison.assistant-bot.plist | 40 ++++++++++++++ docs/DEPLOY.md | 76 +++++++++++++++++++++++++++ 3 files changed, 117 insertions(+) create mode 100644 deploy/com.edison.assistant-bot.plist create mode 100644 docs/DEPLOY.md diff --git a/CLAUDE.md b/CLAUDE.md index 4933dcd..0d7567d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,6 +6,7 @@ Slack bot using [chat-sdk](https://chat-sdk.dev) with `@chat-adapter/slack` (web - HTTP server: `Bun.serve()` on port 3123 - State: local Redis on `localhost:6379` (also used for inbound-webhook dedup via `Bun.redis`) - Slack setup and known issues: `docs/SLACK_INTEGRATION.md` +- Deployment (launchd LaunchAgent, external-volume Full Disk Access, codex cert gotcha): `docs/DEPLOY.md` ## Inbound webhooks (custom, non-Slack) diff --git a/deploy/com.edison.assistant-bot.plist b/deploy/com.edison.assistant-bot.plist new file mode 100644 index 0000000..fc339ac --- /dev/null +++ b/deploy/com.edison.assistant-bot.plist @@ -0,0 +1,40 @@ + + + + + + Labelcom.edison.assistant-bot + + ProgramArguments + + /opt/homebrew/bin/bun + run + index.ts + + + + WorkingDirectory + /Volumes/Data/edison_workdir/Github/assistant + + RunAtLoad + KeepAlive + ThrottleInterval10 + + StandardOutPath/Users/edison/Library/Logs/assistant-bot.log + StandardErrorPath/Users/edison/Library/Logs/assistant-bot.log + + EnvironmentVariables + + PATH + /opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/edison/.bun/bin + HOME + /Users/edison + + + diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md new file mode 100644 index 0000000..e8edb93 --- /dev/null +++ b/docs/DEPLOY.md @@ -0,0 +1,76 @@ +# Deployment (macOS launchd) + +The bot runs as a per-user launchd **LaunchAgent** so it auto-starts on login and +auto-restarts on crash. Template: [`deploy/com.edison.assistant-bot.plist`](../deploy/com.edison.assistant-bot.plist). + +## Install + +```sh +# 1. Copy the template into place and edit WorkingDirectory to your checkout. +cp deploy/com.edison.assistant-bot.plist ~/Library/LaunchAgents/ + +# 2. Make sure Redis is running (used for chat state + inbound-webhook dedup). +brew services start redis + +# 3. Load it (bootstrap into the GUI session so it starts on login). +launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/com.edison.assistant-bot.plist + +# 4. Verify. +tail -n 20 ~/Library/Logs/assistant-bot.log # expect "Bot is running on http://localhost:3123" +lsof -i :3123 # expect a bun process LISTEN +curl -i -X POST http://localhost:3123/api/webhooks/slack -d '{}' # expect 401 (adapter reachable) +``` + +Env vars (`SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`, `REDIS_URL`, `WEBHOOK_SECRET`, +`GRANOLA_SLACK_CHANNEL_ID`) are auto-loaded by Bun from `.env` in `WorkingDirectory`. +**Do not put secrets in the plist.** + +## Managing the agent + +```sh +launchctl kickstart -k gui/$(id -u)/com.edison.assistant-bot # restart +launchctl bootout gui/$(id -u)/com.edison.assistant-bot # stop + unload +launchctl list | grep assistant-bot # status (col 1 = PID, col 2 = last exit) +``` + +## Gotcha: repo on an external / non-boot volume needs Full Disk Access + +If the checkout lives on an **external or removable volume** (e.g. a USB drive under +`/Volumes/...`, possibly reached via a `~/Github` symlink), macOS **TCC blocks +launchd-spawned processes from reading file *contents* there** — metadata/`ls` is +allowed, but `read()` returns `EPERM` ("Operation not permitted"). Symptoms: the agent +shows a PID but the log is empty, port 3123 never binds, and `curl` gets connection +refused. It works from an interactive terminal only because Terminal already holds the +grant. + +**Fix:** grant **Full Disk Access to the bun binary**: + +1. System Settings → Privacy & Security → **Full Disk Access** → **+** +2. Add `/opt/homebrew/bin/bun` (⌘⇧G to type the path), toggle it **on**. +3. Restart the agent: `launchctl kickstart -k gui/$(id -u)/com.edison.assistant-bot` + +The grant is keyed to the binary, so after `brew upgrade bun` (its Cellar path/cdhash +changes) you may need to re-add it. If the volume is slow to mount at boot, the first +launch attempt exits and `KeepAlive`+`ThrottleInterval` retry every 10s until the volume +is readable — the agent self-heals. + +Diagnose a suspected TCC block with a boot-volume probe script that `ls`es then `head`s a +file on the external volume from a throwaway LaunchAgent: `ls` succeeds, `head` fails with +`Operation not permitted`. + +## Gotcha: codex CLI must not be on a revoked-cert build + +The Granola webhook shells out to the `codex` CLI (`lib/codex.ts`). If codex hangs on +**every** invocation — even `codex --version`, stuck in `_dyld_start` before `main()` — +its signing certificate has likely been **revoked**. Check: + +```sh +spctl --assess -vvv --type execute \ + "$(dirname "$(readlink -f "$(command -v codex)")")"/../lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/bin/codex +# CSSMERR_TP_CERT_REVOKED => revoked +``` + +**Fix:** upgrade to a build signed with a valid cert: `npm install -g @openai/codex@latest`. +(Observed: 0.129.0 was revoked and hung; 0.144.0 assesses clean and works.) Note `codex +exec` also requires running from a git repo or trusted dir — the bot satisfies this because +its `WorkingDirectory` is this repo. From 36b5fae67fbbe3755660f7e2392ca1acfe12683a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 09:41:09 +0000 Subject: [PATCH 4/8] Fix deployment template review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two cubic review comments on 0bb7814: - deploy/com.edison.assistant-bot.plist: parameterize user-specific fields with __USER__ and __WORKDIR__ placeholders instead of hardcoding one operator's username in StandardOut/ErrPath, PATH, HOME, and WorkingDirectory. Label stays as com.edison.assistant-bot (reverse-DNS prefix refers to the org, not the local user). - docs/DEPLOY.md: install step now renders the template via sed so all placeholders get substituted in one command. Documents which fields are parameterized and why. - docs/DEPLOY.md: rewrite the revoked-cert diagnostic. The old command relied on `readlink -f` (GNU-only; BSD readlink on macOS errors with "illegal option -- f") and manually joined path segments in a way that double-nested inside the codex npm package. New command uses `find` under `npm root -g` to locate the arch-specific binary — BSD-safe, arch-agnostic (arm64 and x86_64), and resilient to whether npm dedups the platform packages flat or under codex/node_modules. --- deploy/com.edison.assistant-bot.plist | 23 +++++++++++----------- docs/DEPLOY.md | 28 +++++++++++++++++++++++---- 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/deploy/com.edison.assistant-bot.plist b/deploy/com.edison.assistant-bot.plist index fc339ac..b1f141b 100644 --- a/deploy/com.edison.assistant-bot.plist +++ b/deploy/com.edison.assistant-bot.plist @@ -1,10 +1,11 @@ @@ -18,23 +19,23 @@ index.ts - + WorkingDirectory - /Volumes/Data/edison_workdir/Github/assistant + __WORKDIR__ RunAtLoad KeepAlive ThrottleInterval10 - StandardOutPath/Users/edison/Library/Logs/assistant-bot.log - StandardErrorPath/Users/edison/Library/Logs/assistant-bot.log + StandardOutPath/Users/__USER__/Library/Logs/assistant-bot.log + StandardErrorPath/Users/__USER__/Library/Logs/assistant-bot.log EnvironmentVariables PATH - /opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/edison/.bun/bin + /opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/Users/__USER__/.bun/bin HOME - /Users/edison + /Users/__USER__ diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index e8edb93..f33aed8 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -5,9 +5,15 @@ auto-restarts on crash. Template: [`deploy/com.edison.assistant-bot.plist`](../d ## Install +The template contains two placeholders — `__USER__` and `__WORKDIR__` — that must +be substituted before installing. `sed` handles both: + ```sh -# 1. Copy the template into place and edit WorkingDirectory to your checkout. -cp deploy/com.edison.assistant-bot.plist ~/Library/LaunchAgents/ +# 1. Render the template into ~/Library/LaunchAgents/, substituting the current +# user and the absolute path to this checkout. +sed -e "s|__USER__|$USER|g" -e "s|__WORKDIR__|$(pwd -P)|g" \ + deploy/com.edison.assistant-bot.plist \ + > ~/Library/LaunchAgents/com.edison.assistant-bot.plist # 2. Make sure Redis is running (used for chat state + inbound-webhook dedup). brew services start redis @@ -21,6 +27,13 @@ lsof -i :3123 # expect a bun process LISTEN curl -i -X POST http://localhost:3123/api/webhooks/slack -d '{}' # expect 401 (adapter reachable) ``` +Run step 1 from the repo root so `pwd -P` resolves to the real checkout path +(without symlinks — important because a symlinked `WorkingDirectory` may hit the +external-volume TCC gotcha below). If you're deploying for a different Unix user +than the one you're running the shell as, replace `$USER` with the target +username. `Label` intentionally stays `com.edison.assistant-bot` (the reverse-DNS +prefix refers to the org, not the local user). + Env vars (`SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`, `REDIS_URL`, `WEBHOOK_SECRET`, `GRANOLA_SLACK_CHANNEL_ID`) are auto-loaded by Bun from `.env` in `WorkingDirectory`. **Do not put secrets in the plist.** @@ -65,8 +78,15 @@ The Granola webhook shells out to the `codex` CLI (`lib/codex.ts`). If codex han its signing certificate has likely been **revoked**. Check: ```sh -spctl --assess -vvv --type execute \ - "$(dirname "$(readlink -f "$(command -v codex)")")"/../lib/node_modules/@openai/codex/node_modules/@openai/codex-darwin-arm64/vendor/aarch64-apple-darwin/bin/codex +# Locate the arch-specific codex binary under npm's global root without resolving +# symlinks (works for both `arm64` and `x86_64`, and with either flat or nested +# npm layouts). BSD-safe; no `readlink -f` needed. +CODEX_BIN="$(find "$(npm root -g)" -type f -name codex -path '*/vendor/*/bin/codex' 2>/dev/null | head -n 1)" +if [ -z "$CODEX_BIN" ]; then + echo "codex arch binary not found under $(npm root -g)" +else + spctl --assess -vvv --type execute "$CODEX_BIN" +fi # CSSMERR_TP_CERT_REVOKED => revoked ``` From 3ac8890d205f0339c7b2359ba185dad533a89794 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 10 Jul 2026 09:46:31 +0000 Subject: [PATCH 5/8] Fix cross-user note and codex diagnostic accuracy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses two cubic review comments on 36b5fae: - docs/DEPLOY.md: the previous "if deploying for a different user, replace $USER with the target username" note was misleading — the surrounding commands still targeted the current user's ~, $(id -u), and log paths. Replace it with a directive to run the whole install sequence as the target user so every reference resolves consistently. - docs/DEPLOY.md: the codex signing-cert diagnostic used the interactive shell's `npm root -g`, which nvm/nodenv/asdf can shadow. The bot spawns bare `codex` under the LaunchAgent's fixed PATH, so the diagnostic must resolve npm through that same PATH — otherwise it may inspect a different codex install than the one the bot actually runs, giving a false negative on a revoked cert. Rewrite to reproduce the bot's PATH and resolve npm via `command -v` under it (BSD-safe, arch-agnostic). --- docs/DEPLOY.md | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/docs/DEPLOY.md b/docs/DEPLOY.md index f33aed8..18da693 100644 --- a/docs/DEPLOY.md +++ b/docs/DEPLOY.md @@ -27,11 +27,11 @@ lsof -i :3123 # expect a bun process LISTEN curl -i -X POST http://localhost:3123/api/webhooks/slack -d '{}' # expect 401 (adapter reachable) ``` -Run step 1 from the repo root so `pwd -P` resolves to the real checkout path -(without symlinks — important because a symlinked `WorkingDirectory` may hit the -external-volume TCC gotcha below). If you're deploying for a different Unix user -than the one you're running the shell as, replace `$USER` with the target -username. `Label` intentionally stays `com.edison.assistant-bot` (the reverse-DNS +Run the install as the Unix user the bot should run under, from the repo root: +`$USER`, `$(id -u)`, and `~/Library/…` in the block above all resolve against that +same user. `pwd -P` gives the real checkout path without symlinks — important +because a symlinked `WorkingDirectory` may hit the external-volume TCC gotcha +below. `Label` intentionally stays `com.edison.assistant-bot` (the reverse-DNS prefix refers to the org, not the local user). Env vars (`SLACK_BOT_TOKEN`, `SLACK_APP_TOKEN`, `REDIS_URL`, `WEBHOOK_SECRET`, @@ -78,12 +78,18 @@ The Granola webhook shells out to the `codex` CLI (`lib/codex.ts`). If codex han its signing certificate has likely been **revoked**. Check: ```sh -# Locate the arch-specific codex binary under npm's global root without resolving -# symlinks (works for both `arm64` and `x86_64`, and with either flat or nested -# npm layouts). BSD-safe; no `readlink -f` needed. -CODEX_BIN="$(find "$(npm root -g)" -type f -name codex -path '*/vendor/*/bin/codex' 2>/dev/null | head -n 1)" +# Inspect the SAME codex the bot actually spawns. lib/codex.ts runs bare +# `codex`, which the LaunchAgent resolves via its fixed PATH — not the +# interactive shell's PATH (which nvm/nodenv/asdf may shadow). Reproduce that +# PATH here, then find the arch-specific vendor binary near the resolved codex. +# BSD-safe; no `readlink -f`; works on arm64 and x86_64. +BOT_PATH="/opt/homebrew/bin:/opt/homebrew/sbin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:$HOME/.bun/bin" +NPM_BIN="$(PATH="$BOT_PATH" command -v npm)" +[ -z "$NPM_BIN" ] && { echo "npm not on the bot's PATH"; return 1 2>/dev/null || exit 1; } +NPM_ROOT="$("$NPM_BIN" root -g)" +CODEX_BIN="$(find "$NPM_ROOT" -type f -name codex -path '*/vendor/*/bin/codex' 2>/dev/null | head -n 1)" if [ -z "$CODEX_BIN" ]; then - echo "codex arch binary not found under $(npm root -g)" + echo "codex arch binary not found under $NPM_ROOT" else spctl --assess -vvv --type execute "$CODEX_BIN" fi From a3d550d7d4e19d82b2620489d0fd40960b14581f Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 23:30:10 +0000 Subject: [PATCH 6/8] Make webhook dedup crash-safe and bound codex runs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dedup previously wrote a 7-day completion marker before processing began and never released it, so a crash, restart, or hung codex silently dropped the meeting for a week — the sender's retries got a 200 "Duplicate". Claim a 15-minute lease instead, promote it to the 7-day marker only after the summary posts, and delete it on failure so a retry can get through. streamCodex had no timeout and discarded stderr, which is exactly how the revoked-signing-cert hang presented in production: no output, no error, no diagnostic. Add a wall-clock timeout (10 min default) that kills the child and unblocks the read even when a grandchild holds the pipe open, capture the tail of stderr into the thrown error, and kill the process on early consumer exit. Also de-duplicate the JSONL parse path, which fixed a missing messageCount increment in the trailing-buffer branch. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZdVoF8VB8L1q4zUkgKSM1 --- CLAUDE.md | 2 +- lib/codex.ts | 155 ++++++++++++++++++++++++++++++++------------ webhooks/granola.ts | 34 +++++++++- 3 files changed, 148 insertions(+), 43 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0d7567d..ceefab3 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ Pluggable webhook handlers live in `webhooks/`. Each module exports a factory `( Shared conventions: - Auth: senders must include `X-Webhook-Secret: $WEBHOOK_SECRET`. -- Dedup: handlers `SET key NX EX 604800` in Redis; duplicates return 200 without re-processing. +- Dedup: handlers claim a short lease (`SET key NX EX 900`) in Redis, promote it to a 7-day completion marker on success, and delete it on failure so the sender can retry. Duplicates return 200 without re-processing. - Async: handlers return 200 immediately and process in the background. Failures post a `:warning:` notice to the same Slack channel. - Codex: reuse `lib/codex.ts#streamCodex` and pipe it into `bot.channel("slack:Cxxx").post(...)` for a streamed Slack reply. diff --git a/lib/codex.ts b/lib/codex.ts index c262328..7428fd5 100644 --- a/lib/codex.ts +++ b/lib/codex.ts @@ -1,53 +1,128 @@ -export async function* streamCodex(prompt: string): AsyncIterable { +const DEFAULT_TIMEOUT_MS = 10 * 60 * 1000; +const MAX_STDERR_CHARS = 2000; + +export type StreamCodexOptions = { + /** Wall-clock budget for the whole run; the process is killed when it elapses. */ + timeoutMs?: number; +}; + +/** Text of an agent message event, or null if this line isn't one. */ +function agentMessageText(line: string): string | null { + try { + const event = JSON.parse(line); + if ( + event.type === "item.completed" && + event.item?.type === "agent_message" && + event.item.text + ) { + return String(event.item.text); + } + } catch {} + return null; +} + +function stderrDetail(stderr: string): string { + const trimmed = stderr.trim(); + return trimmed ? `: ${trimmed}` : ""; +} + +const TIMED_OUT = Symbol("timed-out"); + +export async function* streamCodex( + prompt: string, + opts: StreamCodexOptions = {}, +): AsyncIterable { + const timeoutMs = opts.timeoutMs ?? DEFAULT_TIMEOUT_MS; + const proc = Bun.spawn( ["codex", "exec", "--ephemeral", "-s", "read-only", "--json", "-"], - { stdin: "pipe", stdout: "pipe", stderr: "ignore" }, + { stdin: "pipe", stdout: "pipe", stderr: "pipe" }, ); proc.stdin.write(prompt); proc.stdin.end(); - const decoder = new TextDecoder(); - let buffer = ""; - let messageCount = 0; - - for await (const chunk of proc.stdout) { - buffer += decoder.decode(chunk, { stream: true }); - const lines = buffer.split("\n"); - buffer = lines.pop() ?? ""; - - for (const line of lines) { - if (!line.trim()) continue; - try { - const event = JSON.parse(line); - if ( - event.type === "item.completed" && - event.item?.type === "agent_message" && - event.item.text - ) { - if (messageCount > 0) yield "\n\n---\n\n"; - yield event.item.text; - messageCount++; - } - } catch {} + // Keep the tail of stderr so a failure carries an actual diagnostic instead of + // just an exit code. + let stderrTail = ""; + const stderrReader = proc.stderr.getReader(); + const stderrDrained = (async () => { + const decoder = new TextDecoder(); + while (true) { + const { done, value } = await stderrReader.read(); + if (done) break; + stderrTail = (stderrTail + decoder.decode(value, { stream: true })).slice( + -MAX_STDERR_CHARS, + ); } - } + })().catch(() => {}); + + let timedOut = false; + let fireTimeout: () => void = () => {}; + const timeoutFired = new Promise((resolve) => { + fireTimeout = () => resolve(TIMED_OUT); + }); + const timer = setTimeout(() => { + timedOut = true; + proc.kill(); + // Killing codex is not enough to end the read: a grandchild can hold the + // pipe open, so unblock the loop explicitly. + fireTimeout(); + }, timeoutMs); + + const stdoutReader = proc.stdout.getReader(); + + try { + const decoder = new TextDecoder(); + let buffer = ""; + let messageCount = 0; - if (buffer.trim()) { - try { - const event = JSON.parse(buffer); - if ( - event.type === "item.completed" && - event.item?.type === "agent_message" && - event.item.text - ) { + while (true) { + const result = await Promise.race([stdoutReader.read(), timeoutFired]); + if (result === TIMED_OUT || result.done) break; + + buffer += decoder.decode(result.value, { stream: true }); + const lines = buffer.split("\n"); + buffer = lines.pop() ?? ""; + + for (const line of lines) { + if (!line.trim()) continue; + const text = agentMessageText(line); + if (text === null) continue; if (messageCount > 0) yield "\n\n---\n\n"; - yield event.item.text; + yield text; + messageCount++; } - } catch {} - } + } + + if (timedOut) { + throw new Error( + `codex timed out after ${timeoutMs}ms${stderrDetail(stderrTail)}`, + ); + } - const exitCode = await proc.exited; - if (exitCode !== 0) { - throw new Error(`codex exited with code ${exitCode}`); + if (buffer.trim()) { + const text = agentMessageText(buffer); + if (text !== null) { + if (messageCount > 0) yield "\n\n---\n\n"; + yield text; + messageCount++; + } + } + + const exitCode = await proc.exited; + await stderrDrained; + + if (exitCode !== 0) { + throw new Error( + `codex exited with code ${exitCode}${stderrDetail(stderrTail)}`, + ); + } + } finally { + clearTimeout(timer); + // No-ops once it has exited; prevents an orphaned child and dangling reads + // if the consumer abandons the stream early. + proc.kill(); + stdoutReader.cancel().catch(() => {}); + stderrReader.cancel().catch(() => {}); } } diff --git a/webhooks/granola.ts b/webhooks/granola.ts index da54372..d6e4c65 100644 --- a/webhooks/granola.ts +++ b/webhooks/granola.ts @@ -10,7 +10,10 @@ type GranolaPayload = { attendees?: string[]; }; +/** How long a completed meeting stays deduped. */ const DEDUP_TTL_SECONDS = 60 * 60 * 24 * 7; +/** How long an in-flight claim is held before a retry may take over. */ +const LEASE_TTL_SECONDS = 15 * 60; export function granolaWebhook(ctx: WebhookContext): WebhookRoute { const channelId = process.env.GRANOLA_SLACK_CHANNEL_ID; @@ -54,13 +57,22 @@ export function granolaWebhook(ctx: WebhookContext): WebhookRoute { return new Response("Missing required fields: meeting_id, title, transcript", { status: 400 }); } + // Claim a short lease rather than the full completion marker: if this + // process dies mid-summary the lease expires and the sender's retry can + // get through, instead of the meeting being silently dropped for a week. const dedupKey = `webhook:granola:${payload.meeting_id}`; - const claimed = await redis.send("SET", [dedupKey, "1", "NX", "EX", String(DEDUP_TTL_SECONDS)]); + const claimed = await redis.send("SET", [ + dedupKey, + "processing", + "NX", + "EX", + String(LEASE_TTL_SECONDS), + ]); if (claimed !== "OK") { return new Response("Duplicate (already processed)", { status: 200 }); } - void processGranolaMeeting(ctx, channelId, payload as GranolaPayload); + void processGranolaMeeting(ctx, channelId, payload as GranolaPayload, dedupKey); return new Response("OK", { status: 200 }); }, @@ -71,6 +83,7 @@ async function processGranolaMeeting( ctx: WebhookContext, channelId: string, payload: GranolaPayload, + dedupKey: string, ) { const channel = ctx.bot.channel(`slack:${channelId}`); const attendees = Array.isArray(payload.attendees) ? payload.attendees : []; @@ -94,6 +107,13 @@ ${payload.transcript}${payload.notes ? `\n\n--- Notes ---\n${payload.notes}` : " await channel.post(streamCodex(prompt)); } catch (err) { console.error("[granola webhook] processing failed:", err); + // Release the lease so the sender can retry. A retry after a partial post + // can duplicate output, which is preferable to losing the summary entirely. + try { + await redis.send("DEL", [dedupKey]); + } catch (delErr) { + console.error("[granola webhook] failed to release dedup lease:", delErr); + } const msg = err instanceof Error ? err.message : String(err); try { await channel.post( @@ -102,5 +122,15 @@ ${payload.transcript}${payload.notes ? `\n\n--- Notes ---\n${payload.notes}` : " } catch (notifyErr) { console.error("[granola webhook] failure notify also failed:", notifyErr); } + return; + } + + // Promote the lease to a completion marker only once the summary landed. + // A failure here is not worth alarming Slack about: the worst case is the + // lease expiring and a later retry re-posting the summary. + try { + await redis.send("SET", [dedupKey, "done", "EX", String(DEDUP_TTL_SECONDS)]); + } catch (err) { + console.error("[granola webhook] failed to mark meeting complete:", err); } } From e2fb35082aa01554668e677afa748305a59fecac Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 23:39:53 +0000 Subject: [PATCH 7/8] Fix lease-ownership race and unbounded pipe waits Review found two races in the previous commit. The dedup lease was released and promoted unconditionally, so a run that outlived its 15-minute lease could clobber the claim a retry had already taken: both jobs post, and the retry loses the ability to clean up. Each claim now carries a unique token and promote/release run as a Lua compare-and-set, so a stale run's writes are no-ops. Waiting for stdout EOF also hung whenever a descendant inherited the pipe and outlived codex. Reported for stderr, but stdout is the worse case: a fully successful run blocked until the 10-minute timeout and was then reported as a timeout. Stop reading stdout once the process has exited and its buffered output has drained, and bound the stderr wait in the failure path, where it is only needed to enrich the error. Verified against a local Redis (stale promote/release leave the retry's lease intact) and a stubbed codex: descendant-held pipes now finish in ~1s on success and ~6s on failure, with 200 buffered messages arriving intact. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZdVoF8VB8L1q4zUkgKSM1 --- lib/codex.ts | 36 +++++++++++++++++++++++++++++++++--- webhooks/granola.ts | 23 +++++++++++++++++++---- 2 files changed, 52 insertions(+), 7 deletions(-) diff --git a/lib/codex.ts b/lib/codex.ts index 7428fd5..3760e73 100644 --- a/lib/codex.ts +++ b/lib/codex.ts @@ -1,5 +1,9 @@ const DEFAULT_TIMEOUT_MS = 10 * 60 * 1000; const MAX_STDERR_CHARS = 2000; +/** How long to wait for stderr to finish draining before reporting a failure. */ +const STDERR_GRACE_MS = 5000; +/** How long to keep reading stdout after codex exits, to collect buffered output. */ +const POST_EXIT_DRAIN_MS = 1000; export type StreamCodexOptions = { /** Wall-clock budget for the whole run; the process is killed when it elapses. */ @@ -27,6 +31,7 @@ function stderrDetail(stderr: string): string { } const TIMED_OUT = Symbol("timed-out"); +const DRAIN_OVER = Symbol("drain-over"); export async function* streamCodex( prompt: string, @@ -69,16 +74,28 @@ export async function* streamCodex( fireTimeout(); }, timeoutMs); + // Once codex has exited, everything it will ever emit is already sitting in + // the pipe buffer. Waiting for EOF instead would hang whenever a descendant + // inherited stdout and outlives it. + const drainOver = proc.exited + .then(() => Bun.sleep(POST_EXIT_DRAIN_MS)) + .then(() => DRAIN_OVER); + const stdoutReader = proc.stdout.getReader(); try { const decoder = new TextDecoder(); let buffer = ""; let messageCount = 0; + let pendingRead: ReturnType | null = null; while (true) { - const result = await Promise.race([stdoutReader.read(), timeoutFired]); - if (result === TIMED_OUT || result.done) break; + pendingRead ??= stdoutReader.read(); + const result = await Promise.race([pendingRead, timeoutFired, drainOver]); + // Either sentinel ends the loop; `timedOut` distinguishes them below. + if (typeof result === "symbol") break; + pendingRead = null; + if (result.done) break; buffer += decoder.decode(result.value, { stream: true }); const lines = buffer.split("\n"); @@ -110,9 +127,22 @@ export async function* streamCodex( } const exitCode = await proc.exited; - await stderrDrained; + + // The kill may have landed after stdout closed rather than during the read. + if (timedOut) { + throw new Error( + `codex timed out after ${timeoutMs}ms${stderrDetail(stderrTail)}`, + ); + } if (exitCode !== 0) { + // Only the failure path needs stderr, and the wait for it must be bounded: + // a descendant can hold the pipe open after codex itself has exited. + await Promise.race([ + stderrDrained, + timeoutFired, + Bun.sleep(STDERR_GRACE_MS), + ]); throw new Error( `codex exited with code ${exitCode}${stderrDetail(stderrTail)}`, ); diff --git a/webhooks/granola.ts b/webhooks/granola.ts index d6e4c65..ec3ecc4 100644 --- a/webhooks/granola.ts +++ b/webhooks/granola.ts @@ -15,6 +15,13 @@ const DEDUP_TTL_SECONDS = 60 * 60 * 24 * 7; /** How long an in-flight claim is held before a retry may take over. */ const LEASE_TTL_SECONDS = 15 * 60; +// Promote/release only if this run still owns the key. A run that outlives its +// lease must not clobber the claim a retry has since taken. +const PROMOTE_IF_OWNED = + "if redis.call('GET', KEYS[1]) == ARGV[1] then return redis.call('SET', KEYS[1], 'done', 'EX', ARGV[2]) else return nil end"; +const RELEASE_IF_OWNED = + "if redis.call('GET', KEYS[1]) == ARGV[1] then return redis.call('DEL', KEYS[1]) else return 0 end"; + export function granolaWebhook(ctx: WebhookContext): WebhookRoute { const channelId = process.env.GRANOLA_SLACK_CHANNEL_ID; const secret = process.env.WEBHOOK_SECRET; @@ -61,9 +68,10 @@ export function granolaWebhook(ctx: WebhookContext): WebhookRoute { // process dies mid-summary the lease expires and the sender's retry can // get through, instead of the meeting being silently dropped for a week. const dedupKey = `webhook:granola:${payload.meeting_id}`; + const leaseValue = `processing:${crypto.randomUUID()}`; const claimed = await redis.send("SET", [ dedupKey, - "processing", + leaseValue, "NX", "EX", String(LEASE_TTL_SECONDS), @@ -72,7 +80,7 @@ export function granolaWebhook(ctx: WebhookContext): WebhookRoute { return new Response("Duplicate (already processed)", { status: 200 }); } - void processGranolaMeeting(ctx, channelId, payload as GranolaPayload, dedupKey); + void processGranolaMeeting(ctx, channelId, payload as GranolaPayload, dedupKey, leaseValue); return new Response("OK", { status: 200 }); }, @@ -84,6 +92,7 @@ async function processGranolaMeeting( channelId: string, payload: GranolaPayload, dedupKey: string, + leaseValue: string, ) { const channel = ctx.bot.channel(`slack:${channelId}`); const attendees = Array.isArray(payload.attendees) ? payload.attendees : []; @@ -110,7 +119,7 @@ ${payload.transcript}${payload.notes ? `\n\n--- Notes ---\n${payload.notes}` : " // Release the lease so the sender can retry. A retry after a partial post // can duplicate output, which is preferable to losing the summary entirely. try { - await redis.send("DEL", [dedupKey]); + await redis.send("EVAL", [RELEASE_IF_OWNED, "1", dedupKey, leaseValue]); } catch (delErr) { console.error("[granola webhook] failed to release dedup lease:", delErr); } @@ -129,7 +138,13 @@ ${payload.transcript}${payload.notes ? `\n\n--- Notes ---\n${payload.notes}` : " // A failure here is not worth alarming Slack about: the worst case is the // lease expiring and a later retry re-posting the summary. try { - await redis.send("SET", [dedupKey, "done", "EX", String(DEDUP_TTL_SECONDS)]); + await redis.send("EVAL", [ + PROMOTE_IF_OWNED, + "1", + dedupKey, + leaseValue, + String(DEDUP_TTL_SECONDS), + ]); } catch (err) { console.error("[granola webhook] failed to mark meeting complete:", err); } From dcce8fe452d9599c74d6a25a5a0501092ce45e7d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 23:52:31 +0000 Subject: [PATCH 8/8] Measure the post-exit stdout grace only while a read is pending MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The grace period was armed once when codex exited, so it also counted time the generator spent suspended at a yield waiting on Slack. A consumer slower than the window could resume to find the grace already expired and drop still-buffered messages. Arm it fresh per read instead, so it measures only time spent waiting on the pipe. Truncation was not reproducible before this change — an already queued read wins the race by argument order — but that made correctness depend on microtask ordering rather than on the intended condition. Verified with a stubbed codex that exits mid-stream while a descendant holds the pipe, against consumers pausing 1.5s and 5s between chunks: all messages arrive in both cases. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HZdVoF8VB8L1q4zUkgKSM1 --- lib/codex.ts | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/codex.ts b/lib/codex.ts index 3760e73..a870905 100644 --- a/lib/codex.ts +++ b/lib/codex.ts @@ -32,6 +32,7 @@ function stderrDetail(stderr: string): string { const TIMED_OUT = Symbol("timed-out"); const DRAIN_OVER = Symbol("drain-over"); +const EXITED = Symbol("exited"); export async function* streamCodex( prompt: string, @@ -77,9 +78,11 @@ export async function* streamCodex( // Once codex has exited, everything it will ever emit is already sitting in // the pipe buffer. Waiting for EOF instead would hang whenever a descendant // inherited stdout and outlives it. - const drainOver = proc.exited - .then(() => Bun.sleep(POST_EXIT_DRAIN_MS)) - .then(() => DRAIN_OVER); + let exited = false; + const exitSeen = proc.exited.then(() => { + exited = true; + return EXITED; + }); const stdoutReader = proc.stdout.getReader(); @@ -91,8 +94,17 @@ export async function* streamCodex( while (true) { pendingRead ??= stdoutReader.read(); - const result = await Promise.race([pendingRead, timeoutFired, drainOver]); - // Either sentinel ends the loop; `timedOut` distinguishes them below. + // The post-exit grace is armed fresh for each read, so it measures only + // time spent waiting on the pipe. Arming it once at exit would let a slow + // Slack consumer burn the window while suspended at a yield below, and a + // still-draining pipe would look empty. + const result = await Promise.race([ + pendingRead, + timeoutFired, + exited ? Bun.sleep(POST_EXIT_DRAIN_MS).then(() => DRAIN_OVER) : exitSeen, + ]); + if (result === EXITED) continue; // re-race with the grace armed + // Either remaining sentinel ends the loop; `timedOut` distinguishes below. if (typeof result === "symbol") break; pendingRead = null; if (result.done) break;