From b76d35d85d76f0a6da71b16ba808c2dbba26413d Mon Sep 17 00:00:00 2001 From: Greg Allen Date: Mon, 22 Jun 2026 16:34:14 -0400 Subject: [PATCH] refactor(config): remove OrgConfig.Agents field entirely (ADR-0045 Phase 4 PR 4) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove AgentEntry type, OrgConfig.Agents field, AgentSlugs(), and HasAgentsBlock() — agent identity now lives exclusively in harness wrapper files. Inline Role/Name/Slug fields into layers.AgentCredentials to replace the embedded AgentEntry. Signed-off-by: Greg Allen Co-Authored-By: Claude Opus 4.6 Signed-off-by: Greg Allen --- .../0045-forge-portable-harness-schema.md | 16 +- .../adr-0045-forge-portable-harness-phase4.md | 10 +- .../plans/2026-06-11-triage-prerequisites.md | 2 - internal/cli/admin.go | 26 +-- internal/cli/admin_test.go | 25 ++- internal/cli/github.go | 2 +- internal/cli/mint_test.go | 4 +- internal/config/config.go | 24 --- internal/config/config_test.go | 170 +++--------------- internal/layers/harnesswrappers_test.go | 47 +++-- internal/layers/secrets.go | 7 +- internal/layers/secrets_test.go | 33 ++-- .../fixtures/configrepo/config-valid.yaml | 1 - 13 files changed, 127 insertions(+), 240 deletions(-) diff --git a/docs/ADRs/0045-forge-portable-harness-schema.md b/docs/ADRs/0045-forge-portable-harness-schema.md index 76efc274b..111fe9d83 100644 --- a/docs/ADRs/0045-forge-portable-harness-schema.md +++ b/docs/ADRs/0045-forge-portable-harness-schema.md @@ -31,7 +31,7 @@ harness. They reside in `config.yaml`'s `agents:` block ([ADR 0011](0011-admin-install-org-config-yaml-v1.md)): ```yaml -# config.yaml (current) +# config.yaml (before ADR-0045) agents: - role: triage name: fullsend-ai-triage @@ -684,14 +684,12 @@ forge-specific artifact. The harness and agent definition are portable. duplication. Script-level factoring (shared functions sourced by forge-specific scripts) is a convention, not a schema concern. -- **config.yaml schema versioning.** Removing `agents:` (Phase 4) changes - the v1 schema contract established by ADR 0011. The current - `OrgConfig.Agents` field uses `yaml:"agents"` without `omitempty`, - meaning it is part of the v1 contract. Adding `omitempty` and treating - absence as "discover from harness files" is likely v1-compatible for - Phase 3 (deprecation), but full removal in Phase 4 may warrant a v2 - schema. Consumers that assume `Agents` is always populated need - auditing. +- **config.yaml schema versioning.** Removing `agents:` (Phase 4) changed + the v1 schema contract established by ADR 0011. The `OrgConfig.Agents` + field was removed in Phase 4; `yaml.Unmarshal` silently ignores the + key in existing config files, so v1 compatibility is preserved. + Phase 3 (PR 6) added `omitempty` as a deprecation step; Phase 4 + completed the removal. No v2 schema bump was needed. *Note: Phase 3 PR 6 added `omitempty` to the `Agents` field. The Phase 4 plan (`docs/plans/adr-0045-forge-portable-harness-phase4.md`) recommends staying on v1 — removal is backward-compatible since diff --git a/docs/plans/adr-0045-forge-portable-harness-phase4.md b/docs/plans/adr-0045-forge-portable-harness-phase4.md index 8ab447917..bb1e0dcd4 100644 --- a/docs/plans/adr-0045-forge-portable-harness-phase4.md +++ b/docs/plans/adr-0045-forge-portable-harness-phase4.md @@ -52,7 +52,7 @@ If a future change requires breaking the v1 contract (e.g., removing `dispatch.p - Does NOT add new harness schema features (forge blocks, base composition improvements) - Does NOT change `PerRepoConfig` -- per-repo mode does not use the `agents:` block -- Does NOT remove `AgentEntry` from `config.go` -- it is still used by `AgentCredentials` in `internal/layers/secrets.go` for the install flow's credential passing. `AgentEntry` represents credentials obtained during app setup, not config.yaml schema. +- ~~Does NOT remove `AgentEntry` from `config.go`~~ — **Done:** `AgentEntry` was removed and its fields (Role, Name, Slug) inlined into `layers.AgentCredentials`. - Does NOT change harness loading pipelines (`Load`, `LoadWithOpts`, `LoadWithBase`) - Does NOT remove `DefaultAgentRoles()` or `ValidRoles()` -- these are used for role validation and app setup, independent of the `agents:` block - Does NOT remove the `forge:` section or `base:` field infrastructure (those are permanent schema additions) @@ -72,9 +72,9 @@ Every consumer of the removed code, and the action taken: | Consumer | Location | Current behavior | Phase 4 action | |---|---|---|---| -| `OrgConfig.Agents` field | `internal/config/config.go:86` | `yaml:"agents,omitempty"` | Remove field | -| `AgentSlugs()` method | `internal/config/config.go:259` | Returns `map[role]slug` from `Agents` | Remove method | -| `HasAgentsBlock()` method | `internal/config/config.go:270` | Returns `len(c.Agents) > 0` | Remove method | +| `OrgConfig.Agents` field | `internal/config/config.go:86` | `yaml:"agents,omitempty"` | ✅ Remove field (#2517) | +| `AgentSlugs()` method | `internal/config/config.go:259` | Returns `map[role]slug` from `Agents` | ✅ Remove method (#2517) | +| `HasAgentsBlock()` method | `internal/config/config.go:270` | Returns `len(c.Agents) > 0` | ✅ Remove method (#2517) | | `NewOrgConfig` agents param | `internal/config/config.go:117` | Accepts `[]AgentEntry`, sets `cfg.Agents` | ✅ Remove parameter, stop setting field (#2447) | | `NewOrgConfig` caller: `runDryRun` | `internal/cli/admin.go:1196` | Passes `nil` for agents | ✅ Remove agents arg (#2447) | | `NewOrgConfig` caller: `runInstall` | `internal/cli/admin.go:1513` | Passes agents built from `agentCreds` | ✅ Remove agents arg (#2447) | @@ -290,7 +290,7 @@ func discoverAgentSlugs(ctx context.Context, client forge.Client, owner, configR - Remove `Agents []AgentEntry` from `OrgConfig` struct (line 86) - Remove `AgentSlugs()` method (lines 258-265) - Remove `HasAgentsBlock()` method (lines 267-272) -- Keep `AgentEntry` type (lines 20-24) -- it is still used by `layers.AgentCredentials` for passing app credentials through the install flow. `AgentEntry` describes credentials obtained during app setup, not config.yaml schema. +- Remove `AgentEntry` type (lines 20-24) -- its fields (Role, Name, Slug) are now inlined directly into `layers.AgentCredentials`. **Modify `internal/config/config_test.go`:** diff --git a/docs/superpowers/plans/2026-06-11-triage-prerequisites.md b/docs/superpowers/plans/2026-06-11-triage-prerequisites.md index 1e6f8a0b3..2ba24ccb1 100644 --- a/docs/superpowers/plans/2026-06-11-triage-prerequisites.md +++ b/docs/superpowers/plans/2026-06-11-triage-prerequisites.md @@ -55,7 +55,6 @@ func TestOrgConfig_CreateIssues_OmittedWhenEmpty(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() @@ -71,7 +70,6 @@ func TestOrgConfig_CreateIssues_Marshal(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, Repos: map[string]RepoConfig{}, CreateIssues: &CreateIssuesConfig{ AllowTargets: AllowTargets{ diff --git a/internal/cli/admin.go b/internal/cli/admin.go index dad112a16..473d161f4 100644 --- a/internal/cli/admin.go +++ b/internal/cli/admin.go @@ -1204,7 +1204,7 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or var agentCreds []layers.AgentCredentials for _, role := range roles { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } @@ -1385,16 +1385,7 @@ func runAppSetup(ctx context.Context, client forge.Client, printer *ui.Printer, if err != nil { return nil, fmt.Errorf("setting up app for role %s: %w", role, err) } - creds = append(creds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{ - Role: role, - Name: appCreds.Name, - Slug: appCreds.Slug, - }, - PEM: appCreds.PEM, - ClientID: appCreds.ClientID, - AppID: appCreds.AppID, - }) + creds = append(creds, toAgentCredentials(role, appCreds)) } if err := setup.PermissionErrors(); err != nil { @@ -1783,7 +1774,7 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o var agentCreds []layers.AgentCredentials for _, role := range defaultRoles { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } @@ -1998,6 +1989,17 @@ func loadExistingEnabledRepos(ctx context.Context, client forge.Client, org stri return cfg.EnabledRepos() } +func toAgentCredentials(role string, ac *appsetup.AppCredentials) layers.AgentCredentials { + return layers.AgentCredentials{ + Role: role, + Name: ac.Name, + Slug: ac.Slug, + PEM: ac.PEM, + ClientID: ac.ClientID, + AppID: ac.AppID, + } +} + // filterSlugsByAppSet returns a new map containing only entries whose slug // matches the convention for the given app set (i.e., slug == appSet + "-" + role). // Slugs from a previous install with a different app set must not be carried diff --git a/internal/cli/admin_test.go b/internal/cli/admin_test.go index 7d89a0a30..2047ee2d9 100644 --- a/internal/cli/admin_test.go +++ b/internal/cli/admin_test.go @@ -1826,7 +1826,7 @@ func TestRunInstall_WithSkipMintCheck(t *testing.T) { var agentCreds []layers.AgentCredentials for _, role := range config.DefaultAgentRoles() { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } @@ -1851,7 +1851,7 @@ func TestRunInstall_DiscoversRepos(t *testing.T) { var agentCreds []layers.AgentCredentials for _, role := range config.DefaultAgentRoles() { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } @@ -1899,7 +1899,7 @@ func TestRunInstall_WithVendorAndSkipMint(t *testing.T) { var agentCreds []layers.AgentCredentials for _, role := range config.DefaultAgentRoles() { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } @@ -2738,3 +2738,22 @@ func TestApplyPerRepoScaffold_ProtectedBranch_BranchUpToDate(t *testing.T) { assert.Contains(t, buf.String(), "up to date") } + +func TestToAgentCredentials(t *testing.T) { + ac := &appsetup.AppCredentials{ + AppID: 42, + Slug: "test-slug", + Name: "test-name", + PEM: "pem-data", + ClientID: "client-id", + } + + cred := toAgentCredentials("triage", ac) + + assert.Equal(t, "triage", cred.Role) + assert.Equal(t, "test-name", cred.Name) + assert.Equal(t, "test-slug", cred.Slug) + assert.Equal(t, "pem-data", cred.PEM) + assert.Equal(t, "client-id", cred.ClientID) + assert.Equal(t, 42, cred.AppID) +} diff --git a/internal/cli/github.go b/internal/cli/github.go index 30412b364..6217c7754 100644 --- a/internal/cli/github.go +++ b/internal/cli/github.go @@ -426,7 +426,7 @@ func runGitHubSetupPerOrg(ctx context.Context, client forge.Client, printer *ui. var agentCreds []layers.AgentCredentials for _, role := range roles { agentCreds = append(agentCreds, layers.AgentCredentials{ - AgentEntry: config.AgentEntry{Role: role}, + Role: role, }) } diff --git a/internal/cli/mint_test.go b/internal/cli/mint_test.go index 0ea16b2a0..313e68c02 100644 --- a/internal/cli/mint_test.go +++ b/internal/cli/mint_test.go @@ -1511,7 +1511,7 @@ func TestResolveAddRoleFromBrowser_Success(t *testing.T) { func(_ context.Context, _ forge.Client, _ *ui.Printer, org string, roles []string, _ string, _ string, _ bool, _ map[string]string, _ string, _ map[string]string) ([]layers.AgentCredentials, error) { assert.Equal(t, "acme-corp", org) assert.Equal(t, []string{"review"}, roles) - return []layers.AgentCredentials{{AgentEntry: config.AgentEntry{Slug: "fullsend-ai-review"}, AppID: 424242}}, nil + return []layers.AgentCredentials{{Slug: "fullsend-ai-review", AppID: 424242}}, nil }, ) printer := ui.New(&strings.Builder{}) @@ -1562,7 +1562,7 @@ func TestMintAddRoleCmd_BrowserRegisters(t *testing.T) { withMintAddRoleHooks(t, func() (string, error) { return "test-token", nil }, func(context.Context, forge.Client, *ui.Printer, string, []string, string, string, bool, map[string]string, string, map[string]string) ([]layers.AgentCredentials, error) { - return []layers.AgentCredentials{{AgentEntry: config.AgentEntry{Slug: "fullsend-ai-review"}, AppID: 55555}}, nil + return []layers.AgentCredentials{{Slug: "fullsend-ai-review", AppID: 55555}}, nil }, ) withMintGCFClient(t, gcf.NewFakeGCFClient( diff --git a/internal/config/config.go b/internal/config/config.go index b8ac30f14..bb0325918 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,13 +16,6 @@ const ( DefaultUpstreamRef = "v0" ) -// AgentEntry represents a configured agent with its role and app identity. -type AgentEntry struct { - Role string `yaml:"role"` - Name string `yaml:"name"` - Slug string `yaml:"slug"` -} - // DispatchConfig configures how agent work is dispatched. type DispatchConfig struct { Platform string `yaml:"platform"` @@ -83,7 +76,6 @@ type OrgConfig struct { Dispatch DispatchConfig `yaml:"dispatch"` Inference InferenceConfig `yaml:"inference,omitempty"` Defaults RepoDefaults `yaml:"defaults"` - Agents []AgentEntry `yaml:"agents,omitempty"` Repos map[string]RepoConfig `yaml:"repos"` AllowedRemoteResources []string `yaml:"allowed_remote_resources,omitempty"` CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"` @@ -254,22 +246,6 @@ func (c *OrgConfig) DisabledRepos() []string { return disabled } -// AgentSlugs returns a map of role to slug from the configured agents. -func (c *OrgConfig) AgentSlugs() map[string]string { - slugs := make(map[string]string, len(c.Agents)) - for _, a := range c.Agents { - slugs[a.Role] = a.Slug - } - return slugs -} - -// HasAgentsBlock reports whether the config contains a non-empty agents list. -// CLI commands use this to decide whether to emit a deprecation notice for the -// legacy agents block (see ADR-0045 Phase 3). -func (c *OrgConfig) HasAgentsBlock() bool { - return len(c.Agents) > 0 -} - // DefaultRoles returns the default roles configured for the organization. func (c *OrgConfig) DefaultRoles() []string { return c.Defaults.Roles diff --git a/internal/config/config_test.go b/internal/config/config_test.go index b44077e32..0542c6462 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -60,8 +60,6 @@ func TestNewOrgConfig(t *testing.T) { assert.False(t, cfg.Repos["repo-b"].Enabled) assert.True(t, cfg.Repos["repo-c"].Enabled) - assert.Empty(t, cfg.Agents) - assert.Equal(t, []string{"https://raw.githubusercontent.com/fullsend-ai/fullsend/"}, cfg.AllowedRemoteResources) } @@ -76,9 +74,6 @@ func TestOrgConfigMarshal(t *testing.T) { MaxImplementationRetries: 2, AutoMerge: false, }, - Agents: []AgentEntry{ - {Role: "fullsend", Name: "test-app", Slug: "test-app-slug"}, - }, Repos: map[string]RepoConfig{ "my-repo": {Enabled: true}, }, @@ -225,20 +220,6 @@ func TestOrgConfigDisabledRepos(t *testing.T) { assert.Equal(t, []string{"alpha", "gamma"}, disabled) } -func TestOrgConfigAgentSlugs(t *testing.T) { - cfg := &OrgConfig{ - Agents: []AgentEntry{ - {Role: "fullsend", Name: "app1", Slug: "slug-1"}, - {Role: "coder", Name: "app2", Slug: "slug-2"}, - }, - } - - slugs := cfg.AgentSlugs() - assert.Equal(t, "slug-1", slugs["fullsend"]) - assert.Equal(t, "slug-2", slugs["coder"]) - assert.Len(t, slugs, 2) -} - func TestOrgConfigDefaultRoles(t *testing.T) { cfg := &OrgConfig{ Defaults: RepoDefaults{ @@ -261,10 +242,6 @@ defaults: - coder max_implementation_retries: 3 auto_merge: true -agents: - - role: fullsend - name: my-app - slug: my-app-slug repos: repo-x: enabled: true @@ -280,14 +257,30 @@ repos: assert.Equal(t, 3, cfg.Defaults.MaxImplementationRetries) assert.True(t, cfg.Defaults.AutoMerge) assert.Equal(t, []string{"fullsend", "coder"}, cfg.Defaults.Roles) - assert.Len(t, cfg.Agents, 1) - assert.Equal(t, "fullsend", cfg.Agents[0].Role) - assert.Equal(t, "my-app", cfg.Agents[0].Name) - assert.Equal(t, "my-app-slug", cfg.Agents[0].Slug) assert.True(t, cfg.Repos["repo-x"].Enabled) assert.False(t, cfg.Repos["repo-y"].Enabled) } +func TestParseOrgConfig_IgnoresLegacyAgentsBlock(t *testing.T) { + yamlData := ` +version: "1" +dispatch: + platform: github-actions +defaults: + roles: + - fullsend + max_implementation_retries: 2 +agents: + - role: fullsend + name: my-app + slug: my-app-slug +repos: {} +` + cfg, err := ParseOrgConfig([]byte(yamlData)) + require.NoError(t, err) + assert.Equal(t, "1", cfg.Version) +} + func TestNewOrgConfig_WithInferenceProvider(t *testing.T) { cfg := NewOrgConfig(nil, nil, nil, "vertex", "") assert.Equal(t, "vertex", cfg.Inference.Provider) @@ -369,8 +362,7 @@ func TestOrgConfigMarshal_WithInference(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() @@ -428,8 +420,7 @@ func TestOrgConfigMarshal_KillSwitch(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() @@ -463,8 +454,7 @@ func TestOrgConfigMarshal_KillSwitchOmitEmpty(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() @@ -556,8 +546,7 @@ func TestOrgConfigMarshal_WithDispatchMode(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() @@ -732,7 +721,6 @@ repos: {} Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, Repos: map[string]RepoConfig{}, AllowedRemoteResources: []string{"https://example.com/skills/"}, } @@ -750,8 +738,7 @@ repos: {} Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() require.NoError(t, err) @@ -861,8 +848,7 @@ func TestOrgConfigMarshal_WithStatusNotifications(t *testing.T) { Comment: CommentNotificationConfig{Start: "enabled"}, }, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() require.NoError(t, err) @@ -878,8 +864,7 @@ func TestOrgConfigMarshal_WithoutStatusNotifications(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() require.NoError(t, err) @@ -922,8 +907,7 @@ func TestOrgConfig_CreateIssues_OmittedWhenEmpty(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, } data, err := cfg.Marshal() require.NoError(t, err) @@ -938,8 +922,7 @@ func TestOrgConfig_CreateIssues_Marshal(t *testing.T) { Roles: []string{"fullsend"}, MaxImplementationRetries: 2, }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, + Repos: map[string]RepoConfig{}, CreateIssues: &CreateIssuesConfig{ AllowTargets: AllowTargets{ Orgs: []string{"my-org"}, @@ -1047,101 +1030,6 @@ func TestOrgConfigValidate_CreateIssues_Nil(t *testing.T) { assert.NoError(t, err) } -// --- Agents optional (ADR-0045 Phase 3) --- - -func TestParseOrgConfig_WithoutAgentsBlock(t *testing.T) { - yamlData := ` -version: "1" -dispatch: - platform: github-actions -defaults: - roles: - - fullsend - max_implementation_retries: 2 -repos: {} -` - cfg, err := ParseOrgConfig([]byte(yamlData)) - require.NoError(t, err) - assert.Nil(t, cfg.Agents) - assert.Empty(t, cfg.AgentSlugs()) -} - -func TestParseOrgConfig_EmptyAgentsList(t *testing.T) { - yamlData := ` -version: "1" -dispatch: - platform: github-actions -defaults: - roles: - - fullsend - max_implementation_retries: 2 -agents: [] -repos: {} -` - cfg, err := ParseOrgConfig([]byte(yamlData)) - require.NoError(t, err) - assert.Empty(t, cfg.AgentSlugs()) -} - -func TestHasAgentsBlock(t *testing.T) { - t.Run("returns true when agents has entries", func(t *testing.T) { - cfg := &OrgConfig{ - Agents: []AgentEntry{ - {Role: "fullsend", Name: "app", Slug: "slug"}, - }, - } - assert.True(t, cfg.HasAgentsBlock()) - }) - - t.Run("returns false when agents is nil", func(t *testing.T) { - cfg := &OrgConfig{Agents: nil} - assert.False(t, cfg.HasAgentsBlock()) - }) - - t.Run("returns false when agents is empty slice", func(t *testing.T) { - cfg := &OrgConfig{Agents: []AgentEntry{}} - assert.False(t, cfg.HasAgentsBlock()) - }) -} - -func TestOrgConfigMarshal_NilAgentsOmitted(t *testing.T) { - cfg := &OrgConfig{ - Version: "1", - Dispatch: DispatchConfig{Platform: "github-actions"}, - Defaults: RepoDefaults{ - Roles: []string{"fullsend"}, - MaxImplementationRetries: 2, - }, - Agents: nil, - Repos: map[string]RepoConfig{}, - } - - data, err := cfg.Marshal() - require.NoError(t, err) - assert.NotContains(t, string(data), "agents:") -} - -func TestOrgConfigMarshal_EmptyAgentsOmitted(t *testing.T) { - // yaml.v3 treats empty (non-nil) slices the same as nil for omitempty: - // both are considered "zero" and omitted. This test locks in that behavior. - cfg := &OrgConfig{ - Version: "1", - Dispatch: DispatchConfig{Platform: "github-actions"}, - Defaults: RepoDefaults{ - Roles: []string{"fullsend"}, - MaxImplementationRetries: 2, - }, - Agents: []AgentEntry{}, - Repos: map[string]RepoConfig{}, - } - - data, err := cfg.Marshal() - require.NoError(t, err) - // yaml.v3 omitempty uses Len()==0 for slices, so empty non-nil slices - // are also omitted — same as nil. - assert.NotContains(t, string(data), "agents:") -} - func TestNewOrgConfig_CreateIssuesDefaults(t *testing.T) { cfg := NewOrgConfig(nil, nil, []string{"fullsend"}, "", "my-org") require.NotNil(t, cfg.CreateIssues) diff --git a/internal/layers/harnesswrappers_test.go b/internal/layers/harnesswrappers_test.go index 86955dcb5..fb9750df6 100644 --- a/internal/layers/harnesswrappers_test.go +++ b/internal/layers/harnesswrappers_test.go @@ -8,7 +8,6 @@ import ( "path/filepath" "testing" - "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/harness" "github.com/fullsend-ai/fullsend/internal/scaffold" @@ -26,12 +25,12 @@ func testPrinter() *ui.Printer { func testAgents() []AgentCredentials { return []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "fullsend", Name: "test-fullsend", Slug: "test-fullsend"}}, - {AgentEntry: config.AgentEntry{Role: "triage", Name: "test-triage", Slug: "test-triage"}}, - {AgentEntry: config.AgentEntry{Role: "coder", Name: "test-coder", Slug: "test-coder"}}, - {AgentEntry: config.AgentEntry{Role: "review", Name: "test-review", Slug: "test-review"}}, - {AgentEntry: config.AgentEntry{Role: "retro", Name: "test-retro", Slug: "test-retro"}}, - {AgentEntry: config.AgentEntry{Role: "prioritize", Name: "test-prioritize", Slug: "test-prioritize"}}, + {Role: "fullsend", Name: "test-fullsend", Slug: "test-fullsend"}, + {Role: "triage", Name: "test-triage", Slug: "test-triage"}, + {Role: "coder", Name: "test-coder", Slug: "test-coder"}, + {Role: "review", Name: "test-review", Slug: "test-review"}, + {Role: "retro", Name: "test-retro", Slug: "test-retro"}, + {Role: "prioritize", Name: "test-prioritize", Slug: "test-prioritize"}, } } @@ -119,7 +118,7 @@ func TestHarnessWrappersLayer_Install_GeneratesWrappers(t *testing.T) { func TestHarnessWrappersLayer_Install_WrapperContainsManagedHeader(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -136,7 +135,7 @@ func TestHarnessWrappersLayer_Install_WrapperContainsManagedHeader(t *testing.T) func TestHarnessWrappersLayer_Install_WrapperContainsIntegrityHash(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -153,7 +152,7 @@ func TestHarnessWrappersLayer_Install_SkipsCustomizedFile(t *testing.T) { client.FileContents["org/.fullsend/harness/triage.yaml"] = []byte("agent: agents/custom-triage.md\nmodel: sonnet\n") agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -170,7 +169,7 @@ func TestHarnessWrappersLayer_Install_OverwritesManagedFile(t *testing.T) { client.FileContents["org/.fullsend/harness/triage.yaml"] = []byte("# This file is managed by fullsend. Do not edit it directly.\nbase: https://old-url\nrole: triage\nslug: old-slug\n") agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -188,7 +187,7 @@ func TestHarnessWrappersLayer_Install_CommitFilesError(t *testing.T) { client.Errors["CommitFiles"] = errors.New("network error") agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -209,7 +208,7 @@ func TestHarnessWrappersLayer_Install_NoAgentsNoCommit(t *testing.T) { func TestHarnessWrappersLayer_Install_OnlyFullsendRoleNoCommit(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "fullsend", Name: "fs", Slug: "test-fullsend"}}, + {Role: "fullsend", Name: "fs", Slug: "test-fullsend"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -221,7 +220,7 @@ func TestHarnessWrappersLayer_Install_OnlyFullsendRoleNoCommit(t *testing.T) { func TestHarnessWrappersLayer_Install_WrapperParsesAsValidHarness(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -246,7 +245,7 @@ func TestHarnessWrappersLayer_Install_WrapperParsesAsValidHarness(t *testing.T) func TestHarnessWrappersLayer_Install_BaseURLMatchesScaffold(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -277,7 +276,7 @@ func TestHarnessWrappersLayer_Analyze_DevBuild(t *testing.T) { func TestHarnessWrappersLayer_Analyze_AllPresent(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } client.FileContents["org/.fullsend/harness/triage.yaml"] = []byte("role: triage\n") @@ -290,7 +289,7 @@ func TestHarnessWrappersLayer_Analyze_AllPresent(t *testing.T) { func TestHarnessWrappersLayer_Analyze_AllMissing(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -304,8 +303,8 @@ func TestHarnessWrappersLayer_Analyze_AllMissing(t *testing.T) { func TestHarnessWrappersLayer_Analyze_Degraded(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, - {AgentEntry: config.AgentEntry{Role: "review", Name: "r", Slug: "test-review"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, + {Role: "review", Name: "r", Slug: "test-review"}, } // Only triage exists client.FileContents["org/.fullsend/harness/triage.yaml"] = []byte("role: triage\n") @@ -350,7 +349,7 @@ func TestHarnessesForRole(t *testing.T) { func TestHarnessWrappersLayer_Install_FileMode(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -366,8 +365,8 @@ func TestHarnessWrappersLayer_Install_FileMode(t *testing.T) { func TestHarnessWrappersLayer_Install_CoderFixDedup(t *testing.T) { client := forge.NewFakeClient() agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "coder", Name: "coder-a", Slug: "slug-a"}}, - {AgentEntry: config.AgentEntry{Role: "coder", Name: "coder-b", Slug: "slug-b"}}, + {Role: "coder", Name: "coder-a", Slug: "slug-a"}, + {Role: "coder", Name: "coder-b", Slug: "slug-b"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -389,7 +388,7 @@ func TestHarnessWrappersLayer_Install_LoadExistingHarnessesError(t *testing.T) { client := forge.NewFakeClient() client.Errors["GetFileContent"] = errors.New("permission denied") agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) @@ -403,7 +402,7 @@ func TestHarnessWrappersLayer_Install_IdempotentNoChange(t *testing.T) { changed := false client.CommitFilesChanged = &changed agents := []AgentCredentials{ - {AgentEntry: config.AgentEntry{Role: "triage", Name: "t", Slug: "test-triage"}}, + {Role: "triage", Name: "t", Slug: "test-triage"}, } layer := NewHarnessWrappersLayer("org", client, testPrinter(), agents, testCommitSHA) diff --git a/internal/layers/secrets.go b/internal/layers/secrets.go index 78782604a..2dd401751 100644 --- a/internal/layers/secrets.go +++ b/internal/layers/secrets.go @@ -5,14 +5,15 @@ import ( "fmt" "strings" - "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/ui" ) -// AgentCredentials extends AgentEntry with app credentials. +// AgentCredentials holds agent identity (role, name, slug) and app credentials for layer operations. type AgentCredentials struct { - config.AgentEntry + Role string + Name string + Slug string PEM string ClientID string AppID int diff --git a/internal/layers/secrets_test.go b/internal/layers/secrets_test.go index d8b9bac9a..00c49537b 100644 --- a/internal/layers/secrets_test.go +++ b/internal/layers/secrets_test.go @@ -9,7 +9,6 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/fullsend-ai/fullsend/internal/config" "github.com/fullsend-ai/fullsend/internal/forge" "github.com/fullsend-ai/fullsend/internal/ui" ) @@ -25,14 +24,18 @@ func newSecretsLayer(t *testing.T, client *forge.FakeClient, agents []AgentCrede func twoAgents() []AgentCredentials { return []AgentCredentials{ { - AgentEntry: config.AgentEntry{Role: "fullsend", Name: "FullsendBot", Slug: "fullsend-bot"}, - PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", - ClientID: "Iv1.abc111", + Role: "fullsend", + Name: "FullsendBot", + Slug: "fullsend-bot", + PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", + ClientID: "Iv1.abc111", }, { - AgentEntry: config.AgentEntry{Role: "triage", Name: "TriageBot", Slug: "triage-bot"}, - PEM: "-----BEGIN RSA PRIVATE KEY-----\ntriage-key\n-----END RSA PRIVATE KEY-----", - ClientID: "Iv1.abc222", + Role: "triage", + Name: "TriageBot", + Slug: "triage-bot", + PEM: "-----BEGIN RSA PRIVATE KEY-----\ntriage-key\n-----END RSA PRIVATE KEY-----", + ClientID: "Iv1.abc222", }, } } @@ -81,14 +84,18 @@ func TestSecretsLayer_Install_SkipsEmptyPEM(t *testing.T) { client := &forge.FakeClient{} agents := []AgentCredentials{ { - AgentEntry: config.AgentEntry{Role: "fullsend", Name: "FullsendBot", Slug: "fullsend-bot"}, - PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", - ClientID: "Iv1.abc111", + Role: "fullsend", + Name: "FullsendBot", + Slug: "fullsend-bot", + PEM: "-----BEGIN RSA PRIVATE KEY-----\nfullsend-key\n-----END RSA PRIVATE KEY-----", + ClientID: "Iv1.abc111", }, { - AgentEntry: config.AgentEntry{Role: "triage", Name: "TriageBot", Slug: "triage-bot"}, - PEM: "", // empty — reused from existing app - ClientID: "Iv1.abc222", + Role: "triage", + Name: "TriageBot", + Slug: "triage-bot", + PEM: "", // empty — reused from existing app + ClientID: "Iv1.abc222", }, } layer, _ := newSecretsLayer(t, client, agents) diff --git a/web/admin/src/lib/layers/fixtures/configrepo/config-valid.yaml b/web/admin/src/lib/layers/fixtures/configrepo/config-valid.yaml index ecd097d42..cfe213661 100644 --- a/web/admin/src/lib/layers/fixtures/configrepo/config-valid.yaml +++ b/web/admin/src/lib/layers/fixtures/configrepo/config-valid.yaml @@ -5,5 +5,4 @@ defaults: roles: [fullsend] max_implementation_retries: 2 auto_merge: false -agents: [] repos: {}