Repository navigation
fix: restore durable ACP session continuity - #307
Merged
Merged
Conversation
|
Too many files changed for review. ( Bypass the limit by tagging |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThe PR adds durable session runtime selection with optimistic concurrency. It persists selected runtime state, exposes HTTP, Host API, CLI, native tool, and web operations, resumes stopped sessions during prompts, and removes the assistant “Use as Goal” action. ChangesDurable runtime selection
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant WebClient
participant SessionAPI
participant SessionManager
participant GlobalDB
WebClient->>SessionAPI: Set runtime with expected revision
SessionAPI->>SessionManager: SetRuntimeSelection
SessionManager->>GlobalDB: Compare revision and persist selection
GlobalDB-->>SessionManager: Updated session revision
SessionManager-->>SessionAPI: Updated session info
SessionAPI-->>WebClient: Session runtime payload
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
Contributor
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (7)
internal/daemon/host_api_session_adapter.go (1)
42-43: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a compile-time assertion for the adapter capability.
hostAPISessionManagerAdapternow implementscore.SessionRuntimeSelectionManager. Add an assertion near the type declaration. This detects an interface signature change at compile time.Proposed change
type hostAPISessionManagerAdapter struct { core.SessionManager exec sandboxExecSessionManager runtimeSelection core.SessionRuntimeSelectionManager } + +var _ core.SessionRuntimeSelectionManager = hostAPISessionManagerAdapter{}As per coding guidelines, production Go must use compile-time interface assertions.
Also applies to: 105-126
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/daemon/host_api_session_adapter.go` around lines 42 - 43, Add a compile-time interface assertion adjacent to the hostAPISessionManagerAdapter type declaration, verifying it implements core.SessionRuntimeSelectionManager. Keep the assertion in production code and leave the adapter implementation unchanged.Source: Coding guidelines
internal/cli/session_test.go (1)
151-196: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the default runtime speed.
The command defaults omitted
--speedtonormal, but this test does not assert that value inSetSessionRuntimeRequest. Assertrequest.Runtime.Speediscontract.SpeedNormal.As per path instructions, tests must verify meaningful business logic and behavior outcomes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/session_test.go` around lines 151 - 196, Update the SetSessionRuntimeRequest validation in the “Should fetch the current revision before setting runtime” test to also assert that request.Runtime.Speed equals contract.SpeedNormal, while preserving the existing provider, model, reasoning effort, ID, and revision checks.Source: Path instructions
internal/api/core/conversions_parsers_test.go (1)
155-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert selected-runtime speed across conversion and persistence tests.
The tests omit
SelectedRuntime.Speed. A regression that removes or changes this runtime selection field can pass.
internal/api/core/conversions_parsers_test.go#L155-L163: Assertpayload.Runtime.Selected.Speed == contract.SpeedNormal.internal/session/session_test.go#L241-L246: Set a non-defaultSpeedinSelectedRuntime.internal/session/session_test.go#L263-L315: Assert thatMeta(),Info(), andReadSessionMeta()retain that speed.As per coding guidelines, edited Go tests must identify the invariant and update the existing canonical suite. As per path instructions, tests must verify behavior outcomes for critical paths.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/api/core/conversions_parsers_test.go` around lines 155 - 163, Extend the selected-runtime assertions to cover Speed preservation: in internal/api/core/conversions_parsers_test.go lines 155-163, assert payload.Runtime.Selected.Speed equals contract.SpeedNormal; in internal/session/session_test.go lines 241-246, initialize SelectedRuntime with a non-default Speed; and in lines 263-315, assert Meta(), Info(), and ReadSessionMeta() retain that speed.Sources: Coding guidelines, Path instructions
internal/store/session_runtime_selection.go (1)
10-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd snake_case JSON tags to
SessionRuntimeSelection.This struct is serialized to disk.
SessionMeta.RuntimeSelectionholds a*SessionRuntimeSelectionState, and that state marshalsSelectedintometa.json. The fields have no tags, so the persisted keys becomeProvider,Model,ReasoningEffort, andSpeed. Every other persisted field inSessionRuntimeSelectionStateand ininternal/store/session_meta_types.gouses snake_case. Fix the format now, while migration00048_schema.sqlis still unreleased. If you add the tags after sessions writemeta.json, the old PascalCase keys no longer decode and the durable selection is silently lost on reload.♻️ Proposed tags
type SessionRuntimeSelection struct { - Provider string - Model string - ReasoningEffort string - Speed speedpkg.Speed + Provider string `json:"provider"` + Model string `json:"model,omitempty"` + ReasoningEffort string `json:"reasoning_effort,omitempty"` + Speed speedpkg.Speed `json:"speed,omitempty"` }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/store/session_runtime_selection.go` around lines 10 - 16, Add snake_case JSON tags to all fields of SessionRuntimeSelection—Provider, Model, ReasoningEffort, and Speed—so its persisted representation matches the existing session metadata format before migration 00048_schema.sql is released.internal/session/manager_prompt_admission.go (1)
88-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared prompt-target resolution.
Lines 92-116 repeat lines 49-73 of
submitAdmittedGoalByTargetalmost verbatim: active-session lookup, runtime resolution at admission, workspace resolution, and the persisted-metadata fallback. Extract one helper that returns the session, the workspace ID, and the resolved runtime selection, then call it from both functions. This keeps the two admission paths from diverging as runtime-selection rules evolve.♻️ Proposed shape
func (m *Manager) resolvePromptTarget( ctx context.Context, req promptRequest, ) (session *Session, workspaceID string, runtime *RuntimeSelection, err error) { if active, ok := m.Get(req.target); ok { resolved, resolveErr := m.resolvePromptRuntimeAtAdmission(ctx, active, req.runtime) if resolveErr != nil { return nil, "", nil, resolveErr } workspaceID, err = sessionPromptWorkspaceID(active) if err != nil { return nil, "", nil, err } return active, workspaceID, resolved, nil } meta, err := m.readMetaWithContext(ctx, req.target) if err != nil { return nil, "", nil, err } resolved, err := normalizePromptRuntimeSelectionFromMeta(meta, req.runtime) if err != nil { return nil, "", nil, err } return nil, meta.WorkspaceID, resolved, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/manager_prompt_admission.go` around lines 88 - 115, Extract the duplicated target-resolution logic from submitAdmittedPromptByTarget and submitAdmittedGoalByTarget into a shared Manager helper that returns the session, workspace ID, resolved runtime selection, and error. Preserve the active-session and persisted-metadata fallback behavior, then update both admission functions to call the helper and use its returned values.internal/session/manager_resume.go (1)
35-37: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRelease the resume run with
defer.
finishSessionResumeruns only on the normal return path. Ifm.resumeSessionpanics,run.doneis never closed and theresumeRuns[target]entry is never deleted. Every concurrent waiter then blocks until its own context expires, and each laterResumefor that session id receives the same stale run and blocks again for the lifetime of the manager. A deferred call removes that permanent-stall path. The namederrreturn in theResumesignature also becomes unused after this refactor; drop it or use it.🛡️ Proposed fix
- resumed, resumeErr := m.resumeSession(ctx, target) - m.finishSessionResume(target, run, resumed, resumeErr) - return resumed, resumeErr + var resumed *Session + var resumeErr error + defer func() { m.finishSessionResume(target, run, resumed, resumeErr) }() + resumed, resumeErr = m.resumeSession(ctx, target) + return resumed, resumeErr🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/manager_resume.go` around lines 35 - 37, Defer cleanup of the resume run in Resume so finishSessionResume executes during both normal returns and panics from m.resumeSession, ensuring run.done is closed and resumeRuns[target] is removed. Update the Resume return handling accordingly, and remove the now-unused named err return or otherwise use it consistently.internal/session/additional_test.go (1)
388-402: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the new resume assertions in a
Shouldsubtest.These lines add a new behavioral case: a prompt on a stopped session now resumes it. The assertions live in the parent function body instead of a named subtest. Move them into
t.Run("Should resume a stopped session on prompt", ...)so the case is named and can run in parallel like the neighboring subtests.As per coding guidelines: "Use
Should …subtest names, defaultt.Parallel". As per path instructions: "MUST use t.Run("Should...") pattern for ALL test cases".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/additional_test.go` around lines 388 - 402, Move the stopped-session resume assertions after the Prompt call into a t.Run("Should resume a stopped session on prompt", ...) subtest, including the resumed-state and driver-start-call checks. Follow the neighboring test structure by calling t.Parallel() in the new subtest and keep the existing prompt/event behavior intact.Sources: Coding guidelines, Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/api/core/handlers_test.go`:
- Around line 724-785: Strengthen the runtime-selection error tests around
ClearRuntimeSelectionFn and the “Should require a compare-and-set revision” and
“Should reject a stale clear revision” cases. Assert that
ClearRuntimeSelectionFn receives the expected session ID and expected revision
0, then decode each error response and verify its contract-specific error value
in addition to the HTTP status. Preserve the existing success assertions.
In `@internal/daemon/native_tool_binding_groups.go`:
- Around line 64-71: Update the availability assigned to ToolIDSessionRuntimeSet
and ToolIDSessionRuntimeClear in the tool binding map to require the runtime
manager capability, specifically core.SessionRuntimeSelectionManager, rather
than the generic sessions availability. Keep their existing call bindings
unchanged and ensure both tools are hidden when that manager is unavailable.
In `@internal/extension/contract/host_api.go`:
- Line 172: Remove the extension runtime path’s dependency on
internal/api/contract: in internal/extension/contract/host_api.go:172, replace
apicontract.PromptRuntimeSelectionPayload with an extension-owned or lower-layer
contract type; in internal/extension/host_api_session_runtime.go:9, remove the
API contract import; and at lines 31-32 convert directly to
session.RuntimeSelection or reuse a conversion moved to a lower-layer package.
In `@internal/extension/host_api_test.go`:
- Around line 647-680: Extend the runtime selection test around the
sessions/runtime/set and sessions/runtime/clear calls to assert every selected
field—Provider, Model, ReasoningEffort, and Speed—along with the revision. Add a
subsequent set or clear request using a stale expected_revision, assert the
specific conflict error code with the test’s established error-assertion
helpers, and verify the selection and revision remain unchanged after the
rejected request.
In `@internal/session/additional_test.go`:
- Around line 452-472: Protect all fakeDriver shared-field accesses with
h.driver.mu: in internal/session/additional_test.go:452-472, lock around the
startHook assignment and add a barrier confirming the second Resume caller is
registered before closing releaseStart; at
internal/session/additional_test.go:371-373 and :400-402, lock before reading
len(h.driver.startCalls).
In `@internal/session/manager_integration_test.go`:
- Around line 61-75: Strengthen the stopped-session reuse test around
h.manager.Prompt by capture the ACP session ID before Stop, then assert
h.driver.startCalls[1].ResumeSessionID matches it after the resumed prompt. Keep
the existing event-count, session lookup, and CWD assertions unchanged.
In `@internal/session/manager_resume_run.go`:
- Around line 61-77: Update waitForSessionResumes to drain every captured
sessionResumeRun instead of returning immediately on the first resume error;
continue waiting through ordinary run.err failures, and only return the context
cancellation/deadline error once the context is done. Preserve the existing
nil-context validation and return successful completion after all runs finish
when the context remains active.
In `@internal/session/manager_transition_test.go`:
- Around line 331-368: Extend the canonical ClearRuntimeSelection tests in the
session manager suite with successful current-revision cases for both active and
stopped sessions. Using the existing session harness and catalog, assert each
clear removes SelectedRuntime, increments RuntimeSelectionRevision, and persists
the cleared metadata through the catalog, while preserving the stopped state and
avoiding an ACP start where applicable.
In `@internal/store/globaldb/global_db_session_test.go`:
- Around line 294-298: Update the test covering blank selected-runtime columns
and revision 0 to assert the decoded info has SelectedRuntime equal to nil and
RuntimeSelectionRevision equal to 0. Add these outcome assertions after decoding
the fixture, preserving the existing setup and other checks.
In `@web/src/hooks/routes/__tests__/use-session-page-controls.test.tsx`:
- Around line 35-37: Update the mock around canPromptSession in the session page
controls tests to reuse the canonical helper from session-running.ts instead of
duplicating its active/stopped logic. Keep the stopped-session coverage aligned
with production behavior, or move that invariant to the helper’s owning suite
while leaving this test focused on hook wiring.
In
`@web/src/systems/session/components/__tests__/session-chat-runtime-provider.test.tsx`:
- Around line 789-805: Wrap the assertions in the “Should unmount a running
readonly thread safely under StrictMode” test in a try/finally block, and move
consoleError.mockRestore() into finally so the console.error spy is restored
even when an assertion fails.
In `@web/src/systems/session/hooks/use-session-runtime-selection.ts`:
- Around line 18-19: Update useSetSessionRuntime and SetSessionRuntimeVariables
to carry an AbortSignal, create or reuse a lifecycle-owned AbortController for
the mutation, and pass its signal to setSessionRuntime alongside workspaceId,
id, and request. Ensure the controller is cleaned up or aborted with the hook
lifecycle.
In `@web/src/systems/session/stores/session-prompt-runtime-store.ts`:
- Around line 61-84: Update inputUpdated and runtimePersisted in
session-prompt-runtime-store.ts to ignore events with a lower selectionRevision
than the current input, preserving the current selected runtime and revision;
continue applying equal or newer revisions normally. Add a regression in
web/src/systems/session/stores/__tests__/session-prompt-runtime-store.test.ts
covering persistence at revision 4 followed by an input snapshot at revision 3,
asserting the selection and revision remain at 4.
---
Nitpick comments:
In `@internal/api/core/conversions_parsers_test.go`:
- Around line 155-163: Extend the selected-runtime assertions to cover Speed
preservation: in internal/api/core/conversions_parsers_test.go lines 155-163,
assert payload.Runtime.Selected.Speed equals contract.SpeedNormal; in
internal/session/session_test.go lines 241-246, initialize SelectedRuntime with
a non-default Speed; and in lines 263-315, assert Meta(), Info(), and
ReadSessionMeta() retain that speed.
In `@internal/cli/session_test.go`:
- Around line 151-196: Update the SetSessionRuntimeRequest validation in the
“Should fetch the current revision before setting runtime” test to also assert
that request.Runtime.Speed equals contract.SpeedNormal, while preserving the
existing provider, model, reasoning effort, ID, and revision checks.
In `@internal/daemon/host_api_session_adapter.go`:
- Around line 42-43: Add a compile-time interface assertion adjacent to the
hostAPISessionManagerAdapter type declaration, verifying it implements
core.SessionRuntimeSelectionManager. Keep the assertion in production code and
leave the adapter implementation unchanged.
In `@internal/session/additional_test.go`:
- Around line 388-402: Move the stopped-session resume assertions after the
Prompt call into a t.Run("Should resume a stopped session on prompt", ...)
subtest, including the resumed-state and driver-start-call checks. Follow the
neighboring test structure by calling t.Parallel() in the new subtest and keep
the existing prompt/event behavior intact.
In `@internal/session/manager_prompt_admission.go`:
- Around line 88-115: Extract the duplicated target-resolution logic from
submitAdmittedPromptByTarget and submitAdmittedGoalByTarget into a shared
Manager helper that returns the session, workspace ID, resolved runtime
selection, and error. Preserve the active-session and persisted-metadata
fallback behavior, then update both admission functions to call the helper and
use its returned values.
In `@internal/session/manager_resume.go`:
- Around line 35-37: Defer cleanup of the resume run in Resume so
finishSessionResume executes during both normal returns and panics from
m.resumeSession, ensuring run.done is closed and resumeRuns[target] is removed.
Update the Resume return handling accordingly, and remove the now-unused named
err return or otherwise use it consistently.
In `@internal/store/session_runtime_selection.go`:
- Around line 10-16: Add snake_case JSON tags to all fields of
SessionRuntimeSelection—Provider, Model, ReasoningEffort, and Speed—so its
persisted representation matches the existing session metadata format before
migration 00048_schema.sql is released.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 38ac7ada-353d-4e6e-8e42-48a333974374
⛔ Files ignored due to path filters (50)
MIGRATION_GUIDE.mdis excluded by!**/*.mdbun.lockis excluded by!**/*.lock,!**/*.lockdocs/design/opendesign/session/SESSION-REDESIGN-SPEC.mdis excluded by!docs/design/**/*,!**/*.mddocs/design/opendesign/session/session-chat.htmlis excluded by!docs/design/**/*docs/design/opendesign/session/session-design-system.htmlis excluded by!docs/design/**/*docs/design/opendesign/session/session.cssis excluded by!docs/design/**/*docs/qa/bugs/BUG-20260804-stopped-prompt-restart-panic.mdis excluded by!**/*.mddocs/qa/charters/CH-durable-session-docs-continuity.mdis excluded by!**/*.mddocs/qa/charters/CH-message-actions-copy-timestamp.mdis excluded by!**/*.mddocs/qa/charters/CH-running-session-window-close.mdis excluded by!**/*.mddocs/qa/charters/CH-runtime-selection-restart-continuity.mdis excluded by!**/*.mddocs/qa/charters/CH-stopped-session-prompt-continuity.mdis excluded by!**/*.mddocs/qa/charters/CH-use-response-as-goal.mdis excluded by!**/*.mddocs/qa/journeys/J-13-follow-a-live-run.mdis excluded by!**/*.mddocs/qa/journeys/J-14-read-a-finished-transcript.mdis excluded by!**/*.mddocs/qa/journeys/J-17-session-create-unified-selector.mdis excluded by!**/*.mddocs/qa/reports/2026-08-04-durable-acp-sessions.mdis excluded by!**/*.mddocs/qa/scenarios/ET-running-session-window-close-clean.mdis excluded by!**/*.mddocs/qa/scenarios/ET-site-docs-first-session.mdis excluded by!**/*.mddocs/qa/scenarios/ET-web-session-prompt-runtime-and-create-navigation.mdis excluded by!**/*.mddocs/qa/scenarios/GL-use-response-as-goal.mdis excluded by!**/*.mddocs/qa/scenarios/RT-018.mdis excluded by!**/*.mddocs/qa/scenarios/RT-053.mdis excluded by!**/*.mddocs/qa/scenarios/RT-059.mdis excluded by!**/*.mddocs/qa/scenarios/RT-session-runtime-selection-continuity.mdis excluded by!**/*.mdinternal/store/globaldb/schema/migrations/atlas.sumis excluded by!**/*.sum,!**/*.suminternal/tools/builtin/testdata/native-tool-catalog.jsonis excluded by!**/*.jsonopenapi/compozy.jsonis excluded by!**/*.jsonpackage.jsonis excluded by!**/*.jsonpackages/site/content/docs/cli/session/index.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/session/meta.jsonis excluded by!**/*.jsonpackages/site/content/docs/cli/session/runtime/clear.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/session/runtime/index.mdxis excluded by!**/*.mdxpackages/site/content/docs/cli/session/runtime/meta.jsonis excluded by!**/*.jsonpackages/site/content/docs/cli/session/runtime/set.mdxis excluded by!**/*.mdxpackages/site/content/docs/getting-started/quick-start.mdxis excluded by!**/*.mdxpackages/site/content/docs/getting-started/web-ui.mdxis excluded by!**/*.mdxpackages/site/content/docs/index.mdxis excluded by!**/*.mdxpackages/site/content/docs/migration/index.mdxis excluded by!**/*.mdxpackages/site/content/docs/sessions/lifecycle.mdxis excluded by!**/*.mdxpackages/site/content/docs/sessions/resume.mdxis excluded by!**/*.mdxsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**,!**/generated/**skills/compozy/references/extension-authoring.mdis excluded by!**/*.mdskills/compozy/references/native-tools.mdis excluded by!**/*.mdskills/compozy/references/runtime-operations.mdis excluded by!**/*.mdweb/src/generated/compozy-openapi.d.tsis excluded by!**/generated/**,!**/generated/**,!**/*.d.tsweb/src/systems/agent/components/stories/agent-sessions-list.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/os/components/stories/attention-surfaces.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/session/components/assistant-ui/stories/session-thread.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/session/routes/agent-session.stories.tsxis excluded by!**/*.stories.tsx
📒 Files selected for processing (141)
internal/api/contract/session_runtime_payloads.gointernal/api/core/conversions.gointernal/api/core/conversions_parsers_test.gointernal/api/core/handlers_session_runtime.gointernal/api/core/handlers_test.gointernal/api/core/interfaces.gointernal/api/core/session_workspace.gointernal/api/core/test_helpers_test.gointernal/api/httpapi/handlers_test.gointernal/api/httpapi/session_routes.gointernal/api/spec/network_coordination_schema.gointernal/api/spec/registry_sessions.gointernal/api/spec/spec.gointernal/api/testutil/session_stub.gointernal/api/testutil/session_stub_runtime.gointernal/api/udsapi/handlers_test.gointernal/api/udsapi/session_routes.gointernal/cli/client_session_api.gointernal/cli/client_session_types.gointernal/cli/client_sessions.gointernal/cli/helpers_test.gointernal/cli/session_command.gointernal/cli/session_runtime.gointernal/cli/session_test.gointernal/daemon/host_api_session_adapter.gointernal/daemon/native_session_runtime.gointernal/daemon/native_tool_binding_groups.gointernal/daemon/native_tools_test.gointernal/daemon/sandbox_reconcile_session_projection.gointernal/extension/contract/host_api.gointernal/extension/contract/host_api_method_registry.gointernal/extension/contract/host_api_method_registry_access.gointernal/extension/contract/permissions.gointernal/extension/host_api.gointernal/extension/host_api_contract_types.gointernal/extension/host_api_methods.gointernal/extension/host_api_session_runtime.gointernal/extension/host_api_session_runtime_projection.gointernal/extension/host_api_test.gointernal/extension/host_api_workspace_binding.gointernal/extensionprotocol/host_api.gointernal/extensionprotocol/host_api_test.gointernal/mcp/serve_projection_decisions.gointernal/observe/reconcile.gointernal/observe/reconcile_test.gointernal/session/additional_test.gointernal/session/manager.gointernal/session/manager_busy_input.gointernal/session/manager_defaults.gointernal/session/manager_integration_test.gointernal/session/manager_prompt_admission.gointernal/session/manager_prompt_input_event.gointernal/session/manager_prompt_request.gointernal/session/manager_prompt_submit.gointernal/session/manager_resume.gointernal/session/manager_resume_run.gointernal/session/manager_runtime_selection.gointernal/session/manager_shutdown.gointernal/session/manager_start_meta.gointernal/session/manager_start_session.gointernal/session/manager_start_test.gointernal/session/manager_start_types.gointernal/session/manager_transition.gointernal/session/manager_transition_test.gointernal/session/manager_types.gointernal/session/provider_lifecycle_test.gointernal/session/query_metadata.gointernal/session/runtime.gointernal/session/session.gointernal/session/session_info.gointernal/session/session_meta.gointernal/session/session_test.gointernal/store/globaldb/global_db_session_input_queue_test.gointernal/store/globaldb/global_db_session_page.gointernal/store/globaldb/global_db_session_scan.gointernal/store/globaldb/global_db_session_test.gointernal/store/globaldb/global_db_session_update.gointernal/store/globaldb/global_db_test.gointernal/store/globaldb/queries/session_core.sqlinternal/store/globaldb/schema/definitions/20_sessions.sqlinternal/store/globaldb/schema/migrations/00048_schema.sqlinternal/store/globaldb/session_core_mapping.gointernal/store/globaldb/session_runtime_selection_mapping.gointernal/store/globaldb/sqlcgen/models.gointernal/store/globaldb/sqlcgen/session_core.sql.gointernal/store/session_meta_types.gointernal/store/session_runtime.gointernal/store/session_runtime_selection.gointernal/store/types_session.gointernal/toolmeta/native_entries.gointernal/toolmeta/registry_test.gointernal/tools/builtin/builtin_test.gointernal/tools/builtin/session_runtime.gointernal/tools/builtin/sessions.gointernal/tools/builtin/toolsets.gointernal/tools/builtin_ids.gopatches/@assistant-ui%[email protected]sdk/go/contracts/host_contracts_gen.gosdk/go/contracts/host_methods_gen.gosdk/go/contracts/types_019_gen.gosdk/go/contracts/types_020_gen.gosdk/go/contracts/types_021_gen.gosdk/go/contracts/types_022_gen.gosdk/go/contracts/types_023_gen.gosdk/go/contracts/types_024_gen.gosdk/go/contracts/types_025_gen.gosdk/go/contracts/types_026_gen.gosdk/go/contracts/types_027_gen.goweb/src/components/assistant-ui/__tests__/session-thread.test.tsxweb/src/components/assistant-ui/hooks/use-thread-provider-identity.tsweb/src/components/assistant-ui/message-actions.logic.tsweb/src/components/assistant-ui/message-actions.tsxweb/src/components/assistant-ui/session-assistant-message.tsxweb/src/components/assistant-ui/session-thread-messages.tsxweb/src/hooks/routes/__tests__/use-session-page-controls.test.tsxweb/src/hooks/routes/use-session-page-controls.tsweb/src/routes/_app/__tests__/-agent-session-deeplink.router.integration.test.tsxweb/src/systems/agent/lib/__tests__/fleet-signals.test.tsweb/src/systems/agent/routes/agent-detail-story-fixtures.tsweb/src/systems/os/apps/session/session-window-view.tsxweb/src/systems/os/components/__tests__/os-window-deck.test.tsxweb/src/systems/os/components/__tests__/sessions-modal.test.tsxweb/src/systems/os/lib/__tests__/attention-model.test.tsweb/src/systems/session/adapters/__tests__/session-api.test.tsweb/src/systems/session/adapters/session-api.tsweb/src/systems/session/components/__tests__/session-chat-runtime-provider.test.tsxweb/src/systems/session/contexts/session-prompt-runtime-context.tsxweb/src/systems/session/hooks/__tests__/use-session-actions.test.tsxweb/src/systems/session/hooks/__tests__/use-session-create-dialog.test.tsxweb/src/systems/session/hooks/__tests__/use-session-prompt-runtime.test.tsxweb/src/systems/session/hooks/__tests__/use-sessions.test.tsxweb/src/systems/session/hooks/use-session-prompt-runtime.tsweb/src/systems/session/hooks/use-session-runtime-selection.tsweb/src/systems/session/index.tsweb/src/systems/session/lib/session-running.tsweb/src/systems/session/mocks/fixtures.tsweb/src/systems/session/mocks/handlers.tsweb/src/systems/session/stores/__tests__/session-prompt-runtime-store.test.tsweb/src/systems/session/stores/session-prompt-runtime-store.tsweb/src/systems/session/types.tsweb/src/systems/workspace/mocks/fixtures.ts
💤 Files with no reviewable changes (1)
- web/src/components/assistant-ui/hooks/use-thread-provider-identity.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/session/additional_test.go (1)
363-371: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAssert
StateActiveafter restart.The test checks that
restarted.Getreturns a session. It does not checkresumed.Info().State. This test cannot distinguish an active session from another returned state.Add the active-state assertion before cleanup.
Proposed test assertion
if !ok { t.Fatalf("Get(%q) found = false, want resumed session", session.ID) } + if got := resumed.Info().State; got != StateActive { + t.Fatalf("resumed state = %q, want %q", got, StateActive) + } t.Cleanup(func() {As per path instructions, tests must focus on state management and verify behavior outcomes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/session/additional_test.go` around lines 363 - 371, Add an assertion immediately after the successful restarted.Get call in the restart test to verify resumed.Info().State equals StateActive, before registering cleanup. Keep the existing session lookup and cleanup behavior unchanged.Source: Path instructions
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/session/additional_test.go`:
- Around line 385-390: Update the Prompt assertions in the test around
h.manager.Prompt to verify the specific expected errors rather than only
checking err != nil. Use the suite’s established error assertion helper and the
expected empty-prompt and missing-session error symbols for each call.
- Around line 569-579: Replace the time.After-based synchronization around
waitForSessionResumes with a deterministic wait-point observer that confirms the
goroutine has entered and is blocked in waitForSessionResumes before closing
pending.done. Split the failed-run and pending-run assertions as needed so the
test explicitly verifies shutdown waits for the pending resume and fails if that
behavior changes.
---
Outside diff comments:
In `@internal/session/additional_test.go`:
- Around line 363-371: Add an assertion immediately after the successful
restarted.Get call in the restart test to verify resumed.Info().State equals
StateActive, before registering cleanup. Keep the existing session lookup and
cleanup behavior unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 826f0495-3b4b-4eaa-ba44-e15a4335a5a1
⛔ Files ignored due to path filters (3)
.codex/agents/explorer.tomlis excluded by!**/*.toml,!.codex/**CLAUDE.mdis excluded by!**/*.mdsdk/typescript/src/generated/contracts.tsis excluded by!**/generated/**,!**/generated/**
📒 Files selected for processing (37)
internal/api/core/conversions_parsers_test.gointernal/api/core/handlers_test.gointernal/cli/session_test.gointernal/daemon/host_api_session_adapter.gointernal/daemon/native_tool_availability.gointernal/daemon/native_tool_binding_groups.gointernal/daemon/native_tool_bindings.gointernal/daemon/native_tools_test.gointernal/extension/contract/host_api.gointernal/extension/host_api_session_runtime.gointernal/extension/host_api_test.gointernal/session/additional_test.gointernal/session/manager_integration_test.gointernal/session/manager_prompt_admission.gointernal/session/manager_resume.gointernal/session/manager_resume_run.gointernal/session/manager_transition_test.gointernal/session/repair_event_payload.gointernal/session/session_test.gointernal/store/globaldb/global_db_session_test.gointernal/store/session_runtime_selection.gosdk/go/contracts/types_020_gen.gosdk/go/contracts/types_021_gen.gosdk/go/contracts/types_022_gen.gosdk/go/contracts/types_023_gen.gosdk/go/contracts/types_024_gen.gosdk/go/contracts/types_025_gen.gosdk/go/contracts/types_026_gen.gosdk/go/contracts/types_027_gen.goweb/src/hooks/routes/__tests__/use-session-page-controls.test.tsxweb/src/systems/session/components/__tests__/session-chat-runtime-provider.test.tsxweb/src/systems/session/hooks/__tests__/use-session-prompt-runtime.test.tsxweb/src/systems/session/hooks/__tests__/use-session-runtime-selection.test.tsxweb/src/systems/session/hooks/use-session-prompt-runtime.tsweb/src/systems/session/hooks/use-session-runtime-selection.tsweb/src/systems/session/stores/__tests__/session-prompt-runtime-store.test.tsweb/src/systems/session/stores/session-prompt-runtime-store.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- internal/cli/session_test.go
- internal/session/manager_resume_run.go
- internal/store/session_runtime_selection.go
- web/src/systems/session/stores/tests/session-prompt-runtime-store.test.ts
- internal/store/globaldb/global_db_session_test.go
- internal/session/manager_prompt_admission.go
- internal/daemon/native_tools_test.go
- web/src/systems/session/hooks/tests/use-session-prompt-runtime.test.tsx
- internal/api/core/conversions_parsers_test.go
- web/src/hooks/routes/tests/use-session-page-controls.test.tsx
- internal/daemon/host_api_session_adapter.go
- web/src/systems/session/hooks/use-session-prompt-runtime.ts
- internal/session/manager_transition_test.go
- internal/session/manager_integration_test.go
- internal/extension/host_api_session_runtime.go
- sdk/go/contracts/types_025_gen.go
- internal/session/manager_resume.go
- internal/api/core/handlers_test.go
- internal/extension/host_api_test.go
- internal/session/session_test.go
Merged
This was referenced Aug 5, 2026
Merged
5 tasks done
This was referenced Aug 7, 2026
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 10, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 14, 2026
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 21, 2026
Merged
Merged
Merged
This was referenced Sep 10, 2026
Merged
Merged
Merged
Merged
This was referenced Sep 27, 2026
Merged
Merged
This was referenced Oct 6, 2026
Merged
Merged
Merged
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Use as Goalmessage action and eliminate the assistant-ui double-unmount error when a window closes while its session is still running.Why
Four behaviors were breaking the core promise of durable sessions:
Use as Goal, but Goal creation is owned by the/goalcommand and the action had no coherent product contract.ReadonlyThreadProvidererror boundary.This change makes the daemon's durable session record the single authority for continuation and next-prompt runtime intent, then projects that truth consistently through CLI, HTTP, UDS, native tools, extensions, generated SDK contracts, and Web.
What changed
Stopped-session continuation
stopped -> starting -> active, recreates the ACP process, loads the stored provider session/history, and submits the prompt against the same Compozy session.session resumedeliberately remains a live attach operation. It never starts a stopped provider process; recap, history, status, repair, and a subsequent normal prompt are the stopped-session paths.Durable runtime selection
Session reads now expose three distinct concepts:
runtime.selectedruntime.effectiveruntime.selection_revision00048_schema.sqlfor the selected provider, model, reasoning effort, speed, and revision on the session row.PUT /api/workspaces/{workspace_id}/sessions/{session_id}/runtimeto set a complete selection andDELETEon the same route to clear it.expected_revision; stale revisions return409 Conflictand require a fresh session read before retrying.compozy session runtime setandcompozy session runtime clear, including structured output and revision fencing.compozy__session_runtime_setandcompozy__session_runtime_clear.sessions/runtime/setandsessions/runtime/clear, workspace binding, permission declarations, generated Go contracts, and TypeScript/OpenAPI projections.selected, then currenteffective. Already queued prompts retain their immutable accepted runtime snapshot.config.tomlkey or default was introduced; this is session-scoped runtime data, not global or workspace configuration.Web behavior
Message actions
Use as Goalfrom settled assistant messages, related logic, stories, design prototypes, and obsolete QA coverage./goalremains the supported Goal entry point through the composer command path.Contracts, docs, and generated artifacts
Use as Goalcharter, added durable-session charters, recorded the stopped-prompt regression, and published the acceptance report.Database migration
00048_schema.sqland updates the declarative session schema plus Atlas checksum.Release notes
🐛 Bug Fixes
Use as Goalaction, and prevent running session windows from double-unmounting assistant-ui resources.QA and verification
make gate-fullpassed for fingerprintc30e7d8ff334328907b6a562e27ebaedce64417c(.cache/gate/logs/full-1785876656.log). This includes code generation drift checks, installer check, source-size guard, Bun/Go lint, TypeScript checks, 4,309 Bun/Vitest tests, Web/site builds, 19,569 race-enabled Go tests, Go build, and architecture boundaries.ReadonlyThreadProvideror resource-fiber teardown error./goal statusremains available.BUG-20260804-stopped-prompt-restart-panic), fixed in production admission code, and passed the canonical regression plus live retest."clean": truewith no surviving daemon, Web, site, browser, watcher, or tmux process.Full report:
docs/qa/reports/2026-08-04-durable-acp-sessions.mdReview hardening
The first full CodeRabbit pass produced 13 inline threads and 7 review-body findings; React Doctor added one compiler finding. The validation pass then added 2 inline findings and 1 outside-diff finding. All 24 findings were verified against the current implementation and addressed. All 15 visible CodeRabbit inline threads now report
isResolved: truethrough GitHub GraphQL; the React Doctor validation check reports 100/100.Resume and shutdown concurrency
defer, so a panic or early exit cannot leave a stale in-flight resume entry that blocks future callers.Runtime contract and capability boundaries
409, matching HTTP, CLI, UDS, and native-tool compare-and-swap behavior.SessionRuntimeSelectionManager.Web lifecycle and stale-response protection
AbortSignalall the way into the HTTP adapter.selection_revision.Regression coverage strengthened
nilat revision zero.finally, verify signal forwarding, and cover stale revision events through both snapshot and persistence paths.StateActiveassertion and exact empty-prompt/missing-session errors.Review verification
Compozy Impact Audit
compozy__session_runtime_setandcompozy__session_runtime_clear; updated tool IDs, descriptors, schema/catalog digest fixtures, runtime toolset membership, binding groups, availability metadata, and native-tool tests. Existing prompt/status tools consume the expanded session runtime projection without fallback behavior.sessions/runtime/setandsessions/runtime/clear, generated SDK contracts, workspace binding, access declarations, and MCP/native projections. Updated the official extension-authoring reference. No lifecycle hook, capability enrollment, MCP sidecar, orconfig.tomlkey/default changed; session runtime intent is managed through the new explicit session surfaces.skills/compozy/references/runtime-operations.md,native-tools.md, andextension-authoring.mdwith stopped-session prompt continuation, selected/effective runtime semantics, revision fencing, the CLI/HTTP/UDS routes, native tool IDs, and Host API methods.Review guide
The highest-value review order is:
internal/session/manager_prompt_admission.go,manager_resume_run.go, andmanager_transition.gofor stopped-session restore and concurrency.internal/store/session_runtime_selection.go, the global DB mappings, and migration00048_schema.sqlfor durability and revision fencing.internal/api/core/handlers_session_runtime.go, CLI/native/Host API adapters, and generated contracts for cross-surface parity.web/src/systems/session/hooks/use-session-runtime-selection.ts, the runtime context/store, and session-running rules for Web synchronization.patches/@assistant-ui%[email protected]plus the session thread/window components for teardown ownership.Summary by CodeRabbit
New Features
Bug Fixes