Skip to content

uphold schema prints the JSON Schema of principles.toml, committed and drift-tested - #417

Merged
HackingGate merged 1 commit into
mainfrom
principles-schema
Oct 23, 2026
Merged

HackingGate merged 1 commit into
mainfrom
principles-schema

Conversation

@HackingGate

@HackingGate HackingGate commented Oct 23, 2026 •

Copy link
Copy Markdown
Owner

uphold schema prints a JSON Schema (draft 2020-12) for principles.toml, modelled on term-server openapi: the binary prints its own spec, the output is committed, and a golden test fails while the two differ.

What changed

  • schemars derives the schema from the serde structs that read the policy file, so deny_unknown_fields becomes additionalProperties: false, rename becomes the key a person types, and doc comments become descriptions. src/schema.rs is the generator; uphold schema takes no arguments.
  • docs/schema/principles.schema.json is the committed output. tests/schema_cli.rs fails while it differs from uphold schema; UPHOLD_SCHEMA_GOLDEN=bless cargo test --test schema_cli rewrites it.
  • The same test uses jsonschema (dev-dependency) to check the file against the 2020-12 metaschema, validate policy/principles.toml and every bundled set against it, and confirm that documents the deserializer refuses (unknown rule key, wrong types, bad enum values, a forbidden override key) are refused by the schema too.
  • Release asset: dist-workspace.toml gains an extra-artifacts entry shipping principles.schema.json and principles.schema.json.sha256. release.yml is untouched: dist generate with the allow-dirty line removed differs only in the existing Dependabot actions/checkout pin.
  • docs/REFERENCE.md has a new section (and table of contents entry) and README.md a line, covering regeneration and the taplo #:schema line pointing at the release asset.

Decisions to look at

  • There are no custom Deserialize impls, deserialize_with or untagged enums on the read path. Two things needed handling instead. [override.<id>] is held as a raw toml::Table and read by hand in Override::of_table, so OverrideShape stands in for the schema, and a unit test writes every key it declares into a table and reads it back through of_table. TOML has no null, so schemars' null alternatives for Option fields are stripped in src/schema.rs (it has no setting for that).
  • dist writes a .sha256 for archives and the source tarball only, and its sha256.sum does not list extra artifacts (checked with dist build --artifacts=global). The extra artifact's build command therefore writes the checksum itself with sha256sum. I verified it locally; the first real run is the next tag.
  • The schema describes shape, not loading. A rule with two checks, a [set] header outside a bundled set, owner beside owner_from, and non-compiling regexes are refused after the read; REFERENCE.md says so.
  • Descriptions are the field doc comments verbatim, rustdoc links included, so editor hovers carry some [Type] markup.
  • tests/structural_documentation.rs: documenting PolicyFile retired its KNOWN exemption, so that entry is removed.
  • cargo-deny passes with no change to deny.toml; the new crates (schemars, jsonschema and their trees) are all under admitted licences.

Not in this PR

No version bump or tag. The #:schema URL in the docs resolves once a release carries the asset.

https://claude.ai/code/session_01YTpdnDJrf2hT6hFyjS7evA

Summary by CodeRabbit

  • New Features
    • Added uphold schema to print a JSON Schema for policy files, with editor support guidance.
    • Included the schema and its checksum among release artifacts.
  • Documentation
    • Added reference documentation explaining schema coverage, validation limits, and editor integration.
  • Tests
    • Added checks for schema validity, policy-file compatibility, and command output consistency.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 33 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 581fa47c-70a6-413b-a10a-88ef034fa821
📥 Commits

Reviewing files that changed from the base of the PR and between 69646ef and b8ee4c9.

⛔ Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 13
  • Cargo.toml
  • README.md
  • dist-workspace.toml
  • docs/REFERENCE.md
  • docs/schema/principles.schema.json
  • src/config.rs
  • src/config/rule.rs
  • src/main.rs
  • src/remote_policy.rs
  • src/schema.rs
  • src/shim.rs
  • tests/schema_cli.rs
  • tests/structural_documentation.rs

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: fe7eab33-88e6-4e5a-9ffb-0e2d64408916
📥 Commits

Reviewing files that changed from the base of the PR and between 8f0d3d4 and 69646ef.

⛔ Files ignored due to path filters: 1
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing: 13
  • Cargo.toml
  • README.md
  • dist-workspace.toml
  • docs/REFERENCE.md
  • docs/schema/principles.schema.json
  • src/config.rs
  • src/config/rule.rs
  • src/main.rs
  • src/remote_policy.rs
  • src/schema.rs
  • src/shim.rs
  • tests/schema_cli.rs
  • tests/structural_documentation.rs
💤 Files with no reviewable changes: 1
  • tests/structural_documentation.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The pull request adds JSON Schema generation for policy files and an uphold schema command. It commits the generated schema, includes the schema and checksum in release artifacts, and adds tests for schema validity and policy-file coverage.

Changes: Policy JSON Schema

Layer / File(s) Summary
Schema model and document generation
Cargo.toml, src/config.rs, src/config/rule.rs, src/remote_policy.rs, src/shim.rs, src/schema.rs, docs/schema/principles.schema.json
Configuration types derive JsonSchema. schema::document generates Draft 2020-12 JSON, removes null representations, and formats the output. The committed schema defines policy-file shapes.
CLI command and schema distribution
src/main.rs, README.md, docs/REFERENCE.md, dist-workspace.toml
uphold schema prints the generated schema when called without arguments and returns a usage error when given arguments. Documentation describes the command and editor support. Release configuration packages the schema and its checksum.
Schema validation and consistency checks
Cargo.toml, src/config.rs, tests/schema_cli.rs, tests/structural_documentation.rs
Tests compare CLI output with the committed schema, validate the schema and policy examples, check rejection cases, and compare override schema keys with parser keys.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant UpholdCLI
  participant schema_document
  participant schemars
  participant PolicyOutput
  UpholdCLI->>schema_document: Call document
  schema_document->>schemars: Generate schema for PolicyFile
  schemars->>schema_document: Return schema object
  schema_document->>UpholdCLI: Return formatted JSON
  UpholdCLI->>PolicyOutput: Print schema
Loading
Merge Risk: ⚪ Minimal · up to 69646

This change adds a read-only uphold schema command, a committed JSON Schema, and release artifacts for the schema and its checksum. It does not alter policy loading or scanning behavior. No concrete merge-blocking risk was identified.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 68.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 7 files. (5 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding uphold schema to print the JSON Schema for principles.toml, with committed and drift-tested output.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 68.42% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 19 functions across 7 files. (5 skipped: 5 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

✨ Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

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

@codecov-commenter

codecov-commenter commented Oct 23, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 99.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 94.87%. Comparing base (d84013c) to head (b8ee4c9).

Files with missing lines Patch % Lines
src/main.rs 80.00% 1 Missing ⚠️

❌ Your patch status has failed because the patch coverage (99.00%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #417      +/-   ##
==========================================
+ Coverage   94.85%   94.87%   +0.01%     
==========================================
  Files          52       53       +1     
  Lines       27259    27359     +100     
==========================================
+ Hits        25857    25957     +100     
  Misses       1402     1402              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

HackingGate added a commit that referenced this pull request Oct 23, 2026
…rrent passes again (#419)

Upstream tagged v0.17.2 while #417 and #418 were in CI, and the manual-stage
pin check failed both on a line neither touched. ruff check and ruff format
under v0.17.2 report nothing new over the whole tree.

Claude-Session: https://claude.ai/code/session_01YTpdnDJrf2hT6hFyjS7evA
…itted copy is tested against the binary, the metaschema and the repository's own policies

The schema is derived with schemars from the structs that deserialize the
policy file, so deny_unknown_fields, rename and default mean to the schema what
they mean to the reader. TOML has no null, so the generator's null alternatives
are removed. The [override.<id>] table is read by hand and is described by a
stand-in type; a unit test feeds every key of it to the reader.

docs/schema/principles.schema.json is what the binary prints, and
tests/schema_cli.rs fails while it differs (UPHOLD_SCHEMA_GOLDEN=bless writes
it). The same test validates the file against the 2020-12 metaschema with
jsonschema, runs policy/principles.toml and every bundled set through it, and
checks that documents the deserializer refuses are refused.

The release carries the schema and a .sha256 beside it as dist extra
artifacts. dist writes no checksum for an extra artifact, so the build command
writes it; release.yml is unchanged by this.

Claude-Session: https://claude.ai/code/session_01YTpdnDJrf2hT6hFyjS7evA
@HackingGate
HackingGate merged commit 952d42a into main Oct 23, 2026
12 checks passed
@HackingGate
HackingGate deleted the principles-schema branch October 23, 2026 13:55
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.

2 participants