fix(penpal): address Codex review findings

apps/penpal/internal/agents/agents.go

@@ -17,12 +17,21 @@ type Info struct {
 var (
 	mu     sync.RWMutex
 	cached map[string][]Info
+
+	pollMu sync.Mutex
 	stopCh chan struct{}
 )
 
 // StartPolling begins background polling for active agents every 5 seconds.
 func StartPolling() {
+	pollMu.Lock()
+	defer pollMu.Unlock()
+
+	if stopCh != nil {
+		close(stopCh)
+	}
 	stopCh = make(chan struct{})
+	ch := stopCh
 	// Do an initial poll synchronously so the first read has data.
 	result := poll()
 	mu.Lock()
@@ -34,7 +43,7 @@ func StartPolling() {
 		defer ticker.Stop()
 		for {
 			select {
-			case <-stopCh:
+			case <-ch:
 				return
 			case <-ticker.C:
 				result := poll()
@@ -48,8 +57,12 @@ func StartPolling() {
 
 // StopPolling stops the background polling goroutine.
 func StopPolling() {
+	pollMu.Lock()
+	defer pollMu.Unlock()
+
 	if stopCh != nil {
 		close(stopCh)
+		stopCh = nil
 	}
 }
 

apps/penpal/internal/agents/manager.go

@@ -127,15 +127,22 @@ func (m *Manager) Start(projectName string) (*Agent, error) {
 		contextWindow: 200000, // default for opus, refined when result arrives
 	}
 
-	// Parse NDJSON stream in background, writing through to log file
-	go agent.parseStream(stdout, logFile)
+	// Parse NDJSON stream in background, writing through to log file.
+	// streamDone is closed when parseStream finishes so we can safely
+	// close logFile only after all writes complete.
+	streamDone := make(chan struct{})
+	go func() {
+		agent.parseStream(stdout, logFile)
+		close(streamDone)
+	}()
 	m.agents[projectName] = agent
 
 	log.Printf("Agent started for %s (PID %d)", projectName, agent.PID)
 
 	// Monitor process exit in background
 	go func() {
 		agent.exitErr = cmd.Wait()
+		<-streamDone // wait for parseStream to finish before closing log
 		logFile.Close()
 		os.Remove(mcpConfigPath)
 		close(agent.done)

apps/penpal/internal/comments/operations.go

@@ -135,6 +135,13 @@ func (s *Store) ReopenThread(projectName, filePath, threadID string) error {
 // project and returns all threads with status "open" across all files.
 // Returned file paths are relative to the project root (e.g., "thoughts/shared/plans/foo.md").
 func (s *Store) ListOpenThreads(projectName string) ([]ThreadWithFile, error) {
+	return s.ListThreadsByStatus(projectName, "open")
+}
+
+// ListThreadsByStatus walks the .penpal/comments/ directory for the given
+// project and returns threads matching the given status filter.
+// An empty status returns all threads regardless of status.
+func (s *Store) ListThreadsByStatus(projectName, status string) ([]ThreadWithFile, error) {
 	project := s.cache.FindProject(projectName)
 	if project == nil {
 		return nil, fmt.Errorf("project not found: %s", projectName)
@@ -172,7 +179,7 @@ func (s *Store) ListOpenThreads(projectName string) ([]ThreadWithFile, error) {
 		filePath := strings.TrimSuffix(rel, ".json")
 
 		for _, t := range fc.Threads {
-			if t.Status == "open" {
+			if status == "" || t.Status == status {
 				results = append(results, ThreadWithFile{
 					Thread:   t,
 					FilePath: filePath,

apps/penpal/internal/mcpserver/tools.go

@@ -81,9 +81,13 @@ func registerTools(server *mcp.Server, store *comments.Store, c *cache.Cache) {
 		}
 
 		if input.Path == "" {
-			// List open threads across the entire project
+			// List threads across the entire project, filtered by status
 			store.RecordHeartbeat(input.Project, "")
-			threads, err := store.ListOpenThreads(input.Project)
+			status := input.Status
+			if status == "" {
+				status = "open"
+			}
+			threads, err := store.ListThreadsByStatus(input.Project, status)
 			if err != nil {
 				return nil, nil, err
 			}
@@ -94,7 +98,7 @@ func registerTools(server *mcp.Server, store *comments.Store, c *cache.Cache) {
 					store.RecordHeartbeat(input.Project, t.FilePath)
 					seen[t.FilePath] = true
 				}
-				if len(t.Comments) > 0 && t.Comments[len(t.Comments)-1].Role == "human" {
+				if t.Status == "open" && len(t.Comments) > 0 && t.Comments[len(t.Comments)-1].Role == "human" {
 					store.SetTyping(input.Project, t.FilePath, t.ID)
 				}
 			}

apps/penpal/internal/server/manage.go

@@ -276,7 +276,7 @@ func (s *Server) handleAddSource(w http.ResponseWriter, r *http.Request) {
 	// Check what the path points to
 	absPath := filepath.Join(project.Path, req.Path)
 	resolved, err := filepath.Abs(absPath)
-	if err != nil || !isSubpath(project.Path, resolved) {
+	if err != nil || (resolved != filepath.Clean(project.Path) && !isSubpath(project.Path, resolved)) {
 		http.Error(w, "invalid path", http.StatusBadRequest)
 		return
 	}

apps/penpal/internal/server/pathutil.go

@@ -5,11 +5,15 @@ import (
 	"strings"
 )
 
-// isSubpath reports whether child is inside (or equal to) parent after
+// isSubpath reports whether child is strictly inside parent after
 // cleaning both paths. It prevents path-traversal attacks by ensuring
 // the resolved child starts with the parent directory prefix.
+// Returns false when child equals parent (e.g. path=".").
 func isSubpath(parent, child string) bool {
-	parent = filepath.Clean(parent) + string(filepath.Separator)
-	child = filepath.Clean(child) + string(filepath.Separator)
-	return strings.HasPrefix(child, parent)
+	parent = filepath.Clean(parent)
+	child = filepath.Clean(child)
+	if parent == child {
+		return false
+	}
+	return strings.HasPrefix(child, parent+string(filepath.Separator))
 }

apps/penpal/internal/server/pathutil_test.go

@@ -10,7 +10,7 @@ func TestIsSubpath(t *testing.T) {
 	}{
 		{"/a/b", "/a/b/c", true},
 		{"/a/b", "/a/b/c/d", true},
-		{"/a/b", "/a/b", true},
+		{"/a/b", "/a/b", false},
 		{"/a/b", "/a/bc", false},
 		{"/a/b", "/a", false},
 		{"/a/b", "/a/b/../c", false},