fix(checkup): bounded PR-head freshness lease for pdd checkup --pr (#1116)

[Serhan-Asad]

Summary

Closes #1116. pdd checkup --pr now auto-reruns up to twice when the remote PR head advances mid-checkup, instead of failing closed and forcing operator manual rerun.

• What changed: rename-and-wrap pattern around run_agentic_checkup_orchestrator adds three freshness checkpoints (A after Step 7 gate, B on rebase-related push failures, C after no-change push outcomes). Each checkpoint refetches PR metadata and restarts the inner orchestrator from scratch when the head moved, capped at MAX_PR_HEAD_REFRESHES = 2. The restart counter persists in a sidecar file outside workflow state so a Ctrl-C + manual resume cannot bypass the bound.
• What did NOT change: _commit_and_push_if_changed return shape (still (bool, str)), pdd/checkup_review_loop.py, pdd/agentic_e2e_fix_orchestrator.py (_run_final_checkup_on_pr stays fail-closed as the outer safety net), README, CHANGELOG. No force-push behavior added.
• --no-fix mode stays fail-closed via the existing _finalize SHA-mismatch downgrade; it does NOT consume restart budget.

Why this design (vs PR #1121)

The pre-existing PR #1121 wraps the entire orchestrator in while True:, changes the push helper's return-tuple shape, and touches checkup_review_loop / README / CHANGELOG (2010+/1732- LOC). This PR uses a minimal rename-and-wrap (~1265 lines including 863 lines of tests):

1. The 1168-line inner body gets a one-line def rename to _run_agentic_checkup_orchestrator_inner.
2. A new ~130-line public wrapper holds the bounded restart loop and catches an internal _PRHeadAdvancedRestart exception.
3. Two checkpoints inside the inner raise on stale-head detection.
4. A third checkpoint guards the no-change-push window between Checkpoint A and the canonical report.

Codex (gpt-5.1-codex) reviewed 4 rounds and signed off (VERDICT: ship).

Commits (4)

• f7ea6cf6f feat(checkup): bounded PR-head freshness lease for pdd checkup --pr
• df003d79b fix(checkup): codex round-1 — close Checkpoint B/C gaps for Make pdd checkup --pr rerun when PR head advances mid-checkup #1116
• 11535aed5 fix(checkup): codex round-2 — explicit state clear + rebase-reset classifier
• ec23e9a5b docs(checkup): sync prompt source for Make pdd checkup --pr rerun when PR head advances mid-checkup #1116 freshness lease

Test plan

• 12 new tests in TestPrHeadAdvanceAutoRerun covering all three checkpoints, budget exhaustion, sidecar persistence, sidecar resume-from-disk, --no-fix no-consume, auth-failure no-trigger, empty-SHA fail-degrade.
• LC_ALL=C pytest tests/test_agentic_checkup_orchestrator.py tests/test_checkup_pr_mode.py -m "not integration and not e2e and not real" --timeout=60 → 180/180 passing.
• CI greens on the full suite.
• Manual smoke: drive a pdd checkup --pr run while an external commit lands on the PR branch mid-run; verify the rerun fires once and a clean second pass completes.

Notes

• Supersedes PR Make pdd checkup --pr rerun when PR head advances mid-checkup #1121 (over-scoped — leave it for the owner to close or transplant any useful test coverage).
• Prompt source pdd/prompts/agentic_checkup_orchestrator_python.prompt updated for parity so a future pdd sync cannot regenerate without the fix.
• Sidecar path is .pdd/checkup-pr-{pr_number}/pr_head_refreshes — outside .pdd/checkup-state so clear_workflow_state never deletes the counter.

🤖 Generated with Claude Code

[greptile-apps]

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

[Serhan-Asad]

Step 7/8: Verification & Final Report (Iteration 1)

Test Results After Fixes

• Total: 460 tests (checkup-scoped) + 19 (focused new/modified)
• Passed: 460 (+ 1 skipped) / 19
• Failed: 0
• Previously failing, now passing: 0 (no failures in prior steps)
• New failures: 0

Build Status

Pass — python -m compileall pdd succeeds; package imports cleanly.

Overall Status

All clear — Steps 6.1 (artifact removal), 6.2 (artifact-hygiene regression), 6.3 (freshness-lease integration tests) and the test_agentic_crash.py chdir fix all verified. No stray new_file.txt at repo root.

Issue Alignment

issue_aligned: true — The PR's commit history (feat(checkup): bounded PR-head freshness lease, codex round-1/round-2 fixes, prompt-source sync) substantively implements the bounded PR-head freshness lease requested by #1116; the checkup-iteration changes here (artifact hygiene + cross-module integration tests pinning the 2-tuple contract, sidecar isolation from clear_workflow_state, and the exhaustion-message SHA chain) reinforce the same mechanism without altering its contract.

Issues Summary

Severity	Category	Module	Description	Fixed
low	artifact	repo root	Stray empty new_file.txt left by an earlier checkup run	Yes
low	test_failure	tests/test_agentic_crash.py	test_file_system_changes_detected could write new_file.txt into the real repo root	Yes (chdir(tmp_path) in test)
low	test_failure	tests	No regression test asserted "no stray top-level artifacts" invariant	Yes (test_checkup_artifact_hygiene.py)
low	interface_mismatch	tests	No integration coverage of the orchestrator↔review-loop 2-tuple contract and sidecar isolation	Yes (test_checkup_freshness_lease_integration.py, 4 tests)

---

Checkup complete.

All Issues Fixed

{
  "success": true,
  "message": "PR #1134 substantively implements the bounded PR-head freshness lease for `pdd checkup --pr` requested by #1116 (capture-then-recheck PR head SHA, bounded MAX_PR_HEAD_REFRESHES reruns with sidecar counter, rebase-conflict classification, no force-push). Step 7 verification: build passes; 460 checkup-scoped tests pass (1 skipped); 19 focused tests (4 new freshness-lease integration tests + 2 new artifact-hygiene tests + 13 existing/modified agentic_crash tests) pass. Iteration-1 fixes removed a stray `new_file.txt` artifact at repo root, fixed `test_file_system_changes_detected` to chdir into tmp_path so it can't recreate that artifact, and added regression coverage pinning both the no-stray-artifact invariant and the orchestrator↔review-loop 2-tuple contract / sidecar isolation / exhaustion-SHA-chain behavior.",
  "tech_stack": ["python", "pytest", "click", "pydantic"],
  "issue_aligned": true,
  "issues": [
    {
      "module": "repo_root",
      "file": "new_file.txt",
      "severity": "low",
      "category": "artifact",
      "description": "Stray empty new_file.txt left at repo root by an earlier checkup run on a different issue.",
      "fixed": true,
      "fix_description": "Removed the untracked empty file in Step 6.1."
    },
    {
      "module": "tests",
      "file": "tests/test_agentic_crash.py",
      "severity": "low",
      "category": "test_failure",
      "description": "test_file_system_changes_detected used Path.cwd() (real repo root) for the mocked agent side effect that writes new_file.txt, which could leave a stray artifact at the repo root if the test was run from the repo.",
      "fixed": true,
      "fix_description": "Added monkeypatch.chdir(tmp_path) so the mocked side effect writes into the test sandbox; documented the rationale referencing issue #1116."
    },
    {
      "module": "tests",
      "file": "tests/test_checkup_artifact_hygiene.py",
      "severity": "low",
      "category": "test_failure",
      "description": "No regression test pinned the 'no stray new_file.txt / empty top-level .txt artifacts at repo root' invariant.",
      "fixed": true,
      "fix_description": "Added 2 parametrized assertions guarding against stray artifacts at the repo root."
    },
    {
      "module": "tests",
      "file": "tests/test_checkup_freshness_lease_integration.py",
      "severity": "low",
      "category": "interface_mismatch",
      "description": "No cross-module integration coverage of the orchestrator↔review-loop 2-tuple contract for _commit_and_push_if_changed, sidecar refresh-counter isolation from clear_workflow_state, or the budget-exhaustion message's observed-SHA chain.",
      "fixed": true,
      "fix_description": "Added 4 integration tests covering the 2-tuple contract, helper-wrapper equivalence, sidecar isolation from workflow-state-dir clearing, and the SHA-chain content of the exhaustion error message."
    }
  ],
  "changed_files": [
    "tests/test_agentic_crash.py",
    "tests/test_checkup_artifact_hygiene.py",
    "tests/test_checkup_freshness_lease_integration.py"
  ]
}

PR Push Status

Pushed fixes to PR branch.