Repository navigation
Harden the approval machine - #2
Conversation
Codex hardening, first slice. The daily order budget is reserved atomically: count and insert serialize under an advisory lock, so several verified approvals racing to submit cannot spend the last slot twice. Blank or junk cap variables now mean the defaults instead of a zero-dollar ceiling (Number of an empty string is 0, the template trap). An approval whose code email fails to send voids itself while still counting toward the rate caps, since excluding voided rows would let re-requests reset the meter. And cancel reopens the cart only when the void actually happened; a consumed approval answers 409.
Codex hardening, second slice. Submission now seals the cart right after consuming the approval, and every mutating op (add, quantity, remove, tip, delete, fulfillment flip) refuses sealed carts with a plain answer, so nothing can drift between the final revalidation and the charge. A snapshot refresh in flight during that window can no longer stomp the seal back to open. Shrinking a line now demands the menu id BEFORE removing, so a grocery line without one decrements safely instead of vanishing. And pickup approvals check pickup availability the way delivery always did; the preview schema carried the flag, normalization just dropped it.
Codex hardening, third slice. Approval ids are sequential, and on a shared-gate instance that made them another guest's lever: five wrong guesses could lock a stranger's approval, cancel could void it. The request tool now returns an HMAC capability bound to the approval id; verify, cancel, and status all demand it before touching the row. The inbox code remains the only key that moves money. The wake is durable too: the bell rings up to three times, the card says plainly when nobody answered, and the agent knows a manual submit is safe because consumption refuses anything unverified.
Codex hardening, fourth slice. Transcripts freeze tool output forever, but the cart keeps living: cards now hydrate from a snapshot GET on mount, so reloads and older copies of the same cart converge on what is actually true, and a sealed cart renders sealed, controls frozen, header saying why. The ops route cancels its run when the deadline fires, so a timed-out edit can no longer mutate the cart after the card was told it failed. And a submit retry that lands on an already-used approval refreshes pending or unknown outcomes against live order status instead of repeating stale news.
Codex hardening, fifth slice. With DD_HIDE_PERSONAL on, the browse card used to render the owner's addresses as zero saved and their history as zero orders, presenting privacy as an empty account; the private answer now says what it is. CartToolCard, ZoomableImage, and the private face all gained the stories the constitution requires, and AGENTS.md records that data backfills legitimately ride pnpm db:generate --custom, which the review misread as handwritten migration SQL.
Found live on production: dd-cli is built for a renderer with widgets, and when its backend wants an address choice, the widget's stage directions arrive as message text. Our card repeated them as data and the agent pointed the user at a picker that only exists in DoorDash's own app. Search results now translate widget-speak into the honest fact (saved addresses are the only delivery targets), keyed on the structural needs_address flag as well as the text, and the agent knows it has no widgets and that only the owner can save a new address, in the DoorDash app itself.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: b3a9bdaf71
ℹ️ 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".
| }): Promise<CartOpResult> { | ||
| const snapshot = await getCartSnapshot(input.cartUuid); | ||
| if (!snapshot) throw new Error("unknown cart"); | ||
| assertUnsealed(snapshot.status); |
There was a problem hiding this comment.
When an edit has already read an open status at this check, submission can change the row to placing while the edit continues toward its DoorDash mutation. If submission revalidates the cart before that mutation but calls orderSubmit afterward, the approved hash passes and the altered cart can be charged. Preserving placing during snapshot refresh only protects the local row, so the mutation and submission seal need serialization or versioned conflict detection.
Useful? React with 👍 / 👎.
| await setCartStatus(approval.cartUuid, "placing"); | ||
| const snapshot = await getCartSnapshot(approval.cartUuid); | ||
| if (!snapshot) return { refused: "the cart is gone" }; |
There was a problem hiding this comment.
If cartShow, orderPreview, or the database reservation throws after this status update but before an order row is created, the approval is already consumed and the cart remains placing. A retry reaches the alreadyConsumed branch with no ddOrders row and returns a refusal, while every cart mutation rejects the lingering placing state, permanently stranding the cart even though orderSubmit was never called. Pre-submit failures need to record a recoverable outcome and transition the cart out of the sealed state.
Useful? React with 👍 / 👎.
| setCart((current) => ({ | ||
| ...current, | ||
| ...(durable.items ? { items: durable.items } : {}), | ||
| ...(durable.quote !== undefined ? { quote: durable.quote } : {}), | ||
| ...(durable.tipCents !== undefined | ||
| ? { tipCents: durable.tipCents } | ||
| : {}), | ||
| ...(durable.fulfillment ? { fulfillment: durable.fulfillment } : {}), | ||
| })); |
There was a problem hiding this comment.
After this effect adopts a newer durable snapshot, any subsequent edit rejection reaches the existing catch path and calls setCart(data), where data is the frozen transcript prop. A card hydrated to current items, pricing, or tip therefore snaps back to stale generated values after a failed or timed-out edit even though durable state did not change. Retain the last durable pre-edit snapshot or rehydrate on failure instead.
AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
| await runs.cancel(handle.id).catch(() => {}); | ||
| return Response.json({ error: "edit timed out" }, { status: 504 }); |
There was a problem hiding this comment.
When the Trigger cancellation request rejects, this catch discards the only evidence that cancellation did not happen and still returns a definitive 504. The worker may remain queued or running and apply the edit after the card has rolled back and told the user it failed, leaving the displayed cart inconsistent with durable state. Propagate an uncertain result or keep polling and verify the final snapshot before reporting failure.
AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
| const approvalId = output?.approvalId; | ||
| const token = output?.token ?? ""; |
There was a problem hiding this comment.
Persisted tool outputs created before this commit contain an approvalId but no token, so this fallback sends an empty capability to the newly mandatory status, verify, and cancel routes. The status client maps the resulting 403 to voided, which prevents still-live pre-deployment codes from being entered or cancelled and makes historical verified or consumed cards claim they are no longer live. Add versioned handling or an authenticated capability-upgrade path for legacy outputs.
AGENTS.md reference: AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
| for (let attempt = 0; attempt < 3 && !woke; attempt++) { | ||
| try { | ||
| await sendSessionText( | ||
| row.conversationId, | ||
| `[order approved, approval ${approvalId}] The owner entered the code. Place the order with doordash-submit.`, | ||
| ); | ||
| woke = true; | ||
| } catch { |
There was a problem hiding this comment.
If a session send commits but its response is lost, the call throws and this loop invokes sendSessionText again. That wrapper creates a fresh crypto.randomUUID() for every invocation, so the retries are distinct durable messages rather than idempotent attempts and can wake multiple agent turns for one verification, producing duplicate submission outcomes and transcript entries even though approval consumption prevents a second charge. Reuse a single message identity across all three attempts.
Useful? React with 👍 / 👎.
Codex round two, the declared final pass. Pre-flight failures after consumption (the CLI, the database) no longer strand a sealed cart with a spent approval and no order row; the cart reopens and the tool says plainly that the approval is spent. Right before the charge, the seal stamp is checked one last time, so a mutation that slipped past the seal while pre-flight CLI calls were in the air aborts the order instead of charging a cart nobody approved; the residual window is the submit call itself, which nothing outside DoorDash can close. A failed edit now rolls the card back to the freshest settled truth instead of the frozen transcript numbers, a timed-out edit looks once more after cancelling in case the run finished anyway, and wake retries reuse one message id so a lost response cannot wake the agent twice.
|
Round-two triage, five of six landed in
Rejected: legacy approval cards. Pre-deployment tool outputs carry no capability token, so their cards will read as no-longer-live. By inspection there are zero live pre-deployment approvals (codes die in fifteen minutes; every historical row is expired, voided, or consumed), and for dead approvals the legacy face is truthful. Versioned capability upgrades would be complexity in the money path for an empty set. Per the repo's review-loop convention this was the declared final round; anything further that is not money-path lands in a follow-up. |
This branch was successfully deployed
1 participant
The approval machine shipped in #1 with a public triage of Codex's review; this PR lands the queued stack, plus one bug found live on production. Six commits, most severe first:
Not in this PR, with reasoning on #1: cart-state readback into the next agent turn (both money steps re-read the live cart, so staleness cannot reach a charge) and address selection among saved addresses (its own research and PR).
Gates green per commit: lint, typecheck, 296 tests, build.