Skip to content

[Refactor] Resolve GitHub task credential scope once - #3627

Open
roomote-roomote[bot] wants to merge 1 commit into
developfrom
feature/github-credential-plan-878c8241
Open

roomote-roomote[bot] wants to merge 1 commit into
developfrom
feature/github-credential-plan-878c8241

Conversation

@roomote-roomote

Copy link
Copy Markdown
Contributor

​Created by Roomote. View the session or mention @roomote-roomote for follow-up asks.

Related issue

Internal maintainability work on the multi-installation task credentials promoted in #3623 and extended in #3624. No linked issue. Checked #3618: its worker-side atomic credential publication changes are independent of this server-side scope resolution.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

What changed

Resolve a task's stamped active GitHub repository rows once and group them by installation in a small credential plan. Both the primary API token and additional Git-only credentials consume that plan, replacing the normal multi-installation throw/catch fallback and repeated stamp queries.

Environment expansion remains a separate compatibility lookup; stamps already contained in that expansion reuse its rows. A spanning stamp without an environment now needs one repository lookup rather than three during worker credential creation. The production resolver is 42 lines smaller.

There is no intended behavior change: explicit environments and unstamped selections still require one installation; primary-repository and oldest-active-installation anchoring retain their precedence; unavailable stamped repositories keep their skip/fail behavior. API-only callers mint only the primary token. Additional tokens remain repository-scoped and Git-only, with unchanged permanent/transient mint failure handling and earliest-expiry scheduling.

How it was tested

  • Focused task-run-token.test.ts: 28 tests passed, including the existing scope, fallback, expiry, and mint-error baseline.
  • Added assertions that spanning API and worker operations read the stamp once, plus coverage for environment expansion with grouped additional credentials, repositories without GitHub IDs, and an unavailable primary repository.
  • pnpm --filter @roomote/github check-types passed.
  • Normal pre-push gates passed: oxlint, residual web/worker ESLint, all 27 fast typecheck tasks, and Knip. Formatting and git diff --check passed.
  • Browser proof is not applicable to this backend-only refactor. Tests mock query and mint collaborators to exercise orchestration; live GitHub installation access was not exercised.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

@roomote-community

roomote-community Bot commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

No code issues found. See session

Reviewed credential scope resolution, environment expansion and installation anchoring, unavailable-repository handling, additional Git credentials, and expiry/error behavior against the previous implementation and tests. Current-head CI tests, lint, type checks, and Knip passed; remaining build/compatibility/security checks were still running when inspected.

Reviewed 0e16b52

@roomote-roomote
roomote-roomote Bot marked this pull request as ready for review October 29, 2026 22:23

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