smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(python-robomp): added DirtyState and inspect_dirty_state in git_ops

Browse Source 
- Added `DirtyState` and `inspect_dirty_state` in `git_ops` to report uncommitted, unpushed, and summary state.
- Updated `_drive_turn` to recheck completion each cycle and emit dirty-state reminders or exit when clean.
- Wired `dirty_state_reminder` into persona rendering with `uncommitted`, `unpushed`, and `summary` context.
- Added `dirty_state_reminder.md` guidance for fixing/restoring changes and pushing only after `bun run fix` completes.
- Added worker tests for dirty, clean, and persistent-dirty flows and prompt-count assertions.
This commit is contained in:
can1357
2026-05-19 18:39:09 +09:00
parent a17f74bcd6
commit 784ac0b6cf
5 changed files with 298 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
python/robomp/src/git_ops.py
+87
View File
@@ -445,6 +445,26 @@ class PushResult:
branch: str
@dataclass(slots=True, frozen=True)
class DirtyState:
"""Summary of the workspace's uncommitted + unpushed state.
`uncommitted` counts entries from `git status --porcelain`. `unpushed`
counts commits in `HEAD` not reachable from any `origin/*` ref — i.e.
commits that would be lost if the workspace were thrown away. `summary`
is a human-friendly multi-line description for embedding in a reminder
prompt; empty when both counts are zero.
"""
uncommitted: int
unpushed: int
summary: str
@property
def is_dirty(self) -> bool:
return self.uncommitted > 0 or self.unpushed > 0
class HeadDriftError(GitCommandError):
"""Raised when `expected_head` no longer matches the current HEAD.
@@ -479,6 +499,71 @@ def rev_parse_head(
return proc.stdout.strip()
def inspect_dirty_state(
repo_dir: Path,
*,
slot_uid: int | None = None,
safe_directory: Path | None = None,
) -> DirtyState:
"""Probe the worktree at `repo_dir` for uncommitted/unpushed work.
Returns a {@link DirtyState}. Errors from the underlying git invocations
are swallowed — the caller treats "we couldn't tell" as clean so a broken
git binary can't pin the agent in a reminder loop forever.
"""
slot_kwargs = _slot_subprocess_kwargs(slot_uid)
uncommitted = 0
uncommitted_sample: list[str] = []
status = _run_git(
["status", "--porcelain=v1", "--untracked-files=normal"],
cwd=repo_dir,
token=None,
safe_directory=safe_directory,
**slot_kwargs,
)
if status.returncode == 0 and status.stdout.strip():
lines = status.stdout.splitlines()
uncommitted = len(lines)
uncommitted_sample = lines[:10]
unpushed = 0
unpushed_sample: list[str] = []
count = _run_git(
["rev-list", "--count", "HEAD", "--not", "--remotes=origin"],
cwd=repo_dir,
token=None,
safe_directory=safe_directory,
**slot_kwargs,
)
if count.returncode == 0:
try:
unpushed = int(count.stdout.strip() or "0")
except ValueError:
unpushed = 0
if unpushed > 0:
log_proc = _run_git(
["log", f"--max-count={min(unpushed, 5)}", "--oneline", "HEAD", "--not", "--remotes=origin"],
cwd=repo_dir,
token=None,
safe_directory=safe_directory,
**slot_kwargs,
)
if log_proc.returncode == 0:
unpushed_sample = [line for line in log_proc.stdout.splitlines() if line.strip()]
if uncommitted == 0 and unpushed == 0:
return DirtyState(uncommitted=0, unpushed=0, summary="")
parts: list[str] = []
if uncommitted:
more = f"\n… and {uncommitted - len(uncommitted_sample)} more" if uncommitted > len(uncommitted_sample) else ""
parts.append(f"Uncommitted changes ({uncommitted}):\n" + "\n".join(uncommitted_sample) + more)
if unpushed:
log_text = "\n".join(unpushed_sample) if unpushed_sample else "(no log available)"
parts.append(f"Unpushed commits ({unpushed}):\n{log_text}")
return DirtyState(uncommitted=uncommitted, unpushed=unpushed, summary="\n\n".join(parts))
def push(
repo_dir: Path,
*,
@@ -556,12 +641,14 @@ def push(
__all__ = [
"AUTH_ENV_VAR",
"DirtyState",
"GitCommandError",
"HeadDriftError",
"PushResult",
"clone",
"fetch_prune",
"fetch_ref",
"inspect_dirty_state",
"push",
"redact_credentials",
"rev_parse_head",
python/robomp/src/persona.py
+31
View File
@@ -15,6 +15,7 @@ from functools import cache
from importlib import resources
from typing import Any
from robomp.git_ops import DirtyState
from robomp.github_client import CommentInfo, IssueInfo, RepoInfo
from robomp.sandbox import Workspace
@@ -145,6 +146,35 @@ def completion_reminder(*, repo: RepoInfo, issue: IssueInfo, workspace: Workspac
return render(_load("completion_reminder.md"), {"repo": repo, "issue": issue, "workspace": workspace})
def dirty_state_reminder(
*,
repo: RepoInfo,
issue: IssueInfo,
workspace: Workspace,
dirty: DirtyState,
) -> str:
"""Reminder injected when the worktree has uncommitted or unpushed work.
Fired by `worker._drive_turn` after the model emits a terminal turn but
leaves changes behind that roboomp would otherwise discard. The summary
embedded in the template comes from `git_ops.inspect_dirty_state` so the
agent sees the exact paths / commits it forgot about.
"""
return render(
_load("dirty_state_reminder.md"),
{
"repo": repo,
"issue": issue,
"workspace": workspace,
"dirty": {
"uncommitted": dirty.uncommitted,
"unpushed": dirty.unpushed,
"summary": dirty.summary,
},
},
)
def _render_thread(messages: tuple) -> str:
"""Render a `tuple[ThreadMessage, ...]` as a markdown block for prompt embed.
@@ -346,6 +376,7 @@ __all__ = [
"kickoff_directive",
"render",
"completion_reminder",
"dirty_state_reminder",
"resume_triage",
"seed_phases",
"system_append",
python/robomp/src/prompts/dirty_state_reminder.md
+17
View File
@@ -0,0 +1,17 @@
You ended your turn with unpushed work in the worktree.
Issue: {{repo.full_name}}#{{issue.number}} — {{issue.title}}
Branch: `{{workspace.branch}}`
Workspace state at end of your turn:
{{dirty.summary}}
Either of these counts being non-zero means roboomp will discard your work when this session ends. Read the summary above and act on it:
- **Uncommitted changes** → stage and commit them (or `git restore` if they were unintentional). Re-run `bun run fix` afterward — your last push refusal was a formatter failure, so the same gate will reject this push too if `fix` exits non-zero.
- **Unpushed commits** → call `gh_push_branch` once `bun run fix` succeeds. If the push still refuses for a different reason, fix that root cause; do not skip the gate.
If your fix is genuinely complete and the gates pass, push and then comment back on the PR with a one-line summary of what changed since the previous push. Do not re-classify the issue, do not re-post the original preamble, and do not call `abort_task` — this is recoverable.
You MUST end this turn either with a successful `gh_push_branch`, or with a clean worktree (no uncommitted changes, no commits ahead of `origin`) and an explanation in a comment.
python/robomp/src/worker.py
+73 -13
View File
@@ -33,6 +33,7 @@ from robomp import host_tools, persona, pragmas
from robomp.cancellation import register_cancel_hook, unregister_cancel_hook
from robomp.config import Settings
from robomp.db import Database, issue_key
from robomp.git_ops import DirtyState, inspect_dirty_state
from robomp.github_backend import GitHubBackend
from robomp.github_client import CommentInfo, IssueInfo, RepoInfo
from robomp.host_tools import AbortController, ToolBindings, _git_identity_env
@@ -215,6 +216,26 @@ def _needs_completion_reminder(
return not (tools_called & _TERMINAL_TRIAGE_TOOLS)
def _probe_workspace_dirty(workspace: Workspace, slot_uid: int | None) -> DirtyState:
"""Return the workspace's dirty state, swallowing inspection errors.
`inspect_dirty_state` already swallows individual git failures, but the
function itself can still raise if e.g. the workspace was wiped between
`_drive_turn` and the post-turn check. The reminder loop treats any
exception as "clean enough" so a corrupted workspace doesn't pin the
agent in a reminder loop.
"""
try:
return inspect_dirty_state(
workspace.repo_dir,
slot_uid=slot_uid,
safe_directory=workspace.repo_dir,
)
except Exception as exc: # noqa: BLE001 — best-effort post-turn probe
log.debug("workspace dirty probe failed", extra={"error": str(exc)})
return DirtyState(uncommitted=0, unpushed=0, summary="")
def _drive_turn(
client: RpcClient,
initial_prompt: str,
@@ -253,20 +274,46 @@ def _drive_turn(
return None
reminders_used = 0
while reminders_used < max_reminders and _needs_completion_reminder(
task_kind=task_kind, inputs=inputs, bindings=bindings, tools_called=tools_called
):
reminders_used += 1
log.warning(
"rpc_completion_reminder",
extra={
"issue": bindings.issue_key,
"task": task_kind,
"attempt": reminders_used,
"max": max_reminders,
},
while reminders_used < max_reminders:
needs_completion = _needs_completion_reminder(
task_kind=task_kind, inputs=inputs, bindings=bindings, tools_called=tools_called
)
reminder = persona.completion_reminder(repo=inputs.repo, issue=inputs.issue, workspace=inputs.workspace)
dirty: DirtyState | None = None
if not needs_completion:
dirty = _probe_workspace_dirty(inputs.workspace, inputs.slot_uid)
if not dirty.is_dirty:
break
reminders_used += 1
if needs_completion:
log.warning(
"rpc_completion_reminder",
extra={
"issue": bindings.issue_key,
"task": task_kind,
"attempt": reminders_used,
"max": max_reminders,
},
)
reminder = persona.completion_reminder(repo=inputs.repo, issue=inputs.issue, workspace=inputs.workspace)
else:
assert dirty is not None
log.warning(
"rpc_dirty_state_reminder",
extra={
"issue": bindings.issue_key,
"task": task_kind,
"attempt": reminders_used,
"max": max_reminders,
"uncommitted": dirty.uncommitted,
"unpushed": dirty.unpushed,
},
)
reminder = persona.dirty_state_reminder(
repo=inputs.repo,
issue=inputs.issue,
workspace=inputs.workspace,
dirty=dirty,
)
next_turn = _run(reminder)
if next_turn is None:
return None
@@ -284,6 +331,19 @@ def _drive_turn(
"tools_called": sorted(tools_called),
},
)
if reminders_used:
final_dirty = _probe_workspace_dirty(inputs.workspace, inputs.slot_uid)
if final_dirty.is_dirty:
log.warning(
"rpc_dirty_state_unfinished",
extra={
"issue": bindings.issue_key,
"task": task_kind,
"reminders": reminders_used,
"uncommitted": final_dirty.uncommitted,
"unpushed": final_dirty.unpushed,
},
)
return turn
python/robomp/tests/test_worker.py
+90
View File
@@ -16,6 +16,7 @@ import pytest
from robomp import worker
from robomp.config import Settings
from robomp.git_ops import DirtyState
class _FakeRpcClient:
@@ -658,6 +659,95 @@ async def test_run_rpc_skips_reminder_when_unclassified(tmp_path: Path, settings
assert len(fake.prompts) == 1
# ---------------------------------------------------------------------------
# Dirty-state watchdog
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_run_rpc_sends_dirty_state_reminder_when_worktree_has_unpushed_work(
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Agent ended its turn with unpushed commits → reminder fires; clean → loop exits."""
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
dirty = DirtyState(uncommitted=2, unpushed=1, summary="Unpushed commits (1):\nabc1234 wip")
clean = DirtyState(uncommitted=0, unpushed=0, summary="")
states = iter([dirty, clean])
monkeypatch.setattr(worker, "_probe_workspace_dirty", lambda _ws, _slot: next(states, clean))
loop = asyncio.new_event_loop()
try:
worker._run_rpc_blocking(
inputs,
task_kind="handle_comment",
prompt="kickoff",
loop=loop,
bindings=bindings, # type: ignore[arg-type]
)
finally:
loop.close()
fake = _FakeRpcClient.instances[0]
assert len(fake.prompts) == 2, fake.prompts
reminder = fake.prompts[1]
assert "Unpushed commits" in reminder
assert "abc1234" in reminder
assert "{{" not in reminder, "template placeholder leaked"
@pytest.mark.asyncio
async def test_run_rpc_skips_dirty_state_reminder_when_worktree_is_clean(
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Clean worktree at end of turn → no extra prompts."""
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
monkeypatch.setattr(
worker,
"_probe_workspace_dirty",
lambda _ws, _slot: DirtyState(uncommitted=0, unpushed=0, summary=""),
)
loop = asyncio.new_event_loop()
try:
worker._run_rpc_blocking(
inputs,
task_kind="handle_comment",
prompt="kickoff",
loop=loop,
bindings=bindings, # type: ignore[arg-type]
)
finally:
loop.close()
fake = _FakeRpcClient.instances[0]
assert len(fake.prompts) == 1
@pytest.mark.asyncio
async def test_run_rpc_caps_dirty_state_reminders_at_budget(
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Persistently dirty workspace must not loop past the reminder budget."""
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
dirty = DirtyState(uncommitted=1, unpushed=0, summary="Uncommitted changes (1):\n?? oops.txt")
monkeypatch.setattr(worker, "_probe_workspace_dirty", lambda _ws, _slot: dirty)
loop = asyncio.new_event_loop()
try:
worker._run_rpc_blocking(
inputs,
task_kind="handle_comment",
prompt="kickoff",
loop=loop,
bindings=bindings, # type: ignore[arg-type]
)
finally:
loop.close()
fake = _FakeRpcClient.instances[0]
# kickoff + N reminders, capped at the configured budget (default 2).
assert len(fake.prompts) == 1 + settings.task_completion_max_reminders
# ---------------------------------------------------------------------------
# Natives-cache capture-on-success
# ---------------------------------------------------------------------------