11 KiB
Resilience Review: Required Remediation
Reviewed: 2026-09-25, source commit ec78b5a.
Scope: Review of the resilience implementation for unintentionally strengthened
invariants, contracts, guarantees, and test requirements, compared with
resilience.md, historical implementation plan, and the
agent policy.
Status: R11–R14 addressed. The findings below describe the reviewed source;
see the resolution record for the implemented changes and verification.
The earlier findings R1–R10 have been addressed. Their findings and resolution
records remain in Git history; R6–R10 were resolved in ec78b5a.
Numbering continues here to distinguish new work from those resolved findings.
Architecture Assessment
The intended resilience architecture remains substantially implemented:
- ConfigKit owns configuration loading and credentials; PromptKit owns transport, bounded retries/repairs, and backend admission.
- Audita normalizes and chunks once, preserves section identity and stage barriers, and validates corrections before applying them. Recoverable model failures stay local; fatal failures cancel peers and join workers.
- Processing outcomes feed separate report mapping and presentation. Ordinary YAML reports and Markdown matrices share counts with the CLI summary; debug persistence is independently selectable.
- The output session prepares destinations, initializes reports, and publishes required outputs. Optional debug failures remain nonfatal and visible.
The findings below concern unnecessary restrictions and incidental test contracts, not missing resilience machinery. Some restrictions were introduced by the implementation plan itself, rather than by a coding agent deviating from it. Preserve validation-before-application, fatal cancellation, required-report initialization and finalization checks, ordinary file permissions, configured- secret redaction, and independent debug selection while addressing these findings.
R11 — P2: Allow Intentional In-Place Transcript Output
Category: Overly broad filesystem collision restriction.
Evidence: PrepareOutput
compares every active destination against every protected input path and rejects
all overlaps, including aliases. The CLI includes the transcript input among
those protected paths. Collision tests in
output_session_test.go
enforce rejection for direct, symbolic-link, and hard-link aliases.
The required protection was to avoid destroying inputs during preflight or early report initialization. It did not require forbidding intentional replacement of the transcript after it has been read and successfully processed. The broad prohibition appeared in the implementation plan and should not become a lasting product restriction solely on that basis.
Required change: Distinguish transcript publication from early report writes. Allow intentional in-place transcript output after successful processing, while retaining checks for destructive report/input collisions and incompatible output destinations. Keep the change proportional; no filesystem transaction or concurrent-host-mutation protection framework is needed. Update affected contract documentation and tests when the behavior changes.
Acceptance: A representative CLI workflow can read and successfully replace its input transcript. Preparation does not truncate the source, and failed processing does not publish a replacement. Alias checks do not impose a blanket ban on otherwise valid in-place output. Required report destinations still cannot destroy inputs during initialization or conflict with transcript publication.
R12 — P2: Protect Only Profiles That Were Actually Loaded
Category: Speculative protected-path restriction.
Evidence: selectedProfilePaths constructs
both .yaml and .yml paths for each selected profile, regardless of which file
was loaded or whether the alternate exists. These paths enter the output-session
protected-input set alongside actual configuration sources.
As a result, an unused alternate profile filename is reserved as though it were an input. This exceeds the intended protection of known loaded sources and can reject an otherwise valid output selection.
Required change: Derive protected profile paths from actual loaded sources, using the existing profile-loading boundary rather than reserving hypothetical alternates. Retain protection appropriate to real inputs and the publication phase, consistent with R11. Do not introduce a second profile-discovery policy.
Acceptance: An unused alternate profile filename is not rejected solely because its stem matches a selected profile. The actual loaded profile remains protected from destructive early report writes. Configuration-source protection continues to use actual loaded sources.
R13 — P2: Do Not Limit Transcript Destinations to Regular Files
Category: New output-interface restriction introduced by replacement mechanics.
Evidence: preflightTranscript
rejects existing destinations that are not regular files. The previous ordinary
transcript writer did not impose that restriction, allowing destinations such as
/dev/null and named pipes. Per-file atomic replacement is useful for regular
files, but does not justify narrowing every transcript destination to that kind
of file.
Required change: Separate regular-file replacement from writing to supported stream destinations. Preserve normal write-error handling and directory rejection; do not attempt to rename over a device or pipe. Apply preparation checks relevant to the selected destination, without requiring regular-file replacement machinery for stream output. Keep ordinary report-file handling unchanged.
Acceptance: Transcript output to /dev/null succeeds without replacing the
device. A representative named-pipe workflow can deliver the transcript to its
reader without replacing the pipe. Preparation does not open a pipe merely to
probe it and thereby unnecessarily wait for a reader. Regular-file publication,
permission handling, directory rejection, and real write-failure reporting remain
covered by focused tests.
R14 — P2: Test Markdown Semantics Without Freezing Incidental Prose
Category: Over-specified presentation tests and redundant assertions.
Evidence: markdown_test.go
compares the complete rendered report byte-for-byte with a golden file, then
checks overlapping exact fragments. Additional assertions depend on exact prose.
This treats incidental wording and whitespace as durable contracts beyond the
selected requirements for complete fixed matrices, ordering, counts, states,
escaping, and readability.
Required change: Prefer semantic assertions for the intended report contract, or limit a retained layout golden to the table structure whose layout matters. Remove redundant assertions and exact-prose requirements that do not protect meaningful behavior. Do not weaken the selected fixed-layout reporting contract or introduce adaptive formatting as part of this cleanup.
Acceptance: Tests still detect missing or misordered stage/chunk rows and columns, incorrect error/recovery counts or states, and broken table escaping. Harmless prose or whitespace edits outside the required layout no longer require updating a whole-document fixture and overlapping assertions. Retain meaningful agreement checks between YAML, Markdown, and CLI totals.
Original Review Verification and Limits
- This was a source and contract review; no application changes or new runtime reproductions were made during this pass.
- The preceding remediation passed the full Go test suite, vet, focused race checks, and Linux amd64/arm64 builds. Those checks were not repeated for this documentation-only review and do not establish that R11–R14 are resolved.
- Several current tests deliberately enforce the restrictions identified above; passing them does not establish that those restrictions were intended.
- The prior race checks and target-platform builds were proportionate to the resilience refactor. They should not be promoted into mandatory gates for every subsequent edit solely because they were used there.
- Application fixes, upstream changes, deferred feature work, and release publication remain outside this document update.
Resolution Record
- R11: Output selection distinguishes the transcript input from other protected
sources. Final transcript publication may replace its input; early report writes
still cannot overwrite it, and other source/output collisions remain checked.
TestSubprocessInPlaceTranscriptproves corrected output replaces the source on success and an invalid source remains untouched on failure. Output-session tests cover direct and alias acceptance, nontruncating preparation, and rejection of report/input aliases and conflicting output destinations. - R12: The runtime records successful reads through PromptKit's existing profile
filesystem and supplies actual loaded paths to CLI. It no longer guesses two
filenames per profile ID.
TestSubprocessProfileSourcesproves an unused alternate filename is allowed and a loaded profile with a different filename remains protected from report initialization. Configuration sources continue to come from ConfigKit's source records. - R13: Regular transcript files retain per-file replacement. Existing device
and pipe destinations receive ordinary writes during finalization and are not
opened during preflight.
TestTranscriptStreamDestinationsverifies/dev/nulland named-pipe delivery without replacing either destination, including pipe preparation before opening its reader.TestTranscriptStreamWriteFailurechecks a real stream write failure using/dev/fullwhere available. Existing directory, parent, permission, cleanup, and late-report-failure tests remain. - R14: Removed the whole-document Markdown golden fixture and overlapping matrix assertions. Tests compare complete ordered table rows, summary quantities, reason counts, escaping/redaction, and YAML/CLI count agreement. Summary prose is not snapshotted, and empty-work handling no longer requires an exact sentence. No renderer or output schema changed.
Current operations and internal documentation describe the revised boundaries. The completed implementation plan explicitly identifies its superseded choices.
Validation: go test ./... and go vet ./... passed, as did focused race checks
for the LLM runtime, diagnostics/output, and CLI subprocess packages. The refined
Markdown assertions passed their package tests. Changed-document links and
git diff --check passed. No live provider, dependency change, tag, or publication
was involved.