Skip to content

fix: detect git checkout separator commands#39

Draft
realmehmetali wants to merge 1 commit into
openclaw:mainfrom
realmehmetali:codex/git-checkout-safety
Draft

fix: detect git checkout separator commands#39
realmehmetali wants to merge 1 commit into
openclaw:mainfrom
realmehmetali:codex/git-checkout-safety

Conversation

@realmehmetali

@realmehmetali realmehmetali commented Jul 18, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes dangerous-shell detection for git checkout -- and adds focused regression coverage for one target, multiple targets, and the separator at end of input.

Why?

The existing pattern ends with --\b. A word boundary cannot occur between the second hyphen and whitespace or end of input, so normal destructive checkout-restoration commands were never flagged.

Fixes #38.

Why this matters for frontier AI evaluation

ClawBench uses dangerous-shell matches to score agent trajectories. Missing a destructive working-tree command understates safety violations and makes comparisons between frontier AI agents less reliable. This patch keeps the correction deliberately narrow so it improves signal quality without expanding the matcher into quoted documentation examples.

Changes

  • replace the impossible trailing word boundary with an explicit whitespace-or-end condition
  • cover one target, multiple targets, and a terminal --

Tests

  • python -m pytest -q passes locally — 319 passed, 5 skipped
  • python -m ruff check clawbench app.py scripts tests passes locally
  • all configured pre-commit hooks pass
  • independent Codex patch review found no actionable issues

AI assistance and public-source provenance

This contribution used significant OpenAI Codex assistance. I manually scoped the change, validated the root cause, ran the complete test/lint/pre-commit gate, and reviewed the final two-file diff. The work used only the public repository and synthetic filenames. No private code, data, product plans, prompts, internal architecture, or proprietary examples were used.

@realmehmetali
fix: detect git checkout separator commands
71c9e01 
Fix the dangerous-shell matcher for git checkout -- and add regression coverage. Fixes openclaw#38.
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. labels Jul 18, 2026
@clawsweeper

clawsweeper Bot commented Jul 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed July 19, 2026, 11:07 AM ET / 15:07 UTC.

Summary
The branch replaces the git checkout -- dangerous-shell regex suffix with whitespace-or-end matching and adds regression cases for one target, multiple targets, and a terminal separator.

Reproducibility: yes. from the supplied source-level evidence: the report identifies an impossible --\b boundary and gives concrete commands that should fail under the old pattern. I could not execute the current-main reproduction because the managed sandbox prevented repository commands from starting.

Review metrics: 1 noteworthy metric.

  • Focused patch: 2 files affected; 10 added, 1 removed. The proposed change is narrowly limited to one dangerous-command matcher and directly related regression coverage.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #38
Summary: This PR explicitly proposes the matcher change requested by the linked issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🦐 gold shrimp
Result: blocked until real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Attach redacted terminal output demonstrating that each of the three new git checkout -- command forms is detected after the change.
  • Refresh the review after adding proof by updating the PR body or asking a maintainer to comment @clawsweeper re-review.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body reports tests, lint, and pre-commit success but provides no inspectable after-fix terminal output, recording, log, or artifact. Add redacted live terminal proof that the matcher flags the three new commands; after updating the PR body, ClawSweeper should re-review automatically, or a maintainer can comment @clawsweeper re-review.

Risk before merge

  • [P1] The required full AGENTS.md read, current-main comparison, and git-history ownership check could not be completed because the repository command sandbox failed to start.
  • [P1] The PR body reports local validation but does not include inspectable after-fix runtime evidence proving the matcher detects the new command forms.

Maintainer options:

  1. Decide the mitigation before merge
    Preserve the narrow regex-and-regression-test approach if current main still contains the reported gap, and attach redacted live terminal output showing the matcher flags all three added command forms before merge.
  2. Pause or close
    Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

  • [P1] Contributor-supplied after-fix real behavior proof is required before merge, and automation cannot establish that proof on the contributor’s setup.

Security
Cleared: The supplied diff is limited to a local Python regex and test coverage, with no indicated dependency, workflow, secret, permission, or supply-chain change.

Review details

Best possible solution:

Preserve the narrow regex-and-regression-test approach if current main still contains the reported gap, and attach redacted live terminal output showing the matcher flags all three added command forms before merge.

Do we have a high-confidence way to reproduce the issue?

Yes, from the supplied source-level evidence: the report identifies an impossible --\b boundary and gives concrete commands that should fail under the old pattern. I could not execute the current-main reproduction because the managed sandbox prevented repository commands from starting.

Is this the best way to solve the issue?

Likely yes: changing the suffix to whitespace-or-end is the narrowest maintainable correction and the added examples directly cover the claimed gap, subject to current-main verification.

AGENTS.md: unclear because the file could not be read completely.

Codex review notes: model internal, reasoning high; reviewed against c7256221f52d.

Label changes

Label changes:

  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • remove rating: 🦪 silver shellfish: Current PR rating is rating: 🧂 unranked krab, so this older rating label is no longer current.

Label justifications:

  • P2: The gap can undercount dangerous shell behavior in evaluation trajectories, but the affected matcher has a limited blast radius.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🦐 gold shrimp.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body reports tests, lint, and pre-commit success but provides no inspectable after-fix terminal output, recording, log, or artifact. Add redacted live terminal proof that the matcher flags the three new commands; after updating the PR body, ClawSweeper should re-review automatically, or a maintainer can comment @clawsweeper re-review.
Evidence reviewed

What I checked:

  • Proposed matcher repair: The supplied PR diff changes the pattern from an impossible trailing word boundary to (?:\s|$), matching the linked report's stated failure mode. (clawbench/trajectory.py:56, 71c9e01e9a58)
  • Focused regression coverage: The supplied diff adds three direct commands covering a single path, multiple paths, and end-of-input. (tests/test_trajectory.py:291, 71c9e01e9a58)
  • Canonical reported bug: The linked issue describes the same regex and proposed replacement; this PR is a candidate fix, not a reason to close the issue before merge. (71c9e01e9a58)
  • Repository inspection blocked: The required read-only shell inspection could not start because the managed sandbox rejected loopback setup, so AGENTS.md, current main, git history, and maintainer notes could not be independently verified.

Likely related people:

  • realmehmetali: The supplied context ties this contributor to both the concrete reproduction in the linked report and the proposed matcher/test change; current-main ownership could not be traced because repository inspection was unavailable. (role: reporter and proposed patch author; confidence: low; commits: 71c9e01e9a58; files: clawbench/trajectory.py, tests/test_trajectory.py)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (1 earlier review cycle)
  • reviewed 2026-07-18T14:09:23.473Z sha 71c9e01 :: needs real behavior proof before merge. :: none

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dangerous-shell pattern never matches git checkout --

1 participant

@realmehmetali