diff --git a/.plannotator/tripwires.json b/.plannotator/tripwires.json new file mode 100644 index 000000000..6ef4ca1ba --- /dev/null +++ b/.plannotator/tripwires.json @@ -0,0 +1,26 @@ +{ + "rules": [ + { + "id": "annotation-pipeline", + "globs": ["packages/shared/external-annotation.ts"], + "note": "External annotation pipeline — agent findings, tour stops, and tripwires all flow through here. Changes affect every runtime." + }, + { + "id": "review-server-core", + "globs": ["packages/server/review.ts", "apps/pi-extension/server/serverReview.ts"], + "symbols": ["resolveAgentCwd", "waitForDecision"], + "note": "Review server lifecycle — cwd resolution and the decision promise are load-bearing for every integration." + }, + { + "id": "pi-vendoring", + "globs": ["apps/pi-extension/vendor.sh"], + "note": "Vendor list controls which shared modules reach Pi. A miss here silently breaks runtime parity." + }, + { + "id": "feedback-submission", + "globs": ["packages/review-editor/App.tsx"], + "symbols": ["exportReviewFeedback", "reviewerAnnotations"], + "note": "Feedback submission path — what reviewers send back to the agent. Double-check exclusion filters." + } + ] +} diff --git a/apps/copilot/commands/plannotator-review.md b/apps/copilot/commands/plannotator-review.md index de33be791..cec69dcb3 100644 --- a/apps/copilot/commands/plannotator-review.md +++ b/apps/copilot/commands/plannotator-review.md @@ -10,3 +10,12 @@ allowed-tools: shell(plannotator:*) ## Your task If the review above contains feedback or annotations, address them. If no changes were requested, acknowledge and continue. + +## Tripwires (slop-free zones) + +Two extra flags drive tripwires non-interactively (no review UI opens), with their output captured above: + +- `plannotator review --tripwires` (or `-t`) prints a scan report for the current diff — which configured slop-free zones the changes trip, plus the global and repo rule tables. Read it and address any tripped zones. +- `plannotator review --add-tripwire ` returns an instruction and a JSON rule snippet to apply. It does NOT write the file — apply the returned snippet yourself. + +Rules merge two layers: a private, auto-created global file at `~/.plannotator/tripwires/.json` (never committed) plus an optional committed `.plannotator/tripwires.json` at the repo root. diff --git a/apps/hook/server/cli.test.ts b/apps/hook/server/cli.test.ts index fb51ce33c..0a7fd74f8 100644 --- a/apps/hook/server/cli.test.ts +++ b/apps/hook/server/cli.test.ts @@ -21,7 +21,11 @@ describe("CLI top-level help", () => { expect(output).toContain("plannotator --help"); expect(output).toContain("plannotator --version, -v"); expect(output).toContain("plannotator [--browser ]"); - expect(output).toContain("plannotator review [--git] [PR_URL]"); + expect(output).toContain("plannotator review [--git] [--tripwires|-t] [--add-tripwire ] [PR_URL]"); + expect(output).toContain("plannotator tripwires "); + expect(output).toContain("plannotator tripwires add"); + expect(output).toContain("--tripwires"); + expect(output).toContain("--add-tripwire"); expect(output).toContain("plannotator annotate "); expect(output).toContain("plannotator annotate-last [--stdin]"); expect(output).toContain("plannotator setup-goal "); diff --git a/apps/hook/server/cli.ts b/apps/hook/server/cli.ts index e20287052..e904a8873 100644 --- a/apps/hook/server/cli.ts +++ b/apps/hook/server/cli.ts @@ -25,7 +25,9 @@ export function formatTopLevelHelp(): string { " plannotator --help", " plannotator --version, -v", " plannotator [--browser ]", - " plannotator review [--git] [PR_URL]", + " plannotator review [--git] [--tripwires|-t] [--add-tripwire ] [PR_URL]", + " plannotator tripwires ", + " plannotator tripwires add | --glob [--symbol ] [--note ] [--repo]", " plannotator annotate [--no-jina] [--gate] [--json] [--hook]", " plannotator annotate-last [--stdin] [--gate] [--json] [--hook]", " plannotator setup-goal [--json]", @@ -46,6 +48,7 @@ export function formatInteractiveNoArgClarification(): string { "", "For interactive use, try:", " plannotator review", + " plannotator tripwires list", " plannotator annotate ", " plannotator setup-goal interview bundle.json --json", " plannotator last", diff --git a/apps/hook/server/index.ts b/apps/hook/server/index.ts index 74f700fa6..877c290ad 100644 --- a/apps/hook/server/index.ts +++ b/apps/hook/server/index.ts @@ -86,6 +86,22 @@ import { import { type DiffType, prepareLocalReviewDiff, gitRuntime } from "@plannotator/server/vcs"; import { loadConfig, resolveDefaultDiffType, resolveUseJina } from "@plannotator/shared/config"; import { parseReviewArgs } from "@plannotator/shared/review-args"; +import { + evaluateTripwires, + formatTripwiresMarkdown, + buildAddTripwirePrompt, + appendRuleToTripwiresJson, + parseTripwiresConfigDetailed, +} from "@plannotator/shared/tripwires"; +import { + resolveMergedTripwires, + globalTripwiresPath, + readGlobalTripwiresFile, + readTripwiresFile, + repoRootFromCwd, + writeGlobalTripwiresFile, + writeRepoTripwiresFile, +} from "@plannotator/server/tripwires"; import { normalizeGoalSetupBundle, type GoalSetupStage, @@ -490,6 +506,178 @@ if (args[0] === "sessions") { } process.exit(0); +} else if (args[0] === "tripwires") { + // ============================================ + // TRIPWIRES MODE (list | validate | path) + // ============================================ + // + // Inspect the two-layer slop-free-zone config: a global per-project file + // under /tripwires/.json plus the repo-local + // .plannotator/tripwires.json. All subcommands are read-only and fail-open. + + const sub = args[1]; + + if (sub === "list") { + // Resolve key + root (ensures the global file exists), then re-read each + // layer separately so the report can group global vs. repo rules. + const { key, root } = await resolveMergedTripwires(process.cwd()); + const globalPath = key ? globalTripwiresPath(key) : "(unknown)"; + const globalRules = parseTripwiresConfigDetailed( + key ? readGlobalTripwiresFile(key) : null, + ).config.rules; + const repoPath = root ? `${root}/.plannotator/tripwires.json` : null; + const repoRules = root + ? parseTripwiresConfigDetailed(readTripwiresFile(root)).config.rules + : []; + + console.log( + formatTripwiresMarkdown({ + globalKey: key, + globalPath, + repoPath, + globalRules, + repoRules, + }), + ); + process.exit(0); + } + + if (sub === "validate") { + const { key, root } = await resolveMergedTripwires(process.cwd()); + const globalPath = key ? globalTripwiresPath(key) : "(unknown)"; + const repoPath = root ? `${root}/.plannotator/tripwires.json` : null; + + const globalResult = parseTripwiresConfigDetailed( + key ? readGlobalTripwiresFile(key) : null, + ); + const repoResult = parseTripwiresConfigDetailed( + root ? readTripwiresFile(root) : null, + ); + + let hasError = false; + const report = (label: string, path: string, result: ReturnType) => { + console.log(`${label} (${path})`); + if (result.diagnostics.length === 0) { + console.log(` ${result.config.rules.length} rule(s), no problems`); + } else { + for (const d of result.diagnostics) { + if (d.level === "error") hasError = true; + const where = typeof d.ruleIndex === "number" ? ` [rule ${d.ruleIndex}]` : ""; + console.log(` ${d.level}: ${d.message}${where}`); + } + } + }; + + report("Global", globalPath, globalResult); + report("Repo", repoPath ?? ".plannotator/tripwires.json", repoResult); + process.exit(hasError ? 1 : 0); + } + + if (sub === "path") { + const root = await repoRootFromCwd(process.cwd()); + const { key } = await resolveMergedTripwires(process.cwd()); + if (key) console.log(globalTripwiresPath(key)); + console.log(root ? `${root}/.plannotator/tripwires.json` : ".plannotator/tripwires.json"); + process.exit(0); + } + + if (sub === "add") { + // Two modes sharing one verb: + // structured flags (--glob ...) → write the rule directly + // free text → print the agent instruction (no write) + const globs: string[] = []; + const symbols: string[] = []; + let note: string | undefined; + let id: string | undefined; + let toRepo = false; + const descriptionParts: string[] = []; + + const rest = args.slice(2); + for (let i = 0; i < rest.length; i++) { + const token = rest[i]; + if (token === "--glob" || token === "-g") { + if (rest[i + 1]) globs.push(rest[++i]); + } else if (token === "--symbol" || token === "-s") { + if (rest[i + 1]) symbols.push(rest[++i]); + } else if (token === "--note") { + if (rest[i + 1]) note = rest[++i]; + } else if (token === "--id") { + if (rest[i + 1]) id = rest[++i]; + } else if (token === "--repo") { + toRepo = true; + } else { + descriptionParts.push(token); + } + } + + if (globs.length > 0) { + // Direct write. Explicit user action → errors are reported, not + // swallowed (unlike review-time fail-open reads). + const { root, key } = await resolveMergedTripwires(process.cwd()); + let targetPath: string; + let raw: string | null; + if (toRepo) { + if (!root) { + console.error("tripwires add --repo requires running inside a git repository."); + process.exit(1); + } + targetPath = `${root}/.plannotator/tripwires.json`; + raw = readTripwiresFile(root); + } else { + if (!key) { + console.error("Could not derive a project key (not inside a git repository?). Run from the repo, or use --repo inside one."); + process.exit(1); + } + targetPath = globalTripwiresPath(key); + raw = readGlobalTripwiresFile(key); + } + + const result = appendRuleToTripwiresJson(raw, { globs, symbols, note, id }); + if (!result.ok) { + console.error(`Cannot add rule: ${result.error}`); + process.exit(1); + } + try { + if (toRepo) { + writeRepoTripwiresFile(root!, result.json); + } else { + writeGlobalTripwiresFile(key!, result.json); + } + } catch (err) { + console.error(`Failed to write ${targetPath}:`, err instanceof Error ? err.message : err); + process.exit(1); + } + + console.log(`Added tripwire "${result.rule.id}" to ${toRepo ? "repo" : "global"} config:`); + console.log(` ${targetPath}`); + console.log(""); + console.log(JSON.stringify(result.rule, null, 2)); + console.log(""); + console.log("Run `plannotator tripwires list` to see all rules."); + process.exit(0); + } + + const description = descriptionParts.join(" ").trim(); + if (description) { + const { root, key } = await resolveMergedTripwires(process.cwd()); + console.log( + buildAddTripwirePrompt({ + description, + globalPath: key ? globalTripwiresPath(key) : undefined, + repoPath: root ? `${root}/.plannotator/tripwires.json` : undefined, + }), + ); + process.exit(0); + } + + console.error("Usage: plannotator tripwires add "); + console.error(" plannotator tripwires add --glob [--glob ...] [--symbol ...] [--note ] [--id ] [--repo]"); + process.exit(1); + } + + console.error("Usage: plannotator tripwires "); + process.exit(1); + } else if (args[0] === "review") { // ============================================ // CODE REVIEW MODE @@ -510,6 +698,45 @@ if (args[0] === "sessions") { let worktreePool: WorktreePool | undefined; let worktreeCleanup: (() => void | Promise) | undefined; + // Non-interactive tripwire flags short-circuit as soon as rawPatch is known — + // BEFORE the --local PR checkout (worktree/clone) so a PR scan never pays for a + // checkout it discards, and BEFORE detectProjectName/server startup. The scan + // only needs rawPatch. Keyed off the launch repo (no agentCwd yet → cwd falls + // through to gitContext?.cwd / process.cwd()), matching Pi and OpenCode. + const maybeRunTripwireShortCircuit = async (): Promise => { + if (reviewArgs.tripwires) { + const tripwireCwd = gitContext?.cwd ?? process.cwd(); + const { config, root, key } = await resolveMergedTripwires(tripwireCwd); + const hits = evaluateTripwires(rawPatch, config, { cwd: root ?? undefined }); + const globalPath = key ? globalTripwiresPath(key) : "(unknown)"; + const globalRules = parseTripwiresConfigDetailed( + key ? readGlobalTripwiresFile(key) : null, + ).config.rules; + const repoPath = root ? `${root}/.plannotator/tripwires.json` : null; + const repoRules = root + ? parseTripwiresConfigDetailed(readTripwiresFile(root)).config.rules + : []; + console.log( + formatTripwiresMarkdown({ globalKey: key, globalPath, repoPath, globalRules, repoRules, hits }), + ); + process.exit(0); + } + if (reviewArgs.addTripwire) { + // The description is natural language; resolve real paths so the + // emitted instruction points the agent at the exact files. + const tripwireCwd = gitContext?.cwd ?? process.cwd(); + const { root, key } = await resolveMergedTripwires(tripwireCwd); + console.log( + buildAddTripwirePrompt({ + description: reviewArgs.addTripwire, + globalPath: key ? globalTripwiresPath(key) : undefined, + repoPath: root ? `${root}/.plannotator/tripwires.json` : undefined, + }), + ); + process.exit(0); + } + }; + if (isPRMode) { // --- PR Review Mode --- const prRef = parsePRUrl(urlArg); @@ -548,6 +775,9 @@ if (args[0] === "sessions") { process.exit(1); } + // Scan with the PR rawPatch and exit before any --local checkout work. + await maybeRunTripwireShortCircuit(); + // --local: create a local checkout with the PR head for full file access if (useLocal && prMetadata) { // Hoisted so catch block can clean up partially-created directories @@ -709,6 +939,9 @@ if (args[0] === "sessions") { rawPatch = diffResult.rawPatch; gitRef = diffResult.gitRef; diffError = diffResult.error; + + // Local mode: rawPatch is ready, scan and exit before server startup. + await maybeRunTripwireShortCircuit(); } const reviewProject = (await detectProjectName()) ?? "_unknown"; @@ -1317,6 +1550,40 @@ if (args[0] === "sessions") { diffError = diffResult.error; } + // Non-interactive tripwire flags short-circuit BEFORE server startup. Emits + // {decision:"annotated", feedback, isPRMode:true} — the bridge returns + // feedback verbatim for isPRMode:true (no deny suffix), so no bridge change. + if (reviewArgs.tripwires) { + const tripwireCwd = gitContext?.cwd ?? process.env.PLANNOTATOR_CWD ?? process.cwd(); + const { config, root, key } = await resolveMergedTripwires(tripwireCwd); + const hits = evaluateTripwires(rawPatch, config, { cwd: root ?? undefined }); + const globalPath = key ? globalTripwiresPath(key) : "(unknown)"; + const globalRules = parseTripwiresConfigDetailed( + key ? readGlobalTripwiresFile(key) : null, + ).config.rules; + const repoPath = root ? `${root}/.plannotator/tripwires.json` : null; + const repoRules = root + ? parseTripwiresConfigDetailed(readTripwiresFile(root)).config.rules + : []; + const report = formatTripwiresMarkdown({ globalKey: key, globalPath, repoPath, globalRules, repoRules, hits }); + console.log(JSON.stringify({ decision: "annotated", feedback: report, isPRMode: true })); + process.exit(0); + } + if (reviewArgs.addTripwire) { + const tripwireCwd = gitContext?.cwd ?? process.env.PLANNOTATOR_CWD ?? process.cwd(); + const { root, key } = await resolveMergedTripwires(tripwireCwd); + console.log(JSON.stringify({ + decision: "annotated", + feedback: buildAddTripwirePrompt({ + description: reviewArgs.addTripwire, + globalPath: key ? globalTripwiresPath(key) : undefined, + repoPath: root ? `${root}/.plannotator/tripwires.json` : undefined, + }), + isPRMode: true, + })); + process.exit(0); + } + const bridgeSharingEnabled = getBridgeSharingEnabled(input); const bridgeShareBaseUrl = getBridgeShareBaseUrl(input); const reviewProject = (await detectProjectName()) ?? "_unknown"; diff --git a/apps/kiro-cli/skills/plannotator-review/SKILL.md b/apps/kiro-cli/skills/plannotator-review/SKILL.md index 3e13b55d4..ec983e6e8 100644 --- a/apps/kiro-cli/skills/plannotator-review/SKILL.md +++ b/apps/kiro-cli/skills/plannotator-review/SKILL.md @@ -17,3 +17,10 @@ You may append an optional PR URL: ```bash PLANNOTATOR_ORIGIN=kiro-cli plannotator review ``` + +You may also append the tripwire flags, whose stdout is captured back to you: +`--tripwires` (or `-t`) prints a slop-free-zone scan report for the current diff +(no UI opens), and `--add-tripwire ` returns a snippet to apply (it does +not write the file). Tripwire rules merge a private, auto-created global file at +`~/.plannotator/tripwires/.json` with an optional committed +`.plannotator/tripwires.json` at the repo root. diff --git a/apps/marketing/src/content/blog/slop-free-zones.md b/apps/marketing/src/content/blog/slop-free-zones.md new file mode 100644 index 000000000..90cc91b0a --- /dev/null +++ b/apps/marketing/src/content/blog/slop-free-zones.md @@ -0,0 +1,65 @@ +--- +title: "Slop-Free Zones: Tripwires for Code Agents Shouldn't Touch" +description: "Some code is too load-bearing to let an agent casually rewrite. Tripwires let you mark slop-free zones in your repo and surface an unmissable warning whenever a diff touches them." +date: 2026-06-05 +author: "backnotprop" +tags: ["tripwires", "code-review", "slop-free-zones"] +--- + +**Plannotator is an open-source review UI for AI coding agents.** The latest release adds Tripwires — a way to mark the parts of your codebase that agents and people must not casually touch, so any change that reaches them lights up during code review. + +## The problem + +Coding agents are confident. That's most of the time a feature. But confidence is dangerous around load-bearing code: the hand-tuned database query, the security check at the edge of an auth flow, the migration that already shipped to production, the one config value the whole deploy hinges on. An agent will rewrite any of it without hesitation, and the diff looks just as clean as every other diff. + +The risk isn't that the change is obviously wrong. It's that it's *quiet*. You're reviewing eight files of agent output, you're moving fast, and the one edit that actually matters scrolls by looking exactly like the boilerplate around it. By the time something breaks, the context is gone. + +## Slop-free zones + +The fix is to give that code a perimeter. A **slop-free zone** is a part of the codebase you've decided needs care — and Tripwires let you declare those zones with a few lines of JSON: + +```json +{ + "rules": [ + { + "globs": ["src/auth/**"], + "symbols": ["verifyToken", "hashPassword"], + "note": "Security-critical — changes need a second reviewer." + }, + { + "globs": ["migrations/**", "db/schema.sql"], + "note": "Schema changes ship to production — review carefully." + } + ] +} +``` + +The first rule watches the auth package, but only fires on lines that mention `verifyToken` or `hashPassword`. The second watches every migration and the schema file wholesale — no symbols, so any change at all trips it. + +These rules can live in two places, and Plannotator merges both. There's a **private global file** — auto-created the first time you review in a repo at `~/.plannotator/tripwires/.json` — that follows you across every repo you work in, no commit required. And there's an **optional per-repo file** at `.plannotator/tripwires.json` that you commit so the whole team gets the same zones. You can run with just one or layer both; the repo rules merge on top of your global ones. + +The semantics are deliberately simple: **any change that touches a tripwired file or symbol trips the wire** — adding, editing, deleting, or renaming. There's nothing to learn beyond globs and an optional list of names. + +## No gates, no modals — just amber + +The easiest version of this feature would be a blocking modal: "You're about to review changes to a protected file. Continue?" We didn't build that. Gates train you to dismiss them, and a dismissed gate protects nothing. + +Tripwires are passive on purpose. When a change touches a slop-free zone, you get an unmissable amber signal in three places — an amber gutter marker on the affected line, an amber-accented card in the review sidebar carrying your note, and an amber glyph on the file in the tree. Nothing stops you. Nothing demands a click. The warning is just *there*, exactly where you're already looking, and it stays accurate because it's re-evaluated on every diff and PR switch. + +And tripwires are strictly for you, the human reviewer. They are **never** included in the feedback sent back to the agent, and they don't count toward your annotation total. They're a heads-up display, not an instruction. + +If either config file is missing or malformed, tripwires simply do nothing for that layer — the two fail independently, so a bad repo file never wipes out your global rules and vice versa. One bad rule never discards the others, and you can't break review by writing a bad file. Fail-open, always. + +## Try it + +Update to the latest version: + +```bash +curl -fsSL https://plannotator.ai/install.sh | bash +``` + +Then just run `/plannotator-review` — the global tripwires file is auto-created for the repo, ready for your first rule. Add the files you care about most and watch the zones light up. + +Two flags make it scriptable. `plannotator review --tripwires` prints a scan report instead of opening the browser, so you (or an agent) can see exactly which wires the current diff trips. And `plannotator review --add-tripwire ` takes plain language — "protect the billing module and anything touching refunds" — and hands your agent precise instructions for turning that into a rule: explore the repo, pick the globs and symbols, write the config, validate. Prefer doing it yourself? `plannotator tripwires add --glob "src/billing/**" --note "money path"` writes the rule straight into your global config (or the repo's, with `--repo`). + +Full details are in the [Tripwires guide](/docs/guides/tripwires/). diff --git a/apps/marketing/src/content/docs/guides/tripwires.md b/apps/marketing/src/content/docs/guides/tripwires.md new file mode 100644 index 000000000..e345c3d56 --- /dev/null +++ b/apps/marketing/src/content/docs/guides/tripwires.md @@ -0,0 +1,159 @@ +--- +title: "Tripwires" +description: "Mark slop-free zones so any change that touches a protected file or symbol surfaces a warning during code review." +sidebar: + order: 17 +section: "Guides" +--- + +Tripwires let you mark **slop-free zones** — the parts of your codebase that agents and people must not casually touch. When a diff changes one of those files or symbols, Plannotator surfaces an unmissable amber warning during code review. No gates, no modals, no blocking. Just a clear signal that says "be careful, you're in a protected area." + +Some code is load-bearing in ways that aren't obvious from the diff: a hand-tuned query, a security check, a migration that already shipped, a config the whole deploy depends on. An agent will happily rewrite any of it with full confidence. Tripwires give that code a perimeter so a careless edit doesn't slip through review unnoticed. + +## Configuration + +Tripwires live in two layers that are merged together: + +- **Global** — a private file that is auto-created the first time you review in a repo and lives at `~/.plannotator/tripwires/.json` (under your home `~/.plannotator` data directory, or wherever `PLANNOTATOR_DATA_DIR` points). It is **never committed** — it follows you across machines only if you sync your data directory, and it stays with you across every repo you work in. +- **Repo** (optional) — a committed file at `.plannotator/tripwires.json` in the repo root. This is the team opt-in: anyone who clones the repo gets these rules. + +Both layers use the same shape. Each file has one top-level key, `rules`, which is an array. Each rule names the files it protects (`globs`), optionally the symbols within them (`symbols`), and an optional `note` explaining why the zone is protected. The two layers are merged additively — global rules first, then repo rules appended on top — so a repo rule never overrides a global one; both apply. + +```json +{ + "rules": [ + { + "id": "rule-id", + "globs": ["src/auth/**"], + "symbols": ["verifyToken", "hashPassword"], + "note": "Security-critical — changes need a second reviewer." + } + ] +} +``` + +| Field | Required | Description | +|-------|----------|-------------| +| `id` | No | Identifier for the rule. Defaults to `rule-` if omitted. | +| `globs` | Yes | One or more repo-relative glob patterns. A rule with no valid globs is dropped. | +| `symbols` | No | Names that narrow the rule to lines mentioning those symbols. Omit to protect every change in the matched files. | +| `note` | No | Message shown on the warning. Defaults to `Touches a slop-free zone`. | + +### Config locations + +The global file is keyed to the repository, not the folder. Plannotator derives the key from the repo's `origin` remote (so `git@github.com:you/app.git`, `https://github.com/you/app.git`, and an SSH URL with a custom port all resolve to the same project key). Repos with no remote fall back to the repository's shared git directory, so **linked worktrees of the same clone share one global file** rather than each getting their own. + +The two layers are merged by concatenation, global first. If a rule in each layer happens to share an `id`, the duplicate is renamed (`my-rule`, `my-rule-2`) so both still apply rather than one silently shadowing the other. + +You don't have to create either file by hand. The global file is created empty (`{ "rules": [] }`) the first time you review in a repo, and you can manage both layers from the command line: + +```bash +plannotator tripwires list # show the merged global + repo rules +plannotator tripwires validate # check both files and report any problems +plannotator tripwires path # print the path to each layer's file +``` + +Add rules directly with flags — global by default, or into the repo-committed file with `--repo` (the only time Plannotator ever writes inside your repo, because you asked): + +```bash +plannotator tripwires add --glob "src/billing/**" --symbol chargeCard --note "Money path" +plannotator tripwires add --glob "migrations/**" --repo # team-shared, committed +``` + +Or describe what you want protected in plain language — from the terminal or inside your agent: + +```bash +plannotator tripwires add protect the auth core and token validation +plannotator review --add-tripwire the billing module and anything touching refunds +``` + +The description form **prints instructions for your agent**: explore the repo, turn the description into concrete globs and symbols, write the rule into the indicated file, then validate and show the result. The command itself never writes files in this mode. + +### Example: protect whole files + +The simplest tripwire watches a set of files. Any change anywhere in them trips the wire. + +```json +{ + "rules": [ + { + "globs": ["migrations/**", "db/schema.sql"], + "note": "Schema changes ship to production — review carefully." + } + ] +} +``` + +### Example: protect specific symbols + +Add `symbols` to narrow a rule to changes that touch particular functions, constants, or types. Only changes mentioning one of those names within the matched files will trip. + +```json +{ + "rules": [ + { + "globs": ["packages/billing/**"], + "symbols": ["chargeCard", "PRICE_TABLE", "refund"], + "note": "Money-touching code — get sign-off before changing." + } + ] +} +``` + +## Glob syntax + +Globs are matched against repo-relative paths: + +- `*` matches anything within a single path segment (does not cross `/`) +- `**` matches across path segments, including none +- `?` matches a single character +- A leading `**/` also matches files at the repo root, so `**/config.ts` matches both `config.ts` and `src/config.ts` + +Examples: + +- `src/auth/**` — every file under `src/auth/` +- `**/*.sql` — every `.sql` file anywhere in the repo +- `packages/*/index.ts` — `index.ts` directly inside any package folder + +## What trips a wire + +The rule is simple: **any change that touches a tripwired file or symbol trips the wire.** That includes: + +- **Adding** new lines in a protected file +- **Editing** existing lines +- **Deleting** lines +- **Renaming** a protected file (matched against either the old or new path) + +If a rule has `symbols`, only changes that mention one of those names count. If it has no `symbols`, any change in the matched files trips it. + +## How hits surface + +Tripwires are re-evaluated every time the diff loads — on initial review, on diff-type switches, and on PR switches. When a change trips a wire, you'll see it in three places: + +- **Gutter marker** — an amber warning marker on the affected line in the diff, with the rule's note. Click it to jump to the matching entry in the sidebar. +- **Sidebar list** — each hit appears as an amber-accented card with its note and the file it touched. Click a card to jump to the line, or to the file panel for file-scope hits (renames that have no single line to anchor to). +- **File tree** — protected files that were touched show an amber warning glyph next to their name, so you can spot them before opening the diff. + +Because tripwires are re-evaluated on every diff and PR switch, a hit you've already seen will reappear after you switch views. This is intended — the warning reflects the current diff, not a dismissible to-do. + +## Tripwires are informational only + +Tripwires never block anything and they are **never included in the feedback sent back to the agent.** They are a signal for the human reviewer, not an instruction for the agent. When you click **Send Feedback**, only your own annotations are submitted — tripwire warnings are filtered out entirely, and they don't count toward the annotation total that gates submission. + +## Non-interactive scan + +You don't have to open the review UI to see what would trip. Run: + +```bash +plannotator review --tripwires +``` + +(or `-t`) to print a report instead of opening the browser. The report lists the rules from each layer and a live status section showing which wires the current diff trips. It's handy for a quick check, for scripting, or for letting an agent see the slop-free zones it's about to touch. + +## Fail-open behavior + +If either tripwires file is missing, empty, or malformed, tripwires simply do nothing for that layer — review proceeds exactly as it would without it. The two layers fail independently: a broken repo file never wipes out your global rules, and a broken global file never wipes out the repo's. A single broken rule never discards its siblings; the valid rules still apply. You can't break code review by writing a bad config. + +## PR mode + +Tripwires evaluate during PR review as well. Your **global** rules still fire in PR mode — they're keyed to the repository you launched the review from, so the same slop-free zones you've set up locally apply even without a checkout of the PR. The **repo** layer needs a local checkout of the repo, since its config file lives in the working tree; without one, only the global rules fire. diff --git a/apps/opencode-plugin/cli-bridge.test.ts b/apps/opencode-plugin/cli-bridge.test.ts index 0f6d7d1c9..a54580548 100644 --- a/apps/opencode-plugin/cli-bridge.test.ts +++ b/apps/opencode-plugin/cli-bridge.test.ts @@ -158,4 +158,29 @@ describe("OpenCode CLI bridge helpers", () => { }); expect(prFeedback.message).toBe("PR comment only."); }); + + test("returns a tripwire scan report verbatim (no review-denied suffix)", () => { + // The opencode-review tripwire short-circuit emits + // { decision: "annotated", feedback: , isPRMode: true }. This locks + // the rebuttal contract it relies on: isPRMode:true returns the report + // unchanged, with NO getReviewDeniedSuffix appended. + const report = [ + "## Global (~/.plannotator/tripwires/abc123.json)", + "(none)", + "## Repo (.plannotator/tripwires.json)", + "| id | globs | symbols | note |\n| --- | --- | --- | --- |\n| auth | src/auth/** | | Touches a slop-free zone |", + "## Live status", + "No tripwires tripped.", + ].join("\n\n"); + + const outcome = buildReviewPromptFromBridgeOutcome({ + decision: "annotated", + approved: false, + isPRMode: true, + feedback: report, + }); + + expect(outcome).toEqual({ message: report }); + expect(outcome.agent).toBeUndefined(); + }); }); diff --git a/apps/opencode-plugin/commands.ts b/apps/opencode-plugin/commands.ts index 6f56fb0bc..eee22ae01 100644 --- a/apps/opencode-plugin/commands.ts +++ b/apps/opencode-plugin/commands.ts @@ -31,6 +31,18 @@ import { FILE_BROWSER_EXCLUDED } from "@plannotator/shared/reference-common"; import { htmlToMarkdown } from "@plannotator/shared/html-to-markdown"; import { parseAnnotateArgs } from "@plannotator/shared/annotate-args"; import { parseReviewArgs } from "@plannotator/shared/review-args"; +import { + evaluateTripwires, + formatTripwiresMarkdown, + buildAddTripwirePrompt, + parseTripwiresConfig, +} from "@plannotator/shared/tripwires"; +import { + resolveMergedTripwires, + globalTripwiresPath, + readGlobalTripwiresFile, + readTripwiresFile, +} from "@plannotator/server/tripwires"; import { urlToMarkdown, isConvertedSource } from "@plannotator/shared/url-to-markdown"; import { statSync } from "fs"; import path from "path"; @@ -107,6 +119,59 @@ export async function handleReviewCommand( diffError = diffResult.error; } + // Non-interactive tripwire flags short-circuit the review UI: scan the diff + // we just captured (rawPatch is assigned in both the PR and local branches + // above) and inject the result into the session instead of opening a browser. + if (reviewArgs.tripwires || reviewArgs.addTripwire) { + // @ts-ignore - Event properties contain sessionID + const sessionId = event.properties?.sessionID; + + const sendToSession = async (text: string) => { + if (!sessionId) return; + try { + await client.session.prompt({ + path: { id: sessionId }, + body: { parts: [{ type: "text", text }] }, + }); + } catch { + // Session may not be available + } + }; + + if (reviewArgs.addTripwire) { + const { root, key } = await resolveMergedTripwires(directory ?? process.cwd()); + await sendToSession( + buildAddTripwirePrompt({ + description: reviewArgs.addTripwire, + globalPath: key ? globalTripwiresPath(key) : undefined, + repoPath: root ? `${root}/.plannotator/tripwires.json` : undefined, + }), + ); + return; + } + + const { config, root, key } = await resolveMergedTripwires( + directory ?? process.cwd(), + ); + const hits = evaluateTripwires(rawPatch, config, { cwd: root ?? undefined }); + const globalRules = parseTripwiresConfig( + key ? readGlobalTripwiresFile(key) : null, + ).rules; + const repoRules = parseTripwiresConfig( + root ? readTripwiresFile(root) : null, + ).rules; + const report = formatTripwiresMarkdown({ + globalKey: key, + globalPath: key ? globalTripwiresPath(key) : "~/.plannotator/tripwires/.json", + repoPath: root ? `${root}/.plannotator/tripwires.json` : null, + globalRules, + repoRules, + hits, + }); + await sendToSession(report); + return; + } + const server = await startReviewServer({ rawPatch, gitRef, diff --git a/apps/opencode-plugin/commands/plannotator-review.md b/apps/opencode-plugin/commands/plannotator-review.md index bd150d01d..feafce935 100644 --- a/apps/opencode-plugin/commands/plannotator-review.md +++ b/apps/opencode-plugin/commands/plannotator-review.md @@ -1,3 +1,5 @@ --- -description: Open interactive code review for current changes or a PR URL; pass --git to force Git in JJ workspaces +description: Open interactive code review for current changes or a PR URL; pass --git to force Git in JJ workspaces, --tripwires to print a non-interactive slop-free-zone scan instead of opening the UI, or --add-tripwire to get instructions for adding a rule --- + +Pass `--tripwires` (or `-t`) to skip the review UI and instead get a tripwires scan report for the current diff (which slop-free zones the changes touch). Tripwires are configured in a private, auto-created global file at `~/.plannotator/tripwires/.json` plus an optional committed `.plannotator/tripwires.json` in the repo; the two are merged. Pass `--add-tripwire ` to get agent instructions for turning that description into a concrete rule and writing it (the command itself never writes files). diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index aa86c1b3b..c553f21e8 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -58,6 +58,8 @@ import { hasReviewBrowserHtml, getStartupErrorMessage, openArchiveBrowserAction, + scanTripwires, + buildAddTripwireInstruction, startCodeReviewBrowserSession, startLastMessageAnnotationSession, startMarkdownAnnotationSession, @@ -424,6 +426,36 @@ export default function plannotator(pi: ExtensionAPI): void { try { const reviewArgs = parseReviewArgs(args ?? ""); + + // Non-interactive tripwire flags short-circuit before any review UI: + // compute/scan and deliver the report (or add-tripwire snippet) back to + // the session, then return without opening the browser. + if (reviewArgs.tripwires) { + const report = await scanTripwires(ctx, { + prUrl: reviewArgs.prUrl, + vcsType: reviewArgs.vcsType, + useLocal: reviewArgs.useLocal, + }); + sendUserMessageWithCurrentSessionFallback( + pi, + report, + { deliverAs: "followUp" }, + "Tripwire scan could not be sent", + origin, + ); + return; + } + if (reviewArgs.addTripwire) { + sendUserMessageWithCurrentSessionFallback( + pi, + buildAddTripwireInstruction(ctx, reviewArgs.addTripwire), + { deliverAs: "followUp" }, + "Tripwire scan could not be sent", + origin, + ); + return; + } + const isPRReview = reviewArgs.prUrl !== undefined; const session = await startCodeReviewBrowserSession(ctx, { prUrl: reviewArgs.prUrl, diff --git a/apps/pi-extension/plannotator-browser.ts b/apps/pi-extension/plannotator-browser.ts index f0c898ffe..3a4ef10ba 100644 --- a/apps/pi-extension/plannotator-browser.ts +++ b/apps/pi-extension/plannotator-browser.ts @@ -16,6 +16,13 @@ import { } from "./server.js"; import { openBrowser, isRemoteSession } from "./server/network.js"; import { parsePRUrl, checkPRAuth, fetchPR } from "./server/pr.js"; +import { piResolveMergedTripwires, piGlobalTripwiresPath } from "./server/serverReview.js"; +import { + evaluateTripwires, + formatTripwiresMarkdown, + parseTripwiresConfig, + buildAddTripwirePrompt, +} from "./generated/tripwires.js"; import { getMRLabel, getMRNumberLabel, @@ -439,6 +446,103 @@ export async function startCodeReviewBrowserSession( return startBrowserDecisionSession(server, ctx, server.waitForDecision); } +/** + * Non-interactive tripwire scan: compute the review diff (PR or local) along the + * LIGHTWEIGHT path — no worktree, no exit handlers, no review server — evaluate + * the merged (global + repo) tripwires config against it, and return a markdown + * report. Mirrors the Bun CLI `--tripwires` short-circuit. + */ +export async function scanTripwires( + ctx: ExtensionContext, + options: { prUrl?: string; vcsType?: VcsSelection; useLocal?: boolean; cwd?: string } = {}, +): Promise { + const urlArg = options.prUrl; + const isPRMode = urlArg?.startsWith("http://") || urlArg?.startsWith("https://"); + + let rawPatch: string; + + if (isPRMode && urlArg) { + // --- PR scan: fetch the patch only (no checkout) --- + const prRef = parsePRUrl(urlArg); + if (!prRef) { + throw new Error( + `Invalid PR/MR URL: ${urlArg}\n` + + "Supported formats:\n" + + " GitHub: https://github.com/owner/repo/pull/123\n" + + " GitLab: https://gitlab.com/group/project/-/merge_requests/42", + ); + } + + const cliName = getCliName(prRef); + const cliUrl = getCliInstallUrl(prRef); + try { + await checkPRAuth(prRef); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + if (msg.includes("not found") || msg.includes("ENOENT")) { + throw new Error(`${cliName === "gh" ? "GitHub" : "GitLab"} CLI (${cliName}) is not installed. Install it from ${cliUrl}`); + } + throw err; + } + + const pr = await fetchPR(prRef); + rawPatch = pr.rawPatch; + } else { + // --- Local scan: compute the working-tree diff only --- + const cwd = options.cwd ?? ctx.cwd; + const config = loadConfig(); + const result = await prepareLocalReviewDiff({ + cwd, + vcsType: options.vcsType, + configuredDiffType: resolveDefaultDiffType(config), + hideWhitespace: config.diffOptions?.hideWhitespace ?? false, + }); + rawPatch = result.rawPatch; + } + + const scanCwd = options.cwd ?? ctx.cwd; + const { config, root, key } = piResolveMergedTripwires(scanCwd); + const hits = evaluateTripwires(rawPatch, config, { cwd: root ?? undefined }); + + // Re-read each layer separately for the grouped display (the resolver only + // returns the merged config). Key + root are already resolved, so no extra + // git calls — just the file reads. + const globalPath = key ? piGlobalTripwiresPath(key) : ""; + const globalRules = key && globalPath && existsSync(globalPath) + ? parseTripwiresConfig(readFileSync(globalPath, "utf-8")).rules + : []; + const repoPath = root ? join(root, ".plannotator", "tripwires.json") : null; + const repoRules = repoPath && existsSync(repoPath) + ? parseTripwiresConfig(readFileSync(repoPath, "utf-8")).rules + : []; + + return formatTripwiresMarkdown({ + globalKey: key, + globalPath, + repoPath, + globalRules, + repoRules, + hits, + }); +} + +/** + * Build the agent-facing add-tripwire instruction with the project's resolved + * config paths filled in. Mirrors the Bun CLI's `--add-tripwire` short-circuit. + */ +export function buildAddTripwireInstruction( + ctx: ExtensionContext, + description: string, + cwd?: string, +): string { + const { root, key } = piResolveMergedTripwires(cwd ?? ctx.cwd); + return buildAddTripwirePrompt({ + description, + globalPath: key ? piGlobalTripwiresPath(key) : undefined, + repoPath: root ? join(root, ".plannotator", "tripwires.json") : undefined, + }); +} + export async function openMarkdownAnnotation( ctx: ExtensionContext, filePath: string, diff --git a/apps/pi-extension/plannotator-events.ts b/apps/pi-extension/plannotator-events.ts index 0b8edf22e..bbaf71b34 100644 --- a/apps/pi-extension/plannotator-events.ts +++ b/apps/pi-extension/plannotator-events.ts @@ -329,6 +329,8 @@ export { getLastAssistantMessageText, hasPlanBrowserHtml, hasReviewBrowserHtml, + scanTripwires, + buildAddTripwireInstruction, startCodeReviewBrowserSession, startLastMessageAnnotationSession, startMarkdownAnnotationSession, diff --git a/apps/pi-extension/server/external-annotations.ts b/apps/pi-extension/server/external-annotations.ts index 4bb48aa3f..7fac7bd42 100644 --- a/apps/pi-extension/server/external-annotations.ts +++ b/apps/pi-extension/server/external-annotations.ts @@ -57,6 +57,11 @@ export function createExternalAnnotationHandler(mode: "plan" | "review") { return { ids: created.map((a: { id: string }) => a.id) }; }, + /** Remove every annotation from a given source. Returns the count removed. */ + clearBySource(source: string): number { + return store.clearBySource(source); + }, + async handle( req: IncomingMessage, res: ServerResponse, diff --git a/apps/pi-extension/server/serverReview.ts b/apps/pi-extension/server/serverReview.ts index f3436f016..ba29353c9 100644 --- a/apps/pi-extension/server/serverReview.ts +++ b/apps/pi-extension/server/serverReview.ts @@ -1,6 +1,8 @@ -import { spawn } from "node:child_process"; -import { readFileSync, existsSync } from "node:fs"; +import { spawn, execFileSync } from "node:child_process"; +import { readFileSync, existsSync, mkdirSync, writeFileSync } from "node:fs"; +import { createHash } from "node:crypto"; import { createServer } from "node:http"; +import { dirname, join, resolve } from "node:path"; import os from "node:os"; import { contentHash, deleteDraft } from "../generated/draft.js"; @@ -90,6 +92,18 @@ import { validateCodeNavRequest, extractChangedFiles, } from "../generated/code-nav.js"; +import { + parseTripwiresConfig, + parseTripwiresConfigDetailed, + evaluateTripwires, + tripwireHitToReviewAnnotation, + normalizeRemoteIdentity, + mergeTripwiresConfigs, + TRIPWIRE_SOURCE, + type TripwiresConfig, + type TripwireDiagnostic, +} from "../generated/tripwires.js"; +import { getPlannotatorDataDir } from "../generated/data-dir.js"; import { canStageFiles, detectRemoteDefaultCompareTarget, @@ -146,6 +160,152 @@ function detectWSL(): boolean { return false; } +/** Resolve the git repo root for a cwd, or null when not in a git repo. */ +function piRepoRoot(cwd: string): string | null { + try { + return execFileSync("git", ["rev-parse", "--show-toplevel"], { + cwd, + encoding: "utf-8", + }).trim(); + } catch { + return null; + } +} + +/** Read `/.plannotator/tripwires.json`, or null on miss/throw. */ +function piReadTripwires(root: string): string | null { + try { + const path = `${root}/.plannotator/tripwires.json`; + if (!existsSync(path)) return null; + return readFileSync(path, "utf-8"); + } catch { + return null; + } +} + +// --------------------------------------------------------------------------- +// Tripwires — global store glue (node:crypto + sync fs + execFileSync). +// +// Mirrors the Bun glue in packages/server/tripwires.ts. The pure normalization +// + merge live in the shared module; everything impure (git remote/common-dir +// resolution, hashing, file I/O) lives here so it stays runtime-specific. +// --------------------------------------------------------------------------- + +/** The `origin` remote URL for a cwd, or null when there is no remote/git. */ +function piRemoteUrl(cwd: string): string | null { + try { + const url = execFileSync("git", ["remote", "get-url", "origin"], { + cwd, + encoding: "utf-8", + }).trim(); + return url || null; + } catch { + return null; + } +} + +/** + * Canonical per-repo base dir for the remote-less key: the parent of the + * git-common-dir. Unlike `--show-toplevel` (per-worktree), the common dir is + * shared across linked worktrees, so they collapse to one key. Null when not a + * git repo. + */ +function piRepoKeyBase(cwd: string): string | null { + try { + const commonDir = execFileSync( + "git", + ["rev-parse", "--path-format=absolute", "--git-common-dir"], + { cwd, encoding: "utf-8" }, + ).trim(); + if (!commonDir) return null; + return dirname(commonDir); + } catch { + // `--path-format=absolute` was added in git 2.31 (2021); on older git the + // command above throws. Fall back to the plain (possibly relative) + // common-dir resolved against cwd so distinct remote-less repos never + // collapse onto one shared global key (worktree-sharing is best-effort). + try { + const commonDir = execFileSync( + "git", + ["rev-parse", "--git-common-dir"], + { cwd, encoding: "utf-8" }, + ).trim(); + if (!commonDir) return null; + return dirname(resolve(cwd, commonDir)); + } catch { + return null; + } + } +} + +/** Hash a normalized identity into the 16-char project key used in the path. */ +function piProjectKey(identity: string): string { + return createHash("sha256").update(identity).digest("hex").slice(0, 16); +} + +/** Absolute path to the global tripwires file for a project key. */ +export function piGlobalTripwiresPath(key: string): string { + return join(getPlannotatorDataDir(), "tripwires", `${key}.json`); +} + +/** Read the global tripwires file for a key, or null on miss/throw (fail-open). */ +function piReadGlobalTripwires(key: string): string | null { + try { + const path = piGlobalTripwiresPath(key); + if (!existsSync(path)) return null; + return readFileSync(path, "utf-8"); + } catch { + return null; + } +} + +/** + * Write-once create the global tripwires file ({rules:[]}) if it is missing. + * Race-tolerant: if another process created it between the existence check and + * the write, that's a no-op; any other write failure is logged but never fatal. + */ +function piEnsureGlobalTripwires(key: string): void { + try { + const path = piGlobalTripwiresPath(key); + if (existsSync(path)) return; + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, `${JSON.stringify({ rules: [] }, null, 2)}\n`, { flag: "wx" }); + } catch (err) { + // `wx` throws EEXIST if the file appeared concurrently — that's fine. + if (!(err instanceof Error && "code" in err && (err as { code?: string }).code === "EEXIST")) { + console.error("[tripwires] failed to create global file:", err); + } + } +} + +/** + * Resolve the merged (global + repo) tripwires config for a cwd. Mirrors the + * Bun `resolveMergedTripwires`: derives the project key from the remote + * identity (or git-common-dir for remote-less repos), ensures the global file + * exists, then merges global rules first with the repo layer appended. + */ +export function piResolveMergedTripwires(cwd: string): { + config: TripwiresConfig; + root: string | null; + key: string | null; + globalDiagnostics: TripwireDiagnostic[]; +} { + const root = piRepoRoot(cwd); + const remote = piRemoteUrl(cwd); + const keyBase = piRepoKeyBase(cwd); + const identity = normalizeRemoteIdentity(remote, keyBase); + const key = piProjectKey(identity); + piEnsureGlobalTripwires(key); + const globalParsed = parseTripwiresConfigDetailed(piReadGlobalTripwires(key)); + const repo = root ? parseTripwiresConfig(piReadTripwires(root)) : { rules: [] }; + return { + config: mergeTripwiresConfigs(globalParsed.config, repo), + root, + key, + globalDiagnostics: globalParsed.diagnostics, + }; +} + export interface ReviewServerResult { port: number; portSource: "env" | "remote-default" | "random"; @@ -284,6 +444,50 @@ export async function startReviewServer(options: { if (options.agentCwd) return options.agentCwd; return resolveVcsCwd(currentDiffType, options.gitContext?.cwd) ?? process.cwd(); } + + // Resolve the global project key ONCE at server start (git remote / + // common-dir resolution + global-file creation). Cached for the session; + // subsequent refreshes re-read only the file CONTENTS and re-merge. A + // corrupt global file is logged once here, distinguishing it from an empty + // one. Fail-open: any failure leaves the key null and refreshes use repo-only. + let tripwiresGlobalKey: string | null = null; + try { + const resolved = piResolveMergedTripwires(resolveAgentCwd()); + tripwiresGlobalKey = resolved.key; + for (const diag of resolved.globalDiagnostics) { + if (diag.level === "error") { + console.error(`[tripwires] global config: ${diag.message}`); + } + } + } catch (err) { + console.error(`[tripwires] global key resolution failed:`, err); + } + + // Re-evaluate tripwires against the current patch and republish them as + // external annotations. Fail-open: any error leaves the review usable. Called + // at startup and after every patch reassignment (diff/PR switches). Global + // rules fire even when `root` is null (PR mode without a local checkout). + function refreshTripwires(): void { + try { + externalAnnotations.clearBySource(TRIPWIRE_SOURCE); + const root = piRepoRoot(resolveAgentCwd()); + const globalConfig = tripwiresGlobalKey + ? parseTripwiresConfig(piReadGlobalTripwires(tripwiresGlobalKey)) + : { rules: [] }; + const repoConfig = root ? parseTripwiresConfig(piReadTripwires(root)) : { rules: [] }; + const config = mergeTripwiresConfigs(globalConfig, repoConfig); + const hits = evaluateTripwires(currentPatch, config, { cwd: root ?? undefined }); + if (hits.length === 0) return; + const result = externalAnnotations.addAnnotations({ + annotations: hits.map(tripwireHitToReviewAnnotation), + }); + if ("error" in result) console.error(`[tripwires] addAnnotations error:`, result.error); + } catch (err) { + console.error(`[tripwires] refresh failed:`, err); + } + } + refreshTripwires(); + const tour = createTourSession(); const agentJobs = createAgentJobHandler({ @@ -530,6 +734,7 @@ export async function startReviewServer(options: { currentBase = base; baseEverSwitched = true; currentError = result.error; + refreshTripwires(); // Recompute gitContext for the effective cwd so the client's // sidebar reflects the worktree we're now reviewing. @@ -578,6 +783,7 @@ export async function startReviewServer(options: { currentGitRef = originalPRGitRef; currentError = originalPRError; currentPRDiffScope = "layer"; + refreshTripwires(); json(res, { rawPatch: currentPatch, gitRef: currentGitRef, @@ -605,6 +811,7 @@ export async function startReviewServer(options: { currentGitRef = result.label; currentError = undefined; currentPRDiffScope = "full-stack"; + refreshTripwires(); json(res, { rawPatch: currentPatch, gitRef: currentGitRef, @@ -679,6 +886,9 @@ export async function startReviewServer(options: { branch: `${getMRLabel(pr.metadata)} ${getMRNumberLabel(pr.metadata)}`, }; + // Re-evaluate against the switched-to PR's patch and local checkout. + refreshTripwires(); + return json(res, { rawPatch: currentPatch, gitRef: currentGitRef, diff --git a/apps/pi-extension/vendor.sh b/apps/pi-extension/vendor.sh index edab67bfb..0263e13f6 100755 --- a/apps/pi-extension/vendor.sh +++ b/apps/pi-extension/vendor.sh @@ -7,7 +7,7 @@ cd "$(dirname "$0")" rm -rf generated mkdir -p generated generated/ai/providers -for f in feedback-templates prompts review-core jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference pfm-reminder improvement-hooks code-nav data-dir; do +for f in feedback-templates prompts review-core jj-core vcs-core review-args storage draft project pr-types pr-provider pr-stack pr-github pr-gitlab checklist integrations-common repo reference-common favicon code-file resolve-file config external-annotation agent-jobs worktree worktree-pool html-to-markdown url-to-markdown tour annotate-args at-reference pfm-reminder improvement-hooks code-nav data-dir tripwires; do src="../../packages/shared/$f.ts" printf '// @generated — DO NOT EDIT. Source: packages/shared/%s.ts\n' "$f" | cat - "$src" > "generated/$f.ts" done diff --git a/apps/skills/core/plannotator-review/SKILL.md b/apps/skills/core/plannotator-review/SKILL.md index a807eae41..048ee9c67 100644 --- a/apps/skills/core/plannotator-review/SKILL.md +++ b/apps/skills/core/plannotator-review/SKILL.md @@ -22,3 +22,38 @@ Behavior: 4. If it returns an approval/LGTM-style message, acknowledge that review passed and continue. Do not ask the user to copy shell commands into chat. Run the command yourself. + +## Tripwires (slop-free zones) + +Tripwires flag diffs that touch protected regions of the codebase. Two extra +flags drive them non-interactively (no review UI opens). Forward the user's +flags and wording verbatim: + +```bash +plannotator review --tripwires # or -t +plannotator review --add-tripwire +``` + +- `--tripwires` / `-t` prints a scan report for the current diff: which + configured tripwires (slop-free zones) the changes trip, plus the global and + repo rule tables. Use it to check changes against protected regions without + opening the browser. Read the report and address any tripped zones. +- `--add-tripwire` takes the user's natural-language description of what to + protect (e.g. `--add-tripwire the billing module and chargeCustomer`). It + prints instructions for YOU: explore the repo, turn the description into + concrete globs/symbols, write the rule into the indicated config file, then + run `plannotator tripwires validate` and show the user the final rule. It + does NOT write anything itself. + +Related CLI (also available to the user directly): `plannotator tripwires +list|add|validate|path`. + +Tripwire rules live in two layers, merged additively: + +- A private, auto-created global file at + `~/.plannotator/tripwires/.json` (a directory of per-project + files — never the flat `~/.plannotator/tripwires.json`). It is never committed + and follows you across checkouts of the same repo. The base directory honors + the `` override. +- An optional committed `.plannotator/tripwires.json` at the repo root (team + opt-in), appended on top of the global rules. diff --git a/packages/review-editor/App.tsx b/packages/review-editor/App.tsx index 1228a0e21..2b6db1638 100644 --- a/packages/review-editor/App.tsx +++ b/packages/review-editor/App.tsx @@ -408,6 +408,13 @@ const ReviewApp: React.FC = () => { const allAnnotationsRef = useRef(allAnnotations); allAnnotationsRef.current = allAnnotations; + // Tripwires are informational only and must NEVER be sent to the agent. + // Display surfaces use `allAnnotations`; every submit/export path uses this. + const reviewerAnnotations = useMemo( + () => allAnnotations.filter(a => a.source !== 'tripwire'), + [allAnnotations], + ); + // Auto-save code annotation drafts const { draftBanner, restoreDraft, dismissDraft } = useCodeAnnotationDraft({ annotations: allAnnotations, @@ -1502,12 +1509,12 @@ const ReviewApp: React.FC = () => { // Copy feedback markdown to clipboard const handleCopyFeedback = useCallback(async () => { - if (allAnnotations.length === 0) { + if (reviewerAnnotations.length === 0) { setShowNoAnnotationsDialog(true); return; } try { - const feedback = exportReviewFeedback(allAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel); + const feedback = exportReviewFeedback(reviewerAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel); await navigator.clipboard.writeText(feedback); setCopyFeedback('Feedback copied!'); setTimeout(() => setCopyFeedback(null), 2000); @@ -1516,17 +1523,17 @@ const ReviewApp: React.FC = () => { setCopyFeedback('Failed to copy'); setTimeout(() => setCopyFeedback(null), 2000); } - }, [allAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel]); + }, [reviewerAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel]); const feedbackMarkdown = useMemo(() => { - let output = exportReviewFeedback(allAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel); + let output = exportReviewFeedback(reviewerAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel); if (editorAnnotations.length > 0) { output += exportEditorAnnotations(editorAnnotations); } return output; - }, [allAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel, editorAnnotations]); + }, [reviewerAnnotations, prMetadata, feedbackDiffContext, prReviewScopeLabel, editorAnnotations]); - const totalAnnotationCount = allAnnotations.length + editorAnnotations.length; + const totalAnnotationCount = reviewerAnnotations.length + editorAnnotations.length; // Send feedback to OpenCode via API const handleSendFeedback = useCallback(async () => { @@ -1545,7 +1552,7 @@ const ReviewApp: React.FC = () => { body: JSON.stringify({ approved: false, feedback: feedbackMarkdown, - annotations: allAnnotations, + annotations: reviewerAnnotations, ...(effectiveAgent && { agentSwitch: effectiveAgent }), }), }); @@ -1560,7 +1567,7 @@ const ReviewApp: React.FC = () => { setTimeout(() => setCopyFeedback(null), 2000); setIsSendingFeedback(false); } - }, [totalAnnotationCount, feedbackMarkdown, allAnnotations]); + }, [totalAnnotationCount, feedbackMarkdown, reviewerAnnotations]); // Exit review session without sending any feedback const handleExit = useCallback(async () => { @@ -1708,10 +1715,10 @@ const ReviewApp: React.FC = () => { title: prMetadata.title, repo: getDisplayRepo(prMetadata), } : undefined; - const plan = buildReviewSubmission(allAnnotations, editorAnnotations, prMetadata?.url, diffPaths, prMeta); + const plan = buildReviewSubmission(reviewerAnnotations, editorAnnotations, prMetadata?.url, diffPaths, prMeta); setPlatformGeneralComment(''); setPlatformCommentDialog({ action, plan }); - }, [allAnnotations, editorAnnotations, files, prMetadata]); + }, [reviewerAnnotations, editorAnnotations, files, prMetadata]); // Double-tap Option/Alt to toggle review destination (PR mode only) useEffect(() => { @@ -2353,7 +2360,7 @@ const ReviewApp: React.FC = () => {
- {allAnnotations.length} annotation{allAnnotations.length !== 1 ? 's' : ''} + {reviewerAnnotations.length} annotation{reviewerAnnotations.length !== 1 ? 's' : ''}
                   {feedbackMarkdown}
diff --git a/packages/review-editor/components/AllFilesDiffView.tsx b/packages/review-editor/components/AllFilesDiffView.tsx
index 5f6ea917f..59abefff3 100644
--- a/packages/review-editor/components/AllFilesDiffView.tsx
+++ b/packages/review-editor/components/AllFilesDiffView.tsx
@@ -8,6 +8,8 @@ import { usePierreTheme } from '../hooks/usePierreTheme';
 import { LazyFileDiff } from './LazyFileDiff';
 import { ToolbarHost, type ToolbarHostHandle } from './ToolbarHost';
 import { InlineAnnotation } from './InlineAnnotation';
+import { InlineTripwireMarker } from './InlineTripwireMarker';
+import { isTripwireAnnotation } from '../utils/tripwire';
 import { FileHeader } from './FileHeader';
 import { CommentPopover } from '@plannotator/ui/components/CommentPopover';
 import { detectLanguage } from '../utils/detectLanguage';
@@ -405,9 +407,15 @@ export const AllFilesDiffView: React.FC = ({
               reasoning: ann.reasoning,
               conventionalLabel: ann.conventionalLabel,
               decorations: ann.decorations,
+              source: ann.source,
+              kind: isTripwireAnnotation(ann) ? 'tripwire' as const : undefined,
             } as DiffAnnotationMetadata,
           }));
 
