Skip to content

feat(py): add run_python, which runs model code in the agent's sandboxed session - #417

Draft
jat255 wants to merge 25 commits into
jat255/d5sr-stopped-worker-freezefrom
jat255/2q9b-run-python-tool
Draft

jat255 wants to merge 25 commits into
jat255/d5sr-stopped-worker-freezefrom
jat255/2q9b-run-python-tool

Conversation

@jat255

@jat255 jat255 commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

This PR wires up the python code execution functionality by adding the run_python tool, which lets a Commons agent run the model's Python code in a sandboxed session and shows the code, its output, and any matplotlib plots in the chat. Commons gains a network argument ("none" or "full") that sets whether that session can reach the network.

Stacked on #415.

Agent-written notes

Decisions

  • The session runs on its own event loop in a background thread, and the agent registers run_python as an async tool, so stream_async() never blocks the caller's loop. chatlas refuses a sync chat() while an async tool is registered, so Commons.chat() uses a sync version of the tool for the length of the call. As a result, Python's chat() can run code, and R's cannot.
  • Building a Commons also builds the worker, so a host that cannot sandbox the session (Windows, or Linux without seccomp or without Landlock and user namespaces) fails at construction unless COMMONS_ALLOW_UNSAFE_FALLBACK is set. Such hosts could build an agent before this PR. R behaves the same way, but R's trusted-only mode skips the worker; Python has no such mode yet, so the opt-in is the only way through until it does.
  • The tool description asks the session's interpreter whether matplotlib and pip are importable, and adjusts its plot and install rules to match. It checks pip only when the network is on.
  • With network="full", the model is told to run pip inside the session rather than in a subprocess. The macOS sandbox aborts every child process (likely in R too), and the guardrails fallback refuses them. Whether to loosen that is a sandbox policy question for both packages, so it is tracked separately.
  • When a cancelled call restarted the session, the next result tells the model its variables were reset.
  • The display CSS classes are renamed from commons-run-r-* to commons-run-*, so both packages use the same names.

Left out

  • A shared fixture for the REPL behaviour. A follow-up PR stacked on this one adds it.
  • The governance and feature-parity docs pages still say Python has no code execution. A separate docs task rewrites them.
  • The shared system prompt invites charts even when matplotlib is missing. This is rare, and the tool description says plots are unavailable.
  • Plot alt text is generic in both packages.

Testing: ruff, pyrefly, and the full pytest suite pass (2170 passed, 50 skipped), and CI's R CMD check passes on every platform. The tool was also exercised against a live model through chat(), including variables persisting across calls, and the in-session pip install was run in the macOS sandbox. A local review covered correctness, API and parity, tests, docs, security, and accessibility, and its findings are fixed or listed above.

kata: 2q9b (follow-ups: 1bph for the macOS child-process abort, 4r4a for trusted-only mode)

R changes

@simonpcouch

Just a small change to the names of CSS classes used by run_r to allow them to be shared with run_python.
Class names are now commons-run-display, commons-run-details, commons-run-code, and commons-run-plot instead of commons-run-r-*. The same inputs give the same HTML apart from the class names. App CSS that existing commons apps have already written to target the old names will stop matching (mentioned in NEWS.md, though we can remove it if not important enough for that file). See run-r.R and the stylesheet.

jat255 added 16 commits October 9, 2026 21:52
The stylesheet's code and plot display classes were named for run_r. run_python renders the same display, so the classes become commons-run-display, commons-run-details, commons-run-code, and commons-run-plot, and run_r's HTML uses the new names.
chatlas runs an async tool only from stream_async(), and runs a sync tool directly on the caller's event loop, where a call that takes a minute stalls every other task. WorkerThread keeps a Worker on a private loop in a daemon thread, started by the first call. An async caller awaits a call without blocking its loop, a sync caller blocks on the same session, and cancelling an async caller cancels the call.
…xed session

Every agent registers run_python. Its description follows run_r's in Python's idiom: the sandbox framing, the session the user cannot reach, the handles preloaded from whichever registered tools store results, measure sources read with inspect.getsource(), and rules whose network line says whether pip can install packages. A result is tagged B and asks for citations; the model gets the text the call produced and each plot as an image, and the reader gets the highlighted code with its output and the plots at their size.

Commons takes a network argument and builds its Worker during construction, so a host that cannot sandbox the session fails there, before any model asks to run code. The worker lives on a WorkerThread: the registered tool is async, and chat() swaps in a sync variant for its length, because chatlas refuses a synchronous chat while any async tool is registered.
…lls after

close() waited for the worker's shutdown, which waits out a running call, so a call longer than the close timeout left the loop stopped under a shutdown still in progress. close() now cancels the calls in flight first, and stops the loop only once the shutdown finishes. Submitting a call holds the same lock close() takes, so a call is either cancelled by the close or refused as closed, and a caller whose call the close cancelled is told the session is closed.
…port

run_python's rules told every agent to draw matplotlib figures, though matplotlib is optional, and offered pip when the host found it only in the user site, which the session's -I leaves out. Both rules now follow session_can_import(), which ignores the user site.
Filtering the user site out of the host's import path still counted modules found through PYTHONPATH or the current directory, which -I also leaves out. session_can_import() now asks the interpreter the session runs, under -I, once per module per process.
…ectory

A sitecustomize or .pth hook still runs under -I and can move import paths by environment, so the probe now runs with worker_env() and an empty scratch directory as its working directory, as the session does.
LocalBackend sets the thread-count variables when the sandbox needs a single-threaded worker, so the probe builds its environment with the same needs_single_thread() answer.
Replies now carry one ordered output of text and plots. run_python's result walks it: adjacent text joins into one run, each plot sits where it was drawn in what the model receives, and the value or the traceback starts a line after everything the call wrote. An error therefore shows what the call printed before it raised.
…hread

Ctrl-C while a sync caller waits now cancels the call, as cancelling an
async caller does, so the next call does not queue behind it until its
timeout. close() called from the loop's own thread (a garbage collection
there) starts the shutdown and returns instead of blocking the loop it
waits on, and a shutdown that raises still joins the thread and closes
the loop. The no-thread-before-first-call test checks the thread itself
rather than the process's thread count, and the docstrings say what
they mean in plain terms.
- Highlighting splits lines as tokenize does, so a carriage return or
  form feed in the output no longer shifts spans onto the wrong text.
- pip is probed only when the session has network access.
- Without matplotlib, the description no longer opens by offering plots.
- chat() swaps in the sync tool only while run_python is registered, so
  a tool the user removed stays removed.
- Commons' network argument is typed inline, since the alias is private.

Tests now cover the network argument reaching the description and
annotations, the async tool coming back after a failed turn, the
citation request on a run_python result, and the finalizer stopping
the session's thread. The docstrings are rewritten in plain terms.
@github-actions

Copy link
Copy Markdown
Contributor

Preview root: https://posit-dev.github.io/commons/pr-417/

Python site preview: https://posit-dev.github.io/commons/pr-417/py/

Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds.

@jat255
jat255 added this pull request to stack #416 October 10, 2026 02:48
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (dogfood.team.pct.posit.it): https://dogfood.team.pct.posit.it/connect/#/apps/d7a36cae-8f27-448b-a478-61b81fbe3942/draft/378683

Deployed from commit 688aff4.

The loop's thread now closes the loop when run_forever() returns, so it
is closed on every path, including a close() called from that thread,
which returns without waiting.
@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (connect.staging.pct.posit.it): https://connect.staging.pct.posit.it/connect/#/apps/ad662e1b-5048-4acc-9ad7-f9478c92274e/draft/3452

Deployed from commit 688aff4.

@jat255 jat255 added this to the py-M6: code execution milestone Oct 10, 2026
A cancelled call or Ctrl-C shuts the worker down, so the next call starts
a fresh session. WorkerThread now records that, and take_restart()
reports it once. The cancel and Ctrl-C tests use a 60-second call timeout
with a time bound, so they fail if cancelling does nothing; the Ctrl-C
test cancels its signal timer; and every test that builds its own worker
closes it in a finally.
…p advice

- The highlighter also falls back to plain escaping on UnicodeDecodeError,
  which Python 3.12+ tokenize raises for a carriage return before
  non-ASCII text. That error used to replace the call's whole result.
- The network="full" rule tells the model to run pip in the session. The
  macOS sandbox aborts every child process and guardrails refuse them, so
  the subprocess route it gave before could not work.
- A model that cancelled a call is told, on the next result, that the
  session restarted and its variables were reset.
- A probe that fails to run is no longer cached, so one slow start does
  not leave every later agent saying matplotlib is missing.
The restart flag moves from WorkerThread to Worker, where it is set in
the one place a cancel shuts the session down: a call cancelled while it
ran. A call cancelled while it waited for the lock leaves the session and
its variables alone, and no longer makes the next result claim a reset.
The install rule now names the import and where the target path comes
from, since the session starts with neither pip nor sys bound. Verified
in the macOS sandbox: the steps as written install and import tabulate.
The rule now gives every import and the target directory, and the
snippet, taken from the description verbatim, installs and imports
tabulate in the macOS sandbox.
Python 3.11 tokenizes a carriage return before non-ASCII text and
highlights it, while 3.12+ raises and falls back to plain escaping, so
the test asserts what both share: the displayed text is the source.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant