Skip to content

Conversation

@clavery
Copy link
Collaborator

@clavery clavery commented Jan 15, 2026

This is largely a cleanup PR, removing unused or unnecessary code, refining interfaces, removing internal functions from public API, and making logging more verbose. no real functional changes. Net negative lines of code.

Summary

  • Change loadDwJson() to return {config, path} for proper path tracking in DwJsonSource
  • Make internal utilities internal (remove mapDwJsonToNormalizedConfig, mergeConfigsWithProtection, getPopulatedFields from public exports)
  • Remove deprecated sources option from ResolveConfigOptions
  • CLI commands now use ResolvedB2CConfig directly with .values.* access pattern
  • SDK command classes use factory methods (createB2CInstance(), createMrtAuth(), etc.) instead of manual construction
  • Fix replaceDefaultSources option to work without requiring sourcesAfter
  • Adds verbose trace logging to ConfigSource's so we know where config is coming from and what they contrib.

Bug Fix: account-manager-host from dw.json ignored

Fixed an issue where account-manager-host configured in dw.json was being ignored. The oclif flag had a default value that was always passed as an override to config resolution, preventing dw.json values from being used. Removed the default from the flag definition and rely on the existing fallback in the accountManagerHost getter.

Structured Logging Improvements

Improved structured logging across the SDK to ensure log calls have proper structured fields for machine-parseable output while preserving human-readable messages:

Pattern Applied

// Before (anemic - values only in message):
logger.warn(`Job ${jobId} already running`);

// After (proper structured + readable):
logger.warn({jobId}, `Job ${jobId} already running`);

Files Updated

  • Auth: api-key.ts, basic.ts, oauth.ts, oauth-implicit.ts - Added auth-specific fields (headerName, keyPreview, username, port, duration)
  • Operations: jobs/run.ts, jobs/site-archive.ts, code/deploy.ts, code/versions.ts, code/watch.ts - Added domain-specific fields (jobId, executionId, codeVersionId, cartridgeName, path)
  • Clients: middleware.ts, webdav.ts - Added HTTP fields (method, url, status, duration)

Key Changes

  • Use B2C-specific keys for domain objects (jobId not id, codeVersionId not version)
  • Use simple keys for general-purpose domains (path, method, url, server)
  • Changed reserved Pino field hostname to server in watch.ts

Breaking Changes

  • loadConfig() now returns ResolvedB2CConfig instead of NormalizedConfig
  • Access config values via .values.* property (e.g., this.resolvedConfig.values.hostname)
  • ResolvedConfig type alias removed from exports
  • Internal mapping utilities no longer exported

Test plan

  • All SDK tests pass (83 tests)
  • All CLI tests pass (83 tests)
  • Build succeeds for all packages
  • Manual test with dw.json configuration

@clavery clavery marked this pull request as ready for review January 15, 2026 22:42
@clavery clavery marked this pull request as draft January 15, 2026 23:42
- Change loadDwJson() to return {config, path} for proper path tracking
- Make internal utilities internal (remove mapDwJsonToNormalizedConfig,
  mergeConfigsWithProtection, getPopulatedFields from exports)
- Remove deprecated 'sources' option from ResolveConfigOptions
- CLI commands now use ResolvedB2CConfig directly with .values.* access
- SDK command classes use factory methods (createB2CInstance(),
  createMrtAuth(), etc.) instead of manual construction
- Fix replaceDefaultSources option to work without sourcesAfter

BREAKING CHANGE: loadConfig() now returns ResolvedB2CConfig instead of
NormalizedConfig. Access config values via .values.* property.
@clavery clavery changed the title better logging in config sources for debugging Refactor ConfigSource API for better encapsulation Jan 15, 2026
Add missing structured fields to log calls while preserving
human-readable messages with interpolated values. This makes
logs both machine-parseable and human-friendly.

Changes:
- Add jobId, executionId, path fields to job operations
- Add cartridgeName, codeVersionId fields to code operations
- Add method, url fields to client logging
- Add headerName, keyPreview, username, port fields to auth
- Change reserved 'hostname' field to 'server' in watch.ts
The oclif flag had a default value that was always passed as an
override to config resolution, preventing dw.json values from being
used. Remove the default from the flag and rely on the existing
fallback in the accountManagerHost getter.
@clavery clavery marked this pull request as ready for review January 16, 2026 02:56
@clavery clavery merged commit fd97748 into main Jan 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants