diff --git a/.gitea/workflows/docker-build.yml b/.gitea/workflows/docker-build.yml index 7b0f1d6c0..2e3e79735 100644 --- a/.gitea/workflows/docker-build.yml +++ b/.gitea/workflows/docker-build.yml @@ -865,7 +865,27 @@ jobs: INFO_VERSION="${VERSION}" TAGS=("${IMAGE}:prod" "${IMAGE}:${VERSION}" "${IMAGE}:${SHORT}") else - DESC=$(git describe --tags --abbrev=0 2>/dev/null || echo v0.0.0) + # `git describe` MUST resolve here, and a failure is fatal rather than defaulted + # (ersatztv#836). This job checks out `fetch-depth: 0`, so the tags are present; the + # only thing that ever stopped `describe` from seeing them was the detector step above + # grafting this complete clone shallow. The old `|| echo v0.0.0` was a fallback that + # cannot fail, so from 2026-07-17 (when #416 introduced the depth) until #836 every + # `:latest` image was published carrying + # `InformationalVersion 0.0.0-` and nothing anywhere went red — the defect was + # found by reading the string out of a running container, which is not a detector. + # Failing the job instead means no `:latest` is published at all: visible, recoverable, + # and never a mislabelled image promoted downstream. The tag path above never calls + # `describe`, so a release cut is unaffected by this. + # stderr is discarded on the CAPTURE and re-run for the diagnostic, rather than folded + # in with `2>&1`: a git warning on the SUCCESS path would otherwise land inside DESC and + # become part of the version string — the same shape of silent corruption this whole + # step is being hardened against. + if ! DESC=$(git describe --tags --abbrev=0 2>/dev/null); then + echo "is-shallow-repository=$(git rev-parse --is-shallow-repository)" + git describe --tags --abbrev=0 || true + echo "::error::git describe --tags --abbrev=0 failed, so this image would ship InformationalVersion 0.0.0-${SHORT} instead of a real version (ersatztv#836). The usual cause is a --depth fetch grafting this complete clone shallow; the two lines above say which." + exit 1 + fi INFO_VERSION="${DESC#v}-${SHORT}" TAGS=("${IMAGE}:latest" "${IMAGE}:${SHORT}") fi diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 5e9f4666f..40612bd0f 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -313,7 +313,9 @@ further back than N. Measured 2026-08-26: a 150-commit repo branched 130 back, ` returns the correct file list. Under the old `|| true` that surfaced as a silent skip; unmasked it would have reddened `api-docs` and `Formatting` on any branch not rebased recently, sent `docs-reminder`'s parity nudge advisory-red, and left `decisions lifecycle` green with its -diff-dependent checks silently skipped — so the depth came off with the mask. The same graft is why `:latest` images ship `InformationalVersion 0.0.0-` (ersatztv#836). +diff-dependent checks silently skipped — so the depth came off with the mask. The same graft, +reached through the detector script rather than a workflow step, is what stamped `:latest` images +`InformationalVersion 0.0.0-` until ersatztv#836 — see below. **Two masked fetches that stay masked, deliberately.** `scripts/ci-detect-docs-only.sh` and `scripts/ci-detect-already-validated.sh` both swallow a failed fetch — and should. In both, a failed @@ -323,14 +325,38 @@ runner time and nothing else, which is the opposite direction from the workflow the same masking bought a silently skipped check. Judge these by which way the failure falls, not by the `|| true`. -That clears their **masking** and nothing else. Their `--depth` is a separate question with a -separate answer: `ci-detect-docs-only.sh`'s push-path `--depth=2` runs inside the `build` job's -`fetch-depth: 0` checkout and grafts it shallow, which is why the `git describe` in the immediately -following step returns nothing and `:latest` ships `InformationalVersion 0.0.0-` — measured on -the live test container as `0.0.0-5a7a4ed7` (ersatztv#836). That depth is correct for the script's -other three consumers — `test`, `migrations` and `functional-e2e` — whose checkouts really are -shallow (`fetch-depth: 2`). `build` is the only one that checks out complete, which is why it is the -only one that grafts. +That clears their **masking** and nothing else. Their `--depth` was a separate question with a +separate answer, and it is the one that shipped a defect (ersatztv#836). + +**A depth is passed only to a checkout that is already shallow.** `ci-detect-docs-only.sh` routes +both its fetches through `fetch_ref`, which asks `git rev-parse --is-shallow-repository` and drops +the `--depth` on any answer but `true`. The script has **exactly four consumers** — `test`, +`migrations` and `functional-e2e` at `fetch-depth: 2`, and `build` at `fetch-depth: 0`. Enumerate +them with `grep -rn ci-detect-docs-only .gitea/`, not by looking for a step with `id: detect`: +`api-docs` and `format` have one of those too, and it runs their own inline diff. + +So `build` is the only complete-clone consumer, and the push arm is the only arm it takes. That is +where the cost landed: its `--depth=2` grafted `build`'s complete clone, the `git describe --tags` +in the immediately following step found no reachable tag, and a `|| echo v0.0.0` fallback that +cannot fail turned that into a version. Every `:latest` image published from `main` carried +`InformationalVersion 0.0.0-` instead of `26.x.y-` — read off the live test container as +`0.0.0-5a7a4ed7` on 2026-08-26, six weeks after the depth landed with #416 on 2026-07-17, because +nothing went red. + +The PR arm's `--depth=200` had **no complete-clone consumer at all** (on `pull_request` only the +three shallow jobs run this script), so it was inert rather than latently firing. It goes through +the same helper anyway, because the clause is shared and the next `fetch-depth: 0` consumer of that +arm would inherit the graft silently. Keeping the depth for the shallow three is likewise +conservative rather than required: a depth-less fetch into a `--depth=2` clone leaves `.git/shallow` +in place, still writes `FETCH_HEAD`, and still reports the parents the push arm reads. + +Both halves are now held by something that executes. `scripts/tests/test_docs_only_detector_clone_depth.py` +drives the real script over real `file://` clones and asserts the clone is still complete and +`git describe` still resolves afterwards, with a negative control that requires the fixture to be +able to graft at all — a plain path clone silently ignores `--depth`, which would make the whole +file vacuous. And `Compute version and tags` no longer defaults: a failed `git describe` on the +non-tag path fails the job rather than stamping `0.0.0`, so no `:latest` is published instead of a +mislabelled one. The tag path never calls `describe`, so a release cut is unaffected either way. ### The last exemption, and how it closed diff --git a/docs/decisions/README.md b/docs/decisions/README.md index 498006640..7e5681dc2 100644 --- a/docs/decisions/README.md +++ b/docs/decisions/README.md @@ -47,6 +47,7 @@ the link for rationale. Superseded/retired history lives in `archive/`. Regenera | `ci.docs-only-detect-shallow-safe` | The docs-only detect script must diff against `FETCH_HEAD` (always resolves after `git fetch`, even shallow) using a two-dot tree diff — not `origin/` with three-dot — because a `fetch-depth: 1` shallow clone has no remote-tracking ref and no merge-base, which silently fails the original detect into `docs_only=false` (full matrix, no functional error). A CI-behavior change must be verified by measuring the effect (job durations), not just a green check. | 2026-07-17 | [link](records/ci/docs-only-detect-shallow-safe.md) | | `ci.docs-only-skip-steps` | A docs-only change must still run every required job (`test`, `migrations`) so their commit-status contexts always report; each heavy job runs `scripts/ci-detect-docs-only.sh` first and gates its real STEPS on `if: steps.detect.outputs.docs_only != 'true'`, never `if:`-skips the whole job (an `if:`-skipped job reports `skipped`, not `success`, which branch protection may never unblock on). Detection biases toward running more on any doubt. | 2026-07-17 | [link](records/ci/docs-only-skip-steps.md) | | `ci.exemption-provenance` | The three inputs the exemption decision rests on must each be bound to something the judged PR cannot mutate. (1) BASE — `scripts/pr-changed-files.sh` takes the expected base BRANCH as a REQUIRED 5th argument and re-reads it before and after paging, because `/pulls/{n}/files` diffs against the PR's live base and retargeting moves the answer without moving the head sha; the workflow passes `github.event.pull_request.base.ref` from the `pull_request_target` payload, which a retarget cannot rewrite. (2) BOT EXEMPTION — an author match is necessary but never sufficient: `pull_request.user.login` is the PR's immutable CREATOR while its head is not, so the exemption additionally requires EVERY changed path to be a dependency manifest (`Directory.Packages.props` or `.config/dotnet-tools.json`, and ONLY those — the npm manifests are excluded because `package.json` `scripts` are executed by CI). (3) INHERITED SUCCESS — the never-overwrite short-circuit fires for a `success` only when it is POSITIVELY identified as an APPROVED REVIEWER's verdict for THIS base, meaning a `.creator.login` that is a member of the workflow's `H10_REVIEWERS` allow-list (an existing `failure` takes the weaker ATTRIBUTABLE test and needs no membership, because inheriting a rejection can only withhold an exemption while re-deriving one can turn it green on an exempt PR) — non-null was too weak, since any account's credential satisfies it and `RENOVATE_TOKEN` is one that cannot be scoped down (#742) — AND a `Review-verdict:` description AND, when that description records a base (`(base: …)`, `release.verdict-status-check`), a base matching the PR's — tested by requiring the description to END with the exact literal `(base: )` and to contain exactly ONE such marker, never by extracting a value (see below); a present-but-different base is rejected, an absent one is not, since verdicts predating that convention carry none; every other shape, including any unrecognised one, is re-derived rather than trusted. The bot and docs-only exemptions are evaluated as INDEPENDENT predicates and the decision made afterwards, never as an `elif` chain. `edited` is in the workflow's `types:` so a retarget reclassifies — which gives DETECTION, not atomicity: status writes are not serialized, so a stale run can still post over a fresher one. That residual is now FENCED rather than merely tracked (NARROWED, not resolved — a retarget between the fence's final count and the POST still leaves a PERMANENT forged green, #849) — the job refuses to write at all if the PR's timeline retarget COUNT moved while it was classifying (`ci.verdict-write-retarget-fence`, #706) — leaving TWO windows, not one: the sub-round-trip window that no API without compare-and-set can close, AND the post-final-count/pre-POST window, which is not sub-round-trip and yields a PERMANENT green because the successor can consume the `edited` event and exit before the stale run posts last (#849). The PROTECTED path list additionally covers `CLAUDE.md` and `AGENTS.md` (#751) — they are not prose but the documents DEFINING the completion protocol, the merge-consent convention and the H10 rule, so protecting `.claude/` while the file specifying what it enforces stayed docs-only-exempt was the same self-exemption one directory over; driving the real classify body with a lone `CLAUDE.md` change produced an exemption `success`. `README.md` is deliberately not listed. It also covers `.codex/` (#711), which mirrors `.claude/hooks/` byte for byte including the merge-consent hook — latent while that directory is untracked, live the moment it is tracked; the list stays ENUMERATIVE rather than derived, because a derived rule would have to be evaluated against the very file list being classified. Reading the CURRENT status for input (3) must tolerate `statuses: null`: `GET /commits/{sha}/status` serialises a nil slice as `null`, not `[]`, on a head with no statuses yet, and an `array`-only gate made `read_existing_verdict` `exit 1` and post nothing at all (#751, `ci.workflow-run-body-no-expressions`) — `null` is accepted only when `total_count` is 0, so a body that merely lost its array is still refused. Path predicates are evaluated by COUNTING with `grep -c`, never `\| grep -q` (SIGPIPE inversion) and never a here-string (temp-space failure) — see `ci.grep-q-pipefail-inversion`. | 2026-07-29 | [link](records/ci/exemption-provenance.md) | +| `ci.fetch-depth-never-grafts-a-complete-clone` | Any `git fetch` in CI tooling passes `--depth` only when `git rev-parse --is-shallow-repository` already answers `true`, and passes none on any other answer including an unreadable one. `git fetch --depth=N` GRAFTS a complete clone shallow: it writes `.git/shallow` and cuts history at N even though every object is already present, so a depth chosen for one consumer silently breaks every other consumer that checks out `fetch-depth: 0`. A shared script whose consumers disagree on checkout depth must ask the repository rather than assume. And whatever the fetch feeds — a version, a diff, a base revision — must be ASSERTED rather than defaulted: a fallback that cannot fail converts the graft into a plausible wrong answer that never reddens. | 2026-08-29 | [link](records/ci/fetch-depth-never-grafts-a-complete-clone.md) | | `ci.format-gate-folder-mode` | The blocking `format` CI job (and matching pre-commit hook) runs `dotnet format whitespace . --folder --include ` instead of loading the full MSBuild/Roslyn solution, cutting the gate from ~480s to ~0.5s with unchanged whitespace/charset coverage. | 2026-07-19 | [link](records/ci/format-gate-folder-mode.md) | | `ci.functional-e2e-harness` | The `functional-e2e` CI job boots the PR's own code from source via `dotnet run` (`scripts/e2e-local.sh`) and runs deterministic assertions (`scripts/e2e-functional.sh`) as an advisory (non-blocking) job, not a `build` dependency or required check. Originally curl-only; since #445 the same job carries a second, headless-browser step for the contracts curl cannot express — see `ci.ui-e2e-harness`. | 2026-07-16 | [link](records/ci/functional-e2e-harness.md) | | `ci.gate-trigger-base-resolved` | The workflow that writes the branch-protection-required `review-verdict/h10` status triggers on `pull_request_target` with `branches: [main]`, never on plain `pull_request`. Gitea resolves a `pull_request` workflow DEFINITION from the PR's own head commit, so under that trigger a PR editing `.gitea/workflows/review-verdict.yml` ran its own rewritten copy and could post `h10=success` for itself; `pull_request_target` resolves the definition from the base instead. The `branches: [main]` filter is part of the rule, not a refinement of it: base resolution only relocates the rewrite from the head to the base, so without the filter a PR opened into an attacker-pushed base branch runs that branch's gate. `pull_request_target` is safe HERE only because this job never checks out or executes head-supplied code — it checks out `base.sha` and runs only that tree's scripts (`ci.shared-pr-file-enumeration`); reintroducing a head checkout under this trigger would be worse than the bug it fixed. This closes the rewrite route through THIS workflow and does NOT close the class. Gitea injects `GITEA_TOKEN` into EVERY job and it USED to be write-capable everywhere, so any ref-resolved workflow could forge `review-verdict/h10`; since #748 (2026-08-27) all six workflows here declare `permissions:` — five `code: read`, and `review-verdict.yml`'s gate job `code: write`, deliberately, because it IS the gate (the five rest on the TOP-LEVEL form, which is INFERRED rather than probed — the gate job's JOB-LEVEL form is the one measured; see `ci.actions-credential-scoping`). That narrows the INJECTED token and only that: deliberately NO "only X can post a status" summary is stated here, because any such summary generalizes from the injected token, which `permissions:` scopes, to every credential a job can reach, which it does not touch — `RENOVATE_TOKEN` is in the same secret store and is referenced by `renovate.yml`, one of the five holding `code: read`. `ci-image.yml` was the last holdout — its unfiltered `push:` made it the worst-placed of the six — and #744 closed that trigger route (`ci.toolchain-image-publish-is-a-dispatch`) while #748 scoped its token. So the route is NARROWED, not closed, and the SHAPE is open — a newly added workflow declaring write, or omitting a declaration and inheriting the owner default, is a route again, which is why `test_pr_changed_files.py` asserts every tracked workflow declares a block. And a collaborator's own API token still can, since branch protection binds the context and not its issuer. The credential half is now RESOLVED in `ci.actions-credential-scoping` (#697): CI's registry secret was the ADMIN account's basic auth and is now a PAT that cannot post a status, which removes the ADMIN escalation and that credential's route (an ALLOW-LISTED user credential's forgery carries a matching `creator` and is inherited as a verdict; an Actions job's carries `creator: null` and is re-derived, as since #742 is every account outside `H10_REVIEWERS` — but do NOT read that asymmetry as protection: re-derivation fires only on the trigger's `types`, and posting a status is not one of them, so a POST timed after the last PR event simply stands). It does not remove EVERY route: `RENOVATE_TOKEN` is a `write:repository` bot PAT in the same secret store, reachable by any PR-added workflow. The owner-level Restricted default (server-management#714) remains a separate OPEN lever — its effect on a declared `code: write` is unmeasured, and the version half of that condition has been satisfied since the 1.25.4 -> 1.27.1 upgrade on 2026-08-05, so it is not something to wait on a Gitea release for. A collaborator's own token remains unfixable; the exemption path has its own separate defects in #698. | 2026-07-28 | [link](records/ci/gate-trigger-base-resolved.md) | diff --git a/docs/decisions/records/ci/fetch-depth-never-grafts-a-complete-clone.md b/docs/decisions/records/ci/fetch-depth-never-grafts-a-complete-clone.md new file mode 100644 index 000000000..d428e55a5 --- /dev/null +++ b/docs/decisions/records/ci/fetch-depth-never-grafts-a-complete-clone.md @@ -0,0 +1,68 @@ +--- +key: ci.fetch-depth-never-grafts-a-complete-clone +title: '2026-08-29 — A CI fetch passes --depth only to a checkout that is already shallow (#836)' +status: active +since: '2026-08-29' +supersedes: none +superseded-by: none +rule: 'Any `git fetch` in CI tooling passes `--depth` only when `git rev-parse --is-shallow-repository` already answers `true`, and passes none on any other answer including an unreadable one. `git fetch --depth=N` GRAFTS a complete clone shallow: it writes `.git/shallow` and cuts history at N even though every object is already present, so a depth chosen for one consumer silently breaks every other consumer that checks out `fetch-depth: 0`. A shared script whose consumers disagree on checkout depth must ask the repository rather than assume. And whatever the fetch feeds — a version, a diff, a base revision — must be ASSERTED rather than defaulted: a fallback that cannot fail converts the graft into a plausible wrong answer that never reddens.' +signals: 'git fetch --depth, .git/shallow, grafted clone, git describe finds no tag, InformationalVersion 0.0.0, fetch-depth 0 vs fetch-depth 2 · paths: `scripts/ci-detect-docs-only.sh`, `.gitea/workflows/docker-build.yml` · issues: #836, #746' +mechanics: 'docs/ci-cd.md → "A depth is passed only to a checkout that is already shallow"; guard `scripts/tests/test_docs_only_detector_clone_depth.py`, a MUTATION-graded row in `docs/guard-inventory.md`' +--- + +`scripts/ci-detect-docs-only.sh` has exactly four consumers in `docker-build.yml`, and they do not +agree on checkout depth: `test`, `migrations` and `functional-e2e` check out `fetch-depth: 2`, and +`build` checks out `fetch-depth: 0`. The script carried a `--depth` chosen for the first group and +applied it to all of them. **`build` is the only complete-clone consumer.** `api-docs` and `format` +also check out `fetch-depth: 0` and also have a step with `id: detect`, but that step runs their own +inline diff, not this script. **Enumerate the consumers with `grep -rn ci-detect-docs-only .gitea/`, +never by looking for a step with `id: detect`** — that is the trap, and it points at the wrong two +jobs. The grep returns five hits: the four `run:` call sites plus `docker-build.yml`'s own header +comment. + +The harm is not theoretical. The push arm is the only arm `build` ever FETCHES on — it also takes +the tag early-exit on a release cut and the `workflow_dispatch` arm, neither of which fetches — and +that arm grafted `build`'s complete clone on every push to `main`; +the very next step, `Compute version and tags`, runs `git describe --tags --abbrev=0`, which then +found no reachable tag; and its `|| echo v0.0.0` turned that into a version string. Every `:latest` +image published from `main` carried `InformationalVersion 0.0.0-` instead of `26.x.y-` +from 2026-07-17, when #416 introduced the depth, until this change. Nothing went red at any point — +the defect was found by reading the string out of a running container. + +**Why the depth is kept for the shallow group, stated as measured rather than as load-bearing.** A +depth-less fetch into a `--depth=2` clone leaves `.git/shallow` in place, still writes `FETCH_HEAD`, +and still lets `git rev-list --parents -n1 HEAD` report the parents the push arm reads. So dropping +the depth outright would not have broken those three jobs either. It is retained because it is what +they were given and nothing argues for widening their fetch — not because removing it was shown to +break anything. + +**The PR arm's `--depth=200` had no complete-clone consumer at all.** On `pull_request` the only +jobs running this script are the three shallow ones, so that depth was inert rather than latently +firing. It is fixed in the same change anyway, because the clause is shared: the next +`fetch-depth: 0` consumer added to that arm would inherit the graft silently, and rediscovering it +would cost what #836 cost (`process.fix-the-boundary-not-the-site`). That is a guard against a +future shape, and grading it as a reproduction of a shipped one would overstate it. + +**Why any answer but `true` passes no depth.** There is more than one such answer, and enumerating +them is the point: `git rev-parse` fails outright where there is no readable repository, and on a +git older than 2.15 the flag is unrecognised and rev-parse ECHOES IT BACK verbatim with status 0 +(verified on 2.55 — an unknown `--is-...` flag prints itself and exits 0). Neither is `true`, both +take the no-depth branch, and in neither could a depth have helped: the base stays unresolvable and +every arm falls through to the full matrix, which is the safe direction for this script. Defaulting +the other way would re-graft on exactly the paths nobody can observe. + +**The second half is the loud failure, and it is not decoration.** Removing the graft removes +today's cause; the fallback is what made any cause invisible. `Compute version and tags` now fails +the job when `git describe` fails on the non-tag path. No `:latest` at all is visible and +recoverable; a mislabelled one is neither, and `:latest` is what `ersatztv-test` tracks. The tag +path never calls `describe`, so a release cut is unaffected — which is also why `:prod` and +`:` images were correctly versioned throughout. + +**Fixture note for anyone testing this.** `git clone /path` and `git fetch /path` use the local +transport, which ignores `--depth` outright. A reproduction built on a plain path never grafts, so +every assertion holds and the test proves nothing; use `file://`. The guard carries a negative +control that performs the raw depth fetch and requires the graft to happen, so that if this ever +stops being true it is reported rather than silently making the rest vacuous. + +Related: `ci.docs-only-detect-shallow-safe` covers the other half of this script's shallow +behaviour — which revision it diffs against, and why it is `FETCH_HEAD` with a two-dot diff. diff --git a/docs/guard-inventory.md b/docs/guard-inventory.md index a9448242b..b22928ed6 100644 --- a/docs/guard-inventory.md +++ b/docs/guard-inventory.md @@ -213,7 +213,7 @@ recorded as unexamined rather than as cleared. | `scripts/check-required-contexts.sh` | the merge-consent hook, when `main`'s live required status checks no longer match `.gitea/required-status-contexts.json` (the snapshot `test_ci_dropped_step_guard.py` scopes itself to) | GUARD | MUTATION | `test_check_required_contexts.py::test_MUTATION_disarming_the_set_comparison_stops_every_drift_report` | | `scripts/check-review-verdict.sh` | the merge-consent hook's verdict classification | GUARD | BEHAVIOUR-ONLY | `test_check_review_verdict.py::test_falseopen_token_must_be_a_whole_word` | | `scripts/ci-detect-already-validated.sh` | nothing directly (feeds the skip gate) | GUARD | NONE | — | -| `scripts/ci-detect-docs-only.sh` | nothing directly (feeds the skip gate) | GUARD | NONE | — | +| `scripts/ci-detect-docs-only.sh` | nothing directly (feeds the skip gate), but its `--depth` decides whether a COMPLETE checkout is grafted shallow — which is what stamped every `:latest` image `InformationalVersion 0.0.0-` (#836) | GUARD | MUTATION | `test_docs_only_detector_clone_depth.py::test_the_push_arm_leaves_a_COMPLETE_clone_complete` | | `scripts/ci-peak-anon.sh` | nothing (samples container memory) | TOOLING | NONE | — | | `scripts/ci-prove-ban-detects.sh` | the release path, if the delimiter ban is disarmed | GUARD | NONE | — | | `scripts/ci-step-ran.sh` | the two required contexts, on a dropped step | GUARD | MUTATION | `test_ci_dropped_step_guard.py::test_dropping_ANY_single_step_FAILS_the_guard` | @@ -241,6 +241,7 @@ recorded as unexamined rather than as cleared. | `scripts/tests/test_ci_toolchain_image_resolves.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_decisions_lib.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_decisions_validate.py` | the `script-tests` job | PROOF | NONE | — | +| `scripts/tests/test_docs_only_detector_clone_depth.py` | the `script-tests` job | PROOF | NONE | — | | `scripts/tests/test_guard_inventory.py` | the `script-tests` job, on an unclassified guard or a stale proof ref | GUARD | MUTATION | `test_guard_inventory.py::test_the_inventory_covers_exactly_the_guards_that_exist` | | `scripts/tests/test_guard_populations_derive_from_git.py` | the `script-tests` job, on a guard whose file population admits a file git does not track | GUARD | MUTATION | `test_guard_populations_derive_from_git.py::test_no_derivation_admits_an_untracked_file` | | `scripts/tests/test_hook_fire_log.py` | the `script-tests` job, on a hook that stops reporting that it fired, or whose reporting changes what the harness sees | GUARD | MUTATION | `test_hook_fire_log.py::test_a_hook_that_LOSES_its_instrumentation_is_DETECTED` | @@ -370,8 +371,8 @@ clause-provable and the entry is regraded. ## What the numbers say -44 guards, 6 tooling scripts, 22 proof files. **22 guards carry a mutation proof; 6 are -behaviour-only; 16 have none.** These figures are asserted against the table by +44 guards, 6 tooling scripts, 23 proof files. **23 guards carry a mutation proof; 6 are +behaviour-only; 15 have none.** These figures are asserted against the table by `test_the_summary_counts_match_the_table`, because a hand-maintained summary of a table is a second copy of it — the duplication family this change argues against, and one that has already drifted here. @@ -582,9 +583,13 @@ groups, and the split is the point: the first two are backlogs, the third is not at all: it is one `grep -q '^Co-Authored-By:'` plus a `MERGE_HEAD` exemption, and nothing tests that the exemption fires only for merges. Neither is covered by what it calls — the trap is to read "it dispatches to guards" as "its own clauses are guarded". -- **`ci-detect-already-validated.sh`, `ci-detect-docs-only.sh`, `ci-prove-ban-detects.sh`, - `e2e-functional.sh`, `check-kickoff-guard.sh`, `test_ci_release_path_scan_job.py`** — not +- **`ci-detect-already-validated.sh`, `ci-prove-ban-detects.sh`, `e2e-functional.sh`, + `check-kickoff-guard.sh`, `test_ci_release_path_scan_job.py`** — not unattended either, but each needs its own harness rather than a proof written to clear a row. + `ci-detect-docs-only.sh` left this list in ersatztv#836 because a shipped defect gave it a real + red to reproduce, rather than because someone wrote a proof to fill the column. Its effect is + observable in-process after all — the graft it performs is a flag on a real clone — so a + skip-gate script is not automatically unprovable here. `ci-prove-ban-detects.sh` is the exception noted in `testing.guard-ships-with-mutation-proof`: it runs its own mutation at CI time, because what it proves is disarmable from inside pytest. diff --git a/scripts/ci-detect-docs-only.sh b/scripts/ci-detect-docs-only.sh index 11feb8581..40471212b 100755 --- a/scripts/ci-detect-docs-only.sh +++ b/scripts/ci-detect-docs-only.sh @@ -31,6 +31,55 @@ emit() { echo "-> docs_only=$1" } +# NEVER PASS `--depth` TO A COMPLETE CLONE (ersatztv#836). `git fetch --depth=N` GRAFTS one shallow: +# it writes `.git/shallow` and cuts history at N even though every object is already present. Both +# fetch sites below go through this one function, because they differ only in N. +# +# THE CONSUMERS, enumerated rather than assumed — `grep -rn ci-detect-docs-only .gitea/` returns +# exactly four call sites: `test`, `migrations` and `functional-e2e` (`fetch-depth: 2`, genuinely +# shallow) and `build` (`fetch-depth: 0`, complete). `api-docs` and `format` also have a step with +# `id: detect`, but it is their OWN inline diff, not this script. Enumerate by the grep above, never +# by the step id — that is the trap, and it points at the wrong two jobs. (The grep returns five +# hits: these four call sites plus docker-build.yml's own header comment.) +# +# `build` is therefore the only complete-clone consumer, and the push arm is the only arm it takes. +# That is where the defect was: the depth grafted its clone on every push to `main`, so the +# `git describe --tags` in the very next step ("Compute version and tags") found no reachable tag, +# and every `:latest` image carried `InformationalVersion 0.0.0-` instead of `26.x.y-`. +# +# THE PR ARM'S `--depth=200` HAS NO COMPLETE-CLONE CONSUMER TODAY. On `pull_request` the only +# consumers are the three `fetch-depth: 2` jobs, so that depth was inert, not latently firing. It is +# dropped anyway because the clause is shared: the next `fetch-depth: 0` consumer added to this arm +# would inherit the graft silently, and finding it again would cost what #836 cost. +# +# WHY THE DEPTH IS KEPT FOR SHALLOW CHECKOUTS, stated as measured rather than as load-bearing. A +# depth-LESS fetch into a `--depth=2` clone leaves `.git/shallow` in place, still writes FETCH_HEAD, +# and still lets `git rev-list --parents -n1 HEAD` report the parents the push arm reads — so +# dropping the depth outright would not have broken those three jobs either. It is retained because +# it is what they were given and nothing argues for widening their fetch, not because removing it +# was shown to break anything. +# +# Measured on a real `file://` clone: complete -> `git describe --tags --abbrev=0` = `v26.3.1`; after +# `git fetch --no-tags --depth=2` -> `.git/shallow` written, `fatal: No tags can describe`. +# +# ANY ANSWER BUT `true` PASSES NO DEPTH, and there is more than one such answer. `git rev-parse` +# fails outright where there is no readable repository; on a git older than 2.15 the flag is not +# recognised and rev-parse ECHOES IT BACK verbatim with status 0 (verified on 2.55: an unknown +# `--is-...` flag prints itself and exits 0). Neither is `true`, both take the no-depth branch, and +# in both a depth would have been useless anyway. Defaulting the other way would re-graft on exactly +# the paths nobody can observe. +fetch_ref() { # $1 = ref to fetch; $2 = depth to use ONLY IF this checkout is already shallow + local is_shallow + is_shallow="$(git rev-parse --is-shallow-repository 2>/dev/null || echo unknown)" + if [ "$is_shallow" = "true" ]; then + echo "checkout is shallow; fetching with --depth=$2 (ersatztv#836)" + git fetch --no-tags "--depth=$2" origin "$1" 2>/dev/null + else + echo "checkout is not shallow (is-shallow=$is_shallow); fetching without --depth (ersatztv#836)" + git fetch --no-tags origin "$1" 2>/dev/null + fi +} + # A release tag must NEVER be treated as docs-only, whatever it touches. if [ "${GITHUB_REF_TYPE:-}" = "tag" ]; then echo "tag build (${GITHUB_REF_NAME:-?}); never docs-only" @@ -56,7 +105,7 @@ case "$event" in emit false exit 0 fi - if ! git fetch --no-tags --depth=200 origin "$base" 2>/dev/null; then + if ! fetch_ref "$base" 200; then echo "fetch of base '$base' failed; running full matrix (safe default)" emit false exit 0 @@ -65,11 +114,16 @@ case "$event" in ;; push) # Only branch pushes reach here (tags handled above). Skip only when the range is UNAMBIGUOUS — - # HEAD is a merge commit (how every update to main lands): its first parent is the pre-merge - # tip, so HEAD^1 is the base. Fetch depth 2 so the first-parent tree exists in the shallow - # checkout; if it still can't be resolved/diffed we fall through to the full matrix. A non-merge - # (direct/multi-commit) push is ambiguous here -> full matrix. - git fetch --no-tags --depth=2 origin "${GITHUB_REF_NAME:-main}" 2>/dev/null || true + # HEAD is a merge commit: its first parent is the pre-merge tip, so HEAD^1 is the base. This is + # NOT how most updates to main land: 91 of the last 100 `origin/main` commits have ONE parent + # (measured 2026-08-29), so the arm below usually bails to the full matrix. Safe, and it means + # the docs-only skip rarely fires on a push at all. + # + # `fetch_ref ... 2` asks for depth 2 so the first-parent tree exists in a SHALLOW checkout, and + # for no depth at all in a complete one (see `fetch_ref`); if the base still can't be + # resolved/diffed we fall through to the full matrix. A non-merge (direct/multi-commit) push is + # ambiguous here -> full matrix. + fetch_ref "${GITHUB_REF_NAME:-main}" 2 || true nfields="$(git rev-list --parents -n1 HEAD 2>/dev/null | wc -w | tr -d ' ')" # 1 (self) + parents if [ "${nfields:-0}" -ge 3 ]; then base_rev="$(git rev-parse --verify -q 'HEAD^1' || true)" diff --git a/scripts/tests/mutation_manifest.py b/scripts/tests/mutation_manifest.py index d8143a5f6..1c53bf6b8 100644 --- a/scripts/tests/mutation_manifest.py +++ b/scripts/tests/mutation_manifest.py @@ -81,6 +81,21 @@ MUTATIONS: tuple[Mutation, ...] = ( why="`main()`'s only stale-detection logic, per the proof test's own docstring, which uses this " "exact clause and this exact replacement.", ), + Mutation( + guard="scripts/ci-detect-docs-only.sh", + target="scripts/ci-detect-docs-only.sh", + clause='if [ "$is_shallow" = "true" ]; then', + replacement="if true; then", + proof="test_docs_only_detector_clone_depth.py::test_the_push_arm_leaves_a_COMPLETE_clone_complete", + granularity=CLAUSE, + expect="GRAFTED the complete clone shallow", + why="The shallow test is the whole of ersatztv#836's fix: disarmed, `--depth=2` goes back to " + "every checkout including `build`'s complete one, which grafts it and makes the `git describe` " + "in the next step find no reachable tag. This row was UNDECLARED until #836 on the stated " + "grounds that the script 'feeds the skip gate, so its effect is visible only in a workflow " + "run' — untrue of this clause, whose effect is the shallow flag on a real clone and is " + "observable in-process, which is what the proof test asserts.", + ), Mutation( guard="scripts/ci-step-ran.sh", target="scripts/ci-step-ran.sh", @@ -438,8 +453,6 @@ UNDECLARED: dict[str, str] = { "scripts/ci-detect-already-validated.sh": "Blocks nothing directly — it feeds the skip gate. The " "consequence a mutation would have to be observed through is a job that skips, which is visible " "only in a workflow run.", - "scripts/ci-detect-docs-only.sh": "Same shape: it feeds the skip gate rather than blocking, so " - "its effect is visible only in a workflow run, not in this suite.", "scripts/ci-prove-ban-detects.sh": "Driven end to end by test_ci_release_path_scan_job.py, and " "it runs a mutation of its own at CI time. Grading it here needs a decision about what a second " "mutation would add; the row nominates no ref today.", diff --git a/scripts/tests/test_docs_only_detector_clone_depth.py b/scripts/tests/test_docs_only_detector_clone_depth.py new file mode 100644 index 000000000..16e4dfd86 --- /dev/null +++ b/scripts/tests/test_docs_only_detector_clone_depth.py @@ -0,0 +1,357 @@ +"""`scripts/ci-detect-docs-only.sh` must never graft a COMPLETE clone shallow (ersatztv#836). + +`git fetch --depth=N` writes `.git/shallow` and cuts history at N even when every object is already +present. Exactly four `docker-build.yml` jobs run the detector — `test`, `migrations` and +`functional-e2e` at `fetch-depth: 2`, and `build` at `fetch-depth: 0` — so `build` is the only +complete-clone consumer, and the detector asks the repository which shape it is in rather than +assuming. (`api-docs` and `format` have a step with `id: detect` too, but it runs their own inline +diff, not this script. Do not add them here.) + +WHAT WENT WRONG WITHOUT THIS. `build`'s next step after the detector is `Compute version and tags`, +whose `git describe --tags --abbrev=0` needs a reachable tag. The graft cut it off, `describe` +failed, and a `|| echo v0.0.0` fallback that cannot fail turned that into a version string: every +`:latest` image published from `main` carried `InformationalVersion 0.0.0-` instead of +`26.x.y-`, from 2026-07-17 (#416) until #836, with nothing red anywhere. It was found by +reading the string out of +a running container. That is why the assertion here is on the RESULTING VERSION and on the shallow +flag — not on the detector's exit status, which was `0` throughout the defect. + +WHY THE FIXTURE USES `file://`. `git clone /path` and `git fetch /path` use the local transport, +which IGNORES `--depth` outright. A fixture built on a plain path would therefore never graft, every +assertion below would hold, and the whole file would pass while testing nothing — +`testing.eight-ways-a-test-passes-for-the-wrong-reason`. `test_the_fixture_can_actually_graft` is the +negative control that keeps that honest: it performs the raw depth fetch the detector is forbidden to +perform and REQUIRES the graft to happen. If git or the transport ever stops grafting, that control +goes red and says so, instead of the other tests quietly becoming vacuous. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +REPO_ROOT = Path(__file__).resolve().parents[2] +DETECTOR = REPO_ROOT / "scripts" / "ci-detect-docs-only.sh" + +TAG = "v26.3.1" + + +def _env(**extra: str) -> dict[str, str]: + """Ambient `GIT_*` removed, everything else kept. + + Exported `GIT_DIR`/`GIT_WORK_TREE`/`GIT_INDEX_FILE` override `cwd` and would aim these git calls + at the real repository. The rest of `os.environ` is carried deliberately rather than built from + scratch: `conftest.py`'s isolation puts `ETV_HOOK_FIRE_LOG_DIR` there, and a from-scratch + environment would drop it and write into the production hook-fire log. + + `GITHUB_` is dropped alongside `GIT_` because it does NOT match the `GIT_` prefix + (`"GITHUB_OUTPUT".startswith("GIT_")` is False), and every `GITHUB_*` the detector reads decides + which arm it takes. Pinning only the one variable that bites today would leave the class open for + the next arm that starts reading a sibling — the whole prefix goes, and `_run_detector` puts back + exactly what each test means to set. + """ + env = { + k: v for k, v in os.environ.items() if not k.startswith(("GIT_", "GITHUB_")) + } + env.update(extra) + return env + + +def _git(args: list[str], cwd: Path) -> str: + r = subprocess.run( + ["git", *args], cwd=str(cwd), env=_env(), capture_output=True, text=True + ) + assert r.returncode == 0, f"git {' '.join(args)} failed: {r.stderr}" + return r.stdout.strip() + + +def _git_rc(args: list[str], cwd: Path) -> subprocess.CompletedProcess: + return subprocess.run( + ["git", *args], cwd=str(cwd), env=_env(), capture_output=True, text=True + ) + + +def _is_shallow(repo: Path) -> str: + return _git(["rev-parse", "--is-shallow-repository"], repo) + + +@pytest.fixture(scope="module") +def origin_url(tmp_path_factory) -> str: + """An origin shaped like ours: the newest tag is well back, and the tip is a merge commit. + + Both properties matter. The tag has to be further back than the fetch depth or the graft cannot + cut it off, and the tip has to be a merge commit or the detector's push arm reports + `non-merge push` and returns before reaching anything this file is about. + """ + root = tmp_path_factory.mktemp("detector-depth") + origin = root / "origin" + origin.mkdir() + _git(["init", "-q", "-b", "main", "."], origin) + _git(["config", "user.email", "test@example.com"], origin) + _git(["config", "user.name", "Test"], origin) + + for i in range(1, 13): + (origin / f"f{i}.txt").write_text(f"{i}\n") + _git(["add", "-A"], origin) + _git(["commit", "-q", "-m", f"c{i}"], origin) + _git(["tag", TAG], origin) + for i in range(13, 31): + (origin / f"f{i}.txt").write_text(f"{i}\n") + _git(["add", "-A"], origin) + _git(["commit", "-q", "-m", f"c{i}"], origin) + + # A docs-only branch merged with --no-ff, i.e. exactly how an update to `main` lands here. + _git(["checkout", "-q", "-b", "feature"], origin) + (origin / "docs").mkdir() + (origin / "docs" / "a.md").write_text("doc\n") + _git(["add", "-A"], origin) + _git(["commit", "-q", "-m", "docs change"], origin) + _git(["checkout", "-q", "main"], origin) + _git(["merge", "-q", "--no-ff", "feature", "-m", "merge feature (#1)"], origin) + + return f"file://{origin}" + + +@pytest.fixture(scope="module") +def deep_origin_url(tmp_path_factory) -> str: + """An origin with MORE than 200 commits after its newest tag, for the PR arm's `--depth=200`. + + Built separately, and the extra ~3s is the price of a test that is not vacuous. The other + fixture's 31 commits all fit inside a depth of 200, so a `--depth=200` fetch there has nothing + to cut: no `.git/shallow` is written, `describe` keeps resolving, and a PR-arm test built on it + passes against the UNFIXED script — measured, which is how this fixture came to exist. A guard + whose positive case cannot occur is the defect it is meant to catch, one level up. + """ + root = tmp_path_factory.mktemp("detector-depth-deep") + origin = root / "origin" + origin.mkdir() + _git(["init", "-q", "-b", "main", "."], origin) + _git(["config", "user.email", "test@example.com"], origin) + _git(["config", "user.name", "Test"], origin) + (origin / "a.txt").write_text("a\n") + _git(["add", "-A"], origin) + _git(["commit", "-q", "-m", "c0"], origin) + _git(["tag", TAG], origin) + # Empty commits: the depth boundary counts commits, and 210 real ones would only be slower. + for i in range(1, 211): + _git(["commit", "-q", "--allow-empty", "-m", f"c{i}"], origin) + return f"file://{origin}" + + +def _clone(origin_url: str, dest: Path, depth: int | None = None) -> Path: + args = ["clone", "-q"] + if depth is not None: + args += [f"--depth={depth}"] + args += [origin_url, str(dest)] + _git(args, dest.parent) + return dest + + +def _run_detector(repo: Path, tmp_path: Path, **github_env: str) -> tuple[subprocess.CompletedProcess, str]: + """Drive the real detector. + + `GITHUB_REF_TYPE` is pinned to `branch` rather than left to the ambient environment. `_env()` + deliberately carries the rest of `os.environ` (for `ETV_HOOK_FIRE_LOG_DIR`), and the detector's + FIRST branch returns early on `GITHUB_REF_TYPE=tag` — before any fetch — so an exported `tag` + turns every test here red for a reason that has nothing to do with what they assert. + + Not live today, stated precisely rather than dramatised: the `scan` job DOES run pytest on tag + pushes, but over two named files, and this is not one of them; `script-tests`, which does run + this file, is `on: pull_request`. Adding this file to `scan`'s argument list is the one edit that + would make it live. It is a default (`env.update(github_env)` follows), so a future test can + still drive the tag arm deliberately. + """ + out_file = tmp_path / f"gh-output-{repo.name}" + out_file.write_text("") + env = {"GITHUB_REF_TYPE": "branch"} + env.update(github_env) + r = subprocess.run( + ["bash", str(DETECTOR)], + cwd=str(repo), + env=_env(GITHUB_OUTPUT=str(out_file), **env), + capture_output=True, + text=True, + ) + return r, out_file.read_text() + + +# -------------------------------------------------------------------------------------------- +# The negative control. Everything else here asserts that a graft does NOT happen, so it is all +# vacuous if the fixture cannot graft in the first place. +# -------------------------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "fixture_name, depth", + [("origin_url", 2), ("deep_origin_url", 200)], + ids=["push-arm-depth-2", "pr-arm-depth-200"], +) +def test_the_fixture_can_actually_graft(request, fixture_name, depth, tmp_path): + """The raw `--depth=N` fetch the detector must not perform DOES graft this fixture. + + Parametrised over BOTH fixtures and BOTH depths, because a control that covers only one of + them leaves the other free to go vacuous unreported — which is what happened: the PR arm was + first written against the 31-commit origin, where a depth of 200 has nothing to cut, and it + passed against the unfixed script. One control per (fixture, depth) pair the file relies on. + """ + origin = request.getfixturevalue(fixture_name) + repo = _clone(origin, tmp_path / f"control-{depth}") + assert _is_shallow(repo) == "false", "a plain clone should be complete" + assert _git(["describe", "--tags", "--abbrev=0"], repo) == TAG + + _git(["fetch", "--no-tags", f"--depth={depth}", "origin", "main"], repo) + + assert _is_shallow(repo) == "true", ( + f"a --depth={depth} fetch did not graft this fixture, so the test that relies on it is " + "vacuous — most likely the clone/fetch fell back to the local transport, which ignores " + "--depth (use file://), or the fixture has fewer commits than the depth" + ) + described = _git_rc(["describe", "--tags", "--abbrev=0"], repo) + assert described.returncode != 0, ( + "the graft happened but `git describe` still resolved, so the version this file protects " + "is not actually reachable from the shallow boundary — the fixture's tag is too close to " + f"the tip. stdout={described.stdout!r}" + ) + + +# -------------------------------------------------------------------------------------------- +# The guard proper. +# -------------------------------------------------------------------------------------------- + + +def test_the_push_arm_leaves_a_COMPLETE_clone_complete(origin_url, tmp_path): + """`build`'s shape: `fetch-depth: 0`, push to main. The version must survive the detector. + + This is the proof named by `scripts/ci-detect-docs-only.sh`'s `MUTATION` row in + `docs/guard-inventory.md`; the declared mutation in `mutation_manifest.py` disarms the shallow + condition, which puts the `--depth=2` fetch back and reddens the two assertions below. + """ + repo = _clone(origin_url, tmp_path / "build-shape") + assert _is_shallow(repo) == "false" + + r, output = _run_detector( + repo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main" + ) + assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}" + + assert _is_shallow(repo) == "false", ( + "the detector GRAFTED the complete clone shallow — this is ersatztv#836: the next step in " + "`build` is `Compute version and tags`, whose `git describe` then finds no reachable tag " + f"and the image ships InformationalVersion 0.0.0-.\ndetector said:\n{r.stdout}" + ) + + described = _git_rc(["describe", "--tags", "--abbrev=0"], repo) + assert described.returncode == 0 and described.stdout.strip() == TAG, ( + "after the detector, `git describe --tags --abbrev=0` no longer resolves, so `build` would " + f"stamp this image 0.0.0. git said: {described.stdout.strip()!r} {described.stderr.strip()!r}" + ) + + # The detector still has to do its actual job on this shape. + assert "docs_only=true" in output, ( + f"the merge tip is docs-only but the detector did not say so:\n{r.stdout}\n{output}" + ) + + +def test_the_pull_request_arm_leaves_a_COMPLETE_clone_complete(deep_origin_url, tmp_path): + """A complete checkout on the PR arm, N=200. NO PRODUCTION CONSUMER TODAY — deliberately. + + Said plainly because the first version of this docstring got it wrong: on `pull_request` the only + jobs running this script are the three `fetch-depth: 2` ones, so the PR arm's `--depth=200` was + inert rather than latently firing. What this pins is the SHARED CLAUSE — the next `fetch-depth: 0` + consumer added to this arm must not silently inherit the graft that cost #836. It is a guard + against a future shape, not a reproduction of a shipped one, and grading it as the latter would + overstate what the file proves. + + The deep fixture is what makes the case real at all: with the 32-commit origin a depth of 200 has + nothing to cut, and this test passed against the unfixed script. + """ + repo = _clone(deep_origin_url, tmp_path / "pr-shape") + assert _is_shallow(repo) == "false" + # A PR head: one docs commit on top of the base branch. + _git(["checkout", "-q", "-b", "pr-branch"], repo) + (repo / "docs").mkdir(exist_ok=True) + (repo / "docs" / "b.md").write_text("doc\n") + _git(["add", "-A"], repo) + _git(["commit", "-q", "-m", "docs on the PR head"], repo) + + r, output = _run_detector( + repo, tmp_path, GITHUB_EVENT_NAME="pull_request", GITHUB_BASE_REF="main" + ) + assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}" + + assert _is_shallow(repo) == "false", ( + "the PR arm GRAFTED a complete clone shallow (ersatztv#836). No job runs this arm against a " + f"complete checkout today, so this is the shared clause regressing.\n{r.stdout}" + ) + described = _git_rc(["describe", "--tags", "--abbrev=0"], repo) + assert described.returncode == 0 and described.stdout.strip() == TAG, ( + "after the PR arm, `git describe --tags --abbrev=0` no longer resolves: " + f"{described.stdout.strip()!r} {described.stderr.strip()!r}" + ) + # FETCH_HEAD is what the PR arm diffs against, so the depth-less fetch must still write it. + assert _git_rc(["rev-parse", "--verify", "-q", "FETCH_HEAD"], repo).returncode == 0, ( + "the PR arm no longer writes FETCH_HEAD, so its base revision is unresolvable" + ) + assert "docs_only=true" in output, ( + f"the PR head is docs-only but the detector did not say so:\n{r.stdout}\n{output}" + ) + + +def test_a_SHALLOW_checkout_still_gets_its_depth_and_still_decides(origin_url, tmp_path): + """The other half: `test`/`migrations`/`functional-e2e` at `fetch-depth: 2` must be unchanged. + + Removing the graft must not turn into removing the depth. A shallow checkout stays shallow, and + the detector still resolves `HEAD^1` and classifies the docs-only merge — the behaviour the + `--depth=2` was added for in the first place. + """ + repo = _clone(origin_url, tmp_path / "shallow-shape", depth=2) + assert _is_shallow(repo) == "true", "the fixture clone was meant to be shallow" + + r, output = _run_detector( + repo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main" + ) + assert r.returncode == 0, f"detector failed: {r.stdout}{r.stderr}" + + assert _is_shallow(repo) == "true", "a shallow checkout was silently unshallowed" + assert "docs_only=true" in output, ( + "the shallow checkout no longer classifies the docs-only merge — the depth stopped being " + f"applied where it IS needed:\n{r.stdout}\n{output}" + ) + assert "fetching with --depth=2" in r.stdout, ( + f"the shallow checkout did not take the depth branch:\n{r.stdout}" + ) + assert "fetching without --depth" not in r.stdout, ( + f"the shallow checkout took the complete-clone branch:\n{r.stdout}" + ) + + +def test_an_UNKNOWN_shallow_answer_passes_no_depth(tmp_path): + """Fail-direction, stated and executed: `--is-shallow-repository` unanswerable => no depth. + + Defaulting the other way would re-graft on exactly the paths nobody can observe, so the branch + is pinned rather than argued. Running outside a repository is one way to make the query + unanswerable and the one this drives; it is NOT the only one — on a git older than 2.15 the flag + is unrecognised and rev-parse echoes it back verbatim with status 0. That shape is not covered + here (it needs an ancient git), and it reaches the same no-depth branch. + + WHAT THIS DELIBERATELY DOES NOT ASSERT. Outside a repository the detector exits 128 with an + empty `$GITHUB_OUTPUT`, because `set -o pipefail` makes the `git rev-list ... | wc -w` assignment + below the fetch abort under `set -e`. That is PRE-EXISTING and unchanged here — measured against + `8aeacd534`, the commit this fix branched from, which exits 128 the same way. It is also not a + fail-open: the step goes red, so the job does, and no `docs_only=true` is ever emitted. Asserting + a clean exit here would be asserting a behaviour the script has never had. + """ + notarepo = tmp_path / "notarepo" + notarepo.mkdir() + r, _output = _run_detector( + notarepo, tmp_path, GITHUB_EVENT_NAME="push", GITHUB_REF_NAME="main" + ) + assert "is-shallow=unknown" in r.stdout, ( + f"the unknown answer was not reached or not reported:\n{r.stdout}" + ) + assert "fetching without --depth" in r.stdout, ( + f"an unknown shallow answer must not ask for a depth:\n{r.stdout}" + )