diff --git a/cmd/agentsview/cli.go b/cmd/agentsview/cli.go index 6088176e..5d3d5237 100644 --- a/cmd/agentsview/cli.go +++ b/cmd/agentsview/cli.go @@ -100,8 +100,18 @@ func newServeCommand() *cobra.Command { func newSyncCommand() *cobra.Command { var cfg SyncConfig cmd := &cobra.Command{ - Use: "sync", - Short: "Sync session data without serving", + Use: "sync", + Short: "Sync session data without serving", + Long: "Sync session data into the local database without starting the\n" + + "HTTP server.\n\n" + + "With no --host, sync runs the local sync and then fans out to\n" + + "every host listed in the [[remote_hosts]] array in config.toml,\n" + + "syncing each over SSH. A failure on one configured host is logged\n" + + "and the run continues; the command exits non-zero if any\n" + + "configured host failed.\n\n" + + "With --host, sync ignores remote_hosts and syncs only that host.\n\n" + + "Remote sync uses your existing SSH configuration and requires\n" + + "key-based (passwordless) auth; it never prompts for a password.", GroupID: groupCore, SilenceUsage: true, Args: cobra.NoArgs, @@ -483,6 +493,16 @@ func writeRootHelp(w io.Writer, root *cobra.Command) { fmt.Fprintln(w, " When set, these override default directory. Environment variables") fmt.Fprintln(w, " override config file arrays.") fmt.Fprintln(w) + fmt.Fprintln(w, "Remote hosts:") + fmt.Fprintln(w, " Add a [[remote_hosts]] array to ~/.agentsview/config.toml so that") + fmt.Fprintln(w, " \"agentsview sync\" (no --host) also syncs each host over SSH:") + fmt.Fprintln(w, " [[remote_hosts]]") + fmt.Fprintln(w, " host = \"devbox1\"") + fmt.Fprintln(w, " user = \"jesse\" # optional") + fmt.Fprintln(w, " port = 22 # optional") + fmt.Fprintln(w, " Each host must be unique.") + fmt.Fprintln(w, " Requires key-based (passwordless) SSH to each host.") + fmt.Fprintln(w) fmt.Fprintln(w, "Data stored in ~/.agentsview/ by default.") } diff --git a/cmd/agentsview/cli_test.go b/cmd/agentsview/cli_test.go index acbdebb2..b03ec3ba 100644 --- a/cmd/agentsview/cli_test.go +++ b/cmd/agentsview/cli_test.go @@ -175,3 +175,20 @@ func TestExecuteCLIWithLegacyFlagCompatWarnsOnce(t *testing.T) { want := "warning: deprecated single-dash long flags detected; use GNU-style long flags instead: -version -> --version\n" assert.Equal(t, want, stderr.String()) } + +func TestRootHelpDocumentsRemoteHosts(t *testing.T) { + help, err := executeCommand(newRootCommand(), "--help") + require.NoError(t, err, "Execute") + for _, want := range []string{"remote_hosts", "passwordless"} { + assert.Contains(t, help, want, + "root help should document %q", want) + } +} + +func TestSyncHelpMentionsConfiguredHosts(t *testing.T) { + help, err := executeCommand(newRootCommand(), "sync", "--help") + require.NoError(t, err, "Execute") + for _, want := range []string{"remote_hosts", "--host", "passwordless"} { + assert.Contains(t, help, want, "sync help missing %q", want) + } +} diff --git a/cmd/agentsview/sync.go b/cmd/agentsview/sync.go index 0abb7129..0070a18a 100644 --- a/cmd/agentsview/sync.go +++ b/cmd/agentsview/sync.go @@ -31,6 +31,16 @@ type SyncConfig struct { } func runSync(cfg SyncConfig) { + if doSync(cfg) { + os.Exit(1) + } +} + +// doSync performs the sync run and reports whether any configured +// remote host failed. It owns the deferred cleanup (profile stop, +// db close) so runSync can translate the result into a non-zero +// exit code without skipping that cleanup. +func doSync(cfg SyncConfig) (hadRemoteFailures bool) { appCfg, err := config.LoadMinimal() if err != nil { log.Fatalf("loading config: %v", err) @@ -64,26 +74,116 @@ func runSync(cfg SyncConfig) { if cfg.Host != "" { runRemoteSync(appCfg, database, cfg) - return + return false } - runLocalSync(appCfg, database, cfg.Full) + if err := appCfg.ValidateRemoteHosts(); err != nil { + fatal("invalid remote_hosts config: %v", err) + } + + failures := syncLocalAndRemotes( + appCfg.RemoteHosts, cfg.Full, + func() bool { return runLocalSync(appCfg, database, cfg.Full) }, + func(rh config.RemoteHost, full bool) error { + return runRemoteSyncOnce(appCfg, database, rh, full) + }, + ) + reportRemoteFailures(failures) + return len(failures) > 0 +} + +// syncLocalAndRemotes runs the local sync, then the configured +// remote hosts. A local resync (forced via --full or an automatic +// data-version resync) forces every remote sync full as well, so +// remote sessions are re-parsed rather than skipped via the remote +// skip cache. localSync and remoteSync are injected for testing; +// localSync returns whether a full resync was performed. +func syncLocalAndRemotes( + hosts []config.RemoteHost, cfgFull bool, + localSync func() bool, + remoteSync func(config.RemoteHost, bool) error, +) []remoteHostFailure { + didResync := localSync() + full := cfgFull || didResync + return runRemoteHosts(hosts, full, remoteSync) } func runRemoteSync( appCfg config.Config, database *db.DB, cfg SyncConfig, ) { + rh := config.RemoteHost{ + Host: cfg.Host, + User: cfg.User, + Port: cfg.Port, + } + if err := runRemoteSyncOnce( + appCfg, database, rh, cfg.Full, + ); err != nil { + fatal("remote sync: %v", err) + } +} + +// runRemoteSyncOnce syncs a single remote host and returns any +// error instead of exiting, so it backs both the single-host +// --host path and the configured-hosts fan-out. +func runRemoteSyncOnce( + appCfg config.Config, database *db.DB, + rh config.RemoteHost, full bool, +) error { rs := &ssh.RemoteSync{ - Host: cfg.Host, - User: cfg.User, - Port: cfg.Port, - Full: cfg.Full, + Host: rh.Host, + User: rh.User, + Port: rh.Port, + Full: full, DB: database, BlockedResultCategories: appCfg.ResultContentBlockedCategories, } - ctx := context.Background() - if _, err := rs.Run(ctx); err != nil { - fatal("remote sync: %v", err) + _, err := rs.Run(context.Background()) + return err +} + +// remoteHostFailure records a configured remote host that failed +// to sync. It keeps the full RemoteHost (not just the name) so +// duplicate hostnames that differ by user/port stay distinct. +type remoteHostFailure struct { + Host config.RemoteHost + Err error +} + +// runRemoteHosts syncs each configured host in declared order via +// syncFn, continuing past failures, and returns the collected +// failures. It performs no logging so it can be unit-tested +// without capturing the global logger; callers own all output. +func runRemoteHosts( + hosts []config.RemoteHost, full bool, + syncFn func(config.RemoteHost, bool) error, +) []remoteHostFailure { + var failures []remoteHostFailure + for _, rh := range hosts { + if err := syncFn(rh, full); err != nil { + failures = append(failures, remoteHostFailure{ + Host: rh, + Err: err, + }) + } + } + return failures +} + +// reportRemoteFailures writes per-host failures to the debug log +// and a summary to stderr, so unattended (cron) runs surface them +// even though setupLogFile redirects log output to a file. +func reportRemoteFailures(failures []remoteHostFailure) { + if len(failures) == 0 { + return + } + for _, f := range failures { + log.Printf("remote sync %s failed: %v", f.Host.Host, f.Err) + } + fmt.Fprintf(os.Stderr, + "sync: %d remote host(s) failed:\n", len(failures)) + for _, f := range failures { + fmt.Fprintf(os.Stderr, " %s: %v\n", f.Host.Host, f.Err) } } diff --git a/cmd/agentsview/sync_test.go b/cmd/agentsview/sync_test.go new file mode 100644 index 00000000..23a7c157 --- /dev/null +++ b/cmd/agentsview/sync_test.go @@ -0,0 +1,78 @@ +package main + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.kenn.io/agentsview/internal/config" +) + +func TestRunRemoteHosts_AttemptsAllAndCollectsFailures(t *testing.T) { + hosts := []config.RemoteHost{ + {Host: "alpha"}, + {Host: "beta", User: "u", Port: 2222}, + {Host: "gamma"}, + } + failBeta := errors.New("ssh down") + + var attempted []config.RemoteHost + failures := runRemoteHosts(hosts, true, func(rh config.RemoteHost, full bool) error { + attempted = append(attempted, rh) + assert.True(t, full, "full flag should propagate to syncFn") + if rh.Host == "beta" { + return failBeta + } + return nil + }) + + // Every host attempted, in declared order, even after a failure. + require.Equal(t, hosts, attempted) + // Only beta failed; its full RemoteHost (user/port) is preserved. + require.Len(t, failures, 1) + assert.Equal(t, hosts[1], failures[0].Host) + assert.Equal(t, failBeta, failures[0].Err) +} + +func TestRunRemoteHosts_AllSucceedReturnsEmpty(t *testing.T) { + hosts := []config.RemoteHost{{Host: "alpha"}, {Host: "beta"}} + failures := runRemoteHosts(hosts, false, func(config.RemoteHost, bool) error { + return nil + }) + assert.Empty(t, failures) +} + +func TestSyncLocalAndRemotes_ResyncForcesRemoteFull(t *testing.T) { + tests := []struct { + name string + cfgFull bool + didResync bool + wantFull bool + }{ + {"no full, no resync", false, false, false}, + {"automatic resync forces remote full", false, true, true}, + {"cli --full", true, false, true}, + {"both", true, true, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + hosts := []config.RemoteHost{{Host: "alpha"}, {Host: "beta"}} + localCalled := false + var gotFull []bool + failures := syncLocalAndRemotes(hosts, tt.cfgFull, + func() bool { localCalled = true; return tt.didResync }, + func(_ config.RemoteHost, full bool) error { + gotFull = append(gotFull, full) + return nil + }) + + require.True(t, localCalled, "local sync must run") + assert.Empty(t, failures) + require.Len(t, gotFull, len(hosts)) + for _, full := range gotFull { + assert.Equal(t, tt.wantFull, full) + } + }) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index c75cf4fe..bd795ca4 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -85,6 +85,15 @@ type CustomModelRate struct { CacheRead float64 `json:"cache_read,omitempty" toml:"cache_read"` } +// RemoteHost describes one SSH target for config-driven +// `agentsview sync` fan-out. Host is required; User and Port are +// optional (Port 0 means the ssh default of 22). +type RemoteHost struct { + Host string `toml:"host" json:"host"` + User string `toml:"user,omitempty" json:"user,omitempty"` + Port int `toml:"port,omitempty" json:"port,omitempty"` +} + // Config holds all application configuration. type Config struct { Host string `json:"host" toml:"host"` @@ -128,6 +137,12 @@ type Config struct { CustomModelPricing map[string]CustomModelRate `json:"custom_model_pricing,omitempty" toml:"custom_model_pricing"` + // RemoteHosts is the config-file list of SSH targets that + // `agentsview sync` (with no --host) syncs after the local + // pass. CLI/config-file only; never serialized to the + // settings API, so there is no web-UI editing of this list. + RemoteHosts []RemoteHost `json:"-" toml:"-"` + // HostExplicit is true when the user passed --host on the CLI. // Used to prevent auto-bind to 0.0.0.0 when the user // explicitly requested a specific host. @@ -158,6 +173,46 @@ func (c *Config) IsUserConfigured( return c.agentDirSource[agent] != dirDefault } +// ValidateRemoteHosts checks the configured remote_hosts entries +// for semantic errors: a non-empty host and a port within 0..65535 +// (0 means the ssh default). It checks the trimmed values that +// loadFile already normalized, so what is validated here is exactly +// what is passed to ssh. Returns an aggregated error naming every +// offending entry, or nil when all entries are valid. +func (c Config) ValidateRemoteHosts() error { + var problems []string + seen := make(map[string]int, len(c.RemoteHosts)) + for i, h := range c.RemoteHosts { + if h.Host == "" { + problems = append(problems, + fmt.Sprintf("entry %d: host is required", i+1)) + } + if h.Port < 0 || h.Port > 65535 { + problems = append(problems, + fmt.Sprintf("entry %d (%q): invalid port %d", + i+1, h.Host, h.Port)) + } + // Remote sync namespaces sessions and the skip cache by + // host alone (see ssh.RemoteSync), so two entries sharing a + // host collide regardless of user/port. Reject duplicates + // rather than silently share or overwrite cached state. + if h.Host != "" { + if first, ok := seen[h.Host]; ok { + problems = append(problems, + fmt.Sprintf("entry %d: duplicate host %q (already at entry %d)", + i+1, h.Host, first)) + } else { + seen[h.Host] = i + 1 + } + } + } + if len(problems) > 0 { + return fmt.Errorf("remote_hosts: %s", + strings.Join(problems, "; ")) + } + return nil +} + // Default returns a Config with default values. func Default() (Config, error) { home, err := os.UserHomeDir() @@ -375,6 +430,7 @@ func (c *Config) loadFile() error { Agent map[string]AgentConfig `toml:"agent"` EventsCoalesceInterval time.Duration `toml:"events_coalesce_interval"` CustomModelPricing map[string]CustomModelRate `toml:"custom_model_pricing"` + RemoteHosts []RemoteHost `toml:"remote_hosts"` } meta, err := toml.DecodeFile(path, &file) if err != nil { @@ -457,6 +513,17 @@ func (c *Config) loadFile() error { if len(file.CustomModelPricing) > 0 { c.CustomModelPricing = file.CustomModelPricing } + if len(file.RemoteHosts) > 0 { + hosts := make([]RemoteHost, len(file.RemoteHosts)) + for i, h := range file.RemoteHosts { + hosts[i] = RemoteHost{ + Host: strings.TrimSpace(h.Host), + User: strings.TrimSpace(h.User), + Port: h.Port, + } + } + c.RemoteHosts = hosts + } // Parse config-file dir arrays for agents that have a // ConfigKey. Only apply when not already set by env var. diff --git a/internal/config/config_test.go b/internal/config/config_test.go index c5f492e6..f23badee 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -945,3 +945,64 @@ func TestLoadFile_CustomModelPricing(t *testing.T) { }) } } + +func TestLoadFile_RemoteHosts(t *testing.T) { + dir := setupTestEnv(t) + path := filepath.Join(dir, configFileName) + data := []byte(`[[remote_hosts]] +host = "devbox1" +user = "jesse" +port = 22 + +[[remote_hosts]] +host = " laptop2 " +`) + require.NoError(t, os.WriteFile(path, data, 0o600), "write config") + + cfg, err := LoadMinimal() + require.NoError(t, err) + + require.Len(t, cfg.RemoteHosts, 2) + assert.Equal(t, RemoteHost{Host: "devbox1", User: "jesse", Port: 22}, cfg.RemoteHosts[0]) + // host is trimmed at load so validation and SSH see the same value + assert.Equal(t, RemoteHost{Host: "laptop2"}, cfg.RemoteHosts[1]) +} + +func TestLoadFile_RemoteHostsAbsentIsNil(t *testing.T) { + dir := setupTestEnv(t) + writeConfig(t, dir, map[string]any{"public_url": "http://example.com"}) + + cfg, err := LoadMinimal() + require.NoError(t, err) + assert.Nil(t, cfg.RemoteHosts) +} + +func TestValidateRemoteHosts(t *testing.T) { + tests := []struct { + name string + hosts []RemoteHost + wantErr []string // substrings expected in error; empty => no error + }{ + {"valid", []RemoteHost{{Host: "a"}, {Host: "b", Port: 22}, {Host: "c", Port: 0}}, nil}, + {"empty host", []RemoteHost{{Host: ""}}, []string{"host is required"}}, + {"negative port", []RemoteHost{{Host: "a", Port: -1}}, []string{"invalid port"}}, + {"port too large", []RemoteHost{{Host: "a", Port: 70000}}, []string{"invalid port"}}, + {"aggregates both", []RemoteHost{{Host: ""}, {Host: "b", Port: 99999}}, []string{"host is required", "invalid port"}}, + {"duplicate host", []RemoteHost{{Host: "a"}, {Host: "a"}}, []string{"duplicate host"}}, + {"duplicate host different user or port", []RemoteHost{{Host: "box", User: "alice"}, {Host: "box", User: "bob", Port: 2222}}, []string{"duplicate host"}}, + {"none configured", nil, nil}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := Config{RemoteHosts: tt.hosts}.ValidateRemoteHosts() + if len(tt.wantErr) == 0 { + require.NoError(t, err) + return + } + require.Error(t, err) + for _, want := range tt.wantErr { + assert.Contains(t, err.Error(), want) + } + }) + } +} diff --git a/internal/ssh/ssh.go b/internal/ssh/ssh.go index 54e5bce6..72341677 100644 --- a/internal/ssh/ssh.go +++ b/internal/ssh/ssh.go @@ -10,16 +10,28 @@ import ( "strings" ) +// sshConnectTimeoutSecs bounds the TCP connect phase of each ssh +// invocation so an unattended sync fails fast on an unreachable +// host instead of stalling on the OS default timeout. +const sshConnectTimeoutSecs = 10 + // buildSSHArgs constructs args for the ssh command. // // Remote commands are always executed through a POSIX shell via // "sh -c ''" so behavior is independent of the remote user's // login shell (e.g. fish). // -// Returns ["ssh", "user@host", "--", "sh -c ''"] or -// ["ssh", "host", "--", "sh -c ''"] when user is empty. -// Port adds "-p N" when > 0. Extra opts are inserted before the -// target (e.g. "-i keyfile"). +// The invocation is non-interactive: it passes BatchMode=yes (never +// prompt for a password/passphrase -- remote sync requires key-based +// auth) and a bounded ConnectTimeout, so unattended runs fail fast +// with a clear error instead of stalling. These defaults follow +// sshOpts so an explicit override there wins (ssh uses the first +// value seen for each option). +// +// Returns ["ssh", "-o", "BatchMode=yes", "-o", "ConnectTimeout=N", +// "user@host", "--", "sh -c ''"] (or "host" when user is +// empty). Port adds "-p N" when > 0; extra sshOpts (e.g. "-i +// keyfile") are inserted before the defaults. func buildSSHArgs( host, user string, port int, sshOpts []string, cmd string, ) []string { @@ -33,6 +45,10 @@ func buildSSHArgs( args = append(args, "-p", strconv.Itoa(port)) } args = append(args, sshOpts...) + args = append(args, + "-o", "BatchMode=yes", + "-o", "ConnectTimeout="+strconv.Itoa(sshConnectTimeoutSecs), + ) return append(args, target, "--", remoteCmd) } diff --git a/internal/ssh/ssh_test.go b/internal/ssh/ssh_test.go index c5cb357f..e82150de 100644 --- a/internal/ssh/ssh_test.go +++ b/internal/ssh/ssh_test.go @@ -6,6 +6,14 @@ import ( "github.com/stretchr/testify/assert" ) +// nonInteractive is the trailing option block buildSSHArgs appends +// to every invocation (see sshConnectTimeoutSecs). Kept here so the +// table below documents the full expected arg vector. +var nonInteractive = []string{ + "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", +} + func TestBuildSSHArgs(t *testing.T) { tests := []struct { name string @@ -21,7 +29,9 @@ func TestBuildSSHArgs(t *testing.T) { host: "devbox1", cmd: "echo hello", want: []string{ - "ssh", "devbox1", "--", "sh -c 'echo hello'", + "ssh", "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", + "devbox1", "--", "sh -c 'echo hello'", }, }, { @@ -30,7 +40,9 @@ func TestBuildSSHArgs(t *testing.T) { user: "wes", cmd: "ls -la", want: []string{ - "ssh", "wes@devbox1", "--", "sh -c 'ls -la'", + "ssh", "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", + "wes@devbox1", "--", "sh -c 'ls -la'", }, }, { @@ -41,6 +53,8 @@ func TestBuildSSHArgs(t *testing.T) { cmd: "echo hi", want: []string{ "ssh", "-p", "2222", + "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", "wes@devbox1", "--", "sh -c 'echo hi'", }, }, @@ -49,11 +63,13 @@ func TestBuildSSHArgs(t *testing.T) { host: "devbox1", cmd: "echo hi", want: []string{ - "ssh", "devbox1", "--", "sh -c 'echo hi'", + "ssh", "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", + "devbox1", "--", "sh -c 'echo hi'", }, }, { - name: "with ssh opts", + name: "with ssh opts before defaults", host: "devbox1", user: "wes", port: 2222, @@ -66,6 +82,8 @@ func TestBuildSSHArgs(t *testing.T) { "ssh", "-p", "2222", "-i", "/tmp/key", "-o", "StrictHostKeyChecking=no", + "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", "wes@devbox1", "--", "sh -c 'ls'", }, }, @@ -74,7 +92,9 @@ func TestBuildSSHArgs(t *testing.T) { host: "devbox1", cmd: `printf "it's fine"`, want: []string{ - "ssh", "devbox1", "--", + "ssh", "-o", "BatchMode=yes", + "-o", "ConnectTimeout=10", + "devbox1", "--", `sh -c 'printf "it'\''s fine"'`, }, }, @@ -89,3 +109,12 @@ func TestBuildSSHArgs(t *testing.T) { }) } } + +// TestBuildSSHArgs_NonInteractiveDefaults locks the security intent +// independent of arg ordering: remote sync never prompts (BatchMode) +// and bounds the connect phase (ConnectTimeout). +func TestBuildSSHArgs_NonInteractiveDefaults(t *testing.T) { + got := buildSSHArgs("devbox1", "", 0, nil, "true") + assert.Subset(t, got, nonInteractive, + "ssh invocation must include non-interactive defaults") +}