feat(workflow): add scheduled PR health example

[fanhongy]

Summary

• add a deterministic open-PR health scoring workflow with guarded comments, draft transitions, and allowlisted closure
• add exact biweekly CAO schedule examples with separate dry-run and explicitly authorized apply templates
• document installation, scoring, artifacts, safety controls, and schedule management
• add focused workflow, cadence, schedule, and enforcement tests

Safety

• scheduled runs default to dry-run
• unattended apply runs keep closure disabled
• all mutations re-fetch and re-score open PRs
• lifecycle markers must be authored by the authenticated workflow identity
• per-repository locking serializes concurrent runs

Validation

• cao workflow validate: valid
• uv run black --check ...: passed
• uv run isort --check-only ...: passed
• uv run mypy ... --ignore-missing-imports: passed
• uv run pytest test/examples/test_pr_health_workflow_example.py test/services/test_flow_service.py test/services/test_script_lint.py test/services/test_workflow_spec_service.py -q --no-cov: 131 passed
• uv run python scripts/validate_markdown_links.py: passed
• git diff --check: passed

[Copilot]

Pull request overview

This PR adds a new “PR health” workflow example to the CAO examples set, including deterministic scoring for open PRs, optional importance synthesis, and guarded enforcement actions, along with scheduled-flow templates and documentation explaining setup and safety controls.

Changes:

• Introduces a new deterministic PR-health workflow (pr_health.py) with snapshotting, scoring, artifact output, and apply-mode enforcement with safeguards.
• Adds a biweekly scheduling guard script plus two scheduled-flow templates (dry-run vs explicitly authorized apply).
• Adds documentation and tests covering example validation, cadence guarding, and schedule/enforcement behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
test/examples/test_pr_health_workflow_example.py	Adds tests to validate the new PR-health example script and scheduled flow templates.
examples/workflows/pr-health/README.md	Documents installation, scoring model, safety controls, artifacts, and scheduling guidance for the example.
examples/workflows/pr-health/pr-health-biweekly.md	Adds a scheduled-flow template intended for non-mutating biweekly dry runs.
examples/workflows/pr-health/pr-health-biweekly-apply.md	Adds a scheduled-flow template intended for explicitly authorized apply-mode runs.
examples/workflows/pr-health/pr_health.py	Implements the deterministic scoring workflow, persistence, reporting, optional analysis step, and guarded enforcement.
examples/workflows/pr-health/pr_health_biweekly_guard.py	Implements a deterministic date-based gate to achieve an exact 14-day cadence.
docs/workflows.md	Adds a “see also” link to the new PR-health workflow examples directory.
docs/flows.md	Adds a short section pointing readers to the PR-health workflow scheduling example.

Suppressed comments (1)

test/examples/test_pr_health_workflow_example.py:72

• Same issue as the dry-run schedule test: day-of-week 0 is Sunday in crontab format. Update the expected schedule to Monday (1) to match the intended cadence and the flow files.

    assert metadata["schedule"] == "0 9 * * 0"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[gutosantos82]

PR Review: #529 — feat(workflow): add scheduled PR health example

Summary

Adds examples/workflows/pr-health/: a 1309-line deterministic open-PR health scorer with guarded comments, draft transitions, and allowlisted closure, plus biweekly scheduled-flow templates (dry-run and apply), README, and tests. The engineering quality is high — no command injection, marker authenticity is genuinely enforceable via viewerDidAuthor, closure is double-gated, and doc↔code consistency is unusually strong. However, two behavioral defects contradict the PR's own documented safety model (duplicate notifications; cadence-coupled lifecycle), and the enforcement paths that actually mutate GitHub are almost entirely untested. Since this ships as a copyable reference policy that comments on, drafts, and closes other people's PRs, those should be fixed before merge.

Blocking (must fix before merge)

