Reject invalid structured values in storage - #1865
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis change adds schema-backed stored JSON validation, replaces duration normalization with bounded clamping, strengthens admin API and payment metadata validation, expands database and integration tests, and updates equivalent-mutant coordinates. ChangesValidation and persistence migration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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: 44b5e87805
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/features/admin/attendees.ts`:
- Around line 208-210: Update the comment describing clampDurationDays near the
daily booking duration logic to state that valid whole numbers outside the
allowed range are clamped, while fractions, NaN, and unsafe integers are
rejected. Keep the distinction between customizable daily bookings and
fixed-duration bookings accurate.
In `@src/shared/db/built-sites.ts`:
- Around line 68-81: Update SiteDataBlobSchema and the built_sites
restore/parsing flow so legacy site_data blobs missing v are backfilled with
SITE_DATA_BLOB_VERSION before strict validation, or migrate them before parsing.
Preserve strict validation for all other fields and ensure current blobs
continue using the existing v value.
In `@src/shared/types.ts`:
- Around line 214-230: Update DayPriceKeySchema to accept only canonical decimal
day-count keys, rejecting leading-zero values such as "01" while preserving
valid keys like "1". Add a regression case covering a record containing both
"01" and "1", ensuring the schema rejects this collision.
- Around line 203-204: Update the exported rejectDurationDays declaration to
include an explicit function parameter and return type, while preserving its
existing delegation to durationDays.reject and validation behavior.
In `@src/shared/validation/number.ts`:
- Around line 13-14: Declare an explicit return type for the exported
integerAtLeast function, using the appropriate validator/schema type for the
value returned by its v.pipe expression. Preserve the existing number,
safe-integer, and minimum-value validation behavior.
- Around line 47-52: Update NonNegativeIntSchema to reject digit-only values
outside JavaScript’s safe-integer range before or during the Number
transformation, while preserving valid non-negative integer behavior and
PositiveIntSchema’s existing constraint. Add a regression test using a
digit-only value greater than Number.MAX_SAFE_INTEGER and assert that validation
returns null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3555d1ea-6b0a-401c-9e24-84de03a1bb98
📒 Files selected for processing (53)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/attendee-form-model.tssrc/features/admin/attendees.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/catalog-transfer/schema.tssrc/features/api/payment-processing/metadata.tssrc/features/public/pages.tssrc/shared/address-lookup/types.tssrc/shared/admin-features.tssrc/shared/booking-date-fields.tssrc/shared/booking/model.tssrc/shared/dates.tssrc/shared/db/address-cache.tssrc/shared/db/attendees/capacity/range.tssrc/shared/db/attendees/create.tssrc/shared/db/attendees/pii.tssrc/shared/db/attendees/servicing.tssrc/shared/db/attendees/update.tssrc/shared/db/built-sites.tssrc/shared/db/capacity.tssrc/shared/db/contact-preferences.tssrc/shared/db/groups.tssrc/shared/db/listings/table.tssrc/shared/db/processed-payments.tssrc/shared/db/sumup-checkouts.tssrc/shared/db/table.tssrc/shared/db/token-attempts.tssrc/shared/email-renderer.tssrc/shared/listing-defaults.tssrc/shared/listing-parents-rules.tssrc/shared/listings-actions.tssrc/shared/types.tssrc/shared/validation/number.tssrc/shared/validation/stored-json.tssrc/shared/validation/string.tssrc/ui/templates/admin/listings/details.tsxsrc/ui/templates/admin/listings/form-values.tsxsrc/ui/templates/tickets.tsxtest/shared/admin-features.test.tstest/shared/db/address-cache.test.tstest/shared/db/attendees/pii.test.tstest/shared/db/built-sites.test.tstest/shared/db/contact-preferences.test.tstest/shared/db/listings/table.test.tstest/shared/db/processed-payments.test.tstest/shared/db/sumup-checkouts.test.tstest/shared/db/table.test.tstest/shared/db/table/cache.test.tstest/shared/db/table/write-row.test.tstest/shared/db/token-attempts.test.tstest/shared/listing-defaults.test.tstest/shared/validation/number.test.tstest/shared/validation/stored-json.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37ffc03162
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/shared/db/built-sites.test.ts`:
- Around line 211-223: Update the “toDbValues identifies invalid site data” test
to handle the asynchronous rejection from builtSitesCrudTable.toDbValues: pass
its returned promise directly to expect(...).rejects and assert the existing
error message, rather than using synchronous toThrow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 46c70b38-979a-4579-8d2a-83d20716ffa5
📒 Files selected for processing (17)
scripts/mutation/equivalent-mutants.txtsrc/features/admin/attendees.tssrc/shared/db/built-sites.tssrc/shared/payment-helpers.tssrc/shared/types.tssrc/shared/validation/number.tstest/lib/server-webhooks/single-ticket-refunds.test.tstest/shared/db/built-sites.test.tstest/shared/db/listings/table.test.tstest/shared/logger/log-output.test.tstest/shared/payment-helpers/build-items.test.tstest/shared/payment-helpers/dispatch.test.tstest/shared/payment-helpers/limits.test.tstest/shared/payment-helpers/metadata.test.tstest/shared/payments.test.tstest/shared/validation/number.test.tstest/test-utils/debug-log.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9d82c7545
ℹ️ 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".
* Fix reservation balance summaries * Extend external screenshot scenarios * Fix mobile booking controls * Address reservation balance review * Derive balance query arguments * Clarify reservation order totals * Improve paid checkout screenshots * Start Stripe mock for screenshots * Clean up failed screenshot startup * Clean up all screenshot resources
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/ui/client/admin/order-gallery.test.ts`:
- Around line 120-121: Update the afterEach teardown in the test to restore
clock.time with the repository’s invariant-enforcing non-null assertion instead
of optional chaining. Add a brief comment documenting that beforeEach always
installs FakeTime, so the value must exist and teardown should fail loudly if it
does not.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f0b33177-c991-43a2-9e72-5027e50dcc21
📒 Files selected for processing (4)
src/shared/types.tssrc/shared/validation/number.tstest/shared/validation/number.test.tstest/ui/client/admin/order-gallery.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 209e1e6267
ℹ️ 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".
* Normalize declarative form inputs * Consolidate declarative input handling * Address declarative input review feedback * Unify declarative input sources * Address final declarative input review * Use functional column layout parsing
* Normalize lookup and result contracts * Move changed integration tests into mapped suites * Map admin API tests to their source * Map listing API tests to their source * Add branch code line task * Keep required lookups at domain boundaries * Address lookup contract review feedback * Preserve optional provider and write-back outcomes * Keep cache hits stable during batch loads
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47c9451747
ℹ️ 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".
1 participant
What changed
01.400errors when listing API requests contain fractional durations, non-text bookable days, or unsafe day prices.Why
Structured values were often parsed from storage without checking their shape. Bad data could then fail later in unrelated code or be accepted with the wrong types. Reads and writes now check the same declared format and stop at the boundary with a useful error that does not expose the rejected value.
Tests
deno task precommitpassed, including lint, typecheck, duplication checks, edge build, full tests, and deterministic coverage.