+        const isTripwired = annotations.some(
+          a => a.filePath === file.path && isTripwireAnnotation(a),
+        );
+
         return (
           
= ({ onStage={onStage ? () => onStage(file.path) : undefined} canStage={canStageFiles} stageError={stagingFile === file.path ? stageError : null} + isTripwired={isTripwired} collapseToggle={
+ {isTripwired && ( + + + + )} {onToggleViewed && ( + ); +}; diff --git a/packages/review-editor/components/ReviewSidebar.tsx b/packages/review-editor/components/ReviewSidebar.tsx index c47a4f9fe..2b5f79bcb 100644 --- a/packages/review-editor/components/ReviewSidebar.tsx +++ b/packages/review-editor/components/ReviewSidebar.tsx @@ -7,6 +7,7 @@ import { ConventionalLabelBadge } from './ConventionalLabelPicker'; import { HighlightedCode } from './HighlightedCode'; import { detectLanguage } from '../utils/detectLanguage'; import { renderInlineMarkdown } from '../utils/renderInlineMarkdown'; +import { isTripwireAnnotation } from '../utils/tripwire'; import { formatRelativeTime } from '../utils/formatRelativeTime'; import { AITab } from './AITab'; import { AgentsTab } from '@plannotator/ui/components/AgentsTab'; @@ -143,7 +144,10 @@ export const ReviewSidebar: React.FC = /* React.memo */({ onOpenJobDetail, onOpenPRPanel, }) => { - const totalCount = annotations.length + (editorAnnotations?.length ?? 0); + // Tripwires are informational and never sent to the agent, so they don't + // count toward the reviewer's annotation total (header badge, empty state). + const reviewerAnnotationCount = annotations.filter(a => !isTripwireAnnotation(a)).length; + const totalCount = reviewerAnnotationCount + (editorAnnotations?.length ?? 0); const [copied, setCopied] = useState(false); const handleQuickCopy = async () => { @@ -198,6 +202,7 @@ export const ReviewSidebar: React.FC = /* React.memo */({ function renderAnnotationCard(annotation: CodeAnnotation) { const isSelected = selectedAnnotationId === annotation.id; const isFileScope = getAnnotationScope(annotation) === 'file'; + const isTripwire = isTripwireAnnotation(annotation); return (
= /* React.memo */({ }`} >
-
- {isFileScope ? ( +
+ {isTripwire ? ( + + tripwire + + ) : isFileScope ? ( file @@ -224,6 +236,15 @@ export const ReviewSidebar: React.FC = /* React.memo */({ )} )} + {isTripwire && ( + + {isFileScope + ? annotation.filePath.split('/').pop() + : annotation.lineStart === annotation.lineEnd + ? `L${annotation.lineStart}` + : `L${annotation.lineStart}-${annotation.lineEnd}`} + + )} {annotation.conventionalLabel && ( )} @@ -308,7 +329,7 @@ export const ReviewSidebar: React.FC = /* React.memo */({ {/* Annotations tab */} {activeTab === 'annotations' && (
- {totalCount === 0 ? ( + {totalCount === 0 && annotations.length === 0 ? (
diff --git a/packages/review-editor/index.css b/packages/review-editor/index.css index d11bee39a..d83a6ec45 100644 --- a/packages/review-editor/index.css +++ b/packages/review-editor/index.css @@ -849,6 +849,22 @@ diffs-container { color: var(--primary); } +/* Tripwire inline markers in diff — quiet, editor-native: a single amber + glyph carries the signal; no border, no chrome. Read-only. */ +.tripwire-marker { + color: var(--muted-foreground); + transition: background 150ms ease, color 150ms ease; +} + +.tripwire-marker svg { + color: var(--warning); +} + +.tripwire-marker:hover { + background: oklch(from var(--warning) l c h / 0.06); + color: var(--foreground); +} + /* AI markdown — rendered responses from marked */ .ai-markdown { line-height: 1.5; diff --git a/packages/review-editor/package.json b/packages/review-editor/package.json index e249d2b39..2deace739 100644 --- a/packages/review-editor/package.json +++ b/packages/review-editor/package.json @@ -19,6 +19,7 @@ "@radix-ui/react-popover": "^1.1.15", "@radix-ui/react-tooltip": "^1.2.8", "highlight.js": "^11.11.1", + "lucide-react": "^1.14.0", "motion": "^12.38.0", "react": "^19.2.3", "react-dom": "^19.2.3", diff --git a/packages/review-editor/utils/tripwire.ts b/packages/review-editor/utils/tripwire.ts new file mode 100644 index 000000000..bcbcfe6b5 --- /dev/null +++ b/packages/review-editor/utils/tripwire.ts @@ -0,0 +1,3 @@ +/** Source identifier stamped onto tripwire annotations by the server. */ +export const isTripwireAnnotation = (a: { source?: string }): boolean => + a.source === 'tripwire'; diff --git a/packages/server/external-annotations.ts b/packages/server/external-annotations.ts index 0f49be66f..49814bccb 100644 --- a/packages/server/external-annotations.ts +++ b/packages/server/external-annotations.ts @@ -35,6 +35,8 @@ export interface ExternalAnnotationHandler { ) => Promise; /** Push annotations directly into the store (bypasses HTTP, reuses same validation). */ addAnnotations: (body: unknown) => { ids: string[] } | { error: string }; + /** Remove all annotations with the given source. Returns the removed count. */ + clearBySource: (source: string) => number; } // --------------------------------------------------------------------------- @@ -77,6 +79,10 @@ export function createExternalAnnotationHandler( return { ids: created.map((a) => a.id) }; }, + clearBySource(source: string): number { + return store.clearBySource(source); + }, + async handle( req: Request, url: URL, diff --git a/packages/server/package.json b/packages/server/package.json index a1c79384b..eafa286a0 100644 --- a/packages/server/package.json +++ b/packages/server/package.json @@ -17,6 +17,7 @@ "./p4": "./p4.ts", "./vcs": "./vcs.ts", "./repo": "./repo.ts", + "./tripwires": "./tripwires.ts", "./share-url": "./share-url.ts", "./sessions": "./sessions.ts", "./project": "./project.ts", diff --git a/packages/server/review.ts b/packages/server/review.ts index a084b98a3..916633db9 100644 --- a/packages/server/review.ts +++ b/packages/server/review.ts @@ -29,6 +29,12 @@ import { contentHash, deleteDraft } from "./draft"; import { createEditorAnnotationHandler } from "./editor-annotations"; import { createExternalAnnotationHandler } from "./external-annotations"; import { createAgentJobHandler } from "./agent-jobs"; +import { + evaluateTripwires, + tripwireHitToReviewAnnotation, + TRIPWIRE_SOURCE, +} from "@plannotator/shared/tripwires"; +import { resolveMergedTripwires } from "./tripwires"; import { CODEX_REVIEW_SYSTEM_PROMPT, buildCodexCommand, @@ -194,6 +200,48 @@ export async function startReviewServer( } return options.agentCwd ?? resolveVcsCwd(currentDiffType, gitContext?.cwd) ?? process.cwd(); }; + + // Tripwires — re-evaluate the current patch against the merged global + repo + // tripwires config and surface hits as external annotations. The global layer + // lives under `/tripwires/.json` (keyed by remote identity), the + // repo layer is `.plannotator/tripwires.json`; rules are merged additively. + // Fully fail-open: any failure logs and leaves the review untouched. Runs at + // startup and after every patch reassignment (diff switch, PR scope, PR + // switch). Defined after resolveAgentCwd to avoid a TDZ on the cwd resolver. + const refreshTripwires = async (): Promise => { + try { + // Stale hits from a previous patch never carry over. + externalAnnotations.clearBySource(TRIPWIRE_SOURCE); + + // resolveMergedTripwires runs git resolution, ensures the global file + // exists (write-once), re-reads both layers, and merges them. Re-running + // per refresh re-reads file contents so edits land without a restart; + // ensureGlobalTripwiresFile is idempotent so the once-only intent holds. + const { config, root, globalDiagnostics } = await resolveMergedTripwires(resolveAgentCwd()); + + // Surface a corrupt GLOBAL file (error-level diagnostics) so users can + // tell it apart from an empty one. A bad file still fails open to empty. + for (const d of globalDiagnostics) { + if (d.level === "error") { + console.error(`[tripwires] global config error: ${d.message}`); + } + } + + // Global rules fire even when root is null (PR mode without a checkout), + // so do NOT early-return on a missing repo root. + const hits = evaluateTripwires(currentPatch, config, { cwd: root ?? undefined }); + if (hits.length === 0) return; + + const result = externalAnnotations.addAnnotations({ + annotations: hits.map(tripwireHitToReviewAnnotation), + }); + if ("error" in result) console.error("[tripwires] addAnnotations error:", result.error); + } catch (err) { + console.error("[tripwires] refresh failed:", err); + } + }; + void refreshTripwires(); + const agentJobs = createAgentJobHandler({ mode: "review", getServerUrl: () => serverUrl, @@ -516,6 +564,9 @@ export async function startReviewServer( baseEverSwitched = true; currentError = result.error; + // Re-evaluate tripwires against the freshly switched patch. + await refreshTripwires(); + // Recompute gitContext for the effective cwd so the client's // sidebar (current branch, default branch, diff-mode options) // reflects the worktree we're now reviewing — not the main @@ -568,6 +619,7 @@ export async function startReviewServer( currentGitRef = originalPRGitRef; currentError = originalPRError; currentPRDiffScope = "layer"; + await refreshTripwires(); return Response.json({ rawPatch: currentPatch, gitRef: currentGitRef, @@ -596,6 +648,8 @@ export async function startReviewServer( currentError = undefined; currentPRDiffScope = "full-stack"; + await refreshTripwires(); + return Response.json({ rawPatch: currentPatch, gitRef: currentGitRef, @@ -715,6 +769,9 @@ export async function startReviewServer( branch: `${getMRLabel(pr.metadata)} ${getMRNumberLabel(pr.metadata)}`, }; + // Re-evaluate tripwires against the newly switched PR's patch. + await refreshTripwires(); + return Response.json({ rawPatch: currentPatch, gitRef: currentGitRef, diff --git a/packages/server/tripwires.test.ts b/packages/server/tripwires.test.ts new file mode 100644 index 000000000..9d1ccc7b2 --- /dev/null +++ b/packages/server/tripwires.test.ts @@ -0,0 +1,101 @@ +/** + * Tripwires runtime-glue tests (Bun server). + * + * Run: bun test packages/server/tripwires.test.ts + * + * Focus: repoKeyBaseFromCwd's git<2.31 fallback. The original implementation + * only ran `git rev-parse --path-format=absolute --git-common-dir`; that flag + * was added in git 2.31 (2021) and errors on older git, so the glue returned + * null and every remote-less repo collapsed onto the same `local:` identity / + * shared global tripwires file. The fallback resolves the plain (relative) + * common-dir against cwd so distinct repos stay on distinct keys. + */ + +import { afterEach, describe, expect, test } from "bun:test"; +import { execFileSync } from "node:child_process"; +import { chmodSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { repoKeyBaseFromCwd } from "./tripwires"; + +const tempDirs: string[] = []; + +function makeTempDir(prefix: string): string { + const dir = mkdtempSync(join(tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} + +function initRepo(): string { + const dir = makeTempDir("plannotator-tw-repo-"); + // Remote-less repo (no `origin`) — this is what hits the common-dir fallback. + execFileSync("git", ["init", "-q"], { cwd: dir }); + return dir; +} + +afterEach(() => { + for (const dir of tempDirs.splice(0)) { + rmSync(dir, { recursive: true, force: true }); + } +}); + +/** + * Build a PATH shim whose `git` rejects `--path-format=absolute` (simulating + * git < 2.31) but forwards every other invocation to the real git, then run + * `fn` with that shim prepended to PATH. + */ +async function withOldGit(fn: () => Promise): Promise { + const shimDir = makeTempDir("plannotator-tw-oldgit-"); + const realGit = execFileSync("which", ["git"]).toString().trim(); + const shim = join(shimDir, "git"); + writeFileSync( + shim, + `#!/bin/sh +for arg in "$@"; do + if [ "$arg" = "--path-format=absolute" ]; then + echo "error: unknown option \`path-format=absolute'" 1>&2 + exit 129 + fi +done +exec ${JSON.stringify(realGit)} "$@" +`, + ); + chmodSync(shim, 0o755); + + const prevPath = process.env.PATH; + process.env.PATH = `${shimDir}:${prevPath}`; + try { + return await fn(); + } finally { + process.env.PATH = prevPath; + } +} + +describe("repoKeyBaseFromCwd", () => { + test("returns a per-repo base on modern git", async () => { + const repo = initRepo(); + const base = await repoKeyBaseFromCwd(repo); + expect(base).not.toBeNull(); + }); + + test("git<2.31 fallback keeps distinct remote-less repos on distinct bases", async () => { + const repoA = initRepo(); + const repoB = initRepo(); + + await withOldGit(async () => { + const baseA = await repoKeyBaseFromCwd(repoA); + const baseB = await repoKeyBaseFromCwd(repoB); + + // The whole point: neither collapses to null (which would yield the shared + // `local:` identity), and the two repos never share a base. + expect(baseA).not.toBeNull(); + expect(baseB).not.toBeNull(); + expect(baseA).not.toBe(baseB); + }); + }); + + test("returns null outside a git repo", async () => { + const notARepo = makeTempDir("plannotator-tw-nonrepo-"); + expect(await repoKeyBaseFromCwd(notARepo)).toBeNull(); + }); +}); diff --git a/packages/server/tripwires.ts b/packages/server/tripwires.ts new file mode 100644 index 000000000..5ae4c7167 --- /dev/null +++ b/packages/server/tripwires.ts @@ -0,0 +1,211 @@ +/** + * Tripwires — Bun server helpers. + * + * Runtime glue for the pure tripwire evaluation logic in + * @plannotator/shared/tripwires. Two layers are resolved and merged: a global, + * per-project file under `/tripwires/.json` (keyed by remote + * identity, or the git-common-dir for remote-less repos) and the repo-local + * `.plannotator/tripwires.json`. Every helper fails open (returns null / empty) + * so a missing repo, missing config, or unreadable global file never breaks + * code review. + * + * The Pi extension has mirror helpers using node:child_process at + * apps/pi-extension/server/serverReview.ts. + */ + +import { $ } from "bun"; +import { createHash } from "node:crypto"; +import { dirname, join, resolve } from "node:path"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { + normalizeRemoteIdentity, + mergeTripwiresConfigs, + parseTripwiresConfig, + parseTripwiresConfigDetailed, + type TripwiresConfig, + type TripwireDiagnostic, +} from "@plannotator/shared/tripwires"; +import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; + +/** + * Resolve the git repository root for a working directory. Returns null when + * the directory is not inside a git repo (or git is unavailable). + */ +export async function repoRootFromCwd(cwd: string): Promise { + try { + const result = await $`git rev-parse --show-toplevel`.cwd(cwd).quiet().nothrow(); + if (result.exitCode === 0) { + const root = result.stdout.toString().trim(); + return root || null; + } + } catch { + // Not in a git repo + } + return null; +} + +/** + * Read `.plannotator/tripwires.json` from a repo root. Returns null on miss or + * any read error (fail-open). + */ +export function readTripwiresFile(root: string): string | null { + try { + const path = `${root}/.plannotator/tripwires.json`; + if (!existsSync(path)) return null; + return readFileSync(path, "utf-8"); + } catch { + return null; + } +} + +/** + * Resolve the `origin` remote URL for a working directory. Returns null when + * there is no remote (or git is unavailable). Used to derive a stable project + * identity so the global tripwires file follows a repo across clones/worktrees. + */ +export async function remoteUrlFromCwd(cwd: string): Promise { + try { + const result = await $`git remote get-url origin`.cwd(cwd).quiet().nothrow(); + if (result.exitCode === 0) { + const url = result.stdout.toString().trim(); + return url || null; + } + } catch { + // No remote / not a git repo + } + return null; +} + +/** + * Resolve the canonical per-repo base directory shared across linked worktrees. + * Uses `git rev-parse --git-common-dir` (which resolves to the shared `.git` + * across all worktrees, unlike `--show-toplevel` which is per-worktree), then + * takes its parent. Used only for the remote-less identity fallback so all + * worktrees of one repo collapse to a single global key. Returns null when not + * in a git repo. + */ +export async function repoKeyBaseFromCwd(cwd: string): Promise { + try { + const result = await $`git rev-parse --path-format=absolute --git-common-dir` + .cwd(cwd) + .quiet() + .nothrow(); + if (result.exitCode === 0) { + const commonDir = result.stdout.toString().trim(); + if (commonDir) return dirname(commonDir); + } + // `--path-format=absolute` was added in git 2.31 (2021); on older git the + // command above errors. Fall back to the plain (possibly relative) + // common-dir resolved against cwd, so distinct remote-less repos never + // collapse onto one shared global key (worktree-sharing is then best-effort). + const fallback = await $`git rev-parse --git-common-dir`.cwd(cwd).quiet().nothrow(); + if (fallback.exitCode === 0) { + const commonDir = fallback.stdout.toString().trim(); + if (commonDir) return dirname(resolve(cwd, commonDir)); + } + } catch { + // Not in a git repo + } + return null; +} + +/** Hash a project identity into a short, filesystem-safe key. */ +export function projectKeyFromIdentity(identity: string): string { + return createHash("sha256").update(identity).digest("hex").slice(0, 16); +} + +/** Absolute path of the global tripwires file for a project key. */ +export function globalTripwiresPath(key: string): string { + return join(getPlannotatorDataDir(), "tripwires", `${key}.json`); +} + +/** + * Read the global tripwires file for a project key. Returns null on miss or any + * read error (fail-open). + */ +export function readGlobalTripwiresFile(key: string): string | null { + try { + const path = globalTripwiresPath(key); + if (!existsSync(path)) return null; + return readFileSync(path, "utf-8"); + } catch { + return null; + } +} + +/** + * Lazily create the global tripwires file (`{ "rules": [] }`) the first time a + * project is reviewed, so users have a discoverable file to populate. Write-once + * and race-tolerant: if the file appears between the exists-check and the write + * (another concurrent session), the EEXIST is swallowed; any other error is + * logged but never thrown (fail-open). + */ +export function ensureGlobalTripwiresFile(key: string): void { + try { + const path = globalTripwiresPath(key); + if (existsSync(path)) return; + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, `{\n "rules": []\n}\n`, { flag: "wx" }); + } catch (err) { + const code = (err as NodeJS.ErrnoException)?.code; + // EEXIST: a concurrent session created it first — that's fine. + if (code === "EEXIST") return; + console.error("[tripwires] failed to create global file:", err); + } +} + +/** + * Write the global tripwires file for a project key. Unlike review-time reads + * this is an explicit user action (`tripwires add`), so failures THROW for the + * CLI to report rather than failing open. + */ +export function writeGlobalTripwiresFile(key: string, json: string): string { + const path = globalTripwiresPath(key); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, json, "utf-8"); + return path; +} + +/** + * Write the repo-committed tripwires file. Only called from the explicit + * `tripwires add --repo` path — this is the ONE place Plannotator creates + * `.plannotator/` inside a repo, and only because the user asked. Throws on + * failure (explicit action, not fail-open). + */ +export function writeRepoTripwiresFile(root: string, json: string): string { + const path = join(root, ".plannotator", "tripwires.json"); + mkdirSync(dirname(path), { recursive: true }); + writeFileSync(path, json, "utf-8"); + return path; +} + +/** + * Resolve the merged (global + repo) tripwires config for a working directory. + * Global rules come first, repo rules are appended (see + * {@link mergeTripwiresConfigs}). The global file is auto-created once if + * missing. Returns the merged config plus the repo root, project key, and any + * error-level diagnostics from the global layer (the caller decides logging). + */ +export async function resolveMergedTripwires(cwd: string): Promise<{ + config: TripwiresConfig; + root: string | null; + key: string | null; + globalDiagnostics: TripwireDiagnostic[]; +}> { + const root = await repoRootFromCwd(cwd); + const remote = await remoteUrlFromCwd(cwd); + const keyBase = await repoKeyBaseFromCwd(cwd); + const identity = normalizeRemoteIdentity(remote, keyBase); + const key = projectKeyFromIdentity(identity); + + ensureGlobalTripwiresFile(key); + const globalParsed = parseTripwiresConfigDetailed(readGlobalTripwiresFile(key)); + const repo = root ? parseTripwiresConfig(readTripwiresFile(root)) : { rules: [] }; + + return { + config: mergeTripwiresConfigs(globalParsed.config, repo), + root, + key, + globalDiagnostics: globalParsed.diagnostics, + }; +} diff --git a/packages/shared/external-annotation.ts b/packages/shared/external-annotation.ts index f88cc84b9..2d7ecbfc2 100644 --- a/packages/shared/external-annotation.ts +++ b/packages/shared/external-annotation.ts @@ -215,12 +215,27 @@ export function transformReviewInput( const filePath = requireString(obj, "filePath", i); if (typeof filePath !== "string") return filePath; - if (typeof obj.lineStart !== "number") { - return { error: `annotations[${i}] missing required "lineStart" field` }; + // scope: optional, defaults to "line" + const scope = typeof obj.scope === "string" ? obj.scope : "line"; + if (!VALID_SCOPES.includes(scope)) { + return { + error: `annotations[${i}] invalid scope "${scope}". Must be one of: ${VALID_SCOPES.join(", ")}`, + }; } - if (typeof obj.lineEnd !== "number") { - return { error: `annotations[${i}] missing required "lineEnd" field` }; + + // Line anchors are required for line-scope annotations. File-scope + // annotations (e.g. a rename that touches a tripwired path with no edited + // line) may omit them; they default to 0 and render at the file level. + if (scope === "line") { + if (typeof obj.lineStart !== "number") { + return { error: `annotations[${i}] missing required "lineStart" field` }; + } + if (typeof obj.lineEnd !== "number") { + return { error: `annotations[${i}] missing required "lineEnd" field` }; + } } + const lineStart = typeof obj.lineStart === "number" ? obj.lineStart : 0; + const lineEnd = typeof obj.lineEnd === "number" ? obj.lineEnd : 0; // side: optional, defaults to "new" const side = typeof obj.side === "string" ? obj.side : "new"; @@ -238,14 +253,6 @@ export function transformReviewInput( }; } - // scope: optional, defaults to "line" - const scope = typeof obj.scope === "string" ? obj.scope : "line"; - if (!VALID_SCOPES.includes(scope)) { - return { - error: `annotations[${i}] invalid scope "${scope}". Must be one of: ${VALID_SCOPES.join(", ")}`, - }; - } - // Must have at least text or suggestedCode if (typeof obj.text !== "string" && typeof obj.suggestedCode !== "string") { return { @@ -258,8 +265,8 @@ export function transformReviewInput( type, scope, filePath, - lineStart: obj.lineStart, - lineEnd: obj.lineEnd, + lineStart, + lineEnd, side, text: typeof obj.text === "string" ? obj.text : undefined, suggestedCode: typeof obj.suggestedCode === "string" ? obj.suggestedCode : undefined, diff --git a/packages/shared/package.json b/packages/shared/package.json index acd563ddf..84e715193 100644 --- a/packages/shared/package.json +++ b/packages/shared/package.json @@ -27,6 +27,7 @@ "./resolve-file": "./resolve-file.ts", "./extract-code-paths": "./extract-code-paths.ts", "./external-annotation": "./external-annotation.ts", + "./tripwires": "./tripwires.ts", "./agent-jobs": "./agent-jobs.ts", "./config": "./config.ts", "./data-dir": "./data-dir.ts", diff --git a/packages/shared/review-args.test.ts b/packages/shared/review-args.test.ts index 6bf57ee42..f4f78de7e 100644 --- a/packages/shared/review-args.test.ts +++ b/packages/shared/review-args.test.ts @@ -61,4 +61,54 @@ describe("parseReviewArgs", () => { useLocal: true, }); }); + + test("parses --tripwires", () => { + expect(parseReviewArgs("--tripwires").tripwires).toBe(true); + }); + + test("parses the -t alias", () => { + expect(parseReviewArgs("-t").tripwires).toBe(true); + }); + + test("parses --add-tripwire ", () => { + expect(parseReviewArgs("--add-tripwire src/auth/**").addTripwire).toBe("src/auth/**"); + }); + + test("--add-tripwire consumes the whole unquoted multi-word description", () => { + const parsed = parseReviewArgs("--add-tripwire protect the billing module from edits"); + expect(parsed.addTripwire).toBe("protect the billing module from edits"); + expect(parsed.prUrl).toBeUndefined(); + }); + + test("flags before --add-tripwire still parse; tokens after it join the description", () => { + const parsed = parseReviewArgs("--git --add-tripwire the auth core --no-local"); + expect(parsed.vcsType).toBe("git"); + // Everything after the flag is description, even flag-shaped tokens. + expect(parsed.addTripwire).toBe("the auth core --no-local"); + }); + + test("-t composes with --git and a PR URL", () => { + const parsed = parseReviewArgs("-t --git https://github.com/acme/repo/pull/12"); + expect(parsed.tripwires).toBe(true); + expect(parsed.vcsType).toBe("git"); + expect(parsed.prUrl).toBe("https://github.com/acme/repo/pull/12"); + }); + + test("--add-tripwire as the last token with no value leaves addTripwire undefined", () => { + // The glob is expected as the NEXT token; missing it must not consume a + // positional or default to a URL. + expect(parseReviewArgs("--add-tripwire").addTripwire).toBeUndefined(); + }); + + test("--add-tripwire value is not treated as a positional PR target", () => { + const parsed = parseReviewArgs("--add-tripwire src/**"); + expect(parsed.addTripwire).toBe("src/**"); + expect(parsed.prUrl).toBeUndefined(); + }); + + test("flags parse identically through the argv-array path", () => { + const parsed = parseReviewArgs(["-t", "--add-tripwire", "lib/*.ts"]); + expect(parsed.tripwires).toBe(true); + expect(parsed.addTripwire).toBe("lib/*.ts"); + }); }); diff --git a/packages/shared/review-args.ts b/packages/shared/review-args.ts index c6bc759e7..c6663c431 100644 --- a/packages/shared/review-args.ts +++ b/packages/shared/review-args.ts @@ -5,6 +5,14 @@ export interface ParsedReviewArgs { prUrl?: string; vcsType?: VcsSelection; useLocal: boolean; + /** Non-interactive tripwire scan (`--tripwires` / `-t`). */ + tripwires?: boolean; + /** + * Natural-language description for `--add-tripwire `. + * Consumes everything after the flag (so unquoted multi-word descriptions + * work); undefined when the flag has no value. + */ + addTripwire?: string; } export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { @@ -14,9 +22,21 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { let vcsType: VcsSelection | undefined; let useLocal = true; + let tripwires: boolean | undefined; + let addTripwire: string | undefined; const positional: string[] = []; + // True after `--add-tripwire`: every remaining token is part of the + // natural-language description, not a positional or flag. A missing next + // token leaves addTripwire undefined. + let collectAddTripwire = false; + const addTripwireParts: string[] = []; + for (const token of tokens) { + if (collectAddTripwire) { + addTripwireParts.push(token); + continue; + } switch (token) { case "--git": vcsType = "git"; @@ -27,17 +47,30 @@ export function parseReviewArgs(input: string | string[]): ParsedReviewArgs { case "--no-local": useLocal = false; break; + case "--tripwires": + case "-t": + tripwires = true; + break; + case "--add-tripwire": + collectAddTripwire = true; + break; default: positional.push(token); break; } } + if (addTripwireParts.length > 0) { + addTripwire = addTripwireParts.join(" "); + } + const target = positional[0]; return { prUrl: target && isReviewUrl(target) ? target : undefined, vcsType, useLocal, + ...(tripwires ? { tripwires } : {}), + ...(addTripwire !== undefined ? { addTripwire } : {}), }; } diff --git a/packages/shared/tripwires.test.ts b/packages/shared/tripwires.test.ts new file mode 100644 index 000000000..477aa70c3 --- /dev/null +++ b/packages/shared/tripwires.test.ts @@ -0,0 +1,1062 @@ +import { describe, expect, test } from "bun:test"; +import { transformReviewInput } from "./external-annotation"; +import { + appendRuleToTripwiresJson, + buildAddTripwirePrompt, + evaluateTripwires, + formatTripwiresMarkdown, + globToRegExp, + matchesAnyGlob, + mergeTripwiresConfigs, + normalizeRemoteIdentity, + parseChangedLines, + parseTripwiresConfig, + parseTripwiresConfigDetailed, + tripwireHitToReviewAnnotation, + TRIPWIRE_SOURCE, + type TripwireRule, + type TripwiresConfig, +} from "./tripwires"; + +// --------------------------------------------------------------------------- +// Glob matching +// --------------------------------------------------------------------------- + +describe("globToRegExp / matchesAnyGlob", () => { + test("* matches a single segment but not across /", () => { + expect(globToRegExp("src/*.ts").test("src/index.ts")).toBe(true); + expect(globToRegExp("src/*.ts").test("src/deep/index.ts")).toBe(false); + }); + + test("** matches across path separators", () => { + expect(globToRegExp("src/**/index.ts").test("src/a/b/index.ts")).toBe(true); + expect(globToRegExp("src/**").test("src/a/b/c.ts")).toBe(true); + }); + + test("? matches exactly one non-slash char", () => { + expect(globToRegExp("a?.ts").test("ab.ts")).toBe(true); + expect(globToRegExp("a?.ts").test("abc.ts")).toBe(false); + expect(globToRegExp("a?.ts").test("a/.ts")).toBe(false); + }); + + test("leading **/ matches zero or more leading segments", () => { + const re = globToRegExp("**/auth.ts"); + expect(re.test("auth.ts")).toBe(true); + expect(re.test("packages/server/auth.ts")).toBe(true); + }); + + test("mid-path **/ matches zero or more segments (incl. none)", () => { + // Regression: a mid-path `**/` used to require at least one intervening + // segment (`^a/.*/b$`), so `a/b` silently failed to match. + const re = globToRegExp("packages/**/auth.ts"); + expect(re.test("packages/auth.ts")).toBe(true); // zero segments + expect(re.test("packages/server/auth.ts")).toBe(true); + expect(re.test("packages/a/b/auth.ts")).toBe(true); + expect(matchesAnyGlob("packages/auth.ts", ["packages/**/auth.ts"])).toBe(true); + expect(matchesAnyGlob("packages/server/auth.ts", ["packages/**/auth.ts"])).toBe(true); + }); + + test("mid-path **/ still requires the surrounding separators", () => { + // `a/**/b` must NOT collapse to `ab` (the slash is required). + expect(matchesAnyGlob("ab", ["a/**/b"])).toBe(false); + expect(matchesAnyGlob("aXb", ["a/**/b"])).toBe(false); + expect(matchesAnyGlob("a/b", ["a/**/b"])).toBe(true); + expect(matchesAnyGlob("a/x/y/b", ["a/**/b"])).toBe(true); + }); + + test("trailing /** keeps its leading separator (no zero-collapse)", () => { + expect(matchesAnyGlob("src/auth/x.ts", ["src/auth/**"])).toBe(true); + expect(matchesAnyGlob("src/auth/a/b/x.ts", ["src/auth/**"])).toBe(true); + // Must not match a sibling whose name merely starts with `auth`. + expect(matchesAnyGlob("src/authx.ts", ["src/auth/**"])).toBe(false); + }); + + test("regex specials in the literal portion are escaped", () => { + expect(globToRegExp("a.b.ts").test("a.b.ts")).toBe(true); + expect(globToRegExp("a.b.ts").test("axbxts")).toBe(false); + }); + + test("matchesAnyGlob ORs across globs", () => { + expect(matchesAnyGlob("src/auth.ts", ["lib/*.ts", "src/*.ts"])).toBe(true); + expect(matchesAnyGlob("src/auth.ts", ["lib/*.ts"])).toBe(false); + }); + + test("multiple wildcards separated by literals match without backtracking", () => { + expect(matchesAnyGlob("axbyc", ["a*b*c"])).toBe(true); + expect(matchesAnyGlob("abc", ["a*b*c"])).toBe(true); + expect(matchesAnyGlob("axbyd", ["a*b*c"])).toBe(false); + }); + + test("pathological glob does not catastrophically backtrack (ReDoS guard)", () => { + // The old regex translation emitted adjacent `.*` quantifiers, so a glob + // like `**a**a...Z` against a non-matching path backtracked exponentially. + // The linear matcher must return promptly (well under a wall-clock budget). + const hostileGlob = "**a".repeat(16) + ".ts"; + const path = "a".repeat(40) + "X"; + const start = performance.now(); + const result = matchesAnyGlob(path, [hostileGlob]); + const elapsed = performance.now() - start; + expect(result).toBe(false); + expect(elapsed).toBeLessThan(1000); // was ~36s with the old regex + + const start2 = performance.now(); + const result2 = matchesAnyGlob( + "packages/secret/" + "a".repeat(56), + ["packages/secret/" + "**".repeat(11) + "Z"], + ); + expect(performance.now() - start2).toBeLessThan(1000); // was an effective hang + expect(result2).toBe(false); + }); + + test("globs over MAX_GLOB_LENGTH are skipped (never compiled/matched)", () => { + const huge = "**a".repeat(100); // 300 chars + expect(huge.length).toBeGreaterThan(256); + expect(matchesAnyGlob("a".repeat(50), [huge])).toBe(false); + // A normal glob alongside it still matches. + expect(matchesAnyGlob("a/b.ts", [huge, "a/*.ts"])).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// Config parsing — fail-open +// --------------------------------------------------------------------------- + +describe("parseTripwiresConfig", () => { + test("malformed JSON fails open to empty rules", () => { + expect(parseTripwiresConfig("{not json")).toEqual({ rules: [] }); + }); + + test("null / undefined / wrong shape fail open", () => { + expect(parseTripwiresConfig(null)).toEqual({ rules: [] }); + expect(parseTripwiresConfig(undefined)).toEqual({ rules: [] }); + expect(parseTripwiresConfig("42")).toEqual({ rules: [] }); + expect(parseTripwiresConfig('{"rules":"nope"}')).toEqual({ rules: [] }); + }); + + test("one bad rule never discards siblings", () => { + const raw = JSON.stringify({ + rules: [ + { globs: ["src/*.ts"], note: "good" }, + { note: "no globs at all" }, // dropped + { globs: [] }, // dropped (empty) + { globs: ["lib/*.ts"], symbols: ["foo"] }, + ], + }); + const config = parseTripwiresConfig(raw); + expect(config.rules.length).toBe(2); + expect(config.rules[0].globs).toEqual(["src/*.ts"]); + expect(config.rules[1].globs).toEqual(["lib/*.ts"]); + }); + + test("defaults id to rule- and coerces symbols", () => { + const raw = JSON.stringify({ + rules: [{ globs: ["a.ts"], symbols: ["x", 5, "y"] }], + }); + const config = parseTripwiresConfig(raw); + expect(config.rules[0].id).toBe("rule-0"); + expect(config.rules[0].symbols).toEqual(["x", "y"]); + }); + + test("preserves explicit id and note", () => { + const raw = JSON.stringify({ + rules: [{ id: "auth", globs: ["a.ts"], note: "do not touch" }], + }); + const config = parseTripwiresConfig(raw); + expect(config.rules[0].id).toBe("auth"); + expect(config.rules[0].note).toBe("do not touch"); + }); +}); + +// --------------------------------------------------------------------------- +// parseChangedLines — line-number correctness +// --------------------------------------------------------------------------- + +describe("parseChangedLines", () => { + test("tracks new-side and old-side line numbers across multi-hunk multi-file patches", () => { + const patch = [ + "diff --git a/one.ts b/one.ts", + "index 111..222 100644", + "--- a/one.ts", + "+++ b/one.ts", + "@@ -1,3 +1,4 @@", + " const a = 1;", + "+const added = 2;", + " const b = 3;", + "-const removed = 4;", + " const c = 5;", + "@@ -10,2 +11,2 @@", + " keep();", + "-old();", + "+brandNew();", + "diff --git a/two.ts b/two.ts", + "index 333..444 100644", + "--- a/two.ts", + "+++ b/two.ts", + "@@ -5,1 +5,2 @@", + " base();", + "+extra();", + ].join("\n"); + + const spans = parseChangedLines(patch); + expect(spans.length).toBe(2); + + const one = spans[0]; + expect(one.filePath).toBe("one.ts"); + // added at new line 2 + const added = one.changed.find((c) => c.content === "const added = 2;"); + expect(added).toEqual({ filePath: "one.ts", lineNumber: 2, side: "new", content: "const added = 2;" }); + // removed at old line 3 + const removed = one.changed.find((c) => c.content === "const removed = 4;"); + expect(removed).toEqual({ filePath: "one.ts", lineNumber: 3, side: "old", content: "const removed = 4;" }); + // second hunk: old() removed at old line 11, brandNew() added at new line 12 + const oldCall = one.changed.find((c) => c.content === "old();"); + expect(oldCall).toEqual({ filePath: "one.ts", lineNumber: 11, side: "old", content: "old();" }); + const brandNew = one.changed.find((c) => c.content === "brandNew();"); + expect(brandNew).toEqual({ filePath: "one.ts", lineNumber: 12, side: "new", content: "brandNew();" }); + + const two = spans[1]; + expect(two.filePath).toBe("two.ts"); + const extra = two.changed.find((c) => c.content === "extra();"); + expect(extra).toEqual({ filePath: "two.ts", lineNumber: 6, side: "new", content: "extra();" }); + }); + + test("empty / null patch yields no spans", () => { + expect(parseChangedLines("")).toEqual([]); + // @ts-expect-error null guard + expect(parseChangedLines(null)).toEqual([]); + }); + + test("captures trailing hunk function-context", () => { + const patch = [ + "diff --git a/f.ts b/f.ts", + "--- a/f.ts", + "+++ b/f.ts", + "@@ -1,2 +1,3 @@ export function loadConfig() {", + " const a = 1;", + "+const b = 2;", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans[0].hunkContexts).toEqual(["export function loadConfig() {"]); + }); + + test("binary-file change emits a file-scope span (no hunks)", () => { + const patch = [ + "diff --git a/bin.dat b/bin.dat", + "index eaf36c1..bfa7018 100644", + "Binary files a/bin.dat and b/bin.dat differ", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans.length).toBe(1); + expect(spans[0].filePath).toBe("bin.dat"); + expect(spans[0].changed).toEqual([]); + }); + + test("GIT binary patch emits a file-scope span", () => { + const patch = [ + "diff --git a/img.png b/img.png", + "index aaa..bbb 100644", + "GIT binary patch", + "literal 5", + "McmZQzWcvRP00RpG0RR91", + "", + "literal 4", + "LcmZQzWMT#Y01f~L", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans.length).toBe(1); + expect(spans[0].filePath).toBe("img.png"); + }); + + test("mode-only (chmod) change emits a file-scope span", () => { + const patch = ["diff --git a/run.sh b/run.sh", "old mode 100644", "new mode 100755"].join("\n"); + const spans = parseChangedLines(patch); + expect(spans.length).toBe(1); + expect(spans[0].filePath).toBe("run.sh"); + expect(spans[0].changed).toEqual([]); + }); + + test("mode change bundled with edits does not produce a duplicate span", () => { + const patch = [ + "diff --git a/run.sh b/run.sh", + "old mode 100644", + "new mode 100755", + "index 111..222", + "--- a/run.sh", + "+++ b/run.sh", + "@@ -1 +1,2 @@", + " a", + "+b", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans.length).toBe(1); + expect(spans[0].changed.length).toBe(1); + }); + + test("C-quoted paths are decoded before prefix stripping", () => { + const patch = [ + 'diff --git "a/we\\"ird.ts" "b/we\\"ird.ts"', + "index 111..222 100644", + '--- "a/we\\"ird.ts"', + '+++ "b/we\\"ird.ts"', + "@@ -1 +1,2 @@", + " a", + "+b", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans.length).toBe(1); + expect(spans[0].filePath).toBe('we"ird.ts'); + }); + + test("C-quoted path with an escaped tab is decoded", () => { + const patch = [ + 'diff --git "a/we\\tird.ts" "b/we\\tird.ts"', + '--- "a/we\\tird.ts"', + '+++ "b/we\\tird.ts"', + "@@ -1 +1,2 @@", + " a", + "+b", + ].join("\n"); + const spans = parseChangedLines(patch); + expect(spans[0].filePath).toBe("we\tird.ts"); + }); +}); + +// --------------------------------------------------------------------------- +// evaluateTripwires — semantics +// --------------------------------------------------------------------------- + +function cfg(rules: TripwiresConfig["rules"]): TripwiresConfig { + return { rules }; +} + +describe("evaluateTripwires", () => { + test("globs-only rule trips on any added line, anchored to first change", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/auth.ts", + "--- a/src/auth.ts", + "+++ b/src/auth.ts", + "@@ -1,1 +1,2 @@", + " const a = 1;", + "+const b = 2;", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ id: "auth", globs: ["src/auth.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ ruleId: "auth", filePath: "src/auth.ts", scope: "line", side: "new", line: 2 }); + expect(hits[0].note).toBe("Touches a slop-free zone"); + }); + + test("symbol in added line trips with a line anchor", () => { + const patch = [ + "diff --git a/lib/core.ts b/lib/core.ts", + "--- a/lib/core.ts", + "+++ b/lib/core.ts", + "@@ -1,1 +1,2 @@", + " const a = 1;", + "+export function validateToken() {}", + ].join("\n"); + const hits = evaluateTripwires( + patch, + cfg([{ id: "r", globs: ["lib/*.ts"], symbols: ["validateToken"], note: "guarded" }]), + ); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "line", side: "new", line: 2, note: "guarded" }); + }); + + test("symbol in removed line trips on the old side", () => { + const patch = [ + "diff --git a/lib/core.ts b/lib/core.ts", + "--- a/lib/core.ts", + "+++ b/lib/core.ts", + "@@ -1,2 +1,1 @@", + " keep();", + "-validateToken();", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["lib/*.ts"], symbols: ["validateToken"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "line", side: "old", line: 2 }); + }); + + test("symbol only in hunk context yields a file-scope hit", () => { + const patch = [ + "diff --git a/lib/core.ts b/lib/core.ts", + "--- a/lib/core.ts", + "+++ b/lib/core.ts", + "@@ -2,2 +2,3 @@ export function validateToken() {", + " const a = 1;", + "+const b = 2;", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["lib/*.ts"], symbols: ["validateToken"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "file", filePath: "lib/core.ts" }); + expect(hits[0].side).toBeUndefined(); + expect(hits[0].line).toBeUndefined(); + }); + + test("multi-symbol rule trips on any of its symbols", () => { + const patch = [ + "diff --git a/lib/core.ts b/lib/core.ts", + "--- a/lib/core.ts", + "+++ b/lib/core.ts", + "@@ -1,1 +1,3 @@", + " base();", + "+alpha();", + "+beta();", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["lib/*.ts"], symbols: ["beta", "gamma"] }])); + expect(hits.length).toBe(1); + expect(hits[0].line).toBe(3); + }); + + test("pure-deletion file gives an old-side hit", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/auth.ts", + "--- a/src/auth.ts", + "+++ b/src/auth.ts", + "@@ -1,2 +1,1 @@", + " keep();", + "-gone();", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["src/auth.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "line", side: "old", line: 2 }); + }); + + test("deleted-file patch (/dev/null new path) gives a hit", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/auth.ts", + "deleted file mode 100644", + "--- a/src/auth.ts", + "+++ /dev/null", + "@@ -1,2 +0,0 @@", + "-const a = 1;", + "-const b = 2;", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["src/auth.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "line", side: "old", filePath: "src/auth.ts" }); + }); + + test("rename-only (no edits) gives a file-scope hit", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/login.ts", + "similarity index 100%", + "rename from src/auth.ts", + "rename to src/login.ts", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["src/auth.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "file", filePath: "src/login.ts" }); + }); + + test("rename + edits gives an anchored hit", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/login.ts", + "similarity index 80%", + "rename from src/auth.ts", + "rename to src/login.ts", + "--- a/src/auth.ts", + "+++ b/src/login.ts", + "@@ -1,1 +1,2 @@", + " const a = 1;", + "+const b = 2;", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ globs: ["src/login.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "line", side: "new", filePath: "src/login.ts", line: 2 }); + }); + + test("glob matching the old path of a rename trips", () => { + const patch = [ + "diff --git a/src/auth.ts b/src/login.ts", + "similarity index 80%", + "rename from src/auth.ts", + "rename to src/login.ts", + "--- a/src/auth.ts", + "+++ b/src/login.ts", + "@@ -1,1 +1,2 @@", + " const a = 1;", + "+const b = 2;", + ].join("\n"); + // The rule only knows the OLD path; the file is moving out of the zone. + const hits = evaluateTripwires(patch, cfg([{ id: "auth", globs: ["src/auth.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0].ruleId).toBe("auth"); + expect(hits[0].filePath).toBe("src/login.ts"); + }); + + test("rename + edit whose first changed line is a deletion keys to the NEW path", () => { + // Regression: an old-side anchor (a leading `-` line on a renamed+edited + // file) used to stamp the OLD path, which no review-editor surface keys by — + // the marker dropped, the sidebar card orphaned, click-to-jump broke. The + // hit must carry the NEW (display) path while still anchoring side/line. + const patch = [ + "diff --git a/old/secret.ts b/new/secret.ts", + "similarity index 80%", + "rename from old/secret.ts", + "rename to new/secret.ts", + "--- a/old/secret.ts", + "+++ b/new/secret.ts", + "@@ -1,2 +1,1 @@", + "-const removed = 1;", + " const kept = 2;", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ id: "r1", globs: ["**/secret.ts"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ + ruleId: "r1", + filePath: "new/secret.ts", // NEW path, not "old/secret.ts" + scope: "line", + side: "old", // the matched line still lives on the old side + line: 1, + }); + }); + + test("symbol hit on an old-side line of a renamed+edited file keys to the NEW path", () => { + const patch = [ + "diff --git a/old/secret.ts b/new/secret.ts", + "rename from old/secret.ts", + "rename to new/secret.ts", + "--- a/old/secret.ts", + "+++ b/new/secret.ts", + "@@ -1,2 +1,1 @@", + "-validateToken();", + " keep();", + ].join("\n"); + const hits = evaluateTripwires( + patch, + cfg([{ id: "r", globs: ["**/secret.ts"], symbols: ["validateToken"] }]), + ); + expect(hits.length).toBe(1); + expect(hits[0].filePath).toBe("new/secret.ts"); + expect(hits[0].side).toBe("old"); + }); + + test("binary-file change trips a glob-only rule (file-scope)", () => { + const patch = [ + "diff --git a/assets/logo.png b/assets/logo.png", + "index eaf36c1..bfa7018 100644", + "Binary files a/assets/logo.png and b/assets/logo.png differ", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ id: "r", globs: ["**/*.png"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "file", filePath: "assets/logo.png" }); + expect(hits[0].line).toBeUndefined(); + }); + + test("mode-only (chmod) change trips a glob-only rule (file-scope)", () => { + const patch = ["diff --git a/run.sh b/run.sh", "old mode 100644", "new mode 100755"].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ id: "r", globs: ["run.sh"] }])); + expect(hits.length).toBe(1); + expect(hits[0]).toMatchObject({ scope: "file", filePath: "run.sh" }); + }); + + test("C-quoted file path is decoded so the rule glob matches", () => { + const patch = [ + 'diff --git "a/we\\"ird.ts" "b/we\\"ird.ts"', + 'index 111..222 100644', + '--- "a/we\\"ird.ts"', + '+++ "b/we\\"ird.ts"', + "@@ -1 +1,2 @@", + " a", + "+b", + ].join("\n"); + const hits = evaluateTripwires(patch, cfg([{ id: "r", globs: ['we"ird.ts'] }])); + expect(hits.length).toBe(1); + expect(hits[0].filePath).toBe('we"ird.ts'); + }); + + test("pathological glob in a rule does not hang evaluation (ReDoS guard)", () => { + const patch = "diff --git a/x.ts b/x.ts\n--- a/x.ts\n+++ b/x.ts\n@@ -1 +1,2 @@\n a\n+b\n"; + const start = performance.now(); + const hits = evaluateTripwires(patch, cfg([{ id: "h", globs: ["**a".repeat(14) + ".ts"] }])); + expect(performance.now() - start).toBeLessThan(1000); // was ~50s + expect(hits).toEqual([]); + }); + + test("empty / null patch yields no hits", () => { + expect(evaluateTripwires("", cfg([{ globs: ["a.ts"] }]))).toEqual([]); + // @ts-expect-error null guard + expect(evaluateTripwires(null, cfg([{ globs: ["a.ts"] }]))).toEqual([]); + }); + + test("no rules yields no hits", () => { + const patch = "diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1,2 @@\n a\n+b\n"; + expect(evaluateTripwires(patch, cfg([]))).toEqual([]); + }); + + test("dedup: same rule + file + line emitted once", () => { + const patch = [ + "diff --git a/lib/core.ts b/lib/core.ts", + "--- a/lib/core.ts", + "+++ b/lib/core.ts", + "@@ -1,1 +1,2 @@", + " base();", + "+foo(); foo();", + ].join("\n"); + // Two globs both match the same file — only one hit for the one changed line. + const hits = evaluateTripwires( + patch, + cfg([{ id: "r", globs: ["lib/*.ts", "lib/**"], symbols: ["foo"] }]), + ); + expect(hits.length).toBe(1); + }); +}); + +// --------------------------------------------------------------------------- +// Annotation mapping + round-trip through transformReviewInput +// --------------------------------------------------------------------------- + +describe("tripwireHitToReviewAnnotation", () => { + test("line-scope hit maps to a valid review annotation accepted by transformReviewInput", () => { + const input = tripwireHitToReviewAnnotation({ + ruleId: "auth", + filePath: "src/auth.ts", + note: "do not touch", + scope: "line", + side: "new", + line: 42, + }); + expect(input).toEqual({ + source: TRIPWIRE_SOURCE, + type: "concern", + scope: "line", + filePath: "src/auth.ts", + text: "do not touch", + author: "Tripwire", + side: "new", + lineStart: 42, + lineEnd: 42, + }); + + const result = transformReviewInput(input); + expect("error" in result).toBe(false); + if ("error" in result) throw new Error(result.error); + const ann = result.annotations[0]; + expect(ann.source).toBe(TRIPWIRE_SOURCE); + expect(ann.type).toBe("concern"); + expect(ann.scope).toBe("line"); + expect(ann.filePath).toBe("src/auth.ts"); + expect(ann.lineStart).toBe(42); + expect(ann.lineEnd).toBe(42); + expect(ann.side).toBe("new"); + expect(ann.author).toBe("Tripwire"); + }); + + test("file-scope hit (no line anchor) is accepted by transformReviewInput", () => { + const input = tripwireHitToReviewAnnotation({ + ruleId: "auth", + filePath: "src/login.ts", + note: "Touches a slop-free zone", + scope: "file", + }); + expect(input.lineStart).toBeUndefined(); + expect(input.lineEnd).toBeUndefined(); + expect(input.side).toBeUndefined(); + + const result = transformReviewInput(input); + expect("error" in result).toBe(false); + if ("error" in result) throw new Error(result.error); + const ann = result.annotations[0]; + expect(ann.scope).toBe("file"); + expect(ann.filePath).toBe("src/login.ts"); + expect(ann.lineStart).toBe(0); + expect(ann.lineEnd).toBe(0); + }); + + test("default note is non-empty and survives the round-trip", () => { + const hits = evaluateTripwires( + "diff --git a/a.ts b/a.ts\n--- a/a.ts\n+++ b/a.ts\n@@ -1 +1,2 @@\n a\n+b\n", + cfg([{ globs: ["a.ts"] }]), + ); + expect(hits[0].note.length).toBeGreaterThan(0); + const result = transformReviewInput(tripwireHitToReviewAnnotation(hits[0])); + expect("error" in result).toBe(false); + if ("error" in result) throw new Error(result.error); + expect(result.annotations[0].text).toBe(hits[0].note); + }); +}); + +// --------------------------------------------------------------------------- +// normalizeRemoteIdentity — stable cross-protocol identity +// --------------------------------------------------------------------------- + +describe("normalizeRemoteIdentity", () => { + test("ssh, https, and ssh-with-port forms of the same remote collapse to one identity", () => { + const ssh = normalizeRemoteIdentity("git@github.com:backnotprop/plannotator.git", null); + const https = normalizeRemoteIdentity("https://github.com/backnotprop/plannotator.git", null); + const sshPort = normalizeRemoteIdentity("ssh://git@github.com:22/backnotprop/plannotator.git", null); + expect(ssh).toBe("github.com/backnotprop/plannotator"); + expect(https).toBe(ssh); + expect(sshPort).toBe(ssh); + }); + + test("credentials embedded in an https URL do not change the identity", () => { + const plain = normalizeRemoteIdentity("https://github.com/backnotprop/plannotator.git", null); + const withCreds = normalizeRemoteIdentity( + "https://user:token@github.com/backnotprop/plannotator.git", + null, + ); + expect(withCreds).toBe(plain); + }); + + test("trailing .git and trailing slash are stripped", () => { + expect(normalizeRemoteIdentity("https://github.com/acme/repo.git", null)).toBe("github.com/acme/repo"); + expect(normalizeRemoteIdentity("https://github.com/acme/repo/", null)).toBe("github.com/acme/repo"); + }); + + test("GitHub Enterprise host is lowercased", () => { + expect(normalizeRemoteIdentity("https://Git.Corp.EXAMPLE.com/team/proj.git", null)).toBe( + "git.corp.example.com/team/proj", + ); + }); + + test("no remote with no key base is stable and local-prefixed", () => { + expect(normalizeRemoteIdentity(null, null)).toBe("local:"); + expect(normalizeRemoteIdentity(undefined, null)).toBe("local:"); + expect(normalizeRemoteIdentity("", null)).toBe("local:"); + }); + + test("no remote falls back to local:", () => { + expect(normalizeRemoteIdentity(null, "/home/me/plannotator")).toBe("local:/home/me/plannotator"); + }); + + test("two worktrees sharing one git-common-dir base produce the same identity", () => { + // The runtime glue passes the canonicalized git-common-dir parent, which is + // shared across linked worktrees, so both worktrees collapse to one key. + const base = "/home/me/plannotator"; + const wtA = normalizeRemoteIdentity(null, base); + const wtB = normalizeRemoteIdentity(null, base); + expect(wtA).toBe(wtB); + }); + + test("an unparseable remote falls back to the local key base", () => { + // parseRemoteHost/parseRemoteUrl return null for nonsense, so we fall open + // to the local: form rather than emitting a half-formed host/path. + expect(normalizeRemoteIdentity("not a url", "/repo")).toBe("local:/repo"); + }); +}); + +// --------------------------------------------------------------------------- +// mergeTripwiresConfigs — additive merge with id de-collision +// --------------------------------------------------------------------------- + +describe("mergeTripwiresConfigs", () => { + const rule = (id: string, glob = "a.ts"): TripwireRule => ({ id, globs: [glob] }); + + test("global rules come first, repo rules appended", () => { + const merged = mergeTripwiresConfigs( + { rules: [rule("g1"), rule("g2")] }, + { rules: [rule("r1")] }, + ); + expect(merged.rules.map((r) => r.id)).toEqual(["g1", "g2", "r1"]); + }); + + test("colliding ids are suffixed -2, -3 deterministically", () => { + const merged = mergeTripwiresConfigs( + { rules: [rule("rule-0", "global.ts")] }, + { rules: [rule("rule-0", "repo.ts"), rule("rule-0", "repo2.ts")] }, + ); + expect(merged.rules.map((r) => r.id)).toEqual(["rule-0", "rule-0-2", "rule-0-3"]); + // Globs are preserved (only the id changes on collision). + expect(merged.rules[0].globs).toEqual(["global.ts"]); + expect(merged.rules[1].globs).toEqual(["repo.ts"]); + expect(merged.rules[2].globs).toEqual(["repo2.ts"]); + }); + + test("empty global passes the repo rules through unchanged", () => { + const merged = mergeTripwiresConfigs({ rules: [] }, { rules: [rule("r1")] }); + expect(merged.rules.map((r) => r.id)).toEqual(["r1"]); + }); + + test("a bad (non-array rules) layer does not wipe the other", () => { + // mergeTripwiresConfigs is defensive against a malformed layer object. + const merged = mergeTripwiresConfigs( + { rules: [rule("g1")] }, + { rules: undefined as unknown as TripwireRule[] }, + ); + expect(merged.rules.map((r) => r.id)).toEqual(["g1"]); + }); + + test("collision within the global layer itself is also de-duplicated", () => { + const merged = mergeTripwiresConfigs( + { rules: [rule("dup"), rule("dup")] }, + { rules: [] }, + ); + expect(merged.rules.map((r) => r.id)).toEqual(["dup", "dup-2"]); + }); +}); + +// --------------------------------------------------------------------------- +// parseTripwiresConfigDetailed — diagnostics +// --------------------------------------------------------------------------- + +describe("parseTripwiresConfigDetailed", () => { + test("malformed JSON yields an error diagnostic and an empty config", () => { + const result = parseTripwiresConfigDetailed("{not json"); + expect(result.config).toEqual({ rules: [] }); + expect(result.diagnostics.length).toBe(1); + expect(result.diagnostics[0].level).toBe("error"); + }); + + test("non-object and non-array rules yield error diagnostics", () => { + expect(parseTripwiresConfigDetailed("42").diagnostics[0]).toMatchObject({ level: "error" }); + expect(parseTripwiresConfigDetailed('{"rules":"nope"}').diagnostics[0]).toMatchObject({ + level: "error", + }); + }); + + test("missing-globs rule yields a warning with ruleIndex; siblings kept", () => { + const raw = JSON.stringify({ + rules: [ + { globs: ["src/*.ts"], note: "good" }, + { note: "no globs" }, + { globs: ["lib/*.ts"] }, + ], + }); + const result = parseTripwiresConfigDetailed(raw); + expect(result.config.rules.map((r) => r.globs)).toEqual([["src/*.ts"], ["lib/*.ts"]]); + const warning = result.diagnostics.find((d) => d.level === "warning"); + expect(warning).toBeDefined(); + expect(warning?.ruleIndex).toBe(1); + }); + + test("a valid config produces no diagnostics", () => { + const raw = JSON.stringify({ rules: [{ id: "a", globs: ["a.ts"] }] }); + expect(parseTripwiresConfigDetailed(raw).diagnostics).toEqual([]); + }); + + test("null / undefined produce no diagnostics (empty, not corrupt)", () => { + expect(parseTripwiresConfigDetailed(null).diagnostics).toEqual([]); + expect(parseTripwiresConfigDetailed(undefined).diagnostics).toEqual([]); + }); + + test("parseTripwiresConfig still delegates and returns just the config", () => { + const raw = JSON.stringify({ rules: [{ id: "a", globs: ["a.ts"] }] }); + expect(parseTripwiresConfig(raw)).toEqual({ rules: [{ id: "a", globs: ["a.ts"] }] }); + expect(parseTripwiresConfig("{bad")).toEqual({ rules: [] }); + }); +}); + +// --------------------------------------------------------------------------- +// formatTripwiresMarkdown — grouped list + live status +// --------------------------------------------------------------------------- + +describe("formatTripwiresMarkdown", () => { + test("groups into Global and Repo sections with tables", () => { + const md = formatTripwiresMarkdown({ + globalKey: "abc123", + globalPath: "~/.plannotator/tripwires/abc123.json", + repoPath: "/repo/.plannotator/tripwires.json", + globalRules: [{ id: "g1", globs: ["src/**"], symbols: ["validateToken"], note: "guarded" }], + repoRules: [{ id: "r1", globs: ["lib/*.ts"] }], + }); + expect(md).toContain("## Global (~/.plannotator/tripwires/abc123.json)"); + expect(md).toContain("## Repo (/repo/.plannotator/tripwires.json)"); + expect(md).toContain("| g1 | src/** | validateToken | guarded |"); + expect(md).toContain("| r1 | lib/*.ts | | |"); + }); + + test("empty layers render (none)", () => { + const md = formatTripwiresMarkdown({ + globalKey: null, + globalPath: "~/.plannotator/tripwires/none.json", + repoPath: null, + globalRules: [], + repoRules: [], + }); + expect(md).toContain("## Global"); + expect(md).toContain("## Repo (.plannotator/tripwires.json)"); + expect(md).toContain("(none)"); + }); + + test("hits append a live-status section marking tripped ids", () => { + const md = formatTripwiresMarkdown({ + globalKey: "k", + globalPath: "g.json", + repoPath: "r.json", + globalRules: [{ id: "g1", globs: ["src/**"] }], + repoRules: [], + hits: [{ ruleId: "g1", filePath: "src/auth.ts", note: "guarded", scope: "line", side: "new", line: 12 }], + }); + expect(md).toContain("## Live status"); + expect(md).toContain("`g1`"); + expect(md).toContain("src/auth.ts:12"); + }); + + test("collapses old+new hits at the same line into one status line", () => { + // An in-place edit trips on both the removed (old) and added (new) side at + // the same line; the human-readable status list should show it once. + const md = formatTripwiresMarkdown({ + globalKey: "k", + globalPath: "g.json", + repoPath: "r.json", + globalRules: [{ id: "auth", globs: ["auth.ts"] }], + repoRules: [], + hits: [ + { ruleId: "auth", filePath: "auth.ts", note: "Security critical", scope: "line", side: "old", line: 1 }, + { ruleId: "auth", filePath: "auth.ts", note: "Security critical", scope: "line", side: "new", line: 1 }, + ], + }); + const occurrences = md.split("`auth` tripped — auth.ts:1 — Security critical").length - 1; + expect(occurrences).toBe(1); + }); + + test("keeps distinct line numbers as separate status lines", () => { + const md = formatTripwiresMarkdown({ + globalKey: "k", + globalPath: "g.json", + repoPath: "r.json", + globalRules: [{ id: "auth", globs: ["auth.ts"] }], + repoRules: [], + hits: [ + { ruleId: "auth", filePath: "auth.ts", note: "Security critical", scope: "line", side: "old", line: 1 }, + { ruleId: "auth", filePath: "auth.ts", note: "Security critical", scope: "line", side: "new", line: 9 }, + ], + }); + expect(md).toContain("auth.ts:1"); + expect(md).toContain("auth.ts:9"); + }); + + test("empty hits array reports nothing tripped", () => { + const md = formatTripwiresMarkdown({ + globalKey: "k", + globalPath: "g.json", + repoPath: "r.json", + globalRules: [], + repoRules: [], + hits: [], + }); + expect(md).toContain("## Live status"); + expect(md).toContain("No tripwires tripped."); + }); + + test("omitting hits omits the live-status section entirely", () => { + const md = formatTripwiresMarkdown({ + globalKey: "k", + globalPath: "g.json", + repoPath: "r.json", + globalRules: [], + repoRules: [], + }); + expect(md).not.toContain("Live status"); + }); +}); + +// --------------------------------------------------------------------------- +// buildAddTripwirePrompt — agent-facing instruction (no file write) +// --------------------------------------------------------------------------- + +describe("buildAddTripwirePrompt", () => { + test("embeds the user's description and the no-write disclaimer", () => { + const prompt = buildAddTripwirePrompt({ + description: "protect the billing module from casual edits", + }); + expect(prompt).toContain("protect the billing module from casual edits"); + expect(prompt.toLowerCase()).toContain("did not write"); + // Instructs the validate-then-show ritual. + expect(prompt).toContain("plannotator tripwires validate"); + }); + + test("uses resolved paths when provided, placeholders otherwise", () => { + const resolved = buildAddTripwirePrompt({ + description: "auth core", + globalPath: "/home/u/.plannotator/tripwires/repo-abc123.json", + repoPath: "/work/repo/.plannotator/tripwires.json", + }); + expect(resolved).toContain("/home/u/.plannotator/tripwires/repo-abc123.json"); + expect(resolved).toContain("/work/repo/.plannotator/tripwires.json"); + + const placeholder = buildAddTripwirePrompt({ description: "auth core" }); + expect(placeholder).toContain("~/.plannotator/tripwires/.json"); + expect(placeholder).toContain(".plannotator/tripwires.json"); + }); + + test("defaults to the global file and frames repo as explicit opt-in", () => { + const prompt = buildAddTripwirePrompt({ description: "x" }); + expect(prompt).toContain("GLOBAL"); + expect(prompt.toLowerCase()).toContain("explicitly asked"); + }); + + test("documents the rule schema with a JSON example", () => { + const prompt = buildAddTripwirePrompt({ description: "x" }); + expect(prompt).toContain('"globs"'); + expect(prompt).toContain('"symbols"'); + expect(prompt).toContain('"note"'); + }); +}); + +// --------------------------------------------------------------------------- +// appendRuleToTripwiresJson — pure JSON append (fs lives in runtime glue) +// --------------------------------------------------------------------------- + +describe("appendRuleToTripwiresJson", () => { + test("starts a fresh document from null/empty input", () => { + for (const raw of [null, "", " "]) { + const result = appendRuleToTripwiresJson(raw, { globs: ["src/**"], note: "n" }); + expect(result.ok).toBe(true); + if (!result.ok) continue; + const doc = JSON.parse(result.json); + expect(doc.rules).toHaveLength(1); + expect(doc.rules[0]).toEqual({ id: "rule-1", globs: ["src/**"], note: "n" }); + expect(result.json.endsWith("\n")).toBe(true); + } + }); + + test("preserves existing rules verbatim and unknown top-level keys", () => { + const raw = JSON.stringify({ + $schema: "https://example.com/tripwires.json", + rules: [{ id: "keep-me", globs: ["a/**"], extra: "field" }], + }); + const result = appendRuleToTripwiresJson(raw, { globs: ["b/**"] }); + expect(result.ok).toBe(true); + if (!result.ok) return; + const doc = JSON.parse(result.json); + expect(doc.$schema).toBe("https://example.com/tripwires.json"); + expect(doc.rules[0]).toEqual({ id: "keep-me", globs: ["a/**"], extra: "field" }); + expect(doc.rules[1].globs).toEqual(["b/**"]); + }); + + test("generates non-colliding sequential ids", () => { + const raw = JSON.stringify({ rules: [{ id: "rule-1", globs: ["a"] }, { id: "rule-3", globs: ["b"] }] }); + const result = appendRuleToTripwiresJson(raw, { globs: ["c"] }); + expect(result.ok).toBe(true); + if (!result.ok) return; + const doc = JSON.parse(result.json); + // rules.length + 1 = 3 collides with rule-3 → bumps to rule-4. + expect(doc.rules[2].id).toBe("rule-4"); + }); + + test("rejects an explicit id collision", () => { + const raw = JSON.stringify({ rules: [{ id: "money", globs: ["a"] }] }); + const result = appendRuleToTripwiresJson(raw, { globs: ["b"], id: "money" }); + expect(result.ok).toBe(false); + }); + + test("refuses to clobber unparseable or non-object content", () => { + expect(appendRuleToTripwiresJson("{ not json", { globs: ["a"] }).ok).toBe(false); + expect(appendRuleToTripwiresJson("[1,2,3]", { globs: ["a"] }).ok).toBe(false); + }); + + test("rejects empty globs", () => { + expect(appendRuleToTripwiresJson(null, { globs: [] }).ok).toBe(false); + expect(appendRuleToTripwiresJson(null, { globs: [" "] }).ok).toBe(false); + }); + + test("omits empty symbols and blank notes, trims the note", () => { + const result = appendRuleToTripwiresJson(null, { globs: ["a"], symbols: [], note: " hi " }); + expect(result.ok).toBe(true); + if (!result.ok) return; + const rule = JSON.parse(result.json).rules[0]; + expect(rule.symbols).toBeUndefined(); + expect(rule.note).toBe("hi"); + }); + + test("appended rule round-trips through the detailed parser with no diagnostics", () => { + const result = appendRuleToTripwiresJson(null, { globs: ["src/**"], symbols: ["charge"], note: "money" }); + expect(result.ok).toBe(true); + if (!result.ok) return; + const parsed = parseTripwiresConfigDetailed(result.json); + expect(parsed.diagnostics).toHaveLength(0); + expect(parsed.config.rules).toHaveLength(1); + }); +}); diff --git a/packages/shared/tripwires.ts b/packages/shared/tripwires.ts new file mode 100644 index 000000000..0229e117d --- /dev/null +++ b/packages/shared/tripwires.ts @@ -0,0 +1,1172 @@ +/** + * Tripwires — slop-free zones. + * + * A tripwire declares a region of the codebase (by glob, optionally narrowed to + * symbols) that agents and people must not casually touch. When a diff touches a + * tripwired file or symbol — adding, editing, deleting, or renaming — the wire + * trips and an informational annotation surfaces in the review UI. + * + * Pure and runtime-agnostic: no node:fs, no Bun globals, no spawning. The only + * dependency is the sibling `./repo` parser (also vendored into the Pi + * extension's flat `generated/` layout, so the relative import resolves there); + * everything impure (hashing, git/fs I/O) lives in the runtime glue that wraps + * these helpers. + */ + +import { parseRemoteUrl, parseRemoteHost } from "./repo"; + +// --------------------------------------------------------------------------- +// Types +// --------------------------------------------------------------------------- + +export interface TripwireRule { + /** Stable identifier (defaulted to `rule-` when absent on parse). */ + id: string; + /** Repo-relative globs. Required and non-empty. */ + globs: string[]; + /** Optional symbol names to narrow the match within a file. */ + symbols?: string[]; + /** Optional human-readable note shown on the surfaced annotation. */ + note?: string; +} + +export interface TripwiresConfig { + rules: TripwireRule[]; +} + +/** A single tripped wire, ready to be mapped to a review annotation. */ +export interface TripwireHit { + /** The rule that tripped. */ + ruleId: string; + /** Repo-relative path of the file that touched the zone (new path for renames). */ + filePath: string; + /** Note to display. Always non-empty (defaulted to "Touches a slop-free zone"). */ + note: string; + /** "file" when there is no line anchor (e.g. a rename), "line" otherwise. */ + scope: "line" | "file"; + /** Side of the diff the anchor lives on. Omitted for file-scope hits. */ + side?: "old" | "new"; + /** Anchor line number (1-based). Omitted for file-scope hits. */ + line?: number; +} + +/** A single changed line extracted from a unified diff. */ +export interface ChangedLine { + filePath: string; + lineNumber: number; + side: "old" | "new"; + content: string; +} + +// --------------------------------------------------------------------------- +// Glob matching +// --------------------------------------------------------------------------- + +/** Hard cap on glob length. Defends matching against pathological inputs. */ +export const MAX_GLOB_LENGTH = 256; + +/** + * A compiled glob token. + * - `segGlob`: a `**` followed by `/` (a `**​/` or `/**​/` path component) — + * matches zero or more complete path segments, each terminated by a slash + * (equivalent to the regex `(?:[^/]+/)`-star). This is what lets `a/**​/b` + * also match `a/b` (zero intervening segments), matching git pathspec / + * minimatch and the leading-`**​/` convenience. + * - `anyChar`: a free-standing `**` (e.g. `a**b`, or a trailing `/**`) — + * matches any run of characters including slash (equivalent to `.`-star). + * Preserves the historical behavior for a `**` that is not a path component. + * - `star`: `*` — a run of non-slash characters. + * - `single`: `?` — a single non-slash character. + * - `literal`: matched verbatim. + */ +type GlobToken = + | { kind: "segGlob" } + | { kind: "anyChar" } + | { kind: "star" } + | { kind: "single" } + | { kind: "literal"; value: string }; + +/** + * Compile a glob string into a flat token list. + * + * Consecutive `*`/`**` are collapsed (a `**` wins over a `*`), so the token list + * never holds two adjacent unbounded wildcards — the catastrophic-backtracking + * shape in a naive regex translation. A `**` that forms a full path component + * (bounded by `/` or the string ends on its leading side) compiles to a + * `segGlob` that can match zero segments; any other `**` stays a cross-`/` + * `anyChar`. + */ +function compileGlob(glob: string): GlobToken[] { + const tokens: GlobToken[] = []; + let literal = ""; + const flushLiteral = () => { + if (literal.length > 0) { + tokens.push({ kind: "literal", value: literal }); + literal = ""; + } + }; + + for (let i = 0; i < glob.length; i++) { + const c = glob[i]; + if (c === "*") { + // Greedily consume a run of `*`; two or more means `**` (globstar). + let stars = 1; + while (glob[i + 1] === "*") { + stars++; + i++; + } + if (stars >= 2) { + const prevChar = glob[i - stars]; // char immediately before the run + const nextChar = glob[i + 1]; // char immediately after the run + const leadingBoundary = prevChar === "/" || prevChar === undefined; + // `**/` or `/**/` path component: zero-or-more *complete* segments + // (`segGlob` == `(?:[^/]+/)*`). The leading separator stays in the + // preceding literal (so `a/` is required), and the component's own + // trailing `/` is swallowed — this is what makes `a/**​/b` match both + // `a/b` (zero segments) and `a/x/b`. Only applies when a `/` follows the + // run; a trailing `**` (end of glob) keeps the historical `anyChar` + // cross-`/` semantics so `src/auth/**` still matches `src/auth/x.ts`. + if (leadingBoundary && nextChar === "/") { + flushLiteral(); + tokens.push({ kind: "segGlob" }); + i++; // swallow the component's trailing slash + } else { + // A `**` glued to non-slash chars (e.g. `a**b`, `**.ts`) or a trailing + // `/**` / bare `**`. Keep the historical cross-`/` semantics. + flushLiteral(); + tokens.push({ kind: "anyChar" }); + } + } else { + flushLiteral(); + tokens.push({ kind: "star" }); + } + } else if (c === "?") { + flushLiteral(); + tokens.push({ kind: "single" }); + } else { + literal += c; + } + } + flushLiteral(); + return tokens; +} + +/** + * A character-level matcher state. Literal runs are expanded to one `lit` state + * per char so the matcher can advance exactly one path character per step. + * + * Wildcard states loop: + * - `single` (`?`): exactly one non-slash char. + * - `anySeg` (`*`): zero+ non-slash chars. + * - `anyChar` (`**` not forming a `/**​/` component): zero+ of any char. + * - `segHead`/`segBody`: a `/**​/` component = `(?:[^/]+/)*`, zero or more + * complete path segments. `segHead` is the loop boundary (may exit with zero + * segments); `segBody` is "inside a segment, owe a trailing slash". They are + * always emitted as an adjacent pair. + */ +type MatchState = + | { kind: "lit"; ch: string } + | { kind: "single" } + | { kind: "anySeg" } + | { kind: "anyChar" } + | { kind: "segHead" } + | { kind: "segBody" }; + +/** Expand compiled glob tokens into a flat character-level state list. */ +function tokensToStates(tokens: GlobToken[]): MatchState[] { + const states: MatchState[] = []; + for (const token of tokens) { + if (token.kind === "literal") { + for (const ch of token.value) states.push({ kind: "lit", ch }); + } else if (token.kind === "single") { + states.push({ kind: "single" }); + } else if (token.kind === "star") { + states.push({ kind: "anySeg" }); + } else if (token.kind === "anyChar") { + states.push({ kind: "anyChar" }); + } else { + // segGlob — a `(?:[^/]+/)*` component, encoded as a head/body pair. + states.push({ kind: "segHead" }); + states.push({ kind: "segBody" }); + } + } + return states; +} + +/** + * Glob matcher with no catastrophic backtracking. Models the glob as a tiny NFA + * and advances a *set* of reachable states one path character at a time, so its + * cost is O(states × path.length) in the worst case — never exponential — no + * matter how many wildcards the glob contains. This is what makes a hostile + * `.plannotator/tripwires.json` unable to wedge the event loop (see the ReDoS + * notes on the old single-regex translation). + */ +function matchStates(states: MatchState[], path: string): boolean { + const n = states.length; + let active = new Array(n + 1).fill(false); + let next = new Array(n + 1).fill(false); + + // Epsilon closure: a state that can match zero chars also reaches the next + // state. `anySeg`/`anyChar` are zero-or-more; `segHead` may exit the loop + // with zero segments (its `segBody` companion is skipped to land on `i+2`). + const addState = (set: boolean[], i: number) => { + let cur = i; + while (cur <= n && !set[cur]) { + set[cur] = true; + const s = states[cur]; + if (s && (s.kind === "anySeg" || s.kind === "anyChar")) { + cur++; // wildcard may consume zero chars + continue; + } + if (s && s.kind === "segHead") { + cur += 2; // skip both head and body — zero segments + continue; + } + break; + } + }; + + addState(active, 0); + + for (let pi = 0; pi < path.length; pi++) { + const ch = path[pi]; + next.fill(false); + let any = false; + for (let i = 0; i < n; i++) { + if (!active[i]) continue; + const s = states[i]; + if (s.kind === "lit") { + if (ch === s.ch) { + addState(next, i + 1); + any = true; + } + } else if (s.kind === "single") { + if (ch !== "/") { + addState(next, i + 1); + any = true; + } + } else if (s.kind === "anySeg") { + if (ch !== "/") { + addState(next, i); // consume one non-slash char and stay + any = true; + } + } else if (s.kind === "anyChar") { + // `**`: consume any char, including `/`. + addState(next, i); + any = true; + } else if (s.kind === "segHead") { + // Start of a segment: first char must be non-slash (`[^/]+`). + if (ch !== "/") { + addState(next, i + 1); // enter the body + any = true; + } + } else { + // segBody — inside a `[^/]+/` segment. + if (ch === "/") { + addState(next, i - 1); // segment complete — back to the loop head + any = true; + } else { + addState(next, i); // still consuming the segment's non-slash chars + any = true; + } + } + } + if (!any) return false; + const tmp = active; + active = next; + next = tmp; + } + + return active[n]; +} + +/** Compile + match a single glob against a path (no backtracking). */ +function matchTokens(tokens: GlobToken[], path: string): boolean { + return matchStates(tokensToStates(tokens), path); +} + +/** + * Convert a glob to an anchored, full-match RegExp. + * + * Supports `**` (any path segments incl. `/`), `*` (any non-`/` run), and `?` (a + * single non-`/` char). A `**` that forms a `/**​/` path component matches zero + * or more segments, so both a leading `**​/foo.ts` and a mid-path `a/**​/foo.ts` + * match the zero-segment case (`foo.ts`, `a/foo.ts`). + * + * IMPORTANT: this still emits `.*` for a free-standing `**` (e.g. `a**b`), so a + * pathological glob (`**a**a**a…`) compiles to a catastrophic-backtracking + * regex. Do NOT run `globToRegExp(...).test(...)` on untrusted globs. The + * tripwire runtime never does — it matches via {@link matchesAnyGlob}, which + * uses a linear, backtracking-free NFA matcher and caps glob length. + */ +export function globToRegExp(glob: string): RegExp { + const tokens = compileGlob(glob); + let pattern = ""; + for (const token of tokens) { + if (token.kind === "literal") { + pattern += token.value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + } else if (token.kind === "single") { + pattern += "[^/]"; + } else if (token.kind === "star") { + pattern += "[^/]*"; + } else if (token.kind === "anyChar") { + pattern += ".*"; + } else { + // segGlob — a `/**​/` component: zero or more complete segments. No + // adjacent unbounded quantifiers, so not the catastrophic shape. + pattern += "(?:[^/]+/)*"; + } + } + return new RegExp("^" + pattern + "$"); +} + +/** True when `filePath` matches at least one glob. */ +export function matchesAnyGlob(filePath: string, globs: string[]): boolean { + for (const glob of globs) { + if (glob.length > MAX_GLOB_LENGTH) continue; // skip pathological globs + if (matchTokens(compileGlob(glob), filePath)) return true; + } + return false; +} + +// --------------------------------------------------------------------------- +// Patch parsing +// --------------------------------------------------------------------------- + +// Hunk header: @@ -oldStart,oldCount +newStart,newCount @@ [trailing context] +// Mirrors packages/review-editor/utils/patchParser.ts (the @@ regex) but also +// captures the trailing function-context text after the closing @@. +const HUNK_HEADER = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@(.*)$/; + +interface FileSpan { + /** New-side path (post-image). Falls back to old path for pure deletions. */ + filePath: string; + /** Old-side path (pre-image). Differs from filePath on renames. */ + oldPath: string; + /** True when the file is renamed (old path differs from new path). */ + renamed: boolean; + changed: ChangedLine[]; + /** Trailing hunk-context text per hunk (the bit after the closing @@). */ + hunkContexts: string[]; +} + +/** + * Decode git's C-style path quoting. Git wraps a path in double quotes and + * backslash-escapes special bytes when the name contains a tab, newline, double + * quote, or backslash — even under `core.quotePath=false` (which the review + * server sets). An unquoted string is returned unchanged. + * + * Handles `\"`, `\\`, `\t`, `\n`, `\r`, `\f`, `\b`, `\a`, `\v`, and octal + * `\NNN` escapes, matching how git apply / full diff viewers read these names. + */ +function unquoteGitPath(raw: string): string { + if (raw.length < 2 || raw[0] !== '"' || raw[raw.length - 1] !== '"') return raw; + const body = raw.slice(1, -1); + let out = ""; + for (let i = 0; i < body.length; i++) { + const c = body[i]; + if (c !== "\\") { + out += c; + continue; + } + const n = body[i + 1]; + if (n === undefined) { + out += "\\"; + break; + } + if (n >= "0" && n <= "7") { + // Octal escape: up to three digits. + let oct = n; + let j = i + 2; + while (oct.length < 3 && body[j] >= "0" && body[j] <= "7") { + oct += body[j]; + j++; + } + out += String.fromCharCode(parseInt(oct, 8)); + i = j - 1; + continue; + } + const simple: Record = { + '"': '"', + "\\": "\\", + t: "\t", + n: "\n", + r: "\r", + f: "\f", + b: "\b", + a: "\x07", + v: "\v", + }; + out += simple[n] ?? n; + i++; + } + return out; +} + +/** + * Parse the two paths from a `diff --git a/ b/` header. Returns null + * when the paths can't be recovered unambiguously (e.g. unquoted names with + * spaces — git itself relies on the `---`/`+++` lines in that case). Used to + * recover a path for binary / mode-only changes, which carry no `+++` line. + */ +function parseDiffGitHeader(line: string): { oldPath: string; newPath: string } | null { + const rest = line.slice("diff --git ".length); + // Both paths quoted: `"a/x" "b/y"`. + if (rest.startsWith('"')) { + const end = rest.indexOf('" ', 1); + if (end === -1) return null; + const first = unquoteGitPath(rest.slice(0, end + 1)); + const second = unquoteGitPath(rest.slice(end + 2).trim()); + return stripPair(first, second); + } + // Unquoted: `a/ b/`. Split on ` b/` — unambiguous only when the old + // path has no space (git would quote otherwise). Find the LAST ` b/` so an + // old path that itself contains ` b/` chooses the boundary git intended only + // in the common no-space case. + const sep = rest.lastIndexOf(" b/"); + if (sep === -1) return null; + const first = rest.slice(0, sep); + const second = rest.slice(sep + 1); + if (!first.startsWith("a/") || first.includes(" ")) return null; + return stripPair(first, second); +} + +function stripPair(first: string, second: string): { oldPath: string; newPath: string } | null { + const strip = (p: string) => + p.startsWith("a/") || p.startsWith("b/") ? p.slice(2) : p; + return { oldPath: strip(first), newPath: strip(second) }; +} + +/** + * Parse a unified diff into changed lines, tracking file paths across renames + * and /dev/null (added/deleted) headers. Returns one FileSpan per file. + * + * Also surfaces metadata-only changes that carry no hunks — binary-file edits + * (`Binary files ... differ` / `GIT binary patch`) and mode-only changes + * (`old mode` / `new mode`) — as zero-`changed` file spans, so glob-only rules + * still trip on them. + */ +export function parseChangedLines(patch: string): FileSpan[] { + const spans: FileSpan[] = []; + if (!patch) return spans; + + const lines = patch.split("\n"); + let current: FileSpan | null = null; + let oldLine = 0; + let newLine = 0; + // Rename headers carry the paths before the `+++/---` lines appear. + let pendingRenameFrom: string | null = null; + let pendingRenameTo: string | null = null; + // The `--- a/` header precedes `+++ b/`; remember it so we can + // recover the old path even when the new side is /dev/null (a deletion). + let pendingOldPath: string | null = null; + // Paths from the current block's `diff --git` header — the only path source + // for binary / mode-only changes, which have no `---`/`+++` lines. + let headerOldPath: string | null = null; + let headerNewPath: string | null = null; + // True when the current block has seen a mode change but not yet a span/hunk. + let pendingModeOnly = false; + + const startFile = (oldPath: string, newPath: string): FileSpan => { + const span: FileSpan = { + filePath: newPath, + oldPath, + renamed: oldPath !== newPath && oldPath !== "/dev/null" && newPath !== "/dev/null", + changed: [], + hunkContexts: [], + }; + spans.push(span); + return span; + }; + + // Emit a file-scope span for the block's header path if nothing else has. + // Covers mode-only changes detected at the block boundary. + const finalizeBlock = () => { + if (pendingModeOnly && !current && headerNewPath) { + startFile(headerOldPath ?? headerNewPath, headerNewPath); + } + }; + + // Strip the `a/` or `b/` prefix git puts on diff paths, decoding C-quoting. + const stripPrefix = (raw: string): string => { + const trimmed = unquoteGitPath(raw.trim()); + if (trimmed === "/dev/null") return "/dev/null"; + if (trimmed.startsWith("a/") || trimmed.startsWith("b/")) return trimmed.slice(2); + return trimmed; + }; + + for (const line of lines) { + if (line.startsWith("diff --git")) { + finalizeBlock(); + current = null; + oldLine = 0; + newLine = 0; + pendingRenameFrom = null; + pendingRenameTo = null; + pendingOldPath = null; + pendingModeOnly = false; + const header = parseDiffGitHeader(line); + headerOldPath = header?.oldPath ?? null; + headerNewPath = header?.newPath ?? null; + continue; + } + + if (line.startsWith("rename from ")) { + pendingRenameFrom = unquoteGitPath(line.slice("rename from ".length).trim()); + continue; + } + if (line.startsWith("rename to ")) { + pendingRenameTo = unquoteGitPath(line.slice("rename to ".length).trim()); + // A rename can be pure (no hunks); record a span eagerly so it still + // surfaces a file-scope hit even with zero edited lines. + if (pendingRenameFrom !== null && !current) { + current = startFile(pendingRenameFrom, pendingRenameTo); + } + continue; + } + + // Mode-only changes (chmod) carry `old mode`/`new mode` but no hunks; mark + // the block so finalizeBlock can emit a file-scope span from the header. + if (line.startsWith("old mode ") || line.startsWith("new mode ")) { + pendingModeOnly = true; + continue; + } + + // Binary changes carry no `---`/`+++` and no hunks. Emit a file-scope span + // eagerly from the `diff --git` header path so glob-only rules still trip. + if ( + line.startsWith("Binary files ") || + line.startsWith("GIT binary patch") || + line === "GIT binary patch" + ) { + if (!current && headerNewPath) { + current = startFile(headerOldPath ?? headerNewPath, headerNewPath); + } + pendingModeOnly = false; // a span now exists; don't double-emit + continue; + } + + if (line.startsWith("--- ")) { + // Remember the old path; defer span creation until we also see `+++`. + pendingOldPath = stripPrefix(line.slice(4)); + continue; + } + if (line.startsWith("+++ ")) { + const newPath = stripPrefix(line.slice(4)); + // Recover the old path from the `---` header (falling back to rename + // metadata, then the new path) so deletions and renames keep both sides. + let oldPath = pendingOldPath ?? pendingRenameFrom ?? newPath; + if (oldPath === "/dev/null") oldPath = pendingRenameFrom ?? newPath; + if (!(current && (current.renamed || current.filePath === newPath))) { + current = startFile(oldPath, newPath); + } + pendingRenameFrom = null; + pendingRenameTo = null; + pendingOldPath = null; + pendingModeOnly = false; // a content span exists; don't also emit mode-only + continue; + } + + const hunk = HUNK_HEADER.exec(line); + if (hunk) { + oldLine = parseInt(hunk[1], 10); + newLine = parseInt(hunk[2], 10); + if (current) current.hunkContexts.push(hunk[3].trim()); + continue; + } + + if (!current) continue; + + const prefix = line[0]; + const content = line.slice(1); + if (prefix === "+") { + current.changed.push({ + filePath: current.filePath, + lineNumber: newLine, + side: "new", + content, + }); + newLine++; + } else if (prefix === "-") { + current.changed.push({ + filePath: current.oldPath, + lineNumber: oldLine, + side: "old", + content, + }); + oldLine++; + } else if (prefix === " ") { + oldLine++; + newLine++; + } + // Ignore "\ No newline at end of file" and any other metadata lines. + } + + finalizeBlock(); // flush a trailing mode-only block + return spans; +} + +// --------------------------------------------------------------------------- +// Config parsing +// --------------------------------------------------------------------------- + +const DEFAULT_NOTE = "Touches a slop-free zone"; + +/** A diagnostic surfaced while parsing a tripwires config layer. */ +export interface TripwireDiagnostic { + /** `error` = the whole layer was unusable; `warning` = a single rule dropped. */ + level: "error" | "warning"; + message: string; + /** Index of the offending rule (warnings only). */ + ruleIndex?: number; +} + +/** Result of {@link parseTripwiresConfigDetailed}: the (fail-open) config plus diagnostics. */ +export interface TripwiresParseResult { + config: TripwiresConfig; + diagnostics: TripwireDiagnostic[]; +} + +/** + * Parse a tripwires config from raw JSON text, accumulating diagnostics. + * Fail-open: any parse error, malformed shape, or invalid rule yields a safe + * config — one bad rule never discards its siblings. This variant never logs; + * the runtime glue decides whether (and where) to surface diagnostics so a + * corrupt config can be distinguished from an empty one. + */ +export function parseTripwiresConfigDetailed( + raw: string | null | undefined, +): TripwiresParseResult { + const diagnostics: TripwireDiagnostic[] = []; + if (!raw || typeof raw !== "string") return { config: { rules: [] }, diagnostics }; + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + diagnostics.push({ + level: "error", + message: `failed to parse tripwires JSON: ${err instanceof Error ? err.message : String(err)}`, + }); + return { config: { rules: [] }, diagnostics }; + } + + if (!parsed || typeof parsed !== "object") { + diagnostics.push({ level: "error", message: "tripwires config is not an object" }); + return { config: { rules: [] }, diagnostics }; + } + const rawRules = (parsed as { rules?: unknown }).rules; + if (!Array.isArray(rawRules)) { + diagnostics.push({ level: "error", message: "tripwires config `rules` is not an array" }); + return { config: { rules: [] }, diagnostics }; + } + + const rules: TripwireRule[] = []; + for (let i = 0; i < rawRules.length; i++) { + const rule = rawRules[i]; + if (!rule || typeof rule !== "object") { + diagnostics.push({ level: "warning", message: "rule is not an object", ruleIndex: i }); + continue; + } + const r = rule as Record; + + // globs is required and must contain at least one non-empty string. + if (!Array.isArray(r.globs)) { + diagnostics.push({ level: "warning", message: "rule is missing a `globs` array", ruleIndex: i }); + continue; + } + const globs = r.globs.filter((g): g is string => typeof g === "string" && g.length > 0); + if (globs.length === 0) { + diagnostics.push({ level: "warning", message: "rule has no non-empty globs", ruleIndex: i }); + continue; + } + + const symbols = Array.isArray(r.symbols) + ? r.symbols.filter((s): s is string => typeof s === "string" && s.length > 0) + : undefined; + + rules.push({ + id: typeof r.id === "string" && r.id.length > 0 ? r.id : `rule-${i}`, + globs, + ...(symbols && symbols.length > 0 ? { symbols } : {}), + ...(typeof r.note === "string" && r.note.length > 0 ? { note: r.note } : {}), + }); + } + + return { config: { rules }, diagnostics }; +} + +/** + * Parse a tripwires config from raw JSON text. Fail-open: any parse error, + * malformed shape, or invalid rule yields a safe config — one bad rule never + * discards its siblings. Thin wrapper over {@link parseTripwiresConfigDetailed} + * that drops the diagnostics. + */ +export function parseTripwiresConfig(raw: string | null | undefined): TripwiresConfig { + return parseTripwiresConfigDetailed(raw).config; +} + +// --------------------------------------------------------------------------- +// Identity normalization + config merge (two-layer global/repo store) +// --------------------------------------------------------------------------- + +/** + * Normalize a repo's identity into a stable string used to key the global + * tripwires file. Pure — composes the two existing `./repo` parsers; the runtime + * glue hashes the result. + * + * - Remote present: `host/path` where `host` is the lowercased + * {@link parseRemoteHost} result and `path` is {@link parseRemoteUrl} (which + * already strips the scheme, credentials, `.git` suffix, and host). SSH, HTTPS, + * and SSH-with-port forms of the same remote all collapse to the same string, + * e.g. `github.com/backnotprop/plannotator`. + * - Remote absent (either parser returns null): `local:`, where the + * glue supplies the canonical repo dir (git-common-dir parent) so linked + * worktrees of one repo share a key. + * - Empty everything: `local:`. + */ +export function normalizeRemoteIdentity( + remoteUrl: string | null | undefined, + repoKeyBase: string | null, +): string { + if (remoteUrl) { + // Strip embedded credentials from a scheme://user:pass@host authority before + // parsing. parseRemoteHost otherwise returns the username (its host regex + // stops at the first `:`), so a creds-bearing HTTPS clone URL would key to a + // different identity than the bare one. Scheme-gated so the SSH `git@host:` + // form (where `@` is structural, not a credential) is untouched. + const sanitized = remoteUrl.replace(/^([a-z][a-z0-9+.-]*:\/\/)[^/@]+@/i, "$1"); + const host = parseRemoteHost(sanitized); + const path = parseRemoteUrl(sanitized); + if (host && path) { + const normalizedPath = path.replace(/\/+$/, ""); + return `${host.toLowerCase()}/${normalizedPath}`; + } + } + return `local:${repoKeyBase ?? ""}`; +} + +/** + * Merge a global and a repo tripwires config into one. Global rules come first, + * repo rules are appended. Colliding ids are made unique by suffixing `-2`, + * `-3`, … (deterministic). Distinct ids matter only for the internal dedup key + * in {@link evaluateTripwires} — a colliding global+repo id on the same line + * would otherwise collapse two real hits into one. + */ +export function mergeTripwiresConfigs( + global: TripwiresConfig, + repo: TripwiresConfig, +): TripwiresConfig { + const globalRules = Array.isArray(global?.rules) ? global.rules : []; + const repoRules = Array.isArray(repo?.rules) ? repo.rules : []; + + const seen = new Set(); + const rules: TripwireRule[] = []; + + const add = (rule: TripwireRule) => { + let id = rule.id; + let n = 2; + while (seen.has(id)) { + id = `${rule.id}-${n}`; + n++; + } + seen.add(id); + rules.push(id === rule.id ? rule : { ...rule, id }); + }; + + for (const rule of globalRules) add(rule); + for (const rule of repoRules) add(rule); + + return { rules }; +} + +// --------------------------------------------------------------------------- +// Evaluation +// --------------------------------------------------------------------------- + +export interface EvaluateOptions { + /** Repo root (informational; evaluation is pure on the patch text). */ + cwd?: string; +} + +/** True when `content` contains `symbol` as a whole word. */ +function lineMentionsSymbol(content: string, symbol: string): boolean { + // Cheap substring pre-filter: the boundary regex can only match if the symbol + // appears verbatim, so skip the (relatively expensive) regex build + scan for + // the overwhelming majority of lines that don't mention it. This keeps the + // O(rules × changedLines × symbols) symbol pass fast on large diffs. + if (!content.includes(symbol)) return false; + // Escape regex specials in the symbol so e.g. `foo$bar` is literal. + const escaped = symbol.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + // Word-ish boundary: allow `_` and alnum to count as identifier chars. + return new RegExp(`(?(); + + const push = (hit: TripwireHit) => { + const key = `${hit.ruleId}\x00${hit.filePath}\x00${hit.side ?? ""}\x00${hit.line ?? ""}`; + if (seen.has(key)) return; + seen.add(key); + hits.push(hit); + }; + + for (const rule of config.rules) { + const note = rule.note && rule.note.length > 0 ? rule.note : DEFAULT_NOTE; + + for (const span of spans) { + // A rule matches a file if its new path or its old path is in the zone. + // Checking both covers renames (moving out of a zone) and deletions + // (where the new side is /dev/null). + const newReal = span.filePath !== "/dev/null"; + const oldReal = span.oldPath !== "/dev/null"; + const matchesNew = newReal && matchesAnyGlob(span.filePath, rule.globs); + const matchesOld = + oldReal && span.oldPath !== span.filePath && matchesAnyGlob(span.oldPath, rule.globs); + if (!matchesNew && !matchesOld) continue; + + // The human-meaningful path for file-scope hits: prefer the new path, + // falling back to the old path for deletions. + const displayPath = newReal ? span.filePath : span.oldPath; + + const symbols = rule.symbols; + if (!symbols || symbols.length === 0) { + // Globs-only: any touch trips. Anchor to the first changed line, or + // emit a file-scope hit when there are no edited lines (pure rename). + if (span.changed.length > 0) { + const first = span.changed[0]; + push({ + // Always surface the new-side (display) path so the hit joins the + // review UI's file list, which keys files by the new path. An + // old-side anchor (a leading `-` line on a renamed+edited file) + // would otherwise stamp the OLD path and orphan the annotation. + // `side`/`line` still come from the matched line for gutter + // placement. + ruleId: rule.id, + filePath: displayPath, + note, + scope: "line", + side: first.side, + line: first.lineNumber, + }); + } else { + push({ + ruleId: rule.id, + filePath: displayPath, + note, + scope: "file", + }); + } + continue; + } + + // Symbol-narrowed: trip on changed lines (or hunk context) mentioning a + // symbol. Anchor to the matching changed line; hunk-context matches with + // no matching changed line fall back to a file-scope hit. + let anchored = false; + for (const cl of span.changed) { + if (symbols.some((s) => lineMentionsSymbol(cl.content, s))) { + push({ + // Same as the globs-only branch: surface the new-side path so a + // symbol match on an old-side line of a renamed+edited file still + // keys to the file the UI knows about. + ruleId: rule.id, + filePath: displayPath, + note, + scope: "line", + side: cl.side, + line: cl.lineNumber, + }); + anchored = true; + } + } + + if (!anchored) { + const contextHit = span.hunkContexts.some((ctx) => + symbols.some((s) => lineMentionsSymbol(ctx, s)), + ); + if (contextHit) { + push({ + ruleId: rule.id, + filePath: displayPath, + note, + scope: "file", + }); + } + } + } + } + + return hits; + } catch { + // Pure and fail-open: never throw. + return []; + } +} + +// --------------------------------------------------------------------------- +// Annotation mapping +// --------------------------------------------------------------------------- + +/** Source identifier stamped on every tripwire annotation. */ +export const TRIPWIRE_SOURCE = "tripwire"; + +/** + * The review-annotation input shape accepted by `transformReviewInput` in + * external-annotation.ts. Line fields are omitted for file-scope hits. + */ +export interface TripwireReviewAnnotationInput { + source: typeof TRIPWIRE_SOURCE; + type: "concern"; + scope: "line" | "file"; + side?: "old" | "new"; + filePath: string; + lineStart?: number; + lineEnd?: number; + text: string; + author: "Tripwire"; +} + +/** Map a tripwire hit to the external review-annotation input shape. */ +export function tripwireHitToReviewAnnotation(hit: TripwireHit): TripwireReviewAnnotationInput { + const input: TripwireReviewAnnotationInput = { + source: TRIPWIRE_SOURCE, + type: "concern", + scope: hit.scope, + filePath: hit.filePath, + text: hit.note, + author: "Tripwire", + }; + if (hit.scope === "line" && typeof hit.line === "number") { + input.side = hit.side; + input.lineStart = hit.line; + input.lineEnd = hit.line; + } + return input; +} + +// --------------------------------------------------------------------------- +// CLI / non-interactive formatting (list + scan report) +// --------------------------------------------------------------------------- + +/** Everything `formatTripwiresMarkdown` needs to render a two-layer report. */ +export interface TripwiresListView { + /** Hashed project key for the global file (null when no key was derived). */ + globalKey: string | null; + /** User-facing path to the global tripwires file. */ + globalPath: string; + /** Path to the repo tripwires file, or null outside a git repo. */ + repoPath: string | null; + /** Rules from the global layer (pre-merge, for grouped display). */ + globalRules: TripwireRule[]; + /** Rules from the repo layer (pre-merge, for grouped display). */ + repoRules: TripwireRule[]; + /** Optional live evaluation hits to append as a status section. */ + hits?: TripwireHit[]; +} + +/** Escape pipe characters so a value renders inside a markdown table cell. */ +function escapeCell(value: string): string { + return value.replace(/\|/g, "\\|"); +} + +/** Render one layer's rules as a markdown table, or "(none)" when empty. */ +function rulesTable(rules: TripwireRule[]): string { + if (rules.length === 0) return "(none)"; + const header = "| id | globs | symbols | note |\n| --- | --- | --- | --- |"; + const rows = rules.map((r) => { + const globs = r.globs.join(", "); + const symbols = r.symbols && r.symbols.length > 0 ? r.symbols.join(", ") : ""; + const note = r.note ?? ""; + return `| ${escapeCell(r.id)} | ${escapeCell(globs)} | ${escapeCell(symbols)} | ${escapeCell(note)} |`; + }); + return [header, ...rows].join("\n"); +} + +/** + * Render a two-layer tripwires view as markdown: a Global section and a Repo + * section, each a table (or "(none)"), plus an optional live-status section that + * marks which rule ids tripped. + */ +export function formatTripwiresMarkdown(view: TripwiresListView): string { + const sections: string[] = []; + + sections.push(`## Global (${view.globalPath})`); + sections.push(rulesTable(view.globalRules)); + + const repoLabel = view.repoPath ?? ".plannotator/tripwires.json"; + sections.push(`## Repo (${repoLabel})`); + sections.push(rulesTable(view.repoRules)); + + if (view.hits) { + sections.push("## Live status"); + if (view.hits.length === 0) { + sections.push("No tripwires tripped."); + } else { + const trippedIds = new Set(view.hits.map((h) => h.ruleId)); + // An in-place edit touching a tripwired symbol trips on both the old (`-`) + // and new (`+`) side at the same line number; evaluateTripwires keeps both + // (the review UI's old/new gutters need them), but to a reader they are the + // same location. Collapse status lines that share (ruleId, where, note). + const seen = new Set(); + const lines: string[] = []; + for (const h of view.hits) { + const where = h.scope === "line" && typeof h.line === "number" + ? `${h.filePath}:${h.line}` + : h.filePath; + const dedupKey = `${h.ruleId}\x00${where}\x00${h.note}`; + if (seen.has(dedupKey)) continue; + seen.add(dedupKey); + lines.push(`- \`${h.ruleId}\` tripped — ${escapeCell(where)} — ${escapeCell(h.note)}`); + } + sections.push(`Tripped: ${[...trippedIds].map((id) => `\`${id}\``).join(", ")}`); + sections.push(lines.join("\n")); + } + } + + return sections.join("\n\n"); +} + +/** + * Build the agent-facing instruction for `--add-tripwire ` and + * `tripwires add `. The user describes the sensitive region in + * natural language; the returned prompt tells the agent how to turn that into + * a concrete rule and write it. It never writes the file itself — the command + * output IS the instruction. + */ +export function buildAddTripwirePrompt(args: { + /** The user's natural-language description of what to protect. */ + description: string; + /** Resolved absolute path of the global per-project file, when known. */ + globalPath?: string; + /** Resolved path of the repo-committed file, when inside a git repo. */ + repoPath?: string; +}): string { + const globalPath = + args.globalPath ?? "~/.plannotator/tripwires/.json"; + const repoPath = args.repoPath ?? ".plannotator/tripwires.json"; + + return [ + "Add a tripwire (slop-free zone) so future code-review diffs touching this region are flagged.", + `The user wants to protect:\n\n> ${args.description}`, + [ + "Steps:", + "1. Explore the repository to resolve that description into concrete file globs (prefer the narrowest globs that cover it). If the user named functions, classes, or other symbols, capture them as `symbols` entries; otherwise omit `symbols`.", + "2. Write the rule into the GLOBAL tripwires file (default; private to this machine):", + ` ${globalPath}`, + ` Only use the repo-committed file (${repoPath}) if the user explicitly asked for a team-shared / committed tripwire.`, + "3. If the file does not exist, create it as `{ \"rules\": [] }`. Append the new rule — never remove or rewrite existing rules. Give it a unique `id`.", + "4. Run `plannotator tripwires validate` to confirm the config parses, then show the user the final rule and which file it landed in.", + ].join("\n"), + [ + "Schema for one rule:", + "```json", + JSON.stringify( + { id: "money-path", globs: ["src/billing/**"], symbols: ["chargeCustomer"], note: "Money path — review carefully" }, + null, + 2, + ), + "```", + "`globs` (required): repo-relative; `*` matches within a path segment, `**` across segments, `?` one character. `symbols` (optional): plain substrings — a rule with symbols only trips when a change touches that symbol. `note` (optional): shown on the warning in the review UI.", + "Semantics: any change that touches a matched file or symbol — adding, editing, deleting, renaming — trips the wire.", + ].join("\n"), + "This is an instruction to apply — the command did not write any file.", + ].join("\n\n"); +} + +// --------------------------------------------------------------------------- +// Rule writing (pure JSON manipulation — fs lives in runtime glue) +// --------------------------------------------------------------------------- + +export type AppendRuleResult = + | { ok: true; json: string; rule: TripwireRule } + | { ok: false; error: string }; + +/** + * Append a rule to a tripwires JSON document, preserving everything already + * there (existing rules verbatim, unknown top-level keys). Returns the new + * document text, or a refusal when the existing content cannot be parsed — + * an explicit write must never clobber a file the user could still repair. + * Missing/empty input starts a fresh `{ "rules": [] }` document. + */ +export function appendRuleToTripwiresJson( + raw: string | null, + rule: { globs: string[]; symbols?: string[]; note?: string; id?: string }, +): AppendRuleResult { + if (rule.globs.length === 0 || rule.globs.some((g) => g.trim().length === 0)) { + return { ok: false, error: "a rule needs at least one non-empty glob" }; + } + + let doc: Record; + if (raw === null || raw.trim().length === 0) { + doc = { rules: [] }; + } else { + try { + const parsed: unknown = JSON.parse(raw); + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + return { ok: false, error: "existing config is not a JSON object — fix it first (plannotator tripwires validate)" }; + } + doc = parsed as Record; + } catch { + return { ok: false, error: "existing config is not valid JSON — refusing to overwrite it (plannotator tripwires validate)" }; + } + } + + const rules = Array.isArray(doc.rules) ? (doc.rules as unknown[]) : []; + doc.rules = rules; + + const existingIds = new Set( + rules + .map((r) => (typeof r === "object" && r !== null ? (r as Record).id : undefined)) + .filter((id): id is string => typeof id === "string"), + ); + + let id = rule.id?.trim(); + if (id) { + if (existingIds.has(id)) { + return { ok: false, error: `a rule with id "${id}" already exists` }; + } + } else { + let n = rules.length + 1; + while (existingIds.has(`rule-${n}`)) n++; + id = `rule-${n}`; + } + + const newRule: TripwireRule = { + id, + globs: rule.globs, + ...(rule.symbols && rule.symbols.length > 0 ? { symbols: rule.symbols } : {}), + ...(rule.note && rule.note.trim().length > 0 ? { note: rule.note.trim() } : {}), + }; + rules.push(newRule); + + return { ok: true, json: JSON.stringify(doc, null, 2) + "\n", rule: newRule }; +} diff --git a/packages/ui/types.ts b/packages/ui/types.ts index 26c1da740..ef927d646 100644 --- a/packages/ui/types.ts +++ b/packages/ui/types.ts @@ -153,8 +153,9 @@ export interface DiffAnnotationMetadata { reasoning?: string; conventionalLabel?: ConventionalLabel; decorations?: ConventionalDecoration[]; + source?: string; // External tool identifier (e.g., "tripwire") — set when annotation comes from external API // AI marker fields (set when kind === 'ai-marker') - kind?: 'annotation' | 'ai-marker'; + kind?: 'annotation' | 'ai-marker' | 'tripwire'; questionId?: string; promptPreview?: string; hasResponse?: boolean;