• [correctness] examples/workflows/pr-health/pr_health.py:692, ~645 — Cross-run idempotency is ineffective for warn_owner / propose_draft. Markers embed per-run as_of and score, and _has_marker_for_action dedupes by exact string match (only escalate_protected_pr uses a loose prefix match), so a prior run's marker never matches and the same warning comment is posted again. Reproduced: two warn_owner runs before the 7-day draft threshold post the identical warning twice. This directly contradicts the safety claim that markers make actions idempotent — the workflow spams PR owners. Fix: dedupe on stage presence (as escalation already does), not on score/as_of.
• [tests] test/examples/test_pr_health_workflow_example.py:83-105 — The mutation/enforcement guards are essentially untested. The single _apply_recommendations test covers only the skipped_not_open branch. Untested: already_applied idempotency, skipped_live_drift revalidation, the skipped_closure_not_allowlisted allowlist gate, and every actual mutation path (drafted_and_commented, closed_and_commented, commented). These are exactly the branches whose failure causes an unwanted comment, draft, or closure. Add monkeypatched per-branch tests asserting both result status and the recorded gh commands.

Important (should fix)

• [correctness] pr_health.py:406, 483-500 — The enforcement lifecycle is only correct at exactly the 14-day cadence. _latest_marker returns only the newest marker; once a PR is drafted, a later warning-stage marker shadows the draft marker, so the closure branch (stage=="draft" and marker_age>=14) is never re-evaluated. Reproduced at a 7-day cadence: the PR oscillates warn_owner ↔ owner-notification forever and never reaches propose_close. The README's own examples pass arbitrary as_of for manual runs, so this is a reachable state, not a theoretical one. Fix: drive lifecycle progression off persisted state (which already carries streak/as_of) or the earliest unanswered stage, not the newest comment marker.
• [correctness] pr_health.py:444 — One stale per-PR state entry aborts the entire run: _observation_streak raises ValueError("as_of predates the persisted PR health state") inside the per-PR loop, killing scoring for all other PRs (backfill, reopened PR with old state, clock skew). Prefer per-PR skip/normalize.
• [tests] pr_health.py (_run_self_tests) — Scoring is validated almost entirely by an in-product assert bundle that runs inside _run_locked on every production run and is stripped under python -O; meanwhile --cov=src measures nothing under examples/, so the passing suite gives false coverage confidence for the 1309-line file. Move the self-tests into test/ (or at minimum out of the production run path).
• [consistency] pr-health-biweekly.md / pr-health-biweekly-apply.md — Both templates share the same guard, schedule, and mode-agnostic run_id/snapshot_id (pr-health-biweekly-<date> / scheduled-<date>). If both flows are registered, the second to run on a due Monday hits the manifest guard (snapshot_id already exists with different inputs), making dry-run and apply silently mutually exclusive per day. Differentiate identifiers by mode, or state "do not register both" in the README schedule section.
• [security] pr_health.py (SAFE_ID_RE / _run_locked validation) — SAFE_ID_RE = ^[A-Za-z0-9._-]+$ accepts .., so snapshot_id=".." resolves artifact_dir to the state root (no escape beyond it, since / is rejected). Reject . and .. explicitly.

Nits (optional)

• [correctness] pr_health_biweekly_guard.py / pr_health.py — Timezone mixing: the guard derives as_of from date.today() (server-local) while scoring compares gh UTC timestamps; near midnight, 7/14/21-day threshold crossings can shift by a day.
• [correctness] pr_health.py (_latest_marker, _latest_commit_at) — Marker detection and idle math assume gh pr view returns complete comment/commit history; GraphQL-backed fields are commonly capped. Add a defensive note or cap check, since enforcement mutates GitHub based on these.
• [security] pr-health-biweekly-apply.md / README.md — The scheduled apply mode is a standing unattended write-grant under the operator's gh identity (drafting contributors' PRs has real social impact). Mitigations are solid (closure double-gated off, live re-score before mutation), but make the apply-template warning harder to skim and suggest trialing against a fork first; also add a one-line note that the LLM importance synthesis reads untrusted PR text (advisory-only, cannot change scores/actions).
• [consistency] pr_health.py:698, 73-76 — stage=closed marker is emitted by _action_marker but MARKER_RE only parses warning|draft; write-only vocabulary, harmless but worth aligning.
• [conventions] PR title — scope workflow (singular) vs the new examples/workflows/ directory (plural); cosmetic.

Tests

The new test file is a thin smoke/wiring layer (~5 test functions): flow static validation, one cadence check at 4 dates, script lint, running the in-product self-tests, and one enforcement skip branch. The "131 passed" claim is real but mostly pre-existing suites (test_flow_service.py, test_script_lint.py, test_workflow_spec_service.py) swept in by the command. The bespoke calendar engine (_is_leap, _date_ordinal, _days_between, _validate_as_of) — where month-boundary/rollover bugs would hide — is only touched by two asserts, and the fcntl.flock per-repo locking (a stated safety control) has no test at all. Minimum before merge: per-branch enforcement tests (see Blocking), calendar boundary + invalid-date cases, a lock acquire/release/contention test, and direct _comment_body marker assertions for warn/draft/close.

Verification

Verifier skipped (no changes under src/cli_agent_orchestrator/ — examples/docs/tests only). The correctness reviewer did run the shipped test file in the PR worktree (all 5 pass), live-verified the APScheduler 0 9 * * 0 = Monday claim, and reproduced both the duplicate-comment and cadence-oscillation defects by simulation; the security reviewer confirmed no shell=True anywhere and that forged markers are rejected.

Verdict

Request changes — fix the marker idempotency (duplicate owner comments) and cadence-coupled lifecycle defects that contradict the documented safety model, and add tests for the GitHub-mutating enforcement branches, before this lands as a reference policy.

[fanhongy]

Thanks for the review — all 12 items addressed in 5077414. Every fix below carries a test that was verified to redden when the fix is reverted.

Blocking

1. Marker idempotency (pr_health.py) — _has_exact_marker is gone. Markers embed the run's score and as_of, so exact-text comparison could never match a prior run's marker and the same notification was re-posted on every run. Dedupe is now keyed on the marker stage via a new ACTION_STAGES map (warn_owner→warning, propose_draft/second_owner_notification→draft, propose_close→closed, escalate_protected_pr→escalation), and _has_marker_for_stage scans only viewerDidAuthor bodies. Each stage now notifies at most once, ever.

2. Untested enforcement branches — the test file went 105 → ~1250 lines / 87 tests. _apply_recommendations now has a per-branch test for every outcome: skipped_not_open, already_applied, skipped_live_drift, skipped_closure_not_allowlisted, closed_and_commented, drafted_and_commented, commented, error, non-actionable filtering, and incremental journal writes — all against a captured gh command list, so the exact argv of each mutation is asserted.

Important

3. Cadence-coupled lifecycle — _latest_marker (newest-wins) is now _lifecycle_marker, selecting furthest-stage-then-earliest. A later warning-stage comment could previously shadow an existing draft marker, restarting the draft grace period on every run so closure was never reached. Added a hold branch in _recommend_action (await_owner_deadline) so a PR inside an unexpired grace period doesn't walk back down the ladder. test_lifecycle_reaches_closure_at_a_seven_day_cadence drives 8 weekly runs and asserts warn→draft→close in order, exactly once each.

4. One stale state entry aborting the run — _observation_streak no longer raises ValueError. A stale or unparsable persisted as_of (backfill, clock skew, reopened PR) restarts that PR's streak, which is the conservative direction: one delayed warning instead of no scoring for any other PR.

5. In-product self-tests — _run_self_tests deleted (179 lines) and its call removed. The module docstring now points at test/examples/test_pr_health_workflow_example.py, whose docstring records that it is the sole coverage because examples/ sits outside --cov=src.

6. Dry-run/apply identifier collision — the guard now emits mode-qualified run_id_dry_run/snapshot_id_dry_run/run_id_apply/snapshot_id_apply, and each flow template consumes its own pair, so both schedules can be registered and share a due date. test_both_flows_can_be_registered_without_colliding renders both prompts from the guard's real subprocess stdout, so a dropped guard variable or a re-collided id fails the test.

7. SAFE_ID_RE accepting .. — added a RESERVED_IDS = {".", ".."} rejection alongside the pattern check, with _run_gh stubbed to assert no gh call happens before validation.

Nits

• Timezone — the guard derives as_of from datetime.now(timezone.utc).date(); a local date shifted the 7/14/21-day crossings by a day near midnight. Test uses a date subclass sentinel so it also reddens on a UTC machine.
• GraphQL truncation — _latest_commit_at documents the page-size caveat.
• Apply-mode grant — both the apply flow template and the README lead with a blockquoted standing-unattended-write-grant warning ("drafting a contributor's PR has real social impact; trial against a fork you own first"). README also notes the reviewer agent reads untrusted PR text and is advisory prose only.
• stage=closed vocabulary — now read as well as written: it means a prior close was reverted, and the ladder restarts from observation.
• PR title scope — left as-is; retitling rewrites the commit message for a cosmetic gain.

Additional fix found while testing

commented_existing_draft was unreachable: a draft PR scores second_owner_notification, never propose_draft. Removed the branch and dropped it from the mutated_github status set; replaced its test with test_already_draft_pr_gets_a_second_notification_not_a_draft_call.

Tests requested

All four landed: per-branch enforcement tests (above); calendar boundaries (test_days_between_boundaries with 1900/2000/leap/rollover/clamp, test_date_ordinal_agrees_with_stdlib_across_a_dense_range, 12 invalid-date cases); fcntl.flock acquire/release/contention via LOCK_EX | LOCK_NB + pytest.raises(BlockingIOError), including release on a failed run; and direct _comment_body marker assertions parametrized over all four actions with a mismatched score/date.

Verification

• test/examples/: 117 passed.
• CI-scope suite with these changes: 80 failed, 5590 passed, 30 skipped, 13 deselected, 1 xfailed. Stashed baseline: 80 failed, 5508 passed — the same 80 pre-existing environment-only failures (test_otel_init.py, test_wiki_lint.py, agui/test_run_plane_heartbeat.py), +82 net new passing.
• black, isort, markdown-link validation clean. mypy src/ unchanged (nothing here touches src/).
• Mutation matrix — reverting each fix reddens tests: exact-text dedupe → 6 red; newest-wins marker → 2; newest-wins + no hold branch → 4; stale-state raise → 2; reserved-id check → 2; colliding guard ids → 1; dropped guard variable → 2; guard local date → 1.

Note for maintainers: ci.yml triggers on main only, so the checks on this PR are CodeQL + secret-scan and no test job runs here. The suite numbers above are from a local run at CI's exact command and scope.

[gutosantos82]

PR Review: #529 — feat(workflow): add scheduled PR health example

Summary

Re-review at head 5077414 following the previous request-changes review. The author's point-by-point response holds up under independent verification: all 12 prior items (marker idempotency, cadence-coupled lifecycle, stale-state abort, untested enforcement branches, self-test removal, dry-run/apply id collision, .. reserved-id traversal, and the nits) are genuinely fixed at this head, and the mutation-matrix claims were spot-checked exactly — reverting each fix reddens the stated tests. The test file grew to 87 passing tests with per-branch gh-argv assertions; this is now a well-defended reference example. Two new correctness defects surfaced in the reworked lifecycle logic, however, and both break the README's documented escalation contract in reachable states: a reopened PR can never be re-notified in apply mode (dry-run and apply disagree), and a PR scoring exactly 0 never advances past observation. Both fail in the conservative direction (no wrongful mutation, no spam) and both fixes are small — one more round should close this out.

Important (should fix)

• [correctness] examples/workflows/pr-health/pr_health.py:938, :849 (_has_marker_for_stage / _apply_recommendations) — Reopened PRs can never be re-notified at any stage in apply mode. The new dedup asks "has this identity EVER authored a marker for this stage" with no lifecycle-epoch scoping. After a reverted close, the warning/draft/closed markers from the first lifecycle persist; _recommend_action correctly restarts the ladder (a closed marker falls through to observation, as documented), but _has_marker_for_action finds the stale first-life marker and returns already_applied — forever. Reproduced by simulation over 8 weekly runs on a reopened, still-unhealthy PR: every run recommends warn_owner, yet enforcement is permanently suppressed, so the dry-run report contradicts apply behavior and escalation/closure are unreachable. Fix: scope stage dedup to markers newer than the current lifecycle epoch (e.g. ignore markers at or before the latest closed marker).
• [correctness] pr_health.py:527 (_observation_streak) — int(previous.get("last_score") or 100) treats a persisted last_score of 0 as absent (falsy) and substitutes 100, so prior_qualified is false and the streak resets to 1 on every run. warn_owner requires streak ≥ 2, so a fully-broken PR scoring 0 — precisely the abandoned-PR case this workflow exists to manage — returns observe_again indefinitely and is never warned, drafted, or closed. Reproduced across 12 simulated runs at every cadence (score 0 → observe_again forever; score 50 advances normally). Fix: previous.get("last_score", 100) or an explicit is None check (same pattern risk anywhere or-defaulting is applied to scores/streaks).

Nits (optional)

• [correctness] pr_health.py:558-567 — A PR warned at score 51–59 holds at await_owner_deadline indefinitely: warn_owner fires below 60 but propose_draft requires ≤ 50, so the 51–59 band never progresses nor re-warns. Looks like a designed threshold gap and it doesn't spam, but it's worth a one-line README note so operators aren't surprised.
• [conventions] CHANGELOG.md — No ### Added entry. Example additions historically get one (#377, #253, #109, #11); not mandated by CONTRIBUTING, but a one-liner would match the established pattern.
• [consistency] docs/flows.md:120-122 — "Scheduled runs default to non-mutating dry-run mode" can mislead a skimmer since an apply template also ships and mutates GitHub; clarify that the dry-run template is non-mutating while the separate, explicitly-authorized apply template mutates.
• [consistency] docs/workflows.md:246 / docs/flows.md:118 — The new pr-health links use parent-relative ../examples/... while sibling bullets use doc-relative paths. The targets resolve correctly (verified); style-only.
• [security] pr_health.py:460, :845 — Marker idempotency assumes the same gh identity across runs (viewerDidAuthor); switching the operator identity would re-post stage notifications once. A robustness assumption worth a README line, not a vulnerability.

Tests

The prior blocking tests item is fully resolved. The suite is now 1184 lines / 56 test functions expanding to 87 collected tests — all pass in 1.11 s in the PR worktree. Every _apply_recommendations branch has a dedicated test asserting both result status and the captured gh argv; calendar boundaries (1900/2000/leap/rollover/clamp, 12 invalid-date cases, ordinal cross-checked against stdlib), fcntl.flock acquire/release/contention (LOCK_EX|LOCK_NB + BlockingIOError, release-on-failure), and _comment_body marker assertions across all four actions are all present and non-vacuous. Three of the author's "reverting reddens" claims were independently re-run and matched exactly (reserved-id → 2 red, exact-text dedupe → 6 red, newest-wins marker → 2 red). One correction to the author's comment: the claim that "ci.yml triggers on main only, so no test job runs here" is inaccurate — pull_request: branches: [main] filters on the PR's base branch, so the full unit-test matrix (3.10/3.11/3.12) and markdown-link validation do run on this PR; CI is a real gate here. Remaining gaps are low-risk: the live gh wire path and GraphQL page-truncation behavior are untested (acceptable for an offline example suite), and the two new Important findings above need accompanying tests.

Verification

Verifier skipped (no changes under src/cli_agent_orchestrator/ — examples/docs/tests only). The tests reviewer ran the shipped suite in the PR worktree (87 passed) and re-executed three mutation-matrix claims with exact matches. The correctness reviewer reproduced both new defects by multi-run simulation (reopened-PR suppression over 8 weekly runs; score-0 stagnation over 12 runs at multiple cadences) and verified the five prior behavioral fixes the same way. The security reviewer confirmed validation-before-network ordering for reserved ids, list-form subprocess argv throughout (no shell=True), viewerDidAuthor forgery rejection, and that the new await_owner_deadline branch adds no mutation surface. The conventions reviewer verified black/isort/markdown-links clean with the repo's own tooling and that no dependencies, lockfile, or __pycache__ artifacts entered the diff.

Verdict

Request changes — narrow scope this round: fix the reopened-PR enforcement suppression (epoch-scope the stage dedup) and the score-0 streak reset (or-defaulting on a falsy score), each with a test; everything raised previously is verified fixed and the PR is otherwise in strong shape.