Skip to content

fix: branch teammates from the last stable turn#36

Merged
tmustier merged 3 commits into
mainfrom
fix/issue-35-clean-branch-context
Mar 27, 2026
Merged

fix: branch teammates from the last stable turn#36
tmustier merged 3 commits into
mainfrom
fix/issue-35-clean-branch-context

Conversation

@tmustier
Copy link
Copy Markdown
Owner

Summary

  • fix branch-mode teammate spawning to branch from the last stable completed turn instead of the leader's active in-flight tool-use path
  • add focused smoke coverage for unfinished-turn branch selection
  • add an integration test that starts a real worker from a cleaned branch session and verifies assigned work completes

Testing

  • npm run typecheck
  • npm run lint
  • npm run smoke-test
  • npm run integration-branch-context-test

Closes #35

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1fd451f0e0

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

const candidate = path[i];
if (!candidate) continue;
if (isUserMessageEntry(candidate)) {
return { leafId: candidate.id, adjusted: candidate.id !== requestedLeafId, reason: "clean-turn-user" };
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep branch leaf on persisted assistant path

Returning clean-turn-user here can select a user-only branch when the leader is in its first tool-use turn (no earlier completed assistant). In that case createSessionForTeammate() passes this user leaf to SessionManager.createBranchedSession(...), and the pi-coding-agent implementation does not persist a branched session file unless the branch contains an assistant message; the spawned worker then starts from a fresh/empty session instead of the intended branch context. This regresses branch mode specifically for first-turn delegation with in-flight tool use.

Useful? React with 👍 / 👎.

@tmustier
Copy link
Copy Markdown
Owner Author

Findings

  • [P1] User-leaf fallback spawns workers from an empty session
    File: extensions/teams/leader.ts:94
    resolveBranchLeafSelection() can now return a user message (clean-turn-user), and createSessionForTeammate() passes that straight to createBranchedSession(). In Pi, user-only branches are not flushed to disk, so the returned path can point to a non-existent file; the spawned pi --session ... then reopens as a blank session instead of the cleaned branch context. The new tests only exercise the assistant-leaf path, so this slips through.
    What should change: Either materialize user-only branches before spawning, or never select a user leaf unless you can persist it. Add a regression that spawns from a session with no prior completed assistant.

  • [P2] Stable-point selection drops the delegating user turn
    File: extensions/teams/session-branching.ts:70
    On any mid-turn delegation with prior history, this rewinds to the previous completed assistant and intentionally excludes the current user message. That means contextMode=branch no longer carries the active request/constraints the leader is answering, so workers only see older context plus the task text.
    What should change: Preserve the latest user message while stripping only the unfinished assistant/toolResult suffix, e.g. branch from a persisted assistant leaf and replay the trailing user entry into the child session.

  • [P3] New integration test leaks child workers on failure
    File: scripts/integration-branch-context-test.mts:244
    After the worker is spawned, any failed assertion or timeout exits before terminateAll(procs) and temp-dir cleanup run. The other integration scripts use try/finally; this one will leave orphaned pi processes on red paths.
    What should change: Wrap the main test body in try/finally and always run terminateAll(procs) plus rmSync(...).

Verdict

needs attention — the new branch-selection flow has a broken user-fallback path that can start workers from an empty session, and the chosen stable branch point also strips the active user request from branched context.

@tmustier
Copy link
Copy Markdown
Owner Author

Findings

  • [P2] Clean-turn branching drops compaction and other pre-user context entries
    File: extensions/teams/session-branching.ts:88, extensions/teams/leader.ts:95
    When the leader is mid tool-use, the new logic rewinds the child branch to the previous assistant message and then replays only the active user message. That drops any entries inserted between that assistant and the user — notably auto-compaction summaries, which Pi can append after completed assistant turns. In compacted sessions, a spawned worker can lose the compaction boundary and reopen much more history than the leader currently had.
    What should change: Branch from the active user’s parent entry, or otherwise preserve all non-active-turn entries between the stable boundary and the replayed user message instead of replaying only the user.

Verdict

needs attention — the follow-up fixes address the earlier file materialization, user-request replay, and test-cleanup gaps, but the cleaned-branch path still drops post-assistant context entries in compacted sessions.

@tmustier tmustier merged commit c893455 into main Mar 27, 2026
3 checks passed
@tmustier tmustier deleted the fix/issue-35-clean-branch-context branch March 27, 2026 09:52
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.

delegate + contextMode=branch can spawn idle workers that inherit the leader's current turn

1 participant