Skip to content
Open
97 changes: 97 additions & 0 deletions server/cmd/server/rerun_session_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,103 @@ func TestGetLastTaskSessionExcludesAPIInvalidRequest(t *testing.T) {
}
}

// TestGetLastTaskSessionExcludesPoisonedCompletedViaSubsequentTask covers the
// MUL-5785 fresh-retry scenario: Task A completed successfully with session "S",
// then Task B resumed "S", detected a poisoned history, did a fresh retry that
// also failed, and was stored with session_id = "S" and failure_reason =
// api_invalid_request. The NOT EXISTS subquery must exclude "S" entirely so the
// next issue task does not resume the poisoned session, even though Task A's
// own row looks clean (status='completed').
func TestGetLastTaskSessionExcludesPoisonedCompletedViaSubsequentTask(t *testing.T) {
if testPool == nil {
t.Skip("no database connection")
}

issueID, agentID, runtimeID := setupRerunTestFixture(t)
t.Cleanup(func() { cleanupRerunFixture(t, issueID) })

ctx := context.Background()

// Task A: completed successfully with session "S" — what the resume lookup
// would naively return.
if _, err := testPool.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, started_at, completed_at, session_id, work_dir)
VALUES ($1, $2, $3, 'completed', 0, now() - interval '2 minutes', now() - interval '2 minutes', 'KIMI-POISONED', '/tmp/kimi')
`, agentID, runtimeID, issueID); err != nil {
t.Fatalf("insert Task A: %v", err)
}

// Task B: resumed "S", got poisoned-history error, fresh retry also failed.
// Daemon stores session_id = PriorSessionID ("S") + failure_reason = api_invalid_request.
if _, err := testPool.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, runtime_id, issue_id, status, priority, started_at, completed_at, session_id, work_dir, failure_reason, error)
VALUES ($1, $2, $3, 'failed', 0, now() - interval '1 minute', now() - interval '1 minute', 'KIMI-POISONED', '/tmp/kimi', 'api_invalid_request',
'kimi provider error: the message at position 1 with role ''assistant'' must not be empty')
`, agentID, runtimeID, issueID); err != nil {
t.Fatalf("insert Task B: %v", err)
}

queries := db.New(testPool)
prior, err := queries.GetLastTaskSession(ctx, db.GetLastTaskSessionParams{
AgentID: pgtype.UUID{Bytes: parseUUIDBytes(agentID), Valid: true},
IssueID: pgtype.UUID{Bytes: parseUUIDBytes(issueID), Valid: true},
})
if err == nil && prior.SessionID.Valid {
t.Fatalf("expected poisoned session to be excluded via NOT EXISTS, but got %q", prior.SessionID.String)
}
}

// TestGetLastChatTaskSessionExcludesPoisonedViaSubsequentTask mirrors
// TestGetLastTaskSessionExcludesPoisonedCompletedViaSubsequentTask for the
// chat path. GetLastChatTaskSession must also exclude a session that a
// subsequent task in the same chat_session has marked as api_invalid_request.
func TestGetLastChatTaskSessionExcludesPoisonedViaSubsequentTask(t *testing.T) {
if testPool == nil {
t.Skip("no database connection")
}

_, agentID, runtimeID := setupRerunTestFixture(t)

ctx := context.Background()

var chatSessionID string
if err := testPool.QueryRow(ctx, `
INSERT INTO chat_session (workspace_id, agent_id, creator_id, title, runtime_id)
VALUES ($1, $2, $3, 'kimi-poison-test', $4)
RETURNING id
`, testWorkspaceID, agentID, testUserID, runtimeID).Scan(&chatSessionID); err != nil {
t.Fatalf("insert chat_session: %v", err)
}
t.Cleanup(func() {
testPool.Exec(ctx, `DELETE FROM agent_task_queue WHERE chat_session_id = $1`, chatSessionID)
testPool.Exec(ctx, `DELETE FROM chat_session WHERE id = $1`, chatSessionID)
})

// Task A: completed successfully with session "S" in this chat session.
if _, err := testPool.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, runtime_id, chat_session_id, status, priority, started_at, completed_at, session_id, work_dir)
VALUES ($1, $2, $3, 'completed', 0, now() - interval '2 minutes', now() - interval '2 minutes', 'KIMI-CHAT-POISONED', '/tmp/kimi-chat')
`, agentID, runtimeID, chatSessionID); err != nil {
t.Fatalf("insert Task A: %v", err)
}

// Task B: stored with session_id = "S" + api_invalid_request after a
// failed fresh retry following a poisoned-history rejection.
if _, err := testPool.Exec(ctx, `
INSERT INTO agent_task_queue (agent_id, runtime_id, chat_session_id, status, priority, started_at, completed_at, session_id, work_dir, failure_reason, error)
VALUES ($1, $2, $3, 'failed', 0, now() - interval '1 minute', now() - interval '1 minute', 'KIMI-CHAT-POISONED', '/tmp/kimi-chat', 'api_invalid_request',
'kimi provider error: the message at position 1 with role ''assistant'' must not be empty')
`, agentID, runtimeID, chatSessionID); err != nil {
t.Fatalf("insert Task B: %v", err)
}

queries := db.New(testPool)
prior, err := queries.GetLastChatTaskSession(ctx, pgtype.UUID{Bytes: parseUUIDBytes(chatSessionID), Valid: true})
if err == nil && prior.SessionID.Valid {
t.Fatalf("expected poisoned chat session to be excluded via NOT EXISTS, but got %q", prior.SessionID.String)
}
}

// TestGetLastTaskSessionExcludesCodexSemanticInactivity covers Codex
// semantic inactivity timeouts: the failed task did establish a session, but
// resuming it can replay the same stuck Codex state. The resume lookup must
Expand Down
41 changes: 41 additions & 0 deletions server/internal/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -3667,6 +3667,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
Expand Down Expand Up @@ -4325,6 +4352,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/.
Expand Down Expand Up @@ -4641,6 +4671,7 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i

if shouldRetryWithFreshSession(result, task.PriorSessionID, tools, provider) {
firstUsage := result.Usage
originalError := result.Error // preserve: carries the Kimi poisoned-session fingerprint
taskLog.Warn("session resume failed, retrying with fresh session", "error", result.Error)
execOpts.ResumeSessionID = ""
retryResult, retryTools, retryErr := d.executeAndDrain(ctx, backend, prompt, execOpts, taskLog, task.ID, &msgSeq)
Expand All @@ -4650,6 +4681,16 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i
result = retryResult
result.Usage = mergeUsage(firstUsage, result.Usage)
tools = retryTools
// If the fresh retry also failed without establishing a new session,
// carry forward the original poisoned session ID and error so
// classifyPoisonedError labels this task api_invalid_request.
// GetLastTaskSession's NOT EXISTS check then excludes the poisoned
// session from future resume lookups even though Task A (the prior
// successful run) was stored as "completed" (MUL-5785).
if result.Status != "completed" && result.SessionID == "" {
result.SessionID = task.PriorSessionID
result.Error = originalError
}
}
}

Expand Down
88 changes: 88 additions & 0 deletions server/internal/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
33 changes: 33 additions & 0 deletions server/internal/daemon/execenv/runtime_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
82 changes: 82 additions & 0 deletions server/internal/daemon/execenv/runtime_config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1513,3 +1513,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)
}
})
}
Loading