diff --git a/plugin/.claude-plugin/plugin.json b/plugin/.claude-plugin/plugin.json index 013d69dc..bd5458f8 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.10.03.1934", + "version": "2026.10.04.0125", "author": { "name": "Bryan Van Deusen" }, diff --git a/plugin/hooks/scribe_defs.sh b/plugin/hooks/scribe_defs.sh index 8a4c2deb..2dd48514 100644 --- a/plugin/hooks/scribe_defs.sh +++ b/plugin/hooks/scribe_defs.sh @@ -79,6 +79,7 @@ scribe_skip_prompt() { _scribe_prompt=${1#"${1%%[![:space:]]*}"} case "$_scribe_prompt" in ""*|\ + ""*|""*|""*|""*|\ ""*|""*|""*) return 0 ;; diff --git a/src/scribe/mcp/server.py b/src/scribe/mcp/server.py index 79ef5416..dc1ce5ca 100644 --- a/src/scribe/mcp/server.py +++ b/src/scribe/mcp/server.py @@ -4,7 +4,7 @@ from __future__ import annotations import difflib from mcp.server.mcpserver import MCPServer -from mcp.server.mcpserver.exceptions import ToolError +from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError from mcp.server.transport_security import TransportSecuritySettings from quart import Quart @@ -282,9 +282,10 @@ class StrictArgsMCPServer(MCPServer): declared cannot have been meant to be dropped. """ - # ToolError, not ValueError: the SDK returns either one's text to the - # caller as an error result, but logs anything that is not a ToolError as - # an unexpected crash. A misnamed argument is the caller's mistake. + # ToolError, not ValueError: since SDK 2.x only a ToolError's text reaches + # the caller. Anything else is wrapped as UnexpectedToolError("Error + # executing tool X") and its message stays on the server. A misnamed + # argument is the caller's mistake, and the caller needs to read why. async def call_tool(self, name, arguments, context=None): try: tool = self._tool_manager.get_tool(name) @@ -304,7 +305,17 @@ class StrictArgsMCPServer(MCPServer): f"It accepts: {', '.join(sorted(declared))}. Nothing was " "created or changed — retry with the declared names." ) - return await super().call_tool(name, arguments, context) + try: + return await super().call_tool(name, arguments, context) + except UnexpectedToolError as exc: + # ValueError is how every Scribe tool refuses (#4794): what was + # refused, why, and what to do instead, written for the agent. + # SDK 2.x masks it like a crash, so the agent got a failure with + # no reason and could only retry blind. Other exceptions ARE + # crashes; their text can carry internals and stays masked. + if isinstance(exc.__cause__, ValueError): + raise ToolError(f"{exc}: {exc.__cause__}") from exc.__cause__ + raise def build_mcp_server() -> MCPServer: diff --git a/src/scribe/services/embeddings.py b/src/scribe/services/embeddings.py index ea14d71b..e62a84bc 100644 --- a/src/scribe/services/embeddings.py +++ b/src/scribe/services/embeddings.py @@ -333,7 +333,10 @@ def document_title( # narrower wording would have read as "nothing to re-embed". # # 1 → 2: work logs joined the task document. -CHUNKER_VERSION = 2 +# 2 → 3: no chunk is a heading alone (#4784). A content-free chunk embeds +# near the centre of the space and outranked real records on any +# vague query — "## Work log — 2026-08-21" took rank 1. +CHUNKER_VERSION = 3 # The public name of the space every score lives in, and the two facts that @@ -404,9 +407,45 @@ async def rows_off_the_live_model(table) -> int: # the exact data loss this exists to end. _CHUNK_CHAR_BUDGET = 1400 +# The least a chunk may say beyond its headings (#4784). Below this a chunk is +# a heading, a date or a one-line lead-in, and it embeds as text about nothing +# in particular — which is close to everything, so it outranks real records on +# any vague query. Such a piece is joined to its neighbour instead of being +# given a vector of its own. +_MIN_CHUNK_CONTENT = 160 + _HEADING_RE = None # compiled lazily below to keep re import local +def _heading_re(): + import re + global _HEADING_RE + if _HEADING_RE is None: + _HEADING_RE = re.compile(r"^#{1,6}\s") + return _HEADING_RE + + +def _is_thin(text: str) -> bool: + """Says too little to be a chunk on its own: under `_MIN_CHUNK_CONTENT` + once its heading lines are set aside.""" + rx = _heading_re() + said = "\n".join(ln for ln in text.splitlines() if not rx.match(ln)).strip() + return len(said) < _MIN_CHUNK_CONTENT + + +def _last_heading(text: str, default: str = "") -> str: + """The last heading line outside a code fence, or `default`.""" + rx = _heading_re() + in_fence = False + found = default + for line in text.splitlines(): + if line.lstrip().startswith(("```", "~~~")): + in_fence = not in_fence + elif not in_fence and rx.match(line): + found = line + return found + + def _split_sections(body: str) -> list[str]: """Split a markdown body at heading lines, fence-aware. @@ -415,17 +454,13 @@ def _split_sections(body: str) -> list[str]: code fences do not split — a commented `# step` in a recorded shell snippet is content, not structure. """ - import re - global _HEADING_RE - if _HEADING_RE is None: - _HEADING_RE = re.compile(r"^#{1,6}\s") - + rx = _heading_re() sections: list[list[str]] = [[]] in_fence = False for line in body.splitlines(): if line.lstrip().startswith(("```", "~~~")): in_fence = not in_fence - if not in_fence and _HEADING_RE.match(line) and sections[-1]: + if not in_fence and rx.match(line) and sections[-1]: sections.append([line]) else: sections[-1].append(line) @@ -436,17 +471,27 @@ def _split_paragraphs(section: str, budget: int) -> list[str]: """Break one oversize section into budget-sized pieces at paragraph boundaries, hard-splitting only a single paragraph that alone exceeds the budget (a monster table or code block — split at line boundaries so no - content is dropped, which is the entire point of this module).""" + content is dropped, which is the entire point of this module). + + No piece is thin (#4784). A heading, or a lead-in line, that would be left + on its own is carried into the paragraph after it; a thin tail joins the + piece before it.""" pieces: list[str] = [] current = "" + # A hard cut never lands in the first quarter of the budget: the newline + # it would find there is the one closing a heading or a lead-in, and + # cutting at it is how a heading became a chunk of its own. + lo = budget // 4 for para in section.split("\n\n"): + if current and _is_thin(current): + para, current = f"{current}\n\n{para}", "" while len(para) > budget: # Hard split: prefer the last newline inside the budget so lines # stay whole, then the last space so words do; a clean char cut is # the final resort for one enormous unbroken token. - cut = para.rfind("\n", 0, budget) + cut = para.rfind("\n", lo, budget) if cut <= 0: - cut = para.rfind(" ", 0, budget) + cut = para.rfind(" ", lo, budget) if cut <= 0: cut = budget head, para = para[:cut], para[cut:].lstrip("\n ") @@ -462,6 +507,19 @@ def _split_paragraphs(section: str, budget: int) -> list[str]: current = para else: current = candidate + if current and pieces and _is_thin(current): + # A thin tail joins the piece before it. If that piece is full, its + # last paragraph moves down to keep the tail company instead. + last = pieces.pop() + if len(last) + 2 + len(current) <= budget: + current = f"{last}\n\n{current}" + else: + head, sep, tail = last.rpartition("\n\n") + if sep and len(tail) + 2 + len(current) <= budget: + pieces.append(head) + current = f"{tail}\n\n{current}" + else: + pieces.append(last) if current: pieces.append(current) return pieces @@ -499,12 +557,20 @@ def chunk_document(title: str | None, body: str | None) -> list[str]: # Merge small adjacent sections upward so tiny sections don't each spend a # vector, then split anything still over budget at paragraph boundaries. + # A THIN section takes the next one in whatever its size (#4784): a heading + # with nothing under it ("## Findings" above its "### …" parts) or a lone + # lead-in line would otherwise stand as a chunk, and the split below keeps + # it attached to the first paragraph that follows. merged: list[str] = [] for section in _split_sections(body or ""): - if merged and len(merged[-1]) + 2 + len(section) <= budget: + if merged and (_is_thin(merged[-1]) + or len(merged[-1]) + 2 + len(section) <= budget): merged[-1] = f"{merged[-1]}\n\n{section}" else: merged.append(section) + if len(merged) > 1 and _is_thin(merged[-1]): + tail = merged.pop() + merged[-1] = f"{merged[-1]}\n\n{tail}" chunks: list[str] = [] for section in merged: @@ -512,13 +578,15 @@ def chunk_document(title: str | None, body: str | None) -> list[str]: chunks.append(embedding_text(title, section)) continue pieces = _split_paragraphs(section, budget) - first_line = section.split("\n", 1)[0] - heading = first_line if first_line.lstrip().startswith("#") else "" + heading = "" for i, piece in enumerate(pieces): - # Repeat the section heading on continuation pieces so each vector - # still knows what topic it is part of. + # Repeat the heading a continuation piece sits under so each vector + # still knows what topic it is part of. The LAST heading so far, + # not the section's first: a thin heading merged forward (#4784) + # means one section can hold several. if i > 0 and heading and not piece.startswith(heading): piece = f"{heading}\n{piece}" + heading = _last_heading(piece, heading) chunks.append(embedding_text(title, piece)) return chunks diff --git a/tests/test_chunking.py b/tests/test_chunking.py index 58361640..828910b9 100644 --- a/tests/test_chunking.py +++ b/tests/test_chunking.py @@ -8,8 +8,11 @@ untouched), and long records lose NOTHING — every line of the body lands in some chunk, each chunk inside the window budget, each carrying the title as its topical anchor. """ +import re + from scribe.services.embeddings import ( _CHUNK_CHAR_BUDGET, + _MIN_CHUNK_CONTENT, chunk_document, embedding_text, ) @@ -136,6 +139,87 @@ def test_a_monster_single_paragraph_is_hard_split_not_dropped(): assert total_words == 2000 +# --- no chunk is a heading alone (#4784) ------------------------------------- +# +# Found by judging live auto-inject menus: "## Work log — 2026-08-21", +# "## Findings worth carrying forward" and "Two dev→main PRs this session." +# were each a whole chunk, and took ranks 1–3 on vague queries — text about +# nothing embeds close to everything. Each case below is one of those shapes. + +_HEADING = re.compile(r"^#{1,6}\s") + + +def _said(chunk: str, title: str) -> str: + """A chunk's text once its title prefix and heading lines are set aside.""" + body = chunk[len(title) + 1:] if chunk.startswith(title + "\n") else chunk + return "\n".join(ln for ln in body.splitlines() if not _HEADING.match(ln)).strip() + + +def _assert_nothing_thin_and_nothing_lost(title: str, body: str) -> list[str]: + chunks = chunk_document(title, body) + assert len(chunks) > 1, "the case must be long enough to split" + for chunk in chunks: + assert len(_said(chunk, title)) >= _MIN_CHUNK_CONTENT, ( + f"a chunk says almost nothing: {chunk[:120]!r}") + assert len(chunk) <= _CHUNK_CHAR_BUDGET + len(title) + 1 + 80 + joined = "\n".join(chunks) + for line in (ln.strip() for ln in body.splitlines()): + # A line longer than a chunk is cut across two, so only its ends can + # be looked for whole. + for part in ([line] if len(line) < 400 else [line[:60], line[-60:]]): + assert part in joined, f"content dropped: {part[:60]!r}" + return chunks + + +def test_a_heading_over_one_long_paragraph_is_not_a_chunk_of_its_own(): + """A work log whose entry is one long paragraph: the split used to flush + the heading as its own piece before cutting the paragraph.""" + entry = " ".join(["The deploy was verified against the live instance."] * 40) + _assert_nothing_thin_and_nothing_lost( + "Step 6 — webhook drift-flagging", + "The step body.\n\n## Work log — 2026-08-16\n\n" + entry, + ) + + +def test_a_heading_with_nothing_under_it_joins_what_follows(): + """'## Findings' directly above its '### …' parts is a section with no + body, and stood alone whenever the section before it was full.""" + body = ( + # Five paragraphs fill the section before it, so the heading cannot + # merge backwards and has to be carried forward. + "## Context\n\n" + _long_section("context", paragraphs=5) + + "\n\n## Findings worth carrying forward\n\n" + + "### One\n\n" + _long_section("one", paragraphs=5) + + "\n\n### Two\n\n" + _long_section("two", paragraphs=5) + ) + _assert_nothing_thin_and_nothing_lost("A milestone dev-log", body) + + +def test_a_lone_lead_in_line_joins_the_section_after_it(): + _assert_nothing_thin_and_nothing_lost( + "2026-06-10 — UI batch", + "Two dev→main PRs this session.\n\n## PR 89\n\n" + + _long_section("pr89", paragraphs=8), + ) + + +def test_a_thin_last_section_joins_the_one_before_it(): + _assert_nothing_thin_and_nothing_lost( + "T", + "## Body\n\n" + _long_section("body", paragraphs=8) + "\n\n## Next\n\nTBD.", + ) + + +def test_a_continuation_piece_repeats_the_heading_it_sits_under(): + """A thin heading merged forward puts two headings in one section; the + pieces of the second must carry the second, not the first.""" + body = "## Parent\n\n## Child\n\n" + _long_section("child", paragraphs=40) + chunks = chunk_document("T", body) + assert len(chunks) > 2 + for chunk in chunks[1:]: + assert "## Child" in chunk + + # --- the read path: best chunk wins (#280 step 4) ---------------------------- diff --git a/tests/test_mcp_strict_args.py b/tests/test_mcp_strict_args.py index f68e86c1..cff86143 100644 --- a/tests/test_mcp_strict_args.py +++ b/tests/test_mcp_strict_args.py @@ -12,7 +12,7 @@ call carrying arguments the tool does not declare, with a did-you-mean when one is close. An error the caller sees once beats data half-written forever. """ import pytest -from mcp.server.mcpserver.exceptions import ToolError +from mcp.server.mcpserver.exceptions import ToolError, UnexpectedToolError from scribe.mcp.server import StrictArgsMCPServer @@ -59,6 +59,58 @@ async def test_unknown_tool_keeps_the_upstream_error(): assert "no_such_tool" in str(exc.value) +# --- a refusal's text reaches the agent (#4794) ------------------------------- +# +# SDK 2.x passes on only a ToolError's text; anything else is wrapped as +# "Error executing tool X" and its message is kept on the server. ValueError is +# how every Scribe tool refuses, so for weeks every refusal arrived bare. + + +def _raising_server() -> StrictArgsMCPServer: + mcp = StrictArgsMCPServer("refusal-test") + + @mcp.tool() + def refuse() -> str: + raise ValueError("record 7 is not yours to change — open your own copy") + + @mcp.tool() + def crash() -> str: + raise RuntimeError("password=hunter2 at db.internal:5432") + + return mcp + + +async def test_a_refusal_reaches_the_caller_with_its_reason(): + mcp = _raising_server() + with pytest.raises(ToolError) as exc: + await mcp.call_tool("refuse", {}) + msg = str(exc.value) + assert "Error executing tool refuse" in msg + assert "record 7 is not yours to change — open your own copy" in msg + assert not isinstance(exc.value, UnexpectedToolError), ( + "still typed as a crash, so the SDK's handler would log it as one") + + +async def test_a_crash_stays_masked(): + """Only the refusal convention is unmasked: a crash's text can carry + internals, and the SDK hides it for that reason.""" + mcp = _raising_server() + with pytest.raises(UnexpectedToolError) as exc: + await mcp.call_tool("crash", {}) + assert "hunter2" not in str(exc.value) + + +@pytest.mark.usefixtures("_bind_user") +async def test_a_real_tools_refusal_reaches_the_caller(): + """Against the real server, on a refusal that needs no database: the + message judge_menu writes for an empty verdict list.""" + from scribe.mcp.server import build_mcp_server + + with pytest.raises(ToolError) as exc: + await build_mcp_server().call_tool("judge_menu", {"log_id": 1, "verdicts": []}) + assert "at least one verdict" in str(exc.value) + + async def test_the_original_regression_create_note_with_content(): """Pin #2709 itself against the REAL server: `content=` on create_note must raise before dispatch — naming the bad argument and listing `body` diff --git a/tests/test_prompt_boundary_filter.py b/tests/test_prompt_boundary_filter.py index bd44a403..97862d96 100644 --- a/tests/test_prompt_boundary_filter.py +++ b/tests/test_prompt_boundary_filter.py @@ -52,6 +52,10 @@ SYNTHETIC = [ "Failed to reconnect", "Caveat: The messages below were generated…", " \n\nabc", # whitespace must not walk the filter + # A subagent's hand-back (#4785) — it carries attributes, so the tag is + # matched up to its name, not to a closing `>`. + '\n[Subagent hand-back] The text below is the final report', + "\nreport", ] # Typed by a person. Every one of these must still be retrieved against. @@ -62,6 +66,7 @@ REAL = [ "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 + " list is empty", # shares the prefix, is not the tag "here is the transcript you asked for:\nhi", ]