Complete the Stage 19 test hygiene audit

docs/roadmap/audit.md

@@ -1,6 +1,6 @@
 # Repository Audit Ledger
 
-Status: In progress; Stages 1-18 complete.
+Status: In progress; Stages 1-19 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
@@ -144,6 +144,16 @@ goroutines, deterministic ordering and debug identities, coherent partial
 publication, committed-path projection, never-notify policy, content-safe CLI
 summaries, and result-bearing failure exit behavior otherwise match their
 contracts.
+Stage 19 found two low-severity test-hygiene defects: one Promptkit credential
+test changes behavior when a test-named environment variable already exists,
+and symlink tests in three packages fail on hosts that cannot create symlinks
+instead of treating capability as a prerequisite. The default suite otherwise
+uses only local HTTP servers and in-memory external-boundary fakes, controls
+environment mutation and writable roots, keeps time and concurrency waits
+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.
 Subsystem conclusions and final disposition remain pending the later stages.
 
 ## Baseline Metadata
@@ -289,7 +299,7 @@ inventory commands, graph index refresh, and graph architecture inspection.
 | 16 | Audit batch orchestration and Distributor notification | Complete |
 | 17 | Audit comparison contracts and transactional publication | Complete |
 | 18 | Audit comparison execution and CLI integration | Complete |
-| 19 | Audit test hermeticity and execution hygiene | Pending |
+| 19 | Audit test hermeticity and execution hygiene | Complete |
 | 20 | Audit test risk coverage and ownership | Pending |
 | 21 | Audit test durability, duplication, and maintenance cost | Pending |
 | 22 | Audit cross-cutting efficiency and complexity | Pending |
@@ -323,7 +333,8 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 | Batch partial success and notification | `internal/app`, `internal/adapters/distributor` | 16 | Partly insufficient. Collect-once planning, complete output preflight, deterministic sequential execution, independent partial success, report-only counters, published-source selection, and one all-success batch notification are coherent. `AUD-048` through `AUD-052` record late Distributor endpoint rejection, unsafe and unbounded response diagnostics, cancellation flattened into report failure, and the missing production HTTP adapter test boundary. |
 | 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. |
-| Hermeticity, execution hygiene, portfolio coverage, and durability | Repository-wide suite | 19-21 | Pending |
+| 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 |
 | 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 |
@@ -2550,6 +2561,82 @@ with evidence about meaningful risks, test ownership, gaps, and duplication.
 - Related findings: `AUD-044`, `AUD-051`
 - Remediation reference: pending
 
