Skip to content

feat(mint): add e2e agent role for pool testing #2407

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ifireball merged 6 commits into from
Jun 21, 2026
Merged

feat(mint): add e2e agent role for pool testing #2407

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8117e84
feat(mint): add e2e agent role for pool testing
ifireball Jun 18, 2026
2509f5e
test(config): expect e2e in ValidRoles
ifireball Jun 18, 2026
8ee2f1e
fix(forge): align e2e app manifest with mint role permissions
ifireball Jun 18, 2026
8c2ff27
chore(forge): gofmt AppPermissions struct alignment
ifireball Jun 18, 2026
aa108a2
test: add e2e role drift guards across mintcore and forge
ifireball Jun 18, 2026
01c65cf
fix(layers): skip harness wrapper for e2e mint role
ifireball Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions internal/cli/mint_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -983,6 +983,10 @@ func TestValidateMintSetupRole(t *testing.T) {
require.NoError(t, err)
assert.Equal(t, "coder", role)

role, err = validateMintSetupRole("e2e")
require.NoError(t, err)
assert.Equal(t, "e2e", role)

_, err = validateMintSetupRole("fix")
require.Error(t, err)
assert.Contains(t, err.Error(), "coder")
Expand Down
2 changes: 1 addition & 1 deletion internal/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ type OrgConfig struct {

// ValidRoles returns the set of recognized agent roles.
func ValidRoles() []string {
return []string{"fullsend", "triage", "coder", "review", "fix", "retro", "prioritize"}
return []string{"fullsend", "triage", "coder", "review", "fix", "retro", "prioritize", "e2e"}
Comment thread
ifireball marked this conversation as resolved.
Comment thread
ifireball marked this conversation as resolved.
Comment thread
ifireball marked this conversation as resolved.
Comment thread
ifireball marked this conversation as resolved.

@fullsend-ai-review fullsend-ai-review Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] scope-ambiguity

Adding e2e to ValidRoles() makes it configurable in any org's defaults.roles or per-repo roles list via config YAML. The mint handler has ALLOWED_ROLES and ROLE_APP_IDS environment-variable controls per mint instance, but there is no code-level guard at the config validation layer restricting which orgs may reference the e2e role. Issue #2155 explicitly constrains: 'Limit minted e2e credentials to the dedicated test org pool.'

Suggested fix: Ensure the e2e role's GitHub App and PEM are deployed only to mint instances serving the dedicated test org pool, or add explicit org-allowlist checking for the e2e role.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[medium] scope-inconsistency

The e2e role is added to ValidRoles() but intentionally excluded from DefaultAgentRoles() and PerRepoDefaultRoles(). ValidRoles() is used for config validation, but e2e is a pool/CI mint role that should never appear in org config files. The comment in harnesswrappers.go states 'The e2e role is a pool/CI mint role and is not installed as an agent app', but ValidRoles() is not documented as including non-agent roles. See also: [scope-ambiguity] finding at this location.

Suggested fix: Add a comment to ValidRoles() clarifying it includes mint-only roles that are not installed as agent apps.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[low] scope-inconsistency

The e2e role is added to ValidRoles() but intentionally excluded from DefaultAgentRoles() and PerRepoDefaultRoles(). ValidRoles() is used for config validation, but e2e is a pool/CI mint role. ValidRoles() itself lacks a comment noting it includes mint-only roles. The new TestValidRoles_RecognizedByMintcore test provides drift detection.

Suggested fix: Add a comment to ValidRoles() clarifying it includes mint-only roles that are not installed as agent apps.

}

// ValidProviders returns the set of recognized inference providers.
Expand Down
12 changes: 11 additions & 1 deletion internal/config/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,18 +6,28 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/fullsend-ai/fullsend/internal/mintcore"
)

func TestValidRoles(t *testing.T) {
roles := ValidRoles()
assert.Len(t, roles, 7)
assert.Len(t, roles, 8)
assert.Contains(t, roles, "fullsend")
assert.Contains(t, roles, "triage")
assert.Contains(t, roles, "coder")
assert.Contains(t, roles, "review")
assert.Contains(t, roles, "fix")
assert.Contains(t, roles, "retro")
assert.Contains(t, roles, "prioritize")
assert.Contains(t, roles, "e2e")
}

func TestValidRoles_RecognizedByMintcore(t *testing.T) {
for _, role := range ValidRoles() {
assert.True(t, mintcore.HasRole(role),
"ValidRoles() contains %q but mintcore.HasRole is false — role lists may have drifted (see issue tracking consolidation)", role)
}
}

func TestPerRepoDefaultRoles(t *testing.T) {
Expand Down
6 changes: 6 additions & 0 deletions internal/dispatch/gcf/mintsrc/mintcore/github.go.embed
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ var canonicalRolePermissions = map[string]map[string]string{
"retro": {"actions": "read", "contents": "read", "pull_requests": "write", "issues": "write", "metadata": "read"},
"prioritize": {"contents": "read", "issues": "write", "organization_projects": "write", "metadata": "read"},
"fullsend": {"actions": "write", "actions_variables": "read", "contents": "write", "pull_requests": "write", "workflows": "write", "metadata": "read"},
"e2e": {
"actions": "write", "actions_variables": "read", "administration": "write",
"contents": "write", "issues": "write", "members": "write", "metadata": "read",
"organization_administration": "write", "pull_requests": "write",
"secrets": "write", "workflows": "write",
},
}

// RolePermissions returns a deep copy of the role-to-permissions map,
Expand Down
39 changes: 29 additions & 10 deletions internal/forge/github/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@ import "fmt"

// AppPermissions defines the permissions for a GitHub App.
type AppPermissions struct {
Actions string `json:"actions,omitempty"`
Issues string `json:"issues,omitempty"`
PullRequests string `json:"pull_requests,omitempty"`
Checks string `json:"checks,omitempty"`
Contents string `json:"contents,omitempty"`
Variables string `json:"actions_variables,omitempty"`
Workflows string `json:"workflows,omitempty"`
Administration string `json:"administration,omitempty"`
Members string `json:"members,omitempty"`
OrganizationProjects string `json:"organization_projects,omitempty"`
Actions string `json:"actions,omitempty"`
Issues string `json:"issues,omitempty"`
PullRequests string `json:"pull_requests,omitempty"`
Checks string `json:"checks,omitempty"`
Contents string `json:"contents,omitempty"`
Variables string `json:"actions_variables,omitempty"`
Workflows string `json:"workflows,omitempty"`
Administration string `json:"administration,omitempty"`
Members string `json:"members,omitempty"`
OrganizationProjects string `json:"organization_projects,omitempty"`
OrganizationAdministration string `json:"organization_administration,omitempty"`
Secrets string `json:"secrets,omitempty"`
}

// HookAttributes configures the webhook for a GitHub App.
Expand Down Expand Up @@ -139,6 +141,23 @@ func AgentAppConfig(org, role, appSet string) AppConfig {
// No webhook events — triggered via workflow_dispatch from other agents.
base.Events = []string{}

case "e2e":
base.Description = fmt.Sprintf("Fullsend e2e pool testing for %s", org)
base.Permissions = AppPermissions{
Actions: "write",
Variables: "read",
Administration: "write",
Contents: "write",
Issues: "write",
Members: "write",
OrganizationAdministration: "write",
PullRequests: "write",
Secrets: "write",
Workflows: "write",
}
// Pool tests are API/mint driven; no webhook events required.
base.Events = []string{}

default:
base.Description = fmt.Sprintf("Fullsend %s agent for %s", role, org)
base.Permissions = AppPermissions{
Expand Down
78 changes: 78 additions & 0 deletions internal/forge/github/types_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import (

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/fullsend-ai/fullsend/internal/mintcore"
)

func TestDefaultAgentRoles(t *testing.T) {
Expand Down Expand Up @@ -102,6 +104,82 @@ func TestAgentAppConfig_Retro(t *testing.T) {
assert.Empty(t, cfg.Events)
}

func TestAgentAppConfig_E2e(t *testing.T) {
cfg := AgentAppConfig("myorg", "e2e", "fullsend-ai")

assert.Equal(t, "fullsend-ai-e2e", cfg.Name)
assert.Equal(t, "write", cfg.Permissions.Actions)
assert.Equal(t, "read", cfg.Permissions.Variables)
assert.Equal(t, "write", cfg.Permissions.Administration)
assert.Equal(t, "write", cfg.Permissions.Contents)
assert.Equal(t, "write", cfg.Permissions.Issues)
assert.Equal(t, "write", cfg.Permissions.Members)
assert.Equal(t, "write", cfg.Permissions.OrganizationAdministration)
assert.Equal(t, "write", cfg.Permissions.PullRequests)
assert.Equal(t, "write", cfg.Permissions.Secrets)
assert.Equal(t, "write", cfg.Permissions.Workflows)
assert.Empty(t, cfg.Events)
}

Comment thread
ifireball marked this conversation as resolved.
// appPermissionsAsMap converts manifest permissions to GitHub API permission names.
func appPermissionsAsMap(p AppPermissions) map[string]string {
out := make(map[string]string)
if p.Actions != "" {
out["actions"] = p.Actions
}
if p.Issues != "" {
out["issues"] = p.Issues
}
if p.PullRequests != "" {
out["pull_requests"] = p.PullRequests
}
if p.Checks != "" {
out["checks"] = p.Checks
}
if p.Contents != "" {
out["contents"] = p.Contents
}
if p.Variables != "" {
out["actions_variables"] = p.Variables
}
if p.Workflows != "" {
out["workflows"] = p.Workflows
}
if p.Administration != "" {
out["administration"] = p.Administration
}
if p.Members != "" {
out["members"] = p.Members
}
if p.OrganizationProjects != "" {
out["organization_projects"] = p.OrganizationProjects
}
if p.OrganizationAdministration != "" {
out["organization_administration"] = p.OrganizationAdministration
}
if p.Secrets != "" {
out["secrets"] = p.Secrets
}
return out
}

func TestAgentAppConfig_E2eMatchesMintcorePermissions(t *testing.T) {
canonical := mintcore.RolePermissionsFor("e2e")
require.NotNil(t, canonical)

manifest := appPermissionsAsMap(AgentAppConfig("myorg", "e2e", "fullsend-ai").Permissions)

// metadata is added at mint token time; GitHub App manifests omit it explicitly.
for key, want := range canonical {
if key == "metadata" {
continue
}
got, ok := manifest[key]
assert.True(t, ok, "AgentAppConfig(e2e) missing permission %q from mintcore canonicalRolePermissions", key)
assert.Equal(t, want, got, "permission %q mismatch between AgentAppConfig and mintcore", key)
}
}

func TestAgentAppConfig_UnknownRole(t *testing.T) {
cfg := AgentAppConfig("myorg", "custom-bot", "fullsend")

Expand Down
3 changes: 2 additions & 1 deletion internal/layers/harnesswrappers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,10 @@ func (l *HarnessWrappersLayer) RequiredScopes(op Operation) []string {
// harnessesForRole returns the harness filename(s) for a given agent role.
// The coder role maps to both code and fix harnesses (fix reuses the coder app).
// The fullsend role is the org-level app and has no harness.
// The e2e role is a pool/CI mint role and is not installed as an agent app.
func harnessesForRole(role string) []string {
switch role {
case "fullsend":
case "fullsend", "e2e":
return nil
case "coder":
return []string{"code", "fix"}
Expand Down
6 changes: 6 additions & 0 deletions internal/mintcore/github.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,12 @@ var canonicalRolePermissions = map[string]map[string]string{
"retro": {"actions": "read", "contents": "read", "pull_requests": "write", "issues": "write", "metadata": "read"},
"prioritize": {"contents": "read", "issues": "write", "organization_projects": "write", "metadata": "read"},
"fullsend": {"actions": "write", "actions_variables": "read", "contents": "write", "pull_requests": "write", "workflows": "write", "metadata": "read"},
"e2e": {
Comment thread
ifireball marked this conversation as resolved.
Comment thread
ifireball marked this conversation as resolved.
Comment thread
ifireball marked this conversation as resolved.

@fullsend-ai-review fullsend-ai-review Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler's ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, nor does the code enforce the pool-only constraint from issue #2155.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler's ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, and no code-level constraint restricts the role to known test-pool orgs.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

@fullsend-ai-review fullsend-ai-review Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[high] permission-expansion

The new e2e role grants administration: write, organization_administration: write, secrets: write, and members: write — the most privileged GitHub App permissions available. No other role in the canonical permissions map requests organization_administration or secrets at write level. Combined, a minted e2e token could reconfigure repos, manage org settings, overwrite secrets, and modify org membership. The mint handler ALLOWED_ROLES/ROLE_APP_IDS gating and exclusion from DefaultAgentRoles() are meaningful mitigating controls, but neither the code nor an ADR documents why each elevated permission is necessary, and no code-level constraint restricts the role to known test-pool orgs.

Suggested fix: For each permission beyond what the fullsend role already grants, document the specific e2e test operation that requires it (ideally in an ADR or inline comment). In particular: (1) Justify organization_administration: write — if tests only need repo-level admin, remove it. (2) Justify secrets: write — document whether tests must create/rotate secrets. (3) Justify members: write — if tests do not manage org membership, downscope to read. (4) Consider adding a code-level constraint restricting the e2e role to known test-pool orgs.

"actions": "write", "actions_variables": "read", "administration": "write",
"contents": "write", "issues": "write", "members": "write", "metadata": "read",
"organization_administration": "write", "pull_requests": "write",
"secrets": "write", "workflows": "write",
},
}

// RolePermissions returns a deep copy of the role-to-permissions map,
Expand Down
18 changes: 17 additions & 1 deletion internal/mintcore/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ func TestCreateInstallationToken_UnknownRole(t *testing.T) {
}

func TestRolePermissions_AllRolesPresent(t *testing.T) {
expectedRoles := []string{"triage", "coder", "review", "fix", "retro", "prioritize", "fullsend"}
expectedRoles := []string{"triage", "coder", "review", "fix", "retro", "prioritize", "fullsend", "e2e"}
Comment thread
ifireball marked this conversation as resolved.
allPerms := RolePermissions()
for _, role := range expectedRoles {
perms, ok := allPerms[role]
Expand All @@ -115,6 +115,22 @@ func TestRolePermissions_AllRolesPresent(t *testing.T) {
}
}

func TestRolePermissions_E2e(t *testing.T) {
perms := RolePermissionsFor("e2e")
require.NotNil(t, perms)
assert.Equal(t, "write", perms["actions"])
assert.Equal(t, "read", perms["actions_variables"])
assert.Equal(t, "write", perms["administration"])
assert.Equal(t, "write", perms["contents"])
assert.Equal(t, "write", perms["issues"])
assert.Equal(t, "write", perms["members"])
assert.Equal(t, "read", perms["metadata"])
assert.Equal(t, "write", perms["organization_administration"])
assert.Equal(t, "write", perms["pull_requests"])
assert.Equal(t, "write", perms["secrets"])
assert.Equal(t, "write", perms["workflows"])
}

func TestRolePermissions_ReturnsCopy(t *testing.T) {
// Mutating the returned map must not affect the canonical definitions.
perms := RolePermissions()
Expand Down
Loading