Post-2.0 hardening & enhancement plan#5
Merged
Merged
Conversation
Detailed, code-grounded implementation plan for the five backlog items — async-I/O hardening (overlapped Windows reads, POSIX pidfd/reaper exit-wait), streaming backlog backpressure, ILogger observability enhancements (EventIds, LoggerMessage.Define, correlation, Activity/Meter), DI Extensions enhancements (options/keyed clients/group registration), and record/replay enhancements (bytes + streaming replay, v2 schema). Each item states what/why/acceptance criteria against the current implementation seams. Internal planning doc, kept out of the consumer-facing docs index; no library change, so no CHANGELOG entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…s, auto) RecordReplayRunner gains: - Bytes capture record+replay via a v2 cassette schema (`StdoutBase64`, exact non-UTF-8 stdout bytes). A text/pre-v2 entry replayed through the bytes verb stays honestly `ProcessError.Unsupported` (no lossy re-encode). - Streaming replay: `SpawnAsync` reconstructs a live `FakeProcess` handle from the recording; record-mode `SpawnAsync` stays `Unsupported`. - `RecordReplayOptions`: opt-in file-stdin content hashing, an argument normalizer (tolerate volatile args), and a record-time redaction hook. - `Auto(path, inner)`: replay-with-record-on-miss (VCR "new episodes"). - Version back-compat policy: accept 1..current, reject newer; v1 loads. Public API snapshot, docs/testing.md, and CHANGELOG updated. +13 cassette tests. Green on net10.0 (net8.0 via CI); Fantomas clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- testing.md: drop the now-stale "exactly what RecordReplayRunner does" attribution — RecordReplayRunner now serves SpawnAsync in replay/Auto mode (only record-mode SpawnAsync is Unsupported), which the rest of the doc already states. - cookbook.md: point the record/replay recipe at the new bytes / streaming / Auto / RecordReplayOptions capabilities in testing.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…, metrics - Log.fs: stable EventIds (name + number) per lifecycle event; cached LoggerMessage.Define delegates (no format/boxing when the level is off); a per-run RunId on every run-scoped event (spawn/exit/timeout/retry). - RunId correlation: internal CommandConfig.RunId, stamped once at the verb layer (Runner.withRetry), so a run and all its retries share one id; a direct spawn gets a per-incarnation fallback. - Diag.fs: an ActivitySource "ProcessKit" emitting a backdated span per completed run, and a Meter "ProcessKit" with runs started/completed/active, a duration histogram, retries, and supervisor restart/storm-pause counters — tagged only by program + outcome, never argv/env. Public ProcessKitDiagnostics exposes the source/meter names. - RunningProcess: a single once-guarded conclude() (log + metrics + span) at every terminal verb (Output*/Wait/Finish/Profile/OutputEvents/ExitTask). New docs/observability.md; API snapshot, cookbook, README, CHANGELOG updated. +5 logging tests (EventIds, RunId correlation, level-gate, trace + metric listeners with no-argv assertions). Green on net10.0; Fantomas clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…docs)
From the solution-level review:
- Expose the lifecycle EventIds publicly as ProcessKitDiagnostics.Events
(new Diagnostics.fs, compiled before Log.fs), restoring symmetry with the
public ActivitySource/Meter name constants — consumers filter/route by id
without a magic number.
- Align metric units with OpenTelemetry/UCUM: dimensionless counts carry a
{…} annotation, and the run-duration histogram is now in seconds (was ms),
so OTel-calibrated dashboards aren't off by 1000×. The log line stays in ms.
- Docs: README Observability section + capability table now cover tracing/
metrics and link the new guide; ROADMAP "What the library covers" lists them;
observability.md gains the Events column and the corrected unit table.
API snapshot updated (additive: ProcessKitDiagnostics.Events). Green on
net10.0; Fantomas clean.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…shared group) - ProcessKitOptions + AddProcessKit(Action<ProcessKitOptions>) / AddProcessKit(IConfiguration): default timeout / working directory applied to every DI-resolved run, only where the command sets none (per-command wins). Retry is intentionally NOT an option — the retry loop is a verb-layer policy that runs before a primitive IProcessRunner decorator can inject it, so it would silently no-op; set retry (and encoding/env) on a keyed CliClient instead, whose template precedes the verb. - AddProcessKitClient(name, program, configure): a keyed per-tool CliClient, injected by role, running through the container's registered IProcessRunner. - AddProcessKitGroup(): backs IProcessRunner with a shared, container-managed ProcessGroup (also registered directly); disposing the provider reaps the whole tree. - Deferred: a separate ProcessKit.Extensions.Hosting package for a supervised IHostedService (documented) — kept out to keep the DI package hosting-free. New docs/dependency-injection.md; DI API snapshot, cookbook, README index, CHANGELOG updated. +4 DI tests. Green on net10.0; Fantomas clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
From the solution-level review: - Add symmetric AddProcessKitGroup(Action<ProcessKitOptions>) / AddProcessKitGroup(IConfiguration) overloads, so a shared group can be configured with the same defaults as AddProcessKit in one call. - dependency-injection.md: correct "Abstractions-light" -> "dependency-light" (Options/Options.ConfigurationExtensions are concrete packages), and warn about the AddProcessKit-vs-AddProcessKitGroup "first TryAdd wins" mis-wire. - Cross-reference the DI guide from testing.md; list the new DI ergonomics in ROADMAP "What the library covers". DI API snapshot + CHANGELOG updated. Green on net10.0; Fantomas clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Owner
Author
|
Merged into |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements
docs/planning/post-2.0-plan.md— the post-2.0 hardening & enhancement plan — one item per stage. This PR accumulates the stages; each is independently green (build + tests + Fantomas), with the public-API snapshot, docs, andCHANGELOG.mdupdated in the same change set.Progress
RecordReplayOptions(content-hash / normalizer / redaction),Autorecord-on-miss, back-compat version policy.LoggerMessage.Define, per-runRunIdcorrelation,ActivitySource "ProcessKit"spans +Meter "ProcessKit"metrics (ProcessKitDiagnostics), secret-safe.ProcessKitOptionsdefaults (timeout / working dir, viaAction/IConfiguration), keyed per-toolCliClients, container-managed sharedProcessGroup. (Separate Hosting package deferred, documented.)Order chosen to ship the additive, lower-risk items first and the highest-risk native item (async-I/O) last.
🤖 Generated with Claude Code