Skip to content

Commit c324c38

Browse files
committed
fix: make coder approvals replay-safe
1 parent 016e5f2 commit c324c38

5 files changed

Lines changed: 305 additions & 20 deletions

File tree

agent/coding/repository_tools.py

Lines changed: 33 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class PreparedRepository:
3030
head_branch: str
3131
identity: GitHubIdentity
3232
pr_number: int | None
33+
request_signature: tuple[Any, ...]
3334
approved_signature: tuple[Any, ...] | None = None
3435
pushed_commit: str | None = None
3536

@@ -71,6 +72,20 @@ def _run_git(backend: PerJobDaytonaBackend, path: str, args: str):
7172
return backend.execute(f"git -C {shlex.quote(path)} {args}")
7273

7374

75+
def _prepared_result(prepared: PreparedRepository, *, replayed: bool = False) -> str:
76+
status = "already_prepared" if replayed else "prepared"
77+
return (
78+
f"status: {status}\n"
79+
f"repository: {prepared.repo}\n"
80+
f"push_repository: {prepared.push_repo}\n"
81+
f"working_directory: {prepared.path}\n"
82+
f"base_branch: {prepared.base_branch}\n"
83+
f"head_branch: {prepared.head_branch}\n"
84+
f"actor: {prepared.identity.login}\n"
85+
f"pr_number: {prepared.pr_number or ''}"
86+
)
87+
88+
7489
def build_repository_tools(
7590
backend: PerJobDaytonaBackend,
7691
provider: GitHubCredentialProvider,
@@ -94,16 +109,29 @@ def prepare_repository(
94109
_validate_branch(base_branch)
95110
if head_branch:
96111
_validate_branch(head_branch)
112+
if pr_number is not None and pr_number < 1:
113+
raise RuntimeError("pr_number must be positive")
114+
115+
request_signature = (
116+
repo.casefold(),
117+
base_branch,
118+
head_branch,
119+
pr_number,
120+
sync_base,
121+
)
97122

98123
state = backend.job_state()
99-
if state.get("repository") is not None:
124+
prior = state.get("repository")
125+
if isinstance(prior, PreparedRepository):
126+
if prior.request_signature == request_signature:
127+
return _prepared_result(prior, replayed=True)
100128
raise RuntimeError("prepare_repository may be called only once per coder job")
129+
if prior is not None:
130+
raise RuntimeError("coder job contains invalid repository state")
101131

102132
push_repo = repo
103133
existing_pr: dict[str, Any] | None = None
104134
if pr_number is not None:
105-
if pr_number < 1:
106-
raise RuntimeError("pr_number must be positive")
107135
existing_pr = provider.request_json(
108136
"GET", f"/repos/{repo}/pulls/{pr_number}"
109137
)
@@ -187,18 +215,10 @@ def prepare_repository(
187215
head_branch=head_branch,
188216
identity=identity,
189217
pr_number=pr_number,
218+
request_signature=request_signature,
190219
)
191220
state["repository"] = prepared
192-
return (
193-
"status: prepared\n"
194-
f"repository: {repo}\n"
195-
f"push_repository: {push_repo}\n"
196-
f"working_directory: {path}\n"
197-
f"base_branch: {base_branch}\n"
198-
f"head_branch: {head_branch}\n"
199-
f"actor: {identity.login}\n"
200-
f"pr_number: {pr_number or ''}"
201-
)
221+
return _prepared_result(prepared)
202222

203223
@tool
204224
def publish_changes(

agent/coding/sandbox.py

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -60,20 +60,24 @@
6060

6161

6262
def current_run_id() -> str:
63-
"""LangGraph run id, or thread id, or `unknown`."""
63+
"""Stable coder-job id across nested tool calls and approval resumes."""
6464
try:
6565
from langgraph.config import get_config
6666

6767
config = get_config()
6868
configurable = config.get("configurable") or {}
6969
except Exception:
7070
return "unknown"
71-
return str(
72-
config.get("run_id")
73-
or configurable.get("run_id")
74-
or configurable.get("thread_id")
75-
or "unknown"
76-
)
71+
run_id = config.get("run_id") or configurable.get("run_id")
72+
if run_id:
73+
return str(run_id)
74+
75+
thread_id = configurable.get("thread_id")
76+
checkpoint_ns = str(configurable.get("checkpoint_ns") or "")
77+
job_ns = checkpoint_ns.partition("|")[0]
78+
if job_ns:
79+
return f"{thread_id}:{job_ns}" if thread_id else job_ns
80+
return str(thread_id or "unknown")
7781

7882

7983
def _raw_sandbox(box):
Lines changed: 223 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,223 @@
1+
import asyncio
2+
from types import SimpleNamespace
3+
from typing import Any
4+
5+
from ag_ui.core import RunAgentInput
6+
from copilotkit import CopilotKitMiddleware
7+
from deepagents import create_deep_agent
8+
from langchain_core.language_models import BaseChatModel
9+
from langchain_core.messages import AIMessage, BaseMessage, ToolMessage
10+
from langchain_core.outputs import ChatGeneration, ChatResult
11+
from langgraph.checkpoint.memory import MemorySaver
12+
from pydantic import Field
13+
14+
from agui import build_agui_agent
15+
from coding.github_credentials import GitHubIdentity
16+
from coding.subagent import build_coder_subagent
17+
18+
19+
class ApprovalResumeModel(BaseChatModel):
20+
tool_names: frozenset[str] = Field(default_factory=frozenset)
21+
22+
@property
23+
def _llm_type(self):
24+
return "coder-approval-resume"
25+
26+
def bind_tools(self, tools, **_kwargs):
27+
return self.model_copy(
28+
update={"tool_names": frozenset(tool.name for tool in tools)}
29+
)
30+
31+
def _generate(
32+
self,
33+
messages: list[BaseMessage],
34+
stop=None,
35+
run_manager=None,
36+
**_kwargs: Any,
37+
):
38+
del stop, run_manager
39+
results = {
40+
message.tool_call_id
41+
for message in messages
42+
if isinstance(message, ToolMessage)
43+
}
44+
if "prepare_repository" not in self.tool_names:
45+
message = (
46+
AIMessage(content="done")
47+
if "task-1" in results
48+
else AIMessage(
49+
content="",
50+
tool_calls=[
51+
{
52+
"id": "task-1",
53+
"name": "task",
54+
"args": {
55+
"description": "Make the requested change",
56+
"subagent_type": "coder",
57+
},
58+
}
59+
],
60+
)
61+
)
62+
elif "prepare-1" not in results:
63+
message = AIMessage(
64+
content="",
65+
tool_calls=[
66+
{
67+
"id": "prepare-1",
68+
"name": "prepare_repository",
69+
"args": {
70+
"repo": "org/repo",
71+
"base_branch": "main",
72+
"head_branch": "opentag/test",
73+
},
74+
}
75+
],
76+
)
77+
elif "publish-1" not in results:
78+
message = AIMessage(
79+
content="",
80+
tool_calls=[
81+
{
82+
"id": "publish-1",
83+
"name": "publish_changes",
84+
"args": {
85+
"repo": "org/repo",
86+
"base_branch": "main",
87+
"head_branch": "opentag/test",
88+
"title": "Test approval resume",
89+
"body": "Test body",
90+
"test_command": "true",
91+
"test_exit_code": 0,
92+
},
93+
}
94+
],
95+
)
96+
else:
97+
message = AIMessage(content="coder done")
98+
return ChatResult(generations=[ChatGeneration(message=message)])
99+
100+
101+
class ApprovalResumeProvider:
102+
git_username = "x-access-token"
103+
104+
def __init__(self):
105+
self.requests = []
106+
107+
def token(self):
108+
return "operation-secret"
109+
110+
def identity(self):
111+
return GitHubIdentity(
112+
"open-tag[bot]",
113+
42,
114+
"42+open-tag[bot]@users.noreply.github.com",
115+
)
116+
117+
def request_json(self, method, path, *, json=None):
118+
self.requests.append((method, path, json))
119+
if method == "POST":
120+
return {"html_url": "https://github.com/org/repo/pull/9"}
121+
raise AssertionError((method, path, json))
122+
123+
124+
class ApprovalResumeBackend:
125+
def __init__(self):
126+
self.state = {}
127+
self.branch = ""
128+
self.pushes = 0
129+
130+
@property
131+
def id(self):
132+
return "approval-resume"
133+
134+
def job_state(self):
135+
return self.state
136+
137+
def clone_repository(self, **kwargs):
138+
self.branch = kwargs["branch"]
139+
140+
def set_git_identity(self, **_kwargs):
141+
pass
142+
143+
def push_repository(self, **_kwargs):
144+
self.pushes += 1
145+
146+
def execute(self, command, **_kwargs):
147+
if "switch -c" in command:
148+
self.branch = "opentag/test"
149+
return SimpleNamespace(output="", exit_code=0)
150+
if "branch --show-current" in command:
151+
return SimpleNamespace(output=self.branch, exit_code=0)
152+
if "status --porcelain" in command:
153+
return SimpleNamespace(output="", exit_code=0)
154+
if "rev-parse HEAD" in command:
155+
return SimpleNamespace(output="abc123", exit_code=0)
156+
raise AssertionError(command)
157+
158+
def stop_current(self):
159+
pass
160+
161+
162+
def test_coder_confirmation_survives_subagent_tool_replay():
163+
model = ApprovalResumeModel()
164+
checkpointer = MemorySaver()
165+
backend = ApprovalResumeBackend()
166+
provider = ApprovalResumeProvider()
167+
coder = build_coder_subagent(
168+
model=model,
169+
checkpointer=checkpointer,
170+
provider=provider,
171+
backend=backend,
172+
)
173+
graph = create_deep_agent(
174+
model=model,
175+
middleware=[CopilotKitMiddleware()],
176+
subagents=[coder],
177+
checkpointer=checkpointer,
178+
)
179+
agent = build_agui_agent(graph, recursion_limit=80)
180+
request = {
181+
"threadId": "approval-resume-thread",
182+
"state": {},
183+
"messages": [{"id": "user-1", "role": "user", "content": "go"}],
184+
"tools": [],
185+
"context": [],
186+
}
187+
188+
first = asyncio.run(
189+
_collect(
190+
agent.run(
191+
RunAgentInput(runId="run-1", forwardedProps={}, **request)
192+
)
193+
)
194+
)
195+
assert any(getattr(event, "name", None) == "on_interrupt" for event in first)
196+
197+
# A nested interrupt can replay the parent task if its subgraph checkpoint
198+
# is unavailable. The prepared sandbox state still survives that replay.
199+
namespaces = checkpointer.storage["approval-resume-thread"]
200+
for namespace in list(namespaces):
201+
if namespace:
202+
del namespaces[namespace]
203+
204+
asyncio.run(
205+
_collect(
206+
agent.run(
207+
RunAgentInput(
208+
runId="run-2",
209+
forwardedProps={
210+
"command": {"resume": {"confirmed": True}}
211+
},
212+
**request,
213+
)
214+
)
215+
)
216+
)
217+
218+
assert backend.pushes == 1
219+
assert provider.requests[-1][:2] == ("POST", "/repos/org/repo/pulls")
220+
221+
222+
async def _collect(stream):
223+
return [event async for event in stream]

agent/tests/test_repository_tools.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,26 @@ def test_prepare_clones_with_operation_token_and_configures_local_identity():
134134
assert backend.branch == "opentag/fix"
135135

136136

137+
def test_identical_prepare_replay_reuses_the_existing_repository():
138+
prepare, _publish_tool, backend, _provider = _tools()
139+
140+
_prepare(prepare)
141+
result = _prepare(prepare)
142+
143+
assert "status: already_prepared" in result
144+
assert len(backend.clone_calls) == 1
145+
146+
147+
def test_prepare_replay_cannot_change_the_target():
148+
prepare, _publish_tool, backend, _provider = _tools()
149+
150+
_prepare(prepare)
151+
152+
with pytest.raises(RuntimeError, match="may be called only once"):
153+
_prepare(prepare, head_branch="opentag/other")
154+
assert len(backend.clone_calls) == 1
155+
156+
137157
def test_prepare_existing_fork_pr_and_syncs_base_through_daytona():
138158
pr = {
139159
"base": {"ref": "main", "repo": {"full_name": "org/repo"}},

agent/tests/test_sandbox.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,24 @@ def test_current_run_id_falls_back_to_thread_id(monkeypatch):
6666
assert current_run_id() == "thread-a"
6767

6868

69+
def test_current_run_id_uses_the_parent_task_namespace_for_a_coder_job(monkeypatch):
70+
namespace = {"value": "tools:job-a|tools:prepare"}
71+
monkeypatch.setattr(
72+
"langgraph.config.get_config",
73+
lambda: {
74+
"configurable": {
75+
"thread_id": "thread-a",
76+
"checkpoint_ns": namespace["value"],
77+
}
78+
},
79+
)
80+
81+
prepared_job = current_run_id()
82+
namespace["value"] = "tools:job-a|tools:publish"
83+
84+
assert current_run_id() == prepared_job == "thread-a:tools:job-a"
85+
86+
6987
def test_redact_secrets_strips_github_token_prefixes():
7088
text = "auth ghp_ABCDEFG123 github_pat_ZZ gho_YY"
7189
redacted = redact_secrets(text)

0 commit comments

Comments
 (0)