From e43df4416e0ed987349b2ea04a2c68c25e552772 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Fri, 9 Oct 2026 03:34:31 +0000 Subject: [PATCH] Remove completed roadmaps and repair the benchmark link --- docs/internal/gotool.md | 4 +- docs/roadmap/implementation.md | 685 --------------------------- docs/roadmap/performance.md | 122 ----- docs/roadmap/promptkit-bug-report.md | 150 ------ docs/roadmap/remediation.md | 179 ------- 5 files changed, 2 insertions(+), 1138 deletions(-) delete mode 100644 docs/roadmap/implementation.md delete mode 100644 docs/roadmap/performance.md delete mode 100644 docs/roadmap/promptkit-bug-report.md delete mode 100644 docs/roadmap/remediation.md diff --git a/docs/internal/gotool.md b/docs/internal/gotool.md index c34f035..ce47a33 100644 --- a/docs/internal/gotool.md +++ b/docs/internal/gotool.md @@ -19,8 +19,8 @@ failed output command returns both those bytes and an error containing the diagnostic text. The capture remains in memory for the duration of the child; the component benchmark compares one 8 MiB write with the same output in 32 KiB writes, forwarding to `io.Discard`. Smaller writes incur buffer-growth -allocations. The [workload-specific results](../roadmap/implementation.md#output-chunking-follow-up) -record total allocated bytes per operation, excluding subprocess overhead and +allocations. The [output benchmark](../../internal/gotool/performance_test.go) +measures total allocated bytes per operation, excluding subprocess overhead and diagnostic strings. These totals count all allocations, including superseded buffers. The returned slice retains the final buffer holding the complete output. Peak live memory depends on buffer growth, garbage collection, and other command diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md deleted file mode 100644 index e8ae3d4..0000000 --- a/docs/roadmap/implementation.md +++ /dev/null @@ -1,685 +0,0 @@ -# Computational Efficiency Implementation Plan - -**Status:** Stages 1–11 complete; [review remediation](remediation.md) complete. - -## Purpose And Baseline - -This plan turns the [performance roadmap](performance.md) into ordered work for -a Luna coding agent. The inspected baseline is commit -`ff90510c380b47a23d0ac1cd8c314b1c4366a15b`; this planning change updates only -the two roadmap documents. Recheck the worktree and relevant code before each -stage because subsequent stages build on earlier changes. - -Execute Stage 1 through Stage 11 in order. Each stage is one focused -implementation prompt. Complete its changes, applicable validation, canonical -documentation updates, and local commit under the -[agent policy](../policy/agents.md), then proceed. The sequence ends with a -results report for review. Consumer pilots, hosted operations, and intermediate -releases are outside this implementation sequence. - -Read [development.md](../development.md) and its applicable policy guide. -Apply the [architecture policy](../policy/architecture.md), -[testing policy](../policy/testing.md), and -[documentation policy](../policy/documentation.md). Use graph tools for code -discovery as instructed by `AGENTS.md`. - -## Selected Design And Scope - -| Area | Selected implementation | -| --- | --- | -| Child process | A small piped-process helper in `internal/process`, sharing existing command setup and error wrapping. | -| Git blobs | One lazy `git cat-file --batch` session per candidate tree, with one request/response exchange at a time. | -| Raw file bytes | Caller-owned bytes for regular-file reads; immutable symlink strings cached by object ID. | -| Repeated checks | Compact formatting outcomes, parsed Markdown documents, and replacement summaries cached by object ID within their check. | -| Artifact hashing | Data-only declarations followed by one existing local preflight verification; retain build hashing and missing-upload verification. | -| Go output | Separate status and captured-output methods; transfer completed buffers without copying. | -| Runtime version | A standard-library `sync.OnceValue` cache for the fallback, with an immediate injected-version return. | - -These are internal implementation choices. Preserve public facade signatures, -configuration, CLI semantics, selected check ordering, callback sequencing, -findings, failure causes, publication guards, and delivery recovery. Existing -public contracts remain authoritative. New helpers expose only the internal -operations needed here. File-size selection and existing supported inputs keep -their current semantics. - -The Git session and checker caches belong to one validation operation. Existing -sequential check execution remains sufficient for this design. The runtime -fallback is immutable process data and uses the standard library's concurrent -initialization mechanism. - -## Stage 1: Measure Candidate Content And Validation - -**Owners:** Benchmark files in `internal/validation` and root facade fixtures. - -**Completion:** Complete. Measurement-only benchmarks cover tree metadata, -content reads and formatting for 1,000 unique or repeated-ID Go blobs in loose -and packed repositories, repeated links to identical 1 MiB Markdown documents, -retained content after processing, and a real offline facade `Check`. Fixtures -verify committed bytes, unique/repeated object IDs, link counts, and Git pack -state. The facade fixture selects Go formatting, local-replacement inspection, -and Markdown while using test-owned module and build caches. - -### Baseline Measurement Record - -The baseline is commit `ca571aca4f2fd43798174a6d0d7c5a588b5347c9`, before adding -measurement support. Results use Go `go1.26.8 linux/amd64`, Git `2.53.0`, and a -Linux x86_64 Intel Core i5-7260U at 2.20 GHz. The command was -`go test ./internal/validation . -run '^$' -bench '^BenchmarkPerformance' -benchmem -count=3`. -Each candidate-content operation used a fresh empty content cache with tree -metadata prepared outside its timer. The loose fixtures had loose objects and -no pack; packed fixtures were checked after `git gc --aggressive --prune=now`. -The unique Go fixture contains 1,000 distinct, formatted files totaling 49,000 -bytes. The repeated-ID fixture contains 1,000 paths sharing one 15-byte blob. -The Markdown fixture contains two identical 1 MiB documents, a 47,780-byte -index with 1,000 repeated links, and 2,144,932 bytes processed per check. The -complete-check module contains 906 bytes of Go/module input. Its `GOMODCACHE` -and `GOCACHE` are test-owned temporary directories, and module network access -is disabled; the build cache is warmed before timing. - -| Measurement | Baseline observations | Final comparison | -| --- | --- | --- | -| Tree metadata, 1,000 entries | Loose: 27.5–28.2 ms; packed: 25.0–28.1 ms. About 1.28 MB allocated per operation. | Loose: 33.0–35.1 ms; packed: 7.43–15.10 ms. About 1.28 MB allocated. | -| Unique-blob reads, 49,000 bytes | Loose: 1.86–1.96 s; packed: 2.57–3.08 s. About 34.57 MB allocated per operation. | Loose: 89.8–126.3 ms; packed: 41.5–61.0 ms. About 530 KB allocated. | -| Repeated-ID reads, 15,000 bytes across 1,000 paths | Loose: 2.12–3.00 ms; packed: 3.42–7.75 ms. About 50.7 KB allocated per operation. | Loose: 84.8–99.5 ms; packed: 22.3–22.9 ms. About 480–483 KB allocated; slower than baseline because raw file bytes are deliberately not retained between reads. | -| Formatting, 1,000 unique files | Loose: 1.87–2.94 s; packed: 2.12–4.55 s. About 36.9 MB allocated per operation. | Loose: 111.9–114.6 ms; packed: 56.2–59.1 ms. About 2.89 MB allocated. | -| Formatting, 1,000 paths sharing one object | Loose: 4.52–12.66 ms; packed: 6.11–8.62 ms. About 1.49 MB allocated per operation. | Loose: 2.58–2.81 ms; packed: 2.07–2.13 ms. About 121 KB allocated. | -| Markdown links and parsing | 19.4–21.5 ms for two identical 1 MiB documents and 1,000 links; about 7.67 MB allocated per operation. | 10.90–11.14 ms for the same fixture; about 2.29 MB allocated. | -| Facade check, standard Go suite only | 385–690 ms; 906 module bytes; 336–340 KB allocated in the benchmark process. | 374.0–386.9 ms; 906 module bytes; 336–337 KB allocated in the benchmark process. | -| Facade check with selected file checks | 528–634 ms; 906 module bytes; 632–646 KB allocated in the benchmark process. | 379.7–391.8 ms; 906 module bytes; 503–513 KB allocated in the benchmark process. | -| Retained content after formatting and Markdown parsing | 1,385,040–1,385,616 bytes above the post-GC metadata baseline, with the candidate and Markdown cache kept alive. | 180,384–198,000 bytes above the post-GC metadata baseline, with the same caches kept alive. | - -Raw output is in temporary storage at `/tmp/releasekit-performance-baseline.txt`. -The shared machine produced substantial timing variation, especially for packed -content and the full check; allocations and retained-byte observations were -more stable. `B/op` for facade checks measures allocations in the benchmark -process and does not include allocations inside its Go subprocess. These are -workload-specific component measurements, not runtime guarantees. - -1. Add benchmarks named with the `BenchmarkPerformance` prefix. Reuse local Git - fixture patterns; adapt shared helper parameters to `testing.TB` only where - a benchmark needs them. -2. Measure content access and formatting for 1,000 unique regular Go blobs. - Include loose and packed objects and a repeated-object-ID case. Measure - Markdown checking with a 1 MiB document, repeated links to it, and identical - contents under different directories. -3. Measure a complete facade `Check` using a temporary committed Go module with - a valid release note, Go formatting, local replacements, and Markdown selected. - Keep its Go suite real and offline. Prepare fixtures outside timed work. - Report allocations and bytes processed, separating content access, tree - metadata, and Go test/compile work. -4. Add one diagnostic retained-memory observation after file processing, with - the candidate and document cache kept alive, to expose raw-content retention. - Keep GC/profile collection outside timing loops. -5. Create a measurement record in this document for baseline and final results. - Include commit, Go/Git versions, fixture sizes, packed/loose state, and cache - state. Keep raw benchmark output or profiles in temporary storage. - -**Validation and completion:** Run these benchmarks with `-run '^$'`, -`-bench '^BenchmarkPerformance'`, `-benchmem`, and `-count=3`. Verify the -fixtures inspect the intended bytes. Synthetic modules use test-owned module -caches under the developer guide. Timing ratios remain reported measurements; -behavioral tests protect content, failures, and ownership. This stage changes -measurement support only. - -## Stage 2: Measure Delivery, Output, And Runtime Lookup - -**Owners:** Benchmark files in `internal/delivery`, `internal/gotool`, -`internal/runtimeinfo`, and root delivery fixtures. - -**Completion:** Complete. Measurements cover declarations and local -verification of two deterministic 16 MiB files, prebuilt facade delivery for -fresh uploads and matching retries, combined output capture and forwarding at -4 KiB and 8 MiB, and injected and real metadata version access. The local HTTP -fixture hashes and counts incoming multipart bytes incrementally and streams -matching files from immutable fixture data. It verifies uploaded identities and -that retries match all three declared assets without uploading again. - -### Delivery, Output, And Runtime Baseline - -The baseline is commit `29e1bab`, before production refactoring, using the same -Go, Git, operating system, processor, benchmark command, and three repetitions -recorded above. Artifact declaration and preflight fixtures contain two -deterministic 16 MiB files (33,554,432 binary bytes per operation); setup and -initial writes are outside the timers. Declaration currently hashes each file, -and declaration plus local verification therefore measures both hashing passes. -The facade benchmark uses a committed tagged source fixture and prebuilt -artifacts, so its time includes local source checks, hashing, HTTP requests, and -streamed body comparison or upload, but no Go compilation. Fresh hosted state is -reset outside the timed region for every operation. Retry state is prepared -before timing. The fixture server retains only digest and size metadata for -uploads and streams known matching bytes directly from shared fixture content. - -Output measurements call the combined writer directly, with forwarding to -`io.Discard`, then capture its completed buffer. Each baseline and final -comparison writes its complete input in one call. The baseline includes the -completed-output copy. Runtime fallback measurements call `Version()` repeatedly -against real build metadata; injected access measures the selection helper's -injected-version branch. The benchmark process reports its own allocations, -including local HTTP fixture work; streamed artifact bodies are not buffered by -the fixture server. - -| Measurement | Baseline observations | Final comparison | -| --- | --- | --- | -| Artifact declarations, two 16 MiB files | 94.3–95.4 ms; 33,554,432 bytes processed; about 68.9 KB allocated. | 11.4–19.4 μs; 33,554,432 declared bytes; 2,296 B and 33 allocations. Declaration no longer reads or hashes file contents. | -| Declarations plus local verification | 186.7–188.4 ms; 33,554,432 binary bytes per each of two passes; about 136.2 KB allocated. | 94.3–95.3 ms; one 33,554,432-byte verification pass; about 69.7 KB and 58–59 allocations. | -| Facade delivery, fresh uploads | 429–447 ms; 32 MiB binary payload, two files plus manifest; 1.43–1.46 MB allocated. | 331.9–415.3 ms; same payload; 1.38–1.39 MB allocated. | -| Facade delivery, matching retry | 309.1–310.0 ms; 32 MiB of local verification plus streamed remote comparisons; 1.21–1.22 MB allocated. | 210.7–217.5 ms; same local verification and streamed comparisons; 1.15 MB allocated. | -| Combined output, one 4 KiB write and `io.Discard` | 1.82–1.87 μs; 8,256 B and 3 allocations per operation. | 0.94–1.06 μs; 4,176 B and 2 allocations. | -| Combined output, one 8 MiB write and `io.Discard` | 2.02–2.36 ms; 16.78 MB and 3 allocations per operation, including the completed capture copy. | 1.73–2.22 ms; about 8.39 MB and 2 allocations. | -| Injected version selection | 0.745–0.754 ns; 0 B and 0 allocations. | 0.754–0.758 ns; 0 B and 0 allocations. | -| Repeated real metadata fallback | 1.438–1.474 μs; 1,184 B and 7 allocations per operation. | 4.59–4.65 ns after cache initialization; 0 B and 0 allocations. Separate initialization: 1.54–2.14 μs; 1,264 B and 9 allocations. | - -The delivery observations include local HTTP and filesystem costs specific to -these fixtures. The output component benchmark does not include child startup; -the real delivery benchmark does include HTTP request handling. These results -describe measured workloads and do not establish runtime guarantees. - -### Output Chunking Follow-Up - -The R3 follow-up extends `BenchmarkPerformanceCombinedOutput` with 32 KiB -writes. Three repetitions on Go 1.26.8, linux/amd64, and the Intel Core i5-7260U -used the completed writer at `7d92c47` with the extended benchmark: - -```sh -GOPROXY=off GOSUMDB=off GOTOOLCHAIN=local go test ./internal/gotool -run '^$' -bench '^BenchmarkPerformanceCombinedOutput$' -benchmem -count=3 -benchtime=1s -``` - -| Workload, forwarding to `io.Discard` | Elapsed time | Total allocated bytes/op | Allocations/op | Returned output length | -| --- | --- | --- | --- | --- | -| One 8 MiB write | 1.49–1.56 ms | 8,388,697–8,388,702 | 2 | 8,388,608 bytes | -| 32 KiB writes totaling 8 MiB | 2.56–2.93 ms | 16,744,532–16,744,541 | 10 | 8,388,608 bytes | - -Both workloads verify complete captured and forwarded bytes outside the timed -region. Measurements include capture and forwarding to `io.Discard`, excluding -subprocess costs and failure diagnostic strings. Total allocated bytes count -all allocations during an operation, including superseded buffers. The returned -slice retains the final backing buffer; its logical output length is 8 MiB in -both cases. Peak live memory was not measured and depends on buffer growth, -garbage collection, and other command work. These results describe the measured -workloads and establish no numerical runtime guarantee. Both paths transfer the -completed buffer without the former final copy. - -### Final Comparison Record - -The final benchmark run measured commit `42569b4`, before this Stage 11 record -was committed, using Go `go1.26.8 linux/amd64`, Git `2.53.0`, and Linux x86_64 -on an Intel Core i5-7260U at 2.20 GHz (`devbench-02`). It used the same -`-run '^$' -bench '^BenchmarkPerformance' -benchmem -count=3` boundaries and -the same temporary fixtures and cache setup as the baseline. Raw final output -is in `/tmp/releasekit-performance-final-stage1.txt`, -`/tmp/releasekit-performance-final-delivery.txt`, -`/tmp/releasekit-performance-final-output.txt`, and -`/tmp/releasekit-performance-final-version.txt`. - -The unique-blob batch reader reduced measured reads and formatting from seconds -to tens or hundreds of milliseconds and cut their allocations by roughly an -order of magnitude or more. Formatting repeated object IDs also improved in -both time and allocations because parsed outcomes are reused. Raw repeated-ID -reads regressed: 1,000 loose reads measured 84.8–99.5 ms and 480–483 KB, versus -2.12–3.00 ms and 50.7 KB at baseline; packed reads measured 22.3–22.9 ms versus -3.42–7.75 ms. This workload rereads small object bodies through the batch -session, while regular-file bytes intentionally remain caller-owned and are -not cached by the tree. That is the measured time cost of limiting retained -source bytes; scoped read reuse could be reconsidered if repeated raw reads -become a common workload. - -Retained processed content fell from about 1.39 MB to 180–198 KB with the -candidate and parsed Markdown cache kept alive. The Markdown benchmark now -allocates about 2.29 MB instead of 7.67 MB and measured 10.90–11.14 ms versus -19.4–21.5 ms at baseline. The complete offline facade check with selected file -checks measured 379.7–391.8 ms and 503–513 KB in the benchmark process, versus -528–634 ms and 632–646 KB at baseline. The Go-suite-only check remains -374.0–386.9 ms; its real test/compile subprocess is the main complete-check -cost, and benchmark-process allocations exclude subprocess allocations. - -Artifact declarations measured 11.0–11.5 µs and 2,296 B for two 16 MiB files. -Declarations plus current local verification measured 92.4–93.3 ms and about -69.7 KB, against 186.7–188.4 ms and 136.2 KB at baseline. The current flow -therefore removes one 32 MiB local read/hash pass while retaining one local -verification before hosted requests. In the prebuilt local-HTTP facade fixture, -fresh uploads measured 331.9–415.3 ms and matching retries 210.7–217.5 ms. A -retry still reads and verifies local assets and compares streamed remote bytes; -those fixture and remote-comparison costs remain in the complete workflow. - -For single-write workloads, output capture allocations fell from 8,256 B to -4,176 B for 4 KiB, and from about 16.78 MB to 8.39 MB for 8 MiB. The 8 MiB timing ranges overlap, while one -allocation and the completed-buffer copy are removed. The returned capture -remains caller-owned in memory; a failed output command can also retain its -diagnostic string. Warm runtime fallback access measured 4.59–4.65 ns and zero -allocations after a separate 1.54–2.14 µs, 1,264 B initialization. The -linker-injected selection remains allocation-free. - -Some microbenchmark timings varied substantially between repeated runs on this -shared machine; allocations were steadier. The local delivery server and -filesystem are workload fixtures, not a hosted service. These figures -describe only the recorded toolchain and workloads and do not imply numerical -runtime guarantees. Raw repeated-ID read retention and disk-backed large -diagnostics remain follow-up options if their measured workload frequency -justifies their memory and lifecycle tradeoffs. - -1. Extend the same benchmark prefix to declaration/preflight processing of two - deterministic 16 MiB artifact files. Measure prebuilt facade `Deliver` - against a local HTTP fixture for fresh uploads and matching retries. - Prepare files outside timed work and reset hosted state for each fresh run. - Stream binary bodies through the fixture's digest/size checks and serve - matching files incrementally so fixture buffering does not obscure allocation - improvements. Describe fixture processing costs in the workflow results. -2. Benchmark completed Go-output capture with modest output and 8 MiB diagnostics, - including forwarding to `io.Discard`. Reuse the combined writer for component - measurements and existing child fixtures for any command measurements. -3. Measure injected version access and repeated real metadata-fallback access. - Preserve benchmark coverage of initialization and repeated access when the - implementation later adds caching. -4. Record time, allocations, bytes processed, and workload/cache settings in the - measurement record. Run the benchmark set with the same flags and repetitions - as Stage 1. Verify delivered bytes and matching-retry behavior. - -**Validation and completion:** Measurements are repeatable offline using local -collaborators and temporary files. Fixture state changes occur outside timed -work where they are setup. These two measurement stages establish the baseline -before production refactoring begins. Default tests remain independent of real -credentials and mutable external services. - -## Stage 3: Add The Piped Child-Process Boundary - -**Owner:** [internal/process](../../internal/process/process.go) and its tests. - -**Completion:** Complete. `Run` and `StartPiped` share command construction. -Piped children expose caller-owned input and output pipes, cancellation, and a -single underlying wait/reap while preserving parent-context and exit errors. -Tests cover argument, directory, and environment propagation; an exchange that -responds before input EOF; synchronized cancellation; startup failure; and a -nonzero exit. The package inventory now describes the pipe lifecycle. - -1. Factor existing command construction and command-error wrapping into small - private helpers shared by `Run` and a new internal `StartPiped` operation. - Preserve arguments, directory, environment overlay/removal, stderr routing, - context handling, and wrapped exit errors. -2. `StartPiped` accepts an `Invocation` for command settings and stderr; stdin - and stdout are supplied by pipes. Return a small process handle containing - the input writer and output reader, plus `Cancel` and `Wait` operations. - Keep the underlying `exec.Cmd` private. Use a derived cancellable context so - the owner can abort a blocked exchange without cancelling its caller. -3. Close acquired pipes on setup/start failure. The caller owns input closure, - stdout consumption, and one wait/reap after a successful start. `Cancel` - interrupts the child; `Wait` retains the current context/exit error semantics. - Read stdout to completion before a normal wait. Release the derived context - when the process is reaped; abort paths close active pipes before waiting. -4. Extend the existing test executable helper for an exchange that returns - output before stdin EOF. Test real argument/environment propagation, normal - EOF completion, cancellation after a readiness signal, start failure, and - a nonzero child exit. Synchronize cancellation with the child instead of - depending on sleep duration. - -**Validation and completion:** Run `internal/process` tests. Existing `Run` -behavior still passes, and the piped helper supports repeated exchanges followed -by deterministic cleanup. Update the process inventory description when needed. - -## Stage 4: Implement The Git Batch Reader - -**Owners:** A focused reader file and tests under `internal/git`, reusing -[source.go](../../internal/git/source.go). - -**Completion:** Complete. `BlobReader` lazily starts one local-only -`git cat-file --batch` process, serializes request/response exchanges, and -returns caller-owned body bytes after validating the object ID, blob type, -representable size, and framing. Missing objects produce contextual errors -without invalidating a healthy session. Broken exchanges and cancellation close -the pipes and reap the child, preserving the original failure, Git stderr, and -process causes. Existing standalone blob and committed-file readers remain in -place. Tests cover repeated loose and packed reads, empty and binary bodies, -missing objects, partial clones, malformed/truncated responses, cancellation, -and child failures. - -The process-reuse benchmark used one 9-byte blob. A new reader per read took -1.98–2.24 ms and allocated 37.9 KB in 190 allocations per operation; a shared -session took 57–82 μs and allocated 425 B in 8 allocations. The fresh-session -measurement includes process startup and clean close per read. The shared-session -timer includes lazy startup on its first read; final cleanup is outside the -timer. This is a fixture-specific measurement, not a subprocess-count contract. - -1. Add an internal `BlobReader` that stores a repository directory and parent - context. Create its process lazily on the first content request through - `process.StartPiped` with `git cat-file --batch`. -2. Use the same child-only `GIT_NO_LAZY_FETCH=1` and `GIT_ALLOW_PROTOCOL=` - settings as `readLocalObjects`. Keep existing standalone `ReadBlob`, - `ReadCommitFile`, and batched metadata functions for their current callers. -3. Implement `Read(ctx, objectID) ([]byte, error)` as one serialized exchange: - validate the ID using the existing helper, write its newline-terminated - request, read the response header, validate object/type/size, read exactly - the declared bytes, and consume the framing newline. Compare hexadecimal - IDs without narrowing the existing accepted case conventions. -4. Allocate the response body once and transfer it to the caller. Use buffered - header reading and `io.ReadFull`; blob contents can contain arbitrary bytes - and newlines. Read only requested bodies and avoid whole-session buffering. - Size conversion errors reflect representability on the host. -5. Treat Git's missing-object response as a contextual inspection error. Preserve - stderr and process causes when the child fails. A broken protocol or I/O - exchange aborts that session and reaps the child; return the original cause. -6. Honor the read context during blocked I/O, for example by registering - `context.AfterFunc` to cancel the child and stopping that registration after - the exchange. Coordinate callback completion when the stop function reports - it has begun; stopping alone does not wait for that callback. If cancellation - races completion, preserve its cause and abort the session. Read captured - stderr after reaping to avoid writer races. -7. Add idempotent `Close`: an unused reader has nothing to close; a healthy - reader closes stdin, finishes stdout consumption, and waits; an aborted - reader closes pipes, cancels, and waits. Preserve unexpected exit failures; - deliberate abort cleanup retains the original read failure. - -**Validation and completion:** Test repeated reads from loose and packed Git -objects, empty/binary bodies, and missing objects. Reuse the partial-clone test -approach to verify absent objects cause errors with no transport or object-store -population. Use a small child fixture for truncated framing and cancellation -mid-response. Assert content and cleanup outcomes; benchmark process reuse -without making a subprocess count a public contract. Run Git and process tests. - -## Stage 5: Give Candidate Validation A Scoped Reader - -**Owners:** [tree.go](../../internal/validation/tree.go), -[validation.go](../../internal/validation/validation.go), and tree fixtures. - -1. Give `CandidateTree` one lazy `BlobReader` and an internal `Close` method. - `ReadCandidateTree` still reads metadata only. Route blob misses through the - reader while temporarily retaining the existing byte-cache behavior. -2. The shared validation executor owns closure on every return after loading - a tree. Complete cleanup before product checks or build callbacks run. - Metadata-only selections leave the content reader unstarted. -3. Preserve an earlier check's `Failure` and cancellation cause during cleanup. - If cleanup reveals an unexpected child failure after successful checks, - return a contextual `Failure` attributed to the last content-reading check. - Track that check in the executor; normal intentional cancellation during - cleanup adds no independent failure classification. -4. Update direct tree users in tests and benchmarks to register cleanup or close - explicitly. Update synthetic tree fixtures to support lazy reader creation. - Keep tree paths, source capture, symlink resolution, and check order intact. - -**Validation and completion:** Run validation, candidate, publication, and facade -candidate tests. Cover early check failure and cancellation with cleanup, and -preserve workspace/file-size metadata behavior. Re-run content benchmarks with -reader startup and closure included in the content-operation timing. Update -[validation mechanics](../internal/validation.md) for the landed lifecycle. - -**Completion:** Complete. `CandidateTree` owns a lazy batch reader and exposes -idempotent cleanup. The shared executor closes it on every post-load return, -before product checks or build callbacks; metadata-only runs leave it unstarted. -Earlier check failures and cancellation remain primary, while an unexpected -reader exit after successful content checks becomes a contextual failure for -the last content-reading check. Direct tree users close their readers. The -temporary object-ID byte cache remains until Stage 6. Validation, candidate, -and root facade tests passed. Content benchmark timings now include reader -startup and close (Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U, -three repetitions): unique reads took 59.8–77.9 ms loose and 22.8–43.0 ms -packed; unique-file formatting took 114–188 ms loose and 33.7–55.7 ms packed; -repeated-ID reads took 2.18–6.45 ms, repeated-ID formatting took 4.75–6.62 ms, -and the Markdown workload took 15.0–16.9 ms. Compared with the Stage 1 timings, -these measurements reflect the shared Git process and include its lifecycle; -timing varies on the shared machine. - -## Stage 6: Release One-Use Source Bytes And Reuse Formatting Results - -**Owners:** `internal/validation/tree.go`, `validation.go`, and destination tests. - -1. Remove the tree's general raw-blob cache. A regular-file `ReadFile` returns - its newly read, caller-owned body without a cache copy. Its GoDoc describes - that ownership and the scoped reader; independently repeated raw reads can - request the same object again. -2. Cache committed symlink targets as owned immutable strings keyed by object - ID. Add a private target-reading helper used by destination resolution and - module-symlink handling. Preserve original target bytes and traversal order. - Explicit `ReadFile` of a symlink still returns independently owned bytes. -3. In `checkGoFormat`, cache the formatted/unformatted result for successfully - inspected objects by object ID. Release source and formatted bytes from - retained references after comparing them. Emit a finding at every selected - unformatted path, including duplicate-content paths. Parse/read errors still - stop with the current path and cause; check cancellation for each entry. -4. Adapt tests that construct the old cache directly. Preserve observable - isolation: mutating returned bytes cannot alter later validation or reads. - Reuse destination-cycle and repeated-symlink tests for shared target strings. - -**Validation and completion:** Run validation tests and formatting/destination -benchmarks. Confirm many unique Go files no longer remain as retained source -buffers after formatting, and duplicate-content files retain per-path findings. -Record the tradeoff for explicit repeated raw reads. Update validation mechanics -with the new ownership and reuse choices. - -**Completion:** Complete. Regular-file reads now return independent bytes and -are not retained by `CandidateTree`; repeated explicit reads therefore transfer -the blob again. The tree caches owned symlink-target strings by object ID, and -explicit symlink reads still return independent byte slices. Go formatting -caches only successful formatted/unformatted outcomes by object ID, checks -cancellation per path, and reports every unformatted path even when contents -repeat. Validation tests and the formatting/Markdown destination benchmarks -passed. On the same Go 1.26.8, Git 2.53.0, Linux x86_64 Intel Core i5-7260U -setup, three repetitions measured unique-file formatting at 75.0–75.9 ms loose -and 37.9–42.5 ms packed, repeated-ID formatting at 1.97–2.17 ms, and the -Markdown workload at 16.3–17.0 ms. Formatting used about 2.89 MB/op for unique -files and 121 KB/op for repeated IDs. The retained-memory observation was -129,496 bytes with the candidate and Markdown cache alive, compared with -1,385,040–1,385,616 bytes in the baseline fixture. This retains Markdown parser -data by path until Stage 7; it is a workload-specific heap observation. - -## Stage 7: Reuse Owned Markdown And Module Results - -**Owners:** [markdown.go](../../internal/validation/markdown.go), -[markdown_check.go](../../internal/validation/markdown_check.go), and -[local_replacements.go](../../internal/validation/local_replacements.go). - -1. Key parsed Markdown reuse by resolved regular blob object ID. Keep a path - lookup when useful for callers, backed by the shared document records. - Relative link resolution and findings use the current source path; a shared - parsed document stores content-relative locations, links, and anchors only. -2. Give retained link destinations and titles their own string storage, using - `strings.Clone` at extraction where needed, so parser/source views cannot - keep a large raw body alive. Cache parsed records, release the AST and source - from retained references, and preserve heading collision rules, image behavior, - and use-site locations. -3. Refactor module-declaration selection to identify the resolved regular entry - before reading its body. After successful parsing, cache a compact list of - filesystem replacements by resolved object ID: target string and source line. - Own retained target strings and release raw bytes and the parsed syntax tree - from retained references. -4. Apply each cached replacement summary to the current declaration path when - constructing findings. Resolve each symlink declaration in its own path - context. On read or parse failure, preserve the current declaration's path - and underlying cause; preserve findings order and versioned-replacement rules. - -**Validation and completion:** Run Markdown/module and facade validation tests. -Add behavioral cases for identical Markdown in different directories and two -module declarations resolving to one object, including multiple findings with -correct paths/lines. Reuse parser and boundary/cycle coverage. Re-run -large-document and repeated-content benchmarks and retained-memory observations. -Update the validation document's cache description to match implemented ownership. - -**Completion:** Complete. Parsed Markdown documents are shared by regular blob -object ID with path lookups; destinations and titles own their string storage, -and link locations remain content-relative while resolution uses each current -source path. Module declarations resolve to a regular committed entry before -reading; successfully parsed filesystem replacements are cached as owned -target/line summaries by body object ID and applied to every declaration path. -Validation, candidate, and facade tests passed, including shared Markdown -resolved from distinct directories and two module symlinks receiving the -correct findings at both paths and lines. Three large-document runs measured -10.85–11.22 ms and 2.29 MB/op for two identical 1 MiB documents and 1,000 -repeated links. Three retained-memory observations measured 110,568–120,280 -bytes with the candidate and parsed Markdown cache alive. These fixture results -include the retained per-document links, anchors, and candidate metadata; they -are workload-specific observations on the recorded shared machine. - -## Stage 8: Consolidate Delivery Preflight Hashing - -**Owners:** [delivery declarations](../../internal/delivery/assets.go), -[delivery orchestration](../../internal/delivery/delivery.go), and their tests. - -1. Make `declaredBuildAssets` perform structural declaration checks, stat files, - reuse the recorded artifact SHA-256, and construct expected assets and the - manifest. Remove its file-opening/hash pass. Keep target, filename, regular - file, declared digest format, and manifest validation at the appropriate - existing owner. -2. Keep one `gitea.VerifyLocalAssets` call in `Deliver` after declaration and - build-identity checks and before hosted requests. This verifies actual bytes - and sizes against recorded identities for both managed and prebuilt assets. -3. Keep builder hashing, CLI receipt verification, and missing-upload preflight - in their current phases. Managed delivery reuses its fresh build metadata for - declarations; prebuilt delivery reuses its caller's recorded digests only as - expected identities until current bytes have been verified. -4. Move the declaration helper's tampering assertion to delivery preflight's - behavioral owner. Verify same-size changed contents fail before any hosted - request. Reuse source-only, retained-prebuilt, managed cleanup, conflict, - partial-upload, and retry coverage. - -**Validation and completion:** Run delivery, Gitea, facade delivery, and CLI -receipt/delivery tests. Re-run artifact and complete delivery benchmarks. A fresh -missing binary has three hash passes including a managed build, or two within -prebuilt delivery; these are explanatory measurements of the chosen flow. -Streaming upload, exact remote comparison, and later upload verification remain -intact. Update the inventory's delivery mechanics where needed. - -**Completion:** Complete. `declaredBuildAssets` now performs target/name, -filename, regular-file, and stat checks, then uses each recorded SHA-256 to -build expected assets and the manifest. The separate local verification in -`Deliver` remains after build identity checks and before hosted requests; it -checks current bytes and sizes for managed and prebuilt artifacts. A same-size -prebuilt mutation now fails at this delivery preflight with zero hosted -requests. Delivery, Gitea, facade, and CLI tests passed. On the recorded Go/Git -and machine setup, three artifact runs measured declarations at 11.3–15.4 µs -(2,296 B/op) and declarations plus one local verification at 121–136 ms -(about 69.7 KB/op). The Stage 2 comparison was 94.3–95.4 ms and 186.7–188.4 ms -respectively, each including the declaration hash pass; the new second result -therefore removes one 32 MiB file-read/hash pass from this workload. Three -facade runs measured fresh uploads at 485–520 ms and matching retries at -214–311 ms. These complete-workflow timings overlap the recorded baselines -within shared-machine variation and remain dominated by the local HTTP fixture, -upload, and remote comparison work. - -## Stage 9: Transfer Completed Go Output Without Copies - -**Owners:** [gotool.go](../../internal/gotool/gotool.go), validation/build callers, -and Go tool fixture tests. - -1. Give `Tool` a status-returning `Run(...args) error` and an output-returning - `Output(...args) ([]byte, error)`, with one private execution path. Keep their - existing directory, mode, target, writer, and argument inputs. -2. On completion, a status-only success releases its capture. An output-consuming - call receives the completed buffer's backing bytes with ownership transferred - from the private writer. Replace `combinedWriter.Bytes` copying with a small - synchronized ownership-transfer operation used after child output has finished. -3. Preserve complete failure diagnostics, original causes, combined-output - synchronization, and forwarding errors. If `Output` fails, return its captured - bytes and diagnostic error as before. Status failures format the same useful - diagnostics without creating an unnecessary intermediate snapshot. -4. Convert validation test/vet/compile and artifact-build calls to status-only - `Run`. Use `Output` for compiler identification. Adapt preparation failure - formatting to the same completed-buffer ownership discipline, preserving its - network-capable environment. Leave configuration discovery's separate stdout - JSON and stderr handling intact. -5. Keep full capture in memory. Measure remaining large-log retention and record - the disk-backed alternative as follow-up evidence, as scoped by the roadmap. - -**Validation and completion:** Extend existing process/Go fixtures for successful -capture, large failure output, cancellation, and a failing output writer. Verify -forwarded bytes and meaningful error content/cause. Run Go tool, validation, -build, preparation, and affected CLI tests. Re-run output benchmarks and update -[Go execution mechanics](../internal/gotool.md). - -**Completion:** Complete. `Tool.Run` now returns status only, while `Tool.Output` -returns combined stdout/stderr with ownership of the completed capture buffer; -the shared private execution path preserves settings and arguments. Status -success drops its capture, and both status and output failures retain complete -diagnostics and underlying process or output-writer causes. Validation and -artifact compilation use `Run`; compiler identification uses `Output`. -Preparation retains its network-capable environment and formats diagnostics -from the transferred buffer. Go tool fixtures cover forwarded output, large -failure output, cancellation, and writer failure. Go tool, validation, build, -facade, preparation, and CLI tests passed. The single-write combined-output -benchmark now measures 1.0–1.1 µs and 4,176 B/2 allocations for 4 KiB, and 1.51–1.56 ms and -8,388,699–8,388,703 B/2 allocations for 8 MiB. The Stage 2 8 MiB result was -2.02–2.36 ms and about 16.78 MB/3 allocations, including its capture copy. -The returned 8 MiB slice remains caller-owned in memory; failed Output also -creates diagnostic text. Disk-backed diagnostics remain a measured follow-up -option because avoiding their additional lifecycle and cleanup work has not -been justified here. - -## Stage 10: Cache The Runtime Metadata Fallback - -**Owners:** [runtimeinfo/version.go](../../internal/runtimeinfo/version.go) and -runtime/buildinfo tests. - -1. Preserve `injectedVersion` and its linker identity. `Version` checks and - returns injection first, then calls a fallback accessor created with - `sync.OnceValue`. -2. Keep the existing pure `version(injected, readBuildInfo)` selection helper. - Add a small private fallback factory accepting the metadata reader and - returning the cached accessor. Production supplies `debug.ReadBuildInfo`; - tests create independent cache instances with controlled metadata. -3. Reuse selection tests. Add repeated/concurrent access coverage with a - counting reader to verify reuse and stable results. Test unavailable and - development metadata fallback through isolated instances. Keep shared - production state immutable during tests. - -**Validation and completion:** Run runtime/buildinfo tests, the existing consumer -build-and-run injection test, and focused race validation of the fallback cache. -Run facade/dependency checks. Re-run version benchmarks after initializing the -cache and report initialization separately from repeated access. The runtime -facade and its dependency boundary retain their existing public shape. - -**Completion:** Complete. `Version` returns the linker-injected value before -touching fallback state, and a private factory wraps the existing pure -selection helper with `sync.OnceValue`. A concurrent counting-reader test -confirms one metadata read and stable results; isolated unavailable and -development cases continue to resolve to `dev`. Runtime, buildinfo, facade -dependency, consumer executable, and focused race tests passed. Three version -benchmark repetitions measured injected selection at 0.75–1.03 ns with no -allocations, fallback initialization at 1.53–1.74 µs with 1,264 B/9 allocations, -and warmed production fallback access at 4.54–4.91 ns with no allocations. -The original uncached metadata call measured 1.438–1.474 µs and 1,184 B/7 -allocations per access. The internal package inventory now records the cached -fallback behavior. - -## Stage 11: Verify The Complete Implementation And Record Results - -**Owners:** Affected tests and canonical internal documentation; roadmap status. - -1. Run the complete offline test suite and compiled facade/dependency checks - using the [developer guide](../development.md). Resolve remaining failures - within this performance scope. Reuse targeted race results where code is - unchanged; broaden them only for a concrete remaining concurrency risk. -2. Run the Stage 1 and Stage 2 benchmark set again with comparable toolchain, - fixtures, cache state, repetitions, and measurement boundaries. Compare - component and complete `Check`/`Deliver` results. Record memory and explicit - reread tradeoffs as well as time and allocation improvements. -3. Record final results alongside the baseline in this document. Explain the - remaining Go compilation/test cost, remote-comparison work, and diagnostic - retention where measurements show they matter. Separate measured effects - from expectations and include machine/workload limitations. -4. Check canonical internal docs describe landed reader lifetime, source/result - ownership, verification flow, output ownership, and runtime reuse. Preserve - existing public contracts; update a public document only if its explanation - became inaccurate. Check local links and prose whitespace. -5. Mark each stage's actual completion, update the feature roadmap status, and - commit the completed work under repository policy. End with a concise report - of changes, tests, performance results, and remaining follow-up opportunities. - Retain these planning documents with their results for the final review. - -**Completion evidence:** All five target improvements are implemented, relevant -existing behavior passes, the public facade checks pass, comparable measurements -are recorded, and the worktree is clean. Completion depends on observed behavior -and reviewed diffs; benchmarks establish improvements within their measured -scope without adding numerical runtime guarantees. - -**Completion:** Complete. The full offline suite (`go test ./...`) and compiled -facade/dependency check (`go run ./internal/apicheck`) passed. Focused race -validation passed for the runtime fallback, Go output adapter, and process -boundary. Targeted fixtures also cover cancellation and writer-failure causes. -The canonical internal docs describe Git reader lifetime and source ownership, -validation reuse and delivery verification, Go output ownership, and runtime -fallback reuse. Local links and Markdown whitespace were checked. All stages -are marked complete; this results document is retained for review. - -## Open Questions - -None required for this implementation. The plan selects scoped batch reading, -result-based reuse, preserved verification boundaries, in-memory diagnostics, -and cached runtime fallback. Disk-backed diagnostics remain a follow-up option -whose benefit and lifecycle costs can be assessed from the recorded output -measurements. diff --git a/docs/roadmap/performance.md b/docs/roadmap/performance.md deleted file mode 100644 index 21dbf14..0000000 --- a/docs/roadmap/performance.md +++ /dev/null @@ -1,122 +0,0 @@ -# Computational Efficiency Roadmap - -**Status:** All five target components implemented. The -[implementation record](implementation.md) owns completed stages and performance -results; the [remediation record](remediation.md) owns review findings and their -completion status. - -## Objective And Ownership - -Reduce the time, allocations, retained memory, and repeated I/O of routine -release operations. Improvements belong behind the existing `releasekit` and -`buildinfo` facades and follow the [architecture policy](../policy/architecture.md) -and [routine-release priorities](../adr/0002-routine-releases.md). - -This document owns the desired target state and its rationale. The -[implementation plan](implementation.md) owns stages, tasks, validation, and -implementation status. The [internal inventory](../internal/overview.md) and -focused internal documents own implemented mechanics; Go declarations and -GoDoc own public contracts. - -## Review Evidence - -The computational efficiency review examined commit `db9e24f` on 2026-10-09. -Temporary offline probes produced these local measurements: - -| Probe | Result | -| --- | --- | -| Read 1,000 unique committed Go blobs totaling 59,670 bytes through the existing path | 1.83–2.00 seconds across three runs. | -| Read and verify the same blobs through one Git batch command | 48–59 milliseconds across three runs. | -| Read a cached 1 MiB blob | One allocation of 1 MiB per read. | -| Resolve runtime version through the Go build metadata fallback in a test executable | Approximately 2 microseconds, 1,184 allocated bytes, and seven allocations per call. | -| Resolve an injected runtime version | Zero allocations. | - -Blob timings cover content access, with tree metadata discovery measured -separately. They exclude Go tests, compilation, formatting, and Markdown parsing. -The fixture contained small, loose Git objects. These are component measurements -on one machine; overall release gains depend on consumer workloads. The probes -were temporary and left the repository unchanged. - -Static review also identified repeated artifact hashing and unconditional Go -output copying. The [implementation record](implementation.md) contains their -baseline and completed-implementation measurements. - -## Desired Target State - -### Git Content Access - -Selected file checks share an operation-scoped Git content reader. Requests -remain lazy, use committed object IDs, and consume responses incrementally. -Subprocess startup is amortized across the operation. Metadata-only checks -continue to use the captured tree without starting a content reader. - -The reader preserves offline object access, cancellation, contextual inspection -errors, and explicit process cleanup. Each validation operation owns its reader -and reusable data independently. - -### Validation Memory And Reuse - -One-use regular-file bytes belong to their immediate consumer and are released -from retained references after inspection. Repeated work reuses immutable parsed -or compact results keyed by object ID within the relevant check. Reusable -symlink targets belong to the candidate tree. Link checks apply shared document -data in each source path's own context, preserving findings, source locations, -and relative destinations. - -This replaces retention of every inspected raw blob with data that supports -actual repeated work. Explicit repeated raw reads may reread an object through -the batch reader. The same bytes can be interpreted separately by different -checks; reuse within one interpretation preserves object-ID deduplication. - -### Artifact Verification - -Delivery declarations reuse recorded digests and collect file metadata. One -preflight verification checks current local bytes before hosted requests. This -combines adjacent declaration-time and asset-verification hash passes for both -managed and prebuilt delivery. - -Build-result hashing and upload preflight retain their separate responsibilities. -Prebuilt artifacts remain caller-owned. Exact remote content comparison, -targeted reconciliation, additive retries, and partial-delivery outcomes retain -their existing semantics under the [Gitea contract](../integrations/gitea.md). - -### Go Command Output - -Status-consuming commands return status and useful failure diagnostics. -Output-consuming commands receive ownership of completed captured bytes without -an additional full-buffer copy. Forwarded output, synchronization, command -environments, cancellation, and failure causes retain their existing behavior. - -Complete diagnostic capture remains in memory for this implementation. Its -remaining cost is measured with large output. Disk-backed capture is a possible -follow-up when its benefit justifies additional filesystem and lifecycle work. - -### Runtime Version Lookup - -The runtime implementation resolves its immutable build-metadata fallback once -and shares that result safely across calls. Linker injection retains precedence -and its inexpensive path. Main-module version selection and the development -fallback retain the [runtime facade's semantics](../consumers/runtime-version.md). -The runtime dependency graph remains small and independent of orchestration. - -## Priorities And Evaluation - -| Priority | Improvement | Expected benefit | -| --- | --- | --- | -| High | Batch selected Git blob reads | Amortize subprocess startup and object access across many files. | -| High | Clarify file-content ownership and selective reuse | Reduce copies and memory retained across validation checks. | -| Medium | Consolidate artifact hashing | Reduce repeated full-file reads and SHA-256 calculations. | -| Lower | Avoid unnecessary Go-output copies | Reduce allocation and peak memory for commands with large diagnostics. | -| Low | Cache runtime version fallback | Reduce repeated metadata parsing when consumers call the accessor frequently. | - -Evaluate component and complete-workflow gains with offline local fixtures, -including unique and repeated object IDs, packed objects, large files, fresh -delivery, and retries. Record workload descriptions, allocations, elapsed time, -and retained-memory observations. Timings inform implementation and remain -measurements specific to the workload and machine. - -Preserve the efficient foundations already present: batched tree metadata, -parsed Markdown reuse, streamed artifact I/O, one selected Go test suite, native -artifact reuse for version smoke checks, and source-only delivery without -compilation. Publication guards and exact content comparisons retain their -existing responsibilities throughout this work. diff --git a/docs/roadmap/promptkit-bug-report.md b/docs/roadmap/promptkit-bug-report.md deleted file mode 100644 index c7cf816..0000000 --- a/docs/roadmap/promptkit-bug-report.md +++ /dev/null @@ -1,150 +0,0 @@ -# PromptKit Bug Report: v0.2.0 Module Checksum Mismatch - -**Status:** Stale local module metadata confirmed; test isolation fixed. -Shared-cache recovery and PromptKit's Stage 1 adoption retry succeeded. - -## Summary - -PromptKit's adoption attempt encountered stale `go.mod` metadata in the local -Go module cache. Its checksum differs from the checksum database's recorded -value, so Go stops with a security error. Fresh downloads through both the -public proxy and the Git origin verify successfully. The published v0.2.0 -metadata agrees with the checksum database. - -## PromptKit Context - -PromptKit is adopting ReleaseKit v0.2.0 as a Go `tool` dependency, following the -release's documented module-tooling workflow. The version tag currently points -to commit -[`e030794969fb7787e647a90bbc4eb81bd0610f07`](https://gitea.maximumdirect.net/eric/ReleaseKit/commit/e030794969fb7787e647a90bbc4eb81bd0610f07). -The attempt used Go 1.26.8 with `GOTOOLCHAIN=local`. - -## Original Consumer Reproduction - -From a Go module using the affected cache, run: - -```sh -GOTOOLCHAIN=local go mod download -json gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0 -``` - -The affected cache produces: - -```text -verifying gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0/go.mod: checksum mismatch - downloaded: h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs= - go.sum: h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE= - -SECURITY ERROR -This download does NOT match an earlier download recorded in go.sum. -``` - -The recorded checksum is visible in the -[Go checksum database lookup](https://sum.golang.org/lookup/gitea.maximumdirect.net/eric/!release!kit@v0.2.0). -It records `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` for the module's -`go.mod`. The consumer's diagnostic fetch with `GOSUMDB=off` reported the same -v0.2.0 tag commit above, archive checksum -`h1:EACELngRel4RiulR+Ors56nMZ5fA9orJDyhPxn6UVe4=`, and `go.mod` checksum -`h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs=`. Disabling the checksum -database was used only to inspect the fetched source; it is not a proposed -consumer workaround. - -## Expected Behavior - -The published v0.2.0 module metadata and the checksum database record should -agree, so Go can verify and download the release normally. - -## Consumer Impact - -PromptKit's dependency preparation stops before it can reliably select -ReleaseKit v0.2.0 as a tool. Go's warning prevents proceeding with the normal -integrity check. This does not affect PromptKit's runtime API or behavior; the -tool dependency is for maintainer workflows. - -## Investigation Findings - -Investigation on 2026-10-08 used Go 1.26.8 and isolated temporary module caches. -The local and authoritative remote tags both identify the commit above. - -| Source | `go.mod` checksum | -| --- | --- | -| Tagged source, public proxy `.mod`, and module archive's `go.mod` | `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` | -| Checksum database | `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=` | -| Affected local cached `.mod` | `h1:KmH607Vc/RK/zKrCB96cWh0QZYvkbxBFvtqV+BkKaxs=` | - -The affected entry is -`$GOMODCACHE/cache/download/gitea.maximumdirect.net/eric/!release!kit/@v/v0.2.0.mod`. -It contains the original dependency-free module declaration. Its modification -time is 2026-10-07 at 23:35 UTC, preceding the published candidate. The cached -archive and extracted module contain the correct, dependency-bearing `go.mod`. -The exact command that created the stale metadata remains unknown. - -### Download And Recovery Evidence - -- Fresh downloads using `GOPROXY=https://proxy.golang.org` and `GOPROXY=direct` - both succeeded with `GOSUMDB=sum.golang.org`, checksum bypass settings cleared, - and `GOTOOLCHAIN=local`. Both returned the expected commit and checksums. -- Copying the affected cached version into an isolated module cache reproduced - the exact consumer checksum error. -- Quarantining only the stale `.mod` in that copied cache allowed the same - download to succeed with checksum verification enabled. - -These results support continued use of the published v0.2.0 version. The -investigation left the affected shared cache and repository contents unchanged. -Recovery was initially verified in a copied cache. The subsequent shared-cache -recovery and consumer retry are recorded below. - -### Related Test-Isolation Defect - -The workflow tests' [environment helper](../../cmd/releasekit/woodpecker_test.go) -disabled checksum verification for synthetic fixtures and inherited the caller's -module cache. The version-selection test serves synthetic ReleaseKit versions -from a local module proxy. - -Running `TestWoodpeckerTagDeliveryUsesTaggedModuleVersion` with a disposable -inherited `GOMODCACHE` passed and left synthetic `v0.2.197` and `v0.2.198` -metadata in that cache. This confirmed that the tests could contaminate a -developer's module cache. It established a related defect; attribution of the -stale `v0.2.0` entry to a particular test or command remains unconfirmed. - -### Test-Isolation Resolution - -Both synthetic-proxy test families now use a shared environment helper that -assigns a fresh test-owned `GOMODCACHE` and scopes proxy/checksum settings to -their Go subprocesses. Source-bootstrap preparation and execution retain -separate caches. Tests that build real ReleaseKit source continue to use their -prepared dependencies. - -The existing tagged-version test now supplies a disposable caller cache and -asserts that it remains empty after fixture execution. This regression failed -before the fix and passed afterward. All Woodpecker workflow tests passed. - -## Recommended Follow-Up - -1. Quarantine the affected local `v0.2.0.mod` entry and retry PromptKit's adoption - with normal checksum verification. Retain the quarantined bytes as evidence. -2. Record the actual cache recovery and consumer retry outcomes here. Preserve - the published v0.2.0 tag and checksums; this incident requires no replacement - release. - -## Consumer Recovery Outcome - -On 2026-10-08, PromptKit quarantined only the stale shared-cache -`v0.2.0.mod`. The original 58-byte file is retained at -`/home/eric/releasekit-cache-quarantine-WtdracPk/v0.2.0.mod`; no archive, -extracted module, or unrelated cache entry was removed. - -The retry used Go 1.26.8 with `GOTOOLCHAIN=local`, `GOWORK=off`, -`GOSUMDB=sum.golang.org`, and cleared `GONOSUMDB`/`GOPRIVATE` bypass settings. -`go mod download -json gitea.maximumdirect.net/eric/ReleaseKit@v0.2.0` -succeeded and returned the published commit and expected checksums: - -- Commit: `e030794969fb7787e647a90bbc4eb81bd0610f07`. -- Archive: `h1:EACELngRel4RiulR+Ors56nMZ5fA9orJDyhPxn6UVe4=`. -- `go.mod`: `h1:xN8w1OWjhCwtDxyHVFbKxFWowuEPbMTCZrf0JIIm9pE=`. - -PromptKit then successfully added the v0.2.0 Go tool dependency and prepared -its module dependencies with checksum verification enabled. Its readonly, -offline `go test ./...` run passed under the selected local Go installation; -the root package's runtime dependency list contains no ReleaseKit import. -This completes the previously blocked Stage 1. The remaining adoption stages -are still to be implemented. The published v0.2.0 tag remains unchanged. diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md deleted file mode 100644 index cf618f5..0000000 --- a/docs/roadmap/remediation.md +++ /dev/null @@ -1,179 +0,0 @@ -# Performance Implementation Remediation - -**Status:** Complete; R1–R4 addressed. - -**Review baseline:** df064bd, reviewed on 2026-10-09 against the -[performance target](performance.md) and -[implementation plan](implementation.md). - -## Scope And Architecture - -The five intended improvements are present behind the existing public facades: -an internal piped-process boundary and lazy Git batch reader, operation-local -validation result caches, consolidated local asset preflight, completed Go-output -buffer ownership, and a cached runtime metadata fallback. Public API and -dependency checks pass. This review found no added public input limits, release -gates, or product restrictions in the implementation commits. - -The findings below cover correctness and documentation gaps in that work. -Implement fixes within the existing architecture and preserve the established -public contracts. The [agent](../policy/agents.md), -[testing](../policy/testing.md), and -[documentation](../policy/documentation.md) policies govern the follow-up. - -## R1: Preserve Operation Cancellation Through Reader Shutdown - -**Priority:** High. - -**Owners:** [candidate tree](../../internal/validation/tree.go), -[validation executor](../../internal/validation/validation.go), and -[Git batch reader](../../internal/git/blob_reader.go). - -CandidateTree.readBlob creates its reader with context.Background(). -BlobReader.Read registers cancellation for an individual exchange and removes -that registration when the exchange finishes. The executor's deferred cleanup -then calls tree.Close(), whose stdout drain and process wait have no connection -to the validation operation's context. - -Consequently, cancellation while the child is shutting down cannot interrupt -that cleanup. Validation can remain blocked after its caller cancels. If the -child is subsequently terminated independently, the resulting error reports -the process exit and loses the caller's cancellation cause. - -**Evidence:** A temporary offline fixture served valid committed Go content, -waited until the executor closed batch stdin, signalled readiness, and stalled -before exiting. Cancelling Run at that point left it blocked. After the probe -terminated the fixture child, Run returned a Go-format cleanup failure for -"signal: killed"; errors.Is(err, context.Canceled) was false. The existing -TestRunPreservesCancellationDuringReaderCleanup cancels during an unfinished -blob read, so it exercises the per-read callback rather than this shutdown gap. - -**Required outcome:** Associate shutdown with the validation operation context, -either through the reader's lifetime context or context-aware cleanup. Preserve -individual-read cancellation and the primary failure when a check has already -failed. Reap the child on all paths. Use the supplied context for cancellation; -this fix does not require a new runtime timeout or input restriction. - -**Validation:** Add a regression synchronized to batch-input EOF, cancel while -shutdown is still blocked, and verify completion, child cleanup, and preserved -cancellation identity. Retain coverage of read cancellation, successful closure, -and preservation of an earlier check failure. - -**Completion:** Complete. The tree retains its operation context for the reader -lifetime. An EOF-synchronized regression now verifies shutdown cancellation, -cancellation identity, and child reaping; it failed before the fix and passes -afterward. Race tests passed for validation, Git, and process packages, including -existing read cancellation, successful closure, and primary-failure coverage. - -## R2: Attribute Shutdown Failures To A Check That Read Content - -**Priority:** Medium. - -**Owner:** [validation executor](../../internal/validation/validation.go). - -The lastContentCheck variable is assigned when a content-oriented check is -selected, even when that check has no applicable files and makes no content -request. Deferred cleanup therefore reports the last selected check rather than -the last check that used the reader, diverging from the implementation plan's -failure-attribution rule and the completion record's description. - -**Evidence:** A temporary fixture with only go.mod and main.go selected Go -formatting and Markdown checking. Only formatting read a blob. The child then -exited with code 23 during shutdown, and Run reported "markdown-links". - -**Required outcome:** Track actual reader use by each check and preserve the -last check that used content access when a later selected check performs no -reads. Keep check ordering and earlier failure attribution intact. Reader-use -tracking remains internal implementation bookkeeping. - -**Validation:** Extend the existing unexpected-close-failure fixture with a -later selected check that has no applicable files. Verify the earlier reader -user receives the cleanup cause and the child exit error remains discoverable. - -**Completion:** Complete. Internal blob-read bookkeeping now identifies the last -check that actually used the reader. The extended close-failure fixture preserves -Go-format attribution when Markdown has no files, including the child exit code -and stderr. The regression failed before the fix and passes afterward; validation -race tests and compiled public API and dependency checks passed. - -## R3: Qualify Output Allocation Measurements By Their Workload - -**Priority:** Low. - -**Owners:** [Go execution documentation](../internal/gotool.md), -[output benchmark](../../internal/gotool/performance_test.go), and the -[implementation results](implementation.md). - -The internal Go-output document says an 8 MiB forwarded capture currently -allocates about 8.39 MB per operation. The cited benchmark makes one 8 MiB -Write call. Actual command output can arrive in smaller chunks, which incur -additional bytes.Buffer growth allocations. The statement needs its workload -qualification to describe the measured behavior accurately. - -**Evidence:** Temporary component probes using the current writer and io.Discard -measured approximately 8,388,691 B/op and two allocations for one 8 MiB write, -versus 16,744,535 B/op and ten allocations for the same total output in 32 KiB -writes. These measurements exclude subprocess overhead and failure diagnostic -strings. Both paths correctly avoid the former final capture copy. - -**Required outcome:** Identify the single-write workload wherever that figure -is quoted. Include a representative chunked capture measurement in the benchmark -set and results. Distinguish allocated bytes across an operation from retained -output bytes and peak memory. Complete in-memory capture remains the selected -design; these findings do not require disk-backed diagnostics or a memory cap. - -**Validation:** Compare equal output lengths across the two chunking patterns -and confirm captured bytes and forwarding remain correct. Report allocation -results as workload-specific measurements with no numerical runtime guarantee. - -**Completion:** Complete. The benchmark now includes equal 8 MiB output in one -write and 32 KiB chunks, with untimed byte-for-byte capture and forwarding checks. -Three repetitions passed and measured about 8.39 MB/2 allocations and -16.74 MB/10 allocations respectively. Go-output documentation and implementation -results qualify the single-write figures and distinguish allocation totals, -retained output, and unmeasured peak memory. - -## R4: Reconcile Feature And Implementation Status - -**Priority:** Low. - -**Owner:** [performance roadmap](performance.md). - -The performance roadmap still says "Ready for implementation; implementation -pending." The implementation document marks Stages 1–11 complete, and the code -contains the five target components. Stage 11 explicitly called for updating the -feature roadmap status, and its completion record claims the work is complete. - -**Required outcome:** Update the feature roadmap to distinguish the implemented -target components from the outstanding review findings recorded here. Keep -remediation status in this document and performance results in the implementation -record, consistent with their documentation ownership. - -**Validation:** Check status and navigation across the three roadmap documents -against the actual implementation and remediation outcomes. - -**Completion:** Complete. The feature roadmap identifies all five implemented -components and links to their implementation results and review remediation. -The implementation record links to completed remediation. Status, ownership, -and local navigation across all three documents were checked against the code -and completed R1–R3 work. - -## Remediation Validation - -- The full offline Go test suite passed after the production fixes. -- Compiled public API and dependency checks passed. -- Race tests passed for validation, Git, and process packages. -- The output benchmarks passed three repetitions, including exact capture and - forwarding checks for both chunking patterns. -- Changed documentation links, fragments, and whitespace checks passed. - -## Review Validation - -- The full offline Go test suite passed; some unchanged packages used cached - results. -- The compiled public API and dependency checks passed. -- Focused race tests passed for internal/git, internal/process, - internal/runtimeinfo, internal/gotool, and internal/validation. -- The targeted probes used temporary Go overlays and local child-process - fixtures. They reproduced R1 and R2 and measured the workload distinction in - R3. No production code or permanent tests were changed during this review.