diff --git a/.github/workflows/hooks.yaml b/.github/workflows/hooks.yaml new file mode 100644 index 0000000..9233e74 --- /dev/null +++ b/.github/workflows/hooks.yaml @@ -0,0 +1,58 @@ +name: Hooks + +on: + pull_request: + paths: + - 'hooks/**' + - 'viz/hooks/**' + - 'tests/**' + - '.github/workflows/hooks.yaml' + push: + branches: + - main + paths: + - 'hooks/**' + - 'viz/hooks/**' + - 'tests/**' + - '.github/workflows/hooks.yaml' + +permissions: + contents: read + +jobs: + test: + name: test (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [macos-latest, ubuntu-latest, windows-latest] + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + # git does not set NTFS permission bits on checkout, so the exec bit the + # repo records is not observable on Windows. Git Bash reports .sh files as + # executable anyway, which is what Claude Code relies on there. + - name: Run hook tests + shell: bash + run: bash tests/hooks_test.sh + + # The bug in #27 is that ${CLAUDE_PLUGIN_ROOT} reaches the hook as a native + # Windows path. Git Bash reports $PWD in Unix form, so the job above would + # never see that shape. This one injects the backslash path GitHub gives us, + # which is what Claude Code actually substitutes on Windows. + windows-native-plugin-root: + name: test (windows-latest, native plugin root) + runs-on: windows-latest + steps: + - name: Checkout repository + uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + + - name: Run hook tests with a backslash CLAUDE_PLUGIN_ROOT + shell: bash + env: + CLAUDE_PLUGIN_ROOT: ${{ github.workspace }} + run: | + echo "CLAUDE_PLUGIN_ROOT=$CLAUDE_PLUGIN_ROOT" + bash tests/hooks_test.sh diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..3a3ef81 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,119 @@ +# CLAUDE.md + +Guidance for Claude Code when working in this repository. + +## What this repository is + +The `datadog` plugin for Claude Code: a preconfigured Datadog MCP server, a set +of skills, and a macOS-only visualization panel (`ddviz`). It is published to +the `anthropics/claude-plugins-official` marketplace. + +**This repo is a downstream artifact.** It is regenerated from an internal +Datadog source repository on each release. External contributions are reviewed +here and then ported back internally. Two consequences worth keeping in mind: + +- Keep changes focused and self-explanatory. Someone has to re-apply them by + hand on the other side. +- The test suite that covers most of this code lives in the internal repo, not + here. `forward.sh` still carries a `template.` fallback path for that source + tree. Don't delete it because it looks dead — it is live upstream. + +## Layout + +``` +.claude-plugin/plugin.json Plugin manifest (name, version, MCP server path) +.dd_claude-code_mcp.json MCP server definition +hooks/hooks.json Hook registrations +viz/hooks/ ddviz hook scripts (forward.sh, session_end.sh) +viz/ddviz.swift The macOS panel daemon +skills/ Skills, each a SKILL.md plus optional scripts/ +tests/hooks_test.sh Hook wiring tests (see Testing) +``` + +## Hooks: use shell form, never a bare interpreter + +`hooks/hooks.json` must invoke scripts in **shell form** — a `command` string +with no `args` key: + +```json +{ "type": "command", "command": "\"${CLAUDE_PLUGIN_ROOT}\"/viz/hooks/forward.sh" } +``` + +Not exec form: + +```json +{ "type": "command", "command": "bash", "args": ["${CLAUDE_PLUGIN_ROOT}/viz/hooks/forward.sh"] } +``` + +The difference matters on Windows. Adding `args` switches Claude Code to exec +form, which resolves `command` against `PATH` and spawns it with no shell. On a +default Windows install `bash` on `PATH` is `C:\Windows\System32\bash.exe` — the +**WSL launcher**, not Git Bash. It reads the Windows `${CLAUDE_PLUGIN_ROOT}` as +escape sequences, the script is never found, and the hook fails on every +invocation. That was issue #27: a `SessionEnd` hook whose only job off macOS is +to do nothing still printed an error on every exit. + +Shell form hands the string to `sh -c` on macOS and Linux and to **Git Bash** on +Windows, which resolves the path correctly. + +Two rules follow: + +1. **Hook scripts must be executable** (mode `755`). Shell form runs the file + directly, so mode `644` exits `126` with "Permission denied". +2. **There is no OS conditional for hook registration.** The `if` field takes + permission-rule syntax and is only evaluated on tool events — on `SessionEnd` + and friends, a hook with `if` set *never runs at all*. So a hook that is + macOS-only cannot be unregistered elsewhere; it must be cheap and silent + instead. Return `0` and print nothing on the platforms you don't serve. + +Skill scripts are different: `SKILL.md` tells the model to run +`bash "${CLAUDE_PLUGIN_ROOT}/skills/.../foo.sh"` through the **Bash tool**, +which already uses Git Bash on Windows. Those stay mode `644` and keep the +explicit `bash` prefix. + +## Platform support + +`ddviz` is macOS-only — it needs Swift, `xcode-select`, and `/usr/bin/nc`. The +two hooks behave differently off macOS and this is deliberate: + +- `session_end.sh` has nothing to do. It hits the `*)` branch and exits `0`. +- `forward.sh` **does** have work to do. It calls `bail unsupported_os`, which + tells the model the chart is available at the sandbox URL. Never "optimize" + this into an early exit — the fallback message is the whole point off macOS. + +## Telemetry + +`emit_event` posts a failure-reason code and nothing else: no queries, results, +user, session, or install id. It honors `DO_NOT_TRACK` and `DISABLE_TELEMETRY`. + +**Set `DO_NOT_TRACK=1` in anything that exercises these scripts**, including +tests, so a local run never posts to Datadog's intake. + +## Testing + +```bash +bash tests/hooks_test.sh # requires jq +``` + +Covers hook wiring: that `hooks.json` parses, that no hook uses exec form with a +bare interpreter, that every referenced script exists and is executable, and +that both hooks exit `0` on the host. It stubs `uname` to check the +`MINGW64_NT` / `MSYS_NT` / `CYGWIN_NT` / `Linux` paths are silent no-ops. + +CI runs it on macOS, Linux and Windows. One Windows job injects +`CLAUDE_PLUGIN_ROOT` as a native backslash path, because Git Bash reports `$PWD` +in Unix form and would otherwise never exercise the path shape that caused #27. + +The suite makes no network calls and never opens the ddviz socket. + +## Conventions + +- Shell scripts target bash and must work under Git Bash. Keep them POSIX-ish; + macOS ships bash 3.2, so no `declare -A`, no `${var^^}`. +- JSON is read through the `jq`-or-`plutil` helpers in `ddviz_json.sh`. Don't + assume `jq` is installed — every helper has a `plutil` branch and a "no JSON + engine" return code. +- A hook must never break the session. Fail open: exit `0` and let the tool + result through. +- Don't hand-edit `version` in `plugin.json`; releases are cut by + `.github/workflows/release.yaml`. diff --git a/hooks/hooks.json b/hooks/hooks.json index 7dae625..0558bd0 100644 --- a/hooks/hooks.json +++ b/hooks/hooks.json @@ -4,12 +4,12 @@ "PostToolUse": [ { "matcher": "^mcp__plugin_datadog_mcp__(get_widget|visualize_tabular_data)$", - "hooks": [{ "type": "command", "command": "bash", "args": ["${CLAUDE_PLUGIN_ROOT}/viz/hooks/forward.sh"] }] + "hooks": [{ "type": "command", "command": "\"${CLAUDE_PLUGIN_ROOT}\"/viz/hooks/forward.sh" }] } ], "SessionEnd": [ { - "hooks": [{ "type": "command", "command": "bash", "args": ["${CLAUDE_PLUGIN_ROOT}/viz/hooks/session_end.sh"] }] + "hooks": [{ "type": "command", "command": "\"${CLAUDE_PLUGIN_ROOT}\"/viz/hooks/session_end.sh" }] } ] } diff --git a/tests/hooks_test.sh b/tests/hooks_test.sh new file mode 100755 index 0000000..6856d41 --- /dev/null +++ b/tests/hooks_test.sh @@ -0,0 +1,199 @@ +#!/usr/bin/env bash +# Tests for hooks/hooks.json and the viz hook scripts. +# +# Runs on macOS, Linux and Git Bash. Nothing here touches the network or the +# ddviz socket: telemetry is opted out via DO_NOT_TRACK, and the non-Darwin +# paths under test never open a socket in the first place. +# +# The Windows failure in #27 cannot be reproduced on a Unix host -- it depends +# on `bash` resolving to the WSL launcher. So the Windows-specific check is a +# contract assertion on hooks.json, and the behavioural checks confirm the +# hooks work on whichever host is running the suite. Run it on windows-latest +# and the behavioural checks cover the real thing. + +set -u + +REPO_ROOT=$(cd "$(dirname "$0")/.." && pwd) +HOOKS_JSON="$REPO_ROOT/hooks/hooks.json" + +# What Claude Code would substitute for ${CLAUDE_PLUGIN_ROOT}. Defaults to this +# checkout; CI overrides it on Windows with a native backslash path +# (D:\a\repo\repo) so the suite exercises the shape of plugin root that #27 +# reported, rather than the Unix-style path Git Bash reports for $PWD. +PLUGIN_ROOT="${CLAUDE_PLUGIN_ROOT:-$REPO_ROOT}" + +PASS=0 +FAIL=0 + +ok() { PASS=$((PASS + 1)); printf ' ok %s\n' "$1"; } +fail() { FAIL=$((FAIL + 1)); printf ' FAIL %s\n' "$1"; [ $# -gt 1 ] && printf ' %s\n' "$2"; } + +command -v jq >/dev/null 2>&1 || { echo "jq is required to run this suite"; exit 2; } + +# A directory whose `uname` reports whatever we need, prepended to PATH so the +# hooks' own `$(uname)` resolves to it. +stub_uname_path() { + local dir; dir=$(mktemp -d) + printf '#!/bin/sh\necho %s\n' "$1" > "$dir/uname" + chmod 755 "$dir/uname" + printf '%s' "$dir" +} + +# Run the hook declared for $1 the way Claude Code would, reading the payload on +# stdin. Shell form goes to a shell, which expands ${CLAUDE_PLUGIN_ROOT} from +# the environment; exec form is spawned directly with no shell. Supporting both +# means this suite reports honestly against the pre-fix config too, rather than +# failing it for the wrong reason. +run_hook() { + local event="$1" form command + form=$(jq -r --arg e "$event" '.hooks[$e][0].hooks[0] | if has("args") then "exec" else "shell" end' "$HOOKS_JSON") + command=$(jq -r --arg e "$event" '.hooks[$e][0].hooks[0].command' "$HOOKS_JSON") + + if [ "$form" = shell ]; then + sh -c "$command" + else + local -a argv=() + local a + while IFS= read -r a; do + # Parameter expansion, not sed: sed treats backslashes in the replacement + # as escapes, so a Windows ${CLAUDE_PLUGIN_ROOT} would be eaten here -- + # the same class of bug this suite exists to catch. + argv+=("${a/\$\{CLAUDE_PLUGIN_ROOT\}/$PLUGIN_ROOT}") + done < <(jq -r --arg e "$event" '.hooks[$e][0].hooks[0].args[]' "$HOOKS_JSON") + "$command" "${argv[@]}" + fi +} + +SESSION_PAYLOAD='{"session_id":"test-session","hook_event_name":"SessionEnd","reason":"exit"}' +TOOL_PAYLOAD='{"hook_event_name":"PostToolUse","tool_response":{"content_text":"chart"}}' + +echo "hooks.json is well formed" + +if jq empty "$HOOKS_JSON" 2>/dev/null; then + ok "parses as JSON" +else + fail "parses as JSON" +fi + +echo +echo "hook invocation is safe on Windows (#27)" + +# Exec form resolves `command` against PATH with no shell. On a default Windows +# install `bash` is C:\Windows\System32\bash.exe -- the WSL launcher, which +# reads the Windows ${CLAUDE_PLUGIN_ROOT} as escape sequences and never finds +# the script. Shell form hands the string to Git Bash instead, which resolves +# it correctly. +BARE=$(jq -r ' + .hooks | to_entries[] | .key as $event | .value[] | .hooks[] + | select(has("args")) + | select(.command | test("^(bash|sh|zsh|dash|python|python3|node)$")) + | "\($event): command=\(.command)"' "$HOOKS_JSON") +if [ -z "$BARE" ]; then + ok "no hook spawns a bare interpreter name in exec form" +else + fail "no hook spawns a bare interpreter name in exec form" "$BARE" +fi + +# Exec form keeps the script in args[0]; shell form carries it in the command +# string. +HOOKS=$(jq -r ' + .hooks | to_entries[] | .key as $event | .value[] | .hooks[] + | if has("args") + then [$event, "exec", (.args[0] // "")] + else [$event, "shell", (.command // "")] + end | @tsv' "$HOOKS_JSON") + +# Resolved against this checkout rather than $PLUGIN_ROOT: whether a Windows +# path survives is what the behavioural runs below cover, while these two checks +# are about the files themselves. +while IFS="$(printf '\t')" read -r event form script; do + [ -n "$script" ] || continue + script=${script//\"/} # shell form quotes the variable + script=${script//$'\r'/} # CRLF checkouts on Windows + rel=${script#*\$\{CLAUDE_PLUGIN_ROOT\}} # -> /viz/hooks/forward.sh + rel=${rel#/} + + if [ -f "$REPO_ROOT/$rel" ]; then + ok "$event: script exists" + else + fail "$event: script exists" "$REPO_ROOT/$rel" + continue + fi + + # Only shell form runs the file itself; exec form hands it to an interpreter, + # so the mode does not matter there. Asserted against the mode git records, + # not the working tree: git does not materialise permission bits on Windows, + # so -x there says nothing, while the recorded mode is what ships to users. + if [ "$form" = shell ]; then + mode=$(cd "$REPO_ROOT" && git ls-files -s -- "$rel" 2>/dev/null | awk '{print $1}') + if [ "$mode" = 100755 ]; then + ok "$event: script is executable in git ($mode)" + else + fail "$event: script is executable in git" "got ${mode:-}; shell form runs the file directly, and mode 644 exits 126" + fi + fi +done <&1) +status=$? +if [ $status -eq 0 ]; then + ok "SessionEnd exits 0 on $(uname -s)" +else + fail "SessionEnd exits 0 on $(uname -s)" "exit=$status output=$out" +fi +if [ -z "$out" ]; then + ok "SessionEnd prints nothing" +else + fail "SessionEnd prints nothing" "$out" +fi + +echo +echo "the non-macOS no-op is reachable and silent" + +# Off Darwin the hook has no work to do. Once it can actually be spawned it must +# reach that branch and say nothing -- the visible symptom in #27 was the hook +# failing before it ever got there. +for os in MINGW64_NT-10.0 MSYS_NT-10.0 CYGWIN_NT-10.0 Linux; do + stub=$(stub_uname_path "$os") + out=$(printf '%s' "$SESSION_PAYLOAD" | PATH="$stub:$PATH" CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" \ + DO_NOT_TRACK=1 run_hook SessionEnd 2>&1) + status=$? + rm -rf "$stub" + if [ $status -eq 0 ] && [ -z "$out" ]; then + ok "SessionEnd is a silent no-op when uname reports $os" + else + fail "SessionEnd is a silent no-op when uname reports $os" "exit=$status output=$out" + fi +done + +echo +echo "PostToolUse still falls back off macOS" + +# forward.sh, unlike session_end.sh, has real work to do off macOS: it tells the +# model the chart is available at the sandbox URL. Dropping the registration on +# non-macOS would lose that, so it must stay registered and exit 0 with the +# fallback rather than be skipped. +stub=$(stub_uname_path Linux) +out=$(printf '%s' "$TOOL_PAYLOAD" | PATH="$stub:$PATH" CLAUDE_PLUGIN_ROOT="$PLUGIN_ROOT" \ + CLAUDE_CODE_ENTRYPOINT=cli DO_NOT_TRACK=1 DISABLE_TELEMETRY=1 run_hook PostToolUse 2>&1) +status=$? +rm -rf "$stub" +if [ $status -eq 0 ]; then + ok "PostToolUse exits 0 off macOS" +else + fail "PostToolUse exits 0 off macOS" "exit=$status output=$out" +fi +if printf '%s' "$out" | jq -e '.hookSpecificOutput.additionalContext | test("sandbox URL")' >/dev/null 2>&1; then + ok "PostToolUse returns the sandbox-URL fallback off macOS" +else + fail "PostToolUse returns the sandbox-URL fallback off macOS" "$out" +fi + +echo +echo "$PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ] diff --git a/viz/hooks/forward.sh b/viz/hooks/forward.sh old mode 100644 new mode 100755 diff --git a/viz/hooks/session_end.sh b/viz/hooks/session_end.sh old mode 100644 new mode 100755