fix: agent quality bundle (resume-loop + telemetry + skill governance) - #216
Merged
Conversation
Five cross-cutting fixes surfaced by live session 26f73ee4 analysis. 1. plan_executor resume-loop (bug #5) Before: paused-with-no-ready-subtasks → user types '继续' → executor continues, re-analyzes deps, finds same blocked set, immediately re-pauses with no diagnostic. Observed as an endless 继续→pause cycle. After: - PlanPaused event now carries blocked_ids so the monitor prints 'blocked by: step-4, step-5' instead of just a count. - On entering the blocked path we auto-heal any orphaned InProgress subtasks → Pending and loop back (common root cause: a crashed worker left its subtask running). - After Resume, if the re-analysis produces the exact same blocked set we abort with a descriptive PlanError listing who-blocks-whom and pointing the user at rewind/cancel, instead of re-pausing forever. 2. agentic_step telemetry (bug #4) auto_reflection's llm_round record hardcoded agentic_step: None, which is why 32/32 rounds in session 26f73ee4 had null agentic_step. Populate it via current_agentic_step(state) like every other llm_round emitter does. 3. github_create_pr empty-diff guard (bug #3) Session showed the agent calling github_create_pr on an empty diff (no commits since subtask start). The gh CLI rejects it but we waste an LLM round. Added a pre-flight step to the skill that runs 'git rev-list --count main..HEAD' and stops with a clear message when the count is 0. 4. Subtask prompt sharpening (bug #2) In session 26f73ee4 the agent emitted full implementation as markdown code blocks in the assistant response while calling zero write_file/str_replace tools. Extended format_subtask_prompt to explicitly require tool-call mutations (write_file / str_replace / bash) and forbid markdown-only 'implementations', plus a reminder that consulting a skill doesn't satisfy the subtask. 5. Skill-loop governor (bug #1) Added skill and discover_skills to a new CONSULTATIVE_TOOLS list consumed by the top-tool stall diagnostic. When the build_stall_ reflection detector sees an agent call skill ≥3 times in its 6-round window it now classifies the run as 'used skill N times … without progressing' and emits a nudge telling the agent to stop consulting and take direct action. Kept EXPLORATION_TOOLS unchanged so single-round reward-hacking classification of e.g. [skill, read_file] is not over-blocked — the existing soft-defer path for legitimate post-skill follow-ups stays intact. Tests: - All 2159 astra-runtime lib tests pass. - All 2358 astra-turn-core lib tests pass. - 267 astra-plan + 3 astra-cli tests pass. - make check clean. Refs: session 26f73ee4-51a5-44e9-90c2-fc475b77f463. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
XuPeng-SH
enabled auto-merge (squash)
April 22, 2026 18:09
Review feedback: the empty-diff guard in Phase 1.4 hardcoded 'main', so a
repo whose default is master/develop/trunk would report 0 commits and
wrongly block PR creation. Switch to ${BASE:-main} so the check honours
the same base branch the user passed as the BASE argument.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Lock in the contract of the fixes from earlier in this PR: ChannelSink (plan_executor.rs): - channel_sink_plan_paused_forwards_blocked_ids: verifies that the comma-separated blocked_ids string is parsed into a Vec<String> on PlanUpdate::PlanPaused. Pre-fix this field was underscore-dropped. - channel_sink_plan_paused_handles_empty_blocked: empty / whitespace- only input must not yield phantom '' entries that would render as 'blocked by: ' in the REPL monitor. - channel_sink_interrupted_pause_emits_empty_blocked: the Ctrl+C pause path must emit an empty blocked_ids and zero elapsed so the monitor doesn't mis-render the pause as dependency-blocked. StallReflection (stall.rs): - reflection_triggers_on_skill_obsession: three consecutive rounds of 'skill' with no mutating tool must produce the exploration-stall diagnostic with confidence ≥0.7 and avoid_tools containing 'skill'. This is the regression that session 26f73ee4 surfaced. - reflection_triggers_on_discover_skills_loop: same contract for discover_skills. All 5 tests pass; make check clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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.
Five cross-cutting agent-quality fixes surfaced by live session analysis (
~/.astra/sessions/26f73ee4-51a5-44e9-90c2-fc475b77f463.jsonl, qwen-turbo, 117 events).Fixes
1. Plan-executor resume-loop (highest UX win)
When a plan paused with no ready subtasks (blocked deps) the user would type
继续and the executor immediately re-paused on the same blocked set — an endless loop with no diagnostic info.PlanUpdate::PlanPausednow carriesblocked_idsso the monitor showsblocked by: step-4, step-5instead of just a count.InProgresssubtasks →Pending(common root cause: crashed worker).Resume, if re-analysis yields the same blocked set as before, abort with a descriptivePlanErrorlisting who-blocks-whom instead of looping.2.
agentic_steptelemetry gapauto_reflectionrounds hardcodedagentic_step: None. 32/32llm_roundevents in session 26f73ee4 had nullagentic_step. Now populated viacurrent_agentic_step(state).3.
github_create_prempty-diff guardSession showed the agent calling PR creation on an empty diff. Added pre-flight
git rev-list --count main..HEADcheck to the skill that stops with a clear message when the count is 0.4. Subtask prompt sharpening
The agent emitted full implementation as markdown code blocks while calling zero
write_file/str_replacetools. Extendedformat_subtask_promptto explicitly require tool-call mutations and forbid markdown-only 'implementations', plus a reminder that consulting a skill doesn't satisfy a subtask.5. Skill-loop governor
Added
skillanddiscover_skillsto a newCONSULTATIVE_TOOLSlist consumed only by the top-tool stall diagnostic (build_stall_reflection). When the 6-round window sees ≥3skillcalls the detector fires the 'stop consulting, take direct action' nudge. LeftEXPLORATION_TOOLSunchanged so the existing soft-defer path for legitimate post-skill follow-ups (e.g.[skill, read_file]) stays intact.Tests
make format+make checkcleanRefs: session
26f73ee4-51a5-44e9-90c2-fc475b77f463.