diff --git a/.claude/skills/codebase-design/DEEPENING.md b/.claude/skills/codebase-design/DEEPENING.md new file mode 100644 index 0000000000..3938457b88 --- /dev/null +++ b/.claude/skills/codebase-design/DEEPENING.md @@ -0,0 +1,37 @@ +# Deepening + +How to deepen a cluster of shallow modules safely, given its dependencies. Assumes the vocabulary in [SKILL.md](SKILL.md) — **module**, **interface**, **seam**, **adapter**. + +## Dependency categories + +When assessing a candidate for deepening, classify its dependencies. The category determines how the deepened module is tested across its seam. + +### 1. In-process + +Pure computation, in-memory state, no I/O. Always deepenable — merge the modules and test through the new interface directly. No adapter needed. + +### 2. Local-substitutable + +Dependencies that have local test stand-ins (PGLite for Postgres, in-memory filesystem). Deepenable if the stand-in exists. The deepened module is tested with the stand-in running in the test suite. The seam is internal; no port at the module's external interface. + +### 3. Remote but owned (Ports & Adapters) + +Your own services across a network boundary (microservices, internal APIs). Define a **port** (interface) at the seam. The deep module owns the logic; the transport is injected as an **adapter**. Tests use an in-memory adapter. Production uses an HTTP/gRPC/queue adapter. + +Recommendation shape: *"Define a port at the seam, implement an HTTP adapter for production and an in-memory adapter for testing, so the logic sits in one deep module even though it's deployed across a network."* + +### 4. True external (Mock) + +Third-party services (Stripe, Twilio, etc.) you don't control. The deepened module takes the external dependency as an injected port; tests provide a mock adapter. + +## Seam discipline + +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a port unless at least two adapters are justified (typically production + test). A single-adapter seam is just indirection. +- **Internal seams vs external seams.** A deep module can have internal seams (private to its implementation, used by its own tests) as well as the external seam at its interface. Don't expose internal seams through the interface just because tests use them. + +## Testing strategy: replace, don't layer + +- Old unit tests on shallow modules become waste once tests at the deepened module's interface exist — delete them. +- Write new tests at the deepened module's interface. The **interface is the test surface**. +- Tests assert on observable outcomes through the interface, not internal state. +- Tests should survive internal refactors — they describe behaviour, not implementation. If a test has to change when the implementation changes, it's testing past the interface. diff --git a/.claude/skills/codebase-design/DESIGN-IT-TWICE.md b/.claude/skills/codebase-design/DESIGN-IT-TWICE.md new file mode 100644 index 0000000000..8419ad6fa9 --- /dev/null +++ b/.claude/skills/codebase-design/DESIGN-IT-TWICE.md @@ -0,0 +1,44 @@ +# Design It Twice + +When the user wants to explore alternative interfaces for a chosen deepening candidate, use this parallel sub-agent pattern. Based on "Design It Twice" (Ousterhout) — your first idea is unlikely to be the best. + +Uses the vocabulary in [SKILL.md](SKILL.md) — **module**, **interface**, **seam**, **adapter**, **leverage**. + +## Process + +### 1. Frame the problem space + +Before spawning sub-agents, write a user-facing explanation of the problem space for the chosen candidate: + +- The constraints any new interface would need to satisfy +- The dependencies it would rely on, and which category they fall into (see [DEEPENING.md](DEEPENING.md)) +- A rough illustrative code sketch to ground the constraints — not a proposal, just a way to make the constraints concrete + +Show this to the user, then immediately proceed to Step 2. The user reads and thinks while the sub-agents work in parallel. + +### 2. Spawn sub-agents + +Spawn 3+ sub-agents in parallel. Each must produce a **radically different** interface for the deepened module. + +Prompt each sub-agent with a separate technical brief (file paths, coupling details, dependency category from [DEEPENING.md](DEEPENING.md), what sits behind the seam). The brief is independent of the user-facing problem-space explanation in Step 1. Give each agent a different design constraint: + +- Agent 1: "Minimize the interface — aim for 1–3 entry points max. Maximise leverage per entry point." +- Agent 2: "Maximise flexibility — support many use cases and extension." +- Agent 3: "Optimise for the most common caller — make the default case trivial." +- Agent 4 (if applicable): "Design around ports & adapters for cross-seam dependencies." + +Include both [SKILL.md](SKILL.md) vocabulary and CONTEXT.md vocabulary in the brief so each sub-agent names things consistently with the architecture language and the project's domain language. + +Each sub-agent outputs: + +1. Interface (types, methods, params — plus invariants, ordering, error modes) +2. Usage example showing how callers use it +3. What the implementation hides behind the seam +4. Dependency strategy and adapters (see [DEEPENING.md](DEEPENING.md)) +5. Trade-offs — where leverage is high, where it's thin + +### 3. Present and compare + +Present designs sequentially so the user can absorb each one, then compare them in prose. Contrast by **depth** (leverage at the interface), **locality** (where change concentrates), and **seam placement**. + +After comparing, give your own recommendation: which design you think is strongest and why. If elements from different designs would combine well, propose a hybrid. Be opinionated — the user wants a strong read, not a menu. diff --git a/.claude/skills/codebase-design/LICENSE b/.claude/skills/codebase-design/LICENSE new file mode 100644 index 0000000000..f1dd2c0910 --- /dev/null +++ b/.claude/skills/codebase-design/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Matt Pocock + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/.claude/skills/codebase-design/SKILL.md b/.claude/skills/codebase-design/SKILL.md new file mode 100644 index 0000000000..16620c2452 --- /dev/null +++ b/.claude/skills/codebase-design/SKILL.md @@ -0,0 +1,114 @@ +--- +name: codebase-design +description: Shared vocabulary for designing deep modules. Use when the user wants to design or improve a module's interface, find deepening opportunities, decide where a seam goes, make code more testable or AI-navigable, or when another skill needs the deep-module vocabulary. +--- + +# Codebase Design + +Design **deep modules**: a lot of behaviour behind a small interface, placed at a clean seam, testable through that interface. Use this language and these principles wherever code is being designed or restructured. The aim is leverage for callers, locality for maintainers, and testability for everyone. + +## Glossary + +Use these terms exactly — don't substitute "component," "service," "API," or "boundary." Consistent language is the whole point. + +**Module** — anything with an interface and an implementation. Deliberately scale-agnostic: a function, class, package, or tier-spanning slice. _Avoid_: unit, component, service. + +**Interface** — everything a caller must know to use the module correctly: the type signature, but also invariants, ordering constraints, error modes, required configuration, and performance characteristics. _Avoid_: API, signature (too narrow — they refer only to the type-level surface). + +**Implementation** — what's inside a module, its body of code. Distinct from **Adapter**: a thing can be a small adapter with a large implementation (a Postgres repo) or a large adapter with a small implementation (an in-memory fake). Reach for "adapter" when the seam is the topic; "implementation" otherwise. + +**Depth** — leverage at the interface: the amount of behaviour a caller (or test) can exercise per unit of interface they have to learn. A module is **deep** when a large amount of behaviour sits behind a small interface, **shallow** when the interface is nearly as complex as the implementation. + +**Seam** _(Michael Feathers)_ — a place where you can alter behaviour without editing in that place; the *location* at which a module's interface lives. Where to put the seam is its own design decision, distinct from what goes behind it. _Avoid_: boundary (overloaded with DDD's bounded context). + +**Adapter** — a concrete thing that satisfies an interface at a seam. Describes *role* (what slot it fills), not substance (what's inside). + +**Leverage** — what callers get from depth: more capability per unit of interface they learn. One implementation pays back across N call sites and M tests. + +**Locality** — what maintainers get from depth: change, bugs, knowledge, and verification concentrate in one place rather than spreading across callers. Fix once, fixed everywhere. + +## Deep vs shallow + +**Deep module** = small interface + lots of implementation: + +``` +┌─────────────────────┐ +│ Small Interface │ ← Few methods, simple params +├─────────────────────┤ +│ │ +│ Deep Implementation│ ← Complex logic hidden +│ │ +└─────────────────────┘ +``` + +**Shallow module** = large interface + little implementation (avoid): + +``` +┌─────────────────────────────────┐ +│ Large Interface │ ← Many methods, complex params +├─────────────────────────────────┤ +│ Thin Implementation │ ← Just passes through +└─────────────────────────────────┘ +``` + +When designing an interface, ask: + +- Can I reduce the number of methods? +- Can I simplify the parameters? +- Can I hide more complexity inside? + +## Principles + +- **Depth is a property of the interface, not the implementation.** A deep module can be internally composed of small, mockable, swappable parts — they just aren't part of the interface. A module can have **internal seams** (private to its implementation, used by its own tests) as well as the **external seam** at its interface. +- **The deletion test.** Imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep. +- **The interface is the test surface.** Callers and tests cross the same seam. If you want to test *past* the interface, the module is probably the wrong shape. +- **One adapter means a hypothetical seam. Two adapters means a real one.** Don't introduce a seam unless something actually varies across it. + +## Designing for testability + +Good interfaces make testing natural: + +1. **Accept dependencies, don't create them.** + + ```typescript + // Testable + function processOrder(order, paymentGateway) {} + + // Hard to test + function processOrder(order) { + const gateway = new StripeGateway(); + } + ``` + +2. **Return results, don't produce side effects.** + + ```typescript + // Testable + function calculateDiscount(cart): Discount {} + + // Hard to test + function applyDiscount(cart): void { + cart.total -= discount; + } + ``` + +3. **Small surface area.** Fewer methods = fewer tests needed. Fewer params = simpler test setup. + +## Relationships + +- A **Module** has exactly one **Interface** (the surface it presents to callers and tests). +- **Depth** is a property of a **Module**, measured against its **Interface**. +- A **Seam** is where a **Module**'s **Interface** lives. +- An **Adapter** sits at a **Seam** and satisfies the **Interface**. +- **Depth** produces **Leverage** for callers and **Locality** for maintainers. + +## Rejected framings + +- **Depth as ratio of implementation-lines to interface-lines** (Ousterhout): rewards padding the implementation. We use depth-as-leverage instead. +- **"Interface" as the TypeScript `interface` keyword or a class's public methods**: too narrow — interface here includes every fact a caller must know. +- **"Boundary"**: overloaded with DDD's bounded context. Say **seam** or **interface**. + +## Going deeper + +- **Deepening a cluster given its dependencies** — see [DEEPENING.md](DEEPENING.md): dependency categories, seam discipline, and replace-don't-layer testing. +- **Exploring alternative interfaces** — see [DESIGN-IT-TWICE.md](DESIGN-IT-TWICE.md): spin up parallel sub-agents to design the interface several radically different ways, then compare on depth, locality, and seam placement. diff --git a/.claude/skills/codebase-design/agents/openai.yaml b/.claude/skills/codebase-design/agents/openai.yaml new file mode 100644 index 0000000000..3180715edb --- /dev/null +++ b/.claude/skills/codebase-design/agents/openai.yaml @@ -0,0 +1,3 @@ +interface: + display_name: "Codebase Design" + short_description: "Vocabulary for deep-module design" diff --git a/.claude/skills/improve-codebase-architecture/HTML-REPORT.md b/.claude/skills/improve-codebase-architecture/HTML-REPORT.md new file mode 100644 index 0000000000..17f6d2c7b8 --- /dev/null +++ b/.claude/skills/improve-codebase-architecture/HTML-REPORT.md @@ -0,0 +1,123 @@ +# HTML Report Format + +The architectural review is rendered as a single self-contained HTML file in the OS temp directory. Tailwind and Mermaid both come from CDNs. Mermaid handles graph-shaped diagrams reliably; hand-built divs and inline SVG handle the more editorial visuals (mass diagrams, cross-sections). Mix the two — don't lean on Mermaid for everything, it'll start to look generic. + +## Scaffold + +```html + + + + + Architecture review — {{repo name}} + + + + + +
+
...
+
...
+
...
+
+ + +``` + +## Header + +Repo name, date, and a compact legend: solid box = module, dashed line = seam, red arrow = leakage, thick dark box = deep module. No introduction paragraph — straight into the candidates. + +## Candidate card + +The diagrams carry the weight. Prose is sparse, plain, and uses the glossary terms (from the `/codebase-design` skill) without ceremony. + +Each candidate is one `
`: + +- **Title** — short, names the deepening (e.g. "Collapse the Order intake pipeline"). +- **Badge row** — recommendation strength (`Strong` = emerald, `Worth exploring` = amber, `Speculative` = slate), plus a tag for the dependency category (`in-process`, `local-substitutable`, `ports & adapters`, `mock`). +- **Files** — monospaced list, `font-mono text-sm`. +- **Before / After diagram** — the centrepiece. Two columns, side by side. See patterns below. +- **Problem** — one sentence. What hurts. +- **Solution** — one sentence. What changes. +- **Wins** — bullets, ≤6 words each. e.g. "Tests hit one interface", "Pricing logic stops leaking", "Delete 4 shallow wrappers". +- **ADR callout** (if applicable) — one line in an amber-tinted box. + +No paragraphs of explanation. If the diagram needs a paragraph to be understood, redraw the diagram. + +## Diagram patterns + +Pick the pattern that fits the candidate. Mix them. Don't make every diagram look the same — variety is part of the point. + +### Mermaid graph (the workhorse for dependencies / call flow) + +Use a Mermaid `flowchart` or `graph` when the point is "X calls Y calls Z, and look at the mess." Wrap it in a Tailwind-styled card so it doesn't feel parachuted in. Style with classDef to colour leakage edges red and the deep module dark. Sequence diagrams work well for "before: 6 round-trips; after: 1." + +```html +
+
+    flowchart LR
+      A[OrderHandler] --> B[OrderValidator]
+      B --> C[OrderRepo]
+      C -.leak.-> D[PricingClient]
+      classDef leak stroke:#dc2626,stroke-width:2px;
+      class C,D leak
+  
+
+``` + +### Hand-built boxes-and-arrows (when Mermaid's layout fights you) + +Modules as `
`s with borders and labels. Arrows as inline SVG `` or `` elements positioned absolutely over a relative container. Reach for this when you want the "after" diagram to feel like one thick-bordered deep module with greyed-out internals — Mermaid won't render that with the right weight. + +### Cross-section (good for layered shallowness) + +Stack horizontal bands (`h-12 border-l-4`) to show layers a call passes through. Before: 6 thin layers each doing nothing. After: 1 thick band labelled with the consolidated responsibility. + +### Mass diagram (good for "interface as wide as implementation") + +Two rectangles per module — one for interface surface area, one for implementation. Before: interface rectangle is nearly as tall as the implementation rectangle (shallow). After: interface rectangle is short, implementation rectangle is tall (deep). + +### Call-graph collapse + +Before: a tree of function calls rendered as nested boxes. After: the same tree collapsed into one box, with the now-internal calls shown faded inside it. + +## Style guidance + +- Lean editorial, not corporate-dashboard. Generous whitespace. Serif optional for headings (`font-serif` works well with stone/slate). +- Colour sparingly: one accent (emerald or indigo) plus red for leakage and amber for warnings. +- Keep diagrams ~320px tall so before/after sits comfortably side by side without scrolling. +- Use `text-xs uppercase tracking-wider` for module labels inside diagrams — they should read as schematic, not as UI. +- The only scripts are the Tailwind CDN and the Mermaid ESM import. The report is otherwise static — no app code, no interactivity beyond Mermaid's own rendering. + +## Top recommendation section + +One larger card. Candidate name, one sentence on why, anchor link to its card. That's it. + +## Tone + +Plain English, concise — but the architectural nouns and verbs come straight from the `/codebase-design` skill. Concision is not an excuse to drift. + +**Use exactly:** module, interface, implementation, depth, deep, shallow, seam, adapter, leverage, locality. + +**Never substitute:** component, service, unit (for module) · API, signature (for interface) · boundary (for seam) · layer, wrapper (for module, when you mean module). + +**Phrasings that fit the style:** + +- "Order intake module is shallow — interface nearly matches the implementation." +- "Pricing leaks across the seam." +- "Deepen: one interface, one place to test." +- "Two adapters justify the seam: HTTP in prod, in-memory in tests." + +**Wins bullets** name the gain in glossary terms: *"locality: bugs concentrate in one module"*, *"leverage: one interface, N call sites"*, *"interface shrinks; implementation absorbs the wrappers"*. Don't write *"easier to maintain"* or *"cleaner code"* — those terms aren't in the glossary and don't earn their place. + +No hedging, no throat-clearing, no "it's worth noting that…". If a sentence could be a bullet, make it a bullet. If a bullet could be cut, cut it. If a term isn't in the `/codebase-design` glossary, reach for one that is before inventing a new one. diff --git a/.claude/skills/improve-codebase-architecture/LICENSE b/.claude/skills/improve-codebase-architecture/LICENSE new file mode 100644 index 0000000000..f1dd2c0910 --- /dev/null +++ b/.claude/skills/improve-codebase-architecture/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 Matt Pocock + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/.claude/skills/improve-codebase-architecture/SKILL.md b/.claude/skills/improve-codebase-architecture/SKILL.md new file mode 100644 index 0000000000..529761a3a0 --- /dev/null +++ b/.claude/skills/improve-codebase-architecture/SKILL.md @@ -0,0 +1,71 @@ +--- +name: improve-codebase-architecture +description: Scan a codebase for deepening opportunities, present them as a visual HTML report, then grill through whichever one you pick. +disable-model-invocation: true +--- + +# Improve Codebase Architecture + +Surface architectural friction and propose **deepening opportunities** — refactors that turn shallow modules into deep ones. The aim is testability and AI-navigability. + +This command is _informed_ by the project's domain model and built on a shared design vocabulary: + +- Run the `/codebase-design` skill for the architecture vocabulary (**module**, **interface**, **depth**, **seam**, **adapter**, **leverage**, **locality**) and its principles (the deletion test, "the interface is the test surface", "one adapter = hypothetical seam, two = real"). Use these terms exactly in every suggestion — don't drift into "component," "service," "API," or "boundary." +- The domain language in `CONTEXT.md` gives names to good seams; ADRs in `docs/adr/` record decisions this command should not re-litigate. + +## Process + +### 1. Explore + +**Scope before you scan — YAGNI.** Deepening a module pays off by making future changes to it easier, so put extra weight on the parts of the codebase that have recently changed. Decide *where* to look before you look: + +- If the user named a direction — a module, a subsystem, a pain point — take it, and skip the inference below. +- Otherwise, walk back a good stretch of the commit history (`git log --oneline`) to find the codebase's hot spots — the files and areas that keep coming up — and let those paths pull your attention first. If the changes are scattered with no clear hot spot, widen the net. + +Read the project's domain glossary (`CONTEXT.md`) and any ADRs in the area you're touching first. + +Then spawn a sub-agent to walk the codebase. Don't follow rigid heuristics — explore organically and note where you experience friction: + +- Where does understanding one concept require bouncing between many small modules? +- Where are modules **shallow** — interface nearly as complex as the implementation? +- Where have pure functions been extracted just for testability, but the real bugs hide in how they're called (no **locality**)? +- Where do tightly-coupled modules leak across their seams? +- Which parts of the codebase are untested, or hard to test through their current interface? + +Apply the **deletion test** to anything you suspect is shallow: would deleting it concentrate complexity, or just move it? A "yes, concentrates" is the signal you want. + +### 2. Present candidates as an HTML report + +Write a self-contained HTML file to the OS temp directory so nothing lands in the repo. Resolve the temp dir from `$TMPDIR`, falling back to `/tmp` (or `%TEMP%` on Windows), and write to `/architecture-review-.html` so each run gets a fresh file. Open it for the user — `xdg-open ` on Linux, `open ` on macOS, `start ` on Windows — and tell them the absolute path. + +The report uses **Tailwind via CDN** for layout and styling, and **Mermaid via CDN** for diagrams where a graph/flow/sequence reliably communicates the structure. Mix Mermaid with hand-crafted CSS/SVG visuals — use Mermaid when relationships are graph-shaped (call graphs, dependencies, sequences), and hand-built divs/SVG when you want something more editorial (mass diagrams, cross-sections, collapse animations). Each candidate gets a **before/after visualisation**. Be visual. + +For each candidate, render a card with: + +- **Files** — which files/modules are involved +- **Problem** — why the current architecture is causing friction +- **Solution** — plain English description of what would change +- **Benefits** — explained in terms of locality and leverage, and how tests would improve +- **Before / After diagram** — side-by-side, custom-drawn, illustrating the shallowness and the deepening +- **Recommendation strength** — one of `Strong`, `Worth exploring`, `Speculative`, rendered as a badge + +End the report with a **Top recommendation** section: which candidate you'd tackle first and why. + +**Use CONTEXT.md vocabulary for the domain, and the `/codebase-design` vocabulary for the architecture.** If `CONTEXT.md` defines "Order," talk about "the Order intake module" — not "the FooBarHandler," and not "the Order service." + +**ADR conflicts**: if a candidate contradicts an existing ADR, only surface it when the friction is real enough to warrant revisiting the ADR. Mark it clearly in the card (e.g. a warning callout: _"contradicts ADR-0007 — but worth reopening because…"_). Don't list every theoretical refactor an ADR forbids. + +See [HTML-REPORT.md](HTML-REPORT.md) for the full HTML scaffold, diagram patterns, and styling guidance. + +Do NOT propose interfaces yet. After the file is written, ask the user: "Which of these would you like to explore?" + +### 3. Grilling loop + +Once the user picks a candidate, run the `/grilling` skill to walk the decision tree with them — constraints, dependencies, the shape of the deepened module, what sits behind the seam, what tests survive. + +Side effects happen inline as decisions crystallize — run the `/domain-modeling` skill to keep the domain model current as you go: + +- **Naming a deepened module after a concept not in `CONTEXT.md`?** Add the term to `CONTEXT.md`. Create the file lazily if it doesn't exist. +- **Sharpening a fuzzy term during the conversation?** Update `CONTEXT.md` right there. +- **User rejects the candidate with a load-bearing reason?** Offer an ADR, framed as: _"Want me to record this as an ADR so future architecture reviews don't re-suggest it?"_ Only offer when the reason would actually be needed by a future explorer to avoid re-suggesting the same thing — skip ephemeral reasons ("not worth it right now") and self-evident ones. +- **Want to explore alternative interfaces for the deepened module?** Run the `/codebase-design` skill and use its design-it-twice parallel sub-agent pattern. diff --git a/.claude/skills/improve-codebase-architecture/agents/openai.yaml b/.claude/skills/improve-codebase-architecture/agents/openai.yaml new file mode 100644 index 0000000000..706fdca096 --- /dev/null +++ b/.claude/skills/improve-codebase-architecture/agents/openai.yaml @@ -0,0 +1,5 @@ +interface: + display_name: "Improve Codebase Architecture" + short_description: "Find and grill architecture improvements" +policy: + allow_implicit_invocation: false diff --git a/.env.example b/.env.example index 9a95186e5a..75c52ae5f7 100644 --- a/.env.example +++ b/.env.example @@ -136,6 +136,18 @@ NODE_MAX_OLD_SPACE_SIZE=6144 # with the Skills capability enabled. Defaults to project root ./skill. # DEPLOYMENT_SKILLS_DIR=./skill +# Agent Plugins packages (skills + MCP servers + hooks) are loaded at startup +# from this directory; each child directory is one plugin. Defaults to ./plugin. +# DEPLOYMENT_PLUGINS_DIR=./plugin +# DEPLOYMENT_PLUGIN_DATA_DIR=./data/plugins + +# Opt-in: execute `command` hook handlers declared by installed plugins +# (ai.librechat/hooks/hooks.json). Commands run as child processes on the API +# host with a minimal environment — only enable for plugins you trust, the +# same trust level as toolApproval hook modules. Off by default: hook +# documents are parsed but never executed. +# DEPLOYMENT_PLUGIN_HOOKS=true + #==================# # Langfuse Tracing # #==================# diff --git a/api/server/controllers/agents/client.js b/api/server/controllers/agents/client.js index 3ed4a0cc2b..be892bde64 100644 --- a/api/server/controllers/agents/client.js +++ b/api/server/controllers/agents/client.js @@ -3198,6 +3198,9 @@ class AgentClient extends BaseClient { // The resumed run can pause AGAIN (another tool, a follow-up question), and this // controller owns that lifecycle, so it must keep the HITL wiring on the rebuilt run. hitlCapable: true, + // Plugin SessionStart hooks match on the lifecycle source; a rebuilt run is a + // resume, not a fresh startup. + sessionStartSource: 'resume', toolInputValidationErrors: this.toolInputValidationErrors, // Steering stays live across a pause/resume cycle: steers queued while // the resumed segment runs drain at its tool-batch boundaries. diff --git a/api/server/index.js b/api/server/index.js index 33473a8538..b6a483d7b7 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -29,6 +29,10 @@ const { initializeDeploymentSkills, initializeDeploymentPlugins, getDeploymentPluginSkills, + getDeploymentPluginHookCapabilities, + registerDeploymentPluginHooks, + hasDeploymentPluginHooks, + setPluginHookSource, loadToolApprovalHooks, maybeInjectQueryDevtoolsBootstrap, preAuthTenantMiddleware, @@ -155,7 +159,18 @@ const startServer = async () => { const appConfig = await getAppConfig({ baseOnly: true }); initializeFileStorage(appConfig); const projectRoot = path.resolve(__dirname, '../..'); - await initializeDeploymentPlugins({ projectRoot }); + // Plugin hooks execute only when the operator opts in via DEPLOYMENT_PLUGIN_HOOKS; + // without it, declared hook documents load as parsed-but-inert with a warning. + await initializeDeploymentPlugins({ + projectRoot, + hookCapabilities: getDeploymentPluginHookCapabilities(), + }); + // Hand the run seam its plugin-hook source without a packages/api-internal + // agents -> plugins import (see agents/hooks/source.ts). + setPluginHookSource({ + hasHooks: hasDeploymentPluginHooks, + register: registerDeploymentPluginHooks, + }); await initializeDeploymentSkills({ projectRoot, additionalSkills: getDeploymentPluginSkills(), diff --git a/packages/api/src/agents/hooks/compatibility.spec.ts b/packages/api/src/agents/hooks/compatibility.spec.ts index 99aa5c744a..5c97c3d5e9 100644 --- a/packages/api/src/agents/hooks/compatibility.spec.ts +++ b/packages/api/src/agents/hooks/compatibility.spec.ts @@ -36,6 +36,77 @@ describe('planPluginHooks', () => { ]); }); + test('gives matcherless tool declarations the document namespace', () => { + const plan = planPluginHooks( + document({ + PreToolUse: [{ hooks: [{ type: 'command', command: 'audit' }] }], + PostToolBatch: [{ hooks: [{ type: 'command', command: 'record' }] }], + Stop: [{ hooks: [{ type: 'command', command: 'verify' }] }], + }), + { + handlerTypes: new Set(['command']), + translateMatcher: ({ matcher }: { matcher: string }) => matcher, + toPluginToolName: ({ toolName }) => toolName, + }, + ); + + expect(plan.summary.ready).toBe(3); + expect( + plan.entries.map(({ targetEvent, requiresToolNameTranslation, translatedToolNames }) => ({ + targetEvent, + requiresToolNameTranslation, + translatedToolNames, + })), + ).toEqual([ + { + targetEvent: 'PreToolUse', + requiresToolNameTranslation: true, + translatedToolNames: undefined, + }, + { + targetEvent: 'PostToolBatch', + requiresToolNameTranslation: true, + translatedToolNames: undefined, + }, + { + targetEvent: 'Stop', + requiresToolNameTranslation: undefined, + translatedToolNames: undefined, + }, + ]); + }); + + test('marks handlers unsupported when the executor rejects them for the host', () => { + const plan = planPluginHooks( + document({ + PreToolUse: [ + { matcher: '^write_file$', hooks: [{ type: 'command', command: 'check' }] }, + { + matcher: '^read_file$', + hooks: [{ type: 'command', command: 'check', commandWindows: 'check.ps1' }], + }, + ], + }), + { + handlerTypes: new Set(['command']), + translateMatcher: ({ matcher }: { matcher: string }) => matcher, + supportsHandler: ({ handler }) => + handler.commandWindows === undefined ? 'host requires commandWindows' : undefined, + }, + ); + + expect(plan.summary).toEqual({ declared: 2, ready: 1, unsupported: 1 }); + expect(plan.entries[0].status).toBe('unsupported'); + expect(plan.entries[0].issues).toEqual([ + expect.objectContaining({ + code: 'unsupported_handler', + severity: 'error', + message: 'host requires commandWindows', + }), + ]); + expect(plan.entries[1].status).toBe('ready'); + }); + test('maps SessionStart to RunStart with an explicit lifecycle warning', () => { const plan = planPluginHooks( document({ @@ -143,6 +214,31 @@ describe('planPluginHooks', () => { ); }); + test('fails closed for SessionStart clear matchers, which no run path emits', () => { + const plan = planPluginHooks( + document({ + SessionStart: [ + { matcher: 'resume|clear', hooks: [{ type: 'command', command: 'reload-context' }] }, + ], + }), + { handlerTypes: new Set(['command']), sessionLifecycle: true }, + ); + + expect(plan.summary).toEqual({ declared: 1, ready: 0, unsupported: 1 }); + expect(plan.entries[0]).toEqual( + expect.objectContaining({ + status: 'unsupported', + issues: expect.arrayContaining([ + expect.objectContaining({ + code: 'unsupported_session_source', + severity: 'error', + message: expect.stringContaining('"clear"'), + }), + ]), + }), + ); + }); + test('keeps wildcard SessionStart ready while reporting compact as filtered', () => { const plan = planPluginHooks( document({ diff --git a/packages/api/src/agents/hooks/compatibility.ts b/packages/api/src/agents/hooks/compatibility.ts index f7fb4a5e50..0a477c53c8 100644 --- a/packages/api/src/agents/hooks/compatibility.ts +++ b/packages/api/src/agents/hooks/compatibility.ts @@ -38,6 +38,9 @@ const TOOL_NAME_EVENTS = new Set([ 'PermissionDenied', ]); +/** Events whose payloads carry tool names/inputs, including the matcherless batch event. */ +const TOOL_PAYLOAD_EVENTS = new Set([...TOOL_NAME_EVENTS, 'PostToolBatch']); + const GENERAL_EXACT_MATCHER = /^[-A-Za-z0-9_,| ]+$/; const NARROW_EXACT_MATCHER = /^[A-Za-z0-9_|]+$/; const NARROW_EXACT_MATCHER_EVENTS = new Set(['FileChanged', 'StopFailure']); @@ -84,6 +87,13 @@ export interface PluginHookMatcherTranslationResult { /** Runtime pattern; translations of Claude exact matchers are whole-string anchored. */ matcher: string; requiresToolNameTranslation?: boolean; + /** + * Runtime tool names the translation produced. Reverse payload translation + * applies per invocation to exactly these names, so a mixed matcher like + * `Bash|create_file` keeps native payloads for its natively-authored + * alternative. Omitted, translation applies to the whole declaration. + */ + translatedToolNames?: string[]; } export interface PluginHookToolNameTranslation { @@ -92,6 +102,19 @@ export interface PluginHookToolNameTranslation { toolName: string; } +export interface PluginHookToolInputTranslation { + sourceEvent: string; + targetEvent: HookEvent; + /** LibreChat runtime name of the invoked tool. */ + toolName: string; + toolInput: Record; +} + +export interface PluginHookHandlerSupport { + sourceEvent: string; + handler: PluginHookHandler; +} + export interface PluginHookConditionMatch { sourceEvent: string; targetEvent: HookEvent; @@ -107,6 +130,10 @@ export interface PluginHookCapabilities { ) => string | PluginHookMatcherTranslationResult | undefined; /** Maps a LibreChat runtime tool name back into the plugin's source namespace. */ toPluginToolName?: (input: PluginHookToolNameTranslation) => string; + /** Presents a runtime tool input under the plugin's source field names. */ + toPluginToolInput?: (input: PluginHookToolInputTranslation) => Record; + /** Returns an error message when the executor cannot run this handler on the current host. */ + supportsHandler?: (input: PluginHookHandlerSupport) => string | undefined; /** Evaluates Claude permission-rule syntax before a conditional handler executes. */ matchCondition?: (input: PluginHookConditionMatch) => boolean; async?: boolean; @@ -121,6 +148,15 @@ export interface PluginHookPlanEntry { handlerIndex: number; sourceMatcher?: string; matcher?: string; + /** + * Set when the matcher was authored against the plugin's alias namespace + * and translated to runtime tool names; payload name/input reverse + * translation applies only to such declarations — a native-authored + * matcher keeps native payloads. + */ + requiresToolNameTranslation?: boolean; + /** Runtime tool names the translation produced; see the translation result type. */ + translatedToolNames?: string[]; condition?: string; timeoutMs?: number; handler: PluginHookHandler; @@ -140,6 +176,19 @@ export interface PluginHookPlan { summary: PluginHookPlanSummary; } +/** + * SessionStart lifecycle sources no LibreChat run-construction path emits. + * A matcher naming one is rejected at plan time — registering it would plan + * ready and never fire, the silent-no-op failure mode planning exists to + * surface. + */ +const UNAVAILABLE_SESSION_SOURCES: Readonly> = Object.freeze({ + compact: + 'SessionStart source "compact" is unavailable because LibreChat PostCompact hook output cannot inject session context', + clear: + 'SessionStart source "clear" is unavailable because LibreChat has no clear-conversation lifecycle path', +}); + function normalizeMatcher(matcher: string | undefined): string | undefined { const trimmed = matcher?.trim(); if (!trimmed || trimmed === '*' || trimmed === '.*') { @@ -291,6 +340,10 @@ function getHandlerIssues( message: `The configured executor does not support ${handler.type} hook handlers`, }); } + const supportMessage = capabilities.supportsHandler?.({ sourceEvent, handler }); + if (supportMessage !== undefined) { + issues.push({ code: 'unsupported_handler', severity: 'error', message: supportMessage }); + } if (handler.type === 'prompt' && PROMPT_UNSUPPORTED_EVENTS.has(sourceEvent)) { issues.push({ code: 'unsupported_handler_event', @@ -332,6 +385,8 @@ function getHandlerIssues( interface MatcherPlan { sourceMatcher?: string; matcher?: string; + requiresToolNameTranslation?: boolean; + translatedToolNames?: string[]; issues: PluginHookCompatibilityIssue[]; } @@ -350,11 +405,25 @@ function planMatcher( code: 'unsupported_session_source', severity: 'warning', message: - 'Wildcard SessionStart compatibility covers startup, resume, and clear; compact is runtime-filtered', + 'Wildcard SessionStart compatibility covers startup and resume; compact and clear never occur in LibreChat', }, ], }; } + /** + * A matcherless (or wildcard) declaration carries no namespace evidence + * of its own, so it inherits the document's: hook documents are Claude + * artifacts, and a wildcard guard inspecting standard Claude names or + * fields must receive them. Declaration-wide translation (no produced + * names) presents every aliased runtime tool in the plugin namespace. + */ + if ( + targetEvent !== undefined && + TOOL_PAYLOAD_EVENTS.has(targetEvent) && + capabilities.toPluginToolName !== undefined + ) { + return { requiresToolNameTranslation: true, issues: [] }; + } return { issues: [] }; } const matcherSemantics = getClaudeMatcherSemantics(sourceEvent, sourceMatcher); @@ -373,11 +442,12 @@ function planMatcher( : sourceMatcher; const runtimeValidationIssue = validationIssue ?? getMatcherValidationIssue(sourceEvent, matcher, targetEvent); - const includesCompact = + const unavailableSource = Object.keys(UNAVAILABLE_SESSION_SOURCES).find((source) => matcherSemantics.kind === 'exact' - ? matcherSemantics.values.includes('compact') - : matcherIncludesValue(matcher, 'compact'); - if (!runtimeValidationIssue && includesCompact) { + ? matcherSemantics.values.includes(source) + : matcherIncludesValue(matcher, source), + ); + if (!runtimeValidationIssue && unavailableSource !== undefined) { return { sourceMatcher, matcher, @@ -385,8 +455,7 @@ function planMatcher( { code: 'unsupported_session_source', severity: 'error', - message: - 'SessionStart source "compact" is unavailable because LibreChat PostCompact hook output cannot inject session context', + message: UNAVAILABLE_SESSION_SOURCES[unavailableSource], }, ], }; @@ -423,6 +492,8 @@ function planMatcher( const translatedMatcher = typeof translation === 'string' ? translation : translation?.matcher; const requiresToolNameTranslation = typeof translation === 'object' && translation.requiresToolNameTranslation === true; + const translatedToolNames = + typeof translation === 'object' ? translation.translatedToolNames : undefined; if (!translatedMatcher?.trim()) { return { sourceMatcher, @@ -464,7 +535,14 @@ function planMatcher( message: 'Translated tool matchers require a reverse tool-name mapping for plugin payloads', }); } - return { sourceMatcher, matcher, issues }; + return { + sourceMatcher, + matcher, + ...(requiresToolNameTranslation && { requiresToolNameTranslation }), + ...(requiresToolNameTranslation && + translatedToolNames !== undefined && { translatedToolNames }), + issues, + }; } function hasError(issues: readonly PluginHookCompatibilityIssue[]): boolean { @@ -524,6 +602,12 @@ export function planPluginHooks( sourceMatcher: matcherPlan.sourceMatcher, }), ...(matcherPlan.matcher !== undefined && { matcher: matcherPlan.matcher }), + ...(matcherPlan.requiresToolNameTranslation === true && { + requiresToolNameTranslation: true, + }), + ...(matcherPlan.translatedToolNames !== undefined && { + translatedToolNames: matcherPlan.translatedToolNames, + }), ...(condition !== undefined && { condition }), handler, status, diff --git a/packages/api/src/agents/hooks/executor.spec.ts b/packages/api/src/agents/hooks/executor.spec.ts new file mode 100644 index 0000000000..04ba8a12fa --- /dev/null +++ b/packages/api/src/agents/hooks/executor.spec.ts @@ -0,0 +1,606 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import type { HookInput, HookEvent } from '@librechat/agents'; +import type { PluginHookExecutionRequest } from './runtime'; +import type { PluginHookHandler } from './schema'; +import { + commandExecutorCapabilities, + createCommandExecutor, + getShellHandlerIssue, +} from './executor'; + +let pluginRoot: string; +let pluginData: string; + +const PRE_TOOL_INPUT: HookInput = { + hook_event_name: 'PreToolUse', + runId: 'run-1', + threadId: 'thread-1', + toolName: 'write_file', + toolInput: { path: '/workspace/file.ts' }, + toolUseId: 'tool-1', +}; + +function request( + handler: PluginHookHandler, + overrides: Partial> = {}, +): PluginHookExecutionRequest { + return { + pluginId: 'demo', + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse' as HookEvent, + groupIndex: 0, + handlerIndex: 0, + handler, + input: PRE_TOOL_INPUT, + payload: { + hook_event_name: 'PreToolUse', + session_id: 'conversation-1', + run_id: 'run-1', + tool_name: 'write_file', + tool_input: { path: '/workspace/file.ts' }, + tool_use_id: 'tool-1', + }, + ...overrides, + }; +} + +function execute( + handler: PluginHookHandler, + overrides: Partial> = {}, + env: NodeJS.ProcessEnv = { PATH: process.env.PATH }, + executorOptions: { allowAskDecision?: boolean } = {}, +) { + const executor = createCommandExecutor({ pluginRoot, pluginData, env, ...executorOptions }); + return executor.execute(request(handler, overrides), new AbortController().signal); +} + +beforeEach(async () => { + const base = await fs.promises.realpath( + await fs.promises.mkdtemp(path.join(os.tmpdir(), 'lc-hook-exec-')), + ); + pluginRoot = path.join(base, 'root'); + pluginData = path.join(base, 'data'); + await fs.promises.mkdir(pluginRoot, { recursive: true }); + await fs.promises.mkdir(pluginData, { recursive: true }); +}); + +afterEach(async () => { + await fs.promises.rm(path.dirname(pluginRoot), { recursive: true, force: true }); +}); + +describe('createCommandExecutor', () => { + test('advertises the plan-time capabilities', () => { + const executor = createCommandExecutor({ pluginRoot, pluginData }); + expect(executor.capabilities).toBe(commandExecutorCapabilities); + expect(executor.capabilities.handlerTypes.has('command')).toBe(true); + expect( + executor.capabilities.translateMatcher?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + matcher: 'write_file|execute_code', + }), + ).toBe('write_file|execute_code'); + }); + + test('translates Claude tool aliases in matchers and payload names', () => { + const executor = createCommandExecutor({ pluginRoot, pluginData }); + expect( + executor.capabilities.translateMatcher?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + matcher: 'Bash|write_file', + }), + ).toEqual({ + matcher: 'bash_tool|write_file', + requiresToolNameTranslation: true, + translatedToolNames: ['bash_tool'], + }); + expect( + executor.capabilities.toPluginToolName?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + toolName: 'bash_tool', + }), + ).toBe('Bash'); + expect( + executor.capabilities.toPluginToolName?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + toolName: 'my_mcp_tool', + }), + ).toBe('my_mcp_tool'); + }); + + test('translates Claude aliases inside regex-form matchers or rejects unsafe ones', () => { + const translate = (matcher: string) => + createCommandExecutor({ pluginRoot, pluginData }).capabilities.translateMatcher?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + matcher, + }); + expect(translate('^Bash$')).toEqual({ + matcher: '^bash_tool$', + requiresToolNameTranslation: true, + translatedToolNames: ['bash_tool'], + }); + expect(translate('^(Write|Edit)$')).toEqual({ + matcher: '^(create_file|edit_file)$', + requiresToolNameTranslation: true, + translatedToolNames: ['create_file', 'edit_file'], + }); + expect(translate('Bashful|write_file')).toBe('Bashful|write_file'); + /** Hyphen-joined names are single tool names, never alias sites. */ + expect(translate('deploy-Bash-v2_action_example_com')).toBe( + 'deploy-Bash-v2_action_example_com', + ); + /** Regex metacharacters delimit aliases — runtime names never contain dots. */ + expect(translate('^Bash.*$')).toEqual({ + matcher: '^bash_tool.*$', + requiresToolNameTranslation: true, + translatedToolNames: ['bash_tool'], + }); + expect(translate('[Bash]')).toBeUndefined(); + expect(translate('Bash\\d')).toBeUndefined(); + expect(translate('WebSearch')).toEqual({ + matcher: 'web_search', + requiresToolNameTranslation: true, + translatedToolNames: ['web_search'], + }); + }); + + test('rejects matchers naming Claude built-ins with no runtime equivalent', () => { + const translate = (matcher: string) => + commandExecutorCapabilities.translateMatcher?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + matcher, + }); + expect(translate('Glob')).toBeUndefined(); + expect(translate('Task|my_mcp_tool')).toBeUndefined(); + expect(translate('^(Bash|WebFetch)$')).toBeUndefined(); + expect(translate('Grepish')).toBe('Grepish'); + expect(translate('my-Task-runner')).toBe('my-Task-runner'); + }); + + test('presents aliased tool inputs under Claude field names', () => { + const translate = (toolName: string, toolInput: Record) => + commandExecutorCapabilities.toPluginToolInput?.({ + sourceEvent: 'PreToolUse', + targetEvent: 'PreToolUse', + toolName, + toolInput, + }); + expect(translate('create_file', { path: '/a.md', content: 'x', overwrite: true })).toEqual({ + file_path: '/a.md', + content: 'x', + overwrite: true, + }); + expect( + translate('edit_file', { + path: '/a.md', + old_text: 'foo', + new_text: 'bar', + edits: [{ old_text: 'a', new_text: 'b' }], + }), + ).toEqual({ + file_path: '/a.md', + old_string: 'foo', + new_string: 'bar', + edits: [{ old_string: 'a', new_string: 'b' }], + }); + expect(translate('read_file', { intent: 'read', path: '/a.md' })).toEqual({ + intent: 'read', + file_path: '/a.md', + }); + expect(translate('bash_tool', { command: 'ls' })).toEqual({ command: 'ls' }); + expect(translate('my_mcp_tool', { path: '/a.md' })).toEqual({ path: '/a.md' }); + }); + + test('rejects handlers whose only command targets the wrong host shell', () => { + const portable: PluginHookHandler = { type: 'command', command: 'echo ok' }; + expect(getShellHandlerIssue(portable, 'win32')).toContain('commandWindows'); + expect(getShellHandlerIssue(portable, 'linux')).toBeUndefined(); + expect( + getShellHandlerIssue({ ...portable, commandWindows: 'Write-Output ok' }, 'win32'), + ).toBeUndefined(); + expect(getShellHandlerIssue({ ...portable, shell: 'powershell' }, 'win32')).toBeUndefined(); + expect(getShellHandlerIssue({ type: 'prompt', prompt: 'check' }, 'win32')).toBeUndefined(); + /** A PowerShell-only command cannot run through bash on POSIX hosts. */ + expect(getShellHandlerIssue({ ...portable, shell: 'powershell' }, 'linux')).toContain( + 'commandWindows', + ); + expect( + getShellHandlerIssue( + { ...portable, shell: 'powershell', commandWindows: 'Write-Output ok' }, + 'linux', + ), + ).toBeUndefined(); + }); + + test('skips execution for PowerShell-only handlers on POSIX hosts', async () => { + const output = await execute({ + type: 'command', + command: 'Write-Output should-not-run', + shell: 'powershell', + }); + expect(output).toEqual({}); + }); + + test('leaves matchers for non-tool events untranslated', () => { + const executor = createCommandExecutor({ pluginRoot, pluginData }); + expect( + executor.capabilities.translateMatcher?.({ + sourceEvent: 'StopFailure', + targetEvent: 'StopFailure', + matcher: '^Bash failed$', + }), + ).toBe('^Bash failed$'); + }); + + test('returns sanitized JSON stdout and drops host-only or invalid fields', async () => { + const output = await execute({ + type: 'command', + command: `printf '%s' '{"decision":"deny","reason":"blocked","injectedMessages":[{"content":"x"}],"allowedDecisions":["approve"],"updatedInput":{"path":"/evil"},"extra":1}'`, + }); + expect(output).toEqual({ decision: 'deny', reason: 'blocked' }); + }); + + test('receives the Claude-shaped payload on stdin', async () => { + const output = await execute({ + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);console.log(JSON.stringify({reason:p.tool_name+":"+p.session_id}));});'`, + }); + expect(output).toEqual({ reason: 'write_file:conversation-1' }); + }); + + test('maps exit code 2 to a blocking decision with stderr as the reason', async () => { + const output = await execute({ + type: 'command', + command: `echo 'writes to protected paths are refused' >&2; exit 2`, + }); + expect(output).toEqual({ decision: 'deny', reason: 'writes to protected paths are refused' }); + }); + + test('maps exit code 2 on Stop to a block decision', async () => { + const output = await execute( + { type: 'command', command: 'exit 2' }, + { sourceEvent: 'Stop', targetEvent: 'Stop' }, + ); + expect(output).toEqual({ decision: 'block' }); + }); + + test('maps exit code 2 on events without a decision channel to preventContinuation', async () => { + const handler: PluginHookHandler = { type: 'command', command: 'echo halted >&2; exit 2' }; + await expect( + execute(handler, { sourceEvent: 'UserPromptSubmit', targetEvent: 'UserPromptSubmit' }), + ).resolves.toEqual({ decision: 'deny', reason: 'halted' }); + await expect( + execute(handler, { sourceEvent: 'PostToolUse', targetEvent: 'PostToolUse' }), + ).resolves.toEqual({ preventContinuation: true, stopReason: 'halted' }); + await expect( + execute(handler, { sourceEvent: 'SessionStart', targetEvent: 'RunStart' }), + ).resolves.toEqual({ preventContinuation: true, stopReason: 'halted' }); + }); + + test('tightens ask decisions to deny unless the run supports approvals', async () => { + const handler: PluginHookHandler = { + type: 'command', + command: `printf '%s' '{"decision":"ask","reason":"confirm"}'`, + }; + await expect(execute(handler)).resolves.toEqual({ decision: 'deny', reason: 'confirm' }); + await expect( + execute(handler, {}, { PATH: process.env.PATH }, { allowAskDecision: true }), + ).resolves.toEqual({ decision: 'ask', reason: 'confirm' }); + }); + + test('returns an empty output when the payload cannot be serialized', async () => { + const output = await execute( + { type: 'command', command: 'echo unreachable' }, + { + sourceEvent: 'PostToolUse', + targetEvent: 'PostToolUse', + payload: { + hook_event_name: 'PostToolUse', + session_id: 'conversation-1', + run_id: 'run-1', + tool_response: BigInt(1), + }, + }, + ); + expect(output).toEqual({}); + }); + + test('maps Claude legacy decisions per event channel', async () => { + const handler: PluginHookHandler = { + type: 'command', + command: `printf '%s' '{"decision":"block"}'`, + }; + /** Claude's legacy PreToolUse "block" denies; on Stop it is the stop decision. */ + await expect(execute(handler)).resolves.toEqual({ decision: 'deny' }); + await expect(execute(handler, { sourceEvent: 'Stop', targetEvent: 'Stop' })).resolves.toEqual({ + decision: 'block', + }); + await expect( + execute({ type: 'command', command: `printf '%s' '{"decision":"approve"}'` }), + ).resolves.toEqual({ decision: 'allow' }); + /** "block" on events without a deny channel controls continuation instead. */ + await expect( + execute(handler, { sourceEvent: 'PostToolUse', targetEvent: 'PostToolUse' }), + ).resolves.toEqual({ preventContinuation: true }); + }); + + test('translates Claude hookSpecificOutput into engine fields', async () => { + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"hookSpecificOutput":{"hookEventName":"PreToolUse","permissionDecision":"deny","permissionDecisionReason":"protected path"}}'`, + }), + ).resolves.toEqual({ decision: 'deny', reason: 'protected path' }); + /** The ask gate applies to the Claude dialect too. */ + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"hookSpecificOutput":{"permissionDecision":"ask","permissionDecisionReason":"confirm"}}'`, + }), + ).resolves.toEqual({ decision: 'deny', reason: 'confirm' }); + await expect( + execute( + { + type: 'command', + command: `printf '%s' '{"hookSpecificOutput":{"hookEventName":"UserPromptSubmit","additionalContext":"project notes"}}'`, + }, + { sourceEvent: 'UserPromptSubmit', targetEvent: 'UserPromptSubmit' }, + ), + ).resolves.toEqual({ additionalContext: 'project notes' }); + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"continue":false,"stopReason":"manual halt"}'`, + }), + ).resolves.toEqual({ preventContinuation: true, stopReason: 'manual halt' }); + /** Native fields win when both dialects appear. */ + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"decision":"allow","hookSpecificOutput":{"permissionDecision":"deny"}}'`, + }), + ).resolves.toEqual({ decision: 'allow' }); + }); + + test('translates a structured block into continuation control on post-tool events', async () => { + const handler: PluginHookHandler = { + type: 'command', + command: `printf '%s' '{"decision":"block","reason":"output leaked a secret"}'`, + }; + await expect( + execute(handler, { sourceEvent: 'PostToolUse', targetEvent: 'PostToolUse' }), + ).resolves.toEqual({ + preventContinuation: true, + reason: 'output leaked a secret', + stopReason: 'output leaked a secret', + }); + await expect( + execute(handler, { sourceEvent: 'PostToolUseFailure', targetEvent: 'PostToolUseFailure' }), + ).resolves.toEqual({ + preventContinuation: true, + reason: 'output leaked a secret', + stopReason: 'output leaked a secret', + }); + }); + + test('rejects native decisions from the other event channel', async () => { + /** `continue` belongs to the Stop vocabulary; on a tool event it must not + * shadow the Claude decision. */ + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"decision":"continue","hookSpecificOutput":{"permissionDecision":"deny","permissionDecisionReason":"blocked"}}'`, + }), + ).resolves.toEqual({ decision: 'deny', reason: 'blocked' }); + /** `ask` belongs to the tool vocabulary and is dropped on Stop. */ + await expect( + execute( + { type: 'command', command: `printf '%s' '{"decision":"ask"}'` }, + { sourceEvent: 'Stop', targetEvent: 'Stop' }, + ), + ).resolves.toEqual({}); + }); + + test('falls back to the Claude decision when the native field is malformed', async () => { + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"decision":null,"hookSpecificOutput":{"permissionDecision":"deny","permissionDecisionReason":"protected"}}'`, + }), + ).resolves.toEqual({ decision: 'deny', reason: 'protected' }); + await expect( + execute({ + type: 'command', + command: `printf '%s' '{"decision":"maybe","reason":42,"hookSpecificOutput":{"permissionDecision":"deny","permissionDecisionReason":"unrecognized native"}}'`, + }), + ).resolves.toEqual({ decision: 'deny', reason: 'unrecognized native' }); + }); + + test('ignores non-blocking failures', async () => { + const output = await execute({ type: 'command', command: 'echo oops >&2; exit 1' }); + expect(output).toEqual({}); + }); + + test('runs from the plugin root with PLUGIN_ROOT, PLUGIN_DATA, and only allowlisted vars', async () => { + const output = await execute( + { + type: 'command', + command: `printf '{"reason":"%s|%s|%s|%s|%s"}' "$PWD" "$PLUGIN_ROOT" "$PLUGIN_DATA" "$ALLOWED_TOKEN" "\${SECRET_TOKEN:-absent}"`, + allowedEnvVars: ['ALLOWED_TOKEN', 'PLUGIN_ROOT'], + }, + {}, + { + PATH: process.env.PATH, + ALLOWED_TOKEN: 'granted', + SECRET_TOKEN: 's3cret', + PLUGIN_ROOT: '/poisoned/by/host/env', + }, + ); + expect(output).toEqual({ + reason: `${pluginRoot}|${pluginRoot}|${pluginData}|granted|absent`, + }); + }); + + test('expands PLUGIN_ROOT/PLUGIN_DATA placeholders in the command and binds args to $1..$n', async () => { + const output = await execute({ + type: 'command', + command: 'printf \'{"reason":"%s %s"}\' "$1" "${PLUGIN_DATA}"', + args: ['${PLUGIN_ROOT}/scripts/check.sh'], + }); + expect(output).toEqual({ + reason: `${path.join(pluginRoot, 'scripts/check.sh')} ${pluginData}`, + }); + }); + + test('expands the Claude plugin-root spelling in commands and the environment', async () => { + const output = await execute({ + type: 'command', + command: `printf '{"reason":"%s|%s"}' "\${CLAUDE_PLUGIN_ROOT}/hooks/check.py" "$CLAUDE_PLUGIN_ROOT"`, + }); + expect(output).toEqual({ + reason: `${path.join(pluginRoot, 'hooks/check.py')}|${pluginRoot}`, + }); + }); + + test('treats non-JSON stdout as context for prompt-shaped events and ignores it elsewhere', async () => { + const handler: PluginHookHandler = { type: 'command', command: 'echo loaded project notes' }; + await expect( + execute(handler, { sourceEvent: 'UserPromptSubmit', targetEvent: 'UserPromptSubmit' }), + ).resolves.toEqual({ additionalContext: 'loaded project notes' }); + await expect(execute(handler)).resolves.toEqual({}); + }); + + const isGone = (pid: number): boolean => { + try { + process.kill(pid, 0); + } catch { + return true; + } + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, 'utf8'); + return stat.slice(stat.lastIndexOf(')') + 2, stat.lastIndexOf(')') + 3) === 'Z'; + } catch { + return true; + } + }; + const waitFor = async (condition: () => boolean): Promise => { + const deadline = Date.now() + 5_000; + while (!condition() && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + }; + + test('escalates to a group SIGKILL when a descendant survives SIGTERM past the wrapper', async () => { + const pidFile = path.join(pluginData, 'survivor.pid'); + const controller = new AbortController(); + const executor = createCommandExecutor({ + pluginRoot, + pluginData, + env: { PATH: process.env.PATH }, + killGraceMs: 500, + }); + /** + * The descendant redirects its stdio away from the captured pipes so the + * wrapper's exit emits `close` while the descendant is still alive — + * exercising the window where a close-time cancellation would skip the + * group SIGKILL and leak the survivor. + */ + const pending = executor.execute( + request({ + type: 'command', + command: `bash -c 'trap "" TERM; echo $$ > "$PLUGIN_DATA/survivor.pid"; exec >/dev/null 2>&1; while true; do sleep 0.1; done' & wait`, + }), + controller.signal, + ); + await waitFor(() => fs.existsSync(pidFile)); + const survivorPid = Number((await fs.promises.readFile(pidFile, 'utf8')).trim()); + expect(survivorPid).toBeGreaterThan(0); + controller.abort(); + /** The wrapper exits on SIGTERM while the trap-protected descendant survives. */ + await expect(pending).resolves.toEqual({}); + expect(isGone(survivorPid)).toBe(false); + await waitFor(() => isGone(survivorPid)); + expect(isGone(survivorPid)).toBe(true); + }); + + test('reaps a pipe-holding worker at root exit instead of stalling until close', async () => { + const pidFile = path.join(pluginData, 'holder.pid'); + const executor = createCommandExecutor({ + pluginRoot, + pluginData, + env: { PATH: process.env.PATH }, + killGraceMs: 250, + }); + /** + * The worker keeps the captured pipes open, so `close` cannot fire until + * it dies — without the exit-time sweep this execution would stall for + * the worker's full 30s lifetime. + */ + const output = await executor.execute( + request({ + type: 'command', + command: `bash -c 'trap "" TERM; echo $$ > "$PLUGIN_DATA/holder.pid"; sleep 30' & while [ ! -f "$PLUGIN_DATA/holder.pid" ]; do sleep 0.01; done; printf '%s' '{"reason":"scheduled"}'`, + }), + new AbortController().signal, + ); + expect(output).toEqual({ reason: 'scheduled' }); + const holderPid = Number((await fs.promises.readFile(pidFile, 'utf8')).trim()); + expect(holderPid).toBeGreaterThan(0); + await waitFor(() => isGone(holderPid)); + expect(isGone(holderPid)).toBe(true); + }); + + test('reaps a backgrounded worker that outlives a successful hook', async () => { + const pidFile = path.join(pluginData, 'worker.pid'); + const executor = createCommandExecutor({ + pluginRoot, + pluginData, + env: { PATH: process.env.PATH }, + killGraceMs: 250, + }); + /** + * The wrapper waits for the pid file so the worker's trap is set before + * the exit-time sweep can deliver its SIGTERM. + */ + const output = await executor.execute( + request({ + type: 'command', + command: `bash -c 'trap "" TERM; echo $$ > "$PLUGIN_DATA/worker.pid"; exec >/dev/null 2>&1; while true; do sleep 0.1; done' & while [ ! -f "$PLUGIN_DATA/worker.pid" ]; do sleep 0.01; done; printf '%s' '{"reason":"scheduled"}'`, + }), + new AbortController().signal, + ); + expect(output).toEqual({ reason: 'scheduled' }); + await waitFor(() => fs.existsSync(pidFile)); + const workerPid = Number((await fs.promises.readFile(pidFile, 'utf8')).trim()); + expect(workerPid).toBeGreaterThan(0); + await waitFor(() => isGone(workerPid)); + expect(isGone(workerPid)).toBe(true); + }); + + test('returns an empty output when the signal aborts a running command', async () => { + const controller = new AbortController(); + const executor = createCommandExecutor({ + pluginRoot, + pluginData, + env: { PATH: process.env.PATH }, + }); + const pending = executor.execute( + request({ type: 'command', command: 'sleep 30 & wait' }), + controller.signal, + ); + setTimeout(() => controller.abort(), 50); + await expect(pending).resolves.toEqual({}); + }); + + test('returns an empty output when the handler has no runnable command', async () => { + const output = await execute({ type: 'command', command: ' ' }); + expect(output).toEqual({}); + }); +}); diff --git a/packages/api/src/agents/hooks/executor.ts b/packages/api/src/agents/hooks/executor.ts new file mode 100644 index 0000000000..e0b626fde4 --- /dev/null +++ b/packages/api/src/agents/hooks/executor.ts @@ -0,0 +1,666 @@ +import { spawn } from 'node:child_process'; +import { Tools } from 'librechat-data-provider'; +import { logger } from '@librechat/data-schemas'; +import { BashExecutionToolDefinition, ReadFileToolDefinition } from '@librechat/agents'; +import type { HookEvent, HookOutput, ToolDecision, StopDecision } from '@librechat/agents'; +import type { PluginHookExecutor, PluginHookExecutionRequest } from './runtime'; +import type { PluginHookCapabilities } from './compatibility'; +import type { PluginHookHandler } from './schema'; +import { CREATE_FILE_TOOL_NAME, EDIT_FILE_TOOL_NAME } from '~/agents/tools'; +import { createReaper } from './reaper'; + +const MAX_CAPTURED_STREAM_BYTES = 1_048_576; +const MAX_REASON_LENGTH = 2_000; +const MAX_ADDITIONAL_CONTEXT_LENGTH = 32_768; +const KILL_GRACE_MS = 5_000; +const BLOCKING_EXIT_CODE = 2; + +const TOOL_DECISIONS: ReadonlySet = new Set(['allow', 'deny', 'ask']); +const STOP_DECISIONS: ReadonlySet = new Set(['continue', 'block']); +/** + * Decision tokens accepted as INPUT per channel: the channel's native set + * plus the Claude legacy spellings that map into it. A token valid for the + * other channel (`continue` on a tool event) is malformed here and must not + * suppress a valid Claude decision. + */ +const TOOL_INPUT_DECISIONS: ReadonlySet = new Set([...TOOL_DECISIONS, 'approve', 'block']); +const STOP_INPUT_DECISIONS: ReadonlySet = STOP_DECISIONS; +const STDOUT_CONTEXT_EVENTS: ReadonlySet = new Set(['SessionStart', 'UserPromptSubmit']); +const PASSTHROUGH_ENV_VARS = ['PATH', 'HOME', 'LANG', 'LC_ALL', 'TZ'] as const; + +interface EventTraits { + /** Whether the matcher queries a runtime tool name, enabling alias translation. */ + toolMatcher: boolean; + /** Decision vocabulary a hook output may use for this event. */ + decisions: 'tool' | 'stop'; + /** Blocking output shape produced by the exit-code-2 contract. */ + exitTwo: 'deny' | 'block' | 'prevent'; +} + +/** + * Exhaustive per-event semantics. `satisfies Record` makes the + * compiler demand an answer for every current and future engine event, so a + * new event can never silently inherit an unconsidered default. + */ +const EVENT_TRAITS = { + RunStart: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + UserPromptSubmit: { toolMatcher: false, decisions: 'tool', exitTwo: 'deny' }, + PreToolUse: { toolMatcher: true, decisions: 'tool', exitTwo: 'deny' }, + PostToolUse: { toolMatcher: true, decisions: 'tool', exitTwo: 'prevent' }, + PostToolUseFailure: { toolMatcher: true, decisions: 'tool', exitTwo: 'prevent' }, + PostToolBatch: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + PreemptBoundary: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + PermissionDenied: { toolMatcher: true, decisions: 'tool', exitTwo: 'prevent' }, + SubagentStart: { toolMatcher: false, decisions: 'tool', exitTwo: 'deny' }, + SubagentStop: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + Stop: { toolMatcher: false, decisions: 'stop', exitTwo: 'block' }, + StopFailure: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + PreCompact: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, + PostCompact: { toolMatcher: false, decisions: 'tool', exitTwo: 'prevent' }, +} as const satisfies Record; + +function renameFields( + input: Record, + fields: Readonly>, +): Record { + const output: Record = {}; + for (const [key, value] of Object.entries(input)) { + output[fields[key] ?? key] = value; + } + return output; +} + +function isPlainObject(value: unknown): value is Record { + return typeof value === 'object' && value !== null && !Array.isArray(value); +} + +const FILE_FIELDS: Readonly> = Object.freeze({ path: 'file_path' }); +const EDIT_FIELDS: Readonly> = Object.freeze({ + path: 'file_path', + old_text: 'old_string', + new_text: 'new_string', +}); +const EDIT_BATCH_FIELDS: Readonly> = Object.freeze({ + old_text: 'old_string', + new_text: 'new_string', +}); + +function toClaudeFileInput(toolInput: Record): Record { + return renameFields(toolInput, FILE_FIELDS); +} + +function toClaudeEditInput(toolInput: Record): Record { + const renamed = renameFields(toolInput, EDIT_FIELDS); + if (Array.isArray(renamed.edits)) { + renamed.edits = renamed.edits.map((edit) => + isPlainObject(edit) ? renameFields(edit, EDIT_BATCH_FIELDS) : edit, + ); + } + return renamed; +} + +interface ClaudeToolAlias { + claudeName: string; + runtimeName: string; + /** Presents the runtime tool arguments under Claude's field names. */ + toPluginInput?: (toolInput: Record) => Record; +} + +/** + * Claude-compatible tool aliases, with runtime names imported from their + * canonical definitions rather than restated as literals — a hand-maintained + * parallel table is how the `WebSearch` mapping was originally missed. + * Without this table a plugin authored against Claude's namespace (`Bash`, + * `Write`) would plan as ready yet register a matcher that never fires — a + * silently bypassed guard. `bash_tool` and `web_search` share Claude's + * load-bearing field names (`command`, `query`), so only the file tools + * need input translation. + */ +const CLAUDE_TOOL_ALIASES: readonly ClaudeToolAlias[] = [ + { claudeName: 'Bash', runtimeName: BashExecutionToolDefinition.name }, + { claudeName: 'Write', runtimeName: CREATE_FILE_TOOL_NAME, toPluginInput: toClaudeFileInput }, + { claudeName: 'Edit', runtimeName: EDIT_FILE_TOOL_NAME, toPluginInput: toClaudeEditInput }, + { + claudeName: 'Read', + runtimeName: ReadFileToolDefinition.name, + toPluginInput: toClaudeFileInput, + }, + { claudeName: 'WebSearch', runtimeName: Tools.web_search }, +]; +const ALIAS_BY_CLAUDE: ReadonlyMap = new Map( + CLAUDE_TOOL_ALIASES.map((alias) => [alias.claudeName, alias]), +); +const ALIAS_BY_RUNTIME: ReadonlyMap = new Map( + CLAUDE_TOOL_ALIASES.map((alias) => [alias.runtimeName, alias]), +); +/** + * Alias tokens count only when delimited by characters that cannot appear in + * a runtime tool name (word characters and hyphens). A `\b` boundary is not + * enough: action tool names preserve hyphens, so `deploy-Bash-v2_...` is one + * literal tool name whose embedded alias must never be rewritten — the + * rewritten matcher would silently stop matching the real tool. Dots are + * deliberately NOT name characters: runtime names never contain them (action + * ids underscore domain dots), while regex forms like `^Bash.*$` put a + * metacharacter directly after the alias and must still translate. + */ +const TOOL_NAME_BOUNDARY_BEFORE = '(? alias.claudeName).join( + '|', + )})${TOOL_NAME_BOUNDARY_AFTER}`, + 'g', +); +/** Character classes and escapes where token substitution could corrupt regex semantics. */ +const UNSAFE_ALIAS_CONTEXT = /[\\[\]]/; + +/** + * Claude built-ins with no LibreChat runtime equivalent. A tool matcher naming + * one is rejected as unmapped at plan time: passing it through would register + * a guard that plans ready and never fires — the same silent-bypass failure + * mode the alias table exists to prevent. + */ +const UNSUPPORTED_CLAUDE_TOOLS = [ + 'Task', + 'Glob', + 'Grep', + 'MultiEdit', + 'NotebookEdit', + 'TodoWrite', + 'WebFetch', + 'BashOutput', + 'KillShell', + 'ExitPlanMode', + 'AskUserQuestion', + 'SlashCommand', +] as const; +const UNSUPPORTED_TOOL_PATTERN = new RegExp( + `${TOOL_NAME_BOUNDARY_BEFORE}(?:${UNSUPPORTED_CLAUDE_TOOLS.join( + '|', + )})${TOOL_NAME_BOUNDARY_AFTER}`, +); + +function containsAliasToken(matcher: string): boolean { + ALIAS_TOKEN_PATTERN.lastIndex = 0; + return ALIAS_TOKEN_PATTERN.test(matcher); +} + +/** + * Plan-time gate for handlers the executor cannot run on the current host. + * Windows has no portable `bash`, so a command handler must declare + * `commandWindows` or `shell: "powershell"` to be executable there; POSIX + * hosts run the portable `command` with bash, so a handler declaring + * `shell: "powershell"` without a separate `commandWindows` variant marks + * its only command as PowerShell syntax that bash would fail open on. + * Exported with an explicit platform parameter for direct testing. + */ +export function getShellHandlerIssue( + handler: PluginHookHandler, + platform: NodeJS.Platform = process.platform, +): string | undefined { + if (handler.type !== 'command') { + return undefined; + } + if (platform === 'win32') { + if (handler.shell === 'powershell' || handler.commandWindows !== undefined) { + return undefined; + } + return 'Windows hosts run command hooks with PowerShell; declare commandWindows or shell "powershell"'; + } + if (handler.shell === 'powershell' && handler.commandWindows === undefined) { + return 'This host runs command hooks with bash; shell "powershell" requires a commandWindows variant so the portable command stays bash-compatible'; + } + return undefined; +} + +/** + * Capabilities of the command executor, shared by plan time (plugin loading) + * and run time (hook registration) so a handler the loader marked `ready` is + * always executable. Alias tokens translate in exact matchers ("Bash|Write") + * and in regex matchers ("^(Write|Edit)$") via word-bounded substitution; a + * regex whose alias sits in a character class or escape — or any matcher + * naming a Claude built-in with no runtime equivalent — is rejected as + * unmapped, a loud plan-time diagnostic instead of a guard that never fires. + * Payloads are presented entirely in the plugin's namespace: `tool_name` + * maps back to the Claude alias and `tool_input` fields are renamed to + * Claude's schema, so one hook script works unchanged across both. + */ +export const commandExecutorCapabilities: PluginHookCapabilities = { + handlerTypes: new Set(['command']), + translateMatcher: ({ matcher, targetEvent }) => { + if (EVENT_TRAITS[targetEvent].toolMatcher !== true) { + return matcher; + } + if (UNSUPPORTED_TOOL_PATTERN.test(matcher)) { + return undefined; + } + if (!containsAliasToken(matcher)) { + return matcher; + } + if (UNSAFE_ALIAS_CONTEXT.test(matcher)) { + return undefined; + } + ALIAS_TOKEN_PATTERN.lastIndex = 0; + const translatedToolNames = new Set(); + const mapped = matcher.replace(ALIAS_TOKEN_PATTERN, (claudeName) => { + const alias = ALIAS_BY_CLAUDE.get(claudeName); + if (alias === undefined) { + return claudeName; + } + translatedToolNames.add(alias.runtimeName); + return alias.runtimeName; + }); + /** + * The produced names scope reverse payload translation per invocation: + * a mixed matcher like "Bash|create_file" translates payloads only for + * `bash_tool`, keeping the natively-authored alternative native. + */ + return { + matcher: mapped, + requiresToolNameTranslation: true, + translatedToolNames: Array.from(translatedToolNames), + }; + }, + toPluginToolName: ({ toolName }) => ALIAS_BY_RUNTIME.get(toolName)?.claudeName ?? toolName, + toPluginToolInput: ({ toolName, toolInput }) => + ALIAS_BY_RUNTIME.get(toolName)?.toPluginInput?.(toolInput) ?? toolInput, + supportsHandler: ({ handler }) => getShellHandlerIssue(handler), + sessionLifecycle: true, +}; + +export interface CommandExecutorOptions { + /** Filesystem-resolved plugin root; becomes the command's cwd and `PLUGIN_ROOT`. */ + pluginRoot: string; + /** Persistent per-plugin data directory; becomes `PLUGIN_DATA`. */ + pluginData: string; + /** Environment source for the allowlist (defaults to `process.env`). */ + env?: NodeJS.ProcessEnv; + /** + * Whether `ask` decisions can raise a resumable approval interrupt. Off by + * default: without a HITL surface an `ask` is tightened to `deny` so a + * plugin's confirmation intent still blocks rather than stranding the run. + */ + allowAskDecision?: boolean; + /** SIGTERM-to-SIGKILL escalation delay after an abort (defaults to 5s). */ + killGraceMs?: number; +} + +interface CommandCompletion { + code: number | null; + stdout: string; + stderr: string; +} + +function buildCommandEnv( + options: CommandExecutorOptions, + allowedEnvVars: string[] | undefined, +): NodeJS.ProcessEnv { + const source = options.env ?? process.env; + const env: NodeJS.ProcessEnv = {}; + for (const name of PASSTHROUGH_ENV_VARS) { + if (source[name] !== undefined) { + env[name] = source[name]; + } + } + for (const name of allowedEnvVars ?? []) { + if (source[name] !== undefined) { + env[name] = source[name]; + } + } + /** Reserved names win last so an allowlist entry can never override them. */ + env.PLUGIN_ROOT = options.pluginRoot; + env.PLUGIN_DATA = options.pluginData; + env.CLAUDE_PLUGIN_ROOT = options.pluginRoot; + return env; +} + +/** + * Agent Plugins §9.2 expansion applied to hook commands: one literal, + * non-recursive pass. `CLAUDE_PLUGIN_ROOT` is Claude Code's spelling of the + * plugin root — hooks authored for Claude use it in every standard command — + * and it is also exported in the child environment for unbraced references. + */ +function expandVariables(value: string, options: CommandExecutorOptions): string { + return value.replace( + /\$\{(PLUGIN_ROOT|PLUGIN_DATA|CLAUDE_PLUGIN_ROOT)\}/g, + (_match, name: string) => (name === 'PLUGIN_DATA' ? options.pluginData : options.pluginRoot), + ); +} + +interface ShellInvocation { + executable: string; + argv: string[]; +} + +/** + * POSIX hosts run `bash -c ` with `args` bound to `$1..$n`. Windows + * hosts run PowerShell and require `commandWindows` or `shell: powershell`. + * Both directions are enforced at plan time via `supportsHandler` and again + * here, so a command never silently runs through a shell it was not written + * for: a PowerShell-only handler is skipped on POSIX, while a handler with + * both variants runs its portable `command` there. + */ +function buildInvocation( + request: PluginHookExecutionRequest, + options: CommandExecutorOptions, +): ShellInvocation | undefined { + const { handler } = request; + const isWindows = process.platform === 'win32'; + const rawCommand = isWindows ? (handler.commandWindows ?? handler.command) : handler.command; + if (!rawCommand?.trim()) { + return undefined; + } + const command = expandVariables(rawCommand, options); + const args = (handler.args ?? []).map((arg) => expandVariables(arg, options)); + if (isWindows) { + if (handler.shell !== 'powershell' && handler.commandWindows === undefined) { + return undefined; + } + const quotedArgs = args.map((arg) => `'${arg.replace(/'/g, "''")}'`); + return { + executable: 'powershell.exe', + argv: ['-NoLogo', '-NoProfile', '-Command', [command, ...quotedArgs].join(' ')], + }; + } + if (handler.shell === 'powershell' && handler.commandWindows === undefined) { + return undefined; + } + return { executable: 'bash', argv: ['-c', command, 'bash', ...args] }; +} + +interface CapturedStream { + chunks: Buffer[]; + bytes: number; +} + +function appendCapped(stream: CapturedStream, chunk: Buffer): void { + const remaining = MAX_CAPTURED_STREAM_BYTES - stream.bytes; + if (remaining <= 0) { + return; + } + const kept = chunk.byteLength > remaining ? chunk.subarray(0, remaining) : chunk; + stream.chunks.push(kept); + stream.bytes += kept.byteLength; +} + +function capturedText(stream: CapturedStream): string { + return Buffer.concat(stream.chunks).toString('utf8'); +} + +function runCommand( + invocation: ShellInvocation, + payload: string, + env: NodeJS.ProcessEnv, + cwd: string, + signal: AbortSignal, + killGraceMs: number, +): Promise { + return new Promise((resolve, reject) => { + const child = spawn(invocation.executable, invocation.argv, { + cwd, + env, + detached: process.platform !== 'win32', + stdio: ['pipe', 'pipe', 'pipe'], + }); + const stdout: CapturedStream = { chunks: [], bytes: 0 }; + const stderr: CapturedStream = { chunks: [], bytes: 0 }; + const reaper = createReaper(child, killGraceMs); + const onAbort = (): void => reaper.reap(); + signal.addEventListener('abort', onAbort, { once: true }); + + child.stdout.on('data', (chunk: Buffer) => { + appendCapped(stdout, chunk); + }); + child.stderr.on('data', (chunk: Buffer) => { + appendCapped(stderr, chunk); + }); + child.on('error', (error) => { + signal.removeEventListener('abort', onAbort); + reject(error); + }); + /** + * Swept at `exit` as well as `close`: a backgrounded descendant holding + * the captured pipes delays `close` until it dies, so the exit-time + * sweep is what keeps a successful hook from stalling on its own + * unsupported worker. + */ + child.on('exit', () => { + reaper.sweep(); + }); + child.on('close', (code) => { + signal.removeEventListener('abort', onAbort); + reaper.sweep(); + resolve({ code, stdout: capturedText(stdout), stderr: capturedText(stderr) }); + }); + child.stdin.on('error', () => { + /* A handler that never reads stdin closes the pipe early; EPIPE is not a failure. */ + }); + child.stdin.end(payload); + }); +} + +function truncate(value: string, limit: number): string { + return value.length > limit ? value.slice(0, limit) : value; +} + +/** + * Accepts only the SDK output fields a plugin command may set. Decisions are + * validated against the target event's legal set; message-injection fields + * (`injectedMessages`, `allowedDecisions`) stay host-only. `updatedInput` is + * also host-only: hooks in one dispatch all receive the original arguments, + * so a plugin rewrite would reach the tool without the approval policy ever + * re-evaluating it. + */ +function sanitizeOutput( + raw: Record, + request: PluginHookExecutionRequest, + options: CommandExecutorOptions, +): HookOutput { + const output: Record = {}; + const decisions = + EVENT_TRAITS[request.targetEvent].decisions === 'stop' ? STOP_DECISIONS : TOOL_DECISIONS; + if (typeof raw.decision === 'string' && decisions.has(raw.decision)) { + output.decision = + raw.decision === 'ask' && options.allowAskDecision !== true ? 'deny' : raw.decision; + } + if (typeof raw.reason === 'string') { + output.reason = truncate(raw.reason, MAX_REASON_LENGTH); + } + if (typeof raw.additionalContext === 'string') { + output.additionalContext = truncate(raw.additionalContext, MAX_ADDITIONAL_CONTEXT_LENGTH); + } + if (request.targetEvent === 'PostToolUse' && 'updatedOutput' in raw) { + output.updatedOutput = raw.updatedOutput; + } + if (typeof raw.preventContinuation === 'boolean') { + output.preventContinuation = raw.preventContinuation; + } + if (typeof raw.stopReason === 'string') { + output.stopReason = truncate(raw.stopReason, MAX_REASON_LENGTH); + } + if (raw.async === true) { + output.async = true; + } + return output as HookOutput; +} + +/** + * Normalizes Claude Code's structured hook-output dialect into the engine's + * native field names before sanitizing, so a stock Claude guard works + * unchanged: `hookSpecificOutput.permissionDecision`/`…Reason` become + * `decision`/`reason`, `hookSpecificOutput.additionalContext` surfaces, + * `continue: false` becomes `preventContinuation`, and the legacy decisions + * map (`approve` → `allow`; `block` → `deny` on events that block by + * denying, or into `preventContinuation` where the event has no decision + * channel at all). Native fields win when both dialects appear — but only + * when valid for THIS event: a malformed value (`"decision": null`) or one + * from the other channel's vocabulary (`"continue"` on a tool event) is + * stripped before the dialect merge, so it cannot suppress a valid Claude + * decision into a silent allow. + */ +function normalizeOutput( + raw: Record, + request: PluginHookExecutionRequest, +): Record { + const traits = EVENT_TRAITS[request.targetEvent]; + const acceptedDecisions = + traits.decisions === 'stop' ? STOP_INPUT_DECISIONS : TOOL_INPUT_DECISIONS; + const output: Record = { ...raw }; + if (typeof output.decision !== 'string' || !acceptedDecisions.has(output.decision)) { + delete output.decision; + } + if (typeof output.reason !== 'string') { + delete output.reason; + } + if (typeof output.additionalContext !== 'string') { + delete output.additionalContext; + } + if (raw.continue === false && output.preventContinuation === undefined) { + output.preventContinuation = true; + } + const hookSpecific = isPlainObject(raw.hookSpecificOutput) ? raw.hookSpecificOutput : undefined; + if (hookSpecific !== undefined) { + if (output.decision === undefined && typeof hookSpecific.permissionDecision === 'string') { + output.decision = hookSpecific.permissionDecision; + if ( + output.reason === undefined && + typeof hookSpecific.permissionDecisionReason === 'string' + ) { + output.reason = hookSpecific.permissionDecisionReason; + } + } + if ( + output.additionalContext === undefined && + typeof hookSpecific.additionalContext === 'string' + ) { + output.additionalContext = hookSpecific.additionalContext; + } + } + if (output.decision === 'approve') { + output.decision = 'allow'; + } else if (output.decision === 'block' && traits.decisions === 'tool') { + if (traits.exitTwo === 'deny') { + output.decision = 'deny'; + } else { + /** + * Post-tool and other prevent-trait events have no deny channel, so a + * structured `block` controls the run the only way it can: by stopping + * the next model turn, carrying its reason as the stop reason. + */ + delete output.decision; + output.preventContinuation = true; + if (output.stopReason === undefined && typeof output.reason === 'string') { + output.stopReason = output.reason; + } + } + } + return output; +} + +function parseCompletion( + completion: CommandCompletion, + request: PluginHookExecutionRequest, + options: CommandExecutorOptions, +): HookOutput { + const label = `[pluginHooks] ${request.pluginId} ${request.sourceEvent}`; + if (completion.code === BLOCKING_EXIT_CODE) { + const reason = truncate(completion.stderr.trim(), MAX_REASON_LENGTH); + const exitTwo = EVENT_TRAITS[request.targetEvent].exitTwo; + if (exitTwo === 'block') { + return { decision: 'block', ...(reason && { reason }) }; + } + if (exitTwo === 'deny') { + return { decision: 'deny', ...(reason && { reason }) }; + } + /** Events with no decision channel block by preventing the next model turn. */ + return { preventContinuation: true, ...(reason && { stopReason: reason }) }; + } + if (completion.code !== 0) { + logger.warn( + `${label}: command exited with code ${completion.code}: ${truncate( + completion.stderr.trim(), + MAX_REASON_LENGTH, + )}`, + ); + return {}; + } + const stdout = completion.stdout.trim(); + if (!stdout) { + return {}; + } + if (stdout.startsWith('{')) { + try { + const parsed: unknown = JSON.parse(stdout); + if (isPlainObject(parsed)) { + return sanitizeOutput(normalizeOutput(parsed, request), request, options); + } + } catch (error) { + logger.warn(`${label}: stdout is not valid JSON and was ignored`, error); + return {}; + } + } + if (STDOUT_CONTEXT_EVENTS.has(request.sourceEvent)) { + return { additionalContext: truncate(stdout, MAX_ADDITIONAL_CONTEXT_LENGTH) }; + } + return {}; +} + +/** + * Runs `command` hook handlers as child processes outside the LibreChat API + * process, mirroring Claude Code's contract: the event payload arrives as + * JSON on stdin, exit 0 with JSON stdout returns a (sanitized) hook output, + * exit 2 blocks with stderr as the reason, and any other exit is logged and + * ignored. Commands run from the plugin root with a minimal allowlisted + * environment plus `PLUGIN_ROOT`/`PLUGIN_DATA`. + * + * SECURITY: deployment plugins are operator-installed code, the same trust + * level as `toolApproval.hooks` modules. Execution is additionally gated on + * the `DEPLOYMENT_PLUGIN_HOOKS` environment opt-in (see `plugins/runtime`). + */ +export function createCommandExecutor(options: CommandExecutorOptions): PluginHookExecutor { + return { + capabilities: commandExecutorCapabilities, + async execute(request, signal) { + const invocation = buildInvocation(request, options); + if (invocation === undefined || signal.aborted) { + return {}; + } + const env = buildCommandEnv(options, request.handler.allowedEnvVars); + let payload: string; + try { + payload = JSON.stringify(request.payload); + } catch (error) { + logger.warn( + `[pluginHooks] ${request.pluginId} ${request.sourceEvent}: payload could not be serialized`, + error, + ); + return {}; + } + try { + const completion = await runCommand( + invocation, + payload, + env, + options.pluginRoot, + signal, + options.killGraceMs ?? KILL_GRACE_MS, + ); + if (signal.aborted) { + logger.warn(`[pluginHooks] ${request.pluginId} ${request.sourceEvent}: command aborted`); + return {}; + } + return parseCompletion(completion, request, options); + } catch (error) { + if (signal.aborted) { + logger.warn(`[pluginHooks] ${request.pluginId} ${request.sourceEvent}: command aborted`); + return {}; + } + logger.warn( + `[pluginHooks] ${request.pluginId} ${request.sourceEvent}: command failed to run`, + error, + ); + return {}; + } + }, + }; +} diff --git a/packages/api/src/agents/hooks/index.ts b/packages/api/src/agents/hooks/index.ts index d4ad47f63d..2ab8961574 100644 --- a/packages/api/src/agents/hooks/index.ts +++ b/packages/api/src/agents/hooks/index.ts @@ -1,3 +1,5 @@ export * from './schema'; +export * from './source'; export * from './runtime'; +export * from './executor'; export * from './compatibility'; diff --git a/packages/api/src/agents/hooks/reaper.spec.ts b/packages/api/src/agents/hooks/reaper.spec.ts new file mode 100644 index 0000000000..8898795884 --- /dev/null +++ b/packages/api/src/agents/hooks/reaper.spec.ts @@ -0,0 +1,84 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; +import { createReaper } from './reaper'; + +let base: string; + +function run(command: string): ChildProcess { + return spawn('bash', ['-c', command], { + detached: true, + stdio: 'ignore', + env: { PATH: process.env.PATH }, + }); +} + +function isGone(pid: number): boolean { + try { + process.kill(pid, 0); + } catch { + return true; + } + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, 'utf8'); + return stat.slice(stat.lastIndexOf(')') + 2, stat.lastIndexOf(')') + 3) === 'Z'; + } catch { + return true; + } +} + +async function waitFor(condition: () => boolean): Promise { + const deadline = Date.now() + 5_000; + while (!condition() && Date.now() < deadline) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } +} + +beforeEach(async () => { + base = await fs.promises.mkdtemp(path.join(os.tmpdir(), 'lc-reaper-')); +}); + +afterEach(async () => { + await fs.promises.rm(base, { recursive: true, force: true }); +}); + +describe('createReaper', () => { + test('reap terminates a compliant process tree at SIGTERM', async () => { + const child = run('sleep 30 & wait'); + await waitFor(() => typeof child.pid === 'number'); + const rootPid = child.pid as number; + createReaper(child, 5_000).reap(); + await waitFor(() => isGone(rootPid)); + expect(isGone(rootPid)).toBe(true); + }); + + test('escalates to SIGKILL when the tree ignores SIGTERM', async () => { + const pidFile = path.join(base, 'stubborn.pid'); + const child = run(`trap '' TERM; echo $$ > "${pidFile}"; sleep 30`); + await waitFor(() => fs.existsSync(pidFile)); + const rootPid = Number((await fs.promises.readFile(pidFile, 'utf8')).trim()); + createReaper(child, 200).reap(); + /** SIGTERM alone leaves the trap-protected root running. */ + expect(isGone(rootPid)).toBe(false); + await waitFor(() => isGone(rootPid)); + expect(isGone(rootPid)).toBe(true); + }); + + test('sweep reaps a group that outlived a clean root exit', async () => { + const pidFile = path.join(base, 'worker.pid'); + const child = run( + `bash -c 'trap "" TERM; echo $$ > "${pidFile}"; sleep 30' >/dev/null 2>&1 & exit 0`, + ); + const reaper = createReaper(child, 200); + const closed = new Promise((resolve) => child.once('close', () => resolve())); + await closed; + await waitFor(() => fs.existsSync(pidFile)); + const workerPid = Number((await fs.promises.readFile(pidFile, 'utf8')).trim()); + expect(isGone(workerPid)).toBe(false); + reaper.sweep(); + await waitFor(() => isGone(workerPid)); + expect(isGone(workerPid)).toBe(true); + }); +}); diff --git a/packages/api/src/agents/hooks/reaper.ts b/packages/api/src/agents/hooks/reaper.ts new file mode 100644 index 0000000000..d070f0bece --- /dev/null +++ b/packages/api/src/agents/hooks/reaper.ts @@ -0,0 +1,121 @@ +import { spawn } from 'node:child_process'; +import type { ChildProcess } from 'node:child_process'; + +export interface Reaper { + /** Terminates the process tree now and arms the forced escalation pass. */ + reap(): void; + /** + * Root exit/close notification: cancels an escalation that can no longer + * reap anything, or reaps a group that outlived the root — async handlers + * are unsupported, so no lifecycle owns a process that survives its + * wrapper. Callers notify on `exit` (so a pipe-holding descendant is + * terminated promptly instead of stalling `close` until the hook timeout) + * and again on `close`; the sweep is idempotent across both. + */ + sweep(): void; +} + +/** + * POSIX children detach into their own process group so a reap can kill the + * whole tree — a hook that launches descendants (`worker & wait`) would + * otherwise leave them running with the captured stdio open. Windows has no + * group signal; `taskkill /t` walks the tree there, with `/f` on the + * forced pass, falling back to a direct kill if `taskkill` is unavailable. + */ +function killTree(child: ChildProcess, killSignal: NodeJS.Signals): void { + const pid = child.pid; + if (typeof pid !== 'number') { + child.kill(killSignal); + return; + } + if (process.platform === 'win32') { + const force = killSignal === 'SIGKILL' ? ['/f'] : []; + try { + spawn('taskkill', ['/pid', String(pid), '/t', ...force], { stdio: 'ignore' }).once( + 'error', + () => child.kill(killSignal), + ); + } catch { + child.kill(killSignal); + } + return; + } + try { + process.kill(-pid, killSignal); + } catch { + child.kill(killSignal); + } +} + +function groupExists(pid: number): boolean { + try { + process.kill(-pid, 0); + return true; + } catch { + return false; + } +} + +/** + * Whether a forced escalation pass can still accomplish anything. POSIX group + * signals reach descendants even after the wrapper dies, so the group is + * probed directly. Windows `taskkill /t` walks the tree from the root + * process, so once Node has observed the root's exit the pass can reap + * nothing and a late signal could only hit a recycled PID; orphaned + * SIGTERM-ignoring descendants there are an accepted platform limitation + * without Job Objects. + */ +function escalationTargetAlive(child: ChildProcess): boolean { + if (typeof child.pid !== 'number') { + return false; + } + if (process.platform === 'win32') { + return child.exitCode === null && child.signalCode === null; + } + return groupExists(child.pid); +} + +/** + * Owns the lifecycle of a hook command's process tree: terminate on demand, + * escalate to a forced kill after the grace period, and never signal + * blindly. Escalation must survive the wrapper's close — a SIGTERM-ignoring + * descendant can outlive the shell on POSIX — yet a fully-dead tree's + * numeric id could be recycled during the grace window, so both the + * close-time cancellation and the deadline delivery consult + * `escalationTargetAlive` first. The residual check-to-signal race is + * unavoidable without pidfd support and needs survivors at close AND full + * tree death AND a recycled id inside the same window. + * + * The caller spawns POSIX children with `detached: true` so the root's pid + * doubles as the process-group id — that precondition is part of this + * interface. + */ +export function createReaper(child: ChildProcess, killGraceMs: number): Reaper { + let killTimer: NodeJS.Timeout | undefined; + const reap = (): void => { + if (killTimer !== undefined) { + return; + } + killTree(child, 'SIGTERM'); + killTimer = setTimeout(() => { + if (!escalationTargetAlive(child)) { + return; + } + killTree(child, 'SIGKILL'); + }, killGraceMs); + killTimer.unref?.(); + }; + return { + reap, + sweep(): void { + if (killTimer !== undefined && !escalationTargetAlive(child)) { + clearTimeout(killTimer); + killTimer = undefined; + return; + } + if (killTimer === undefined && escalationTargetAlive(child)) { + reap(); + } + }, + }; +} diff --git a/packages/api/src/agents/hooks/runtime.spec.ts b/packages/api/src/agents/hooks/runtime.spec.ts index 2ec0811873..1b8b7b464e 100644 --- a/packages/api/src/agents/hooks/runtime.spec.ts +++ b/packages/api/src/agents/hooks/runtime.spec.ts @@ -5,6 +5,7 @@ import type { PluginHookCapabilities } from './compatibility'; import type { PluginHookExecutor } from './runtime'; import type { PluginHooksDocument } from './schema'; import { createPluginHookPayload, registerPluginHooks } from './runtime'; +import { planPluginHooks } from './compatibility'; const commandCapabilities: PluginHookCapabilities = { handlerTypes: new Set(['command']), @@ -946,6 +947,27 @@ describe('registerPluginHooks', () => { expect(registry.getMatchers('Stop')).toHaveLength(0); }); + test('registers from a supplied load-time plan without re-planning', () => { + const registry = new HookRegistry(); + const hookExecutor = executor(); + const planned = document({ + Stop: [{ hooks: [{ type: 'command', command: 'stop-hook.sh' }] }], + }); + const plan = planPluginHooks(planned, hookExecutor.capabilities); + const registration = registerPluginHooks({ + pluginId: 'planned-plugin', + registry, + executor: hookExecutor, + /** An empty document proves the supplied plan, not the document, drives registration. */ + document: document({}), + plan, + }); + + expect(registration.plan).toBe(plan); + expect(registration.registered).toBe(1); + expect(registry.getMatchers('Stop')).toHaveLength(1); + }); + test('unregisters only this plugin registration and is idempotent', () => { const registry = new HookRegistry(); const existing = jest.fn(async () => ({})); diff --git a/packages/api/src/agents/hooks/runtime.ts b/packages/api/src/agents/hooks/runtime.ts index 4372e444cc..0425835f53 100644 --- a/packages/api/src/agents/hooks/runtime.ts +++ b/packages/api/src/agents/hooks/runtime.ts @@ -13,6 +13,15 @@ import { planPluginHooks } from './compatibility'; export interface PluginHookRuntimeContext { sessionId?: string; + /** Authenticated principal owning the run; scopes cross-run dedup keys. */ + userId?: string; + /** + * Session working directory for the payload's `cwd`. LibreChat runs supply + * none: tool paths address a remote code-execution sandbox, not the API + * host where hook commands run, so no host directory describes the run. + * Hook commands resolve their own paths from `PLUGIN_ROOT`, which is also + * the process working directory. + */ cwd?: string; transcriptPath?: string | null; permissionMode?: string; @@ -62,6 +71,9 @@ export interface PluginHookExecutionRequest { sourceEvent: string; targetEvent: HookEvent; handler: PluginHookHandler; + /** Declaration position in the source document; distinguishes identical handlers under different matchers. */ + groupIndex: number; + handlerIndex: number; condition?: string; input: HookInput; payload: PluginHookPayload; @@ -73,6 +85,14 @@ export interface PluginHookExecutionRequest { */ export interface PluginHookExecutor { capabilities: PluginHookCapabilities; + /** + * Pre-execution gate consulted before the declaration claims its per-input + * dedup slot. A suppressed declaration (for example, once-state already + * recorded) must decline here rather than no-op inside `execute`, so an + * identical handler under an overlapping matcher can still claim the slot + * and fire independently. + */ + shouldExecute?(request: PluginHookExecutionRequest): boolean | Promise; execute( request: PluginHookExecutionRequest, signal: AbortSignal, @@ -84,6 +104,12 @@ export interface RegisterPluginHooksOptions { registry: HookRegistry; document: PluginHooksDocument; executor: PluginHookExecutor; + /** + * Plan already computed from `document` with the SAME executor capabilities + * (e.g. at plugin load). Supplying it skips re-planning up to 512 handlers + * on every run; omitted, the document is planned here. + */ + plan?: PluginHookPlan; context?: PluginHookRuntimeContext; } @@ -96,6 +122,10 @@ export interface PluginHookRegistration { interface PluginHookPayloadState { compactTrigger?: string; toPluginToolName?: (toolName: string) => string; + toPluginToolInput?: ( + toolName: string, + toolInput: Record, + ) => Record; } function getSessionId(input: HookInput, context: PluginHookRuntimeContext): string { @@ -120,6 +150,14 @@ function getPluginToolName(toolName: string, state: PluginHookPayloadState): str return translated; } +function getPluginToolInput( + toolName: string, + toolInput: Record, + state: PluginHookPayloadState, +): Record { + return state.toPluginToolInput?.(toolName, toolInput) ?? toolInput; +} + function getMessageText(message: unknown): string | undefined { if (!message || typeof message !== 'object' || !('content' in message)) { return undefined; @@ -152,7 +190,7 @@ function toPluginBatchToolCall( const toolResponse = entry.status === 'success' ? entry.toolOutput : entry.error; return { tool_name: getPluginToolName(entry.toolName, state), - tool_input: entry.toolInput, + tool_input: getPluginToolInput(entry.toolName, entry.toolInput, state), tool_use_id: entry.toolUseId, ...(toolResponse !== undefined && { tool_response: toolResponse }), }; @@ -203,14 +241,14 @@ export function createPluginHookPayload( return { ...payload, tool_name: getPluginToolName(input.toolName, state), - tool_input: input.toolInput, + tool_input: getPluginToolInput(input.toolName, input.toolInput, state), tool_use_id: input.toolUseId, }; case 'PostToolUse': return { ...payload, tool_name: getPluginToolName(input.toolName, state), - tool_input: input.toolInput, + tool_input: getPluginToolInput(input.toolName, input.toolInput, state), tool_use_id: input.toolUseId, tool_response: input.toolOutput, }; @@ -218,7 +256,7 @@ export function createPluginHookPayload( return { ...payload, tool_name: getPluginToolName(input.toolName, state), - tool_input: input.toolInput, + tool_input: getPluginToolInput(input.toolName, input.toolInput, state), tool_use_id: input.toolUseId, error: input.error, }; @@ -234,7 +272,7 @@ export function createPluginHookPayload( return { ...payload, tool_name: getPluginToolName(input.toolName, state), - tool_input: input.toolInput, + tool_input: getPluginToolInput(input.toolName, input.toolInput, state), tool_use_id: input.toolUseId, reason: input.reason, }; @@ -301,7 +339,7 @@ function getHandlerIdentity( export function registerPluginHooks(options: RegisterPluginHooksOptions): PluginHookRegistration { const { pluginId, registry, document, executor, context = {} } = options; - const plan = planPluginHooks(document, executor.capabilities); + const plan = options.plan ?? planPluginHooks(document, executor.capabilities); const unregisters: Array<() => void> = []; const compactTriggers = new Map(); const executedHandlers = new WeakMap>(); @@ -331,16 +369,46 @@ export function registerPluginHooks(options: RegisterPluginHooksOptions): Plugin const handlerIdentity = getHandlerIdentity(entry.sourceEvent, entry.handler, entry.condition); const seenSessionIds = entry.sourceEvent === 'SessionStart' ? new Set() : undefined; const firedSessionIds = entry.handler.once === true ? new Set() : undefined; - const toolNameTranslator = executor.capabilities.toPluginToolName; + /** + * Payloads follow the namespace each alternative was authored in: only a + * matcher that required alias translation gets reverse name/input + * translation, and when the plan records which runtime names the + * translation produced, it applies per invocation — a mixed matcher like + * `Bash|create_file` presents Claude-shaped payloads for `bash_tool` and + * native payloads for the natively-authored alternative. + */ + const translated = entry.requiresToolNameTranslation === true; + const translatedNames = + translated && entry.translatedToolNames !== undefined + ? new Set(entry.translatedToolNames) + : undefined; + const inTranslatedNamespace = (toolName: string): boolean => + translated && (translatedNames === undefined || translatedNames.has(toolName)); + const toolNameTranslator = translated ? executor.capabilities.toPluginToolName : undefined; const toPluginToolName = toolNameTranslator === undefined ? undefined : (toolName: string): string => - toolNameTranslator({ - sourceEvent: entry.sourceEvent, - targetEvent, - toolName, - }); + inTranslatedNamespace(toolName) + ? toolNameTranslator({ + sourceEvent: entry.sourceEvent, + targetEvent, + toolName, + }) + : toolName; + const toolInputTranslator = translated ? executor.capabilities.toPluginToolInput : undefined; + const toPluginToolInput = + toolInputTranslator === undefined + ? undefined + : (toolName: string, toolInput: Record): Record => + inTranslatedNamespace(toolName) + ? toolInputTranslator({ + sourceEvent: entry.sourceEvent, + targetEvent, + toolName, + toolInput, + }) + : toolInput; const hook: HookCallback = (input, signal) => { const sessionId = getSessionId(input, context); const compactTrigger = compactTriggers.get(sessionId); @@ -388,7 +456,7 @@ export function registerPluginHooks(options: RegisterPluginHooksOptions): Plugin targetEvent, condition: entry.condition, toolName: getPluginToolName(input.toolName, { toPluginToolName }), - toolInput: input.toolInput, + toolInput: getPluginToolInput(input.toolName, input.toolInput, { toPluginToolInput }), }); } catch { return {}; @@ -405,27 +473,44 @@ export function registerPluginHooks(options: RegisterPluginHooksOptions): Plugin firedSessionIds?.add(sessionId); return {}; } - if (handlersForInput) { - handlersForInput.add(handlerIdentity); - } else { - executedHandlers.set(input, new Set([handlerIdentity])); + const request: PluginHookExecutionRequest = { + pluginId, + sourceEvent: entry.sourceEvent, + targetEvent, + handler: entry.handler, + groupIndex: entry.groupIndex, + handlerIndex: entry.handlerIndex, + ...(entry.condition !== undefined && { condition: entry.condition }), + input, + payload: createPluginHookPayload(entry.sourceEvent, input, context, { + compactTrigger, + toPluginToolName, + toPluginToolInput, + }), + }; + /** + * The per-input dedup slot is claimed only by a declaration that will + * actually run: one the executor suppresses (spent once-state) must + * not consume it, or an identical handler under an overlapping matcher + * could never claim the slot and would be permanently shadowed. + */ + const claimAndExecute = (): HookOutput | Promise => { + if (handlersForInput) { + handlersForInput.add(handlerIdentity); + } else { + executedHandlers.set(input, new Set([handlerIdentity])); + } + firedSessionIds?.add(sessionId); + return executor.execute(request, signal); + }; + const shouldRun = executor.shouldExecute?.(request) ?? true; + if (shouldRun === true) { + return claimAndExecute(); } - firedSessionIds?.add(sessionId); - return executor.execute( - { - pluginId, - sourceEvent: entry.sourceEvent, - targetEvent, - handler: entry.handler, - ...(entry.condition !== undefined && { condition: entry.condition }), - input, - payload: createPluginHookPayload(entry.sourceEvent, input, context, { - compactTrigger, - toPluginToolName, - }), - }, - signal, - ); + if (shouldRun === false) { + return {}; + } + return shouldRun.then((run) => (run ? claimAndExecute() : {})); }; const runtimeFiltered = entry.sourceEvent === 'SessionStart' || diff --git a/packages/api/src/agents/hooks/source.ts b/packages/api/src/agents/hooks/source.ts new file mode 100644 index 0000000000..8774d77910 --- /dev/null +++ b/packages/api/src/agents/hooks/source.ts @@ -0,0 +1,30 @@ +import type { HookRegistry } from '@librechat/agents'; +import type { PluginHookRuntimeContext } from './runtime'; + +export interface PluginHookSourceOptions { + registry: HookRegistry; + context?: PluginHookRuntimeContext; + /** Whether the run can raise resumable `ask` interrupts (HITL wiring attached). */ + askDecisionSupported?: boolean; +} + +/** + * Host-supplied provider of plugin hooks for agent runs. Keeps the dependency + * direction one-way: the run seam reads hooks through this seam while the + * plugins package (which imports agents code) registers the implementation at + * startup — mirroring the tool-approval hook registry pattern. + */ +export interface PluginHookSource { + hasHooks(): boolean; + register(options: PluginHookSourceOptions): number; +} + +let source: PluginHookSource | undefined; + +export function setPluginHookSource(next: PluginHookSource | undefined): void { + source = next; +} + +export function getPluginHookSource(): PluginHookSource | undefined { + return source; +} diff --git a/packages/api/src/agents/run.ts b/packages/api/src/agents/run.ts index 800d3ccfe4..e59fd6ee50 100644 --- a/packages/api/src/agents/run.ts +++ b/packages/api/src/agents/run.ts @@ -59,6 +59,7 @@ import { getProviderConfig } from '~/endpoints/config/providers'; import { extractDefaultParams } from '~/endpoints/openai/llm'; import { resolveHeaders, createSafeUser } from '~/utils/env'; import { getAgentCheckpointer } from '~/agents/checkpointer'; +import { getPluginHookSource } from '~/agents/hooks/source'; import { getOpenAIConfig } from '~/endpoints/openai/config'; import { buildHITLRunWiring } from '~/agents/hitl/runtime'; import { buildLangfuseConfig } from '~/langfuse/config'; @@ -1187,6 +1188,7 @@ export async function createRun({ activityPhase, hitlCapable = false, toolInputValidationErrors, + sessionStartSource, streaming = true, streamUsage = true, }: { @@ -1274,6 +1276,8 @@ export async function createRun({ * final response / `[DONE]` with the tool call left unresolved). */ hitlCapable?: boolean; + /** Plugin-hook SessionStart lifecycle source: 'startup' (default) or 'resume' on HITL-rebuild paths. */ + sessionStartSource?: string; /** Request-scoped tool input failures consumed by the completion handler. */ toolInputValidationErrors?: Map; } & Pick< @@ -1645,6 +1649,34 @@ export async function createRun({ hooks.register('PreemptBoundary', { hooks: [steering.preemptHook] }); } } + /** + * Deployment-plugin hooks (Agent Plugins `ai.librechat/hooks/hooks.json`) + * register last so internal policy hooks (HITL, labels, steering) keep + * their ordering. The source is wired at startup by the plugins package + * (see `setPluginHookSource` in api/server/index.js) and stays empty + * unless the operator installed plugins with hook documents AND opted in + * via DEPLOYMENT_PLUGIN_HOOKS. The conversation id doubles as the plugin + * "session", giving SessionStart its once-per-conversation scope. + */ + const pluginHookSource = getPluginHookSource(); + if (pluginHookSource?.hasHooks() === true) { + hooks = hooks ?? new HookRegistry(); + const primaryAgent = agents[0]; + pluginHookSource.register({ + registry: hooks, + context: { + sessionId: requestBody?.conversationId, + userId: user?.id, + sessionStartSource, + model: primaryAgent?.model_parameters?.model ?? primaryAgent?.model ?? undefined, + agentType: primaryAgent?.id, + }, + // `ask` needs the checkpointer + resume surface; without HITL wiring the + // source tightens plugin `ask` decisions to `deny` rather than stranding + // the run on an un-resumable interrupt. + askDecisionSupported: hitl != null, + }); + } const streamLimits = resolveStreamLimits(agentsEndpointConfig); diff --git a/packages/api/src/plugins/constants.ts b/packages/api/src/plugins/constants.ts index 3ed7b66147..c592d6d48b 100644 --- a/packages/api/src/plugins/constants.ts +++ b/packages/api/src/plugins/constants.ts @@ -20,6 +20,7 @@ export const EXTENSION_HOOKS_FILE = 'hooks/hooks.json'; export const PLUGIN_ROOT_VAR = 'PLUGIN_ROOT'; export const PLUGIN_DATA_VAR = 'PLUGIN_DATA'; +export const DEPLOYMENT_PLUGIN_HOOKS_ENV = 'DEPLOYMENT_PLUGIN_HOOKS'; export const DEPLOYMENT_PLUGINS_DIR_ENV = 'DEPLOYMENT_PLUGINS_DIR'; export const DEFAULT_DEPLOYMENT_PLUGINS_DIR = 'plugin'; export const DEPLOYMENT_PLUGIN_DATA_DIR_ENV = 'DEPLOYMENT_PLUGIN_DATA_DIR'; diff --git a/packages/api/src/plugins/hooks.ts b/packages/api/src/plugins/hooks.ts index 943ab18aac..e3ab16b644 100644 --- a/packages/api/src/plugins/hooks.ts +++ b/packages/api/src/plugins/hooks.ts @@ -12,8 +12,8 @@ export interface PluginHooksResult { /** * Reports a package that declares hooks when the host has registered no hook - * capabilities. Nothing executes plugin hooks yet, and silently ignoring the - * document would leave an operator believing it runs. + * capabilities (execution not opted into via `DEPLOYMENT_PLUGIN_HOOKS`). + * Silently ignoring the document would leave an operator believing it runs. */ export async function reportUnexecutedHooks(realRoot: string): Promise { const location = `${LIBRECHAT_EXTENSION_NAMESPACE}/${EXTENSION_HOOKS_FILE}`; @@ -31,7 +31,7 @@ export async function reportUnexecutedHooks(realRoot: string): Promise { + it('returns true only the first time a key is marked within a scope', () => { + const store = createMemoryOnceStore(); + expect(store.markOnce('conversation-a', 'hook')).toBe(true); + expect(store.markOnce('conversation-a', 'hook')).toBe(false); + expect(store.markOnce('conversation-a', 'other-hook')).toBe(true); + expect(store.markOnce('conversation-b', 'hook')).toBe(true); + }); + + it('evicts the least-recently-active scope, never a touched one', () => { + const store = createMemoryOnceStore(2); + expect(store.markOnce('active', 'rare-hook')).toBe(true); + expect(store.markOnce('idle', 'hook')).toBe(true); + /** + * Touching refreshes the whole scope without marking any key, the way + * registration refreshes a conversation each turn even when no `once` + * handler matches; "idle" becomes the oldest scope. + */ + store.touch('active'); + expect(store.markOnce('new', 'hook')).toBe(true); + /** The rarely-marked key survived because its conversation stayed active. */ + expect(store.markOnce('active', 'rare-hook')).toBe(false); + /** Only the idle conversation was evicted and fires again. */ + expect(store.markOnce('idle', 'hook')).toBe(true); + }); +}); + +describe('setPluginHookOnceStore', () => { + afterEach(() => { + setPluginHookOnceStore(undefined); + }); + + it('installs a replacement store and restores a fresh default when cleared', () => { + const marked: string[] = []; + setPluginHookOnceStore({ + touch() {}, + markOnce(scope, key) { + marked.push(`${scope}:${key}`); + return true; + }, + }); + expect(getPluginHookOnceStore().markOnce('shared-scope', 'shared-key')).toBe(true); + expect(marked).toEqual(['shared-scope:shared-key']); + setPluginHookOnceStore(undefined); + expect(getPluginHookOnceStore().markOnce('fresh-scope', 'fresh')).toBe(true); + expect(getPluginHookOnceStore().markOnce('fresh-scope', 'fresh')).toBe(false); + }); +}); diff --git a/packages/api/src/plugins/once.ts b/packages/api/src/plugins/once.ts new file mode 100644 index 0000000000..ff58008f1c --- /dev/null +++ b/packages/api/src/plugins/once.ts @@ -0,0 +1,70 @@ +const DEFAULT_ONCE_CAPACITY = 10_000; + +/** + * Owner of cross-run "fired once" state for SessionStart and `once: true` + * plugin hooks, keyed by conversation scope. Naming the owner as a seam lets + * `/api` substitute a shared conversation-scoped store (e.g. the Redis/keyv + * cache layer) so once-state survives replicas; the default in-memory store + * is per-process by design — a multi-replica deployment over-fires rather + * than ever dropping a hook. + */ +export interface PluginHookOnceStore { + /** Marks the scope active, refreshing its retention. */ + touch(scope: string): void | Promise; + /** Records the key within the scope; resolves true only the first time. */ + markOnce(scope: string, key: string): boolean | Promise; +} + +/** + * Bounded in-memory store that retains and evicts whole conversation scopes, + * least-recently-active first. Hook registration touches its scope on every + * run, so an active conversation keeps all of its once-state — including + * keys of handlers that match only rarely — and eviction under the capacity + * bound (which counts conversations, not keys) only reaches the + * conversations idle longest. + */ +export function createMemoryOnceStore( + capacity: number = DEFAULT_ONCE_CAPACITY, +): PluginHookOnceStore { + const scopes = new Map>(); + const retain = (scope: string): Set => { + const existing = scopes.get(scope); + if (existing !== undefined) { + scopes.delete(scope); + scopes.set(scope, existing); + return existing; + } + if (scopes.size >= capacity) { + const oldest = scopes.keys().next().value; + if (oldest !== undefined) { + scopes.delete(oldest); + } + } + const created = new Set(); + scopes.set(scope, created); + return created; + }; + return { + touch(scope: string): void { + retain(scope); + }, + markOnce(scope: string, key: string): boolean { + const keys = retain(scope); + if (keys.has(key)) { + return false; + } + keys.add(key); + return true; + }, + }; +} + +let store: PluginHookOnceStore = createMemoryOnceStore(); + +export function setPluginHookOnceStore(next: PluginHookOnceStore | undefined): void { + store = next ?? createMemoryOnceStore(); +} + +export function getPluginHookOnceStore(): PluginHookOnceStore { + return store; +} diff --git a/packages/api/src/plugins/runtime.spec.ts b/packages/api/src/plugins/runtime.spec.ts new file mode 100644 index 0000000000..0a3660a293 --- /dev/null +++ b/packages/api/src/plugins/runtime.spec.ts @@ -0,0 +1,451 @@ +import fs from 'fs'; +import os from 'os'; +import path from 'path'; +import { HookRegistry, executeHooks } from '@librechat/agents'; +import { + getDeploymentPluginHookCapabilities, + registerDeploymentPluginHooks, + hasDeploymentPluginHooks, +} from './runtime'; +import { getPluginHookSource, setPluginHookSource } from '~/agents/hooks'; +import { initializeDeploymentPlugins } from './deployment'; +import { PLUGIN_MANIFEST_SCHEMA_ID } from './constants'; + +let base: string; +let pluginsDir: string; +let dataDir: string; + +const HOOKS_DOCUMENT = { + hooks: { + PreToolUse: [ + { + matcher: '^write_file$', + hooks: [ + { + type: 'command', + command: `printf '%s' '{"decision":"deny","reason":"guarded"}'`, + }, + ], + }, + { + matcher: '^(Write|Edit)$', + hooks: [ + { + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);const claudeShaped=p.tool_name==="Write"&&p.tool_input.file_path==="/workspace/report.md"&&p.tool_input.path===undefined;console.log(JSON.stringify(claudeShaped?{decision:"deny",reason:"alias-guarded"}:{}));});'`, + }, + ], + }, + ], + SessionStart: [ + { + hooks: [ + { type: 'command', command: 'echo started >> "$PLUGIN_DATA/starts.log"' }, + { type: 'command', command: 'echo sibling >> "$PLUGIN_DATA/starts.log"' }, + ], + }, + ], + }, +}; + +async function writePlugin(name: string, document: object = HOOKS_DOCUMENT): Promise { + const root = path.join(pluginsDir, name); + await fs.promises.mkdir(path.join(root, 'ai.librechat', 'hooks'), { recursive: true }); + await fs.promises.writeFile( + path.join(root, 'plugin.json'), + JSON.stringify({ $schema: PLUGIN_MANIFEST_SCHEMA_ID, name }), + ); + await fs.promises.writeFile( + path.join(root, 'ai.librechat', 'hooks', 'hooks.json'), + JSON.stringify(document), + ); +} + +async function initialize(): Promise { + await initializeDeploymentPlugins({ + projectRoot: base, + env: { + DEPLOYMENT_PLUGINS_DIR: pluginsDir, + DEPLOYMENT_PLUGIN_DATA_DIR: dataDir, + }, + hookCapabilities: getDeploymentPluginHookCapabilities({ DEPLOYMENT_PLUGIN_HOOKS: 'true' }), + }); +} + +async function fireRunStart(sessionId: string, sessionStartSource?: string): Promise { + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId, sessionStartSource } }); + await executeHooks({ + registry, + input: { hook_event_name: 'RunStart', runId: `run-${Math.random()}`, messages: [] }, + }); +} + +beforeEach(async () => { + base = await fs.promises.realpath( + await fs.promises.mkdtemp(path.join(os.tmpdir(), 'lc-plugin-runtime-')), + ); + pluginsDir = path.join(base, 'plugin'); + dataDir = path.join(base, 'data'); + await fs.promises.mkdir(pluginsDir, { recursive: true }); +}); + +afterEach(async () => { + await fs.promises.rm(base, { recursive: true, force: true }); + /** Unconfigured missing directories reset the module registry to empty. */ + await initializeDeploymentPlugins({ projectRoot: base, env: {} }); +}); + +describe('getDeploymentPluginHookCapabilities', () => { + it('returns undefined unless DEPLOYMENT_PLUGIN_HOOKS is enabled', () => { + expect(getDeploymentPluginHookCapabilities({})).toBeUndefined(); + expect( + getDeploymentPluginHookCapabilities({ DEPLOYMENT_PLUGIN_HOOKS: 'false' }), + ).toBeUndefined(); + expect( + getDeploymentPluginHookCapabilities({ DEPLOYMENT_PLUGIN_HOOKS: 'true' })?.handlerTypes.has( + 'command', + ), + ).toBe(true); + }); +}); + +describe('registerDeploymentPluginHooks', () => { + it('registers nothing when plugins loaded without hook capabilities', async () => { + await writePlugin('inert'); + await initializeDeploymentPlugins({ + projectRoot: base, + env: { DEPLOYMENT_PLUGINS_DIR: pluginsDir, DEPLOYMENT_PLUGIN_DATA_DIR: dataDir }, + }); + expect(hasDeploymentPluginHooks()).toBe(false); + const registry = new HookRegistry(); + expect(registerDeploymentPluginHooks({ registry })).toBe(0); + }); + + it('executes a plugin command hook end-to-end through a run hook registry', async () => { + await writePlugin('guard'); + await initialize(); + expect(hasDeploymentPluginHooks()).toBe(true); + + const registry = new HookRegistry(); + const registered = registerDeploymentPluginHooks({ + registry, + context: { sessionId: 'conversation-1' }, + }); + expect(registered).toBe(4); + + const result = await executeHooks({ + registry, + matchQuery: 'write_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'write_file', + toolInput: { path: '/etc/passwd' }, + toolUseId: 'tool-1', + }, + }); + expect(result).toEqual(expect.objectContaining({ decision: 'deny', reason: 'guarded' })); + + const unmatched = await executeHooks({ + registry, + matchQuery: 'read_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'read_file', + toolInput: {}, + toolUseId: 'tool-2', + }, + }); + expect(unmatched.decision).toBeUndefined(); + }); + + it('fires Claude-alias matchers against runtime tool names with plugin-namespace payloads', async () => { + await writePlugin('alias'); + await initialize(); + + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId: 'conversation-alias' } }); + + const result = await executeHooks({ + registry, + matchQuery: 'create_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'create_file', + toolInput: { path: '/workspace/report.md' }, + toolUseId: 'tool-1', + }, + }); + expect(result).toEqual(expect.objectContaining({ decision: 'deny', reason: 'alias-guarded' })); + }); + + it('keeps native payloads for matchers authored in the runtime namespace', async () => { + await writePlugin('native', { + hooks: { + PreToolUse: [ + { + matcher: '^create_file$', + hooks: [ + { + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);const nativeShaped=p.tool_name==="create_file"&&p.tool_input.path==="/workspace/native.md"&&p.tool_input.file_path===undefined;console.log(JSON.stringify(nativeShaped?{decision:"deny",reason:"native-guarded"}:{}));});'`, + }, + ], + }, + ], + }, + }); + await initialize(); + + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId: 'conversation-native' } }); + + const result = await executeHooks({ + registry, + matchQuery: 'create_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'create_file', + toolInput: { path: '/workspace/native.md' }, + toolUseId: 'tool-1', + }, + }); + expect(result).toEqual(expect.objectContaining({ decision: 'deny', reason: 'native-guarded' })); + }); + + it('keeps native payloads for the native alternative of a mixed-namespace matcher', async () => { + await writePlugin('mixed', { + hooks: { + PreToolUse: [ + { + matcher: 'Bash|create_file', + hooks: [ + { + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);const nativeShaped=p.tool_name==="create_file"&&p.tool_input.path==="/workspace/mixed.md"&&p.tool_input.file_path===undefined;console.log(JSON.stringify(nativeShaped?{decision:"deny",reason:"mixed-native"}:{}));});'`, + }, + ], + }, + ], + }, + }); + await initialize(); + + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId: 'conversation-mixed' } }); + + const result = await executeHooks({ + registry, + matchQuery: 'create_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'create_file', + toolInput: { path: '/workspace/mixed.md' }, + toolUseId: 'tool-1', + }, + }); + expect(result).toEqual(expect.objectContaining({ decision: 'deny', reason: 'mixed-native' })); + }); + + it('lets an overlapping once declaration fire after its sibling is spent', async () => { + const onceHandler = { + type: 'command', + command: 'echo fired >> "$PLUGIN_DATA/overlap.log"', + once: true, + }; + await writePlugin('overlap', { + hooks: { + PreToolUse: [ + { matcher: 'write_file|read_file', hooks: [onceHandler] }, + { matcher: '^read_file$', hooks: [onceHandler] }, + ], + }, + }); + await initialize(); + + const firePreToolUse = async (toolName: string) => { + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId: 'conversation-overlap' } }); + await executeHooks({ + registry, + matchQuery: toolName, + input: { + hook_event_name: 'PreToolUse', + runId: `run-${Math.random()}`, + toolName, + toolInput: {}, + toolUseId: 'tool-1', + }, + }); + }; + + /** Spends the broad declaration's once key. */ + await firePreToolUse('write_file'); + /** + * The spent broad declaration must not claim the per-input dedup slot, + * or the narrow declaration's independent once-key would never fire. + */ + await firePreToolUse('read_file'); + await firePreToolUse('read_file'); + + const log = await fs.promises.readFile(path.join(dataDir, 'overlap', 'overlap.log'), 'utf8'); + expect(log.trim().split('\n')).toHaveLength(2); + }); + + it('serves the run seam through the plugin hook source', async () => { + await writePlugin('seam'); + await initialize(); + setPluginHookSource({ + hasHooks: hasDeploymentPluginHooks, + register: registerDeploymentPluginHooks, + }); + const source = getPluginHookSource(); + expect(source?.hasHooks()).toBe(true); + const registry = new HookRegistry(); + expect(source?.register({ registry, context: { sessionId: 'conversation-seam' } })).toBe(4); + setPluginHookSource(undefined); + expect(getPluginHookSource()).toBeUndefined(); + }); + + it('fires every SessionStart handler once per conversation and per lifecycle source', async () => { + await writePlugin('session'); + await initialize(); + + await fireRunStart('conversation-a'); + await fireRunStart('conversation-a'); + await fireRunStart('conversation-b'); + /** A startup firing must not suppress the conversation's resume rebuild. */ + await fireRunStart('conversation-a', 'resume'); + await fireRunStart('conversation-a', 'resume'); + + const log = await fs.promises.readFile(path.join(dataDir, 'session', 'starts.log'), 'utf8'); + const lines = log.trim().split('\n').sort(); + expect(lines).toEqual(['sibling', 'sibling', 'sibling', 'started', 'started', 'started']); + }); + + it("reports the caller's working directory in hook payloads", async () => { + await writePlugin('cwd', { + hooks: { + PreToolUse: [ + { + matcher: '^write_file$', + hooks: [ + { + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);console.log(JSON.stringify({decision:"deny",reason:p.cwd+"|"+process.cwd()}));});'`, + }, + ], + }, + ], + }, + }); + await initialize(); + + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ + registry, + context: { sessionId: 'conversation-cwd', cwd: '/workspace/session' }, + }); + + const result = await executeHooks({ + registry, + matchQuery: 'write_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'write_file', + toolInput: {}, + toolUseId: 'tool-1', + }, + }); + /** Payload carries the session cwd; the process still runs from the plugin root. */ + expect(result).toEqual( + expect.objectContaining({ + reason: `/workspace/session|${path.join(pluginsDir, 'cwd')}`, + }), + ); + }); + + it('presents Claude payloads to matcherless tool guards', async () => { + await writePlugin('wildcard', { + hooks: { + PreToolUse: [ + { + hooks: [ + { + type: 'command', + command: `node -e 'let d="";process.stdin.on("data",(c)=>{d+=c;}).on("end",()=>{const p=JSON.parse(d);const claudeShaped=p.tool_name==="Write"&&p.tool_input.file_path==="/workspace/wild.md"&&p.tool_input.path===undefined;console.log(JSON.stringify(claudeShaped?{decision:"deny",reason:"wildcard-guarded"}:{}));});'`, + }, + ], + }, + ], + }, + }); + await initialize(); + + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId: 'conversation-wild' } }); + + const result = await executeHooks({ + registry, + matchQuery: 'create_file', + input: { + hook_event_name: 'PreToolUse', + runId: 'run-1', + toolName: 'create_file', + toolInput: { path: '/workspace/wild.md' }, + toolUseId: 'tool-1', + }, + }); + expect(result).toEqual( + expect.objectContaining({ decision: 'deny', reason: 'wildcard-guarded' }), + ); + }); + + it('persists once-only state per declaration across runs of the same conversation', async () => { + const onceHandler = { + type: 'command', + command: 'echo fired >> "$PLUGIN_DATA/once.log"', + once: true, + }; + await writePlugin('oncely', { + hooks: { + PreToolUse: [ + { matcher: '^write_file$', hooks: [onceHandler] }, + { matcher: '^read_file$', hooks: [onceHandler] }, + ], + }, + }); + await initialize(); + + const firePreToolUse = async (sessionId: string, toolName: string) => { + const registry = new HookRegistry(); + registerDeploymentPluginHooks({ registry, context: { sessionId } }); + await executeHooks({ + registry, + matchQuery: toolName, + input: { + hook_event_name: 'PreToolUse', + runId: `run-${Math.random()}`, + toolName, + toolInput: {}, + toolUseId: 'tool-1', + }, + }); + }; + + await firePreToolUse('conversation-once', 'write_file'); + await firePreToolUse('conversation-once', 'write_file'); + /** A sibling declaration with an identical handler is independently once-only. */ + await firePreToolUse('conversation-once', 'read_file'); + await firePreToolUse('conversation-other', 'write_file'); + + const log = await fs.promises.readFile(path.join(dataDir, 'oncely', 'once.log'), 'utf8'); + expect(log.trim().split('\n')).toHaveLength(3); + }); +}); diff --git a/packages/api/src/plugins/runtime.ts b/packages/api/src/plugins/runtime.ts new file mode 100644 index 0000000000..68cc391812 --- /dev/null +++ b/packages/api/src/plugins/runtime.ts @@ -0,0 +1,189 @@ +import { createHash } from 'node:crypto'; +import { logger } from '@librechat/data-schemas'; +import type { HookRegistry, HookOutput } from '@librechat/agents'; +import type { + PluginHookExecutionRequest, + PluginHookRuntimeContext, + PluginHookCapabilities, + PluginHookExecutor, + PluginHookHandler, +} from '~/agents/hooks'; +import type { LoadedPlugin } from './types'; +import { + commandExecutorCapabilities, + createCommandExecutor, + registerPluginHooks, +} from '~/agents/hooks'; +import { getDeploymentPluginRegistry } from './deployment'; +import { DEPLOYMENT_PLUGIN_HOOKS_ENV } from './constants'; +import { getPluginHookOnceStore } from './once'; +import { isEnabled } from '~/utils/common'; + +const KEY_SEPARATOR = '\u0000'; + +/** + * Compact stable digest of a handler declaration. The declaration indexes + * already identify the handler positionally; this distinguishes an edited + * handler at the same position without storing its full body — commands run + * to 32 KB and may carry 256 similarly sized args, which would otherwise be + * copied into every retained conversation scope. + */ +function handlerIdentity(handler: PluginHookHandler): string { + const canonical = JSON.stringify( + Object.entries(handler).sort(([left], [right]) => left.localeCompare(right)), + ); + return createHash('sha256').update(canonical).digest('base64url').slice(0, 22); +} + +/** Conversation scope: caller-supplied session ids cannot collide across principals. */ +function onceScope(userId: string | undefined, sessionId: string): string { + return [userId ?? '', sessionId].join(KEY_SEPARATOR); +} + +function onceKey(pluginId: string, request: PluginHookExecutionRequest): string { + return [ + pluginId, + request.sourceEvent, + /** SessionStart dedupes per lifecycle source: startup must not suppress resume. */ + request.payload.source ?? '', + String(request.groupIndex), + String(request.handlerIndex), + handlerIdentity(request.handler), + ].join(KEY_SEPARATOR); +} + +/** + * Hook registration is per-run, so the runtime's own SessionStart and `once` + * dedup only spans one run. The once store extends both across runs of the + * same conversation (see `once.ts` for the store's ownership and eviction + * contract). Keys carry the declaration position and handler identity so one + * handler firing never suppresses a sibling declared on the same event. + * Suppression happens in `shouldExecute` — before the runtime's per-input + * dedup slot is claimed — so a spent declaration never shadows an identical + * handler declared under an overlapping matcher. + */ +function withOnceDedup( + pluginId: string, + userId: string | undefined, + executor: PluginHookExecutor, +): PluginHookExecutor { + return { + capabilities: executor.capabilities, + shouldExecute(request): boolean | Promise { + const oncePerSession = + request.sourceEvent === 'SessionStart' || request.handler.once === true; + if (!oncePerSession) { + return true; + } + const scope = onceScope(userId, request.payload.session_id); + const first = getPluginHookOnceStore().markOnce(scope, onceKey(pluginId, request)); + if (first instanceof Promise) { + return first.catch((error) => { + /** A failed store lookup fails open — over-firing is the store's documented direction. */ + logger.warn(`[pluginHooks] Once-store lookup failed for plugin "${pluginId}"`, error); + return true; + }); + } + return first; + }, + execute(request, signal): HookOutput | Promise { + return executor.execute(request, signal); + }, + }; +} + +/** + * Capabilities handed to plugin loading when the operator has opted in to + * hook execution via `DEPLOYMENT_PLUGIN_HOOKS`. Undefined (the default) keeps + * hook documents parsed-but-inert, with the existing "not executed" warning. + */ +export function getDeploymentPluginHookCapabilities( + env: NodeJS.ProcessEnv = process.env, +): PluginHookCapabilities | undefined { + return isEnabled(env[DEPLOYMENT_PLUGIN_HOOKS_ENV]) ? commandExecutorCapabilities : undefined; +} + +function getExecutableHookPlugins(): LoadedPlugin[] { + return getDeploymentPluginRegistry() + .list() + .filter((plugin) => (plugin.hooks?.plan.summary.ready ?? 0) > 0); +} + +export function hasDeploymentPluginHooks(): boolean { + return getExecutableHookPlugins().length > 0; +} + +export interface RegisterDeploymentPluginHooksOptions { + registry: HookRegistry; + context?: PluginHookRuntimeContext; + /** + * Whether the run has a HITL approval surface (checkpointer + resume route). + * Off by default: without it a plugin's `ask` decision is tightened to + * `deny`, since an un-resumable interrupt would strand the run. + */ + askDecisionSupported?: boolean; +} + +/** + * Registers every loaded deployment plugin's ready hooks onto a run's hook + * registry. Called once per run from the run-construction seam; the registry + * (and with it every registration) is garbage-collected with the run. + * Returns the number of handlers registered. + */ +export function registerDeploymentPluginHooks( + options: RegisterDeploymentPluginHooksOptions, +): number { + const sessionId = options.context?.sessionId; + if (sessionId !== undefined) { + /** + * Refreshes the conversation's once-state retention on every run, so even + * rarely-matching `once` handlers keep their keys while the conversation + * stays active. A failed refresh only risks earlier eviction, so an async + * store's rejection is logged rather than failing run construction. + */ + const touched = getPluginHookOnceStore().touch(onceScope(options.context?.userId, sessionId)); + if (touched instanceof Promise) { + touched.catch((error) => logger.warn('[pluginHooks] Once-store touch failed', error)); + } + } + let registered = 0; + for (const plugin of getExecutableHookPlugins()) { + const document = plugin.hooks?.document; + const plan = plugin.hooks?.plan; + if (document === undefined || plan === undefined) { + continue; + } + const pluginId = plugin.manifest.name; + const executor = withOnceDedup( + pluginId, + options.context?.userId, + createCommandExecutor({ + pluginRoot: plugin.root, + pluginData: plugin.dataDirectory, + allowAskDecision: options.askDecisionSupported === true, + }), + ); + try { + const registration = registerPluginHooks({ + pluginId, + registry: options.registry, + document, + /** Load-time plan, computed with the same static executor capabilities. */ + plan, + executor, + /** + * The caller's working directory passes through untouched: payload + * `cwd` reports the run's session context, which a guard may use to + * resolve relative tool paths. The plugin's own installation path + * reaches commands as `PLUGIN_ROOT`/`CLAUDE_PLUGIN_ROOT`, and the + * executor separately runs each process from that directory. + */ + context: options.context, + }); + registered += registration.registered; + } catch (error) { + logger.error(`[pluginHooks] Failed to register hooks for plugin "${pluginId}"`, error); + } + } + return registered; +} diff --git a/packages/api/src/plugins/types.ts b/packages/api/src/plugins/types.ts index 5facb6a589..75ae158ddc 100644 --- a/packages/api/src/plugins/types.ts +++ b/packages/api/src/plugins/types.ts @@ -1,5 +1,5 @@ import type { MCPOptions } from 'librechat-data-provider'; -import type { PluginHookPlan } from '~/agents/hooks'; +import type { PluginHookPlan, PluginHooksDocument } from '~/agents/hooks'; import type { MCP_PLUGIN_SOURCE } from '~/utils/env'; import type { JsonValue } from '~/agents/envelope'; import type { DeploymentSkill } from '~/skills'; @@ -74,6 +74,8 @@ export interface PluginMcpServer { /** Hooks contributed through the `ai.librechat` extension directory. */ export interface PluginHookContribution { plan: PluginHookPlan; + /** Parsed source document, re-planned at registration so plan and runtime never drift. */ + document: PluginHooksDocument; location: string; }