diff --git a/README.md b/README.md index ac8cfe6..0f5cbea 100644 --- a/README.md +++ b/README.md @@ -581,10 +581,10 @@ CLI flags override config file values. Config file overrides defaults. | `--reconcile-interval` | `30s` | How often to check if reviewing tasks are merged | | `-d` / `--detach` | `false` | Run in background | -Daemon listen URLs are derived automatically from the project name. Each project gets an isolated loopback port so multiple daemons can run side-by-side. Custom listen addresses are configured via `listen_addr`, not per-command flags. +Daemon listen URLs depend on spawn policy by default. In `manual` mode, the daemon uses the single global loopback URL `http://127.0.0.1:7070` unless `listen_addr` is set explicitly. In `auto` mode, daemon listen URLs are derived automatically from the project name so multiple auto daemons can run side-by-side. Custom listen addresses are configured via `listen_addr`, not per-command flags. `--project` is required when `--spawn-policy=auto`, and optional when `--spawn-policy=manual`. -Manual mode without a project uses the global default daemon URL. Starting a second daemon on the same listen address fails fast. +Manual mode ignores project for default daemon startup addressing and uses the global default daemon URL unless `listen_addr` is set. Client commands still treat an explicit `--project` as an intentional project-scoped daemon target, so `af status --project myapp` and similar commands continue to reach auto daemons without requiring a config file. Starting a second daemon on the same listen address fails fast. ## CLI Reference diff --git a/cmd/af/cmd/daemon.go b/cmd/af/cmd/daemon.go index 084d9e4..4f8822e 100644 --- a/cmd/af/cmd/daemon.go +++ b/cmd/af/cmd/daemon.go @@ -179,6 +179,8 @@ func init() { f.String("config", "", "Config file path (default: .aetherflow.yaml)") daemonStopCmd.Flags().Bool("force", false, "Stop even when the daemon reports active sessions") + daemonCmd.Flags().String("spawn-policy", "", "Daemon spawn policy hint for endpoint resolution (auto or manual)") + daemonStopCmd.Flags().String("spawn-policy", "", "Daemon spawn policy hint for endpoint resolution (auto or manual)") } func printDaemonNotRunning(w io.Writer) { diff --git a/cmd/af/cmd/root.go b/cmd/af/cmd/root.go index dad57bf..49a5200 100644 --- a/cmd/af/cmd/root.go +++ b/cmd/af/cmd/root.go @@ -35,7 +35,7 @@ func Execute() error { func init() { rootCmd.PersistentFlags().StringP("config", "c", "", "config file (default is $HOME/.aetherflow.yaml)") - rootCmd.PersistentFlags().StringP("project", "p", "", "Project name (derives daemon URL, overrides config file)") + rootCmd.PersistentFlags().StringP("project", "p", "", "Project name (targets a project-scoped daemon URL when set, overrides config file)") rootCmd.PersistentFlags().Bool("no-color", false, "Disable colored output") // Wire --no-color to the term package. OnInitialize runs before any @@ -48,39 +48,48 @@ func init() { }) } -// resolveDaemonURL determines the daemon URL from the CLI flag, -// config file, or default convention. Priority: -// 1. Explicit --project flag -> project-scoped daemon URL -// 2. Config listen_addr -> canonical daemon URL -// 3. Config project -> project-scoped daemon URL -// 4. DefaultDaemonURL fallback +// resolveDaemonURL determines the daemon URL from the CLI flags, +// config file, and daemon mode. Priority: +// 1. Explicit --project -> project-scoped daemon URL +// 2. Explicit/configured listen_addr -> canonical daemon URL +// 3. Auto mode + configured project -> project-scoped daemon URL +// 4. Manual mode default -> DefaultDaemonURL func resolveDaemonURL(cmd *cobra.Command) string { - if cmd.Flags().Changed("project") { - p, _ := cmd.Flags().GetString("project") - return protocol.DaemonURLFor(p) - } - configPath, _ := cmd.Flags().GetString("config") if configPath == "" { configPath = ".aetherflow.yaml" } var cfg daemon.Config - if err := daemon.LoadConfigFile(configPath, &cfg); err == nil { - cfg.ApplyDefaults() - if cfg.ListenAddr != "" { - daemonURL, err := protocol.DaemonURLFromListenAddr(cfg.ListenAddr) - if err == nil { - return daemonURL - } - fmt.Fprintf(os.Stderr, "warning: invalid listen_addr %q in %s: %v (using default daemon URL)\n", cfg.ListenAddr, configPath, err) - } - if cfg.Project != "" { - return protocol.DaemonURLFor(cfg.Project) - } - } else if !os.IsNotExist(err) { + if err := daemon.LoadConfigFile(configPath, &cfg); err != nil && !os.IsNotExist(err) { fmt.Fprintf(os.Stderr, "warning: failed to parse %s: %v (using default daemon URL)\n", configPath, err) } + explicitProject := "" + if cmd.Flags().Changed("project") { + explicitProject, _ = cmd.Flags().GetString("project") + cfg.Project = explicitProject + } + if cmd.Flags().Lookup("spawn-policy") != nil && cmd.Flags().Changed("spawn-policy") { + policy, _ := cmd.Flags().GetString("spawn-policy") + cfg.SpawnPolicy = daemon.SpawnPolicy(policy) + } + + normalizedPolicy := cfg.SpawnPolicy.Normalized() + if explicitProject != "" { + return protocol.DaemonURLFor(explicitProject) + } + + if listenAddr := cfg.ListenAddr; listenAddr != "" { + daemonURL, err := protocol.DaemonURLFromListenAddr(cfg.ListenAddr) + if err == nil { + return daemonURL + } + fmt.Fprintf(os.Stderr, "warning: invalid listen_addr %q in %s: %v (using default daemon URL)\n", listenAddr, configPath, err) + } + if normalizedPolicy == daemon.SpawnPolicyAuto && cfg.Project != "" { + return protocol.DaemonURLFor(cfg.Project) + } + return protocol.DefaultDaemonURL } diff --git a/cmd/af/cmd/root_test.go b/cmd/af/cmd/root_test.go index 750d014..9349476 100644 --- a/cmd/af/cmd/root_test.go +++ b/cmd/af/cmd/root_test.go @@ -5,25 +5,95 @@ import ( "path/filepath" "testing" + "github.com/baiirun/aetherflow/internal/protocol" "github.com/spf13/cobra" ) func TestResolveDaemonURLUsesListenAddrFromConfig(t *testing.T) { - dir := t.TempDir() - configPath := filepath.Join(dir, ".aetherflow.yaml") - if err := os.WriteFile(configPath, []byte("listen_addr: :7099\nproject: ignored\n"), 0644); err != nil { + configPath := writeResolveConfig(t, "listen_addr: :7099\nproject: ignored\n") + + cmd := newResolveTestCommand(t, configPath) + + got := resolveDaemonURL(cmd) + if got != "http://127.0.0.1:7099" { + t.Fatalf("resolveDaemonURL = %q, want %q", got, "http://127.0.0.1:7099") + } +} + +func TestResolveDaemonURLIgnoresConfigProjectInManualMode(t *testing.T) { + configPath := writeResolveConfig(t, "project: from-file\nspawn_policy: manual\n") + + cmd := newResolveTestCommand(t, configPath) + + got := resolveDaemonURL(cmd) + if got != protocol.DefaultDaemonURL { + t.Fatalf("resolveDaemonURL = %q, want %q", got, protocol.DefaultDaemonURL) + } +} + +func TestResolveDaemonURLUsesConfigProjectInAutoMode(t *testing.T) { + configPath := writeResolveConfig(t, "project: from-file\nspawn_policy: auto\n") + + cmd := newResolveTestCommand(t, configPath) + + got := resolveDaemonURL(cmd) + want := protocol.DaemonURLFor("from-file") + if got != want { + t.Fatalf("resolveDaemonURL = %q, want %q", got, want) + } +} + +func TestResolveDaemonURLUsesExplicitProjectInManualMode(t *testing.T) { + cmd := newResolveTestCommand(t, "") + if err := cmd.Flags().Set("project", "manual-target"); err != nil { t.Fatal(err) } + got := resolveDaemonURL(cmd) + want := protocol.DaemonURLFor("manual-target") + if got != want { + t.Fatalf("resolveDaemonURL = %q, want %q", got, want) + } +} + +func TestResolveDaemonURLUsesExplicitProjectInAutoMode(t *testing.T) { + cmd := newResolveTestCommand(t, "") + if err := cmd.Flags().Set("project", "auto-target"); err != nil { + t.Fatal(err) + } + if err := cmd.Flags().Set("spawn-policy", "auto"); err != nil { + t.Fatal(err) + } + + got := resolveDaemonURL(cmd) + want := protocol.DaemonURLFor("auto-target") + if got != want { + t.Fatalf("resolveDaemonURL = %q, want %q", got, want) + } +} + +func newResolveTestCommand(t *testing.T, configPath string) *cobra.Command { + t.Helper() + cmd := &cobra.Command{} cmd.Flags().String("config", "", "") cmd.Flags().String("project", "", "") - if err := cmd.Flags().Set("config", configPath); err != nil { - t.Fatal(err) + cmd.Flags().String("spawn-policy", "", "") + if configPath != "" { + if err := cmd.Flags().Set("config", configPath); err != nil { + t.Fatal(err) + } } + return cmd +} - got := resolveDaemonURL(cmd) - if got != "http://127.0.0.1:7099" { - t.Fatalf("resolveDaemonURL = %q, want %q", got, "http://127.0.0.1:7099") +func writeResolveConfig(t *testing.T, contents string) string { + t.Helper() + + dir := t.TempDir() + configPath := filepath.Join(dir, ".aetherflow.yaml") + if err := os.WriteFile(configPath, []byte(contents), 0644); err != nil { + t.Fatal(err) } + return configPath } diff --git a/docs/solutions/handoffs/manual-daemon-url-resolution-20260314.md b/docs/solutions/handoffs/manual-daemon-url-resolution-20260314.md new file mode 100644 index 0000000..30d7da5 --- /dev/null +++ b/docs/solutions/handoffs/manual-daemon-url-resolution-20260314.md @@ -0,0 +1,66 @@ +# Manual Daemon URL Resolution + +**Date**: 2026-03-14 +**Task**: ts-27d3dd + +## Context + +This ticket changes the daemon-addressing contract so manual mode no longer depends on project-derived URLs by default. The requirement from planning was: + +- manual mode should be global-by-default because sessions are created with `af spawn` +- auto mode should remain project-scoped +- explicit `listen_addr` must continue to win +- explicit `--project` on client commands must keep working as an intentional project-scoped target override + +This work unblocks the macOS follow-up ticket that will point the globally installed app at the default manual daemon URL. + +## What Was Done + +1. Updated daemon config defaults in `internal/daemon/config.go` + - manual mode now defaults `listen_addr` to `protocol.DefaultDaemonURL` + - auto mode still derives `listen_addr` from `protocol.DaemonURLFor(project)` + +2. Updated CLI daemon resolution in `cmd/af/cmd/root.go` + - explicit `--project` now always routes to the project-scoped daemon URL + - explicit or configured `listen_addr` still wins over derived defaults + - config-driven auto mode still routes by project + - manual mode without explicit targeting falls back to the global daemon URL + +3. Added CLI hint flags in `cmd/af/cmd/daemon.go` + - `af daemon` and `af daemon stop` now accept `--spawn-policy` as a resolver hint for lifecycle commands + +4. Expanded regression coverage + - `cmd/af/cmd/root_test.go` covers manual default, auto config routing, explicit `--project`, and `listen_addr` precedence + - `internal/daemon/config_test.go` covers manual-vs-auto default `listen_addr` + +5. Updated `README.md` + - documents the split between global manual defaults and project-scoped auto defaults + - documents that explicit `--project` still targets project-scoped daemons for client commands + +## What Was Tried That Didn't Work + +The first resolver change made `--project` conditional on spawn policy resolving to `auto`. That looked consistent with the new manual default, but it broke operator flows like `af status --project myapp` and `af logs --project myapp` when no config file was present. The fix was to separate explicit client targeting from default manual-mode addressing. + +## Key Decisions + +- Kept manual mode global by default instead of adding another project requirement, because manual sessions are created via `af spawn` +- Preserved explicit `--project` as a transport override so existing project-targeted CLI flows do not regress +- Kept `listen_addr` as the highest-priority explicit override to preserve current escape hatches and config compatibility + +## Files Modified + +- `internal/daemon/config.go` +- `internal/daemon/config_test.go` +- `cmd/af/cmd/root.go` +- `cmd/af/cmd/root_test.go` +- `cmd/af/cmd/daemon.go` +- `README.md` + +## Verification + +- `go test ./...` +- `git diff --check` + +## Next Work + +The next ticket is `ts-2df50e`, which should make the macOS Control Center default to the global manual daemon target instead of requiring project-derived endpoint setup. diff --git a/docs/solutions/learnings.md b/docs/solutions/learnings.md new file mode 100644 index 0000000..5d38bea --- /dev/null +++ b/docs/solutions/learnings.md @@ -0,0 +1,3 @@ +### daemon-routing: explicit `--project` is still a transport override + +Manual mode can be global by default without making `--project` inert. In this CLI, operators use `--project` on non-start commands as the only ad hoc way to reach a project-scoped daemon. Resolver changes need to preserve that explicit routing path even if the default manual endpoint becomes global. diff --git a/docs/solutions/logic-errors/manual-daemon-url-resolution-20260314.md b/docs/solutions/logic-errors/manual-daemon-url-resolution-20260314.md new file mode 100644 index 0000000..0710706 --- /dev/null +++ b/docs/solutions/logic-errors/manual-daemon-url-resolution-20260314.md @@ -0,0 +1,66 @@ +--- +module: cli +date: 2026-03-14 +problem_type: logic_error +component: daemon-addressing +symptoms: + - "manual daemons still resolve to project-hashed URLs even though manual mode does not require a project" + - "the macOS app and CLI disagree about which daemon endpoint to use unless callers precompute a project-derived URL" + - "changing manual mode to global can accidentally break explicit --project targeting on client commands" +root_cause: daemon endpoint resolution conflated default manual-mode addressing with explicit project-targeted client routing +resolution_type: code_fix +severity: medium +tags: + - daemon + - cli + - macos + - endpoint-resolution + - manual-mode + - auto-mode + - project-routing +--- + +# Manual Daemon URL Resolution + +## Problem + +The daemon contract treated manual mode as project-optional in config and behavior, but still derived the endpoint from project identity. That forced callers to know a project-scoped URL even when manual mode only needed a single global daemon that serves `af spawn` requests. + +## Symptoms + +- `af daemon start --spawn-policy manual` still defaulted to a project-hashed URL when a project was present +- a globally installed macOS app had to know repo-local project details or an explicit `AETHERFLOW_DAEMON_URL` +- a naive resolver change risked breaking existing flows like `af status --project myapp`, which operators use to intentionally target project-scoped auto daemons + +## What Didn't Work + +The first resolver pass treated `--project` as meaningful only when spawn policy resolved to `auto`. That preserved the new manual default, but it regressed client commands that relied on `--project` as the only explicit routing hint. + +## Solution + +Split the contract into two separate decisions: + +1. **Daemon startup defaults** + - Manual mode defaults to the global daemon URL: `protocol.DefaultDaemonURL` + - Auto mode keeps project-scoped defaults via `protocol.DaemonURLFor(project)` + - Explicit `listen_addr` still wins + +2. **Client-side explicit routing** + - Explicit `--project` remains an intentional override for client commands + - Config-driven auto mode still routes to the project-scoped URL + - Manual mode without explicit targeting falls back to the global daemon URL + +## Why This Works + +This preserves the intended manual-mode architecture without breaking the established operator contract for project-scoped inspection commands. The result is: + +- zero-config manual daemon startup resolves to one global endpoint +- auto daemons stay isolated per project +- operators can still reach a specific project-scoped daemon with `--project` +- future macOS work can default to the global manual endpoint without requiring precomputed URLs + +## Prevention + +- Treat **default addressing** and **explicit operator targeting** as separate concerns in endpoint resolvers +- When changing a shared resolver, test both daemon lifecycle commands and read-only client commands that reuse the same routing path +- Keep the routing priority order explicit: `--project` override, explicit `listen_addr`, config-driven auto routing, manual default diff --git a/internal/daemon/config.go b/internal/daemon/config.go index 9e78b3f..46ecc99 100644 --- a/internal/daemon/config.go +++ b/internal/daemon/config.go @@ -150,10 +150,17 @@ type Config struct { Logger *slog.Logger `yaml:"-"` } +func (c Config) defaultDaemonURL() string { + if c.SpawnPolicy.Normalized() == SpawnPolicyAuto { + return protocol.DaemonURLFor(c.Project) + } + return protocol.DefaultDaemonURL +} + // ApplyDefaults fills in zero-valued fields with sensible defaults. func (c *Config) ApplyDefaults() { if c.ListenAddr == "" { - c.ListenAddr = listenAddrFromURL(protocol.DaemonURLFor(c.Project)) + c.ListenAddr = listenAddrFromURL(c.defaultDaemonURL()) } if c.PollInterval == 0 { c.PollInterval = DefaultPollInterval diff --git a/internal/daemon/config_test.go b/internal/daemon/config_test.go index d722c78..975c648 100644 --- a/internal/daemon/config_test.go +++ b/internal/daemon/config_test.go @@ -57,7 +57,7 @@ func TestConfigApplyDefaults(t *testing.T) { } func TestConfigApplyDefaultsWithProject(t *testing.T) { - cfg := Config{Project: "myproject"} + cfg := Config{Project: "myproject", SpawnPolicy: SpawnPolicyAuto} cfg.ApplyDefaults() want := listenAddrFromURL(protocol.DaemonURLFor("myproject")) @@ -66,6 +66,16 @@ func TestConfigApplyDefaultsWithProject(t *testing.T) { } } +func TestConfigApplyDefaultsManualWithProjectUsesGlobalListenAddr(t *testing.T) { + cfg := Config{Project: "myproject", SpawnPolicy: SpawnPolicyManual} + cfg.ApplyDefaults() + + want := listenAddrFromURL(protocol.DefaultDaemonURL) + if cfg.ListenAddr != want { + t.Errorf("ListenAddr = %q, want %q (manual mode should ignore project for default listen addr)", cfg.ListenAddr, want) + } +} + func TestConfigApplyDefaultsPreservesExisting(t *testing.T) { cfg := Config{ ListenAddr: "127.0.0.1:9999",