docs: diagnostics subsystem spec and architecture by norrietaylor · Pull Request #802 · gominimal/minimal

norrietaylor · 2026-07-17T05:08:06Z

Summary

Spec + architecture document for the diagnostics subsystem — the plan of record for re-landing PR #784 (min bug diagnostic bundle, +4107/−68, moved to draft) as eight demoable units, each an independently mergeable PR under 1000 lines.

Tracking issue: #801. Reference implementation: branch min-bug-diagnostics (all #784 review findings addressed); baselines in the spec cite it as ref: alongside the origin/main state each unit lands on.

What the spec decides

Crate boundary : all app-agnostic machinery — bundle writer, manifest, fail-closed redaction, listings, subprocess capture, and eventually the net/procs/power collector mechanics — lives in a new diagnostics crate; crates/minimal keeps only bug-command-specific code.
One manifest schema for both bundle layers: BundleWriter generalizes over AsyncWrite (file-backed for the CLI, rootless streaming for the daemon), retiring the guest bundle's separate errors.json before anything ships.
Logs exist before the incident: both daemons persist size-capped rotated file logs (logroller — tracing-appender rotates by time only, and a runaway error loop filling the data volume is precisely the failure class this subsystem diagnoses); the in-VM appender releases through ServerState before volume quiesce; the VMM console lands in boot.log by default.
OTEL-compatible, OTEL-free: span-carried correlation, JSON-lines file logs, W3C-shaped trace ids propagated CLI→daemon via SSH env request. The SDK itself is rejected for the bundle path (pre-1.0, push-based, no wedged-process story — every peer tool ships plain files).
One-shot blob, not streaming RPC: the collection path must work when the transport is the suspect (#788's failure domain was the transport); degraded mode harvests the guest's logs read-only from the ext4 image.

Units

diagnostics crate foundation
min bug host bundle (command + system/env/config/state/log collection)
Persistent, correlatable daemon logs
OTEL-compatible log format + trace propagation
Incident collectors, mechanics in-crate
Daemon bundle serving (DiagBundleTarZst)
Guest fetch + degraded-mode fallback
Bundle explorer dual-format

Test plan

Docs-only change; spec Verification table maps every requirement to a proof artifact carried by its unit PR
Frontmatter/section structure matches the house exemplar (docs/specs/08-spec-vm-ext4-volume/)

🤖 Generated with Claude Code

Summary by CodeRabbit

Documentation
- Added a planned diagnostics specification for a one-command workflow that generates redacted minimal diagnostics bundles with a shared manifest.json, failure-isolated collectors, and standardized met 8000 adata-first listing and error reporting.
- Published an end-to-end diagnostics architecture covering host/daemon/guest behavior, including streaming bundle delivery over SSH, JSON-lines logging, and CLI→daemon trace propagation via W3C TRACEPARENT.
- Included degraded-mode guest log harvesting from a volume image, plus security/design considerations, verification matrix, and assumptions/non-goals.

Decomposes the min bug diagnostic bundle work (PR #784, moved to draft at +4107 lines) into eight demoable units, each an independently mergeable PR under 1000 lines. Records the design decisions from the review and research pass: app-agnostic machinery in a dedicated diagnostics crate, one manifest schema for both bundle layers, size-capped rotation via logroller, OTEL-compatible log conventions with zero OTEL dependencies, and a one-shot blob transport chosen for the wedged-process case. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

coderabbitai · 2026-07-17T05:08:21Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds two architecture specifications for an end-to-end min bug diagnostics subsystem, covering bundle creation, redaction, host and guest collection, daemon RPC streaming, correlated logs, degraded fallback, explorer compatibility, security constraints, and verification.

Changes

Diagnostics subsystem

Layer / File(s)	Summary
Bundle foundation and host collection `docs/specs/10-spec-diagnostics/...`	Defines `BundleWriter`, manifest finalization, redaction, capture/listing helpers, host-side collection, and CLI orchestration.
Persistent logging and trace correlation `docs/specs/10-spec-diagnostics/...`	Specifies persistent daemon and VMM logs, volume-log release behavior, JSON-lines formatting, and SSH `TRACEPARENT` propagation.
Incident collectors and guest bundle flow `docs/specs/10-spec-diagnostics/...`	Defines incident collectors, diagnostics RPC contracts, streaming daemon bundles, guest probing, verification, and volume-based fallback collection.
Compatibility, constraints, and verification `docs/specs/10-spec-diagnostics/...`	Documents explorer compatibility, security requirements, design constraints, assumptions, open questions, non-goals, and verification gates.

Estimated code review effort: 2 (Simple) | ~15 minutes

Possibly related issues

Issue 801 — Covers the diagnostics subsystem specified by this change.

Poem

I’m a rabbit with bundles to pack,
Redacted logs neatly stacked.
Through daemon streams they hop and flow,
With traces that help investigators know.
If guests can’t answer, volumes speak—
A tidy diagnostic trail to seek!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the documentation-only diagnostics subsystem spec and architecture changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 14

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/specs/10-spec-diagnostics/10-spec-diagnostics.md`:
- Around line 406-407: Standardize the SSH environment variable name from
uppercase TRACEPARENT to lowercase traceparent across the diagnostics
specification, including the baseline and R4.4/architecture references. Update
the associated client and daemon propagation tests to assert the exact lowercase
name on both ends, preserving the existing trace propagation behavior.
- Around line 370-373: Update the Unit 3 CLI gate description to state that
boot.log is located under the provider directory, while rotated minimald.log*
files appear on the data volume. Preserve the existing clean unmount,
no-journal-replay, and next-boot validation criteria.
- Around line 163-180: Clarify the file-backed archive contract in R1.2/R1.3 by
explicitly specifying whether finish writes the manifest at {root}/manifest.json
or at the archive top level; keep stream mode rootless as stated. Update the
verification requirements and tests to assert the selected exact manifest path.
- Around line 181-183: Update the R1.4 requirement and the corresponding CLI and
daemon collector implementations to use a no-follow, descriptor-based open and
read, eliminating the race between symlink validation and file access. Ensure
both collectors reject symlinks at open time and retain the recorded-cap read
bound.
- Around line 599-606: Update the R7.3 nested daemon-bundle verification
requirements to enforce bounded decompression, including explicit limits for
decompressed output and archive entry count, while avoiding unbounded buffering
such as an architecture API Vec<u8>. Require bounded streaming verification or a
temporary-file approach with enforced decompression limits, preserving the
existing behavior of recording the raw bytes and manifest error on verification
failure.
- Around line 829-837: Define and apply explicit redaction for collected logs
and
8000
 marker-matched full argv, covering tokens, URLs, and user data consistently
with the bundle’s public-sharing guarantee. Update the “Trust boundary: the
bundle leaves the machine” requirements to include these artifacts, or
explicitly narrow the guarantee and add a prominent warning if they remain raw.
- Around line 208-214: Add a subprocess timeout contract test to the R1.7 proof
table, covering kill_on_drop(true), termination of timed-out children, and
retention of captured stdout, stderr, and exit status. Update the corresponding
proof entry around the subprocess implementation and ensure the test
demonstrates all required timeout outcomes.
- Around line 263-266: Update the R2.4 environment-recording specification to
remove broad prefix-based allowlisting and retain only explicitly approved exact
keys. Ensure values for sensitive or non-approved keys are masked as
<redacted:len=N>, while preserving vars_os and the existing non-UTF-8 safety
requirement.
- Around line 263-266: Update requirement R2.4 to define how non-UTF-8 values
from std::env::vars_os are handled in JSON, choosing either an explicit byte
encoding or masking such allowlisted values while preserving the stated behavior
for UTF-8 values and disallowed variables. Add a regression test covering a
non-UTF-8 allowlisted environment value and its required serialized output.
- Around line 192-194: Define explicit failure and truncation semantics for
listing.rs and listing_text(...): use a structured result or an unambiguous
encoded representation that distinguishes a genuinely empty listing from
permission/read failures, records collector failures in the manifest, and
indicates when the entry cap truncates results.
- Around line 272-276: Update the R2.7 log collection requirement to define an
explicit bounded N, including whether the cap applies per prefix/name or
globally. Specify the expected rotation-order rule used to select the newest
files, while preserving tail-capping and the existing handling of absent and
non-regular files.

In `@docs/specs/10-spec-diagnostics/architecture.md`:
- Around line 203-210: Update the “Trace propagation (Unit 4)” specification to
make the no-RPC/wire-contract statement conditional on successful SSH channel
environment propagation. Document the traceparent request-body fallback as an
intentional additive contract change, and revise the related R4.4 acceptance
criteria to cover both paths explicitly.
- Around line 66-83: The crate root must expose the capture API used by Unit 5.
Add the public capture module and re-export command_capture and Capture from the
crate root, then add a compile-level API test that verifies
diagnostics::capture::command_capture is available with its Capture result type.
- Around line 138-149: Update DiagBundleRequest to apply explicit serde defaults
to log_tail_bytes and include_state_listing so an empty JSON object decodes with
the documented values (0 and true). Add a deserialization test covering {} and
asserting both default field values.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef982469-3122-4779-bdef-4018663bc057

📥 Commits

Reviewing files that changed from the base of the PR and between 9e9f15e and 439c23b.

📒 Files selected for processing (2)

docs/specs/10-spec-diagnostics/10-spec-diagnostics.md
docs/specs/10-spec-diagnostics/architecture.md

Four mermaid views: crate-boundary system overview with the degraded debugfs path, the full min bug collection sequence, the in-VM log pipeline with the release-before-quiesce ordering, and TRACEPARENT propagation across the CLI-daemon boundary. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Pins the mode-specific manifest paths, upgrades the symlink guard to a no-follow open (TOCTOU-safe), gives the listing API failure/truncation semantics, adds the capture-timeout proof, layers the sensitive-key policy over the env allowlist, sets the per-prefix log-file cap at 5, corrects the Unit 3 artifact locations, standardizes the TRACEPARENT env name, bounds nested-bundle verification against decompression bombs, defines the argv key=value scrub and honest log/argv trust wording, exports the capture API in the curated surface, shows the serde defaults on DiagBundleRequest, and reconciles the traceparent fallback with R4.4. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

New R6.6: the in-VM bundle gains routes+addresses (route + fib_trie), hang triage for the marker-matched family (pid-1, session hosts, task children), the fd-to-socket-inode join as the binary-free lsof equivalent, and allowlisted env. The partial wedge — daemon responsive while one binding or child is stuck, the #788 shape — was invisible to the guest bundle. R5.2 now requires the Linux mechanics be pure /proc reads so pid-1 can run them: the microVM rootfs has no lsof, ss, or ip. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Removes issue/PR numbers and reviewer attributions from the spec and architecture doc. The motivating incident is described as a failure class, review guidance as settled design boundaries, and the prior branch strictly as the reference implementation baselines cite. The tracking-issue frontmatter is unchanged (house convention). Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The bundle transport is itself a streaming RPC — one request, one streamed tar.zst, close — mirroring the wire's existing WorkspaceFilesTarZst subsystem pattern in the reverse direction. The spec's earlier 'not streaming' framing conflated that single-shot form with what it actually rejects: a long-lived stream of live debug data from a running daemon. R6.1 now cites the established pattern, and the Non-Goals, Design Considerations, and Alternatives sections use the precise terms. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Sharpens the Unit 4 boundary from 'zero OTEL deps' to 'zero OTEL runtime dependencies' and adopts the two crates that carry none of the rejected costs: opentelemetry-semantic-conventions (consts only) pins the resource-attribute names, and json-subscriber replaces the built-in JSON formatter — flat records with static top-level resource fields the stock formatter cannot emit, dependency closure already in the workspace, composing over the Unit 3 MakeWriter stack unchanged. Resource identity moves from a root span to top-level statics, which map onto the OTLP Resource exactly. Also corrects the future-seam pricing: prost is already resident, so an opentelemetry-proto OTLP-JSON emitter is cheaper than the general estimate. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…undary Technical Considerations now names the lossy(false) backpressure chain — a wedged volume can propagate through the bounded non_blocking channel into every logging daemon thread — as a deliberate coupling with its two mitigations (console layer independent of the volume; volume fallback collects without the daemon), backed by a new needs-spike ledger row measuring the headroom claim during Unit 3. The adopted-crates rationale states json-subscriber's execution honestly: log-write path of a healthy process only, no pipeline machinery, nothing at collection time. tracing-opentelemetry is named as the future export-seam crate and why it cannot be adopted piecemeal (id generation requires TracerProvider machinery; batch-buffered spans lose the tail on a hang). Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The host-bundle unit's generic mechanics (TOML redaction walker, masked process env, rotated-log selection, disk usage, first-line capture) move into the diagnostics crate at introduction rather than migrating later: each has a second consumer in the daemon bundle. The CLI contributes policy only — allowlist, prefixes, paths, archive layout. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…log ordering The per-crate build.rs the daemon-logs unit patched no longer exists — the shared version crate's build script already falls back cleanly without git, so the no-git-panic requirement is satisfied on the baseline and the unit carries no code for it. Rotated-log selection is documented as modified-time ordered: logrotate-style numeric shifting sorts opposite to date suffixes, so filename order cannot be the contract. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The host-bundle unit's crate additions grew: the system probe joins the surface (disk paths as caller policy, best-effort fields), so the incident-collectors unit no longer migrates generic helpers — its requirement now asserts the crate rule holds rather than re-moving already-moved code. Collector requirements pick up the hardened contracts: filesystem errors are never reported as absence, every content read goes through the shared no-follow open, the listing walk runs on a blocking thread so collector timeouts stay effective, and rotation records the logrotate-style shift plus graceful archive publication that keeps an unmounting volume free of partial files. The system probe drops the version field claim — the manifest records the producer version once. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

logroller's size-based rotation is reverted for a smaller dependency surface and simpler shutdown; tracing-appender's daily appender rotates and prunes inline, so the volume-log release is a plain guard drop with no background thread to join and no partial intermediates on unmount. R3.6 becomes time-based with max_log_files retention; the design consideration, technical consideration, assumption ledger, and system diagrams follow, and the accepted tradeoff (no intra-day size cap) is recorded. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

The reference tree is a single 4107-line branch, but the merged units have reshaped the crate and daemon layout the later units build on. Add a Porting baseline section that maps the current state a fresh executor must reconcile against — the diagnostics crate's already-exported mechanics, the crates/minimal/src/diag layout, the minimald DaemonLogger and rpc dispatch anchor, and the minimald-rpc trace module — so Units 5-8 can be picked up without reverse-engineering the merged PRs. Re-anchor the drifted origin/main citation for the streaming-subsystem pattern on its grep-able constant rather than a line number. Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

…880) * feat(minimal): guest fetch and degraded-mode fallback for `min bug` Per provider, `min bug` now performs the staged socket probe and — when the probe handshakes — downloads the daemon's own bundle over the DiagBundleTarZst subsystem, nesting it under providers/<name>/guest/. --no-guest skips daemon contact entirely; --guest-timeout-secs bounds each provider's download. Host-side log-prefix skips are deferred until the provider loop settles whether the daemon's logs reached the bundle another way, so the manifest never claims an absence the archive does not back. Squashed rebuild of the original branch onto main after #878 landed there, replacing the merge-heavy history whose #889 squash title also failed commitlint. Refs: #802 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(minimald): return the served contract from the diag bundle handler PR #895 changed the serve! dispatch so every handler returns Result<(), ConnectionError> for outcome logging; PR #878 merged alongside it with serve_stream_diag_bundle still returning (). The two were each green against a main that lacked the other, and the type mismatch only surfaced on branches built after both landed. Failure still relays the message over the channel's extended-data stream before surfacing as ConnectionError::Internal, mirroring serve_stream_workspace_files. Refs: #878, #895 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

norrietaylor mentioned this pull request Jul 17, 2026

feat(minimal): add min bug diagnostic bundle command #784

Closed

4 of 5 tasks

norrietaylor marked this pull request as ready for review July 17, 2026 05:09

coderabbitai Bot reviewed Jul 17, 2026

View reviewed changes

norrietaylor and others added 4 commits July 16, 2026 22:24

norrietaylor mentioned this pull request Jul 17, 2026

feat(diagnostics): add the diagnostics crate foundation #803

Merged

3 tasks done

norrietaylor and others added 3 commits July 16, 2026 23:33

norrietaylor mentioned this pull request Jul 17, 2026

feat(minimal,diagnostics): add the min bug host diagnostic bundle command #816

Merged

norrietaylor and others added 5 commits July 17, 2026 13:49

docs(specs): system probe joins the diagnostics crate surface

ae3e91e

Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

docs(specs): drop the superseded unit-5 collect.rs sketch

472ff71

Refs: #801 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

norrietaylor mentioned this pull request Jul 17, 2026

feat(minvmd,minimald,minimal): persistent, correlatable daemon logs #833

Merged

Merge branch 'main' into arch/diagnostics-spec

892a9cc

norrietaylor mentioned this pull request Jul 17, 2026

feat(minimald,minvmd,minimal): OTEL-compatible log format and trace propagation #835

Merged

norrietaylor and others added 2 commits July 20, 2026 10:34

This was referenced Jul 21, 2026

feat(diagnostics,minimal): incident collectors, mechanics in-crate #864

Merged

feat(minimald,minimald-rpc): daemon bundle serving (DiagBundleTarZst) #878

Merged

feat(minimal): guest fetch and degraded-mode fallback for min bug #880

Merged

twitchyliquid64 approved these changes Jul 22, 2026

View reviewed changes

norrietaylor merged commit ffe0921 into main Jul 22, 2026
28 checks passed

norrietaylor deleted the arch/diagnostics-spec branch July 22, 2026 17:13

norrietaylor mentioned this pull request Jul 23, 2026

Diagnostics subsystem: min bug support bundles #801

Closed

7 of 8 tasks

coderabbitai Bot mentioned this pull request Jul 29, 2026

docs: fix all STE prose lint errors across the docs tree #1047

Merged

Navigation Menu

Repository navigation

docs: diagnostics subsystem spec and architecture - #802

docs: diagnostics subsystem spec and architecture#802
norrietaylor merged 16 commits into
mainfrom
arch/diagnostics-spec

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Uh oh!

Summary

What the spec decides

Units

Test plan

Summary by CodeRabbit

Uh oh!

Uh oh!

Reviews paused

Walkthrough

Changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants