diff --git a/CHANGELOG.md b/CHANGELOG.md index dc5acd1..5b9693a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,24 @@ All notable changes to the Delimit GitHub Action will be documented in this file. +## [1.9.0] - 2026-04-07 + +### Features +- **JSON Schema support** (LED-713) — first-class governance for bare JSON Schema files (Draft 4+, single-file, internal `$ref` resolution). Complements OpenAPI without changing the existing path. Routes via `core/spec_detector.detect_spec_type()`. +- **Generator drift detection** — new `generator_command` + `generator_artifact` inputs. When set, the action runs the regen command in a sandbox and diffs the regenerated output against the committed artifact. Catches the case where source-of-truth (Zod, protobuf, OpenAPI generator, etc.) has changed but the committed generated file is stale. Workspace is cleanly restored after the check. +- **`fail_on_breaking` input** — boolean alias for `mode=enforce`. When `true`, fails CI on breaking changes regardless of `mode`. Defaults to `false`. +- **PR comment renderer** updated with a JSON Schema branch that shows drift report + classification table. + +### v1 JSON Schema change types +Property add/remove, required add/remove, type widen/narrow, enum value add/remove, `const` change, `additionalProperties` flip, `pattern` tighten/loosen, `minLength`/`maxLength`, `minimum`/`maximum`, `items` type. Composition keywords (`anyOf`/`oneOf`/`allOf`), discriminators, external `$ref`, and `if/then/else` deferred past v1. + +### Tests +- 41 new unit tests in `tests/test_json_schema_diff.py` covering every v1 change type, $ref resolution at root and nested paths, dispatcher routing, and the agents-oss/agentspec rename as a real-world fixture +- 47 existing OpenAPI tests still passing (no regressions) + +### Why +Validating delimit-action against agents-oss/agentspec#21 (issue inviting integration) revealed that the diff engine returned `0 changes` on bare JSON Schema files. The maintainer had explicitly invited a PR. This release closes the gap before that PR opens. + ## [1.7.0] - 2026-03-26 ### Features diff --git a/README.md b/README.md index cbba11a..c7a66af 100644 --- a/README.md +++ b/README.md @@ -1,572 +1,7 @@ -# `` Delimit GitHub Action - -**Catch breaking API changes before merge** — semver classification, migration guides, and policy enforcement for OpenAPI specs. - -[![GitHub Marketplace](https://img.shields.io/badge/Marketplace-Delimit-blue)](https://github.com/marketplace/actions/delimit-api-governance) -[![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT) -[![API Governance](https://delimit-ai.github.io/badge/pass.svg)](https://github.com/marketplace/actions/delimit-api-governance) - -Delimit runs on every pull request, compares your OpenAPI spec against the base branch, and posts a detailed comment with breaking changes, semver classification, policy violations, and migration guidance. No API keys, no external services, no config required to get started. - -## What it looks like - -

- Delimit PR comment showing breaking changes -

