From be14774d3910f9b6db7bb02d90a873a8bbe80cfe Mon Sep 17 00:00:00 2001 From: kakiuwang-ui Date: Sat, 25 Jul 2026 01:02:17 +0800 Subject: [PATCH] fix(daemon): drop prior session when agent is renamed to prevent identity contamination (MUL-5736) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When an agent is renamed (e.g. "Reviewer" → "Backend Builder"), the workdir from its last run still holds a CLAUDE.md whose ## Agent Identity section names the old role. Resuming that session means the conversation history has already established the old identity, causing the agent to misidentify itself and refuse tasks — the behaviour reported in MUL-5736. Fix: before overwriting CLAUDE.md with the new identity, read the name already recorded in the managed marker block. If it differs from the current agent name, drop PriorSessionID and start a fresh conversation. The check runs after gateCodexResumeToRolloutPresence while the old file is still on disk. The drop does not set PriorSessionResumeUnavailable because a rename is intentional: the user changed the agent's identity on purpose, so a fresh session is expected behaviour rather than an unexpected loss to surface. ReadPriorAgentName is added to execenv so the parsing logic is testable in isolation; gateResumeToSameAgentName wraps the daemon-level gate. --- server/internal/daemon/daemon.go | 30 +++++++ server/internal/daemon/daemon_test.go | 88 +++++++++++++++++++ .../internal/daemon/execenv/runtime_config.go | 33 +++++++ .../daemon/execenv/runtime_config_test.go | 82 +++++++++++++++++ 4 files changed, 233 insertions(+) diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 807012c8e19..7d8f9082322 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -3727,6 +3727,33 @@ func gateResumeToReusedWorkdir(task *Task, taskCtx *execenv.TaskContextForEnv, e return reused } +// gateResumeToSameAgentName drops the prior session when the reused workdir's +// existing runtime config records a different agent name than the current task. +// This prevents a renamed agent from resuming a session whose conversation +// history was established under the old identity — which can cause the agent to +// misidentify itself even though CLAUDE.md is updated before launch (MUL-5736). +// No-op when there is nothing to resume, no current agent name, or when the +// prior name cannot be read (e.g. fresh workdir, legacy config without the +// identity section). Unlike gateResumeToReusedWorkdir, this does not set +// PriorSessionResumeUnavailable: the rename is intentional, so starting fresh +// is expected behaviour rather than a loss the agent must disclose. +func gateResumeToSameAgentName(task *Task, taskCtx *execenv.TaskContextForEnv, provider, workDir string, taskLog *slog.Logger) { + if task.PriorSessionID == "" || taskCtx.AgentName == "" { + return + } + priorName := execenv.ReadPriorAgentName(workDir, provider) + if priorName == "" || priorName == taskCtx.AgentName { + return + } + taskLog.Warn("dropping prior session: agent identity changed since last run", + "session_id", task.PriorSessionID, + "prior_name", priorName, + "current_name", taskCtx.AgentName, + ) + task.PriorSessionID = "" + taskCtx.PriorSessionResumed = false +} + // shouldReusePriorWorkdir keeps the local_directory lock invariant without // forcing every squad-leader follow-up onto a fresh provider session. Worker // tasks already expose their current local-directory assignment, so their @@ -4385,6 +4412,9 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i // conversation Codex will silently restart from scratch. if reused { gateCodexResumeToRolloutPresence(&task, &taskCtx, provider, env.CodexHome, taskLog) + // Check for agent rename BEFORE overwriting CLAUDE.md — we need to + // read the old identity while it is still on disk (MUL-5736). + gateResumeToSameAgentName(&task, &taskCtx, provider, env.WorkDir, taskLog) } // Inject runtime-specific config (meta skill) so the agent discovers .agent_context/. diff --git a/server/internal/daemon/daemon_test.go b/server/internal/daemon/daemon_test.go index 4677dd1939a..f6502507e9d 100644 --- a/server/internal/daemon/daemon_test.go +++ b/server/internal/daemon/daemon_test.go @@ -1342,6 +1342,94 @@ func TestGateResumeToReusedWorkdir(t *testing.T) { } } +func TestGateResumeToSameAgentName(t *testing.T) { + t.Parallel() + + // writeConfig writes a CLAUDE.md with the given agent name so we can + // exercise name-mismatch detection against a realistic on-disk file. + writeConfig := func(t *testing.T, dir, agentName string) { + t.Helper() + ctx := execenv.TaskContextForEnv{ + IssueID: "00000000-0000-0000-0000-000000000001", + AgentName: agentName, + } + if _, err := execenv.InjectRuntimeConfig(dir, "claude", ctx); err != nil { + t.Fatalf("InjectRuntimeConfig: %v", err) + } + } + + t.Run("same name keeps session", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeConfig(t, dir, "Backend Builder") + task := Task{PriorSessionID: "sess-bb"} + taskCtx := execenv.TaskContextForEnv{AgentName: "Backend Builder", PriorSessionResumed: true} + gateResumeToSameAgentName(&task, &taskCtx, "claude", dir, slog.Default()) + if task.PriorSessionID != "sess-bb" { + t.Fatalf("session should be kept when name matches, got %q", task.PriorSessionID) + } + if !taskCtx.PriorSessionResumed { + t.Fatal("PriorSessionResumed should remain true") + } + }) + + t.Run("renamed agent drops session", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + // Workdir was last used when agent was named "Reviewer". + writeConfig(t, dir, "Reviewer") + // Agent was renamed to "Backend Builder". + task := Task{PriorSessionID: "sess-reviewer"} + taskCtx := execenv.TaskContextForEnv{AgentName: "Backend Builder", PriorSessionResumed: true} + gateResumeToSameAgentName(&task, &taskCtx, "claude", dir, slog.Default()) + if task.PriorSessionID != "" { + t.Fatalf("session should be dropped on name change, got %q", task.PriorSessionID) + } + if taskCtx.PriorSessionResumed { + t.Fatal("PriorSessionResumed should be cleared") + } + // Name-change drop is intentional, not a lost session — no continuity notice. + if taskCtx.PriorSessionResumeUnavailable { + t.Fatal("PriorSessionResumeUnavailable should not be set on an intentional name-change drop") + } + }) + + t.Run("no prior session is a no-op", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeConfig(t, dir, "Reviewer") + task := Task{PriorSessionID: ""} + taskCtx := execenv.TaskContextForEnv{AgentName: "Backend Builder", PriorSessionResumed: false} + gateResumeToSameAgentName(&task, &taskCtx, "claude", dir, slog.Default()) + if task.PriorSessionID != "" || taskCtx.PriorSessionResumed { + t.Fatal("no-op expected when there is no prior session") + } + }) + + t.Run("no current agent name is a no-op", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeConfig(t, dir, "Reviewer") + task := Task{PriorSessionID: "sess-x"} + taskCtx := execenv.TaskContextForEnv{AgentName: "", PriorSessionResumed: true} + gateResumeToSameAgentName(&task, &taskCtx, "claude", dir, slog.Default()) + if task.PriorSessionID != "sess-x" { + t.Fatalf("session should be kept when current agent name is unknown, got %q", task.PriorSessionID) + } + }) + + t.Run("missing config file keeps session", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() // no CLAUDE.md written + task := Task{PriorSessionID: "sess-x"} + taskCtx := execenv.TaskContextForEnv{AgentName: "Backend Builder", PriorSessionResumed: true} + gateResumeToSameAgentName(&task, &taskCtx, "claude", dir, slog.Default()) + if task.PriorSessionID != "sess-x" { + t.Fatalf("session should be kept when config is missing, got %q", task.PriorSessionID) + } + }) +} + // newCodexStoreGuardDaemon builds a Daemon with only the per-issue Codex session // store guard initialised — enough to exercise the reserve-vs-mark protocol. func newCodexStoreGuardDaemon() *Daemon { diff --git a/server/internal/daemon/execenv/runtime_config.go b/server/internal/daemon/execenv/runtime_config.go index 09b82f7c448..93bfd806d38 100644 --- a/server/internal/daemon/execenv/runtime_config.go +++ b/server/internal/daemon/execenv/runtime_config.go @@ -300,6 +300,39 @@ func locateMarkerBlock(content string) (start, end int, found bool) { return start, end, true } +// ReadPriorAgentName reads the agent name recorded in the Multica-managed +// block of the current runtime config file for the given workDir and provider. +// It returns the agent name from the "## Agent Identity" section, or an empty +// string when the file does not exist, has no managed block, or has no agent +// identity entry. Used by gateResumeToSameAgentName to detect agent renames +// before a session resume is attempted. +func ReadPriorAgentName(workDir, provider string) string { + path := runtimeConfigPath(workDir, provider) + if path == "" { + return "" + } + data, err := os.ReadFile(path) + if err != nil { + return "" + } + start, end, ok := locateMarkerBlock(string(data)) + if !ok { + return "" + } + block := string(data)[start:end] + const prefix = "**You are: " + idx := strings.Index(block, prefix) + if idx < 0 { + return "" + } + rest := block[idx+len(prefix):] + end2 := strings.Index(rest, "**") + if end2 < 0 { + return "" + } + return strings.TrimSpace(rest[:end2]) +} + // CleanupRuntimeConfig excises the Multica marker block from the runtime // config file for the given provider and restores the file to its exact // pre-injection state, byte for byte. The cleanup is the second half of diff --git a/server/internal/daemon/execenv/runtime_config_test.go b/server/internal/daemon/execenv/runtime_config_test.go index 6307dcf4aaa..1e2b393357b 100644 --- a/server/internal/daemon/execenv/runtime_config_test.go +++ b/server/internal/daemon/execenv/runtime_config_test.go @@ -1585,3 +1585,85 @@ func TestCommentTriggeredBriefSingleThreadKeepsSingleReply(t *testing.T) { t.Errorf("single/same-thread brief must keep the single --parent=trigger cookbook, got:\n%s", out) } } + +func TestReadPriorAgentName(t *testing.T) { + t.Parallel() + + writeWithAgentName := func(t *testing.T, dir, name string) { + t.Helper() + ctx := TaskContextForEnv{ + IssueID: "00000000-0000-0000-0000-000000000001", + AgentName: name, + } + if _, err := InjectRuntimeConfig(dir, "claude", ctx); err != nil { + t.Fatalf("InjectRuntimeConfig: %v", err) + } + } + + t.Run("reads name from injected CLAUDE.md", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + writeWithAgentName(t, dir, "Backend Builder") + if got := ReadPriorAgentName(dir, "claude"); got != "Backend Builder" { + t.Fatalf("got %q, want %q", got, "Backend Builder") + } + }) + + t.Run("reads name with ID suffix present", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + ctx := TaskContextForEnv{ + IssueID: "00000000-0000-0000-0000-000000000001", + AgentName: "Reviewer", + AgentID: "de53a53c-3d5e-4829-bf82-10dd35ce4858", + } + if _, err := InjectRuntimeConfig(dir, "claude", ctx); err != nil { + t.Fatalf("InjectRuntimeConfig: %v", err) + } + if got := ReadPriorAgentName(dir, "claude"); got != "Reviewer" { + t.Fatalf("got %q, want %q", got, "Reviewer") + } + }) + + t.Run("no file returns empty", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if got := ReadPriorAgentName(dir, "claude"); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) + + t.Run("file without marker block returns empty", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "CLAUDE.md"), []byte("# My CLAUDE.md\nSome content.\n"), 0o644); err != nil { + t.Fatal(err) + } + if got := ReadPriorAgentName(dir, "claude"); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) + + t.Run("marker block without agent identity returns empty", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + ctx := TaskContextForEnv{ + IssueID: "00000000-0000-0000-0000-000000000001", + // AgentName intentionally empty — no identity section will be written + } + if _, err := InjectRuntimeConfig(dir, "claude", ctx); err != nil { + t.Fatalf("InjectRuntimeConfig: %v", err) + } + if got := ReadPriorAgentName(dir, "claude"); got != "" { + t.Fatalf("got %q, want empty (no agent name in config)", got) + } + }) + + t.Run("unknown provider returns empty", func(t *testing.T) { + t.Parallel() + dir := t.TempDir() + if got := ReadPriorAgentName(dir, "unknown-provider"); got != "" { + t.Fatalf("got %q, want empty", got) + } + }) +}