Repository navigation
fix(render): prune what the composition no longer declares - #1295
Merged
Merged
Conversation
antoinetoussaint-byte
force-pushed
the
fix/render-prunes-undeclared-modules
branch
from
November 4, 2026 05:10
e8a82e4 to
cd3c9b2
Compare
The cell file was derived from the directory listing of deployments/modules/, not from the composition: a module removed from `modules:` left its render record behind and kept its namespace, workloads and consumer edges in every cell file derived after it, its delivered tree stayed in the delivery repository with the ApplicationSet that kept stamping Applications the cell refused, and once the file was right every publish refused until someone named the module back with --allow-cell-removal. The render now derives the cell file from the composition, deletes the records of modules it no longer declares once the file is written, and prints each removal. A removal the composition explains passes the hold without the flag and is reported as such; the flag keeps confirming every other removal. A forward publication to a hosted environment removes the delivered tree of such a module with everything else it writes and shows it in the plan; a rollback removes none. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…aw the base Two findings of the independent review of the first commit. A publish continuing or replacing the module's own promotion branch refused the removal that branch already carried as an unrelated change, because the removed trees were owned only when the clone still held them; the branch guard now owns the delivered tree of any module the composition does not declare by rule. And a stale checkout that never had a module derived the same cell file as one that dropped it, so it could withdraw a live module with no flag; the hold and the pruning now require the checkout to descend from the revision the base's cell file came from — the delivery base commit itself when the workspace repository holds it, else the source revision the file names — and say which revision it lacks when it does not, where naming the module confirms the removal as before. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… advanced The base's cell file names the render's parent as its source, and a `modules:` edit rides in the render's own commit, so a checkout of the one repository that had not fetched the base commit sat at that parent, descended from it, and was judged to have seen a composition it never had. The delivery clone holding the workspace revision the file names is what tells one repository from two: then only the base commit counts, and a checkout that lacks it is held as before, told which commit it lacks. The source revision is judged only for separate repositories. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
#1290 made the derivation refuse a tree of a module the workspace does not compose and ask for its removal. The prune now runs before the derivation, so a render satisfies that hold by removing the tree rather than skipping it; the hold stays for any derivation nothing pruned first, and its test asserts it there. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the module With a separate delivery repository the base's cell file names the render's parent, and a render may run on an uncommitted `modules:` edit, so a checkout at that parent descended from it without ever having composed the module the render named, and could withdraw it on the composition's word. The judged revision's own committed configuration must now have declared the module; a composition rendered before it was committed, or a module of an imported workspace, vouches for nothing and the removal stays the flag's to confirm, with the refusal saying so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
antoinetoussaint-byte
force-pushed
the
fix/render-prunes-undeclared-modules
branch
from
November 4, 2026 05:17
cd3c9b2 to
2e96908
Compare
The revision is read from the delivery branch's cell file and spliced into git arguments; anything but a full hash is treated as no source. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This was referenced Nov 4, 2026
Closed
Closed
Closed
Merged
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
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.
Problem
platform-obin removed the
proposalssolution frommodules:(platform-obin#505). Aftercodefly deploy gitops render --all --env blue:deployments/modules/proposals/stayed, and so didcompute's, which no composition declares;deployments/environments/blue/cells/blue/cell.yamlkept their namespaces, workloads and consumer edges, becausederiveCellderived the file from the directory listing ofdeployments/modules/, not from the composition;deployments/environments/blue/modules/{proposals,compute}/**stayed in the delivery repository, and with them the ApplicationSet each tree carries, which kept stamping Argo Applications whose every pod the cell (once correct) refuses;--allow-cell-removal (proposals)— asking the operator to type back the name of a module the workspace no longer composes.Change
Render derives the cell file from the composition and prunes what it no longer declares. Before deriving the file,
RenderCelldeletes every render record underdeployments/modules/whose module is not inworkspace.Modules, for any environment (pruneUndeclaredRecords), reports them (CellResult.Removed), and the render prints one line per record:removed deployments/modules/proposals: the composition no longer declares it. Every render that re-derives the cell file prunes — a single module's and--allalike — because that is where the claim about the cell is made. An environment with nohost:has no cell file and nothing is pruned.A removal the composition explains needs no naming — from a checkout that has seen the base's composition. A checkout that never had a module and one that dropped it derive the same cell file, so
holdCellToBasejudges with acompositionClaim: the composition's module set, and whether the checkout's HEAD descends from the revision the base's cell file came from — the delivery base commit itself whenever the workspace repository and the delivery repository are one (the workspace holds the base commit, or the delivery clone holds the workspace revision the file names assource.revision: platform-obin is its own delivery repository), because that source revision is the render's parent and amodules:edit rides in the render's own commit, so an unfetched checkout at the parent fails closed; thesource.revisionitself only when the repositories are separate — and then the workspace's own configuration as committed at that revision (git show <rev>:workspace.codefly.yaml,modules:/solutions:, or the workspace itself when flat) must have declared the module (declaredAtRevision), because a render may run on an uncommittedmodules:edit and a module of an imported workspace is another repository's history. The claim is therefore "the checkout descends from a composition that named M and no longer names it", in every topology. From such a checkout a removal whose module is no longer declared is let through without--allow-cell-removaland reported separately (PublishPlan.UndeclaredCellRemovals, printed asThe cell file removes <what>: the composition no longer declares module <m>). From any other checkout — a branch behind the base, a base file naming no source — the hold refuses as before, says which revision the checkout lacks, and naming the module confirms the removal as before (without pruning the tree). The flag keeps its contract for everything else — a module still composed whose render is missing from the checkout, a dependency dropped, a service withdrawn.Publish removes the delivered tree. A forward publication to a hosted environment, under an advanced claim, removes from the promotion clone every
<gitops path>/modules/<name>that carries a render record and whose module the composition no longer declares (removeUndeclaredDeliveries), stages those paths with everything else it writes, and shows them in the plan (PublishPlan.RemovedModules, printed asRemoved <path> from the delivery: the composition no longer declares it). The branch guard owns such trees by rule (compositionClaim.owns), not by what the clone still holds: the branch a publish continues, or the stale tip it replaces, carries this module's own earlier removal, and the base may since have taken it. A rollback removes none; a directory under the modules path with no render record is not a delivery and is left alone.Flag help,
docs/commands.mdand the generateddocs/cli-reference.mdmove with it.Classification
Fix, at the place that owns the behaviour: the cell file is the render's claim about the composition, so the render is what derives it from the composition and discards what contradicts it; the publication is what carries the composition's claim into the delivery repository, so it is what removes the delivered tree. No new mechanism: the hold (
holdCellToBase), the owned/published path sets (publicationCellPaths), the staging (stagePublication) and the plan printer are reused; the prune is a branch in the one derivation loop.Overlap
#1290 (the admission-check work) landed while this was in review, with
refuseUncomposedRender: the derivation refuses a tree of a module the workspace does not compose and asks for its removal. That check is untouched. This PR's prune now runs inRenderCellbeforederiveCell, so a render satisfies the hold by doing what it asks — removing the tree — rather than bypassing it;deriveCellstill refuses such a tree for any caller that does not prune first. #1290's testTestCellFileRefusesATreeOfAModuleTheWorkspaceDoesNotComposehad to move with the contract: it asserts the refusal onderiveCelland the prune onRenderCell(same fixture, same message). The one lint finding left on the touched packages (gocritic rangeValCopyinenvironment_declaration.go:36) is #1290's and pre-existing on main.Tests
Three new tests, each failing with the behaviour toggled off and passing with it:
TestRenderCellPrunesTheRecordOfAModuleTheCompositionNoLongerDeclares(pkg/gitops/cell_test.go): records forproposals(this environment) andcompute(another) render to a cell file with neither, both records deleted and reported, the composed modules' records intact.TestARemovalTheCompositionExplainsNeedsNoNaming(pkg/gitops/cell_removals_test.go): a third module leavesmodules:; the render prunes its record, and the hold passes with no flag, reporting the namespace and the consumer edge as removals the composition explains. The existing test still requires the flag for a composed module whose render is missing.TestPublishRemovesADeliveredModuleTheCompositionNoLongerDeclares(pkg/gitops/publish_cell_test.go): a deliveredlegacytree on the base is removed by a forward publish from a checkout at the base file's source revision — in the plan'sRemovedModules, its files amongChanged, absent from the published commit — while a directory with no render record under the modules path is left alone.TestAPromotionOwnsTheRemovalItsOwnBranchCarries: a second publish on the module's own promotion branch, whose earlier commit removedlegacy, is not refused as an unrelated change and removes nothing again.TestAStaleCheckoutStillNeedsTheFlagToWithdrawAModule: a base file rendered from a revision the checkout does not contain, naming a module the checkout never had, is refused with--allow-cell-removal (legacy)and the reason; named, it passes without pruning the delivered tree.TestACompositionThatNeverDeclaredTheModuleDoesNotRemoveIt: separate delivery repository, base file naming the checkout's own HEAD as source (a render on an uncommitted edit that namedlegacy), the committed composition never declaredlegacy→ refused, naming the revision whose configuration did not declare it; named, passes without pruning.TestAnUnfetchedCheckoutOfTheDeliveryRepositoryStillNeedsTheFlag: the delivery remote holds the workspace's history (one repository); a base file naming the checkout's own HEAD as its source, with a module the checkout never had, is refused and, named, passes without pruning.TestTheCompositionClaimIsAdvancedOnlyFromACheckoutThatSawTheBase(pkg/gitops/cell_removals_test.go): the ancestry judgement — the base commit whenever the repositories are one (the workspace holds it, or the delivery clone holds the source revision: a checkout at the render's parent is not advanced, fetched or not), the named source revision for separate repositories, nothing for a base with no file or no source, an orphan branch never.go test ./pkg/gitops/... ./cmd/deploy/... ./cmd,golangci-lint run ./pkg/gitops/... ./cmd/deploy/...(0 issues),gofmt -lclean,TestCLIReferenceIsCurrentregenerated.Review
An independent fresh-context review of
ef45a13ffound two blockers, fixed in the second commit: (1) the branch guard refused a publish continuing or replacing the module's own promotion branch that had removed a tree, because ownership was derived from the clone's working tree — now a rule (compositionClaim.owns); (2) a stale checkout that never had a module could withdraw it with no flag — now the hold and the pruning require the checkout to descend from the base file's revision. Its re-review ofe8a82e4afound one more: the source-revision fallback was fooled by an unfetched checkout of the one repository, at the render's parent — fixed in the third commit by letting the delivery clone's holding that revision tell one repository from two, in which case only the base commit counts. Its round 3 (cd3c9b2e) found the same hole for two repositories through an uncommittedmodules:edit at render time — fixed in the fifth commit by requiring the judged revision's own committed configuration to have declared the module.Not verified
No real Blue render or publish was run against platform-obin; the fixture trees stand in for it. A module declared only through an imported workspace (
workspaces:) is never explained by the composition — its removal keeps needing the flag — which is conservative, not wrong. I would file (not opened, per instruction): the render could record the workspace commit that carries it rather than its parent, which would make the two-repository judgement as exact as the one-repository one without reading the committed configuration — a cell-file schema change its consumer (infra-base's pin) would read.🤖 Generated with Claude Code