Skip to content

fix(board): store paths home-relative so shared state works in sandboxes#3510

Merged
dgageot merged 2 commits into
docker:mainfrom
dgageot:fix-board-sandbox-paths
Jul 7, 2026
Merged

fix(board): store paths home-relative so shared state works in sandboxes#3510
dgageot merged 2 commits into
docker:mainfrom
dgageot:fix-board-sandbox-paths

Conversation

@dgageot

@dgageot dgageot commented Jul 7, 2026

Copy link
Copy Markdown
Member

The board stores project paths in the user config and card state (RepoPath, Worktree) as absolute paths in ~/.cagent/board/cards.json. When the same state is shared between a host and a Docker sandbox — where the home directory differs but home-relative mounts like ~/src and ~/.cagent are mirrored — those absolute paths don't exist inside the container. The result is that tmux's -c flag silently falls back to $HOME, and agents fail with --worktree requires /home/agent to be inside a git repository.

The fix introduces contractHome/expandHome helpers in pkg/board/paths.go that persist all repo and worktree paths in ~-contracted form and expand them on load relative to the current home. Old absolute-path state files remain loadable; after one run the state is rewritten in the portable form. On top of that, CreateCard now does a fast isGitRepo check before accepting a path, so the failure is a clear error at card-creation time rather than a cryptic tmux/worktree message later. The isGitRepo check also rejects bare repos by inspecting command output.

A second pass hardened the edge cases found in review: contractHome is a no-op when the home directory cannot be determined (avoiding relative-path corruption), null entries in cards.json no longer cause a panic on load, and the isGitRepo logic was tightened. Tests were added for contractHome/expandHome, the store round-trip, and the card-creation guard; the full test suite, build, and linter all pass.

@dgageot dgageot requested a review from a team as a code owner July 7, 2026 13:17

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟡 NEEDS ATTENTION

The path-contraction/expansion round-trip (contractHome/expandHome) is correct, the null-entry guard in the store is correct, and the overall approach is sound. Two issues were found in the new isGitRepo pre-check added to CreateCard.

Comment thread pkg/board/app.go
// 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] isGitRepo silently passes when project.Path is empty, running git in the process's cwd

CreateCard calls isGitRepo(a.ctx, project.Path) without first validating that project.Path is non-empty. In Go's os/exec, setting cmd.Dir = "" causes the subprocess to inherit the parent's working directory — so if the board process happens to be running inside a git repository, isGitRepo("") returns true, card creation succeeds, and a card with no valid path is committed to state.

An empty project.Path can arise if expandHome is called on a "" stored value (it returns "" unchanged), or if a caller constructs a Project struct without setting Path.

The companion AddProject already guards against this via normalizeProjectPath, which rejects empty paths explicitly. CreateCard needs a matching guard:

if strings.TrimSpace(project.Path) == "" {
    return nil, errors.New("project path is required")
}

This guard should be placed before the isGitRepo call (line 241).

Comment thread pkg/board/app.go
// 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[LOW] Context cancellation or missing git binary produces misleading "not a git repository" error

isGitRepo discards the underlying exec error and returns only a bool. If a.ctx is cancelled (e.g. during app shutdown) or git is not on PATH, the function returns false, and CreateCard reports:

"project path X is not a git repository in this environment; re-add the project here"

…even though the real cause is an execution failure, not an invalid path. This message guides the user to remove and re-add the project, which won't help.

In practice a.ctx is only cancelled at shutdown and git is a hard dependency of this tool, so this is a low-impact UX edge case. Still, distinguishing execution errors from "path is not a repo" would make the error actionable. One option: have isGitRepo return (bool, error) and check errors.Is(err, context.Canceled) before emitting the path-specific message.

@dgageot dgageot merged commit dabd716 into docker:main Jul 7, 2026
11 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants