Files
releasekit/docs/roadmap/remediation.md

786 lines
45 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Publication reconciliation remediation
Status: open. R01–R13 are addressed and individually verified. R14–R17 record
remaining findings from the subsequent stage-by-stage review of `363f7cd`.
Address the two handoff defects first, then the classification and format gaps.
R01–R13 describe commits `4dff808` through `599ad81`, against planning baseline
`447b42a`. Their audit evidence and defect descriptions refer to that reviewed
baseline; each completion marker records the implemented correction. The full
offline suite (`go test ./...`) and compiled facade/dependency checks
(`go run ./internal/apicheck`) passed with Go 1.26.8 after that remediation.
The [feature roadmap](publication-reconciliation.md) defines the requested
behavior; the [implementation plan](implementation.md) supplies the intended
implementation. The broad package structure is in place: the root facade owns
public evidence and recovery types, `internal/publish` owns observation,
recording, and discovery, `internal/git` owns scoped endpoints and ref queries,
and the CLI owns command parsing, presentation, and exit classification.
The findings below identify remaining correctness, boundary, and contract gaps.
## Review evidence
The full offline suite (`go test ./...`) and compiled facade/dependency checks
(`go run ./internal/apicheck`) passed with Go 1.26.8. Additional disposable probes
used local Git repositories, synthetic credentials, controlled Git subprocesses,
and failing local output. They reproduced the findings below without changing
production code or adding repository tests. Passing existing tests does not
cover these failure combinations.
## R01: Preserve candidate failures and keep storage failures separate — P1
**Completed:** Capture and storage errors now remain separate. Focused publication
regressions passed for invalid/missing notes, retained Git causes, saved partial
evidence, and successful publication after storage lookup failure.
**Relevant code:** `internal/publish/publish.go:110`, especially the assignment
at line 120 and the error check at line 127.
`candidate.Capture` can return useful repository/identity fields together with an
error, such as a missing or invalid release note. When its repository field is
present, the new storage initialization assigns `resolveRecordDir`'s error to
the same `err` variable before the capture error is checked.
This produces two opposite defects:
- Successful storage resolution erases the capture error. Publication continues
with a candidate that failed capture. The probe used a release note with the
wrong product heading: `Check` rejected it, while `Publish` ran the product
callback, returned `published` with a nil error, and pushed the annotated tag
to the local remote.
- Failed storage resolution replaces a successful capture with a publication
failure. A controlled failure of the Git common-directory lookup returned
`not_attempted`, ran no product callback, and returned a `capture release
candidate` error as well as `RecordError`.
**Expected behavior:** A capture error stops publication while preserving useful
partial evidence. A recording/storage error is reported separately and leaves
the publication result governed by its existing checks and push outcome.
**Remediation:** Keep capture and storage errors in separate variables. Preserve
the original capture failure across record initialization/finalization; use
`RecordError` exclusively for recording failures. Add regressions for invalid
and missing notes, capture-error cause preservation, and a valid candidate whose
storage-directory lookup fails. Verify that rejected candidates run no product
checks or tag creation, and that storage lookup failure permits an otherwise
successful publication.
## R02: Validate the filename that will actually be written — P2
**Completed:** Each generated filename is checked before that exact path is
created, including collision retries. Deterministic selective-ignore tests passed
and confirm that rejected writes leave the candidate clean and existing records
unchanged.
**Relevant code:** `internal/publish/record.go:68`, `record.go:85`, and
`record.go:134`; the pre-push save in `internal/publish/publish.go`.
The override guard generates a random proposed filename and checks its Git
ignore status. The creation loop then generates a different random filename
and writes that file without checking its ignore status.
Filename-dependent ignore rules therefore allow the check and actual write to
disagree. A probe using `selective/attempt-[0-7]*.json` in `.gitignore` successfully
saved a record starting with `attempt-c...`; Git reported that actual record as
untracked. The save returned success. The reverse mismatch can also reject a
name even though another name the writer would choose is ignored.
**Expected behavior:** The storage safeguard applies to the exact path passed to
exclusive creation. A failed storage choice leaves the handoff unavailable
without introducing unignored candidate contents.
**Consequences:** During publication, saving occurs after the final clean-tree
guard. An unchecked actual path can therefore leave the worktree dirty while
publication succeeds, and can expose a sensitive record to a later ordinary
`git add` operation. The already captured release tag does not itself include
that new file.
**Remediation:** Generate one candidate path, validate that path, and use the
same path for exclusive creation. Repeat that sequence if a filename collision
requires another candidate. Add deterministic coverage with filename-dependent
ignore rules; verify that successful writes are ignored and rejected choices
leave the worktree clean. A naming seam can make this test deterministic rather
than relying on random retries.
## R03: Allow an explicit override into Git metadata — P2
**Completed:** The override guard recognizes Git-discovered private and shared
metadata directories. Recording tests passed for ordinary, linked-worktree, and
separate-Git-directory layouts, alongside rejection of unignored worktree files.
**Relevant code:** `internal/publish/record.go:142`, especially the worktree
containment and `git check-ignore` handling at lines 148–159.
For a conventional checkout, the default store is inside
`.git/releasekit/publication-attempts`. Selecting that same directory explicitly
activates the override guard. It treats the path as worktree content and demands
an ignore-rule match, although Git metadata is already excluded from tracked
source and ordinary status reporting.
The probe saved a default record successfully, then selected its parent
directory as the explicit override. The second save failed with `record_write`
and an assertion that the metadata path was inside the worktree and unignored.
**Expected behavior:** The agreed storage choices include Git metadata,
external locations, and Git-ignored worktree locations. An override changes the
location rather than imposing a requirement to add ignore rules for metadata.
The rejection is an additional storage prohibition beyond the intended policy.
**Remediation:** Identify Git metadata through Git's directory discovery and
recognize that it is outside candidate contents, including linked-worktree and
separate-Git-directory layouts. Apply ordinary ignore checks to actual
worktree-content paths. Add coverage for explicitly selecting the default store
and other private locations in Git metadata, retaining rejection of genuine
unignored worktree contents.
## R04: Restore cancellation across storage and recovery phases — P2
**Completed:** Storage and discovery share context-aware Git adapters; later
observation classifies canceled tag validation and queries consistently. Tests
passed for already canceled discovery and cancellation synchronized with active
root/metadata lookups, ignore checks, publication storage, and tag operations.
Focused Git, facade, and CLI recovery checks also passed; immediate failed-push
reconciliation retains its existing bounded context.
**Relevant code:** `internal/publish/record.go:112` and `record.go:152`;
`internal/publish/attempts.go:31` and `attempts.go:74`;
`internal/publish/reconcile.go:109` and `reconcile.go:130`.
Cancellation behavior depends on which phase is active:
- `resolveRecordDir` and the ignore guard use `exec.Command` without the caller's
context. A probe held the common-directory lookup, canceled publication, and
observed that the call remained blocked until the controlled Git command was
explicitly released.
- Discovery resolves a default/relative store before checking `ctx.Err()`, and
wraps all resolution failures as `storage_enumeration`. An already canceled
default-store request consequently returned that kind instead of `canceled`.
- `recordDirectoryFor` uses raw `exec.CommandContext`, which can return only a
killed-process error when cancellation occurs during Git execution. The
active-discovery probe returned `storage_enumeration` and
`errors.Is(err, context.Canceled) == false`.
- Later observation checks cancellation in some branches but not the tag-ref
validation and `QueryTag` error branches. Canceling an active tag query
returned `remote_observation`, even though its error chain correctly retained
`context.Canceled`.
**Expected behavior:** Caller cancellation stops cancellable Git work and is
reported as `canceled`, with the context cause retained. Discovery preserves
already obtained entries. Immediate failed-push reconciliation retains its
separately selected bounded recovery context.
**Remediation:** Consolidate the duplicated storage-directory/Git command work
behind the shared internal Git/process boundary, passing the relevant context
and preserving its cause. Normalize recovery errors at each failed operation
using the known context cause, rather than assigning a phase kind first and
losing cancellation. Keep partial evidence finalization and recording warnings
separate from the original publication failure. Add coordinated tests for
already canceled and actively canceled default/relative discovery, blocked
storage Git commands, and cancellation during tag validation/query. Preserve
the existing immediate-recovery timeout semantics.
## R05: Redact recovery paths and diagnostic fields at display boundaries — P2
**Completed:** Facade errors and CLI recovery displays now redact recognized
HTTP credentials in paths, metadata, diagnostics, and handoff commands. Public
error and CLI human/JSON regressions passed; raw typed causes and private record
contents remain intact.
**Relevant code:** `recovery.go:53`; `cmd/releasekit/recovery.go:76`, its JSON DTO
construction, and human path displays; `cmd/releasekit/main.go`'s record displays.
`PublicationRecoveryError.Error` redacts its wrapped cause but interpolates its
own `RecordPath` directly. The CLI also copies record paths and diagnostic
messages directly into human and JSON output. Safe evidence formatting protects
the endpoint field, but does not provide a general safe-display boundary for
these other strings.
Passing the synthetic value
`https://audit-user:audit-secret@example.invalid/attempt.json` to
`LoadPublicationEvidence` reproduced the issue. The nested file-read cause was
redacted, while the facade error's path still contained `audit-secret`.
`reconcile --record` with the same invalid local-record selection also emitted
the synthetic credential in JSON output. This can expose a credential-bearing
URL pasted into the wrong argument, despite the documented credential-safe
error and diagnostic behavior.
**Expected behavior:** Recognized endpoint credentials are redacted throughout
human and structured diagnostics. Raw values remain available in typed evidence,
private records, and the filesystem arguments used for the actual operation.
**Remediation:** Apply the existing diagnostic/URL formatter to every relevant
display string, including recovery-error paths and CLI path/identity/diagnostic
fields, before formatting or JSON encoding. Keep raw operation inputs and private
serialization intact. Add public-error and CLI human/JSON regressions covering
synthetic credential-bearing paths, causes, and metadata, alongside the existing
endpoint-only case. No new credential-storage policy is needed.
## R06: Propagate structured-output write failures — P2
**Completed:** Both recovery commands now report JSON encoder failures on
standard error and return status 1. Failing-writer regressions passed alongside
ordinary recovery status tests; observed publication state remains unchanged.
The CLI reference records the reporting-failure status.
**Relevant code:** `cmd/releasekit/recovery.go:242` and its two `writeJSON`
call sites.
`writeJSON` explicitly discards the error returned by `json.Encoder.Encode`.
Command status is then chosen entirely from observation/discovery results,
even when no structured result reaches the caller.
The probe enumerated an empty existing store with `--json`, directing stdout
to `/dev/full`. It returned status 0 with no stderr diagnostic despite the
failed write. The same helper is used by successful reconciliation.
**Expected behavior:** Automation can distinguish successful output delivery
from an output failure. A local reporting failure remains separate from the
already observed remote publication state.
**Remediation:** Return and handle the encoder error. Report the local output
failure on stderr and return an appropriate operational failure status, while
preserving the actual observation in the implementation rather than converting
it to unknown remote state. Document that output-failure status in the CLI
contract. Add focused failing-writer coverage for both structured commands;
ordinary writable-output state/status behavior should stay unchanged.
## Integration audit evidence
A subsequent audit examined the interaction with existing consumer launchers,
repository discovery, candidate cleanliness, and the maintainer runbook. The
production code remains identical to the code covered by the full-suite/API
checks above. A fresh focused run of the existing launcher, Woodpecker,
publication-handoff, and lost-output recovery integration tests passed with
Go 1.26.8. Disposable probes reproduced the additional issues below. R01–R06
remain open and are preserved rather than duplicated here.
## R07: Make the recovery handoff survive a cold Go-tool invocation — P2
**Completed:** CLI recovery prefers the selected module's Go tool. API-only
consumers receive privately retained alternate module/checksum files under Git
metadata; native consumers retain the executable-path fallback. Assembled tests
passed after cold executables disappeared and launcher files were removed,
covering both adoption styles, relative replacements, clean candidates, and
recovery exit statuses 0–3. CLI, consumer, and internal guidance is updated.
**Relevant code:** `cmd/releasekit/main.go:134` (`publicationRecoveryCommand`);
`examples/multi-module/release.sh`; `docs/consumers/adoption.md`'s module-selected
Go-tool workflow; `cmd/releasekit/publication_handoff_test.go`.
The new handoff always uses `os.Executable()` as the command to run later.
Existing consumer guidance and the maintained launcher execute ReleaseKit
through `go tool releasekit`. On an uncached tool build, Go 1.26.8 executes a
temporary build executable and removes it when the Go driver exits. A warm
invocation uses a surviving cached executable, so its behavior differs.
**Evidence:** A disposable tool-registered consumer used the current CLI with a
cold tool build and a local remote that rejected the tag push. Publication
returned status 1 and saved a complete record, but its printed recovery command
named `/tmp/go-build.../exe/releasekit`. That executable was already absent when
the driver returned, and copying the command returned status 127. A surviving
CLI reconciled the same record successfully to `not_published` with status 1.
The existing handoff test builds a persistent executable directly and therefore
does not exercise this launch lifecycle.
**Expected behavior:** The copyable handoff works with supported consumer launch
methods after the publication process has exited. Routine first use on a fresh
machine/cache should not require reconstructing another invocation manually.
The existing fallback guidance for a subsequently unavailable executable does
not supply a working handoff for this ordinary cold-build case.
**Decision:** Prefer a module-selected Go invocation for recovery where the
consumer setup supports it. Provide an explicit, durable recovery path for
API-only consumers using temporary tool declarations. ReleaseKit owns the
handoff mechanics and supports both adoption styles. This follows the existing
tool-selection workflow and keeps routine binary lifecycle management out of
consumer code.
**Remediation:** Make the recovery invocation independent of an ephemeral
Go-tool executable using the selected module-based approach and an explicit
API-only recovery path. Account for the maintained launcher's temporary
`-modfile` cleanup and its support for ordinary
package dependencies as well as registered tools; a bare `go tool` command
cannot assume every consumer has a permanent tool declaration. Keep explicit
record selection and distinct exit statuses. Add an assembled cold-build test
that executes the printed command after the original Go driver and launcher
have exited, rather than only testing a directly built CLI or a warm cache.
## R08: Reuse the existing repository-path semantics for record storage — P2
**Completed:** R04's shared resolver reuses the established Git root and
metadata-path normalization. Added regressions passed for repository roots and
separate metadata directories ending in whitespace, default/relative read-write
agreement, and rejection of absolute unignored worktree stores. Existing ordinary
and linked-worktree recording tests also passed.
**Relevant code:** `internal/publish/record.go:112`, especially line 117;
`internal/publish/attempts.go:74`, especially line 82; the existing
`internal/git/source.go:39` and `source.go:50` repository-root helpers.
The pre-existing root resolver removes Git's output newline and resolves
filesystem aliases, preserving valid whitespace in the path itself. The new
recording and discovery resolvers independently use `strings.TrimSpace`, which
changes repository identity when a directory name ends in whitespace. Recording
then uses that altered root both for relative overrides and for deciding whether
an absolute override lies inside the candidate worktree.
**Evidence:** In a temporary checkout whose final directory component was
`consumer `, the existing `git.RepositoryRoot` preserved the exact path and
the existing candidate `Check` completed successfully. The new code produced
three inconsistent results:
- A relative `.releasekit/attempts` override tried to use the different sibling
path under `consumer` and failed its Git ignore check, despite the real
`.releasekit/` location being ignored.
- After a record was saved in the correct absolute ignored location, relative
discovery selected the wrong sibling directory and returned a successful
empty collection. The actual record still existed in the requested store.
- An absolute override inside the actual worktree was misclassified as
external because containment used the trimmed root. It successfully wrote an
unignored record, which Git reported as untracked.
**Expected behavior:** Publication, discovery, relative overrides, and worktree
containment use the same repository identity already supported by the library.
Path data remains intact. This failure is independent of R02's filename mismatch:
the containment decision itself uses the wrong root.
**Remediation:** Reuse `internal/git.RepositoryRoot` and share metadata-path
resolution through the Git boundary, coordinating with R04 rather than adding
another resolver. Remove output terminators without stripping path characters.
Apply the same preservation to common-directory output. Add focused regressions
for a valid root ending in whitespace, relative-store read/write agreement, and
absolute unignored-path rejection. Preserve existing filesystem-alias and
linked-worktree behavior instead of introducing new pathname restrictions.
## R09: Align maintainer recovery instructions with the source bootstrap — P2
**Completed:** Runbook discovery and reconciliation use the prepared source-built
CLI, and the roadmap recovery link targets the current heading. Disposable local
checks executed both snippets with no global `releasekit`, confirming discovery
and the expected incomplete-evidence reconciliation status.
**Relevant code/docs:** `scripts/release:9`; `docs/release.md:104` and
`release.md:149`; `docs/roadmap/publication-reconciliation.md:21`.
The existing maintainer procedure builds and executes
`.releasekit/bin/releasekit`. It does not install an executable named
`releasekit` on `PATH`. The new lost-output recovery instructions instead run
`releasekit publication-attempts` and `releasekit reconcile` without identifying
the executable that the procedure prepared.
**Evidence:** With Go and Git available on `PATH` and only the source-built CLI
in `.releasekit/bin`, the newly documented discovery command returned 127
(`releasekit: not found`). The equivalent command using
`./.releasekit/bin/releasekit` returned 0 and listed the saved attempt. A globally
installed CLI could also be a different version from the source-built one.
The recovery-section rename additionally left the feature roadmap's link to
`docs/release.md#resolve-failures` pointing at a heading that no longer exists.
**Expected behavior:** The repository's executable runbook uses the CLI prepared
by its own prerequisites, and its recovery links resolve to the current owner.
It requires no additional global installation for lost-output recovery.
**Remediation:** Use the source-built executable for discovery and reconciliation
in the maintainer runbook, consistent with its existing delivery commands.
Retain explicit instructions for selecting another supported CLI when the
original executable is genuinely unavailable. Repair the renamed-section link.
Verify the recovery snippets in an environment without a global `releasekit`
executable; keep generic consumer command examples in their consumer-facing
documentation.
## R10: Preserve endpoint bytes across the private-record handoff — P2
**Completed:** Private records retain ordinary version-1 JSON and select a
version-2 base64 byte representation for non-UTF-8 fields. Both versions load;
unknown fields remain ignorable and malformed known byte fields report format
errors. Round-trip tests passed for all evidence fields, and real Git
observations distinguish the original byte path from a replacement-character
decoy consistently before and after loading. The record-format guide is updated.
**Relevant code:** `internal/publish/record.go:77` and its evidence decoding;
the string fields in `internal/publish/reconcile.go`'s `Evidence`; the existing
Git endpoint resolution/query path in `internal/git/remote.go` and `tag.go`.
On POSIX, the existing Git transport can use local repository paths containing
bytes that are not valid UTF-8. Typed evidence retains those bytes in its Go
strings. The new private JSON codec uses `json.Marshal` directly on those
strings, which replaces invalid UTF-8 with the Unicode replacement character
without reporting an error. Loading the record therefore supplies a different
endpoint to the existing Git observer.
**Evidence:** A disposable valid candidate's publication attempt targeted a local
repository whose path contained a non-UTF-8 byte. Its hook rejected the push;
publication returned `not_published` with a complete saved record and the exact
original endpoint in typed evidence. A second local repository, named using the replacement-character
form of that path, held the same intended annotated tag object. Direct typed
reconciliation correctly returned `not_published` at the original destination.
Loading the saved record changed its endpoint to the second repository and
record-based reconciliation returned `published` there. The record save/load
reported no error.
**Expected behavior:** Direct and record-loaded evidence identify the same
captured destination. The private handoff retains exact transport identity,
including pathname bytes supported by the existing Git boundary, or reports
that the handoff could not be saved faithfully. Diagnostic JSON is a display
format and can use safe displays; the private record supplies actual operation
inputs and must preserve their identity.
**Decision:** Use a lossless private-record representation for supported
pathname and endpoint bytes. This preserves existing publication inputs and
keeps record-loaded recovery consistent with direct typed evidence. Recording
failures remain separate from the publication outcome.
**Remediation:** Make private serialization lossless for supported evidence,
using an explicit byte representation where needed. Keep the original typed
evidence and publication behavior intact; avoid adding a general UTF-8
restriction to previously valid Git publication inputs. Cover endpoint/path
round trips and equivalent direct-versus-loaded observations, including a
distinct replacement-character path, in focused regression tests.
## Algorithmic audit scope and evidence
This audit retains the planned flat record store, complete local discovery,
deterministic ordering of selected record paths, versioned JSON format, and
exact-tag observation semantics. It does not propose indexes, pagination,
automatic retention, or changes to those selected algorithms.
No unexpected quadratic algorithm was found in the added production code.
Classification operates on a fixed number of fields and bounded-length object
IDs. Record encoding/decoding uses a fixed number of linear passes over a
record. Facade and CLI result conversions are linear in returned entries and
displayed data. Full-result discovery necessarily retains the records its API
returns. The refinements below concern work outside those necessary operations
and one partial-result correctness defect.
The existing codec, discovery, and classification tests passed in a fresh
focused run with Go 1.26.8. Complexity analysis used the implementation and
the local Go standard-library source, without benchmarks or real-world
performance measurements. Disposable functional probes confirmed the parser
cause loss and partial-enumeration result loss. The latter used a temporary
source overlay only to make the directory-reader boundary return real entries
with a controlled error; repository production and test files were unchanged.
## R11: Filter directory entries before ordering them — P3
**Completed:** Discovery filters unsorted directory batches and sorts selected
record paths once. Directory work is `O(D + R log R)` with bounded intermediate
entry storage. Mixed-directory, ordering, and cancellation regressions passed.
R13 addresses preservation of records alongside enumeration failures.
**Relevant code:** `internal/publish/attempts.go:41` through the subsequent
filter and `sort.Strings(paths)`.
`os.ReadDir` already sorts every directory entry by filename. Discovery then
filters for JSON record files and sorts their full paths again. Consequently,
an overridden store containing many unrelated files pays to order names that
will never appear in the result. This is an implementation choice beyond the
planned requirement to order selected records.
**Complexity:** Let `D` be all directory entries, `R` the selected JSON file
candidates, and `B` their total bytes read. Treating name comparisons as unit
operations, the current directory ordering has worst-case `O(D log D)` cost
before the same `O(B)` record work. Filtering an unsorted enumeration and sorting
only selected paths costs `O(D + R log R + B)`. When `R` is bounded and `D`
grows, directory processing improves from `O(D log D)` to `O(D)`. When nearly
every entry is a record, the planned sorting bound remains unchanged. The
second sort currently receives already ordered paths; it is redundant, rather
than evidence of a quadratic algorithm.
**Expected behavior:** Keep complete discovery and deterministic path ordering
for attempts and their diagnostics. Unrelated directory entries need no order.
**Remediation opportunity:** Use the standard `os.File.ReadDir` interface to
enumerate without implicit sorting, filter candidates, and order those paths
once. Batched reads can also reduce intermediate directory-entry storage from
`O(D)` to `O(batch size + R)`, excluding retained result data. Preserve partial
enumeration errors as described in R13 and cancellation behavior from R04.
Keep this a local enumeration refinement; no public file-count limits or
changes to record selection are required.
## R12: Reject trailing JSON without constructing a discarded value — P3
**Completed:** Loading uses complete-document unmarshalling and wraps the actual
parser error. It avoids constructing a discarded trailing value while retaining
the whole-file read bound. Tests passed for single documents, whitespace,
additional fields, extra documents, malformed suffixes, and retained syntax
causes, alongside byte-preserving record regressions.
**Relevant code:** `internal/publish/record.go:24`, particularly the second
`decoder.Decode(&trailing)` and its error handling at lines 34–37.
After decoding the envelope, the loader decodes another complete JSON value
into `any` solely to check whether one exists. A second value always makes the
record invalid. Nevertheless, an appended array or object is fully parsed and
materialized before being discarded. Only trailing whitespace and EOF are
acceptable at that point.
**Complexity:** For a first envelope of `L` bytes and an appended value of `T`
bytes containing `M` nodes, the second decode adds `Theta(T)` parsing work and
up to `Theta(M)` object/array storage. Rejecting at the first non-whitespace byte
after the envelope needs no second value tree. A standard-library complete-
document unmarshal of the already loaded bytes can reject that trailing input
during syntax validation before constructing it. This removes avoidable parsing
and allocations; the existing whole-file read still has its `Theta(L + T)` I/O
and buffer bound. It is not an overall quadratic-to-linear claim.
There is also a correctness inconsistency: a non-EOF error from that second
decode is replaced by a new message without wrapping the original error.
Functional probes showed that an incomplete first document retained
`io.ErrUnexpectedEOF`, while an incomplete trailing document lost it. A syntax
error in the first document was available through `errors.As` as
`*json.SyntaxError`; the corresponding trailing syntax error was not.
**Expected behavior:** Require one complete document, retain supported partial
evidence/additional-field behavior, and preserve available parsing causes in
the typed `record_format` error. Invalid extra data needs no materialized tree.
**Remediation opportunity:** Simplify loading to a complete-document unmarshal
into the versioned envelope, retaining existing format/evidence validation and
cause wrapping. An equivalent trailing-data check can also work if it avoids
decoding an arbitrary second value and retains real parser failures. Add focused
single-document, extra-document, trailing-whitespace, and malformed-suffix
coverage. Keep raw record identity handling coordinated with R10; no new size
caps or public input restrictions are needed.
## R13: Retain entries returned with a directory-enumeration error — P2
**Completed:** Discovery loads returned record paths before reporting enumeration
failure. Fault-injected entries-plus-error and zero-entry tests passed, preserving
readable records, per-record diagnostics, ordering, and the original cause.
Facade conversion and CLI human/JSON tests confirm partial results survive with
a non-success status. The full offline suite and API/dependency checks passed.
**Relevant code:** `internal/publish/attempts.go:41` through its immediate error
return at lines 42–44; Stage 5's partial-result requirement in the implementation
plan.
Go's `os.ReadDir` can return both entries read successfully and a later read
error. Discovery checks the error first and immediately returns its still-empty
result, dropping every returned entry. Thus a filesystem enumeration failure
can hide readable attempts that were already discovered, even though the API
and CLI support returning attempts alongside a top-level failure.
**Evidence:** A disposable probe supplied the directory-reader boundary with
the real entry for one valid record plus a controlled enumeration error, an
outcome permitted by `os.ReadDir`'s contract. `ListPublicationAttempts` returned
zero attempts with `storage_enumeration`. Without the injected error, it
returned that same record successfully. This was fault injection at the read
boundary, not a change to the discovery algorithm under review.
**Expected behavior:** Preserve readable attempts and per-record diagnostics
from returned entries while reporting the top-level enumeration failure. The
operation remains incomplete and retains its non-success CLI classification.
**Complexity:** Processing a returned prefix of `d` entries containing `r`
record candidates and `b` readable bytes takes the same ordinary filtering,
ordering, and decoding work: `O(d + r log r + b)` with R11's refinement. The
existing `os.ReadDir` approach instead retains its `O(d log d + b)` bound.
Neither correction requires another directory scan: both retain the names
already enumerated and perform the normal record loads for those names.
**Remediation:** Retain the enumeration error, process available entries using
the normal discovery path, and return both the collected result and that error.
Handle zero-entry failures and cancellation consistently with R04. Add a narrow
fault-injected regression for entries-plus-error, verifying readable attempts,
retained causes, and API/CLI partial-result behavior. This concerns data
preservation, separate from R04's cancellation/error classification.
## Follow-up stage review evidence
The subsequent review examined all eight completed stages in the implementation
plan, including the R01–R13 corrections, at `363f7cd`. Existing tests for
classification, Git endpoints, records, public recovery, publication evidence,
discovery, CLI parsing/presentation, cold Go-tool handoffs, and the assembled
lost-output workflow passed with Go 1.26.8. The compiled facade/dependency checks
and local documentation link/fragment checks also passed.
Disposable local probes reproduced R14–R17 using temporary Git repositories and
Go source overlays. Repository production code and tests remained unchanged.
No additional defects were identified in Stages 3–6. Stage 8 needs regression
coverage for the cases below and plan wording aligned with the implemented
record and handoff decisions. Existing passing coverage does not establish
correctness for these additional combinations.
## R14: Support recovery for vendored API-only consumers — P2
**Completed:** API-only recovery uses a private module root with preserved module
settings and relocated local replacements. Vendored API-only, relative/versioned
replacement, cold Go-tool, native handoff, and source-module entry-point tests
passed. Stage 7/8 instructions and consumer/internal guidance are updated.
**Relevant code:** `internal/gotool/recovery.go:95–107`; the command assembly in
`cmd/releasekit/main.go:126–145`. Affects Stage 7 and its Stage 8 coverage.
The API-only handoff prepares its private alternate module files with
`go list -mod=mod`, then prints a `go tool` command running from the consumer's
module directory. Go automatically selects vendor mode when that directory
contains a suitable `vendor` tree. An API-only consumer vendors its imported
library packages, so the CLI package can be absent from that tree.
**Evidence:** A temporary consumer imported ReleaseKit through its Go API,
vendored its dependencies, and used the current CLI for a locally rejected
publication. A complete record and recovery command were produced. Executing
the command failed before reconciliation with `import lookup disabled by
-mod=vendor`. Supplying `GOFLAGS=-mod=mod` also failed to change this Go-tool
behavior. Existing vendored-tool coverage uses an explicitly registered and
vendored CLI, which does not exercise this API-only case.
**Expected behavior:** A consumer's ordinary library vendoring remains compatible
with the supported API-only recovery handoff. The command observes the explicitly
selected record using the selected ReleaseKit dependency.
**Remediation:** Prepare the API-only Go-tool entry point in a private bootstrap
module root whose tool resolution is independent of the consumer's vendor tree.
Preserve the selected dependency graph, checksums, versioned replacements, and
the meaning of relative local replacements when relocating module metadata.
Keep source files and consumer vendoring intact, retain private storage handling,
and keep bootstrap ownership within ReleaseKit. Preserve registered-tool and
native entry points. Update Stage 7's handoff instructions to describe this
module-selected approach and the supported native fallback.
**Regression coverage:** Add an assembled API-only consumer that vendors the
library without a tool declaration. Execute the printed command from another
directory after publication and launcher cleanup; assert that reconciliation
runs against the saved identity and returns its expected state/status. Cover
relative local replacements and unchanged consumer source/vendor contents.
Retain existing cold-build, registered/vendored-tool, quoting, and status cases.
## R15: Preserve the recovery handoff after publication cancellation — P2
**Completed:** Post-failure handoff preparation has a separate bounded context
and retains a persistent native fallback after module-preparation failure.
Coordinated native, cold registered-tool, and temporary-launcher cancellation
tests passed, including execution of the printed command after cleanup and
retained cancellation diagnostics/statuses. Temporary Go-work executable
classification is covered, and Stage 7/8 plus canonical guidance is updated.
**Relevant code:** `cmd/releasekit/main.go:116–134`, especially the caller context
passed to `git.CommonDirectory`. Affects Stage 7 and its Stage 8 coverage.
Command preparation reuses the publication context after publication returns.
When cancellation caused the failure, that context is already canceled. The
metadata lookup fails immediately and the CLI prints `recovery command
unavailable`, even when it has saved the record and a usable native entry point
is available. The native fallback is reached only when module selection returns
no Go invocation; a preparation error bypasses it.
**Evidence:** A probe synchronized cancellation with a blocked tag push. The
publication returned status 1 after immediate reconciliation found the tag
absent. Standard error reported a saved record and an unavailable recovery
command, with no copyable handoff. This was cancellation during execution,
rather than termination before the record could be saved.
**Expected behavior:** An ordinary canceled publication with a saved record
retains a usable recovery entry point. Its preparation leaves the original
publication outcome, cause, and exit classification intact.
**Remediation:** Arrange handoff preparation so the failed publication's canceled
context does not prevent command construction. Prepare necessary durable launch
metadata before cancellation-sensitive publication work where appropriate, and
retain a usable native-command fallback when module-based preparation fails.
Account for cold Go-tool executables that disappear when the original driver
exits. Keep publication and later observation cancellation behavior from R04;
any separate post-failure preparation that performs subprocess work needs a
bounded lifetime. Update Stage 7 to cover cancellation and preparation failures.
**Regression coverage:** Cancel an actively blocked push after pre-push recording,
then execute the resulting command using a fresh invocation/context. Cover a
persistent native executable and supported cold Go-tool adoption paths after
launcher cleanup. Assert saved identity selection, unchanged publication status
and original cancellation cause, and preservation of immediate recovery's bounded
context. Coordinate the push boundary explicitly and retain the existing
no-recovery-step behavior for successful publication.
## R16: Classify endpoint setup failures by their actual cause — P3
**Relevant code:** `internal/publish/reconcile.go:116–128` and
`internal/git/remote.go`'s endpoint setup. Affects Stages 1 and 6.
An `ObserveEndpoint` error defaults to `endpoint_changed` when the evidence has
no original remote name. With a name, the fallback is `transport_configuration`.
The choice can therefore describe the presence of optional context rather than
the failure that occurred.
**Evidence:** A local checkout with malformed Git configuration and evidence
whose `Remote` was empty returned `unknown` with `endpoint_changed`. Its retained
cause was Git's bad-configuration error; no changed destination had been
established. CLI guidance consequently points the operator toward URL rewrite
settings despite the configuration-read failure.
**Expected behavior:** `endpoint_changed` identifies a positively detected
destination change. Configuration and other operational setup failures retain
an appropriate unresolved classification and their original causes, including
when the original remote name is absent.
**Remediation:** Distinguish setup failures at the internal Git boundary where
their source is known, then map them to the supported recovery kinds. Use the
existing endpoint-change sentinel for positively detected rewrites. Preserve
configuration/tooling causes through the facade and use the same interpretation
for human guidance and structured diagnostics. Clarify this mapping in Stage 1
and its Stage 6 presentation requirements using the existing recovery kinds.
**Regression coverage:** Exercise malformed local configuration with an empty
original remote name, a genuine rewrite mismatch, and a separate operational
setup failure at the Git boundary. Verify typed kinds, retained causes, unknown
state, and corresponding CLI human/JSON diagnostics. Keep remote queries from
reaching a substituted endpoint when a rewrite mismatch is detected.
## R17: Validate the documented version-2 byte-field representation — P3
**Relevant code:** `internal/publish/record.go:48–59`;
[private record format](../integrations/publication-records.md#private-attempt-record).
Affects Stage 2 and its Stage 8 format coverage.
Version-2 byte fields are documented as standard base64 strings. The loader
unmarshals each known field directly into `[]byte`; Go's JSON decoder also accepts
numeric arrays for that destination type. Consequently, an invalid structural
type under the documented format is silently treated as usable evidence.
**Evidence:** Both `"djEuMi4z"` and `[118,49,46,50,46,51]` as the value of
`evidence_bytes.tag` loaded as `v1.2.3` without a format error. The array case is
outside the declared representation. This is a format-validation gap; the probe
did not demonstrate altered bytes or an incorrect remote observation.
**Expected behavior:** Known byte fields follow the documented base64-string
representation. Structural and encoding failures return `record_format` with
their available causes. Supported partial evidence and ignored additional fields
retain their existing behavior.
**Remediation:** Decode known byte-field values as strings explicitly, then use
the standard base64 decoder to recover their bytes, preserving decoding causes.
Keep version-1 behavior and version-2 lossless identity from R10. Update Stage 2
to describe both supported versions and the writer's version selection, linking
to the canonical format definition.
**Regression coverage:** Add known byte-field cases for valid base64, malformed
base64, numeric arrays, and other invalid structural types. Assert typed format
errors and retained causes for invalid inputs. Reuse the existing all-field
round trips, non-UTF-8 endpoint observation, partial-evidence, and unknown-field
tests rather than duplicating their fixtures.
## Implementation plan and verification follow-through
When implementing R14–R17, update [implementation.md](implementation.md) alongside
the corresponding changes:
- Stage 1: record the cause-based endpoint setup classification from R16.
- Stage 2: describe ordinary version-1 records and version-2 byte-preserving
records, including the structural validation from R17.
- Stage 6: align typed failures, human guidance, and structured diagnostics with
R16; retain the implemented JSON output-failure handling from R06.
- Stage 7: replace the original executable-only handoff instructions with R07's
module-selected/durable API-only approach, extended for vendoring and
cancellation by R14 and R15. Retain a usable native fallback and explicit
record selection.
- Stage 8: add the regression coverage assigned above, update affected canonical
documentation as behavior lands, and run the offline suite and compiled
facade/dependency checks after assembly. Review local links and prose changes.
Preserve earlier completion evidence and distinguish these follow-up corrections
from the original completed implementation. Add completion markers as the new
issues are addressed. The corrections retain the existing public facade,
publication completion point, separate recording failures, and operator-owned
recovery choices; they introduce no consumer pilot, intermediate release, or
external review gate.