Skip to content

Commit 741aee9

Browse files
committed
Refactor tools so that all except bash and agent execute one by one like gptel.
1 parent 66a2d6e commit 741aee9

12 files changed

Lines changed: 286 additions & 255 deletions

File tree

README.md

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,11 @@ A Python port of the Emacs [gptel-agent-harness](https://github.com/beacoder/gpt
2222
- **Tools** — Agent (sub-agents), TodoWrite, Glob (git-aware), Grep
2323
(git grep → rg → grep), Read, Insert, Edit (incl. unified diffs), Write,
2424
Mkdir, Bash, Skill, Question, and PlanExit (registered while in plan
25-
mode) — all OpenAI-compatible tool schemas. Every tool call issued
26-
in one round runs concurrently in a thread pool (up to
27-
`PARALLEL_TOOL_MAX`, default 8) — Agent calls included — with results
28-
delivered in the original call order.
25+
mode) — all OpenAI-compatible tool schemas. Tool execution mirrors
26+
gptel's `gptel--handle-tool-use`: synchronous tools (Read, Edit,
27+
Glob, ...) run ONE AT A TIME in model-emitted order, while the
28+
asynchronous tools (Bash, Agent) are dispatched and run concurrently
29+
in the background — results are delivered in the original call order.
2930
- **Default agent prompts** — the main agent and sub-agents each get a
3031
distinct default system prompt bundled with the package
3132
(`prompts/agent.md`, `prompts/subagent.md`), with YAML frontmatter

python_agent_harness/agent.py

Lines changed: 62 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@
1717
- tool results are sanitized (None -> error placeholder, non-str -> str)
1818
- tool-call batches never strand the machine: failures become error
1919
results
20-
- every tool call in a round runs concurrently in a thread pool (results
21-
delivered in original order); async tools (e.g. Bash) return a
22-
``PendingToolResult`` and deliver their result when the work completes,
23-
without occupying a pool slot while waiting; interactive prompts stay
24-
serialized
20+
- tool execution mirrors gptel's `gptel--handle-tool-use': synchronous
21+
tools (Read, Edit, Glob, ...) run ONE AT A TIME in model-emitted
22+
order; asynchronous tools (Bash, Agent) return a ``PendingToolResult``
23+
and run concurrently in the background, their results awaited
24+
afterwards in original call order; interactive prompts stay serialized
2525
- token calibration is updated from API-reported input tokens
2626
- sessions are auto-saved after each response
2727
- a cancelled run with no successor salvages its partial history
@@ -303,7 +303,7 @@ def compact(self) -> bool:
303303
# ------------------------------------------------------------------
304304
# tool execution
305305
# ------------------------------------------------------------------
306-
def _execute_tool_call(self, call: ToolCall) -> str:
306+
def _execute_tool_call(self, call: ToolCall) -> str | PendingToolResult:
307307
if not self.top_level and call.name in config.SUBAGENT_EXCLUDED_TOOLS:
308308
# defense in depth: a hallucinated call must never reach the
309309
# registry — the spec was filtered, so refuse it here too
@@ -351,64 +351,69 @@ def _deliver_tool_result(self, p: ToolCall, result: str) -> None:
351351
# conversation history (the TUI renders from it)
352352
self.session.last_messages = list(self.messages)
353353

