diff --git a/.github/specs/008-label-audit-consolidation/contracts/dry-run-and-drift-report-schema.md b/.github/specs/008-label-audit-consolidation/contracts/dry-run-and-drift-report-schema.md index c766d5ff56..d4fb9d906a 100644 --- a/.github/specs/008-label-audit-consolidation/contracts/dry-run-and-drift-report-schema.md +++ b/.github/specs/008-label-audit-consolidation/contracts/dry-run-and-drift-report-schema.md @@ -4,7 +4,7 @@ Defines the per-repository deletion dry run that @ashleyshaw approves before any ## Per-repository dry run -Saved as `evidence/dry-run/{repo}.json` and summarised in a comment on the gate issue that replaces #95. +Saved as `evidence/dry-run/{repo}.json` and summarised in a comment on the gate issue that replaces #95. For a private repository the file stays under `.private-evidence/dry-run/` and is neither committed nor summarised in the public gate issue (decision of 2026-10-04). ```json { diff --git a/.github/specs/008-label-audit-consolidation/plan.md b/.github/specs/008-label-audit-consolidation/plan.md index 0c84931985..290a7cc269 100644 --- a/.github/specs/008-label-audit-consolidation/plan.md +++ b/.github/specs/008-label-audit-consolidation/plan.md @@ -256,7 +256,7 @@ Each stage starts only when the previous stage's exit check passes. Validation s | 0a. Issue types and colours (configuration done in #3534; T040c to do) | PR updating `issue-types.yml` (25 entries with descriptions, Decision replaces Question) and `type:*` label colours in `labels.yml` from `contracts/issue-types-org-settings.md`, together with the Decision template and `issue-fields.yml`; then a manual update of the organisation's issue types page in the order given in the contract | for #3534, which merged on 2026-09-25 before approval, T040n's dated sign-off on #3556 and #3557 (or a revert of any rejected part); for every later locked-file PR, approval before merge (FR-009); configuration validation and `npm test` pass; settings page matches the contract (Test 10b) | FR-009, FR-014, FR-019, FR-020 | | 0b. Labelling agent guard (done in #3564) | Separate fix PR (`fix/` branch): the agent stops removing labels for being outside `labels.yml`, honours `DRY_RUN`, and applies only `type:*` labels that exist in `labels.yml` | Test 16 passes; the agent's own tests pass | FR-017, FR-022 | | 0c. Label names on the audit branch (done in #3362) | On `audit/label-consolidation`: replace `ai-ops`/`type:ai-ops` label names with `aiops`/`type:aiops` and `openspec:` with `spec:` in the spec 008 artefacts, the audit's analysis reports and recommendation files, `docs/LABEL_INVENTORY.md` and `docs/LABELING_FAQ.md`; add `target_name` to the four snapshot files (`canonical-labels.json`, `label-families.json`, `label-inventory.json`, `label-inventory.csv`) and a note in `evidence/README.md`; leave `renamed-label-references.json` unchanged. No locked file, script, workflow or test changes | Test 17 passes; the evidence test still passes | FR-011 | -| 0. Evidence | Paginated label inventory for every repository (after the T075 token guard); `evidence/linear-labels.json` with issue counts and proposed mappings (`contracts/label-mapping-schema.md`) | Mapping validation rules pass (Test 9); the inventory lists private repositories | FR-006, FR-010, FR-012, FR-018 | +| 0. Evidence | Paginated label inventory for every repository (after the T075 token guard); `evidence/linear-labels.json` with issue counts and proposed mappings (`contracts/label-mapping-schema.md`) | Mapping validation rules pass (Test 9); the inventory counts every private repository (names withheld, 2026-10-04) | FR-006, FR-010, FR-012, FR-018 | | 1. Approve | Raise `[LABEL-UPDATE-REQUEST]` (mapping table, referencing #3554 for its five imports), `[ISSUE-TYPE-UPDATE-REQUEST]` and `[TEMPLATE-UPDATE-REQUEST]` (Question → Decision), a `[LABEL-UPDATE-REQUEST]` for the 16 FR-011 prefix renames and the `spec:NNN` → `spec-id:NNN` move, a `[TEMPLATE-UPDATE-REQUEST]` for the six PR templates (`type:ai-ops` → `type:aiops`, `type:documentation` → `type:docs`, `meta:needs-review` removed) that also covers the routing files `branch-labels.yml` (T074) and `PULL_REQUEST_TEMPLATE/config.yml` (T076), locked since constitution v1.4.0, a migration issue for OpenSpec paths, and a new gate issue replacing #95; each waiting request carries `meta:needs-approval` once the label exists | @ashleyshaw approves all requests | FR-009, FR-011, FR-013, FR-014, FR-016, FR-021 | | 2. Configuration PR | One PR: `labels.yml` (renames, imports including the five #3554 labels with the FR-012 colour rule, the `area:*` → `aiops:*` AI merges), `label-governance-policy.yml` (new gate issue, `enabled: false`), `labeler.yml`, `branch-labels.yml` (one canonical `type:*` label per branch type, matching its routed template: T074), `PULL_REQUEST_TEMPLATE/config.yml` (mirrors `branch-types.yml`: T076), scripts, workflows (including `orchestrate-phase-progression.yml`), validators (`validate-labeling-configs.cjs` prefix list) and tests referencing `ai-ops:` or `openspec:` labels, `pr_aiops.md`, the `spec:NNN` → `spec-id:NNN` move (`prd-combined-agent` plan, `bulk-label-issues.sh` and its test), the six docs files, and the FR-021 policy in `docs/LABEL_STRATEGY.md`. The `type:question` removal, `issue-types.yml`, `06-decision.md` and `issue-fields.yml` belong to Stage 0a; the never-delete list clean-up shipped in #3362 (T055 stays open only for `gated_by_issue`, which waits for the new gate issue) | Merges only after #3564 (Stage 0b), and its label renames are applied in GitHub (Stage 3) in the same session, so no item loses an `ai-ops:*` or `openspec:*` label. CI green; Test 10 and Test 15 pass | FR-011, FR-012, FR-014, FR-021 | | 2b. Spec Kit rename PR | Separate PR for the OpenSpec → Spec Kit rename in live files and paths, with links updated | Test 11 passes | FR-013 | diff --git a/.github/specs/008-label-audit-consolidation/spec.md b/.github/specs/008-label-audit-consolidation/spec.md index faf72d9b9a..6aa6b96f47 100644 --- a/.github/specs/008-label-audit-consolidation/spec.md +++ b/.github/specs/008-label-audit-consolidation/spec.md @@ -324,5 +324,9 @@ The audit itself makes no changes to the production label configuration. The app - Q: FR-018 requires the drift check to authenticate with an App holding only Issues (read/write) and Metadata (read). May the existing `BOT_PR_APP_*` App be reused unchanged? → A: Yes, by decision of Chris on 2026-10-02 (recorded in `evidence/change-requests.json` as `APP-CREDENTIAL-VARIANCE`). The App is shared by `documentation.yml`, `keep-pr-current.yml` and `branch-validation-metrics-aggregator.yml`, so narrowing it would break those; no new App and no new secret values were created. **This is a documented variance from FR-018 and T071a: the App's installed permission set is broader than "only Issues (read/write) and Metadata (read)", and the limit is applied per token instead.** **Amended 2026-10-02 after a failed live dry run:** `BOT_PR_APP_*` has no Issues permission and cannot serve the drift check; the check now uses the installed `lightspeed-project-automation` App (permissions verified and recorded) with downscoped tokens. `LINEAR_API_KEY` was confirmed by Chris and stays repository-level. - Q: What does the `label-drift-check` GitHub environment actually protect, given it holds no secrets? → A: Branch gating only. The environment exists (created 2026-10-02, admin bypass off, deployment branch policy `develop` only) and the drift job binds to it, so a `workflow_dispatch` from another branch cannot deploy that job. It provides **no secret isolation**: the environment stores no secrets, so every credential remains reachable at repository level by any workflow that runs in the repository. Secret isolation would require moving the three secrets into the environment, which has not been done. The job is therefore gated by branch, not by secret scope. +### Session 2026-10-04 + +- Q: This repository is public and the organisation has private repositories, some of which may identify clients. Should the label inventory and the drift report name them? → A: No. By decision of Chris on 2026-10-04, the clean-up and the drift check still cover private repositories, but nothing committed to or published in `lightspeedwp/.github` (evidence files, the drift issue, its artifact, Actions logs) names a private repository or lists its labels. The tracked inventory keeps each private repository as a numbered placeholder with its counts (`label_count`, `pages_read`), the drift issue counts them, and the full detail is kept outside this repository: a local `.private-evidence/` directory for the inventory and the per-repository dry runs, and a private report repository for the drift detail (`PRIVATE_REPORT_REPO`). This supersedes the earlier assumption that the inventory and per-repository dry runs are committed in full. Recorded in FR-006, T041, T065a and the dry-run contract. + *This page brought to you by the 🦄 Magic Automation Unicorns of LightSpeedWP.* [Automation Docs](https://github.com/lightspeedwp/.github/tree/main/instructions) diff --git a/.github/specs/008-label-audit-consolidation/tasks.md b/.github/specs/008-label-audit-consolidation/tasks.md index e447d9d208..242cf0b037 100644 --- a/.github/specs/008-label-audit-consolidation/tasks.md +++ b/.github/specs/008-label-audit-consolidation/tasks.md @@ -405,7 +405,7 @@ The active labelling agent removes labels outside `labels.yml` today, so this ru ### Stage 0: Evidence (FR-006, FR-010, FR-012) -- [ ] T041 [US4] (FR-006, FR-010, FR-018, SC-001; after T075) Run `scripts/automation/label-inventory.js` (already written, commit 7d799e60) from @ashleyshaw's session with a fine-grained personal access token (all `lightspeedwp` repositories, Issues and Metadata read-only, expiring in 7 days or less, revoked after the run) in `LABEL_INVENTORY_TOKEN` (the variable the script reads once T075 has merged in #3734; T041 does not run before that, because today the script reads `GITHUB_TOKEN`), never `GITHUB_TOKEN` (spec clarification 2026-10-02). The script lists every `lightspeedwp` repository and pages through each repository's labels with `per_page=100` until no `next` link remains; save the result to `.github/reports/audits/2026-09-14-label-audit/evidence/github-api-labels.json` (replacing the empty inventory) with, per repository, `label_count` and `pages_read` where "`pages_read × 100 ≥ label_count`" +- [ ] T041 [US4] (FR-006, FR-010, FR-018, SC-001; after T075) Run `scripts/automation/label-inventory.js` (already written, commit 7d799e60) from @ashleyshaw's session with a fine-grained personal access token (all `lightspeedwp` repositories, Issues and Metadata read-only, expiring in 7 days or less, revoked after the run) in `LABEL_INVENTORY_TOKEN` (the variable the script reads once T075 has merged in #3734; T041 does not run before that, because today the script reads `GITHUB_TOKEN`), never `GITHUB_TOKEN` (spec clarification 2026-10-02). The script lists every `lightspeedwp` repository and pages through each repository's labels with `per_page=100` until no `next` link remains; save the result to `.github/reports/audits/2026-09-14-label-audit/evidence/github-api-labels.json` (replacing the empty inventory) with, per repository, `label_count` and `pages_read` where "`pages_read × 100 ≥ label_count`" **Private repositories (decision of 2026-10-04): run it with `--full-output .private-evidence/label-inventory-full.json`, so the tracked file gets the redacted inventory (each private repository a numbered placeholder with its counts, no name and no labels) and the full inventory stays local; the script refuses a full-output path under `.github/`. The completeness checks run on the full inventory first.** - [ ] T041a [P] [US4] List the `lightspeedwp` organisation's native issue types and count the issues using each (especially Maintenance, Story and Integration) across all repositories; save to `.github/reports/audits/2026-09-14-label-audit/evidence/native-issue-types.json` (FR-019) - [x] T042 [P] [US4] Export all Linear workspace and team labels with per-label issue counts (exact label ID filter, archived issues included) into the `sources` block of `.github/reports/audits/2026-09-14-label-audit/evidence/linear-labels.json`, following `contracts/label-mapping-schema.md` - [ ] T043 [US4] Fill `mappings[]` in `.github/reports/audits/2026-09-14-label-audit/evidence/linear-labels.json` from spec FR-011 (renames), FR-012 (imports, merges, retirements, team-scope), FR-014 (swap) and FR-015 (re-prefix), with each entry's `action` one of "`rename`, `import`, `merge`, `re-prefix`, `retire`, `team-scope`, `swap`"; include the five #3554 imports (`area:builds`, `area:monitoring`, `area:observability`, `area:workflows`, `meta:needs-approval`) with the colours and descriptions set in FR-012, and no `area:observability` → `area:monitoring` merge; fill `color`, `description` and `change_request` for every import as `contracts/label-mapping-schema.md` requires (a `meta:*` import uses `57606A`); map `area:agents`, `area:instructions` and `area:prompts` to their `aiops:*` labels and decide `area:skills`; check validation rules 1 to 7 of the schema, including "No mapping leaves an issue with zero or two `type:*` labels" (depends on T041, T042) @@ -452,8 +452,8 @@ The active labelling agent removes labels outside `labels.yml` today, so this ru - [ ] T062a [P] [US4] Create `scripts/automation/includes/label-write-queue.js`, a shared helper for every mutating GitHub and Linear call in Stages 3 to 5: it takes each request as a transport-specific callback, with a GitHub adapter that wraps `githubApiRequest` from `scripts/automation/includes/github-api-optimized.js` and a Linear adapter, so one queue serves both, runs mutating requests one at a time and at least one second apart, honours `Retry-After` and `x-ratelimit-reset`, keeps Linear calls within Linear's complexity limits, and on a limit pauses and then resumes (FR-023 point 6; research R21). Add unit tests in `scripts/automation/__tests__/label-write-queue.test.js` for spacing, `Retry-After`, pause-and-resume and both adapters - [ ] T062b [US4] Make `scripts/automation/label-consolidate.js` safe to stop and repeat (FR-023 points 1, 2 and 8; research R21): send every write through T062a; take `.github/reports/audits/2026-09-14-label-audit/evidence/run-lock.json` (`run_id`, `run_by`, `started_at`, `stage` one of "`3`, `4` or `5`", `epoch`, `host` and `resumed_from`, which is null on a first start and set by `--resume` to the previous `epoch` and the takeover time) at start with an exclusive create and hold an operating-system advisory lock (`flock`) on it while running, refuse to start while the file exists (naming its `run_id`), release the advisory lock and delete `run-lock.json` when the run, or a run resumed with `--resume `, finishes successfully (an interrupted run keeps the file so it can be resumed again), add `--resume ` (it succeeds only after the original process has released the advisory lock, keeps the `run_id`, raises `epoch` by one and reconciles that `run_id`'s unmatched records first) and `--abandon-run ` (only after @ashleyshaw confirms on the gate issue), refuse any append when the lock's `epoch` is not the run's own, add a `record` subcommand that appends the `intended` and `done` records for steps done by hand (T063), and prefix every `op_id` with the `run_id` (the UTC start time to the second plus eight random hex digits, so two runs started in the same minute never share one) so a resume reconciles only its own records (FR-023 point 11); skip any repository whose `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` has `executed_at` set for the stage being run (Stage 3 for T064, Stage 4 for T067); compare the current state with the approved set before each action and make no write when they match; set `executed_at` for that stage when a repository finishes; treat labels that differ only by case or spacing (for example `scope: website`) as separate sources and never merge them automatically, doing a case-only change as an in-place rename. Log every change to `.github/reports/audits/2026-09-14-label-audit/evidence/consolidation-log.jsonl` with the Consolidation Log Entry fields from `data-model.md` (`run_by`, `at`, `repository`, `action` one of "`rename`, `create`, `update`, `relabel`, `convert`, `delete`", `label`, `before` / `after`, `gate_issue`, `op_id`, `state` one of "`intended` (written before the API call) or `done` (written after it succeeds)"; "append-only; every change has an `intended` record before its API call and a `done` record after it succeeds; on resume, an `intended` record with no `done` record is checked against the live state and then completed or retried; a re-run that makes no write adds no record"); on start, truncate a log whose last line was cut off back to its last newline (keeping the removed bytes in a `.partial` file), then reconcile unmatched `intended` records before any other write (contract log rules 4 and 5; research R21), and post one summary comment per run on the gate issue (FR-023 point 10) (depends on T062, T062a) - [ ] T062c [US4] Add `scripts/automation/__tests__/label-consolidate.test.js`: a re-run on a finished repository makes no API write and adds no log record; a stopped run resumes with the repositories that have no `executed_at` for its stage, and a repository finished for Stage 3 is still processed by Stage 4; a failed API call writes no `done` record and leaves its `intended` record, which the next run reconciles against the live state (completing it if the change happened, retrying it if not); a log whose last line was cut off mid-write is truncated back to its last newline (the removed bytes kept in a `.partial` file) before a resumed run appends, so the next record parses and the interrupted change is reconciled; case-only variants stay separate mapping sources; two fresh runs started within the same second get different `run_id`s; a second run refuses to start while `run-lock.json` exists; a run killed mid-write leaves a lock that a plain start refuses and that `--resume ` takes over only after the first process has released its advisory lock, adopting the same `run_id`, raising `epoch` by one, recording `resumed_from` (the previous `epoch` and the takeover time) and reconciling its unmatched `intended` records first; a holder with a stale `epoch` cannot append; `--abandon-run` needs the gate-issue confirmation; and a resume ignores `intended` records from another `run_id` (depends on T062b) -- [ ] T063 [US4] First, before changing any label, write a snapshot of every issue's and PR's `type:*` labels (item kind, number and labels) to `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-snapshot.json`, which SC-011 is checked against (T064, T067a); then, in each repository with open `type:question` issues, convert them to Discussions (Q&A category) with the Labels page bulk action; relabel closed ones `type:task` + `discussion:support`; record repositories where Discussions are disabled in `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` and relabel their open issues the same way. Must finish before any `type:question` deletion in Stage 4 (FR-014). Record every conversion and fallback relabel through `label-consolidate.js record` (T062b) as an `intended` record before and a `done` record after, under the Stage 3 run lock (a conversion carries the source issue in `before.item` and the resulting Discussion in `after.item`, each as kind and number); on resume an unmatched `intended` conversion is checked against the live state (the issue is already a Discussion, or the relabel is present) and then completed or repeated (FR-023 points 5 and 11) (depends on T059, T062b) -- [ ] T064 [US4] Run `scripts/automation/label-consolidate.js --apply` for renames, creates and relabels across all repositories (every change logged per T062b, and `executed_at` for Stage 3 set per repository), then confirm no issue or PR has zero or two `type:*` labels, recording the check in `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-check.json` against the pre-run snapshot that T063 wrote before any change, which T067a uses for SC-011 (depends on T062b, T062c, T063, T071) +- [ ] T063 [US4] First, before changing any label, write a snapshot of every issue's and PR's `type:*` labels (item kind, number and labels) to `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-snapshot.json` for public repositories and to `.private-evidence/type-label-snapshot.json` (never committed) for private ones, which SC-011 is checked against (T064, T067a); then, in each repository with open `type:question` issues, convert them to Discussions (Q&A category) with the Labels page bulk action; relabel closed ones `type:task` + `discussion:support`; record repositories where Discussions are disabled in their dry-run file (`evidence/dry-run/{repo}.json`, or `.private-evidence/dry-run/{repo}.json` for a private repository) and relabel their open issues the same way. Must finish before any `type:question` deletion in Stage 4 (FR-014). Record every conversion and fallback relabel through `label-consolidate.js record` (T062b) as an `intended` record before and a `done` record after, under the Stage 3 run lock (a conversion carries the source issue in `before.item` and the resulting Discussion in `after.item`, each as kind and number); on resume an unmatched `intended` conversion is checked against the live state (the issue is already a Discussion, or the relabel is present) and then completed or repeated (FR-023 points 5 and 11) (depends on T059, T062b) +- [ ] T064 [US4] Run `scripts/automation/label-consolidate.js --apply` for renames, creates and relabels across all repositories (every change logged per T062b, and `executed_at` for Stage 3 set per repository), then confirm no issue or PR has zero or two `type:*` labels, recording the check in `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-check.json` for public repositories (private repositories are checked against `.private-evidence/` and add only an aggregate pass or fail, with no repository name, label name or item number) against the pre-run snapshot that T063 wrote before any change, which T067a uses for SC-011 (depends on T062b, T062c, T063, T071) - [ ] T064d [US4] After T059 merges, switch the labelling agent to the approved mapping: load the FR-012 mapping from `evidence/linear-labels.json` (or aliases added to `labels.yml`) as its alias list replacing mapped labels only and keeping unmapped removal off, so unmapped and retiring labels stay until their repository's approved Stage 4 deletion (FR-022; depends on T040d, T059) - [ ] T064a [US4] Superseded by T040c (manual organisation update); verify only. Original scope: Move every issue on the Maintenance, Story or Integration native types to its mapped type (Maintenance → Chore plus `area:maintenance`, Integration → Feature plus `area:integration`, Story → Feature) and update its `type:*` label to match, recording each change in `.github/reports/audits/2026-09-14-label-audit/evidence/native-issue-types.json` (depends on T041a, T059) - [ ] T064b [US4] Superseded by T040c (manual organisation update); verify only. Original scope: Change the organisation's native issue types in this order (the organisation is limited to 25): rename A11y → Accessibility, Code Refactor → Refactor, Code Review → Review, Build & CI → CI; remove Maintenance, Story and Integration once T064a shows zero issues on them; add Build, Dependency Update and Decision. Use the organisation issue-types API if available, otherwise organisation settings; confirm the final list matches `.github/issue-types.yml` and record it in `.github/reports/audits/2026-09-14-label-audit/evidence/native-issue-types.json` (depends on T064a) @@ -461,11 +461,11 @@ The active labelling agent removes labels outside `labels.yml` today, so this ru ### Stage 4: Gated GitHub Deletion (FR-016) - [ ] T064c [US4] Gate: before any deletion task (T065 onwards) or Linear clean-up (T069 onwards) starts, @ashleyshaw reviews every item in `checklists/destructive-changes.md` (CHK001–CHK038) and ticks it; stop if any item is unchecked or answered "no". Also confirm T063 has finished for every repository (FR-014, FR-016) -- [ ] T065 [US4] Extend `scripts/automation/label-consolidate.js` with a deletion dry run that fills `to_delete` in `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` with a full snapshot per label (name, colour, description, and the kind and number of each open and closed item) and a `migrate_to` for every label on open items, per `contracts/dry-run-and-drift-report-schema.md`; it leaves the Stage 3 `executed_at` as the record of Stage 3 and requires the Stage 4 `executed_at` to be null (depends on T064, T064c) -- [ ] T065a [US4] In `scripts/automation/label-consolidate.js`, re-read each repository just before deleting: if its labels, or the items carrying any `to_delete` label, differ from `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` once this run's own completed `done` records are applied to it (so a resumed run does not treat its own earlier deletions as drift, and only differences beyond them are stale), or `labels.yml` on `develop` differs from `approved_set_commit`, mark the dry run stale, skip the repository, post the reason on the gate issue, and require a new dry run and approval (FR-023 points 3 and 4; dry-run rules 6 and 7 in `contracts/dry-run-and-drift-report-schema.md`). Add the stale-dry-run case and the resumed-run case (own `done` records applied, no false stale) to `scripts/automation/__tests__/label-consolidate.test.js` (depends on T065, T062c) -- [ ] T066 [US4] Post each repository's dry-run summary on the gate issue and record @ashleyshaw's per-repository approval (the comment author must be the `ashleyshaw` login, FR-016) in the `approval` block of `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json`; repositories without approval are marked `skipped` (depends on T065) -- [ ] T067 [US4] From @ashleyshaw's session, run `scripts/automation/label-consolidate.js --apply --confirm-gate `, which migrates open issues, PRs and Discussions, migrates closed items for labels that have a mapping target (FR-016), and deletes the listed labels only in repositories whose `evidence/dry-run/{repo}.json` approval is `approved` and refuses otherwise; it sets the Stage 4 `executed_at` as each repository finishes; `destructive_cleanup.enabled` in `.github/label-governance-policy.yml` stays `false`; then run quickstart Test 12 (depends on T066, T065a) -- [ ] T067a [US4] Verify SC-011 and SC-012 after T067: every issue and PR that had a `type:*` label in the T063 pre-run snapshot has exactly one now, and every deleted label has a snapshot entry with name, colour, description and item kinds and numbers in its `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json`; record both results in `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-check.json` and on the gate issue (the SC-011 and SC-012 checks in quickstart Test 12) (depends on T067) +- [ ] T065 [US4] Extend `scripts/automation/label-consolidate.js` with a deletion dry run that fills `to_delete` in `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` with a full snapshot per label (name, colour, description, and the kind and number of each open and closed item) and a `migrate_to` for every label on open items, per `contracts/dry-run-and-drift-report-schema.md`; it leaves the Stage 3 `executed_at` as the record of Stage 3 and requires the Stage 4 `executed_at` to be null. For a private repository (decision of 2026-10-04) the file lives under `.private-evidence/dry-run/{repo}.json`, which is never committed, and its summary, approval and gate comment go to the private report repository only, never to this public repository or its gate issue. (depends on T064, T064c) +- [ ] T065a [US4] In `scripts/automation/label-consolidate.js`, re-read each repository just before deleting: if its labels, or the items carrying any `to_delete` label, differ from `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` once this run's own completed `done` records are applied to it (so a resumed run does not treat its own earlier deletions as drift, and only differences beyond them are stale), or `labels.yml` on `develop` differs from `approved_set_commit`, mark the dry run stale, skip the repository, post the reason on the gate issue, and require a new dry run and approval (FR-023 points 3 and 4; dry-run rules 6 and 7 in `contracts/dry-run-and-drift-report-schema.md`). Add the stale-dry-run case and the resumed-run case (own `done` records applied, no false stale) to `scripts/automation/__tests__/label-consolidate.test.js` (depends on T065, T062c) **Private repositories (decision of 2026-10-04): the dry-run file and its gate-issue comment for a private repository are not committed or posted in this public repository; keep it under `.private-evidence/dry-run/` and report it only in a private repository.** +- [ ] T066 [US4] For each public repository, post its dry-run summary on the gate issue and record @ashleyshaw's per-repository approval (the comment author must be the `ashleyshaw` login, FR-016) in the `approval` block of `.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json`. For each private repository, post the summary and take the approval in the private report repository (`PRIVATE_REPORT_REPO`) instead and record it in the `approval` block of `.private-evidence/dry-run/{repo}.json`; nothing about a private repository is posted on the public gate issue or committed here. Repositories without approval are marked `skipped` (depends on T065) +- [ ] T067 [US4] From @ashleyshaw's session, run `scripts/automation/label-consolidate.js --apply --confirm-gate `, which migrates open issues, PRs and Discussions, migrates closed items for labels that have a mapping target (FR-016), and deletes the listed labels only in repositories whose approval is `approved` and refuses otherwise (public repositories read `evidence/dry-run/{repo}.json`, private repositories read `.private-evidence/dry-run/{repo}.json`, and a private repository with no such file is refused, never read from the public path); it sets the Stage 4 `executed_at` as each repository finishes; `destructive_cleanup.enabled` in `.github/label-governance-policy.yml` stays `false`; then run quickstart Test 12 (depends on T066, T065a) +- [ ] T067a [US4] Verify SC-011 and SC-012 after T067: every issue and PR that had a `type:*` label in the T063 pre-run snapshot has exactly one now, and every deleted label has a snapshot entry with name, colour, description and item kinds and numbers in its dry-run file (`.github/reports/audits/2026-09-14-label-audit/evidence/dry-run/{repo}.json` for a public repository, `.private-evidence/dry-run/{repo}.json` for a private one, which is checked and reported privately only); record the per-repository SC-011 and SC-012 results for public repositories in `.github/reports/audits/2026-09-14-label-audit/evidence/type-label-check.json` and on the gate issue, and keep the per-repository results for private repositories in `PRIVATE_REPORT_REPO` only; the public file and the public gate issue carry only an aggregate pass or fail for the private repositories as a group, with no private repository name, label name or item number (the SC-011 and SC-012 checks in quickstart Test 12) (depends on T067) - [ ] T068 [P] [US4] Update the organisation's default repository labels to match `.github/labels.yml` (API if available, otherwise organisation settings), recording the method used on the gate issue and in `.github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json` (research R9) (depends on T059) ### Stage 5: Linear Clean-up (FR-012, FR-015, FR-017) @@ -591,7 +591,7 @@ The active labelling agent removes labels outside `labels.yml` today, so this ru ## Phase 10: Convergence - [ ] T080 Add a required pull request check, `.github/workflows/locked-files-guard.yml` running a new `scripts/validation/check-locked-file-approval.cjs` (with tests in `scripts/validation/__tests__/`), that fails when a PR changes a constitution v1.4.0 LOCKED file (`.github/labels.yml`, `.github/issue-types.yml`, `.github/ISSUE_TEMPLATE/*.md`, `.github/PULL_REQUEST_TEMPLATE/*.md`, `.github/PULL_REQUEST_TEMPLATE/config.yml`, `.github/branch-types.yml`, `.github/branch-labels.yml`) unless the PR body links a change-request issue whose entry in `.github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json` (or, outside spec 008, a dated approval comment by `ashleyshaw` on that issue) shows `status: "approved"`; also give those paths explicit `CODEOWNERS` lines owned by `@ashleyshaw` only, so a team member's approval cannot stand in for the approver's (the gap that let #3534 merge) per Constitution II, plan: Constitution Check remediation (partial) — implemented in #3734, which is still open (guard, tests and `CODEOWNERS`; approvals count only on issues tagged `[LABEL-UPDATE-REQUEST]`, `[ISSUE-TYPE-UPDATE-REQUEST]` or `[TEMPLATE-UPDATE-REQUEST]`, per the 2026-10-02 clarification); it checks only that an approved request is linked and does not compare which locked files that request covers, which T084 adds; the branch-protection settings that make it enforce are T080a -- [ ] T080a (@ashleyshaw, manual; after #3734 merges) In the branch protection rules for `develop` and `main`, make "Locked files need an approved change request" a required check and turn on "Require review from Code Owners"; then open a test PR that changes `.github/labels.yml` with no linked change request, confirm the check fails and the PR cannot merge, close it, and record the settings on #3733; must be done before the Stage 2 configuration PR (T059) per Constitution II, FR-009 (partial) +- [ ] T080a (repository admin, manual; no further sign-off is needed, as the decision is recorded for 2026-10-04; after #3734 merges) In the branch protection rules for `develop` and `main`, make "Locked files need an approved change request" a required check and turn on "Require review from Code Owners"; then open a test PR that changes `.github/labels.yml` with no linked change request, confirm the check fails and the PR cannot merge, close it, and record the settings on #3733; first verify the Actions event policy that lets `.github/workflows/locked-files-guard.yml` use `pull_request_target`, which already exists (policy 6525, created and read back on 2026-10-05, scoped to that one workflow): `gh api -H "X-GitHub-Api-Version: 2026-03-10" repos/lightspeedwp/.github/actions/policies/6525` must show enforcement `active`, that workflow path only, and the `pull_request_target` event, and the policy insights page must show no blocked runs for the guard. From 2 November 2026 GitHub blocks `pull_request_target` by default in public repositories without such a policy, so without it the required check would never report; must be done before the Stage 2 configuration PR (T059) per Constitution II, FR-009 (partial) - [ ] T081 Extend `scripts/validation/validate-labeling-configs.cjs` (and its tests) to enforce the constitution v1.4.0 routing rules: every branch type in `.github/branch-types.yml` has exactly one `type:*` label in `.github/branch-labels.yml` `default_labels`, equal to the canonical type label in its routed template's frontmatter (mapping `type:documentation` → `type:docs` and `type:ai-ops` → `type:aiops` until T046b is applied), and every route in `.github/PULL_REQUEST_TEMPLATE/config.yml` matches `branch-types.yml`; run it after T074 and T076 so it starts green (depends on T074, T076; #3732 has merged) per Constitution v1.4.0 routing, plan: R23 (partial) - [ ] T084 Extend the T080 guard so an approval authorises only the locked files its request names: add a `covers` list (paths or patterns) to each request entry in `.github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json`, and make `scripts/validation/check-locked-file-approval.cjs` fail when any locked file the PR changes is not matched by the `covers` list of an approved linked request; add tests in `scripts/validation/__tests__/check-locked-file-approval.test.js` showing that an approval covering file A does not pass a PR that changes file B, that a PR changing A and B passes only when both are covered, and that the existing passing cases still pass; whether a change within a covered file matches what was approved stays the approver's review through the `CODEOWNERS` line (depends on T080) per FR-009, Constitution II - [ ] T082 [US4] Before T043 fills `mappings[]`, re-read every Linear label with the Linear MCP integration (`list_issue_labels` for the workspace and for every team, archived included) and record in `.github/reports/audits/2026-09-14-label-audit/evidence/linear-labels.json` how many `migrate:*` labels remain active, with their scope and issue counts; exclude every `migrate:*` label from the FR-012 import, the mapping and any sync back to GitHub; list any that are still active (on 2026-10-02 the GIT team still listed about 30) on the gate issue for @ashleyshaw to retire; and check each valid Linear label against `.github/labels.yml` on `develop`, so the T043 import list holds only labels that are used or required by automation and are not yet in `labels.yml` per FR-012, FR-006 (partial) diff --git a/.github/workflows/label-drift-check.yml b/.github/workflows/label-drift-check.yml index c6d15830a1..5e61af2f4d 100644 --- a/.github/workflows/label-drift-check.yml +++ b/.github/workflows/label-drift-check.yml @@ -90,11 +90,56 @@ jobs: repositories: .github permission-issues: write + - name: Resolve the private report repository + # Optional. The variable PRIVATE_REPORT_REPO (owner/name) names a PRIVATE + # repository that receives the detail for private repositories. This + # public repository never names a private one, so with the variable unset + # that detail is withheld. + id: private-report + env: + PRIVATE_REPORT_REPO: ${{ vars.PRIVATE_REPORT_REPO }} + run: | + if [ -z "$PRIVATE_REPORT_REPO" ]; then + exit 0 + fi + # The App is installed on lightspeedwp only, so a repository under any + # other owner could never be reached; stop here with a clear message + # instead of failing later at the mint step or the first write. + if ! [[ "$PRIVATE_REPORT_REPO" =~ ^([A-Za-z0-9-]+)/([A-Za-z0-9._-]+)$ ]]; then + echo "::error::PRIVATE_REPORT_REPO must be owner/repository" + exit 1 + fi + if [ "${BASH_REMATCH[1]}" != "lightspeedwp" ]; then + echo "::error::PRIVATE_REPORT_REPO must be a lightspeedwp repository" + exit 1 + fi + { + echo "owner=${BASH_REMATCH[1]}" + echo "name=${BASH_REMATCH[2]}" + } >> "$GITHUB_OUTPUT" + + - name: Mint private report App token + # Write-scoped to the private report repository only, using the same + # installed App as the tokens above, so no extra secret is stored. + if: steps.private-report.outputs.name != '' + id: app-token-private + uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 + with: + app-id: ${{ vars.LS_APP_ID }} + private-key: ${{ secrets.LS_APP_PRIVATE_KEY }} + owner: ${{ steps.private-report.outputs.owner }} + repositories: ${{ steps.private-report.outputs.name }} + permission-issues: write + - name: Run label drift check env: GITHUB_TOKEN: ${{ steps.app-token-read.outputs.token }} GITHUB_WRITE_TOKEN: ${{ steps.app-token-write.outputs.token }} LINEAR_API_KEY: ${{ secrets.LINEAR_API_KEY }} + # Private repository detail is written only to this private repository, + # and only after the script confirms it is private. + PRIVATE_REPORT_REPO: ${{ vars.PRIVATE_REPORT_REPO }} + PRIVATE_REPORT_TOKEN: ${{ steps.app-token-private.outputs.token }} DRY_RUN: ${{ inputs.dry_run }} run: | args="" diff --git a/.github/workflows/locked-files-guard.yml b/.github/workflows/locked-files-guard.yml new file mode 100644 index 0000000000..2d2ce826d4 --- /dev/null +++ b/.github/workflows/locked-files-guard.yml @@ -0,0 +1,177 @@ +# Locked Files Guard +# +# Constitution Principle II: LOCKED files change only after an approved change +# request. This check fails a pull request that changes a locked file unless +# its body links a change-request issue (title tagged [LABEL-UPDATE-REQUEST], +# [ISSUE-TYPE-UPDATE-REQUEST] or [TEMPLATE-UPDATE-REQUEST]) that is approved, either recorded as +# `approved` in the spec 008 change-request register or carrying an +# "Approved…" comment by @ashleyshaw (spec 008, task T080). +# +# The guard script and the register are read from the DEFAULT branch, so a pull +# request cannot approve itself by editing them. Make this check required in +# the branch protection rules for `develop` and `main`. +# +# The trigger is `pull_request_target`, so GitHub loads this workflow from the +# default branch rather than from the pull request's merge commit; a pull +# request that changes both a locked file and this workflow therefore cannot +# swap the guard step. That is safe only because the job never checks out or +# runs pull request code: it checks out only the guard and register from the +# default branch and reads the changed files through the API. Keep it that way, and keep this workflow on `develop` +# before requiring the check. It does not run on the pull request that first +# adds it, and it does not replace Code Owner review. +# +# From 2 November 2026 GitHub blocks `pull_request_target` by default in public +# repositories that have no Actions event policy +# (https://github.blog/changelog/2026-09-17-workflow-execution-protections-in-github-actions-generally-available/). +# This repository has none, so an Actions event policy must allow this event for +# this workflow before then (spec 008 task T080a); the policy is a repository +# setting, not a file, so it is not created here. + +name: Locked Files Guard + +on: + pull_request_target: + branches: [develop, main] + types: [opened, synchronize, reopened, edited, ready_for_review] + +concurrency: + group: locked-files-guard-${{ github.event.pull_request.number }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + guard: + name: Locked files need an approved change request + runs-on: ubuntu-latest + timeout-minutes: 5 + # pull-requests: read for the PR files API; issues: read for approval comments. + permissions: + contents: read + pull-requests: read + issues: read + steps: + # The guard and register come from the default branch, the same place this + # workflow definition was loaded from, so a pull request to `main` is + # checked by the guard on `develop` even before `main` has a copy of it. + - name: Check out the default branch (guard and register only) + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + ref: ${{ github.event.repository.default_branch }} + fetch-depth: 1 + persist-credentials: false + sparse-checkout: | + scripts/validation/check-locked-file-approval.cjs + .github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json + sparse-checkout-cone-mode: false + + - name: Check locked files + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + with: + script: | + const fs = require('fs'); + const path = require('path'); + const guardPath = path.join(process.env.GITHUB_WORKSPACE, 'scripts/validation/check-locked-file-approval.cjs'); + // This workflow is only ever loaded from the default branch, which + // carries the guard in the same change, so a missing guard means + // it was removed. Fail closed rather than let every pull request pass. + if (!fs.existsSync(guardPath)) { + core.setFailed('The locked-file guard script is missing from the default branch; refusing to pass without checking.'); + return; + } + const guard = require(guardPath); + + const pr = context.payload.pull_request; + const files = await github.paginate(github.rest.pulls.listFiles, { + owner: context.repo.owner, + repo: context.repo.repo, + pull_number: pr.number, + per_page: 100, + }); + // The files API caps results at 3,000 entries: a locked file + // beyond the cap would otherwise pass silently, so refuse an + // incomplete file list instead of evaluating it. + if (typeof pr.changed_files === 'number' && pr.changed_files > files.length) { + core.setFailed( + `Locked Files Guard cannot verify: the pull request touches ${pr.changed_files} files ` + + `but the API returned ${files.length}; refusing to pass an incomplete file list.` + ); + return; + } + // A rename moves a locked file too, so check both paths. + const changedFiles = files.flatMap((file) => + file.previous_filename ? [file.filename, file.previous_filename] : [file.filename] + ); + + let register = null; + const registerPath = path.join(process.env.GITHUB_WORKSPACE, guard.REGISTER_PATH); + if (fs.existsSync(registerPath)) { + register = JSON.parse(fs.readFileSync(registerPath, 'utf8')); + } + + const commentsByIssue = {}; + const titlesByIssue = {}; + const lookupFailures = []; + // Which issues the body links to is decided by GitHub itself: the + // body is rendered with the Markdown API, so anything GitHub does not + // render as a link (fenced or indented code, code spans, comments, + // other repositories) cannot count. Nothing here parses Markdown. A + // render failure fails closed. Fetch every linked issue: guard.evaluate + // processes all of them, and truncating here would fail a valid + // approval linked after other references. This happens only when + // locked files changed; the job timeout bounds pathological bodies, + // failing closed. + let issues = []; + if (changedFiles.some(guard.isLocked)) { + try { + issues = await guard.linkedIssues( + github, + pr.body || '', + `${context.repo.owner}/${context.repo.repo}` + ); + } catch (error) { + core.setFailed( + `Locked Files Guard could not render the pull request body to find its linked issues (${error.message}); re-run the check. Refusing to pass without it.` + ); + return; + } + for (const issue of issues) { + try { + const { data } = await github.rest.issues.get({ + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue, + }); + // Only change-request issues count; skip PRs and other issues. + if (data.pull_request || !guard.isChangeRequestTitle(data.title)) { + continue; + } + titlesByIssue[issue] = data.title; + commentsByIssue[issue] = await github.paginate(github.rest.issues.listComments, { + owner: context.repo.owner, + repo: context.repo.repo, + issue_number: issue, + per_page: 100, + }); + } catch (error) { + core.warning(`Could not read comments on #${issue}: ${error.message}`); + lookupFailures.push({ issue, message: error.message }); + } + } + } + + const result = guard.evaluate({ + changedFiles, + issues, + register, + commentsByIssue, + titlesByIssue, + lookupFailures, + }); + await core.summary.addHeading('Locked Files Guard', 3).addRaw(result.message).write(); + if (result.ok) { + core.info(result.message); + } else { + core.setFailed(result.message); + } diff --git a/.gitignore b/.gitignore index d13e59f4e7..6e49404f19 100644 --- a/.gitignore +++ b/.gitignore @@ -79,3 +79,6 @@ node_modules/ .claude/.settings*.json /graft/ /graphify-out/ + +# Local evidence that names private repositories; never commit (spec 008, 2026-10-04) +/.private-evidence/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c3bf76eb1..f22adcb320 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Locked Files Guard** — Pull requests that change locked labels, issue types, templates or routing files now fail a check until a linked change request is approved by @ashleyshaw. (#3734) +- **Governance Audit Phase 0-2 Infrastructure** — Added audit rule loader, violation tracker and report generator scripts with governance rules, plus Spec 006 design docs. (#3367) +- **Governance audit report ids are unique and self-consistent** — Fixed duplicate violation ids and recommendation references that no longer resolved, plus Spec 006 count and enforcement wording. (#3367) +- **Governance audit reports now render in a stable order** — Fixed report output changing with the order findings arrived, so two runs of the same audit are comparable. (#3367) - **Specs and Skills Validation Tooling** — Added validation tooling and docs for agent folders, skills registries, and manifest files. Registries now validate against the 014 contract. ([PR #3434](https://github.com/lightspeedwp/.github/pull/3434)) - **Label Write Queue** — Added a queue that spaces label writes one second apart and pauses on GitHub or Linear rate limits. Nothing calls it yet; later consolidation stages will. (#3704) - **Weekly Label Drift Check** — A scheduled workflow compares every org repository and Linear label against labels.yml and updates one drift report issue; read-only, never writes labels. (#3754) @@ -52,6 +56,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Label inventory token** — The organisation label inventory now needs its own dedicated token, refuses the GitHub Actions token and fails when private repositories are missing. (#3734) +- **Private Repositories Stay Out Of Public Reports** — The label inventory and the weekly drift report now count private repositories without naming them or listing their labels. Detail goes to a private repository when one is set. (#3734) - **Dependabot Merge Queue Fixed** — Dependabot pull requests no longer fail the Mergify queue check, because the queue now tests each pull request in place. ([PR #3817](https://github.com/lightspeedwp/.github/pull/3817)) - **Branch Cleanup Specs Closed Out** — The cloud environment and branch cleanup specs now show the merged audit and how a maintainer reviews `claude/*` branches before deletion. ([PR #3813](https://github.com/lightspeedwp/.github/pull/3813)) - **Cloud Spec Decisions Recorded** — Spec 018 now records seven clarified rules, including that empty `claude/*` branches are not auto-deleted yet, and the guard tests also cover the setup script's Node install. (#3726) diff --git a/CODEOWNERS b/CODEOWNERS index ac474f50c3..4a2e8dc762 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -16,10 +16,17 @@ /.github/workflows/ @ashleyshaw @lightspeedwp/lightspeed # Labels and Issue Types (critical for automation) -/.github/labels.yml @ashleyshaw @lightspeedwp/lightspeed -/.github/issue-types.yml @ashleyshaw @lightspeedwp/lightspeed /.github/labeler.yml @ashleyshaw @lightspeedwp/lightspeed +# LOCKED files (constitution Principle II, v1.4.0): only @ashleyshaw approves. +# Changes also need an approved change request (Locked Files Guard, spec 008 T080). +/.github/labels.yml @ashleyshaw +/.github/issue-types.yml @ashleyshaw +/.github/ISSUE_TEMPLATE/*.md @ashleyshaw +/.github/PULL_REQUEST_TEMPLATE/ @ashleyshaw +/.github/branch-types.yml @ashleyshaw +/.github/branch-labels.yml @ashleyshaw + # AI and Copilot Instructions /.github/custom-instructions.md @ashleyshaw @lightspeedwp/lightspeed /.github/agents/ @ashleyshaw @lightspeedwp/lightspeed @@ -41,3 +48,9 @@ # Schemas (JSON validation) /.schemas/ @ashleyshaw @lightspeedwp/lightspeed + +# Locked-file approval controls: only @ashleyshaw approves. +/.github/workflows/locked-files-guard.yml @ashleyshaw +/scripts/validation/check-locked-file-approval.cjs @ashleyshaw +/.github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json @ashleyshaw +/CODEOWNERS @ashleyshaw diff --git a/scripts/automation/__tests__/label-drift-check-config.test.js b/scripts/automation/__tests__/label-drift-check-config.test.js index 3dbe043527..a698fa7b65 100644 --- a/scripts/automation/__tests__/label-drift-check-config.test.js +++ b/scripts/automation/__tests__/label-drift-check-config.test.js @@ -16,7 +16,9 @@ */ const fs = require('fs'); +const os = require('os'); const path = require('path'); +const { spawnSync } = require('child_process'); const yaml = require('js-yaml'); const REPO_ROOT = path.resolve(__dirname, '..', '..', '..'); @@ -84,7 +86,7 @@ describe('label drift check wiring', () => { (step) => typeof step.uses === 'string' && step.uses.startsWith('actions/create-github-app-token@') ); - expect(mintSteps).toHaveLength(2); + expect(mintSteps).toHaveLength(3); const byId = Object.fromEntries(mintSteps.map((step) => [step.id, step])); // Read token: organisation-wide (no repositories key), read-only. expect(byId['app-token-read']).toBeDefined(); @@ -106,6 +108,73 @@ describe('label drift check wiring', () => { expect(byId['app-token-write']).toBeDefined(); expect(String(byId['app-token-write'].with.repositories)).toContain('.github'); expect(String(byId['app-token-write'].with['permission-issues'])).toBe('write'); + // Private report token: issues write on the private report repository only, + // minted only when that repository is configured, and never named in this + // public file: the name comes from a variable. + const privateMint = byId['app-token-private']; + expect(privateMint).toBeDefined(); + expect(privateMint.if).toBe("steps.private-report.outputs.name != ''"); + expect(String(privateMint.with.repositories)).toBe('${{ steps.private-report.outputs.name }}'); + // The owner comes from the variable too, not a hard-coded value. + expect(String(privateMint.with.owner)).toBe('${{ steps.private-report.outputs.owner }}'); + expect(String(privateMint.with['permission-issues'])).toBe('write'); + expect(Object.keys(privateMint.with).filter((key) => key.startsWith('permission-'))).toEqual([ + 'permission-issues', + ]); + }); + + describe('resolving the private report repository', () => { + const run = (value) => { + const step = Object.values(loadWorkflow().jobs) + .flatMap((job) => job.steps || []) + .find((candidate) => candidate.id === 'private-report'); + const output = path.join(os.tmpdir(), `drift-resolve-${process.pid}-${Math.random()}`); + fs.writeFileSync(output, ''); + const result = spawnSync('bash', ['-c', step.run], { + encoding: 'utf8', + env: { PATH: process.env.PATH, PRIVATE_REPORT_REPO: value, GITHUB_OUTPUT: output }, + }); + const written = fs.readFileSync(output, 'utf8'); + fs.rmSync(output, { force: true }); + return { status: result.status, stdout: result.stdout, written }; + }; + + it('outputs the owner and the name of a lightspeedwp repository', () => { + expect(run('lightspeedwp/private-reports')).toMatchObject({ + status: 0, + written: 'owner=lightspeedwp\nname=private-reports\n', + }); + }); + + it('does nothing when the variable is unset, so the private detail is withheld', () => { + expect(run('')).toMatchObject({ status: 0, written: '' }); + }); + + it('rejects a repository under another owner before any token is minted', () => { + const result = run('other-org/reports'); + expect(result.status).toBe(1); + expect(result.stdout).toContain('must be a lightspeedwp repository'); + expect(result.written).toBe(''); + }); + + it.each([ + 'reports', + 'lightspeedwp/', + '/reports', + 'lightspeedwp/a/b', + 'lightspeedwp/x\nowner=evil', + ])('rejects a malformed value (%j) without writing outputs', (value) => { + const result = run(value); + expect(result.status).toBe(1); + expect(result.written).toBe(''); + }); + }); + + it('takes the private report repository from a variable and stores no token for it', () => { + const raw = workflowText(); + expect(raw).toContain('vars.PRIVATE_REPORT_REPO'); + expect(raw).toContain('steps.app-token-private.outputs.token'); + expect(raw).not.toMatch(/secrets\.PRIVATE_REPORT_TOKEN/); }); it('only invokes npm scripts that package.json declares', () => { diff --git a/scripts/automation/__tests__/label-drift-check.test.js b/scripts/automation/__tests__/label-drift-check.test.js index b761b92765..f6af6585c9 100644 --- a/scripts/automation/__tests__/label-drift-check.test.js +++ b/scripts/automation/__tests__/label-drift-check.test.js @@ -22,6 +22,7 @@ import { diffGithubRepo, diffLinearLabels, countGithubItems, + enrichAllGithubRows, enrichGithubRows, parseFirstSeen, parseFirstSeenBlock, @@ -29,6 +30,13 @@ import { buildFirstSeenBlock, renderReport, upsertDriftIssue, + PRIVATE_DRIFT_ISSUE_TITLE, + summarisePrivate, + renderPrivateSummary, + renderPrivateReport, + maskCommands, + assertPrivateRepository, + resolvePrivateSink, } from '../label-drift-check.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); @@ -294,6 +302,95 @@ describe('label-drift-check', () => { expect(await countGithubItems(client, 'o/r', 'a')).toBe(3); }); + describe('one search budget for the public and private passes', () => { + const makeClient = () => { + const queries = []; + return { + queries, + rest: { + search: { + issuesAndPullRequests: async ({ q }) => { + queries.push(q); + return { data: { total_count: 1 } }; + }, + }, + }, + }; + }; + const rowsFor = (repo, count) => + Array.from({ length: count }, (_, i) => ({ + location: repo, + label: `label-${i}`, + difference: 'unapproved', + })); + + it('gives the private pass only what the public pass left unused', async () => { + const client = makeClient(); + const publicRows = rowsFor('o/public', 3); + const privateRows = rowsFor('o/private', 5); + const result = await enrichAllGithubRows(client, publicRows, privateRows, { + includePrivate: true, + maxRows: 5, + paceMs: 0, + }); + expect(result).toEqual({ counted: 3, uncounted: 0, privateCounted: 2 }); + expect(client.queries).toHaveLength(5); + expect(privateRows.filter((row) => row.items === 1)).toHaveLength(2); + expect(privateRows.filter((row) => row.items === '')).toHaveLength(3); + }); + + it('never exceeds the shared cap across both passes', async () => { + const client = makeClient(); + await enrichAllGithubRows(client, rowsFor('o/public', 4), rowsFor('o/private', 4), { + includePrivate: true, + maxRows: 6, + paceMs: 0, + }); + expect(client.queries).toHaveLength(6); + }); + + it('makes no private search when the public pass used the whole budget', async () => { + const client = makeClient(); + const privateRows = rowsFor('o/private', 3); + const result = await enrichAllGithubRows(client, rowsFor('o/public', 4), privateRows, { + includePrivate: true, + maxRows: 4, + paceMs: 0, + }); + expect(result.privateCounted).toBe(0); + expect(client.queries.some((q) => q.includes('o/private'))).toBe(false); + expect(privateRows.every((row) => row.items === '')).toBe(true); + }); + + it('skips the private pass entirely when it is not included, as in a dry run', async () => { + const client = makeClient(); + const privateRows = rowsFor('o/private', 3); + const result = await enrichAllGithubRows(client, rowsFor('o/public', 2), privateRows, { + includePrivate: false, + maxRows: 10, + paceMs: 0, + }); + expect(result).toEqual({ counted: 2, uncounted: 0, privateCounted: 0 }); + expect(client.queries.every((q) => q.includes('o/public'))).toBe(true); + expect(privateRows.every((row) => row.items === undefined)).toBe(true); + }); + + it('does not spend budget on missing rows', async () => { + const client = makeClient(); + const publicRows = [ + ...rowsFor('o/public', 1), + { location: 'o/public', label: 'gone', difference: 'missing' }, + ]; + const result = await enrichAllGithubRows(client, publicRows, rowsFor('o/private', 2), { + includePrivate: true, + maxRows: 2, + paceMs: 0, + }); + expect(result.counted).toBe(1); + expect(result.privateCounted).toBe(1); + }); + }); + it('persists first-seen dates for rows omitted from the tables', () => { const rows = Array.from({ length: 5 }, (_, i) => ({ location: 'o/r', @@ -399,4 +496,137 @@ describe('label-drift-check', () => { { owner: 'o', repo: 'r', issue_number: 7, body: 'body two' }, ]); }); + + // This repository is public, so a private repository is counted and never + // named, in the issue, the artifact or the log (decision of 2026-10-04). + describe('private repositories', () => { + const privateRepos = [ + { repository: 'lightspeedwp/acme-client-site', private: true, labels: [] }, + { repository: 'lightspeedwp/other-client-site', private: true, labels: [] }, + ]; + const privateRows = [ + { location: 'lightspeedwp/acme-client-site', label: 'acme-launch', difference: 'unapproved' }, + { + location: 'lightspeedwp/acme-client-site', + label: 'type:bug', + difference: 'colour mismatch', + }, + { location: 'lightspeedwp/other-client-site', label: 'area:core', difference: 'missing' }, + ]; + + it('counts private drift without naming a repository or a label', () => { + const summary = summarisePrivate(privateRepos, privateRows); + expect(summary).toEqual({ + checked: 2, + withDifferences: 2, + differences: 3, + byKind: { unapproved: 1, 'colour mismatch': 1, missing: 1 }, + }); + const text = renderPrivateSummary(summary); + for (const leaked of ['acme', 'other-client', 'acme-launch', 'area:core']) { + expect(text).not.toContain(leaked); + } + expect(text).toContain('2 private repositories were checked'); + expect(text).toContain('3 difference(s)'); + }); + + it('says so plainly when no private repository has drift', () => { + expect(renderPrivateSummary(summarisePrivate(privateRepos, []))).toContain( + 'None of them has drift.' + ); + }); + + it('puts the summary in the public report and counts it in the total', () => { + const body = renderReport({ + generatedAt: '2026-10-04T00:00:00.000Z', + canonicalCommit: 'abc123', + githubRows: [], + linearRows: [], + allowed: [], + skippedRepos: [], + org: 'lightspeedwp', + firstSeen: new Map(), + privateSummary: summarisePrivate(privateRepos, privateRows), + }); + expect(body).toContain('## Private repositories'); + expect(body).toContain('3 difference(s) across GitHub and Linear.'); + expect(body).toContain('Private repository differences (names withheld): 3'); + for (const leaked of ['acme-client-site', 'other-client-site', 'acme-launch']) { + expect(body).not.toContain(leaked); + } + }); + + it('writes the detail only into the private report', () => { + const body = renderPrivateReport({ + generatedAt: '2026-10-04T00:00:00.000Z', + canonicalCommit: 'abc123', + rows: privateRows, + org: 'lightspeedwp', + }); + expect(body).toContain('lightspeedwp/acme-client-site'); + expect(body).toContain('3 difference(s)'); + }); + + it('registers every private name as a log mask inside GitHub Actions only', () => { + const inventory = { + repositories: [{ repository: 'lightspeedwp/public-tool', private: false }, ...privateRepos], + }; + expect(maskCommands(inventory, { GITHUB_ACTIONS: 'true' })).toEqual([ + '::add-mask::lightspeedwp/acme-client-site', + '::add-mask::acme-client-site', + '::add-mask::lightspeedwp/other-client-site', + '::add-mask::other-client-site', + ]); + expect(maskCommands(inventory, {})).toEqual([]); + }); + + it('refuses to write the private report to a repository that is not private', async () => { + const publicClient = { rest: { repos: { get: async () => ({ data: { private: false } }) } } }; + await expect(assertPrivateRepository(publicClient, 'o', 'r')).rejects.toThrow( + /not a private repository/ + ); + const privateClient = { rest: { repos: { get: async () => ({ data: { private: true } }) } } }; + await expect(assertPrivateRepository(privateClient, 'o', 'r')).resolves.toBeUndefined(); + }); + + it('has no private sink unless one is configured, and needs its token and a valid name', async () => { + await expect(resolvePrivateSink({})).resolves.toBeNull(); + await expect(resolvePrivateSink({ PRIVATE_REPORT_REPO: 'bad' })).rejects.toThrow( + /owner\/repository/ + ); + // Extra segments would be dropped silently, and the privacy check would run + // against a different repository than the one configured. + for (const value of ['org/repo/extra', 'org/repo/', '/repo', 'org/']) { + await expect(resolvePrivateSink({ PRIVATE_REPORT_REPO: value })).rejects.toThrow( + /owner\/repository/ + ); + } + await expect(resolvePrivateSink({ PRIVATE_REPORT_REPO: 'o/r' })).rejects.toThrow( + /PRIVATE_REPORT_TOKEN/ + ); + const client = { rest: { repos: { get: async () => ({ data: { private: true } }) } } }; + const sink = await resolvePrivateSink( + { PRIVATE_REPORT_REPO: 'o/r', PRIVATE_REPORT_TOKEN: 't' }, + () => client + ); + expect(sink).toEqual({ client, owner: 'o', repo: 'r' }); + }); + + it('upserts the private report under its own title', async () => { + const creates = []; + const client = { + rest: { + search: { issuesAndPullRequests: async () => ({ data: { items: [] } }) }, + issues: { + create: async (params) => { + creates.push(params); + return { data: { number: 1 } }; + }, + }, + }, + }; + await upsertDriftIssue(client, client, 'o', 'r', 'b', PRIVATE_DRIFT_ISSUE_TITLE); + expect(creates[0].title).toBe('Label drift report (private repositories)'); + }); + }); }); diff --git a/scripts/automation/__tests__/label-inventory.test.js b/scripts/automation/__tests__/label-inventory.test.js index 82b00d27b2..a11efa2a53 100644 --- a/scripts/automation/__tests__/label-inventory.test.js +++ b/scripts/automation/__tests__/label-inventory.test.js @@ -3,12 +3,22 @@ * @module scripts/automation/__tests__/label-inventory.test.js */ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; import { describe, it, expect } from '@jest/globals'; import { PER_PAGE, collectPages, buildInventory, incompleteRepositories, + incompleteRepositoryLabels, + findRepositoryRoot, + isInsidePublicRepository, + privateRepositoryGap, + publicRepositoryName, + redactInventory, + resolveToken, } from '../label-inventory.js'; const labelsOf = (n, prefix = 'l') => @@ -76,4 +86,250 @@ describe('label-inventory', () => { }; expect(incompleteRepositories(inv)).toEqual(['o/full', 'o/short']); }); + + describe('token and private repository guards (T075)', () => { + it('reads the token from LABEL_INVENTORY_TOKEN, not GITHUB_TOKEN', () => { + expect(resolveToken({ LABEL_INVENTORY_TOKEN: 'org-token' })).toEqual({ token: 'org-token' }); + expect(resolveToken({ GITHUB_TOKEN: 'repo-token' }).error).toMatch( + /LABEL_INVENTORY_TOKEN is required/ + ); + }); + + it('refuses to run inside GitHub Actions even when a token is set', () => { + const result = resolveToken({ GITHUB_ACTIONS: 'true', LABEL_INVENTORY_TOKEN: 'org-token' }); + expect(result.token).toBeUndefined(); + expect(result.error).toMatch(/refusing to run inside GitHub Actions/); + }); + + it('records private repositories and the organisation-reported count', async () => { + const client = pagedClient([{ name: 'pub' }, { name: 'priv', private: true }], { + pub: 1, + priv: 2, + }); + client.rest.orgs = { get: async () => ({ data: { total_private_repos: 1 } }) }; + const inv = await buildInventory(client, 'lightspeedwp'); + expect(inv.private_repository_count).toBe(1); + expect(inv.reported_private_repository_count).toBe(1); + expect(inv.repositories.find((r) => r.repository === 'lightspeedwp/priv').private).toBe(true); + expect(privateRepositoryGap(inv)).toBeNull(); + }); + + it('fails when the token cannot see every private repository', async () => { + const client = pagedClient([{ name: 'pub' }], { pub: 1 }); + client.rest.orgs = { get: async () => ({ data: { total_private_repos: 3 } }) }; + const inv = await buildInventory(client, 'lightspeedwp'); + expect(privateRepositoryGap(inv)).toMatch( + /listed 0 private repositories but lightspeedwp reports 3/ + ); + }); + + it('fails when the organisation count is not readable', async () => { + const inv = await buildInventory(pagedClient([{ name: 'a' }], { a: 1 }), 'lightspeedwp'); + expect(inv.reported_private_repository_count).toBeNull(); + expect(privateRepositoryGap(inv)).toMatch( + /total_private_repos.*organisation-owner token is required/ + ); + }); + + it.each([null, undefined])( + 'requires organisation-owner permission when the reported count is %s', + (reported) => { + expect( + privateRepositoryGap({ + organisation: 'lightspeedwp', + private_repository_count: 0, + reported_private_repository_count: reported, + }) + ).toMatch(/total_private_repos.*organisation-owner token is required/); + } + ); + + it('accepts an organisation that reports zero private repositories', () => { + expect( + privateRepositoryGap({ + organisation: 'lightspeedwp', + private_repository_count: 0, + reported_private_repository_count: 0, + }) + ).toBeNull(); + }); + }); + + // This repository is public, so the tracked evidence must not name a private + // repository or list its labels (decision of 2026-10-04), while completeness is + // still judged on every repository. + describe('redacting private repositories', () => { + const inventory = { + generated_at: '2026-10-04T00:00:00.000Z', + organisation: 'lightspeedwp', + repository_count: 3, + private_repository_count: 2, + reported_private_repository_count: 2, + label_total: 12, + repositories: [ + { + repository: 'lightspeedwp/public-tool', + archived: false, + fork: false, + private: false, + label_count: 2, + pages_read: 1, + labels: [{ name: 'type:bug', color: 'AAAAAA', description: 'x' }], + }, + { + repository: 'lightspeedwp/acme-client-site', + archived: false, + fork: false, + private: true, + label_count: 7, + pages_read: 1, + labels: [{ name: 'acme-launch', color: 'BBBBBB', description: 'secret' }], + }, + { + repository: 'lightspeedwp/other-client-site', + archived: true, + fork: false, + private: true, + label_count: 3, + pages_read: 1, + labels: [], + }, + ], + }; + + it('names no private repository and lists none of its labels', () => { + const text = JSON.stringify(redactInventory(inventory)); + for (const leaked of ['acme-client-site', 'other-client-site', 'acme-launch', 'secret']) { + expect(text).not.toContain(leaked); + } + }); + + it('keeps public repositories whole and private ones as counts under placeholders', () => { + const redacted = redactInventory(inventory); + expect(redacted.repositories[0]).toEqual(inventory.repositories[0]); + expect(redacted.repositories[1]).toEqual({ + repository: 'lightspeedwp/private-repository-001', + archived: false, + fork: false, + private: true, + label_count: 7, + pages_read: 1, + }); + expect(redacted.repositories[2].repository).toBe('lightspeedwp/private-repository-002'); + expect(redacted.redacted).toBe(true); + expect(redacted.private_repository_count).toBe(2); + expect(redacted.label_total).toBe(12); + }); + + it('leaves the full inventory untouched', () => { + redactInventory(inventory); + expect(inventory.repositories[1].repository).toBe('lightspeedwp/acme-client-site'); + expect(inventory.repositories[1].labels).toHaveLength(1); + }); + + it('still passes the completeness checks, because they ran on the full inventory', () => { + const redacted = redactInventory(inventory); + expect(incompleteRepositories(redacted)).toEqual([]); + expect(privateRepositoryGap(redacted)).toBeNull(); + }); + + it('says a private repository in a message, not its name', () => { + expect(publicRepositoryName(inventory.repositories[0])).toBe('lightspeedwp/public-tool'); + expect(publicRepositoryName(inventory.repositories[1])).toBe('a private repository'); + const broken = { + ...inventory, + repositories: inventory.repositories.map((repo) => ({ ...repo, pages_read: 0 })), + }; + const labels = incompleteRepositoryLabels(broken); + expect(labels).toContain('lightspeedwp/public-tool'); + expect(labels.join(' ')).not.toContain('client-site'); + }); + + it('refuses a full-output path anywhere inside the public repository except .private-evidence', () => { + const cwd = '/repo'; + for (const refused of [ + '.github/reports/full.json', + '.github', + 'full.json', + 'docs/full.json', + 'scripts/automation/full.json', + '.githubx/full.json', + '.private-evidencex/full.json', + '.', + 'docs/../full.json', + ]) { + expect(isInsidePublicRepository(refused, cwd)).toBe(true); + } + for (const allowed of [ + '.private-evidence/full.json', + '.private-evidence/nested/full.json', + '/tmp/full.json', + '../outside/full.json', + ]) { + expect(isInsidePublicRepository(allowed, cwd)).toBe(false); + } + }); + + it('judges a path against the repository root, not the directory the script runs from', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'inventory-sub-')); + try { + fs.mkdirSync(path.join(root, 'scripts')); + fs.mkdirSync(path.join(root, 'docs')); + fs.mkdirSync(path.join(root, '.private-evidence')); + const cwd = path.join(root, 'scripts'); + // From scripts/, ../docs/full.json starts with .. but is still inside the repository. + expect(isInsidePublicRepository('../docs/full.json', root, cwd)).toBe(true); + expect(isInsidePublicRepository('full.json', root, cwd)).toBe(true); + expect(isInsidePublicRepository('../.private-evidence/full.json', root, cwd)).toBe(false); + // The old behaviour took the working directory as the root and accepted the first one. + expect(isInsidePublicRepository('../docs/full.json', cwd)).toBe(false); + expect(isInsidePublicRepository('../../outside/full.json', root, cwd)).toBe(false); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('finds the repository root from the script location, whatever the working directory', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'inventory-root-')); + try { + const scriptDir = path.join(root, 'scripts', 'automation'); + fs.mkdirSync(scriptDir, { recursive: true }); + const script = path.join(scriptDir, 'label-inventory.js'); + fs.writeFileSync(script, ''); + const real = fs.realpathSync(root); + // Git reports the top level. + const git = () => ({ status: 0, stdout: `${real}\n` }); + expect(findRepositoryRoot(script, git)).toBe(real); + // Without git, it is two directories above scripts/automation/. + const noGit = () => ({ status: 128, stdout: '' }); + expect(findRepositoryRoot(script, noGit)).toBe(real); + // It is asked from the script's directory, not the caller's. + const seen = []; + findRepositoryRoot(script, (command, args, options) => { + seen.push(options.cwd); + return git(); + }); + expect(seen).toEqual([fs.realpathSync(scriptDir)]); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + } + }); + + it('is not fooled by a path that climbs out of .private-evidence or a symlink into the repository', () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'inventory-root-')); + const outside = fs.mkdtempSync(path.join(os.tmpdir(), 'inventory-outside-')); + try { + fs.mkdirSync(path.join(root, '.private-evidence')); + fs.mkdirSync(path.join(root, 'docs')); + expect(isInsidePublicRepository('.private-evidence/../docs/full.json', root)).toBe(true); + fs.symlinkSync(path.join(root, 'docs'), path.join(outside, 'link')); + expect(isInsidePublicRepository(path.join(outside, 'link', 'full.json'), root)).toBe(true); + fs.symlinkSync(outside, path.join(root, '.private-evidence', 'out')); + expect(isInsidePublicRepository('.private-evidence/out/full.json', root)).toBe(false); + } finally { + fs.rmSync(root, { recursive: true, force: true }); + fs.rmSync(outside, { recursive: true, force: true }); + } + }); + }); }); diff --git a/scripts/automation/label-drift-check.js b/scripts/automation/label-drift-check.js index 6579edceac..ccbbd27dc1 100644 --- a/scripts/automation/label-drift-check.js +++ b/scripts/automation/label-drift-check.js @@ -12,6 +12,14 @@ * reads, issue search, and create/update of the one report issue. It never * creates, edits or deletes labels (FR-017 rule 2, FR-018). * + * Private repositories (decision of 2026-10-04): this repository is public, so + * the issue, the JSON artifact and the log count private repositories but never + * name one or list its labels. Their detail goes only to a private repository, + * when `PRIVATE_REPORT_REPO` (owner/name) and `PRIVATE_REPORT_TOKEN` (issues + * write on that repository, minted by the workflow from the installed App) are + * set; the repository is confirmed private before anything is written, and + * without them the detail is withheld. + * * Authentication (FR-018): GitHub via an organisation-wide App installation * token passed as `GITHUB_TOKEN` (manual step T071a) for reads, plus a * report-scoped token as `GITHUB_WRITE_TOKEN` (issues write on the report @@ -25,10 +33,11 @@ import fs from 'fs'; import path from 'path'; import * as yaml from 'js-yaml'; -import { buildInventory, incompleteRepositories } from './label-inventory.js'; +import { buildInventory, incompleteRepositoryLabels } from './label-inventory.js'; import { buildLinearInventory } from './linear-label-inventory.js'; export const DRIFT_ISSUE_TITLE = 'Label drift report'; +export const PRIVATE_DRIFT_ISSUE_TITLE = 'Label drift report (private repositories)'; export const DRIFT_ISSUE_LABELS = ['type:audit', 'area:labels', 'status:needs-triage']; // Team-scoped Linear labels documented in spec 008 FR-012 as project labels @@ -324,6 +333,7 @@ export function renderReport({ uncountedRows = 0, maxTableRows = MAX_TABLE_ROWS, maxFirstSeenBytes = MAX_FIRST_SEEN_BYTES, + privateSummary = null, }) { const today = generatedAt.slice(0, 10); const rows = [...githubRows, ...linearRows].sort((a, b) => @@ -339,9 +349,9 @@ export function renderReport({ const lines = [ '## Summary', '', - withDates.length === 0 + withDates.length + (privateSummary?.differences ?? 0) === 0 ? 'No drift.' - : `${withDates.length} difference(s) across GitHub and Linear.`, + : `${withDates.length + (privateSummary?.differences ?? 0)} difference(s) across GitHub and Linear.`, '', '## GitHub repositories', '', @@ -360,6 +370,11 @@ export function renderReport({ `| … | _Showing ${maxTableRows} of ${githubOnly.length}; see the run artifact for all rows._ | | | |` ); } + if (privateSummary) { + // This repository is public. A private repository is counted, never named, + // and its labels are not listed (decision of 2026-10-04). + lines.push('', '## Private repositories', '', renderPrivateSummary(privateSummary)); + } lines.push( '', '## Linear', @@ -398,6 +413,9 @@ export function renderReport({ `- Organisation: ${org}`, `- Approved set: .github/labels.yml at ${canonicalCommit}`, `- GitHub differences: ${githubRows.length}`, + ...(privateSummary + ? [`- Private repository differences (names withheld): ${privateSummary.differences}`] + : []), `- Linear differences: ${linearRows.length}`, `- Allowed team-scoped exceptions: ${allowed.length}`, uncountedRows === 0 @@ -414,6 +432,82 @@ export function renderReport({ return lines.join('\n'); } +/** + * Counts for the private repositories, safe for a public report: no name and no + * label appears in it. + * @param {Array} privateRepos Private inventory entries. + * @param {Array} privateRows Drift rows for those repositories. + * @returns {{checked: number, withDifferences: number, differences: number, byKind: Record}} + */ +export function summarisePrivate(privateRepos, privateRows) { + const byKind = {}; + for (const row of privateRows) byKind[row.difference] = (byKind[row.difference] ?? 0) + 1; + return { + checked: privateRepos.length, + withDifferences: new Set(privateRows.map((row) => row.location)).size, + differences: privateRows.length, + byKind, + }; +} + +/** + * The paragraph for the public report. + * @param {ReturnType} summary + * @returns {string} + */ +export function renderPrivateSummary(summary) { + const kinds = Object.entries(summary.byKind) + .sort(([a], [b]) => a.localeCompare(b)) + .map(([kind, count]) => `${count} ${kind}`) + .join(', '); + return [ + `${summary.checked} private repositories were checked. Their names and labels are withheld from this public repository.`, + summary.differences === 0 + ? 'None of them has drift.' + : `${summary.withDifferences} of them have drift: ${summary.differences} difference(s) (${kinds}). The detail is in the private report, when one is configured.`, + ].join('\n\n'); +} + +/** + * The report for a private repository, which names the repositories and so must + * only ever be written to a private repository. + * @param {object} options + * @returns {string} + */ +export function renderPrivateReport({ generatedAt, canonicalCommit, rows, org, maxRows = 400 }) { + const sorted = sortRows(rows); + const lines = [ + '## Summary', + '', + sorted.length === 0 + ? 'No drift in private repositories.' + : `${sorted.length} difference(s) across ${org}'s private repositories.`, + '', + '## Private repositories', + '', + '| Location | Label | Difference | Items |', + '| --- | --- | --- | --- |', + ]; + if (sorted.length === 0) lines.push('| — | — | — | — |'); + for (const row of sorted.slice(0, maxRows)) { + lines.push( + `| ${escapeCell(row.location)} | ${escapeCell(row.label)} | ${row.difference} | ${row.items ?? ''} |` + ); + } + if (sorted.length > maxRows) { + lines.push(`| … | _Showing ${maxRows} of ${sorted.length}._ | | |`); + } + lines.push( + '', + '## Run details', + '', + `- Generated at: ${generatedAt}`, + `- Organisation: ${org}`, + `- Approved set: .github/labels.yml at ${canonicalCommit}` + ); + return lines.join('\n'); +} + function sortRows(rows) { return [...rows].sort((a, b) => firstSeenKey(a.location, a.label, a.difference).localeCompare( @@ -437,12 +531,29 @@ const SEARCH_PACING_MS = 2200; * Find the open drift report issue by exact title. * @returns {Promise} Issue or null. */ -export async function findDriftIssue(client, owner, repo) { +export async function findDriftIssue(client, owner, repo, title = DRIFT_ISSUE_TITLE) { const { data } = await client.rest.search.issuesAndPullRequests({ - q: `repo:${owner}/${repo} "${DRIFT_ISSUE_TITLE}" in:title type:issue state:open`, + q: `repo:${owner}/${repo} "${title}" in:title type:issue state:open`, per_page: 10, }); - return data.items.find((item) => item.title === DRIFT_ISSUE_TITLE) ?? null; + return data.items.find((item) => item.title === title) ?? null; +} + +/** + * Refuse to name private repositories anywhere that is not private. The private + * report is only written to a repository this call has confirmed is private. + * @param {object} client Octokit-style client with read access to the repository. + * @param {string} owner Repository owner. + * @param {string} repo Repository name. + * @returns {Promise} + */ +export async function assertPrivateRepository(client, owner, repo) { + const { data } = await client.rest.repos.get({ owner, repo }); + if (data.private !== true) { + throw new Error( + 'PRIVATE_REPORT_REPO is not a private repository, so the private report is not written: it names private repositories' + ); + } } /** @@ -451,13 +562,20 @@ export async function findDriftIssue(client, owner, repo) { * Lookup runs on the read client; the create/update runs on the * report-scoped write client, so a read-only run cannot write at all. */ -export async function upsertDriftIssue(readClient, writeClient, owner, repo, body) { - const existing = await findDriftIssue(readClient, owner, repo); +export async function upsertDriftIssue( + readClient, + writeClient, + owner, + repo, + body, + title = DRIFT_ISSUE_TITLE +) { + const existing = await findDriftIssue(readClient, owner, repo, title); if (!existing) { const { data } = await writeClient.rest.issues.create({ owner, repo, - title: DRIFT_ISSUE_TITLE, + title, body, labels: DRIFT_ISSUE_LABELS, }); @@ -472,6 +590,38 @@ export async function upsertDriftIssue(readClient, writeClient, owner, repo, bod return { issue: data, created: false }; } +/** + * Counts items for both row sets under one search budget. + * + * Each paced search takes about 2.2 seconds, so two full budgets could overrun + * the job timeout and lose both reports. The private pass gets only what the + * public pass left unused, and it is skipped unless `includePrivate` is set + * (a dry run never writes the private report, so it does not need the counts). + * + * @param {object} client - GitHub client + * @param {object[]} publicRows - Public drift rows, enriched in place + * @param {object[]} privateRows - Private drift rows, enriched in place + * @param {object} [options] - `includePrivate`, `maxRows` (shared cap), `paceMs` + * @returns {Promise<{ counted: number, uncounted: number, privateCounted: number }>} Public counts and private rows counted + */ +export async function enrichAllGithubRows(client, publicRows, privateRows, options = {}) { + const maxRows = options.maxRows ?? MAX_COUNTED_ROWS; + const paceMs = options.paceMs; + const publicPass = await enrichGithubRows(client, publicRows, { maxRows, paceMs }); + if (!options.includePrivate) { + return { counted: publicPass.counted, uncounted: publicPass.uncounted, privateCounted: 0 }; + } + const privatePass = await enrichGithubRows(client, privateRows, { + maxRows: Math.max(maxRows - publicPass.counted, 0), + paceMs, + }); + return { + counted: publicPass.counted, + uncounted: publicPass.uncounted, + privateCounted: privatePass.counted, + }; +} + /** * Enrich GitHub drift rows with issue/PR counts, paced for the search rate * limit. `missing` rows carry no items; rows past MAX_COUNTED_ROWS keep an @@ -502,6 +652,56 @@ export async function enrichGithubRows(client, rows, options = {}) { return { counted, uncounted }; } +/** + * Actions workflow commands that mask every private repository name in the log. + * `add-mask` only covers the log, so it is a backstop and not the control: the + * control is that no name is ever written to the issue, the artifact or the log. + * @param {object} inventory Result of buildInventory. + * @param {Record} [env] Environment. + * @returns {string[]} Lines to print, empty outside GitHub Actions. + */ +export function maskCommands(inventory, env = process.env) { + if (env.GITHUB_ACTIONS !== 'true') return []; + const lines = []; + for (const repo of inventory.repositories) { + if (!repo.private) continue; + lines.push(`::add-mask::${repo.repository}`); + const bare = repo.repository.split('/').pop(); + if (bare) lines.push(`::add-mask::${bare}`); + } + return lines; +} + +/** + * The private report target, when one is configured. + * @param {Record} env Environment. + * @param {(options: object) => object} [makeClient] Builds an Octokit-style client. + * @returns {Promise<{client: object, owner: string, repo: string}|null>} Null when + * PRIVATE_REPORT_REPO is not set. + */ +export async function resolvePrivateSink(env, makeClient = null) { + const target = env.PRIVATE_REPORT_REPO; + if (!target) return null; + const parts = target.split('/'); + const [owner, repo] = parts; + // Exactly two segments: extra ones would be dropped silently, and the privacy + // check would then run against a different repository than the one configured. + if (parts.length !== 2 || !owner || !repo) { + throw new Error('PRIVATE_REPORT_REPO must be owner/repository'); + } + const token = env.PRIVATE_REPORT_TOKEN; + if (!token) { + throw new Error( + 'PRIVATE_REPORT_TOKEN is required when PRIVATE_REPORT_REPO is set (issues write on that private repository only)' + ); + } + const client = makeClient + ? makeClient({ auth: token }) + : new (await import('octokit')).Octokit({ auth: token }); + await assertPrivateRepository(client, owner, repo); + return { client, owner, repo }; +} + function parseArgs(argv) { const args = { org: 'lightspeedwp', @@ -540,48 +740,69 @@ async function main() { const canonical = await loadCanonicalLabels(path.join(process.cwd(), '.github', 'labels.yml')); const inventory = await buildInventory(client, org); - const incomplete = incompleteRepositories(inventory); + // Defence in depth for the Actions log: the names below are never printed, but + // registering them as masks means an accidental print is replaced by asterisks. + for (const line of maskCommands(inventory)) console.log(line); + const incomplete = incompleteRepositoryLabels(inventory); if (incomplete.length > 0) { throw new Error(`Incomplete GitHub pagination for: ${incomplete.join(', ')}`); } - const skippedRepos = inventory.repositories - .filter((r) => r.archived || r.fork) + // This repository is public, so a private repository is never named in the + // issue, the artifact or the log (decision of 2026-10-04). It is still checked. + const publicSkipped = inventory.repositories + .filter((r) => !r.private && (r.archived || r.fork)) .map((r) => r.repository); const activeRepos = inventory.repositories.filter((r) => !r.archived && !r.fork); + const activePublic = activeRepos.filter((r) => !r.private); + const activePrivate = activeRepos.filter((r) => r.private); const linear = await buildLinearInventory(); const githubRows = []; - for (const repoEntry of activeRepos) { + for (const repoEntry of activePublic) { githubRows.push(...diffGithubRepo(repoEntry, canonical)); } + const privateRows = []; + for (const repoEntry of activePrivate) { + privateRows.push(...diffGithubRepo(repoEntry, canonical)); + } const { rows: linearRows, allowed } = diffLinearLabels(linear.labels, canonical); + // Where the private detail goes, if anywhere: a private repository, confirmed + // private before anything is written to it. + const privateSink = await resolvePrivateSink(process.env); + // Enrich GitHub rows with issue/PR counts, paced for the search rate limit. - const { uncounted: uncountedRows } = await enrichGithubRows(client, githubRows); + const { uncounted: uncountedRows } = await enrichAllGithubRows(client, githubRows, privateRows, { + includePrivate: Boolean(privateSink) && !dryRun, + }); let firstSeen = new Map(); if (!dryRun) { const existing = await findDriftIssue(client, owner, repoName); if (existing) firstSeen = parseFirstSeen(existing.body); } + const privateSummary = summarisePrivate(activePrivate, privateRows); + const canonicalCommit = process.env.GITHUB_SHA ?? 'local run'; const body = renderReport({ generatedAt, - canonicalCommit: process.env.GITHUB_SHA ?? 'local run', + canonicalCommit, githubRows: sortRows(githubRows), linearRows: sortRows(linearRows), allowed, - skippedRepos, + skippedRepos: publicSkipped, org, firstSeen, uncountedRows, + privateSummary, }); const report = { generated_at: generatedAt, organisation: org, canonical_labels: canonical.size, - github_repositories: activeRepos.length, - skipped_repositories: skippedRepos, + github_repositories: activePublic.length, + private_repositories: privateSummary, + skipped_repositories: publicSkipped, linear_labels: linear.labels.length, differences: sortRows([...githubRows, ...linearRows]), allowed_exceptions: allowed.map((label) => ({ @@ -602,6 +823,29 @@ async function main() { console.log( `✅ ${report.differences.length} difference(s), ${allowed.length} allowed exception(s) → ${created ? 'created' : 'updated'} issue #${issue.number}` ); + if (privateSink) { + const privateBody = renderPrivateReport({ + generatedAt, + canonicalCommit, + rows: privateRows, + org, + }); + const written = await upsertDriftIssue( + privateSink.client, + privateSink.client, + privateSink.owner, + privateSink.repo, + privateBody, + PRIVATE_DRIFT_ISSUE_TITLE + ); + console.log( + `✅ private repositories: ${privateSummary.differences} difference(s) → ${written.created ? 'created' : 'updated'} the private report (names withheld here)` + ); + } else if (privateSummary.differences > 0) { + console.log( + `ℹ️ ${privateSummary.differences} private repository difference(s) found; the detail is withheld because PRIVATE_REPORT_REPO is not set` + ); + } } if (process.argv[1] && path.basename(process.argv[1]) === 'label-drift-check.js') { diff --git a/scripts/automation/label-inventory.js b/scripts/automation/label-inventory.js index 9a0180bffe..3ef740577d 100644 --- a/scripts/automation/label-inventory.js +++ b/scripts/automation/label-inventory.js @@ -9,15 +9,31 @@ * label-sync.js read only the first 100). Read-only: it never creates, * edits or deletes labels. * + * Credentials (FR-018, task T075): the token comes from LABEL_INVENTORY_TOKEN, + * a token with organisation read access (the organisation GitHub App or a + * fine-grained token). The repository-scoped Actions GITHUB_TOKEN sees only + * public repositories in other organisation repos, so the script refuses to + * run inside GitHub Actions and fails when private repositories are missing. + * + * Private repositories (decision of 2026-10-04): they are inventoried, so the + * clean-up covers them, but this repository is public, so the file committed to + * it never names one. `--output` (the tracked evidence file) is always the + * redacted inventory; `--full-output` writes the complete one and must be + * under `.private-evidence/` (ignored by git) or outside this repository, never + * anywhere else inside it. + * * Usage: - * GITHUB_TOKEN=... node scripts/automation/label-inventory.js [--org lightspeedwp] [--output path] + * LABEL_INVENTORY_TOKEN=... node scripts/automation/label-inventory.js [--org lightspeedwp] [--output path] [--full-output path] */ +import { spawnSync } from 'child_process'; import fs from 'fs'; import path from 'path'; export const PER_PAGE = 100; +export const TOKEN_VARIABLE = 'LABEL_INVENTORY_TOKEN'; + const DEFAULT_OUTPUT = path.join( '.github', 'reports', @@ -51,6 +67,16 @@ export async function collectPages(fetchPage) { * @returns {Promise} Inventory with one entry per repository. */ export async function buildInventory(client, org) { + // The organisation's own count of private repositories, used to detect a + // token that cannot see them. Null when the client cannot read it. + let reportedPrivateCount = null; + if (client.rest.orgs && typeof client.rest.orgs.get === 'function') { + const { data: orgData } = await client.rest.orgs.get({ org }); + if (Number.isInteger(orgData.total_private_repos)) { + reportedPrivateCount = orgData.total_private_repos; + } + } + const { items: repos } = await collectPages(async (page) => { const { data } = await client.rest.repos.listForOrg({ org, @@ -76,6 +102,7 @@ export async function buildInventory(client, org) { repository: `${org}/${repo.name}`, archived: Boolean(repo.archived), fork: Boolean(repo.fork), + private: Boolean(repo.private), label_count: labels.length, pages_read: pagesRead, labels: labels.map((l) => ({ @@ -90,6 +117,8 @@ export async function buildInventory(client, org) { generated_at: new Date().toISOString(), organisation: org, repository_count: repositories.length, + private_repository_count: repositories.filter((r) => r.private).length, + reported_private_repository_count: reportedPrivateCount, label_total: repositories.reduce((n, r) => n + r.label_count, 0), repositories, }; @@ -112,36 +141,213 @@ export function incompleteRepositories(inventory) { .map((r) => r.repository); } +/** + * Check that the token could see every private repository the organisation + * reports, so a repository-scoped token cannot produce a silently incomplete + * inventory. + * @param {object} inventory Result of buildInventory. + * @returns {string|null} Error message, or null when the inventory is complete. + */ +export function privateRepositoryGap(inventory) { + const reported = inventory.reported_private_repository_count; + if (reported === null || reported === undefined) { + return `cannot read total_private_repos for ${inventory.organisation}; an organisation-owner token is required to verify private repository completeness`; + } + if (inventory.private_repository_count < reported) { + return `listed ${inventory.private_repository_count} private repositories but ${inventory.organisation} reports ${reported}; the token cannot see them all`; + } + return null; +} + +/** + * Resolve the token from the environment (FR-018, task T075). + * @param {Record} env Environment variables. + * @returns {{token?: string, error?: string}} The token, or why none can be used. + */ +export function resolveToken(env) { + if (env.GITHUB_ACTIONS === 'true') { + return { + error: + 'refusing to run inside GitHub Actions: the inventory needs an organisation read token, never the repository-scoped GITHUB_TOKEN', + }; + } + const token = env[TOKEN_VARIABLE]; + if (!token) { + return { + error: `${TOKEN_VARIABLE} is required (a token with organisation read access; never GITHUB_TOKEN)`, + }; + } + return { token }; +} + +/** + * Withhold what identifies a private repository, keeping what completeness + * needs. This repository is public, so the tracked evidence must not name a + * private repository or list its labels (decision of 2026-10-04): a private + * entry keeps its counts and flags under a numbered placeholder, and its + * label list is dropped. Public repositories are unchanged. Completeness is + * judged on the full inventory before this runs. + * @param {object} inventory Result of buildInventory. + * @returns {object} A copy that is safe to commit to a public repository. + */ +export function redactInventory(inventory) { + let number = 0; + const repositories = inventory.repositories.map((repo) => { + if (!repo.private) return repo; + number += 1; + return { + repository: `${inventory.organisation}/private-repository-${String(number).padStart(3, '0')}`, + archived: repo.archived, + fork: repo.fork, + private: true, + label_count: repo.label_count, + pages_read: repo.pages_read, + }; + }); + return { + ...inventory, + redacted: true, + redaction_note: + 'Private repository names and labels are withheld from this public repository; the counts are kept so completeness can be checked.', + repositories, + }; +} + +/** + * A repository's name for a message or a public report: its own name when it + * is public, and a generic phrase when it is private. + * @param {object} repo An inventory entry. + * @returns {string} + */ +export function publicRepositoryName(repo) { + return repo.private ? 'a private repository' : repo.repository; +} + +/** + * The repositories that fail the pagination rule, named only where public. + * @param {object} inventory Result of buildInventory. + * @returns {string[]} + */ +export function incompleteRepositoryLabels(inventory) { + const failing = new Set(incompleteRepositories(inventory)); + return inventory.repositories + .filter((repo) => failing.has(repo.repository)) + .map((repo) => publicRepositoryName(repo)); +} + +/** + * Resolves a path through symbolic links as far as it exists, so a link that + * points into the repository cannot be used to slip past the check below. + * @param {string} target Absolute path, which need not exist yet. + * @returns {string} The real path, with any missing tail appended. + */ +function resolveReal(target) { + let current = target; + const missing = []; + while (!fs.existsSync(current)) { + const parent = path.dirname(current); + if (parent === current) break; + missing.unshift(path.basename(current)); + current = parent; + } + return path.join(fs.realpathSync(current), ...missing); +} + +/** + * Tells whether a path would put the file inside this public repository, other + * than under the git-ignored `.private-evidence/` directory. The complete + * inventory names private repositories, so it may be written only there or + * outside the repository. + * @param {string} target Output path; a relative path is taken from `cwd`. + * @param {string} [root] Repository root. It is not the working directory: a + * script run from a subdirectory must still be judged against the repository. + * @param {string} [cwd] Directory a relative `target` is resolved from. + * @returns {boolean} True when the path is inside the repository and not private evidence. + */ +export function isInsidePublicRepository(target, root = process.cwd(), cwd = root) { + const realRoot = resolveReal(path.resolve(root)); + const resolved = resolveReal(path.resolve(cwd, target)); + const relative = path.relative(realRoot, resolved); + if (relative === '') return true; + if (relative.startsWith('..') || path.isAbsolute(relative)) return false; + return relative.split(path.sep)[0] !== '.private-evidence'; +} + +/** + * Finds the repository root from where this script lives, not from the working + * directory, so running it from a subdirectory cannot change which directory the + * output guard protects. Uses `git rev-parse --show-toplevel` and falls back to + * two directories above `scripts/automation/`. + * @param {string} [scriptPath] Path of this script. + * @param {typeof spawnSync} [run] Process runner, for tests. + * @returns {string} Absolute repository root. + */ +export function findRepositoryRoot(scriptPath = process.argv[1], run = spawnSync) { + const scriptDir = path.dirname(fs.realpathSync(scriptPath)); + const result = run('git', ['rev-parse', '--show-toplevel'], { cwd: scriptDir, encoding: 'utf8' }); + const top = result.status === 0 ? String(result.stdout).trim() : ''; + return top ? fs.realpathSync(top) : path.resolve(scriptDir, '..', '..'); +} + function parseArgs(argv) { - const args = { org: 'lightspeedwp', output: DEFAULT_OUTPUT }; + const args = { org: 'lightspeedwp', output: DEFAULT_OUTPUT, fullOutput: null }; for (let i = 0; i < argv.length; i += 1) { if (argv[i] === '--org') args.org = argv[(i += 1)]; else if (argv[i] === '--output') args.output = argv[(i += 1)]; + else if (argv[i] === '--full-output') args.fullOutput = argv[(i += 1)]; } return args; } async function main() { - const token = process.env.GITHUB_TOKEN; - if (!token) { - console.error('❌ GITHUB_TOKEN environment variable is required'); + const { token, error } = resolveToken(process.env); + if (error) { + console.error(`❌ ${error}`); process.exit(1); } const { Octokit } = await import('octokit'); const client = new Octokit({ auth: token }); - const { org, output } = parseArgs(process.argv.slice(2)); + const parsed = parseArgs(process.argv.slice(2)); + const { org, fullOutput } = parsed; + const output = + parsed.output === DEFAULT_OUTPUT + ? path.join(findRepositoryRoot(), DEFAULT_OUTPUT) + : parsed.output; + + // The full inventory may name private repositories, so it is only ever written + // under .private-evidence/ or outside this public repository, never anywhere + // else inside it. The repository is found from the script's location, so the + // check holds whichever directory the script is run from. Checked before any + // API call. + const root = findRepositoryRoot(); + if (fullOutput && isInsidePublicRepository(fullOutput, root, process.cwd())) { + console.error( + '❌ --full-output must be under .private-evidence/ or outside this repository, because the full inventory names private repositories and this repository is public' + ); + process.exit(1); + } const inventory = await buildInventory(client, org); - const incomplete = incompleteRepositories(inventory); + const incomplete = incompleteRepositoryLabels(inventory); if (incomplete.length > 0) { console.error(`❌ Incomplete pagination for: ${incomplete.join(', ')}`); process.exit(1); } + const privateGap = privateRepositoryGap(inventory); + if (privateGap) { + console.error(`❌ Incomplete inventory: ${privateGap}`); + process.exit(1); + } + if (fullOutput) { + fs.mkdirSync(path.dirname(fullOutput), { recursive: true }); + fs.writeFileSync(fullOutput, `${JSON.stringify(inventory, null, 2)}\n`); + } + const publicInventory = redactInventory(inventory); fs.mkdirSync(path.dirname(output), { recursive: true }); - fs.writeFileSync(output, `${JSON.stringify(inventory, null, 2)}\n`); + fs.writeFileSync(output, `${JSON.stringify(publicInventory, null, 2)}\n`); console.log( - `✅ ${inventory.repository_count} repositories, ${inventory.label_total} labels → ${output}` + `✅ ${inventory.repository_count} repositories (${inventory.private_repository_count} private, names withheld), ${inventory.label_total} labels → ${output}${fullOutput ? ` (full inventory: ${fullOutput})` : ''}` ); } diff --git a/scripts/validation/__tests__/check-locked-file-approval.test.js b/scripts/validation/__tests__/check-locked-file-approval.test.js new file mode 100644 index 0000000000..ce38ededf5 --- /dev/null +++ b/scripts/validation/__tests__/check-locked-file-approval.test.js @@ -0,0 +1,308 @@ +/** + * Tests for the locked-file approval guard (spec 008, task T080). + */ + +const { + APPROVER, + approvedInRegister, + evaluate, + isApprovalComment, + isChangeRequestTitle, + isLocked, + issuesFromRenderedHtml, + linkedIssues, +} = require('../check-locked-file-approval.cjs'); + +const register = { + requests: [ + { issue_number: 3556, status: 'approved' }, + { issue_number: 3557, status: 'pending' }, + ], +}; + +const titlesByIssue = { + 3556: '[ISSUE-TYPE-UPDATE-REQUEST] Replace Question with Decision', + 3557: '[TEMPLATE-UPDATE-REQUEST] Decision issue template', + 4000: 'task: label-consolidation - Six PR templates [TEMPLATE-UPDATE-REQUEST]', + 449: 'epic: label-governance - Stabilise canonical labels', +}; + +describe('check-locked-file-approval', () => { + test.each([ + '.github/labels.yml', + '.github/issue-types.yml', + '.github/ISSUE_TEMPLATE/06-decision.md', + '.github/PULL_REQUEST_TEMPLATE/pr_aiops.md', + '.github/PULL_REQUEST_TEMPLATE/config.yml', + '.github/branch-types.yml', + '.github/branch-labels.yml', + ])('treats %s as locked', (file) => { + expect(isLocked(file)).toBe(true); + }); + + test.each([ + '.github/labeler.yml', + '.github/ISSUE_TEMPLATE/config.yml', + '.github/workflows/labeling-unified.yml', + 'docs/LABEL_STRATEGY.md', + 'labels.yml', + ])('does not treat %s as locked', (file) => { + expect(isLocked(file)).toBe(false); + }); + + describe('issues linked from a body rendered by the GitHub Markdown API', () => { + // Real output of POST /markdown (mode gfm, context lightspeedwp/.github), + // saved so the tests need no network. Nothing in the guard parses Markdown. + const fixtures = require('./fixtures/rendered-pr-bodies.json').bodies; + const linked = (name) => issuesFromRenderedHtml(fixtures[name].html); + + test.each([ + ['plain', [3556, 449]], + ['fullUrl', [3556]], + ['markdownLink', [3556]], + ['pullRequestRef', [3734]], + ['htmlAnchor', [3556]], + ['paragraphContinuation', [3556]], + ['nestedList', [3556]], + ])('finds the links GitHub renders in %s', (name, expected) => { + expect(linked(name)).toEqual(expected); + }); + + test.each([ + 'fenceFour', + 'fenceThree', + 'fenceTilde', + 'fenceUnclosed', + 'indented', + 'indentedTab', + 'indentedAfterHeading', + 'indentedInList', + 'inlineSingle', + 'inlineDouble', + 'htmlComment', + 'otherRepository', + 'noReferences', + ])('finds no link to #3556 in %s, because GitHub renders none', (name) => { + expect(linked(name)).not.toContain(3556); + }); + + test('a reference outside a four-backtick fence is still found', () => { + expect(linked('fenceFour')).toEqual([449]); + }); + + test('evaluate does not accept an approval borrowed through code', () => { + for (const name of ['fenceFour', 'indented', 'inlineDouble', 'htmlComment']) { + const result = evaluate({ + changedFiles: ['.github/labels.yml'], + issues: linked(name), + register, + titlesByIssue, + }); + expect(result.ok).toBe(false); + expect(result.approvedBy).toEqual({}); + } + }); + + test('evaluate accepts a link GitHub renders', () => { + const result = evaluate({ + changedFiles: ['.github/issue-types.yml'], + issues: linked('plain'), + register, + titlesByIssue, + }); + expect(result.ok).toBe(true); + }); + + test('ignores links into other repositories and tolerates empty or missing HTML', () => { + const html = 'x'; + expect(issuesFromRenderedHtml(html)).toEqual([]); + expect(issuesFromRenderedHtml('')).toEqual([]); + expect(issuesFromRenderedHtml(undefined)).toEqual([]); + }); + + test('ignores text that only looks like a link, such as escaped HTML in code', () => { + const html = + '
<a href="https://github.com/lightspeedwp/.github/issues/3556">x</a>
'; + expect(issuesFromRenderedHtml(html)).toEqual([]); + }); + + test('keeps order, drops duplicates and accepts a link with a fragment', () => { + const html = + 'a' + + 'b' + + 'c'; + expect(issuesFromRenderedHtml(html)).toEqual([10, 7]); + }); + + describe('linkedIssues', () => { + const clientReturning = (data) => ({ + calls: [], + rest: { + markdown: { + render(args) { + this.calls.push(args); + return Promise.resolve({ data }); + }, + }, + }, + }); + + test('renders the body in gfm mode with the repository as context', async () => { + const github = clientReturning(fixtures.plain.html); + github.rest.markdown.calls = github.calls; + await expect(linkedIssues(github, fixtures.plain.body)).resolves.toEqual([3556, 449]); + expect(github.calls).toEqual([ + { text: fixtures.plain.body, mode: 'gfm', context: 'lightspeedwp/.github' }, + ]); + }); + + test('makes no request for an empty body', async () => { + const github = clientReturning(''); + github.rest.markdown.calls = github.calls; + await expect(linkedIssues(github, ' ')).resolves.toEqual([]); + expect(github.calls).toEqual([]); + }); + + test('lets a render failure through, so the workflow fails closed', async () => { + const github = { + rest: { markdown: { render: () => Promise.reject(new Error('HTTP 502')) } }, + }; + await expect(linkedIssues(github, 'Closes #3556')).rejects.toThrow('HTTP 502'); + }); + }); + }); + + test('reads approved requests from the register and tolerates a missing one', () => { + expect([...approvedInRegister(register)]).toEqual([3556]); + expect(approvedInRegister(null).size).toBe(0); + }); + + test('accepts only the approver’s comment that starts with "Approved"', () => { + expect( + isApprovalComment({ user: { login: APPROVER }, body: 'Approved 2026-10-02: all of it' }) + ).toBe(true); + expect(isApprovalComment({ user: { login: 'someone-else' }, body: 'Approved' })).toBe(false); + expect(isApprovalComment({ user: { login: APPROVER }, body: 'Not approved yet' })).toBe(false); + }); + + test('passes when no locked file changes', () => { + const result = evaluate({ changedFiles: ['docs/README.md'], issues: [], register }); + expect(result.ok).toBe(true); + expect(result.locked).toEqual([]); + }); + + test('fails a locked-file change with no linked approved request', () => { + const result = evaluate({ + changedFiles: ['.github/labels.yml', 'docs/LABEL_STRATEGY.md'], + issues: [3557], + register, + titlesByIssue, + }); + expect(result.ok).toBe(false); + expect(result.locked).toEqual(['.github/labels.yml']); + expect(result.message).toMatch(/no linked change request is approved/); + }); + + test('names a failed issue lookup in the failure, still failing closed', () => { + const result = evaluate({ + changedFiles: ['.github/labels.yml'], + issues: [4000], + register, + // #4000 could not be read, so it has no title and cannot count as approved. + titlesByIssue: {}, + lookupFailures: [{ issue: 4000, message: 'HTTP 502' }], + }); + expect(result.ok).toBe(false); + expect(result.message).toMatch(/no linked change request is approved/); + expect(result.message).toMatch(/Could not read #4000 \(HTTP 502\)/); + expect(result.message).toMatch(/re-run the check/); + }); + + test('does not mention lookup failures when the request is approved anyway', () => { + const result = evaluate({ + changedFiles: ['.github/issue-types.yml'], + issues: [3556, 4000], + register, + titlesByIssue, + lookupFailures: [{ issue: 4000, message: 'HTTP 502' }], + }); + expect(result.ok).toBe(true); + expect(result.message).not.toMatch(/Could not read/); + }); + + test('says nothing extra for an ordinary rejection with no lookup failure', () => { + const result = evaluate({ + changedFiles: ['.github/labels.yml'], + issues: [3557], + register, + titlesByIssue, + }); + expect(result.message).not.toMatch(/Could not read/); + }); + + test('passes a locked-file change linked to a request approved in the register', () => { + const result = evaluate({ + changedFiles: ['.github/issue-types.yml'], + issues: [3556], + register, + titlesByIssue, + }); + expect(result.ok).toBe(true); + expect(result.approvedBy).toEqual({ 3556: 'change-request register' }); + }); + + test('passes a locked-file change linked to an issue with the approver’s approval comment', () => { + const result = evaluate({ + changedFiles: ['.github/PULL_REQUEST_TEMPLATE/pr_docs.md'], + issues: [4000], + register, + titlesByIssue, + commentsByIssue: { 4000: [{ user: { login: APPROVER }, body: 'Approved, ship it' }] }, + }); + expect(result.ok).toBe(true); + expect(result.approvedBy).toEqual({ 4000: `approval comment by @${APPROVER}` }); + }); + + test('does not accept an approval comment from anyone else', () => { + const result = evaluate({ + changedFiles: ['.github/branch-labels.yml'], + issues: [4000], + register, + titlesByIssue, + commentsByIssue: { 4000: [{ user: { login: 'teammate' }, body: 'Approved' }] }, + }); + expect(result.ok).toBe(false); + }); + + test.each([ + ['[LABEL-UPDATE-REQUEST] Import five labels', true], + ['[ISSUE-TYPE-UPDATE-REQUEST] Decision type', true], + ['task: six PR templates [TEMPLATE-UPDATE-REQUEST]', true], + ['epic: label-governance - Stabilise canonical labels', false], + ['LABEL-UPDATE-REQUEST without brackets', false], + [undefined, false], + ])('isChangeRequestTitle(%p) is %p', (title, expected) => { + expect(isChangeRequestTitle(title)).toBe(expected); + }); + + test('does not accept the approver’s comment on an issue that is not a change request', () => { + const result = evaluate({ + changedFiles: ['.github/labels.yml'], + issues: [449], + register, + titlesByIssue, + commentsByIssue: { 449: [{ user: { login: APPROVER }, body: 'Approved' }] }, + }); + expect(result.ok).toBe(false); + expect(result.message).toMatch(/without one of those tags/); + }); + + test('does not accept a register approval when the issue title is unknown', () => { + const result = evaluate({ + changedFiles: ['.github/issue-types.yml'], + issues: [3556], + register, + }); + expect(result.ok).toBe(false); + }); +}); diff --git a/scripts/validation/__tests__/fixtures/rendered-pr-bodies.json b/scripts/validation/__tests__/fixtures/rendered-pr-bodies.json new file mode 100644 index 0000000000..48e88198fd --- /dev/null +++ b/scripts/validation/__tests__/fixtures/rendered-pr-bodies.json @@ -0,0 +1,86 @@ +{ + "renderedAt": "2026-10-05", + "repository": "lightspeedwp/.github", + "bodies": { + "plain": { + "body": "Closes #3556 and Relates to #449", + "html": "

