feat(gittarget): add spec.suspend and status.placement, refuse ambiguous layouts, and execute the layout corpus - #326
Merged
sunib merged 26 commits intoSep 1, 2026
Conversation
The eighteen fixture folders under docs/layout/shapes/ and docs/layout/specific-examples/ were read by nothing but a human: no Go file referenced either directory, so every claim they make about where a document lands and what the commit looks like was prose that nothing could falsify. Each scenario now seeds a worktree from repository/, folds input/ through the real plan-then-flush path with the flush policy derived from config/gittarget.yaml, and compares a normalized diff (no index lines, no blob hashes) against expected-*.patch. A -update flag rewrites the patches. Three conventions carry over from docs/design/build-order.md and are enforced here rather than described: scenarios for unbuilt behavior are written now and skipped naming the PR that unskips them (PR 2 is finished when the last skip is gone); config/gittarget.yaml parses into a harness-local struct, itself checked against the real v1alpha3.GitTarget so the examples cannot describe an API nobody built; and refusals are fixtures too — shape 8's base-owned env-var edit asserts a refusal, not a patch. Two things the fixtures got wrong, now corrected by running them: - repository/ used two different conventions — six folders were rooted at spec.path and five at the repository root — and swapping either broke its expected patch. Every repository/ is now rooted at the repository root, which is also the only reading shapes 6 to 8 can use, since their base lives outside spec.path. - The expected patches were hand-authored approximations: wrong hunk headers, file order that no diff produces, and a committed Argo CD Application whose spec keys were not in the order the writer emits them. They are regenerated from the observed diff and are now what Git actually gets.
…r's layout Two additive fields and the pure function that fills the second one in. spec.suspend stops the WRITE, not the scan, and that asymmetry is the feature. Placement only ever affects documents that do not exist yet, so nothing about it is observable by inspecting the folder: a user declaring a target against a real repository has no way to see what the operator would do until it has already done it. A suspended target that still scans and still publishes status.placement is the dry run that closes that gap. status.placement is what the last scan resolved: the render root governing new documents, whether they carry their own namespace, how many byType templates are declared, when it was observed, and up to three illustrative destinations. Three things it deliberately is not: it carries no accumulating counters (those are metrics, and a counter in status is a status write per event), the resolution reason lives on the LayoutResolved condition rather than in a bespoke field, and nothing in it depends on a placement having HAPPENED — every value is a fact about the folder, so the stanza is populated before the target writes a byte. manifestanalyzer.ResolveLayout computes it, with three verdicts: SingleKustomization, Ambiguous, and None. Its "exactly one writable supported kustomization" predicate is lifted out of resolveKustomizeRoot and both now call it, so the layout that is REPORTED and the layout placement TAKES cannot drift — a LayoutResolved saying SingleKustomization while the ladder declines would be worse than no report. The examples are resolved through LocateNew itself for the same reason.
…spend The scan that precedes a write already builds the store the layout resolves from, but it happened on a branch-worker goroutine with no result channel back to the controller, so the resolution was computed and dropped. LayoutReporter is the hook that carries it out, modelled on PathRefusalReporter for the same structural reason. The report is taken from the batch's own store — the one the write resolves placements against — rather than from a second scan, so it describes the layout this write actually used and not a second opinion. Both scan sites report: the live flush, and refuseUnsafeWorktree on the resync path. The second is the one that matters most, because it runs whether or not any event arrived, which is what populates status.placement on a target that has never written a byte. spec.suspend gates the write and only the write. A suspended target reaches the scan, publishes what it resolved, and then stops: no flush, no commit, no push. Deliberately NOT skipping the scan with the write — a suspend that also stopped scanning would freeze status.placement at whatever the folder looked like when suspension began, and a dry run showing a stale answer with no way to tell is worse than no dry run. Suppressed events are dropped rather than deferred: resuming replays the cluster's current state on the next resync.
…concile request The controller half of the dry run. status.placement and LayoutResolved are published AHEAD of every gate, so a target held unready still shows what its folder resolved to. That ordering is the point rather than a convenience: the stanza exists to be readable before the target is doing anything, and a projection that only ran on the happy path would be missing exactly when it is wanted. A folder nothing has scanned yet reports Unknown/NotScanned, which is a different state from a folder that resolved to no root — collapsing the two would make a target that has never read its folder indistinguishable from one that read it and found a plain directory. Only Ambiguous is False. None is a healthy, definite answer, and reporting the ordinary case as False is how a condition gets trained out of a reader's attention — the same mistake status.retention was designed not to make. The Ambiguous rule GATES, and this is the one place PR 1 changes what the operator writes: a folder covering several render roots refuses to place a new document instead of putting it into whichever root an arbitrary rule picks. It gates at the WRITE, not on the Validated condition the spec named, because Validated is evaluated before the data plane exists. A target failing it never registers a worker, so it would never scan, so it could never observe that the folder had been fixed — and for a target that had never scanned, the rule could never fire in the first place. Refusing in createNew keeps the target declared and scanning: the refusal reaches status as GitPathAccepted=False/AmbiguousLayout through the same reporter every other content refusal uses, and narrowing the target to a leaf overlay clears it. It is also raised at the placement site rather than over the whole flush, so it never pre-empts the file-level L1/L2 preconditions, whose messages name the offending file rather than the folder. spec.suspend reports Ready=True with reason Suspended. Not writing is the configured outcome, and no condition may go False for one; every real gate still applies, so a suspended target with a broken provider is still not Ready. reconcile.configbutler.ai/requestedAt is the reconcile request, spelled as Flux spells it because that is what a user brings the muscle memory for. It needs its own For() predicate — an annotation edit does not bump metadata.generation, which is exactly why GenerationChangedPredicate is safe against our own status writes — and it forces the same re-anchor a refused Git path does, so a suspended target can be iterated with rather than waited on. Which values have been acted on is remembered in memory rather than echoed into status: the cost is one extra re-check per controller restart, which is cheaper than a field every consumer then has to understand.
…change configuration.md gains the two user-facing sections the fields need: the dry run (spec.suspend plus the reconcile-request annotation, with the resume semantics spelled out — the current cluster state, not a backlog) and what the folder resolved to (status.placement field by field, and the Ambiguous rule with the fix it implies). status-conditions-guide.md gains LayoutResolved and the suspended read, including why SingleKustomization and None are both True: reporting the ordinary case as False is how a condition gets trained out of a reader's attention. build-order.md is CORRECTED rather than extended. It asserted, three times and as the argument for merging PR 1's four parts into one review, that nothing in PR 1 changes what the operator writes. The Ambiguous rule gates, so that is now false in exactly one case, and a plan-of-record page that misstates the property its own cut rests on is worse than one that admits the exception. It now names the case, the before-and-after behavior, and why the gate is at the write rather than on Validated. Also adds the e2e spec for suspend. Its claim is a wiring claim the unit tests cannot reach: a suspended target has to still declare, still start its watches, still resync, and still have the resulting layout report cross the watch plane into status — while writing nothing. The negative half is paired with a barrier, a co-resident active GitTarget in the same repository fed by the same events, so "nothing was written" means the target decided not to write rather than that the pipeline was asleep.
…ally raises The two expected-*-status.yaml files were prose: nothing read them, so their messages described a refusal in wording the writer never used. The corpus now reads each one's GitPathAccepted condition and requires the flush to refuse with that message, and both files are corrected to the text the writer emits. GitPathAccepted is the condition asserted here because its message is the writer's own. Every other condition in those files is the controller's projection of the same refusal, and belongs to the controller's tests.
Two scenarios in one folder can share a config and differ only in their input — shape 8's image bump and env change do — so naming subtests by config produced "gittarget-prod" and "gittarget-prod#01", where the interesting half was the suffix Go appended. They are keyed on the expectation file now.
docs/design/build-order.md names four refusals the set needs, on the grounds that a scenario set where every write succeeds is advertising rather than specification. Two were already here (the two render roots, the base-owned field edit) and two were not. - shape 2 gains a second WatchRule from a second source namespace against an explicit serializeNamespace: false. Refused, because what follows is not a collision but a MATCH: both documents carry no namespace, so their manifest identities are equal and each write flips one document between two live objects. - shape 4 gains a serializeNamespace: false with nothing in the repository supplying the namespace. It REPORTS rather than refuses, which makes it the one scenario asserting neither a patch nor a write refusal — so its skip message says PR 2 must add that third assertion mode along with the rule. model.md's list of gaps the corpus should fill is corrected with them. It also named a missing ClusterProvider for empty-repo-bootstrap, a folder that no longer exists: it was merged into shape 5 two commits before this branch started.
The suspend spec asserted status.placement.observedRevision on the first scan, where the repository branch has no commit yet — nothing has written to it — so the scan honestly read the folder at no revision and the field was correctly absent. The assertion moves to after the barrier, where the active target has produced a commit and the scan has a revision to name.
The three refusal configs added to shapes 2, 4 and 6 were reachable only by listing the directory. Each shape's README now links the config and the status it produces from the section that already argues for it, so the fixture is where a reader is when they want it.
The suspend gate returns success, so the drain marks the scope render-fidelity clean and reports zero retained documents without either having been measured. Both are fine while writes are off and both are re-measured on resume, but a reader arriving at that early return deserves to be told rather than to work it out from the drain. The alternative — returning an error — is named and rejected: a suspended target is not a failing one.
…scribes It is a folder-wide answer to a question the writer decides per document: inference omits metadata.namespace only when the governing root's value equals THIS object's own namespace, so an object arriving from a different namespace has its namespace written whatever the stanza reports. The folder-wide reading is still the right one to publish — a namespace-free folder receiving two source namespaces is a contradiction rather than a nuance, and it is the shape PR 2's one-source-namespace rule refuses — but the field must not be read as a guarantee it does not make.
The transition test ignored the revision so that a busy branch would not write status once per commit per target. The consequence, which the e2e caught, is that the field was written by the FIRST scan — of a branch that usually has no commit yet, so empty — and then never advanced, leaving it permanently blank on exactly the targets it is meant to inform. A report that has a revision where the last had none is now always a change. A later revision alone still is not, so the steady state stays quiet, and the field means the revision this resolution was FIRST observed at rather than the latest scanned. The API doc says so now instead of implying the opposite. The republish rules get their own tests: unchanged is quiet, a later revision alone is quiet, gaining a first revision is not, and absent vs. false serializeNamespace are different answers. The e2e asserts the revision through a reconcile request, which is the loop a user iterating on a dry run actually runs.
Adding IssueAmbiguousLayout with a comment above it split the map into two gofmt alignment groups, so a one-line addition reflowed eleven lines around it. The comment moves to the constant's own doc, where the rationale already lives.
Every path in a refusal is relative to the write jail, so the folder's own name is ".", and the message opened with a bare period that reads as nothing at all. It names the GitTarget path instead, matching the LayoutResolved message a reader sees beside it. The roots it lists stay relative, which is what makes it actionable.
…uspend's cutover Review follow-ups. The status fixtures were only half executable: the corpus read GitPathAccepted's MESSAGE and nothing else, so its status and reason could drift while the suite stayed green, and no test covered the new IssueAmbiguousLayout -> AmbiguousLayout mapping at all. Three changes close it: - The reason mapping moves from internal/watch to manifestanalyzer, beside the IssueKind constants it reads. Both the projection and the corpus need the same answer and the corpus cannot import watch, which is why three comments used to ask callers to keep their strings in sync with it by hand. - The corpus now asserts GitPathAccepted whole: status, reason (through that mapping) and message. Perturbing any of the three fails it. - LayoutResolved and Stalled are the CONTROLLER's projection, which no write-path test can produce, so internal/controller asserts them against the same files. internal/layoutfixture is the shared reader: a parser copied into both packages would be two things that must agree about a file format, which is the drift the fixtures exist to prevent. spec.suspend's cutover is now stated rather than implied. It takes effect at the next planning boundary: the gate reads the value captured when the write was planned, so a suspension arriving later does not retract it, and a commit already made locally is still pushed. That is the contract rather than a gap in it — a local commit that is never pushed would sit in the worker's checkout and surface later, out of order, on resume. The API doc, configuration.md's wording and a new test say so. Three documentation contradictions the review found: - The CRD said status.placement.serializeNamespace is absent when no render root governs the folder; ResolveLayout sets it true there, because nothing supplies the namespace. It is absent only when SEVERAL roots make the answer per document. - model.md still claimed PR 1 changes no writes, which build-order.md had already been corrected on. - "PR 2 is finished when the last skip is gone" was wrong in three places: shape 8's images: authoring names track C and outlives PR 2. The rule is PR 2's own skips. UPGRADING.md gains the entry the Ambiguous gate needs, including how to find affected targets before upgrading and the suspend-plus-status.placement dry run for checking the verdict without writing.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
CVE-2026-56854 is CRITICAL and fixable, so the image gate fails on it. It is not ours: Trivy reports our own manager binary clean, and all nine findings are in usr/local/bin/sops, which getsops builds against golang.org/x/crypto v0.54.0. This repository already requires v0.55.0, which carries the fix. The vulnerable path is x/crypto/ssh's SERVER-side handling of the authorized_keys `from=` criterion. SOPS is a CLI we exec to encrypt and decrypt with age; it listens on nothing and accepts no SSH connection, so that path cannot be entered in this image. Suppressed rather than fixed because v3.13.3 IS the latest SOPS release — there is nothing to bump to, and building SOPS ourselves would trade an upstream signed release for one of ours, which is a larger supply-chain change than the finding warrants. The entry expires 2026-12-01 so it is revisited even if nobody is watching upstream. Three things checked against a real Trivy 0.70.0 (the version CI pins) and the real SOPS binary, rather than assumed: - Trivy does NOT read .trivyignore.yaml from the working directory on its own, so both gates now pass it explicitly. Without that the file would have been inert and the gate would have stayed red. - The entry is scoped by path: the same binary at a different path still fails the gate, so this is not a blanket ignore. - The expiry is enforced: backdating expired_at makes the gate fail again. It is wired onto the GATES only, never the report steps, so the finding stays visible in the scan output and in the SARIF uploaded to code scanning — it does not block, and it does not disappear. The release scan job had no checkout, so the ignore file would have been absent there and the gate would have silently stopped honouring it (a missing ignore file is not an error). It checks out now.
…age` Trivy was the only gate that existed solely as a GitHub Action, while sixteen other tools — golangci-lint, actionlint, hadolint, vale, cosign, oras — ship in the container behind a task. That inconsistency had a cost I paid an hour ago: verifying a security suppression meant hand-downloading Trivy to a scratch directory, because there was no way to run the gate locally. It now installs like the rest (pinned TRIVY_VERSION, checksum-verified, one line in the version block) and runs as `task scan-image`, which both scan jobs invoke inside the CI container the way lint-helm and the e2e shards already do. The aquasecurity/trivy-action is gone from the workflow entirely. The task is the single definition of what blocks. The severities, --ignore-unfixed, --ignorefile and the exit codes were duplicated across four workflow steps in two jobs: four places to keep in agreement, none of them runnable before pushing. A maintainer reproduces the exact gate with: task scan-image SCAN_ARCHIVE=project-image.tar Two behaviours improve rather than merely move: - The project image is scanned straight from its artifact tarball (--input), so that job no longer needs to `docker load` it at all. - Trivy treats a missing --ignorefile as FATAL, where the action silently continued. The gate can no longer quietly stop honouring its own suppressions — which is exactly the failure the release job was one missing checkout away from. The release job keeps its SARIF upload and now runs it with `if: always()`: the report pass writes the file before the gate runs, so a blocking finding is still reported to code scanning instead of only appearing in a failed job's log. A cache step replaces the DB caching the action did for us; without it Trivy re-downloads its whole vulnerability DB per job, which is slow and rate-limited at the source. Checked rather than assumed, using Trivy 0.70.0 and an image built to carry the real CVE: the Dockerfile's install commands (URL, checksum, extract, --version), the task's precondition, both report modes (table, and SARIF that parses), that the gate fails when the CVE is not in the ignore file, and that it passes when it is. The actions/cache pin was resolved against the API rather than recalled.
…nt the task Two review follow-ups. Trivy pulls the image with its own registry client rather than through the docker daemon, so the runner's `docker login` never reached it once the scan moved into the container: the credentials live in the runner's home and the container sees only the workspace. Nothing is broken today — the package is public, as the login step's own comment says — but that is an implicit dependency, and it is a capability the scan HAD before this refactor, since trivy-action ran on the runner where the config already existed. The config is mounted read-only from outside the workspace, so an artifact upload can never pick the credential up, and addressed through DOCKER_CONFIG rather than a mount onto ~/.docker so it does not care which user the image runs as. `task scan-image` is now in docs/tasks-overview.md, the human-facing catalogue, rather than only in the agent-facing AGENTS.md. It is documented as deliberately outside `task lint` and outside that DAG: it needs an image to scan, which a lint run has no reason to build. Both places now also say that a devcontainer built before trivy was added needs rebuilding — the first person to hit "command not found" should not have to work that out. Deliberately NOT changed: the SARIF upload keeps a bare `if: always()` rather than gaining a file-exists guard. If the report pass fails, the scan step has already failed the job, so the guard would only convert a loud second error into a silent skip — and this repository would rather fail loudly than quietly not upload.
Asked more than once, so it is worth two lines next to the install: the vulnerability DB unpacks to 1.3GB, goes stale in 24h, and ten CI jobs pull this image while only two of them scan. CI caches it instead.
…ady says status.placement carried three things a reader could get from the GitTarget's own spec in the same GET: serializeNamespace copied spec.serializeNamespace, byTypeEntries counted spec.placement.byType, and examples resolved a fabricated object at a fabricated path -- which for a declared type is the byType template with its variables filled in, the one rung a user cannot get wrong because they wrote it. A status field that restates the spec is a second place to look that can disagree with the first. They are replaced by the fact none of them carried: how this folder is written. status.placement.mode is Plain, KustomizeRoot or KustomizeOverlay, and it is what predicts the behaviour that surprises people -- whether a new file is registered in a resources: list, whether a delete also drops that entry, and whether deleting an object the folder inherits authors a $patch: delete instead of removing anything. readOnlyBases names the directories a WriteBoundaryRefused will fire on, which is the only genuinely per-folder half of that. Mode is derived from the same predicate the write path uses -- a root plus a kustomization outside the write jail -- so the reported shape and the taken one cannot drift. observedRevision/observedTime become resolvedAtRevision/resolvedAt. They were always the resolution's date rather than the last scan's, since an unchanged resolution is not republished, and the old names invited reading a stable folder as a stalled scanner. The rule these were held to, recorded on the type so a future field is held to it too: a status field earns its place only if a reader cannot get it from the spec, and it varies with this folder. The write behaviours that follow from mode are constants of the mode, so they are documented on the field rather than enumerated per folder. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… suspend a stop A suspended target's resync returns before the mark-and-sweep, so nothing is swept and nothing is counted -- but the drain still reported zero retained documents, which reads as "converged" when it means "not measured". The stanza now goes absent instead, which is what absent already means everywhere else: no resync has reported. The field's documentation is reframed to match what it is. spec.suspend is a panic knob: one field that stops a target writing without deleting it or unpicking the WatchRule objects that would have to be rebuilt afterwards. That justifies it on its own. It was additionally sold as a dry run, which it cannot be -- a target that writes nothing has nothing to show, and pointing a GitTarget at a scratch branch previews the real bytes, the real registrations and the real deletes in a diff a reviewer can read. The scan is still not suspended with the write, for a different reason than before: a valve that stopped looking as well as writing would freeze status.placement at whatever the folder looked like the moment someone panicked, which is exactly when a stale answer costs the most. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…end the examples The post-scan pass was to have a second rule: serializeNamespace: false reports Validated=False when no kustomization in the folder supplies the namespace. It is withdrawn, because it fires on the correct configuration. For a raw namespace-free folder the supplier is a Flux targetNamespace or an Argo destination.namespace in a different cluster, and being unbound that way is the point of the shape -- two deployers may land the same folder in two namespaces, both correctly. There is no fact for the operator to check. So the scenario and its two fixtures go, and with them the third assertion mode the corpus harness would have needed for a check that reports rather than refuses. The rule that stays is the one on the inside of the folder: an explicit serializeNamespace: false admits one source namespace, because two namespace-free documents with the same name are a loss the operator can see in the folder it owns. The worked examples also stop setting spec.suspend. An example exists to show what gets written, and previewing that is a scratch branch rather than a suspended target -- so suspend: comes off the configs and off the harness's unbuilt-field filter, where it no longer belongs now that the field ships. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three changes to the layout pages, and one editorial rule applied throughout. Previewing a target is pointing one at a scratch branch and reading the commits it makes: real bytes, real resources: registrations, real $patch: delete files, in a diff a reviewer can read. It costs one field value and needs nothing built, so status.placement is not a preview and spec.suspend is not a dry run. model.md gains that section; build-order.md, gittarget-api-wave.md, configuration.md and the status-conditions guide drop the framing that said otherwise. status.placement is documented as what it is -- the answer to "why did that write take that shape, or why was it refused" -- with the mode table that separates a plain folder from a kustomize root from an overlay, including what each one does on a delete. That table is the raw-versus-kustomize difference stated once, in model.md, shapes/README.md and configuration.md. serializeNamespace: false is documented as unguarded, and shapes 2 and 4 say why in their own terms: the supplier is in another cluster and there may not be a single one, so there is nothing to name and nothing to verify. The rule: these pages state the design and its reasons, not the route taken to it. The withdrawn drafts are described where a reader would otherwise re-propose them, and nowhere else -- so build-order.md's claim that the post-scan pass splits across two PRs is corrected rather than annotated, and the answered open question is deleted rather than struck through. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bootstrap staging ran for every event in a window before the suspend gate, so a suspended target's path got .gittargetignore (and .sops.yaml) written into it and added to the index. The index belongs to the branch rather than to one target, so the next ACTIVE target's commit carried those files along: a target configured to write nothing appeared in history with two files in its folder, on a commit it did not author. Staging moves inside the per-target loop, after the suspend check, which is the ordering the resync path already had. It still runs before any resource write into that path, because it is now the first thing the non-suspended branch does. The regression test asserts both halves — nothing on disk and nothing in the commit — and asserts the active target in the same window still gets both files, so it cannot pass by bootstrap staging having quietly stopped working. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The refusal's one actionable instruction is "point the GitTarget at one of them", which is a GitTarget edit — and ActorPlatformOperator is defined as the person who owns the GitTarget's scope and path. It was classified ActorRepositoryAuthor, which sends that instruction to someone who does not own the object it names. The folder itself is a perfectly good base-plus-overlays tree; nothing in the repository is wrong. IssueWriteEscapesScope, the closest neighbour, was already platform-operator. The Actor doc comment enumerated the two sites that can reach the platform operator and now names three. The structure-only guarantee is unaffected and still holds for a structural reason rather than by luck: AmbiguousLayoutRefusal has one caller, in the flush planner, so a scan resolves the layout but never refuses on it. TestStructureOnlyScanNeverNamesThePlatformOperator still passes. Co-Authored-By: Claude Opus 5 <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.
PR 1 of the layout queue in
docs/design/build-order.md. Stacked on #325 — base isdocs-layout-two-booleans, notmain.Placement only ever affects documents that do not exist yet, so a
GitTargetpointed at a realrepository gives you nothing to inspect until it has already written. This PR is the dry run that
closes that gap, plus the corpus that proves it behaves as documented.
What lands
spec.suspendstops the planning of new writes and nothing before it. A suspended target keepsits watches, keeps scanning, and keeps publishing what it resolved;
ReadystaysTruewith reasonSuspended, because not writing on request is a configured outcome rather than a fault. It takeseffect at the next planning boundary: work already committed locally is still pushed, which is
stated in the API doc rather than left to be discovered — a local commit that is never pushed would
sit in the worker's checkout and surface later, out of order, on resume.
reconcile.configbutler.ai/requestedAtre-reads the folder now instead of on the periodiccadence, so a dry run is something you can iterate with. It needs its own
For()predicate, sincean annotation edit does not bump
metadata.generation.status.placementandLayoutResolvedreport what the last scan resolved: the render rootgoverning new documents, whether they carry their own namespace, how many
byTypetemplates aredeclared, when and at what revision it was observed, and up to three illustrative destinations.
Published ahead of every gate, so a target held unready still shows what its folder resolved to —
nothing in the stanza depends on a placement having happened.
The post-scan pass's
Ambiguousrule. AGitTargetcovering more than one kustomize renderroot refuses to place new documents instead of putting them at a canonical path inside whichever
folder it covered, where no render root would reach them.
The corpus. The eighteen fixture folders under
docs/layout/were read by nothing but a human.Fifteen scenarios now run through the real plan-then-flush path: nine pass, six are skipped naming
the track that unskips them.
-updateregenerates the patches.The one behavior change, and where it gates
Ambiguousgates, so this PR is not purely additive in behavior. It refuses at the write, noton
Validated: that condition is evaluated before the data plane exists, so a target failing itwould never register a worker, never scan, and could never observe the folder being fixed — and one
that had never scanned could never trip the rule at all. Refusing in
createNewkeeps the targetdeclared and scanning, so narrowing it to a leaf clears the refusal. Existing documents are
untouched either way; they are edited where they live.
docs/UPGRADING.mdcarries the entry, including how to find affected targets before upgrading.docs/design/build-order.mdis corrected rather than extended: it asserted three times that nothingin PR 1 changes what the operator writes, and used that as the argument for merging four parts into
one review. It now names the exception.
Two things the fixtures got wrong, found by executing them
repository/used two conventions — six folders rooted atspec.path, five at the repositoryroot — and swapping either broke that folder's expected patch. All are repository-root now, the
only reading shapes 6 to 8 can use.
produces, and a committed Argo CD
Applicationwhosespeckeys were not in the order the writeremits. They are regenerated from the observed diff.
Compatibility
Additive schema: two optional fields, three priority-1 printer columns, no migration. The behavioral
change is the
Ambiguousrefusal above.Verification
task lintandtask testpass locally. Localtask test-e2ereached 84 passed / 0 failed beforethe last round of review fixes; a re-run at HEAD was still in flight when this was opened, so CI is
the authority here — it also covers the Helm install path and the shards the local label filter
excludes.