Skip to content

feat(agent): add python repl skill - #3688

Open
hvent90 wants to merge 4 commits into
mainfrom
feat/python-repl-memory-blueprint
Open

feat(agent): add python repl skill#3688
hvent90 wants to merge 4 commits into
mainfrom
feat/python-repl-memory-blueprint

Conversation

@hvent90

@hvent90 hvent90 commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Contribution path

  • Small, safe change that does not need a tracking issue

Problem

All that wonderful data was in memory2 but the agent couldn't access it.

Solution

A new skill is added to our MCP server: python_repl(code: str) -> str:

  • A Python process is spun up only to run code and then is shut down
  • The returned value is a concatenation of stdout and stderr

A new blueprint is also created: unitree-go2-agentic-memory:

  • Inherits from the unitree-go2 blueprint
  • adds Go2Memory and PythonReplSkill blueprints

How to Test

Turn on your Go2.

ROBOT_IP=<go2_ip> uv run dimos run unitree-go2-agentic-memory

Then, either hook up your harness of choice to the DimOS MCP or do uv run dimos humancli and say something like:

Use the python skill to look at recorded memory and tell me about the world around you
demo-2026-08-25-13-47-27.mp4

AI assistance

I used gpt-5.6-sol to write the vast majority of the code diff.

Checklist

  • I have read and approved the CLA.

@github-actions github-actions Bot added the first-time-contributor PR opened by an author who had not previously committed to this repository label Aug 25, 2026
@codecov

codecov Bot commented Aug 25, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.00000% with 2 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
dimos/agents/skills/run_python.py 94.73% 1 Missing and 1 partial ⚠️
@@            Coverage Diff             @@
##             main    #3688      +/-   ##
==========================================
+ Coverage   77.78%   77.80%   +0.02%     
==========================================
  Files        1287     1290       +3     
  Lines      123467   123566      +99     
  Branches    10822    10827       +5     
==========================================
+ Hits        96044    96146     +102     
+ Misses      24281    24280       -1     
+ Partials     3142     3140       -2     
Flag Coverage Δ
OS-ubuntu-24.04-arm 72.80% <98.00%> (+0.03%) ⬆️
OS-ubuntu-latest 74.63% <98.00%> (+0.01%) ⬆️
Py-3.10 74.62% <98.00%> (+0.02%) ⬆️
Py-3.11 74.62% <98.00%> (+0.01%) ⬆️
Py-3.12 74.62% <98.00%> (+0.02%) ⬆️
Py-3.13 74.63% <98.00%> (+0.02%) ⬆️
Py-3.14 74.63% <98.00%> (+0.02%) ⬆️
Py-3.14t 74.62% <98.00%> (+0.01%) ⬆️
SelfHosted-Large 30.00% <45.26%> (+0.01%) ⬆️
SelfHosted-Linux 35.22% <45.26%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/agents/skills/test_run_python.py 100.00% <100.00%> (ø)
dimos/memory/module.py 59.41% <100.00%> (+1.78%) ⬆️
dimos/memory/test_module.py 93.47% <100.00%> (+0.97%) ⬆️
dimos/robot/all_blueprints.py 100.00% <ø> (ø)
...2/blueprints/agentic/unitree_go2_agentic_memory.py 100.00% <100.00%> (ø)
dimos/agents/skills/run_python.py 94.73% <94.73%> (ø)

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This change adds an agent-callable Python analysis skill that opens the active robot-memory recording in a new process, captures its output, and registers the supporting recorder and memory blueprint.

The no-output test was executed through the skill branch and is confirmed to fail because its expected message differs from the implementation's returned message. The new execution interface also accepts arbitrary Python under the service account and has no execution deadline, so it is not safe to expose to untrusted callers.

T-Rex validation blocked

End-to-end checks of unrestricted code execution and an infinite-loop payload could not reach run_python because the environment is missing the zenoh package during DimOS import. Alternate attempts were also blocked by missing pydantic, while focused test collection required missing python-dotenv.

Confidence Score: 1/5

Do not merge until arbitrary code execution is isolated and bounded, and the failing no-output test is reconciled with the implementation.

Three independent blocking issues remain: one exposes service-account capabilities to callers, one permits a submitted program to wait indefinitely, and one makes the new test fail. The message mismatch was directly exercised; the other two are explicit in the execution path but their runtime harnesses were stopped by missing packages.

Files Needing Attention: dimos/agents/skills/python_repl.py needs an authorization and isolation boundary plus an execution deadline; dimos/agents/skills/test_python_repl.py needs its no-output assertion aligned with the selected public message.

Security Review

The new run_python interface forwards caller-controlled Python to an unrestricted exec call in a service-launched child process. The path does not apply an authorization check, sandbox, privilege reduction, or restricted interpreter, so anyone able to invoke the skill can use the DimOS service account's filesystem and process capabilities. Restrict access to authorized principals and replace free-form execution with a least-privileged sandbox or constrained analysis API before deployment.

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding, supported by artifacts showing the Direct no-output branch harness source and a mismatch in the Direct Python REPL no-output harness.
  • An attempt to perform temporary-recording execution and infinite-loop checks ran but stopped during module import due to missing dependencies in both environments (one lacking pydantic, the other lacking zenoh).
  • Static execution evidence confirms the PythonReplSkill flow, showing that dimos/agents/skills/python_repl.py launches sys.executable -c _PYTHON_BOOTSTRAP and that _PYTHON_BOOTSTRAP executes stdin via exec(compile(...)); two runtime proofs were attempted but could not run because required imports were absent.
  • The harness is described as running PythonReplSkill in a separate process with a harmless while True: pass payload, a 1.5-second deadline, and full-process cleanup, but both runs halted during module import, so runtime lifetime cannot be confirmed from this environment.
  • The harness observed an output mismatch where the actual result was reported as no output, contrasting with the expected message that code should end with a print; no product code changes were made.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 No-output test expectation does not match PythonReplSkill output

    • Bug
      • test_run_python_reports_when_code_prints_nothing expects (no output — end your code with print()), but the executed run_python no-output branch returned (no output - did you forget to print()?).
    • Cause
      • The test literal and the implementation literal use different wording and punctuation.
    • Fix
      • Make the intended public message identical in the test and PythonReplSkill.run_python.

    T-Rex Ran code and verified through T-Rex

Reviews (1): Last reviewed commit: "better printed message on empty output" | Re-trigger Greptile

Comment thread dimos/agents/skills/python_repl.py Outdated
Comment thread dimos/agents/skills/python_repl.py Outdated
Comment thread dimos/agents/skills/test_python_repl.py Outdated
Comment thread dimos/agents/skills/python_repl.py Outdated
Comment thread dimos/agents/skills/python_repl.py Outdated
Comment thread dimos/agents/skills/run_python.py Outdated
return text[:limit] + f"\n... [truncated, {len(text)} chars total]"


run_python_skill = RunPythonSkill.blueprint

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you end up creating this? :D We've removed this style a long time ago.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woops, fixed. looks like a stale AGENTS.md instruction - I'm letting the fix to that hitch hike into this PR

@github-actions github-actions Bot added the ready-to-merge Required CI checks have passed on this PR label Aug 25, 2026
@github-actions github-actions Bot added ready-to-merge Required CI checks have passed on this PR and removed ready-to-merge Required CI checks have passed on this PR labels Aug 25, 2026
class RunPythonSkill(Module):
"""Give an agent a fresh Python process for each tool call."""

dedicated_worker: ClassVar[bool] = True

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not need a dedicated worker. They are for compute intensive python work, but this runs Python in separate processes.

namespace starts with ``store`` (the live memory store) and ``np`` (NumPy),
and normal Python imports are available.

Each call runs in a new Python process. Variables and imports do not carry

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each call runs in a new Python process.

But then this is not a REPL. L is for loop.


from dimos.memory.store.sqlite import SqliteStore

store = SqliteStore(path=sys.argv[1], must_exist=True)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why force the construction of a store? Shouldn't the agent create one if it wants to?

Same with numpy.

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

Labels

first-time-contributor PR opened by an author who had not previously committed to this repository ready-to-merge Required CI checks have passed on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants