Skip to content

test: guard declarative coverage of deploy/#114

Merged
devantler merged 4 commits into
mainfrom
claude/declarative-coverage-guard-113
Jul 18, 2026
Merged

test: guard declarative coverage of deploy/#114
devantler merged 4 commits into
mainfrom
claude/declarative-coverage-guard-113

Conversation

@devantler

Copy link
Copy Markdown
Contributor

🤖 Generated by the Daily AI Engineer

Why

The org's GitHub-as-code model is only trustworthy if "declared in deploy/" means all of it.
Today nothing checks that. When a repository is created or renamed it typically gets added to one
or two parts of deploy/ and quietly missed in the rest — so it looks managed on inspection while
its un-modelled settings drift underneath. Seven repositories are already in that state, and the
only way to notice was to hand-audit the tree.

What

Adds a test that fails CI when a repository is declared in some parts of deploy/ but not others,
naming every gap it finds. The seven known gaps are recorded as explicit, tracked exemptions so
this lands green and ratchets forward — new drift fails immediately, and an exemption that is no
longer true also fails, so the list cannot go stale.

Closing the known gaps is deliberately left to follow-up work: repository-settings writes are
currently blocked (#112), and bundling seven policy decisions into a guard would make both harder
to review.

Fixes #113
Part of #56

Every deploy/ dimension should name every managed repository. When a repo is
created or renamed it is typically added to one or two dimensions and missed in
the rest, leaving it partly declared while un-modelled settings drift.

Adds a hermetic guard that derives the expected repository set from the union of
all dimensions and fails closed on any repo missing from one of them. The 14
pre-existing gaps are exempted explicitly with a tracked reason so the guard
lands green and ratchets; a stale exemption also fails.

Part of #56
"${arr[@]}" on an empty array is an unbound-variable error under set -u on
bash < 4.4. The exemption list is designed to shrink to empty as #115 closes the
gaps, so that end state must work on every bash, not just the CI runner's.
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

Readiness record — head 960ce578

Tried and evaluated as a user (ran the guard, did not reason about it):

  • Green on the real tree: 21 repositories across 5 dimensions (14 declared exemption(s)).
  • RED/GREEN-proved every branch individually, each naming the exact rule broken, with GREEN
    restored and a clean tree after each:
    1. New drift — removed deploy/labels/ksail.yamlexit=1, - labels: ksail.
    2. Stale exemption — added the exempted repository-permissions/monorepo.yamlexit=1,
      reported as STALE with the instruction to remove the exemption.
    3. Missing dimension directoryexit=1, names the directory.
    4. Broken selector (directory present, no matches) → exit=1, "broken selector?" — this is the
      vacuous-pass case, so it fails loudly rather than reporting success over an empty set.
  • Non-vacuity control: emptying the exemption list reports exactly 14 gaps, matching the
    matrix computed independently against the live org repo list before the guard was written. The
    exemptions suppress real findings; they are not decorative.

The control found a real defect rather than confirming one. With an empty exemption list,
"${exemptions[@]}" is an unbound-variable error under set -u on bash < 4.4 (verified on bash
3.2.57). That is precisely the end state this guard ratchets toward — when #115 closes all 14 gaps
the list empties — so it is fixed in 960ce578 and the control re-run passes on bash 3.2.

shellcheck and actionlint both clean.

Design note

The guard is hermetic — it reads only the repository tree. CI here runs with permissions: {}
and no token, so a live org listing isn't available, and the expected repository set is the union of
repos named across all dimensions. Known limitation, stated in the script: a repository absent from
every dimension is invisible to it. Closing that needs a token in CI and is deliberately not
bundled here.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 960ce57871

ℹ️ About Codex in GitHub

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

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

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

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

Comment thread tests/declarative-coverage.sh Outdated
Comment thread tests/declarative-coverage.sh Outdated
Comment thread tests/declarative-coverage.sh Outdated
Comment thread .github/workflows/ci.yaml
Codex review (P2 x2): a filename-based check counts a manifest that is not wired
into its directory's kustomization.yaml, and a renamed file whose forProvider
still names the old repo — both are the partial-add/partial-rename drift this
guard exists to catch.

Reads kubectl kustomize output instead. This also removes repository-permissions
as a dimension: it is deliberately not wired into deploy/kustomization.yaml, so
its 4 reported gaps were phantoms. Real gap count 14 -> 10.

Also expires an exemption whose repo has left the model (P3), and syncs the
PR-validation instructions in AGENTS.md and deploy/README.md (P2).
@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

Review resolution — all four findings fixed in 502a87a4

P2 "Drive coverage from kustomization resources" — valid, and it exposed a live error. The guard
now parses kubectl kustomize deploy/ instead of globbing filenames. Checking this also revealed
that deploy/repository-permissions/ is deliberately not wired into deploy/kustomization.yaml
(documented precondition: the sha_pinning_required provider bump; it stalled the tenant health
check for 20 min on 2026-07-09). So four of the fourteen gaps I originally reported were phantoms —
#115 is corrected to 10 real gaps in 2 dimensions.
Proof: with labels/newrepo.yaml wired and team-repositories/grant-admins-on-newrepo.yaml
present but unwired, the guard reports team-admins: newrepo missing. The filename-based
version counted that file as declared.

P2 "Validate repo names from manifest contents" — valid, fixed by the same change. Names now
come from spec.forProvider.repository / .name in the rendered output, never the filename, so a
renamed file whose forProvider still targets the old repo can no longer read as declared. A
welcome side effect: .github is now its real name rather than the dot-github filename alias.

P3 "Expire exemptions when repos leave the union" — valid, fixed. The stale check now has two
arms: the gap was closed (repo is now declared) or the repo has left the model entirely. Proof:
renaming an exemption to team-maintainers/ghost-repo fails with "repo is no longer in the model;
delete this exemption"
.

P2 "Update PR validation instructions" — valid, fixed. AGENTS.md and deploy/README.md both
described kubectl kustomize deploy/ as the sole PR validation; both now list all three commands
CI runs. deploy/README.md also notes the rendered-vs-disk distinction, since that is the
non-obvious part for a contributor.

Every branch re-proven RED individually with GREEN restored and a clean tree; shellcheck and
actionlint clean.

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 502a87a4cb

ℹ️ About Codex in GitHub

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

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

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

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

@devantler

Copy link
Copy Markdown
Contributor Author

🤖 Generated by the Daily AI Engineer

Readiness — pentad clear at 502a87a4, but this is a DEFINITION PR: not self-promoting

surface state
CI all green / skipped
unresolved threads 0 (all 4 Codex findings fixed and resolved)
conflicts mergeStateStatus: CLEAN
pre-merge checks not-posted — CodeRabbit never reviewed; consequence of the one-tool-per-PR Codex lane, not a gap
green review Codex green at 502a87a4cb == head

Held as a draft deliberately. This PR edits AGENTS.md lines 79–81, which sit inside its
## Maintenance section — part of the agent's own definition under the shared contract, and the one
class that keeps the human promotion gate. So it awaits @devantler's promotion; I'll drive it to
merge myself once promoted.

Evaluated as its own user — that is what found the real problems

Running the guard against the tree it governs did more than confirm it:

  1. It surfaced that deploy/repository-permissions/ is deliberately unwired from
    deploy/kustomization.yaml, so the four gaps I originally reported there were phantoms. chore: close the 10 declarative coverage gaps found by the deploy/ coverage guard #115 is
    corrected: 14 → 10 real gaps.
  2. The non-vacuity control (emptying the exemption list) failed with exemptions[@]: unbound variable"${arr[@]}" on an empty array under set -u on bash < 4.4. That is precisely the
    end state this guard ratchets toward, so it would have broken exactly when it finally succeeded.

Every branch RED-proven individually with GREEN restored and a clean tree, including the two cases
the review predicted: a manifest present on disk but unwired (reported missing — the filename-based
version counted it as declared), and an exemption whose repo has left the model.

@devantler
devantler marked this pull request as ready for review July 18, 2026 11:09
@devantler
devantler merged commit 5f16834 into main Jul 18, 2026
11 checks passed
@devantler
devantler deleted the claude/declarative-coverage-guard-113 branch July 18, 2026 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test: guard declarative coverage so deploy/ drift fails CI instead of going unnoticed

1 participant