-
Notifications
You must be signed in to change notification settings - Fork 0
Schema versioning #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds CI workflows for Java and Rust that read Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant Repo as Repository
participant GH as GitHub Actions
participant JavaMod as java/ (working dir)
participant Maven as Maven
participant GHReleases as GitHub Releases
Dev->>Repo: Push to schemas/**, java/**, or pom.xml (or manual)
Repo-->>GH: Trigger ci-java.yml
GH->>Repo: actions/checkout
GH->>GH: Setup JDK 17, restore Maven cache
GH->>Repo: Read ../schemas/VERSION -> schema_version
GH->>JavaMod: Read Maven impl version -> impl_version
GH->>GH: Create combined_version = "<impl_version>-schema-<schema_version>"
GH->>JavaMod: mvn versions:set (set pom to combined_version) & mvn package && mvn deploy
GH->>GHReleases: Create release tag java-v<combined_version>, attach java/target/pacts-<combined_version>*.jar
GH-->>Dev: Workflow complete
sequenceDiagram
autonumber
actor Dev as Developer
participant Repo as Repository
participant GH as GitHub Actions
participant RustMod as rust/ (working dir)
participant Cargo as Cargo
participant GHReleases as GitHub Releases
Dev->>Repo: Push to schemas/** or rust/** (or manual)
Repo-->>GH: Trigger ci-rust.yml
GH->>Repo: actions/checkout
GH->>GH: Setup Rust toolchain, restore Cargo cache
GH->>Repo: Read ../schemas/VERSION -> schema_version
GH->>RustMod: Read Cargo.toml impl_version
GH->>GH: Create combined_version = "<impl_version>-schema.<schema_version>"
GH->>RustMod: cargo set-version <combined_version>
GH->>Cargo: cargo build --release && cargo test
GH->>GHReleases: Create release tag rust-v<combined_version>, attach rust/target/release/libpacts.rlib
GH-->>Dev: Workflow complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches
🧪 Generate unit tests
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 40
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
schemas/bees/v1/inventory/inventory_item.json (2)
78-81: Nested nectar slot should be integer; add minimum.- "slot": { - "type": "number", - "description": "Nectar slot number" - }, + "slot": { + "type": "integer", + "minimum": 0, + "description": "Nectar slot number" + },
92-99: boosts should be integer ≥ 0.- "boosts": { - "type": "number", - "description": "Number of boosts applied" - } + "boosts": { + "type": "integer", + "minimum": 0, + "description": "Number of boosts applied" + }schemas/bees/v1/profile/profile_response.json (1)
50-57: profiles: replace “nullable” with union type.Also fine to keep description with $ref.
- "type": "array", + "type": ["array", "null"], "description": "Profile data", - "nullable": true, "readOnly": true, "items": { "description": "Array of profiles", "$ref": "profile.json" }
🧹 Nitpick comments (36)
rust/Cargo.toml (1)
3-3: Confirm crate version strategy vs schema versionCargo.toml is 0.0.4 but CI reportedly overwrites to schemas/VERSION for releases. Ensure ci-rust does that, or bump here pre-merge to avoid confusion between local builds and published crate versions.
java/pom.xml (1)
8-8: POM version vs CI-set versionPOM is 1.0.0, while the workflow sets it to schemas/VERSION at build time. If that’s intentional, fine; otherwise consider aligning pre-merge to reduce diff-only churn.
rust/build.rs (1)
12-17: Also watch the VERSION file explicitly for rebuildsAdd a rerun-if-changed for the version file to avoid relying solely on directory mtime.
- println!("cargo:rerun-if-changed={}", schemas_dir.display()); + println!("cargo:rerun-if-changed={}", schemas_dir.display()); + println!("cargo:rerun-if-changed={}", version_file.display());.github/workflows/ci-java.yml (1)
3-11: Optional: tighten triggersYou already match java/**; "pom.xml" at repo root may be unnecessary in a multi-module layout. Consider restricting or adding path-ignore for non-release changes to avoid noisy releases.
.github/workflows/ci-rust.yml (2)
60-71: Gate releases to main and prevent tag collisions; attach portable package
- Limit release creation to main to avoid duplicate tags from PRs.
- Attach the crate package (.crate) instead of a platform-specific .rlib.
- name: Build run: cargo build --release - name: Run tests run: cargo test + - name: Package crate + run: cargo package + - name: Create GitHub Release + if: github.ref == 'refs/heads/main' uses: softprops/action-gh-release@v2 with: tag_name: rust-v${{ steps.schema.outputs.schema_version }} name: Rust Crate v${{ steps.schema.outputs.schema_version }} body: | Rust crate release for schema version ${{ steps.schema.outputs.schema_version }} Changes: - Updated schemas to version ${{ steps.schema.outputs.schema_version }} files: | - rust/target/release/libpacts.rlib + rust/target/package/*.crate
43-49: Validate schemas/VERSION is SemVerAdd a quick guard to fail fast on invalid versions.
- name: Read Schema Version id: schema run: | SCHEMA_VERSION=$(cat ../schemas/VERSION | tr -d '\n') echo "Using schema version: $SCHEMA_VERSION" + [[ "$SCHEMA_VERSION" =~ ^[0-9]+\.[0-9]+\.[0-9]+(-[0-9A-Za-z\.-]+)?(\+[0-9A-Za-z\.-]+)?$ ]] || { echo "Invalid SemVer: $SCHEMA_VERSION"; exit 1; } echo "schema_version=$SCHEMA_VERSION" >> $GITHUB_OUTPUTschemas/bees/v1/profile/profile.json (4)
5-7: Update description or reintroduce invitations to align with schemaDescription mentions “invitations” but the property is absent. Adjust the description or add an invitations property.
Possible fix (description):
- "description": "A user profile containing members and invitations", + "description": "A user profile containing members",
2-3: Prefer stable, absolute $id URIs for reliable $ref resolutionUsing a bare filename may confuse resolvers. Consider an absolute, versioned ID.
- "$id": "profile.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/profile/profile.json",Align sibling $refs accordingly across the repo.
19-22: Avoid placing validation keywords alongside $refDraft-07 ignores validation siblings when $ref is present; “type”: "object" here is ineffective. Keep only $ref or wrap in allOf if needed.
- "type": "object", - "description": "Array of profile members", - "$ref": "profile_member.json" + "$ref": "profile_member.json"If you want to keep the description, wrap:
"items": { "allOf": [ { "$ref": "profile_member.json" } ], "description": "Array of profile members" }
10-13: Use “examples” (array) instead of non-standard “example”“example” is not defined in JSON Schema draft-07. Prefer “examples”.
- "example": "987e6543-e21b-34c3-b456-426614174999" + "examples": ["987e6543-e21b-34c3-b456-426614174999"]schemas/bees/v1/invitation/invitation_request.json (4)
20-25: Clarify “target_id” description to match the nameName says “target”; description says “requester”. Align to avoid confusion.
- "description": "The UUID of the requester", + "description": "The UUID of the target entity (e.g., player or resource)",
2-4: Adopt absolute, versioned $idFor consistent ref resolution and documentation links, prefer an absolute $id.
- "$id": "invitation_request.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/invitation/invitation_request.json",
10-14: Use “examples” instead of “example” across propertiesDraft-07 defines “examples” (array). Replace occurrences for better tool support.
Example:
- "example": "2025-09-05T15:30:00Z" + "examples": ["2025-09-05T15:30:00Z"]Apply similarly to request_type, target_id, recipient_id.
Also applies to: 16-19, 21-25, 31-31
39-39: Confirm additionalProperties: true is intentionalIf the API is closed over time, consider “false” plus explicit extension container fields instead.
schemas/bees/v1/invitation/invitation.json (4)
2-3: Stabilize $id to an absolute URIUse a canonical, versioned ID to improve cross-repo $ref resolution and docs linking.
- "$id": "invitation.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/invitation/invitation.json",
21-31: Replace “example” with draft-07 “examples”Swap to “examples” arrays for sender_id, receiver_id, status, created_at, profile_id, party_id.
Example:
- "example": "PENDING" + "examples": ["PENDING"]Also applies to: 33-37, 39-49
57-68: Enforce exactly one of profile_id or party_id (if that’s the intent)Current anyOf allows both present. If mutual exclusivity is desired, use oneOf with not constraints.
Minimal oneOf variant:
- "anyOf": [ + "oneOf": [ { "required": ["profile_id"] }, { "required": ["party_id"] } ],Strict XOR (optional):
"allOf": [ { "oneOf": [ { "required": ["profile_id"] }, { "required": ["party_id"] } ] }, { "not": { "required": ["profile_id","party_id"] } } ]
69-69: Re-evaluate additionalProperties: trueIf schema is public and versioned, consider tightening later to reduce drift.
schemas/bees/v1/invitation/invitation_response.json (3)
2-3: Provide absolute, versioned $idStandardize $id for portability.
- "$id": "invitation_response.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/invitation/invitation_response.json",
10-14: Prefer “examples” over “example”Update all example annotations to “examples”.
Example:
- "example": "OK" + "examples": ["OK"]Also applies to: 18-19, 23-24, 29-30, 33-36, 39-45
53-53: Keep additionalProperties: true under reviewIf clients rely on strict contracts, consider setting to false and adding explicit extension fields.
schemas/bees/v1/player/player_request.json (2)
21-24: Use “examples” for target_id. Consider enumerating valid request types.Also consider adding an enum for request_type to tighten the contract.
- "description": "The UUID of the target player", - "example": "123e4567-e89b-12d3-a456-426614174000" + "description": "The UUID of the target player", + "examples": ["123e4567-e89b-12d3-a456-426614174000"]
31-32: Confirm additionalProperties policy.True enables forward-compat but weakens validation. If strictness is desired for v1, set to false and explicitly allow extensions via a namespaced field.
schemas/bees/v1/profile/profile_request.json (1)
31-32: Re-evaluate additionalProperties.Same rationale as in player_request.json; confirm intended validation strictness for requests.
schemas/bees/v1/inventory/inventory_item.json (1)
64-67: Item level is typically an integer.Consider constraining to integer ≥ 0.
- "level": { - "type": "number", - "description": "Item level" - }, + "level": { + "type": "integer", + "minimum": 0, + "description": "Item level" + },schemas/bees/v1/profile/profile_member.json (1)
21-28: Use “examples” for role. Consider enum completeness.If other roles exist (e.g., ADMIN), expand enum.
"type": "string", "description": "Role of the member within the profile", "enum": [ "OWNER", "MEMBER" ], - "example": "MEMBER" + "examples": ["MEMBER"]schemas/bees/v1/profile/profile_response.json (3)
29-33: Align target_id description and use “examples”.The description says “target player”; requests allow “profile or player”. Align wording.
- "description": "The UUID of the target player", + "description": "The UUID of the target profile or player", "readOnly": true, - "example": "123e4567-e89b-12d3-a456-426614174000" + "examples": ["123e4567-e89b-12d3-a456-426614174000"]
65-66: Confirm additionalProperties policy for responses.Same forward-compat vs strictness trade-off as requests.
1-67: Normalizeexampleandnullableusage across all JSON schemas
Convert inlineexamplevalues to standardizedexamplesarrays and replace"nullable": truewith explicit union types (type: ["null", …]) in every file underschemas/to enforce consistency.schemas/bees/v1/player/player_response.json (2)
14-14: Prefer “examples” (array) over non-standard “example” in draft-07“example” is not defined in draft-07; “examples” is. Tools may ignore “example”.
E.g.:
- "example": "2025-09-05T15:30:00Z" + "examples": ["2025-09-05T15:30:00Z"]Repeat similarly for other properties using example.
Also applies to: 20-20, 26-26, 33-33, 40-40
67-67: additionalProperties: true — confirm intentRelaxing from false to true (if that’s the change) broadens accepted payloads and can hide typos. If you want strictness, set to false and explicitly extend via $defs.
schemas/bees/v1/inventory/inventory_request.json (3)
62-78: oneOf is sufficient; “not … allOf(inventory,items)” is redundant and can be droppedoneOf across the three alternatives already prevents multiple from matching simultaneously (assuming non-null semantics). The extra not adds noise.
Additionally, if you intend non-empty arrays, enforce it:
"inventory": { - "type": ["array", "null"], + "type": "array", + "minItems": 1, "description": "An array of inventory items, typically the entire inventory.", - "nullable": true, "items": { "$ref": "inventory_item.json" } }, "items": { - "type": ["array", "null"], + "type": "array", + "minItems": 1, "description": "An array of inventory items, typically the items to be added or removed.", - "nullable": true, "items": { "$ref": "inventory_item.json" } }And remove the redundant not block:
- "not": { - "allOf": [ - { "required": ["inventory"] }, - { "required": ["items"] } - ] - },If nulls are intended, keep the unions and add "nullable" behavior via unions only.
Also applies to: 79-92
13-13: Use “examples” instead of “example” for draft-07 compatibilitySwitch to the standard annotation.
Example:
- "example": "INVENTORY_GET" + "examples": ["INVENTORY_GET"]Also applies to: 19-19, 25-25, 31-31, 53-53
93-93: additionalProperties: true — confirm schema opennessIf strict contracts are desired for requests, set to false and explicitly whitelist.
schemas/bees/v1/inventory/inventory_response.json (2)
13-13: Switch non-standard “example” to “examples”Aligns with draft-07.
- "example": "OK" + "examples": ["OK"]Apply similarly to the other example fields, including the inventory property’s example (wrap the array inside examples: [ … ]).
Also applies to: 19-19, 24-24, 30-30, 36-36, 51-57
67-67: additionalProperties: true — confirmBroader acceptance can mask issues.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
rust/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.github/workflows/ci-java.yml(1 hunks).github/workflows/ci-rust.yml(1 hunks)java/pom.xml(3 hunks)rust/Cargo.toml(1 hunks)rust/build.rs(1 hunks)schemas/VERSION(1 hunks)schemas/bees/v1/inventory/inventory_item.json(2 hunks)schemas/bees/v1/inventory/inventory_request.json(3 hunks)schemas/bees/v1/inventory/inventory_response.json(2 hunks)schemas/bees/v1/invitation/invitation.json(1 hunks)schemas/bees/v1/invitation/invitation_request.json(1 hunks)schemas/bees/v1/invitation/invitation_response.json(1 hunks)schemas/bees/v1/player/player_request.json(1 hunks)schemas/bees/v1/player/player_response.json(2 hunks)schemas/bees/v1/profile/profile.json(2 hunks)schemas/bees/v1/profile/profile_invitation.json(0 hunks)schemas/bees/v1/profile/profile_member.json(1 hunks)schemas/bees/v1/profile/profile_request.json(1 hunks)schemas/bees/v1/profile/profile_response.json(2 hunks)
💤 Files with no reviewable changes (1)
- schemas/bees/v1/profile/profile_invitation.json
🧰 Additional context used
🧬 Code graph analysis (3)
schemas/VERSION (1)
java/src/main/java/net/hydrius/pacts/core/SchemaLoader.java (1)
getVersion(164-166)
rust/build.rs (3)
rust/src/core/schema_loader.rs (6)
EmbeddedSchemas(12-12)SchemaLoader(16-21)get_version(160-162)new(30-41)SchemaLoader(23-172)get_parsed_version(168-170)rust/src/model/header.rs (1)
schema_version(52-54)rust/src/impl/service.rs (1)
create_envelope(25-38)
.github/workflows/ci-rust.yml (2)
rust/src/impl/service.rs (2)
PactsService(12-97)PactsService(7-10)rust/src/lib.rs (2)
test_basic_workflow(36-57)tests(12-58)
🪛 actionlint (1.7.7)
.github/workflows/ci-java.yml
35-35: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
62-62: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
.github/workflows/ci-rust.yml
27-27: the runner of "actions-rs/toolchain@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
33-33: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
61-61: the runner of "softprops/action-gh-release@v1" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (6)
java/pom.xml (1)
25-25: Including schemas/VERSION in resources is goodEnsures Java can access the schema version at runtime/tests. LGTM.
Also applies to: 35-35
rust/build.rs (1)
19-26: Good: trim + expose SCHEMA_VERSIONStraightforward and safe handling (CRLF-safe). LGTM.
.github/workflows/ci-rust.yml (1)
35-39: Fix cache path so target dir is actually cachedWith defaults.run.working-directory: rust, actions/cache “path” is still relative to repo root. Caching “target” won’t hit; use “rust/target”.
Run a build twice and confirm cache hit after changing:
- target + rust/targetschemas/bees/v1/profile/profile.json (1)
28-29: Confirm additionalProperties: false won’t break existing producersLocking down unknown fields is good, but verify no extra fields are currently emitted in production.
schemas/bees/v1/inventory/inventory_item.json (1)
33-36: Use “examples” for enchantment_id.- "description": "The enchantment identifier", - "example": "minecraft:sharpness" + "description": "The enchantment identifier", + "examples": ["minecraft:sharpness"]Likely an incorrect or invalid review comment.
schemas/bees/v1/player/player_response.json (1)
51-58: Replace broken $ref for invitations- "items": { - "$ref": "../profile/profile_invitation.json" - } + "items": { + "$ref": "../invitation/invitation_response.json" + }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
26 issues found across 20 files
React with 👍 or 👎 to teach cubic. You can also tag @cubic-dev-ai to give feedback, ask questions, or re-run the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
java/src/main/java/net/hydrius/pacts/core/Validator.java (3)
137-144: Support union types in "type" (e.g., ["array","null"]).Current code treats an array
typeas empty string and silently passes. Validate against any allowed member.- if (schema.has("type")) { - String expectedType = schema.get("type").asText(); - if (!validateType(dataNode, expectedType)) { - errors.add("Invalid type. Expected: " + expectedType); - } - } + if (schema.has("type")) { + var typeNode = schema.get("type"); + boolean ok = true; + String expectedForMsg = null; + if (typeNode.isArray()) { + ok = false; + StringBuilder allowed = new StringBuilder(); + for (JsonNode t : typeNode) { + String ty = t.asText(); + if (allowed.length() > 0) allowed.append("|"); + allowed.append(ty); + if (validateType(dataNode, ty)) { + ok = true; + break; + } + } + expectedForMsg = allowed.toString(); + } else { + String expectedType = typeNode.asText(); + expectedForMsg = expectedType; + ok = validateType(dataNode, expectedType); + } + if (!ok) { + errors.add("Invalid type. Expected: " + expectedForMsg); + } + }
137-144: Enforce "format": uuid and date-time.Schemas now rely on formats; add minimal checks.
if (schema.has("type")) { ... } + if (schema.has("format") && dataNode.isTextual()) { + String format = schema.get("format").asText(); + String v = dataNode.asText(); + switch (format) { + case "uuid" -> { + try { java.util.UUID.fromString(v); } + catch (IllegalArgumentException ex) { errors.add("Invalid format: uuid"); } + } + case "date-time" -> { + try { java.time.Instant.parse(v); } + catch (Exception ex) { errors.add("Invalid format: date-time"); } + } + default -> { /* ignore other formats for now */ } + } + }Additional imports required:
import java.util.UUID; import java.time.Instant;
61-74: Schema version in header is ignored when loading the schema.Given schema versioning is introduced, consider loading/validating against
schemaVersionand failing fast on mismatches.rust/src/core/validator.rs (2)
166-174: Support union types for "type".Handle
typeas an array of allowed primitives (e.g., ["array","null"]).- if let Some(type_value) = schema.get("type") { - if let Some(expected_type) = type_value.as_str() { - if !self.validate_type(data, expected_type) { - errors.push(format!("Invalid type. Expected: {}", expected_type)); - } - } - } + if let Some(type_value) = schema.get("type") { + match type_value { + Value::String(s) => { + if !self.validate_type(data, s) { + errors.push(format!("Invalid type. Expected: {}", s)); + } + } + Value::Array(arr) => { + let mut ok = false; + let mut allowed = Vec::new(); + for v in arr { + if let Some(s) = v.as_str() { + allowed.push(s.to_string()); + if self.validate_type(data, s) { ok = true; break; } + } + } + if !ok { + errors.push(format!("Invalid type. Expected one of: {}", allowed.join("|"))); + } + } + _ => {} + } + }
106-114: Refactor SchemaLoader.load_schema to return Result and handle failures in Validator
Load_schema currently panics on missing schemas (rust/src/core/schema_loader.rs:54); change its signature to return Result<Value, anyhow::Error> and, in rust/src/core/validator.rs:106–114, match on that Result—pushing a “schema not found” validation error on Err instead of unwrapping and panicking.
♻️ Duplicate comments (8)
.github/workflows/ci-rust.yml (2)
25-29: Good: Node20-ready actions and cache v4 in placedtolnay/rust-toolchain, actions/cache@v4, and softprops/action-gh-release@v2 address the Node16 deprecations.
Also applies to: 30-37, 60-62
51-53: Version bump not persisted to repoYou’re mutating Cargo.toml in CI without committing; tags will diverge from main’s Cargo.toml. Either commit the bump on main or derive the tag purely from schemas/VERSION and don’t edit Cargo.toml.
Option A — commit on main:
- name: Update Cargo version run: cargo set-version ${{ steps.schema.outputs.schema_version }} + + - name: Commit version bump + if: github.ref == 'refs/heads/main' + run: | + git config user.name "github-actions[bot]" + git config user.email "github-actions[bot]@users.noreply.github.com" + git add Cargo.toml Cargo.lock + git commit -m "chore(rust): bump crate version to ${{ steps.schema.outputs.schema_version }}" + git pushOption B — skip editing Cargo.toml and just tag (remove the set-version step and keep release tagging).
schemas/bees/v1/invitation/invitation.json (1)
75-86: Ensure exactly one of profile_id or party_id. Use oneOf, not anyOf.anyOf permits both; oneOf forbids both being present.
- "anyOf": [ + "oneOf": [ { "required": ["profile_id"] }, { "required": ["party_id"] } ],schemas/bees/v1/inventory/inventory_item.json (2)
55-62: enchantment_level should be integer ≥ 1.- "enchantment_level": { - "type": "number", + "enchantment_level": { + "type": "integer", "description": "The level of the enchantment", "minimum": 1,
29-36: amount should be an integer.Quantities are discrete; aligns with new integer handling in validators.
- "amount": { - "type": "number", + "amount": { + "type": "integer", "description": "The quantity of the item", "minimum": 1,schemas/bees/v1/profile/profile_member.json (1)
3-3: Canonical, stable $id (absolute URL) to avoid resolver ambiguity.Use the published HTTPS location for this schema instead of a bare filename.
- "$id": "profile_member.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/profile/profile_member.json",schemas/bees/v1/player/player_response.json (1)
3-3: Stabilize $id using an absolute HTTPS URL.Align with other schemas and avoid ref base confusions.
- "$id": "player_response.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/player/player_response.json",schemas/bees/v1/inventory/inventory_response.json (1)
3-3: Use a canonical absolute $id.Replace the local filename with a stable HTTPS identifier.
- "$id": "inventory_response.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/inventory/inventory_response.json",
🧹 Nitpick comments (20)
.github/workflows/ci-rust.yml (4)
33-39: Scope cache key to Rust lockfileMinor: using **/Cargo.lock can cause unnecessary cache churn if other lockfiles exist. Prefer the crate lockfile explicitly.
- key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + key: ${{ runner.os }}-cargo-${{ hashFiles('rust/Cargo.lock') }} restore-keys: | ${{ runner.os }}-cargo-
41-47: Validate schema version valueAdd a quick guard so empty or non-SemVer values don’t propagate to tags/releases.
- name: Read Schema Version id: schema run: | SCHEMA_VERSION=$(cat ../schemas/VERSION | tr -d '\n') + if ! [[ "$SCHEMA_VERSION" =~ ^[0-9]+(\.[0-9]+){2}(-[0-9A-Za-z.-]+)?(\+[0-9A-Za-z.-]+)?$ ]]; then + echo "Invalid schema version: '$SCHEMA_VERSION'"; exit 1 + fi echo "Using schema version: $SCHEMA_VERSION" echo "schema_version=$SCHEMA_VERSION" >> $GITHUB_OUTPUT
48-49: Faster cargo-edit installUsing a prebuilt installer speeds CI noticeably.
- - name: Install cargo-edit - run: cargo install cargo-edit + - name: Install cargo-edit + uses: taiki-e/install-action@v2 + with: + tool: cargo-edit
71-71: Verify artifact pathGiven working-directory: rust, the artifact also exists at target/release/libpacts.rlib. Current path is fine; if builds run from subdirs later, prefer a matrix/upload-artifact. Just confirm the crate outputs an rlib.
java/src/main/java/net/hydrius/pacts/core/Validator.java (1)
170-182: No recursive validation of nested properties/items.Child object/array constraints (e.g., nested enums, min/max) aren’t enforced. Consider recursing into object properties and array items.
rust/src/core/validator.rs (1)
137-149: Consider basic "format" checks (uuid, date-time).Add optional checks using
uuidandtime/chronocrates to honor schema formats.schemas/bees/v1/invitation/invitation.json (2)
3-3: Use canonical, absolute $id.Provide a dereferenceable, versioned URL to improve tooling interoperability.
- "$id": "invitation.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/invitation/invitation.json",
87-87: Consider tightening additionalProperties.If the payload should be closed, set
additionalProperties: falseto reject unexpected fields.schemas/bees/v1/inventory/inventory_item.json (2)
3-3: Use canonical, absolute $id.- "$id": "inventory_item.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/inventory/inventory_item.json",
31-31: Note: min constraints aren’t enforced by validators.Current Java/Rust validators don’t check minimum/maximum; consider implementing numeric constraints if you rely on them.
Also applies to: 57-57
schemas/bees/v1/profile/profile_response.json (2)
3-3: Use canonical, absolute $id.- "$id": "profile_response.json", + "$id": "https://schemas.project-hydrius.dev/bees/v1/profile/profile_response.json",
76-79: Prefer absolute $ref for cross-file references.Use a canonical URL (aligned with $id) to simplify external tooling resolution.
- "$ref": "profile.json" + "$ref": "https://schemas.project-hydrius.dev/bees/v1/profile/profile.json"schemas/bees/v1/profile/profile_member.json (1)
59-65: Confirm intent: readOnly field in required.
created_atis readOnly yet required; fine for response models, but it will block create/update payloads if this schema is reused there. Consider splitting read vs write schemas or usingallOfoverlays.Would you like a quick split into read/write variants via
allOf?schemas/bees/v1/player/player_response.json (1)
88-88: Consider locking the contract (additionalProperties: false).Responses are currently open-ended. If you need forward-compat extension, prefer a dedicated
extensionsobject rather than global openness.- "additionalProperties": true + "additionalProperties": falseschemas/bees/v1/inventory/inventory_response.json (6)
10-16: Mark server-populated fields as readOnly for consistency.
datelooks server-set; mirror PlayerResponse style."type": "string", "format": "date-time", "description": "The date and time of the response, in ISO 8601 format.", + "readOnly": true,
19-24: Make request_type readOnly (response metadata)."type": "string", "description": "The type of request that was made (e.g., 'INVENTORY_GET', 'INVENTORY_UPDATE').", + "readOnly": true,
27-32: Make response_status readOnly (response metadata)."type": "string", "description": "The status of the response.", + "readOnly": true,
35-41: Make target_id readOnly (echoed identifier)."type": "string", "format": "uuid", "description": "The UUID of the target player or entity.", + "readOnly": true,
44-50: Make profile_id readOnly (echoed identifier)."type": "string", "format": "uuid", "description": "The UUID of the profile.", + "readOnly": true,
103-103: Consider closing the schema (additionalProperties: false).Prevents accidental contract drift; add an explicit
extensionsmap if needed.- "additionalProperties": true + "additionalProperties": false
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (16)
.github/workflows/ci-java.yml(1 hunks).github/workflows/ci-rust.yml(1 hunks)java/src/main/java/net/hydrius/pacts/core/Validator.java(3 hunks)rust/src/core/validator.rs(1 hunks)schemas/bees/v1/inventory/inventory_item.json(3 hunks)schemas/bees/v1/inventory/inventory_request.json(3 hunks)schemas/bees/v1/inventory/inventory_response.json(2 hunks)schemas/bees/v1/invitation/invitation.json(1 hunks)schemas/bees/v1/invitation/invitation_request.json(1 hunks)schemas/bees/v1/invitation/invitation_response.json(1 hunks)schemas/bees/v1/player/player_request.json(1 hunks)schemas/bees/v1/player/player_response.json(2 hunks)schemas/bees/v1/profile/profile.json(2 hunks)schemas/bees/v1/profile/profile_member.json(1 hunks)schemas/bees/v1/profile/profile_request.json(1 hunks)schemas/bees/v1/profile/profile_response.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- schemas/bees/v1/invitation/invitation_request.json
- schemas/bees/v1/profile/profile_request.json
- .github/workflows/ci-java.yml
- schemas/bees/v1/profile/profile.json
- schemas/bees/v1/inventory/inventory_request.json
- schemas/bees/v1/invitation/invitation_response.json
- schemas/bees/v1/player/player_request.json
🧰 Additional context used
🧬 Code graph analysis (2)
rust/src/core/validator.rs (1)
rust/src/model/envelope.rs (1)
data(47-49)
.github/workflows/ci-rust.yml (3)
rust/build.rs (1)
main(5-17)rust/src/impl/service.rs (2)
PactsService(12-97)PactsService(7-10)rust/src/lib.rs (1)
test_basic_workflow(36-57)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Publish Rust Crate
🔇 Additional comments (1)
schemas/bees/v1/profile/profile_response.json (1)
10-17: LGTM on metadata, examples, and nullable unions.The switch to plural examples, uuid/date-time formats, and union types is consistent and clear.
Also applies to: 20-27, 29-36, 38-46, 48-58, 60-67, 70-79, 83-88
| "anyOf": [ | ||
| { | ||
| "required": [ | ||
| "profile_id" | ||
| ] | ||
| }, | ||
| { | ||
| "required": [ | ||
| "party_id" | ||
| ] | ||
| } | ||
| ], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Heads-up: current validators don’t enforce anyOf/oneOf.
Even with oneOf, Java/Rust validators ignore composition keywords; the constraint won’t be checked until validator support is added.
Would you like a patch to add minimal anyOf/oneOf support to both validators?
🏁 Script executed:
#!/bin/bash
set -e
echo "=== Locating build files ==="
JAVA_BUILD_FILES=$(find . -maxdepth 2 -type f \( -name "pom.xml" -o -name "build.gradle*" \))
if [ -n "$JAVA_BUILD_FILES" ]; then
echo "Java build files found:"
echo "$JAVA_BUILD_FILES"
echo "--- JSON schema dependencies in Java build files ---"
grep -R "json[-_]schema\|jsonschema4j\|networknt" -n $JAVA_BUILD_FILES || echo "No JSON schema–related dependencies in Java build files."
else
echo "No Java build files detected."
fi
if [ -f Cargo.toml ]; then
echo "Rust Cargo.toml found."
echo "--- JSON schema crates in Cargo.toml ---"
grep -R "schemars\|jsonschema" -n Cargo.toml || echo "No JSON schema–related crates listed in Cargo.toml."
else
echo "No Rust Cargo.toml detected."
fi
echo "=== Searching Java source for anyOf/oneOf support ==="
rg -nP "(anyOf|oneOf)" --type java -C3 || echo "No composition keywords (anyOf/oneOf) referenced in Java source."
echo "=== Searching Rust source for anyOf/oneOf support ==="
rg -nP "(anyOf|oneOf)" --type rust -C3 || echo "No composition keywords (anyOf/oneOf) referenced in Rust source."Length of output: 1301
Integrate a JSON Schema validator with anyOf/oneOf support.
The invitation schema’s anyOf constraint won’t be enforced—there’s currently no JSON Schema library in Java (pom.xml has no validator deps) or Rust (no Cargo.toml)—so add and configure a validator that supports composition before adding any minimal anyOf/oneOf logic.
🤖 Prompt for AI Agents
In schemas/bees/v1/invitation/invitation.json around lines 75-86 the anyOf
constraint will not be enforced because there is no JSON Schema validator
configured in the project; add and configure a validator that supports
composition (anyOf/oneOf) in the language(s) you use: for Java add a dependency
to a compliant validator (e.g., networknt/json-schema-validator or
everit-org/json-schema) in pom.xml and instantiate/validate the invitation
schema with that library; for Rust add a crate such as jsonschema to Cargo.toml
and validate the schema at runtime (or build-time) using that crate; wire the
validator into the code path that loads/validates these JSON schemas, update
error handling to surface validation failures, and add unit tests exercising
both branches (profile_id present and party_id present) to confirm anyOf
behavior.
| "$ref": "../profile/profile_invitation.json" | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Broken $ref: profile_invitation.json was removed in this PR.
Update to the new invitation schema path to restore validation.
- "items": {
- "$ref": "../profile/profile_invitation.json"
- }
+ "items": {
+ "$ref": "../invitation/invitation.json"
+ }🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Refs still pointing to removed profile_invitation.json:"
rg -nP '\$ref"\s*:\s*"\.\./profile/profile_invitation\.json' schemas || true
echo "Check new invitation schema presence:"
fd invitation.json schemas/bees/v1/invitation || trueLength of output: 483
Fix broken $ref to the new invitation schema
In schemas/bees/v1/player/player_response.json (line 78), update the reference:
- "items": {
- "$ref": "../profile/profile_invitation.json"
- }
+ "items": {
+ "$ref": "../invitation/invitation.json"
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "$ref": "../profile/profile_invitation.json" | |
| } | |
| "items": { | |
| "$ref": "../invitation/invitation.json" | |
| } |
🤖 Prompt for AI Agents
In schemas/bees/v1/player/player_response.json around lines 78-79, the "$ref":
"../profile/profile_invitation.json" is broken; update that $ref to point to the
new invitation schema location (use the correct relative path and filename where
the invitation schema now lives), verify the target file exists and the path
resolves, and run a schema validation to confirm the reference is fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
.github/workflows/ci-rust.yml (2)
4-8: Restrict push trigger or gate publish steps to mainPrevents releases/tags from non-main branches. This mirrors prior feedback.
on: push: + branches: + - main paths: - "schemas/**" - "rust/**"
74-88: Add tag-exists check and gate release to mainAvoid duplicate releases and off-branch tags. This reflects earlier guidance.
+ - name: Fetch tags + run: git fetch --tags --depth=1 + + - name: Check if tag exists + id: tagcheck + run: | + if git rev-parse -q --verify "refs/tags/rust-v${{ steps.version.outputs.combined_version }}"; then + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + - name: Create GitHub Release - uses: softprops/action-gh-release@v2 + if: github.ref == 'refs/heads/main' && steps.tagcheck.outputs.exists == 'false' + uses: softprops/action-gh-release@v2
🧹 Nitpick comments (6)
.github/workflows/ci-java.yml (3)
3-9: Gate publishes to main to avoid releasing from feature branchesKeep builds on branches, but only publish/release on main.
on: push: + branches: + - main paths: - "schemas/**" - "java/**" - "pom.xml"
65-69: Only deploy on main; streamline Maven flagsPrevent accidental deploys and speed up CI.
- - name: Publish to GitHub Packages + - name: Publish to GitHub Packages + if: github.ref == 'refs/heads/main' env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - GITHUB_ACTOR: ${{ github.actor }} - run: mvn deploy --batch-mode + run: mvn -B -ntp -DskipTests deploy
71-88: Skip release if tag already exists; gate to mainAvoid duplicate/misplaced releases.
+ - name: Fetch tags + run: git fetch --tags --depth=1 + + - name: Check if tag exists + id: tagcheck + run: | + if git rev-parse -q --verify "refs/tags/java-v${{ steps.version.outputs.combined_version }}"; then + echo "exists=true" >> "$GITHUB_OUTPUT" + else + echo "exists=false" >> "$GITHUB_OUTPUT" + fi + - name: Create GitHub Release - uses: softprops/action-gh-release@v2 + if: github.ref == 'refs/heads/main' && steps.tagcheck.outputs.exists == 'false' + uses: softprops/action-gh-release@v2 with: tag_name: java-v${{ steps.version.outputs.combined_version }}.github/workflows/ci-rust.yml (3)
25-29: Use dtolnay/rust-toolchain@stable and add components@stable is the recommended pin; include clippy/rustfmt for consistency.
- - name: Install Rust - uses: dtolnay/rust-toolchain@v1 - with: - toolchain: stable + - name: Install Rust + uses: dtolnay/rust-toolchain@stable + with: + components: clippy,rustfmt
48-53: Make impl version extraction robust (limit to [package])Current grep may catch the wrong “version”. Use awk scoped to [package] without extra deps.
- IMPL_VERSION=$(grep '^version = ' Cargo.toml | sed 's/version = "\(.*\)"/\1/') + IMPL_VERSION=$(awk 'BEGIN{in_pkg=0} /^\[package\]/{in_pkg=1; next} in_pkg && /^version *=/ {match($0, /"([^"]+)"/, a); print a[1]; exit}' Cargo.toml)
62-67: Version bump isn’t persisted; consider committing or deriving from tagRight now Cargo.toml is modified only in CI, causing tag/repo drift. Either commit/push on main or skip mutating Cargo.toml and derive version purely for tagging.
Option A — commit on main:
- name: Update Cargo version run: cargo set-version ${{ steps.version.outputs.combined_version }} + + - name: Commit version bump + if: github.ref == 'refs/heads/main' + run: | + git config user.name "github-actions[bot]" + git config user.email "41898282+github-actions[bot]@users.noreply.github.com" + git add Cargo.toml + git commit -m "chore(rust): set version to ${{ steps.version.outputs.combined_version }} [skip ci]" || echo "No changes" + git pushOption B — don’t change Cargo.toml; keep combined_version only for tag/release metadata.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
.github/workflows/ci-java.yml(1 hunks).github/workflows/ci-rust.yml(1 hunks)java/src/main/java/net/hydrius/pacts/core/Validator.java(3 hunks)rust/src/core/validator.rs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- rust/src/core/validator.rs
- java/src/main/java/net/hydrius/pacts/core/Validator.java
🧰 Additional context used
🧬 Code graph analysis (1)
.github/workflows/ci-rust.yml (3)
rust/build.rs (1)
main(5-17)rust/src/lib.rs (1)
test_basic_workflow(36-57)rust/src/impl/service.rs (2)
PactsService(12-97)PactsService(7-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build and Publish Rust Crate
🔇 Additional comments (2)
.github/workflows/ci-java.yml (1)
27-36: Setup-java and POM settings verified
id “github” and primary pacts confirmed in java/pom.xml; configuration is correct..github/workflows/ci-rust.yml (1)
30-40: Cargo cache paths are correctGood use of actions/cache@v4 and rust/target path.
This adds schema versioning, and workflows should publish new releases on update, including schema only pushes. Java implementation is actively being used and seems stable enough to release, it works as expected. Rust needs more testing, so it has only bumped a patch version. This should allow everything to independently update if needed.
Summary by cubic
Introduces schema versioning and automated releases for Java and Rust tied to a new schemas/VERSION file. Adds richer JSON Schemas and a new invitation model to support clearer APIs and independent updates.
New Features
Migration
Summary by CodeRabbit
New Features
Bug Fixes
Chores