feat(platform): add remote Local PC execution targets - #13050
Conversation
⚠️ EXPERIMENTAL / DANGEROUS / UNTESTED — DO NOT MERGE TO MAIN Adds spec, docs, and skeleton code for connecting the AutoGPT hosted platform to a user's local machine as an execution backend, instead of (or alongside) E2B cloud sandboxes. What's here: - docs/VISION.md — dream vision + concrete platform changes required for each capability (computer use, hardware, LLM routing, privacy mode, multi-machine, background tasks) - docs/PROTOCOL.md — full WebSocket message protocol spec (v0.1) - docs/PLATFORM_HOOKS.md — every file in the platform that needs changing, with code sketches for each insertion point - docs/OAUTH_FLOW.md — auth design using AutoGPT's existing OAuth provider - docs/SECURITY.md — threat model, defense layers, known limitations - shim/ — Python package skeleton (autogpt-local-executor) - daemon.py — WebSocket reconnect loop + message dispatcher - handlers.py — FileHandler, CommandHandler, ComputerUseHandler - auth.py — OAuth PKCE flow + OS keychain token storage - config.py — ShimConfig (pydantic-settings) - protocol.py — MessageType constants + build_hello() - cli.py — autogpt-shim auth|start|stop|status|revoke - pyproject.toml — package definition + optional deps - platform/local_pc_shim.py — platform-side duck-type AsyncSandbox proxy Nothing here runs yet. All execution stubs raise NotImplementedError. Rich docstrings describe what each piece needs to do and how it connects to the existing platform code. Computer use notes (from research): Claude's computer_20251124 beta tool drives a screenshot→analyze→act loop. Shim captures screen via pyautogui, returns base64 JPEG. Platform must pass betas=["computer-use-2025-11-24"] to Anthropic API. Injection point: stream_chat_completion_sdk() in sdk/service.py. Primary platform insertion point: _setup_e2b() in copilot/sdk/service.py ~L3815 → rename to _setup_executor(), add LocalPCShim branch before E2B branch → LocalPCShim satisfies AsyncSandbox duck-type → zero changes downstream Co-authored-by: AutoPilot <autopilot@autogpt.net>
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR introduces a Local PC Executor feature spanning backend and frontend: session execution-target selection (cloud/local), a shim/relay WebSocket protocol, machine control, consent, workflow recording and skill generation, Copilot SDK tool integration, and UI for picking/using local execution. It also hardens OAuth with PKCE public-client support, deny-authorization, and refresh-token rotation/revocation, plus supporting schema, CLI, and API-spec changes. ChangesLocal PC Executor Platform
OAuth PKCE, Public Clients, and Refresh-Token Rotation
Estimated code review effort: 5 (Critical) | ~180 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
This PR targets the Automatically setting the base branch to |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
/dev-screenshot |
![autogpt-pr-reviewer-in-dev[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
Now I have all the evidence I need. Let me write the report.
🔎 QA Validation: PR #13050
Test Plan
This PR is a docs/spec/scaffold-only PR — all files live under experimental/local-pc-executor/, no production code is modified, and all stubs raise NotImplementedError. My QA focus is:
- No regression — production services (backend, frontend, copilot) still work
- Syntax validity — skeleton Python files compile without errors
- No route registration — no new WebSocket endpoints actually exist on the running backend
- Negative test — unauthorized access still blocked
| # | Scenario | Result | Expected | Actual | Evidence |
|---|---|---|---|---|---|
| 1 | Backend API still works | ✅ | 200 + valid response | graphs returns 0, sessions create OK |
session_id: dd650b2a-31fc-4ee6-9a49-5355c95aaa66 |
| 2 | Frontend loads | ✅ | 200 OK | 200 OK | curl -sf http://localhost:3000 → OK |
| 3 | Copilot page loads after login | ✅ | Page renders | Page renders (45KB screenshot) |  |
| 4 | No local-executor routes registered | ✅ | No routes in OpenAPI | Empty result from jq filter | `jq '.paths |
| 5 | Skeleton Python files compile | ✅ | All 8 files compile | All 8 py_compile pass |
init.py: OK, config.py: OK, etc. |
| 6 | Unauthenticated access blocked | ✅ | 401/403 | "Authorization header is missing" |
Confirmed |
| 7 | No experimental-related backend errors | ✅ | No errors | No grep matches | grep "local_pc|experimental" → empty |
Evidence
Scenario 1 — Backend API
- Copilot session creation:
{"id": "dd650b2a-31fc-4ee6-9a49-5355c95aaa66"} - Graphs listing: returned
0(valid empty array)
Scenario 3 — Copilot page
Screenshot 45KB — page rendered properly.
Scenario 5 — Syntax check
All 8 Python files pass py_compile: init.py, config.py, cli.py, auth.py, daemon.py, handlers.py, protocol.py, local_pc_shim.py — all OK.
Scenario 6 — Negative test
curl -s http://localhost:8006/api/graphs → "Authorization header is missing"
Code Issue Found (not from code review — from compile/import analysis)
protocol.py line 13 has import pyautogui at module top level — this is an unconditional import of an optional dependency. The comment on line 14 says "If pyautogui not installed, screen_resolution advertised as None" but the import will raise ImportError before reaching that logic. This will break from .protocol import ... for anyone who installs the base package without the [computer-use] extra.
Verdict: ✅ QA PASS
This is a pure scaffold/spec PR — no production code modified. All services remain fully operational with zero regressions. The only executable concern is the unconditional pyautogui import in protocol.py which would fail at import time if pyautogui isn't installed.
{
"recommendation": "APPROVE",
"summary": "Docs/scaffold-only PR with no production impact; all services verified operational, one unconditional import bug found in skeleton code",
"findings": [
{
"severity": "medium",
"category": "import error",
"file": "experimental/local-pc-executor/shim/autogpt_local_executor/protocol.py",
"line": 13,
"description": "Unconditional `import pyautogui` at module level will raise ImportError for anyone installing the base package without the `[computer-use]` optional dependency. The comment on line 14 claims graceful degradation but the import crashes before reaching that logic.",
"suggestion": "Wrap in try/except: `try: import pyautogui except ImportError: pyautogui = None` and guard usage in `build_hello()`."
},
{
"severity": "low",
"category": "unreachable code",
"file": "experimental/local-pc-executor/shim/autogpt_local_executor/handlers.py",
"line": 322,
"description": "The `return _ack(msg['id'])` after `raise NotImplementedError(...)` in `_handle_input` is unreachable dead code.",
"suggestion": "Remove the unreachable return statement, or move it to replace the raise once the implementation is complete."
},
{
"severity": "low",
"category": "missing type import",
"file": "experimental/local-pc-executor/shim/autogpt_local_executor/auth.py",
"line": 82,
"description": "`OAuthFlow.__init__` uses `config: Any` but `Any` is not imported from `typing` in this file — only `Optional` is imported.",
"suggestion": "Add `Any` to the typing imports on line 20: `from typing import Any, Optional`"
}
]
}
|
|
|
Platform-side implementation for routing copilot execution to the
user's local machine via the autogpt-local-executor shim.
- ShimConnectionManager: in-memory WebSocket registry with wait_for()
- LocalPCShim: duck-type drop-in for E2B AsyncSandbox
- .commands.run(), .files.read(), .files.write(), .pause(), .kill()
- WebSocket endpoint: /ws/local-executor/{session_id}?token=<access_token>
- Validates token via introspect_token() before accepting connection
- HELLO/HELLO_ACK handshake
- Wire into _setup_e2b(): LocalPC branch runs before E2B branch
- Pause guard: skip pause_sandbox_direct for LocalPCShim instances
- Config fields: use_local_pc_executor, allow_computer_use,
local_pc_executor_ws_path
Requires: autogpt-local-executor shim running on user's machine
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
🟢 Low Risk — File Overlap OnlyThese PRs touch the same files but different sections (click to expand)
Summary: 3 conflict(s), 0 medium risk, 9 low risk (out of 12 PRs with file overlap) Auto-generated on push. Ignores: |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13050 +/- ##
==========================================
+ Coverage 76.02% 76.58% +0.56%
==========================================
Files 2688 2752 +64
Lines 204206 212950 +8744
Branches 19674 20408 +734
==========================================
+ Hits 155246 163090 +7844
- Misses 44672 45305 +633
- Partials 4288 4555 +267
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The shim repo (autogpt-local-executor) is the source of truth for the protocol and platform-hook docs; the PR carries a snapshot under experimental/local-pc-executor/ for reviewers who pull only this branch. Mirrors: - new docs/CROSS_PLATFORM.md (OS matrix, per-dimension tables, path-jail algorithm with pseudocode, WSL2 section) - PROTOCOL.md: HELLO platform enum normalised to darwin|linux|windows|wsl2; arch enum normalised to x86_64|arm64; EXECUTE_COMMAND shell selector + argv form; encoding/format mapping for FILE_READ; FILE_STAT/LIST/DELETE/ MOVE message types; CRLF-as-is policy - SECURITY.md: per-OS path-attack table; per-OS keychain availability with encrypted-file fallback - PLATFORM_HOOKS.md: new section 10 listing platform-side adapter work (drop E2B_WORKDIR, FILE_STAT instead of readlink -f, shell pass-through, E2B-kwarg translation, OAuth port range) - OAUTH_FLOW.md: port fallback range 41899-41910 - README.md: link to CROSS_PLATFORM.md
Implements the platform-side groundwork for the cross-OS spec landed in experimental/local-pc-executor/docs/. - LocalPCShim now exposes machine_id, platform, arch, allowed_root, capabilities, shim_version, screen_resolution, local_llm_models, and hardware_devices as attributes, populated from a ShimHello dataclass captured by the WebSocket route during the HELLO handshake. - ShimConnectionManager.register() takes the parsed ShimHello and stores it alongside the WebSocket so LocalPCShim.for_session() can construct with the right metadata. - _CommandsProxy.run() gains shell= (defaults to "auto") and argv= kwargs so platform code can avoid bash-c assumptions on Windows; keeps E2B- compatible envs= and timeout= kwarg translation to wire env / timeout_seconds. - _FilesProxy gains stat/list/delete/move methods mirroring the new FILE_STAT, FILE_LIST, FILE_DELETE, FILE_MOVE message types so callers no longer have to shell out to readlink/ls/find/rm/mv for cross-OS paths. - context.py adds get_workdir(sandbox) and get_allowed_dirs(sandbox) helpers that return shim.allowed_root for LocalPCShim or fall back to the existing E2B_WORKDIR / E2B_ALLOWED_DIRS for E2B. Call-site audit (E2B_WORKDIR → get_workdir, etc.) is a follow-up; these helpers are the entry point. - Adds local_pc_shim_test.py covering ShimHello parsing, the format=bytes/text contract, str/bytes write encoding, shell selector defaults, argv form skipping the shell field, E2B kwarg translation (envs→env, timeout→timeout_seconds), and FILE_STAT. PROTOCOL.md and PLATFORM_HOOKS.md §10 in this PR document the contracts this commit implements.
Splits LocalPCShim file ops into a parallel module and teaches the
existing E2B file-tool handlers to delegate when the active executor
is a shim. The MCP tool registration shape is unchanged — same
read_file/write_file/edit_file/glob/grep names, same schemas — so the
LLM-facing interface is identical for either executor.
What landed:
- New backend/copilot/sdk/local_pc_file_tools.py with helpers that wrap
sandbox.files.read/write/stat/list/delete/move directly, plus
is_local_pc() type-guard and describe_workspace() for tool-prompt
rendering.
- New backend/copilot/context.resolve_executor_path(path, sandbox)
that picks workdir + allowed_dirs per executor (shim.allowed_root for
LocalPCShim, E2B_WORKDIR/E2B_ALLOWED_DIRS otherwise). resolve_sandbox_path
is kept for callers that are E2B-only.
- e2b_file_tools.py chokepoint branches:
* _get_sandbox_and_path uses resolve_executor_path.
* _sandbox_write skips the /tmp base64-tee uid-mismatch workaround
when the executor is a shim (single OS user, no sticky-bit issue).
* _check_sandbox_symlink_escape uses FILE_STAT(follow_symlinks=True)
when the executor is a shim instead of `readlink -f`, which
doesn't exist on macOS or Windows.
* _handle_glob branches: shim uses FILE_LIST(glob=..., recursive=True)
so cross-OS works without POSIX `find`.
* _handle_grep branches: shim sends the grep argv via the wire's
argv form. On Windows without bash/grep the shim returns
SHELL_NOT_AVAILABLE and the LLM can retry via bash_exec.
- bash_exec.py: when the executor is a shim, send `command` with
shell="auto" instead of wrapping in `bash -c "..."`. Avoids the
fail-on-Windows path and lets the shim pick the OS-native default
shell per CROSS_PLATFORM.md.
- Tests covering resolve_executor_path jail boundaries (sibling-root
attack, traversal, absolute outside-root), is_local_pc type-guard,
describe_workspace per OS, and stat/list/move pre-RPC jail-fail
semantics.
The E2B uid-mismatch workaround, `readlink -f`, `find`, and POSIX
`grep` shellouts all remain wired for the E2B path — this is purely
additive for LocalPCShim and doesn't change E2B behavior.
…into experimental/local-pc-executor # Conflicts: # autogpt_platform/backend/backend/api/rest_api.py
…into experimental/local-pc-executor # Conflicts: # autogpt_platform/frontend/src/app/api/openapi.json
github-actions
Bot
removed
the
conflicts
|
Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly. |
ntindle
changed the title
|
/review |
|
/dev-review |
|
I couldn't load the latest pull request details from GitHub, so the review was not queued: Failed to load pull request diff: Client error '406 Not Acceptable' for url 'https://api.github.com/repos/Significant-Gravitas/AutoGPT/pulls/13050' |
|
@coderabbitai review |
|
I couldn't load the latest pull request details from GitHub, so the review was not queued: Failed to load pull request diff: Client error '406 Not Acceptable' for url 'https://api.github.com/repos/Significant-Gravitas/AutoGPT/pulls/13050' |
✅ Action performedReview finished.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit e9d1ed5. Configure here.
| else 503 | ||
| ) | ||
| raise HTTPException(status_code=status_code, detail=str(exc)) from exc | ||
| raise |
There was a problem hiding this comment.
Detach failure deletes valid session
Medium Severity
After Local PC session creation succeeds (DB row, activation, and data-channel checks), a failure on the final detach_machine_session still runs _compensate_local_session_creation, which deletes the new chat session. The API then returns an error even though setup completed, and the executor may keep a stale machine attachment for a session id that no longer exists.
Reviewed by Cursor Bugbot for commit e9d1ed5. Configure here.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (30)
autogpt_platform/backend/backend/api/features/chat/routes.py-757-793 (1)
757-793: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDelete the session before tearing down its local executor.
Detachment and shim termination happen before the org-scoped
delete_chat_sessioncall. If deletion fails, raises, or rejects an org mismatch, the still-persisted session has already been disrupted. Move this cleanup after a successful deletion, matching the E2B cleanup ordering.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/chat/routes.py` around lines 757 - 793, Move the local executor cleanup blocks using detach_machine_session and shim.kill in the session-deletion flow so they run only after delete_chat_session completes successfully. Preserve the existing cleanup error handling, and ensure deletion—including organization validation—occurs before any local session disruption, matching the E2B cleanup ordering.autogpt_platform/backend/backend/api/features/local_executor/state.py-121-142 (1)
121-142: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake lifecycle transitions atomic and enforce the expected prior status.
Both functions read state and later overwrite it unconditionally. Concurrent stop/review requests can lose fields, while a delayed stop can regress
reviewedback tostopped. Move the read, status validation, merge, and write into one Lua script or a Redis transaction with optimistic locking; permit onlyrecording → stopped → reviewed.As per coding guidelines, Redis multi-step operations must be atomic and transactional.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/local_executor/state.py` around lines 121 - 142, Update mark_recording_stopped and mark_recording_reviewed to perform state read, expected-status validation, field merge, and write atomically using a Lua script or Redis transaction with optimistic locking. Enforce only recording → stopped → reviewed transitions, rejecting stale or concurrent requests without overwriting existing fields, and preserve the returned RecordingState behavior for successful transitions.Source: Coding guidelines
autogpt_platform/backend/backend/api/features/local_executor/websocket.py-213-224 (1)
213-224: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject Local PC shims for cloud-target sessions.
Ownership alone lets a shim bind to a cloud session; the route helpers then report or operate on that shim because they only validate bindings for local targets.
autogpt_platform/backend/backend/api/features/local_executor/websocket.py#L213-L224: require the owned session’s execution target to belocal.autogpt_platform/backend/backend/api/features/local_executor/routes.py#L380-L387: return no executor for non-local targets before reading remembered HELLO data.autogpt_platform/backend/backend/api/features/local_executor/routes.py#L477-L486: reject non-local targets before returning a connected shim.This violates the stated fail-closed executor-routing objective.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/local_executor/websocket.py` around lines 213 - 224, Reject cloud-target sessions throughout local executor routing: in websocket.py lines 213-224, require the owned session metadata execution target to be local before accepting the shim; in routes.py lines 380-387, return no executor for non-local targets before reading remembered HELLO data; and in routes.py lines 477-486, reject non-local targets before returning a connected shim. Preserve the existing ownership and denial behavior for local sessions.autogpt_platform/backend/backend/api/features/local_executor/routes.py-318-327 (1)
318-327: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake recording STOP retry-safe when state persistence fails.
If
stop()succeeds butmark_recording_stopped()fails, the retry finds no summary and sends the explicitly non-idempotent STOP again. Make STOP idempotent byrecording_id, or durably record a recoverable stop intent/result before allowing retries.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/local_executor/routes.py` around lines 318 - 327, Update the STOP flow around _get_stopped_recording_summary, shim.recording.stop, and mark_recording_stopped so a successful stop remains recoverable if persistence fails. Make the stop operation idempotent by recording_id, or durably persist a recoverable stop intent/result before retrying; ensure retries do not invoke the non-idempotent shim STOP a second time.autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalFolderPicker/useLocalFolderPicker.ts-59-62 (1)
59-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winClear the previous directory whenever the executor identity becomes stale.
These mismatch branches leave
directorypopulated, soUse This Foldercan remain enabled and submit the old connection/browse grant. Centralize stale handling and cleardirectory,history, andlastTarget, as the 409 path already does.Proposed stale-state handling
+ function markStale(message: string) { + setDirectory(null); + setHistory([]); + setLastTarget({ directoryRef: null, history: [] }); + setError(message); + onStale(message); + }Use
markStale(...)in every connection, browse, and directory mismatch branch.Also applies to: 112-116, 175-183
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalFolderPicker/useLocalFolderPicker.ts around lines 59 - 62, Centralize stale executor handling in useLocalFolderPicker via markStale, and use it for the connection, browse, and directory mismatch branches (including the shown response.connection_id check and the branches around the additional referenced ranges). Ensure markStale clears directory, history, and lastTarget while preserving each branch’s stale message, matching the existing 409 behavior.autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalExecutorSetup/LocalExecutorSetup.tsx-16-21 (1)
16-21: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPin the installer target. This installs from the repo branch tip, so the code pulled by
pipxcan change over time and introduces avoidable supply-chain risk. Use an immutable commit SHA or versioned release instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalExecutorSetup/LocalExecutorSetup.tsx around lines 16 - 21, Update the installer command in REQUIRED_SETUP_STEPS to reference an immutable commit SHA or explicitly versioned release instead of the repository branch tip, while preserving the existing pipx installation flow.autogpt_platform/frontend/src/app/(platform)/copilot/components/LocalPCRecordingConsent/LocalPCRecordingConsent.tsx-47-52 (1)
47-52: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPrevent dialog dismissal while cloud consent is being submitted.
The buttons are disabled during submission, but Escape or an overlay dismissal still calls
onKeepLocal. That can return the UI to review while the cloud-processing request continues, misrepresenting whether screenshots remain local.Proposed fix
controlled={{ isOpen, set: async (open) => { - if (!open) onKeepLocal(); + if (!open && !isSubmitting) onKeepLocal(); }, }}Also applies to: 100-115
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/LocalPCRecordingConsent/LocalPCRecordingConsent.tsx around lines 47 - 52, Update the controlled dialog state setter in LocalPCRecordingConsent so dismissal attempts are ignored while cloud consent submission is in progress; only invoke onKeepLocal when closing is allowed, preserving normal dismissal behavior otherwise.autogpt_platform/frontend/src/app/(platform)/copilot/components/LocalPCWarning/LocalPCWarning.tsx-25-27 (1)
25-27: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftScope the safety acknowledgement to the authenticated user.
This origin-wide key means one account’s acknowledgement suppresses the Local PC shell warning for every later account using the same browser profile. Store it under a user-scoped key or persist the acknowledgement server-side so each user explicitly accepts the warning.
Also applies to: 32-40
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/LocalPCWarning/LocalPCWarning.tsx around lines 25 - 27, Update the LocalPCWarning acknowledgement persistence to be scoped to the authenticated user rather than the shared origin-wide localStorage key. Use the available authenticated user identifier when constructing the storage key and when reading, writing, or clearing the acknowledgement, so each user must explicitly acknowledge the warning independently.autogpt_platform/frontend/src/app/(platform)/copilot/hooks/useRecordingWorkflow.ts-22-64 (1)
22-64: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winScope the workflow state to
sessionID.The hook retains
recordingID, steps, and phase when the active session changes. BecauseCopilotChatHost.tsxrendersRecordWorkflowwithout a session key, session A’s review can appear under session B, and submission combines B’s session ID with A’s recording ID.Key
RecordWorkflowbysessionID, or explicitly stop/reset this hook whenever the session identity changes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/hooks/useRecordingWorkflow.ts around lines 22 - 64, The useRecordingWorkflow state must reset when sessionID changes to prevent recording data from one session appearing in another. Update the workflow integration around useRecordingWorkflow and RecordWorkflow so the component is keyed by sessionID, or explicitly stop and clear recordingID, steps, originalStepSeqs, phase, and related state on session changes.autogpt_platform/frontend/src/app/(platform)/copilot/components/RecordingReview/RecordingReview.tsx-253-276 (1)
253-276: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDisable step edits while the review submission is in flight.
submitReviewsnapshots the edits before the request, but these controls remain active. A user can redact or delete sensitive data afterward and reach “ready” even though that edit was never applied.Proposed fix
<Button variant="ghost" size="icon" + disabled={isSubmitting} aria-label={`Hide value for step ${step.seq}`} onClick={() => onRedactStep(step.seq)} > <Button variant="ghost" size="icon" + disabled={isSubmitting} aria-label={`Delete step ${step.seq}`} onClick={() => onDeleteStep(step.seq)} >Also extend the submitting-state test to cover both controls.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/RecordingReview/RecordingReview.tsx around lines 253 - 276, Disable both the redact and delete controls in the step action area while review submission is in flight by wiring the existing submitting state into the Buttons around onRedactStep and onDeleteStep. Preserve their current visibility and handlers, and extend the submitting-state test to verify both controls are disabled.autogpt_platform/backend/backend/copilot/tools/recording_skill.py-54-62 (1)
54-62: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not serialize demonstrated parameter values.
_strip_values()clears the recording, but the same names, emails, and other values remain inSkillParameter.sample_values, andGeneratedSkill.to_dict()exports them. Remove this field from serialized output or keep samples in a separate ephemeral inference context.Also applies to: 150-160, 341-348
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/recording_skill.py` around lines 54 - 62, Stop exposing demonstrated parameter values in serialized skill recordings. Update SkillParameter.to_dict() and the corresponding GeneratedSkill.to_dict() serialization paths to omit sample_values, while preserving sample_values only for in-memory inference; ensure _strip_values() leaves no names, emails, or other captured values reachable through exported output.autogpt_platform/backend/backend/copilot/tools/recording_skill.py-164-164 (1)
164-164: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit skill generation and replay into focused modules.
This 901-line file combines DTOs, inference/rendering, and asynchronous replay, with several functions exceeding 40 lines and public APIs below helpers. Extract models, generation, and replay responsibilities before extending the scaffold further.
As per coding guidelines, “Keep files under ~300 lines,” “Keep functions under ~40 lines,” and “Use top-down ordering.”
Also applies to: 474-556, 610-901
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/recording_skill.py` at line 164, Refactor recording_skill.py by separating DTO/model definitions, skill generation and inference/rendering, and asynchronous replay into focused modules. Move the corresponding helpers and public APIs together, preserve existing behavior and interfaces, keep each module under roughly 300 lines, and reduce functions exceeding 40 lines while ordering public APIs before their helpers.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/recording_skill.py-710-717 (1)
710-717: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not mark unavailable read-back as a successful dry run.
A missing reader, read error, or
Noneresult returns(False, True), allowingrows_okandDryRunResult.okto report success despite validating nothing. Return a failed/inconclusive result and type the shim/capabilities with a protocol instead of probing them dynamically.As per coding guidelines, “Do not use duck typing — avoid
hasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead.”Also applies to: 772-779, 878-900
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/recording_skill.py` around lines 710 - 717, Update dry_run and its read-back validation paths to treat a missing reader, read error, or None result as failed/inconclusive so rows_ok and DryRunResult.ok cannot report success without validation. Replace dynamic capability probing in dry_run and the referenced paths with a typed protocol or union for shim capabilities, using explicit interface methods and typed error handling instead of hasattr, getattr, or isinstance dispatch.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/local_pc_machine.py-156-178 (1)
156-178: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winTranslate malformed machine envelopes at the RPC boundary.
Invalid JSON currently escapes as
JSONDecodeError, while any matching-ID dictionary with a dictionary payload is accepted regardless of response type. Validate the full envelope and convert malformed responses intoMachineControlError; otherwise executor version drift produces an unstructured 500.As per coding guidelines, “Use Pydantic models over dataclass/namedtuple/dict for structured data.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_machine.py` around lines 156 - 178, Update the response-processing loop around transport.iter_text() to validate each matching machine envelope with a Pydantic model, requiring the expected response type and payload structure instead of accepting arbitrary dictionaries. Catch JSON parsing and envelope-validation failures and translate them into MachineControlError with the existing invalid-response classification, while preserving structured executor ERROR handling and details.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/local_pc_machine.py-212-222 (1)
212-222: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winVerify that every returned binding belongs to the requested session.
model_validate()checks shape but accepts a differentsession_id. A faulty executor can therefore attach, activate, or restore another local session and root grant. Compare the validated ID againstsession_id/binding.session_idand raiseINVALID_MACHINE_RESPONSEon mismatch.Also applies to: 229-236, 243-248
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_machine.py` around lines 212 - 222, Validate that every MachineSessionBinding returned by the ATTACH_SESSION, activation, and restore flows has a session_id matching the requested session_id. After each model_validate call, compare binding.session_id with the requested value and raise INVALID_MACHINE_RESPONSE on mismatch; preserve the existing return behavior for matching bindings.autogpt_platform/backend/backend/copilot/tools/bash_exec.py-249-268 (1)
249-268: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPreserve Local PC error translation instead of returning an E2B failure.
Exceptions from this new branch fall through to Lines 296–301, producing
e2b_execution_errorand"E2B execution failed". Route Local PC exceptions through the cohort’s Local PC error translator so disconnect, stale-session, shell, and protocol errors retain their actionable codes.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/bash_exec.py` around lines 249 - 268, Update the local_pc execution branch around sandbox.commands.run and _build_completion_response to catch its exceptions and pass them through the existing Local PC error translator. Ensure disconnect, stale-session, shell, and protocol failures retain their actionable Local PC error codes and messages instead of reaching the generic E2B failure handler.autogpt_platform/backend/backend/copilot/tools/local_llm_router_test.py-23-44 (1)
23-44: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace the duck-typed executor stand-in.
Use a typed fake or
MagicMock/autospec backed byLocalPCShim, and annotate_make_executoraccordingly. The currentSimpleNamespacecan drift from the production contract unnoticed.As per coding guidelines, “Do not use duck typing — use typed interfaces/unions/protocols instead.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_llm_router_test.py` around lines 23 - 44, Replace the SimpleNamespace returned by _make_executor with a typed LocalPCShim-backed fake or an autospecced MagicMock, and annotate the helper’s return type accordingly. Ensure the stand-in exposes the capabilities, local_llm_models, and capability_set values required by the router while remaining checked against the LocalPCShim contract.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py-37-59 (1)
37-59: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expose full local filesystem paths in error text.
These helpers include
allowed_root, requested paths, and sometimes the entire raw message in LLM-visible errors. Use basename-only structured path details and a generic “workspace root” label; do not fall back to the raw path-bearing message.As per coding guidelines, “Sanitize error paths by using
os.path.basename()in error messages to avoid leaking directory structure.”Also applies to: 87-142
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py` around lines 37 - 59, The path helpers in _shim_allowed_root and _details_path expose directory structure and raw path-bearing messages. Return a generic “workspace root” label for allowed_root, sanitize selected detail values with os.path.basename(), and use a non-path generic fallback instead of message; apply the same sanitization to related error construction in the referenced local-PC error handling.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/local_pc_shim.py-1496-1566 (1)
1496-1566: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound and time-limit Local LLM streams.
Queue()is unbounded andqueue.get()has no deadline. A connected but faulty shim can flood chunks until OOM or omit the terminal response and pin the Copilot request indefinitely. Add a bounded frame/byte budget and an overall or idle completion timeout that terminates the stream withLocalLLMError.Also applies to: 2346-2356
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_shim.py` around lines 1496 - 1566, Bound and time-limit the stream handled by complete: register a bounded per-request queue or enforce cumulative frame/byte limits while consuming queue, and apply an overall or idle timeout to queue.get(). When either limit is exceeded, raise LocalLLMError with an appropriate failure code/message; preserve normal chunk yielding, terminal response handling, and cleanup via _cleanup_stream(msg_id).autogpt_platform/backend/backend/copilot/tools/local_pc_shim.py-423-440 (1)
423-440: 📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy liftSplit the 2,500-line shim module by responsibility.
Connection management, file/computer proxies, Local LLM streaming, recording, and the adapter lifecycle should be separate modules with
LocalPCShimas the facade. Several functions also substantially exceed the 40-line limit.As per coding guidelines, “Keep files under ~300 lines; if a file grows beyond this, split by responsibility” and “Keep functions under ~40 lines.”
Also applies to: 812-813, 1478-1494, 1637-1648, 1886-1958
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_shim.py` around lines 423 - 440, Split the oversized shim module by responsibility, keeping LocalPCShim as the facade: move ShimConnectionManager and connection lifecycle into a dedicated module, file/computer proxy operations, Local LLM streaming, recording, and adapter lifecycle into separate modules. Refactor the functions identified by the review, including the regions around ShimConnectionManager and the listed ranges, so each stays under roughly 40 lines and each resulting module remains near 300 lines or less while preserving existing behavior and public interfaces.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/tools/recording_models.py-180-205 (1)
180-205: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winParse consent and redaction fields strictly and fail closed.
bool("false")evaluates toTrue, so malformed wire payloads can approve recording or mark unredacted data as redacted. Anexpires_atvalue of NaN also bypasses the expiry check. Accept only literal JSON booleans and finite timestamps.Proposed fix
+import math from typing import Any ... - redacted=bool(payload.get("redacted", False)), + redacted=payload.get("redacted") is True, ... - redaction_applied=bool(payload.get("redaction_applied", False)), + redaction_applied=payload.get("redaction_applied") is True, ... try: parsed_expiry = float(expires_at) if expires_at is not None else None except (TypeError, ValueError): parsed_expiry = None + if parsed_expiry is not None and not math.isfinite(parsed_expiry): + parsed_expiry = None return cls( - approved=bool(payload.get("approved", False)), + approved=payload.get("approved") is True,Add regressions for
"approved": "false","redaction_applied": "false", and non-finite expiration values.Also applies to: 227-251, 291-309
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/recording_models.py` around lines 180 - 205, Update the payload parsing methods for consent, redaction, and expiration fields to accept only literal boolean values, avoiding truthiness conversion of strings such as "false"; invalid values must fail closed. Validate expires_at timestamps as finite before applying expiry checks, treating NaN and infinities as invalid. Add regressions covering string "false" values for approved and redaction_applied and non-finite expiration values.autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py-394-421 (1)
394-421: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd the missing
OP_UNCONFIRMEDtranslation.
EXECUTE_COMMAND, delete, move, input, focus, launch, and clipboard writes all emit this code, but it currently falls through to the raw passthrough message. That omits the required warning to inspect state before retrying a potentially completed side effect.Proposed fix
+def _op_unconfirmed( + code: str, message: str, details: dict, shim: "LocalPCShim | None" +) -> str: + op = details.get("op") or "operation" + return ( + f"The {op} request may have completed before the connection dropped. " + "Inspect the current state with a read-only operation before retrying." + ) + _TRANSLATIONS: dict[str, _Translator] = { + "OP_UNCONFIRMED": _op_unconfirmed,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py` around lines 394 - 421, Add an OP_UNCONFIRMED entry to the _TRANSLATIONS mapping, pointing to the existing translator that provides the required inspect-before-retry warning. Keep the surrounding error-code mappings unchanged and ensure all operations emitting OP_UNCONFIRMED use this translated message instead of raw passthrough.autogpt_platform/backend/backend/copilot/sdk/service.py-3821-3839 (1)
3821-3839: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClose a shim that fails the final session-binding check.
If
LocalPCShim.for_session()returns the wrong machine or root, the exception path returns without killing that shim. The stale data channel remains attached until another attempt happens to replace it. Ensure any shim created inside this attempt is closed on failure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/service.py` around lines 3821 - 3839, Update the LocalPC shim setup around LocalPCShim.for_session and the shim_err exception handler so any shim created during the attempt is closed before returning _ExecutorSetupResult on failure, including final session-binding mismatches. Preserve the existing error logging and return behavior, and avoid closing an uninitialized shim.autogpt_platform/backend/backend/copilot/sdk/recording_tools.py-410-426 (1)
410-426: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winEnforce and test the multi-row dry-run requirement. The implementation and test currently allow the documented safety invariant to regress.
autogpt_platform/backend/backend/copilot/sdk/recording_tools.py#L410-L426: returnINVALID_ARGUMENTunless at least two row objects remain after validation.autogpt_platform/backend/backend/copilot/sdk/recording_tools_test.py#L386-L394: add a one-row regression case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/recording_tools.py` around lines 410 - 426, Enforce the multi-row invariant in the dry-run validation: after filtering `data_rows` to dictionary rows in `dry_run_skill`, return `INVALID_ARGUMENT` when fewer than two valid rows remain instead of logging and continuing. Add a one-row regression test in `autogpt_platform/backend/backend/copilot/sdk/recording_tools_test.py` covering the expected error response.autogpt_platform/backend/backend/copilot/sdk/env.py-145-155 (1)
145-155: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGate the beta on the effective SDK transport, not configured OpenRouter fields.
config.openrouter_activecan remain true in subscription mode even though the SDK bypasses OpenRouter. That incorrectly disables computer use for a valid subscription session. Check the selected/effective transport instead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/env.py` around lines 145 - 155, Update the computer_use_via_cli condition in the environment setup to gate the beta using the SDK’s selected/effective transport rather than config.openrouter_active. Preserve the existing beta environment-variable updates and disable flag behavior, while allowing computer use when subscription mode bypasses OpenRouter.autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py-851-871 (1)
851-871: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winDo not expand one fine-grained input grant into every input tool.
input.clickcurrently matchesinput.and enables click, type, key, and scroll. This over-grants the shim’s advertised capability. Map each fine feature to its corresponding tool; reserve whole-family expansion for the coarseinputfeature.Proposed mapping
- "input.": { - "local_pc_click", - "local_pc_type", - "local_pc_key", - "local_pc_scroll", - }, + "input.click": {"local_pc_click"}, + "input.type": {"local_pc_type"}, + "input.key": {"local_pc_key"}, + "input.scroll": {"local_pc_scroll"},🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/tool_adapter.py` around lines 851 - 871, Update the fine-grained tool mapping used by the loop over fine so each input feature, such as input.click, input.type, input.key, and input.scroll, enables only its corresponding local_pc tool. Keep whole input-family expansion only for the coarse input feature, and preserve the existing mappings for unrelated features.autogpt_platform/backend/backend/copilot/sdk/file_ref.py-189-190 (1)
189-190: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUse executor-aware path resolution before reading from LocalPC.
This normalization supports the shim, but Line 183 still resolves through the E2B-only
/home/user//tmpresolver. Local relative paths and Windows paths therefore fail or target the wrong location. Useresolve_executor_path(plain, sandbox).Proposed fix
- remote = resolve_sandbox_path(plain) + remote = resolve_executor_path(plain, sandbox)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/file_ref.py` around lines 189 - 190, Update the path resolution immediately before the sandbox.files.read call to use resolve_executor_path(plain, sandbox) instead of the existing E2B-only resolver, ensuring LocalPC relative and Windows paths resolve correctly while preserving the current byte normalization.autogpt_platform/backend/backend/api/features/oauth.py-382-388 (1)
382-388: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReplace existing OAuth response keys instead of appending duplicates.
A registered URI containing
state,code, orerrorproduces duplicate parameters; clients reading the first value can validate the wrong state or code. Preserve unrelated query entries, but remove keys owned byparamsbefore appending them.Proposed fix
- query = parse_qsl(parts.query, keep_blank_values=True) + query = [ + (key, value) + for key, value in parse_qsl(parts.query, keep_blank_values=True) + if key not in params + ] query.extend(params.items())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/oauth.py` around lines 382 - 388, Update _redirect_url_with_params to remove existing query entries whose keys appear in params before appending the new params. Preserve all unrelated query entries and maintain the existing URL reconstruction behavior.autogpt_platform/backend/backend/data/auth/oauth.py-795-827 (1)
795-827: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake OAuth code consumption and token issuance transactional.
A failure after the one-time claim can permanently burn the code/refresh token and leave only one side of the new pair saved.
autogpt_platform/backend/backend/data/auth/oauth.py#L530-L537: fold the authorization-code claim into the same transaction as token minting.autogpt_platform/backend/backend/data/auth/oauth.py#L795-L827: wrap refresh rotation claim + descendant token creation in one transaction.autogpt_platform/backend/backend/api/features/oauth.py#L506-L518: persist the refresh/access pair as one unit under that transaction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/data/auth/oauth.py` around lines 795 - 827, Make authorization-code consumption and OAuth token issuance atomic. In autogpt_platform/backend/backend/data/auth/oauth.py:530-537, include the authorization-code claim in the transaction that mints tokens; in autogpt_platform/backend/backend/data/auth/oauth.py:795-827, wrap the refresh-token claim and descendant creation in the same transaction; and in autogpt_platform/backend/backend/api/features/oauth.py:506-518, persist the refresh/access pair together within that transaction so any failure rolls back all changes.autogpt_platform/backend/migrations/20260709200000_add_oauth_refresh_token_families/migration.sql-9-17 (1)
9-17: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftStage
familyIdas a backfill, not a direct default.
autogpt_platform/backend/migrations/20260709200000_add_oauth_refresh_token_families/migration.sql:9-17
DEFAULT gen_random_uuid()rewritesOAuthRefreshTokenunder anACCESS EXCLUSIVElock. AddfamilyIdnullable, backfill existing rows in batches, then setNOT NULL/default in a follow-up step; keep the index builds separate if they need to stay online.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/migrations/20260709200000_add_oauth_refresh_token_families/migration.sql` around lines 9 - 17, Update the migration around the OAuthRefreshToken family columns to add familyId as nullable without a default, backfill existing rows in batches with generated UUIDs, then enforce NOT NULL and add the default in a subsequent step. Keep familyRevokedAt and the existing indexes intact, separating index creation as needed for online operation.Source: Linters/SAST tools
🟡 Minor comments (16)
autogpt_platform/backend/backend/copilot/context.py-199-235 (1)
199-235: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winDo not expose full allowed roots in path errors.
Line 232 interpolates the complete Local PC directory path. Use a generic message such as “Path must remain within the configured workspace” while retaining only
basename(path).As per coding guidelines, “Sanitize error paths by using
os.path.basename()in error messages to avoid leaking directory structure.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/context.py` around lines 199 - 235, The ValueError in resolve_executor_path exposes full allowed directory paths through the allowed variable. Replace that portion of the message with a generic configured-workspace description, while retaining only basename(path) as the user-provided path detail.Source: Coding guidelines
autogpt_platform/backend/backend/api/features/chat/routes.py-370-370 (1)
370-370: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not default paginated local sessions back to cloud.
The
before_sequencebranch omitsmetadata, so this default constructs cloud metadata even for Local PC sessions. Make the field required and populate it in both return branches, or make it explicitly nullable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/chat/routes.py` at line 370, Make the metadata field in the paginated local-session response explicit instead of defaulting to PublicChatSessionMetadata(), which incorrectly implies cloud metadata. Update both return branches in the before_sequence handling to provide the appropriate local-session metadata, or declare the field nullable when metadata is unavailable.autogpt_platform/backend/backend/copilot/local_executor.py-11-39 (1)
11-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not HTML-encode filesystem paths in a plain-text prompt.
html.escapechanges valid roots such as/Users/A&Binto/Users/A&B, so the model receives the wrong working directory. Preserve the value with a plain-text-safe encoding such as JSON after removing control characters.Proposed fix
+import json import unicodedata -from html import escape ... - return escape(single_line, quote=True) + return json.dumps(single_line, ensure_ascii=False)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/local_executor.py` around lines 11 - 39, Update _escape_context_value, used by build_local_pc_env_context, to remove control and line-separator characters without applying HTML escaping. Encode the resulting plain-text value with JSON (or the repository’s equivalent plain-text-safe encoding) so paths such as “/Users/A&B” remain semantically correct in the prompt.autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/useExecutionTargetPicker.ts-44-67 (1)
44-67: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGate reconciliation on a successful fetch
Before the first 200 response,machinesis[], so this effect can clear a previously selected local target and show an offline error while the request is still pending. Reconcile only aftermachinesQuery.isSuccess.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/useExecutionTargetPicker.ts around lines 44 - 67, Update reconcileSelectedMachine in useExecutionTargetPicker to run the empty-machines reconciliation only when machinesQuery.isSuccess is true. Preserve the existing clearing and offline-error behavior after a successful fetch, but do not clear the selected target or set an error while the machines request is pending.Source: Learnings
autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts-194-218 (1)
194-218: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winSurface the backend detail in the 409 path. The reconnect toast always overwrites the
ApiErrorpayload with fixed copy; prefererror.response?.detail, thenerror.message, then the generic fallback so server-provided context isn’t lost.autogpt_platform/frontend/src/app/(platform)/copilot/useChatSession.ts:194-218🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/useChatSession.ts around lines 194 - 218, Update the 409 local execution-target handling in the chat session error path to derive the reconnect message from error.response?.detail first, then error.message, with the existing generic text as fallback. Reuse this resolved message for setExecutionTargetError and the reconnect toast while preserving the current target reset and picker behavior.Source: Learnings
autogpt_platform/frontend/src/app/(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalFolderPicker/useLocalFolderPicker.ts-84-96 (1)
84-96: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve structured API error details for non-stale failures.
After handling status 409, prefer
ApiError.response?.detail, thenerror.message, before the generic fallback. The current branch hides actionable backend errors.Based on learnings, Copilot Orval error handling should explicitly handle
ApiError, preferresponse.detail, thenerror.message, and finally a generic message.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/EmptySession/components/ExecutionTargetPicker/components/LocalFolderPicker/useLocalFolderPicker.ts around lines 84 - 96, Update handleRequestError for non-409 failures to preserve structured ApiError details: prefer ApiError.response?.detail, then the error message, and finally the existing generic fallback. Keep the current status-409 stale-session handling unchanged.Source: Learnings
autogpt_platform/frontend/src/services/feature-flags/__tests__/envFlagOverride.test.ts-87-108 (1)
87-108: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRestore environment variables after every test.
beforeEachprotects these tests from earlier state but the final case leaks its override into later suites in the same worker. Reuse a cleanup helper from bothbeforeEachandafterEach.Proposed cleanup
+function clearLocalPCOverrides() { + delete process.env["NEXT_PUBLIC_FORCE_FLAG_LOCAL_PC_EXECUTOR"]; + delete process.env["NEXT_PUBLIC_FORCE_FLAG_WORKFLOW_RECORDING"]; +} + beforeEach(() => { - delete process.env["NEXT_PUBLIC_FORCE_FLAG_LOCAL_PC_EXECUTOR"]; - delete process.env["NEXT_PUBLIC_FORCE_FLAG_WORKFLOW_RECORDING"]; + clearLocalPCOverrides(); }); + +afterEach(clearLocalPCOverrides);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/services/feature-flags/__tests__/envFlagOverride.test.ts` around lines 87 - 108, Update the “Local PC feature overrides” test suite to define a shared environment-variable cleanup helper, then invoke it from both beforeEach and afterEach. Ensure both NEXT_PUBLIC_FORCE_FLAG_LOCAL_PC_EXECUTOR and NEXT_PUBLIC_FORCE_FLAG_WORKFLOW_RECORDING are removed after every test so overrides do not leak into later suites.autogpt_platform/frontend/src/app/(platform)/copilot/components/LocalPCBadge/LocalPCBadge.tsx-26-40 (1)
26-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDo not render stale executor data as connected after a polling failure.
React Query may retain the previous shim response when a refetch errors. In that state, the label says “status unavailable,” but
connectedremains true, leaving the badge green and showing connected-only details. DeriveconnectedfromisSuccessas well.Proposed fix
- const { data: executor, isError, isLoading } = useLocalPCExecutor(sessionID); + const { + data: executor, + isError, + isLoading, + isSuccess, + } = useLocalPCExecutor(sessionID); - const connected = executor?.kind === "shim"; + const connected = isSuccess && executor?.kind === "shim";Based on learnings, query-derived UI should use
isSuccessrather than treating retained data as successfully loaded.Also applies to: 58-61, 83-84
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/components/LocalPCBadge/LocalPCBadge.tsx around lines 26 - 40, Update the connected state in LocalPCBadge using the useLocalPCExecutor result so it requires both executor?.kind === "shim" and isSuccess; ensure retained executor data cannot render connected-only status after a polling error. Apply the same success-state guard to the related status and styling logic referenced by the comment.Source: Learnings
autogpt_platform/frontend/src/app/(platform)/copilot/hooks/useLocalPCExecutor.ts-8-14 (1)
8-14: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winDrop the local
LocalPCExecutorStatusoverride inuseLocalPCExecutor
ExecutorStatusalready includesrecording_routesandrecording_channelsas nullable arrays, so the custom alias andas LocalPCExecutorStatuscast only weaken type safety. Use the generated model directly.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/copilot/hooks/useLocalPCExecutor.ts around lines 8 - 14, Remove the custom LocalPCExecutorStatus alias and update useLocalPCExecutor to use the generated ExecutorStatus type directly. Delete any related as LocalPCExecutorStatus casts while preserving the existing handling of recording_routes and recording_channels.Source: Learnings
autogpt_platform/frontend/src/app/api/openapi.json-8962-8983 (1)
8962-8983: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winTighten the PKCE/token schema
code_verifiershould carry the RFC 7636 43–128 unreserved-character constraint, not just a max length, so generated clients and docs match the backend contract. UseSecretStr/writeOnly typing for the secret-bearing fields too.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/api/openapi.json` around lines 8962 - 8983, Update the token schema fields around code_verifier to enforce the RFC 7636 constraint: require a 43–128 character value containing only unreserved characters. Mark code_verifier and other secret-bearing fields such as client_secret with SecretStr/writeOnly typing, while preserving the existing titles and descriptions.Source: Learnings
autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py-117-124 (1)
117-124: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRemove unsupported recovery instructions.
local_pc_list_windowslists GUI windows, not directory entries, and_FilesProxy.read/writedo not support the suggestedoffset+lengtharguments. Point path recovery toFILE_LIST; either implement chunked file RPCs or recommend only currently supported alternatives.Also applies to: 276-290
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_errors.py` around lines 117 - 124, Update _path_not_found and the related recovery message around the _FilesProxy read/write handling to remove unsupported local_pc_list_windows and offset+length guidance. Recommend FILE_LIST and only alternatives currently supported by the file RPCs; do not suggest chunked offset/length operations unless those RPCs are implemented.autogpt_platform/backend/backend/copilot/tools/local_llm_router.py-3-7 (1)
3-7: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winCorrect the Local LLM privacy claim.
Inference runs locally, but prompts and responses still transit the platform backend and WebSocket relay. Saying they “never leave the user's machine” overstates the privacy guarantee.
Proposed wording
-``LOCAL_LLM_COMPLETION`` wire op instead of Anthropic / OpenRouter. The -prompt + response never leave the user's machine. +``LOCAL_LLM_COMPLETION`` wire op instead of Anthropic / OpenRouter. Model +inference runs locally, while prompt and response data still transit the +platform relay.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_llm_router.py` around lines 3 - 7, Update the Local LLM routing description to remove the claim that prompts and responses never leave the user’s machine; explicitly state that inference runs on the local shim while prompt and response data still transit the platform backend and WebSocket relay.autogpt_platform/backend/backend/copilot/tools/local_pc_shim_test.py-1424-1429 (1)
1424-1429: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winMove
loggingto the top-level imports.This is not a lazy import of a heavy optional dependency.
Proposed fix
import json +import logging from unittest.mock import AsyncMock, MagicMock ... - import logging - with caplog.at_level(As per coding guidelines, “Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/tools/local_pc_shim_test.py` around lines 1424 - 1429, Move the logging import from inside test_status_with_partial_fields_still_logs to the module’s top-level imports, keeping the existing caplog usage and logger behavior unchanged.Source: Coding guidelines
autogpt_platform/backend/backend/copilot/sdk/recording_tools_test.py-386-394 (1)
386-394: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winCover the prohibited single-row dry run.
The current test only checks
[], so it passes while a one-row replay is incorrectly accepted. Add a one-row case expectingINVALID_ARGUMENT.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/copilot/sdk/recording_tools_test.py` around lines 386 - 394, Extend test_dry_run_requires_rows to invoke _h_dry_run_skill with exactly one data row and assert the response is an error with code INVALID_ARGUMENT, alongside the existing empty-row case.autogpt_platform/backend/backend/api/features/oauth.py-587-588 (1)
587-588: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle malformed JSON as a client error.
Because the endpoint parses
Requestmanually, invalid JSON raises before Pydantic validation and can become a 500 response. Catch decoding errors and convert them to the same sanitized validation response.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/backend/backend/api/features/oauth.py` around lines 587 - 588, Update the JSON parsing branch in the request-handling function to catch malformed JSON decoding errors from http_request.json(). Convert them into the endpoint’s existing sanitized validation response, matching the response used for other client validation failures instead of allowing a 500 error.autogpt_platform/frontend/src/app/(platform)/auth/authorize/page.tsx-128-133 (1)
128-133: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winCapture authorization errors in Sentry.
This catch only logs to the console; callSentry.captureException(err)before showing the user-facing error. The approval handler has the same gap.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@autogpt_platform/frontend/src/app/`(platform)/auth/authorize/page.tsx around lines 128 - 133, Update the authorization denial catch block to call Sentry.captureException(err) before setting the user-facing error, and make the same change in the approval handler’s catch block. Preserve the existing console logging, error-message selection, and loading-state reset.Sources: Coding guidelines, Learnings
github-actions
Bot
added
the
conflicts
|
This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request. |
|
/dev-review |
|
I couldn't load the latest pull request details from GitHub, so the review was not queued: Failed to load pull request diff: Client error '406 Not Acceptable' for url 'https://api.github.com/repos/Significant-Gravitas/AutoGPT/pulls/13050' |
4 participants
Why / What / How
Copilot chats currently execute in the cloud, which means they cannot work directly in a repository or folder on a user's own computer. A Local PC target also needs to be selectable from a remote browser or phone: the browser's native folder picker can only see the device running the browser, not the connected computer.
This PR adds an explicit execution target to new chats. Cloud remains the default. When Local PC is selected, the user can follow inline setup instructions, choose one of their connected machines, and remotely browse that machine's folders before creating the chat.
The installed
autogpt-local-executoropens an owner-scoped outbound control WebSocket to the platform. Directory browsing is relayed over that connection with opaque browse and directory references; selecting a directory returns a signed root grant and resolved allowed root for the chat. Chat execution then uses a session data channel bound to that machine, connection, and root. Missing, stale, revoked, or mismatched Local PC state fails closed and never falls back to a cloud executor.The standalone executor is maintained separately at Significant-Gravitas/autogpt-local-executor. This PR contains the platform integration, UI, protocol boundary, OAuth support, and tests; it does not embed a second copy of that daemon.
Changes 🏗️
--enable-recording, so live capture remains design-preview rather than a shipping runtime capability.Read,Grep, and peers) to their MCP equivalents so denied or unavailable tools cannot surface as fake permission prompts.Configuration
CHAT_USE_LOCAL_PC_EXECUTOR=trueenables the backend deployment kill switch.local-pc-executorfeature flag enables the user-facing rollout; it remains off by default.CHAT_ALLOW_COMPUTER_USE=truepermits the separate computer-use consent path.FORCE_FLAG_LOCAL_PC_EXECUTOR=trueandNEXT_PUBLIC_FORCE_FLAG_LOCAL_PC_EXECUTOR=true.autogpt-local-executormust be active, public, allowUSE_TOOLS, and register loopback callbacks on ports41899through41910.Checklist 📋
For code changes:
For configuration changes:
.env.defaultis updated or already compatible with my changesdocker-compose.ymlis updated or already compatible with my changesSecurity and review notes
This feature crosses an intentional trust boundary: an opted-in chat can operate inside a user-selected root on their computer. Rollout is therefore default-off, doubly gated, owner-scoped, capability-scoped, and fail-closed. OAuth, WebSocket ownership, directory grants, revocation, path handling, metadata/log redaction, and computer-use consent deserve explicit human security review before production enablement.
Note
High Risk
Introduces a user-opt-in trust boundary (local filesystem/shell/computer use), new OAuth and WebSocket auth paths, and session binding/revocation logic that must fail closed.
Overview
Copilot session creation now accepts an execution target (
cloudby default, orlocalwith machine, connection, browse, and directory refs). Local creation validates rollout gates, binds the chat to a connected executor (attach → persist metadata → activate → verify the session data channel), then detaches the validation child; failures run compensated detach/delete. Session APIs expose redactedPublicChatSessionMetadata, and deleting a local session detaches the machine and closes shim channels.A new
local_executorsurface adds owner-scoped HTTP and WebSocket routes: list machines, remote directory browse, per-session executor status, machine- and feature-scoped computer-use consent (Redis), and workflow recording lifecycle with shared Redis state. WebSockets authenticate OAuth bearer tokens (no query tokens), negotiateHELLO/ protocol 1.1, and support both persistent machine-control and per-session data channels.OAuth gains PKCE public-client flows (form and JSON token requests, public refresh/revoke without secret), an authorize deny endpoint, refresh-family revocation that pushes
SESSION_REVOKEDto connected shims, and safer validation error handling that avoids leaking credentials.Reviewed by Cursor Bugbot for commit e9d1ed5. Bugbot is set up for automated code reviews on this repo. Configure here.