Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 7 additions & 9 deletions docs/ADRs/0045-forge-portable-harness-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 5 additions & 5 deletions docs/plans/adr-0045-forge-portable-harness-phase4.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) |
Expand Down Expand Up @@ -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`:**

Expand Down
2 changes: 0 additions & 2 deletions docs/superpowers/plans/2026-06-11-triage-prerequisites.md
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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{
Expand Down
26 changes: 14 additions & 12 deletions internal/cli/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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
Expand Down
25 changes: 22 additions & 3 deletions internal/cli/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}

Expand All @@ -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,
})
}

Expand Down Expand Up @@ -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,
})
}

Expand Down Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion internal/cli/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
})
}

Expand Down
4 changes: 2 additions & 2 deletions internal/cli/mint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand Down Expand Up @@ -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(
Expand Down
24 changes: 0 additions & 24 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand Down Expand Up @@ -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"`
Comment thread
ggallen marked this conversation as resolved.
Repos map[string]RepoConfig `yaml:"repos"`
AllowedRemoteResources []string `yaml:"allowed_remote_resources,omitempty"`
CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"`
Expand Down Expand Up @@ -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
Expand Down
Loading
Loading