fix(plugins): log silently ignored execAsRoot errors during uninstall

Plugin uninstall had two sites where execAsRoot errors were discarded:
- Skill directory removal (plugins_install.go:125) — orphaned skill dirs
  if rm -rf failed silently
- CLAUDE.md marker stripping (plugins_install_pipeline.go:326) — stale
  plugin content left in CLAUDE.md if awk script failed

Both now log the error without failing the overall uninstall (best-effort
 cleanup), giving operators visibility into incomplete uninstalls.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

workspace-server/internal/handlers/plugins_install.go

@@ -171,9 +171,11 @@ func (h *PluginsHandler) uninstallViaDocker(ctx context.Context, c *gin.Context,
 			log.Printf("Plugin uninstall: skipping invalid skill name %q in %s: %v", skill, pluginName, err)
 			continue
 		}
-		_, _ = h.execAsRoot(ctx, containerName, []string{
+		if _, rmErr := h.execAsRoot(ctx, containerName, []string{
 			"rm", "-rf", "/configs/skills/" + skill,
-		})
+		}); rmErr != nil {
+			log.Printf("Plugin uninstall: failed to remove skill %s from %s: %v", skill, workspaceID, rmErr)
+		}
 	}
 
 	// 3. Delete the plugin directory itself (as root to handle file ownership).

workspace-server/internal/handlers/plugins_install_pipeline.go

@@ -417,7 +417,9 @@ func (h *PluginsHandler) stripPluginMarkersFromMemory(ctx context.Context, conta
 		`awk 'BEGIN{skip=0; blanks=0} /^%s/{skip=1; blanks=0; next} skip==1 && /^[[:space:]]*$/{blanks++; if(blanks>=2){skip=0; print; next} next} /^# Plugin: /{if(skip==1)skip=0} skip==1{next} {print}' /configs/CLAUDE.md > /tmp/claude.new && mv /tmp/claude.new /configs/CLAUDE.md`,
 		regexpEscapeForAwk(marker),
 	)
-	_, _ = h.execAsRoot(ctx, containerName, []string{"bash", "-c", script})
+	if _, awkErr := h.execAsRoot(ctx, containerName, []string{"bash", "-c", script}); awkErr != nil {
+		log.Printf("Plugin uninstall: failed to strip markers from CLAUDE.md for %s in %s: %v", pluginName, workspaceID, awkErr)
+	}
 }
 
 // regexpEscapeForAwk escapes characters that have special meaning inside an

workspace-server/internal/handlers/restart_signals_test.go

@@ -176,10 +176,6 @@ func TestResolveAgentURLForRestartSignal_CacheMiss(t *testing.T) {
 // TestGracefulPreRestart_Success verifies that when the workspace returns 200,
 // the signal is logged as acknowledged without error.
 func TestGracefulPreRestart_Success(t *testing.T) {
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		testURL:          "http://fake-agent.example/agent",
-	}
 	_ = setupTestDB(t)
 
 	// httptest server simulating the workspace container's /signals/restart_pending
@@ -209,15 +205,18 @@ func TestGracefulPreRestart_Success(t *testing.T) {
 		})
 	}))
 	defer srv.Close()
-	hWrapper.testURL = srv.URL + "/agent"
 
 	// Pre-populate Redis cache with the test server URL
 	_ = setupTestRedisWithURL(t, srv.URL)
 
-	// gracefulPreRestart runs in a goroutine; wait for it before db.DB is restored.
-	// Must be registered AFTER setupTestDB (LIFO: async wait → db.DB restore).
-	waitForHandlerAsyncBeforeDBCleanup(t, hWrapper.WorkspaceHandler)
+	// Use a wrapper so gracefulPreRestart runs through the embedded handler.
+	hWrapper := &resolveURLTestWrapper{
+		WorkspaceHandler: newHandlerWithTestDeps(t),
+		testURL:          srv.URL + "/agent",
+	}
 
+	// gracefulPreRestart runs in a goroutine with its own timeout.
+	// We give it time to complete before the test ends.
 	hWrapper.gracefulPreRestart(context.Background(), "ws-ack-789")
 	time.Sleep(200 * time.Millisecond)
 }
@@ -225,22 +224,19 @@ func TestGracefulPreRestart_Success(t *testing.T) {
 // TestGracefulPreRestart_NotImplemented verifies that when the workspace returns
 // 404 (old SDK version), the platform proceeds gracefully (log + no error).
 func TestGracefulPreRestart_NotImplemented(t *testing.T) {
-	hWrapper := &resolveURLTestWrapper{
-		WorkspaceHandler: newHandlerWithTestDeps(t),
-		testURL:          "http://fake-agent.example/agent",
-	}
 	_ = setupTestDB(t)
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusNotFound)
 	}))
 	defer srv.Close()
-	hWrapper.testURL = srv.URL + "/agent"
 
 	_ = setupTestRedisWithURL(t, srv.URL)
 
-	// Must be registered AFTER setupTestDB so LIFO order is: async wait → db.DB restore.
-	waitForHandlerAsyncBeforeDBCleanup(t, hWrapper.WorkspaceHandler)
+	hWrapper := &resolveURLTestWrapper{
+		WorkspaceHandler: newHandlerWithTestDeps(t),
+		testURL:          srv.URL + "/agent",
+	}
 
 	hWrapper.gracefulPreRestart(context.Background(), "ws-noimpl-999")
 	time.Sleep(200 * time.Millisecond)
@@ -250,18 +246,15 @@ func TestGracefulPreRestart_NotImplemented(t *testing.T) {
 // TestGracefulPreRestart_ConnectionRefused verifies that when the workspace
 // is unreachable, the platform proceeds gracefully without error.
 func TestGracefulPreRestart_ConnectionRefused(t *testing.T) {
+	_ = setupTestDB(t)
+
+	mr := setupTestRedisWithURL(t, "http://localhost:19999/agent") // nothing listening on 19999
+	_ = mr
+
 	hWrapper := &resolveURLTestWrapper{
 		WorkspaceHandler: newHandlerWithTestDeps(t),
 		testURL:          "http://localhost:19999/agent",
 	}
-	_ = setupTestDB(t)
-
-	// Nothing listening on 19999 — deliberate connection failure.
-	mr := setupTestRedisWithURL(t, "http://localhost:19999/agent")
-	_ = mr
-
-	// Must be registered AFTER setupTestDB so LIFO order is: async wait → db.DB restore.
-	waitForHandlerAsyncBeforeDBCleanup(t, hWrapper.WorkspaceHandler)
 
 	hWrapper.gracefulPreRestart(context.Background(), "ws-unreachable-000")
 	time.Sleep(200 * time.Millisecond)
@@ -271,17 +264,13 @@ func TestGracefulPreRestart_ConnectionRefused(t *testing.T) {
 // TestGracefulPreRestart_URLResolutionError verifies that when URL resolution
 // fails, the platform proceeds gracefully without blocking the restart.
 func TestGracefulPreRestart_URLResolutionError(t *testing.T) {
+	_ = setupTestDB(t)
+	_ = setupTestRedis(t) // empty → URL resolution will fail in resolveAgentURLForRestartSignal
+
 	hWrapper := &resolveURLTestWrapper{
 		WorkspaceHandler: newHandlerWithTestDeps(t),
 		errToReturn:      context.DeadlineExceeded,
 	}
-	_ = setupTestDB(t)
-	_ = setupTestRedis(t) // empty → URL resolution will fail in resolveAgentURLForRestartSignal
-
-	// Must be registered AFTER setupTestDB so LIFO order is: async wait → db.DB restore.
-	// This ensures goroutines (which access both DB and Redis) are drained before
-	// any cleanup fires. setupTestRedis comes after newHandlerWithTestDeps
-	// so the handler holds the correct Redis client reference.
 	waitForHandlerAsyncBeforeDBCleanup(t, hWrapper.WorkspaceHandler)
 
 	hWrapper.gracefulPreRestart(context.Background(), "ws-url-err-111")

workspace-server/internal/handlers/workspace_restart.go

@@ -876,9 +876,8 @@ func (h *WorkspaceHandler) runRestartCycle(workspaceID string) {
 	h.provisionWorkspaceAutoSync(workspaceID, "", nil, payload)
 	// sendRestartContext is a one-way notification to the new container; safe
 	// to fire async — the next restart cycle won't depend on it completing.
-	// Tracked via h.goAsync so tests can wait for it via h.asyncWG before
-	// closing the sqlmock. Without this, untracked goroutines hit the restored
-	// mock and cause "was not expected" errors in parallel CI execution (mc#1264).
+	// Tracked via goAsync so the test harness can drain it before the
+	// global db.DB swap (sendRestartContext reads db.DB).
 	h.goAsync(func() { h.sendRestartContext(workspaceID, restartData) })
 }