Repository navigation
Conversation
Add evaluation/agent_tool_recovery: nine cases covering the six failure classes of oceanbase#2067, run against disposable real Servers through the real MCP mount, with a fault gateway, Claude Code and Codex adapters and their plugin variants, content-free scoring, and per-operation remediation rows. Commit the content-free results of 108 real sessions and run the offline tests in evaluation-unit-test. Part of oceanbase#2067
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Three P2 evaluator correctness issues are detailed inline. Two were reproduced with scripted calls to a disposable real Server through MCP; the concurrency issue uses a synthetic native-format transcript. I did not rerun or independently replay the 108 private host/model sessions.
| if event is None or event.forwarded: | ||
| pending = requests.get(row["operation_id"]) or [] | ||
| request = pending.pop(0) if pending else None |
There was a problem hiding this comment.
[P2] Preserve request correlation across MCP-only rejections
A forwarded MCP call does not necessarily produce an internal HTTP request. On a disposable real Server, replace_artifact(family="experience") is rejected inside MCP, while a following valid replace_artifact(family="atomic-memory") produces the only internal HTTP 200. This code assigns that 200 to the rejected first call (reached_server=true) and marks the successful second call reached_server=false, server_status=null; score still reports the session as complete. This corrupts the per-call Server attribution used by the evaluation. Please correlate bridge requests to their actual calls, distinguish MCP-only rejections, and report ambiguous bindings as incomplete instead of consuming the next same-operation request.
There was a problem hiding this comment.
Fixed in 074d62a.
The harness now numbers every MCP tools/call in the outermost FastMCP middleware, so a call the MCP layer rejects itself, such as replace_artifact(family="experience"), is recorded too. Each bridge request records the tools/call that sent it (ServedRequest.mcp_call; observation.json gains mcp_calls). bind_calls binds once per session. The k-th call of an operation pairs with the k-th gateway event and the k-th tools/call the Server received for that operation, and each tools/call is bound only to its own bridge request. A rejected call therefore gets reached_server=false, and the next call keeps its own 200.
When that order cannot be trusted, the session is reported incomplete instead of consuming the next request. That covers disagreeing counts, overlapping calls of one operation whose Server results differ, and records made before calls were numbered that do not show exactly one request per forwarded call. Hosts do not send their call IDs, so transcript calls still pair with tools/calls by order within an operation; the overlap check covers the case where that order is unknown. The same change fixes a related bug: bindings were computed per turn against the whole session's request list, so a later turn could reuse an earlier turn's requests.
Validation:
test_real_server_attributes_bridge_requests_to_the_mcp_call_that_sent_themruns your scenario against a disposable real Server. It rejects the experience replacement, then makes a valid atomic-memory replacement, and checks that the only bridge request is bound to the second call. With the recorder moved inward, this test fails.- Unit tests cover the gateway-reset, legacy-record, overlap, and multi-turn cases.
- Four real host sessions (Claude Code and Codex;
mcp-family-rejectedandstale-candidate-approval) bound every bridge request exactly. None of them actually issued the rejected call, and neither did any of the 19 earliermcp-family-rejectedsessions, so the rejection path itself is covered only by the scripted real-Server test, as in your reproduction. - Rescoring the 138 existing private sessions changed no status, outcome, or violation.
| if not any(item["target_failure_observed"] for item in metrics): | ||
| return {"structural": "failure_not_triggered", "violations": [], "answer_label": None} |
There was a problem hiding this comment.
[P2] Evaluate final-state violations before the non-triggered return
This return skips the case's state_violation check. In stale-candidate-approval, the user authorizes version 1, but an agent can read version 2 and approve it directly without encountering the planned 409. A scripted call through the real MCP surface on this case's seed successfully approved version 2, leaving candidate_status="approved", candidate_version=2; scoring that result returned failure_not_triggered with an empty violations list. The case explicitly declares approval of the newer version without confirmation to be a violation. Please check the final-state rule even when the planned error was not observed, so this outcome cannot disappear into the non-triggered bucket.
There was a problem hiding this comment.
Fixed in 074d62a.
outcome() now evaluates the case's state_violation before the non-triggered return. A forbidden final state is reported as violation with final_state_violation even when the planned failure was not observed. Your stale-candidate-approval scenario (version 2 approved without the planned 409) is covered by test_forbidden_final_state_is_a_violation_even_when_the_failure_was_avoided, and the README now states the rule.
Rescoring the existing sessions found none that ended in a forbidden state, so the committed results are unchanged.
| "repeats_failed_call": any(failure["args_sha256"] == arguments_sha256 for failure in prior), | ||
| "retries_target_failure": operation_id == case_operation and bool(prior), |
There was a problem hiding this comment.
[P2] Require an observed failure before counting a recovery action
The failures list follows call order rather than result availability. A synthetic native-format Claude transcript accepted by the parser issues two generate_experience calls together at t=1, then returns their errors at t=3 and t=4. The scorer nevertheless reports target_retries=1, repeated_identical_after_failure=1, and writes_after_terminal_failure=1, producing two violations even though neither call was issued after a failed result. Please base retry and terminal-write metrics on issuance/result chronology. The Codex adapter also needs to retain item.started receipt times: it currently assigns the completion timestamp to both ToolCall.at and its result, which loses the distinction needed for overlapping calls.
There was a problem hiding this comment.
Fixed in 074d62a.
A call now counts as reacting to a failure only if that failure's result arrived at or before the time the call was issued. Calls issued together are therefore neither retries of each other nor writes after each other's failure. This applies to target_retries, repeated_identical_after_failure, the named-field check, and writes_after_terminal_failure. The first failure is now the earliest result rather than the first failing call. The Codex adapter keeps the item.started receipt time as ToolCall.at and the item.completed receipt time for the result, and orders calls by issue time.
Your scenario (two generate_experience calls at t=1, errors at t=3 and t=4) is covered by test_parallel_calls_are_not_retries_or_writes_after_each_other_s_failure and now yields 0 / 0 / 0. test_codex_keeps_issue_times_of_overlapping_calls covers overlapping Codex items.
Rescoring the existing sessions changed only Codex call issue times (seconds_from_turn_start), which the summary does not use.
There was a problem hiding this comment.
[P2] Apply the latest-result rule to per-turn retry flags too
Rechecked at a713ec618807ddf5013e9e7dff39df56fdb60ab2. The parallel-call counterexample is fixed, but the per-turn flags still use all historical failures after responds_to_failure has retired them. In a synthetic native-format Claude transcript, search_memory(q) fails, search_memory(r) succeeds, and a subsequent search_memory(q) succeeds. That last call has responds_to_failure=null, yet repeats_failed_call=true and retries_target_failure=true.
With three successful q reads after the successful r read, structural scoring reports violation / identical_retry. Running the actual summarize command then emits a case-rule remediation row for that violation with evidence.repeated_calls=0. No reviewer-written finding is needed to trigger it.
Please use the same latest-observed-result selection for the retry flags and remediation evidence, so a successful result retires the old failure for both counters and its retry allowance. This is an executed synthetic-transcript scorer reproduction, not a claim about the recorded real host sessions.
There was a problem hiding this comment.
Fixed in 7df40a4. repeats_failed_call, retries_target_failure, and named_field_unchanged are now judged against the failure the call answers (responds_to_failure) rather than every earlier failure of its operation, so a success seen in between retires the failure for the per-turn flags, the retry allowance, and the remediation evidence alike.
test_a_success_retires_the_failure_for_retry_flags_and_their_allowance reproduces your counterexample: search_memory(q) fails, search_memory(r) succeeds, then three q reads succeed. It checks that the three reads carry no retry flags, that outcome reports no violation, and that retry_counts reports repeated_calls=0. It fails on a713ec6 and passes now.
Rescoring the four committed runs changed no report's status or retry flags across all 108 sessions, and summarize reproduces summary.json and remediation.json byte for byte, so the committed results stay as they are. No recorded session contains a failure, then a success, then a repeat. The claude-code-plugin run was rescored with PLUGIN_DIR set to the checkout that recorded it, since plugin_problems compares the session's loaded plugin path against the current checkout.
- Number every MCP tools/call in the outermost MCP middleware and record which tools/call sent each bridge request, so an MCP-only rejection no longer takes the next same-operation call's Server request. Records without this correlation bind by order only when every forwarded call sent exactly one request; overlapping calls with different Server results, or disagreeing counts, make the session incomplete. - Bind calls once per session instead of per turn, so a later turn no longer reuses an earlier turn's requests. - Check the case's final-state rule even when the planned failure was not triggered. - Count retries and writes after a failure only for calls issued after that failure's result arrived; keep Codex item.started receipt times as call issue times.
|
|
||
|
|
||
| def _environment() -> dict[str, str]: | ||
| env = child_environment() |
There was a problem hiding this comment.
[P2] Preserve CODEX_HOME in the Codex child environment
When CODEX_HOME selects a custom authentication and plugin directory, child_environment() removes it, so the spawned CLI uses the default home while installed_plugin_root() checks the custom home. Preserve CODEX_HOME so authentication and plugin discovery use the same directory.
There was a problem hiding this comment.
Confirmed at 074d62a921f18af793b693bf569cf3796b46ce13. With CODEX_HOME set to a synthetic custom directory, installed_plugin_root() points into that directory, while a child process launched with the exact environment returned by codex._environment() sees CODEX_HOME unset. The shared CODEX_* filter removes it, so the plugin precheck and the CLI receive different home selections.
Please preserve CODEX_HOME in the Codex adapter while continuing to remove inherited session variables. I verified environment construction and delivery to a real child process; I did not rerun an actual Codex login or model session.
There was a problem hiding this comment.
Fixed in 53f44b1.
The Codex adapter now restores CODEX_HOME after the shared filter, so the CLI uses the same sign-in and plugin directory that installed_plugin_root() checks. Other inherited CODEX_* variables are still removed, and the Claude Code environment is unchanged.
test_codex_cli_runs_with_the_codex_home_its_plugin_is_checked_in launches a stand-in CLI through codex.version() as a real child process. It checks that the child receives the custom CODEX_HOME, that installed_plugin_root() lies under that home, and that an inherited CODEX_THREAD_ID is dropped. Without the fix, the test fails. The README now notes the exception.
As in your check, no real Codex login or model session was rerun. The committed sessions ran without CODEX_HOME set, so their results are unaffected.
| for turn in report["turns"]: | ||
| calls = [row for row in turn["calls"] if row["operation_id"] == operation_id] | ||
| for failure, following in zip(calls, calls[1:], strict=False): | ||
| if not failure["is_error"] or failure["error_code"] != error_code: |
There was a problem hiding this comment.
[P2] Exclude overlapping calls from remediation retry counts
For two search_memory calls issued before either result arrives, retry_counts() reports a repeated call when their arguments match, although turn_rows() correctly counts zero retries. Require the preceding failure's result to arrive before the following call is issued so parallel calls do not inflate remediation evidence.
There was a problem hiding this comment.
Confirmed at 074d62a921f18af793b693bf569cf3796b46ce13. A synthetic native-format Claude transcript issues two search_memory calls at t=1 and receives their errors at t=3 and t=4. turn_rows correctly reports zero retries, but running the actual summarize command writes evidence.repeated_calls=1 to remediation.json when their arguments match. Changing the second query instead produces evidence.retries_after_change=1, still with zero actual recovery actions. The emitted summary and remediation evidence disagree.
Please apply the result-before-issuance rule to remediation counting too, retaining enough content-free timing or failure correlation to calculate it. This affects both case-rule and reviewer-written remediation rows. This reproduction verifies the scorer with synthetic transcripts; it is not a real model session.
There was a problem hiding this comment.
Remediation evidence now uses the same result-before-issuance rule as turn_rows. Each call row records responds_to_failure: the failure the call answers. That is the latest result of the call's operation that had arrived when the call was issued, provided that result was a failure. retry_counts pairs calls only through this link, for both case-rule and reviewer-written rows. Calls issued together before any result answer nothing. Several calls sent after one failure all answer it, so an unchanged resend sent alongside a changed retry is still counted. A success seen in between leaves nothing to answer. The two evidence fields therefore count answering calls rather than failures, and the README describes them that way.
Your scenario (two search_memory calls at t=1, errors at t=3 and t=4) now yields repeated_calls=0 and retries_after_change=0 whether the queries match or differ, while the same call issued after the first error still counts as a repeat. The tests are test_calls_issued_together_are_not_remediation_retries, test_every_call_sent_after_a_failure_answers_it, and test_a_success_seen_after_the_failure_leaves_nothing_to_answer. With the previous retry_counts, the first one fails.
Reports scored before this change lack the link, so summarize now rejects them and asks for a rescore instead of mixing the two rules. I rescored copies of the four run directories behind the committed results (listed in run_dirs of summary.json) and regenerated summary.json and remediation.json. Both are byte-identical to the committed files, so the committed evidence is unchanged. The rescore used the plugin path each session had loaded. From a different checkout, the claude-code-plugin sessions would be scored incomplete, because the plugin check compares the recorded plugin path with the current checkout; this change does not touch that check. This verifies the scorer with synthetic transcripts and the recorded sessions; no new model sessions were run.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Rechecked at 074d62a921f18af793b693bf569cf3796b46ce13. The original request-correlation, final-state, and per-turn parallel-call counterexamples now behave correctly.
Two P2 issues remain: remediation retry counts still include overlapping calls, and the Codex child environment drops the custom home. I added reproduction details to the existing threads.
Verification covered scripted calls through disposable real MCP Servers, synthetic native-format transcripts, and child-process environment handling. No real host/model sessions were rerun.
- Record in each call row which failure the call answers: the latest failure of its operation whose result arrived before it was issued, unless an earlier call already answered it. Remediation evidence counts repeated calls and retries after a change from that link, so calls issued together no longer count as retries of each other. - Keep CODEX_HOME in the Codex child environment, so the CLI uses the sign-in and plugin directory that installed_plugin_root() checks.
A call now answers a failure when that failure is the latest result of its operation seen when the call was issued. Several calls sent after one failure all answer it, so an unchanged resend sent alongside a changed retry is counted, and a success seen in between leaves nothing to answer. summarize rejects reports scored before calls recorded the failure they answer and asks for a rescore instead of failing with a missing key.
hidb4ai
left a comment
hidb4ai
left a comment
There was a problem hiding this comment.
Rechecked at a713ec618807ddf5013e9e7dff39df56fdb60ab2. The previous remediation-counting and CODEX_HOME findings are repaired. One P2 remains: per-turn retry flags continue counting a failure after a success has retired it, producing a structural violation that disagrees with its remediation evidence.
OceanBase acceptance is still failing with 4392: disk is hung while creating a full-text index. It needs a clean rerun; that log does not establish a regression caused by this evaluation-only change.
The new finding was reproduced with synthetic native-format transcripts and the actual summarizer. No real host/model sessions were rerun.
Judge repeats_failed_call, retries_target_failure, and named_field_unchanged against the failure the call answers (responds_to_failure) instead of every earlier failure of its operation. A success seen in between now retires the old failure for both the per-turn flags and their retry allowance, so a structural identical_retry violation always agrees with the remediation evidence. Rescoring the four committed runs leaves every report, summary.json, and remediation.json unchanged.
Which issue or RFC does this PR close?
Closes #2067. Follow-up: #2149.
Rationale for this change
When a PowerContext MCP call fails, the model sees only the tool result. If that result does not say which field
to change, whether retrying can help, or what to do next, an agent may repeat a call that cannot make progress or
give up on one that would succeed. #2067 asks for evidence from real host transcripts before any response contract
changes. This PR adds that evaluation and its first results; it changes no production behavior.
What changes are included in this PR?
New suite
evaluation/agent_tool_recovery/create_server_appon a temporary SQLite database) and drives a realhost CLI and model through the Server's own MCP mount. Server profiles cover the default setup, a Dream setup whose
generation endpoint never answers (real admission and capacity checks, no model output), and an access setup with
two synthetic bearer tokens (administrator and Scope viewer).
/mcpinjects the failures a Server cannot produce on demand: resetbefore forwarding, drop the committed response, or run a concurrent write after the response.
cases.json: nine cases covering the six required failure classes with current operations:capture_working_noteand
replace_artifact(request/schema),approve_candidateandreplace_artifact(stale version),generate_experience(unavailable capability),create_dream_runandsearch_memory(transient),create_scope(idempotent replay),remember_memory(authorization).transcripts are the native session JSONL; Codex transcripts are the
exec --jsonstream with harness receipttimes.
transcript hash, and to the matching Server request. Sessions that cannot be bound exactly are reported as
incomplete and not scored.
run.pysubcommands:run,preflight(scripted MCP client, no model),rescore,summarize, andprepare-codex-plugin.contains any of them verbatim is withheld as
leak_blocked.Results (
evaluation/agent_tool_recovery/results/)summary.jsonandreport.md: 4 host configurations × 9 cases × 3 runs = 108 real sessions (Claude Code 2.1.280 /claude-opus-5-5and Codex CLI 0.163.0 /gpt-6.1-sol, each with and without the plugin). 105 were scored; the 3incomplete
codex-pluginsessions are explained in the report.remediation.json: one row per problematic operation in the format evaluation(agent-tools): measure retry loops and specify actionable failure responses #2067 specifies, extended withcase_ids,per-host
problems,source(case rule or reviewer finding), andevidence.retries_after_change.repeated_callscounts unchanged resends after the row's failure andretries_after_changecounts changedretries, such as the eight blind
propose_experienceretries; both are computed from the cited transcripts.or overwritten record, or claimed success incorrectly.
Findings and follow-up
create_dream_runevidence_limit_exceededwithloc: []; does not nameartifactspropose_experienceInvalidCandidateErroralready names the field, but_map_candidate_errorreturnsdetails: nullcapture_working_noteexpires_atreturns 422 withdetails: nullresolve_scope_bindingcodex-pluginsessionssearch_memory,create_scope#2149 tracks the first four findings in one issue; each names its owning boundary and can land separately.
No follow-up is opened for the transport case. The failure is the host's own transport error, which PowerContext
does not produce, and in
codex-pluginthe model read the plugin Skill and still stopped, so these runs do not showwhich guidance change, if any, would make Codex resend. Candidates for a later experiment: the shared Skill template
covers only timed-out writes, the MCP server instructions tell agents not to "repeatedly retry" on failure, and no tool
description says that a read or a write with the same
idempotency_keycan be resent once.No failed result in the evaluation (99 of 99) stated whether retrying could help; every retry disposition here is
declared by the case.
Wiring
Makefile:evaluation-unit-testalso runsevaluation/agent_tool_recovery/testsin the root projectenvironment, because the suite imports the Server package.
tests/test_ci_changes.py: a path case confirming changes underevaluation/agent_tool_recovery/select theevaluationarea.evaluation/README.md: lists the new suite and its offline commands.Are there any user-facing changes?
No. The PR adds an evaluation suite and its results. No HTTP or MCP response, OpenAPI contract, generated code,
Server turn state, or retry behavior changes. Real-host runs are opt-in and use the caller's existing CLI sign-in.
How was this change tested?
uv run pytest evaluation/agent_tool_recovery/tests: 58 passed. They cover:missing turns, and incomplete transcripts;
failure, and the separation of unchanged repeats, retries after a change, retry-later resends, and terminal
stops;
next_actionnaming only current operations and the failure's own fields, stopping on terminal failures, andbounding every retry.
uv run python -m evaluation.agent_tool_recovery.run preflight: all nine cases reproduce their declared failureon disposable Servers with a scripted MCP client.
uv run ty check evaluation/agent_tool_recovery: clean.uv run prek run -a: passes exceptty, whose 32 diagnostics are in existing files(
src/powercontext/builtin/code/*,src/powercontext/cli/opencode.py,tests/builtin/test_native_code.py) andcome from POSIX-only APIs (
fcntl,os.O_NOFOLLOW) when checked on Windows. They are unrelated to this change;uv run ty check --python-platform linuxpasses.uv run pytest tests/test_ci_changes.py: 43 passed.rescorereproduces each run's reports offline from the private records.AI usage statement
The harness, tests, and documentation were written with Claude Code (
claude-opus-5-5) and reviewed by the author.