diff --git a/apps/workspace-engine/pkg/github/client.go b/apps/workspace-engine/pkg/github/client.go index 2fba34d30..85fb66cd9 100644 --- a/apps/workspace-engine/pkg/github/client.go +++ b/apps/workspace-engine/pkg/github/client.go @@ -45,7 +45,10 @@ func generateJWT() (string, error) { return token.SignedString(key) } -func appClient() (*github.Client, error) { +// AppClient returns a GitHub client authenticated as the GitHub App +// itself (JWT auth, no installation context). Use this for App-level +// operations like looking up organizations or installations. +func AppClient() (*github.Client, error) { jwtStr, err := generateJWT() if err != nil { return nil, err @@ -60,7 +63,7 @@ func CreateClientForInstallation( ctx context.Context, installationID int64, ) (*github.Client, error) { - app, err := appClient() + app, err := AppClient() if err != nil { return nil, err } @@ -77,7 +80,7 @@ func CreateClientForInstallation( // installation that covers owner/repo. It discovers the installation via // the GitHub API. Returns (nil, nil) if the GitHub bot is not configured. func CreateClientForRepo(ctx context.Context, owner, repo string) (*github.Client, error) { - app, err := appClient() + app, err := AppClient() if err != nil { if config.Global.GithubBotAppID == "" || config.Global.GithubBotPrivateKey == "" { return nil, nil diff --git a/apps/workspace-engine/pkg/jobagents/github/config.go b/apps/workspace-engine/pkg/jobagents/github/config.go index 4b4c2c0b2..2d03fde82 100644 --- a/apps/workspace-engine/pkg/jobagents/github/config.go +++ b/apps/workspace-engine/pkg/jobagents/github/config.go @@ -1,21 +1,28 @@ package github import ( + "context" "fmt" + gh "workspace-engine/pkg/github" "workspace-engine/pkg/oapi" ) -// ParseJobAgentConfig extracts and validates a GithubJobAgentConfig from raw config. -func ParseJobAgentConfig(jobAgentConfig oapi.JobAgentConfig) (oapi.GithubJobAgentConfig, error) { +// ParseJobAgentConfig extracts and validates a GithubJobAgentConfig from +// raw config. Accepts either an explicit "owner" field or an +// "organizationId" field (resolved to the org login via the GitHub API). +func ParseJobAgentConfig( + ctx context.Context, + jobAgentConfig oapi.JobAgentConfig, +) (oapi.GithubJobAgentConfig, error) { installationId := toInt(jobAgentConfig["installationId"]) if installationId == 0 { return oapi.GithubJobAgentConfig{}, fmt.Errorf("installationId is required") } - owner, ok := jobAgentConfig["owner"].(string) - if !ok || owner == "" { - return oapi.GithubJobAgentConfig{}, fmt.Errorf("owner is required") + owner, err := resolveOwner(ctx, jobAgentConfig, int64(installationId)) + if err != nil { + return oapi.GithubJobAgentConfig{}, err } repo, ok := jobAgentConfig["repo"].(string) @@ -42,6 +49,41 @@ func ParseJobAgentConfig(jobAgentConfig oapi.JobAgentConfig) (oapi.GithubJobAgen }, nil } +// resolveOwner returns the GitHub owner/org login from the config. +// If "owner" is present it is used directly; otherwise "organizationId" +// is resolved via the GitHub API using an installation-authenticated +// client (App JWT auth can't hit /organizations/{id}). +func resolveOwner( + ctx context.Context, + cfg oapi.JobAgentConfig, + installationID int64, +) (string, error) { + if owner, ok := cfg["owner"].(string); ok && owner != "" { + return owner, nil + } + + orgID := toInt64(cfg["organizationId"]) + if orgID == 0 { + return "", fmt.Errorf("owner or organizationId is required") + } + + client, err := gh.CreateClientForInstallation(ctx, installationID) + if err != nil { + return "", fmt.Errorf("create github installation client: %w", err) + } + + org, _, err := client.Organizations.GetByID(ctx, orgID) + if err != nil { + return "", fmt.Errorf("lookup organization %d: %w", orgID, err) + } + + login := org.GetLogin() + if login == "" { + return "", fmt.Errorf("organization %d has no login", orgID) + } + return login, nil +} + func toInt(v any) int { switch val := v.(type) { case int: diff --git a/apps/workspace-engine/pkg/jobagents/github/githubaction.go b/apps/workspace-engine/pkg/jobagents/github/githubaction.go index 238ad7945..89e6030b0 100644 --- a/apps/workspace-engine/pkg/jobagents/github/githubaction.go +++ b/apps/workspace-engine/pkg/jobagents/github/githubaction.go @@ -50,7 +50,7 @@ func (a *GithubAction) Dispatch(ctx context.Context, job *oapi.Job) error { return fmt.Errorf("job %s has no dispatch context", job.Id) } - cfg, err := ParseJobAgentConfig(job.DispatchContext.JobAgentConfig) + cfg, err := ParseJobAgentConfig(ctx, job.DispatchContext.JobAgentConfig) if err != nil { return fmt.Errorf("failed to parse job agent config: %w", err) } diff --git a/apps/workspace-engine/pkg/jobagents/github/githubaction_test.go b/apps/workspace-engine/pkg/jobagents/github/githubaction_test.go index b83150d1f..1d99dde3f 100644 --- a/apps/workspace-engine/pkg/jobagents/github/githubaction_test.go +++ b/apps/workspace-engine/pkg/jobagents/github/githubaction_test.go @@ -207,7 +207,7 @@ func TestDispatch_InvalidConfig_ReturnsError(t *testing.T) { // ----- ParseJobAgentConfig ----- func TestParseJobAgentConfig_Valid(t *testing.T) { - cfg, err := ParseJobAgentConfig(validConfig()) + cfg, err := ParseJobAgentConfig(context.Background(), validConfig()) require.NoError(t, err) assert.Equal(t, 12345, cfg.InstallationId) assert.Equal(t, "my-org", cfg.Owner) @@ -219,7 +219,7 @@ func TestParseJobAgentConfig_Valid(t *testing.T) { func TestParseJobAgentConfig_WithRef(t *testing.T) { raw := validConfig() raw["ref"] = "develop" - cfg, err := ParseJobAgentConfig(raw) + cfg, err := ParseJobAgentConfig(context.Background(), raw) require.NoError(t, err) require.NotNil(t, cfg.Ref) assert.Equal(t, "develop", *cfg.Ref) @@ -228,7 +228,7 @@ func TestParseJobAgentConfig_WithRef(t *testing.T) { func TestParseJobAgentConfig_MissingInstallationId(t *testing.T) { raw := validConfig() delete(raw, "installationId") - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "installationId") } @@ -236,7 +236,7 @@ func TestParseJobAgentConfig_MissingInstallationId(t *testing.T) { func TestParseJobAgentConfig_MissingOwner(t *testing.T) { raw := validConfig() delete(raw, "owner") - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "owner") } @@ -244,7 +244,7 @@ func TestParseJobAgentConfig_MissingOwner(t *testing.T) { func TestParseJobAgentConfig_MissingRepo(t *testing.T) { raw := validConfig() delete(raw, "repo") - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "repo") } @@ -252,7 +252,7 @@ func TestParseJobAgentConfig_MissingRepo(t *testing.T) { func TestParseJobAgentConfig_MissingWorkflowId(t *testing.T) { raw := validConfig() delete(raw, "workflowId") - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "workflowId") } @@ -260,7 +260,7 @@ func TestParseJobAgentConfig_MissingWorkflowId(t *testing.T) { func TestParseJobAgentConfig_EmptyOwner(t *testing.T) { raw := validConfig() raw["owner"] = "" - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "owner") } @@ -268,7 +268,7 @@ func TestParseJobAgentConfig_EmptyOwner(t *testing.T) { func TestParseJobAgentConfig_EmptyRepo(t *testing.T) { raw := validConfig() raw["repo"] = "" - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "repo") } @@ -276,7 +276,7 @@ func TestParseJobAgentConfig_EmptyRepo(t *testing.T) { func TestParseJobAgentConfig_EmptyRefIgnored(t *testing.T) { raw := validConfig() raw["ref"] = "" - cfg, err := ParseJobAgentConfig(raw) + cfg, err := ParseJobAgentConfig(context.Background(), raw) require.NoError(t, err) assert.Nil(t, cfg.Ref) } @@ -284,7 +284,7 @@ func TestParseJobAgentConfig_EmptyRefIgnored(t *testing.T) { func TestParseJobAgentConfig_ZeroInstallationId(t *testing.T) { raw := validConfig() raw["installationId"] = float64(0) - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "installationId") } @@ -292,7 +292,7 @@ func TestParseJobAgentConfig_ZeroInstallationId(t *testing.T) { func TestParseJobAgentConfig_ZeroWorkflowId(t *testing.T) { raw := validConfig() raw["workflowId"] = float64(0) - _, err := ParseJobAgentConfig(raw) + _, err := ParseJobAgentConfig(context.Background(), raw) require.Error(t, err) assert.Contains(t, err.Error(), "workflowId") }