diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md index 63147b6..505069b 100644 --- a/docs/roadmap/remediation.md +++ b/docs/roadmap/remediation.md @@ -365,3 +365,134 @@ 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 + +**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 + +**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 + +**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.