chore: fix drifted references and workflow schema gap#29
Merged
Conversation
Repo hygiene sweep. Structure and factual-accuracy fixes, plus one deliberately-accepted coverage change. ci.yml: the workflow schema-validation step hand-enumerated three files. do-not-merge.yml and pr-issue-linkage.yml were added later (#25) without updating the list, so 2 of 5 workflows shipped unvalidated. Replaced with a glob, which the check-jsonschema action documents and supports. This broadens what CI validates from 3 files to 5 — all five pass the schema today, so the gate is green now and the gap cannot silently reopen. PULL_REQUEST_TEMPLATE.md: claimed two forms were "the only two top-line forms the pr-issue-linkage gate accepts". Both counts were false — the gate's keyword match is unanchored, and it accepts all nine GitHub closing-keyword spellings plus two no-issue markers. Rewritten to point at the gate's own failure output; restating its rules is what let the description drift. pr-issue-linkage.yml: header comment named only one of the two accepted no-issue markers. Now defers to the reusable for the exact forms. profile/README.md: listed 2 of 5 public repositories. Added standards and claude-code-plugins, both already described in the page's own prose. .gitignore: comment attributed bin/ and obj/ to "the dotnet fixtures"; this repo contains no .NET files. The issue-forms validation list stays enumerated on purpose — config.yml shares that directory and needs a different schema, so a glob there would be a bug. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UmDSbhvfsnpL1DuTMjjJtP
This was referenced Jul 20, 2026
Closed
Closed
Closed
Contributor
Author
|
Follow-ups from this sweep, all routed to the repo that owns the fix — none are branch-owned, so none block this PR: ci-workflows
github-iac
standards
|
This was referenced Jul 20, 2026
Merged
kyle-sexton
added a commit
that referenced
this pull request
Jul 20, 2026
) Closes melodic-software/standards#213 ## Summary The `.gitignore` header called this a *"Language/framework-agnostic base"* that *"language overlays (.NET, Python, …) extend with their own ignores when adopted"* — framing that implies a shared baseline distributed from `standards`. **No such distribution exists.** `.gitignore` appears nowhere in `standards/distribution/sync-manifest.yml`, for any component or any target. **Nor could it, in this shape.** Git has no include mechanism, so a managed base plus a local overlay cannot coexist in one `.gitignore`. That is exactly the partial ownership the distribution README rules out: *"A component that needs a different destination or partial ownership is the wrong component boundary and must be split first."* So the file cannot become a managed component as designed, and the header should stop implying it is one. **It was never a real baseline in practice.** Of the 10 org repos carrying a `.gitignore`, only this one and `dotfiles` used the "agnostic base" framing — and those two had already diverged from each other (26 vs 33 lines). The other eight are entirely unique, ranging 7 to 821 lines. ## Test plan - [x] Header comment only — **no ignore patterns changed**, so tracking behavior is byte-for-byte identical. - [x] `git status` clean against an unchanged working tree before and after; no files newly ignored or un-ignored. - [x] Verified `.gitignore` absent from `sync-manifest.yml` (`grep -c gitignore` → 0). - [x] Verified the header framing appears in only 2 of 10 org repos, and that those 2 already diverge. ## Related - The companion header in `melodic-software/dotfiles` is worse — it explicitly attributes the base to `(melodic-software/standards)`, naming a distribution source that does not distribute this file. Fixed in a separate PR against that repo. - The Node, Lefthook, and .NET sections are deliberately **kept**. They guard tooling that can run against a local checkout regardless of what source this repo tracks. Whether to prune them is a separate question, not decided here. - Surfaced by the repo hygiene sweep in #29. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
to melodic-software/ci-workflows
that referenced
this pull request
Jul 20, 2026
…165) ## Summary `.github/actions/check-jsonschema` documents `files` as accepting shell globs and deliberately leaves `$FILES` unquoted so they expand — but never set `nullglob`, so a pattern matching nothing was passed through literally and crashed the tool with `OSError`. That makes the documented glob support unsafe to use defensively (the `*.yml` + `*.yaml` case from melodic-software/.github#29). Both halves of the fix land together, because `nullglob` **alone** would convert a loud crash into `check-jsonschema` being invoked with zero files — which exits 0 and becomes a **false-green CI lane**, strictly worse than the bug: 1. `shopt -s nullglob`, scoped to the expansion only (`shopt -u nullglob` immediately after), capturing into a `targets` array. 2. A guard on the **total** expansion: if `targets` is empty the step fails with its own message. The call site now passes `"${targets[@]}"` (already expanded) rather than re-expanding `$FILES`, which also clears the pre-existing SC2086 on that line. The `files` input description gained one sentence stating the new contract. ### Two distinct failure paths | Input | Message | | --- | --- | | `files` empty / whitespace-only (pre-existing guard, unchanged) | `::error::files is required.` | | `files` non-empty but expands to nothing (new) | `::error::files expanded to no existing paths: <patterns>` | Behavior deliberately **not** changed: a literal filename with no glob metacharacters is untouched by `nullglob`, so a non-existent literal still reaches the tool and errors there exactly as before. ## Verification ### 1. Before/after against the real tool, in this repo The pre-fix block reproduces the issue's crash on the defensive pattern; the fixed block passes: ```text --- REAL: the #162 defensive pattern (.yml + .yaml, repo has no .yaml) [FIXED BLOCK] ok -- validation done EXIT=0 --- REAL: same pattern against the PRE-FIX block (reproduces the bug) File ".../check_jsonschema/cli/param_types.py", line 146, in __init__ if "r" in mode and not stat.S_ISFIFO(os.stat(filename).st_mode): ~~~~~~~^^^^^^^^^^ OSError: [WinError 123] The filename, directory name, or volume label syntax is incorrect: '.github/workflows/*.yaml' EXIT=1 ``` Live regression cases — the three real `ci.yml` call sites, run with the actual `check-jsonschema@0.37.4` against this worktree: ```text --- REAL: literal single file (ci.yml case 1) files=.github/dependabot.yml ok -- validation done EXIT=0 --- REAL: workflows glob (ci.yml case 2) files=.github/workflows/*.yml ok -- validation done EXIT=0 --- REAL: composite actions glob (ci.yml case 3) files=.github/actions/*/action.yml ok -- validation done EXIT=0 ``` ### 2. Matrix over the modified shell logic A throwaway harness extracts the Validate step's `run:` block **verbatim** from `action.yml` (byte-diffed against the file to prove it is not a paraphrase), runs it under the real `set -euo pipefail`, and stubs `uvx` with an arg-counter so the number of file arguments actually handed to the tool is observable. Fixture tree: two `.yml` workflows, zero `.yaml`. ```text --- CASE: literal single file files=[.github/workflows/ci.yml] uvx-stub: invoked with 1 file arg(s): .github/workflows/ci.yml EXIT=0 --- CASE: multiple space-separated literals files=[.github/workflows/ci.yml .github/workflows/release.yml] uvx-stub: invoked with 2 file arg(s): .github/workflows/ci.yml .github/workflows/release.yml EXIT=0 --- CASE: glob matching several files files=[.github/workflows/*.yml] uvx-stub: invoked with 2 file arg(s): .github/workflows/ci.yml .github/workflows/release.yml EXIT=0 --- CASE: glob matching nothing files=[.github/workflows/*.yaml] ::error::files expanded to no existing paths: .github/workflows/*.yaml EXIT=1 --- CASE: mix: matching + non-matching glob files=[.github/workflows/*.yml .github/workflows/*.yaml] uvx-stub: invoked with 2 file arg(s): .github/workflows/ci.yml .github/workflows/release.yml EXIT=0 --- CASE: all-empty expansion (2 dud globs) files=[.github/workflows/*.yaml *.toml] ::error::files expanded to no existing paths: .github/workflows/*.yaml *.toml EXIT=1 --- CASE: empty input (pre-existing guard) files=[] ::error::files is required. EXIT=1 --- CASE: whitespace-only input files=[ ] ::error::files is required. EXIT=1 --- CASE: literal missing file (unchanged) files=[.github/workflows/nope.yml] uvx-stub: invoked with 1 file arg(s): .github/workflows/nope.yml EXIT=0 ``` ### 3. The false-green is demonstrated, not asserted Same harness, with **only** the empty-expansion guard stripped out (`nullglob` retained) — i.e. what half 1 without half 2 would ship: ```text === FALSE-GREEN CONTROL: nullglob WITHOUT the empty guard === uvx-stub: invoked with 0 file arg(s): <none> EXIT=0 <-- exit 0 with 0 files = the false green half-2 prevents ``` ### 4. Static analysis ShellCheck 0.11.0 (the version this repo pins) against the extracted block, using the repo's `.shellcheckrc`: - **Before:** `SC2086` (info) on the unquoted `$FILES` at the call site, plus two `SC2154` warnings. - **After:** no `SC2086`, no `SC2206`. Only the same two `SC2154` warnings (`NO_CACHE`, `VERSION`), which are artifacts of extracting the block out of its `env:` block — present identically before the change. The `# shellcheck disable=SC2206` directive was proven load-bearing by stripping it and re-linting: ```text In - line 23: targets=($FILES) ^----^ SC2206 (warning): Quote to prevent word splitting/globbing, or split robustly with mapfile or read -a. ``` Also clean: `editorconfig-checker` (exit 0), `typos` (exit 0), and this repo's own `check-jsonschema` `vendor.github-actions` validation over `.github/actions/*/action.yml` including the edited file (exit 0). ## Blast radius The composite is called by every repo in the constellation. Existing callers pass plain space-separated literal file lists **and** globs; both paths are covered above and behave identically to before. The only new failure mode is one that previously either crashed with `OSError` or (after half 1 alone) would have silently passed. ## Not verified here `actionlint` does not lint composite `action.yml` files — passing the file explicitly makes it parse it as a workflow and emit `"jobs" section is missing` / `"on" section is missing`. This composite's `run:` block therefore has never been covered by the repo's `actionlint` job; the standalone ShellCheck run above is the equivalent check. Not changed here — out of scope for #162. Closes #162 ## Related - melodic-software/.github#29 — the consumer hygiene sweep that surfaced this; it wants `files: .github/workflows/*.yml .github/workflows/*.yaml`, which this makes safe. - `.github/workflows/ci.yml` — the three in-repo call sites (one literal, two globs) that serve as the live regression cases above. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01UmDSbhvfsnpL1DuTMjjJtP Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
kyle-sexton
added a commit
to melodic-software/standards
that referenced
this pull request
Jul 20, 2026
Closes #212 ## Summary The private-repo exclusion list named **4 of the org's 9** private repositories. Missing: `itinerary-planner`, `knowledge-corpus`, `medley-archive`, `melodic-main-archive`, `songwriting`. A link to any of the five 404s in the online link lane for precisely the reason the list exists to prevent — the calling repo's `GITHUB_TOKEN` cannot read another private repo. Latent rather than breaking today, since no tracked file currently links to them. The block's own comment states the rule it drifted from: *"Keep in sync with actual visibility: add newly created private repos here."* `medley-archive` is ordered before `medley` in the alternation so the longer name cannot be shadowed by a prefix match. ## Test plan Verified empirically with `lychee --dump` over a fixture covering all 9 private repos plus `.git` and `github.com/ghraw` variants, comparing old config against new. - [x] **Old config leaked 8 private URLs:** ``` itinerary-planner, knowledge-corpus, medley-archive, melodic-main-archive, songwriting, medley-archive.git, github.com/ghraw/.../medley-archive/..., raw.../songwriting/... ``` - [x] **New config excludes all 9**, including the `.git` and `raw.` variants. - [x] **The 3 public repos still survive exclusion and stay checked** — `standards`, `ci-workflows`, `.github` — confirming the change does not over-exclude. - [x] `medley` vs `medley-archive` disambiguation confirmed by the fixture, not by reasoning about alternation order. - [x] lefthook pre-commit: typos, gitleaks, editorconfig all pass. ## Related - Follows the same maintenance pattern as #204 (`fix(lychee): drop claude-code-plugins and standards from the private-repo exclusions`), which moved entries the other direction when repos went public. - Surfaced by a repo hygiene sweep of `melodic-software/.github` (melodic-software/.github#29). Not fixable there — `lychee` is an upstream-managed component, so the change belongs here and reaches consumers through the reconciliation sync. - Worth considering separately: this list is hand-maintained against a queryable fact (`gh repo list --visibility private`), so it will drift again. A generation or CI-check step would remove the class of problem rather than this instance. Not in scope here.
kyle-sexton
added a commit
to melodic-software/ci-workflows
that referenced
this pull request
Jul 20, 2026
…164) ## The issue's framing is stale — read this first #163 was filed **leaning reject**, on the observation that the aggregation block appeared in only two repos (this one and `melodic-software/.github`). That count was wrong. The block is byte-identical in **seven**: `.github`, `ci-runner`, `ci-workflows`, `dotfiles`, `github-iac`, `provisioning`, `standards`. At seven call sites the "weak case" the issue argues against no longer holds, so the decision is to **adopt**. The issue body is not edited; this PR is the documented answer it asked for. The issue's two reservations still stand as facts, and neither is fatal: - **The `needs` list stays duplicated.** GitHub has no "all other jobs" selector. The composite owns the *semantics*, not the lane inventory — the `needs` array and the matching results string remain caller-owned. That is a real limit on how much this removes, not a reason to leave the semantics forked seven ways. - **It adds a dependency to the merge-gating job.** True, and it is why this PR adopts the composite **only here**. Consumers pin by SHA per the repo's pin-everything rule; a same-repo caller references it by local path, so the gate resolves from the checkout it already has rather than over the network. ## What this adds `.github/actions/ci-status` — a composite taking the caller-built results string as one input. Semantics preserved exactly: `success` and `skipped` pass, anything else fails with the original message naming the offending result. House style followed from `.github/actions/check-jsonschema`: `shell: bash`, `set -euo pipefail`, and the input passed via `env:` rather than interpolated into `run:`. The `env:` indirection matters specifically here — the string is built from `needs.*.result` expressions, and the expression-into-`run:` shape is the template-injection pattern this repo avoids everywhere else. ## Deliberate behavior change: empty input now fails closed Verified baseline of the inline block: ``` OLD input=[] -> exit=0 out=All lanes passed or were skipped. ``` `read -ra` on an empty string yields an empty array, the loop body never runs, and the gate reports green. On the one job every merge depends on, a results string that came out empty means a miswired `needs` list or a typo'd expression — and the current block reports that as a pass. The composite rejects it instead (`::error::results is required.`, matching the sibling composite's required-input idiom). This is the only intentional divergence from the inline block; every non-empty input behaves identically. ## Self-adoption `ci-workflows`' own `ci.yml` `ci-status` job (27 lanes) now calls `./.github/actions/ci-status`, matching the local-composite reference pattern the file already uses throughout. The job gains an `actions/checkout` step — a local `./` action needs the repo in the workspace, and this job previously had no checkout. Same pinned `actions/checkout@9c091bb…` + `persist-credentials: false` as every other lane. The `needs` list and the results expression are unchanged, character for character. ## Testing Local matrix against the exact `run:` body extracted from the shipped `action.yml` (not a retyped copy): ``` all-success input=[success success success] exit=0 out=All lanes passed or were skipped. success+skipped input=[success skipped success skipped] exit=0 out=All lanes passed or were skipped. one failure input=[success skipped failure success] exit=1 out=A lane did not pass (result: failure). one cancelled input=[success cancelled skipped] exit=1 out=A lane did not pass (result: cancelled). empty string input=[] exit=1 out=::error::results is required. whitespace only input=[ ] exit=1 out=::error::results is required. ``` Also run locally, all clean: - `actionlint .github/workflows/ci.yml` — clean - `shellcheck --shell=bash` on the extracted composite body — clean - `check-jsonschema --builtin-schema vendor.github-actions` on the new `action.yml` — `ok -- validation done` - `markdownlint-cli2 README.md` — 0 errors ### The first CI run proved the failure path for real The initial run's `ci-status` **failed, correctly**. `selector-contract` failed, and the composite caught it in the live 27-lane environment: ``` RESULTS: success success success success success success success success success success success success success success success failure success success success success success success success success success success success A lane did not pass (result: failure). ``` That is the failure path exercised against 27 real `needs.*.result` values — better evidence than a green run alone would have been. The `selector-contract` failure was caused by this PR and is now fixed. `.github/scripts/go-quality.test.cjs` asserts that the go-quality lane's result actually reaches the gate, and pinned the literal `RESULTS:` env key. Aggregation moved to the composite's `results:` input at the same indent, so the assertion was updated to match the line it always meant. The test's intent is unchanged; `node --test .github/scripts/*.test.cjs` → **199 pass, 0 fail**. ## Scope Only what #163 requires plus this repo's self-adoption. The other six repos are separate follow-up PRs — deliberately not touched, so the composite proves itself on one caller before it gates six more. Closes #163 ## Related - melodic-software/.github#29 — the reinvented-wheel audit that surfaced this - `.github/actions/check-jsonschema` — the house-style precedent for inputs, `env:` passing, and the required-input error idiom --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4 tasks
kyle-sexton
added a commit
that referenced
this pull request
Jul 20, 2026
No related issue: rollout of the decision recorded in melodic-software/ci-workflows#163, plus the pin bump that makes melodic-software/ci-workflows#162 live here. ## Summary Two changes to `ci.yml`, both about consuming `ci-workflows` composites. **1. Adopt the `ci-status` composite.** The nine-line lane-aggregation loop was duplicated byte-identically across seven repos. It now lives in `ci-workflows` (melodic-software/ci-workflows#164) and this is one of the six adopters. **2. Bump the four `check-jsonschema` pins** to the same SHA, which carries the `nullglob` fix from melodic-software/ci-workflows#165. ##⚠️ The composite is NOT a like-for-like copy — it fixes two false greens Worth reviewing deliberately rather than as a mechanical extraction. The inline block being replaced had two defects, both found by Codex review on melodic-software/ci-workflows#164: - `read -ra results <<< "$RESULTS"` consumed **only the first line**. A multi-line results string left every lane after the first unchecked — the gate reported success while a lane had failed. - The empty guard trimmed spaces only, so newline-only or tab-only input passed it, produced an empty array, skipped the loop, and reported success. Both were unreachable while the block was fed a single-line `${{ }}` interpolation, and both become reachable once the input is an action input. Reproduced against the old logic: ``` OLD MULTILINE failure 2nd EXIT=0 All lanes passed or were skipped. OLD MULTILINE failure last EXIT=0 All lanes passed or were skipped. OLD newline only EXIT=0 All lanes passed or were skipped. OLD tab only EXIT=0 All lanes passed or were skipped. ``` All four now exit 1. **Empty input also now fails closed** where it previously passed — a deliberate behavior change: a gate with nothing to aggregate is a miswired `needs` list, not a pass. ## Why the pin bump matters here specifically #29 replaced this repo's hand-enumerated workflow list with `.github/workflows/*.yml`. The `nullglob` fix guards a fully-empty expansion so it fails loudly instead of validating nothing — that guard is what keeps the glob safe if a future rename ever leaves it matching zero files. ## Test plan - [x] `actionlint` — clean - [x] `check-jsonschema` `vendor.github-workflows` against all 5 workflows — `ok`, exit 0 - [x] Composite behavior verified across 13 cases before adoption (single-line, multi-line, tab-separated, cancelled, skipped, empty, whitespace-only) - [x] CI on this PR exercises the composite against this repo's real 8-lane `needs` list ## Related - melodic-software/ci-workflows#164 — the composite, and the dogfooding adoption in `ci-workflows` itself - melodic-software/ci-workflows#165 — the `nullglob` fix now pinned here - melodic-software/ci-workflows#167 — composite `run:` blocks are not ShellCheck-covered, which is why both bugs above reached review rather than CI - Other `ci-workflows` composite pins deliberately stay at `1d3762c`; Dependabot owns those bumps, and moving all of them here would pull ten days of unrelated change into a targeted fix. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No related issue: proactive repo hygiene sweep.
Summary
Structure and factual-accuracy fixes across the locally-owned surface. The 7 upstream-managed files (
.editorconfig,.gitattributes,.editorconfig-checker.json,.markdownlint-cli2.jsonc,_typos.toml,lychee.toml,.gitleaks.toml) were classified againststandards/distribution/sync-manifest.ymland left untouched.ci.yml— the workflow schema-validation step hand-enumerated three files.do-not-merge.ymlandpr-issue-linkage.ymlwere added later in ci: add do-not-merge and pr-issue-linkage gate callers #25 without updating the list, so 2 of 5 workflows shipped unvalidated. Replaced with.github/workflows/*.yml.PULL_REQUEST_TEMPLATE.md— claimed two forms were "the only two top-line forms the pr-issue-linkage gate accepts". False on both counts: the gate's keyword match is unanchored, and it accepts all nine GitHub closing-keyword spellings plus two no-issue markers. Now points at the gate's own failure output instead of restating its rules — restating is what let it drift.pr-issue-linkage.yml— header comment named only one of the two accepted no-issue markers. Now defers to the reusable.profile/README.md— listed 2 of 5 public repositories. Addedstandardsandclaude-code-plugins, both already described in the page's own prose..gitignore— comment attributedbin//obj/to "the dotnet fixtures"; this repo contains no .NET files.The
ci.ymlglob broadens what CI validates from 3 files to 5. Zero diff today is not zero behavior change: a schema-invaliddo-not-merge.ymlorpr-issue-linkage.ymlpreviously passed and now fails, and future workflow files auto-enter validation.Verified before applying: the
check-jsonschemacomposite action documents glob support (action.yml:9) and deliberately leaves$FILESunquoted so globs expand (action.yml:70); all five workflows passvendor.github-workflowsat the pinned version today.Known latent gap: the glob matches
.ymlonly. A future.yamlworkflow would be silently unvalidated — same as the old enumeration, so not a regression. The naive fix of adding a second*.yamlglob is wrong: the action sets nonullglob, so an unmatched pattern is passed through literally and crashes the tool. Correct fix routes upstream toci-workflows; filed separately.The issue-forms validation list stays enumerated on purpose —
config.ymlshares that directory and needs a different schema, so a glob there would be a bug.Test plan
actionlint— cleancheck-jsonschemavendor.github-workflowsagainst all 5 workflows —ok, exit 0markdownlint-cli2v0.23.0 against repo config — 0 errorslychee --offline— 21 links, 0 errorstypos,editorconfig-checker,gitleaks— cleangit diff --check— cleanRelated
do-not-mergeandpr-issue-linkagewere verified to have no native GitHub equivalent (full ruleset rule-type enum + changelog sweep), so both are correctly hand-rolled rather than reinvented.ci-workflows,github-iac, andstandardsare filed separately; none are branch-owned.