P1 HOLD fixes: streaming caps, absolute deadline, JSON errors, ack-gated receipts - #43
Conversation
…read reads Closes #34 (delivery states: rejected/accepted/unknown, IDs preserved), #35 (idempotent shutdown, handshake/fragmentation/budgets, no leaks), #36 (truncation metadata, bounded full reads, safe normalize, 1-200 limits). Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
… errors, ack validation, bounded full reads Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 3fda090 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
Lines 534 to 535 in 85b212a
Warn plain-text users when send delivery is unknown
When sendPrompt receives a successful response without a messageId, it now correctly returns delivery: "unknown", but non-JSON gbot send still unconditionally prints Sent to ... with no no-receipt warning. Users of the default output therefore cannot distinguish a confirmed send from an unknown delivery and are not told to inspect the thread before retrying, unlike MCP callers. Include the unknown-delivery guidance in the plain-text branch.
ℹ️ 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".
| if (cmd === "send") { | ||
| const ref = sub; | ||
| if (hasFlag(rest, "--json")) json = true; | ||
| if (hasFlag(rest, "--json")) { json = true; jsonErrors = true; } |
There was a problem hiding this comment.
When callers use the supported command-local form gbot send ... --json or gbot thread ... --json, this flag is not processed until after openBackend() has run. If backend initialization fails—for example, because the agents directory is missing or the app gateway session is unusable—jsonErrors remains false and the CLI emits plaintext instead of the promised structured JSON error. Parse these command-local flags before opening the backend.
Useful? React with 👍 / 👎.
| if (bytes > maxBytes) { | ||
| try { await reader.cancel(); } catch { /* already closed */ } | ||
| throw new GatewayError("Gateway response too large (over " + maxBytes + " bytes)"); |
There was a problem hiding this comment.
If sendPrompt receives a definite 4xx response whose body exceeds the new cap, this throw creates a GatewayError without the already-known HTTP status. The send error classifier consequently treats it as delivery: "unknown" rather than "rejected", even though the server explicitly rejected the request. Propagate res.status when capped reading fails, or classify using the response status before reading its body.
Useful? React with 👍 / 👎.
Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
2 participants
Why: PR #40 (first revision) was merged while the HOLD fixes were in flight. This follow-up from the same branch lands the six GPT-6 Pro hold items on top of current main (incl. the history feature), plus the two remaining contract gaps from the #43 review. Same potato-mode rules: shortest diffs, no new deps, no approval flips.
Hold fixes (vs the merged rev-1 content):
res.text()-then-pretend).close(); malformed RPC (incl. JSONnull) routed throughfailAll.--jsonfailures emit structured stderr JSON witherror,delivery,threadId/turnId/targetId.unknownwithout a confirmed string receipt — never a silent accept;gbot_sendreflects the gateway verdict.truncated/fullLengthmetadata; remainder retrievable viagbot thread --full/--json. Route-schema bound ceiling documented withponytail:(zod still enforces 1–200).hasFlaghelper and keeping itssaveHistorycalls; transcript keeps the preview-last tweak.Contract gaps fixed in this revision:
--jsonis parsed before dispatch/backend init:codex sendpeels a trailing--jsononly (mid-message tokens stay content;--protects the rest), structured subcommands andthread/send/historyhonor it, so early auth failures also emit JSON. Both placements tested with the transmitted prompt asserted free of--json.Correlation IDs:
sendPromptreuses the existingclientNonce/replyToId; the full send/execution correlation envelope stays in #37 and is not claimed here.Verification: root
npm test108/108 green; pluginnpm run checkgreen (validate, build, artifact, typecheck, 11 route-unit tests).Closes #34 #35 #36 as the remainder of the P1 follow-ups. Out of scope: #37 correlation envelope, #38 native queue, #39 status diagnostics, Desktop private pipes. No
approval_policychanges; public app-server JSON-RPC only.