Repository navigation
fix: apply the chosen runtime to continued sessions and keep continue/fork top-level - #702
Conversation
A derived child got the chosen runtime as create-time fields only. While unbound it publishes no effective runtime, so the web composer fell back to the agent default and its first prompt bound that instead. The derive now records the explicit runtime, route, or fork source runtime as the child's selected runtime at revision 1. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Continue and fork record their source as provenance, not hierarchy. One predicate over lineage.kind now keeps them out of the session list tree, the supervision active-child signal, and Goal descendant authorization, while spawned, provenance, and recovery children keep nesting. Co-Authored-By: Claude Opus 5.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
⛔ Files ignored due to path filters (7)
📒 Files selected for processing (13)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. WalkthroughThe changes separate lineage provenance from session hierarchy in daemon authorization, child-work discovery, and the web session tree. They also add selected runtime derivation for continued and forked sessions, including default-speed handling, selection revisions, and runtime-selection coverage. ChangesSession hierarchy
Derived-session runtime selection
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to No actionable issue remains in the reviewed change; it is ready to merge after normal checks. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
pedronauck
left a comment
There was a problem hiding this comment.
PR #702 review
Verdict: approve — 0 blocking findings.
Reviewed 0f55cc51535da12c13abdd8abe45a060050e22b6...854fd0eef3d03c528939d90dbf513078ce0c59d3 (main...HEAD). The remote PR head matched the local reviewed commit before publication. Acceptance contract: /tmp/herdr-packets/p2.md.
Findings
No actionable correctness or regression findings in the reviewed diff.
- Runtime: the derive path now persists the chosen runtime as the existing selected-runtime record at revision 1. I traced it through accepted logical creation, metadata persistence, resume, prompt normalization, runtime binding, and composer hydration. The route-binding predicate continues to recognize the chosen provider/model; fork selection comes from the source runtime. The new focused tests exercise restart/resume and the first runtime-less prompt, rather than only create-time fields.
- Hierarchy: the existing typed lineage distinguishes Continue/Fork provenance from nesting without dropping source metadata. The shared Web tree, active-child supervision, and Goal descendant authorization exclude these edges. Ordinary spawned/provenance/recovery children keep their existing treatment. Archive and deletion operate on individual session targets; the change introduces no cascade over provenance edges. Public parent/root catalog filters intentionally retain their stored-field semantics, documented in the PR and accompanying docs.
- Compatibility: no persisted or public DTO shape changes were introduced. Existing typed Continue/Fork rows immediately receive the corrected placement. Existing source links and runtime selection fields remain compatible. No migration/codegen change is necessary for these changes.
- Scope and conventions: all seven changed production source files remain below 500 lines. Existing tests were extended without weakened assertions. The new daemon suite owns the active-child work-source behavior. QA scenarios, the cross-surface audit, lifecycle docs, and the official skill reference were updated.
- Reference research: spot-checked the named local reference sources. t3code distinguishes subagent relationships in its sidebar, synara gives handoffs a null hierarchy parent, and monocode models handoffs separately from subagent transcript blocks. The adopted typed-edge approach fits the existing Compozy representation.
Focused checks performed
go test ./internal/session ./internal/store ./internal/daemon -run 'TestContinueSession|TestForkSession|TestForkAccountInheritance|TestHierarchyParentSessionID|TestSessionWorkSourcesChildren|TestDaemonGoalCommandHandlerShouldExecuteCanonicalSessionLifecycle' -count=1- PASS: session (6.089s), store (0.020s), daemon (1.018s).
- Log:
/tmp/herdr-packets/checks-702-go.log.
bunx turbo run test --filter=compozy-web -- src/systems/session/lib/__tests__/session-hierarchy.test.ts src/systems/session/hooks/__tests__/use-session-prompt-runtime.test.tsx- PASS: 2 files, 16 tests. Turborepo's required
codegen-checkdependency also passed. - Log:
/tmp/herdr-packets/checks-702-web.log.
- PASS: 2 files, 16 tests. Turborepo's required
- Read the PR body, full changed-file diff, applicable root/internal/web/site instructions, SD-013, and adjacent runtime/hierarchy consumers. Verified the working tree remained clean.
Limits and delivery status
- This is a focused code-review approval, not a claim that release/CI gates or scenario QA have passed. No
make gate,gate-full, full suite, E2E, or live-provider run was performed. The changed scenarios explicitly remain pending/untested. - As disclosed in the PR, previously created unbound derived sessions are not backfilled with a selected runtime; the selection fix applies to new derives. Stored parent/root filter semantics remain unchanged.
- The historical
.compozy/tasks/session-continue-fork/spec artifacts were unavailable in this checkout; review used the supplied acceptance packet, current code/tests, and PR's documented rationale. - Controller handoff is blocked by the environment:
test "${HERDR_ENV:-}" = 1failed. The Herdr skill explicitly prohibits controlling a Herdr session from outside Herdr, so no pane command was sent.
No product files were edited and no commits were created or pushed.
|
Problem
Two bugs Pedro reported on desktop v0.3.0, both in "Continue with another agent" / "Fork from here" (spec
.compozy/tasks/session-continue-fork/, shipped in #684).Bug 1: the chosen runtime/model was not applied to the continued session.
Repro: create a session on Codex GPT-6.1-Sol and send "oi". Open the session menu → Continue with another agent. Leave Agent =
general, pick Runtime → Fable 5 (Claude Code), set Open in = This window, and click Continue. The new session opens ("Continued from Saudação inicial ao Pedro", "Nothing said here yet"), but the composer model selector shows GPT-6.1-Sol. Sending a message would bind Codex, not Claude.Bug 2: continued and forked sessions rendered nested under their source.
The sidebar showed the source row with "⌄ 1" and the new session indented under it. Product rule: sessions spawned by an agent (sub-agents, orchestration children) are children and may nest. Sessions the user creates with Continue or Fork are new top-level sessions. Their lineage ("Continued from X" / "Forked from X") is provenance metadata (pill, divider, inspector), not hierarchy.
Root cause
Bug 1: the composer never sees the chosen runtime
web/src/systems/session/hooks/use-session-continue-dialog.tsruntimeRequest), and the handler passes it through (internal/api/core/session_derive.goderiveRuntimeFromPayload).Provider/Model/...inapplyDeriveRuntime,internal/session/derive_prepare.go). It never sets the session'sSelectedRuntime, the durable runtime intent.CreateAccepted, so it stays unbound until the first prompt (internal/session/manager_create_accepted.go). While unbound, the API leaves outruntime.effective(runtimeHasEffectiveSelectionininternal/api/core/conversions.go:UnboundwithTransitionNone→ no effective runtime), andruntime.selectedisnil.selectedand falls back toeffective, then to the agent's default runtime (promptRuntimeDefaults→resolveAgentRuntimeValue(agent)inweb/src/systems/session/hooks/use-session-prompt-runtime.ts). So it showed Codex, and its first prompt sent Codex as an explicit runtime snapshot, overriding the create-time fields.pendingRouteMatchesSelectionand silently dropped the pending route. It also hit forks whose source ran on a runtime other than the agent default: the child is locked to the source runtime byforkRuntimeFromMeta, but its composer showed the agent default.Bug 2: hierarchy consumers ignored the typed lineage edge
kind("",provenance,recovery,spawn,continue,fork). It is stored as a catalog column and inmeta.json, with a lossless backfill (internal/store/session_lineage.go). The store already separates the two meanings.parent_session_idas a parent:buildSessionTree(web/src/systems/session/lib/session-hierarchy.ts) nested any session whoselineage.parent_session_idwas loaded. The spec had decided "nesting unchanged" (_uiux.mdS10), and Pedro's new rule overrides that.sessionWorkSources.children(internal/daemon/session_work_sources.go) counted every row withparent=<id>as anactive_childwork signal, and that signal defers inactivity stops. A continued child (an accepted logical session isactive) kept its source session alive.authorizeGoalDescendant(internal/daemon/loop_goal_command.go) walkedparent_session_id, so the source session's agent could command Goals on a session the user had continued from it.Fix
Session runtime (Go,
internal/session)applyDeriveRuntimemarks the create opts withderiveSelectsRuntimewhenever an explicit runtime, a declared route, or the fork source runtime applies. A speed left unset takes the target agent's default speed, as the bind would resolve it.prepareCreateStartturns those (post-hook) runtime fields into the child'sSelectedRuntimeat revision 1 (deriveSelectedRuntime,manager_create_prepare.go). This goes through the existing start-spec →Session→meta.json/ catalog path, so the existing payload mapper exposes it asruntime.selected. A runtime without a provider (a model or speed alone) still refines the agent default at bind and records no selection.normalizePromptRuntimeSelectionalready prefersSelectedRuntime). A routed child's selection matches its pending route, so the route is no longer dropped by the composer's default.Hierarchy vs provenance (Go + web)
store.HierarchyParentSessionID(lineage)(internal/store/session_lineage.go) is the single rule: it returns the parent for root/provenance/recovery/spawn edges and""forcontinue/fork.internal/daemon/session_work_sources.go: a continued or forked session is not active-child work, so it never defers the source's inactivity stop.internal/daemon/loop_goal_command.go: Goal descendant authorization walks hierarchy parents only.web/src/systems/session/lib/session-hierarchy.ts:sessionTreeParentIdreusessessionOriginKind(the same predicate as the origin pill), socontinue/forkrows are roots in every list built on the shared tree (window sidebar, sessions modal, agent detail, the cycle shortcut order, and thread filtering). Spawned, provenance, and recovery children keep nesting, including sub-agents a continued session spawns, which nest under it.lineage.parent_session_idstill names the source for the origin pill/divider, the inspector,compozy session statusOrigin, and the CLI PARENT column.Reference research
docs/orchestration-v2/core-graph-and-data-model.md,packages/client-runtime/src/state/threadRelationships.ts,apps/web/src/components/Sidebar.logic.ts): uses one lineage record{ parentThreadId, relationshipToParent: "fork" | "subagent", rootThreadId }plusforkedFrom. Forks keepparentThreadId= source, and the sidebar decides by relationship kind. Forks render as ordinary top-level threads, and subagents are handled separately (isSidebarSubagentThread). Adopted: one parent pointer plus a typed relationship, with the UI and daemon deciding hierarchy by kind. Compozy already persists this shape aslineage.kind.packages/contracts/src/orchestration.ts,apps/server/src/orchestration/decider.ts): uses separate fields, withparentThreadIdfor subagents,forkSourceThreadIdfor forks (created withparentThreadId: null), andhandoff.sourceThreadIdfor provider handoffs. Its sidebar nests byparentThreadIdonly. Same product rule (forks/handoffs are top-level). I did not adopt the field split, because Compozy can express the rule with the existing typed kind without a migration or a DTO break.src/features/sessions/model/session.ts): handoff is an in-session harness switch (HandoffMeta), and subagents are mirrored as blocks inside the parent transcript. There is no fork-as-child nesting, so it agrees with the rule but has no data-model equivalent.Compatibility notes (SD-013)
meta.jsonshape change, so no migration. Existing continue/fork rows already carrylineage_kind(continue/fork), written by feat: continue and fork sessions across agents, with fallback account routes #684 and backfilled for older rows, so they become top-level immediately and losslessly. Existing unbound derived children made before this fix have noselectedruntime. They keep today's behavior (agent default until the user picks a model). New derives get the selection.runtime.selected/selection_revisionare existing fields that are now populated for derived children.lineage.parent_session_idkeeps naming the source. Theparent=/root=catalog filters (CLI--parent/--root,compozy__session_list) still match stored fields as-is, so they include continued/forked sessions. The docs say to readlineage.kindto tell them apart. No OpenAPI or codegen change, andcodegen-checkpassed locally.CreateOpts.deriveSelectsRuntimeis unexported and set only by the derive path.Tests added/changed
internal/session/manager_derive_test.goTestContinueSession"Should select the chosen runtime on the unbound child and bind it"TestForkSessionwhole-session subtest (added assertion)web/.../hooks/__tests__/use-session-prompt-runtime.test.tsx"Should prefer an unbound session's selected runtime over the agent default"internal/store/session_lineage_test.goTestHierarchyParentSessionIDweb/.../lib/__tests__/session-hierarchy.test.ts"Should keep continued and forked sessions top-level while spawned children nest"internal/daemon/session_work_sources_test.go(new, there was no suite forsessionWorkSources.children)internal/daemon/loop_goal_command_test.go"Should reject a session continued from the caller"Each new Go regression fails without the production change: I checked this by temporarily reverting each fix.
Focused checks run
go test ./internal/session/ ./internal/store/: 1651 passed. 4 failures (TestManagerSpawnHooksCarryLineageAndCannotWidenPermissions,TestSpawnGovernanceUsesOnlyContiguousSpawnedLineage) wereStop(...) context deadline exceededwhile the daemon suite ran concurrently on the same machine. They pass 3/3 with-count=3alone and don't touch the derive path.go test ./internal/daemon/ ./internal/api/core/: 4015 passed.go test ./skills/: 33 passed.golangci-lint run ./internal/session/ ./internal/store/ ./internal/daemon/: clean.vitest run src/systems/session/lib src/systems/session/components/session-list …: 36 files / 278 tests passed.use-session-prompt-runtime.test.tsx: 6 passed.turbo run typecheck lint --filter=compozy-web: passed (includingcodegen-check).Left to CI
make gate/ full Go race lanes, full Vitest, Playwright E2E (session-derive.spec.ts), and Storybook. Per the controller's instructions I did not run these locally.QA scenarios touched
ET-web-session-continue: new steps 10 (runtime) and 11 (top-level row), fix note, reset tountested.ET-web-session-fork-from-here: fix note and walk, reset tountested.ET-web-session-sidebar-threads,ET-web-sessions-catalog-modal:qa-impactflags (continue/fork no longer nest).docs/_memory/change-impact.md(2026-10-06). Docs:packages/site/content/docs/sessions/lifecycle.mdx. Official skill:skills/compozy/references/runtime-operations.md.Risks / follow-ups
root_session_id/spawn_depthof a derived child still point at the source's tree, and spawned sessions under a continued child inherit that root. This only affects observability grouping androot=filters. Re-rooting would mean a data migration plus a filter-semantics change, so I left it out of scope.🤖 Generated with Claude Code
Summary by CodeRabbit