From b266db5069f4bb837cceb596450de0f2c8990ce0 Mon Sep 17 00:00:00 2001
From: Snider <snider@host.uk.com>
Date: Sun, 22 Mar 2026 03:45:50 +0000
Subject: [PATCH] fix(pkg): address code review findings

- Fix import ordering in verify.go and remote_client.go (stdlib before third-party)
- Convert os.MkdirAll to fs.EnsureDir in prep.go
- Preserve underlying error in !r.OK branches (writeStatus, writePlan, planDelete, planList, resume)

Co-Authored-By: Virgil <virgil@lethean.io>
---
 pkg/agentic/plan.go          | 12 ++++++++----
 pkg/agentic/prep.go          |  4 ++--
 pkg/agentic/remote_client.go |  2 +-
 pkg/agentic/resume.go        |  3 ++-
 pkg/agentic/status.go        |  3 ++-
 pkg/agentic/verify.go        |  2 +-
 6 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/pkg/agentic/plan.go b/pkg/agentic/plan.go
index c0971dd..bd5bb1b 100644
--- a/pkg/agentic/plan.go
+++ b/pkg/agentic/plan.go
@@ -262,7 +262,8 @@ func (s *PrepSubsystem) planDelete(_ context.Context, _ *mcp.CallToolRequest, in
 	}
 
 	if r := fs.Delete(path); !r.OK {
-		return nil, PlanDeleteOutput{}, core.E("planDelete", "failed to delete plan", nil)
+		err, _ := r.Value.(error)
+		return nil, PlanDeleteOutput{}, core.E("planDelete", "failed to delete plan", err)
 	}
 
 	return nil, PlanDeleteOutput{
@@ -274,7 +275,8 @@ func (s *PrepSubsystem) planDelete(_ context.Context, _ *mcp.CallToolRequest, in
 func (s *PrepSubsystem) planList(_ context.Context, _ *mcp.CallToolRequest, input PlanListInput) (*mcp.CallToolResult, PlanListOutput, error) {
 	dir := PlansRoot()
 	if r := fs.EnsureDir(dir); !r.OK {
-		return nil, PlanListOutput{}, core.E("planList", "failed to access plans directory", nil)
+		err, _ := r.Value.(error)
+		return nil, PlanListOutput{}, core.E("planList", "failed to access plans directory", err)
 	}
 
 	entries, err := os.ReadDir(dir)
@@ -368,7 +370,8 @@ func readPlan(dir, id string) (*Plan, error) {
 
 func writePlan(dir string, plan *Plan) (string, error) {
 	if r := fs.EnsureDir(dir); !r.OK {
-		return "", core.E("writePlan", "failed to create plans directory", nil)
+		err, _ := r.Value.(error)
+		return "", core.E("writePlan", "failed to create plans directory", err)
 	}
 
 	path := planPath(dir, plan.ID)
@@ -378,7 +381,8 @@ func writePlan(dir string, plan *Plan) (string, error) {
 	}
 
 	if r := fs.Write(path, string(data)); !r.OK {
-		return "", core.E("writePlan", "failed to write plan", nil)
+		err, _ := r.Value.(error)
+		return "", core.E("writePlan", "failed to write plan", err)
 	}
 	return path, nil
 }
diff --git a/pkg/agentic/prep.go b/pkg/agentic/prep.go
index 795ef81..8d27da3 100644
--- a/pkg/agentic/prep.go
+++ b/pkg/agentic/prep.go
@@ -169,8 +169,8 @@ func (s *PrepSubsystem) prepWorkspace(ctx context.Context, _ *mcp.CallToolReques
 	// kb/ and specs/ will be created inside src/ after clone
 
 	// Ensure workspace directory exists
-	if err := os.MkdirAll(wsDir, 0755); err != nil {
-		return nil, PrepOutput{}, core.E("prep", "failed to create workspace dir", err)
+	if r := fs.EnsureDir(wsDir); !r.OK {
+		return nil, PrepOutput{}, core.E("prep", "failed to create workspace dir", nil)
 	}
 
 	out := PrepOutput{WorkspaceDir: wsDir}
diff --git a/pkg/agentic/remote_client.go b/pkg/agentic/remote_client.go
index c5af7b9..3f64aa4 100644
--- a/pkg/agentic/remote_client.go
+++ b/pkg/agentic/remote_client.go
@@ -3,7 +3,6 @@
 package agentic
 
 import (
-	core "dappco.re/go/core"
 	"bufio"
 	"bytes"
 	"context"
@@ -12,6 +11,7 @@ import (
 	"net/http"
 	"strings"
 
+	core "dappco.re/go/core"
 )
 
 // mcpInitialize performs the MCP initialize handshake over Streamable HTTP.
diff --git a/pkg/agentic/resume.go b/pkg/agentic/resume.go
index 20a54f7..35a498f 100644
--- a/pkg/agentic/resume.go
+++ b/pkg/agentic/resume.go
@@ -71,7 +71,8 @@ func (s *PrepSubsystem) resume(ctx context.Context, _ *mcp.CallToolRequest, inpu
 		answerPath := filepath.Join(srcDir, "ANSWER.md")
 		content := fmt.Sprintf("# Answer\n\n%s\n", input.Answer)
 		if r := fs.Write(answerPath, content); !r.OK {
-			return nil, ResumeOutput{}, core.E("resume", "failed to write ANSWER.md", nil)
+			err, _ := r.Value.(error)
+			return nil, ResumeOutput{}, core.E("resume", "failed to write ANSWER.md", err)
 		}
 	}
 
diff --git a/pkg/agentic/status.go b/pkg/agentic/status.go
index e5e4036..a575d4f 100644
--- a/pkg/agentic/status.go
+++ b/pkg/agentic/status.go
@@ -56,7 +56,8 @@ func writeStatus(wsDir string, status *WorkspaceStatus) error {
 		return err
 	}
 	if r := fs.Write(filepath.Join(wsDir, "status.json"), string(data)); !r.OK {
-		return core.E("writeStatus", "failed to write status", nil)
+		err, _ := r.Value.(error)
+		return core.E("writeStatus", "failed to write status", err)
 	}
 	return nil
 }
diff --git a/pkg/agentic/verify.go b/pkg/agentic/verify.go
index a327ea8..615b296 100644
--- a/pkg/agentic/verify.go
+++ b/pkg/agentic/verify.go
@@ -3,7 +3,6 @@
 package agentic
 
 import (
-	core "dappco.re/go/core"
 	"bytes"
 	"context"
 	"encoding/json"
@@ -15,6 +14,7 @@ import (
 	"strings"
 	"time"
 
+	core "dappco.re/go/core"
 )
 
 // autoVerifyAndMerge runs inline tests (fast gate) and merges if they pass.