docs: correct issue/PR lifecycle table, fix reminder dedupe

[yuanchen8911]

Summary

Corrects the issue/PR lifecycle table in CONTRIBUTING.md. The day counts had drifted from the workflows that implement them, and the table's framing implied absolute days since opening when every threshold is really an inactivity window.

Motivation / Context

The table drifted from the automation. PR #480 ("align issue management with OSS policy") changed .github/workflows/stale.yaml from days-before-stale: 60 / days-before-close: 14 to 90 / 30, and did not touch CONTRIBUTING.md. The documented numbers have been wrong since then, so a contributor reading the guide gets thresholds the bot does not use.

The counts drifted. Verified against the workflows that run today:

Threshold	Documented	Actual	Source
Issue marked stale	60	90 days of inactivity	stale.yaml days-before-stale: 90
Stale issue closed	74	30 days after the stale mark	days-before-close: 30
Stale PR closed	44	30 days after the stale mark	days-before-close: 30
Thread locked	"90+"	90 days of inactivity while closed	lock-threads.yaml

The framing was also wrong, and this half predates the drift. actions/stale measures from updated_at, so each threshold is an inactivity window, not an age. Any comment or update restarts the counter — including the day-14 reminder, which is itself a comment, so an untouched PR is marked stale roughly 44 days after opening rather than 30. lock-threads.yaml likewise keys on updated_at, so a thread locks after 90 days of inactivity while closed, not 90 days after being closed.

The table now states each threshold relative to what it measures, and says once that activity restarts the applicable counter. The day-14 PR reminder is real — inactive-pr-reminder.yaml fires in a 14–29 day window.

The exemption note also listed only lifecycle/frozen and do-not-merge. stale.yaml exempts priority/critical and good first issue from issue auto-close as well, so those are now named.

Fixes: N/A
Related: #1920, #1911

Scope note (updated)

This PR was doc-only until review. @ArangoGutierrez pointed out that the
"once per pull request" claim is stronger than the workflow guarantees:
listComments in .github/workflows/inactive-pr-reminder.yaml was unpaginated,
so it saw only the 30 oldest comments and a busy PR could receive duplicate
reminders.

Rather than document that defect as intended behavior, the fix is folded in —
the call now uses github.paginate(...) with per_page: 100, and the prose
keeps the exact claim. Note per_page: 100 alone would not have been sufficient:
it raises the ceiling from 30 to 100 without removing the failure, and this repo
has PRs in that range. Pagination makes the sentence true without a bound.

Consequence: the PR now touches .github/workflows/ as well as
CONTRIBUTING.md, and carries area/ci in addition to area/docs.

Type of Change

• Bug fix (non-breaking change that fixes an issue)
• New feature (non-breaking change that adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Documentation update
• Build/CI/tooling
• Refactoring (no functional changes)
• Build/CI/tooling

Component(s) Affected

• CLI (cmd/aicr, pkg/cli)
• API server (cmd/aicrd, pkg/server)
• Recipe engine / data (pkg/recipe)
• Bundlers (pkg/bundler, pkg/component/*)
• Collectors / snapshotter (pkg/collector, pkg/snapshotter)
• Validator (pkg/validator)
• Core libraries (pkg/errors, pkg/k8s)
• Docs/examples (docs/, examples/)
• Other: ____________

Implementation Notes

Documentation only — no workflow behavior changes. The docs were moved to match the automation rather than the reverse, because #480 changed the timings deliberately as an OSS-policy alignment; treating the workflow as the source of truth preserves that decision.

No attempt is made to state an absolute "day N" for each event, since the real answer depends on activity. The relative form is both correct and shorter.

This covers the first of the three items in #1920. The remaining two touch .agents/skills/aicr-triage/SKILL.md, which does not exist on main yet — it is added by #1911. They are deliberately left for a follow-up once that merges:

• whether lifecycle/frozen / priority/critical should hard-exclude an issue from the skill's Close bucket
• whether the skill's 30-day "Stalled" table should be renamed or realigned, so one word does not mean two things

Testing

grep -nE "days-before|exempt" .github/workflows/stale.yaml
grep -n "daysInactive" .github/workflows/inactive-pr-reminder.yaml
grep -nE "inactiveDays|updated_at" .github/workflows/lock-threads.yaml
git log --oneline -S "days-before-stale: 60" -- .github/workflows/stale.yaml

Each corrected threshold was read out of the workflow that implements it. The last command returns 0e12dc200 (#480) followed by the original introduction in #13, confirming #480 as the commit that moved the timings without updating the docs.

Note the value is required in the -S argument: git log -S "days-before-stale" alone returns only #13, because the key's occurrence count never changed when its value did. An earlier revision of this PR cited the keyless form and claimed it found #480 — it does not.

make qualify was run on this branch with isolated GOCACHE / GOLANGCI_LINT_CACHE, per the repository rule that the gate applies to doc-only PRs:

• Pass — unit tests with -race, coverage gate, shell tests, e2e, go vet, lint-go
• Fail — scan (grype), on vendored dependency advisories in golang.org/x/crypto, golang.org/x/net, and k8s.io/kubernetes

The scan failure is pre-existing and unrelated to this change: the identical failure reproduces on a clean origin/main worktree, this PR contains no Go files or dependency changes, and CI's own grype check passes on this head. Recorded here rather than papered over.

Risk Assessment

• Low — Isolated change, well-tested, easy to revert
• Medium — Touches multiple components or has broader impact
• High — Breaking change, affects critical paths, or complex rollout

Rollout notes: N/A — one Markdown table.

Checklist

• Tests pass locally (make test with -race)
• Linter passes (make lint) — lint-go clean with isolated caches
• I did not skip/disable tests to make CI green
• I added/updated tests for new functionality
• I updated docs if user-facing behavior changed
• Changes follow existing patterns in the codebase
• Commits are cryptographically signed (git commit -S) — GPG signing info

[yuanchen8911]

Addresses the first item of #1920. The other two touch .agents/skills/aicr-triage/SKILL.md, which #1911 adds and which does not exist on main yet, so they follow once that merges.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Updates CONTRIBUTING.md with inactivity-based stale, lock, and auto-closure timing for issues and pull requests, and clarifies lifecycle exemptions.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related issues

• Issue 1920 — Addresses the documented issue and pull request lifecycle timing discrepancy.

Suggested reviewers: arangogutierrez, mchmarny

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title is concise and accurately reflects the doc update plus the reminder dedupe fix.
Description check	✅ Passed	The description is directly about the same CONTRIBUTING.md lifecycle-table changes and workflow fix.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CONTRIBUTING.md`:
- Around line 222-225: Update the stale-process exemptions section in
CONTRIBUTING.md to include the priority/critical label for issues, alongside the
existing lifecycle/frozen and good first issue exemptions. Clarify that
priority/critical issues are exempt from being marked lifecycle/stale.
- Around line 200-225: Update DEVELOPMENT.md to document the same lifecycle
thresholds and behavior described in CONTRIBUTING.md, including inactivity-based
timers, the one-time day-14 PR reminder, daily schedule lag, and exemption
labels. Keep the workflow documentation synchronized without changing the
implementation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: 31a1bbd1-dbce-43d2-88a3-1efd039ec5b8

📥 Commits

Reviewing files that changed from the base of the PR and between 49d654a and d9a9a48.

📒 Files selected for processing (1)

• CONTRIBUTING.md

[ArangoGutierrez]

Checked each row against the workflows and they line up — days-before-pr-stale: 30, days-before-stale: 90, days-before-close: 30, and inactiveDays = 90 for locking. The note that the day-14 reminder resets the counter is right too, since actions/stale keys off updated_at. One thing on the exemption line.

1. The priority/critical exemption can't actually fire — that label doesn't exist in this repo (38 labels; lifecycle/frozen, good first issue, and do-not-merge are all real, but nothing priority-related). It's faithfully copied from exempt-issue-labels in stale.yaml, but that entry has never matched anything. Worth dropping it from the sentence or creating the label, since documenting an exemption that silently never applies is the same doc/reality drift this PR is fixing. (CONTRIBUTING.md:215)

[ArangoGutierrez]

The priority/critical exemption can't actually fire — that label doesn't exist in this repo (38 labels; lifecycle/frozen, good first issue, and do-not-merge are all real, but nothing priority-related). It's faithfully copied from exempt-issue-labels in stale.yaml, but that entry has never matched anything. Worth dropping it from the sentence or creating the label, since documenting an exemption that silently never applies is the same doc/reality drift this PR is fixing.

[yuanchen8911]

Already addressed — the sentence no longer names priority/critical; the exemption paragraph now lists lifecycle/frozen, good first issue, and do-not-merge.

You also identified the source, and that is fixed now too: #1925 removed the priority/critical entry from exempt-issue-labels and the matching line from the comment block above it. It merged, so stale.yaml on main no longer references the label at all.

Agreed on the framing — documenting an exemption that silently never applies is the same doc/reality drift this PR is fixing, which is why the config needed to change rather than only the prose.

[ArangoGutierrez]

The rewrite reads much better, and the exemption sentence now matches main after #1925 landed. Re-checked the new claims against the workflows: the daily-cron lag note is right (all three bots are daily — 06:00, 06:00, 07:00 UTC), and the once-per-PR dedupe is real. One caveat on how absolute that guarantee is.

1. "At most once per pull request" is very slightly stronger than the workflow guarantees. The listComments call in inactive-pr-reminder.yaml has no per_page and isn't paginated, so it only sees the 30 oldest comments — on a PR that piled up 30+ comments before its first reminder, the reminder falls off page 1, alreadyReminded comes back false, and a second one goes out. Easy to hit here given how chatty the review bots are. Either soften this to "normally once" or add per_page: 100 to that call, which would make the sentence exactly true. (CONTRIBUTING.md:215)

[ArangoGutierrez]

"At most once per pull request" is very slightly stronger than the workflow guarantees. The listComments call in inactive-pr-reminder.yaml has no per_page and isn't paginated, so it only sees the 30 oldest comments — on a PR that piled up 30+ comments before its first reminder, the reminder falls off page 1, alreadyReminded comes back false, and a second one goes out. Easy to hit here given how chatty the review bots are. Either soften this to "normally once" or add per_page: 100 to that call, which would make the sentence exactly true.

[yuanchen8911]

Confirmed and fixed in ce352ca42.

listComments at inactive-pr-reminder.yaml:66 passes no per_page and is not paginated, so it sees only the 30 oldest comments — while the PR-list call at :52 does pass per_page: 100. On a pull request with heavy traffic before day 14, the reminder falls off page one, alreadyReminded returns false, and another reminder goes out; each subsequent one compounds it.

"At most once per pull request" was too strong. It now reads "normally posted only once per pull request", with a clause naming the first-page limitation as the reason a second can occur.

I softened the prose rather than adding per_page: 100 to keep this PR doc-only — the workflow fix is worth doing, and belongs in its own PR for the same reason the stale.yaml change went to #1925 instead of here.

[yuanchen8911]

Update — folded the workflow fix in rather than documenting the defect, so this reply supersedes my previous one.

inactive-pr-reminder.yaml now paginates:

const comments = await github.paginate(github.rest.issues.listComments, {
  owner: context.repo.owner, repo: context.repo.repo,
  issue_number: pr.number, per_page: 100,
});

One correction to the suggested fix: per_page: 100 on its own would not have made the sentence exactly true. It raises the ceiling from 30 to 100 without removing the failure mode, and this repo has PRs in that range — so the absolute claim would still have been false, just less often. github.paginate removes the bound, which is what lets the prose keep "at most once per pull request" rather than hedging to "normally once".

actionlint passes. The PR now also carries area/ci, and the title and body are updated to reflect that it is no longer doc-only.

Thanks for catching this — the original wording was mine, written this morning while fixing the opposite error (the table implied the reminder recurred), and I overcorrected into an absolute the code did not support.

[njhensley]

Review method

Four independent persona reviewers (correctness, docs-accuracy, operability, repo-consistency), then an adversarial senior meta-review that re-derived every finding from the resolved code rather than from the reviewer's claim. 8 of 14 candidate findings were refuted or downgraded in that pass.

Legend: 🔴 Blocker · 🟠 Major · 🟡 Minor · 🔵 Nitpick

Assessment

This is a well-executed correction, and I checked the substance rather than taking the table on trust: all six new rows were independently re-derived from the YAML and all six are factually correct. The old 60/74/44 figures were genuinely wrong, and the reframing from absolute days to inactivity windows is right — all three bots key on updated_at, and actions/stale defaults remove-stale-when-updated: true. The claim that the day-14 reminder itself pushes the stale mark out is correct. Row-by-row: :203↔inactive-pr-reminder.yaml:63, :204↔stale.yaml:78, :205/:207↔stale.yaml:95, :206↔stale.yaml:64, :208↔lock-threads.yaml:45,60,89, :202↔triage.yaml:36-45.

The pagination change is idiomatic — github.paginate(route, params) is the documented octokit form, corroborated by the identical in-production pattern at issue-report.yaml:70/:78 on the same pinned action SHA — and dropping the const { data } destructure was required and correct.

Approving. The findings below are all non-blocking; the single 🟠 is in the PR description, not the code.

🟠 The PR body contradicts its own diff in three places

Not code, but it is what a future reader reaches for first:

1. "Documentation only — no workflow behavior changes" and "Rollout notes: N/A — one Markdown table" both survive un-retracted from the pre-scope-change revision, directly contradicting the "Scope note (updated)" section. Component(s) Affected checks only Docs/examples (nothing covers .github/workflows/), and Type of Change leaves Bug fix unchecked though the dedupe fix is one.
2. The body says "stale.yaml exempts priority/critical and good first issue … so those are now named." stale.yaml:66 is 'lifecycle/frozen,good first issue' — #1925 removed the phantom label. The diff is right; the body describes a change it did not make.
3. The body defers items 2–3 because the triage skill "does not exist on main yet — it is added by #1911." #1911 merged (ff0301b5, 15:57Z) and .agents/skills/aicr-triage/SKILL.md is in this commit's tree. The deferral's substance still holds — the skill's "Stalled" is a board concept carrying its own caveat, so there is no contradiction to resolve — so fix the stated reason rather than expanding scope.

Also worth correcting for the record: the body's rationale for choosing github.paginate over per_page: 100 — "this repo has PRs in that range" — does not hold. issues.listComments returns issue comments only; review comments live at pulls/N/comments. The repo maximum is 20 (#1915), and no PR has ever exceeded 30. The comments:>30 search qualifier returns 91 PRs but counts review comments this call never sees. paginate is still the better choice — just not for the stated reason. Detail in the inline comment on the new code comment.

🟡 stale.yaml's own header omits good first issue

.github/workflows/stale.yaml:24-26 lists exemptions as only lifecycle/frozen and do-not-merge, while :66 has 'lifecycle/frozen,good first issue'. #1925 removed the phantom line but never added the real one, so this is now the last inconsistent restatement left in the tree — nothing in docs/, README.md, .github/ISSUE_TEMPLATE/, AGENTS.md, or the skills restates these counts. Not inline-commentable (the file is outside this diff), but it is a three-line fix in a directory this PR already touches, and closing it would finish the thesis.

🔵 Remaining nitpicks

• No retry or try-catch (:68) and no logging at all (:55-85) — one transient error aborts the loop and skips the remaining PRs, and a green run is indistinguishable from a silent duplicate. Both pre-existing and verifiably unworsened: paginate issues exactly one request when results fit in a page, so request count is literally unchanged. Noting only because this PR's own correctness claim is otherwise unobservable in production.
• Outer pulls.list is still unpaginated (:46-53), same argument as the new comment. It fails safe, and more strongly than it looks: sort: updated, direction: asc puts the stalest first, and stale-marking posts a comment that resets updated_at, pushing already-stale PRs to the back. Worth a one-line note that the ordering is load-bearing so a future edit doesn't flip it to desc.
• Nothing exercises the change. workflow_dispatch runs the real createComment against production PRs, and 0 PRs are currently in the 14–29 day window — a dispatch today executes the modified line zero times.

Confirmed non-issues (examined and cleared)

• CodeRabbit's "mirror these thresholds in DEVELOPMENT.md" — DEVELOPMENT.md contains zero lifecycle content today; mirroring would create a third source of the same numbers. The author's rebuttal is correct, and declining was the right call.
• "never marked lifecycle/stale in the first place, not merely spared from closing" — accurate; actions/stale exemptions gate marking, not just closing.
• The 90-day lock row is more accurate than the lock bot's own comment text ("closed for 90 days"), since the workflow reads updated_at.
• concurrency without cancel-in-progress — correct as-is; the default queues a dispatch behind a scheduled run rather than killing it mid-loop, which matters because the script has no resumability.
• Rate limits and timeout-minutes: 10 — no realistic path to either (7 open PRs, one request each).
• Every label the doc names (lifecycle/frozen, good first issue, do-not-merge) exists in the repo — no repeat of the phantom-label class of bug.
• Labels, 58-char title, DCO sign-off, GPG signature, AGENTS.md sync scope, yamllint (87 chars vs 200 max), lychee — all clean; CI green including actionlint.

🔴 0  |  🟠 1  |  🟡 6  |  🔵 6

[njhensley]

🟡 Minor — Inline comment asserts an unreachable failure as observed behavior

"so on a busy PR the reminder falls off page one and a duplicate goes out" reads as a described incident. Measured across all 1363 PRs in this repo, the maximum issue-comment count is 20 (#1915) — no PR has ever crossed 30. issues.listComments returns issue comments only; review comments live at pulls/N/comments, which is why GitHub's comments:>30 search qualifier (91 PRs) overcounts for this call. Unreachability is doubled: a duplicate needs >30 issue comments accumulated after the reminder — which itself resets updated_at — plus 14 more days of silence. Production history is 2 reminders ever posted (#485, #676), zero duplicates.

Blast radius: None at runtime. The fix is correct hardening and is cost-neutral (with max 20 comments, paginate issues exactly one request — identical to the pre-change call count). The concern is the durable comment describing a failure mode that has never been reachable here.

Fix: Reframe defensively rather than as an incident report, e.g. "an unpaginated call returns only the 30 oldest comments; paginating removes the bound so the guarantee holds regardless of comment volume."

[njhensley]

🟡 Minor — Dedupe marker is a bare literal, now load-bearing for a documented guarantee

The key 'inactive for' is coupled by nothing but convention to the message template on line 83. Reword that template ("idle for", "no activity in") and every previously-reminded PR reads as un-reminded; because the reminder bumps updated_at, each PR re-enters the 14–29 day window ~14 days later and is nudged again on a permanent 14-day cadence. A copy edit silently re-creates the exact defect this PR fixes — while CONTRIBUTING.md is simultaneously upgraded to assert "at most once per pull request" as a guarantee. Converse risk: any other Bot comment containing the substring suppresses the reminder forever (the stale and lock messages don't match, but a bot quoting PR discussion could).

Separately on this line: c.user.type is dereferenced without a guard. GitHub's schema marks issue-comment.user nullable, and there is no try/catch, so a null would kill the whole step. (Low likelihood — GitHub substitutes the ghost account in practice, and I found zero null-user comments in this repo.)

Blast radius: Latent. Requires a future edit to the message text to trigger, but the trigger is an innocuous-looking copy edit and the failure is silent and recurring.

Fix: Hoist a hidden marker — const MARKER = '<!-- aicr:inactive-pr-reminder -->' — append it to the line-83 body and match on it instead of the prose substring. Optionally narrow c.user.type === 'Bot' to c.user?.login === 'github-actions[bot]', which fixes the null deref and the false-positive at the same time.

[njhensley]

🟡 Minor — "stay out of the stale process entirely" overclaims — no label exempts locking

lock-threads.yaml has no label check anywhere — its only skips are issue.locked / pr.locked (:58, :87). Nothing exempts a closed thread from the 90-day lock, lifecycle/frozen included, yet the table at :208 lists locking as part of this same lifecycle. A reader who freezes a thread to keep the conversation open still finds it locked 90 days after close.

Also omitted here: both lifecycle/frozen and do-not-merge additionally suppress the day-14 reminder (skipLabels, :44, checked :57), so a contributor with do-not-merge gets no nudge either.

Blast radius: Doc-only, but it is the one place in the new section where a reader can be led to expect protection that does not exist.

Fix: Narrow the heading to "To stay out of stale-marking and auto-close:", note that exempt items are also skipped by the day-14 reminder, and add that no label exempts a closed thread from the 90-day lock.

[njhensley]

🟡 Minor — "the day-30 stale mark" reintroduces the absolute-day framing line 210 just warned against

Substantively correct in its own scenario — after reply-then-quiet the reminder genuinely will not re-fire, so the stale mark is next. But "day-30" reads as a date three lines after the paragraph telling readers these are not days since opening. If the contributor does not reply, the day-14 reminder itself bumps updated_at, so the mark actually lands around day 44. Self-inflicted ambiguity in the one section whose purpose is removing exactly this confusion.

Blast radius: Doc-only; a reader gets two different implied dates from adjacent paragraphs.

Fix: "— the next automated action is the stale mark, 30 days after that reply."

[njhensley]

🟡 Minor — needs-triage row omits that the label is removed automatically

triage.yaml adds the label on issues: opened (:36-45), but the very next step infers an area/* label and strips needs-triage in the same run (:222-236); a separate step removes it on any non-needs-triage label event (:238-257). The inference is five-deep (component field → documentation label → title scope → title text → body text) and effectively always hits for template-created issues. Empirically 0 of 83 open issues currently carry the label.

Blast radius: Doc-only. The row is accurate as far as it goes; it just names an automation step whose immediate automated reversal is absent.

Fix: "| On open | needs-triage added to issues (removed automatically once an area/* label is applied) |"

[njhensley]

🔵 Nitpick — Two rows share identical "When" cell text

:205 and :207 both read "30 days after being marked stale", disambiguated only by row adjacency.

Blast radius: Cosmetic.

Fix: "30 days after the PR is marked stale" / "30 days after the issue is marked stale".

[github-actions]

Recipe evidence check

No leaf overlays affected by this PR.

This gate is warning-only and never blocks merge.