fix(review): age stale CI from required checks

[neubig]

Why

GitHub updates a pull request's updated_at timestamp for comments and reviews. The stale-CI automation used that timestamp as a proxy for failure age, so an automated reviewer comment could hide required CI that had already been failing for weeks. A live audit found 43 non-draft PRs with required failures older than seven days, while only seven were tracked.

Summary

• Paginate every open PR directly and classify its latest required-check rollup.
• Start the warning window from the newest required-check failure, independent of comments and reviews.
• Fall back to the existing paginated REST readers if a PR has more check contexts than the inline GraphQL page.
• Backfill existing unwarned records while preserving fresh windows created by author follow-up.

Issue Number

Closes #683

How to Test

uv run --with pytest --with pyyaml --with jsonschema --with pydantic --with deprecation --with packaging --with requests --with openhands-sdk python -m pytest -q
python3 scripts/sync_extensions.py --check
npm run build:automations
git diff --check

The complete suite passes with 992 tests passed and 23 skipped. Focused pagination and catalog validation passes with 128 tests. Coverage includes multiple PR pages, cursor progress, more than 100 CI contexts, REST fallback pagination, and retry of transient GitHub 5xx responses. A read-only live audit paginated 837 open PRs across the four configured repositories and identified all 43 PRs whose newest required failure was older than seven days: 2 in extensions, 40 in OpenHands, 0 in software-agent-sdk because it has no required checks configured, and 1 in automation.

Live evidence

Exact head e590e9c is installed in hourly automation 1cad57e0-012e-4e0c-9729-9c25f9bbfaa2 on the OSS Agent Canvas.

• Run 5d6be4ee-1483-4a30-b6f5-fa2fdff0fa3c completed in 177 seconds without error.
• It paginated the complete open-PR set for all four repositories and found the same 43 stale failures. All 43 were already in the warning window, so it reported waiting without posting duplicate comments.
• Representative original warning comments remain on extensions #392, OpenHands #16105, and automation #211.
• Git sync completed without error at OpenHands/oss-agent-canvas@638cc0a1df249f7b809bb33d50dbefbe49b9a28f; its synced worker is byte-for-byte identical to the worker at this PR head.

HUMAN

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope

This change belongs in this repository. It modifies the github-stale-ci-pr-closer automation bundle (skills/github-stale-ci-pr-closer/scripts/worker.py, its tests, and automations/catalog/github-stale-ci-pr-closer/manifest.json), which is exactly the artifact OpenHands/extensions owns. No product/architecture decision is needed.

What I verified on the current head (e590e9c)

• Scope/ownership per AGENTS.md: in-repo and focused on one automation bundle.
• git diff --check clean; python scripts/sync_extensions.py --check exits 0; npm run build:automations leaves automations/bundle-index.js unchanged (generated artifact already in sync with the source worker.py and manifest).
• skills/github-stale-ci-pr-closer/tests/ plus tests/test_automation_setup.py and tests/test_catalog_schema.py: 241 passed, 17 skipped.
• GitHub Actions for the exact head: all success/skipped (no failures).
• Issue #683 is still ready-for-dev, priority:low; the PR still references it via Closes #683.
• GraphQL open-PR pagination, the REST fallback for >100 contexts, the newest-failure max(failures) aging, the retry-on-5xx, and the cursor-progress guard all match the acceptance criteria and are covered by tests.

Finding

The legacy-state backfill is the one place where behavior diverges from the stated intent, and it is not covered by tests. Details on the inline comment.

Because that defect can immediately re-open a warning/close cycle for a PR whose author had just followed up, I am not approving as-is.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope

This change belongs in this repository. It modifies the github-stale-ci-pr-closer automation bundle (skills/github-stale-ci-pr-closer/scripts/worker.py, its tests, and automations/catalog/github-stale-ci-pr-closer/manifest.json), which OpenHands/extensions owns. No cross-repository move or maintainer decision is required.

State re-checked at review time

• Head is still e590e9c (unchanged), PR open and mergeable, label type: fix, Closes #683.
• GitHub Actions for this exact head: all success/skipped, no failures.
• Issue #683 is still ready-for-dev, priority:low.
• Focused tests: skills/github-stale-ci-pr-closer/tests/ + tests/test_automation_setup.py + tests/test_catalog_schema.py -> 241 passed, 17 skipped.
• Generated artifacts in sync: npm run build:automations produces no drift; git diff --check clean.

Blocking finding (still present, unchanged since the previous review)

The legacy-state backfill at skills/github-stale-ci-pr-closer/scripts/worker.py:381 still collapses fresh warning windows that the previous release created via author follow-up. The previous release's follow-up branch replaced the record with {"head_sha": head, "first_failed_at": now} and wrote no reset_at marker, so legacy follow-up records are indistinguishable from first-observation records; min() then re-ages them from the older newest required-check failure and undoes the follow-up reset.

I re-reproduced this on the current head: required check failing 30 days (_failed_at = now - 30d), legacy record {"head_sha": "abc", "first_failed_at": now - 3d} (reset by an author follow-up 3 days ago), no persisted warning. reconcile returns "warned" and posts a new warning in the same run, when preserving the fresh window would return "observing". This contradicts the PR summary's "Backfill existing unwarned records while preserving fresh windows created by author follow-up", and the existing test_backfills_legacy_observation_time_from_check_failure does not exercise this case.

This is tracked in the existing unresolved inline thread on line 381, so I am not opening a duplicate comment. Fix suggestion: gate the backfill so it cannot move first_failed_at earlier when the record came from a follow-up (e.g. only backfill true first-observation records, or derive the reset from the newest required-check failure only when no follow-up comment predates it), and add a test for a legacy follow-up record.

Not approving as-is while this migration defect remains.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope

This change belongs in this repository. It modifies the github-stale-ci-pr-closer automation bundle (skills/github-stale-ci-pr-closer/scripts/worker.py, its tests, and automations/catalog/github-stale-ci-pr-closer/manifest.json), which OpenHands/extensions owns. No cross-repository move or maintainer decision is required.

State re-checked at review time

• Head is still e590e9cf014e7039759ca4fc4c86c711deeb8a92; PR open, label type: fix, Closes #683.
• GitHub Actions for this exact head: all success/skipped, no failures.
• Issue #683 is still ready-for-dev, priority:low.
• Focused tests: skills/github-stale-ci-pr-closer/tests/ + tests/test_automation_setup.py + tests/test_catalog_schema.py -> 241 passed, 17 skipped.

Merge conflict (new since the previous review)

The PR is now CONFLICTING / dirty against main (which has advanced to bea7a20). git merge-tree reports exactly one conflict, in the auto-generated automations/bundle-index.js; main regenerated that file for other bundles (e.g. github-pr-reviewer) while this branch regenerated it for github-stale-ci-pr-closer. This is mechanical generated-file churn, not a code defect: it resolves by rebasing and re-running npm run build:automations. I verified main's concurrent changes to the shared skills/github/scripts/github_client.py are additive (check_runs, required_check_contexts) and do not break this worker, which still uses gh_pages/api. Flagging only because the branch cannot merge as-is.

Blocking finding (still present, unchanged)

The legacy-state backfill at skills/github-stale-ci-pr-closer/scripts/worker.py:381 still collapses fresh warning windows that the previous release created via author follow-up. The previous release's follow-up branch replaced the record with {"head_sha": head, "first_failed_at": now} and wrote no reset_at marker (see origin/main:skills/github-stale-ci-pr-closer/scripts/worker.py:284), so legacy follow-up records are indistinguishable from first-observation records; the not record.get("reset_at") guard is true for both, and min() then re-ages the record from the older newest required-check failure, undoing the follow-up reset.

Re-reproduced on this head: required check failing 30 days (_failed_at = now - 30d), legacy record {"head_sha": "abc", "first_failed_at": now - 3d} (reset by an author follow-up 3 days ago), no persisted warning. reconcile returns "warned" and posts a new warning in the same run, whereas preserving the fresh window would return "observing". This contradicts the PR summary's "Backfill existing unwarned records while preserving fresh windows created by author follow-up", and test_backfills_legacy_observation_time_from_check_failure does not exercise the follow-up case.

This is the same defect tracked in the existing unresolved inline thread on line 381 (PRRT_kwDOQcEru86l2dLc), so I am not opening a duplicate comment. Suggested fix: gate the backfill so it cannot move first_failed_at earlier when the record came from a follow-up (for example, only backfill true first-observation records, or derive the reset from the newest required-check failure only when no follow-up comment predates it), and add a test for a legacy follow-up record.

Not approving while this migration defect remains.

🔄 CHANGES REQUESTED

[all-hands-bot]

This review was posted by an AI agent (OpenHands).

Scope

This change belongs in this repository. It modifies the github-stale-ci-pr-closer automation bundle (skills/github-stale-ci-pr-closer/scripts/worker.py, its tests, and automations/catalog/github-stale-ci-pr-closer/manifest.json), which OpenHands/extensions owns. No cross-repository move or maintainer decision is required.

State re-checked at review time

• Head is fb33332d8f181a0335d47f579ed79af29139fc7a; PR open, MERGEABLE, label type: fix, Closes #683.
• GitHub Actions for this exact head: all success/skipped, no failures.
• Issue #683 is still ready-for-dev, priority:low.
• Focused tests: skills/github-stale-ci-pr-closer/tests/ + tests/test_automation_setup.py + tests/test_catalog_schema.py -> 243 passed, 17 skipped.
• npm run build:automations produces no drift; the regenerated automations/bundle-index.js embeds the current worker.py (verified byte-for-byte, including _backfill_first_failed_at); git diff --check clean; scripts/sync_extensions.py --check passes (only the pre-existing non-blocking marketplace-coverage warning).

Prior finding resolved

The earlier blocking issue — the legacy-state backfill at worker.py:381 re-aging a record that a pre-marker release had reset on author follow-up, collapsing the fresh seven-day window — is fixed in ec3951e and the old thread is resolved/outdated. The fix moves the decision into _backfill_first_failed_at(), which recognizes a legacy reset from the comment history (author_followed_up(pr, comments, failed_at)) rather than from record shape, marks reset_at, and leaves the fresh window intact; records with no such follow-up comment are still aged, which is the behavior the backfill exists for. reconcile now also skips the backfill when reset_at is already present, and the follow-up branch persists reset_at.

I re-verified this independently on the current head with three scenarios: (a) legacy follow-up reset -> observing, no comment posted, reset_at recorded; (b) genuine legacy first sighting with no author comment -> still aged and warned; (c) legacy reset whose fresh window has fully elapsed -> warned. The two new regression tests (test_backfill_leaves_a_legacy_follow_up_reset_alone, test_legacy_follow_up_reset_still_warns_after_a_full_fresh_window) cover both directions.

The merge of main (release 0.25.0) is clean — the conflict in the generated automations/bundle-index.js reported at the previous head is gone, and main's additive changes to skills/github/scripts/github_client.py do not affect this worker. I found no other material defects in the required-check classification, GraphQL pagination, REST fallback, or retry paths.

No material findings remain.

✅ APPROVED

[openhands-release-bot]

🚀 Released in v0.26.0.