fix(backend/frontend): error handling, stream reconnection, and chat switching - #12205
Conversation
…limit Previously, task_spawn_count only incremented and never decremented, making it a per-session lifetime cap. Once 10 subtasks were spawned (even if all had completed), no more could be created — forcing the agent to work without sub-agents for the rest of the session. Now the counter decrements in PostToolUse when a Task completes, making it a true concurrency limit. Finished subtasks free their slots for reuse.
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
github-actions
Bot
added
platform/frontend
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 6 conflict(s), 0 medium risk, 2 low risk (out of 8 PRs with file overlap) Auto-generated on push. Ignores: |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
When the SSE stream drops unexpectedly (status transitions to "error" while backend is still running), the frontend now automatically: 1. Refetches the session to check for active_stream 2. Resets the resume guard so the existing resume effect fires 3. Calls resumeStream() to reconnect to the Redis-backed stream Capped at 3 reconnect attempts per session to avoid infinite loops. On clean finish (status "ready"), the counter resets.
352a793 to
db3d10f
Compare
Failed Tasks were not decrementing task_spawn_count, permanently consuming a concurrency slot. Add the same release logic to post_tool_failure_hook so slots are freed regardless of outcome.
|
Addressed Sentry's review comment in 142198d: |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughClarified a config field to indicate a concurrent subtask limit; replaced a per-session scalar counter with per-session concurrency tracking and added slot release on task completion/failure in backend hooks; exposed Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI
participant Page as useCopilotPage
participant Session as useChatSession
participant Server as Server
UI->>Page: observe stream / mount
Page->>Session: subscribe + get sessionQuery + refetchSession
Session->>Server: open stream / fetch session
Server-->>Session: stream events (active / ready / error / idle)
Session-->>Page: emit status updates
alt stream ends (idle or error) and prior activity & valid sessionId
Page->>Page: compute backoff delay, increment attempts
Page->>Session: schedule refetchSession (after delay)
Session->>Server: refetch session
else stream ready
Page->>Page: clear per-session reconnect state
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (1)
236-289:⚠️ Potential issue | 🟠 MajorChange return type from
SyncHookJSONOutputtodict[str, Any]for async hook functions.Per the Claude Agent SDK v0.1.39 documentation, async hooks must have return type
dict[str, Any], notSyncHookJSONOutput. The word "Sync" in the type name contradicts these async functions. Remove thecast()calls on lines 254, 258, and 265 and update the return type annotation forpost_tool_use_hook,pre_tool_use_hook, andpost_tool_failure_hooktodict[str, Any].🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks.py` around lines 236 - 289, Update the async hook functions post_tool_use_hook, pre_tool_use_hook, and post_tool_failure_hook to use the async-compatible return annotation dict[str, Any] instead of SyncHookJSONOutput, and return plain dicts (e.g., {}) directly; remove the cast(...) wrappers around returned values (the cast calls used around SyncHookJSONOutput) so the functions return a native dict[str, Any] without casting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks.py`:
- Around line 253-260: The post-hook currently decrements task_spawn_count
blindly (only guarded by task_spawn_count > 0) so a PostToolUse fired for a
pre-denied Task could release a slot that was never taken; introduce a
closure-scoped Set[str] (e.g., task_tool_use_ids) to record tool_use_id values
when you actually increment task_spawn_count (in the PreToolUse/where Task
increments), then in post_tool_use_hook and post_tool_failure_hook only
decrement task_spawn_count if the incoming tool_use_id is present in
task_tool_use_ids and remove it after decrementing; update logging (the
logger.info call using task_spawn_count/max_subtasks/user_id) to remain the same
but ensure the decrement only runs when the id was tracked.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 245-280: The reconnectAttemptsRef map is never cleared when the
user navigates away, so stale counters block future retries for that session;
update the effect to detect sessionId changes and delete the old session's
counter from reconnectAttemptsRef (and clear any related state like
hasResumedRef and pending reconnectTimerRef) when sessionId changes — e.g., keep
a prevSessionIdRef, on change call reconnectAttemptsRef.current.delete(prevId)
(and clearTimeout(reconnectTimerRef.current) /
hasResumedRef.current.delete(prevId) as needed) before setting up the new
reconnect logic in the existing useEffect that references sessionId,
reconnectTimerRef, reconnectAttemptsRef, prevStatusRef, hasResumedRef, and
refetchSession.
- Around line 252-280: The bug is cross-session bleeding of prevStatusRef when
sessionId changes; to fix it, detect session switches at the start of the
useEffect that watches status/sessionId and reset prevStatusRef for the new
session. Concretely, add a ref like prevSessionIdRef (or reuse an existing one),
and inside the useEffect check if prevSessionIdRef.current !== sessionId; if so
set prevStatusRef.current = undefined (or null) and update
prevSessionIdRef.current = sessionId before using prevStatusRef.current, so the
logic around wasActive/isIdle, queryClient.invalidateQueries,
reconnectAttemptsRef, hasResumedRef, and reconnectTimerRef uses a clean
previous-status for each session.
---
Outside diff comments:
In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks.py`:
- Around line 236-289: Update the async hook functions post_tool_use_hook,
pre_tool_use_hook, and post_tool_failure_hook to use the async-compatible return
annotation dict[str, Any] instead of SyncHookJSONOutput, and return plain dicts
(e.g., {}) directly; remove the cast(...) wrappers around returned values (the
cast calls used around SyncHookJSONOutput) so the functions return a native
dict[str, Any] without casting.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: end-to-end tests
- GitHub Check: test (3.12)
- GitHub Check: test (3.11)
- GitHub Check: test (3.13)
- GitHub Check: Check PR Status
🧰 Additional context used
📓 Path-based instructions (15)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Fully capitalize acronyms in symbols, e.g.graphID,useBackendAPI
Use function declarations (not arrow functions) for components and handlers
Separate render logic (.tsx) from business logic (use*.tshooks)
Use shadcn/ui (Radix UI primitives) with Tailwind CSS styling for UI components
Use Phosphor Icons only for icons
Use ErrorCard for render errors, toast for mutations, and Sentry for exceptions
Use design system components fromsrc/components/(atoms, molecules, organisms)
Never usesrc/components/__legacy__/*components
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Use Tailwind CSS only for styling, with design tokens
Do not useuseCallbackoruseMemounless asked to optimize a given function
Never type withanyunless a variable/attribute can ACTUALLY be of any type
autogpt_platform/frontend/src/**/*.{ts,tsx}: Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.tsand use design system components fromsrc/components/(atoms, molecules, organisms)
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}and regenerate withpnpm generate:api
Use function declarations (not arrow functions) for components and handlers
Separate render logic from business logic with component.tsx + useComponent.ts + helpers.ts structure
Colocate state when possible, avoid creating large components, use sub-components in local/componentsfolder
Avoid large hooks, abstract logic intohelpers.tsfiles when sensible
Use arrow functions only for callbacks, not for component declarations
Avoid comments at all times unless the code is very complex
Do not useuseCallbackoruseMemounless asked to optimize a given function
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*use*.ts
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Do not type hook returns, let TypeScript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}: Format frontend code usingpnpm format
Never use components fromsrc/components/__legacy__/*
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS only for styling, use design tokens, and use Phosphor Icons only
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
🧠 Learnings (1)
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{tsx,ts} : Use generated API hooks from '@/app/api/__generated__/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/*'
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
🔇 Additional comments (5)
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (2)
209-218: LGTM — fixture correctly exposes triple of hooks.The updated
_hooksfixture properly returns(pre, post, post_failure)and each test unpacks only the handle it needs. Default function-scope ensures each test gets a freshtask_spawn_count=0closure.
272-345: LGTM — new slot-release tests are thorough and symmetric.Both
test_task_slot_released_on_completionandtest_task_slot_released_on_failurefollow the same fill-to-capacity → verify-denial → release-one → verify-allowed pattern, giving clear coverage of both happy and failure paths.autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts (1)
113-123: LGTM — minimal, correct exposure ofrefetchSession.Forwarding
sessionQuery.refetchdirectly keeps the return type inferred and is the idiomatic pattern for exposing React Query refetch handles.autogpt_platform/backend/backend/copilot/config.py (1)
86-89: LGTM — description correctly reflects the new concurrency semantics.autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
19-20: LGTM — reconnect constants are appropriately conservative.3 attempts with 1s base delay (1s / 2s / 4s) gives a reasonable 7-second retry window without hammering the backend.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
244-246: Memory consideration:reconnectAttemptsRefMap is never pruned.Entries are only deleted on successful reconnect (
status === "ready"). If a user visits many sessions that all error out without recovering, entries accumulate. This is unlikely to matter in practice (session IDs are small strings), but you could add a cleanup whensessionIdchanges, similar to howhasResumedRefis used.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts around lines 244 - 246, reconnectAttemptsRef is never pruned which can accumulate entries for visited sessions; add a cleanup effect that runs when sessionId changes (similar to hasResumedRef handling) to remove stale entries from reconnectAttemptsRef (e.g., delete all keys !== sessionId or reset the map if null) so only the current session's retry count is kept; reference reconnectAttemptsRef and sessionId inside that useEffect to perform the pruning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 252-280: When reconnect attempts are exhausted the effect silently
returns and the user gets no feedback, and the global reconnectTimerRef.current
is overwritten which can drop the first timeout ID; modify the effect so that
when attempts >= RECONNECT_MAX_ATTEMPTS you set an explicit error/notification
(e.g., trigger a toast or set an error state via your existing UI/error handler)
referencing reconnectAttemptsRef and RECONNECT_MAX_ATTEMPTS so the UI shows a
final failure, and change the timer handling to use a local const (e.g., const
timer = setTimeout(...)) and clear that local timer in the cleanup instead of
directly mutating reconnectTimerRef.current (you can still keep
reconnectTimerRef for other usages but avoid assigning it unconditionally here),
keeping the existing logic that calls refetchSession() and deletes hasResumedRef
on retry and using
queryClient.invalidateQueries(getGetV2GetSessionQueryKey(sessionId)) as before.
---
Nitpick comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 244-246: reconnectAttemptsRef is never pruned which can accumulate
entries for visited sessions; add a cleanup effect that runs when sessionId
changes (similar to hasResumedRef handling) to remove stale entries from
reconnectAttemptsRef (e.g., delete all keys !== sessionId or reset the map if
null) so only the current session's retry count is kept; reference
reconnectAttemptsRef and sessionId inside that useEffect to perform the pruning.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Fully capitalize acronyms in symbols, e.g.graphID,useBackendAPI
Use function declarations (not arrow functions) for components and handlers
Separate render logic (.tsx) from business logic (use*.tshooks)
Use shadcn/ui (Radix UI primitives) with Tailwind CSS styling for UI components
Use Phosphor Icons only for icons
Use ErrorCard for render errors, toast for mutations, and Sentry for exceptions
Use design system components fromsrc/components/(atoms, molecules, organisms)
Never usesrc/components/__legacy__/*components
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Use Tailwind CSS only for styling, with design tokens
Do not useuseCallbackoruseMemounless asked to optimize a given function
Never type withanyunless a variable/attribute can ACTUALLY be of any type
autogpt_platform/frontend/src/**/*.{ts,tsx}: Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.tsand use design system components fromsrc/components/(atoms, molecules, organisms)
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}and regenerate withpnpm generate:api
Use function declarations (not arrow functions) for components and handlers
Separate render logic from business logic with component.tsx + useComponent.ts + helpers.ts structure
Colocate state when possible, avoid creating large components, use sub-components in local/componentsfolder
Avoid large hooks, abstract logic intohelpers.tsfiles when sensible
Use arrow functions only for callbacks, not for component declarations
Avoid comments at all times unless the code is very complex
Do not useuseCallbackoruseMemounless asked to optimize a given function
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*use*.ts
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Do not type hook returns, let TypeScript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}: Format frontend code usingpnpm format
Never use components fromsrc/components/__legacy__/*
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS only for styling, use design tokens, and use Phosphor Icons only
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
🔇 Additional comments (8)
autogpt_platform/backend/backend/copilot/config.py (1)
86-89: LGTM — description accurately reflects the new concurrency semantics.The wording change from "spawn" to "concurrent" correctly aligns with the runtime behavior introduced in
security_hooks.pywheretask_spawn_countnow decrements on task completion/failure.autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (3)
248-260: Solid concurrency-release logic with proper underflow guard.The
task_spawn_count > 0check on both the success and failure paths is a good defensive measure preventing negative drift if the SDK were to fire a post hook without a matching pre-increment (e.g., for a denied tool call). Thenonlocalsharing across closures is correct given Python's single-threaded asyncio model.
291-313: Good addition — failed tasks now correctly release their concurrency slot.Mirrors the completion path consistently. The logging distinguishes failure from success which aids debugging.
209-213: Improved denial message is clear and actionable.The updated message tells the user both what the limit is and what they can do about it (wait or continue in the main conversation). Good UX improvement.
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (2)
209-218: Good refactor of the fixture to expose all three hook handlers.Clean unpacking pattern that makes the new concurrency tests possible while remaining backward-compatible with existing tests via
pre, _, _.
272-345: Thorough tests for both the completion and failure slot-release paths.Both tests follow the same rigorous pattern: fill to capacity → verify denial → release one slot → verify a new task is allowed. This directly validates the core behavioral change of this PR.
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts (1)
122-122: Clean exposure of the React Query refetch capability.Minimal addition that enables the reconnect flow in
useCopilotPagewithout introducing new state or side effects.autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
19-20: Reasonable reconnect constants.3 attempts with exponential backoff (1s → 2s → 4s) is a sensible default for SSE reconnection. Total wait before giving up is ~7 seconds, which balances user experience with recovery probability.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (2)
245-246: Minor:reconnectAttemptsRefentries for errored sessions are never cleaned up.Entries are deleted on
"ready"(line 265) but persist for sessions that exhaust retries. Over a long-lived page with many failed sessions, the Map grows (though practically this is negligible). Consider clearing entries whensessionIdchanges or when the component unmounts if you want to be thorough.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts around lines 245 - 246, reconnectAttemptsRef currently holds Map entries for sessions that exhaust retries and are only deleted on the "ready" path; add cleanup to remove entries when the active session changes and on component unmount to prevent indefinite growth. Specifically, in the hook where sessionId is tracked (the same area that deletes entries on "ready"), add a useEffect that watches sessionId and deletes reconnectAttemptsRef.current.get(oldSessionId) (or remove the key for the previous session) whenever sessionId changes, and also return a cleanup function from a top-level useEffect to clear reconnectAttemptsRef.current (or delete relevant keys) on unmount; keep the existing deletion-on-"ready" logic intact.
252-280: Reconnect logic looks solid overall — one edge case to consider.The reconnect flow is well-designed: invalidate cache → backoff → refetch → resume effect re-triggers. The cleanup properly clears pending timers on effect re-runs.
One thing to note: if
refetchSession()fires and the backend reportsactive_stream = false(i.e., the task finished naturally while we were retrying), the resume effect won't run andstatuswill remain"error"with no further transitions. The stale error state may confuse users since the session is actually complete. Consider invalidating the cache and checking whether the session is done post-refetch, potentially resetting the error by setting messages from the refreshed session data.That said, the cache invalidation on line 260–262 should trigger
hydratedMessagesto re-derive, and the hydration effect (lines 231–239) would update messages sincestatusis no longer"streaming"or"submitted". So the messages should render correctly even if the status label stays"error". This may be acceptable as-is — flagging for awareness.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts around lines 252 - 280, The reconnect effect may leave UI stuck in "error" if refetchSession returns a session where active_stream is false; to fix, after invalidating the cache call refetchSession and inspect its result (use the existing refetchSession() call), and if the returned session indicates finished (active_stream === false) then update state/hydration so the error is cleared — e.g., trigger the same message hydration path used elsewhere (set messages from the refreshed session or call the hydration helper), reset reconnectAttemptsRef.current.delete(sessionId) and hasResumedRef.current.delete(sessionId), and set status to "ready" (or invoke the existing resume/ready handler) so the UI reflects the completed session; keep the backoff flow otherwise. Ensure you reference getGetV2GetSessionQueryKey, queryClient.invalidateQueries, refetchSession, reconnectAttemptsRef, and hasResumedRef when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 245-246: reconnectAttemptsRef currently holds Map entries for
sessions that exhaust retries and are only deleted on the "ready" path; add
cleanup to remove entries when the active session changes and on component
unmount to prevent indefinite growth. Specifically, in the hook where sessionId
is tracked (the same area that deletes entries on "ready"), add a useEffect that
watches sessionId and deletes reconnectAttemptsRef.current.get(oldSessionId) (or
remove the key for the previous session) whenever sessionId changes, and also
return a cleanup function from a top-level useEffect to clear
reconnectAttemptsRef.current (or delete relevant keys) on unmount; keep the
existing deletion-on-"ready" logic intact.
- Around line 252-280: The reconnect effect may leave UI stuck in "error" if
refetchSession returns a session where active_stream is false; to fix, after
invalidating the cache call refetchSession and inspect its result (use the
existing refetchSession() call), and if the returned session indicates finished
(active_stream === false) then update state/hydration so the error is cleared —
e.g., trigger the same message hydration path used elsewhere (set messages from
the refreshed session or call the hydration helper), reset
reconnectAttemptsRef.current.delete(sessionId) and
hasResumedRef.current.delete(sessionId), and set status to "ready" (or invoke
the existing resume/ready handler) so the UI reflects the completed session;
keep the backoff flow otherwise. Ensure you reference
getGetV2GetSessionQueryKey, queryClient.invalidateQueries, refetchSession,
reconnectAttemptsRef, and hasResumedRef when making the change.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
autogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (15)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx,js,jsx}: Use Node.js 21+ with pnpm package manager for frontend development
Always run 'pnpm format' for formatting and linting code in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{tsx,ts}: Use function declarations for components and handlers (not arrow functions) in React components
Only use arrow functions for small inline lambdas (map, filter, etc.) in React components
Use PascalCase for component names and camelCase with 'use' prefix for hook names in React
Use Tailwind CSS utilities only for styling in frontend components
Use design system components from 'src/components/' (atoms, molecules, organisms) in frontend development
Never use 'src/components/legacy/' in frontend code
Only use Phosphor Icons (@phosphor-icons/react) for icons in frontend components
Use generated API hooks from '@/app/api/generated/endpoints/' instead of deprecated 'BackendAPI' or 'src/lib/autogpt-server-api/'
Use React Query for server state (via generated hooks) in frontend development
Default to client components ('use client') in Next.js; only use server components for SEO or extreme TTFB needs
Use '' component for rendering errors in frontend UI; use toast notifications for mutation errors; use 'Sentry.captureException()' for manual exceptions
Separate render logic from data/behavior in React components; keep comments minimal (code should be self-documenting)
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/frontend/**/*.{ts,tsx}: No barrel files or 'index.ts' re-exports in frontend code
Regenerate API hooks with 'pnpm generate:api' after backend OpenAPI spec changes in frontend development
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
autogpt_platform/frontend/src/**/*.{ts,tsx}: Fully capitalize acronyms in symbols, e.g.graphID,useBackendAPI
Use function declarations (not arrow functions) for components and handlers
Separate render logic (.tsx) from business logic (use*.tshooks)
Use shadcn/ui (Radix UI primitives) with Tailwind CSS styling for UI components
Use Phosphor Icons only for icons
Use ErrorCard for render errors, toast for mutations, and Sentry for exceptions
Use design system components fromsrc/components/(atoms, molecules, organisms)
Never usesrc/components/__legacy__/*components
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}
Use Tailwind CSS only for styling, with design tokens
Do not useuseCallbackoruseMemounless asked to optimize a given function
Never type withanyunless a variable/attribute can ACTUALLY be of any type
autogpt_platform/frontend/src/**/*.{ts,tsx}: Structure components asComponentName/ComponentName.tsx+useComponentName.ts+helpers.tsand use design system components fromsrc/components/(atoms, molecules, organisms)
Use generated API hooks from@/app/api/__generated__/endpoints/with patternuse{Method}{Version}{OperationName}and regenerate withpnpm generate:api
Use function declarations (not arrow functions) for components and handlers
Separate render logic from business logic with component.tsx + useComponent.ts + helpers.ts structure
Colocate state when possible, avoid creating large components, use sub-components in local/componentsfolder
Avoid large hooks, abstract logic intohelpers.tsfiles when sensible
Use arrow functions only for callbacks, not for component declarations
Avoid comments at all times unless the code is very complex
Do not useuseCallbackoruseMemounless asked to optimize a given function
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*use*.ts
📄 CodeRabbit inference engine (autogpt_platform/frontend/CLAUDE.md)
Do not type hook returns, let TypeScript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx}: Format frontend code usingpnpm format
Never use components fromsrc/components/__legacy__/*
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/**/*.{js,jsx,ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
Use Tailwind CSS only for styling, use design tokens, and use Phosphor Icons only
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/frontend/src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not type hook returns, let Typescript infer as much as possible
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Never type with
any, if no types available useunknown
Files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
🧠 Learnings (1)
📚 Learning: 2026-02-04T16:49:42.490Z
Learnt from: CR
Repo: Significant-Gravitas/AutoGPT PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-02-04T16:49:42.490Z
Learning: Applies to autogpt_platform/frontend/**/*.{tsx,ts} : Use React Query for server state (via generated hooks) in frontend development
Applied to files:
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts
🧬 Code graph analysis (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (2)
autogpt_platform/backend/backend/data/db_manager.py (1)
_(175-184)autogpt_platform/backend/backend/util/logging.py (3)
info(41-43)error(49-51)warning(45-47)
🔇 Additional comments (9)
autogpt_platform/backend/backend/copilot/config.py (1)
86-89: LGTM! Description accurately reflects the new concurrency-limit semantics introduced insecurity_hooks.py.autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts (1)
122-122: LGTM! Clean exposure of the React Queryrefetchhandle — enables the reconnect flow inuseCopilotPagewithout altering existing behavior.autogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.ts (1)
19-20: LGTM on the reconnect constants. Exponential backoff with delays of 1 s → 2 s → 4 s and a 3-attempt cap is reasonable for SSE reconnect.autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (3)
297-313: LGTM — failure path mirrors completion path.Slot release on failure ensures failed Tasks don't permanently consume concurrency slots. Consistent with the
post_tool_use_hookpattern.
210-213: Good UX improvement on the denial message. The new message clearly communicates that this is a concurrency limit (not permanent) and gives actionable advice ("wait" or "continue in main conversation").
248-260: Remove the thread safety concern — SDK hooks run on a single asyncio event loop.The code is safe. SDK hooks are dispatched asynchronously via
start_soon()(single event loop, no threading), making thenonlocal intdecrement atomic. The> 0guard remains good defensive coding, preventing underflow if a completion hook fires without a matching pre-hook increment.Likely an incorrect or invalid review comment.
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (3)
209-218: LGTM — fixture cleanly exposes all three hook handlers. Function-scoped by default, ensuring each test gets isolatedtask_spawn_countstate.
272-307: Well-structured test for slot release on completion. The fill → deny → release → allow pattern clearly validates the concurrency accounting.
310-345: Good coverage of the failure path. Mirrors the completion test, ensuring failed Tasks also release their slots.
Use a per-session set (task_tool_use_ids) to record which tool_use_ids actually consumed a concurrency slot. Post hooks only release a slot if the tool_use_id is in the set, preventing spurious decrements if PostToolUse fires for a pre-denied call or the same id completes twice. Addresses CodeRabbit review feedback.
|
Addressed CodeRabbit's review in 3c7545c: slot accounting now uses a per-session |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (1)
192-236:⚠️ Potential issue | 🟠 MajorReserve a Task slot only after all pre-checks pass.
task_spawn_countis incremented before_validate_tool_accessand_validate_user_isolation. If either check denies, the task never starts but the slot remains consumed, which can permanently exhaust session concurrency.Proposed fix
# Rate-limit Task (sub-agent) spawns per session if tool_name == "Task": # Block background task execution first — denied calls # should not consume a subtask slot. if tool_input.get("run_in_background"): logger.info(f"[SDK] Blocked background Task, user={user_id}") return cast( SyncHookJSONOutput, _deny( "Background task execution is not supported. " "Run tasks in the foreground instead " "(remove the run_in_background parameter)." ), ) - if task_spawn_count >= max_subtasks: - logger.warning( - f"[SDK] Task limit reached ({max_subtasks}), user={user_id}" - ) - return cast( - SyncHookJSONOutput, - _deny( - f"Maximum {max_subtasks} concurrent sub-tasks. " - "Wait for running sub-tasks to finish, " - "or continue in the main conversation." - ), - ) - task_spawn_count += 1 - if tool_use_id: - task_tool_use_ids.add(tool_use_id) # Strip MCP prefix for consistent validation is_copilot_tool = tool_name.startswith(MCP_TOOL_PREFIX) clean_name = tool_name.removeprefix(MCP_TOOL_PREFIX) @@ # Validate user isolation result = _validate_user_isolation(clean_name, tool_input, user_id) if result: return cast(SyncHookJSONOutput, result) + + # Reserve a slot only for Task calls that passed all validations. + if tool_name == "Task": + if task_spawn_count >= max_subtasks: + logger.warning( + f"[SDK] Task limit reached ({max_subtasks}), user={user_id}" + ) + return cast( + SyncHookJSONOutput, + _deny( + f"Maximum {max_subtasks} concurrent sub-tasks. " + "Wait for running sub-tasks to finish, " + "or continue in the main conversation." + ), + ) + task_spawn_count += 1 + if tool_use_id: + task_tool_use_ids.add(tool_use_id)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks.py` around lines 192 - 236, The code currently increments task_spawn_count and adds to task_tool_use_ids before running _validate_tool_access and _validate_user_isolation, which can consume a slot on denied requests; move the reservation logic (task_spawn_count += 1 and task_tool_use_ids.add(tool_use_id)) so it only runs after both _validate_tool_access(clean_name, ...) and _validate_user_isolation(clean_name, ...) succeed (i.e., after is_copilot_tool handling and both validations return falsy), and ensure any early returns (denies) happen before modifying task_spawn_count/task_tool_use_ids; keep references to tool_name, tool_use_id, task_spawn_count, task_tool_use_ids, _validate_tool_access, _validate_user_isolation, MCP_TOOL_PREFIX, is_copilot_tool, and clean_name to locate where to move the lines.
🧹 Nitpick comments (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (1)
272-346: Add regressions for untracked and duplicatetool_use_idpost events.These tests validate release on normal completion/failure, but they don’t lock in the new defensive behavior against spurious/duplicate callbacks. Add one test for an unknown
tool_use_idand one for duplicate completion calls.Suggested test additions
+@pytest.mark.skipif(not _sdk_available(), reason="claude_agent_sdk not installed") +@pytest.mark.asyncio +async def test_task_slot_not_released_for_untracked_completion(_hooks): + pre, post, _ = _hooks + for i in range(2): + result = await pre( + {"tool_name": "Task", "tool_input": {"prompt": "ok"}}, + tool_use_id=f"tu-untracked-{i}", + context={}, + ) + assert not _is_denied(result) + + # Unknown completion id should not free capacity + await post({"tool_name": "Task", "tool_input": {}}, tool_use_id="tu-unknown", context={}) + + result = await pre( + {"tool_name": "Task", "tool_input": {"prompt": "still full"}}, + tool_use_id="tu-untracked-2", + context={}, + ) + assert _is_denied(result) + + +@pytest.mark.skipif(not _sdk_available(), reason="claude_agent_sdk not installed") +@pytest.mark.asyncio +async def test_task_slot_released_only_once_for_duplicate_completion(_hooks): + pre, post, _ = _hooks + for i in range(2): + result = await pre( + {"tool_name": "Task", "tool_input": {"prompt": "ok"}}, + tool_use_id=f"tu-dup-{i}", + context={}, + ) + assert not _is_denied(result) + + await post({"tool_name": "Task", "tool_input": {}}, tool_use_id="tu-dup-0", context={}) + await post({"tool_name": "Task", "tool_input": {}}, tool_use_id="tu-dup-0", context={}) + + # Only one slot should be freed + ok = await pre( + {"tool_name": "Task", "tool_input": {"prompt": "one free slot"}}, + tool_use_id="tu-dup-2", + context={}, + ) + assert not _is_denied(ok) + + denied = await pre( + {"tool_name": "Task", "tool_input": {"prompt": "no second free slot"}}, + tool_use_id="tu-dup-3", + context={}, + ) + assert _is_denied(denied)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py` around lines 272 - 346, Add two regression tests in security_hooks_test.py that exercise untracked and duplicate post callbacks: use the existing fixtures and helpers (pre, post, post_failure, and _is_denied) to (1) call post or post_failure with a tool_use_id that was never accepted (e.g., "tu-unknown") and assert it is treated as a no-op (no exception and capacity remains unchanged so a subsequent pre for a new Task is still denied if at capacity), and (2) call post twice for the same accepted tool_use_id and assert the second call is safe (no exception and does not double-free a slot) by checking capacity with a subsequent pre call. Ensure tests mirror the style of test_task_slot_released_on_completion and test_task_slot_released_on_failure and use tool_name "Task" and _is_denied to validate capacity.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks.py`:
- Around line 192-236: The code currently increments task_spawn_count and adds
to task_tool_use_ids before running _validate_tool_access and
_validate_user_isolation, which can consume a slot on denied requests; move the
reservation logic (task_spawn_count += 1 and task_tool_use_ids.add(tool_use_id))
so it only runs after both _validate_tool_access(clean_name, ...) and
_validate_user_isolation(clean_name, ...) succeed (i.e., after is_copilot_tool
handling and both validations return falsy), and ensure any early returns
(denies) happen before modifying task_spawn_count/task_tool_use_ids; keep
references to tool_name, tool_use_id, task_spawn_count, task_tool_use_ids,
_validate_tool_access, _validate_user_isolation, MCP_TOOL_PREFIX,
is_copilot_tool, and clean_name to locate where to move the lines.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py`:
- Around line 272-346: Add two regression tests in security_hooks_test.py that
exercise untracked and duplicate post callbacks: use the existing fixtures and
helpers (pre, post, post_failure, and _is_denied) to (1) call post or
post_failure with a tool_use_id that was never accepted (e.g., "tu-unknown") and
assert it is treated as a no-op (no exception and capacity remains unchanged so
a subsequent pre for a new Task is still denied if at capacity), and (2) call
post twice for the same accepted tool_use_id and assert the second call is safe
(no exception and does not double-free a slot) by checking capacity with a
subsequent pre call. Ensure tests mirror the style of
test_task_slot_released_on_completion and test_task_slot_released_on_failure and
use tool_name "Task" and _is_denied to validate capacity.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*.{py,txt}
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use
poetry runprefix for all Python commands, including testing, linting, formatting, and migrations
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
autogpt_platform/backend/**/*_test.py: Always review snapshot changes withgit diffbefore committing when updating snapshots withpoetry run pytest --snapshot-update
Use pytest with snapshot testing for API responses in test files
Colocate test files with source files using the*_test.pynaming convention
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
autogpt_platform/backend/backend/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Use Prisma ORM for database operations in PostgreSQL with pgvector for embeddings
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.pyautogpt_platform/backend/backend/copilot/sdk/security_hooks.py
autogpt_platform/backend/**/*test*.py
📄 CodeRabbit inference engine (AGENTS.md)
Run
poetry run testfor backend testing (runs pytest with docker based postgres + prisma)
Files:
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py
🧬 Code graph analysis (2)
autogpt_platform/backend/backend/copilot/sdk/security_hooks_test.py (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (1)
create_security_hooks(145-381)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (1)
autogpt_platform/backend/backend/util/logging.py (3)
info(41-43)error(49-51)warning(45-47)
🔇 Additional comments (1)
autogpt_platform/backend/backend/copilot/sdk/security_hooks.py (1)
257-320: Nice hardening on slot-release accounting.Guarding decrement with
tool_use_id in task_tool_use_idsin both success and failure hooks cleanly prevents double-release and untracked release paths.
- Reset prevStatusRef when sessionId changes to prevent stale status from the old session triggering false reconnect/invalidation logic. - Clear pending reconnect timer on session switch. - Show a toast when all reconnect attempts are exhausted so the user knows to refresh the page. Addresses CodeRabbit review feedback.
|
Addressed all review comments in cff210f:
|
- Reduce refs from 6 to 2 (timer + toast flag) - Use state for reconnect tracking (enables UI feedback) - Consolidate scheduleReconnect + refetch into single handleReconnect - Remove redundant message clearing effect (now inline) - Simplify session cleanup into focused effects - Change staleTime from 0 to Infinity (manual invalidation already in place) - Add max reconnect attempts (5) with error toast
- On resume, reuse existing assistant message instead of creating new one - Prevents first assistant message from being duplicated on each reconnect - Only creates new assistant message on initial stream or after tool results
This reverts commit 50c0b0c.
- Invalidate session cache when clearing messages on reconnect - Prevents hydration from re-adding old assistant messages - Fixes duplicate first assistant message appearing on each reconnect
- Remove message clearing that caused temporary message loss - Implement context-based deduplication for assistant messages - Keep most complete message when duplicates found after same user message - Rely on deduplication instead of clearing for reconnect handling
- Remove complex context-based deduplication - not needed - Remove cache invalidation on reconnect - not needed - Use simple ID-based deduplication only - Trust backend to have correct data
- Add simple content-based dedup for assistant messages - Catches duplicate first message with different IDs (hydration vs stream) - Minimal implementation - just compare content strings
- Log all deduplication decisions - Show message IDs and content being compared - Help diagnose why first message still duplicates
This reverts commit de693f0.
- Content deduplication wasn't fixing duplicate first message - Back to simple ID-only dedup - Trust backend to provide correct data
- Prevents cross-session state bleeding - prevStatusRef now resets when switching sessions - Fixes stale status logic causing incorrect cache invalidation - Addresses CodeRabbit review comment
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 👍🏼 Mergeable
in AutoGPT development kanban
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
4 participants
Problem
CoPilot executions were experiencing:
execute()and_execute_async()calledmark_session_completed, sending duplicate completion markershasResumedRefChanges 🏗️
1. Consolidated Exception Handling
Files:
backend/copilot/executor/processor.py,backend/copilot/sdk/service.pyprocessor.py:
execute()method_execute_async()where work happensCancelledErrorandBaseExceptionhandlers into single catchisinstance()to determine error messageservice.py:
CancelledErrorandExceptionhandlers into single catchImpact: Eliminates duplicate
mark_session_completedcalls, ~70 lines of code removed2. Fixed StreamError Message
File:
backend/copilot/sdk/service.py"An error occurred. Please try again."errorText=error_msg3. Deferred Message Clearing on Reconnect
File:
frontend/src/app/(platform)/copilot/useCopilotPage.tsshouldClearOnNextMessageRefflag4. Fixed Chat Switching Stream Resume
File:
frontend/src/app/(platform)/copilot/useCopilotPage.tsProblem: When switching Chat A → B → A, the stream didn't resume because
hasResumedRef.current.get(sessionId)was stilltrueFix: Clear
hasResumedRefentry when navigating away from sessionFlow now:
hasResumedReffor Chat AhasResumedRefis false → resumes stream ✅5. Removed Diagnostic Logging
Files:
frontend/useCopilotPage.ts,frontend/useChatSession.ts,backend/stream_registry.py,backend/processor.py,backend/routes.py[STREAM_DIAG]console logs (60+ lines)6. Exception Handling Order Consistency
File:
backend/copilot/executor/processor.pyerror_msgbefore logging in both casesArchitecture Quality: 9/10
Strengths:
Trade-offs:
Test Plan
pnpm format && pnpm lint && pnpm types- all passedpoetry run format- all passedChecklist 📋
.env.defaultis updated or compatible (no config changes)docker-compose.ymlis updated or compatible (no config changes)