Align GTS entity validation with strict schema resolution

[aviator5]

• Tightens GTS entity validation around parsed GTS IDs and strict schema reference resolution.
• Removes redundant/best-effort resolver paths, lets schema validation own trait completeness checks, and
  adds regression coverage for closed descendant schemas and x-gts-traits-schema compatibility across ancestor branches.

Summary by CodeRabbit

Summary of Release Updates

• Bug Fixes / Behavior Changes

  • $ref resolution is now strict: unresolved external references and circular references fail validation.
  • Schema ref resolution is consistently used throughout schema compilation/validation; resolve_schema_refs is now fallible and removed try_* helpers.
  • Instance handling and entity validation behavior were tightened (schema-only x-gts-* keywords no longer accepted on instances; validation now routes by type vs instance IDs).

• Bug Fixes / Compatibility Improvements

  • Stronger compatibility checks for closed schemas (additionalProperties: false) with orphaning detection and depth guard; added trait-schema inheritance compatibility.

• Documentation

  • Updated guidance to reflect strict resolution semantics.

• Chores / Tests

  • Expanded tests (OP#12/OP#13 and $ref resolution) and bumped spec version to v0.12.1.

[coderabbitai]

Warning

Review limit reached

@aviator5, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 39 minutes. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0dae3bd0-44a0-42e6-b596-a40cf67fec58

📥 Commits

Reviewing files that changed from the base of the PR and between 3751a8c and fdcbc0a.

📒 Files selected for processing (3)

• gts/src/schema_resolver.rs
• gts/src/schema_resolver_test.rs
• gts/src/store.rs

📝 Walkthrough

Walkthrough

The PR makes schema $ref resolution strict, refactors schema compatibility checks, adds trait-schema compatibility validation, updates entity validation routing, and removes instance-side schema-only keyword checks. Tests and related docs were updated to match the new behavior.

Changes

Strict resolution and compatibility validation

Layer / File(s)	Summary
Strict schema resolution gts/src/schema_resolver.rs, gts/src/store.rs, gts/src/schema_refs.rs, gts/src/schema_resolver_test.rs, gts-macros/tests/*.rs, gts/src/store_test.rs	SchemaResolver::resolve and GtsStore::resolve_schema_refs now return errors for unresolved or circular refs, store validation paths use the strict resolver, retriever wiring is removed from payload validation, and tests switch to checked resolution.
Schema compatibility checks gts/src/schema_compat.rs, gts/src/store_test.rs	validate_schema_compatibility now accepts raw schema values, computes effective schemas internally, and also checks closed descendant branches with additionalProperties: false; the compatibility tests were updated to use raw schemas and cover the new branch-orphan cases.
Trait-schema compatibility validation gts/src/schema_traits.rs, gts/src/store_test.rs	EffectiveTraits::validate now runs an additional trait-schema compatibility pass, and the new tests cover abstract trait-schema inheritance cases, including closed descendants, nested orphaning, and valid narrowing.
Entity validation routing gts/src/ops.rs, gts/src/schema_modifiers.rs	GtsOps now routes entity validation by parsed ID type, treats wildcard IDs as non-types, removes schema-only keyword checks from instance validation, and updates tests to match the new schema/instance split.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• GlobalTypeSystem/gts-rust#55: Introduces schema compatibility logic that this PR refactors and expands with descendant-branch validation.
• GlobalTypeSystem/gts-rust#57: Adds trait-schema validation machinery that this PR extends with compatibility checks across inherited trait schemas.
• GlobalTypeSystem/gts-rust#103: Also changes the $ref resolution path in SchemaResolver and GtsStore, overlapping the same resolution API surface.

Suggested reviewers

• Artifizer

Poem

🐇 I hopped through refs and found the way,
No dangling roots can hide today.
Traits now compare, and branches stay in line,
The schema burrow runs just fine.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: stricter schema resolution and validation behavior across GTS entities.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a4860f90-503e-462f-9769-1047315565a2

📥 Commits

Reviewing files that changed from the base of the PR and between a645f4d and 14c52df.

📒 Files selected for processing (2)

• gts/src/schema_resolver.rs
• gts/src/store.rs

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 98.01877% with 19 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
gts/src/schema_resolver.rs	92.30%	8 Missing ⚠️
gts/src/ops.rs	92.15%	4 Missing ⚠️
gts/src/schema_traits.rs	98.44%	3 Missing ⚠️
gts/src/schema_compat.rs	99.09%	2 Missing ⚠️
gts/src/store.rs	81.81%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[coderabbitai]

Actionable comments posted: 5

🤖 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 `@gts/src/ops.rs`:
- Around line 644-654: Preserve anonymous-instance handling in validate_entity
by not immediately returning on GtsId::try_new failure; instead, let
add_entity/validate_instance-style raw instance IDs continue through instance
validation and only use parsed_id when parsing succeeds. Update the
validate_entity flow around GtsId::try_new and the subsequent store lookup so
anonymous IDs are validated via the existing instance path rather than rejected
as invalid upfront.

In `@gts/src/schema_compat.rs`:
- Around line 232-234: The depth guard in the recursive compatibility check
currently returns silently in the validation path, which can let deeply nested
descendants bypass orphan-property validation. Update the recursion in
schema_compat.rs around the depth check in the compatibility traversal to fail
closed by reporting a validation error when MAX_RECURSION_DEPTH is hit instead
of treating the truncated subtree as compatible, using the same validation flow
that handles other incompatibilities.
- Around line 242-247: The orphaned-property check in
validate_closed_descendant_branches only looks at descendant properties and
misses valid matches from patternProperties. Update the filter around the
ancestor.properties iteration to treat an ancestor property as satisfied when it
is either explicitly present in descendant_props or matched by any pattern in
descendant_obj.get("patternProperties"). Preserve the existing
additionalProperties=false logic, but extend the compatibility check so schema
names like userId aren’t falsely flagged when a matching patternProperties entry
allows them.

In `@gts/src/schema_resolver_test.rs`:
- Around line 3-5: The module docs in schema_resolver_test are stale: they still
refer to SchemaResolver::try_resolve even though the tests now cover
SchemaResolver::resolve. Update the wording in the test module comment to match
the current method name so the documentation stays accurate and points to the
correct resolver API.

In `@gts/src/schema_resolver.rs`:
- Line 10: Update the module-level doc comment in schema_resolver so it refers
to the current store wrapper name, GtsStore::resolve_schema_refs, instead of
try_resolve_schema_refs; keep the wording aligned with the existing thin-wrapper
description and make sure the doc points readers to the symbol used by the
current call sites.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: b80e055b-1ac5-40ef-bd38-0562f4596db5

📥 Commits

Reviewing files that changed from the base of the PR and between 14c52df and 6b29be6.

📒 Files selected for processing (12)

• .gts-spec-version
• gts-macros/tests/inheritance_tests.rs
• gts-macros/tests/integration_tests.rs
• gts/src/ops.rs
• gts/src/schema_compat.rs
• gts/src/schema_modifiers.rs
• gts/src/schema_refs.rs
• gts/src/schema_resolver.rs
• gts/src/schema_resolver_test.rs
• gts/src/schema_traits.rs
• gts/src/store.rs
• gts/src/store_test.rs

💤 Files with no reviewable changes (1)

• gts/src/schema_modifiers.rs

✅ Files skipped from review due to trivial changes (2)

• .gts-spec-version
• gts/src/schema_refs.rs

[Artifizer]

I think there’s a medium-risk correctness issue in gts/src/schema_resolver.rs around remote pointer-fragment resolution.

When resolving a ref like gts://...#/some/fragment, the resolver selects that fragment with content.pointer(pointer) and then recursively resolves only the fragment subtree. However, local refs inside that fragment, such as #/$defs/Foo, are still interpreted as local refs and preserved as-is rather than being resolved against the remote document root.

That means a remote fragment can be inlined while still containing #/$defs/... refs whose targets live in the original remote document root, but whose $defs are no longer present in the inlined result. The resolved schema can therefore contain dangling local refs.

Concretely, a remote schema like:

{
  "$defs": { "Name": { "type": "string" } },
  "properties": {
    "name": { "$ref": "#/$defs/Name" }
  }
}

referenced as gts://...#/properties/name would currently inline the name fragment but leave #/$defs/Name unresolved, even though that pointer only makes sense relative to the remote root document.

I think the resolver should either:

1. resolve local #/... refs against the remote root before returning the fragment, or
2. reject remote fragments that still contain unresolved local refs.

A regression test covering this case would be useful.

[coderabbitai]

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 `@gts/src/schema_resolver.rs`:
- Around line 96-124: Handle the root self-reference case in
schema_resolver::resolve_inner by treating the exact "$ref" value "#" as a local
root pointer, not as an external reference. Update the branch that currently
matches only references starting with "`#/`" so it also includes "#", then resolve
it through local_root.pointer and the existing visited/cycle_found path to keep
circular reference detection consistent with other local refs.

🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 9d8a1e3f-6b0d-437e-a32f-d1d38102a74e

📥 Commits

Reviewing files that changed from the base of the PR and between 02ca1ac and 3751a8c.

📒 Files selected for processing (3)

• gts/src/schema_resolver.rs
• gts/src/schema_resolver_test.rs
• gts/src/store.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• gts/src/store.rs

[aviator5]

I think there’s a medium-risk correctness issue in gts/src/schema_resolver.rs around remote pointer-fragment resolution.

When resolving a ref like gts://...#/some/fragment, the resolver selects that fragment with content.pointer(pointer) and then recursively resolves only the fragment subtree. However, local refs inside that fragment, such as #/$defs/Foo, are still interpreted as local refs and preserved as-is rather than being resolved against the remote document root.

That means a remote fragment can be inlined while still containing #/$defs/... refs whose targets live in the original remote document root, but whose $defs are no longer present in the inlined result. The resolved schema can therefore contain dangling local refs.

Concretely, a remote schema like:

{
  "$defs": { "Name": { "type": "string" } },
  "properties": {
    "name": { "$ref": "#/$defs/Name" }
  }
}

referenced as gts://...#/properties/name would currently inline the name fragment but leave #/$defs/Name unresolved, even though that pointer only makes sense relative to the remote root document.

I think the resolver should either:

1. resolve local #/... refs against the remote root before returning the fragment, or
2. reject remote fragments that still contain unresolved local refs.

A regression test covering this case would be useful.

good catch, fixed