feat: spec-sync pipeline (Problem 3)#99
Conversation
There was a problem hiding this comment.
Pull request overview
Implements the “spec-sync” automation loop for ade-python: periodically fetch/normalize the staging OpenAPI spec, detect drift vs a committed snapshot, regenerate reference models, and open a gated PR that is protected by a released-surface breaking-change check and live staging contract tests.
Changes:
- Added spec-sync scripts for fetch/normalize, drift detection, reference model generation, released-surface lock, and release gating.
- Added scheduled
spec-sync.ymlworkflow to open a two-phase PR on drift, pluspr-gates.ymlto enforce surface-lock and staging contract tests on PRs. - Added initial committed V1 spec snapshot + generated reference models, plus a minimal staging contract smoke test and contributor docs.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/contract/test_v1_smoke.py |
Adds a staging smoke contract test to validate auth + routing for an implemented V1 endpoint. |
tests/contract/__init__.py |
Establishes the tests.contract package for marker-based collection. |
specs/v1-ade.json |
Adds the committed normalized V1 ADE OpenAPI snapshot used as the drift baseline. |
specs/_generated/v1_models.py |
Adds generated (reference-only) Pydantic models from the committed spec snapshot. |
scripts/spec-sync/fetch-normalize.sh |
Fetches and key-sorts live OpenAPI JSON for deterministic diffs. |
scripts/spec-sync/check-drift.sh |
Compares live vs committed spec snapshot and updates snapshot on drift (exit 10). |
scripts/spec-sync/gen-models.sh |
Generates reference Pydantic v2 models from the snapshot using datamodel-codegen. |
scripts/spec-sync/surface-lock.sh |
Uses griffe to prevent breaking changes to the last-released public surface. |
scripts/spec-sync/release-gate.sh |
Blocks release when staging snapshot contains paths not yet present in production spec. |
pyproject.toml |
Adds dev deps for codegen/surface-lock and registers the contract pytest marker. |
CONTRIBUTING.md |
Documents the spec-sync pipeline, phases, gates, and required secrets. |
.github/workflows/spec-sync.yml |
Scheduled + dispatch workflow to detect drift, commit mechanical updates, run AI wiring, and open a PR. |
.github/workflows/pr-gates.yml |
PR checks for surface-lock and staging contract tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
MingruiZhang
left a comment
There was a problem hiding this comment.
Reviewed the pipeline end to end (scripts, workflows, contract test vs. the actual client surface). The architecture is right — the two-phase mechanical/AI commit split, release-tag baseline for surface-lock, and the documented anti-recursion PAT reasoning are all solid — and the scripts are unusually well-documented. I verified the contract test matches the real client (apikey=, parse_jobs.list(page=, page_size=)) and that tags are v*-prefixed for git describe.
I'd hold merge for three items (detailed inline):
- The AI wiring commit is never pushed — sync PRs would silently contain only the mechanical commit.
- Duplicate sync PRs every ~6h while one awaits review (timestamped branch + drift computed against
main). - Codegen version skew — verified with datamodel-code-generator 0.68 / griffe 2.1.0, but the hand-edited lock pins 0.45.0 / 1.14.0, so the first CI regeneration produces a spurious diff against the committed
v1_models.py.
Not anchorable inline:
- PR description vs. implementation mismatch: the body says the workflow needs
SPEC_SYNC_APP_ID+SPEC_SYNC_APP_PRIVATE_KEY(GitHub App), but the code and CONTRIBUTING use aSPEC_SYNC_TOKENfine-grained PAT. Whoever provisions secrets from the PR body will set up the wrong ones — please update the body. - Stainless interplay: until the repo is fully off Stainless, codegen pushes can conflict with the hand edits to
pyproject.toml(new markers/dev-deps). Worth confirming those blocks survive a codegen cycle. - Cosmetic:
check-drift.shexits 2 for usage errors, which can collide with curl/jq failure codes — harmless since the workflow treats all non-0/10 codes as operational failure.
Security posture is thoughtful for an AI-automation pipeline (human review required before merge), but see the inline note on the prompt-injection surface and verify branch protection on main before the PAT is provisioned.
…eline Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Reference models under specs/_generated/ are an input to the AI wiring phase and for review; not shipped. requirements-dev.lock to be refreshed via rye sync. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… tag) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…on-out) Wiring into the self-hosted release-please workflow depends on Problem 1. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ing via base_url - ruff/pyright/mypy now exclude specs/ (generated reference models are not shipped and are not held to SDK lint/type standards) — fixes the lint CI failure - requirements-dev.lock regenerated to pin datamodel-code-generator + griffe - contract test reaches staging via base_url instead of environment='staging' (the 4-environment map is Problem 2 scope, not yet available) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…s with pinned toolchain - datamodel-code-generator>=0.45 (supports the pydantic_v2 target; matches the locked/validated version) and griffe>=1.14 - regenerate specs/_generated/v1_models.py with the rye toolchain (0.45.0) so the committed reference matches what the spec-sync workflow will produce (the prior file was generated by a newer local version and would have diffed on first run) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…tHub App Org-owned App creation needs org-owner permission; a repo-scoped fine-grained PAT (Contents+PRs RW) is the self-serve workaround and still satisfies the anti-recursion requirement (not GITHUB_TOKEN, so gate workflows fire). All three writes — checkout, claude-code-action, gh pr create — use the same token. Swappable for an org App later. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
- release-gate compares (path, method) pairs, not just path keys, so a path present in prod but missing a specific method (e.g. a new POST) still blocks the release - check-drift creates the snapshot's parent dir before writing (usable with arbitrary paths; robust if the dir is cleaned in CI) Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…rface
Drop /v1/ade/extract/jobs and /v1/ade/extract/jobs/{job_id} from the committed
snapshot: the SDK does not implement them yet, so the honest 'last-synced' baseline
excludes them. This makes the snapshot truthful and lets the pipeline's first run
legitimately detect the extract/jobs gap as drift (rather than appearing fully synced).
Reference models are unchanged (those schemas live in components/schemas).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
4ed80f6 to
0ad5bea
Compare
…dening Addresses review (MingruiZhang): - AI commit now pushed by a deterministic step (claude-code-action doesn't push); the agent runs edits-only with NO git/push capability (prompt-injection hardening) - skip the run entirely when a spec-sync PR is already open (kills duplicate PRs and hourly Claude API spend during review); fixed branch spec-sync/v1 + concurrency group - open the PR before the AI step so a failed phase-2 leaves a visible PR, not an orphan branch - cron hourly Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ntract tests Addresses review (MingruiZhang / Copilot): - contract-tests only gate spec-sync PRs (not every PR repo-wide) and run serially (-n 0) - surface-lock escape hatch: skip on 'breaking-change-approved' label - pin datamodel-code-generator==0.45.0 / griffe==1.14.0 (committed models match CI regen) - addopts '-m not contract' so plain pytest never hits live staging - document jq -S array-ordering assumption in fetch-normalize.sh Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
… cron Addresses Copilot review: - contract test uses a yield-fixture inside a context manager so the HTTP client is closed in teardown (no socket leak) - CONTRIBUTING now says hourly cron, matching spec-sync.yml (was stale '~6h') Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Implements the spec-sync pipeline (Problem 3) per the plan in #97 / #98. Scope: ade-python.
What's here (verified locally)
scripts/spec-sync/fetch-normalize.sh+ committedspecs/v1-ade.json(seeded from staging, which drives the loop)scripts/spec-sync/check-drift.sh(exit 10 = drift)scripts/spec-sync/gen-models.sh+specs/_generated/v1_models.py(reference only)scripts/spec-sync/surface-lock.sh(griffe vs last release tag)v1.12.0; fails on a simulated public-method removaltests/contract/+contractmarker.github/workflows/spec-sync.yml(two-phase: mechanical +claude-code-action).github/workflows/pr-gates.yml(surface-lock + contract-tests)scripts/spec-sync/release-gate.sh(staging-in/prod-out)Verification note:
ryeisn't available in my environment, so I verified script/logic behavior with a throwaway venv (griffe 2.1.0,datamodel-code-generator 0.68, pytest). CI runs these throughrye runas the workflows specify.Not done here — ops / dependency-gated (intentionally)
SPEC_SYNC_APP_ID+SPEC_SYNC_APP_PRIVATE_KEYGitHub App,ANTHROPIC_API_KEY,LANDINGAI_ADE_STAGING_APIKEY) and dispatching the workflow in Actions. Once secrets are set,workflow_dispatchshould produce the extract/jobs PR; compare it to feat: add async extract jobs support to SDK #93, then close feat: add async extract jobs support to SDK #93.requirements-dev.lockneeds arye syncto pin the two new dev deps (datamodel-code-generator,griffe) — I can't run rye here.pydantic_v2.BaseModelis used for reference-model codegen (datamodel-code-generator 0.68 dropped the v1 target); reference models are throwaway so this is fine.Follow-ups
specs/v2-aide.json+ URL to the workflow once aide serves the V2 spec unauthenticated.Tracking: #97 · plan: #98