Skip to content

Quota Guard Three-Layer Resilience#668

Merged
Trecek merged 9 commits intointegrationfrom
quota-guard-hook-inconsistently-blocks-run-skill-instead-of/663
Apr 8, 2026
Merged

Quota Guard Three-Layer Resilience#668
Trecek merged 9 commits intointegrationfrom
quota-guard-hook-inconsistently-blocks-run-skill-instead-of/663

Conversation

@Trecek
Copy link
Copy Markdown
Collaborator

@Trecek Trecek commented Apr 8, 2026

Summary

The quota guard hook (quota_check.py) blocks run_skill via PreToolUse deny when API quota exceeds threshold, embedding a sleep-and-retry instruction in the deny reason text. The LLM inconsistently follows this instruction — sometimes it sleeps and retries, sometimes it treats the denial as terminal. The root cause is architectural: the entire retry mechanism has a single point of failure (LLM compliance with a text instruction in a deny message).

Fix with three reinforcing layers:

  1. PreToolUse hardening — Improve the deny message with echo/repeat technique and structured formatting
  2. PostToolUse quota warning — New hook that appends quota warning + sleep instruction to run_skill output via updatedMCPToolOutput when post-execution utilization exceeds threshold
  3. Instruction hardening — Add explicit quota denial handling rules to orchestrator prompt and sous-chef global rules

Architecture Impact

Error/Resilience Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef gap fill:#ff6f00,stroke:#ffa726,stroke-width:2px,color:#000;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    %% TERMINAL NODES %%
    CALL(["run_skill invocation"])
    SUCCESS(["STEP COMPLETE"])
    DENIED(["DENIED — sleep then retry"])

    subgraph CacheLayer ["QUOTA CACHE (execution/quota.py)"]
        direction TB
        CACHE["Quota Cache File<br/>━━━━━━━━━━<br/>~/.claude/autoskillit_quota_cache.json<br/>TTL: 300s"]
        FETCH["_fetch_quota<br/>━━━━━━━━━━<br/>Anthropic API via httpx"]
        CRED["_read_credentials<br/>━━━━━━━━━━<br/>OAuth token file"]
        CACHE_MISS{"Cache<br/>miss or<br/>stale?"}
        CACHE_ERR["Cache Read Error<br/>━━━━━━━━━━<br/>FileNotFound, JSON,<br/>KeyError → None"]
    end

    subgraph Layer1 ["LAYER 1 — PreToolUse Gate (● quota_check.py)"]
        direction TB
        L1_READ["● Read Cache<br/>━━━━━━━━━━<br/>_read_quota_cache()"]
        L1_VALID{"Cache<br/>valid?"}
        L1_PARSE["● Parse Utilization<br/>━━━━━━━━━━<br/>five_hour.utilization"]
        L1_GATE{"utilization<br/>≥ threshold<br/>(default 85%)?"}
        L1_SLEEP["● Compute Sleep<br/>━━━━━━━━━━<br/>resets_at + 60s buffer<br/>fallback: 60s"]
        L1_DENY["● Emit Deny JSON<br/>━━━━━━━━━━<br/>permissionDecision: deny<br/>+ run_cmd sleep instruction<br/>+ QUOTA WAIT REQUIRED"]
        L1_LOG["● Log Event<br/>━━━━━━━━━━<br/>quota_events.jsonl<br/>blocked / approved / cache_miss"]
    end

    subgraph Layer2 ["LAYER 2 — PostToolUse Warning (★ quota_post_check.py)"]
        direction TB
        L2_READ["★ Read Cache<br/>━━━━━━━━━━<br/>_read_quota_cache()"]
        L2_VALID{"Cache<br/>valid?"}
        L2_PARSE["★ Parse Utilization<br/>━━━━━━━━━━<br/>five_hour.utilization"]
        L2_GATE{"utilization<br/>≥ threshold?"}
        L2_SLEEP["★ Compute Sleep<br/>━━━━━━━━━━<br/>resets_at + 60s buffer"]
        L2_WARN["★ Inject Warning<br/>━━━━━━━━━━<br/>updatedMCPToolOutput<br/>--- QUOTA WARNING ---<br/>+ run_cmd sleep instruction"]
        L2_LOG["★ Log Event<br/>━━━━━━━━━━<br/>quota_events.jsonl<br/>post_check_warning / pass"]
    end

    subgraph Layer3 ["LAYER 3 — Behavioral Contract (● sous-chef SKILL.md)"]
        direction TB
        L3_DETECT{"Denial or<br/>warning<br/>detected?"}
        L3_SLEEP_CMD["● Execute run_cmd<br/>━━━━━━━━━━<br/>python3 -c time.sleep(N)<br/>timeout=N+30"]
        L3_RETRY["● Retry Exact Same<br/>━━━━━━━━━━<br/>Same skill_command,<br/>cwd, model, step_name"]
        L3_PROCEED["● Proceed to Next Step<br/>━━━━━━━━━━<br/>Normal pipeline flow"]
    end

    subgraph FailOpen ["FAIL-OPEN GATES (all hooks)"]
        direction TB
        FO_CACHE["Cache Missing/Corrupt<br/>━━━━━━━━━━<br/>→ sys.exit(0) approve"]
        FO_PARSE["Utilization Unparseable<br/>━━━━━━━━━━<br/>→ sys.exit(0) approve"]
        FO_STDIN["Malformed Stdin<br/>━━━━━━━━━━<br/>→ sys.exit(0) approve"]
        FO_LOG["Log Dir Unresolvable<br/>━━━━━━━━━━<br/>→ silently skip logging"]
    end

    subgraph PromptRouting ["ORCHESTRATOR PROMPT (● _prompts.py)"]
        direction TB
        PR_INLINE["● Inline Quota Rules<br/>━━━━━━━━━━<br/>QUOTA DENIAL ROUTING<br/>hardcoded in prompt"]
        PR_INJECT["● Sous-Chef Injection<br/>━━━━━━━━━━<br/>SKILL.md appended verbatim<br/>redundant reinforcement"]
    end

    %% MAIN FLOW — Layer 1 %%
    CALL --> L1_READ
    L1_READ --> CACHE
    CACHE --> L1_VALID
    L1_VALID -->|"missing/stale/corrupt"| FO_CACHE
    FO_CACHE -->|"fail-open: approve"| SUCCESS
    L1_VALID -->|"valid"| L1_PARSE
    L1_PARSE -->|"parse error"| FO_PARSE
    FO_PARSE -->|"fail-open: approve"| SUCCESS
    L1_PARSE -->|"parsed"| L1_GATE
    L1_GATE -->|"below threshold"| L1_LOG
    L1_LOG -->|"approved"| SUCCESS
    L1_GATE -->|"≥ threshold"| L1_SLEEP
    L1_SLEEP --> L1_DENY
    L1_DENY --> L1_LOG
    L1_LOG -->|"blocked"| DENIED

    %% CACHE REFRESH %%
    CACHE_MISS -->|"yes"| CRED
    CRED -->|"expired token"| CACHE_ERR
    CRED -->|"valid"| FETCH
    FETCH -->|"success"| CACHE
    FETCH -->|"httpx.HTTPError / TimeoutError"| CACHE_ERR
    CACHE_ERR -->|"fail-open: should_sleep=false"| SUCCESS

    %% MAIN FLOW — Layer 2 (post-execution) %%
    SUCCESS -->|"run_skill completes"| L2_READ
    L2_READ --> CACHE
    CACHE --> L2_VALID
    L2_VALID -->|"missing/stale"| FO_CACHE
    L2_VALID -->|"valid"| L2_PARSE
    L2_PARSE -->|"parse error"| FO_PARSE
    L2_PARSE -->|"parsed"| L2_GATE
    L2_GATE -->|"below threshold"| L2_LOG
    L2_LOG -->|"post_check_pass"| L3_PROCEED
    L2_GATE -->|"≥ threshold"| L2_SLEEP
    L2_SLEEP --> L2_WARN
    L2_WARN --> L2_LOG

    %% MAIN FLOW — Layer 3 (behavioral) %%
    DENIED --> L3_DETECT
    L2_LOG -->|"post_check_warning"| L3_DETECT
    L3_DETECT -->|"PreToolUse denial"| L3_SLEEP_CMD
    L3_SLEEP_CMD -->|"sleep complete"| L3_RETRY
    L3_RETRY -->|"re-invoke"| CALL
    L3_DETECT -->|"PostToolUse warning"| L3_SLEEP_CMD
    L3_SLEEP_CMD -->|"warning path"| L3_PROCEED

    %% PROMPT ROUTING feeds Layer 3 %%
    PR_INLINE --> L3_DETECT
    PR_INJECT --> L3_DETECT

    %% FAIL-OPEN: malformed stdin %%
    CALL -->|"stdin parse fails"| FO_STDIN
    FO_STDIN --> SUCCESS

    %% CLASS ASSIGNMENTS %%
    class CALL,SUCCESS,DENIED terminal;
    class CACHE,CACHE_MISS stateNode;
    class FETCH,CRED handler;
    class CACHE_ERR gap;
    class L1_READ,L1_PARSE,L1_SLEEP,L1_DENY,L1_LOG handler;
    class L1_VALID,L1_GATE stateNode;
    class L2_READ,L2_PARSE,L2_SLEEP,L2_WARN,L2_LOG newComponent;
    class L2_VALID,L2_GATE stateNode;
    class L3_DETECT stateNode;
    class L3_SLEEP_CMD,L3_RETRY,L3_PROCEED handler;
    class FO_CACHE,FO_PARSE,FO_STDIN,FO_LOG gap;
    class PR_INLINE,PR_INJECT phase;
Loading

Process Flow Diagram

%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 60, 'curve': 'basis'}}}%%
flowchart TB
    %% CLASS DEFINITIONS %%
    classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;
    classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff;
    classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff;
    classDef newComponent fill:#2e7d32,stroke:#81c784,stroke-width:2px,color:#fff;
    classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff;
    classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff;
    classDef terminal fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff;

    %% TERMINALS %%
    START(["START<br/>━━━━━━━━━━<br/>Orchestrator calls run_skill"])
    COMPLETE(["COMPLETE<br/>━━━━━━━━━━<br/>run_skill result delivered"])
    ERROR(["ERROR<br/>━━━━━━━━━━<br/>Pipeline failure route"])

    subgraph L1 ["Layer 1: ● PreToolUse — quota_check.py"]
        direction TB
        L1_READ["● Read Hook Config<br/>━━━━━━━━━━<br/>threshold, cache_max_age<br/>from .autoskillit_hook_config.json"]
        L1_CACHE{"● Cache Fresh?<br/>━━━━━━━━━━<br/>reads quota_cache.json<br/>max_age check"}
        L1_PARSE{"● Utilization<br/>>= Threshold?<br/>━━━━━━━━━━<br/>five_hour.utilization"}
        L1_APPROVE["● Approve<br/>━━━━━━━━━━<br/>exit 0, no JSON output"]
        L1_DENY["● Deny + Sleep Cmd<br/>━━━━━━━━━━<br/>permissionDecision: deny<br/>QUOTA WAIT REQUIRED<br/>writes sleep_seconds to msg"]
        L1_LOG["● Log Event<br/>━━━━━━━━━━<br/>writes quota_events.jsonl<br/>blocked / approved / cache_miss"]
    end

    subgraph L2 ["Layer 2: ★ PostToolUse — quota_post_check.py"]
        direction TB
        L2_READ["★ Read Hook Config<br/>━━━━━━━━━━<br/>threshold, cache_max_age"]
        L2_CACHE{"★ Cache Fresh?<br/>━━━━━━━━━━<br/>reads quota_cache.json"}
        L2_PARSE{"★ Utilization<br/>>= Threshold?<br/>━━━━━━━━━━<br/>post-execution check"}
        L2_PASS["★ Pass Through<br/>━━━━━━━━━━<br/>exit 0, output unchanged"]
        L2_WARN["★ Inject Warning<br/>━━━━━━━━━━<br/>updatedMCPToolOutput<br/>--- QUOTA WARNING ---<br/>+ sleep command"]
        L2_EXTRACT["★ Extract Result Summary<br/>━━━━━━━━━━<br/>unwrap double-wrapped JSON<br/>truncate to 500 chars"]
    end

    subgraph L3 ["Layer 3: ● Orchestrator Protocol — sous-chef SKILL.md + _prompts.py"]
        direction TB
        L3_DETECT_DENY{"● Deny Detected?<br/>━━━━━━━━━━<br/>QUOTA WAIT REQUIRED<br/>in hook response"}
        L3_SLEEP_DENY["● Execute Sleep<br/>━━━━━━━━━━<br/>run_cmd: python3 -c<br/>time.sleep N"]
        L3_RETRY["● Retry Same run_skill<br/>━━━━━━━━━━<br/>identical arguments<br/>skill_command, cwd, model"]
        L3_DETECT_WARN{"● Warning Detected?<br/>━━━━━━━━━━<br/>--- QUOTA WARNING ---<br/>in tool output"}
        L3_SLEEP_WARN["● Execute Pre-emptive Sleep<br/>━━━━━━━━━━<br/>run_cmd: python3 -c<br/>time.sleep N"]
    end

    subgraph INFRA ["Shared Infrastructure"]
        direction LR
        CACHE_FILE["Quota Cache File<br/>━━━━━━━━━━<br/>~/.claude/<br/>autoskillit_quota_cache.json<br/>written by execution/quota.py"]
        LOG_FILE["● Quota Events Log<br/>━━━━━━━━━━<br/>quota_events.jsonl<br/>at session log root"]
        HOOK_CFG["Hook Config<br/>━━━━━━━━━━<br/>.autoskillit/temp/<br/>.autoskillit_hook_config.json<br/>written by open_kitchen"]
    end

    subgraph REG ["● Hook Registry — hook_registry.py"]
        direction LR
        REG_PRE["● PreToolUse Entry<br/>━━━━━━━━━━<br/>matcher: run_skill<br/>scripts: quota_check.py"]
        REG_POST["● PostToolUse Entry<br/>━━━━━━━━━━<br/>matcher: run_skill<br/>scripts: quota_post_check.py"]
    end

    %% MAIN FLOW %%
    START -->|"orchestrator calls<br/>run_skill"| REG_PRE
    REG_PRE -->|"Claude Code fires<br/>PreToolUse hook"| L1_READ
    L1_READ -->|"reads"| L1_CACHE
    L1_CACHE -->|"miss / stale / corrupt"| L1_APPROVE
    L1_CACHE -->|"fresh cache"| L1_PARSE
    L1_PARSE -->|"below threshold"| L1_APPROVE
    L1_PARSE -->|"at or above threshold"| L1_DENY
    L1_APPROVE --> L1_LOG
    L1_DENY --> L1_LOG

    L1_APPROVE -->|"run_skill proceeds"| EXEC["run_skill Executes<br/>━━━━━━━━━━<br/>headless Claude session<br/>updates quota_cache.json"]
    L1_DENY -->|"run_skill blocked"| L3_DETECT_DENY

    %% Post-execution path %%
    EXEC -->|"Claude Code fires<br/>PostToolUse hook"| REG_POST
    REG_POST --> L2_READ
    L2_READ -->|"reads"| L2_CACHE
    L2_CACHE -->|"miss / stale"| L2_PASS
    L2_CACHE -->|"fresh cache"| L2_PARSE
    L2_PARSE -->|"below threshold"| L2_PASS
    L2_PARSE -->|"at or above"| L2_EXTRACT
    L2_EXTRACT --> L2_WARN

    L2_PASS -->|"output unchanged"| L3_DETECT_WARN
    L2_WARN -->|"output replaced with<br/>summary + warning"| L3_DETECT_WARN

    %% Layer 3 routing %%
    L3_DETECT_DENY -->|"yes"| L3_SLEEP_DENY
    L3_SLEEP_DENY -->|"sleep complete"| L3_RETRY
    L3_RETRY -->|"re-enters Layer 1"| REG_PRE

    L3_DETECT_WARN -->|"no warning"| COMPLETE
    L3_DETECT_WARN -->|"warning present"| L3_SLEEP_WARN
    L3_SLEEP_WARN -->|"sleep complete,<br/>proceed to next step"| COMPLETE

    %% Infrastructure reads %%
    L1_CACHE -.->|"reads"| CACHE_FILE
    L2_CACHE -.->|"reads"| CACHE_FILE
    L1_READ -.->|"reads"| HOOK_CFG
    L2_READ -.->|"reads"| HOOK_CFG
    L1_LOG -.->|"writes"| LOG_FILE
    L2_WARN -.->|"writes"| LOG_FILE
    EXEC -.->|"writes"| CACHE_FILE

    %% CLASS ASSIGNMENTS %%
    class START,COMPLETE,ERROR terminal;
    class L1_READ,L1_APPROVE,L1_DENY,L1_LOG handler;
    class L1_CACHE,L1_PARSE stateNode;
    class L2_READ,L2_EXTRACT newComponent;
    class L2_PASS,L2_WARN newComponent;
    class L2_CACHE,L2_PARSE newComponent;
    class L3_DETECT_DENY,L3_DETECT_WARN stateNode;
    class L3_SLEEP_DENY,L3_SLEEP_WARN,L3_RETRY phase;
    class EXEC handler;
    class CACHE_FILE,LOG_FILE,HOOK_CFG output;
    class REG_PRE,REG_POST handler;
