From 4d9211b5ae31bfcfb9d2b4ef7c9c9c091d42fb49 Mon Sep 17 00:00:00 2001 From: John Dilts Date: Fri, 18 Sep 2026 10:42:07 -0400 Subject: [PATCH 1/2] feat(opencode): upgrade plugin to full hook-guard port (search + read + strict) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the once-per-session bash reminder with a JS port of the Claude Code PreToolUse guard pair (graphify hook-guard ): - Search guard: fires on the bash command's executed tokens — heredoc bodies and quoted spans dropped, wrappers skipped, git grep / VAR=x grep counted (the #3121 contract, mirrored from cli.py _bash_invokes_search). - Read/glob guard: source-extension + in-project gating, output-dir reads exempt, files newer than graph.json (or a .needs_update marker) soften to the stale nudge (the #1840 contract). - Strict mode (GRAPHIFY_HOOK_STRICT=1, GRAPHIFY_HOOK_STRICT_TTL): a forceful once-per-session reminder for reads of files the graph indexes, suppressed while cache/last_query_stamp is fresh. Shares the same hook_sessions markers as the Claude hook, so the once-per-session budget spans harnesses. The Claude deny degrades to a strong reminder: opencode plugins cannot block a tool call from the before hook. - Transport: opencode has no PreToolUse additionalContext channel, so bash nudges ride a shell-inert echo '…' ; prepend (no backticks, $ or single quotes — #1413) and read/grep/glob nudges are appended to the tool result via tool.execute.after. - Honors GRAPHIFY_OUT like the git hook gates (#3546). - New tests/test_opencode_plugin_hook.py drives the extracted plugin in node with the same parametrized cases as test_hook_guard_token_match, plus orientation budget, staleness, strict once-per-session, and shell-inertness pins. Updated the two test_install.py shape tests for the new echo construction. --- CHANGELOG.md | 4 + graphify/install.py | 309 +++++++++++++++++++++++++++-- tests/test_install.py | 32 +-- tests/test_opencode_plugin_hook.py | 309 +++++++++++++++++++++++++++++ 4 files changed, 620 insertions(+), 34 deletions(-) create mode 100644 tests/test_opencode_plugin_hook.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ae58e4c560..071ff26830 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Full release notes with details on each version: [GitHub Releases](https://github.com/safishamsi/graphify/releases) +## Unreleased + +- Feature: the OpenCode plugin is upgraded from a once-per-session bash reminder to a full JS port of the Claude Code PreToolUse guard pair (`graphify hook-guard `): a search guard that fires on the bash command's executed tokens (heredoc bodies and quoted spans dropped, wrappers skipped, `git grep`/`VAR=x grep` counted — the #3121 contract), a read/glob guard with source-extension, in-project, and staleness gating (the #1840 contract), and the opt-in strict mode (`GRAPHIFY_HOOK_STRICT=1`, `GRAPHIFY_HOOK_STRICT_TTL`) that emits a forceful once-per-session reminder for reads of indexed files, sharing the same `hook_sessions` markers and `last_query_stamp` as the Claude hook so the budget spans harnesses. Because opencode has no PreToolUse `additionalContext` channel, bash nudges ride a shell-inert `echo '…' ;` prepend and read/grep/glob nudges are appended to the tool result via `tool.execute.after`; the Claude `deny` degrades to the strong reminder since opencode plugins cannot block a tool call from the `before` hook. + ## 0.9.62 (2026-09-15) - Feature: Terraform module calls with a literal local `source` (`./…` or `../…`) now resolve to a directory-scoped module node, exposing the caller→implementation topology (e.g. environment → application → base); remote and registry sources and source expressions are left unresolved and never fabricate a target. After upgrading an existing graph, run `graphify update .` once to regenerate Terraform ids and topology (#3571, thanks @vstepko). diff --git a/graphify/install.py b/graphify/install.py index 2fb2192760..a6cf65d8bb 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -1376,35 +1376,300 @@ def _uninstall_kilo_plugin(project_dir: Path) -> None: print( f" {write_config_file.relative_to(project_dir)} -> plugin deregistered" ) -# OpenCode tool.execute.before plugin — fires before every tool call. -# Injects a graph reminder into bash command output when graph.json exists. +# OpenCode plugin — a JS port of the Claude Code PreToolUse guard pair +# (`graphify hook-guard `, see cli.py) onto opencode's +# `tool.execute.before` / `tool.execute.after` plugin surface. +# +# opencode has no PreToolUse "additionalContext" channel, so the same guard +# decisions ride two transports: +# - bash: echo '' ; prepended to the command (before) +# - read / grep / glob: nudge appended to the tool result (after) +# The decision logic mirrors the Python guards: executed-token bash analysis +# (heredoc bodies and quoted spans dropped, wrappers skipped, #3121), +# source-extension + in-project + staleness gating for reads (#1840), and the +# opt-in once-per-session strict reminder (GRAPHIFY_HOOK_STRICT) that shares +# the same hook_sessions markers and last_query_stamp as the Claude hook. +# Fails open everywhere: any error means no nudge, never a blocked call. _OPENCODE_PLUGIN_JS = """\ -// graphify OpenCode plugin -// Injects a knowledge graph reminder before bash tool calls when the graph exists. +// graphify OpenCode plugin — port of graphify's Claude Code PreToolUse hooks +// (graphify hook-guard search + read) to opencode's plugin surface. // -// IMPORTANT: keep the reminder string free of backticks and $(...) constructs. -// The hook prepends `echo "" && ` to the user's bash command; -// backticks inside the double-quoted echo trigger bash command substitution, -// which both corrupts tool output and silently executes the very graphify -// command we are only suggesting. Plain words render fine in opencode's TUI. -import { existsSync } from "fs"; -import { join } from "path"; +// opencode has no PreToolUse "additionalContext" channel, so nudges ride two +// ways: +// - bash: echo '' ; prepended to the command (tool.execute.before) +// - read / grep / glob: nudge appended to the tool result (tool.execute.after) +// +// IMPORTANT: keep every prepended echo string free of backticks, $ and single +// quotes. It is wrapped in `echo '...'` and glued onto the user's command; +// backticks or $() inside would execute the graphify command we only suggest, +// and a single quote would terminate the echo. Appended (after) texts are not +// shell-interpreted, so they may use backticks freely. +// +// Guard semantics mirror the Claude hooks: +// - search guard: fires on the command's EXECUTED tokens (heredoc bodies and +// quoted spans dropped, wrappers skipped) so prose like +// `git commit -m "add flag support"` never triggers (#3121). +// - read guard: only in-project source files; output-dir reads are +// exempt; a file newer than graph.json (or a .needs_update marker) softens +// to the stale nudge instead of the mandatory one (#1840). +// - strict mode (GRAPHIFY_HOOK_STRICT=1): the first read per session of a +// file the graph indexes gets the once-only strict reminder; suppressed +// while /cache/last_query_stamp is fresh (TTL +// GRAPHIFY_HOOK_STRICT_TTL, default 1800s). opencode cannot deny a tool +// from tool.execute.before, so the Claude "deny" degrades to a forceful +// once-per-session nudge. Session markers live in +// /cache/hook_sessions/.denied — the same files the +// Claude Code hook writes, so the once-per-session budget is shared +// across harnesses. +// Fails open everywhere: any error means no nudge, never a blocked call. +// The output directory name honors GRAPHIFY_OUT (default graphify-out). +import { existsSync, statSync, mkdirSync, openSync, closeSync, readFileSync, unlinkSync, readdirSync } from "fs"; +import { join, relative, resolve, basename, isAbsolute } from "path"; + +const OUT = process.env.GRAPHIFY_OUT || "graphify-out"; + +const ORIENT_ECHO = + "[" + OUT + "] knowledge graph at " + OUT + "/. For focused questions, run graphify query with your question (scoped subgraph, usually much smaller than GRAPH_REPORT.md) instead of grepping raw files. Read GRAPH_REPORT.md only for broad architecture context."; + +const SEARCH_ECHO = + "[" + OUT + "] MANDATORY: " + OUT + "/graph.json exists. Run graphify query with your question before grepping raw files. Only grep after graphify has oriented you, or to modify/debug specific lines."; + +const SEARCH_NUDGE = + 'MANDATORY: ' + OUT + '/graph.json exists. You MUST run `graphify query ""` before grepping raw files. Only grep after graphify has oriented you, or to modify/debug specific lines.'; + +const READ_NUDGE = + 'MANDATORY: ' + OUT + '/graph.json exists. You MUST run graphify before reading source files. Use: `graphify query ""` (scoped subgraph), `graphify explain ""`, or `graphify path "" ""`. Only read raw files after graphify has oriented you, or to modify/debug specific lines. This rule applies to subagents too — include it in every subagent prompt involving code exploration.'; + +const STALE_NUDGE = + OUT + '/graph.json exists but may be STALE for this file (the file changed after the last build). Prefer `graphify query ""` for orientation, and run `graphify update` to refresh the graph. Reading the file directly is fine.'; + +const STRICT_NUDGE = + 'graphify strict mode: this project has a fresh knowledge graph that covers this file. Run `graphify query ""` (or `graphify explain` / `graphify path`) FIRST to orient yourself, then re-issue this Read — it will proceed normally. This reminder fires at most once per session; reading raw files to modify or debug specific lines is fine after one query. Apply the same rule in any subagent prompt that explores code.'; + +const SOURCE_EXTS = new Set([ + ".py", ".js", ".cjs", ".ts", ".tsx", ".jsx", ".astro", ".vue", ".svelte", ".go", + ".rs", ".java", ".rb", ".c", ".h", ".cpp", ".hpp", ".cc", ".cs", ".kt", + ".swift", ".php", ".scala", ".lua", ".sh", ".md", ".rst", ".txt", ".mdx", +]); + +const SEARCH_COMMANDS = new Set([ + "grep", "egrep", "fgrep", "zgrep", "rg", "ripgrep", "find", "fd", "ack", "ag", +]); + +const COMMAND_WRAPPERS = new Set([ + "sudo", "command", "exec", "nohup", "time", "nice", "ionice", "env", + "xargs", "timeout", "stdbuf", "doas", +]); + +const HEREDOC_OPEN = /<<-?\\s*(['"]?)(\\w+)\\1/g; + +function escapeRe(s) { + return s.replace(/[.*+?^${}()|[\\]\\\\]/g, "\\\\$&"); +} + +function stripHeredocs(text) { + // Mirror cli.py: keep scanning AFTER each opener line, so the same opener + // never re-matches once its body is stripped (a reset to 0 would re-find + // the opener and drop the lines that follow the terminator). + let pos = 0; + for (;;) { + HEREDOC_OPEN.lastIndex = pos; + const m = HEREDOC_OPEN.exec(text); + if (!m) break; + const nl = text.indexOf("\\n", m.index + m[0].length); + if (nl === -1) break; + const termRe = new RegExp("^[ \\\\t]*" + escapeRe(m[2]) + "[ \\\\t]*$", "m"); + const rest = text.slice(nl + 1); + const t = termRe.exec(rest); + if (!t) { + // Unterminated heredoc: everything after the opener line is body. + text = text.slice(0, nl + 1); + break; + } + text = text.slice(0, nl + 1) + rest.slice(t.index + t[0].length); + pos = nl + 1; + } + return text; +} + +function bashInvokesSearch(cmdStr) { + let text = stripHeredocs(cmdStr); + // Anything quoted is an argument, never the executable. + text = text.replace(/'[^']*'/g, " ").replace(/"[^"]*"/g, " "); + for (const segment of text.split(/[|;&\\n]|\\$\\(|`|\\(|\\)|\\{|\\}/)) { + const tokens = segment.trim().split(/\\s+/).filter(Boolean); + let i = 0; + while (i < tokens.length) { + const tok = tokens[i]; + if (tok.split("/").pop().includes("=") && !tok.startsWith("-") && !tok.startsWith("/")) { + i++; // VAR=value prefix + continue; + } + let name = tok.replaceAll("\\\\", "/").split("/").pop().toLowerCase(); + if (name.endsWith(".exe")) name = name.slice(0, -4); + if (COMMAND_WRAPPERS.has(name)) { + i++; + while (i < tokens.length && tokens[i].startsWith("-")) i++; + continue; + } + if (SEARCH_COMMANDS.has(name)) return true; + if (name === "git" && tokens.slice(i + 1, i + 4).some((t) => t === "grep" && !t.startsWith("-"))) return true; + break; // first real token decides this segment + } + } + return false; +} + +function strictEnabled() { + const v = (process.env.GRAPHIFY_HOOK_STRICT || "").trim().toLowerCase(); + if (["1", "true", "yes", "on"].includes(v)) return true; + if (["0", "false", "no", "off"].includes(v)) return false; + return false; +} + +function queryStampFresh(directory) { + try { + const ttl = parseFloat(process.env.GRAPHIFY_HOOK_STRICT_TTL || "1800") * 1000; + const stamp = join(directory, OUT, "cache", "last_query_stamp"); + return Date.now() - statSync(stamp).mtimeMs < ttl; + } catch { + return false; + } +} + +function markSessionDenied(directory, sessionID) { + const sid = String(sessionID || "").replace(/[^A-Za-z0-9_-]/g, "_").slice(0, 64); + if (!sid) return false; + try { + const d = join(directory, OUT, "cache", "hook_sessions"); + mkdirSync(d, { recursive: true }); + const fd = openSync(join(d, sid + ".denied"), "wx"); + closeSync(fd); + try { + const cutoff = Date.now() - 86400000; + for (const e of readdirSync(d)) { + const p = join(d, e); + try { + if (statSync(p).mtimeMs < cutoff) unlinkSync(p); + } catch {} + } + } catch {} + return true; + } catch { + return false; + } +} + +function targetIsIndexed(directory, filePath) { + if (!filePath) return true; + try { + const mp = join(directory, OUT, "manifest.json"); + if (statSync(mp).size > 2000000) return true; + const manifest = JSON.parse(readFileSync(mp, "utf8")); + if (!manifest || typeof manifest !== "object" || !Object.keys(manifest).length) return true; + const keys = new Set(Object.keys(manifest).map((k) => String(k).replaceAll("\\\\", "/"))); + const abskey = String(filePath).replaceAll("\\\\", "/"); + if (keys.has(abskey)) return true; + const rels = new Set(); + try { + const r = relative(directory, resolve(filePath)).replaceAll("\\\\", "/"); + if (r) rels.add(r); + } catch {} + rels.add(basename(filePath)); + for (const r of rels) { + if (keys.has(r)) return true; + for (const k of keys) if (k.endsWith("/" + r) || k === r) return true; + } + return false; + } catch { + return true; + } +} + +function fileTails(vals) { + const tails = []; + for (const v of vals) { + const seg = v.toLowerCase().replaceAll("\\\\", "/").split("/").pop(); + if (seg.includes(".")) tails.push("." + seg.split(".").pop()); + } + return tails; +} export const GraphifyPlugin = async ({ directory }) => { - let reminded = false; + const graphPath = join(directory, OUT, "graph.json"); + const oriented = new Set(); + + function inProject(v) { + if (!isAbsolute(v)) return true; // relative paths anchor at the project cwd + try { + const r = relative(directory, resolve(v)); + return r === "" || (!r.startsWith("..") && !isAbsolute(r)); + } catch { + return false; + } + } + + function readGuard(tool, args, sessionID) { + const filePath = String(args.file_path ?? args.filePath ?? ""); + const pathArg = String(args.path ?? ""); + const pattern = String(args.pattern ?? ""); + const vals = [filePath, pathArg, pattern].filter(Boolean); + if (!vals.length) return null; + const joined = vals.join(" ").toLowerCase().replaceAll("\\\\", "/"); + if (joined.includes(OUT + "/")) return null; + if (!fileTails(vals).some((t) => SOURCE_EXTS.has(t))) return null; + const explicit = [filePath, pathArg].filter(Boolean); + if (explicit.length && !explicit.some((v) => inProject(v))) return null; + let stale = false; + try { + if (filePath && statSync(filePath).mtimeMs > statSync(graphPath).mtimeMs) stale = true; + } catch { + stale = false; + } + if (!stale && existsSync(join(directory, OUT, ".needs_update"))) stale = true; + if (stale) return STALE_NUDGE; + if ( + strictEnabled() && + tool === "read" && + !queryStampFresh(directory) && + targetIsIndexed(directory, filePath) && + markSessionDenied(directory, sessionID) + ) { + return STRICT_NUDGE; + } + return READ_NUDGE; + } return { "tool.execute.before": async (input, output) => { - if (reminded) return; - if (!existsSync(join(directory, "graphify-out", "graph.json"))) return; - - if (input.tool === "bash") { - // ';' not '&&' — Windows PowerShell 5.1 rejects '&&' as a statement - // separator, breaking the first bash command of the session (#1646). - output.args.command = - 'echo "[graphify] knowledge graph at graphify-out/. For focused questions, run graphify query with your question (scoped subgraph, usually much smaller than GRAPH_REPORT.md) instead of grepping raw files. Read GRAPH_REPORT.md only for broad architecture context." ; ' + - output.args.command; - reminded = true; + if (!existsSync(graphPath)) return; + const tool = String(input.tool || "").toLowerCase(); + if (tool === "bash") { + const cmd = String(output.args?.command ?? ""); + const nudges = []; + if (!oriented.has(input.sessionID)) { + oriented.add(input.sessionID); + nudges.push(ORIENT_ECHO); + } + if (cmd && bashInvokesSearch(cmd)) nudges.push(SEARCH_ECHO); + if (nudges.length) { + // ';' not '&&' — Windows PowerShell 5.1 rejects '&&' as a statement + // separator (#1646). + output.args.command = "echo '" + nudges.join(" ") + "' ; " + cmd; + } + } + }, + "tool.execute.after": async (input, output) => { + if (!existsSync(graphPath)) return; + const tool = String(input.tool || "").toLowerCase(); + let nudge = null; + if (tool === "grep") { + nudge = SEARCH_NUDGE; + } else if (tool === "read" || tool === "glob") { + nudge = readGuard(tool, output.args || {}, input.sessionID); + } + if (nudge) { + output.output = (output.output ? output.output + "\\n\\n" : "") + nudge; } }, }; diff --git a/tests/test_install.py b/tests/test_install.py index 72d4e4dd26..73c63a13c9 100644 --- a/tests/test_install.py +++ b/tests/test_install.py @@ -853,25 +853,31 @@ def test_opencode_agents_install_writes_plugin(tmp_path): def test_opencode_plugin_reminder_has_no_backticks(tmp_path): - """The bash reminder string must not contain backticks or $(...) (regression test for #1413). + """The bash reminder string must be shell-inert (regression test for #1413). - The plugin prepends `echo "" && ` to the user's bash command. + The plugin prepends `echo '' ; ` to the user's bash command. Backticks or $() inside the reminder trigger bash command substitution when the echo runs, which both corrupts tool output and silently executes - the very graphify command we are only suggesting. + the very graphify command we are only suggesting. A single quote would + terminate the echo early, so the echoed text must also avoid it. """ _agents_install(tmp_path, "opencode") plugin = tmp_path / ".opencode" / "plugins" / "graphify.js" body = plugin.read_text() - # Extract the echoed reminder string literal between the double-quotes - # of the `output.args.command = 'echo "..." && ' +` line. + # Only the ECHOED literals are shell-interpreted (ORIENT_ECHO / SEARCH_ECHO, + # both named _ECHO). The `tool.execute.after` nudges ride the tool result + # and are never shell-interpreted, so they may contain backticks. import re - m = re.search(r'echo "([^"]*)"', body) - assert m, "echo reminder not found in plugin body" - reminder = m.group(1) - assert "`" not in reminder, f"backtick in reminder would trigger command substitution: {reminder!r}" - assert "$(" not in reminder, f"$() in reminder would trigger command substitution: {reminder!r}" + echo_block = re.search(r"const ORIENT_ECHO[\s\S]*?const SEARCH_ECHO =[^;]*;", body) + assert echo_block, "echo reminder constants not found in plugin body" + literals = re.findall(r'"([^"\n]*)"', echo_block.group(0)) + reminders = [s for s in literals if "graph" in s and "query" in s] + assert reminders, "echo reminder literals not found in plugin body" + for reminder in reminders: + assert "`" not in reminder, f"backtick in echoed text would trigger command substitution: {reminder!r}" + assert "$" not in reminder, f"$ in echoed text would trigger command substitution: {reminder!r}" + assert "'" not in reminder, f"single quote in echoed text would terminate the echo: {reminder!r}" def test_opencode_plugin_uses_semicolon_not_ampersand(tmp_path): @@ -881,8 +887,10 @@ def test_opencode_plugin_uses_semicolon_not_ampersand(tmp_path): in PowerShell 5.1, Bash, and POSIX shells.""" _agents_install(tmp_path, "opencode") body = (tmp_path / ".opencode" / "plugins" / "graphify.js").read_text() - # The prepend line ends with the separator before `' +`. - assert '" ; \' +' in body or '." ; \' +' in body, "reminder should join with ';'" + # The prepend line joins with a single-quoted echo and ';'. + assert "\"echo '\" + nudges.join(\" \") + \"' ; \" + cmd" in body, ( + "reminder should join with ';'" + ) assert '" && \' +' not in body, "'&&' breaks PowerShell 5.1 (#1646)" diff --git a/tests/test_opencode_plugin_hook.py b/tests/test_opencode_plugin_hook.py new file mode 100644 index 0000000000..ad2ce47cf2 --- /dev/null +++ b/tests/test_opencode_plugin_hook.py @@ -0,0 +1,309 @@ +"""The OpenCode plugin mirrors the Claude Code hook-guard contract. + +The plugin ships as the `_OPENCODE_PLUGIN_JS` string in graphify.install and is +written to .opencode/plugins/graphify.js at install time. Its decision logic is +a JS port of the Python guards (cli.py `_run_hook_guard`, #3121 token analysis, +#1840 staleness softening). These tests extract the plugin body to disk, load +it with the system node, and drive `tool.execute.before` / `tool.execute.after` +with the same cases the Python hook-guard tests use, so the two harnesses stay +in lockstep. + +Skipped on hosts without a `node` binary (environmental precondition). +""" +from __future__ import annotations + +import json +import shutil +import subprocess +from pathlib import Path + +import pytest + +from graphify.install import _OPENCODE_PLUGIN_JS + +pytestmark = pytest.mark.skipif(shutil.which("node") is None, reason="node binary not installed") + +_NODE = shutil.which("node") + + +def _run_plugin( + plugin_file: str, calls: list[dict], tmp_path: Path, *, env: dict[str, str] | None = None, graph: bool = True +) -> list[dict]: + """Load the plugin in node and run a batch of tool-call fixtures. + + Each call: {hook, tool, args|command, sid}. Returns the post-hook output + payloads, with bash commands unwrapped so tests can assert on the command + itself. + """ + graph_dir = tmp_path / "graphify-out" + graph_dir.mkdir(exist_ok=True) + if graph: + if not (graph_dir / "graph.json").exists(): + (graph_dir / "graph.json").write_text("{}", encoding="utf-8") + # A manifest listing AGENTS.md, so targetIsIndexed can be exercised. + (graph_dir / "manifest.json").write_text( + json.dumps({"AGENTS.md": [], "src/main.py": []}), encoding="utf-8" + ) + src = tmp_path / "AGENTS.md" + if not src.exists(): + src.write_text("hello", encoding="utf-8") + src_py = tmp_path / "main.py" + src_py.parent.joinpath("src").mkdir(exist_ok=True) + if not (tmp_path / "src" / "main.py").exists(): + (tmp_path / "src" / "main.py").write_text("x = 1", encoding="utf-8") + if graph and not (graph_dir / ".seeded").exists(): + # Backdate the source files so they predate graph.json: without this, + # files written after the graph read as stale (#1840 softening). Only + # on first seed per tmp_path — a test that touches a file afterwards + # to simulate staleness must not be undone here. + import os + + past = (graph_dir / "graph.json").stat().st_mtime - 60 + os.utime(src, (past, past)) + os.utime(tmp_path / "src" / "main.py", (past, past)) + (graph_dir / ".seeded").write_text("", encoding="utf-8") + + driver = tmp_path / "drive.mjs" + driver.write_text( + json.dumps({ + "directory": str(tmp_path), + "env": env or {}, + "calls": calls, + }), + encoding="utf-8", + ) + script = tmp_path / "run_driver.mjs" + script.write_text( + "import { readFileSync } from 'fs';\n" + "import { pathToFileURL } from 'url';\n" + "const cfg = JSON.parse(readFileSync(process.argv[2], 'utf8'));\n" + "for (const [k, v] of Object.entries(cfg.env)) process.env[k] = v;\n" + "const mod = await import(pathToFileURL(process.argv[3]).href);\n" + "const plugin = await mod.GraphifyPlugin({ directory: cfg.directory });\n" + "const out = [];\n" + "for (const c of cfg.calls) {\n" + " if (c.hook === 'before') {\n" + " const output = { args: { command: c.command } };\n" + " await plugin['tool.execute.before']({ tool: c.tool, sessionID: c.sid }, output);\n" + " out.push({ command: output.args.command });\n" + " } else {\n" + " const output = { args: c.args, output: '' };\n" + " await plugin['tool.execute.after']({ tool: c.tool, sessionID: c.sid }, output);\n" + " out.push({ output: output.output });\n" + " }\n" + "}\n" + "console.log(JSON.stringify(out));\n", + encoding="utf-8", + ) + import subprocess + + res = subprocess.run( + [_NODE, str(script), str(driver), plugin_file], + capture_output=True, text=True, timeout=30, + ) + assert res.returncode == 0, f"node driver failed:\n{res.stderr}" + return json.loads(res.stdout) + + +@pytest.fixture(scope="module") +def extracted_js(tmp_path_factory) -> str: + out = tmp_path_factory.mktemp("extract") + js = out / "extracted_graphify.js" + js.write_text(_OPENCODE_PLUGIN_JS, encoding="utf-8") + return str(js) + + +def _plugin_path(extracted_js: str, tmp_path: Path) -> str: + """Copy the extracted plugin into this test's tmp dir (node resolves it + relative to the driver's tmp tree) and return the path.""" + dst = tmp_path / "extracted_graphify.js" + dst.write_text(Path(extracted_js).read_text(encoding="utf-8"), encoding="utf-8") + return str(dst) + + +# Mirrors tests/test_hook_guard_token_match.py (the same #3121 contract). +FIRE = [ + "grep -rn foo .", + "rg foo", + "rg.exe foo src", + "/usr/bin/grep -c x f", + "egrep 'a|b' f", + "fd -e py", + "ack pattern", + "ag pattern src/", + "find . -name '*.py'", + "git grep TODO", + "git -C repo grep TODO", + "cat f.txt | grep needle", + "make build && grep -q ok build.log", + "sudo grep root /etc/passwd", + "xargs -0 grep -l pattern", + "FOO=1 grep x f", + "echo done; rg leftover", + "result=$(grep -c x f)", +] +QUIET = [ + "git commit -m x", + "git log -S foo", + 'git commit -m "add flag support"', + 'gh pr create --body "you can find it here"', + 'echo "see grep docs"', + "cat asdfd file", + "python manage.py runserver", + "cargo build --release", + "./gradlew test", + "echo 'grep is a fine tool'", + "printf 'use find sparingly'", + "magick convert x.png y.jpg", +] + + +@pytest.mark.parametrize("cmd", FIRE) +def test_js_search_guard_fires_on_real_searches(tmp_path, extracted_js, cmd): + (out,) = _run_plugin(_plugin_path(extracted_js, tmp_path), [{"hook": "before", "tool": "bash", "command": cmd, "sid": "s"}], tmp_path) + assert "MANDATORY" in out["command"] + + +@pytest.mark.parametrize("cmd", QUIET) +def test_js_search_guard_stays_quiet_on_prose(tmp_path, extracted_js, cmd): + (out,) = _run_plugin(_plugin_path(extracted_js, tmp_path), [{"hook": "before", "tool": "bash", "command": cmd, "sid": "s"}], tmp_path) + assert "MANDATORY" not in out["command"] + + +def test_js_search_guard_heredoc_cases(tmp_path, extracted_js): + quiet = ( + "cat > docs/design.md <<'EOF'\n" + "# Search strategy\n" + "We use grep and rg for quick scans; find . -name works too.\n" + "EOF\n" + ) + fires = ( + "cat > notes.txt <<'EOF'\nnothing here\nEOF\n" + "grep -rn needle src/\n" + ) + unterminated = "cat <<'EOF'\nall of this is grep prose with find . in it\n" + quoted = 'echo "grep -rn foo ."' + (q, f, u) = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [ + {"hook": "before", "tool": "bash", "command": quiet, "sid": "s"}, + {"hook": "before", "tool": "bash", "command": fires, "sid": "s"}, + {"hook": "before", "tool": "bash", "command": quoted, "sid": "s"}, + ], + tmp_path, + ) + assert "MANDATORY" not in q["command"] + assert "MANDATORY" in f["command"] + assert "MANDATORY" not in u["command"] + (u2,) = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [{"hook": "before", "tool": "bash", "command": unterminated, "sid": "s"}], + tmp_path, + ) + assert "MANDATORY" not in u2["command"] + + +def test_js_orientation_echo_fires_once_per_session(tmp_path, extracted_js): + outs = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [ + {"hook": "before", "tool": "bash", "command": "ls", "sid": "a"}, + {"hook": "before", "tool": "bash", "command": "ls", "sid": "a"}, + {"hook": "before", "tool": "bash", "command": "ls", "sid": "b"}, + ], + tmp_path, + ) + assert "knowledge graph at graphify-out" in outs[0]["command"] + assert "knowledge graph at graphify-out" not in outs[1]["command"] + assert "knowledge graph at graphify-out" in outs[2]["command"] + + +def test_js_read_guard_gates(tmp_path, extracted_js): + outs = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [ + # graphify-out read: exempt + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "graphify-out" / "GRAPH_REPORT.md")}, "sid": "s"}, + # non-source ext: exempt + {"hook": "after", "tool": "read", "args": {"file_path": "/tmp/x.png"}, "sid": "s"}, + # out-of-project: exempt + {"hook": "after", "tool": "read", "args": {"file_path": "/etc/hostname"}, "sid": "s"}, + # in-project source: nudge + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "AGENTS.md")}, "sid": "s"}, + ], + tmp_path, + ) + assert outs[0]["output"] == "" + assert outs[1]["output"] == "" + assert outs[2]["output"] == "" + assert "MANDATORY" in outs[3]["output"] + + +def test_js_read_guard_stale_softens(tmp_path, extracted_js): + # The fixture writes AGENTS.md then backdates it; touch it again AFTER the + # graph exists so its mtime is newer -> stale nudge, not mandatory (#1840). + target = tmp_path / "AGENTS.md" + outs = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [{"hook": "after", "tool": "read", "args": {"file_path": str(target)}, "sid": "s"}], + tmp_path, + ) + # Sanity: backdated read gets the mandatory nudge, not stale. + assert "MANDATORY" in outs[0]["output"] + target.write_text("changed", encoding="utf-8") + (out,) = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [{"hook": "after", "tool": "read", "args": {"file_path": str(target)}, "sid": "s"}], + tmp_path, + ) + assert "STALE" in out["output"] + + +def test_js_strict_fires_once_per_session(tmp_path, extracted_js): + outs = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [ + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "AGENTS.md")}, "sid": "s1"}, + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "AGENTS.md")}, "sid": "s1"}, + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "AGENTS.md")}, "sid": "s2"}, + ], + tmp_path, + env={"GRAPHIFY_HOOK_STRICT": "1", "GRAPHIFY_HOOK_STRICT_TTL": "0"}, + ) + assert "strict mode" in outs[0]["output"] + assert "strict mode" not in outs[1]["output"] + assert "MANDATORY" in outs[1]["output"] + assert "strict mode" in outs[2]["output"] + + +def test_js_grep_tool_gets_search_nudge(tmp_path, extracted_js): + (out,) = _run_plugin(_plugin_path(extracted_js, tmp_path), [{"hook": "after", "tool": "grep", "args": {"pattern": "p"}, "sid": "s"}], tmp_path) + assert "MANDATORY" in out["output"] + + +def test_js_no_graph_no_nudges(tmp_path, extracted_js): + outs = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [ + {"hook": "before", "tool": "bash", "command": "grep -rn foo .", "sid": "s"}, + {"hook": "after", "tool": "read", "args": {"file_path": str(tmp_path / "AGENTS.md")}, "sid": "s"}, + ], + tmp_path, + graph=False, + ) + assert outs[0]["command"] == "grep -rn foo ." + assert outs[1]["output"] == "" + + +def test_js_echo_is_shell_inert(tmp_path, extracted_js): + (out,) = _run_plugin( + _plugin_path(extracted_js, tmp_path), + [{"hook": "before", "tool": "bash", "command": "grep -rn foo .", "sid": "s"}], + tmp_path, + ) + cmd = out["command"] + assert cmd.startswith("echo '") + echo_part = cmd[: cmd.index("' ; ")] + assert "`" not in echo_part + assert "$" not in echo_part.replace("' ;", "") + assert cmd.split("' ; ", 1)[1] == "grep -rn foo ." \ No newline at end of file From d7c0b9702e8cb3b41f8bc70facbf9e438895c3ff Mon Sep 17 00:00:00 2001 From: John Dilts Date: Mon, 21 Sep 2026 10:10:14 -0400 Subject: [PATCH 2/2] fix(hooks): skip positional wrapper args in search guard, cap oriented session set Addresses the two confirmed graphify-bot findings on #3660: - Wrapper parsing missed timeout-wrapped searches: 'timeout 10 grep -rn foo .' resolved the positional duration token, broke out of the segment, and never fired the search guard. Both the Python guard (_bash_invokes_search) and the JS plugin port (bashInvokesSearch) now skip positional wrapper arguments after the wrapper's flags, stopping at the wrapped command (a search tool, git, a path, or VAR=value), so 'timeout -k 5 10 rg foo' and 'nice -n 5 find .' fire while 'timeout 10 cargo build' stays quiet. - The plugin's per-session 'oriented' Set grew without eviction in a long-lived server process. Capped at 256 sessionIDs with FIFO eviction, trading at most one extra echo per evicted session for bounded memory (the same spirit as the strict mode's 24h disk eviction). The third finding (stray top-level name in test_opencode_plugin_hook.py) was checked and is a false positive: the module imports cleanly and all 38 tests passed on the pre-fix tree. Regression tests added to both tests/test_hook_guard_token_match.py and tests/test_opencode_plugin_hook.py mirroring the #3121 contract. --- graphify/cli.py | 5 +++++ graphify/install.py | 9 +++++++++ tests/test_hook_guard_token_match.py | 6 ++++++ tests/test_opencode_plugin_hook.py | 28 +++++++++++++++++++++++++++- 4 files changed, 47 insertions(+), 1 deletion(-) diff --git a/graphify/cli.py b/graphify/cli.py index feecee3841..b786b08899 100644 --- a/graphify/cli.py +++ b/graphify/cli.py @@ -800,6 +800,11 @@ def _bash_invokes_search(cmd_str: str) -> bool: # skip the wrapper's own flags (`xargs -0`, `env -i`) while i < len(tokens) and tokens[i].startswith("-"): i += 1 + # skip positional wrapper arguments (`timeout 10`, + # `timeout -k 5 10`, `nice -n 5`): non-flag tokens until the + # wrapped command (a search tool, git, a path or VAR=value) + while i < len(tokens) and not tokens[i].startswith("-") and tokens[i].lower() not in _SEARCH_COMMANDS and tokens[i].lower() != "git" and not re.search(r"[/=]", tokens[i]): + i += 1 continue if name in _SEARCH_COMMANDS: return True diff --git a/graphify/install.py b/graphify/install.py index a6cf65d8bb..2d098e591b 100644 --- a/graphify/install.py +++ b/graphify/install.py @@ -1510,6 +1510,10 @@ def _uninstall_kilo_plugin(project_dir: Path) -> None: if (COMMAND_WRAPPERS.has(name)) { i++; while (i < tokens.length && tokens[i].startsWith("-")) i++; + // skip positional wrapper arguments (`timeout 10`, `timeout -k 5 10`, + // `nice -n 5`): non-flag tokens until the wrapped command (a search + // tool, git, a path or VAR=value) + while (i < tokens.length && !tokens[i].startsWith("-") && !SEARCH_COMMANDS.has(tokens[i].toLowerCase()) && tokens[i].toLowerCase() !== "git" && !/[/=]/.test(tokens[i])) i++; continue; } if (SEARCH_COMMANDS.has(name)) return true; @@ -1598,6 +1602,10 @@ def _uninstall_kilo_plugin(project_dir: Path) -> None: export const GraphifyPlugin = async ({ directory }) => { const graphPath = join(directory, OUT, "graph.json"); const oriented = new Set(); + // Cap the per-session set so a long-lived server process can't grow it + // without bound (one sessionID per opencode session, but the plugin module + // outlives any single session). + const ORIENTED_CAP = 256; function inProject(v) { if (!isAbsolute(v)) return true; // relative paths anchor at the project cwd @@ -1649,6 +1657,7 @@ def _uninstall_kilo_plugin(project_dir: Path) -> None: const nudges = []; if (!oriented.has(input.sessionID)) { oriented.add(input.sessionID); + if (oriented.size > ORIENTED_CAP) oriented.delete(oriented.values().next().value); nudges.push(ORIENT_ECHO); } if (cmd && bashInvokesSearch(cmd)) nudges.push(SEARCH_ECHO); diff --git a/tests/test_hook_guard_token_match.py b/tests/test_hook_guard_token_match.py index cb399b4e79..c26c269f31 100644 --- a/tests/test_hook_guard_token_match.py +++ b/tests/test_hook_guard_token_match.py @@ -39,6 +39,9 @@ "FOO=1 grep x f", "echo done; rg leftover", "result=$(grep -c x f)", + "timeout 10 grep -rn foo .", + "timeout -k 5 10 rg foo", + "nice -n 5 grep x f", ]) def test_real_searches_fire(cmd): assert _bash_invokes_search(cmd) is True @@ -57,6 +60,9 @@ def test_real_searches_fire(cmd): "echo 'grep is a fine tool'", "printf 'use find sparingly'", "magick convert x.png y.jpg", + "timeout 10 cargo build", # a wrapper with a positional arg, + "timeout 5 make test", # but no search behind it, stays quiet + "nice 5 python run.py", # 'nice' without flags, non-search target ]) def test_prose_and_lookalikes_stay_quiet(cmd): assert _bash_invokes_search(cmd) is False diff --git a/tests/test_opencode_plugin_hook.py b/tests/test_opencode_plugin_hook.py index ad2ce47cf2..0f5f36291b 100644 --- a/tests/test_opencode_plugin_hook.py +++ b/tests/test_opencode_plugin_hook.py @@ -141,6 +141,9 @@ def _plugin_path(extracted_js: str, tmp_path: Path) -> str: "FOO=1 grep x f", "echo done; rg leftover", "result=$(grep -c x f)", + "timeout 10 grep -rn foo .", + "timeout -k 5 10 rg foo", + "nice -n 5 grep x f", ] QUIET = [ "git commit -m x", @@ -155,6 +158,9 @@ def _plugin_path(extracted_js: str, tmp_path: Path) -> str: "echo 'grep is a fine tool'", "printf 'use find sparingly'", "magick convert x.png y.jpg", + "timeout 10 cargo build", # a wrapper with a positional arg, + "timeout 5 make test", # but no search behind it, stays quiet + "nice 5 python run.py", # 'nice' without flags, non-search target ] @@ -306,4 +312,24 @@ def test_js_echo_is_shell_inert(tmp_path, extracted_js): echo_part = cmd[: cmd.index("' ; ")] assert "`" not in echo_part assert "$" not in echo_part.replace("' ;", "") - assert cmd.split("' ; ", 1)[1] == "grep -rn foo ." \ No newline at end of file + assert cmd.split("' ; ", 1)[1] == "grep -rn foo ." + + +def test_js_oriented_set_is_capped(tmp_path, extracted_js): + """The in-memory oriented set is capped so a long-lived server process + can't grow it unbounded. Eviction trades one extra echo for bounded + memory: an evicted session re-fires exactly once, then goes quiet again — + the once-per-session budget still holds.""" + calls = [ + {"hook": "before", "tool": "bash", "command": "echo hi", "sid": f"s{i}"} + for i in range(600) + ] + [ + {"hook": "before", "tool": "bash", "command": "echo again", "sid": "s7"}, + {"hook": "before", "tool": "bash", "command": "echo once more", "sid": "s7"}, + ] + outs = _run_plugin(_plugin_path(extracted_js, tmp_path), calls, tmp_path) + # s7's first call (index 7) consumed its echo; the cap (256) evicted it + # long before call 600, so this re-fire is the eviction tradeoff. + assert "knowledge graph" in outs[600]["command"] + # Re-added on that call, the echo is again once-per-session: quiet now. + assert "knowledge graph" not in outs[601]["command"] \ No newline at end of file