feat: implement issue #1193 — pr-review advisory gate: timeout-proceed / skip absent advisory bots (durable hardening)

[don-petry]

Closes #1193

Implemented by dev-lead agent. Please review.

Summary by CodeRabbit

• Bug Fixes

  • Advisory review checks no longer wait indefinitely when advisory bots have not submitted results.
  • Reviews now proceed after the configured head-age timeout when no bot output is available.
  • Recent changes continue to wait, while unavailable commit timing information is handled conservatively.

• Tests

  • Expanded coverage for recent, timed-out, and unavailable head-commit scenarios.
  • Added validation for configurable timeout behavior.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Review limit reached

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

Next review available in: 18 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: ASSERTIVE

Plan: Pro

Run ID: 70d2bd2d-bc19-4764-bd3b-878aea9ae9c4

📥 Commits

Reviewing files that changed from the base of the PR and between 2cb05ff and a722f05.

📒 Files selected for processing (2)

• scripts/lib/advisory-review-gate.sh
• tests/dev-lead/unit/test_advisory_review_gate.bats

📝 Walkthrough

Walkthrough

The advisory review gate now uses configurable head-age and quiescence timeouts when advisory bots produce no output. It parses GraphQL committer timestamps, proceeds after the head-age timeout, and waits conservatively when head time is unavailable. Bats tests cover these outcomes.

Changes

Advisory gate timeout handling

Layer / File(s)	Summary
Head-age timeout decision flow scripts/lib/advisory-review-gate.sh	Adds timeout constants and _head_age_seconds, applies head-age checks to empty bot output, expands date parsing, and replaces the hardcoded fallback timeout.
Timeout behavior validation tests/dev-lead/unit/test_advisory_review_gate.bats	Tests recent heads, expired head-age timeouts, unavailable head times, and static timeout/helper declarations.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant check_advisory_reviews
  participant get_advisory_bot_states
  participant GitHubGraphQL
  check_advisory_reviews->>get_advisory_bot_states: request advisory bot states
  get_advisory_bot_states-->>check_advisory_reviews: no bot output
  check_advisory_reviews->>GitHubGraphQL: read committer.date
  GitHubGraphQL-->>check_advisory_reviews: head timestamp
  check_advisory_reviews-->>check_advisory_reviews: wait or proceed based on head age

Loading

Possibly related PRs

• petry-projects/.github-private#458: Earlier advisory gate implementation updated by this PR.

Suggested labels: needs-human-review

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	[#1193] The gate logic was hardened for no-output and absent-bot cases, but the PR does not show the required updates in review-batch/sweep-stuck-reviews.	Update scripts/review-batch.sh and scripts/sweep-stuck-reviews.sh to use the new timeout-proceed/skip-absent-bot behavior, and verify the 900/1200/3600 intervals remain intact.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is specific and matches the main change: timeout-based advisory-gate hardening for issue #1193.
Out of Scope Changes check	✅ Passed	All changes stay within the advisory gate and its tests; no unrelated features or files appear.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch dev-lead/issue-1193-20260714-0022

---

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.}

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[gemini-code-assist]

Code Review

This pull request introduces timeout-proceed windows to prevent pull requests from being permanently blocked when advisory bots are absent or fail to produce output. It defines timeout constants, implements a helper function _head_age_seconds to determine the age of the head commit, and integrates this check into the advisory review gate. Unit tests are also added to verify these timeout behaviors. Feedback on the changes suggests improving the portability of the date parsing in _head_age_seconds by adding a fallback for macOS/BSD systems.

[don-petry]

Dev-Lead — fix-reviews (applied)

Changes committed and pushed.

[don-petry]

Dev-Lead — waiting on PR blockers (intent: review-changes)

PR: #1197
No changes were committed, but the PR still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews). The retry cron will re-attempt automatically. Next attempt after: 2026-07-14T01:09:21Z

[don-petry]

Note

@don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically.
Next attempt after: 2026-07-14T01:09:21Z

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

scripts/lib/advisory-review-gate.sh (1)

368-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Missing BSD date fallback for latest_sub_at.

While the BSD date -u -jf fallback was successfully added for parsing head_time (line 368), the same fallback was omitted for latest_sub_at (line 379).

On macOS environments (which use BSD date), date -u -d will fail to parse the timestamp, leaving latest_sub_raw empty. This silently bypasses the conditional block at line 380, leaving time_since_last_sub as 0 and effectively disabling the quiescence timeout feature for local testing.

Apply the same cross-platform date parsing to latest_sub_raw to maintain consistency and ensure the quiescence timeout operates correctly across environments.

🛠️ Proposed fix

-      latest_sub_raw=$(date -u -d "$latest_sub_at" +%s 2>/dev/null) || latest_sub_raw=""
+      latest_sub_raw=$(date -u -d "$latest_sub_at" +%s 2>/dev/null) || latest_sub_raw=$(date -u -jf "%Y-%m-%dT%H:%M:%SZ" "$latest_sub_at" +%s 2>/dev/null) || latest_sub_raw=""

🤖 Prompt for 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.

In `@scripts/lib/advisory-review-gate.sh` around lines 368 - 380, Apply the same
GNU-then-BSD date parsing fallback used for head_time to latest_sub_raw when
parsing latest_sub_at, falling back to an empty value only if both date commands
fail. Preserve the existing latest_sub_at conditional and quiescence timeout
flow.

🤖 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.

Outside diff comments:
In `@scripts/lib/advisory-review-gate.sh`:
- Around line 368-380: Apply the same GNU-then-BSD date parsing fallback used
for head_time to latest_sub_raw when parsing latest_sub_at, falling back to an
empty value only if both date commands fail. Preserve the existing latest_sub_at
conditional and quiescence timeout flow.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 95cf89cb-6100-4eb5-9255-5f0e2c31b3e4

📥 Commits

Reviewing files that changed from the base of the PR and between 67ec50f and 2cb05ff.

📒 Files selected for processing (2)

• scripts/lib/advisory-review-gate.sh
• tests/dev-lead/unit/test_advisory_review_gate.bats

[don-petry]

Dev-Lead — fix-reviews (no-changes)

Agent reasoning

The background bats search returned empty — bats isn't installed in this environment. That's fine; the CI `bats` job already reported `success` in the provided status JSON, confirming all tests passed on the branch. No action needed.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[don-petry]

Dev-Lead — review-changes (no-changes)

No changes were needed for this PR.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[don-petry]

Dev-Lead — fix-bot-comment (no-changes)

Agent reasoning

**No code changes needed.** The SonarCloud bot found zero code issues and the Quality Gate passed. The code changes are clean and pass all available static analysis:
- SonarCloud: 0 new issues ✓
- ShellCheck: 0 errors ✓  
- Syntax/logic: Covered by passing unit-tests ✓
- Test coverage: Added comprehensive Bats test cases ✓
The dev-lead workflow failures appear to be infrastructure/orchestration issues outside the scope of the bot's code analysis.
**Summary:**
- Bot (SonarCloud) findings addressed: **0 issues** (Quality Gate passed)
- Code changes required: **None**
- Files: No modifications needed (all bot checks clean)

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: a722f05660220d75e8279bb4026ed5a961d78fee
Review mode: triage-approved (single reviewer)

Summary

Adds a head-age timeout (1200s) to the zero-output branch of the advisory review gate so PRs on repos where advisory bots are not enabled/installed (or during a bot outage) no longer block forever. Magic numbers are extracted into named constants (ADVISORY_HEAD_AGE_TIMEOUT_SEC / ADVISORY_QUIESCENCE_TIMEOUT_SEC), a BSD date parsing fallback is added, and 5 new bats tests cover the recent / timed-out / head-time-unavailable scenarios. Confirms the triage assessment: well-scoped, correct, and fully tested.

Linked issue analysis

Closes #1193 (durable hardening split from #1181). The issue asks the gate to timeout-proceed / skip absent advisory bots instead of re-driving indefinitely. The PR implements exactly this: the previously unconditional wait in the zero-bot-output branch now applies the same head-age timeout the partial-submission path uses, treating absent bots as missing reviews past the window. Graceful degradation when head age is undeterminable (GraphQL unreachable) is handled conservatively — the gate waits and the scheduled sweep retries. Substantively addressed.

Findings

No blocking findings.

• Correctness verified: _head_age_seconds returns empty output on any failure (missing committer date, unparseable timestamp) and the caller guards with [[ -n ... ]] before comparing, so the conservative-wait path is sound. Negative ages from clock skew fall through to waiting.
• Test helpers _make_mock_gh_dir_recent and _make_mock_gh_dir_no_head_time exist in the test file; bats passed at this head SHA.
• Constants replace the previous magic numbers (1200/600) consistently in both the new and pre-existing timeout paths.
• Nit (non-blocking): the shellcheck disable=SC2034 annotations on the two new constants are unnecessary since both are referenced later in the same file — harmless.
• Secret scan: run_secret_scanning MCP tool not available in this environment; the gitleaks CI check passed and no credential-like content appears in the diff.

CI status

All quality checks green: shellcheck, ShellCheck, bats, unit, unit-tests, Lint, CodeQL, Analyze (actions/python), SonarCloud (quality gate passed), Secret scan (gitleaks), AgentShield, Agent Security Scan, validate-* and stub/permission guards. CodeRabbit approved. The two CANCELLED dev-lead jobs (dispatch, ci-relay) are the dev-lead orchestration workflow's own superseded runs — not code-quality checks. The in-progress review/review check is this review. Zero unresolved review threads.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.

[donpetry-bot]

Automated review — APPROVED ✓

Risk: MEDIUM
Reviewed commit: a722f05660220d75e8279bb4026ed5a961d78fee
Review mode: triage-approved (single reviewer)

Summary

Hardens the advisory review gate so PRs no longer block forever when NO advisory bot ever produces output (bot not vendor-enabled, uninstalled, or during an outage). The zero-output branch now applies the same 1200s head-age timeout as the partial-submission path, treating absent bots as missing reviews rather than a permanent block. The previously hardcoded 1200s/600s windows are extracted into named constants (ADVISORY_HEAD_AGE_TIMEOUT_SEC / ADVISORY_QUIESCENCE_TIMEOUT_SEC), head-age computation is factored into a _head_age_seconds helper with a BSD date fallback, and the gate stays conservative (keeps waiting) when the head commit time cannot be determined. 5 bats tests cover the recent-head wait, timeout-proceed, unavailable-head-time conservative wait, constant naming, and the empty-state branch consulting head age.

Linked issue analysis

Closes #1193 (split from #1181), which asks to make the advisory gate timeout-proceed / skip absent bots so a per-repo enablement gap or bot outage becomes a missing review, not a permanent block, including graceful degradation when a bot is uninstalled entirely. The PR implements exactly this in the shared gate library (scripts/lib/advisory-review-gate.sh) used by the review engine, aligned with the existing timeout windows from #457/#458, with the requested graceful-degradation coverage (zero-output timeout-proceed test) plus a conservative-wait path when timing data is unavailable. Substantively addressed.

Findings

This is a re-review after a prior single-reviewer approval at dedf87c was dismissed by new commits. Delta since then: merges of main plus one review-changes commit that addressed the two Copilot inline comments (corrected the _head_age_seconds doc comment to say it is used only by the zero-output branch; tightened the bats grep to a fixed-string match on the call site) — both threads resolved. Gemini's BSD date portability suggestion is incorporated (date -u -jf fallback in both parse sites). All 3 inline review threads are resolved; CodeRabbit APPROVED the current head. No security concerns: no secrets, no injection vectors (committer dates from the GitHub API are passed as arguments, and an unparseable date degrades to the conservative wait path). Secret-scanning MCP tool unavailable in this environment; the gitleaks CI check passed. No blocking findings.

CI status

All validation checks green: shellcheck, ShellCheck, bats, unit, unit-tests, Lint, CodeQL (actions + python), Agent Security Scan, agent-shield, Secret scan (gitleaks), SonarCloud, holdout-guard, template-drift, prompt-coverage, and all structural guards SUCCESS. Dependency-audit ecosystem jobs SKIPPED (no matching ecosystems). The two CANCELLED dev-lead checks (dispatch, ci-relay) are agent-orchestration runs superseded by newer activity, not code validation. review/review IN_PROGRESS is this review run itself.

---

Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.