[Agentic-JJ-Web3]

Part of #885. This is the first of three PRs — steps 1–3 of your outline
(harness loop, basic tools, running it with tests and the Burr UI). CLI and
permissions follow separately.

What's here

examples/coding-agent/ — an agent that reads files, writes files, lists a
directory, and runs shell commands, looping until the model answers without
requesting a tool or the step budget runs out.

The graph is human_input → create_prompt → call_llm, with four bound tool
actions fanning out of call_llm and looping back to it. Each tool is bound
separately rather than dispatched from a registry, so the Burr UI shows which
one ran at each step.

Decisions I made without you

You hadn't replied yet, so these are assumptions rather than agreements —
happy to change any of them:

1. Scripted LLM client by default. notebook.ipynb is a required file and
   has to execute, so the example falls back to a client replaying a fixed
   sequence of tool calls when OPENAI_API_KEY isn't set. Real OpenAI client
   when it is.
2. run_bash is not sandboxed. File paths are confined to a workspace
   directory, but a shell command can cd out. The README says so plainly,
   which leaves step 5 something concrete to fix.
3. Four tools, not three — added list_files, since an agent that can't
   see the directory can't orient itself.
4. One tool call per iteration. If the model requests several, the extras
   are dropped. Documented as a known simplification.

Notes

• validate_examples.py passes for coding-agent.
• The tool schema marks only parameters without defaults as required. The
  tool-calling example marks all of them, which misreports optional args —
  happy to open a separate issue for that.
• examples/hello-world-counter crashes at app.visualize() unless the
  Graphviz binary is installed, and passing a stale app_id after clearing
  ~/.burr raises a pydantic error on PointerModel.app_id. Both separate
  from this PR; can file them if useful.

[Agentic-JJ-Web3]

Opened #886 with steps 1–3. I made a few calls in the absence of answers to my questions above — they're listed in the PR description, happy to change any of them.

[DeepGandhi01]

Thanks for putting this together — the state-machine framing of the agent loop reads really well, and the scripted client makes it easy to try. I checked out this branch and ran it in a clean Python 3.14 venv built only from examples/coding-agent/requirements.txt (burr 0.42.0, openai 3.22.1), then probed the loop with ScriptedClient scripts. A few things came up that seem worth fixing before merge; each is reproducible with the scripted client and no API key.

1. Example fails to import from its own requirements

tools.py imports requests (and openai, inspect, json, typing, and burr.core, plus os twice), none of which it uses. requests is not in requirements.txt, so in a clean environment:

$ pip install -r requirements.txt && python application.py
  File ".../tools.py", line 25, in <module>
    import requests
ModuleNotFoundError: No module named 'requests'

Suggested fix: drop the unused imports from tools.py (it only needs os and subprocess). With requests installed manually, the scripted run completes, which confirms this is the only blocker.

2. create_prompt only runs once per task, not before each LLM call

The README describes create_prompt as the seam "for shaping the prompt before each call", but the tool actions transition straight back to call_llm:

(["read_file", "write_file", "list_files", "run_bash"], "call_llm"),

Observed trace for one tool call: human_input → create_prompt → call_llm → list_files → call_llm → respond. Anyone adding a file tree or history summary in create_prompt (the extension the README suggests) would only see it applied to the first call. Routing tool actions back to create_prompt fixes it:

(["read_file", "write_file", "list_files", "run_bash"], "create_prompt"),

3. An unknown tool name halts the run silently

If the model requests a tool that isn't registered (common with real models), no transition out of call_llm matches, and app.run(halt_after=["respond"]) stops at call_llm with final_answer=None and no error. Trace: human_input → create_prompt → call_llm. One option is to validate in call_llm and feed an error back as a tool message, or add a fallback transition such as ("call_llm", "respond", default) after the tool transitions so the run always terminates in respond.

4. Tool errors crash the whole run instead of going back to the model

execute_tool calls tool_function(**state["next_args"]) directly, so:

• Wrong argument names from the model ({"filename": "x"} for read_file) raise TypeError: read_file() got an unexpected keyword argument 'filename'.
• run_bash before the workspace directory exists raises FileNotFoundError from subprocess.run(cwd=...) (only TimeoutExpired is caught). ./workspace isn't created until the first write_file, so a model that starts with run_bash crashes on the first step.

Since the example's point is that the agent "sees them fail, and tries again", catching exceptions in execute_tool and returning {"error": ...} as the tool message would make the loop self-correcting. Creating the workspace up front (os.makedirs(WORKSPACE, exist_ok=True)) fixes the second case directly.

5. Malformed tool-call JSON crashes OpenAIClient

json.loads(c.function.arguments) raises JSONDecodeError on malformed arguments, which models occasionally produce. Worth catching and returning an error tool message, same as (4).

6. Minor: _resolve can be escaped via symlinks

_resolve uses os.path.abspath, which doesn't resolve symlinks, so a symlink inside the workspace pointing outside it passes the check (read_file("link/secret.txt") returned a file from outside the workspace). The README is clear that this isn't a sandbox and run_bash can cd anywhere, so this is low severity, but os.path.realpath for both root and full would make _resolve do what its docstring says.

7. Nit

Without the Graphviz binary, app.visualize() in __main__ prints a ModuleNotFoundError: No module named 'graphviz' traceback before the run (it still completes). Wrapping it in a try/except ImportError or only visualizing on request would keep first-run output clean.

Happy to help with any of these or open a follow-up PR against this branch if that's useful.

[skrawcz]

Thanks for putting this together, and apologies for the delayed maintainer response. Your initial design choices make sense:

• The scripted client fallback is the right default so the example runs without credentials and can be tested in CI.
• Unsandboxed run_bash is acceptable for this first teaching stage, provided the warning remains prominent; permissions are intentionally a later step.
• Adding list_files is useful.
• One tool call per iteration is acceptable as a clearly documented simplification.
• The proposed sequence of separate PRs also makes sense.

The functional findings from @DeepGandhi01 should be addressed before merge: clean-install imports, routing back through create_prompt, unknown-tool handling, tool/argument failures, workspace creation, and the symlink confinement issue.

Two additional requirements:

1. examples/coding-agent/README.md needs the standard ASF license header; otherwise Apache RAT will reject the release.
2. This PR should include automated tests for the scripted happy path, step-budget termination, unknown tools, invalid tool arguments/tool failures, and workspace confinement. Testing the harness was part of the original issue, and currently CI does not exercise it.

The inactivity bot removed the assignment today, but I’d still like to move this contribution forward. Are you interested in making this follow-up? If useful, I’m also happy to push a cleanup commit to your branch since maintainer edits are enabled.

[haigou-web]

Following up on the review above — I opened a PR against this branch with the fixes and the tests:

Agentic-JJ-Web3#1 (base: docs/coding-agent-harness-example)

It covers all seven findings from @DeepGandhi01 and the two requirements from @skrawcz:

Finding	Fix
1. Clean-install imports	dropped the unused requests / openai / inspect / json / typing / burr.core imports and the duplicate os in tools.py
2. create_prompt only ran once	tool actions now route back through create_prompt, so the seam runs before every model call
3. Unknown tool halted the run	the error is fed back to the model as a tool message and the loop retries
4. Tool errors crashed the run	execute_tool catches and returns an error result; run_bash creates the workspace first
5. Malformed tool-call JSON	JSONDecodeError is caught and surfaced as a tool error
6. _resolve symlink escape	realpath for both root and full
7. Graphviz nit	visualize() wrapped in try/except
ASF header	added to README.md
Tests	new test_coding_agent.py — 10 tests, driven by ScriptedClient, no API key needed

Verified locally: pytest test_coding_agent.py → 10 passed; pytest validate_examples.py → 48 passed; python application.py runs clean without credentials; black 23.11.0 / isort 5.12.0 / flake8 6.1.0 clean.

The PR's base is this branch rather than main, so its diff is only the follow-up — the example itself is unchanged from #886.

Happy to adjust anything, and if it's easier to fold this straight into #886 rather than merging the follow-up, that works too.

[Agentic-JJ-Web3]

Yes, I’d like to make the follow-up — apologies for the slow response. @DeepGandhi01’s findings are all fair and I’ll work through them: the imports, routing tool actions back through create_prompt, unknown-tool handling, catching tool and argument failures, creating the workspace up front, realpath in _resolve, and the graphviz guard. Plus the ASF header and the test file. Should have something up this week.

[Agentic-JJ-Web3]

Thanks both — @DeepGandhi01 that review was genuinely useful, and all seven
findings were real.

Pushed fixes for everything, plus the two requirements:

Finding	Fix
1. Clean-install imports	removed the unused imports from tools.py; verified with a fresh venv built only from requirements.txt
2. create_prompt ran once	tool actions now route back through create_prompt; there's a test asserting it runs twice for a one-tool run
3. Unknown tool halted silently	the error goes back to the model as a tool message so it retries, plus a default transition to respond as a safety net
4. Tool errors crashed the run	execute_tool catches and returns {"error": ...}; workspace is created at import so run_bash works first
5. Malformed tool-call JSON	caught and surfaced as a bad argument
6. _resolve symlink escape	realpath for both root and full
7. Graphviz nit	visualize() wrapped in try/except
ASF header	added to README.md
Tests	test_coding_agent.py — 10 tests via ScriptedClient, no API key

Also regenerated notebook.ipynb from the fixed sources — it inlined the old
code, so it would have shipped the bugs this PR removes.

Verified: pytest test_coding_agent.py → 10 passed; validate_examples.py →
passes for coding-agent; pre-commit clean; python application.py runs
without credentials.

@haigou-web thanks for the offer on Agentic-JJ-Web3#1 — I've worked through
these directly, so I'll close that one.

[Agentic-JJ-Web3]

The documentation workflow shows a startup failure — that’s the ASF allowlist rejecting hasura/comment-progress@v2.2.0 before any job runs, not anything in this change. Looks like it affects all PRs.

[Agentic-JJ-Web3]

CI flagged two things in this PR, both fixed in the latest push:

• tests/test_release_config.py requires every example directory to be listed
  in pyproject.toml's [tool.flit.sdist]. I added examples/coding-agent/**
  to the exclude list, since it isn't one of the five release examples.
  Happy to move it to include if you'd prefer it shipped.
• requirements.txt was missing a trailing newline (end-of-file-fixer).

The documentation workflow still fails at startup. That's the ASF allowlist
rejecting hasura/comment-progress@v2.3.0, and it affects every PR.

[Copilot]

🟡 Changes recommended

Invalid write contents can truncate existing files before validation, and timeout handling can leave child processes running and modifying files.

2 open issues

• Validate file contents before truncating destination · New
• Terminate timed-out commands' child processes · New

What changed in this PR

Adds a coding-agent example for #885, showing how Burr models an autonomous tool-calling loop with a step budget and tracked execution. CLI and permission controls remain separate follow-up work.

Changes:

• Adds four separately bound tools, prompt preparation, and recoverable tool-error handling.
• Supports OpenAI and a scripted client for runs without an API key.
• Includes a notebook walkthrough, tests, and setup and safety documentation.

File	Description
pyproject.toml	Adds the example to an existing exclusion list.
examples/​coding-agent/​tools.py	Implements workspace file operations and shell execution.
examples/​coding-agent/​test_coding_agent.py	Tests the loop, termination, error recovery, and workspace boundaries.
examples/​coding-agent/​requirements.txt	Declares Burr and OpenAI dependencies.
examples/​coding-agent/​README.md	Explains the graph, setup, safety limits, and simplifications.
examples/​coding-agent/​notebook.ipynb	Walks through the harness with a recorded example run.
examples/​coding-agent/​application.py	Defines clients, actions, graph transitions, and tracking.
examples/​coding-agent/​__init__.py	Adds the package marker and license header.

_{Review effort: Balanced}

[haigou-web]

Checked the current head (2b30eb27) against main and found two things the Copilot pass didn't cover. Both are in examples/coding-agent/tools.py.

1. write_file truncates the file before validating contents (High)

open(full, "w") truncates the target file before f.write(contents) is reached. If the model calls the tool with contents as a list or dict — easy with a small local model — the TypeError fires after truncation, so the file is left empty. And except (ValueError, OSError) does not catch TypeError, so it escapes the tool and kills the run:

TypeError: write() argument must be str, not list

Net effect: silent data loss plus a crashed run. run_bash will happily cat the now-empty file on the next turn and the agent has no way to know it destroyed its own work.

 def write_file(path: str, contents: str) -> dict:
     """Writes text to a file in the workspace, creating or overwriting it."""
+    if not isinstance(contents, str):
+        return {"error": f"contents must be a string, got {type(contents).__name__}"}
     try:
         full = _resolve(path)
         os.makedirs(os.path.dirname(full), exist_ok=True)
-        with open(full, "w") as f:
+        with open(full, "w", encoding="utf-8") as f:
             f.write(contents)
         return {"path": path, "bytes_written": len(contents)}
     except (ValueError, OSError) as e:
         return {"error": str(e)}

2. run_bash timeout does not kill the command (Medium)

subprocess.run(..., shell=True, timeout=30) kills the shell, not the process the shell started. With shell=True the direct child is sh -c "<command>", so on timeout the real command survives:

>>> run_bash("sleep 300 & echo started")   # returns "timed out after 30s"
>>> # the sleep is still alive afterwards

Worse: after TimeoutExpired, subprocess.run calls communicate() again without a timeout, and the orphaned child still holds the stdout/stderr pipes — so the call blocks until the child exits and the 30s budget is not actually enforced. The existing tests don't catch this because nothing they run outlives the timeout.

+import signal
 import subprocess

+BASH_TIMEOUT_SECONDS = 30
+
+
 def run_bash(command: str) -> dict:
     """Runs a shell command in the workspace and returns its output."""
-    try:
-        proc = subprocess.run(
-            command,
-            shell=True,
-            cwd=os.path.abspath(WORKSPACE),
-            capture_output=True,
-            text=True,
-            timeout=30,
-        )
+    proc = subprocess.Popen(
+        command,
+        shell=True,
+        cwd=os.path.abspath(WORKSPACE),
+        stdout=subprocess.PIPE,
+        stderr=subprocess.PIPE,
+        text=True,
+        start_new_session=(os.name != "nt"),
+        creationflags=(subprocess.CREATE_NEW_PROCESS_GROUP if os.name == "nt" else 0),
+    )
+    try:
+        stdout, stderr = proc.communicate(timeout=BASH_TIMEOUT_SECONDS)
+    except subprocess.TimeoutExpired:
+        _kill_tree(proc)
+        stdout, stderr = proc.communicate()
         return {
-            "exit_code": proc.returncode,
-            "stdout": proc.stdout[-4000:],
-            "stderr": proc.stderr[-4000:],
+            "error": f"command timed out after {BASH_TIMEOUT_SECONDS}s",
+            "stdout": (stdout or "")[-4000:],
+            "stderr": (stderr or "")[-4000:],
         }
-    except subprocess.TimeoutExpired:
-        return {"error": "command timed out after 30s"}
+    return {
+        "exit_code": proc.returncode,
+        "stdout": stdout[-4000:],
+        "stderr": stderr[-4000:],
+    }
+
+
+def _kill_tree(proc: subprocess.Popen) -> None:
+    """Kills the process and everything it spawned, then reaps it."""
+    if proc.poll() is not None:
+        return
+    if os.name == "nt":
+        subprocess.run(
+            ["taskkill", "/F", "/T", "/PID", str(proc.pid)],
+            capture_output=True,
+            check=False,
+        )
+    else:
+        try:
+            os.killpg(os.getpgid(proc.pid), signal.SIGKILL)
+        except ProcessLookupError:
+            pass
+    try:
+        proc.wait(timeout=5)
+    except subprocess.TimeoutExpired:
+        pass

Regression tests worth pinning both fixes:

def test_write_file_rejects_non_string_without_truncating(tmp_path, monkeypatch):
    monkeypatch.setenv("CODING_AGENT_WORKSPACE", str(tmp_path))
    # ... rebuild the tool module here, as the other tests do ...
    write_file("notes.txt", "original")
    assert "error" in write_file("notes.txt", ["not", "a", "string"])
    assert (tmp_path / "notes.txt").read_text() == "original"


def test_timeout_kills_child_processes(tmp_path, monkeypatch):
    monkeypatch.setenv("CODING_AGENT_WORKSPACE", str(tmp_path))
    # ...
    marker = tmp_path / "survivor"
    assert "error" in run_bash(f"sh -c 'sleep 60; touch {marker}' &")
    time.sleep(1)
    assert not marker.exists()

black --line-length=100 / isort / flake8 stay clean with both patches, and the 10 existing tests still pass (the two symlink/pwd failures on Windows are platform limits that don't apply on CI).

Happy to open a follow-up PR against this branch if that's easier than applying it here — just say the word.