Loading

Closes #663

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/impl-20260408-085339-101359/.autoskillit/temp/make-plan/quota_guard_three_layer_resilience_plan_2026-04-08_085339.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step uncached output cache_read cache_write count time
plan 36 16.7k 1.1M 77.1k 1 12m 58s
verify 19 15.7k 774.6k 56.3k 1 8m 24s
implement 60 22.5k 3.7M 89.4k 1 8m 48s
fix 56 12.7k 1.9M 87.6k 2 9m 39s
prepare_pr 8 4.1k 96.4k 28.5k 1 1m 12s
run_arch_lenses 5.7k 12.4k 574.2k 81.9k 2 5m 39s
compose_pr 10 7.7k 207.7k 31.8k 1 2m 30s
Total 5.9k 91.8k 8.4M 452.5k 49m 13s

Trecek and others added 6 commits April 8, 2026 09:25
Layer 2 of the three-layer quota resilience strategy. Fires after
run_skill completes and checks post-execution quota utilization.
When over threshold, replaces tool output with a compact result
summary + quota warning + explicit sleep instruction via
updatedMCPToolOutput.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Register the new PostToolUse hook in HOOK_REGISTRY after
token_summary_appender.py (last-writer-wins for updatedMCPToolOutput).

Harden the PreToolUse deny message with echo/repeat technique:
- Mark as "QUOTA WAIT REQUIRED" (temporary, NOT permanent)
- Show utilization/threshold values
- Use "MANDATORY ACTION" framing
- Include echo/repeat instruction ("state aloud")
- Explicitly instruct retry with identical arguments

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Layer 3 of the three-layer quota resilience strategy:
- Add QUOTA DENIAL ROUTING section to orchestrator prompt between
  CONTEXT LIMIT ROUTING and TWO FAILURE TIERS blocks
- Add QUOTA WAIT PROTOCOL section to sous-chef global rules after
  MERGE PHASE section

Both instruct the LLM to treat quota denials as temporary blocks,
execute the sleep command, and retry — reinforcing the PreToolUse
deny message and PostToolUse warning.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- tests/infra/test_quota_post_check.py: T1-T12, T17-T18 covering
  PostToolUse quota warning hook behavior, fail-open, logging, config
- tests/infra/test_quota_check.py: T14 echo/repeat deny message +
  update T2 assertion for new deny message format
- tests/cli/test_cli_prompts.py: T15 QUOTA DENIAL ROUTING in prompt
- tests/contracts/test_sous_chef_quota_protocol.py: T16 QUOTA WAIT
  PROTOCOL section presence

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Break long f-string in quota_check.py deny message and shorten
docstring in test_quota_post_check.py to fit within 99-char limit.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y dedup

- Add quota_post_check.py to _PRINT_EXEMPT and _BROAD_EXCEPT_EXEMPT (stdlib-only hook)
- Bump hooks/ subpackage file limit from 12 to 13
- Consolidate duplicate PostToolUse run_skill matchers into single HookDef
- Document quota_post_check.py in CLAUDE.md architecture section

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Collaborator Author

@Trecek Trecek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AutoSkillit PR Review — Verdict: changes_requested (posted as COMMENT because reviewer is PR author)

except Exception:
sys.exit(0)

tool_name = event.get("tool_name", "")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[critical] defense: event = json.loads(raw) may produce a non-dict (e.g. JSON array or string). The .get() calls at L118-L119 are outside the try/except block, so a non-dict JSON value raises AttributeError uncaught, crashing the hook. Add if not isinstance(event, dict): sys.exit(0) immediately after parsing.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — already fixed in 28ec684. Lines 116-117 now have if not isinstance(event, dict): sys.exit(0) immediately after json.loads, preventing AttributeError on non-dict JSON values.

