From 8df7caa453d9bc83c75ceb892dc36c165f4fbf2c Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 7 Jul 2026 15:09:16 +0200 Subject: [PATCH 1/2] fix(board): store paths home-relative so shared state works in sandboxes Assisted-By: docker-agent --- pkg/board/app.go | 21 ++++++++++++-------- pkg/board/paths.go | 39 ++++++++++++++++++++++++++++++++++++ pkg/board/paths_test.go | 44 +++++++++++++++++++++++++++++++++++++++++ pkg/board/store.go | 17 ++++++++++++++-- pkg/board/store_test.go | 35 ++++++++++++++++++++++++++++++++ 5 files changed, 146 insertions(+), 10 deletions(-) create mode 100644 pkg/board/paths.go create mode 100644 pkg/board/paths_test.go diff --git a/pkg/board/app.go b/pkg/board/app.go index 5c5875a367..ae66a0e6f9 100644 --- a/pkg/board/app.go +++ b/pkg/board/app.go @@ -12,7 +12,6 @@ import ( "strings" "sync" - "github.com/docker/docker-agent/pkg/paths" "github.com/docker/docker-agent/pkg/userconfig" ) @@ -146,7 +145,7 @@ func (a *App) Projects() []Project { } projects := make([]Project, 0, len(a.config.Board.Projects)) for _, p := range a.config.Board.Projects { - projects = append(projects, Project{Name: p.Name, Path: p.Path, Agent: p.Agent}) + projects = append(projects, Project{Name: p.Name, Path: expandHome(p.Path), Agent: p.Agent}) } return projects } @@ -178,8 +177,10 @@ func (a *App) AddProject(p Project) error { } } a.config.Board.Projects = append(a.config.Board.Projects, userconfig.BoardProject{ - Name: p.Name, - Path: p.Path, + Name: p.Name, + // Stored ~-contracted so the shared config works across + // environments whose home differs (host vs. docker sandbox). + Path: contractHome(p.Path), Agent: p.Agent, }) return a.saveConfigLocked() @@ -193,10 +194,7 @@ func normalizeProjectPath(path string) (string, error) { if path == "" { return "", errors.New("project path is required") } - if path == "~" || strings.HasPrefix(path, "~/") { - path = filepath.Join(paths.GetHomeDir(), strings.TrimPrefix(path[1:], "/")) - } - abs, err := filepath.Abs(path) + abs, err := filepath.Abs(expandHome(path)) if err != nil { return "", fmt.Errorf("resolve project path: %w", err) } @@ -237,6 +235,13 @@ func (a *App) CreateCard(project Project, prompt string) (card *Card, err error) agent = DefaultAgent } + // Checked before launch because tmux silently falls back to $HOME when + // its start directory is missing, which would surface as a confusing + // worktree error from the agent instead of this one. + if !isGitRepo(a.ctx, project.Path) { + return nil, fmt.Errorf("project path %s is not a git repository in this environment; re-add the project here", project.Path) + } + worktreeName := newWorktreeName() branch := worktreeBranch(worktreeName) wtPath := worktreeDir(worktreeName) diff --git a/pkg/board/paths.go b/pkg/board/paths.go new file mode 100644 index 0000000000..782abbc7df --- /dev/null +++ b/pkg/board/paths.go @@ -0,0 +1,39 @@ +package board + +import ( + "path/filepath" + "strings" + + "github.com/docker/docker-agent/pkg/paths" +) + +// The board's project list and card state live in files shared across +// environments with different home directories — e.g. a host and a docker +// sandbox that mirrors the host's home-relative mounts (~/src, ~/.cagent). +// Paths under home are therefore persisted as "~/…" and expanded against +// the current home on load, so the same state works in both. + +// contractHome replaces a current-home prefix with "~", using forward +// slashes so the stored form is platform-neutral. +func contractHome(path string) string { + home := paths.GetHomeDir() + if home == "" || path == "" { + return path + } + if path == home { + return "~" + } + if rel, ok := strings.CutPrefix(path, home+string(filepath.Separator)); ok { + return "~/" + filepath.ToSlash(rel) + } + return path +} + +// expandHome resolves a leading "~" against the current home directory. +// Other paths are returned unchanged. +func expandHome(path string) string { + if path == "~" || strings.HasPrefix(path, "~/") { + return filepath.Join(paths.GetHomeDir(), strings.TrimPrefix(path[1:], "/")) + } + return path +} diff --git a/pkg/board/paths_test.go b/pkg/board/paths_test.go new file mode 100644 index 0000000000..f71285eb95 --- /dev/null +++ b/pkg/board/paths_test.go @@ -0,0 +1,44 @@ +package board + +import ( + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + + "github.com/docker/docker-agent/pkg/paths" +) + +func TestContractHome(t *testing.T) { + t.Parallel() + + home := paths.GetHomeDir() + + assert.Equal(t, "~/src/repo", contractHome(filepath.Join(home, "src", "repo"))) + assert.Equal(t, "~", contractHome(home)) + assert.Empty(t, contractHome("")) + // A path outside home is stored as-is. + assert.Equal(t, filepath.FromSlash("/opt/repo"), contractHome(filepath.FromSlash("/opt/repo"))) + // A sibling of home must not lose its prefix. + assert.Equal(t, home+"-other", contractHome(home+"-other")) +} + +func TestExpandHome(t *testing.T) { + t.Parallel() + + home := paths.GetHomeDir() + + assert.Equal(t, filepath.Join(home, "src", "repo"), expandHome("~/src/repo")) + assert.Equal(t, home, expandHome("~")) + assert.Empty(t, expandHome("")) + assert.Equal(t, filepath.FromSlash("/opt/repo"), expandHome(filepath.FromSlash("/opt/repo"))) + // Only a leading ~/ is expanded, not ~ elsewhere or ~user. + assert.Equal(t, "~user/src", expandHome("~user/src")) +} + +func TestContractExpandRoundTrip(t *testing.T) { + t.Parallel() + + p := filepath.Join(paths.GetHomeDir(), ".cagent", "worktrees", "board-abc") + assert.Equal(t, p, expandHome(contractHome(p))) +} diff --git a/pkg/board/store.go b/pkg/board/store.go index f75cb8b667..3615249d45 100644 --- a/pkg/board/store.go +++ b/pkg/board/store.go @@ -51,15 +51,28 @@ func OpenStore(path string) (*Store, error) { if err := json.Unmarshal(data, &s.cards); err != nil { return nil, fmt.Errorf("parse board state %s: %w", path, err) } + for _, c := range s.cards { + c.RepoPath = expandHome(c.RepoPath) + c.Worktree = expandHome(c.Worktree) + } return s, nil } -// save persists the cards. Callers must hold s.mu. +// save persists the cards. Callers must hold s.mu. Paths under the current +// home are written ~-contracted so the state file stays valid across +// environments whose home differs (host vs. docker sandbox). func (s *Store) save() error { if err := os.MkdirAll(filepath.Dir(s.path), 0o750); err != nil { return err } - data, err := json.MarshalIndent(s.cards, "", " ") + cards := make([]*Card, len(s.cards)) + for i, c := range s.cards { + clone := *c + clone.RepoPath = contractHome(clone.RepoPath) + clone.Worktree = contractHome(clone.Worktree) + cards[i] = &clone + } + data, err := json.MarshalIndent(cards, "", " ") if err != nil { return err } diff --git a/pkg/board/store_test.go b/pkg/board/store_test.go index e7265d2dee..d92fcffc7c 100644 --- a/pkg/board/store_test.go +++ b/pkg/board/store_test.go @@ -1,11 +1,14 @@ package board import ( + "os" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/paths" ) func testStore(t *testing.T) *Store { @@ -117,3 +120,35 @@ func TestStoreDeleteCard(t *testing.T) { // Deleting a missing card is a no-op. require.NoError(t, s.DeleteCard("c1")) } + +// TestStorePortablePaths proves card paths under home are persisted +// ~-contracted (so shared state files work across environments with +// different home directories) and expanded again on load. +func TestStorePortablePaths(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "cards.json") + s, err := OpenStore(path) + require.NoError(t, err) + + home := paths.GetHomeDir() + card := &Card{ + ID: "c1", + Column: "dev", + RepoPath: filepath.Join(home, "src", "repo"), + Worktree: filepath.Join(home, ".cagent", "worktrees", "board-abc"), + } + require.NoError(t, s.InsertCard(card)) + + data, err := os.ReadFile(path) + require.NoError(t, err) + assert.Contains(t, string(data), `"~/src/repo"`) + assert.Contains(t, string(data), `"~/.cagent/worktrees/board-abc"`) + + s2, err := OpenStore(path) + require.NoError(t, err) + got, err := s2.GetCard("c1") + require.NoError(t, err) + assert.Equal(t, card.RepoPath, got.RepoPath) + assert.Equal(t, card.Worktree, got.Worktree) +} From ea014f76095d3dff023f291f69c986ef100b5cb0 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Tue, 7 Jul 2026 15:14:56 +0200 Subject: [PATCH 2/2] fix(board): harden path expansion, state load, and git repo check Assisted-By: claude-opus-4-5 --- pkg/board/git.go | 7 +++++-- pkg/board/paths.go | 11 +++++++++-- pkg/board/paths_test.go | 11 +++++++++++ pkg/board/store.go | 3 +++ pkg/board/store_test.go | 15 +++++++++++++++ 5 files changed, 43 insertions(+), 4 deletions(-) diff --git a/pkg/board/git.go b/pkg/board/git.go index b05ab2929c..56b0b57b59 100644 --- a/pkg/board/git.go +++ b/pkg/board/git.go @@ -23,11 +23,14 @@ func removeWorktree(ctx context.Context, repoPath, worktreePath, branch string) _ = cmd.Run() } -// isGitRepo reports whether path is inside a git working tree. +// isGitRepo reports whether path is inside a git working tree. The output +// is checked, not just the exit code: inside a bare repo or a .git dir the +// command succeeds but prints "false". func isGitRepo(ctx context.Context, path string) bool { cmd := exec.CommandContext(ctx, "git", "rev-parse", "--is-inside-work-tree") cmd.Dir = path - return cmd.Run() == nil + out, err := cmd.Output() + return err == nil && strings.TrimSpace(string(out)) == "true" } // worktreeDiff returns the full diff of all changes in the worktree relative diff --git a/pkg/board/paths.go b/pkg/board/paths.go index 782abbc7df..f41d8dba24 100644 --- a/pkg/board/paths.go +++ b/pkg/board/paths.go @@ -30,10 +30,17 @@ func contractHome(path string) string { } // expandHome resolves a leading "~" against the current home directory. -// Other paths are returned unchanged. +// Other paths — and "~" paths when the home cannot be determined — are +// returned unchanged, so a failed lookup can never turn a stored path into +// a relative one (which a later save would persist, corrupting the shared +// state). func expandHome(path string) string { if path == "~" || strings.HasPrefix(path, "~/") { - return filepath.Join(paths.GetHomeDir(), strings.TrimPrefix(path[1:], "/")) + home := paths.GetHomeDir() + if home == "" { + return path + } + return filepath.Join(home, strings.TrimPrefix(path[1:], "/")) } return path } diff --git a/pkg/board/paths_test.go b/pkg/board/paths_test.go index f71285eb95..cc5c344395 100644 --- a/pkg/board/paths_test.go +++ b/pkg/board/paths_test.go @@ -36,6 +36,17 @@ func TestExpandHome(t *testing.T) { assert.Equal(t, "~user/src", expandHome("~user/src")) } +// TestExpandHomeNoHome proves a failed home lookup leaves "~" paths +// untouched instead of turning them into relative paths that a later save +// would persist. +func TestExpandHomeNoHome(t *testing.T) { + t.Setenv("HOME", "") + t.Setenv("USERPROFILE", "") + + assert.Equal(t, "~/src/repo", expandHome("~/src/repo")) + assert.Equal(t, "~", expandHome("~")) +} + func TestContractExpandRoundTrip(t *testing.T) { t.Parallel() diff --git a/pkg/board/store.go b/pkg/board/store.go index 3615249d45..1a960c31bf 100644 --- a/pkg/board/store.go +++ b/pkg/board/store.go @@ -51,6 +51,9 @@ func OpenStore(path string) (*Store, error) { if err := json.Unmarshal(data, &s.cards); err != nil { return nil, fmt.Errorf("parse board state %s: %w", path, err) } + // Drop null entries (hand-edited or corrupted file) rather than panic; + // they carry no data worth preserving. + s.cards = slices.DeleteFunc(s.cards, func(c *Card) bool { return c == nil }) for _, c := range s.cards { c.RepoPath = expandHome(c.RepoPath) c.Worktree = expandHome(c.Worktree) diff --git a/pkg/board/store_test.go b/pkg/board/store_test.go index d92fcffc7c..d684695639 100644 --- a/pkg/board/store_test.go +++ b/pkg/board/store_test.go @@ -121,6 +121,21 @@ func TestStoreDeleteCard(t *testing.T) { require.NoError(t, s.DeleteCard("c1")) } +// TestOpenStoreSkipsNullCards proves a hand-edited or corrupted state file +// containing null entries loads instead of panicking. +func TestOpenStoreSkipsNullCards(t *testing.T) { + t.Parallel() + + path := filepath.Join(t.TempDir(), "cards.json") + require.NoError(t, os.WriteFile(path, []byte(`[null, {"id": "c1", "column": "dev"}]`), 0o644)) + + s, err := OpenStore(path) + require.NoError(t, err) + cards := s.ListCards() + require.Len(t, cards, 1) + assert.Equal(t, "c1", cards[0].ID) +} + // TestStorePortablePaths proves card paths under home are persisted // ~-contracted (so shared state files work across environments with // different home directories) and expanded again on load.