Add a stale-assignment sweep: warn-then-release ownership decay (#209)

[justin808]

Summary

Adds skills/pr-batch/bin/stale-assignment-sweep — a dry-run-first warn-then-release sweep that decays inactive human assignments so owned means skip (#208) can't reserve work forever. Assignment is a lease, not a deed.

Behavior

• Default is dry-run — zero GitHub mutations; prints a digest of would-nudge / would-release items with their clock, days inactive, and assignee. Writes only behind --apply. (Ship report-only for ~2 weeks to tune TTLs before enabling writes.)
• Clocks (config-driven): time-to-first-activity (7d anti-squatting) and inactivity-after-start (14d issues / 7d PRs). A lease is renewed only by the assignee's own commits/comments/reviews/linked-PR events.
• Flow: nudge → 4-day grace → release. Never releases without a prior unanswered nudge; any assignee reply resets; exempt labels (blocked, on-hold) pause the clock.
• Automation never swept — automation set resolved exactly as Skip human-assigned issues and PRs in batch selection and triage #208 selection does: trust-config trusted_bots + [bot]-suffixed logins via the pr-security-preflight chain (trusted_users are human/sweepable). Fails closed (leaves human assignments untouched) when the set can't be resolved. agent-claimed items are skipped entirely (owned by backend heartbeats — the Mirror agent lane claims to GitHub with an agent-claimed label #210 hook).
• Safety: leading-@ guard on comment bodies, valid_login? checks before any gh -f field, --repo shape validation, timeout-bounded gh. Nothing silently dropped — every skip is reported with its reason.

Config / flags

--repo (repeatable), --apply, --now/STALE_ASSIGNMENT_SWEEP_NOW, --first-activity-ttl-days, --issue-inactivity-ttl-days, --pr-inactivity-ttl-days, --grace-days, --exempt-label, --comment-identity, --trust-config; gh bounded by STALE_ASSIGNMENT_SWEEP_GH_TIMEOUT_SECONDS.

Tests

skills/pr-batch/bin/stale-assignment-sweep-test.rb — 16 tests / 113 assertions (fake gh on PATH, deterministic injected clock), registered in bin/validate. Covers: dry-run zero-mutation, nudge/grace/release, release-requires-prior-nudge, reply-resets, exempt-pause, agent-claimed skip, automation-never-swept, co-assigned-bot-kept, fail-closed, and @-injection guards. rubocop _1.87.0_ clean.

Acceptance criteria (issue #209)

• Dry-run digest listing would-nudge / would-release with reasons (clock, days).
• With writes: nudge → 4-day grace → release, each visible as comments.
• Exempt labels pause; assignee reply resets (both tested).
• Config (repos, TTLs, exempt labels, identity) documented.
• Skips agent-claimed items (Mirror agent lane claims to GitHub with an agent-claimed label #210 marker).

Closes #209.

Part of the assignment-aware series (#208 merged → #209 → #210).

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 9 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 66261859-e087-4d63-bedf-ed4c9173a7b9

📥 Commits

Reviewing files that changed from the base of the PR and between 0b2f048 and fa9d22a.

📒 Files selected for processing (5)

• CHANGELOG.md
• bin/validate
• docs/pr-batch-skills.md
• skills/pr-batch/bin/stale-assignment-sweep
• skills/pr-batch/bin/stale-assignment-sweep-test.rb

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg/209-stale-assignment-sweep

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0574053fd5

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: stale-assignment sweep

Well-structured addition — dry-run-first default, fail-closed automation resolution, @-injection guards before any gh -f field, and a solid 16-test/113-assertion suite with a fake gh fixture harness. Overall design (nudge → grace → release, lease renewed only by the assignee's own activity, exempt labels pausing the clock) is sound and matches the issue #209 acceptance criteria.

Two issues flagged inline, one worth a hard look before merge:

🔴 Correctness bug — automation_login? (line 220)

It matches trusted_bots by bare login (no [bot] suffix required), unlike the reference implementation in pr-security-preflight's bot_login_in_set?, which requires end_with?("[bot]") before checking set membership. String#delete_suffix is a no-op when the suffix isn't present, so any login whose plain name matches a trusted_bots entry is misclassified as automation. This repo's own .agents/trusted-github-actors.yml lists claude in both trusted_users and trusted_bots — so a human-assigned claude login would be silently ignored by the sweep forever, the exact failure mode this PR is meant to prevent. See inline comment for a one-line fix.

🟡 Robustness — nudge marker spoofable without --comment-identity (line 437)

--comment-identity is optional; without it, last_nudge_time accepts a NUDGE_MARKER from any commenter (the marker is a public, hardcoded string). A forged comment could start the grace clock and lead to a premature --apply release. Consider requiring --comment-identity when --apply is set, or documenting it as mandatory for scheduled runs.

Minor / non-blocking

• N+1 API calls: sweep_repo fetches a full timeline per assigned item sequentially (item_timeline). Fine for typical repo assignment volumes given this is a periodic/cron job, but worth noting if repos with hundreds of open assigned items are in scope.
• Co-assignee semantics: when an item has multiple human assignees, any one assignee's activity protects the whole item, and release removes all human assignees together. That's a defensible simplification for v1, but worth confirming it matches intent — a genuinely-active co-assignee currently shields a squatting co-assignee indefinitely.
• Nice touches: valid_login? defense-in-depth (checked both at classification and again in remove_assignee), EPOCH fallback for missing assignment timestamps favoring leniency, and hidden HTML-comment markers for idempotent nudge/release detection regardless of posting identity.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 952a5498e8

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Stale-Assignment Sweep

Overview: Adds skills/pr-batch/bin/stale-assignment-sweep, a dry-run-first CLI that nudges then releases stale human assignments on issues/PRs (closes #209). Well-scoped: default dry-run with zero mutations, fail-closed automation-set resolution mirroring pr-security-preflight, re-classification immediately before any release to guard against races, and guards against comment-forgery and -f @file injection. 16 tests / 113 assertions cover the main flows, including forged-nudge and re-check-abort scenarios.

Strengths

• Defense in depth: valid_login? gate before any gh -f field, leading-@ rejection on comment bodies, --repo shape validation before any gh call, fail-closed on unresolved trust config/identity.
• Re-fetches and re-classifies the live item immediately before apply_release, closing the classify→mutate race window (nice touch, and it's tested).
• All gh invocations go through Open3.capture3("gh", *args) (argv array, not a shell string) — no shell-injection surface.
• Comprehensive negative-case coverage: forged nudge markers, coassigned bots, unresolved trust config, unresolvable identity.

Issues found (see inline comments for detail):

1. Likely bug — event_actor_login can't attribute GitHub's committed timeline event to a login, since that event's author/committer fields are raw git identities (name/email/date), not linked GitHub accounts with a login. Since committed is listed in ACTIVITY_EVENTS and both the code comments and PR docs advertise "renewed... by the assignee's own commits," pushing commits alone likely never renews the lease in practice — undermining the anti-squatting guarantee for PR assignees who push code without commenting. No test fixture exercises a real committed event, so this wouldn't be caught today.
2. Minor — run_gh's Timeout.timeout(gh_timeout) { Open3.capture3(...) } raises on timeout but doesn't kill the underlying gh child process, so a hung gh call can leave an orphaned subprocess running after the sweep reports the timeout error.

Nice work overall — the design is careful about the destructive-action risk surface here (leases, grace windows, fail-closed everywhere). The commit-activity gap is the one thing worth confirming/fixing before relying on this for PRs where assignees mostly push commits rather than comment.

[claude]

Review: Stale-assignment sweep (#217)

Overall this is a well-designed, defensively-written addition. It mirrors pr-security-preflight's trust-config resolution and bot-detection logic almost exactly (same env vars, same precedence order, same [bot]-suffix + trusted_bots check), fails closed when the automation set cannot be resolved, validates all logins before they reach gh -f fields, and re-fetches + re-classifies live state immediately before every mutation (nudge and release) to guard against races between scan and apply. Dry-run-by-default with zero mutations is the right default for a tool that removes assignees.

I traced the logic by hand against the accompanying test fixtures (could not execute ruby in this sandbox: command execution required approval that was not available) and did not find a functional defect. Two things worth a look, flagged inline on the relevant lines:

1. comment-identity mismatch risk: if the value passed does not match the login gh actually posts as, the sweep can never recognize its own past nudge comments, so it keeps re-nudging the same stale item on every scheduled run instead of ever reaching grace/release. Worth a warning, since this tool is meant to run unattended on a cron.
2. Multi-assignee lease sharing: for co-assigned items, any one assignee's activity resets the clock for the whole item (and a release removes all human co-assignees together). Reasonable simplification, but it means one active co-assignee can indefinitely shield an inactive one from ever being nudged or released. Worth calling out explicitly in the docs if intentional.

Neither blocks merging given the safety-first dry-run design; both are candidates for a follow-up or doc note.

Other observations (non-blocking):

• resolve_identity (and its gh api user call) runs before --repo format validation in run, so a malformed --repo still triggers one gh call first. Harmless ordering nit.
• No concurrency for the per-item gh timeline fetches, and --apply doubles them (scan plus re-check before mutation). Fine at current scale; could matter on repos with very large numbers of stale assignments.
• The packaged fallback trust config path (skills/pr-batch/trusted-github-actors.yml) does exist, and .rubocop.yml already disables the metrics cops that would otherwise flag classify/decide branching, so the rubocop-clean claim checks out structurally.

Tests are extensive (dry-run, nudge, grace, release, reply-reset, exempt-label pause, agent-claimed skip, forged-marker rejection, re-check aborts, bot vs bare-login disambiguation, fail-closed) and I traced several of them (e.g. items #2 and #3 grace-day math) against the implementation by hand, and the numbers line up.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 78dea697fc

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: stale-assignment sweep (#217)

Overview. Adds skills/pr-batch/bin/stale-assignment-sweep, a dry-run-first Ruby tool that decays stale human issue/PR assignments (nudge → 4-day grace → release), plus a thorough 16-test/113-assertion suite, docs, and bin/validate registration. This is a well-scoped, carefully-defended piece of automation — no external gems, all gh calls go through Open3.capture3 with an argv array (no shell interpolation, no injection risk), YAML is loaded with safe_load_file(aliases: false), assignee logins are validated before being used in -f fields, and comment bodies are guarded against a leading @ (which gh -f would otherwise treat as a file reference). The automation-detection logic ([bot] suffix + trusted_bots base-name match) is verified to mirror pr-security-preflight's bot_login_in_set? exactly, and the trust-config resolution order (explicit → repo-local → env → user-global → packaged-fallback) matches that script's chain too.

The core state machine (fail-closed on unresolved trust, own-identity-only nudge markers, live re-check before every mutation, multi-human items surfaced instead of auto-swept, committed events correctly excluded from lease-renewal since they carry no GitHub login) is sound and the test fixtures exercise the tricky edges (forged nudge marker, mismatched --comment-identity, re-check aborting a nudge/release when state changed mid-run, bot vs. bare-login-matching-a-bot-name).

Issues found (posted inline)

1. --exempt-label replaces the built-in defaults instead of adding to them (stale-assignment-sweep:96-108). options[:exempt_labels] starts nil; the first --exempt-label flag creates a fresh array, and the blocked/on-hold defaults are only backfilled via ||= if the flag was never passed. An operator passing --exempt-label needs-design to add a project-specific pause label will silently lose the blocked/on-hold exemptions — contradicting the --help text ("repeatable (default: blocked, on-hold)"). No test covers this flag's actual runtime behavior.

2. A single gh failure aborts the entire run, across all repos (stale-assignment-sweep:61). Error from a timed-out/rate-limited/transient gh call inside classify's timeline fetch is only rescued at the top of run, so one flaky call on one item in one repo drops the digest for that repo (computed results are discarded mid-items.map) and skips every other --repo still queued. Given this is meant to run unattended on a cron/schedule, some form of per-item or per-repo isolation for the read-only classification path would make it meaningfully more robust.

Minor / non-blocking observations

• The nudge comment text ("Any reply keeps it") and the internal "renewed: assignee replied after nudge" label both describe the reset trigger as a reply, but reviews and cross-referenced PR/commit events also renew the lease per ACTIVITY_EVENTS. Slightly imprecise wording, not a functional bug.
• --grace-days/--*-ttl-days accept negative integers with no validation; harmless self-inflicted misconfiguration but an Integer positivity check would fail faster than confusing downstream date math.
• Per-item gh api .../timeline calls (one per candidate item, plus a second re-fetch per mutated item under --apply) mean sweep cost scales roughly linearly with the number of currently-assigned items per repo; fine at current scale, worth watching if --repo lists grow large or issues carry very long timelines (ties into finding Add Claude Code Review GitHub Actions workflows #2's timeout risk).

Overall: solid, well-tested implementation of a genuinely tricky decay/lease state machine. The two issues above are worth addressing before relying on this for unattended scheduled runs, but neither is a security concern.

[claude]

Review: stale-assignment sweep (#209)

Overall this is a well-designed, cautious piece of automation — dry-run-first default, fail-closed on unresolved trust config, re-check-before-mutate on both nudge and release, per-item/per-repo error isolation, and a genuinely thorough test suite (16 tests covering the tricky edge cases: forged markers, grace windows, multi-human items, closed-between-scan-and-apply, bot co-assignees). Nice work.

Findings

1. Automation detection can default "open" instead of fail-closed when the trust config is valid but empty (automation_login?, resolve_trust_config_path). @trust_resolved only goes false on a load error (missing/malformed file). If a deployment falls through to the packaged fallback (skills/pr-batch/trusted-github-actors.yml, which ships with trusted_bots: []), the config loads successfully but classifies every [bot]-suffixed login as human — because trusted_bots is empty, no bot ever matches. That means a repo with no repo-local/env/home trust config configured would have dependabot[bot], renovate[bot], etc. treated as sweepable "human" assignees: nudge comments posted to them and, after grace, their assignment removed under --apply. This mirrors pr-security-preflight's trust-config resolution, but there "not a trusted bot" fails toward suspicion (safe direction); here it fails toward "sweepable" (permissive direction) — same file/logic reused for an inverted risk profile. Left commented inline.

2. Minor / edge case: in lease_reference, when an assignee's assigned timeline event can't be found, last_activity falls back to EPOCH (1970) as the "since" cutoff rather than skipping to the time-to-first-activity branch. If that assignee has any old activity on the issue from before the (missing) assignment event — e.g., they commented pre-assignment, then went quiet — it could be picked up as "recent enough" and mask genuine inactivity. Low likelihood (GitHub timelines normally do carry an assigned event), but worth a comment noting why EPOCH is the intended fallback vs. a rare misclassification path.

3. Duplication risk (maintainability, non-blocking): automation_login?, resolve_trust_config_path, normalized_login/normalized_bot_login, and the trust-config load path are a near copy of the equivalent logic in pr-security-preflight. Since these are single-file portable scripts there's no easy shared-lib extraction today, but if the bot-detection semantics in pr-security-preflight ever change, this file won't pick it up automatically. Not asking for a refactor now, just flagging for awareness.

4. Performance/scale note: sweep_repo fetches all open assigned items via one paginated call, then does a separate item_timeline fetch per human-assigned item (plus another fetch_issue + item_timeline re-check per nudge/release under --apply). For a repo with a large number of assigned issues/PRs, or a long --repo list, this is O(N) additional gh api calls per run and could add up against rate limits. Probably fine for a periodic cron sweep, but worth keeping in mind if this gets pointed at busy repos.

Nice details

• The -f body=@...-injection guard (reject_leading_at!) and repo-shape validation (validate_repo!) are exactly the kind of defensive checks I'd want to see before anything reaches gh -f.
• Re-classifying from a live fetch immediately before apply_nudge/apply_release (not just trusting the snapshot) closes a real TOCTOU-style gap.
• Fail-closed behavior when the sweep identity can't be resolved (no releases, but nudging/reporting still proceed) is a sensible default.

Nothing here is release-blocking given the dry-run-first rollout plan, but #1 is worth a decision before --apply is turned on anywhere without a hand-authored trust config.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 65eadaa002

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Review: Add stale-assignment sweep (#209)

Overview

Adds skills/pr-batch/bin/stale-assignment-sweep, a dry-run-first Ruby CLI that decays inactive human assignments on issues/PRs (nudge → 4-day grace → release) so owned means skip (#208) can't reserve work indefinitely. Ships with a thorough test suite (16 tests / 113 assertions against a fake gh), docs in docs/pr-batch-skills.md, a CHANGELOG entry, and registration in bin/validate.

Code quality

• Very well-hardened for a first version: fail-closed automation detection, fail-closed on unresolved trust config, live re-check immediately before every mutating action (nudge/release), release-requires-prior-own-nudge, leading-@ guard against gh -f field=@value file-path interpretation, and valid_login? gating before any assignee is passed to gh.
• Open3.capture3 is called with an argv array (not a shell string), so there's no shell-injection surface even though comment bodies interpolate assignee-derived text.
• Good exception boundaries: a single item's gh failure degrades to an UNKNOWN … skipped digest line instead of aborting the run; a single repo's listing failure warns and moves on to the next --repo; a failed DELETE during --apply skips just that item's audit comment (no misleading "released" comment on failure).
• The doc explicitly and correctly calls out that the "empty trusted_bots" fallback in this script (treat any [bot]-suffixed login as automation) intentionally diverges from pr-security-preflight's stricter bot_login_in_set? (which requires explicit membership). That's the safer default for a sweep that only ever removes access, so no concern there — just noting it's a deliberate, well-documented divergence rather than an accidental one.
• Two minor items left as inline comments: an unused @trusted_users field, and an N+1 API-call pattern (one full timeline fetch per assigned item, no caching between runs) worth watching once this moves from dry-run to scheduled --apply on larger repos.

Correctness

• Traced through the clock/lease logic (lease_reference, decide, release_or_wait) including reassignment edge cases (old nudge markers surviving a release+reassignment cycle, pre-assignment activity not renewing a lease, activity attribution only via login-bearing timeline events) — didn't find a logic bug; the test suite's "Fix A"-"Fix K" labels show these edge cases were already iterated on.
• Multi-human-assignee items are conservatively surfaced for manual review rather than swept, which correctly avoids the "one active co-assignee shields an inactive squatter" and "release removes an active co-assignee" failure modes.
• Pagination handling (gh api --paginate --slurp + .flatten) is consistent with how the test fixtures model paged output.

Security

• No shell injection (array-based subprocess invocation throughout).
• Fail-closed posture is consistent: unresolved trust config leaves all human assignments untouched; unresolved sweep identity disables releases (but still allows nudging/reporting) so a misconfigured --comment-identity can't cause a silent re-nudge loop or, worse, be spoofed into triggering a release via a forged nudge marker from another commenter (tested).
• remove_assignee re-validates the login format before issuing the DELETE, even though the login already passed through classification — good defense in depth.

Performance

• See inline note on item_timeline: one full paginated timeline fetch per assigned item every run, plus a second fetch per apply candidate for the live re-check. Fine at current scale (dry-run rollout), but no caching/cursor exists yet if this is later run frequently against repos with many long-lived assigned items — could be worth a follow-up if the two-week dry-run period surfaces rate-limit or latency issues.

Test coverage

• Strong: dry-run zero-mutation guarantee, nudge/grace/release flow, forged-marker rejection, identity mismatch warning, fail-closed trust/identity resolution, agent-claimed skip, automation-never-swept (including the empty-trusted_bots fallback and bare-login-matching-a-bot-base-name case), multi-human reservation, closed-between-scan-and-apply, negative-TTL rejection, and partial-failure resilience (one item / one repo). No gaps stood out during review.

Overall this is a careful, conservative implementation with no blocking issues found. Two minor suggestions left as inline comments.