From 3a46b48e6cec2f2cdbe60819b43fcc2013a9ad85 Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Wed, 22 Apr 2026 11:52:52 -0700 Subject: [PATCH 1/2] chore: gh agent config also works with direct org id --- apps/workspace-engine/pkg/github/client.go | 9 ++-- .../pkg/jobagents/github/config.go | 47 +++++++++++++++++-- .../pkg/jobagents/github/githubaction.go | 2 +- .../pkg/jobagents/github/githubaction_test.go | 22 ++++----- 4 files changed, 60 insertions(+), 20 deletions(-) diff --git a/apps/workspace-engine/pkg/github/client.go b/apps/workspace-engine/pkg/github/client.go index 2fba34d30a..85fb66cd98 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 4b4c2c0b24..a1eb948670 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) + if err != nil { + return oapi.GithubJobAgentConfig{}, err } repo, ok := jobAgentConfig["repo"].(string) @@ -42,6 +49,36 @@ 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 looked up via the GitHub API. Returns an error if neither is set. +func resolveOwner(ctx context.Context, cfg oapi.JobAgentConfig) (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.AppClient() + if err != nil { + return "", fmt.Errorf("create github app 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 238ad7945b..89e6030b08 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 b83150d1fd..1d99dde3ff 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") } From 689f6017f94e150dfc71cc0bcafc1a85e992e89e Mon Sep 17 00:00:00 2001 From: Aditya Choudhari Date: Wed, 22 Apr 2026 12:50:48 -0700 Subject: [PATCH 2/2] cleanup --- .../pkg/jobagents/github/config.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/workspace-engine/pkg/jobagents/github/config.go b/apps/workspace-engine/pkg/jobagents/github/config.go index a1eb948670..2d03fde829 100644 --- a/apps/workspace-engine/pkg/jobagents/github/config.go +++ b/apps/workspace-engine/pkg/jobagents/github/config.go @@ -20,7 +20,7 @@ func ParseJobAgentConfig( return oapi.GithubJobAgentConfig{}, fmt.Errorf("installationId is required") } - owner, err := resolveOwner(ctx, jobAgentConfig) + owner, err := resolveOwner(ctx, jobAgentConfig, int64(installationId)) if err != nil { return oapi.GithubJobAgentConfig{}, err } @@ -51,8 +51,13 @@ func ParseJobAgentConfig( // resolveOwner returns the GitHub owner/org login from the config. // If "owner" is present it is used directly; otherwise "organizationId" -// is looked up via the GitHub API. Returns an error if neither is set. -func resolveOwner(ctx context.Context, cfg oapi.JobAgentConfig) (string, error) { +// 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 } @@ -62,9 +67,9 @@ func resolveOwner(ctx context.Context, cfg oapi.JobAgentConfig) (string, error) return "", fmt.Errorf("owner or organizationId is required") } - client, err := gh.AppClient() + client, err := gh.CreateClientForInstallation(ctx, installationID) if err != nil { - return "", fmt.Errorf("create github app client: %w", err) + return "", fmt.Errorf("create github installation client: %w", err) } org, _, err := client.Organizations.GetByID(ctx, orgID)