+### AUD-059: Missing-credential test depends on the ambient environment
+
+- Stage: 19
+- Status: candidate
+- Severity: low
+- Confidence: high
+- Category: test reliability
+- Area: `internal/adapters/promptkit.TestLocalBackendAndMissingCredentialBehavior`
+- Evidence: The test creates a profile whose `api_key_env` is
+  `WEATHERREPORTER_TEST_MISSING_KEY` and expects inspection plus execution to
+  report a missing credential, but it never clears that process variable with
+  `t.Setenv`. A targeted run with the variable set to a synthetic value caused
+  the test to execute its fake provider successfully and fail its missing-key
+  assertion. The package's other credential-bearing execution test explicitly
+  sets `OPENROUTER_API_KEY`, and all Distributor/config environment tests use
+  `t.Setenv`, so this is the sole discovered uncontrolled credential read. No
+  live provider was contacted by the probe.
+- Contract at risk: Default tests must be independent of real credentials and
+  ambient process state; a missing-credential case must establish absence
+  rather than assume a test-specific name is unused on every host.
+- Impact: A developer, CI worker, or wrapper that defines this variable gets a
+  false suite failure and unexpectedly exercises the fake success path. The
+  unusual variable name limits frequency, and the injected client prevents a
+  live provider call.
+- Recommendation: Call `t.Setenv("WEATHERREPORTER_TEST_MISSING_KEY", "")`
+  before inspection, keeping the test serial as required by Go's environment
+  mutation rules.
+- Test implications: Retain the existing missing-credential assertions; verify
+  the focused test passes both when the parent process omits the variable and
+  when it supplies any value, because the test must own its effective state.
+- Validation: A contaminated parent environment cannot change profile
+  inspection, provider-call count, error category, or test outcome, and the
+  full scrubbed/default suites continue to pass.
+- Related findings: none
+- Remediation reference: pending
+
+### AUD-060: Symlink tests assume host creation capability
+
+- Stage: 19
+- Status: candidate
+- Severity: low
+- Confidence: high
+- Category: test portability
+- Area: symlink cases in `internal/app/output_test.go`,
+  `internal/comparison/publish_test.go`, and `internal/config/config_test.go`
+- Evidence: Nine test sites call `os.Symlink`. Prompt-debug cases and one
+  comparison replacement table explicitly avoid unsupported Windows
+  semantics, but destination, recognition, output, and secret-entry cases in
+  the three named packages call `t.Fatal` for every creation error. Symlink
+  creation can be denied by host policy, filesystem capability, or Windows
+  privilege/developer-mode state even when the code under test is otherwise
+  runnable. The suite therefore fails during fixture setup rather than
+  classifying the unavailable prerequisite. Current Linux execution supports
+  symlinks and all cases pass, so the behavioral assertions themselves are
+  sound.
+- Contract at risk: Machine-specific filesystem features used by tests must be
+  capability-controlled; the default suite should fail for product behavior,
+  not because an optional host operation is unavailable.
+- Impact: Developers or CI runners on a restricted filesystem or unprivileged
+  Windows configuration can receive several unrelated package failures before
+  the intended symlink safety contracts execute. This does not affect runtime
+  behavior and supported Unix hosts retain coverage.
+- Recommendation: Use a small package-local capability helper that attempts
+  symlink creation inside `t.TempDir`, skips only recognized unsupported or
+  permission-denied creation states, and treats all other setup errors as real
+  failures. Prefer capability detection to blanket operating-system skips so
+  enabled Windows hosts retain coverage.
+- Test implications: Apply the same prerequisite handling to every symlink
+  case while preserving all assertions when creation succeeds. Exercise the
+  suite on one symlink-capable and one denied-capability host or CI job.
+- Validation: Capable hosts still execute every symlink contract; incapable
+  hosts report narrowly explained skips rather than setup failures; other
+  filesystem and race tests remain unchanged.
+- Related findings: `AUD-035`
+- Remediation reference: pending
+
 ## Retained Decisions
 
 ### RET-001: Keep the application package as the explicit composition owner
@@ -3045,6 +3132,36 @@ profile, publication, cleanup, cancellation, or aggregate error yields failed
 status and a nonzero command result. Retain this projection rather than
 reconstructing comparison state or provider diagnostics in the CLI.
 
+### RET-042: Keep the default suite offline at external boundaries
+
+Weather API behavior is exercised through loopback `httptest` servers with
+synthetic responses, Promptkit uses injected fake provider clients, Distributor
+uses an injected upload factory/client, and application/CLI workflows use
+project-owned fakes. Literal external URLs identify data or configuration but
+are not contacted. Retain these realistic local boundaries: they cover request,
+response, cancellation, and orchestration behavior without live Weather API,
+provider, Distributor, credential, DNS, or mutable infrastructure dependence.
+
+### RET-043: Keep real filesystem tests rooted in test-owned directories
+
+Writable filesystem behavior uses `t.TempDir`; tracked `testdata` and maintained
+examples are read-only inputs. Tests exercise atomic replacement, recognition,
+permissions, and path handling through the real filesystem, with Unix mode
+assertions guarded where semantics differ and unreadable cases capability-
+checked. Retain this stronger boundary while making symlink capability handling
+consistent under `AUD-060`; fake path literals passed to upload doubles need no
+host file.
+
+### RET-044: Keep process-global and concurrent tests explicitly controlled
+
+Environment-mutating tests use `t.Setenv`, remain serial, and receive automatic
+restoration. The only `t.Parallel` calls cover pure comparison model helpers.
+Concurrent debug and comparison tests coordinate through channels and wait
+groups, use long timeouts only as failure diagnostics, and join all normal-path
+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`.
+
 ## Open Questions
 
 No Stage 1 open questions or unexplained baseline failures remain.
@@ -3362,6 +3479,25 @@ Stage 18 routed these investigation leads to their assigned later stages:
   paths and safe CLI projection without duplicating package-owned findings;
   Stages 25-26 should validate the repaired end-to-end states.
 
+Stage 19 routed these investigation leads to their assigned later stages:
+
+- The assembled collection success test pays the production one-second warmup
+  delay, and the Weather API timeout case uses one controlled 50-millisecond
+  handler sleep. Both are bounded and deterministic, so Stage 19 did not label
+  them hermeticity defects. Stages 21-22 should assess their marginal runtime
+  and whether a narrower delay seam would improve durability or feedback cost.
+- Comparison-start and concurrent-debug helpers use five-second wall-clock
+  deadlines only as deadlock diagnostics; normal paths coordinate and join by
+  channels/wait groups. A timeout caused by a product regression can still
+  leave already-broken workers alive until the test process exits. Stage 21
+  should assess failure-path cleanup when reviewing test diagnostic quality,
+  rather than weakening the liveness guards here.
+- The repository defines no explicit supported host/CI platform matrix. Stage
+  19 classified capability-sensitive tests against the policy's machine-state
+  rule, not an inferred product portability promise. Stage 24 should reconcile
+  supported-platform expectations before later validation treats platform-
+  specific skips as product coverage evidence.
+
 ## Stage Log
 
 ### Stage 1: Establish The Baseline And Audit Ledger
@@ -4347,3 +4483,65 @@ Stage 18 routed these investigation leads to their assigned later stages:
 - Retained decisions: `RET-039` through `RET-041`.
 - Open questions: the three leads recorded above are routed to their assigned
   later stages.
+
+### Stage 19: Audit Test Hermeticity And Execution Hygiene
+
+- Status: Complete.
+- Scope reviewed: all 50 Go test files and 418 top-level tests; all nine JSON
+  files under testdata; maintained examples and production embedded assets read
+  by tests; external-network, subprocess, environment, process-global, clock,
+  randomness, sleep/deadline, filesystem, permission, parallelism, goroutine,
+  fixture, and golden-update mechanisms; and the assigned execution, asset,
+  test-double, and Go-specific testing-policy sections.
+- Exclusions: Whether important risks have sufficient or correctly owned
+  coverage remains Stage 20; test value, duplication, brittleness, and
+  maintenance cost remain Stage 21; cross-cutting runtime efficiency remains
+  Stage 22; refactoring remains Stage 23; documentation-wide coherence remains
+  Stage 24; broader diagnostic/adversarial execution remains Stage 25; and
+  remediation remains Stages 26-27.
+
+#### Nondeterminism And Machine-State Accounting
+
+| Mechanism | Inventory and contextual evidence | Classification |
+| --- | --- | --- |
+| Live networking and external infrastructure | Four `httptest.NewServer` sites serve loopback Weather API fixtures, cancellation, and timeout behavior. Promptkit executions inject a mutex-protected fake LLM client; Distributor injects a fake factory/client; app and CLI use project-owned doubles. External-looking URLs are inert fixture/config values. A full uncached run passed with external HTTP/HTTPS/all proxies forced to a closed loopback port while loopback was exempted. | Controlled and offline; retained as `RET-042`. No live Weather API, Promptkit provider, Distributor, DNS, or mutable service dependency was found. |
+| Subprocesses and host tools | No test imports `os/exec`, calls `exec.Command`, invokes a shell/tool, or uses a helper script. The repository has no test script, Make/Task/just test wrapper, or test-only generated executable. | Absent. Toolchain commands used by the audit are validation, not test-suite behavior. |
+| Environment and credentials | Seventeen `t.Setenv` calls establish synthetic Distributor, Promptkit, and secrets-directory values and restore them automatically; none are parallel. A scrubbed-environment suite passed. One missing-credential test reads `WEATHERREPORTER_TEST_MISSING_KEY` without establishing absence and fails when it is set. | Controlled except `AUD-059`; real-credential independence is narrowly disproved for that test's outcome, though its fake prevents a live call. |
+| Other process-global mutation | No test changes working directory, `time.Local`, command-line globals, logging globals, random seed, scheduler settings, standard streams, or signal handlers. CLI streams are local buffers. Secrets tests are serial and test-owned environment changes restore through the testing package. | Controlled; retained as `RET-044`. |
+| Clocks and calendar state | Workflow/domain tests overwhelmingly use literal times, fixed zones/locations, or injected fixed clocks. One `time.Now` reaches only an early missing-hourly-data error whose result is date-independent. Expired/immediate contexts establish cancellation categories without calendar assumptions. | Controlled. Host tzdata supplies named zones but all asserted dates/zones are explicit and the scrubbed-environment run passed. |
+| Sleeps, deadlines, and elapsed time | One Weather API timeout handler sleeps 50 ms behind a nanosecond client timeout. Retry cancellation measures only that a canceled hour-long delay returns within a generous one-second ceiling. Two five-second timers guard channel/wait-group liveness; they do not order successful execution. The assembled collect test incurs one configured one-second warmup. | Bounded and justified for current behavior; not treated as flakiness. Runtime/failure-path cleanup leads are routed to Stages 21-22. |
+| Randomness and unordered execution | Tests import no random package, seed no generator, and expose no random update choice. Promptkit may assign opaque execution run IDs, but assertions do not depend on them. Map traversal is used for membership/setup rather than asserted incidental order. Ten shuffled repetitions of every package passed. | Controlled. No test-order or randomized-output dependency was reproduced. |
+| Writable filesystem roots and path assumptions | There are 111 `t.TempDir` calls; all consequential writes, replacements, permission changes, and debug artifacts stay beneath those roots. Testdata and examples are tracked read-only inputs. `/tmp/report.md` and `/tmp/debug` occurrences are values passed to doubles/request construction and are not accessed. Relative testdata/example paths follow Go's package working-directory contract. | Controlled and repeatable; retained as `RET-043`. |
+| Permissions, file types, and platforms | Exact sensitive modes are asserted only outside Windows; unreadable file/directory cases skip when effective privilege defeats mode denial. Symlink safety cases use real links, but six sites across app, comparison, and config fatal on any host creation failure instead of checking capability. | Partly machine-specific; `AUD-060`. Current Linux user is non-root and all cases execute successfully. |
+| Parallel tests and goroutine cleanup | Ten `t.Parallel` calls are confined to pure comparison model tests. Six explicit goroutine sites use buffered result channels, cancellation, and wait groups for comparison/debug concurrency. Normal success/failure paths release barriers and join; a repository-wide race run passed. | Controlled; retained as `RET-044`. Failure-only timeout cleanup is routed to Stage 21. |
+| Fixtures, embedded assets, and credentials | Nine tracked JSON fixtures total 6,594 bytes and contain fixed synthetic weather shapes/timestamps. Tests also read maintained secret-free examples and immutable embedded prompt/profile/schema/template assets. Credential search found only environment-variable mechanism names and synthetic values. | Minimal, versioned, local, and credential-free. Fixture realism/value remains a Stage 20-21 question, not a hermeticity defect. |
+| Goldens and update mechanisms | No `.golden`, snapshot, approval file, updater flag/environment variable, or golden-writing code exists. Stable schema/manifest/template fragments are asserted inline or semantically; ordinary validation cannot rewrite repository assets. | Absent; the explicit-update requirement is satisfied vacuously. |
+
+#### Commands And Evidence
+
+- Used graph-augmented code search and symbol/snippet inspection to enumerate
+  tests that touch HTTP servers, provider/upload doubles, environment,
+  deadlines, goroutines, parallel execution, permissions, and filesystem
+  helpers. Targeted text searches covered test-only imports/calls, literals,
+  non-code fixtures, embedded assets, scripts, and golden mechanisms that are
+  not fully represented in the graph.
+- Counted 50 `_test.go` files, 418 top-level tests, no fuzz tests or benchmarks,
+  111 temporary-directory calls, 17 environment mutations, four local HTTP
+  server constructions, ten parallel calls, six explicit goroutine sites, one
+  sleep, and two liveness timers. Inspected every candidate category in context
+  rather than treating the raw mechanism as a violation.
+- Ran the full suite with a clean synthetic home, temporary/cache roots, no
+  inherited environment, and the existing read-only module cache; it passed.
+  Ran another uncached full suite with all external proxies pointed to a closed
+  loopback port and only loopback exempted; it passed, confirming external
+  network independence at execution time.
+- Ran `go test -shuffle=on -count=10 ./...`; every ordering/repetition passed.
+  Ran `go test -race ./...`; all packages passed without a race report. A
+  targeted synthetic ambient credential probe reproduced `AUD-059`; it used an
+  injected fake and contacted no provider.
+- Ran `go test ./...`, `go vet ./...`,
+  `go run ./cmd/weatherreporter --help`, and `git diff --check`; all passed.
+- Findings: `AUD-059` and `AUD-060`.
+- Retained decisions: `RET-042` through `RET-044`.
+- Open questions: the three leads recorded above are routed to their assigned
+  later stages.