Skip to content

chore(deps): remove dependabot.yml, fold action bumps into the monthly sweep - #2239

Merged
cliffhall merged 4 commits into
v2/chore/2231-audit-fix-and-npm-outdatedfrom
v2/chore/2235-remove-dependabot-config
Sep 4, 2026
Merged

chore(deps): remove dependabot.yml, fold action bumps into the monthly sweep#2239
cliffhall merged 4 commits into
v2/chore/2231-audit-fix-and-npm-outdatedfrom
v2/chore/2235-remove-dependabot-config

Conversation

@cliffhall

@cliffhall cliffhall commented Sep 4, 2026

Copy link
Copy Markdown
Member

Closes #2235

Switches the old dependency flow off, now that #2232 has landed the replacement. This is the only PR that touches .github/dependabot.yml — and it removes the file outright.

Stacked on #2232. It edits files that PR introduces, so its base is v2/chore/2231-audit-fix-and-npm-outdated, not v2/main. GitHub will retarget it to v2/main automatically once #2232 merges.

1. The five npm entries are gone

Root plus clients/{web,cli,tui,launcher}. Routine npm bumps are the #2232 sweep's job from here on.

2. The github-actions entry is gone too — option A

#2235 laid out three ways to resolve the entry #2229 left live, and A is what's implemented here: fold action bumps into the monthly sweep.

The entry had exactly the property #2229 exists to remove — it opened a grouped monthly PR carrying no Closes #N and no board card. Leaving it was the one standing exception to "every PR references an issue", enforced by nothing. Deleting it without a replacement would instead have left 9 actions unwatched, and npm outdated says nothing about actions, so scripts/dependency-refresh.mjs gained a release-tag check:

new export does
parseActionRefs pulls owner/repo@ref out of a workflow file
parseVersionRef splits a numeric ref, or null for a SHA/branch
isActionStale ranks a ref against a release tag
staleActions dedupes, drops unknowns, sorts

Stale actions render as one more section in the same monthly tracking issue:

### GitHub Actions

| Action | Current | Latest |
| --- | --- | --- |
| `actions/cache` | v6 | v7.0.0 |

The comparison is to the precision the ref specifies, which is the one subtle bit. v7 is a moving major tag that GitHub repoints at every v7.x release, so v7 against a latest of v7.0.1 is up to date and only v8 makes it stale. An exactly-pinned v7.0.0 is behind v7.0.1. A SHA pin is deliberately immovable, so a tag comparison says nothing about it and it is never reported.

latestReleaseTag treats every non-zero response as fatal, 404 included. Nothing is suppressed: the release list endpoint answers "this action cuts no releases" with a successful empty array, so a 404 means the repository is missing or inaccessible — a uses: ref the sweep cannot check at all — and swallowing it would silently drop a broken or renamed action.

That distinction matters more than it looks: an API error read as "no release" is indistinguishable from "not stale", so the sweep would exit green having checked nothing, and because this repo's actions are all on their latest major the resulting empty section would look exactly like a healthy run.

(Two corrections from review. Round 2: the first version suppressed every failure. Round 4: after round 3 switched endpoints, the surviving 404 suppression was wrong for the new one — releases/latest 404s benignly, the list endpoint does not.)

Why the file is deleted, not emptied

The issue offered "an empty updates: list or deleted outright". Empty isn't actually available: Dependabot's schema requires at least one entry and flags a zero-entry list as an invalid config. Deleted it is.

The one still-load-bearing note in that file's header — that Dependabot security updates are configured in repo settings rather than here, and kept working while dependabot.yml was missing entirely (#1833, #1840) — moved into the workflow header rather than being lost with the file.

No AGENTS.md change is needed: the carve-out was never written down there, so there is nothing to correct. (Option C is the one that would have required an edit.)

3. The two stale comments from #2232 are corrected

.github/workflows/dependency-refresh.yml:3 and scripts/dependency-refresh.mjs:4 both described the overlap as pending and pointed here. Flipped to past tense, and the "the two flows deliberately overlap" paragraph is dropped — after this there is no Dependabot flow left to overlap with.

Also renamed the workflow's npm-outdated job to dependency-sweep, since the sweep is no longer npm-only. Not a required check, so no branch-protection impact.

Verification

  • scripts/dependency-refresh.test.mjs goes 5 → 13 tests, covering the four new pure functions and the new body section.
  • Dry-run against this repo's real workflows: all 9 actions resolve, none are stale, and buildIssueBody returns null — the correct no-op.

⚠️ Read that empty section precisely. All 9 refs here are moving major tags (@v4/@v6/@v7), so an empty GitHub Actions section means "no major bump is available", not "everything is current to the patch". With the repo pinned this way, a v7v8 jump is the only thing that section will ever surface. The exactly-pinned case (v7.0.0 behind v7.0.1) is coverage for a pinning style this repo doesn't currently use — deliberate, so the check stays correct if any workflow later pins hard or moves to SHA pins.

  • npm run local:gate green.

⚠️ What this PR cannot verify

Dependabot reads .github/dependabot.yml from the default branch (main), so nothing here takes effect when this merges to v2/main — only at the next milestone merge. That the npm PRs actually stopped, and that security-update PRs kept working, has to be re-checked after that merge. It's the one "Done when" item on #2235 that no PR review can close.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax

@cliffhall cliffhall added the v2 Issues and PRs for v2 label Sep 4, 2026
@cliffhall
cliffhall requested a balanced review from Copilot September 4, 2026 00:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The reviewed changes have no unresolved approval-blocking issues.

