Skip to content

fix(gts-macros): add additionalProperties: false to derived schemas#63

Merged
Artifizer merged 5 commits intoGlobalTypeSystem:mainfrom
Artifizer:main
Feb 20, 2026
Merged

fix(gts-macros): add additionalProperties: false to derived schemas#63
Artifizer merged 5 commits intoGlobalTypeSystem:mainfrom
Artifizer:main

Conversation

@Artifizer
Copy link
Copy Markdown
Contributor

@Artifizer Artifizer commented Feb 20, 2026

Summary by CodeRabbit

  • Chores

    • Bumped package versions across workspace components to 0.8.1 and updated workspace dependency versions.
  • Refactor

    • Generated JSON Schemas now disallow additional properties at the root for certain derived/nested schemas, tightening validation.
  • Tests

    • Removed an outdated JSON Schema test artifact.

Child/derived schemas generated by `gts_schema_with_refs_allof()` were
missing `"additionalProperties": false` at the root level, while base
GTS schemas always included it. This caused schema-vs-schema compatibility
validation (OP#12) to reject all derived schemas with:

  "derived schema '...' loosens additionalProperties from false in base"

The fix adds `"additionalProperties": false` to both child schema
generation paths (generic and non-generic), ensuring derived schemas
match the base schema constraint and pass

Signed-off-by: Artifizer <artifizer@gmail.com>
Signed-off-by: Artifizer <artifizer@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Feb 20, 2026

No actionable comments were generated in the recent review. 🎉


📝 Walkthrough

Walkthrough

Bumped five workspace package versions to 0.8.1, updated gts-macros to emit "additionalProperties": false in two generated JSON Schema locations, incremented several package versions, and removed a test User schema file.

Changes

Cohort / File(s) Summary
Workspace Manifest
Cargo.toml
Bumped five workspace dependencies from 0.8.0 to 0.8.1 (paths unchanged).
Macro Code
gts-macros/src/lib.rs
Emit "additionalProperties": false at two generated child/allOf JSON Schema roots.
Package Manifests
gts-macros/Cargo.toml, gts-cli/Cargo.toml, gts-id/Cargo.toml, gts-macros-cli/Cargo.toml, gts/Cargo.toml
Bumped package versions from 0.8.0 to 0.8.1 for multiple crates.
Test Schema Removal
gts-macros/tests/schemas/gts.x.test.entities.user.v1~.schema.json
Removed test JSON Schema file defining a User object with required id, name, email, and age properties.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 I hopped through crates and nudged a patch,
Five versions forward, tidy as a batch.
Schemas now strict — no extras allowed,
A test file cleared, the tree stands proud.
🥕✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding 'additionalProperties': false to derived schemas in gts-macros. This is the primary functional change in the PR, with other changes being version bumps.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Signed-off-by: Artifizer <artifizer@gmail.com>
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
gts-macros/src/lib.rs (1)

1225-1236: ⚠️ Potential issue | 🟠 Major

Root-level additionalProperties: false with allOf makes derived schemas unsatisfiable in draft-07.

In draft-07, additionalProperties only considers properties defined in the same schema object, not those from allOf subschemas. Since the root level has no properties defined (only nested within the second allOf subschema), additionalProperties: false forbids all properties, making these schemas reject valid instances.

Remove additionalProperties: false from the root of both schemas, or upgrade to JSON Schema 2019-09+ and use unevaluatedProperties: false instead.

🛠️ Suggested fix
                 serde_json::json!({
                     "$id": format!("gts://{}", schema_id),
                     "$schema": "http://json-schema.org/draft-07/schema#",
                     "type": "object",
-                    "additionalProperties": false,
                     "allOf": [
                         { "$ref": format!("gts://{}", parent_schema_id) },
                         {
                             "type": "object",
                             "properties": nested_properties
                         }
                     ]
                 })

Also applies to: 1332-1342

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@gts-macros/src/lib.rs` around lines 1225 - 1236, The root-level
"additionalProperties": false in the constructed JSON schema makes derived
schemas unsatisfiable under draft-07 because properties live inside the second
allOf subschema; update the schema construction in the block that builds the
serde_json::json! (the object that includes "$id": format!("gts://{}",
schema_id), "$schema", "type": "object", "additionalProperties": false, "allOf":
[...]) by removing the root-level additionalProperties entry (or alternatively
switch the "$schema" to a 2019-09+ draft and replace additionalProperties with
"unevaluatedProperties": false), or move additionalProperties:false into the
same subschema that defines "properties" (the second allOf element built from
nested_properties) so that the restriction applies where properties are
declared; apply the same change to the other occurrence noted (around the second
range mentioned).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@gts-macros/src/lib.rs`:
- Around line 1225-1236: The root-level "additionalProperties": false in the
constructed JSON schema makes derived schemas unsatisfiable under draft-07
because properties live inside the second allOf subschema; update the schema
construction in the block that builds the serde_json::json! (the object that
includes "$id": format!("gts://{}", schema_id), "$schema", "type": "object",
"additionalProperties": false, "allOf": [...]) by removing the root-level
additionalProperties entry (or alternatively switch the "$schema" to a 2019-09+
draft and replace additionalProperties with "unevaluatedProperties": false), or
move additionalProperties:false into the same subschema that defines
"properties" (the second allOf element built from nested_properties) so that the
restriction applies where properties are declared; apply the same change to the
other occurrence noted (around the second range mentioned).

Signed-off-by: Artifizer <artifizer@gmail.com>
Signed-off-by: Artifizer <artifizer@gmail.com>
@Artifizer Artifizer merged commit 268f250 into GlobalTypeSystem:main Feb 20, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant