Skip to content

fix: sync all repository-owning app installations - #1094

Merged
decyjphr merged 3 commits into
yadhav/fix-recent-issuesfrom
decyjphr-all-installations-audit
Sep 29, 2026
Merged

decyjphr merged 3 commits into
yadhav/fix-recent-issuesfrom
decyjphr-all-installations-audit

Conversation

@decyjphr

Copy link
Copy Markdown
Collaborator

Manual and scheduled full sync currently process only the first App installation, silently leaving other accounts without reconciliation. This adapts #1044 onto yadhav/fix-recent-issues, preserving the target branch's enterprise support and making partial failures visible.

Approach

  • Process repository-owning installations sequentially, with separate authentication and owner context. Continue after authentication, configuration, sync, or missing-result failures.
  • Return { results, errors }, retaining returned Settings objects, including partial results, and aggregating their errors. Count installations with reported errors as failed in the summary.
  • Exclude enterprise-only installations from repository sync while preserving enterprise context enrichment, user installations, and the existing info() App lookup. Reject missing account logins explicitly.
  • Await NOP error reporting so later installations do not race it and reporting failures are collected. Await scheduled runs and log enumeration failures.
  • Preserve null for zero eligible installations and the CLI's nonzero exit, replacing its incidental TypeError with an explicit diagnostic.
  • Add Phase 22 for real Settings NOP against one verified test-org installation, with controlled enumeration/auth and a read-only request guard. Add a smoke-entry CRON check and document behavior.

Validation

Using Node 22.12.0 / npm 10.9.0:

  • npm run test:unit -- --runInBand: 841 passed, 12 skipped, including 46 new tests for sequential fanout, owner/token separation, enterprise enrichment, error aggregation, CLI exit behavior, and smoke boundaries.
  • The 34 installation/CLI regression cases against exact baseline 188c604929201774699fd1d8dabd445358cee9e8: 31 failed, 3 passed.
  • Standard/ESLint pass for new tests and full-sync.js; syntax and diff checks pass. The 10 existing lint errors in index.js and smoke-test.js are unchanged.
  • Integration remains blocked by the same baseline Probot ESM/Jest load error: 7 suites fail before tests execute.
  • Live Setup -> Phase 22 -> Teardown: 9 passed, 0 failed, no fatal error, and server HTTP 200 observed. Historical before/after snapshots match for the admin policy, 12 property definitions, all 15 foreign refs, and fixture cleanup. Owned processes exited and the port was released.

Important limitations and integration notes

Smoke startup correction: the live run used a session-only containment preload, not committed here, to filter real App enumeration and restrict installation auth. However, the spawned Probot CLI reloaded .env after preload checks: saved logs confirm webhook forwarding started and enterprise verification ran despite supplied empty variables. The smoke-entry CRON check does not guarantee the spawned server retains an empty CRON value. No webhook delivery or cron tick is recorded, but the logs do not prove that neither was enabled. Recorded intercepted API traffic contains only the authorized installation's token request; the instrumentation was not exhaustive. The 9 assertions establish controlled single-org Settings behavior, not unmodified startup isolation or live multi-org fanout.

CLI runtime limitation: entrypoint tests run the real CLI code with mocked Probot boundaries. A separate network-disabled check with installed Probot 14 reproduced the pre-existing null logger failure before probot.ready(). This branch does not fix that readiness issue; reconcile the independent #1053 follow-up when landing together.

GH_ORG does not filter full sync on this baseline and is not introduced here. Combined landing with #1053 must preserve all-installations behavior by default and explicit organization filtering, reconciling overlapping code, tests, CLI changes, and documentation.

Source audit

Source commits 9b63a02 and dependent f720b1a were missing and adapted into be88186; 6c4f0ea adds guarded smoke coverage. Upstream merge commits ad45b4d and 207de06 were excluded. No wholesale upstream or sibling merge was used.

decyjphr and others added 2 commits September 28, 2026 23:29
Adapt PR #1044 source commits 9b63a02 and f720b1a onto the enterprise target without upstream merges. Preserve enterprise enrichment and user installations, isolate errors sequentially, count returned errors in summaries, await NOP error reporting, and keep CLI failures explicit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add phase 22 using real Settings NOP with verified test-org installation metadata, controlled enumeration/auth, and read-only request boundaries. Refuse CRON before smoke setup can start an unscoped scheduled sync. Keep live multi-installation claims separate from mocked fanout tests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@decyjphr decyjphr closed this Sep 29, 2026
@decyjphr decyjphr reopened this Sep 29, 2026
Resolve README and smoke phase catalog conflicts by retaining variable pagination phase 21 and installation full-sync phase 22. Clarify that the spawned Probot CLI can reload .env after the harness CRON check.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@decyjphr
decyjphr merged commit 4c5742b into yadhav/fix-recent-issues Sep 29, 2026
2 checks passed
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