Skip to content

fix(upgrade): honor --dry-run as preview-only instead of silently upgrading#416

Open
jackjin1997 wants to merge 1 commit into
DingTalk-Real-AI:mainfrom
jackjin1997:fix/upgrade-dry-run-364
Open

fix(upgrade): honor --dry-run as preview-only instead of silently upgrading#416
jackjin1997 wants to merge 1 commit into
DingTalk-Real-AI:mainfrom
jackjin1997:fix/upgrade-dry-run-364

Conversation

@jackjin1997
Copy link
Copy Markdown

Summary

  • What changed? dws upgrade now honors --dry-run: it resolves the target release and platform asset (so "already latest" / a missing build for the platform is still reported), prints the 1–5 steps it would perform, and returns before any side effect — no backup, no download, no replace. --dry-run is also advertised in the command examples.
  • Why? newUpgradeCommand registered no --dry-run flag and never read the global persistent one, so --dry-run fell through to runUpgrade and performed a real, irreversible upgrade (download + binary replace). This directly contradicts the flag's documented contract (预览操作内容,不实际执行) and is dangerous: users running a "preview" actually got upgraded.

Fixes #364

Verification

  • make build
  • make lintgo vet passes; staticcheck not installed in my local env (CI covers it)
  • go test ./internal/app/ (incl. new TestWriteDryRunPlan_*, updated help test)
  • make policy
  • ./scripts/policy/check-generated-drift.sh
  • ./scripts/policy/check-command-surface.sh --strict

Notes

  • Semantics chosen: pure preview (zero side effects), aligning with the flag's own description. The dry-run still performs the read-only version/asset resolution so it surfaces "no matching asset for this platform" and "already latest". The alternative (download+verify, skip only the replace) was considered but rejected because it would still write to disk, contradicting "不实际执行".
  • The preview renderer (writeDryRunPlan) is writer-injected and side-effect-free for testability.
  • Scope: limited to the reported dws upgrade path. --dry-run + --rollback is the same "mutating op ignores dry-run" class and could be a small follow-up, but is intentionally out of scope here to keep the change atomic.

…rading

dws upgrade registered no --dry-run flag and never read the global one,
so --dry-run fell through to runUpgrade and performed a real, irreversible
upgrade (download + replace binary). This contradicts the flag's documented
contract (预览操作内容,不实际执行).

Resolve the target release and platform asset (so a missing build / 'already
latest' is still reported), then render the planned 1-5 steps and return
before any side effect: no backup, no download, no replace. Advertise
--dry-run in the command examples.

Fixes DingTalk-Real-AI#364
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.

bug: dws upgrade --dry-run performs the actual upgrade

1 participant