From ab571cd8ab725ace04e35c9b76d05c97232b6578 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Wed, 12 Aug 2026 17:18:36 +0000 Subject: [PATCH] Complete the Stage 20 test risk audit --- docs/roadmap/audit.md | 125 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 122 insertions(+), 3 deletions(-) diff --git a/docs/roadmap/audit.md b/docs/roadmap/audit.md index 920e2bd..ed15cbb 100644 --- a/docs/roadmap/audit.md +++ b/docs/roadmap/audit.md @@ -1,6 +1,6 @@ # Repository Audit Ledger -Status: In progress; Stages 1-19 complete. +Status: In progress; Stages 1-20 complete. This temporary roadmap document is the evidence ledger for the staged audit defined by [the audit plan](audit-plan.md). It records audit evidence and @@ -154,6 +154,17 @@ bounded, joins normal-path goroutines, contains synthetic credential-free fixtures, has no subprocess or golden-update mechanism, passes shuffled repetition and the race detector, and succeeds with a scrubbed environment and failing external proxies. +Stage 20 found that the important runtime and compatibility risks have credible +test owners at the narrowest stable package, adapter, application, filesystem, +or CLI boundary. It did not create a coverage-percentage target or duplicate +the missing regression cases already attached to `AUD-003`, `AUD-010`, +`AUD-033`, `AUD-045` through `AUD-047`, `AUD-052`, and `AUD-053` through +`AUD-060`. Coverage diagnostics reached 79.4% of statements and highlighted +thin wrappers, dormant compatibility helpers, and defensive error accessors, +but no additional consequential unowned failure mode. Representative app and +CLI tests appropriately prove cross-package ordering and result projection +without repeating every lower-level malformed-input case. Detailed test +durability, fixture cost, and consolidation remain Stage 21. Subsystem conclusions and final disposition remain pending the later stages. ## Baseline Metadata @@ -300,7 +311,7 @@ inventory commands, graph index refresh, and graph architecture inspection. | 17 | Audit comparison contracts and transactional publication | Complete | | 18 | Audit comparison execution and CLI integration | Complete | | 19 | Audit test hermeticity and execution hygiene | Complete | -| 20 | Audit test risk coverage and ownership | Pending | +| 20 | Audit test risk coverage and ownership | Complete | | 21 | Audit test durability, duplication, and maintenance cost | Pending | | 22 | Audit cross-cutting efficiency and complexity | Pending | | 23 | Audit cross-cutting refactoring and deduplication opportunities | Pending | @@ -334,7 +345,8 @@ with evidence about meaningful risks, test ownership, gaps, and duplication. | Comparison identity and transactional publication | `internal/comparison` | 17 | Partly insufficient. Versioned flat manifests, ordered identity helpers, canonical encoding, data-package hashing, exact file sets/types, read-only destination classification, commit-time reauthorization, sibling staging, replacement rollback, restrictive modes, and committed cleanup state have focused package owners. `AUD-053` through `AUD-057` record ambiguous manifest fields, unenforced report filenames, a replacement cancellation gap, partially deleted retained backups, and late long-destination staging failure. | | Comparison concurrency and CLI behavior | `internal/app`, `internal/cli` | 18 | Partly insufficient. Explicit profiles are preflighted sequentially, one immutable prepared package feeds concurrent profile-local executions, goroutines join, slice positions lock durable/CLI order, partial bundles publish coherently, and safe summaries preserve committed paths and fail the action on any error. `AUD-058` records a missing mixed failure/cancellation case and the resulting overwrite of an already completed profile failure. | | Test hermeticity and execution hygiene | All Go tests, fixtures, test assets, and helper mechanisms | 19 | Partly insufficient. The suite is offline, uses controlled clocks/roots/environment for almost every stateful case, has no subprocesses or automatic golden updates, passes race and shuffled-repeat checks, and succeeds with a scrubbed environment and broken external proxies. `AUD-059` records one ambient credential assumption; `AUD-060` records unguarded symlink-capability assumptions. | -| Test risk coverage, ownership, and durability | Repository-wide suite | 20-21 | Pending | +| Test risk coverage and ownership | Repository-wide suite | 20 | Partly insufficient. Consequential contracts have narrow owners and representative cross-boundary workflows; every identified missing regression is already attached to the production or test-gap finding for the defect it would catch. Coverage diagnostics found no additional unowned critical branch. Accepted omissions are thin entry-point/delegation wrappers, defensive accessors, and dormant helpers already routed under `AUD-001`; broad duplication and durability remain Stage 21. | +| Test durability, duplication, and maintenance cost | Repository-wide suite | 21 | Pending | | Cross-cutting efficiency and maintainability | Graph metrics plus focused tests | 22-23 | Pending | | Documentation and executable-contract coherence | Canonical documents, code, schemas, templates, examples | 24 | Pending | | Dynamic robustness and diagnostic checks | Repository-wide deterministic checks | 25 | Pending | @@ -3162,6 +3174,28 @@ workers; the repository-wide race suite passes. Retain this selective parallelism and event-driven coordination while correcting the one ambient environment omission in `AUD-059`. +### RET-045: Keep consequential risks at the narrowest stable test owner + +Pure report, time, forecast, fact, module, schema, and naming rules belong to +their domain packages; HTTP/provider/upload translation belongs to adapters; +atomic file mechanics belong to `fileutil` and comparison publication; +workflow ordering and partial outcomes belong to `app`; and flags, structured +summaries, streams, and action status belong to `cli`. This distribution makes +the defect caught by each test legible and keeps malformed-input matrices near +the code that defines them. Retain representative higher-level proofs only +where correctness emerges from collaboration, such as collect-once preparation, +publish-before-notify, comparison ordering, and result-to-exit projection. + +### RET-046: Keep coverage diagnostic rather than percentage-driven + +The Stage 20 coverage run reported 79.4% statement coverage, with intentional +zeros among the process entry point, delegation wrappers, error `Unwrap` +methods, optional constructors, and dormant compatibility/persistence helpers. +Those lines do not independently justify tests. Retain coverage as a way to +locate surprising weak spots, while requiring each added regression to name a +realistic consequential defect and its stable owner. The currently important +weak spots are already recorded by defect rather than by uncovered line. + ## Open Questions No Stage 1 open questions or unexplained baseline failures remain. @@ -3498,6 +3532,25 @@ Stage 19 routed these investigation leads to their assigned later stages: supported-platform expectations before later validation treats platform- specific skips as product coverage evidence. +Stage 20 routed these investigation leads to their assigned later stages: + +- Generated-text policy assertions already cross package boundaries under + `AUD-039`, and report/template/application fixtures repeat some report-family + shapes. Stage 21 should assess marginal protection, change amplification, and + consolidation without moving schema, rendering, or workflow ownership. +- Several application and CLI suites deliberately use complete prepared + reports to prove collaboration, while package suites own the exhaustive + negative cases. Stage 21 should verify that these representatives remain + behavior-focused and diagnose failures locally; Stage 20 found no reason to + expand them into duplicated lower-level matrices. +- Broad parser, normalization, and path input spaces have table-driven boundary + tests but no fuzz targets. No specific uncovered defect justified adding a + percentage-driven target in Stage 20; Stage 25 should use its adversarial + corpus to decide whether a durable fuzz regression is warranted. +- The one-second assembled collection warmup and failure-only goroutine cleanup + concerns remain execution/durability questions for Stages 21-22. They do not + leave a consequential production invariant without a test owner. + ## Stage Log ### Stage 1: Establish The Baseline And Audit Ledger @@ -4545,3 +4598,69 @@ Stage 19 routed these investigation leads to their assigned later stages: - Retained decisions: `RET-042` through `RET-044`. - Open questions: the three leads recorded above are routed to their assigned later stages. + +### Stage 20: Audit Test Risk Coverage And Ownership + +- Status: Complete. +- Scope reviewed: the complete 50-file, 418-test Go suite as a risk portfolio; + architecture invariants; CLI, configuration, domain, adapter, prompt, + generated-text, schema, template, publication, comparison, concurrency, + cancellation, partial-success, notification, compatibility, security, + idempotency, and recovery contracts; meaningful negative paths; test-layer + ownership; representative cross-boundary repetition; and statement coverage + only as a gap-finding diagnostic. +- Exclusions: test brittleness, fixture and helper cost, exact duplication, and + consolidation remain Stage 21; runtime and algorithmic cost remain Stage 22; + refactoring remains Stage 23; documentation coherence remains Stage 24; + broader adversarial execution remains Stage 25; and remediation remains + Stages 26-27. No production code or tests were changed. + +#### Consequential Risk-To-Test Matrix + +| Risk | Narrowest credible owner and meaningful negative paths | Assessment and higher-level repetition | +| --- | --- | --- | +| Architecture invariants | Package import/build checks and assembled `app`/`cli` workflows protect dependency direction, adapter containment, collect/prepare/execute/publish order, and stateless normal operation. | Sufficient for active paths. Higher layers repeat only collaboration; dormant persistence remains `AUD-001`. | +| CLI | `internal/cli` owns parsing, required and conflicting flags, working/output paths, quiet/JSON streams, safe summaries, one-executor wiring, and action status. | Partly insufficient under `AUD-008` through `AUD-010`; lower workflow failures need no exhaustive CLI repetition. | +| Configuration | `internal/config` owns defaults, strict YAML, precedence, active/inactive validation, secrets, durations, URLs, templates, module options, and maintained examples. | Partly insufficient under `AUD-004` through `AUD-007`; adapter/app tests check only consumed mappings and consequences. | +| Domain rules | `internal/report`, `timeutil`, `forecast`, `facts`, `module`, and `briefing` own identity, civil/elapsed periods, selection, derivation, availability, ordering, and meteorological thresholds with boundary and malformed cases. | Partly insufficient under `AUD-002`, `AUD-003`, and `AUD-014` through `AUD-028`; app/template cases remain representative projections. | +| Weather adapter | `internal/adapters/weatherapi` uses local HTTP servers for query shape, all source products, required/optional/checked-empty policy, malformed JSON, statuses, retries, timeouts, cancellation, and provenance. | Partly insufficient under `AUD-011` through `AUD-015`; collection tests need only normalized bundle/error behavior. | +| Promptkit adapter | `internal/adapters/promptkit` owns exact inspection/preparation/execution mapping, credential absence, callback order, cancellation, copies, provenance fields, and safe classified provider errors through an injected provider client. | Partly insufficient under `AUD-034` and `AUD-044`; app tests appropriately prove lifecycle use rather than provider variants. | +| Distributor adapter | `internal/adapters/distributor` owns request/path mapping, one upload, idempotency-key forwarding, accepted/conflict states, polling, timeout/cancellation, and token replacement through injected clients. | Partly insufficient under `AUD-048` through `AUD-052`, especially the absent production HTTP boundary; app tests own only notification policy. | +| Prompt contracts | `promptassets`, `promptinput`, and `promptexec` own embedded identities, category/order rules, validation, deterministic YAML, neutral lifecycle, error categories, bounds, and copy isolation. | Partly insufficient under `AUD-029` through `AUD-033`; complete app inputs are representative composition proofs, not duplicate serializers. | +| Schemas and generated text | `internal/generatedtext` owns catalog lookup, schema validation, typed decoding, normalization, required fields, and immutable asset copies. | Partly insufficient under `AUD-036` through `AUD-039`; application tests check selected schema/result consequences only. | +| Templates and Markdown | `internal/reporttemplate` and typed generated-text context tests own lookup failures, report-specific sections, missing values, semantic output, dynamic prose placement, and deterministic repeated renders. | Partly insufficient under `AUD-040` through `AUD-042`; app tests prove publication of one rendered result rather than restating template matrices. | +| Atomic single output | `internal/fileutil` owns same-directory temporary write/close/rename and cleanup; `internal/app` owns destination preflight, prior-output preservation, committed paths, and notification ordering. | Partly insufficient under `AUD-045` through `AUD-047`; the split correctly separates mechanism from workflow policy. | +| Comparison replacement | `internal/comparison` owns destination classification, exact recognition, sibling staging, reauthorization, install, rollback, backup cleanup, restrictive modes, and committed-state errors on a real filesystem. | Partly insufficient under `AUD-053` through `AUD-057`; app/CLI repeat only durable outcome projection. | +| Concurrency | `internal/app` owns one immutable preparation, concurrent profile-local work, backend capacity observation, joining, isolation, stable caller order, and distinct debug identities; the race suite supplies dynamic corroboration. | Partly insufficient under `AUD-058`; pure package tests need no concurrency repetition. | +| Cancellation | Adapter tests own propagation through requests/retries/providers/uploads; app tests own stop/start, preservation, joining, and no-publication outcomes; CLI should own signal conversion. | Partly insufficient under `AUD-008`, `AUD-047`, `AUD-051`, `AUD-055`, and `AUD-058`; each gap is attached to its observable boundary. | +| Partial success | App tests own independent batch report results and comparison profile outcomes, counters, partial artifacts, aggregate errors, and deterministic ordering; CLI owns structured partial summaries and failure status. | Sufficient for implemented semantics except the cancellation overlaps in `AUD-051` and `AUD-058`; package negative cases are not repeated. | +| Notification | App tests own publish-before-notify, all-success batch gating, one call, request identities/paths, notification failure, and never-notify comparisons; the Distributor adapter owns delivery mechanics. | Partly insufficient under `AUD-005` and `AUD-048` through `AUD-052`; ordinary comparison omission is an explicit contract. | +| Compatibility | Report/prompt/generated-text/comparison packages own stable IDs, versions, catalog pairs, strict decoding, round trips, canonical encodings, and unsupported-version behavior; asset tests exercise embedded current pairs. | Partly insufficient under `AUD-022`, `AUD-032`, `AUD-036`, `AUD-037`, `AUD-053`, and `AUD-054`; dormant JSON/persistence surfaces do not justify workflow tests. | +| Security | Config, Promptkit, prompt-debug, Weather API, Distributor, file publication, comparison, and CLI tests own secret handling, bounded safe errors, explicit debug, redaction, symlink/type checks, restrictive modes, and content-safe summaries. | Partly insufficient under `AUD-007`, `AUD-012`, `AUD-034`, `AUD-035`, `AUD-038`, `AUD-045`, `AUD-049`, `AUD-050`, and `AUD-053`; higher layers verify safe projection only. | +| Idempotency | App tests own one batch notification and stable run-derived key construction; Distributor tests own exact key forwarding and already-accepted conflict mapping without replaying app workflow. | Credibly owned at two distinct contracts. Production HTTP traversal remains the broader adapter gap `AUD-052`, not a separate idempotency finding. | +| Recovery | File and comparison tests own pre-commit cleanup, preservation, replacement rollback, concurrent destination changes, restoration failure, post-commit cleanup truth, and retained recovery paths. | Partly insufficient under `AUD-046`, `AUD-056`, and `AUD-057`; app/CLI tests appropriately assert only user-visible committed and recovery state. | + +#### Commands And Evidence + +- Used graph search, inbound/outbound relationship traces, and symbol snippets to + map active production paths to their package and higher-level tests. Used + targeted text search for non-code assets, test names, configuration literals, + and finding references that are not completely represented in the graph. +- Reviewed every prior stage's test-ownership accounting and reconciled all + missing consequential cases with the existing findings. No uncovered risk + required a new finding; the accepted omissions are thin entry/delegation + wrappers, defensive error accessors, impossible-under-current-type branches, + and dormant helpers already routed for removal or justification. +- Ran `go test -coverprofile=/tmp/weatherreporter-stage20-cover.out ./...`; all + packages passed and total statement coverage was 79.4%. Inspected the + function report for unexpectedly weak CLI, app, adapter, domain, prompt, + filesystem, and comparison areas rather than treating uncovered lines or + package percentages as targets. +- Ran `go test ./...`, `go vet ./...`, + `go run ./cmd/weatherreporter --help`, and `git diff --check`; all passed. +- Findings: none new. Existing missing-test or defect-specific regression gaps + remain `AUD-003`, `AUD-010`, `AUD-033`, `AUD-045` through `AUD-047`, + `AUD-052`, and `AUD-053` through `AUD-060`. +- Retained decisions: `RET-045` and `RET-046`. +- Open questions: the four leads recorded above are routed to their assigned + later stages.