From 4743df93b7f5dc12b35b78e7ae07749d3cb57906 Mon Sep 17 00:00:00 2001 From: Federico Bartoli Date: Sat, 18 Apr 2026 08:49:29 +0200 Subject: [PATCH] refactor(hooks): address review feedback on sdd-cache Drop the 24h TTL: HTTP validators are the whole freshness contract. Key cache on URL alone; prompt-aware keying with normalization gave false safety (semantic differences slipped through). Prompt is kept as metadata and surfaced in the hit message so the next agent can judge whether the earlier reading applies. Reframe docs around "HTTP resource cache, not prompt cache". While here, fix two latent bugs: - Replace the unquoted heredoc in the pre-hook with printf. The heredoc expanded $vars and backticks inside cached content, so a compromised doc page could trigger command substitution on cache hit. - Strip CR before awk paragraph-mode parsing of curl -I -L output so blank separators between response blocks on a redirect chain are recognised (was silently picking intermediate headers). Remove dead -v IGNORECASE=1 (gawk-only; tolower() already handles it). --- hooks/SDD-CACHE.md | 20 ++++++---- hooks/sdd-cache-post.sh | 51 +++++++++--------------- hooks/sdd-cache-pre.sh | 87 ++++++++++++++++------------------------- 3 files changed, 65 insertions(+), 93 deletions(-) diff --git a/hooks/SDD-CACHE.md b/hooks/SDD-CACHE.md index 23c21729..8908bcb0 100644 --- a/hooks/SDD-CACHE.md +++ b/hooks/SDD-CACHE.md @@ -50,21 +50,26 @@ This hook caches fetched content on disk, but **revalidates with the origin serv 3. Use `/source-driven-development` (or the skill) as usual. No changes to the skill or the agent's workflow — the cache is transparent. +## Mental model + +HTTP resource cache keyed by URL. Freshness is delegated to the origin via `ETag` / `Last-Modified`; no TTL, no prompt in the key. + +The stored body is not raw HTML — `WebFetch` post-processes each response through a model using the caller's prompt, so what we cache is one agent's reading of the page. The key stays URL-only so reads reuse across sessions; the original prompt is kept as metadata and surfaced in the hit message so the next agent can tell whether the earlier reading fits. + ## How it works -One cache entry per `(url, prompt)` pair, stored as JSON in `.claude/sdd-cache/.json`: +One cache entry per URL, stored as JSON in `.claude/sdd-cache/.json`: | Event | Action | |---|---| -| `PreToolUse WebFetch` | If an entry exists, sends a `HEAD` request with `If-None-Match` / `If-Modified-Since`. On `304`, blocks the fetch and returns the cached content to the agent via stderr. Otherwise allows the fetch. | +| `PreToolUse WebFetch` | If an entry exists, sends a `HEAD` request with `If-None-Match` / `If-Modified-Since`. On `304`, blocks the fetch and returns the cached content to the agent via stderr, with the original prompt surfaced as metadata. Otherwise allows the fetch. | | `PostToolUse WebFetch` | Captures the response, issues a `HEAD` request to record the current `ETag` / `Last-Modified`, and stores `{url, prompt, etag, last_modified, content, fetched_at}`. | **Freshness rules:** - Entry is served only if the origin confirms `304 Not Modified`. - Entries without an `ETag` or `Last-Modified` header are never cached — without a validator, the hook cannot verify freshness later, and caching would mean trusting memory. -- A hard 24-hour TTL bypasses the cache regardless, as a safety net against misbehaving servers. -- Cache key is `sha256(url + normalized_prompt)`: the same URL asked with a different question is a separate entry. Prompts are normalized (lowercased, whitespace collapsed) before hashing, so stylistic variants like `"Extract the signature"` and `"extract the\nsignature"` hit the same entry. Semantically different prompts still miss. +- Cache key is `sha256(url)`. The same URL asked with a different prompt hits the same entry; the cached body reflects the prompt used on the first fetch, and that prompt is shown alongside the hit so the agent can decide whether to re-use or re-fetch manually. **What the agent sees:** @@ -144,13 +149,14 @@ mkdir -p .claude/sdd-cache && touch .claude/sdd-cache/.debug # …disable with: rm .claude/sdd-cache/.debug ``` -The log captures URL, prompt excerpt, detected `tool_response` shape, HEAD status, and why each invocation hit / missed / bypassed the cache. Useful when a cache miss looks unexpected (typically: the origin stopped emitting validators, or the `(url, prompt)` pair differs by a whitespace). +The log captures URL, detected `tool_response` shape, HEAD status, and why each invocation hit or missed. Useful when a cache miss looks unexpected (typically: the origin stopped emitting validators). ## Known limitations -- **WebFetch output is prompt-dependent.** The same URL queried with a different prompt produces different output; the cache keys on `(url, prompt)` to handle this. Cache hit rate depends on the agent reusing the same prompt wording across sessions. +- **Body is prompt-shaped.** A hit returns the earlier agent's reading of the page, with the original prompt surfaced so the current agent can decide whether it applies. If it doesn't, delete the file under `.claude/sdd-cache/` to force a re-fetch. +- **Every cache write costs an extra HEAD.** Claude Code doesn't expose the response headers that `WebFetch` already received, so the post hook re-queries the origin to capture `ETag` / `Last-Modified`. One extra roundtrip per miss — the price of keeping this a pure hook with no core changes. - **Servers without `ETag` or `Last-Modified` are never cached.** Most official doc sites (react.dev, docs.djangoproject.com, developer.mozilla.org) emit validators. Sites that don't are always re-fetched. -- **A misbehaving server can serve a wrong `304`.** The 24-hour TTL limits the damage. If a doc changes and the server incorrectly reports it unchanged, the stale entry expires within a day. +- **A misbehaving server can serve a wrong `304`.** That's a server bug to diagnose, not a cache invariant to defend against; we don't paper over it with a TTL. Delete the entry if you spot a stale one. - **Cache is local and per-project.** There is no team-wide shared cache. Adding one would require a signed-content-addressable storage layer, which is out of scope. ## Requirements diff --git a/hooks/sdd-cache-post.sh b/hooks/sdd-cache-post.sh index 7d39ebcf..7a9c5b54 100755 --- a/hooks/sdd-cache-post.sh +++ b/hooks/sdd-cache-post.sh @@ -1,17 +1,15 @@ #!/bin/bash -# sdd-cache-post.sh — PostToolUse hook for WebFetch +# sdd-cache-post.sh — PostToolUse hook for WebFetch. # -# After a successful WebFetch, stores the fetched content in -# .claude/sdd-cache/.json together with the server's ETag and/or -# Last-Modified headers captured via a HEAD request. The Pre hook uses -# those validators to revalidate on the next fetch. +# After WebFetch, stores the response body in .claude/sdd-cache/.json +# with the current ETag / Last-Modified captured via a HEAD request so the +# pre hook can revalidate on the next fetch. # -# The cache only stores entries for URLs whose servers emit ETag or -# Last-Modified. Without a validator, the Pre hook cannot verify freshness, -# so caching the entry would amount to trusting memory — exactly what the -# skill forbids. +# Keyed by URL. The caller's prompt is stored as metadata (not part of the +# key) so a future cache hit can show what question produced the cached +# reading. Entries without ETag or Last-Modified are not cached. # -# Dependencies: jq, curl, shasum (or sha256sum) +# Dependencies: jq, curl, shasum (or sha256sum). set -euo pipefail @@ -65,36 +63,26 @@ if [ -z "$CONTENT" ]; then fi dbg "extracted content bytes=${#CONTENT}" -# Cache key is (url + normalized prompt) — must match the Pre hook. See -# sdd-cache-pre.sh for the rationale behind normalization. -normalize_prompt() { - printf '%s' "$1" \ - | tr '[:upper:]' '[:lower:]' \ - | tr -s '[:space:]' ' ' \ - | sed -e 's/^ //' -e 's/ $//' -} - +# Must match the pre hook: sha256(URL), first 32 hex chars. hash_key() { - local norm - norm=$(normalize_prompt "$2") - local key="$1"$'\x1f'"$norm" if command -v shasum >/dev/null 2>&1; then - printf '%s' "$key" | shasum -a 256 | cut -c1-32 + printf '%s' "$1" | shasum -a 256 | cut -c1-32 else - printf '%s' "$key" | sha256sum | cut -c1-32 + printf '%s' "$1" | sha256sum | cut -c1-32 fi } CACHE_DIR="${CLAUDE_PROJECT_DIR:-$PWD}/.claude/sdd-cache" mkdir -p "$CACHE_DIR" -CACHE_FILE="$CACHE_DIR/$(hash_key "$URL" "$PROMPT").json" +CACHE_FILE="$CACHE_DIR/$(hash_key "$URL").json" -# Capture current validators from the origin. Follow redirects so the -# validators match the URL the agent is actually talking to. -HEAD_OUT=$(curl -sI -L --max-time 5 "$URL" 2>/dev/null || true) +# Capture validators from the origin. Follow redirects so they match the +# URL the agent actually talked to. Strip CR so awk's paragraph mode +# recognises blank separators between response blocks on a redirect chain. +HEAD_OUT=$(curl -sI -L --max-time 5 "$URL" 2>/dev/null | tr -d '\r' || true) -# Parse only the headers of the final response (after last blank line), -# to avoid picking up validators from intermediate 301/302 hops. +# Take only the final response's headers (last paragraph) to avoid picking +# up validators from intermediate 301/302 hops. FINAL_HEADERS=$(printf '%s' "$HEAD_OUT" | awk ' BEGIN { RS = ""; last = "" } { last = $0 } @@ -103,11 +91,10 @@ FINAL_HEADERS=$(printf '%s' "$HEAD_OUT" | awk ' extract_header() { local name="$1" - printf '%s' "$FINAL_HEADERS" | awk -v IGNORECASE=1 -v h="$name" ' + printf '%s' "$FINAL_HEADERS" | awk -v h="$name" ' BEGIN { FS = ":" } tolower($1) == tolower(h) { sub(/^[^:]*:[ \t]*/, "") - gsub(/\r/, "") sub(/[ \t]+$/, "") print exit diff --git a/hooks/sdd-cache-pre.sh b/hooks/sdd-cache-pre.sh index 4c823752..1c16aa07 100755 --- a/hooks/sdd-cache-pre.sh +++ b/hooks/sdd-cache-pre.sh @@ -1,21 +1,19 @@ #!/bin/bash -# sdd-cache-pre.sh — PreToolUse hook for WebFetch +# sdd-cache-pre.sh — PreToolUse hook for WebFetch. # -# Before every WebFetch, checks whether the target URL has a cached entry -# in .claude/sdd-cache/. If so, issues a conditional HEAD request with -# If-None-Match / If-Modified-Since. If the server responds 304 Not Modified, -# the hook blocks the WebFetch (exit 2) and returns the cached content to the -# agent via stderr. Otherwise it exits 0 and lets the fetch proceed. +# HTTP resource cache keyed by URL. Freshness is delegated to the origin via +# HTTP validators; 304 Not Modified is the only signal to serve from cache. +# On hit, exits 2 and writes the cached body to stderr so Claude Code can +# deliver it to the agent in place of the WebFetch result. Otherwise exits 0. # -# Freshness is verified by the origin server on every call, so the skill's -# "don't trust memory, verify against current docs" invariant still holds. -# The hook only serves content the server has just confirmed is unchanged. +# No TTL: if validators don't catch a change, nothing will. Entries without +# ETag or Last-Modified are never cached (can't revalidate). # -# A 24h hard TTL acts as a safety net in case a server misreports freshness. -# Entries without ETag/Last-Modified are never cached, so nothing is served -# without a validator. +# Cached bodies are prompt-shaped (WebFetch post-processes through a model), +# so the key is URL-only and the original prompt is surfaced in the hit +# message so the next agent can tell if the earlier reading still applies. # -# Dependencies: jq, curl, shasum (or sha256sum) +# Dependencies: jq, curl, shasum (or sha256sum). set -euo pipefail @@ -36,49 +34,27 @@ dbg() { } dbg "fired" -URL=$(printf '%s' "$INPUT" | jq -r '.tool_input.url // empty' 2>/dev/null || true) -PROMPT=$(printf '%s' "$INPUT" | jq -r '.tool_input.prompt // empty' 2>/dev/null || true) +URL=$(printf '%s' "$INPUT" | jq -r '.tool_input.url // empty' 2>/dev/null || true) if [ -z "$URL" ]; then dbg "no url in tool_input, exit"; exit 0; fi -dbg "url=$URL prompt=$(printf '%s' "$PROMPT" | head -c 80)" - -# Cache key is (url + normalized prompt): WebFetch output is prompt-dependent, -# so the same URL with a different question must miss the cache. Prompt is -# normalized (lowercase + whitespace collapse) so stylistic variants like -# "Extract the signature" vs "extract the\nsignature" hash to the same key. -# Semantically different prompts still differ. -normalize_prompt() { - printf '%s' "$1" \ - | tr '[:upper:]' '[:lower:]' \ - | tr -s '[:space:]' ' ' \ - | sed -e 's/^ //' -e 's/ $//' -} +dbg "url=$URL" +# Cache key is sha256(URL), truncated to 128 bits. hash_key() { - local norm - norm=$(normalize_prompt "$2") - local key="$1"$'\x1f'"$norm" if command -v shasum >/dev/null 2>&1; then - printf '%s' "$key" | shasum -a 256 | cut -c1-32 + printf '%s' "$1" | shasum -a 256 | cut -c1-32 else - printf '%s' "$key" | sha256sum | cut -c1-32 + printf '%s' "$1" | sha256sum | cut -c1-32 fi } CACHE_DIR="${CLAUDE_PROJECT_DIR:-$PWD}/.claude/sdd-cache" -CACHE_FILE="$CACHE_DIR/$(hash_key "$URL" "$PROMPT").json" +CACHE_FILE="$CACHE_DIR/$(hash_key "$URL").json" if [ ! -f "$CACHE_FILE" ]; then dbg "no cache file at $CACHE_FILE, exit"; exit 0; fi dbg "cache file exists: $CACHE_FILE" -# Hard TTL: 24h. If the entry is older, bypass the cache entirely. FETCHED_AT=$(jq -r '.fetched_at // 0' "$CACHE_FILE" 2>/dev/null || echo 0) -NOW=$(date +%s) -AGE=$((NOW - FETCHED_AT)) -if [ "$AGE" -gt 86400 ]; then - dbg "entry older than 24h (age=${AGE}s), bypass" - exit 0 -fi - +ORIGINAL_PROMPT=$(jq -r '.prompt // empty' "$CACHE_FILE" 2>/dev/null || true) ETAG=$(jq -r '.etag // empty' "$CACHE_FILE" 2>/dev/null || true) LAST_MOD=$(jq -r '.last_modified // empty' "$CACHE_FILE" 2>/dev/null || true) @@ -112,16 +88,19 @@ VERIFIED_AT_ISO=$(date -u -r "$FETCHED_AT" +"%Y-%m-%dT%H:%M:%SZ" 2>/dev/null \ || date -u -d "@$FETCHED_AT" +"%Y-%m-%dT%H:%M:%SZ" 2>/dev/null \ || echo "unknown") -cat >&2 <&2 exit 2