|
| 1 | +--- |
| 2 | +name: review-service-upgrades |
| 3 | +description: "Expert dependency upgrade reviewer for MMO FES services. Use when: reviewing npm dependencies for vulnerabilities, upgrading packages safely, running npm audits, producing upgrade PRs with before/after evidence." |
| 4 | +--- |
| 5 | + |
| 6 | +# Service Dependency Upgrade Review Skill |
| 7 | + |
| 8 | +Expert in safe npm dependency upgrades, vulnerability auditing, and producing clean upgrade PRs with full audit traceability. |
| 9 | + |
| 10 | +## When to Use |
| 11 | + |
| 12 | +- Reviewing npm dependencies for known vulnerabilities |
| 13 | +- Applying safe non-breaking upgrades (patch/minor only) |
| 14 | +- Fixing transitive vulnerabilities via `"overrides"` in `package.json` |
| 15 | +- Producing auditable upgrade PRs with before/after evidence |
| 16 | +- Establishing or verifying `.nvmrc` Node version alignment |
| 17 | + |
| 18 | +## Key Commands |
| 19 | + |
| 20 | +- Install from lockfile: `npm ci` |
| 21 | +- Audit vulnerabilities: `npm audit` |
| 22 | +- Non-breaking auto-fix: `npm audit fix` |
| 23 | +- Check outdated packages: `npm outdated` |
| 24 | +- Targeted upgrade: `npm install <pkg>@<version>` |
| 25 | +- Install report tool (once): `npm i -g npm-audit-markdown` |
| 26 | +- Generate audit report (Confluence/PR): `npm audit --json | npm-audit-markdown --output report.md` |
| 27 | + |
| 28 | +## Upgrade Workflow (5 Steps) |
| 29 | + |
| 30 | +### Step 0: Prep |
| 31 | + |
| 32 | +1. **Ensure `npm-audit-markdown` is installed** |
| 33 | + |
| 34 | + - Check: `npm-audit-markdown --version` |
| 35 | + - If not found, install globally: `npm i -g npm-audit-markdown` |
| 36 | + - This is a one-time setup per machine. It converts `npm audit --json` output into a readable Markdown report for PRs and Confluence. |
| 37 | + |
| 38 | +2. **Capture work identifiers before branching** |
| 39 | + |
| 40 | + - Ask for and confirm: |
| 41 | + - Ticket number (for example: `ABC-123`) |
| 42 | + - Sprint number (for example: `182`) |
| 43 | + - Short work summary slug (for example: `review-service-to-identify-if-upgrades-are-required`) |
| 44 | + |
| 45 | +3. **Checkout a fresh branch** |
| 46 | + |
| 47 | + - `git checkout -b feature/<ticket>-sprint-<sprint>-<summary-slug>` |
| 48 | + |
| 49 | +4. **Clean slate** |
| 50 | + |
| 51 | + - Ensure no local build artefacts; confirm correct Node version (use `.nvmrc` / `.node-version` if present). |
| 52 | + |
| 53 | +5. **Ensure `.nvmrc` exists and is correct** |
| 54 | + - If absent, create from preferred source order: |
| 55 | + - `package.json` → `engines.node` |
| 56 | + - Existing `.node-version` |
| 57 | + - Team standard/default (document what was used) |
| 58 | + - Example: `echo "24" > .nvmrc`, then `nvm use` |
| 59 | + |
| 60 | +### Step 1: Establish a Baseline |
| 61 | + |
| 62 | +1. **Install exactly what the lockfile specifies** |
| 63 | + |
| 64 | + - `npm ci` (preferred for reproducibility) |
| 65 | + - If no lockfile exists, use `npm install` and consider adding one. |
| 66 | + |
| 67 | +2. **Run tests/build once (baseline sanity)** |
| 68 | + |
| 69 | + - `npm test` and `npm run build` |
| 70 | + - Optional but helpful: `npm run lint` |
| 71 | + |
| 72 | +3. **Create a rollback checkpoint** |
| 73 | + - `git add package.json package-lock.json .nvmrc 2>/dev/null || true` |
| 74 | + - `git commit -m "chore: baseline before dependency upgrades"` |
| 75 | + - To avoid a commit, keep working tree clean and use: `git checkout -- package.json package-lock.json` |
| 76 | + |
| 77 | +### Step 2: Vulnerability Check |
| 78 | + |
| 79 | +1. **Run the audit**: `npm audit` |
| 80 | + |
| 81 | +2. **Capture the output** — note: |
| 82 | + |
| 83 | + - Package(s) affected |
| 84 | + - Severity |
| 85 | + - Whether it is **direct** or **transitive** |
| 86 | + - Recommended fix version (if available) |
| 87 | + |
| 88 | +3. **Generate the BEFORE report** (for PR and Confluence): |
| 89 | + ```bash |
| 90 | + npm audit --json | npm-audit-markdown --output report-before.md |
| 91 | + ``` |
| 92 | + Keep `report-before.md` — it will be attached to the PR and copied to Confluence. |
| 93 | + |
| 94 | +> Tip: If the repo is noisy with transitive issues, focus on _direct deps_ first — upgrades there often lift the transitive chain. |
| 95 | +
|
| 96 | +### Step 3: Apply Fixes Safely |
| 97 | + |
| 98 | +Work from least risky to most risky: |
| 99 | + |
| 100 | +#### A) Safe automated patch/minor fix |
| 101 | + |
| 102 | +1. `npm audit fix` — never use `--force`, it can introduce breaking majors |
| 103 | +2. Re-run `npm audit` |
| 104 | + |
| 105 | +#### B) Targeted upgrades for direct dependencies (minor/patch only) |
| 106 | + |
| 107 | +1. `npm outdated` — see what is outdated |
| 108 | +2. **Upgrade one dependency at a time with validation:** |
| 109 | + 1. `npm install <pkg>@<target_non_breaking_version>` |
| 110 | + 2. `npm test` |
| 111 | + 3. `npm run build` |
| 112 | + 4. If either fails: |
| 113 | + - Immediately revert: `git checkout -- package.json package-lock.json` |
| 114 | + - Record the package name and error summary in your report |
| 115 | + - Continue with the next package |
| 116 | + 5. If both pass: keep the change and continue |
| 117 | + |
| 118 | +#### C) Transitive-only vulnerabilities |
| 119 | + |
| 120 | +1. **Upgrade the nearest direct dependency** that brings in the vulnerable transitive. |
| 121 | +2. If still stuck, add an `"overrides"` entry in `package.json` to force a safe transitive version. |
| 122 | +3. Document why overrides were used — they are powerful, and future maintainers need the context. |
| 123 | + |
| 124 | +### Step 4: Validate the Repo Still Works |
| 125 | + |
| 126 | +1. `npm test` |
| 127 | +2. `npm run build` |
| 128 | +3. Smoke check the app (if it is a UI) |
| 129 | +4. Optional: run e2e or contract tests if the repo has them |
| 130 | + |
| 131 | +5. **Generate the AFTER report** (for PR and Confluence): |
| 132 | + ```bash |
| 133 | + npm audit --json | npm-audit-markdown --output report.md |
| 134 | + ``` |
| 135 | + - `report.md` = final clean state — attach to the PR and paste into Confluence. |
| 136 | + - `report-before.md` = baseline captured in Step 2 — keep both for a clear before/after diff. |
| 137 | + |
| 138 | +### Step 5: Produce a Clean PR |
| 139 | + |
| 140 | +1. **Commit only what is needed**: `package.json` and `package-lock.json` |
| 141 | + |
| 142 | +2. **Commit message** — ticket number is mandatory in every message: |
| 143 | + |
| 144 | + - Format: `<type>(<scope>): <change summary> <TICKET-NUMBER>` |
| 145 | + - Example: `chore(deps): upgrade direct dependencies and patch transitive tar vulnerability ABC-123` |
| 146 | + |
| 147 | +3. **Suggested commit sequence**: |
| 148 | + |
| 149 | + - Baseline/setup (optional): `git commit -m "chore(setup): align node version and baseline checks ABC-123"` |
| 150 | + - Dependency upgrade: `git commit -m "chore(deps): apply non-breaking dependency upgrades ABC-123"` |
| 151 | + - Overrides (only if used): `git commit -m "chore(deps): add npm overrides for transitive vulnerability mitigation ABC-123"` |
| 152 | + |
| 153 | +4. **PR description must include**: |
| 154 | + |
| 155 | + - What you ran: `npm ci`, `npm audit`, `npm audit fix`, `npm outdated`, update approach |
| 156 | + - What changed (high level) |
| 157 | + - Before/after audit summary (vuln counts by severity) |
| 158 | + - Any overrides and why |
| 159 | + - Test evidence (commands + results) |
| 160 | + - Rollback log (packages reverted due to test/build failures) |
| 161 | + - `.nvmrc` status (created/updated + value) |
| 162 | + |
| 163 | +5. **Print the completed PR description** using the template below — fill in all placeholders, then output it in full so it can be copied directly into the GitHub PR and Confluence: |
| 164 | + |
| 165 | +```markdown |
| 166 | +--- |
| 167 | +**Branch:** `feature/<ticket>-sprint-<sprint>-<summary-slug>` |
| 168 | + |
| 169 | +### .nvmrc |
| 170 | + |
| 171 | +<Created / Already existed> with `<node-version>` (from `package.json engines.node: <engines-value>`). |
| 172 | + |
| 173 | +### Audit: before → after |
| 174 | + |
| 175 | +| Severity | Before | After | |
| 176 | +| :-- | --: | --: | |
| 177 | +| Critical | <n> | **<n>** | |
| 178 | +| High | <n> | **<n>** | |
| 179 | +| Moderate | <n> | **<n>** | |
| 180 | +| Low | <n> | **<n>** | |
| 181 | + |
| 182 | +### Upgrades applied (all tests + build pass) |
| 183 | + |
| 184 | +| Package | Change | |
| 185 | +| :-- | :-- | |
| 186 | +| `<pkg>` | `<old>` → `<new>` | |
| 187 | + |
| 188 | +### Overrides added |
| 189 | + |
| 190 | +| Override key | Version | Reason | |
| 191 | +| :-- | :-- | :-- | |
| 192 | +| `<pkg>` | `<version>` | `<why>` | |
| 193 | + |
| 194 | +_Remove this section if no overrides were added._ |
| 195 | + |
| 196 | +### Reverted |
| 197 | + |
| 198 | +_<List any packages reverted due to test/build failures, or `None.`>_ |
| 199 | + |
| 200 | +### Final state |
| 201 | + |
| 202 | +- `npm test` ✅ |
| 203 | +- `npm run build` ✅ |
| 204 | +- `npm run lint` ✅ |
| 205 | +- `npm audit` → `found 0 vulnerabilities` ✅ |
| 206 | + |
| 207 | +Full evidence in [report.md](report.md). |
| 208 | + |
| 209 | +--- |
| 210 | + |
| 211 | +### What was run |
| 212 | + |
| 213 | +1. `npm ci` |
| 214 | +2. `npm audit` (baseline — see `report-before.md`) |
| 215 | +3. `npm audit fix` |
| 216 | +4. `npm outdated` |
| 217 | +5. Targeted one-by-one upgrades (minor/patch only) with `npm test && npm run build` after each |
| 218 | +6. `npm audit` (final — see `report.md`) |
| 219 | + |
| 220 | +--- |
| 221 | +``` |
| 222 | + |
| 223 | +## Key Rules |
| 224 | + |
| 225 | +- **Never** use `npm audit fix --force` — it can introduce breaking major versions |
| 226 | +- Upgrade **one dependency at a time** and validate after each change |
| 227 | +- **Immediately revert** `package.json` + `package-lock.json` on any test or build failure |
| 228 | +- Include the **ticket number** in every commit message |
| 229 | +- `"overrides"` are powerful — always document why they were added |
| 230 | +- Run `npm audit --json | npm-audit-markdown --output report-before.md` **before** fixes and `npm audit --json | npm-audit-markdown --output report.md` **after** — attach both to the PR and copy to Confluence |
| 231 | +- `npm-audit-markdown` is a reporting tool only — it does not fix vulnerabilities |
| 232 | + |
| 233 | +## Workflow |
| 234 | + |
| 235 | +1. Check `npm-audit-markdown` is installed: `npm-audit-markdown --version` (install with `npm i -g npm-audit-markdown` if absent) |
| 236 | +2. Ask for ticket number + sprint number + summary slug |
| 237 | +3. `git checkout -b feature/<ticket>-sprint-<sprint>-<summary-slug>` |
| 238 | +4. Ensure `.nvmrc` exists/updated, then `nvm use` |
| 239 | +5. `npm ci` |
| 240 | +6. Baseline: `npm test && npm run build` |
| 241 | +7. `npm audit` |
| 242 | +8. Generate BEFORE report: `npm audit --json | npm-audit-markdown --output report-before.md` |
| 243 | +9. `npm audit fix` |
| 244 | +10. `npm audit` |
| 245 | +11. `npm outdated` |
| 246 | +12. Upgrade direct deps one-by-one (minor/patch only) |
| 247 | +13. After each package: `npm test && npm run build` |
| 248 | +14. On failure: revert `package.json` + `package-lock.json`, log failure, continue |
| 249 | +15. Final: `npm test && npm run build && npm audit` |
| 250 | +16. Generate AFTER report: `npm audit --json | npm-audit-markdown --output report.md` |
| 251 | +17. Commit using ticket in message |
| 252 | +18. Print the completed PR description template (see Step 5 above) for copy-paste into GitHub PR and Confluence |
0 commit comments