Skip to content

Add a zero-tolerance TypeDoc documentation gate - #436

Open
leynos wants to merge 19 commits into
mainfrom
typedoc-rollout
Open

Add a zero-tolerance TypeDoc documentation gate#436
leynos wants to merge 19 commits into
mainfrom
typedoc-rollout

Conversation

@leynos

@leynos leynos commented Jul 17, 2026

Copy link
Copy Markdown
Owner

Summary

This branch adds a zero-tolerance TypeDoc documentation gate to make all and
pull-request CI. It covers the three maintained JavaScript and TypeScript
surfaces: frontend-pwa/src, packages/types/src, and the JavaScript token
tooling under packages/tokens.

Each surface keeps its TypeDoc configuration beside its TypeScript
configuration. The root docs:check script chains all three configurations,
and make docs-check runs the script. Every run uses emit: "none", treats
validation warnings as errors, and writes no documentation artefacts. Generated
Orval clients, generated declarations, tests, fixtures, and generated output
remain excluded.

TypeDoc is pinned to 0.28.20 exactly in the root manifest and resolved to that
version in both pnpm-lock.yaml and bun.lock. A caret range would let a
minor release change the gate's verdict without a reviewed commit.

Base

Rebased onto main now that #485 has merged as 9108047. This branch was
stacked on it while it was open; those commits are gone from the diff and the
19 that remain are this pull request's own.

Rebase

Rebased onto main at c48ad2b, through #485 while it was open, and onto
main again at 9108047 once it merged. The pre-rebase head was
7e62a49; the branch had been conflicting
since July, so GitHub had stopped computing a merge commit and the
pull_request workflows were no longer running against it.

The fourth commit, "Repair gate-discovered validation failures", was almost
entirely superseded during the rebase. main has since landed the same
Whitaker helper-panic fixes, the same uv rework in the Makefile, the same
local_k8s test isolation, and the same "on": quoting in
dependabot-automerge.yml, all independently. What survived the structural
merge was damaged: a duplicated "on": mapping and a backend/src/doc.rs
carrying repeated import lists and a module doc comment spliced into the middle
of the file. A commit on top restores those five files to match main exactly.

What remained after that was an unrelated specification edit dropping the
ToSchema derives from ListEnrichmentProvenanceQuery and
ListOfflineBundlesQuery so the two unused component schemas leave
spec/openapi.json. Redocly reports those as warnings rather than errors and
make lint-openapi passes on main with them present, so that is a
specification cleanup rather than a gate repair. It is tracked as #484 and is
not carried here.

Changes on top of the original branch

  • TypeDoc pinned to an exact version, with contract tests asserting the pin's
    shape and each lockfile's resolution of it.
  • The CI contract now matches the gate's command rather than the step's name.
    A step name is prose: renaming it, or deleting the step beside a similarly
    named neighbour, satisfied a name-shaped assertion. It now searches the
    build job for a step whose command line is make docs-check, and asserts
    that step carries neither an if guard nor continue-on-error. Both
    mutations, replacing the command while keeping the name and deleting the step
    outright, fail the contract.
  • A parametrized contract asserts each of the three real configurations sets
    validation.notDocumented, treats warnings as errors, emits nothing, and
    lists at least one required declaration kind. The fixture test proved TypeDoc
    honours the policy but said nothing about the configurations that ship.
  • packages/tokens/typedoc.json gained the excludeInternal, excludePrivate
    and excludeProtected settings the other two surfaces already carried.
  • docs-check names deps as a prerequisite, so a clean checkout gets a
    documentation verdict rather than a missing-binary error.
  • Link validation is enabled on all three surfaces, so a {@link} naming a
    symbol that does not exist now fails the gate. All three already passed with
    it on. Pointing usersQueryKeys.all at a symbol that does not exist makes
    typedoc --options frontend-pwa/typedoc.json exit 4. notExported stays
    off; it reports a different thing.
  • The Makefile contract parses the all target's prerequisite list instead of
    matching its whole line, so reordering an unrelated prerequisite no longer
    fails a contract that is about docs-check.
  • The developers' guide gains the declaration kinds each surface requires, the
    pinning rule, a local run recipe, and a worked example of documenting an
    export.

Validation

Run from the rebased branch:

  • make docs-check: all three configurations pass and emit no files.
  • Removing one export's JSDoc block in each surface in turn fails that
    surface's configuration, naming the declaration:
    api/client.usersQueryKey, UserIdSchema, and
    src/utils/resolve-token.resolveToken.
  • Enabling link validation, then pointing a {@link} at a symbol that does
    not exist, makes the frontend configuration exit 4.
  • A file under frontend-pwa/src/api/generated/ with an undocumented export
    does not fail the gate, so the Orval exclusion holds.
  • make test-workflow-contracts: 120 passed.
  • make check-fmt, make typecheck, make markdownlint, make nixie,
    make spelling, make lint-openapi, make lint-actions,
    make lint-makefile, make yamllint: pass.
  • No Rust, Cargo.lock or Cargo.toml delta against main.
  • CI run 34068655093 on 1b83985: build and coverage both green, with the
    TypeDoc gate step's log showing all three configurations running.

The four pre-existing failures previously listed here are fixed in #485, which
this branch now sits on, so every gate above is green with no known exceptions.

Notes

References

Summary by Sourcery

Enforce zero-tolerance TypeDoc validation across maintained JavaScript and TypeScript APIs in local aggregate checks and pull-request CI.

New Features:

  • Add a zero-tolerance TypeDoc gate covering the frontend, shared types, and token tooling surfaces, with no generated documentation output.

Enhancements:

  • Integrate documentation validation into make all and unconditional pull-request CI.
  • Pin TypeDoc to an exact version and enforce configuration, scope, lockfile, and CI contracts.
  • Enable validation for missing documentation, broken links, unknown tags, and required declaration kinds across all maintained surfaces.

Build:

  • Add the docs:check package script and make docs-check target, including dependency installation as a prerequisite.

CI:

  • Run make docs-check in the CI build job and add workflow contract coverage for the gate.

Documentation:

  • Document the TypeDoc gate, required declaration coverage, local usage, version pinning, exclusions, and guidance for documenting exports.

Tests:

  • Add static and behavioral contract tests covering TypeDoc configuration, lockfile resolution, CI integration, failure behavior, exclusions, and artifact-free execution.

Chores:

  • Add missing JSDoc documentation to selected frontend, shared type, and token-tooling exports.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary

  • Add strict, zero-output TypeDoc checks for the frontend, shared types and token utilities.
  • Integrate docs-check into make all and pull-request CI.
  • Pin TypeDoc and TypeScript dependencies with synchronised lockfiles.
  • Document previously undocumented declarations and add dedicated TypeDoc configurations.
  • Add contract and behaviour tests for TypeDoc policy, CI integration, Makefile failure propagation and dependency resolution.
  • Add a standalone workflow-linting CLI with pinned dependencies and tests.
  • Update contributor guidance and docs/wildside-pwa-design.md with TypeDoc requirements.
  • Confirm validation passes, including the TypeDoc gate and workflow contracts.
  • No new execplan, issue, ADR or RFC was added.

Walkthrough

Add zero-output TypeDoc validation for workspace packages, document exported entities, and include docs-check in the default Makefile and CI quality workflow. Add deterministic action-lint tooling, strengthen Makefile failure propagation, and update workflow contract tests.

Changes

Quality gates and lint tooling

