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/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 diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 62a9f31..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.0317", + "version": "2026.09.20.2244", "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..48193a9 100755 --- a/plugin/hooks/scribe_autoinject.sh +++ b/plugin/hooks/scribe_autoinject.sh @@ -38,18 +38,29 @@ 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 +# 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 @@ -59,11 +70,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 +123,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..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) @@ -19,6 +21,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 +45,257 @@ 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). +# +# 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 +538,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 +682,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 +726,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..90ce991 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 '{ last = $0 } END { if (NR) print last }') +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"(?` 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" diff --git a/tests/test_report_check_hook.py b/tests/test_report_check_hook.py index d401d0f..293ac98 100644 --- a/tests/test_report_check_hook.py +++ b/tests/test_report_check_hook.py @@ -28,7 +28,7 @@ REASON = "SERVER REASON: rewrite as a completion report" def _env(tmp_path, url="http://127.0.0.1:9"): - for tool in ("jq", "curl", "bash"): + for tool in ("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_rule_opened_ledger.py b/tests/test_rule_opened_ledger.py index 72e23c8..1d33bab 100644 --- a/tests/test_rule_opened_ledger.py +++ b/tests/test_rule_opened_ledger.py @@ -92,7 +92,7 @@ def test_held_outranks_seen_regardless_of_kind(): # ── the recorder ─────────────────────────────────────────────────────────── def _run_recorder(event: dict, tmp: Path) -> 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..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(): @@ -1108,7 +1109,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"], @@ -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