354-
def _run_tools_parallel(
354+
def _run_tools(
355355
self, calls: list[ToolCall], results: dict[str, str]
356356
) -> None:
357-
"""Run CALLS concurrently in a thread pool, filling RESULTS.
358-
359-
Each call executes in its own worker thread: the session's
360-
shared state is concurrency-safe (thread-local diff slots,
361-
serialized interactive prompts), so every
362-
tool issued in the same round — Agent calls included, whose
363-
sub-agents are isolated by design — runs in parallel. Delivery
364-
happens later, in original tool-call order, by the parent
365-
thread.
357+
"""Run CALLS in model-emitted order, filling RESULTS.
358+
359+
Mirrors gptel's `gptel--handle-tool-use': synchronous tools
360+
(Read, Edit, Glob, ...) execute ONE AT A TIME, in call order;
361+
asynchronous tools (Bash, Agent — those whose ``run`` returns a
362+
``PendingToolResult``) are dispatched in line and run
363+
concurrently in the background, their results awaited
364+
afterwards, again in original call order. Delivery happens
365+
later, in original tool-call order, by the caller.
366+
367+
A cancel landing before a call starts skips it (tools have side
368+
effects); a call already running — or an async tool already
369+
dispatched — cannot be stopped, but its result stays local to
370+
the (dead) run.
366371
"""
367-
from concurrent.futures import ThreadPoolExecutor, as_completed
368-
369-
def run_one(p: ToolCall) -> str:
370-
# A cancel landing while the task is still QUEUED must skip
371-
# it (tools have side effects): the sequential loop used to
372-
# check before every call, so keep that guarantee — a
373-
# task already RUNNING cannot be stopped, but one that has
374-
# not started yet must not run after Ctrl-C.
372+
async_calls: list[tuple[ToolCall, PendingToolResult]] = []
373+
for p in calls:
374+
# A cancel landing while a call is still QUEUED must skip
375+
# it (tools have side effects): the sequential loop checks
376+
# before every call, so a call that has not started yet must
377+
# not run after Ctrl-C.
375378
if self._is_cancelled():
376-
return "Error: tool call cancelled (user aborted the run)."
377-
return self._execute_tool_call(p)
378-
379-
with ThreadPoolExecutor(
380-
max_workers=min(len(calls), config.PARALLEL_TOOL_MAX),
381-
thread_name_prefix="tool",
382-
) as pool:
383-
futures = {pool.submit(run_one, p): p for p in calls}
384-
for fut in as_completed(futures):
385-
p = futures[fut]
386-
try:
387-
result = fut.result()
388-
if isinstance(result, PendingToolResult):
389-
# async tool (e.g. Bash): the worker returned
390-
# its handle as soon as the work was spawned and
391-
# freed its pool slot; wait for the real result
392-
# here (delivered when the process exits)
393-
result = result.wait()
394-
results[p.id] = sanitize_tool_result(result)
395-
except Exception as e: # noqa: BLE001 - containment boundary
396-
results[p.id] = (
397-
f"Error: tool {p.name!r} crashed in a worker "
398-
f"thread — {e}"
399-
)
379+
results[p.id] = (
380+
"Error: tool call cancelled (user aborted the run)."
381+
)
382+
continue
383+
try:
384+
result = self._execute_tool_call(p)
385+
except Exception as e: # noqa: BLE001 - containment boundary
386+
results[p.id] = (
387+
f"Error: tool {p.name!r} crashed during execution — {e}"
388+
)
389+
continue
390+
if isinstance(result, PendingToolResult):
391+
# async tool (e.g. Bash): run() spawned the work and
392+
# returned its handle immediately; await the real result
393+
# after the sequential loop so sibling calls keep
394+
# executing in the meantime
395+
async_calls.append((p, result))
396+
else:
397+
results[p.id] = sanitize_tool_result(result)
398+
for p, pending in async_calls:
399+
try:
400+
results[p.id] = sanitize_tool_result(pending.wait())
401+
except Exception as e: # noqa: BLE001 - containment boundary
402+
results[p.id] = (
403+
f"Error: tool {p.name!r} crashed during execution — {e}"
404+
)
400405

401406
def _execute_pending(self) -> None:
402-
"""TOOL state: run the round's pending tool calls concurrently.
407+
"""TOOL state: run the round's pending tool calls.
403408
404409
The assistant message carrying the tool calls was already
405410
appended by the WAIT state. Results land in
406411
``self.info["tool_result"]`` and are delivered by the TRET
407412
state in original tool-call order.
408413
409-
All tools issued in the round run CONCURRENTLY in a thread
410-
pool — the session's shared state is concurrency-safe
411-
(thread-local diff slots, serialized interactive prompts).
414+
Synchronous tools run ONE AT A TIME in model-emitted order
415+
(gptel-style); asynchronous tools (Bash, Agent) are dispatched
416+
in line and run concurrently in the background.
412417
"""
413418
pending = list(self.pending)
414419
if not pending:
@@ -420,7 +425,7 @@ def _execute_pending(self) -> None:
420425
# run's `session.last_messages`.
421426
return
422427
results: dict[str, str] = {}
423-
self._run_tools_parallel(pending, results)
428+
self._run_tools(pending, results)
424429
if self._is_cancelled():
425430
# cancelled mid-round: tools already submitted may have run
426431
# (their side effects are done), but the results stay local
@@ -457,7 +462,8 @@ def _deliver_results(self) -> None:
457462
self.pending = []
458463

459464
def _run_tool_round(self) -> None:
460-
"""Execute all pending tool calls concurrently; deliver results.
465+
"""Execute all pending tool calls (sync one at a time, async
466+
dispatched); deliver results.
461467
462468
Convenience wrapper around the FSM's TOOL (execute) and TRET
463469
(deliver) handlers, kept for direct callers and tests; the
@@ -663,9 +669,8 @@ def safe_delta(text: str) -> None:
663669
)
664670

665671
def _handle_tool(self) -> None:
666-
"""TOOL — run the round's tools concurrently (see
667-
``_execute_pending``); the table routes ABRT on cancel and TRET
668-
otherwise."""
672+
"""TOOL — run the round's tools (see ``_execute_pending``); the
673+
table routes ABRT on cancel and TRET otherwise."""
669674
self.pending = list(self.info["tool_calls"])
670675
self.supervisor.reset_nudges()
671676
# Notify the TUI that tool execution is starting: this clears

python_agent_harness/agent_session.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,15 +98,13 @@ def __init__(
9898
self.plan_mode = PlanMode(project_dir)
9999
self.tool_ctx = ToolContext(self)
100100
self._tool_diffs: dict[str, str] = {}
101-
# thread-local: parallel sub-agents each execute tools in their
102-
# own pool thread; the "currently executing call" that Edit/Write
103-
# attach their diff to must be per-thread, or concurrent
104-
# sub-agents would clobber each other's diff slot
101+
# thread-local: sub-agents each execute tools in their own
102+
# background thread; the "currently executing call" that
103+
# Edit/Write attach their diff to must be per-thread, or
104+
# concurrent sub-agents would clobber each other's diff slot
105105
self._active_call = threading.local()
106106
# serializes interactive prompts (Question tool, PlanExit
107-
# confirmation): parallel tool rounds may hit them
108-
# simultaneously, but the TUI can only ask one question at a
109-
# time
107+
# confirmation): the TUI can only ask one question at a time
110108
self._interactive_lock = threading.Lock()
111109
self.store = SessionStore(
112110
project_dir=project_dir,

python_agent_harness/config.py

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,11 @@
100100

101101
# ---- tool execution ----------------------------------------------------------
102102
SUBAGENT_MAX_ROUNDS = 60
103-
# Max tool calls that may run CONCURRENTLY in one tool round (all tools
104-
# issued together in a round — Agent calls included — execute in
105-
# parallel; excess calls queue).
106-
PARALLEL_TOOL_MAX = 8
103+
# Tool execution mirrors gptel's `gptel--handle-tool-use': synchronous
104+
# tools (Read, Edit, Glob, ...) run ONE AT A TIME in model-emitted
105+
# order; asynchronous tools (Bash, Agent) are dispatched in line and
106+
# run concurrently in the background, their results awaited afterwards
107+
# in original call order.
107108
# Tools a sub-agent must NOT see or call: it runs autonomously as a
108109
# one-shot task inside the parent's tool round, so it cannot spawn
109110
# further sub-agents (Agent), ask the user questions (Question), nor

python_agent_harness/tools/agent_tool.py

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,19 @@
33
Asynchronous (mirrors ``:async t``): ``run`` returns a
44
``PendingToolResult`` immediately and a background thread runs the
55
sub-agent loop, delivering the result string when it finishes — a
6-
long-running sub-agent never occupies a thread-pool slot.
6+
long-running sub-agent never blocks the parent's sequential tool loop.
77
88
Sub-agents run the same agent loop with a fresh loop instance; their
99
backend/model can be overridden (see config). Results flow back to the
1010
parent as a single tool result string. Errors are contained: an
1111
unexpected sub-agent response becomes an error string fed to the parent,
1212
never a crash.
1313
14-
Every tool call in a round — Agent calls included — runs CONCURRENTLY:
15-
each sub-agent is fully isolated (own loop, own history, own stream), so
16-
independent tasks can be delegated in parallel.
14+
Tool execution mirrors gptel: synchronous tools (Read, Edit, ...) run
15+
ONE AT A TIME in model-emitted order, while asynchronous tools — Agent
16+
and Bash — are dispatched in line and run concurrently in the
17+
background. Each sub-agent is fully isolated (own loop, own history,
18+
own stream), so independent tasks can be delegated in parallel.
1719
"""
1820

1921
from __future__ import annotations
@@ -27,9 +29,9 @@
2729
"autonomously. Sub-agents run independently and return results in one "
2830
"message. Use for open-ended searches, complex research, or when "
2931
"uncertain about finding results in the first few tries.\n\n"
30-
"Tool calls issued in the same round — including multiple Agent "
31-
"calls — run concurrently, so delegate independent tasks in parallel "
32-
"for efficiency."
32+
"Multiple Agent calls issued in the same round run concurrently (like "
33+
"Bash), while other tools execute one by one — delegate independent "
34+
"tasks in parallel for efficiency."
3335
)
3436

3537
PARAMETERS = {

python_agent_harness/tools/base.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class PendingToolResult:
1515
An async tool's ``run`` returns this handle instead of a string: it
1616
starts its background work (e.g. a spawned process) and returns
1717
immediately, then delivers the final result string later via
18-
``deliver`` — so the wait never occupies a thread-pool slot.
18+
``deliver`` — so the wait never blocks the sequential tool loop.
1919
2020
``deliver`` is idempotent (first delivery wins, late duplicates are
2121
no-ops — mirroring the gptel-agent FSM's idempotent-result advice);

python_agent_harness/tools/bash.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33
Asynchronous (mirrors ``:async t`` in gptel-agent-tools): ``run``
44
spawns the process and returns a ``PendingToolResult`` immediately; a
55
background thread collects the output and delivers it when the process
6-
exits. A long-running command therefore never occupies a thread-pool
7-
slot — sibling tools keep their slots and the round completes by
8-
delivery, not by thread blocking.
6+
exits. A long-running command therefore never blocks the parent's
7+
sequential tool loop — it runs concurrently with sibling async tools
8+
(Agent) while sync tools execute one at a time.
99
1010
A session cancel (Ctrl-C) kills the process group and delivers a
1111
cancelled error.
Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
"""PlanExit tool: ask the user to approve switching from plan to build.
22
3-
Asynchronous (mirrors ``:async t``): ``run`` returns a
4-
``PendingToolResult`` immediately and a background thread asks for the
5-
approval, delivering the outcome when the user responds — the wait for
6-
user input never occupies a thread-pool slot.
3+
Synchronous (mirrors gptel's PlanExit tool, which is NOT ``:async t``):
4+
``run`` blocks until the user answers and returns the outcome as a
5+
plain string — it executes one at a time, in call order, like every
6+
other non-Bash/non-Agent tool.
77
"""
88

99
from __future__ import annotations
1010

11-
import threading
12-
13-
from .base import PendingToolResult, Tool, ToolContext
11+
from .base import Tool, ToolContext
1412

1513
DESCRIPTION = (
1614
"Use this tool when you have completed the planning phase and are "
@@ -38,17 +36,10 @@ class PlanExit(Tool):
3836
description = DESCRIPTION
3937
parameters = {"type": "object", "properties": {}}
4038

41-
def run(self, args: dict, ctx: ToolContext) -> str | PendingToolResult:
42-
pending = PendingToolResult()
43-
44-
def worker() -> None:
45-
# containment boundary: a failure in the approval prompt
46-
# becomes an error string for the model, never a crash
47-
try:
48-
result = ctx.plan_exit()
49-
except Exception as e: # noqa: BLE001 - error string for the model
50-
result = f"Error: PlanExit failed — {e}"
51-
pending.deliver(result)
52-
53-
threading.Thread(target=worker, daemon=True, name="planexit-tool").start()
54-
return pending
39+
def run(self, args: dict, ctx: ToolContext) -> str:
40+
# containment boundary: a failure in the approval prompt
41+
# becomes an error string for the model, never a crash
42+
try:
43+
return ctx.plan_exit()
44+
except Exception as e: # noqa: BLE001 - error string for the model
45+
return f"Error: PlanExit failed — {e}"

python_agent_harness/tools/question.py

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,14 @@
11
"""Question tool: ask the user one or more questions during execution.
22
3-
Asynchronous (mirrors ``:async t``): ``run`` returns a
4-
``PendingToolResult`` immediately and a background thread presents the
5-
questions, delivering the answers when the user responds — the wait for
6-
user input never occupies a thread-pool slot.
3+
Synchronous (mirrors gptel's Question tool, which is NOT ``:async t``):
4+
``run`` blocks until the user answers and returns the answers as a
5+
plain string — it executes one at a time, in call order, like every
6+
other non-Bash/non-Agent tool.
77
"""
88

99
from __future__ import annotations
1010

11-
import threading
12-
13-
from .base import PendingToolResult, Tool, ToolContext
11+
from .base import Tool, ToolContext
1412

1513
DESCRIPTION = (
1614
"Ask the user one or more questions during execution.\n\n"
@@ -55,7 +53,7 @@ class Question(Tool):
5553
description = DESCRIPTION
5654
parameters = PARAMETERS
5755

58-
def run(self, args: dict, ctx: ToolContext) -> str | PendingToolResult:
56+
def run(self, args: dict, ctx: ToolContext) -> str:
5957
raw = args.get("questions")
6058
if isinstance(raw, list):
6159
questions = raw
@@ -64,16 +62,9 @@ def run(self, args: dict, ctx: ToolContext) -> str | PendingToolResult:
6462
else:
6563
return "Error: questions must be an array"
6664

67-
pending = PendingToolResult()
68-
69-
def worker() -> None:
70-
# containment boundary: a failure in the interactive prompt
71-
# becomes an error string for the model, never a crash
72-
try:
73-
result = ctx.ask_questions(questions)
74-
except Exception as e: # noqa: BLE001 - error string for the model
75-
result = f"Error: Question failed — {e}"
76-
pending.deliver(result)
77-
78-
threading.Thread(target=worker, daemon=True, name="question-tool").start()
79-
return pending
65+
# containment boundary: a failure in the interactive prompt
66+
# becomes an error string for the model, never a crash
67+
try:
68+
return ctx.ask_questions(questions)
69+
except Exception as e: # noqa: BLE001 - error string for the model
70+
return f"Error: Question failed — {e}"

0 commit comments

Comments
 (0)