Skip to content

refactor: pass publish overrides to resolveConfig without re-listing (#50) - #52

Open
jkas2016 wants to merge 1 commit into
mainfrom
refactor/50-publish-overrides-passthrough
Open

refactor: pass publish overrides to resolveConfig without re-listing (#50)#52
jkas2016 wants to merge 1 commit into
mainfrom
refactor/50-publish-overrides-passthrough

Conversation

@jkas2016

Copy link
Copy Markdown
Owner

Summary

Removes the manual override re-listing in runPublish that caused the --mode footgun caught pre-release in #48, and adds command-level threading coverage.

Every command except publish already passes its args/flags straight to resolveConfig (which reads only Overrides keys and ignores extras). runPublish alone hand-listed each field, so a new Overrides field had to be added in a second place — forgetting it silently drops the field in publish only.

Changes

  • src/commands/publish.tsPublishArgs redefined as { path; slug?; title?; force?; dryRun?; profile? } & Overrides; runPublish passes args directly to resolveConfig. Net −11 lines, single source of truth.
  • Command-layer threading tests (the gap that let feat: explicit --mode / HOSTDOC_MODE + mode-derivation footgun warning (#39) #48's bug through — prior coverage was resolveConfig-unit-only):
    • test/publish.test.ts--bucket override reaches the URL
    • test/open.test.ts — bucket/region overrides reach the URL
    • test/list.test.ts — region override reaches row URLs
    • test/rm.test.ts — domain/distribution overrides derive cloudfront (invalidation fires)

Verification

  • npm run typecheck clean
  • npx vitest run — all pass except the pre-existing skill.test.ts preflight case that is environment-dependent (fails identically on main; it asserts "No AWS credentials", which is false on a machine that has AWS creds — CI has none so it passes there)
  • Real CLI (dev mode): publish --dry-run --bucket override-b --region eu-west-1http://override-b.s3-website-eu-west-1.amazonaws.com/...; --mode s3-website over a cloudfront env still forces s3-website.

Non-goals

  • No change to resolveConfig derivation/warning logic.
  • No commander parsing overhaul.

Closes #50

🤖 Generated with Claude Code

…50)

runPublish hand-listed every Overrides field when building the object it
passed to resolveConfig. Each new Overrides field had to be added there
too, and forgetting silently dropped it in publish only — exactly the
footgun that hid the `--mode` bug fixed in #48.

Redefine PublishArgs as `{ path; slug?; title?; force?; dryRun?; profile? }
& Overrides` and pass `args` straight to resolveConfig (which reads only
the Overrides keys), matching how open/rm/list already thread overrides.
Single source of truth, no second sync point.

Add command-level threading coverage (publish/open/rm/list) so a dropped
override field is caught at the command layer, not just in resolveConfig
unit tests.

Closes #50
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.

[Refactor] publish가 Overrides 필드를 손으로 재나열 — 재발 방지 + 커맨드 계층 통합 테스트 (#39/#48 후속)

1 participant