Fix the five resilience review findings

This commit is contained in:
2026-09-24 21:48:18 +00:00
parent 9fc4de22b7
commit ec78b5a5db
19 changed files with 550 additions and 132 deletions

View File

@@ -4,7 +4,8 @@
**Scope:** The fourteen implementation commits after `8ffbf41`, compared with
[implementation.md](implementation.md), [resilience.md](resilience.md), and the
current architecture and output contracts.
**Status:** Five open findings, R6–R10. This review implements no fixes.
**Status:** R6–R10 addressed and verified. The findings below describe the reviewed
source; their line references are historical. See the resolution record below.
The earlier library-adoption findings R1–R5 were resolved before this work.
Their findings and resolution evidence remain in the
@@ -192,8 +193,49 @@ one assembled CLI case is sufficient. Debug-off must still create no artifacts.
- A temporary runner probe reproduced R9; its expected evidence-preservation
assertion failed. The probe and all temporary artifacts were removed.
Passing retained tests do not cover the five failures above. Add proportionate
regression coverage with their fixes. This review changes only this document;
At review time, the retained tests did not cover the five failures above.
The initial review changed only this document;
application fixes, upstream changes, deferred feature work, and release publication
remain outside scope. Target-platform release builds were not repeated for this
review; the native temporary CLI build succeeded.
## Resolution Record
All five findings are addressed without changing retry budgets, library ownership,
correction acceptance, or the independent reporting/debug selections.
- **R6:** Per-file replacement uses ordinary file creation inside a private
temporary directory in the destination parent. Creation applies the umask;
existing permission bits are restored only for existing destinations. Temporary
files/directories are removed on success and failure. The subprocess permission
test covers both restrictive and ordinary masks and subsequent replacement of
restrictive files; existing output-session failure/cleanup tests still pass.
- **R7:** File selection uses the literal nonempty output path throughout report
placement and output preparation. Final CLI stream selection uses the prepared
output session. `TestSubprocessWhitespaceOutputIsALiteralFile` proves the output
and reports are produced; the existing stdout integration test remains passing.
- **R8:** Every CLI report/debug location announcement uses the shared configured-
secret string redactor. `TestSubprocessPathAnnouncementsAreRedacted` covers
success and failure with selected and unselected credentials in real paths,
while confirming that the real destinations are used.
- **R9:** Fatal validator returns use partial identity/cardinality validation,
retain completed decisions and rejection details, and never return application
approvals. `TestFatalBatchRetainsEarlierDecisionsInLedger` exercises the real
runner and batching validator with two completed batches and a third fatal
failure, retaining their decisions and all operation facts in the debug ledger.
A separate runner regression rejects unknown/duplicate partial identities while
retaining the originating failure. Existing cancellation/joining tests pass.
- **R10:** An explicit diagnostic-error callback connects proposal and validator
writers to the CLI's synchronized, redacted nonfatal diagnostics. Origin/profile
serialization failures use the same path. Narrow proposal/validator tests and
`TestSubprocessInteractionWriteFailureIsVisibleAndNonfatal` verify visible
failures, unchanged transcript/counts/exit status, and no fabricated artifact
paths. Existing reporting/debug-selection tests verify debug-off persistence
remains disabled.
Validation completed: full `go test ./...`, `go vet ./...`, focused race tests for
runner, validators, proposal generation, report assembly, diagnostics/output, CLI,
and subprocess integration; required Linux amd64/arm64 builds in a temporary
output directory; and documentation link/whitespace checks. No live provider,
upstream change, tag, or publication was involved.