refactor(backend): plumb ExecutionContext at direct-block-execute boundary#13236
Conversation
18062706139fcz
requested review from
0ubbe and
majdyz
and removed request for
a team
github-project-automation
Bot
moved this to 🆕 Needs initial review
in AutoGPT development kanban
|
/review |
github-actions
Bot
added
platform/backend
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR refactors block execution to require an explicit ChangesDirect block execution context propagation
Sequence DiagramsequenceDiagram
participant Client
participant ExternalRoute as External API Route
participant FeatureRoute as Feature API Route
participant BlockExec as Block.execute
participant BlockInternal as Block._execute
Client->>ExternalRoute: POST /blocks/{id}/execute
ExternalRoute->>ExternalRoute: fetch user
ExternalRoute->>ExternalRoute: build ExecutionContext(user_id, timezone)
ExternalRoute->>BlockExec: execute(input_data, execution_context=ctx)
Client->>FeatureRoute: POST /blocks/{id}/execute
FeatureRoute->>FeatureRoute: build ExecutionContext(user_id, timezone)
FeatureRoute->>BlockExec: execute(input_data, execution_context=ctx)
BlockExec->>BlockInternal: _execute(input_data, execution_context=ctx, **kwargs)
BlockInternal->>BlockInternal: check execution_context.graph_exec_id for review gating
BlockInternal->>BlockInternal: check execution_context.dry_run for simulation
BlockInternal->>BlockInternal: run(..., execution_context=ctx)
BlockInternal-->>BlockExec: output
BlockExec-->>Client: output
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #13236 +/- ##
==========================================
- Coverage 72.38% 72.31% -0.08%
==========================================
Files 2304 2337 +33
Lines 173347 174558 +1211
Branches 17560 17667 +107
==========================================
+ Hits 125484 126238 +754
- Misses 44140 44565 +425
- Partials 3723 3755 +32
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![autogpt-pr-reviewer[bot]](https://avatars.githubusercontent.com/u/130738209?s=60&v=4){kind=small}
There was a problem hiding this comment.
📋 Automated Review — PR #13236
PR #13236 — fix(backend): allow get current date without context
Author: 18062706139fcz | Files: 2
🎯 Verdict: REQUEST_CHANGES
PR Description Quality
What This PR Does
GetCurrentDateBlock.run() required execution_context as a mandatory keyword argument, which caused a crash when the block was invoked without one (e.g., in certain execution paths that don't supply context). This PR makes execution_context optional with a None default, falling back gracefully when absent — the existing _get_timezone() helper already handles user_timezone=None. A new regression test exercises the no-context path.
Specialist Findings
🛡️ Security ✅ — No concerns. The only data extracted from execution_context is user_timezone (a timezone string), which safely falls back to the format's default timezone when absent. No auth bypass, injection, or data exposure vectors.
🏗️ Architecture
🟠 Sibling blocks have the identical bug (time_blocks.py:191, time_blocks.py:413): GetCurrentTimeBlock and GetCurrentDateAndTimeBlock both still require execution_context as mandatory. If the no-context execution path triggers GetCurrentDateBlock, it will logically trigger these blocks too. Fixing only one of three identically-structured blocks creates an inconsistency and guarantees future identical bug reports.
⚡ Performance ✅ — Adds a single O(1) None check on a non-hot code path. No new allocations, loops, or I/O. Zero performance impact.
🧪 Testing
🟠 datetime.strptime() result is never asserted (test_time_blocks.py:18) — format validation is implicit via exception, not explicit assertion. No check that the returned date is actually correct (would pass with "1970-01-01").
🟠 Only the no-context path is tested. The with-context branch (time_blocks.py:308) is not exercised, so a regression in timezone handling would go undetected.
🟡 No coverage for non-zero offset, invalid offset strings, or non-default format types without context.
📖 Quality ✅ — The conditional expression for effective_timezone is clean and idiomatic. Naming is consistent with the codebase. The only concern is the inconsistency with sibling blocks (covered under Architecture).
📦 Product GetCurrentDateBlock, but users hitting the same execution path on GetCurrentDateAndTimeBlock will encounter the identical crash. This is a partial fix from the user's perspective.
📬 Discussion license/cla check pending). Zero human reviews exist on this PR. All PR checklist items remain unchecked. check-docs-sync is failing (likely pre-existing, unrelated to this backend change).
🔎 QA ✅ — QA specialist validated all scenarios directly:
- No-context path returns correct date (
2026-05-28) ✅ - With-context path (America/New_York) preserves existing behavior ✅
- Invalid offset (
"not_a_number") handled gracefully (defaults to 0) ✅ - pytest regression test passes (
1 passed in 1.55s) ✅ - Block still registered in API ✅
🔴 Blockers
- CLA not signed — The
license/clacheck reports the contributor's Git identity is not linked to a GitHub account. This must be resolved before merge. (Flagged by: discussion — 1)
🟠 Should Fix
- Incomplete fix —
GetCurrentTimeBlockstill crashes without context (time_blocks.py:191) — Apply the sameexecution_context: ExecutionContext | None = Nonepattern with the ternary fallback. (Flagged by: architect, quality, product, testing — 4) - Incomplete fix —
GetCurrentDateAndTimeBlockstill crashes without context (time_blocks.py:413) — Same issue, same fix needed. (Flagged by: architect, quality, product, testing — 4) - Test lacks explicit assertion on date correctness (
test_time_blocks.py:18) —datetime.strptime()is called but the result is discarded. Add:assert abs((datetime.now().date() - parsed.date()).days) <= 1. (Flagged by: testing, quality, product — 3) - Missing test for with-context path (
test_time_blocks.py:13) — Add a test that passes a mockExecutionContextwith a knownuser_timezoneto confirm existing behavior is preserved after the signature change. (Flagged by: testing, architect, discussion — 3)
🟡 Nice to Have
- Additional edge-case tests (
test_time_blocks.py:11) — Parametrized tests for non-zero offset, invalid offset strings ("abc"), and non-defaultDateStrftimeFormatwithout context. (testing, architect)
Human Review Needed
YES — First-time contributor with unsigned CLA, zero human reviews, and the fix scope should be validated (whether sibling blocks should be included).
Risk Assessment
Merge risk: LOW | Rollback: EASY
The change itself is safe and backward-compatible — it only relaxes a function signature. However, the incomplete fix across sibling blocks means the underlying issue (#12648) is only partially resolved.
CI Status
❌ 4/6 quality checks failed — frontend typecheck, backend tests, frontend unit tests, and frontend build all failed. However, these failures appear to be pre-existing/environmental (backend test couldn't start, frontend failures are unrelated to this 2-file backend change). Both lint checks passed ✅.
UI Testing — Variant Results
✅ local: GetCurrentDateBlock correctly handles missing execution_context, regression test passes, existing behavior preserved
❌ hosted: Variant 'hosted' failed: 500: Failed to place sandbox
github-project-automation
Bot
moved this from 🆕 Needs initial review
to 🚧 Needs work
in AutoGPT development kanban
github-actions
Bot
added
the
documentation
|
Actionable comments posted: 0 |
github-actions
Bot
added
size/l
and removed
platform/backend
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In `@autogpt_platform/backend/backend/blocks/test/test_time_blocks.py`:
- Around line 69-70: The test currently compares value against
datetime.now(tz=ZoneInfo("America/New_York")).strftime("%Y-%m-%d") which can
fail if the date flips during the assertion; instead capture a before and after
timestamp around the operation (e.g., before = datetime.now(...).strftime(...)
before invoking the code-under-test and after = datetime.now(...).strftime(...)
immediately after) and then assert that value is either before or after
(membership) or that it lies within the inclusive before/after window; update
the assertions that reference key and value in test_time_blocks.py to use these
captured before/after strings rather than a single now() call.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b6a05526-39e1-480e-8261-fad8305d9a4a
📒 Files selected for processing (2)
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
autogpt_platform/backend/backend/blocks/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/backend/blocks/**/*.py: Inherit from 'Block' base class with input/output schemas when adding new blocks in backend
Implement 'run' method with proper error handling in backend blocks
Generate block UUID using 'uuid.uuid4()' when creating new blocks in backend
Write tests alongside block implementation when adding new blocks in backend
autogpt_platform/backend/backend/blocks/**/*.py: For blocks handling files, usestore_media_file()withreturn_format="for_local_processing"when processing with local tools (ffmpeg, MoviePy, PIL)
For blocks handling files, usestore_media_file()withreturn_format="for_external_api"when sending content to external APIs (Replicate, OpenAI)
For blocks returning files, usestore_media_file()withreturn_format="for_block_output"to enable auto-adaptation to execution context (workspace:// in CoPilot, data URI in graphs)
When creating new blocks, inherit fromBlockbase class, define input/output schemas usingBlockSchema, implement asyncrunmethod, and generate unique block ID usinguuid.uuid4()
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
autogpt_platform/backend/**/test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
autogpt_platform/backend/**/test_*.py
📄 CodeRabbit inference engine (autogpt_platform/AGENTS.md)
Create a failing test first using
@pytest.mark.xfaildecorator (backend) when fixing a bug or adding a feature, then implement the fix and remove the xfail marker
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py
🧠 Learnings (17)
📚 Learning: 2026-02-05T04:11:00.596Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11796
File: autogpt_platform/backend/backend/blocks/video/concat.py:3-4
Timestamp: 2026-02-05T04:11:00.596Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, when creating a new block, generate a UUID once with uuid.uuid4() and hard-code the resulting string as the block's id parameter. Do not call uuid.uuid4() at runtime; IDs must be constant across all imports and runs to ensure stability.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-16T16:32:21.686Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:32:21.686Z
Learning: In autogpt_platform/backend/backend/blocks/, the Block base class execute() already wraps run() in a try/except to convert uncaught exceptions into BlockExecutionError/BlockUnknownError. Do not add per-block try/except in individual block run() methods, as this is not the established pattern (e.g., Gmail, Slack, Todoist blocks omit it). Only use explicit try/except within blocks that need to distinguish between success and error yield paths inside a generator (e.g., attachment blocks). This guidance applies to all Python files under autogpt_platform/backend/backend/blocks/ and similar block implementations; avoid duplicating error handling in run() unless a block requires generator-based branching.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-04-23T12:55:26.122Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12893
File: autogpt_platform/backend/backend/blocks/ayrshare/post_to_tiktok.py:24-24
Timestamp: 2026-04-23T12:55:26.122Z
Learning: Cost billing via the cost(*costs) decorator is applied at input-evaluation time (before a block’s run() executes). Therefore, mutating input_data inside run() will not change billing. When a block’s billing depends on a field plus URL/sniff-derived signals, treat the explicitly declared billing field (e.g., is_video) as the only billing source—set it correctly before run() (or in the code path that occurs before the decorator evaluates input_data). This should be checked for all blocks under autogpt_platform/backend/backend/blocks/ so billing signals are not mistakenly assumed to update during run().
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: In autogpt_platform/backend/backend/blocks/ (and related blocks under autogpt_platform/backend/backend/blocks/), do not add try/except blocks around a block's run() method for standard error propagation. The block executor framework (backend/executor/manager.py) catches uncaught exceptions from run() and emits them on the 'error' output. Only add explicit try/except blocks when you need to control partial outputs in failure cases (e.g., certain outputs must not be yielded on error, as in attachment blocks). This is the standard pattern across the codebase; apply it broadly to blocks' run() implementations.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-16T16:30:23.196Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:30:23.196Z
Learning: In any Python file under autogpt_platform/backend/backend/blocks, do not add a try/except around run() solely for standard error handling. The block framework’s _execute() in _base.py already catches unhandled exceptions and re-raises as BlockExecutionError or BlockUnknownError. If you yield ("error", message), _execute() raises BlockExecutionError immediately, so the error port will not propagate downstream. Reserve explicit try/except for scenarios where you must control partial output (e.g., attachment blocks that must skip yielding content_base64 on failure).
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: Do not wrap synchronous AgentMail SDK calls with asyncio.to_thread() in blocks under autogpt_platform/backend/backend/blocks (and across the codebase). The block executor runs node execution in dedicated threads via asyncio.run_coroutine_threadsafe (see manager.py around lines ~745-752 and ~1079). The existing pattern avoids using asyncio.to_thread for SDK calls inside async run() methods, so maintain that approach and do not add to_thread usage in these code paths.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-03-19T15:10:50.676Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:50.676Z
Learning: When using Python’s `unittest.mock.patch` in tests, choose the patch target based on how the imported name is resolved:
- If the code under test uses an **eager/module-level import** (e.g., `from foo.bar import baz` at module top), patch **the module where the name is looked up** (i.e., where it is used in the SUT), e.g. `patch("mymodule.baz")`.
- If the code under test uses a **lazy import** executed later (e.g., `from foo.bar import baz` inside a function/branch), patch **the source module** (e.g., `patch("foo.bar.baz")`) because the late `from ... import` will read the (potentially patched) name from the source module at call time.
For a concrete example: if `simulate_block` is imported inside an `if dry_run:` block in the SUT, then the correct test patch target is the source module path for `simulate_block` as it exists at call time (e.g., `patch("backend.executor.simulator.simulate_block")`), not the test file’s import location.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/blocks/time_blocks.py
🔇 Additional comments (2)
autogpt_platform/backend/backend/blocks/time_blocks.py (1)
191-199: LGTM!Also applies to: 419-427
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py (1)
1-67: LGTM!Also applies to: 73-103
|
Hey @18062706139fcz — thanks for the contribution! The PR is otherwise in good shape but the CLA check is currently failing. Could you sign the CLA when you have a moment? It's a one-time step (the bot link should be in the failing check above) and unblocks the merge. Appreciated! |
18062706139fcz
force-pushed
the
codex/open-12648-getcurrentdate-no-context
branch
from
64db1aa to
4ca0675
Compare
@majdyz Thanks for the reminder! The CLA check didn’t go through earlier due to an account issue when I clicked the link, but it has been fixed now and the CLA should be signed successfully. Could you please check again and see if the PR can be merged now? Thanks! |
…ndary Fixes the root cause of Significant-Gravitas#12648: blocks that read ``execution_context`` (time blocks, basic, text, http, autopilot, …) crash on the direct block-execute REST routes because those routes never construct one. Instead of patching individual blocks to make ``execution_context`` optional, fix the boundary: - Make ``execution_context`` an explicit required keyword on ``Block.execute()``/``Block._execute()`` so missing-context callers surface at the type system, not as a runtime TypeError deep inside a block's ``run()``. - Build a real ``ExecutionContext`` (carrying the caller's ``user_id`` + ``user_timezone``) inside the internal and external ``POST /blocks/{id}/execute`` routes and forward it to ``Block.execute()``. - Gate the HITL-review path on ``execution_context.graph_exec_id`` instead of probing kwarg presence — direct block execution has no graph and never needs review. Regression tests: - ``test_execute_graph_block_forwards_execution_context`` (internal) and ``test_execute_block_forwards_execution_context`` (external) assert the route passes an ``ExecutionContext`` carrying the caller's user_id + timezone to the block. - Time-block tests cover both empty-context and user-timezone paths end-to-end through ``block.run``.
majdyz
changed the title
|
Hey @18062706139fcz — thanks for the PR. After digging in I think the fix as written was hitting the wrong layer, so I pushed a follow-up commit to the same branch (maintainer edits are on). Sharing the reasoning here so it's transparent: Why per-block was the wrong layer
So patching the three time blocks fixes the headline bug in #12648 but leaves every other block crashing the same way through those two routes. What I changed
PR title and description updated to reflect the new scope. Let me know if you'd rather I rolled this back and you take it from here — happy to do either. Thanks again for catching the underlying issue. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
autogpt_platform/backend/backend/api/external/v1/routes.py (1)
108-127:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the user lookup ahead of billing.
This route charges credits before Lines 117-119 verify that
auth.user_idstill resolves to a user. If the API key points at a deleted/missing user, the request can debit credits and then return 404.Suggested fix
- try: - await charge_for_direct_block_execution( - user_id=auth.user_id, block=obj, input_data=data, source="external" - ) - except InsufficientBalanceError as e: - raise HTTPException( - status_code=status.HTTP_402_PAYMENT_REQUIRED, detail=str(e) - ) from e - user = await user_db.get_user_by_id(auth.user_id) if not user: raise HTTPException(status_code=404, detail="User not found.") + try: + await charge_for_direct_block_execution( + user_id=auth.user_id, block=obj, input_data=data, source="external" + ) + except InsufficientBalanceError as e: + raise HTTPException( + status_code=status.HTTP_402_PAYMENT_REQUIRED, detail=str(e) + ) from e + # Direct block execution has no graph; build a minimal ExecutionContext🤖 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/external/v1/routes.py` around lines 108 - 127, Move the user existence check to before charging: call user_db.get_user_by_id(auth.user_id) and validate the user (raise HTTPException 404 if missing) prior to invoking charge_for_direct_block_execution; keep the InsufficientBalanceError handling around charge_for_direct_block_execution and then continue to construct the ExecutionContext (user_id and get_user_timezone_or_utc(user.timezone)) once the user is confirmed. Ensure you reference auth.user_id, user_db.get_user_by_id, charge_for_direct_block_execution, InsufficientBalanceError, and ExecutionContext when relocating the lookup.
🤖 Prompt for all review comments with 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.
Outside diff comments:
In `@autogpt_platform/backend/backend/api/external/v1/routes.py`:
- Around line 108-127: Move the user existence check to before charging: call
user_db.get_user_by_id(auth.user_id) and validate the user (raise HTTPException
404 if missing) prior to invoking charge_for_direct_block_execution; keep the
InsufficientBalanceError handling around charge_for_direct_block_execution and
then continue to construct the ExecutionContext (user_id and
get_user_timezone_or_utc(user.timezone)) once the user is confirmed. Ensure you
reference auth.user_id, user_db.get_user_by_id,
charge_for_direct_block_execution, InsufficientBalanceError, and
ExecutionContext when relocating the lookup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 69c232e6-ff07-4300-8c86-bd3cd1d94973
📒 Files selected for processing (7)
autogpt_platform/backend/backend/api/external/v1/routes.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: check API types
- GitHub Check: Check PR Status
- GitHub Check: type-check (3.11)
- GitHub Check: test (3.13)
- GitHub Check: type-check (3.13)
- GitHub Check: test (3.12)
- GitHub Check: type-check (3.12)
- GitHub Check: test (3.11)
- GitHub Check: check-docs-sync
- GitHub Check: end-to-end tests
- GitHub Check: Analyze (python)
- GitHub Check: Analyze (typescript)
🧰 Additional context used
📓 Path-based instructions (8)
autogpt_platform/backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/**/*.py: Use Python 3.11 (required; managed by Poetry via pyproject.toml) for backend development
Always run 'poetry run format' (Black + isort) before linting in backend development
Always run 'poetry run lint' (ruff) after formatting in backend development
autogpt_platform/backend/**/*.py: Usepoetry run ...command for executing Python package dependencies
Use top-level imports only — avoid local/inner imports except for lazy imports of heavy optional dependencies likeopenpyxl
Use absolute imports withfrom backend.module import ...for cross-package imports; single-dot relative imports are acceptable for sibling modules within the same package; avoid double-dot relative imports
Do not use duck typing — avoidhasattr/getattr/isinstancefor type dispatch; use typed interfaces/unions/protocols instead
Use Pydantic models over dataclass/namedtuple/dict for structured data
Do not use linter suppressors — no# type: ignore,# noqa,# pyright: ignore; fix the type/code instead
Prefer list comprehensions over manual loop-and-append patterns
Use early return with guard clauses first to avoid deep nesting
Use%sfor deferred interpolation indebuglog statements for efficiency; use f-strings elsewhere for readability (e.g.,logger.debug("Processing %s items", count)vslogger.info(f"Processing {count} items"))
Sanitize error paths by usingos.path.basename()in error messages to avoid leaking directory structure
Be aware of TOCTOU (Time-Of-Check-Time-Of-Use) issues — avoid check-then-act patterns for file access and credit charging
Usetransaction=Truefor Redis pipelines to ensure atomicity on multi-step operations
Usemax(0, value)guards for computed values that should never be negative
Keep files under ~300 lines; if a file grows beyond this, split by responsibility (extract helpers, models, or a sub-module into a new file)
Keep functions under ~40 lines; extract named helpers when a function grows longer
...
Files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
autogpt_platform/backend/backend/blocks/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
autogpt_platform/backend/backend/blocks/**/*.py: Inherit from 'Block' base class with input/output schemas when adding new blocks in backend
Implement 'run' method with proper error handling in backend blocks
Generate block UUID using 'uuid.uuid4()' when creating new blocks in backend
Write tests alongside block implementation when adding new blocks in backend
autogpt_platform/backend/backend/blocks/**/*.py: For blocks handling files, usestore_media_file()withreturn_format="for_local_processing"when processing with local tools (ffmpeg, MoviePy, PIL)
For blocks handling files, usestore_media_file()withreturn_format="for_external_api"when sending content to external APIs (Replicate, OpenAI)
For blocks returning files, usestore_media_file()withreturn_format="for_block_output"to enable auto-adaptation to execution context (workspace:// in CoPilot, data URI in graphs)
When creating new blocks, inherit fromBlockbase class, define input/output schemas usingBlockSchema, implement asyncrunmethod, and generate unique block ID usinguuid.uuid4()
Files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
autogpt_platform/{backend,autogpt_libs}/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Format Python code with
poetry run format
Files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
autogpt_platform/backend/**/test_*.py
📄 CodeRabbit inference engine (autogpt_platform/AGENTS.md)
Create a failing test first using
@pytest.mark.xfaildecorator (backend) when fixing a bug or adding a feature, then implement the fix and remove the xfail marker
Files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
autogpt_platform/backend/backend/api/features/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Update routes in '/backend/backend/api/features/' and add/update Pydantic models in the same directory for API development
Files:
autogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/features/v1.py
autogpt_platform/backend/**/api/**/*.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/api/**/*.py: UseSecurity()instead ofDepends()for authentication dependencies to get proper OpenAPI security specification
Follow SSE (Server-Sent Events) protocol: usedata:lines for frontend-parsed events (must match Zod schema) and: commentlines for heartbeats/status
Files:
autogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/api/external/v1/routes.py
autogpt_platform/backend/**/*_test.py
📄 CodeRabbit inference engine (autogpt_platform/backend/AGENTS.md)
autogpt_platform/backend/**/*_test.py: Use pytest with snapshot testing for API responses
Colocate test files with source files using*_test.pynaming convention
Mock at boundaries — mock where the symbol is used, not where it's defined; after refactoring, update mock targets to match new module paths
UseAsyncMockfromunittest.mockfor async functions in tests
When writing tests, use Test-Driven Development (TDD): write failing tests marked with@pytest.mark.xfailbefore implementation, then remove the marker once the implementation is complete
When creating snapshots in tests, usepoetry run pytest path/to/test.py --snapshot-update; always review snapshot changes withgit diffbefore committing
Files:
autogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.py
autogpt_platform/backend/**/test/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use snapshot testing with '--snapshot-update' flag in backend tests when output changes; always review with 'git diff'
Files:
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py
🧠 Learnings (17)
📚 Learning: 2026-02-05T04:11:00.596Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 11796
File: autogpt_platform/backend/backend/blocks/video/concat.py:3-4
Timestamp: 2026-02-05T04:11:00.596Z
Learning: In autogpt_platform/backend/backend/blocks/**/*.py, when creating a new block, generate a UUID once with uuid.uuid4() and hard-code the resulting string as the block's id parameter. Do not call uuid.uuid4() at runtime; IDs must be constant across all imports and runs to ensure stability.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-16T16:32:21.686Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:32:21.686Z
Learning: In autogpt_platform/backend/backend/blocks/, the Block base class execute() already wraps run() in a try/except to convert uncaught exceptions into BlockExecutionError/BlockUnknownError. Do not add per-block try/except in individual block run() methods, as this is not the established pattern (e.g., Gmail, Slack, Todoist blocks omit it). Only use explicit try/except within blocks that need to distinguish between success and error yield paths inside a generator (e.g., attachment blocks). This guidance applies to all Python files under autogpt_platform/backend/backend/blocks/ and similar block implementations; avoid duplicating error handling in run() unless a block requires generator-based branching.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-04-23T12:55:26.122Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12893
File: autogpt_platform/backend/backend/blocks/ayrshare/post_to_tiktok.py:24-24
Timestamp: 2026-04-23T12:55:26.122Z
Learning: Cost billing via the cost(*costs) decorator is applied at input-evaluation time (before a block’s run() executes). Therefore, mutating input_data inside run() will not change billing. When a block’s billing depends on a field plus URL/sniff-derived signals, treat the explicitly declared billing field (e.g., is_video) as the only billing source—set it correctly before run() (or in the code path that occurs before the decorator evaluates input_data). This should be checked for all blocks under autogpt_platform/backend/backend/blocks/ so billing signals are not mistakenly assumed to update during run().
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-02-26T17:02:22.448Z
Learnt from: Pwuts
Repo: Significant-Gravitas/AutoGPT PR: 12211
File: .pre-commit-config.yaml:160-179
Timestamp: 2026-02-26T17:02:22.448Z
Learning: Keep the pre-commit hook pattern broad for autogpt_platform/backend to ensure OpenAPI schema changes are captured. Do not narrow to backend/api/ alone, since the generated schema depends on Pydantic models across multiple directories (backend/data/, backend/blocks/, backend/copilot/, backend/integrations/, backend/util/). Narrowing could miss schema changes and cause frontend type desynchronization.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-03-05T15:42:08.207Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12297
File: .claude/skills/backend-check/SKILL.md:14-16
Timestamp: 2026-03-05T15:42:08.207Z
Learning: In Python files under autogpt_platform/backend (recursively), rely on poetry run format to perform formatting (Black + isort) and linting (ruff). Do not run poetry run lint as a separate step after poetry run format, since format already includes linting checks.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: In autogpt_platform/backend/backend/blocks/ (and related blocks under autogpt_platform/backend/backend/blocks/), do not add try/except blocks around a block's run() method for standard error propagation. The block executor framework (backend/executor/manager.py) catches uncaught exceptions from run() and emits them on the 'error' output. Only add explicit try/except blocks when you need to control partial outputs in failure cases (e.g., certain outputs must not be yielded on error, as in attachment blocks). This is the standard pattern across the codebase; apply it broadly to blocks' run() implementations.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-16T16:30:23.196Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/pods.py:62-74
Timestamp: 2026-03-16T16:30:23.196Z
Learning: In any Python file under autogpt_platform/backend/backend/blocks, do not add a try/except around run() solely for standard error handling. The block framework’s _execute() in _base.py already catches unhandled exceptions and re-raises as BlockExecutionError or BlockUnknownError. If you yield ("error", message), _execute() raises BlockExecutionError immediately, so the error port will not propagate downstream. Reserve explicit try/except for scenarios where you must control partial output (e.g., attachment blocks that must skip yielding content_base64 on failure).
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-16T16:30:11.452Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 12417
File: autogpt_platform/backend/backend/blocks/agent_mail/threads.py:80-102
Timestamp: 2026-03-16T16:30:11.452Z
Learning: Do not wrap synchronous AgentMail SDK calls with asyncio.to_thread() in blocks under autogpt_platform/backend/backend/blocks (and across the codebase). The block executor runs node execution in dedicated threads via asyncio.run_coroutine_threadsafe (see manager.py around lines ~745-752 and ~1079). The existing pattern avoids using asyncio.to_thread for SDK calls inside async run() methods, so maintain that approach and do not add to_thread usage in these code paths.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-16T16:35:40.236Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12440
File: autogpt_platform/backend/backend/api/features/workflow_import.py:54-63
Timestamp: 2026-03-16T16:35:40.236Z
Learning: Avoid using the word 'competitor' in public-facing identifiers and text. Use neutral naming for API paths, model names, function names, and UI text. Examples: rename 'CompetitorFormat' to 'SourcePlatform', 'convert_competitor_workflow' to 'convert_workflow', '/competitor-workflow' to '/workflow'. Apply this guideline to files under autogpt_platform/backend and autogpt_platform/frontend.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-03-19T15:10:50.676Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12483
File: autogpt_platform/backend/backend/copilot/tools/test_dry_run.py:298-303
Timestamp: 2026-03-19T15:10:50.676Z
Learning: When using Python’s `unittest.mock.patch` in tests, choose the patch target based on how the imported name is resolved:
- If the code under test uses an **eager/module-level import** (e.g., `from foo.bar import baz` at module top), patch **the module where the name is looked up** (i.e., where it is used in the SUT), e.g. `patch("mymodule.baz")`.
- If the code under test uses a **lazy import** executed later (e.g., `from foo.bar import baz` inside a function/branch), patch **the source module** (e.g., `patch("foo.bar.baz")`) because the late `from ... import` will read the (potentially patched) name from the source module at call time.
For a concrete example: if `simulate_block` is imported inside an `if dry_run:` block in the SUT, then the correct test patch target is the source module path for `simulate_block` as it exists at call time (e.g., `patch("backend.executor.simulator.simulate_block")`), not the test file’s import location.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.py
📚 Learning: 2026-03-31T15:37:38.626Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12623
File: autogpt_platform/backend/backend/copilot/tools/agent_generator/fixer.py:37-47
Timestamp: 2026-03-31T15:37:38.626Z
Learning: When validating/constructing Anthropic API model IDs in Significant-Gravitas/AutoGPT, allow the hyphen-separated Claude Opus 4.6 model ID `claude-opus-4-6` (it corresponds to `LlmModel.CLAUDE_4_6_OPUS` in `autogpt_platform/backend/backend/blocks/llm.py`). Do NOT require the dot-separated form in Anthropic contexts. Only OpenRouter routing variants should use the dot separator (e.g., `anthropic/claude-opus-4.6`); `claude-opus-4-6` should be treated as correct when passed to Anthropic, and flagged only if it’s used in the OpenRouter path where the dot form is expected.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-04-15T02:43:36.890Z
Learnt from: ntindle
Repo: Significant-Gravitas/AutoGPT PR: 12780
File: autogpt_platform/backend/backend/copilot/tools/workspace_files.py:0-0
Timestamp: 2026-04-15T02:43:36.890Z
Learning: When reviewing Python exception handlers, do not flag `isinstance(e, X)` checks as dead/unreachable if the caught exception `X` is a subclass of the exception type being handled. For example, if `X` (e.g., `VirusScanError`) inherits from `ValueError` (directly or via an intermediate class) and it can be raised within an `except ValueError:` block, then `isinstance(e, X)` inside that handler is reachable and should not be treated as dead code.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-05-23T05:29:43.085Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13200
File: autogpt_platform/backend/backend/executor/scheduler.py:590-593
Timestamp: 2026-05-23T05:29:43.085Z
Learning: When reviewing Python code that uses Pydantic discriminated/tagged unions (e.g., `Annotated[Union[...], Field(discriminator="kind")]`), recognize that using `isinstance(x, SomeVariantInfo)` to narrow the union is an intentional and correct runtime guard and should also enable static type narrowing in tools like Pyright. Do not recommend replacing such `isinstance`-based narrowing with `cast(...)` when the check already proves the variant at runtime.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-04-22T11:46:04.431Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/config.py:0-0
Timestamp: 2026-04-22T11:46:04.431Z
Learning: Do not flag the Claude Sonnet 4.6 model ID as incorrect when it uses the project’s established hyphenated convention: `anthropic/claude-sonnet-4-6`. This hyphen form is the intentional, production convention and should be treated as valid (including in files like llm.py, blocks tests, reasoning.py, `_is_anthropic_model` tests, and config defaults). Note that OpenRouter also accepts the dot variant `anthropic/claude-sonnet-4.6`, so either form may be tolerated, but `anthropic/claude-sonnet-4-6` should be considered the standard to match project usage.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-04-22T11:46:12.892Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 12881
File: autogpt_platform/backend/backend/copilot/baseline/service.py:322-332
Timestamp: 2026-04-22T11:46:12.892Z
Learning: In this codebase (Significant-Gravitas/AutoGPT), OpenRouter-routed Anthropic model IDs should use the hyphen-separated convention (e.g., `anthropic/claude-sonnet-4-6`, `anthropic/claude-opus-4-6`). Although OpenRouter may accept both hyphen and dot variants, treat the hyphen-separated form as the intended, correct codebase-wide convention and do not flag it as an error. Only flag the dot-separated variant (e.g., `anthropic/claude-sonnet-4.6`) as incorrect when reviewing/validating model ID strings for OpenRouter-routed Anthropic models.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-05-07T18:48:14.242Z
Learnt from: majdyz
Repo: Significant-Gravitas/AutoGPT PR: 13040
File: autogpt_platform/backend/backend/blocks/llm.py:0-0
Timestamp: 2026-05-07T18:48:14.242Z
Learning: In this repository, isort may split imports from the same module into separate blocks when some imports are aliased (e.g., `from module import X as Y`) and others are not. Preserve the two-block layout when it results from isort (such as keeping `from openai.types.chat import ChatCompletion as OpenAIChatCompletion` separate from non-aliased imports from `openai.types.chat`). Do not treat that split as a style issue during review; merging them into a single block can fail CI with `Imports are incorrectly sorted and/or formatted`.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
📚 Learning: 2026-05-26T14:24:34.866Z
Learnt from: Abhi1992002
Repo: Significant-Gravitas/AutoGPT PR: 13217
File: autogpt_platform/backend/backend/api/features/search/service.py:137-137
Timestamp: 2026-05-26T14:24:34.866Z
Learning: In the Significant-Gravitas/AutoGPT backend, treat `user_id` (an opaque UUID used only for correlation/tracing) as non-PII. Do not flag direct logging of `user_id` in `logger.warning`/`logger.info` statements as a PII exposure issue, as the established convention is to log `user_id` for tracing while reserving PII for fields like email or display name.
Applied to files:
autogpt_platform/backend/backend/blocks/github/test_github_blocks.pyautogpt_platform/backend/backend/api/features/v1_test.pyautogpt_platform/backend/backend/api/external/v1/routes_test.pyautogpt_platform/backend/backend/blocks/_base.pyautogpt_platform/backend/backend/api/features/v1.pyautogpt_platform/backend/backend/blocks/test/test_time_blocks.pyautogpt_platform/backend/backend/api/external/v1/routes.py
🔇 Additional comments (1)
autogpt_platform/backend/backend/blocks/test/test_time_blocks.py (1)
74-79: Already noted: avoid exact wall-clock equality in this assertion.
value == datetime.now(...).strftime(...)can still flake if the date flips between the block call and the assertion; capture a before/after window around the execution instead.
- External direct-block-execute route: move user_db.get_user_by_id above charge_for_direct_block_execution so a missing-user 404 can't trigger after the charge has already been taken (matches the internal route's ordering). - Time-block midnight rollover: capture before/after timestamps around the run and assert membership instead of exact equality, so the user-timezone assertion can't flake when the date flips mid-test.
…gle sheets tests CI caught 2 more direct ``block.execute(input_data)`` callers in ``backend/blocks/google/sheets_test.py`` that needed updating after the ``Block.execute()`` signature change. Update all 4 calls in that file (including the two non-failing ones that would have broken once the credentials guard let them through) to pass ``ExecutionContext()``.
|
🤖 @coderabbitai — the |
|
Tip For best results, initiate chat on the files or code changes.
|
✅ /pr-test --fix — 7/7 PASSE2E run on the dev environment against latest commit (worktree at PR head, native backend,
Issue #12648 reproduction check: before this PR, Signature-change regression check (Scenario 6): the
|
github-project-automation
Bot
moved this from 🚧 Needs work
to 👍🏼 Mergeable
in AutoGPT development kanban
4449625
github-project-automation
Bot
moved this from 👍🏼 Mergeable
to ✅ Done
in AutoGPT development kanban
Thanks for the explanation. I understand the issue now. My original change was fixing the symptom by making the time blocks tolerate a missing Since many blocks rely on I agree with your direction here. The direct execute routes should create the proper |
…xt (#13247) ### Why / What / How **Why**: Follow-up to #13236. That PR plumbed `ExecutionContext` (with `user_id` + `user_timezone`) at the direct-block-execute REST boundary so time blocks invoked via `POST /api/blocks/{id}/execute` use the user's profile timezone. The same logical bug existed — and still exists post-#13236 — at the copilot/autopilot `run_block` tool boundary: `backend/copilot/tools/helpers.py::prepare_block_for_execution` constructs an `ExecutionContext` for every block invoked by autopilot, but never set `user_timezone`. Time blocks called from autopilot with `use_user_timezone=true` therefore returned UTC time regardless of the user's profile. **What**: Set `user_timezone` on the `ExecutionContext` built for copilot block execution, sourced from the live user record (same helper as the REST routes use). **How**: - Add `user_db.get_user_by_id(user_id)` next to the existing `workspace_db().get_or_create_workspace(user_id)` call. - Pass `user_timezone=get_user_timezone_or_utc(user.timezone if user else None)` into the `ExecutionContext(...)` construction. - The unrelated `review_context` further down only flows into `is_block_exec_need_review` (which reads `sensitive_action_safe_mode`, not timezone) and doesn't need the same change. ### Changes 🏗️ - `backend/copilot/tools/helpers.py::prepare_block_for_execution`: add user lookup + `user_timezone` to the `ExecutionContext`. - No schema, migration, or env changes. ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] `poetry run format && poetry run lint --skip-pyright` - [x] Manual E2E via `/pr-test --fix` (queued)
…ndary (Significant-Gravitas#13236) [bench] Reproduces upstream PR Significant-Gravitas#13236's diff exactly, on a Shadow-installed fork. This bench tests whether Shadow flags the *incomplete-fix* gap that CodeRabbit missed: the same logical bug that Significant-Gravitas#13236 fixes at the REST boundary still exists at the autopilot/copilot run_block tool boundary (later fixed in Significant-Gravitas#13247). Original commit message preserved below. Why: Issue Significant-Gravitas#12648 — GetCurrentDateBlock.run() crashes with `missing 1 required keyword-only argument: 'execution_context'` when invoked through the direct- block-execute REST routes.
…ndary (Significant-Gravitas#13236) ### Why / What / How **Why**: Issue Significant-Gravitas#12648 — `GetCurrentDateBlock.run()` crashes with `missing 1 required keyword-only argument: 'execution_context'` when invoked through the direct-block-execute REST routes. The original PR patched the three time blocks individually, but the same bug applies to ~40 other blocks (basic, text, http, agent, autopilot, claude_code, ai_image_*, orchestrator, spreadsheet, talking_head, video/*, screenshotone, io, human_in_the_loop, flux_kontext, code_executor, …) that declare `execution_context: ExecutionContext` as a required kwarg. The root cause is the API boundary, not the blocks. **What**: Fix the bug at the boundary — make `Block.execute()` require `execution_context` explicitly, and have the direct-block-execute routes construct a real `ExecutionContext` (carrying the caller's `user_id` + `user_timezone`) before invoking the block. **How**: 1. `Block.execute()` / `Block._execute()` now take `execution_context: ExecutionContext` as an explicit required keyword-only parameter. Missing-context callers now fail at the type system instead of as a `TypeError` deep inside a block's `run()`. 2. The internal route (`backend/api/features/v1.py::execute_graph_block`) and the external route (`backend/api/external/v1/routes.py::execute_graph_block`) both now build: ```python execution_context = ExecutionContext( user_id=user_id, user_timezone=get_user_timezone_or_utc(user.timezone), ) ``` and forward it to `obj.execute(...)`. Time blocks (and every other block that reads `execution_context.user_timezone`) now get the correct user timezone instead of crashing. 3. The HITL-review gate inside `Block._execute()` now checks `execution_context.graph_exec_id is not None` instead of probing kwarg presence — direct block execution has no graph and never needs review, which keeps the existing behaviour intact. 4. Reverted the per-block changes in `time_blocks.py`; the three time blocks once again declare `execution_context` as a required kwarg, matching every other block. ### Changes 🏗️ - `Block.execute()` / `Block._execute()`: `execution_context` is now an explicit, required keyword argument. - Internal & external `POST /blocks/{id}/execute` routes: build a real `ExecutionContext` from the caller's user record and forward it to the block. - Reverted per-block patches in `backend/blocks/time_blocks.py`. - Regression tests: - `test_execute_graph_block_forwards_execution_context` (internal route) - `test_execute_block_forwards_execution_context` (external route) - Updated time-block tests to cover the real paths (empty-context vs user-timezone). - Updated `backend/blocks/github/test_github_blocks.py` to pass `execution_context=ExecutionContext()` to its direct `block.execute()` calls. - No configuration or environment changes. ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] `poetry run pytest backend/blocks/test/test_time_blocks.py backend/blocks/github/test_github_blocks.py backend/api/features/v1_test.py backend/api/external/v1/routes_test.py` - [x] `poetry run lint --skip-pyright` --------- Co-authored-by: majdyz <zamil.majdy@agpt.co>
3 participants
Why / What / How
Why: Issue #12648 —
GetCurrentDateBlock.run()crashes withmissing 1 required keyword-only argument: 'execution_context'when invoked through the direct-block-execute REST routes. The original PR patched the three time blocks individually, but the same bug applies to ~40 other blocks (basic, text, http, agent, autopilot, claude_code, ai_image_, orchestrator, spreadsheet, talking_head, video/, screenshotone, io, human_in_the_loop, flux_kontext, code_executor, …) that declareexecution_context: ExecutionContextas a required kwarg. The root cause is the API boundary, not the blocks.What: Fix the bug at the boundary — make
Block.execute()requireexecution_contextexplicitly, and have the direct-block-execute routes construct a realExecutionContext(carrying the caller'suser_id+user_timezone) before invoking the block.How:
Block.execute()/Block._execute()now takeexecution_context: ExecutionContextas an explicit required keyword-only parameter. Missing-context callers now fail at the type system instead of as aTypeErrordeep inside a block'srun().The internal route (
backend/api/features/v1.py::execute_graph_block) and the external route (backend/api/external/v1/routes.py::execute_graph_block) both now build:and forward it to
obj.execute(...). Time blocks (and every other block that readsexecution_context.user_timezone) now get the correct user timezone instead of crashing.The HITL-review gate inside
Block._execute()now checksexecution_context.graph_exec_id is not Noneinstead of probing kwarg presence — direct block execution has no graph and never needs review, which keeps the existing behaviour intact.Reverted the per-block changes in
time_blocks.py; the three time blocks once again declareexecution_contextas a required kwarg, matching every other block.Changes 🏗️
Block.execute()/Block._execute():execution_contextis now an explicit, required keyword argument.POST /blocks/{id}/executeroutes: build a realExecutionContextfrom the caller's user record and forward it to the block.backend/blocks/time_blocks.py.test_execute_graph_block_forwards_execution_context(internal route)test_execute_block_forwards_execution_context(external route)backend/blocks/github/test_github_blocks.pyto passexecution_context=ExecutionContext()to its directblock.execute()calls.Checklist 📋
For code changes:
poetry run pytest backend/blocks/test/test_time_blocks.py backend/blocks/github/test_github_blocks.py backend/api/features/v1_test.py backend/api/external/v1/routes_test.pypoetry run lint --skip-pyright