fix(extensions): surface type-extraction warnings in doctor JSON output (swamp-club#351)

[stack72]

Summary

• Adds a warnings[] field to swamp doctor extensions --json output to surface extension load warnings (e.g. non-literal type fields that prevent static catalog extraction)
• Warnings are advisory — overallStatus and exit code are unaffected
• Wires structured warning collection through DI callbacks (getWarnings/resetWarnings) on DoctorExtensionsDeps, matching the getRecentTransitions pattern
• Resets warning state at each doctor invocation start to prevent stale bootstrap warnings from leaking

Each warning entry has the schema: { sourcePath, category, message } — the category field future-proofs for additional warning types.

Key design decision

Triage verified that extensions with non-literal type fields work correctly at runtime (type search, method execution, and describe all succeed). The original issue framing ("model doesn't register") was incorrect — the failure is only in static regex-based catalog extraction. This means a new TypeExtractionFailed RowState would have produced false alarms for working extensions. The warnings[] surface treats these as what they are: diagnostic findings about suboptimal-but-working patterns.

Closes swamp-club#351.
Follow-up: swamp-club#353 (docs update for JSON schema).

Test Plan

• 3 new unit tests in doctor_test.ts pin reset-at-invocation semantics (stale leak prevention, during-loader appearance, no double-counting)
• Extended silent_load_failure_test.ts — asserts warnings[] in doctor JSON with TypeExtractionFailed category
• Extended doctor_extensions_failure_states_test.ts — warnings-only scenario: overallStatus=pass, exit 0, warnings[] populated
• Updated renderer test fixtures with warnings: [] field
• Verified against reproduction repo: swamp doctor extensions --json shows warning, exit code 0, overallStatus: "pass"
• Full test suite: 5930 passed, 0 failed

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

1. Log-mode: missing actionable hint after warnings — The orphan-files section (line 307–310) follows its warning header with a dim hint explaining what to do next (swamp extension pull --force / swamp extension update). The new warnings section (lines 314–330) shows sourcePath and message but stops there. A short dim hint — e.g. Use a string literal for the typefield (e.g.type: '@org/name') to enable static catalog extraction. — would give users a clear next action without having to look up the docs.

2. Log-mode: slight wording drift — Orphan files says (warnings, not failures); the new warnings section says (advisory, not failures). Both are clear, but aligning on one phrase would be cleaner.

3. Stale comment — Line 387–388: // All five keys are always present — there are now more than five keys. Code-quality only, no UX impact.

Verdict

PASS — warnings[] is correctly added to JSON output (always present, empty array on clean run), log-mode renders with consistent icon/color/indentation, exit code and overallStatus are unaffected by warnings. No blocking issues.

[github-actions]

Code Review

Clean, well-structured PR. The warning plumbing follows the established getRecentTransitions DI-callback pattern exactly, the domain layer stays free of infrastructure dependencies, and the test coverage is thorough (unit + integration + renderer fixtures).

Blocking Issues

None.

Suggestions

1. Dead variable in doctor_test.ts:553 — const warnings: DoctorWarning[] = [] is declared, never populated, and silenced with void warnings. Looks like leftover scaffolding — can be removed.

2. category typed as string — DoctorWarning.category is a plain string, but the only value produced today is "TypeExtractionFailed". A string literal union (e.g., category: "TypeExtractionFailed") that can be extended later would give consumers autocomplete and exhaustive-switch support while keeping the future-proofing intent.

3. Hardcoded category in getWarnings callback (src/cli/commands/doctor_extensions.ts:280) — All events from the warning emitter get mapped to category: "TypeExtractionFailed", which is correct today since that's the only warning type. If a second warning type is added to the emitter, this mapping would silently assign the wrong category. Consider either attaching the category at emit time (in the infrastructure layer) or adding a brief comment noting the assumption.

All three are nice-to-haves — the PR is correct and ready to merge as-is.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. src/cli/commands/doctor_extensions.ts:280 — Hardcoded category: "TypeExtractionFailed" for all warnings.
   The getWarnings callback maps every ExtensionLoadWarningEvent to category: "TypeExtractionFailed" regardless of what warning was actually emitted. Today this is correct — emitTypeExtractionFailure is the only production caller of emitExtensionLoadWarning (confirmed via extension_loader.ts:753). However, the PR description states the category field "future-proofs for additional warning types," yet the mapping doesn't source the category from the event — it hardcodes it. A future developer adding a new warning kind through emitExtensionLoadWarning would get silently miscategorized output.

   Breaking scenario: Someone adds emitExtensionLoadWarning({ kind: "vault", file, error: "manifest checksum mismatch" }) — the JSON output would label it TypeExtractionFailed instead of a distinct category.

   Suggested fix: Either (a) add a category field to ExtensionLoadWarningEvent so the emitter site declares its own category, or (b) add a comment at the mapping site noting this assumption so a future developer knows to update it. Not blocking since the invariant holds today.

Low

1. src/libswamp/extensions/doctor_test.ts:553 — Unused warnings variable. const warnings: DoctorWarning[] = [] is created, never populated, and void-ed at line 569. Looks like a leftover from an earlier approach. Harmless dead code but slightly confusing.

2. src/infrastructure/logging/extension_load_warnings.ts:98-102 — events array grows without bound within a process. Each unique (kind, file, error) tuple appends to events[]. In the doctor flow this is fine because resetExtensionLoadWarnings() is called at each invocation. In a hypothetical long-running process that never calls reset, the array grows monotonically. Non-issue for a CLI tool, but worth noting.

Verdict

PASS — Clean, well-structured change. The warning collection/reset lifecycle is correct: resetWarnings clears stale state at the top of doctorExtensions, loaders re-emit fresh warnings during ensureLoaded, and getWarnings reads them after all loaders complete. Deduplication, reset semantics, and double-invocation are all tested. The overallStatus is correctly unaffected by warning presence. JSON and log renderers both handle the new field. The one design concern (hardcoded category) is valid but not a current bug.