Layer / File(s) Summary
TypeDoc configuration and documentation
package.json, frontend-pwa/typedoc.json, packages/tokens/*, packages/types/*, frontend-pwa/src/*
Configure three zero-output TypeDoc surfaces and document exported declarations.
TypeDoc contract and behaviour validation
tests/workflow_contracts/documentation_gate*, tests/workflow_contracts/fixtures/*
Validate configuration, version pinning, documented exports, failure diagnostics, and output behaviour.
Makefile and CI gate wiring
Makefile, .github/workflows/ci.yml, .gitignore, .markdownlint-cli2.jsonc, docs/*
Run documentation checks and action-lint tests through Makefile and CI gates, and document the checks.
Action-lint CLI and test suite
scripts/lint_actions.py, scripts/tests/test_lint_actions.py, tests/workflow_contracts/makefile_tooling_test.py
Discover lint targets, run deterministic pinned tools, stop on failure, and validate the CLI with mocked and property-based tests.
Makefile failure propagation and contract maintenance
tests/workflow_contracts/makefile_failure_propagation_test.py, scripts/makefile-audit.test.mjs, tests/workflow_contracts/tool_installation_test.py, .redocly.lint-ignore.yaml
Verify shell failure propagation and update audit, installation, temporary-directory, and lint-ignore contracts.

Sequence Diagram(s)

sequenceDiagram
  participant CI
  participant Makefile
  participant pnpm
  participant TypeDoc
  CI->>Makefile: run docs-check
  Makefile->>pnpm: run docs:check
  pnpm->>TypeDoc: load three configurations
  TypeDoc-->>pnpm: return validation status
  pnpm-->>Makefile: complete docs-check
  Makefile-->>CI: return gate status
Loading

Change: Feature

Merge Risk: 🟡 Moderate · up to 9b902

The new quality gates can either miss action files when traversal fails or fail the repository lint job on the added suppressions. These issues should be corrected before merge.


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Testing (Overall) ❌ Error The new CI contract tests do not fully guard the changed quality gates. _assert_gate_runs_unconditionally checks pull_request, the exact run command, and absence of if, but it does not reject … Strengthen _assert_gate_runs_unconditionally to reject continue-on-error on both the selected job and the matched step, and add mutation coverage that proves each addition fails the contract. Add a Makefile contract for `test-lint-actio…
✅ Passed checks (14 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 97.30% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 74 functions across 16 files. (8 skipped: 8…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
User-Facing Documentation ✅ Passed PASS — The check is not applicable. The diff adds repository developer tooling and CI quality gates: make docs-check, TypeDoc configuration, action-lint tooling, tests, and contributor guidance in `…
Developer Documentation ✅ Passed Pass the Developer documentation check. The changed developer guide documents the TypeDoc gate, covered surfaces and exclusions, validation policy, exact pinning, local commands, CI and Make integrati…
Module-Level Documentation ✅ Passed Pass the module-level documentation check. Every added executable module has clear module-level documentation: scripts/lint_actions.py documents its linting purpose, Makefile relationship, behaviour…
Testing (Unit And Behavioural) ✅ Passed Pass the testing check. The pull request adds meaningful tests at the correct boundaries: real TypeDoc subprocess tests cover pass, no-emission, undocumented exports, broken links, unknown tags, and c…
Testing (Property / Proof) ✅ Passed The change introduces ordering and failure-propagation invariants in scripts/lint_actions.py. scripts/tests/test_lint_actions.py covers generated action/workflow sets and generated failure positio…
Testing (Compile-Time / Ui) ✅ Passed Pass the check. The changed TypeScript and JavaScript source adds JSDoc comments only; exported signatures, types, runtime statements, and UI behaviour do not change. The generated declaration diff al…
Unit Architecture ✅ Passed Pass the Unit Architecture check. The changed application files add documentation only; they do not add query, command, network, storage, clock, or mutation behaviour. The new `scripts/lint_actions.py…
Domain Architecture ✅ Passed Accept this change for Domain Architecture. The actual diff changes documentation comments, TypeDoc configuration, Make targets, CI gates, lockfiles, lint tooling, and tests. The only domain-facing fi…
Observability ✅ Passed PASS. The pull request does not change production runtime behaviour. The runtime-facing diffs add JSDoc only. The new behaviour is limited to CI and build-time documentation and lint gates. `scripts/l…
Title check ✅ Passed Accept the title. It clearly identifies the main change: adding a zero-tolerance TypeDoc documentation gate. No roadmap or issue reference is required by the supplied context.
Description check ✅ Passed Accept the description. It directly explains the TypeDoc gate, its scope, CI and Makefile integration, configuration, tests, and validation results.
Full details: Testing (Overall)

Explanation

The new CI contract tests do not fully guard the changed quality gates. _assert_gate_runs_unconditionally checks pull_request, the exact run command, and absence of if, but it does not reject continue-on-error on the job or gate step. Adding continue-on-error: true to either new CI gate would therefore let the workflow pass while the documentation gate or its tests fail. The PR description explicitly claims that this condition is checked, but tests/workflow_contracts/ci_workflow_test.py:30-84 does not check it. The new test-lint-actions Make target also has no Makefile contract: the CI test only checks that make test-lint-actions is named, so replacing that target with a no-op would still satisfy the contract and skip the new test suite.

Resolution

Strengthen _assert_gate_runs_unconditionally to reject continue-on-error on both the selected job and the matched step, and add mutation coverage that proves each addition fails the contract. Add a Makefile contract for test-lint-actions that verifies the venv creation, pinned dependency installation, pytest target, and cmd_mox.pytest_plugin; ensure a no-op or incomplete recipe fails that contract.


TypeDoc checks each public sign,
JSDoc keeps the entries fine.
Make stops when commands fail,
CI guards the documentation trail.
Lint tools march in a stable line.

Comment @coderabbitai help to get the list of available commands.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review July 26, 2026 22:56

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ddfe8b22a2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile
Comment thread Makefile
Comment thread packages/tokens/src/utils/resolve-token.js
Comment thread frontend-pwa/src/api/fetcher.ts
@buzzybee-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@frontend-pwa/src/api/fetcher.ts`:
- Around line 178-181: Update the documentation for customFetchParsedSafe to
clarify that safeParse returns a result only for schema-validation failures,
while transport errors and non-2xx responses from customFetch still reject.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: e59c38df-dc1c-4ca9-b25a-e92025e65a60

📥 Commits

Reviewing files that changed from the base of the PR and between 05d2895 and ddfe8b2.

⛔ Files ignored due to path filters (2)
  • packages/types/dist/src/user.d.ts is excluded by !**/dist/**
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (12)
  • Makefile
  • frontend-pwa/src/api/client.ts
  • frontend-pwa/src/api/fetcher.ts
  • frontend-pwa/src/app/App.tsx
  • frontend-pwa/typedoc.json
  • package.json
  • packages/tokens/build/validate-contrast.js
  • packages/tokens/src/utils/resolve-token.js
  • packages/tokens/tsconfig.typedoc.json
  • packages/tokens/typedoc.json
  • packages/types/src/user.ts
  • packages/types/typedoc.json
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/cuprum (auto-detected)
  • leynos/rstest-bdd (auto-detected)
  • leynos/pg-embed-setup-unpriv (auto-detected)
  • leynos/ortho-config (auto-detected)

Comment thread frontend-pwa/src/api/fetcher.ts
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos changed the base branch from main to makefile-shellflags-ec September 7, 2026 02:37
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos changed the base branch from makefile-shellflags-ec to main September 7, 2026 02:54
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@wafflecat-df12

Copy link
Copy Markdown
Collaborator

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

leynos added a commit that referenced this pull request Sep 8, 2026
…e head (#485)

* Clear the three Ruff findings the Python gate was hiding

`make lint-python` has been printing these while exiting zero, because the
Makefile's global `.ONESHELL` gives a recipe its last command's status and
Ruff is not the last command. The next commit closes that hole, so these
have to go first or it lands on a red tree.

`cache_ownership_test.py` imported `pathlib` and never used it.

`tool_installation_test.py` conflated two claims in one assertion: that
the Makefile invokes `cargo binstall` at all, and that every invocation
carries `--force`. Split, an empty list now says "no invocation found"
rather than reporting a missing flag. The other finding is a long line,
rewrapped.

* Give the Makefile tooling contracts their own TMPDIR

Two contracts asserted the command doubles saw an empty `TMPDIR`. Nothing
in the Makefile sets or clears `TMPDIR`, so that assertion was reading
the ambient environment: it held on GitHub's runners, which set none, and
failed on any developer machine that does. The suite reported a Makefile
defect that did not exist.

The fixture now owns a `TMPDIR` under the test's own temporary directory
and both contracts assert the tools receive exactly that. The claim
becomes "the Makefile passes the caller's TMPDIR through untouched",
which is about the Makefile rather than about whoever ran the suite, and
it holds with `TMPDIR` unset, set to the fixture's value, or set to
anything else.

* Expect both cargo-audit ignores in the audit contract

#473 added `--ignore RUSTSEC-2026-0258` to `CARGO_AUDIT_IGNORES` without
updating this suite, so its exact-match assertion has been failing since.
`make test-frontend` runs `pnpm run test` before `pnpm run test:workspaces`
and the Makefile's global `.ONESHELL` hands the target the last command's
status, so the failure never reached the gate.

Both ignores stay. Each is justified in the Makefile's header comment with
its own review date, and RUSTSEC-2026-0258 has no patched release for
actix-http's h2 dependency.

The expected command is now a named constant used by all three
assertions. The two that used `toContain` matched only the first ignore,
so dropping the second passed them; against the full command a dropped
suppression fails all three.

* Retire the three expired Redocly lint exceptions

All three carried `review by: 2026-08-31`, which passed a week ago.
`scripts/check_redoc_ignore.py` has been reporting them, but it is not the
last command in the `lint-openapi` recipe and the Makefile's global
`.ONESHELL` hands the target the last command's status, so the gate stayed
green over an expired exception.

The review the date was demanding finds all three obsolete rather than due
for renewal:

- `no-empty-servers` on `#/servers`: the specification now declares a
  server, `/`, described as relative to the deployment base URL.
- `operation-4xx-response` on both health probes: each documents a 405
  alongside 200 and 503, so each carries a 4xx response.

Redocly lints the specification cleanly with no exceptions at all, and
emptying `servers` still trips `no-empty-servers`, so the rule is live and
the exception was simply spent. The two remaining warnings are the unused
component schemas tracked in #484.

The `spec/openapi.json` key stays, because the recipe asserts the
specification has an entry here, and the file gains a header recording
what the exceptions were and why they went.

* Make a failing recipe line fail its Make target

`.ONESHELL` is a global special target: GNU make ignores its prerequisite
list, so the declaration naming `prepare-pg-worker` documents which recipe
needed one-shell recipes but turns them on for the whole file. Every
multi-line recipe reaches the shell as a single script, and under make's
default `.SHELLFLAGS` of `-c` that script's status is its last command's
status. Every earlier failure is discarded.

The symptom is the worst one a gate can have. Run 33939820204 went green
with `make lint-python` printing Ruff findings, and `make lint-openapi`
passed for a week over an expired review-by annotation, because in each
recipe the failing tool was not the last command. The four commits before
this one clear everything the flag surfaces, so it lands on a green tree.

`.SHELLFLAGS := -ec` makes the shell abort at the first failing command.
The `c` stays: make passes the recipe to the shell as a command string. No
recipe in this Makefile relies on a non-zero intermediate status; every
tolerated failure is already inside an `if`, a `||`, or a captured status,
and no recipe line carries make's `-` prefix. A recipe that later needs a
command to be allowed to fail should say so itself rather than have this
flag weakened for it.

`makefile_failure_propagation_test.py` holds the contract. It builds a
scratch Makefile from the repository's own prologue lines plus a probe
target whose first line fails and last line succeeds, drives GNU make over
it, and asserts the target fails. A companion test removes the
`.SHELLFLAGS` line from that same prologue and asserts the identical probe
passes, so the first test's verdict is attributable to the flag and not to
a mistake in the probe. A third pins the assumption the whole thing rests
on, that `.ONESHELL` applies to targets it does not name.

Deleting the flag fails the contract; weakening it to `-c` fails both the
declaration check and the behavioural probe, the latter reporting exit 0
with the probe's failing line having run.

* Fail pipelines at the head as well as recipes at the line

`-ec` alone was half a fix. It aborts at the first failing command, which
catches a tool that is not the recipe's last line, but a pipeline still
reports its LAST stage's status, so a failure at the head is discarded
exactly as before.

That is not hypothetical here. This Makefile pipes into the tool that does
the checking: `spelling` feeds `git ls-files` into typos, and
`lint-actions` feeds `find` into yamllint and actionlint. A head that dies
produces an empty list, and the gate passes having examined nothing.

`-o pipefail` gives a pipeline its first failing stage's status. Neither
option covers the other, so both are needed and neither can be dropped.

The contract now drives two probes. `earlier-line` fails on a line that is
not the last; `pipeline-head` fails in a pipeline's first stage while its
last stage succeeds. Each is mutation-proved against the half-measure that
masks it: with `-ec` alone `pipeline-head` passes, and with
`-o pipefail -c` alone `earlier-line` passes. A third case asserts the
copied prologue still declares `.ONESHELL`, without which make would run
each line in its own shell and both probes would pass under any flags.

The guide no longer offers make's `-` line prefix as an escape hatch for a
recipe that needs a tolerated failure. Under `.ONESHELL` it applies to the
first recipe line only, so on any later line it silently does nothing.

Reported by the review round on lille #345.

* Address the review round on the failure-propagation contract

Match `--force` as a whole argument. The comment above the assertion
already warned that `--force-exclude` elsewhere in the Makefile would
satisfy a search for the flag, and then the assertion did exactly that
search. Splitting the line into arguments makes the check match the rule it
serves: rewriting the invocation to `--force-exclude` now fails the
contract, where before it passed.

Give `_make` a single-line docstring, per the numpy convention for private
helpers, and move its rationale to a comment above the definition.

Caption the settings table. The documentation style guide puts the caption
below the table, so it goes there rather than above it.

* Guard each linter in lint-actions on its own

Two shapes in this recipe lose a command's status without help, and both
are invisible in the output. `action-validator` runs once per composite
action, so an earlier iteration's failure is replaced by the last one's
status; the loop is dead today with no `.github/actions` directory and
goes live the day one appears. The workflows branch runs
`find | xargs yamllint` and then `find | xargs actionlint` inside one
`if`, where the second's status replaces the first's.

`.SHELLFLAGS`'s `-e` covers both today. Each invocation carries
`|| exit 1` anyway: the guard states the intent where the reader meets it,
and it survives a future change to the flag that these shapes would not.

Three contracts hold it, because no single one can. A static assertion
requires every linter invocation in the recipe to carry a guard, and
dropping any one of the three fails its own case. A behavioural pair
drives the real recipe with a failing first linter and a passing second,
with the all-passing case beside it so a failure is attributable. A probe
in `makefile_failure_propagation_test.py` switches `-e` off with `set +e`
and measures the guard alone, since no behavioural test can tell the guard
and the flag apart while both are present; its unguarded twin must still
pass, or the probe is measuring something else.

Reported by env-wildside's loop audit.

* Move the lint-actions gate logic out of the recipe

The `|| exit 1` guards were interim. The estate's scripting standards say
gate logic of this size does not belong in a recipe at all, and this recipe
had two shapes that lose a command's status: `action-validator` in a loop
over composite actions, and `find | xargs yamllint` followed by
`find | xargs actionlint` in one `if`. Guards paper over both; a script
removes them.

`scripts/lint_actions.py` builds the list of invocations first and runs
them in one loop, so "the first failure wins" is a property of four lines
rather than of control flow spread through a shell fragment. It reports the
tool the reader cares about rather than the one that ran it: yamllint
arrives through `uvx`, and naming `uvx` would send them to the wrong place.
The recipe is now one command.

`scripts/tests/test_lint_actions.py` places a failing tool first, a failing
tool last, and no failing tool at all, with cmd-mox supplying the
executables. The plan's ordering is asserted separately, since without it
"the first tool failed" is not a claim the failure tests can make.

The contract over the recipe changes shape with it: instead of asserting
each invocation carries a guard, it asserts the recipe runs exactly one
command besides its tool checks, that the command invokes the script with
the yamllint pin, and that it contains no `while`, `;`, `&&` or `||`, any
of which would reintroduce the defect. The shell probes for the guarded
shapes retire; the ordering they stood in for is now tested directly.

Two things this turned up, both recorded in the guide for the next script.
cuprum 0.1.0 has no `Catalogue.from_programs`, which is what
`docs/scripting-standards.md` shows; a `ProgramCatalogue` is built from a
`ProjectSettings` and passed to `sh.make`, and program names are the
`Program` type rather than plain strings. And cmd-mox needs an interpreter
that can import it on the shim's PATH, which a layered `uv run --with`
environment is not: the shim hangs rather than failing. `test-lint-actions`
builds a materialized virtual environment instead, as `typecheck-python`
does, which is also why the new dependencies join the typecheck set.

* Run the lint-actions script tests in CI

The new suite reached `make test` but not the workflow, which invokes the
individual test targets rather than the aggregate. It therefore passed
locally and never ran on a pull request, which is the quietest way for a
test to be useless.

The contract matches the command rather than the step name and asserts the
step is unconditional; deleting the step fails it.

* Stream the linters' output through cuprum's echo

The script relayed both captured streams after each tool finished, which
made a slow linter look hung and reordered its output against the script's
own messages. `run_sync(echo=True)` mirrors them as the tool writes them,
and capture stays on so a failure can still quote stderr.

The skip notices are flushed for the same reason: cuprum's echo writes to
the file descriptor directly while print buffers, so without a flush the
notice arrived after the output it introduces.

The guide records that this is the repository's first cuprum script, that
the three plumbum scripts are the migration the standard anticipates, and
the two upstream reports this work produced: leynos/concordat#154 for the
standard's examples not matching cuprum's shipped API, and
leynos/cmd-mox#249 for the shim hanging rather than failing.

* Correct the account of why the cmd-mox tests use a venv

The comment and the guide both blamed the layered `uv run --with`
environment, saying the shim could not import cmd_mox there. That is wrong.
The shim's shebang resolves `python3` to the layered interpreter, both
`CMOX_IPC_SOCKET` and `CMOX_IPC_TIMEOUT` reach the child, and the same
invocation passes repeatedly; two other repositories run cmd-mox under
layered environments without trouble.

What is real is a stall that appears under load, always alongside the
server logging `IPC received malformed JSON`, with the shim ignoring its
own `CMOX_IPC_TIMEOUT` while it waits. Three consecutive runs stalled while
the machine was busy compiling; three later runs of the same file passed.

The target still builds a materialized virtual environment, because that
is where the suite has been stable and `typecheck-python` already needs
one, but the reason is now stated as the observation it is rather than as
a cause I had not tested. leynos/cmd-mox#249 carries the disproof and asks
for a bounded wait, which is the fix worth having whatever the race is.

* Address the review round on the lint-actions script

Require the CI step's whole `run` value to be the command. A per-line
search is satisfied by a step that wraps the command in `if false; then
... fi` and runs nothing, which is worse than no contract: this one exists
because the suite it guards would otherwise never run on a pull request.
The `if` key assertion stays, since the two catch different things, a
YAML-level condition and a shell-level one. Rewriting the step into a
false guard now fails the contract.

Require GNU Make in the failure-propagation contract. `.ONESHELL` is a GNU
extension, so under a non-GNU make the probe targets run line by line and
the whole contract passes while measuring nothing; a green run would have
been indistinguishable from a correct one. The helper prefers `gmake`,
falls back to `make`, and accepts a candidate only when `--version` begins
with `GNU Make`.

Give the script's public interfaces their NumPy sections: `Parameters` and
`Attributes` on `LintError`, `Attributes` on `Invocation`, `Parameters` and
`Returns` on `plan`, `Parameters` and `Raises` on `lint`, `Raises` on
`main`. The private helpers keep single-line summaries with their rationale
in adjacent comments, which is the same instruction read the other way.

Annotate `TMPDIR_SENTINEL`.

* Separate discovery from planning, and test the entry point

Three pre-merge rows, all fair.

`plan` was presented as a query but walked the filesystem, and an
unreadable directory yielded nothing exactly as an absent one does. That is
this pull request's own defect one level down: a gate that reports success
having examined nothing. `discover` now does the walking and raises
`DiscoveryError` when a directory exists but cannot be listed, while an
absent directory still yields nothing, because a repository with no
composite actions is normal. `plan` is left a pure function of its inputs,
which is what lets the ordering be asserted without a filesystem.

The listing failure is caught both when it starts and while it is consumed,
since `rglob` can raise at either point depending on the failure.

Nothing tested `main`. Every test drove `lint`, so a script that detected a
failure and exited zero would have passed all of them. Two cases now drive
the entry point: a failing linter must exit with that linter's own status
and name it on stderr, and an all-passing run must exit quietly.

Two property tests cover the invariants stated over an unbounded set, which
fixed fixtures cannot show: `plan` orders any number of surfaces the same
way, and `lint` stops at whichever invocation fails. The second drives the
real loop with a substituted runner rather than reimplementing it, and
patches through a context because Hypothesis will not reuse a
function-scoped fixture across examples.

* Split the surface-ordering property test

CodeScene flagged `test_plan_orders_every_surface_the_same_way` as a
Complex Method: it built the repository, computed the expected order, and
asserted both the tool sequence and the manifests' sort order in one body.

The repository construction and the expected sequence move to helpers, and
the sort-order claim becomes its own property. They are separate
invariants: one says which tools run and in what order, the other says the
manifests within a run do not depend on what the filesystem returned. A
test asserting both told the reader which had failed only by line number.

* Assert the gate cannot be skipped as well as not rewritten

Requiring the step's whole `run` value closed two holes and left a third:
a condition skips the step with its `run` untouched, so the contract passes
while the gate never executes. `if: false` does it, and so does an ordinary
looking `github.event_name == 'push'`, which skips the gate on exactly the
event it exists to cover.

A gate holds only when three things hold together, each of which defeats
the others alone: the workflow triggers on `pull_request`, exactly one step
has the command as its whole `run` value, and neither the job nor that step
carries an `if` key.

The condition is asserted on the key's presence, never on its value. A
condition need not be falsy to skip the gate, and comparing against the
string "false" would pass its own mutation anyway: YAML parses `false` to a
boolean whose string form is `False`.

Eight mutations, all failing the contract: `if: false` on the step, `if:
false` on the job, a push-only condition on each, the command wrapped in
`if false; then ... fi`, `|| true` appended, the command changed, and the
`pull_request` trigger removed.

Reported by the round on repovec-appliance #105.

* Report a directory the gate cannot read, rather than skipping it

`discover` raised `DiscoveryError` on a listing failure and its test
passed, but neither did anything: `Path.rglob` and `Path.glob` suppress the
`OSError` a denied directory raises and yield nothing instead. A subtree the
gate could not read was therefore indistinguishable from a subtree with no
manifests, and the run reported success having examined neither.

The test passed because it patched `rglob` to raise, which real `rglob`
never does. It measured the error path of a function that has no error path.

`_manifests_under` now walks with `Path.walk(on_error=...)`, which reports
what `glob` discards, and `_workflows_in` uses `iterdir`, which raises
rather than suppressing. The test creates a directory it genuinely cannot
enter and asserts the failure surfaces; restoring the `rglob` implementation
fails it, where before it passed.

A companion test pins the behaviour this rests on, so if a future Python
makes `rglob` propagate, the walk becomes belt and braces and the test says
so by failing rather than by silently becoming redundant.

Both errors now derive from a `LintActionsError` base, so a caller can catch
this gate's expected failures without catching every `Exception`.

Reported by the round on #436, where this script appears in the diff.

* Reject continue-on-error, and hold the lint-actions target's shape

`_assert_gate_runs_unconditionally` rejected a condition but not
`continue-on-error`, which I dropped when the per-gate assertions moved into
the shared helper. The two are different routes to the same place: a
condition skips the gate, `continue-on-error` runs it and discards the
verdict, and both leave a green pull request the gate never held. Both keys
are now rejected on the job and on the step, by presence rather than value,
and each of the two new mutations fails the contract.

Two contracts hold `test-lint-actions` itself. `make test` must gather it,
so a contributor gets the answer CI will give them rather than discovering
it after pushing. And the recipe must materialize a virtual environment
rather than layer one, because cmd-mox's shim hangs under `uv run --with`
instead of failing, and a hang is the worst thing to leave for the next
person to diagnose. Dropping the prerequisite and switching the recipe to
`uv run --with` each fail their own case.

* Document the traversal error handler

`interrogate` requires every definition to carry a docstring, including a
nested one, and the `on_error` handler had none. CI caught it because this
branch is the one that makes `lint-python` report its own verdict; I had
run Ruff directly and skipped the target, so I missed the tier behind it.
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

leynos and others added 19 commits September 8, 2026 10:28
Prepare for a zero-tolerance TypeDoc documentation gate across the
JavaScript/TypeScript workspaces: document the fetch mutators and query
keys in `frontend-pwa/src/api`, the root `App` component, the zod user
schema fields in `@app/types` (field-site comments so the inferred
output types carry them), the token-reference resolver in
`@app/tokens`, and the contrast validator's `PackageJson` typedef
(converted to `@property` form so each field can carry a description).
Add `docs-check` to `make all` (and a root `docs:check` script)
covering three surfaces, each with its own configuration beside its
tsconfig: `frontend-pwa` (`src`, `expand`, excluding the generated
Orval client under `src/api/generated` and tests), `packages/types`
(`src`, `expand`), and `packages/tokens` (JavaScript via a dedicated
`tsconfig.typedoc.json` with `allowJs`, `checkJs: false`). Each runs
TypeDoc's `notDocumented` validation with `emit: "none"` and validation
warnings treated as errors: 100% documentation of the selected
surfaces, qualified names reported for any omission, and no
documentation artefacts written.

TypeDoc and TypeScript 5.9.2 are added at the workspace root with pnpm.
Wire `docs-check` into pull-request CI and document the gate's
maintained surfaces and exclusions.

Correct the fetcher and token utility API documentation, and add
build-contract coverage for the Make, package-script, CI, and
validation-failure paths.
Keep fallible setup and synchronization failures visible at Rust test
boundaries so Whitaker can enforce panic placement without suppressions.

Correct the specification, workflow, AsyncAPI, local Kubernetes, and
Nixie validation paths exposed by the full commit-gate run.
Record the TypeDoc and TypeScript development dependencies in Bun's
lockfile so documentation validation no longer rewrites the working
tree during its install step.
… change

The rebase onto main carried four files that are out of scope for the
TypeDoc gate and that main has since resolved on its own.

`.github/workflows/dependabot-automerge.yml` gained a duplicated `"on":`
mapping because main adopted the same quoting fix independently and the
structural merge kept both copies. `backend/src/doc.rs` was left with
repeated import lists and a module doc comment spliced into the middle of
the file, neither of which compiles.

`backend/src/inbound/http/admin_enrichment.rs`,
`backend/src/inbound/http/offline.rs` and `spec/openapi.json` dropped the
`ToSchema` derives for `ListEnrichmentProvenanceQuery` and
`ListOfflineBundlesQuery` so the two unused component schemas would leave
the published specification. Redocly reports those as warnings, not
errors, and `make lint-openapi` passes on main with them present, so the
change is an unrelated specification edit rather than a gate repair. It
belongs in its own pull request against the OpenAPI surface.

Every file here now matches main exactly.
A caret range on `typedoc` lets a minor release change the gate's verdict
without a reviewed commit, so the root manifest now names 0.28.20 exactly
and both lockfiles resolve that version. New contract tests assert the
shape of the pin and each lockfile's resolution.

The CI contract matched the step by name and then read its `run` value. A
name is prose: renaming the step, or deleting it beside a similarly named
neighbour, satisfied a name-shaped assertion. It now searches every step
in the `build` job for one whose command line is `make docs-check`, and
asserts that step carries neither an `if` guard nor `continue-on-error`,
so the gate is unconditional and fails the job. Both mutations, replacing
the command while keeping the name and deleting the step outright, fail
the contract.

The fixture test proves TypeDoc honours `notDocumented` with warnings
treated as errors, but it says nothing about the three real
configurations. A parametrized test now asserts each of them sets that
policy, emits nothing, and lists at least one required declaration kind.

`packages/tokens/typedoc.json` gained the `excludeInternal`,
`excludePrivate` and `excludeProtected` settings the other two surfaces
already carried, so `@internal` means the same thing everywhere.

The developers' guide gains the declaration kinds each surface requires,
the pinning rule, a local run recipe, and a worked example of documenting
an export. It no longer claims a "qualified declaration names" setting
that no configuration key controls.
Ruff wants a blank line after the module docstring and its import block
sorted; mdtablefix rewraps the paragraphs this branch added. No wording
or assertion changes.
`make lint-python` reported these but still exited zero: the Makefile's
global `.ONESHELL` means only the recipe's last command decides the
target's status, so Ruff's verdict was discarded. Read from the full
output instead.

The two subprocess calls now resolve their executable through
`shutil.which`, so PATH no longer decides what runs, and each carries the
`S603` suppression the resolved-but-still-external call needs. The
membership test against `None` and `False` uses a set literal.
TypeDoc runs from `node_modules`, so on a clean checkout `make docs-check`
failed with a missing binary rather than a documentation verdict. Naming
`deps` as a prerequisite matches `typecheck`, which has the same need.
The three configurations disabled TypeDoc's link validation, so a
`{@link}` naming a symbol that does not exist passed silently. A broken
cross-reference is a documentation defect in the same category as a
missing comment: the reader follows it and lands nowhere.

All three surfaces already pass with `invalidLink`, `invalidPath` and
`rewrittenLink` enabled, so this costs nothing today and holds the line
tomorrow. Pointing `usersQueryKeys.all` at a symbol that does not exist
makes `typedoc --options frontend-pwa/typedoc.json` exit 4.

`notExported` stays off. It reports a different thing, a referenced type
that is not part of the public surface, and that is not what this gate
is for.
The contract matched the literal text of the `all` target's line, so
reordering an unrelated prerequisite failed it while saying nothing about
docs-check. It now parses the line and checks the prerequisite list.
Dropping docs-check fails the contract; reordering the rest does not.
Assert each surface's `requiredToBeDocumented` set whole rather than
merely non-empty. That key decides which declaration kinds
`validation.notDocumented` can see, so a list that drops `Function` leaves
undocumented exported functions sailing through a gate still calling
itself zero-tolerance, and a non-empty check cannot tell the difference.
The two TypeScript surfaces expect all ten kinds; the tokens surface is
JavaScript, so the expectation is the same set less the four kinds that
cannot occur there, derived rather than restated. Dropping `Function` from
the frontend configuration now fails the contract.

Describe `customFetch`'s content-type behaviour as it is. The previous
wording implied plain objects and strings were treated alike, when a plain
object is serialized and a string is passed through on the assumption that
it is already JSON. It also omitted that native body types are sent
unchanged with no content type, and that a caller-supplied `Content-Type`
is never overwritten.
A static contract over typedoc.json proves the gate is configured. It
cannot prove it catches anything: a configuration can name every key a
reviewer expects and still admit an undocumented export, because what each
key does is TypeDoc's business rather than the configuration's.

`documentation_gate_behaviour_test.py` runs the real TypeDoc against the
repository's own frontend configuration over a fixture, overriding only the
entry point, the TypeScript configuration and the project name. A
documented fixture passes and leaves behind only the three files the test
wrote, so `emit: "none"` is observed rather than assumed. A missing doc
comment, a link to a symbol that does not exist and an unknown block tag
each fail with a diagnostic naming them. Clearing the responsible key
admits exactly its own fixture and leaves the other two failing, which is
what ties a key to a defect rather than to an expectation of one.

Measuring that matrix turned up a real hole: an unknown block tag warned
and the gate still exited zero, because `treatValidationWarningsAsErrors`
promotes validation warnings only. A misspelled tag name passed silently.
`treatWarningsAsErrors` closes it, and all three surfaces already pass with
it on. The narrower key stays, since it carries the intent on its own if
the broader one is ever removed, but it is subsumed today and neither the
contract nor the guide pretends otherwise.

The static contract gains what it was missing: the validation object
compared whole rather than key by key, the entry points and exclusions per
surface, the `deps` prerequisite on `docs-check`, and an assertion that no
recipe line carries make's `-` prefix. Each is mutation-proved: dropping
`deps`, adding a `-` prefix, narrowing an entry point, widening the
exclusions and removing `treatWarningsAsErrors` each fail exactly one test.

The undocumented fixture gains a module comment explaining its purpose, in
line comments rather than a JSDoc block. A block comment there attaches to
the declaration as its documentation, which made the export documented and
the test it serves pass for the wrong reason.
The cases read only the front-end configuration, which made the other two
surfaces' behaviour an assumption. The three are maintained separately, so
one can lose a key while the others keep it, and a test reading only the
front end would never notice.

Each case now runs against every surface's own configuration. Clearing
`treatWarningsAsErrors` in `packages/tokens/typedoc.json` alone fails four
cases, all of them that surface's, where before it failed none.
The insertion left a 126-column line, which `make markdownlint` rejects
under MD013. I pushed 90d162b without seeing that, because I piped the
gate through `tail` and read the filter s exit status instead of the
gate s.
The contract matched a command line within the step's `run`, so a step
wrapping the command in `if false; then ... fi` satisfied it while running
nothing, and a condition on the step or the job skipped the gate with the
`run` value untouched. Either leaves a green pull request that never ran
the gate.

It now uses the shared `_assert_gate_runs_unconditionally` helper, which
requires three things together: the workflow triggers on `pull_request`,
exactly one step has the command as its whole `run` value, and neither the
job nor that step carries an `if` key. The condition is rejected by the
key's presence rather than by its value, because a condition need not be
falsy to skip the gate: `github.event_name == 'push'` skips it on precisely
the event it exists to cover.

Nine mutations, all failing the contract: `if: false` on the step, `if:
false` on the job, a push-only condition on each, the command wrapped in a
false guard, `|| true` appended, the command changed, the step deleted, and
the `pull_request` trigger removed.

Rebased onto #485 at 202a2cd, which adds the helper.
`f"typedoc@{pin}" in text` is an unanchored substring search, so a pin of
`0.28.2` matched the text `typedoc@0.28.20`. The manifest and the lockfiles
could disagree and the contract would still report green, which is exactly
the drift it exists to catch.

The match now ends on a non-version character. Shortening the manifest pin
to `0.28.2` fails both lockfile cases, where before it passed both.
The structural merge joined the docs-check contract to the function below
it, leaving one blank line where Ruff wants two. I pushed d30502e without
seeing it: I ran the gate sweep and the push in one chain, so the failing
format check scrolled past above a successful `cs delta` and the push ran
regardless.
codescene-access[bot]

This comment was marked as outdated.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No quality gates enabled for this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants