Skip to content

refactor(runtime): extract session branching into session-branch.ts - #1253

Closed
UncertaintyDeterminesYou4ndMe wants to merge 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:refactor/runtime-session-branch
Closed

refactor(runtime): extract session branching into session-branch.ts#1253
UncertaintyDeterminesYou4ndMe wants to merge 2 commits into
apache:mainfrom
UncertaintyDeterminesYou4ndMe:refactor/runtime-session-branch

Conversation

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor

Part of #1084 — the session-manager.ts slice from the runtime/other lane (claimed in #1084 (comment)).

What moved

Behavior-neutral pure move of the branch-session subsystem (~250 lines) out of session-manager.ts into a new leaf module packages/runtime/src/session-branch.ts:

  • createBranchSession and cloneBranchRuntimeLedger, behind a minimal structural SessionBranchDeps seam (store is an inline structural subset of the manager-local SessionStore, following the RuntimeLedgerRepairDeps precedent, so the leaf never imports back into session-manager.ts).
  • The turn-boundary copy helpers copyMessagesThroughTurnBoundary / copyMessagesBeforeTurn (exported; branchFromTurn / branchBeforeTurn stay behind as thin public methods).
  • The clone helpers cloneRuntimeEventForBranch / cloneRunHeaderForBranchCreate (module-private).

One deliberate seam adjustment: createBranchSession now returns the child SessionHeader; SessionManager wraps it with headerToSummary at the call sites, keeping the summary projection with its owner and the import direction one-way.

Dedup

headerLineage in session-manager.ts was a verbatim private copy of the one in agent-run-recovery.ts. The recovery module's copy is now exported and imported; the duplicate is deleted.

Invariants (per #1084 / #363)

  • No logic changes; comments moved verbatim with their code.
  • @maka/runtime public exports unchanged (index.ts untouched).
  • No reverse imports from the new leaf.
  • Flat branch off main, single slice.

Verification

  • npm --workspace @maka/runtime run build — clean.
  • npm --workspace @maka/runtime run test — 2269 tests, 2261 pass, 0 fail, 7 skipped, 1 cancelled. The cancelled one is the known pre-existing test(runtime): session-title deadline-abort test hangs under full-suite load on main #1222 (session-title deadline-abort, Node v22.17.0), reproduced identically before and after this change; all branchFromTurn/branchBeforeTurn tests pass.
  • biome check clean on the three touched files.

Pure move from session-manager.ts (apache#1084, runtime/other lane):
- createBranchSession, cloneBranchRuntimeLedger, the turn-boundary
  message copy helpers, and the run/event clone helpers now live in
  session-branch.ts behind a minimal SessionBranchDeps seam.
- createBranchSession returns the child SessionHeader; SessionManager
  wraps it with headerToSummary, so the new leaf never imports back
  into session-manager.ts.
- headerLineage is now imported from agent-run-recovery.ts instead of
  keeping a verbatim private copy.

Behavior-neutral; @maka/runtime public exports unchanged.
…nch.ts

The source-grep contract asserted branchFromTurn and the sticky-model
capture in one session-manager.ts read; the capture now lives in
session-branch.ts. Assert each side of the seam where it lives:
branchFromTurn delegates to createBranchSession, and createBranchSession
carries model: header.model.
@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor Author

CI test caught a source-grep contract this move had to co-migrate: session-sticky-model-contract.test.ts asserted branchFromTurn … model: header.model in one session-manager.ts read, and the sticky-model capture now lives in session-branch.ts. Fixed in 24112ee by asserting each side of the seam where it lives — branchFromTurn delegates to createBranchSession (session-manager) and createBranchSession carries model: header.model (session-branch). Verified locally: desktop build + the contract test file pass; also scanned the repo for other source-reading tests touching the moved code (only session-startup-recovery-contract.test.ts reads session-manager.ts, and everything it asserts stayed put).

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor Author

Latest CI on this PR: test and e2e pass (the co-migrated contract fix landed). The remaining typecheck failure is not from this branch — it's the Biome format gate failing on main itself since #1254 (one unformatted file, packages/storage/src/__tests__/settings-store-usage.test.ts), inherited through the merge ref. Filed #1260 with the one-file format fix; once that lands, a re-run here should go green.

@Astro-Han

Copy link
Copy Markdown
Contributor

I think main is clear on typecheck now, we can merge main or run biome linter locally to fix it.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No code findings. The extraction preserves the existing branching behavior. Please rebase or rerun CI before merge because the current typecheck check is red on the old formatting baseline.

@Astro-Han

Copy link
Copy Markdown
Contributor

Hi — this PR conflicts with current main and cannot be merged as-is.

I tested a rebase onto current main locally (in a throwaway worktree — your branch was not touched). It stops on these files:

  • packages/runtime/src/session-manager.ts

These are real source conflicts, so they need your judgement rather than a mechanical rebase — please rebase onto current main and resolve them yourself, then push. Once the branch is conflict-free and CI is green on the new head, I will pick it up for review.

git fetch upstream && git rebase upstream/main
# resolve, then
git push --force-with-lease

Thanks for the contribution — happy to help if any conflict is unclear.


AI-assisted maintenance note, not a review. It does not count as the required human review under CONTRIBUTING.md §Review.

@UncertaintyDeterminesYou4ndMe

Copy link
Copy Markdown
Contributor Author

Thanks for testing the rebase. Before resolving the conflict I re-checked whether the extraction still stands on current main — it doesn't, so I'm closing this instead of rebasing it.

What this PR moved into session-branch.ts (the ledger clone machinery, the copy-slice helpers, and createBranchSession) has since been extracted on main by a different, more general route:

  • packages/runtime/src/conversation-copy.ts (introduced in feat(runtime-host): own session revision authority #1664, extended through fix(core): read AgentRun ledgers written by any version #1986/feat(runtime-host): preserve Agent Graph history across revisions #2039) now owns the slice/clone machinery (createConversationCopySlice, cloneConversationRuntimeLedger, …) with its own test suite — and serves branching, the newer revision sessions, and cross-Session copy from one place. Re-extracting a branch-only session-branch.ts on top of it would fight that factoring rather than complete it.
  • What remains in session-manager.ts (branchFromTurn / branchBeforeTurn / createBranchSession, now alongside reviseBeforeTurn / createRevisionSession) are thin orchestrators over that module — the ~200-line extraction this PR carried no longer exists to extract.
  • The two supporting changes are moot on main too: the headerLineage export was only needed by the extraction, and session-sticky-model-contract.test.ts (the co-migrated contract) has been deleted entirely.

So the goal of this refactor was reached on main via conversation-copy.ts, the same way #2476's goal was reached via run-command-core.ts. Closing as superseded.

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.

2 participants