feat(codex): machine-readable contract, delivery envelope, busy-thread refusal (#37 #38 #39) - #46
Conversation
🦋 Changeset detectedLatest commit: 88ce32b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 480a145bb8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| session = await openSession(env); | ||
| } catch (err) { | ||
| if (err instanceof CodexRouteError || err instanceof CodexProtocolError) { | ||
| return { ...base, reachable: false, mode: err.mode, message: err.message }; |
There was a problem hiding this comment.
When the socket is absent, inaccessible, or fails to connect, this return path emits no schema property at all because schema is only constructed in the reachable branch. Consumers of the new status contract therefore cannot read schema.compatibility as "unknown" during the most common failure cases; include the unknown schema object in the base result.
Useful? React with 👍 / 👎.
| if (/^Codex app-server refused the WebSocket upgrade|^Codex app-server (violated|sent|handshake)/.test(message)) { | ||
| return new CodexRouteError(message, "handshake-failed"); | ||
| } | ||
| return err; |
There was a problem hiding this comment.
If the peer completes the WebSocket upgrade but never answers initialize, rejects that RPC, or sends an oversized frame, none of the current patterns match and routeError returns the original error. Consequently gbot codex status --json throws through codexStatus and prints a generic stderr error without the promised stable mode; map these initialization/protocol failures to handshake-failed or bad-response instead.
Useful? React with 👍 / 👎.
| function assertThreadIdle(resumed, threadId) { | ||
| const status = resumed && resumed.thread && resumed.thread.status; | ||
| const type = status && typeof status.type === "string" ? status.type : "unknown"; |
There was a problem hiding this comment.
When thread/resume omits status or a newer daemon returns an unrecognized status type, this fallback produces "unknown", which assertThreadIdle accepts and immediately follows with turn/start. If that unknown state represents an active turn, the request steers existing human work—the exact behavior this change is intended to prevent. Treat any state other than a positively recognized safe state as a bad response or busy refusal.
Useful? React with 👍 / 👎.
| assertThreadAllowed(threadId, env); | ||
| const body = withEnvelopeHeader(text, envelope, env); | ||
| const { client } = await openSession(env); |
There was a problem hiding this comment.
When an allowlist rejects the thread or openSession fails because the socket is unavailable, these calls throw before entering the surrounding try, and those errors do not carry the already-created envelope. A codex send --json rejection therefore lacks messageId, correlationId, and hop, preventing callers from correlating precisely the route failures for which the new receipt contract is intended.
Useful? React with 👍 / 👎.
|
Addressed the review in 1d7fc50:
|
|
Follow-up a9e62bb: |
…, busy-thread refusal Closes #37, #38, #39. - status: socketState, stable failure mode, schema compatibility vs reachability, bounded codex --version probe, desktopAttached unknown; unknown args rejected - list-threads: --cursor paging, --limit 1-200, response shape validation, terminal-control stripping in JSON, normalized status/activeFlags - send: --correlation-id/--reply-to/--hop/--envelope, receipts with messageId (clientUserMessageId), correlationId, hop, maxHops; GROK_BOT_MAX_HOPS relay bound; GROK_BOT_CODEX_THREADS operator allowlist; structured reason/mode in --json failures; active threads refused (busy) instead of steering - docs: README contract + docs/codex-busy-threads.md decision record
…fication, sanitization - #38: Codex 0.154.0 does expose experimental thread/queue/* behind initialize.capabilities.experimentalApi; live-probed add/list/delete. Add `--when-busy queue` (GROK_BOT_CODEX_EXPERIMENTAL=1) -> thread/queue/add with the envelope id, `delivery: "queued"`, and `gbot codex queue <threadId>`; rewrite the decision record accordingly - only idle/notLoaded proceed to turn/start; unknown statuses -> unknown-status - status: phase-based classification (connect EACCES -> permission-denied, initialize failures -> handshake-failed, off-schema init -> reachable:true + bad-response, exit 1); list-threads transport errors carry reason transport - sanitization: single-line fields drop line breaks; header user/host restricted to [A-Za-z0-9_.:-]; structured `source` preserved; entries without string id -> bad-response instead of silent drop - every send rejection (route, allowlist, busy, unknown-status) carries the envelope - CLI: opaque --cursor may start with '-'; `gbot send` peels only a trailing --json and honors `--`; usage errors carry reason "usage" - README/changeset: correct the "every --json failure" claim
…dex per subcommand
a9e62bb to
88ce32b
Compare
* feat: collapse grok-bot-cli into one root Agent Bundle project (#24) One dispatcher, one CI gate. The generated `gbot` CLI (`dist/bin/gbot.mjs`) replaces the hand-written `src/cli.js`; the nested private `plugin/` project and its `file:..` hatch are deleted. `gbot-install` binds `runInstallCli` to the same npm `dist/` so Codex, Claude Code, and Cursor install the `grok-bot` MCP tools and `talk-to-grok-bot` skill from one package. - Domain JS moves to `src/core/`; #46 exit-code derivation and failure document flattening live in `src/core/codex/contract.js`, and `sendToCodexThread` / `codexStatus` return documents instead of throwing. - `src/cli/**` routes: bots, groups, doctor, history, send, thread (dedicated CLI routes, not MCP projections) and codex status/list-threads/send/queue. - `send`, `codex send`, and `codex status` use `exitCode: 'result'`: one JSON document on stdout with `exitCode`; refusals keep `reason`/`delivery`. - Gateway-backed routes redact thrown error text before the framework prints it, matching the MCP tools and the old `fail()`. - Breaking (pre-1.0 minor): Node >= 22.19.0, command-local options only, `--json` reserved before `--`, failure JSON on stdout, usage errors exit 2. Supersedes #47. * chore: reference #50 in the changeset * fix: address change-risk review of the root collapse - withRedactedErrors keeps the error name and own fields (reason, mode, delivery, envelope) so failure documents still classify; `gbot send` usage refusals report reason "usage" again. - codex list-threads / queue return the flat failure document on stdout (reason "bad-response", "experimental-disabled", ...) instead of letting the framework print a stack trace; shared failureDocumentSchema. - bots list / groups list --json are bare arrays again, as before. - codex queue human output uses the old formatter (formatCodexQueue). - README/changeset: drop the removed --instructions alias, name the on|off narrowing, the `chat` history event, stderr failures for non-send commands, and the gbot vs grok-bot bundle rename. - Tests cover send `--` protection, usage reason, and list-threads bad-response documents. --------- Co-authored-by: Ubuntu <zack@ubuntu-main.local>
1 participant
Summary
Closes #37, closes #38, closes #39.
#39 — status / thread discovery contract
codex status --json:socketState(socket|absent|permission-denied|not-a-socket), stable failuremode(socket-absent|permission-denied|not-a-socket|connect-failed|handshake-failed|bad-response|windows-unsupported),schema.compatibility(exact|unverified|unknown) separate fromreachable, bounded 3 scodex --versionprobe (cliVersionProbe),desktopAttached: "unknown"(not observable from the socket). Permission failures no longer collapse into "absent". Unknown args rejected.codex list-threads:--cursorpaging withnextCursor,--limitbounded 1–200, unknown args rejected,thread/list/initialize/thread/resumeshape validation (reason: "bad-response"), text fields stripped of terminal controls in JSON as well as text,statusnormalized to the schema enum,activeFlagsexposed.#37 — attributable routes + loop-safe correlation
codex send/sendtake--correlation-id,--reply-to,--hop,--envelope(before the target). Receipts carrymessageId(sent as Codex's nativeclientUserMessageId),correlationId,replyTo,hop,maxHops.hop >= GROK_BOT_MAX_HOPS(default 4) is refused withreason: "hop-limit"before anything reaches the daemon;--reply-torequires the original correlation id; gbot never auto-acks.GROK_BOT_CODEX_THREADS=id,idallowlist (reason: "route-not-allowed"); socket path only fromCODEX_HOME.mode/reason), every--jsonfailure hasreason.#38 — busy-thread delivery
turn/starton an active thread steers it (turnTriggerdoc), no client request enqueues (onlythread/queue/changed+QueuedSubmission),turn/steerneedsexpectedTurnId.activethreads (reason: "busy") andsystemError(thread-error); never callturn/steer/turn/interrupt; no local queue. Receipts distinguishaccepted/rejected/unknown. Record:docs/codex-busy-threads.md(incl. the resume→start race ceiling).Changeset: patch.
Verification
npm test— 121/121 (13 new tests: permission-denied/not-a-socket/connect-failed status, schema compat, bounded probe timeout, unknown args, cursor paging + sanitized cursor hint, bad-response, JSON text sanitization, envelope/hop/reply validation, GROK_BOT_MAX_HOPS, allowlist, unavailable route, busy/systemError refusal, external-owner/unknown-thread/transport reasons)cd plugin && npm run check— validate/build/artifact/typecheck/11 route tests passgbot codex statusagainst the local 0.154.0 daemon → reachable, schema exact;gbot codex list-threads --limit 2 --jsonreturns 2 threads + nextCursor.