|
| 1 | +# Migration Plan — Bundled JS Action → Hybrid Composite Action |
| 2 | + |
| 3 | +## Goal |
| 4 | + |
| 5 | +Replace the bundled TypeScript GitHub Action (compiled with `ncc` into a committed |
| 6 | +`dist/index.js`) with a **hybrid composite action**. The win we're after is killing |
| 7 | +the build step and the committed bundle, shrinking the dependency surface, and making |
| 8 | +what runs match what you read in `action.yml`. |
| 9 | + |
| 10 | +## Why hybrid (not pure shell) |
| 11 | + |
| 12 | +Most of the action is orchestration around two CLIs (`dotnet format`, `jscpd`) plus |
| 13 | +GitHub plumbing — all of which is *simpler* as composite steps. But two parts are |
| 14 | +error-prone in bash: |
| 15 | + |
| 16 | +1. **Config merging** — a 3-way deep merge (code defaults → root config → workspace |
| 17 | + config) over JSON **and** YAML, with array de-duplication and the |
| 18 | + `isEnabled ?? isEabled ?? default` fallback plus simple-vs-granular precedence. |
| 19 | +2. **`--include`/`--exclude` list building** — space-joined argument lists are a |
| 20 | + classic bash quoting hazard. |
| 21 | + |
| 22 | +Those two stay in small, unit-tested JavaScript helpers. Everything else becomes |
| 23 | +shell or [`actions/github-script`](https://github.com/actions/github-script). |
| 24 | + |
| 25 | +## Three kinds of steps |
| 26 | + |
| 27 | +| Kind | Used for | |
| 28 | +|---|---| |
| 29 | +| **shell** | run the CLIs: `dotnet format`, `jscpd`, `git`, nuget restore | |
| 30 | +| **`actions/github-script`** | anything touching the GitHub API or `@actions/core`: changed-files lookup, PR comments, job summary, outputs, annotations, problem matcher | |
| 31 | +| **tested `scripts/*.mjs` helpers** | the two error-prone pure-logic bits (config merge, report→markdown), `require()`d from a github-script step so they stay unit-testable | |
| 32 | + |
| 33 | +`actions/github-script` hands you a pre-authenticated `github` (octokit), `context`, |
| 34 | +`core`, `glob`, `io`, and `exec` with **no build and no committed bundle** — removing |
| 35 | +almost the entire reason `dist/` exists today. |
| 36 | + |
| 37 | +## Architecture / step sequence |
| 38 | + |
| 39 | +`action.yml` becomes `runs.using: "composite"`, same `inputs` and `outputs`. Data flows |
| 40 | +between steps via a normalized JSON file in `$RUNNER_TEMP` (resolved config + ready-to-run |
| 41 | +arg arrays) and small step outputs for scalars. |
| 42 | + |
| 43 | +> **Gotcha baked in everywhere:** composite steps don't auto-expose `inputs` to the |
| 44 | +> step body. Every shell/github-script step receives what it needs via `env:`, and |
| 45 | +> github-script auth uses `github-token: ${{ inputs.authToken }}`. |
| 46 | +
|
| 47 | +1. setup env + `::add-matcher::` (shell) |
| 48 | +2. resolve config → `$RUNNER_TEMP/df-config.json` (github-script → `read-config.mjs` + `format-args.mjs`) |
| 49 | +3. changed files, if `onlyChangedFiles` + PR event (github-script) |
| 50 | +4. `dotnet format` per enabled block (shell, consumes config json) |
| 51 | +5. format report → markdown → summary + PR comment (github-script → `dotnet-report.mjs`) |
| 52 | +6. `actions/upload-artifact@v7.0.1` (dotnet report) |
| 53 | +7. commit & push with rebase-retry + `hasChanges` output (shell) |
| 54 | +8. `::remove-matcher::` (shell) |
| 55 | +9. jscpd run (shell, PATH-or-`npx jscpd@5`) |
| 56 | +10. jscpd report → threshold → annotations → comment → summary + `hasDuplicates` (github-script → `jscpd-report.mjs`) |
| 57 | +11. `actions/upload-artifact@v7.0.1` (jscpd report) |
| 58 | +12. `failFast` gate (shell) |
| 59 | + |
| 60 | +Note: artifact upload is the one thing github-script can't do (no `@actions/artifact` |
| 61 | +bundled), so it uses the standard `upload-artifact` action — simpler than the current |
| 62 | +`DefaultArtifactClient` code anyway. |
| 63 | + |
| 64 | +## Task list (commit after each) |
| 65 | + |
| 66 | +### Phase 0 — scaffold |
| 67 | +- **T0**: Worktree `../dotnet-format-plus-composite` on branch `feat/composite-action`. ✅ |
| 68 | + |
| 69 | +### Phase 1 — tested helpers (port logic 1:1) ✅ |
| 70 | +- **T1** ✅: `scripts/merge.mjs` (deep-merge + dedupe, replaces `deepmerge`) + |
| 71 | + `scripts/read-config.mjs` — port `readConfig` 3-way merge (defaults → root → workspace), |
| 72 | + JSON+YAML (parser injected), array-dedup. `isEnabled ?? isEabled ?? default` + |
| 73 | + simple-vs-granular precedence live in `format-args.mjs`. |
| 74 | +- **T2** ✅: `scripts/format-args.mjs` — port `buildArgs` (esp. `--include`/`--exclude` |
| 75 | + list joining) so the shell step never does bash quoting, plus `buildDefaultOptions`, |
| 76 | + `finalizeEnabled`, `checkIsDryRun`, and a `planFormat` convenience emitting ready-to-run |
| 77 | + `dotnet` argv arrays + `isDryRun`. |
| 78 | +- **T3** ✅: `scripts/report-common.mjs` (shared footer) + `scripts/dotnet-report.mjs` + |
| 79 | + `scripts/jscpd-report.mjs` — port JSON→markdown with the GitHub blob links (context |
| 80 | + injected, not read from `@actions/github`). |
| 81 | +- **T4** ✅: Ported the existing `node:test` suite onto these helpers (config-merge |
| 82 | + / arg / report coverage). YAML via inline `js-yaml` parse or `npx -y js-yaml` conversion — |
| 83 | + no repo `node_modules` needed at action runtime. |
| 84 | + |
| 85 | +### Phase 2 — composite skeleton ✅ |
| 86 | +- **T5** ✅: Rewrite `action.yml` → composite, same inputs/outputs; ship `problem-matcher.json` |
| 87 | + at action root; matcher add/remove via workflow commands. |
| 88 | + |
| 89 | +### Phase 3 — dotnet format path ✅ |
| 90 | +- **T6** ✅: config-resolve step → temp JSON. |
| 91 | +- **T7** ✅: changed-files github-script step. |
| 92 | +- **T8** ✅: format runner (shell loop over blocks). |
| 93 | +- **T9** ✅: report→summary→comment github-script step (reuse existing-comment-by-header + |
| 94 | + bot-user matching). |
| 95 | +- **T10** ✅: `upload-artifact` (dotnet). |
| 96 | +- **T11** ✅: commit/push shell step + `hasChanges`. |
| 97 | + |
| 98 | +### Phase 4 — jscpd path ✅ |
| 99 | +- **T12** ✅: jscpd runner (shell). |
| 100 | +- **T13** ✅: jscpd report github-script step (threshold, annotations, comment, summary, |
| 101 | + `hasDuplicates`). |
| 102 | +- **T14** ✅: `upload-artifact` (jscpd) plus cleanup after upload. |
| 103 | + |
| 104 | +### Phase 5 — failFast + teardown of the build pipeline ✅ |
| 105 | +- **T15** ✅: `failFast` gate. |
| 106 | +- **T16** ✅: Delete `dist/`, drop `package`/`ncc`/`finalize-dist`, remove `@actions/*`, |
| 107 | + `@octokit/rest`, `deepmerge` from deps (keep only what helpers/tests need). |
| 108 | + `package.json` shrinks to lint + test. |
| 109 | +- **T17** ✅: Update CI: drop the dist-sync check; run biome + helper tests only. |
| 110 | + |
| 111 | +### Phase 6 — docs & end-to-end ✅ |
| 112 | +- **T18** ✅: Rewrite README (no build step) + WALKTHROUGH for the composite migration. |
| 113 | +- **T19** ✅: Local verification completed (`pnpm install --frozen-lockfile`, `pnpm run |
| 114 | + format-check`, `pnpm test`, `pnpm all`, `git diff --check`, action metadata parse). |
| 115 | + GitHub-hosted end-to-end confirmation remains the release gate: run |
| 116 | + `test-dotnet-format.yml` (both fixture jobs) and verify PR comment, summary, |
| 117 | + `hasChanges`/`hasDuplicates`, and annotations. |
| 118 | + |
| 119 | +## Parity-risk checklist (things that can silently diverge) |
| 120 | + |
| 121 | +- 3-way deep merge + array dedup + `isEabled` fallback (T1) — highest risk; the tests in |
| 122 | + T4 exist to pin it. |
| 123 | +- `--include` with changed-files list quoting — handled by keeping it in JS (T2). |
| 124 | +- Existing-comment matching: header `startsWith` + `user.type === 'Bot'` / login (T9/T13). |
| 125 | +- Non-PR events (push): comments skipped, summary still written. |
| 126 | +- Outputs are exactly the strings `"true"` / `"false"`. |
| 127 | + |
| 128 | +## Net result |
| 129 | + |
| 130 | +No bundler, no committed `dist/`, dependency surface drops from 8 runtime deps to ~1, |
| 131 | +and the only "real code" left is ~4 small tested helpers. |
0 commit comments