Skip to content

Fix the GitHub install admin tests that broke main - #1698

Merged
Makisuo merged 1 commit into
mainfrom
fix/github-connect-admin-tests-initiator
Oct 22, 2026
Merged

Makisuo merged 1 commit into
mainfrom
fix/github-connect-admin-tests-initiator

Conversation

@Makisuo

@Makisuo Makisuo commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

Main has failed 6 GithubConnectService tests since #1685 landed. Its new account-admin cases call completeConnect without the initiator that #1675 made required, so the call is rejected before the org role lookup and every case sees no role URLs.

This passes INITIATOR like the neighbouring tests. The cases still assert the security behaviour: a non-admin, a pending invite, an unreadable role and another user's account are refused after the role lookup; an active org admin and the user's own account are bound.

Verified: GithubConnectService.test.ts 28/28 on current main (6 failing before).

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

#1685's account-admin cases called completeConnect without the initiator #1675 made
required, so the call failed before the role lookup and every case read no role URLs.
Main has failed these 6 tests since both landed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@maple-review-bot

maple-review-bot Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

Maple review

🟢 Confidence 10/10 · safe to merge
One-argument test fix in a case loop; the state minted at line 279 carries the same org and user the case now passes.
no findings · tests covered · risk low

Passes the OAuthFlowInitiator that completeConnect requires in the six adminCases of GithubConnectService.test.ts, so the call reaches the role lookup instead of failing on state consumption. Safe to merge.

  • adminCases in GithubConnectService.test.ts pass INITIATOR to completeConnect
What was checked
  • startConnect at test line 279 mints the state for org_test/user_1, the same pair in INITIATOR (line 55), so states.consume accepts it
  • The cases still assert the role URL was hit, and bound/queue outcomes, so a refusal before the role lookup still fails the test

0c880a6 · Updated on every push. Reply "won't fix" to dismiss a finding, or mention @maple-review-bot to ask about one.

@coderabbitai

coderabbitai Bot commented Oct 22, 2026

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 20 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 4 included reviews currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 0b965bc0-bf8e-46e9-9706-423ef6f5baf1
📥 Commits

Reviewing files that changed from the base of the PR and between 23c97ea and 0c880a6.

📒 Files selected for processing: 1
  • packages/backend/src/services/integrations/vcs/vendor/github/__tests__/GithubConnectService.test.ts
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@Makisuo
Makisuo merged commit 1b06bb5 into main Oct 22, 2026
27 checks passed
@Makisuo
Makisuo deleted the fix/github-connect-admin-tests-initiator branch October 22, 2026 16:54
Thegreatsura pushed a commit to Thegreatsura/maple-analytics that referenced this pull request Oct 23, 2026
…#1635)

* feat(integrations): connect an AWS account as a source

An AWS connection is a pull source backed by an IAM role in the customer's
account. Maple mints an external id, the customer deploys a generated
CloudFormation stack (CloudShell script, console template or Terraform), and
Maple assumes the role from its own identity to confirm the setup. No AWS
secret is stored.

- aws_connections table (source_id required, cascades with its source),
  AwsConnectionService and /v2/integrations/aws (status, create, update,
  setup, check, delete).
- The role trusts Maple's account root, conditioned on the exact principal
  ARN and the connection's external id. Its name is derived from the
  connection id, so a caller can never point Maple at another role.
- Each future module (metrics, inventory, security, cost) is its own policy
  on the same role; the check reads which are attached and the setup
  version tag, so a new module is a stack update, not a reconnect.
- Hourly alerting lane re-checks connections: paused sources are skipped in
  SQL, the batch is claimed in one statement, two checks run at a time.
  A deleted stack reports the source disconnected; AWS outages only record
  the error.
- Connect card under Sources: account form, setup panel that re-checks every
  10s while open, connection health, disconnect with a cleanup script kept
  until dismissed.
- Docs page integrations/aws.

* chore(aws): regenerate the iOS OpenAPI spec and anticipated error identifiers

* fix(aws): the unavailable card drops the steps it cannot offer

* fix(aws): a background check only closes its own setup; pending setup is not a failure

* fix(aws): the hourly sweep runs batches until nothing is due, never-checked first

* fix(aws): a sweep checks its batch in due order, never-checked first

* fix(aws): no check starts past the sweep budget

* fix(aws): a disconnect closes only its own dialog; document and test PATCH

A completed disconnect read the last-clicked connection, so it could close
another connection's dialog and setup. The update request now documents that
omitted modules stay unchanged, and update is tested in the service and route.

* test(web): the in-flight click case starts a poll before it clicks

The case clicked a page with no check in flight, so it never reached the
flip that makes a background check the user's.

* test(aws): the budget sweep allows a check that timed out unassumed

Two checks run at once and each fake call jumps the shared clock 9s, so a
call can pass its 20s timeout and count as checked without an AssumeRole.

* fix(web): an AWS stack that needs updating reads as needing attention

The hub card, sources picker and header now agree with the connection row.

* test(aws): another org's admin cannot see or touch a connection; members cannot check it

* test(github): pass the initiator in the install admin cases

Same one-line fix as MapleTechLabs#1698: main's MapleTechLabs#1675 and MapleTechLabs#1685 crossed, so these cases
called completeConnect without the initiator MapleTechLabs#1675 made required.
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