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
71 changes: 18 additions & 53 deletions docs/plans/adr-0045-forge-portable-harness-phase4.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ Phase 4 completes the "Remove" milestone from the ADR migration path. Specifical

1. **Require `role` in `Validate()`** -- move from `Lint()` warning to hard error. Harnesses without `role` will fail to load.

2. **Stop writing the `agents:` block during install** -- remove the dual-write. `NewOrgConfig()` will no longer accept an agents parameter. The `ConfigRepoLayer` will write a config.yaml that omits `agents:` entirely.
2. **Stop writing the `agents:` block during install** -- ✅ Shipped (#2447). Removed the `agents` parameter from `NewOrgConfig()`. The `ConfigRepoLayer` writes config.yaml without an `agents:` block.
Comment thread
ggallen marked this conversation as resolved.

3. **Remove `OrgConfig.Agents` field and `AgentSlugs()` method** -- the field and its accessor are dead code after the dual-write stops and all consumers migrate.

Expand All @@ -34,15 +34,15 @@ Phase 3 plan: `docs/plans/adr-0045-forge-portable-harness-phase3.md`
| `AgentSlugs()` method | Remove method |
| `HasAgentsBlock()` method | Remove method |
| Deprecation notice in `runOrgInstall` | Remove notice code |
| Dual-write in `runInstall` / `runGitHubSetup` | Stop passing agents to `NewOrgConfig` |
| Dual-write in `runInstall` / `runGitHubSetup` | Stop passing agents to `NewOrgConfig` (#2447) |
| `HarnessWrappersLayer` generating role/slug | Unchanged -- remains the sole source of agent identity |

### Config schema version: stay on v1

The ADR asks whether removing `agents:` warrants a v2 schema. The recommendation is to stay on v1 for the following reasons:

- **The change is backward-compatible on the read path.** Phase 3 already made `Agents` use `omitempty`. Existing configs without `agents:` parse successfully today. No consumer requires the field to be present -- all have harness-first fallbacks.
- **The change is backward-compatible on the write path.** `NewOrgConfig` will simply not populate the field. `Marshal()` with `omitempty` already omits nil/empty slices.
- **The change is backward-compatible on the write path.** `NewOrgConfig` no longer accepts or populates the field. `Marshal()` with `omitempty` already omits nil/empty slices.
- **A v2 bump would break all existing installations.** `OrgConfig.Validate()` rejects `Version != "1"`. A v2 would require either accepting both versions or migrating every deployed config.yaml, adding complexity for no user-facing benefit.
- **The v1 schema contract (ADR-0011) defines minimum required fields, not an exhaustive field list.** Optional fields with `omitempty` can be added or removed without a version bump.

Expand Down Expand Up @@ -75,13 +75,13 @@ Every consumer of the removed code, and the action taken:
| `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 |
| `NewOrgConfig` agents param | `internal/config/config.go:117` | Accepts `[]AgentEntry`, sets `cfg.Agents` | Remove parameter, stop setting field |
| `NewOrgConfig` caller: `runDryRun` | `internal/cli/admin.go:1196` | Passes `nil` for agents | Remove agents arg |
| `NewOrgConfig` caller: `runInstall` | `internal/cli/admin.go:1513` | Passes agents built from `agentCreds` | Remove agents arg |
| `NewOrgConfig` caller: `runUninstall` | `internal/cli/admin.go:1659` | Passes `nil` for agents | Remove agents arg |
| `NewOrgConfig` caller: `runAnalyze` | `internal/cli/admin.go:1800` | Passes `nil` for agents | Remove agents arg |
| `NewOrgConfig` caller: `runGitHubSetup` (dry-run) | `internal/cli/github.go:437` | Passes `dummyAgents` | Remove agents arg |
| `NewOrgConfig` caller: `runGitHubSetup` (real) | `internal/cli/github.go:487` | Passes `agents` from creds | Remove agents arg |
| `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) |
| `NewOrgConfig` caller: `runUninstall` | `internal/cli/admin.go:1659` | Passes `nil` for agents | Remove agents arg (#2447) |
| `NewOrgConfig` caller: `runAnalyze` | `internal/cli/admin.go:1800` | Passes `nil` for agents | Remove agents arg (#2447) |
| `NewOrgConfig` caller: `runGitHubSetup` (dry-run) | `internal/cli/github.go:437` | Passes `dummyAgents` | Remove agents arg (#2447) |
| `NewOrgConfig` caller: `runGitHubSetup` (real) | `internal/cli/github.go:487` | Passes `agents` from creds | Remove agents arg (#2447) |
| `loadKnownSlugsLegacy` | `internal/cli/admin.go:2064` | Reads `cfg.AgentSlugs()` from config.yaml | Remove function |
| `loadKnownSlugs` legacy fallback | `internal/cli/admin.go:2056` | Calls `loadKnownSlugsLegacy` if harness discovery empty | Remove fallback call |
| `discoverAgentSlugs` tier 2 | `internal/cli/discover_slugs.go:49-66` | Falls back to `cfg.Agents` | Remove fallback block |
Expand Down Expand Up @@ -153,53 +153,18 @@ PRs 1, 2, and 3 can all start in parallel. PR 4 depends on PRs 2 and 3 (all call

---

## PR 2: Stop writing `agents:` block during install
## PR 2: Stop writing `agents:` block during install — ✅ Shipped (#2447)

**Scope:** Remove the `agents` parameter from `NewOrgConfig()`. All `NewOrgConfig` callers stop building and passing agent entries. The `ConfigRepoLayer` writes config.yaml without an `agents:` block. The `HarnessWrappersLayer` remains unchanged -- it is now the sole source of agent identity.

**Modify `internal/config/config.go` -- `NewOrgConfig`:**
- Remove the `agents []AgentEntry` parameter from the function signature:
```go
func NewOrgConfig(allRepos, enabledRepos, roles []string, inferenceProvider, org string) *OrgConfig {
```
- Remove `Agents: agents` from the struct literal inside the function.
- The `Agents` field still exists on `OrgConfig` at this point (removed in PR 4). With `omitempty`, marshaling produces no `agents:` key.

**Modify `internal/cli/admin.go` -- all `NewOrgConfig` callers:**

- `runDryRun` (line ~1196): remove the `nil` agents argument:
```go
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```
- `runInstall` (line ~1508-1513): remove the `agents` slice construction and the agents argument. The lines that build `agents := make([]config.AgentEntry, len(agentCreds))` and populate them are deleted.
```go
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```
- `runUninstall` (line ~1659): remove the `nil` agents argument:
```go
emptyCfg := config.NewOrgConfig(nil, nil, nil, "", "")
```
- `runAnalyze` (line ~1800): remove the `nil` agents argument:
```go
cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, "", org)
```

**Modify `internal/cli/github.go` -- `runGitHubSetup`:**

- Dry-run path (line ~433-437): remove `dummyAgents` construction and the agents argument:
```go
orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```
- Real path (line ~483-487): remove `agents` construction and the agents argument:
```go
orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```

**Modify `internal/config/config_test.go`:**
- Update all `NewOrgConfig` calls in tests to match the new signature (remove agents argument).
- Verify that `Marshal()` output does not contain `agents:`.
All items below were completed in #2447:

**After merge:** `fullsend install` writes config.yaml without an `agents:` block. Agent identity lives exclusively in harness wrapper files. The `HarnessWrappersLayer` (unchanged) continues to write `role:` and `slug:` into harness wrappers.
- ✅ Removed `agents []AgentEntry` parameter from `NewOrgConfig` signature
- ✅ Removed `Agents: agents` from struct literal
- ✅ Updated all `NewOrgConfig` callers in `admin.go` (`runDryRun`, `runInstall`, `runUninstall`, `runAnalyze`)
- ✅ Updated all `NewOrgConfig` callers in `github.go` (`runGitHubSetup` dry-run and real paths)
- ✅ Removed agent entry construction code (`dummyAgents`, `agents` slices built from `agentCreds`)
- ✅ Updated all test files (`config_test.go`, `admin_test.go`, `github_test.go`, `configrepo_test.go`)

---

Expand Down
26 changes: 13 additions & 13 deletions docs/superpowers/plans/2026-06-11-triage-prerequisites.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func TestOrgConfigValidate_CreateIssues_Nil(t *testing.T) {
}

func TestNewOrgConfig_CreateIssuesDefaults(t *testing.T) {
cfg := NewOrgConfig([]string{"repo-a"}, []string{"repo-a"}, []string{"fullsend"}, nil, "", "my-org")
cfg := NewOrgConfig([]string{"repo-a"}, []string{"repo-a"}, []string{"fullsend"}, "", "my-org")
require.NotNil(t, cfg.CreateIssues)
assert.Contains(t, cfg.CreateIssues.AllowTargets.Orgs, "my-org")
assert.Contains(t, cfg.CreateIssues.AllowTargets.Repos, "fullsend-ai/fullsend")
Expand Down Expand Up @@ -226,7 +226,7 @@ CreateIssues *CreateIssuesConfig `yaml:"create_issues,omitempty"`
Change `NewOrgConfig` signature to add `org string` parameter:

```go
func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider, org string) *OrgConfig {
func NewOrgConfig(allRepos, enabledRepos, roles []string, inferenceProvider, org string) *OrgConfig {
Comment thread
ggallen marked this conversation as resolved.
```

Inside the function, after the existing config construction, add:
Expand Down Expand Up @@ -331,32 +331,32 @@ Update each `NewOrgConfig(...)` call to pass the `org` variable as the final arg

In `internal/cli/github.go:464`:
```go
orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, dummyAgents, inferenceProviderName, org)
orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```

In `internal/cli/github.go:513`:
```go
orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org)
orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```

In `internal/cli/admin.go:1174`:
```go
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName, org)
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```

In `internal/cli/admin.go:1502`:
```go
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org)
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
```

In `internal/cli/admin.go:1640`:
```go
emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "", "")
emptyCfg := config.NewOrgConfig(nil, nil, nil, "", "")
```

In `internal/cli/admin.go:1781`:
```go
cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "", org)
cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, "", org)
```

Update each `NewPerRepoConfig(...)` call to pass `cfg.target` (the `owner/repo` string):
Expand All @@ -376,7 +376,7 @@ Update test call sites — these typically pass `""` for the new parameters sinc

In `internal/cli/admin_test.go:583`:
```go
return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, nil, "", "")
return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, "", "")
```

In `internal/cli/admin_test.go:1082`, `1123`:
Expand All @@ -386,15 +386,15 @@ config.NewOrgConfig(..., "")

In `internal/cli/github_test.go:395`:
```go
cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, nil, "", "")
cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, "", "")
```

In `internal/config/config_test.go`, update existing tests that call `NewOrgConfig` without the org param:

`TestNewOrgConfig`: add `""` as last arg.
`TestNewOrgConfig_WithInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, nil, "vertex", "")`.
`TestNewOrgConfig_WithoutInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, nil, "", "")`.
`TestNewOrgConfig_KillSwitchDefaultFalse`: change to `NewOrgConfig(nil, nil, []string{"fullsend"}, nil, "", "")`.
`TestNewOrgConfig_WithInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, "vertex", "")`.
`TestNewOrgConfig_WithoutInferenceProvider`: change to `NewOrgConfig(nil, nil, nil, "", "")`.
`TestNewOrgConfig_KillSwitchDefaultFalse`: change to `NewOrgConfig(nil, nil, []string{"fullsend"}, "", "")`.

In `internal/config/config_test.go`, update existing tests for `NewPerRepoConfig`:

Expand Down
15 changes: 4 additions & 11 deletions internal/cli/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -1192,8 +1192,7 @@ func runDryRun(ctx context.Context, client forge.Client, printer *ui.Printer, or
return err
}

// Build config with empty agents for analysis.
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, nil, inferenceProviderName, org)
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
cfg.Dispatch.Mode = "oidc-mint"

user, err := client.GetAuthenticatedUser(ctx)
Expand Down Expand Up @@ -1504,13 +1503,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o
// Collect IDs for repos that will be enrolled.
enrolledRepoIDs := collectEnrolledRepoIDs(allRepos, enabledRepos)

// Build agent entries for config.
agents := make([]config.AgentEntry, len(agentCreds))
for i, ac := range agentCreds {
agents[i] = ac.AgentEntry
}

cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org)
cfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
cfg.Dispatch.Mode = "oidc-mint"

user, err := client.GetAuthenticatedUser(ctx)
Expand Down Expand Up @@ -1656,7 +1649,7 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer,

// Build a minimal stack for uninstall.
// Only ConfigRepoLayer matters for uninstall since other layers are no-ops.
emptyCfg := config.NewOrgConfig(nil, nil, nil, nil, "", "")
emptyCfg := config.NewOrgConfig(nil, nil, nil, "", "")
stack := layers.NewStack(
layers.NewConfigRepoLayer(org, client, emptyCfg, printer, false),
layers.NewWorkflowsLayer(org, client, printer, "", version, false),
Expand Down Expand Up @@ -1797,7 +1790,7 @@ func runAnalyze(ctx context.Context, client forge.Client, printer *ui.Printer, o
})
}

cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, nil, "", org)
cfg := config.NewOrgConfig(repoNames, nil, defaultRoles, "", org)

user, err := client.GetAuthenticatedUser(ctx)
if err != nil {
Expand Down
4 changes: 1 addition & 3 deletions internal/cli/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -581,7 +581,7 @@ func setupTestConfig(repos map[string]bool) *config.OrgConfig {
// Sort to ensure deterministic order despite map iteration being non-deterministic.
sort.Strings(repoNames)
sort.Strings(enabledRepos)
return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, nil, "", "")
return config.NewOrgConfig(repoNames, enabledRepos, []string{"triage"}, "", "")
}

func setupTestClient(org string, cfg *config.OrgConfig, orgRepos []string) *forge.FakeClient {
Expand Down Expand Up @@ -1084,7 +1084,6 @@ func TestBuildLayerStack_NilEnabledRepos_SkipsDisabledRepos(t *testing.T) {
[]string{"repo-a", "repo-b"},
nil, // nil enabledRepos → all repos are disabled in cfg
[]string{"triage"},
nil,
"",
"",
)
Expand Down Expand Up @@ -1129,7 +1128,6 @@ func TestBuildLayerStack_EmptyEnabledRepos_IncludesDisabledRepos(t *testing.T) {
[]string{"repo-a", "repo-b"},
[]string{}, // explicitly empty → all repos are disabled
[]string{"triage"},
nil,
"",
"",
)
Expand Down
12 changes: 2 additions & 10 deletions internal/cli/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,7 @@ func runGitHubSetupPerOrg(ctx context.Context, client forge.Client, printer *ui.
})
}

dummyAgents := make([]config.AgentEntry, len(agentCreds))
for i, ac := range agentCreds {
dummyAgents[i] = ac.AgentEntry
}
orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, dummyAgents, inferenceProviderName, org)
orgCfg := config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
orgCfg.Dispatch.Mode = "oidc-mint"

user, err := client.GetAuthenticatedUser(ctx)
Expand Down Expand Up @@ -480,11 +476,7 @@ func runGitHubSetupPerOrg(ctx context.Context, client forge.Client, printer *ui.

// Rebuild with real credentials.
agentCreds = creds
agents := make([]config.AgentEntry, len(agentCreds))
for i, ac := range agentCreds {
agents[i] = ac.AgentEntry
}
orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, agents, inferenceProviderName, org)
orgCfg = config.NewOrgConfig(repoNames, enabledRepos, roles, inferenceProviderName, org)
orgCfg.Dispatch.Mode = "oidc-mint"

stack = buildLayerStack(org, client, orgCfg, printer, user, privateRepo, enabledRepos, agentCreds, enrolledRepoIDs, inferenceProvider, cfg.vendor, vendorFn, vendorCollect, "", dispatcher, commitSHA)
Expand Down
2 changes: 1 addition & 1 deletion internal/cli/github_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ func TestRunGitHubStatus_BasicReport(t *testing.T) {
client.Repos = []forge.Repository{
{Name: ".fullsend", FullName: "acme/.fullsend"},
}
cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, nil, "", "")
cfg := config.NewOrgConfig([]string{"widget"}, []string{"widget"}, []string{"triage"}, "", "")
cfgData, _ := cfg.Marshal()
client.FileContents["acme/.fullsend/config.yaml"] = cfgData
client.OrgVariables = map[string]bool{"acme/FULLSEND_MINT_URL": true}
Expand Down
5 changes: 2 additions & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func PerRepoDefaultRoles() []string {
}

Comment thread
ggallen marked this conversation as resolved.
// NewOrgConfig creates a new OrgConfig with sensible defaults.
func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, inferenceProvider, org string) *OrgConfig {
func NewOrgConfig(allRepos, enabledRepos, roles []string, inferenceProvider, org string) *OrgConfig {
repos := make(map[string]RepoConfig, len(allRepos))
for _, r := range allRepos {
repos[r] = RepoConfig{
Expand All @@ -132,8 +132,7 @@ func NewOrgConfig(allRepos, enabledRepos, roles []string, agents []AgentEntry, i
MaxImplementationRetries: 2,
AutoMerge: false,
},
Agents: agents,
Repos: repos,
Repos: repos,
// Default allowlist for base: composition in harness wrappers (ADR-0045 Phase 2).
AllowedRemoteResources: []string{
"https://raw.githubusercontent.com/fullsend-ai/fullsend/",
Expand Down
Loading
Loading