refactor(scripts): move the CI half to scripts/ci, index it with task - #2061
Merged
Merged
Conversation
Contributor
🔍 Rendered manifest diff — this PR vs
|
scripts/ is 55 flat executables mixing CI gates, unit suites, day-2 ops tooling and apply-time config drivers. Reorganise by audience, index the entry points with go-task, and replace ci.yaml's hand-maintained test list with a `# requires:` declaration the runner reads. Measured rather than assumed: 1246 references to scripts/, of which 359 are live and 887 sit in dated plans and specs no validator can see. The archive stays as written. Deploy-time invocation does not follow the directory split -- 11 of 18 ops scripts and 2 CI validators are reached from terramate -- so the PR sequence is ordered by that, not by directory.
The design measured external references and missed the larger one: 42 of 55 scripts resolve paths from their own location, so a git mv breaks them whether or not every external reference is rewritten. 11 hardcode a "/.." to reach the repo root, 25 build paths from HERE/SCRIPT_DIR, and 14 source lines across 9 scripts reach lib/ by a scripts-root-relative path. Six of those nine are deploy-time invoked, so a wrong depth fails during an apply -- the PR-3 failure mode, through a door the reference rewrite does not cover. Records the fix (correct each depth in the commit that moves the file) and the gate that makes it reviewable (test-script-paths.sh, shipped in PR 1 so later phases move underneath it).
…cture Covers the CI half only: scripts/ci/, the taskfile index, the discovering test runner, and the test-script-paths.sh gate. PRs 2 and 3 get their own plans -- each phase produces working software on its own, and planning the moves is far cheaper once the gate exists. Task order is deliberate: the three safety nets are built on the unmoved tree and proved green before any file relocates, so each move has a check in front of it rather than behind it.
Every task body is a single-line call to a script that still runs standalone, so the scripts stay liftable into another repo. The ci:test task points at a runner that Task 3 creates.
Measuring the gate's regexes against the tree before dispatching it found that two lines in test-cloud-secret-store.sh source through nested quoting, so the extraction yields a bare quote and the gate would check it as a filename -- two failures on a correct tree. Adds a suffix guard, and replaces the guessed verification counts with measured ones: 12 roots, 19 sources, 15 subjects.
The first implementation attempt came back blocked, correctly. Both bugs
were in the plan, not in any repo script, and both reproduced in isolation:
A literal ${BASH_SOURCE[0]} inside a ${var//pat/rep} ends the expansion at
its own closing brace, so bash appends the remainder as text instead of
substituting -- every source check was silently comparing garbage. The
patterns now live in variables.
The gate also scanned itself, and its own explanatory comment matched its
own subject detector, reporting a failure that existed only in its docs.
Fixing those surfaced a third: the quote-delimited extraction truncated at
the inner quote of `. "$(dirname "$0")/lib/x.sh"` -- the dominant idiom in
this repo -- so the check could never have examined the lines it exists
for. Check 2 now strips quotes before resolving.
Verification changes with it: both fixture failures must fire, and the
assertion is 0 failed plus a coverage floor of 20 sources rather than an
exact count, because a low count is the tell that the checker stopped
checking.
42 of 55 scripts compute paths from where they sit, so a git mv breaks them whether or not every external reference is rewritten. A wrong /.. depth makes cd succeed at the wrong directory, and six of the nine lib/-sourcing scripts run during a terramate apply. Ships before anything moves, so the restructure relocates files underneath a check that already passes.
test-validate-idp-topology.sh was to declare `kustomize`, on the strength of a grep hit that turns out to be `apiVersion: kustomize.toolkit.fluxcd.io/v1` written into a YAML fixture. validate-idp-topology.sh never invokes the binary. Declaring it would have made the suite skip permanently on any machine without kustomize, silently dropping the ADR-0027 gate -- the exact failure the requires mechanism exists to prevent, caused by the mechanism. The remaining three headers were each confirmed against the source: the tool is actually executed, not merely mentioned.
Replaces a hand-maintained list of 11 with a glob over all 20. Each suite declares what it needs on PATH; the runner skips what is absent and says which tool was missing, so 'did not run' can never read as 'passed'.
… dead Running the suites for the first time proved test-flux-schema.sh asserts against hardcoded bundle filenames the render's naming scheme outgrew. Nothing caught it because nothing ran it -- all three mentions of it in ci.yaml are comments. Checked before ruling: the six chartRef HelmReleases it names ARE in the rendered bundle, so this is not a gap in validate-manifests.sh. Quarantined rather than repaired. Fixing assertions inside a guard is not relocation, and a hasty fix risks making it vacuous. Spec criterion 4 drops to 19 discovered plus 1 quarantined, and the plan now says so.
It asserts against hardcoded bundle filenames the render's naming scheme outgrew. Nothing caught it because nothing ran it: all three mentions in ci.yaml are comments. Not repaired here. Fixing assertions inside a guard is not relocation, and a hasty fix risks making it vacuous -- which is worse than red, because a vacuous guard reports success forever. Also corrected its self-resolved REPO_ROOT: one level up reached the repo root from scripts/, not from the new scripts/ci/tests/quarantine/. Caught by test-script-paths.sh, which scans quarantine too -- a suite need not run to have a checkable self-location.
Depths corrected in the same commit as the move, and test-script-paths.sh proves every self-resolved root still lands on the repo root. Also fixed test-validate-idp-topology.sh's subject reference, which the move broke and which test-script-paths.sh's subject-defaults check (a $HERE/subject.sh idiom, single segment) could not statically re-verify until the reference was written bare rather than braced -- matching the convention every other test-*.sh in this directory already uses. validate-manifests.sh's flux-schema/ references now point at Task 5's destination and do not resolve until that task lands; test-script-paths.sh still reports that one FAIL, as expected.
gen-catalog.sh went from one level deep to two, so its repo-root hop goes from /../.. to /../../.. -- the depth-coupling the gate exists to catch. Also fixed two references the gate cannot see, since neither is a source statement or a HERE/SCRIPT_DIR subject: gen-catalog.sh's vendored-crds extract call, and the quarantined test-flux-schema.sh's own preflight.sh source line, both still pointing at the pre-move path. render-both.sh's base-checkout render (line 34, cwd /tmp/base) also now reads scripts/ci/flux-schema/render-bundle.py. That base checkout predates this move on any PR based off pre-refactor main, so render-diff CI breaks transiently for such PRs; every base after this merges has the new layout.
Sixteen suites reach their subject by a relative path, and thirteen of those subjects do not move until a later phase -- so those paths are temporary and carry a comment saying so. The gate covers all of them. Also fixed three references the gate cannot see: - test-tm-provisioner.sh used $(dirname "$0")/tm-provisioner.sh inline rather than a HERE variable, so the gate's subject-default check (which only matches $HERE/SCRIPT_DIR) never looked at it. Rewritten onto the same HERE idiom every other suite here uses -- both to fix the runtime break and because the inline dirname form false-positived the gate's self-resolved-root check (category 1 pattern-matches any dirname+$0 next to a /.. sequence, root climb or not). It moves to scripts/provision/ with the rest of the provision-phase scripts, so it gets the same temporary-path comment as the other twelve. - validate-alertmanager-templates.sh's FIXTURE_DIR is a Python string literal inside a heredoc, invisible to a shell-oriented gate; it still pointed at scripts/alertmanager-fixtures. Re-verified end to end with task ci:validate (gate 3 still renders and matches golden). - validate-vector-vrl.sh's VRL_DIR is a $SCRIPT_DIR reference to a directory, not a .sh/.py file, so the gate's regex (which requires that suffix) never checked it either. scripts/lib/ and scripts/openbao-snapshot.sh are not part of the sixteen: per the design doc's target layout, lib/ is "unchanged -- already correct" and openbao-snapshot.sh is a symlink that never moves, so neither gets the revisit comment.
Task 7 selected files with a *.md glob, which covers all ten symlinked CLAUDE.md files. `sed -i` replaces a symlink with a regular file even when the pattern matches nothing -- measured -- so the rewrite would have converted every one and silently undone ADR-0038, merged five days ago. It now selects with `git grep -l` on the moved basenames and passes --follow-symlinks. The same glob both over- and under-selected: 180 files chosen to rewrite 56, while clusters/, observability/, infrastructure/, flux/ and every opentofu .tf/.tm.hcl went unselected despite holding live references. Task 9 said to create docs/specs/README.md, which already exists and carries the retired-workflow notice. It now appends.
Five files kept a depth the moves invalidated; none is visible to test-script-paths.sh, since none is a self-resolved root, a source statement, or a $HERE/$SCRIPT_DIR reference ending in .sh/.py -- the gate's three categories, by design. - test-no-secret-argv.sh: scan_dir_for_argv_leaks was rooted two levels too deep, so its four scan roots (scripts/*.sh, lib/, workflows/, opentofu/) resolved to nothing and the credential-leak guard passed over an empty set while still printing "ok". - check-substitution.py and render-bundle.py: REPO_ROOT used parents[2], correct one move ago, wrong by one now. The former made validate-manifests.sh's step [1/6] report "0 Flux Kustomization(s) checked; consistent" -- a false green I had already captured in this report's Task 5/6 evidence and read past. The latter made render-bundle.py's own mise.toml lookup miss silently and made test-render-bundle.py's "real HelmReleases" case find 0. - preflight.sh: same off-by-one on its own REPO_ROOT, same silent mise.toml miss, falling back to a bare PATH lookup for flux/helm/kustomize with no error. - test-check-substitution.py and test-render-bundle.py: loaded their subject relative to their own directory, two levels short of where Task 5 actually put it; both raised FileNotFoundError. Also: added the "subject moves later" comment to the two openbao-snapshot.sh references, on review's adjudication that the symlink's final home is not yet settled despite Task 6's "never moves" reasoning. Verified by running each fixed thing directly, not by re-running the gate: check-substitution.py now reports 68 Kustomizations (was 0), the argv scan's four roots cover 39 files again (was ~0), both flux-schema unit-test files run to completion, task ci:validate still renders 2115 resources in 278 files with all three gates green, and task ci:test stays 0 failed.
…e split -maxdepth 1 on the first scan root was correct while every script lived flat in scripts/; Task 4/5/6 split scripts/ into subdirectories, and the flag silently dropped every moved file (28 scripts) from a credential-leak guard that kept reporting "ok". Same failure class as fix round 1, this time introduced by the moves rather than merely exposed by them. Dropped -maxdepth 1 and made the first root fully recursive; removed the now-redundant separate lib/ root, since it is inside scripts/ and the recursive root already reaches it. The two external roots (.github/workflows, opentofu/) are unchanged. Reworded the self-test fixture's stale "lives under scripts/ too" comment, which assumed a depth this suite no longer sits at and will move from again. Verified with canaries planted in the newly-covered territory (scripts/ci/ and scripts/ci/tests/), not by re-running the gate: both caught and correctly named, both removed, tree confirmed clean afterward. First root now covers 62 files (26 former top level + 33 under ci/ + 3 lib/, folded into one root instead of two) -- more than the pre-fix estimate, not less, since nothing previously covered dropped out.
…ed state The quarantine moved test-flux-schema.sh out of scripts/ci/tests/, so Task 7's basename list is 36 entries, not 37, and must not map the quarantined suite. Task 7 now carries the 11 doc paths verify-doc-paths.sh actually reports, as its acceptance criterion, rather than leaving the implementer to discover them. Task 9 gains a step for commands.md, which still advertises the quarantined suite as working. A bare filename is not a repo path, so no gate flags it. Also corrects the temporary-subject note: 17 lines over 8 subjects, and the openbao-snapshot pair does carry a revisit comment, per my own earlier adjudication. Two prior drafts of that note were wrong.
Anchored on moved basenames rather than the bare 'scripts/' token, so the module-local opentofu/**/cluster/scripts/ directories are untouched. The dated plan and spec archive keeps its original paths. Also fixes scripts/ci/tests/test-no-secret-argv.sh's docstring, which still described the pre-split scan (non-recursive, separate lib/ pass): the scan has been fully recursive since the test suites moved under scripts/ci/tests/.
The 60 lines arguing for which suites to name are replaced by a requires header on the four suites that need one. Job names are unchanged, so the required-check list on main is untouched.
…uite main added test-ci-notify-main-broken.sh at scripts/ root with a hand-added step in the links job. This branch replaces hand-added steps with discovery under scripts/ci/tests/, where run.sh never looked for it. - Move the suite to scripts/ci/tests/ and fix its workflow path depth (../ -> ../../../). - run.sh: exit 77 is a skip. It prints SKIP with the suite's last output line as the reason and counts toward skipped, not passed or failed. - The suite exits 77 without pyyaml instead of 0, closing the known gap its comment recorded. - Drop the links job's notifier step; task ci:test in kubernetes-validation runs it, after that job's pyyaml install. Job names are unchanged. - scripts/AGENTS.md says where it runs now.
ADR-0039 also gives the 2026-07 Dagger decommission a durable home; until now its only trace was three orphaned comments in ci.yaml. The commands reference now marks test-flux-schema.sh as quarantined. No gate would have flagged the stale row: verify-doc-paths.sh skips a bare filename, because it is not a repository path.
c1c86f9 said the Dagger decommission's only trace was three orphaned comments in ci.yaml. That was wrong: ci.yaml:63-73 explains why the last Dagger step left (tflint's anonymous rate limit). What had no durable home was the decision itself: why it was made, and that it covers every job. task check now runs every scripts/ci gate the workflows run, adding doc-claims, idp-topology and doc-paths. CI still calls them one per job, because a required check is a job.
A pure rename, so history follows the file. go-task stops its upward search at the first taskfile it finds, so from under scripts/ the old name shadowed the root taskfile and no ci:* task existed there.
A bare `task` called a task named `--list` and exited 201. From under
scripts/, the included taskfile's commands resolved to scripts/scripts/ci/;
they are now anchored on {{.TASKFILE_DIR}}, so nothing depends on the
directory task was started from.
go-task floated on `task = "3"` while every other tool is pinned exactly;
it is now 3.53.1. ci:vector-vrl joins the index so `task --list` names
every entry point; it stays out of `check` because CI does not run it.
ADR-0039 and the root AGENTS.md now say only what is true: three of seven
ci.yaml jobs run mise-action, the links job runs three gates, 19 of 21
shell suites ran from the old list, and `check` is a hand-kept mirror of
the workflows.
run.sh exits 1 when it finds no suite, test-script-paths.sh when its coverage falls below the floor measured when it was written (20 sources), check-substitution.py when it checked no Kustomization, and test-no-secret-argv.sh when any scan root yields no file. Each of these reported green over an empty input before. run.sh also discovers top-level test-*.py, so "every suite in this directory" is true, prints "no reason given" for a silent exit 77, and its header records exit 77 as a deliberate, narrow exception to the design.
Revisit comments said the subjects move to scripts/provision/. That is wrong for cnpg-promote-seed.sh, which is an operator script, and unsettled for openbao-snapshot.sh, so every one now says only that the path moves with its subject. test-secret-store-lint.sh gains the same note. The quarantined flux-schema suite and the .gitignore comment now name scripts/ci/flux-schema/, and the quarantine README drops a ci.yaml line number that no longer exists and says how to move the suite back out.
The suites moved to the Kubernetes validation job, but jq was still installed in the shellcheck job "for the test suites below". It now installs in the job that runs them, and the shellcheck job installs only shellcheck. The manifest gate runs even when a suite fails, so a red suite no longer hides whether the manifests validate; the job still fails. Job names are unchanged, since they are the required-check contexts.
Smana
force-pushed
the
worktree-scripts-restructure
branch
from
September 21, 2026 21:21
7ec01df to
6c50b14
Compare
1 task
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.
scripts/held 55 flat executables for four audiences that share nothing but a parent directory: the gates CI runs, the test suites, day-2 ops tooling, and config drivers that terramate shells out to during an apply. This PR moves the CI half intoscripts/ci/. It indexes every entry point with go-task, and replacesci.yaml's hand-maintained suite list with a runner that discovers suites. PR 1 of 3.flowchart LR subgraph before["before"] A["ci.yaml · 526 lines<br/>239 comment"] -->|"11 list entries +<br/>2 named steps"| B["scripts/<br/>55 flat files"] end subgraph after["after (PR 1)"] C["ci.yaml · 413 lines<br/>145 comment"] -->|"task ci:test<br/>task ci:validate"| D["taskfile.yaml"] H["a human"] -->|"task check<br/>(any directory)"| D D --> E["scripts/ci/<br/>gates"] D --> T["scripts/ci/tests/<br/>run.sh discovers"] D -.->|"PR 2"| F["ops/"] D -.->|"PR 3"| G["provision/"] end before ==> after classDef new fill:#d4edda,stroke:#28a745 classDef pending fill:#f8f9fa,stroke:#adb5bd,stroke-dasharray:4 class C,D,E,T new class F,G pending📋 Design
2026-09-17-scripts-restructure-design.md2026-09-17-scripts-restructure-pr1-plan.md0039-go-task-as-the-entry-point.md. It records go-task chosen over a bespoke dispatcher, over Make, and over Dagger. It is also the first durable record of the 2026-07 Dagger decommission.This PR delivers PR 1 of the plan:
scripts/ci/, the taskfile, and the gates.What changed
scripts/ci/flux-schema/scripts/ci/tests/run.shtest-*.shandtest-*.py, honours# requires:, and reports exit 77 asSKIP. A skip is always a printed line. Zero suites found is a failure.taskfile.yaml+scripts/tasks.yamltask --listis the index.task checkruns all sixscripts/cigates CI runs. Every task is a one-line script call, so scripts stay liftable. Tasks work from any directory.test-script-paths.shsourcetarget and suite subject still resolves.ci.yamlmainare untouched.mise.tomltask = "3.53.1", an exact pin, so it goes through Renovate's release-age gate like every other tool.Rebased onto #2057/#2058.
mainaddedtest-ci-notify-main-broken.shthis week, together with the hand-writtenci.yamlstep it needed to run. That is exactly the "second act to forget" this PR removes. The suite now sits inscripts/ci/tests/, and its step is deleted becauserun.shdiscovers it. Its missing-pyyaml skip used to exit 0, which the runner would have reported as PASS. It now exits 77 and shows asSKIP.Review carefully: a move breaks scripts from the inside
42 of the 55 scripts compute paths from where they sit. The paths gate shipped before anything moved, and it is necessary, not sufficient. Reviews found breaks it could not see:
check-substitution.pyresolved its repo root one level short. It printed0 Flux Kustomization(s) checkedand exited 0. It is gate 1/6 ofvalidate-manifests.sh.parents[N]test-no-secret-argv.sh, the credential-on-argv guard, scanned 0 files and still printedok.find -maxdepth 1. The move took 28 scripts out of its scan.../.githubdepthsourcegen-catalog.sh,validate-alertmanager-templates.sh, a bare directory referenceEach of these reported green. So the fix wave made every guard on this branch fail when it covers nothing:
run.shon zero suites, the paths gate below a coverage floor,check-substitution.pyat 0 checked, and the argv guard on an empty scan root. The argv scan was set-compared againstmain, not counted: 69 files before ⊂ 74 after (comm -23empty).Rendered output is unchanged. I rendered
origin/mainand this branch in separate worktrees:2115 resources found in 278 files - Valid: 2115, Invalid: 0and 279 bundle files, measured before this branch was rebased onto chore(tooling): remove the Dagger engine and its runner scale set #2073, which removed Dagger's 10 resources;caBundle,tls.*, generated secrets);Already broken, surfaced here
test-flux-schema.shhas been dead for an unknown period. It asserts bundle filenames the render outgrew, and nothing caught it because nothing ran it. It is quarantined, not repaired, underscripts/ci/tests/quarantine/with a README. A hasty fix to a guard risks making it vacuous.validate-manifests.shitself covers the chartRefs it named.sed -iturns a symlink into a regular file even when nothing matches. A*.mdglob would have flattened all tenCLAUDE.mdsymlinks and silently undone ADR-0038. The rewrite selects files withgit grep -l. All 13 repo symlinks are intact.Not done here
scripts/root until PRs 2 and 3.../../subject paths. They cover 7 subjects that move later:cnpg-promote-seed.sh,openbao-config.sh,openbao-snapshot.sh,tm-provisioner.sh,zitadel-idp.sh,zitadel-oidc-clients.shandsecret-store.sh. Each carries a destination-neutral revisit comment, andtest-script-paths.shcovers them.# requires:headers: three suites declare one (python3,openssl,vector). The commit message on7b54499bsays "four", which is wrong; it disappears in the squash.test-vector-vrlskips in CI, becausevectoris not installed there. That was also true before this PR.validate-vector-vrl.shneeds docker and no CI job runs it;task ci:vector-vrlnow at least names it.Expect these on this PR
render-diffcomment will look like everything was added. The base checkout has noscripts/ci/, so the base render fails. The local comparison above is the real diff.openbao-snapshotgets rebuilt on merge. A comment edit incontainer-images/openbao-snapshot/openbao-snapshot.shmatches the image workflow's path filter. Deployments pin<branch>-<sha>tags, so nothing rolls out.main. The three flagged files this branch touches produce identical findings onorigin/main: the same 3, with the same hash. CI's Trivy step only uploads SARIF.Follow-ups (not filed)
run.shself-test covering pass, missing tool, 77, fail, and nested.py.main'sgh-repo-removedcase in the notifier suite accepts any non-zero exit.tooling/base/dagger-engine/,ci-workflows.md:248, anddocs/architecture/ci-pipeline.drawio.taskfile.yamldefaultshould call{{.TASK_EXE}}so a baretaskoutside mise uses the pinned version.validate-manifests.sh:20has a working-directory-relative# shellcheck source=.Evidence
Rebased again on 2026-09-21 onto
mainwith #2072 (GCP drift) and #2073 (Dagger removal); both merged cleanly into the new layout. The render and suite lines below come from this PR's CI run on the rebased head (Kubernetes validation). The rest were re-run locally:Job names are equal to
origin/main's (7/7, including the displayname:). ShellCheck passes with CI's exactfind.Closes #1951.