32 KiB
Remediation: module release implementation review
Status: open; issues 1–12 are complete, and issues 13–15 require remediation.
The initial review examined 90559fe, compared with planning baseline dbfae9e.
The follow-up repository audit examined 6b0b8ce, including those corrections.
Scope and evidence
The feature roadmap and implementation plan define the intended module-release architecture. Most of that architecture is implemented: the two public facades, shared module/tag derivation, scoped committed inputs with full reference lookup, module-based Go execution, full-tag publication and hosted delivery, current-only receipts, and tagged-source workflow selection.
The ordinary offline go test ./... suite and go run ./internal/apicheck
passed with Go 1.26.8. The findings below also use source inspection and temporary
fixtures outside the checkout. Publication reproduction used a private local
bare repository. No ReleaseKit source or real release refs were changed.
Each issue describes required work and verification. Preserve the selected scope, existing error causes, publication/delivery outcomes, facade boundaries, and clean API cutover. These corrections add no release, live-service, or human review gates. Each completion record describes the correction and verification.
Issue 1: Resolve the actual Git repository root before release identity (P1)
Status: complete. Category: architecture gap and incorrect publication.
Completion: Source operations and CLI profile selection resolve the containing Git checkout root before module selection. Regression tests verify that a child directory cannot bypass a failing root suite or publish its tag, explicit nested selection binds its own source and tag, aliases and independent worktrees retain their identity, and dependency preparation works without Git. The focused public API tests and candidate, delivery, CLI, and profile suites pass offline.
Evidence: candidate capture treats
filepath.Abs(request.RepositoryDir) as the repository root, and
selection resolution uses that
directory as its base. Git committed-file paths are relative to the actual Git
root. Go commands instead execute in the supplied directory.
A temporary repository had a failing root suite, a passing module at
apps/widget, and a root release note. Check from the actual root failed.
Check with RepositoryDir set to apps/widget and omitted module selection
succeeded, reporting the root module's declared path and unprefixed v0.1.0
tag while executing the nested suite. Publish with that request successfully
pushed the root tag to the private fixture remote. The declared release unit
and validated release unit therefore differ.
Required change: Resolve the containing Git checkout's actual root at source operation boundaries, then anchor repository-relative module and note paths to that root. Carry that resolved repository identity through checks, publication, builds, delivery, and CLI profile lookup. Normalize an input directory within the checkout rather than introducing a blanket prohibition on such inputs. Keep dependency preparation's own prerequisites separate; this correction should not require a Git checkout merely to prepare a Go module's dependencies.
Verification: Repeat the failing-root/passing-nested fixture through both
Check and Publish, including default CLI invocation from a nested directory.
Omitted selection must validate the actual root module and prevent publication
when that suite fails. Explicit nested selection must bind its declaration,
note, suite, and prefixed tag consistently. Retain symlinked checkout and
independent worktree behavior.
Issue 2: Preserve standalone Build error classification and partial evidence (P2)
Status: complete. Category: regression and incomplete failure evidence.
Completion: Standalone builds classify invalid versions before source access and retain resolved selection and normalized build settings on identity failures. Working-tree derivation preserves progressive identity evidence. Native CLI validation reports invalid-version status 2 before repository access. Focused API, native CLI, parser, identity, cancellation, and partial-artifact tests pass offline.
Evidence: public Build now calls working-tree identity
resolution before the internal builder. Invalid stable versions are returned by
Derive as ordinary formatted errors, bypassing the existing
ErrInvalidVersion identity. An API request with bad-version produced
errors.Is(err, releasekit.ErrInvalidVersion) == false; the compiled CLI exited
1 rather than its documented invalid-version status 2.
The same entry point returns only Version when identity resolution fails.
Selecting an existing nested directory with an invalid go.mod returned empty
repository and module fields, although both directories had already been
resolved. This contradicts the roadmap's progressive failure-evidence decision
and loses supplied build facts that the previous build boundary retained.
Required change: Preserve the existing invalid-version error at the build entry boundary, including its classification before unrelated filesystem work. Return available selection and build evidence when later declaration or identity work fails. Share the appropriate conversion/resolution facts without resolving the declaration again or inventing stronger source-verification guarantees.
Verification: Add API and native CLI cases for invalid versions, plus a declaration failure after successful directory selection. Assert error identity, exit classification, and available result fields. Keep cancellation causes and later partial-artifact behavior intact.
Issue 3: Use the module path base for relative prebuilt assets (P2)
Status: complete. Category: incomplete module path adoption.
Completion: Build output and CLI prebuilt assets share the selected module's directory resolver. CLI and pending migration guidance describe the common base. Local hosted regression tests verify root and nested relative build/delivery pairs, absolute output, module binding, and rejection of changed artifact content before hosted requests. Existing receipt/content checks and nested build cases pass offline.
Evidence: CLI prebuilt delivery joins a
relative --assets directory to invocation.Repository. Builds now resolve
relative --output from the selected module. A nested profile's successful
build --output dist v0.1.0 wrote
apps/widget/dist/.releasekit-build.json; the corresponding
deliver --assets dist v0.1.0 attempted to open the repository-root receipt and
failed. The documented build/deliver pair therefore uses incompatible bases.
Required change: Resolve relative prebuilt asset directories from the same selected module base as build output, using shared path normalization. Preserve absolute directories and repository-relative profile lookup. Document the base in the CLI owner and update affected examples and migration guidance.
Verification: Exercise a nested profile's relative build/output and delivery pair using a local hosted fixture. Include the omitted root selection and an absolute caller-owned asset directory. Verify module binding and content checks still happen before hosted requests.
Issue 4: Compare receipt repository identity consistently through aliases (P2)
Status: complete. Category: regression from mixed path normalization.
Completion: Receipt source and location comparisons use filesystem directory identity, accepting aliases of the same checkout or output directory. Regression tests build and deliver through a checkout symlink, retry through its physical path, and reject different repositories and modules. Receipt structure, exact release binding, and delivery-owned content checks remain intact. Receipt and assembled prebuilt-delivery cases pass offline.
Evidence: Release identity canonicalizes repository symlinks, so generated
receipts record the physical repository directory.
BuildReceipt.MatchesBuild only
applies filepath.Abs/Clean to the supplied repository. A fixture built normally
and delivered with --repo pointing to a symlink of that same checkout, with
an absolute asset directory, failed the preliminary repository/settings comparison.
The delivery facade's shared resolver would identify the same repository.
Required change: Use consistent resolved repository identity for receipt comparison, or leave source identity comparisons with the delivery owner that already resolves and verifies them. Preserve structural receipt checks and exact module/version/tag/commit binding. Avoid adding another declaration resolution or a requirement that callers use one textual spelling of a checkout path.
Verification: Build and deliver through the same symlinked checkout argument, and through its equivalent physical path. Both must identify the same source; a genuinely different repository or module must still fail before hosted work.
Issue 5: Keep Go environment reuse lazy and proportionate (P2)
Status: complete. Category: additional validation gates and unrequested restrictions.
Completion: Go discovery happens on acquisition for actual Go work. Completed callbacks trigger no additional discovery; settings-file snapshot failures disable reuse while preserving the Go tool's accepted configurations. Regression tests cover check/publication completion after Go leaves PATH, subsequent builds needing rediscovery, the final callback, unreadable GOENV snapshots, normal environment and persistent-file changes, and cancellation. Focused public cases and complete candidate and Go-tool suites pass offline.
Evidence: candidate coordination and
publication eagerly call
Owner.Refresh after callbacks. A candidate
with successful standard validation and a successful additional callback that
left Git available but removed Go from PATH failed during rediscovery, although
no subsequent Go command or build was selected. Acquire already detects changed
inputs before actual Go work.
The owner also requires its settings-file snapshot to be readable. With GOENV
set to a directory, existing gotool.New and the Go discovery commands succeeded,
while NewOwner(...).Acquire failed on the added os.ReadFile. Go's acceptance
of its configuration has become narrower through a cache bookkeeping step.
Required change: Rediscover changed settings when a subsequent Go command needs them. Preserve successful callback/operation completion when no more Go work remains. Make snapshot limitations disable safe reuse or otherwise follow the Go tool's effective behavior; they should not independently reject a configuration that discovery accepts. Retain operation-local ownership, normal settings-change detection, offline compilation, and separate preparation settings.
Verification: Cover a completed validation operation whose callback changes environment inputs with no later build, and one whose subsequent build actually needs rediscovery. Compare owner discovery with the underlying Go-tool behavior when persistent settings cannot be snapshotted. Keep existing settings-change and cancellation tests.
Issue 6: Avoid requiring downloadable module paths for ordinary main modules (P2)
Status: complete. Category: unrequested restriction.
Completion: Identity derivation uses Go import-path lexical validation and retains path-major compatibility. A valid non-domain main module now completes candidate checking, native standalone compilation, and hosted page-only delivery against a local fixture. Qualified and local major-version derivation, missing and malformed declarations, and incompatible majors pass their focused offline regression tests.
Evidence: Derive adds
module.CheckPath, including its requirement for a dot in the first path element.
A standard main module declared as module example passed go test ./... but
was rejected by standalone Build before any Go work. The
Go module reference permits main modules
that are not fetched as dependencies to use valid package paths.
The selected requirement is stable-version and path-major tag derivation. Deriving a root or physical-directory tag does not require the declaration to be a downloadable network module. No user decision imposed that extra restriction on executable applications or local builds.
Required change: Validate the declaration and path-major facts needed by identity derivation using Go's appropriate lexical rules for the supported operations. Preserve standard major-version consistency. Remove the accidental downloadability prerequisite rather than adding a consumer bypass setting.
Verification: Exercise a valid non-domain main module through standalone build and representative check/page delivery, alongside ordinary qualified module paths. Continue rejecting missing/invalid declarations and incompatible major identities for the selected release convention.
Issue 7: Resolve committed identity from committed material (P2)
Status: complete. Category: declaration-source divergence and added prerequisites.
Completion: Shared Git reference traversal now serves validation and committed module identity. Committed selection follows directory and declaration links using targeted object reads; working-tree builds retain filesystem selection. CLI mapping and prebuilt path selection use committed directory facts. Regression tests cover captured links, changed working declarations, cycles, checkout boundaries, unavailable blobs, cancellation, and ordinary/prepared CI page-only delivery from a sparse checkout without Go. The complete offline suite and compiled public API/dependency checks pass.
Evidence:
FromCommittedSelectionpasses rawgit showbytes to the declaration parser. A committedgo.modsymlink therefore supplies its link text. In a local fixture, Go validation and working-tree identity succeeded while committed identity reported no valid module directive. Existing committed validation already has reference/symlink resolution.ResolveSelectionalso requires the selected module directory to exist in the worktree throughEvalSymlinks. A clean, correctly tagged sparse checkout retained a nested module's declaration and note in Git but omitted its physical directory. Page-only delivery failed onlstatbefore inspecting committed identity. Its source was present and it needed no application compilation.
Required change: Separate filesystem execution selection from committed source selection where their prerequisites differ. Resolve committed declarations and directory/reference identity from the captured tree, retaining existing checkout boundaries, cycle handling, missing-object errors, and cancellation. Go execution may naturally require materialized source when it runs; page-only metadata delivery should not gain that compilation prerequisite. Keep working-tree builds tied to the declaration Go actually reads.
Verification: Cover an in-checkout committed declaration symlink and a page-only nested release from a clean sparse checkout with available committed objects. Verify committed/working-tree differences deliberately and retain reference-cycle, outside-checkout, unavailable-content, and source-tag guards.
Issue 8: Preserve literal shared filenames (P2)
Status: complete. Category: unrequested path prohibition.
Completion: Shared selection preserves literal backslashes in committed Git paths. A Unix regression selects and reads such a file once despite duplicates; traversal, absolute paths, and directories remain invalid selections. Existing literal-metacharacter, missing/default/empty-list, scope, and lazy-input tests pass offline.
Evidence: normalizeSharedFile rejects
every backslash. On Linux, a committed file named docs/notice\copy.txt is a
literal repository-relative file, but explicit selection failed as shared-files.
The roadmap selected literal individual paths and ordinary normalization without
globs; it did not exclude legal filename characters for committed inputs.
Required change: Preserve literal committed path characters under the chosen repository-path convention. Keep absolute/outside-repository and directory selection checks needed for the agreed scope. Introduce neither glob expansion nor a portability restriction on otherwise valid committed filenames.
Verification: On a filesystem where backslash is a literal filename character, select the committed file successfully. Retain traversal, missing explicit files, literal metacharacters, deduplication, and empty/default-list coverage.
Issue 9: Separate synthetic bootstrap versions from maintained examples (P2)
Status: complete. Category: broken adoption example and incomplete cutover guidance.
Completion: Maintained module skeletons no longer pin synthetic releases. Example setup explicitly selects compatible released tooling and documents source verification in disposable checkouts. Workflow fixtures generate their synthetic declarations. Current-source tool registration and all three example profiles, including native version injection, pass offline; mapped launchers/workflows and tagged-tool retries retain isolated fixtures. Adoption and migration guidance cover coordinated tool/workflow upgrades and historical retries. The pending release notes record concrete example pins as normal version-selection work.
Evidence: All three maintained
multi-module declarations require ReleaseKit
v0.2.197. This is a synthetic test version served by
writeWorkflowToolProxy,
whose executable forwards to the separately compiled current test binary.
A read-only query of the authoritative repository returned v0.3.0 and no
v0.2.197 tag. The copyable example cannot obtain its declared release tool
through ordinary module resolution.
Published pre-feature tooling also lacks the new profile/flag interface. The
maintained workflows always pass --prepared, and mapped launchers additionally
pass --expected-module. Migration guidance does not explain that adopting the
new workflow requires a compatible tool declaration, or that an older tag's
tool declaration requires its matching historical workflow for a retry.
Required change: Confine synthetic versions to test-owned module files and proxies. Make the maintained example's setup explicitly select a ReleaseKit version containing this feature, and update its concrete release dependency during normal release preparation when that version is selected. Keep setup usable for local source verification without fabricating a published release. Document tool/workflow cutover and historical retry guidance in the pending release notes and adoption owners. Maintain the clean API cutover; add no flag, receipt, or source-identity compatibility shim.
Verification: Tests generate their own synthetic dependency declarations instead of relying on fake versions committed in the copyable example. Verify the setup's selected tool accepts the actual new interface, using current source and isolated fixtures offline. Preserve tagged-source tool-version selection and test-owned caches. This adds no live module-service completion gate.
Issue 10: Remove the repeated boundary scan from file selection (P2)
Status: complete. Category: code quality and avoidable computational cost.
Completion: File selection indexes boundary directories and looks up each entry's ancestors. Existing ordering, vendor/shared selection, full reference lookup, and reader/cache behavior pass the complete offline validation suite. A component benchmark retains 20,000 inputs with 0, 1,000, and 4,000 boundaries. On this host, the 4,000-boundary case decreased from about 2.73 seconds to 38 milliseconds per selection; the new cases ranged from 29 to 38 milliseconds. These are illustrative measurements without timing gates or input limits.
Evidence: selectInputEntries loops
over all nested-module boundaries for every candidate entry. This costs
O(files × boundaries) even when the retained input set is unchanged.
Temporary committed-tree fixtures with 20,000 retained files took approximately
0.5 seconds with no boundaries, 1.0 second with 1,000 boundaries, and 2.8 seconds
with 4,000 boundaries. These timings include metadata reads and are illustrative
local measurements, not performance contracts.
Required change: Use direct ancestor-boundary lookup or another straightforward path-based selection that avoids scanning every unrelated boundary for each file. Preserve one ordered input set, vendor handling, path deduplication, explicit shared files, full reference lookup, and selective content-reader ownership.
Verification: Retain the existing scope behavior tests and add a representative component benchmark varying boundary count with the retained file count fixed. Verify the algorithmic improvement without adding timing thresholds, input caps, or new validation policies.
Issue 11: Reconcile documentation with implemented scope and status (P3)
Status: complete. Category: incomplete documentation adoption and misleading contracts.
Completion: Facade GoDoc and canonical API, CLI, configuration, Gitea, and internal guidance describe selected inputs, full-tag uniqueness, module-local notes, shared-file scope, committed references, and lazy Go reuse consistently. The feature roadmap identifies its historical baseline and completed stages, with this review recording the corrections. Consumer-owned license layouts and note links are distinguished from optional validation defaults. ReleaseKit's root release policy and procedure retain their existing requirements. Reviewed root/nested examples, verified local links/fragments across 37 Markdown documents, checked example JSON/shell syntax and whitespace, and passed the final complete offline suite and compiled API/dependency checks.
Evidence:
- Configuration, CLI,
check guidance, and
publication guidance still describe formatting as
including nested-module source.
CheckGoDoc similarly says local-replacement inspection includes nested modules. Automatic input selection now excludes independent nested modules, retaining vendor and explicitly selected files. - Check guidance retains the old short-version wording for note location and tag uniqueness. The new identities use physical module-local notes and full tags, allowing equal semantic versions across modules.
- The feature roadmap still says the feature is planned and module selection is not implemented, while the implementation plan claims completion. Its baseline needs to be identified as the starting baseline.
- Gitea guidance presents root shared-license and footer locations as an unconditional layout. Describe the default convention and consumer-owned note links accurately; shared-file validation selection supplies neither a license-location guarantee nor automatic footer rewriting.
Required change: Refresh each affected canonical owner using the implemented selected-input and full-tag semantics. Mark feature status accurately, including these open remediation items, while retaining useful decision context and the historical starting baseline. Preserve ReleaseKit's own root release policy. Update migration/examples alongside the behavior corrections above.
Verification: Review GoDoc and canonical guidance against the current code; check root/nested examples, note and tag wording, roadmap status, local links, fragments, and Markdown whitespace. Keep exact contract definitions at their canonical owners.
Follow-up audit
The follow-up reviewed module identity, candidate validation and publication,
builds, receipts, delivery, Go/process ownership, committed reference handling,
facade conversions, CLI/profile mapping, maintained workflows, documentation,
and the eleven previous corrections. The complete offline suite and compiled
API/dependency checks pass with Go 1.26.8. The additional findings below were
reproduced with temporary local repositories, a filtered sparse clone, a Git
transport stub, and a local HTTP server. The artifact-read evidence also used
strace. This audit changes the remediation document only.
Issue 12: Resolve module paths consistently across declaration sources (P2)
Status: complete. Category: inconsistent release-unit selection.
Completion: Filesystem selection follows symlinks before processing parent
components, matching committed traversal. Public build, candidate check, and
hosted page delivery now agree on the selected module and tag for link/..;
dependency preparation executes in that same module. The new regression cases
failed before the correction and pass afterward. Focused API/preparation tests
and the complete identity/Git suites pass offline, including aliases, separate
declaration sources, sparse selection, cycles, boundaries, and cancellation.
Evidence: Filesystem selection
cleans the joined module path before following symlinks. Committed selection
uses component traversal, following a symlink
before processing a subsequent ... The same selection can consequently
identify different physical modules in an unchanged, clean checkout.
A fixture contained root and nested modules, a tracked directory
nested/deep, and link -> nested/deep. With ModuleDir: "link/.." and
Version: "v0.1.0", public Build succeeded with the root module and tag
v0.1.0; public Check succeeded with the nested module and tag
nested/v0.1.0. Working-tree and committed declaration readers independently
confirmed the difference. Dependency preparation uses the filesystem selector,
while candidate and delivery operations use committed selection.
Required change: Give filesystem and committed selection the same component
and symlink semantics for identical paths in an unchanged checkout. Preserve
the agreed declaration source for each operation, support for working-tree
changes and committed sparse selection, checkout boundaries, and progressive
identity evidence. Module paths containing .. need consistent resolution;
this correction does not require prohibiting those paths.
Verification: Exercise a tracked directory symlink followed by .. through
standalone build, candidate check, dependency preparation, and representative
delivery. An unchanged checkout must identify the same physical module and tag
across operations. Retain deliberate committed/working-tree declaration
differences, ordinary aliases, cycles, and outside-checkout failures.
Issue 13: Reuse local asset verification through the complete delivery path (P2)
Status: open. Category: incomplete verification ownership and redundant I/O.
Evidence: Delivery calls
VerifyLocalAssets before hosted requests. After release and attachment
inspection, UploadMissing calls
verifyLocalAsset again for every missing attachment. Both calls stream and
hash each local binary. The upload then reads it a third time to send its bytes.
Stages 8 and 9 of the implementation plan select one
delivery-owned content-verification pass.
A public prebuilt-delivery fixture completed against a local HTTP server with one missing binary and its manifest. A system-call trace recorded three complete reads of the binary: the delivery preflight, the uploader's repeated verification, and the multipart upload. Receipt loading is already free of artifact hashing; the remaining duplicate sits across the delivery/Gitea boundary. The current asset-preflight benchmark ends before that second verification.
Required change: Reuse the completed local verification in the assembled delivery path so each binary receives one delivery-time digest-and-size pass before hosted requests, followed by its necessary upload stream. Preserve validation for independently used internal client operations, build-time hashing, deterministic manifests, actual remote-content comparison, additive retry behavior, caller-owned files, and existing failure outcomes.
Verification: Observe file reads across a complete public binary delivery with missing attachments, covering the uploader as well as preflight. Establish one verification read plus the upload read. Keep tampered/missing-file rejection before hosted requests and matching-asset/partial-retry cases. Measure the whole local path without timing thresholds or additional source guarantees.
Issue 14: Keep committed release-note inspection within the local object boundary (P2)
Status: open. Category: incomplete earlier offline-inspection correction.
Evidence: ReadCommitFile, used by candidate
capture and delivery for release notes, executes git show through the ordinary
Git runner. Its child does not receive the lazy-fetch and transport controls
used by ReadBlob, committed-path lookup, tree inspection, and the batch reader.
An unavailable note in a partial clone can therefore initiate remote transport
during source inspection. The earlier correction in 0205001 and its partial
clone tests cover the other object readers, leaving this entry point uncovered.
A clean filtered sparse clone had its root declaration and Go source locally
available, with the committed note omitted and its blob unavailable locally.
Public Check resolved the root identity, then invoked a fixture SSH transport
while reading docs/releases/v0.1.0.md. The stub recorded the attempt and exited;
the resulting error reported failure to fetch the note from the promisor remote.
The fixture contacted no external service.
Required change: Read committed note bytes through the same local-only object boundary as the other source-inspection readers. Return an inspection error when required objects are unavailable locally, preserving path context, cancellation, underlying Git causes, and exact note bytes. Explicit checkout and dependency preparation retain their network-capable responsibilities.
Verification: Cover direct committed-file reading and representative candidate/delivery note inspection in filtered clones with unavailable note objects. A controlled transport stub must remain unused, and inspection must leave the object database unchanged. Existing ordinary note reads and explicit network-capable preparation must continue to work.
Issue 15: Limit blob availability requirements to selected validation inputs (P2)
Status: open. Category: module-scope leakage through eager metadata enrichment.
Evidence: ReadCandidateTreeForScope
calls ReadTree before constructing module and
shared-file inputs. ReadTree asks populateBlobSizes for every repository blob
and fails on any unavailable object. Excluded modules and unreferenced files
therefore become availability prerequisites for a selected module's file checks,
even when the selected check needs no blob sizes.
In a clean filtered sparse clone, the root module's source, declaration, and
note were available; independently scoped nested module blobs were omitted.
Public root Check succeeded with ordinary validation. Enabling GoFormat
with an explicit empty shared-file list made the same check fail during size
enumeration on the unavailable blob for nested/deep/placeholder. That file
belongs to the excluded nested module and is neither a formatting input nor a
referenced destination.
Required change: Separate full repository path/type/object metadata from blob enrichment. Construct the agreed input set before acquiring required blob sizes or contents, and inspect referenced destinations when a check actually needs them. Keep full lookup available without requiring every unrelated blob locally. Preserve selected-input ordering, boundaries, vendor/shared selection, size semantics, local-only reads, cancellation, and reader ownership.
Verification: Use a filtered sparse fixture with a complete selected module and unavailable blobs in an excluded sibling or nested module. Representative formatting, file-size, and explicit shared-file validation must work when their required inputs are available. A selected missing blob or required referenced destination must still produce the appropriate inspection error. Verify that excluded unreferenced blobs cause neither content reads nor transport.