feat(validator): fill declared defaults during validation

[David Chicaiza (david0723)]

[Has Ai Code]

Jira: https://make.atlassian.net/browse/MAIA-1286

Releasing as 2.0.0. Adds an opt-in fillDefaults validation option mirroring BlueprintValidator's useDefaults modes: with 'requiredOnly', an omitted required field whose schema declares a usable default validates as that default instead of failing "Field is mandatory."; with 'always', omitted optional fields with defaults fill too. Every result now carries normalizedValues (the input values with fills applied) and appliedDefaults (what was filled, where), with or without the option, so the caller-side pattern (if (valid) use(normalizedValues)) does not fork on the flag.

Why

A required field with a declared default fails validation when the caller omits it, even though the form UI would have materialized exactly that default on save. Consumers that build configs programmatically (MCP tools driving module configuration) either have to guess a value for a field they may never have seen (rpc://-injected fields only exist after resolution) or re-implement forman traversal outside the library to fill defaults themselves, which cannot be done correctly: a filled toggle conditions nested fields, and nested resolution is this library's own domain knowledge. #61 tried suppressing the error without producing values and was closed for exactly that reason; this PR does the filling where the walk already happens.

How it works

The substitution sits just before the mandatory check in validateFormanValue: option on, value undefined or '' (the same fillable predicate as BlueprintValidator's useDefaults and the builder UI it cites; an explicit null stays a provided value and fails), default able to satisfy the required check (null/'' cannot). 'requiredOnly' fills required fields only; 'always' extends the same fill to omitted optional fields. The filled value then continues through the normal walk, so a filled boolean conditions its nested branch exactly as a provided one would (false leaves it inactive) and defaults under an armed branch fill recursively, in the same single pass, rpc-resolved specs included. Nothing fills under suppressRequired and provided values are never overwritten. Fills are recorded per domain root; normalizedValues is built with a copy-on-write path write (setValueAtPath), so inputs are never mutated — though subtrees nothing was written into are shared with the input rather than deep-copied, which keeps the zero-fill case free.

Where to look

• src/types.ts — the public contract: the fillDefaults option and the two new result fields. Start here.
• src/validator.ts — the fill just before the mandatory check (~16 lines) and the result assembly.
• src/utils.ts — setValueAtPath / setIn.

Shape

+702/-114 across 16 files. Ordered so the bulk reads separately from the mechanism:

Read	Commits	What
The mechanism	3b1df93, 68aca7f, plus review fixes b0e2fdc 8955361 f9a097c a57f056 cb9c769	+116/-4 of src across four files. This is the review surface.
Its tests	same commits	test/fill-defaults.spec.ts, 22 cases, plus setValueAtPath unit tests in test/utils.spec.ts.
Mechanical churn	0065b14	~225 lines across 8 existing specs, matcher-only: exact-equality assertions also asserted the absence of every other result key, so they are widened to subset matching. Green at 497/497 on the pre-feature source, which is the proof it changes no intent.
Docs	91be2b3, 7686e77	README release note + option contract, AGENTS.md.

An adversarial trim pass ran over the whole branch before review (a57f056): it cut comment/JSDoc duplication of the option contract back to the constraints the code cannot show, shortened the release note to what breaks and how to migrate, and removed six test cases a named earlier case already pinned — the 'always' block was a cross-product over predicate arms that do not vary by mode. It also corrected two claims that were overstated here and in the docs; see Notes. 27 cases → 21, +740 → +660. Review has since added back three tests (see below), landing at +702.

Each fill guard was mutation-verified: dropping the undefined gate, the 'always' arm, the inactive-branch guard, and no-oping the fill record each turn their intended tests red.

Verified against a live consumer

Measured at 8955361, before two later changes (see the caveat below). The branch build was staged into a live internal deployment of the service that consumes this library, and the real validation endpoint was called over HTTP with the stored module configuration from the motivating incident — its two required toggles are injected by rpc:// resolution and exist in no manifest. With the option on, the failing payload dropped from three errors to the one genuine defect with both toggles filled and reported, the honestly-repaired payload flipped from rejected to accepted, and explicitly provided values stayed untouched. Full table in the comments.

Two things changed after that run, so read the option-off column with them in mind. f9a097c made '' fill (it previously required strict undefined), and 68aca7f made the two result fields unconditional. With the option off, validation outcomes — valid, errors, warnings — are unchanged, but the result object now carries two additional keys. That is precisely why this ships as a major rather than a minor, and it is not a drop-in guarantee at the object level. The consumer PR needed a response guard for exactly this reason.

Notes

• Review (cb9c769) made two impossible states loud instead of silent: the fill record and setValueAtPath both failed open on states that cannot occur, and either would have reported a fill in appliedDefaults that normalizedValues does not contain — the one invariant this feature sells. setValueAtPath also gained the unit tests it never had.
• '' counts as an omission and fills, matching the blueprint predicate. Under 'always' that means a deliberately cleared optional field comes back with its default; 'requiredOnly' leaves it alone. Both are pinned by a test, and the docs now state the exception once rather than asserting the opposite beside it. Review raised restricting ''-as-omission to required fields; kept the parity behaviour instead, since it would make this 'always' mean something different from BlueprintValidator's 'always', and no caller uses the mode yet. Clear-preserving semantics would be a new named mode, not a change to this one.
• The 'always' mode and the always-present normalizedValues both come from review feedback, for parity with BlueprintValidator's option surface and for a consistent consumer pattern.
• Why a major. Nothing was removed or renamed and the return type only narrows, so valid/errors readers are untouched. But always-present result fields change the shape for anyone deep-comparing the result or forwarding it into a fixed-shape response — this repo's own suite needed the assertion widening above, and the MCP Server consumer needed its response guard extended to three more tools. Cheaper to call that a major than to surprise someone. The README carries the release note and the migration line.
• Both fill outputs stay present on the failure path on purpose. A filled default can legitimately arm a nested requirement that has no default; the caller repairing that error needs the filled values the error was computed against.
• appliedDefaults paths are dot-joined to match error paths, so the two correlate directly. pathToString is deliberately not reused: it emits bracket notation, which would decorrelate the two.
• The semantics deliberately mirror BlueprintValidator's useDefaults modes, so the two validators agree on what an omitted field means. Two named divergences. The first is why the option is not called useDefaults: no in-place mutation. This library never casts or modifies caller input; the filled values come back in normalizedValues instead. Reusing the name would promise behavior it does not have.
• The second divergence is the gate on the default value itself. A null/'' default is never filled, in either mode, where Blueprint fills any default !== undefined and so would write one. It could not satisfy a required check, and on an optional field it is indistinguishable from the omission itself. Strictly the safer of the two behaviours, and 0/false defaults still fill, which is what the motivating boolean toggles depend on. Surfaced in review, which checked the predicate against the @integromat/blueprint source rather than taking the parity claim on trust.
• Only the field-level default is consulted. A select whose options carry their own default: true marker but whose field declares no default still fails as mandatory; deriving a fill from option markers would be a separate decision.

[Copilot]

Pull request overview

Adds an opt-in validation mode to fill declared defaults for omitted required fields during validateForman / validateFormanWithDomains, and returns the filled output (normalizedValues) plus an audit trail (appliedDefaults) so callers can persist/repair configurations using the same values validation actually walked.

Changes:

• Introduces fillDefaults: 'requiredOnly' and records filled defaults per domain during validation, emitting normalizedValues + appliedDefaults in the result.
• Adds setValueAtPath (copy-on-write path write) to build normalizedValues without mutating inputs.
• Adds a comprehensive Jest spec suite covering fills across nesting, arrays, RPC-injected specs, and cross-domain routing.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/validator.ts	Implements default-filling at the required check, tracks fills per domain, and assembles normalizedValues/appliedDefaults in the result.
src/utils.ts	Adds setValueAtPath to build normalized values immutably via copy-on-write updates.
src/types.ts	Extends public types/options/result shape to include fillDefaults, normalizedValues, and appliedDefaults.
test/fill-defaults.spec.ts	Adds targeted test coverage for required-only default filling behavior and edge cases.
README.md	Documents the new fillDefaults: 'requiredOnly' option and its outputs.
AGENTS.md	Updates validator architecture notes to include the new default-filling behavior and result fields.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[David Chicaiza (david0723)]

Service-level proof, run against a live internal deployment that consumes this library: the real validation tool called over HTTP with real auth, on the module whose rpc-injected required defaults motivated this PR (the two toggles exist in no manifest, they only appear once the validator resolves rpc://, so nothing outside this library can fill them correctly).

Payload	released library	#65 build, option off	#65 build, consumer passes fillDefaults: 'requiredOnly'
stored legacy config, both rpc-injected required toggles absent	REJECTED, 3 errors (both toggles mandatory + one genuine type error)	byte-identical to released	REJECTED, 1 error (only the genuine one); both toggles filled (false/true) and reported in appliedDefaults + normalizedValues
same config with the genuine error fixed	REJECTED, 2 errors (both toggles mandatory)	byte-identical to released	ACCEPTED
control: toggles explicitly provided by the caller	genuine errors only	identical	identical, appliedDefaults: [] — provided values untouched

Middle column is the drop-in guarantee: with the option off the wire responses are byte-identical, so releasing this changes nothing for existing consumers. Right column took a one-line change in the consumer (adding the option to its existing validateFormanWithDomains call); since that service already returns the whole validation result, normalizedValues and appliedDefaults reached its API response with no further changes.

[Jakub Stok (jakubstokcelonis)]

Reviewed the mechanism (src/*, ~116 lines) closely and spot-checked the churn claim. This is careful work — the guard placement is right, the walk-order/copy-on-write interaction is correct, and the PR description is unusually honest about its own caveats. I verified the behaviour empirically rather than by reading, on top of the branch at a57f056:

Verified working as described

• Array-index paths fill and reconstruct correctly (items.0.mode), including through the synthetic filter inline schema.
• Cross-domain nested.domain fills land in the target domain's root with a root-relative path — validateFields resets path: [] and switches domain, same as fieldStates already does. Confirmed with a two-domain case.
• Parent-before-child ordering holds, so an object default on a collection is written first and the child fill lands on the clone rather than being clobbered by the reduce.
• structuredClone genuinely de-aliases the schema's own default instance (checked by identity).
• Input is not mutated; normalizedValues.default === input when nothing filled, as documented.
• registerOnly early-returns above the fill block and suppressRequired gates it, so neither branch of an inactive nested arm fills. Both guards are load-bearing and correct.
• The churn really is matcher-only: the only non-toEqual→toMatchObject hunks in the 8 existing specs are two one-liner reformats with identical assertions. toMatchObject still enforces array length, so error-count assertions are not weakened.
• npx tsc clean, 499/499 green.

Findings — three inline, none blocking; the first is the one I would actually want changed before merge.

The '' handling is the substantive one: f9a097c made '' fill, which is defensible for a required field but silently rewrites a deliberately-cleared optional field under 'always'. The README documents it and then contradicts itself two clauses earlier in the same sentence.

The other two are latent rather than live: setValueAtPath fails open on path shapes it cannot write, and the ?. on the fill record can drop a fill without a trace. Neither is reachable today; both would desync appliedDefaults from normalizedValues silently if they ever became reachable, which is a bad failure mode for a feature whose whole contract is that those two agree.

[Jakub Stok (jakubstokcelonis)]

Re-reviewed at cb9c769. All three findings addressed; verified the fixes rather than reading them.

Checked on the branch

• npx tsc --noEmit clean, 502/502 green (499 + your three new tests).
• Both new tests genuinely fail pre-fix. Restoring return values in setValueAtPath turns the throw test red; reverting the predicate to strict undefined turns two '' tests red. Real regression tests.
• The ! does what you said it does. It erases at compile time, so I checked the runtime: the property access on an absent root throws TypeError: Cannot read properties of undefined (reading 'appliedDefaults') at the push. Loud, not silent — which was the actual concern, not the spelling.
• Docs no longer contradict themselves, and the 'requiredOnly' escape hatch is named in both the README and the JSDoc.

On keeping the '' parity — you convinced me, and I checked the premise rather than taking it. I read the actual useDefaults implementation in @integromat/blueprint (lib/type.validator.js:72-78):

if ((options.useDefaults === true ||
     options.useDefaults === 'always' ||
     (options.useDefaults === 'requiredOnly' && param.required === true)) &&
    (value === undefined || value === '') &&
    param.default !== undefined) {

Same predicate, same mode split. So restricting ''-as-omission to required fields really would have made this 'always' mean something other than Blueprint's 'always' — exactly the one-cell divergence you described, and it would have been found the hard way years from now. Your three reasons hold and the third is the decisive one: clear-preserving semantics is a new named mode, not a redefinition of this one. Dropping it.

One divergence from Blueprint that is real and worth knowing you have: Blueprint gates on param.default !== undefined, so it fills a null/'' default; you additionally require != null && !== ''. Verified the falsy cases that actually matter still fill correctly:

default: null  -> no fill      default: 0     -> fills 0
default: ''    -> no fill      default: false -> fills false

0/false filling is the case the motivating boolean toggles depend on, so this is right. The divergence is documented and strictly safer than Blueprint's. Not asking for a change — just noting it's a second named divergence alongside the no-mutation one, if you ever reconcile the two validators.

LGTM. Approving.

[Jakub Stok (jakubstokcelonis)]

Addendum — the boolean arm of useDefaults, which my approval walked straight past.

Dominik raised this and he's right that it's unaddressed: BlueprintValidator's option is boolean | 'always' | 'requiredOnly' and fillDefaults is 'always' | 'requiredOnly'. I quoted that exact predicate above with options.useDefaults === true visible on the first line and then said "same mode split" without mentioning it — I was comparing the fillable predicate (which values count as omitted), which is what the '' question was about. The option type is a different question and my review didn't close it.

Having looked, I think your two-string type is right, and the argument is stronger than "it's fine":

true is an exact alias for 'always' — same lib/type.validator.js:72 snippet, first arm and second arm reach the identical branch. There is no third behaviour behind it.

web-api never exposes it. lib/controllers/validations/query.js:40 defines the public query param as a select with exactly two options:

useDefaults: [{ name: 'useDefaults', type: 'select',
    options: [{ value: 'always' }, { value: 'requiredOnly' }], required: false }]

The internal callers (keys.js:192, hooks.js:333, connections.js:875) all read query.useDefaults ?? false — so the string is what actually flows when the param is set, and the boolean's live role is just the absent-value "off". A new library has no reason to inherit a deprecated spelling that the platform's own public surface already dropped.

So: no change requested, approval stands. But it deserves one line saying so, because read cold — two types side by side, one arm missing — it looks like an oversight rather than a decision, which is exactly how it was read. Something like:

boolean is deliberately not carried over: useDefaults: true is a legacy alias for 'always', and web-api's public query param already exposes only the two named modes.

Worth adding next to the fillDefaults JSDoc. It costs nothing and saves the next person the grep.

One process note, and I mean this constructively. This one took three rounds — 'requiredOnly', then 'always' | 'requiredOnly' after Dominik asked for Blueprint parity, and the boolean still open. Each round answered the literal ask and stopped there. Landing "here's the whole option surface, here's what I'm porting and what I'm dropping and why" in one go would have closed it the first time, and it's the difference between a reviewer feeling read and feeling processed. The underlying work is genuinely careful — that's why the gap is worth naming rather than shrugging at.

[David Chicaiza (david0723)]

Thanks for going to the blueprint source instead of taking the parity claim on trust, thats the check I couldnt really do from inside this repo.

And youre right about the second divergence. param.default !== undefined means Blueprint would write a null or '' where we wont, and the PR body was claiming exactly one named divergence. Fixed, both are named now, with the 0/false case spelled out since thats the one the boolean toggles actually depend on. No code change, agreed we're on the safer side of that one.

Merging. Version bump to 2.0.0 goes in as its own chore: release PR to match how the others were done.

[David Chicaiza (david0723)]

You were right and I talked past this one. My reply above answered the param.default !== undefined divergence and merged, without touching the boolean arm you had just written up.

Added it in #68, one line next to the fillDefaults JSDoc, phrased without the internal references since this repo is public:

BlueprintValidator's option also accepts boolean; that arm is deliberately not carried over. useDefaults: true is a legacy alias for 'always' rather than a third behaviour, and the platform's own public validation parameter already exposes just the two named modes, so there is nothing behind the boolean for a new library to inherit.

And the process note landed. Answering the literal ask and stopping is exactly what happened here, three times over on this PR and then once more on your addendum. What I should have brought the first time is the whole option surface with what I was porting, what I was dropping and why, rather than making you find each gap. Taking that one.