Pull request overview

Replaces Dependabot version-update PRs with the monthly dependency sweep, extending it to monitor GitHub Actions.

Changes:

  • Removes Dependabot configuration.
  • Adds GitHub Actions release checks.
  • Expands tests and updates workflow documentation and naming.
File summaries
File Description
scripts/dependency-refresh.test.mjs Tests action parsing, comparison, deduplication, and issue rendering.
scripts/dependency-refresh.mjs Detects and reports stale workflow action references.
.github/workflows/dependency-refresh.yml Runs and documents the combined dependency sweep.
.github/dependabot.yml Removes automated version-update configuration.
Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

GitHub API failures can silently disable action update detection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread scripts/dependency-refresh.mjs Outdated
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 2 on #2239.

`latestReleaseTag` returned null on any non-zero `gh api` exit, so a rate
limit, an expired token or a transient 5xx read as "this action has no
release" and therefore as "not stale". That is the silent-success shape:
the sweep exits green having checked nothing.

It is worse here than the equivalent npm case, because every action in
this repo already sits on its latest major — so the actions section is
empty on a healthy run too, and an API-wide failure produces byte
identical output to a clean sweep. Nothing would have surfaced it.

Now only a 404 is suppressed, which is the legitimate "this action has
never cut a GitHub release" answer; every other failure throws with the
repo and stderr, and a spawn error throws too. The 404/not-404 split is
extracted as a pure `isMissingRelease(stderr)` so it is covered by the
suite under this file's convention of keeping `main()` and its spawns
out of it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 2 — 14223927

Mirroring at PR level, since the inline thread goes outdated once the fix is pushed.

One finding, accepted and fixed. latestReleaseTag returned null on any non-zero gh api exit, so a rate limit, an expired token or a transient 5xx read as "this action has no release" — and therefore as "not stale".

It now suppresses only a 404, the legitimate "this action has never cut a GitHub release" answer. Every other failure throws with the repo name and stderr; a spawn error (gh missing) throws too. The 404/not-404 split is extracted as a pure isMissingRelease(stderr) so it is covered by the suite under this file's convention of keeping main() and its spawnSync calls out of it. New test asserts 404 → suppressed, and 403 rate-limit / 401 bad-credentials / 502 / empty-stderr → not suppressed. 13 → 14 tests, npm run local:gate green.

Why this one mattered more than it reads

The failure mode is invisible by construction. Every action in this repo already sits on its latest major, so the actions section is empty on a healthy run too — an API-wide failure and a completely clean sweep produce byte-identical output. There is no signal that would have told anyone the check had stopped working.

Worth recording that the same defect shape was found independently on the parent PR #2232 at almost the same moment: runOutdated there accepted any exit status, so a failed npm outdated (exit 2 on a registry error) parsed to an empty package list and reported a clean no-op across all five installs. Two reviewers, two files, one bug class. runOutdated and latestReleaseTag will land with matching semantics.

⚠️ Note on the review rounds

Round 1 on this identical diff returned 🟢 Approval recommended, 0 comments. Round 2, no changes in between, returned 🟡 Changes recommended with the finding above. A single clean Copilot round on this PR was not evidence of anything.

Still to come, before the next review round

This PR is stacked on #2232, whose branch is about to move past e32a1be with its own review fixes. Two of those land on this same file and I will rebase and integrate before requesting round 3:

  • main() there now takes an injected spawn (main(repo, spawn = spawnSync)); latestReleaseTag calls spawnSync directly today and would drop out of coverage, so it needs threading through the same seam, with 404 and 403 branches added to the fakeSpawn helper.
  • A new buildClearedBody() rewrites a stale tracking issue in place once everything is current. "Current" has to mean npm clean and no stale actions, or an open issue keeps showing a stale actions table after an action bump lands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The release lookup can miss available major action upgrades and must be corrected before approval.

Review details

Suppressed comments (4)

Previously missed (1) — in code that hasn't changed since the last review.

scripts/dependency-refresh.mjs:244

  • releases/latest is GitHub's designated most-recent release, not the highest numeric tag. If an action publishes or marks a maintenance release for an older major as latest (for example, v6.9.1 after v8.0.0), this comparison sees only v6 and incorrectly treats a workflow on v7 as current, so the monthly replacement can miss an available major upgrade. Query the release list/tags and select the greatest parseable version before calling isActionStale.

.github/workflows/dependency-refresh.yml:1

  • This header says Dependabot is replaced entirely, but lines 12–15 state that Dependabot security updates remain enabled and still open PRs. Describe this as replacing Dependabot version-update PRs to keep the workflow's own operational documentation consistent.
# Monthly dependency sweep (#2229), replacing Dependabot on this repo entirely.

scripts/dependency-refresh.mjs:187

  • The generated issue says this sweep replaces Dependabot wholesale, but this PR explicitly leaves Dependabot security updates enabled in repository settings; the workflow header also says those PRs remain active. Qualify this as replacing Dependabot version-update PRs so the tracking issue does not misstate the operational setup.
    "Routine dependency refresh — `npm outdated` plus a workflow `uses:` check, run against `v2/main` on a monthly schedule. This sweep replaces Dependabot on this repo (#2229, #2235).",

scripts/dependency-refresh.mjs:2

  • “Replacing Dependabot entirely” is inaccurate while Dependabot security updates remain enabled separately, as documented in the workflow. Qualify this as replacing Dependabot version-update PRs so the script header matches the actual scope.
