From 159310269fcfbc6f0b01cfa002115dc03232e77b Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:12:37 +0000 Subject: [PATCH 01/10] ci: breaking-change gate on the pg-pkg OpenAPI contract (oasdiff) The gate itself is an oasdiff job in .github/workflows/api-diff.yml, which the App cannot push, so the YAML ships as a comment on this PR for a maintainer to apply. What is committed here is the docs half: COMPATIBILITY.md and CLAUDE.md now say the /v2 rules are still a review rule, name the job and where its YAML is waiting, and record how to reproduce the verdict locally (oasdiff v1.26.1, the version the pinned action runs). Refs #249. Part of #247 (workstream C). --- CLAUDE.md | 1 + COMPATIBILITY.md | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index df77c61c..d211aa1d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,6 +9,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you. Reproduce the verdict locally with the same engine CI uses — `oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so `go install github.com/oasdiff/oasdiff@v1.26.1`, write the base copy out with `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml`, then `oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --fail-on ERR`. The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. Checked against v1.26.1: a removed route and a request property becoming required are ERR; a new endpoint and a new optional response field are not; neither is a `/v3` route added alongside `/v2`, which is the escape hatch for a change `/v2` cannot take additively. - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. ## Dependencies diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index b9b89ebd..9ff1ca65 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -26,8 +26,12 @@ removed once the deprecation process at the bottom of this file has run for it ([#257]). Until then it keeps working, so a deployed client on `/v2/irma/...` is not broken by this notice. -Planned enforcement: an oasdiff gate that diffs the spec against `main` and -fails on a breaking change ([#249]). Until that lands, this is a review rule. +Enforcement is written but not running yet: the `API breaking changes (oasdiff)` +job diffs the spec against the branch a PR targets and fails on anything oasdiff +rates ERR ([#249]). The workflow file cannot be pushed by the bot that wrote it, +so it sits in a comment on [#269] until a maintainer applies it. Until then the +rules above are a review rule. A `/v3` route added next to `/v2` is additive, so +the gate passes the escape hatch. ## Stored artifacts @@ -129,6 +133,7 @@ Skipping step 2 is how you break the consumers you cannot see. [#257]: https://github.com/encryption4all/postguard/issues/257 [#260]: https://github.com/encryption4all/postguard/issues/260 [#261]: https://github.com/encryption4all/postguard/issues/261 +[#269]: https://github.com/encryption4all/postguard/pull/269 [postguard-js#131]: https://github.com/encryption4all/postguard-js/issues/131 [postguard-e2e#19]: https://github.com/encryption4all/postguard-e2e/issues/19 [postguard-e2e#21]: https://github.com/encryption4all/postguard-e2e/issues/21 From 56ce480427cccae800a2eb8840989d0ae3adf693 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 11:54:03 +0000 Subject: [PATCH 02/10] ci: raise the oasdiff gate to WARN and pin its verdicts in a test The gate as reviewed promised more than fail-on: ERR delivers. Measured against oasdiff v1.26.1 on the real spec, ERR silently passed six changes COMPATIBILITY.md forbids: a changed status code, a dropped response enum value, a removed and a renamed optional response property, a removed non-success status, and a removed request parameter. Two of those (response-non-success-status-removed, response-property-enum-value-removed) rate ERR but are opt-in, so they only run when named in include-checks. The rest are WARN, and on this spec that is most of the "no removing a route or a field" rule: only `status` is ever `required`, so `key`, `proofStatus`, `pubSignKey` and the rest are optional. So the gate now runs fail-on: WARN plus those two checks. Of the 31 WARN checks, response-property-enum-value-added is the only one that fires on something COMPATIBILITY.md did not already forbid, so that document now names a new response enum value as non-additive too. pg-pkg/tests/api_gate.rs is the regression test: it mutates the spec 18 ways, runs the engine with the flags the action's entrypoint builds, and asserts which mutations the gate stops. It fails on the old settings with all six gaps named, and skips when oasdiff is not on PATH, which is the case in CI. Refs #249, #242. Part of #247 (workstream C). Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 3 +- COMPATIBILITY.md | 21 +- pg-pkg/tests/api_gate.rs | 521 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 538 insertions(+), 7 deletions(-) create mode 100644 pg-pkg/tests/api_gate.rs diff --git a/CLAUDE.md b/CLAUDE.md index d211aa1d..870d7e52 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,7 +9,8 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you. Reproduce the verdict locally with the same engine CI uses — `oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so `go install github.com/oasdiff/oasdiff@v1.26.1`, write the base copy out with `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml`, then `oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --fail-on ERR`. The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. Checked against v1.26.1: a removed route and a request property becoming required are ERR; a new endpoint and a new optional response field are not; neither is a `/v3` route added alongside `/v2`, which is the escape hatch for a change `/v2` cannot take additively. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you and `pg-pkg/tests/api_gate.rs`. That test is the executable record of what the gate stops; it mutates the spec 18 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. ## Dependencies diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index 9ff1ca65..9ceb839f 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -14,7 +14,9 @@ Changes to `/v2` are additive only. New endpoints, new optional request fields and new response fields are allowed, so a client written against an older revision of the spec keeps working. These are not allowed on `/v2`: removing a route or a field, renaming either, narrowing a type, making an optional field -required, or changing the status code for a condition a client already handles. +required, changing the status code for a condition a client already handles, or +adding a value to a response enum, since a client that switches on `status` +without a default branch breaks on a value it has never seen. A change that cannot be made additively ships under a new prefix (`/v3`), with `/v2` left running until its consumers are gone. @@ -27,11 +29,18 @@ removed once the deprecation process at the bottom of this file has run for it is not broken by this notice. Enforcement is written but not running yet: the `API breaking changes (oasdiff)` -job diffs the spec against the branch a PR targets and fails on anything oasdiff -rates ERR ([#249]). The workflow file cannot be pushed by the bot that wrote it, -so it sits in a comment on [#269] until a maintainer applies it. Until then the -rules above are a review rule. A `/v3` route added next to `/v2` is additive, so -the gate passes the escape hatch. +job diffs the spec against the branch a PR targets and fails on any change +oasdiff rates WARN or ERR ([#249]). That covers every rule above except one. The +gate compares documented paths, and this spec documents the canonical +`/v2/request/...` paths only, so dropping the `/v2/irma/...` alias handlers +before the deprecation above has run passes it; that one stays a review rule. A +`/v3` route added next to `/v2` reads as additive, so the gate passes the escape +hatch. `pg-pkg/tests/api_gate.rs` pins which changes the gate stops and which it +lets through; run it before changing what the gate checks. + +The workflow file cannot be pushed by the bot that wrote it, so it sits in a +comment on [#269] until a maintainer applies it. Until then all of the above is +a review rule. ## Stored artifacts diff --git a/pg-pkg/tests/api_gate.rs b/pg-pkg/tests/api_gate.rs new file mode 100644 index 00000000..ce3ae387 --- /dev/null +++ b/pg-pkg/tests/api_gate.rs @@ -0,0 +1,521 @@ +//! What the API breaking-change gate actually catches (issue #249). +//! +//! `.github/workflows/api-diff.yml` runs `oasdiff breaking` over +//! `api-description.yaml`, and it is what stops a careless edit from breaking a +//! deployed client. Its verdict is decided entirely by two step inputs, +//! `fail-on` and `include-checks`, and getting them wrong fails open: the job +//! goes green and nobody learns that the change it was supposed to stop went +//! through. `fail-on: ERR` alone passes a removed optional response property, a +//! removed request parameter, a changed status code and a dropped response enum +//! value, all of which COMPATIBILITY.md forbids. +//! +//! So the inputs are pinned here rather than only in the workflow: this test +//! mutates the real spec, runs the real engine with the real flags, and asserts +//! which mutations the gate stops. Change [`FAIL_ON`] or [`INCLUDE_CHECKS`] +//! without changing the workflow to match and the two drift apart with nothing +//! to say so; change the workflow without running this and the gate weakens. +//! +//! The engine is not vendored, so the test needs `oasdiff` on `PATH` (or +//! `OASDIFF` pointing at it) and skips when it is absent, which is the case in +//! CI. Install the version the action pins, so a local verdict is CI's verdict: +//! +//! ```text +//! go install github.com/oasdiff/oasdiff@v1.26.1 +//! cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate +//! ``` + +use std::env; +use std::fs; +use std::path::{Path, PathBuf}; +use std::process::Command; + +/// `fail-on` in the workflow's oasdiff step. WARN, not ERR: on this spec only +/// `status` is ever `required`, so removing or renaming any other response +/// property is WARN, and at ERR the gate would pass most of what +/// COMPATIBILITY.md forbids. +const FAIL_ON: &str = "WARN"; + +/// `include-checks` in the workflow's oasdiff step. Both rate ERR but are +/// opt-in, so they do not run unless named. +const INCLUDE_CHECKS: &str = + "response-non-success-status-removed,response-property-enum-value-removed"; + +/// Whether the gate stops a change, i.e. whether the job goes red. +#[derive(Debug, PartialEq, Eq)] +enum Gate { + /// Additive as far as a deployed client is concerned. + Passes, + /// Breaking under COMPATIBILITY.md's `/v2` rules. + Stops, +} + +impl Gate { + fn verb(&self) -> &'static str { + match self { + Gate::Passes => "pass", + Gate::Stops => "stop", + } + } + + fn past(&self) -> &'static str { + match self { + Gate::Passes => "passed", + Gate::Stops => "stopped", + } + } +} + +fn spec_path() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("api-description.yaml") +} + +/// The `oasdiff` binary, or `None` when it is not installed. +fn oasdiff() -> Option { + if let Some(explicit) = env::var_os("OASDIFF") { + return Some(PathBuf::from(explicit)); + } + let found = Command::new("oasdiff") + .arg("--help") + .output() + .is_ok_and(|out| out.status.success()); + found.then(|| PathBuf::from("oasdiff")) +} + +/// Replaces `old` with `new`, requiring `old` to occur exactly once so a spec +/// edit that moves an anchor fails loudly instead of silently mutating nothing. +fn once(text: &str, old: &str, new: &str) -> String { + assert_eq!( + text.matches(old).count(), + 1, + "anchor is not unique in the spec, so this mutation no longer means what it says: {old:?}" + ); + text.replacen(old, new, 1) +} + +/// The half-open byte range of the block starting at `start` and ending where +/// the next `end` begins. +fn block(text: &str, start: &str, end: &str) -> (usize, usize) { + let from = text.find(start).unwrap_or_else(|| panic!("no {start:?}")); + let to = text[from..] + .find(end) + .unwrap_or_else(|| panic!("no {end:?} after {start:?}")); + (from, from + to) +} + +/// Runs the gate exactly as the workflow's step does: the committed spec as the +/// base, `revision` as the PR's version. +/// +/// The entrypoint of `oasdiff/oasdiff-action/breaking@v0.1.10` turns the step +/// inputs into `--allow-external-refs=false --include-checks +/// --composed=false --fail-on `, so those are the flags used here. Exit 0 +/// is a clean diff and exit 1 is "breaking changes found"; anything else is the +/// engine refusing the input, which means the mutation produced a spec oasdiff +/// cannot load and any verdict read off it would be meaningless. +fn gate(oasdiff: &Path, revision: &Path) -> Gate { + let out = Command::new(oasdiff) + .arg("breaking") + .arg(spec_path()) + .arg(revision) + .arg("--allow-external-refs=false") + .args(["--include-checks", INCLUDE_CHECKS]) + .arg("--composed=false") + .args(["--fail-on", FAIL_ON]) + .output() + .expect("run oasdiff"); + + match out.status.code() { + Some(0) => Gate::Passes, + Some(1) => Gate::Stops, + other => panic!( + "oasdiff exited {other:?} instead of 0 or 1, so it never reached a verdict:\n{}\n{}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ), + } +} + +// The two response blocks the mutations below anchor on. Both are unique in the +// spec, and `once` fails the test if that ever stops being true. + +const TIMESTAMP_PARAMETER: &str = r##" - name: timestamp + in: path + required: true + description: "Unix seconds; the policy timestamp the key must match." + schema: + type: integer + format: int64 +"##; + +const RATE_LIMITED_THEN_IRMA_503: &str = r##" "429": + description: "Rate limited." + "503": + description: "IRMA verification key unavailable (IRMA server unreachable)." +"##; + +const JWT_401: &str = r##" "401": + description: "Missing/invalid JWT." + content: + application/json: + schema: + $ref: "#/components/schemas/Error" +"##; + +// --------------------------------------------------------------------------- +// Additive: allowed on `/v2`, so the gate must let these through. A gate that +// stops them is worse than no gate, because the way around it is to switch it +// off. +// --------------------------------------------------------------------------- + +fn add_endpoint(spec: &str) -> String { + once( + spec, + " /v2/request/key:\n", + r##" /v2/request/echo: + get: + tags: ["Keys"] + summary: "Echo" + operationId: "echo" + responses: + "200": + description: "ok" + /v2/request/key: +"##, + ) +} + +fn add_optional_response_property(spec: &str) -> String { + once( + spec, + " key:\n description: |\n Opaque pg-core-serialized IBE", + r##" issuedAt: + type: integer + format: int64 + description: "Unix seconds the key was derived." + key: + description: | + Opaque pg-core-serialized IBE"##, + ) +} + +fn add_optional_request_property(spec: &str) -> String { + once( + spec, + " required: [pubSignId]\n additionalProperties: false\n properties:\n", + r##" required: [pubSignId] + additionalProperties: false + properties: + clientHint: + type: string +"##, + ) +} + +fn add_required_response_property(spec: &str) -> String { + once( + spec, + " required: [tenant_id]\n properties:\n", + r##" required: [tenant_id, checked_at] + properties: + checked_at: + type: integer + format: int64 +"##, + ) +} + +fn add_optional_query_parameter(spec: &str) -> String { + once( + spec, + " operationId: \"key\"\n security:", + r##" operationId: "key" + parameters: + - name: verbose + in: query + required: false + schema: + type: boolean + security:"##, + ) +} + +fn add_response_status(spec: &str) -> String { + let with_404 = format!( + " \"404\":\n description: \"No such timestamp.\"\n{RATE_LIMITED_THEN_IRMA_503}" + ); + once(spec, RATE_LIMITED_THEN_IRMA_503, &with_404) +} + +fn edit_a_description(spec: &str) -> String { + once( + spec, + "description: \"Yivi session status.\"", + "description: \"Yivi session status (see the Yivi docs).\"", + ) +} + +/// The escape hatch COMPATIBILITY.md points at: a change `/v2` cannot take +/// additively ships under `/v3` with `/v2` left running. If the gate stopped +/// this there would be no way to make a breaking change at all. +fn add_v3_route_beside_v2(spec: &str) -> String { + let (from, to) = block(spec, " /v2/request/key:\n", " /v2/request/sign/key:\n"); + let v3 = spec[from..to] + .replacen(" /v2/request/key:", " /v3/request/key:", 1) + .replacen("operationId: \"key\"", "operationId: \"keyV3\"", 1); + once( + spec, + " /v2/request/sign/key:\n", + &format!("{v3} /v2/request/sign/key:\n"), + ) +} + +// --------------------------------------------------------------------------- +// Breaking: forbidden on `/v2` by COMPATIBILITY.md, so the gate must stop +// these. Each one breaks a client written against today's spec. +// --------------------------------------------------------------------------- + +fn remove_route(spec: &str) -> String { + let (from, to) = block( + spec, + " /v2/request/key/{timestamp}:\n", + " /v2/request/key:\n", + ); + format!("{}{}", &spec[..from], &spec[to..]) +} + +fn request_property_becomes_required(spec: &str) -> String { + once( + spec, + " required: [pubSignId]\n", + " required: [pubSignId, privSignId]\n", + ) +} + +/// A client that handles 401 by refreshing its JWT sees an unhandled 403. +fn change_a_status_code(spec: &str) -> String { + once( + spec, + &format!("{JWT_401}{RATE_LIMITED_THEN_IRMA_503}"), + &format!( + "{}{RATE_LIMITED_THEN_IRMA_503}", + JWT_401.replacen("\"401\":", "\"403\":", 1) + ), + ) +} + +fn remove_a_non_success_status(spec: &str) -> String { + once( + spec, + r##" "401": + description: "Unknown, expired or revoked key." + content: + application/json: + schema: + $ref: "#/components/schemas/Error" +"##, + "", + ) +} + +fn remove_a_response_enum_value(spec: &str) -> String { + once( + spec, + "enum: [INITIALIZED, PAIRING, CONNECTED, CANCELLED, DONE, TIMEOUT]", + "enum: [INITIALIZED, PAIRING, CONNECTED, CANCELLED, DONE]", + ) +} + +/// `key` is the IBE user secret key: the payload the endpoint exists to return, +/// and optional only because it is absent until the session is `DONE`. +fn remove_optional_response_property(spec: &str) -> String { + once( + spec, + r##" key: + description: | + Opaque pg-core-serialized IBE user secret key. Present only when + `status` is `DONE` and `proofStatus` is `VALID`. +"##, + "", + ) +} + +fn rename_optional_response_property(spec: &str) -> String { + once( + spec, + " proofStatus:\n $ref: \"#/components/schemas/ProofStatus\"\n key:\n", + " proof_status:\n $ref: \"#/components/schemas/ProofStatus\"\n key:\n", + ) +} + +fn remove_required_response_property(spec: &str) -> String { + once( + spec, + r##" KeyResponse: + type: object + required: [status] + properties: + status: + $ref: "#/components/schemas/SessionStatus" +"##, + " KeyResponse:\n type: object\n properties:\n", + ) +} + +fn narrow_a_parameter_type(spec: &str) -> String { + once( + spec, + TIMESTAMP_PARAMETER, + &TIMESTAMP_PARAMETER.replacen("format: int64", "format: int32", 1), + ) +} + +fn remove_a_request_parameter(spec: &str) -> String { + once( + spec, + &format!(" parameters:\n{TIMESTAMP_PARAMETER}"), + "", + ) +} + +type Mutation = (&'static str, fn(&str) -> String, Gate); + +fn mutations() -> Vec { + vec![ + ("a new endpoint", add_endpoint, Gate::Passes), + ( + "a new optional response property", + add_optional_response_property, + Gate::Passes, + ), + ( + "a new optional request property", + add_optional_request_property, + Gate::Passes, + ), + ( + "a new required response property", + add_required_response_property, + Gate::Passes, + ), + ( + "a new optional query parameter", + add_optional_query_parameter, + Gate::Passes, + ), + ("a new response status", add_response_status, Gate::Passes), + ("an edited description", edit_a_description, Gate::Passes), + ( + "a /v3 route beside /v2", + add_v3_route_beside_v2, + Gate::Passes, + ), + ("a removed route", remove_route, Gate::Stops), + ( + "a request property becoming required", + request_property_becomes_required, + Gate::Stops, + ), + ("a changed status code", change_a_status_code, Gate::Stops), + ( + "a removed non-success status", + remove_a_non_success_status, + Gate::Stops, + ), + ( + "a removed response enum value", + remove_a_response_enum_value, + Gate::Stops, + ), + ( + "a removed optional response property", + remove_optional_response_property, + Gate::Stops, + ), + ( + "a renamed optional response property", + rename_optional_response_property, + Gate::Stops, + ), + ( + "a removed required response property", + remove_required_response_property, + Gate::Stops, + ), + ( + "a narrowed parameter type", + narrow_a_parameter_type, + Gate::Stops, + ), + ( + "a removed request parameter", + remove_a_request_parameter, + Gate::Stops, + ), + ] +} + +/// Every mutation must still edit the spec, whether or not oasdiff is +/// installed, so a spec edit that strands an anchor is caught in CI too. +#[test] +fn every_mutation_still_applies() { + let spec = fs::read_to_string(spec_path()).expect("read the spec"); + for (name, mutate, _) in mutations() { + assert_ne!( + mutate(&spec), + spec, + "the mutation for {name} changed nothing, so whatever it asserts is vacuous" + ); + } +} + +#[test] +fn the_gate_stops_breaking_changes_and_passes_additive_ones() { + let Some(oasdiff) = oasdiff() else { + eprintln!( + "skipping: oasdiff is not installed. This is the case in CI, where the gate itself \ + runs in the API diff workflow. To run this test, `go install \ + github.com/oasdiff/oasdiff@v1.26.1` (the version the action pins), or set OASDIFF." + ); + return; + }; + + let spec = fs::read_to_string(spec_path()).expect("read the spec"); + let dir = env::temp_dir().join(format!("pg-pkg-api-gate-{}", std::process::id())); + fs::create_dir_all(&dir).expect("create the scratch directory"); + + // The unmutated spec first: without this, a gate that stopped everything + // would satisfy every Stops case below and only look half broken. + let unchanged = dir.join("unchanged.yaml"); + fs::write(&unchanged, &spec).expect("write the spec"); + let baseline = gate(&oasdiff, &unchanged); + + let mut wrong = Vec::new(); + if baseline != Gate::Passes { + wrong.push(" no change at all: the gate should pass it, it stopped it".to_owned()); + } + for (name, mutate, expected) in mutations() { + let slug: String = name + .chars() + .map(|c| if c.is_ascii_alphanumeric() { c } else { '-' }) + .collect(); + let revision = dir.join(format!("{slug}.yaml")); + fs::write(&revision, mutate(&spec)).expect("write the mutated spec"); + let actual = gate(&oasdiff, &revision); + if actual != expected { + wrong.push(format!( + " {name}: the gate should {} it, it {} it", + expected.verb(), + actual.past() + )); + } + } + + fs::remove_dir_all(&dir).ok(); + assert!( + wrong.is_empty(), + "the gate's verdict on {} of {} changes is not what fail-on={FAIL_ON} and \ + include-checks={INCLUDE_CHECKS} are supposed to deliver:\n{}", + wrong.len(), + mutations().len() + 1, + wrong.join("\n"), + ); +} From ffc5bd4d08b4449c78d513e47a6871ed4f4b3e36 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:04:02 +0000 Subject: [PATCH 03/10] docs: say the api-diff workflow is not committed yet in the test header The test's module docs described the gate in the present tense, which is the same overpromise the reviewed docs had: the workflow YAML is still in a PR comment waiting for workflows: write. CLAUDE.md and COMPATIBILITY.md already say so; this file now does too. Co-Authored-By: Claude Opus 5 --- pg-pkg/tests/api_gate.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/pg-pkg/tests/api_gate.rs b/pg-pkg/tests/api_gate.rs index ce3ae387..75723b8b 100644 --- a/pg-pkg/tests/api_gate.rs +++ b/pg-pkg/tests/api_gate.rs @@ -1,13 +1,18 @@ //! What the API breaking-change gate actually catches (issue #249). //! -//! `.github/workflows/api-diff.yml` runs `oasdiff breaking` over -//! `api-description.yaml`, and it is what stops a careless edit from breaking a -//! deployed client. Its verdict is decided entirely by two step inputs, -//! `fail-on` and `include-checks`, and getting them wrong fails open: the job -//! goes green and nobody learns that the change it was supposed to stop went -//! through. `fail-on: ERR` alone passes a removed optional response property, a -//! removed request parameter, a changed status code and a dropped response enum -//! value, all of which COMPATIBILITY.md forbids. +//! The gate is `.github/workflows/api-diff.yml`, which runs `oasdiff breaking` +//! over `api-description.yaml` and is what stops a careless edit from breaking a +//! deployed client. It is not committed yet: the bot that wrote it has no +//! `workflows: write`, so the YAML waits in a comment on PR #269 for a +//! maintainer to apply. This test is the half that could be committed, and it +//! stands on its own either way. +//! +//! The gate's verdict is decided entirely by two step inputs, `fail-on` and +//! `include-checks`, and getting them wrong fails open: the job goes green and +//! nobody learns that the change it was supposed to stop went through. +//! `fail-on: ERR` alone passes a removed optional response property, a removed +//! request parameter, a changed status code and a dropped response enum value, +//! all of which COMPATIBILITY.md forbids. //! //! So the inputs are pinned here rather than only in the workflow: this test //! mutates the real spec, runs the real engine with the real flags, and asserts @@ -471,9 +476,9 @@ fn every_mutation_still_applies() { fn the_gate_stops_breaking_changes_and_passes_additive_ones() { let Some(oasdiff) = oasdiff() else { eprintln!( - "skipping: oasdiff is not installed. This is the case in CI, where the gate itself \ - runs in the API diff workflow. To run this test, `go install \ - github.com/oasdiff/oasdiff@v1.26.1` (the version the action pins), or set OASDIFF." + "skipping: oasdiff is not installed, which is the case on every runner. To run this \ + test, `go install github.com/oasdiff/oasdiff@v1.26.1` (the version the action pins), \ + or set OASDIFF." ); return; }; From f0edf4439c3b466ba5f37b7a01b0b45bd10c49a5 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Tue, 28 Jul 2026 12:21:53 +0000 Subject: [PATCH 04/10] test(pg-pkg): pin that the gate stops a new response enum value COMPATIBILITY.md gained "adding a value to a response enum" for this gate, and it was the one rule api_gate.rs did not cover. It holds only at WARN (measured on oasdiff v1.26.1: exit 0 at ERR+include-checks, exit 1 at WARN+include-checks), so a revert to fail-on: ERR dropped it with the test still green -- exactly the drift the test exists to catch. At ERR the test now reports four wrong verdicts instead of three, the fourth being this one. --- CLAUDE.md | 2 +- pg-pkg/tests/api_gate.rs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 870d7e52..bf192484 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,7 +9,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you and `pg-pkg/tests/api_gate.rs`. That test is the executable record of what the gate stops; it mutates the spec 18 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you and `pg-pkg/tests/api_gate.rs`. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. - The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. diff --git a/pg-pkg/tests/api_gate.rs b/pg-pkg/tests/api_gate.rs index 75723b8b..74d75fad 100644 --- a/pg-pkg/tests/api_gate.rs +++ b/pg-pkg/tests/api_gate.rs @@ -329,6 +329,20 @@ fn remove_a_response_enum_value(spec: &str) -> String { ) } +/// The rule COMPATIBILITY.md gained for this gate: the one WARN check that fires +/// on a change the document did not already forbid, kept rather than suppressed +/// because a client switching on `status` with no default branch breaks on a +/// value it has never seen. Pinned here because it is the newest rule and the +/// most fragile: it holds only at WARN, so a revert to `fail-on: ERR` drops it, +/// and without this case the test would stay green while it did. +fn add_a_response_enum_value(spec: &str) -> String { + once( + spec, + "enum: [INITIALIZED, PAIRING, CONNECTED, CANCELLED, DONE, TIMEOUT]", + "enum: [INITIALIZED, PAIRING, CONNECTED, CANCELLED, DONE, TIMEOUT, EXPIRED]", + ) +} + /// `key` is the IBE user secret key: the payload the endpoint exists to return, /// and optional only because it is absent until the session is `DONE`. fn remove_optional_response_property(spec: &str) -> String { @@ -430,6 +444,11 @@ fn mutations() -> Vec { remove_a_response_enum_value, Gate::Stops, ), + ( + "a new response enum value", + add_a_response_enum_value, + Gate::Stops, + ), ( "a removed optional response property", remove_optional_response_property, From 8f26cf7fb14b6dcbccdb15ee3da51221a8995d1f Mon Sep 17 00:00:00 2001 From: Ruben Hensen Date: Tue, 28 Jul 2026 23:19:29 +0200 Subject: [PATCH 05/10] ci: run the pg-pkg OpenAPI breaking-change gate Dobby's YAML from the #269 comment, applied verbatim (the App cannot push workflows). Landing it here rather than on main so the gate self-tests on this PR, and tightening the two doc lines that described it as not-yet-live. --- .github/workflows/api-diff.yml | 107 +++++++++++++++++++++++++++++++++ CLAUDE.md | 2 +- COMPATIBILITY.md | 2 +- 3 files changed, 109 insertions(+), 2 deletions(-) create mode 100644 .github/workflows/api-diff.yml diff --git a/.github/workflows/api-diff.yml b/.github/workflows/api-diff.yml new file mode 100644 index 00000000..7652d30e --- /dev/null +++ b/.github/workflows/api-diff.yml @@ -0,0 +1,107 @@ +name: API diff +# +# Breaking-change gate on the pg-pkg OpenAPI contract (#249). +# +# pg-pkg/api-description.yaml is the pinned v2 HTTP contract (#242), one of the +# three seams COMPATIBILITY.md guarantees. This job diffs the PR's spec against +# the branch the PR targets and fails on any change oasdiff rates WARN or ERR. +# +# Escape hatch, not a wall: pg-pkg's routes are versioned (unlike cryptify's, +# whose gate this mirrors), so a change /v2 cannot take additively goes under a +# new prefix (/v3/...) with /v2 left running, and /v2 is retired later through +# the deprecation process in COMPATIBILITY.md, once postguard-ops#64 telemetry +# shows nobody calls it. A /v3 route added next to /v2 reads as additive, so +# this gate passes it. Reach for that before reaching for err-ignore. +# +# Why WARN and not ERR +# -------------------- +# `fail-on: ERR` would leave four of the changes COMPATIBILITY.md forbids +# passing silently, because oasdiff rates them WARN, and on this spec that is +# most of the "no removing a route or a field" rule: only `status` is ever +# `required`, so `key` (the IBE user secret key the endpoint exists to return), +# `proofStatus`, `pubSignKey` and the rest are optional, and removing or +# renaming an optional response property is WARN. So is removing a request +# parameter. Measured on this spec against oasdiff v1.26.1: +# +# mutation fail-on ERR fail-on WARN +# optional response property removed (`key`) passes fails +# optional response property renamed passes fails +# required path parameter removed passes fails +# response enum value added passes fails +# +# The fourth is the rule COMPATIBILITY.md gains alongside this gate; see below. +# +# WARN adds 31 checks on top of ERR's 213. All but one are changes +# COMPATIBILITY.md already forbids (request-parameter-removed, +# request-property-removed, response-body-media-type-schema-removed, and the +# constraint-narrowing *-set family). The exception is +# response-property-enum-value-added: adding a value to SessionStatus or +# ProofStatus fails this gate. That is deliberate, and COMPATIBILITY.md now +# says so too. A client that switches on the enum without a default branch +# breaks on a value it has never seen, so a new status is a /v3 change (or an +# x-extensible-enum one), not an additive one. It is the one rule here that +# only WARN enforces, so it is also the first casualty of a revert to ERR; +# pg-pkg/tests/api_gate.rs pins it. +# +# Two more checks are opt-in: they rate ERR but only run when named, so they +# need the include-checks input below. Without it, changing a 401 to a 403 and +# dropping an enum value from a response both pass. Keep include-checks and +# fail-on in step with the local-repro command in CLAUDE.md, or a local run +# quietly disagrees with CI. +# +# Not covered: the gate compares documented paths, and the spec documents the +# canonical /v2/request/... paths only (see its "Path prefix aliases" note), so +# dropping the /v2/irma/... alias handlers before #257's deprecation has run +# passes this gate. That one stays a review rule. +# +# There is deliberately no `on: paths:` filter. A path-filtered job reports no +# status on the PRs it skips, so as a required check it would leave every PR +# that does not touch the spec pending forever. The job is two checkouts and one +# container, so it just always runs. +# + +on: + pull_request: + # `edited` included for base retargets (e.g. a stacked PR's base merging): + # this job's verdict depends on the base sha, and without `edited` a stale + # verdict stays attached to the unchanged head sha. + types: [opened, synchronize, reopened, edited] + +permissions: + contents: read + +jobs: + + breaking-changes: + name: API breaking changes (oasdiff) + runs-on: ubuntu-latest + steps: + - name: Check out the pull request + uses: actions/checkout@v6 + with: + # The spec is all this job reads, and the oasdiff container gets the + # workspace mounted; do not leave a push token in .git/config for it. + persist-credentials: false + - name: Check out the base spec + uses: actions/checkout@v6 + with: + ref: ${{ github.event.pull_request.base.sha }} + path: base + persist-credentials: false + - name: Diff the spec against the base branch + # Pinned by sha because this step is the gate's verdict. The tag pins + # the engine: v0.1.10 is FROM tufin/oasdiff:v1.26.1, so a local + # `oasdiff v1.26.1` reproduces what CI decides here. + uses: oasdiff/oasdiff-action/breaking@0ab8ad204b00d25acc5ae87106281433e288d0c1 # v0.1.10 + with: + base: base/pg-pkg/api-description.yaml + revision: pg-pkg/api-description.yaml + fail-on: WARN + # Both of these rate ERR but are opt-in, so they do not run unless + # named: a changed non-success status (401 -> 403) and an enum value + # dropped from a response property. + include-checks: response-non-success-status-removed,response-property-enum-value-removed + # Do not upload the two specs to oasdiff.com for a side-by-side + # review page. The default is `true`; the detection and the inline + # annotations work without it, so nothing leaves CI. + review: false diff --git a/CLAUDE.md b/CLAUDE.md index bf192484..0648337a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -9,7 +9,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate (`.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`) is **not** a CI gate yet: the YAML sits in a comment on PR #269 waiting for a maintainer to apply it, so until then nothing checks a spec change but you and `pg-pkg/tests/api_gate.rs`. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. - The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. diff --git a/COMPATIBILITY.md b/COMPATIBILITY.md index 9ceb839f..7167d0d5 100644 --- a/COMPATIBILITY.md +++ b/COMPATIBILITY.md @@ -28,7 +28,7 @@ removed once the deprecation process at the bottom of this file has run for it ([#257]). Until then it keeps working, so a deployed client on `/v2/irma/...` is not broken by this notice. -Enforcement is written but not running yet: the `API breaking changes (oasdiff)` +Enforcement: the `API breaking changes (oasdiff)` job diffs the spec against the branch a PR targets and fails on any change oasdiff rates WARN or ERR ([#249]). That covers every rule above except one. The gate compares documented paths, and this spec documents the canonical From 44d6379e3222ba5ba296fe7be00c69c74d3d4209 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:33:09 +0000 Subject: [PATCH 06/10] test(pg-pkg): pin the workflow's oasdiff inputs against the test's constants api_gate.rs's header named a hole that only existed while the YAML was uncommitted: FAIL_ON and INCLUDE_CHECKS are what the test runs oasdiff with, the workflow step is what CI runs it with, and nothing compared the two. The verdict test skips on every runner, so a `fail-on` edited down to ERR in the workflow alone lands green with the whole suite passing. The new test reads both values back out of the YAML and needs no engine, so it runs in CI. Also drops the rest of the header's not-committed-yet wording (the merge before this one did the same for COMPATIBILITY.md), and records in CLAUDE.md that the remote's workflow-path rejection is per commit diff, so merging a main that changed build.yml is pushable. --- CLAUDE.md | 4 +-- pg-pkg/tests/api_gate.rs | 76 +++++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2e5d1a31..a35cda4d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -11,9 +11,9 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict, and it skips when `oasdiff` is not on `PATH`, which is the case in CI. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict. The verdict test skips when `oasdiff` is not on `PATH`, which is the case in CI; the other two run everywhere, one checking that every mutation still finds its anchor and one reading `fail-on`/`include-checks` back out of the workflow YAML, so editing the step and the constants apart fails the suite instead of quietly weakening the gate. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. - The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. -- The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. +- The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. What the remote rejects is a commit whose own diff touches those paths, so `git merge origin/main` is fine even when main's side of the merge changes `build.yml` — a conflicted branch can still be synced without a maintainer. ## Dependencies diff --git a/pg-pkg/tests/api_gate.rs b/pg-pkg/tests/api_gate.rs index 74d75fad..15bec123 100644 --- a/pg-pkg/tests/api_gate.rs +++ b/pg-pkg/tests/api_gate.rs @@ -2,10 +2,7 @@ //! //! The gate is `.github/workflows/api-diff.yml`, which runs `oasdiff breaking` //! over `api-description.yaml` and is what stops a careless edit from breaking a -//! deployed client. It is not committed yet: the bot that wrote it has no -//! `workflows: write`, so the YAML waits in a comment on PR #269 for a -//! maintainer to apply. This test is the half that could be committed, and it -//! stands on its own either way. +//! deployed client. //! //! The gate's verdict is decided entirely by two step inputs, `fail-on` and //! `include-checks`, and getting them wrong fails open: the job goes green and @@ -14,15 +11,15 @@ //! request parameter, a changed status code and a dropped response enum value, //! all of which COMPATIBILITY.md forbids. //! -//! So the inputs are pinned here rather than only in the workflow: this test -//! mutates the real spec, runs the real engine with the real flags, and asserts -//! which mutations the gate stops. Change [`FAIL_ON`] or [`INCLUDE_CHECKS`] -//! without changing the workflow to match and the two drift apart with nothing -//! to say so; change the workflow without running this and the gate weakens. +//! So the inputs are pinned here as well as in the workflow: this test mutates +//! the real spec, runs the real engine with the real flags, and asserts which +//! mutations the gate stops. [`the_workflow_step_matches_the_pinned_inputs`] +//! reads the two values back out of the YAML, so editing one side without the +//! other fails here instead of silently weakening the gate. //! -//! The engine is not vendored, so the test needs `oasdiff` on `PATH` (or -//! `OASDIFF` pointing at it) and skips when it is absent, which is the case in -//! CI. Install the version the action pins, so a local verdict is CI's verdict: +//! The engine is not vendored, so the verdict test needs `oasdiff` on `PATH` +//! (or `OASDIFF` pointing at it) and skips when it is absent, which is the case +//! in CI. Install the version the action pins, so a local verdict is CI's: //! //! ```text //! go install github.com/oasdiff/oasdiff@v1.26.1 @@ -74,6 +71,35 @@ fn spec_path() -> PathBuf { PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("api-description.yaml") } +fn workflow_path() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join(".github/workflows/api-diff.yml") +} + +/// The value of the `key:` mapping entry in the workflow, with the surrounding +/// comment lines ignored. Requires exactly one such entry, so a second oasdiff +/// step (or an input moved into a matrix) fails the test rather than having one +/// of the two verdicts silently go unchecked. +fn workflow_input(workflow: &str, key: &str) -> String { + let needle = format!("{key}:"); + let values: Vec<&str> = workflow + .lines() + .map(str::trim) + .filter(|line| !line.starts_with('#')) + .filter_map(|line| line.strip_prefix(&needle)) + .map(str::trim) + .collect(); + assert_eq!( + values.len(), + 1, + "expected exactly one `{key}:` in {}, found {}", + workflow_path().display(), + values.len() + ); + values[0].to_owned() +} + /// The `oasdiff` binary, or `None` when it is not installed. fn oasdiff() -> Option { if let Some(explicit) = env::var_os("OASDIFF") { @@ -477,6 +503,32 @@ fn mutations() -> Vec { ] } +/// The verdicts below are only CI's verdicts if CI runs the engine with these +/// flags, and nothing else compares the two: `the_gate_stops_...` skips on every +/// runner, so a `fail-on` edited down to `ERR` in the workflow alone would land +/// green. This test needs no engine, so it runs in CI, where it is the only +/// thing standing between the workflow and the promises this file makes about +/// it. +#[test] +fn the_workflow_step_matches_the_pinned_inputs() { + let path = workflow_path(); + let workflow = + fs::read_to_string(&path).unwrap_or_else(|e| panic!("read {}: {e}", path.display())); + + assert_eq!( + workflow_input(&workflow, "fail-on"), + FAIL_ON, + "the workflow's oasdiff step and FAIL_ON disagree, so the verdicts this \ + test pins are not the ones CI reaches" + ); + assert_eq!( + workflow_input(&workflow, "include-checks"), + INCLUDE_CHECKS, + "the workflow's oasdiff step and INCLUDE_CHECKS disagree, so the \ + verdicts this test pins are not the ones CI reaches" + ); +} + /// Every mutation must still edit the spec, whether or not oasdiff is /// installed, so a spec edit that strands an anchor is caught in CI too. #[test] From e86c71baa69a2dec089168cb4e807c7edf424f7c Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:44:55 +0000 Subject: [PATCH 07/10] docs: restore the two CLAUDE.md bullets on the oasdiff gate The merge in 94e6ed5 hit a conflict where main's two new semver bullets and this branch's two oasdiff bullets both landed under the Docker one, and took main's side whole, so the gate this PR adds ended up documented nowhere in CLAUDE.md. Put them back in place, unchanged, alongside the semver pair. Also notes on the workflow-push bullet that the App did push a merge carrying main's build.yml changes (ce0fc59 on this branch), so a sync is worth trying before treating it as maintainer-only. --- CLAUDE.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index f706ca72..1fa70489 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,7 +13,9 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - `scripts/semver-checks.sh` runs `cargo-semver-checks` over the two surfaces external consumers build against: `pg-core` against its crates.io release, and `pg-wasm` against `origin/main` (it has no crates.io release; the npm package is versioned from `pg-core`). The `semver-checks` job in `build.yml` calls it on any PR touching `pg-core`, `pg-wasm`, the root manifest or the script itself; run it yourself too before pushing such a change, since the job needs a wasm32 toolchain and a pinned cargo-semver-checks download and is therefore not the fastest feedback. Four things it encodes. (1) `pg-core` needs `--only-explicit-features --features test,rust,stream`, the same set the test and clippy matrices use: cargo-semver-checks otherwise enables everything that doesn't look unstable, which pulls in `web` and hits its `compile_error!`. (2) `pg-core`'s `web,stream` surface is deliberately not checked. `Unsealer` has two `unseal` methods there on different instantiations (owned `self` in `client/web/mod.rs`, `&mut self` in `client/web/stream.rs`) and cargo-semver-checks 0.49 pairs them by name alone, so it reports `method_receiver_mut_ref_became_owned` against byte-identical source; `rust,stream` is clean because both receivers are owned there. (3) Any wasm32 run needs `RUSTFLAGS=--cap-lints=warn`, because the `--cap-lints allow` cargo-semver-checks sets silences the "dropping unsupported crate type" warnings cargo reads back when probing rustc, and cargo then dies with "output of --print=file-names missing". (4) `cargo-semver-checks` splits its non-zero exits: `100` is a semver violation, `101` is the tool or the build failing (unresolvable baseline rev, missing rustup target, registry fetch failure, compile error in the crate). Never treat "non-zero" as "breaking change" here, because the advice a semver gate prints is "declare the break", and on this repo that means a `!` in the PR title and a spurious major release of `pg-core`. `scripts/semver-checks-test.sh` pins that mapping; it stubs `cargo`, so it runs in well under a second and needs neither cargo-semver-checks nor a wasm32 toolchain. Run it after touching the gate. - release-plz owns the version numbers, so the PR making a breaking change cannot bump the crate to match (bumping `pg-core` alone doesn't even resolve: `pg-cli` requires `^0.6.1`). What the semver gate accepts as the declaration is the conventional-commit `!` in the PR title, and only that; CI turns it into `SEMVER_RELEASE_TYPE=major`, which the script passes as `--release-type major`. A `BREAKING CHANGE:` footer in the PR body is not accepted and must not be: this repo's `squash_merge_commit_message` is `COMMIT_MESSAGES`, so the body never reaches the squashed commit, and release-plz reading a bare `fix(pg-core):` subject would cut a patch release of a break the gate had already waved through. Two consequences of the merge settings worth knowing when you declare a break. `squash_merge_commit_title` is `COMMIT_OR_PR_TITLE`, which is the PR title on a multi-commit PR but the commit's subject when the PR has exactly one commit — so on a single-commit PR put the `!` in the commit subject too, or the gate goes green off the PR title while release-plz cuts a patch. And `--release-type major` doesn't merely permit a bigger bump: every lint exists to demand a bump the declaration already grants, so all of them skip and the run checks nothing (`0 checks: 0 pass, 253 skip`) on both surfaces at once. A green gate on a `!` PR verified nothing; a `!` added for a pg-wasm break also passes any unrelated pg-core break in the same PR. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. The block covers *merge* commits too, which is easy to miss: once a branch carries its own `build.yml` change (typically a maintainer applying such a patch onto it), a later `git merge origin/main` that has to touch `build.yml` produces a commit updating a workflow file, and the push is rejected even when the resolution is only "keep both new jobs". Nothing can be split out of a merge commit, so that merge has to be landed by a maintainer, or the App needs `workflows: write`. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict. The verdict test skips when `oasdiff` is not on `PATH`, which is the case in CI; the other two run everywhere, one checking that every mutation still finds its anchor and one reading `fail-on`/`include-checks` back out of the workflow YAML, so editing the step and the constants apart fails the suite instead of quietly weakening the gate. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. +- The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. The block covers *merge* commits too, which is easy to miss: once a branch carries its own `build.yml` change (typically a maintainer applying such a patch onto it), a later `git merge origin/main` that has to touch `build.yml` produces a commit updating a workflow file, and the push is rejected even when the resolution is only "keep both new jobs". Nothing can be split out of a merge commit, so that merge has to be landed by a maintainer, or the App needs `workflows: write`. Measured exception, worth trying before handing the sync over: the App pushed `ce0fc59` on this branch, a merge whose diff against its first parent added main's 64 new `build.yml` lines. That merge needed no resolution inside `build.yml` — it took main's side whole, so the blob it committed already existed in the repo. Try the merge and read the remote's answer; only escalate on an actual rejection. ## Dependencies From 6aedefd39d2e4c857921edc10909b1b739e66d97 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 07:54:00 +0000 Subject: [PATCH 08/10] docs: record the wasm browser-test flake and what its error actually means Hit on this branch: the same docs-only sha passed all three browser jobs at 07:45 and failed at 07:48. The visible error, `missing field 'chunk'`, is the test runner choking on a truncated webdriver reply, so it reads like a harness version mismatch; the actual cause is the renderer timeout logged one line above it. Also notes that only chrome failed and the other two were cancelled by the fail-fast matrix but reported as failures, and that the App cannot re-run a workflow. --- CLAUDE.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CLAUDE.md b/CLAUDE.md index 1fa70489..fd1a39a5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,6 +10,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - JS-reader gotchas the Node gate ran into, all still true of `@e4a/pg-wasm` 0.6.1 and `@e4a/pg-js` 1.11.0/2.3.3: (a) `@e4a/pg-wasm`'s default (bundler) entry does `import * as wasm from "./index_bg.wasm"`, which plain Node cannot resolve — import `@e4a/pg-wasm/web` and pass the module bytes to its default export, resolving them via `new URL('index_bg.wasm', import.meta.resolve('@e4a/pg-wasm/web'))` since the `.wasm` file is not in the package's `exports`; (b) `Unsealer.unseal()`/`StreamUnsealer.unseal()` *consume* the unsealer (wasm-bindgen `__destroy_into_raw`), so a tidy `free()` afterwards is a double free reported as `null pointer passed to rust`, which reads exactly like a corrupt container; (c) `pg-js` is stream-mode only in both directions — `toBytes()` seals with `sealStream` and its decrypt path only ever builds a `StreamUnsealer`, so a memory-mode container fails with `mode is not supported: InMemory { size: N }`; (d) `pg-js` 1.x discards what `StreamUnsealer.unseal()` returns and reports `public_identity()` instead, so it never surfaces the private signing policy of a `*-privsig` container and its `sender.raw` is the bare header policy rather than 2.x's `{public, private?}`; (e) `pg-js`'s decrypt path needs a PKG at `pkgUrl` for exactly two GETs — `/v2/sign/parameters` for the verifying key and `/v2/irma/key/` (bearer token) for the USK — which is why the gate can drive it offline from the artifact's own `vk.json`/`usk-*.json`. - Appending a field at the *end* of `Header` really is additive: the header is a length-prefixed region and `bincode` ignores trailing bytes, so published `pg-core` 0.6.1 still opens it. A field inserted anywhere else, a changed field type, or a reorder shifts every following byte and the containers stop opening, but *not* with a decode error: 0.6.1 reads a garbage length prefix, attempts a ~20 GiB allocation, and the process aborts (SIGABRT). Expect `reader died on signal 6 ... memory allocation of N bytes failed`, not a message naming the header. This is also why `pg-compat` opens each case in a child process (its `pg-compat-case` binary): an abort is not a panic, `catch_unwind` cannot contain it, and in one process the first broken case would take the run down before the others were tried. Don't reason about "additive" from the struct alone; run the compat gate. - CI's `Format workspace` matrix runs `cargo fmt --manifest-path pg-/Cargo.toml --all -- --check` per crate over shared workspace files; always run `cargo fmt --all -- --check` from repo root before pushing, or one crate's drift fails the whole matrix. +- `Run wasm tests in browsers` flakes, and the error names the wrong culprit. `Error: missing field 'chunk'` is `wasm-bindgen-test-runner` failing to parse a truncated webdriver reply; the cause is the line above it, `[SEVERE]: Timed out receiving message from renderer: 30.000`. Read the driver stderr before suspecting the test. The matrix is fail-fast, so one browser timing out reports the other two as failures when they were cancelled: check each job's own conclusion, not the summary. Seen on the same sha passing at 07:45 and failing at 07:48 (runs 30432815599 and 30432995207 on #269, a docs-only commit). Re-run rather than debug, and note that `dobby-coder` cannot: `POST /actions/runs/{id}/rerun-failed-jobs` is 403 for the App, so a maintainer has to click it, or a fresh push has to supersede the run. - `scripts/semver-checks.sh` runs `cargo-semver-checks` over the two surfaces external consumers build against: `pg-core` against its crates.io release, and `pg-wasm` against `origin/main` (it has no crates.io release; the npm package is versioned from `pg-core`). The `semver-checks` job in `build.yml` calls it on any PR touching `pg-core`, `pg-wasm`, the root manifest or the script itself; run it yourself too before pushing such a change, since the job needs a wasm32 toolchain and a pinned cargo-semver-checks download and is therefore not the fastest feedback. Four things it encodes. (1) `pg-core` needs `--only-explicit-features --features test,rust,stream`, the same set the test and clippy matrices use: cargo-semver-checks otherwise enables everything that doesn't look unstable, which pulls in `web` and hits its `compile_error!`. (2) `pg-core`'s `web,stream` surface is deliberately not checked. `Unsealer` has two `unseal` methods there on different instantiations (owned `self` in `client/web/mod.rs`, `&mut self` in `client/web/stream.rs`) and cargo-semver-checks 0.49 pairs them by name alone, so it reports `method_receiver_mut_ref_became_owned` against byte-identical source; `rust,stream` is clean because both receivers are owned there. (3) Any wasm32 run needs `RUSTFLAGS=--cap-lints=warn`, because the `--cap-lints allow` cargo-semver-checks sets silences the "dropping unsupported crate type" warnings cargo reads back when probing rustc, and cargo then dies with "output of --print=file-names missing". (4) `cargo-semver-checks` splits its non-zero exits: `100` is a semver violation, `101` is the tool or the build failing (unresolvable baseline rev, missing rustup target, registry fetch failure, compile error in the crate). Never treat "non-zero" as "breaking change" here, because the advice a semver gate prints is "declare the break", and on this repo that means a `!` in the PR title and a spurious major release of `pg-core`. `scripts/semver-checks-test.sh` pins that mapping; it stubs `cargo`, so it runs in well under a second and needs neither cargo-semver-checks nor a wasm32 toolchain. Run it after touching the gate. - release-plz owns the version numbers, so the PR making a breaking change cannot bump the crate to match (bumping `pg-core` alone doesn't even resolve: `pg-cli` requires `^0.6.1`). What the semver gate accepts as the declaration is the conventional-commit `!` in the PR title, and only that; CI turns it into `SEMVER_RELEASE_TYPE=major`, which the script passes as `--release-type major`. A `BREAKING CHANGE:` footer in the PR body is not accepted and must not be: this repo's `squash_merge_commit_message` is `COMMIT_MESSAGES`, so the body never reaches the squashed commit, and release-plz reading a bare `fix(pg-core):` subject would cut a patch release of a break the gate had already waved through. Two consequences of the merge settings worth knowing when you declare a break. `squash_merge_commit_title` is `COMMIT_OR_PR_TITLE`, which is the PR title on a multi-commit PR but the commit's subject when the PR has exactly one commit — so on a single-commit PR put the `!` in the commit subject too, or the gate goes green off the PR title while release-plz cuts a patch. And `--release-type major` doesn't merely permit a bigger bump: every lint exists to demand a bump the declaration already grants, so all of them skip and the run checks nothing (`0 checks: 0 pass, 253 skip`) on both surfaces at once. A green gate on a `!` PR verified nothing; a `!` added for a pg-wasm break also passes any unrelated pg-core break in the same PR. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. From 4334f9adaf048033ec4b863d44f074bb37180868 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:19:14 +0000 Subject: [PATCH 09/10] test(pg-pkg): pin the oasdiff action ref alongside the gate's inputs `the_workflow_step_matches_the_pinned_inputs` read `fail-on` and `include-checks` back out of the workflow but not the action ref, which is the third thing the pinned verdicts depend on: `breaking@0ab8ad2` is v0.1.10, whose Dockerfile is `FROM tufin/oasdiff:v1.26.1`, and every verdict in this file was measured against v1.26.1. So bumping the action left all three tests green while the verdicts quietly stopped describing CI -- the same fail-open the `fail-on` pin exists to prevent, and the file's header claims its verdict is CI's verdict. Assert the workflow contains the ref too. Verified by bumping the ref in the workflow: the test fails and names it. --- CLAUDE.md | 2 +- pg-pkg/tests/api_gate.rs | 25 ++++++++++++++++++++----- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index fd1a39a5..d51a2f83 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -14,7 +14,7 @@ Migrated from the dobby memory repo (`encryption4all/dobby`). This file is the s - `scripts/semver-checks.sh` runs `cargo-semver-checks` over the two surfaces external consumers build against: `pg-core` against its crates.io release, and `pg-wasm` against `origin/main` (it has no crates.io release; the npm package is versioned from `pg-core`). The `semver-checks` job in `build.yml` calls it on any PR touching `pg-core`, `pg-wasm`, the root manifest or the script itself; run it yourself too before pushing such a change, since the job needs a wasm32 toolchain and a pinned cargo-semver-checks download and is therefore not the fastest feedback. Four things it encodes. (1) `pg-core` needs `--only-explicit-features --features test,rust,stream`, the same set the test and clippy matrices use: cargo-semver-checks otherwise enables everything that doesn't look unstable, which pulls in `web` and hits its `compile_error!`. (2) `pg-core`'s `web,stream` surface is deliberately not checked. `Unsealer` has two `unseal` methods there on different instantiations (owned `self` in `client/web/mod.rs`, `&mut self` in `client/web/stream.rs`) and cargo-semver-checks 0.49 pairs them by name alone, so it reports `method_receiver_mut_ref_became_owned` against byte-identical source; `rust,stream` is clean because both receivers are owned there. (3) Any wasm32 run needs `RUSTFLAGS=--cap-lints=warn`, because the `--cap-lints allow` cargo-semver-checks sets silences the "dropping unsupported crate type" warnings cargo reads back when probing rustc, and cargo then dies with "output of --print=file-names missing". (4) `cargo-semver-checks` splits its non-zero exits: `100` is a semver violation, `101` is the tool or the build failing (unresolvable baseline rev, missing rustup target, registry fetch failure, compile error in the crate). Never treat "non-zero" as "breaking change" here, because the advice a semver gate prints is "declare the break", and on this repo that means a `!` in the PR title and a spurious major release of `pg-core`. `scripts/semver-checks-test.sh` pins that mapping; it stubs `cargo`, so it runs in well under a second and needs neither cargo-semver-checks nor a wasm32 toolchain. Run it after touching the gate. - release-plz owns the version numbers, so the PR making a breaking change cannot bump the crate to match (bumping `pg-core` alone doesn't even resolve: `pg-cli` requires `^0.6.1`). What the semver gate accepts as the declaration is the conventional-commit `!` in the PR title, and only that; CI turns it into `SEMVER_RELEASE_TYPE=major`, which the script passes as `--release-type major`. A `BREAKING CHANGE:` footer in the PR body is not accepted and must not be: this repo's `squash_merge_commit_message` is `COMMIT_MESSAGES`, so the body never reaches the squashed commit, and release-plz reading a bare `fix(pg-core):` subject would cut a patch release of a break the gate had already waved through. Two consequences of the merge settings worth knowing when you declare a break. `squash_merge_commit_title` is `COMMIT_OR_PR_TITLE`, which is the PR title on a multi-commit PR but the commit's subject when the PR has exactly one commit — so on a single-commit PR put the `!` in the commit subject too, or the gate goes green off the PR title while release-plz cuts a patch. And `--release-type major` doesn't merely permit a bigger bump: every lint exists to demand a bump the declaration already grants, so all of them skip and the run checks nothing (`0 checks: 0 pass, 253 skip`) on both surfaces at once. A green gate on a `!` PR verified nothing; a `!` added for a pg-wasm break also passes any unrelated pg-core break in the same PR. - The Docker build (`Dockerfile`, `FROM rust:-slim`) pins an older or different Rust than the `Test workspace`/`Format workspace` jobs' `dtolnay/rust-toolchain@stable`. A change can pass every workspace test and still fail Docker Build on a type-inference difference that doesn't reproduce on host stable (e.g. a slice-element-type unification difference across rustc versions). Check the Dockerfile's current pin, and run `cargo build --profile edge --bin pg-pkg` locally before pushing any `Cargo.toml` dependency bump; for a true repro, build the Docker image. -- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict. The verdict test skips when `oasdiff` is not on `PATH`, which is the case in CI; the other two run everywhere, one checking that every mutation still finds its anchor and one reading `fail-on`/`include-checks` back out of the workflow YAML, so editing the step and the constants apart fails the suite instead of quietly weakening the gate. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. +- `pg-pkg/api-description.yaml` is the pinned v2 HTTP contract and is additive-only (see `COMPATIBILITY.md`). Its breaking-change gate is `.github/workflows/api-diff.yml`, job `API breaking changes (oasdiff)`, which runs on every PR; `pg-pkg/tests/api_gate.rs` is its executable spec. That test is the executable record of what the gate stops; it mutates the spec 19 ways and asserts each verdict. The verdict test skips when `oasdiff` is not on `PATH`, which is the case in CI; the other two run everywhere, one checking that every mutation still finds its anchor and one reading `fail-on`/`include-checks`/the pinned action ref back out of the workflow YAML, so editing the step and the constants apart fails the suite instead of quietly weakening the gate. The action ref is pinned because it is what selects the engine version (`v0.1.10` is `FROM tufin/oasdiff:v1.26.1`), so bumping the action silently re-measures every verdict. Those anchors are multi-line raw strings matched against the spec read off disk, which is why root `.gitattributes` marks `pg-pkg/api-description.yaml -text`: rustc normalises CRLF inside raw strings but `read_to_string` does not, so without it a Windows checkout fails `every_mutation_still_applies` on a clean tree. Run it (and read it) before touching the gate's settings: `go install github.com/oasdiff/oasdiff@v1.26.1 && cargo test --manifest-path pg-pkg/Cargo.toml --all-features --test api_gate`. - The oasdiff gate's settings are **not** self-evident and `--fail-on ERR` alone fails open. `fail-on: WARN` is deliberate: oasdiff rates removing or renaming an *optional* response property, and removing a request parameter, as WARN, and this spec marks only `status` as `required`, so at ERR the gate silently passed a removed `key` (the IBE user secret key `/v2/request/key` exists to return), a renamed `proofStatus` and a dropped `timestamp` parameter. Two more, a changed non-success status (401 to 403) and a dropped response enum value, rate ERR but are **opt-in**, so they only run when named in `include-checks`. Of the 31 WARN checks, `response-property-enum-value-added` is the only one that fires on something `COMPATIBILITY.md` does not already forbid, which is why that document now names a new response enum value as non-additive too. Reproduce a verdict with the exact flags the action's entrypoint builds (`oasdiff/oasdiff-action/breaking@v0.1.10` is `FROM tufin/oasdiff:v1.26.1`, so the pinned tag is what makes a local run authoritative): `git show origin/main:pg-pkg/api-description.yaml > /tmp/base.yaml && oasdiff breaking /tmp/base.yaml pg-pkg/api-description.yaml --allow-external-refs=false --composed=false --fail-on WARN --include-checks response-non-success-status-removed,response-property-enum-value-removed`. Two traps: `oasdiff --version` prints `oasdiff version main` after a `go install` of a tag because the version comes from release ldflags (the code is still the tag), and `--fail-on` takes `ERR`/`WARN` while `oasdiff checks --severity` takes `error`/`warn`/`info`. `--severity ERR` is a usage error, so `oasdiff checks --severity ERR | wc -l` counts the help text instead and badly undercounts the tier (it is 213 error checks, 31 warn, 265 info). The spec has no external `$ref`s, so `allow-external-refs` stays at its safe (SSRF-guarding) default. The gate only sees paths the spec documents, and the spec documents canonical paths only, so dropping the `/v2/irma/...` alias handlers (#257) passes it. - The `dobby-coder` GitHub App lacks `workflows: write` on this repo; any push touching `.github/workflows/*.yml` is rejected at the remote. Before treating a fix as blocked, check whether the same effect can be achieved in a pushable file (crate manifest, source, committed script); if a fix genuinely can only live in a workflow file, ship the pushable half and hand the maintainer ready-to-paste YAML in the PR body. The block covers *merge* commits too, which is easy to miss: once a branch carries its own `build.yml` change (typically a maintainer applying such a patch onto it), a later `git merge origin/main` that has to touch `build.yml` produces a commit updating a workflow file, and the push is rejected even when the resolution is only "keep both new jobs". Nothing can be split out of a merge commit, so that merge has to be landed by a maintainer, or the App needs `workflows: write`. Measured exception, worth trying before handing the sync over: the App pushed `ce0fc59` on this branch, a merge whose diff against its first parent added main's 64 new `build.yml` lines. That merge needed no resolution inside `build.yml` — it took main's side whole, so the blob it committed already existed in the repo. Try the merge and read the remote's answer; only escalate on an actual rejection. diff --git a/pg-pkg/tests/api_gate.rs b/pg-pkg/tests/api_gate.rs index 15bec123..c36393bb 100644 --- a/pg-pkg/tests/api_gate.rs +++ b/pg-pkg/tests/api_gate.rs @@ -4,9 +4,11 @@ //! over `api-description.yaml` and is what stops a careless edit from breaking a //! deployed client. //! -//! The gate's verdict is decided entirely by two step inputs, `fail-on` and -//! `include-checks`, and getting them wrong fails open: the job goes green and -//! nobody learns that the change it was supposed to stop went through. +//! The gate's verdict is decided by two step inputs, `fail-on` and +//! `include-checks`, plus the pinned action ref that decides which engine +//! version reads them, and getting any of the three wrong fails open: the job +//! goes green and nobody learns that the change it was supposed to stop went +//! through. //! `fail-on: ERR` alone passes a removed optional response property, a removed //! request parameter, a changed status code and a dropped response enum value, //! all of which COMPATIBILITY.md forbids. @@ -14,8 +16,8 @@ //! So the inputs are pinned here as well as in the workflow: this test mutates //! the real spec, runs the real engine with the real flags, and asserts which //! mutations the gate stops. [`the_workflow_step_matches_the_pinned_inputs`] -//! reads the two values back out of the YAML, so editing one side without the -//! other fails here instead of silently weakening the gate. +//! reads all three back out of the YAML, so editing one side without the other +//! fails here instead of silently weakening the gate. //! //! The engine is not vendored, so the verdict test needs `oasdiff` on `PATH` //! (or `OASDIFF` pointing at it) and skips when it is absent, which is the case @@ -42,6 +44,13 @@ const FAIL_ON: &str = "WARN"; const INCLUDE_CHECKS: &str = "response-non-success-status-removed,response-property-enum-value-removed"; +/// The action ref in the workflow's oasdiff step. This is the third input the +/// verdicts depend on, and the least obvious: it pins the *engine*, because +/// `v0.1.10`'s Dockerfile is `FROM tufin/oasdiff:v1.26.1` and every verdict +/// below was measured against v1.26.1. A bump with no re-measurement is the +/// same fail-open `FAIL_ON` guards against, so it is pinned here too. +const ACTION_REF: &str = "oasdiff/oasdiff-action/breaking@0ab8ad204b00d25acc5ae87106281433e288d0c1"; + /// Whether the gate stops a change, i.e. whether the job goes red. #[derive(Debug, PartialEq, Eq)] enum Gate { @@ -527,6 +536,12 @@ fn the_workflow_step_matches_the_pinned_inputs() { "the workflow's oasdiff step and INCLUDE_CHECKS disagree, so the \ verdicts this test pins are not the ones CI reaches" ); + assert!( + workflow.contains(ACTION_REF), + "the workflow's oasdiff step does not use {ACTION_REF}, so CI is not \ + running the engine (tufin/oasdiff:v1.26.1) the verdicts this test pins \ + were measured against" + ); } /// Every mutation must still edit the spec, whether or not oasdiff is From 3ff6d8e55d737d47a739153e2a5a254d85de15a7 Mon Sep 17 00:00:00 2001 From: "dobby-yivi-agent[bot]" <275734547+dobby-yivi-agent[bot]@users.noreply.github.com> Date: Wed, 29 Jul 2026 08:19:14 +0000 Subject: [PATCH 10/10] build: keep api-description.yaml LF on Windows checkouts pg-pkg/tests/api_gate.rs locates blocks in the spec with multi-line raw-string anchors. rustc normalises CRLF to LF inside raw strings, but fs::read_to_string of the spec keeps whatever git wrote, so on a Windows checkout (core.autocrlf=true is the Git for Windows default) every multi-line anchor misses. Reproduced rather than argued: converting the spec to CRLF fails every_mutation_still_applies with "anchor is not unique in the spec", i.e. the one gate test that runs without the external engine, so a Windows contributor gets a red suite on a clean tree with a message blaming the spec. CI is ubuntu/macos only, so it never catches this. Mark the spec -text, which keeps stored bytes equal to checked-out bytes in both directions. --- .gitattributes | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..107e94cf --- /dev/null +++ b/.gitattributes @@ -0,0 +1,7 @@ +# pg-pkg/tests/api_gate.rs locates blocks in this spec with multi-line raw-string +# anchors. rustc normalises CRLF to LF inside raw strings, but the file read off +# disk keeps whatever git wrote, so on a Windows checkout (core.autocrlf=true is +# the Git for Windows default) every anchor misses and the suite goes red on a +# clean tree. Keeping the stored bytes is also what makes the oasdiff gate's +# verdict reproducible off a local checkout. +pg-pkg/api-description.yaml -text