Apply optional polish from PR #1 sidecar review - #2
Open
shadowbrush wants to merge 1 commit into
Open
Conversation
The four blocking review items already landed in the merged PR #1. This addresses the remaining optional "Additional observations": - resend.ts: log a warning (status only, never the body) when a non-JSON provider response is swallowed, to aid debugging the upstream_unreachable path. - errors.ts + tsconfig.json: add `override` to the four extraFields() overrides and enable noImplicitOverride so it stays enforced. - README.md: document a manual staging smoke test against real Resend before cutting a release. Skipped the awareness-only items (idempotency-key charset is intentionally tighter than Resend's spec; existing log-leak assertion is sufficient). Typecheck clean, 31 tests pass. Co-Authored-By: Claude Opus 4.8 <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.
Overview
Follow-up to the PR #1 sidecar review. The four blocking items the review recommended (prototype-safe operation lookup, 128kb body limit, parser-error mapping to
validation_error, closing out the Zod-v4 false positives) already landed in merged PR #1. This PR takes the remaining optional "Additional observations."What changed
src/resend.ts— Log alogger.warn(status only, never the body) when a non-JSON provider response is swallowed. Previously the failure was silent until id-extraction mapped it toupstream_unreachable; the warning aids debugging without leaking payload contents.src/errors.ts+tsconfig.json— Add theoverridekeyword to the fourextraFields()overrides and enablenoImplicitOverride, so the compiler enforces it rather than leaving it to chance if the base method ever changes.README.md— Document a manual staging smoke test against real Resend before cutting a release (the suite uses an injected fakefetchand never hits the network).Deliberately skipped
stdout.writetest spy — the existing log-leak assertion already covers body + logs; no change needed.Verification
npm run typecheck— clean (confirmsnoImplicitOverrideis satisfied)npm test— 31/31 pass🤖 Generated with Claude Code