fix: thread checkSchemas through IfcxLayerStack so --no-validate works - #134
Open
mikan-atomoki wants to merge 1 commit into
Open
fix: thread checkSchemas through IfcxLayerStack so --no-validate works#134mikan-atomoki wants to merge 1 commit into
mikan-atomoki wants to merge 1 commit into
Conversation
compose parses --no-validate into a local validate flag, but IfcxLayerStackBuilder.Build() always constructed IfcxLayerStack with no argument, whose constructor eagerly ran Compose() -> LoadIfcxFile() with checkSchemas defaulting to true. The SchemaValidationError threw before the CLI's own correct LoadIfcxFile(federated, validate, true) call was ever reached. Thread checkSchemas: boolean = true through IfcxLayerStack's constructor and IfcxLayerStackBuilder.Build(), and pass the CLI's parsed validate flag into .Build(validate). Defaulting to true keeps every other caller's behavior unchanged.
aothms
approved these changes
Aug 1, 2026
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.
Fix: make
compose --no-validateactually disable schema validationFixes #133
What
composeparses--no-validatecorrectly, but validation still always runs:IfcxLayerStackBuilder.Build()constructsIfcxLayerStack, whose constructor eagerly runsCompose()→LoadIfcxFile(this.federated)withcheckSchemasdefaulting totrue(layer-stack.ts#L15-L32, workflows.ts#L24). TheSchemaValidationErrorescapes before the CLI's own (correct)LoadIfcxFile(federated, validate, true)call is ever reached (ifcx-cli.ts#L102-L109).This PR threads a
checkSchemas: boolean = trueparameter throughIfcxLayerStack's constructor andIfcxLayerStackBuilder.Build(), and passes the CLI's parsedvalidateflag into.Build(validate). Defaulting totruekeeps every other caller's behavior unchanged; the style mirrors the optional booleanLoadIfcxFilealready uses.mainis still at02b0b21aaa4ab10354cdf931a6ec0d704d6e8d22(the commit this patch was written and verified against), so no rebase was needed.Repro
bad-type.ifcx— schema-declaredIntegerattribute holding a string (plain schema violation, no null semantics involved){ "header": { "id": "poc/repro/bad-type.ifcx", "ifcxVersion": "ifcx_alpha", "dataVersion": "1.0.0", "author": "poc", "timestamp": "2026-07-22" }, "imports": [], "schemas": { "test::count": { "uri": "https://example.com/poc/repro/test-count", "value": { "dataType": "Integer" } } }, "data": [ { "path": "aaaaaaaa-0000-0000-0000-000000000001", "attributes": { "test::count": "this-is-not-an-integer" } } ] }Before this PR, both of these crash with the byte-identical
SchemaValidationErrorstack trace:Verification
compose --no-fetch bad-type.ifcx out.json(no flag) → still throwsSchemaValidationError, unchanged (exit 1)compose --no-fetch --no-validate bad-type.ifcx out.json→ now exits 0 and writes the composed outputnpm testinsrc/(28 existing cases insrc/test/test.ts, including thelayerStack buildersuite) passes identically before and after the patch (28/28, 0 failures)Note: alternative fix considered
In this code path the CLI only uses
layerStack.GetFederatedLayer(); the constructor's eagerly-builttreeis computed, validated, and then discarded. A larger alternative fix would makeCompose()lazy (run on firstGetFullTree()), removing the wasted validate+compose pass entirely — but that changes when validation errors surface for other callers ofIfcxLayerStack, so this PR takes the minimal route instead. Happy to rework in that direction if you prefer it.Related: this unblocks observing the composition engine's actual null-attribute semantics through the CLI — see the companion issue (#132) about
Validate()rejectingnulloverrides thatcompose.tsis built to retain.