feat: add runtime per session - #338
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Disabled knowledge base sources:
WalkthroughSession creation now accepts an optional initial prompt across API contracts, durable session startup, CLI commands, host APIs, transport tests, and the web session-creation dialog with integrated runtime control. ChangesInitial prompt session creation
Estimated code review effort: 4 (Complex) | ~65 minutes Sequence Diagram(s)sequenceDiagram
participant Dialog as SessionCreateDialog
participant Hook as useSessionCreateDialog
participant API as createSession
participant Handler as SessionAcceptanceManager
participant Manager as SessionManager
Dialog->>Hook: onPromptChange(text), submit()
Hook->>Hook: trim prompt, validate non-empty
Hook->>API: createSession(agentName, prompt, ...)
API->>Handler: CreateAccepted(InitialPrompt: trimmed)
Handler->>Manager: stage prompt + start session
Manager->>Manager: dispatch prompt to input queue
Manager-->>Handler: return starting session
Handler-->>API: session.state = starting
API-->>Dialog: session created
Hook->>Hook: reset draft to EMPTY_DRAFT
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
internal/session/manager_create_accepted.go (1)
27-29: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winStage the prompt on a cancellation-detached context.
The session is already durably accepted when
stageAcceptedInitialPromptruns, but it enqueues on the caller's request context. A client disconnect between acceptance and enqueue turns into a staging error and rolls the whole accepted session back, even though the durable record already exists. Detaching with a bounded deadline keeps the post-acceptance work independent of the request lifetime.As per coding guidelines, "Work that outlives an HTTP or UDS request must use
context.WithoutCancel(ctx)and reattach a deadline when needed".♻️ Proposed change
func (m *Manager) stageAcceptedInitialPrompt(ctx context.Context, sessionID string, prompt string) error { message := strings.TrimSpace(prompt) if message == "" { return nil } if m.inputQueue == nil { return errors.New("session: input queue is not configured") } - generation, err := m.currentInputGeneration(ctx, sessionID) + stageCtx, cancel := context.WithTimeout(context.WithoutCancel(ctx), defaultLifecycleTimeout) + defer cancel() + generation, err := m.currentInputGeneration(stageCtx, sessionID) if err != nil { return fmt.Errorf("session: read initial prompt queue generation for %q: %w", sessionID, err) } - if _, _, err := m.inputQueue.Enqueue(ctx, sessionID, message, generation); err != nil { + if _, _, err := m.inputQueue.Enqueue(stageCtx, sessionID, message, generation); err != nil { return fmt.Errorf("session: stage initial prompt for %q: %w", sessionID, err) } return nil }Also applies to: 44-58
🤖 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_create_accepted.go` around lines 27 - 29, Update the accepted-session flow around stageAcceptedInitialPrompt to use a context detached from request cancellation via context.WithoutCancel(ctx), while preserving or reattaching an appropriate bounded deadline for the post-acceptance enqueue. Pass this derived context to stageAcceptedInitialPrompt so client disconnects do not roll back an already durable session; apply the same change to the other referenced accepted-session path.Source: Coding guidelines
web/src/systems/session/hooks/use-session-create-dialog.ts (1)
330-337: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueEmpty-prompt submit returns silently.
submitreturns without settingsubmitErrorwhen the trimmed prompt is empty. The dialog'scanSubmitcurrently blocks that path, so it is unreachable from the UI, but any other caller (or a future keyboard shortcut bypassingcanSubmit) gets no feedback. Consider surfacing a message like the other validation branches below.♻️ Optional: surface the validation failure
if (agentName.length === 0) return; - if (prompt.length === 0) return; + if (prompt.length === 0) { + setSubmitError("Write the first message before starting the session."); + return; + }🤖 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 `@web/src/systems/session/hooks/use-session-create-dialog.ts` around lines 330 - 337, Update the empty-prompt validation in submit to set submitError before returning, matching the feedback behavior of the other validation branches. Keep the trimmed prompt check and existing early-return flow unchanged.web/src/systems/session/components/session-create-prompt-composer.tsx (1)
77-94: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer the shared
Buttonprimitive over a hand-rolled submit button.The send control re-implements sizing, hover, focus-ring, and disabled styling inline. Using the
@agh/uiButton(icon size + accent variant) keeps this composer aligned with the design system as those tokens evolve.🤖 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 `@web/src/systems/session/components/session-create-prompt-composer.tsx` around lines 77 - 94, The session-create send control should use the shared `@agh/ui` Button primitive instead of the hand-rolled button. Update the button in the session creation prompt composer to use the primitive’s submit behavior with its icon size and accent variant, while preserving data-testid, disabled={!canSubmit}, loading icon content, and accessibility labeling.
🤖 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/extension/host_api_test.go`:
- Around line 137-174: Wrap the body of
TestHostAPIHandlerSessionsCreateUsesAtomicAcceptedPrompt in a t.Run subtest
named with the “Should ...” convention, keeping the existing setup, handler
invocation, and assertions inside that subtest.
---
Nitpick comments:
In `@internal/session/manager_create_accepted.go`:
- Around line 27-29: Update the accepted-session flow around
stageAcceptedInitialPrompt to use a context detached from request cancellation
via context.WithoutCancel(ctx), while preserving or reattaching an appropriate
bounded deadline for the post-acceptance enqueue. Pass this derived context to
stageAcceptedInitialPrompt so client disconnects do not roll back an already
durable session; apply the same change to the other referenced accepted-session
path.
In `@web/src/systems/session/components/session-create-prompt-composer.tsx`:
- Around line 77-94: The session-create send control should use the shared
`@agh/ui` Button primitive instead of the hand-rolled button. Update the button in
the session creation prompt composer to use the primitive’s submit behavior with
its icon size and accent variant, while preserving data-testid,
disabled={!canSubmit}, loading icon content, and accessibility labeling.
In `@web/src/systems/session/hooks/use-session-create-dialog.ts`:
- Around line 330-337: Update the empty-prompt validation in submit to set
submitError before returning, matching the feedback behavior of the other
validation branches. Keep the trimmed prompt check and existing early-return
flow unchanged.
🪄 Autofix (Beta)
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: fb608e32-1c81-4436-b3e4-b3597499397f
⛔ Files ignored due to path filters (18)
docs/qa/scenarios/RT-010.mdis excluded by!**/*.mddocs/qa/scenarios/RT-063.mdis excluded by!**/*.mddocs/qa/scenarios/RT-072.mdis excluded by!**/*.mddocs/qa/scenarios/RT-new-session-fast-feedback.mdis excluded by!**/*.mdopenapi/agh.jsonis excluded by!**/*.jsonpackages/site/content/runtime/cli-reference/session/new.mdxis excluded by!**/*.mdxpackages/site/content/runtime/core/sessions/lifecycle.mdxis excluded by!**/*.mdxskills/agh/references/runtime-operations.mdis excluded by!**/*.mdweb/e2e/__tests__/agent-categories.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/knowledge.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/marketplace.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/session-onboarding.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/session-provider-override.spec.tsis excluded by!web/e2e/**web/e2e/__tests__/tasks-coordinator-handoff.spec.tsis excluded by!web/e2e/**web/e2e/fixtures/selectors.tsis excluded by!**/fixtures/**,!web/e2e/**web/src/generated/agh-openapi.d.tsis excluded by!**/generated/**,!**/generated/**,!**/*.d.tsweb/src/systems/runtime/components/stories/runtime-selector.stories.tsxis excluded by!**/*.stories.tsxweb/src/systems/session/components/stories/session-create-dialog.stories.tsxis excluded by!**/*.stories.tsx
📒 Files selected for processing (30)
internal/api/contract/contract_test.gointernal/api/contract/session_runtime_payloads.gointernal/api/core/handlers.gointernal/api/core/handlers_test.gointernal/api/core/interfaces.gointernal/api/testutil/session_stub.gointernal/api/udsapi/transport_parity_integration_test.gointernal/cli/session_create.gointernal/cli/session_test.gointernal/extension/host_api.gointernal/extension/host_api_sessions.gointernal/extension/host_api_test.gointernal/session/manager_create_accepted.gointernal/session/manager_delete_staging.gointernal/session/manager_start.gointernal/session/manager_start_launch_accepted.gointernal/session/manager_start_test.gointernal/session/manager_types.goweb/src/systems/os/components/desktop-shell.tsxweb/src/systems/runtime/components/runtime-selector/__tests__/runtime-selector.test.tsxweb/src/systems/runtime/components/runtime-selector/trigger.tsxweb/src/systems/runtime/components/runtime-selector/types.tsweb/src/systems/session/adapters/__tests__/session-api.test.tsweb/src/systems/session/components/__tests__/session-create-dialog.test.tsxweb/src/systems/session/components/session-create-dialog.tsxweb/src/systems/session/components/session-create-prompt-composer.tsxweb/src/systems/session/hooks/__tests__/use-session-create-dialog.test.tsxweb/src/systems/session/hooks/use-session-create-dialog.tsweb/src/systems/session/mocks/__tests__/handlers.test.tsweb/src/systems/session/mocks/handlers.ts
Code Review Could Not Complete
|
| Options | Enabled |
|---|---|
| Bug | ✅ |
| Performance | ✅ |
| Security | ✅ |
| Business Logic | ✅ |
Summary by CodeRabbit
--promptoption to the session creation command.