fix: address outstanding Codex review findings#21
Merged
Conversation
…Mode Three fixes flagged by Codex review on earlier PRs (#1, #14): - formToNestedObject: skip keys that yield no segments (e.g. `=x`) so hostile urlencoded input can't make webhook handlers throw. - redactSensitiveText: mask processor codes as `Upay_[REDACTED]` so isFailedStatus still sees the processor failure signal; responses whose only failure indicator is an Upay_* code classify as payment.failed again instead of sumit.trigger.unmapped. - buildCreateDocumentPayload: derive SearchMode from blank-stripped ids so a whitespace-only `id` can't select search-by-ID while ID is omitted from the payload. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The create-document builder never sends Payments rows, so Receipt and InvoiceAndReceipt would be rejected or issued without the payment they document (Codex finding on #13). Point those at the charge endpoints or the SUMIT UI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Only main may run the publish jobs (workflow_dispatch was unrestricted), and the GitHub Packages workflow no longer points installs at npm.pkg.github.com — the publish step configures that registry and its auth itself. Mirrors Codex findings on the sumit-react workflows, which share this shape. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A sweep of all Codex review comments across the repo's PR history found four findings that were never addressed (the getpdf/promotion doc findings from #16 were already fixed in #18). Verified each against main before fixing.
Changes
=x) crashesformToNestedObjectwith aTypeError=xin the query string.Upay_*erases the tokenisFailedStatusrelies on, downgrading failures tosumit.trigger.unmappedUpay_[REDACTED]— the sensitive part is the code, not the processor name. The failure signal survives redaction; test covers an Upay-only failure.customer.idselectsSearchMode: 1whileIDis stripped from the payloadSearchModefrom the blank-stripped identifiers; test asserts fallback toExternalIdentifier(mode 2).Receipt,InvoiceAndReceipt) silently unsupported — builder never sendsPaymentsPayments?: never[]already enforces it.Also hardens the publish workflows (main-only ref guard; GitHub Packages installs resolve from npmjs) — Codex flagged these patterns on the sumit-react twins (#4, #7) and this repo shares them.
Verification
pnpm typecheckclean,pnpm test32/32 (3 new regression tests),pnpm buildgreen. One existing test expectation updated for the newUpay_[REDACTED]mask.🤖 Generated with Claude Code