Skip to content

refactor(cli): split release scripts into pure rules and CLI shells - #39660

Open
GareArc wants to merge 1 commit into
mainfrom
refactor/cli-release-script-layering
Open

refactor(cli): split release scripts into pure rules and CLI shells#39660
GareArc wants to merge 1 commit into
mainfrom
refactor/cli-release-script-layering

Conversation

@GareArc

@GareArc GareArc commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #39658. That PR stopped the compat-window bump from breaking tests, but left the underlying problem: the release script tests could only observe behaviour through a process exit code, so they had to spawn a subprocess, so their input had to come from cli/package.json. Editing that file kept breaking logic tests.

It was still live. Bumping the difyctl version — which cli-release.yml requires on every release, via a gate that errors with "bump cli/package.json version" — broke 3 tests. Changing tagPrefix or channel broke 1 each. And #39658 had quietly stopped covering the real difyctl.release.targets at all.

die() called process.exit from inside otherwise-pure functions, which is where that chain starts. This splits them:

  • cli/scripts/lib/release-rules.mjs — version and naming rules. Every input an argument, returns values instead of exiting.
  • cli/scripts/lib/edge-manifest.mjs — R2 manifest/index builders, same contract.
  • release-naming.mjs / release-r2-edge.mjs — argv, manifest reading, stdout, exit codes. Nothing else.

The shells themselves are now importable: die() throws a UsageError, the entry block is the only place that prints and exits, and both scripts export main(argv). Their shebangs are gone — every call site (3 workflows, 4 shell scripts) already invokes them via node, and neither file had its exec bit set, so the shebang's only real effect was breaking vitest's Windows module runner on import.

Tests split by what they ask. Logic is unit-tested against literal inputs (no fixture file, no env seam — DIFYCTL_PKG_PATH and test/fixtures/pkg-manifest.ts are both gone). The shells are tested for plumbing by importing main() directly. No release-script test spawns a subprocess anymore. release-config.test.ts is the single place that reads the real manifest, and asserts only that it is internally consistent — never what it currently contains.

Test policy: no integration tests in the unit suite

Five files in the unit suite were integration tests of shell scripts — they spawned sh/pwsh/bash to drive the installers and the R2 publisher end-to-end. Shell has no import boundary, so they cannot become unit tests; the e2e suite has no infra for them yet, so they are removed rather than relocated:

  • install-cli.test.ts, install.ps1.test.ts, install-r2.test.ts, install-r2.ps1.test.ts (69 tests)
  • release-r2-publish.test.ts (2 tests — upload ordering: binaries → verify → survivors → index → manifest, and the no-targets safety gate)

Candidates for the e2e suite when installer coverage lands there. src/http/client-tls.test.ts keeps its openssl spawn — that generates a cert fixture; the assertions are in-process.

Behaviour changes

  1. compat-check compares the numeric A.B.C core only, ignoring prerelease and build suffixes — so Dify 1.16.0-rc1 now satisfies a 1.16.0 window. This matches the shipped runtime check in src/version/compat.ts, which already stripped suffixes; the release gate was the one disagreeing. Removes the hand-rolled prerelease ordering (~45 lines).
  2. validate now rejects a missing or inverted compat window. It previously passed such a config while github-env emitted minDify=undefined into $GITHUB_ENV.
  3. release-r2-edge reports a bad channel version under its own name instead of release-naming's, since it no longer borrows that script's die().

Install scripts

The installers hardcode artifact names by necessity — they run standalone on a user's machine with no repo and no node. Nothing checked the two copies agreed, so changing tagPrefix published dfctl-1.2.3-linux-x64 while all four installers still hunted for difyctl-v*: green CI, broken installs.

release-config.test.ts now pins them to the naming config. Changing tagPrefix or checksumsSuffix fails with the exact installer files to update.

Verification

Every field of cli/package.json mutated in both directions, against the full suite:

Legitimate change version compat channel targetId binName
pass pass pass pass pass

tagPrefix deliberately now fails (exactly the 4 installer-alignment tests) until the installers are updated — see above.

