Skip to content

add better tests for the github permissions#131

Merged
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:add-better-tests-for-the-github-permissions
Apr 15, 2026
Merged

add better tests for the github permissions#131
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:add-better-tests-for-the-github-permissions

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

@derekmisler derekmisler commented Apr 15, 2026

Related Issues

Closes: https://github.com/docker/gordon/issues/390

Summary

Fixes GitHub App token generation to dynamically fetch installation permissions instead of hardcoding them, adds comprehensive test coverage for the permissions flow, and restructures CI workflows for better separation of concerns.

Permission handling (src/app-token.ts)

  • Replaces the hardcoded permissions map with a dynamic lookup via appOctokit.apps.getInstallation() — the token now always reflects what the org has actually granted, so permissions never drift out of sync with the App's settings.

Test restructuring

  • Deleted lint-actions.yml — lint, typecheck, and unit tests are consolidated into test.yml
  • New test-e2e.yml — E2E tests (prompt sanitization, output extraction, job summary, security, exploits, pirate agent, invalid agent) run as a separate workflow
  • New vitest.integration.config.ts + pnpm test:integration — integration tests (real GitHub App API calls) are isolated from unit tests
  • Updated app-token.test.ts — proper mocks for the new installation-permission flow, plus a test that verifies permissions are passed through to createAppAuth
  • New app-token.integration.test.ts — live tests that verify App identity resolution, installation permission fetching, and token generation against the real API (credentials via env vars or 1Password CLI)

Release workflow (release.yml)

  • Reordered jobs: publish-agentupdate-self-refsupdate-consumersnotify for a logical dependency chain
  • notify now waits for all post-release jobs and is skipped on pre-release
  • Pre-release description updated to mention Slack skip

Agent model upgrade (review-pr/agents/pr-review.yaml)

  • sonnet model updated from claude-sonnet-4-5claude-sonnet-4-6 with 64k max tokens
  • Replaced haiku with sonnet-thinking (claude-sonnet-4-6 + adaptive/high thinking budget)
  • drafter agent switched to sonnet-thinking for better bug hypothesis generation

Tooling

  • New scripts/debug-permissions.ts for locally inspecting GitHub App and installation permissions

Tip

Comment /review to trigger the PR Reviewer agent for automated feedback.
Comment /describe to generate a PR description.

Signed-off-by: Derek Misler <derek.misler@docker.com>
@derekmisler derekmisler self-assigned this Apr 15, 2026
@derekmisler derekmisler requested a review from a team April 15, 2026 14:34
@derekmisler derekmisler marked this pull request as ready for review April 15, 2026 14:34
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just reordered these steps in a logical way

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i just renamed this one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just renamed this one

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i added a separate "integration test" job

Comment on lines +4 to +6
* Credentials are resolved in order:
* 1. GITHUB_APP_ID / GITHUB_APP_PRIVATE_KEY env vars (if set)
* 2. 1Password CLI (`op read`)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allows me to run them locally, which is nice.

Comment thread src/app-token.ts
issues: 'write',
pull_requests: 'write',
statuses: 'write',
variables: 'read',
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this line was the whole reason for this PR. it's supposed to be action_variables, not variables (even though it's called "Variables" in the GitHub UI). so anyway, i'm just fetching all the permissions from GH and passing them along to this token generation step, so we won't have typos in the future.

@derekmisler derekmisler enabled auto-merge (squash) April 15, 2026 14:39
@derekmisler derekmisler merged commit 0114825 into docker:main Apr 15, 2026
37 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.

2 participants