diff --git a/.github/workflows/promote.yml b/.github/workflows/promote.yml index d519db7..620c8dd 100644 --- a/.github/workflows/promote.yml +++ b/.github/workflows/promote.yml @@ -7,7 +7,8 @@ name: Promote (test / prod) # - Push a v* tag -> PROD: deploy the exact image TEST already validated, by digest, # referencing TEST's registry directly (no rebuild, no copy). Gated by the `prod` # Environment's required-reviewer rule. prod's deployer SA + Cloud Run service agent must -# have read on TEST's Artifact Registry. +# have read on TEST's Artifact Registry. The tag must equal "v" + pyproject.toml's version +# (checked below): no rebuild happens here, so the version is whatever test baked in. # # For the prod path, test's registry coordinates come from repo-level Variables (test's project # isn't in prod's Environment): TEST_PROJECT_ID (required), TEST_REGION / TEST_AR_REPO (optional). @@ -89,6 +90,24 @@ jobs: build_label: ${{ steps.r.outputs.sha }} source_image: ${{ steps.r.outputs.image }} steps: + - uses: actions/checkout@v7 + + # The tag names the release, but the version the running server reports comes from + # pyproject.toml, baked into the image at build time. Prod redeploys test's existing digest + # and never rebuilds, so a tag that disagrees with the packaged version would deploy fine + # and quietly serve the wrong version at /v3/version. Fail here, before the reviewer gate. + - name: Verify the tag matches the packaged version + env: + TAG: ${{ github.ref_name }} + run: | + set -euo pipefail + VERSION=$(python3 -c 'import tomllib; print(tomllib.load(open("pyproject.toml","rb"))["project"]["version"])') + if [ "${TAG#v}" != "$VERSION" ]; then + echo "::error::Tag '${TAG}' does not match pyproject.toml version '${VERSION}'. The version is baked in when test builds the image, so bump pyproject.toml, promote that commit to test, then tag it. See CONTRIBUTING.md -> Releasing." + exit 1 + fi + echo "Tag ${TAG} matches packaged version ${VERSION}." + - id: r env: TEST_PROJECT: ${{ vars.TEST_PROJECT_ID }} diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..d84f377 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,52 @@ +# Changelog + +All notable changes to this project are documented here. + +The format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project +adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html) with one house rule: the +**MAJOR version is pinned to the served URL prefix** — `/v3` ↔ `3.y.z`. A breaking change to the +REST or MCP contract means a new prefix (`/v4`) and a new major (`4.0.0`), never a silent break +under `/v3`. See [CONTRIBUTING.md](CONTRIBUTING.md#versioning) for the full policy. + +Entries describe **user-visible** change — endpoints, MCP tools, response shapes, configuration. +Refactors, CI, and formatting land in the git history, not here. + +## [Unreleased] + +Nothing yet. + +## [3.0.0b1] — unreleased + +First public release of the v3 API: a rewrite that replaces the v1/v2 service with a single +backend-agnostic core behind two thin adapters (REST + MCP), served from the `idc-index` Parquet +index queried locally with DuckDB. + +**This is a beta.** The `/v3` contract may still change in response to feedback before `3.0.0`. +Pin to an exact version if you need stability. Legacy v1/v2 endpoints are unaffected — they are +served by a different backend and v3 lives only under `/v3/*`. + +### Added + +- **REST API**, entirely under the `/v3` prefix: discovery (`/v3/version`, `/v3/stats`, + `/v3/collections`, `/v3/analysis_results`, `/v3/attributes`, `/v3/tables`), cohort building, + retrieval (manifests / cohort URLs, viewer URLs), citations and licenses, guarded SQL, and + `/v3/health`. Interactive docs at `/v3/docs`; the bare domain redirects there. +- **MCP server** over stdio (local) and streamable-http (hosted at `/mcp`), exposing the same + capabilities as tools — `list_collections`, `get_collection`, `list_attributes`, + `get_attribute_values`, `build_cohort`, `get_cohort_urls`, `download_cohort`, `get_viewer_url`, + `run_sql`, `get_citations`, `get_licenses`, and the clinical/table introspection tools — plus + an `idc://guide` resource describing the data model and workflow. +- **Guarded SQL** (`POST /v3/sql`, `run_sql`): read-only DuckDB with external access and + extension loading disabled, single-statement enforcement, a server row cap, and a timeout. + See [SECURITY.md](SECURITY.md). +- **Specialized indices** joinable to `index` on `SeriesInstanceUID` (`seg_index`, `ann_index`, + `ct_index`, `mr_index`, `pt_index`, `sm_index`, …) and per-collection **clinical tables** under + a `clinical` schema, both fetched from `idc-index` releases at build time. +- **Software version reporting**, distinct from the IDC *data* version: `/v3/version` returns + `api_version` (and `build`, when a deploy stamps `IDC_API_BUILD`); the same string appears in + the OpenAPI `info.version` and the MCP `initialize` handshake (`serverInfo.version`). +- **Structured audit logging** — one JSON line per REST request and MCP tool call. + `IDC_API_SQL_LOG_MODE` selects how the guarded SQL query is rendered (`snippet` or `hash`). + +[Unreleased]: https://github.com/ImagingDataCommons/IDC-REST-MCP/compare/v3.0.0b1...HEAD +[3.0.0b1]: https://github.com/ImagingDataCommons/IDC-REST-MCP/releases/tag/v3.0.0b1 diff --git a/CLAUDE.md b/CLAUDE.md index 2e85626..bcde1a6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -65,10 +65,20 @@ Docs are split by audience — keep them in their lanes: - **[README.md](README.md)** — kept **lean**: intro, status, install, run one-liners, deploy, and pointers. **Do not** add endpoint/tool reference or usage detail here — that goes in the user guide. +- **[CONTRIBUTING.md](CONTRIBUTING.md)** — the **process**: branching, Conventional Commits, the + changelog rule, and the versioning policy (SemVer with MAJOR pinned to the URL prefix, + `/v3` ↔ `3.y.z`). **Do not** restate setup/test commands or the architecture invariants here — + it points at `dev/developer_guide.md` for those. The release *runbook* lives in + `dev/deployment.md`, not here: it is a maintainer task, not a contributor one. +- **[CHANGELOG.md](CHANGELOG.md)** — hand-curated, [Keep a Changelog](https://keepachangelog.com/) + format. Describes **user-visible** change only (endpoints, MCP tools, response shapes, config + vars, defaults) — never refactors, CI, or dependency bumps. Not generated from commits. - **`dev/`** — design & contributor docs: [architecture.md](dev/architecture.md), [api_v3_plan.md](dev/api_v3_plan.md) (design rationale + SQL threat model), - [deployment.md](dev/deployment.md), [developer_guide.md](dev/developer_guide.md). + [deployment.md](dev/deployment.md) (Cloud Run tiers **and** the release runbook), + [developer_guide.md](dev/developer_guide.md) (setup, test, CI, the invariants, walkthroughs). When you add or change a capability: update `docs/user-guide.md`; mirror any conceptual change into the `idc://guide` resource; check whether `INSTRUCTIONS` needs a one-line touch (it -usually shouldn't, if it stays lean); keep `README.md` a pointer. +usually shouldn't, if it stays lean); keep `README.md` a pointer. If the change is user-visible, +add a `CHANGELOG.md` entry under `## [Unreleased]` in the same PR. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..a60f80a --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,127 @@ +# Contributing + +Thanks for helping improve the IDC REST API + MCP server. This document is the **process**: how we +branch, commit, changelog, and version. + +The **code** is documented elsewhere, and this file does not repeat it: + +| You want to | Read | +|---|---| +| Set up, run, and test the project | [dev/developer_guide.md](dev/developer_guide.md) — *Setup*, *Run*, *Test* | +| Know what CI will run against your PR | [dev/developer_guide.md](dev/developer_guide.md) — *Continuous integration* | +| Understand the invariants reviewers check | [dev/developer_guide.md](dev/developer_guide.md) — *Conventions* | +| Add a capability (model → service → REST → MCP → parity test) | [dev/developer_guide.md](dev/developer_guide.md) — *Walkthrough* | +| Understand *why* it's built this way | [dev/architecture.md](dev/architecture.md) | +| Cut a release / deploy | [dev/deployment.md](dev/deployment.md) — *Cutting a release* | + +In short: `uv pip install -e ".[dev]"`, then make sure `ruff`, `bandit`, and `pytest` pass before +you push — the developer guide has the exact commands CI uses. + +Documentation is split by audience (user guide vs. the agent-facing `idc://guide` resource vs. +the always-on MCP `INSTRUCTIONS` vs. `dev/`). Keep each in its lane — the conventions, and which +file to touch when, are in [CLAUDE.md](CLAUDE.md#documentation-conventions). + +## Branches + +Branch off `main`, named `/` using the same type vocabulary as commits: + +``` +feat/cohort-size-estimate fix/mcp-trailing-slash +docs/api-endpoint-examples ci/multi-tier-deploy +``` + +Pull requests are merged with a merge commit, so the individual commits on your branch land in +`main`'s history. Make them ones you'd want to read later. + +## Commits + +We follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/): + +``` +(): +``` + +**Types:** `feat`, `fix`, `docs`, `test`, `refactor`, `perf`, `style`, `build`, `ci`, `chore`. +**Scopes** in use: `rest`, `mcp`, `api`, `deploy`, `deps`. + +``` +feat(rest): redirect the bare domain to the interactive docs +fix(mcp): serve /mcp and /mcp/ directly instead of redirecting +docs(api): add OpenAPI summaries/descriptions to every REST route +``` + +This is a convention, not a CI gate — nothing will fail your build if you deviate. It exists so +history stays skimmable and so the changelog is easy to assemble at release time. Dependabot's +commits don't always conform; that's fine. + +Mark a breaking change to the REST or MCP contract with a `!` (`feat(rest)!: …`) and a +`BREAKING CHANGE:` footer. See [Versioning](#versioning) — such a change needs a new URL prefix, +so it is a much bigger conversation than a commit message. + +## Changelog + +[CHANGELOG.md](CHANGELOG.md) is **hand-curated**, in [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) +format. It is not generated from commits: it describes what changed for *callers of the API*, +which is a different thing from what changed in the tree. + +**If your PR changes user-visible behavior, add an entry to `## [Unreleased]` in the same PR.** +User-visible means an endpoint, an MCP tool or its description, a response shape, a configuration +variable, or a default. Use the standard groupings — `Added`, `Changed`, `Deprecated`, `Removed`, +`Fixed`, `Security` — and write for someone consuming the API, not someone reading the diff: + +```markdown +### Fixed +- MCP: `/mcp` and `/mcp/` are both served directly; neither redirects. +``` + +Refactors, test changes, CI, formatting, and dependency bumps do **not** get an entry. The git +history already records them. + +> While `3.0.0b1` is still unreleased, fixes to code that has never shipped are folded into that +> release's `Added` section rather than listed under `Fixed` — there is no released behavior to +> have fixed. Once the beta ships, use the groupings normally. + +## Versioning + +[Semantic Versioning](https://semver.org/spec/v2.0.0.html), with one house rule: + +**MAJOR is pinned to the served URL prefix.** `/v3` ↔ `3.y.z`, always. + +| Change | Version | URL | +|---|---|---| +| Add an endpoint, MCP tool, or optional field | MINOR — `3.1.0` | `/v3` | +| Fix a bug without changing the contract | PATCH — `3.0.1` | `/v3` | +| Break the REST or MCP contract | MAJOR — `4.0.0` | new prefix `/v4` | + +So a breaking change is never a silent break under `/v3`: it is a new prefix served alongside the +old one. This keeps `api_version` predictive of the URL, and matches the clean break v3 already +made from v1/v2. + +Pre-releases use [PEP 440](https://peps.python.org/pep-0440/) spelling so the Python package +version and the git tag agree: `3.0.0b1` → tag `v3.0.0b1`; `3.0.0rc1` → tag `v3.0.0rc1`. + +**The version lives in exactly one place: `version` in [pyproject.toml](pyproject.toml).** +Everything else derives from it — `idc_api.__version__` and `core/version.py:package_version()` +both read the installed distribution metadata. Never hardcode it a second time. + +## Releasing + +Cutting a release is a **maintainer** task that deploys to production. The runbook — including the +v3 beta plan — lives with the deploy machinery it depends on: +[dev/deployment.md § Cutting a release](dev/deployment.md#cutting-a-release). + +Two things every contributor should know: + +> [!IMPORTANT] +> **Pushing a `v*` tag deploys to production.** [promote.yml](.github/workflows/promote.yml) +> triggers on `push: tags: ["v*"]`, and that glob matches pre-release tags too — `v3.0.0b1` goes +> to prod exactly like `v3.0.0`. Never create a `v*` tag for bookkeeping, and be careful with +> `git push --tags`, which can fire a deploy from a stale local tag. + +And you cannot tag a release without bumping `pyproject.toml` first: `promote.yml` asserts that +the tag equals `"v"` + the packaged version, and fails before the reviewer gate if they disagree. + +## Reporting security issues + +Please don't open a public issue for a vulnerability — use GitHub's private reporting flow, as +described in [SECURITY.md](SECURITY.md). diff --git a/README.md b/README.md index d40369b..2ad5373 100644 --- a/README.md +++ b/README.md @@ -29,7 +29,9 @@ both. - [`dev/architecture.md`](dev/architecture.md) — internal design (one core, two adapters). - [`dev/api_v3_plan.md`](dev/api_v3_plan.md) — full design rationale + SQL threat model. - [`dev/deployment.md`](dev/deployment.md) — Cloud Run deployment. -- [`dev/developer_guide.md`](dev/developer_guide.md) — contributing / adding capabilities. +- [`dev/developer_guide.md`](dev/developer_guide.md) — codebase tour / adding capabilities. +- [`CONTRIBUTING.md`](CONTRIBUTING.md) — branching, commits, changelog, versioning, releases. +- [`CHANGELOG.md`](CHANGELOG.md) — what changed for callers, per release. - [`SECURITY.md`](SECURITY.md) — threat model, hardening in place, and how to report a vulnerability. diff --git a/dev/deployment.md b/dev/deployment.md index 785c704..829f4fe 100644 --- a/dev/deployment.md +++ b/dev/deployment.md @@ -248,10 +248,64 @@ file — the first becomes the *trigger*, the second becomes an *Environment set | [promote.yml](../.github/workflows/promote.yml) (tag) | push a `v*` tag | deploy test's digest to **prod** (behind the required-reviewer gate) | | [deploy.yml](../.github/workflows/deploy.yml) | reusable (`workflow_call`) | the shared deploy job the callers invoke | -Prod deploys **test's** image, so a `v*` tag must point at a commit that was **already promoted to -test** (its image exists in test's registry). Cut releases from a ref you've dispatched to test; -tag a commit that never went through test and the prod deploy fails fast at the digest-resolve -step. +### Cutting a release + +Versioning policy — what a version *number* means, and when to bump which part — is in +[CONTRIBUTING.md](../CONTRIBUTING.md#versioning). This section is the mechanics. + +> [!IMPORTANT] +> **Pushing a `v*` tag deploys to production**, and the glob matches pre-release tags too — +> `v3.0.0b1` goes to prod exactly like `v3.0.0`. Never create a `v*` tag for bookkeeping, and +> beware `git push --tags` firing a deploy from a stale local tag. + +Two constraints fall out of the security boundary above — prod runs test's exact bytes and never +rebuilds: + +1. **A `v*` tag must point at a commit already promoted to test** (its image exists in test's + registry). Cut releases from a ref you've dispatched to test; tag a commit that never went + through test and the prod deploy fails fast at the digest-resolve step. +2. **The version bump must be its own commit, and it must go through test.** The version comes from + the installed package metadata, baked into the image when *test* builds it; `IDC_API_BUILD` only + stamps the git SHA on top. Tagging `v3.0.0` on the same commit that shipped as `3.0.0b1` would + redeploy an image that still reports `3.0.0b1` at `/v3/version`. `promote.yml` guards this: on a + `v*` tag it asserts `tag == "v" + pyproject version` and fails **before** the reviewer gate if + they disagree. + +#### Steps + +1. **Bump and curate.** In one PR: set `version` in [pyproject.toml](../pyproject.toml), and in + [CHANGELOG.md](../CHANGELOG.md) rename `## [Unreleased]` to `## [X.Y.Z] — YYYY-MM-DD`, open a + fresh empty `[Unreleased]`, and update the link definitions at the foot of the file. Merge it. +2. **Promote to test.** Run [promote.yml](../.github/workflows/promote.yml) via workflow dispatch + against that merge commit. It builds the canonical image into test's registry and deploys + `testing-api.canceridc.dev`. +3. **Verify** against test — `/v3/health`, `/v3/version` (confirm it reports the version you just + set), and the MCP handshake at `/mcp`. +4. **Tag.** `git tag -a v3.0.0 -m "v3.0.0" && git push origin v3.0.0`. This starts the + prod deploy, which waits on the `prod` Environment's required-reviewer gate. +5. **Approve** the deployment, then confirm `api.imaging.datacommons.cancer.gov/v3/version`. +6. **Publish a GitHub Release** on the tag, with the changelog section as its body. Tick **"Set as a + pre-release"** for `bN` / `rcN` tags. + +#### The v3 beta + +v3 ships to production as `3.0.0b1` before `3.0.0`. + +The beta is **not** a traffic-safety measure — it can't be one. The prod load balancer routes only +`/v3/*` to the `idc-api-v3` service (see [Shared-domain path routing](#shared-domain-path-routing-as-deployed--the-glob-gotcha) +below); every other path falls through to the legacy ESP backend. No existing caller reaches v3, so +shipping it cannot break them. What the beta buys is the freedom to **change the `/v3` contract in +response to real usage** without spending a major version — which, under the versioning policy, +would otherwise cost a whole new `/v4` prefix — plus an honest signal to early adopters that the +surface may move. Exit the beta by tagging `v3.0.0` once the contract has held under real use. + +Deliberately **not** doing a Cloud Run traffic split (`--tag beta --no-traffic` plus a percentage +rollout) for this release. There is no incumbent v3 revision to canary against, and percentage +splits are applied **per request**, not per session — an MCP streamable-http session could have its +requests land on different revisions mid-conversation unless `--session-affinity` is enabled. If a +canary becomes worthwhile for a later release (`3.1.0` onward, once v3 has consumers), use a +**tagged revision** at zero traffic, which gets its own `beta---idc-api-v3-*.run.app` URL that +testers opt into explicitly, rather than a percentage split of the live domain. ### One-time setup diff --git a/dev/developer_guide.md b/dev/developer_guide.md index 1d2a2b7..5dbb795 100644 --- a/dev/developer_guide.md +++ b/dev/developer_guide.md @@ -64,14 +64,17 @@ Fixtures live in [tests/conftest.py](../tests/conftest.py): `ctx` (the core [`.github/workflows/ci.yml`](../.github/workflows/ci.yml) runs on pushes to `main` (and manual dispatch) and on PRs that touch `src/idc_api/**`, `tests/**`, `pyproject.toml`, -or `uv.lock`. It installs locked deps with `uv sync --extra dev`, then runs `ruff check` and -`pytest tests` on Python 3.11 and 3.12. CI needs no secrets or GCP; its first test run +or `uv.lock`. It installs locked deps with `uv sync --extra dev`, then lints, scans, and tests on +Python 3.11 and 3.12. CI needs no secrets or GCP; its first test run downloads the specialized indices (`IDC_API_INCLUDE_INDICES=all` by default — set `none` to run -from bundled data only). Before pushing, run the same two commands locally: +from bundled data only). Before pushing, run the same checks locally: ```bash -uv run ruff check src tests -uv run pytest tests -q +uv run ruff check src tests # lint +uv run ruff format --check src tests # formatting +uv run bandit -q -r src/idc_api # static security lint +uv run pip-audit # dependency CVEs +uv run pytest tests -q # tests ``` ## Project layout diff --git a/src/idc_api/__init__.py b/src/idc_api/__init__.py index cbaa1ea..3032ba3 100644 --- a/src/idc_api/__init__.py +++ b/src/idc_api/__init__.py @@ -4,4 +4,12 @@ ``mcp`` packages are thin adapters over it. See ``dev/api_v3_plan.md`` for the design. """ -__version__ = "3.0.0.dev0" +from importlib.metadata import PackageNotFoundError, version as _pkg_version + +try: + # Single source of truth: `version` in pyproject.toml. Do not hardcode it here — the version + # the *running server* advertises resolves through ``core.version``, which reads the same + # distribution metadata, so a literal here would silently drift at release time. + __version__ = _pkg_version("idc-api") +except PackageNotFoundError: # running from a source tree without an install + __version__ = "0.0.0+unknown"