Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP - add library and presets backend #9357

Open
wants to merge 39 commits into
base: dev
Choose a base branch
from

Conversation

Swiftyos
Copy link
Contributor

@Swiftyos Swiftyos commented Jan 29, 2025

Getting full CI to run so I have a full idea of all issues.

Changes 🏗️

Checklist 📋

For code changes:

  • I have clearly listed my changes in the PR description
  • I have made a test plan
  • I have tested my changes according to the test plan:
    • ...
Example test plan
  • Create from scratch and execute an agent with at least 3 blocks
  • Import an agent from file upload, and confirm it executes correctly
  • Upload agent to marketplace
  • Import an agent from marketplace and confirm it executes correctly
  • Edit an agent from monitor, and confirm it executes correctly

For configuration changes:

  • .env.example is updated or already compatible with my changes
  • docker-compose.yml is updated or already compatible with my changes
  • I have included a list of my configuration changes in the PR description (under Changes)
Examples of configuration changes
  • Changing ports
  • Adding new services that need to communicate with each other
  • Secrets or environment variable changes
  • New or infrastructure changes such as databases

Swiftyos and others added 26 commits January 3, 2025 11:26
…without-agent-ownership' of github.com:Significant-Gravitas/AutoGPT into swiftyos/open-2276-add-ability-to-execute-store-agents-without-agent-ownership
…without-agent-ownership' into swiftyos/open-2277-implement-library-add-update-remove-archive-functionality
…archive-functionality' into swiftyos/open-2278-implement-agent-preset-functionality
@Swiftyos Swiftyos requested review from a team as code owners January 29, 2025 08:04
@Swiftyos Swiftyos requested review from Bentlybro and removed request for a team January 29, 2025 08:04
Copy link

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 PR contains tests
🔒 Security concerns

Sensitive information exposure:
The code includes handling of Stripe API keys and payment processing. While the keys themselves are properly handled through environment variables, the code should be reviewed to ensure no payment details or API keys are logged or exposed in error messages or transaction metadata.

⚡ Recommended focus areas for review

Potential Race Condition

The credit transaction system uses locks but could still have edge cases around concurrent transactions and balance updates. The running balance calculation and transaction creation should be carefully reviewed for thread safety.

async def _get_credits(self, user_id: str) -> tuple[int, datetime]:
    """
    Returns the current balance of the user & the latest balance snapshot time.
    """
    top_time = self.time_now()
    snapshot = await CreditTransaction.prisma().find_first(
        where={
            "userId": user_id,
            "createdAt": {"lte": top_time},
            "isActive": True,
            "runningBalance": {"not": None},  # type: ignore
        },
        order={"createdAt": "desc"},
    )
    datetime_min = datetime.min.replace(tzinfo=timezone.utc)
    snapshot_balance = snapshot.runningBalance or 0 if snapshot else 0
    snapshot_time = snapshot.createdAt if snapshot else datetime_min

    # Get transactions after the snapshot, this should not exist, but just in case.
    transactions = await CreditTransaction.prisma().group_by(
        by=["userId"],
        sum={"amount": True},
        max={"createdAt": True},
        where={
            "userId": user_id,
            "createdAt": {
                "gt": snapshot_time,
                "lte": top_time,
            },
            "isActive": True,
        },
    )
    transaction_balance = (
        transactions[0].get("_sum", {}).get("amount", 0) + snapshot_balance
        if transactions
        else snapshot_balance
    )
    transaction_time = (
        transactions[0].get("_max", {}).get("createdAt", datetime_min)
        if transactions
        else snapshot_time
    )
    return transaction_balance, transaction_time
Error Handling

The auto top-up error handling logs errors but continues execution. This could lead to users running out of credits unexpectedly if auto top-up silently fails.

# Auto top-up if balance just went below threshold due to this transaction.
auto_top_up = await get_auto_top_up(user_id)
if balance < auto_top_up.threshold <= balance - cost:
    try:
        await self.top_up_credits(user_id=user_id, amount=auto_top_up.amount)
    except Exception as e:
        # Failed top-up is not critical, we can move on.
        logger.error(
            f"Auto top-up failed for user {user_id}, balance: {balance}, amount: {auto_top_up.amount}, error: {e}"
        )
API Error Handling

The GitHub API calls lack comprehensive error handling for rate limits, authentication failures and other API errors. This could lead to silent failures or unclear error messages.

    check_runs_url = f"{repo_url}/check-runs"
    response = api.post(check_runs_url)
    result = response.json()

    return {
        "id": result["id"],
        "html_url": result["html_url"],
        "status": result["status"],
    }

def run(
    self,
    input_data: Input,
    *,
    credentials: GithubCredentials,
    **kwargs,
) -> BlockOutput:
    try:
        result = self.create_check_run(
            credentials=credentials,
            repo_url=input_data.repo_url,
            name=input_data.name,
            head_sha=input_data.head_sha,
            status=input_data.status,
            conclusion=input_data.conclusion,
            details_url=input_data.details_url,
            output_title=input_data.output_title,
            output_summary=input_data.output_summary,
            output_text=input_data.output_text,
        )
        yield "check_run", result
    except Exception as e:

Copy link

netlify bot commented Jan 29, 2025

Deploy Preview for auto-gpt-docs canceled.

Name Link
🔨 Latest commit 7a14e5d
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs/deploys/67a0d7821b57e00008d99006

Copy link

netlify bot commented Jan 30, 2025

Deploy Preview for auto-gpt-docs-dev canceled.

Name Link
🔨 Latest commit 7a14e5d
🔍 Latest deploy log https://app.netlify.com/sites/auto-gpt-docs-dev/deploys/67a0d782477ce10008d2ff0f

@Swiftyos Swiftyos requested review from Pwuts and Abhi1992002 and removed request for Bentlybro, kcze and a team January 30, 2025 09:36
Copy link

deepsource-io bot commented Jan 30, 2025

Here's the code health analysis summary for commits 7b50e9b..7a14e5d. View details on DeepSource ↗.

Analysis Summary

AnalyzerStatusSummaryLink
DeepSource JavaScript LogoJavaScript✅ Success
❗ 63 occurences introduced
🎯 51 occurences resolved
View Check ↗
DeepSource Python LogoPython✅ Success
❗ 4 occurences introduced
🎯 21 occurences resolved
View Check ↗

💡 If you’re a repository administrator, you can configure the quality gates from the settings.

@github-actions github-actions bot added the platform/frontend AutoGPT Platform - Front end label Jan 30, 2025
@github-actions github-actions bot added the conflicts Automatically applied to PRs with merge conflicts label Jan 30, 2025
Copy link
Contributor

This pull request has conflicts with the base branch, please resolve those so we can evaluate the pull request.

@github-actions github-actions bot removed the conflicts Automatically applied to PRs with merge conflicts label Jan 30, 2025
Copy link
Contributor

Conflicts have been resolved! 🎉 A maintainer will review the pull request shortly.

…' of github.com:Significant-Gravitas/AutoGPT into swiftyos/open-2278-implement-agent-preset-functionality
@Swiftyos Swiftyos requested a review from Bentlybro January 31, 2025 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform/backend AutoGPT Platform - Back end platform/frontend AutoGPT Platform - Front end Review effort [1-5]: 4 size/xl
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants