Skip to content

Commit daca707

Browse files
tclemCopilot
andcommitted
Address PR review feedback on update-deps skill
- Fix misleading cargo workspace description (deps are per-crate, not managed at workspace root) - Don't hardcode npm let the agent discover package.json filespaths - Fix 'preserve lockfiles' wording to 'regenerate lockfiles' - Fix reviewer assignment to use CODEOWNERS, not the PR author - Add --paginate to dependabot alert API calls - Fix npm outdated exit code handling (|| true) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 9e873d9 commit daca707

File tree

1 file changed

+9
-15
lines changed

1 file changed

+9
-15
lines changed

.github/skills/update-deps/SKILL.md

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ Supporting packages (not published): `bpe-tests`, `bpe-benchmarks`.
2929

3030
| Ecosystem | Directories | Notes |
3131
|---|---|---|
32-
| **cargo** | `/` (workspace root) | All Rust deps managed at workspace level via `Cargo.lock` |
32+
| **cargo** | `/` (workspace root) | Deps declared per-crate; `Cargo.lock` at workspace root pins versions |
3333
| **github-actions** | `.github/workflows/` | CI and publish workflows |
3434
| **npm** | `crates/string-offsets/js/` | JS bindings for string-offsets (WASM) |
3535

@@ -77,18 +77,18 @@ gh pr list --author 'app/dependabot' --base main --state open --json number,titl
7777
Fetch open dependabot alerts:
7878

7979
```bash
80-
gh api /repos/{owner}/{repo}/dependabot/alerts --jq '[.[] | select(.state=="open") | {number: .number, package: .security_vulnerability.package.name, ecosystem: .security_vulnerability.package.ecosystem, severity: .security_advisory.severity, summary: .security_advisory.summary}]'
80+
gh api --paginate /repos/{owner}/{repo}/dependabot/alerts --jq '[.[] | select(.state=="open") | {number: .number, package: .security_vulnerability.package.name, ecosystem: .security_vulnerability.package.ecosystem, severity: .security_advisory.severity, summary: .security_advisory.summary}]'
8181
```
8282

8383
For ecosystems without dependabot coverage or when running ad-hoc, use native tooling:
8484

8585
- **cargo:** `cargo update --dry-run`
86-
- **npm:** `cd crates/string-offsets/js && npm outdated --json`
86+
- **npm:** find directories containing `package.json`, then run `npm outdated --json || true` in each (npm exits non-zero when updates exist)
8787

8888
Also fetch the advisory URLs for any security-related updates. Individual alert details are at `https://github.com/{owner}/{repo}/security/dependabot/{alert_number}`. Fetch alert numbers and GHSA IDs via:
8989

9090
```bash
91-
gh api /repos/{owner}/{repo}/dependabot/alerts --jq '[.[] | {number: .number, state, package: .security_vulnerability.package.name, ecosystem: .security_vulnerability.package.ecosystem, severity: .security_advisory.severity, ghsa_id: .security_advisory.ghsa_id, summary: .security_advisory.summary}]'
91+
gh api --paginate /repos/{owner}/{repo}/dependabot/alerts --jq '[.[] | {number: .number, state, package: .security_vulnerability.package.name, ecosystem: .security_vulnerability.package.ecosystem, severity: .security_advisory.severity, ghsa_id: .security_advisory.ghsa_id, summary: .security_advisory.summary}]'
9292
```
9393

9494
Include both open and auto_dismissed/dismissed alerts — the update may resolve alerts in any state.
@@ -258,16 +258,10 @@ gh pr close {dependabot_pr_number} --comment "Superseded by #{new_pr_number} whi
258258

259259
### 9. Assign for review
260260

261-
Determine the current user:
261+
Request review from CODEOWNERS or a user-provided reviewer (not the PR author):
262262

263263
```bash
264-
gh api user --jq '.login'
265-
```
266-
267-
Request review:
268-
269-
```bash
270-
gh pr edit {pr_number} --add-reviewer {user_login}
264+
gh pr edit {pr_number} --add-reviewer {reviewer_login}
271265
```
272266

273267
Report the final PR URL and a summary of what was done.
@@ -278,16 +272,16 @@ Report the final PR URL and a summary of what was done.
278272
- **Never push to `main` directly.** Always work on a feature branch.
279273
- **Never push code that doesn't pass `make lint` and `make test`.** If you can't fix it in 3 tries, stop and ask.
280274
- **Be conservative with major version bumps.** If a major version update breaks things and the fix isn't obvious, skip that package and note it in the PR description.
281-
- **Preserve lockfiles.** Always regenerate `Cargo.lock` and `package-lock.json` after updating — don't just edit manifests.
275+
- **Regenerate lockfiles.** Always regenerate `Cargo.lock` and `package-lock.json` after updating — don't just edit manifests.
282276
- **One ecosystem at a time.** Complete the full cycle (update → build → push → PR → CI green) for one ecosystem before moving to the next.
283277
- **If no updates are needed** for an ecosystem, skip it and tell the user.
284278
- **Security alerts take priority.** Address security alerts first within each ecosystem.
285279
- **Clippy is strict.** This repo forbids `unwrap_used` outside tests and denies all warnings. New dependency versions may trigger new clippy lints — fix them.
286280

287281
## Edge cases
288282

289-
- **Cargo workspace:** All Rust dependencies are managed at the workspace root. Always run `cargo update` and `cargo check` from the repo root.
290-
- **npm is scoped to string-offsets:** The only npm package is in `crates/string-offsets/js/`. Don't look for npm elsewhere.
283+
- **Cargo workspace:** Dependencies are declared per-crate but share a single `Cargo.lock` at the workspace root. Always run `cargo update` and `cargo check` from the repo root.
284+
- **npm:** Look for `package.json` files to discover npm packages rather than hardcoding paths — the repo layout may change.
291285
- **WASM builds:** After updating `wasm-bindgen` or related deps, verify `make build-js` still works — WASM toolchain version mismatches are common.
292286
- **Rate limits:** If `gh api` hits rate limits, wait and retry. Report to user if persistent.
293287
- **Nothing to update:** Report cleanly and move to the next ecosystem (or exit).

0 commit comments

Comments
 (0)