fix(agentctl): write ACP debug logs under KANDEV_HOME_DIR (#1196)

2026-05-31 17:10:08 +01:00
parent 644d428ccb
commit 700ff4199c
4 changed files with 97 additions and 3 deletions
@@ -44,7 +44,7 @@ JSON-RPC 2.0 over stdin/stdout between agentctl and agent process. Requests: `in
 When `KANDEV_DEBUG_AGENT_MESSAGES=true` (on by default in the **dev** profile), the ACP adapter dumps every raw + normalized frame to **per-session** JSONL files:

 - Files: `raw-{protocol}-{agentID}-{sessionID}.jsonl` and `normalized-{protocol}-{agentID}-{sessionID}.jsonl` (the `raw-`/`normalized-` prefix + `.jsonl` suffix is a contract with the reader in `internal/debug`).
- Default dir: `~/.kandev/logs/acp/` (override with `KANDEV_DEBUG_LOG_DIR`).
+- Dir resolution: `KANDEV_DEBUG_LOG_DIR` (explicit override) → `<KANDEV_HOME_DIR>/logs/acp` (honors dev/e2e isolation — `KANDEV_HOME_DIR` is already the Kandev root, so no extra `.kandev` segment) → `~/.kandev/logs/acp/` → process CWD.
 - One kept-open buffered writer + dedicated mutex per session (no global lock on the hot path); rotates on a per-file byte cap.
 - A `shared.Janitor` (owned by `cmd/agentctl/main.go run()`, `Start`/`Stop`) flushes periodically and prunes oldest-by-mtime first, enforcing a total-file cap and an age cap so an always-on dev session can't fill the disk. It also closes idle writers so handles don't leak.
 - Dev-only live tail of a stuck session from an in-memory ring buffer (zero disk growth): `GET /api/v1/debug/acp/{session}?n=200`.
@@ -12,7 +12,7 @@
 //
 // Debug logging (set externally — read at package init time). Frames are
 // written to per-session files raw-/normalized-{protocol}-{agentID}-{sessionID}.jsonl
-// under KANDEV_DEBUG_LOG_DIR (default ~/.kandev/logs/acp):
+// under KANDEV_DEBUG_LOG_DIR (default <KANDEV_HOME_DIR>/logs/acp, else ~/.kandev/logs/acp):
 //
 //	KANDEV_DEBUG_AGENT_MESSAGES=true KANDEV_DEBUG_LOG_DIR=/tmp/e2e-debug ...
 //
@@ -35,6 +35,7 @@ const (
 const (
 	envDebugMessages   = "KANDEV_DEBUG_AGENT_MESSAGES"
 	envLogDir          = "KANDEV_DEBUG_LOG_DIR"
+	envHomeDir         = "KANDEV_HOME_DIR"
 	envACPMaxFiles     = "KANDEV_DEBUG_ACP_MAX_FILES"
 	envACPRetentionHrs = "KANDEV_DEBUG_ACP_RETENTION_HOURS"
 	envACPMaxFileBytes = "KANDEV_DEBUG_ACP_MAX_FILE_BYTES"
@@ -62,7 +63,8 @@ type acpLogConfig struct {

 // acpLogConfigFromEnv resolves the on-disk config from the environment,
 // applying defaults. The output directory resolves to KANDEV_DEBUG_LOG_DIR,
-// then ~/.kandev/logs/acp, then the process CWD as a last resort.
+// then <KANDEV_HOME_DIR>/logs/acp, then ~/.kandev/logs/acp, then the process
+// CWD as a last resort.
 func acpLogConfigFromEnv() acpLogConfig {
 	return acpLogConfig{
 		dir:          resolveACPLogDir(),
@@ -89,6 +91,16 @@ func resolveACPLogDir() string {
 	if dir := os.Getenv(envLogDir); dir != "" {
 		return dir
 	}
+	// Honor KANDEV_HOME_DIR before $HOME so dev/e2e isolation (and Docker/K8s
+	// roots) keep ACP logs inside the configured Kandev home. The env value is
+	// already the Kandev root (e.g. <repo>/.kandev-dev), mirroring
+	// config.Config.ResolvedHomeDir — so we append logs/acp directly and must
+	// NOT add another ".kandev" segment. agentctl is a lean container binary
+	// that doesn't pull in the viper-based common/config, so we read the env
+	// directly rather than importing ResolvedHomeDir.
+	if home := os.Getenv(envHomeDir); home != "" {
+		return filepath.Join(expandTilde(home), "logs", "acp")
+	}
 	if home, err := os.UserHomeDir(); err == nil && home != "" {
 		return filepath.Join(home, ".kandev", "logs", "acp")
 	}
@@ -98,6 +110,23 @@ func resolveACPLogDir() string {
 	return "."
 }

+// expandTilde expands a leading "~/" to the user's home directory, mirroring
+// config.expandTilde. Returns the input unchanged if expansion is unnecessary
+// or fails.
+func expandTilde(p string) string {
+	if p != "~" && !strings.HasPrefix(p, "~/") {
+		return p
+	}
+	home, err := os.UserHomeDir()
+	if err != nil {
+		return p
+	}
+	if p == "~" {
+		return home
+	}
+	return filepath.Join(home, p[2:])
+}
+
 func envInt64(key string, def int64) int64 {
 	if v := os.Getenv(key); v != "" {
 		if n, err := strconv.ParseInt(v, 10, 64); err == nil && n > 0 {
@@ -16,6 +16,71 @@ func testEvent(sessionID string) *streams.AgentEvent {
 	return &streams.AgentEvent{Type: streams.EventTypeMessageChunk, SessionID: sessionID, Text: "hello"}
 }

+// TestResolveACPLogDir_Precedence pins the directory resolution order:
+// KANDEV_DEBUG_LOG_DIR > KANDEV_HOME_DIR > $HOME/.kandev. Critically,
+// KANDEV_HOME_DIR is already the Kandev root (e.g. <repo>/.kandev-dev), so the
+// logs land at <home>/logs/acp with NO extra ".kandev" segment — keeping dev
+// logs inside the isolated dev home that `make clean-db` wipes.
+func TestResolveACPLogDir_Precedence(t *testing.T) {
+	t.Run("explicit log dir wins over home dir", func(t *testing.T) {
+		explicit := filepath.Join(t.TempDir(), "explicit")
+		t.Setenv(envLogDir, explicit)
+		t.Setenv(envHomeDir, filepath.Join("/repo", ".kandev-dev"))
+		if got := resolveACPLogDir(); got != explicit {
+			t.Errorf("got %q, want %q", got, explicit)
+		}
+	})
+
+	t.Run("home dir honored before $HOME, no extra dotdir", func(t *testing.T) {
+		t.Setenv(envLogDir, "")
+		home := filepath.Join("/repo", ".kandev-dev")
+		t.Setenv(envHomeDir, home)
+		want := filepath.Join(home, "logs", "acp")
+		if got := resolveACPLogDir(); got != want {
+			t.Errorf("got %q, want %q", got, want)
+		}
+	})
+
+	t.Run("expands leading tilde in home dir", func(t *testing.T) {
+		userHome, err := os.UserHomeDir()
+		if err != nil || userHome == "" {
+			t.Skip("no user home dir on this platform")
+		}
+		t.Setenv(envLogDir, "")
+		t.Setenv(envHomeDir, "~/.kandev-dev")
+		want := filepath.Join(userHome, ".kandev-dev", "logs", "acp")
+		if got := resolveACPLogDir(); got != want {
+			t.Errorf("got %q, want %q", got, want)
+		}
+	})
+
+	t.Run("expands bare tilde home dir", func(t *testing.T) {
+		userHome, err := os.UserHomeDir()
+		if err != nil || userHome == "" {
+			t.Skip("no user home dir on this platform")
+		}
+		t.Setenv(envLogDir, "")
+		t.Setenv(envHomeDir, "~")
+		want := filepath.Join(userHome, "logs", "acp")
+		if got := resolveACPLogDir(); got != want {
+			t.Errorf("got %q, want %q", got, want)
+		}
+	})
+
+	t.Run("falls back to $HOME/.kandev when home dir unset", func(t *testing.T) {
+		userHome, err := os.UserHomeDir()
+		if err != nil || userHome == "" {
+			t.Skip("no user home dir on this platform")
+		}
+		t.Setenv(envLogDir, "")
+		t.Setenv(envHomeDir, "")
+		want := filepath.Join(userHome, ".kandev", "logs", "acp")
+		if got := resolveACPLogDir(); got != want {
+			t.Errorf("got %q, want %q", got, want)
+		}
+	})
+}
+
 // TestACPLog_PerSessionRouting verifies two sessions land in two distinct
 // files, each filename carrying its own session id, and keeping the
 // normalized-/raw- prefix + .jsonl suffix the debug reader requires.