mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
fix(copilot): address all review findings
- prompting: rename _SDK_TOOL_NOTES → _E2B_TOOL_NOTES; pass it only to _get_cloud_sandbox_supplement() via new extra_notes param — local (bubblewrap) mode uses --unshare-net so gh CLI cannot reach GitHub - integration_creds: cache None results with 60 s TTL (_NULL_CACHE_TTL) to avoid a DB hit on every E2B bash_exec for users without GitHub creds; found tokens still cached for 5 min (_TOKEN_CACHE_TTL) - connect_integration: add cross-reference comment to PROVIDER_ENV_VARS - ConnectIntegrationTool: use provider-specific credentialsLabel (e.g. "GitHub credentials" instead of "Integration credentials")
This commit is contained in:
@@ -4,8 +4,12 @@ Provides token retrieval for connected integrations so that copilot tools
|
||||
(e.g. bash_exec) can inject auth tokens into the execution environment without
|
||||
hitting the database on every command.
|
||||
|
||||
Only non-None tokens are cached — a user who just connected an account will
|
||||
have their token picked up on the very next command, with no TTL wait.
|
||||
Cache semantics:
|
||||
- Token found → cached for _TOKEN_CACHE_TTL (5 min). Avoids repeated DB hits
|
||||
for users who have credentials and are running many bash commands.
|
||||
- No credentials found → cached for _NULL_CACHE_TTL (60 s). Avoids a DB hit
|
||||
on every E2B command for users who haven't connected an account yet, while
|
||||
still picking up a newly-connected account within one minute.
|
||||
"""
|
||||
|
||||
import logging
|
||||
@@ -19,12 +23,16 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
# Maps provider slug → env var names to inject when the provider is connected.
|
||||
# Add new providers here when adding integration support.
|
||||
# NOTE: keep in sync with connect_integration._PROVIDER_INFO — both registries
|
||||
# must be updated when adding a new provider.
|
||||
PROVIDER_ENV_VARS: dict[str, list[str]] = {
|
||||
"github": ["GH_TOKEN", "GITHUB_TOKEN"],
|
||||
}
|
||||
|
||||
_TOKEN_CACHE_TTL = 300.0 # seconds
|
||||
# (user_id, provider) → (token, monotonic expiry)
|
||||
_TOKEN_CACHE_TTL = 300.0 # seconds — for found tokens
|
||||
_NULL_CACHE_TTL = 60.0 # seconds — for "not connected" results
|
||||
_SENTINEL = "" # cached value meaning "no credentials found"
|
||||
# (user_id, provider) → (token_or_sentinel, monotonic expiry)
|
||||
_token_cache: dict[tuple[str, str], tuple[str, float]] = {}
|
||||
|
||||
|
||||
@@ -32,8 +40,10 @@ async def get_provider_token(user_id: str, provider: str) -> str | None:
|
||||
"""Return the user's access token for *provider*, or ``None`` if not connected.
|
||||
|
||||
OAuth2 tokens are preferred (refreshed if needed); API keys are the fallback.
|
||||
Results are cached per ``(user_id, provider)`` for :data:`_TOKEN_CACHE_TTL`
|
||||
seconds so repeated calls within a session do not hit the database.
|
||||
Found tokens are cached for _TOKEN_CACHE_TTL (5 min). "Not connected" results
|
||||
are cached for _NULL_CACHE_TTL (60 s) to avoid a DB hit on every bash_exec
|
||||
command for users who haven't connected yet, while still picking up a
|
||||
newly-connected account within one minute.
|
||||
"""
|
||||
now = time.monotonic()
|
||||
cache_key = (user_id, provider)
|
||||
@@ -41,7 +51,7 @@ async def get_provider_token(user_id: str, provider: str) -> str | None:
|
||||
if cached is not None:
|
||||
token, expires_at = cached
|
||||
if now < expires_at:
|
||||
return token
|
||||
return token or None # _SENTINEL ("") → None
|
||||
del _token_cache[cache_key]
|
||||
|
||||
manager = IntegrationCredentialsManager()
|
||||
@@ -75,6 +85,8 @@ async def get_provider_token(user_id: str, provider: str) -> str | None:
|
||||
_token_cache[cache_key] = (token, now + _TOKEN_CACHE_TTL)
|
||||
return token
|
||||
|
||||
# No credentials found — cache the sentinel to avoid repeated DB hits.
|
||||
_token_cache[cache_key] = (_SENTINEL, now + _NULL_CACHE_TTL)
|
||||
return None
|
||||
|
||||
|
||||
|
||||
@@ -63,8 +63,9 @@ an @@agptfile: expansion), the string will be parsed into the correct type.
|
||||
All tasks must run in the foreground.
|
||||
"""
|
||||
|
||||
# SDK-only notes — not shown in baseline mode, which has no subprocess or gh CLI.
|
||||
_SDK_TOOL_NOTES = """
|
||||
# E2B-only notes — E2B has full internet access so gh CLI works there.
|
||||
# Not shown in local (bubblewrap) mode: --unshare-net blocks all network.
|
||||
_E2B_TOOL_NOTES = """
|
||||
### GitHub CLI (`gh`)
|
||||
- If the user has connected their GitHub account, `GH_TOKEN` is automatically
|
||||
set in the environment — `gh` CLI commands work without any login step.
|
||||
@@ -84,6 +85,7 @@ def _build_storage_supplement(
|
||||
storage_system_1_persistence: list[str],
|
||||
file_move_name_1_to_2: str,
|
||||
file_move_name_2_to_1: str,
|
||||
extra_notes: str = "",
|
||||
) -> str:
|
||||
"""Build storage/filesystem supplement for a specific environment.
|
||||
|
||||
@@ -98,6 +100,7 @@ def _build_storage_supplement(
|
||||
storage_system_1_persistence: List of persistence behavior descriptions
|
||||
file_move_name_1_to_2: Direction label for primary→persistent
|
||||
file_move_name_2_to_1: Direction label for persistent→primary
|
||||
extra_notes: Environment-specific notes appended after shared notes
|
||||
"""
|
||||
# Format lists as bullet points with proper indentation
|
||||
characteristics = "\n".join(f" - {c}" for c in storage_system_1_characteristics)
|
||||
@@ -131,12 +134,16 @@ def _build_storage_supplement(
|
||||
|
||||
### File persistence
|
||||
Important files (code, configs, outputs) should be saved to workspace to ensure they persist.
|
||||
{_SHARED_TOOL_NOTES}{_SDK_TOOL_NOTES}"""
|
||||
{_SHARED_TOOL_NOTES}{extra_notes}"""
|
||||
|
||||
|
||||
# Pre-built supplements for common environments
|
||||
def _get_local_storage_supplement(cwd: str) -> str:
|
||||
"""Local ephemeral storage (files lost between turns)."""
|
||||
"""Local ephemeral storage (files lost between turns).
|
||||
|
||||
Network is isolated (bubblewrap --unshare-net), so internet-dependent CLIs
|
||||
like gh will not work — no integration env-var notes are included.
|
||||
"""
|
||||
return _build_storage_supplement(
|
||||
working_dir=cwd,
|
||||
sandbox_type="in a network-isolated sandbox",
|
||||
@@ -154,7 +161,11 @@ def _get_local_storage_supplement(cwd: str) -> str:
|
||||
|
||||
|
||||
def _get_cloud_sandbox_supplement() -> str:
|
||||
"""Cloud persistent sandbox (files survive across turns in session)."""
|
||||
"""Cloud persistent sandbox (files survive across turns in session).
|
||||
|
||||
E2B has full internet access, so integration tokens (GH_TOKEN etc.) are
|
||||
injected per command in bash_exec — include the CLI guidance notes.
|
||||
"""
|
||||
return _build_storage_supplement(
|
||||
working_dir="/home/user",
|
||||
sandbox_type="in a cloud sandbox with full internet access",
|
||||
@@ -169,6 +180,7 @@ def _get_cloud_sandbox_supplement() -> str:
|
||||
],
|
||||
file_move_name_1_to_2="Sandbox → Persistent",
|
||||
file_move_name_2_to_1="Persistent → Sandbox",
|
||||
extra_notes=_E2B_TOOL_NOTES,
|
||||
)
|
||||
|
||||
|
||||
|
||||
@@ -21,8 +21,9 @@ from backend.copilot.tools.models import (
|
||||
|
||||
from .base import BaseTool
|
||||
|
||||
# Registry of known providers: name + supported credential types.
|
||||
# Extend this dict when adding support for new integrations.
|
||||
# Registry of known providers: name + supported credential types for the UI.
|
||||
# When adding a new provider, also add its env var names to
|
||||
# backend.copilot.integration_creds.PROVIDER_ENV_VARS.
|
||||
_PROVIDER_INFO: dict[str, dict[str, Any]] = {
|
||||
"github": {
|
||||
"name": "GitHub",
|
||||
|
||||
@@ -54,7 +54,7 @@ export function ConnectIntegrationTool({ part }: Props) {
|
||||
<div className="mt-2">
|
||||
<SetupRequirementsCard
|
||||
output={output}
|
||||
credentialsLabel="Integration credentials"
|
||||
credentialsLabel={`${output.setup_info?.agent_name ?? providerName} credentials`}
|
||||
retryInstruction="I've connected my account. Please continue."
|
||||
/>
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user