diff --git a/plugins/concise/.claude/concise.json.example b/plugins/concise/.claude/concise.json.example index 18ee219..27db28c 100644 --- a/plugins/concise/.claude/concise.json.example +++ b/plugins/concise/.claude/concise.json.example @@ -20,6 +20,16 @@ "fileSize": true, "prBody": true }, + "scan": { + "codeFiles": true, + "notebooks": true, + "heredocWrites": true, + "shellWrites": true, + "mcp": true, + "plans": true, + "tasks": true, + "questions": true + }, "stopHook": true, "softFail": false, "styleIgnoreGlobs": [ diff --git a/plugins/concise/README.md b/plugins/concise/README.md index 6529fdc..ce814ba 100644 --- a/plugins/concise/README.md +++ b/plugins/concise/README.md @@ -140,7 +140,21 @@ The 47 categories fall into 5 groups. The full table, with every category id, its presets, and the scopes it runs in, is in [docs/categories.md](docs/categories.md). The flagged text and the replacement for every pattern are in [skills/concise-rules/references/ai-speak-patterns.md](skills/concise-rules/references/ai-speak-patterns.md). -What gets scanned: prose files (`md`, `mdx`, `markdown`, `txt`, `rst`, `adoc`, `asciidoc`) whole, with fenced blocks, inline code, URLs, and HTML comments blanked first; comment runs in code files; `gh pr` and `gh issue` bodies; `git commit` messages from `-m`, `--message=`, and the heredoc form; and the agent's final reply through the `Stop` hook when `replies` is true. +What gets scanned: + +- Prose files (`md`, `mdx`, `markdown`, `txt`, `rst`, `adoc`, `asciidoc`) whole, with fenced blocks, inline code, URLs, and HTML comments blanked first. +- Comment runs in code files. Packs with the `code` scope, `hidden-characters` by default, read the whole file, string literals included. +- Notebook cells: a markdown cell as prose, a code cell as code. +- Files a shell command writes: a `cat` or `tee` heredoc before the command runs, and any other file the command changed in a git work tree after it runs. +- `gh pr`, `gh issue`, and `gh release` titles and bodies, inline, heredoc, or from a body file, and `gh api` request bodies. +- `git commit`, `git tag`, `git merge`, `git notes`, `jj`, and `hg` messages from `-m`, `--message=`, `-F`, `--trailer`, and the heredoc form. +- Text and files that MCP tools post or write, such as a PR body, a chat message, a doc page, or a pushed file. +- The plan that `ExitPlanMode` shows for approval, task text, and questions that `AskUserQuestion` asks you. +- The agent's final reply through the `Stop` hook when `replies` is true. + +Each place past Write and Edit has an on-by-default switch under `scan`, listed in [docs/configuration.md](docs/configuration.md). + +Not scanned: the agent's text between tool calls, because no hook can return it to the agent; page text in HTML files; and shell writes outside a git work tree. In Codex, shell writes are checked only for `cat` and `tee` heredocs. The omp extension checks neither MCP tools nor shell writes. ## Cloud agents and environment variables diff --git a/plugins/concise/docs/INDEX.md b/plugins/concise/docs/INDEX.md index 798be36..f35368b 100644 --- a/plugins/concise/docs/INDEX.md +++ b/plugins/concise/docs/INDEX.md @@ -5,11 +5,11 @@ Last updated: 2026-10-08 | Doc | Read it to | Created | Last updated | |---|---|---|---| -| [configuration.md](configuration.md) | Find a config key, its default, the layer order, the merge rules, `mode`, and the dictionary. | 2026-09-04 | 2026-10-06 | +| [configuration.md](configuration.md) | Find a config key, its default, the layer order, the merge rules, `mode`, and the dictionary. | 2026-09-04 | 2026-10-08 | | [tools.md](tools.md) | Change settings with the `concise-config` CLI or the MCP tools, or tune them from writing samples. | 2026-10-05 | 2026-10-05 | -| [environment.md](environment.md) | Control a session with `BEC_` variables. | 2026-09-04 | 2026-10-05 | +| [environment.md](environment.md) | Control a session with `BEC_` variables. | 2026-09-04 | 2026-10-08 | | [categories.md](categories.md) | Look up an AI writing category, its presets, and its scopes. | 2026-09-04 | 2026-10-08 | | [packs.md](packs.md) | Write or check a pattern pack. | 2026-09-04 | 2026-10-08 | -| [host-features.md](host-features.md) | Check which Claude Code, Codex, and omp hook, skill, and MCP features the manifests and the omp extension use, with the host doc sources and the live test results. | 2026-10-05 | 2026-10-07 | +| [host-features.md](host-features.md) | Check which Claude Code, Codex, and omp hook, skill, and MCP features the manifests and the omp extension use, with the host doc sources and the live test results. | 2026-10-05 | 2026-10-08 | Read [host-features.md](host-features.md) and its sources before you change `hooks/hooks.json`, `hooks/codex.json`, `.mcp.json`, `.codex-mcp.json`, `package.json`, `omp/`, or a plugin manifest. diff --git a/plugins/concise/docs/categories.md b/plugins/concise/docs/categories.md index b837ab3..8f5d223 100644 --- a/plugins/concise/docs/categories.md +++ b/plugins/concise/docs/categories.md @@ -4,7 +4,7 @@ The `aiWriting` feature ships 47 categories. Each category is one pack file unde Pick categories with `features.aiWriting.preset`, or replace the preset list with `features.aiWriting.categories`. Both keys are described in [configuration.md](configuration.md). The flagged text and the replacement for every pattern are in [../skills/concise-rules/references/ai-speak-patterns.md](../skills/concise-rules/references/ai-speak-patterns.md). -Scope names: `files` (prose files), `comments` (comment runs in code files), `gh` (`gh pr` and `gh issue` bodies), `commit` (`git commit` messages), `command` (the full command line), `reply` (the agent's final reply). +Scope names: `files` (prose files and plans), `comments` (comment runs in code files), `code` (whole code files, string literals included), `gh` (`gh pr`, `gh issue`, and `gh release` titles and bodies, `gh api` bodies, and MCP tool posts), `commit` (`git commit`, `git tag`, `git merge`, `jj`, and `hg` messages), `command` (the full command line), `reply` (the agent's final reply, task text, and questions to the user). ## Phrase categories @@ -63,11 +63,11 @@ Scopes across this group: `comments`, `gh`, `commit`, `command`, `reply`. The sc ## Punctuation categories -Scopes across this group: `files`, `comments`, `gh`, `commit`, `command`, `reply`. The scope column below gives the scopes for each category. +Scopes across this group: `files`, `comments`, `code`, `gh`, `commit`, `command`, `reply`. The scope column below gives the scopes for each category. | id | What it catches | Flagged example | Fix | Presets | Scopes | |---|---|---|---|---|---| -| `hidden-characters` | Invisible or look-alike characters that model output and chat UIs leave in text. | a zero-width space | remove the character | `default`, `ryan`, `technical`, `git`, `all` | `files`, `comments`, `gh`, `commit`, `command`, `reply` | +| `hidden-characters` | Invisible or look-alike characters that model output and chat UIs leave in text. | a zero-width space | remove the character | `default`, `ryan`, `technical`, `git`, `all` | `files`, `code`, `gh`, `commit`, `command`, `reply` | | `smart-punctuation` | Curly quotes and the ellipsis character typical of text pasted from a chat UI. | a curly apostrophe | use a straight apostrophe | `default`, `ryan`, `technical`, `all` | `comments`, `commit`, `gh`, `command`, `reply` | | `unicode-glyphs` | Arrows, box drawing, dingbats, and geometric symbols used in place of plain ASCII or words. | an arrow character | use `->` | `default`, `ryan`, `technical`, `all` | `commit`, `gh`, `command`, `reply` | diff --git a/plugins/concise/docs/configuration.md b/plugins/concise/docs/configuration.md index 78c68d7..2921cdd 100644 --- a/plugins/concise/docs/configuration.md +++ b/plugins/concise/docs/configuration.md @@ -15,6 +15,14 @@ Copy `.claude/concise.json.example` to `.claude/concise.json` (Claude Code) or ` | `checks.comments` | `true` | Runs the comment length check. | | `checks.fileSize` | `true` | Runs the file length check. | | `checks.prBody` | `true` | Runs the `gh` body check. | +| `scan.codeFiles` | `true` | Runs `code`-scope packs and dictionary entries over whole code files, string literals included. | +| `scan.notebooks` | `true` | Checks notebook cells that `NotebookEdit` writes. | +| `scan.heredocWrites` | `true` | Checks files that `cat` or `tee` writes from a heredoc, before the command runs. | +| `scan.shellWrites` | `true` | Checks the lines a shell command added to files in a git work tree, after it runs. | +| `scan.mcp` | `true` | Checks the text and files that MCP tools post or write. | +| `scan.plans` | `true` | Checks the plan that `ExitPlanMode` shows for approval. | +| `scan.tasks` | `true` | Checks `TaskCreate` and `TaskUpdate` text. | +| `scan.questions` | `true` | Checks `AskUserQuestion` questions and options. | | `stopHook` | `true` | Runs reply checks on `Stop` and `SubagentStop`. | | `context.enabled` | `true` | Sends resolved rules at startup, resume, compaction, and subagent creation. | | `context.perTurn` | `false` | Also sends rules on `UserPromptSubmit`. | @@ -78,7 +86,7 @@ A project file overrides `BEC_FEATURE_ENABLE`. `BEC_FEATURE_ALWAYS_ENABLE` overr A config file or a `BEC_CONFIG_JSON` value that does not parse as a JSON object is skipped. The other layers still apply, and the hook reports the skipped layer once per session in a `systemMessage`. -## checks and stopHook +## checks, scan, and stopHook Each core check has its own switch under `checks`: `comments`, `fileSize`, and `prBody`, all `true` by default. Setting one to `false` skips that check and leaves the style checks running. @@ -89,6 +97,16 @@ Each core check has its own switch under `checks`: `comments`, `fileSize`, and ` } ``` +Each place the style checks read beyond Write and Edit has a switch under `scan`, all `true` by default. Setting one to `false` stops the style checks there and leaves the rest running. The README lists what each one covers. + +`scan.shellWrites` runs on `PostToolUse`, so the text is already on disk and the findings come back next to the command output. It reads files whose mtime falls inside the command's run time, at most 20 per command, and reports each finding once per session. It compares the work tree with the git index: older unstaged lines in a changed file are reported once too, and a change the command also staged is missed. + +```json +{ + "scan": { "shellWrites": false, "tasks": false } +} +``` + `"stopHook": false` skips `Stop` and `SubagentStop` reply checks before reading transcripts. `SubagentStop` uses `last_assistant_message`, then `agent_transcript_path`. Retry and confirmation state is separate for each session and agent. `SessionEnd` removes that session's state within a bounded cleanup window. ## Lifecycle context and host hooks @@ -159,7 +177,7 @@ The evaluator checks reply padding with the resolved rules, bypasses, soft-fail | `caseSensitive` | no | `false` by default. | | `flags` | no | Regex flags for `regex` entries. Default `i`. The scanner adds `g` and `d` and drops `y`. | | `hooks` | no | `edit`, `bash`, `stop`, `subagentStop`. Absent means all 4. | -| `scopes` | no | `files`, `comments`, `gh`, `commit`, `command`, `reply`. Absent means every scope except `command`. | +| `scopes` | no | `files`, `comments`, `code`, `gh`, `commit`, `command`, `reply`. Absent means every scope except `command` and `code`. | | `enabled` | no | `false` switches the entry off. A switched-off entry needs only its `id`. | How `on` changes the match: diff --git a/plugins/concise/docs/environment.md b/plugins/concise/docs/environment.md index 84e4b02..5c38edc 100644 --- a/plugins/concise/docs/environment.md +++ b/plugins/concise/docs/environment.md @@ -40,7 +40,7 @@ The layer column refers to the 5 config layers in [configuration.md](configurati | `BEC_BYPASS_PHRASES` | phrase list | `bypass.phrases` | 5 | | `BEC_BYPASS_PATTERNS` | regex list | `bypass.patterns` | 5 | -Feature ids for `BEC_FEATURE_*`: `emDash`, `aiWriting`, `dictionary`, `comments`, `fileSize`, `prBody`, and `stopHook`. +Feature ids for `BEC_FEATURE_*`: `emDash`, `aiWriting`, `dictionary`, `comments`, `fileSize`, `prBody`, `stopHook`, and the `scan` switches `codeFiles`, `notebooks`, `heredocWrites`, `shellWrites`, `mcp`, `plans`, `tasks`, and `questions`. Pattern ids for `BEC_*_PATTERNS`: a category id, a pack id, or `tag:`. An unknown id is ignored. The category ids are listed in [categories.md](categories.md). diff --git a/plugins/concise/docs/host-features.md b/plugins/concise/docs/host-features.md index 1434a39..c5cf7f0 100644 --- a/plugins/concise/docs/host-features.md +++ b/plugins/concise/docs/host-features.md @@ -1,9 +1,9 @@ # Host features Created: 2026-10-05 -Last updated: 2026-10-07 +Last updated: 2026-10-08 -The Claude Code and Codex features the concise manifests use, as the host docs described them on 2026-10-05, and the omp features the omp extension uses, as of 2026-10-07. Read the sources again before you change a manifest or `omp/extension.mjs`. +The Claude Code and Codex features the concise manifests use, as the host docs described them on 2026-10-05 (2026-10-08 for the `NotebookEdit`, `PowerShell`, MCP, task, question, and `PostToolUse` rows), and the omp features the omp extension uses, as of 2026-10-07. Read the sources again before you change a manifest or `omp/extension.mjs`. ## Used by the manifests @@ -12,12 +12,20 @@ The Claude Code and Codex features the concise manifests use, as the host docs d | `statusMessage` on a command handler | yes | yes | Every handler in `hooks/hooks.json` and `hooks/codex.json` | | `SessionStart` matcher `fork` | yes | no: `startup`, `resume`, `clear`, `compact` | `hooks/hooks.json` | | `if` on a handler | yes | no | `hooks/hooks.json` shell hooks only | +| `if` with a permission rule per handler, such as `Bash(cat *)`, matched against each subcommand | yes | no | `hooks/hooks.json`: the heredoc-write handlers of `check-edit`, the `jj` handlers of `check-bash`, and the `PowerShell` handlers | +| `NotebookEdit` input: `notebook_path`, `cell_id`, `new_source`, `cell_type`, `edit_mode` | yes | no | `hooks/hooks.json` `PreToolUse`, read by `check-edit.mjs` | +| `PowerShell` tool with `tool_input.command` | yes | no | `hooks/hooks.json` `PreToolUse`, read by `check-bash.mjs` | +| A matcher with characters outside letters, digits, `_`, `-`, `\|`, and `,` is a regex, so `mcp__.*` matches every `mcp____` call | yes | yes: every matcher is a regex, and `PreToolUse` fires for MCP tools | `PreToolUse` in both manifests, read by `check-tool-text.mjs` | +| `PreToolUse` on `ExitPlanMode` (`plan`, which Claude Code fills from the plan file), on `TaskCreate` and `TaskUpdate` (`subject`, `description`, `activeForm`) and on `AskUserQuestion` (`questions[].question`, `options[].label`, `options[].description`) | yes | no | `hooks/hooks.json`, read by `check-tool-text.mjs` | +| `PostToolUse` input with `duration_ms`, and `decision: "block"`, which adds the reason next to the tool result | yes | not used | `hooks/hooks.json` `PostToolUse` on `Bash\|PowerShell`, read by `check-shell-writes.mjs` | | `SubagentHandback` tool carries the subagent report in `tool_input.message` (auto mode, v2.1.271 or later) | yes | no | `hooks/hooks.json` `PreToolUse` | | `SubagentStop` fires for internal agents, with an empty `agent_type` when the session has no `--agent` | yes | no | `check-reply.mjs` skips them | | Plugin MCP server file with `${CLAUDE_PLUGIN_ROOT}` in `args` | yes | no: see below | `.mcp.json` (Claude Code), `.codex-mcp.json` (Codex) | | `${CLAUDE_PLUGIN_ROOT}` in plugin skill text | yes | not documented | `skills/*/SKILL.md`, with a relative path for Codex | | `interface.composerIcon` and `interface.logo` | n/a | yes | `.codex-plugin/plugin.json` | +The hooks docs say a handler defined in more than one settings file runs once. They do not say whether two handlers in one plugin with the same command and different `if` rules both run for one call. When their rules can overlap, `check-edit` and `check-bash` claim the call's `tool_use_id`, so a second run exits without output. + ## Codex MCP server The Codex plugin docs describe bundled MCP servers and plugin-scoped policy under `plugins..mcp_servers.`, with a remote HTTP server as the example. They do not cover local stdio paths. A live run with codex-cli 0.159.0 on 2026-10-05, with the plugin installed from this repository into a temporary `CODEX_HOME`, showed: @@ -54,13 +62,15 @@ A live run with omp 18.7.0 on 2026-10-07 confirmed these shapes. In that run, a omp's Claude marketplace loader, `src/discovery/claude-plugins.ts`, substitutes `${CLAUDE_PLUGIN_ROOT}` in MCP `args` and loads `skills/`. The omp docs do not mention `CLAUDE_PROJECT_DIR`, so pass `cwd` to the project tools as in Codex. -Gaps: omp has no subagent stop event, so subagent replies are not checked. omp blocks a tool call whose handler throws or runs past 30 s, so the extension catches every hook failure and allows the call. omp loads extensions at session start: start a new session after an install or update. +Gaps: omp has no subagent stop event, so subagent replies are not checked. The extension does not check MCP tool calls, and it does not scan files that a shell command changes. omp blocks a tool call whose handler throws or runs past 30 s, so the extension catches every hook failure and allows the call. omp loads extensions at session start: start a new session after an install or update. ## Left out - Plugin `bin/` on `PATH` (Claude Code): claude.ai and Cowork refuse a plugin with a `bin/` directory. - `userConfig` prompts at install time (Claude Code): deferred. - `PostToolBatch`, `additionalContext` on `Stop`, and exec-form `args` (Claude Code): no current use. +- `MessageDisplay` (Claude Code): it only changes what the terminal shows. It cannot block a message or send text to Claude, so text between tool calls stays unchecked. +- `TaskCreated` (Claude Code): the `PreToolUse` hook on `TaskCreate` reads the same subject and description. ## Sources diff --git a/plugins/concise/docs/packs.md b/plugins/concise/docs/packs.md index 660cca0..aa26204 100644 --- a/plugins/concise/docs/packs.md +++ b/plugins/concise/docs/packs.md @@ -61,14 +61,17 @@ Every `'` inside a `phrase` or an `opening` compiles to `['’]`, so the straigh | Scope | Text scanned | Hook | |---|---|---| -| `files` | A prose file, whole (`md`, `mdx`, `markdown`, `txt`, `rst`, `adoc`, `asciidoc`) | `check-edit.mjs` | -| `comments` | Comment runs in a code file | `check-edit.mjs` | -| `gh` | A `gh pr` or `gh issue` body | `check-bash.mjs` | -| `commit` | A `git commit` message | `check-bash.mjs` | -| `command` | The full `git commit` or `gh` command line | `check-bash.mjs` | -| `reply` | The agent's final reply | `check-reply.mjs` | - -Leaving `scope` out gives the pack every scope except `command`. +| `files` | A prose file, whole (`md`, `mdx`, `markdown`, `txt`, `rst`, `adoc`, `asciidoc`), and the plan `ExitPlanMode` shows | `check-edit.mjs`, `check-shell-writes.mjs`, `check-tool-text.mjs` | +| `comments` | Comment runs in a code file | `check-edit.mjs`, `check-shell-writes.mjs`, `check-tool-text.mjs` | +| `code` | A code file or a file of unknown type, whole, string literals included | `check-edit.mjs`, `check-shell-writes.mjs`, `check-tool-text.mjs` | +| `gh` | A `gh pr`, `gh issue`, or `gh release` title or body, a `gh api` request body, and the text an MCP tool posts | `check-bash.mjs`, `check-tool-text.mjs` | +| `commit` | A `git commit`, `git tag`, `git merge`, `git notes`, `jj`, or `hg` message, and the commit message of an MCP file push | `check-bash.mjs`, `check-tool-text.mjs` | +| `command` | The full `git` or `gh` command line | `check-bash.mjs` | +| `reply` | The agent's final reply, task text, and questions to the user | `check-reply.mjs`, `check-tool-text.mjs` | + +`check-edit.mjs` also reads a file that `cat` or `tee` writes from a heredoc, and a notebook cell: a markdown cell as a prose file, a code cell as a code file. `check-shell-writes.mjs` reads the lines a shell command added to files in a git work tree, after the command runs. `check-tool-text.mjs` reads a file an MCP tool writes the same way. + +Leaving `scope` out gives the pack every scope except `command` and `code`. ## presets semantics diff --git a/plugins/concise/hooks/check-bash.mjs b/plugins/concise/hooks/check-bash.mjs index 9e1a8c7..b4afed2 100644 --- a/plugins/concise/hooks/check-bash.mjs +++ b/plugins/concise/hooks/check-bash.mjs @@ -1,31 +1,59 @@ #!/usr/bin/env node import { createHash } from "node:crypto"; import { loadConfig } from "./lib/config.mjs"; -import { extractBody, isVerbose } from "./lib/pr-body.mjs"; +import { GH_COMMAND, apiFields, extractBody, extractTitle, isVerbose, maskHeredocs } from "./lib/pr-body.mjs"; import { gitCommitMessages } from "./lib/prose.mjs"; import { bumpAttempt, resetAttempt } from "./lib/state.mjs"; -import { deny, mergeFlag } from "./lib/respond.mjs"; +import { ask, deny, mergeFlag } from "./lib/respond.mjs"; import { styleDecisionForText, prepareStyle, withPackWarnings } from "./lib/style-check.mjs"; import { runHook, bypassResult } from "./lib/hook-main.mjs"; -const GH_PATTERN = /\bgh\s+(pr|issue)\s+(create|comment|edit)\b/; +// Release notes and merge-commit bodies get the style check but not the PR prose limit. +const PROSE_LIMITED = /\bgh\s+(?:pr|issue)\s+(?:create|comment|edit|review)\b/; +const SHELL_TOOLS = ["Bash", "PowerShell"]; const shortHash = (text) => createHash("sha256").update(text).digest("hex").slice(0, 12); -// Keyed on the command minus its body, so revisions of the same PR share a counter -// while an unrelated PR later in the session starts fresh. -const scaffoldHash = (command, bodies) => shortHash(bodies.reduce((acc, body) => acc.replace(body, ""), command)); +// Keyed on the command minus its heredocs and the texts a check reads, so revisions share a counter. +// Longest first, so a title repeated inside the body can't spoil the body's removal. +const scaffoldHash = (command, texts) => + shortHash(texts.filter(Boolean).sort((a, b) => b.length - a.length).reduce((acc, text) => acc.replace(text, ""), maskHeredocs(command).masked)); + +function labelOf(command, part) { + const kind = /\bgh\s+release\b/.test(command) ? "release" : /\bgh\s+issue\b/.test(command) ? "issue" : "PR"; + if (part === "title") return /\bgh\s+pr\s+merge\b/.test(command) ? "merge subject" : `${kind} title`; + return kind === "release" ? "release notes" : `${kind} body`; +} + +function textDecision(texts, key, label, scope, input, config) { + const text = texts.filter(Boolean).join("\n\n"); + if (!text || text.includes("concise-ignore")) return {}; + return styleDecisionForText(text, key, label, input, config, "PreToolUse", scope, "bash"); +} + +// The title and body are scanned apart, so line numbers and whole-text packs see each as written, +// and one held call carries both reasons, so one round fixes both. +function joined(results, input) { + const held = results.filter((r) => r.hookSpecificOutput?.permissionDecisionReason); + if (held.length < 2) return combine(...results); + const reason = held.map((r) => r.hookSpecificOutput.permissionDecisionReason).join("\n\n"); + return held.some((r) => r.hookSpecificOutput.permissionDecision === "deny") ? deny(reason) : ask(reason, input); +} function ghDecision(command, input, config) { - const body = extractBody(command); - if (!body || body.includes("concise-ignore")) return {}; + const body = extractBody(command, input.cwd); + const title = extractTitle(command); + if (!(title || body) || [title, body].join("\n").includes("concise-ignore")) return {}; - const digest = scaffoldHash(command, [body]); - const key = `pr-body:${digest}`; - const label = /\bgh\s+issue\b/.test(command) ? "issue body" : "PR body"; - const styled = () => styleDecisionForText(body, `style:gh:${digest}`, label, input, config, "PreToolUse", "gh", "bash"); + const digest = scaffoldHash(command, [title, body]); + const styled = () => joined([ + textDecision([title], `style:gh-title:${digest}`, labelOf(command, "title"), "gh", input, config), + textDecision([body], `style:gh:${digest}`, labelOf(command, "body"), "gh", input, config), + ], input); - const off = (config.checks || {}).prBody === false; + // The prose limit reads only the body, so its counter keeps the title in the key. + const key = `pr-body:${scaffoldHash(command, [body])}`; + const off = !body || (config.checks || {}).prBody === false || !PROSE_LIMITED.test(command); const result = off ? { verbose: false } : isVerbose(body, { maxParagraphs: config.maxPrBodyParagraphs, maxSentences: config.maxPrBodySentences }); @@ -44,13 +72,6 @@ function ghDecision(command, input, config) { return mergeFlag(flagText, styled()); } -function commitDecision(command, messages, input, config) { - const text = messages.join("\n\n"); - if (text.includes("concise-ignore")) return {}; - const key = `style:commit:${scaffoldHash(command, messages)}`; - return styleDecisionForText(text, key, "commit message", input, config, "PreToolUse", "commit", "bash"); -} - // Packs scoped to `command` also see the flags and trailers around the message. function commandDecision(command, input, config) { if (command.includes("concise-ignore")) return {}; @@ -64,19 +85,28 @@ function combine(...results) { } async function decide(input, ctx) { - if (input.tool_name !== "Bash") return {}; + if (!SHELL_TOOLS.includes(input.tool_name)) return {}; const command = (input.tool_input || {}).command || ""; - const isGh = GH_PATTERN.test(command); - const messages = isGh ? [] : gitCommitMessages(command); - if (!isGh && messages.length === 0) return {}; + const isGh = GH_COMMAND.test(command); + const fields = isGh ? [] : apiFields(command, input.cwd); + const messages = isGh ? [] : gitCommitMessages(command, input.cwd); + if (!isGh && fields.length + messages.length === 0) return {}; const config = loadConfig(input.cwd); ctx.config = config; const bypassed = bypassResult(command, config, ctx); if (bypassed) return bypassed; await prepareStyle(input.cwd, config); - const main = isGh ? ghDecision(command, input, config) : commitDecision(command, messages, input, config); - return withPackWarnings(combine(main, commandDecision(command, input, config)), input.session_id); + const main = isGh + ? [ghDecision(command, input, config)] + : [ + textDecision(fields, `style:api:${scaffoldHash(command, fields)}`, "API request body", "gh", input, config), + textDecision(messages, `style:commit:${scaffoldHash(command, messages)}`, "commit message", "commit", input, config), + ]; + return withPackWarnings(combine(...main, commandDecision(command, input, config)), input.session_id); } -await runHook({ hook: "check-bash", event: "PreToolUse" }, decide); +// `*g*` and `jj *` both match a jj command that holds a "g". +const overlaps = (input) => /\bjj\s/.test(input.tool_input?.command || ""); + +await runHook({ hook: "check-bash", event: "PreToolUse", overlaps }, decide); diff --git a/plugins/concise/hooks/check-edit.mjs b/plugins/concise/hooks/check-edit.mjs index e94677b..4e06170 100644 --- a/plugins/concise/hooks/check-edit.mjs +++ b/plugins/concise/hooks/check-edit.mjs @@ -1,65 +1,16 @@ #!/usr/bin/env node -import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; import { loadConfig, isIgnored } from "./lib/config.mjs"; import { scanComments } from "./lib/comment-scan.mjs"; -import { extractPatch, parseApplyPatch } from "./lib/apply-patch.mjs"; +import { targetsOf, isExempt } from "./lib/edit-targets.mjs"; import { bumpAttempt, resetAttempt } from "./lib/state.mjs"; import { deny, mergeFlag } from "./lib/respond.mjs"; import { styleDecision, prepareStyle, withPackWarnings } from "./lib/style-check.mjs"; import { runHook, bypassResult } from "./lib/hook-main.mjs"; -const EDIT_TOOLS = ["Write", "Edit", "MultiEdit"]; - -// Only the text this call writes, so a one-line edit isn't blamed for what's already -// on disk. Chunks stay separate: joining them could invent a comment run. -function writtenChunks(toolName, toolInput) { - if (toolName === "Write") return [toolInput.content || ""]; - if (toolName === "Edit") return [toolInput.new_string || ""]; - return (toolInput.edits || []).map((edit) => edit.new_string || ""); -} - -// Claude Code sends Write/Edit/MultiEdit; Codex sends apply_patch (or a shell heredoc -// carrying one). Both become { path, chunks, wholeFile } targets. -function targetsOf(input) { - const toolName = input.tool_name; - const toolInput = input.tool_input || {}; - - if (EDIT_TOOLS.includes(toolName)) { - if (!toolInput.file_path) return []; - return [{ path: toolInput.file_path, chunks: writtenChunks(toolName, toolInput), wholeFile: toolName === "Write" }]; - } - - let patch = null; - if (toolName === "apply_patch") patch = toolInput.command || toolInput.input || ""; - if (toolName === "Bash") patch = extractPatch(toolInput.command); - if (!patch) return []; - - return parseApplyPatch(patch).map((file) => ({ - path: resolve(input.cwd || ".", file.path), - chunks: file.chunks, - wholeFile: file.kind === "add", - })); -} - -function hasFileMarker(filePath) { - try { - return readFileSync(filePath, "utf8").includes("concise-ignore-file"); - } catch { - return false; - } -} - function firstLineOf(text) { return text.split("\n")[0].trim().slice(0, 60); } -// A whole-file write replaces the file, so its new content is the only authority on the marker. -function isExempt({ path, chunks, wholeFile }) { - if (chunks.some((chunk) => chunk.includes("concise-ignore-file"))) return true; - return !wholeFile && hasFileMarker(path); -} - function checkTarget(target, config) { const { path, chunks, wholeFile } = target; if (isIgnored(path, config.ignoreGlobs)) return []; @@ -93,10 +44,12 @@ function checkTarget(target, config) { } async function decide(input, ctx) { - const targets = targetsOf(input); - if (targets.length === 0) return {}; + const found = targetsOf(input); + if (found.length === 0) return {}; const config = loadConfig(input.cwd); ctx.config = config; + const targets = found.filter((target) => config.scan[target.scan] !== false && !(target.file && isIgnored(target.file, config.ignoreGlobs))); + if (targets.length === 0) return {}; const bypassed = bypassResult(targets.flatMap((target) => target.chunks), config, ctx); if (bypassed) return bypassed; await prepareStyle(input.cwd, config); @@ -132,5 +85,9 @@ function check(targets, input, config, ctx) { return mergeFlag(flagText, styleDecision(styled, input, config)); } +// The Begin Patch, `cat *`, and `tee *` rules can match one command together. +const GATES = [/Begin Patch/, /\bcat\s/, /\btee\s/]; +const overlaps = (input) => GATES.filter((re) => re.test(input.tool_input?.command || "")).length > 1; + // A hook bug must never block real work: runHook turns a throw into an allow. -await runHook({ hook: "check-edit", event: "PreToolUse" }, decide); +await runHook({ hook: "check-edit", event: "PreToolUse", overlaps }, decide); diff --git a/plugins/concise/hooks/check-reply.mjs b/plugins/concise/hooks/check-reply.mjs index dc26e92..fce56a4 100755 --- a/plugins/concise/hooks/check-reply.mjs +++ b/plugins/concise/hooks/check-reply.mjs @@ -2,7 +2,7 @@ import { openSync, readSync, fstatSync, closeSync } from "node:fs"; import { loadConfig } from "./lib/config.mjs"; import { dictionaryActive } from "./lib/dictionary.mjs"; -import { styleFindings, styleSummary, styleDecisionForText, prepareStyle, withPackWarnings } from "./lib/style-check.mjs"; +import { styleFindings, styleSummary, styleDecisionForText, prepareStyle, replyConfig, withPackWarnings } from "./lib/style-check.mjs"; import { sha256 } from "./lib/confirm.mjs"; import { takePending, resetAttempt } from "./lib/state.mjs"; import { runHook, bypassResult } from "./lib/hook-main.mjs"; @@ -51,22 +51,6 @@ function lastAssistantText(path) { return null; } -// A feature that is off for replies is off for this hook, so styleFindings skips it. -// Dictionary entries pick their hooks themselves. -function replyConfig(config) { - const { emDash, aiWriting } = config.features; - return { - ...config, - ignoreGlobs: [], - styleIgnoreGlobs: [], - features: { - ...config.features, - emDash: { ...emDash, enabled: Boolean(emDash.enabled && emDash.replies) }, - aiWriting: { ...aiWriting, enabled: Boolean(aiWriting.enabled && aiWriting.replies) }, - }, - }; -} - const anyStyle = (config) => config.features.emDash.enabled || config.features.aiWriting.enabled || dictionaryActive(config); function afterBlock(text, input, config, hook) { diff --git a/plugins/concise/hooks/check-shell-writes.mjs b/plugins/concise/hooks/check-shell-writes.mjs new file mode 100644 index 0000000..47e69a5 --- /dev/null +++ b/plugins/concise/hooks/check-shell-writes.mjs @@ -0,0 +1,109 @@ +#!/usr/bin/env node +import { createHash } from "node:crypto"; +import { resolve } from "node:path"; +import { loadConfig, isIgnored } from "./lib/config.mjs"; +import { dictionaryActive } from "./lib/dictionary.mjs"; +import { gitChanges, readChanged, addedChunks, realPath } from "./lib/changed-files.mjs"; +import { isProsePath } from "./lib/prose.mjs"; +import { extractPatch, parseApplyPatch } from "./lib/apply-patch.mjs"; +import { heredocWrites, fileFlagPaths } from "./lib/shell-text.mjs"; +import { once } from "./lib/state.mjs"; +import { styleFindings, prepareStyle, withPackWarnings } from "./lib/style-check.mjs"; +import { PLAIN, parts, firedAny, referenceFor } from "./lib/style-message.mjs"; +import { runHook, bypassResult } from "./lib/hook-main.mjs"; + +const SHELL_TOOLS = ["Bash", "PowerShell"]; +const MAX_FILES = 20; +// The hook starts a moment after the tool ends, and some filesystems keep mtime in whole +// seconds, so the window opens a little before the command started. +const SLACK_MS = 2000; + +const shortHash = (text) => createHash("sha256").update(text).digest("hex").slice(0, 16); + +function styleOn(config) { + const features = config.features || {}; + return Boolean(features.emDash?.enabled || features.aiWriting?.enabled || dictionaryActive(config)); +} + +// check-edit read heredoc writes and patches before they ran; check-bash read body and message files. +function skippedPaths(command, cwd, config) { + const paths = [...fileFlagPaths(command), ...parseApplyPatch(extractPatch(command)).map((file) => file.path)]; + if (config.scan?.heredocWrites !== false) paths.push(...heredocWrites(command).map((write) => write.path)); + return new Set(paths.map((path) => realPath(resolve(cwd, path)))); +} + +function changedTexts(command, cwd, config, since) { + const skipped = skippedPaths(command, cwd, config); + const globs = [...(config.ignoreGlobs || []), ...(config.styleIgnoreGlobs || [])]; + const files = []; + for (const file of gitChanges(cwd)) { + if (files.length >= MAX_FILES) break; + if (skipped.has(file.path)) continue; + const text = readChanged(file.path, since); + if (text === null || text.includes("concise-ignore-file") || isIgnored(file.path, globs)) continue; + files.push({ ...file, text }); + } + return files; +} + +function findingKey(path, group, hit, source) { + const category = group === "dictionary" ? `dictionary:${hit.id}` : hit.category || group; + return `shell-write:${shortHash([path, category, hit.match ?? hit.char, source].join("\0"))}`; +} + +/** The file's findings that this session has not reported, numbered by their line in the file. */ +function unreported(file, config, sessionId) { + const scope = isProsePath(file.path) ? "files" : "comments"; + const out = { emDash: [], aiWriting: [], dictionary: [] }; + const fresh = new Set(); + for (const chunk of addedChunks(file, file.text)) { + const lines = chunk.text.split("\n"); + const found = styleFindings(chunk.text, file.path, config, scope, "edit"); + for (const [group, hits] of Object.entries(found)) { + for (const hit of hits) { + const key = findingKey(file.path, group, hit, lines[hit.line - 1]); + if (!fresh.has(key) && !once(sessionId, key)) continue; + fresh.add(key); + out[group].push({ ...hit, line: chunk.start + hit.line - 1 }); + } + } + } + return out; +} + +function report(files, config, input, ctx) { + const texts = []; + const fired = { emDash: false, aiWriting: false }; + for (const file of files) { + const found = unreported(file, config, input.session_id); + const lines = parts(found, file.path, PLAIN); + if (lines.length === 0) continue; + texts.push(...lines); + for (const [name, on] of Object.entries(firedAny(found))) fired[name] ||= on; + ctx.key ||= `shell-write:${file.path}`; + } + if (texts.length === 0) return {}; + // The command already ran: this result only adds the reason next to its output. + ctx.decision = "flag"; + const reference = referenceFor(fired); + const fix = `This text is already on disk. Fix it with Edit${reference ? ` (read ${reference})` : ""}, or add concise-ignore on the line to keep it.`; + return { decision: "block", reason: [...texts, fix].join("\n") }; +} + +async function decide(input, ctx) { + const command = input.tool_input?.command; + if (!SHELL_TOOLS.includes(input.tool_name) || typeof command !== "string") return {}; + const since = Date.now() - (Number(input.duration_ms) || 0) - SLACK_MS; + const config = loadConfig(input.cwd); + ctx.config = config; + if (config.scan?.shellWrites === false || !styleOn(config)) return {}; + const cwd = input.cwd || "."; + const files = changedTexts(command, cwd, config, since); + if (files.length === 0) return {}; + const bypassed = bypassResult(command, config, ctx, "PostToolUse"); + if (bypassed) return bypassed; + await prepareStyle(cwd, config); + return withPackWarnings(report(files, config, input, ctx), input.session_id); +} + +await runHook({ hook: "check-shell-writes", event: "PostToolUse" }, decide); diff --git a/plugins/concise/hooks/check-tool-text.mjs b/plugins/concise/hooks/check-tool-text.mjs new file mode 100644 index 0000000..58be0c8 --- /dev/null +++ b/plugins/concise/hooks/check-tool-text.mjs @@ -0,0 +1,100 @@ +#!/usr/bin/env node +import { createHash } from "node:crypto"; +import { loadConfig } from "./lib/config.mjs"; +import { styleDecision, styleDecisionForText, prepareStyle, replyConfig, withPackWarnings } from "./lib/style-check.mjs"; +import { runHook, bypassResult } from "./lib/hook-main.mjs"; + +// MCP fields people read once the tool posts them, picked by words in the server and tool name. +const ALWAYS = ["body", "comment", "review_body"]; +const BY_NAME = [ + [/issue|pull|ticket|comment|review|task|project/i, ["description", "title"]], + [/message|post|comment|reply|chat|send|mail/i, ["text", "message", "subject"]], + [/doc|page|note|wiki|article|post/i, ["content", "markdown", "text", "title", "payload"]], +]; +// Built-in tools whose text shows in the session, each behind its own `scan` switch. +// A plan is a document; its file sits under .claude/, which styleIgnoreGlobs skip. +const BUILT_IN = { + ExitPlanMode: { scan: "plans", label: "plan", keys: ["plan"], scope: "files" }, + TaskCreate: { scan: "tasks", label: "task", keys: ["subject", "description", "activeForm"], scope: "reply" }, + TaskUpdate: { scan: "tasks", label: "task", keys: ["subject", "description", "activeForm"], scope: "reply" }, + AskUserQuestion: { scan: "questions", label: "question", keys: ["question", "label", "description"], scope: "reply" }, +}; +const PATH_KEYS = ["path", "file_path", "filePath"]; +const MIN_LENGTH = 20; +const MAX_DEPTH = 4; + +const shortHash = (text) => createHash("sha256").update(text).digest("hex").slice(0, 12); +const uniq = (list) => [...new Set(list)]; + +/** [key, text] pairs for the named string fields anywhere in the input, a few levels deep. */ +function collect(value, keys, out = [], depth = 0) { + if (depth > MAX_DEPTH || !value || typeof value !== "object") return out; + for (const [key, item] of Object.entries(value)) { + if (typeof item !== "string") collect(item, keys, out, depth + 1); + else if (keys.includes(key) && item.trim()) out.push([key, item]); + } + return out; +} + +/** Files an MCP tool writes: `{ path, content }`, `files: [{ path, content }]`, or `edits: [{ newText }]`. */ +function fileTargets(input) { + const out = []; + for (const item of [input, ...(Array.isArray(input.files) ? input.files : [])]) { + const path = PATH_KEYS.map((key) => item?.[key]).find((value) => typeof value === "string" && value); + if (!path) continue; + const edits = (Array.isArray(item.edits) ? item.edits : []).map((edit) => edit?.newText).filter((text) => typeof text === "string"); + if (typeof item.content === "string") out.push({ path, chunks: [item.content], wholeFile: true }); + else if (edits.length) out.push({ path, chunks: edits, wholeFile: false }); + } + return out; +} + +/** What to scan for one tool call, or null when the tool posts nothing people read. */ +function planFor(toolName, input) { + const builtIn = BUILT_IN[toolName]; + if (builtIn) return { ...builtIn, files: [], texts: collect(input, builtIn.keys) }; + if (typeof toolName !== "string" || !toolName.startsWith("mcp__") || /concise/i.test(toolName)) return null; + const name = toolName.slice("mcp__".length).split("__").join(" "); + const files = fileTargets(input); + // A tool that writes files carries a commit message at most, never a post. + if (files.length) return { scan: "mcp", label: `${name} commit message`, files, texts: collect({ message: input.message }, ["message"]), scope: "commit" }; + const keys = uniq([...ALWAYS, ...BY_NAME.filter(([re]) => re.test(name)).flatMap(([, list]) => list)]); + const texts = collect(input, keys); + return { scan: "mcp", label: `${name} ${uniq(texts.map(([key]) => key)).join(" and ")}`, files, texts, scope: "gh" }; +} + +// Keyed on the call minus its text, so revisions of one post share a retry counter. +function keyOf(toolName, input, texts) { + const scanned = new Set(texts.map(([, text]) => text)); + const scaffold = JSON.stringify(input, (key, value) => (typeof value === "string" && scanned.has(value) ? undefined : value)); + return `style:tool:${shortHash(`${toolName}\0${scaffold}`)}`; +} + +function combine(...results) { + return results.find((r) => r.hookSpecificOutput?.permissionDecision) || results.find((r) => Object.keys(r).length > 0) || {}; +} + +async function decide(input, ctx) { + const toolInput = input.tool_input || {}; + const plan = planFor(input.tool_name, toolInput); + if (!plan) return {}; + const text = plan.texts.map(([, value]) => value).join("\n\n"); + const files = plan.files.filter((file) => !file.chunks.some((chunk) => chunk.includes("concise-ignore-file"))); + const textOn = text.trim().length >= MIN_LENGTH && !text.includes("concise-ignore"); + if (!textOn && files.length === 0) return {}; + + const config = loadConfig(input.cwd); + ctx.config = config; + if (config.scan[plan.scan] === false) return {}; + const bypassed = bypassResult([text, ...files.flatMap((file) => file.chunks)], config, ctx); + if (bypassed) return bypassed; + await prepareStyle(input.cwd, config); + const fileResult = files.length ? styleDecision(files, input, config) : {}; + const textConfig = plan.scope === "reply" ? replyConfig(config) : config; + const textResult = textOn + ? styleDecisionForText(text, keyOf(input.tool_name, toolInput, plan.texts), plan.label, input, textConfig, "PreToolUse", plan.scope, null) + : {}; + return withPackWarnings(combine(fileResult, textResult), input.session_id); +} + +await runHook({ hook: "check-tool-text", event: "PreToolUse" }, decide); diff --git a/plugins/concise/hooks/codex.json b/plugins/concise/hooks/codex.json index fe8ce3f..02530a5 100644 --- a/plugins/concise/hooks/codex.json +++ b/plugins/concise/hooks/codex.json @@ -35,6 +35,17 @@ "statusMessage": "concise: checking written text" } ] + }, + { + "matcher": "mcp__.*", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-tool-text.mjs\"", + "timeout": 15, + "statusMessage": "concise: checking posted tool text" + } + ] } ], "Stop": [ diff --git a/plugins/concise/hooks/hooks.json b/plugins/concise/hooks/hooks.json index 4d44f2e..da6eadb 100644 --- a/plugins/concise/hooks/hooks.json +++ b/plugins/concise/hooks/hooks.json @@ -3,7 +3,7 @@ "hooks": { "PreToolUse": [ { - "matcher": "Write|Edit|MultiEdit|apply_patch", + "matcher": "Write|Edit|MultiEdit|apply_patch|NotebookEdit", "hooks": [ { "type": "command", @@ -30,12 +30,74 @@ "if": "Bash(*g*)", "statusMessage": "concise: checking commit and PR text" }, + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-bash.mjs\"", + "timeout": 15, + "if": "Bash(jj *)", + "statusMessage": "concise: checking commit and PR text" + }, { "type": "command", "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-edit.mjs\"", "timeout": 15, "if": "Bash(*Begin Patch*)", "statusMessage": "concise: checking patch text" + }, + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-edit.mjs\"", + "timeout": 15, + "if": "Bash(cat *)", + "statusMessage": "concise: checking heredoc file writes" + }, + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-edit.mjs\"", + "timeout": 15, + "if": "Bash(tee *)", + "statusMessage": "concise: checking heredoc file writes" + } + ] + }, + { + "matcher": "PowerShell", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-bash.mjs\"", + "timeout": 15, + "if": "PowerShell(*g*)", + "statusMessage": "concise: checking commit and PR text" + }, + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-bash.mjs\"", + "timeout": 15, + "if": "PowerShell(jj *)", + "statusMessage": "concise: checking commit and PR text" + } + ] + }, + { + "matcher": "mcp__.*", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-tool-text.mjs\"", + "timeout": 15, + "statusMessage": "concise: checking posted tool text" + } + ] + }, + { + "matcher": "ExitPlanMode|TaskCreate|TaskUpdate|AskUserQuestion", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-tool-text.mjs\"", + "timeout": 15, + "statusMessage": "concise: checking plan, task, and question text" } ] }, @@ -51,6 +113,19 @@ ] } ], + "PostToolUse": [ + { + "matcher": "Bash|PowerShell", + "hooks": [ + { + "type": "command", + "command": "node \"${CLAUDE_PLUGIN_ROOT}/hooks/check-shell-writes.mjs\"", + "timeout": 15, + "statusMessage": "concise: checking files the command wrote" + } + ] + } + ], "Stop": [ { "hooks": [ diff --git a/plugins/concise/hooks/lib/changed-files.mjs b/plugins/concise/hooks/lib/changed-files.mjs new file mode 100644 index 0000000..1dc1168 --- /dev/null +++ b/plugins/concise/hooks/lib/changed-files.mjs @@ -0,0 +1,62 @@ +import { execFileSync } from "node:child_process"; +import { lstatSync, readFileSync, realpathSync } from "node:fs"; +import { join } from "node:path"; + +const MAX_ENTRIES = 2000; +const MAX_BYTES = 256 * 1024; +const SNIFF_BYTES = 8192; +const GIT_OPTIONS = { encoding: "utf8", timeout: 3000, maxBuffer: 1024 * 1024, stdio: ["ignore", "pipe", "ignore"] }; +const HUNK = /^@@+ [^+]*\+(\d+)/; + +// Literal pathspecs, so a file named `a[1].md` does not match `a1.md` as a glob. +function git(cwd, args) { + try { + return execFileSync("git", ["-C", cwd, "--literal-pathspecs", ...args], GIT_OPTIONS); + } catch { + return null; + } +} + +/** The path with symlinks resolved, so a path from the command and a path from git compare equal. */ +export function realPath(path) { + try { + return realpathSync.native(path); + } catch { + return path; + } +} + +/** Tracked files with unstaged changes and untracked files git does not ignore, as { root, rel, path, tracked }. */ +export function gitChanges(cwd) { + const top = git(cwd, ["rev-parse", "--show-toplevel"]); + const root = top && realPath(top.trim()); + const listed = root && git(root, ["ls-files", "-t", "-m", "-o", "--exclude-standard", "-z"]); + if (!listed) return []; + const entries = [...new Set(listed.split("\0"))].filter(Boolean).slice(0, MAX_ENTRIES); + return entries.map((entry) => ({ root, rel: entry.slice(2), path: join(root, entry.slice(2)), tracked: !entry.startsWith("?") })); +} + +/** The file's text when it changed at or after `since`, or null when it is older, too big, binary, or gone. */ +export function readChanged(path, since) { + try { + const stat = lstatSync(path); + if (!stat.isFile() || stat.mtimeMs < since || stat.size > MAX_BYTES) return null; + const bytes = readFileSync(path); + return bytes.subarray(0, SNIFF_BYTES).includes(0) ? null : bytes.toString("utf8"); + } catch { + return null; + } +} + +/** The text a change added, as { text, start }: the whole untracked file, or each hunk of the work tree diff. */ +export function addedChunks(file, text) { + if (!file.tracked) return [{ text, start: 1 }]; + const diff = git(file.root, ["diff", "--no-color", "--no-ext-diff", "--no-textconv", "-U0", "--", file.rel]) || ""; + const hunks = []; + for (const line of diff.split("\n")) { + const header = HUNK.exec(line); + if (header) hunks.push({ start: Number(header[1]), lines: [] }); + else if (hunks.length > 0 && line.startsWith("+")) hunks.at(-1).lines.push(line.slice(1)); + } + return hunks.filter((hunk) => hunk.lines.length > 0).map((hunk) => ({ text: hunk.lines.join("\n"), start: hunk.start })); +} diff --git a/plugins/concise/hooks/lib/config-layers.mjs b/plugins/concise/hooks/lib/config-layers.mjs index 11858e3..546ae15 100644 --- a/plugins/concise/hooks/lib/config-layers.mjs +++ b/plugins/concise/hooks/lib/config-layers.mjs @@ -10,6 +10,14 @@ const FEATURE_TARGETS = { fileSize: ["checks", "fileSize"], prBody: ["checks", "prBody"], stopHook: ["stopHook"], + codeFiles: ["scan", "codeFiles"], + notebooks: ["scan", "notebooks"], + heredocWrites: ["scan", "heredocWrites"], + shellWrites: ["scan", "shellWrites"], + mcp: ["scan", "mcp"], + plans: ["scan", "plans"], + tasks: ["scan", "tasks"], + questions: ["scan", "questions"], }; const isObject = (value) => Boolean(value) && typeof value === "object" && !Array.isArray(value); @@ -84,7 +92,7 @@ export function applyLayer(base, layer) { next.checks = { ...base.checks, ...(isObject(layer.checks) ? layer.checks : {}) }; next.log = { ...base.log, ...(isObject(layer.log) ? layer.log : {}) }; next.monitor = { ...base.monitor, ...(isObject(layer.monitor) ? layer.monitor : {}) }; - for (const name of ["context", "subagentStop", "testFilter"]) { + for (const name of ["context", "subagentStop", "testFilter", "scan"]) { next[name] = { ...base[name], ...(isObject(layer[name]) ? layer[name] : {}) }; } next.allowList = mergeStringSets(base.allowList, layer.allowList); diff --git a/plugins/concise/hooks/lib/config.mjs b/plugins/concise/hooks/lib/config.mjs index 69b2850..225c797 100644 --- a/plugins/concise/hooks/lib/config.mjs +++ b/plugins/concise/hooks/lib/config.mjs @@ -71,6 +71,7 @@ const DEFAULTS = { "**/*.lock", ], checks: { comments: true, fileSize: true, prBody: true }, + scan: { codeFiles: true, notebooks: true, heredocWrites: true, shellWrites: true, mcp: true, plans: true, tasks: true, questions: true }, stopHook: true, context: { enabled: true, perTurn: false }, subagentStop: { enabled: true, exemptAgentTypes: [] }, @@ -135,6 +136,11 @@ export function loadConfig(cwd, env = process.env) { problems.push({ source: `${group}.${key}`, reason: "expected a boolean" }); config[group][key] = DEFAULTS[group][key]; } + for (const [key, on] of Object.entries(DEFAULTS.scan)) { + if (typeof config.scan[key] === "boolean") continue; + problems.push({ source: `scan.${key}`, reason: "expected a boolean" }); + config.scan[key] = on; + } if (!Array.isArray(config.subagentStop.exemptAgentTypes) || config.subagentStop.exemptAgentTypes.some((type) => typeof type !== "string")) { problems.push({ source: "subagentStop.exemptAgentTypes", reason: "expected an array of strings" }); config.subagentStop.exemptAgentTypes = []; diff --git a/plugins/concise/hooks/lib/edit-targets.mjs b/plugins/concise/hooks/lib/edit-targets.mjs new file mode 100644 index 0000000..903e000 --- /dev/null +++ b/plugins/concise/hooks/lib/edit-targets.mjs @@ -0,0 +1,83 @@ +import { readFileSync } from "node:fs"; +import { resolve } from "node:path"; +import { extractPatch, parseApplyPatch } from "./apply-patch.mjs"; +import { heredocWrites, fileFlagPaths } from "./shell-text.mjs"; + +const EDIT_TOOLS = ["Write", "Edit", "MultiEdit"]; + +// Only the text this call writes, so a one-line edit isn't blamed for what's already +// on disk. Chunks stay separate: joining them could invent a comment run. +function writtenChunks(toolName, toolInput) { + if (toolName === "Write") return [toolInput.content || ""]; + if (toolName === "Edit") return [toolInput.new_string || ""]; + return (toolInput.edits || []).map((edit) => edit.new_string || ""); +} + +// Claude Code sends Write/Edit/MultiEdit; Codex sends apply_patch (or a shell heredoc +// carrying one). Both become { path, chunks, wholeFile } targets. +export function targetsOf(input) { + const toolName = input.tool_name; + const toolInput = input.tool_input || {}; + + if (EDIT_TOOLS.includes(toolName)) { + if (!toolInput.file_path) return []; + return [{ path: toolInput.file_path, chunks: writtenChunks(toolName, toolInput), wholeFile: toolName === "Write" }]; + } + + if (toolName === "NotebookEdit") return notebookTargets(toolInput); + + let patch = null; + if (toolName === "apply_patch") patch = toolInput.command || toolInput.input || ""; + if (toolName === "Bash") patch = extractPatch(toolInput.command); + const shellWrites = toolName === "Bash" ? heredocTargets(toolInput.command, input.cwd) : []; + if (!patch) return shellWrites; + + return parseApplyPatch(patch) + .map((file) => ({ + path: resolve(input.cwd || ".", file.path), + chunks: file.chunks, + wholeFile: file.kind === "add", + })) + .concat(shellWrites); +} + +// `cat > notes.md <<'EOF'` writes a file that no Write call ever shows. A body file that +// `gh --body-file` or `git commit -F` reads in the same command is check-bash's to scan. +function heredocTargets(command, cwd = ".") { + const consumed = new Set(fileFlagPaths(command).map((path) => resolve(cwd, path))); + return heredocWrites(command) + .map((write) => ({ path: resolve(cwd, write.path), chunks: [write.body], wholeFile: !write.append, scan: "heredocWrites" })) + .filter((target) => !consumed.has(target.path)); +} + +// A cell has no file of its own, so the path gets an extension that picks prose or comment rules. +// `file` keeps the notebook's own path for ignoreGlobs and the concise-ignore-file marker. +function notebookTargets({ notebook_path: path, new_source: source, cell_type: type, cell_id: id, edit_mode: mode }) { + if (!path || typeof source !== "string" || mode === "delete") return []; + const notebook = readNotebook(path); + const cellType = type || notebook?.cells?.find((cell) => cell.id === id)?.cell_type || "code"; + const ext = cellType === "markdown" ? ".md" : notebook?.metadata?.language_info?.file_extension || ".py"; + return [{ path: `${path}#${id || "new"}${ext}`, file: path, chunks: [source], wholeFile: false, scan: "notebooks" }]; +} + +function readNotebook(path) { + try { + return JSON.parse(readFileSync(path, "utf8")); + } catch { + return null; + } +} + +function hasFileMarker(filePath) { + try { + return readFileSync(filePath, "utf8").includes("concise-ignore-file"); + } catch { + return false; + } +} + +// A whole-file write replaces the file, so its new content is the only authority on the marker. +export function isExempt({ path, file, chunks, wholeFile }) { + if (chunks.some((chunk) => chunk.includes("concise-ignore-file"))) return true; + return !wholeFile && hasFileMarker(file || path); +} diff --git a/plugins/concise/hooks/lib/hook-main.mjs b/plugins/concise/hooks/lib/hook-main.mjs index 1d8e1af..dce1f02 100644 --- a/plugins/concise/hooks/lib/hook-main.mjs +++ b/plugins/concise/hooks/lib/hook-main.mjs @@ -1,7 +1,7 @@ import { flagged, modelNotices } from "./respond.mjs"; import { loadConfig } from "./config.mjs"; import { createLogger, softFailResult } from "./log.mjs"; -import { once, withStateScope } from "./state.mjs"; +import { claim, once, withStateScope } from "./state.mjs"; import { styleLog } from "./style-check.mjs"; import { STOP_BLOCK_MESSAGE } from "./confirm.mjs"; import { publishMonitor } from "./monitor.mjs"; @@ -113,7 +113,7 @@ function logRun({ hook, event, input, ctx, config, result, error, started }) { } /** Reads the event, runs decide, applies soft fail and logging, writes the hook JSON. */ -export async function runHook({ hook, event }, decide) { +export async function runHook({ hook, event, overlaps }, decide) { const started = Date.now(); const ctx = { config: null, key: null, decision: null }; const raw = await readStdin(); @@ -125,6 +125,11 @@ export async function runHook({ hook, event }, decide) { } catch (err) { error = err.message; } + // Two `if` rules can both start this handler for one call; the second run exits without output. + if (!error && overlaps?.(input) && typeof input.tool_use_id === "string" && !claim(input.session_id, `${hook}:${input.tool_use_id}`)) { + process.stdout.write("{}"); + return; + } const actualEvent = input.hook_event_name || event; let config; await withStateScope(input, async () => { diff --git a/plugins/concise/hooks/lib/packs.mjs b/plugins/concise/hooks/lib/packs.mjs index b4c42c0..d02dc5a 100644 --- a/plugins/concise/hooks/lib/packs.mjs +++ b/plugins/concise/hooks/lib/packs.mjs @@ -8,8 +8,8 @@ const PRESETS_FILE = join(BUILTIN_DIR, "presets.json"); export const ID_RE = /^[a-z0-9][a-z0-9-]*$/; const FEATURES = ["aiWriting", "emDash"]; -export const SCOPES = ["files", "comments", "gh", "commit", "command", "reply"]; -export const DEFAULT_SCOPE = SCOPES.filter((s) => s !== "command"); +export const SCOPES = ["files", "comments", "code", "gh", "commit", "command", "reply"]; +export const DEFAULT_SCOPE = SCOPES.filter((s) => s !== "command" && s !== "code"); const KINDS = ["phrase", "opening", "regex"]; const AI_DEFAULTS = { diff --git a/plugins/concise/hooks/lib/patterns/git/ai-attribution.json b/plugins/concise/hooks/lib/patterns/git/ai-attribution.json index 3716923..c22108f 100644 --- a/plugins/concise/hooks/lib/patterns/git/ai-attribution.json +++ b/plugins/concise/hooks/lib/patterns/git/ai-attribution.json @@ -15,9 +15,9 @@ { "regex": "^Generated-By:.*$", "flags": "gim", "fix": "drop the trailer", "show": "a `Generated-By:` trailer" }, { "phrase": "This commit was generated by", "fix": "drop the disclosure sentence" }, { "regex": "^Assisted-by:.*$", "flags": "gim", "fix": "drop the trailer", "show": "an `Assisted-by:` trailer" }, - { "regex": "^Claude-Session:.*$", "flags": "gim", "fix": "drop the session trailer", "show": "a `Claude-Session:` trailer" }, + { "regex": "^Claude-Session:", "flags": "gim", "fix": "drop the session trailer", "show": "a `Claude-Session:` trailer" }, { "regex": "^Made-with:\\s*Cursor\\b.*$", "flags": "gim", "fix": "drop the trailer", "show": "a `Made-with: Cursor` trailer" }, - { "regex": "^Agent-Logs-Url:.*$", "flags": "gim", "fix": "drop the trailer", "show": "an `Agent-Logs-Url:` trailer" }, + { "regex": "^Agent-Logs-Url:", "flags": "gim", "fix": "drop the trailer", "show": "an `Agent-Logs-Url:` trailer" }, { "regex": "^Signed-off-by:.*\\[bot\\].*$", "flags": "gim", "fix": "drop the bot sign-off", "show": "`Signed-off-by:` plus a `[bot]` account" }, { "regex": "^Signed-off-by:.*(?:noreply@anthropic\\.com|noreply@openai\\.com|all-hands\\.dev|cursor\\.com)>?\\s*$", "flags": "gim", "fix": "drop the bot sign-off", "show": "`Signed-off-by:` plus an AI tool email" } ] diff --git a/plugins/concise/hooks/lib/patterns/prose/hidden-characters.json b/plugins/concise/hooks/lib/patterns/prose/hidden-characters.json index 10822db..4c99640 100644 --- a/plugins/concise/hooks/lib/patterns/prose/hidden-characters.json +++ b/plugins/concise/hooks/lib/patterns/prose/hidden-characters.json @@ -4,8 +4,8 @@ "category": { "id": "hidden-characters", "label": "hidden characters", "description": "Invisible or look-alike characters that model output and chat UIs leave in text.", "tags": ["punctuation", "watermark"] }, "source": "Opus 5.5 replies (U+2011 at 0.33 per 10,000 words), Microsoft known issue on LLM hyphens (Apr 2026), OpenAI U+202F reports (2025), Trojan Source bidi controls, Wikipedia:Signs of AI writing", "presets": ["default", "ryan", "technical", "git", "all"], - "scope": ["files", "comments", "gh", "commit", "command", "reply"], - "notes": "Anthropic's text watermark adds no characters, so this pack cannot see it. U+00A0 is left out: npm READMEs carry it 5.6 times per 10,000 words. A zero-width joiner inside an emoji sequence and tag characters after the black flag (subdivision flags) stay unflagged. A right-to-left project lists `hidden-characters` in `excludePacks` or drops the bidi row with its own pack.", + "scope": ["files", "code", "gh", "commit", "command", "reply"], + "notes": "Anthropic's text watermark adds no characters, so this pack cannot see it. In a code file the pack reads the whole file, string literals included, through the `code` scope. U+00A0 is left out: npm READMEs carry it 5.6 times per 10,000 words. A zero-width joiner inside an emoji sequence and tag characters after the black flag (subdivision flags) stay unflagged. A right-to-left project lists `hidden-characters` in `excludePacks` or drops the bidi row with its own pack.", "patterns": [ { "regex": "[\\u200B\\u200C\\u2060\\u180E]", "flags": "gu", "fix": "remove the zero-width character", "show": "a zero-width space, zero-width non-joiner, word joiner, or Mongolian vowel separator" }, { "regex": "(? `{{concise-heredoc-${bodies.push(body) - 1}}}`); + return { masked, bodies, bodyIn: (text) => bodies[MASKED_BODY.exec(text)?.[1]] ?? null }; +} + +/** A `gh` command that posts a title or body. */ +export const GH_COMMAND = /\bgh\s+(?:(?:pr|issue)\s+(?:create|comment|edit|review|merge)|release\s+(?:create|edit))\b/; -/** Pulls the body text out of a `gh pr/issue ...` command: heredoc form or a quoted flag. */ -export function extractBody(command) { +/** The body of a `gh pr`, `gh issue`, or `gh release` command: heredoc, quoted flag, or body file. */ +export function extractBody(command, cwd = ".") { const heredoc = HEREDOC.exec(command); if (heredoc) return heredoc[2]; - const dq = /(?:--body|-b)[= ]"((?:[^"\\]|\\.)*)"/.exec(command); + const dq = /(?:--body|-b|--notes|-n)[= ]"((?:[^"\\]|\\.)*)"/.exec(command); if (dq) return dq[1]; - const sq = /(?:--body|-b)[= ]'((?:[^'\\]|\\.)*)'/.exec(command); + const sq = /(?:--body|-b|--notes|-n)[= ]'((?:[^'\\]|\\.)*)'/.exec(command); if (sq) return sq[1]; - return null; + const [file] = fileFlagPaths(command); + return file ? readMessageFile(file, { cwd, command }) : null; +} + +const TITLE = /(?:^|\s)(?:--title|--subject|-t)[= ](?:"((?:[^"\\]|\\.)*)"|'((?:[^'\\]|\\.)*)')/; + +/** The quoted `--title`, `--subject`, or `-t` value of a gh command, outside its heredocs. */ +export function extractTitle(command) { + const m = TITLE.exec(maskHeredocs(command).masked); + return m ? (m[1] ?? m[2]) : null; +} + +const API_CALL = /\bgh\s+api\b/g; +const WORD = String.raw`((?:"(?:[^"\\]|\\.)*"|'[^']*'|\\.|[^\s"'\\])+)`; +const API_FIELD = new RegExp(String.raw`(?:^|\s)(-[fF]|--(?:raw-)?field)(?:=|\s+)` + WORD, "g"); +const API_INPUT = new RegExp(String.raw`(?:^|\s)--input(?:=|\s+)` + WORD); +const API_KEYS = ["body", "message", "description", "title"]; + +function jsonFields(text) { + try { + return Object.entries(JSON.parse(text)).filter(([key, value]) => API_KEYS.includes(key) && typeof value === "string").map(([, value]) => value); + } catch { + return []; + } +} + +/** The text fields of each `gh api` call: `-f`/`-F` values, `-F key=@file`, and the top-level strings of `--input` JSON. */ +export function apiFields(command, cwd = ".") { + if (typeof command !== "string" || !command.match(API_CALL)) return []; + const { masked, bodyIn } = maskHeredocs(command); + const out = []; + for (const call of masked.matchAll(API_CALL)) { + const segment = segmentFrom(masked, call.index); + const read = (path) => (path === "-" ? bodyIn(segment) : readMessageFile(path, { cwd, command })); + for (const [, flag, word] of segment.matchAll(API_FIELD)) { + const [key, value = ""] = unquote(word).split(/=(.*)/s); + if (!API_KEYS.includes(key)) continue; + // Only the typed `-F` form reads `@file`; a raw `-f` value is literal text. + const typed = flag === "-F" || flag === "--field"; + out.push(typed && value.startsWith("@") ? read(value.slice(1)) : bodyIn(value) ?? unquote(value)); + } + const input = API_INPUT.exec(segment); + if (input) out.push(...jsonFields(read(unquote(input[1])))); + } + return out.filter(Boolean); } const STRUCTURAL = [/^#{1,6}\s/, /^[-*]\s/, /^\d+\.\s/, /^>/]; diff --git a/plugins/concise/hooks/lib/prose.mjs b/plugins/concise/hooks/lib/prose.mjs index 3108149..b415010 100644 --- a/plugins/concise/hooks/lib/prose.mjs +++ b/plugins/concise/hooks/lib/prose.mjs @@ -1,5 +1,6 @@ import { scanComments } from "./comment-scan.mjs"; -import { HEREDOC } from "./pr-body.mjs"; +import { maskHeredocs } from "./pr-body.mjs"; +import { readMessageFile, segmentFrom, unquote } from "./shell-text.mjs"; export const PROSE_EXTENSIONS = ["md", "mdx", "markdown", "txt", "rst", "adoc", "asciidoc"]; @@ -56,26 +57,33 @@ export function proseSpans(text, path) { } const MESSAGE_FLAG = /(?:^|\s)(?:--message(?:=|\s+)|-[A-Za-z]*m\s*)("(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*')/g; -const HEREDOC_G = new RegExp(HEREDOC.source, "g"); -const MASKED_BODY = /\{\{concise-heredoc-(\d+)\}\}/; -function maskHeredocs(command) { - const bodies = []; - const masked = command.replace(HEREDOC_G, (_full, _tag, body) => { - bodies.push(body); - return `{{concise-heredoc-${bodies.length - 1}}}`; - }); - return { masked, bodies }; -} +// `git -C dir commit`, `git -c k=v tag`, `git merge`, `git notes add`, `jj -R repo describe`, and `hg ci` all take a message. +const GLOBAL_OPTIONS = String.raw`(?:\s+(?:-[CcR]\s+\S+|--[\w-]+(?:=\S+)?))*`; +const GIT_MESSAGE_COMMAND = new RegExp( + String.raw`\b(?:git${GLOBAL_OPTIONS}\s+(?:commit|tag|merge|notes\s+(?:add|append|edit))|jj${GLOBAL_OPTIONS}\s+(?:describe|desc|commit|new|split)|hg${GLOBAL_OPTIONS}\s+(?:commit|ci))\b`, +); +const QUOTED_OR_WORD = String.raw`("(?:[^"\\]|\\.)*"|'(?:[^'\\]|\\.)*'|[^\s;&|]+)`; +const GIT_FILE_FLAG = new RegExp(String.raw`(?:^|\s)(?:--(?:log)?file(?:=|\s+)|-[A-Za-z]*[Fl]\s*)` + QUOTED_OR_WORD, "g"); +const TRAILER_FLAG = new RegExp(String.raw`(?:^|\s)--trailer(?:=|\s+)` + QUOTED_OR_WORD, "g"); -export function gitCommitMessages(command) { - if (typeof command !== "string" || !/\bgit\s+commit\b/.test(command)) return []; - const { masked, bodies } = maskHeredocs(command); +/** Message text from `-m`, `--message`, `-F`/`--file` or hg's `-l`/`--logfile` (a file, or `-` for the heredoc), and `--trailer`. */ +export function gitCommitMessages(command, cwd = ".") { + if (typeof command !== "string" || !GIT_MESSAGE_COMMAND.test(command)) return []; + const { masked, bodies, bodyIn } = maskHeredocs(command); const messages = []; for (const m of masked.matchAll(MESSAGE_FLAG)) { const quoted = m[1].slice(1, -1); - const body = MASKED_BODY.exec(quoted); - messages.push(body ? bodies[Number(body[1])] : quoted); + messages.push(bodyIn(quoted) ?? quoted); + } + const start = masked.search(GIT_MESSAGE_COMMAND); + const segment = start === -1 ? "" : segmentFrom(masked, start); + for (const m of segment.matchAll(GIT_FILE_FLAG)) { + const path = unquote(m[1]); + const body = path === "-" ? bodies[0] : readMessageFile(path, { cwd, command }); + if (body) messages.push(body); } + const trailers = [...segment.matchAll(TRAILER_FLAG)].map((m) => unquote(m[1])).map((t) => (t.includes(":") ? t : t.replace("=", ": "))); + if (trailers.length > 0) messages.push(trailers.join("\n")); return messages; } diff --git a/plugins/concise/hooks/lib/shell-text.mjs b/plugins/concise/hooks/lib/shell-text.mjs new file mode 100644 index 0000000..b06528e --- /dev/null +++ b/plugins/concise/hooks/lib/shell-text.mjs @@ -0,0 +1,73 @@ +import { readFileSync, statSync } from "node:fs"; +import { resolve } from "node:path"; + +const MAX_BYTES = 256 * 1024; +// Unlike pr-body's HEREDOC, this one keeps the rest of the opener line, where `> file` can sit. +const HEREDOC_LINE = /<<[-~]?\s*(['"]?)(\w+)\1([^\n]*)\r?\n([\s\S]*?)\r?\n\t*\2(?=\r?\n|$)/g; +const TARGET = String.raw`("[^"\n]+"|'[^'\n]+'|[^\s;&|<>()]+)`; +const REDIRECT = new RegExp(String.raw`(?>?)\s*` + TARGET); +const TEE = new RegExp(String.raw`\btee\s+((?:-[a-z]+\s+)*)` + TARGET); +const FILE_FLAG = new RegExp(String.raw`(?:^|\s)(?:--(?:body-file|notes-file|file|field|input)(?:=|\s+)|-[A-Za-z]*F(?:=|\s*))` + TARGET, "g"); +// `gh api -F key=value` is a field, and only `key=@path` reads a file. +const FIELD = /^[\w[\]-]+=(.*)$/s; + +export const unquote = (value) => (/^(["']).*\1$/s.test(value) ? value.slice(1, -1) : value); + +function writeOf(head, tail) { + const tee = TEE.exec(tail) || TEE.exec(head); + if (tee) return { path: unquote(tee[2]), append: /-\w*a/.test(tee[1]) }; + if (!/\bcat\b/.test(head)) return null; + const redirect = REDIRECT.exec(tail) || REDIRECT.exec(head); + return redirect ? { path: unquote(redirect[2]), append: redirect[1] === ">>" } : null; +} + +/** Files that `cat` or `tee` write from a heredoc in this command, as { path, body, append }. */ +export function heredocWrites(command) { + if (typeof command !== "string" || !command.includes("<<")) return []; + const out = []; + for (const m of command.matchAll(HEREDOC_LINE)) { + const head = command.slice(command.lastIndexOf("\n", m.index) + 1, m.index); + const write = writeOf(head, m[3]); + if (write && !write.path.startsWith("/dev/")) out.push({ ...write, body: m[4] }); + } + return out; +} + +/** Paths the command passes to `--body-file`, `--notes-file`, `--file`, `-F`, `--input`, or `gh api -F key=@path`. */ +export function fileFlagPaths(command) { + if (typeof command !== "string") return []; + return [...command.matchAll(FILE_FLAG)].flatMap((m) => { + const value = unquote(m[1]); + const field = FIELD.exec(value); + if (!field) return [value]; + return field[1].startsWith("@") ? [field[1].slice(1)] : []; + }); +} + +/** A message file's text: a heredoc write earlier in the same command wins over the file on disk. */ +export function readMessageFile(path, { cwd = ".", command = "" } = {}) { + if (!path || path === "-") return null; + const full = resolve(cwd, path); + const written = heredocWrites(command).find((w) => resolve(cwd, w.path) === full); + if (written) return written.body; + try { + if (statSync(full).size > MAX_BYTES) return null; + return readFileSync(full, "utf8"); + } catch { + return null; + } +} + +/** The text from `index` to the next `&&`, `||`, `;`, `|`, or newline outside quotes. */ +export function segmentFrom(text, index) { + let quote = null; + for (let i = index; i < text.length; i += 1) { + const ch = text[i]; + if (quote) { + if (ch === "\\" && quote === '"') i += 1; + else if (ch === quote) quote = null; + } else if (ch === '"' || ch === "'") quote = ch; + else if (ch === ";" || ch === "|" || ch === "\n" || (ch === "&" && text[i + 1] === "&")) return text.slice(index, i); + } + return text.slice(index); +} diff --git a/plugins/concise/hooks/lib/state.mjs b/plugins/concise/hooks/lib/state.mjs index 5fa6d53..85c51a5 100644 --- a/plugins/concise/hooks/lib/state.mjs +++ b/plugins/concise/hooks/lib/state.mjs @@ -1,6 +1,6 @@ import { AsyncLocalStorage } from "node:async_hooks"; import { createHash } from "node:crypto"; -import { readFileSync, writeFileSync, mkdirSync, renameSync, opendirSync, unlinkSync, rmdirSync } from "node:fs"; +import { readFileSync, writeFileSync, mkdirSync, renameSync, opendirSync, unlinkSync, rmdirSync, openSync, closeSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; @@ -78,6 +78,18 @@ export function resetAttempt(sessionId, key) { writeState(sessionId, state); } +/** True for the first process to claim this (session, key). An exclusive create, so parallel hooks agree. */ +export function claim(sessionId, key) { + const path = join(sessionPath(sessionId), `${digest(`claim:${key}`)}.json`); + try { + mkdirSync(dirname(path), { recursive: true, mode: 0o700 }); + closeSync(openSync(path, "wx", 0o600)); + return true; + } catch (error) { + return error.code !== "EEXIST"; + } +} + /** True the first time this (session, key) pair is seen, false after that. */ export function once(sessionId, key) { const state = readState(sessionId); diff --git a/plugins/concise/hooks/lib/style-check.mjs b/plugins/concise/hooks/lib/style-check.mjs index 30a4380..c0af1d7 100644 --- a/plugins/concise/hooks/lib/style-check.mjs +++ b/plugins/concise/hooks/lib/style-check.mjs @@ -101,6 +101,22 @@ export function withPackWarnings(result, sessionId) { return { ...result, systemMessage: result.systemMessage ? `${result.systemMessage} ${text}` : text }; } +// A feature that is off for replies is off for reply text, so styleFindings skips it. +// Dictionary entries pick their hooks themselves. +export function replyConfig(config) { + const { emDash, aiWriting } = config.features; + return { + ...config, + ignoreGlobs: [], + styleIgnoreGlobs: [], + features: { + ...config.features, + emDash: { ...emDash, enabled: Boolean(emDash.enabled && emDash.replies) }, + aiWriting: { ...aiWriting, enabled: Boolean(aiWriting.enabled && aiWriting.replies) }, + }, + }; +} + /** `hook` is edit, bash, stop, or subagentStop; dictionary entries can be limited to some of them. */ export function styleFindings(text, path, config, scope = "files", hook = null) { const found = { emDash: [], aiWriting: [], dictionary: [] }; @@ -140,6 +156,13 @@ export function styleFindings(text, path, config, scope = "files", hook = null) if (keep(at(hit.line), hit.match)) aiWriting.push({ ...hit, line: at(hit.line) }); } } + // The `code` scope reads the whole code file, string literals included. + if (scope === "comments" && config.scan?.codeFiles !== false) { + for (const hit of scanDictionary(text, entries, { hook, scope: "code" })) if (keep(hit.line, hit.match)) dictionary.push(hit); + const codePacks = resolved ? resolved.packs.filter((p) => inScope(p, "code")) : []; + const hits = codePacks.length ? scanAiWriting(text, { packs: codePacks, allow: resolved.allow, ctx: { path, scope: "code", raw: text }, problems: runtime }) : []; + for (const hit of hits) if (keep(hit.line, hit.match)) aiWriting.push(hit); + } collect(found, scope); return found; } diff --git a/plugins/concise/omp/extension.mjs b/plugins/concise/omp/extension.mjs index 6a9066f..15a0f13 100644 --- a/plugins/concise/omp/extension.mjs +++ b/plugins/concise/omp/extension.mjs @@ -5,7 +5,7 @@ import { contextOf, editCall, replyText, toolCallResult } from "./translate.mjs" const HOOKS = fileURLToPath(new URL("../hooks/", import.meta.url)); const TIMEOUT_MS = 15000; // The same cheap gates as the `if` conditions in hooks/hooks.json. -const BASH_HOOKS = [["monitor-filter", "t"], ["check-bash", "g"], ["check-edit", "Begin Patch"]]; +const BASH_HOOKS = [["monitor-filter", "t"], ["check-bash", /g|\bjj\s/], ["check-edit", /Begin Patch|< command.includes(needle)).map(([name]) => [name, hookInput]); + const gated = ([, needle]) => (typeof needle === "string" ? command.includes(needle) : needle.test(command)); + return BASH_HOOKS.filter(gated).map(([name]) => [name, hookInput]); } const call = editCall(event.toolName, event.input, ctx.cwd); return call ? [["check-edit", { ...input, ...call }]] : []; diff --git a/plugins/concise/skills/concise-config/SKILL.md b/plugins/concise/skills/concise-config/SKILL.md index a089a34..7a12133 100644 --- a/plugins/concise/skills/concise-config/SKILL.md +++ b/plugins/concise/skills/concise-config/SKILL.md @@ -60,7 +60,7 @@ node "${CLAUDE_PLUGIN_ROOT}/scripts/concise-config.mjs" set maxCommentLines 4 -- | `caseSensitive` | `false` by default. | | `flags` | Regex flags for `regex` entries. Default `i`. | | `hooks` | `edit`, `bash`, `stop`, `subagentStop`. Omit for all. | -| `scopes` | `files`, `comments`, `gh`, `commit`, `command`, `reply`. Omit for every scope except `command`. | +| `scopes` | `files`, `comments`, `code`, `gh`, `commit`, `command`, `reply`. Omit for every scope except `command` and `code`. | Pick `match` and `on` from what the user wants to catch: diff --git a/plugins/concise/skills/concise-rules/SKILL.md b/plugins/concise/skills/concise-rules/SKILL.md index cbe70dc..fb83dcb 100644 --- a/plugins/concise/skills/concise-rules/SKILL.md +++ b/plugins/concise/skills/concise-rules/SKILL.md @@ -34,17 +34,25 @@ The 2 style checks ship disabled. Turn either one on per project in `.claude/con A third style check, the dictionary (`features.dictionary`), flags terms the user lists. It runs when it has at least one entry. A deny tagged `[concise:dictionary:]` names the entry and the fix the user wrote for it. The same confirm flow applies. The `concise-config` skill covers the entry fields. 1. Em dash detection. Flags the em dash (`U+2014`). Flags the en dash (`U+2013`) when `enDash` is true. Flags `--` between word characters or between spaces when `doubleHyphen` is true. A `--flag` at the start of a token never matches. -2. AI writing patterns. Flags 44 categories of phrasing, commit hygiene, punctuation, and text statistics, shipped as pattern packs and selected by a preset or by an explicit category list. +2. AI writing patterns. Flags 47 categories of phrasing, commit hygiene, punctuation, and text statistics, shipped as pattern packs and selected by a preset or by an explicit category list. ### What gets scanned - Prose files (`md`, `mdx`, `markdown`, `txt`, `rst`, `adoc`, `asciidoc`): the whole written text. Fenced blocks, inline code, URLs, and HTML comments are blanked first, so an example inside backticks is exempt. -- Code files: comment runs only, the same runs the comment-length check reads. String literals and code are never scanned. -- Every other extension (`json`, `csv`, lock files, unknown): nothing. -- `gh pr` and `gh issue` bodies, inline `--body` and heredoc forms. -- `git commit` messages from `-m`, repeated `-m`, `--message=`, and the heredoc form. +- Code files: comment runs, the same runs the comment-length check reads. Packs with the `code` scope, `hidden-characters` by default, read the whole file, string literals included. +- Every other extension (`json`, `csv`, lock files, unknown): the whole file, for `code`-scope packs only. +- Notebook cells: a markdown cell as prose, a code cell as code. +- A file that `cat` or `tee` writes from a heredoc, before the command runs. After a shell command runs, any other file it changed in a git work tree; findings come back next to the command output, so fix them with Edit. +- `gh pr`, `gh issue`, and `gh release` titles and bodies: inline `--title`, `--body`, or `--notes`, the heredoc form, and `--body-file` or `-F`. `gh api` request fields `body`, `message`, `description`, and `title`. +- `git commit`, `git tag`, `git merge`, and `git notes` messages from `-m`, repeated `-m`, `--message=`, `-F`, `--trailer`, and the heredoc form, with `git -C ` or `-c` options in front. `jj` and `hg` messages the same way. +- Text an MCP tool posts (a PR body, an issue, a chat message, a doc page) and files it writes, read like a Write of that file. +- The plan you show with `ExitPlanMode`, `TaskCreate` and `TaskUpdate` text, and `AskUserQuestion` questions and options. - Your final chat reply, through the `Stop` hook, when `replies` is true. +Each of these past Write and Edit has a switch under `scan`: `codeFiles`, `notebooks`, `heredocWrites`, `shellWrites`, `mcp`, `plans`, `tasks`, `questions`. + +Not scanned: your text between tool calls, page text in HTML files, and shell writes outside a git work tree. + Only the text being written is scanned, as with the other 3 checks. ### The confirm flow @@ -97,7 +105,7 @@ Only the text being written is scanned, as with the other 3 checks. ### Environment control -Both features, the 3 core checks, and the `Stop` hook also answer to `BEC_` environment variables, so a session can differ from the project file. `BEC_FEATURE_ENABLE=emDash,aiWriting` turns the style checks on, and `BEC_FEATURE_ALWAYS_DISABLE=aiWriting` turns one off over the project file. Feature ids are `emDash`, `aiWriting`, `comments`, `fileSize`, `prBody`, and `stopHook`. `BEC_HOOK_SOFT_FAIL=1` downgrades every deny and block to a flagged allow, `BEC_DISABLE_STOP_HOOK=1` silences the reply check, and `BEC_LOG_ENABLED=1` writes one record per hook call to `~/.cache/concise/concise.log`. The `allowList` and `bypass` keys hold phrases and regex strings: an `allowList` entry drops the findings on a line, and a `bypass` entry allows the whole tool call with a flag. The full table is in [../../docs/environment.md](../../docs/environment.md). +Both features, the 3 core checks, and the `Stop` hook also answer to `BEC_` environment variables, so a session can differ from the project file. `BEC_FEATURE_ENABLE=emDash,aiWriting` turns the style checks on, and `BEC_FEATURE_ALWAYS_DISABLE=aiWriting` turns one off over the project file. Feature ids are `emDash`, `aiWriting`, `dictionary`, `comments`, `fileSize`, `prBody`, `stopHook`, and the `scan` switches. `BEC_HOOK_SOFT_FAIL=1` downgrades every deny and block to a flagged allow, `BEC_DISABLE_STOP_HOOK=1` silences the reply check, and `BEC_LOG_ENABLED=1` writes one record per hook call to `~/.cache/concise/concise.log`. The `allowList` and `bypass` keys hold phrases and regex strings: an `allowList` entry drops the findings on a line, and a `bypass` entry allows the whole tool call with a flag. The full table is in [../../docs/environment.md](../../docs/environment.md). ### Presets diff --git a/plugins/concise/skills/concise-rules/references/ai-speak-patterns-4.md b/plugins/concise/skills/concise-rules/references/ai-speak-patterns-4.md index 2032fd6..77caa04 100644 --- a/plugins/concise/skills/concise-rules/references/ai-speak-patterns-4.md +++ b/plugins/concise/skills/concise-rules/references/ai-speak-patterns-4.md @@ -112,9 +112,9 @@ Invisible or look-alike characters that model output and chat UIs leave in text. | a Unicode tag character | remove the tag character, it can hide text | | a variation selector other than the emoji and text selectors | remove the variation selector, it can hide data | | a bidirectional control character | remove the bidi control | -| `citeturn0search0`, `contentReference[oaicite:0]` | remove the citation residue | +| ChatGPT citation markers: `cite` followed by `turn0search0`, or `contentReference` followed by an `oaicite` index | remove the citation residue | -Anthropic's text watermark adds no characters, so this pack cannot see it. U+00A0 is left out: npm READMEs carry it 5.6 times per 10,000 words. A zero-width joiner inside an emoji sequence and tag characters after the black flag (subdivision flags) stay unflagged. A right-to-left project lists `hidden-characters` in `excludePacks` or drops the bidi row with its own pack. +Anthropic's text watermark adds no characters, so this pack cannot see it. In a code file the pack reads the whole file, string literals included, through the `code` scope. U+00A0 is left out: npm READMEs carry it 5.6 times per 10,000 words. A zero-width joiner inside an emoji sequence and tag characters after the black flag (subdivision flags) stay unflagged. A right-to-left project lists `hidden-characters` in `excludePacks` or drops the bidi row with its own pack. ## smart-punctuation diff --git a/plugins/concise/test/bash-text-tests.mjs b/plugins/concise/test/bash-text-tests.mjs new file mode 100644 index 0000000..e46fb18 --- /dev/null +++ b/plugins/concise/test/bash-text-tests.mjs @@ -0,0 +1,163 @@ +#!/usr/bin/env node +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { ok, bad, summary, withConfig, run, CHECK_EDIT, CHECK_BASH, assertDenied, assertAllowed, assertFlagged } from "./lib.mjs"; +import { apiFields, extractTitle, maskHeredocs } from "../hooks/lib/pr-body.mjs"; +import { gitCommitMessages } from "../hooks/lib/prose.mjs"; +import { fileFlagPaths } from "../hooks/lib/shell-text.mjs"; + +const dirs = []; +let seq = 0; + +function project(extra = {}) { + const dir = mkdtempSync(join(tmpdir(), "concise-bash-text-")); + dirs.push(dir); + // softFail is off here because a user config on the test machine can turn it on. + withConfig(dir, { softFail: false, features: { aiWriting: { enabled: true, preset: "default" } }, ...extra }); + return dir; +} + +const event = (dir, command, sid = `bash-text-${process.pid}-${++seq}`) => ({ tool_name: "Bash", tool_input: { command }, cwd: dir, session_id: sid }); +const reasonOf = (result) => result.hookSpecificOutput?.permissionDecisionReason || result.systemMessage || ""; +const DELVE = "We delve into the parser."; +const LONG = Array.from({ length: 12 }, (_, i) => `Paragraph ${i} explains one part of the change in plain words.`).join("\n\n"); + +function eq(name, actual, expected) { + if (JSON.stringify(actual) === JSON.stringify(expected)) return ok(name); + bad(name, `expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`); +} + +function includes(name, result, needle) { + if (reasonOf(result).includes(needle)) return ok(name); + bad(name, `expected ${JSON.stringify(needle)} in ${JSON.stringify(reasonOf(result)).slice(0, 300)}`); +} + +console.log("\nbash text: gh api request bodies"); + +{ + const dir = project(); + writeFileSync(join(dir, "body.md"), `${DELVE}\n`); + writeFileSync(join(dir, "req.json"), JSON.stringify({ tag_name: "v1", body: DELVE })); + writeFileSync(join(dir, "bad.json"), "{not json"); + const api = (args) => run(CHECK_BASH, event(dir, `gh api repos/o/r/issues/1/comments ${args}`)); + const raw = api(`-f body='${DELVE}'`); + assertDenied("gh api -f body is read", raw); + includes("the finding names the API request body", raw, "in API request body"); + assertDenied("a quoted -f 'key=value' is read", api(`-f 'body=${DELVE}'`)); + assertDenied("--raw-field=key=value is read", api(`--raw-field=title="${DELVE}"`)); + assertDenied("-F message is read", api(`-F message="${DELVE}"`)); + assertDenied("--field description is read", api(`--field "description=${DELVE}"`)); + assertDenied("-F key=@file reads the file", api("-F body=@body.md")); + assertDenied("-F key=@- reads the heredoc", api(`-F body=@- <<'EOF'\n${DELVE}\nEOF`)); + assertDenied("a heredoc inside a field value is read", api(`-f body="$(cat <<'EOF'\n${DELVE}\nEOF\n)"`)); + assertDenied("--input reads the JSON file", api("--input req.json")); + assertDenied("--input - reads JSON from the heredoc", api(`--input - <<'EOF'\n{"title": "${DELVE}"}\nEOF`)); + assertAllowed("a clean field passes", api("-f body='The parser keeps the header field.'")); + assertAllowed("a field outside the text keys is skipped", api(`-f state='${DELVE}'`)); + assertAllowed("an --input file that is not JSON is skipped", api("--input bad.json")); + assertAllowed("API bodies skip the PR prose limit", api(`-f body="${LONG}"`)); +} + +{ + // A bypass phrase answers only once config loads, so an empty result shows the early exit. + const dir = project({ bypass: { phrases: ["repos/"] } }); + eq("a gh api GET exits before it loads config", run(CHECK_BASH, event(dir, "gh api repos/o/r/pulls --jq '.[].title'")), {}); + includes("a call with a text field loads config", run(CHECK_BASH, event(dir, "gh api repos/o/r/issues -f title='Plain words'")), "Allowed by bypass phrase"); +} + +{ + const dir = project(); + const write = "cat > body.md <<'EOF'\nWe delve into it.\nEOF"; + const consumed = `${write}\ngh api repos/o/r/issues/1/comments -F body=@body.md`; + assertDenied("check-edit reads a heredoc file on its own", run(CHECK_EDIT, event(dir, write))); + assertAllowed("check-edit leaves a file that gh api -F consumes to check-bash", run(CHECK_EDIT, event(dir, consumed))); + assertDenied("check-bash reads that file", run(CHECK_BASH, event(dir, consumed))); + const json = `cat > req.json <<'EOF'\n{"body": "We delve into it."}\nEOF\ngh api repos/o/r/issues --input req.json`; + assertDenied("check-bash reads a heredoc JSON file that --input consumes", run(CHECK_BASH, event(dir, json))); +} + +console.log("\nbash text: titles"); + +{ + const dir = project(); + const bash = (command) => run(CHECK_BASH, event(dir, command)); + const both = bash(`gh pr create --title "We delve into the parser" --body "We delve into the lexer."`); + assertDenied("a PR title and body are read", both); + includes("the deny names the title", both, "in PR title."); + includes("the same deny names the body", both, "in PR body."); + assertDenied("a title with no body is read", bash(`gh pr edit 3 --title "We delve into the parser"`)); + assertDenied("an issue -t title is read", bash(`gh issue create -t 'We delve into the parser' --body "Steps below."`)); + includes("a merge subject is read", bash(`gh pr merge 3 --squash --subject "We delve into the parser"`), "in merge subject."); + includes("a release title is read", bash(`gh release edit v1 --title "We delve into the release"`), "in release title."); + assertAllowed("a long release body with a clean title passes", bash(`gh release create v1 --title "v1.0.0" --notes "${LONG}"`)); + assertAllowed("the prose limit counts only the body", bash(`gh pr create --title "One. Two. Three. Four. Five." --body "Short body."`)); + assertAllowed("concise-ignore in the title skips the scan", bash(`gh pr create --title "We delve into it concise-ignore" --body "Adds the parser."`)); +} + +{ + const dir = project({ features: { aiWriting: { enabled: true, preset: "default", mode: "ask" } } }); + const asked = run(CHECK_BASH, event(dir, `gh pr create --title "We delve into the parser" --body "We delve into the lexer."`)); + eq("ask mode asks once for the title and body", asked.hookSpecificOutput?.permissionDecision, "ask"); + includes("the ask names the title", asked, "in PR title."); + includes("the ask names the body", asked, "in PR body."); +} + +{ + const dir = project(); + const sid = `bash-text-retry-${process.pid}`; + const pr = (title) => run(CHECK_BASH, event(dir, `gh pr create --base main --title "${title}" --body "Adds the parser."`, sid)); + assertDenied("a flagged title is held", pr("We delve into the parser")); + assertDenied("a revised title on the same PR is held again", pr("We delve into the lexer")); + assertFlagged("title revisions share one retry counter", pr("We delve into the tokens")); +} + +console.log("\nbash text: jj and hg messages"); + +{ + const dir = project(); + writeFileSync(join(dir, "msg.txt"), `${DELVE}\n`); + const bash = (command) => run(CHECK_BASH, event(dir, command)); + const jj = bash(`jj describe -m "${DELVE}"`); + assertDenied("jj describe -m is read", jj); + includes("a jj message is a commit message", jj, "in commit message"); + assertDenied("jj new --message= is read", bash(`jj new --message="${DELVE}"`)); + assertDenied("hg commit -m is read", bash(`hg commit -m "${DELVE}"`)); + assertDenied("hg ci -l file is read", bash("hg ci -l msg.txt")); + assertDenied("hg commit --logfile - reads the heredoc", bash(`hg commit --logfile - <<'EOF'\n${DELVE}\nEOF`)); + assertAllowed("a clean jj message passes", bash('jj commit -m "Fix the parser header field"')); + assertAllowed("jj log carries no message", bash("jj log -r @")); +} + +console.log("\nbash text: helpers"); + +eq("apiFields reads each field form", apiFields(`gh api x -f body='a b' -f "title=c" --raw-field=description="d" -F message=e --field=body=f -f state=g`), ["a b", "c", "d", "e", "f"]); +eq("apiFields reads every gh api call", apiFields("gh api x -f body=one && gh api y -f body=two"), ["one", "two"]); +eq("apiFields reads @- from its own heredoc", apiFields("cat > a.md <<'EOF'\nfirst\nEOF\ngh api x -F body=@- <<'EOF'\nsecond\nEOF"), ["second"]); +eq("apiFields takes top-level JSON strings", apiFields(`gh api x --input - <<'EOF'\n{"body": "a", "title": 3, "head": "b", "comments": [{"body": "c"}]}\nEOF`), ["a"]); +eq("apiFields keeps a raw -f @ value as text", apiFields("gh api x -f body=@notes.md"), ["@notes.md"]); +eq("apiFields skips a missing @file", apiFields("gh api x -F body=@/nonexistent.md"), []); +eq("apiFields reads nothing from a GET", apiFields("gh api repos/o/r/pulls --jq '.[].title'"), []); +eq("apiFields skips other commands", apiFields("gh pr view 3"), []); +eq("extractTitle reads --title", extractTitle('gh pr create --title "Fix it" --body x'), "Fix it"); +eq("extractTitle reads a single-quoted -t", extractTitle("gh issue create -t 'Fix it'"), "Fix it"); +eq("extractTitle reads --subject=", extractTitle('gh pr merge 3 --subject="feat: x"'), "feat: x"); +eq("extractTitle skips a flag inside a heredoc", extractTitle(`gh pr create --body "$(cat <<'EOF'\nUse --title "x".\nEOF\n)" --title "Real"`), "Real"); +eq("extractTitle returns null without a title", extractTitle("gh pr comment 3 --body x"), null); +{ + const { masked, bodyIn } = maskHeredocs("a <<'EOF'\nx\nEOF\nb"); + eq("maskHeredocs swaps a heredoc for a placeholder", [masked, bodyIn(masked), bodyIn("b")], ["a {{concise-heredoc-0}}\nb", "x", null]); +} +eq("fileFlagPaths reads gh api @files and --input", fileFlagPaths("gh api x -F body=@notes.md -F title=plain --input req.json -f body=@raw.md"), ["notes.md", "req.json"]); +eq("fileFlagPaths needs a space or = after a long flag", fileFlagPaths("tool --files a --inputs b"), []); +eq("gitCommitMessages reads jj -R repo desc", gitCommitMessages('jj -R ../repo desc -m "x"'), ["x"]); +eq("gitCommitMessages reads jj split", gitCommitMessages("jj split -m 'first part'"), ["first part"]); +eq("gitCommitMessages reads hg -l -", gitCommitMessages("hg commit -l - <<'EOF'\nbody\nEOF"), ["body"]); +eq("gitCommitMessages skips jj log", gitCommitMessages('jj log -T "x"'), []); +eq("git tag -l reads no message", gitCommitMessages("git tag -l 'v1*'"), []); + +for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); + +const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ""; +if (import.meta.url === entry) process.exit(summary()); diff --git a/plugins/concise/test/context-tests.mjs b/plugins/concise/test/context-tests.mjs index 275d046..992ffbd 100644 --- a/plugins/concise/test/context-tests.mjs +++ b/plugins/concise/test/context-tests.mjs @@ -134,7 +134,7 @@ console.log("\ncontext and optional model hooks"); const shellHooks = claude.hooks.PreToolUse.find((group) => group.matcher === "Bash").hooks; const matches = (condition, command) => new RegExp(`^${condition.slice(5, -1).split("*").map((part) => part.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")).join("[\\s\\S]*")}$`).test(command); for (const [script, commands] of [ - ["check-bash.mjs", ["FOO=1 gh pr create --body x", "echo ok && git commit -m x", "echo $(gh issue comment 1 --body x)", "echo `git commit -m x`"]], + ["check-bash.mjs", ["FOO=1 gh pr create --body x", "echo ok && git commit -m x", "echo $(gh issue comment 1 --body x)", "echo `git commit -m x`", "jj describe -m x"]], ["monitor-filter.mjs", ["pytest", "npm test", "go test ./...", "FOO=1 npx jest", "echo $(vitest)", "echo ok && jest"]], ["check-edit.mjs", ["FOO=1 apply_patch <<'PATCH'\n*** Begin Patch\n*** End Patch\nPATCH", "python -c '*** Begin Patch\n*** End Patch'"]], ]) { diff --git a/plugins/concise/test/hook-gaps-tests.mjs b/plugins/concise/test/hook-gaps-tests.mjs new file mode 100644 index 0000000..b7326fe --- /dev/null +++ b/plugins/concise/test/hook-gaps-tests.mjs @@ -0,0 +1,257 @@ +#!/usr/bin/env node +import { mkdtempSync, writeFileSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { ROOT, ok, bad, summary, withConfig, run, CHECK_EDIT, CHECK_BASH, assertDenied, assertAllowed } from "./lib.mjs"; +import { gitCommitMessages } from "../hooks/lib/prose.mjs"; +import { extractBody } from "../hooks/lib/pr-body.mjs"; +import { heredocWrites, fileFlagPaths, readMessageFile, segmentFrom } from "../hooks/lib/shell-text.mjs"; +import { checkText } from "../tools/check.mjs"; +import { loadConfig } from "../hooks/lib/config.mjs"; +import { claim, cleanupSession } from "../hooks/lib/state.mjs"; + +const CHECK_TOOL = join(ROOT, "hooks", "check-tool-text.mjs"); +const ZWSP = "\u200B"; +const dirs = []; +let seq = 0; + +function project(aiWriting = {}, extra = {}) { + const dir = mkdtempSync(join(tmpdir(), "concise-gaps-")); + dirs.push(dir); + // softFail and mode are set here because a user config on the test machine can change them. + withConfig(dir, { softFail: false, features: { aiWriting: { enabled: true, preset: "default", mode: "confirm", ...aiWriting } }, ...extra }); + return dir; +} + +const event = (dir, tool_name, tool_input) => ({ tool_name, tool_input, cwd: dir, session_id: `gaps-${process.pid}-${++seq}` }); +const reasonOf = (result) => result.hookSpecificOutput?.permissionDecisionReason || result.systemMessage || ""; + +function eq(name, actual, expected) { + if (JSON.stringify(actual) === JSON.stringify(expected)) return ok(name); + bad(name, `expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`); +} + +function includes(name, result, needle) { + if (reasonOf(result).includes(needle)) return ok(name); + bad(name, `expected ${JSON.stringify(needle)} in ${JSON.stringify(reasonOf(result)).slice(0, 300)}`); +} + +console.log("\nhook gaps: the code scope"); + +{ + const dir = project(); + const ts = (content) => run(CHECK_EDIT, event(dir, "Write", { file_path: join(dir, "a.ts"), content })); + const hit = ts(`export const label = "zero${ZWSP}width";\n`); + assertDenied("a zero-width space in a string literal is flagged", hit); + includes("the finding names hidden-characters", hit, "[concise:hidden-characters]"); + assertAllowed("a code file without hidden characters passes", ts('export const label = "plain";\n')); + assertDenied("an unknown extension is scanned whole", run(CHECK_EDIT, event(dir, "Write", { file_path: join(dir, "a.json"), content: `{"a": "b\u202Ec"}\n` }))); + assertAllowed("prose packs stay out of code text", ts('export const note = "We delve into it.";\n')); +} + +{ + const dir = project({}, { features: { aiWriting: { enabled: false }, dictionary: { enabled: true, mode: "deny", entries: [ + { id: "no-foo", match: "exact", value: "fooBar", fix: "use bazQux", scopes: ["code"] }, + { id: "comment-only", match: "exact", value: "fooBar", fix: "rename" }, + ] } } }); + const result = run(CHECK_EDIT, event(dir, "Write", { file_path: join(dir, "b.js"), content: "const fooBar = 1;\n" })); + assertDenied("a dictionary entry scoped to code flags an identifier", result); + includes("only the code-scoped entry reports", result, "no-foo"); + if (reasonOf(result).includes("comment-only")) bad("a default-scoped entry skips code text", reasonOf(result)); + else ok("a default-scoped entry skips code text"); +} + +{ + const dir = project(); + const found = await checkText({ text: `const a = "x${ZWSP}y";\n`, scope: "code", cwd: dir, env: {} }); + eq("check_text takes the code scope", found.findings.map((f) => f.category), ["hidden-characters"]); + eq("check_text code scope uses a code path", found.path, "check.js"); +} + +console.log("\nhook gaps: commit messages"); + +{ + const dir = project(); + writeFileSync(join(dir, "msg.txt"), "We delve into the parser.\n"); + const bash = (command) => run(CHECK_BASH, event(dir, "Bash", { command })); + assertDenied("git -C dir commit is read", bash(`git -C ${dir} commit -m "We delve into the parser."`)); + assertDenied("git commit -F file is read", bash("git commit -F msg.txt")); + assertDenied("git commit -F - reads the heredoc", bash("git commit -F - <<'EOF'\nWe delve into the parser.\nEOF")); + assertDenied("a heredoc written and then passed to -F is read", bash("cat > m2.txt <<'EOF'\nWe delve into it.\nEOF\ngit commit -F m2.txt")); + assertDenied("a --trailer co-author is flagged", bash('git commit -m "fix parser" --trailer "Co-authored-by: Claude "')); + assertDenied("git tag -m is read", bash('git tag -a v1 -m "We delve into the release."')); + assertDenied("git merge -m is read", bash('git merge topic -m "We delve into the merge."')); + assertDenied("a PowerShell commit is read", run(CHECK_BASH, event(dir, "PowerShell", { command: 'git commit -m "We delve into it."' }))); + assertAllowed("git revert -m 1 carries no message", bash("git revert -m 1 HEAD")); +} + +eq("--trailer=key=value becomes key: value", gitCommitMessages('git commit -m "x" --trailer=Assisted-by=Codex'), ["x", "Assisted-by: Codex"]); +eq("a missing -F file adds nothing", gitCommitMessages("git commit -F /nonexistent/msg.txt"), []); +eq("-F before the git command is ignored", gitCommitMessages('grep -F foo bar && git commit -m "y"'), ["y"]); +eq("segmentFrom stops at && outside quotes", segmentFrom('a "x && y" && b', 0), 'a "x && y" '); +eq("segmentFrom keeps an escaped quote", segmentFrom('a "x \\" ; y"; b', 0), 'a "x \\" ; y"'); + +console.log("\nhook gaps: gh bodies"); + +{ + const dir = project(); + writeFileSync(join(dir, "body.md"), "We delve into the parser.\n"); + const bash = (command) => run(CHECK_BASH, event(dir, "Bash", { command })); + assertDenied("gh pr review -b is read", bash('gh pr review 3 --comment -b "We delve into the parser."')); + const notes = bash('gh release create v1 --notes "We delve into the release."'); + assertDenied("gh release --notes is read", notes); + includes("release notes are labeled", notes, "release notes"); + assertDenied("gh pr create --body-file is read from disk", bash("gh pr create --title x --body-file body.md")); + const long = Array.from({ length: 12 }, (_, i) => `Paragraph ${i} explains one part of the release in plain words.`).join("\n\n"); + assertAllowed("release notes skip the PR prose limit", bash(`gh release create v1 --notes "${long}"`)); + const written = "cat > /tmp/concise-gaps-body.md <<'EOF'\nWe delve into it.\nEOF\ngh pr create --title x --body-file /tmp/concise-gaps-body.md"; + assertDenied("a heredoc body file in the same command is read by check-bash", bash(written)); + assertAllowed("check-edit leaves a consumed body file to check-bash", run(CHECK_EDIT, event(dir, "Bash", { command: written }))); +} + +eq("extractBody reads -n for release notes", extractBody("gh release create v1 -n 'short notes'"), "short notes"); +eq("extractBody returns null for a missing body file", extractBody("gh pr create -F /nonexistent.md"), null); + +console.log("\nhook gaps: heredoc file writes"); + +{ + const dir = project(); + const bash = (command) => run(CHECK_EDIT, event(dir, "Bash", { command })); + assertDenied("cat > file < notes.md <<'EOF'\nWe delve into it.\nEOF")); + assertDenied("cat < file is read", bash("cat <<'EOF' > notes.md\nWe delve into it.\nEOF")); + assertDenied("tee file < out.md\nconsole.log('We delve');\nEOF")); + assertAllowed("a heredoc to /dev/null is skipped", bash("cat > /dev/null <<'EOF'\nWe delve into it.\nEOF")); +} + +eq("heredocWrites reads append and the path", heredocWrites("cat >> 'a b.md' < [w.path, w.append, w.body]), [["a b.md", true, "x"]]); +eq("heredocWrites skips a plain command", heredocWrites("ls -la"), []); +eq("fileFlagPaths finds body and notes files", fileFlagPaths("gh pr create --body-file a.md && gh release create v1 --notes-file=b.md"), ["a.md", "b.md"]); +eq("readMessageFile returns null for -", readMessageFile("-"), null); + +console.log("\nhook gaps: notebook cells"); + +{ + const dir = project(); + const notebook = join(dir, "n.ipynb"); + writeFileSync(notebook, JSON.stringify({ cells: [{ id: "m1", cell_type: "markdown", source: [] }], metadata: { language_info: { file_extension: ".py" } } })); + const cell = (input) => run(CHECK_EDIT, event(dir, "NotebookEdit", { notebook_path: notebook, ...input })); + assertDenied("a markdown cell is read as prose", cell({ cell_id: "m2", cell_type: "markdown", new_source: "We delve into the data." })); + assertDenied("the cell type comes from the notebook when it is left out", cell({ cell_id: "m1", new_source: "We delve into the data." })); + assertDenied("a code cell comment is read", cell({ cell_id: "c1", cell_type: "code", new_source: "# We delve into the data.\nx = 1\n" })); + assertAllowed("a code cell string is not prose", cell({ cell_id: "c1", cell_type: "code", new_source: 'x = "We delve into the data."\n' })); + assertAllowed("a deleted cell is skipped", cell({ cell_id: "m1", edit_mode: "delete", new_source: "We delve." })); + assertAllowed("a missing notebook falls back to code rules", run(CHECK_EDIT, event(dir, "NotebookEdit", { notebook_path: join(dir, "none.ipynb"), new_source: 'x = "We delve."' }))); +} + +console.log("\nhook gaps: MCP tool bodies"); + +{ + const dir = project(); + const mcp = (tool, input) => run(CHECK_TOOL, event(dir, tool, input)); + const pr = mcp("mcp__github__create_pull_request", { title: "x", body: "We delve into the parser and fix it." }); + assertDenied("a GitHub MCP PR body is read", pr); + includes("the label names the server, the tool, and the fields", pr, "github create_pull_request title and body"); + assertDenied("a chat message tool's text is read", mcp("mcp__slack__post_message", { channel: "c", text: "We delve into the outage today." })); + assertDenied("an issue tool's description is read", mcp("mcp__linear__create_issue", { description: "We delve into the outage today." })); + assertAllowed("a description on another tool is skipped", mcp("mcp__runpod__create_template", { description: "We delve into the outage today." })); + assertAllowed("concise's own tools are skipped", mcp("mcp__plugin_concise_concise__concise_check_text", { text: "We delve into it at length today." })); + assertAllowed("a short body is skipped", mcp("mcp__github__add_issue_comment", { body: "We delve." })); + assertAllowed("concise-ignore skips the body", mcp("mcp__github__add_issue_comment", { body: "We delve into it. concise-ignore" })); + assertAllowed("a clean body passes", mcp("mcp__github__add_issue_comment", { body: "The parser now keeps the header field." })); + assertAllowed("a non-MCP tool is skipped", mcp("Bash", { body: "We delve into the parser and fix it." })); + assertDenied("an email subject is read", mcp("mcp__gmail__send_email", { to: "a@b.c", subject: "We delve into the outage today" })); + assertDenied("a docs payload is read a few levels deep", mcp("mcp__claude_ai_Claude_Docs__update", { ref: { id: "1" }, payload: { text: "We delve into the plan for the release." } })); + const file = mcp("mcp__filesystem__write_file", { path: "notes.md", content: "We delve into it.\n" }); + assertDenied("an MCP file write is read as that file", file); + includes("the file finding names the path", file, "notes.md"); + assertAllowed("an MCP code file write keeps code rules", mcp("mcp__filesystem__write_file", { path: "a.ts", content: 'const a = "We delve into it.";\n' })); + assertDenied("MCP file edits are read", mcp("mcp__filesystem__edit_file", { path: "notes.md", edits: [{ oldText: "a", newText: "We delve into it." }] })); + assertAllowed("concise-ignore-file skips an MCP file write", mcp("mcp__filesystem__write_file", { path: "notes.md", content: "We delve. concise-ignore-file\n" })); + const pushed = mcp("mcp__github__push_files", { files: [{ path: "README.md", content: "Plain text.\n" }], message: "Add readme\n\nCo-authored-by: Claude " }); + assertDenied("the commit message of an MCP file push is read", pushed); + includes("the commit finding names the tool", pushed, "github push_files commit message"); +} + +{ + const dir = project(); + const sid = `gaps-retries-${process.pid}`; + const post = (body) => run(CHECK_TOOL, { tool_name: "mcp__github__add_issue_comment", tool_input: { issue_number: 7, body }, cwd: dir, session_id: sid }); + assertDenied("a flagged comment is held", post("We delve into the parser today.")); + assertDenied("a revised comment on the same issue is held again", post("We delve into the parser right now.")); + const third = post("We delve into the parser this week."); + assertAllowed("revisions share a retry counter, so the third goes through", third); + includes("the third revision is flagged", third, "Allowed through after 2 nudges"); +} + +console.log("\nhook gaps: plans, tasks, and questions"); + +{ + const dir = project(); + const tool = (name, input) => run(CHECK_TOOL, event(dir, name, input)); + assertDenied("a task subject is read", tool("TaskCreate", { subject: "Delve into the parser errors", description: "Find the cause." })); + assertDenied("a task update is read", tool("TaskUpdate", { taskId: "1", description: "We delve into the parser errors." })); + const question = { questions: [{ question: "Should we delve into the parser first?", header: "Order", options: [{ label: "Yes", description: "Start there" }, { label: "No", description: "Later" }] }] }; + assertDenied("a question to the user is read", tool("AskUserQuestion", question)); + assertAllowed("a clean task passes", tool("TaskCreate", { subject: "Fix the parser header field" })); + const plan = tool("ExitPlanMode", { plan: "## Plan\n\nWe delve into the parser first.\n", planFilePath: "/home/u/.claude/plans/p.md" }); + assertDenied("a plan is read although its file sits under .claude/", plan); + includes("the plan finding is labeled", plan, "plan"); + assertAllowed("scan.plans off skips plans", run(CHECK_TOOL, event(project({}, { scan: { plans: false } }), "ExitPlanMode", { plan: "We delve into the parser first." }))); + const quiet = project({ replies: false }); + assertAllowed("the reply switch covers tasks", run(CHECK_TOOL, event(quiet, "TaskCreate", { subject: "Delve into the parser errors" }))); +} + +console.log("\nhook gaps: scan switches"); + +{ + const off = (scan) => project({}, { scan }); + const notebook = (dir) => join(dir, "n.ipynb"); + assertAllowed("scan.codeFiles off skips the whole-file pass", run(CHECK_EDIT, event(off({ codeFiles: false }), "Write", { file_path: "/tmp/a.ts", content: `const a = "x${ZWSP}y";\n` }))); + const nb = off({ notebooks: false }); + assertAllowed("scan.notebooks off skips cells", run(CHECK_EDIT, event(nb, "NotebookEdit", { notebook_path: notebook(nb), cell_type: "markdown", new_source: "We delve into the data." }))); + const hd = off({ heredocWrites: false }); + assertAllowed("scan.heredocWrites off skips heredoc writes", run(CHECK_EDIT, event(hd, "Bash", { command: "cat > notes.md <<'EOF'\nWe delve into it.\nEOF" }))); + assertAllowed("scan.mcp off skips MCP posts", run(CHECK_TOOL, event(off({ mcp: false }), "mcp__github__add_issue_comment", { body: "We delve into the parser today." }))); + assertAllowed("scan.tasks off skips tasks", run(CHECK_TOOL, event(off({ tasks: false }), "TaskCreate", { subject: "Delve into the parser errors" }))); + assertAllowed("scan.questions off skips questions", run(CHECK_TOOL, event(off({ questions: false }), "AskUserQuestion", { questions: [{ question: "Should we delve into the parser first?" }] }))); + const ignored = project({}, { ignoreGlobs: ["**/*.ipynb"] }); + assertAllowed("ignoreGlobs match the notebook's own path", run(CHECK_EDIT, event(ignored, "NotebookEdit", { notebook_path: notebook(ignored), cell_type: "markdown", new_source: "We delve into the data." }))); + const marked = project(); + writeFileSync(notebook(marked), JSON.stringify({ cells: [{ id: "a", cell_type: "markdown", source: ["concise-ignore-file"] }] })); + assertAllowed("a marker in the notebook file exempts its cells", run(CHECK_EDIT, event(marked, "NotebookEdit", { notebook_path: notebook(marked), cell_type: "markdown", new_source: "We delve into the data." }))); +} + +{ + const config = loadConfig("/nonexistent", { BEC_FEATURE_DISABLE: "notebooks,mcp", BEC_CONFIG_JSON: JSON.stringify({ scan: { tasks: "no" } }) }); + eq("BEC_FEATURE_DISABLE turns scan switches off", [config.scan.notebooks, config.scan.mcp, config.scan.shellWrites], [false, false, true]); + eq("a scan switch that is not a boolean falls back to on", config.scan.tasks, true); + eq("the bad switch is reported", config.problems.map((problem) => problem.source), ["scan.tasks"]); +} + +console.log("\nhook gaps: overlapping handlers"); + +{ + const dir = project(); + const sid = `gaps-overlap-${process.pid}`; + const call = (script, command, id) => run(script, { tool_name: "Bash", tool_input: { command }, tool_use_id: id, cwd: dir, session_id: sid }); + const jj = 'jj describe -m "We delve into the bug."'; + assertDenied("the first run for a jj call decides", call(CHECK_BASH, jj, "toolu_jj")); + eq("a second run for the same jj call exits without output", call(CHECK_BASH, jj, "toolu_jj"), {}); + const piped = "cat <<'EOF' | tee notes.md\nWe delve into it.\nEOF"; + assertDenied("the first run for a cat and tee call decides", call(CHECK_EDIT, piped, "toolu_tee")); + eq("a second run for the same cat and tee call exits without output", call(CHECK_EDIT, piped, "toolu_tee"), {}); + const commit = 'git commit -m "We delve into the parser."'; + assertDenied("a call that cannot overlap is not claimed", call(CHECK_BASH, commit, "toolu_git")); + includes("so a repeat still runs and confirms", call(CHECK_BASH, commit, "toolu_git"), "Kept after confirmation"); + eq("a claim is taken once", [claim(sid, "k"), claim(sid, "k")], [true, false]); + eq("session cleanup removes claims", [cleanupSession(sid), claim(sid, "k")], [true, true]); + cleanupSession(sid); +} + +for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); + +const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ""; +if (import.meta.url === entry) process.exit(summary()); diff --git a/plugins/concise/test/shell-writes-tests.mjs b/plugins/concise/test/shell-writes-tests.mjs new file mode 100644 index 0000000..06ab365 --- /dev/null +++ b/plugins/concise/test/shell-writes-tests.mjs @@ -0,0 +1,238 @@ +#!/usr/bin/env node +import { execFileSync } from "node:child_process"; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, symlinkSync, unlinkSync, utimesSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { dirname, join } from "node:path"; +import { pathToFileURL } from "node:url"; +import { ROOT, ok, bad, run, summary, withConfig } from "./lib.mjs"; +import { assertBlocked, assertEmpty, includes, reasonOf } from "./features-lib.mjs"; + +const HOOK = join(ROOT, "hooks", "check-shell-writes.mjs"); +// The user's concise and git config stay out of the hook and the fixture repos. +const QUIET = { HOME: "", USERPROFILE: "", XDG_CONFIG_HOME: "", BEC_MONITOR_DISABLED: "1" }; +const GIT_ENV = { ...Object.fromEntries(Object.entries(process.env).filter(([name]) => !name.startsWith("GIT_"))), ...QUIET, GIT_CONFIG_NOSYSTEM: "1" }; +const DELVE = "We delve into it.\n"; +const dirs = []; +let seq = 0; + +const git = (dir, ...args) => execFileSync("git", args, { cwd: dir, env: GIT_ENV, stdio: "ignore" }); +const hook = (input) => run(HOOK, input, QUIET); + +function check(name, condition, detail) { + if (condition) return ok(name); + bad(name, JSON.stringify(detail).slice(0, 400)); +} + +function lacks(name, result, needle) { + check(name, !reasonOf(result).includes(needle), reasonOf(result)); +} + +function write(dir, name, text) { + const path = join(dir, name); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, text); + return path; +} + +function tempDir() { + const dir = realpathSync(mkdtempSync(join(tmpdir(), "concise-shell-writes-"))); + dirs.push(dir); + return dir; +} + +/** A repo with the project config and `files` committed, so only later writes show as changes. */ +function repo(config = {}, files = {}) { + const dir = tempDir(); + withConfig(dir, { softFail: false, features: { aiWriting: { enabled: true, preset: "default" } }, ...config }); + git(dir, "init", "-q"); + git(dir, "config", "user.name", "Concise Test"); + git(dir, "config", "user.email", "test@example.test"); + for (const [name, text] of Object.entries({ "base.md": "Plain base text.\n", ...files })) write(dir, name, text); + git(dir, "add", "-A"); + git(dir, "commit", "-q", "-m", "base"); + return dir; +} + +const event = (cwd, command, extra = {}) => ({ + hook_event_name: "PostToolUse", + tool_name: "Bash", + tool_input: { command }, + tool_response: { stdout: "", stderr: "", interrupted: false }, + tool_use_id: `toolu_${++seq}`, + cwd, + session_id: `shell-writes-${process.pid}-${seq}`, + duration_ms: 200, + ...extra, +}); + +console.log("\ncheck-shell-writes (files a shell command changed)"); + +{ + const dir = repo(); + const path = write(dir, "notes.md", DELVE); + const result = hook(event(dir, "python gen.py")); + assertBlocked("a new untracked .md file is reported", result); + includes("the report names the file and the finding", result, `[concise] 1 AI writing pattern in ${path}.\n[concise:vocabulary] 1 match at line 1: "delve"`); + includes("the report names the reference", result, "Fix it with Edit (read "); + check("the report ends with the fix", reasonOf(result).endsWith("or add concise-ignore on the line to keep it."), reasonOf(result)); +} + +{ + const dir = repo({}, { "guide.md": "We delve into the old parser.\n" }); + write(dir, "guide.md", "We delve into the old parser.\nWe delve into the new parser.\n"); + const result = hook(event(dir, "sed -i '$a We delve into the new parser.' guide.md")); + includes("only the added line of a tracked file is reported", result, "[concise] 1 AI writing pattern in"); + includes("the line number counts from the top of the file", result, "1 match at line 2:"); + const globbed = repo({}, { "a[1].md": "Plain text.\n", "a1.md": "Plain text.\n" }); + write(globbed, "a[1].md", "Plain text, changed.\n"); + write(globbed, "a1.md", `Plain text.\n${DELVE}`); + lacks("a file name with glob characters reads only its own diff", hook(event(globbed, "python gen.py")), `in ${join(globbed, "a[1].md")}`); +} + +{ + const dir = repo(); + const path = write(dir, "old.md", DELVE); + const minuteAgo = (Date.now() - 60_000) / 1000; + utimesSync(path, minuteAgo, minuteAgo); + assertEmpty("a file changed before the command started is skipped", hook(event(dir, "ls", { duration_ms: 1000 }))); + assertBlocked("a long command's window reaches back to it", hook(event(dir, "make docs", { duration_ms: 120_000 }))); +} + +{ + const command = "cat > notes.md <<'EOF'\nWe delve into it.\nEOF"; + const dir = repo(); + write(dir, "notes.md", DELVE); + assertEmpty("a heredoc write is left to check-edit", hook(event(dir, command))); + const off = repo({ scan: { heredocWrites: false } }); + write(off, "notes.md", DELVE); + assertBlocked("with scan.heredocWrites off, the heredoc write is scanned here", hook(event(off, command))); + const linked = repo(); + write(linked, "notes.md", DELVE); + const link = join(tempDir(), "link"); + symlinkSync(linked, link, "junction"); + assertEmpty("a heredoc write through a symlinked cwd is still skipped", hook(event(link, command))); +} + +{ + const dir = repo(); + write(dir, "notes.md", DELVE); + const guide = write(dir, "docs/guide.md", DELVE); + // loadConfig reads the project config from cwd only. + withConfig(join(dir, "docs"), { softFail: false, features: { aiWriting: { enabled: true, preset: "default" } } }); + const result = hook(event(join(dir, "docs"), "cat > ../notes.md <<'EOF'\nWe delve into it.\nEOF\npython gen.py")); + includes("a command run in a subdirectory sees the whole repo", result, guide); + lacks("a heredoc path relative to that subdirectory is skipped", result, join(dir, "notes.md")); +} + +{ + const dir = repo(); + write(dir, "body.md", DELVE); + assertEmpty("a --body-file path is left to check-bash", hook(event(dir, "gh pr create --title x --body-file body.md"))); + const patched = repo(); + write(patched, "patched.md", DELVE); + const patch = "apply_patch <<'PATCH'\n*** Begin Patch\n*** Add File: patched.md\n+We delve into it.\n*** End Patch\nPATCH"; + assertEmpty("a file an apply_patch in the command wrote is left to check-edit", hook(event(patched, patch))); +} + +{ + const dir = repo(); + write(dir, "notes.md", DELVE); + const input = event(dir, "python gen.py"); + assertBlocked("the first run reports the finding", hook(input)); + assertEmpty("the same finding is not reported twice in a session", hook(input)); + write(dir, "notes.md", `${DELVE}We delve into the docs.\n`); + const next = hook(input); + includes("a new finding in the same file is reported", next, "[concise] 1 AI writing pattern in"); + includes("the new finding is the only one", next, "1 match at line 2:"); +} + +{ + const off = repo({ scan: { shellWrites: false } }); + write(off, "notes.md", DELVE); + assertEmpty("scan.shellWrites false returns {}", hook(event(off, "python gen.py"))); + const plain = repo({ features: { aiWriting: { enabled: false } } }); + write(plain, "notes.md", DELVE); + assertEmpty("no style feature on returns {}", hook(event(plain, "python gen.py"))); + const outside = tempDir(); + withConfig(outside, { softFail: false, features: { aiWriting: { enabled: true } } }); + write(outside, "notes.md", DELVE); + assertEmpty("a directory outside git returns {}", hook(event(outside, "python gen.py"))); +} + +for (const [name, file, text, committed] of [ + ["a gitignored file is skipped", "ignored.md", DELVE, { ".gitignore": "ignored.md\n" }], + ["a binary file is skipped", "data.md", Buffer.from(`${DELVE}\0`)], + ["a file under ignoreGlobs is skipped", "node_modules/pkg/readme.md", DELVE], + ["an agent instruction file is skipped", "CLAUDE.md", DELVE], + ["concise-ignore-file skips the file", "marked.md", `\n${DELVE}`], + ["a file over 256 KB is skipped", "big.md", `${DELVE}${"Plain filler text.\n".repeat(15_000)}`], + ["concise-ignore on the line keeps it", "kept.md", "We delve into it. concise-ignore\n"], +]) { + const dir = repo({}, committed); + write(dir, file, text); + assertEmpty(name, hook(event(dir, "python gen.py"))); +} + +{ + const dir = repo(); + write(dir, "a.js", '// We delve into it.\nconst note = "We delve into it.";\n'); + const result = hook(event(dir, "node scripts/gen.mjs")); + includes("a comment in a .js file is reported", result, "1 match at line 1:"); + includes("a string in code is not prose", result, "[concise] 1 AI writing pattern in"); +} + +{ + const entries = [{ id: "no-foo", match: "exact", value: "fooBar", fix: "use bazQux" }]; + const dir = repo({ features: { emDash: { enabled: true }, dictionary: { enabled: true, entries } } }); + write(dir, "notes.md", "Call fooBar here.\nShip it — fast.\n"); + const result = hook(event(dir, "python gen.py")); + includes("a dictionary entry is reported", result, "[concise:dictionary:no-foo] 1 match at line 1:"); + includes("an em dash is reported", result, "[concise:emDash] 1 em dash at line 2:"); +} + +{ + const dir = repo(); + write(dir, "notes.md", DELVE); + assertBlocked("a PowerShell command is checked", hook(event(dir, "Set-Content notes.md 'We delve into it.'", { tool_name: "PowerShell" }))); + assertEmpty("a non-shell tool is ignored", hook(event(dir, "python gen.py", { tool_name: "Write" }))); + assertEmpty("a command that is not a string is ignored", hook(event(dir, null))); +} + +{ + const dir = repo({ bypass: { phrases: ["skip-concise"] } }); + write(dir, "notes.md", DELVE); + const result = hook(event(dir, "python gen.py # skip-concise")); + check("a bypass phrase allows the call", !result.decision && reasonOf(result).includes('Allowed by bypass phrase "skip-concise"'), result); +} + +{ + const dir = repo({ softFail: true }); + write(dir, "notes.md", DELVE); + const result = hook(event(dir, "python gen.py")); + const context = result.hookSpecificOutput?.additionalContext || ""; + check("soft fail drops the block", !result.decision, result); + check("soft fail keeps the report for the model", result.hookSpecificOutput?.hookEventName === "PostToolUse" && context.includes("soft-fail") && context.includes("[concise:vocabulary]"), result); +} + +{ + const dir = repo(); + for (let i = 0; i < 22; i += 1) write(dir, `n${String(i).padStart(2, "0")}.md`, DELVE); + const named = reasonOf(hook(event(dir, "python gen.py"))).match(/^\[concise\] 1 AI writing pattern in .*\.md\.$/gm) || []; + check("the scan stops at 20 files", named.length === 20, named.length); +} + +{ + const big = Array.from({ length: 60_000 }, (_, i) => `line ${i} of plain filler text`).join("\n"); + const dir = repo({}, { "gone.md": "Plain text.\n", "big.md": big }); + unlinkSync(join(dir, "gone.md")); + write(dir, "big.md", DELVE); + const path = write(dir, "notes.md", DELVE); + const result = hook(event(dir, "python gen.py && gh pr create --body-file missing.md")); + includes("a deleted file and a missing body file do not stop the scan", result, path); + lacks("a tracked file with a diff too big to read is skipped", result, join(dir, "big.md")); +} + +for (const dir of dirs) rmSync(dir, { recursive: true, force: true }); + +const entry = process.argv[1] ? pathToFileURL(process.argv[1]).href : ""; +if (import.meta.url === entry) process.exit(summary()); diff --git a/plugins/concise/tools/check.mjs b/plugins/concise/tools/check.mjs index 461eb3c..3f7b693 100644 --- a/plugins/concise/tools/check.mjs +++ b/plugins/concise/tools/check.mjs @@ -7,12 +7,13 @@ import { SCOPES } from "../hooks/lib/packs.mjs"; import { HOOKS } from "../hooks/lib/dictionary.mjs"; import { problem } from "../web/configuration.mjs"; -const HOOK_FOR = { files: "edit", comments: "edit", gh: "bash", commit: "bash", command: "bash", reply: "stop" }; -const PATH_FOR = { files: "check.md", comments: "check.js" }; +const HOOK_FOR = { files: "edit", comments: "edit", code: "edit", gh: "bash", commit: "bash", command: "bash", reply: "stop" }; +const PATH_FOR = { files: "check.md", comments: "check.js", code: "check.js" }; +const FILE_SCOPES = ["files", "comments", "code"]; const DASH_FIX = "a comma, period, colon, parentheses, or two sentences"; function rulesFor(config, scope, hook, path) { - const rules = path && ["files", "comments"].includes(scope) ? { ...config } : { ...config, ignoreGlobs: [], styleIgnoreGlobs: [] }; + const rules = path && FILE_SCOPES.includes(scope) ? { ...config } : { ...config, ignoreGlobs: [], styleIgnoreGlobs: [] }; if (scope !== "reply" && !["stop", "subagentStop"].includes(hook)) return rules; const { emDash, aiWriting } = config.features; return { @@ -32,14 +33,14 @@ function coreFindings(text, path, scope, config) { const verdict = isVerbose(text, { maxParagraphs: config.maxPrBodyParagraphs, maxSentences: config.maxPrBodySentences }); if (verdict.verbose) out.push({ check: "prBody", reason: verdict.reason }); } - if (scope === "comments" && checks.comments !== false) { + if (["comments", "code"].includes(scope) && checks.comments !== false) { for (const run of scanComments(text, path)) { if (run.length > config.maxCommentLines && !run.text.includes("concise-ignore")) { out.push({ check: "comments", line: run.startLine, reason: `comment run of ${run.length} lines (limit ${config.maxCommentLines})` }); } } } - if (["files", "comments"].includes(scope) && checks.fileSize !== false) { + if (FILE_SCOPES.includes(scope) && checks.fileSize !== false) { const lines = text.split("\n").length; if (lines > config.maxFileLines) out.push({ check: "fileSize", reason: `${lines} lines as a new file (limit ${config.maxFileLines})` }); } @@ -58,7 +59,8 @@ export async function checkText({ text, scope = "reply", hook, path, cwd = proce const target = path || PATH_FOR[scope] || "reply.md"; const rules = rulesFor(effective, scope, hookId, path); await prepareStyle(cwd, rules); - const found = styleFindings(text, target, rules, scope, hookId); + // A code file is checked as an edit is: comment runs, then the whole text for `code` packs. + const found = styleFindings(text, target, rules, scope === "code" ? "comments" : scope, hookId); const findings = [ ...found.emDash.map((hit) => ({ category: "emDash", match: hit.char, line: hit.line, fix: DASH_FIX })), ...found.aiWriting.map(({ category, match, line, fix }) => ({ category, match, line, fix })), diff --git a/plugins/concise/tools/mcp-server.mjs b/plugins/concise/tools/mcp-server.mjs index 586a257..872abbd 100644 --- a/plugins/concise/tools/mcp-server.mjs +++ b/plugins/concise/tools/mcp-server.mjs @@ -66,7 +66,7 @@ export const TOOLS = [ caseSensitive: flag("Match case. Defaults to false."), flags: text("Regex flags for match regex. Defaults to i."), hooks: list(HOOKS, "Hooks the entry runs in. Omit for all."), - scopes: list(SCOPES, "Text scopes the entry runs in. Omit for every scope except command."), + scopes: list(SCOPES, "Text scopes the entry runs in. Omit for every scope except command and code."), text: text("Sample text for action test."), scope: { type: "string", enum: SCOPES, description: "Scope for action test. Defaults to reply." }, hook: { type: "string", enum: HOOKS, description: "Hook for action test." }, @@ -80,7 +80,7 @@ export const TOOLS = [ description: "Run the concise checks over text the way a hook would, without changing retry or confirmation state.", inputSchema: schema({ text: text("Text to check."), - scope: { type: "string", enum: SCOPES, description: "files (prose), comments (code), gh, commit, command, or reply (default)." }, + scope: { type: "string", enum: SCOPES, description: "files (prose), comments (code comments), code (a whole code file), gh, commit, command, or reply (default)." }, hook: { type: "string", enum: HOOKS, description: "Hook to imitate. Defaults from the scope." }, path: text("File path that decides prose or comment rules and the ignore globs."), cwd, diff --git a/plugins/concise/web/public/configuration.mjs b/plugins/concise/web/public/configuration.mjs index 8e85f34..589e5dc 100644 --- a/plugins/concise/web/public/configuration.mjs +++ b/plugins/concise/web/public/configuration.mjs @@ -70,6 +70,7 @@ export function configurationView(ctx) { const presets = Array.isArray(state.presets) ? state.presets.map((preset) => typeof preset === "string" ? preset : preset.id || preset.name) : Object.keys(state.presets || {}); common.append( panel("Core checks", el("div", { class: "control-body" }, toggle("Comment length", "checks.comments"), toggle("File length", "checks.fileSize"), toggle("PR body length", "checks.prBody"), toggle("Final reply hook", "stopHook"), toggle("Soft fail", "softFail"))), + panel("Scanned text", el("div", { class: "control-body" }, toggle("Whole code files", "scan.codeFiles"), toggle("Notebook cells", "scan.notebooks"), toggle("Heredoc file writes", "scan.heredocWrites"), toggle("Files shell commands change", "scan.shellWrites"), toggle("MCP tool posts", "scan.mcp"), toggle("Plans", "scan.plans"), toggle("Tasks", "scan.tasks"), toggle("Questions to you", "scan.questions"))), panel("Thresholds", el("div", { class: "control-body fields-grid" }, number("Comment lines", "maxCommentLines"), number("File lines", "maxFileLines"), number("PR paragraphs", "maxPrBodyParagraphs"), number("PR sentences", "maxPrBodySentences"), number("Retries", "maxRetries"))), panel("AI writing", el("div", { class: "control-body" }, toggle("Enabled", "features.aiWriting.enabled"), toggle("Check final replies", "features.aiWriting.replies"), field("Preset", select(presets.length ? presets : ["default"], value("features.aiWriting.preset"), (event) => update("features.aiWriting.preset", event.target.value))), mode("features.aiWriting.mode"))), diff --git a/plugins/concise/web/testing/runner.mjs b/plugins/concise/web/testing/runner.mjs index 3b19f29..1d9e297 100644 --- a/plugins/concise/web/testing/runner.mjs +++ b/plugins/concise/web/testing/runner.mjs @@ -16,6 +16,7 @@ const KNOWN = new Map([ ['node "${CLAUDE_PLUGIN_ROOT}/hooks/check-edit.mjs"', { name: "check-edit", worker: true }], ['node "${CLAUDE_PLUGIN_ROOT}/hooks/check-bash.mjs"', { name: "check-bash", worker: true }], ['node "${CLAUDE_PLUGIN_ROOT}/hooks/check-reply.mjs"', { name: "check-reply", worker: true }], + ['node "${CLAUDE_PLUGIN_ROOT}/hooks/check-tool-text.mjs"', { name: "check-tool-text", file: "check-tool-text.mjs" }], ['node "${CLAUDE_PLUGIN_ROOT}/hooks/monitor-filter.mjs"', { name: "test-filter", file: "monitor-filter.mjs" }], ['bash "${CLAUDE_PLUGIN_ROOT}/hooks/PreToolUse-test-filter.sh"', { name: "test-filter", file: "PreToolUse-test-filter.sh", shell: true }], ]); @@ -92,7 +93,8 @@ async function applicable(request) { if (group.matcher && !new RegExp(`^(?:${group.matcher})$`).test(request.tool_name || "")) continue; for (const hook of group.hooks || []) { if (hook.type !== "command" || !KNOWN.has(hook.command)) throw new Error(`Unsupported hook command: ${hook.command}`); - hooks.push(KNOWN.get(hook.command)); + // The playground ignores `if`, so check-edit's several Bash handlers run once. + if (!hooks.includes(KNOWN.get(hook.command))) hooks.push(KNOWN.get(hook.command)); } } return hooks; diff --git a/plugins/concise/web/testing/scan.mjs b/plugins/concise/web/testing/scan.mjs index 01df931..f1e7de0 100644 --- a/plugins/concise/web/testing/scan.mjs +++ b/plugins/concise/web/testing/scan.mjs @@ -1,37 +1,11 @@ import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { isIgnored } from "../../hooks/lib/config.mjs"; import { bypassMatch } from "../../hooks/lib/hook-main.mjs"; -import { extractPatch, parseApplyPatch } from "../../hooks/lib/apply-patch.mjs"; -import { extractBody } from "../../hooks/lib/pr-body.mjs"; +import { targetsOf, isExempt } from "../../hooks/lib/edit-targets.mjs"; +import { GH_COMMAND, apiFields, extractBody, extractTitle } from "../../hooks/lib/pr-body.mjs"; import { gitCommitMessages, isProsePath } from "../../hooks/lib/prose.mjs"; import { prepareStyle, styleFindings } from "../../hooks/lib/style-check.mjs"; -function targets(input) { - const tool = input.tool_input || {}; - if (["Write", "Edit", "MultiEdit"].includes(input.tool_name)) { - if (!tool.file_path) return []; - const chunks = input.tool_name === "Write" ? [tool.content || ""] - : input.tool_name === "Edit" ? [tool.new_string || ""] - : (tool.edits || []).map((edit) => edit.new_string || ""); - return [{ path: tool.file_path, chunks, wholeFile: input.tool_name === "Write" }]; - } - const patch = input.tool_name === "apply_patch" ? tool.command || tool.input - : input.tool_name === "Bash" ? extractPatch(tool.command) : null; - return patch ? parseApplyPatch(patch).map((file) => ({ - path: resolve(input.cwd, file.path), chunks: file.chunks, wholeFile: file.kind === "add", - })) : []; -} - -function exempt(target) { - if (target.chunks.some((chunk) => chunk.includes("concise-ignore-file"))) return true; - if (target.wholeFile) return false; - try { - return readFileSync(target.path, "utf8").includes("concise-ignore-file"); - } catch { - return false; - } -} - function replyText(input) { const lines = readFileSync(input.transcript_path, "utf8").trim().split("\n"); for (const line of lines.reverse()) { @@ -68,22 +42,23 @@ export async function scan(input, config, hook) { ...result.dictionary.map((hit) => ({ ...hit, category: `dictionary:${hit.id}`, path, scope, chunk, hook }))); }; if (hook === "check-edit") { - const list = targets(input); + const list = targetsOf(input).filter((target) => config.scan?.[target.scan] !== false && !(target.file && isIgnored(target.file, config.ignoreGlobs))); if (bypassMatch(list.flatMap((target) => target.chunks), config)) return out; await prepareStyle(input.cwd, config); - for (const target of list.filter((item) => !exempt(item))) { + for (const target of list.filter((item) => !isExempt(item))) { target.chunks.forEach((text, index) => add(text, target.path, isProsePath(target.path) ? "files" : "comments", config, index)); } } else if (hook === "check-bash") { const command = input.tool_input?.command || ""; if (bypassMatch(command, config) || command.includes("concise-ignore")) return out; - const isGh = /\bgh\s+(pr|issue)\s+(create|comment|edit)\b/.test(command); - const messages = isGh ? [] : gitCommitMessages(command); - if (!isGh && !messages.length) return out; + const isGh = GH_COMMAND.test(command); + const posts = (isGh ? [extractTitle(command), extractBody(command, input.cwd)] : apiFields(command, input.cwd)).filter(Boolean); + const messages = isGh ? [] : gitCommitMessages(command, input.cwd); + if (!posts.length && !messages.length) return out; await prepareStyle(input.cwd, config); const rules = { ...config, ignoreGlobs: [], styleIgnoreGlobs: [] }; - const text = isGh ? extractBody(command) : messages.join("\n\n"); - if (text) add(text, "reply.md", isGh ? "gh" : "commit", rules); + posts.forEach((text, index) => add(text, "reply.md", "gh", rules, index)); + if (messages.length) add(messages.join("\n\n"), "reply.md", "commit", rules); add(command, "reply.md", "command", rules); } else if (hook === "check-reply" && config.stopHook) { const text = replySource(input);