diff --git a/docs/roadmap/audit/10-equivalence.md b/docs/roadmap/audit/10-equivalence.md new file mode 100644 index 0000000..121612b --- /dev/null +++ b/docs/roadmap/audit/10-equivalence.md @@ -0,0 +1,84 @@ +# Behavioral equivalence and shared abstraction audit + +Reviewed baseline: `5e6db4d` on `main`. This review started with tests of +corresponding behavior and traced the implementation behind each candidate. +It considered the existing facade and package boundaries, including the +[architecture policy](../../policy/architecture.md), and retained deliberate +differences between operations. No production code or tests were modified. + +The offline tests for `internal/candidate`, `internal/git`, `internal/delivery`, +and `cmd/releasekit/internal/cli` passed using the Go test cache. Focused public +tests of build source eligibility, inspection errors, and dirty tagged-source +delivery also passed. + +## 1. Source-state observations are independently implemented + +**Test evidence:** +[TestCheckRejectsDirtyTrackedAndUntrackedState and TestCheckRechecksHeadAndWorktreeAfterCallback](../../../internal/candidate/check_test.go) +exercise clean worktree/index and stable-HEAD requirements for candidate +checking. [TestBuildSourceEligibilitySurvivesLaterCheckoutCleanup](../../../build_test.go) +checks the same kinds of changes before and during a build, while +[TestDeliverRejectsDirtyTaggedSourceBeforeBuildOrHostedWrites](../../../delivery_test.go) +and [TestInspectTaggedSourceReportsDirtyCheckoutAndReadCommitFile](../../../internal/git/source_test.go) +exercise dirty-state observations for tagged delivery. These tests imply a +common need for accurate Git HEAD and repository-wide status observations; +they do not imply identical decisions about dirty source. + +**Implementation:** [candidate.inspectLocalState](../../../internal/candidate/check.go) +runs branch, HEAD and porcelain-status commands. [build.gitCommit and cleanSource](../../../internal/build/build.go) +independently run HEAD and status commands through direct subprocesses. +[git.CaptureHead and InspectTaggedSource](../../../internal/git/source.go) +perform overlapping observations through the Git package's command boundary. + +**Opportunity:** The existing Git package could own small shared commit and +status observation primitives, leaving each operation to choose when to read +them and how to interpret them. This would centralize a concrete Git +responsibility and reduce the number of paths needing parallel fixes for +inspection, error propagation, and cancellation. The +[complexity audit](03-complexity.md#2-git-source-inspection-is-repeated-across-package-boundaries) +also identifies the duplication; the tests above establish the common +behavioral evidence for considering it here. + +**Tradeoffs:** Candidate checking rejects dirty state and checks the branch; +builds can succeed with dirty state but record source eligibility before and +after compilation; delivery additionally checks a specific tag. Do not merge +those policies or alter observation timing. Retain separate caller-level +tests, so a shared helper cannot make every test pass while all consumers +interpret its results incorrectly. Coordinate with the existing +[replacement-object and cancellation findings](01-correctness.md). + +## 2. Build-artifact declarations are checked independently by receipts and delivery + +**Test evidence:** +[TestBuildReceiptRoundTripBindsRootAndNestedModuleIdentity](../../../cmd/releasekit/internal/cli/receipt_test.go) +constructs and reloads a receipt binding a selected target to its expected +filename. [TestDeclaredBuildAssetsGenerateManifestFromBuildResult and TestDeclaredBuildAssetsRejectsArtifactDeclarationMismatch](../../../internal/delivery/assets_test.go) +exercise the corresponding build-result declarations and reject a mismatched +filename. [TestBuildAndPrebuiltDeliveryUseSameDirectoryBase](../../../cmd/releasekit/assets_test.go) +connects the two paths through a real build, receipt, and prebuilt delivery. +Together they support a shared requirement that a target's recorded artifact +identity agree with the selected build, without requiring receipt loading to +read artifact contents. + +**Implementation:** Both +[validateBuildReceipt](../../../cmd/releasekit/internal/cli/receipt.go) and +[declaredBuildAssets](../../../internal/delivery/assets.go) independently +check target uniqueness, artifact counts, one artifact per target, and names +derived from targets. They already share `artifact.ExpectedBuild` and +`artifact.ValidComponent`, but each builds and checks its own target-to-name +mapping. + +**Opportunity:** Consider a narrow, pure declaration-matching helper in the +existing [artifact package](../../../internal/artifact/artifact.go), accepting +target/filename metadata and reporting a mismatch without opening files. Both +paths could reuse it after converting their own types. Centralizing the shared +identity rule would reduce drift when target or filename conventions evolve. + +**Tradeoffs:** The receipt additionally requires a schema, specific output +path and digest syntax, and intentionally loads without checking the files; +delivery accepts source-only results and separately stats and verifies local +content after preflight. Keep those boundary-specific rules and ordering where +they are. Avoid making `internal/artifact` depend on CLI receipt types, +`internal/build`, or forge policy. Preserve independent public and CLI tests +with concrete expected filenames and bytes rather than using the new helper to +compute every test expectation.