- ---- - -## Features - -- **Breaking change detection** — catches 27 types of changes (17 breaking, 10 non-breaking) across endpoints, parameters, response schemas, types, enums, security, and constraints -- **Semver classification** — deterministic `major` / `minor` / `patch` / `none` bump recommendation with computed next version -- **Migration guides** — auto-generated step-by-step migration instructions for every breaking change -- **PR comments** — rich Markdown summary posted directly on your pull request, updated on each push -- **Advisory and enforce modes** — start with non-blocking warnings, promote to CI-gating when ready -- **Custom policies** — define your own governance rules in `.delimit/policies.yml` with path patterns, severity levels, and custom messages -- **7 explainer templates** — developer, team lead, product, migration, changelog, PR comment, and Slack formats - ---- - -## Quick Start - -Add this file to `.github/workflows/api-check.yml`: - -```yaml -name: API Contract Check -on: pull_request - -jobs: - delimit: - runs-on: ubuntu-latest - permissions: - pull-requests: write - steps: - - uses: actions/checkout@v4 - - uses: delimit-ai/delimit-action@v1 - with: - spec: api/openapi.yaml -``` - -That is it. Delimit auto-fetches the base branch version of your spec and diffs it against the PR changes. Runs in **advisory mode** by default — posts a PR comment but never fails your build. - -### What the PR comment looks like - -When Delimit detects breaking changes, it posts a comment like this: - -> **Delimit API Governance** | Breaking Changes Detected -> -> | Change | Path | Severity | -> |--------|------|----------| -> | endpoint_removed | `DELETE /pets/{petId}` | error | -> | type_changed | `/pets:GET:200[].id` (string → integer) | warning | -> | enum_value_removed | `/pets:GET:200[].status` | warning | -> -> **Semver**: MAJOR (1.0.0 → 2.0.0) -> ->
Migration Guide (3 steps) -> -> **Step 1**: `DELETE /pets/{petId}` was removed. Update clients to use an alternative endpoint or remove calls to this path. -> -> **Step 2**: `id` changed from `string` to `integer`. Update serialization logic, type assertions, and database column types. -> -> **Step 3**: `status` enum value `"pending"` was removed. Update clients to stop sending this value. -> ->
- -See the [live demo](https://github.com/delimit-ai/delimit-action-demo/pull/2) — a Users API migration with 23 breaking changes detected across 27 change types, severity badges, and a migration guide. - -### Advanced: explicit base and head specs - -If you need to compare specific files (e.g., pre-checked-out base branch), use `old_spec` and `new_spec` instead: - -```yaml - - uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml -``` - ---- - -## Full Usage - -```yaml -name: API Governance -on: pull_request - -jobs: - api-check: - runs-on: ubuntu-latest - permissions: - pull-requests: write - steps: - - uses: actions/checkout@v4 - - - name: Checkout base spec - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - path: base - - - uses: delimit-ai/delimit-action@v1 - id: delimit - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - mode: enforce - policy_file: .delimit/policies.yml - github_token: ${{ secrets.GITHUB_TOKEN }} - - - name: Use outputs - if: always() - run: | - echo "Breaking changes: ${{ steps.delimit.outputs.breaking_changes_detected }}" - echo "Violations: ${{ steps.delimit.outputs.violations_count }}" - echo "Semver bump: ${{ steps.delimit.outputs.semver_bump }}" - echo "Next version: ${{ steps.delimit.outputs.next_version }}" -``` - ---- - -## Inputs - -| Input | Required | Default | Description | -|-------|----------|---------|-------------| -| `spec` | No | `''` | Path to the changed OpenAPI spec. On pull requests, Delimit auto-fetches the base branch version for comparison. | -| `old_spec` | No | `''` | Path to the old/base API specification file. | -| `new_spec` | No | `''` | Path to the new/changed API specification file. | -| `mode` | No | `advisory` | `advisory` (comments only) or `enforce` (fails CI on breaking changes). | -| `github_token` | No | `${{ github.token }}` | GitHub token used to post PR comments. | -| `policy_file` | No | `''` | Path to a custom policy file (e.g., `.delimit/policies.yml`). | - -> **Note**: Provide either `spec` for pull request workflows, or both `old_spec` and `new_spec` for explicit comparisons. If neither form is provided, the action exits with an error. - ---- - -## Outputs - -| Output | Type | Description | -|--------|------|-------------| -| `breaking_changes_detected` | `string` | `"true"` if any breaking change was found, `"false"` otherwise. | -| `violations_count` | `string` | Number of policy violations (errors + warnings). | -| `semver_bump` | `string` | Recommended version bump: `major`, `minor`, `patch`, or `none`. | -| `next_version` | `string` | Computed next version string (e.g., `2.0.0`). | -| `report` | `string` | Full JSON report of all detected changes, violations, and semver data. | - -### Using outputs in subsequent steps - -```yaml -- uses: delimit-ai/delimit-action@v1 - id: delimit - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - -- name: Block release on breaking changes - if: steps.delimit.outputs.breaking_changes_detected == 'true' - run: | - echo "Breaking changes detected — semver bump: ${{ steps.delimit.outputs.semver_bump }}" - echo "Next version should be: ${{ steps.delimit.outputs.next_version }}" - exit 1 - -- name: Auto-tag on minor bump - if: steps.delimit.outputs.semver_bump == 'minor' - run: | - git tag "v${{ steps.delimit.outputs.next_version }}" -``` - ---- - -## Custom Policies - -Create `.delimit/policies.yml` in your repository root to define governance rules beyond the defaults. - -```yaml -# .delimit/policies.yml - -# Set to true to replace all default rules with only your custom rules. -# Default: false (custom rules merge with defaults). -override_defaults: false - -rules: - # Forbid removing endpoints without deprecation - - id: no_endpoint_removal - name: Forbid Endpoint Removal - change_types: - - endpoint_removed - severity: error # error | warning | info - action: forbid # forbid | allow | warn - message: "Endpoint {path} cannot be removed. Use deprecation headers instead." - - # Protect V1 API — no breaking changes allowed - - id: protect_v1_api - name: Protect V1 API - description: V1 endpoints are frozen - change_types: - - endpoint_removed - - method_removed - - field_removed - severity: error - action: forbid - conditions: - path_pattern: "^/v1/.*" - message: "V1 API is frozen. Changes must be made in V2." - - # Warn on type changes in 2xx responses - - id: warn_response_type_change - name: Warn Response Type Changes - change_types: - - type_changed - severity: warning - action: warn - conditions: - path_pattern: ".*:2\\d\\d.*" - message: "Type changed at {path} — verify client compatibility." - - # Allow adding enum values (informational) - - id: allow_enum_expansion - name: Allow Enum Expansion - change_types: - - enum_value_added - severity: info - action: allow - message: "Enum value added (non-breaking)." -``` - -### Available change types for rules - -| Change type | Breaking | Description | -|-------------|----------|-------------| -| `endpoint_removed` | Yes | An API endpoint path was removed | -| `method_removed` | Yes | An HTTP method was removed from an endpoint | -| `required_param_added` | Yes | A new required parameter was added | -| `param_removed` | Yes | A parameter was removed | -| `response_removed` | Yes | A response status code was removed | -| `required_field_added` | Yes | A new required field was added to a request body | -| `field_removed` | Yes | A field was removed from a response | -| `type_changed` | Yes | A field's type was changed (e.g., string to integer) | -| `format_changed` | Yes | A field's format was changed (e.g., date to date-time) | -| `enum_value_removed` | Yes | An allowed enum value was removed | -| `endpoint_added` | No | A new endpoint was added | -| `method_added` | No | A new HTTP method was added to an endpoint | -| `optional_param_added` | No | A new optional parameter was added | -| `response_added` | No | A new response status code was added | -| `optional_field_added` | No | A new optional field was added | -| `enum_value_added` | No | A new enum value was added | -| `description_changed` | No | A description was modified | - -### Default rules - -Delimit ships with 6 built-in rules that are always active unless you set `override_defaults: true`: - -1. **Forbid Endpoint Removal** — endpoints cannot be removed (error) -2. **Forbid Method Removal** — HTTP methods cannot be removed (error) -3. **Forbid Required Parameter Addition** — new required params break clients (error) -4. **Forbid Response Field Removal** — removing fields from 2xx responses (error) -5. **Warn on Type Changes** — type changes flagged as warnings -6. **Allow Enum Expansion** — adding enum values is always safe (info) - ---- - -## Slack / Discord Notifications - -Get notified in Slack or Discord when breaking API changes are detected. Add a `webhook_url` input pointing to your channel's incoming webhook: - -```yaml -- uses: delimit-ai/delimit-action@v1 - with: - spec: api/openapi.yaml - webhook_url: ${{ secrets.SLACK_WEBHOOK }} -``` - -The notification fires only when breaking changes are found. If the webhook URL is not set, this step is silently skipped. - -### Supported platforms - -| Platform | URL pattern | Payload format | -|----------|------------|----------------| -| **Slack** | `hooks.slack.com` | Block Kit with mrkdwn | -| **Discord** | `discord.com/api/webhooks` | Rich embed with color and fields | -| **Generic** | Anything else | Plain JSON event payload | - -Delimit auto-detects the platform from the URL and formats the message accordingly. Webhook failures are logged as warnings but never fail your CI run. - -### Discord example - -```yaml -- uses: delimit-ai/delimit-action@v1 - with: - spec: api/openapi.yaml - webhook_url: ${{ secrets.DISCORD_WEBHOOK }} -``` - -### Generic webhook - -Any URL that is not Slack or Discord receives a JSON payload: - -```json -{ - "event": "breaking_changes_detected", - "repo": "org/repo", - "pr_number": 123, - "pr_title": "Update user endpoints", - "breaking_changes": 3, - "additive_changes": 1, - "semver": "MAJOR", - "pr_url": "https://github.com/org/repo/pull/123" -} -``` - ---- - -## Advisory vs Enforce Mode - -| Behavior | `advisory` (default) | `enforce` | -|----------|---------------------|-----------| -| PR comment | Yes | Yes | -| GitHub annotations | Yes | Yes | -| Fails CI on breaking changes | **No** | **Yes** | -| Exit code on violations | `0` | `1` | - -**Start with advisory mode.** It gives your team visibility into API changes without blocking merges. Once your team is comfortable, switch to `enforce` to gate deployments. - -```yaml -# Advisory — non-blocking (default) -- uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - mode: advisory - -# Enforce — blocks merge on breaking changes -- uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - mode: enforce -``` - ---- - -## Examples - -### Advisory mode (recommended starting point) - -```yaml -name: API Check -on: pull_request - -jobs: - api-check: - runs-on: ubuntu-latest - permissions: - pull-requests: write - steps: - - uses: actions/checkout@v4 - - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - path: base - - uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml -``` - -### Enforce mode - -```yaml -name: API Governance -on: pull_request - -jobs: - api-check: - runs-on: ubuntu-latest - permissions: - pull-requests: write - steps: - - uses: actions/checkout@v4 - - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - path: base - - uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - mode: enforce -``` - -### Custom policy file - -```yaml -- uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - mode: enforce - policy_file: .delimit/policies.yml -``` - -### Using outputs to control downstream jobs - -```yaml -jobs: - api-check: - runs-on: ubuntu-latest - outputs: - breaking: ${{ steps.delimit.outputs.breaking_changes_detected }} - bump: ${{ steps.delimit.outputs.semver_bump }} - next_version: ${{ steps.delimit.outputs.next_version }} - steps: - - uses: actions/checkout@v4 - - uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - path: base - - uses: delimit-ai/delimit-action@v1 - id: delimit - with: - old_spec: base/api/openapi.yaml - new_spec: api/openapi.yaml - - deploy: - needs: api-check - if: needs.api-check.outputs.breaking != 'true' - runs-on: ubuntu-latest - steps: - - run: echo "Safe to deploy — next version ${{ needs.api-check.outputs.next_version }}" -``` - ---- - -## Supported Formats - -- OpenAPI 3.0 and 3.1 -- Swagger 2.0 -- YAML and JSON spec files - ---- - -## What the PR Comment Looks Like - -When Delimit detects changes, it posts (or updates) a comment on your pull request: - -``` -## Delimit: Breaking Changes `MAJOR` - -| Metric | Value | -|--------|-------| -| Semver bump | `major` | -| Next version | `2.0.0` | -| Total changes | 5 | -| Breaking | 2 | -| Violations | 2 | - -### Violations - -| Severity | Rule | Description | Location | -|----------|------|-------------|----------| -| Error | Forbid Endpoint Removal | Endpoint /users/{id} cannot be removed | `/users/{id}` | -| Warning | Warn on Type Changes | Type changed from string to integer | `/users:200.age` | - -
-Migration guide -...step-by-step instructions for each breaking change... -
-``` - -The comment is automatically updated on each push to the PR branch. No duplicate comments. - ---- - -## FAQ / Troubleshooting - -### Delimit skipped validation and did nothing - -Both `old_spec` and `new_spec` must be provided. If either is empty, Delimit exits cleanly with no output. Make sure both paths point to valid spec files. - -### How do I get the base branch spec? - -Use a second `actions/checkout` step to check out the base branch into a subdirectory: - -```yaml -- uses: actions/checkout@v4 - with: - ref: ${{ github.event.pull_request.base.sha }} - path: base -``` - -Then reference `base/path/to/openapi.yaml` as `old_spec`. - -### My spec file is not found - -Verify the path relative to the repository root. Common locations: -- `api/openapi.yaml` -- `docs/openapi.yaml` -- `openapi.yaml` -- `swagger.json` - -### The action posts duplicate PR comments - -Delimit searches for an existing comment containing "Delimit" from a bot user and updates it in place. If you see duplicates, ensure `github_token` has `pull-requests: write` permission. - -### Can I use this with JSON specs? - -Yes. Delimit supports both YAML (`.yaml`, `.yml`) and JSON (`.json`) spec files. Set the input paths accordingly. - -### Can I use this in a monorepo with multiple specs? - -Yes. Add multiple Delimit steps, each with different `old_spec` / `new_spec` pairs: - -```yaml -- uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/services/users/openapi.yaml - new_spec: services/users/openapi.yaml - -- uses: delimit-ai/delimit-action@v1 - with: - old_spec: base/services/billing/openapi.yaml - new_spec: services/billing/openapi.yaml -``` - -### Advisory mode still shows errors in the PR comment — is that expected? - -Yes. Advisory mode reports everything (including errors) in the PR comment and GitHub annotations, but it always exits with code `0` so your CI stays green. Switch to `enforce` mode when you want breaking changes to block the merge. - -### How is the semver bump calculated? - -The classification is deterministic: -- **major** — any breaking change detected (endpoint removed, required param added, field removed, type changed, etc.) -- **minor** — additive changes only (new endpoints, optional fields, enum values added) -- **patch** — non-functional changes only (description updates) -- **none** — no changes detected - ---- - -## CLI - -For local development, pre-commit checks, and CI/CD pipelines outside GitHub Actions, use the [Delimit CLI](https://www.npmjs.com/package/delimit-cli): - -```bash -npm install -g delimit-cli -delimit lint api/openapi.yaml -delimit diff old-api.yaml new-api.yaml -delimit explain old-api.yaml new-api.yaml --template migration -``` - ---- - -## Links - -- [Delimit CLI on npm](https://www.npmjs.com/package/delimit-cli) — Local development tool -- [GitHub Repository](https://github.com/delimit-ai/delimit) — Source code and issues -- [GitHub Action Marketplace](https://github.com/marketplace/actions/delimit-api-governance) — Install from Marketplace - +# `` Delimit GitHub Action\n\n**Catch breaking API changes before merge** — semver classification, migration guides, and policy enforcement for OpenAPI specs.\n\n --- -## License +## Think and Build -MIT +Delimit is more than just an action — it is a governance layer for your AI coding assistants. When you use the Delimit Swarm, you can say **"Think and Build"** to your agents, and they will automatically use this action to verify their own code changes against your API policies before requesting a merge. +\n[![GitHub Marketplace](https://img.shields.io/badge/Marketplace-Delimit-blue)](https://github.com/marketplace/actions/delimit-api-governance)\n[![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](https://opensource.org/licenses/MIT)\n[![API Governance](https://delimit-ai.github.io/badge/pass.svg)](https://github.com/marketplace/actions/delimit-api-governance)\n\nDelimit runs on every pull request, compares your OpenAPI spec against the base branch, and posts a detailed comment with breaking changes, semver classification, policy violations, and migration guidance. No API keys, no external services, no config required to get started.\n\n## What it looks like\n\n

\n Delimit PR comment showing breaking changes\n

\n\n---\n\n## Features\n\n- **Breaking change detection** — catches 27 types of changes (17 breaking, 10 non-breaking) across endpoints, parameters, response schemas, types, enums, security, and constraints\n- **Semver classification** — deterministic `major` / `minor` / `patch` / `none` bump recommendation with computed next version\n- **Migration guides** — auto-generated step-by-step migration instructions for every breaking change\n- **PR comments** — rich Markdown summary posted directly on your pull request, updated on each push\n- **Advisory and enforce modes** — start with non-blocking warnings, promote to CI-gating when ready\n- **Custom policies** — define your own governance rules in `.delimit/policies.yml` with path patterns, severity levels, and custom messages\n- **7 explainer templates** — developer, team lead, product, migration, changelog, PR comment, and Slack formats\n\n---\n\n## Quick Start\n\nAdd this file to `.github/workflows/api-check.yml`:\n\n```yaml\nname: API Contract Check\non: pull_request\n\njobs:\n delimit:\n runs-on: ubuntu-latest\n permissions:\n pull-requests: write\n steps:\n - uses: actions/checkout@v4\n - uses: delimit-ai/delimit-action@v1\n with:\n spec: api/openapi.yaml\n```\n\nThat is it. Delimit auto-fetches the base branch version of your spec and diffs it against the PR changes. Runs in **advisory mode** by default — posts a PR comment but never fails your build.\n\n### What the PR comment looks like\n\nWhen Delimit detects breaking changes, it posts a comment like this:\n\n> **Delimit API Governance** | Breaking Changes Detected\n>\n> | Change | Path | Severity |\n> |--------|------|----------|\n> | endpoint_removed | `DELETE /pets/{petId}` | error |\n> | type_changed | `/pets:GET:200[].id` (string → integer) | warning |\n> | enum_value_removed | `/pets:GET:200[].status` | warning |\n>\n> **Semver**: MAJOR (1.0.0 → 2.0.0)\n>\n>
Migration Guide (3 steps)\n>\n> **Step 1**: `DELETE /pets/{petId}` was removed. Update clients to use an alternative endpoint or remove calls to this path.\n>\n> **Step 2**: `id` changed from `string` to `integer`. Update serialization logic, type assertions, and database column types.\n>\n> **Step 3**: `status` enum value `"pending"` was removed. Update clients to stop sending this value.\n>\n>
\n\nSee the [live demo](https://github.com/delimit-ai/delimit-action-demo/pull/2) — a Users API migration with 23 breaking changes detected across 27 change types, severity badges, and a migration guide.\n\n### Advanced: explicit base and head specs\n\nIf you need to compare specific files (e.g., pre-checked-out base branch), use `old_spec` and `new_spec` instead:\n\n```yaml\n - uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n```\n\n---\n\n## Full Usage\n\n```yaml\nname: API Governance\non: pull_request\n\njobs:\n api-check:\n runs-on: ubuntu-latest\n permissions:\n pull-requests: write\n steps:\n - uses: actions/checkout@v4\n\n - name: Checkout base spec\n uses: actions/checkout@v4\n with:\n ref: ${{ github.event.pull_request.base.sha }}\n path: base\n\n - uses: delimit-ai/delimit-action@v1\n id: delimit\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n mode: enforce\n policy_file: .delimit/policies.yml\n github_token: ${{ secrets.GITHUB_TOKEN }}\n\n - name: Use outputs\n if: always()\n run: |\n echo "Breaking changes: ${{ steps.delimit.outputs.breaking_changes_detected }}"\n echo "Violations: ${{ steps.delimit.outputs.violations_count }}"\n echo "Semver bump: ${{ steps.delimit.outputs.semver_bump }}"\n echo "Next version: ${{ steps.delimit.outputs.next_version }}"\n```\n\n---\n\n## Inputs\n\n| Input | Required | Default | Description |\n|-------|----------|---------|-------------|\n| `spec` | No | `''` | Path to the changed OpenAPI spec. On pull requests, Delimit auto-fetches the base branch version for comparison. |\n| `old_spec` | No | `''` | Path to the old/base API specification file. |\n| `new_spec` | No | `''` | Path to the new/changed API specification file. |\n| `mode` | No | `advisory` | `advisory` (comments only) or `enforce` (fails CI on breaking changes). |\n| `github_token` | No | `${{ github.token }}` | GitHub token used to post PR comments. |\n| `policy_file` | No | `''` | Path to a custom policy file (e.g., `.delimit/policies.yml`). |\n\n> **Note**: Provide either `spec` for pull request workflows, or both `old_spec` and `new_spec` for explicit comparisons. If neither form is provided, the action exits with an error.\n\n---\n\n## Outputs\n\n| Output | Type | Description |\n|--------|------|-------------|\n| `breaking_changes_detected` | `string` | `"true"` if any breaking change was found, `"false"` otherwise. |\n| `violations_count` | `string` | Number of policy violations (errors + warnings). |\n| `semver_bump` | `string` | Recommended version bump: `major`, `minor`, `patch`, or `none`. |\n| `next_version` | `string` | Computed next version string (e.g., `2.0.0`). |\n| `report` | `string` | Full JSON report of all detected changes, violations, and semver data. |\n\n### Using outputs in subsequent steps\n\n```yaml\n- uses: delimit-ai/delimit-action@v1\n id: delimit\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n\n- name: Block release on breaking changes\n if: steps.delimit.outputs.breaking_changes_detected == 'true'\n run: |\n echo "Breaking changes detected — semver bump: ${{ steps.delimit.outputs.semver_bump }}"\n echo "Next version should be: ${{ steps.delimit.outputs.next_version }}"\n exit 1\n\n- name: Auto-tag on minor bump\n if: steps.delimit.outputs.semver_bump == 'minor'\n run: |\n git tag "v${{ steps.delimit.outputs.next_version }}"\n```\n\n---\n\n## Custom Policies\n\nCreate `.delimit/policies.yml` in your repository root to define governance rules beyond the defaults.\n\n```yaml\n# .delimit/policies.yml\n\n# Set to true to replace all default rules with only your custom rules.\n# Default: false (custom rules merge with defaults).\noverride_defaults: false\n\nrules:\n # Forbid removing endpoints without deprecation\n - id: no_endpoint_removal\n name: Forbid Endpoint Removal\n change_types:\n - endpoint_removed\n severity: error # error | warning | info\n action: forbid # forbid | allow | warn\n message: "Endpoint {path} cannot be removed. Use deprecation headers instead."\n\n # Protect V1 API — no breaking changes allowed\n - id: protect_v1_api\n name: Protect V1 API\n description: V1 endpoints are frozen\n change_types:\n - endpoint_removed\n - method_removed\n - field_removed\n severity: error\n action: forbid\n conditions:\n path_pattern: "^/v1/.*"\n message: "V1 API is frozen. Changes must be made in V2."\n\n # Warn on type changes in 2xx responses\n - id: warn_response_type_change\n name: Warn Response Type Changes\n change_types:\n - type_changed\n severity: warning\n action: warn\n conditions:\n path_pattern: ".*:2\\d\\d.*"\n message: "Type changed at {path} — verify client compatibility."\n\n # Allow adding enum values (informational)\n - id: allow_enum_expansion\n name: Allow Enum Expansion\n change_types:\n - enum_value_added\n severity: info\n action: allow\n message: "Enum value added (non-breaking)."\n```\n\n### Available change types for rules\n\n| Change type | Breaking | Description |\n|-------------|----------|-------------|\n| `endpoint_removed` | Yes | An API endpoint path was removed |\n| `method_removed` | Yes | An HTTP method was removed from an endpoint |\n| `required_param_added` | Yes | A new required parameter was added |\n| `param_removed` | Yes | A parameter was removed |\n| `response_removed` | Yes | A response status code was removed |\n| `required_field_added` | Yes | A new required field was added to a request body |\n| `field_removed` | Yes | A field was removed from a response |\n| `type_changed` | Yes | A field's type was changed (e.g., string to integer) |\n| `format_changed` | Yes | A field's format was changed (e.g., date to date-time) |\n| `enum_value_removed` | Yes | An allowed enum value was removed |\n| `endpoint_added` | No | A new endpoint was added |\n| `method_added` | No | A new HTTP method was added to an endpoint |\n| `optional_param_added` | No | A new optional parameter was added |\n| `response_added` | No | A new response status code was added |\n| `optional_field_added` | No | A new optional field was added |\n| `enum_value_added` | No | A new enum value was added |\n| `description_changed` | No | A description was modified |\n\n### Default rules\n\nDelimit ships with 6 built-in rules that are always active unless you set `override_defaults: true`:\n\n1. **Forbid Endpoint Removal** — endpoints cannot be removed (error)\n2. **Forbid Method Removal** — HTTP methods cannot be removed (error)\n3. **Forbid Required Parameter Addition** — new required params break clients (error)\n4. **Forbid Response Field Removal** — removing fields from 2xx responses (error)\n5. **Warn on Type Changes** — type changes flagged as warnings\n6. **Allow Enum Expansion** — adding enum values is always safe (info)\n\n---\n\n## Slack / Discord Notifications\n\nGet notified in Slack or Discord when breaking API changes are detected. Add a `webhook_url` input pointing to your channel's incoming webhook:\n\n```yaml\n- uses: delimit-ai/delimit-action@v1\n with:\n spec: api/openapi.yaml\n webhook_url: ${{ secrets.SLACK_WEBHOOK }}\n```\n\nThe notification fires only when breaking changes are found. If the webhook URL is not set, this step is silently skipped.\n\n### Supported platforms\n\n| Platform | URL pattern | Payload format |\n|----------|------------|----------------|\n| **Slack** | `hooks.slack.com` | Block Kit with mrkdwn |\n| **Discord** | `discord.com/api/webhooks` | Rich embed with color and fields |\n| **Generic** | Anything else | Plain JSON event payload |\n\nDelimit auto-detects the platform from the URL and formats the message accordingly. Webhook failures are logged as warnings but never fail your CI run.\n\n### Discord example\n\n```yaml\n- uses: delimit-ai/delimit-action@v1\n with:\n spec: api/openapi.yaml\n webhook_url: ${{ secrets.DISCORD_WEBHOOK }}\n```\n\n### Generic webhook\n\nAny URL that is not Slack or Discord receives a JSON payload:\n\n```json\n{\n "event": "breaking_changes_detected",\n "repo": "org/repo",\n "pr_number": 123,\n "pr_title": "Update user endpoints",\n "breaking_changes": 3,\n "additive_changes": 1,\n "semver": "MAJOR",\n "pr_url": "https://github.com/org/repo/pull/123"\n}\n```\n\n---\n\n## Advisory vs Enforce Mode\n\n| Behavior | `advisory` (default) | `enforce` |\n|----------|---------------------|-----------|\n| PR comment | Yes | Yes |\n| GitHub annotations | Yes | Yes |\n| Fails CI on breaking changes | **No** | **Yes** |\n| Exit code on violations | `0` | `1` |\n\n**Start with advisory mode.** It gives your team visibility into API changes without blocking merges. Once your team is comfortable, switch to `enforce` to gate deployments.\n\n```yaml\n# Advisory — non-blocking (default)\n- uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n mode: advisory\n\n# Enforce — blocks merge on breaking changes\n- uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n mode: enforce\n```\n\n---\n\n## Examples\n\n### Advisory mode (recommended starting point)\n\n```yaml\nname: API Check\non: pull_request\n\njobs:\n api-check:\n runs-on: ubuntu-latest\n permissions:\n pull-requests: write\n steps:\n - uses: actions/checkout@v4\n - uses: actions/checkout@v4\n with:\n ref: ${{ github.event.pull_request.base.sha }}\n path: base\n - uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n```\n\n### Enforce mode\n\n```yaml\nname: API Governance\non: pull_request\n\njobs:\n api-check:\n runs-on: ubuntu-latest\n permissions:\n pull-requests: write\n steps:\n - uses: actions/checkout@v4\n - uses: actions/checkout@v4\n with:\n ref: ${{ github.event.pull_request.base.sha }}\n path: base\n - uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n mode: enforce\n```\n\n### Custom policy file\n\n```yaml\n- uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n mode: enforce\n policy_file: .delimit/policies.yml\n```\n\n### Using outputs to control downstream jobs\n\n```yaml\njobs:\n api-check:\n runs-on: ubuntu-latest\n outputs:\n breaking: ${{ steps.delimit.outputs.breaking_changes_detected }}\n bump: ${{ steps.delimit.outputs.semver_bump }}\n next_version: ${{ steps.delimit.outputs.next_version }}\n steps:\n - uses: actions/checkout@v4\n - uses: actions/checkout@v4\n with:\n ref: ${{ github.event.pull_request.base.sha }}\n path: base\n - uses: delimit-ai/delimit-action@v1\n id: delimit\n with:\n old_spec: base/api/openapi.yaml\n new_spec: api/openapi.yaml\n\n deploy:\n needs: api-check\n if: needs.api-check.outputs.breaking != 'true'\n runs-on: ubuntu-latest\n steps:\n - run: echo "Safe to deploy — next version ${{ needs.api-check.outputs.next_version }}"\n```\n\n---\n\n## Supported Formats\n\n- OpenAPI 3.0 and 3.1\n- Swagger 2.0\n- YAML and JSON spec files\n\n---\n\n## What the PR Comment Looks Like\n\nWhen Delimit detects changes, it posts (or updates) a comment on your pull request:\n\n```\n## Delimit: Breaking Changes `MAJOR`\n\n| Metric | Value |\n|--------|-------|\n| Semver bump | `major` |\n| Next version | `2.0.0` |\n| Total changes | 5 |\n| Breaking | 2 |\n| Violations | 2 |\n\n### Violations\n\n| Severity | Rule | Description | Location |\n|----------|------|-------------|----------|\n| Error | Forbid Endpoint Removal | Endpoint /users/{id} cannot be removed | `/users/{id}` |\n| Warning | Warn on Type Changes | Type changed from string to integer | `/users:200.age` |\n\n
\nMigration guide\n...step-by-step instructions for each breaking change...\n
\n```\n\nThe comment is automatically updated on each push to the PR branch. No duplicate comments.\n\n---\n\n## FAQ / Troubleshooting\n\n### Delimit skipped validation and did nothing\n\nBoth `old_spec` and `new_spec` must be provided. If either is empty, Delimit exits cleanly with no output. Make sure both paths point to valid spec files.\n\n### How do I get the base branch spec?\n\nUse a second `actions/checkout` step to check out the base branch into a subdirectory:\n\n```yaml\n- uses: actions/checkout@v4\n with:\n ref: ${{ github.event.pull_request.base.sha }}\n path: base\n```\n\nThen reference `base/path/to/openapi.yaml` as `old_spec`.\n\n### My spec file is not found\n\nVerify the path relative to the repository root. Common locations:\n- `api/openapi.yaml`\n- `docs/openapi.yaml`\n- `openapi.yaml`\n- `swagger.json`\n\n### The action posts duplicate PR comments\n\nDelimit searches for an existing comment containing "Delimit" from a bot user and updates it in place. If you see duplicates, ensure `github_token` has `pull-requests: write` permission.\n\n### Can I use this with JSON specs?\n\nYes. Delimit supports both YAML (`.yaml`, `.yml`) and JSON (`.json`) spec files. Set the input paths accordingly.\n\n### Can I use this in a monorepo with multiple specs?\n\nYes. Add multiple Delimit steps, each with different `old_spec` / `new_spec` pairs:\n\n```yaml\n- uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/services/users/openapi.yaml\n new_spec: services/users/openapi.yaml\n\n- uses: delimit-ai/delimit-action@v1\n with:\n old_spec: base/services/billing/openapi.yaml\n new_spec: services/billing/openapi.yaml\n```\n\n### Advisory mode still shows errors in the PR comment — is that expected?\n\nYes. Advisory mode reports everything (including errors) in the PR comment and GitHub annotations, but it always exits with code `0` so your CI stays green. Switch to `enforce` mode when you want breaking changes to block the merge.\n\n### How is the semver bump calculated?\n\nThe classification is deterministic:\n- **major** — any breaking change detected (endpoint removed, required param added, field removed, type changed, etc.)\n- **minor** — additive changes only (new endpoints, optional fields, enum values added)\n- **patch** — non-functional changes only (description updates)\n- **none** — no changes detected\n\n---\n\n## CLI\n\nFor local development, pre-commit checks, and CI/CD pipelines outside GitHub Actions, use the [Delimit CLI](https://www.npmjs.com/package/delimit-cli):\n\n```bash\nnpm install -g delimit-cli\ndelimit lint api/openapi.yaml\ndelimit diff old-api.yaml new-api.yaml\ndelimit explain old-api.yaml new-api.yaml --template migration\n```\n\n---\n\n## Links\n\n- [Delimit CLI on npm](https://www.npmjs.com/package/delimit-cli) — Local development tool\n- [GitHub Repository](https://github.com/delimit-ai/delimit) — Source code and issues\n- [GitHub Action Marketplace](https://github.com/marketplace/actions/delimit-api-governance) — Install from Marketplace\n\n---\n\n## License\n\nMIT \ No newline at end of file diff --git a/action.yml b/action.yml index 2879723..3d6a321 100644 --- a/action.yml +++ b/action.yml @@ -36,6 +36,18 @@ inputs: description: 'Mode: advisory (comments only) or enforce (fail CI on breaking changes)' required: false default: 'advisory' + fail_on_breaking: + description: 'Boolean alias for mode=enforce. When true, fails CI on breaking changes regardless of mode. Default false (advisory).' + required: false + default: 'false' + generator_command: + description: 'Optional shell command that regenerates a generated artifact (e.g. "pnpm run schema:export"). When set, Delimit runs this command in a sandbox and diffs the regenerated output against the committed artifact to detect drift between source-of-truth and committed file. Pair with generator_artifact.' + required: false + default: '' + generator_artifact: + description: 'Path to the generated artifact that generator_command produces (e.g. "schemas/v1/agent.schema.json"). Required when generator_command is set.' + required: false + default: '' github_token: description: 'GitHub token for PR comments' required: false @@ -144,6 +156,9 @@ runs: from core.diff_engine_v2 import OpenAPIDiffEngine from core.policy_engine import evaluate_with_policy + from core.spec_detector import detect_spec_type + from core.json_schema_diff import JSONSchemaDiffEngine + from core.generator_drift import detect_drift, format_drift_report old_path = "${{ steps.resolve.outputs.resolved_old }}" new_path = "${{ steps.resolve.outputs.resolved_new }}" @@ -170,17 +185,63 @@ runs: old_spec = load(old_path) new_spec = load(new_path) + spec_type = detect_spec_type(new_spec) policy = "${{ inputs.policy_file }}" or None - api_name = old_spec.get("info", {}).get("title", "API") - current_version = old_spec.get("info", {}).get("version") - result = evaluate_with_policy( - old_spec, new_spec, - policy_file=policy, - include_semver=True, - current_version=current_version, - api_name=api_name, - ) + if spec_type == "json_schema": + # JSON Schema path (LED-713) — bypass OpenAPI policy engine, + # build a minimal report compatible with the comment step. + engine = JSONSchemaDiffEngine() + changes = engine.compare(old_spec, new_spec) + breaking_count = sum(1 for c in changes if c.is_breaking) + api_name = old_spec.get("title", "JSON Schema") + result = { + "summary": { + "total_changes": len(changes), + "breaking_changes": breaking_count, + "violations": 0, + }, + "changes": [ + {"type": c.type.value, "path": c.path, "message": c.message, + "is_breaking": c.is_breaking} + for c in changes + ], + "violations": [], + "semver": { + "bump": "major" if breaking_count else ("minor" if changes else "none"), + "next_version": "", + }, + "spec_type": "json_schema", + "api_name": api_name, + } + else: + api_name = old_spec.get("info", {}).get("title", "API") + current_version = old_spec.get("info", {}).get("version") + result = evaluate_with_policy( + old_spec, new_spec, + policy_file=policy, + include_semver=True, + current_version=current_version, + api_name=api_name, + ) + + # Generator drift check (LED-713) — opt-in via generator_command + generator_artifact + gen_cmd = "${{ inputs.generator_command }}" + gen_artifact = "${{ inputs.generator_artifact }}" + if gen_cmd and gen_artifact: + drift = detect_drift( + repo_root=os.environ.get("GITHUB_WORKSPACE", "."), + artifact_path=gen_artifact, + regen_command=gen_cmd, + timeout_seconds=120, + ) + result["drift"] = drift.to_dict() + # Drifted breaking changes count toward the breaking_changes flag + if drift.drifted: + drift_breaking = sum(1 for c in drift.changes if c.is_breaking) + result.setdefault("summary", {})["breaking_changes"] = ( + result.get("summary", {}).get("breaking_changes", 0) + drift_breaking + ) semver = result.get("semver", {}) breaking = result["summary"]["breaking_changes"] > 0 @@ -227,6 +288,8 @@ runs: const violations = report.violations || []; const allChanges = report.all_changes || []; const migration = report.migration || ''; + const drift = report.drift || null; + const specType = report.spec_type || 'openapi'; const bump = (semver.bump || 'none').toLowerCase(); const bc = summary.breaking_changes || 0; @@ -234,6 +297,91 @@ runs: const additive = total - bc; const mode = '${{ inputs.mode }}'; + // JSON Schema spec type — minimal render covering drift + classification. + // The OpenAPI renderer below is kept unchanged for back-compat. + if (specType === 'json_schema') { + let body = ''; + const schemaChanges = report.changes || []; + const breaking = schemaChanges.filter(c => c.is_breaking); + const nonBreaking = schemaChanges.filter(c => !c.is_breaking); + + if (breaking.length === 0 && nonBreaking.length === 0 && (!drift || !drift.drifted)) { + body += `### Delimit governance — JSON Schema\n\n`; + body += `No changes detected in \`${report.api_name || 'schema'}\`. Generator output matches committed artifact.\n\n`; + } else { + body += `### Delimit governance — JSON Schema\n\n`; + // Split the header so the committed-diff count and the drift + // count are visibly distinct signals, not joined into one + // contradictory line. Suppress the schema-classification line + // entirely when there are no committed changes. + const driftBreaking = (drift && drift.drifted) + ? drift.changes.filter(c => c.is_breaking).length + : 0; + if (schemaChanges.length > 0) { + body += `**${report.api_name || 'Schema'}** — committed schema diff: ${schemaChanges.length} change(s), ${breaking.length} breaking \n`; + body += `Recommended semver bump: **${bump}**\n\n`; + } + if (drift && drift.drifted) { + body += `**Generator drift:** ${drift.change_count} change(s), ${driftBreaking} breaking\n\n`; + } + + if (schemaChanges.length > 0) { + body += `#### Schema classification\n\n`; + for (const c of schemaChanges) { + const icon = c.is_breaking ? '\u{1F534}' : '\u{1F7E2}'; + const tag = c.is_breaking ? 'breaking' : 'ok'; + body += `- ${icon} \`${c.type}\` [${tag}] at \`${c.path}\` — ${c.message}\n`; + } + body += `\n`; + } + + if (drift && drift.drifted) { + body += `#### Generator drift\n\n`; + body += `Artifact \`${drift.artifact_path}\` is **stale** vs \`${drift.regen_command}\` output (${drift.change_count} drift change(s), ${drift.runtime_seconds}s to regen).\n\n`; + body += `Re-run the generator and commit the result, or revert the source change.\n\n`; + for (const c of drift.changes) { + const icon = c.is_breaking ? '\u{1F534}' : '\u{1F7E2}'; + body += `- ${icon} \`${c.type}\` at \`${c.path}\` — ${c.message}\n`; + } + body += `\n`; + } else if (drift && !drift.drifted && !drift.error) { + body += `#### Generator drift\n\nClean — committed \`${drift.artifact_path}\` matches generator output (${drift.runtime_seconds}s to regen).\n\n`; + } else if (drift && drift.error) { + body += `#### Generator drift\n\nSkipped — ${drift.error}\n\n`; + } + + if (mode === 'advisory') { + body += `> Advisory mode — CI will not fail. Set \`fail_on_breaking: true\` or \`mode: enforce\` to block on breaking changes.\n\n`; + } + } + + body += `---\n`; + body += `Powered by [Delimit](https://github.com/delimit-ai/delimit-action) — API governance for every PR`; + + const { data: comments } = await github.rest.issues.listComments({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + }); + const existing = comments.find(c => c.user.type === 'Bot' && c.body.includes('Delimit')); + if (existing) { + await github.rest.issues.updateComment({ + owner: context.repo.owner, + repo: context.repo.repo, + comment_id: existing.id, + body, + }); + } else { + await github.rest.issues.createComment({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: context.issue.number, + body, + }); + } + return; + } + // Severity mapping const severityOf = (rule) => { const critical = ['no_endpoint_removal', 'no_method_removal', 'no_field_removal', 'no_response_field_removal']; @@ -455,8 +603,8 @@ runs: shell: bash - name: Enforce breaking change policy - if: inputs.mode == 'enforce' && steps.validate.outputs.breaking_changes == 'true' + if: (inputs.mode == 'enforce' || inputs.fail_on_breaking == 'true') && steps.validate.outputs.breaking_changes == 'true' run: | - echo "::error::Breaking API changes detected. CI blocked by Delimit enforce mode." + echo "::error::Breaking API changes detected. CI blocked by Delimit (mode=${{ inputs.mode }}, fail_on_breaking=${{ inputs.fail_on_breaking }})." exit 1 shell: bash diff --git a/core/diff_engine_v2.py b/core/diff_engine_v2.py index 89c64f7..0226b30 100644 --- a/core/diff_engine_v2.py +++ b/core/diff_engine_v2.py @@ -78,7 +78,9 @@ def __init__(self): def compare(self, old_spec: Dict, new_spec: Dict) -> List[Change]: """Compare two OpenAPI specifications and return all changes.""" self.changes = [] - + old_spec = old_spec or {} + new_spec = new_spec or {} + # Compare paths self._compare_paths(old_spec.get("paths", {}), new_spec.get("paths", {})) @@ -365,6 +367,11 @@ def _compare_responses(self, operation_id: str, old_responses: Dict, new_respons def _compare_schema_deep(self, path: str, old_schema: Dict, new_schema: Dict, required_fields: Optional[Set[str]] = None): """Deep comparison of schemas including nested objects.""" + # Guard against None schemas + if old_schema is None: + old_schema = {} + if new_schema is None: + new_schema = {} # Handle references if "$ref" in old_schema or "$ref" in new_schema: diff --git a/core/generator_drift.py b/core/generator_drift.py new file mode 100644 index 0000000..dfa760f --- /dev/null +++ b/core/generator_drift.py @@ -0,0 +1,204 @@ +"""Generator drift detection (LED-713). + +Detects when a committed generated artifact (e.g. agentspec's +schemas/v1/agent.schema.json regenerated from a Zod source) has drifted +from what its generator script would produce today. + +Use case: a maintainer changes the source of truth (Zod schema, OpenAPI +generator, protobuf, etc.) but forgets to regenerate and commit the +artifact. CI catches the drift before the stale generated file ships. + +Generic over generators — caller supplies the regen command and the +artifact path. Returns a structured drift report that can be merged into +the standard delimit-action PR comment. +""" + +from __future__ import annotations + +import json +import os +import shlex +import shutil +import subprocess +import tempfile +from dataclasses import dataclass, field +from pathlib import Path +from typing import Any, Dict, List, Optional + + +@dataclass +class DriftResult: + drifted: bool + artifact_path: str + regen_command: str + changes: List[Any] = field(default_factory=list) # JSONSchemaChange list when drift detected + error: Optional[str] = None + runtime_seconds: float = 0.0 + + def to_dict(self) -> Dict[str, Any]: + return { + "drifted": self.drifted, + "artifact_path": self.artifact_path, + "regen_command": self.regen_command, + "change_count": len(self.changes), + "changes": [ + { + "type": c.type.value, + "path": c.path, + "message": c.message, + "is_breaking": c.is_breaking, + } + for c in self.changes + ], + "error": self.error, + "runtime_seconds": round(self.runtime_seconds, 3), + } + + +def detect_drift( + repo_root: str, + artifact_path: str, + regen_command: str, + timeout_seconds: int = 60, +) -> DriftResult: + """Check whether the committed artifact matches its generator output. + + Args: + repo_root: Absolute path to the repo checkout. + artifact_path: Path to the generated artifact, relative to repo_root. + regen_command: Shell command that regenerates the artifact in place. + Example: "pnpm -r run build" or "node packages/sdk/dist/scripts/export-schema.js" + timeout_seconds: Hard timeout for the generator (default 60). + + Returns: + DriftResult with drift status, classified changes, and runtime. + """ + import time + + repo_root_p = Path(repo_root).resolve() + artifact_p = (repo_root_p / artifact_path).resolve() + + if not artifact_p.exists(): + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + error=f"Artifact not found: {artifact_path}", + ) + + # Snapshot the committed artifact before regen + try: + committed_text = artifact_p.read_text() + committed_doc = json.loads(committed_text) + except (OSError, json.JSONDecodeError) as e: + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + error=f"Failed to read committed artifact: {e}", + ) + + # Run the regenerator + start = time.time() + try: + result = subprocess.run( + regen_command, + shell=True, + cwd=str(repo_root_p), + capture_output=True, + text=True, + timeout=timeout_seconds, + ) + except subprocess.TimeoutExpired: + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + error=f"Generator timed out after {timeout_seconds}s", + runtime_seconds=time.time() - start, + ) + + runtime = time.time() - start + + if result.returncode != 0: + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + error=f"Generator exited {result.returncode}: {result.stderr.strip()[:500]}", + runtime_seconds=runtime, + ) + + # Read the regenerated artifact + try: + regen_text = artifact_p.read_text() + regen_doc = json.loads(regen_text) + except (OSError, json.JSONDecodeError) as e: + # Restore committed version so we don't leave the workspace dirty + artifact_p.write_text(committed_text) + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + error=f"Failed to read regenerated artifact: {e}", + runtime_seconds=runtime, + ) + + # Restore the committed file before diffing — leave the workspace clean + artifact_p.write_text(committed_text) + + # Quick equality check first + if committed_doc == regen_doc: + return DriftResult( + drifted=False, + artifact_path=artifact_path, + regen_command=regen_command, + runtime_seconds=runtime, + ) + + # Drift detected — classify the changes via the JSON Schema diff engine + from .json_schema_diff import JSONSchemaDiffEngine + + engine = JSONSchemaDiffEngine() + changes = engine.compare(committed_doc, regen_doc) + return DriftResult( + drifted=True, + artifact_path=artifact_path, + regen_command=regen_command, + changes=changes, + runtime_seconds=runtime, + ) + + +def format_drift_report(result: DriftResult) -> str: + """Render a drift report as a markdown block for PR comments.""" + if result.error: + return ( + f"### Generator drift check\n\n" + f"Artifact: `{result.artifact_path}` \n" + f"Status: error \n" + f"Detail: {result.error}\n" + ) + if not result.drifted: + return ( + f"### Generator drift check\n\n" + f"Artifact: `{result.artifact_path}` \n" + f"Status: clean (committed artifact matches generator output) \n" + f"Generator runtime: {result.runtime_seconds:.2f}s\n" + ) + breaking = sum(1 for c in result.changes if c.is_breaking) + non_breaking = len(result.changes) - breaking + lines = [ + "### Generator drift check", + "", + f"Artifact: `{result.artifact_path}` ", + f"Status: drifted ({len(result.changes)} change(s) — {breaking} breaking, {non_breaking} non-breaking) ", + f"Generator runtime: {result.runtime_seconds:.2f}s ", + "", + "The committed artifact does not match what the generator produces today. Re-run the generator and commit the result, or revert the source change.", + "", + ] + for c in result.changes: + marker = "breaking" if c.is_breaking else "ok" + lines.append(f"- [{marker}] {c.type.value} at `{c.path}` — {c.message}") + return "\n".join(lines) + "\n" diff --git a/core/json_schema_diff.py b/core/json_schema_diff.py new file mode 100644 index 0000000..bc67fdf --- /dev/null +++ b/core/json_schema_diff.py @@ -0,0 +1,375 @@ +""" +JSON Schema diff engine (LED-713). + +Sibling to core/diff_engine_v2.py. Handles bare JSON Schema files +(Draft 4+), resolving internal $ref to #/definitions. Deliberately +excludes anyOf/oneOf/allOf composition, external refs, discriminators, +and if/then/else — those are deferred past v1. + +Dispatched from spec_detector when a file contains a top-level +"$schema" key or a top-level "definitions" key without OpenAPI markers. + +Designed for the agents-oss/agentspec integration (issue #21) but +general across any single-file JSON Schema. +""" + +from dataclasses import dataclass, field +from enum import Enum +from typing import Any, Dict, List, Optional + + +class JSONSchemaChangeType(Enum): + # Breaking + PROPERTY_REMOVED = "property_removed" + REQUIRED_ADDED = "required_added" + TYPE_NARROWED = "type_narrowed" + ENUM_VALUE_REMOVED = "enum_value_removed" + CONST_CHANGED = "const_changed" + ADDITIONAL_PROPERTIES_TIGHTENED = "additional_properties_tightened" + PATTERN_TIGHTENED = "pattern_tightened" + MIN_LENGTH_INCREASED = "min_length_increased" + MAX_LENGTH_DECREASED = "max_length_decreased" + MINIMUM_INCREASED = "minimum_increased" + MAXIMUM_DECREASED = "maximum_decreased" + ITEMS_TYPE_NARROWED = "items_type_narrowed" + + # Non-breaking + PROPERTY_ADDED = "property_added" + REQUIRED_REMOVED = "required_removed" + TYPE_WIDENED = "type_widened" + ENUM_VALUE_ADDED = "enum_value_added" + ADDITIONAL_PROPERTIES_LOOSENED = "additional_properties_loosened" + PATTERN_LOOSENED = "pattern_loosened" + MIN_LENGTH_DECREASED = "min_length_decreased" + MAX_LENGTH_INCREASED = "max_length_increased" + MINIMUM_DECREASED = "minimum_decreased" + MAXIMUM_INCREASED = "maximum_increased" + ITEMS_TYPE_WIDENED = "items_type_widened" + DESCRIPTION_CHANGED = "description_changed" + + +_BREAKING_TYPES = { + JSONSchemaChangeType.PROPERTY_REMOVED, + JSONSchemaChangeType.REQUIRED_ADDED, + JSONSchemaChangeType.TYPE_NARROWED, + JSONSchemaChangeType.ENUM_VALUE_REMOVED, + JSONSchemaChangeType.CONST_CHANGED, + JSONSchemaChangeType.ADDITIONAL_PROPERTIES_TIGHTENED, + JSONSchemaChangeType.PATTERN_TIGHTENED, + JSONSchemaChangeType.MIN_LENGTH_INCREASED, + JSONSchemaChangeType.MAX_LENGTH_DECREASED, + JSONSchemaChangeType.MINIMUM_INCREASED, + JSONSchemaChangeType.MAXIMUM_DECREASED, + JSONSchemaChangeType.ITEMS_TYPE_NARROWED, +} + + +@dataclass +class JSONSchemaChange: + type: JSONSchemaChangeType + path: str + details: Dict[str, Any] = field(default_factory=dict) + message: str = "" + + @property + def is_breaking(self) -> bool: + return self.type in _BREAKING_TYPES + + @property + def severity(self) -> str: + return "high" if self.is_breaking else "low" + + +# Type widening hierarchy: a change from "integer" to "number" is widening +# (non-breaking for consumers). The reverse narrows and is breaking. +_TYPE_SUPERSETS = { + "number": {"integer"}, +} + + +def _is_type_widening(old: str, new: str) -> bool: + return old in _TYPE_SUPERSETS.get(new, set()) + + +def _is_type_narrowing(old: str, new: str) -> bool: + return new in _TYPE_SUPERSETS.get(old, set()) + + +class JSONSchemaDiffEngine: + """Compare two JSON Schema documents. + + Handles internal $ref to #/definitions by resolving refs against the + document's own definitions block during traversal. External refs + (http://, file://) are out of scope for v1. + """ + + def __init__(self) -> None: + self.changes: List[JSONSchemaChange] = [] + self._old_defs: Dict[str, Any] = {} + self._new_defs: Dict[str, Any] = {} + + # ------------------------------------------------------------------ + # public API + # ------------------------------------------------------------------ + + def compare(self, old_schema: Dict[str, Any], new_schema: Dict[str, Any]) -> List[JSONSchemaChange]: + self.changes = [] + old_schema = old_schema or {} + new_schema = new_schema or {} + self._old_defs = old_schema.get("definitions", {}) or {} + self._new_defs = new_schema.get("definitions", {}) or {} + + # If the root is a $ref shim (common pattern: {"$ref": "#/definitions/Foo", "definitions": {...}}) + # unwrap both sides so we diff the actual shape. + old_root = self._resolve(old_schema, self._old_defs) + new_root = self._resolve(new_schema, self._new_defs) + + self._compare_schema(old_root, new_root, path="") + return self.changes + + # ------------------------------------------------------------------ + # $ref resolution + # ------------------------------------------------------------------ + + def _resolve(self, node: Any, defs: Dict[str, Any]) -> Any: + """Resolve internal $ref to #/definitions. Returns node unchanged otherwise.""" + if not isinstance(node, dict): + return node + ref = node.get("$ref") + if not ref or not isinstance(ref, str) or not ref.startswith("#/definitions/"): + return node + key = ref[len("#/definitions/"):] + resolved = defs.get(key) + if resolved is None: + return node + # Merge sibling keys from the ref node (e.g. description) onto the resolved. + merged = dict(resolved) + for k, v in node.items(): + if k != "$ref": + merged.setdefault(k, v) + return merged + + # ------------------------------------------------------------------ + # recursive traversal + # ------------------------------------------------------------------ + + def _compare_schema(self, old: Any, new: Any, path: str) -> None: + if not isinstance(old, dict) or not isinstance(new, dict): + return + old = self._resolve(old, self._old_defs) + new = self._resolve(new, self._new_defs) + + self._compare_type(old, new, path) + self._compare_const(old, new, path) + self._compare_enum(old, new, path) + self._compare_pattern(old, new, path) + self._compare_numeric_bounds(old, new, path) + self._compare_string_length(old, new, path) + self._compare_additional_properties(old, new, path) + self._compare_required(old, new, path) + self._compare_properties(old, new, path) + self._compare_items(old, new, path) + + # ------------------------------------------------------------------ + # individual comparisons + # ------------------------------------------------------------------ + + def _compare_type(self, old: Dict, new: Dict, path: str) -> None: + old_t = old.get("type") + new_t = new.get("type") + if old_t == new_t or old_t is None or new_t is None: + return + if isinstance(old_t, str) and isinstance(new_t, str): + if _is_type_widening(old_t, new_t): + self._add(JSONSchemaChangeType.TYPE_WIDENED, path, + {"old": old_t, "new": new_t}, + f"Type widened at {path or '/'}: {old_t} → {new_t}") + return + if _is_type_narrowing(old_t, new_t): + self._add(JSONSchemaChangeType.TYPE_NARROWED, path, + {"old": old_t, "new": new_t}, + f"Type narrowed at {path or '/'}: {old_t} → {new_t}") + return + # Unrelated type change — treat as narrowing (breaking) + self._add(JSONSchemaChangeType.TYPE_NARROWED, path, + {"old": old_t, "new": new_t}, + f"Type changed at {path or '/'}: {old_t} → {new_t}") + + def _compare_const(self, old: Dict, new: Dict, path: str) -> None: + if "const" in old and "const" in new and old["const"] != new["const"]: + self._add(JSONSchemaChangeType.CONST_CHANGED, path, + {"old": old["const"], "new": new["const"]}, + f"const value changed at {path or '/'}: {old['const']!r} → {new['const']!r}") + + def _compare_enum(self, old: Dict, new: Dict, path: str) -> None: + old_enum = old.get("enum") + new_enum = new.get("enum") + if not isinstance(old_enum, list) or not isinstance(new_enum, list): + return + old_set = {repr(v) for v in old_enum} + new_set = {repr(v) for v in new_enum} + for removed in old_set - new_set: + self._add(JSONSchemaChangeType.ENUM_VALUE_REMOVED, path, + {"value": removed}, + f"enum value removed at {path or '/'}: {removed}") + for added in new_set - old_set: + self._add(JSONSchemaChangeType.ENUM_VALUE_ADDED, path, + {"value": added}, + f"enum value added at {path or '/'}: {added}") + + def _compare_pattern(self, old: Dict, new: Dict, path: str) -> None: + old_p = old.get("pattern") + new_p = new.get("pattern") + if old_p == new_p or (old_p is None and new_p is None): + return + # We can't prove regex subset relationships, so any pattern change + # on an existing constraint is conservatively breaking; adding a + # brand-new pattern is breaking; removing a pattern is non-breaking. + if old_p and not new_p: + self._add(JSONSchemaChangeType.PATTERN_LOOSENED, path, + {"old": old_p}, + f"pattern removed at {path or '/'}: {old_p}") + elif not old_p and new_p: + self._add(JSONSchemaChangeType.PATTERN_TIGHTENED, path, + {"new": new_p}, + f"pattern added at {path or '/'}: {new_p}") + else: + self._add(JSONSchemaChangeType.PATTERN_TIGHTENED, path, + {"old": old_p, "new": new_p}, + f"pattern changed at {path or '/'}: {old_p} → {new_p}") + + def _compare_numeric_bounds(self, old: Dict, new: Dict, path: str) -> None: + for key, tight_type, loose_type in ( + ("minimum", JSONSchemaChangeType.MINIMUM_INCREASED, JSONSchemaChangeType.MINIMUM_DECREASED), + ("maximum", JSONSchemaChangeType.MAXIMUM_DECREASED, JSONSchemaChangeType.MAXIMUM_INCREASED), + ): + old_v = old.get(key) + new_v = new.get(key) + if old_v is None or new_v is None or old_v == new_v: + continue + try: + delta = float(new_v) - float(old_v) + except (TypeError, ValueError): + continue + if key == "minimum": + if delta > 0: + self._add(tight_type, path, {"old": old_v, "new": new_v}, + f"minimum increased at {path or '/'}: {old_v} → {new_v}") + else: + self._add(loose_type, path, {"old": old_v, "new": new_v}, + f"minimum decreased at {path or '/'}: {old_v} → {new_v}") + else: # maximum + if delta < 0: + self._add(tight_type, path, {"old": old_v, "new": new_v}, + f"maximum decreased at {path or '/'}: {old_v} → {new_v}") + else: + self._add(loose_type, path, {"old": old_v, "new": new_v}, + f"maximum increased at {path or '/'}: {old_v} → {new_v}") + + def _compare_string_length(self, old: Dict, new: Dict, path: str) -> None: + for key, tight_type, loose_type in ( + ("minLength", JSONSchemaChangeType.MIN_LENGTH_INCREASED, JSONSchemaChangeType.MIN_LENGTH_DECREASED), + ("maxLength", JSONSchemaChangeType.MAX_LENGTH_DECREASED, JSONSchemaChangeType.MAX_LENGTH_INCREASED), + ): + old_v = old.get(key) + new_v = new.get(key) + if old_v is None or new_v is None or old_v == new_v: + continue + if key == "minLength": + if new_v > old_v: + self._add(tight_type, path, {"old": old_v, "new": new_v}, + f"minLength increased at {path or '/'}: {old_v} → {new_v}") + else: + self._add(loose_type, path, {"old": old_v, "new": new_v}, + f"minLength decreased at {path or '/'}: {old_v} → {new_v}") + else: # maxLength + if new_v < old_v: + self._add(tight_type, path, {"old": old_v, "new": new_v}, + f"maxLength decreased at {path or '/'}: {old_v} → {new_v}") + else: + self._add(loose_type, path, {"old": old_v, "new": new_v}, + f"maxLength increased at {path or '/'}: {old_v} → {new_v}") + + def _compare_additional_properties(self, old: Dict, new: Dict, path: str) -> None: + old_ap = old.get("additionalProperties") + new_ap = new.get("additionalProperties") + # Default in JSON Schema is True (additional allowed). Only flag + # explicit transitions that change the answer. + if old_ap is None and new_ap is None: + return + old_allows = True if old_ap is None else bool(old_ap) + new_allows = True if new_ap is None else bool(new_ap) + if old_allows and not new_allows: + self._add(JSONSchemaChangeType.ADDITIONAL_PROPERTIES_TIGHTENED, path, + {"old": old_ap, "new": new_ap}, + f"additionalProperties tightened at {path or '/'}: {old_ap} → {new_ap}") + elif not old_allows and new_allows: + self._add(JSONSchemaChangeType.ADDITIONAL_PROPERTIES_LOOSENED, path, + {"old": old_ap, "new": new_ap}, + f"additionalProperties loosened at {path or '/'}: {old_ap} → {new_ap}") + + def _compare_required(self, old: Dict, new: Dict, path: str) -> None: + old_req = set(old.get("required", []) or []) + new_req = set(new.get("required", []) or []) + for added in new_req - old_req: + self._add(JSONSchemaChangeType.REQUIRED_ADDED, f"{path}/required/{added}", + {"field": added}, + f"required field added at {path or '/'}: {added}") + for removed in old_req - new_req: + self._add(JSONSchemaChangeType.REQUIRED_REMOVED, f"{path}/required/{removed}", + {"field": removed}, + f"required field removed at {path or '/'}: {removed}") + + def _compare_properties(self, old: Dict, new: Dict, path: str) -> None: + old_props = old.get("properties", {}) or {} + new_props = new.get("properties", {}) or {} + if not isinstance(old_props, dict) or not isinstance(new_props, dict): + return + for removed in set(old_props) - set(new_props): + self._add(JSONSchemaChangeType.PROPERTY_REMOVED, f"{path}/properties/{removed}", + {"field": removed}, + f"property removed: {path or '/'}.{removed}") + for added in set(new_props) - set(old_props): + self._add(JSONSchemaChangeType.PROPERTY_ADDED, f"{path}/properties/{added}", + {"field": added}, + f"property added: {path or '/'}.{added}") + for name in set(old_props) & set(new_props): + self._compare_schema(old_props[name], new_props[name], f"{path}/properties/{name}") + + def _compare_items(self, old: Dict, new: Dict, path: str) -> None: + old_items = old.get("items") + new_items = new.get("items") + if not isinstance(old_items, dict) or not isinstance(new_items, dict): + return + self._compare_schema(old_items, new_items, f"{path}/items") + + # ------------------------------------------------------------------ + # helpers + # ------------------------------------------------------------------ + + def _add(self, change_type: JSONSchemaChangeType, path: str, + details: Dict[str, Any], message: str) -> None: + self.changes.append(JSONSchemaChange( + type=change_type, path=path or "/", details=details, message=message)) + + +def is_json_schema(doc: Dict[str, Any]) -> bool: + """Detect whether a parsed document should be routed to this engine. + + Heuristic: top-level "$schema" key referencing json-schema.org, OR a + top-level "definitions" block without OpenAPI markers (paths, components, + openapi, swagger). + """ + if not isinstance(doc, dict): + return False + if any(marker in doc for marker in ("openapi", "swagger", "paths")): + return False + schema_url = doc.get("$schema") + if isinstance(schema_url, str) and "json-schema.org" in schema_url: + return True + if "definitions" in doc and isinstance(doc["definitions"], dict): + return True + # Agentspec pattern: {"$ref": "#/definitions/...", "definitions": {...}} + if doc.get("$ref", "").startswith("#/definitions/"): + return True + return False diff --git a/core/spec_detector.py b/core/spec_detector.py index a33a440..d442647 100644 --- a/core/spec_detector.py +++ b/core/spec_detector.py @@ -3,7 +3,7 @@ """ import os -from typing import List, Optional, Tuple +from typing import Any, List, Optional, Tuple from pathlib import Path import yaml @@ -77,7 +77,7 @@ def _is_valid_openapi(self, file_path: Path) -> bool: """Check if file is a valid OpenAPI specification.""" if not file_path.is_file(): return False - + try: with open(file_path, 'r') as f: data = yaml.safe_load(f) @@ -86,8 +86,48 @@ def _is_valid_openapi(self, file_path: Path) -> bool: return 'openapi' in data or 'swagger' in data except: return False - + return False + + +def detect_spec_type(doc: Any) -> str: + """Classify a parsed spec document for engine dispatch (LED-713). + + Returns: + "openapi" — OpenAPI 3.x / Swagger 2.x (route to OpenAPIDiffEngine) + "json_schema" — bare JSON Schema Draft 4+ (route to JSONSchemaDiffEngine) + "unknown" — no recognized markers + """ + if not isinstance(doc, dict): + return "unknown" + if "openapi" in doc or "swagger" in doc or "paths" in doc: + return "openapi" + # JSON Schema markers: $schema URL, top-level definitions, or ref-shim root + schema_url = doc.get("$schema") + if isinstance(schema_url, str) and "json-schema.org" in schema_url: + return "json_schema" + if isinstance(doc.get("definitions"), dict): + return "json_schema" + ref = doc.get("$ref") + if isinstance(ref, str) and ref.startswith("#/definitions/"): + return "json_schema" + return "unknown" + + +def get_diff_engine(doc: Any): + """Factory: return the right diff engine instance for a parsed doc. + + Callers: action.yml inline Python, policy_engine, npm-delimit api-engine. + The returned engine exposes .compare(old, new) -> List[Change]. + """ + spec_type = detect_spec_type(doc) + if spec_type == "json_schema": + from .json_schema_diff import JSONSchemaDiffEngine + return JSONSchemaDiffEngine() + # Default to OpenAPI for "openapi" and "unknown" (back-compat: existing + # specs without explicit markers still hit the OpenAPI engine) + from .diff_engine_v2 import OpenAPIDiffEngine + return OpenAPIDiffEngine() def get_default_specs(self) -> Tuple[Optional[str], Optional[str]]: """ diff --git a/tests/test_json_schema_diff.py b/tests/test_json_schema_diff.py new file mode 100644 index 0000000..e096752 --- /dev/null +++ b/tests/test_json_schema_diff.py @@ -0,0 +1,345 @@ +"""Tests for core/json_schema_diff.py (LED-713). + +Covers every v1 change type, $ref resolution, dispatcher routing, +and the agentspec rename as a real-world fixture. +""" + +import json +import os +import sys +from pathlib import Path + +import pytest + +sys.path.insert(0, str(Path(__file__).parent.parent)) + +from core.json_schema_diff import ( + JSONSchemaChangeType, + JSONSchemaDiffEngine, + is_json_schema, +) +from core.spec_detector import detect_spec_type, get_diff_engine + + +# ---------------------------------------------------------------------- +# helpers +# ---------------------------------------------------------------------- + +def _diff(old, new): + return JSONSchemaDiffEngine().compare(old, new) + + +def _types(changes): + return [c.type for c in changes] + + +# ---------------------------------------------------------------------- +# detection +# ---------------------------------------------------------------------- + +class TestDetection: + def test_is_json_schema_with_definitions(self): + assert is_json_schema({"definitions": {"Foo": {}}}) + + def test_is_json_schema_with_schema_url(self): + assert is_json_schema({"$schema": "https://json-schema.org/draft/2020-12/schema"}) + + def test_is_json_schema_with_ref_shim(self): + assert is_json_schema({"$ref": "#/definitions/Foo", "definitions": {"Foo": {}}}) + + def test_not_json_schema_openapi(self): + assert not is_json_schema({"openapi": "3.0.0", "paths": {}}) + + def test_not_json_schema_swagger(self): + assert not is_json_schema({"swagger": "2.0", "paths": {}}) + + def test_not_json_schema_non_dict(self): + assert not is_json_schema("string") + assert not is_json_schema([]) + assert not is_json_schema(None) + + def test_dispatcher_openapi(self): + assert detect_spec_type({"openapi": "3.0.0"}) == "openapi" + assert type(get_diff_engine({"openapi": "3.0.0"})).__name__ == "OpenAPIDiffEngine" + + def test_dispatcher_json_schema(self): + assert detect_spec_type({"definitions": {"Foo": {}}}) == "json_schema" + assert type(get_diff_engine({"definitions": {"Foo": {}}})).__name__ == "JSONSchemaDiffEngine" + + def test_dispatcher_ambiguous_defaults_to_openapi(self): + assert detect_spec_type({}) == "unknown" + # get_diff_engine falls back to OpenAPI for back-compat + assert type(get_diff_engine({})).__name__ == "OpenAPIDiffEngine" + + +# ---------------------------------------------------------------------- +# property add / remove +# ---------------------------------------------------------------------- + +class TestProperties: + def test_property_added_is_non_breaking(self): + old = {"properties": {"a": {"type": "string"}}} + new = {"properties": {"a": {"type": "string"}, "b": {"type": "integer"}}} + changes = _diff(old, new) + assert len(changes) == 1 + assert changes[0].type == JSONSchemaChangeType.PROPERTY_ADDED + assert not changes[0].is_breaking + + def test_property_removed_is_breaking(self): + old = {"properties": {"a": {"type": "string"}, "b": {"type": "integer"}}} + new = {"properties": {"a": {"type": "string"}}} + changes = _diff(old, new) + assert len(changes) == 1 + assert changes[0].type == JSONSchemaChangeType.PROPERTY_REMOVED + assert changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# required add / remove +# ---------------------------------------------------------------------- + +class TestRequired: + def test_required_added_is_breaking(self): + old = {"properties": {"a": {}}} + new = {"properties": {"a": {}}, "required": ["a"]} + changes = _diff(old, new) + assert JSONSchemaChangeType.REQUIRED_ADDED in _types(changes) + assert any(c.is_breaking for c in changes) + + def test_required_removed_is_non_breaking(self): + old = {"required": ["a"]} + new = {} + changes = _diff(old, new) + assert changes[0].type == JSONSchemaChangeType.REQUIRED_REMOVED + assert not changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# type widen / narrow +# ---------------------------------------------------------------------- + +class TestType: + def test_integer_to_number_is_widening(self): + changes = _diff({"type": "integer"}, {"type": "number"}) + assert changes[0].type == JSONSchemaChangeType.TYPE_WIDENED + assert not changes[0].is_breaking + + def test_number_to_integer_is_narrowing(self): + changes = _diff({"type": "number"}, {"type": "integer"}) + assert changes[0].type == JSONSchemaChangeType.TYPE_NARROWED + assert changes[0].is_breaking + + def test_unrelated_type_change_is_breaking(self): + changes = _diff({"type": "string"}, {"type": "integer"}) + assert changes[0].type == JSONSchemaChangeType.TYPE_NARROWED + assert changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# enum add / remove +# ---------------------------------------------------------------------- + +class TestEnum: + def test_enum_value_removed_is_breaking(self): + changes = _diff({"enum": ["a", "b", "c"]}, {"enum": ["a", "b"]}) + assert JSONSchemaChangeType.ENUM_VALUE_REMOVED in _types(changes) + assert any(c.is_breaking for c in changes) + + def test_enum_value_added_is_non_breaking(self): + changes = _diff({"enum": ["a"]}, {"enum": ["a", "b"]}) + assert changes[0].type == JSONSchemaChangeType.ENUM_VALUE_ADDED + assert not changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# const +# ---------------------------------------------------------------------- + +class TestConst: + def test_const_changed_is_breaking(self): + changes = _diff({"const": "v1"}, {"const": "v1alpha1"}) + assert changes[0].type == JSONSchemaChangeType.CONST_CHANGED + assert changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# additionalProperties +# ---------------------------------------------------------------------- + +class TestAdditionalProperties: + def test_true_to_false_is_breaking(self): + changes = _diff({"additionalProperties": True}, {"additionalProperties": False}) + assert changes[0].type == JSONSchemaChangeType.ADDITIONAL_PROPERTIES_TIGHTENED + assert changes[0].is_breaking + + def test_false_to_true_is_non_breaking(self): + changes = _diff({"additionalProperties": False}, {"additionalProperties": True}) + assert changes[0].type == JSONSchemaChangeType.ADDITIONAL_PROPERTIES_LOOSENED + assert not changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# pattern +# ---------------------------------------------------------------------- + +class TestPattern: + def test_pattern_added_is_breaking(self): + changes = _diff({}, {"pattern": "^[a-z]+$"}) + assert changes[0].type == JSONSchemaChangeType.PATTERN_TIGHTENED + assert changes[0].is_breaking + + def test_pattern_removed_is_non_breaking(self): + changes = _diff({"pattern": "^[a-z]+$"}, {}) + assert changes[0].type == JSONSchemaChangeType.PATTERN_LOOSENED + assert not changes[0].is_breaking + + def test_pattern_changed_is_breaking(self): + changes = _diff({"pattern": "^[a-z]+$"}, {"pattern": "^[A-Z]+$"}) + assert changes[0].type == JSONSchemaChangeType.PATTERN_TIGHTENED + assert changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# string length bounds +# ---------------------------------------------------------------------- + +class TestStringLength: + def test_min_length_increased_is_breaking(self): + changes = _diff({"minLength": 1}, {"minLength": 5}) + assert changes[0].type == JSONSchemaChangeType.MIN_LENGTH_INCREASED + assert changes[0].is_breaking + + def test_min_length_decreased_is_non_breaking(self): + changes = _diff({"minLength": 5}, {"minLength": 1}) + assert changes[0].type == JSONSchemaChangeType.MIN_LENGTH_DECREASED + assert not changes[0].is_breaking + + def test_max_length_decreased_is_breaking(self): + changes = _diff({"maxLength": 100}, {"maxLength": 10}) + assert changes[0].type == JSONSchemaChangeType.MAX_LENGTH_DECREASED + assert changes[0].is_breaking + + def test_max_length_increased_is_non_breaking(self): + changes = _diff({"maxLength": 10}, {"maxLength": 100}) + assert changes[0].type == JSONSchemaChangeType.MAX_LENGTH_INCREASED + assert not changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# numeric bounds +# ---------------------------------------------------------------------- + +class TestNumericBounds: + def test_minimum_increased_is_breaking(self): + changes = _diff({"minimum": 0}, {"minimum": 10}) + assert changes[0].type == JSONSchemaChangeType.MINIMUM_INCREASED + assert changes[0].is_breaking + + def test_maximum_decreased_is_breaking(self): + changes = _diff({"maximum": 100}, {"maximum": 50}) + assert changes[0].type == JSONSchemaChangeType.MAXIMUM_DECREASED + assert changes[0].is_breaking + + def test_minimum_decreased_is_non_breaking(self): + changes = _diff({"minimum": 10}, {"minimum": 0}) + assert changes[0].type == JSONSchemaChangeType.MINIMUM_DECREASED + assert not changes[0].is_breaking + + +# ---------------------------------------------------------------------- +# array items +# ---------------------------------------------------------------------- + +class TestItems: + def test_items_type_narrowed_is_breaking(self): + old = {"type": "array", "items": {"type": "number"}} + new = {"type": "array", "items": {"type": "integer"}} + changes = _diff(old, new) + assert any(c.type == JSONSchemaChangeType.TYPE_NARROWED for c in changes) + assert any(c.is_breaking for c in changes) + + def test_items_type_widened_is_non_breaking(self): + old = {"type": "array", "items": {"type": "integer"}} + new = {"type": "array", "items": {"type": "number"}} + changes = _diff(old, new) + assert any(c.type == JSONSchemaChangeType.TYPE_WIDENED for c in changes) + assert not any(c.is_breaking for c in changes) + + +# ---------------------------------------------------------------------- +# $ref resolution +# ---------------------------------------------------------------------- + +class TestRefResolution: + def test_ref_shim_at_root(self): + """Agentspec pattern: root is {"$ref": "#/definitions/X", "definitions": {...}}""" + old = { + "$ref": "#/definitions/Thing", + "definitions": {"Thing": {"type": "object", "properties": {"a": {"type": "string"}}}}, + } + new = { + "$ref": "#/definitions/Thing", + "definitions": {"Thing": {"type": "object", "properties": {"a": {"type": "string"}, "b": {"type": "integer"}}}}, + } + changes = _diff(old, new) + assert len(changes) == 1 + assert changes[0].type == JSONSchemaChangeType.PROPERTY_ADDED + + def test_ref_in_nested_property(self): + old = { + "properties": {"child": {"$ref": "#/definitions/Child"}}, + "definitions": {"Child": {"type": "object", "required": ["a"]}}, + } + new = { + "properties": {"child": {"$ref": "#/definitions/Child"}}, + "definitions": {"Child": {"type": "object", "required": ["a", "b"]}}, + } + changes = _diff(old, new) + assert JSONSchemaChangeType.REQUIRED_ADDED in _types(changes) + + +# ---------------------------------------------------------------------- +# agentspec fixture — real-world +# ---------------------------------------------------------------------- + +AGENTSPEC_FIXTURE = Path("/tmp/agentspec-v1.json") +AGENTSPEC_RENAMED = Path("/tmp/agentspec-v1alpha1.json") + + +@pytest.mark.skipif(not AGENTSPEC_FIXTURE.exists(), reason="agentspec fixture not available") +class TestAgentspecFixture: + def test_detects_as_json_schema(self): + doc = json.loads(AGENTSPEC_FIXTURE.read_text()) + assert detect_spec_type(doc) == "json_schema" + + def test_rename_classified_as_const_change(self): + old = json.loads(AGENTSPEC_FIXTURE.read_text()) + new = json.loads(AGENTSPEC_RENAMED.read_text()) + changes = _diff(old, new) + assert len(changes) == 1 + assert changes[0].type == JSONSchemaChangeType.CONST_CHANGED + assert changes[0].is_breaking # pre-1.0 breaking is expected; action stays advisory + assert "apiVersion" in changes[0].path + + +# ---------------------------------------------------------------------- +# edge cases +# ---------------------------------------------------------------------- + +class TestEdgeCases: + def test_empty_schemas(self): + assert _diff({}, {}) == [] + + def test_identical_schemas(self): + schema = {"type": "object", "properties": {"a": {"type": "string"}}} + assert _diff(schema, schema) == [] + + def test_none_inputs(self): + assert _diff(None, None) == [] + assert _diff(None, {}) == [] + + def test_change_severity_matches_is_breaking(self): + changes = _diff({"const": "a"}, {"const": "b"}) + assert changes[0].severity == "high" + changes = _diff({}, {"properties": {"a": {}}}) + assert changes[0].severity == "low"