From 5c9ef01aa0f6e7f1314ebe6a8cdbea21f9b4ef85 Mon Sep 17 00:00:00 2001 From: Matt Norris Date: Mon, 20 Jul 2026 16:42:30 -0400 Subject: [PATCH 1/2] feat(skills): add portable agent skills Add four Agent Skills under skills/ess/ (pr-address-comments, pr-address-comments-all, mcp-hide-secrets, summarize-change-log), each with a LICENSE, plus skills/README.md with an install matrix and a validate-skills workflow that runs skills-ref validate on every skill under skills/. --- .github/workflows/validate-skills.yml | 40 +++ .gitignore | 6 + skills/README.md | 49 ++++ skills/ess/mcp-hide-secrets/LICENSE | 13 + skills/ess/mcp-hide-secrets/SKILL.md | 103 +++++++ .../references/install-cursor.md | 44 +++ .../references/launchagent-naming.md | 37 +++ .../mcp-hide-secrets/references/launchctl.md | 98 +++++++ .../mcp-hide-secrets/references/rationale.md | 119 ++++++++ .../scripts/check-inline-secrets.py | 62 ++++ .../scripts/fix-launchagents-dir.sh | 47 +++ .../ess/mcp-hide-secrets/scripts/install.sh | 205 +++++++++++++ skills/ess/mcp-hide-secrets/scripts/lib.sh | 19 ++ .../mcp-hide-secrets/scripts/load-mcp-env.sh | 71 +++++ .../local.your-username.cursor-mcp-env.plist | 21 ++ .../mcp-hide-secrets/scripts/mcp.env.example | 11 + .../ess/mcp-hide-secrets/scripts/preflight.sh | 65 +++++ skills/ess/mcp-hide-secrets/scripts/run.sh | 114 ++++++++ skills/ess/mcp-hide-secrets/scripts/status.sh | 58 ++++ skills/ess/mcp-hide-secrets/scripts/verify.sh | 37 +++ skills/ess/pr-address-comments-all/LICENSE | 13 + skills/ess/pr-address-comments-all/SKILL.md | 240 ++++++++++++++++ .../references/subagent-prompts.md | 82 ++++++ .../references/worktree-mechanics.md | 90 ++++++ .../scripts/create_pr_worktree.sh | 115 ++++++++ .../scripts/fetch_pr.sh | 156 ++++++++++ .../scripts/find_my_prs.sh | 126 ++++++++ .../scripts/reply_and_resolve.sh | 66 +++++ .../scripts/safe_push.sh | 111 ++++++++ skills/ess/pr-address-comments/LICENSE | 13 + skills/ess/pr-address-comments/SKILL.md | 269 ++++++++++++++++++ skills/ess/summarize-change-log/LICENSE | 13 + skills/ess/summarize-change-log/SKILL.md | 91 ++++++ .../ess/summarize-change-log/examples-good.md | 5 + skills/ess/summarize-change-log/examples.md | 52 ++++ .../scripts/test_validate_summary.py | 96 +++++++ .../scripts/validate-summary.sh | 11 + .../scripts/validate_summary.py | 185 ++++++++++++ 38 files changed, 2953 insertions(+) create mode 100644 .github/workflows/validate-skills.yml create mode 100644 skills/README.md create mode 100644 skills/ess/mcp-hide-secrets/LICENSE create mode 100644 skills/ess/mcp-hide-secrets/SKILL.md create mode 100644 skills/ess/mcp-hide-secrets/references/install-cursor.md create mode 100644 skills/ess/mcp-hide-secrets/references/launchagent-naming.md create mode 100644 skills/ess/mcp-hide-secrets/references/launchctl.md create mode 100644 skills/ess/mcp-hide-secrets/references/rationale.md create mode 100755 skills/ess/mcp-hide-secrets/scripts/check-inline-secrets.py create mode 100755 skills/ess/mcp-hide-secrets/scripts/fix-launchagents-dir.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/install.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/lib.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/load-mcp-env.sh create mode 100644 skills/ess/mcp-hide-secrets/scripts/local.your-username.cursor-mcp-env.plist create mode 100644 skills/ess/mcp-hide-secrets/scripts/mcp.env.example create mode 100755 skills/ess/mcp-hide-secrets/scripts/preflight.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/run.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/status.sh create mode 100755 skills/ess/mcp-hide-secrets/scripts/verify.sh create mode 100644 skills/ess/pr-address-comments-all/LICENSE create mode 100644 skills/ess/pr-address-comments-all/SKILL.md create mode 100644 skills/ess/pr-address-comments-all/references/subagent-prompts.md create mode 100644 skills/ess/pr-address-comments-all/references/worktree-mechanics.md create mode 100755 skills/ess/pr-address-comments-all/scripts/create_pr_worktree.sh create mode 100755 skills/ess/pr-address-comments-all/scripts/fetch_pr.sh create mode 100755 skills/ess/pr-address-comments-all/scripts/find_my_prs.sh create mode 100755 skills/ess/pr-address-comments-all/scripts/reply_and_resolve.sh create mode 100755 skills/ess/pr-address-comments-all/scripts/safe_push.sh create mode 100644 skills/ess/pr-address-comments/LICENSE create mode 100644 skills/ess/pr-address-comments/SKILL.md create mode 100644 skills/ess/summarize-change-log/LICENSE create mode 100644 skills/ess/summarize-change-log/SKILL.md create mode 100644 skills/ess/summarize-change-log/examples-good.md create mode 100644 skills/ess/summarize-change-log/examples.md create mode 100644 skills/ess/summarize-change-log/scripts/test_validate_summary.py create mode 100755 skills/ess/summarize-change-log/scripts/validate-summary.sh create mode 100644 skills/ess/summarize-change-log/scripts/validate_summary.py diff --git a/.github/workflows/validate-skills.yml b/.github/workflows/validate-skills.yml new file mode 100644 index 0000000..18a5afb --- /dev/null +++ b/.github/workflows/validate-skills.yml @@ -0,0 +1,40 @@ +# GitHub Action: Validate Agent Skills +# +# Validates every skill under skills/ (any depth) against the agentskills.io spec using +# skills-ref (https://github.com/agentskills/agentskills). Fails if any skill's +# SKILL.md frontmatter or referenced files are invalid. +# +# Maintainer: Matt Norris +# +# For more information on Agent Skills, see: https://agentskills.io/specification + +name: Validate Skills +on: + workflow_dispatch: + pull_request: + types: [opened, reopened, synchronize] + paths: + - 'skills/**' + - '.github/workflows/validate-skills.yml' + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Install uv + uses: astral-sh/setup-uv@v5 + + - name: Validate each skill + run: | + set -euo pipefail + status=0 + while IFS= read -r skill_md; do + skill="$(dirname "$skill_md")" + echo "== validating ${skill} ==" + uvx --from git+https://github.com/agentskills/agentskills#subdirectory=skills-ref \ + skills-ref validate "$skill" || status=1 + done < <(find skills -type f -name SKILL.md | sort) + exit $status diff --git a/.gitignore b/.gitignore index f61e873..4d3dc49 100644 --- a/.gitignore +++ b/.gitignore @@ -274,3 +274,9 @@ output/ # Claude .claude/settings.json + +# Agent-skills install output from `npx skills add ./skills` (source of truth is +# skills/ess/). The CLI installs to .agents/skills/ and writes a machine-specific +# lock file. +.agents/skills/ +skills-lock.json diff --git a/skills/README.md b/skills/README.md new file mode 100644 index 0000000..55db9d7 --- /dev/null +++ b/skills/README.md @@ -0,0 +1,49 @@ +# Skills + +[Agent Skills](https://agentskills.io) — portable `SKILL.md` workflows that work across Cursor, Claude Code, Codex, and other compatible agents. + +The skills live under [`ess/`](ess/). Each is self-contained (`SKILL.md` plus optional `scripts/`, `references/`, and a `LICENSE`) and host-neutral: no IDE-specific frontmatter committed. + +## Install + +Install with the standard agent-skills CLI ([skills.sh](https://skills.sh)). It auto-detects your agent and places files in the right directory. + +From a local clone (run in the repo root): + +```bash +npx skills add ./skills --skill '*' # all skills +npx skills add ./skills --skill mcp-hide-secrets # a single skill +npx skills add ./skills --list # list without installing +``` + +Or straight from GitHub (replace `` with the repo owner): + +```bash +npx skills add /essentials --skill '*' +``` + +### Install matrix + +| Agent | Project path | Global (`-g`) | +| ----- | ------------ | ------------- | +| Cursor | `.agents/skills//` (also reads `.cursor/skills//`) | `~/.cursor/skills//` | +| Claude Code | `.claude/skills//` | `~/.claude/skills//` | +| Codex | `.agents/skills//` | `~/.agents/skills//` | + +## Skills + +| Skill | What it does | +| ----- | ------------ | +| [`ess/pr-address-comments`](ess/pr-address-comments/) | Read a remote GitHub PR's review comments, implement fixes, then reply to and resolve the addressed threads. | +| [`ess/pr-address-comments-all`](ess/pr-address-comments-all/) | Address review comments across one or more PRs in parallel, each in its own git worktree on the PR's branch. | +| [`ess/mcp-hide-secrets`](ess/mcp-hide-secrets/) | Move Cursor MCP credentials out of `mcp.json` into a login-loaded `mcp.env` (macOS LaunchAgent). | +| [`ess/summarize-change-log`](ess/summarize-change-log/) | Condense a long git log or squash message into 1–5 Conventional-Commit bullets. | + +## Notes + +- Cursor-only behavior such as `disable-model-invocation: true` (slash-command-only activation) is kept out of the committed `SKILL.md`. To enable it locally, see [`ess/mcp-hide-secrets/references/install-cursor.md`](ess/mcp-hide-secrets/references/install-cursor.md). +- `pr-address-comments-all` invokes its scripts from their committed location (`skills/ess/pr-address-comments-all/scripts/`), so they are present in every checkout and worktree of a repo that vendors these skills. + +## License + +[Apache 2.0](../LICENSE) — each skill also carries its own `LICENSE`. diff --git a/skills/ess/mcp-hide-secrets/LICENSE b/skills/ess/mcp-hide-secrets/LICENSE new file mode 100644 index 0000000..9c7afe3 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/LICENSE @@ -0,0 +1,13 @@ +Copyright 2025 Cisco Systems, Inc. or its Affiliates + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. diff --git a/skills/ess/mcp-hide-secrets/SKILL.md b/skills/ess/mcp-hide-secrets/SKILL.md new file mode 100644 index 0000000..b69575e --- /dev/null +++ b/skills/ess/mcp-hide-secrets/SKILL.md @@ -0,0 +1,103 @@ +--- +name: mcp-hide-secrets +description: >- + Secure Cursor MCP credentials on macOS by moving secrets from ~/.cursor/mcp.json + or a project .cursor/mcp.json into ~/.local/share/cursor-mcp/mcp.env and loading + them via a LaunchAgent at login. Use when the user runs /mcp-hide-secrets, asks to + secure MCP config, externalize MCP secrets, set up mcp.env, or fix MCP auth after + cleaning up mcp.json. macOS only — do not use on Windows or Linux. +compatibility: >- + macOS only. Requires bash, python3, launchctl, and write access to + ~/Library/LaunchAgents. Windows support is planned via scripts/windows/ (not yet + implemented). +metadata: + version: "1.0" + supported_os: macos +--- + +# MCP Hide Secrets (macOS — LaunchAgent + .env) + +Move live credentials out of `~/.cursor/mcp.json` and/or a project `.cursor/mcp.json` into `~/.local/share/cursor-mcp/mcp.env`, loaded at login by a macOS LaunchAgent via `launchctl setenv`. Cursor interpolates `${env:VAR}` in MCP `env` and `headers` blocks. + +## Agent workflow (required) + +**Run only `scripts/run.sh`.** Do not write ad-hoc preflight, migrate, or verify commands. + +```bash +# From this skill root (set PROJECT_MCP_JSON when a repo .cursor/mcp.json should be included): +chmod +x scripts/run.sh +USERNAME="$(whoami)" PROJECT_MCP_JSON="/.cursor/mcp.json" scripts/run.sh +USERNAME="$(whoami)" scripts/run.sh migrate +USERNAME="$(whoami)" scripts/run.sh status +scripts/run.sh fix-launchagents # sudo in Terminal when agent cannot prompt +``` + +Never read or display secret values from `mcp.json` or `mcp.env`. Report script output only. + +| User says | Script | +| --------- | ------ | +| `/mcp-hide-secrets` | `scripts/run.sh` (default) | +| `/mcp-hide-secrets migrate` | `scripts/run.sh migrate` [optional `mcp.json` path] | +| `/mcp-hide-secrets status` | `scripts/run.sh status` | +| `/mcp-hide-secrets install` | `scripts/run.sh install` (after `fix-launchagents`) | + +If sudo fails in a non-interactive shell, ask the user to run `scripts/fix-launchagents-dir.sh` in Terminal, then `USERNAME="$(whoami)" scripts/run.sh install`. + +After success: user quits Cursor (Cmd+Q) and reopens from Dock. + +## MCP config targets + +| Location | Path | +| -------- | ---- | +| Global | `~/.cursor/mcp.json` | +| Project | `/.cursor/mcp.json` | + +Both merge into `~/.local/share/cursor-mcp/mcp.env`. Default `run.sh` migrates global first, then project when `PROJECT_MCP_JSON` is set. Migrate dirty files separately with `scripts/run.sh migrate `. + +## Gotchas (keep in SKILL.md) + +- Dock-launched Cursor does **not** see `~/.zshrc` exports — session env via LaunchAgent is required. +- `~/Library/LaunchAgents` is sometimes owned by `root`; `fix-launchagents-dir.sh` is a normal step, not a one-off fix. +- Migrate runs **before** LaunchAgent install so secrets move even when sudo is pending. +- A server named `github` (any case) drops `headers` on migrate so OAuth can run. + +## Migration rules (summary) + +| Source in `mcpServers` | In `mcp.env` | In `mcp.json` | +| ---------------------- | ------------ | ------------- | +| `env` literals | same key | `${env:KEY}` | +| `headers` literals | `MCP__
` | `${env:MCP_…}` | + +Identifiers (`X-User-ID`, `ATLASSIAN_USER_EMAIL`, …) stay inline. Details: [references/launchctl.md](references/launchctl.md). + +## References (read when needed) + +| File | When | +| ---- | ---- | +| [references/install-cursor.md](references/install-cursor.md) | Cursor-only optional steps (slash command, `disable-model-invocation`) | +| [references/launchctl.md](references/launchctl.md) | Manual install, migration tables, ongoing ops | +| [references/rationale.md](references/rationale.md) | Why session env instead of per-server `envFile` | +| [references/launchagent-naming.md](references/launchagent-naming.md) | Label `local..cursor-mcp-env`, launchctl commands | + +## Scripts + +| Script | Role | +| ------ | ---- | +| `run.sh` | Orchestrator — **only agent entry point** | +| `preflight.sh` | Read-only checks | +| `status.sh` | Read-only report | +| `check-inline-secrets.py` | Scan `mcp.json` without printing values | +| `install.sh` | Migrate-only or LaunchAgent install (called by `run.sh`) | +| `fix-launchagents-dir.sh` | `sudo chown "$(whoami):staff" ~/Library/LaunchAgents` | +| `verify.sh` | Post-install session env check | + +## Files on the user's Mac + +| Path | Purpose | +| ---- | ------- | +| `~/.local/share/cursor-mcp/mcp.env` | Secrets (chmod 600, never commit) | +| `~/.local/share/cursor-mcp/load-mcp-env.sh` | Parser + `launchctl setenv` | +| `~/Library/LaunchAgents/local..cursor-mcp-env.plist` | Login hook | +| `~/.cursor/mcp.json` and/or project `.cursor/mcp.json` | Sanitized MCP config | + +Rotate credentials that were ever committed or pasted into chat before or after migration. diff --git a/skills/ess/mcp-hide-secrets/references/install-cursor.md b/skills/ess/mcp-hide-secrets/references/install-cursor.md new file mode 100644 index 0000000..05d7b60 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/references/install-cursor.md @@ -0,0 +1,44 @@ +# Cursor installation (optional) + +This skill is portable per [agentskills.io](https://agentskills.io/specification). The committed `SKILL.md` stays host-neutral; apply these steps when installing under **Cursor** locally or in a team monorepo fork. + +## Slash command only (`/mcp-hide-secrets`) + +Cursor can load a skill for explicit slash invocation without auto-attaching it to every agent turn. Add this line to the **top-level** YAML frontmatter in your local copy of `SKILL.md` (not under `metadata` — Cursor reads the top-level key): + +```yaml +disable-model-invocation: true +``` + +Example (after `description`, before `compatibility`): + +```yaml +--- +name: mcp-hide-secrets +description: >- + ... +disable-model-invocation: true +compatibility: >- + macOS only. ... +--- +``` + +**Why not in the published skill?** That key is Cursor-specific and not part of the agentskills.io spec. Keeping it out of the committed skill lets `skills-ref validate` pass and lets Claude Code / other hosts use the same files without host-only frontmatter. + +**Team monorepo forks:** A Cursor-first monorepo may commit `disable-model-invocation: true` directly in `SKILL.md`. For a portable publish copy, leave it out and document this file instead. + +## Run the skill + +After optional frontmatter edits: + +``` +/mcp-hide-secrets +``` + +Or from the skill root: + +```bash +USERNAME=$(whoami) PROJECT_MCP_JSON="/.cursor/mcp.json" scripts/run.sh +``` + +See [launchctl.md](launchctl.md) for manual install, migration, and ongoing ops. diff --git a/skills/ess/mcp-hide-secrets/references/launchagent-naming.md b/skills/ess/mcp-hide-secrets/references/launchagent-naming.md new file mode 100644 index 0000000..460bfd8 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/references/launchagent-naming.md @@ -0,0 +1,37 @@ +# LaunchAgent label for this skill + +This skill installs one LaunchAgent. Keep the **filename** and plist **`Label`** identical. + +| Item | Value | +| ---- | ----- | +| Label | `local..cursor-mcp-env` | +| Plist path | `~/Library/LaunchAgents/local..cursor-mcp-env.plist` | +| Bootstrap domain | `gui/$(id -u)` | + +Replace `` with your macOS username (`whoami`). Use kebab-case if the username contains underscores. + +## Why `local..…` + +- **`local.`** — personal namespace, not a vendor domain (avoids squatting `com.*` for one-off scripts). +- **``** — collision-safe within shared machines; matches `USERNAME` passed to `scripts/run.sh`. +- **`cursor-mcp-env`** — service purpose, kebab-case. + +`launchd` only requires the label be unique in your user domain; the shape above is convention for scanability in `launchctl list` and logs. + +## Managing the agent + +```bash +# Load (install.sh does this) +launchctl bootstrap gui/$(id -u) ~/Library/LaunchAgents/local.$(whoami).cursor-mcp-env.plist + +# Re-run loader after editing mcp.env +launchctl kickstart -k gui/$(id -u)/local.$(whoami).cursor-mcp-env + +# Inspect +launchctl print gui/$(id -u)/local.$(whoami).cursor-mcp-env + +# Remove +launchctl bootout gui/$(id -u)/local.$(whoami).cursor-mcp-env +``` + +Commands use the **Label**, not the `.plist` filename — keeping them in sync avoids copy-paste mistakes. diff --git a/skills/ess/mcp-hide-secrets/references/launchctl.md b/skills/ess/mcp-hide-secrets/references/launchctl.md new file mode 100644 index 0000000..05aa246 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/references/launchctl.md @@ -0,0 +1,98 @@ +# MCP hide secrets — LaunchAgent + .env + +Move credentials out of `~/.cursor/mcp.json` and/or a project `.cursor/mcp.json` into `~/.local/share/cursor-mcp/mcp.env`, loaded at login by a macOS LaunchAgent via `launchctl setenv`. Cursor interpolates `${env:VAR}` in MCP `env` and `headers` blocks. Secrets and the loader live outside `~/.cursor/`. + +**Recommended:** run the skill in Cursor (`/mcp-hide-secrets`). Optional Cursor-only setup (slash-only loading): [install-cursor.md](install-cursor.md). + +## Manual install + +From the skill root (`mcp-hide-secrets/`): + +```bash +chmod +x scripts/run.sh +USERNAME=$(whoami) PROJECT_MCP_JSON=.cursor/mcp.json scripts/run.sh +# Or step by step: +# scripts/run.sh preflight +# scripts/run.sh fix-launchagents +# USERNAME=$(whoami) scripts/run.sh migrate +# USERNAME=$(whoami) scripts/run.sh install +# scripts/run.sh status +``` + +This will: + +1. Back up the target `mcp.json` (global or project path you pass) +2. Extract secrets into `~/.local/share/cursor-mcp/mcp.env` (chmod 600) +3. Rewrite `mcp.json` with `${env:VAR}` references +4. Fix `~/Library/LaunchAgents` ownership if needed (`scripts/fix-launchagents-dir.sh` — may prompt for sudo) +5. Install `~/.local/share/cursor-mcp/load-mcp-env.sh` and a LaunchAgent plist +6. Bootstrap the agent and load env vars into the GUI session + +If you previously used `~/.cursor/mcp.env`, install moves it to the new location automatically. + +### LaunchAgents ownership (step 4) + +On some Macs `~/Library/LaunchAgents` is owned by `root` and not writable. This is expected; install fixes it automatically: + +```bash +scripts/fix-launchagents-dir.sh +# runs: sudo chown "$(whoami):staff" ~/Library/LaunchAgents +``` + +Run that script alone in Terminal if you need to approve sudo before rerunning install. + +## Fresh install + +```bash +USERNAME=$(whoami) scripts/run.sh install +``` + +Edit `~/.local/share/cursor-mcp/mcp.env` with your values, then: + +```bash +launchctl kickstart -k gui/$(id -u)/local.$(whoami).cursor-mcp-env +``` + +Quit Cursor completely (Cmd+Q) and reopen from Dock. + +## Migration rules + +`scripts/run.sh migrate` (via `install.sh --migrate-only`) scans every entry in `mcpServers`: + +| Source | Stored in `mcp.env` as | Rewritten in `mcp.json` | +| ------ | ------------------------ | ----------------------- | +| `env` block | same key | `${env:KEY}` | +| `headers` block | `MCP__
` | `${env:MCP_…}` | + +User identifiers (`X-User-ID`, `ATLASSIAN_USER_EMAIL`, …) stay inline — they are not secrets. + +A server named `github` (any case) drops its `headers` block so Cursor can use OAuth. + +After migrate, list keys with: + +```bash +grep -E '^[A-Za-z_]' ~/.local/share/cursor-mcp/mcp.env +``` + +## LaunchAgent label + +Use `local..cursor-mcp-env`. See [launchagent-naming.md](launchagent-naming.md). + +## Ongoing workflow + +- **Rotate a secret:** edit `~/.local/share/cursor-mcp/mcp.env` → `launchctl kickstart -k gui/$(id -u)/local..cursor-mcp-env` → restart Cursor +- **Debug:** `tail /tmp/cursor-mcp-env.log`, then `launchctl getenv ` for any key from `mcp.env` +- **Never commit** `~/.local/share/cursor-mcp/mcp.env` + +## Files + +| Path | Purpose | +| ---- | ------- | +| `~/.local/share/cursor-mcp/mcp.env` | Secret values (chmod 600) | +| `~/.local/share/cursor-mcp/load-mcp-env.sh` | Parser + `launchctl setenv` | +| `~/Library/LaunchAgents/local..cursor-mcp-env.plist` | Login hook | +| `~/.cursor/mcp.json` and/or `/.cursor/mcp.json` | Credential-free MCP config | + +Skill scripts: [../scripts/](../scripts/) + +Further reading: [rationale.md](rationale.md) — why this skill uses session env instead of per-server `envFile`. diff --git a/skills/ess/mcp-hide-secrets/references/rationale.md b/skills/ess/mcp-hide-secrets/references/rationale.md new file mode 100644 index 0000000..189f05a --- /dev/null +++ b/skills/ess/mcp-hide-secrets/references/rationale.md @@ -0,0 +1,119 @@ +# Rationale: LaunchAgent + session env (not per-server `envFile`) + +For install steps see [launchctl.md](launchctl.md). + +This document records **why** this skill uses a macOS LaunchAgent (`launchctl setenv` → `${env:VAR}` in `mcp.json`) instead of Cursor’s per-server `envFile` option. It follows a lightweight [Architecture Decision Record](https://cognitect.com/blog/2011/11/15/documenting-architecture-decisions) shape: context, decision, consequences. + +## Status + +**Accepted** — LaunchAgent + session env is what this skill implements. + +## Context + +Cursor MCP config lives at `~/.cursor/mcp.json` (global) and/or `/.cursor/mcp.json` (project). We want secrets out of those files while keeping a single shared env file (`~/.local/share/cursor-mcp/mcp.env`). + +Cursor supports two main secret patterns ([MCP docs](https://cursor.com/docs/mcp)): + +1. **`${env:VAR}`** — Cursor resolves values from the **environment of the process that launched Cursor** (on macOS, the GUI login session when opened from Dock/Spotlight). +2. **`envFile`** — Per **stdio** server only: path to a dotenv file loaded into **that MCP child process** when Cursor spawns it. + +Our typical global config mixes **stdio** servers (`command` + `npx` …) and **remote** servers (`url` + `headers` with `${env:…}`). Remote entries do not support `envFile`. + +On macOS, Dock-launched Cursor does **not** inherit exports from `~/.zshrc`. Shell profile env is insufficient without extra machinery. + +## Decision + +Use a LaunchAgent at login to run `load-mcp-env.sh`, which reads `~/.local/share/cursor-mcp/mcp.env` and calls `launchctl setenv` for each key. Rewrite `mcp.json` to reference secrets only via `${env:VAR}`. + +Secrets and the loader live under `~/.local/share/cursor-mcp/`, not `~/.cursor/`, so Cursor’s config directory stays limited to Cursor-owned files (principally `mcp.json`). + +## Why not `envFile`? + +`envFile` is a **good, standard** pattern — the same idea as VS Code `launch.json` / `tasks.json` `envFile`, scoped to the process being started. We do not use it as the primary approach here because of **scope**, not quality. + +| Concern | `envFile` (per stdio server) | Session env (`launchctl setenv`) | +| -------- | ------------------------------ | ------------------------------------- | +| Who sees the vars | Only that MCP server’s child process | macOS GUI session → Cursor and all `${env:…}` resolution | +| Remote MCP (`url` + `headers`) | **Not supported** — no `envFile` field | `${env:…}` in headers works | +| `${env:VAR}` in `mcp.json` | Cursor does **not** read the child’s `envFile` for interpolation | Works for stdio `env` and remote `headers` | +| Least privilege | Stronger — secret only in one child | Broader — vars in GUI session (still better than plaintext in `mcp.json`) | +| Portability | stdio-only; `${userHome}/…` paths work cross-OS | macOS-specific (launchd) | +| Moving parts | One `envFile` path per server (or repeated path) | One env file + one LaunchAgent | + +### Mental model + +**`envFile`:** + +``` +Cursor (no API_KEY in its env) + └── spawns npx/mcp-server + └── envFile loaded here only +``` + +**Session env (this skill):** + +``` +LaunchAgent → launchctl setenv → GUI session has API_KEY + └── Cursor starts (inherits session) + ├── resolves ${env:API_KEY} for remote headers + └── spawns stdio MCPs with session env available +``` + +If the config were **stdio-only**, `envFile` on each server (or a shared file referenced per server) would often be the simpler choice. Our config is not stdio-only. + +## Alternatives considered + +| Approach | Verdict | +| -------- | ------- | +| **Per-server `envFile`** for stdio + **LaunchAgent only for remote keys** | Valid hybrid; more complex (two mechanisms, two docs paths). Rejected for this skill in favor of one env file + one loader. | +| **Secrets only in `~/.zshrc`** | Fails for Dock-launched Cursor on macOS. | +| **Inline secrets in `mcp.json`** | Easy but unsafe to share or commit. | +| **`envFile` in project `.cursor/mcp.json`** | Helps per-repo stdio servers; does not fix global remote MCPs or GUI session. | + +## Consequences + +**Positive** + +- One `mcp.env` feeds every `${env:…}` reference (stdio and remote). +- `mcp.json` is safe to inspect and share; secrets stay in `~/.local/share/cursor-mcp/`. +- Matches Cursor’s documented interpolation model for remote MCP auth. + +**Negative** + +- macOS-only until a Windows/Linux equivalent exists in this skill. +- Requires LaunchAgent + writable `~/Library/LaunchAgents`. +- Session-wide env is wider exposure than per-process `envFile` (mitigated by file permissions and not committing secrets). + +**Neutral** + +- Users who expect VS Code–style `envFile` for every server should read this doc — remote MCPs force session env or another Cursor-side mechanism. + +## When `envFile` *is* appropriate + +Use per-server `envFile` when: + +- The server is **stdio** (`command` + `args`). +- Secrets are needed **only** inside that server process. +- You do not rely on `${env:…}` in `mcp.json` for **remote** servers in the same config. + +Example (stdio only): + +```json +{ + "mcpServers": { + "example-search": { + "command": "npx", + "args": ["-y", "some-mcp-package"], + "envFile": "${userHome}/.local/share/cursor-mcp/mcp.env" + } + } +} +``` + +That does **not** supply **remote** MCP `headers` — those still need session env (this skill) or a different pattern. + +## References + +- [Cursor MCP — config interpolation and `envFile`](https://cursor.com/docs/mcp) +- [Cursor forum — secure MCP secrets (`${env:…}` vs `envFile`)](https://forum.cursor.com/t/secure-secret-handing-for-mcps/155638) +- Install and operations: [launchctl.md](launchctl.md) diff --git a/skills/ess/mcp-hide-secrets/scripts/check-inline-secrets.py b/skills/ess/mcp-hide-secrets/scripts/check-inline-secrets.py new file mode 100755 index 0000000..1eec5fd --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/check-inline-secrets.py @@ -0,0 +1,62 @@ +#!/usr/bin/env python3 +"""Report whether mcp.json files still contain inline secrets (never print values).""" + +from __future__ import annotations + +import json +import re +import sys +from pathlib import Path + +ENV_REF = re.compile(r"^\$\{env:[^}]+\}$") +_MIN_ARGC = 2 +_USAGE_EXIT_CODE = 2 + + +def scan(path: Path) -> tuple[bool, int]: + """Return (has_inline_secrets, env_reference_count).""" + if not path.is_file(): + return False, 0 + + config = json.loads(path.read_text(encoding="utf-8")) + has_inline = False + env_refs = 0 + + for server in config.get("mcpServers", {}).values(): + for block in (server.get("env"), server.get("headers")): + if not isinstance(block, dict): + continue + for value in block.values(): + if not isinstance(value, str) or not value: + continue + if ENV_REF.match(value): + env_refs += 1 + else: + has_inline = True + + return has_inline, env_refs + + +def main() -> int: + if len(sys.argv) < _MIN_ARGC: + print("usage: check-inline-secrets.py [...]", file=sys.stderr) + return _USAGE_EXIT_CODE + + exit_code = 0 + for arg in sys.argv[1:]: + path = Path(arg).expanduser().resolve() + if not path.is_file(): + print(f"{path}:missing") + exit_code = 1 + continue + has_inline, env_refs = scan(path) + inline = "yes" if has_inline else "no" + print(f"{path}:inline={inline} env_refs={env_refs}") + if has_inline: + exit_code = 1 + + return exit_code + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/skills/ess/mcp-hide-secrets/scripts/fix-launchagents-dir.sh b/skills/ess/mcp-hide-secrets/scripts/fix-launchagents-dir.sh new file mode 100755 index 0000000..ace9282 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/fix-launchagents-dir.sh @@ -0,0 +1,47 @@ +#!/bin/bash +# Ensure ~/Library/LaunchAgents is owned by the current user (staff group). +# Required before install.sh can bootstrap the cursor-mcp-env LaunchAgent. +# +# Usage: +# ./fix-launchagents-dir.sh +# +# Common case: directory exists but is owned by root (not writable). Runs: +# sudo chown "$(whoami):staff" ~/Library/LaunchAgents +# +# See ../SKILL.md + +set -euo pipefail + +LAUNCH_AGENTS_DIR="${LAUNCH_AGENTS_DIR:-$HOME/Library/LaunchAgents}" +CURRENT_USER="$(whoami)" +EXPECTED_GROUP="staff" + +mkdir -p "$LAUNCH_AGENTS_DIR" + +actual_owner="$(stat -f '%Su' "$LAUNCH_AGENTS_DIR")" +actual_group="$(stat -f '%Sg' "$LAUNCH_AGENTS_DIR")" + +if [ -w "$LAUNCH_AGENTS_DIR" ] && [ "$actual_owner" = "$CURRENT_USER" ]; then + echo "OK: $LAUNCH_AGENTS_DIR is writable by $CURRENT_USER." + exit 0 +fi + +echo "LaunchAgents directory is not writable by $CURRENT_USER." +echo " Path: $LAUNCH_AGENTS_DIR" +echo " Owner: ${actual_owner}:${actual_group}" +echo " Fix: sudo chown \"${CURRENT_USER}:${EXPECTED_GROUP}\" \"$LAUNCH_AGENTS_DIR\"" +echo "" +echo "Running fix (sudo will prompt for your password)..." + +sudo chown "${CURRENT_USER}:${EXPECTED_GROUP}" "$LAUNCH_AGENTS_DIR" || { + echo "" + echo "ERROR: sudo required — run this script in Terminal and approve the password prompt." >&2 + exit 2 +} + +if [ ! -w "$LAUNCH_AGENTS_DIR" ]; then + echo "ERROR: $LAUNCH_AGENTS_DIR is still not writable after chown." >&2 + exit 1 +fi + +echo "OK: $LAUNCH_AGENTS_DIR is now owned by ${CURRENT_USER}:${EXPECTED_GROUP}." diff --git a/skills/ess/mcp-hide-secrets/scripts/install.sh b/skills/ess/mcp-hide-secrets/scripts/install.sh new file mode 100755 index 0000000..facb5cb --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/install.sh @@ -0,0 +1,205 @@ +#!/bin/bash +# Install LaunchAgent + ~/.local/share/cursor-mcp/mcp.env for Cursor MCP secrets. +# +# Usage: +# USERNAME=your-username ./install.sh # fresh install from mcp.env.example +# USERNAME=your-username ./install.sh --migrate-only [mcp.json] # rewrite mcp.json + mcp.env only +# USERNAME=your-username ./install.sh --migrate [mcp.json] # alias for --migrate-only +# ./install.sh --fix-launchagents +# +# --migrate / --migrate-only do NOT install the LaunchAgent; they exit after rewriting files. +# For migrate + LaunchAgent install, use scripts/run.sh (default or `migrate` / `install` modes). +# +# See ../SKILL.md + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +MIGRATE=false +MIGRATE_ONLY=false +MCP_JSON="${MCP_JSON:-$GLOBAL_MCP_JSON}" + +while [ $# -gt 0 ]; do + case "$1" in + --migrate) + MIGRATE=true + shift + ;; + --migrate-only) + MIGRATE_ONLY=true + shift + ;; + --fix-launchagents) + exec "$SCRIPT_DIR/fix-launchagents-dir.sh" + ;; + *) + if [ -f "$1" ]; then + MCP_JSON="$(cd "$(dirname "$1")" && pwd)/$(basename "$1")" + fi + shift + ;; + esac +done + +if [ "$MIGRATE" = true ]; then + MIGRATE_ONLY=true +fi + +USERNAME="${USERNAME:-}" +if [ -z "$USERNAME" ]; then + echo "ERROR: set USERNAME (your macOS username, kebab-case if needed)." >&2 + echo "Example: USERNAME=\$(whoami) $0" >&2 + exit 1 +fi + +LABEL="local.${USERNAME}.cursor-mcp-env" +PLIST_DEST="$HOME/Library/LaunchAgents/${LABEL}.plist" +MCP_ENV="$MCP_DATA_DIR/mcp.env" +LOADER_DEST="$MCP_DATA_DIR/load-mcp-env.sh" +LEGACY_MCP_ENV="$HOME/.cursor/mcp.env" +LEGACY_LOADER="$HOME/.cursor/load-mcp-env.sh" +GUI_DOMAIN="gui/$(id -u)" +LAUNCH_AGENTS_DIR="$HOME/Library/LaunchAgents" + +migrate_mcp_json() { + python3 - "$MCP_JSON" "$MCP_ENV" <<'PY' +import json +import re +import sys +from pathlib import Path + +mcp_json_path = Path(sys.argv[1]) +mcp_env_path = Path(sys.argv[2]) +config = json.loads(mcp_json_path.read_text(encoding="utf-8")) +servers = config.get("mcpServers", {}) + +existing_env: dict[str, str] = {} +if mcp_env_path.is_file(): + for line in mcp_env_path.read_text(encoding="utf-8").splitlines(): + if not line or line.lstrip().startswith("#") or "=" not in line: + continue + key, _, value = line.partition("=") + key = key.strip() + if key: + existing_env[key] = value + +# Env/header keys that identify the user but are not secrets — leave inline in mcp.json. +INLINE_KEYS = frozenset( + { + "X-User-ID", + "X-User-Id", + "ATLASSIAN_USER_EMAIL", + } +) + + +def is_secret_value(value: str | None) -> bool: + if not value: + return False + return not (value.startswith("${env:") and value.endswith("}")) + + +def normalize_token(name: str) -> str: + return re.sub(r"[^A-Za-z0-9]+", "_", name).strip("_").upper() + + +def header_env_key(server_name: str, header_name: str) -> str: + return f"MCP_{normalize_token(server_name)}_{normalize_token(header_name)}" + + +for server_name, server in servers.items(): + if server_name.lower() == "github": + server.pop("headers", None) + + env_block = server.get("env") + if isinstance(env_block, dict): + for key, value in list(env_block.items()): + if key in INLINE_KEYS or not is_secret_value(value): + continue + existing_env[key] = value + env_block[key] = f"${{env:{key}}}" + + headers = server.get("headers") + if isinstance(headers, dict): + for key, value in list(headers.items()): + if key in INLINE_KEYS or not is_secret_value(value): + continue + env_key = header_env_key(server_name, key) + existing_env[env_key] = value + headers[key] = f"${{env:{env_key}}}" + +if existing_env: + mcp_env_path.write_text( + "\n".join(f"{key}={value}" for key, value in sorted(existing_env.items())) + "\n", + encoding="utf-8", + ) + +mcp_json_path.write_text( + json.dumps(config, indent=2, ensure_ascii=False) + "\n", + encoding="utf-8", +) +PY + chmod 600 "$MCP_ENV" + echo "Migrated: $MCP_JSON → $MCP_ENV" +} + +mkdir -p "$HOME/.cursor" "$MCP_DATA_DIR" "$LAUNCH_AGENTS_DIR" + +if [ -f "$LEGACY_MCP_ENV" ] && [ ! -f "$MCP_ENV" ]; then + mv "$LEGACY_MCP_ENV" "$MCP_ENV" + chmod 600 "$MCP_ENV" + echo "Moved legacy secrets: $LEGACY_MCP_ENV → $MCP_ENV" +fi + +if [ "$MIGRATE_ONLY" = true ]; then + if [ ! -f "$MCP_JSON" ]; then + echo "ERROR: --migrate-only requires an existing mcp.json." >&2 + echo " Tried: $MCP_JSON" >&2 + exit 1 + fi + backup_path="$MCP_JSON.bak.$(date +%Y%m%d%H%M%S)" + cp "$MCP_JSON" "$backup_path" + chmod 600 "$backup_path" + migrate_mcp_json + exit 0 +fi + +install -m 700 "$SCRIPT_DIR/load-mcp-env.sh" "$LOADER_DEST" + +if [ -f "$LEGACY_LOADER" ] && [ "$LEGACY_LOADER" != "$LOADER_DEST" ]; then + rm -f "$LEGACY_LOADER" + echo "Removed legacy loader: $LEGACY_LOADER" +fi + +if [ ! -f "$MCP_ENV" ]; then + cp "$SCRIPT_DIR/mcp.env.example" "$MCP_ENV" + chmod 600 "$MCP_ENV" + echo "Created $MCP_ENV from example — fill in secret values before restarting Cursor." +elif [ -f "$MCP_ENV" ]; then + echo "Keeping existing $MCP_ENV" +fi + +if ! "$SCRIPT_DIR/fix-launchagents-dir.sh"; then + echo "ERROR: fix LaunchAgents directory before installing plist." >&2 + exit 1 +fi + +sed "s/local.your-username.cursor-mcp-env/${LABEL}/g" \ + "$SCRIPT_DIR/local.your-username.cursor-mcp-env.plist" >"$PLIST_DEST" + +if launchctl print "$GUI_DOMAIN/$LABEL" >/dev/null 2>&1; then + launchctl bootout "$GUI_DOMAIN/$LABEL" || true +fi + +launchctl bootstrap "$GUI_DOMAIN" "$PLIST_DEST" +"$LOADER_DEST" + +"$SCRIPT_DIR/verify.sh" + +echo "Installed LaunchAgent: $LABEL" +echo "Loader: $LOADER_DEST" +echo "Secrets: $MCP_ENV" +echo "Reload after edits: launchctl kickstart -k $GUI_DOMAIN/$LABEL && restart Cursor (Cmd+Q)" diff --git a/skills/ess/mcp-hide-secrets/scripts/lib.sh b/skills/ess/mcp-hide-secrets/scripts/lib.sh new file mode 100755 index 0000000..89761cf --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/lib.sh @@ -0,0 +1,19 @@ +# Shared paths for mcp-hide-secrets scripts. From sibling scripts: +# SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# # shellcheck source=lib.sh +# source "$SCRIPT_DIR/lib.sh" + +MCP_HIDE_SECRETS_LIB_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" + +USERNAME="${USERNAME:-$(whoami)}" +LABEL="local.${USERNAME}.cursor-mcp-env" +GLOBAL_MCP_JSON="${GLOBAL_MCP_JSON:-$HOME/.cursor/mcp.json}" +PROJECT_MCP_JSON="${PROJECT_MCP_JSON:-}" +MCP_DATA_DIR="${MCP_DATA_DIR:-$HOME/.local/share/cursor-mcp}" +MCP_ENV="$MCP_DATA_DIR/mcp.env" +LOADER_DEST="$MCP_DATA_DIR/load-mcp-env.sh" +PLIST_DEST="$HOME/Library/LaunchAgents/${LABEL}.plist" +LAUNCH_AGENTS_DIR="$HOME/Library/LaunchAgents" +GUI_DOMAIN="gui/$(id -u)" +LOG_FILE="/tmp/cursor-mcp-env.log" +CHECK_INLINE_SECRETS="$MCP_HIDE_SECRETS_LIB_DIR/check-inline-secrets.py" diff --git a/skills/ess/mcp-hide-secrets/scripts/load-mcp-env.sh b/skills/ess/mcp-hide-secrets/scripts/load-mcp-env.sh new file mode 100755 index 0000000..77ec62d --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/load-mcp-env.sh @@ -0,0 +1,71 @@ +#!/bin/bash +# Load ~/.local/share/cursor-mcp/mcp.env into the macOS GUI session via launchctl setenv. +# Parses line-by-line without sourcing (no shell expansion on values). + +set -u + +ENV_FILE="${MCP_ENV_FILE:-$HOME/.local/share/cursor-mcp/mcp.env}" +LOG="${MCP_ENV_LOG:-/tmp/cursor-mcp-env.log}" + +log() { + printf '%s [load-mcp-env] %s\n' "$(date -Iseconds)" "$*" >>"$LOG" +} + +log_err() { + printf '%s [load-mcp-env] %s\n' "$(date -Iseconds)" "$*" >>"$LOG" >&2 +} + +is_valid_key() { + case "$1" in + [A-Za-z_]*) + case "$1" in + *[!A-Za-z0-9_]*) return 1 ;; + *) return 0 ;; + esac + ;; + *) return 1 ;; + esac +} + +[ -f "$ENV_FILE" ] || { + log "missing $ENV_FILE; nothing to do" + exit 0 +} + +failures=0 +loaded=0 + +while IFS= read -r line || [ -n "$line" ]; do + case "$line" in + '' | \#*) continue ;; + esac + case "$line" in + *=*) ;; + *) + log_err "skip invalid line (missing =)" + failures=$((failures + 1)) + continue + ;; + esac + key="${line%%=*}" + value="${line#*=}" + if [ -z "$key" ] || ! is_valid_key "$key"; then + log_err "skip invalid key name" + failures=$((failures + 1)) + continue + fi + if ! /bin/launchctl setenv "$key" "$value"; then + log_err "failed to set $key" + failures=$((failures + 1)) + continue + fi + loaded=$((loaded + 1)) +done <"$ENV_FILE" + +if [ "$loaded" -gt 0 ]; then + log "loaded $loaded keys from $ENV_FILE" +fi +if [ "$failures" -gt 0 ]; then + log_err "$failures line(s) skipped or failed from $ENV_FILE" + exit 1 +fi diff --git a/skills/ess/mcp-hide-secrets/scripts/local.your-username.cursor-mcp-env.plist b/skills/ess/mcp-hide-secrets/scripts/local.your-username.cursor-mcp-env.plist new file mode 100644 index 0000000..142567b --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/local.your-username.cursor-mcp-env.plist @@ -0,0 +1,21 @@ + + + + + Label + local.your-username.cursor-mcp-env + ProgramArguments + + /bin/bash + -c + $HOME/.local/share/cursor-mcp/load-mcp-env.sh + + RunAtLoad + + StandardOutPath + /tmp/cursor-mcp-env.log + StandardErrorPath + /tmp/cursor-mcp-env.log + + diff --git a/skills/ess/mcp-hide-secrets/scripts/mcp.env.example b/skills/ess/mcp-hide-secrets/scripts/mcp.env.example new file mode 100644 index 0000000..7ec04eb --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/mcp.env.example @@ -0,0 +1,11 @@ +# Installed to ~/.local/share/cursor-mcp/mcp.env (chmod 600). +# One KEY=VALUE per line. No quotes, no export, no shell expansion. +# Comments and blank lines are ignored. +# +# Populated by install.sh --migrate from inline values in mcp.json: +# - stdio server "env" keys keep their original names (e.g. MY_API_KEY) +# - remote server "headers" become MCP__
(e.g. MCP_MY_API_X_API_KEY) +# +# Example (delete and replace after migrate): +# MY_API_KEY= +# MCP_MY_REMOTE_SERVER_AUTHORIZATION= diff --git a/skills/ess/mcp-hide-secrets/scripts/preflight.sh b/skills/ess/mcp-hide-secrets/scripts/preflight.sh new file mode 100755 index 0000000..07614a4 --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/preflight.sh @@ -0,0 +1,65 @@ +#!/bin/bash +# Read-only preflight for mcp-hide-secrets. Never reads or prints secret values. +# +# Usage: +# ./preflight.sh +# PROJECT_MCP_JSON=/path/to/.cursor/mcp.json ./preflight.sh +# +# See ../SKILL.md + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +issues=0 + +echo "== mcp-hide-secrets preflight ==" + +if [ "$(uname -s)" != "Darwin" ]; then + echo "FAIL: macOS required (got $(uname -s))" + exit 1 +fi +echo "OK: macOS" + +if [ -w "$LAUNCH_AGENTS_DIR" ] && [ "$(stat -f '%Su' "$LAUNCH_AGENTS_DIR")" = "$USERNAME" ]; then + echo "OK: $LAUNCH_AGENTS_DIR writable by $USERNAME" +else + owner="$(stat -f '%Su:%Sg' "$LAUNCH_AGENTS_DIR" 2>/dev/null || echo 'unknown')" + echo "WARN: $LAUNCH_AGENTS_DIR not writable by $USERNAME (owner: $owner)" + echo " Run: $SCRIPT_DIR/fix-launchagents-dir.sh" + issues=$((issues + 1)) +fi + +if [ -f "$MCP_ENV" ]; then + key_count="$(grep -cE '^[A-Za-z_][A-Za-z0-9_]*=' "$MCP_ENV" 2>/dev/null || echo 0)" + echo "OK: $MCP_ENV exists ($key_count keys)" +else + echo "WARN: $MCP_ENV missing" + issues=$((issues + 1)) +fi + +mcp_json_found=false +for path in "$GLOBAL_MCP_JSON" ${PROJECT_MCP_JSON:+"$PROJECT_MCP_JSON"}; do + [ -n "$path" ] || continue + if [ ! -f "$path" ]; then + echo "SKIP: $path (not found)" + continue + fi + mcp_json_found=true + while IFS= read -r line; do + echo "mcp.json: $line" + if [[ "$line" == *":inline=yes"* ]]; then + issues=$((issues + 1)) + fi + done < <(python3 "$CHECK_INLINE_SECRETS" "$path" || true) +done + +if [ "$mcp_json_found" = false ]; then + echo "FAIL: no mcp.json found (configure ~/.cursor/mcp.json first)" + exit 1 +fi + +echo "== preflight done (issues: $issues) ==" +exit 0 diff --git a/skills/ess/mcp-hide-secrets/scripts/run.sh b/skills/ess/mcp-hide-secrets/scripts/run.sh new file mode 100755 index 0000000..5a501cb --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/run.sh @@ -0,0 +1,114 @@ +#!/bin/bash +# Entry point for /mcp-hide-secrets — run this script only; do not improvise checks in the agent. +# +# Usage: +# USERNAME=$(whoami) ./run.sh # install (migrate if needed, then LaunchAgent) +# USERNAME=$(whoami) ./run.sh migrate # ~/.cursor/mcp.json +# USERNAME=$(whoami) ./run.sh migrate .cursor/mcp.json +# USERNAME=$(whoami) ./run.sh status +# USERNAME=$(whoami) ./run.sh preflight +# ./run.sh fix-launchagents +# +# Optional: +# PROJECT_MCP_JSON=/path/to/repo/.cursor/mcp.json ./run.sh +# +# See ../SKILL.md + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +chmod +x "$SCRIPT_DIR"/*.sh "$SCRIPT_DIR/check-inline-secrets.py" 2>/dev/null || true + +if [ -z "${USERNAME:-}" ]; then + echo "ERROR: set USERNAME (your macOS username)." >&2 + echo "Example: USERNAME=\$(whoami) $0" >&2 + exit 1 +fi + +MODE="${1:-default}" +shift || true + +mcp_json_paths_need_migrate() { + local path + for path in "$@"; do + [ -f "$path" ] || continue + if python3 "$CHECK_INLINE_SECRETS" "$path" >/dev/null 2>&1; then + continue + fi + echo "$path" + done +} + +case "$MODE" in + status) + exec "$SCRIPT_DIR/status.sh" + ;; + preflight) + exec "$SCRIPT_DIR/preflight.sh" + ;; + fix-launchagents) + exec "$SCRIPT_DIR/fix-launchagents-dir.sh" + ;; + migrate) + MCP_JSON="${1:-$GLOBAL_MCP_JSON}" + "$SCRIPT_DIR/preflight.sh" + "$SCRIPT_DIR/install.sh" --migrate-only "$MCP_JSON" + if ! "$SCRIPT_DIR/fix-launchagents-dir.sh"; then + echo "" + echo "Next: run $SCRIPT_DIR/fix-launchagents-dir.sh in Terminal (sudo), then:" + echo " USERNAME=$USERNAME $SCRIPT_DIR/run.sh install" + exit 1 + fi + "$SCRIPT_DIR/install.sh" + "$SCRIPT_DIR/verify.sh" + ;; + install) + "$SCRIPT_DIR/preflight.sh" + if ! "$SCRIPT_DIR/fix-launchagents-dir.sh"; then + echo "" + echo "Next: run $SCRIPT_DIR/fix-launchagents-dir.sh in Terminal (sudo), then:" + echo " USERNAME=$USERNAME $SCRIPT_DIR/run.sh install" + exit 1 + fi + "$SCRIPT_DIR/install.sh" + "$SCRIPT_DIR/verify.sh" + ;; + default) + "$SCRIPT_DIR/preflight.sh" + + paths=("$GLOBAL_MCP_JSON") + if [ -n "$PROJECT_MCP_JSON" ] && [ -f "$PROJECT_MCP_JSON" ]; then + paths+=("$PROJECT_MCP_JSON") + fi + + while IFS= read -r dirty_path; do + [ -n "$dirty_path" ] || continue + echo "" + echo "== migrate $dirty_path ==" + "$SCRIPT_DIR/install.sh" --migrate-only "$dirty_path" + done < <(mcp_json_paths_need_migrate "${paths[@]}") + + if ! "$SCRIPT_DIR/fix-launchagents-dir.sh"; then + echo "" + echo "Secrets migrated (if any). LaunchAgent step needs sudo in Terminal:" + echo " $SCRIPT_DIR/fix-launchagents-dir.sh" + echo " USERNAME=$USERNAME $SCRIPT_DIR/run.sh install" + "$SCRIPT_DIR/status.sh" + exit 1 + fi + + echo "" + echo "== install LaunchAgent ==" + "$SCRIPT_DIR/install.sh" + "$SCRIPT_DIR/verify.sh" + "$SCRIPT_DIR/status.sh" + ;; + *) + echo "ERROR: unknown mode: $MODE" >&2 + echo "Usage: $0 [default|migrate|install|status|preflight|fix-launchagents] [mcp.json]" >&2 + exit 1 + ;; +esac diff --git a/skills/ess/mcp-hide-secrets/scripts/status.sh b/skills/ess/mcp-hide-secrets/scripts/status.sh new file mode 100755 index 0000000..9fef80e --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/status.sh @@ -0,0 +1,58 @@ +#!/bin/bash +# Read-only status for mcp-hide-secrets. Never reads or prints secret values. +# +# Usage: +# ./status.sh +# PROJECT_MCP_JSON=/path/to/.cursor/mcp.json ./status.sh +# +# See ../SKILL.md + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +"$SCRIPT_DIR/preflight.sh" + +echo "" +echo "== LaunchAgent ==" + +if [ -f "$PLIST_DEST" ]; then + echo "OK: plist $PLIST_DEST" +else + echo "WARN: plist missing ($PLIST_DEST)" +fi + +if launchctl print "$GUI_DOMAIN/$LABEL" >/dev/null 2>&1; then + echo "OK: agent loaded ($LABEL)" + launchctl print "$GUI_DOMAIN/$LABEL" 2>/dev/null | head -5 || true +else + echo "WARN: agent not loaded ($GUI_DOMAIN/$LABEL)" +fi + +echo "" +echo "== Session env (key names only) ==" + +if [ ! -f "$MCP_ENV" ]; then + echo "WARN: no $MCP_ENV" + exit 0 +fi + +while IFS= read -r key; do + [ -n "$key" ] || continue + value="$(launchctl getenv "$key" 2>/dev/null || true)" + if [ -n "$value" ]; then + echo "OK: $key is set in GUI session" + else + echo "WARN: $key is empty in GUI session" + fi +done < <(grep -E '^[A-Za-z_][A-Za-z0-9_]*=' "$MCP_ENV" | cut -d= -f1) + +echo "" +echo "== Loader log (last 5 lines) ==" +if [ -f "$LOG_FILE" ]; then + tail -5 "$LOG_FILE" +else + echo "(no $LOG_FILE yet)" +fi diff --git a/skills/ess/mcp-hide-secrets/scripts/verify.sh b/skills/ess/mcp-hide-secrets/scripts/verify.sh new file mode 100755 index 0000000..f71364b --- /dev/null +++ b/skills/ess/mcp-hide-secrets/scripts/verify.sh @@ -0,0 +1,37 @@ +#!/bin/bash +# Post-install verification. Never prints secret values. +# +# Usage: +# ./verify.sh +# +# See ../SKILL.md + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib.sh +source "$SCRIPT_DIR/lib.sh" + +sample_key="" +if [ -f "$MCP_ENV" ]; then + sample_key="$(grep -E '^[A-Za-z_][A-Za-z0-9_]*=' "$MCP_ENV" | head -1 | cut -d= -f1 || true)" +fi + +if [ -n "$sample_key" ]; then + value="$(launchctl getenv "$sample_key" 2>/dev/null || true)" + if [ -n "$value" ]; then + echo "OK: $sample_key is set in the GUI session." + else + echo "WARN: $sample_key is empty — check $MCP_ENV and $LOG_FILE" + fi +elif [ -f "$MCP_ENV" ]; then + echo "WARN: no keys in $MCP_ENV — add secrets before restarting Cursor." +else + echo "WARN: $MCP_ENV missing" +fi + +if launchctl print "$GUI_DOMAIN/$LABEL" >/dev/null 2>&1; then + echo "OK: LaunchAgent $LABEL is loaded." +else + echo "WARN: LaunchAgent $LABEL is not loaded." +fi diff --git a/skills/ess/pr-address-comments-all/LICENSE b/skills/ess/pr-address-comments-all/LICENSE new file mode 100644 index 0000000..9c7afe3 --- /dev/null +++ b/skills/ess/pr-address-comments-all/LICENSE @@ -0,0 +1,13 @@ +Copyright 2025 Cisco Systems, Inc. or its Affiliates + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. diff --git a/skills/ess/pr-address-comments-all/SKILL.md b/skills/ess/pr-address-comments-all/SKILL.md new file mode 100644 index 0000000..6533b1e --- /dev/null +++ b/skills/ess/pr-address-comments-all/SKILL.md @@ -0,0 +1,240 @@ +--- +name: pr-address-comments-all +description: Address GitHub PR review comments on one or more PRs in parallel, each in its own git worktree on the PR's branch, reusing the pr-address-comments procedure. Auto-starts (look up PRs, create worktrees, fetch comments), then pauses to show a per-PR addressing plan for approval; after approval each PR is handled by a background subagent that implements, commits, pushes to the PR branch, and resolves threads. With no PR given, it discovers your own open PRs on the current repo's remote that have changes-requested or unresolved review comments and runs the same workflow. Use when the user gives one or more PR URLs/numbers, or asks to address/fix review comments on their own open PRs on this repo without naming them, especially in parallel or batch. +--- + +# PR Address Comments — All (parallel, one worktree per PR) + +Address review comments across **one or more** GitHub PRs at the same time. Each PR is +isolated in its own git worktree checked out **on the PR's branch**, so work commits +locally and pushes straight back to the PR. This skill orchestrates the existing single-PR +procedure in [pr-address-comments/SKILL.md](../pr-address-comments/SKILL.md) — it does +not re-document its `gh`/GraphQL/commit steps; helper scripts and reference docs hold the +mechanics. + +## Quick start + +``` +You: address the review comments on my open PRs +You: fix my PRs that need changes +You: address the review comments on PRs 103 and 110 +You: fix the comments on https://github.com///pull/103 +You: address comments on PR 103, 110, 121 in parallel +``` + +Phases with **one human checkpoint** between analysis and code: + +``` +Phase 0 (auto) -> if no PR given, discover my open PRs that need attention (current repo) +Phase 1 (auto) -> look up each PR, create a worktree on its branch, fetch comments +Phase 2 (auto) -> draft a concrete fix-per-thread plan per PR (read-only) +GATE -> show the plans, wait for your approval +Phase 3 (auto) -> per PR: implement, commit, push to branch, reply + resolve threads +``` + +Phases 0–2 just start. Phase 3 never begins until you approve the plans, because a per-PR +subagent may not understand the whole-repo context. + +## Inputs + +- **Zero** or more PR references, each a **full URL** + (`https://github.com///pull/`) or a **bare number** (`103`). +- Bare numbers resolve against the current repo's remote. +- With **no** reference, Phase 0 discovers your open PRs on the current repo's remote that + have changes-requested or >=1 unresolved review comment. + +## Prerequisites + +- `gh` CLI authenticated with access to each PR's repository. +- Inside a git repo; `git worktree` and `openssl` available. +- Write access to each PR's branch for pushing (`git_write`). + +## Limitations + +- **Same-repo PRs only.** The workflow fetches and pushes the PR head branch through the + local repo's `origin` remote (`git fetch origin -- ` / `git push origin `). + **Fork-based PRs** — whose head branch lives in a fork or another remote — are not + supported, because `origin` does not have that branch. + +## Scripts + +Invoke by workspace-relative path. Each takes `--help`. + +| Script | Purpose | +|--------|---------| +| [`scripts/find_my_prs.sh`](scripts/find_my_prs.sh) | Discover my open PRs on the current repo with changes-requested or unresolved comments | +| [`scripts/fetch_pr.sh`](scripts/fetch_pr.sh) | Look up ONE PR ref -> `meta.env` + `comments.json` + `threads.json` | +| [`scripts/create_pr_worktree.sh`](scripts/create_pr_worktree.sh) | Create a worktree on the PR branch (`-B`) + run repo setup | +| [`scripts/safe_push.sh`](scripts/safe_push.sh) | Push HEAD to the PR branch without hanging — auto-rebases if the branch moved, fails cleanly on conflict | +| [`scripts/reply_and_resolve.sh`](scripts/reply_and_resolve.sh) | Reply to a comment (REST `in_reply_to`) + resolve its thread (GraphQL) | + +--- + +## Phase 0 — Discover my PRs (when no PR given) + +If the user named no PR, find their own open PRs on the **current repo's remote** that need +attention (review decision `CHANGES_REQUESTED`, or >=1 unresolved review thread): + +```bash +SKILL=skills/ess/pr-address-comments-all +mapfile -t PRS < <("$SKILL/scripts/find_my_prs.sh") # current repo, my open PRs +``` + +The script prints a table to stderr and the matching PR numbers to stdout. If `PRS` is +empty, report "no open PRs need attention" and stop. Otherwise **auto-proceed** to Phase 1 +for each number in `PRS` (in parallel) — no extra confirmation; the per-PR plan GATE before +any code still applies. Discovery is scoped to the current repo only; for other repos, pass +explicit PR URLs. + +## Phase 1 — Look up PRs and create worktrees (auto-start) + +"Look up" = parse each input into `owner/repo/number` + head branch. This is distinct from +"resolve threads" (the GraphQL `resolveReviewThread` step in Phase 3). + +Pre-flight once: `gh auth status` and `git rev-parse --show-toplevel` must both succeed. + +Then, **for each PR in parallel**: + +```bash +SKILL=skills/ess/pr-address-comments-all + +# 1) Look up the PR -> writes meta.env + comments.json + threads.json +"$SKILL/scripts/fetch_pr.sh" # add --repo owner/repo for a bare number + +# 2) Create a worktree checked out on the PR's branch + run worktrees.json setup +"$SKILL/scripts/create_pr_worktree.sh" --pr --repo / +``` + +Record each PR's `meta.env` values plus the printed `WORKTREE_ID` / `WORKTREE_PATH`. +Skip any PR whose `threads.json` is empty (no unresolved comments) and note it in the +summary. Worktree internals and edge cases: +[references/worktree-mechanics.md](references/worktree-mechanics.md). + +## Phase 2 — Draft a per-PR addressing plan (parallel, read-only) + +For each PR, launch one read-only `explore` subagent pointed at its `WORKTREE_PATH` using +the analysis template in +[references/subagent-prompts.md](references/subagent-prompts.md). It reads the code behind +each thread and returns a concrete fix-per-thread table (or "skip" with a reason). No edits. + +Run all analysis subagents concurrently. + +## GATE — Approve the plan (required) + +Compile every PR's returned plan into one view and **stop for the user**: + +```markdown +## Plan to address comments + +### PR # — (branch `<BRANCH>`, <count> comments, worktree `<WORKTREE_ID>`) +| # | File:Line | Reviewer ask | Proposed fix | Action | +|---|-----------|--------------|--------------|--------| +| 1 | path:42 | ... | ... | fix | +| 2 | path:17 | ... | ... | skip — needs discussion | +``` + +The `File:Line` column comes straight from each thread's `path` and `line` in +`threads.json` (see [`scripts/fetch_pr.sh`](scripts/fetch_pr.sh)). + +Wait for explicit approval. Allow the user to edit, drop, or skip items per PR. Do not +start Phase 3 until approved; if changes are requested, revise and re-present. + +## Phase 3 — Implement, commit, push, resolve (parallel) + +For each approved PR, launch one background `generalPurpose` subagent using the +implementation template in +[references/subagent-prompts.md](references/subagent-prompts.md). Each subagent works +entirely inside its `WORKTREE_PATH`, follows +[pr-address-comments/SKILL.md](../pr-address-comments/SKILL.md) Steps 6–8 +(implement -> verify -> **non-interactive** conventional commit), pushes with +`"$SKILL/scripts/safe_push.sh" --branch <BRANCH>` (with `SKILL=skills/ess/pr-address-comments-all`), then calls +`"$SKILL/scripts/reply_and_resolve.sh"` for each ADDRESSED thread (never for "skip" threads). + +Run all implementation subagents concurrently; each pushes to its own branch. + +> **Background subagents have no TTY.** Any command that waits for input (interactive +> `cz c`, `git pull --rebase` that stops on a conflict, a pager, a credential prompt) +> hangs that subagent forever. Subagents MUST use non-interactive git and push via +> `safe_push.sh` — see [Resilience](#resilience--moving-branches--stuck-subagents). + +## Resilience — moving branches & stuck subagents + +The branch can move **while** a subagent is working (you push to it, CI amends it, or a +human reviewer pushes). A raw `git push` then fails non-fast-forward, and the classic +follow-up — interactive `git pull --rebase` — drops a TTY-less subagent into a conflict +prompt it can never answer, so it hangs indefinitely. Guard against this on both sides: + +**Subagent side (built into the template):** + +- Push only via `"$SKILL/scripts/safe_push.sh" --branch <BRANCH>`. It forces non-interactive git, + retries the push, auto-rebases when the remote moved with non-conflicting changes, and + on a real conflict **aborts the rebase** (leaving a clean tree) and exits `3` instead of + blocking. Never run a bare `git push`/`git pull --rebase` in a subagent. +- Commit non-interactively (manual `git commit -m`, never `cz c`). +- If anything can't be finished autonomously, **stop and report the exact state** — never + leave a rebase/merge in progress. + +**Orchestrator side (watchdog):** after dispatching the background subagents, do not block +on them indefinitely. If a subagent runs far past its expected time with no result, or +returns a `SAFE_PUSH_CONFLICT` / non-zero push, **finish that PR inline** instead of +waiting or re-dispatching: in its worktree, `git fetch origin -- <BRANCH>` then rebase the +subagent's fix commit onto `origin/<BRANCH>` (or `git reset --hard origin/<BRANCH>` and +re-apply the approved fixes), `"$SKILL/scripts/safe_push.sh" --branch <BRANCH>` (with +`SKILL=skills/ess/pr-address-comments-all`), and `"$SKILL/scripts/reply_and_resolve.sh"`. +Never run two agents +against the same branch at once — if a tick/retry finds a PR already has an in-flight fix, +skip it. + +## Aggregate summary + +After all subagents finish, present one combined table: + +```markdown +## Done — <K> PRs + +| PR | Branch | Addressed | Skipped | Resolved threads | Commit | Pushed | +|----|--------|-----------|---------|------------------|--------|--------| +| #103 | feat/x | 3 | 1 | 3/4 | abc1234 | yes | +| #110 | fix/y | 2 | 0 | 2/2 | def5678 | yes | +``` + +Call out any PR that failed to push, had no comments, or needs follow-up discussion. + +## Cleanup + +Each worktree is on the PR branch and has already pushed — nothing to merge back. Remove +each when done (`git worktree remove "$WORKTREE_PATH" && git worktree prune`, or +`/delete-worktree <WORKTREE_ID>`). Details: +[references/worktree-mechanics.md](references/worktree-mechanics.md). Confirm before +removing any worktree whose push failed. + +## Progressive disclosure + +- Worktree-on-branch rationale, layout, cleanup, edge cases -> + [references/worktree-mechanics.md](references/worktree-mechanics.md). +- Phase 2 / Phase 3 subagent prompt templates -> + [references/subagent-prompts.md](references/subagent-prompts.md). +- Script flags -> run any script with `--help`. +- The per-PR commit / reply / resolve specifics -> + [pr-address-comments/SKILL.md](../pr-address-comments/SKILL.md) Steps 6–9. + +## Anti-patterns + +- Starting Phase 3 before the user approves the GATE — never touch code first. +- Resolving "skip" threads — only reply + resolve threads actually addressed in code. +- Fabricating comments/threads when `gh` fails — surface the failure instead. +- Detached worktrees for this flow — use `-B` so commits land on the PR branch (see + worktree-mechanics.md for the one exception). +- Duplicating the command's Steps 6–9 in subagent prompts — reference them, don't copy. +- Stacking multiple PRs in one worktree — one worktree per PR, always. +- Running discovery against a repo you didn't intend — Phase 0 always uses the current + remote; pass explicit PR URLs for any other repo. +- Interactive commands in a background subagent (`cz c`, `git pull --rebase` on a conflict, + a pager) — they have no TTY and will hang forever. Commit with `git commit -m` and push + via `safe_push.sh`. +- A bare `git push` in a subagent with no recovery — use `safe_push.sh` so a moved branch + rebases or fails cleanly instead of stalling. +- Leaving a rebase/merge in progress when a subagent gives up — always abort to a clean + tree and report, so the orchestrator can finish inline. +- Blocking forever on a stalled subagent, or dispatching a second agent for the same PR — + watchdog the run and finish inline instead. diff --git a/skills/ess/pr-address-comments-all/references/subagent-prompts.md b/skills/ess/pr-address-comments-all/references/subagent-prompts.md new file mode 100644 index 0000000..03e68d8 --- /dev/null +++ b/skills/ess/pr-address-comments-all/references/subagent-prompts.md @@ -0,0 +1,82 @@ +# Subagent prompt templates + +Reusable prompts the orchestrator passes to the per-PR subagents. Fill in the +`<...>` placeholders from each PR's `meta.env` / `threads.json` (produced by +[`scripts/fetch_pr.sh`](../scripts/fetch_pr.sh)) and the worktree path (from +[`scripts/create_pr_worktree.sh`](../scripts/create_pr_worktree.sh)). + +## Phase 2 — analysis (read-only) + +One per PR. Launch as an `explore` subagent with `readonly: true` and +`run_in_background: true`. It must NOT edit anything — it only proposes fixes. + +``` +Repo worktree (read-only): <WORKTREE_PATH> +PR #<N> in <owner>/<repo>, branch <BRANCH>. + +For each review thread below, read the referenced file and surrounding code in +this worktree and propose a concrete, minimal fix. If a thread needs discussion +rather than a code change, mark it "skip" and say why. Do NOT edit anything. + +Threads (from threads.json — path | line | comment | threadId | commentId): +<paste the NDJSON rows from <OUTPUT_DIR>/threads.json> + +Return a markdown table with columns: +# | path:line | reviewer ask | proposed fix (files + approach) | action (fix/skip) +``` + +## Phase 3 — implementation (read/write, background) + +One per PR. Launch as a `generalPurpose` subagent with `run_in_background: true` +only AFTER the user approves the plan at the GATE. Embed that PR's approved plan +verbatim. + +``` +Work entirely inside this worktree: <WORKTREE_PATH> (cd there first). +It is already checked out on branch <BRANCH> for PR #<N> in <owner>/<repo>. + +You are a BACKGROUND subagent with NO terminal. Never run a command that waits +for input — it will hang you forever. So: no `cz c`, no bare `git pull --rebase`, +no pager. Up front, force non-interactive git: + export GIT_EDITOR=true GIT_SEQUENCE_EDITOR=true GIT_PAGER=cat PAGER=cat GIT_TERMINAL_PROMPT=0 + +Read skills/ess/pr-address-comments/SKILL.md and follow Steps 6–8 for this +PR, using the APPROVED PLAN below as the exact list of fixes to make: + +<approved per-PR plan from the GATE> + +Specifics: +- Step 6: implement each "fix" item. Skip items marked "skip". +- Step 7: verify (grep for stale refs; run cheap, relevant linters/tests). +- Step 8: conventional-commit non-interactively (NOT `cz c`): + git commit -m "fix(<scope>): address PR #<N> review comments" +- Push with the safe helper (handles a branch that moved under you; never hangs): + skills/ess/pr-address-comments-all/scripts/safe_push.sh --branch <BRANCH> + If it exits 3 (SAFE_PUSH_CONFLICT) or otherwise non-zero, STOP — do NOT leave a + rebase in progress — and report the exact state so the orchestrator finishes + inline. Do not retry endlessly or improvise an interactive rebase. +- For each ADDRESSED thread, reply + resolve via the helper (do NOT resolve + "skip" threads): + skills/ess/pr-address-comments-all/scripts/reply_and_resolve.sh \ + --repo <owner>/<repo> --pr <N> \ + --comment-id <commentId> --thread-id <threadId> \ + --body "Fixed — <what changed>" + +Return: commit SHA, files changed, threads replied-to/resolved, threads skipped, +and the push result (including any SAFE_PUSH_CONFLICT/non-zero exit). +``` + +## Notes + +- Steps 6–8 (implement / verify / commit) are NOT duplicated here — the subagent + follows them from `skills/ess/pr-address-comments/SKILL.md`, with two background + overrides: commit with `git commit -m` (Step 8's `cz c` is interactive and would + hang a TTY-less subagent), and push via `safe_push.sh` instead of a bare + `git push` (so a branch that moved mid-run rebases or fails cleanly). +- `reply_and_resolve.sh` and `safe_push.sh` live in the repo, so they are present + inside the worktree (a checkout of the same repo) and can be called by their + workspace-relative paths. +- A `SAFE_PUSH_CONFLICT` (exit 3) is the orchestrator's cue to finish that PR + inline — see SKILL.md "Resilience". The subagent must report it, not retry. +- For a single PR, the orchestrator may run Phases 2–3 inline instead of spawning + subagents — there is no parallelism to gain. diff --git a/skills/ess/pr-address-comments-all/references/worktree-mechanics.md b/skills/ess/pr-address-comments-all/references/worktree-mechanics.md new file mode 100644 index 0000000..5f754b3 --- /dev/null +++ b/skills/ess/pr-address-comments-all/references/worktree-mechanics.md @@ -0,0 +1,90 @@ +# Worktree mechanics + +How `pr-address-comments-all` isolates each PR in its own git worktree and why +it differs from the stock `/worktree` command. The script +[`scripts/create_pr_worktree.sh`](../scripts/create_pr_worktree.sh) implements +all of this; read here when you need to understand or debug it. + +## `-B <branch>` instead of `--detach` + +`/worktree` creates a **detached** worktree (`git worktree add --detach`), which +is right for throwaway experiments but awkward for PR work: there is no branch +to commit onto or push from. + +This skill instead creates a worktree **on the PR's head branch**: + +```bash +git worktree add -B "$BRANCH" "$WORKTREE_DIR" "origin/$BRANCH" +``` + +`-B` creates (or resets) a real local branch `$BRANCH` pointing at +`origin/$BRANCH` and checks it out in the new worktree. As a result: + +- Commits land directly on the PR branch. +- `git push origin "$BRANCH"` updates the PR — no `push HEAD:<branch>` dance. +- There is nothing to merge back into the main working tree, so `/apply-worktree` + is not part of this workflow. + +## On-disk layout (so `/delete-worktree` still works) + +The worktree is created under the **same tree** the `/worktree` command uses, with +the same repo-key scheme, so cleanup tooling recognizes it: + +``` +~/.cursor/worktrees/<WORKTREE_ID>/<REPO_KEY> +``` + +- `WORKTREE_ID = pr-<number>-<8 hex>` — unique per PR, so multiple PRs never collide. +- `REPO_KEY = <repo-basename>-<sha256(REPO_ROOT)[:12]>` — identical to the + `/worktree` create block, so the path is one `/worktree`-style worktree would + produce. + +Because each PR gets a distinct `WORKTREE_ID`, every PR is independently +removable. + +## Setup + +After creating the worktree, the script runs the repo's `setup-worktree` steps +from `.cursor/worktrees.json` with `ROOT_WORKTREE_PATH` exported to the main repo +root (jq-parsed when available, otherwise the documented default). For this repo +that is: rsync `.env` files from the main tree, then `uv sync --all-packages`. + +## Cleanup + +Each worktree has already pushed to the PR branch, so just remove it: + +```bash +git worktree remove "$WORKTREE_PATH" # add --force if it complains about state +git worktree prune +``` + +Or, since the layout matches `/worktree`, use `/delete-worktree <WORKTREE_ID>` +per PR. Confirm with the user before removing any worktree whose push failed. + +## Edge cases + +### Branch already checked out elsewhere + +`git worktree add -B` refuses if `$BRANCH` is checked out in the main tree or +another worktree (git forbids the same branch in two worktrees). Options: + +- Reuse the existing checkout instead of creating a new worktree, or +- Create the worktree detached and push explicitly: + + ```bash + SKILL=skills/ess/pr-address-comments-all + git worktree add --detach "$WORKTREE_DIR" "origin/$BRANCH" + # ... commit ... + "$SKILL/scripts/safe_push.sh" --branch "$BRANCH" # pushes HEAD:<branch>, rebases if moved + ``` + + `safe_push.sh` already pushes `HEAD:<branch>`, so it works the same whether the + worktree is attached (`-B`) or detached — and it will not hang if the branch + moved while you worked. + +### Cross-repo PRs + +When PRs span multiple repositories (full URLs in different repos), run +`create_pr_worktree.sh` from **each PR's own repo root** so `REPO_ROOT` and the +remote resolve correctly. Always pass `--repo owner/repo` so a bare-number +assumption from one repo never leaks into another. diff --git a/skills/ess/pr-address-comments-all/scripts/create_pr_worktree.sh b/skills/ess/pr-address-comments-all/scripts/create_pr_worktree.sh new file mode 100755 index 0000000..73c5cc4 --- /dev/null +++ b/skills/ess/pr-address-comments-all/scripts/create_pr_worktree.sh @@ -0,0 +1,115 @@ +#!/usr/bin/env bash +# +# create_pr_worktree.sh +# Create a git worktree checked out ON a PR's branch (so work commits locally +# and pushes straight back to the PR), then run the repo's worktree setup. +# +# Mirrors the /worktree command's on-disk layout +# (~/.cursor/worktrees/<WORKTREE_ID>/<repo-key>) so /delete-worktree still +# works, but uses `git worktree add -B <branch>` instead of --detach. +# +# Usage: create_pr_worktree.sh --pr N --repo owner/repo [--branch B] +# --pr N - PR number (used to name the worktree) +# --repo owner/repo - Repo the PR lives in +# --branch B - PR head branch (default: looked up via gh) +# +# Run from inside the target repo. Prints WORKTREE_ID, WORKTREE_PATH, BRANCH. + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 1; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } +warn() { echo -e "${YELLOW}Warning:${NC} $1" >&2; } + +PR_NUMBER="" +OWNER_REPO="" +BRANCH="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --pr) PR_NUMBER="${2:-}"; shift 2 ;; + --repo) OWNER_REPO="${2:-}"; shift 2 ;; + --branch) BRANCH="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,19p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +[[ -n "$PR_NUMBER" ]] || error "Missing --pr N" +[[ -n "$OWNER_REPO" ]] || error "Missing --repo owner/repo" +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" +command -v openssl >/dev/null 2>&1 || error "openssl not found on PATH" + +REPO_ROOT="$(git rev-parse --show-toplevel)" || error "Not inside a git repository" + +if [[ -z "$BRANCH" ]]; then + BRANCH="$(gh pr view "$PR_NUMBER" --repo "$OWNER_REPO" --json headRefName --jq '.headRefName')" \ + || error "Could not look up branch for ${OWNER_REPO}#${PR_NUMBER}" +fi + +# /worktree-compatible repo key: <basename>-<sha256(repo_root)[:12]>. +REPO_BASENAME="$(basename "$REPO_ROOT")" +if command -v shasum >/dev/null 2>&1; then + REPO_HASH="$(printf '%s' "$REPO_ROOT" | shasum -a 256 | cut -c1-12)" +else + REPO_HASH="$(printf '%s' "$REPO_ROOT" | sha256sum | cut -c1-12)" +fi +REPO_KEY="${REPO_BASENAME}-${REPO_HASH}" + +WORKTREE_ID="pr-${PR_NUMBER}-$(openssl rand -hex 4)" +WORKTREE_DIR="${HOME}/.cursor/worktrees/${WORKTREE_ID}/${REPO_KEY}" + +[[ -d "$WORKTREE_DIR" ]] && error "Worktree directory already exists: ${WORKTREE_DIR}" +mkdir -p "$(dirname "$WORKTREE_DIR")" + +info "Fetching origin/${BRANCH}" +# `--` so a branch name starting with `-` is treated as a refspec, not an option. +git fetch origin -- "$BRANCH" + +info "Creating worktree on branch ${BRANCH}" +# -B resets/creates a real local branch tracking origin/<branch>, checked out +# in the new worktree. Commits land on the branch; `git push origin <branch>` +# updates the PR. Fails if the branch is already checked out elsewhere -- see +# references/worktree-mechanics.md for that edge case. +git worktree add -B "$BRANCH" "$WORKTREE_DIR" "origin/${BRANCH}" + +# Run the repo's worktree setup (mirrors the /worktree command). +WORKTREES_JSON="${REPO_ROOT}/.cursor/worktrees.json" +export ROOT_WORKTREE_PATH="$REPO_ROOT" +( + cd "$WORKTREE_DIR" + if [[ -f "$WORKTREES_JSON" ]] && command -v jq >/dev/null 2>&1; then + info "Running setup-worktree from .cursor/worktrees.json" + # setup-worktree may be a string, an array of strings, or absent. `// empty` + # makes a missing/null key emit nothing (instead of the literal "null"). + jq -r 'if (."setup-worktree"|type) == "array" then ."setup-worktree"[] else (."setup-worktree" // empty) end' \ + "$WORKTREES_JSON" | while IFS= read -r cmd; do + [[ -z "$cmd" || "$cmd" == "null" ]] && continue + info " \$ ${cmd}" + # Fail fast: a half-provisioned worktree (e.g. failed `uv sync`) would + # otherwise be reported as success and cause confusing downstream errors. + bash -c "$cmd" || error "setup step failed: ${cmd}" + done + else + # No .cursor/worktrees.json (or no jq): best-effort, project-agnostic + # setup. Copy any .env files from the main tree so the worktree can run, + # then sync the venv ONLY if this looks like a uv project. Non-uv repos + # simply skip that step instead of hard-failing. + warn "jq or .cursor/worktrees.json unavailable; running best-effort default setup" + rsync -am --exclude='node_modules' --exclude='.next' --exclude='.git' \ + --include='*/' --include='.env' --exclude='*' "${ROOT_WORKTREE_PATH}/" . \ + || warn "rsync of .env files into the worktree failed (continuing)" + if command -v uv >/dev/null 2>&1 && [[ -f pyproject.toml || -f uv.lock ]]; then + uv sync --all-packages || warn "uv sync --all-packages failed (continuing)" + fi + fi +) + +echo "WORKTREE_ID=${WORKTREE_ID}" +echo "WORKTREE_PATH=${WORKTREE_DIR}" +echo "BRANCH=${BRANCH}" diff --git a/skills/ess/pr-address-comments-all/scripts/fetch_pr.sh b/skills/ess/pr-address-comments-all/scripts/fetch_pr.sh new file mode 100755 index 0000000..21f4772 --- /dev/null +++ b/skills/ess/pr-address-comments-all/scripts/fetch_pr.sh @@ -0,0 +1,156 @@ +#!/usr/bin/env bash +# +# fetch_pr.sh +# Look up ONE PR reference and dump everything the skill needs to plan and +# address its review comments. +# +# Usage: fetch_pr.sh <pr-ref> [--repo owner/repo] [--output DIR] +# <pr-ref> - Full PR URL (https://github.com/<o>/<r>/pull/<N>) or a bare number +# --repo owner/repo - Repo to use for a bare number (default: current remote) +# --output DIR - Output directory (default: /tmp/pr-address-<owner>-<repo>-<number>) +# +# Writes into DIR: +# meta.env - OWNER=, REPO=, NUMBER=, BRANCH=, TITLE=, STATE= (shell-escaped) +# comments.json - raw inline review comments (gh api .../pulls/<N>/comments) +# threads.json - unresolved review threads as NDJSON {threadId,commentId,path,line,body} +# +# Also prints the meta block to stdout. Exits non-zero if gh fails -- never +# fabricates data. + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 1; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } +warn() { echo -e "${YELLOW}Warning:${NC} $1" >&2; } + +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" + +PR_REF="" +REPO_OVERRIDE="" +OUTPUT_DIR="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --repo) + REPO_OVERRIDE="${2:-}"; shift 2 ;; + --output) + OUTPUT_DIR="${2:-}"; shift 2 ;; + -h|--help) + sed -n '2,22p' "$0"; exit 0 ;; + -*) + error "Unknown option: $1" ;; + *) + if [[ -z "$PR_REF" ]]; then PR_REF="$1"; else error "Unexpected argument: $1"; fi + shift ;; + esac +done + +[[ -n "$PR_REF" ]] || error "Missing <pr-ref>. See --help." + +# Parse the PR reference into OWNER / REPO / NUMBER. +if [[ "$PR_REF" =~ ^https?://[^/]+/([^/]+)/([^/]+)/pull/([0-9]+) ]]; then + OWNER="${BASH_REMATCH[1]}" + REPO="${BASH_REMATCH[2]}" + NUMBER="${BASH_REMATCH[3]}" +elif [[ "$PR_REF" =~ ^[0-9]+$ ]]; then + NUMBER="$PR_REF" + if [[ -n "$REPO_OVERRIDE" ]]; then + OWNER="${REPO_OVERRIDE%%/*}" + REPO="${REPO_OVERRIDE#*/}" + else + NAME_WITH_OWNER="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" \ + || error "Could not determine repo from current remote; pass --repo owner/repo" + OWNER="${NAME_WITH_OWNER%%/*}" + REPO="${NAME_WITH_OWNER#*/}" + fi +else + error "Could not parse PR reference: ${PR_REF} (expected a URL or a bare number)" +fi + +# Default output dir is owner/repo-qualified so the same PR number in different +# repos does not collide under /tmp. Sanitize any '/' in owner/repo to '-'. +if [[ -z "$OUTPUT_DIR" ]]; then + SAFE_OWNER="${OWNER//\//-}" + SAFE_REPO="${REPO//\//-}" + OUTPUT_DIR="/tmp/pr-address-${SAFE_OWNER}-${SAFE_REPO}-${NUMBER}" +fi +mkdir -p "$OUTPUT_DIR" + +info "PR #${NUMBER} in ${OWNER}/${REPO}" + +# Metadata (branch is what the worktree checks out). gh has a built-in --jq, +# so no external jq/python dependency is needed. +BRANCH="$(gh pr view "$NUMBER" --repo "${OWNER}/${REPO}" --json headRefName --jq '.headRefName')" \ + || error "gh pr view failed for ${OWNER}/${REPO}#${NUMBER}" +TITLE="$(gh pr view "$NUMBER" --repo "${OWNER}/${REPO}" --json title --jq '.title')" \ + || error "gh pr view (title) failed for ${OWNER}/${REPO}#${NUMBER}" +STATE="$(gh pr view "$NUMBER" --repo "${OWNER}/${REPO}" --json state --jq '.state')" \ + || error "gh pr view (state) failed for ${OWNER}/${REPO}#${NUMBER}" + +# Inline review comments to address. +gh api "repos/${OWNER}/${REPO}/pulls/${NUMBER}/comments" > "${OUTPUT_DIR}/comments.json" \ + || error "gh api comments failed for ${OWNER}/${REPO}#${NUMBER}" + +# Unresolved review threads (thread id + first comment's databaseId/path/line/body), +# paginated through ALL pages so a PR with >100 threads is never truncated. One +# gh call per page; each page emits its unresolved threads as NDJSON plus a final +# "@@PAGE<TAB>hasNextPage<TAB>endCursor" marker, so no external jq is needed. +THREADS_QUERY=' +query($owner:String!, $repo:String!, $number:Int!, $cursor:String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $cursor) { + nodes { + id + isResolved + comments(first: 1) { nodes { databaseId path line originalLine body } } + } + pageInfo { hasNextPage endCursor } + } + } + } +}' +: > "${OUTPUT_DIR}/threads.json" +CURSOR="" +while :; do + if [[ -z "$CURSOR" ]]; then PAGE_ARG=(-F cursor=null); else PAGE_ARG=(-f cursor="$CURSOR"); fi + PAGE_OUT="$(gh api graphql -f query="$THREADS_QUERY" \ + -F owner="$OWNER" -F repo="$REPO" -F number="$NUMBER" "${PAGE_ARG[@]}" \ + --jq '(.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {threadId: .id, commentId: .comments.nodes[0].databaseId, path: .comments.nodes[0].path, line: (.comments.nodes[0].line // .comments.nodes[0].originalLine), body: .comments.nodes[0].body}), ("@@PAGE\t" + (.data.repository.pullRequest.reviewThreads.pageInfo.hasNextPage|tostring) + "\t" + (.data.repository.pullRequest.reviewThreads.pageInfo.endCursor // ""))')" \ + || error "gh api graphql (reviewThreads) failed for ${OWNER}/${REPO}#${NUMBER}" + HAS_NEXT="false"; NEXT_CURSOR="" + while IFS= read -r line; do + [[ -z "$line" ]] && continue + if [[ "$line" == "@@PAGE"$'\t'* ]]; then + IFS=$'\t' read -r _ HAS_NEXT NEXT_CURSOR <<< "$line" + else + printf '%s\n' "$line" >> "${OUTPUT_DIR}/threads.json" + fi + done <<< "$PAGE_OUT" + CURSOR="$NEXT_CURSOR" + [[ "$HAS_NEXT" == "true" && -n "$CURSOR" ]] || break +done + +THREAD_COUNT="$(grep -c '"threadId"' "${OUTPUT_DIR}/threads.json" 2>/dev/null || echo 0)" +info "Unresolved threads: ${THREAD_COUNT}" + +# Write + emit the meta block. Values are shell-escaped with printf %q so a +# title containing spaces/quotes/newlines round-trips safely when sourced. +META_FILE="${OUTPUT_DIR}/meta.env" +{ + printf 'OWNER=%q\n' "$OWNER" + printf 'REPO=%q\n' "$REPO" + printf 'NUMBER=%q\n' "$NUMBER" + printf 'BRANCH=%q\n' "$BRANCH" + printf 'TITLE=%q\n' "$TITLE" + printf 'STATE=%q\n' "$STATE" + printf 'OUTPUT_DIR=%q\n' "$OUTPUT_DIR" + printf 'THREAD_COUNT=%q\n' "$THREAD_COUNT" +} | tee "$META_FILE" + +info "Wrote ${META_FILE}, comments.json, threads.json to ${OUTPUT_DIR}" diff --git a/skills/ess/pr-address-comments-all/scripts/find_my_prs.sh b/skills/ess/pr-address-comments-all/scripts/find_my_prs.sh new file mode 100755 index 0000000..047a611 --- /dev/null +++ b/skills/ess/pr-address-comments-all/scripts/find_my_prs.sh @@ -0,0 +1,126 @@ +#!/usr/bin/env bash +# +# find_my_prs.sh +# Discover the current repo's open PRs authored by you that need attention: +# either the review decision is CHANGES_REQUESTED, or there is at least one +# unresolved review thread. +# +# Usage: find_my_prs.sh [--repo owner/repo] [--limit N] +# --repo owner/repo - Repo to query (default: current remote) +# --limit N - Max PRs to scan (default: 50) +# +# Output: +# stdout - matching PR numbers, one per line (for the skill to loop over) +# stderr - a human-readable table (number, decision, unresolved, branch, title) +# +# Exits 0 with empty stdout when nothing matches. + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 1; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } +warn() { echo -e "${YELLOW}Warning:${NC} $1" >&2; } + +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" + +OWNER_REPO="" +LIMIT="50" + +while [[ $# -gt 0 ]]; do + case "$1" in + --repo) OWNER_REPO="${2:-}"; shift 2 ;; + --limit) LIMIT="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,18p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +if [[ -z "$OWNER_REPO" ]]; then + OWNER_REPO="$(gh repo view --json nameWithOwner --jq .nameWithOwner)" \ + || error "Could not determine repo from current remote; pass --repo owner/repo" +fi +OWNER="${OWNER_REPO%%/*}" +REPO="${OWNER_REPO#*/}" + +info "Scanning my open PRs in ${OWNER_REPO} (limit ${LIMIT})" + +# My open PRs with their review decision and branch, as TSV (number<TAB>decision<TAB>branch<TAB>title). +PR_ROWS="$(gh pr list --repo "$OWNER_REPO" --author "@me" --state open --limit "$LIMIT" \ + --json number,reviewDecision,headRefName,title \ + --jq '.[] | [.number, (.reviewDecision // ""), .headRefName, .title] | @tsv')" \ + || error "gh pr list failed for ${OWNER_REPO}" + +if [[ -z "$PR_ROWS" ]]; then + info "No open PRs authored by you in ${OWNER_REPO}" + exit 0 +fi + +UNRESOLVED_QUERY=' +query($owner:String!, $repo:String!, $number:Int!, $cursor:String) { + repository(owner: $owner, name: $repo) { + pullRequest(number: $number) { + reviewThreads(first: 100, after: $cursor) { + nodes { isResolved } + pageInfo { hasNextPage endCursor } + } + } + } +}' + +# Count unresolved review threads for one PR, paginating through ALL threads (a +# single page of 100 can otherwise misclassify a busy PR as having 0 unresolved). +# Echoes the total on success; returns non-zero if any gh call fails. +count_unresolved_threads() { + local number="$1" + local cursor="" total=0 page resp count has_next end_cursor + while :; do + # First page passes a JSON null cursor (-F magic); later pages pass the + # opaque endCursor as a raw string (-f) to avoid any injection concerns. + if [[ -z "$cursor" ]]; then + page=(-F cursor=null) + else + page=(-f cursor="$cursor") + fi + resp="$(gh api graphql -f query="$UNRESOLVED_QUERY" \ + -F owner="$OWNER" -F repo="$REPO" -F number="$number" "${page[@]}" \ + --jq '.data.repository.pullRequest.reviewThreads as $t + | [ ([$t.nodes[] | select(.isResolved == false)] | length | tostring), + ($t.pageInfo.hasNextPage | tostring), + ($t.pageInfo.endCursor // "") ] | @tsv')" || return 1 + IFS=$'\t' read -r count has_next end_cursor <<< "$resp" + total=$(( total + count )) + if [[ "$has_next" == "true" && -n "$end_cursor" ]]; then + cursor="$end_cursor" + else + break + fi + done + printf '%s' "$total" +} + +printf 'PR\tDECISION\tUNRESOLVED\tBRANCH\tTITLE\n' >&2 + +MATCHES=() +while IFS=$'\t' read -r NUMBER DECISION BRANCH TITLE; do + [[ -z "$NUMBER" ]] && continue + UNRESOLVED="$(count_unresolved_threads "$NUMBER")" \ + || { warn "Could not read threads for #${NUMBER}; skipping"; continue; } + + if [[ "$DECISION" == "CHANGES_REQUESTED" || "$UNRESOLVED" -gt 0 ]]; then + MATCHES+=("$NUMBER") + printf '%s\t%s\t%s\t%s\t%s\n' "$NUMBER" "${DECISION:-NONE}" "$UNRESOLVED" "$BRANCH" "$TITLE" >&2 + fi +done <<< "$PR_ROWS" + +if [[ ${#MATCHES[@]} -eq 0 ]]; then + info "No open PRs need attention (no changes-requested or unresolved comments)" + exit 0 +fi + +info "Matched ${#MATCHES[@]} PR(s)" +printf '%s\n' "${MATCHES[@]}" diff --git a/skills/ess/pr-address-comments-all/scripts/reply_and_resolve.sh b/skills/ess/pr-address-comments-all/scripts/reply_and_resolve.sh new file mode 100755 index 0000000..0feff3b --- /dev/null +++ b/skills/ess/pr-address-comments-all/scripts/reply_and_resolve.sh @@ -0,0 +1,66 @@ +#!/usr/bin/env bash +# +# reply_and_resolve.sh +# Reply to a PR review comment and resolve its thread. Encapsulates the +# error-prone Step 9 of pr-address-comments (SKILL.md): +# - REST reply via POST /repos/<o>/<r>/pulls/<N>/comments with in_reply_to +# (NOT the /replies sub-resource, which 404s) +# - GraphQL resolveReviewThread mutation +# +# Usage: +# reply_and_resolve.sh --repo owner/repo --pr N \ +# --comment-id ID --thread-id TID --body "Fixed — <what changed>" +# +# Only call this for threads that were actually ADDRESSED in code. Skip +# discussion-only threads. + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +NC='\033[0m' + +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 1; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } + +OWNER_REPO="" +PR_NUMBER="" +COMMENT_ID="" +THREAD_ID="" +BODY="" + +while [[ $# -gt 0 ]]; do + case "$1" in + --repo) OWNER_REPO="${2:-}"; shift 2 ;; + --pr) PR_NUMBER="${2:-}"; shift 2 ;; + --comment-id) COMMENT_ID="${2:-}"; shift 2 ;; + --thread-id) THREAD_ID="${2:-}"; shift 2 ;; + --body) BODY="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,17p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +[[ -n "$OWNER_REPO" ]] || error "Missing --repo owner/repo" +[[ -n "$PR_NUMBER" ]] || error "Missing --pr N" +[[ -n "$COMMENT_ID" ]] || error "Missing --comment-id ID" +[[ -n "$THREAD_ID" ]] || error "Missing --thread-id TID" +[[ -n "$BODY" ]] || error "Missing --body \"message\"" +command -v gh >/dev/null 2>&1 || error "gh CLI not found on PATH" + +info "Replying to comment ${COMMENT_ID} on ${OWNER_REPO}#${PR_NUMBER}" +gh api "repos/${OWNER_REPO}/pulls/${PR_NUMBER}/comments" \ + -f body="$BODY" \ + -F in_reply_to="$COMMENT_ID" >/dev/null \ + || error "Reply failed for comment ${COMMENT_ID}" + +info "Resolving thread ${THREAD_ID}" +gh api graphql -f query=' +mutation($threadId:ID!) { + resolveReviewThread(input: {threadId: $threadId}) { + thread { isResolved } + } +}' -F threadId="$THREAD_ID" >/dev/null \ + || error "Resolve failed for thread ${THREAD_ID}" + +info "Replied + resolved thread ${THREAD_ID}" diff --git a/skills/ess/pr-address-comments-all/scripts/safe_push.sh b/skills/ess/pr-address-comments-all/scripts/safe_push.sh new file mode 100755 index 0000000..b05d27a --- /dev/null +++ b/skills/ess/pr-address-comments-all/scripts/safe_push.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# +# safe_push.sh +# Push the current worktree HEAD to a PR branch WITHOUT ever hanging, even when +# the remote branch moved while a subagent was working. This prevents the known +# failure where a raw `git push` is rejected (non-fast-forward) and a follow-up +# interactive `git pull --rebase` stalls a non-interactive background subagent. +# +# Behaviour: +# 1) Force non-interactive git so no editor/pager/credential prompt can block. +# 2) Try `git push <remote> HEAD:<branch>` (works for attached or detached HEAD). +# 3) On rejection: fetch, then non-interactive `git rebase <remote>/<branch>`: +# - clean rebase -> retry the push (bounded by --max-attempts) +# - CONFLICT -> `git rebase --abort` (leave NO rebase in progress) and +# exit 3 so the caller re-applies on the fresh base or +# escalates to the orchestrator. It never waits for input. +# +# Usage: safe_push.sh --branch B [--remote origin] [--max-attempts 3] +# +# Exit codes: +# 0 pushed successfully +# 2 bad usage / not in a git work tree / dirty worktree / pre-existing rebase|merge|cherry-pick|revert +# 3 remote moved with CONFLICTING changes -- re-apply on the updated base or +# escalate (guaranteed no rebase left in progress) +# 4 push still rejected after --max-attempts (e.g. a flapping remote) +# 5 operational git failure (e.g. fetch failed) + +set -euo pipefail + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +NC='\033[0m' + +error() { echo -e "${RED}Error:${NC} $1" >&2; exit 2; } +op_error() { echo -e "${RED}Error:${NC} $1" >&2; exit 5; } +info() { echo -e "${GREEN}→${NC} $1" >&2; } +warn() { echo -e "${YELLOW}Warning:${NC} $1" >&2; } + +BRANCH="" +REMOTE="origin" +MAX_ATTEMPTS=3 + +while [[ $# -gt 0 ]]; do + case "$1" in + --branch) BRANCH="${2:-}"; shift 2 ;; + --remote) REMOTE="${2:-}"; shift 2 ;; + --max-attempts) MAX_ATTEMPTS="${2:-}"; shift 2 ;; + -h|--help) sed -n '2,26p' "$0"; exit 0 ;; + *) error "Unknown argument: $1" ;; + esac +done + +[[ -n "$BRANCH" ]] || error "Missing --branch B" +if ! [[ "$MAX_ATTEMPTS" =~ ^[0-9]+$ ]] || (( MAX_ATTEMPTS < 1 )); then + error "--max-attempts must be an integer >= 1 (got: ${MAX_ATTEMPTS})" +fi +git rev-parse --is-inside-work-tree >/dev/null 2>&1 || error "Not inside a git work tree" +if ! git diff-index --quiet HEAD --; then + error "Working tree has uncommitted changes to tracked files -- commit or stash before pushing" +fi + +# A background agent has no TTY: an editor, pager, or credential prompt would +# block forever. Force every git step to be non-interactive. +export GIT_EDITOR=true +export GIT_SEQUENCE_EDITOR=true +export GIT_PAGER=cat +export PAGER=cat +export GIT_TERMINAL_PROMPT=0 + +# Refuse to start on top of a half-finished rebase/merge/cherry-pick/revert -- that +# is exactly the stuck state this script exists to avoid creating. +_git_path() { git rev-parse --git-path "$1"; } +if [[ -d "$(_git_path rebase-merge)" || -d "$(_git_path rebase-apply)" \ + || -f "$(_git_path MERGE_HEAD)" || -f "$(_git_path CHERRY_PICK_HEAD)" \ + || -f "$(_git_path REVERT_HEAD)" ]]; then + error "A rebase, merge, cherry-pick, or revert is already in progress -- finish or abort it before pushing" +fi + +attempt=1 +while (( attempt <= MAX_ATTEMPTS )); do + info "Push attempt ${attempt}/${MAX_ATTEMPTS}: ${REMOTE} HEAD:${BRANCH}" + if git push "$REMOTE" "HEAD:${BRANCH}"; then + info "Pushed to ${REMOTE}/${BRANCH}" + exit 0 + fi + + if (( attempt >= MAX_ATTEMPTS )); then + echo "SAFE_PUSH_REJECTED: push still rejected after ${MAX_ATTEMPTS} attempts (remote may be flapping)." >&2 + exit 4 + fi + + warn "Push rejected -- ${REMOTE}/${BRANCH} likely moved; fetching and rebasing" + git fetch "$REMOTE" -- "$BRANCH" || op_error "git fetch ${REMOTE} ${BRANCH} failed" + + if git rebase "${REMOTE}/${BRANCH}"; then + info "Rebased cleanly onto ${REMOTE}/${BRANCH}; retrying push" + attempt=$(( attempt + 1 )) + continue + fi + + # Conflict: abort so we never leave a half-finished rebase that hangs the agent. + warn "Rebase hit conflicts; aborting to leave a clean tree" + git rebase --abort || true + echo "SAFE_PUSH_CONFLICT: ${REMOTE}/${BRANCH} moved with conflicting changes." >&2 + echo "Re-apply the approved fixes on the updated base, or escalate to the orchestrator to finish inline." >&2 + exit 3 +done + +echo "SAFE_PUSH_REJECTED: push still rejected after ${MAX_ATTEMPTS} attempts (remote may be flapping)." >&2 +exit 4 diff --git a/skills/ess/pr-address-comments/LICENSE b/skills/ess/pr-address-comments/LICENSE new file mode 100644 index 0000000..9c7afe3 --- /dev/null +++ b/skills/ess/pr-address-comments/LICENSE @@ -0,0 +1,13 @@ +Copyright 2025 Cisco Systems, Inc. or its Affiliates + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. diff --git a/skills/ess/pr-address-comments/SKILL.md b/skills/ess/pr-address-comments/SKILL.md new file mode 100644 index 0000000..39dc005 --- /dev/null +++ b/skills/ess/pr-address-comments/SKILL.md @@ -0,0 +1,269 @@ +--- +name: pr-address-comments +description: >- + Read a remote GitHub PR's review comments, implement fixes on the PR branch, + then reply to and resolve addressed threads (REST reply + GraphQL + resolveReviewThread). Use for /pr-address-comments, a single PR URL or number, + when not using the parallel batch skill pr-address-comments-all. Requires gh + and a git checkout of the repo. +metadata: + version: "1.0" +--- + +# PR Address Comments + +Read a remote PR's review comments, implement fixes, then reply to and resolve the addressed threads. + +**Phase**: Fix + +## Quick start + +``` +/pr-address-comments https://github.com/<owner>/<repo>/pull/123 +/pr-address-comments 103 +``` + +--- + +## Prerequisites + +- **GitHub CLI** (`gh`) — authenticated and with access to the repository +- The current directory must be a git repo (`.git/` present) + +--- + +## Limitations + +- GitHub's REST API does not have a "resolve thread" endpoint. Threads must be resolved via the **GraphQL API** using `resolveReviewThread`. +- The `minimizeComment` mutation hides comments but does NOT resolve threads — do not use it for this purpose. +- Reply endpoint: `POST /repos/{owner}/{repo}/pulls/{number}/comments` with `in_reply_to` field. The older `/replies` sub-resource returns 404. + +--- + +## Instructions + +### Step 1: Resolve the PR + +Parse the user's input: + +- If a **full URL** is provided (e.g., `https://github.com/org/repo/pull/123`), extract the owner, repo, and PR number directly. +- If only a **number** is provided, determine the remote from `.git`: + +```bash +# Get the GitHub owner/repo from the git remote +gh repo view --json nameWithOwner --jq .nameWithOwner +``` + +### Step 2: Fetch PR Details and Review Comments + +```bash +# Get PR metadata +gh pr view <NUMBER> --json title,body,state,headRefName,files + +# Get inline review comments (these are the ones to address) +gh api repos/<OWNER>/<REPO>/pulls/<NUMBER>/comments +``` + +Parse each comment for: +- `id` — the comment ID (needed for responding later) +- `path` — the file path +- `line` / `original_line` — the line number +- `body` — the reviewer's feedback + +### Step 3: Get Unresolved Thread IDs (GraphQL) + +Fetch thread metadata upfront so you can resolve them later: + +```bash +gh api graphql -f query=' +query { + repository(owner: "<OWNER>", name: "<REPO>") { + pullRequest(number: <NUMBER>) { + reviewThreads(first: 100) { + nodes { + id + isResolved + comments(first: 1) { + nodes { + databaseId + path + body + } + } + } + } + } + } +}' --jq '.data.repository.pullRequest.reviewThreads.nodes[] | select(.isResolved == false) | {threadId: .id, commentId: .comments.nodes[0].databaseId, path: .comments.nodes[0].path, body: .comments.nodes[0].body}' +``` + +This gives you both the **thread ID** (for resolving) and the **comment database ID** (for replying). + +### Step 4: Categorize and Present + +Show the user a summary: + +```markdown +## PR #<NUMBER>: <TITLE> + +### Review Comments (<COUNT>) + +| # | File | Line | Comment | +|---|------|------|---------| +| 1 | path/to/file.py | 42 | "Add error handling..." | +| 2 | path/to/other.md | 17 | "Update reference..." | +``` + +### Step 5: Checkout the PR Branch + +```bash +git fetch origin <HEAD_REF_NAME> +git checkout <HEAD_REF_NAME> +``` + +### Step 6: Address All Comments + +For each comment: + +1. **Read** the file and surrounding context +2. **Implement** the fix suggested by the reviewer +3. **Verify** the fix (grep for stale references, check consistency) + +Work through all comments systematically. Group related fixes (e.g., multiple comments about the same file) to avoid redundant reads. + +**Track which comments were addressed** — maintain a list of comment IDs that received code fixes. Only these will be replied to and resolved in Step 9. + +### Step 7: Verify + +Run verification checks based on the nature of the fixes: + +```bash +# Example: check for stale references +grep -r "<old-term>" <relevant-dirs>/ + +# Example: confirm file structure matches docs +ls <dir>/ +``` + +### Step 8: Commit + +Commit using [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#specification). + +**Check if commitizen is available:** + +```bash +command -v cz >/dev/null 2>&1 && echo "cz available" || echo "cz not found" +``` + +**If `cz` is installed**, use it interactively: + +```bash +git add <files> +cz c +``` + +Follow the commitizen prompts, selecting the appropriate type (usually `fix`) and scope. + +**If `cz` is NOT installed**, commit manually following the spec: + +```bash +git add <files> +git commit -m "fix(<scope>): address PR #<NUMBER> review comments + +<brief description of what was fixed>" +``` + +**Conventional Commits format:** + +``` +<type>(<scope>): <short description> + +[optional body — what and why] + +[optional footer(s)] +``` + +| Type | When to use | +|------|-------------| +| `fix` | Addressing review comments, bug fixes | +| `refactor` | Code restructuring without behavior change | +| `docs` | Documentation-only changes | +| `style` | Formatting, whitespace | +| `chore` | Tooling, config, non-production changes | + +### Step 9: Reply and Resolve Addressed Threads + +For each comment that was **addressed in Step 6**, reply and resolve its thread. + +**Reply to each comment (REST API):** + +```bash +gh api repos/<OWNER>/<REPO>/pulls/<NUMBER>/comments \ + -f body="<reply-message>" \ + -F in_reply_to=<COMMENT_DATABASE_ID> +``` + +Reply format: +- Be concise: "Fixed — <what was done>." +- Reference the specific change if helpful: "Changed `grep hello-world` to `grep hello-agent-auth`." +- Do NOT repeat the reviewer's comment back to them. + +**IMPORTANT:** The reply endpoint is: +``` +POST /repos/{owner}/{repo}/pulls/{pull_number}/comments +``` +with `in_reply_to` as a field in the body. Do NOT use `/replies` sub-resource (returns 404). + +**Resolve each thread (GraphQL):** + +```bash +gh api graphql -f query=' +mutation { + resolveReviewThread(input: {threadId: "<THREAD_ID>"}) { + thread { isResolved } + } +}' +``` + +Only resolve threads whose comments were addressed in code. Skip threads that: +- Require discussion rather than a code fix +- Were not implemented (skipped or deferred) + +### Step 10: Summary + +```markdown +## Done + +### Addressed & Resolved +| # | File | Comment | Reply | +|---|------|---------|-------| +| 1 | path/to/file.md | "Update reference..." | Fixed — updated to use new name. | +| 2 | path/to/other.py | "Remove auth..." | Fixed — removed auth prerequisites. | + +### Skipped (not addressed) +| # | File | Comment | Reason | +|---|------|---------|--------| +| 3 | path/to/config.py | "Consider refactoring..." | Requires discussion | + +**Resolved**: X / Y threads +**Skipped**: Z (not addressed in code) +``` + +--- + +## Error Handling + +| Error | Cause | Fix | +|-------|-------|-----| +| 404 on reply | Used wrong endpoint (`/replies`) | Use `POST /pulls/{number}/comments` with `in_reply_to` field | +| 422 on resolve | Thread already resolved | Safe to ignore — thread is already in desired state | +| 403 on GraphQL | Token lacks `write:discussion` scope | Re-authenticate: `gh auth refresh -s write:discussion` | + +--- + +## Related Commands + +- `/fix-pr-comments` — Interactive mode (asks before each fix) +- `/review-pr` — Review someone else's PR +- `/create-pr` — Create a new PR +- `pr-address-comments-all` — Parallel batch with worktrees and approval gate diff --git a/skills/ess/summarize-change-log/LICENSE b/skills/ess/summarize-change-log/LICENSE new file mode 100644 index 0000000..9c7afe3 --- /dev/null +++ b/skills/ess/summarize-change-log/LICENSE @@ -0,0 +1,13 @@ +Copyright 2025 Cisco Systems, Inc. or its Affiliates + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. diff --git a/skills/ess/summarize-change-log/SKILL.md b/skills/ess/summarize-change-log/SKILL.md new file mode 100644 index 0000000..7bc5d4d --- /dev/null +++ b/skills/ess/summarize-change-log/SKILL.md @@ -0,0 +1,91 @@ +--- +name: summarize-change-log +description: >- + Condense long commit logs, squash messages, or change narratives into 1–5 + GitHub-ready markdown bullets with Conventional Commit subjects (≤50 chars). + Use when the user asks to summarize a git log, boil down commits, write a + squash-merge message, or produce a five-bullet PR/merge summary. +--- + +# Summarize Change Log + +Turn a long git log, squash message draft, or PR narrative into **1–5 copy-paste markdown bullets** for a GitHub merge commit or extended description. + +## Output format + +Return **only** markdown bullets. No preamble, no section headings unless the user asks. + +```markdown +- **type(scope): subject** — One sentence: what changed and why it matters. +``` + +- **1 to 5 bullets, never more than 5.** +- Preserve `Co-authored-by:` footers from the input **only if present**; place after bullets with a blank line before them. + +## Subject rules + +Follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/#specification): + +- Format: `<type>[optional scope]: <description>` — imperative mood, lowercase type. +- Types: `feat`, `fix`, `docs`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`. +- **Entire subject ≤ 50 characters** (type, scope, colon, space, description). +- Scope matches the primary area (`langsmith-client`, `azure-ai`, `hello-agent`). + +### What changed, not process + +Every subject must state **what changed**. Ban vague/process subjects: + +| Bad | Good | +| --- | --- | +| `address PR #513 review comments` | `fix(langsmith-client): defer delete confirm` | +| `port docs from #510` | `docs(langsmith-client): document --deployment` | +| `follow up on review` | `fix(hello-agent): align listener id messaging` | + +## Consolidation (>5 input commits) + +1. Group by scope + intent; merge overlapping fixes. +2. Keep distinct areas separate (infra vs CLI vs docs). +3. If still >5, drop least user-visible items; fold detail into a bullet body. + +## Validation (required) + +Before returning output, run from this skill's directory: + +```bash +scripts/validate-summary.sh /tmp/draft-summary.md +``` + +Or validate the checked-in fixture: + +```bash +scripts/validate-summary.sh \ + examples-good.md +``` + +Or pipe stdin: + +```bash +printf '%s\n' '- **fix: short subject** — Details.' \ + | scripts/validate-summary.sh +``` + +The script enforces: + +- 1–5 bullets in the expected format +- `uv run cz check -m … -l 50` (explicit 50-char limit; config alone is not applied without `-l`) +- Vague-subject blocklist + +Fix every failure and re-run until exit 0. Only then return the markdown. + +## Workflow + +1. Read input; extract **outcomes**, not chronology. +2. Draft 1–5 bullets; merge if needed. +3. Rewrite process-oriented subjects to outcome-oriented ones. +4. **Validate** with `validate-summary.sh`. +5. Output validated markdown only. + +## Examples + +- [examples.md](examples.md) — before/after narrative and bad-vs-good subjects +- [examples-good.md](examples-good.md) — canonical validated bullets (fixture for `validate-summary.sh`) diff --git a/skills/ess/summarize-change-log/examples-good.md b/skills/ess/summarize-change-log/examples-good.md new file mode 100644 index 0000000..5dc6f78 --- /dev/null +++ b/skills/ess/summarize-change-log/examples-good.md @@ -0,0 +1,5 @@ +- **fix(langsmith-client): centralize deploy naming** — Delegate `<service>-<env>` resolution to `langsmith-deploy-docker`, add `--deployment` for explicit base names, and resolve live deployments for idempotent delete. +- **fix(azure-ai): align prod NAT gateway IPs** — Prod Azure OpenAI firewall rules were allowlisting dev EKS egress; prod and aoh-prod stacks now use the aoh-langsmith-hybrid-prod NAT IP. +- **fix(langsmith-client): defer delete confirm** — Fix `deploy_docker` docstring, defer delete confirmation until a target exists, and remove erroneous `.env` gates from deploy scripts. +- **docs(langsmith-client): document --deployment** — Clarify `DEPLOYMENT_NAME` in `push_secrets.py` and document `--deployment` in `docker-deployment.md`. +- **fix(hello-agent): align listener id messaging** — Use `resolve_deploy_base` consistently, fix deploy docs/env references, and align `LANGSMITH_LISTENER_ID` messaging with the script requirement. diff --git a/skills/ess/summarize-change-log/examples.md b/skills/ess/summarize-change-log/examples.md new file mode 100644 index 0000000..5e8c663 --- /dev/null +++ b/skills/ess/summarize-change-log/examples.md @@ -0,0 +1,52 @@ +# Examples + +## Before (verbose input) + +``` +* fix(langsmith-client): centralize deployment naming in deploy CLI + +Shell scripts were building deployment names and passing pre-suffixed --name +values, which produced double-encoded names like hello-agent-prod-dev. ... + +* fix(azure-ai): align prod NAT gateway IPs with LangSmith hybrid cluster +... + +* fix(langsmith-client): address PR #513 review comments +... + +* docs(langsmith-client): port deployment naming docs from #510 +... + +* fix(langsmith-client,hello-agent): address PR #513 review comments +... +``` + +## After (5 bullets, validated) + +See [examples-good.md](examples-good.md) — the canonical fixture `validate-summary.sh` runs against. + +Subject lengths (all ≤50): + +| Subject | Length | +| --- | --- | +| `fix(langsmith-client): centralize deploy naming` | 47 | +| `fix(azure-ai): align prod NAT gateway IPs` | 41 | +| `fix(langsmith-client): defer delete confirm` | 43 | +| `docs(langsmith-client): document --deployment` | 45 | +| `fix(hello-agent): align listener id messaging` | 45 | + +## Bad vs good subjects + +| Bad (process/meta) | Good (outcome) | +| --- | --- | +| `fix: address PR #513 review comments` | `fix(langsmith-client): defer delete confirm` | +| `docs: port deployment naming docs from #510` | `docs(langsmith-client): document --deployment` | +| `chore: follow up on review` | `fix(hello-agent): align listener id messaging` | +| `fix: cherry-pick doc improvements` | `docs(langsmith-client): document --deployment` | + +## Validate the good example + +```bash +scripts/validate-summary.sh \ + examples-good.md +``` diff --git a/skills/ess/summarize-change-log/scripts/test_validate_summary.py b/skills/ess/summarize-change-log/scripts/test_validate_summary.py new file mode 100644 index 0000000..f9a46a2 --- /dev/null +++ b/skills/ess/summarize-change-log/scripts/test_validate_summary.py @@ -0,0 +1,96 @@ +"""Tests for summarize-change-log summary validation.""" + +from __future__ import annotations + +import sys +from pathlib import Path +from unittest.mock import MagicMock, patch + +import pytest + +sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from validate_summary import ( # noqa: E402 + ValidationError, + is_vague_subject, + parse_document, + validate_summary_text, +) + +_REPO_ROOT = Path(__file__).resolve().parents[5] + + +def test_parse_document_extracts_subjects() -> None: + text = "- **fix(api): short subject** — Details.\n" + document = parse_document(text) + assert document.subjects == ["fix(api): short subject"] + assert document.footers == [] + + +def test_parse_document_allows_footer_after_blank_line() -> None: + text = ( + "- **fix(api): short subject** — Details.\n" + "\n" + "Co-authored-by: Alice <alice@example.com>\n" + ) + document = parse_document(text) + assert len(document.subjects) == 1 + assert len(document.footers) == 1 + + +def test_parse_document_rejects_footer_without_blank_line() -> None: + text = ( + "- **fix(api): short subject** — Details.\n" + "Co-authored-by: Alice <alice@example.com>\n" + ) + with pytest.raises(ValidationError, match="blank line required"): + parse_document(text) + + +def test_parse_document_rejects_footer_between_bullets() -> None: + text = ( + "- **fix(api): first** — One.\n" + "\n" + "Co-authored-by: Alice <alice@example.com>\n" + "- **fix(api): second** — Two.\n" + ) + with pytest.raises(ValidationError, match="must follow all bullets"): + parse_document(text) + + +def test_parse_document_rejects_malformed_bullet() -> None: + with pytest.raises(ValidationError, match="malformed bullet"): + parse_document("- fix(api): missing bold wrapper — Details.\n") + + +def test_parse_document_rejects_unexpected_line() -> None: + with pytest.raises(ValidationError, match="unexpected non-bullet"): + parse_document("Intro prose\n- **fix(api): ok** — Details.\n") + + +def test_is_vague_subject_detects_port_from_inside_support() -> None: + assert not is_vague_subject("fix(api): support migration from #123") + + +def test_is_vague_subject_detects_port_from_phrase() -> None: + assert is_vague_subject("docs: port deployment naming docs from #510") + + +@patch("validate_summary.run_commitizen_check") +def test_validate_summary_text_checks_each_subject(mock_cz: MagicMock) -> None: + text = ( + "- **fix(api): short subject** — Details.\n" + "- **docs(web): update guide** — More.\n" + ) + expected_count = len(parse_document(text).subjects) + count = validate_summary_text(text, repo_root=_REPO_ROOT) + assert count == expected_count + assert mock_cz.call_count == expected_count + + +@patch("validate_summary.run_commitizen_check") +def test_validate_summary_text_rejects_vague_subject(mock_cz: MagicMock) -> None: + text = "- **fix: address PR #123** — Process-only subject.\n" + with pytest.raises(ValidationError, match="vague subject"): + validate_summary_text(text, repo_root=_REPO_ROOT) + mock_cz.assert_not_called() diff --git a/skills/ess/summarize-change-log/scripts/validate-summary.sh b/skills/ess/summarize-change-log/scripts/validate-summary.sh new file mode 100755 index 0000000..148dd5e --- /dev/null +++ b/skills/ess/summarize-change-log/scripts/validate-summary.sh @@ -0,0 +1,11 @@ +#!/usr/bin/env bash +# Validate summarize-change-log markdown bullets (wrapper for validate_summary.py). +set -euo pipefail + +if ! command -v uv >/dev/null 2>&1; then + echo "validate-summary: uv not found on PATH (required for validation)" >&2 + exit 1 +fi + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +exec uv run python "$SCRIPT_DIR/validate_summary.py" "$@" diff --git a/skills/ess/summarize-change-log/scripts/validate_summary.py b/skills/ess/summarize-change-log/scripts/validate_summary.py new file mode 100644 index 0000000..83751df --- /dev/null +++ b/skills/ess/summarize-change-log/scripts/validate_summary.py @@ -0,0 +1,185 @@ +#!/usr/bin/env python3 +"""Validate 1–5 markdown summary bullets for the summarize-change-log skill.""" + +from __future__ import annotations + +import argparse +import re +import subprocess +import sys +from dataclasses import dataclass +from pathlib import Path + +MESSAGE_LENGTH_LIMIT = 50 +MAX_BULLETS = 5 + +BULLET_PATTERN = re.compile(r"^\s*-\s+\*\*([^*]+)\*\*\s*(—|-)\s*") +LIST_LINE_PATTERN = re.compile(r"^\s*-\s+") +CO_AUTHOR_PATTERN = re.compile(r"^Co-authored-by:") + +VAGUE_SUBJECT_PATTERNS = ( + re.compile(r"[Aa][Dd][Dd][Rr][Ee][Ss][Ss]\s+[Pp][Rr]"), + re.compile(r"[Rr][Ee][Vv][Ii][Ee][Ww]\s+[Cc][Oo][Mm][Mm][Ee][Nn][Tt]"), + re.compile(r"(^|[^a-zA-Z0-9])[Pp][Oo][Rr][Tt]\s+.*[Ff][Rr][Oo][Mm]\s+#"), + re.compile(r"[Ff][Oo][Ll][Ll][Oo][Ww]\s+[Uu][Pp]"), + re.compile(r"[Cc][Hh][Ee][Rr][Rr][Yy][- ][Pp][Ii][Cc][Kk]"), +) + + +class ValidationError(Exception): + """Summary markdown failed skill validation.""" + + +@dataclass(frozen=True) +class SummaryDocument: + """Parsed bullet subjects and optional Co-authored-by footers.""" + + subjects: list[str] + footers: list[str] + + +def find_repo_root(start: Path) -> Path: + """Walk parents until a pyproject.toml with Commitizen config is found.""" + path = start.resolve() + while True: + pyproject = path / "pyproject.toml" + if pyproject.is_file() and "[tool.commitizen]" in pyproject.read_text( + encoding="utf-8" + ): + return path + if path.parent == path: + raise ValidationError( + "could not find repo root with [tool.commitizen] in pyproject.toml" + ) + path = path.parent + + +def is_vague_subject(subject: str) -> bool: + """Return True when the subject matches a banned process/meta phrase.""" + return any(pattern.search(subject) for pattern in VAGUE_SUBJECT_PATTERNS) + + +def parse_document(text: str) -> SummaryDocument: + """Parse bullets and Co-authored-by footers, enforcing layout rules.""" + subjects: list[str] = [] + footers: list[str] = [] + saw_bullet = False + saw_footer = False + prev_was_blank = False + + for line in text.splitlines(): + if not line.strip(): + if saw_bullet and not saw_footer: + prev_was_blank = True + continue + + bullet_match = BULLET_PATTERN.match(line) + if bullet_match: + if saw_footer: + raise ValidationError("Co-authored-by footer must follow all bullets") + subjects.append(bullet_match.group(1)) + saw_bullet = True + prev_was_blank = False + continue + + if LIST_LINE_PATTERN.match(line): + raise ValidationError( + "malformed bullet(s); expected - **type(scope): subject** — ..." + ) + + if CO_AUTHOR_PATTERN.match(line): + if not saw_bullet: + raise ValidationError("Co-authored-by footer must follow bullets") + if not saw_footer and not prev_was_blank: + raise ValidationError( + "blank line required before Co-authored-by footer" + ) + footers.append(line) + saw_footer = True + prev_was_blank = False + continue + + raise ValidationError(f"unexpected non-bullet line: {line}") + + return SummaryDocument(subjects=subjects, footers=footers) + + +def run_commitizen_check(subject: str, repo_root: Path) -> None: + """Run ``uv run cz check`` for a single Conventional Commit subject.""" + result = subprocess.run( + [ + "uv", + "run", + "cz", + "check", + "-m", + subject, + "-l", + str(MESSAGE_LENGTH_LIMIT), + ], + cwd=repo_root, + check=False, + ) + if result.returncode != 0: + raise ValidationError(f"failed Conventional Commits check for: {subject}") + + +def validate_summary_text(text: str, *, repo_root: Path) -> int: + """Validate summary markdown. Returns bullet count on success.""" + document = parse_document(text) + bullet_count = len(document.subjects) + + if bullet_count == 0: + raise ValidationError( + "no bullets found (expected: - **type(scope): subject** — ...)" + ) + if bullet_count > MAX_BULLETS: + raise ValidationError( + f"too many bullets ({bullet_count}); maximum is {MAX_BULLETS}" + ) + + for subject in document.subjects: + if is_vague_subject(subject): + raise ValidationError( + f"vague subject (describe what changed, not process): {subject}" + ) + run_commitizen_check(subject, repo_root) + + return bullet_count + + +def _read_input(path: Path | None) -> str: + if path is None: + return sys.stdin.read() + if not path.is_file(): + raise ValidationError(f"file not found: {path}") + return path.read_text(encoding="utf-8") + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser( + description="Validate summarize-change-log markdown bullets.", + ) + parser.add_argument( + "path", + nargs="?", + help="Summary markdown file (stdin when omitted)", + ) + args = parser.parse_args(argv) + + try: + input_path = Path(args.path).resolve() if args.path else None + text = _read_input(input_path) + script_dir = Path(__file__).resolve().parent + repo_root = find_repo_root(script_dir) + bullet_count = validate_summary_text(text, repo_root=repo_root) + except ValidationError as exc: + print(f"validate-summary: {exc}", file=sys.stderr) + return 1 + + print(f"validate-summary: OK ({bullet_count} bullet(s))") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From bd5a2664f2de98b4f9fc341d640c4cb56eac263b Mon Sep 17 00:00:00 2001 From: Matt Norris <matt@mattnorris.dev> Date: Mon, 20 Jul 2026 23:41:41 -0400 Subject: [PATCH 2/2] docs(skills): document --full-depth for nested ess/ install Verified npx skills add installs all four skills; the CLI needs --full-depth to discover skills under skills/ess/. --- skills/README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/skills/README.md b/skills/README.md index 55db9d7..388ca85 100644 --- a/skills/README.md +++ b/skills/README.md @@ -11,15 +11,19 @@ Install with the standard agent-skills CLI ([skills.sh](https://skills.sh)). It From a local clone (run in the repo root): ```bash -npx skills add ./skills --skill '*' # all skills -npx skills add ./skills --skill mcp-hide-secrets # a single skill -npx skills add ./skills --list # list without installing +npx skills add ./skills --skill '*' --full-depth # all skills +npx skills add ./skills --skill mcp-hide-secrets --full-depth # a single skill +npx skills add ./skills --list --full-depth # list without installing ``` -Or straight from GitHub (replace `<owner>` with the repo owner): +`--full-depth` is required so the CLI discovers skills under nested paths like `skills/ess/` (not just top-level directories). + +The CLI writes project skills to `.agents/skills/<name>/` (which Cursor reads natively, alongside `.cursor/skills/`). These are gitignored install output — the source of truth stays in `skills/`. + +Or straight from GitHub: ```bash -npx skills add <owner>/essentials --skill '*' +npx skills add CiscoDevNet/essentials --skill '*' --full-depth ``` ### Install matrix