25 KiB
Audit remediation implementation plan
Baseline and execution
This plan targets 59bb074 on 2026-11-01. The worktree was clean when planning
began. The production code still contains the findings recorded in
correctness.md and interfaces.md.
remediation.md defines the agreed behavior and scope; this
plan defines how to reach it.
Luna should execute Stage 1 through Stage 10 in order in one end-to-end sprint. The stage boundaries divide implementation prompts. Intermediate stages may leave dependent code or tests incomplete; compilation at every boundary is not a requirement. Record deferred checks and carry them forward. Stage 10 requires the complete, coherent result. There are no intermediate release, live-service, operator-action, or human-review gates.
Read development.md and the applicable policies before work. Use the repository's code graph for code discovery, with targeted source reads where the graph is incomplete; the build package may be excluded from its index. Follow the standing commit policy for completed reviewable units. Update stage status and evidence here as work progresses; a deferred regression check remains outstanding until it has run successfully.
Implementation decisions
- Keep both public facades, public signatures, receipt schema, and package dependency direction. New helpers belong under the existing internal owners.
- Resolve a checkout root automatically for preparation; retain standalone supplied-directory preparation. Git discovery supplies the root, while the existing selection resolver supplies relative-path, symlink, and containment behavior.
- Override only alternate module-file selection with the explicit argument
-modfile=on dependency downloads. Preserve other effective Go settings. Local Go 1.26.10 probes verified precedence over inherited and persistentGOFLAGSwithout preventing normal upward module discovery. - Represent a peeled-only remote ref as
RefConflictafter parsing the whole response. Existing publication reconciliation already maps that inconsistent observation toPublicationUnknown. - Classify a Gitea handle of the expected private type with a nonpositive ID as
forge.FailureMalformedat asset use. Preserve minimal page-only lookup fields and wrong-type handle classification. - Keep empty-prebuilt identity checking and the existing preflight-first order. Their remediation is documentation, rather than a new request interpretation.
These decisions cover the reported cases. Preserve existing recovery bounds, write evidence, source observations, and preflight evidence limits. No new public options, directory-name policy, response-field requirements, or build reproducibility guarantees are part of this work.
Coverage map
| Finding | Implementation stages |
|---|---|
| Correctness 3: incomplete remote tag reconciliation | 1 |
| Interfaces 3: preparation path anchoring | 2, 3, 5 |
| Correctness 2: ambient module-file redirection | 4, 5 |
| Correctness 1: invalid derived candidate tag | 6 |
| Interfaces 1: source-inspection cancellation | 7 |
| Interfaces 2: incomplete Gitea release handle | 8 |
| Interfaces 4: empty-prebuilt documentation | 9 |
| Interfaces 5: documented hashing order | 9 |
| Complete integration and evidence | 10 |
Stage 1: Finalize remote ref observations and verify recovery
Status: complete. The Git query, public publication, and CLI publication
regressions pass with the offline environment; the root TestPublish tests and
the full internal/git package also pass.
Files: internal/git/remote.go, internal/git/tag.go, their tests,
publish_test.go, and a focused CLI publication test under cmd/releasekit/.
- Add a small ref-finalization helper beside
mergeRef. After parsing a whole response, aRefPresentvalue with an emptyObjectand a populatedPeeledbecomesRefConflict. Retain its available fields for diagnostics. - Apply finalization to the tag observation returned by both
Endpoint.QueryandEndpoint.QueryTag. Do not finalize after each record: a peeled record followed by its exact object is valid. Preserve existing duplicate-record and malformed-line handling. - Keep
internal/publish.reconcilePush's existing treatment ofRefConflict: unknown outcome, reconciliation phase, and the original push error retained. Its existing path should suffice once the observation is classified correctly; improve a diagnostic only if necessary to explain incomplete evidence. - Extend the existing local Git wrapper/fixture in
publish_test.go, starting withTestPublishUninterpretableReconciliationRemainsUnknownandinstallGitWrapperWithMutationMode. Add a mode that performs the real push, returns a failed acknowledgment, and supplies only the selected peeled tag record on the recovery query. Keep endpoint URL inspection functional and alter only the recovery response, not the pre-publication guards. - Verify the public result is
PublicationUnknowninPublicationReconciliation, retainsTagObjectand the push cause, and leaves the local tag intact. Independently inspect the local bare remote to prove the intended exact object was actually published. - Exercise the CLI's
runpublication path with the same failure shape in a package-local fixture: status 3, unknown-state diagnostic, and no success message. Retain the current mapping; do not add a new exit category or shared test framework solely to share fixtures across packages.
Verification: exercise both query entry points and the following response shapes: empty response, exact object only, exact object plus peeled commit in both orders, peeled record only, and the already covered duplicate/conflicting records. Preserve successful recovery, confirmed absence, actual different-object conflict, and the existing single recovery query.
Focused checks: go test ./internal/git; the TestPublish tests at the root
and CLI publication tests. Use the offline environment specified in Stage 10.
Stage 2: Add optional checkout-root discovery with precise failure handling
Status: pending.
Files: internal/git/source.go or a focused companion file, Git tests, and
internal/process/process.go only if an existing invocation capability proves
insufficient. Its current stdout/stderr capture and environment overlay should
already suffice.
- Add an internal Git function such as
DiscoverRepositoryRoot(ctx, directory) (root string, found bool, err error). Success returns a canonical containing checkout root andfound=true. Confirmed standalone use returnsfound=falseand nil error. Operational failures return their wrapped cause. - Use Git's own
rev-parse --show-topleveldiscovery, preserving normal Git environment and worktree behavior. Capture stdout and stderr separately throughinternal/process. ApplyLC_ALL=Cto this child only so the known no-repository diagnostics can be recognized without depending on operator locale. Do not change process-global locale or configuration. - Git uses exit status 128 for both absence and other failures. Classify absence
only when that status accompanies its recognized no-parent-repository
diagnostic:
fatal: not a git repository (or any of the parent directories): .git, or the form beginningfatal: not a git repository (or any parent up to mount pointand ending), followed byStopping at filesystem boundary (GIT_DISCOVERY_ACROSS_FILESYSTEM not set).. Match the fixed text around the variable mount path. Keep the classifier small, private, and covered by diagnostic samples. Do not accept every message starting withnot a git repositoryas standalone: an invalid explicitGIT_DIRhas a different diagnostic. - Treat cancellation, inaccessible working directories, broken Git metadata,
ownership errors, bare-repository worktree errors, and unrecognized
diagnostics as errors. Preserve
errors.Is/errors.Ascauses and useful stderr. An arbitrary error is never permission to choose another base. - Reuse the existing successful-root normalization with
filepath.EvalSymlinks. Keep strictRepositoryRoot's current public-to-internal behavior: its callers still require a checkout. Share private mechanics where useful while retaining the original error cause for that strict operation. - Preserve the existing Go-only standalone use when the Git executable is
unavailable. Only for
exec.ErrNotFound, check the supplied physical directory and its ancestors for.gitentries with standard filesystem operations. With no such entry and no explicitGIT_DIRorGIT_WORK_TREE, return confirmed standalone use. If a marker or explicit Git context exists, return the missing-Git error; checkout discovery is unavailable. Propagate filesystem inspection failures instead of treating them as absence. This limited presence check establishes standalone use only; Git remains the authority for selecting an actual checkout root. Do not apply this fallback to the strictRepositoryRootfunction.
Verification: real temporary repositories for root/subdirectory discovery,
filesystem aliases, and an independent worktree with a .git file; real
standalone directories for absence. Use controlled Git-process failures for
permission/ownership errors and cancellation, rather than relying on host user
privileges. Test both no-repository diagnostic forms and a status-128 failure
that must remain an error. A broken .git file must not trigger standalone
fallback. Existing strict-root tests continue to pass.
Also cover a Go-only executable path in a marker-free standalone directory:
preparation can still run there. With that same executable path inside a
checkout, discovery fails with the missing-Git cause before Go runs.
Focused check: go test ./internal/git.
Stage 3: Apply checkout-root selection to dependency preparation
Status: pending. Depends on: Stage 2.
Files: preparation.go, preparation_test.go, and
docs/consumers/delivery.md's preparation guidance.
- Retain the existing rejection of an empty/blank
RepositoryDir. - Discover the optional checkout root from the supplied directory. If found,
pass that root and the request's unchanged
ModuleDirintoreleaseidentity.ResolveSelection. Otherwise pass the supplied directory andModuleDirinto that same resolver. Propagate discovery failures before invoking Go. - Continue calling
internal/preparation.PrepareBuildDependencieswith the selected absolute directory. Preserve the current selection resolver's handling of symlinks before parent traversal and its containment checks. - Leave CLI root normalization and
runCIDelivery's preflight/preparation order in place. They already supply the containing root; no new CLI flag or profile field is needed. - Update request/function GoDoc and the preparation guide: checkout-relative selection inside Git, supplied-base selection for standalone use, and the changed interpretation of calls from checkout subdirectories.
Verification: extend TestPrepareBuildDependenciesUsesSelectedModuleDirectory
and related public tests. Include checkout root and child-directory inputs with
empty and explicit nested ModuleDir, standalone selection, symlink/link/..
selection, and discovery failure before any Go invocation. Use the existing Go
recording shim to observe the chosen working directory. Keep the current
standalone tests, which intentionally have no Git repository; do not introduce
an immediate-go.mod requirement just to simplify fixtures. Stage 5 verifies
composition with ordinary build identity and ambient flags.
Focused checks: public TestPrepareBuildDependencies tests and existing CLI
ci-deliver preparation tests.
Stage 4: Override alternate module-file selection during downloads
Status: pending.
Files: internal/gotool/gotool.go, internal/gotool/gotool_test.go,
preparation.go GoDoc, and docs/internal/gotool.md.
- Change the download argument vector to
go mod download -modfile=. The explicit empty flag overrides the alternate module file selected throughGOFLAGSand restores ordinary discovery. - Keep the existing
GOWORK=offandGOTOOLCHAIN=localoverlays. Leave other effective flags, proxy/private-module/checksum/authentication/cache settings, and persistent Go configuration available to this network-capable operation. - Preserve combined diagnostics, wrapped command/cancellation errors, and
permission for the Go command to update its normal
go.sum. - Update
TestPrepareBuildDependenciesPreservesPreparationEnvironment's recording fixture for the intentional argument change and retain its environment assertions. Keep command failure and cancellation tests. - Document only this preparation-specific flag override. Build/validation environments and caller-controlled bootstrap commands keep their current responsibilities. Actual graph-selection regression evidence follows in Stage 5; argument recording alone is insufficient.
Focused check: go test ./internal/gotool.
Stage 5: Verify selected-module preparation and build composition
Status: pending. Depends on: Stages 3 and 4.
Files: preparation_test.go and small test-local helpers beside it. The
existing local module-proxy construction in cmd/releasekit/woodpecker_test.go
can inform fixture layout, but its package-local helpers cannot be imported.
- Create a real local repository with root and nested Go modules using distinct
synthetic dependency graphs, plus an alternate dependency-free module file.
Serve the synthetic dependency metadata and archives through a temporary
file proxy or
httptestserver. Use the standard library and a small fixture, not a new general-purpose testing package. - For each preparation attempt, supply a fresh test-owned
GOMODCACHEand fixture-scoped proxy/checksum/private-module settings. Ensure that a syntheticexample.invalidmodule cannot escape to a public proxy or VCS lookup. - Cover
GOFLAGS=-modfile=...from the inherited environment and from a temporaryGOENVfile with no process override. Do not treatGOFLAGS=""as sufficient isolation from persistent settings. The audit demonstrated that Go can still use the persistent value. - Call public
PrepareBuildDependenciesfrom checkout subdirectories for both default root selection and explicit nested selection. Prove that the chosen module's dependency was downloaded into its owned cache and that the alternate graph did not satisfy the request. Retain a local negative control showing the ordinary uncorrected Go command would select the alternate file. - Call public
Buildwith the sameRepositoryDirandModuleDir, using an empty target list, and compare its repository/module identity with the graph prepared above. This checks path composition without adding compiler or buildinfo dependencies to the synthetic proxy. Existing binary build and CI tests remain the owners of actual compilation/version-injection behavior. - Add a standalone package-subdirectory case proving Go still discovers its
parent declaration, and retain an unrelated effective flag such as
-xin a representative case to prove the fix did not replace all caller flags. Keep changed checksum files within the disposable fixture.
Verification: observed dependency metadata/cache files and public build identity are the assertions. Avoid only checking command counts or argument spelling. Inspect fixture subprocess settings to ensure every synthetic module consumption uses an owned cache.
Focused checks: the new root preparation tests and existing CLI binary
ci-deliver tests. Preserve their credential-before-download sequencing.
Stage 6: Reject invalid derived candidate tags before validation
Status: pending.
Files: internal/git/remote.go and its current tag-helper callers/tests,
internal/candidate/check.go, public candidate tests, and candidate guidance
only where its explanation needs adjustment.
- Make the existing internal Git
checkTagRefhelper available to the candidate package asgit.CheckTagRef, updating its internal callers and GoDoc. Retain the existinggit check-ref-format refs/tags/<tag>behavior. This is an internal-package export, not a new facade API. - In
candidate.Capture, validate the fully derived tag before accepting its absence throughhasLocalTag. Return a contextual local-candidate error while retaining the captured module/tag identity. A malformed derived ref is not anErrInvalidVersionsyntax failure and needs no new public error type. - Keep name validity separate from
show-ref's existence result. The captured tag is then immutable across ordinary rechecks; reuse the shared validator if another entry path needs it, without adding a separate character parser. - Keep working-tree identity derivation and standalone
Buildbehavior intact. Do not reject module directories globally or invent alternate tag encodings.
Verification: public Check rejects the reported module space and
module.lock cases before Go validation and additional/build callbacks. Keep
ordinary root/nested/major-version and slash-containing tags accepted when
otherwise valid. A Go marker shim can prove validation did not start. Existing
publication guard and local-tag conflict tests continue to cover those paths.
Focused checks: go test ./internal/git ./internal/candidate and the public
candidate regression tests.
Stage 7: Preserve build source-inspection cancellation causes
Status: pending.
Files: internal/build/build.go and focused public tests in build_test.go
or a companion root test file.
- In
cleanSource, when the Git command fails, inspectctx.Err()as neighboringgitCommitandcheckVersionalready do. Wrap the context error when present; otherwise retain the Git command failure. Keep the existing diagnostic context and partial result behavior. No new public error type is needed. - Reuse
newBuildConsumerand the existing cancellation fixture style. Add a Git wrapper that delegates ordinary operations and signals when the selected build-timestatusinvocation is entered before blocking. Use an explicit readiness signal to cancel; do not rely on a guessed startup sleep. - Cover cancellation in the initial inspection, which needs no compilation,
and in the final inspection after a host-target binary has been built and
smoke-checked. Verify
errors.Is(err, context.Canceled)and retention of completed artifact paths/digests for the latter case. - Add one focused deadline-expiration variant with synchronized readiness and
a bounded wait. Verify
context.DeadlineExceeded, rather than asserting an exact killed-process error string. Join test goroutines and reap children.
Verification: ordinary source-inspection Git errors retain their causes;
SourceVerified is not reported true after an interrupted build. Retain
TestBuildCancellationReturnsCompletedArtifacts and managed-delivery temporary
cleanup coverage. Add another delivery test only if a distinct propagation or
cleanup risk remains uncovered.
Focused checks: root build/cancellation tests and affected managed-delivery cancellation tests; run these with the race detector after they are coherent.
Stage 8: Classify missing Gitea release IDs at asset use
Status: pending.
Files: internal/gitea/backend.go, its tests, a focused root delivery test,
and docs/integrations/gitea.md.
- Split handle validation in
ListAssetsandUploadAsset: a failed privatereleaseHandletype assertion remainsforge.FailureInvalid; a correctly typed handle withid <= 0becomesforge.FailureMalformedwith a useful missing-release-identity diagnostic. A small private validation helper is sufficient if it avoids duplicating this distinction. - Perform these checks before making an asset HTTP request. For a locally
rejected upload, return
Attempted=falseandWriteNotAttemptedexactly as other local upload failures do. Do not pass a bad ID into the lower client and reclassify its invalid-request error afterward. - Keep
FindRelease's minimal read response andCreateRelease's existing positive-ID acknowledgment check. No extra lookup, retry, public failure type, handle-provenance framework, or mandatory page URL is needed. - Drive lookup responses with missing, zero, and negative IDs through the real backend into both asset operations. Assert malformed neutral failures and absence of asset requests. Test wrong-type handles and valid IDs as controls.
- Retain
TestBackendFindReleaseDoesNotRequireWriteResponseFields. Add a paired public delivery regression using the existing root fixture: the same matching existing page without an ID completes page-only delivery, while a request with a prebuilt artifact fails asset use withDeliveryNotAttemptedand no new hosted writes. Preserve existing page creation progress and write-recovery behavior.
Focused checks: go test ./internal/gitea ./internal/delivery and affected
root delivery tests. Describe the corrected observation classification in the
integration guide without strengthening page-only read requirements.
Stage 9: Align delivery documentation with existing behavior
Status: pending.
Files: delivery.go GoDoc and docs/consumers/delivery.md; review related
current descriptions for consistency. This stage changes documentation only.
- Change
DeliveryRequestGoDoc to distinguish a matching prebuilt result with no targets/artifacts from the zeroBuildResult. A supplied result still needs matching repository, module directory/path, tag, commit, and version. Binary-only source/artifact requirements remain conditional on binary work. - Present nil
Buildand nilPrebuiltas the ordinary page-only API use. Preserve matching-empty-result behavior, current identity validation, and mutual exclusion of managed and prebuilt sources. Add no compatibility shim. - In the prebuilt consumer paragraph, replace verification before every hosted request with verification before hosted writes. Describe the current order: cheap local/source checks, read-only preflight, full local verification, then hosted writes. Keep available-access-evidence limits clear.
- Review preparation wording delivered in Stages 3 and 4, plus current CLI and integration descriptions, for agreement. Update only canonical owners that actually need correction. Accepted ADR decision content remains intact.
Verification: compare documentation with validateBuildIdentity, automatic
preflight, and existing page-only/prebuilt tests. Reuse
TestAutomaticPreflightFailurePrecedesPrebuiltArtifactHashing. These wording
fixes require no new tests that merely repeat existing implementation behavior.
Validate affected local links/fragments and prose whitespace.
Stage 10: Verify the complete sprint and close the findings
Status: pending. Depends on: all preceding stages.
-
Resolve every deferred check, format changed Go files, and inspect the final diff for changes outside the agreed remediation scope. Verify all eight findings against the coverage map and target behavior in
remediation.md. -
Run the complete offline suite and compiled API/dependency-boundary check:
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test ./... GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go run ./internal/apicheck -
Run race-enabled tests for the changed Git/subprocess, Go preparation, Gitea/delivery, and public cancellation paths. A single
GOTOOLCHAIN=local GOPROXY=off GOSUMDB=off go test -race ./...is an acceptable combined check; targeted race runs are sufficient when they cover the affected paths. Repeat tests only when changes, failures, or unresolved concerns justify it. -
Confirm public types and receipt schema are unchanged; strict root lookup still requires a checkout; standalone preparation still works; default and explicit module selections agree across preparation/build; and publication uncertainty and write evidence retain their selected meanings.
-
Review fixture isolation, cancellation readiness/cleanup, useful error chains, and CLI status assertions. Keep synthetic module caches test-owned and live infrastructure out of default tests.
-
Validate changed Markdown links/fragments and run the documented whitespace checks. Review current consumer/GoDoc/integration owners together so the delivered behavior has one consistent description.
-
Mark verified stage statuses complete here and add completion markers with relevant test/commit evidence to the corresponding findings in
correctness.mdandinterfaces.md. Retain their audit evidence. Setremediation.md's status to complete only after every finding is addressed. -
Follow the repository's normal commit discipline for completed work and report the implementation changes, validation, remaining limitations, and worktree status. No tag, push, hosted release, or pilot is part of this plan.
Completion means the final repository implements the agreed target, the regressions and existing relevant checks pass, and every finding has verified completion evidence. No unresolved product or policy question remains from the planning review.