fix: surface extension loading errors and improve method extraction

[stack72]

Summary

Fixes #875

• Extension loading errors are now surfaced to users instead of being silently swallowed
• extractContentMetadata now handles models that use imported/shorthand methods
• Only entry point files (not companion files) are passed to the content metadata extractor

Root Cause Analysis

The reporter observed swamp model type describe showing no methods for a model using local imports (companion .ts files in subdirectories). Our investigation found that multi-file models actually bundle and load correctly — the bundler (deno bundle) inlines all local imports, and the runtime model registry has the methods.

The real issue is that extension loading errors are silently swallowed. The loadUserModels function runs during CLI startup before initializeLogging is called. When a model fails validation for any reason (bad resources spec, incompatible zod version, incorrect garbageCollection format, etc.), the logger.warn call has no configured sink and the message is lost. The user sees only "Model type not found" with zero diagnostic information.

We reproduced this by creating a multi-file model with an intentionally invalid resources spec — the model silently failed to load with no visible error output.

What was actually broken

1. Silent error swallowing (src/cli/mod.ts): All five extension loaders (models, vaults, drivers, datastores, reports) logged warnings via logger.warn before logging was initialized. These warnings were discarded.

2. Content metadata extraction (extension_content_extractor.ts): The extractMethods function only matched methods: { ... } (inline definition). Models using methods, (shorthand property) or methods: importedVar (variable reference) returned empty methods in extension push --dry-run output. This is a separate, real bug affecting registry metadata but not the runtime model type describe command.

3. Entry point filtering (push.ts): extractContentMetadata received all model files including companion files, which could be falsely identified as separate model entries.

Changes

src/cli/mod.ts

• Added DeferredWarning interface to collect loading errors during startup
• All five loader functions now collect failures into a shared deferredWarnings array instead of calling logger.warn directly
• Warnings are emitted after initializeLogging completes in globalAction

src/domain/extensions/extension_content_extractor.ts

• Refactored extractMethods to handle three patterns:
  1. Inline: methods: { name: { ... } } (existing)
  2. Shorthand: methods, (new — resolves variable named methods)
  3. Reference: methods: someVar (new — resolves named variable)
• Added resolveMethodsReference and findVariableObjectBody helpers

src/libswamp/extensions/push.ts

• Changed extractContentMetadata call to pass input.modelEntryPoints instead of input.allModelFiles

User Impact

Before: Users with extension models that fail to load see only "Model type not found" with no explanation. This is especially confusing for models using local imports, since the structure looks correct but a subtle validation error (e.g., wrong resources format) causes silent failure.

After: Users see a clear warning like:

WRN Failed to load user "model" "mymodel.ts": "resources.result.schema: Invalid input; resources.result.lifetime: Duration must match pattern..."

This tells them exactly which file failed and why, enabling self-diagnosis.

Test plan

• 3564 unit tests pass
• New tests for shorthand and variable-reference method extraction
• Reproduced silent error with compiled binary — confirmed warning now appears
• Verified multi-file models with valid specs still load correctly
• deno check, deno lint, deno fmt all pass
• deno run compile succeeds

🤖 Generated with Claude Code

[github-actions]

CLI UX Review

Blocking

None.

Suggestions

None.

Verdict

PASS — This PR fixes a silent-failure UX problem. Extension load errors that were previously discarded before logging initialization are now deferred and emitted as warnings after init. The warning format Failed to load user <kind> <file>: <error> is clear, actionable, and consistent with existing patterns. The extension push --dry-run method extraction improvement is an invisible correctness fix for users with shorthand/reference method patterns. No new flags or output formats introduced.

[github-actions]

Code Review

Clean, well-motivated fix. The root cause analysis in the PR description is thorough and the three changes address distinct, real issues.

Blocking Issues

None.

Suggestions

1. Minor: dead check in resolveMethodsReference (src/domain/extensions/extension_content_extractor.ts:414): The check refMatch[1] !== "{" is always true because the capture group (\w+) cannot match { (\w = [a-zA-Z0-9_]). Harmless but could be removed for clarity.

2. DDD note: DeferredWarning is defined as an exported interface in the CLI layer (src/cli/mod.ts) and used only within that file. The export is harmless, but since it's purely a CLI-internal concern (bridging the gap between pre-logging startup and post-logging emission), it could be unexported. Very minor.

3. Concurrency note on deferredWarnings: The shared mutable array is passed to five concurrent Promise.all tasks. This is safe in JS's single-threaded model since Array.push is synchronous, but a brief comment noting this would help future readers understand why it's safe.

Overall this is a solid fix with good test coverage for the new extraction patterns and a clear user-facing improvement for extension loading errors.

[github-actions]

Adversarial Review

Critical / High

None found.

Medium

1. src/domain/extensions/extension_content_extractor.ts:398-410 — Shorthand detection can be defeated by methods: appearing in comments or strings

   resolveMethodsReference checks whether the matched shorthand is really shorthand by scanning all text up to the match for methods\s*:\s*[{\w]. This includes comments, string literals, and unrelated code above the model export.

   Breaking example: A model file with a comment like // methods: {legacy API} or a string like description: "See methods: get, set" before the export const model = { ... methods, } would cause beforeMethods.match(/methods\s*:\s*[{\w]/) to match, skipping the shorthand branch. The function returns null, and the model's methods silently disappear from extracted metadata.

   Suggested fix: Narrow the verification check to only inspect the text within the model/extension object literal, or anchor the methods: check to be within the same object scope as the shorthand match.

2. src/domain/extensions/extension_content_extractor.ts:333-336 — Inline-first strategy can match methods: in unrelated objects

   extractMethodsFromBlock(content, content) searches the entire file for /methods:\s*\{/. If a helper object, type annotation, or unrelated const anywhere in the file happens to contain methods: { ... }, it matches first and the model's actual methods (via shorthand or reference) are never checked.

   Breaking example:

   const helperConfig = { methods: { unused: { description: "Not a real method" } } };
   const realMethods = { sync: { description: "Sync" } };
   export const model = { type: "@test/foo", methods: realMethods };

   extractMethods would return [{ name: "unused", ... }] instead of [{ name: "sync", ... }].

   Note: This is a pre-existing issue (the original code had the same search scope), but the refactoring adds new paths that can never be reached in these cases. Consider scoping the search to the model export's object body.

Low

1. src/cli/mod.ts:498 — Shared mutable array across Promise.all is safe but fragile

   deferredWarnings is pushed to concurrently by 5 async loaders via Promise.all. This is safe due to JS single-threaded execution (each push happens in a separate microtask, never truly concurrent). However, a future refactor using workers or Atomics could break this assumption. A brief comment noting the single-threaded safety assumption would help future readers.

2. src/cli/mod.ts:222-225 — Bare catch swallows all errors including unexpected ones

   The old code logged caught errors at debug level (logger.debug\Skipping user models: ${error}`). The new code uses a bare catch {}` that discards the error entirely. If the loader throws for an unexpected reason (e.g., permission denied on a directory that exists, corrupted file), the user gets zero signal. This is a minor regression in debuggability for the five loader functions.

3. src/domain/extensions/extension_content_extractor.ts:413 — refMatch regex could match methods: type where type is a TS keyword used as identifier

   The regex /methods:\s*(\w+)\s*[,}\n]/ would capture identifiers like methods: type or methods: default. findVariableObjectBody would then look for const type = { which could theoretically match an unrelated variable. In practice this is unlikely since type and default aren't commonly used as variable names for method objects.

Verdict

PASS — The core changes (deferred warnings, entry point filtering) are correct and well-tested. The method extraction improvements work for the stated patterns. The medium findings affect edge cases in static content extraction (not runtime model loading) and do not block.