Repository navigation
acli: sync 0.6.6 agent-loop hardening and fix the retry budget - #197
Merged
Merged
Conversation
Vendors release/agentic-cli-5 (851c3a7..8ba15e5) plus the 0.6.5 version bump. A1 read_file now windows to 200 lines by default under a 100k-char cap, takes a 1-based offset matching the line numbers it prints, and ends with the omitted count and the offset that continues from there. A2 Tool success is a structured ToolOutcome (signal kind + exit code) instead of a text prefix the agent had to sniff back out. Non-zero exits count as failures except for commands whose exit status is the answer (grep, diff, test) — Reflexion's data favours false negatives over false positives. A3 Reflection/stagnation/convergence hints moved out of the system message into a trailing user message, keeping the prompt prefix byte-stable so provider-side caching actually hits. A4 Truncated tool results from read-only tools record the original call in the omission marker, so the model can re-read instead of reasoning over a hole. Idempotency now matches the marker by shape: read_file's own windowing trailer also says "[omitted", which was silently exempting every windowed read from shrinking. dashscope unit: 702 passed, 6 skipped. pre-commit --all-files clean.
Mirrors agenticCLI 556255d. The three-failure reflection hint was a template string about a count; it now quotes the failing tool's own output plus the structured verdict (signal_kind / exit_code), tail-biased so tracebacks and FAILED lines survive truncation, and asks which assumption that output falsifies. Diagnosis and fix are demanded in the same response, so anchoring adds no model call. Only the latest failure is kept — accumulating failure history is what AlphaCodium measured as "no improvement". The experience lesson moves from hint-injection time to after the stream, storing the model's own diagnosis with an evidence fallback. test_hint_deduplicates_tool_names asserted dedup by counting a raw substring across the whole hint. The verdict line now names the last failing tool a second time by design, so the count is scoped to the summary line that renders the deduplicated set.
Vendored from agenticCLI a59e505 + 56afb36. The stagnation detector only counted read-only streaks and answered with advice the model could ignore. It now also keys repeats on call identity (tool plus canonicalised arguments) and a fingerprint of the result, at OpenHands' production thresholds (repeat action-observation 4, repeat action-error 3). _execute_tool consults the tracker before execution, so a call that already failed identically with nothing changed in between is refused rather than run — a structural interruption in the spirit of ADaPT forcing decomposition on failure instead of offering it. The refusal is audited as a denial and still answers its tool_call id. Only a successful mutating call counts as a state change, so re-running the test suite after an edit is never mistaken for a loop; and only failures are refused, so a repeated read still runs and merely escalates the prompt to a replan demand. tests/unit gains the repeat-detection coverage. The agent-level tests stay in agenticCLI, which is where the agent harness lives.
4 vendored files (acceptance.py new) plus the ported unit tests; the agent
loop now withholds a oneshot final answer until the project's test command
passes, against a baseline captured before the first mutating call.
Upstream: bda8fda (fence helper moved to utils.text) + 104a0c9 (the gate).
dashscope/acli/{acceptance,agent}.py, utils/text.py and memory/reflection.py
are byte-identical to the agenticCLI sources modulo the dashscope. import
prefix.
Verification: tests/unit 855 passed / 6 skipped (net +50, all from the new
test_acli_acceptance.py); pre-commit run --all-files green, mypy included.
The agent-loop half of the upstream tests is not ported — dashscope has no
counterpart for tests/ut/test_agent_core_fixes.py, and no dashscope test
constructs an acli Agent.
ACLI_USAGE_FILE names a path a oneshot run flushes its token totals to, so an external harness can report real usage instead of zeros. Flushed every 15s rather than once at the end: the runs that most need measuring are the ones a timeout kills, and those never reach a final write. Unset, nothing is written. Also carries the 0.6.5 -> 0.6.6 version bump. The Terminal-Bench adapter that reads the file back lives in agenticCLI's bench/, which is not vendored here.
… evaluated Synced from agenticCLI 8a2c0f6. A check that cannot execute was being read as a failure of the change. The gate only excused a byte-identical result, on the theory that a suite which genuinely fails prints timings. That holds for a real failure but not for the two commonest ways a check dies: the shell cannot find the command (127), and pytest cannot import the project at collection time (exit 2), whose output ends in `1 error in 0.05s` -- a timing, so the two runs never match. The discovered command names sys.executable, which is the right interpreter only when acli is installed into the project environment. Under pipx, `uv tool`, or a benchmark image that installs the project during its grading step instead, every run fails collection with differing timings and the gate withholds a correct answer after burning a full model round-trip. Read the exit status instead: 127 and pytest's 2/3/4/5 mean the check never reached the code. pytest's 1 stays a genuine verdict. Requiring the baseline to be inert too is what keeps this from excusing real damage -- a green baseline proves the check does run here, so a collection error the agent introduced still reads as a REGRESSION. The pytest statuses are matched only when the command names pytest, because cargo reuses 2 for a build failure.
…onour From agenticCLI d90b1bd. The advertised per-call `model` only ever retargeted `Agent.model_name`, which picks vision/audio normalization and the truncation window; the child shares the parent's provider, whose model is fixed at construction, so it prepared messages for a model that never saw them. The kwarg stays in `_delegate`'s signature so an already-learned call gets no TypeError. /subagents now labels `model` and `temperature` as stored but not applied — they have no reader anywhere.
The flush used to hang off the oneshot output loop, so a run wedged inside run_stream -- exactly the run a benchmark most needs to measure -- produced no usage file at all and the harness read back zeros.
The flusher is the part that decides whether a bench run yields numbers at all, and it had no coverage here. Each new test was checked against a deliberately broken flusher: narrowing the tick's exception guard, dropping the join in stop(), and writing the destination in place instead of renaming each fail exactly one test.
Syncs agenticCLI a2c515f into the vendored tree. HardenedProvider now marks an error whose retries it already spent with RetriesExhausted, carrying the original message verbatim so is_retryable_error still answers True and ProviderChain keeps falling back to the next profile. Agent skips its turn-level retry for that type only. Without the mark, every production provider (all of them arrive wrapped in HardenedProvider) had its budget re-run by the agent: measured 12 HTTP attempts and ~819s of silent wall clock in one turn from a 60s request_timeout, against an endpoint that accepts and never replies. Now 4 attempts. Adds tests/unit/test_acli_provider_retries.py covering the vendored copy, including the fallback invariant that a reworded marker would silently break.
Syncs agenticCLI ef05859 into the vendored tree. oneshot now reports an unabsorbed provider error on stderr and exits 1 rather than letting it escape asyncio.run as a traceback; RetriesExhausted carries the wall clock its budget cost, and Agent skips its turn-level retry only when that shows the attempts were stalling rather than failing fast; a retryable error after the first chunk keeps the partial turn and continues the loop, since no layer can retry it without duplicating output.
The module docstring promised unconditionally that Agent never re-pays an exhausted budget, which stopped being the contract: the exception type alone cannot tell a stalled endpoint (268s) from a refused connection (~28s), and telling them apart is what elapsed_sec was added for. Covers a real fast exhaustion reporting the backoff it spent, and pins the threshold on both sides plus the unmeasured default, which must not claim the expensive case -- giving up on a number nobody collected would recreate the type-only skip. Each was checked against a deliberately broken implementation and each break fails exactly one. The first attempt at breaking elapsed_sec was a false negative worth recording: the chat() and chat_stream() raise sites are textually identical, so a single replace patched chat() while the test exercises chat_stream(), and all 8 passed. Confirming the anchor count is part of the check, or "did not hit" reads as "test too weak".
Vendored copy of the agenticCLI fix. Answering [a]lways at a run_command confirmation cached the tool name, so approving one harmless command silently approved every later shell command in the turn: `curl ... | sh` and `rm -rf ~/work` both came back "trusted this turn" without a prompt. Grants now key on the exact stripped command in their own set, both permission paths consult them, the prompt label states the scope actually granted, and the echoed command is escaped. The /trust handler goes through the new trust_snapshot/trust_tool/deny_tool API instead of the executor's privates.
Twelve tests over the vendored executor: a second command still prompts, grant matching is exact rather than prefix/wildcard/case-folded, a grant with no usable command string trusts nothing, other tools still cache by name, the turn reset and snapshot copies hold, and the sync prompt path echoes the command escaped. The read-only allowlist and the global permission policy are both neutralised per-test: each is consulted before the trust cache, so either could answer the check without the grant ever being exercised.
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
dashscope/acliagent loop with agenticCLI 0.6.6: evidence-anchored reflection (A7), futility refusal with OpenHands-calibrated repeat detection (A8), and an opt-in acceptance gate (A9).HardenedProvidernow raisesRetriesExhaustedwhen it exhausts its own attempts, andAgentskips its turn-level retry for that type. Against a stalled endpoint this measured 12 HTTP attempts and ~819s of silent wall clock from a 60s configured timeout; it is now 4 attempts and ~268s. The wrapped message is preserved verbatim becauseProviderChaindecides profile fallback viais_retryable_error(e)— rewriting it would turn "this profile is spent" into "abort the whole chain".os.replaceso the harness never reads a half-written file.Test plan
pre-commit run --all-filesgreen — black, flake8, pylint, mypy and the rest, no modifications leftpytest tests/unit/test_acli_{provider_retries,acceptance,reflection,usage_report}.py— 194 passedscripts/sync_acli.shrun from agenticCLI HEAD13a68c94(acli 0.6.6)