Skip to content

Commit bda1a99

Browse files
authored
[FIX] Remediation Assess Context-Limit Recovery (#4211)
## Summary Fix the remediation recipe so context-exhausted `resolve-failures` assess sessions verify persisted partial fixes instead of failing the dispatch immediately. The pre-merge assess path routes context and rate-limit recovery back to `test`, while the merge-gate assess path routes recovery back to `merge_gate_test` so each loop keeps its existing bounded counter. Closes #4206 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/fix-4206-remediation-assess-context-limit/.autoskillit/temp/make-plan/4206-live-plan.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr -->
1 parent eba1ce1 commit bda1a99

8 files changed

Lines changed: 47 additions & 56 deletions

File tree

docs/safety/hooks.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,10 +148,11 @@ table to the PR body so reviewers can see per-step token cost.
148148

149149
### `quota_post_hook.py`
150150
**Guarded tool:** `run_skill`
151-
After `run_skill` returns, appends a quota warning to the tool output when
151+
After `run_skill` returns, replaces the display output with a quota warning when
152152
the cached binding window marks `should_block=True` (per-window threshold —
153153
see `quota_guard.py` above). Gives the orchestrator a chance to back off
154-
voluntarily before the PreToolUse hook starts denying.
154+
voluntarily before the PreToolUse hook starts denying. The replacement output is
155+
generated by the hook and does not echo raw `tool_response` content.
155156

156157
### `review_gate_post_hook.py`
157158
**Guarded tools:** `run_skill`, `run_python`

src/autoskillit/hooks/quota_post_hook.py

Lines changed: 2 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
"""PostToolUse hook: quota warning after run_skill execution.
33
44
Fires after run_skill completes and checks whether the cached binding marks
5-
``should_block=True``. When set, replaces the tool output with a compact
6-
result summary + quota warning + sleep instruction via updatedMCPToolOutput.
5+
``should_block=True``. When set, replaces the tool output with a quota warning
6+
and sleep instruction via updatedMCPToolOutput.
77
88
This script is stdlib-only so it can run under any Python interpreter without
99
requiring the autoskillit package to be importable.
@@ -37,29 +37,6 @@
3737
QUOTA_POST_BUDGET_EXCEEDED_TRIGGER: str = "QUOTA BUDGET EXCEEDED"
3838

3939

40-
def _extract_run_skill_result(tool_response: str | dict) -> str:
41-
"""Extract a compact summary from the run_skill double-wrapped JSON response."""
42-
try:
43-
outer = json.loads(tool_response) if isinstance(tool_response, str) else tool_response
44-
if isinstance(outer, dict) and "result" in outer:
45-
inner_str = outer["result"]
46-
if isinstance(inner_str, str):
47-
try:
48-
inner = json.loads(inner_str)
49-
if isinstance(inner, dict):
50-
success = inner.get("success", "unknown")
51-
result_text = inner.get("result", "")
52-
if isinstance(result_text, str) and len(result_text) > 500:
53-
result_text = result_text[:500] + "..."
54-
return f"success: {success}\nresult: {result_text}"
55-
except (json.JSONDecodeError, ValueError):
56-
pass
57-
return inner_str[:500] if len(inner_str) > 500 else inner_str
58-
return str(outer)[:500]
59-
except (json.JSONDecodeError, ValueError, TypeError):
60-
return str(tool_response)[:500]
61-
62-
6340
def main(*, cache_path_override: str | None = None) -> None:
6441
try:
6542
raw = sys.stdin.read()
@@ -71,8 +48,6 @@ def main(*, cache_path_override: str | None = None) -> None:
7148
sys.exit(0)
7249

7350
tool_name = event.get("tool_name", "")
74-
tool_response = event.get("tool_response") or ""
75-
7651
settings = resolve_quota_settings(cache_path_override=cache_path_override)
7752
if settings.disabled:
7853
sys.exit(0) # quota guard disabled for this session
@@ -140,8 +115,6 @@ def main(*, cache_path_override: str | None = None) -> None:
140115
else:
141116
n = settings.buffer_seconds
142117

143-
result_summary = _extract_run_skill_result(tool_response)
144-
145118
session_deadline_str = os.environ.get("AUTOSKILLIT_SESSION_DEADLINE")
146119
budget_exceeded = False
147120
remaining_budget = float("inf")
@@ -157,7 +130,6 @@ def main(*, cache_path_override: str | None = None) -> None:
157130
if budget_exceeded:
158131
resets_at_display = resets_at_str or "unknown"
159132
warning_text = (
160-
f"{result_summary}\n\n"
161133
f"{QUOTA_POST_BUDGET_EXCEEDED_TRIGGER}\n"
162134
f"Post-execution utilization: {utilization:.0f}% on window '{window_name}' "
163135
f"(threshold: {effective_threshold:.0f}%)\n"
@@ -173,7 +145,6 @@ def main(*, cache_path_override: str | None = None) -> None:
173145
)
174146
else:
175147
warning_text = (
176-
f"{result_summary}\n\n"
177148
f"{QUOTA_POST_WARNING_TRIGGER}\n"
178149
f"Post-execution utilization: {utilization:.0f}% on window '{window_name}' "
179150
f"(threshold: {effective_threshold:.0f}%)\n"

src/autoskillit/recipes/diagrams/remediation.md

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
<!-- autoskillit-recipe-hash: sha256:b46326a0d43bc9c41fa2615979dfbe9df18d296cd2e171be01348976a34d4b1a -->
1+
<!-- autoskillit-recipe-hash: sha256:c8205806500c54e98e1dd94e2f09f106c0b1603fcc60e2958ab70788c59a39c9 -->
22
<!-- autoskillit-diagram-format: v7 -->
33
## remediation
44
Investigate, rectify, implement, and merge a bug fix with CI and PR gates.
@@ -12,10 +12,13 @@ investigate → rectify → review_approach (optional)
1212
|
1313
dry_walkthrough → implement ↔ [retry_worktree on context limit]
1414
|
15-
test → assess
15+
test ↔ assess
16+
[assess context/rate limit → test]
1617
|
1718
+-- audit_impl → reset_test_fix_counter → reset_merge_test_fix_counter
1819
→ reset_ref_push_counter → pre_remediation_merge → remediate (optional)
20+
→ merge_gate_test ↔ merge_gate_assess
21+
[merge_gate_assess context/rate limit → merge_gate_test]
1922
|
2023
make_plan → commit_guard → merge → push
2124
|

src/autoskillit/recipes/remediation.json

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -777,8 +777,8 @@
777777
"route": "release_issue_failure"
778778
}
779779
],
780-
"on_context_limit": "release_issue_failure",
781-
"on_rate_limit": "release_issue_failure",
780+
"on_context_limit": "test",
781+
"on_rate_limit": "test",
782782
"on_failure": "release_issue_failure",
783783
"on_exhausted": "release_issue_failure",
784784
"optional": true,
@@ -855,8 +855,8 @@
855855
"route": "release_issue_failure"
856856
}
857857
],
858-
"on_context_limit": "release_issue_failure",
859-
"on_rate_limit": "release_issue_failure",
858+
"on_context_limit": "merge_gate_test",
859+
"on_rate_limit": "merge_gate_test",
860860
"on_failure": "release_issue_failure",
861861
"on_exhausted": "release_issue_failure",
862862
"optional": true,

src/autoskillit/recipes/remediation.yaml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -716,8 +716,8 @@ steps:
716716
- when: ${{ result.verdict }} == 'no_test_infrastructure'
717717
route: release_issue_failure
718718
- route: release_issue_failure
719-
on_context_limit: release_issue_failure
720-
on_rate_limit: release_issue_failure
719+
on_context_limit: test
720+
on_rate_limit: test
721721
on_failure: release_issue_failure
722722
on_exhausted: release_issue_failure
723723
optional: true
@@ -772,8 +772,8 @@ steps:
772772
- when: ${{ result.verdict }} == 'no_test_infrastructure'
773773
route: release_issue_failure
774774
- route: release_issue_failure
775-
on_context_limit: release_issue_failure
776-
on_rate_limit: release_issue_failure
775+
on_context_limit: merge_gate_test
776+
on_rate_limit: merge_gate_test
777777
on_failure: release_issue_failure
778778
on_exhausted: release_issue_failure
779779
optional: true

tests/hooks/test_quota_post_check.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -184,17 +184,18 @@ def test_qpc6_warning_contains_sleep_instruction(tmp_path):
184184
assert "time.sleep" in output
185185

186186

187-
# T7: Warning preserves run_skill result summary
188-
def test_qpc7_warning_preserves_result_summary(tmp_path):
189-
"""updatedMCPToolOutput includes the run_skill result (success/fail, key fields)."""
187+
# T7: Warning output is hook-generated only
188+
def test_qpc7_warning_does_not_forward_tool_response(tmp_path):
189+
"""updatedMCPToolOutput must not echo arbitrary raw tool_response content."""
190190
cache = tmp_path / "quota_cache.json"
191191
_write_cache(cache, utilization=90.0)
192192
event = _build_event(success=True, result_text="plan written")
193193
out, _ = _run_hook(event=event, cache_path=cache)
194194
data = json.loads(out)
195195
output = data["hookSpecificOutput"]["updatedMCPToolOutput"]
196-
assert "success: True" in output
197-
assert "plan written" in output
196+
assert "QUOTA WARNING" in output
197+
assert "success: True" not in output
198+
assert "plan written" not in output
198199

199200

200201
# T8: JSONL event logging for post-check warning

tests/recipe/test_bundled_recipes_general.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,10 @@ def test_pre_merge_test_fix_cycle_has_guard(recipe_name: str) -> None:
8585

8686

8787
@pytest.mark.parametrize("recipe_name", _PRE_MERGE_CYCLE_RECIPES)
88-
def test_pre_merge_fix_on_context_limit_routes_to_failure(recipe_name: str) -> None:
89-
"""fix/assess on_context_limit must route to release_issue_failure (safe default)."""
88+
def test_pre_merge_fix_on_context_limit_uses_expected_recovery(
89+
recipe_name: str,
90+
) -> None:
91+
"""fix/assess context-limit routing must preserve resumable partial work."""
9092
recipe = load_recipe(_resolve_recipe_path(recipe_name))
9193
test_step = recipe.steps["test"]
9294
guard_name = test_step.on_failure
@@ -98,9 +100,10 @@ def test_pre_merge_fix_on_context_limit_routes_to_failure(recipe_name: str) -> N
98100
)
99101
assert fix_step_name is not None
100102
fix_step = recipe.steps[fix_step_name]
101-
assert fix_step.on_context_limit == "release_issue_failure", (
103+
expected_route = "test" if recipe_name == "remediation" else "release_issue_failure"
104+
assert fix_step.on_context_limit == expected_route, (
102105
f"{recipe_name}: {fix_step_name}.on_context_limit must route to "
103-
f"'release_issue_failure', got: {fix_step.on_context_limit!r}"
106+
f"{expected_route!r}, got: {fix_step.on_context_limit!r}"
104107
)
105108

106109

tests/recipe/test_bundled_recipes_pipeline_structure.py

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1005,13 +1005,25 @@ def test_remediation_no_add_dir_dead_param(self, recipe) -> None:
10051005
f"{[(f.step_name, f.message) for f in add_dir_dead]}"
10061006
)
10071007

1008-
def test_remediation_assess_step_on_context_limit_routes_to_failure(self, recipe) -> None:
1009-
"""assess step must route on_context_limit to release_issue_failure (safe default)."""
1008+
def test_remediation_assess_step_on_context_limit_routes_to_test(self, recipe) -> None:
1009+
"""assess context exhaustion must verify persisted partial fixes."""
10101010
assess = recipe.steps["assess"]
1011-
assert assess.on_context_limit == "release_issue_failure", (
1012-
f"remediation.yaml assess step must declare on_context_limit: release_issue_failure, "
1011+
assert assess.on_context_limit == "test", (
1012+
f"remediation.yaml assess step must declare on_context_limit: test, "
10131013
f"got: {assess.on_context_limit!r}"
10141014
)
1015+
assert assess.on_rate_limit == assess.on_context_limit
1016+
1017+
def test_remediation_merge_gate_assess_context_limit_routes_to_merge_gate_test(
1018+
self, recipe
1019+
) -> None:
1020+
"""merge-gate assess context exhaustion must stay on the merge-gate loop."""
1021+
assess = recipe.steps["merge_gate_assess"]
1022+
assert assess.on_context_limit == "merge_gate_test", (
1023+
"remediation.yaml merge_gate_assess step must declare "
1024+
f"on_context_limit: merge_gate_test, got: {assess.on_context_limit!r}"
1025+
)
1026+
assert assess.on_rate_limit == assess.on_context_limit
10151027

10161028

10171029
class TestDisplayConditionContract:

0 commit comments

Comments
 (0)