Skip to content

feat(ci): enforce artifact retention from one policy table - #10478

Merged
kyle-sexton merged 12 commits into
mainfrom
feat/artifact-retention-policy
Oct 28, 2026
Merged

kyle-sexton merged 12 commits into
mainfrom
feat/artifact-retention-policy

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 27, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner direction 2026-10-27 (enforced artifact retention; the verify reader pages every artifact)

Summary

Nothing checked artifact retention. An actions/upload-artifact step with no retention-days keeps its artifact for the repository default of 90 days, and every kept artifact lengthens the repository-wide listing that babysit_verify_evidence.py pages through on each read. This adds one checked-in policy table and a PR-time check that holds every upload to it.

Fix

  • Table: .github/artifact-retention.json, validated against .github/artifact-retention.schema.json. One row per artifact name family (40 rows), each with retention-days, readers (each starting with a repository path the check confirms exists) and the reason that length is needed. max (90) is the only way to write 90, and a max row must name a max-reader from its readers. The table sits beside the other repo-owned CI config (lane-funding.json, runner-policy.json) rather than in docs/conventions/: ADR 0064 covers plugin and skill customization, and under .github/ an edit to the table alone still triggers the gate through the workflows filter group.

  • Three cuts, every other row at today's value.

    • selection-audit-*, selection-red-replay-* (maintenance-audit-test-selection.yml) and eval-browser-* (eval-browser-dispatch.yml) drop from 30 to 7 days.
    • Each is read by manual inspection only. A repository grep finds no download or other consumer of these names after the run, which matches the storage measurement in .work/gh-monitor/storage-2026-10-27/REPORT.md.
  • perf(ci): shorten lane patch, dependency-verdict and findings artifact retention #10470's lane cuts are in the table: patch-*, dependency-verdict-* and findings-* keep 3 days. findings-* stays at 3, not 1, because the file job downloads it (maintenance-audit-lane-runs.yml), so a manual re-run of that job a day later must still find it.

  • Over-long rows are marked, not changed. skill-reply-* carries a pending-cut note (owner decision) and keeps today's value.

    pr-merge-dispatched-* and pr-merge-summary are marked as under owner review.

  • pr-verify follows perf(pr-verify): keep PASS verdicts 30 days and FAIL verdicts 90 #10477. write-verdict.mjs retentionDays keeps FAIL 90 days and PASS or PASS-NOTES 30, passed to the upload as ${{ steps.verdict.outputs.retention-days }}. The row records max (the longest) and names the script in a new computed-by field; the check accepts that one expression only for a row naming computed-by, and the script must exist. FAIL stays at 90 because babysit_verify_evidence.py sets TOO_OLD_DAYS = 80 and scripts/credential-sweep.sh defaults --since to 90 days.

  • Check: scripts/check-artifact-retention.sh (logic in check-artifact-retention.mjs, YAML parsed with the yaml package and the table validated with ajv, both from the runner-policy install). It reads every upload step in .github/workflows/*.yml and .github/actions/*/action.yml. It fails on any of these: a step with no retention-days, a non-literal value (other than a computed-by row's step output), a value that differs from the row, a name that matches no family, or a row that matches no step. Matching rule, stated in the script header:

    1. A static name matches its exact key.
    2. A dynamic name matches <literal text before ${{>* exactly.
    3. A name that starts with an expression matches the row whose uploads lists <file>#<job id>.
  • Wiring: a lint-repo step in pr-require-checks.yml, gated on run_workflows like app-key-references and fed to the aggregator. It is also registered in check-affected-gates.txt and in the check-script contract registry.

  • pr-verify: the three verdict uploads (pr-verify.yml jobs check-intent, check-intent-subscription and check-intent-fallback) keep main's computed retention-days. The carry-run-cache.mjs RETENTION_DAYS comment names retentionDays and the table row.

  • mutation-range: a 7-day row for main's post-merge-select-mutation-range.yml upload, read by post-merge-test-mutation.yml on workflow_run.

  • Docs: a new README section, "Artifact retention", next to the check-script contract. The check's failure message also names the table and the row fields.

Verification

  • scripts/check-artifact-retention.test.sh: 23 passed, 0 failed (adds a computed-by row passing a step-output retention, the same step failing without computed-by, and a computed-by naming no file). The fixtures cover a missing retention-days, an unknown family, a mismatch, a prefix-only match, a composite action upload, a computed name, max without or outside its readers, 90 written as a number, a missing reader file, a stale row, environment exits, and the real repository.
  • scripts/check-artifact-retention.sh --check on this tree: 50 upload steps match 40 families. After merging main: carry-run-cache.test.mjs 19/19, pr-verify/workflow.test.mjs with write-verdict.test.mjs 35/35.
  • scripts/workflow-self-paths.test.sh 40/40, scripts/check-affected-gates.test.sh 13/13 and scripts/check-docs-only-gate.test.sh 88/88. scripts/check-docs-only-gate.sh --check and scripts/aggregate-hygiene-results.sh --self-test pass.
  • scripts/check-script-contract.test.sh: 57 passed, including the new row's environment case. The 2 failures are check-html-assets.sh with no root npm ci on the local host; they are unrelated.
  • actionlint is clean on the five changed workflows. .github/actions/resolve-config/callers.test.mjs passes 190/190, and pr-verify/workflow.test.mjs with carry-run-cache.test.mjs pass 36/36.
  • scripts/check-affected.sh: 11 passed, 3 skipped. No changelog fragment is required, since no plugin changed.

Related

  • Includes perf(ci): shorten lane patch, dependency-verdict and findings artifact retention #10470 (lane patch, dependency-verdict and findings retention), so the table matches the workflows whichever merges first.

  • Pending retention cuts from the 2026-10-27 inventory, each shipping separately:

    • skill-reply-* to 3 days (owner decision).
    • A one-time delete of the legacy 90-day verdict-* artifacts (owner decision).
  • poteto's FAIL-marker redesign: known-name FAIL marker artifacts, so FAIL lookups become name= reads instead of a full listing. Their rows will be added here when that lands.

🤖 Generated with Claude Code

kyle-sexton and others added 6 commits October 27, 2026 10:09
… days

patch-*, dependency-verdict-* and findings-* are downloaded only inside
the run that uploads them. patch-* goes from 7 to 3 days so a manual
write-job re-run still works over a weekend; dependency-verdict-* and
findings-* go to 1 day. The verify reader pages every artifact, so
fewer live artifacts means fewer pages.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every actions/upload-artifact step in .github/workflows and
.github/actions must now set retention-days equal to its artifact
family's row in .github/artifact-retention.json. A row gives the days,
the readers that need the artifact and the reason for that length; `max`
(90) also names the reader that needs it. Rows the inventory found
over-long carry a pending-cut note and keep today's value.

Three families with no reader after their run drop from 30 to 7 days:
selection-audit-*, selection-red-replay-* and eval-browser-*.

scripts/check-artifact-retention.sh runs in lint-repo behind the
workflows filter group, with its contract test and fixtures. The three
pr-verify verdict uploads, which relied on the repository default, now
state retention-days: 90, the same value.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ch-findings' into feat/artifact-retention-policy
…ention cuts in the policy table

Stacks on perf/lane-artifact-retention-patch-findings so the table and the
workflows agree in either merge order.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T15:26:40.849169Z 4fa8e83 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

claude-security-review reviewed 443acb8d617d2ff2808fe237ceffddbdc2a457db: 0 finding(s), 0 as inline comments.

No security issues found in this pull request.

critical-findings: 0

I read the whole diff (14 files) and the PR context. I also read the pr-merge.yml alert step (lines 580-696), which is the only trusted-path change that alters behavior.

Trusted-path files

  • .github/workflows/pr-merge.yml: the only change is pr-merge-alerted-* retention going from 7 days to 1.
    • The sole reader is post-merge-dispatch, which asks babysit_postmerge.py --lane-alerted only for full runs created within LOOKBACK_HOURS: 6 (lines 593 and 610).
    • Each record is uploaded after its run was created, so 1 day outlasts that window.
    • The record only suppresses a duplicate demotion alert. It guards no merge hold and no verdict, so a missing record could at worst resend an alert.
    • The 90-day families that feed the post-merge check (pr-merge-dispatched-*, pr-merge-rerun-*) and pr-verify-* are unchanged.
  • .github/workflows/pr-require-checks.yml: adds an artifact-retention step to lint-repo and its aggregator entry. It follows the app-key-references step beside it and reads only the checkout. It has no secrets, no new permissions:, no new trigger, and no change to any existing hold step.
  • .github/scripts/pr-merge/carry-run-cache.mjs: comment-only change. RETENTION_DAYS is still 90.
  • The new retention table, schema and scripts/check-artifact-retention.{sh,mjs} read repository files and compare values. The only filesystem calls are existsSync on table-supplied paths, with no execution or output of their content. The yaml and ajv packages come from the existing runner-policy install.

Retention changes

  • Cuts: selection-audit-*, selection-red-replay-* and eval-browser-* go from 30 to 7 days.
  • Increase: findings-* goes from 1 to 3 days.
  • None of these artifacts feeds a verdict, a merge hold or a token, so none weakens a control.

Manifests: no plugin.json or marketplace.json change.

Instruction-surface deletions: none.

Cost: nothing here can start a real charge. The shorter retention reduces artifact storage.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

claude-review: 1 inline finding(s) on 4fa8e83dd3ba44301a9645a88e80ce5f84b4ef8c; the summary is in this lane's pull request comment.

claude-lane: claude-review

Comment thread .github/artifact-retention.json Resolved
@github-actions

github-actions Bot commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

claude-review reviewed 443acb8d617d2ff2808fe237ceffddbdc2a457db: 0 finding(s), 0 as inline comments.

No blocking findings. I read the full diff and the PR context, and checked the CI wiring and the pr-merge.yml alert path against the repository files. I did not run the check or its tests; the pass counts in the PR body are the author's claims and unverified by me.

What I checked

  • Check script. scripts/check-artifact-retention.mjs matches names by the three stated rules. Its exit codes follow the contract: 0 clean, 1 findings, 2 environment. In the schema-failure path, report() is called before the steps const is initialised, but it only reads steps on the clean branch, so that path is safe.
  • Wiring. The lint-repo step runs after the runner-policy npm ci and is gated on run_workflows like its neighbours. It is fed to the aggregator as artifact-retention=… and registered in check-affected-gates.txt and the contract registry. The workflows filter group (.github/**, scripts/**) covers the table, the schema and the new scripts, so an edit to any of them triggers the gate.
  • pr-merge-alerted-* 7 to 1 day. pr-merge.yml lists main's full runs created in the last LOOKBACK_HOURS=6 and asks --lane-alerted only about those. The record is uploaded after the run is created, so 1 day outlasts every read.
  • Other cuts. The 30 to 7 day cuts (selection-audit-*, selection-red-replay-*, eval-browser-*) and the findings-* 1 to 3 day raise match the table rows. The PR body says nothing reads these three families after the run, so the cuts lose no consumer; I did not re-check that claim.

Not counted as findings

  • The PR body says "Three cuts, every other row at today's value". The diff also cuts pr-merge-alerted-* from 7 to 1 day (the head commit message says so) and raises findings-* from 1 to 3. Both are correct against the table, but the description undercounts the behavioural changes.
  • Cost: none. Shorter retention only reduces artifact storage, and no billed resource or plan is touched.
  • Manifests and closing issues: the PR changes no plugin manifest and has no closing issue ("No related issue"), so there is nothing to report.

@melodic-automation-lanes melodic-automation-lanes Bot added the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The pr-unblock lane stopped on this pull request and needs a person: the lane could not resolve every conflict.

  • Reason: unresolved-conflict
  • Resume key: 4fa8e83dd3ba44301a9645a88e80ce5f84b4ef8c (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=10478 and activity=resolve-conflicts:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10478 -f activity=resolve-conflicts

kyle-sexton and others added 2 commits October 27, 2026 16:01
Main's #10477 sets pr-verify verdict retention per verdict (PASS 30,
FAIL 90) through a write-verdict.mjs step output, which collided with
this branch's literal retention-days: 90 (duplicate keys). Keep main's
computed value; the policy table gains a computed-by field so the
checker accepts that one expression for a row that names its script,
and the pr-verify-* row drops its pending-cut, now made. Add the
mutation-range row main's post-merge-select-mutation-range.yml needs.
Keep findings-* at 3 days so a manual re-run of the file job still
finds them (claude-review thread on artifact-retention.json).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The pr-unblock lane escalated this pull request as unresolved-conflict, and that no longer holds at 54db28a3349bb6603e659d21376e3927b7dd598c: GitHub reports no conflict with main. pr-merge removed needs-human, so the lanes run on it again. To keep a person on it, add needs-human back: a label a person adds is never removed.

@melodic-automation-lanes melodic-automation-lanes Bot added needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. and removed needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. labels Oct 27, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The pr-unblock lane stopped on this pull request and needs a person: a conflicted path is a workflow, submodule config or a settings file, which the lane does not resolve.

  • Reason: protected-path-conflict
  • Resume key: 54db28a3349bb6603e659d21376e3927b7dd598c (the head the lane stopped on)

To resume, deal with the reason above, remove the needs-human label (and do-not-merge, if set), then dispatch the pr-unblock workflow with pr-number=10478 and activity=resolve-conflicts:

gh workflow run pr-unblock.yml -R melodic-software/claude-code-plugins -f pr-number=10478 -f activity=resolve-conflicts

kyle-sexton and others added 2 commits October 27, 2026 18:47
Main's #10470 sets findings-* retention to 1 day; this branch chose 3
days after that change, so a manual re-run of the file job still finds
the findings over a weekend (claude-review thread on
artifact-retention.json). Keep 3 in the workflow and the table.

Main's #10488 adds the pr-merge-verify-listing upload (1 day, read by
the next run through listing-carry.mjs); add its row so the retention
checker passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

The pr-unblock lane escalated this pull request as protected-path-conflict, and that no longer holds at 60fa6e63b38c28fdd33680bd1d4e9f0f245b1665: GitHub reports no conflict with main. pr-merge removed needs-human, so the lanes run on it again. To keep a person on it, add needs-human back: a label a person adds is never removed.

@melodic-automation-lanes melodic-automation-lanes Bot removed the needs-human Human-in-the-loop required; autonomous sessions must not resolve items carrying this. label Oct 27, 2026
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Oct 28, 2026
@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Unattended merge: risk HIGH at head 60fa6e63b38c28fdd33680bd1d4e9f0f245b1665. Only LOW, or MED on class, size and reported review-finding reasons alone, merges without a human. A floor path holds it: a human merges it in person, never an agent or a lane.

HIGH high-path:.github/scripts/pr-merge/carry-run-cache.mjs: .github/scripts/pr-merge/carry-run-cache.mjs
HIGH high-path:.github/workflows/pr-require-checks.yml: .github/workflows/pr-require-checks.yml
MED evidence-unbound:claude-review: not bound: lane-source-changed
MED evidence-unbound:claude-security-review: not bound: lane-source-changed

kyle-sexton and others added 2 commits October 27, 2026 21:01
… 1 day

main added the pr-merge-alerted-<id> upload after this table, so
check-artifact-retention failed in the merge queue. Its only reader,
babysit_postmerge.py --lane-alerted, is asked only for full runs created
within pr-merge.yml's LOOKBACK_HOURS (6), so the 1-day minimum covers it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 28, 2026
Merged via the queue into main with commit 045a815 Oct 28, 2026
49 checks passed
@kyle-sexton
kyle-sexton deleted the feat/artifact-retention-policy branch October 28, 2026 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant