fix(cli): report leftover markers on every refusal path, never the root marker (MUL-6132)

Two gaps from the previous round's review.

requireTaskLocalConfigRoot is the third path a leftover marker can refuse
through — config show, config set and auth status all call it — and it still
returned a message that named no file. Whether the error was actionable
depended on which command the user happened to run first, which is exactly what
unifying the other two paths was meant to end. It now appends the same suffix
when a marker is the only daemon signal.

The leftover helper also could not tell the two markers apart, because
daemonTaskContextMarkerPath matches on managed_by alone. Running from the
daemon workspaces root therefore described the permanent node-wide guard as a
leftover to delete — advice that closes the hole it exists to cover, aimed at
whoever reads the error, including an agent subprocess that lost its
environment. The helper now requires task identity, defined as agent_id OR
issue_id: a chat task has no issue, so requiring both would misfile its marker
as the root marker, while the root marker has neither field.

chdirWithDaemonTaskMarker was writing the bare managed_by form, which under
this rule models the root marker rather than the workdir marker its tests are
about; it now writes a task-scoped one.

Also corrects two comments the earlier commits left stale: cmd_agent.go still
described an automatic retirement that no longer exists, and
writeSidecarManifest still claimed every caller treats a write failure as
non-fatal, which stopped being true when the in-place path was made fatal.

Co-authored-by: multica-agent <[email protected]>

server/cmd/multica/cmd_agent.go

@@ -406,6 +406,14 @@ func requireTaskLocalConfigRoot() error {
 		return nil
 	}
 	if strings.TrimSpace(os.Getenv(cli.TaskConfigRootEnv)) == "" {
+		// The third refusal path a leftover marker can trigger, alongside
+		// newAPIClient and requireHumanLocalCommand. It has to name the file
+		// too: a user hitting this one through `config show` or `auth status`
+		// is as stuck as one hitting the others, and "which command did you
+		// happen to run first" must not decide whether the error is actionable.
+		if markerPath := leftoverDaemonTaskMarkerPath(); markerPath != "" {
+			return fmt.Errorf("daemon-managed task requires a task-local Multica config root in %s%s", cli.TaskConfigRootEnv, leftoverMarkerSuffix(markerPath))
+		}
 		return fmt.Errorf("daemon-managed task requires a task-local Multica config root in %s%s", cli.TaskConfigRootEnv, daemonPortOnlyContextHint())
 	}
 	return nil
@@ -418,14 +426,13 @@ func requireHumanLocalCommand(command string) error {
 	if !inDaemonTaskIdentityContext() {
 		return nil
 	}
-	// A workdir marker with no task identity in the environment is the one
-	// signal that can outlive the task that wrote it: a local_directory run
-	// that never cleaned up leaves it in the user's own repository, where it
+	// A task-scoped workdir marker with no task identity in the environment is
+	// the one signal that can outlive the task that wrote it: a local_directory
+	// run that never cleaned up leaves it in the user's own repository, where it
 	// disables every command below this function for that whole directory tree
-	// until someone deletes the file by hand (MUL-6132). Try to retire it
-	// automatically, and when that cannot be proven safe, at least name it —
-	// the bare message below sends the user to the source to find out which
-	// file to remove. Mirrors newAPIClient's leftover-marker handling.
+	// until someone deletes the file by hand (MUL-6132). Name it, so the user
+	// knows which file that is; the bare message below sends them to the source
+	// instead. Mirrors newAPIClient's leftover-marker handling.
 	if markerPath := leftoverDaemonTaskMarkerPath(); markerPath != "" {
 		return fmt.Errorf("%s is not available inside a daemon-managed task%s", command, leftoverMarkerSuffix(markerPath))
 	}

server/cmd/multica/cmd_agent_test.go

@@ -41,7 +41,12 @@ func chdirWithDaemonTaskMarker(t *testing.T) {
 	if err := os.MkdirAll(filepath.Dir(markerPath), 0o755); err != nil {
 		t.Fatalf("create marker dir: %v", err)
 	}
-	data := []byte(`{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `"}`)
+	// Task-scoped: a real workdir marker always carries the identity of the
+	// task that wrote it, and that identity is what separates a leftover from
+	// the permanent workspaces root marker, which has managed_by and nothing
+	// else (MUL-6132). Writing the bare form here would model the root marker
+	// rather than the workdir marker these tests are about.
+	data := []byte(`{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `","agent_id":"agent-1","issue_id":"issue-1"}`)
 	if err := os.WriteFile(markerPath, data, 0o644); err != nil {
 		t.Fatalf("write marker: %v", err)
 	}

server/cmd/multica/stale_marker.go

@@ -1,6 +1,7 @@
 package main
 
 import (
+	"encoding/json"
 	"fmt"
 	"os"
 	"strings"
@@ -9,21 +10,61 @@ import (
 )
 
 // leftoverDaemonTaskMarkerPath returns the workdir marker path when a marker is
-// the ONLY reason the CLI considers itself inside a daemon-managed task, and ""
-// otherwise.
+// the ONLY reason the CLI considers itself inside a daemon-managed task AND
+// that marker could actually be a leftover. It returns "" otherwise.
+//
+// Two things disqualify a marker from the leftover treatment:
 //
 // Any MULTICA_* task identity in the environment means the daemon really did
-// launch this process, so the marker is doing its job and none of the
-// leftover-specific handling applies. MULTICA_DAEMON_PORT is treated the same
-// way even though it is not sufficient on its own for identity: it still says a
-// daemon environment is present, which is not a leftover's fingerprint.
+// launch this process, so the marker is doing its job. MULTICA_DAEMON_PORT
+// counts here even though it is not sufficient on its own for identity: it
+// still says a daemon environment is present, which is not a leftover's
+// fingerprint.
+//
+// A marker that is not task-scoped is the permanent workspaces root marker,
+// which never expires and must never be described as removable.
 func leftoverDaemonTaskMarkerPath() string {
 	if inAgentExecutionContext() ||
 		strings.TrimSpace(os.Getenv(cli.TaskConfigRootEnv)) != "" ||
 		strings.TrimSpace(os.Getenv("MULTICA_DAEMON_PORT")) != "" {
 		return ""
 	}
-	return daemonTaskContextMarkerPath()
+	markerPath := daemonTaskContextMarkerPath()
+	if markerPath == "" || !markerIsTaskScoped(markerPath) {
+		return ""
+	}
+	return markerPath
+}
+
+// markerIsTaskScoped reports whether the marker at path belongs to one specific
+// task, as opposed to being the workspaces root marker.
+//
+// Two different markers share this filename, and daemonTaskContextMarkerPath
+// tells them apart by nothing but managed_by. The per-task marker carries the
+// identity of the task that wrote it and dies with that task. The workspaces
+// root marker carries no identity at all: the daemon re-creates it on every
+// start as a permanent, node-wide guard so a subprocess that escapes above its
+// workdir still fails closed. Telling anyone to delete THAT — a user, or an
+// agent subprocess that lost its environment and is reading the error — closes
+// the hole it exists to cover, so it gets the plain refusal instead.
+//
+// Identity means agent_id OR issue_id, not both: a chat task has no issue, and
+// requiring one would misfile its marker as the root marker and lose the hint
+// exactly where a leftover is possible. The root marker has neither field, so
+// either one present is enough to separate them.
+func markerIsTaskScoped(markerPath string) bool {
+	data, err := os.ReadFile(markerPath)
+	if err != nil {
+		return false
+	}
+	var marker struct {
+		AgentID string `json:"agent_id"`
+		IssueID string `json:"issue_id"`
+	}
+	if json.Unmarshal(data, &marker) != nil {
+		return false
+	}
+	return strings.TrimSpace(marker.AgentID) != "" || strings.TrimSpace(marker.IssueID) != ""
 }
 
 // leftoverMarkerSuffix renders the shared tail both refusal paths append when a

server/cmd/multica/stale_marker_test.go

@@ -10,11 +10,11 @@ import (
 	"github.com/multica-ai/multica/server/internal/daemon/execenv"
 )
 
-// seedDaemonTaskMarker writes a task-scoped daemon marker into a fresh temp
+// seedMarker writes a daemon marker with the given body into a fresh temp
 // directory and chdirs into a nested subdirectory, so the CLI's upward walk has
 // to find it the same way it would in a user's repository. Returns the marker
 // path.
-func seedDaemonTaskMarker(t *testing.T) string {
+func seedMarker(t *testing.T, body string) string {
 	t.Helper()
 
 	workDir := t.TempDir()
@@ -22,7 +22,6 @@ func seedDaemonTaskMarker(t *testing.T) string {
 	if err := os.MkdirAll(filepath.Dir(markerPath), 0o755); err != nil {
 		t.Fatalf("create marker dir: %v", err)
 	}
-	body := `{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `","agent_id":"agent-1","issue_id":"issue-1"}`
 	if err := os.WriteFile(markerPath, []byte(body), 0o644); err != nil {
 		t.Fatalf("write marker: %v", err)
 	}
@@ -34,6 +33,42 @@ func seedDaemonTaskMarker(t *testing.T) string {
 	return markerPath
 }
 
+// issueTaskMarker is the marker an issue task writes: agent and issue both set.
+func issueTaskMarker() string {
+	return `{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `","agent_id":"agent-1","issue_id":"issue-1"}`
+}
+
+// chatTaskMarker is the marker a chat task writes. It has no issue, which is
+// why task identity cannot be defined as "agent_id AND issue_id".
+func chatTaskMarker() string {
+	return `{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `","agent_id":"agent-1"}`
+}
+
+// workspacesRootMarker is the permanent node-wide guard: managed_by and nothing
+// else. EnsureWorkspacesRootMarker writes exactly this.
+func workspacesRootMarker() string {
+	return `{"managed_by":"` + execenv.TaskContextMarkerManagedBy + `"}`
+}
+
+func seedDaemonTaskMarker(t *testing.T) string {
+	t.Helper()
+	return seedMarker(t, issueTaskMarker())
+}
+
+// refusalPaths returns every CLI entry point that can refuse because of a
+// leftover marker, so a new one cannot be added without deciding whether it
+// reports the file.
+func refusalPaths() map[string]func() error {
+	return map[string]func() error{
+		"human-local": func() error { return requireHumanLocalCommand("daemon stop") },
+		"api": func() error {
+			_, err := newAPIClient(testCmd())
+			return err
+		},
+		"task-local-config": requireTaskLocalConfigRoot,
+	}
+}
+
 func clearDaemonEnvSignals(t *testing.T) {
 	t.Helper()
 	t.Setenv("MULTICA_AGENT_ID", "")
@@ -47,35 +82,67 @@ func clearDaemonEnvSignals(t *testing.T) {
 // file, so whether a user could act on the refusal depended on which command
 // they happened to run first. Both paths must now point at the same file with
 // the same remedy.
-func TestLeftoverMarkerReportedIdenticallyOnBothRefusalPaths(t *testing.T) {
-	markerPath := seedDaemonTaskMarker(t)
+func TestLeftoverMarkerReportedIdenticallyOnEveryRefusalPath(t *testing.T) {
+	for _, marker := range []struct{ name, body string }{
+		{name: "issue task", body: issueTaskMarker()},
+		{name: "chat task", body: chatTaskMarker()},
+	} {
+		t.Run(marker.name, func(t *testing.T) {
+			markerPath := seedMarker(t, marker.body)
+			clearDaemonEnvSignals(t)
+			t.Setenv("MULTICA_TOKEN", "")
+			t.Setenv("MULTICA_SERVER_URL", "http://127.0.0.1:8080")
+
+			for name, refuse := range refusalPaths() {
+				err := refuse()
+				if err == nil {
+					t.Fatalf("%s: got nil error; want refusal on a leftover marker", name)
+				}
+				if !strings.Contains(err.Error(), markerPath) {
+					t.Fatalf("%s error should name the marker path %q; got %q", name, markerPath, err.Error())
+				}
+				if !strings.Contains(err.Error(), "leftover") {
+					t.Fatalf("%s error should hint it may be a leftover; got %q", name, err.Error())
+				}
+			}
+
+			// The CLI never removes the marker: taking down a fail-closed guard
+			// needs a proof the CLI cannot produce, because the daemon's active
+			// task count only rises after a claim returns.
+			if _, statErr := os.Stat(markerPath); statErr != nil {
+				t.Fatalf("CLI must not delete the marker: %v", statErr)
+			}
+		})
+	}
+}
+
+// TestWorkspacesRootMarkerNeverReportedAsLeftover is the regression test for the
+// permanent node-wide guard. It shares a filename with the per-task marker but
+// is re-created by the daemon on every start and must never expire. Telling
+// anyone to delete it — a user, or an agent subprocess that lost its
+// environment and is reading this error — closes the hole it exists to cover.
+func TestWorkspacesRootMarkerNeverReportedAsLeftover(t *testing.T) {
+	markerPath := seedMarker(t, workspacesRootMarker())
 	clearDaemonEnvSignals(t)
 	t.Setenv("MULTICA_TOKEN", "")
 	t.Setenv("MULTICA_SERVER_URL", "http://127.0.0.1:8080")
 
-	humanErr := requireHumanLocalCommand("daemon stop")
-	if humanErr == nil {
-		t.Fatal("requireHumanLocalCommand() = nil; want refusal on a leftover marker")
-	}
-	_, apiErr := newAPIClient(testCmd())
-	if apiErr == nil {
-		t.Fatal("newAPIClient() = nil error; want refusal on a leftover marker")
+	if got := leftoverDaemonTaskMarkerPath(); got != "" {
+		t.Fatalf("leftoverDaemonTaskMarkerPath() = %q; want \"\" for the workspaces root marker", got)
 	}
 
-	for name, err := range map[string]error{"human-local": humanErr, "api": apiErr} {
-		if !strings.Contains(err.Error(), markerPath) {
-			t.Fatalf("%s error should name the marker path %q; got %q", name, markerPath, err.Error())
+	for name, refuse := range refusalPaths() {
+		err := refuse()
+		if err == nil {
+			t.Fatalf("%s: got nil error; want the plain refusal to stay in place", name)
 		}
-		if !strings.Contains(err.Error(), "leftover") {
-			t.Fatalf("%s error should hint it may be a leftover; got %q", name, err.Error())
+		if strings.Contains(err.Error(), "leftover") || strings.Contains(err.Error(), "remove it") {
+			t.Fatalf("%s must not describe the workspaces root marker as removable; got %q", name, err.Error())
 		}
 	}
 
-	// The CLI never removes the marker: taking down a fail-closed guard needs a
-	// proof the CLI cannot produce, because the daemon's active task count only
-	// rises after a claim returns.
 	if _, statErr := os.Stat(markerPath); statErr != nil {
-		t.Fatalf("CLI must not delete the marker: %v", statErr)
+		t.Fatalf("workspaces root marker must survive: %v", statErr)
 	}
 }
 

server/internal/daemon/execenv/sidecar_manifest.go

@@ -209,10 +209,14 @@ func skillSlugCandidate(baseSlug string, attempt int) string {
 // writeSidecarManifest persists m to {envRoot}/{sidecarManifestFile}.
 // Empty manifests are still written so a later Cleanup that finds the
 // file knows tracking was attempted (vs. an old build that predates this
-// mechanism, where the file is absent and Cleanup must no-op). Failures
-// are returned to the caller; the caller treats them as non-fatal because
-// a missed manifest only degrades local_directory cleanup, not task
-// execution.
+// mechanism, where the file is absent and Cleanup must no-op).
+//
+// Failures are returned to the caller, and how much they matter depends on
+// where the sidecars landed. For a cloud envRoot the manifest is a convenience
+// the GC can do without, so Prepare logs and continues. For an in-place
+// local_directory run it is the only record of what was written into the user's
+// own repository, so Prepare treats the failure as fatal and rolls back while
+// it still holds the in-memory manifest (MUL-6132).
 func writeSidecarManifest(envRoot string, m *sidecarManifest) error {
 	if envRoot == "" {
 		return nil