From aa94c73d9e61133550e1edd425ce9f1dd21f780f Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 16 Sep 2026 08:32:29 -0400 Subject: [PATCH] feat(plugin): a directory says which project it belongs to, git repo or not (#4085) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All six hooks scoped their requests one way: `git remote get-url origin`, resolved server-side through the repo bindings. That key does not exist outside a git repo, so a session in a plain directory was unscoped in every hook at once — no project context, no prior-art scoping, no project rules — and silently, because a missing remote is indistinguishable from a remote nobody bound. A `.scribe` file is the second key, read by the shared scribe_scope_query so a directory scopes the same way everywhere: {"instance": "https://scribe.example.com", "project_id": 2, "project": "…"} `instance` is why the file is not just a number: an id is a different project on every Scribe, so a marker that travels — a copied directory, a shared machine, a repo someone else clones — would otherwise scope the session to the wrong project without a word. Compared host-only, and a mismatch drops the id: no project beats the wrong project. A bare integer is accepted too, since it is what a person writes by hand. The marker beats a git remote — someone put the file there on purpose — which is also how a directory overrides its binding. Two things it found on the way: * An explicit project_id that did not resolve rendered NO message at all — the branch hung off `if project_id` as an `elif`, so a caller holding a pointer it believed in got a context that silently omitted the project it had asked for. Now reported. * The refusal reason was a global set inside a function every caller reads through `$( )`. The assignment died with the subshell, leaving the caller to read an unset variable under `set -u` — which aborts the hook and costs the whole session's SessionStart context, to fetch a warning about a file. It comes back through stdout with the id instead, and a test pins it. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01821k5B3Ysecp9fNYs92Kuy --- plugin/.claude-plugin/plugin.json | 2 +- plugin/hooks/scribe_after_write.sh | 7 +- plugin/hooks/scribe_autoinject.sh | 9 +- plugin/hooks/scribe_defs.sh | 126 +++++++++++++++++++++ plugin/hooks/scribe_prior_art.sh | 9 +- plugin/hooks/scribe_report_check.sh | 7 +- plugin/hooks/scribe_session_context.sh | 38 ++++++- plugin/hooks/scribe_tool_rules.sh | 7 +- plugin/skills/using-scribe/SKILL.md | 21 +++- src/scribe/routes/plugin.py | 7 +- src/scribe/services/plugin_context.py | 45 +++++--- tests/test_scribe_marker.py | 150 +++++++++++++++++++++++++ tests/test_services_plugin_context.py | 25 +++++ 13 files changed, 401 insertions(+), 52 deletions(-) create mode 100644 tests/test_scribe_marker.py diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index b28ac5d..2b8533f 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.16.1211", + "version": "2026.09.16.1232", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_after_write.sh b/plugin/hooks/scribe_after_write.sh index 977c7fb..1510983 100644 --- a/plugin/hooks/scribe_after_write.sh +++ b/plugin/hooks/scribe_after_write.sh @@ -105,12 +105,9 @@ done <<< "$current" [ -n "$changed" ] || exit 0 scribe_config || : # sets url/token; the call below is guarded on them -repo=$(git -C "$repo_root" remote get-url origin 2>/dev/null || true) +scope=$(scribe_scope_query "$repo_root") repo_q="" -if [ -n "$repo" ]; then - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" - [ -n "$enc" ] && repo_q="&repo=${enc}" -fi +[ -n "$scope" ] && repo_q="&${scope}" # The dedup channels are the PRE-write hook's files, on purpose (see header). state_dir="${TMPDIR:-/tmp}/scribe-priorart" diff --git a/plugin/hooks/scribe_autoinject.sh b/plugin/hooks/scribe_autoinject.sh index c32d324..956d8a2 100755 --- a/plugin/hooks/scribe_autoinject.sh +++ b/plugin/hooks/scribe_autoinject.sh @@ -65,14 +65,11 @@ q=$(printf '%s' "$prompt" | head -c 2000) # worth retrieving against were exactly the ones silently dropped. -s slurps. q_enc=$(printf '%s' "$q" | jq -sRr '@uri' 2>/dev/null) || exit 0 -# Resolve the working repo's remote so the server can scope to the bound project. +# Scope to this directory's project — a `.scribe` marker, else the git remote. repo_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} -repo=$(git -C "$repo_dir" remote get-url origin 2>/dev/null || true) +scope=$(scribe_scope_query "$repo_dir") repo_q="" -if [ -n "$repo" ]; then - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" - [ -n "$enc" ] && repo_q="&repo=${enc}" -fi +[ -n "$scope" ] && repo_q="&${scope}" # Per-session dedup: ids already injected this session are skipped. state_dir="${TMPDIR:-/tmp}/scribe-autoinject" diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 86ea6bb..4d75438 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -19,6 +19,11 @@ # 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_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, +# scribe_marker_read (idwhy-not), +# scribe_marker_project (the id alone) # # Sourced, not executed: `. "$(dirname "${BASH_SOURCE[0]}")/scribe_defs.sh"`. @@ -283,3 +288,124 @@ scribe_rules_append() { now=$(date +%s 2>/dev/null) || now=0 awk -v ts="$now" 'NF { print $1 "\t" ts }' >> "$f" 2>/dev/null || true } + +# --------------------------------------------------------------------------- +# WHICH PROJECT IS THIS DIRECTORY'S? (#4085) +# +# Every hook here scopes its request to a project, and until this existed all +# six did it the same single way: `git remote get-url origin`, resolved +# server-side through the repo bindings. That works well inside a bound repo +# and not at all outside one — a session in a plain directory got no project +# scope from ANY hook, silently, because the one key the whole chain turns on +# only exists in a git repo. +# +# The second key is a `.scribe` file in the directory (or above it, the way +# git finds its root), naming the project the work belongs to. It is a +# POINTER, not a copy: an id and enough to check the id means what it says. +# Nothing Scribe should be holding goes in it. +# +# {"instance": "https://scribe.example.com", "project_id": 2, +# "project": "FabledScribe"} +# +# instance WHICH Scribe the id belongs to, and the reason this file is +# not just a number. A project id means nothing on its own: id 2 +# is a different project on every instance, so a marker that +# travels — a copied directory, a shared machine, a repo someone +# else clones — would silently scope a session to the wrong +# project. Compared HOST-ONLY against the configured endpoint, so +# http/https and a trailing slash don't cause a false mismatch. +# A mismatch drops the id: no project beats the wrong project. +# project_id the pointer itself. Required. +# project a human label. NOTHING READS IT. It is there so the file +# answers "what is this?" when opened, and so a stale one is +# visible rather than inert. +# +# A bare integer is also accepted (`echo 2 > .scribe`), because it is what a +# person writes by hand and it parses as JSON already. It skips the instance +# check by having nothing to check — deliberate, and the reason the written +# form carries `instance`. +# +# THE MARKER WINS over a git remote. Someone put the file there on purpose; +# a remote is just where the code happens to be pushed. That also makes the +# marker the way to override a binding for one directory. + +# Nearest `.scribe` at or above DIR. Walks up to the filesystem root, capped so +# a pathological path cannot spin. +scribe_marker_file() { + local dir="$1" depth=0 + [ -n "$dir" ] || return 0 + while [ "$depth" -lt 40 ]; do + [ -f "$dir/.scribe" ] && { printf '%s' "$dir/.scribe"; return 0; } + case "$dir" in ""|"/") return 0 ;; esac + dir=$(dirname -- "$dir" 2>/dev/null) || return 0 + depth=$((depth + 1)) + done + return 0 +} + +# Host of a URL, lowercased, port kept. "" for empty input. +scribe_url_host() { + printf '%s' "${1:-}" \ + | sed -e 's#^[A-Za-z][A-Za-z0-9+.-]*://##' -e 's#^[^/@]*@##' -e 's#[/?].*$##' \ + | tr 'A-Z' 'a-z' +} + +# Read a marker file: prints "IDREASON", at most one of them non-empty. +# +# "7\t" use project 7 +# "\tnames no …" a file is there and deliberately NOT used; say why +# "\t" no marker file at all — the ordinary case, say nothing +# +# One line rather than an id plus a global, because every caller reads this +# through `$( )` and a global set inside a command substitution dies with the +# subshell. The caller would then read an unset variable, which under the +# `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 + [ -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="" + 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="" + if [ -n "$inst" ]; then + want=$(scribe_url_host "${url:-}") + inst=$(scribe_url_host "$inst") + if [ -n "$want" ] && [ "$inst" != "$want" ]; then + printf '\tpoints at %s, but this session is configured for %s' "$inst" "$want" + return 0 + fi + fi + printf '%s\t' "$id" +} + +# Just the id a marker names, or "" — the half scribe_scope_query needs. +scribe_marker_project() { + scribe_marker_read "$1" | cut -f1 +} + +# The query args identifying DIR's project — `project_id=N` or `repo=` — +# with NO leading `?` or `&`, so each caller keeps its own separator. Empty +# when neither key is available. Requires `url` to be set (scribe_config) for +# the marker's instance check; without it a marker is still honoured, since an +# unconfigured hook is not going to send the request anyway. +scribe_scope_query() { + local dir="$1" id repo enc + id=$(scribe_marker_project "$(scribe_marker_file "$dir")") + if [ -n "$id" ]; then + printf 'project_id=%s' "$id" + return 0 + 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="" + [ -n "$enc" ] && printf 'repo=%s' "$enc" +} diff --git a/plugin/hooks/scribe_prior_art.sh b/plugin/hooks/scribe_prior_art.sh index d0dce6f..db2081f 100755 --- a/plugin/hooks/scribe_prior_art.sh +++ b/plugin/hooks/scribe_prior_art.sh @@ -150,13 +150,10 @@ q=$(printf '%s' "$code" | head -c 1200) 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="" -# Resolve the working repo's remote so the server can scope to the bound project. -repo=$(git -C "$lookup_dir" remote get-url origin 2>/dev/null || true) +# Scope to this directory's project — a `.scribe` marker, else the git remote. +scope=$(scribe_scope_query "$lookup_dir") repo_q="" -if [ -n "$repo" ]; then - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" - [ -n "$enc" ] && repo_q="&repo=${enc}" -fi +[ -n "$scope" ] && repo_q="&${scope}" # Per-session dedup, in its own file rather than sharing auto-inject's. Each # surface shows a given snippet at most once per session, but they don't silence diff --git a/plugin/hooks/scribe_report_check.sh b/plugin/hooks/scribe_report_check.sh index 54cf313..1b79045 100644 --- a/plugin/hooks/scribe_report_check.sh +++ b/plugin/hooks/scribe_report_check.sh @@ -149,11 +149,8 @@ report() { enc=$(printf '%s' "$m" | jq -sRr '@uri' 2>/dev/null) || enc="" q="${q}&missing=${enc}" fi - repo=$(git -C "${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}}" remote get-url origin 2>/dev/null || true) - if [ -n "$repo" ]; then - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" - [ -n "$enc" ] && q="${q}&repo=${enc}" - fi + scope=$(scribe_scope_query "${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}}") + [ -n "$scope" ] && q="${q}&${scope}" curl -fsS --max-time 4 \ -H "Authorization: Bearer ${token}" \ "${url%/}/api/plugin/report-check?${q}" 2>/dev/null diff --git a/plugin/hooks/scribe_session_context.sh b/plugin/hooks/scribe_session_context.sh index 9d6b8be..85aa216 100755 --- a/plugin/hooks/scribe_session_context.sh +++ b/plugin/hooks/scribe_session_context.sh @@ -151,14 +151,19 @@ scribe_config || : dyn="" status="" if [ -n "$url" ] && [ -n "$token" ] && command -v curl >/dev/null 2>&1; then - # Resolve the working repo's remote so the server can map it to a project. + # Which project is this directory's? A `.scribe` marker first, then the git + # remote (#4085). scribe_scope_query answers that, and is shared with the + # other five hooks so a directory scopes the same way everywhere; the pieces + # are re-read here only to explain what happened when nothing resolved. repo_dir=${CLAUDE_PROJECT_DIR:-$PWD} + marker=$(scribe_marker_file "$repo_dir") + marker_read=$(scribe_marker_read "$marker") + marker_id=${marker_read%%$'\t'*} + marker_why=${marker_read#*$'\t'} repo=$(git -C "$repo_dir" remote get-url origin 2>/dev/null || true) + scope=$(scribe_scope_query "$repo_dir") q="" - if [ -n "$repo" ]; then - enc=$(printf '%s' "$repo" | jq -sRr '@uri' 2>/dev/null) || enc="" - [ -n "$enc" ] && q="?repo=${enc}" - fi + [ -n "$scope" ] && q="?${scope}" body=$(curl -fsS --max-time 8 \ -H "Authorization: Bearer ${token}" \ "${url%/}/api/plugin/context${q}" 2>/dev/null) || body="" @@ -184,6 +189,29 @@ fi [ -n "$dyn" ] && append "$dyn" [ -n "$status" ] && append "$status" +# --- Nothing resolved: say WHICH nothing, and what would fix it (#4085) --- +# +# The server can say "no project is bound to this working directory", and +# until now that was the whole answer. It is the same sentence for a directory +# that is not a repo, a repo whose remote nobody bound, and a marker file +# naming a project this account cannot read — three different problems with +# three different fixes, and no way to tell them apart from inside the session. +# +# The marker is the adapter's convention, so the adapter explains it: the +# 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 + 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." + elif [ -n "$marker_id" ]; then + append "> ⚠️ Scribe: \`${marker}\` names project ${marker_id}, which this account cannot read on ${host} — it may belong to a different Scribe instance, or the project may have been deleted. Check with \`list_projects()\` and correct the file." + elif [ -z "$repo" ]; then + append "> ℹ️ Scribe: this directory is not a git repository and has no \`.scribe\` marker, so no project context was loaded — every hook this session is unscoped. If this work belongs to a Scribe project, call \`list_projects()\` and write the marker: \`{\"instance\": \"${url%/}\", \"project_id\": , \"project\": \"\"}\` in \`${repo_dir}/.scribe\`. Future sessions here load that project on their own." + fi +fi + # Compaction re-grounding: lead with a reload banner when this fire is a compact. if [ "$source" = "compact" ]; then prepend "> ⟳ This session was just COMPACTED — earlier turns are now a summary, so in-flight detail may be lost. Any rules that had been retrieved went into that summary with everything else, so treat yourself as holding none: before the next consequential act, ask again with \`search(content_type=\"rule\")\` rather than trusting a half-remembered one. Re-run \`enter_project()\` for the active project, check its recent milestones and open tasks, and reconcile what you are mid-way through against what Scribe records. Scribe is the record." diff --git a/plugin/hooks/scribe_tool_rules.sh b/plugin/hooks/scribe_tool_rules.sh index 16a1bd4..3262460 100644 --- a/plugin/hooks/scribe_tool_rules.sh +++ b/plugin/hooks/scribe_tool_rules.sh @@ -63,11 +63,8 @@ tool_enc=$(printf '%s' "$tool_name" | jq -sRr '@uri' 2>/dev/null) || exit 0 repo_q="" lookup_dir=${event_cwd:-${CLAUDE_PROJECT_DIR:-$PWD}} -repo_remote=$(git -C "$lookup_dir" remote get-url origin 2>/dev/null || true) -if [ -n "$repo_remote" ]; then - repo_enc=$(printf '%s' "$repo_remote" | jq -sRr '@uri' 2>/dev/null) || repo_enc="" - [ -n "$repo_enc" ] && repo_q="&repo=${repo_enc}" -fi +scope=$(scribe_scope_query "$lookup_dir") +[ -n "$scope" ] && repo_q="&${scope}" # THE SHARED SESSION LEDGER, and the thing most worth getting right here. # diff --git a/plugin/skills/using-scribe/SKILL.md b/plugin/skills/using-scribe/SKILL.md index 3ada364..972ee1a 100644 --- a/plugin/skills/using-scribe/SKILL.md +++ b/plugin/skills/using-scribe/SKILL.md @@ -13,10 +13,23 @@ asked for. ## Do this first (every session) -If the working repo maps to a Scribe project (you're in a known repo, or -`list_repo_bindings` shows a binding), call `enter_project(id)` — it returns the -project's goal, the milestones and open tasks worked on most recently, its -Systems and the titles of its own rules in one shot. +If the working directory maps to a Scribe project, call `enter_project(id)` — +it returns the project's goal, the milestones and open tasks worked on most +recently, its Systems and the titles of its own rules in one shot. + +**A directory does not have to be a git repo to have a project.** A repo is +bound by its remote (`list_repo_bindings` shows the bindings). Anything else — +a notes folder, a server's config directory, a scratch directory — is bound by +a `.scribe` file naming the project: + + {"instance": "https://scribe.example.com", "project_id": 2, "project": "Homelab"} + +`instance` is what makes the id trustworthy. An id means nothing on its own — +it is a different project on every Scribe — so a marker that has travelled to +another instance is ignored rather than followed to the wrong project. A bare +`2` also works when writing the file by hand. When work plainly belongs to a +project and the directory names none, offer to write the marker; +`list_projects` has the id. Then **ask before you act**: before anything hard to reverse or outward-facing, search the rules for what you are about to do. Reflex 2 below is why asking, diff --git a/src/scribe/routes/plugin.py b/src/scribe/routes/plugin.py index b812980..ee9e6e6 100644 --- a/src/scribe/routes/plugin.py +++ b/src/scribe/routes/plugin.py @@ -64,8 +64,11 @@ async def session_context(): resolves it to the bound project (see services/repo_bindings); an unbound repo yields a "bind this repo" hint instead. This is how the active project is determined — the plugin never pins a project id. - project_id (optional int) — explicit override, mainly for manual/ad-hoc - curl testing; takes precedence over `repo` when set. + project_id (optional int) — the project named directly, and the only + key a caller outside a git repo has (#4085): the plugin's hooks + send it when a `.scribe` marker file names a project. Takes + precedence over `repo`. Access-checked like any other read — an id + this account cannot read loads no project rather than failing. """ project_id, _repo, unbound_repo = await _project_scope() result = await plugin_ctx_svc.build_session_context( diff --git a/src/scribe/services/plugin_context.py b/src/scribe/services/plugin_context.py index 63e82a6..acea2bb 100644 --- a/src/scribe/services/plugin_context.py +++ b/src/scribe/services/plugin_context.py @@ -2171,7 +2171,10 @@ async def build_session_context( Args: user_id: the operator. project_id: the resolved active project (0 = none). The endpoint - resolves this from the working repo's remote, not from config. + resolves this from the working repo's remote, or from a `.scribe` + marker file naming the project directly — the only key a session + outside a git repo has (#4085). A non-zero id that does not + resolve is reported rather than silently dropped. unbound_repo: when the hook sent a repo remote that maps to no project, its normalized key — triggers a one-line "bind this repo" hint so the binding is self-healing. @@ -2241,18 +2244,34 @@ async def build_session_context( f"`get_design_system({design['id']})` → " f"`resolved_guidance`.", ] - elif unbound_repo: - lines += [ - "", - "## Repository not yet bound", - f"This repo (`{unbound_repo}`) isn't mapped to a Scribe project, so " - "no project context was loaded. Bind it once with " - f'`bind_repo(repo_url="{unbound_repo}", project_id=<id>)` ' - "(call `list_projects` to find the id) and future sessions here will " - "auto-load that project's context.", - ] - else: - lines += ["", "No Scribe project is bound to this working directory."] + # Nothing loaded — say which nothing (#4085). This used to hang off the + # `if project_id:` above as an `elif`, which meant an id that was SENT and + # did not resolve produced no message at all: the outer branch was taken, + # the inner one was not, and the caller got a context that simply omitted + # the project it had asked for. That is the one case worth being loudest + # about, because the caller is holding a pointer it believes in. + if project_dict is None: + if project_id: + lines += [ + "", + f"## Project {project_id} could not be loaded", + f"This session asked for project {project_id}, but this account " + "cannot read it — the id may belong to a different Scribe " + "instance, or the project may have been deleted. " + "`list_projects` shows what is readable here.", + ] + elif unbound_repo: + lines += [ + "", + "## Repository not yet bound", + f"This repo (`{unbound_repo}`) isn't mapped to a Scribe project, so " + "no project context was loaded. Bind it once with " + f'`bind_repo(repo_url="{unbound_repo}", project_id=<id>)` ' + "(call `list_projects` to find the id) and future sessions here will " + "auto-load that project's context.", + ] + else: + lines += ["", "No Scribe project is bound to this working directory."] context = "\n".join(line for line in lines if line is not None) if len(context) > _MAX_CHARS: diff --git a/tests/test_scribe_marker.py b/tests/test_scribe_marker.py new file mode 100644 index 0000000..7af04f3 --- /dev/null +++ b/tests/test_scribe_marker.py @@ -0,0 +1,150 @@ +"""`.scribe` — the second key a directory can be scoped by (#4085). + +Every hook in `plugin/hooks/` scopes its request to a project, and until this +existed all six turned on one key: `git remote get-url origin`. That key does +not exist outside a git repo, so a session in a plain directory was unscoped +everywhere at once — silently, because a missing remote is indistinguishable +from a remote nobody bound. + +`scribe_scope_query` is the shared answer, so these run the real shell rather +than a reimplementation of it. What they pin: + + * the marker is found from a subdirectory, the way git finds its root; + * a bare integer works, because that is what a person writes by hand; + * the marker BEATS a git remote, since someone put the file there on purpose; + * an `instance` naming a different Scribe is refused — the case the field + exists for. A project id means nothing on its own: id 2 is a different + project on every instance, so a marker that travels would otherwise scope + the session to the wrong project, confidently and without a word. + +That last one is the reason the file is JSON and not a number, so it is tested +from both directions: the matching host is honoured, the mismatched one is not. +""" +from __future__ import annotations + +import json +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" +INSTANCE = "https://scribe.example.com" + + +def _sh(script: str, cwd: Path, env_extra: dict | None = None) -> str: + for tool in ("bash", "jq"): + if shutil.which(tool) is None: + pytest.skip(f"hook runtime tool {tool!r} not installed") + env = {"PATH": os.environ["PATH"], "HOME": str(cwd)} + env.update(env_extra or {}) + body = f'set -uo pipefail\n. "{DEFS}"\nurl="{INSTANCE}"\n{script}\n' + out = subprocess.run(["bash", "-c", body], cwd=cwd, capture_output=True, + text=True, env=env, timeout=30) + assert out.returncode == 0, out.stderr + return out.stdout.strip() + + +def _marker(d: Path, **fields) -> None: + (d / ".scribe").write_text(json.dumps(fields)) + + +def _git_repo(d: Path, remote: str) -> None: + env = {"PATH": os.environ["PATH"], "HOME": str(d)} + subprocess.run(["git", "init", "-q"], cwd=d, check=True, env=env) + subprocess.run(["git", "remote", "add", "origin", remote], cwd=d, check=True, env=env) + + +def test_a_bare_integer_is_a_valid_marker(tmp_path): + """What someone writes by hand. It parses as JSON already, so one filter + reads it and the written form alike.""" + (tmp_path / ".scribe").write_text("7\n") + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "project_id=7" + + +def test_the_marker_is_found_from_a_subdirectory(tmp_path): + _marker(tmp_path, instance=INSTANCE, project_id=2, project="FabledScribe") + deep = tmp_path / "src" / "scribe" / "services" + deep.mkdir(parents=True) + assert _sh(f'scribe_scope_query "{deep}"', tmp_path) == "project_id=2" + + +def test_a_matching_instance_is_honoured(tmp_path): + """Host-only comparison, so http/https and a trailing slash are not a + mismatch — only a genuinely different Scribe is.""" + _marker(tmp_path, instance="http://scribe.example.com/", project_id=2) + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "project_id=2" + + +def test_a_marker_for_another_instance_is_refused_with_a_reason(tmp_path): + """The case the `instance` field exists for. No project beats the wrong + project, and the reason is carried so the session can say which it was.""" + _marker(tmp_path, instance="https://someone-elses.example.org", project_id=2) + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "" + said = _sh(f'scribe_marker_read "{tmp_path}/.scribe"', tmp_path) + assert "someone-elses.example.org" in said and "scribe.example.com" in said + + +def test_the_marker_beats_a_git_remote(tmp_path): + """Someone put the file there deliberately; a remote is only where the + code happens to be pushed. This is also how a directory overrides its + binding.""" + _git_repo(tmp_path, "git@git.example.com:someone/thing.git") + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path).startswith("repo=") + _marker(tmp_path, instance=INSTANCE, project_id=2) + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "project_id=2" + + +def test_without_a_marker_the_git_remote_still_answers(tmp_path): + """The path every existing install is on — it must not have moved.""" + _git_repo(tmp_path, "git@git.example.com:someone/thing.git") + got = _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) + assert got.startswith("repo=") and "git.example.com" in got.replace("%2F", "/") + + +def test_a_plain_directory_with_no_marker_scopes_to_nothing(tmp_path): + """Not an error — the caller sends no scope and the server says so.""" + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "" + + +@pytest.mark.parametrize("body", ["", "not json at all", "{}", '{"project_id": 0}', + '{"project_id": "../../etc"}', "[1,2,3]"]) +def test_an_unusable_marker_is_refused_rather_than_sent(tmp_path, body): + """A marker is operator-written and can say anything. Nothing that is not + a positive integer may reach the query string.""" + (tmp_path / ".scribe").write_text(body) + assert _sh(f'scribe_scope_query "{tmp_path}"', tmp_path) == "" + + +def test_the_reason_survives_the_command_substitution_that_fetches_it(tmp_path): + """Regression on a bug this nearly shipped with. + + The reason used to be a global the function set, which every caller reads + through `$( )` — a subshell, so the assignment died with it and the caller + saw an UNSET variable. The hooks all run `set -u`, where reading one aborts + the script: a directory with a misaddressed marker would have cost the + session its entire SessionStart context, to fetch a warning about a file. + + So the reason comes back through stdout with the id, and this asserts on + the shape the caller actually uses: read in a subshell, split, and used + under `set -u` without the shell dying. + """ + _marker(tmp_path, instance="https://elsewhere.example.org", project_id=2) + got = _sh( + f'read_out=$(scribe_marker_read "$(scribe_marker_file "{tmp_path}")")\n' + 'printf "id=[%s] why=[%s]" "${read_out%%$\'\\t\'*}" "${read_out#*$\'\\t\'}"', + tmp_path, + ) + assert got.startswith("id=[]") + assert "elsewhere.example.org" in got + + +def test_the_walk_up_terminates_at_the_root(tmp_path): + """No marker anywhere above: the loop must end rather than spin. tmp_path + is several levels down from /, so this really does walk.""" + deep = tmp_path / "a" / "b" / "c" + deep.mkdir(parents=True) + assert _sh(f'scribe_scope_query "{deep}"', tmp_path) == "" diff --git a/tests/test_services_plugin_context.py b/tests/test_services_plugin_context.py index 33bc3d4..fff5f62 100644 --- a/tests/test_services_plugin_context.py +++ b/tests/test_services_plugin_context.py @@ -215,6 +215,31 @@ async def test_build_session_context_unbound_repo_emits_bind_hint(): assert "## Active project" not in ctx +@pytest.mark.asyncio +async def test_a_project_id_that_does_not_resolve_is_reported_not_dropped(): + """A `.scribe` marker names a project directly (#4085), so for the first + time a caller arrives holding a pointer it BELIEVES in. If the id is for + another instance, or names a deleted project, the session has to be told — + it cannot infer it from an absence. + + This used to render nothing whatsoever: the branch hung off `if project_id` + as an `elif`, so an id that was sent and failed took the outer arm, found + no project, and fell out of the block having said nothing at all. + """ + from scribe.services.plugin_context import build_session_context + with patch("scribe.services.plugin_context.projects_svc.get_project", + AsyncMock(return_value=None)): + out = await build_session_context(user_id=7, project_id=41) + + ctx = out["context"] + assert out["project"] is None + assert "## Project 41 could not be loaded" in ctx + assert "list_projects" in ctx + # Not mistaken for the repo case, which has a different remedy. + assert "## Repository not yet bound" not in ctx + assert "No Scribe project is bound" not in ctx + + @pytest.mark.asyncio async def test_build_process_manifest_renders_stub_specs(): items = [