Malformed config emptyTagPrefix dupTargetId emptyTargets version≠channel exeMismatch compatInverted badBunTarget
caught caught caught caught caught caught caught

Mutation-tested the tests themselves, after an earlier round found three assertions that could not fail — deleting the compat-check release gate, emitting a bogus compat window, and dropping validate-version's rejection all passed a full run. Each now trips a test.

CLI behaviour diffed against a pre-refactor baseline: 40 subcommand invocations covering every subcommand and error path of both scripts, byte-identical apart from the three intentional changes above.

  • pnpm test (cli): 1277 passed, 0 failed, 0 skipped
  • vp check cli: clean

Known, not addressed here

Pre-existing, surfaced while reviewing:

  • install.ps1:80 only understands -rc.N when sorting assets; any other prerelease sorts as MaxValue and can outrank a stable release of the same core.
  • The uname→target mapping is duplicated across the installers, so adding a build target does not reach them.
  • install-r2.sh:52 parses index.json line-wise and depends on dir being the last key; no test guards that ordering (verified — reordering it passes the whole suite).

Checklist

  • This change requires a documentation update, included: Dify Document — n/a
  • I understand that this PR may be closed in case there was no previous discussion or issues. (This doesn't apply to typos!)
  • I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
  • I've updated the documentation accordingly.
  • I ran make lint && make type-check (backend) and cd web && pnpm exec vp staged (frontend) to appease the lint gods

The release script tests could only observe behaviour through a process
exit code, because die() called process.exit from inside otherwise-pure
functions. That forced every test to spawn a subprocess, which forced
its input to come from cli/package.json, which is why editing that file
kept breaking tests — most recently #39658, and next the difyctl version
bump that cli-release.yml requires on every release.

Move the decisions into lib/release-rules.mjs and lib/edge-manifest.mjs,
which take every input as an argument and return values instead of
exiting. release-naming.mjs and release-r2-edge.mjs keep argv parsing,
manifest reading, stdout and exit codes, and nothing else.

Tests now split by what they ask. Logic is unit-tested against literal
inputs, the shells are tested for plumbing only, and release-config.test.ts
is the single place that reads the real manifest — asserting it is
internally consistent, never what it currently contains. Editing
cli/package.json no longer breaks a logic test; a malformed config still
fails. Verified by mutating each field in both directions.

Three intentional behaviour changes:

- compat-check compares the numeric A.B.C core only, ignoring prerelease
  and build suffixes, so Dify 1.16.0-rc1 now satisfies a 1.16.0 window.
  This matches the shipped runtime check in src/version/compat.ts, which
  already stripped suffixes; the release gate was the one disagreeing.
  Removes the hand-rolled prerelease ordering entirely.
- validate now rejects a missing or inverted compat window. It previously
  passed such a config while github-env emitted minDify=undefined.
- release-r2-edge reports a bad channel version under its own name rather
  than release-naming's, since it no longer borrows that script's die().

release-config.test.ts also pins the install scripts to the naming config.
They hardcode artifact names by necessity — they run standalone on a user
machine with no repo and no node — so changing tagPrefix or checksumsSuffix
now fails until all four installers are updated to match, instead of
silently publishing names no installer looks for.
@GareArc
GareArc force-pushed the refactor/cli-release-script-layering branch from 9ab8097 to f1c32e1 Compare July 28, 2026 02:15
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.15%. Comparing base (a57b0b9) to head (f1c32e1).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #39660      +/-   ##
==========================================
+ Coverage   86.10%   86.15%   +0.04%     
==========================================
  Files        5075     5232     +157     
  Lines      287042   291169    +4127     
  Branches    57240    58345    +1105     
==========================================
+ Hits       247159   250846    +3687     
- Misses      35133    35573     +440     
  Partials     4750     4750              
Flag Coverage Δ
cli 89.33% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@GareArc
GareArc marked this pull request as ready for review July 28, 2026 03:41
@dosubot dosubot Bot added size:XL This PR changes 500-999 lines, ignoring generated files. refactor labels Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

refactor size:XL This PR changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant