Skip to content

rebase: guard automatic debug patches - #129

Merged
tzhouam merged 1 commit into
mainfrom
agent/debug-patch-safety
Sep 5, 2026
Merged

tzhouam merged 1 commit into
mainfrom
agent/debug-patch-safety

Conversation

@tzhouam

@tzhouam tzhouam commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • reject automatic debug patches that modify assertions or tolerances
  • require an explicit local pass before accepting any test-file edit
  • restore rejected patches and trace the policy reason and affected paths

Tests

  • full pytest -q suite
  • focused CI debug-policy integration and unit tests
  • ruff check for the new policy module
  • strict architecture-spec freshness gate

Signed-off-by: tzhouam <tzhouam@connect.ust.hk>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 5, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-09-05T19:42:31.119361Z 0f59b2b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@tzhouam
tzhouam merged commit e9fd24e into main Sep 5, 2026
2 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0f59b2b3a9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +2090 to +2091
policy = evaluate_debug_patch(
Path(repo), policy_before, local_verdict=local)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Apply the patch policy to local debug attempts

The new policy is invoked only by the remote-CI debug_fn; the other automatic debug path in rebase.v3_test_loop (lines 1332–1372) still accepts any agent edit once its post-debug rerun turns green. If that agent weakens the failing assertion, the modified test itself makes the rerun pass and the change can later be committed and pushed, bypassing the safety invariant this commit introduces. Capture and evaluate the policy snapshot around that debug agent as well.

Useful? React with 👍 / 👎.

Comment on lines +89 to +92
if Path(relative).suffix.lower() in _POLICY_SUFFIXES:
text = content.decode("utf-8", errors="replace")
lines = tuple(line.strip() for line in text.splitlines()
if _SENSITIVE_LINE.search(line))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detect changes to complete assertion expressions

Recording only lines that themselves contain assert or a tolerance keyword misses common multiline and indirect oracles. For example, changing expected = 0.8 to 0.7 while leaving a later multiline assert ( line unchanged produces identical sensitive_lines; when the modified test passes locally, the test-hash check also permits it. The automatic agent can therefore weaken the oracle and send the resulting green patch to remote CI despite the advertised prohibition.

Useful? React with 👍 / 👎.

Comment on lines +119 to +123
test_paths = tuple(sorted(
path for path in set(before.test_hashes) | set(after.test_hashes)
if before.test_hashes.get(path) != after.test_hashes.get(path)
))
if test_paths and local_verdict != "passed":

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Tie the local pass to every changed test file

A single local_verdict="passed" authorizes every changed test file, but _verify_locally runs only the manifest job associated with the failed CI slug. If the agent edits a test outside that command's selection—such as another test directory or job—and avoids the sensitive-line heuristic, an unrelated green job permits the unexecuted test edit to be pushed. The decision needs evidence that each reported test_path was covered by the local verification, or it should reject cross-job test edits.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant