feat(gitops-api)!: layout corpus, support baseline, and doc-link checker#224
Merged
Conversation
|
Important Review skippedToo many files! This PR contains 504 files, which is 354 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (504)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
sunib
force-pushed
the
feat/gitops-layout-fixtures
branch
from
July 11, 2026 10:17
0346d28 to
c027fef
Compare
The cluster-scoped Secret grant is a property of the packaged manager ClusterRole, not of the API: secretRef on GitProvider, commit signing, and GitTarget encryption all name a Secret in the owning object's namespace. Record that locality as the intended direction, so cross-namespace Secret use would arrive as an explicit access grant rather than a namespace field on the existing refs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
"Does GitOps Reverser support Argo CD / Flux repositories?" is the wrong question: both are mostly orchestration around folders, and the real compatibility boundary is set by what a folder contains and how it is rendered. Deciding that boundary honestly needs the shapes users actually have, so check them in. Fifteen self-contained example repositories under test/fixtures/gitops-layouts: Argo CD plain directories, App of Apps, both ApplicationSet generators (directory and file), kustomize base+overlays, a standard Helm chart, a chart with per-environment values, an external chart with a values-only repo, the canonical Flux monorepo, Flux HelmRelease, repo-per-environment, a cluster x app matrix, SOPS, committed rendered manifests, and a deliberately hostile mixed folder. The corpus is descriptive and drives no test. Each fixture states what the pattern is and what questions it raises; none records a verdict about what is supported, accepted, or refused. Deciding those answers, and pinning them as golden reports under internal/manifestanalyzer/testdata/scan-repo, is a separate step. Every credential, token, key, and ciphertext is a placeholder. The kustomize secretGenerator fixture checks in a secrets.env on purpose, so .gitignore gains a scoped negation for it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…image automation
Review pass on the corpus. The fixtures claim to be realistic, so they should
survive the tools that consume these shapes.
Correctness:
- Two charts did not lint. Go's template parser reads {{ ... }} inside YAML
comments too, so the very comment explaining "this is a template" broke the
chart. Reword both; 06 now says why an example action cannot appear there.
- Fixture 04's chart called include "tenant-app.fullname" and shipped no
_helpers.tpl to define it. Add one. All three charts now `helm lint` clean,
and 04 and 07 `helm template` offline. 06 declares an unvendored redis
subchart, so it needs `helm dependency build` first — the normal state of a
real chart repo, now stated in its README.
- Fixture 07's Argo CD Applications pointed at 07-helm-environment-values/chart.
Every other fixture treats its own directory as the repo root, so the path
resolved nowhere. It is `chart`.
- Fixture 12 set goTemplate: false while templating dotted variables like
{{.path.basename}}, which only the Go engine resolves. It is true now, with
goTemplateOptions, and a comment on why the two syntaxes are not
interchangeable.
- Fixture 15's Flux Kustomization pointed at ./apps, which does not exist there
— an undocumented second failure mode on top of the intended filename
collision. Point it at ./mixed, and document that its GitRepository sourceRef
resolving outside the folder is deliberate, not a defect.
- render.sh had a shebang and mode 100644. It is executable.
New coverage — the repository has other writers:
- 16-flux-image-automation: ImageRepository, ImagePolicy, ImageUpdateAutomation.
A controller clones the repo, rewrites image tags, and pushes a commit, so Git
is an output as well as an input. The tag is marked by a
# {"$imagepolicy": "flux-system:frontend"} setter — a load-bearing YAML
comment whose loss silently disables automation, and a cross-object reference
expressed as a string no schema validates.
- .argocd-source.yaml (path-wide) and .argocd-source-<app>.yaml (per-app) are
hidden dotfiles merged over an Argo CD Application's spec.source, outranking
the values and transformers a reader would inspect. Added to 07, where it beats
the Application's own helm.parameters, and to 05, where Argo CD Image Updater
writes and commits it — the Argo analogue of ImageUpdateAutomation.
The F8 design doc records the question these raise and does not answer it:
comment-preserving in-place editing is necessary but may not be sufficient when
another controller commits to the same branch.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…as a second axis
Running scan-repo across the new layout corpus produced a snapshot whose
refusals are healthy and whose accepts are the finding: three fixtures written
to say "a tool must not touch this" came back green. A SOPS Secret, a folder
rewritten on an interval by ImageUpdateAutomation, and a file whose first line
is DO NOT EDIT.
All three are perfectly renderable — plain KRM, one document, one home file —
so the support boundary passes them. They fail a question it never asks.
Renderability ("can an edit be mapped back to one source document?") and
ownership ("does something else already write this file?") are independent axes,
and only the first exists in the code. Renderability is a property of a folder's
own contents; ownership is a property of something outside the folder pointing
at it. That asymmetry is why the structure-only walk cannot answer it, and why
Argo CD / Flux knowledge is needed at all.
The doc argues that knowledge is cheap to isolate, because it arrives as
ordinary KRM the analyzer has already parsed: an Application is an object with
spec.source.path; a Flux Kustomization is an object with spec.path and
spec.postBuild. So orchestrator-specific packages emit *claims about paths* in a
closed, tool-neutral vocabulary (RenderRootFor, WrittenBy, Generated,
OverriddenBy, TransformedOutOfBand, NotAnEditingTarget) that core consumes
without ever learning what Flux is. No argo-cd or fluxcd Go dependency, ever.
fleetRoot is the specimen: Flux's vocabulary, none of Flux's evidence, and a
three-directory-name guess that false-negatives on the canonical Flux layout
(which spells it `infrastructure`, not `infra`) and false-positives elsewhere.
It is also a public field in pkg/manifestanalyzer at SchemaVersion v1, so
renaming it is free today and a major bump once anything depends on it.
Also records two confirmed defects the snapshot exposed: `editable` is
documented as an ownership claim and computed as pathWithin, which is why an
encrypted Secret counts as editable; and isFleetRoot tests directory names
rather than flux-system/gotk-sync.yaml.
Ships no code. Proposes two ladder entries and answers none of its own open
questions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The package told consumers that "fields are added, never repurposed or removed within a schema major", that its refusal codes were "stable across releases", and that it was the "public, stable answer". The project is at v0.34.0. None of those promises are ones we are ready to keep, and the first API rename we want (RepoSummary.FleetRoot) would have had to break one of them. Say the true thing instead: pre-1.0, no compatibility guarantee, pin a version. Keep the two habits that genuinely help a consumer — ignore unknown fields, and match on IssueKind and the refusal reason codes rather than on the prose in Issue.Message and RefusalReason.Detail. SchemaVersion stays, demoted from a promise to a marker. docs/UPGRADING.md repeated the same guarantee to users and is corrected alongside. The design note that quoted the old wording is updated: the clock on renaming FleetRoot was never the doc comment, it is adoption. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ext, never read it A SOPS Secret in Git is not a folder to refuse. It is a resource we can describe precisely and write blindly: "this is a SOPS Secret, it holds the keys DATABASE_PASSWORD and API_KEY, it is encrypted to these recipients, you may set a new value, and neither you nor GitOps Reverser can read the current one." This works because encryption is asymmetric. Encrypting to an age recipient needs only the public key that .sops.yaml already publishes in the repository, in plaintext. "Write but never read" is not a discipline we enforce; it is what the cryptography gives for free — and it is already how internal/git/encryption.go behaves. ResolvedEncryptionConfig "carries public age recipients only"; a Flux .agekey Secret is read "only to derive the public recipient"; SOPSEncryptor has Encrypt and no Decrypt. The gap is the model, not the crypto. The MAC is what shapes the API. It spans every plaintext value in the document, so one key cannot be patched while keys we cannot read are preserved: the unit of write is the whole document, and its plaintext comes from the live cluster rather than from Git. That kills the tempting CRD whose spec carries plaintext — that is just a Secret with worse RBAC. Instead EncryptedSecret is a projection: sourceRef to the live Secret, and a status that is entirely readable without a key, because SOPS leaves metadata and key *names* in cleartext and encrypts only values. Readable=False is the guarantee, not a degraded state. Four mermaid diagrams: key custody, today's false accept on fixture 13, the rotation sequence, and how the analyzer should classify. Records six open questions, including the sharp one: two encryptions of identical plaintext differ byte-for-byte, so the writer cannot converge by diffing. Ships no code. The boundary doc gains a one-liner and a link. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Mermaid's sequenceDiagram lexer maps `;` to NEWLINE, so a semicolon in note or message prose ends the statement and the remaining words are parsed as a new one — "Expecting SOLID_ARROW..., got 'NEWLINE'". The diagram renders as an error box. The rotation sequence in write-only-encrypted-secrets.md hit it. Parsing every mermaid block in docs/ against mermaid 11.16 found three more, all predating this branch and all the same construct: design/manifest/gitpathaccepted-projection-race-and-external-drift.md finished/canonical-stream-retirement.md finished/commitrequest-multi-finalize-design.md Semicolons become commas. No prose meaning changes. All 78 mermaid blocks under docs/ now parse. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…tent-cluster hydration
Answers the open question left by write-only-encrypted-secrets.md — "one
abstraction, or three kinds?" — with: neither. One capability model, and exactly
one new kind, for a reason that is not encryption.
A real API server settles it. The same ciphertext that Kubernetes rejects inside
a Secret ("illegal base64 data at input byte 3", because `data` is typed as
base64 bytes, plus "unknown field sops") is accepted and round-tripped byte for
byte inside a CRD field typed `string`. So a SOPS Secret is hard because it is
not schema-conformant, not because it is opaque. A SealedSecret holds the same
sort of ciphertext in its own CRD and is ordinary, valid KRM. An ExternalSecret
holds no ciphertext at all.
resource-capability-model.md — Tier 0b, between "is this KRM?" and "how does
kustomize render it?". Four knobs asserted by the document itself:
schemaConformant, visibility (plain/opaque/elsewhere), writeUnit
(document/key/none), derived. A compiled-in classifier table, explicitly not a
plugin API and never a decryption interface. Unknown kinds default to ordinary
KRM, so the registry only ever narrows.
sealed-secrets-and-external-secrets.md — SealedSecrets seals each value
independently with no MAC across the document, so unlike SOPS it supports
per-key writes. Its real cost is not crypto: a live Secret producing a
SealedSecret document is a *kind* transform, and the writer's identity model
assumes one identity per document. Strict sealing scope binds ciphertext to
(namespace, name), so renaming silently destroys every value we did not re-seal
— refuse the rename. ExternalSecrets needs no support at all; it already works.
What both make urgent is a gate we do not have: ESO and sealed-secrets stamp an
ownerReference on the Secret they derive, and a WatchRule matching v1/Secret
would mirror that derived Secret into Git as a second source of truth. Not a
plaintext leak — content_writer.go refuses an unencrypted Secret — but
sanitize.go deletes ownerReferences without ever gating on them, so the evidence
is destroyed rather than used. Never mirror an object with a controller
ownerReference. Tier 0, live-state, protects far more than secrets.
intent-cluster-hydration.md — can Argo CD or Flux sync these into a temporary
cluster? No, on two independent grounds. A reconciler's whole job is to revert
divergence, and in the intent cluster divergence IS the user's edit. And a SOPS
Secret fails to deserialize regardless of validation settings, so Flux does not
ignore it, it fails the Kustomization. Worse, a lenient applier stores the
ciphertext as the value and an app reads its password as the literal text
"ENC[AES256_GCM,...]" with no error anywhere.
The rule: the intent cluster gets CRDs, never controllers. It is a schema
surface, not a reconciliation surface. SealedSecret and ExternalSecret then
hydrate as themselves and sit inert and editable. A SOPS Secret cannot, which is
the entire reason EncryptedSecret exists. Hydration is a one-shot strict apply of
the document store the analyzer already builds, and RepoReport should grow
requiredCRDs to say what must be installed first.
Ships no code. Eleven new open questions, none answered.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…is the design, not the hazard The hydration doc claimed "the intent cluster must never have a controller reconciling it toward Git". That is too absolute, and docs/bi-directional.md already says something sharper: the problem is causality, not co-existence. An *autonomous* reconciler races the commit and replays a stale revision. A deliberately triggered one, waiting on the exact SHA, does not. That handshake is not a proposal. test/e2e/bi_directional_e2e_test.go runs it against real Flux with prune: true, and asserts the hard part — one live edit produces exactly one commit and Flux converges without a second. Both the GitRepository and the Kustomization carry a 30m interval while the test drives `flux reconcile` explicitly and waits for the expected revision. That long interval is the discipline: it makes the reconciler an applier rather than a competitor. The same test also covers a SOPS Secret written by the operator and reconciled by Flux, and a revert. So hydration is the degenerate case of the acknowledgment handshake — apply revision X into an empty cluster and wait for it to be reported applied — and a triggered Flux Kustomization is the better hydrator whenever the subtree is schema-conformant. GitOps Reverser applies the document store itself only where no applier can: the SOPS class. The rule becomes "always install the CRDs; install a controller only if it is an applier you drive, and never one that materialises secrets." ESO and sealed-secrets stay out for what they would pull into a throwaway cluster, not because they reconcile. Also corrects the key-custody claim in write-only-encrypted-secrets.md. The operator does not merely receive a public recipient: GitTargetReconciler calls generateAgeIdentity(), mints a keypair, stores the private half in a Secret annotated "Back up the private key", and the bi-directional e2e hands that same Secret to Flux as its decryption key. The write path still never reads it back — Encrypt is the only method — but under that bootstrap model "cannot decrypt" is a design rule enforced by a code shape, not a cryptographic guarantee. Under the user-supplied-recipient model it is cryptographic. Name the model before making the claim. Argo CD remains untested for the handshake; its analogue is Refresh plus selfHeal: false. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…real Flux
Ran four experiments against the running kustomize-controller on a folder holding
one plain ConfigMap and one SOPS Secret, no decryption key configured.
First correction: Flux never reaches the API server's base64 error. It guards
explicitly, and says so — "Secret/.../frontend-credentials is SOPS encrypted,
configuring decryption is required for this secret to be reconciled". kustomize
build itself is perfectly happy and passes the document through, sops: stanza and
all. The guard is kustomize-controller's. But the refusal is all-or-nothing: the
ordinary ConfigMap beside it was not applied either.
Then, can it be told to skip?
Kustomization.spec.ignore, paths /data and /sops -> Ready=False, same message.
It excludes JSON-pointer paths from drift detection and apply, and the SOPS
guard fires before the ignore rules run. The obvious answer is not the answer.
GitRepository.spec.ignore excluding **/*.enc.yaml -> Ready=False,
"accumulating resources from 'secret.enc.yaml': no such file". The
kustomization.yaml still lists it, so removing it from the artifact breaks
kustomize build.
Kustomization.spec.patches with $patch: delete -> Ready=True. ConfigMap
applied, Secret never created.
plain folder (no kustomization.yaml) + GitRepository.spec.ignore -> Ready=True.
So both working mechanisms exist and the choice is decided by folder layout —
precisely the plain-versus-kustomize split the analyzer already reports. And
neither should be hand-maintained: the analyzer already knows which documents are
encrypted (CauseEncrypted) and which folders are render roots, so the hydrator can
generate the right exclusion and project an EncryptedSecret for each document it
excluded.
This makes triggered Flux the hydrator for SOPS repositories too, not only for
schema-conformant ones. Applying the document store directly stays as the fallback
for environments with no GitOps controller, and is a smaller thing to build than it
looked.
Caveats recorded: GitRepository.spec.ignore is a property of the source shared by
every Kustomization referencing it, so an intent cluster wants its own
GitRepository; excluded resources are never applied and so never pruned. Argo CD is
untested and has no SOPS guard at all — its analogues are directory.exclude and
kustomize.patches, and an unexcluded document would hit the raw base64 error.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
force-pushed
the
feat/gitops-layout-fixtures
branch
from
July 11, 2026 18:23
c027fef to
4c3d433
Compare
…oc tree Two proposals, no code. expansion-boundary-and-corpus-organisation.md adds the axis the model was missing. Renderability and ownership are both questions about files in Git; neither asks whether a live object has a home in Git at all. ApplicationSets, ResourceSets, KRO, Crossplane and HelmReleases all synthesise objects that have no home, and mirroring them creates a second source of truth that fights the controller that made it. Source-verified against external-sources/: ApplicationSet generates pointers into Git (the workload KRM stays as files, so it round-trips), while flux-operator's ResourceSet generates the KRM itself, inline in the CR spec. That asymmetry decides the support story, and it exposes a defect: the planned derived-object gate keys on ownerReference, which ApplicationSet sets and ResourceSet does not. documentation-triage.md classifies all 180 docs. About 35 bind; the rest is history. The folders lie in both directions: docs/design/ holds 23 shipped designs, and docs/finished/ holds 10 documents that are the sole record of a live invariant. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
180 documents, ~55k lines, in two folders whose names lied in both directions: docs/design/ held 23 designs whose work had shipped, and docs/finished/ held 10 documents that were the sole record of a live invariant — including current-manifest-support-review.md, the spec of internal/manifestanalyzer, cited by section name from eight Go files. Three folders now mean three things: design/ we are still deciding (9 open items + the gitops-api workstream) spec/ this is true now, and the code depends on it (NEW, 25 docs) finished/ this happened (history; nothing here binds) docs/design/manifest/ is gone. All three of its subsystems (internal/typeset, internal/git/manifestedit, internal/manifestreport) have shipped, so its 19 working-note documents collapse into one spec/manifest-system.md that states how the pipeline works today, plus the four that were genuinely live specs. The important part: 17 doc citations in the Go source were ALREADY broken before this change — six pointed at documents that no longer existed anywhere. Earlier "docs: cleaning" commits moved files and never fixed the pointers, which is how the tree became unreadable. Every citation now resolves: 125 in Go, 244 relative links rewritten across the docs, 39 de-linked where the target is genuinely gone. Verified by a link check over every markdown file and every docs/*.md path mentioned in Go. 180 -> 117 documents. Nothing is lost; git log --follow has all of it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 0 of the triage, which is the part that actually answers "this is too much to oversee": - docs/INDEX.md names the ~35 documents that bind, out of 117. Everything not on that page is a user guide or history. - docs/design/gitops-api/support-contract.md is the single page that says what we support and what we refuse. That statement was previously asserted in 17 places across two doc trees, in three unrelated refusal vocabularies, and assembled nowhere. - Every doc in spec/, design/, finished/ and facts/ now opens with a one-line lifecycle banner, so a reader arriving from a Go citation knows immediately whether they are holding a contract or an artefact. Consolidation, so the boundary stops being restated: - fan-in = 1 now has one home (gittarget-granularity §1). kustomize-support §4 quotes it and says so. - The kubectl --dry-run=server evidence matrix lived twice, near-verbatim; intent-cluster-hydration now cites resource-capability-model instead of repeating it. - The gitops-api README's flat list of nine "Related:" links is now a table grouped by topic, with support-contract.md as the front door. Two stale headers corrected: unsupported-folder-refusal-plan.md said PROPOSAL while shipping as GitPathAccepted with an e2e test, and the triage plan itself moves to finished/ now that it has been executed. task lint: 0 issues. task test: pass, coverage 75.5% (unchanged). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three documents in spec/ are not cited by path from Go (gvk-gvr-mapping-layer, sops-single-file-no-multidoc, e2e-test-design), so "everything here is cited from Go source" was an overclaim in the very document that defines the folder. They belong in spec/ regardless — each is the only written record of a rule the code still obeys — but the claim now says what is true. Full suite green on the reorg: lint 0 issues, unit tests pass at 75.5% coverage, e2e 54 passed / 0 failed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…c-link checker Adds the GitOps layout fixture corpus with a generated behavioural baseline (test/fixtures/gitops-layouts/support-today.md), the expansion provenance-marker e2e spec in the bi-directional corner, and doccheck -- a dev CLI that resolves both markdown links and the docs/**.md citations buried in Go comments, which nothing off the shelf checks together. Also fixes three problems the above surfaced: - lint-docs fingerprinted '**/*.md' and '**/*.go' unrooted, so Task walked every path under the repo root -- including external-sources/ (1.2 GB of upstream checkouts) and bin/ (815 MB). Computing that fingerprint allocated tens of GB inside the Task process itself and OOM-killed the host; `task --status lint-docs` alone hit 4.4 GB and was still climbing. Scoping the globs per top-level tree takes it to 42 MB. - lint-golang's sources omitted hack/**/*.go and pkg/**/*.go, so Task could report "up to date" while golangci-lint had never seen a change in either tree. It lints both. - The docs reorg left eight dangling docs/ citations in Taskfiles, workflows, charts, and scripts. doccheck reads markdown links and Go comments, so it does not cover those surfaces; they were repointed by hand. The devcontainer now caps its own memory so a runaway build cannot starve the host. Note this bounds only what runs in-container: docker-outside-of-docker means the e2e k3d containers are siblings on the host daemon. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…g the check doccheck read Markdown links and Go comments, but nothing looked at the docs/**.md paths cited in Taskfiles, CI workflows, chart values and hack scripts. A docs reorg left eight of them dangling and the tool reported success. It now scans those too (510 YAML/shell files), and skips docs paths that are the tail of a URL -- those name a rendered page, not a file in this repo. That URL guard fixes a live bug on the Go surface as well: a comment linking to the published docs was reported as a dangling repo-relative path. doccheck's own package doc contains such a link, and it reported itself as broken. lint-docs also loses its `sources:` and now always runs. Its real input is "every git-tracked .md, .go, .yml, .yaml and .sh", which Task cannot express, and both approximations failed: an unrooted '**/*.go' walks external-sources/ and OOM-kills the host (and `exclude:` does not help -- Task expands the glob before filtering), while a hand-maintained per-tree list silently drifts out of sync with the file types doccheck scans, and a stale "up to date" means the check never runs. The whole check takes 0.2s, which is not worth a cache that can be wrong. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The design folder was named docs/design/gitops-api/ — after a separate, private product repo — and it had grown into that repo's roadmap living inside the public operator. Three docs described the product, not the operator, and the feature ladder (F1-F8, launch scope, delivery order) told anyone reading the OSS project where the commercial layer was going. The line drawn here: the operator publishes what it will and will not write, because that is the contract it owes its users and it announces that boundary at runtime anyway. It does not publish where it is going. - Moved to the private repo: the-layer-above-and-the-expansion-graph, intent-cluster-hydration, and §8-§10 of the kustomize doc (mirror/intent mode, the three arrows, the ladder). The invertibility taxonomy, layout allowlist and fan-in invariant stay public. - Renamed docs/design/gitops-api/ -> docs/design/support-boundary/, and rewrote the README from "editing GitOps folders as a product surface" into an index of the boundary. - Scrubbed the ladder and product-layer vocabulary from the docs that stay. The refusal reasons, the corpus taxonomy, the write boundary, and every piece of test evidence are untouched — that content is the point. - The ladder had leaked into runtime output: the exported ReasonOverlayFanOutNeedsF2 made the CLI tell users "overlay-fan-out-needs-f2 / render-root scoping (F2) required", naming a roadmap item they cannot see. Renamed to ReasonOverlayFanOutUnsupported. No consumer imports the package yet. - CI now regenerates the GitOps layout baseline and fails on a diff, so a change to the acceptance boundary shows up in review as exactly which fixtures moved. Without it the corpus was evidence of nothing. e2e labels (f4-placement, f7-higher-level-krm) are deliberately left alone: CI shards on them and a test label leaks no roadmap. task lint, task test (54 specs) and task test-e2e (54 passed, 0 failed) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The f4-placement / f7-higher-level-krm labels were the last place the feature ladder was visible, and nothing depended on them: CI shards the suite on "manager", "!manager && !image-refresh && !bi-directional" and "bi-directional", never on these. They follow the house pattern of a descriptive second label for ad-hoc local runs, so they are renamed rather than deleted: f4-placement -> new-file-placement f7-higher-level-krm -> higher-level-krm The rename also covers the spec filenames, Describe names, fixture folder, GitProvider/GitTarget/WatchRule names, git path, Gitea repo name and test namespace, so no F-number survives in what an e2e run prints. Verified by running exactly the two renamed specs: task test-e2e E2E_LABEL_FILTER="new-file-placement || higher-level-krm" Ran 2 of 65 Specs -- 2 Passed | 0 Failed task lint and task test pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
CI was already red on this branch before any of the boundary work, and the reason is a hole in doccheck itself: it listed git-tracked files as the *sources* to scan but resolved link *targets* with os.Stat. A link into a gitignored path therefore existed on the author's machine and passed, then broke in CI's fresh clone. That is the exact class of dangling reference the check exists to catch, and it was blind to it. docs/spec/unsupported-folder-refusal-plan.md linked twice into .agents/skills/, which .gitignore excludes (line 56). Locally: OK. In CI: 2 broken references, Lint red. Both citations are now plain text — the skill is named, not linked. Existence is a question about the repository, not about this disk, so targets now resolve against `git ls-files` plus the directories those files imply (a link may legitimately point at a folder). Two regression tests pin both halves. task lint, task test and doccheck all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…oc check Second pre-existing failure on this branch, and the doccheck one was masking it: 63bbcf4 deliberately dropped `sources:` from lint-docs so it always runs, but the Lint job still asserts a second `task --dry lint` is *fully* cached. Those cannot both be true, so Lint went red the moment doccheck started passing. lint-docs is exempt on purpose: its real input is "every tracked .md, .go, .yml and .sh", which Task cannot express — an unrooted `**` walks external-sources/ and has OOM'd the host, and a hand-maintained list silently drifts out of sync with what doccheck scans. A stale "up to date" there means the check never runs. It costs ~0.2s, so it always runs. The assertion now exempts it and still fails on any other task re-running. Written with awk rather than `grep -qv`: the CI image ships ugrep as grep, and its -q -v returns 1 even when -c reports a selected line — the obvious spelling of this assertion would have silently passed whatever it was handed. Verified against the real dry-run plus negative and empty controls. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
changed the title
The message was a double-quoted string inside a `bash -lc` block, so the backticks around the remediation command were command substitution, not quoting: on a stale baseline the echo re-ran the whole analyzer build and 18-fixture scan, and spliced its stdout into the ::error annotation — losing the instruction it existed to print. Third instance of this bug class on this branch (f550b5d, 34fffc9), so the site now carries a comment saying why backticks cannot appear there. Also drops the 3.9 MB `doccheck` ELF binary that `go build ./hack/doccheck` leaves at the repo root, committed by accident in 1574ed4. The .gitignore entry is root-anchored on purpose: a bare `doccheck` would also match the hack/doccheck/ SOURCE directory and untrack it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pattern was anchored on `docs/`, so a package that keeps its contract next to its code — internal/git/manifestedit/DECISION.md — was cited by a Go comment the checker structurally could not see. This branch introduced such a citation while claiming to verify "every documentation reference". Widening it to any repo-relative *.md path immediately found two real ones: argocd_bi_directional_e2e_test.go cited the support-boundary doc twice WITHOUT its `docs/` prefix, and the old regex was blind to exactly that. Both fixed here. A citation must contain a slash (a bare `README.md` in prose names no particular file) and must start at the beginning of its path. The second rule is what isPathTail enforces: when the regex can only latch onto the middle of a longer token, what it captured is a fragment — an example `../`-relative link, or a shell `$var/`-assembled path — and reporting it would be a false positive the shell case could not even fix. The golangci exclusion note is corrected while here: it justified the gosec exemption with os.Stat calls the tool has not made since 1574ed4 replaced filesystem probing with a git ls-files set. The exemption is for reading tracked files by a computed path (G304). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`overlay-fan-out-needs-f2` -> `overlay-fan-out-unsupported` changed both the Go constant and the machine-readable RefusalReason.Code that pkg/ documents as a contract. A Go consumer gets a compile error; a consumer matching the JSON string gets nothing at all — it silently stops matching and the refusal falls through to their default branch. That is the case the entry calls out. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A refusal detail is free prose the analyzer wrote. The day one of them quotes a shell pipeline or a YAML block scalar, an unescaped `|` splits that row into extra columns and support-today.md — a CI-enforced baseline whose entire job is to be reviewable — renders wrong with no failure anywhere. No fixture produces a pipe today, so the regenerated baseline is byte-identical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
sunib
changed the title
This was referenced Jul 14, 2026
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.
Summary
Four strands, all in service of one thing: making the support boundary something you can check rather than something you have to remember.
test/fixtures/gitops-layouts/, grouped into six families by what the repository actually is, not by which controller reads it.support-today.md— a generated, CI-enforced behavioural baseline recording whatmanifest-analyzer --mode scan-reporeports for every fixture, so a change to the acceptance boundary shows up in review as "exactly which fixtures moved, and in which direction".hack/doccheck— a doc-reference checker wired intotask lint, because the references that rot silently are the ones inside Go comments, Taskfiles and CI workflows, and nothing off the shelf looks there.spec/binds,design/is undecided,facts/is reference,finished/is history) with a precise, public support contract at the centre of it.Plus a new e2e spec that measures what five real GitOps controllers actually stamp on the objects they create.
The diff is large (~508 files) but lopsided: the great majority is fixture YAML and documentation moves. The only change to Go behaviour is one renamed refusal code — see "Behaviour change" below.
Why
Argo CD and Flux are orchestration around repository content. Whether GitOps Reverser can safely edit a folder depends on that folder's contents and rendering boundary, not on which controller happens to point at it. That claim was previously spread across prose and memory. The corpus turns it into concrete inputs, and the baseline turns the current answer into a diffable artifact.
1. The layout corpus — 18 fixtures, six families
1-desired-state2-rendered3-expanded4-machine-written5-opaque6-hostileEach fixture carries a README stating what it is and what a live tool would have to understand to edit it safely.
2.
support-today.md— a baseline, not documentationGenerated by
task gitops-layouts-baseline(hack/gitops-layouts-baseline.sh) from a live analyzer scan. It is descriptive: it records what the tool does today, not what it should do. Where it disagrees withdocs/design/support-boundary/support-contract.md, that disagreement is the backlog.CI regenerates it and fails on a diff. Without that, the corpus stops being evidence of anything.
Note what the baseline explicitly does not claim:
rc=0means the scan command succeeded, not that the fixture is supported.scan-repois structure-only — it never executes Helm, Kustomize, Argo CD, Flux, SOPS, or a remote fetch. A missing candidate matters as much as a refusal: it means the tool did not explain that part of the repository at all.3.
hack/doccheck— the link checker, and why it existsChecks that every repository-local documentation reference resolves, across three surfaces:
*.mdpaths cited inside Go comments — the ones that rot silently. Seventeen were dangling before this check existed.Three design points worth reviewing:
gittargetignoretests build in-memory filesystems whose entries are named like documentation paths; those are fixtures, not citations. A regex cannot tell them apart, the parser can.git ls-files, neveros.Stat. Resolving against the filesystem would pass on the author's machine and fail in a fresh clone — a link into a gitignored path (.agents/,external-sources/) exists locally and nowhere else. That is precisely the reference this check must catch.docs/. A package may keep its contract next to its code (internal/git/manifestedit/DECISION.md), and those citations rot the same way. Anchoring ondocs/made a whole class structurally invisible — including two comments inargocd_bi_directional_e2e_test.gothat cited the support-boundary doc without itsdocs/prefix, which the widened check immediately found and which are fixed here. A citation must contain a slash (a bareREADME.mdnames no particular file) and must start at the beginning of its path, so a../-relative example link or a shell$var/-assembled path is not mistaken for one.Wired in as
task lint-docs, deliberately un-fingerprinted (nosources:). Its real input is "every tracked.md,.go,.yml,.sh", which Task cannot express: an unrooted**glob walksexternal-sources/and has OOMed the host, and a hand-maintained file list silently drifts. A stale "up to date" means the check never runs. It costs ~0.2s. The CI lint-cache assertion exempts it explicitly.4. The documentation tree, split by lifecycle
70 documents removed, 54 moved, 13 added; the tree went from 180 files to 117, of which about 35 actually bind.
docs/INDEX.mdnames those.The rule that was missing before:
design/held shipped work andfinished/held live contracts. Folders now mean lifecycle, not topic.spec/design/facts/finished/New:
docs/design/support-boundary/— the support contract, the orchestrator knowledge boundary, the resource capability model, write-only encrypted secrets, SealedSecrets/ExternalSecrets, and the kustomize boundary. The support boundary is public; the product roadmap is not.5. Behaviour change: the internal ladder shorthand is gone
The
F1–F8shorthand leaked out of private planning into code, e2e labels, fixture names and doc filenames. It is removed. Mostly this is renames (f4_placement_e2e_test.go→new_file_placement_e2e_test.go, labelf4→new-file-placement), but one rename crosses the public API surface:RefusalReason.Codeis documented as machine-readable. A Go consumer gets a compile error; a consumer matching the JSON string does not — it silently stops matching, and the refusal falls through to their default branch. Recorded indocs/UPGRADING.md;pkg/manifestanalyzeris pre-1.0 and consumers are told to pin, but reviewers should look at this deliberately.Apart from this rename, no Go source line outside a comment changes.
6. New e2e: expansion provenance markers
test/e2e/provenance_markers_e2e_test.go— five producers, one repository, one commit:kustomize.toolkit.fluxcd.io/*argocd.argoproj.io/tracking-idhelm.toolkit.fluxcd.io/*resourceset.fluxcd.controlplane.io/*The design docs assumed the evidence of a synthesised object was a controller
ownerReference. It is not — four of the five producers set none. Andkustomize.toolkit/helm.toolkitare sibling prefixes from the same vendor with opposite verdicts, so any rule gating on the prefix family gets one of them exactly backwards.This spec pins the markers against real controllers, so the gate is built on measurement rather than memory. If a controller upgrade changes a marker, it fails loudly. It lives in the bi-directional corner (the only place Argo CD is installed), drives no GitTarget and no WatchRule — the reverser is not under test here, the ecosystem is.
BI_DIRECTIONAL_GINKGO_PROCS2 → 3.Recorded as
docs/facts/expansion-provenance-markers.md.Smaller changes
.devcontainer:--memory=12g --memory-swap=16g, so a runaway build is OOM-killed in the container instead of starving the host. (Bounds only what runs in the container; k3d's e2e containers are siblings on the host daemon.)awk, notgrep -qv: the CI image ships ugrep asgrep, whose-q -vreturns 1 even when-creports a selected line — which would make the assertion silently pass whatever it was handed.lint-golangnow also lintshack/**andpkg/**..gitignoreun-ignores the corpus's kustomizesecretGeneratorenv file (placeholder text, never a credential).hack/audit-kind-check/(a scratch kind investigation) deleted; its conclusion is recorded indocs/ci/findings.md.Review points
.argocd-source*.yamldotfiles can change effective application source configuration.Validation
Full sequence run on the head commit, in the dev container:
task fmt/task generate/task manifests— no diff; the generated tree was already current.task vet— clean.task lint— 0 issues (golangci-lint,actionlint,hadolint,helm lint,doccheck).task test— pass; unit coverage 75.4%, within the 75.5% baseline's 0.5% tolerance.task test-e2e— 54 passed, 0 failed, 11 skipped (534s).task gitops-layouts-baseline— regeneratessupport-today.mdbyte-identical to the committed file, so the CI staleness gate is green.The 11 skipped are the opt-in bi-directional corner, which is where the new provenance-markers spec lives. It does not run under the default
task test-e2e; it needsE2E_ENABLE_BI_DIRECTIONAL(task test-e2e-bi-directional), and in CI it has its own leg and its own cluster. Reviewers should confirm that leg is green rather than reading "e2e passed" as covering it.