Skip to content

refactor(config): remove legacy agent discovery fallbacks (ADR-0045 Phase 4 PR 3) #2448

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ggallen wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(config): remove legacy agent discovery fallbacks (ADR-0045 Phase 4 PR 3) #2448

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
64 changes: 22 additions & 42 deletions internal/cli/admin.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1585,8 +1585,7 @@ func runInstall(ctx context.Context, client forge.Client, printer *ui.Printer, o

// runUninstall tears down the fullsend installation.
func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer, org, appSet string, browser appsetup.BrowserOpener, stdin io.Reader) error {
// Try to discover agent slugs. Prefer harness wrapper files, then
// fall back to config.yaml agents: block, then default naming.
// Try to discover agent slugs from harness wrapper files, then default naming.
// If the .fullsend repo is already gone (e.g., previous partial
// uninstall), fall back to the default naming convention so we can
// still guide the user to delete the apps. Without this fallback,
Expand All @@ -1595,19 +1594,17 @@ func runUninstall(ctx context.Context, client forge.Client, printer *ui.Printer,
var agentSlugs []string
var configMode string
var enrolledRepos []string
var parsedCfg *config.OrgConfig
cfgData, err := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml")
if err == nil {
if parsed, parseErr := config.ParseOrgConfig(cfgData); parseErr == nil {
parsedCfg = parsed
configMode = parsed.Dispatch.Mode
enrolledRepos = parsed.EnabledRepos()
} else {
printer.StepWarn(fmt.Sprintf("Could not parse existing config: %v; using defaults", parseErr))
}
}

agentSlugs = discoverAgentSlugs(ctx, client, org, forge.ConfigRepoName, "main", appSet, parsedCfg, printer)
agentSlugs = discoverAgentSlugs(ctx, client, org, forge.ConfigRepoName, "main", appSet, printer)

if len(agentSlugs) == 0 {
// Neither harness files nor config agents found — assume default
Expand Down Expand Up @@ -2024,53 +2021,36 @@ func filterSlugsByAppSet(slugs map[string]string, appSet string) map[string]stri
}

// loadKnownSlugs discovers agent slugs from harness wrapper files in the
// config repo, falling back to the config.yaml agents: block.
// config repo.
func loadKnownSlugs(ctx context.Context, client forge.Client, org, configRepo, ref string, printer *ui.Printer) map[string]string {
agents, err := harness.DiscoverRemoteAgents(ctx, client, org, configRepo, ref)
if err != nil {
printer.StepWarn(fmt.Sprintf("harness discovery: %v", err))
}
if len(agents) > 0 {
slugs := make(map[string]string, len(agents))
seen := make(map[string]bool, len(agents))
for _, a := range agents {
if a.Role == "" && a.Slug == "" {
continue
}
if a.Role == "" || a.Slug == "" {
printer.StepWarn(fmt.Sprintf("harness %s has role=%q slug=%q; both must be set", a.Filename, a.Role, a.Slug))
continue
}
if seen[a.Role] {
printer.StepInfo(fmt.Sprintf("duplicate role %q in harness file %s, using first occurrence", a.Role, a.Filename))
continue
}
seen[a.Role] = true
slugs[a.Role] = a.Slug
if len(agents) == 0 {
return nil
}
slugs := make(map[string]string, len(agents))
seen := make(map[string]bool, len(agents))
for _, a := range agents {
if a.Role == "" && a.Slug == "" {
continue
}
if len(slugs) > 0 {
return slugs
if a.Role == "" || a.Slug == "" {
printer.StepWarn(fmt.Sprintf("harness %s has role=%q slug=%q; both must be set", a.Filename, a.Role, a.Slug))
continue
}
if seen[a.Role] {
printer.StepInfo(fmt.Sprintf("duplicate role %q in harness file %s, using first occurrence", a.Role, a.Filename))
continue
}
seen[a.Role] = true
slugs[a.Role] = a.Slug
}

slugs := loadKnownSlugsLegacy(ctx, client, org)
if len(slugs) > 0 {
printer.StepWarn("config.yaml agents: block is deprecated; agent identity should be in harness files with role/slug fields")
}
return slugs
}

// loadKnownSlugsLegacy reads agent slugs from the config.yaml agents: block.
func loadKnownSlugsLegacy(ctx context.Context, client forge.Client, org string) map[string]string {
data, err := client.GetFileContent(ctx, org, forge.ConfigRepoName, "config.yaml")
if err != nil {
return nil
}
cfg, err := config.ParseOrgConfig(data)
if err != nil {
return nil
return slugs
}
return cfg.AgentSlugs()
return nil
}

// collectEnrolledRepoIDs returns the IDs of repos whose names appear in
Expand Down
104 changes: 13 additions & 91 deletions internal/cli/admin_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2191,17 +2191,18 @@ func TestRunUninstall_UsesHarnessDiscovery(t *testing.T) {
assert.NotContains(t, output, "agents: block")
}

func TestRunUninstall_FallsBackToAgentsBlockWithWarning(t *testing.T) {
func TestRunUninstall_NoHarnessFiles_FallsBackToDefaultNaming(t *testing.T) {
client := forge.NewFakeClient()
client.TokenScopes = []string{"admin:org", "repo", "delete_repo"}

// Provide config.yaml with agents: block but no harness directory.
// Provide config.yaml but no harness directory — discoverAgentSlugs
// returns nil, and runUninstall falls back to default naming.
client.FileContents = map[string][]byte{
"test-org/.fullsend/config.yaml": []byte("version: v1\ndispatch:\n platform: github-actions\nagents:\n - role: triage\n slug: cfg-triage\n"),
"test-org/.fullsend/config.yaml": []byte("version: v1\ndispatch:\n platform: github-actions\n"),
}

client.Installations = []forge.Installation{
{ID: 1, AppSlug: "cfg-triage"},
{ID: 1, AppSlug: "fullsend-ai-triage"},
}

var buf strings.Builder
Expand All @@ -2211,8 +2212,7 @@ func TestRunUninstall_FallsBackToAgentsBlockWithWarning(t *testing.T) {
require.NoError(t, err)

output := buf.String()
assert.Contains(t, output, "cfg-triage")
assert.Contains(t, output, "agents: block")
assert.Contains(t, output, "fullsend-ai-triage")
}

func TestAwaitRepoMaintenance_Success(t *testing.T) {
Expand Down Expand Up @@ -2610,7 +2610,7 @@ func TestApplyPerRepoScaffold_ProtectedBranch_DuplicatePR(t *testing.T) {
assert.Contains(t, output, "Merge the PR")
}

func TestLoadKnownSlugs_HarnessFilesPreferred(t *testing.T) {
func TestLoadKnownSlugs_HarnessFiles(t *testing.T) {
client := forge.NewFakeClient()
client.DirContents["myorg/.fullsend/harness@HEAD"] = []forge.DirectoryEntry{
{Path: "harness/triage.yaml", Type: "file"},
Expand All @@ -2619,14 +2619,6 @@ func TestLoadKnownSlugs_HarnessFilesPreferred(t *testing.T) {
client.FileContentsRef["myorg/.fullsend/harness/triage.yaml@HEAD"] = []byte("role: triage\nslug: fullsend-ai-triage\n")
client.FileContentsRef["myorg/.fullsend/harness/coder.yaml@HEAD"] = []byte("role: coder\nslug: fullsend-ai-coder\n")

// Also set up config.yaml agents: block — should NOT be used.
client.FileContents["myorg/.fullsend/config.yaml"] = []byte(`version: "1"
agents:
- role: triage
slug: old-triage-slug
name: old-triage
`)

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)
Expand All @@ -2635,69 +2627,30 @@ agents:
"triage": "fullsend-ai-triage",
"coder": "fullsend-ai-coder",
}, slugs)
assert.NotContains(t, buf.String(), "agents: block")
}

func TestLoadKnownSlugs_FallbackToAgentsBlock(t *testing.T) {
func TestLoadKnownSlugs_NoHarnessFiles_ReturnsNil(t *testing.T) {
client := forge.NewFakeClient()
// No harness/ directory → ErrNotFound from DirContents.

client.FileContents["myorg/.fullsend/config.yaml"] = []byte(`version: "1"
agents:
- role: triage
slug: fullsend-ai-triage
name: fullsend-ai-triage
- role: coder
slug: fullsend-ai-coder
name: fullsend-ai-coder
`)

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Equal(t, map[string]string{
"triage": "fullsend-ai-triage",
"coder": "fullsend-ai-coder",
}, slugs)
assert.Contains(t, buf.String(), "agents: block")
assert.Nil(t, slugs)
}

func TestLoadKnownSlugs_HarnessFilesWithoutRoleSlug_FallsBack(t *testing.T) {
func TestLoadKnownSlugs_HarnessFilesWithoutRoleSlug_ReturnsNil(t *testing.T) {
client := forge.NewFakeClient()
// Harness files exist but lack role/slug (legacy format).
client.DirContents["myorg/.fullsend/harness@HEAD"] = []forge.DirectoryEntry{
{Path: "harness/triage.yaml", Type: "file"},
}
client.FileContentsRef["myorg/.fullsend/harness/triage.yaml@HEAD"] = []byte("agent: agents/triage.md\nmodel: opus\n")

client.FileContents["myorg/.fullsend/config.yaml"] = []byte(`version: "1"
agents:
- role: triage
slug: fullsend-ai-triage
name: fullsend-ai-triage
`)

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Equal(t, map[string]string{
"triage": "fullsend-ai-triage",
}, slugs)
assert.Contains(t, buf.String(), "agents: block")
}

func TestLoadKnownSlugs_NeitherSource_ReturnsNil(t *testing.T) {
client := forge.NewFakeClient()
// No harness/ dir, no config.yaml.

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Nil(t, slugs)
assert.NotContains(t, buf.String(), "agents: block")
}

func TestLoadKnownSlugs_DuplicateRoles_FirstWins(t *testing.T) {
Expand Down Expand Up @@ -2747,55 +2700,24 @@ func TestLoadKnownSlugs_RoleWithoutSlug_WarnsAndSkips(t *testing.T) {
}
client.FileContentsRef["myorg/.fullsend/harness/triage.yaml@HEAD"] = []byte("role: triage\n")

client.FileContents["myorg/.fullsend/config.yaml"] = []byte(`version: "1"
agents:
- role: triage
slug: fullsend-ai-triage
name: fullsend-ai-triage
`)

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Equal(t, map[string]string{
"triage": "fullsend-ai-triage",
}, slugs)
assert.Nil(t, slugs)
assert.Contains(t, buf.String(), "both must be set")
}

func TestLoadKnownSlugs_HardError_ZeroAgents_FallsBack(t *testing.T) {
func TestLoadKnownSlugs_HardError_ReturnsNil(t *testing.T) {
client := forge.NewFakeClient()
client.Errors["ListDirectoryContents"] = fmt.Errorf("network timeout")

client.FileContents["myorg/.fullsend/config.yaml"] = []byte(`version: "1"
agents:
- role: triage
slug: fullsend-ai-triage
name: fullsend-ai-triage
`)

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Equal(t, map[string]string{
"triage": "fullsend-ai-triage",
}, slugs)
assert.Contains(t, buf.String(), "harness discovery")
assert.Contains(t, buf.String(), "deprecated")
}

func TestLoadKnownSlugs_MalformedConfig_ReturnsNil(t *testing.T) {
client := forge.NewFakeClient()
// No harness/ dir, malformed config.yaml.
client.FileContents["myorg/.fullsend/config.yaml"] = []byte("not: valid: yaml: [")

var buf bytes.Buffer
printer := ui.New(&buf)
slugs := loadKnownSlugs(context.Background(), client, "myorg", forge.ConfigRepoName, "HEAD", printer)

assert.Nil(t, slugs)
assert.Contains(t, buf.String(), "harness discovery")
}

func TestApplyPerRepoScaffold_ProtectedBranch_BranchUpToDate(t *testing.T) {
Expand Down
31 changes: 4 additions & 27 deletions internal/cli/discover_slugs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,18 @@ import (
"fmt"

"github.com/fullsend-ai/fullsend/internal/appsetup"
"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/ui"
)

// discoverAgentSlugs discovers agent slugs using a three-tier fallback:
// discoverAgentSlugs discovers agent slugs from harness wrapper files in the
// config repo. Returns nil when no slugs are found — the caller is responsible
// for its own default-role fallback.
//
// 1. Harness wrapper files in the config repo (via DiscoverRemoteAgents)
// 2. config.yaml agents: block (legacy, emits deprecation warning)
// 3. Empty — caller is responsible for its own default-role fallback
//
// The ref parameter specifies the git ref for harness directory discovery.
// When an agent has a role but no slug, the slug is derived from appSet and
// the role using the standard naming convention.
func discoverAgentSlugs(ctx context.Context, client forge.Client, owner, configRepo, ref, appSet string, cfg *config.OrgConfig, printer *ui.Printer) []string {
func discoverAgentSlugs(ctx context.Context, client forge.Client, owner, configRepo, ref, appSet string, printer *ui.Printer) []string {
agents, err := harness.DiscoverRemoteAgents(ctx, client, owner, configRepo, ref)
if err != nil {
printer.StepWarn(fmt.Sprintf("some harness files could not be read: %v", err))
Comment thread
ggallen marked this conversation as resolved.
Expand All @@ -46,24 +42,5 @@ func discoverAgentSlugs(ctx context.Context, client forge.Client, owner, configR
}
}

if cfg != nil && cfg.HasAgentsBlock() {
printer.StepWarn("agent identity read from config.yaml agents: block; migrate to harness files with role/slug fields")
var slugs []string
seen := make(map[string]bool, len(cfg.Agents))
for _, a := range cfg.Agents {
slug := a.Slug
if slug == "" && a.Role != "" {
slug = appsetup.AppSlug(appSet, a.Role)
}
if slug != "" && !seen[slug] {
seen[slug] = true
slugs = append(slugs, slug)
}
}
if len(slugs) > 0 {
return slugs
}
}

return nil
}
Loading
Loading