mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-10-03 09:13:27 +00:00
fix(open-swe): make workflow-push-guard blocks tell the agent how to recover (#173)
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Typecheck (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
Some checks are pending
CI / Lint (push) Waiting to run
CI / Format check (push) Waiting to run
CI / Typecheck (push) Waiting to run
CI / Unit tests (push) Waiting to run
CI / Playwright E2E (push) Waiting to run
CI / Docker build smoke (push) Waiting to run
CI / Triage ledger up to date (push) Waiting to run
CI / ui bun.lock in sync (push) Waiting to run
The WorkflowPushGuardMiddleware fails closed and blocks a git push whenever
it cannot parse the command into a plain, inspectable form (chained shell
operators, obfuscation, unrecognized refspecs). Most block reasons named only
the symptom ("chained shell commands"), so the agent kept retrying other
chained variants (cd &&, pushd &&) instead of dropping the chaining.
Append a single actionable remedy to every block reason, pointing at a plain
`git push origin <branch>` or `git -C <dir> push`, so a blocked run recovers
on the next attempt instead of looping.
This commit is contained in:
parent
5350b63c85
commit
23cf6d2d4d
2 changed files with 42 additions and 5 deletions
|
|
@ -42,6 +42,13 @@ from ..utils.slack import post_slack_thread_reply_with_ts
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
|
|
||||||
_WORKFLOW_PREFIX = ".github/workflows/"
|
_WORKFLOW_PREFIX = ".github/workflows/"
|
||||||
|
# Appended to every block reason so the agent always sees how to recover. The guard blocks
|
||||||
|
# because it cannot inspect the push, not because the command is dangerous; re-issuing it as a
|
||||||
|
# single plain git command makes it inspectable and lets it through.
|
||||||
|
_BLOCK_REMEDY = (
|
||||||
|
" Re-issue it as a single plain command with no shell operators (no &&, ;, |, (), etc.) — "
|
||||||
|
"`git push origin <branch>`, or `git -C <dir> push origin <branch>` to target a subdirectory."
|
||||||
|
)
|
||||||
_SHELL_OPERATORS = {";", "|", "||", "&"}
|
_SHELL_OPERATORS = {";", "|", "||", "&"}
|
||||||
_REF_NAME = re.compile(r"^[A-Za-z0-9._/@+-]+$")
|
_REF_NAME = re.compile(r"^[A-Za-z0-9._/@+-]+$")
|
||||||
_GIT_OBJECT_ID = re.compile(r"^[0-9a-fA-F]{40,64}$")
|
_GIT_OBJECT_ID = re.compile(r"^[0-9a-fA-F]{40,64}$")
|
||||||
|
|
@ -307,10 +314,7 @@ def _parse_git_push(command: str) -> ParsedGitPush | _BlockedGitPush | None:
|
||||||
# No push the token scan can normalize. Fail closed if one is hidden behind shell
|
# No push the token scan can normalize. Fail closed if one is hidden behind shell
|
||||||
# metacharacters/expansion; otherwise leave the command untouched.
|
# metacharacters/expansion; otherwise leave the command untouched.
|
||||||
if _maybe_obfuscated_git_push(stripped):
|
if _maybe_obfuscated_git_push(stripped):
|
||||||
return _BlockedGitPush(
|
return _BlockedGitPush("possible git push obscured by shell metacharacters")
|
||||||
"possible git push obscured by shell metacharacters; issue it as a plain "
|
|
||||||
"`git push origin <branch>`"
|
|
||||||
)
|
|
||||||
return None
|
return None
|
||||||
if _UNSAFE_RAW_COMMAND.search(stripped) or "&" in stripped.replace("&&", ""):
|
if _UNSAFE_RAW_COMMAND.search(stripped) or "&" in stripped.replace("&&", ""):
|
||||||
return _BlockedGitPush("unsafe shell characters in git push command")
|
return _BlockedGitPush("unsafe shell characters in git push command")
|
||||||
|
|
@ -714,7 +718,13 @@ def _blocked_message(
|
||||||
) -> ToolMessage:
|
) -> ToolMessage:
|
||||||
status = "rejected" if already_rejected else "approval_required"
|
status = "rejected" if already_rejected else "approval_required"
|
||||||
if blocked:
|
if blocked:
|
||||||
error = change.blocked_reason or "This git push command is not recognized as safe."
|
error = (
|
||||||
|
(change.blocked_reason or "This git push command is not recognized as safe.").rstrip(
|
||||||
|
"."
|
||||||
|
)
|
||||||
|
+ "."
|
||||||
|
+ _BLOCK_REMEDY
|
||||||
|
)
|
||||||
error_type = "WorkflowPushBlocked"
|
error_type = "WorkflowPushBlocked"
|
||||||
elif stale:
|
elif stale:
|
||||||
error = (
|
error = (
|
||||||
|
|
|
||||||
|
|
@ -990,3 +990,30 @@ async def test_unreadable_head_tree_blocks(monkeypatch: pytest.MonkeyPatch) -> N
|
||||||
assert result.status == "error"
|
assert result.status == "error"
|
||||||
payload = json.loads(str(result.content))
|
payload = json.loads(str(result.content))
|
||||||
assert payload["error_type"] == "WorkflowPushBlocked"
|
assert payload["error_type"] == "WorkflowPushBlocked"
|
||||||
|
# The block must tell the agent how to recover, or it just retries other chained forms.
|
||||||
|
assert "git push origin <branch>" in payload["error"]
|
||||||
|
assert "git -C <dir> push" in payload["error"]
|
||||||
|
|
||||||
|
|
||||||
|
def test_blocked_message_appends_actionable_remedy() -> None:
|
||||||
|
change = guard.WorkflowPushChange(
|
||||||
|
fingerprint="fp",
|
||||||
|
repo="acme/app",
|
||||||
|
branch="feature",
|
||||||
|
files=[],
|
||||||
|
head_sha="deadbeef",
|
||||||
|
remote="origin",
|
||||||
|
local_ref="feature",
|
||||||
|
remote_ref="feature",
|
||||||
|
fixed_command="",
|
||||||
|
blocked=True,
|
||||||
|
blocked_reason="chained shell commands",
|
||||||
|
)
|
||||||
|
message = guard._blocked_message(change, blocked=True)
|
||||||
|
payload = json.loads(str(message.content))
|
||||||
|
error = payload["error"]
|
||||||
|
# Original reason is preserved, and the remedy is appended exactly once.
|
||||||
|
assert error.startswith("chained shell commands.")
|
||||||
|
assert "git push origin <branch>" in error
|
||||||
|
assert "git -C <dir> push" in error
|
||||||
|
assert error.count("Re-issue it as a single plain command") == 1
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue