diff --git a/.changeset/quiet-goats-shave.md b/.changeset/quiet-goats-shave.md new file mode 100644 index 0000000..03d31b0 --- /dev/null +++ b/.changeset/quiet-goats-shave.md @@ -0,0 +1,18 @@ +--- +'@cosyte/cli': patch +--- + +Caller workflow and branch protection only, with no runtime impact: the pre-publish check now blocks a merge here instead of merely showing a red X. + +The shared pipeline grew a `prepublish` job on 2026-08-05, so this repo started emitting a +`ci / prepublish` context on every pull request without a commit landing here. The branch ruleset +did not name it, which meant a red pre-publish result reported a failure and merged anyway. That +job is the offline manifest lint plus the pack-and-install probe, and this package is the reason +both exist: two published versions carry local-path dependency specifiers and are permanently +uninstallable. The context was read off a real check run first and then added to the ruleset, in +that order, because requiring a context nothing emits strands every pull request instead of +failing it. + +The banner on the caller now records the hazard it did not previously cover: a context can arrive +in this repo with no commit in this repo, because the `uses:` reference is unpinned, and it always +arrives unrequired. diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b4d9d13..0438cfd 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,16 +14,45 @@ permissions: contents: read jobs: - # ┌─────────────────────────────────────────────────────────────────────────────────────────┐ - # │ A REQUIRED JOB GATES ALL OF ITS STEPS. The `ci-required-checks` ruleset on this repo │ - # │ requires the CONTEXTS `ci / verify (22, ubuntu-latest)`, `ci / verify (24, │ - # │ ubuntu-latest)` and `ci / actionlint`. Every gate the shared pipeline runs (typecheck, │ - # │ lint, format:check, PHI scan, test, gating coverage, build, attw, dual ESM/CJS smoke) is │ - # │ a STEP inside `verify`, and is required only because that job is. Promote any one of │ - # │ them to its own job and it is silently NO LONGER REQUIRED: the ruleset names contexts, │ - # │ the new context is not in it, and nothing anywhere errors or warns. If you split a step │ - # │ out, add its context to the ruleset in the same change. │ - # └─────────────────────────────────────────────────────────────────────────────────────────┘ + # ┌──────────────────────────────────────────────────────────────────────────────────────────────────┐ + # │ A REQUIRED JOB GATES ALL OF ITS STEPS. The `ci-required-checks` ruleset on this repo requires │ + # │ all four contexts this caller emits: `ci / verify (22, ubuntu-latest)`, `ci / verify (24, │ + # │ ubuntu-latest)`, `ci / prepublish` and `ci / actionlint`. Every gate the shared pipeline runs │ + # │ (typecheck, lint, format:check, PHI scan, test, gating coverage, build, attw, dual ESM/CJS │ + # │ smoke) is a STEP inside `verify`, and is required only because that job is. Promote any one of │ + # │ them to its own job and it is silently NO LONGER REQUIRED: the ruleset names contexts, the new │ + # │ context is not in it, and nothing anywhere errors or warns. If you split a step out, add its │ + # │ context to the ruleset in the same change. │ + # │ │ + # │ AND THE HAZARD IS NOT ONLY A LOCAL SPLIT: THE CALLED WORKFLOW CAN GROW A JOB UPSTREAM. The │ + # │ `uses:` below is pinned to `@main`, so a job added over there starts emitting a new `ci / ` │ + # │ context here with NO COMMIT IN THIS REPO, and it arrives NOT REQUIRED: a red X that does not │ + # │ block a merge. That is not hypothetical. `prepublish` arrived exactly that way on 2026-08-05 and │ + # │ was unrequired here until it was measured off a real check run and added. A `ci / *` context you │ + # │ do not see in the list above is that same event happening again. Decide it, and write the │ + # │ decision down: require it, or record here why it must not be. │ + # └──────────────────────────────────────────────────────────────────────────────────────────────────┘ + # + # `prepublish` is the pre-publish gate: it refuses a dependency specifier no registry can resolve + # (offline manifest lint) and then packs this tree and installs the tarball into a clean directory. + # Both layers are ON by default upstream and this caller passes neither toggle, so the defaults are + # the whole story for whether it runs. It is the only place a consumer's install question gets + # asked while the answer can still prevent a permanent publish: `@cosyte/cli@0.0.1` and `0.0.2` are + # on the registry uninstallable forever, and this job is what would have refused them. + # + # ONE DEFAULT IS PERMANENTLY STALE HERE, AND THE OBVIOUS "FIX" FOR IT IS FORBIDDEN. The upstream + # default for `expect-unpublished-deps` is `@cosyte/fhir=blocked`. Be precise about why that is + # stale, because the short version is false: `@cosyte/fhir` IS declared in this manifest, as + # `devDependencies: {"@cosyte/fhir": "file:vendor/cosyte-fhir-0.0.0.tgz"}`, which is what lets this + # repo's own FHIR tests run. The operative fact is that `prepublish-check.mjs` does not read + # `devDependencies` at all (a `file:` specifier is fine there, since no consumer installs them), so + # the allowance names a package that is not a CONSUMER dependency of this one and is reported stale + # on every run. That notice never fails the job. + # + # It must NOT be silenced by promoting `@cosyte/fhir` into `dependencies` or `optionalDependencies`: + # declaring it in either, alongside `@cosyte/transform`, fails the whole install with `ERESOLVE`, + # which is the measured reason it is not there. See CLAUDE.md. If the notice is ever worth + # silencing, the way is to pass an empty `expect-unpublished-deps` from this caller. # # Universal gates from the shared pipeline (typecheck, lint, format, test, coverage, build, attw, # dual ESM/CJS smoke, actionlint) on the standard Node matrix. Repo-specific extra jobs (e.g. an diff --git a/CHANGELOG.md b/CHANGELOG.md index e2772a4..cc763eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,36 @@ still do. Each entry was assigned to the release whose tag first contains it, re ### Fixed +- **A red pre-publish gate showed a red X and merged anyway, because `ci / prepublish` was a + required check nowhere (CI-REQUIRED-CHECKS).** The shared pipeline this repo calls grew a + `prepublish` job on 2026-08-05 (`cosyte/.github#35`, `6142ac4`; its second layer defaulted on in + `#36`, `90936ea`), so `ci.yml` began emitting a `ci / prepublish` context on every pull request + here **with no commit landing in this repo**. Repository ruleset `19907924` did not name it, so + the job could fail and the merge still landed on `main`, which is the branch that publishes. + `ci / prepublish` is now a required context, `integration_id`-pinned like the other six. + - **What it gates is this package's own worst shipped defect.** The job is an offline manifest + lint that refuses a dependency specifier no registry can resolve, plus a probe that packs this + tree and installs the tarball into a clean directory. `@cosyte/cli@0.0.1` and `0.0.2` reached + the registry carrying `file:vendor/*.tgz` specifiers and are permanently uninstallable; the + manifest lint would have refused both. A gate that catches that and then cannot block the merge + reintroducing it is documentation. + - **The order was load-bearing and is recorded because it is easy to get backwards.** The context + name was read off a real `pull_request` check run before the ruleset was written, never off the + workflow's `name:` field. Requiring a context nothing emits does not fail a pull request, it + leaves it pending and unmergeable forever. A census of the eight most recent head shas here + (`#27` through `#34`) finds `ci / prepublish` **zero** times, because the newest of them merged + before the upstream job existed, so it could not have been required any earlier. + - **`ci.yml`'s banner now covers the hazard that actually bit.** Every previous wording of it was + scoped to splitting a step out of `verify` locally. The `uses:` reference is unpinned, so a job + added upstream emits a new `ci / ` context here with no commit in this repo, and it always + arrives unrequired. + - **Deliberately NOT in this change: `release.yml`'s version-PR trap note.** It carries a stale + required-context count and a claim about the "Version Packages" PR arriving with zero check runs + whose truth depends on whether `RELEASE_PR_TOKEN` is authoring that PR. Two attempts to correct + it inside this change each produced a fresh false claim: the first a false operational premise, + the second a false claim about the note's own history. So it is cut out to its own change rather + than rewritten a third time. Nothing about it is made worse here; it is left exactly as it was. + - **The shipped documentation sidebar was off the canonical IA spine, and it was holding up the docs site's deploy (CLI-SIDEBAR-IA-NONCANONICAL).** `docs-content/sidebars.json` declared two top-level categories that are not on the spine, **"MCP server"** and **"Reference"**. The docs diff --git a/CLAUDE.md b/CLAUDE.md index 49999a8..1250be1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -254,11 +254,21 @@ Full ruleset, the required-context table and the per-check reasoning: [agent-notes § Branch protection](documentation/agent-notes.md#branch-protection-and-the-limits-of-this-claim). - `main` is protected by the repository ruleset **`ci-required-checks`** (id `19907924`). Before it - existed every check here was advisory, on the branch that publishes. Six required contexts, each - pinned to **`integration_id: 15368`** so a same-named status from another actor cannot satisfy it. + existed every check here was advisory, on the branch that publishes. **Seven** required contexts, + each pinned to **`integration_id: 15368`** so a same-named status from another actor cannot satisfy + it. **Do not quote that count without re-deriving it** (`gh api repos/cosyte/cli/rulesets/19907924`): + it was `6` until `ci / prepublish` was added, and it moves when the called workflow does. - **Read a required context off a REAL check run, never off a workflow's `name:` field.** Requiring a context nothing emits does not fail a PR: it leaves it **pending and unmergeable forever**, with no error and no warning. +- **▶ A `ci / *` CONTEXT CAN APPEAR HERE WITH NO COMMIT IN THIS REPO, AND IT ARRIVES NOT REQUIRED.** + `ci.yml` calls `cosyte/.github/.github/workflows/ci.yml@main` unpinned, so a job added upstream + starts emitting a context here that the ruleset does not name: **a red X that does not block a + merge**, which is the failure this repo's whole protection claim exists to prevent. `ci / prepublish` + arrived that way on 2026-08-05 and was unrequired until measured and added. **Census `ci / *` against + a real check run whenever `.github` moves**, then require it or write down why not, in `ci.yml`'s + banner. Detail: + [agent-notes § Branch protection](documentation/agent-notes.md#branch-protection-and-the-limits-of-this-claim). - **`no-internal-refs` and `no-emdash` are bare JOB IDS**, not ` / `, because they are ordinary jobs in this repo's own workflows. **Renaming the job silently detaches the required check.** Rename the job and the ruleset together, or neither. diff --git a/documentation/agent-notes.md b/documentation/agent-notes.md index 71c7dfc..3a7895c 100644 --- a/documentation/agent-notes.md +++ b/documentation/agent-notes.md @@ -402,20 +402,128 @@ advisory: `ci`, `codeql`, `scorecard` and `fuzz` could all be red and the merge Required contexts, each pinned to **`integration_id: 15368`** (the `github-actions` app) so that a commit status of the same name posted by any other actor with write access cannot satisfy it: -| context | -| ------------------------------------------ | -| `ci / verify (22, ubuntu-latest)` | -| `ci / verify (24, ubuntu-latest)` | -| `ci / actionlint` | -| `codeql / analyze (javascript-typescript)` | -| `no-internal-refs` | -| `no-emdash` | +| context | added | +| ------------------------------------------ | ---------- | +| `ci / verify (22, ubuntu-latest)` | 2026-07-28 | +| `ci / verify (24, ubuntu-latest)` | 2026-07-28 | +| `ci / actionlint` | 2026-07-28 | +| `ci / prepublish` | 2026-08-06 | +| `codeql / analyze (javascript-typescript)` | 2026-07-28 | +| `no-internal-refs` | 2026-07-28 | +| `no-emdash` | 2026-07-28 | These are the names GitHub actually reports, read off real check runs, **not** off a workflow's `name:` field. Requiring a context nothing emits does not fail a PR; it leaves it pending and unmergeable forever. None of `ci.yml`, `codeql.yml`, `no-internal-refs.yml` or `no-emdash.yml` carries a `paths:` filter, so no PR can skip one. +**Do not quote the row count from memory, and do not carry it into another document.** It was `6` +until `ci / prepublish` was added and it moves whenever the called workflow does. Derive it: +`gh api repos/cosyte/cli/rulesets/19907924 --jq '[.rules[]|select(.type=="required_status_checks").parameters.required_status_checks[].context]'`. + +### `ci / prepublish`, and the hazard that a context can arrive with no commit here + +**▶ A `ci / *` CONTEXT CAN APPEAR IN THIS REPO WITH NO COMMIT IN THIS REPO, AND IT ARRIVES NOT +REQUIRED.** `ci.yml` calls `cosyte/.github/.github/workflows/ci.yml@main`, unpinned. A job added to +that reusable workflow starts emitting `ci / ` on every PR here immediately, the ruleset does +not name it, and so **a red result from it shows a red X and the merge lands anyway**. Nothing +errors, nothing warns, and no commit in this repo records that the surface changed. Every previously +written form of the "a required job gates all of its steps" warning was scoped to a **local** split, +so none of them covered this. + +(Two corrections this paragraph has already needed, kept visible because both are easy to write +again. There is **no merge queue** on this repo: `gh api repos/cosyte/cli/branches/main/protection` +returns `404 Branch not protected` and the ruleset carries no `merge_queue` rule. And +`required_approving_review_count: 0` is **not** the reason a red non-required check merges: a context +absent from `required_status_checks` blocks nothing at **any** approval count, because the ruleset +only ever evaluates the contexts it names. The approval count is why the merge needs no **review**, +which is a different question and does not bear on this one.) + +That is not hypothetical. The `prepublish` job was added upstream in `cosyte/.github#35` (`6142ac4`, +2026-08-05) and its second layer was defaulted on in `#36` (`90936ea`, the same day). **The census +that proves it was unrequired**: on the eight most recent `pull_request` head shas here (`#27` +through `#34`), `ci / prepublish` appears **zero** times, because the newest of them (`#34`) merged +`2026-08-04T22:22:24Z` and the upstream job postdates it. So the job could not have been read off a +real check run before this slice, and requiring it earlier would have been the `knowledgebase` +mistake (naming a context nothing emits) rather than a fix. **The order is load-bearing: it has to +run first, then be added, in that order.** It was read off the real check run on the pull request +that shipped this section before the ruleset was written. + +**▶ THE COST THAT ORDERING DOES NOT COVER, DISCLOSED BECAUSE IT WAS PAID: ADDING A REQUIRED CONTEXT +STRANDS EVERY OPEN PULL REQUEST WHOSE HEAD SHA ALREADY RAN.** The ordering protects PRs opened +_after_ the write, because their head shas produce the new context. It does nothing for a head sha +that ran before it existed: that PR now needs a context nothing will ever post for it, so it goes +`BLOCKED` and stays there. + +**Attribute this by measuring each head sha, not by listing what is open.** Six pull requests were +open at the time of this write, one of them being the PR performing it; of the other five, exactly +**three** were stranded by it, and the first draft of this paragraph named the wrong set by reading +`mergeStateStatus` instead of the check runs: + +| PR | head sha | state before the write | stranded by this write? | +| --- | ---------- | ------------------------------------------------------- | ----------------------- | +| #33 | `f69ab63a` | six older required contexts green | **yes** | +| #18 | `73758565` | six older required contexts green | **yes** | +| #16 | `6cc21d8a` | six older required contexts green | **yes** | +| #29 | `95510b9d` | `ci / verify` **red on both matrix legs** | no, already unmergeable | +| #15 | `b63cd115` | no `no-emdash`, no `no-internal-refs` (predates both) | no, already stranded | + +All three affected PRs are Dependabot's, and Dependabot regenerates its branches, so **nothing was +pushed to them**: a push onto a branch this slice does not own, to clear a condition this slice +created, is the more intrusive fix. **The remedy, when it is yours to apply, is one push per +branch**, which re-runs CI and produces the context. It is not a ruleset problem and must not be +fixed by removing the requirement. Check before the next such write: +`gh api repos/cosyte/cli/commits//check-runs`, per open PR. + +**What it gates, and why leaving it unrequired was the expensive kind of hole.** `prepublish` runs +two layers: an offline **manifest lint** that refuses a dependency specifier no registry can resolve, +and a **pack-and-install** probe that `npm pack`s this tree and installs the tarball into a clean +anonymous directory. Both default on upstream and this caller passes neither input. This package is +the reason that gate exists: `@cosyte/cli@0.0.1` and `0.0.2` were published carrying +`file:vendor/*.tgz` specifiers and are **permanently uninstallable** (ADR 0001, a published version +never moves backwards). The manifest lint would have refused both. A gate that catches that, and then +does not block the merge that reintroduces it, is documentation. + +**It is required, not merely present, deliberately.** The alternative considered and rejected was to +leave it advisory on the grounds that it touches the network on every PR and a registry blip would +red it. That cost is real and is disclosed upstream: the `pnpm install --frozen-lockfile` in this job +has no registry-outage softening, unlike the pack layer's `inconclusive` verdict. It was accepted +here anyway, because an advisory pre-publish gate on the branch that publishes is the exact shape of +"a green check that cannot block a merge". + +### The one time a required check actually blocked something in this slice, and what it caught + +**▶ THE DEMONSTRATION THIS WHOLE SUBJECT HAD BEEN MISSING, AND IT LANDED ON THE AUTHOR.** Everything +above is about a check that reports without blocking. While shipping it, the first thing a required +context actually blocked was **this author's own prose**: editing the pull request body turned +`no-emdash` **red**, twice, on a **required** context, over four `U+2014` characters that had been +typed into the PR body's explanatory sections. The pull request went `mergeStateStatus: BLOCKED` and +stayed unmergeable until the body was rewritten (runs at `12:14:31Z` and `12:14:59Z` failed; +`12:16:34Z` passed). + +**Why it is worth a section rather than a footnote.** It is the concrete counter-example to the +failure this section documents. A red X that does not block a merge is documentation; this was a red +X that stopped a merge dead, and the difference between the two is exactly one line in a ruleset. + +**And it landed on the half of the gate that nothing local can see.** `scripts/check-no-emdash.sh` +scans **tracked files** and was green throughout, both in the pre-commit hook and in `verify.sh`; +`git diff` over the branch carried zero `U+2014`. The offending text was never in a file. The PR +body is a surface that exists only on GitHub, reached only by `no-emdash.yml`'s `edited` trigger, and +**no local run of anything in this repo could have caught it**. So the two halves of that gate are not +redundant: the tracked-file half is the one a worker exercises constantly and the PR-text half is the +one that catches what a worker writes *about* the work. + +**Read alongside the standing note that the PR body lands under none of the three merge methods.** +That is still true, and the gate scans it anyway as deliberate over-strictness. This is what that +over-strictness buys, observed rather than argued: without it, four em dashes would have gone onto a +public pull request on a public repository, and the ban is absolute. + +**Not built, and it must not be built without answering one question first.** A gate inside CI that +`curl`s this repo's own ruleset and asserts the required set would close the observability gap named +at the end of this section. Anonymous GitHub API is **60 requests per hour, per IP, and hosted +runners share IPs**, so such a gate trades a false green for a **flaky red on a required context**, +which is worse than the hole. Answer the flakiness question with a measurement before writing it. + **`no-internal-refs` is the one that is NOT ` / `, and the shape is worth knowing.** `ci / verify (22, ubuntu-latest)` is prefixed because `verify` runs inside a _called_ reusable workflow, so the context is ` / (matrix)`. `no-internal-refs` is an