Skip to content

feat(github): issue and status reads use gh-ci once its token grants them - #6325

Open
schickling-assistant wants to merge 2 commits into
mainfrom
schickling-assistant/gh-ci-issue-status-reads
Open

schickling-assistant wants to merge 2 commits into
mainfrom
schickling-assistant/gh-ci-issue-status-reads

Conversation

@schickling-assistant

@schickling-assistant schickling-assistant commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Why

GitHub observers read through ROOT/gh-ci installation tokens when the operator publishes them (dotfiles#9097 enables that on dev3/dev5), but two read classes still use the shared schickling-assistant PAT:

  • issue/comment listings and issue identity GraphQL route to github-ledger, which is not published on any host (st_github_auth_fallback{github_app="github-ledger",reason=~"missing|no-installation"} on dev3/dev5/mbp2025);
  • legacy commit statuses are PAT-only (reason="statuses"), one extra GET per distinct PR head per GraphQL read.

That PAT ran to 0 core requests left on 2026-11-10.

What

  • ReadSource keeps the sidecar's permissions; grants(permission).
  • select(..., "github-ledger", ...): if no ledger installation is published (missing/no-installation) and gh-ci grants issues read, use gh-ci (no fallback recorded). Expired/unreadable ledger still falls back.
  • Commit-status URLs route to gh-ci when it grants statuses read; PR detail reads skip the per-head legacy status GET in that case (the rollup already carries StatusContext).
  • Docs: docs/st3/github-observer-admission.md.

Behaviour is unchanged until the gh-ci App installations grant Issues/Commit statuses read (a person step; the dotfiles publisher already requests them once granted). Existing 403 "Resource not accessible by integration" replay to the PAT still applies.

Tests

issue_reads_use_gh_ci_only_when_it_grants_issues_and_no_ledger_is_published, status_reads_use_gh_ci_once_it_grants_statuses (crates/fractal-github-base/src/github_app_read.rs). Burst: https://github.com/compoundingtech/fractal/actions/runs/42910452423 (cargo test -p fractal-github-base --lib).

Pushed with ST_SKIP_EVIDENCE=1 (no evidence signer on this machine).

Why statuses matter for the PAT

Without statuses read, PR detail reads under an installation add one PAT combined-status GET per distinct head. They are conditional (pat_legacy_status_reads_revalidate_with_their_etag; a live 304 is free), but a cold cache after every daemon start costs one charged GET per open PR: 379 + 304 for dotfiles + fractal (a dev3 canary spent ~650 PAT requests in 6 min). With the statuses grant, this PR skips those GETs entirely.

…them

When no github-ledger installation is published for an owner and the gh-ci
sidecar grants issues read, issue/comment reads use the gh-ci installation
instead of the PAT. When gh-ci grants statuses read, commit-status reads route
to gh-ci and PR detail reads skip the per-head legacy status GET, since the
GraphQL rollup already carries StatusContext entries. Behaviour is unchanged
until the App's installations grant those permissions.
@schickling
schickling force-pushed the schickling-assistant/gh-ci-issue-status-reads branch from c65e095 to b97483b Compare November 10, 2026 22:08
Without statuses read, an installation-selected PR detail read adds one PAT
combined-status GET per distinct head. Cover that a repeat read is conditional
(304, free of primary quota), so only a new head or a cold cache costs the PAT.

This branch was previously deployed

1 inactive deployment
evidence-signing — b93317c4 Deployed Nov 10, 2026 by schickling-assistant via typescript-client #19717
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.

2 participants