Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions .design/project-log/2026-06-03-fix-workspace-file-browser-path.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# Fix: Workspace file browser path resolution (Issue #130)

**Date:** 2026-06-03
**PR:** #132
**Issue:** #130

## Problem

The Hub UI workspace file browser was showing the wrong directory contents. The `hubManagedProjectPath()` function resolved workspace paths to `~/.scion/projects/<slug>/` instead of `~/.scion/groves/<slug>/`.

The three relevant directories per project:
1. `~/.scion/groves/<slug>/` — actual git checkout, mounted as `/workspace` in agents (correct target)
2. `~/.scion/projects/<slug>/` — project metadata + Telegram plugin downloads (what the UI was showing)
3. `~/.scion/grove-configs/<slug>__<uuid>/` — agent configs and shared-dirs

## Root Cause

`hubManagedProjectPath()` checked `projects/` first, fell back to `groves/`, and defaulted to `projects/`. This was backwards — the git checkout (what agents actually work in) lives under `groves/`.

## Fix

Reversed the lookup priority in `hubManagedProjectPath()`:
1. Check `groves/<slug>` first (preferred — actual workspace)
2. Fall back to `projects/<slug>` (backward compatibility)
3. Default to `groves/<slug>` when neither has content

## Files Changed

- `pkg/hub/handlers.go` — reversed path resolution priority
- `pkg/hub/handlers_project_test.go` — updated existing test, added 3 new test cases

## Observations

- The `pkg/config` test suite has a pre-existing failure (`TestEnsureHubReady_GlobalFallbackWithHubEnabled`) caused by leaked `SCION_*` environment variables in the container. This is unrelated to this change and passes when those env vars are cleared.
21 changes: 13 additions & 8 deletions pkg/hub/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -3763,21 +3763,26 @@ func (s *Server) createProjectMembersGroupAndPolicy(ctx context.Context, project
}

// hubManagedProjectPath returns the filesystem path for a hub-managed project workspace.
// It prefers groves/<slug> (the actual git checkout mounted as /workspace in agents)
// over projects/<slug> (which only contains project metadata).
func hubManagedProjectPath(slug string) (string, error) {
if slug == "" {
return "", fmt.Errorf("project slug must not be empty")
}
globalDir, err := config.GetGlobalDir()
Comment on lines 3768 to 3772
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

An empty slug passed to hubManagedProjectPath could resolve to the parent directory (e.g., ~/.scion/groves or ~/.scion/projects). Since hasWorkspaceContent checks if the directory contains any files other than .scion or shared-dirs, a parent directory containing other projects would return true, causing the function to incorrectly return the parent directory. This could lead to catastrophic operations (like directory deletion or overwriting) on the entire parent directory. Adding a defensive check for an empty slug prevents this.

Suggested change
func hubManagedProjectPath(slug string) (string, error) {
globalDir, err := config.GetGlobalDir()
func hubManagedProjectPath(slug string) (string, error) {
if slug == "" {
return "", errors.New("project slug cannot be empty")
}
globalDir, err := config.GetGlobalDir()

if err != nil {
return "", fmt.Errorf("failed to get global dir: %w", err)
}
newPath := filepath.Join(globalDir, "projects", slug)
if hasWorkspaceContent(newPath) {
return newPath, nil
grovesPath := filepath.Join(globalDir, "groves", slug)
if hasWorkspaceContent(grovesPath) {
return grovesPath, nil
}
oldPath := filepath.Join(globalDir, "groves", slug)
if hasWorkspaceContent(oldPath) {
return oldPath, nil
projectsPath := filepath.Join(globalDir, "projects", slug)
if hasWorkspaceContent(projectsPath) {
return projectsPath, nil
}
// Neither has content — return new path (will be created on demand)
return newPath, nil
// Neither has content — return groves path (will be created on demand)
return grovesPath, nil
}

// hasWorkspaceContent returns true if dir exists and contains meaningful
Expand Down
69 changes: 60 additions & 9 deletions pkg/hub/handlers_project_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,32 +44,83 @@ func TestHubManagedProjectPath(t *testing.T) {
homeDir, err := os.UserHomeDir()
require.NoError(t, err)

expected := filepath.Join(homeDir, ".scion", "projects", "my-test-project")
// Default (no content in either dir) should resolve to groves/ —
// that's where the actual git checkout lives (mounted as /workspace in agents).
expected := filepath.Join(homeDir, ".scion", "groves", "my-test-project")
assert.Equal(t, expected, path)
}

func TestHubManagedProjectPath_EmptyProjectsFallsBackToGroves(t *testing.T) {
func TestHubManagedProjectPath_PrefersGrovesOverProjects(t *testing.T) {
// Use a temp directory as HOME to avoid polluting real ~/.scion
tmpHome := t.TempDir()
t.Setenv("HOME", tmpHome)

slug := "empty-projects-grove"
slug := "both-dirs-exist"
globalDir := filepath.Join(tmpHome, ".scion")

// Create projects/{slug} with only infrastructure dirs (no real content)
// Create both directories with workspace content
projectsDir := filepath.Join(globalDir, "projects", slug)
require.NoError(t, os.MkdirAll(filepath.Join(projectsDir, "shared-dirs"), 0755))
require.NoError(t, os.MkdirAll(filepath.Join(projectsDir, ".scion"), 0755))
require.NoError(t, os.MkdirAll(projectsDir, 0755))
require.NoError(t, os.WriteFile(filepath.Join(projectsDir, "metadata.json"), []byte("{}"), 0644))

// Create groves/{slug} with actual workspace content
grovesDir := filepath.Join(globalDir, "groves", slug)
require.NoError(t, os.MkdirAll(grovesDir, 0755))
require.NoError(t, os.WriteFile(filepath.Join(grovesDir, "README.md"), []byte("# workspace"), 0644))

// hubManagedProjectPath should fall back to groves/ since projects/ has no real content
// hubManagedProjectPath should prefer groves/ — that's the actual git checkout
path, err := hubManagedProjectPath(slug)
require.NoError(t, err)
assert.Equal(t, grovesDir, path, "should prefer groves path over projects path")
}

func TestHubManagedProjectPath_FallsBackToProjectsWhenGrovesEmpty(t *testing.T) {
// Use a temp directory as HOME to avoid polluting real ~/.scion
tmpHome := t.TempDir()
t.Setenv("HOME", tmpHome)

slug := "groves-empty-projects-has-content"
globalDir := filepath.Join(tmpHome, ".scion")

// Create groves/{slug} with only infrastructure dirs (no real content)
grovesDir := filepath.Join(globalDir, "groves", slug)
require.NoError(t, os.MkdirAll(filepath.Join(grovesDir, ".scion"), 0755))

// Create projects/{slug} with actual workspace content
projectsDir := filepath.Join(globalDir, "projects", slug)
require.NoError(t, os.MkdirAll(projectsDir, 0755))
require.NoError(t, os.WriteFile(filepath.Join(projectsDir, "README.md"), []byte("# workspace"), 0644))

// hubManagedProjectPath should fall back to projects/ since groves/ has no real content
path, err := hubManagedProjectPath(slug)
require.NoError(t, err)
assert.Equal(t, projectsDir, path, "should fall back to projects path when groves dir only contains infrastructure dirs")
}

func TestHubManagedProjectPath_DefaultsToGrovesWhenNeitherHasContent(t *testing.T) {
// Use a temp directory as HOME to avoid polluting real ~/.scion
tmpHome := t.TempDir()
t.Setenv("HOME", tmpHome)

slug := "neither-has-content"
globalDir := filepath.Join(tmpHome, ".scion")

// Create both directories with only infrastructure dirs
grovesDir := filepath.Join(globalDir, "groves", slug)
require.NoError(t, os.MkdirAll(filepath.Join(grovesDir, ".scion"), 0755))

projectsDir := filepath.Join(globalDir, "projects", slug)
require.NoError(t, os.MkdirAll(filepath.Join(projectsDir, "shared-dirs"), 0755))

// When neither has content, should default to groves/
path, err := hubManagedProjectPath(slug)
require.NoError(t, err)
assert.Equal(t, grovesDir, path, "should fall back to groves path when projects dir only contains infrastructure dirs")
assert.Equal(t, grovesDir, path, "should default to groves path when neither dir has workspace content")
}
Comment on lines +114 to 118
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

Add a unit test to verify that hubManagedProjectPath returns an error when an empty slug is provided, ensuring the defensive check works as expected.

	// When neither has content, should default to groves/
	path, err := hubManagedProjectPath(slug)
	require.NoError(t, err)
	assert.Equal(t, grovesDir, path, "should default to groves path when neither dir has workspace content")
}

func TestHubManagedProjectPath_EmptySlug(t *testing.T) {
	_, err := hubManagedProjectPath("")
	assert.Error(t, err)
	assert.Contains(t, err.Error(), "project slug cannot be empty")
}


func TestHubManagedProjectPath_EmptySlug(t *testing.T) {
_, err := hubManagedProjectPath("")
require.Error(t, err, "empty slug should return an error")
assert.Contains(t, err.Error(), "slug must not be empty")
}

func TestCreateProject_HubManaged_NoGitRemote(t *testing.T) {
Expand Down