-
Notifications
You must be signed in to change notification settings - Fork 5
feat(core): ExecutionContext abstraction for task isolation (#532) #536
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -538,6 +538,7 @@ class BatchRun: | |
| stall_action: str = "blocker" | ||
| concurrency: ConcurrencyConfig = field(default_factory=ConcurrencyConfig) | ||
| isolate: bool = True | ||
| isolation: str = "none" | ||
|
|
||
|
|
||
| def start_batch( | ||
|
|
@@ -554,6 +555,7 @@ def start_batch( | |
| stall_action: str = "blocker", | ||
| concurrency_by_status: Optional[dict[str, int]] = None, | ||
| isolate: bool = True, | ||
| isolation: str = "none", | ||
| ) -> BatchRun: | ||
| """Start a batch execution of multiple tasks. | ||
|
|
||
|
|
@@ -609,6 +611,7 @@ def start_batch( | |
| stall_action=stall_action, | ||
| concurrency=concurrency, | ||
| isolate=isolate, | ||
| isolation=isolation, | ||
| ) | ||
|
|
||
| # Save to database | ||
|
|
@@ -1014,8 +1017,17 @@ def _execute_serial_resume( | |
| if on_event: | ||
| on_event("batch_task_started", {"task_id": task_id, "position": i + 1, "is_retry": True}) | ||
|
|
||
| # Execute task via subprocess | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # Execute task via subprocess (with isolation context) | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context(task_id, IsolationLevel(batch.isolation), workspace.repo_path) | ||
| try: | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
| finally: | ||
| exec_ctx.cleanup() | ||
|
Comment on lines
+1020
to
+1030
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These paths tear the Based on learnings, "agent tool functions ... must be stateless with signature (input_data: dict, workspace_path: Path, tool_call_id: str) -> ToolResult". Also applies to: 1197-1207, 1426-1448, 1840-1870, 1960-1976 🤖 Prompt for AI Agents |
||
|
|
||
| # Record result (overwrites previous result) | ||
| batch.results[task_id] = result_status | ||
|
|
@@ -1182,8 +1194,17 @@ def _execute_retries( | |
| print(f"\n[Retry {retry_num}, {i + 1}/{len(failed_tasks)}] {task_id}: {task_title}") | ||
| print(f" Previous: {previous_status}") | ||
|
|
||
| # Execute task | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # Execute task (with isolation context) | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context(task_id, IsolationLevel(batch.isolation), workspace.repo_path) | ||
| try: | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
| finally: | ||
| exec_ctx.cleanup() | ||
|
|
||
| # Update result | ||
| batch.results[task_id] = result_status | ||
|
|
@@ -1402,16 +1423,29 @@ def _execute_serial( | |
| if on_event: | ||
| on_event("batch_task_started", {"task_id": task_id, "position": i + 1}) | ||
|
|
||
| # Execute task via subprocess | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # Execute task via subprocess (with isolation context) | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context(task_id, IsolationLevel(batch.isolation), workspace.repo_path) | ||
| try: | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
|
|
||
| # If task is BLOCKED, try supervisor resolution | ||
| if result_status == RunStatus.BLOCKED.value: | ||
| supervisor = get_supervisor(workspace) | ||
| if supervisor.try_resolve_blocked_task(task_id): | ||
| # Supervisor resolved the blocker - retry the task | ||
| print(" [Supervisor] Retrying task after auto-resolution...") | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # If task is BLOCKED, try supervisor resolution | ||
| if result_status == RunStatus.BLOCKED.value: | ||
| supervisor = get_supervisor(workspace) | ||
| if supervisor.try_resolve_blocked_task(task_id): | ||
| # Supervisor resolved the blocker - retry the task | ||
| print(" [Supervisor] Retrying task after auto-resolution...") | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
| finally: | ||
| exec_ctx.cleanup() | ||
|
|
||
| # Record result | ||
| batch.results[task_id] = result_status | ||
|
|
@@ -1803,16 +1837,37 @@ def _execute_single_task( | |
| if on_event: | ||
| on_event("batch_task_started", {"task_id": task_id, "position": position}) | ||
|
|
||
| # Execute task via subprocess | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # Create execution context (handles isolation level; NONE is a no-op) | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context( | ||
| task_id, IsolationLevel(batch.isolation), workspace.repo_path | ||
| ) | ||
|
|
||
| try: | ||
| # Execute task via subprocess | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, | ||
| engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, | ||
| stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
|
|
||
| # If task is BLOCKED, try supervisor resolution | ||
| if result_status == RunStatus.BLOCKED.value: | ||
| supervisor = get_supervisor(workspace) | ||
| if supervisor.try_resolve_blocked_task(task_id): | ||
| # Supervisor resolved the blocker - retry the task | ||
| print(" [Supervisor] Retrying task after auto-resolution...") | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # If task is BLOCKED, try supervisor resolution | ||
| if result_status == RunStatus.BLOCKED.value: | ||
| supervisor = get_supervisor(workspace) | ||
| if supervisor.try_resolve_blocked_task(task_id): | ||
| # Supervisor resolved the blocker - retry the task | ||
| print(" [Supervisor] Retrying task after auto-resolution...") | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, | ||
| engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, | ||
| stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
| finally: | ||
| exec_ctx.cleanup() | ||
|
|
||
| # Record result | ||
| batch.results[task_id] = result_status | ||
|
|
@@ -1902,8 +1957,23 @@ def execute_task(task_id: str) -> tuple[str, str]: | |
| if on_event: | ||
| on_event("batch_task_started", {"task_id": task_id, "parallel": True}) | ||
|
|
||
| # Execute via subprocess | ||
| result_status = _execute_task_subprocess(workspace, task_id, batch.id, engine=batch.engine, stall_timeout_s=batch.stall_timeout_s, stall_action=batch.stall_action) | ||
| # Create execution context for this task | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context( | ||
| task_id, IsolationLevel(batch.isolation), workspace.repo_path | ||
| ) | ||
|
|
||
| try: | ||
| # Execute via subprocess | ||
| result_status = _execute_task_subprocess( | ||
| workspace, task_id, batch.id, | ||
| engine=batch.engine, | ||
| stall_timeout_s=batch.stall_timeout_s, | ||
| stall_action=batch.stall_action, | ||
| worktree_path=exec_ctx.workspace_path if exec_ctx.workspace_path != workspace.repo_path else None, | ||
| ) | ||
| finally: | ||
| exec_ctx.cleanup() | ||
|
|
||
| # Record result (thread-safe due to GIL for simple dict operations) | ||
| batch.results[task_id] = result_status | ||
|
|
@@ -2062,12 +2132,21 @@ def _save_batch(workspace: Workspace, batch: BatchRun) -> None: | |
| conn = get_db_connection(workspace) | ||
| try: | ||
| cursor = conn.cursor() | ||
| # Ensure isolation column exists (migration for existing databases) | ||
| try: | ||
| cursor.execute( | ||
| "ALTER TABLE batch_runs ADD COLUMN isolation TEXT DEFAULT 'none'" | ||
| ) | ||
| conn.commit() | ||
| except Exception: | ||
| pass # Column already exists | ||
|
|
||
| cursor.execute( | ||
| """ | ||
| INSERT OR REPLACE INTO batch_runs | ||
| (id, workspace_id, task_ids, status, strategy, max_parallel, on_failure, | ||
| started_at, completed_at, results, engine) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
| started_at, completed_at, results, engine, isolation) | ||
| VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?) | ||
|
Comment on lines
2146
to
+2149
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Loaded batches always fall back to
💾 Suggested query fix SELECT id, workspace_id, task_ids, status, strategy, max_parallel,
- on_failure, started_at, completed_at, results, engine
+ on_failure, started_at, completed_at, results, engine, isolation
FROM batch_runs
@@
SELECT id, workspace_id, task_ids, status, strategy, max_parallel,
- on_failure, started_at, completed_at, results, engine
+ on_failure, started_at, completed_at, results, engine, isolation
FROM batch_runs
@@
SELECT id, workspace_id, task_ids, status, strategy, max_parallel,
- on_failure, started_at, completed_at, results, engine
+ on_failure, started_at, completed_at, results, engine, isolation
FROM batch_runsAlso applies to: 2162-2163, 2171-2185 🤖 Prompt for AI Agents |
||
| """, | ||
| ( | ||
| batch.id, | ||
|
|
@@ -2081,6 +2160,7 @@ def _save_batch(workspace: Workspace, batch: BatchRun) -> None: | |
| completed_at, | ||
| results_json, | ||
| batch.engine, | ||
| batch.isolation, | ||
| ), | ||
| ) | ||
| conn.commit() | ||
|
|
@@ -2102,4 +2182,5 @@ def _row_to_batch(row: tuple) -> BatchRun: | |
| completed_at=datetime.fromisoformat(row[8]) if row[8] else None, | ||
| results=json.loads(row[9]) if row[9] else {}, | ||
| engine=row[10] if len(row) > 10 and row[10] else "plan", | ||
| isolation=row[11] if len(row) > 11 and row[11] else "none", | ||
| ) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -599,6 +599,7 @@ def execute_agent( | |
| engine: str = "react", | ||
| stall_timeout_s: int = 300, | ||
| stall_action: str = "blocker", | ||
| isolation: str = "none", | ||
| ) -> "AgentState": | ||
|
Comment on lines
+602
to
603
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Move isolation setup under the guarded execution path.
🛠️ Suggested fix- # Create execution context (handles isolation; NONE is a no-op)
- from codeframe.core.sandbox.context import IsolationLevel, create_execution_context
- exec_ctx = create_execution_context(
- run.task_id, IsolationLevel(isolation), workspace.repo_path
- )
- effective_repo_path = exec_ctx.workspace_path
-
- try:
+ # Create execution context (handles isolation; NONE is a no-op)
+ from codeframe.core.sandbox.context import IsolationLevel, create_execution_context
+ exec_ctx = None
+ effective_repo_path = workspace.repo_path
+
+ try:
+ exec_ctx = create_execution_context(
+ run.task_id, IsolationLevel(isolation), workspace.repo_path
+ )
+ effective_repo_path = exec_ctx.workspace_path- exec_ctx.cleanup()
+ if exec_ctx is not None:
+ exec_ctx.cleanup()Also applies to: 679-685, 877-878 🤖 Prompt for AI Agents |
||
| """Execute a task using the agent orchestrator. | ||
|
|
||
|
|
@@ -675,6 +676,13 @@ def execute_agent( | |
| import time as _time_mod | ||
| _perf_start_ms = int(_time_mod.monotonic() * 1000) | ||
|
|
||
| # Create execution context (handles isolation; NONE is a no-op) | ||
| from codeframe.core.sandbox.context import IsolationLevel, create_execution_context | ||
| exec_ctx = create_execution_context( | ||
| run.task_id, IsolationLevel(isolation), workspace.repo_path | ||
| ) | ||
| effective_repo_path = exec_ctx.workspace_path | ||
|
|
||
|
Comment on lines
+679
to
+685
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Create the context before building hook state. Because the execution context is created after 🤖 Prompt for AI Agents |
||
| try: | ||
| # Execute before_task hook (aborts on failure) | ||
| if env_config and hook_ctx: | ||
|
|
@@ -733,7 +741,7 @@ def on_adapter_event(event: AdapterEvent) -> None: | |
| ) | ||
|
|
||
| result = wrapper.run( | ||
| run.task_id, packaged.prompt, workspace.repo_path, | ||
| run.task_id, packaged.prompt, effective_repo_path, | ||
| on_event=on_adapter_event, | ||
| ) | ||
| else: | ||
|
|
@@ -758,7 +766,7 @@ def on_adapter_event(event: AdapterEvent) -> None: | |
| ) | ||
|
|
||
| result = adapter.run( | ||
| run.task_id, "", workspace.repo_path, | ||
| run.task_id, "", effective_repo_path, | ||
| on_event=on_adapter_event, | ||
| ) | ||
|
|
||
|
|
@@ -866,6 +874,8 @@ def on_adapter_event(event: AdapterEvent) -> None: | |
| finally: | ||
| # Always close the output logger to ensure file is properly flushed | ||
| output_logger.close() | ||
| # Clean up execution context (no-op for NONE, removes worktree for WORKTREE) | ||
| exec_ctx.cleanup() | ||
|
|
||
|
|
||
| def _event_type_to_category(event_type: str): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| """Execution environment sandbox abstraction. | ||
|
|
||
| Provides ExecutionContext and IsolationLevel for isolating task execution | ||
| from the shared filesystem. | ||
| """ | ||
|
|
||
| from codeframe.core.sandbox.context import ( | ||
| ExecutionContext, | ||
| IsolationLevel, | ||
| create_execution_context, | ||
| ) | ||
|
|
||
| __all__ = ["ExecutionContext", "IsolationLevel", "create_execution_context"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Validate
isolationbefore the batch is persisted.isolationis stored as a raw string here and only converted inside the execution loops. If callers pass"cloud"or any invalid value, the firstcreate_execution_context(...)raises after the batch has already been saved and moved toRUNNING, which leaves a half-started batch instead of a clean validation error.🛠️ Suggested preflight validation
def start_batch( @@ if not task_ids: raise ValueError("task_ids cannot be empty") + + from codeframe.core.sandbox.context import IsolationLevel + + try: + isolation_level = IsolationLevel(isolation) + except ValueError as exc: + raise ValueError(f"Unsupported isolation level: {isolation}") from exc + if isolation_level is IsolationLevel.CLOUD: + raise ValueError( + "Isolation level 'cloud' is not implemented yet. Use 'none' or 'worktree'." + ) @@ concurrency=concurrency, isolate=isolate, - isolation=isolation, + isolation=isolation_level.value, )Also applies to: 598-615
🤖 Prompt for AI Agents