feat(diagnostics): cargo/toolchain pre-flight family (DIAG-002)#34
Open
joshuaboys wants to merge 1 commit into
Open
feat(diagnostics): cargo/toolchain pre-flight family (DIAG-002)#34joshuaboys wants to merge 1 commit into
joshuaboys wants to merge 1 commit into
Conversation
Route cargo/rustup failures through the DIAG-001 formatDiagnostic envelope so a missing toolchain, target, or cargo surfaces as a structured, actionable nxrust error instead of a raw shell failure. - diagnostics.ts: slug-based codes (DIAGNOSTIC_CODES, D-D5), CataloguedDiagnostic + NxrustDiagnosticError, builders for the family, and runWithDiagnostic to classify ENOENT and rustup/cargo stderr. Unknown cargo output passes through unchanged. Secret redaction extended to --token flags. - run-process.ts: tee stderr (live + 64KB tail) so the single cargo chokepoint can classify failures; replaces the bare console.error; settles once across error/close. - rust-toolchain.ts: validateChannelLiteral throws a catalogued invalid-toolchain-literal diagnostic instead of a bare Error. - docs/diagnostics.md: catalogue stub for the six codes. Full suite 225 green; e2e confirms cargo's live colourised output still surfaces through the teed-stderr path. Resolves the codes open question via D-D5.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds the DIAG-002 “cargo/toolchain pre-flight” diagnostic family on top of the existing [nxrust] diagnostic envelope, so common cargo/rustup/toolchain failures become actionable, structured errors instead of raw spawn/stdio failures.
Changes:
- Introduces stable slug-based diagnostic codes (
nxrust:*) and aNxrustDiagnosticErrorwrapper to carry catalogued diagnostics programmatically. - Updates process execution to tee/capture stderr (bounded tail) and classify known cargo/rustup failure shapes into structured diagnostics while leaving unknown cargo output untouched.
- Routes toolchain literal validation errors through the diagnostic catalogue and adds unit/integration test coverage plus docs/changelog/plan updates.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/utils/diagnostics.ts | Adds diagnostic codes + builders and failure classifier (runWithDiagnostic), plus extended secret redaction. |
| src/utils/run-process.ts | Tees stderr with a bounded tail and emits structured diagnostics on classified failures. |
| src/utils/rust-toolchain.ts | Throws a catalogued diagnostic error for invalid toolchain literals. |
| src/utils/diagnostics.spec.ts | Tests diagnostic builders, codes, error wrapper, classifier, and redaction behavior. |
| src/utils/run-process.spec.ts | Integration tests for spawn failure classification and passthrough behavior. |
| docs/diagnostics.md | Adds a diagnostics catalogue stub documenting the new codes, triggers, and fixes. |
| plans/modules/14-diagnostics.aps.md | Records DIAG-002 work item status and design decisions in the plan. |
| CHANGELOG.md | Adds an Unreleased entry describing the new DIAG-002 behavior and public contract notes. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
148
to
150
| - `src/utils/diagnostics.ts` — the shared `formatDiagnostic({ what, why, | ||
| command?, fix, severity? })` helper (spec §6.14 envelope) with secret | ||
| command?, fix, severity? })` helper (spec §6.14 envelope) with secret | ||
| redaction (`redactSecrets` strips `TOKEN`/`SECRET`/`KEY`/`PASSWORD` values |
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
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.
DIAG-002 — Cargo/toolchain pre-flight diagnostic family
The next DIAG slice after DIAG-001 (
formatDiagnostic+doctor, alreadymerged). Built on top of the existing envelope — it does not rebuild it.
Routes cargo/rustup failures through the structured diagnostic so a missing
toolchain/target/cargo surfaces as an actionable nxrust error instead of a raw
shell failure.
What changed
src/utils/diagnostics.ts— slug-based diagnostic codes (DIAGNOSTIC_CODES,DiagnosticCode, D-D5), acodefield onDiagnostic+ aCataloguedDiagnostictype, andNxrustDiagnosticErrorcarrying the code.Builder functions for the family (
cargo-not-found,toolchain-not-installed,target-not-installed,nightly-required,invalid-toolchain-literal, plusgeneric
spawn-failed). ArunWithDiagnosticclassifier maps spawnENOENTon cargo/rustup →
cargo-not-foundand classifies rustup/cargo stderr; unknowncargo output passes through unchanged (no translation of arbitrary cargo
output — module 14 Out-of-Scope). Secret redaction extended to
--token-styleflags (
--token xand--token=x).src/utils/run-process.ts— stderr is teed (live passthrough + 64 KB tailcapture) so the single cargo-invocation chokepoint can classify failures;
replaces the bare
console.error(constraint: noconsole.*outside theenvelope). Settles once across
error/close; signal-kills (code === null)are not classified.
src/utils/rust-toolchain.ts—validateChannelLiteralnow throws acatalogued
invalid-toolchain-literaldiagnostic instead of a bareError(legacy
from <origin>wording preserved, so existing assertions still match).docs/diagnostics.md— catalogue stub for the six codes (trigger + fix).via D-D5; CHANGELOG Unreleased entry.
Tests / verification
diagnostics.spec.ts— builders, codes,NxrustDiagnosticError, classifier,redaction (incl.
--token=form and the stablerequires -Znon-misfire).run-process.spec.ts— integration: ENOENT envelope, classified stderr,unknown pass-through, success path.
Checking smoke …,Finished) still surfaces through the new teed-stderr path (no regressionfrom the
stdiochange).Review
An independent code review pass was run; its two MAJOR findings were fixed in
this branch (double-settle on ENOENT drifting
setMaxListeners; an over-broadrequires -Znightly matcher that could misfire on stable cargo output) plusthe MINOR items (defensive toolchain-quote matching, compile-time code
enforcement via
CataloguedDiagnostic, explicit--token=redaction, redactedspawn-faileddetail). Regression tests added for the two behavioural fixes.Notes
origin/main(the canonical trunk). Localmainhad divergedonto an unpushed planning fork; this PR is built on the published state where
DIAG-001 and the cache-report/doctor work already live.