feat: bound gateway requests, stdin sends, and normalized transcripts - #20
feat: bound gateway requests, stdin sends, and normalized transcripts#20ImL1s wants to merge 15 commits into
Conversation
🦋 Changeset detectedLatest commit: 26c54c5 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: e4fbcb19a0
ℹ️ 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".
When the gateway returns a non-OK status but its body stream stalls, requestJson previously raced readJson against the deadline, causing the known HTTP error to be misclassified as GATEWAY_TIMEOUT with unknown delivery effect. Now requestJson checks res.ok immediately after the fetch resolves and returns before attempting to read the body. This ensures client errors (4xx) and server errors (5xx) are reported with their actual status codes even when the response body never completes. Addresses Codex review feedback on PR ScriptedAlchemy#20.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: edd6a36b9c
ℹ️ 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".
Three independent fixes addressing all remaining P2 suggestions from the Codex automated review on PR ScriptedAlchemy#20: 1. Treat JSON null sendPrompt responses as unknown delivery — null provides no confirmation the prompt was accepted, so it follows the same path as empty or non-JSON success responses. 2. Preserve leading UTF-8 BOM during stdin decoding (ignoreBOM: true) so the whitespace check rejects it rather than silently stripping the first character of the message. 3. Include entry.sender in the normalizer's role candidate set so transcripts with an explicit sender field produce the correct user/assistant role instead of unknown.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55302127a6
ℹ️ 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".
When a transcript entry carries an explicit but unsupported role
string (e.g. role: 'system' or sender: 'system') alongside a
user/assistant carrier from a nested field, the normalizer now
returns 'unknown' instead of letting the nested carrier win.
This only applies to strict role fields (role, sender, nested.role,
nested.type). Loose category carriers like entry.kind ('send-message')
and entry.type ('text') continue to contribute user/assistant roles
without vetoing, since they are multi-purpose identifiers.
Addresses Codex review round 3 on PR ScriptedAlchemy#20.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a041a86e6f
ℹ️ 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".
Two fixes from Codex review round 4: 1. Abort the controller when returning early on non-OK HTTP status. Without this, the response body stream and underlying socket remain active indefinitely, potentially exhausting the connection pool on repeated failures. 2. Treat JSON false sendPrompt responses as unknown delivery. A boolean cannot confirm prompt acceptance, so it follows the same unknown-delivery path as null and empty body responses. Non-send methods continue to pass false through unchanged.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ba2e655c0
ℹ️ 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".
…iers Two fixes from Codex review round 5: 1. Require sendPrompt data to be a non-null object for a successful acknowledgement. Any non-object JSON value (null, false, 0, '') is treated as unknown delivery. This replaces the growing list of individual falsy-value checks with a positive shape assertion. 2. Malformed non-string strict role carriers (e.g. role: 42, role: true) now veto role inference the same way unsupported string roles do. Empty strings are treated as absent (no veto).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6533c79b
ℹ️ 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".
Two fixes from Codex review round 6: 1. Exclude arrays from valid sendPrompt acknowledgements. Since typeof [] === 'object', the previous positive shape check still accepted empty arrays. Added Array.isArray guard. 2. Reject non-string, non-object text carrier values (e.g. text: 42) as malformed evidence instead of silently filtering them out and falling back to secondary carriers. This preserves the fail-closed normalizer contract.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cecb43b3e1
ℹ️ 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".
When content arrays contain unsupported primitives (e.g. ["hello", 42]), the array branch silently filters them out. Validate each element and reject non-string, non-object primitives to preserve fail-closed.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b906bb8dd6
ℹ️ 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".
- sendPrompt now requires response to have at least one key
- Strict text carriers (text, prompt, preview) with opaque objects
(no TEXT_CARRIERS subkeys) are rejected as malformed evidence
- content/message carriers continue to accept rich objects like
{ type: 'image' }
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31b03adbda
ℹ️ 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".
Extract sendPrompt validation into isAffirmativeSendAck helper. Reject responses with ok:false, success:false, or an error field since they explicitly indicate the gateway rejected the prompt despite returning a 2xx status code.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 26c54c5b33
ℹ️ 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".
When ok or success fields are present but not exactly boolean true (e.g. ok:'false', success:[], ok:0), reject the response as unknown delivery instead of trusting truthy coercion.
1 participant
Three related changes that came out of driving
gbotfrom scripts and cron rather than by hand. Each is independent in the diff, so I'm happy to split this into separate PRs if you'd prefer to review them apart.1. Bound gateway requests and stop leaking response bodies
ensureSandbox/gatewayCallused barefetchwith no deadline, so a hung gateway hung the CLI indefinitely, andreadJsonfolded an unparseable body into{ raw: text }which then went into the thrown error message.AbortController+Promise.raceso a stalled body read can't outlive it.ensureSandboxandgatewayCallaccept an optional positivetimeoutMsin their final options argument.redirect: "error"— a redirected gateway call fails instead of silently following.sendPromptthat hits a timeout, a network failure, HTTP 408/5xx, or an invalid/empty success response throws witheffect: "unknown"and a message saying delivery is unknown and not to resend automatically. Nothing is retried for you, and a fresh CLI send uses a new client nonce, so re-invoking is not a deduplicated retry — the README says to read the thread back and verify first.message.content/message.textcarriers, which previously printed as empty lines.2.
send <target> --stdinPassing a message as an argument puts it in the process argument list, where anything reading
pscan see it, and shell quoting mangles exact UTF-8.--stdinreads the message from standard input instead, validated strictly: non-empty, valid UTF-8, no NUL bytes, no surrounding whitespace, at most 64 KiB, and not combined with a positional message. The README notes usingprintf %srather thanechowhen a trailing newline isn't intended.3. A normalized transcript contract for adapters
--json thread/--json chatreturned the raw gateway response, whose shape varies, so every integration wrote its own defensive parser.--normalizedreturns a stable shape: the target identity plus messages withid, an explicituser/assistant/unknownrole, and text.The normalizer fails closed rather than guessing — malformed containers, malformed target/message identities, and conflicting or malformed ID carriers are rejected instead of being coerced into something plausible. Normalized text is kept separate from display stringification, so non-text content is never synthesized into JSON that looks like a message body. Without
--normalized, JSON output is unchanged.Testing
node --test— 71 tests, 0 failures (674 new lines of tests:test/gateway.test.jscovers the deadline, redirect rejection, and each ambiguous-send classification;test/send-stdin.test.jscovers the stdin validation matrix;test/transcript.test.jscovers the normalizer's fail-closed paths)npx publint@0.3.24— All goodnpm pack+ global install +gbot --helpsmoke test — passesbots list,thread,--json thread --normalized, andsend --stdinChangesets
Two included: a
patchfor the gateway/transcript fix and aminorfor the--stdinflag and--normalizedcontract.🤖 Generated with Claude Code