feat(agent): integrate eval coding safeguards into main - #1
Merged
Conversation
SWE-bench Verified analysis showed 33.8% of failures come from incomplete fixes — agents patching one variant of a symbol while missing function vs class, sync vs async, or version-specific sites (Sphinx autodoc_typehints_description: 10/16 failures). Extend the shared agentic_mode "Doing tasks" guidance: before editing a bug or behavior change, enumerate the scope of impact via Grep (+ inline python ast when grep is ambiguous), and record candidate sites in TodoWrite as the completion checklist. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Continuing the SWE-bench Verified incomplete-fix work (P0a covered scope enumeration before edits; this covers verification after). Add to shared agentic_mode "Doing tasks": after a behavior-changing edit, run static checks, repo-shipped tests scoped to the modified module, and any tests the task description quotes. Treat failures as the next signal, not as the end state. Avoids hidden-evaluator leakage by sourcing tests only from the repo and task input. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Update the verification block in agentic_mode to match the harness gate
added in the previous commit, and to head off two patterns observed
during initial dogfooding:
(a) Agents over-interpreting "verify the fix" as "build after every
individual edit", which on large repos explodes runtime; and
(b) Agents defaulting to whole-workspace verifier commands
(`go build ./...`, `cargo check --workspace`) that take many
minutes and rarely add signal beyond a scoped build, leaving the
harness no budget for retries.
Specifically:
- Reframe from "verify the fix yourself" to "when you are done
editing — not between every edit — run one verification pass".
- New explicit bullet on scoping: build the package(s) you touched,
not the workspace; widen only if scoped passes and you suspect
cross-package breakage.
- New explicit bullet on batching: make the related set of edits,
then verify once.
- Reuse the same three-layer model as the harness detector
(parse / build / targeted tests) so prompt and gate agree.
Together with the gate, this makes the contract bilateral: the prompt
tells the agent to self-verify scoped + once, the harness re-prompts
if it didn't.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A failure mode the verifier gate cannot address: the harness applies
hidden test files after the agent's patch. If the patch deletes or
renames a public symbol that those hidden tests import, the build of
the test fixture itself fails with "undefined: X" — and `go vet` /
`cargo check --tests` inside the agent's workspace won't see it,
because the test files don't exist there yet.
The only place we can intervene is at the agent's planning step:
- Prefer changing X's implementation to changing X's name when the
issue says "X should behave like Y" — the grader almost certainly
references X.
- If a rename or removal is genuinely required, leave a thin alias
/ re-export at the old name so external (invisible) callers still
compile.
- Before deleting a non-trivial public symbol, Grep its name. The
visible callers are what you can verify; the invisible callers
are hidden tests, which always reference current names.
The framing is "you cannot see the test files, so be conservative
about anything that could break them at the link/compile boundary."
This won't change behavior on tasks where the gold patch really does
require a rename, but it should help with the larger class where the
agent unnecessarily renames a symbol because the issue happens to
describe a new name.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Models occasionally call Read with limit: 5 to "probe" a file's type before reading it properly. Source files begin with copyright headers, so probe reads return no useful code and waste a round trip. The existing anti-fragmentation guidance targets 30-100 line chunks (progressive slicing) but misses this distinct pattern. Add an explicit bullet calling out small limit values (< 50) as a probe anti-pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…failures Agents on SWE-bench Pro were declaring success after reasoning that a failing test was "flaky" or "pre-existing", without actually verifying this on the unmodified codebase. The ansible-11c177 case is a concrete example: the agent ran git stash + pytest to confirm the test passed without its changes, but the real issue was an ordering dependency that only surfaced in the full test suite. Add an explicit rule: a test failure may only be dismissed as pre-existing if `git stash && <test> && git stash pop` reproduces the failure. If the stashed run passes, the failure belongs to the patch. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Using && to chain git stash, test, and git stash pop means a failing test (non-zero exit) silently skips git stash pop, leaving the patch stranded in the stash. Rewrite as separate steps with an explicit note to always run git stash pop regardless of test outcome. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…flags Agents were piping pytest output through | head or | tail, hiding either the traceback or the FAILED summary. Prompt now explicitly bans this and recommends --tb=short / --tb=line for compact-but-complete output. Also warns against -x/--exitfirst which stops at the first failure and hides subsequent ones — a problem when all required tests must pass. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…rify time Agents were only running task-specified tests and missing file-specific tests (e.g. editing elf.py but never running test_elf.py). New rule instructs agents to search for test_<filename> / <filename>_test counterparts for every edited file and run them in the same verification pass — not after each individual edit, consistent with the existing batch-before-verify principle. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nd static analysis rules Three targeted fixes based on trace analysis of Go compilation regressions: 1. Replace behavior checklist (which included "compatibility requirements" and "idempotency") with intent-clarification guidance. The checklist caused agents to revert correct signature changes citing "existing callers would break" — removing the compatibility framing cuts this reasoning off at the source. 2. Add package-level caveat to the test discovery rule: in compiled languages (Go, Rust), run discovered tests at the package level (go test ./pkg/...) not by individual file, consistent with the existing scope-verifier guidance. 3. Add static analysis precedence rule: go vet / cargo check / tsc errors take precedence over test results. A passing go test does not override a failing go vet on the changed package. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bad-case analysis on SWE-bench Pro showed the agent adding Go imports without updating go.mod, so gold tests fail to compile with "no required module provides package" (~6 flipt/vuls cases). Add two rules to agentic_mode.md: treat manifest updates (go.mod/Cargo.toml/ package.json/pyproject.toml) as part of any change that introduces a new import, and watch for the Go missing-module error during scoped verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The OpenAI message converter treated any JSON array string as structured multimodal content. For the valid JSON string '[]', both the Chat Completions and Responses parsers returned an empty parts vector, producing an empty content array that some OpenAI-compatible providers reject. Require structured content arrays to contain at least one supported part. An empty JSON array now follows the existing plain-text fallback, while non-empty text and image part arrays retain their current conversion behavior in both protocol paths. Add a focused integration test through the public converter API for Chat Completions and Responses. Verified with cargo test -p bitfun-ai-adapters --test openai_empty_content_parts (2 passed) and cargo check -p bitfun-ai-adapters.
Port eval commit 5c85a2f onto main's extracted tool-runtime owner. Bash, Zsh, and Ksh now use pipefail for local and remote commands; shell metadata exposes the effective pipeline failure policy.
SWE-bench Pro full 731 audit found 102 trials where the task prompt carries a "don't modify test files" constraint that the agent's final patch violated anyway (48 failed, 54 passed despite it). Trace-level root cause on one case confirmed the verifier restores gold test files before grading, so the edit itself isn't the direct cause of failure — it's a behavioral marker of the agent papering over a contract mismatch instead of reconsidering its implementation. Design: session-scoped LLM constraint extraction (once, fail-open) into a closed-vocabulary matcher (TestFiles/PathContains/PathUnderDir/ Extension/Unmatched), enforced via a deterministic pre-call hook at Edit/Write/Delete time (no per-edit LLM cost — validated against real deepseek-v4-flash latency numbers and the fact all 102 known cases are path-matchable). Targets the dev/main product line, not evals-on-release. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ctions Implements docs/plans/edit-constraint-guard-plan.md: extracts explicit file-edit prohibitions from a session's first user message (once, via a fast-model call, fail-open on any error) and enforces them deterministically at Edit/Write/Delete's existing validate_input() gate — no new hook infra, no per-edit LLM cost. - edit_constraint_guard.rs: ExtractedConstraint/ConstraintMatcher (closed vocabulary: TestFiles/PathContains/PathUnderDir/Extension/ Unmatched), extraction prompt + parsing, matching, rejection message text designed to redirect the agent toward reconsidering its implementation rather than papering over a contract mismatch. - SessionManager: in-memory edit_constraints_store (same pattern as skill_agent_baseline_override_snapshot_store), seed_forked_edit_constraints for subagent inheritance on Fork-mode spawns. - execution_engine.rs: extraction triggers once on turn_index == 0. - FileEditTool/FileWriteTool/DeleteFileTool: validate_input() checks the cached constraints and rejects with a `force: true` escape hatch (schema-documented, requires the agent to state a reason). Write additionally skips enforcement when the target file doesn't already exist, so legitimate new-file creation (e.g. SWE-Atlas-style test writing tasks) isn't blocked. Two pre-existing test failures on this branch (shared_coding_mode_tools_include_plan_and_debug_specific_tools, product_catalog_facade_resolves_get_tool_spec_results_from_same_provider_owner) reproduce identically on a clean stash — unrelated to this change, left alone (stale assertions from f2e9bb4's eval tool-list trim). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keep normal product Web and shell behavior independent from benchmark execution policy. Network egress controls, repository-history sanitization, and benchmark leakage auditing belong in the evaluation pipeline rather than shared agent tools. Changes: - restore WebFetch and WebSearch access to GitHub, Sourcegraph, redirects, and code-hosting search results - remove shell domain, Git network, remote-ref, commit-hash, and GitHub CLI restrictions while retaining SQLite WAL safety and user-authored edit constraints - remove the eval-oriented AI request lifecycle wrapper while preserving the existing effective-options audit and provider request fixes - split edit-constraint state, shell target parsing, and tests into focused modules without changing the public facade - replace the stale benchmark retrospective with the current product boundary design and remove the evaluation-network research note Verification: - cargo check --workspace - cargo check -p bitfun-ai-adapters - cargo test -p bitfun-core edit_constraint_guard --lib (22 passed) - cargo test -p bitfun-core shell_command_safety --lib (4 passed) - focused WebFetch, WebSearch, and normal Git/shell allow tests passed The existing bitfun-ai-adapters lib-test AIClient::new Result migration issue is outside this change; non-test adapter builds pass.
Shell mutation preflight previously reduced every parsed target to the same explicit_shell_mutation operation. Delete-only constraints only recognize delete operations, so rm, git rm, unlink, and move sources could bypass a user instruction that prohibited deletion. Represent parsed shell targets as typed write or delete mutations and pass that operation through the guard. Classify move sources as deletes and destinations as writes, and preserve the same distinction for Python pathlib and Node filesystem calls. Add regression coverage for rm, git rm, pathlib unlink, Node unlink, move source/destination handling, and the invariant that delete-only constraints still allow non-delete writes. Verification: cargo test -p bitfun-core edit_constraint_guard --lib (23 passed); cargo check -p bitfun-core.
Edit constraints were enforced for direct file tools and the initial Bash or ExecCommand string, but interactive WriteStdin input and the standalone Git tool bypassed the guard. Commands with variable, archive, patch, or implicit worktree targets could also run without a path the guard could validate. Route WriteStdin input and Git operations through the same preflight. Extend literal target extraction for dd, rsync, links, directories, and combined in-place flags. While a constraint is active, reject high-risk commands whose mutation targets remain dynamic or implicit; keep normal build, test, read-only shell, and all no-constraint sessions unchanged. Update the guard design boundary and add regression coverage for dynamic variables, scripting runtimes, nested shells, find, checkout, archive extraction, patch input, and resolved literal alternatives. Verification: cargo test -p bitfun-core edit_constraint_guard --lib (24 passed); cargo test -p bitfun-core git_tool --lib (14 passed); WriteStdin focused tests (2 passed); cargo check -p bitfun-core.
Forked sessions copied the parent edit-constraint state with parent dialog turn ids. Child rollback rebuilt state only from child surviving turn ids, so inherited constraints and agent-created provenance disappeared at the first rollback. Persist an explicit fork baseline for active constraints and inherited agent-created paths. Rollback now starts from that baseline and replays only surviving child events, so inherited protections remain while a surviving child-authored revocation can still relax them. Bump the edit-constraint state schema, document the baseline semantics, and add both state-machine and SessionManager regression tests for rollback to the fork point and surviving child revocations. Verification: fork baseline state test passed; forked SessionManager rollback test passed; cargo test -p bitfun-core edit_constraint_guard --lib (25 passed); cargo check -p bitfun-core.
Ordinary product sessions paid edit-constraint costs even when the user had not established any protected paths. Tool validation resolved candidate paths, remote writes could probe file existence, shell commands were parsed, every no-op extraction rewrote session metadata, and guard events were appended to JSONL by default. Make edit-constraint JSONL diagnostics an explicit BITFUN_EDIT_CONSTRAINT_TELEMETRY opt-in while leaving provider request auditing unchanged. Skip shell parsing, path resolution, remote existence checks, provenance maintenance, and no-signal extraction persistence unless an active constraint requires them. Active constraints now invoke the fast classifier only for messages that can add or relax a boundary, so unrelated follow-up turns stay local. Document the default-off diagnostics contract and add regression coverage for telemetry opt-in parsing, no-op persistence, active-constraint follow-ups, and provenance tracking. Verified with cargo test -p bitfun-core edit_constraint_guard --lib (29 passed) and cargo check -p bitfun-core.
The deterministic test-file guard inferred prohibitions from loose keyword co-occurrence and included wording tailored to benchmark task prompts. Phrases such as 'you do not have to modify tests' or a prohibition aimed at source code could therefore be mistaken for a hard test-file boundary. The shared agent prompt also justified API and dependency choices in terms of hidden graders rather than normal product compatibility. Replace the co-occurrence heuristic with explicit, reusable English and Chinese sentence patterns in which the prohibition, mutation operation, and test target are directly related. Keep delete-only restrictions distinct, preserve model fallback for generic path phrases such as 'off limits' and 'leave untouched', and remove benchmark-specific assumptions from the product prompt in favor of downstream API compatibility and repository conventions. Add positive coverage for direct restrictions and regression cases for non-obligations, unrelated source restrictions, negated reminders, and incidental test mentions. Verified with cargo test -p bitfun-core edit_constraint_guard --lib (31 passed), cargo check -p bitfun-core, and a source scan for removed benchmark-specific phrases.
# Conflicts: # src/web-ui/src/flow_chat/services/flow-chat-manager/PeerSessionRefreshModule.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Integrate the final product-generic behavior from the author's eval-only work into the current main architecture.
Reviewed 48 author-owned eval-only commits: 40 non-merge commits, 8 merge commits, and 21 Claude-coauthored non-merge commits. Merge commits were treated as topology rather than replayed. Reverted pairs, changes already present on main, and temporary evaluation-only restrictions were intentionally not reintroduced.
Main changes:
No primary, fast, subagent, or function-agent model mapping was changed. The EditConstraintGuard resolves the existing fast route only for ambiguous add/relax instructions; deterministic and irrelevant paths avoid that model call.
Fixes #
Type and Areas
Type:
Feature / regression fix / refactor / docs / test
Areas:
Rust core, CLI, AI adapters, execution tools, agent prompts, session persistence
Motivation / Impact
The eval branch accumulated general coding-agent reliability fixes while main continued to evolve its CLI, tool protocols, session lifecycle, and prompt surface. Replaying the old files directly would regress those main changes. This PR ports the final net behavior onto the current owners and explicitly excludes benchmark-only network isolation and request-audit code that the eval branch later removed itself.
System prompt conflicts were resolved semantically:
Verification
102cb340b: CLI, Frontend Build, Ubuntu Rust, macOS Rust, and Windows Rust all passedThe only emitted warnings are existing upstream deprecation warnings in the MCP integration.
Reviewer Notes
Important compatibility decisions:
origin/mainchanges throughd8a1133b6, including automatic context-compression recovery, retry-attempt diagnostics, and Remote Connect weak-link recovery, were merged and preservedRollback can be performed by reverting this PR; no data migration is required. Edit-constraint state is schema-versioned and defaults to inactive. Its JSONL diagnostics remain opt-in.
Checklist