Skip to content
Open
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
23 changes: 23 additions & 0 deletions .semgrep/architecture.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,29 @@ rules:
category: architecture
violation: identity-leakage

- id: daemon-no-worktree-path-to-ensure-server
patterns:
- pattern-either:
- pattern: $S.ensureOpenCodeServerRunning($RESULT.WorktreePath)
- pattern: $S.ensureOpenCodeServerRunning(worktreePath)
paths:
include:
- /internal/daemon/
exclude:
- /internal/daemon/*_test.go
message: >
Do not pass worktree paths to ensureOpenCodeServerRunning().
This function uses the path as a registry key - worktree paths create
duplicate servers for the same repo. Pass the main repo root
(req.ProjectRoot or resolve via git.FindMainRepoRoot).
Note: ensureOpenCodeServerRunning also self-normalizes as defense-in-depth,
but call sites should pass the correct value.
severity: ERROR
languages: [go]
metadata:
category: architecture
violation: server-identity

# ---------------------------------------------------------------------------
# 7. Transport contract fidelity: prevent Message field overloading
# ---------------------------------------------------------------------------
Expand Down
38 changes: 34 additions & 4 deletions internal/daemon/managed_servers_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,10 @@ func (s *SocketServer) persistManagedServerStart(srv *managedServer) error {
return nil
}

projectRoot := s.normalizeProjectRoot(srv.ProjectRoot)

record := managedServerRecord{
ProjectRoot: srv.ProjectRoot,
ProjectRoot: projectRoot,
PID: serverPID(srv),
Port: srv.Port,
LogPath: srv.LogPath,
Expand All @@ -277,8 +279,18 @@ func (s *SocketServer) deleteManagedServerRecord(projectRoot string) {
return
}

if err := s.managedServerStore.Delete(projectRoot); err != nil && s.logger != nil {
s.logger.Printf("warning: failed to delete managed server record for %s: %v", projectRoot, err)
normalizedProjectRoot := s.normalizeProjectRoot(projectRoot)
if normalizedProjectRoot != "" {
if err := s.managedServerStore.Delete(normalizedProjectRoot); err != nil && s.logger != nil {
s.logger.Printf("warning: failed to delete managed server record for %s: %v", normalizedProjectRoot, err)
}
}

// Delete legacy pre-normalized keys as best-effort cleanup.
if normalizedProjectRoot != projectRoot {
if err := s.managedServerStore.Delete(projectRoot); err != nil && s.logger != nil {
s.logger.Printf("warning: failed to delete legacy managed server record for %s: %v", projectRoot, err)
}
}
}

Expand All @@ -287,6 +299,7 @@ func (s *SocketServer) updateManagedServerHealth(projectRoot string, at time.Tim
return
}

projectRoot = s.normalizeProjectRoot(projectRoot)
if err := s.managedServerStore.UpdateLastHealthy(projectRoot, at); err != nil && s.logger != nil {
s.logger.Printf("warning: failed to update managed server health for %s: %v", projectRoot, err)
}
Expand Down Expand Up @@ -314,8 +327,25 @@ func (s *SocketServer) reconcileManagedServersOnStartup() error {
}

func (s *SocketServer) reconcileManagedServerRecord(record managedServerRecord, adoptedPorts map[int]string) {
legacyProjectRoot := record.ProjectRoot
record.ProjectRoot = s.normalizeProjectRoot(record.ProjectRoot)
if record.ProjectRoot == "" {
record.ProjectRoot = legacyProjectRoot
}

// Migrate legacy worktree-path keys to normalized repo-root keys.
if legacyProjectRoot != "" && legacyProjectRoot != record.ProjectRoot && s.managedServerStore != nil {
if err := s.managedServerStore.Upsert(record); err != nil {
if s.logger != nil {
s.logger.Printf("warning: failed to migrate managed server record from %s to %s: %v", legacyProjectRoot, record.ProjectRoot, err)
}
} else if err := s.managedServerStore.Delete(legacyProjectRoot); err != nil && s.logger != nil {
s.logger.Printf("warning: failed to remove legacy managed server record for %s: %v", legacyProjectRoot, err)
}
}

if record.ProjectRoot == "" || record.PID <= 0 || record.Port <= 0 {
s.deleteManagedServerRecord(record.ProjectRoot)
s.deleteManagedServerRecord(legacyProjectRoot)
return
}

Expand Down
60 changes: 60 additions & 0 deletions internal/daemon/managed_servers_db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,66 @@ func TestManagedServerStoreCRUD(t *testing.T) {
}
}

func TestPersistManagedServerStartNormalizesProjectRoot(t *testing.T) {
setupManagedServerDBEnv(t)

repoRoot, worktreePath, _ := createMainRepoWithTwoWorktrees(t)

store, err := newManagedServerStore(xdg.DaemonDBPath())
if err != nil {
t.Fatalf("newManagedServerStore() error = %v", err)
}
defer store.Close()

server := NewSocketServer(nil, log.New(io.Discard, "", 0))
server.managedServerStore = store
expectedProjectRoot := server.normalizeProjectRoot(repoRoot)

if err := server.persistManagedServerStart(&managedServer{
ProjectRoot: worktreePath,
PID: 12345,
Port: 4101,
LogPath: "/tmp/opencode-normalized.log",
StartTime: time.Now().Add(-5 * time.Second),
}); err != nil {
t.Fatalf("persistManagedServerStart() error = %v", err)
}

rows, err := store.List()
if err != nil {
t.Fatalf("List() error = %v", err)
}
if len(rows) != 1 {
t.Fatalf("expected 1 row, got %d", len(rows))
}
if rows[0].ProjectRoot != expectedProjectRoot {
t.Fatalf("expected normalized project_root %q, got %q", expectedProjectRoot, rows[0].ProjectRoot)
}

now := time.Now().UTC().Truncate(time.Second)
server.updateManagedServerHealth(worktreePath, now)

rows, err = store.List()
if err != nil {
t.Fatalf("List() error = %v", err)
}
if len(rows) != 1 {
t.Fatalf("expected 1 row after health update, got %d", len(rows))
}
if rows[0].LastHealthy.IsZero() {
t.Fatal("expected non-zero last_healthy after normalized update")
}

server.deleteManagedServerRecord(worktreePath)
rows, err = store.List()
if err != nil {
t.Fatalf("List() error = %v", err)
}
if len(rows) != 0 {
t.Fatalf("expected row deleted via normalized key, got %d rows", len(rows))
}
}

func TestReconcileManagedServersOnStartupAdoptsHealthyServer(t *testing.T) {
setupManagedServerDBEnv(t)

Expand Down
66 changes: 66 additions & 0 deletions internal/daemon/opencode_server_project_root_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
package daemon

import (
"fmt"
"io"
"log"
"net/http"
"net/http/httptest"
"testing"
)

func TestEnsureOpenCodeServerRunningNormalizesWorktreePath(t *testing.T) {
repoRoot, worktreeA, worktreeB := createMainRepoWithTwoWorktrees(t)
server := NewSocketServer(nil, log.New(io.Discard, "", 0))

normalizedRepoRoot := server.normalizeProjectRoot(repoRoot)
if normalizedRepoRoot == "" {
t.Fatal("expected normalized repo root to be non-empty")
}

ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
switch r.URL.Path {
case "/global/health":
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, `{"healthy":true,"version":"test"}`)
case "/project/current":
w.Header().Set("Content-Type", "application/json")
_, _ = io.WriteString(w, fmt.Sprintf(`{"id":"proj","worktree":%q,"sandboxes":[]}`, normalizedRepoRoot))
default:
http.NotFound(w, r)
}
}))
defer ts.Close()

port := getPortFromURL(t, ts.URL)

server.openCodeServers[normalizedRepoRoot] = &managedServer{
ProjectRoot: normalizedRepoRoot,
Port: port,
WaitResult: make(chan error),
}

gotA, err := server.ensureOpenCodeServerRunning(worktreeA)
if err != nil {
t.Fatalf("ensureOpenCodeServerRunning(worktreeA) error = %v", err)
}
gotB, err := server.ensureOpenCodeServerRunning(worktreeB)
if err != nil {
t.Fatalf("ensureOpenCodeServerRunning(worktreeB) error = %v", err)
}

if gotA != port || gotB != port {
t.Fatalf("expected both worktrees to resolve to port %d, got %d and %d", port, gotA, gotB)
}
if len(server.openCodeServers) != 1 {
t.Fatalf("expected one server entry, got %d", len(server.openCodeServers))
}
if _, ok := server.openCodeServers[normalizedRepoRoot]; !ok {
t.Fatalf("expected server map entry keyed by repo root %q", normalizedRepoRoot)
}

gotPort := server.getOpenCodeServerPort(worktreeB)
if gotPort != port {
t.Fatalf("getOpenCodeServerPort(worktreeB) = %d, want %d", gotPort, port)
}
}
48 changes: 48 additions & 0 deletions internal/daemon/project_root_test_helpers_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package daemon

import (
"os"
"os/exec"
"path/filepath"
"strings"
"testing"
)

func createMainRepoWithTwoWorktrees(t *testing.T) (repoRoot string, worktreeA string, worktreeB string) {
t.Helper()

baseDir := t.TempDir()
repoRoot = filepath.Join(baseDir, "repo")
if err := os.MkdirAll(repoRoot, 0755); err != nil {
t.Fatalf("failed to create repo dir: %v", err)
}

runGit(t, repoRoot, "init")
runGit(t, repoRoot, "config", "user.email", "orch-tests@example.com")
runGit(t, repoRoot, "config", "user.name", "Orch Tests")

readmePath := filepath.Join(repoRoot, "README.md")
if err := os.WriteFile(readmePath, []byte("test\n"), 0644); err != nil {
t.Fatalf("failed to write README.md: %v", err)
}
runGit(t, repoRoot, "add", "README.md")
runGit(t, repoRoot, "commit", "-m", "initial commit")

worktreeA = filepath.Join(baseDir, "worktree-a")
worktreeB = filepath.Join(baseDir, "worktree-b")
runGit(t, repoRoot, "worktree", "add", "-b", "feature-a", worktreeA, "HEAD")
runGit(t, repoRoot, "worktree", "add", "-b", "feature-b", worktreeB, "HEAD")

return repoRoot, worktreeA, worktreeB
}

func runGit(t *testing.T, dir string, args ...string) {
t.Helper()

cmd := exec.Command("git", args...)
cmd.Dir = dir
out, err := cmd.CombinedOutput()
if err != nil {
t.Fatalf("git %s failed: %v\n%s", strings.Join(args, " "), err, string(out))
}
}
4 changes: 3 additions & 1 deletion internal/daemon/proto_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1400,8 +1400,10 @@ func (s *SocketServer) handleProtoEnsureOpenCodeServer(req *orchpb.EnsureOpenCod
return errorResponse(fmt.Sprintf("failed to ensure opencode server: %v", err))
}

projectRoot := s.normalizeProjectRoot(req.ProjectRoot)

s.openCodeServersMu.RLock()
srv, exists := s.openCodeServers[req.ProjectRoot]
srv, exists := s.openCodeServers[projectRoot]
alreadyRunning := exists && srv != nil
s.openCodeServersMu.RUnlock()

Expand Down
Loading
Loading