Skip to content

ci: reconcile Project Intake through REST with scheduled recovery - #421

Open
codeforester wants to merge 12 commits into
bug/379-20261003-bug-json-single-envelope-stdout-contract-is-broken-by-any-wrfrom
ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti
Open

codeforester wants to merge 12 commits into
bug/379-20261003-bug-json-single-envelope-stdout-contract-is-broken-by-any-wrfrom
ci/311-20261003-v1-0-make-project-intake-resilient-to-graphql-quota-exhausti

Conversation

@codeforester

@codeforester codeforester commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

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 main at a576cc279739eae5e4cfc33ffab2a7fb56de24de.

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.

…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 codeforester left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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> (or sort=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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant