Skip to content

Commit f4fd794

Browse files
caohy1988claude
andcommitted
docs: Fix PermissionError kill fallback, align non-goals with Option A
- PermissionError from os.kill now triggers container.restart() instead of being silently swallowed (fixes timeout DoS for the common case where container runs as root and ADK does not) - ProcessLookupError remains the only silently-ignored exception (process already exited, no action needed) - Non-Goals §2.2 updated to describe persistent REPL crash recovery instead of stale cumulative-replay language Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent f55da53 commit f4fd794

File tree

1 file changed

+14
-11
lines changed

1 file changed

+14
-11
lines changed

docs/design/code_executor_enhancements.md

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,13 @@ The following are explicitly **out of scope** for this design:
4848
`object.__subclasses__()`, `importlib` through `__builtins__`, etc.
4949
True isolation requires a process or container boundary.
5050

51-
2. **Idempotent replay of side-effecting code** — The stateful
52-
`ContainerCodeExecutor` (Proposal 2) replays prior code blocks.
53-
Code with non-idempotent side effects (file writes, network calls,
54-
database mutations) is **not supported** in stateful replay mode.
55-
The design suppresses stdout but cannot suppress arbitrary I/O.
56-
Users must keep side-effecting code in the final block or use the
57-
persistent-process approach (Phase 2 / Option A).
51+
2. **Automatic state recovery after crash** — The stateful
52+
`ContainerCodeExecutor` (Proposal 2) uses a persistent REPL
53+
(Option A). If the REPL or container crashes, in-process state
54+
is lost. The executor reports an error; it does **not** attempt
55+
automatic replay of prior code blocks, because prior blocks may
56+
have had non-idempotent side effects (file writes, network calls,
57+
database mutations) that should not be re-executed.
5858

5959
3. **Multi-tenant per-execution isolation** — Per-execution isolation
6060
(fresh sandbox per call) is the domain of `GkeCodeExecutor` and
@@ -314,10 +314,13 @@ def execute_code(self, invocation_context, code_execution_input):
314314
host_pid = info.get('Pid', 0)
315315
if host_pid > 0:
316316
os.kill(host_pid, signal.SIGKILL)
317-
except (ProcessLookupError, PermissionError):
318-
pass # Process already exited
319-
except Exception:
320-
# Last resort: restart the container
317+
except ProcessLookupError:
318+
pass # Process already exited — no action needed
319+
except (PermissionError, Exception):
320+
# os.kill failed (most commonly PermissionError
321+
# when container runs as root and ADK does not).
322+
# Restart the container to ensure the runaway
323+
# process is terminated.
321324
try:
322325
self._container.restart(timeout=1)
323326
except Exception:

0 commit comments

Comments
 (0)