Closes #3556 and Relates to #449

" + }, + "fullUrl": { + "body": "See https://github.com/lightspeedwp/.github/issues/3556 for the request", + "html": "

See #3556 for the request

" + }, + "markdownLink": { + "body": "[request](https://github.com/lightspeedwp/.github/issues/3556)", + "html": "

request

" + }, + "pullRequestRef": { + "body": "Stacked on #3734", + "html": "

Stacked on #3734

" + }, + "fenceFour": { + "body": "Intro\n````\nCloses #3556\n````\n\nCloses #449\n", + "html": "

Intro

\n
Closes #3556\n
\n

Closes #449

" + }, + "fenceThree": { + "body": "Intro\n```\nCloses #3556\n```\n", + "html": "

Intro

\n
Closes #3556\n
" + }, + "fenceTilde": { + "body": "Intro\n~~~\nCloses #3556\n~~~\n", + "html": "

Intro

\n
Closes #3556\n
" + }, + "fenceUnclosed": { + "body": "Intro\n```\nCloses #3556\n", + "html": "

Intro

\n
Closes #3556\n
" + }, + "indented": { + "body": "Intro\n\n Closes #3556\n", + "html": "

Intro

\n
Closes #3556\n
" + }, + "indentedTab": { + "body": "Intro\n\n\tCloses #3556\n", + "html": "

Intro

\n
Closes #3556\n
" + }, + "indentedAfterHeading": { + "body": "# Notes\n Closes #3556\n", + "html": "

Notes

\n
Closes #3556\n
" + }, + "indentedInList": { + "body": "- item\n\n Closes #3556\n", + "html": "
    \n
  • \n

    item

    \n
    Closes #3556\n
    \n
    • \n
" + }, + "paragraphContinuation": { + "body": "Intro text\n Closes #3556\n", + "html": "

Intro text
\nCloses #3556

" + }, + "nestedList": { + "body": "- parent\n - Closes #3556\n", + "html": "
    \n
  • parent\n\n
    • \n
" + }, + "inlineSingle": { + "body": "x `Closes #3556` y", + "html": "

x Closes #3556 y

" + }, + "inlineDouble": { + "body": "x ``Closes #3556`` y", + "html": "

x Closes #3556 y

" + }, + "htmlComment": { + "body": "\nDone", + "html": "\n

Done

" + }, + "htmlAnchor": { + "body": "request", + "html": "

request

" + }, + "otherRepository": { + "body": "other/repo#12 and https://github.com/other/repo/issues/9", + "html": "

other/repo#12 and https://github.com/other/repo/issues/9

" + }, + "noReferences": { + "body": "Just a description with no links", + "html": "

Just a description with no links

" + } + } +} diff --git a/scripts/validation/check-locked-file-approval.cjs b/scripts/validation/check-locked-file-approval.cjs new file mode 100644 index 0000000000..77d2fe4b37 --- /dev/null +++ b/scripts/validation/check-locked-file-approval.cjs @@ -0,0 +1,205 @@ +#!/usr/bin/env node + +/** + * Locked-file approval guard (spec 008, task T080; constitution Principle II). + * + * A pull request that changes a LOCKED file may merge only after its change + * request is approved. The PR body must link the change-request issue, and + * that issue must either be recorded as `approved` in the spec 008 + * change-request register or carry an approval comment by the approver. Only + * issues whose title carries a change-request tag count, so an approval given + * on any other issue (an epic, for example) cannot unlock a locked file. + * + * The workflow `.github/workflows/locked-files-guard.yml` runs this module + * from the base branch, and reads the register from the base branch too, so + * a pull request cannot approve itself by editing either file. + * + * @module scripts/validation/check-locked-file-approval + */ + +'use strict'; + +/** Constitution v1.4.0 LOCKED files. */ +const LOCKED_PATTERNS = [ + /^\.github\/labels\.yml$/, + /^\.github\/issue-types\.yml$/, + /^\.github\/ISSUE_TEMPLATE\/[^/]+\.md$/, + /^\.github\/PULL_REQUEST_TEMPLATE\/[^/]+\.md$/, + /^\.github\/PULL_REQUEST_TEMPLATE\/config\.yml$/, + /^\.github\/branch-types\.yml$/, + /^\.github\/branch-labels\.yml$/, +]; + +/** The GitHub login whose approval counts (constitution: Approval Authority). */ +const APPROVER = 'ashleyshaw'; + +/** Title tags that mark a change-request issue (constitution Principle II). */ +const CHANGE_REQUEST_TAG = /\[(?:LABEL|ISSUE-TYPE|TEMPLATE)-UPDATE-REQUEST\]/; + +/** Path of the spec 008 change-request register. */ +const REGISTER_PATH = '.github/reports/audits/2026-09-14-label-audit/evidence/change-requests.json'; + +/** + * @param {string} file Repository-relative path. + * @returns {boolean} True when the file is LOCKED. + */ +function isLocked(file) { + return LOCKED_PATTERNS.some((pattern) => pattern.test(file)); +} + +/** + * Issue and pull request numbers a rendered pull request body links to in this + * repository. The input is the HTML GitHub's own Markdown renderer produced for + * the body, so exactly what GitHub does not render as a link (fenced and + * indented code, code spans, HTML comments, references to other repositories, + * references to issues that do not exist) is not found here. Nothing in this + * module parses Markdown. + * @param {string} html Output of the GitHub Markdown render API (gfm mode). + * @param {string} [repo] `owner/name`, to ignore links into other repositories. + * @returns {number[]} Unique numbers, in order of appearance. + */ +function issuesFromRenderedHtml(html, repo = 'lightspeedwp/.github') { + const escaped = repo.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + const link = new RegExp( + `]*?\\bhref="https://github\\.com/${escaped}/(?:issues|pull)/(\\d+)(?:[/?#][^"]*)?"`, + 'gi' + ); + const numbers = []; + for (const match of String(html || '').matchAll(link)) { + numbers.push(Number(match[1])); + } + return [...new Set(numbers)]; +} + +/** + * Asks GitHub which issues a pull request body links to, by rendering it. + * A render failure throws, so the caller fails closed instead of guessing. + * @param {{ rest: { markdown: { render: Function } } }} github Octokit client (github-script). + * @param {string} body Pull request body. + * @param {string} [repo] `owner/name`, used as the rendering context. + * @returns {Promise} Unique issue numbers, in order of appearance. + */ +async function linkedIssues(github, body, repo = 'lightspeedwp/.github') { + if (!String(body || '').trim()) { + return []; + } + const { data } = await github.rest.markdown.render({ + text: String(body), + mode: 'gfm', + context: repo, + }); + return issuesFromRenderedHtml(data, repo); +} + +/** + * True when an issue title marks a change request. + * @param {string} title Issue title. + * @returns {boolean} + */ +function isChangeRequestTitle(title) { + return CHANGE_REQUEST_TAG.test(String(title || '')); +} + +/** + * Issue numbers recorded as approved in the change-request register. + * @param {object|null} register Parsed register (`{ requests: [...] }`). + * @returns {Set} Approved issue numbers. + */ +function approvedInRegister(register) { + const requests = (register && Array.isArray(register.requests) && register.requests) || []; + return new Set( + requests + .filter((request) => request && request.status === 'approved') + .map((request) => Number(request.issue_number)) + .filter(Number.isInteger) + ); +} + +/** + * True when a comment is the approver's approval. + * @param {{user?: {login?: string}, body?: string}} comment Issue comment. + * @returns {boolean} + */ +function isApprovalComment(comment) { + return ( + Boolean(comment) && + comment.user && + comment.user.login === APPROVER && + /^\s*approved\b/i.test(String(comment.body || '')) + ); +} + +/** + * Decide whether a pull request may merge. + * @param {object} input + * @param {string[]} input.changedFiles Paths the pull request changes. + * @param {number[]} input.issues Issues the pull request body links to, from `linkedIssues`. + * @param {object|null} input.register Change-request register from the base branch. + * @param {Record>} [input.commentsByIssue] Comments on each referenced issue. + * @param {Record} [input.titlesByIssue] Title of each referenced issue; an issue + * with no known title is not treated as a change request. + * @returns {{locked: string[], issues: number[], approvedBy: Record, ok: boolean, message: string}} + */ +function evaluate({ + changedFiles, + issues = [], + register, + commentsByIssue = {}, + titlesByIssue = {}, + lookupFailures = [], +}) { + const locked = changedFiles.filter(isLocked); + if (locked.length === 0) { + return { locked, issues, approvedBy: {}, ok: true, message: 'No locked file changed.' }; + } + + const approved = approvedInRegister(register); + const approvedBy = {}; + for (const issue of issues) { + if (!isChangeRequestTitle(titlesByIssue[issue])) { + continue; + } + if (approved.has(issue)) { + approvedBy[issue] = 'change-request register'; + } else if ((commentsByIssue[issue] || []).some(isApprovalComment)) { + approvedBy[issue] = `approval comment by @${APPROVER}`; + } + } + + const ok = Object.keys(approvedBy).length > 0; + const list = locked.map((file) => `\`${file}\``).join(', '); + const message = ok + ? `Locked files changed (${list}); approved through ${Object.entries(approvedBy) + .map(([issue, how]) => `#${issue} (${how})`) + .join(', ')}.` + : `Locked files changed (${list}), but no linked change request is approved. ` + + `Link the [LABEL-UPDATE-REQUEST], [ISSUE-TYPE-UPDATE-REQUEST] or [TEMPLATE-UPDATE-REQUEST] issue ` + + `in the PR body, and get it approved by @${APPROVER} (constitution Principle II). ` + + `Approvals on issues without one of those tags in the title do not count.`; + // A failed lookup leaves that issue without a title, so it cannot be approved + // and the check still fails closed. Name it, so the author can tell a + // transient API error from a missing approval and re-run the check. + const retry = + !ok && lookupFailures.length > 0 + ? ` Could not read ${lookupFailures + .map(({ issue, message: reason }) => `#${issue} (${reason})`) + .join( + ', ' + )}; this may be a transient GitHub API error, so re-run the check before changing the approval.` + : ''; + return { locked, issues, approvedBy, ok, message: message + retry }; +} + +module.exports = { + APPROVER, + CHANGE_REQUEST_TAG, + LOCKED_PATTERNS, + REGISTER_PATH, + approvedInRegister, + evaluate, + isApprovalComment, + isChangeRequestTitle, + isLocked, + issuesFromRenderedHtml, + linkedIssues, +};