[TMP] [TESTING] merge(preview): consolidated preview of 5 active PRs - #12783
[TMP] [TESTING] merge(preview): consolidated preview of 5 active PRs#12783majdyz wants to merge 366 commits into
Conversation
Resolve merge conflicts between builder-chat-panel feature and the per-model cost breakdown PR (#12726): - Flow.tsx: keep ErrorBoundary wrapper from our branch - BuilderChatPanel.tsx + useBuilderChatPanel.ts: keep our latest refactor - platform_cost_test.py: use Prisma ORM style for export test (theirs) - useBuilderChatPanel.test.ts + BuilderChatPanel.test.tsx: keep latest tests
…illing Backend: - Cache stripe.Price.retrieve with 5-min TTL via _get_stripe_price_amount to avoid 200-600ms Stripe round-trip on every GET /credits/subscription - Use SubscriptionTier enum .value for FREE/ENTERPRISE in tier_costs dict for consistency (instead of hardcoded strings) - Rename misleading test names: "defaults_to_FREE" → "preserves_current_tier" to reflect actual behaviour (unknown price IDs preserve tier, not reset) - Update subscription_routes_test to mock _get_stripe_price_amount instead of stripe.Price.retrieve directly, avoiding cached-result interference Frontend: - Handle ?subscription=success return from Stripe Checkout: refetch + toast - Add downgrade confirmation Dialog before cancelling paid subscription - Handle ENTERPRISE tier: render dedicated admin-managed plan card, not the FREE/PRO/BUSINESS tier cards (which would show no "Current" badge) - Track pendingTier (via variables) so only the clicked button shows "Updating..." - Show "Pricing available soon" for paid tiers with cost=0 (unconfigured LD flags) instead of misleading "Free" - Move tierError state into the hook, set via changeTier internally - Move TIER_ORDER constant to module scope (was magic array inside render body) - Add aria-current="true" to active tier card for screen reader accessibility - Add role="alert" to all error paragraph elements - Improve tier descriptions with concrete capacity values
…Section Covers: tier card rendering, Current badge, cost display, upgrade/downgrade flow (with Stripe redirect), confirmation dialog, error handling, ENTERPRISE user messaging, and success param handling.
- Replace __legacy__ Dialog import with molecules/Dialog in SubscriptionTierSection - Update test mock to match new Dialog API (controlled pattern) - Guard still_has_active_sub against empty new_sub_id in sync_subscription_from_stripe - Move urlparse import from inside _validate_checkout_redirect_url to module level
…al tool-call scanning Per AGENTS.md conventions, useMemo/useCallback should not be used unless asked to optimise. Remove useMemo from ActionList (nodeMap), MessageList (visibleMessages filter), and useBuilderChatPanel (transport). Also add lastScannedToolCallIndexRef to make tool-call detection O(new messages) matching the action parser's incremental approach.
…e_amount Return None on StripeError instead of 0 so the @cached decorator (which skips caching None) does not persist the error state for 5 min. Added test to verify the None→0 fallback path in get_subscription_status.
…ts test - Dialog controlled set callback: use explicit if-block to avoid returning 'false | void' (TS2322) - Test redirect test: use vi.stubGlobal to replace window.location with a plain object (Proxy on jsdom Location breaks private-field access)
Wraps the DefaultChatTransport instantiation in useMemo([sessionId]) so the same transport object is reused across renders. Without memoisation, each streaming chunk (which triggers a re-render) created a new transport instance, resetting useChat's internal Chat state mid-stream. Matches the pattern already used in useCopilotStream.ts.
Reject URLs containing '@', backslashes, or control characters before urlparse to prevent auth-trick and backslash-normalisation attacks. Add parametrized tests covering 11 adversarial inputs + valid cases.
The `_on_node_execution` path called `charge_extra_iterations` and ignored the returned `remaining_balance`, so users were never notified when their balance crossed the low-balance threshold via these post-hoc per-LLM-call charges. `charge_node_usage` already does the right thing — mirror that pattern here so all charging paths route through `_handle_low_balance` consistently. Sentry bug prediction: PRRT_kwDOJKSTjM56Mibw (severity MEDIUM).
The @cached decorator could not differentiate "no entry" from "entry is None" — both `_get_from_memory` and `_get_from_redis` returned `None` for misses, and the wrappers checked `result is not None` to decide whether to recompute. Functions that returned `None` as a valid value were therefore re-executed on every call, defeating the cache and (for shared_cache=False) potentially causing per-pod thundering herd against upstream APIs. Fix: - Use a module-level `_MISSING = object()` sentinel for "no entry". - Wrappers now check `result is not _MISSING` so cached `None` is returned correctly. - Add a `cache_none: bool = True` parameter so callers that *want* the retry-on-None behavior (e.g. external API calls returning `None` to signal a transient error) can explicitly opt out via `cache_none=False`. - `_get_stripe_price_amount` opts out: returning None on a Stripe error must not poison the 5-minute cache window. Updated its docstring to describe the actual behavior. New tests cover both default (None is cached) and `cache_none=False` (None is not stored, next call retries) for sync, async, and shared cache paths. Sentry bug prediction: PRRT_kwDOJKSTjM56RTEu (severity HIGH).
When the user navigates between graphs, the flowID-reset effect resets
`lastParsedMessageIndexRef` and the parsed-actions cache, then queues
`setMessages([])`. The parse-actions effect runs in the same effect
cycle — *before* the queued state updates are committed — so its
`messages` closure still belongs to the previous graph. With the index
reset to -1 and the cache empty, it would re-scan those stale messages
from index 0 and briefly flash the previous graph's actions in the new
panel.
A previous guard (`277c19642`) was lost when commit `1935137c1` (the
DefaultChatTransport memoization fix) accidentally dropped the
`if (currentFlowIDRef.current !== flowID) return;` line. That guard
was actually a no-op because `currentFlowIDRef` is updated by an earlier
effect in the same cycle, so the check never fired — the bug was masked
in practice but came back into view when sentry re-flagged it.
Replace the removed line with a one-shot `skipNextParseRef` flag that
the cleanup effect sets only on *actual* navigation (not initial mount,
detected via `prevFlowIDRef`). The parse-actions effect skips one pass
when the flag is set, then clears it. This correctly handles:
- Initial mount: no skip (flag stays false), first run parses normally.
- Navigation: skip one pass; next render arrives with fresh messages
from useChat's re-key and parses them correctly.
- Same-flowID re-render: cleanup doesn't fire, no skip, normal parse.
New regression test reproduces the navigation race in the parsed-actions
integration suite.
Sentry bug prediction: PRRT_kwDOJKSTjM56RVeU (severity HIGH).
…on effect Mirrors the existing `skipNextParseRef` guard on the parse-actions effect. When `flowID` changes, the reset effect clears `processedToolCallsRef` and `lastScannedToolCallIndexRef` and queues `setMessages([])`, but the cleared messages are not yet committed when the tool-call detection effect runs in the same effect cycle. Without the skip, the effect would re-scan the previous graph's messages from index 0 and re-fire `onGraphEdited` / `setQueryStates(flowExecutionID)` for tool calls belonging to the old graph — triggering a stray `refetchGraph()` on the new graph or auto-following a stale execution. Uses a separate `skipNextToolScanRef` so each effect consumes its own flag independently; a shared ref would let whichever effect ran first clear the guard before the other could skip.
…tests charge_node_usage is async and is awaited in _execute_single_tool_with_manager. Mocking it as MagicMock returned a non-awaitable tuple, which raised TypeError inside the tool execution path; the error was silently swallowed by the orchestrator's catch-all and converted into a 'Tool execution failed' string, so downstream assertions effectively ran against an error response instead of the success path. Switch both tests to AsyncMock to actually exercise the post-tool charging branch — matching the fix already applied in test_orchestrator.py:929.
Address review findings on the subscription tier billing PR: 1. get_stripe_customer_id race: two concurrent calls (double-click, retried request) could each create a Stripe Customer for the same user, leaving an orphaned billable customer. Pass an idempotency_key so Stripe collapses concurrent + retried calls server-side, and use a conditional update_many so the loser of a longer-window race re-reads the persisted ID instead of overwriting. 2. update_subscription_tier no-op short-circuit: if the user is already on the requested paid tier, return without creating a Checkout Session. Without this guard, a duplicate request creates a second subscription for the same price; the user would be charged for both until _cleanup_stale_subscriptions runs from the resulting webhook — which only fires after the second charge has cleared. 3. stripe_webhook payload defensive extraction: a malformed payload (missing/non-dict data.object, missing id) would raise KeyError / TypeError after signature verification, which Stripe interprets as a delivery failure and retries forever. Validate shape, log a warning, and ack with 200 so Stripe stops retrying. 4. _cleanup_stale_subscriptions: bump the swallowed-error log from warning to exception so Sentry surfaces it as an error, include the customer/sub IDs needed for manual reconciliation, and add a TODO referencing the missing periodic reconcile job that the docstring already promises as the backstop.
Revert the tentative update_many conditional guard (prisma where-clause null semantics are fiddly and the test suite mocks get_stripe_customer_id end-to-end, so a real prisma error wouldn't be caught locally). The idempotency_key on Customer.create is sufficient: Stripe collapses concurrent + retried calls to the same Customer object for 24h, which comfortably covers every realistic in-flight retry window. Also invalidate the get_user_by_id cache after the DB write so the freshly-persisted stripeCustomerId is visible on the next read.
…rain and wrap turn-start persist Address three review comments on the pending-message PR: 1. (Blocker) Mid-loop pending drain now flushes state.session_messages into session.messages before appending the pending user message, so assistant+tool entries from completed rounds land in chronological order. Without this, the next turn's replay could hit OpenAI tool-call ordering errors (user message interposed between assistant tool_call and its tool result). 2. (Should-Fix) Turn-start upsert_chat_session wrapped in try/except so a transient DB failure doesn't silently lose messages already popped from Redis. Matches the pattern used in mid-loop and SDK drain paths. 3. (Nice-to-Have) Added TestMidLoopPendingFlushOrdering regression test in service_unit_test.py that replays the production flush sequence and asserts chronological ordering of assistant/tool/pending entries.
… pending drain Track _flushed_assistant_text_len on _BaselineStreamState so the finally block only appends assistant text produced AFTER the last mid-loop flush. Without this, state.assistant_text (all rounds) vs state.session_messages (post-flush only) desync caused the startswith(recorded) dedup to fail, duplicating round-1 assistant text in session.messages. Adds regression test in service_unit_test.py.
- Fix off-by-one in rate limit: use >= instead of > for call count check - Move track_user_message() after push_pending_message() so analytics only fires on successful push - Add logger.warning in rate-limiter except-Exception catch instead of silent pass - Use fullmatch instead of match for UUID regex validation - Add extra="forbid" to PendingMessageContext to reject unexpected fields
- Replace assert with proper if-guard + RuntimeError for node_exec_result - Wrap on_node_execution in try/except to always resolve the Future via set_exception on error, preventing dangling unresolved Futures - Add separate try/except around charge_node_usage so non-balance billing exceptions propagate instead of being silently swallowed by the outer except Exception handler - Add _is_error flag to tool response dicts to replace fragile string matching on "Tool execution failed:" prefix
…_more check - Use set difference instead of any() to correctly detect other active subs (any(sub["id"] != new_sub_id ...) returns True if ANY sub has a different ID, which is always true when >1 sub exists regardless of whether the cancelled sub is in the list). - Add has_more check with logger.error in _cancel_customer_subscriptions so we surface when a customer has >10 subs and some were silently skipped.
…alidation The pre-parse rejection of @ was overly broad — it rejected valid URLs with @ in query strings or fragments (e.g. ?ref=user@company.com). The user:pass@host authority attack only applies to the netloc component. Move the @ check to run against parsed.netloc after urlparse.
…promise React onClick handlers don't await async functions, so passing an async function directly creates a floating promise. Wrap in void to make the intent explicit and prevent unhandled rejections.
- Clear graphSessionCache on user change to prevent session leaks across sign-outs - Reset all per-session state in retrySession including skipNextParseRef/skipNextToolScanRef - Trim whitespace in sendRawMessage before empty guard - Remove duplicate setAppliedActionKeys call from alreadyExists branch in applyConnectNodes
…quest to prevent 500 Change context field from dict[str,str] to PendingMessageContext so Pydantic validates (including extra="forbid") at request parse time, returning a proper 422 instead of an unhandled ValidationError / 500 when the caller sends unexpected keys.
context is now a PendingMessageContext object, not a dict — use .url attribute instead of ["url"] subscript.
…edge test After the double-call fix removed the direct setAppliedActionKeys call from the alreadyExists branch, the test still expected it to be called once. Updated to .not.toHaveBeenCalled() since the caller (handleApplyAction) now handles marking applied keys.
If the Redis drain fails mid-tool-loop, log a warning and treat it as no pending messages rather than crashing the entire copilot turn.
If on_node_execution returns None, return an error response with _is_error=True instead of falling through to the success path.
…into feat/copilot-model-targeting-flag
…ement The cwd argument was already ignored (del cwd on entry) — kept only for call-site compatibility. Remove it entirely and update all three call sites: sdk/service.py, sdk/service_test.py, and test/copilot/dry_run_loop_test.py.
54bf060 to
6e56c29
Compare
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
| async for event in stream_chat_completion_sdk( | ||
| session_id=session_id, | ||
| message=auto_message, | ||
| is_user_message=True, | ||
| user_id=user_id, | ||
| file_ids=None, | ||
| permissions=permissions, | ||
| mode=mode, | ||
| ): |
There was a problem hiding this comment.
Bug: The recursive call to stream_chat_completion_sdk in the auto-continue logic omits the model parameter, causing it to revert to the default model instead of the user's selection.
Severity: HIGH
Suggested Fix
Forward the model parameter in the recursive call to stream_chat_completion_sdk within the auto-continue logic, similar to how the mode parameter is already being forwarded. This will ensure the user's selected model persists across auto-continued turns.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: autogpt_platform/backend/backend/copilot/sdk/service.py#L3513-L3521
Potential issue: When a user selects a non-default model, such as "advanced" for Opus,
and queues multiple messages, the auto-continue feature recursively calls
`stream_chat_completion_sdk`. This recursive call omits the `model` parameter, causing
subsequent turns to fall back to the default model instead of respecting the user's
original choice. This leads to incorrect rate-limit quota charging, specifically
under-charging by approximately 80% for users who selected the premium model.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/__tests__/ChatInput.test.tsx (1)
22-35:⚠️ Potential issue | 🔴 CriticalFix the broken store mock before adding more assertions.
Line 25 still returns
setCopilotMode: mockSetCopilotMode, but that symbol no longer exists after the rename tomockSetCopilotChatMode. This crashes render for the entire ChatInput suite, which matches the current CI failure.Minimal fix
vi.mock("@/app/(platform)/copilot/store", () => ({ useCopilotUIStore: () => ({ copilotMode: mockCopilotMode, - setCopilotMode: mockSetCopilotMode, + setCopilotMode: mockSetCopilotChatMode, copilotChatMode: mockCopilotMode, setCopilotChatMode: mockSetCopilotChatMode, copilotLlmModel: mockCopilotLlmModel, setCopilotLlmModel: mockSetCopilotLlmModel,🤖 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/components/ChatInput/__tests__/ChatInput.test.tsx around lines 22 - 35, The store mock references a nonexistent symbol: update the mocked useCopilotUIStore return to use the renamed setter (replace the outdated setCopilotMode reference with mockSetCopilotChatMode) so the mock aligns with the new variable names (mockSetCopilotChatMode, mockCopilotMode, mockSetCopilotLlmModel, etc.); ensure the object keys match the actual hooks used in ChatInput (e.g., setCopilotChatMode / setCopilotLlmModel) so the test render no longer throws due to a missing mock function.
♻️ Duplicate comments (5)
autogpt_platform/backend/backend/api/features/v1.py (1)
849-876:⚠️ Potential issue | 🔴 CriticalStill cancel Stripe on FREE downgrades even when payments are disabled.
The
payment_enabled == Falsebranch writesFREEdirectly to the DB and returns. If a user already has an active Stripe subscription and that flag is later turned off, they will keep getting billed in Stripe while the app shows them as downgraded. This path still needs to attemptcancel_stripe_subscription(user_id)before the DB write, regardless of the feature flag.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/api/features/v1.py` around lines 849 - 876, For downgrades to SubscriptionTier.FREE ensure you always attempt to cancel any existing Stripe subscription by calling cancel_stripe_subscription(user_id) before writing the FREE tier to the DB, even when payment_enabled is False; move or duplicate the cancellation logic used in the payment_enabled branch (including the try/except stripe.StripeError logging and HTTPException behavior) to run unconditionally for the FREE branch, and keep the existing fallback that if cancel_stripe_subscription returns False then call set_subscription_tier(user_id, tier) immediately (since webhook won’t fire).autogpt_platform/backend/backend/data/credit.py (2)
1779-1807:⚠️ Potential issue | 🔴 CriticalRetrying
invoice.payment_failedcan dead-end on the unique constraint.If
_add_transaction(... transaction_key=invoice_id)succeeds andstripe.Invoice.pay(invoice_id)then fails, Stripe retries the webhook. The second delivery hitsUniqueViolationError, which isn't caught here, so the handler exits before retryingInvoice.pay. That leaves the balance already deducted while Stripe keeps retrying the unpaid invoice. Treat duplicatetransaction_keys as “already deducted” and continue toInvoice.pay.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/data/credit.py` around lines 1779 - 1807, The handler must treat a UniqueViolation on the idempotency key as "already deducted" and continue to mark the Stripe invoice paid; wrap the call to credit_model._add_transaction (the call that sets transaction_key=invoice_id) in a try/except that catches the DB unique-key duplicate error (e.g., UniqueViolationError / IntegrityError with SQLSTATE 23505) and, on that specific error, proceed as if the debit succeeded (i.e., continue to call run_in_threadpool(stripe.Invoice.pay, invoice_id)); keep all other exceptions propagated or logged as before. Ensure you reference credit_model._add_transaction, transaction_key/invoice_id, and run_in_threadpool(stripe.Invoice.pay, invoice_id) when locating the change.
1440-1444:⚠️ Potential issue | 🟠 MajorDon't create a Stripe customer from the “modify existing subscription” path.
modify_stripe_subscription_for_tier()is supposed to detect and update an existing paid subscription, butget_stripe_customer_id()eagerly creates one whenstripe_customer_idis missing. For admin-granted paid tiers, a request that later falls back to Checkout—or even fails validation upstream—now leaves behind an orphan Stripe customer. Read the storedstripe_customer_idand returnFalsewhen it's absent.🧩 Minimal fix
- customer_id = await get_stripe_customer_id(user_id) + user = await get_user_by_id(user_id) + if not user.stripe_customer_id: + return False + customer_id = user.stripe_customer_id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/data/credit.py` around lines 1440 - 1444, modify_stripe_subscription_for_tier currently calls get_stripe_customer_id(user_id) which eagerly creates a Stripe customer; change the flow to read the stored stripe customer id without creating one and return False if it's missing. Concretely, replace the call to get_stripe_customer_id in modify_stripe_subscription_for_tier with a read-only lookup of the persisted stripe_customer_id (or add a non-creating parameter to get_stripe_customer_id) and if the value is falsy immediately return False before calling stripe.Subscription.list or creating any customer.autogpt_platform/backend/backend/copilot/sdk/service.py (2)
3504-3522:⚠️ Potential issue | 🟠 MajorKeep auto-continue iterative instead of recursively re-entering the stream.
A burst of queued follow-ups still creates one nested async-generator frame per drained batch here. That keeps the old stack-growth risk alive and is the same problem previously called out on this path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/service.py` around lines 3504 - 3522, The current code re-enters the stream by calling async for event in stream_chat_completion_sdk(...) inside the existing stream handler, causing nested async-generator frames; replace this recursive pattern with an iterative loop: after the initial check on ended_with_stream_error, repeatedly call drain_pending_safe(session_id, log_prefix) and if it returns texts, build auto_message and then iterate over the async generator returned by stream_chat_completion_sdk(session_id=..., message=auto_message, ...) yielding each event; repeat the drain->stream cycle until drain_pending_safe returns empty or ended_with_stream_error becomes true, ensuring you reference ended_with_stream_error, drain_pending_safe, stream_chat_completion_sdk, _auto_pending_texts and auto_message and avoid re-invoking the outer stream handler recursively.
3513-3521:⚠️ Potential issue | 🟠 MajorPropagate the selected model tier into auto-continued turns.
The handoff drops
model, so queued follow-ups from anadvancedturn silently fall back to default model selection. That changes both behavior and billing/rate-limit accounting mid-conversation.Suggested fix
async for event in stream_chat_completion_sdk( session_id=session_id, message=auto_message, is_user_message=True, user_id=user_id, file_ids=None, permissions=permissions, mode=mode, + model=model, ): yield event🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/sdk/service.py` around lines 3513 - 3521, The auto-continued handoff call to stream_chat_completion_sdk is dropping the selected model (causing advanced turns to fall back to defaults); update the call site in service.py where stream_chat_completion_sdk(...) is invoked (the async for loop that passes session_id, message=auto_message, is_user_message, user_id, file_ids, permissions, mode) to also pass through the current model variable (e.g., model=model) and ensure the stream_chat_completion_sdk signature (and any intermediate callers) accept and forward a model argument so queued follow-ups preserve the chosen model tier.
🧹 Nitpick comments (10)
autogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/usePulseChips.ts (1)
6-21: The memo here isn’t buying anything.This is just a cheap map over a tiny array, so the extra
useMemoadds noise and bookkeeping for no real win.♻️ Inline the mapping
-import { useMemo } from "react"; - export function usePulseChips(): PulseChipData[] { const { agents } = useLibraryAgents(); const sitrepItems = useSitrepItems(agents, 5); - return useMemo(() => { - return sitrepItems.map((item) => ({ - id: item.id, - agentID: item.agentID, - name: item.agentName, - status: item.status, - shortMessage: item.message, - })); - }, [sitrepItems]); + return sitrepItems.map((item) => ({ + id: item.id, + agentID: item.agentID, + name: item.agentName, + status: item.status, + shortMessage: item.message, + })); }As per coding guidelines, "Do not use
useCallbackoruseMemounless asked to optimise a given function".🤖 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/components/PulseChips/usePulseChips.ts around lines 6 - 21, The useMemo wrapper in usePulseChips is unnecessary; remove useMemo and return the mapped array directly: call useLibraryAgents() and useSitrepItems(agents, 5) as before, then immediately return sitrepItems.map(item => ({ id: item.id, agentID: item.agentID, name: item.agentName, status: item.status, shortMessage: item.message })); keep the function signature usePulseChips(): PulseChipData[] and the same property names to preserve types and behavior.autogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/useSitrepItems.ts (1)
93-97: Drop the per-agent re-sort here.
groupByAgent()preserves the order fromuseGetV1ListAllExecutions, so this extrasort()adds work on every agent without changing which failure/completion.find()will pick.♻️ Simplify the recent-executions path
- const recent = executions - .filter((e) => endedAfter(e, cutoff)) - .sort((a, b) => toEndTime(b) - toEndTime(a)); + const recent = executions.filter((e) => endedAfter(e, cutoff));Also remove
toEndTimefrom the import list once this sort is gone.Based on learnings,
useGetV1ListAllExecutionsalready returns executions in reverse-chronological order (most recent first).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/library/components/SitrepItem/useSitrepItems.ts around lines 93 - 97, The per-agent re-sort is redundant and costly: remove the .sort((a, b) => toEndTime(b) - toEndTime(a)) from the recent variable (which currently filters via endedAfter and assigns to recent) so recent preserves the reverse-chronological order returned by useGetV1ListAllExecutions; also remove toEndTime from the import list and ensure groupByAgent() continues to consume recent as-is (keeping endedAfter and cutoff logic intact).autogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/BriefingTabContent.tsx (1)
1-345: Consider splitting this file as it grows.At ~345 lines, this file exceeds the ~200 line guideline. The sub-components (
UsageSection,ExecutionListSection,AgentListSection,UsageFooter,UsageMeter,EmptyMessage) could be extracted into a localcomponents/folder if the file continues to grow.As per coding guidelines: "Keep files under ~200 lines; extract sub-components or hooks into their own files when a file grows beyond this."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/library/components/AgentBriefingPanel/BriefingTabContent.tsx around lines 1 - 345, The file is over the ~200-line guideline; extract the large sub-components into separate files under a new local components/ folder (e.g., UsageSection.tsx, ExecutionListSection.tsx, AgentListSection.tsx, UsageFooter.tsx, UsageMeter.tsx, EmptyMessage.tsx) by moving the corresponding functions (UsageSection, ExecutionListSection, AgentListSection, UsageFooter, UsageMeter, EmptyMessage and any constants they need like MAX_VISIBLE and TAB_STATUS_LABEL) into those modules, export them, and update BriefingTabContent to import those components and any hooks/props (e.g., useGetV2GetCopilotUsage, useSitrepItems, useResetRateLimit, useCredits) to preserve behavior; ensure all types (CoPilotUsageStatus, LibraryAgent, AgentStatusFilter) are imported where needed and keep default behavior and props unchanged.autogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/StatsGrid.tsx (1)
14-65: Unusedcolorproperty in TILES configuration.Each tile in the
TILESarray defines acolorproperty (e.g.,"text-zinc-700","text-blue-600"), but this property is never used in the render logic. Either apply the color to the value text or remove the unused property.🧹 Option A: Apply the color to the value
- <Text variant="h4">{value}</Text> + <Text variant="h4" className={tile.color}>{value}</Text>🧹 Option B: Remove the unused property
const TILES: { label: string; key: keyof FleetSummary; format?: (v: number) => string; filter: AgentStatusFilter; emoji: string; - color: string; }[] = [ { label: "Spent this month", key: "monthlySpend", format: (v) => `$${v.toLocaleString()}`, filter: "all", emoji: "💵", - color: "text-zinc-700", }, // ... remove color from all other tiles🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/library/components/AgentBriefingPanel/StatsGrid.tsx around lines 14 - 65, The TILES array defines a color property that isn't used; update the StatsGrid component's render where each tile is mapped (referencing the TILES constant) to apply tile.color as a CSS class on the value element (the rendered numeric/string for each tile) so the color is used, ensuring you preserve existing classNames and formatting (e.g., combine with any existing classes via string interpolation or classnames utility); alternatively, if you prefer not to style values, remove the color field from each object in TILES and any related typings to eliminate the unused property.autogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/SubscriptionTierSection.tsx (1)
80-83: Avoid adding moredark:overrides in this component.These new styles hardcode dark-mode behavior in several places, but the frontend guidelines route theme handling through the design system instead of per-component
dark:*classes. Please switch these to neutral/theme-token styles exposed by the shared components. As per coding guidelines, "Nodark:Tailwind classes — the design system handles dark mode."Also applies to: 98-103, 123-126, 215-216, 248-251
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/profile/(user)/credits/components/SubscriptionTierSection/SubscriptionTierSection.tsx around lines 80 - 83, The component SubscriptionTierSection adds several `dark:` Tailwind overrides (e.g., the <p role="alert"> alert, plus other spans/divs around lines you modified) which violates the design system rule; remove all `dark:*` classes in this component and replace them with the shared design-system theme tokens or shared components (e.g., use the shared Alert/Badge/Token classes or the exported theme token class names instead of `dark:bg-...`, `dark:text-...`, `dark:border-...`) so theme handling is delegated to the design system; update every instance you changed (the alert paragraph and the other className usages noted in the comment) to use the appropriate shared component or token class names and props exposed by the shared UI library rather than per-component `dark:` overrides.autogpt_platform/backend/backend/api/features/library/db.py (1)
96-100: Consider using%splaceholders for debug log statements.Per coding guidelines, debug log statements should use
%splaceholders for deferred interpolation to avoid string formatting overhead when debug logging is disabled.♻️ Suggested change
- logger.debug( - f"Fetching library agents for user_id={user_id}, " - f"search_term={repr(search_term)}, " - f"sort_by={sort_by}, page={page}, page_size={page_size}" - ) + logger.debug( + "Fetching library agents for user_id=%s, search_term=%r, sort_by=%s, page=%s, page_size=%s", + user_id, + search_term, + sort_by, + page, + page_size, + )As per coding guidelines: "Use
%sfor deferred interpolation indebuglog statements for efficiency".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/api/features/library/db.py` around lines 96 - 100, Change the debug log to use deferred interpolation with %s placeholders instead of f-strings: update the logger.debug call that currently formats f"Fetching library agents..." to use a format string with %s placeholders and pass user_id, repr(search_term), sort_by, page, page_size as separate arguments to logger.debug (refer to the existing logger.debug invocation and the variables user_id, search_term, sort_by, page, page_size).autogpt_platform/backend/backend/copilot/token_tracking_test.py (1)
213-234: Add one non-default multiplier forwarding test for stronger coverage.This test now validates the default path (
1.0), but a single explicit non-default case would better protect the new multiplier plumbing end-to-end.🧪 Suggested additive test
class TestRateLimitRecording: @@ async def test_calls_record_token_usage_when_user_id_present(self): @@ mock_record.assert_awaited_once_with( user_id="user-abc", prompt_tokens=100, completion_tokens=50, cache_read_tokens=1000, cache_creation_tokens=200, model_cost_multiplier=1.0, ) + + `@pytest.mark.asyncio` + async def test_forwards_non_default_model_cost_multiplier(self): + mock_record = AsyncMock() + with patch( + "backend.copilot.token_tracking.record_token_usage", + new=mock_record, + ): + await persist_and_record_usage( + session=None, + user_id="user-abc", + prompt_tokens=100, + completion_tokens=50, + model_cost_multiplier=5.0, + ) + mock_record.assert_awaited_once_with( + user_id="user-abc", + prompt_tokens=100, + completion_tokens=50, + cache_read_tokens=0, + cache_creation_tokens=0, + model_cost_multiplier=5.0, + )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/backend/backend/copilot/token_tracking_test.py` around lines 213 - 234, Add a new async test in token_tracking_test.py that patches backend.copilot.token_tracking.record_token_usage (like the existing test) but calls persist_and_record_usage with a non-default model_cost_multiplier (e.g., 1.5) and asserts record_token_usage was awaited once with the same user_id, token counts, and model_cost_multiplier set to that non-default value; reference the existing test_calls_record_token_usage_when_user_id_present and the persist_and_record_usage/record_token_usage symbols to place and implement the new test.autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/components/ModelToggleButton.tsx (1)
5-5: Consider sourcingCopilotLlmModelfrom generated OpenAPI types to avoid contract drift.Using a store-local enum for a request contract type can diverge from backend schema over time; prefer a generated API type as the source of truth and re-export from the store if needed.
Based on learnings: “Prefer using generated OpenAPI types from '@/app/api/generated/' for payloads defined in openapi.json … avoid re-implementing types when a generated type is available.”
🤖 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/components/ChatInput/components/ModelToggleButton.tsx at line 5, The file imports a local store type CopilotLlmModel which risks drifting from the backend contract; replace that import with the generated OpenAPI type (e.g., import the corresponding enum/type from '@/app/api/__generated__' and use it in ModelToggleButton and any consumers), or update the store to re-export the generated type so callers keep the same import path; update the ModelToggleButton.tsx import to the generated type, adjust any type names if needed, and run typechecks/build to ensure the new generated type matches usage.autogpt_platform/frontend/src/app/(platform)/copilot/store.ts (1)
56-57: Rename these symbols toLLM, notLlm.
CopilotLlmModel/copilotLlmModel/setCopilotLlmModelbreak the frontend acronym-casing rule and will spread that inconsistency to every caller. Please switch this family toCopilotLLMModel/copilotLLMModel/setCopilotLLMModelbefore the new API settles.As per coding guidelines, "Fully capitalize acronyms in symbols, e.g.
graphID,useBackendAPI."Also applies to: 140-145
🤖 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/store.ts around lines 56 - 57, Rename the Llm-acronym symbols to use uppercase LLM: change the type CopilotLlmModel -> CopilotLLMModel, the state variable copilotLlmModel -> copilotLLMModel, and the setter setCopilotLlmModel -> setCopilotLLMModel (and any other occurrences from this family, including the ones referenced around the 140-145 region). Update all declarations, exports, imports, usages, and related JSDoc/comments to the new names so callers and selectors compile cleanly and maintain the frontend acronym-casing rule.autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useSessionManager.ts (1)
63-72: Please remove theseeslint-disableescapes.These effects are now core session-lifecycle code. Keeping
react-hooks/exhaustive-depssuppressed here makes future changes much harder to reason about; it would be better to restructure the guards/refs so the real dependency set can be declared explicitly.As per coding guidelines, "No linter suppressors (
//@ts-ignore``,// eslint-disable) — fix the actual issue."Also applies to: 123-130
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@autogpt_platform/frontend/src/app/`(platform)/build/components/BuilderChatPanel/useSessionManager.ts around lines 63 - 72, Remove the eslint-disable comment and make the effect dependencies explicit in useSessionManager: update the useEffect that currently references flowID, graphSessionCache, setSessionId, setSessionError, isCreatingSessionRef, and hasSentSeedMessageRef so the dependency array includes stable references (e.g., flowID and graphSessionCache) and ensure setters and refs are stable (do not include refs in deps; useRef values need not be listed). If any referenced functions/objects (like graphSessionCache or setters) are not stable, memoize them (useMemo/useCallback) or derive their values inside the effect to avoid stale closures. Apply the same change to the other effect at the block around lines 123-130 so both effects declare precise dependencies instead of disabling react-hooks/exhaustive-deps.
🤖 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/api/features/chat/routes_test.py`:
- Around line 1099-1101: The test function
test_stream_chat_accepts_exactly_max_length_message uses the wrong pytest-mock
type; replace the incorrect annotation pytest_mock.MockFixture with
pytest_mock.MockerFixture in the function signature so the test uses the correct
MockerFixture type (consistent with other tests) and avoids AttributeError at
collection time.
- Around line 202-205: The test is patching the wrong symbol: change the mock
target from backend.api.features.chat.routes.get_or_create_workspace to
backend.data.workspace.get_or_create_workspace so you mock where the function is
used via resolve_workspace_files; update the mocker.patch call that currently
references get_or_create_workspace in routes to instead patch
backend.data.workspace.get_or_create_workspace (matching the other workspace
mocks) so all workspace mocks are consistent.
In `@autogpt_platform/backend/backend/api/features/chat/routes.py`:
- Around line 909-914: The dedup lock acquired by acquire_dedup_lock(session_id,
original_message, sanitized_file_ids) can be leaked if any awaited setup step
throws before event_generator() is returned; ensure you release the dedup key
immediately on any setup failure by calling the existing dedup-release path (the
same cleanup used on normal finish/error) before re-raising or returning an
error response, i.e., wrap the setup sequence after acquire_dedup_lock in
try/except/finally (or explicitly release and then raise) so dedup_lock is
released on exceptions; apply the identical fix to the other code path that also
calls acquire_dedup_lock (the second acquire_dedup_lock usage) so both
acquisition sites free the lock on setup failure.
- Around line 906-914: The dedup lock key currently only uses original_message
and sanitized_file_ids, causing requests that differ by request.context,
request.mode, or request.model to be incorrectly deduplicated; update the calls
to acquire_dedup_lock (the invocation that sets dedup_lock and the similar call
at the later block) to include request.context, request.mode, and request.model
(or their normalized equivalents) alongside session_id, original_message, and
sanitized_file_ids so the fingerprint covers context, mode, and model; ensure
the acquire_dedup_lock implementation and any hashing/fingerprint helper consume
these new fields in the same order/format used by the two call sites.
In `@autogpt_platform/backend/backend/copilot/message_dedup_test.py`:
- Around line 33-44: Update the test_acquire_returns_lock_on_first_request to
assert that the Redis set call uses NX and TTL: when mocking Redis via
_patch_redis and calling acquire_dedup_lock("sess-1", "hello", None), verify
mock_redis.set was called with a key that starts with f"{_KEY_PREFIX}:sess-1:"
and that the call included nx=True and ex=30 (i.e., ensure the set invocation
from acquire_dedup_lock uses NX and a 30s TTL to enforce the dedup idempotency
contract).
In `@autogpt_platform/backend/backend/copilot/message_dedup.py`:
- Around line 25-39: The current _DedupLock implementation is unsafe across TTL
expiry because release() blindly deletes _key and can remove a newer lock;
modify lock creation to store a unique token (e.g., UUID) as the value at _key
and update the lock API (class _DedupLock) to keep that token, change release()
to perform a compare-and-delete (delete only if stored value equals the token)
using an atomic Redis script/command, and add a keepalive/extend method (or have
the lock refresh TTL periodically while held) that only updates the TTL for the
key if the token still matches; apply the same compare-and-delete token pattern
to the other lock usage referenced around lines 63-71.
In `@autogpt_platform/backend/backend/copilot/rate_limit.py`:
- Around line 341-343: The code computes total using a clamped multiplier but
logs the raw model_cost_multiplier; update the calculation to assign the clamped
value to a named variable (e.g., effective_multiplier = max(1.0,
model_cost_multiplier)), use effective_multiplier in the total calculation
((weighted_input + completion_tokens) * effective_multiplier) and change the log
statements to print effective_multiplier instead of model_cost_multiplier; apply
the same change to the other similar block referenced around lines 351-357 so
all telemetry shows the actual multiplier used.
In `@autogpt_platform/backend/backend/copilot/sdk/service.py`:
- Around line 718-725: The per-request override currently passes the hyphenated
identifier "anthropic/claude-opus-4-6" to _normalize_model_name, which prevents
OpenRouter-aware normalization; change the string passed in the model ==
"advanced" branch to the OpenRouter-standard dot form
"anthropic/claude-opus-4.6" so _normalize_model_name can correctly adapt routing
when config.openrouter_active is true (the rest of the block using session_id,
logger.info and returning sdk_model and _OPUS_COST_MULTIPLIER remains
unchanged).
- Around line 2575-2578: The code currently computes model_cost_multiplier via
_resolve_model_and_multiplier before the SDK may switch models via its fallback
logic; update the flow so the actual model used is captured when a fallback is
activated and used when persisting usage: when
fallback_model_activated_per_attempt is set, record the chosen fallback model
name into the request context (or a local variable) and recompute or look up the
model_cost_multiplier for that actual model immediately before calling
persist_and_record_usage; ensure persist_and_record_usage receives the actual
model name (not the pre-resolved one) and its corresponding multiplier so rate
limits and DB records reflect the model that actually ran.
In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.ts:
- Around line 1271-1288: The test currently only checks that the seed prefix is
present in the first request but doesn't assert that the original user message
("hello") is preserved; update the first-request assertion for
ctorArg.prepareSendMessagesRequest({ messages }) (req) to also assert the
request body includes the original user text (e.g., expect(req).toMatchObject({
body: { is_user_message: true }, headers: { Authorization: "Bearer tok" } }) and
additionally assert the message string contains both the seed prefix and "hello"
or equals the expected concatenated string) so the test fails if "hello" is
dropped while keeping the existing second-request assertion for req2 unchanged.
In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/BuilderChatPanel/useSessionManager.ts:
- Around line 148-168: After constructing and possibly mutating messageText (via
extractTextFromParts and buildSeedPrompt) ensure you clamp the final messageText
to StreamChatRequest.message's maxLength (64000) before returning the body;
specifically, in the block that sets body.message, replace the raw messageText
with a truncated version (e.g., messageText.slice(0, 64000)) so seeded first
turns produced by buildSeedPrompt cannot exceed the 64k limit and cause a 422.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useCopilotPage.ts:
- Around line 268-295: When isInFlight is true we currently pass the full
trimmed text into postV2QueuePendingMessage which will 422 when message > 32000
chars; before calling postV2QueuePendingMessage (and before adding to
setQueuedMessages) clamp/validate trimmed to 32000 chars
(QueuePendingMessageRequest.message maxLength = 32000) — e.g., compute a
truncatedMessage = trimmed.slice(0, 32000) and use that for
postV2QueuePendingMessage(sessionId, { message: truncatedMessage }) and for
setQueuedMessages((prev) => [...prev, truncatedMessage]); optionally surface a
toast if truncation occurred.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/components/LibraryAgentCard/LibraryAgentCard.tsx:
- Around line 158-165: The error tooltip is currently only hover-reachable
because TooltipTrigger wraps a plain div (card), so keyboard users can't access
statusInfo.lastError; update LibraryAgentCard to attach the TooltipTrigger to a
focusable control instead (e.g., wrap the StatusBadge component or replace the
div trigger with a button or anchor) or alternatively render
statusInfo.lastError inline when hasError is true; ensure TooltipTrigger,
StatusBadge and the hasError/statusInfo.lastError conditional are updated so the
trigger is keyboard-focusable and the tooltip content remains the same.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/hooks/useAgentStatus.ts:
- Around line 96-123: The hook currently computes concrete statuses using
executions ?? [], which yields misleading idle states while the query is loading
or errored; update useAgentStatusMap to read isSuccess (from
useGetV1ListAllExecutions) and only run the memoized computation when isSuccess
is true, otherwise return a neutral result (e.g., an empty Map or a Map with a
loading/unknown status) instead of deriving statuses from missing data; ensure
you stop using executions ?? [] and only pass executions into
computeAgentStatus(agent, agentExecs) when isSuccess is true.
- Around line 144-208: The hook useFleetSummary currently builds and returns an
all-zero FleetSummary while the executions query is still loading; update it to
detect the query status from useGetV1ListAllExecutions (use its isSuccess flag)
and return a distinct loading state instead of the zeroed summary. Specifically,
inside useFleetSummary, read isSuccess from useGetV1ListAllExecutions and if
isSuccess is false return an explicit loading sentinel (e.g., null or a
FleetSummary with a loading boolean) so callers can differentiate "loading" from
a genuine empty fleet; only compute the counts inside the useMemo when isSuccess
is true and executions is available. Ensure references to useFleetSummary,
useGetV1ListAllExecutions, and FleetSummary are updated accordingly.
In `@autogpt_platform/frontend/src/app/`(platform)/library/page.tsx:
- Around line 24-27: The statusFilter dropdown is showing counts derived from
fleetSummary (created from useLibraryAgents()) while the UI list is scoped by
activeTab, causing mismatched counts on non-"all" tabs like Favorites; update
the logic so the summary is computed from the same dataset the active tab uses
(e.g., derive a tabScopedAgents list from useLibraryAgents() filtered by
activeTab and pass that into useLibraryFleetSummary) or, alternatively, clear or
hide statusFilter (via setStatusFilter) when activeTab !== "all" so the counts
and filter behavior stay consistent with the displayed agents.
---
Outside diff comments:
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ChatInput/__tests__/ChatInput.test.tsx:
- Around line 22-35: The store mock references a nonexistent symbol: update the
mocked useCopilotUIStore return to use the renamed setter (replace the outdated
setCopilotMode reference with mockSetCopilotChatMode) so the mock aligns with
the new variable names (mockSetCopilotChatMode, mockCopilotMode,
mockSetCopilotLlmModel, etc.); ensure the object keys match the actual hooks
used in ChatInput (e.g., setCopilotChatMode / setCopilotLlmModel) so the test
render no longer throws due to a missing mock function.
---
Duplicate comments:
In `@autogpt_platform/backend/backend/api/features/v1.py`:
- Around line 849-876: For downgrades to SubscriptionTier.FREE ensure you always
attempt to cancel any existing Stripe subscription by calling
cancel_stripe_subscription(user_id) before writing the FREE tier to the DB, even
when payment_enabled is False; move or duplicate the cancellation logic used in
the payment_enabled branch (including the try/except stripe.StripeError logging
and HTTPException behavior) to run unconditionally for the FREE branch, and keep
the existing fallback that if cancel_stripe_subscription returns False then call
set_subscription_tier(user_id, tier) immediately (since webhook won’t fire).
In `@autogpt_platform/backend/backend/copilot/sdk/service.py`:
- Around line 3504-3522: The current code re-enters the stream by calling async
for event in stream_chat_completion_sdk(...) inside the existing stream handler,
causing nested async-generator frames; replace this recursive pattern with an
iterative loop: after the initial check on ended_with_stream_error, repeatedly
call drain_pending_safe(session_id, log_prefix) and if it returns texts, build
auto_message and then iterate over the async generator returned by
stream_chat_completion_sdk(session_id=..., message=auto_message, ...) yielding
each event; repeat the drain->stream cycle until drain_pending_safe returns
empty or ended_with_stream_error becomes true, ensuring you reference
ended_with_stream_error, drain_pending_safe, stream_chat_completion_sdk,
_auto_pending_texts and auto_message and avoid re-invoking the outer stream
handler recursively.
- Around line 3513-3521: The auto-continued handoff call to
stream_chat_completion_sdk is dropping the selected model (causing advanced
turns to fall back to defaults); update the call site in service.py where
stream_chat_completion_sdk(...) is invoked (the async for loop that passes
session_id, message=auto_message, is_user_message, user_id, file_ids,
permissions, mode) to also pass through the current model variable (e.g.,
model=model) and ensure the stream_chat_completion_sdk signature (and any
intermediate callers) accept and forward a model argument so queued follow-ups
preserve the chosen model tier.
In `@autogpt_platform/backend/backend/data/credit.py`:
- Around line 1779-1807: The handler must treat a UniqueViolation on the
idempotency key as "already deducted" and continue to mark the Stripe invoice
paid; wrap the call to credit_model._add_transaction (the call that sets
transaction_key=invoice_id) in a try/except that catches the DB unique-key
duplicate error (e.g., UniqueViolationError / IntegrityError with SQLSTATE
23505) and, on that specific error, proceed as if the debit succeeded (i.e.,
continue to call run_in_threadpool(stripe.Invoice.pay, invoice_id)); keep all
other exceptions propagated or logged as before. Ensure you reference
credit_model._add_transaction, transaction_key/invoice_id, and
run_in_threadpool(stripe.Invoice.pay, invoice_id) when locating the change.
- Around line 1440-1444: modify_stripe_subscription_for_tier currently calls
get_stripe_customer_id(user_id) which eagerly creates a Stripe customer; change
the flow to read the stored stripe customer id without creating one and return
False if it's missing. Concretely, replace the call to get_stripe_customer_id in
modify_stripe_subscription_for_tier with a read-only lookup of the persisted
stripe_customer_id (or add a non-creating parameter to get_stripe_customer_id)
and if the value is falsy immediately return False before calling
stripe.Subscription.list or creating any customer.
---
Nitpick comments:
In `@autogpt_platform/backend/backend/api/features/library/db.py`:
- Around line 96-100: Change the debug log to use deferred interpolation with %s
placeholders instead of f-strings: update the logger.debug call that currently
formats f"Fetching library agents..." to use a format string with %s
placeholders and pass user_id, repr(search_term), sort_by, page, page_size as
separate arguments to logger.debug (refer to the existing logger.debug
invocation and the variables user_id, search_term, sort_by, page, page_size).
In `@autogpt_platform/backend/backend/copilot/token_tracking_test.py`:
- Around line 213-234: Add a new async test in token_tracking_test.py that
patches backend.copilot.token_tracking.record_token_usage (like the existing
test) but calls persist_and_record_usage with a non-default
model_cost_multiplier (e.g., 1.5) and asserts record_token_usage was awaited
once with the same user_id, token counts, and model_cost_multiplier set to that
non-default value; reference the existing
test_calls_record_token_usage_when_user_id_present and the
persist_and_record_usage/record_token_usage symbols to place and implement the
new test.
In
`@autogpt_platform/frontend/src/app/`(platform)/build/components/BuilderChatPanel/useSessionManager.ts:
- Around line 63-72: Remove the eslint-disable comment and make the effect
dependencies explicit in useSessionManager: update the useEffect that currently
references flowID, graphSessionCache, setSessionId, setSessionError,
isCreatingSessionRef, and hasSentSeedMessageRef so the dependency array includes
stable references (e.g., flowID and graphSessionCache) and ensure setters and
refs are stable (do not include refs in deps; useRef values need not be listed).
If any referenced functions/objects (like graphSessionCache or setters) are not
stable, memoize them (useMemo/useCallback) or derive their values inside the
effect to avoid stale closures. Apply the same change to the other effect at the
block around lines 123-130 so both effects declare precise dependencies instead
of disabling react-hooks/exhaustive-deps.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/ChatInput/components/ModelToggleButton.tsx:
- Line 5: The file imports a local store type CopilotLlmModel which risks
drifting from the backend contract; replace that import with the generated
OpenAPI type (e.g., import the corresponding enum/type from
'@/app/api/__generated__' and use it in ModelToggleButton and any consumers), or
update the store to re-export the generated type so callers keep the same import
path; update the ModelToggleButton.tsx import to the generated type, adjust any
type names if needed, and run typechecks/build to ensure the new generated type
matches usage.
In
`@autogpt_platform/frontend/src/app/`(platform)/copilot/components/PulseChips/usePulseChips.ts:
- Around line 6-21: The useMemo wrapper in usePulseChips is unnecessary; remove
useMemo and return the mapped array directly: call useLibraryAgents() and
useSitrepItems(agents, 5) as before, then immediately return
sitrepItems.map(item => ({ id: item.id, agentID: item.agentID, name:
item.agentName, status: item.status, shortMessage: item.message })); keep the
function signature usePulseChips(): PulseChipData[] and the same property names
to preserve types and behavior.
In `@autogpt_platform/frontend/src/app/`(platform)/copilot/store.ts:
- Around line 56-57: Rename the Llm-acronym symbols to use uppercase LLM: change
the type CopilotLlmModel -> CopilotLLMModel, the state variable copilotLlmModel
-> copilotLLMModel, and the setter setCopilotLlmModel -> setCopilotLLMModel (and
any other occurrences from this family, including the ones referenced around the
140-145 region). Update all declarations, exports, imports, usages, and related
JSDoc/comments to the new names so callers and selectors compile cleanly and
maintain the frontend acronym-casing rule.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/components/AgentBriefingPanel/BriefingTabContent.tsx:
- Around line 1-345: The file is over the ~200-line guideline; extract the large
sub-components into separate files under a new local components/ folder (e.g.,
UsageSection.tsx, ExecutionListSection.tsx, AgentListSection.tsx,
UsageFooter.tsx, UsageMeter.tsx, EmptyMessage.tsx) by moving the corresponding
functions (UsageSection, ExecutionListSection, AgentListSection, UsageFooter,
UsageMeter, EmptyMessage and any constants they need like MAX_VISIBLE and
TAB_STATUS_LABEL) into those modules, export them, and update BriefingTabContent
to import those components and any hooks/props (e.g., useGetV2GetCopilotUsage,
useSitrepItems, useResetRateLimit, useCredits) to preserve behavior; ensure all
types (CoPilotUsageStatus, LibraryAgent, AgentStatusFilter) are imported where
needed and keep default behavior and props unchanged.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/components/AgentBriefingPanel/StatsGrid.tsx:
- Around line 14-65: The TILES array defines a color property that isn't used;
update the StatsGrid component's render where each tile is mapped (referencing
the TILES constant) to apply tile.color as a CSS class on the value element (the
rendered numeric/string for each tile) so the color is used, ensuring you
preserve existing classNames and formatting (e.g., combine with any existing
classes via string interpolation or classnames utility); alternatively, if you
prefer not to style values, remove the color field from each object in TILES and
any related typings to eliminate the unused property.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/components/SitrepItem/useSitrepItems.ts:
- Around line 93-97: The per-agent re-sort is redundant and costly: remove the
.sort((a, b) => toEndTime(b) - toEndTime(a)) from the recent variable (which
currently filters via endedAfter and assigns to recent) so recent preserves the
reverse-chronological order returned by useGetV1ListAllExecutions; also remove
toEndTime from the import list and ensure groupByAgent() continues to consume
recent as-is (keeping endedAfter and cutoff logic intact).
In
`@autogpt_platform/frontend/src/app/`(platform)/profile/(user)/credits/components/SubscriptionTierSection/SubscriptionTierSection.tsx:
- Around line 80-83: The component SubscriptionTierSection adds several `dark:`
Tailwind overrides (e.g., the <p role="alert"> alert, plus other spans/divs
around lines you modified) which violates the design system rule; remove all
`dark:*` classes in this component and replace them with the shared
design-system theme tokens or shared components (e.g., use the shared
Alert/Badge/Token classes or the exported theme token class names instead of
`dark:bg-...`, `dark:text-...`, `dark:border-...`) so theme handling is
delegated to the design system; update every instance you changed (the alert
paragraph and the other className usages noted in the comment) to use the
appropriate shared component or token class names and props exposed by the
shared UI library rather than per-component `dark:` overrides.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5091c22c-0748-4ec8-afab-9e229fa24b76
⛔ Files ignored due to path filters (1)
autogpt_platform/frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (114)
autogpt_platform/backend/backend/api/features/chat/routes.pyautogpt_platform/backend/backend/api/features/chat/routes_test.pyautogpt_platform/backend/backend/api/features/library/db.pyautogpt_platform/backend/backend/api/features/library/db_test.pyautogpt_platform/backend/backend/api/features/library/model.pyautogpt_platform/backend/backend/api/features/library/model_test.pyautogpt_platform/backend/backend/api/features/subscription_routes_test.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/copilot/baseline/service.pyautogpt_platform/backend/backend/copilot/baseline/service_unit_test.pyautogpt_platform/backend/backend/copilot/config.pyautogpt_platform/backend/backend/copilot/executor/processor.pyautogpt_platform/backend/backend/copilot/executor/utils.pyautogpt_platform/backend/backend/copilot/message_dedup.pyautogpt_platform/backend/backend/copilot/message_dedup_test.pyautogpt_platform/backend/backend/copilot/pending_message_helpers.pyautogpt_platform/backend/backend/copilot/pending_message_helpers_test.pyautogpt_platform/backend/backend/copilot/pending_messages.pyautogpt_platform/backend/backend/copilot/pending_messages_test.pyautogpt_platform/backend/backend/copilot/prompt_cache_test.pyautogpt_platform/backend/backend/copilot/prompting.pyautogpt_platform/backend/backend/copilot/prompting_test.pyautogpt_platform/backend/backend/copilot/rate_limit.pyautogpt_platform/backend/backend/copilot/sdk/p0_guardrails_test.pyautogpt_platform/backend/backend/copilot/sdk/query_builder_test.pyautogpt_platform/backend/backend/copilot/sdk/retry_scenarios_test.pyautogpt_platform/backend/backend/copilot/sdk/service.pyautogpt_platform/backend/backend/copilot/sdk/service_helpers_test.pyautogpt_platform/backend/backend/copilot/sdk/service_test.pyautogpt_platform/backend/backend/copilot/service.pyautogpt_platform/backend/backend/copilot/stream_registry.pyautogpt_platform/backend/backend/copilot/stream_registry_test.pyautogpt_platform/backend/backend/copilot/token_tracking.pyautogpt_platform/backend/backend/copilot/token_tracking_test.pyautogpt_platform/backend/backend/data/credit.pyautogpt_platform/backend/backend/data/credit_subscription_test.pyautogpt_platform/backend/backend/data/platform_cost_test.pyautogpt_platform/backend/backend/data/workspace.pyautogpt_platform/backend/backend/util/cache.pyautogpt_platform/backend/backend/util/cache_test.pyautogpt_platform/backend/backend/util/feature_flag.pyautogpt_platform/backend/test/copilot/dry_run_loop_test.pyautogpt_platform/frontend/package.jsonautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsxautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/BuilderChatPanel.test.tsxautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/__tests__/useBuilderChatPanel.test.tsautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useActionParser.tsautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useBuilderChatPanel.tsautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useSessionManager.tsautogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/useToolCallHandler.tsautogpt_platform/frontend/src/app/(platform)/build/components/FlowEditor/Flow/Flow.tsxautogpt_platform/frontend/src/app/(platform)/copilot/CopilotPage.tsxautogpt_platform/frontend/src/app/(platform)/copilot/__tests__/helpers.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/__tests__/store.test.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/ChatInput.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/__tests__/ChatInput.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/components/DryRunToggleButton.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/components/ModeToggleButton.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/components/ModelToggleButton.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/components/__tests__/ModelToggleButton.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/useVoiceRecording.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/__tests__/ChatMessagesContainer.test.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/EmptySession.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/EditNameDialog/EditNameDialog.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/PulseChips.module.cssautogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/PulseChips.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/types.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/usePulseChips.tsautogpt_platform/frontend/src/app/(platform)/copilot/components/RateLimitResetDialog/RateLimitResetDialog.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/UsageLimits/UsagePanelContent.tsxautogpt_platform/frontend/src/app/(platform)/copilot/components/usageHelpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/helpers.tsautogpt_platform/frontend/src/app/(platform)/copilot/store.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotPage.tsautogpt_platform/frontend/src/app/(platform)/copilot/useCopilotStream.tsautogpt_platform/frontend/src/app/(platform)/layout.tsxautogpt_platform/frontend/src/app/(platform)/library/__tests__/main.test.tsxautogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/AgentBriefingPanel.module.cssautogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/AgentBriefingPanel.tsxautogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/BriefingTabContent.tsxautogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/StatsGrid.tsxautogpt_platform/frontend/src/app/(platform)/library/components/AgentFilterMenu/AgentFilterMenu.tsxautogpt_platform/frontend/src/app/(platform)/library/components/ContextualActionButton/ContextualActionButton.tsxautogpt_platform/frontend/src/app/(platform)/library/components/JumpBackIn/JumpBackIn.tsxautogpt_platform/frontend/src/app/(platform)/library/components/JumpBackIn/useJumpBackIn.tsautogpt_platform/frontend/src/app/(platform)/library/components/LibraryActionHeader/LibraryActionHeader.tsxautogpt_platform/frontend/src/app/(platform)/library/components/LibraryAgentCard/LibraryAgentCard.tsxautogpt_platform/frontend/src/app/(platform)/library/components/LibraryAgentList/LibraryAgentList.tsxautogpt_platform/frontend/src/app/(platform)/library/components/LibraryAgentList/useLibraryAgentList.tsautogpt_platform/frontend/src/app/(platform)/library/components/LibraryFolder/LibraryFolder.tsxautogpt_platform/frontend/src/app/(platform)/library/components/LibrarySubSection/LibrarySubSection.tsxautogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/SitrepItem.module.cssautogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/SitrepItem.tsxautogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/SitrepList.tsxautogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/useSitrepItems.tsautogpt_platform/frontend/src/app/(platform)/library/components/StatusBadge/StatusBadge.tsxautogpt_platform/frontend/src/app/(platform)/library/hooks/executionHelpers.tsautogpt_platform/frontend/src/app/(platform)/library/hooks/useAgentStatus.tsautogpt_platform/frontend/src/app/(platform)/library/hooks/useLibraryFleetSummary.tsautogpt_platform/frontend/src/app/(platform)/library/page.tsxautogpt_platform/frontend/src/app/(platform)/library/types.tsautogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/SubscriptionTierSection.tsxautogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/__tests__/SubscriptionTierSection.test.tsxautogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/useSubscriptionTierSection.tsautogpt_platform/frontend/src/app/api/openapi.jsonautogpt_platform/frontend/src/app/layout.tsxautogpt_platform/frontend/src/components/AgentationDevtool.tsxautogpt_platform/frontend/src/contexts/AutoPilotBridgeContext.tsxautogpt_platform/frontend/src/lib/autogpt-server-api/client.tsautogpt_platform/frontend/src/services/feature-flags/use-get-flag.tsautogpt_platform/frontend/src/services/storage/local-storage.tscodecov.yml
💤 Files with no reviewable changes (3)
- autogpt_platform/frontend/src/app/(platform)/library/components/JumpBackIn/JumpBackIn.tsx
- autogpt_platform/frontend/src/app/(platform)/library/components/JumpBackIn/useJumpBackIn.ts
- autogpt_platform/frontend/src/lib/autogpt-server-api/client.ts
✅ Files skipped from review due to trivial changes (17)
- autogpt_platform/backend/test/copilot/dry_run_loop_test.py
- autogpt_platform/frontend/src/services/storage/local-storage.ts
- autogpt_platform/frontend/src/app/(platform)/library/components/LibraryActionHeader/LibraryActionHeader.tsx
- autogpt_platform/backend/backend/data/platform_cost_test.py
- codecov.yml
- autogpt_platform/frontend/package.json
- autogpt_platform/frontend/src/services/feature-flags/use-get-flag.ts
- autogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/types.ts
- autogpt_platform/frontend/src/components/AgentationDevtool.tsx
- autogpt_platform/frontend/src/app/(platform)/library/components/AgentBriefingPanel/AgentBriefingPanel.module.css
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/tests/ChatMessagesContainer.test.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/usageHelpers.ts
- autogpt_platform/frontend/src/app/(platform)/copilot/components/PulseChips/PulseChips.module.css
- autogpt_platform/backend/backend/copilot/pending_messages_test.py
- autogpt_platform/frontend/src/app/(platform)/library/components/SitrepItem/SitrepItem.module.css
- autogpt_platform/backend/backend/copilot/pending_message_helpers_test.py
- autogpt_platform/frontend/src/app/(platform)/library/types.ts
🚧 Files skipped from review as they are similar to previous changes (18)
- autogpt_platform/backend/backend/util/feature_flag.py
- autogpt_platform/backend/backend/copilot/sdk/retry_scenarios_test.py
- autogpt_platform/backend/backend/copilot/service.py
- autogpt_platform/frontend/src/app/(platform)/build/components/FlowEditor/Flow/Flow.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/CopilotPage.tsx
- autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/tests/BuilderChatPanel.test.tsx
- autogpt_platform/backend/backend/util/cache_test.py
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatMessagesContainer/ChatMessagesContainer.tsx
- autogpt_platform/backend/backend/util/cache.py
- autogpt_platform/backend/backend/copilot/sdk/query_builder_test.py
- autogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/tests/SubscriptionTierSection.test.tsx
- autogpt_platform/frontend/src/app/(platform)/build/components/BuilderChatPanel/BuilderChatPanel.tsx
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatInput/ChatInput.tsx
- autogpt_platform/backend/backend/copilot/pending_message_helpers.py
- autogpt_platform/frontend/src/app/(platform)/copilot/components/ChatContainer/ChatContainer.tsx
- autogpt_platform/frontend/src/app/(platform)/profile/(user)/credits/components/SubscriptionTierSection/useSubscriptionTierSection.ts
- autogpt_platform/backend/backend/copilot/pending_messages.py
- autogpt_platform/frontend/src/app/api/openapi.json
| mocker.patch( | ||
| "backend.api.features.chat.routes.get_or_create_workspace", | ||
| "backend.data.workspace.get_or_create_workspace", | ||
| return_value=type("W", (), {"id": "ws-1"})(), | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how get_or_create_workspace is imported and used in routes.py
rg -n "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.pyRepository: Significant-Gravitas/AutoGPT
Length of output: 54
🏁 Script executed:
#!/bin/bash
# Check if get_or_create_workspace is lazily imported or used anywhere in routes.py
# Search more broadly including imports
head -50 autogpt_platform/backend/backend/api/features/chat/routes.py | cat -n
# Also search for any conditional/lazy imports containing this name
rg "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -A 3 -B 3 2>/dev/null || echo "Not found in routes.py"
# Count lines in routes.py to see if line 1897 is even in this file
wc -l autogpt_platform/backend/backend/api/features/chat/routes.py
# Search in test file for line 1897 context
rg -n "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes_test.py | head -20Repository: Significant-Gravitas/AutoGPT
Length of output: 2401
🏁 Script executed:
#!/bin/bash
# Search for "workspace" in routes.py to find how it's used (module reference or lazy import)
rg "workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -n
# Look at the test file around line 1897 to see what test case uses that old patch target
sed -n '1890,1910p' autogpt_platform/backend/backend/api/features/chat/routes_test.py | cat -n
# Search for lazy imports of get_or_create_workspace
rg "from backend.data.workspace import" autogpt_platform/backend/backend/api/features/chat/routes.pyRepository: Significant-Gravitas/AutoGPT
Length of output: 1348
🏁 Script executed:
#!/bin/bash
# Search more comprehensively for get_or_create_workspace usage in routes.py
rg "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -C 5
# If not found, try searching across all backend files to see where it's actually used
if ! rg "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py &>/dev/null; then
echo "=== get_or_create_workspace NOT found in routes.py ==="
echo ""
echo "=== Checking where get_or_create_workspace IS defined and used ==="
rg "def get_or_create_workspace" autogpt_platform/backend/ --type py
fi
# Get full context around line 1897 in test file
echo ""
echo "=== Context around line 1897 in routes_test.py ==="
sed -n '1880,1920p' autogpt_platform/backend/backend/api/features/chat/routes_test.py | cat -nRepository: Significant-Gravitas/AutoGPT
Length of output: 2019
🏁 Script executed:
#!/bin/bash
# Search for any conditional/lazy imports of get_or_create_workspace in routes.py
# including inside functions or if blocks
rg "if.*workspace|import.*workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -A 3 -B 1
# Check if line 1897 test is in the same test function or a different one
echo "=== Test function containing line 1897 ==="
sed -n '1870,1900p' autogpt_platform/backend/backend/api/features/chat/routes_test.py | cat -n
# Search entire routes_test.py to see all test functions that use get_or_create_workspace patch
echo ""
echo "=== All patch targets for get_or_create_workspace in routes_test.py ==="
rg "\.patch\(" autogpt_platform/backend/backend/api/features/chat/routes_test.py | grep -i "get_or_create_workspace" | catRepository: Significant-Gravitas/AutoGPT
Length of output: 1897
🏁 Script executed:
#!/bin/bash
# Search for lazy imports - look for import statements inside functions or conditional blocks
rg "import.*get_or_create_workspace|from.*import.*get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -B 5 -A 5
# Also check if workspace module is imported and used as workspace.get_or_create_workspace
rg "workspace\.get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py
# Look more broadly - check if the function name appears in any string or dynamic call
rg "get_or_create_workspace" autogpt_platform/backend/backend/api/features/chat/routes.py -i
# Let's check the exact test again - line 1897 and see the whole test function
echo "=== Full test function at line 1897 ==="
sed -n '1883,1950p' autogpt_platform/backend/backend/api/features/chat/routes_test.py | cat -nRepository: Significant-Gravitas/AutoGPT
Length of output: 3071
🏁 Script executed:
#!/bin/bash
# Since get_or_create_workspace is not found in routes.py at all,
# let's check if the test is actually exercising code that needs this mock
# by looking at what the test does
# Check if the test route being called (/sessions/sess-hash/stream) actually uses get_or_create_workspace
echo "=== Searching for the stream endpoint handler in routes.py ==="
rg "def.*stream|@router.*stream" autogpt_platform/backend/backend/api/features/chat/routes.py -A 2 | head -30
# Let's search for file_ids handling since the test sends file_ids
rg "file_ids|resolve_workspace_files" autogpt_platform/backend/backend/api/features/chat/routes.py -B 3 -A 3 | head -40
# Check if maybe get_or_create_workspace was recently removed
git log --oneline --all -S "get_or_create_workspace" -- autogpt_platform/backend/backend/api/features/chat/routes.py 2>/dev/null | head -10 || echo "Git log search not available or no history"Repository: Significant-Gravitas/AutoGPT
Length of output: 2304
🏁 Script executed:
#!/bin/bash
# Check if resolve_workspace_files or build_files_block call get_or_create_workspace
echo "=== Checking resolve_workspace_files and build_files_block implementations ==="
rg "def resolve_workspace_files|def build_files_block" autogpt_platform/backend/backend/data/workspace.py -A 20 | head -50
# Also check if get_or_create_workspace is called by those functions
rg "get_or_create_workspace" autogpt_platform/backend/backend/data/workspace.py -B 5 -A 5Repository: Significant-Gravitas/AutoGPT
Length of output: 2376
Update line 1897 patch target to match other workspace mocks.
Line 1897 patches backend.api.features.chat.routes.get_or_create_workspace, but get_or_create_workspace is not imported in routes.py. It is only called indirectly via resolve_workspace_files() (line 85 import), which internally invokes it. Per the coding guideline to "mock where the symbol is used, not where it's defined", the patch target should be backend.data.workspace.get_or_create_workspace to match lines 203, 232, 270, and 899.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/api/features/chat/routes_test.py` around
lines 202 - 205, The test is patching the wrong symbol: change the mock target
from backend.api.features.chat.routes.get_or_create_workspace to
backend.data.workspace.get_or_create_workspace so you mock where the function is
used via resolve_workspace_files; update the mocker.patch call that currently
references get_or_create_workspace in routes to instead patch
backend.data.workspace.get_or_create_workspace (matching the other workspace
mocks) so all workspace mocks are consistent.
| def test_stream_chat_accepts_exactly_max_length_message( | ||
| mocker: pytest_mock.MockFixture, | ||
| ): |
There was a problem hiding this comment.
Typo: MockFixture should be MockerFixture.
pytest_mock.MockFixture does not exist. This will raise AttributeError at test collection time, preventing this test from running. The correct class is MockerFixture, consistent with all other tests in this file.
🐛 Proposed fix
def test_stream_chat_accepts_exactly_max_length_message(
- mocker: pytest_mock.MockFixture,
+ mocker: pytest_mock.MockerFixture,
):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_stream_chat_accepts_exactly_max_length_message( | |
| mocker: pytest_mock.MockFixture, | |
| ): | |
| def test_stream_chat_accepts_exactly_max_length_message( | |
| mocker: pytest_mock.MockerFixture, | |
| ): |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/api/features/chat/routes_test.py` around
lines 1099 - 1101, The test function
test_stream_chat_accepts_exactly_max_length_message uses the wrong pytest-mock
type; replace the incorrect annotation pytest_mock.MockFixture with
pytest_mock.MockerFixture in the function signature so the test uses the correct
MockerFixture type (consistent with other tests) and avoids AttributeError at
collection time.
| # ── Idempotency guard ──────────────────────────────────────────────────── | ||
| # Blocks duplicate executor tasks from concurrent/retried POSTs. | ||
| # See backend/copilot/message_dedup.py for the full lifecycle description. | ||
| dedup_lock = None | ||
| if request.is_user_message: | ||
| dedup_lock = await acquire_dedup_lock( | ||
| session_id, original_message, sanitized_file_ids | ||
| ) | ||
| if dedup_lock is None and (original_message or sanitized_file_ids): |
There was a problem hiding this comment.
Include context, mode, and model in the dedup fingerprint.
The lock key only hashes message text plus file IDs, but the queued executor payload now also depends on request.context, request.mode, and request.model. Two POSTs with the same text/files but different page context or model tier will be treated as duplicates even though they should execute differently.
Also applies to: 975-985
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/api/features/chat/routes.py` around lines
906 - 914, The dedup lock key currently only uses original_message and
sanitized_file_ids, causing requests that differ by request.context,
request.mode, or request.model to be incorrectly deduplicated; update the calls
to acquire_dedup_lock (the invocation that sets dedup_lock and the similar call
at the later block) to include request.context, request.mode, and request.model
(or their normalized equivalents) alongside session_id, original_message, and
sanitized_file_ids so the fingerprint covers context, mode, and model; ensure
the acquire_dedup_lock implementation and any hashing/fingerprint helper consume
these new fields in the same order/format used by the two call sites.
| dedup_lock = None | ||
| if request.is_user_message: | ||
| dedup_lock = await acquire_dedup_lock( | ||
| session_id, original_message, sanitized_file_ids | ||
| ) | ||
| if dedup_lock is None and (original_message or sanitized_file_ids): |
There was a problem hiding this comment.
Release the dedup key if setup fails before the SSE generator starts.
From acquire_dedup_lock(...) to return StreamingResponse(...) there are multiple awaited setup steps. If one of them raises, event_generator().finally never runs, so retries for the same message are suppressed until the 30s TTL expires.
Based on learnings, the dedup key is intentionally retained on GeneratorExit but released on normal finish/error; setup failures before event_generator() currently hit neither path.
Also applies to: 1097-1100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/api/features/chat/routes.py` around lines
909 - 914, The dedup lock acquired by acquire_dedup_lock(session_id,
original_message, sanitized_file_ids) can be leaked if any awaited setup step
throws before event_generator() is returned; ensure you release the dedup key
immediately on any setup failure by calling the existing dedup-release path (the
same cleanup used on normal finish/error) before re-raising or returning an
error response, i.e., wrap the setup sequence after acquire_dedup_lock in
try/except/finally (or explicitly release and then raise) so dedup_lock is
released on exceptions; apply the identical fix to the other code path that also
calls acquire_dedup_lock (the second acquire_dedup_lock usage) so both
acquisition sites free the lock on setup failure.
| @pytest.mark.asyncio | ||
| async def test_acquire_returns_lock_on_first_request( | ||
| mocker: pytest_mock.MockerFixture, | ||
| ) -> None: | ||
| """First request acquires the lock and returns a _DedupLock.""" | ||
| mock_redis = _patch_redis(mocker, set_returns=True) | ||
| lock = await acquire_dedup_lock("sess-1", "hello", None) | ||
| assert lock is not None | ||
| mock_redis.set.assert_called_once() | ||
| key_arg = mock_redis.set.call_args.args[0] | ||
| assert key_arg.startswith(f"{_KEY_PREFIX}:sess-1:") | ||
|
|
There was a problem hiding this comment.
Assert Redis NX + TTL on lock acquisition to protect idempotency contract.
This test should also verify set(..., nx=True, ex=30) so regressions in dedup lock semantics are caught.
Suggested test assertion update
mock_redis.set.assert_called_once()
key_arg = mock_redis.set.call_args.args[0]
assert key_arg.startswith(f"{_KEY_PREFIX}:sess-1:")
+ assert mock_redis.set.call_args.kwargs.get("nx") is True
+ assert mock_redis.set.call_args.kwargs.get("ex") == 30Based on learnings: “Redis idempotency dedup key ... uses NX with 30s TTL ... 30s TTL is the fallback for unhandled paths.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/backend/backend/copilot/message_dedup_test.py` around lines
33 - 44, Update the test_acquire_returns_lock_on_first_request to assert that
the Redis set call uses NX and TTL: when mocking Redis via _patch_redis and
calling acquire_dedup_lock("sess-1", "hello", None), verify mock_redis.set was
called with a key that starts with f"{_KEY_PREFIX}:sess-1:" and that the call
included nx=True and ex=30 (i.e., ensure the set invocation from
acquire_dedup_lock uses NX and a 30s TTL to enforce the dedup idempotency
contract).
| if (isInFlight) { | ||
| // File attachments cannot be included in a queued pending message — | ||
| // the queue API does not support file_ids. Inform the user and bail. | ||
| if (files && files.length > 0) { | ||
| toast({ | ||
| title: "Please wait to attach files", | ||
| description: | ||
| "File attachments can't be queued until the current response finishes.", | ||
| variant: "destructive", | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Queue the message into the pending buffer so it is picked up between | ||
| // tool-call rounds by the currently running executor turn. | ||
| try { | ||
| await postV2QueuePendingMessage(sessionId, { message: trimmed }); | ||
| setQueuedMessages((prev) => [...prev, trimmed]); | ||
| } catch (err) { | ||
| toast({ | ||
| title: "Could not queue message", | ||
| description: "Please wait for the current response to finish.", | ||
| variant: "destructive", | ||
| }); | ||
| throw err; | ||
| } | ||
| return; | ||
| } |
There was a problem hiding this comment.
Clamp queued messages to the /pending limit before enqueueing.
When the turn is already in flight, Lines 284-285 send the full trimmed text to postV2QueuePendingMessage. That endpoint caps message at 32,000 chars, so oversized follow-ups fail there even though the normal /stream path accepts up to 64,000. Clamp or validate before enqueueing so this new queue flow doesn't degrade into a 422 + generic error toast for long messages.
Possible fix
if (isInFlight) {
+ const MAX_QUEUED_MESSAGE_LENGTH = 32_000;
+
// File attachments cannot be included in a queued pending message —
// the queue API does not support file_ids. Inform the user and bail.
if (files && files.length > 0) {
@@
// Queue the message into the pending buffer so it is picked up between
// tool-call rounds by the currently running executor turn.
try {
- await postV2QueuePendingMessage(sessionId, { message: trimmed });
- setQueuedMessages((prev) => [...prev, trimmed]);
+ const queuedMessage = trimmed.slice(0, MAX_QUEUED_MESSAGE_LENGTH);
+ await postV2QueuePendingMessage(sessionId, { message: queuedMessage });
+ setQueuedMessages((prev) => [...prev, queuedMessage]);
} catch (err) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (isInFlight) { | |
| // File attachments cannot be included in a queued pending message — | |
| // the queue API does not support file_ids. Inform the user and bail. | |
| if (files && files.length > 0) { | |
| toast({ | |
| title: "Please wait to attach files", | |
| description: | |
| "File attachments can't be queued until the current response finishes.", | |
| variant: "destructive", | |
| }); | |
| return; | |
| } | |
| // Queue the message into the pending buffer so it is picked up between | |
| // tool-call rounds by the currently running executor turn. | |
| try { | |
| await postV2QueuePendingMessage(sessionId, { message: trimmed }); | |
| setQueuedMessages((prev) => [...prev, trimmed]); | |
| } catch (err) { | |
| toast({ | |
| title: "Could not queue message", | |
| description: "Please wait for the current response to finish.", | |
| variant: "destructive", | |
| }); | |
| throw err; | |
| } | |
| return; | |
| } | |
| if (isInFlight) { | |
| const MAX_QUEUED_MESSAGE_LENGTH = 32_000; | |
| // File attachments cannot be included in a queued pending message — | |
| // the queue API does not support file_ids. Inform the user and bail. | |
| if (files && files.length > 0) { | |
| toast({ | |
| title: "Please wait to attach files", | |
| description: | |
| "File attachments can't be queued until the current response finishes.", | |
| variant: "destructive", | |
| }); | |
| return; | |
| } | |
| // Queue the message into the pending buffer so it is picked up between | |
| // tool-call rounds by the currently running executor turn. | |
| try { | |
| const queuedMessage = trimmed.slice(0, MAX_QUEUED_MESSAGE_LENGTH); | |
| await postV2QueuePendingMessage(sessionId, { message: queuedMessage }); | |
| setQueuedMessages((prev) => [...prev, queuedMessage]); | |
| } catch (err) { | |
| toast({ | |
| title: "Could not queue message", | |
| description: "Please wait for the current response to finish.", | |
| variant: "destructive", | |
| }); | |
| throw err; | |
| } | |
| return; | |
| } |
🤖 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 268 - 295, When isInFlight is true we currently pass the full
trimmed text into postV2QueuePendingMessage which will 422 when message > 32000
chars; before calling postV2QueuePendingMessage (and before adding to
setQueuedMessages) clamp/validate trimmed to 32000 chars
(QueuePendingMessageRequest.message maxLength = 32000) — e.g., compute a
truncatedMessage = trimmed.slice(0, 32000) and use that for
postV2QueuePendingMessage(sessionId, { message: truncatedMessage }) and for
setQueuedMessages((prev) => [...prev, truncatedMessage]); optionally surface a
toast if truncation occurred.
| if (hasError && statusInfo.lastError) { | ||
| return ( | ||
| <Tooltip> | ||
| <TooltipTrigger asChild>{card}</TooltipTrigger> | ||
| <TooltipContent className="max-w-xs text-red-600"> | ||
| {statusInfo.lastError} | ||
| </TooltipContent> | ||
| </Tooltip> |
There was a problem hiding this comment.
Make the error tooltip keyboard reachable.
TooltipTrigger is wrapping a plain div, so statusInfo.lastError is effectively hover-only here. Keyboard users can't focus the trigger and won't be able to access the error details. Please anchor the tooltip to a focusable control in the card (for example the error StatusBadge) or expose the message inline for the error state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@autogpt_platform/frontend/src/app/`(platform)/library/components/LibraryAgentCard/LibraryAgentCard.tsx
around lines 158 - 165, The error tooltip is currently only hover-reachable
because TooltipTrigger wraps a plain div (card), so keyboard users can't access
statusInfo.lastError; update LibraryAgentCard to attach the TooltipTrigger to a
focusable control instead (e.g., wrap the StatusBadge component or replace the
div trigger with a button or anchor) or alternatively render
statusInfo.lastError inline when hasError is true; ensure TooltipTrigger,
StatusBadge and the hasError/statusInfo.lastError conditional are updated so the
trigger is keyboard-focusable and the tooltip content remains the same.
| export function useAgentStatusMap( | ||
| agents: LibraryAgent[], | ||
| ): Map<string, AgentStatusInfo> { | ||
| const { data: executions } = useGetV1ListAllExecutions({ | ||
| query: { select: okData }, | ||
| }); | ||
|
|
||
| return useMemo(() => { | ||
| const map = new Map<string, AgentStatusInfo>(); | ||
| const execsByGraph = new Map<string, GraphExecutionMeta[]>(); | ||
|
|
||
| for (const exec of executions ?? []) { | ||
| const list = execsByGraph.get(exec.graph_id); | ||
| if (list) { | ||
| list.push(exec); | ||
| } else { | ||
| execsByGraph.set(exec.graph_id, [exec]); | ||
| } | ||
| } | ||
|
|
||
| for (const agent of agents) { | ||
| const agentExecs = execsByGraph.get(agent.graph_id) ?? []; | ||
| map.set(agent.graph_id, computeAgentStatus(agent, agentExecs)); | ||
| } | ||
|
|
||
| return map; | ||
| }, [agents, executions]); | ||
| } |
There was a problem hiding this comment.
Don’t derive concrete agent statuses before the executions query succeeds.
With executions ?? [], a pending or failed query makes every agent look idle/listening/scheduled, so cards can briefly show the wrong state and real failures disappear on fetch errors. Please gate this map on isSuccess or surface an explicit loading state instead of computing statuses from missing data.
Based on learnings, prefer checking isSuccess when gating on React Query results because data may be undefined in error/idle states.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/frontend/src/app/`(platform)/library/hooks/useAgentStatus.ts
around lines 96 - 123, The hook currently computes concrete statuses using
executions ?? [], which yields misleading idle states while the query is loading
or errored; update useAgentStatusMap to read isSuccess (from
useGetV1ListAllExecutions) and only run the memoized computation when isSuccess
is true, otherwise return a neutral result (e.g., an empty Map or a Map with a
loading/unknown status) instead of deriving statuses from missing data; ensure
you stop using executions ?? [] and only pass executions into
computeAgentStatus(agent, agentExecs) when isSuccess is true.
| export function useFleetSummary(agents: LibraryAgent[]): FleetSummary { | ||
| const { data: executions } = useGetV1ListAllExecutions({ | ||
| query: { select: okData }, | ||
| }); | ||
|
|
||
| return useMemo(() => { | ||
| const counts: FleetSummary = { | ||
| running: 0, | ||
| error: 0, | ||
| completed: 0, | ||
| listening: 0, | ||
| scheduled: 0, | ||
| idle: 0, | ||
| monthlySpend: 0, | ||
| }; | ||
|
|
||
| const activeGraphIds = new Set<string>(); | ||
| const errorGraphIds = new Set<string>(); | ||
| const completedGraphIds = new Set<string>(); | ||
|
|
||
| if (executions) { | ||
| const cutoff = Date.now() - SEVENTY_TWO_HOURS_MS; | ||
| for (const exec of executions) { | ||
| if (isActive(exec.status)) { | ||
| activeGraphIds.add(exec.graph_id); | ||
| } | ||
| const endedTs = exec.ended_at | ||
| ? new Date( | ||
| exec.ended_at instanceof Date | ||
| ? exec.ended_at.getTime() | ||
| : exec.ended_at, | ||
| ).getTime() | ||
| : 0; | ||
| if (isFailed(exec.status) && endedTs > cutoff) { | ||
| errorGraphIds.add(exec.graph_id); | ||
| } | ||
| if ( | ||
| exec.status === AgentExecutionStatus.COMPLETED && | ||
| endedTs > cutoff | ||
| ) { | ||
| completedGraphIds.add(exec.graph_id); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| for (const agent of agents) { | ||
| if (activeGraphIds.has(agent.graph_id)) { | ||
| counts.running += 1; | ||
| } else if (errorGraphIds.has(agent.graph_id)) { | ||
| counts.error += 1; | ||
| } else if (agent.has_external_trigger) { | ||
| counts.listening += 1; | ||
| } else if (agent.recommended_schedule_cron) { | ||
| counts.scheduled += 1; | ||
| } else { | ||
| counts.idle += 1; | ||
| } | ||
| if (completedGraphIds.has(agent.graph_id)) { | ||
| counts.completed += 1; | ||
| } | ||
| } | ||
|
|
||
| return counts; | ||
| }, [agents, executions]); | ||
| } |
There was a problem hiding this comment.
Return a loading state here instead of an all-zero summary.
This hook currently reports 0 for every fleet count until executions load, which is indistinguishable from a genuinely empty fleet and can flash incorrect briefing/filter totals in the UI. useLibraryFleetSummary in this same PR already avoids that by waiting for a successful query.
Based on learnings, prefer checking isSuccess when gating on React Query results because data may be undefined in error/idle states.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/frontend/src/app/`(platform)/library/hooks/useAgentStatus.ts
around lines 144 - 208, The hook useFleetSummary currently builds and returns an
all-zero FleetSummary while the executions query is still loading; update it to
detect the query status from useGetV1ListAllExecutions (use its isSuccess flag)
and return a distinct loading state instead of the zeroed summary. Specifically,
inside useFleetSummary, read isSuccess from useGetV1ListAllExecutions and if
isSuccess is false return an explicit loading sentinel (e.g., null or a
FleetSummary with a loading boolean) so callers can differentiate "loading" from
a genuine empty fleet; only compute the counts inside the useMemo when isSuccess
is true and executions is available. Ensure references to useFleetSummary,
useGetV1ListAllExecutions, and FleetSummary are updated accordingly.
| const [statusFilter, setStatusFilter] = useState<AgentStatusFilter>("all"); | ||
| const isAgentBriefingEnabled = useGetFlag(Flag.AGENT_BRIEFING); | ||
| const { agents } = useLibraryAgents(); | ||
| const fleetSummary = useLibraryFleetSummary(agents); |
There was a problem hiding this comment.
Scope the status filter UI to the same dataset as the active tab.
fleetSummary is built from the global useLibraryAgents() result, but statusFilter is applied to whatever list the active tab is showing. On the Favorites tab, that means the dropdown counts describe all agents while the list only shows favorites. Either compute a favorites-specific summary, or reset/hide the status filter when activeTab !== "all".
One simple way to avoid the mismatch
function handleTabChange(tabId: string) {
setActiveTab(tabId);
setSelectedFolderId(null);
+ if (tabId !== "all") {
+ setStatusFilter("all");
+ }
}
...
- fleetSummary={isAgentBriefingEnabled ? fleetSummary : undefined}
- briefingAgents={isAgentBriefingEnabled ? agents : undefined}
+ fleetSummary={
+ isAgentBriefingEnabled && activeTab === "all"
+ ? fleetSummary
+ : undefined
+ }
+ briefingAgents={
+ isAgentBriefingEnabled && activeTab === "all" ? agents : undefined
+ }Also applies to: 58-61
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@autogpt_platform/frontend/src/app/`(platform)/library/page.tsx around lines
24 - 27, The statusFilter dropdown is showing counts derived from fleetSummary
(created from useLibraryAgents()) while the UI list is scoped by activeTab,
causing mismatched counts on non-"all" tabs like Favorites; update the logic so
the summary is computed from the same dataset the active tab uses (e.g., derive
a tabScopedAgents list from useLibraryAgents() filtered by activeTab and pass
that into useLibraryFleetSummary) or, alternatively, clear or hide statusFilter
(via setStatusFilter) when activeTab !== "all" so the counts and filter behavior
stay consistent with the displayed agents.
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
Summary
Temporary preview branch consolidating 5 active PRs for joint testing:
Do not merge — this branch is for preview/testing only. Each PR will be merged individually.