// Monthly dependency sweep (#2229), replacing Dependabot on this repo entirely.
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@cliffhall
cliffhall force-pushed the v2/chore/2235-remove-dependabot-config branch from 1422392 to 7e13626 Compare September 4, 2026 02:12
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 3 — 7e13626d

Round 3 reported 0 new comments but 4 suppressed, and all four were acted on. Recording them here because a suppressed comment has no inline thread to reply to, and one of them was the most serious finding on this PR.

Also note this commit is a rebase onto bb9af94d (#2232's own review fixes), so the branch was squashed and force-pushed. The conflict that appeared in the meantime is resolved.


1. releases/latest is not the highest version — accepted, and this was a real bug

releases/latest is GitHub's designated most-recent release, not the highest numeric tag.

Correct, and it would have shipped a check that was green while blind. An action that cuts a maintenance release for an older major — a v6.9.1 published after v8.0.0 — reports v6.9.1 at that endpoint. A workflow pinned to v7 would then compare against v6, read as current, and silently miss an entire major upgrade, which is the one thing this check exists to catch.

Now reads the release list (repos/<a>/releases?per_page=100, drafts and prereleases filtered out) and selects the greatest parseable version through a new pure highestVersionTag. One page rather than full pagination: the list is newest-first, so the greatest version is within it for any real action — stated in the doc comment rather than left as an assumption.

Two things worth noting about how this is tested, because behaviour alone cannot catch a regression here:

  • Both endpoints return a plausible tag, so a test asserting only on the result would pass against either. There is now a test asserting the call shape — that the lookup does not request releases/latest and does request /releases?.
  • The fake in dependency-refresh.test.mjs matches gh api calls on args[0] === "api", which the milestone lookup already claimed. Without discriminating on the path, my release lookup would have silently received the milestone string "v2.6.0" and every actions test would have passed while asserting nothing — the same silent-wrong-answer shape, one level down in the harness. The releases branch is now matched first, with a comment saying why the order is load-bearing.

2–4. "Replacing Dependabot entirely" is inaccurate — accepted

…while Dependabot security updates remain enabled separately.

Right, and worth more than a wording nit: security updates are a separate mechanism configured in repo settings, not in the deleted file, and this PR deliberately leaves them on. Claiming otherwise in the generated tracking issue (dependency-refresh.mjs:187) is the worst of the three, because that text lands in an issue a maintainer reads months later with no context. Fixed in all three places — script header, workflow header, and issue body — each now saying version-update PRs and naming security updates as still active.


Also in this commit, from the rebase

buildClearedBody (introduced by #2232) said "Every install is up to date — nothing is outdated at the root or in any client." Once actions are in scope that asserts a clean bill of health the sweep never checked — the same silent-all-clear shape as finding #1. It now speaks for both halves, pinned by a test.

main() threads its injected spawn into the release lookup so that path sits inside #2232's coverage seam rather than dropping out of it.

28 tests (12 from #2232 + 16 here). npm run local:gate is running on the merged tree; I pushed ahead of it to clear the merge conflict rather than leave the PR unreviewable, and will follow up here if it surfaces anything.

@cliffhall
cliffhall force-pushed the v2/chore/2235-remove-dependabot-config branch from 7e13626 to fb83ee7 Compare September 4, 2026 02:21
@cliffhall
cliffhall force-pushed the v2/chore/2235-remove-dependabot-config branch from fb83ee7 to 2acbc16 Compare September 4, 2026 02:23
@cliffhall
cliffhall requested a balanced review from Copilot September 4, 2026 03:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The releases API error handling and corresponding tests must be corrected before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

scripts/dependency-refresh.mjs:224

  • This all-clear overstates what the sweep established. SHA/branch refs return null from parseVersionRef, and actions with no parseable release tag are also dropped, so those refs were not ranked against a release at all. Qualify the message to avoid telling maintainers that every workflow ref was verified.
    `Everything this sweep watches is current as of ${isoDate} — no npm package is outdated at the root or in any client, and no workflow \`uses:\` ref is behind its action's highest release.`,

scripts/dependency-refresh.test.mjs:449

  • This integration test locks in silent success for an inaccessible or nonexistent action repository. After making all non-zero release-list responses fatal, this case should assert that main() throws rather than logging a no-op.
test("main treats a 404 release lookup as 'this action cuts no releases'", () => {
  const spawn = fakeSpawn({
    releasesStatus: 1,
    releasesStderr: "gh: Not Found (HTTP 404)",
  });
  const log = captureLog(() => main("o/r", spawn));
  assert.match(log.join("\n"), /no-op/);
});
  • Files reviewed: 4/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

Comment thread scripts/dependency-refresh.mjs Outdated
Comment thread scripts/dependency-refresh.test.mjs Outdated
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 4 on #2239. Both findings are regressions
I introduced in round 3 and both are correct.

Round 3 moved the lookup from `releases/latest` to the release LIST, to
rank by version rather than by GitHub's designated latest — but kept the
404 suppression that only made sense for the old endpoint. The list
endpoint answers "this action cuts no releases" with a successful empty
array, so a 404 there does not mean that at all: it means the repository
is missing or inaccessible, i.e. a `uses:` ref the sweep cannot check.
Converting it to null silently dropped a broken or renamed action from
the flow that replaced Dependabot — the same silent-success shape this
PR has been closing everywhere else, reintroduced one layer down.

Every non-zero status is now fatal and `isMissingRelease` is gone
entirely; the benign no-releases case is the successful empty array,
covered by its own test.

`buildClearedBody` also overstated its all-clear. Refs pinned to a commit
SHA or a branch return null from `parseVersionRef` and are never ranked,
so "no workflow `uses:` ref is behind" asserted a check that had not
happened for them. It now says version-pinned refs and names the
exclusion explicitly.

Both guards are mutation-checked: restoring the 404 suppression, and
dropping the SHA/branch qualification, each fail exactly one test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 4 — cb5a687c

Two findings, both accepted, and both are regressions I introduced in round 3 rather than pre-existing issues. Recording that plainly because it changes how the review history reads: round 3's fix created round 4's bug.


1. A failed release-list response is now fatal, 404 included

GET /releases returns an empty successful list for a repository with no releases, while 404 means the repository cannot be accessed.

Correct. Round 3 moved the lookup from releases/latest to the release list, to rank by version rather than by GitHub's designated latest — but I kept the 404 suppression that only made sense for the old endpoint. releases/latest 404s when an action has never cut a release, which is benign and common. The list endpoint answers that same state with a successful empty array. So after the switch, a 404 no longer meant "no releases"; it meant the repository is missing or inaccessible — a uses: ref this sweep cannot check at all — and I was converting it to null.

That is precisely the silent-success shape this PR has been closing everywhere else (round 2's latestReleaseTag, and runOutdated on #2232), reintroduced one layer down by my own fix. A renamed or deleted action would have been dropped from the sweep that replaced Dependabot, with no signal.

isMissingRelease is deleted outright rather than narrowed — with the empty array covering the benign case, there is no failure mode left that should be suppressed. Every non-zero status throws with the repo, the exit code and stderr.

2. buildClearedBody claimed more than the sweep checked

SHA/branch refs return null from parseVersionRef … those refs were not ranked against a release at all.

Also correct, and the same class of defect as #1 in prose form. Refs pinned to a commit SHA or a branch cannot be ranked against a release tag, so no workflow uses: ref is behind its action's highest release asserted a check that never ran for them. It now reads version-pinned and names the exclusion explicitly, so a maintainer reading a cleared issue months later is not told their SHA-pinned actions were verified.


Both guards are mutation-checked

A passing suite does not prove a new test detects anything, so each guard was verified by reintroducing the bug it exists to catch:

Mutation Result
Restore the 404 suppression in latestReleaseTag 1 failed, 28 passed
Drop the SHA/branch qualification from buildClearedBody 1 failed, 28 passed
Baseline (neither) 29 passed

Exactly one test dies per mutation — so each detects its own defect, and neither is redundant with the other.

npm run local:gate green: 7389/7389, exit 0.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

The release-list error-handling documentation must match the implemented behavior.

Review details

Suppressed comments (1)

scripts/dependency-refresh.mjs:308

  • This contract is stale: the implementation and new 404 test now make every non-zero release-list response fatal, including 404, while this JSDoc (and the PR description) still says 404 is suppressed. Update both descriptions to reserve null for a successful empty release list.
 * @throws when the lookup fails for any reason other than a 404
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@cliffhall

Copy link
Copy Markdown
Member Author

Relaying a Copilot finding from #2243, which stacks on this branch and so carries its diff.

On scripts/dependency-refresh.mjs latestReleaseTag:

For the releases-list endpoint, an existing repository with no releases returns 200 []; a 404 means the action repository is missing or inaccessible. Returning null here makes a deleted/renamed action look current and lets the sweep exit green, so this response must be surfaced as a failure instead of suppressed.

It has a point worth weighing against the comment already on that function. Since the switch from releases/latest to releases?per_page=100, the "publishes no releases" case that motivated the 404 suppression now comes back as 200 [] and is handled by highestVersionTag([]) — so the 404 branch only fires for a repo that is gone or inaccessible, which is the state the check should report rather than treat as up to date.

Not fixing it in #2243, since that would put a change to this PR's code in an unrelated diff. Your call whether to take it here or file a follow-up.

cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 5 on #2239.

Round 4 made every non-zero release-list response fatal and deleted
`isMissingRelease`, but `latestReleaseTag`'s JSDoc still promised
`@throws when the lookup fails for any reason other than a 404` — the
exact behavior that change removed. The PR description carried the same
stale claim.

A contract that documents the opposite of the code is worse than none:
the next reader reasonably trusts it, and here it would tell them a
missing action repo is silently tolerated when it now fails the sweep.

The contract now reserves `null` for a successful response carrying no
usable release — an action that has never cut one, or whose tags are all
unparseable — and states that a failed lookup is never `null`.

Documentation only; no behavior change, and the 29 tests are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 5 — 3a9c3e3e

0 new comments, 1 suppressed. Accepted — and it caught drift I created in round 4.

The contract described the behavior round 4 removed

This contract is stale: the implementation and new 404 test now make every non-zero release-list response fatal, including 404, while this JSDoc (and the PR description) still says 404 is suppressed.

Exactly right. Round 4 deleted isMissingRelease and made every non-zero status throw, but latestReleaseTag still carried @throws when the lookup fails for any reason other than a 404 — a promise about the precise behavior that commit removed. The PR description had the same stale sentence, and the review was right to name both.

Worth stating why this is not a nit: a contract that documents the opposite of the code is worse than no contract at all, because the next reader has no reason to distrust it. This one would have told them a missing or renamed action repo is silently tolerated, when it now fails the sweep — the exact misconception rounds 2 and 4 existed to remove. Documentation drift here reintroduces the bug in the reader's head.

Rewritten to state both halves rather than just deleting the stale clause:

 * @returns {string | null} `null` ONLY for a successful response carrying no
 *   usable release  an action that has never cut one, or whose tags are all
 *   unparseable. A failed lookup is never `null`; see below.
 * @throws on any non-zero status, 404 included

The PR description now carries the same correction, with both rounds noted so the reasoning survives: round 2 suppressed every failure; round 3 switched endpoints, which silently invalidated the 404 case that round 4 then fixed.

Documentation only — no behavior change, and the 29 tests are untouched. npm run local:gate green: 7389/7389, exit 0.


Where the rounds have landed

Round Found
1 🟢 nothing — on a diff rounds 2 and 3 then found real bugs in
2 Any gh api failure read as "not stale"
3 releases/latest ranks by designation, not version — missed major upgrades
4 404 on the list endpoint dropped missing actions; cleared-body claimed unranked refs were checked
5 Stale @throws contract from round 4

Every executable finding has been the same class: a failure that reads as "nothing to do." That is unusually hard to catch here because a healthy sweep also produces an empty actions section — broken and working states emit byte-identical output. It is also why round 3's fix produced round 4's bug: changing endpoints silently changed what a 404 meant.

Requesting another round rather than treating one quiet review as final, given round 1.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The release lookup must paginate to reliably identify the highest released action version.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread scripts/dependency-refresh.mjs
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 7 on #2239.

The lookup read one page of 100 and the doc comment justified it with an
assumption rather than a guarantee: "the greatest version is within it
for any real action". The list is ordered by release DATE, not version,
so an action that keeps cutting maintenance releases on lower majors
pushes the genuine maximum off page 1. Ranking what remained would
report a lower major as highest — the exact false-current result that
dropping `releases/latest` in round 3 was meant to prevent, arrived at
by a different route.

Now passes `--paginate`. Cost is unchanged for every action this repo
uses (57 and 64 releases are the largest, so still one request each);
it only costs more where correctness actually required it.

The command-shape test now requires `--paginate` as well as the list
endpoint. It has to: pagination is invisible in the result — one page
and every page return an identical-looking tag list until the day they
do not — so nothing about the returned value can detect its absence.
Verified by mutation: removing `--paginate` fails exactly one test.

Adding the flag also moved the URL out of args[1], which the test's own
filter had hardcoded — it caught that itself. Both the filter and
fakeSpawn's matcher are now argument-position agnostic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Response to Copilot review round 7 — bb2f4388

Accepted. Round 6 was 🟢 with zero comments on the same code, so this is the second time on this PR that a clean round has not meant a clean diff — noting that because it is the reason I kept requesting reviews rather than stopping at the approval.

The single-page read only looked correct

If an older major remains the maximum but falls beyond page 1 after enough newer maintenance releases on lower majors, this recreates the exact false-current result that replacing releases/latest was meant to prevent.

Right, and my own doc comment gave it away: "the greatest version is within it for any real action" is an assumption, not a guarantee, and I wrote it as if it were the latter. The list is ordered by release date, not by version. An action that keeps shipping maintenance releases on lower majors pushes the genuine maximum off page 1, and ranking the remainder reports a lower major as highest — the same false-current failure round 3 fixed, arrived at by a different route.

Now passes --paginate. The cost objection does not really exist: the largest surfaces here are 57 (actions/checkout) and 64 (docker/build-push-action) releases, so every action this repo uses is still a single request. Pagination only costs more in exactly the case where correctness required it.

Why this needed a command-shape assertion

Pagination is invisible in the result. One page and every page return identical-looking tag lists, right up until the day they do not — so no assertion on the returned value can detect a missing --paginate. The existing command-shape test now requires the flag alongside the list endpoint.

Verified by mutation rather than assumed:

Mutation Result
Remove --paginate from the lookup 1 failed, 28 passed
Baseline 29 passed

The fix broke my own test, and the test caught it

Adding the flag shifted the URL out of args[1], which the test's filter had hardcoded — it failed with "expected at least one release lookup" rather than passing vacuously. Both that filter and fakeSpawn's matcher are now argument-position agnostic, which also closes the trap where a mis-positioned match falls through to the milestone branch and silently returns "v2.6.0" as a release tag.

npm run local:gate green: 7389/7389, exit 0.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The reviewed changes have comprehensive tests and no unresolved issues.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

All reviewed changes are covered with no unresolved approval-blocking issues.

Review details
  • Files reviewed: 4/4 changed files
  • Comments generated: 0 new
  • Review effort level: Balanced

@cliffhall

Copy link
Copy Markdown
Member Author

Review loop complete — rounds 8 and 9 both clean

Two consecutive rounds of 🟢 Approval recommended, 0 new comments, no suppressed-comments block, and the inline comment count unchanged at 5. Nothing outstanding.

Stopping at two rather than one is deliberate, and this PR is the argument for it: a single clean round was misleading twice.

Round Verdict What it actually found
1 🟢 0 comments nothing — on a diff rounds 2–4 then found four bugs in
2 🟡 1 any gh api failure read as "not stale"
3 🔵 0 new, 4 suppressed releases/latest ranks by designation, not version; 3× overstated "replaces Dependabot entirely"
4 🟡 2 404 on the list endpoint dropped missing actions; cleared-body claimed unranked refs were checked
5 🔵 0 new, 1 suppressed @throws contract left describing behavior round 4 removed
6 🟢 0 comments nothing — round 7 then found the pagination bug
7 🟡 1 single-page read could miss the highest version entirely
8 🟢 0 comments
9 🟢 0 comments

Two lessons worth leaving in the record:

Every executable finding was the same defect class: a failure that reads as "nothing to do." That is unusually hard to see here, because a healthy sweep also emits an empty actions section — broken and working states produce byte-identical output. There is no signal that would have told anyone the check had stopped working. runOutdated on #2232 was independently found to have the same shape.

Fixes in this area kept producing the next bug. Round 3 switched endpoints to fix ranking, which silently changed what a 404 meant (round 4) and left the @throws contract lying (round 5). Round 3's replacement then turned out to have its own ranking hole (round 7). Each fix was correct and each moved the failure somewhere new.

Verification

  • npm run local:gate green on the final commit: 7389/7389, exit 0
  • 29 script tests, run under both GITHUB_REPOSITORY set and unset — the env-dependent shape that took chore(deps): npm audit fix before release bump; monthly npm-outdated issue #2232's CI red, which local:gate structurally cannot catch
  • Every guard added in response to a finding is mutation-checked: reintroducing the bug fails exactly one test, proving detection and non-redundancy

⚠️ Still open after merge, and not closable at review time

Dependabot reads .github/dependabot.yml from the default branch (main), so nothing here takes effect when this merges to v2/main — only at the next milestone merge. That the npm PRs actually stopped, and that security-update PRs kept working, has to be re-checked after that merge. It is the one "Done when" item on #2235 that no PR review can close.

cliffhall added a commit that referenced this pull request Sep 4, 2026
Both findings were "previously missed" suppressed ones; the two against
scripts/dependency-refresh.mjs and its workflow are relayed to #2239.

- Serialize the sweep with a fixed concurrency group and
  cancel-in-progress: false. The marker check is a read-before-write and
  a workflow_dispatch can land on top of the scheduled run, so two runs
  could both see no open issue and both file one — the duplicate the
  whole idempotency design exists to prevent. The queued run must wait
  and re-read, never be cancelled.
- Pick the milestone in JS, not in jq. jq sorts null before every
  string, so sort_by(.due_on) | .[0] returns an UNDATED open milestone
  in preference to every dated one. An undated bucket has no due date
  and so cannot be the nearest; pickMilestone drops it, and files the
  issue unmilestoned if nothing dated is open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall

Copy link
Copy Markdown
Member Author

Two more Copilot findings relayed from #2243, which stacks on this branch and so carries its diff. Both are against this PR's code.

1. scripts/dependency-refresh.mjscurrentMilestone can pick an undated milestone. This one is a live bug, not cosmetic:

An undated open milestone sorts before every dated milestone because jq orders null before strings, so the monthly issue can be assigned to an undated bucket rather than the current release milestone.

Verified:

$ echo '[{"t":"dated","due_on":"2026-09-09T00:00:00Z"},{"t":"undated","due_on":null}]' | jq -c 'sort_by(.due_on) | .[0]'
{"t":"undated","due_on":null}

No impact today (both open milestones are dated), but a Backlog-style bucket would silently misfile the tracking issue. In #2243 I moved the selection out of jq into a pure pickMilestone(milestones) that drops undated buckets — an undated one has no due date and so cannot be "the nearest" — and returns null when nothing dated is open. Same shape would work here and is unit-testable, which the jq expression is not.

2. .github/workflows/dependency-refresh.yml — the sweep can race itself.

The issue marker makes sequential runs idempotent, but the lookup-and-create sequence is not atomic and scheduled/manual runs may overlap. Both executions can observe no open tracking issue and create duplicates.

Fixed in #2243 with:

concurrency:
  group: dependency-refresh
  cancel-in-progress: false

cancel-in-progress: false matters — the queued run has to wait and re-read what the first wrote; cancelling it drops a sweep.

Not fixing either here, to keep #2243's diff to its own code. If this PR merges first, the milestone one at least deserves a follow-up issue rather than being dropped.

cliffhall added a commit that referenced this pull request Sep 4, 2026
Both findings were "previously missed" suppressed ones; the two against
scripts/dependency-refresh.mjs and its workflow are relayed to #2239.

- Serialize the sweep with a fixed concurrency group and
  cancel-in-progress: false. The marker check is a read-before-write and
  a workflow_dispatch can land on top of the scheduled run, so two runs
  could both see no open issue and both file one — the duplicate the
  whole idempotency design exists to prevent. The queued run must wait
  and re-read, never be cancelled.
- Pick the milestone in JS, not in jq. jq sorts null before every
  string, so sort_by(.due_on) | .[0] returns an UNDATED open milestone
  in preference to every dated one. An undated bucket has no due date
  and so cannot be the nearest; pickMilestone drops it, and files the
  issue unmilestoned if nothing dated is open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 4 on #2239. Both findings are regressions
I introduced in round 3 and both are correct.

Round 3 moved the lookup from `releases/latest` to the release LIST, to
rank by version rather than by GitHub's designated latest — but kept the
404 suppression that only made sense for the old endpoint. The list
endpoint answers "this action cuts no releases" with a successful empty
array, so a 404 there does not mean that at all: it means the repository
is missing or inaccessible, i.e. a `uses:` ref the sweep cannot check.
Converting it to null silently dropped a broken or renamed action from
the flow that replaced Dependabot — the same silent-success shape this
PR has been closing everywhere else, reintroduced one layer down.

Every non-zero status is now fatal and `isMissingRelease` is gone
entirely; the benign no-releases case is the successful empty array,
covered by its own test.

`buildClearedBody` also overstated its all-clear. Refs pinned to a commit
SHA or a branch return null from `parseVersionRef` and are never ranked,
so "no workflow `uses:` ref is behind" asserted a check that had not
happened for them. It now says version-pinned refs and names the
exclusion explicitly.

Both guards are mutation-checked: restoring the 404 suppression, and
dropping the SHA/branch qualification, each fail exactly one test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 5 on #2239.

Round 4 made every non-zero release-list response fatal and deleted
`isMissingRelease`, but `latestReleaseTag`'s JSDoc still promised
`@throws when the lookup fails for any reason other than a 404` — the
exact behavior that change removed. The PR description carried the same
stale claim.

A contract that documents the opposite of the code is worse than none:
the next reader reasonably trusts it, and here it would tell them a
missing action repo is silently tolerated when it now fails the sweep.

The contract now reserves `null` for a successful response carrying no
usable release — an action that has never cut one, or whose tags are all
unparseable — and states that a failed lookup is never `null`.

Documentation only; no behavior change, and the 29 tests are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall
cliffhall force-pushed the v2/chore/2235-remove-dependabot-config branch from bb2f438 to 17a714c Compare September 4, 2026 12:53
cliffhall added a commit that referenced this pull request Sep 4, 2026
Addresses Copilot review round 7 on #2239.

The lookup read one page of 100 and the doc comment justified it with an
assumption rather than a guarantee: "the greatest version is within it
for any real action". The list is ordered by release DATE, not version,
so an action that keeps cutting maintenance releases on lower majors
pushes the genuine maximum off page 1. Ranking what remained would
report a lower major as highest — the exact false-current result that
dropping `releases/latest` in round 3 was meant to prevent, arrived at
by a different route.

Now passes `--paginate`. Cost is unchanged for every action this repo
uses (57 and 64 releases are the largest, so still one request each);
it only costs more where correctness actually required it.

The command-shape test now requires `--paginate` as well as the list
endpoint. It has to: pagination is invisible in the result — one page
and every page return an identical-looking tag list until the day they
do not — so nothing about the returned value can detect its absence.
Verified by mutation: removing `--paginate` fails exactly one test.

Adding the flag also moved the URL out of args[1], which the test's own
filter had hardcoded — it caught that itself. Both the filter and
fakeSpawn's matcher are now argument-position agnostic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
cliffhall added a commit that referenced this pull request Sep 4, 2026
Both findings were "previously missed" suppressed ones; the two against
scripts/dependency-refresh.mjs and its workflow are relayed to #2239.

- Serialize the sweep with a fixed concurrency group and
  cancel-in-progress: false. The marker check is a read-before-write and
  a workflow_dispatch can land on top of the scheduled run, so two runs
  could both see no open issue and both file one — the duplicate the
  whole idempotency design exists to prevent. The queued run must wait
  and re-read, never be cancelled.
- Pick the milestone in JS, not in jq. jq sorts null before every
  string, so sort_by(.due_on) | .[0] returns an UNDATED open milestone
  in preference to every dated one. An undated bucket has no due date
  and so cannot be the nearest; pickMilestone drops it, and files the
  issue unmilestoned if nothing dated is open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
cliffhall and others added 4 commits September 4, 2026 09:06
Closes #2235.

Switches the old dependency flow off now that #2232 landed the
replacement. Removes the five npm ecosystem entries (root plus each
client under clients/*) and, resolving the question #2229 left open, the
github-actions entry as well — so .github/dependabot.yml goes away
outright rather than being emptied, which its schema does not allow.

The github-actions entry had exactly the property #2229 exists to
remove: it opened a grouped monthly PR carrying no `Closes #N` and no
board card, the one standing exception to "every PR references an
issue". Deleting it unreplaced would have left 9 actions unwatched, and
`npm outdated` says nothing about actions, so the monthly sweep now also
checks every `uses:` ref under .github/workflows and renders the stale
ones as one more section of the same tracking issue.

Ranking comes from the release LIST, not `releases/latest`. That
endpoint returns the release GitHub designates most recent, not the
greatest version, so an action publishing a maintenance release for an
older major (a v6.9.1 cut after v8.0.0) would make a workflow pinned to
v7 compare against v6 and read as current — silently missing a whole
major upgrade, the one thing this check exists to catch.

Staleness is compared only to the precision the ref specifies. `v7` is a
moving major tag that GitHub repoints at every v7.x release, so `v7`
against a highest of `v7.0.1` is current and only `v8` makes it stale;
an exactly-pinned `v7.0.0` is behind `v7.0.1`; a SHA pin is deliberately
immovable and is never reported.

A release lookup suppresses only a 404 — the legitimate "this action has
never cut a release" answer — and throws on anything else. Treating a
rate limit or an expired token as "no release" is indistinguishable from
"not stale", and since every action here already sits on its latest
major, the resulting empty section is byte-identical to a healthy run.

`buildClearedBody` now speaks for both halves: once actions are in scope,
its npm-only wording would assert a clean bill of health the sweep never
checked.

Dependabot security updates are unaffected — they are configured in repo
settings, not in this file, and kept working while it was missing
entirely (#1833, #1840). That note moves into the workflow header rather
than dying with the file; #2233 is where they are turned off
deliberately. The script header, the workflow header and the generated
issue body all say version-update PRs rather than claiming Dependabot is
replaced wholesale.

Also renames the workflow's npm-outdated job to dependency-sweep now
that the sweep is no longer npm-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
Addresses Copilot review round 4 on #2239. Both findings are regressions
I introduced in round 3 and both are correct.

Round 3 moved the lookup from `releases/latest` to the release LIST, to
rank by version rather than by GitHub's designated latest — but kept the
404 suppression that only made sense for the old endpoint. The list
endpoint answers "this action cuts no releases" with a successful empty
array, so a 404 there does not mean that at all: it means the repository
is missing or inaccessible, i.e. a `uses:` ref the sweep cannot check.
Converting it to null silently dropped a broken or renamed action from
the flow that replaced Dependabot — the same silent-success shape this
PR has been closing everywhere else, reintroduced one layer down.

Every non-zero status is now fatal and `isMissingRelease` is gone
entirely; the benign no-releases case is the successful empty array,
covered by its own test.

`buildClearedBody` also overstated its all-clear. Refs pinned to a commit
SHA or a branch return null from `parseVersionRef` and are never ranked,
so "no workflow `uses:` ref is behind" asserted a check that had not
happened for them. It now says version-pinned refs and names the
exclusion explicitly.

Both guards are mutation-checked: restoring the 404 suppression, and
dropping the SHA/branch qualification, each fail exactly one test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
Addresses Copilot review round 5 on #2239.

Round 4 made every non-zero release-list response fatal and deleted
`isMissingRelease`, but `latestReleaseTag`'s JSDoc still promised
`@throws when the lookup fails for any reason other than a 404` — the
exact behavior that change removed. The PR description carried the same
stale claim.

A contract that documents the opposite of the code is worse than none:
the next reader reasonably trusts it, and here it would tell them a
missing action repo is silently tolerated when it now fails the sweep.

The contract now reserves `null` for a successful response carrying no
usable release — an action that has never cut one, or whose tags are all
unparseable — and states that a failed lookup is never `null`.

Documentation only; no behavior change, and the 29 tests are untouched.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
Addresses Copilot review round 7 on #2239.

The lookup read one page of 100 and the doc comment justified it with an
assumption rather than a guarantee: "the greatest version is within it
for any real action". The list is ordered by release DATE, not version,
so an action that keeps cutting maintenance releases on lower majors
pushes the genuine maximum off page 1. Ranking what remained would
report a lower major as highest — the exact false-current result that
dropping `releases/latest` in round 3 was meant to prevent, arrived at
by a different route.

Now passes `--paginate`. Cost is unchanged for every action this repo
uses (57 and 64 releases are the largest, so still one request each);
it only costs more where correctness actually required it.

The command-shape test now requires `--paginate` as well as the list
endpoint. It has to: pagination is invisible in the result — one page
and every page return an identical-looking tag list until the day they
do not — so nothing about the returned value can detect its absence.
Verified by mutation: removing `--paginate` fails exactly one test.

Adding the flag also moved the URL out of args[1], which the test's own
filter had hardcoded — it caught that itself. Both the filter and
fakeSpawn's matcher are now argument-position agnostic.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NGtzPg3UxMLszysQXXfqax
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall
cliffhall force-pushed the v2/chore/2235-remove-dependabot-config branch from 17a714c to 0cb12a1 Compare September 4, 2026 13:06
cliffhall added a commit that referenced this pull request Sep 4, 2026
Both findings were "previously missed" suppressed ones; the two against
scripts/dependency-refresh.mjs and its workflow are relayed to #2239.

- Serialize the sweep with a fixed concurrency group and
  cancel-in-progress: false. The marker check is a read-before-write and
  a workflow_dispatch can land on top of the scheduled run, so two runs
  could both see no open issue and both file one — the duplicate the
  whole idempotency design exists to prevent. The queued run must wait
  and re-read, never be cancelled.
- Pick the milestone in JS, not in jq. jq sorts null before every
  string, so sort_by(.due_on) | .[0] returns an UNDATED open milestone
  in preference to every dated one. An undated bucket has no due date
  and so cannot be the nearest; pickMilestone drops it, and files the
  issue unmilestoned if nothing dated is open.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
cliffhall added a commit that referenced this pull request Sep 4, 2026
- Reconcile on (package, manifest, GHSA) tuples, not GHSAs alone.
  Dependabot alerts are per manifest and this repo has five lockfiles,
  so the same advisory legitimately covers the root install and a
  client. Keyed on the GHSA alone, another manifest's still-filable
  alert could vouch for this one and clear an issue whose own alert is
  open with no bump available.
- Rename the cleared-date test: it asserts the NEW date is taken when
  the reason changes, which is the opposite of what its name said.
- AGENTS.md no longer implies both sweeps select a dated milestone.
  Only the security sweep filters undated buckets; the monthly one's
  selection is raised on #2239.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013H6Cew3KB3jVmQ5x4Dq1sq
Signed-off-by: cliffhall <cliff@futurescale.com>
@cliffhall
cliffhall merged commit 1c1e708 into v2/main Sep 4, 2026
4 checks passed
@cliffhall
cliffhall deleted the v2/chore/2235-remove-dependabot-config branch September 4, 2026 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Disable Dependabot npm version updates in dependabot.yml, and close the github-actions PR carve-out

2 participants