From 3629e36f43fdfb9a69e6ad024395e63818c83a07 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Fri, 4 Sep 2026 20:22:15 -0700 Subject: [PATCH 1/2] improvement(ci): collect idle node_modules sticky disks at 3 days MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit node_modules sticky disks are keyed on hashFiles('bun.lock') by design, so a lockfile bump orphans the old disk. Blacksmith already evicts after 7 days of inactivity, so this is not a leak — it is a retention window far too generous for a key that churns this fast. Steady state is (new disks/day x GB/disk x retention). Measured on the live account: 6.3 new disks/day at 13.7 GB each, which at 7 days predicts 606 GB against 601 GB actually observed, so the model holds. At 3 days it predicts 260 GB, and 24 of the 44 current disks (348 GB) are already past that. 3 days rather than 2 because the data says 2 buys almost nothing: only 14 GB more is idle at 2 days than at 3, while 3 clears a long weekend so a PR whose last CI run was Friday afternoon still hits a warm disk on Monday. Age-based, never PR-triggered. The key holds no PR identifier, so every open PR whose checkout has the same bun.lock mounts the same disk, and there are far more open PRs than distinct lockfile hashes — sharing is the common case. Deleting on PR close would destroy a disk many other open PRs are using. Guards, all exercised against real and crafted listings before shipping: a server-side --search plus a local regex re-proving the full key shape; an [a-z_]+ event segment rather than an enumerated push|pull_request, because the key interpolates github.event_name and a workflow_dispatch disk already exists that an enumerated list would have skipped forever; a refusal when every disk looks idle, which means a listing or clock fault rather than genuine idleness; and rejection of a non-numeric or zero retention. A turbo key injected into the listing is not selected. Runs on a GitHub-hosted runner so collection still works during a Blacksmith outage or a CI_PROVIDER break-glass switch, which is exactly when disks idle and still bill. The CLI is pinned by version and SHA256 checked against the vendor's published sidecar rather than piped from a remote installer, since the job holds an org-wide token, and auto-update is disabled so the pin holds. Deletes fail the job rather than continue-on-error. Requires a BLACKSMITH_CLI_TOKEN repository secret; run once with dry_run first. --- .github/workflows/stickydisk-gc.yml | 172 ++++++++++++++++++++++++++++ 1 file changed, 172 insertions(+) create mode 100644 .github/workflows/stickydisk-gc.yml diff --git a/.github/workflows/stickydisk-gc.yml b/.github/workflows/stickydisk-gc.yml new file mode 100644 index 00000000000..3ecd18b6302 --- /dev/null +++ b/.github/workflows/stickydisk-gc.yml @@ -0,0 +1,172 @@ +name: Sticky Disk GC + +# node_modules sticky disks are keyed on hashFiles('bun.lock') by design — see the +# "Mount node_modules" comment in test-build.yml. A sticky disk is a mutable volume +# and `bun install --frozen-lockfile` adds what the lockfile needs without pruning +# what it dropped, so branches on different lockfiles must not share one. That +# design is correct and is preserved here. +# +# Its cost is a fresh ~13.7 GB disk per lockfile hash per event class, created at +# ~6.3/day. Blacksmith evicts a sticky disk after 7 days of inactivity, so this is +# not a leak — it is a retention window far too generous for a key that churns +# this fast. Steady state is (new disks/day x GB/disk x retention days), which at +# 7 days predicts 606 GB against 601 GB actually observed. +# +# Age-based on purpose. The key contains NO PR identifier, so every open PR whose +# checkout has the same bun.lock mounts the SAME disk — there are far more open +# PRs than distinct lockfile hashes, so sharing is the common case, not an edge +# case. Deleting on PR close would destroy a disk many other open PRs are using. +# Never add a pull_request or pull_request_target trigger here. + +on: + schedule: + - cron: '17 9 * * *' + workflow_dispatch: + inputs: + retention_days: + description: Delete node_modules disks unused for more than this many days. + required: false + default: '3' + dry_run: + description: List what would be deleted without deleting it. + type: boolean + required: false + default: false + +# Nothing in this job reads the repository. +permissions: {} + +concurrency: + group: stickydisk-gc + cancel-in-progress: false + +jobs: + gc: + name: Reclaim idle node_modules sticky disks + # GitHub-hosted on purpose, not a Blacksmith runner: the CLI is a pure API + # client, and collection has to keep working during a Blacksmith outage or a + # CI_PROVIDER break-glass switch — exactly when disks sit idle and still bill. + runs-on: ubuntu-latest + timeout-minutes: 10 + env: + # Pinned binary + checksum rather than `curl https://get.blacksmith.sh | sh`: + # this job holds an org-wide token, so it must not execute unpinned remote + # shell. The vendor publishes a .sha256 sidecar next to each binary; check a + # new version against it and bump both values together. + BLACKSMITH_CLI_VERSION: v0.4.58 + BLACKSMITH_CLI_SHA256: 0b54a4398e9b35344d8fb32891703d8a393343f5001914d7482f93d068c76822 + # The CLI self-updates in the background on every invocation, which would + # silently defeat the pin above. + BLACKSMITH_DISABLE_AUTO_UPDATE: '1' + BLACKSMITH_ORG: simstudioai + TARGET_REPO: ${{ github.repository }} + RETENTION_DAYS: ${{ inputs.retention_days || '3' }} + DRY_RUN: ${{ inputs.dry_run || 'false' }} + + steps: + - name: Install Blacksmith CLI + run: | + set -euo pipefail + url="https://clireleases.blacksmith.sh/cli/${BLACKSMITH_CLI_VERSION}/linux/amd64/blacksmith" + curl -fsSL "$url" -o /usr/local/bin/blacksmith + echo "${BLACKSMITH_CLI_SHA256} /usr/local/bin/blacksmith" | sha256sum -c - + chmod +x /usr/local/bin/blacksmith + + - name: Authenticate + env: + BLACKSMITH_CLI_TOKEN: ${{ secrets.BLACKSMITH_CLI_TOKEN }} + run: | + set -euo pipefail + if [ -z "${BLACKSMITH_CLI_TOKEN:-}" ]; then + echo "::error::BLACKSMITH_CLI_TOKEN is not set. Mint one with 'blacksmith org-token create' and add it as a repository secret." + exit 1 + fi + printf '%s' "$BLACKSMITH_CLI_TOKEN" \ + | blacksmith auth login --api-token - --non-interactive --organization "$BLACKSMITH_ORG" + + - name: Delete node_modules disks idle beyond the retention window + run: | + set -euo pipefail + + case "$RETENTION_DAYS" in + ''|*[!0-9]*) + echo "::error::retention_days must be a whole number of days, got '${RETENTION_DAYS}'" + exit 1 + ;; + esac + if [ "$RETENTION_DAYS" -lt 1 ]; then + echo "::error::retention_days must be at least 1; 0 would delete disks a running job is using" + exit 1 + fi + + blacksmith stickydisk list \ + --repo "$TARGET_REPO" \ + --search '-node-modules-' \ + --per-page 100 \ + --format json > disks.json + + # Two independent guards, because the blast radius of a wrong key is a + # cache every CI job depends on: + # 1. --search narrows server-side to the node_modules family. + # 2. The regex re-proves each key's full shape locally. + # The event segment is [a-z_]+ rather than an enumerated push|pull_request + # because the key interpolates ${{ github.event_name }}: a + # workflow_dispatch disk already exists that an enumerated list would + # have skipped forever. Verified against the live account — this matches + # every node_modules disk and none of the bun/turbo/Docker disks, which + # are mounted every run, never go idle, and must survive. + # + # Grouped by key before the staleness test because `delete` without + # --arch removes every architecture variant, so a key may only go when + # its NEWEST variant is stale. + jq -r --arg repo "$TARGET_REPO" --argjson days "$RETENTION_DAYS" ' + (now - ($days * 86400)) as $cutoff + | .entries + | map(select(.type == "stickydisk")) + | map(select(.key | test("^" + ($repo | gsub("/"; "\\/")) + "-node-modules-[a-z_]+-[0-9a-f]{64}$"))) + | group_by(.key) + | map({ + key: .[0].key, + gb: (map(.size_bytes) | add / 1000000000 * 100 | round / 100), + last_used: (map(.last_used_at | sub("\\.[0-9]+Z$"; "Z") | fromdateiso8601) | max) + }) + | map(select(.last_used < $cutoff)) + | .[] | "\(.key)\t\(.gb)" + ' disks.json > stale.tsv + + total=$(jq '[.entries[] | select(.type == "stickydisk")] | length' disks.json) + count=$(wc -l < stale.tsv | tr -d ' ') + reclaimed=$(awk -F'\t' '{s+=$2} END {printf "%.1f", s+0}' stale.tsv) + { + echo "### Sticky disk GC" + echo "" + echo "Retention **${RETENTION_DAYS}d** · dry run **${DRY_RUN}** · **${count}** of ${total} node_modules disks idle (**${reclaimed} GB**)" + echo "" + } >> "$GITHUB_STEP_SUMMARY" + + # A run that would delete everything means the listing or the clock is + # wrong, not that every disk went idle at once. Refuse rather than wipe + # the caches every CI job depends on. + if [ "$count" -gt 0 ] && [ "$count" -eq "$total" ]; then + echo "::error::Refusing to delete all ${total} node_modules disks — that indicates a listing or clock fault, not genuine idleness." + exit 1 + fi + + failed=0 + while IFS=$'\t' read -r key gb; do + [ -n "$key" ] || continue + if [ "$DRY_RUN" = "true" ]; then + echo "- would delete \`${key}\` (${gb} GB)" >> "$GITHUB_STEP_SUMMARY" + continue + fi + if blacksmith stickydisk delete --repo "$TARGET_REPO" --key "$key" --yes; then + echo "- deleted \`${key}\` (${gb} GB)" >> "$GITHUB_STEP_SUMMARY" + else + echo "- FAILED \`${key}\`" >> "$GITHUB_STEP_SUMMARY" + failed=1 + fi + done < stale.tsv + + # Fail loudly rather than continue-on-error: a revoked token or a changed + # CLI JSON shape would otherwise silently revert us to 7-day billing. + exit "$failed" From ff61d54495bbab3706cdb49e11d72b56929f2bbb Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Sat, 5 Sep 2026 09:56:03 -0700 Subject: [PATCH 2/2] fix(ci): collect fork disks, and count like with like in the bulk guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings, both real. Fork PR disks were never collected. test-build.yml appends `-fork` after the event name, so a fork key reads `pull_request-fork-`, and the `[a-z_]+` event class cannot span that hyphen. `-fork` is now its own optional segment. The live account has no fork disks right now, which is why testing the filter against real data did not catch it. The bulk-delete guard could never fire. It compared grouped deletion keys against raw sticky-disk entries, so architecture variants — deliberately grouped into one key — and entries the regex had already rejected both inflated the denominator. The listing is now reduced to the deletable set once, and the staleness test and the guard both count that same set. Verified against the live listing and crafted ones: a stale fork key is selected; a key with two arch variants collapses to a single delete; a key whose newest variant is fresh is not selected; the guard fires when every eligible key is stale even with an arch variant present; and an injected turbo key is still never selected. --- .github/workflows/stickydisk-gc.yml | 41 ++++++++++++++++++----------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/.github/workflows/stickydisk-gc.yml b/.github/workflows/stickydisk-gc.yml index 3ecd18b6302..5ac5e54d800 100644 --- a/.github/workflows/stickydisk-gc.yml +++ b/.github/workflows/stickydisk-gc.yml @@ -105,36 +105,47 @@ jobs: --per-page 100 \ --format json > disks.json - # Two independent guards, because the blast radius of a wrong key is a + # Reduce the listing to the deletable set ONCE, so the staleness test and + # the bulk-delete guard below both count the same things. Deriving the + # guard's denominator from the raw entries instead would double-count + # architecture variants (which are deliberately grouped into one key) and + # would also count entries the regex rejected, so the guard could never + # fire. + # + # Two independent filters, because the blast radius of a wrong key is a # cache every CI job depends on: # 1. --search narrows server-side to the node_modules family. # 2. The regex re-proves each key's full shape locally. # The event segment is [a-z_]+ rather than an enumerated push|pull_request - # because the key interpolates ${{ github.event_name }}: a - # workflow_dispatch disk already exists that an enumerated list would - # have skipped forever. Verified against the live account — this matches - # every node_modules disk and none of the bun/turbo/Docker disks, which - # are mounted every run, never go idle, and must survive. + # because the key interpolates ${{ github.event_name }} and a + # workflow_dispatch disk already exists that an enumerated list would have + # skipped forever. `-fork` is a separate optional segment rather than part + # of that class: test-build.yml appends it after the event name, so a fork + # key reads `pull_request-fork` and a character class cannot span the + # hyphen. Without it, fork disks would never be collected. # - # Grouped by key before the staleness test because `delete` without - # --arch removes every architecture variant, so a key may only go when - # its NEWEST variant is stale. - jq -r --arg repo "$TARGET_REPO" --argjson days "$RETENTION_DAYS" ' - (now - ($days * 86400)) as $cutoff - | .entries + # Grouped by key because `delete` without --arch removes every + # architecture variant, so a key may only go when its NEWEST variant is + # stale. + jq --arg repo "$TARGET_REPO" ' + .entries | map(select(.type == "stickydisk")) - | map(select(.key | test("^" + ($repo | gsub("/"; "\\/")) + "-node-modules-[a-z_]+-[0-9a-f]{64}$"))) + | map(select(.key | test("^" + ($repo | gsub("/"; "\\/")) + "-node-modules-[a-z_]+(-fork)?-[0-9a-f]{64}$"))) | group_by(.key) | map({ key: .[0].key, gb: (map(.size_bytes) | add / 1000000000 * 100 | round / 100), last_used: (map(.last_used_at | sub("\\.[0-9]+Z$"; "Z") | fromdateiso8601) | max) }) + ' disks.json > eligible.json + + jq -r --argjson days "$RETENTION_DAYS" ' + (now - ($days * 86400)) as $cutoff | map(select(.last_used < $cutoff)) | .[] | "\(.key)\t\(.gb)" - ' disks.json > stale.tsv + ' eligible.json > stale.tsv - total=$(jq '[.entries[] | select(.type == "stickydisk")] | length' disks.json) + total=$(jq 'length' eligible.json) count=$(wc -l < stale.tsv | tr -d ' ') reclaimed=$(awk -F'\t' '{s+=$2} END {printf "%.1f", s+0}' stale.tsv) {