fix(act): revive dates on read instead of re-validating the payload - #1601
Merged
Conversation
Reading an event ran a throwing z.parse() over its data whenever the schema mentioned a date. All that is needed there is to turn stored ISO strings back into Dates — the payload was already validated on write — and the extra validation rejected payloads the framework itself wrote. A sensitive() field is moved into the pii sidecar on write, so data structurally cannot hold it, and a schema that requires it fails. That made an aggregate carrying both a sensitive field and a date unreadable and unwritable: query, load, and every later command threw. Three changes. The sensitive keys are optional in the rebuilt schema, since they are never in data. Reviving is a safeParse that falls back to the stored value, so a payload written against an older declaration still reads. The sidecar gets its half of the same rebuild, so a disclosed sensitive(z.date()) is a Date rather than a string beside a plain sibling that is one. The rebuild also descends through default, prefault, catch, readonly, nonoptional and tuple, which previously left a date inside them unrevived. Cost is unchanged (0.0%, -2.7%, -0.5% across a one-date, a nested and an array payload): safeParse costs what parse did, and only the sensitive keys gained a wrapper. Reviving the sidecar runs only when one is present. Closes #1594 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
They were optional, which says they might be there. They are not: the write path moves them into the pii sidecar, so data does not hold them and there is nothing to describe. Anything that does turn up under one of those names rides through the loose object untouched. Also 3.8% cheaper on an event with three sensitive fields, since the schema has three fewer keys to consider. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
The copied schema listed every field, so reading re-checked data that was already checked on save. Now it names the date paths and nothing else; everything else rides through the loose object untouched. This is what makes the read tolerant rather than a list of exceptions. A field that is not a date is not described, so a sensitive field sitting in the pii sidecar, a field added to the declaration after an event was written, and a field since removed are all simply not this schema's business. Dates are optional for the same reason. Costs 0.07us per event read against a 545us cold load on Postgres (act-pg/PERFORMANCE.md:1430) — about 0.01% of a read. The earlier version of this branch kept every field declared because that measured faster in isolation, which was the wrong thing to optimize: the same PERFORMANCE.md table records a faster hand-rolled walk being rejected for the same reason. Also keeps null in a nullable date, which coercion would have turned into the epoch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
date_reviver_schema builds the schema, and each event carries a date_reviver for its data and a pii_date_reviver for its sidecar. They were called to_date_schema and parse/parse_pii, which described the mechanism rather than the job and read like a second validation pass — the thing this branch removed. Also stops the tests leaving sqlite WAL and SHM files behind: cleanup removed the .db and not its two siblings, so they were committed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
It took a list of sensitive keys and skipped them, which is not what a date reviver is for. That was left over from when the schema copied every field and a sensitive key would have been required and missing. Now that it names only the dates, a sensitive key reaches it only if it is a date, and every date is already optional — so the skip decided nothing. Dropping the parameter leaves the function doing what it is called: given a declared schema, say where the dates are. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
It was revived whenever a sidecar was present, before the gate decided anything, so the values were converted and then thrown away on every denied read. Two of those are unconditional: a strip reader drops pii outright, and query/query_array carry no actor so they always default-deny — which is the common read surface. The gate still owns the decision. The guard checks only for an actor and a disclosure predicate, and never calls the predicate, so a caller's code still runs exactly once. Output is unchanged on every path: a denied read still reads REDACTED, a disclosed one still gets a Date. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
Working out where an event's dates are is a Zod question, not an event one — the code takes a declared schema and returns the schema that revives its dates, and knows nothing about events, states or PII. It sat inside the event builder because that is where it was first needed. internal/date-reviver.ts now owns it, beside the other schema helpers, reached through the internal barrel. The event builder composes it: one reviver for an event's data, another for the sensitive fields in its pii sidecar. Also fixes a union bug found while reviewing. Reducing a variant to its date paths leaves it matching almost any payload, so Zod picked the first option and the variant that declared the date was never tried — a union event's dates were not revived at all, and a sibling's payload could be read under the wrong variant's rules. Variants keep their fields, so a discriminator can still reject a payload that is not its own. Four union cases added to schema-dates.spec.ts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
A union can be told apart by the type of an ordinary field rather than by a literal discriminator. Narrowing a variant to its dates, or relaxing its other fields, makes the first variant match everything — so the one that declared the date is never tried and a sibling's payload is read under the wrong rules. Measured both ways before writing this down: with the fields kept, each variant's payload reads correctly; with them relaxed, the dated variant comes back as text. The test is the case that would otherwise invite someone to narrow it back. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
def_of was exported so the event builder could reach an event's shape while collecting sensitive keys — a Zod internals accessor leaking out of a module whose job is to return one schema. The builder doesn't need it. It reads `.shape` directly, the same way pii_fields in sensitive.ts already does for the same walk. def_of is private again and date_reviver_schema is the whole interface: how a Zod schema is taken apart stays inside the module that takes it apart. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
The event builder walked an event's shape itself to find sensitive keys — top-level keys plus union options, the same traversal pii_fields already does — because it needed each key's schema and pii_fields returns only names. pii_schemas now returns the schemas, and pii_fields is its keys, so there is one walk instead of two. The builder asks sensitive.ts which fields are sensitive and date-reviver.ts where the dates are, then composes the two answers. It no longer touches .shape, .options or _zod at all. Same shape as the date change: the module that knows how to take a schema apart is the one that does it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
event_tags called pii_schemas for the sidecar's date revivers and then pii_fields for the names, which walks the same shape a second time — the duplication the previous commit set out to remove, reintroduced in the same function. One call, keys taken from the result. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 30, 2026
# [@rotorsoft/act-tck-v1.36.15](https://github.com/Rotorsoft/act-root/compare/@rotorsoft/act-tck-v1.36.14...@rotorsoft/act-tck-v1.36.15) (2026-08-30) ### Bug Fixes * **act-pg:** exclude the boundary event from created_after ([#1603](#1603)) ([e8c6a18](e8c6a18)), closes [#1595](#1595) * **act:** revive dates on read instead of re-validating the payload ([#1601](#1601)) ([97ed5b9](97ed5b9)), closes [#1594](#1594)
|
🎉 This PR is included in version @rotorsoft/act-tck-v1.36.15 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
github-actions Bot
pushed a commit
that referenced
this pull request
Sep 4, 2026
# [@rotorsoft/act-v1.31.15](https://github.com/Rotorsoft/act-root/compare/@rotorsoft/act-v1.31.14...@rotorsoft/act-v1.31.15) (2026-09-04) ### Bug Fixes * **act:** give every Act its own ports frame ([#1609](#1609)) ([32b415a](32b415a)), closes [#1597](#1597) * **act:** revive dates on read instead of re-validating the payload ([#1601](#1601)) ([97ed5b9](97ed5b9)), closes [#1594](#1594) * **act:** unsubscribe from notify before stopping the settle loop ([#1602](#1602)) ([2e07111](2e07111)), closes [#1468](#1468) [#1468](#1468) [#1596](#1596)
|
🎉 This PR is included in version @rotorsoft/act-v1.31.15 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
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
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.
Closes #1594.
An event that had both a
sensitive()field and a date in its schema could not be read at all.query,query_arrayandloadall threw, and because running a command loads the state first, the stream stopped accepting writes too. This fixes that and several related problems found on the way.What went wrong
When an event's schema mentions a date anywhere, the framework builds a copy of that schema and runs it over the event's data on every read. Dates do have to be revived — JSON has no date type, so a
Dateis stored as text and something has to turn it back.The trouble was that the copy listed every field and ran as a validating parse, so each read re-checked the whole payload. That check was never needed: events are validated when they are committed, so anything in the store has already passed its schema once.
And it did more than waste time. Fields marked
sensitive()are moved out ofdatainto a separatepiifield on write, but the copy still listed them as required. The framework rejected data it had written itself.What changed
The copied schema now names the date fields and nothing else. Everything else passes through untouched.
That single idea is what makes reading tolerant, rather than a growing list of exceptions. A field that is not a date is simply not described, so none of these are the schema's business any more: a
sensitive()field living inpii, a field added to the declaration after an event was written, a field since removed. The dates themselves are optional for the same reason — absent is not wrong. If a payload still cannot be revived, the read hands back what is stored instead of throwing.Unions are the exception, and they need their fields. Zod picks a union variant by trying each until one matches. A variant reduced to its dates matches almost any payload, so the first option wins and the variant that declared the date is never tried — meaning a union event's dates were not revived at all, and a sibling's payload could be read under the wrong variant's rules. Variants therefore keep their fields (optionally), so a discriminator can still reject a payload that is not its own.
This is not laziness, and narrowing it was measured before being rejected. Two variants told apart by the type of a field rather than a literal:
atis a date)atis a string)DateStringString— not revivedStringThere is no smaller subset that still discriminates, because which fields do the discriminating is not knowable — a literal usually does it, but a field type can too. A test pins this so nobody tidies it away.
The
piisidecar is revived too, but only for readers allowed to see it. Without this, a disclosedsensitive(z.date())arrived as text beside an ordinary date that was aDate. It is skipped for handler readers, which droppiioutright, and forquery/query_array, which carry no actor and always deny — so plaintext is no longer converted just to be overwritten with[REDACTED]. The gate still decides; the guard never calls the disclosure predicate, so a caller's code runs exactly once.Each question is answered by the module that owns it. Working out where a schema's dates are is a Zod concern, not an event one, so
internal/date-reviver.tsowns it and exports exactly one function. Which fields are sensitive was alreadyinternal/sensitive.ts's subject, but the event builder walked the shape itself to get each field's schema —pii_schemasnow returns them andpii_fieldsis its keys, so there is one walk instead of two.event_tagscomposes the two answers and no longer touches.shape,.optionsor_zodat all.The walk also now descends through
.default(),.catch(),.readonly(),.nonoptional(),.prefault()and tuples, which previously left a date inside them as text..default()is common enough that this was likely to be hit.Known limits
Dates inside a
z.intersection(...)or az.lazy(...)are not revived — those constructs build no reviver and the value stays text. Unchanged from before this PR, and it is the documented fallthrough ("a construct this doesn't recognise contributes no date");lazyin particular can be recursive, so descending into it needs care. Verified rather than assumed:z.discriminatedUnionreports itself as a plain union and is handled.pii_schemasmerges union variants first-wins, so a key sensitive in two variants with different declared types is revived using the first variant's schema. That matches the previous behaviour and no test exercises it.What it costs
About 0.07µs per event read. A cold load on Postgres is 545µs (
libs/act-pg/PERFORMANCE.md:1430), so roughly 0.01% of a read.Worth stating plainly, because naming only the date fields is slower in a micro-benchmark than listing them all — 0.272µs against 0.200µs on a six-field payload. Zod charges more for a key it was not told about than for one it validates, so leaving fields out moves them onto the more expensive passthrough path.
An earlier version of this branch kept every field declared for exactly that reason, which was optimizing the wrong thing. The same
PERFORMANCE.mdtable records a hand-rolled walk that was faster than the shipped Zod parse (0.188µs vs 0.285µs) and was rejected anyway, because correctness and maintainability beat a tenth of a microsecond. Same trade, same answer.What was considered and rejected
superjsonanddevaluesolve the general "JSON lost my types" problem by carrying type metadata beside the payload, and superjson is the usual answer in the TypeScript ecosystem. It is the wrong layer here: it changes the stored format, so existing events carry no metadata and external SQL readers would see a wrapped shape.Zod 4 codecs (
z.codec(z.iso.datetime(), z.date(), …)withz.decode) are the most principled option and were tested against the installed 4.4.3.z.decodestill validates the whole payload — a field added to the declaration after an event was written is rejected, the exact failure this PR is about — and a plainz.date()is not decodable from a string, so adopting codecs would mean every Act user rewriting their event schemas. Naming only the date paths is what avoids re-validation, and it works the same whichever coercion primitive sits at the leaves.Test plan
libs/act/test/schema-dates.spec.tsandlibs/act-sqlite/test/read-schema-dates.spec.tspnpm test— 3622 passingpnpm typecheck, biome clean, architecture spec passing (it enforces thatpii_*helpers stay reachable only fromsensitive.tsand the event builder)How this relates to #1556
#1556 was the opposite problem: adapters revived dates by looking at the shape of every string, so an ISO-looking
z.string()came back as aDate. #1570 fixed it by letting the schema decide, which was right, and this PR keeps that. Zod still does the walking, so nesting, arrays, records, unions and wrappers are handled by the engine rather than by a traversal of our own that would drift as Zod grows. A test pins the #1556 behaviour so it stays fixed.Stability charter impact
Additive and internal.
EventTagsgainsdate_reviverandpii_date_reviver(replacingparse), the schema walk moved tointernal/date-reviver.ts, andsensitive.tsgainspii_schemas. All@internal. The one publicly reachable name involved,pii_fields(re-exported throughtypes/schemas.tsand used by act-http's OpenAPI emitter), keeps its exact signature — it is now implemented as the keys ofpii_schemas. The publicdateReviverinutils.tsis untouched. No builder method,IActmethod, port, lifecycle event or exported type changed. Behaviour changes only in that reads which used to throw now succeed.rfc-gate: exempt — the stability snapshot grew only because it captures source text, and what grew is doc comments plus an internal module. No public surface was added.
Found by
Debug wave 21, reproduced in the main loop before filing. The union bug was found while reviewing this branch, not by the wave.