Skip to content

fix(daemon): reuse managed workdirs for squad leaders#5429

Open
wjueyao wants to merge 5 commits into
multica-ai:mainfrom
wjueyao:codex/fix-leader-session-reuse
Open

fix(daemon): reuse managed workdirs for squad leaders#5429
wjueyao wants to merge 5 commits into
multica-ai:mainfrom
wjueyao:codex/fix-leader-session-reuse

Conversation

@wjueyao

@wjueyao wjueyao commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Squad-leader follow-ups on the same issue currently receive prior_session_id
and prior_workdir from the server, but the daemon unconditionally skips
workdir reuse for leader tasks. The subsequent per-cwd resume gate then drops
the prior session because the new task has a different workdir, so every
leader follow-up starts a fresh provider session.

This change allows leader tasks to reuse a prior workdir only when its
canonical path has the managed {workspace}/{task}/workdir shape, its parent
GC metadata identifies a completed non-local issue env for the same
workspace/issue, and its task-context marker matches the same agent/issue.
External, malformed, unmarked, and local-directory paths fail closed.
Non-leader behavior is unchanged.

Thinking path:

  1. Reproduced with two comments on one squad-assigned issue: the server set
    resume_session=true and reuse_workdir=true, but the daemon logged
    dropping prior session: workdir not reused and started a new Codex thread.
  2. Traced the fresh session to the !task.IsLeaderTask reuse condition in
    runTask, before gateResumeToReusedWorkdir.
  3. Kept the external-workdir safety boundary and required canonical containment,
    completed non-local GC metadata, and a matching daemon task marker.
  4. Added integration coverage for real provider-session resume plus adversarial
    coverage for lookalike paths, missing/mismatched metadata, and regular files.

Related Issue

Closes #5535

Regression introduced by #4951. Related to the same-issue session-resume contract tracked in #3435 and #3625.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Refactor / code improvement (no behavior change)
  • Documentation update
  • Tests (adding or improving test coverage)
  • CI / infrastructure

Changes Made

  • Add shouldReusePriorWorkdir to distinguish completed daemon-managed leader
    workdirs from external or residual local_directory paths.
  • Require canonical containment, a non-local matching GC record, and matching
    task-context marker before preserving the provider session.
  • Assert the provider receives the prior resume ID and cover external, unmarked,
    mismatched, metadata-free, local-directory, and non-directory paths.

How to Test

  1. Assign an issue to a squad with an agent leader and let the leader finish a run.
  2. Add a follow-up comment that mentions the same leader.
  3. Confirm the second task reuses the first workdir and provider session.

Commands run:

go test ./internal/daemon/... -count=1
go test -race ./internal/daemon -run 'TestRunTaskSquadLeader|TestShouldReusePriorWorkdirSquadLeader|TestAcquireLocalDirectoryLockSkipsSquadLeaderTasks' -count=1
go vet ./internal/daemon/...

All scoped checks pass. I also ran go test ./...; it exposed an unrelated,
pre-existing timing failure in
TestExecuteAndDrain_CodexInactivityReportsToolResultTranscript where Codex
initialization exceeded its 5-second deadline. The same test failed 2/3 times
when rerun alone, while go test ./internal/daemon/... -count=1 passed.

Local dogfood

I built this branch as a local multica binary, started its daemon against a
real workspace, and added a third follow-up comment to the same squad-assigned
issue and leader used for the reproduction.

  • Previous run: f5bfb6c8-1a39-4307-8bb8-ecc189e787f9
  • Fixed run: 4e0b1f0a-994a-42c2-a274-37d444db32f8
  • Both runs used the same workdir ending in f5bfb6c8/workdir.
  • Both runs returned Codex session 019f63b9-68ff-7521-875c-97455589ff2f.
  • The fixed daemon logged codex thread resumed with that same thread ID.

The fixed run completed successfully, and the agent independently reported the
same CODEX_THREAD_ID. No builder or reviewer was dispatched.

Risks

The reuse check canonicalizes both paths with filepath.EvalSymlinks, rejects
non-directories and symlink escapes, then requires parent GC metadata for the
same workspace/issue with local_directory=false plus a task marker for the
same agent/issue. Missing or malformed provenance fails closed. This means a
follow-up after an interrupted run with incomplete metadata may start fresh;
that availability trade-off preserves the local-directory lock invariant.

Checklist

  • I have included a thinking path that traces from project context to this change
  • I have run the relevant tests locally and they pass
  • I have added or updated tests where applicable
  • If this change affects the UI, I have included before/after screenshots
  • I have updated relevant documentation to reflect my changes
  • If I added a new runtime / coding tool / UI tab, I synced the change to landing copy (apps/web/features/landing/i18n/) and relevant docs (apps/docs/content/docs/)
  • If this PR touches Chinese product copy, I checked it against apps/docs/content/docs/developers/conventions.zh.mdx (terminology, mixed-rule for task / issue / skill)
  • I have considered and documented any risks above
  • I will address all reviewer comments before requesting merge

AI Disclosure

AI tool used: OpenAI Codex App / Codex CLI

Prompt / approach: Dogfood a minimal squad workflow, inspect the daemon and
task logs when same-issue leader follow-ups failed to resume, trace the workdir
reuse gate in source, then implement the smallest fail-closed fix with regression
tests before running scoped package, race, and vet checks.

Screenshots (optional)

Not applicable; daemon-only behavior change.

@vercel

vercel Bot commented Jul 15, 2026

Copy link
Copy Markdown

@wjueyao is attempting to deploy a commit to the IndexLabs Team on Vercel.

A member of the Team first needs to authorize it.

@wjueyao wjueyao marked this pull request as draft July 15, 2026 03:34
@wjueyao wjueyao marked this pull request as ready for review July 15, 2026 03:39
@wjueyao

wjueyao commented Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Pre-merge design audit after tracing the role and session contracts:

  • Session reuse is the documented default for supported providers, and issue tasks load prior state by (agent_id, issue_id). There is no product-level fresh-session exception for squad leaders.
  • The regression was introduced by Fix local-directory lock for squad leader wrap-up (ZIC-37) #4951. That fix correctly made is_leader_task skip the user local_directory assignment and mutex, but it also conservatively skipped every prior-workdir reuse for leaders.
  • The broken path is therefore role-specific, not provider-specific: any is_leader_task=true follow-up using a daemon-managed workdir started fresh. Worker and direct-agent tasks kept their existing reuse path; worker local_directory tasks still prepare against the same locked path.
  • This PR preserves Fix local-directory lock for squad leader wrap-up (ZIC-37) #4951's safety intent. A leader may reuse only an existing workdir with the exact daemon-managed {WorkspacesRoot}/{workspace_id}/{task-segment}/workdir shape. External paths and arbitrary contained paths fail closed, covering pre-Fix local-directory lock for squad leader wrap-up (ZIC-37) #4951 leader sessions recorded against a user directory.

The last point is locked by commit 7d381563f: its regression test failed against the earlier containment-only predicate and passes after the managed-shape check.

Fresh verification on the updated branch:

go test ./internal/daemon/... -count=1
go test -race ./internal/daemon -run 'TestRunTaskSquadLeader|TestShouldReusePriorWorkdirSquadLeader|TestAcquireLocalDirectoryLockSkipsSquadLeaderTasks' -count=1
go vet ./internal/daemon/...
git diff --check

All passed locally.

wjueyao and others added 2 commits July 16, 2026 18:22
Require matching managed-env metadata and task markers before reusing a squad leader's prior workdir. Cover spoofed path shapes and assert the provider receives the resume session.

Refs: multica-ai#5535
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Squad leader follow-ups start fresh sessions instead of resuming prior issue sessions

1 participant