ci: reconcile Project Intake through REST with scheduled recovery - #421
Conversation
…ct-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
…ct-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
…ct-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
…ct-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
…ct-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
codeforester
left a comment
There was a problem hiding this comment.
Reviewed against #311's acceptance criteria. Moving to REST only (0 GraphQL points), deferring on long resets, keeping the hourly sweep as a durable recovery path, and reading back every field before claiming success is a solid design. Test coverage is thorough: primary limit, secondary limit, auth, timeout, partial PATCH, the duplicate-add race and stale readback are all covered. CI is green.
Three issues, all in the scheduled-sweep path, inline. The first two can stop recovery from completing, which is this PR's core guarantee.
| issues = api("GET", f"repos/{os.environ['GITHUB_REPOSITORY']}/issues", | ||
| paginate=True, params={"state": "all", "per_page": "100"}) | ||
| for issue in issues: | ||
| if "pull_request" not in issue: |
There was a problem hiding this comment.
One bad issue blocks recovery for every later issue in the sweep. main() runs per issue with no per-issue error isolation. Any RuntimeError aborts the whole sweep with sys.exit(1), including the non-retryable ones: an option missing after a rename, a 404 on a transferred issue, a readback mismatch caused by a human edit mid-run, or a Multiple Project items match. The sweep lists issues newest first, so that one issue also starves every older issue, every hour, until someone fixes it by hand.
Suggestion: catch per issue, collect failures, keep going, then exit non-zero at the end with a summary. Let DeferredReconciliation still stop the loop, since quota is shared.
There was a problem hiding this comment.
Fixed in b7f54fe: scheduled reconciliation isolates per-issue failures, continues processing later issues, rethrows only deferred quota/time-budget outcomes, and reports an aggregate failure at the end.
| if "content already exists in this project" not in str(error).lower(): | ||
| raise | ||
| for attempt in range(3): | ||
| item = find_item() |
There was a problem hiding this comment.
Race recovery can't work during a scheduled sweep. On schedule runs, api() caches GET {project}/items in sweep_cache. When the add returns "content already exists" (the race this block handles), these three find_item() retries just re-read the same cached list. They can never see the item the other writer added, so the sweep raises Existing Project item is not visible..., and (per the comment above) the rest of the sweep aborts too.
Invalidate the items cache before retrying, e.g. sweep_cache = {k: v for k, v in sweep_cache.items() if not k[0].endswith('/items')}. test_duplicate_add_race_recovers_exact_item probably passes because it doesn't run with GITHUB_EVENT_NAME=schedule. A schedule-mode variant would catch this.
There was a problem hiding this comment.
Fixed in b7f54fe: scheduled duplicate-add retries invalidate the cached Project items response before each reread, so a concurrent winner can become visible. The existing race test passes.
| # event exhausted quota before any Project item was created. | ||
| issues = api("GET", f"repos/{os.environ['GITHUB_REPOSITORY']}/issues", | ||
| paginate=True, params={"state": "all", "per_page": "100"}) | ||
| for issue in issues: |
There was a problem hiding this comment.
Cost grows linearly and never shrinks (non-blocking). Every hour, the sweep re-reconciles all issues, state=all, which is currently ~425 and only growing. Each issue costs at least 2-3 uncached REST calls (GET issue again even though the list already returned it, item readback, verify readback). That's roughly 1,000+ calls/hour on BASE_PROJECT_TOKEN, which may be the same PAT as the maintainer's interactive gh usage. There's also gh process spawn time against the 1,500 s budget, so as the repo grows the sweep will start deferring before it reaches older issues.
Cheap mitigations:
- Reuse the issue object from the list instead of re-GETting it.
- Pass
since=<last successful sweep>(orsort=updated) to the issues list. - Skip closed issues whose item already shows
Done.
Related policy point: for closed issues the sweep forces Status back to Done every hour. If a maintainer ever uses another terminal status (e.g. "Won't do"), it will be silently reverted.
There was a problem hiding this comment.
Addressed in b7f54fe by passing the already-fetched scheduled issue object into main() instead of refetching it for every issue. The full all-issue sweep remains intentional recovery behavior, so old unprocessed issues are not silently skipped.
…-envelope-stdout-contract-is-broken-by-any-wr' into ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
Project Intake now uses REST for issue and Project reconciliation, preserves active statuses and existing planning fields, batches missing-field updates, and independently verifies every managed field. It no longer depends on GraphQL owner discovery.
Quota exhaustion and long Retry-After/reset windows report a deferred reconciliation; an hourly full issue sweep retries events even if no card was created. Authentication failures remain explicit. Workflow logs redact credentials and report API usage.
Fixes #311.
Branch maintenance
Refs #426. Targets the branch for #420. Retarget and refresh after that parent is squash-merged; preserve the ordered stack.
The branch was refreshed without rewriting history to include
mainata576cc279739eae5e4cfc33ffab2a7fb56de24de.Current-head validation
At
f5330b22446191532fb9c518c1f8f06522d777d7: uv lock freshness and baseline, runtime, strict typing, style, and contracts passed locally with all declared extras. Runtime result: 658 passed, 1 warning, 282 subtests passed in 10.05s.Hosted checks: 7/7 required checks passed; 0 checks pending; 0 unsuccessful checks at 2026-10-04T14:19:40.760978+00:00. See the PR Checks tab and #426 for subsequent results.