From a49e7ed2af94e7bfe5a67134b6212dfa0d2ec01a Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 20 Sep 2026 12:20:45 -0400 Subject: [PATCH 1/4] fix(plugin): the hooks need no jq and no tac (#4107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every hook opened `command -v jq >/dev/null 2>&1 || exit 0`, so on a machine without jq the operator got no session context, no rules, no prior art and no process sync — and not one word saying why, because `exit 0` is indistinguishable from "ran fine, nothing to say". jq is absent by default on macOS, on the Debian/Ubuntu slim images, on Alpine and in most CI containers. That is not a prerequisite to document; it is the plugin handing its own packaging problem to whoever installs it. `tac` was worse: GNU-only, so the prior-art hook's enclosing-definition arm did nothing at all on every Mac, silently, from the day it shipped. It is not replaced but removed — scribe_defs judges each line independently, so extracting forward and taking `tail -1` is the same answer as reversing and taking the head, and it drops the early-exit `head` that #4042 was filed for. No server contract changed, so a lagging plugin cache keeps working. scribe_json.awk JSON -> IDXPATHVALUE. Two modes: `whole` for an event or a response body, `lines` for a transcript, where an unparseable record is dropped and the rest still read — the `map(try fromjson catch empty)` the jq program opened with. Arrays also report their LENGTH at `[#]`, which is what keeps "zero notes" distinct from "no answer" (#2932). scribe_turn.awk the turn-bounding program, replacing the thirty lines of jq in the Stop hook. scribe_defs.sh scribe_json_flat / _pick / _list / _len / _list_minus read, scribe_json_out writes the envelope (five copies of one shape, gone), scribe_urlenc replaces `jq -sRr '@uri'`. Percent-encoding goes through `od -tu1` rather than an awk character loop on purpose: awk's idea of a character follows the locale, so gawk reads an accented letter as one and mawk as two, and an encoder built on substr() would emit a different URL depending on which awk is installed. Encoding is defined on bytes. Verified byte-identical to `jq -sRr '@uri'`. Measured, not assumed. The per-event path costs 8ms against jq's 3ms. The transcript path was 70x slower until two fixes: the Stop hook now finds where the turn starts with a fixed-string grep before parsing (a needle carrying unescaped quotes cannot occur inside a JSON string, so it matches only at a record's top level — checked against a full JSON parse of a 27MB transcript: 152 prompt records, 152 matches, no misses, no extras), and the parser reads each token out of a 1024-byte window instead of copying the rest of the buffer per token, which was quadratic in line length on the 400KB tool results a transcript carries. Differential-tested against the jq program it replaces over 724 windows cut from three real transcripts — 724 identical, 0 mismatched, 45 of them exercising a real task close and a real reply. That sweep is what caught `scribe_turn.awk` never setting FS, which truncated every multi-word reply at its first space and was invisible to a test whose replies were all empty. check_plugin.py's `jq -R` lint becomes a guard against either binary coming back, and three smoke checks lose their `shutil.which("jq")` skip. jq is not in `ci-python` either, so those three announced a skip on every CI run and had never once run there: removing the dependency from the product also closed a permanent hole in its verification. They pass now across all ten hooks. tests/test_hook_json_reader.py is a differential against Python's `json` over nested objects, arrays, unicode, escapes, control characters, empty cases and a value longer than the token window, plus the envelope, the encoder and the turn analyzer. 139 cases. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- plugin/.claude-plugin/plugin.json | 2 +- plugin/hooks/scribe_after_write.sh | 31 +- plugin/hooks/scribe_autoinject.sh | 32 ++- plugin/hooks/scribe_defs.sh | 234 ++++++++++++++- plugin/hooks/scribe_json.awk | 217 ++++++++++++++ plugin/hooks/scribe_prior_art.sh | 89 +++--- plugin/hooks/scribe_record_opened.sh | 17 +- plugin/hooks/scribe_report_check.sh | 109 ++++--- plugin/hooks/scribe_session_context.sh | 20 +- plugin/hooks/scribe_sync_processes.sh | 15 +- plugin/hooks/scribe_tool_rules.sh | 47 ++- plugin/hooks/scribe_turn.awk | 108 +++++++ scripts/check_plugin.py | 71 +++-- tests/test_after_write_hook.py | 2 +- tests/test_hook_json_reader.py | 380 +++++++++++++++++++++++++ tests/test_plugin_version_mint.py | 9 +- tests/test_report_check_hook.py | 2 +- tests/test_rule_opened_ledger.py | 2 +- tests/test_scribe_marker.py | 2 +- tests/test_session_context_ledger.py | 2 +- tests/test_session_ledger_clear.py | 2 +- tests/test_write_path_trigger.py | 2 +- 22 files changed, 1185 insertions(+), 210 deletions(-) create mode 100644 plugin/hooks/scribe_json.awk create mode 100644 plugin/hooks/scribe_turn.awk create mode 100644 tests/test_hook_json_reader.py diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 62a9f31..15ac240 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.20.0317", + "version": "2026.09.20.1620", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_after_write.sh b/plugin/hooks/scribe_after_write.sh index 1510983..689bead 100644 --- a/plugin/hooks/scribe_after_write.sh +++ b/plugin/hooks/scribe_after_write.sh @@ -31,7 +31,6 @@ # SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path. set -uo pipefail -command -v jq >/dev/null 2>&1 || exit 0 command -v git >/dev/null 2>&1 || exit 0 # shellcheck source=plugin/hooks/scribe_defs.sh @@ -39,10 +38,11 @@ command -v git >/dev/null 2>&1 || exit 0 # PostToolUse delivers { session_id, cwd, tool_name, tool_input, tool_response }. event=$(cat 2>/dev/null || true) -tool_name=$(printf '%s' "$event" | jq -r '.tool_name // empty' 2>/dev/null) || exit 0 +event_flat=$(printf '%s' "$event" | scribe_json_flat) +tool_name=$(scribe_json_pick "$event_flat" '.tool_name') [ "$tool_name" = "Bash" ] || exit 0 -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" -event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_cwd="" +session_id=$(scribe_json_pick "$event_flat" '.session_id') +event_cwd=$(scribe_json_pick "$event_flat" '.cwd') work_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} repo_root=$(git -C "$work_dir" rev-parse --show-toplevel 2>/dev/null) || exit 0 [ -n "$repo_root" ] || exit 0 @@ -153,12 +153,12 @@ while IFS= read -r rel_path; do unreached_context="" if [ -n "$url" ] && [ -n "$token" ]; then q=$(printf '%s' "$code" | head -c 1200) - path_enc=$(printf '%s' "$rel_path" | jq -sRr '@uri' 2>/dev/null) || path_enc="" - code_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || code_enc="" + path_enc=$(printf '%s' "$rel_path" | scribe_urlenc) || path_enc="" + code_enc=$(printf '%s' "$q" | scribe_urlenc) || code_enc="" shapes_q="" enc=$(printf '%s\n' "$names" \ | awk -F'\t' 'NF>=2 {printf "%s%s:%s", (n++?",":""), $1, $2}' \ - | jq -sRr '@uri' 2>/dev/null) || enc="" + | scribe_urlenc) || enc="" [ -n "$enc" ] && shapes_q="&shapes=${enc}" exclude_q=""; sync_exclude_q=""; derive_exclude_q="" if [ -f "$idfile" ]; then @@ -170,7 +170,7 @@ while IFS= read -r rel_path; do [ -n "$sync_seen" ] && sync_exclude_q="&exclude_sync_ids=${sync_seen}" fi if [ -f "$derivefile" ]; then - derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen="" + derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | scribe_urlenc) || derive_seen="" [ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}" fi # The rules marker is gone with the resident set it aged (milestone 394). @@ -197,12 +197,14 @@ while IFS= read -r rel_path; do unreached_context=$(scribe_unreached "$state_dir" "$safe_sid" 8 "$rel_path") fi fi + body_flat="" if [ -n "$body" ]; then - context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || context="" + body_flat=$(printf '%s' "$body" | scribe_json_flat) + context=$(scribe_json_pick "$body_flat" '.context') if [ -n "$context" ]; then - printf '%s' "$body" | jq -r '((.note_ids // []) - (.sync_note_ids // []))[]?' 2>/dev/null >> "$idfile" || true - printf '%s' "$body" | jq -r '(.sync_note_ids // [])[]?' 2>/dev/null >> "$syncfile" || true - printf '%s' "$body" | jq -r '(.derive_keys // [])[]?' 2>/dev/null >> "$derivefile" || true + scribe_json_list_minus "$body_flat" '.note_ids' '.sync_note_ids' >> "$idfile" || true + scribe_json_list "$body_flat" '.sync_note_ids' >> "$syncfile" || true + scribe_json_list "$body_flat" '.derive_keys' >> "$derivefile" || true # Several files in one call may name the same family: keep each # token once, so the next request's exclude list stays exact. for f in "$idfile" "$syncfile" "$derivefile"; do @@ -217,7 +219,7 @@ while IFS= read -r rel_path; do # call that did not answer; "nothing recorded" is a claim only an answer # can back. if [ -n "$local_lines" ] && [ "$reached" != 0 ]; then - n_recorded=$(printf '%s' "$body" | jq -r '.note_ids | length' 2>/dev/null) || n_recorded=0 + n_recorded=$(scribe_json_len "$body_flat" '.note_ids') if [ "${n_recorded:-0}" = "0" ] || [ "$n_recorded" = "" ]; then local_context="${local_context}"$'\n'"> None of those existing copies is recorded in Scribe. If the version just written is the canonical one — or this edit is consolidating the copies — record it now with create_snippet so the next session is offered it instead of writing another copy." fi @@ -238,6 +240,5 @@ while IFS= read -r rel_path; do done <<< "$changed" [ -n "$combined" ] || exit 0 -jq -n --arg c "$combined" \ - '{hookSpecificOutput: {hookEventName: "PostToolUse", additionalContext: $c}}' +scribe_json_out PostToolUse "$combined" exit 0 diff --git a/plugin/hooks/scribe_autoinject.sh b/plugin/hooks/scribe_autoinject.sh index b61aac6..e960efe 100755 --- a/plugin/hooks/scribe_autoinject.sh +++ b/plugin/hooks/scribe_autoinject.sh @@ -38,14 +38,17 @@ set -uo pipefail # shellcheck source=plugin/hooks/scribe_defs.sh . "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" -command -v jq >/dev/null 2>&1 || exit 0 command -v curl >/dev/null 2>&1 || exit 0 # UserPromptSubmit delivers a JSON event on stdin: { prompt, session_id, cwd, ... } +# Parsed ONCE into flat lines and then queried three times (#4107): a prompt is +# the largest payload any hook reads, and re-parsing it per field is three +# passes over the same text. event=$(cat 2>/dev/null || true) -prompt=$(printf '%s' "$event" | jq -r '.prompt // empty' 2>/dev/null) || prompt="" -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" -event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_cwd="" +event_flat=$(printf '%s' "$event" | scribe_json_flat) +prompt=$(scribe_json_pick "$event_flat" '.prompt') +session_id=$(scribe_json_pick "$event_flat" '.session_id') +event_cwd=$(scribe_json_pick "$event_flat" '.cwd') # Nothing to retrieve against. [ -n "$prompt" ] || exit 0 @@ -59,11 +62,14 @@ scribe_config || exit 0 # prior-art hook's code cap; this copy was missed when that one was fixed, and # scripts/check_plugin.py caught it. q=$(printf '%s' "$prompt" | head -c 2000) -# `-sRr`, not `-rR`: jq -R reads LINE BY LINE, so a multi-line prompt encoded as -# several lines joined by raw newlines and the request died. Single-line prompts +# Encoded whole, never line by line. The predecessor here was `jq -rR`, which +# reads a line at a time: a multi-line prompt came back as several separately +# encoded lines joined by raw newlines and the request died. Single-line prompts # worked, which is why this looked healthy — the long, substantial prompts most -# worth retrieving against were exactly the ones silently dropped. -s slurps. -q_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || exit 0 +# worth retrieving against were exactly the ones silently dropped. scribe_urlenc +# reads bytes and has no notion of a line. +q_enc=$(printf '%s' "$q" | scribe_urlenc) || exit 0 +[ -n "$q_enc" ] || exit 0 # Scope to this directory's project — a `.scribe` marker, else the git remote. repo_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} @@ -109,22 +115,22 @@ body=$(curl -fsS --max-time 5 \ "${url%/}/api/plugin/retrieve?q=${q_enc}${repo_q}${exclude_q}" 2>/dev/null) || exit 0 [ -n "$body" ] || exit 0 -context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || exit 0 +body_flat=$(printf '%s' "$body" | scribe_json_flat) +context=$(scribe_json_pick "$body_flat" '.context') [ -n "$context" ] || exit 0 # Remember the surfaced ids so they aren't injected again this session. if [ -n "$idfile" ]; then - printf '%s' "$body" | jq -r '.note_ids[]? // empty' 2>/dev/null >> "$idfile" || true + scribe_json_list "$body_flat" '.note_ids' >> "$idfile" || true fi # Rules onto the SHARED ledger, stamped so they can age out. Only FRESH ids # come back in rule_ids (#3752) — a rule rendered as a repeat is already on # the ledger, and re-appending it would keep pushing its stamp forward so it # never aged at all. if [ -n "$rulefile" ]; then - printf '%s' "$body" | jq -r '.rule_ids[]? // empty' 2>/dev/null \ + scribe_json_list "$body_flat" '.rule_ids' \ | scribe_rules_append "$rulefile" fi -jq -n --arg c "$context" \ - '{hookSpecificOutput: {hookEventName: "UserPromptSubmit", additionalContext: $c}}' +scribe_json_out UserPromptSubmit "$context" exit 0 diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 76e6370..33b950f 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -19,6 +19,10 @@ # scribe_rules_live FILE live rule ids from the exclusion ledger, # comma-joined; entries age out (#3751) # scribe_rules_append FILE stdin ids -> the ledger, timestamped +# scribe_json_flat stdin JSON -> IDXPATHVALUE lines, +# queried with scribe_json_pick / _list / _len. +# scribe_json_out writes the hook envelope, +# scribe_urlenc percent-encodes. No jq (#4107). # scribe_scope_query DIR `project_id=N` or `repo=` for DIR — the # project-scope key EVERY hook sends (#4085). # Helpers: scribe_marker_file, scribe_url_host, @@ -39,6 +43,216 @@ scribe_skip_path() { return 1 } +# --------------------------------------------------------------------------- +# JSON AND URL-ENCODING, WITHOUT jq (#4107). +# +# Until this section every hook opened `command -v jq >/dev/null 2>&1 || exit 0` +# and a machine without jq got no session context, no rules, no prior art and +# no process sync — silently, because `exit 0` is indistinguishable from "ran +# fine, nothing to say". That is the plugin pushing its own packaging problem +# onto whoever installs it, and it is the same silence #4085 existed to remove. +# jq is absent by default on macOS, on the Debian/Ubuntu slim images, on Alpine +# and in most CI containers. awk, sed, tr and od are POSIX; every one of them +# is already required by code above this line. +# +# scribe_json.awk does the parsing and carries the format. These are the four +# jobs the hooks actually had jq for: +# +# READ scribe_json_flat / _flat_lines turn stdin into `IDXPATH +# VALUE` lines ONCE, and scribe_json_pick / _list / _len query that +# text. Parse once, query many: a hook reads four fields off one +# event, and re-parsing per field is four passes over a payload that +# may be an entire source file. +# WRITE scribe_json_out emits the hookSpecificOutput envelope, which is the +# one piece of JSON these hooks produce. +# ENCODE scribe_urlenc replaces `jq -sRr '@uri'`, byte-exact for UTF-8. +# DECODE scribe_json_unescape turns a raw JSON string body into text. + +_scribe_self=${BASH_SOURCE[0]:-$0} +# shellcheck disable=SC1007 # `CDPATH= cd` scopes one variable to one command +SCRIBE_HOOK_DIR=$(CDPATH= cd -- "$(dirname -- "$_scribe_self")" 2>/dev/null && pwd) \ + || SCRIBE_HOOK_DIR=$(dirname -- "$_scribe_self") + +# One JSON value on stdin → flat lines. Empty output means it did not parse. +scribe_json_flat() { + awk -f "$SCRIBE_HOOK_DIR/scribe_json.awk" 2>/dev/null || true +} + +# JSONL on stdin → flat lines, IDX counting the records that PARSED. A line +# that does not parse is dropped and the rest are still read. +scribe_json_flat_lines() { + awk -v mode=lines -f "$SCRIBE_HOOK_DIR/scribe_json.awk" 2>/dev/null || true +} + +# $1 flat text, $2 exact path → the decoded scalar, or "" if absent. +# +# `null` READS AS ABSENT, matching the `// empty` every call site used to carry. +# A field the server left null and one it never sent mean the same thing to +# every caller here, and the shells downstream test `[ -n "$x" ]`. +scribe_json_pick() { + printf '%s\n' "${1:-}" \ + | awk -F'\t' -v p="$2" '$2 == p { if ($3 != "null") print $3; exit }' 2>/dev/null \ + | scribe_json_unescape +} + +# $1 flat text, $2 array path → each element decoded, one per line. Elements +# are ids and keys here, so a value containing a newline is not a case that +# arises; one would split into two lines. +scribe_json_list() { + printf '%s\n' "${1:-}" \ + | awk -F'\t' -v p="$2" ' + BEGIN { plen = length(p) } + substr($2, 1, plen) == p { + rest = substr($2, plen + 1) + if (rest ~ /^\[[0-9]+\]$/ && $3 != "null") print $3 + }' 2>/dev/null \ + | scribe_json_unescape +} + +# $1 flat text, $2 array path, $3 array path to SUBTRACT → the elements of $2 +# that are not in $3, in order. The reuse and sync channels are two classes of +# one answer and each has its own session ledger, so the ids that go in the +# reuse file are precisely `note_ids` minus `sync_note_ids` — jq wrote that as +# `(.note_ids // []) - (.sync_note_ids // [])`. +scribe_json_list_minus() { + local keep + keep=$(scribe_json_list "$1" "$3") + scribe_json_list "$1" "$2" \ + | awk -v drop="$keep" ' + BEGIN { n = split(drop, a, "\n"); for (i = 1; i <= n; i++) if (a[i] != "") s[a[i]] = 1 } + NF && !($0 in s)' 2>/dev/null || true +} + +# $1 flat text, $2 array path → its length. "" when the path is not an array, +# which is NOT the same as 0 — see the `[#]` note in scribe_json.awk. +scribe_json_len() { + printf '%s\n' "${1:-}" \ + | awk -F'\t' -v p="$2" '$2 == p "[#]" { print $3; exit }' 2>/dev/null +} + +# Raw JSON string bodies on stdin → text. One line in, one value out. +scribe_json_unescape() { + awk ' + function hex4(h, i, c, d, v) { + v = 0 + for (i = 1; i <= 4; i++) { + c = tolower(substr(h, i, 1)) + d = index("0123456789abcdef", c) - 1 + if (d < 0) return -1 + v = v * 16 + d + } + return v + } + { + v = $0 + if (index(v, "\\") == 0) { print v; next } + o = "" + i = 1 + L = length(v) + while (i <= L) { + c = substr(v, i, 1) + if (c != "\\") { o = o c; i++; continue } + d = substr(v, i + 1, 1) + i += 2 + if (d == "n") o = o "\n" + else if (d == "t") o = o "\t" + else if (d == "r") o = o "\r" + else if (d == "b") o = o sprintf("%c", 8) + else if (d == "f") o = o sprintf("%c", 12) + else if (d == "u") { + hi = hex4(substr(v, i, 4)) + if (hi < 0) { o = o "\\u"; continue } + i += 4 + # A surrogate PAIR is one character written as two escapes; decoding + # the halves separately yields two replacement characters instead. + if (hi >= 55296 && hi <= 56319 && substr(v, i, 2) == "\\u") { + lo = hex4(substr(v, i + 2, 4)) + if (lo >= 56320 && lo <= 57343) { + hi = 65536 + (hi - 55296) * 1024 + (lo - 56320) + i += 6 + } + } + o = o sprintf("%c", hi) + } + else o = o d + } + print o + }' 2>/dev/null || true +} + +# Text on stdin → a JSON string BODY (escaped, no surrounding quotes, one line). +# +# split/join rather than gsub: gsub reads `\` and `&` in its replacement as +# metacharacters, so emitting a literal backslash through it takes a +# quadruple-escape whose meaning varies between awks. Plain concatenation has +# no such reading, and getting this wrong corrupts every injected context that +# happens to contain a Windows path or a regex. +scribe_json_escape() { + awk ' + function rep(str, sep, with, parts, cnt, i, o) { + cnt = split(str, parts, sep) + o = parts[1] + for (i = 2; i <= cnt; i++) o = o with parts[i] + return o + } + { + line = $0 + line = rep(line, "\\\\", "\\\\") + line = rep(line, "\"", "\\\"") + line = rep(line, "\t", "\\t") + line = rep(line, "\r", "\\r") + # EVERY OTHER CONTROL CHARACTER TOO, or the envelope is not JSON. A raw + # byte below 0x20 inside a string is invalid, and Claude Code discards + # the whole hook output rather than the one field — so a single stray + # character anywhere in an injected context costs the entire injection. + # jq escaped these; a writer that knew only about tab and carriage + # return would have regressed quietly, on the rare input nobody thinks + # to test by hand. + for (c = 1; c < 32; c++) { + if (c == 9 || c == 10 || c == 13) continue + ctl = sprintf("%c", c) + if (index(line, ctl)) line = rep(line, ctl, sprintf("\\u%04x", c)) + } + if (NR > 1) printf "\\n" + printf "%s", line + }' 2>/dev/null || true +} + +# The one JSON document these hooks WRITE: $1 hook event name, $2 the context. +# Every hook emitted this through `jq -n --arg c`, five copies of one shape. +scribe_json_out() { + local esc + esc=$(printf '%s' "$2" | scribe_json_escape) || return 0 + printf '{"hookSpecificOutput":{"hookEventName":"%s","additionalContext":"%s"}}\n' \ + "$1" "$esc" +} + +# stdin → percent-encoded, replacing `jq -sRr '@uri'`. +# +# THROUGH `od`, NOT A CHARACTER LOOP IN awk, and the reason is the bug this +# would otherwise reintroduce. awk's idea of a "character" follows the locale: +# in a UTF-8 locale gawk reads é as one character and mawk as two bytes, so any +# encoder built on substr() produces a different URL on the same input +# depending on which awk is installed. Percent-encoding is defined on BYTES. od +# -tu1 gives bytes, on every platform, and the only characters passed through +# unencoded are the unreserved ASCII set — which every awk agrees about. +# +# Slightly more aggressive than jq's `@uri`, which leaves `!~*'()` alone. Those +# are legal either way and the server decodes both identically. +scribe_urlenc() { + od -An -v -tu1 2>/dev/null \ + | awk '{ + for (i = 1; i <= NF; i++) { + b = $i + 0 + if ((b >= 48 && b <= 57) || (b >= 65 && b <= 90) || + (b >= 97 && b <= 122) || b == 45 || b == 46 || b == 95 || b == 126) + printf "%s", sprintf("%c", b) + else + printf "%%%02X", b + } + }' 2>/dev/null || true +} + # --------------------------------------------------------------------------- # kindname for each thing a piece of code DEFINES, in source order. One # program, two consumers: the local duplicate arm (every definition in the @@ -281,7 +495,7 @@ scribe_rules_live() { } # Append surfaced ids, stamped. Reads ids on stdin, one per line — the shape -# `jq -r '(.rule_ids // [])[]?'` already produces at both call sites. +# `scribe_json_list "$flat" '.rule_ids'` already produces at both call sites. scribe_rules_append() { local f="$1" now [ -n "$f" ] || return 0 @@ -425,19 +639,20 @@ scribe_url_host() { # `set -u` these hooks all run with aborts the hook and costs the whole # session's context — a failure far larger than the message it was fetching. scribe_marker_read() { - local f="$1" id inst want + local f="$1" flat id inst want [ -n "$f" ] && [ -f "$f" ] || { printf '\t'; return 0; } - command -v jq >/dev/null 2>&1 || { printf '\t'; return 0; } - # A bare integer is valid JSON, so one filter reads both forms. - id=$(jq -r 'if type=="number" then (.|floor|tostring) - elif type=="object" then (.project_id // empty | tostring) - else empty end' "$f" 2>/dev/null) || id="" + flat=$(scribe_json_flat < "$f") || flat="" + # Both accepted forms, read off one parse: the object's `project_id`, or — + # for the bare integer someone writes by hand — the root scalar, whose path + # is the empty string because it sits under no key. + id=$(scribe_json_pick "$flat" '.project_id') + [ -n "$id" ] || id=$(scribe_json_pick "$flat" '') case "$id" in ''|*[!0-9]*) id="" ;; esac if [ -z "$id" ] || [ "$id" = "0" ]; then printf '\tnames no project_id' return 0 fi - inst=$(jq -r 'if type=="object" then (.instance // empty) else empty end' "$f" 2>/dev/null) || inst="" + inst=$(scribe_json_pick "$flat" '.instance') if [ -n "$inst" ]; then want=$(scribe_url_host "${url:-}") inst=$(scribe_url_host "$inst") @@ -468,7 +683,6 @@ scribe_scope_query() { fi repo=$(git -C "$dir" remote get-url origin 2>/dev/null || true) [ -n "$repo" ] || return 0 - command -v jq >/dev/null 2>&1 || return 0 - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" + enc=$(printf '%s' "$repo" | scribe_urlenc) || enc="" [ -n "$enc" ] && printf 'repo=%s' "$enc" } diff --git a/plugin/hooks/scribe_json.awk b/plugin/hooks/scribe_json.awk new file mode 100644 index 0000000..2b61ab8 --- /dev/null +++ b/plugin/hooks/scribe_json.awk @@ -0,0 +1,217 @@ +# Scribe plugin — JSON, read without jq (#4107). +# +# WHY THIS FILE EXISTS. Every hook here parsed JSON with `jq`, and every hook +# opened with `command -v jq >/dev/null 2>&1 || exit 0`. jq is not installed by +# default on macOS, on Debian/Ubuntu base images, on Alpine, or in most CI +# containers — so on a machine without it the operator got no session context, +# no rules, no prior art and no process sync, and not one word saying why. That +# is not a missing prerequisite to document; it is the plugin pushing its own +# packaging problem onto whoever installs it. awk is POSIX: it is present as +# gawk, mawk, nawk or busybox awk everywhere a shell is. +# +# WHAT IT DOES. Reads JSON on stdin, writes one line per SCALAR: +# +# IDX PATH VALUE +# +# 1 .prompt write the tests first +# 1 .tool_input.file_path /src/app.py +# 1 .note_ids[0] 4107 +# 1 .note_ids[#] 3 +# +# IDX 1-based index of the top-level JSON value (always 1 unless mode=lines). +# PATH dot/bracket path. Array indices are 0-based. +# VALUE for a string, the RAW JSON-escaped body with the quotes stripped — not +# the decoded text. Decoding here would let a newline or a tab inside a +# value break the line format that makes this readable from shell. The +# caller decodes what it actually uses, with `scribe_json_unescape`. +# Numbers, `true`, `false` and `null` are emitted literally. +# +# EVERY ARRAY ALSO EMITS `PATH[#]` with its LENGTH, including an empty one — +# which is what makes "the server answered with zero notes" distinguishable +# from "the server did not answer", a distinction #2932 built a whole outage +# marker around. An empty object emits `PATH{#}` 0 for the same reason. +# +# TWO MODES, because the two jobs have opposite failure behaviour: +# +# mode=whole (default) the entire input is ONE JSON value, possibly spread +# over many lines. A parse error produces no output at all. +# This is a hook event on stdin, or a server response body. +# +# mode=lines the input is JSONL — one JSON value per line — and a line that +# does not parse is DROPPED, the rest still read. This is the +# transcript in scribe_report_check.sh, where the window starts +# mid-record by construction: `tail -n 3000` cuts wherever it +# cuts, and the first line is routinely half a record. It is +# exactly the `map(try fromjson catch empty)` the jq program it +# replaces opened with. IDX counts lines that PARSED, so the +# ordering a turn is bounded by is unaffected by the dropped one. +# +# A KEY CONTAINING `.` OR `[` WOULD MAKE AN AMBIGUOUS PATH. Nothing in the two +# JSON dialects this reads — Claude Code hook events and Scribe's own API — has +# one, and inventing an escaping scheme for a case neither producer can emit +# would cost every caller a decode for nothing. Stated so the next person hits +# a comment rather than a mystery. + +function skipws( c) { + while (pos <= n) { + c = substr(s, pos, 1) + if (c == " " || c == "\t" || c == "\n" || c == "\r") pos++ + else return + } +} + +# Values are emitted one per line, so a literal control character inside a +# string — invalid JSON, but producers emit it — must not become a line break +# that silently splits one value into two records. +function emit(path, val) { + ocount++ + opath[ocount] = path + oval[ocount] = tame(val) +} + +function tame(v) { + if (index(v, "\n")) v = rep(v, "\n", "\\n") + if (index(v, "\r")) v = rep(v, "\r", "\\r") + if (index(v, "\t")) v = rep(v, "\t", "\\t") + return v +} + +# Replace every occurrence of a literal separator, via split/join rather than +# gsub: gsub's replacement string treats `\` and `&` as metacharacters, so +# emitting a literal backslash through it needs a quadruple-escape whose +# meaning varies by awk. Concatenation has no such reading. +function rep(str, sep, with, parts, cnt, i, o) { + cnt = split(str, parts, sep) + o = parts[1] + for (i = 2; i <= cnt; i++) o = o with parts[i] + return o +} + +function parse_value(path, c) { + if (failed) return + skipws() + if (pos > n) { failed = 1; return } + c = substr(s, pos, 1) + if (c == "{") { parse_object(path); return } + if (c == "[") { parse_array(path); return } + if (c == "\"") { emit(path, parse_string()); return } + parse_literal(path) +} + +# HOW FAR AHEAD A TOKEN IS READ, and why there is a limit at all. +# +# awk has no way to match a regex STARTING AT AN OFFSET, so reading a token +# means copying the rest of the buffer and anchoring with `^`. Do that per +# token and the cost is quadratic in the length of the line: a transcript +# record carrying a 400KB tool result has a hundred small tokens after it, and +# each one copied the whole 400KB again. Measured: ten such lines, 500KB in +# total, took 2.2s — while 291 ordinary lines totalling 684KB took 0.38s. +# +# So an ordinary token is read out of a WINDOW, and only a token that does not +# fit in one pays for the whole remainder — once, for itself, rather than once +# for every token that follows it. 1024 is where the curve flattens, measured +# over a real 1.19MB turn: 8192 → 800ms, 2048 → 425ms, 1024 → 351ms, 256 → +# 315ms, every one of them producing byte-identical output. Below 1024 the +# gain stops paying for the long values that then miss the window. +# +# A MATCH INSIDE THE WINDOW IS ALWAYS THE TRUE TOKEN, which is what makes this +# safe rather than merely fast. Every quote inside a JSON string is escaped, so +# the `\\.` branch consumes it; an unescaped `"` can only be the real closing +# quote. A window that cuts a string short therefore yields no match at all — +# it cannot yield a WRONG one — and the wide read below handles it. +# +# SET IN `BEGIN`, and it has to be there. A bare assignment at file scope +# is not a statement to awk, it is a PATTERN — a truthy expression with no +# action — so every input record took the default action and was ECHOED to +# stdout. Nothing downstream matched the echoed line, so it cost only noise +# until a test compared this parser's output against Python's and found one +# row too many. + +function parse_string( rest, tok) { + rest = substr(s, pos, WINDOW) + if (match(rest, /^"([^"\\]|\\.)*"/) == 0) { + rest = substr(s, pos) + if (match(rest, /^"([^"\\]|\\.)*"/) == 0) { failed = 1; return "" } + } + tok = substr(rest, 2, RLENGTH - 2) + pos += RLENGTH + return tok +} + +function parse_literal(path, rest) { + rest = substr(s, pos, WINDOW) + if (match(rest, /^(-?[0-9]+(\.[0-9]+)?([eE][-+]?[0-9]+)?|true|false|null)/) == 0) { + failed = 1 + return + } + emit(path, substr(rest, 1, RLENGTH)) + pos += RLENGTH +} + +function parse_object(path, key, c) { + pos++ + skipws() + if (substr(s, pos, 1) == "}") { pos++; emit(path "{#}", 0); return } + while (1) { + skipws() + if (substr(s, pos, 1) != "\"") { failed = 1; return } + key = parse_string() + if (failed) return + skipws() + if (substr(s, pos, 1) != ":") { failed = 1; return } + pos++ + parse_value(path "." key) + if (failed) return + skipws() + c = substr(s, pos, 1) + if (c == ",") { pos++; continue } + if (c == "}") { pos++; return } + failed = 1 + return + } +} + +function parse_array(path, i, c) { + pos++ + i = 0 + skipws() + if (substr(s, pos, 1) == "]") { pos++; emit(path "[#]", 0); return } + while (1) { + parse_value(path "[" i "]") + if (failed) return + i++ + skipws() + c = substr(s, pos, 1) + if (c == ",") { pos++; continue } + if (c == "]") { pos++; emit(path "[#]", i); return } + failed = 1 + return + } +} + +# Parse the buffer in `s` as one value. Emits nothing unless it parses whole. +function run( i) { + n = length(s) + pos = 1 + failed = 0 + ocount = 0 + parse_value("") + if (!failed) { skipws(); if (pos <= n) failed = 1 } + if (failed) { ocount = 0; return 0 } + idx++ + for (i = 1; i <= ocount; i++) printf "%d\t%s\t%s\n", idx, opath[i], oval[i] + ocount = 0 + return 1 +} + +BEGIN { idx = 0; WINDOW = 1024 } + +mode == "lines" { s = $0; run(); next } + +{ buf = buf $0 "\n" } + +END { + if (mode == "lines") exit 0 + s = buf + run() +} diff --git a/plugin/hooks/scribe_prior_art.sh b/plugin/hooks/scribe_prior_art.sh index af4eda6..f012943 100755 --- a/plugin/hooks/scribe_prior_art.sh +++ b/plugin/hooks/scribe_prior_art.sh @@ -33,29 +33,34 @@ # SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path. set -uo pipefail -command -v jq >/dev/null 2>&1 || exit 0 command -v curl >/dev/null 2>&1 || exit 0 +# Shared with the after-write hook (#2901): the prose/data skip list, the +# definition extractor and the local by-name duplicate arm live in +# scribe_defs.sh so the two hooks cannot drift apart. Sourced FIRST because the +# JSON reader is there too now (#4107). +# shellcheck source=plugin/hooks/scribe_defs.sh +. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" + # PreToolUse delivers { session_id, cwd, tool_name, tool_input: {...}, ... } +# One parse, five fields. `.tool_input.content` on a Write is the entire file +# being written, so parsing per field would mean reading it five times. event=$(cat 2>/dev/null || true) -file_path=$(printf '%s' "$event" | jq -r '.tool_input.file_path // empty' 2>/dev/null) || exit 0 -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" -event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_cwd="" +event_flat=$(printf '%s' "$event" | scribe_json_flat) +file_path=$(scribe_json_pick "$event_flat" '.tool_input.file_path') +session_id=$(scribe_json_pick "$event_flat" '.session_id') +event_cwd=$(scribe_json_pick "$event_flat" '.cwd') [ -n "$file_path" ] || exit 0 # The code about to be written. Write and Edit name this field differently, and # the names have changed across Claude Code versions — take whichever is present # rather than betting on one shape. -code=$(printf '%s' "$event" | jq -r ' - .tool_input.content // .tool_input.file_content // - .tool_input.new_string // .tool_input.new_str // empty' 2>/dev/null) || code="" +code=$(scribe_json_pick "$event_flat" '.tool_input.content') +[ -n "$code" ] || code=$(scribe_json_pick "$event_flat" '.tool_input.file_content') +[ -n "$code" ] || code=$(scribe_json_pick "$event_flat" '.tool_input.new_string') +[ -n "$code" ] || code=$(scribe_json_pick "$event_flat" '.tool_input.new_str') -# Shared with the after-write hook (#2901): the prose/data skip list, the -# definition extractor and the local by-name duplicate arm live in -# scribe_defs.sh so the two hooks cannot drift apart. -# shellcheck source=plugin/hooks/scribe_defs.sh -. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" scribe_skip_path "$file_path" && exit 0 # Snippet locations are recorded repo-relative, so send a repo-relative path — @@ -102,15 +107,24 @@ fi # no pulled canon in play, nothing is recorded. Titles only still — this sends # names, not bodies. # --------------------------------------------------------------------------- +# +# THE ENCLOSING DEFINITION IS THE LAST ONE AT OR ABOVE THE EDIT, and taking it +# needs no `tac`. This read `head -n $ln | tac | scribe_defs | head -1` — reverse +# the lines, extract, take the first. `tac` is GNU-only: it is absent on macOS +# (whose equivalent is `tail -r`), so the guard below it meant this arm did +# nothing at all on every Mac, silently, since the day it shipped. scribe_defs +# judges each line independently, so reversing first and taking the head is the +# same answer as extracting forward and taking the tail — and `tail` also drops +# the `head -1` whose early exit is the SIGPIPE trap #4042 was filed for. shapes="$names" -if [ -z "$shapes" ] && [ -f "$file_path" ] && command -v tac >/dev/null 2>&1; then - old_first=$(printf '%s' "$event" \ - | jq -r '.tool_input.old_string // .tool_input.old_str // empty' 2>/dev/null \ - | grep -m1 -v '^[[:space:]]*$' || true) +if [ -z "$shapes" ] && [ -f "$file_path" ]; then + old_first=$(scribe_json_pick "$event_flat" '.tool_input.old_string') + [ -n "$old_first" ] || old_first=$(scribe_json_pick "$event_flat" '.tool_input.old_str') + old_first=$(printf '%s' "$old_first" | grep -m1 -v '^[[:space:]]*$' || true) if [ -n "$old_first" ]; then ln=$(grep -nF -m1 -- "$old_first" "$file_path" 2>/dev/null | cut -d: -f1) || ln="" if [ -n "$ln" ]; then - shapes=$(head -n "$ln" "$file_path" | tac | scribe_defs | head -1 || true) + shapes=$(head -n "$ln" "$file_path" | scribe_defs | tail -1 || true) fi fi fi @@ -118,7 +132,7 @@ shapes_q="" if [ -n "$shapes" ]; then enc=$(printf '%s\n' "$shapes" \ | awk -F'\t' 'NF>=2 {printf "%s%s:%s", (n++?",":""), $1, $2}' \ - | jq -sRr '@uri' 2>/dev/null) || enc="" + | scribe_urlenc) || enc="" [ -n "$enc" ] && shapes_q="&shapes=${enc}" fi @@ -127,8 +141,7 @@ scribe_config || : # sets url/token; unconfigured is handled just below # arm above already ran and may have something to say. if [ -z "$url" ] || [ -z "$token" ]; then if [ -n "$local_context" ]; then - jq -n --arg c "$local_context" \ - '{hookSpecificOutput: {hookEventName: "PreToolUse", additionalContext: $c}}' + scribe_json_out PreToolUse "$local_context" fi exit 0 fi @@ -142,13 +155,15 @@ fi # built a URL from the whole payload. head -c caps the total, which is the point. q=$(printf '%s' "$code" | head -c 1200) -# `-sRr`, not `-rR`: jq -R reads input LINE BY LINE, so a multi-line payload came -# back as several separately-encoded lines joined by raw newlines — an invalid -# URL that made curl fail, and this hook then exited 0 in silence. -s slurps the -# whole input into one string first. Newlines are exactly what code contains, so -# this hook could never have worked without it (issue #2198 / #2082). -path_enc=$(printf '%s' "$rel_path" | jq -sRr '@uri' 2>/dev/null) || exit 0 -code_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || code_enc="" +# Encoded whole, never line by line. The predecessor (`jq -rR`) read input LINE +# BY LINE, so a multi-line payload came back as several separately-encoded lines +# joined by raw newlines — an invalid URL that made curl fail, and this hook then +# exited 0 in silence. Newlines are exactly what code contains, so this hook +# could never have worked that way (issue #2198 / #2082). scribe_urlenc reads +# bytes and has no notion of a line. +path_enc=$(printf '%s' "$rel_path" | scribe_urlenc) +[ -n "$path_enc" ] || exit 0 +code_enc=$(printf '%s' "$q" | scribe_urlenc) # Scope to this directory's project — a `.scribe` marker, else the git remote. scope=$(scribe_scope_query "$lookup_dir") @@ -199,7 +214,7 @@ if [ -n "$session_id" ]; then [ -n "$sync_seen" ] && sync_exclude_q="&exclude_sync_ids=${sync_seen}" fi if [ -f "$derivefile" ]; then - derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | jq -sRr '@uri' 2>/dev/null) || derive_seen="" + derive_seen=$(tr '\n' ',' < "$derivefile" 2>/dev/null | sed 's/,$//' | scribe_urlenc) || derive_seen="" [ -n "$derive_seen" ] && derive_exclude_q="&exclude_derive=${derive_seen}" fi # Ageing, not a flat read (#3751), and the ONLY ledger here that ages: the @@ -227,24 +242,25 @@ else fi context="" +body_flat="" if [ -n "$body" ]; then - context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || context="" + body_flat=$(printf '%s' "$body" | scribe_json_flat) + context=$(scribe_json_pick "$body_flat" '.context') # Remember what was surfaced so it isn't shown again this session — each # class into its own channel: sync ids (snippets recording the edited file) # to the sync file, everything else to the reuse file. if [ -n "$context" ]; then if [ -n "$idfile" ]; then - printf '%s' "$body" | jq -r '((.note_ids // []) - (.sync_note_ids // []))[]?' 2>/dev/null >> "$idfile" || true + scribe_json_list_minus "$body_flat" '.note_ids' '.sync_note_ids' >> "$idfile" || true fi if [ -n "$syncfile" ]; then - printf '%s' "$body" | jq -r '(.sync_note_ids // [])[]?' 2>/dev/null >> "$syncfile" || true + scribe_json_list "$body_flat" '.sync_note_ids' >> "$syncfile" || true fi if [ -n "$rulefile" ]; then - printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \ - | scribe_rules_append "$rulefile" + scribe_json_list "$body_flat" '.rule_ids' | scribe_rules_append "$rulefile" fi if [ -n "$derivefile" ]; then - printf '%s' "$body" | jq -r '(.derive_keys // [])[]?' 2>/dev/null >> "$derivefile" || true + scribe_json_list "$body_flat" '.derive_keys' >> "$derivefile" || true fi fi fi @@ -259,7 +275,7 @@ fi # of those copies is recorded" is a claim only an answer can back — the # unreached line says what actually happened instead. if [ -n "$local_lines" ] && [ "$reached" = 1 ]; then - n_recorded=$(printf '%s' "$body" | jq -r '.note_ids | length' 2>/dev/null) || n_recorded=0 + n_recorded=$(scribe_json_len "$body_flat" '.note_ids') if [ "${n_recorded:-0}" = "0" ] || [ "$n_recorded" = "" ]; then local_context="${local_context}"$'\n'"> None of those existing copies is recorded in Scribe. If the version being written is the canonical one — or this edit is consolidating the copies — record it now with create_snippet (name, code, when-to-reach-for-it, location) so the next session is offered it instead of writing another copy." fi @@ -280,6 +296,5 @@ fi [ -n "$combined" ] || exit 0 # No permissionDecision: this is a nudge, not a gate. The write goes ahead. -jq -n --arg c "$combined" \ - '{hookSpecificOutput: {hookEventName: "PreToolUse", additionalContext: $c}}' +scribe_json_out PreToolUse "$combined" exit 0 diff --git a/plugin/hooks/scribe_record_opened.sh b/plugin/hooks/scribe_record_opened.sh index f41c7b2..9935902 100644 --- a/plugin/hooks/scribe_record_opened.sh +++ b/plugin/hooks/scribe_record_opened.sh @@ -39,23 +39,21 @@ # here is one extra line, in the direction that shows more rather than less. set -uo pipefail +# shellcheck source=plugin/hooks/scribe_defs.sh +. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" + event=$(cat 2>/dev/null || true) [ -n "$event" ] || exit 0 -# No jq, no ledger — and no complaint. Every other hook degrades the same way -# rather than printing a tooling error in front of the operator's work (#4107 -# tracks making that dependency honest; this is not the place to diverge). -command -v jq >/dev/null 2>&1 || exit 0 - -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" +event_flat=$(printf '%s' "$event" | scribe_json_flat) +session_id=$(scribe_json_pick "$event_flat" '.session_id') [ -n "$session_id" ] || exit 0 # The matcher in hooks.json already narrows to the get_rule tools, but the # server segment of an MCP tool name varies with how the plugin was installed, # so the id is read from whichever field is actually present rather than from # an assumed tool name. An event that carries none simply records nothing. -rule_id=$(printf '%s' "$event" \ - | jq -r '(.tool_input.rule_id // empty) | tostring' 2>/dev/null) || rule_id="" +rule_id=$(scribe_json_pick "$event_flat" '.tool_input.rule_id') rule_id=$(printf '%s' "$rule_id" | tr -cd '0-9') [ -n "$rule_id" ] || exit 0 @@ -67,9 +65,6 @@ state_dir="${TMPDIR:-/tmp}/scribe-priorart" mkdir -p "$state_dir" 2>/dev/null || true safe_sid=$(printf '%s' "$session_id" | tr -c 'A-Za-z0-9._-' '_') -# shellcheck source=plugin/hooks/scribe_defs.sh -. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" - # Stamped and append-only, exactly like the naming ledger — so the same reader # (`scribe_rules_live`) ages both, and the last entry for an id wins. printf '%s\n' "$rule_id" | scribe_rules_append "$state_dir/${safe_sid}.opened.ids" diff --git a/plugin/hooks/scribe_report_check.sh b/plugin/hooks/scribe_report_check.sh index 1b79045..d8311f4 100644 --- a/plugin/hooks/scribe_report_check.sh +++ b/plugin/hooks/scribe_report_check.sh @@ -50,7 +50,6 @@ # SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path. set -uo pipefail -command -v jq >/dev/null 2>&1 || exit 0 command -v curl >/dev/null 2>&1 || exit 0 # shellcheck source=plugin/hooks/scribe_defs.sh @@ -58,11 +57,12 @@ command -v curl >/dev/null 2>&1 || exit 0 # Stop delivers { session_id, transcript_path, cwd, hook_event_name, stop_hook_active }. event=$(cat 2>/dev/null || true) -transcript=$(printf '%s' "$event" | jq -r '.transcript_path // empty' 2>/dev/null) || exit 0 +event_flat=$(printf '%s' "$event" | scribe_json_flat) +transcript=$(scribe_json_pick "$event_flat" '.transcript_path') [ -n "$transcript" ] && [ -f "$transcript" ] || exit 0 -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" -active=$(printf '%s' "$event" | jq -r '.stop_hook_active // false' 2>/dev/null) || active="false" -event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_cwd="" +session_id=$(scribe_json_pick "$event_flat" '.session_id') +active=$(scribe_json_pick "$event_flat" '.stop_hook_active') +event_cwd=$(scribe_json_pick "$event_flat" '.cwd') safe_sid=$(printf '%s' "${session_id:-nosession}" | tr -c 'A-Za-z0-9._-' '_') state_dir="${TMPDIR:-/tmp}/scribe-reportcheck" @@ -78,48 +78,67 @@ grep -q -E '"name":[[:space:]]*"([^"]*__)?(update|create)_task"' < <(tail -c 200 exit 0 } -# The turn, parsed once. A window of recent lines, slurped raw and split inside -# jq (a line-by-line `-R` read is the #2198 trap). A first line cut mid-record -# fails to parse and is dropped. If the window holds no prompt, the turn cannot -# be bounded, so the hook reports nothing and stays out of the way. -facts=$(tail -n 3000 "$transcript" 2>/dev/null | jq -sRc ' - split("\n") | map(try fromjson catch empty) - | map(select((.isSidechain // false) | not)) - | . as $lines - | [range(0; length) | select( - $lines[.].type == "user" and ($lines[.].isMeta // false | not) - and ($lines[.].message.content | type) == "string")] as $prompts - | if ($prompts | length) == 0 then {bounded: false} else - $lines[($prompts | last) + 1:] as $turn - | [ $turn[] | select(.type == "assistant") | .message.content[]? - | select(.type == "tool_use" - and ((.name // "") | test("(^|__)(update|create)_task$")) - and (.input.status? == "done")) - | {id, task: (.input.task_id? // null)} ] as $closes - | [ $turn[] | select(.type == "user") | .message.content[]? - | select(type == "object" and .type == "tool_result" and .is_error == true) - | .tool_use_id ] as $errors - | [ $closes[] | select(.id as $i | ($errors | index($i)) | not) ] as $closed - | ([range(0; $turn | length) | select( - $turn[.].type == "user" - or ($turn[.].type == "assistant" - and ([$turn[.].message.content[]?.type] | index("tool_use"))))] - | last // -1) as $last_act - | {bounded: true, - closed: ($closed | length), - task_ids: [$closed[].task | select(. != null)], - reply: ([ $turn[$last_act + 1:][] | select(.type == "assistant") - | .message.content[]? | select(.type == "text") | .text ] | join("\n"))} - end' 2>/dev/null) || exit 0 +# The turn, parsed once. A window of recent lines, flattened record by record +# and then read by scribe_turn.awk, which carries the turn-bounding rules. A +# line that does not parse is dropped and the rest are still read — the first +# line of a `tail -n 3000` window is routinely half a record. If the window +# holds no prompt, the turn cannot be bounded, so the hook reports nothing and +# stays out of the way. +window() { tail -n 3000 "$transcript" 2>/dev/null; } +turn_facts() { window | tail -n +"${1:-1}" | scribe_json_flat_lines \ + | awk -f "$SCRIBE_HOOK_DIR/scribe_turn.awk" 2>/dev/null; } -[ "$(printf '%s' "$facts" | jq -r '.bounded // false')" = "true" ] || exit 0 -closed=$(printf '%s' "$facts" | jq -r '.closed // 0') +# WHERE THE TURN STARTS, FOUND BEFORE PARSING RATHER THAN AFTER. The window is +# 3000 lines and routinely 7MB, of which a turn is the last few hundred lines +# and about a sixth of the bytes — the rest is tool results this check never +# looks at. The predecessor parsed all of it and threw most away, which jq +# could afford and a parser written in awk cannot: measured at 6.5s for a 7MB +# window against 94ms, on a hook that runs at the end of every turn. +# +# So grep — C, and reading a FIXED string — narrows first. A prompt record is +# `"type":"user"` whose `content` is a STRING; a tool result is the same type +# with an ARRAY, and the two are told apart by the character after `"content":`. +# +# WHY A FIXED STRING IS EXACT HERE, and not the usual regex-over-JSON guess. +# Every quote inside a JSON string is backslash-escaped, so a needle carrying +# UNESCAPED quotes cannot occur inside any string value — it can only match at +# a record's own top level. `"message":{"role":"user","content":"` therefore +# matches real prompt records and nothing else. Measured over a 27MB transcript +# against a full JSON parse: 152 prompt records, 152 matches, no misses and no +# extras. The looser `"content":"` matched 1101 lines, because a tool_result +# block has a `content` key of its own — which is the trap this avoids. +# +# THE LAST MATCH, not a few before it, because the margin is not free: the +# lines between two prompts are mostly tool results, and backing off three +# matches took the window from 53KB to 1.3MB and the parse from 21ms to 2.6s. +# The fallback below is the safety net instead — it is exact where a margin is +# only approximate, and it costs nothing in the case that actually happens. +# +# The needle assumes a key ORDER that a future Claude Code could change. If it +# does, grep matches nothing, `start` stays 1, and the whole window is read the +# slow way — correct, and slow, which is the right way round for a check that +# can block a stop. +start=$(window | grep -n -F '"message":{"role":"user","content":"' 2>/dev/null \ + | cut -d: -f1 | awk 'END { if (NR) print $0 }') +case "$start" in ''|*[!0-9]*) start=1 ;; esac + +facts=$(turn_facts "$start") +fact() { printf '%s\n' "$facts" | awk -F'\t' -v k="$1" '$1 == k { print substr($0, index($0, "\t") + 1); exit }'; } + +if [ "$(fact bounded)" != "1" ] && [ "$start" != "1" ]; then + facts=$(turn_facts 1) +fi + +[ "$(fact bounded)" = "1" ] || exit 0 +closed=$(fact closed) if [ "${closed:-0}" = "0" ]; then rm -f "$marker" 2>/dev/null || true exit 0 fi -reply=$(printf '%s' "$facts" | jq -r '.reply // ""') -task_ids=$(printf '%s' "$facts" | jq -r '.task_ids | map(tostring) | join(",")') +# Escaped on one line coming out of awk, so the format survives a multi-line +# reply; decoded here, once, where it is about to be read as text. +reply=$(fact reply | scribe_json_unescape) +task_ids=$(fact task_ids) # The reply may not be written to the transcript yet when the hook fires. An # empty reply is "cannot tell", not "missing everything" — stay out of the way. @@ -146,7 +165,7 @@ report() { q="outcome=$1&task_ids=${task_ids}" m=$(IFS=,; printf '%s' "${missing[*]:-}") if [ -n "$m" ]; then - enc=$(printf '%s' "$m" | jq -sRr '@uri' 2>/dev/null) || enc="" + enc=$(printf '%s' "$m" | scribe_urlenc) || enc="" q="${q}&missing=${enc}" fi scope=$(scribe_scope_query "${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}}") @@ -176,8 +195,8 @@ fi # a hook carries timing and transport). No reason back → nothing recorded → # no block. answer=$(report blocked) || exit 0 -reason=$(printf '%s' "$answer" | jq -r '.reason // empty' 2>/dev/null) || reason="" +reason=$(scribe_json_pick "$(printf '%s' "$answer" | scribe_json_flat)" '.reason') [ -n "$reason" ] || exit 0 : > "$marker" 2>/dev/null || true -jq -n --arg r "$reason" '{decision: "block", reason: $r}' +printf '{"decision":"block","reason":"%s"}\n' "$(printf '%s' "$reason" | scribe_json_escape)" exit 0 diff --git a/plugin/hooks/scribe_session_context.sh b/plugin/hooks/scribe_session_context.sh index b0dacaa..c95d721 100755 --- a/plugin/hooks/scribe_session_context.sh +++ b/plugin/hooks/scribe_session_context.sh @@ -49,8 +49,6 @@ set -uo pipefail # shellcheck source=plugin/hooks/scribe_defs.sh . "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" -command -v jq >/dev/null 2>&1 || exit 0 # needed to emit the JSON envelope safely - # `CDPATH= cd` is deliberate, not a typo'd assignment: it runs this one `cd` # with CDPATH empty, so an operator whose CDPATH happens to contain a matching # directory name can't send us somewhere else — and `cd` won't echo the resolved @@ -60,7 +58,8 @@ here=$(CDPATH= cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || exit 0 # SessionStart delivers a JSON event on stdin; `source` is startup|resume|compact|clear. event=$(cat 2>/dev/null || true) -source=$(printf '%s' "$event" | jq -r '.source // empty' 2>/dev/null) || source="" +event_flat=$(printf '%s' "$event" | scribe_json_flat) +source=$(scribe_json_pick "$event_flat" '.source') # --- The rule ledger outlives the context it describes (#3749) --- # @@ -127,7 +126,7 @@ source=$(printf '%s' "$event" | jq -r '.source // empty' 2>/dev/null) || source= # missed exactly the arm that fires most. case "$source" in compact|clear) - sid=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || sid="" + sid=$(scribe_json_pick "$event_flat" '.session_id') if [ -n "$sid" ]; then safe_sid=$(printf '%s' "$sid" | tr -c 'A-Za-z0-9._-' '_') scribe_clear_session_ledgers "$safe_sid" @@ -163,7 +162,7 @@ prepend() { if [ -n "$out" ]; then out="$1"$'\n\n---\n\n'"${out}"; else out="$1" # appears there. manifest="$here/../.claude-plugin/plugin.json" if [ -f "$manifest" ]; then - plugin_version=$(jq -r '.version // empty' "$manifest" 2>/dev/null) || plugin_version="" + plugin_version=$(scribe_json_pick "$(scribe_json_flat < "$manifest")" '.version') if [ -n "$plugin_version" ]; then append "> Scribe plugin **v${plugin_version}** is executing in this session. A fix merged after this version has not reached it — the marketplace clone updates on its own, but the cache that runs only refreshes when the manifest version changes." fi @@ -193,7 +192,11 @@ if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then body=$(curl -fsS --max-time 8 \ -H "Authorization: Bearer ${token}" \ "${url%/}/api/plugin/context${q}" 2>/dev/null) || body="" - [ -n "$body" ] && dyn=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) + body_flat="" + if [ -n "$body" ]; then + body_flat=$(printf '%s' "$body" | scribe_json_flat) + dyn=$(scribe_json_pick "$body_flat" '.context') + fi # The rules marker is gone with the resident set it described # (milestone 394). Nothing is preloaded, so there is no set whose # drift a later write could be told about — a rule is retrieved at @@ -227,7 +230,7 @@ fi # server reports whether a project resolved, and this hook — which knows what # it sent and why — turns that into the sentence the operator can act on. The # repo case is left to the server's existing "bind this repo" hint. -if [ -n "$dyn" ] && [ -z "$(printf '%s' "$body" | jq -r '.project.id // empty' 2>/dev/null)" ]; then +if [ -n "$dyn" ] && [ -z "$(scribe_json_pick "$body_flat" '.project.id')" ]; then host=$(scribe_url_host "$url") if [ -n "$marker_why" ]; then append "> ⚠️ Scribe: the marker file \`${marker}\` ${marker_why}, so no project context was loaded. Fix the file, or ignore it and bind this directory another way." @@ -246,6 +249,5 @@ fi # Nothing at all to inject → stay silent. [ -n "$out" ] || exit 0 -jq -n --arg c "$out" \ - '{hookSpecificOutput: {hookEventName: "SessionStart", additionalContext: $c}}' +scribe_json_out SessionStart "$out" exit 0 diff --git a/plugin/hooks/scribe_sync_processes.sh b/plugin/hooks/scribe_sync_processes.sh index 8e0d0dc..bc7a5c6 100755 --- a/plugin/hooks/scribe_sync_processes.sh +++ b/plugin/hooks/scribe_sync_processes.sh @@ -26,7 +26,6 @@ set -uo pipefail # shellcheck source=plugin/hooks/scribe_defs.sh . "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" -command -v jq >/dev/null 2>&1 || exit 0 command -v curl >/dev/null 2>&1 || exit 0 scribe_config || exit 0 @@ -36,8 +35,12 @@ body=$(curl -fsS --max-time 8 \ "${url%/}/api/plugin/processes" 2>/dev/null) || exit 0 [ -n "$body" ] || exit 0 -count=$(printf '%s' "$body" | jq -r '.processes | length' 2>/dev/null) || exit 0 -[ -n "$count" ] && [ "$count" != "null" ] || exit 0 +body_flat=$(printf '%s' "$body" | scribe_json_flat) +# The array's LENGTH, which is "" when `.processes` is not an array at all — +# a different answer from 0, and the one that means the response was not what +# this hook expects. +count=$(scribe_json_len "$body_flat" '.processes') +case "$count" in ''|*[!0-9]*) exit 0 ;; esac skills_dir="${HOME}/.claude/skills" mkdir -p "$skills_dir" 2>/dev/null || exit 0 @@ -47,9 +50,9 @@ managed=" " i=0 while [ "$i" -lt "$count" ]; do - name=$(printf '%s' "$body" | jq -r ".processes[$i].name // empty" 2>/dev/null) - slug=$(printf '%s' "$body" | jq -r ".processes[$i].slug // empty" 2>/dev/null) - desc=$(printf '%s' "$body" | jq -r ".processes[$i].description // empty" 2>/dev/null) + name=$(scribe_json_pick "$body_flat" ".processes[$i].name") + slug=$(scribe_json_pick "$body_flat" ".processes[$i].slug") + desc=$(scribe_json_pick "$body_flat" ".processes[$i].description") i=$((i + 1)) [ -n "$slug" ] && [ -n "$name" ] || continue diff --git a/plugin/hooks/scribe_tool_rules.sh b/plugin/hooks/scribe_tool_rules.sh index 1995aaa..44d1a4d 100644 --- a/plugin/hooks/scribe_tool_rules.sh +++ b/plugin/hooks/scribe_tool_rules.sh @@ -20,31 +20,31 @@ # Env: # SCRIBE_URL / SCRIBE_TOKEN override for the settings.json dogfooding path. -command -v jq >/dev/null 2>&1 || exit 0 command -v curl >/dev/null 2>&1 || exit 0 +# Sourced FIRST, because the JSON reader below lives there (#4107). It defines +# functions and clears `url`/`token`; nothing here runs before it is needed. +# shellcheck source=plugin/hooks/scribe_defs.sh +. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" + # PreToolUse delivers { session_id, cwd, tool_name, tool_input: {...}, ... } event=$(cat 2>/dev/null || true) -tool_name=$(printf '%s' "$event" | jq -r '.tool_name // empty' 2>/dev/null) || exit 0 -session_id=$(printf '%s' "$event" | jq -r '.session_id // empty' 2>/dev/null) || session_id="" -event_cwd=$(printf '%s' "$event" | jq -r '.cwd // empty' 2>/dev/null) || event_cwd="" +event_flat=$(printf '%s' "$event" | scribe_json_flat) +tool_name=$(scribe_json_pick "$event_flat" '.tool_name') +session_id=$(scribe_json_pick "$event_flat" '.session_id') +event_cwd=$(scribe_json_pick "$event_flat" '.cwd') [ -n "$tool_name" ] || exit 0 # The action, as text. `.command` is Bash's field; the fallbacks let the matcher # in hooks.json widen to other tools without this script changing — which is the # whole reason the server side takes a name and a string rather than a schema. -command_text=$(printf '%s' "$event" | jq -r ' - .tool_input.command // - .tool_input.url // - .tool_input.prompt // - empty' 2>/dev/null) || command_text="" +command_text=$(scribe_json_pick "$event_flat" '.tool_input.command') +[ -n "$command_text" ] || command_text=$(scribe_json_pick "$event_flat" '.tool_input.url') +[ -n "$command_text" ] || command_text=$(scribe_json_pick "$event_flat" '.tool_input.prompt') [ -n "$command_text" ] || exit 0 -# shellcheck source=plugin/hooks/scribe_defs.sh -. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh" - # scribe_config, not a hand-rolled pair of parameter expansions: it also treats # an UNEXPANDED `${...}` placeholder as unset, which would otherwise be sent as # a garbage Bearer token and 401 on every call (#2198's class). @@ -56,10 +56,12 @@ scribe_config || exit 0 # place. `head -c`, never `cut -c`: cut truncates each LINE and caps nothing. command_text=$(printf '%s' "$command_text" | head -c 2000) -# -sRr, never -rR: jq -R without -s reads LINE BY LINE, so a multi-line command -# would encode per line and join with raw newlines — an invalid URL. -cmd_enc=$(printf '%s' "$command_text" | jq -sRr '@uri' 2>/dev/null) || exit 0 -tool_enc=$(printf '%s' "$tool_name" | jq -sRr '@uri' 2>/dev/null) || exit 0 +# Whole, never line by line: the predecessor (`jq -rR`) encoded a multi-line +# command one line at a time and joined them with raw newlines — an invalid URL. +# scribe_urlenc reads bytes and has no notion of a line. +cmd_enc=$(printf '%s' "$command_text" | scribe_urlenc) +tool_enc=$(printf '%s' "$tool_name" | scribe_urlenc) +[ -n "$cmd_enc" ] && [ -n "$tool_enc" ] || exit 0 repo_q="" lookup_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} @@ -98,19 +100,14 @@ body=$(curl -fsS --max-time 5 \ -H "Authorization: Bearer ${token}" \ "${url%/}/api/plugin/tool-rules?tool=${tool_enc}&command=${cmd_enc}${repo_q}${rule_exclude_q}" 2>/dev/null) || exit 0 -context=$(printf '%s' "$body" | jq -r '.context // empty' 2>/dev/null) || exit 0 +body_flat=$(printf '%s' "$body" | scribe_json_flat) +context=$(scribe_json_pick "$body_flat" '.context') [ -n "$context" ] || exit 0 # Remember what was named so it is not repeated this session. if [ -n "$rulefile" ]; then - printf '%s' "$body" | jq -r '(.rule_ids // [])[]?' 2>/dev/null \ - | scribe_rules_append "$rulefile" + scribe_json_list "$body_flat" '.rule_ids' | scribe_rules_append "$rulefile" fi -jq -cn --arg ctx "$context" '{ - hookSpecificOutput: { - hookEventName: "PreToolUse", - additionalContext: $ctx - } -}' 2>/dev/null || true +scribe_json_out PreToolUse "$context" exit 0 diff --git a/plugin/hooks/scribe_turn.awk b/plugin/hooks/scribe_turn.awk new file mode 100644 index 0000000..44f07a9 --- /dev/null +++ b/plugin/hooks/scribe_turn.awk @@ -0,0 +1,108 @@ +# Scribe plugin — what happened in the LAST TURN of a transcript (#4107). +# +# Reads the flat `IDXPATHVALUE` stream that scribe_json.awk produces +# in `mode=lines` from a Claude Code transcript, and answers the four questions +# scribe_report_check.sh asks. It replaces a thirty-line jq program; the shape +# of the answer is unchanged, so the hook around it reads the same. +# +# bounded 1 if a user prompt was found in the window, else empty. A window +# with no prompt cannot be cut into a turn, and the hook then says +# nothing rather than guessing — `tail -n 3000` cuts wherever it +# cuts, and a turn that began before the cut is not this hook's. +# closed how many task-closing tool calls SUCCEEDED in that turn. +# task_ids their task ids, comma-joined, for the server to record. +# reply the assistant text after the last action, STILL JSON-ESCAPED and +# on one line. The caller decodes it with scribe_json_unescape — +# decoding here would put newlines into a line-oriented format. +# +# A SIDECHAIN IS NOT THIS SESSION. Subagent records interleave into the same +# file, and a subagent closing a task is not the operator's session closing +# one — counting those made the check fire on turns that closed nothing. +# +# A CLOSE THAT ERRORED IS NOT A CLOSE, which is why the error pass runs first: +# the tool_result carrying `is_error` arrives in a LATER record than the +# tool_use it refutes, so a single forward pass would have already counted it. + +# TAB-SEPARATED, and stated rather than assumed. Under awk's default splitting +# a value is cut at its first SPACE, so `$3` of a reply line was the reply's +# first word — the kind of defect that hides completely behind a test whose +# replies are all empty. Found by differential-testing this against the jq +# program it replaces, over real transcript windows (#4107). +BEGIN { FS = "\t" } + +{ + i = $1 + 0 + if (i > maxidx) maxidx = i + p = $2 + if (p == ".type") { f[i, "type"] = $3; next } + if (p == ".isSidechain") { f[i, "side"] = $3; next } + if (p == ".isMeta") { f[i, "meta"] = $3; next } + # `.message.content` as a SCALAR is what marks a real user prompt; a tool + # result carries an array at the same path, and counting one as a prompt + # would cut the turn at the wrong place. + if (p == ".message.content") { if ($3 != "null") f[i, "str"] = 1; next } + if (substr(p, 1, 17) != ".message.content[") next + rest = substr(p, 18) + if (rest == "#]") { nb[i] = $3 + 0; next } + c = index(rest, "]") + if (c < 2) next + b[i, substr(rest, 1, c - 1) + 0, substr(rest, c + 1)] = $3 +} + +function is_mine(i) { + return (f[i, "side"] != "true") +} + +function has_tool_use(i, j) { + for (j = 0; j < nb[i]; j++) if (b[i, j, ".type"] == "tool_use") return 1 + return 0 +} + +END { + for (i = 1; i <= maxidx; i++) + if (is_mine(i) && f[i, "type"] == "user" && f[i, "meta"] != "true" && f[i, "str"] == 1) + prompt = i + if (!prompt) { print "bounded\t"; exit 0 } + + for (i = prompt + 1; i <= maxidx; i++) { + if (!is_mine(i) || f[i, "type"] != "user") continue + for (j = 0; j < nb[i]; j++) + if (b[i, j, ".type"] == "tool_result" && b[i, j, ".is_error"] == "true") + errored[b[i, j, ".tool_use_id"]] = 1 + } + + for (i = prompt + 1; i <= maxidx; i++) { + if (!is_mine(i) || f[i, "type"] != "assistant") continue + for (j = 0; j < nb[i]; j++) { + if (b[i, j, ".type"] != "tool_use") continue + if (b[i, j, ".name"] !~ /(^|__)(update|create)_task$/) continue + if (b[i, j, ".input.status"] != "done") continue + if (b[i, j, ".id"] in errored) continue + closed++ + t = b[i, j, ".input.task_id"] + if (t != "") ids = ids (ids == "" ? "" : ",") t + } + } + + # Where the WORK stopped and the report began. Everything after the last + # action is the reply being checked; text emitted between two tool calls is + # narration mid-work, not a report, and holding it to the report shape would + # block turns that did report properly at the end. + act = prompt + for (i = prompt + 1; i <= maxidx; i++) { + if (!is_mine(i)) continue + if (f[i, "type"] == "user" || (f[i, "type"] == "assistant" && has_tool_use(i))) act = i + } + + for (i = act + 1; i <= maxidx; i++) { + if (!is_mine(i) || f[i, "type"] != "assistant") continue + for (j = 0; j < nb[i]; j++) + if (b[i, j, ".type"] == "text") + reply = reply (reply == "" ? "" : "\\n") b[i, j, ".text"] + } + + printf "bounded\t1\n" + printf "closed\t%d\n", closed + 0 + printf "task_ids\t%s\n", ids + printf "reply\t%s\n", reply +} diff --git a/scripts/check_plugin.py b/scripts/check_plugin.py index b1df3cf..eb1f7eb 100755 --- a/scripts/check_plugin.py +++ b/scripts/check_plugin.py @@ -28,18 +28,23 @@ forgetting to run it is still possible. What changed is that forgetting is now LOUD — a red lane on the batch that forgot, instead of a silent no-op found weeks later when somebody says "I don't think it updated" (#2220). -shellcheck and jq are NOT in `ci-python` (verified against CI-runner's Dockerfile -and scripts/install-common.sh, not from memory — rule #37). CI installs both -per-job, which is what CI-runner's own docs/process.md prescribes for a dep with -a single consumer: "If only one project needs the dep, prefer that project +shellcheck is NOT in `ci-python` (verified against CI-runner's Dockerfile and +scripts/install-common.sh, not from memory — rule #37). CI installs it per-job, +which is what CI-runner's own docs/process.md prescribes for a dep with a +single consumer: "If only one project needs the dep, prefer that project installing it per-job in their workflow — at least until a second consumer arrives." Promotion into the image is filed as an issue there rather than assumed here. -Both are optional at runtime: without shellcheck the lint step is SKIPPED and -says so, and without jq the smoke test is skipped. A skipped check announces -itself loudly, because a check that quietly no-ops is the failure mode this -whole file exists to prevent. +It is optional at runtime: without shellcheck the lint step is SKIPPED and says +so. A skipped check announces itself loudly, because a check that quietly +no-ops is the failure mode this whole file exists to prevent. + +jq USED TO BE IN THAT SAME SENTENCE, and the smoke tests below skipped +themselves without it. Since jq is not in `ci-python` either, that meant three +of them announced a skip on every CI run and had never actually run there. The +hooks need no jq now (#4107), so those checks run unconditionally — removing a +dependency from the product removed a permanent hole in its verification. Usage: python3 scripts/check_plugin.py # all checks @@ -197,16 +202,24 @@ PATTERNS: list[tuple[re.Pattern, str, str]] = [ "hook then does nothing, silently.", ), ( - # -R without -s: reads input line by line, so a multi-line payload is - # encoded per line and joined with raw newlines. The class is a-r + t-z - # (i.e. every letter EXCEPT `s`) so `-rR` is caught and `-sRr` is not — - # an earlier a-q spelling silently excluded `r` and missed the real - # defect, which is exactly the flag combination that shipped. - re.compile(r"jq\s+-(?:[a-rt-zA-Z]*R[a-rt-zA-Z]*)\s"), - "line-oriented jq -R", - "jq -R reads input LINE BY LINE. Encoding a multi-line payload that way " - "produces separate encoded lines joined by raw newlines — an invalid " - "URL. Use -s (slurp) as well, e.g. `jq -sRr '@uri'`.", + # NEITHER OF THESE MAY COME BACK (#4107). This replaced a narrower rule + # about `jq -R` being line-oriented, which is now moot in the only way + # that rule could become moot: there is no jq left to pass flags to. + # + # The wider rule is the one worth having. jq is absent by default on + # macOS, on the Debian/Ubuntu slim images, on Alpine and in most CI + # containers; `tac` is GNU-only and absent on macOS. Every hook used to + # guard itself with `command -v jq || exit 0`, so a machine without it + # got no context, no rules, no prior art and no process sync, silently — + # `exit 0` is indistinguishable from "nothing to say". A hook may use + # only what POSIX guarantees; scribe_defs.sh has the readers. + re.compile(r"(? subprocess def check_fail_open() -> None: - if not shutil.which("jq"): - # Without jq every hook bails at its first line, so this would pass - # while exercising nothing. Say so rather than bank a green tick. - skip("jq not installed — the hooks would exit at line 1, so this " - "check would pass without testing anything") - return - + # No jq gate any more (#4107). This check used to skip itself when jq was + # missing, because without it every hook bailed at line 1 and the check + # would have passed while exercising nothing. jq is NOT in `ci-python`, so + # what that actually meant is that this smoke test announced a skip on + # every CI run and never once ran there. The hooks now need only POSIX + # tools, so it runs everywhere — which is the point of the change it is + # testing. scenarios = [ ("unconfigured", {}), # Connection refused immediately — exercises the unreachable-instance @@ -467,8 +480,8 @@ def check_local_prior_art_needs_no_instance() -> None: nothing to say, and speaking when there is — both with no instance at all. """ script = HOOKS_DIR / "scribe_prior_art.sh" - if not script.is_file() or not shutil.which("jq"): - skip("prior-art local arm: hook or jq missing") + if not script.is_file(): + skip("prior-art local arm: hook missing") return # A definition this repo really does contain, written into a DIFFERENT file @@ -510,8 +523,8 @@ def check_session_context_reports_its_version() -> None: would be missing exactly when it is wanted. """ script = HOOKS_DIR / "scribe_session_context.sh" - if not script.is_file() or not shutil.which("jq"): - skip("version marker: hook or jq missing") + if not script.is_file(): + skip("version marker: hook missing") return manifest_v = manifest_version() diff --git a/tests/test_after_write_hook.py b/tests/test_after_write_hook.py index 8ac6501..06de4ac 100644 --- a/tests/test_after_write_hook.py +++ b/tests/test_after_write_hook.py @@ -21,7 +21,7 @@ HOOK = PLUGIN / "hooks" / "scribe_after_write.sh" def _env(tmp_path, url="http://127.0.0.1:9"): - for tool in ("git", "jq", "curl", "bash"): + for tool in ("git", "curl", "bash"): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") return {"PATH": os.environ["PATH"], "SCRIBE_URL": url, "SCRIBE_TOKEN": "t", diff --git a/tests/test_hook_json_reader.py b/tests/test_hook_json_reader.py new file mode 100644 index 0000000..fcdc371 --- /dev/null +++ b/tests/test_hook_json_reader.py @@ -0,0 +1,380 @@ +"""JSON, read and written by the hooks without jq (#4107). + +WHAT THIS REPLACED, AND WHY IT NEEDS TESTING AT ALL. Every hook used to open +`command -v jq >/dev/null 2>&1 || exit 0`, so on a machine without jq the +operator got no session context, no rules, no prior art and no process sync, +and not one word saying why — `exit 0` is indistinguishable from "ran fine, +nothing to say". jq is absent by default on macOS, on the Debian/Ubuntu slim +images, on Alpine and in most CI containers. The parser that replaced it is +`plugin/hooks/scribe_json.awk`, written in POSIX awk, and a parser is exactly +the kind of thing that works on the six documents you tried it on. + +So the centre of this file is a DIFFERENTIAL against Python's `json`: the same +documents, flattened by both, compared. A reader that agrees with a real JSON +implementation on nested objects, arrays, escapes, unicode and the empty cases +is one the hooks can be handed an arbitrary event with. + +The rest pins the three jobs around it — writing the hook envelope, +percent-encoding a URL, and bounding a turn in a transcript — and that each +guard here can fail (rule 167). +""" +from __future__ import annotations + +import json +import re +import shutil +import subprocess +from pathlib import Path +from urllib.parse import quote + +import pytest + +HOOKS = Path(__file__).resolve().parents[1] / "plugin" / "hooks" +DEFS = HOOKS / "scribe_defs.sh" +PARSER = HOOKS / "scribe_json.awk" +TURN = HOOKS / "scribe_turn.awk" + + +def _need(*tools): + for t in tools: + if shutil.which(t) is None: + pytest.skip(f"hook runtime tool {t!r} not installed") + + +def sh(script: str, stdin: str = "") -> str: + """Run a snippet with scribe_defs.sh sourced, under the hooks' own flags. + + BYTES IN, BYTES OUT, decoded here — NOT `text=True`. Text mode turns on + universal newlines, which rewrites a carriage return in the output to a + newline before the assertion ever sees it. A test of an escaper cannot + quietly normalise the characters it exists to check: the first version of + this file did, and reported a round-trip failure that was entirely its own. + """ + _need("bash", "awk") + r = subprocess.run( + ["bash", "-c", f'set -uo pipefail\n. "{DEFS}"\n{script}'], + input=stdin.encode(), capture_output=True, + ) + assert r.returncode == 0, f"exit {r.returncode}: {r.stderr.decode()}" + return r.stdout.decode() + + +def flat(doc: str, mode: str = "whole") -> list[tuple[str, str, str]]: + _need("awk") + r = subprocess.run(["awk", "-v", f"mode={mode}", "-f", str(PARSER)], + input=doc.encode(), capture_output=True) + assert r.returncode == 0, r.stderr.decode() + return [tuple(line.split("\t", 2)) for line in r.stdout.decode().split("\n") if line] + + +# -------------------------------------------------------------------------- +# The differential: this parser against Python's. + +def reference_flatten(value, path="", out=None): + """What scribe_json.awk is specified to emit, computed with `json`. + + A string's VALUE is its RAW escaped body — the parser deliberately does not + decode, because a newline inside a value would break the line format the + shell reads it back with. Every array also reports its LENGTH at `[#]`, + including an empty one, and an empty object reports `{#}` 0 — that is what + lets a caller tell "the server answered with zero notes" from "the server + did not answer", which #2932 built an outage marker around. + """ + out = [] if out is None else out + if isinstance(value, dict): + if not value: + out.append((path + "{#}", "0")) + for k, v in value.items(): + reference_flatten(v, f"{path}.{k}", out) + elif isinstance(value, list): + for i, v in enumerate(value): + reference_flatten(v, f"{path}[{i}]", out) + out.append((path + "[#]", str(len(value)))) + elif isinstance(value, str): + out.append((path, json.dumps(value, ensure_ascii=False)[1:-1])) + elif value is None: + out.append((path, "null")) + elif isinstance(value, bool): + out.append((path, "true" if value else "false")) + else: + out.append((path, json.dumps(value))) + return out + + +NASTY = [ + "plain", + "", + 'has "quotes" inside', + "back \\ slash and \\\\ two", + "line one\nline two\nline three", + "tab\there and\ttwice", + "carriage\rreturn", + "héllo wörld — em dash", + "emoji \U0001f600 and \U0001f9ea and a ZWJ \U0001f469‍\U0001f4bb", + 'mixed: "q" \\ \n \t é \U0001f600', + "a" * 3000, # longer than the parser's token WINDOW + '{"looks":"like json"}', # JSON inside a string must not be parsed + "trailing backslash \\", + " bell and  unit-sep", +] + +DOCUMENTS = [ + {"prompt": "write the tests first", "session_id": "abc-1", "cwd": "/x/y"}, + {"tool_input": {"file_path": "/a/b.py", "content": "def f():\n pass\n"}}, + {"note_ids": [1, 2, 3], "sync_note_ids": [], "rule_ids": [9]}, + {"context": "", "n": None, "ok": True, "no": False, "num": -12.5, "exp": 1000.0}, + {"processes": [{"name": "A", "slug": "a"}, {"name": "B", "slug": "b"}]}, + {"deep": {"a": {"b": {"c": {"d": [{"e": "f"}]}}}}}, + {"empty_obj": {}, "empty_arr": [], "nested_empty": {"x": []}}, + {"message": {"role": "user", "content": [{"type": "text", "text": "hi"}]}}, + 2, + "bare string", + [1, [2, [3, []]]], +] + [{"v": s} for s in NASTY] + [{"k": {"inner": s, "arr": [s, s]}} for s in NASTY] + + +@pytest.mark.parametrize("doc", DOCUMENTS, ids=range(len(DOCUMENTS))) +def test_the_parser_agrees_with_a_real_json_implementation(doc): + encoded = json.dumps(doc, ensure_ascii=False) + got = [(p, v) for _idx, p, v in flat(encoded)] + assert got == reference_flatten(doc) + + +@pytest.mark.parametrize("doc", DOCUMENTS, ids=range(len(DOCUMENTS))) +def test_the_parser_reads_a_pretty_printed_document_the_same_way(doc): + """Whole mode accumulates until the value is complete, so a document spread + over many lines reads identically to the compact form. Claude Code writes + compact events today; betting on that is how a reader breaks quietly.""" + compact = [(p, v) for _i, p, v in flat(json.dumps(doc, ensure_ascii=False))] + pretty = [(p, v) for _i, p, v in flat(json.dumps(doc, indent=2, ensure_ascii=False))] + assert compact == pretty + + +@pytest.mark.parametrize("value", NASTY) +def test_a_value_survives_the_round_trip_the_hooks_actually_make(value): + """Parse, pick, decode — what every hook does to read one field. The 3000 + character entry matters most: it is longer than the parser's token window, + so it takes the wide-read fallback rather than the fast path.""" + doc = json.dumps({"tool_input": {"content": value}}, ensure_ascii=False) + got = sh('flat=$(scribe_json_flat); scribe_json_pick "$flat" \'.tool_input.content\'', + stdin=doc) + assert got.rstrip("\n") == value.rstrip("\n") + + +# -------------------------------------------------------------------------- +# Reading: the shell side. + +def test_pick_list_and_len_read_what_the_server_sends(): + body = json.dumps({"context": "some markdown", "note_ids": [11, 22, 33], + "sync_note_ids": [22], "derive_keys": ["k1", "k2"], + "rule_ids": [], "project": {"id": 7}}) + out = sh(r""" + flat=$(scribe_json_flat) + echo "ctx=$(scribe_json_pick "$flat" '.context')" + echo "pid=$(scribe_json_pick "$flat" '.project.id')" + echo "notes=$(scribe_json_list "$flat" '.note_ids' | tr '\n' ',')" + echo "keys=$(scribe_json_list "$flat" '.derive_keys' | tr '\n' ',')" + echo "n=$(scribe_json_len "$flat" '.note_ids')" + echo "rules_n=$(scribe_json_len "$flat" '.rule_ids')" + echo "minus=$(scribe_json_list_minus "$flat" '.note_ids' '.sync_note_ids' | tr '\n' ',')" + """, stdin=body) + got = dict(line.split("=", 1) for line in out.splitlines()) + assert got == {"ctx": "some markdown", "pid": "7", "notes": "11,22,33,", + "keys": "k1,k2,", "n": "3", "rules_n": "0", "minus": "11,33,"} + + +def test_an_absent_field_and_a_null_field_both_read_as_empty(): + """`// empty` is what every call site carried, and the shells downstream + test `[ -n "$x" ]`. A field the server left null and one it never sent mean + the same thing to all of them.""" + out = sh(r""" + flat=$(scribe_json_flat) + printf '[%s][%s][%s]\n' "$(scribe_json_pick "$flat" '.nothing')" \ + "$(scribe_json_pick "$flat" '.nulled')" "$(scribe_json_pick "$flat" '.real')" + """, stdin=json.dumps({"nulled": None, "real": "here"})) + assert out.strip() == "[][][here]" + + +def test_an_absent_array_length_is_empty_not_zero(): + """NOT the same answer as 0, and the distinction is load-bearing: the + record nudge fires when the server said "zero notes" and must not fire + when the server said nothing at all (#2932).""" + out = sh(r"""flat=$(scribe_json_flat) + printf '[%s][%s]\n' "$(scribe_json_len "$flat" '.note_ids')" \ + "$(scribe_json_len "$flat" '.missing')" """, + stdin=json.dumps({"note_ids": []})) + assert out.strip() == "[0][]" + + +def test_a_document_that_does_not_parse_yields_nothing(): + for broken in ['{"a":', '{"a" 1}', "not json at all", '{"a":1}{"b":2}', ""]: + assert flat(broken) == [], broken + + +def test_lines_mode_drops_only_the_record_that_did_not_parse(): + """`tail -n 3000` of a transcript cuts wherever it cuts, so the first line + is routinely half a record. This is the `map(try fromjson catch empty)` + the jq program it replaces opened with.""" + doc = "\n".join(['ent":"truncated"}', + json.dumps({"type": "user", "n": 1}), + "{ also broken", + json.dumps({"type": "assistant", "n": 2})]) + rows = flat(doc, mode="lines") + assert [r for r in rows if r[1] == ".n"] == [("1", ".n", "1"), ("2", ".n", "2")] + + +# -------------------------------------------------------------------------- +# Writing, and encoding. + +@pytest.mark.parametrize("value", NASTY) +def test_the_hook_envelope_is_valid_json_carrying_the_exact_context(value): + """The one JSON document these hooks WRITE. Five hooks emitted it through + `jq -n --arg c`; getting the escaping wrong corrupts every injected context + that contains a Windows path, a regex or a quoted word.""" + out = sh('scribe_json_out PreToolUse "$(cat)"', stdin=value) + parsed = json.loads(out) + assert parsed["hookSpecificOutput"]["hookEventName"] == "PreToolUse" + assert parsed["hookSpecificOutput"]["additionalContext"] == value.rstrip("\n") + + +@pytest.mark.parametrize("value", NASTY + ["a b&c=d", "~!*()", "/path/to?q=1#f", "100%"]) +def test_urlenc_percent_encodes_exactly_the_unreserved_set(value): + """Byte-exact, which is why it goes through `od` rather than an awk + character loop: awk's idea of a character follows the locale, so gawk reads + an accented letter as one and mawk as two, and an encoder built on substr() + would produce a different URL depending on which awk is installed. + Percent-encoding is defined on BYTES.""" + got = sh("scribe_urlenc", stdin=value) + assert got == quote(value, safe="") + + +def test_urlenc_and_the_envelope_can_fail(): + """Rule 167 — a guard that cannot fail is decoration. If the escaper were a + no-op, this pair would be the assertion that noticed.""" + assert sh("scribe_urlenc", stdin="a b") != "a b" + raw = sh('scribe_json_out PreToolUse "$(cat)"', stdin='he said "hi"') + assert '\\"hi\\"' in raw, raw + + +# -------------------------------------------------------------------------- +# The transcript turn, which is the largest thing jq was doing here. + +def _turn(records: list[dict]) -> dict: + _need("awk") + doc = "\n".join(json.dumps(r) for r in records) + "\n" + p1 = subprocess.run(["awk", "-v", "mode=lines", "-f", str(PARSER)], + input=doc, capture_output=True, text=True) + assert p1.returncode == 0, p1.stderr + p2 = subprocess.run(["awk", "-f", str(TURN)], input=p1.stdout, + capture_output=True, text=True) + assert p2.returncode == 0, p2.stderr + out = {} + for line in p2.stdout.splitlines(): + k, _, v = line.partition("\t") + out[k] = v + return out + + +def _prompt(text="do the thing", **kw): + return {"type": "user", "message": {"role": "user", "content": text}, **kw} + + +def _close(tid="t1", task_id=41, status="done", name="mcp__x__update_task", **kw): + return {"type": "assistant", "message": {"content": [ + {"type": "tool_use", "id": tid, "name": name, + "input": {"task_id": task_id, "status": status}}]}, **kw} + + +def _text(body, **kw): + return {"type": "assistant", + "message": {"content": [{"type": "text", "text": body}]}, **kw} + + +def _error(tid="t1"): + return {"type": "user", "message": {"content": [ + {"type": "tool_result", "tool_use_id": tid, "is_error": True}]}} + + +def test_a_window_with_no_prompt_is_not_bounded(): + assert _turn([_close(), _text("done")])["bounded"] == "" + + +def test_the_turn_starts_at_the_last_prompt(): + facts = _turn([_prompt("first"), _close("t1", 11), _prompt("second"), + _close("t2", 22), _text("report")]) + assert facts["bounded"] == "1" + assert facts["closed"] == "1" + assert facts["task_ids"] == "22" + + +def test_a_close_that_errored_is_not_a_close(): + """The tool_result carrying `is_error` arrives in a LATER record than the + tool_use it refutes, so a single forward pass would already have counted + it. Closing a task that failed to write must not count as closing it.""" + assert _turn([_prompt(), _close("t1", 41), _error("t1"), _text("r")])["closed"] == "0" + assert _turn([_prompt(), _close("t1", 41), _error("t9"), _text("r")])["closed"] == "1" + + +def test_a_subagents_work_is_not_this_sessions(): + """Sidechain records interleave into the same file. A subagent closing a + task is not the operator's session closing one, and counting those made the + check fire on turns that closed nothing.""" + assert _turn([_prompt(), _close("t1", 41, isSidechain=True), _text("r")])["closed"] == "0" + side = _turn([_prompt(), _prompt("subagent asked", isSidechain=True), + _close("t1", 41), _text("r")]) + assert side["closed"] == "1", "a sidechain PROMPT must not re-cut the turn" + + +def test_a_meta_record_does_not_start_a_turn(): + facts = _turn([_prompt("real"), _close("t1", 41), + _prompt("injected", isMeta=True), _text("report")]) + assert facts["closed"] == "1" + + +def test_the_reply_is_the_text_after_the_last_action_and_keeps_its_newlines(): + """Text emitted BETWEEN two tool calls is narration mid-work, not a report; + holding it to the report shape would block turns that did report properly + at the end. The reply comes back still escaped and on one line, so the + format survives a multi-line answer.""" + facts = _turn([_prompt(), _text("thinking out loud"), _close("t1", 41), + _text("line one\nline two"), _text("line three")]) + assert facts["reply"] == "line one\\nline two\\nline three" + decoded = sh("scribe_json_unescape", stdin=facts["reply"]) + assert decoded == "line one\nline two\nline three\n" + assert "thinking out loud" not in decoded + + +def test_a_tool_result_does_not_read_as_a_prompt(): + """A prompt's `message.content` is a STRING; a tool result carries an ARRAY + at the same path. Counting one as a prompt would cut the turn in the wrong + place — which is the whole bounding decision.""" + facts = _turn([_prompt("real"), _close("t1", 41), _error("t9"), _text("report")]) + assert facts["closed"] == "1" and facts["task_ids"] == "41" + + +def test_the_turn_guards_can_fail(): + """Rule 167. If the analyzer counted everything, or nothing, these are the + assertions that would notice.""" + assert _turn([_prompt(), _close("t1", 41), _text("r")])["closed"] == "1" + assert _turn([_prompt(), _text("r")])["closed"] == "0" + + +# -------------------------------------------------------------------------- +# And the dependency itself. + +def test_no_hook_reaches_for_jq_or_tac(): + """The point of all of the above. A hook may use only what POSIX + guarantees: jq is not installed by default on macOS, the Debian/Ubuntu slim + images, Alpine or most CI containers, and tac is GNU-only — absent on macOS, + where the prior-art hook's enclosing-definition arm silently did nothing + from the day it shipped. Comment lines are exempt: several hooks now name + the old form so the next reader knows what changed.""" + banned = re.compile(r"(? Path: - for tool in ("bash", "jq"): + for tool in ("bash",): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") env = {"PATH": os.environ["PATH"], "HOME": str(tmp), "TMPDIR": str(tmp)} diff --git a/tests/test_scribe_marker.py b/tests/test_scribe_marker.py index 7af04f3..1c40604 100644 --- a/tests/test_scribe_marker.py +++ b/tests/test_scribe_marker.py @@ -36,7 +36,7 @@ INSTANCE = "https://scribe.example.com" def _sh(script: str, cwd: Path, env_extra: dict | None = None) -> str: - for tool in ("bash", "jq"): + for tool in ("bash",): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") env = {"PATH": os.environ["PATH"], "HOME": str(cwd)} diff --git a/tests/test_session_context_ledger.py b/tests/test_session_context_ledger.py index b0e28b1..6133228 100644 --- a/tests/test_session_context_ledger.py +++ b/tests/test_session_context_ledger.py @@ -51,7 +51,7 @@ def _env(tmp_path): the clear happens with no credentials and no network at all — so sharing a helper would mean testing this case in an environment that cannot show it. """ - for tool in ("jq", "bash"): + for tool in ("bash",): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") # No SCRIBE_URL / SCRIBE_TOKEN on purpose — see the module docstring. diff --git a/tests/test_session_ledger_clear.py b/tests/test_session_ledger_clear.py index 372c77d..b24dd49 100644 --- a/tests/test_session_ledger_clear.py +++ b/tests/test_session_ledger_clear.py @@ -77,7 +77,7 @@ def _swept_dirs() -> set[str]: def _run_session_start(source: str, tmp: Path) -> Path: """Run the SessionStart hook for real, with the ledger directories filled.""" - for tool in ("bash", "jq"): + for tool in ("bash",): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index a6a738a..53d8359 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -1108,7 +1108,7 @@ def _hook_runtime_env(): import os import shutil - for tool in ("git", "jq", "curl", "bash"): + for tool in ("git", "curl", "bash"): if shutil.which(tool) is None: pytest.skip(f"hook runtime tool {tool!r} not installed") return {"PATH": os.environ["PATH"], From 4bfa0cbbc046552e6e211c6911cb3341d8a7c1f6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 20 Sep 2026 12:23:42 -0400 Subject: [PATCH 2/4] fix(plugin): stop CI installing the jq the hooks no longer use (#4107) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three loose ends from a49e7ed, all found by CI or by re-reading it. TWO TESTS PINNED THE jq SYNTAX, not the word `jq`, so grepping the suite for the tool name missed them: test_write_path_trigger asserted the literal filter strings `(.note_ids // []) - (.sync_note_ids // [])` and `(.rule_ids // [])[]?` appear in the hook. Re-spelled with the readers that replaced them. The claim each makes is unchanged — that the reuse and sync channels keep their own state file, their own query parameter and their own write-back. CI STILL INSTALLED jq, TWICE. The "Install jq for hook execution tests" step existed because those tests skipped rather than failed without it; the Plugin hooks job installed it alongside shellcheck for the same reason. Removing a dependency from the product and leaving CI to install it anyway means the smoke tests never run under the condition they exist to assert. Both gone — the hook tests now run on a bare image. `awk 'END { print $0 }'` IS UNDEFINED IN POSIX. gawk retains the last record in END and mawk does too, but it is not guaranteed, and reaching for a non-portable idiom inside the change whose entire purpose is portability is the same mistake one layer down. Now `{ last = $0 } END { if (NR) print last }`. Verified it still selects the same start line on a real transcript. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- .forgejo/workflows/ci.yml | 27 +++++++++++++-------------- plugin/.claude-plugin/plugin.json | 2 +- plugin/hooks/scribe_report_check.sh | 2 +- tests/test_write_path_trigger.py | 7 ++++--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/.forgejo/workflows/ci.yml b/.forgejo/workflows/ci.yml index 3d11996..d25cdc0 100644 --- a/.forgejo/workflows/ci.yml +++ b/.forgejo/workflows/ci.yml @@ -111,7 +111,7 @@ jobs: # Guards the one part of this repo that ships to users without a build step. # See scripts/check_plugin.py for what it checks and, as importantly, what it - # can't check yet (shellcheck and jq are absent from ci-python). + # can't check yet (shellcheck is absent from ci-python). plugin: name: Plugin hooks if: github.ref == 'refs/heads/dev' || github.ref == 'refs/heads/main' || startsWith(github.ref, 'refs/tags/v') @@ -133,12 +133,15 @@ jobs: # the only consumer today. Promotion into ci-python is filed as an issue # on CI-runner rather than assumed here. # - # jq is not optional for the smoke test: every hook exits at line 1 - # without it, so the check would pass while exercising nothing. + # jq WAS installed here too, because every hook exited at line 1 without + # it and the smoke test would have passed while exercising nothing. The + # hooks need no jq since #4107 — they use only what POSIX guarantees — so + # the smoke test now runs on a bare image, which is the condition it is + # actually meant to be asserting. - name: Install shell tooling run: | apt-get update -qq - apt-get install -y -qq --no-install-recommends jq shellcheck + apt-get install -y -qq --no-install-recommends shellcheck # On main the comparison would be against itself, so only the syntax and # pattern checks mean anything there. @@ -223,16 +226,12 @@ jobs: UV_PROJECT_ENVIRONMENT: /opt/venv run: uv sync --locked --extra dev - # The hook-EXECUTION tests (test_write_path_trigger's nudge pair) run the - # real bash hook, which exits silently without jq — and those tests skip - # rather than fail when it's absent, so without this step they would - # quietly never be verified anywhere (ci-python ships without jq; same - # install the Plugin hooks job does). - - name: Install jq for hook execution tests - run: | - apt-get update -qq - apt-get install -y -qq --no-install-recommends jq - + # An "Install jq for hook execution tests" step stood here. The hook + # EXECUTION tests run the real bash hook, which exited silently without + # jq and skipped rather than failed, so the step existed to stop them + # being silently unverified. Since #4107 the hooks need no jq, so the + # step is gone and those tests run against a bare image — and the point + # of removing a dependency is lost if CI keeps installing it anyway. - name: Run tests # Integration tests (real Postgres) run in the `integration` job below. run: /opt/venv/bin/python -m pytest tests/ -q -m "not integration" diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 15ac240..07268f1 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.20.1620", + "version": "2026.09.20.1623", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_report_check.sh b/plugin/hooks/scribe_report_check.sh index d8311f4..90ce991 100644 --- a/plugin/hooks/scribe_report_check.sh +++ b/plugin/hooks/scribe_report_check.sh @@ -119,7 +119,7 @@ turn_facts() { window | tail -n +"${1:-1}" | scribe_json_flat_lines \ # slow way — correct, and slow, which is the right way round for a check that # can block a stop. start=$(window | grep -n -F '"message":{"role":"user","content":"' 2>/dev/null \ - | cut -d: -f1 | awk 'END { if (NR) print $0 }') + | cut -d: -f1 | awk '{ last = $0 } END { if (NR) print last }') case "$start" in ''|*[!0-9]*) start=1 ;; esac facts=$(turn_facts "$start") diff --git a/tests/test_write_path_trigger.py b/tests/test_write_path_trigger.py index 53d8359..e8d488a 100644 --- a/tests/test_write_path_trigger.py +++ b/tests/test_write_path_trigger.py @@ -1091,8 +1091,9 @@ def test_hook_keeps_sync_and_reuse_dedup_apart(): assert ".sync.ids" in src # its own state file assert "exclude_sync_ids=" in src # its own query channel # The reuse file must NOT swallow sync ids — the write-back subtracts them. - assert "(.note_ids // []) - (.sync_note_ids // [])" in src - assert "(.sync_note_ids // [])[]?" in src + # Spelled with the readers that replaced jq (#4107); the claim is unchanged. + assert "scribe_json_list_minus \"$body_flat\" '.note_ids' '.sync_note_ids'" in src + assert "scribe_json_list \"$body_flat\" '.sync_note_ids'" in src def _hook_runtime_env(): @@ -1668,7 +1669,7 @@ def test_hook_keeps_the_rule_channel_apart_from_the_other_three(): src = HOOK.read_text() assert ".rules.ids" in src # its own state file assert "exclude_rule_ids=" in src # its own query channel - assert "(.rule_ids // [])[]?" in src # its own write-back + assert "scribe_json_list \"$body_flat\" '.rule_ids'" in src # its own write-back # And it rides the same request as the rest, not a second round trip. assert "${rule_exclude_q}" in src From 7e653e16dc437e3b5b945227ca3268a36861f405 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 20 Sep 2026 12:24:37 -0400 Subject: [PATCH 3/4] docs(ci): jq is not a CI requirement and should not be promoted (#4107) ci-requirements.md is the document that drives promoting a per-job dep into the ci-python image, and it still recorded jq as installed in the plugin job and "load-bearing for the smoke test specifically", on the grounds that every hook opened `command -v jq || exit 0` and the test would otherwise pass while exercising nothing. That was the tail wagging the dog. The hooks ship to users; jq is absent by default on macOS, the Debian/Ubuntu slim images, Alpine and most CI containers, and a machine without it got no context, no rules, no prior art and no process sync in silence. The answer was to remove the dependency, not to install it harder. Recorded as a promotion the entry now argues AGAINST rather than deleted: the next person to read this file should find out why jq is absent, not merely that it is. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- ci-requirements.md | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/ci-requirements.md b/ci-requirements.md index ee09c7e..41972e9 100644 --- a/ci-requirements.md +++ b/ci-requirements.md @@ -29,15 +29,25 @@ real Postgres), build (docker buildx). Anything CI installs at job time that isn't in the image. Promotion candidates if more than one project needs them. -- `jq` + `shellcheck` — apt-installed in the **plugin** job, which lints - the four Claude Code hook scripts and runs their fail-open smoke test. - Per `docs/process.md`'s decision checkpoint, single-consumer deps stay +- `shellcheck` — apt-installed in the **plugin** job, which lints the + Claude Code hook scripts and runs their fail-open smoke test. Per + `docs/process.md`'s decision checkpoint, single-consumer deps stay per-job until a second consumer wants them; Scribe is the only one so - far. Both are small (jq ~1 MB, shellcheck ~20 MB) and would be - promotion candidates the moment another project lints shell. - **jq is load-bearing for the smoke test specifically**: every hook - starts with `command -v jq || exit 0`, so without it the test passes - while exercising nothing. + far. It is small (~20 MB) and would be a promotion candidate the moment + another project lints shell. + +- `jq` — **no longer installed anywhere, and should not be promoted.** + It was installed in two jobs, and this file used to record it as "load- + bearing for the smoke test specifically", because every hook opened + `command -v jq || exit 0` and the test would otherwise pass while + exercising nothing. That was the tail wagging the dog: the hooks ship to + users, jq is absent by default on macOS, the Debian/Ubuntu slim images, + Alpine and most CI containers, and a machine without it got no context, + no rules, no prior art and no process sync in silence. #4107 removed the + dependency rather than documenting it, so the smoke test now runs on a + bare image — which is the condition it was always meant to assert. The + hooks use only POSIX tools (awk, sed, tr, od, cut, head, tail, grep, + sort, date, printf) plus `git` and `curl`. ## Notes From 1f7ff7b215cac0a768809ca174fe30023135dd29 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 20 Sep 2026 18:45:13 -0400 Subject: [PATCH 4/4] fix(plugin): the prompt boundary retrieves against prompts, not plumbing (#4200) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Claude Code submits more than typed words through UserPromptSubmit. A task notification, a slash-command echo and the caveat banner a local command prints all arrive as user turns, reaching `.prompt` indistinguishable from something the operator wrote. scribe_autoinject.sh believed all of them. The cost that matters is not the wasted embedding — it is the log row. Every such call counts in the denominator of every prompt-boundary surface, so delivery rate reads low for a reason unrelated to retrieval; and each refusal lands in `near_misses`, where a later tuning decision reads it as demand. Measured while taking milestone 399's acceptance (#3898): 15 of the top 20 `preference_slot` near-misses were `` blocks, all matching ONE record — #140 "Let each action land before starting the next" — all within thousandths of the 0.70 floor. A notification that an action finished really does resemble a preference about letting actions land. Lowering the floor to serve that apparent demand would have injected that record into every notification: the instrument arguing for the wrong fix, which is #379 again. scribe_skip_prompt is a PREFIX test, not a substring one, and that is the whole safety argument. A real prompt may contain one of these tags — an operator pasting a transcript, or a `` after typed words — and must still be retrieved against. Nothing an operator types begins with a client envelope. The compaction-resume injection is deliberately NOT filtered: it is machine-written, but it summarises real work, and a resumed session is where recalling a rule earns its keep. Client-side only, so no server contract moves and lagging plugin caches keep working. The tags are Claude Code protocol constructs, identical on every install — instance-agnostic under rule 115. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- plugin/.claude-plugin/plugin.json | 2 +- plugin/hooks/scribe_autoinject.sh | 8 ++ plugin/hooks/scribe_defs.sh | 43 ++++++++++ tests/test_prompt_boundary_filter.py | 117 +++++++++++++++++++++++++++ 4 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 tests/test_prompt_boundary_filter.py diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 07268f1..f6bc7e8 100644 --- a/plugin/.claude-plugin/plugin.json +++ b/plugin/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "scribe", "description": "Scribe for Claude Code: connects the scribe MCP server, adds the hooks that deliver live project state and relevant records at the right moment, ships the shared client-neutral Scribe skills (using-scribe, writing-plans, reporting-back, systematic-debugging, verification, brainstorming, reusing-code, shape-accounting), and syncs your saved Scribe Processes as skills (/scribe:sync).", - "version": "2026.09.20.1623", + "version": "2026.09.20.2244", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_autoinject.sh b/plugin/hooks/scribe_autoinject.sh index e960efe..48193a9 100755 --- a/plugin/hooks/scribe_autoinject.sh +++ b/plugin/hooks/scribe_autoinject.sh @@ -53,6 +53,14 @@ event_cwd=$(scribe_json_pick "$event_flat" '.cwd') # Nothing to retrieve against. [ -n "$prompt" ] || exit 0 +# A turn CLAUDE CODE wrote, not the operator (#4200) — a task notification, a +# slash-command echo, a local command's caveat banner. Leaving before the +# request matters more for the LOG than for the noise: a retrieval call +# recorded against a notification inflates this surface's denominator and +# seeds `near_misses` with demand nobody expressed. scribe_skip_prompt carries +# the measurement. +scribe_skip_prompt "$prompt" && exit 0 + # Unconfigured install → silent (auto-inject is pure enrichment). scribe_config || exit 0 diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 33b950f..7d1b99d 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -8,6 +8,8 @@ # same three things, kept here so they cannot drift apart: # # scribe_skip_path PATH formats that hold prose or data, not shapes +# scribe_skip_prompt TEXT the turn was written by the CLIENT, not the +# operator — skip it entirely (#4200) # scribe_defs stdin code → "kindname" per definition # scribe_local_dups ROOT REL "kindname" lines on stdin → the by-name # local-duplicate lines (ARM 1, #2280) @@ -43,6 +45,47 @@ scribe_skip_path() { return 1 } +# Skip a turn the CLIENT wrote rather than the operator (#4200). Claude Code +# submits several kinds of machine-written text through UserPromptSubmit — a +# task notification, the echo of a slash command, the caveat banner a local +# command prints — and they reach `.prompt` indistinguishable from typed words +# unless something looks. +# +# THIS IS A TELEMETRY FIX FIRST AND A NOISE FIX SECOND, which is the part +# worth keeping straight: retrieving against a notification usually returns +# nothing, so the visible cost looks like one wasted embedding. The real cost +# is that the call is LOGGED. It inflates the denominator of every +# prompt-boundary surface, so delivery rate reads low for a reason that has +# nothing to do with retrieval; and the refusal lands in `near_misses`, where +# a later tuning decision reads it as unmet demand. Measured on #3898: 15 of +# the top 20 preference near-misses were `` blocks, ALL +# matching one record, ALL within thousandths of the floor. A floor lowered to +# serve that apparent demand would inject that record into every notification +# — the instrument arguing for the wrong fix, which is #379's lesson again. +# +# A PREFIX TEST, NOT A SUBSTRING ONE, and that distinction is the whole safety +# argument. A real prompt may well CONTAIN one of these tags — an operator +# pasting a transcript, or a `` appended after typed words — +# and must still be retrieved against. Nothing an operator types BEGINS with a +# client envelope. +# +# Leading whitespace goes first: the tag is what identifies the turn, and one +# stray newline ahead of it would otherwise walk past the entire filter. +# +# NOT LISTED, ON PURPOSE: the compaction-resume injection. It is machine +# written too, but it is a summary of real work rather than plumbing, and a +# resumed session is exactly where recalling a rule earns its keep. +scribe_skip_prompt() { + _scribe_prompt=${1#"${1%%[![:space:]]*}"} + case "$_scribe_prompt" in + ""*|\ + ""*|""*|""*|\ + ""*|""*|""*) + return 0 ;; + esac + return 1 +} + # --------------------------------------------------------------------------- # JSON AND URL-ENCODING, WITHOUT jq (#4107). # diff --git a/tests/test_prompt_boundary_filter.py b/tests/test_prompt_boundary_filter.py new file mode 100644 index 0000000..bd44a40 --- /dev/null +++ b/tests/test_prompt_boundary_filter.py @@ -0,0 +1,117 @@ +"""The prompt boundary retrieves against PROMPTS, not against plumbing (#4200). + +Claude Code submits more than typed words through `UserPromptSubmit`. A task +notification, the echo of a slash command and the caveat banner a local command +prints all arrive as user turns, and all reach `.prompt` looking exactly like +something the operator wrote. `scribe_autoinject.sh` believed them. + +WHY THIS IS TESTED AT ALL, given that retrieving against a notification almost +never returns anything. The cost that matters is not the wasted embedding, it +is the LOG ROW. Every one of those calls counts in the denominator of every +prompt-boundary surface, so delivery rate reads low for a reason unrelated to +retrieval quality; and each refusal lands in `near_misses`, where a later +tuning decision reads it as demand. Measured on #3898: 15 of the top 20 +preference near-misses were `` blocks, all matching one +record, all within thousandths of the floor. A floor lowered to serve that +apparent demand would have injected that record into every notification — an +instrument arguing for the wrong fix, which is exactly #379. + +So the assertions split in two, and the SECOND half is the one guarding +against the obvious wrong implementation: + + * the client's envelopes are skipped; + * a real prompt that merely CONTAINS one of those tags is NOT — because the + cheap version of this filter is a substring search, and a substring search + silences an operator who pastes a transcript or asks a question about a + tag by name. The filter reads the FIRST token only. + +The corpus below is drawn from real transcript shapes rather than invented, +which is the reason the whitespace and trailing-`` cases are +here: both occur. +""" +from __future__ import annotations + +import os +import shutil +import subprocess +from pathlib import Path + +import pytest + +ROOT = Path(__file__).resolve().parents[1] +DEFS = ROOT / "plugin" / "hooks" / "scribe_defs.sh" +HOOK = ROOT / "plugin" / "hooks" / "scribe_autoinject.sh" + +# Written by the client. None of these is a question anyone asked. +SYNTHETIC = [ + "\nb1oojuqu3\n/tmp/x", + "/compact", + "compact", + "focus on the hooks", + "Reconnected to plugin:scribe", + "Failed to reconnect", + "Caveat: The messages below were generated…", + " \n\nabc", # whitespace must not walk the filter +] + +# Typed by a person. Every one of these must still be retrieved against. +REAL = [ + "please merge to main", + "awesome go for 3898", + "why does show up in the telemetry?", # names the tag + "fix the hookinjected", # tag, but trailing + "This session is being continued from a previous conversation.", # see module docstring + " is rendering wrong", # a tag, but not one of the client's + "here is the transcript you asked for:\nhi", +] + + +def _skip(prompt: str) -> bool: + """Run the real predicate in the real shell — not a reimplementation.""" + if shutil.which("bash") is None: # pragma: no cover + pytest.skip("hook runtime tool 'bash' not installed") + script = f'set -uo pipefail\n. "{DEFS}"\nscribe_skip_prompt "$1" && echo SKIP || echo KEEP\n' + out = subprocess.run( + ["bash", "-c", script, "_", prompt], + capture_output=True, text=True, timeout=30, + env={"PATH": os.environ["PATH"], "HOME": os.environ.get("HOME", "/tmp")}, + ) + assert out.returncode == 0, out.stderr + verdict = out.stdout.strip() + assert verdict in {"SKIP", "KEEP"}, out.stdout + return verdict == "SKIP" + + +@pytest.mark.parametrize("prompt", SYNTHETIC, ids=lambda p: p[:28]) +def test_client_written_turns_are_skipped(prompt: str) -> None: + assert _skip(prompt), f"retrieval would fire on a machine-written turn: {prompt[:60]!r}" + + +@pytest.mark.parametrize("prompt", REAL, ids=lambda p: p[:28]) +def test_operator_prompts_survive(prompt: str) -> None: + assert not _skip(prompt), f"a real prompt was silenced: {prompt[:60]!r}" + + +def test_the_filter_is_a_prefix_test_not_a_substring_test() -> None: + """The distinction the whole design rests on, asserted as one fact. + + Stated separately from the parametrised cases because a future rewrite + that reaches for `grep -q` or `case *""*` passes nothing here, and + the parametrised failure would read as "one odd input" rather than "the + implementation changed shape". + """ + tag = "" + assert _skip(tag + " trailing") + assert not _skip("what is " + tag + "?") + + +def test_the_hook_consults_the_filter_before_spending_a_request() -> None: + """Placement, not merely presence. + + The guard is worthless below the `curl`: the point is that no row is + logged, so it has to sit ahead of the request that would log one. + """ + body = HOOK.read_text() + assert "scribe_skip_prompt" in body, "the hook does not consult the filter" + assert body.index("scribe_skip_prompt") < body.index("curl -fsS"), \ + "the filter runs after the request it exists to prevent"