return None
try:
data = json.loads(cache_path.read_text())
fetched = datetime.fromisoformat(data["fetched_at"])
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] defense: datetime.fromisoformat(data["fetched_at"]) raises TypeError if fetched_at is a non-string JSON value (e.g. integer or null). TypeError is not in the except tuple at L53, so a malformed cache entry escapes the guard and propagates uncaught. Add TypeError to the except tuple.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — already fixed in 28ec684. Line 53 except tuple now includes TypeError: except (json.JSONDecodeError, KeyError, ValueError, OSError, TypeError).

"""Extract a compact summary from the run_skill double-wrapped JSON response."""
try:
outer = json.loads(tool_response) if isinstance(tool_response, str) else tool_response
if isinstance(outer, dict) and list(outer.keys()) == ["result"]:
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] bugs: list(outer.keys()) == ["result"] only matches when the outer dict has exactly one key. If the PostToolUse event envelope includes additional keys alongside "result", the inner run_skill result is not extracted. Change to "result" in outer.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — already fixed in 28ec684. Line 90 now uses if isinstance(outer, dict) and "result" in outer: instead of strict single-key equality.

sys.exit(0)

tool_name = event.get("tool_name", "")
tool_response = event.get("tool_response", "")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] bugs: event.get("tool_response", "") returns None if the key is explicitly present with a None value. _extract_run_skill_result(None) then produces "None" as the summary string. Use event.get("tool_response") or "" instead.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — already fixed in 28ec684. Line 120 now uses event.get("tool_response") or "" to handle explicit None values.

try:
raw = sys.stdin.read()
event = json.loads(raw)
except (json.JSONDecodeError, ValueError):
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] slop: Two consecutive except clauses both call sys.exit(0)except (json.JSONDecodeError, ValueError) followed by except Exception. The second clause is redundant. Merge into a single except Exception: sys.exit(0).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Investigated — already fixed in 28ec684. The two consecutive except clauses have been merged into a single except Exception: sys.exit(0) at lines 110-114.

Comment thread tests/infra/test_quota_post_check.py Outdated
events = [
json.loads(line) for line in (log_dir / "quota_events.jsonl").read_text().splitlines()
]
assert len(events) == 1
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] tests: assert len(events) == 1 is brittle — if the implementation ever logs an additional event the assertion fails spuriously. Prefer assert any(e["event"] == "post_check_warning" for e in events).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation — flagged for design decision. The len(events) == 1 assertion intentionally verifies that exactly one event is logged per hook invocation (no spurious extra events). The any() alternative would weaken this isolation contract. This is a strictness trade-off that warrants human input.

events = [
json.loads(line) for line in (log_dir / "quota_events.jsonl").read_text().splitlines()
]
assert len(events) == 1
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[warning] tests: Same brittle count assertion as T8 — assert len(events) == 1 over-constrains. Use assert any(e["event"] == "post_check_pass" for e in events) instead.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid observation — flagged for design decision. Same trade-off as T8: the count assertion enforces a single-event-per-invocation contract. Relaxing to any() trades strictness for resilience. Deferring to human judgment on the appropriate level of assertion granularity.

Comment thread tests/infra/test_quota_post_check.py Outdated
Comment thread tests/infra/test_quota_post_check.py Outdated
Trecek and others added 3 commits April 8, 2026 10:06
- Add isinstance(event, dict) guard after json.loads to prevent
  AttributeError on non-dict JSON values
- Add TypeError to _read_quota_cache except tuple for non-string
  fetched_at values
- Use "result" in outer instead of strict single-key check for
  more robust response extraction
- Use event.get("tool_response") or "" to handle explicit None values
- Merge redundant except clauses into single except Exception
- Fix _extract_run_skill_result type annotation to str | dict

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@Trecek Trecek added this pull request to the merge queue Apr 8, 2026
Merged via the queue into integration with commit 4628522 Apr 8, 2026
2 checks passed
@Trecek Trecek deleted the quota-guard-hook-inconsistently-blocks-run-skill-instead-of/663 branch April 8, 2026 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant