You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Real-provider E2E is architecturally sound (production provider does Act, an independent git-CLI verifier does Assert — never the service reading back its own write), but the current design does not treat GitHub API request budget as a governed test resource. As currently structured, provider E2E is at real risk of tripping GitHub's secondary rate limit (content-generating mutations, guideline ~80/min, ~500/hour), and the CI retry policy actively makes that worse rather than better.
This is not "the E2E tests are wrong." It's that full-contract regression coverage is being run on every PR with a retry policy tuned for transient network blips, not for rate-limiting.
Problems identified
A single pushFile() is not one API call.GitHubService.pushFile() calls listFilesDetailed() (recursive tree REST GET), then pushBatch() → commitOnBranch(), which itself does a GraphQL branch-HEAD query plus a createCommitOnBranch mutation — ~3 requests minimum per push, before commitOnBranch()'s own up-to-3x stale-HEAD retry.
The E2E suites are mutation-heavy by design.github.e2e.test.ts covers create/read/update/delete/batch/rename/symlink/GraphQL-error/concurrent-write, where each scenario like "update" is itself create+update, "delete" is create+delete, etc. The same CI leg also runs the full sync-manager.e2e.test.ts (new file, unchanged, remote update, conflict, rename, delete, batch, pull) against a real GitHub sandbox.
Whole-suite retry amplifies rate-limit hits instead of recovering from them..github/workflows/ci.yml:341-347 wraps the entire provider E2E step in nick-fields/retry with max_attempts: 3, retry_wait_seconds: 15. A rate-limit failure re-runs the entire suite (20+ remote mutations) after a flat 15s wait — each Vitest process generates fresh random test paths, so no successful mutation from the failed attempt is reused. This is effective for transient ENOTFOUND-style network errors but counterproductive for rate limiting: get throttled → wait 15s → hit the whole suite again → more throttled.
No Retry-After/x-ratelimit-reset awareness. GitHub's own guidance for secondary rate limits is to respect Retry-After when present, or wait at least a minute with exponential backoff otherwise. The current fixed 15s whole-suite retry doesn't follow that model, and doesn't distinguish 403/429/rate-limit responses from generic transient network failures.
CI retry: cap whole-suite retry at 2 attempts (from 3), and/or move toward selective retry that only re-runs on genuine network errors, not on 403/429/rate-limit responses — those should back off per Retry-After/x-ratelimit-reset instead of blindly re-running the whole suite 15s later.
Test tiering — split PR-time coverage from full-contract coverage:
GitHubService.pushFile() calling listFilesDetailed(recursive=1) on every push is a separate, real optimization opportunity (reduces the per-push request count on the production code path, not just tests) — worth its own follow-up once this CI-side fix lands, not bundled here.
No change to the real-provider E2E test content or the production-provider Act/independent-verifier-Assert architecture — that boundary is correct and should be preserved.
Suggested first PR
Small, retry-and-tiering only: test: reduce real-provider API pressure — adjust nick-fields/retry policy in .github/workflows/ci.yml and wire GitHub/GitLab suites into the existing core/full tier split. Do not touch production provider logic in this PR.
Summary
Real-provider E2E is architecturally sound (production provider does Act, an independent git-CLI verifier does Assert — never the service reading back its own write), but the current design does not treat GitHub API request budget as a governed test resource. As currently structured, provider E2E is at real risk of tripping GitHub's secondary rate limit (content-generating mutations, guideline ~80/min, ~500/hour), and the CI retry policy actively makes that worse rather than better.
This is not "the E2E tests are wrong." It's that full-contract regression coverage is being run on every PR with a retry policy tuned for transient network blips, not for rate-limiting.
Problems identified
A single
pushFile()is not one API call.GitHubService.pushFile()callslistFilesDetailed()(recursive tree REST GET), thenpushBatch()→commitOnBranch(), which itself does a GraphQL branch-HEAD query plus acreateCommitOnBranchmutation — ~3 requests minimum per push, beforecommitOnBranch()'s own up-to-3x stale-HEAD retry.The E2E suites are mutation-heavy by design.
github.e2e.test.tscovers create/read/update/delete/batch/rename/symlink/GraphQL-error/concurrent-write, where each scenario like "update" is itself create+update, "delete" is create+delete, etc. The same CI leg also runs the fullsync-manager.e2e.test.ts(new file, unchanged, remote update, conflict, rename, delete, batch, pull) against a real GitHub sandbox.Whole-suite retry amplifies rate-limit hits instead of recovering from them.
.github/workflows/ci.yml:341-347wraps the entire provider E2E step innick-fields/retrywithmax_attempts: 3,retry_wait_seconds: 15. A rate-limit failure re-runs the entire suite (20+ remote mutations) after a flat 15s wait — each Vitest process generates fresh random test paths, so no successful mutation from the failed attempt is reused. This is effective for transientENOTFOUND-style network errors but counterproductive for rate limiting: get throttled → wait 15s → hit the whole suite again → more throttled.No
Retry-After/x-ratelimit-resetawareness. GitHub's own guidance for secondary rate limits is to respectRetry-Afterwhen present, or wait at least a minute with exponential backoff otherwise. The current fixed 15s whole-suite retry doesn't follow that model, and doesn't distinguish 403/429/rate-limit responses from generic transient network failures.Proposed fix (scoped small, production-provider-logic untouched)
CI retry: cap whole-suite retry at 2 attempts (from 3), and/or move toward selective retry that only re-runs on genuine network errors, not on 403/429/rate-limit responses — those should back off per
Retry-After/x-ratelimit-resetinstead of blindly re-running the whole suite 15s later.Test tiering — split PR-time coverage from full-contract coverage:
Note:
scripts/run-e2e.shalready has--tier core|full|autoplumbing from feat(source-control): replace sync status panel with the source control workflow #129 — this fits into that, not a new mechanism.Out of scope for this issue
GitHubService.pushFile()callinglistFilesDetailed(recursive=1)on every push is a separate, real optimization opportunity (reduces the per-push request count on the production code path, not just tests) — worth its own follow-up once this CI-side fix lands, not bundled here.Suggested first PR
Small, retry-and-tiering only:
test: reduce real-provider API pressure— adjustnick-fields/retrypolicy in.github/workflows/ci.ymland wire GitHub/GitLab suites into the existing core/full tier split. Do not touch production provider logic in this PR.