From 5211fc9900469d2b478cb057ab7368a96cd77f20 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:17:58 +0000 Subject: [PATCH 01/33] fix(quickstart): document agent's allowed values to moat init --- internal/config/config.go | 2 +- internal/quickstart/prompt.go | 3 +- internal/quickstart/quickstart_test.go | 40 ++++++++++++++++++++++++++ internal/quickstart/schema.go | 4 +++ 4 files changed, 47 insertions(+), 2 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 9a01371a..b44138ab 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -32,7 +32,7 @@ var imageRefRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._\-/:]*(@sha256:[a-f // Config represents a moat.yaml manifest. type Config struct { Name string `yaml:"name,omitempty"` - Agent string `yaml:"agent"` + Agent string `yaml:"agent" doc:"one of: claude, claude-code, codex, copilot, gemini, pi. Omit unless the project pins a specific agent."` Version string `yaml:"version,omitempty"` Dependencies []string `yaml:"dependencies,omitempty"` Grants []string `yaml:"grants,omitempty"` diff --git a/internal/quickstart/prompt.go b/internal/quickstart/prompt.go index 91bc3ba5..42213fee 100644 --- a/internal/quickstart/prompt.go +++ b/internal/quickstart/prompt.go @@ -71,7 +71,8 @@ func BuildPrompt(workspace string) string { b.WriteString("9. Keep the config minimal — only include what the project actually needs, but don't miss dependencies used by tests or build scripts.\n") b.WriteString("10. Use pre_run hooks for dependency installation (npm install, pip install, etc.).\n") b.WriteString("11. Use post_build_root hooks only for system packages not available as dependencies.\n") - b.WriteString("12. Output only valid YAML, nothing else. No markdown fences, no explanation.\n") + b.WriteString("12. Do not invent a value for `agent`. Omit it unless the project genuinely pins one agent; it accepts only the listed agent names, not a project name.\n") + b.WriteString("13. Output only valid YAML, nothing else. No markdown fences, no explanation.\n") return b.String() } diff --git a/internal/quickstart/quickstart_test.go b/internal/quickstart/quickstart_test.go index f54b230b..cdbe6c24 100644 --- a/internal/quickstart/quickstart_test.go +++ b/internal/quickstart/quickstart_test.go @@ -1,6 +1,7 @@ package quickstart import ( + "reflect" "strings" "testing" ) @@ -105,6 +106,45 @@ func TestBuildPrompt(t *testing.T) { } } +func TestWalkStructDocTag(t *testing.T) { + type sample struct { + Tagged string `yaml:"tagged" doc:"one of: a, b, c."` + Untagged string `yaml:"untagged"` + } + + var b strings.Builder + walkStruct(reflect.TypeOf(sample{}), "", &b) + got := b.String() + + // A doc-tagged field renders its guidance inline. + if !strings.Contains(got, "- `tagged` (string) — one of: a, b, c.") { + t.Errorf("tagged field missing doc text; got:\n%s", got) + } + // Companion: an untagged field still renders the bare declaration. + if !strings.Contains(got, "- `untagged` (string)\n") { + t.Errorf("untagged field should render bare; got:\n%s", got) + } + if strings.Contains(got, "- `untagged` (string) —") { + t.Errorf("untagged field must not gain a dash suffix; got:\n%s", got) + } +} + +func TestSchemaReferenceDocumentsAgentFields(t *testing.T) { + ref := GenerateSchemaReference() + for _, want := range []string{"claude-code", "codex", "gemini"} { + if !strings.Contains(ref, want) { + t.Errorf("schema reference should list allowed agent value %q", want) + } + } +} + +func TestPromptForbidsInventingAgentValues(t *testing.T) { + p := BuildPrompt(t.TempDir()) + if !strings.Contains(p, "Do not invent") { + t.Error("prompt should instruct the model not to invent agent values") + } +} + func TestGenerateDepsReference(t *testing.T) { ref := GenerateDepsReference() diff --git a/internal/quickstart/schema.go b/internal/quickstart/schema.go index 75e124fb..cf390129 100644 --- a/internal/quickstart/schema.go +++ b/internal/quickstart/schema.go @@ -54,6 +54,10 @@ func walkStruct(t reflect.Type, prefix string, b *strings.Builder) { continue } + if doc := f.Tag.Get("doc"); doc != "" { + fmt.Fprintf(b, "- `%s` (%s) — %s\n", fullName, friendlyType(ft), doc) + continue + } fmt.Fprintf(b, "- `%s` (%s)\n", fullName, friendlyType(ft)) } } From 2a6a8923782faab47c9cc42299fa4956a8d19139 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:24:23 +0000 Subject: [PATCH 02/33] feat(cli): validate moat.yaml agent against the provider registry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds CanonicalAgent, KnownAgentNames, and ValidateAgent so an unknown agent: value (e.g. a hallucinated or project-shaped string) is caught and cleared with a warning instead of silently disabling agent-specific behavior. Also adds ui.Writer() to pair with ui.SetWriter, since SetWriter has no "reset to default" sentinel — passing nil leaves the writer nil and panics on the next Warn/Error/Info call. Tests capture the prior writer via ui.Writer() and restore it instead. --- internal/cli/agents.go | 77 +++++++++++++++++++++++++++ internal/cli/agents_test.go | 100 ++++++++++++++++++++++++++++++++++++ internal/ui/ui.go | 8 +++ 3 files changed, 185 insertions(+) create mode 100644 internal/cli/agents.go create mode 100644 internal/cli/agents_test.go diff --git a/internal/cli/agents.go b/internal/cli/agents.go new file mode 100644 index 00000000..25905964 --- /dev/null +++ b/internal/cli/agents.go @@ -0,0 +1,77 @@ +package cli + +import ( + "sort" + "strings" + + "github.com/majorcontext/moat/internal/config" + "github.com/majorcontext/moat/internal/provider" + "github.com/majorcontext/moat/internal/ui" +) + +// agentVariants maps documented agent-name variants onto their registered +// provider name. The provider registry has no alias for these, but they are +// long-standing valid moat.yaml values: the claude join gate accepts +// "claude-code" (internal/providers/claude/join.go) and storage metadata +// documents it as the example agent value. +var agentVariants = map[string]string{ + "claude-code": "claude", +} + +// CanonicalAgent resolves an agent name to its registered provider name, +// accepting registry aliases (openai -> codex) and documented variants +// (claude-code -> claude). Returns "" when name is not a known agent. +// +// provider.ResolveName alone cannot validate: it returns unknown input +// unchanged. provider.GetAgent is the membership test, and it also excludes +// non-agent providers (github, aws, oauth, …) that a bare registry lookup +// would wrongly accept. +func CanonicalAgent(name string) string { + if name == "" { + return "" + } + if v, ok := agentVariants[name]; ok { + name = v + } + resolved := provider.ResolveName(name) + if provider.GetAgent(resolved) == nil { + return "" + } + return resolved +} + +// KnownAgentNames returns the sorted set of values accepted by `agent:`. +func KnownAgentNames() []string { + seen := make(map[string]bool) + for _, a := range provider.Agents() { + seen[a.Name()] = true + } + for variant := range agentVariants { + seen[variant] = true + } + names := make([]string, 0, len(seen)) + for n := range seen { + names = append(names, n) + } + sort.Strings(names) + return names +} + +// ValidateAgent warns and clears cfg.Agent when it names something that is not +// a known agent. It warns rather than failing: moat init and the reference docs +// have both generated project-shaped values in the wild, and those runs work +// today apart from the silent degradation. Clearing the field lets the CLI verb +// backfill a correct value, so the run self-heals without the user editing +// moat.yaml. +func ValidateAgent(cfg *config.Config) { + if cfg == nil || cfg.Agent == "" { + return + } + if CanonicalAgent(cfg.Agent) != "" { + return + } + ui.Warnf("moat.yaml `agent: %s` is not a known agent (valid: %s) — ignoring.\n"+ + "Remove the field or set `agent: claude`.", + cfg.Agent, strings.Join(KnownAgentNames(), ", ")) + cfg.Agent = "" +} diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go new file mode 100644 index 00000000..66d57c46 --- /dev/null +++ b/internal/cli/agents_test.go @@ -0,0 +1,100 @@ +// Package cli_test is an external test package (not `package cli`) because +// every agent provider package (claude, codex, gemini, copilot, pi) imports +// internal/cli itself, to register its CLI command via a cli.go file in that +// package. An internal test file (package cli) blank-importing +// internal/providers to populate the registry would therefore create a real +// import cycle: cli -> providers -> providers/claude -> cli. As an external +// test package, cli_test is a distinct package from cli, so it can import +// internal/providers without cycling back. It only needs the exported +// surface (CanonicalAgent, KnownAgentNames, ValidateAgent), so nothing here +// requires internal access. +package cli_test + +import ( + "bytes" + "strings" + "testing" + + "github.com/majorcontext/moat/internal/cli" + "github.com/majorcontext/moat/internal/config" + "github.com/majorcontext/moat/internal/ui" + + // Registers all credential/agent providers (claude, codex, github, ...) via + // import side effects, matching the pattern in + // internal/provider/interfaces_test.go. Without this, the registry is empty + // under test and CanonicalAgent/KnownAgentNames/ValidateAgent see no agents. + _ "github.com/majorcontext/moat/internal/providers" +) + +func TestCanonicalAgent(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"registered agent", "claude", "claude"}, + {"documented variant", "claude-code", "claude"}, + {"registry alias", "openai", "codex"}, + {"hallucinated value", "vibrant-code", ""}, + {"non-agent provider", "github", ""}, + {"empty", "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := cli.CanonicalAgent(tt.input); got != tt.want { + t.Errorf("CanonicalAgent(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestKnownAgentNamesIncludesVariants(t *testing.T) { + names := cli.KnownAgentNames() + joined := strings.Join(names, ",") + for _, want := range []string{"claude", "claude-code", "codex"} { + if !strings.Contains(joined, want) { + t.Errorf("KnownAgentNames() = %v, missing %q", names, want) + } + } + // Companion: non-agent providers must not leak into the valid set. + if strings.Contains(joined, "github") { + t.Errorf("KnownAgentNames() = %v, should not include non-agent providers", names) + } +} + +func TestValidateAgent(t *testing.T) { + tests := []struct { + name string + agent string + wantAgent string + wantWarn bool + }{ + {"unknown is cleared and warned", "vibrant-code", "", true}, + {"valid is preserved", "claude", "claude", false}, + {"documented variant is preserved", "claude-code", "claude-code", false}, + {"non-agent provider is cleared", "github", "", true}, + {"empty is untouched and silent", "", "", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cfg := &config.Config{Agent: tt.agent} + cli.ValidateAgent(cfg) + + if cfg.Agent != tt.wantAgent { + t.Errorf("cfg.Agent = %q, want %q", cfg.Agent, tt.wantAgent) + } + gotWarn := buf.Len() > 0 + if gotWarn != tt.wantWarn { + t.Errorf("warned = %v, want %v (output: %q)", gotWarn, tt.wantWarn, buf.String()) + } + if tt.wantWarn && !strings.Contains(buf.String(), tt.agent) { + t.Errorf("warning should name the offending value %q; got %q", tt.agent, buf.String()) + } + }) + } +} diff --git a/internal/ui/ui.go b/internal/ui/ui.go index acc881b1..dc9d52a4 100644 --- a/internal/ui/ui.go +++ b/internal/ui/ui.go @@ -16,6 +16,14 @@ func SetWriter(w io.Writer) { writer = w } +// Writer returns the current output writer (for testing, to save/restore +// around SetWriter). There is no "reset to default" sentinel: passing nil to +// SetWriter would leave writer nil and panic on the next Warn/Error/Info +// call, so callers must capture the prior value here and restore it. +func Writer() io.Writer { + return writer +} + // --- Color detection --- var ( From 1df21f4ac45d9e3640f06dea06de3ebdaf2f9271 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:31:33 +0000 Subject: [PATCH 03/33] fix(cli): let the CLI verb win over moat.yaml agent --- cmd/moat/cli/run.go | 5 +++ internal/cli/agents.go | 22 +++++++++++++ internal/cli/agents_test.go | 58 +++++++++++++++++++++++++++++++++++ internal/cli/provider.go | 21 +++++++++---- internal/cli/provider_test.go | 27 ++++++++++++++++ 5 files changed, 127 insertions(+), 6 deletions(-) diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index da9defa2..4b0dd895 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -6,6 +6,7 @@ import ( "os" "path/filepath" + intcli "github.com/majorcontext/moat/internal/cli" "github.com/majorcontext/moat/internal/config" "github.com/majorcontext/moat/internal/log" "github.com/majorcontext/moat/internal/ui" @@ -167,6 +168,10 @@ func runAgent(cmd *cobra.Command, args []string) error { ctx := context.Background() + // moat run has no verb, so a valid agent: is preserved and an invalid one + // warns and is cleared. + intcli.ResolveAgentField(cfg, "") + opts := ExecOptions{ Flags: runFlags, Workspace: absPath, diff --git a/internal/cli/agents.go b/internal/cli/agents.go index 25905964..62f0f294 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -75,3 +75,25 @@ func ValidateAgent(cfg *config.Config) { cfg.Agent, strings.Join(KnownAgentNames(), ", ")) cfg.Agent = "" } + +// ResolveAgentField normalizes cfg.Agent. verb is the provider name the user +// typed (e.g. "claude"), or "" for `moat run`, which has no verb. +// +// The verb always wins when there is one: if the user typed `moat claude`, the +// run is claude regardless of what moat.yaml says. Before this, a moat.yaml +// value silently overrode the command line, so `moat claude` could record a run +// as codex. +func ResolveAgentField(cfg *config.Config, verb string) { + if cfg == nil { + return + } + ValidateAgent(cfg) + if verb == "" { + return + } + if cfg.Agent != "" && CanonicalAgent(cfg.Agent) != CanonicalAgent(verb) { + ui.Warnf("moat.yaml `agent: %s` conflicts with `moat %s` — using %s.", + cfg.Agent, verb, verb) + } + cfg.Agent = verb +} diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index 66d57c46..adcac963 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -98,3 +98,61 @@ func TestValidateAgent(t *testing.T) { }) } } + +func TestResolveAgentField(t *testing.T) { + tests := []struct { + name string + agent string + verb string + wantAgent string + wantWarn bool + }{ + {"verb backfills an empty field", "", "claude", "claude", false}, + {"verb overrides a conflicting value", "codex", "claude", "claude", true}, + {"verb agrees with the field", "claude", "claude", "claude", false}, + {"verb agrees via variant", "claude-code", "claude", "claude", false}, + {"moat run keeps a valid field", "codex", "", "codex", false}, + {"moat run clears an invalid field", "vibrant-code", "", "", true}, + } + // The point of this fix is that the five HasPrefix call sites downstream + // (isAIAgent, agentImpliedDependencies, language_servers, copilot init, pi + // staging) receive a usable value. TestAgentFieldReachesDegradationSites + // below asserts that directly. + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cfg := &config.Config{Agent: tt.agent} + cli.ResolveAgentField(cfg, tt.verb) + + if cfg.Agent != tt.wantAgent { + t.Errorf("cfg.Agent = %q, want %q", cfg.Agent, tt.wantAgent) + } + if gotWarn := buf.Len() > 0; gotWarn != tt.wantWarn { + t.Errorf("warned = %v, want %v (output: %q)", gotWarn, tt.wantWarn, buf.String()) + } + }) + } +} + +func TestAgentFieldReachesDegradationSites(t *testing.T) { + // isAIAgent is the cheapest observable proxy for the five HasPrefix call + // sites in manager_create.go that a bogus agent: silently disabled. + cfg := &config.Config{Agent: "vibrant-code"} + cli.ResolveAgentField(cfg, "claude") + if !strings.HasPrefix(cfg.Agent, "claude") { + t.Errorf("agent %q must satisfy the HasPrefix checks that gate memory "+ + "limits, implied deps, and language servers", cfg.Agent) + } + + // Companion: moat run with no verb and no valid field leaves it empty, and + // those sites correctly stay off rather than matching something wrong. + bare := &config.Config{Agent: "vibrant-code"} + cli.ResolveAgentField(bare, "") + if bare.Agent != "" { + t.Errorf("cfg.Agent = %q, want empty so the HasPrefix sites stay off", bare.Agent) + } +} diff --git a/internal/cli/provider.go b/internal/cli/provider.go index 37eea73e..663267b8 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -213,12 +213,9 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error ctx := context.Background() - // Ensure the agent name is set so the manager can apply agent-specific - // defaults (e.g., memory limits). When there's no moat.yaml, cfg.Agent - // is empty — fill it from the provider name (e.g., "claude", "codex"). - if cfg.Agent == "" { - cfg.Agent = rc.Name - } + // The verb the user typed always names the agent. ValidateAgent runs inside + // so an unknown moat.yaml value warns once and is discarded. + ResolveAgentField(cfg, agentVerbFor(rc.Name, cfg)) opts := ExecOptions{ Flags: *rc.Flags, @@ -239,6 +236,18 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error return err } +// agentVerbFor selects the verb passed to ResolveAgentField for a given +// provider run. "init" is not an agent name — moat init's own ConfigureAgent +// hook (which runs earlier, at line ~186) already sets cfg.Agent to the +// auto-detected agent, so that value is reused as the verb instead of +// overwriting it with the literal string "init". +func agentVerbFor(rcName string, cfg *config.Config) string { + if rcName == "init" { + return cfg.Agent + } + return rcName +} + // containsGrant reports whether grants contains the named grant. func containsGrant(grants []string, name string) bool { for _, g := range grants { diff --git a/internal/cli/provider_test.go b/internal/cli/provider_test.go index 8a48dfaa..a6e1068b 100644 --- a/internal/cli/provider_test.go +++ b/internal/cli/provider_test.go @@ -12,6 +12,33 @@ import ( "github.com/majorcontext/moat/internal/config" ) +// TestAgentVerbForInit is a regression test for the moat-init special case in +// ResolveAgentField's wiring: RunProvider's ConfigureAgent hook (called before +// the agent-name block) sets cfg.Agent for "init" runs, so "init" itself is +// not a usable verb — it must be swapped for the already-resolved cfg.Agent +// rather than clobbering it. +func TestAgentVerbForInit(t *testing.T) { + tests := []struct { + name string + rcName string + agent string + want string + }{ + {"init reuses the auto-detected agent instead of the literal name", "init", "claude", "claude"}, + // Companion: every other provider's own name is a real agent verb and + // must pass through unchanged. + {"non-init providers use their own name as the verb", "claude", "codex", "claude"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &config.Config{Agent: tt.agent} + if got := agentVerbFor(tt.rcName, cfg); got != tt.want { + t.Errorf("agentVerbFor(%q, cfg.Agent=%q) = %q, want %q", tt.rcName, tt.agent, got, tt.want) + } + }) + } +} + func TestBuildGrants(t *testing.T) { tests := []struct { name string From 5e93f6a5ff9a24189ad9dfb45e005115c48662b8 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:38:57 +0000 Subject: [PATCH 04/33] fix(cli): restore pre-ConfigureAgent snapshot for the conflict check copilot and pi's ConfigureAgent hooks unconditionally overwrite cfg.Agent with their own name before ResolveAgentField's conflict check runs, so a conflicting agent: field in moat.yaml silently produced no warning for those two commands (the check compared the provider name against itself). Snapshot cfg.Agent before ConfigureAgent runs and restore it before resolving, so the warning fires uniformly across all six provider commands. --- internal/cli/provider.go | 26 ++++++++++- internal/cli/provider_test.go | 81 +++++++++++++++++++++++++++++++++++ 2 files changed, 106 insertions(+), 1 deletion(-) diff --git a/internal/cli/provider.go b/internal/cli/provider.go index 663267b8..42ba3e6b 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -182,6 +182,12 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error cfg.Network.Rules = append(cfg.Network.Rules, netrules.NetworkRuleEntry{HostRules: netrules.HostRules{Host: host}}) } + // Snapshot cfg.Agent before provider-specific hooks run. Some ConfigureAgent + // hooks (copilot, pi, init) unconditionally overwrite cfg.Agent with their + // own name below, which would otherwise erase the moat.yaml value before + // the conflict check further down ever sees it. + agentBeforeConfigure := cfg.Agent + // Provider-specific config tweaks (e.g., enabling log sync) if rc.ConfigureAgent != nil { rc.ConfigureAgent(cfg) @@ -215,7 +221,7 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error // The verb the user typed always names the agent. ValidateAgent runs inside // so an unknown moat.yaml value warns once and is discarded. - ResolveAgentField(cfg, agentVerbFor(rc.Name, cfg)) + resolveProviderAgentField(rc.Name, cfg, agentBeforeConfigure) opts := ExecOptions{ Flags: *rc.Flags, @@ -248,6 +254,24 @@ func agentVerbFor(rcName string, cfg *config.Config) string { return rcName } +// resolveProviderAgentField determines and applies the final cfg.Agent for a +// provider run. preConfigureAgent is the cfg.Agent snapshot taken before +// rc.ConfigureAgent ran: several hooks (copilot, pi, init) unconditionally +// overwrite cfg.Agent with their own provider name, which — if left in +// place — would make ResolveAgentField's conflict check compare the provider +// name against itself and silently swallow the warning. Restoring the +// snapshot first means the check always compares against what moat.yaml +// actually said, uniformly across every provider command. +// +// agentVerbFor still needs the post-ConfigureAgent value for "init" (it reads +// cfg.Agent to find the auto-detected agent), so the verb is computed before +// the snapshot is restored. +func resolveProviderAgentField(rcName string, cfg *config.Config, preConfigureAgent string) { + verb := agentVerbFor(rcName, cfg) + cfg.Agent = preConfigureAgent + ResolveAgentField(cfg, verb) +} + // containsGrant reports whether grants contains the named grant. func containsGrant(grants []string, name string) bool { for _, g := range grants { diff --git a/internal/cli/provider_test.go b/internal/cli/provider_test.go index a6e1068b..9ffdb18d 100644 --- a/internal/cli/provider_test.go +++ b/internal/cli/provider_test.go @@ -1,6 +1,7 @@ package cli import ( + "bytes" "os" "os/exec" "path/filepath" @@ -10,6 +11,7 @@ import ( "github.com/spf13/cobra" "github.com/majorcontext/moat/internal/config" + "github.com/majorcontext/moat/internal/ui" ) // TestAgentVerbForInit is a regression test for the moat-init special case in @@ -39,6 +41,85 @@ func TestAgentVerbForInit(t *testing.T) { } } +// TestResolveProviderAgentFieldWarnsAcrossConfigureAgentHooks is a regression +// test for the copilot/pi warning gap: their ConfigureAgent hooks +// unconditionally overwrite cfg.Agent with their own name (exactly like +// init's does), which erases the moat.yaml value before ResolveAgentField's +// conflict check ever sees it — so a conflicting `agent:` silently produced +// no warning for those two commands, unlike claude/codex/gemini whose +// ConfigureAgent hooks don't touch cfg.Agent at all. resolveProviderAgentField +// restores the pre-ConfigureAgent snapshot before resolving, so the conflict +// check is uniform across all six provider commands. +func TestResolveProviderAgentFieldWarnsAcrossConfigureAgentHooks(t *testing.T) { + tests := []struct { + name string + rcName string + preConfigureAgent string // cfg.Agent before the simulated ConfigureAgent stomp (i.e. what moat.yaml said) + postConfigureAgent string // cfg.Agent after the simulated ConfigureAgent stomp (its own provider name) + wantAgent string + wantWarn bool + }{ + { + name: "copilot with a conflicting agent: warns", + rcName: "copilot", + preConfigureAgent: "claude", + postConfigureAgent: "copilot", + wantAgent: "copilot", + wantWarn: true, + }, + // Companion: no conflicting field, stays silent. + { + name: "copilot with no conflicting agent: stays silent", + rcName: "copilot", + preConfigureAgent: "", + postConfigureAgent: "copilot", + wantAgent: "copilot", + wantWarn: false, + }, + { + name: "pi with a conflicting agent: warns", + rcName: "pi", + preConfigureAgent: "codex", + postConfigureAgent: "pi", + wantAgent: "pi", + wantWarn: true, + }, + // Companion: no conflicting field, stays silent. + { + name: "pi with no conflicting agent: stays silent", + rcName: "pi", + preConfigureAgent: "pi", + postConfigureAgent: "pi", + wantAgent: "pi", + wantWarn: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + // Simulate what RunProvider does: snapshot cfg.Agent, then run a + // ConfigureAgent hook that unconditionally stomps it (as copilot, + // pi, and init all do). + cfg := &config.Config{Agent: tt.preConfigureAgent} + preConfigureAgent := cfg.Agent + cfg.Agent = tt.postConfigureAgent + + resolveProviderAgentField(tt.rcName, cfg, preConfigureAgent) + + if cfg.Agent != tt.wantAgent { + t.Errorf("cfg.Agent = %q, want %q", cfg.Agent, tt.wantAgent) + } + if gotWarn := buf.Len() > 0; gotWarn != tt.wantWarn { + t.Errorf("warned = %v, want %v (output: %q)", gotWarn, tt.wantWarn, buf.String()) + } + }) + } +} + func TestBuildGrants(t *testing.T) { tests := []struct { name string From 4d03cfe1c46b3965d3222fb54396cfeb536bc3fc Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:44:19 +0000 Subject: [PATCH 05/33] fix(run): stage codex from the codex-cli dependency like every other agent --- internal/run/imageneeds.go | 6 ++++++ internal/run/imageneeds_test.go | 34 +++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/internal/run/imageneeds.go b/internal/run/imageneeds.go index f8b2e851..46de26bb 100644 --- a/internal/run/imageneeds.go +++ b/internal/run/imageneeds.go @@ -106,6 +106,12 @@ func resolveImageNeedsWithStore(grants []string, depList []deps.Dependency, stor if !initSet["gemini"] && hasDep(depList, "gemini-cli") { initSet["gemini"] = true } + // Codex staging writes a placeholder API key and the proxy injects the real + // credential at request time (see providers/codex.PopulateStagingDir), so + // staging does not need a credential — same as the other agents. + if !initSet[providerCodex] && hasDep(depList, "codex-cli") { + initSet[providerCodex] = true + } // Pi has no credential of its own, so it is never triggered by a grant. // Its staging (runtime context) runs whenever the pi-cli dependency is present. if !initSet["pi"] && hasDep(depList, "pi-cli") { diff --git a/internal/run/imageneeds_test.go b/internal/run/imageneeds_test.go index d630bfec..0e2874b3 100644 --- a/internal/run/imageneeds_test.go +++ b/internal/run/imageneeds_test.go @@ -2,6 +2,7 @@ package run import ( "fmt" + "slices" "testing" "time" @@ -302,3 +303,36 @@ func contains(ss []string, s string) bool { } return false } + +func TestResolveImageNeedsCodexDependencyFallback(t *testing.T) { + depList := []deps.Dependency{{Name: "codex-cli"}} + + // No credential store at all — the dependency alone must stage codex. + needs := resolveImageNeedsWithStore(nil, depList, nil) + if !slices.Contains(needs.initProviders, "codex") { + t.Errorf("codex-cli dependency should stage codex; got %v", needs.initProviders) + } + + // Companion: no dependency and no grant means codex is not staged. + bare := resolveImageNeedsWithStore(nil, []deps.Dependency{{Name: "git"}}, nil) + if slices.Contains(bare.initProviders, "codex") { + t.Errorf("codex should not be staged without dep or grant; got %v", bare.initProviders) + } +} + +func TestResolveImageNeedsCLIDepFallbackIsUniform(t *testing.T) { + for dep, agent := range map[string]string{ + "claude-code": "claude", + "codex-cli": "codex", + "copilot-cli": "copilot", + "gemini-cli": "gemini", + "pi-cli": "pi", + } { + t.Run(dep, func(t *testing.T) { + needs := resolveImageNeedsWithStore(nil, []deps.Dependency{{Name: dep}}, nil) + if !slices.Contains(needs.initProviders, agent) { + t.Errorf("%s should stage %s; got %v", dep, agent, needs.initProviders) + } + }) + } +} From e7e5cf096cf3a647f35aa8292ad0e000a05b573c Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:48:05 +0000 Subject: [PATCH 06/33] feat(run): compute the set of agents provisioned into a container --- internal/run/joinable.go | 42 ++++++++++++++++++++++ internal/run/joinable_test.go | 65 +++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 internal/run/joinable.go create mode 100644 internal/run/joinable_test.go diff --git a/internal/run/joinable.go b/internal/run/joinable.go new file mode 100644 index 00000000..34506b72 --- /dev/null +++ b/internal/run/joinable.go @@ -0,0 +1,42 @@ +package run + +import ( + "sort" + + "github.com/majorcontext/moat/internal/deps" +) + +// agentCLIDep maps an agent provider name to the dependency that installs its +// CLI binary into the container. +var agentCLIDep = map[string]string{ + "claude": "claude-code", + "codex": "codex-cli", + "copilot": "copilot-cli", + "gemini": "gemini-cli", + "pi": "pi-cli", +} + +// computeJoinableAgents returns the agents moat provisioned into the container: +// those whose config was staged AND whose CLI was installed. +// +// Both halves matter. initProviders is grant-driven, so a run with a claude +// grant but no claude-code dependency has staged config and no binary; the +// intersection rejects it. +// +// The result is always non-nil. Callers persist it without omitempty so an +// empty set (no joinable agents) stays distinguishable from an absent field +// (a run created before capability tracking existed). +func computeJoinableAgents(initProviders []string, depList []deps.Dependency) []string { + out := []string{} + for _, agent := range initProviders { + dep, ok := agentCLIDep[agent] + if !ok { + continue + } + if hasDep(depList, dep) { + out = append(out, agent) + } + } + sort.Strings(out) + return out +} diff --git a/internal/run/joinable_test.go b/internal/run/joinable_test.go new file mode 100644 index 00000000..1bcd7b62 --- /dev/null +++ b/internal/run/joinable_test.go @@ -0,0 +1,65 @@ +package run + +import ( + "reflect" + "testing" + + "github.com/majorcontext/moat/internal/deps" +) + +func TestComputeJoinableAgents(t *testing.T) { + tests := []struct { + name string + initProviders []string + depList []deps.Dependency + want []string + }{ + { + name: "staged with CLI dep is joinable", + initProviders: []string{"claude"}, + depList: []deps.Dependency{{Name: "claude-code"}}, + want: []string{"claude"}, + }, + { + name: "staged without CLI dep is not joinable", + initProviders: []string{"claude"}, + depList: []deps.Dependency{{Name: "git"}}, + want: []string{}, + }, + { + name: "CLI dep without staging is not joinable", + initProviders: nil, + depList: []deps.Dependency{{Name: "claude-code"}}, + want: []string{}, + }, + { + name: "dual agent", + initProviders: []string{"claude", "codex"}, + depList: []deps.Dependency{{Name: "claude-code"}, {Name: "codex-cli"}}, + want: []string{"claude", "codex"}, + }, + { + name: "result is sorted regardless of input order", + initProviders: []string{"codex", "claude"}, + depList: []deps.Dependency{{Name: "codex-cli"}, {Name: "claude-code"}}, + want: []string{"claude", "codex"}, + }, + { + name: "unknown provider name is ignored", + initProviders: []string{"nonsense"}, + depList: []deps.Dependency{{Name: "claude-code"}}, + want: []string{}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := computeJoinableAgents(tt.initProviders, tt.depList) + if got == nil { + t.Fatal("computeJoinableAgents must never return nil — nil means 'pre-upgrade run' downstream") + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("computeJoinableAgents() = %v, want %v", got, tt.want) + } + }) + } +} From 8d1c3833686e355a56c46b315b484193fe7cd953 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:51:51 +0000 Subject: [PATCH 07/33] feat(run): persist the joinable-agent set to run metadata --- internal/run/joinable_test.go | 60 +++++++++++++++++++++++++++++ internal/run/manager_create.go | 5 +++ internal/run/manager_persistence.go | 1 + internal/run/run.go | 2 + internal/storage/storage.go | 34 +++++++++------- 5 files changed, 89 insertions(+), 13 deletions(-) diff --git a/internal/run/joinable_test.go b/internal/run/joinable_test.go index 1bcd7b62..3f80dab4 100644 --- a/internal/run/joinable_test.go +++ b/internal/run/joinable_test.go @@ -1,10 +1,13 @@ package run import ( + "os" + "path/filepath" "reflect" "testing" "github.com/majorcontext/moat/internal/deps" + "github.com/majorcontext/moat/internal/storage" ) func TestComputeJoinableAgents(t *testing.T) { @@ -63,3 +66,60 @@ func TestComputeJoinableAgents(t *testing.T) { }) } } + +func TestJoinableAgentsRoundTrip(t *testing.T) { + tests := []struct { + name string + value []string + }{ + {"populated set survives", []string{"claude", "codex"}}, + {"empty set stays empty, not nil", []string{}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + store, err := storage.NewRunStore(dir, "run_test12345678") + if err != nil { + t.Fatalf("NewRunStore: %v", err) + } + if err := store.SaveMetadata(storage.Metadata{JoinableAgents: tt.value}); err != nil { + t.Fatalf("SaveMetadata: %v", err) + } + got, err := store.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if got.JoinableAgents == nil { + t.Fatal("a persisted set must not load back as nil — nil means pre-upgrade") + } + if !reflect.DeepEqual(got.JoinableAgents, tt.value) { + t.Errorf("JoinableAgents = %v, want %v", got.JoinableAgents, tt.value) + } + }) + } +} + +func TestJoinableAgentsAbsentLoadsAsNil(t *testing.T) { + // Companion to the round-trip: metadata written before this field existed + // must load as nil so join can tell it apart from an empty set. + dir := t.TempDir() + path := filepath.Join(dir, "run_legacy12345678") + if err := os.MkdirAll(path, 0o755); err != nil { + t.Fatal(err) + } + legacy := `{"name":"old-run","workspace":"/tmp","agent":"claude"}` + if err := os.WriteFile(filepath.Join(path, "metadata.json"), []byte(legacy), 0o644); err != nil { + t.Fatal(err) + } + store, err := storage.NewRunStore(dir, "run_legacy12345678") + if err != nil { + t.Fatalf("NewRunStore: %v", err) + } + got, err := store.LoadMetadata() + if err != nil { + t.Fatalf("LoadMetadata: %v", err) + } + if got.JoinableAgents != nil { + t.Errorf("legacy metadata should load JoinableAgents as nil, got %v", got.JoinableAgents) + } +} diff --git a/internal/run/manager_create.go b/internal/run/manager_create.go index 1dcf43dd..1a6dd395 100644 --- a/internal/run/manager_create.go +++ b/internal/run/manager_create.go @@ -1213,6 +1213,11 @@ region = %s NeedsWorkspaceVolume: volumeMode, } + // Record which agents this container can host, for `moat join`. + // installableDeps is a separately scoped local (deps.FilterInstallable, + // ~line 1000) — not a field on imgNeeds — so both operands are read here. + r.JoinableAgents = computeJoinableAgents(imgNeeds.initProviders, installableDeps) + // Resolve container image based on dependencies and image spec hasDeps := len(installableDeps) > 0 containerImage := image.Resolve(installableDeps, imageSpec) diff --git a/internal/run/manager_persistence.go b/internal/run/manager_persistence.go index 2ea2fcd0..8cc74167 100644 --- a/internal/run/manager_persistence.go +++ b/internal/run/manager_persistence.go @@ -193,6 +193,7 @@ func (m *Manager) registerPersistedRun(runState State, stateConfirmed bool, skip Workspace: meta.Workspace, Grants: meta.Grants, Agent: meta.Agent, + JoinableAgents: meta.JoinableAgents, Image: meta.Image, Runtime: meta.Runtime, Ports: meta.Ports, diff --git a/internal/run/run.go b/internal/run/run.go index a6a46ccb..07a7dbdd 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -52,6 +52,7 @@ type Run struct { WorktreeRepoID string Grants []string Agent string // Agent type from config (e.g., "claude-code", "codex") + JoinableAgents []string // Agents provisioned into the container; nil = pre-upgrade run Image string // Container image used for this run Runtime string // Container runtime type ("docker" or "apple") ProviderMeta map[string]string // Provider-specific metadata (e.g., claude_session_id) @@ -196,6 +197,7 @@ func (r *Run) SaveMetadata() error { Workspace: r.Workspace, Grants: r.Grants, Agent: r.Agent, + JoinableAgents: r.JoinableAgents, Image: r.Image, Ports: r.Ports, ContainerID: r.ContainerID, diff --git a/internal/storage/storage.go b/internal/storage/storage.go index eeb46d6b..2c79f73c 100644 --- a/internal/storage/storage.go +++ b/internal/storage/storage.go @@ -17,19 +17,27 @@ import ( // Metadata holds information about an agent run. type Metadata struct { - Name string `json:"name"` - Workspace string `json:"workspace"` - Grants []string `json:"grants,omitempty"` - Agent string `json:"agent,omitempty"` // Agent type from config (e.g., "claude-code") - Image string `json:"image,omitempty"` // Container image used - Ports map[string]int `json:"ports,omitempty"` - ContainerID string `json:"container_id,omitempty"` - State string `json:"state,omitempty"` - Interactive bool `json:"interactive,omitempty"` - CreatedAt time.Time `json:"created_at,omitempty"` - StartedAt time.Time `json:"started_at,omitempty"` - StoppedAt time.Time `json:"stopped_at,omitempty"` - Error string `json:"error,omitempty"` + Name string `json:"name"` + Workspace string `json:"workspace"` + Grants []string `json:"grants,omitempty"` + Agent string `json:"agent,omitempty"` // Agent type from config (e.g., "claude-code") + + // JoinableAgents lists the agents moat provisioned into the container — + // staged config plus an installed CLI. `moat join` tests membership here. + // + // Deliberately NOT omitempty: an empty slice (no joinable agents) must stay + // distinguishable from an absent field (a run created before this existed, + // which falls back to the legacy agent-string check). + JoinableAgents []string `json:"joinable_agents"` + Image string `json:"image,omitempty"` // Container image used + Ports map[string]int `json:"ports,omitempty"` + ContainerID string `json:"container_id,omitempty"` + State string `json:"state,omitempty"` + Interactive bool `json:"interactive,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` + StartedAt time.Time `json:"started_at,omitempty"` + StoppedAt time.Time `json:"stopped_at,omitempty"` + Error string `json:"error,omitempty"` // ProviderMeta holds provider-specific metadata captured during the run lifecycle. // For example, the Claude provider stores {"claude_session_id": ""}. From 4983ecc3dbb2470f7a635f0bb81e96c07fd10bbf Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Tue, 11 Aug 2026 23:56:12 +0000 Subject: [PATCH 08/33] fix(join): gate on provisioned agents instead of the agent string --- cmd/moat/cli/join_cmd.go | 41 ++++++++++++++------ cmd/moat/cli/join_cmd_test.go | 72 ++++++++++++++++++++++++++++------- 2 files changed, 89 insertions(+), 24 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index b2c34a13..131bfb81 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -49,17 +49,36 @@ func init() { rootCmd.AddCommand(joinCmd) } -// validateJoinAgent checks that the run (whose recorded agent field is runAgent) -// was created by the requested provider. agentArg is the user-typed agent name, -// used only for the error message. -func validateJoinAgent(j provider.JoinableAgent, agentArg, runAgent string) error { - if !j.IdentifiesAs(runAgent) { - return fmt.Errorf("run has no %s configuration.\n"+ - "v1 join only attaches an agent the run was started with (run agent: %q).\n"+ - "To run %s here, start the run with %s configured.", - agentArg, runAgent, agentArg, agentArg) +// validateJoinAgent reports whether agentArg may be launched into r. +// +// The authority is r.JoinableAgents — what moat actually provisioned. A nil set +// means the run was created before capability tracking, so we fall back to the +// legacy agent-string check; an EMPTY set is a real answer ("nothing joinable +// here") and must not fall back. That distinction is why the metadata field is +// persisted without omitempty. +func validateJoinAgent(j provider.JoinableAgent, agentArg string, r *run.Run) error { + if r.JoinableAgents != nil { + for _, a := range r.JoinableAgents { + if a == agentArg || j.IdentifiesAs(a) { + return nil + } + } + hosted := "none" + if len(r.JoinableAgents) > 0 { + hosted = strings.Join(r.JoinableAgents, ", ") + } + return fmt.Errorf("run %s cannot host %s (provisioned agents: %s).\n"+ + "Add %s to this project's moat.yaml `agents:` list and recreate the run.", + r.ID, agentArg, hosted, agentArg) } - return nil + + // Pre-upgrade run: no capability set was ever recorded. + if j.IdentifiesAs(r.Agent) { + return nil + } + return fmt.Errorf("run %s was created before capability tracking and records agent %q.\n"+ + "Recreate the run to join it (moat stop %s && moat %s).", + r.ID, r.Agent, r.ID, agentArg) } // joinableAgentNames returns the sorted names of registered agents that support @@ -110,7 +129,7 @@ func runJoin(cmd *cobra.Command, args []string) error { if !ok { return fmt.Errorf("agent %q does not support join yet", agentArg) } - if valErr := validateJoinAgent(joinable, agentArg, r.Agent); valErr != nil { + if valErr := validateJoinAgent(joinable, agentArg, r); valErr != nil { return valErr } diff --git a/cmd/moat/cli/join_cmd_test.go b/cmd/moat/cli/join_cmd_test.go index 6414168c..70a372b6 100644 --- a/cmd/moat/cli/join_cmd_test.go +++ b/cmd/moat/cli/join_cmd_test.go @@ -2,6 +2,7 @@ package cli import ( "os" + "slices" "strings" "syscall" "testing" @@ -9,24 +10,69 @@ import ( "github.com/majorcontext/moat/internal/container" "github.com/majorcontext/moat/internal/provider" + "github.com/majorcontext/moat/internal/run" ) -// fakeJoinable implements provider.JoinableAgent for validation tests. -type fakeJoinable struct{ identifies bool } +// fakeJoinable implements provider.JoinableAgent for validation tests. It is +// also reused by joinpick_test.go. +type fakeJoinable struct{ names []string } -func (f fakeJoinable) JoinCommand(provider.JoinOpts) ([]string, error) { return []string{"x"}, nil } -func (f fakeJoinable) IdentifiesAs(string) bool { return f.identifies } - -func TestValidateJoinAgent_OK(t *testing.T) { - if err := validateJoinAgent(fakeJoinable{identifies: true}, "claude", "claude-code"); err != nil { - t.Fatalf("unexpected error: %v", err) - } +func (f fakeJoinable) JoinCommand(provider.JoinOpts) ([]string, error) { return nil, nil } +func (f fakeJoinable) IdentifiesAs(agent string) bool { + return slices.Contains(f.names, agent) } -func TestValidateJoinAgent_WrongProvider(t *testing.T) { - err := validateJoinAgent(fakeJoinable{identifies: false}, "codex", "claude-code") - if err == nil || !strings.Contains(err.Error(), "no codex configuration") { - t.Fatalf("got %v, want a clear 'no codex configuration' error", err) +func TestValidateJoinAgent(t *testing.T) { + claude := fakeJoinable{names: []string{"claude", "claude-code"}} + + tests := []struct { + name string + run *run.Run + agent string + wantErr bool + errHas string + }{ + { + name: "member of the capability set is accepted", + run: &run.Run{ID: "run_1", JoinableAgents: []string{"claude"}}, + agent: "claude", + }, + { + name: "non-member is rejected even when the agent string matches", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{"codex"}}, + agent: "claude", + wantErr: true, + errHas: "codex", + }, + { + name: "empty set refuses", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{}}, + agent: "claude", + wantErr: true, + }, + { + name: "nil set falls back and accepts a matching agent string", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: nil}, + agent: "claude", + }, + { + name: "nil set falls back and refuses a stale agent string", + run: &run.Run{ID: "run_1", Agent: "vibrant-code", JoinableAgents: nil}, + agent: "claude", + wantErr: true, + errHas: "Recreate the run", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateJoinAgent(claude, tt.agent, tt.run) + if (err != nil) != tt.wantErr { + t.Fatalf("validateJoinAgent() error = %v, wantErr %v", err, tt.wantErr) + } + if tt.errHas != "" && !strings.Contains(err.Error(), tt.errHas) { + t.Errorf("error %q should mention %q", err, tt.errHas) + } + }) } } From ce3eb4aeb3a5492e06e5f34dab3b12aa066b1c43 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:02:42 +0000 Subject: [PATCH 09/33] fix(join): resolve run names against running runs only --- cmd/moat/cli/join_cmd.go | 16 ++++---- cmd/moat/cli/resolve.go | 56 ++++++++++++++++++++++++++++ cmd/moat/cli/resolve_test.go | 72 ++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+), 9 deletions(-) create mode 100644 cmd/moat/cli/resolve_test.go diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index 131bfb81..32e39b3d 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -108,17 +108,15 @@ func runJoin(cmd *cobra.Command, args []string) error { } defer manager.Close() - runID, err := resolveRunArgSingle(manager, runArg) + r, candidates, err := resolveRunningRunArg(manager, runArg) if err != nil { return err } - - r, gErr := manager.Get(runID) - if gErr != nil { - return gErr - } - if r.GetState() != run.StateRunning { - return fmt.Errorf("run %s is not running (state: %s)", runID, r.GetState()) + if r == nil { + // Several running runs share this name. Task 17 routes these to the + // picker; until then, list them and ask for an ID. + printMatchingRuns(candidates, runArg) + return fmt.Errorf("name %q matches %d running runs; specify a run ID", runArg, len(candidates)) } agent := provider.GetAgent(agentArg) @@ -148,7 +146,7 @@ func runJoin(cmd *cobra.Command, args []string) error { // and headless paths need an index so console output lands in logs..jsonl. // Do NOT defer release here — we call it explicitly before exitWithExecError // so registry cleanup runs even when the agent exits with a non-zero code. - index, release, regErr := manager.RegisterJoinedAgent(runID) + index, release, regErr := manager.RegisterJoinedAgent(r.ID) var execErr error // Headless (--prompt with no TTY) vs interactive. diff --git a/cmd/moat/cli/resolve.go b/cmd/moat/cli/resolve.go index 28a34566..465e3265 100644 --- a/cmd/moat/cli/resolve.go +++ b/cmd/moat/cli/resolve.go @@ -4,6 +4,7 @@ import ( "bufio" "fmt" "os" + "sort" "strings" "text/tabwriter" @@ -79,6 +80,61 @@ func disambiguateRuns(matches []*run.Run, arg string, action string) ([]string, return ids, nil } +// filterRunning returns only the runs currently in the running state. +func filterRunning(matches []*run.Run) []*run.Run { + out := make([]*run.Run, 0, len(matches)) + for _, r := range matches { + if r.GetState() == run.StateRunning { + out = append(out, r) + } + } + return out +} + +// resolveRunningFrom narrows a name/ID match set to running runs. +// +// Returns exactly one of: a single run, a candidate list for the caller to +// disambiguate, or an error. When filtering empties a non-empty match set the +// error names the state ("not running (state: stopped)") rather than degrading +// to "no run found" — the specific cause is what tells the user what to do. +func resolveRunningFrom(matches []*run.Run, arg string) (*run.Run, []*run.Run, error) { + if len(matches) == 0 { + return nil, nil, fmt.Errorf("no run found matching %q\n\nRun 'moat list' to see available runs.", arg) + } + running := filterRunning(matches) + if len(running) == 0 { + sortRunsByCreatedAt(matches) + r := matches[0] + return nil, nil, fmt.Errorf("run %s is not running (state: %s)", r.ID, r.GetState()) + } + if len(running) == 1 { + return running[0], nil, nil + } + sortRunsByCreatedAt(running) + return nil, running, nil +} + +// resolveRunningRunArg resolves a user-supplied run argument to a running run. +func resolveRunningRunArg(manager *run.Manager, arg string) (*run.Run, []*run.Run, error) { + matches, err := manager.Resolve(arg) + if err != nil { + return nil, nil, err + } + return resolveRunningFrom(matches, arg) +} + +// sortRunsByCreatedAt sorts runs newest first. manager.Resolve already +// returns matches in this order, but resolveRunningFrom is also exercised +// directly with hand-built slices (see resolve_test.go), and filtering +// itself doesn't change ordering — so this keeps both entry points +// consistent. (internal/run has an equivalent helper, but it is unexported +// and not reachable from this package.) +func sortRunsByCreatedAt(matches []*run.Run) { + sort.Slice(matches, func(i, j int) bool { + return matches[i].CreatedAt.After(matches[j].CreatedAt) + }) +} + // printMatchingRuns prints a table of matching runs to stderr. func printMatchingRuns(matches []*run.Run, arg string) { fmt.Fprintf(os.Stderr, "Multiple runs match %q:\n", arg) diff --git a/cmd/moat/cli/resolve_test.go b/cmd/moat/cli/resolve_test.go new file mode 100644 index 00000000..b1f36b05 --- /dev/null +++ b/cmd/moat/cli/resolve_test.go @@ -0,0 +1,72 @@ +package cli + +import ( + "strings" + "testing" + + "github.com/majorcontext/moat/internal/run" +) + +func TestFilterRunning(t *testing.T) { + running := &run.Run{ID: "run_a", State: run.StateRunning} + stopped := &run.Run{ID: "run_b", State: run.StateStopped} + + got := filterRunning([]*run.Run{running, stopped}) + if len(got) != 1 || got[0].ID != "run_a" { + t.Errorf("filterRunning should keep only running runs; got %v", got) + } + + // Companion: an all-stopped set filters to empty rather than erroring. + if got := filterRunning([]*run.Run{stopped}); len(got) != 0 { + t.Errorf("all-stopped set should filter to empty; got %v", got) + } +} + +func TestResolveRunningRunArgReportsStoppedState(t *testing.T) { + // A named run that exists but is stopped keeps the specific error rather + // than degrading to "no run found" — the specific message tells the user + // what to do. + stopped := &run.Run{ID: "run_b", Name: "solo", State: run.StateStopped} + _, _, err := resolveRunningFrom([]*run.Run{stopped}, "solo") + if err == nil { + t.Fatal("expected an error for a stopped run") + } + if !strings.Contains(err.Error(), "not running") || !strings.Contains(err.Error(), "stopped") { + t.Errorf("error should name the state; got %q", err) + } +} + +func TestResolveRunningFromSingleRunning(t *testing.T) { + // Companion to the multiple-running and stopped-state cases: a match set + // that resolves to exactly one running run returns it directly with no + // error and no candidate list. + stopped := &run.Run{ID: "run_a", Name: "solo", State: run.StateStopped} + running := &run.Run{ID: "run_b", Name: "solo", State: run.StateRunning} + + single, candidates, err := resolveRunningFrom([]*run.Run{stopped, running}, "solo") + if err != nil { + t.Fatalf("expected no error, got %v", err) + } + if single == nil || single.ID != "run_b" { + t.Errorf("expected single match run_b, got %v", single) + } + if candidates != nil { + t.Errorf("expected no candidates, got %v", candidates) + } +} + +func TestResolveRunningFromMultipleRunning(t *testing.T) { + a := &run.Run{ID: "run_a", Name: "moat-dev", State: run.StateRunning} + b := &run.Run{ID: "run_b", Name: "moat-dev", State: run.StateRunning} + + single, candidates, err := resolveRunningFrom([]*run.Run{a, b}, "moat-dev") + if err != nil { + t.Fatalf("two running runs should not error here; got %v", err) + } + if single != nil { + t.Errorf("expected no single match, got %v", single) + } + if len(candidates) != 2 { + t.Errorf("expected 2 candidates for the picker, got %d", len(candidates)) + } +} From 82013564ac3a4cfacb97b295532712de495f8a58 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:06:53 +0000 Subject: [PATCH 10/33] refactor(run): export SortRunsByCreatedAt to de-duplicate cli helper --- cmd/moat/cli/resolve.go | 17 ++--------------- internal/run/resolve.go | 8 ++++---- 2 files changed, 6 insertions(+), 19 deletions(-) diff --git a/cmd/moat/cli/resolve.go b/cmd/moat/cli/resolve.go index 465e3265..a41efeb1 100644 --- a/cmd/moat/cli/resolve.go +++ b/cmd/moat/cli/resolve.go @@ -4,7 +4,6 @@ import ( "bufio" "fmt" "os" - "sort" "strings" "text/tabwriter" @@ -103,14 +102,14 @@ func resolveRunningFrom(matches []*run.Run, arg string) (*run.Run, []*run.Run, e } running := filterRunning(matches) if len(running) == 0 { - sortRunsByCreatedAt(matches) + run.SortRunsByCreatedAt(matches) r := matches[0] return nil, nil, fmt.Errorf("run %s is not running (state: %s)", r.ID, r.GetState()) } if len(running) == 1 { return running[0], nil, nil } - sortRunsByCreatedAt(running) + run.SortRunsByCreatedAt(running) return nil, running, nil } @@ -123,18 +122,6 @@ func resolveRunningRunArg(manager *run.Manager, arg string) (*run.Run, []*run.Ru return resolveRunningFrom(matches, arg) } -// sortRunsByCreatedAt sorts runs newest first. manager.Resolve already -// returns matches in this order, but resolveRunningFrom is also exercised -// directly with hand-built slices (see resolve_test.go), and filtering -// itself doesn't change ordering — so this keeps both entry points -// consistent. (internal/run has an equivalent helper, but it is unexported -// and not reachable from this package.) -func sortRunsByCreatedAt(matches []*run.Run) { - sort.Slice(matches, func(i, j int) bool { - return matches[i].CreatedAt.After(matches[j].CreatedAt) - }) -} - // printMatchingRuns prints a table of matching runs to stderr. func printMatchingRuns(matches []*run.Run, arg string) { fmt.Fprintf(os.Stderr, "Multiple runs match %q:\n", arg) diff --git a/internal/run/resolve.go b/internal/run/resolve.go index aa11d327..52f6cec8 100644 --- a/internal/run/resolve.go +++ b/internal/run/resolve.go @@ -41,7 +41,7 @@ func (m *Manager) Resolve(arg string) ([]*Run, error) { } } if len(matches) > 0 { - sortRunsByCreatedAt(matches) + SortRunsByCreatedAt(matches) return matches, nil } // Fall through to name match @@ -59,12 +59,12 @@ func (m *Manager) Resolve(arg string) ([]*Run, error) { return nil, fmt.Errorf("no run found matching %q\n\nRun 'moat list' to see available runs.", arg) } - sortRunsByCreatedAt(matches) + SortRunsByCreatedAt(matches) return matches, nil } -// sortRunsByCreatedAt sorts runs newest first. -func sortRunsByCreatedAt(runs []*Run) { +// SortRunsByCreatedAt sorts runs newest first. +func SortRunsByCreatedAt(runs []*Run) { sort.Slice(runs, func(i, j int) bool { return runs[i].CreatedAt.After(runs[j].CreatedAt) }) From 469bfa9c17dcb28c9ee9e30c527460ab82e7d5d5 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:13:28 +0000 Subject: [PATCH 11/33] feat(provider): add AgentRuntime for declarative agent provisioning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the AgentRuntime interface (DefaultDependencies, NetworkHosts, CredentialGrant) so agent providers can be provisioned into a container declaratively via moat.yaml agents: (Task 10). CredentialGrant is a static per-provider constant, not a delegation to the existing GetCredentialName() store-probing funcs — those answer which credential is configured right now, not which grant an agent needs. codex maps to the openai grant (not codex, where GetCredentialName would resolve). pi deliberately does not implement the interface: its grant is resolved per-invocation from flags, config, and the store, so there is no static answer. Also fix registry_test.go: several tests call Clear() on the shared provider registry without restoring it, permanently wiping the real provider registrations (populated once via init()) for the rest of the test binary. This only surfaced now because the new runtime_test.go hard-asserts real registrations exist, running after registry_test.go alphabetically. Snapshot/restore the registry around each Clear(). --- internal/provider/interfaces.go | 18 +++++++++ internal/provider/registry_test.go | 44 +++++++++++++++++++++ internal/provider/runtime_test.go | 57 +++++++++++++++++++++++++++ internal/providers/claude/runtime.go | 9 +++++ internal/providers/codex/runtime.go | 12 ++++++ internal/providers/copilot/runtime.go | 10 +++++ internal/providers/gemini/runtime.go | 11 ++++++ 7 files changed, 161 insertions(+) create mode 100644 internal/provider/runtime_test.go create mode 100644 internal/providers/claude/runtime.go create mode 100644 internal/providers/codex/runtime.go create mode 100644 internal/providers/copilot/runtime.go create mode 100644 internal/providers/gemini/runtime.go diff --git a/internal/provider/interfaces.go b/internal/provider/interfaces.go index 699a324f..810324aa 100644 --- a/internal/provider/interfaces.go +++ b/internal/provider/interfaces.go @@ -111,6 +111,24 @@ type JoinableAgent interface { IdentifiesAs(agent string) bool } +// AgentRuntime is implemented by agent providers that can be provisioned into a +// container declaratively via moat.yaml `agents:`. It is optional: a provider +// that does not implement it cannot appear in an `agents:` list. +type AgentRuntime interface { + // DefaultDependencies returns the dependencies that install this agent's CLI. + DefaultDependencies() []string + + // NetworkHosts returns the hosts this agent needs to reach. + NetworkHosts() []string + + // CredentialGrant returns the STATIC grant name this agent needs — not a + // credential-store lookup. "" means the agent has no static grant and none + // should be unioned in. Store lookups stay in the verb-path + // GetCredentialName funcs; conflating the two puts empty or wrong grant + // names into the grants list. + CredentialGrant() string +} + // AgentProvider extends CredentialProvider for AI agent runtimes. // Implemented by claude, copilot, codex, gemini, and pi providers. type AgentProvider interface { diff --git a/internal/provider/registry_test.go b/internal/provider/registry_test.go index b9da2529..247fd9b1 100644 --- a/internal/provider/registry_test.go +++ b/internal/provider/registry_test.go @@ -40,7 +40,36 @@ type mockEndpointProvider struct { func (m *mockEndpointProvider) RegisterEndpoints(mux *http.ServeMux, cred *Credential) {} +// snapshotRegistry captures the current registry state so tests that call +// Clear() can restore it afterward. Real agents are registered once via +// init() (triggered by the blank import of internal/providers in +// interfaces_test.go) — Clear() without a restore would wipe them for the +// rest of the test binary, since init() never runs again. +func snapshotRegistry() (map[string]CredentialProvider, map[string]string) { + mu.RLock() + defer mu.RUnlock() + p := make(map[string]CredentialProvider, len(providers)) + for k, v := range providers { + p[k] = v + } + a := make(map[string]string, len(aliases)) + for k, v := range aliases { + a[k] = v + } + return p, a +} + +func restoreRegistry(p map[string]CredentialProvider, a map[string]string) { + mu.Lock() + defer mu.Unlock() + providers = p + aliases = a +} + func TestRegistry(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() // Start fresh defer Clear() @@ -81,6 +110,9 @@ func TestRegistry(t *testing.T) { } func TestRegisterAlias(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() defer Clear() @@ -107,6 +139,9 @@ func TestRegisterAlias(t *testing.T) { } func TestGetAgent(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() defer Clear() @@ -137,6 +172,9 @@ func TestGetAgent(t *testing.T) { } func TestGetEndpoint(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() defer Clear() @@ -167,6 +205,9 @@ func TestGetEndpoint(t *testing.T) { } func TestAll(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() defer Clear() @@ -188,6 +229,9 @@ func TestAll(t *testing.T) { } func TestAgents(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + Clear() defer Clear() diff --git a/internal/provider/runtime_test.go b/internal/provider/runtime_test.go new file mode 100644 index 00000000..e056d846 --- /dev/null +++ b/internal/provider/runtime_test.go @@ -0,0 +1,57 @@ +package provider_test + +import ( + "testing" + + "github.com/majorcontext/moat/internal/provider" + _ "github.com/majorcontext/moat/internal/providers/claude" + _ "github.com/majorcontext/moat/internal/providers/codex" + _ "github.com/majorcontext/moat/internal/providers/copilot" + _ "github.com/majorcontext/moat/internal/providers/gemini" +) + +func TestAgentRuntimeCredentialGrantIsStatic(t *testing.T) { + tests := []struct { + agent string + want string + }{ + {"claude", "claude"}, + {"codex", "openai"}, // NOT "codex" — the credential lives under openai + {"copilot", "github"}, + {"gemini", "gemini"}, + } + for _, tt := range tests { + t.Run(tt.agent, func(t *testing.T) { + a := provider.GetAgent(tt.agent) + if a == nil { + t.Fatalf("agent %q not registered", tt.agent) + } + rt, ok := a.(provider.AgentRuntime) + if !ok { + t.Fatalf("agent %q should implement AgentRuntime", tt.agent) + } + // Static: the answer must not depend on the credential store. + if got := rt.CredentialGrant(); got != tt.want { + t.Errorf("CredentialGrant() = %q, want %q", got, tt.want) + } + if len(rt.DefaultDependencies()) == 0 { + t.Errorf("agent %q should declare CLI dependencies", tt.agent) + } + if len(rt.NetworkHosts()) == 0 { + t.Errorf("agent %q should declare network hosts", tt.agent) + } + }) + } +} + +func TestPiDoesNotImplementAgentRuntime(t *testing.T) { + // Companion: pi's backend grant is resolved per-invocation from flags, + // config, and the store, so there is no static answer. agents: rejects it. + a := provider.GetAgent("pi") + if a == nil { + t.Skip("pi provider not registered in this build") + } + if _, ok := a.(provider.AgentRuntime); ok { + t.Error("pi must not implement AgentRuntime — its grant has no static value") + } +} diff --git a/internal/providers/claude/runtime.go b/internal/providers/claude/runtime.go new file mode 100644 index 00000000..21149383 --- /dev/null +++ b/internal/providers/claude/runtime.go @@ -0,0 +1,9 @@ +package claude + +// AgentRuntime implementation. See provider.AgentRuntime. + +func (p *OAuthProvider) DefaultDependencies() []string { return DefaultDependencies() } +func (p *OAuthProvider) NetworkHosts() []string { return NetworkHosts() } + +// CredentialGrant is static: the claude grant, regardless of what is stored. +func (p *OAuthProvider) CredentialGrant() string { return "claude" } diff --git a/internal/providers/codex/runtime.go b/internal/providers/codex/runtime.go new file mode 100644 index 00000000..c9a44a2b --- /dev/null +++ b/internal/providers/codex/runtime.go @@ -0,0 +1,12 @@ +package codex + +// AgentRuntime implementation. See provider.AgentRuntime. + +func (p *Provider) DefaultDependencies() []string { return DefaultDependencies() } +func (p *Provider) NetworkHosts() []string { return NetworkHosts() } + +// CredentialGrant is "openai", not "codex": the provider registry name is +// codex, but the credential is stored under openai (credential.ProviderOpenAI). +// GetCredentialName returns whichever key happens to exist, which is the wrong +// question here. +func (p *Provider) CredentialGrant() string { return "openai" } diff --git a/internal/providers/copilot/runtime.go b/internal/providers/copilot/runtime.go new file mode 100644 index 00000000..28d03974 --- /dev/null +++ b/internal/providers/copilot/runtime.go @@ -0,0 +1,10 @@ +package copilot + +// AgentRuntime implementation. See provider.AgentRuntime. + +func (p *Provider) DefaultDependencies() []string { return DefaultDependencies() } +func (p *Provider) NetworkHosts() []string { return NetworkHosts() } + +// CredentialGrant is "github": copilot rides the GitHub credential +// (credentialStoreKey maps copilot -> ProviderGitHub). +func (p *Provider) CredentialGrant() string { return "github" } diff --git a/internal/providers/gemini/runtime.go b/internal/providers/gemini/runtime.go new file mode 100644 index 00000000..034104d4 --- /dev/null +++ b/internal/providers/gemini/runtime.go @@ -0,0 +1,11 @@ +package gemini + +// AgentRuntime implementation. See provider.AgentRuntime. + +func (p *Provider) DefaultDependencies() []string { return DefaultDependencies() } +func (p *Provider) NetworkHosts() []string { return NetworkHosts() } + +// CredentialGrant is static, unlike the store-probing closure gemini wires into +// ProviderRunConfig. That closure returns "" when nothing is stored, which +// would put an empty grant into the grants list. +func (p *Provider) CredentialGrant() string { return "gemini" } From ef1d9654237f4e442147febc029ae1b12591ff14 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:21:18 +0000 Subject: [PATCH 12/33] feat(config): add agents: to provision several agents into one container Expands moat.yaml's agents: list into the dependencies, grants, and network rules each named agent needs. Unlike agent:, unknown entries are a hard error since a silently dropped agent leaves the container short a credential and its firewall rules, surfacing only much later as an opaque join refusal. --- cmd/moat/cli/run.go | 8 ++++ internal/cli/agents.go | 73 ++++++++++++++++++++++++++++++++ internal/cli/agents_test.go | 84 +++++++++++++++++++++++++++++++++++++ internal/cli/provider.go | 11 +++++ internal/config/config.go | 1 + 5 files changed, 177 insertions(+) diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index 4b0dd895..66d96890 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -111,6 +111,14 @@ func runAgent(cmd *cobra.Command, args []string) error { return fmt.Errorf("loading config: %w", err) } + // Expand agents: into dependencies/grants/network hosts before the + // "Apply config defaults" block below reads cfg.Grants into runFlags.Grants + // — an expansion that lands after would never reach the grants flag (see + // intcli.ExpandAgents doc comment for why ordering matters here). + if err = intcli.ExpandAgents(cfg); err != nil { + return err + } + // Determine agent name: --name flag > config.Name > random if runFlags.Name == "" && cfg != nil && cfg.Name != "" { runFlags.Name = cfg.Name diff --git a/internal/cli/agents.go b/internal/cli/agents.go index 62f0f294..dc5f7e79 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -1,10 +1,13 @@ package cli import ( + "fmt" + "slices" "sort" "strings" "github.com/majorcontext/moat/internal/config" + "github.com/majorcontext/moat/internal/netrules" "github.com/majorcontext/moat/internal/provider" "github.com/majorcontext/moat/internal/ui" ) @@ -97,3 +100,73 @@ func ResolveAgentField(cfg *config.Config, verb string) { } cfg.Agent = verb } + +// ExpandAgents expands moat.yaml's `agents:` list into the dependencies, +// grants, and network rules each named agent needs, deduping against what the +// config already declares. +// +// It must run BEFORE grant resolution and the network-rule loop in +// RunProvider — an expansion that lands after either contributes nothing. The +// failure is fail-closed (the agent is absent from the capability set and join +// refuses) but opaque, so ordering is a requirement, not an accident. +// +// Unknown entries are a hard error, unlike `agent:`, which warns. There is no +// legacy corpus of hallucinated `agents:` values to stay compatible with, and a +// silently dropped entry leaves the container short a credential AND its +// firewall rules — surfacing much later as an opaque join refusal or a blocked +// request under a strict network policy. +func ExpandAgents(cfg *config.Config) error { + if cfg == nil || len(cfg.Agents) == 0 { + return nil + } + for _, entry := range cfg.Agents { + if entry == "" { + return fmt.Errorf("moat.yaml `agents:` contains an empty entry; remove it or name an agent (valid: %s)", + strings.Join(KnownAgentNames(), ", ")) + } + canonical := CanonicalAgent(entry) + if canonical == "" { + return fmt.Errorf("moat.yaml `agents: [%s]` is not a known agent (valid: %s)", + entry, strings.Join(KnownAgentNames(), ", ")) + } + agent := provider.GetAgent(canonical) + rt, ok := agent.(provider.AgentRuntime) + if !ok { + return fmt.Errorf("moat.yaml `agents: [%s]` cannot be provisioned declaratively; "+ + "run it with `moat %s` instead", entry, canonical) + } + + for _, dep := range rt.DefaultDependencies() { + name := dep + if i := strings.IndexByte(dep, '@'); i >= 0 { + name = dep[:i] + } + if !HasDependency(cfg.Dependencies, name) { + cfg.Dependencies = append(cfg.Dependencies, dep) + } + } + + if grant := rt.CredentialGrant(); grant != "" && !slices.Contains(cfg.Grants, grant) { + cfg.Grants = append(cfg.Grants, grant) + } + + for _, host := range rt.NetworkHosts() { + if hasNetworkHost(cfg.Network.Rules, host) { + continue + } + cfg.Network.Rules = append(cfg.Network.Rules, + netrules.NetworkRuleEntry{HostRules: netrules.HostRules{Host: host}}) + } + } + return nil +} + +// hasNetworkHost reports whether rules already contains an entry for host. +func hasNetworkHost(rules []netrules.NetworkRuleEntry, host string) bool { + for _, r := range rules { + if r.Host == host { + return true + } + } + return false +} diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index adcac963..5f6dd810 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -12,6 +12,7 @@ package cli_test import ( "bytes" + "slices" "strings" "testing" @@ -138,6 +139,89 @@ func TestResolveAgentField(t *testing.T) { } } +func TestExpandAgents(t *testing.T) { + cfg := &config.Config{Agents: []string{"claude", "codex"}} + if err := cli.ExpandAgents(cfg); err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + + for _, dep := range []string{"claude-code", "codex-cli"} { + if !cli.HasDependency(cfg.Dependencies, dep) { + t.Errorf("expected dependency %q; got %v", dep, cfg.Dependencies) + } + } + // codex's grant is openai, not codex. + if !slices.Contains(cfg.Grants, "openai") { + t.Errorf("expected grant openai; got %v", cfg.Grants) + } + if slices.Contains(cfg.Grants, "codex") { + t.Errorf("codex must expand to the openai grant, not codex; got %v", cfg.Grants) + } + + hosts := make([]string, 0, len(cfg.Network.Rules)) + for _, r := range cfg.Network.Rules { + hosts = append(hosts, r.Host) + } + for _, want := range []string{"claude.ai", "api.openai.com"} { + if !slices.Contains(hosts, want) { + t.Errorf("expected host %q; got %v", want, hosts) + } + } +} + +func TestExpandAgentsDoesNotDuplicate(t *testing.T) { + // Companion to the expansion test: already-declared values are not repeated. + cfg := &config.Config{ + Agents: []string{"claude"}, + Dependencies: []string{"claude-code"}, + Grants: []string{"claude"}, + } + if err := cli.ExpandAgents(cfg); err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + if got := countOccurrences(cfg.Dependencies, "claude-code"); got != 1 { + t.Errorf("claude-code appears %d times, want 1: %v", got, cfg.Dependencies) + } + if got := countOccurrences(cfg.Grants, "claude"); got != 1 { + t.Errorf("claude grant appears %d times, want 1: %v", got, cfg.Grants) + } +} + +func TestExpandAgentsRejectsBadEntries(t *testing.T) { + tests := []struct { + name string + agents []string + errHas string + }{ + {"unknown name", []string{"vibrant-code"}, "vibrant-code"}, + {"non-agent provider", []string{"github"}, "github"}, + {"agent without AgentRuntime", []string{"pi"}, "pi"}, + {"empty string entry", []string{""}, "empty"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := &config.Config{Agents: tt.agents} + err := cli.ExpandAgents(cfg) + if err == nil { + t.Fatal("expected a hard error — a dropped entry silently costs a credential and firewall rules") + } + if !strings.Contains(err.Error(), tt.errHas) { + t.Errorf("error %q should name %q", err, tt.errHas) + } + }) + } +} + +func countOccurrences(list []string, want string) int { + n := 0 + for _, s := range list { + if s == want || strings.HasPrefix(s, want+"@") { + n++ + } + } + return n +} + func TestAgentFieldReachesDegradationSites(t *testing.T) { // isAIAgent is the cheapest observable proxy for the five HasPrefix call // sites in manager_create.go that a bogus agent: silently disabled. diff --git a/internal/cli/provider.go b/internal/cli/provider.go index 42ba3e6b..d6136ef3 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -130,6 +130,17 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error } } + // Expand `agents:` into dependencies/grants/network hosts before grant + // resolution and the network-rule loop below — an expansion that lands + // after either contributes nothing: the grants never reach buildGrants + // (which reads cfg.Grants immediately below) and the hosts never reach + // the proxy registration (see ExpandAgents doc comment). cfg may still be + // nil here (no moat.yaml); ExpandAgents is a no-op in that case since a + // nil config can't carry an agents: list either. + if err = ExpandAgents(cfg); err != nil { + return err + } + // Build grants list with deduplication: credential grant first, // then config grants, then flag grants. Auto-detected grants are // suppressed when they conflict with an explicit grant. diff --git a/internal/config/config.go b/internal/config/config.go index b44138ab..d0fab420 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -33,6 +33,7 @@ var imageRefRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._\-/:]*(@sha256:[a-f type Config struct { Name string `yaml:"name,omitempty"` Agent string `yaml:"agent" doc:"one of: claude, claude-code, codex, copilot, gemini, pi. Omit unless the project pins a specific agent."` + Agents []string `yaml:"agents,omitempty" doc:"agents to provision into the container, e.g. [claude, codex]. Same allowed values as agent. For moat run, the first entry is the foreground agent."` Version string `yaml:"version,omitempty"` Dependencies []string `yaml:"dependencies,omitempty"` Grants []string `yaml:"grants,omitempty"` From cfff03adfdc00feb745ac2afecda69f1e1e8ee54 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:30:40 +0000 Subject: [PATCH 13/33] feat(cli): resolve the primary agent when agents: is set --- internal/cli/agents.go | 47 ++++++++++++++++++++++++++++--------- internal/cli/agents_test.go | 35 +++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 11 deletions(-) diff --git a/internal/cli/agents.go b/internal/cli/agents.go index dc5f7e79..147439ab 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -79,26 +79,51 @@ func ValidateAgent(cfg *config.Config) { cfg.Agent = "" } -// ResolveAgentField normalizes cfg.Agent. verb is the provider name the user -// typed (e.g. "claude"), or "" for `moat run`, which has no verb. +// ResolveAgentField normalizes cfg.Agent to name the run's PRIMARY agent — the +// one launched in the foreground that owns the container lifecycle. Every other +// entry in cfg.Agents is provisioned but reachable only via `moat join`. // -// The verb always wins when there is one: if the user typed `moat claude`, the -// run is claude regardless of what moat.yaml says. Before this, a moat.yaml -// value silently overrode the command line, so `moat claude` could record a run -// as codex. +// One rule: the CLI verb always names the primary when there is one; otherwise +// `agent:` names it; if neither is set, `moat run` falls back to Agents[0]. +// verb is "" for `moat run`. func ResolveAgentField(cfg *config.Config, verb string) { if cfg == nil { return } ValidateAgent(cfg) - if verb == "" { + + if cfg.Agent != "" && len(cfg.Agents) > 0 && !agentsListContains(cfg.Agents, cfg.Agent) { + ui.Warnf("moat.yaml `agent: %s` is not in `agents: %v`; it will run as the primary but "+ + "add it to `agents:` so its dependencies and grants are provisioned.", + cfg.Agent, cfg.Agents) + } + + if verb != "" { + if cfg.Agent != "" && CanonicalAgent(cfg.Agent) != CanonicalAgent(verb) { + ui.Warnf("moat.yaml `agent: %s` conflicts with `moat %s` — using %s.", + cfg.Agent, verb, verb) + } + cfg.Agent = verb return } - if cfg.Agent != "" && CanonicalAgent(cfg.Agent) != CanonicalAgent(verb) { - ui.Warnf("moat.yaml `agent: %s` conflicts with `moat %s` — using %s.", - cfg.Agent, verb, verb) + + // No verb: `moat run`. Fall back to the first entry in agents:. + if cfg.Agent == "" && len(cfg.Agents) > 0 { + cfg.Agent = cfg.Agents[0] } - cfg.Agent = verb +} + +// agentsListContains reports whether agents contains agent, comparing by +// canonical name so documented variants (claude-code) and registry aliases +// (openai) match their canonical entry. +func agentsListContains(agents []string, agent string) bool { + want := CanonicalAgent(agent) + for _, a := range agents { + if CanonicalAgent(a) == want { + return true + } + } + return false } // ExpandAgents expands moat.yaml's `agents:` list into the dependencies, diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index 5f6dd810..8c77da4e 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -139,6 +139,41 @@ func TestResolveAgentField(t *testing.T) { } } +func TestResolveAgentFieldWithAgentsList(t *testing.T) { + tests := []struct { + name string + agent string + agents []string + verb string + wantAgent string + wantWarn bool + }{ + {"moat run falls back to agents[0]", "", []string{"claude", "codex"}, "", "claude", false}, + {"list order decides the fallback", "", []string{"codex", "claude"}, "", "codex", false}, + {"agent: still wins over agents[0]", "codex", []string{"claude", "codex"}, "", "codex", false}, + {"verb still wins over both", "codex", []string{"claude", "codex"}, "claude", "claude", true}, + {"agent: outside agents: warns", "gemini", []string{"claude", "codex"}, "", "gemini", true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cfg := &config.Config{Agent: tt.agent, Agents: tt.agents} + cli.ResolveAgentField(cfg, tt.verb) + + if cfg.Agent != tt.wantAgent { + t.Errorf("cfg.Agent = %q, want %q", cfg.Agent, tt.wantAgent) + } + if gotWarn := buf.Len() > 0; gotWarn != tt.wantWarn { + t.Errorf("warned = %v, want %v (output: %q)", gotWarn, tt.wantWarn, buf.String()) + } + }) + } +} + func TestExpandAgents(t *testing.T) { cfg := &config.Config{Agents: []string{"claude", "codex"}} if err := cli.ExpandAgents(cfg); err != nil { From 70c0a179224911cfbcef4319ad554329a101095b Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:34:59 +0000 Subject: [PATCH 14/33] feat(codex): support moat join --- internal/providers/claude/join.go | 7 +++- internal/providers/codex/join.go | 36 ++++++++++++++++ internal/providers/codex/join_test.go | 59 +++++++++++++++++++++++++++ 3 files changed, 100 insertions(+), 2 deletions(-) create mode 100644 internal/providers/codex/join.go create mode 100644 internal/providers/codex/join_test.go diff --git a/internal/providers/claude/join.go b/internal/providers/claude/join.go index 45d5c9e4..1aee3013 100644 --- a/internal/providers/claude/join.go +++ b/internal/providers/claude/join.go @@ -21,8 +21,11 @@ func (p *OAuthProvider) JoinCommand(opts provider.JoinOpts) ([]string, error) { } // IdentifiesAs reports whether a run with the given recorded Agent field was -// created by the claude provider. cfg.Agent defaults to the provider name -// ("claude") but is "claude-code" when set explicitly in moat.yaml. +// created by the claude provider. This serves only the pre-upgrade fallback path +// in `moat join` — runs created before joinable_agents existed. Runs created +// since are matched against their persisted capability set instead. +// cfg.Agent defaults to the provider name ("claude") but is "claude-code" when +// set explicitly in moat.yaml. func (p *OAuthProvider) IdentifiesAs(agent string) bool { return agent == "claude" || agent == "claude-code" } diff --git a/internal/providers/codex/join.go b/internal/providers/codex/join.go new file mode 100644 index 00000000..8d8e1d34 --- /dev/null +++ b/internal/providers/codex/join.go @@ -0,0 +1,36 @@ +package codex + +import ( + "errors" + + "github.com/majorcontext/moat/internal/provider" +) + +// JoinCommand builds the in-container command for a joined codex session, +// mirroring the BuildCommand closure in runCodex (cli.go). +// +// Approval policy and sandbox mode are deliberately absent: they live in the +// generated ~/.codex/config.toml so every launch path gets the same behavior, +// and `codex` and `codex exec` accept different flags. +func (p *Provider) JoinCommand(opts provider.JoinOpts) ([]string, error) { + // moat has never wired resume/continue for codex. Erroring is honest; + // silently dropping the flag would look like it worked. + if opts.Continue { + return nil, errors.New("codex join does not support --continue") + } + if opts.Resume != "" { + return nil, errors.New("codex join does not support --resume") + } + if opts.Prompt != "" { + return []string{"codex", "exec", opts.Prompt}, nil + } + return []string{"codex"}, nil +} + +// IdentifiesAs reports whether a run with the given recorded Agent field was +// created by the codex provider. This serves only the pre-upgrade fallback path +// in `moat join` — runs created before joinable_agents existed. Runs created +// since are matched against their persisted capability set instead. +func (p *Provider) IdentifiesAs(agent string) bool { + return agent == "codex" +} diff --git a/internal/providers/codex/join_test.go b/internal/providers/codex/join_test.go new file mode 100644 index 00000000..d6457858 --- /dev/null +++ b/internal/providers/codex/join_test.go @@ -0,0 +1,59 @@ +package codex + +import ( + "reflect" + "strings" + "testing" + + "github.com/majorcontext/moat/internal/provider" +) + +func TestJoinCommand(t *testing.T) { + p := &Provider{} + + // Interactive: bare codex, mirroring BuildCommand. + got, err := p.JoinCommand(provider.JoinOpts{}) + if err != nil { + t.Fatalf("JoinCommand: %v", err) + } + if !reflect.DeepEqual(got, []string{"codex"}) { + t.Errorf("interactive join = %v, want [codex]", got) + } + + // Companion: headless uses codex exec. + got, err = p.JoinCommand(provider.JoinOpts{Prompt: "summarize the diff"}) + if err != nil { + t.Fatalf("JoinCommand: %v", err) + } + if !reflect.DeepEqual(got, []string{"codex", "exec", "summarize the diff"}) { + t.Errorf("headless join = %v, want [codex exec summarize the diff]", got) + } +} + +func TestJoinCommandRejectsUnsupportedSessionFlags(t *testing.T) { + p := &Provider{} + + if _, err := p.JoinCommand(provider.JoinOpts{Continue: true}); err == nil { + t.Error("--continue should error rather than being silently dropped") + } else if !strings.Contains(err.Error(), "--continue") { + t.Errorf("error should name the flag; got %q", err) + } + + // Companion: --resume errors the same way. + if _, err := p.JoinCommand(provider.JoinOpts{Resume: "abc123"}); err == nil { + t.Error("--resume should error rather than being silently dropped") + } else if !strings.Contains(err.Error(), "--resume") { + t.Errorf("error should name the flag; got %q", err) + } +} + +func TestIdentifiesAs(t *testing.T) { + p := &Provider{} + if !p.IdentifiesAs("codex") { + t.Error("IdentifiesAs(codex) should be true") + } + // Companion: it must not claim other agents' runs. + if p.IdentifiesAs("claude") { + t.Error("IdentifiesAs(claude) should be false") + } +} From 3d59db1cfa626779c97f06661691d73606614cb7 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:39:12 +0000 Subject: [PATCH 15/33] test(run): cover missing-grant detection for expanded agent grants --- internal/run/grants_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/internal/run/grants_test.go b/internal/run/grants_test.go index 1bd9c121..d84f83f7 100644 --- a/internal/run/grants_test.go +++ b/internal/run/grants_test.go @@ -127,6 +127,36 @@ func TestDetectMissingGrantsMatchesValidators(t *testing.T) { } } +// agents: [claude, codex] expands to grants: [claude, openai] (Task 10). With +// no openai credential stored, the user must be told at create time — the +// capability set no longer silently excludes codex. DetectMissingGrants +// validates every grant generically via credentialStoreKey + store.Get, so +// this should already be covered with no production change. +func TestDetectMissingGrantsCoversExpandedOpenAIGrant(t *testing.T) { + store := newGrantsTestStore(t) + + missing := DetectMissingGrants([]string{"claude", "openai"}, nil, store) + found := false + for _, m := range missing { + if m.Grant == "openai" { + found = true + } + } + if !found { + t.Errorf("openai should be reported missing; got %+v", missing) + } + + // Companion: with the credential present under the openai store key, + // nothing is reported. + withCred := newGrantsTestStore(t) + if err := withCred.Save(credential.Credential{Provider: credential.ProviderOpenAI, Token: "tok"}); err != nil { + t.Fatalf("Save: %v", err) + } + if got := DetectMissingGrants([]string{"openai"}, nil, withCred); len(got) != 0 { + t.Errorf("openai should not be reported when stored; got %+v", got) + } +} + func TestClassifyMissingReason(t *testing.T) { cases := []struct { name string From 7a03aa68d2daba79dc8a5a03e04f2f512ef06c0c Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:44:31 +0000 Subject: [PATCH 16/33] feat(join): accept moat join with an inferred run --- cmd/moat/cli/join_cmd.go | 43 ++++++++++++++++++++++++++++++----- cmd/moat/cli/join_cmd_test.go | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index 32e39b3d..e69c3efc 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -17,6 +17,7 @@ import ( "github.com/majorcontext/moat/internal/provider" "github.com/majorcontext/moat/internal/run" "github.com/majorcontext/moat/internal/term" + "github.com/majorcontext/moat/internal/ui" ) var ( @@ -26,7 +27,7 @@ var ( ) var joinCmd = &cobra.Command{ - Use: "join [flags]", + Use: "join [run] [flags]", Short: "Launch another agent inside a running container", Long: `Launch a second agent inside an already-running container, reusing its workspace, grants, and credentials — without creating a new container. @@ -37,8 +38,10 @@ joins, e.g. joining claude into a run started by 'moat claude'). Examples: moat join run_a1b2c3d4e5f6 claude moat join my-feature claude --continue - moat join run_a1b2c3d4e5f6 claude -p "summarize the diff"`, - Args: cobra.MinimumNArgs(2), + moat join run_a1b2c3d4e5f6 claude -p "summarize the diff" + moat join claude # infer the run from this workspace + moat join run_a1b2c3d4e5f6 codex`, + Args: cobra.RangeArgs(1, 2), RunE: runJoin, } @@ -94,20 +97,48 @@ func joinableAgentNames() []string { return names } +// parseJoinArgs interprets join's positional arguments. +// +// Two args are ` `, unchanged. A single arg is the AGENT — it is +// the required half, while the run is what gets inferred. When that arg is also +// a run name, the agent wins and collided is set so the caller can say so; the +// two-arg form is the escape hatch. +func parseJoinArgs(args []string, isRunName func(string) bool) (runArg, agentArg string, collided bool, err error) { + if len(args) >= 2 { + return args[0], args[1], false, nil + } + arg := args[0] + if provider.GetAgent(arg) == nil { + return "", "", false, fmt.Errorf("unknown agent %q; joinable agents: %s", + arg, strings.Join(joinableAgentNames(), ", ")) + } + return "", arg, isRunName(arg), nil +} + func runJoin(cmd *cobra.Command, args []string) error { if joinContinue && joinResume != "" { return fmt.Errorf("--continue and --resume are mutually exclusive") } - runArg := args[0] - agentArg := args[1] - manager, err := run.NewManager() if err != nil { return fmt.Errorf("creating run manager: %w", err) } defer manager.Close() + isRunName := func(s string) bool { + matches, rErr := manager.Resolve(s) + return rErr == nil && len(matches) > 0 + } + runArg, agentArg, collided, err := parseJoinArgs(args, isRunName) + if err != nil { + return err + } + if collided { + ui.Warnf("%q matches both an agent and a run name; interpreting as agent.\n"+ + "Use `moat join %s ` to target the run.", agentArg, agentArg) + } + r, candidates, err := resolveRunningRunArg(manager, runArg) if err != nil { return err diff --git a/cmd/moat/cli/join_cmd_test.go b/cmd/moat/cli/join_cmd_test.go index 70a372b6..c5e332e3 100644 --- a/cmd/moat/cli/join_cmd_test.go +++ b/cmd/moat/cli/join_cmd_test.go @@ -76,6 +76,47 @@ func TestValidateJoinAgent(t *testing.T) { } } +func TestParseJoinArgs(t *testing.T) { + noRuns := func(string) bool { return false } + claudeIsARun := func(s string) bool { return s == "claude" } + + tests := []struct { + name string + args []string + isRunName func(string) bool + wantRun string + wantAgent string + wantCollid bool + wantErr bool + }{ + {"two args unchanged", []string{"run_abc", "claude"}, noRuns, "run_abc", "claude", false, false}, + {"one arg is the agent", []string{"claude"}, noRuns, "", "claude", false, false}, + {"one arg that is not an agent errors", []string{"sometypo"}, noRuns, "", "", false, true}, + {"collision resolves to the agent", []string{"claude"}, claudeIsARun, "", "claude", true, false}, + {"two-arg form escapes the collision", []string{"claude", "codex"}, claudeIsARun, "claude", "codex", false, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotRun, gotAgent, gotCollid, err := parseJoinArgs(tt.args, tt.isRunName) + if (err != nil) != tt.wantErr { + t.Fatalf("error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil { + if !strings.Contains(err.Error(), "sometypo") { + t.Errorf("error should name the bad agent; got %q", err) + } + return + } + if gotRun != tt.wantRun || gotAgent != tt.wantAgent { + t.Errorf("= (%q, %q), want (%q, %q)", gotRun, gotAgent, tt.wantRun, tt.wantAgent) + } + if gotCollid != tt.wantCollid { + t.Errorf("collided = %v, want %v", gotCollid, tt.wantCollid) + } + }) + } +} + // TestResizePump_NoSendOnClosed verifies that resizePump never sends on a // closed channel. It is intentionally run under -race to catch any concurrent // close vs send on the out channel. From 25f194722308d890588a1d00821668a5988ef428 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:49:47 +0000 Subject: [PATCH 17/33] feat(join): infer join candidates from workspace and capability --- cmd/moat/cli/joinpick.go | 69 +++++++++++++++++++++++++++++++++ cmd/moat/cli/joinpick_test.go | 72 +++++++++++++++++++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 cmd/moat/cli/joinpick.go create mode 100644 cmd/moat/cli/joinpick_test.go diff --git a/cmd/moat/cli/joinpick.go b/cmd/moat/cli/joinpick.go new file mode 100644 index 00000000..62b18d0d --- /dev/null +++ b/cmd/moat/cli/joinpick.go @@ -0,0 +1,69 @@ +package cli + +import ( + "github.com/majorcontext/moat/internal/provider" + "github.com/majorcontext/moat/internal/run" +) + +// runHostsAgent reports whether r can host agentArg. +// +// It applies the same nil-vs-empty rule as validateJoinAgent: a nil capability +// set means the run predates capability tracking, so fall back to the recorded +// agent string. Without this fallback the shorthand would report "no running +// runs can host claude" for the entire pre-upgrade population while the +// explicit two-arg form succeeded on the same run. +func runHostsAgent(r *run.Run, agentArg string, j provider.JoinableAgent) bool { + if r.JoinableAgents != nil { + for _, a := range r.JoinableAgents { + if a == agentArg || j.IdentifiesAs(a) { + return true + } + } + return false + } + return j.IdentifiesAs(r.Agent) +} + +// hostedAgents returns the agent names to show in the picker's AGENTS column, +// deriving them from the recorded agent string for pre-upgrade runs. +// +//nolint:unused // wired into the picker's AGENTS column by Task 16 +func hostedAgents(r *run.Run) []string { + if r.JoinableAgents != nil { + return r.JoinableAgents + } + if r.Agent != "" { + return []string{r.Agent} + } + return nil +} + +// inferJoinCandidates narrows runs to those that can host agentArg, preferring +// the current workspace. widened reports that no run in cwd qualified and the +// search covered every running run — the caller must disclose that, because +// attaching to another workspace's run means using that run's grants. +// +//nolint:unparam // cwd is a literal only in today's tests; Task 16 wires runJoin's os.Getwd() through here +func inferJoinCandidates(runs []*run.Run, cwd, agentArg string, j provider.JoinableAgent) (candidates []*run.Run, widened bool) { + var all []*run.Run + for _, r := range runs { + if r.GetState() != run.StateRunning { + continue + } + if !runHostsAgent(r, agentArg, j) { + continue + } + all = append(all, r) + } + + var local []*run.Run + for _, r := range all { + if r.Workspace == cwd { + local = append(local, r) + } + } + if len(local) > 0 { + return local, false + } + return all, len(all) > 0 +} diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go new file mode 100644 index 00000000..794a4c03 --- /dev/null +++ b/cmd/moat/cli/joinpick_test.go @@ -0,0 +1,72 @@ +package cli + +import ( + "testing" + + "github.com/majorcontext/moat/internal/run" +) + +func TestRunHostsAgent(t *testing.T) { + claude := fakeJoinable{names: []string{"claude", "claude-code"}} + + tests := []struct { + name string + r *run.Run + want bool + }{ + {"member of the set", &run.Run{JoinableAgents: []string{"claude"}}, true}, + {"non-member", &run.Run{JoinableAgents: []string{"codex"}}, false}, + {"empty set hosts nothing", &run.Run{JoinableAgents: []string{}}, false}, + {"nil set falls back to the agent string", &run.Run{Agent: "claude", JoinableAgents: nil}, true}, + {"nil set with a stale agent string", &run.Run{Agent: "vibrant-code", JoinableAgents: nil}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := runHostsAgent(tt.r, "claude", claude); got != tt.want { + t.Errorf("runHostsAgent() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestInferJoinCandidates(t *testing.T) { + claude := fakeJoinable{names: []string{"claude"}} + here := &run.Run{ID: "run_here", Workspace: "/work", State: run.StateRunning, JoinableAgents: []string{"claude"}} + elsewhere := &run.Run{ID: "run_far", Workspace: "/other", State: run.StateRunning, JoinableAgents: []string{"claude"}} + stopped := &run.Run{ID: "run_old", Workspace: "/work", State: run.StateStopped, JoinableAgents: []string{"claude"}} + codexOnly := &run.Run{ID: "run_cx", Workspace: "/work", State: run.StateRunning, JoinableAgents: []string{"codex"}} + + t.Run("prefers the current workspace", func(t *testing.T) { + got, widened := inferJoinCandidates([]*run.Run{here, elsewhere}, "/work", "claude", claude) + if len(got) != 1 || got[0].ID != "run_here" { + t.Errorf("expected only the cwd run; got %v", got) + } + if widened { + t.Error("should not widen when the workspace has a match") + } + }) + + t.Run("widens when the workspace has none", func(t *testing.T) { + got, widened := inferJoinCandidates([]*run.Run{elsewhere}, "/work", "claude", claude) + if len(got) != 1 || got[0].ID != "run_far" { + t.Errorf("expected the widened match; got %v", got) + } + if !widened { + t.Error("should report widening so the caller can disclose it") + } + }) + + t.Run("excludes stopped runs", func(t *testing.T) { + got, _ := inferJoinCandidates([]*run.Run{stopped}, "/work", "claude", claude) + if len(got) != 0 { + t.Errorf("stopped runs must never be candidates; got %v", got) + } + }) + + t.Run("capability filter narrows to the viable run", func(t *testing.T) { + got, _ := inferJoinCandidates([]*run.Run{codexOnly, here}, "/work", "claude", claude) + if len(got) != 1 || got[0].ID != "run_here" { + t.Errorf("expected only the claude-capable run; got %v", got) + } + }) +} From 9f27b90b08db8fee69d842f23eb51d53aaf17b06 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 00:54:47 +0000 Subject: [PATCH 18/33] feat(join): add an interactive run picker for the shorthand --- cmd/moat/cli/join_cmd.go | 41 ++++++++++---- cmd/moat/cli/joinpick.go | 101 +++++++++++++++++++++++++++++++-- cmd/moat/cli/joinpick_test.go | 103 ++++++++++++++++++++++++++++++++++ 3 files changed, 230 insertions(+), 15 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index e69c3efc..ecbe3888 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -139,17 +139,8 @@ func runJoin(cmd *cobra.Command, args []string) error { "Use `moat join %s ` to target the run.", agentArg, agentArg) } - r, candidates, err := resolveRunningRunArg(manager, runArg) - if err != nil { - return err - } - if r == nil { - // Several running runs share this name. Task 17 routes these to the - // picker; until then, list them and ask for an ID. - printMatchingRuns(candidates, runArg) - return fmt.Errorf("name %q matches %d running runs; specify a run ID", runArg, len(candidates)) - } - + // The agent/provider lookup happens before run resolution: the shorthand + // path needs `joinable` to filter candidates by hosting capability. agent := provider.GetAgent(agentArg) if agent == nil { return fmt.Errorf("unknown agent %q; joinable agents: %s", agentArg, strings.Join(joinableAgentNames(), ", ")) @@ -158,6 +149,34 @@ func runJoin(cmd *cobra.Command, args []string) error { if !ok { return fmt.Errorf("agent %q does not support join yet", agentArg) } + + var r *run.Run + if runArg == "" { + cwd, cwdErr := os.Getwd() + if cwdErr != nil { + return fmt.Errorf("resolving working directory: %w", cwdErr) + } + candidates, widened := inferJoinCandidates(manager.List(), cwd, agentArg, joinable) + picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, widened, + term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr)) + if pickErr != nil { + return pickErr + } + r = picked + } else { + var candidates []*run.Run + r, candidates, err = resolveRunningRunArg(manager, runArg) + if err != nil { + return err + } + if r == nil { + // Several running runs share this name. Task 17 routes these to the + // picker; until then, list them and ask for an ID. + printMatchingRuns(candidates, runArg) + return fmt.Errorf("name %q matches %d running runs; specify a run ID", runArg, len(candidates)) + } + } + if valErr := validateJoinAgent(joinable, agentArg, r); valErr != nil { return valErr } diff --git a/cmd/moat/cli/joinpick.go b/cmd/moat/cli/joinpick.go index 62b18d0d..4acabdcf 100644 --- a/cmd/moat/cli/joinpick.go +++ b/cmd/moat/cli/joinpick.go @@ -1,6 +1,13 @@ package cli import ( + "bufio" + "fmt" + "io" + "strconv" + "strings" + "text/tabwriter" + "github.com/majorcontext/moat/internal/provider" "github.com/majorcontext/moat/internal/run" ) @@ -26,8 +33,6 @@ func runHostsAgent(r *run.Run, agentArg string, j provider.JoinableAgent) bool { // hostedAgents returns the agent names to show in the picker's AGENTS column, // deriving them from the recorded agent string for pre-upgrade runs. -// -//nolint:unused // wired into the picker's AGENTS column by Task 16 func hostedAgents(r *run.Run) []string { if r.JoinableAgents != nil { return r.JoinableAgents @@ -42,8 +47,6 @@ func hostedAgents(r *run.Run) []string { // the current workspace. widened reports that no run in cwd qualified and the // search covered every running run — the caller must disclose that, because // attaching to another workspace's run means using that run's grants. -// -//nolint:unparam // cwd is a literal only in today's tests; Task 16 wires runJoin's os.Getwd() through here func inferJoinCandidates(runs []*run.Run, cwd, agentArg string, j provider.JoinableAgent) (candidates []*run.Run, widened bool) { var all []*run.Run for _, r := range runs { @@ -67,3 +70,93 @@ func inferJoinCandidates(runs []*run.Run, cwd, agentArg string, j provider.Joina } return all, len(all) > 0 } + +// renderPicker writes the numbered candidate table. +// +// Writes to the caller-supplied writer, which is os.Stderr in production — +// matching printMatchingRuns and disambiguateRuns. CLAUDE.md's "write command +// output to stdout" rule covers results, not interactive prompts: a picker on +// stdout hangs invisibly under `moat join claude | tee log` when stdin is still +// a TTY. No ui style functions inside the tabwriter — ANSI codes break column +// alignment. +func renderPicker(w io.Writer, candidates []*run.Run, agentArg string, widened bool) { + if widened { + fmt.Fprintf(w, "No running runs in this workspace can host %s — showing all running runs:\n\n", agentArg) + } else { + fmt.Fprintf(w, "Multiple running runs can host %s:\n\n", agentArg) + } + + tw := tabwriter.NewWriter(w, 0, 0, 2, ' ', 0) + if widened { + fmt.Fprintln(tw, " NAME\tRUN ID\tAGENTS\tAGE\tWORKSPACE") + } else { + fmt.Fprintln(tw, " NAME\tRUN ID\tAGENTS\tAGE") + } + for i, r := range candidates { + agents := strings.Join(hostedAgents(r), ", ") + if agents == "" { + agents = "-" + } + if widened { + fmt.Fprintf(tw, " %d %s\t%s\t%s\t%s\t%s\n", + i+1, r.Name, r.ID, agents, formatTimeAgo(r.CreatedAt), r.Workspace) + continue + } + fmt.Fprintf(tw, " %d %s\t%s\t%s\t%s\n", + i+1, r.Name, r.ID, agents, formatTimeAgo(r.CreatedAt)) + } + tw.Flush() + fmt.Fprintln(w) +} + +// readSelection reads a 1-based choice in [1, n]. +// +// Invalid input aborts rather than re-prompting, matching disambiguateRuns's +// abort-on-invalid-input convention. Re-prompting in a loop is a trap for +// scripted callers whose stdin never produces a valid answer. +func readSelection(r io.Reader, n int) (int, error) { + line, err := bufio.NewReader(r).ReadString('\n') + if err != nil && line == "" { + return 0, fmt.Errorf("no selection made; run `moat join ` to specify directly") + } + choice, convErr := strconv.Atoi(strings.TrimSpace(line)) + if convErr != nil || choice < 1 || choice > n { + return 0, fmt.Errorf("invalid selection %q; run `moat join ` to specify directly", + strings.TrimSpace(line)) + } + return choice, nil +} + +// pickJoinRun resolves a candidate list to one run. +// +// A single candidate auto-selects UNLESS the search was widened past the +// current workspace: attaching to another workspace's run silently borrows that +// run's grants and network policy, so it is confirmed rather than assumed. +func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg string, widened, isTTY bool) (*run.Run, error) { + switch len(candidates) { + case 0: + return nil, fmt.Errorf("no running run can host %s in this workspace.\n"+ + "Start one with `moat %s`, or run `moat list` to see what is running.", agentArg, agentArg) + case 1: + if !widened { + return candidates[0], nil + } + } + + if !isTTY { + ids := make([]string, len(candidates)) + for i, r := range candidates { + ids[i] = r.ID + } + return nil, fmt.Errorf("%d running runs can host %s; specify one: %s", + len(candidates), agentArg, strings.Join(ids, ", ")) + } + + renderPicker(out, candidates, agentArg, widened) + fmt.Fprintf(out, "Select [1-%d]: ", len(candidates)) + choice, err := readSelection(in, len(candidates)) + if err != nil { + return nil, err + } + return candidates[choice-1], nil +} diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go index 794a4c03..97258918 100644 --- a/cmd/moat/cli/joinpick_test.go +++ b/cmd/moat/cli/joinpick_test.go @@ -1,6 +1,9 @@ package cli import ( + "bytes" + "io" + "strings" "testing" "github.com/majorcontext/moat/internal/run" @@ -70,3 +73,103 @@ func TestInferJoinCandidates(t *testing.T) { } }) } + +func TestReadSelection(t *testing.T) { + tests := []struct { + name string + input string + n int + want int + wantErr bool + }{ + {"valid choice", "2\n", 3, 2, false}, + {"first choice", "1\n", 2, 1, false}, + {"out of range high", "5\n", 2, 0, true}, + {"out of range low", "0\n", 2, 0, true}, + {"non-numeric", "abc\n", 2, 0, true}, + {"EOF", "", 2, 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := readSelection(strings.NewReader(tt.input), tt.n) + if (err != nil) != tt.wantErr { + t.Fatalf("error = %v, wantErr %v", err, tt.wantErr) + } + if err != nil { + // Invalid input aborts; it never retries. + if !strings.Contains(err.Error(), "moat join ") { + t.Errorf("error should point at the explicit form; got %q", err) + } + return + } + if got != tt.want { + t.Errorf("readSelection() = %d, want %d", got, tt.want) + } + }) + } +} + +func TestRenderPickerHeaders(t *testing.T) { + candidates := []*run.Run{ + {ID: "run_a1b2c3d4e5f6", Name: "happy-otter", Workspace: "/work", JoinableAgents: []string{"claude"}}, + } + + var normal bytes.Buffer + renderPicker(&normal, candidates, "claude", false) + if !strings.Contains(normal.String(), "Multiple running runs can host claude") { + t.Errorf("unexpected header: %q", normal.String()) + } + if strings.Contains(normal.String(), "WORKSPACE") { + t.Error("non-widened picker should not show the WORKSPACE column") + } + + // Companion: the widened header says so and shows workspaces, because the + // user is about to attach outside their current directory. + var widened bytes.Buffer + renderPicker(&widened, candidates, "claude", true) + if !strings.Contains(widened.String(), "No running runs in this workspace") { + t.Errorf("widened header missing: %q", widened.String()) + } + if !strings.Contains(widened.String(), "WORKSPACE") { + t.Error("widened picker should show the WORKSPACE column") + } +} + +func TestPickJoinRun(t *testing.T) { + one := &run.Run{ID: "run_only", Name: "solo", JoinableAgents: []string{"claude"}} + two := &run.Run{ID: "run_two", Name: "other", JoinableAgents: []string{"claude"}} + + t.Run("single candidate auto-selects", func(t *testing.T) { + got, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", false, true) + if err != nil || got != one { + t.Errorf("expected auto-select; got %v, %v", got, err) + } + }) + + t.Run("single WIDENED candidate still prompts", func(t *testing.T) { + // Companion to the auto-select case. Attaching to another workspace's + // run means using that run's credentials, so it must be confirmed. + got, err := pickJoinRun(strings.NewReader("1\n"), io.Discard, []*run.Run{one}, "claude", true, true) + if err != nil || got != one { + t.Errorf("expected prompted selection; got %v, %v", got, err) + } + }) + + t.Run("non-TTY errors with the IDs", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one, two}, "claude", false, false) + if err == nil { + t.Fatal("expected an error without a TTY") + } + for _, id := range []string{"run_only", "run_two"} { + if !strings.Contains(err.Error(), id) { + t.Errorf("error should list %s; got %q", id, err) + } + } + }) + + t.Run("zero candidates errors", func(t *testing.T) { + if _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true); err == nil { + t.Error("expected an error with no candidates") + } + }) +} From c230ba2a6f5082476c7c105cbf929cbfcf3a721e Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:04:32 +0000 Subject: [PATCH 19/33] fix(join): sort picker candidates and disambiguate zero-host errors - inferJoinCandidates now sorts by CreatedAt (newest-first) before partitioning into local/widened, since manager.List() iterates a map and is otherwise unordered per call. Without this, the picker's slice-index numbering shuffled between invocations, letting a remembered selection attach to a different run (and, when widened, a different workspace's grants). - pickJoinRun gains an anyRunning parameter so the zero-candidate error can distinguish nothing running at all (start a run) from running runs that exist but can't host this agent (recreate one with it in moat.yaml's agents: list) - these imply different next steps. - Strengthened the single-WIDENED-candidate test to assert on the picker's actual output instead of just its return value, so deleting the widened guard is caught. --- cmd/moat/cli/join_cmd.go | 6 ++- cmd/moat/cli/joinpick.go | 27 +++++++++++-- cmd/moat/cli/joinpick_test.go | 75 ++++++++++++++++++++++++++++++++--- 3 files changed, 97 insertions(+), 11 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index ecbe3888..205ea11c 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -156,9 +156,11 @@ func runJoin(cmd *cobra.Command, args []string) error { if cwdErr != nil { return fmt.Errorf("resolving working directory: %w", cwdErr) } - candidates, widened := inferJoinCandidates(manager.List(), cwd, agentArg, joinable) + allRuns := manager.List() + candidates, widened := inferJoinCandidates(allRuns, cwd, agentArg, joinable) + anyRunning := len(filterRunning(allRuns)) > 0 picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, widened, - term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr)) + term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr), anyRunning) if pickErr != nil { return pickErr } diff --git a/cmd/moat/cli/joinpick.go b/cmd/moat/cli/joinpick.go index 4acabdcf..8d43c81c 100644 --- a/cmd/moat/cli/joinpick.go +++ b/cmd/moat/cli/joinpick.go @@ -47,6 +47,14 @@ func hostedAgents(r *run.Run) []string { // the current workspace. widened reports that no run in cwd qualified and the // search covered every running run — the caller must disclose that, because // attaching to another workspace's run means using that run's grants. +// +// Results are sorted newest-first, matching every other multi-match surface +// in this CLI (resolve.go's SortRunsByCreatedAt). runs typically comes from +// manager.List(), which iterates a map and is unordered per call — without +// this sort, renderPicker's slice-index numbering would shuffle between +// invocations, so a number the user remembers from one run of the picker +// could attach to a different run (and, when widened, a different +// workspace's grants) the next time. func inferJoinCandidates(runs []*run.Run, cwd, agentArg string, j provider.JoinableAgent) (candidates []*run.Run, widened bool) { var all []*run.Run for _, r := range runs { @@ -58,6 +66,7 @@ func inferJoinCandidates(runs []*run.Run, cwd, agentArg string, j provider.Joina } all = append(all, r) } + run.SortRunsByCreatedAt(all) var local []*run.Run for _, r := range all { @@ -132,11 +141,23 @@ func readSelection(r io.Reader, n int) (int, error) { // A single candidate auto-selects UNLESS the search was widened past the // current workspace: attaching to another workspace's run silently borrows that // run's grants and network policy, so it is confirmed rather than assumed. -func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg string, widened, isTTY bool) (*run.Run, error) { +// +// anyRunning distinguishes the two zero-candidate causes, which imply +// different next steps: no running runs anywhere (start one) vs. running runs +// exist but none can host agentArg (add it to moat.yaml's agents: list and +// recreate). candidates alone can't tell these apart — by the time it is +// empty, inferJoinCandidates has already filtered by capability across every +// running run, local or not — so the caller must supply the distinction from +// the unfiltered population. +func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg string, widened, isTTY, anyRunning bool) (*run.Run, error) { switch len(candidates) { case 0: - return nil, fmt.Errorf("no running run can host %s in this workspace.\n"+ - "Start one with `moat %s`, or run `moat list` to see what is running.", agentArg, agentArg) + if !anyRunning { + return nil, fmt.Errorf("no runs are running; start one with `moat %s`.", agentArg) + } + return nil, fmt.Errorf("no running run can host %s.\n"+ + "Add %s to this project's moat.yaml `agents:` list and recreate the run, or run `moat list` to see what is running.", + agentArg, agentArg) case 1: if !widened { return candidates[0], nil diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go index 97258918..a93e913a 100644 --- a/cmd/moat/cli/joinpick_test.go +++ b/cmd/moat/cli/joinpick_test.go @@ -5,6 +5,7 @@ import ( "io" "strings" "testing" + "time" "github.com/majorcontext/moat/internal/run" ) @@ -72,6 +73,37 @@ func TestInferJoinCandidates(t *testing.T) { t.Errorf("expected only the claude-capable run; got %v", got) } }) + + // manager.List() (the real caller) iterates a map and is unordered per + // call. renderPicker numbers candidates by slice index, so without a + // deterministic sort the same run gets a different number across + // invocations — a remembered "2" could attach to a different run. + older := &run.Run{ID: "run_old", Workspace: "/work", State: run.StateRunning, JoinableAgents: []string{"claude"}, CreatedAt: time.Unix(100, 0)} + newer := &run.Run{ID: "run_new", Workspace: "/work", State: run.StateRunning, JoinableAgents: []string{"claude"}, CreatedAt: time.Unix(200, 0)} + elsewhereOlder := &run.Run{ID: "run_old_far", Workspace: "/other", State: run.StateRunning, JoinableAgents: []string{"claude"}, CreatedAt: time.Unix(100, 0)} + elsewhereNewer := &run.Run{ID: "run_new_far", Workspace: "/another", State: run.StateRunning, JoinableAgents: []string{"claude"}, CreatedAt: time.Unix(200, 0)} + + t.Run("sorts local candidates newest-first regardless of input order", func(t *testing.T) { + got, widened := inferJoinCandidates([]*run.Run{older, newer}, "/work", "claude", claude) + if widened { + t.Fatal("expected a local match, not widened") + } + if len(got) != 2 || got[0].ID != "run_new" || got[1].ID != "run_old" { + t.Errorf("expected newest-first order; got %v", got) + } + }) + + // Companion: the widened path returns a different slice (`all` instead of + // `local`), so the sort must cover it too, not just the local branch. + t.Run("sorts widened candidates newest-first regardless of input order", func(t *testing.T) { + got, widened := inferJoinCandidates([]*run.Run{elsewhereOlder, elsewhereNewer}, "/work", "claude", claude) + if !widened { + t.Fatal("expected widening since neither run is in /work") + } + if len(got) != 2 || got[0].ID != "run_new_far" || got[1].ID != "run_old_far" { + t.Errorf("expected newest-first order; got %v", got) + } + }) } func TestReadSelection(t *testing.T) { @@ -140,7 +172,7 @@ func TestPickJoinRun(t *testing.T) { two := &run.Run{ID: "run_two", Name: "other", JoinableAgents: []string{"claude"}} t.Run("single candidate auto-selects", func(t *testing.T) { - got, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", false, true) + got, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", false, true, true) if err != nil || got != one { t.Errorf("expected auto-select; got %v, %v", got, err) } @@ -149,14 +181,24 @@ func TestPickJoinRun(t *testing.T) { t.Run("single WIDENED candidate still prompts", func(t *testing.T) { // Companion to the auto-select case. Attaching to another workspace's // run means using that run's credentials, so it must be confirmed. - got, err := pickJoinRun(strings.NewReader("1\n"), io.Discard, []*run.Run{one}, "claude", true, true) + // Assert the picker actually ran (via its output), not just the return + // value — the return value alone is satisfied even if the widened + // guard is deleted and case 1 falls straight through to auto-select. + var out bytes.Buffer + got, err := pickJoinRun(strings.NewReader("1\n"), &out, []*run.Run{one}, "claude", true, true, true) if err != nil || got != one { t.Errorf("expected prompted selection; got %v, %v", got, err) } + if !strings.Contains(out.String(), "No running runs in this workspace") { + t.Errorf("expected the widened picker to actually render; got %q", out.String()) + } + if !strings.Contains(out.String(), "Select [1-1]:") { + t.Errorf("expected a selection prompt to actually be shown; got %q", out.String()) + } }) t.Run("non-TTY errors with the IDs", func(t *testing.T) { - _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one, two}, "claude", false, false) + _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one, two}, "claude", false, false, true) if err == nil { t.Fatal("expected an error without a TTY") } @@ -167,9 +209,30 @@ func TestPickJoinRun(t *testing.T) { } }) - t.Run("zero candidates errors", func(t *testing.T) { - if _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true); err == nil { - t.Error("expected an error with no candidates") + t.Run("zero candidates, nothing running at all", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true, false) + if err == nil { + t.Fatal("expected an error with no candidates") + } + if !strings.Contains(err.Error(), "start one") { + t.Errorf("error should tell the user to start a run; got %q", err) + } + }) + + // Companion: candidates are empty either because nothing is running at + // all, or because something is running but can't host this agent. Those + // imply different next steps (start a run vs. recreate one with this + // agent), so the messages must differ. + t.Run("zero candidates, runs are running but none can host the agent", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true, true) + if err == nil { + t.Fatal("expected an error with no candidates") + } + if !strings.Contains(err.Error(), "agents:") { + t.Errorf("error should point at moat.yaml's agents: list; got %q", err) + } + if strings.Contains(err.Error(), "start one") { + t.Errorf("this case must not suggest starting a run; got %q", err) } }) } From ec0b3c936f322cca2793f5d2a46fe2981a62bba7 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:09:14 +0000 Subject: [PATCH 20/33] fix(join): pick between same-named running runs instead of erroring --- cmd/moat/cli/join_cmd.go | 18 ++++++++++++++---- cmd/moat/cli/joinpick_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index 205ea11c..3dbe4b3f 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -172,10 +172,20 @@ func runJoin(cmd *cobra.Command, args []string) error { return err } if r == nil { - // Several running runs share this name. Task 17 routes these to the - // picker; until then, list them and ask for an ID. - printMatchingRuns(candidates, runArg) - return fmt.Errorf("name %q matches %d running runs; specify a run ID", runArg, len(candidates)) + // Several running runs share this name — nothing enforces run-name + // uniqueness, and moat.yaml's `name:` field means every run in a + // project commonly shares one. Route through the same picker the + // shorthand form uses (Task 16) rather than erroring, so the + // explicit and shorthand forms behave the same way. widened=false: + // the user named a run explicitly, so there was no workspace-widening + // search to disclose. anyRunning=true: resolveRunningRunArg only + // returns a candidate list when more than one running run matched. + picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, false, + term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr), true) + if pickErr != nil { + return pickErr + } + r = picked } } diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go index a93e913a..d29d11a5 100644 --- a/cmd/moat/cli/joinpick_test.go +++ b/cmd/moat/cli/joinpick_test.go @@ -236,3 +236,33 @@ func TestPickJoinRun(t *testing.T) { } }) } + +// TestTwoArgMultiMatchUsesPicker locks in the shape runJoin's two-arg path +// relies on: when resolveRunningRunArg finds several running runs sharing a +// name (common, since nothing enforces run-name uniqueness and moat.yaml's +// `name:` field puts every run in a project under one name), pickJoinRun +// must pick interactively rather than error — matching the shorthand form. +func TestTwoArgMultiMatchUsesPicker(t *testing.T) { + a := &run.Run{ID: "run_a", Name: "moat-dev", State: run.StateRunning, JoinableAgents: []string{"claude"}} + b := &run.Run{ID: "run_b", Name: "moat-dev", State: run.StateRunning, JoinableAgents: []string{"claude"}} + + // TTY: selecting 2 picks the second candidate. + got, err := pickJoinRun(strings.NewReader("2\n"), io.Discard, []*run.Run{a, b}, "claude", false, true, true) + if err != nil { + t.Fatalf("pickJoinRun: %v", err) + } + if got.ID != "run_b" { + t.Errorf("selected %s, want run_b", got.ID) + } + + // Companion: no TTY still errors with both IDs listed. + _, err = pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{a, b}, "claude", false, false, true) + if err == nil { + t.Fatal("non-TTY should error rather than prompt") + } + for _, id := range []string{"run_a", "run_b"} { + if !strings.Contains(err.Error(), id) { + t.Errorf("error should list %s; got %q", id, err) + } + } +} From dd114c1a90ee1dff261903e5c59167718f0ebbe1 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:27:46 +0000 Subject: [PATCH 21/33] docs: correct agent: reference and document multi-agent joins --- CHANGELOG.md | 6 ++ docs/content/guides/14-multi-agent.md | 78 +++++++++++++++++++++----- docs/content/reference/01-cli.md | 25 +++++++-- docs/content/reference/02-moat-yaml.md | 36 ++++++++++-- 4 files changed, 123 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f33df99e..5dcf9003 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ A second pass fixes the terminal handling that made Codex hard to use inside Moa - **`MOAT_TTY_TRACE`** — an environment-variable equivalent of `--tty-trace`, recording a session's terminal I/O to a file for debugging TUI and input problems. The sessions worth tracing are the broken ones, where the in-session `ctrl+/ d` dump may itself be unreachable, and exporting a variable captures every subsequent run without editing each command line. See [Environment variables](https://majorcontext.com/moat/reference/environment). ([#450](https://github.com/majorcontext/moat/pull/450)) - **Remote MCP servers for Codex** — top-level `mcp:` entries are now wired into Codex, not just Claude Code. They are written to the `[mcp_servers]` table of the generated `~/.codex/config.toml` as streamable HTTP servers whose `url` points at the proxy relay, so the proxy injects the real credential exactly as it does for Claude Code. Previously `mcp:` was silently ignored for Codex runs. See [MCP servers](https://majorcontext.com/moat/guides/mcp). ([#449](https://github.com/majorcontext/moat/pull/449)) +- **Multi-agent containers** — `agents: [claude, codex]` in `moat.yaml` provisions several agents into one container, each with its dependencies, credential grant, and network rules. `moat join codex` then works in a container started by `moat claude`. See [Multi-agent sessions](https://majorcontext.com/moat/guides/multi-agent). ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- `moat join ` infers the run from the current workspace, offering a picker when several qualify. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) ### Changed @@ -32,6 +34,10 @@ A second pass fixes the terminal handling that made Codex hard to use inside Moa - Fix `codex.sync_logs` never syncing anything — the setting was documented as writing session logs to the host, and defaulted on whenever the `openai` grant was configured, but no mount was ever created: the flag only influenced whether the Codex staging directory was built. Codex session transcripts now appear on the host at `~/.moat/codex/sessions/-/YYYY/MM/DD/rollout-*.jsonl`, in Codex's own format. ([#449](https://github.com/majorcontext/moat/pull/449)) - Fix Codex prompting to trust `/workspace` on first run — the generated config now marks the workspace trusted. ([#449](https://github.com/majorcontext/moat/pull/449)) - `codex.mcp` and `gemini.mcp` may now both declare local MCP servers in one `moat.yaml`. They previously collided on `/workspace/.mcp.json` and were rejected at config load; Codex no longer uses that file. ([#449](https://github.com/majorcontext/moat/pull/449)) +- Fix `moat join` refusing to attach to containers that were running the agent — previously, join compared `moat.yaml`'s `agent:` field against a fixed list, so any other value (including the project-shaped names the reference docs and `moat init` both produced) made every join fail. Join now uses the set of agents moat actually provisioned into the container. Runs created before this change must be recreated. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- Fix `moat join ` failing when several runs share a name — previously, any project setting `name:` in `moat.yaml` gave every run the same name, and a second concurrent run made join error instead of offering a choice. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- Fix `moat join`'s extra positional arguments being silently ignored — `moat join ` used to accept and discard ``; it is now a usage error. `moat join` now takes one or two positional arguments (`[run] agent`), not a minimum of two. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- Fix the `agent:` reference documentation, which described the field as a free-form identifier defaulting to `name`. It is a fixed set of agent names and defaults to the command you ran. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) ### Breaking diff --git a/docs/content/guides/14-multi-agent.md b/docs/content/guides/14-multi-agent.md index cb74f2b2..5a2ec4b1 100644 --- a/docs/content/guides/14-multi-agent.md +++ b/docs/content/guides/14-multi-agent.md @@ -1,8 +1,8 @@ --- title: "Multi-agent sessions" navTitle: "Multi-agent" -description: "Run a second agent inside an already-running container using moat join." -keywords: ["moat", "join", "multi-agent", "claude", "parallel", "session"] +description: "Run several agents in one container and join them with moat join." +keywords: ["moat", "join", "multi-agent", "claude", "codex", "parallel", "session"] --- # Multi-agent sessions @@ -15,7 +15,9 @@ A `moat join` session is an exec child of the existing container. The original a Console output is split: the primary agent writes to `logs.jsonl`; each joined agent writes to `logs..jsonl`. -## Quick start +The agent you join must be one moat actually provisioned into the container: the agent the run was started with, or any agent listed in that project's [`agents:`](../reference/02-moat-yaml.md#agents). Joining an agent moat never provisioned fails with a clear error naming what the run can host. + +## Quick start: same agent, second session In a first terminal, start a claude run: @@ -32,6 +34,57 @@ moat join run_a1b2c3d4e5f6 claude The second terminal opens an interactive claude session in the same workspace. The status footer on the join shows `joined · 1`; the primary's footer shows `primary +1`. +## Cross-agent joins with `agents:` + +By default, a run only provisions the agent you started it with — `moat claude` provisions claude, and only claude can join it. To make a second agent joinable, list it in `moat.yaml`'s [`agents:`](../reference/02-moat-yaml.md#agents): + +```yaml +agents: [claude, codex] +``` + +This provisions both agents' dependencies, credential grants, and network rules into the container. With no `agent:` field set, `moat run` launches the first entry (`claude`) in the foreground; `codex` is reachable only via `moat join`: + +```bash +moat run +# or: moat claude, since agent: is unset and agents[0] is claude + +# from a second terminal +moat join run_a1b2c3d4e5f6 codex +``` + +Order in `agents:` matters only for picking the foreground agent — every entry after the first is equally joinable. + +## Joining without a run ID + +`moat join ` infers the run instead of requiring one: + +```bash +moat join claude +``` + +Resolution narrows candidates in this order: + +1. Only `running` runs are considered. +2. Runs are filtered to ones that can host the named agent (started with it, or provisioning it via `agents:`). +3. If any qualifying run's workspace matches the current directory, only those are offered; otherwise every qualifying run is offered, and moat says so, because attaching to another workspace's run means using that run's grants and network policy. +4. A single candidate in the current workspace attaches immediately. Multiple candidates print a numbered table, newest run first, and prompt for a selection: + +``` +Multiple running runs can host claude: + + NAME RUN ID AGENTS AGE + 1 my-feature run_a1b2c3d4e5f6 claude, codex 2m + 2 my-feature run_9f8e7d6c5b4a claude 14m + +Select [1-2]: +``` + +The table and prompt are written to **stderr**, not stdout — `moat join claude | tee log` still shows the picker even though stdin is a TTY. A non-interactive caller (no TTY, or more than one candidate arriving over a pipe) gets an error listing the candidate run IDs instead of a prompt. + +If no run anywhere is running, the error says so directly. If runs are running but none can host the agent, the error says that instead and points at `agents:`. + +If the single argument you pass names both a known agent and a run (for example, a project whose runs happen to be named `claude`), moat interprets it as the agent and warns, showing the two-argument form to target the run explicitly. + ## Headless join Use `--prompt` / `-p` to run a join non-interactively and exit when done: @@ -68,19 +121,16 @@ The original run owns the container. Joined agents are exec children: - A joined agent exiting (or being interrupted) does not affect the primary or other joins. - `moat destroy` removes the run after it is stopped; joined log files (`logs.N.jsonl`) are removed with the run. -## v1 constraints - -v1 supports same-agent joins only. The agent argument must match the agent the run was created with: - -```bash -# Works: joining claude into a moat claude run -moat join run_a1b2c3d4e5f6 claude +## What agents in one container share -# Error: run has no codex configuration -moat join run_a1b2c3d4e5f6 codex -``` +Every agent and process in a container shares the run's single proxy token, so +**every credential granted to the run is reachable by every agent in it**. A +container with `agents: [claude, codex]` lets either agent reach both the +Anthropic and the OpenAI credential. Use separate runs when the agents should +not share credentials. -Cross-provider join (e.g. running codex inside a claude run) and container-side worktree joins (`moat join … --wt`) are not supported in v1. +Joined agents also share the primary's git working tree and can contend over it. +Container-side worktree isolation is not implemented. ## Relationship to moat exec diff --git a/docs/content/reference/01-cli.md b/docs/content/reference/01-cli.md index 14252beb..8004dc1e 100644 --- a/docs/content/reference/01-cli.md +++ b/docs/content/reference/01-cli.md @@ -1253,19 +1253,30 @@ moat exec run_a1b2c3d4e5f6 -- sh -c "ps aux" Launch a second agent inside a running container, reusing its workspace, grants, and credentials. ``` -moat join [flags] +moat join [run] [flags] ``` `moat join` is the run-first counterpart to `moat exec`: where `exec` runs an arbitrary command, `join` resolves an agent provider by name, constructs its standard in-container invocation, and execs it into the existing container. The original run owns the container lifecycle — stopping the run tears down the container and any joined agents. -v1 supports same-agent joins only (e.g. joining `claude` into a run started by `moat claude`). The agent argument must match the agent the run was created with. +The agent must be one moat actually provisioned into the target container — either the agent the run was started with, or one listed in that project's `agents:` (see [`agents:`](./02-moat-yaml.md#agents)). `moat join run_a1b2c3d4e5f6 codex` works against a container started by `moat claude` only when that project's `moat.yaml` also lists `codex` in `agents:`. ### Arguments | Argument | Description | |----------|-------------| -| `run` | Run ID or name of the target (must be in the running state) | -| `agent` | Agent to launch (`claude`) | +| `run` | Run ID or name of the target (must be in the running state). May be omitted — see below. | +| `agent` | Agent to launch (`claude`, `codex`, …) | + +#### Inferring the run + +When `run` is omitted, `moat join ` picks it for you: + +1. Only runs in the `running` state are candidates. +2. Candidates are narrowed to runs that can host `` — the run's provisioned-agent set includes it. +3. If any candidate's workspace matches the current directory, only those are offered. Otherwise every running, capable run is offered, and moat says so — attaching outside the current workspace uses that run's grants and network policy. +4. One remaining candidate in the current workspace attaches directly. Otherwise moat prints a numbered table (newest run first) and prompts for a choice; the table and prompt are written to stderr, so `moat join claude | tee log` still shows the picker even though stdin is a TTY. Piped or non-interactive input with more than one candidate is an error listing the run IDs, rather than a prompt. + +If the single positional argument names both a known agent and a run, it resolves as the agent — moat warns and shows the two-argument form to target the run instead. ### Flags @@ -1293,6 +1304,12 @@ moat join run_a1b2c3d4e5f6 claude -p "summarize the diff" # Identify the run by name moat join my-feature claude + +# Infer the run from the current workspace +moat join claude + +# Join codex into a container provisioned with agents: [claude, codex] +moat join run_a1b2c3d4e5f6 codex ``` --- diff --git a/docs/content/reference/02-moat-yaml.md b/docs/content/reference/02-moat-yaml.md index feb6e070..595b4693 100644 --- a/docs/content/reference/02-moat-yaml.md +++ b/docs/content/reference/02-moat-yaml.md @@ -16,7 +16,7 @@ The `moat.yaml` file configures how Moat runs your agent. Place it in your works ```yaml # Metadata name: my-agent -agent: my-agent +agent: claude version: 1.0.0 # Runtime @@ -201,14 +201,42 @@ When using `moat wt` or `--worktree`, the `name` field is used to generate the r ### agent -Agent identifier. Used internally for tracking. +Names the agent this project runs. Moat uses it to apply agent-specific defaults +(container memory, implied dependencies, language-server support). ```yaml -agent: my-agent +agent: claude ``` - Type: `string` -- Default: Same as `name` +- Allowed values: `claude`, `claude-code`, `codex`, `copilot`, `gemini`, `pi` +- Default: the provider name of the command you ran (`moat claude` → `claude`). + For `moat run`, `agent:` is used as set; if it is unset and `agents:` is + present, it falls back to the first entry in `agents:`; otherwise it stays + unset. +- The command wins: `moat claude` runs claude even if this field says otherwise. + +This is not a free-form label. An unrecognized value is ignored with a warning. + +### agents + +Provisions several agents into one container so `moat join` can launch any of +them. Each entry contributes its CLI dependency, its credential grant, and its +network rules. + +```yaml +agents: [claude, codex] +``` + +- Type: `array[string]` +- Allowed values: same as `agent`, except `pi` +- Order matters: for `moat run` with no `agent:` set, the first entry is the + foreground agent. Every other entry is reachable only via `moat join`. +- An unrecognized entry is an error, not a warning — a dropped entry would leave + the container without that agent's credential and firewall rules. + +See [Multi-agent sessions](../guides/14-multi-agent.md) for how `moat join` +uses this list. ### version From 04421f303c032bd7a83a8830d80a9128c3ab1b25 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:40:20 +0000 Subject: [PATCH 22/33] test(e2e): add a dual-agent join test for agents: [claude, codex] Provisions a real container via moat.yaml's agents: [claude, codex], asserts both land in the persisted joinable_agents set, and drives real `moat join claude` / `moat join codex` through the real binary to prove both get past the capability gate and reach a live process in the same container. TestJoinHeadless only covers the single-agent case. --- internal/e2e/join_test.go | 232 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 232 insertions(+) diff --git a/internal/e2e/join_test.go b/internal/e2e/join_test.go index 73bf002b..cb1cac01 100644 --- a/internal/e2e/join_test.go +++ b/internal/e2e/join_test.go @@ -13,8 +13,10 @@ import ( "testing" "time" + intcli "github.com/majorcontext/moat/internal/cli" "github.com/majorcontext/moat/internal/config" "github.com/majorcontext/moat/internal/container" + "github.com/majorcontext/moat/internal/credential" "github.com/majorcontext/moat/internal/daemon" "github.com/majorcontext/moat/internal/run" "github.com/majorcontext/moat/internal/storage" @@ -189,6 +191,236 @@ func TestJoinHeadless(t *testing.T) { }) } +// TestDualAgentJoin_E2E provisions a real container via moat.yaml's +// `agents: [claude, codex]` and proves it is joinable as BOTH agents — the +// combination TestJoinHeadless doesn't cover (it is single-agent). +// +// What it asserts: +// 1. The persisted joinable_agents set (what moat actually provisioned — +// internal/run/joinable.go's computeJoinableAgents) contains both +// "claude" and "codex", proving `agents:` expansion (dependencies + +// grants) actually ran and actually staged both agents. +// 2. `moat join claude` and `moat join codex` both get PAST +// validateJoinAgent's capability gate: neither is refused with the +// "cannot host" error that fires for an agent the run never provisioned +// (cmd/moat/cli/join_cmd.go). That refusal path is already covered by +// pure unit tests (join_cmd_test.go); what only a real container proves +// is the positive case — that both real, distinct in-container agent +// binaries are reachable through the same gate on the same run. +// 3. Both joins run inside the SAME container the primary run started (no +// new container created), matching TestJoinHeadless's invariant (a), and +// joined output does not leak into the primary's logs.jsonl (split +// console isolation). +// +// What this test CANNOT assert: that claude/codex complete a real +// conversation turn. This sandbox has no real Anthropic or OpenAI +// credentials, so both grants are stored with fake tokens; the real CLI +// processes are expected to fail authentication once they reach the network. +// That failure is fine and out of scope — what matters here is purely the +// join *mechanism*, not agent behavior once inside. +func TestDualAgentJoin_E2E(t *testing.T) { + // Isolated test keyring so the fake credentials below never touch the + // user's real credential store (same pattern as TestCodexContainerConfig_E2E). + t.Setenv("MOAT_KEYRING_SERVICE", "moat-test") + t.Cleanup(func() { cleanupKeychainKey(t) }) + + ctx, cancel := context.WithTimeout(context.Background(), testTimeout) + defer cancel() + + // Fake credentials for both grants `agents: [claude, codex]` expands to. + // Neither needs to look like a real token: the claude provider writes a + // fixed placeholder into the container regardless of the stored value + // (internal/providers/claude/config.go WriteCredentialsFile — the real + // token is never written to the container), and codex's staged auth.json + // is likewise a placeholder the proxy replaces at request time. Storing + // *something* under each provider is what satisfies validateGrants and + // gets each agent into JoinableAgents. + encKey, err := credential.DefaultEncryptionKey() + if err != nil { + t.Fatalf("DefaultEncryptionKey: %v", err) + } + credStore, err := credential.NewFileStore(credential.DefaultStoreDir(), encKey) + if err != nil { + t.Fatalf("NewFileStore: %v", err) + } + if err := credStore.Save(credential.Credential{ + Provider: credential.ProviderClaude, + Token: "e2e-not-a-real-oauth-token", + CreatedAt: time.Now(), + }); err != nil { + t.Fatalf("Save claude credential: %v", err) + } + defer credStore.Delete(credential.ProviderClaude) + if err := credStore.Save(credential.Credential{ + Provider: credential.ProviderOpenAI, + Token: "sk-e2e-not-a-real-key", + CreatedAt: time.Now(), + }); err != nil { + t.Fatalf("Save openai credential: %v", err) + } + defer credStore.Delete(credential.ProviderOpenAI) + + // moat.yaml's `agents:` list is the thing under test. ExpandAgents is the + // exact function `moat run` calls (cmd/moat/cli/run.go) to turn it into + // dependencies, grants, and network rules — using it here, rather than + // hand-building a Config with Dependencies/Grants set directly, is what + // makes this test exercise the `agents:` feature and not just the join + // gate in isolation. + workspace := t.TempDir() + yaml := "agents: [claude, codex]\nversion: 1.0.0\n" + if err := os.WriteFile(filepath.Join(workspace, "moat.yaml"), []byte(yaml), 0o644); err != nil { + t.Fatalf("WriteFile moat.yaml: %v", err) + } + cfg, err := config.Load(workspace) + if err != nil { + t.Fatalf("config.Load: %v", err) + } + if err := intcli.ExpandAgents(cfg); err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + + mgr, err := run.NewManagerWithOptions(run.ManagerOptions{NoSandbox: &[]bool{true}[0]}) + if err != nil { + t.Fatalf("NewManager: %v", err) + } + defer mgr.Close() + + // "sleep 600" keeps the container alive long enough for both joins, and — + // crucially for the split-console assertion below — never writes to + // stdout itself, so anything found in logs.jsonl can only have leaked + // from a join. + r, err := mgr.Create(ctx, run.Options{ + Name: "e2e-dual-agent-join", + Workspace: workspace, + Grants: cfg.Grants, + Config: cfg, + Cmd: []string{"sleep", "600"}, + }) + if err != nil { + t.Fatalf("Create: %v", err) + } + defer mgr.Destroy(context.Background(), r.ID) + defer mgr.Stop(context.Background(), r.ID) + + if err := mgr.Start(ctx, r.ID); err != nil { + t.Fatalf("Start: %v", err) + } + + // Wait briefly for the container to be fully running before joining. + time.Sleep(500 * time.Millisecond) + + primaryContainerID := r.ContainerID + if primaryContainerID == "" { + t.Fatal("run has no container ID after Start") + } + + // --- Assertion (1): both agents landed in the persisted capability set --- + refreshed, err := mgr.Get(r.ID) + if err != nil { + t.Fatalf("Get: %v", err) + } + t.Logf("JoinableAgents = %v", refreshed.JoinableAgents) + gotAgents := make(map[string]bool, len(refreshed.JoinableAgents)) + for _, a := range refreshed.JoinableAgents { + gotAgents[a] = true + } + for _, want := range []string{"claude", "codex"} { + if !gotAgents[want] { + t.Fatalf("JoinableAgents = %v, missing %q — `agents: [claude, codex]` did not provision it as claimed", + refreshed.JoinableAgents, want) + } + } + + moatBin := joinTestMoatExecutable(t) + + // --- Assertion (2): join as claude, then as codex --- + // Real moat binary, so the full CLI path is exercised (agent/provider + // resolution, validateJoinAgent, ExecInteractive) — same approach as + // TestJoinHeadless. Headless (-p) avoids needing a pty. + claudeOut := runJoinHeadlessCLI(t, moatBin, r.ID, "claude", "say OK and nothing else") + if strings.Contains(claudeOut, "cannot host") { + t.Errorf("join claude was refused by the capability gate despite being in JoinableAgents:\n%s", claudeOut) + } + + codexOut := runJoinHeadlessCLI(t, moatBin, r.ID, "codex", "say OK and nothing else") + if strings.Contains(codexOut, "cannot host") { + t.Errorf("join codex was refused by the capability gate despite being in JoinableAgents:\n%s", codexOut) + } + + // --- Assertion (3a): no new container across either join --- + afterJoins, err := mgr.Get(r.ID) + if err != nil { + t.Fatalf("Get after joins: %v", err) + } + if afterJoins.ContainerID != primaryContainerID { + t.Errorf("container ID changed after joins: before=%q after=%q (join must reuse the existing container)", + primaryContainerID, afterJoins.ContainerID) + } + + // --- Assertion (3b): split-console isolation --- + // The primary command is "sleep 600"; it never writes to stdout, so + // logs.jsonl must stay empty regardless of what either joined agent + // printed. + time.Sleep(100 * time.Millisecond) + store, err := storage.NewRunStore(storage.DefaultBaseDir(), r.ID) + if err != nil { + t.Fatalf("NewRunStore: %v", err) + } + primaryLogs, logsErr := store.ReadLogs(0, 500) + if logsErr != nil { + t.Errorf("ReadLogs: %v", logsErr) + } else if len(primaryLogs) != 0 { + t.Errorf("primary logs.jsonl should be empty (\"sleep 600\" writes nothing), got %d lines — joined agent output leaked in: %+v", + len(primaryLogs), primaryLogs) + } + + // Informational only: real evidence the joins reached a live process + // inside the container, not just the gate check. Not asserted as fatal + // because a joined agent that fails auth may write only to stderr, which + // runJoinHeadless (join_cmd.go) does not tee into logs..jsonl — that's + // a pre-existing, out-of-scope asymmetry, not something this test owns. + if strings.TrimSpace(claudeOut) == "" { + t.Log("join claude produced no captured output (fake credentials — may have failed before printing anything)") + } + if strings.TrimSpace(codexOut) == "" { + t.Log("join codex produced no captured output (fake credentials — may have failed before printing anything)") + } +} + +// runJoinHeadlessCLI runs `moat join -p ` via the +// real moat binary and returns its combined stdout+stderr. It deliberately +// never fails the test on a non-zero exit: the joined agents in this test +// carry fake credentials, so a failed auth attempt is expected and is not +// what this test is checking. Bounded to 90s so a CLI that unexpectedly +// blocks on interactive auth (rather than failing fast) doesn't hang the +// suite — moat join runs headless (no TTY), which should prevent that, but +// the bound makes the failure mode "test times out with a clear log" rather +// than "test hangs forever." +func runJoinHeadlessCLI(t *testing.T, moatBin, runID, agent, prompt string) string { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) + defer cancel() + + cmd := exec.CommandContext(ctx, moatBin, "join", runID, agent, "-p", prompt) + // join_cmd.go calls run.NewManager() unconditionally (it has no + // --no-sandbox flag of its own — joining execs into an already-running + // container, so it shouldn't need one), but NewManager() still probes for + // gVisor up front as part of building the runtime pool. This sandbox has + // no gVisor (runsc) installed, so without this the join subprocess fails + // before ever reaching validateJoinAgent — the same MOAT_NO_SANDBOX=1 + // escape hatch Task 17's manual verification needed for `moat run`. + cmd.Env = append(os.Environ(), "MOAT_NO_SANDBOX=1") + var out bytes.Buffer + cmd.Stdout = &out + cmd.Stderr = &out + runErr := cmd.Run() + t.Logf("join %s output: %s", agent, out.String()) + if runErr != nil { + t.Logf("join %s exited with error (may be expected — fake credentials): %v", agent, runErr) + } + return out.String() +} + // joinTestMoatExecutable returns the path to the moat binary set by TestMain // via MOAT_EXECUTABLE, skipping the test if it is not set. func joinTestMoatExecutable(t *testing.T) string { From aa29c8b53b4a017edb1e1b1a3791562e2b708af8 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:42:28 +0000 Subject: [PATCH 23/33] docs(cli): fix stale v1 join help text --- cmd/moat/cli/join_cmd.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index 3dbe4b3f..555a37a7 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -32,8 +32,13 @@ var joinCmd = &cobra.Command{ Long: `Launch a second agent inside an already-running container, reusing its workspace, grants, and credentials — without creating a new container. -The agent must match the one the run was started with (v1 supports same-agent -joins, e.g. joining claude into a run started by 'moat claude'). +The agent must be one the run was provisioned with — not necessarily the one +it was started with. A run created from moat.yaml's 'agents:' list can be +joined as any agent in that list. + +The run argument is optional: with just an agent, moat infers the run from +the running runs in the current workspace, and prompts when more than one +qualifies. Examples: moat join run_a1b2c3d4e5f6 claude From c32628e5c29b1d651431e922ab02925bcee63dc1 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 01:51:20 +0000 Subject: [PATCH 24/33] fix(e2e): make dual-agent join gate check fatal on empty output The negative "cannot host" check alone passes vacuously if the moat join subprocess never launches or the joined process crashes before printing anything. Add a fatal non-empty-output check alongside it so the test requires real evidence a live process was reached, not just the absence of the literal rejection string. --- internal/e2e/join_test.go | 69 ++++++++++++++++++++++++++++----------- 1 file changed, 50 insertions(+), 19 deletions(-) diff --git a/internal/e2e/join_test.go b/internal/e2e/join_test.go index cb1cac01..c81e080a 100644 --- a/internal/e2e/join_test.go +++ b/internal/e2e/join_test.go @@ -337,15 +337,35 @@ func TestDualAgentJoin_E2E(t *testing.T) { // Real moat binary, so the full CLI path is exercised (agent/provider // resolution, validateJoinAgent, ExecInteractive) — same approach as // TestJoinHeadless. Headless (-p) avoids needing a pty. + // + // Two checks per join, and both are load-bearing: + // - Negative: the output must not contain "cannot host" — the literal + // string validateJoinAgent (join_cmd.go) emits when the gate refuses + // an agent that isn't in JoinableAgents. + // - Positive: the output must be non-empty. Without this, the negative + // check alone passes vacuously if the subprocess never launches, or + // the joined process crashes before printing anything — out stays + // "", and "" plainly does not contain "cannot host" either. Fatal, + // not merely logged: in 3/3 real runs this captured genuine evidence + // the join reached a live process (claude's real 401 from Anthropic; + // codex's own trust-directory check), so it carries no flake risk + // here — see runJoinHeadlessCLI's doc comment for what "output" + // means and why it's safe to require. claudeOut := runJoinHeadlessCLI(t, moatBin, r.ID, "claude", "say OK and nothing else") if strings.Contains(claudeOut, "cannot host") { t.Errorf("join claude was refused by the capability gate despite being in JoinableAgents:\n%s", claudeOut) } + if strings.TrimSpace(claudeOut) == "" { + t.Fatalf("join claude produced no output at all — cannot tell whether the gate passed or the subprocess never ran") + } codexOut := runJoinHeadlessCLI(t, moatBin, r.ID, "codex", "say OK and nothing else") if strings.Contains(codexOut, "cannot host") { t.Errorf("join codex was refused by the capability gate despite being in JoinableAgents:\n%s", codexOut) } + if strings.TrimSpace(codexOut) == "" { + t.Fatalf("join codex produced no output at all — cannot tell whether the gate passed or the subprocess never ran") + } // --- Assertion (3a): no new container across either join --- afterJoins, err := mgr.Get(r.ID) @@ -374,28 +394,39 @@ func TestDualAgentJoin_E2E(t *testing.T) { len(primaryLogs), primaryLogs) } - // Informational only: real evidence the joins reached a live process - // inside the container, not just the gate check. Not asserted as fatal - // because a joined agent that fails auth may write only to stderr, which - // runJoinHeadless (join_cmd.go) does not tee into logs..jsonl — that's - // a pre-existing, out-of-scope asymmetry, not something this test owns. - if strings.TrimSpace(claudeOut) == "" { - t.Log("join claude produced no captured output (fake credentials — may have failed before printing anything)") - } - if strings.TrimSpace(codexOut) == "" { - t.Log("join codex produced no captured output (fake credentials — may have failed before printing anything)") - } + // Deliberately NOT asserted: the on-disk logs..jsonl file. That would + // be a stronger check in principle (proof the output was actually teed + // to storage, not just visible to this test's own subprocess capture), + // but runJoinHeadless (join_cmd.go) only tees the joined process's stdout + // into logs..jsonl, not its stderr — and claude/codex may write an + // auth failure to either stream. Asserting on logs..jsonl here would + // couple this test to that pre-existing, out-of-scope asymmetry. The + // fatal non-empty checks above already use the reliable signal: the + // `moat join` subprocess's own combined stdout+stderr, captured directly + // by runJoinHeadlessCLI regardless of which stream the joined agent used. } // runJoinHeadlessCLI runs `moat join -p ` via the -// real moat binary and returns its combined stdout+stderr. It deliberately -// never fails the test on a non-zero exit: the joined agents in this test -// carry fake credentials, so a failed auth attempt is expected and is not -// what this test is checking. Bounded to 90s so a CLI that unexpectedly -// blocks on interactive auth (rather than failing fast) doesn't hang the -// suite — moat join runs headless (no TTY), which should prevent that, but -// the bound makes the failure mode "test times out with a clear log" rather -// than "test hangs forever." +// real moat binary and returns its combined stdout+stderr — the `moat join` +// process's own streams, which is why it reliably captures the joined +// agent's output regardless of whether that agent wrote to its own stdout or +// stderr (see the comment above the call sites for why that distinction +// matters). The caller, not this helper, decides what the returned string +// must contain; this helper only runs the process and reports what happened. +// +// It deliberately does NOT fail the test on a non-zero exit: the joined +// agents in this test carry fake credentials, so a failed auth attempt is +// expected and is not what this test is checking — a real exit code from a +// real process that got past the gate is success, not failure, for this +// test's purposes. (What the caller DOES require is that some output was +// produced at all — see the fatal checks at the call sites — which is a +// different, and necessary, signal from the exit code.) +// +// Bounded to 90s so a CLI that unexpectedly blocks on interactive auth +// (rather than failing fast) doesn't hang the suite — moat join runs +// headless (no TTY), which should prevent that, but the bound makes the +// failure mode "test times out with a clear log" rather than "test hangs +// forever." func runJoinHeadlessCLI(t *testing.T, moatBin, runID, agent, prompt string) string { t.Helper() ctx, cancel := context.WithTimeout(context.Background(), 90*time.Second) From a33e6f64a4b1de6511679aa67d07dffcf59952c2 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 02:13:16 +0000 Subject: [PATCH 25/33] fix(cli): stop agents:-derived grants outranking an auto-detected credential `agents: [claude, ...]` injected "claude" straight into cfg.Grants, which buildGrants treats as an explicit user grant and uses to suppress an auto-detected credential. A user whose Anthropic credential is stored as an API key under `anthropic` (a supported, documented fallback) would have it discarded in favor of the OAuth-only "claude" grant the expansion added, forcing a login they never asked for even though their existing credential already worked. ExpandAgents now returns derived grants instead of mutating cfg.Grants, and buildGrants takes them as a fourth, lowest-precedence input: they never suppress an auto-detected grant, and are skipped when an equivalent credential (the claude/anthropic pair) is already present. moat run merges the returned grants into its own default-grants logic, since it has no buildGrants precedence chain of its own. --- cmd/moat/cli/run.go | 15 +++++++-- internal/cli/agents.go | 30 +++++++++++------- internal/cli/agents_test.go | 28 +++++++++++------ internal/cli/provider.go | 52 +++++++++++++++++++++++++++----- internal/cli/provider_test.go | 57 +++++++++++++++++++++++++++++++---- internal/e2e/join_test.go | 9 ++++-- 6 files changed, 152 insertions(+), 39 deletions(-) diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index 66d96890..59fb2631 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -115,7 +115,12 @@ func runAgent(cmd *cobra.Command, args []string) error { // "Apply config defaults" block below reads cfg.Grants into runFlags.Grants // — an expansion that lands after would never reach the grants flag (see // intcli.ExpandAgents doc comment for why ordering matters here). - if err = intcli.ExpandAgents(cfg); err != nil { + // + // derivedGrants is returned rather than merged into cfg.Grants by + // ExpandAgents itself — see its doc comment — so it's combined into the + // default grants list explicitly below. + derivedGrants, err := intcli.ExpandAgents(cfg) + if err != nil { return err } @@ -127,8 +132,12 @@ func runAgent(cmd *cobra.Command, args []string) error { // Apply config defaults if cfg != nil { - if len(runFlags.Grants) == 0 && len(cfg.Grants) > 0 { - runFlags.Grants = cfg.Grants + if len(runFlags.Grants) == 0 { + grants := append([]string{}, cfg.Grants...) + grants = append(grants, derivedGrants...) + if len(grants) > 0 { + runFlags.Grants = grants + } } if len(containerCmd) == 0 && len(cfg.Command) > 0 { containerCmd = cfg.Command diff --git a/internal/cli/agents.go b/internal/cli/agents.go index 147439ab..7553ab0d 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -126,9 +126,16 @@ func agentsListContains(agents []string, agent string) bool { return false } -// ExpandAgents expands moat.yaml's `agents:` list into the dependencies, -// grants, and network rules each named agent needs, deduping against what the -// config already declares. +// ExpandAgents expands moat.yaml's `agents:` list into the dependencies and +// network rules each named agent needs, deduping against what the config +// already declares. It mutates cfg.Dependencies and cfg.Network.Rules +// directly, but returns credential grants rather than appending them to +// cfg.Grants — callers must merge the return value into their own grants +// list. This keeps agent-derived grants out of cfg.Grants, which callers +// like buildGrants treat as "explicit" (user-written) and use to suppress an +// auto-detected credential; a derived grant is a fallback, not a user +// declaration, and must never win that suppression. See buildGrants in +// internal/cli/provider.go. // // It must run BEFORE grant resolution and the network-rule loop in // RunProvider — an expansion that lands after either contributes nothing. The @@ -140,24 +147,25 @@ func agentsListContains(agents []string, agent string) bool { // silently dropped entry leaves the container short a credential AND its // firewall rules — surfacing much later as an opaque join refusal or a blocked // request under a strict network policy. -func ExpandAgents(cfg *config.Config) error { +func ExpandAgents(cfg *config.Config) ([]string, error) { if cfg == nil || len(cfg.Agents) == 0 { - return nil + return nil, nil } + var derivedGrants []string for _, entry := range cfg.Agents { if entry == "" { - return fmt.Errorf("moat.yaml `agents:` contains an empty entry; remove it or name an agent (valid: %s)", + return nil, fmt.Errorf("moat.yaml `agents:` contains an empty entry; remove it or name an agent (valid: %s)", strings.Join(KnownAgentNames(), ", ")) } canonical := CanonicalAgent(entry) if canonical == "" { - return fmt.Errorf("moat.yaml `agents: [%s]` is not a known agent (valid: %s)", + return nil, fmt.Errorf("moat.yaml `agents: [%s]` is not a known agent (valid: %s)", entry, strings.Join(KnownAgentNames(), ", ")) } agent := provider.GetAgent(canonical) rt, ok := agent.(provider.AgentRuntime) if !ok { - return fmt.Errorf("moat.yaml `agents: [%s]` cannot be provisioned declaratively; "+ + return nil, fmt.Errorf("moat.yaml `agents: [%s]` cannot be provisioned declaratively; "+ "run it with `moat %s` instead", entry, canonical) } @@ -171,8 +179,8 @@ func ExpandAgents(cfg *config.Config) error { } } - if grant := rt.CredentialGrant(); grant != "" && !slices.Contains(cfg.Grants, grant) { - cfg.Grants = append(cfg.Grants, grant) + if grant := rt.CredentialGrant(); grant != "" && !slices.Contains(cfg.Grants, grant) && !slices.Contains(derivedGrants, grant) { + derivedGrants = append(derivedGrants, grant) } for _, host := range rt.NetworkHosts() { @@ -183,7 +191,7 @@ func ExpandAgents(cfg *config.Config) error { netrules.NetworkRuleEntry{HostRules: netrules.HostRules{Host: host}}) } } - return nil + return derivedGrants, nil } // hasNetworkHost reports whether rules already contains an entry for host. diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index 8c77da4e..cde4982c 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -176,7 +176,8 @@ func TestResolveAgentFieldWithAgentsList(t *testing.T) { func TestExpandAgents(t *testing.T) { cfg := &config.Config{Agents: []string{"claude", "codex"}} - if err := cli.ExpandAgents(cfg); err != nil { + grants, err := cli.ExpandAgents(cfg) + if err != nil { t.Fatalf("ExpandAgents: %v", err) } @@ -186,11 +187,17 @@ func TestExpandAgents(t *testing.T) { } } // codex's grant is openai, not codex. - if !slices.Contains(cfg.Grants, "openai") { - t.Errorf("expected grant openai; got %v", cfg.Grants) + if !slices.Contains(grants, "openai") { + t.Errorf("expected grant openai; got %v", grants) } - if slices.Contains(cfg.Grants, "codex") { - t.Errorf("codex must expand to the openai grant, not codex; got %v", cfg.Grants) + if slices.Contains(grants, "codex") { + t.Errorf("codex must expand to the openai grant, not codex; got %v", grants) + } + // ExpandAgents must not mutate cfg.Grants directly — see its doc comment: + // derived grants are returned so callers can give them lower precedence + // than an auto-detected credential. + if len(cfg.Grants) != 0 { + t.Errorf("cfg.Grants should be untouched by ExpandAgents; got %v", cfg.Grants) } hosts := make([]string, 0, len(cfg.Network.Rules)) @@ -211,14 +218,17 @@ func TestExpandAgentsDoesNotDuplicate(t *testing.T) { Dependencies: []string{"claude-code"}, Grants: []string{"claude"}, } - if err := cli.ExpandAgents(cfg); err != nil { + grants, err := cli.ExpandAgents(cfg) + if err != nil { t.Fatalf("ExpandAgents: %v", err) } if got := countOccurrences(cfg.Dependencies, "claude-code"); got != 1 { t.Errorf("claude-code appears %d times, want 1: %v", got, cfg.Dependencies) } - if got := countOccurrences(cfg.Grants, "claude"); got != 1 { - t.Errorf("claude grant appears %d times, want 1: %v", got, cfg.Grants) + // claude is already in cfg.Grants, so it must not also come back as a + // derived grant — the caller would otherwise re-add it. + if len(grants) != 0 { + t.Errorf("expected no derived grants (claude already in cfg.Grants); got %v", grants) } } @@ -236,7 +246,7 @@ func TestExpandAgentsRejectsBadEntries(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { cfg := &config.Config{Agents: tt.agents} - err := cli.ExpandAgents(cfg) + _, err := cli.ExpandAgents(cfg) if err == nil { t.Fatal("expected a hard error — a dropped entry silently costs a credential and firewall rules") } diff --git a/internal/cli/provider.go b/internal/cli/provider.go index d6136ef3..526339c9 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -137,13 +137,16 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error // the proxy registration (see ExpandAgents doc comment). cfg may still be // nil here (no moat.yaml); ExpandAgents is a no-op in that case since a // nil config can't carry an agents: list either. - if err = ExpandAgents(cfg); err != nil { + var derivedGrants []string + derivedGrants, err = ExpandAgents(cfg) + if err != nil { return err } // Build grants list with deduplication: credential grant first, - // then config grants, then flag grants. Auto-detected grants are - // suppressed when they conflict with an explicit grant. + // then config grants, then flag grants, then agents:-derived grants. + // Auto-detected grants are suppressed when they conflict with an + // explicit grant. var autoDetected string if rc.GetCredentialGrant != nil { autoDetected = rc.GetCredentialGrant() @@ -152,7 +155,7 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error if cfg != nil { configGrants = cfg.Grants } - grants := buildGrants(autoDetected, configGrants, rc.Flags.Grants) + grants := buildGrants(autoDetected, configGrants, rc.Flags.Grants, derivedGrants) rc.Flags.Grants = grants interactive := rc.PromptFlag == "" @@ -293,11 +296,30 @@ func containsGrant(grants []string, name string) bool { return false } +// grantsEquivalent reports whether a and b name the same underlying +// credential under different grant keys — today, only the claude +// (OAuth-token) / anthropic (API-key) pair, which both authenticate Claude +// Code against the same host. +func grantsEquivalent(a, b string) bool { + return (a == "claude" && b == "anthropic") || (a == "anthropic" && b == "claude") +} + // buildGrants assembles the final grants list from an auto-detected -// credential grant, config grants, and flag grants. Auto-detected grants -// are suppressed when they conflict with an explicit grant (e.g., -// "claude" conflicts with "anthropic" since both target the same host). -func buildGrants(autoDetected string, configGrants, flagGrants []string) []string { +// credential grant, config grants, flag grants, and agents:-derived grants, +// in descending precedence. +// +// Auto-detected grants are suppressed when they conflict with an EXPLICIT +// (config or flag) grant — e.g. "claude" conflicts with "anthropic" since +// both target the same host. derivedGrants (from moat.yaml's `agents:` +// expansion, see ExpandAgents) never participates in that suppression: it is +// a machine-filled fallback, not a user declaration, so it must not outrank +// whatever credential the user actually has. A derived grant is instead +// dropped when an equivalent credential is already present in the result — +// otherwise a user who stores their Anthropic credential as an API key +// (autoDetected == "anthropic") would have it discarded in favor of a +// "claude" grant injected by `agents: [claude, ...]`, forcing an OAuth login +// they never asked for even though their existing credential already works. +func buildGrants(autoDetected string, configGrants, flagGrants, derivedGrants []string) []string { grantSet := make(map[string]bool) var grants []string addGrant := func(g string) { @@ -306,6 +328,14 @@ func buildGrants(autoDetected string, configGrants, flagGrants []string) []strin grants = append(grants, g) } } + equivalentPresent := func(g string) bool { + for _, existing := range grants { + if grantsEquivalent(existing, g) { + return true + } + } + return false + } explicitGrants := make([]string, 0, len(configGrants)+len(flagGrants)) explicitGrants = append(explicitGrants, configGrants...) @@ -321,5 +351,11 @@ func buildGrants(autoDetected string, configGrants, flagGrants []string) []strin for _, g := range explicitGrants { addGrant(g) } + for _, g := range derivedGrants { + if equivalentPresent(g) { + continue + } + addGrant(g) + } return grants } diff --git a/internal/cli/provider_test.go b/internal/cli/provider_test.go index 9ffdb18d..d42e4f0d 100644 --- a/internal/cli/provider_test.go +++ b/internal/cli/provider_test.go @@ -122,11 +122,12 @@ func TestResolveProviderAgentFieldWarnsAcrossConfigureAgentHooks(t *testing.T) { func TestBuildGrants(t *testing.T) { tests := []struct { - name string - autoDetected string - configGrants []string - flagGrants []string - want []string + name string + autoDetected string + configGrants []string + flagGrants []string + derivedGrants []string + want []string }{ { name: "auto-detected claude with no explicit grants", @@ -176,11 +177,55 @@ func TestBuildGrants(t *testing.T) { flagGrants: []string{"claude", "anthropic"}, want: []string{"claude", "anthropic"}, }, + // C1 regression: a derived grant (from moat.yaml `agents:` expansion, + // e.g. `agents: [claude, ...]`) must never outrank an auto-detected + // credential, even when they're the claude/anthropic equivalence + // pair. Previously ExpandAgents appended "claude" straight into + // cfg.Grants, which buildGrants treated as an explicit grant and used + // to suppress the auto-detected "anthropic" API-key credential — + // discarding a credential that actually works in favor of one the + // user never configured. + { + name: "derived claude does not suppress auto-detected anthropic", + autoDetected: "anthropic", + derivedGrants: []string{"claude"}, + want: []string{"anthropic"}, + }, + // Companion: the equivalence check is symmetric. + { + name: "derived anthropic does not suppress auto-detected claude", + autoDetected: "claude", + derivedGrants: []string{"anthropic"}, + want: []string{"claude"}, + }, + // Companion: with no conflict, a derived grant still populates — + // agents: expansion is a real fallback source, not a no-op. + { + name: "derived grant populates when nothing else claims the credential", + derivedGrants: []string{"openai"}, + want: []string{"openai"}, + }, + // Companion: a derived grant that duplicates an explicit one is + // deduped, not appended twice. + { + name: "derived grant already explicit is not duplicated", + configGrants: []string{"openai"}, + derivedGrants: []string{"openai"}, + want: []string{"openai"}, + }, + // Companion: derived grants are lower precedence than explicit ones + // too, but still contribute when they don't conflict. + { + name: "derived grant added after explicit, non-conflicting grants", + configGrants: []string{"github"}, + derivedGrants: []string{"claude"}, + want: []string{"github", "claude"}, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := buildGrants(tt.autoDetected, tt.configGrants, tt.flagGrants) + got := buildGrants(tt.autoDetected, tt.configGrants, tt.flagGrants, tt.derivedGrants) if !reflect.DeepEqual(got, tt.want) { t.Errorf("buildGrants(%q, %v, %v) = %v, want %v", tt.autoDetected, tt.configGrants, tt.flagGrants, got, tt.want) diff --git a/internal/e2e/join_test.go b/internal/e2e/join_test.go index c81e080a..98e3f797 100644 --- a/internal/e2e/join_test.go +++ b/internal/e2e/join_test.go @@ -275,9 +275,14 @@ func TestDualAgentJoin_E2E(t *testing.T) { if err != nil { t.Fatalf("config.Load: %v", err) } - if err := intcli.ExpandAgents(cfg); err != nil { + // ExpandAgents returns derived grants rather than merging them into + // cfg.Grants (see its doc comment) — merge them the same way + // cmd/moat/cli/run.go does before passing grants to the run manager. + derivedGrants, err := intcli.ExpandAgents(cfg) + if err != nil { t.Fatalf("ExpandAgents: %v", err) } + grants := append(append([]string{}, cfg.Grants...), derivedGrants...) mgr, err := run.NewManagerWithOptions(run.ManagerOptions{NoSandbox: &[]bool{true}[0]}) if err != nil { @@ -292,7 +297,7 @@ func TestDualAgentJoin_E2E(t *testing.T) { r, err := mgr.Create(ctx, run.Options{ Name: "e2e-dual-agent-join", Workspace: workspace, - Grants: cfg.Grants, + Grants: grants, Config: cfg, Cmd: []string{"sleep", "600"}, }) From b94d8d4d45556577cb20ac7cd7050e1360ac0ce7 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 02:13:23 +0000 Subject: [PATCH 26/33] fix(wt): expand agents: and resolve agent: in moat wt MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit moat wt loads moat.yaml and is documented as following "the same pattern as moat run", but it never called ExpandAgents or ResolveAgentField — the two hooks every other run-creating entry point (moat run, the provider verbs via RunProvider) uses. Two consequences: `agents:` was a silent no-op under `moat wt` (no extra deps, grants, or network rules, so `moat join` refused with advice the user had already followed), and an invalid `agent:` value kept silently disabling container memory defaults, implied dependencies, and language-server support — the original bug this branch set out to fix, just unfixed on this one path. Both calls are added in the same relative positions run.go uses: ExpandAgents after the worktree config reload and before the grants-defaulting block, ResolveAgentField right before ExecuteRun. runWorktree has no existing unit tests (neither does run.go's runAgent) — it isn't unit-testable in its current shape without a git repo, run manager, and cobra command harness. Verified the ordering by reading against run.go's placement instead. --- cmd/moat/cli/wt.go | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/cmd/moat/cli/wt.go b/cmd/moat/cli/wt.go index 1038c95a..55b1dbf0 100644 --- a/cmd/moat/cli/wt.go +++ b/cmd/moat/cli/wt.go @@ -131,6 +131,17 @@ func runWorktree(cmd *cobra.Command, args []string) error { cfg = wtCfg } + // Expand agents: into dependencies/grants/network hosts before the "Apply + // config defaults" block below reads cfg.Grants into wtFlags.Grants — same + // pattern as moat run (cmd/moat/cli/run.go). derivedGrants is returned + // rather than merged into cfg.Grants by ExpandAgents itself (see its doc + // comment), so it's combined into the default grants list explicitly + // below. + derivedGrants, err := intcli.ExpandAgents(cfg) + if err != nil { + return err + } + // Check for active run in this worktree manager, err := run.NewManager() if err != nil { @@ -151,8 +162,12 @@ func runWorktree(cmd *cobra.Command, args []string) error { // Apply config defaults (same pattern as moat run) if cfg != nil { - if len(wtFlags.Grants) == 0 && len(cfg.Grants) > 0 { - wtFlags.Grants = cfg.Grants + if len(wtFlags.Grants) == 0 { + grants := append([]string{}, cfg.Grants...) + grants = append(grants, derivedGrants...) + if len(grants) > 0 { + wtFlags.Grants = grants + } } if len(containerCmd) == 0 && len(cfg.Command) > 0 { containerCmd = cfg.Command @@ -183,6 +198,10 @@ func runWorktree(cmd *cobra.Command, args []string) error { ctx := cmd.Context() + // moat wt has no verb, so a valid agent: is preserved and an invalid one + // warns and is cleared (same pattern as moat run). + intcli.ResolveAgentField(cfg, "") + opts := intcli.ExecOptions{ Flags: wtFlags, Workspace: result.WorkspacePath, From e0a4213bbca9dc709783254ce8a1f3cbde0b6ca8 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 02:13:31 +0000 Subject: [PATCH 27/33] fix(join): point join remedy messages at the canonical agent name MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `moat join openai` is valid — provider.GetAgent resolves it through the openai->codex alias — but every remedy message interpolated the raw argument, so the suggested fix was `moat openai`, which is not a command. The diagnosis half of each message keeps the string the user typed, so they recognize what they asked for; the remedy half (`moat ` / `agents: []`) now uses the canonical name resolved via agent.Name(). Adds an `openai` row to TestValidateJoinAgent. A prior review believed the `j.IdentifiesAs(a)` branch in the membership loop was unreachable and proposed deleting it; this case proves it's load-bearing — `moat join openai` is accepted only through that branch, since agentArg stays "openai" while JoinableAgents holds "codex". --- cmd/moat/cli/join_cmd.go | 24 ++++++++--- cmd/moat/cli/join_cmd_test.go | 75 +++++++++++++++++++++++------------ cmd/moat/cli/joinpick.go | 11 +++-- cmd/moat/cli/joinpick_test.go | 46 +++++++++++++++++---- 4 files changed, 114 insertions(+), 42 deletions(-) diff --git a/cmd/moat/cli/join_cmd.go b/cmd/moat/cli/join_cmd.go index 555a37a7..1b7747f2 100644 --- a/cmd/moat/cli/join_cmd.go +++ b/cmd/moat/cli/join_cmd.go @@ -64,7 +64,13 @@ func init() { // legacy agent-string check; an EMPTY set is a real answer ("nothing joinable // here") and must not fall back. That distinction is why the metadata field is // persisted without omitempty. -func validateJoinAgent(j provider.JoinableAgent, agentArg string, r *run.Run) error { +// +// canonical is agentArg resolved through registry aliases (openai -> codex). +// The diagnosis half of each error keeps agentArg, the string the user typed, +// so they recognize what they asked for; the remedy half uses canonical, so a +// suggested `moat ` or `agents: []` names a real command/value — +// `moat openai` is not a command. +func validateJoinAgent(j provider.JoinableAgent, agentArg, canonical string, r *run.Run) error { if r.JoinableAgents != nil { for _, a := range r.JoinableAgents { if a == agentArg || j.IdentifiesAs(a) { @@ -77,7 +83,7 @@ func validateJoinAgent(j provider.JoinableAgent, agentArg string, r *run.Run) er } return fmt.Errorf("run %s cannot host %s (provisioned agents: %s).\n"+ "Add %s to this project's moat.yaml `agents:` list and recreate the run.", - r.ID, agentArg, hosted, agentArg) + r.ID, agentArg, hosted, canonical) } // Pre-upgrade run: no capability set was ever recorded. @@ -86,7 +92,7 @@ func validateJoinAgent(j provider.JoinableAgent, agentArg string, r *run.Run) er } return fmt.Errorf("run %s was created before capability tracking and records agent %q.\n"+ "Recreate the run to join it (moat stop %s && moat %s).", - r.ID, r.Agent, r.ID, agentArg) + r.ID, r.Agent, r.ID, canonical) } // joinableAgentNames returns the sorted names of registered agents that support @@ -154,6 +160,12 @@ func runJoin(cmd *cobra.Command, args []string) error { if !ok { return fmt.Errorf("agent %q does not support join yet", agentArg) } + // canonical is what the user typed, resolved through registry aliases + // (openai -> codex) — agentArg itself must stay as typed for the + // membership checks below (JoinableAgents / IdentifiesAs match against + // the canonical name while agentArg may be the alias), but any remedy + // text suggesting a command must say `moat codex`, not `moat openai`. + canonical := agent.Name() var r *run.Run if runArg == "" { @@ -164,7 +176,7 @@ func runJoin(cmd *cobra.Command, args []string) error { allRuns := manager.List() candidates, widened := inferJoinCandidates(allRuns, cwd, agentArg, joinable) anyRunning := len(filterRunning(allRuns)) > 0 - picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, widened, + picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, canonical, widened, term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr), anyRunning) if pickErr != nil { return pickErr @@ -185,7 +197,7 @@ func runJoin(cmd *cobra.Command, args []string) error { // the user named a run explicitly, so there was no workspace-widening // search to disclose. anyRunning=true: resolveRunningRunArg only // returns a candidate list when more than one running run matched. - picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, false, + picked, pickErr := pickJoinRun(os.Stdin, os.Stderr, candidates, agentArg, canonical, false, term.IsTerminal(os.Stdin) && term.IsTerminal(os.Stderr), true) if pickErr != nil { return pickErr @@ -194,7 +206,7 @@ func runJoin(cmd *cobra.Command, args []string) error { } } - if valErr := validateJoinAgent(joinable, agentArg, r); valErr != nil { + if valErr := validateJoinAgent(joinable, agentArg, canonical, r); valErr != nil { return valErr } diff --git a/cmd/moat/cli/join_cmd_test.go b/cmd/moat/cli/join_cmd_test.go index c5e332e3..576d8069 100644 --- a/cmd/moat/cli/join_cmd_test.go +++ b/cmd/moat/cli/join_cmd_test.go @@ -24,48 +24,71 @@ func (f fakeJoinable) IdentifiesAs(agent string) bool { func TestValidateJoinAgent(t *testing.T) { claude := fakeJoinable{names: []string{"claude", "claude-code"}} + codex := fakeJoinable{names: []string{"codex"}} tests := []struct { - name string - run *run.Run - agent string - wantErr bool - errHas string + name string + run *run.Run + agent string + canonical string + joinable fakeJoinable + wantErr bool + errHas string }{ { - name: "member of the capability set is accepted", - run: &run.Run{ID: "run_1", JoinableAgents: []string{"claude"}}, - agent: "claude", + name: "member of the capability set is accepted", + run: &run.Run{ID: "run_1", JoinableAgents: []string{"claude"}}, + agent: "claude", + joinable: claude, }, { - name: "non-member is rejected even when the agent string matches", - run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{"codex"}}, - agent: "claude", - wantErr: true, - errHas: "codex", + name: "non-member is rejected even when the agent string matches", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{"codex"}}, + agent: "claude", + joinable: claude, + wantErr: true, + errHas: "codex", }, { - name: "empty set refuses", - run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{}}, - agent: "claude", - wantErr: true, + name: "empty set refuses", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: []string{}}, + agent: "claude", + joinable: claude, + wantErr: true, }, { - name: "nil set falls back and accepts a matching agent string", - run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: nil}, - agent: "claude", + name: "nil set falls back and accepts a matching agent string", + run: &run.Run{ID: "run_1", Agent: "claude", JoinableAgents: nil}, + agent: "claude", + joinable: claude, }, { - name: "nil set falls back and refuses a stale agent string", - run: &run.Run{ID: "run_1", Agent: "vibrant-code", JoinableAgents: nil}, - agent: "claude", - wantErr: true, - errHas: "Recreate the run", + name: "nil set falls back and refuses a stale agent string", + run: &run.Run{ID: "run_1", Agent: "vibrant-code", JoinableAgents: nil}, + agent: "claude", + joinable: claude, + wantErr: true, + errHas: "Recreate the run", + }, + // Regression: `moat join openai` must keep working. agentArg + // stays the alias "openai" (parseJoinArgs/runJoin never rewrite it — + // only the remedy text uses the canonical name), while JoinableAgents + // holds the canonical "codex" recorded at provisioning time. The + // direct `a == agentArg` comparison can never match here ("codex" != + // "openai"); acceptance depends entirely on the `j.IdentifiesAs(a)` + // branch. A prior review believed that branch was unreachable and + // proposed deleting it — this case is what proves it's load-bearing. + { + name: "alias arg (openai) matches a canonical member via IdentifiesAs", + run: &run.Run{ID: "run_1", JoinableAgents: []string{"codex"}}, + agent: "openai", + canonical: "codex", + joinable: codex, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - err := validateJoinAgent(claude, tt.agent, tt.run) + err := validateJoinAgent(tt.joinable, tt.agent, tt.canonical, tt.run) if (err != nil) != tt.wantErr { t.Fatalf("validateJoinAgent() error = %v, wantErr %v", err, tt.wantErr) } diff --git a/cmd/moat/cli/joinpick.go b/cmd/moat/cli/joinpick.go index 8d43c81c..b906dcbe 100644 --- a/cmd/moat/cli/joinpick.go +++ b/cmd/moat/cli/joinpick.go @@ -149,15 +149,20 @@ func readSelection(r io.Reader, n int) (int, error) { // empty, inferJoinCandidates has already filtered by capability across every // running run, local or not — so the caller must supply the distinction from // the unfiltered population. -func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg string, widened, isTTY, anyRunning bool) (*run.Run, error) { +// +// canonical is agentArg resolved through registry aliases (openai -> codex); +// the diagnosis halves of the errors below keep agentArg (what the user +// typed), the remedy halves use canonical, so a suggested `moat ` or +// `agents: []` names a real command/value. +func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg, canonical string, widened, isTTY, anyRunning bool) (*run.Run, error) { switch len(candidates) { case 0: if !anyRunning { - return nil, fmt.Errorf("no runs are running; start one with `moat %s`.", agentArg) + return nil, fmt.Errorf("no runs are running; start one with `moat %s`.", canonical) } return nil, fmt.Errorf("no running run can host %s.\n"+ "Add %s to this project's moat.yaml `agents:` list and recreate the run, or run `moat list` to see what is running.", - agentArg, agentArg) + agentArg, canonical) case 1: if !widened { return candidates[0], nil diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go index d29d11a5..529137fe 100644 --- a/cmd/moat/cli/joinpick_test.go +++ b/cmd/moat/cli/joinpick_test.go @@ -172,7 +172,7 @@ func TestPickJoinRun(t *testing.T) { two := &run.Run{ID: "run_two", Name: "other", JoinableAgents: []string{"claude"}} t.Run("single candidate auto-selects", func(t *testing.T) { - got, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", false, true, true) + got, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", "claude", false, true, true) if err != nil || got != one { t.Errorf("expected auto-select; got %v, %v", got, err) } @@ -185,7 +185,7 @@ func TestPickJoinRun(t *testing.T) { // value — the return value alone is satisfied even if the widened // guard is deleted and case 1 falls straight through to auto-select. var out bytes.Buffer - got, err := pickJoinRun(strings.NewReader("1\n"), &out, []*run.Run{one}, "claude", true, true, true) + got, err := pickJoinRun(strings.NewReader("1\n"), &out, []*run.Run{one}, "claude", "claude", true, true, true) if err != nil || got != one { t.Errorf("expected prompted selection; got %v, %v", got, err) } @@ -198,7 +198,7 @@ func TestPickJoinRun(t *testing.T) { }) t.Run("non-TTY errors with the IDs", func(t *testing.T) { - _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one, two}, "claude", false, false, true) + _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one, two}, "claude", "claude", false, false, true) if err == nil { t.Fatal("expected an error without a TTY") } @@ -210,7 +210,7 @@ func TestPickJoinRun(t *testing.T) { }) t.Run("zero candidates, nothing running at all", func(t *testing.T) { - _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true, false) + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", "claude", false, true, false) if err == nil { t.Fatal("expected an error with no candidates") } @@ -224,7 +224,7 @@ func TestPickJoinRun(t *testing.T) { // imply different next steps (start a run vs. recreate one with this // agent), so the messages must differ. t.Run("zero candidates, runs are running but none can host the agent", func(t *testing.T) { - _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", false, true, true) + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", "claude", false, true, true) if err == nil { t.Fatal("expected an error with no candidates") } @@ -235,6 +235,38 @@ func TestPickJoinRun(t *testing.T) { t.Errorf("this case must not suggest starting a run; got %q", err) } }) + + // I3 regression: `moat join openai` must not suggest running `moat + // openai` — that's not a command. The diagnosis half of each message + // keeps agentArg (what the user typed); the remedy half (the `moat + // ` / `agents: []` suggestion) must use the canonical name. + t.Run("zero candidates, remedy names the canonical agent, not the alias", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "openai", "codex", false, true, false) + if err == nil { + t.Fatal("expected an error with no candidates") + } + if !strings.Contains(err.Error(), "moat codex") { + t.Errorf("error should suggest `moat codex`; got %q", err) + } + if strings.Contains(err.Error(), "moat openai") { + t.Errorf("error must not suggest the non-existent `moat openai`; got %q", err) + } + }) + + // Companion: the "runs exist but none can host it" remedy must also use + // the canonical name, while the diagnosis half keeps the typed alias. + t.Run("zero candidates but running, remedy names the canonical agent, diagnosis names the alias", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "openai", "codex", false, true, true) + if err == nil { + t.Fatal("expected an error with no candidates") + } + if !strings.Contains(err.Error(), "host openai") { + t.Errorf("diagnosis should name what the user typed (openai); got %q", err) + } + if !strings.Contains(err.Error(), "Add codex to") { + t.Errorf("remedy should name the canonical agent (codex); got %q", err) + } + }) } // TestTwoArgMultiMatchUsesPicker locks in the shape runJoin's two-arg path @@ -247,7 +279,7 @@ func TestTwoArgMultiMatchUsesPicker(t *testing.T) { b := &run.Run{ID: "run_b", Name: "moat-dev", State: run.StateRunning, JoinableAgents: []string{"claude"}} // TTY: selecting 2 picks the second candidate. - got, err := pickJoinRun(strings.NewReader("2\n"), io.Discard, []*run.Run{a, b}, "claude", false, true, true) + got, err := pickJoinRun(strings.NewReader("2\n"), io.Discard, []*run.Run{a, b}, "claude", "claude", false, true, true) if err != nil { t.Fatalf("pickJoinRun: %v", err) } @@ -256,7 +288,7 @@ func TestTwoArgMultiMatchUsesPicker(t *testing.T) { } // Companion: no TTY still errors with both IDs listed. - _, err = pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{a, b}, "claude", false, false, true) + _, err = pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{a, b}, "claude", "claude", false, false, true) if err == nil { t.Fatal("non-TTY should error rather than prompt") } From 4ba906a61e58796cb5d5ac1fcd14f5f4d93bd8b9 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 02:13:38 +0000 Subject: [PATCH 28/33] docs: correct agents[0] foreground claim and document --grant's override of agents: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The multi-agent guide and moat.yaml reference both claimed that with no agent: set, moat run launches agents[0] in the foreground. It doesn't: moat run with no `-- command` and no `command:` in moat.yaml runs /bin/bash (internal/run/manager_create.go). agents[0] only backfills agent:, which drives agent-specific defaults (container memory, implied dependencies, language-server support) — nothing constructs an agent invocation from it. This branch exists partly because agent: was mis-documented as something it wasn't; the docs now describe agents[0]'s actual effect and point at the agent's own verb (moat claude) for running it in the foreground. Also documents that --grant on moat run / moat wt replaces moat.yaml's configured grants rather than adding to them, including grants agents: derives — previously undocumented, and now the first case where it silently drops something a user's moat.yaml declares. This is existing flag behavior; changing it to union is out of scope here. --- docs/content/guides/14-multi-agent.md | 10 ++++++---- docs/content/reference/01-cli.md | 4 ++++ docs/content/reference/02-moat-yaml.md | 16 ++++++++++++++-- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/docs/content/guides/14-multi-agent.md b/docs/content/guides/14-multi-agent.md index 5a2ec4b1..ccaa1beb 100644 --- a/docs/content/guides/14-multi-agent.md +++ b/docs/content/guides/14-multi-agent.md @@ -42,17 +42,19 @@ By default, a run only provisions the agent you started it with — `moat claude agents: [claude, codex] ``` -This provisions both agents' dependencies, credential grants, and network rules into the container. With no `agent:` field set, `moat run` launches the first entry (`claude`) in the foreground; `codex` is reachable only via `moat join`: +This provisions both agents' dependencies, credential grants, and network rules into the container. With no `agent:` field set, `agents[0]` (`claude`) becomes the run's recorded **primary agent** — used for agent-specific defaults like container memory, implied dependencies, and language-server support. It does not change what the run executes: `moat run` with no `-- command` and no `command:` in `moat.yaml` still starts a shell, never an agent. To run an agent in the foreground, use its own verb: ```bash -moat run -# or: moat claude, since agent: is unset and agents[0] is claude +moat claude +# agents[0] (claude) would be recorded as the primary agent even if you ran +# `moat run` instead — but moat run itself still starts your configured +# command, or a shell, not an agent. moat claude runs it directly. # from a second terminal moat join run_a1b2c3d4e5f6 codex ``` -Order in `agents:` matters only for picking the foreground agent — every entry after the first is equally joinable. +Order in `agents:` matters only for picking the primary agent — every entry after the first is equally joinable. ## Joining without a run ID diff --git a/docs/content/reference/01-cli.md b/docs/content/reference/01-cli.md index 8004dc1e..bd8093c5 100644 --- a/docs/content/reference/01-cli.md +++ b/docs/content/reference/01-cli.md @@ -135,6 +135,8 @@ moat run [flags] [path] [-- command] | `--no-prompt` | Never prompt to grant missing credentials; fail with the missing-grants error instead. Also set via `MOAT_NO_PROMPT=1`. Prompting only happens on an interactive terminal. | | `--tty-trace FILE` | Capture terminal I/O to file for debugging (e.g., `session.json`) | +`--grant` **replaces** `moat.yaml`'s configured grants rather than adding to them — this includes grants derived from [`agents:`](./02-moat-yaml.md#agents). `moat run --grant github` with `agents: [claude, codex]` runs with only the `github` grant; neither agent's credential is injected, so an agent that needs one fails to authenticate inside the container. Pass every grant you need explicitly (`--grant github --grant claude --grant openai`), or omit `--grant` to use `moat.yaml`'s grants (including `agents:`-derived ones) unmodified. + ### Execution modes **Non-interactive (default):** Output streams to the terminal. Press `Ctrl+C` to stop. @@ -544,6 +546,8 @@ Configuration is read from `moat.yaml` in the repository root. If a run is alrea | `--no-prompt` | Never prompt to grant missing credentials; fail instead. Also set via `MOAT_NO_PROMPT=1`. | | `--tty-trace FILE` | Capture terminal I/O to file for debugging | +`--grant` **replaces** `moat.yaml`'s configured grants rather than adding to them — this includes grants derived from [`agents:`](./02-moat-yaml.md#agents), the same as `moat run` (see above). + ### Run naming The run name is `{name}-{branch}` when `moat.yaml` has a `name` field, otherwise just `{branch}`. diff --git a/docs/content/reference/02-moat-yaml.md b/docs/content/reference/02-moat-yaml.md index 595b4693..1a0eff87 100644 --- a/docs/content/reference/02-moat-yaml.md +++ b/docs/content/reference/02-moat-yaml.md @@ -230,10 +230,22 @@ agents: [claude, codex] - Type: `array[string]` - Allowed values: same as `agent`, except `pi` -- Order matters: for `moat run` with no `agent:` set, the first entry is the - foreground agent. Every other entry is reachable only via `moat join`. +- Order matters: with no `agent:` set, the first entry backfills `agent:` as + the run's primary agent — used for agent-specific defaults (container + memory, implied dependencies, language-server support), not for what the + run executes. `moat run` still starts `command:` / `-- command`, or a + shell, regardless of `agents:`; to run an agent in the foreground, use its + verb (`moat claude`). Every entry after the first is reachable only via + `moat join`. - An unrecognized entry is an error, not a warning — a dropped entry would leave the container without that agent's credential and firewall rules. +- `--grant` on `moat run` / `moat wt` **replaces** the grants this field + derives, rather than adding to them. `moat run --grant github` with + `agents: [claude, codex]` runs with only the `github` grant; neither agent's + credential is injected. The agents still get their dependencies and network + rules, so `moat join` still succeeds — the agent then fails to authenticate + once joined. Pass every grant you need explicitly with `--grant` when using + it alongside `agents:`. See [Multi-agent sessions](../guides/14-multi-agent.md) for how `moat join` uses this list. From 0a5da8b835f1e51f41a542c110d5803f5c2a2b48 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Wed, 12 Aug 2026 02:13:41 +0000 Subject: [PATCH 29/33] docs(changelog): summarize the multi-agent join work in Unreleased The Unreleased summary paragraph covered only the Codex/TTY work and never mentioned agents: or the moat join capability-gating fix, even though it's a headline feature of this release. --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5dcf9003..aab87081 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ Realigns the Codex integration with the current Codex CLI (0.146). Codex's own a A second pass fixes the terminal handling that made Codex hard to use inside Moat: `ctrl+/` did not register at all, and Codex's inline rendering was corrupted because Moat overrode terminal state the agent legitimately drives. +A third pass adds multi-agent containers: `moat.yaml`'s new `agents:` list provisions several agents into one run, and `moat join` gates on what moat actually provisioned instead of an unvalidated `agent:` string, fixing joins that previously failed for any project using a project-shaped `agent:` value. + ### Added - **`MOAT_TTY_TRACE`** — an environment-variable equivalent of `--tty-trace`, recording a session's terminal I/O to a file for debugging TUI and input problems. The sessions worth tracing are the broken ones, where the in-session `ctrl+/ d` dump may itself be unreachable, and exporting a variable captures every subsequent run without editing each command line. See [Environment variables](https://majorcontext.com/moat/reference/environment). ([#450](https://github.com/majorcontext/moat/pull/450)) From ec93d5cd01213ce2bed2f7064cb0f0f7ec8c06fa Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Thu, 13 Aug 2026 01:20:37 +0000 Subject: [PATCH 30/33] fix(cli): write agents:-derived grants back onto cfg.Grants ExpandAgents returns derived grants instead of appending them to cfg.Grants (a33e6f6) so buildGrants can't wrongly treat them as explicit and suppress an auto-detected credential. But nothing wrote the returned grants back, so two downstream readers that consult cfg.Grants directly silently stopped seeing agent-derived grants: Config.ShouldSyncCodexLogs/ShouldSyncGeminiLogs (dropping the session-transcript host mount) and buildLocalMCPConfig's grant validation (hard-failing run creation for a local MCP server's declared grant). Add AppendDerivedGrants and call it after grant precedence resolution completes at all three call sites, so the write-back can't re-enter buildGrants' suppression logic. --- cmd/moat/cli/run.go | 7 +++ cmd/moat/cli/wt.go | 7 +++ internal/cli/agents.go | 29 +++++++++++++ internal/cli/agents_test.go | 59 ++++++++++++++++++++++++++ internal/cli/provider.go | 8 ++++ internal/run/manager_agentinit_test.go | 34 +++++++++++++++ 6 files changed, 144 insertions(+) diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index 59fb2631..fa7df475 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -139,6 +139,13 @@ func runAgent(cmd *cobra.Command, args []string) error { runFlags.Grants = grants } } + // Write derived grants back into cfg.Grants now that the defaulting + // block above has already read cfg.Grants into runFlags.Grants — see + // intcli.AppendDerivedGrants' doc comment for why this must run after, + // not before. cfg.Grants has its own direct readers downstream + // (ShouldSyncCodexLogs, ShouldSyncGeminiLogs, buildLocalMCPConfig's + // grant validation) that never see runFlags.Grants. + intcli.AppendDerivedGrants(cfg, derivedGrants) if len(containerCmd) == 0 && len(cfg.Command) > 0 { containerCmd = cfg.Command } diff --git a/cmd/moat/cli/wt.go b/cmd/moat/cli/wt.go index 55b1dbf0..bc085c60 100644 --- a/cmd/moat/cli/wt.go +++ b/cmd/moat/cli/wt.go @@ -169,6 +169,13 @@ func runWorktree(cmd *cobra.Command, args []string) error { wtFlags.Grants = grants } } + // Write derived grants back into cfg.Grants now that the defaulting + // block above has already read cfg.Grants into wtFlags.Grants — see + // intcli.AppendDerivedGrants' doc comment for why this must run after, + // not before. cfg.Grants has its own direct readers downstream + // (ShouldSyncCodexLogs, ShouldSyncGeminiLogs, buildLocalMCPConfig's + // grant validation) that never see wtFlags.Grants. + intcli.AppendDerivedGrants(cfg, derivedGrants) if len(containerCmd) == 0 && len(cfg.Command) > 0 { containerCmd = cfg.Command } diff --git a/internal/cli/agents.go b/internal/cli/agents.go index 7553ab0d..b06fcbff 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -194,6 +194,35 @@ func ExpandAgents(cfg *config.Config) ([]string, error) { return derivedGrants, nil } +// AppendDerivedGrants appends the derived grants returned by ExpandAgents +// onto cfg.Grants, skipping any already present. Callers must invoke this +// AFTER grant precedence resolution has read cfg.Grants as the "explicit" +// bucket (buildGrants in internal/cli/provider.go, or the equivalent +// grants-defaulting block in `moat run`/`moat wt`) — never before. Writing +// derived grants into cfg.Grants earlier would let them re-enter that +// resolution as if the user had declared them, resurrecting the bug +// ExpandAgents' doc comment describes (a derived "claude" grant wrongly +// suppressing an auto-detected "anthropic" API-key credential). +// +// This exists because cfg.Grants has two downstream readers besides grant +// resolution — Config.ShouldSyncCodexLogs/ShouldSyncGeminiLogs and +// buildLocalMCPConfig's grant validation (internal/run/manager_agentinit.go) +// — that read cfg.Grants directly rather than the resolved grants list. +// Without this write-back, an agents:-derived grant (e.g. "openai" from +// `agents: [codex]`) is invisible to them: log sync silently stays off, and a +// local MCP server's `grant: openai` is rejected as "not declared in +// top-level grants list" even though the credential is provisioned. +func AppendDerivedGrants(cfg *config.Config, derivedGrants []string) { + if cfg == nil { + return + } + for _, g := range derivedGrants { + if !slices.Contains(cfg.Grants, g) { + cfg.Grants = append(cfg.Grants, g) + } + } +} + // hasNetworkHost reports whether rules already contains an entry for host. func hasNetworkHost(rules []netrules.NetworkRuleEntry, host string) bool { for _, r := range rules { diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index cde4982c..8ebd7795 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -232,6 +232,65 @@ func TestExpandAgentsDoesNotDuplicate(t *testing.T) { } } +// TestAppendDerivedGrants is a regression test: ExpandAgents deliberately +// returns derived grants instead of writing them into cfg.Grants (see its +// doc comment), but two downstream readers — Config.ShouldSyncCodexLogs / +// ShouldSyncGeminiLogs and buildLocalMCPConfig's grant validation in +// internal/run — read cfg.Grants directly and never see the returned slice. +// Callers must write the derived grants back with AppendDerivedGrants after +// grant precedence resolution runs. +func TestAppendDerivedGrants(t *testing.T) { + cfg := &config.Config{Grants: []string{"github"}} + cli.AppendDerivedGrants(cfg, []string{"openai", "github"}) + if !slices.Contains(cfg.Grants, "openai") { + t.Errorf("expected openai appended; got %v", cfg.Grants) + } + if countOccurrences(cfg.Grants, "github") != 1 { + t.Errorf("github was already present; must not be duplicated: got %v", cfg.Grants) + } + // A nil cfg must not panic — provider.go can still hold a nil *Config at + // the point buildGrants runs, before it's defaulted to &config.Config{}. + cli.AppendDerivedGrants(nil, []string{"openai"}) +} + +// TestExpandAgentsWriteBackEnablesCodexLogSync reproduces the regression this +// fix addresses: `agents: [codex]` with no explicit top-level grants used to +// leave cfg.Grants empty after ExpandAgents, so ShouldSyncCodexLogs (which +// reads cfg.Grants directly, not ExpandAgents' return value) silently stayed +// false and the codex session-transcript mount was never added. Writing the +// derived grants back with AppendDerivedGrants — the fix — makes cfg.Grants +// carry "openai" so ShouldSyncCodexLogs sees it. +func TestExpandAgentsWriteBackEnablesCodexLogSync(t *testing.T) { + cfg := &config.Config{Agents: []string{"codex"}} + derived, err := cli.ExpandAgents(cfg) + if err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + cli.AppendDerivedGrants(cfg, derived) + if !slices.Contains(cfg.Grants, "openai") { + t.Fatalf("expected openai written back into cfg.Grants; got %v", cfg.Grants) + } + if !cfg.ShouldSyncCodexLogs() { + t.Errorf("ShouldSyncCodexLogs() = false, want true once the derived openai grant is on cfg.Grants") + } +} + +// TestExpandAgentsWriteBackCompanionNoAgentsNoGrant is the companion to +// TestExpandAgentsWriteBackEnablesCodexLogSync: a config with neither +// `agents:` nor an explicit openai grant must still report false — the fix +// must not make ShouldSyncCodexLogs default to true. +func TestExpandAgentsWriteBackCompanionNoAgentsNoGrant(t *testing.T) { + cfg := &config.Config{} + derived, err := cli.ExpandAgents(cfg) + if err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + cli.AppendDerivedGrants(cfg, derived) + if cfg.ShouldSyncCodexLogs() { + t.Errorf("ShouldSyncCodexLogs() = true, want false with no agents: and no openai grant; cfg.Grants = %v", cfg.Grants) + } +} + func TestExpandAgentsRejectsBadEntries(t *testing.T) { tests := []struct { name string diff --git a/internal/cli/provider.go b/internal/cli/provider.go index 526339c9..5c875c64 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -158,6 +158,14 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error grants := buildGrants(autoDetected, configGrants, rc.Flags.Grants, derivedGrants) rc.Flags.Grants = grants + // Write derived grants back into cfg.Grants now that buildGrants has + // already read configGrants as the "explicit" bucket above — see + // AppendDerivedGrants' doc comment for why this must run after, not + // before. Downstream readers of cfg.Grants directly (ShouldSyncCodexLogs, + // ShouldSyncGeminiLogs, buildLocalMCPConfig's grant validation) need + // agents:-derived grants to be visible on cfg, not just on rc.Flags.Grants. + AppendDerivedGrants(cfg, derivedGrants) + interactive := rc.PromptFlag == "" // Build container command (provider-specific logic) diff --git a/internal/run/manager_agentinit_test.go b/internal/run/manager_agentinit_test.go index 12a2357b..2c59928b 100644 --- a/internal/run/manager_agentinit_test.go +++ b/internal/run/manager_agentinit_test.go @@ -6,9 +6,14 @@ import ( "strings" "testing" + intcli "github.com/majorcontext/moat/internal/cli" "github.com/majorcontext/moat/internal/config" "github.com/majorcontext/moat/internal/credential" "github.com/majorcontext/moat/internal/provider" + + // Registers the codex provider so intcli.ExpandAgents can resolve + // "codex" to its AgentRuntime (dependencies, grant, network hosts). + _ "github.com/majorcontext/moat/internal/providers" ) func TestSetupCodexStaging_UnknownGrant(t *testing.T) { @@ -34,6 +39,35 @@ func TestSetupCodexStaging_GrantNotDeclared(t *testing.T) { } } +// TestSetupCodexStaging_AgentsDerivedGrantSatisfiesLocalMCP reproduces the +// full CLI pipeline for the regression fixed alongside this test: `agents: +// [codex]` derives the "openai" grant, but ExpandAgents deliberately returns +// it instead of writing it into cfg.Grants (see its doc comment). Without the +// intcli.AppendDerivedGrants write-back that `moat run`/`moat wt`/RunProvider +// now perform after grant resolution, cfg.Grants stays empty and a local +// codex.mcp entry declaring `grant: openai` is rejected here as "not +// declared in top-level grants list" even though no top-level `grants:` was +// ever needed — the agents: entry alone should suffice. +func TestSetupCodexStaging_AgentsDerivedGrantSatisfiesLocalMCP(t *testing.T) { + m := &Manager{} + cfg := &config.Config{Agents: []string{"codex"}} + cfg.Codex.MCP = map[string]config.MCPServerSpec{"srv": {Grant: "openai", Command: "run"}} + + derived, err := intcli.ExpandAgents(cfg) + if err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + intcli.AppendDerivedGrants(cfg, derived) + if !strings.Contains(strings.Join(cfg.Grants, ","), "openai") { + t.Fatalf("expected openai written back into cfg.Grants by AppendDerivedGrants; got %v", cfg.Grants) + } + + fake := &captureAgentProvider{} + if _, err := m.setupCodexStaging(context.Background(), fake, Options{Config: cfg}, &Run{}, false, "", "", nil); err != nil { + t.Fatalf("expected agents:-derived openai grant to satisfy the local MCP grant check, got error: %v", err) + } +} + // captureAgentProvider records the PrepareOpts passed to PrepareContainer. // The embedded interface is nil; only PrepareContainer may be called. type captureAgentProvider struct { From 24ce3fbd39e77af46197ce9938e1a9cd447751eb Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Thu, 13 Aug 2026 05:25:42 +0000 Subject: [PATCH 31/33] docs(changelog): link the multi-agent join entries to their PR --- CHANGELOG.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aab87081..09a9125e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,8 +16,8 @@ A third pass adds multi-agent containers: `moat.yaml`'s new `agents:` list provi - **`MOAT_TTY_TRACE`** — an environment-variable equivalent of `--tty-trace`, recording a session's terminal I/O to a file for debugging TUI and input problems. The sessions worth tracing are the broken ones, where the in-session `ctrl+/ d` dump may itself be unreachable, and exporting a variable captures every subsequent run without editing each command line. See [Environment variables](https://majorcontext.com/moat/reference/environment). ([#450](https://github.com/majorcontext/moat/pull/450)) - **Remote MCP servers for Codex** — top-level `mcp:` entries are now wired into Codex, not just Claude Code. They are written to the `[mcp_servers]` table of the generated `~/.codex/config.toml` as streamable HTTP servers whose `url` points at the proxy relay, so the proxy injects the real credential exactly as it does for Claude Code. Previously `mcp:` was silently ignored for Codex runs. See [MCP servers](https://majorcontext.com/moat/guides/mcp). ([#449](https://github.com/majorcontext/moat/pull/449)) -- **Multi-agent containers** — `agents: [claude, codex]` in `moat.yaml` provisions several agents into one container, each with its dependencies, credential grant, and network rules. `moat join codex` then works in a container started by `moat claude`. See [Multi-agent sessions](https://majorcontext.com/moat/guides/multi-agent). ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) -- `moat join ` infers the run from the current workspace, offering a picker when several qualify. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- **Multi-agent containers** — `agents: [claude, codex]` in `moat.yaml` provisions several agents into one container, each with its dependencies, credential grant, and network rules. `moat join codex` then works in a container started by `moat claude`. See [Multi-agent sessions](https://majorcontext.com/moat/guides/multi-agent). ([#454](https://github.com/majorcontext/moat/pull/454)) +- `moat join ` infers the run from the current workspace, offering a picker when several qualify. ([#454](https://github.com/majorcontext/moat/pull/454)) ### Changed @@ -36,10 +36,10 @@ A third pass adds multi-agent containers: `moat.yaml`'s new `agents:` list provi - Fix `codex.sync_logs` never syncing anything — the setting was documented as writing session logs to the host, and defaulted on whenever the `openai` grant was configured, but no mount was ever created: the flag only influenced whether the Codex staging directory was built. Codex session transcripts now appear on the host at `~/.moat/codex/sessions/-/YYYY/MM/DD/rollout-*.jsonl`, in Codex's own format. ([#449](https://github.com/majorcontext/moat/pull/449)) - Fix Codex prompting to trust `/workspace` on first run — the generated config now marks the workspace trusted. ([#449](https://github.com/majorcontext/moat/pull/449)) - `codex.mcp` and `gemini.mcp` may now both declare local MCP servers in one `moat.yaml`. They previously collided on `/workspace/.mcp.json` and were rejected at config load; Codex no longer uses that file. ([#449](https://github.com/majorcontext/moat/pull/449)) -- Fix `moat join` refusing to attach to containers that were running the agent — previously, join compared `moat.yaml`'s `agent:` field against a fixed list, so any other value (including the project-shaped names the reference docs and `moat init` both produced) made every join fail. Join now uses the set of agents moat actually provisioned into the container. Runs created before this change must be recreated. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) -- Fix `moat join ` failing when several runs share a name — previously, any project setting `name:` in `moat.yaml` gave every run the same name, and a second concurrent run made join error instead of offering a choice. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) -- Fix `moat join`'s extra positional arguments being silently ignored — `moat join ` used to accept and discard ``; it is now a usage error. `moat join` now takes one or two positional arguments (`[run] agent`), not a minimum of two. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) -- Fix the `agent:` reference documentation, which described the field as a free-form identifier defaulting to `name`. It is a fixed set of agent names and defaults to the command you ran. ([#NNN](https://github.com/majorcontext/moat/pull/NNN)) +- Fix `moat join` refusing to attach to containers that were running the agent — previously, join compared `moat.yaml`'s `agent:` field against a fixed list, so any other value (including the project-shaped names the reference docs and `moat init` both produced) made every join fail. Join now uses the set of agents moat actually provisioned into the container. Runs created before this change must be recreated. ([#454](https://github.com/majorcontext/moat/pull/454)) +- Fix `moat join ` failing when several runs share a name — previously, any project setting `name:` in `moat.yaml` gave every run the same name, and a second concurrent run made join error instead of offering a choice. ([#454](https://github.com/majorcontext/moat/pull/454)) +- Fix `moat join`'s extra positional arguments being silently ignored — `moat join ` used to accept and discard ``; it is now a usage error. `moat join` now takes one or two positional arguments (`[run] agent`), not a minimum of two. ([#454](https://github.com/majorcontext/moat/pull/454)) +- Fix the `agent:` reference documentation, which described the field as a free-form identifier defaulting to `name`. It is a fixed set of agent names and defaults to the command you ran. ([#454](https://github.com/majorcontext/moat/pull/454)) ### Breaking From ee08d404f6c0e10898429eeb2134a7b79935bdea Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Thu, 13 Aug 2026 16:56:06 +0000 Subject: [PATCH 32/33] fix: address review feedback on multi-agent join (#454) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Gate the not-in-`agents:` warning on the no-verb path. With a CLI verb, RunProvider provisions that agent's dependencies, grants, and network hosts independently of `agents:`, so the warning's claim was false there, not just misleadingly worded — and it fired alongside the conflict warning. - Add provider.AgentAliases() and include agent aliases in KnownAgentNames, so `openai` appears in the valid-values list that already accepts it. - Extract the ExpandAgents -> grants-merge -> AppendDerivedGrants sequence into intcli.ApplyAgentDefaults, shared by `moat run` and `moat wt`. The ordering invariant is now documented once instead of duplicated at both call sites. - Add a drift guard tying agentCLIDep to the AgentRuntime implementors, so a new provider that forgets an entry fails loudly instead of becoming silently unjoinable. - Fix noun agreement in the picker's single-candidate non-TTY error. Note: pre-existing failure in internal/deps.TestRegistryGithubBinaryURLsExist not addressed by this PR. --- cmd/moat/cli/joinpick.go | 11 ++- cmd/moat/cli/joinpick_test.go | 22 ++++++ cmd/moat/cli/run.go | 37 ++--------- cmd/moat/cli/wt.go | 42 +++--------- internal/cli/agents.go | 10 ++- internal/cli/agents_test.go | 42 +++++++++++- internal/cli/rundefaults.go | 57 ++++++++++++++++ internal/cli/rundefaults_test.go | 103 +++++++++++++++++++++++++++++ internal/provider/registry.go | 20 ++++++ internal/provider/registry_test.go | 20 ++++++ internal/run/joinable.go | 8 +++ internal/run/joinable_test.go | 41 ++++++++++++ 12 files changed, 345 insertions(+), 68 deletions(-) create mode 100644 internal/cli/rundefaults.go create mode 100644 internal/cli/rundefaults_test.go diff --git a/cmd/moat/cli/joinpick.go b/cmd/moat/cli/joinpick.go index b906dcbe..5955e5ac 100644 --- a/cmd/moat/cli/joinpick.go +++ b/cmd/moat/cli/joinpick.go @@ -174,8 +174,15 @@ func pickJoinRun(in io.Reader, out io.Writer, candidates []*run.Run, agentArg, c for i, r := range candidates { ids[i] = r.ID } - return nil, fmt.Errorf("%d running runs can host %s; specify one: %s", - len(candidates), agentArg, strings.Join(ids, ", ")) + plural := len(candidates) != 1 + runWord := "run" + specifyWord := "it" + if plural { + runWord = "runs" + specifyWord = "one" + } + return nil, fmt.Errorf("%d running %s can host %s; specify %s: %s", + len(candidates), runWord, agentArg, specifyWord, strings.Join(ids, ", ")) } renderPicker(out, candidates, agentArg, widened) diff --git a/cmd/moat/cli/joinpick_test.go b/cmd/moat/cli/joinpick_test.go index 529137fe..087d48ae 100644 --- a/cmd/moat/cli/joinpick_test.go +++ b/cmd/moat/cli/joinpick_test.go @@ -209,6 +209,28 @@ func TestPickJoinRun(t *testing.T) { } }) + // Companion: single widened candidate also errors in non-TTY (must confirm + // cross-workspace attachment), and the error message must use singular + // "run", not plural "runs". + t.Run("single WIDENED candidate non-TTY errors with correct pluralization", func(t *testing.T) { + _, err := pickJoinRun(strings.NewReader(""), io.Discard, []*run.Run{one}, "claude", "claude", true, false, true) + if err == nil { + t.Fatal("expected an error for single widened candidate without a TTY") + } + if !strings.Contains(err.Error(), "run_only") { + t.Errorf("error should list the candidate ID; got %q", err) + } + if !strings.Contains(err.Error(), "1 running run can host") { + t.Errorf("error should use singular 'run' not 'runs'; got %q", err) + } + if strings.Contains(err.Error(), "1 running runs") { + t.Errorf("error has incorrect plural form; got %q", err) + } + if !strings.Contains(err.Error(), "specify it:") { + t.Errorf("error should say 'specify it:' for a single candidate; got %q", err) + } + }) + t.Run("zero candidates, nothing running at all", func(t *testing.T) { _, err := pickJoinRun(strings.NewReader(""), io.Discard, nil, "claude", "claude", false, true, false) if err == nil { diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index fa7df475..86948262 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -111,44 +111,19 @@ func runAgent(cmd *cobra.Command, args []string) error { return fmt.Errorf("loading config: %w", err) } - // Expand agents: into dependencies/grants/network hosts before the - // "Apply config defaults" block below reads cfg.Grants into runFlags.Grants - // — an expansion that lands after would never reach the grants flag (see - // intcli.ExpandAgents doc comment for why ordering matters here). - // - // derivedGrants is returned rather than merged into cfg.Grants by - // ExpandAgents itself — see its doc comment — so it's combined into the - // default grants list explicitly below. - derivedGrants, err := intcli.ExpandAgents(cfg) - if err != nil { - return err - } - // Determine agent name: --name flag > config.Name > random if runFlags.Name == "" && cfg != nil && cfg.Name != "" { runFlags.Name = cfg.Name } // Random name generation happens in manager.Create if still empty - // Apply config defaults + // Apply config defaults: agents: expansion, grants, and command. See + // intcli.ApplyAgentDefaults for the ExpandAgents → grants-merge → + // AppendDerivedGrants ordering invariant this preserves. + if err = intcli.ApplyAgentDefaults(cfg, &runFlags.Grants, &containerCmd); err != nil { + return err + } if cfg != nil { - if len(runFlags.Grants) == 0 { - grants := append([]string{}, cfg.Grants...) - grants = append(grants, derivedGrants...) - if len(grants) > 0 { - runFlags.Grants = grants - } - } - // Write derived grants back into cfg.Grants now that the defaulting - // block above has already read cfg.Grants into runFlags.Grants — see - // intcli.AppendDerivedGrants' doc comment for why this must run after, - // not before. cfg.Grants has its own direct readers downstream - // (ShouldSyncCodexLogs, ShouldSyncGeminiLogs, buildLocalMCPConfig's - // grant validation) that never see runFlags.Grants. - intcli.AppendDerivedGrants(cfg, derivedGrants) - if len(containerCmd) == 0 && len(cfg.Command) > 0 { - containerCmd = cfg.Command - } // Check sandbox setting from config if cfg.Sandbox == "none" && !runFlags.NoSandbox { runFlags.NoSandbox = true diff --git a/cmd/moat/cli/wt.go b/cmd/moat/cli/wt.go index bc085c60..77f33204 100644 --- a/cmd/moat/cli/wt.go +++ b/cmd/moat/cli/wt.go @@ -131,17 +131,6 @@ func runWorktree(cmd *cobra.Command, args []string) error { cfg = wtCfg } - // Expand agents: into dependencies/grants/network hosts before the "Apply - // config defaults" block below reads cfg.Grants into wtFlags.Grants — same - // pattern as moat run (cmd/moat/cli/run.go). derivedGrants is returned - // rather than merged into cfg.Grants by ExpandAgents itself (see its doc - // comment), so it's combined into the default grants list explicitly - // below. - derivedGrants, err := intcli.ExpandAgents(cfg) - if err != nil { - return err - } - // Check for active run in this worktree manager, err := run.NewManager() if err != nil { @@ -160,28 +149,15 @@ func runWorktree(cmd *cobra.Command, args []string) error { wtFlags.Name = result.RunName } - // Apply config defaults (same pattern as moat run) - if cfg != nil { - if len(wtFlags.Grants) == 0 { - grants := append([]string{}, cfg.Grants...) - grants = append(grants, derivedGrants...) - if len(grants) > 0 { - wtFlags.Grants = grants - } - } - // Write derived grants back into cfg.Grants now that the defaulting - // block above has already read cfg.Grants into wtFlags.Grants — see - // intcli.AppendDerivedGrants' doc comment for why this must run after, - // not before. cfg.Grants has its own direct readers downstream - // (ShouldSyncCodexLogs, ShouldSyncGeminiLogs, buildLocalMCPConfig's - // grant validation) that never see wtFlags.Grants. - intcli.AppendDerivedGrants(cfg, derivedGrants) - if len(containerCmd) == 0 && len(cfg.Command) > 0 { - containerCmd = cfg.Command - } - if cfg.Sandbox == "none" && !wtFlags.NoSandbox { - wtFlags.NoSandbox = true - } + // Apply config defaults: agents: expansion, grants, and command (same + // pattern as moat run). See intcli.ApplyAgentDefaults for the + // ExpandAgents → grants-merge → AppendDerivedGrants ordering invariant + // this preserves. + if err = intcli.ApplyAgentDefaults(cfg, &wtFlags.Grants, &containerCmd); err != nil { + return err + } + if cfg != nil && cfg.Sandbox == "none" && !wtFlags.NoSandbox { + wtFlags.NoSandbox = true } // Determine interactive mode from config diff --git a/internal/cli/agents.go b/internal/cli/agents.go index b06fcbff..ec98712d 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -52,6 +52,9 @@ func KnownAgentNames() []string { for variant := range agentVariants { seen[variant] = true } + for _, alias := range provider.AgentAliases() { + seen[alias] = true + } names := make([]string, 0, len(seen)) for n := range seen { names = append(names, n) @@ -92,7 +95,12 @@ func ResolveAgentField(cfg *config.Config, verb string) { } ValidateAgent(cfg) - if cfg.Agent != "" && len(cfg.Agents) > 0 && !agentsListContains(cfg.Agents, cfg.Agent) { + // This warning only holds on the no-verb (`moat run`) path: when a verb is + // present, the block below immediately overwrites cfg.Agent with verb, and + // RunProvider provisions the verb's own dependencies/grants/network hosts + // independently of `agents:` — so the named agent genuinely is provisioned, + // and warning here would just contradict the conflict warning that follows. + if verb == "" && cfg.Agent != "" && len(cfg.Agents) > 0 && !agentsListContains(cfg.Agents, cfg.Agent) { ui.Warnf("moat.yaml `agent: %s` is not in `agents: %v`; it will run as the primary but "+ "add it to `agents:` so its dependencies and grants are provisioned.", cfg.Agent, cfg.Agents) diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index 8ebd7795..a48b268c 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -52,7 +52,11 @@ func TestCanonicalAgent(t *testing.T) { func TestKnownAgentNamesIncludesVariants(t *testing.T) { names := cli.KnownAgentNames() joined := strings.Join(names, ",") - for _, want := range []string{"claude", "claude-code", "codex"} { + // "openai" is a registry alias (RegisterAlias("openai", "codex") in + // internal/providers/codex/provider.go), not a documented variant like + // claude-code — it must still appear here so a typo'd `agent:` warning + // tells the user it's an accepted value. + for _, want := range []string{"claude", "claude-code", "codex", "openai"} { if !strings.Contains(joined, want) { t.Errorf("KnownAgentNames() = %v, missing %q", names, want) } @@ -153,6 +157,10 @@ func TestResolveAgentFieldWithAgentsList(t *testing.T) { {"agent: still wins over agents[0]", "codex", []string{"claude", "codex"}, "", "codex", false}, {"verb still wins over both", "codex", []string{"claude", "codex"}, "claude", "claude", true}, {"agent: outside agents: warns", "gemini", []string{"claude", "codex"}, "", "gemini", true}, + // Companion: with a verb present, the "not in agents:" warning must not + // fire — see TestResolveAgentFieldOutsideAgentsListVerbWarnsOnce for the + // assertion that only the conflict warning appears (not both). + {"agent: outside agents: with a verb only warns about the conflict", "gemini", []string{"claude", "codex"}, "claude", "claude", true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -174,6 +182,38 @@ func TestResolveAgentFieldWithAgentsList(t *testing.T) { } } +// TestResolveAgentFieldOutsideAgentsListVerbWarnsOnce is a regression test: +// the "not in agents:" warning claims the field "will run as the primary", +// which is only true on the no-verb (`moat run`) path. When a verb is +// present, ResolveAgentField immediately overwrites cfg.Agent with verb a +// few lines later, and RunProvider provisions the verb's own dependencies, +// grants, and network hosts independently of `agents:` — so the field is +// genuinely provisioned without being in the list, and the "not in agents:" +// warning must stay silent. Only the verb-conflict warning should fire. +func TestResolveAgentFieldOutsideAgentsListVerbWarnsOnce(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cfg := &config.Config{Agent: "gemini", Agents: []string{"claude", "codex"}} + cli.ResolveAgentField(cfg, "claude") + + if cfg.Agent != "claude" { + t.Fatalf("cfg.Agent = %q, want %q", cfg.Agent, "claude") + } + out := buf.String() + if got := strings.Count(out, "Warning:"); got != 1 { + t.Errorf("expected exactly 1 warning, got %d: %q", got, out) + } + if strings.Contains(out, "is not in `agents:") { + t.Errorf("the not-in-agents warning must not fire when a verb is present: %q", out) + } + if !strings.Contains(out, "conflicts with") { + t.Errorf("expected the verb-conflict warning: %q", out) + } +} + func TestExpandAgents(t *testing.T) { cfg := &config.Config{Agents: []string{"claude", "codex"}} grants, err := cli.ExpandAgents(cfg) diff --git a/internal/cli/rundefaults.go b/internal/cli/rundefaults.go new file mode 100644 index 00000000..072f700a --- /dev/null +++ b/internal/cli/rundefaults.go @@ -0,0 +1,57 @@ +package cli + +import "github.com/majorcontext/moat/internal/config" + +// ApplyAgentDefaults runs the agents: → grants-merge → AppendDerivedGrants +// sequence shared by `moat run` and `moat wt`, plus the adjacent +// command-from-config defaulting. Both call sites relied on this exact +// sequence duplicated near-verbatim; keeping it in one place means a future +// fix to the merge/precedence logic can't be applied to only one of the two +// entry points. +// +// The ordering it preserves: +// +// 1. ExpandAgents mutates cfg.Dependencies and cfg.Network.Rules directly, +// but returns derived credential grants separately rather than writing +// them into cfg.Grants — see its doc comment for why (cfg.Grants is +// treated as the "explicit" bucket during grant-precedence resolution, +// and a derived grant must never win that precedence as if the user had +// declared it). +// 2. *flagsGrants is defaulted from cfg.Grants + the derived grants ONLY +// when the caller passed no --grant flags (*flagsGrants is empty). This +// is override semantics, not merge semantics — do not change it. +// 3. AppendDerivedGrants writes the derived grants into cfg.Grants AFTER +// step 2 has already read cfg.Grants — never before. cfg.Grants has its +// own direct downstream readers (ShouldSyncCodexLogs, +// ShouldSyncGeminiLogs, buildLocalMCPConfig's grant validation) that +// only ever see cfg.Grants, not *flagsGrants, so running this earlier +// would let a derived grant re-enter step 2 as if the user had declared +// it — resurrecting the bug ExpandAgents' doc comment describes. +// +// command is defaulted from cfg.Command when the caller passed none — +// identical at both call sites and adjacent to the grants defaulting. +func ApplyAgentDefaults(cfg *config.Config, flagsGrants *[]string, command *[]string) error { + derivedGrants, err := ExpandAgents(cfg) + if err != nil { + return err + } + + if cfg == nil { + return nil + } + + if len(*flagsGrants) == 0 { + grants := append([]string{}, cfg.Grants...) + grants = append(grants, derivedGrants...) + if len(grants) > 0 { + *flagsGrants = grants + } + } + AppendDerivedGrants(cfg, derivedGrants) + + if len(*command) == 0 && len(cfg.Command) > 0 { + *command = cfg.Command + } + + return nil +} diff --git a/internal/cli/rundefaults_test.go b/internal/cli/rundefaults_test.go new file mode 100644 index 00000000..68171272 --- /dev/null +++ b/internal/cli/rundefaults_test.go @@ -0,0 +1,103 @@ +package cli_test + +import ( + "slices" + "testing" + + "github.com/majorcontext/moat/internal/cli" + "github.com/majorcontext/moat/internal/config" + + // Registers all credential/agent providers so ExpandAgents (called inside + // ApplyAgentDefaults) sees a populated registry. Same pattern as + // agents_test.go. + _ "github.com/majorcontext/moat/internal/providers" +) + +// TestApplyAgentDefaultsDefaultsGrantsAndCommand covers the no-override path: +// no --grant flags and no explicit command, so both are populated from +// config — including a derived grant from agents:. +func TestApplyAgentDefaultsDefaultsGrantsAndCommand(t *testing.T) { + cfg := &config.Config{ + Agents: []string{"codex"}, + Grants: []string{"github"}, + Command: []string{"npm", "test"}, + } + var flagsGrants, command []string + + if err := cli.ApplyAgentDefaults(cfg, &flagsGrants, &command); err != nil { + t.Fatalf("ApplyAgentDefaults: %v", err) + } + + // codex's derived grant is openai; it must join the explicit "github" + // grant in the defaulted flags list. + for _, want := range []string{"github", "openai"} { + if !slices.Contains(flagsGrants, want) { + t.Errorf("expected flagsGrants to contain %q; got %v", want, flagsGrants) + } + } + // AppendDerivedGrants must have written the derived grant back into + // cfg.Grants too, since it has its own downstream readers + // (ShouldSyncCodexLogs etc.) that never see flagsGrants. + if !slices.Contains(cfg.Grants, "openai") { + t.Errorf("expected cfg.Grants to contain derived grant openai; got %v", cfg.Grants) + } + if got := []string{"npm", "test"}; !slices.Equal(command, got) { + t.Errorf("expected command defaulted to %v; got %v", got, command) + } +} + +// TestApplyAgentDefaultsCompanionExplicitOverridesLeftAlone is the mirror of +// the defaulting test above: when the caller already passed --grant flags or +// an explicit command, neither is overwritten by config — this is override +// semantics, not merge semantics. The derived grant must still land in +// cfg.Grants regardless, since AppendDerivedGrants always runs. +func TestApplyAgentDefaultsCompanionExplicitOverridesLeftAlone(t *testing.T) { + cfg := &config.Config{ + Agents: []string{"codex"}, + Grants: []string{"github"}, + Command: []string{"npm", "test"}, + } + flagsGrants := []string{"aws:s3.read"} + command := []string{"bash"} + + if err := cli.ApplyAgentDefaults(cfg, &flagsGrants, &command); err != nil { + t.Fatalf("ApplyAgentDefaults: %v", err) + } + + if got := []string{"aws:s3.read"}; !slices.Equal(flagsGrants, got) { + t.Errorf("explicit --grant flags must not be overwritten by config; got %v, want %v", flagsGrants, got) + } + if got := []string{"bash"}; !slices.Equal(command, got) { + t.Errorf("explicit command must not be overwritten by config; got %v, want %v", command, got) + } + if !slices.Contains(cfg.Grants, "openai") { + t.Errorf("derived grant must still be written back into cfg.Grants even when flags override; got %v", cfg.Grants) + } +} + +// TestApplyAgentDefaultsNilConfig ensures a nil *config.Config (moat run with +// no moat.yaml present) is handled without panicking and leaves the caller's +// slices untouched. +func TestApplyAgentDefaultsNilConfig(t *testing.T) { + var flagsGrants, command []string + if err := cli.ApplyAgentDefaults(nil, &flagsGrants, &command); err != nil { + t.Fatalf("ApplyAgentDefaults(nil, ...): %v", err) + } + if len(flagsGrants) != 0 { + t.Errorf("expected no grants defaulted from a nil config; got %v", flagsGrants) + } + if len(command) != 0 { + t.Errorf("expected no command defaulted from a nil config; got %v", command) + } +} + +// TestApplyAgentDefaultsPropagatesExpandAgentsError checks that an invalid +// agents: entry surfaces as an error rather than being silently swallowed by +// the extraction. +func TestApplyAgentDefaultsPropagatesExpandAgentsError(t *testing.T) { + cfg := &config.Config{Agents: []string{"not-a-real-agent"}} + var flagsGrants, command []string + if err := cli.ApplyAgentDefaults(cfg, &flagsGrants, &command); err == nil { + t.Fatal("expected an error for an unknown agents: entry, got nil") + } +} diff --git a/internal/provider/registry.go b/internal/provider/registry.go index 0801dfb7..99f32c92 100644 --- a/internal/provider/registry.go +++ b/internal/provider/registry.go @@ -99,6 +99,26 @@ func Agents() []AgentProvider { return result } +// AgentAliases returns the registered alias names (e.g. "openai" for +// "codex") whose canonical provider is an AgentProvider, sorted. Aliases +// that resolve to a non-agent provider (or to nothing) are excluded — an +// alias only belongs in an "accepted agent name" listing if it actually +// names an agent. +func AgentAliases() []string { + mu.RLock() + defer mu.RUnlock() + var result []string + for alias, canonical := range aliases { + if p, ok := providers[canonical]; ok { + if _, ok := p.(AgentProvider); ok { + result = append(result, alias) + } + } + } + sort.Strings(result) + return result +} + // Names returns the names of all registered providers, sorted. func Names() []string { mu.RLock() diff --git a/internal/provider/registry_test.go b/internal/provider/registry_test.go index 247fd9b1..cdf7e39f 100644 --- a/internal/provider/registry_test.go +++ b/internal/provider/registry_test.go @@ -228,6 +228,26 @@ func TestAll(t *testing.T) { } } +func TestAgentAliases(t *testing.T) { + snapP, snapA := snapshotRegistry() + defer restoreRegistry(snapP, snapA) + + Clear() + defer Clear() + + Register(&mockAgentProvider{mockProvider{name: "codex"}}) + Register(&mockProvider{name: "gitlab"}) + + RegisterAlias("openai", "codex") // alias to an agent provider + RegisterAlias("gl", "gitlab") // alias to a non-agent provider + RegisterAlias("ghost", "nobody") // alias to a provider that doesn't exist + + got := AgentAliases() + if len(got) != 1 || got[0] != "openai" { + t.Errorf("AgentAliases() = %v, want [openai]", got) + } +} + func TestAgents(t *testing.T) { snapP, snapA := snapshotRegistry() defer restoreRegistry(snapP, snapA) diff --git a/internal/run/joinable.go b/internal/run/joinable.go index 34506b72..9722b684 100644 --- a/internal/run/joinable.go +++ b/internal/run/joinable.go @@ -8,6 +8,14 @@ import ( // agentCLIDep maps an agent provider name to the dependency that installs its // CLI binary into the container. +// +// This hand-duplicates information each AgentRuntime provider already exposes +// via DefaultDependencies() — there's no marker in that slice identifying +// which entry is the agent's own CLI, so it can't be derived automatically. +// TestAgentCLIDepMatchesAgentRuntimeProviders in joinable_test.go guards +// against drift: it fails if a provider implementing provider.AgentRuntime +// has no entry here (or vice versa). Adding a new AgentRuntime provider? +// Add its CLI dependency name here too, or that test will catch it. var agentCLIDep = map[string]string{ "claude": "claude-code", "codex": "codex-cli", diff --git a/internal/run/joinable_test.go b/internal/run/joinable_test.go index 3f80dab4..716d1190 100644 --- a/internal/run/joinable_test.go +++ b/internal/run/joinable_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/majorcontext/moat/internal/deps" + "github.com/majorcontext/moat/internal/provider" "github.com/majorcontext/moat/internal/storage" ) @@ -67,6 +68,46 @@ func TestComputeJoinableAgents(t *testing.T) { } } +// TestAgentCLIDepMatchesAgentRuntimeProviders guards agentCLIDep against +// drift: it's a hand-maintained map that duplicates information each +// AgentRuntime provider already exposes, so a new provider whose author +// forgets an entry here would silently never be joinable (see the doc +// comment on agentCLIDep). This enumerates the actual registry — via +// provider.Agents(), populated by production init() side effects, not a +// restated hardcoded list — so it can't drift in lockstep with the map it's +// checking. +func TestAgentCLIDepMatchesAgentRuntimeProviders(t *testing.T) { + // pi implements provider.AgentProvider but not provider.AgentRuntime (see + // internal/providers/pi/provider.go): it can't appear in moat.yaml + // `agents:`, but it does track a CLI dependency for join purposes. That's + // an intentional, documented exception, not drift. + exceptions := map[string]bool{"pi": true} + + runtimeAgents := map[string]bool{} + for _, ap := range provider.Agents() { + if _, ok := ap.(provider.AgentRuntime); ok { + runtimeAgents[ap.Name()] = true + } + } + if len(runtimeAgents) == 0 { + t.Fatal("no AgentRuntime providers found in the registry — provider init() side effects didn't run; " + + "is internal/providers still blank-imported from internal/run?") + } + + for name := range runtimeAgents { + if _, ok := agentCLIDep[name]; !ok { + t.Errorf("provider %q implements provider.AgentRuntime but has no agentCLIDep entry — "+ + "moat join will wrongly report it as never provisioned", name) + } + } + for name := range agentCLIDep { + if !runtimeAgents[name] && !exceptions[name] { + t.Errorf("agentCLIDep[%q] has no registered provider.AgentRuntime provider — "+ + "stale entry from a removed/renamed provider?", name) + } + } +} + func TestJoinableAgentsRoundTrip(t *testing.T) { tests := []struct { name string From cffec48a8518106ba6dfb5aa26b7516f4e2a8f33 Mon Sep 17 00:00:00 2001 From: Dan Pupius Date: Fri, 14 Aug 2026 16:17:13 +0000 Subject: [PATCH 33/33] fix(cli): canonicalize agent: and validate it under --dry-run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four findings from a clean-room review of the multi-agent join work. KnownAgentNames advertises the provider aliases (openai, google) as valid `agent:` values and ValidateAgent accepts them, but ResolveAgentField never normalized them. Every downstream consumer matches with strings.HasPrefix(cfg.Agent, "") — isAIAgent's container-memory default, agentImpliedDependencies, the language_servers gate, copilot init, pi staging — and "openai" prefix-matches none of them. So an alias moat itself advertises passed validation and then silently switched those defaults off, which is the degradation this code exists to prevent. `agents: [openai]` reached the same place via the Agents[0] backfill. ResolveAgentField now canonicalizes on the way out; ValidateAgent stays pure validation. resolveProviderAgentField (and its moat run / moat wt equivalents) sat after the dry-run return, so --dry-run — the flag people reach for to check a moat.yaml before committing to a run — was the one path that never surfaced the bad-`agent:` warning. Moved above the return at all three call sites. The `doc:` tag on `agents:` claimed the first entry is "the foreground agent" for moat run, contradicting both doc pages and manager_create.go, which defaults to /bin/bash when no command is configured. That tag is rendered into moat init's LLM prompt via GenerateSchemaReference, i.e. the same docs-as-bad-value-generator mechanism this branch fixes for `agent:`. The Agent doc tag and KnownAgentNames also listed different "valid" sets; a new drift guard asserts every accepted name appears in the tag. --- CHANGELOG.md | 2 + cmd/moat/cli/run.go | 10 +- cmd/moat/cli/wt.go | 10 +- docs/content/reference/02-moat-yaml.md | 5 +- internal/cli/agents.go | 28 ++++ internal/cli/agents_test.go | 172 +++++++++++++++++++++++++ internal/cli/provider.go | 11 +- internal/config/config.go | 4 +- 8 files changed, 227 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc1cd517..be089626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,8 @@ Separately, port-exposing agents no longer disappear from the routing proxy's di - Fix `moat join ` failing when several runs share a name — previously, any project setting `name:` in `moat.yaml` gave every run the same name, and a second concurrent run made join error instead of offering a choice. ([#454](https://github.com/majorcontext/moat/pull/454)) - Fix `moat join`'s extra positional arguments being silently ignored — `moat join ` used to accept and discard ``; it is now a usage error. `moat join` now takes one or two positional arguments (`[run] agent`), not a minimum of two. ([#454](https://github.com/majorcontext/moat/pull/454)) - Fix the `agent:` reference documentation, which described the field as a free-form identifier defaulting to `name`. It is a fixed set of agent names and defaults to the command you ran. ([#454](https://github.com/majorcontext/moat/pull/454)) +- Fix `agent: openai` and `agent: google` being accepted and then ignored — the provider aliases passed validation but were never normalized to `codex` / `gemini`, so the agent-specific defaults (container memory, implied dependencies, language-server support) all silently switched off. `agent:` is now normalized to the provider name, including when it is backfilled from the first entry of `agents:`. ([#454](https://github.com/majorcontext/moat/pull/454)) +- Fix `--dry-run` skipping `agent:` validation — a `moat.yaml` with an unrecognized `agent:` previewed cleanly and only warned once you started a real run, which is backwards for the flag people use to check their config. ([#454](https://github.com/majorcontext/moat/pull/454)) ### Breaking diff --git a/cmd/moat/cli/run.go b/cmd/moat/cli/run.go index 86948262..6c8e74c7 100644 --- a/cmd/moat/cli/run.go +++ b/cmd/moat/cli/run.go @@ -149,6 +149,12 @@ func runAgent(cmd *cobra.Command, args []string) error { } } + // moat run has no verb, so a valid agent: is preserved and an invalid one + // warns and is cleared. This runs BEFORE the dry-run return below: --dry-run + // is what someone reaches for to check a moat.yaml, so it must surface the + // same agent: warnings a real run would. + intcli.ResolveAgentField(cfg, "") + log.Debug("preparing run", "name", runFlags.Name, "workspace", absPath, @@ -167,10 +173,6 @@ func runAgent(cmd *cobra.Command, args []string) error { ctx := context.Background() - // moat run has no verb, so a valid agent: is preserved and an invalid one - // warns and is cleared. - intcli.ResolveAgentField(cfg, "") - opts := ExecOptions{ Flags: runFlags, Workspace: absPath, diff --git a/cmd/moat/cli/wt.go b/cmd/moat/cli/wt.go index 77f33204..debfff40 100644 --- a/cmd/moat/cli/wt.go +++ b/cmd/moat/cli/wt.go @@ -163,6 +163,12 @@ func runWorktree(cmd *cobra.Command, args []string) error { // Determine interactive mode from config interactive := cfg != nil && cfg.Interactive + // moat wt has no verb, so a valid agent: is preserved and an invalid one + // warns and is cleared (same pattern as moat run). This runs BEFORE the + // dry-run return below so --dry-run surfaces the same agent: warnings a + // real run would. + intcli.ResolveAgentField(cfg, "") + log.Debug("starting worktree run", "branch", branch, "workspace", result.WorkspacePath, @@ -181,10 +187,6 @@ func runWorktree(cmd *cobra.Command, args []string) error { ctx := cmd.Context() - // moat wt has no verb, so a valid agent: is preserved and an invalid one - // warns and is cleared (same pattern as moat run). - intcli.ResolveAgentField(cfg, "") - opts := intcli.ExecOptions{ Flags: wtFlags, Workspace: result.WorkspacePath, diff --git a/docs/content/reference/02-moat-yaml.md b/docs/content/reference/02-moat-yaml.md index 1a0eff87..172dc1b8 100644 --- a/docs/content/reference/02-moat-yaml.md +++ b/docs/content/reference/02-moat-yaml.md @@ -209,7 +209,10 @@ agent: claude ``` - Type: `string` -- Allowed values: `claude`, `claude-code`, `codex`, `copilot`, `gemini`, `pi` +- Allowed values: `claude`, `claude-code`, `codex`, `copilot`, `gemini`, `pi`. + The provider aliases `openai` (codex) and `google` (gemini) are accepted too. + Whatever you write is normalized to the provider name, so `agent: openai` is + recorded as `codex`. - Default: the provider name of the command you ran (`moat claude` → `claude`). For `moat run`, `agent:` is used as set; if it is unset and `agents:` is present, it falls back to the first entry in `agents:`; otherwise it stays diff --git a/internal/cli/agents.go b/internal/cli/agents.go index ec98712d..72e46208 100644 --- a/internal/cli/agents.go +++ b/internal/cli/agents.go @@ -89,10 +89,15 @@ func ValidateAgent(cfg *config.Config) { // One rule: the CLI verb always names the primary when there is one; otherwise // `agent:` names it; if neither is set, `moat run` falls back to Agents[0]. // verb is "" for `moat run`. +// +// Whichever path wins, the result is canonicalized on the way out — see +// canonicalizeAgentField. func ResolveAgentField(cfg *config.Config, verb string) { if cfg == nil { return } + defer canonicalizeAgentField(cfg) + ValidateAgent(cfg) // This warning only holds on the no-verb (`moat run`) path: when a verb is @@ -121,6 +126,29 @@ func ResolveAgentField(cfg *config.Config, verb string) { } } +// canonicalizeAgentField rewrites cfg.Agent to its registered provider name, so +// registry aliases (openai -> codex, google -> gemini) and documented variants +// (claude-code -> claude) all collapse to one spelling. +// +// Every downstream consumer of cfg.Agent matches it with +// strings.HasPrefix(cfg.Agent, ""): isAIAgent's container-memory +// default, agentImpliedDependencies, the language_servers gate, copilot init, +// and pi staging — all in internal/run/manager_create.go. "claude-code" +// satisfies those by prefix; "openai" and "google" do not. Without this, +// an alias that ValidateAgent accepts (and that KnownAgentNames advertises as +// valid) would sail through validation and then silently switch those defaults +// off — the exact degradation this file exists to prevent. `agents: [openai]` +// reaches the same place via the Agents[0] backfill above. +// +// A value that does not resolve is left alone: ValidateAgent has already +// cleared unknown moat.yaml values, and the verb path assigns a name the +// provider registry owns. +func canonicalizeAgentField(cfg *config.Config) { + if canonical := CanonicalAgent(cfg.Agent); canonical != "" { + cfg.Agent = canonical + } +} + // agentsListContains reports whether agents contains agent, comparing by // canonical name so documented variants (claude-code) and registry aliases // (openai) match their canonical entry. diff --git a/internal/cli/agents_test.go b/internal/cli/agents_test.go index a48b268c..5408077f 100644 --- a/internal/cli/agents_test.go +++ b/internal/cli/agents_test.go @@ -12,10 +12,15 @@ package cli_test import ( "bytes" + "os" + "path/filepath" + "reflect" "slices" "strings" "testing" + "github.com/spf13/cobra" + "github.com/majorcontext/moat/internal/cli" "github.com/majorcontext/moat/internal/config" "github.com/majorcontext/moat/internal/ui" @@ -384,3 +389,170 @@ func TestAgentFieldReachesDegradationSites(t *testing.T) { t.Errorf("cfg.Agent = %q, want empty so the HasPrefix sites stay off", bare.Agent) } } + +// TestResolveAgentFieldCanonicalizes covers the alias half of the degradation +// this file exists to prevent. KnownAgentNames advertises the registry aliases +// (openai, google) as valid `agent:` values and ValidateAgent accepts them, so +// they must also reach cfg.Agent in the spelling the downstream +// strings.HasPrefix consumers in manager_create.go match against. "openai" +// prefix-matches none of claude/codex/copilot/gemini/pi; "codex" does. +func TestResolveAgentFieldCanonicalizes(t *testing.T) { + tests := []struct { + name string + agent string + agents []string + verb string + want string + }{ + {"alias in agent: resolves to the provider name", "openai", nil, "", "codex"}, + {"google resolves to gemini", "google", nil, "", "gemini"}, + {"alias via the agents[0] backfill", "", []string{"openai"}, "", "codex"}, + {"documented variant collapses too", "claude-code", nil, "", "claude"}, + {"variant in agents[0] collapses", "", []string{"claude-code"}, "", "claude"}, + // Companion: the verb path already assigns a canonical provider name, + // so canonicalization must leave it exactly as it was. + {"verb value is already canonical", "openai", nil, "claude", "claude"}, + // Companion: an unknown value is still cleared, not "canonicalized" + // into some nearby agent. + {"unknown stays cleared", "vibrant-code", nil, "", ""}, + // Companion: no agent info at all stays empty. + {"empty stays empty", "", nil, "", ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cfg := &config.Config{Agent: tt.agent, Agents: tt.agents} + cli.ResolveAgentField(cfg, tt.verb) + + if cfg.Agent != tt.want { + t.Errorf("cfg.Agent = %q, want %q", cfg.Agent, tt.want) + } + }) + } +} + +// TestAliasAgentReachesDegradationSites is the companion to +// TestAgentFieldReachesDegradationSites: that one proves a hallucinated value +// is repaired by the verb, this one proves an alias moat itself advertises as +// valid ends up satisfying the same HasPrefix gates. Before canonicalization +// both `agent: openai` and `agents: [openai]` passed validation and then +// silently switched off the memory limit, implied deps, and language-server +// support. +func TestAliasAgentReachesDegradationSites(t *testing.T) { + canonicalPrefixes := []string{"claude", "codex", "copilot", "gemini", "pi"} + satisfiesPrefixGate := func(agent string) bool { + for _, p := range canonicalPrefixes { + if strings.HasPrefix(agent, p) { + return true + } + } + return false + } + + for _, alias := range []string{"openai", "google"} { + t.Run("agent field "+alias, func(t *testing.T) { + cfg := &config.Config{Agent: alias} + cli.ResolveAgentField(cfg, "") + if !satisfiesPrefixGate(cfg.Agent) { + t.Errorf("agent: %s resolved to %q, which matches none of the "+ + "HasPrefix gates in manager_create.go", alias, cfg.Agent) + } + }) + t.Run("agents list "+alias, func(t *testing.T) { + cfg := &config.Config{Agents: []string{alias}} + if _, err := cli.ExpandAgents(cfg); err != nil { + t.Fatalf("ExpandAgents: %v", err) + } + cli.ResolveAgentField(cfg, "") + if !satisfiesPrefixGate(cfg.Agent) { + t.Errorf("agents: [%s] backfilled %q, which matches none of the "+ + "HasPrefix gates in manager_create.go", alias, cfg.Agent) + } + }) + } +} + +// TestAgentDocTagListsEveryKnownAgentName guards the two "valid values" lists +// against drift. The `doc:` tag on config.Config.Agent is rendered into moat +// init's LLM prompt (quickstart.GenerateSchemaReference), while +// KnownAgentNames() produces the list in the runtime warning — a value named by +// one and not the other is either an unadvertised accepted value or, worse, a +// prompt telling the model to write something moat rejects. +func TestAgentDocTagListsEveryKnownAgentName(t *testing.T) { + field, ok := reflect.TypeOf(config.Config{}).FieldByName("Agent") + if !ok { + t.Fatal("config.Config has no Agent field") + } + doc := field.Tag.Get("doc") + if doc == "" { + t.Fatal("config.Config.Agent has no doc tag; moat init's prompt would document it as a bare string") + } + for _, name := range cli.KnownAgentNames() { + if !strings.Contains(doc, name) { + t.Errorf("KnownAgentNames() accepts %q but the Agent doc tag never mentions it: %q", name, doc) + } + } +} + +// runProviderDryRun drives RunProvider against a workspace containing the given +// moat.yaml with DryRun set, and returns everything written to the ui writer. +// The verb is "codex" so ResolveAgentField takes the real provider path. +func runProviderDryRun(t *testing.T, moatYAML string) string { + t.Helper() + + ws := t.TempDir() + if err := os.WriteFile(filepath.Join(ws, "moat.yaml"), []byte(moatYAML), 0o600); err != nil { + t.Fatalf("writing moat.yaml: %v", err) + } + + oldDryRun := cli.DryRun + cli.DryRun = true + t.Cleanup(func() { cli.DryRun = oldDryRun }) + + var buf bytes.Buffer + orig := ui.Writer() + ui.SetWriter(&buf) + t.Cleanup(func() { ui.SetWriter(orig) }) + + cmd := &cobra.Command{ + Use: "codex", + SilenceUsage: true, + SilenceErrors: true, + RunE: func(c *cobra.Command, a []string) error { + return cli.RunProvider(c, a, cli.ProviderRunConfig{ + Name: "codex", + Flags: &cli.ExecFlags{}, + BuildCommand: func(_, _ string) ([]string, error) { return []string{"noop"}, nil }, + }) + }, + } + cmd.SetArgs([]string{ws}) + cmd.SetOut(&bytes.Buffer{}) + if err := cmd.Execute(); err != nil { + t.Fatalf("Execute: %v", err) + } + return buf.String() +} + +// TestDryRunStillValidatesAgentField is a regression test for the ordering of +// resolveProviderAgentField relative to the DryRun return. --dry-run is what +// someone reaches for to check a moat.yaml before committing to a run, so it +// is the worst place to skip the very warning that tells them `agent:` is +// wrong. The call used to sit after the dry-run return and never fired. +func TestDryRunStillValidatesAgentField(t *testing.T) { + out := runProviderDryRun(t, "agent: vibrant-code\n") + if !strings.Contains(out, "not a known agent") { + t.Errorf("dry run should warn about an unknown agent: value; got %q", out) + } + + // Companion: a valid agent: under the same dry-run path stays silent, so + // the test above is detecting the bad value rather than a warning that + // fires unconditionally. + if out := runProviderDryRun(t, "agent: codex\n"); out != "" { + t.Errorf("dry run with a valid agent: should warn about nothing; got %q", out) + } +} diff --git a/internal/cli/provider.go b/internal/cli/provider.go index 5c875c64..14a6145d 100644 --- a/internal/cli/provider.go +++ b/internal/cli/provider.go @@ -219,6 +219,13 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error return envErr } + // The verb the user typed always names the agent. ValidateAgent runs inside + // so an unknown moat.yaml value warns once and is discarded. This runs + // BEFORE the dry-run return below: --dry-run is what someone reaches for to + // check a moat.yaml, so it must surface the same agent: warnings a real run + // would. + resolveProviderAgentField(rc.Name, cfg, agentBeforeConfigure) + log.Debug(fmt.Sprintf("starting %s", rc.Name), "workspace", absPath, "grants", grants, @@ -241,10 +248,6 @@ func RunProvider(cmd *cobra.Command, args []string, rc ProviderRunConfig) error ctx := context.Background() - // The verb the user typed always names the agent. ValidateAgent runs inside - // so an unknown moat.yaml value warns once and is discarded. - resolveProviderAgentField(rc.Name, cfg, agentBeforeConfigure) - opts := ExecOptions{ Flags: *rc.Flags, Workspace: absPath, diff --git a/internal/config/config.go b/internal/config/config.go index d0fab420..04e17104 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -32,8 +32,8 @@ var imageRefRe = regexp.MustCompile(`^[a-zA-Z0-9][a-zA-Z0-9._\-/:]*(@sha256:[a-f // Config represents a moat.yaml manifest. type Config struct { Name string `yaml:"name,omitempty"` - Agent string `yaml:"agent" doc:"one of: claude, claude-code, codex, copilot, gemini, pi. Omit unless the project pins a specific agent."` - Agents []string `yaml:"agents,omitempty" doc:"agents to provision into the container, e.g. [claude, codex]. Same allowed values as agent. For moat run, the first entry is the foreground agent."` + Agent string `yaml:"agent" doc:"one of: claude, claude-code, codex, copilot, gemini, pi (the provider aliases openai and google are accepted too). Omit unless the project pins a specific agent."` + Agents []string `yaml:"agents,omitempty" doc:"agents to provision into the container so moat join can launch any of them, e.g. [claude, codex]. Same allowed values as agent, except pi. The first entry only backfills agent:; it does not change what the run executes."` Version string `yaml:"version,omitempty"` Dependencies []string `yaml:"dependencies,omitempty"` Grants []string `yaml:"grants,omitempty"`