Perform code optimization and document cleanup (#1)

## Summary - add repository-wide quality tooling and verification scaffolding, including CI workflows, pnpm workspace setup, ESLint/Prettier/markdown checks, and generated-output verification helpers - reorganize skill sources and generation flow by introducing canonical `_source` variants, generator/manifests, reusable helper abstractions, and shared web-automation/browser utilities - clean up and expand documentation so the root README flows into docs and skill docs, with clearer development, reviewer, installer, and workflow guidance ## Notable changes - docs flow and consistency cleanup across `README.md`, `docs/README.md`, and related docs - new scripts for `check`, docs verification, generated-file verification, shell portability, and safe directory replacement - refactors in Atlassian and web-automation skill runtimes to reduce duplication and centralize reusable code - changelog, development documentation, and CI surface updates ## Test Plan - [ ] `pnpm run check` - [ ] review generated/manifests and skill sync outputs - [ ] smoke-check docs flow from `README.md` to `docs/README.md` to skill docs ## Notes - this branch currently includes tracked `skills/web-automation/shared/node_modules` content that should be reviewed carefully as potentially noisy/accidental committed artifacts Co-authored-by: Stefano Fiorini <stefano.fiorini@firsthorizon.com> Reviewed-on: #1
2026-05-04 04:41:34 +00:00
parent 2deab1c1b4
commit 251148c3ff
373 changed files with 28504 additions and 1281 deletions
@@ -0,0 +1,460 @@
+# Development Guide — ai-coding-skills
+
+This document covers prerequisites, how to run checks locally, the quality
+tooling, the workspace policy, and the `pnpm run check` contract.
+
+## Prerequisites
+
+| Tool | Minimum version | Install |
+|------|----------------|---------|
+| Node.js | 20 | `fnm install 22` or `nvm install 22` |
+| pnpm | 10 | `npm install -g pnpm` |
+| `rg` (ripgrep) | any | `brew install ripgrep` / `apt-get install ripgrep` |
+| **shellcheck** | **any** | `brew install shellcheck` / `apt-get install shellcheck` |
+| `stat` (BSD or GNU) | any | pre-installed on macOS; GNU variant on Linux |
+| Python 3 | 3.8+ | pre-installed on most systems |
+
+**`shellcheck` is required.** The `pnpm run lint` script will exit 2 with a
+clear error message if `shellcheck` is not on `PATH`.  Every contributor must
+install it before running any quality checks.
+
+## Quick start
+
+```bash
+# Install dependencies (workspace-aware, no nested package.json modifications)
+pnpm install
+
+# Run the full quality suite
+pnpm run check
+```
+
+## Individual checks
+
+| Command | What it does |
+|---------|-------------|
+| `pnpm run sync:pi` | Mirror pi skill variants into `pi-package/skills/` |
+| `pnpm run verify:pi` | Assert pi resource and workflow invariants |
+| `pnpm run verify:reviewers` | Assert reviewer-runtime skill invariants |
+| `pnpm run test:installer` | Run root-level Node.js unit tests (22 tests) |
+| `pnpm run lint` | ESLint on root scripts + shellcheck on all `.sh` files |
+| `pnpm run lint:fix` | Auto-fix ESLint + Prettier (do not run on pre-existing code until M2) |
+| `pnpm run typecheck` | TypeScript `tsc --noEmit` in workspace packages |
+| `pnpm run test` | Run all tests (root + workspace packages) |
+| `pnpm run verify:docs` | markdownlint + offline link-check + docs-flow verifier |
+| `pnpm run verify:docs:online` | Same as `verify:docs` but with full external link checking |
+| `pnpm run verify:generated` | Assert generated output freshness (stub; fleshed out in M3) |
+| `pnpm run verify:ci` | Assert CI workflow files carry no pnpm version pins (see [pnpm version pinning](#pnpm-version-pinning)) |
+| `pnpm run check` | Aggregate: run every gate above and report a summary |
+
+## Quality tooling (added in M1)
+
+### ESLint
+
+Root-level flat config in `eslint.config.mjs`.  Covers `scripts/**/*.mjs`
+and `scripts/**/*.js`.  Uses `@eslint/js` recommended rules and Node.js
+globals.  Nested workspace packages are responsible for their own ESLint
+configuration.
+
+### Prettier
+
+Config in `.prettierrc.json` (print-width 100, LF line endings).  Ignore
+file at `.prettierignore` excludes generated agent-variant directories and
+`pnpm-lock.yaml`.
+
+### markdownlint
+
+Config in `.markdownlint.jsonc` (rules) and `.markdownlint-cli2.jsonc`
+(file globs and ignores).  Key overrides vs defaults:
+
+- `MD013` line-length relaxed to 120 chars (code blocks and tables excluded).
+- `MD033` (inline HTML) disabled.
+- `MD034` (bare URLs) disabled.
+- `MD041` (first-line heading) disabled.
+- `MD060` (table column style) disabled.
+
+Run `pnpm run verify:docs` to lint all `README.md`, `docs/*.md`, and
+`skills/**/SKILL.md` files (node\_modules excluded automatically).
+
+### markdown-link-check
+
+Two configs:
+
+- `markdown-link-check.json` — **offline** mode (default): ignores all
+  `http://` and `https://` links.  Safe for local dev and CI.
+- `markdown-link-check.online.json` — **online** mode: checks external links
+  with a 10 s timeout, 2 retries, and retry-on-429.  Use `--online` flag on
+  `scripts/lib/run-link-check.mjs`.
+
+`pnpm run verify:docs` uses the offline config by default.
+
+### shellcheck
+
+Wrapper script at `scripts/lib/run-shellcheck.mjs`.  Discovers every `*.sh`
+file under `scripts/` and `skills/` (excluding node\_modules and generated
+agent-variant directories) and runs shellcheck on each.
+
+**Installation:**
+
+- macOS: `brew install shellcheck`
+- Debian/Ubuntu: `sudo apt-get install shellcheck`
+- Other: <https://github.com/koalaman/shellcheck#installing>
+
+The wrapper exits with code **2** (not 1) when shellcheck is missing, so CI
+can distinguish "shellcheck absent" from "shellcheck found violations".
+
+## Cross-platform shell support (M2)
+
+All shell scripts under `scripts/` and `skills/reviewer-runtime/` that are
+exercised by `verify:pi`, `verify:reviewers`, `sync:pi`, and `test:installer`
+must work on both **macOS** (BSD userland) and **Ubuntu/Debian** (GNU
+userland) without modification.
+
+### BSD vs GNU differences encountered
+
+| Feature | macOS (BSD) | Linux (GNU) | Portable form |
+|---------|-------------|-------------|---------------|
+| `stat` permissions | `stat -f '%Lp' <path>` | `stat -c '%a' <path>` | `portable_stat_perms` helper |
+| herestrings (`<<<`) | supported (bash) | supported (bash) | OK (scripts use `#!/usr/bin/env bash`) |
+| `find -E` (extended regex) | macOS-only | not available | use `grep` or POSIX `-name` |
+| `sed -i ''` | macOS form | use `sed -i` on Linux | detect or avoid in-place sed |
+| `date -j` (date arithmetic) | macOS-only | use `date -d` on Linux | Node helper or `date +%s` |
+| `readlink -f` | not on macOS by default | GNU standard | `realpath` or `cd && pwd` |
+
+### `scripts/lib/portable.sh`
+
+A shared helper that abstracts the two known BSD/GNU divergences hit in
+this repo:
+
+```bash
+# Source from any script that needs portable stat
+# shellcheck source=lib/portable.sh
+source "$(dirname "${BASH_SOURCE[0]}")/lib/portable.sh"
+
+# Returns octal permission string: e.g. "755"
+portable_stat_perms "$path"
+```
+
+The `shellcheck` wrapper passes `-x --source-path=SCRIPTDIR` so source
+directives resolve relative to the script file, not the working directory.
+
+### How to run the Ubuntu smoke test locally
+
+```bash
+# Requires Docker
+docker run --rm \
+  -v "$PWD:/w" \
+  -w /w \
+  node:20-bookworm \
+  bash -lc 'apt-get update -q && apt-get install -y -q shellcheck ripgrep python3 \
+            && corepack enable \
+            && pnpm install --frozen-lockfile \
+            && pnpm run check'
+```
+
+This runs the full `pnpm run check` suite (lint, typecheck, test, verify:pi,
+verify:reviewers, verify:docs, verify:generated) inside a clean Debian Bookworm
+container with Node 20.
+
+## `check` contract (final — M5)
+
+`pnpm run check` is **fully green** on a clean checkout with the documented
+prerequisites installed.
+
+```text
+PASS  lint
+PASS  typecheck
+PASS  test
+PASS  verify:pi
+PASS  verify:reviewers
+PASS  verify:docs
+PASS  verify:generated
+PASS  verify:ci
+```
+
+This is the only acceptable state for merge.  Any failure on a check not in
+`docs/CLEANUP-BASELINE.md` is a regression and must be fixed before merge.
+
+> **History:** In M1 the contract was transitional and allowed pre-existing
+> failures recorded in the baseline.  M2 fixed `verify:docs`.  M3 fixed
+> `lint` and promoted `verify:generated` from stub to real implementation.
+> M4 added abstractions without regressions.  M5 added CI and finalized docs.
+> The baseline is now closed out — see `docs/CLEANUP-BASELINE.md`.
+
+## pnpm workspace policy (updated in M3)
+
+The `pnpm-workspace.yaml` at the repo root uses a **positive-include** policy
+introduced in M3.  There are no negative-glob exclusions.
+
+**Canonical source packages** (never generated):
+
+- `skills/atlassian/shared/scripts` — shared Atlassian TypeScript runtime
+- `skills/web-automation/shared` — shared web-automation runtime template
+
+**Generated agent-variant packages** (uniquely named, positively included):
+
+- `skills/atlassian/{claude-code,codex,cursor,opencode,pi}/scripts`
+  → names `@ai-coding-skills/atlassian-{claude-code,codex,cursor,opencode,pi}`
+- `skills/web-automation/{claude-code,codex,cursor,opencode,pi}/scripts`
+  → names `@ai-coding-skills/web-automation-{claude-code,codex,cursor,opencode,pi}`
+- `pi-package/skills/atlassian/scripts`
+  → name `@ai-coding-skills/atlassian-pi-mirror`
+- `pi-package/skills/web-automation/scripts`
+  → name `@ai-coding-skills/web-automation-pi-mirror`
+
+**Why unique names matter:**
+
+Each package in a pnpm workspace must have a distinct `name` field.  In M1 all
+generated agent-variant packages shared the same non-unique name as their
+canonical source package.  In M3 every package receives a scoped unique name
+of the form `@ai-coding-skills/<skill>-<agent>`, enabling pnpm to include them
+alongside canonical source packages without conflicts.  The pi-package mirrors
+use the `-mirror` suffix to distinguish them from the `pi` agent variants.
+
+All generated packages are `"private": true` and are never published to any
+registry.
+
+After `pnpm install`, `git status` should show zero modifications to any
+package.json file under any generated directory.  If it does not, the workspace
+config or generator is broken.
+
+## How to interpret the baseline report
+
+See `docs/CLEANUP-BASELINE.md` for the historical as-is capture from M1.  That
+baseline is now **closed** — all pre-existing failures have been resolved.
+When a CI run fails on any check it is a regression and must be fixed before
+merge.
+
+## How variants are generated (M3)
+
+Every agent-variant directory under `skills/<skill>/<agent>/` and every
+`pi-package/skills/<skill>/` mirror is **generator-owned**.  Do not edit
+files inside these directories directly — edit the canonical source instead
+and run `pnpm run sync:pi` to regenerate.
+
+### Canonical sources (non-generated)
+
+These directories are **never** generated and are **never** inside a generated
+root.  They are the single source of truth for all content.
+
+| Skill | Canonical source |
+|-------|------------------|
+| `atlassian` | SKILL.md: `skills/atlassian/_source/<agent>/SKILL.md`; scripts: `skills/atlassian/shared/scripts/` |
+| `web-automation` | SKILL.md: `skills/web-automation/_source/<agent>/SKILL.md`; scripts: `skills/web-automation/shared/` |
+| `create-plan` | `skills/create-plan/_source/<agent>/` (SKILL.md + templates) |
+| `do-task` | `skills/do-task/_source/<agent>/` (SKILL.md + templates) |
+| `implement-plan` | `skills/implement-plan/_source/<agent>/SKILL.md` |
+| `reviewer-runtime` (base) | `skills/reviewer-runtime/run-review.sh`, `notify-telegram.sh` |
+
+The `_source/` directories and `shared/` directories sit under the skill root
+**outside** every generated root.  Stale-file detection (`verify:generated`)
+never traverses them.
+
+### Generated roots (authoritative list)
+
+All paths are relative to the repo root.
+
+```text
+skills/atlassian/{claude-code,codex,cursor,opencode,pi}/
+skills/web-automation/{claude-code,codex,cursor,opencode,pi}/
+skills/create-plan/{claude-code,codex,cursor,opencode,pi}/
+skills/do-task/{claude-code,codex,cursor,opencode,pi}/
+skills/implement-plan/{claude-code,codex,cursor,opencode,pi}/
+skills/reviewer-runtime/pi/
+pi-package/skills/{atlassian,create-plan,do-task,implement-plan,web-automation}/
+```
+
+### Contributor workflow
+
+1. **Edit the canonical source** in the appropriate `_source/<agent>/`, `shared/`, or
+   base `skills/reviewer-runtime/` directory.
+2. **Run the generator:**
+
+   ```bash
+   pnpm run sync:pi        # regenerates all 31 generated roots
+   ```
+
+3. **Verify no drift:**
+
+   ```bash
+   pnpm run verify:generated   # must exit 0
+   ```
+
+4. **Stage both the canonical source AND the generated output** in the same commit.
+   Never commit a canonical change without regenerating.
+
+### File-type-aware header policy
+
+Every generated file (except JSON and `pnpm-lock.yaml`) receives a header
+identifying it as generated and pointing to the canonical source.  Headers are
+inserted so they do not break parsers or tools:
+
+| File type | Header form | Placement |
+|-----------|-------------|----------|
+| Markdown (with YAML front matter) | `<!-- ⚠️ GENERATED FILE ... -->` | After closing `---` |
+| Markdown (no front matter) | `<!-- ⚠️ GENERATED FILE ... -->` | Top of file |
+| Shell script | `# ⚠️ GENERATED FILE ...` | After `#!` shebang |
+| TypeScript / JavaScript | `// ⚠️ GENERATED FILE ...` | After `#!` shebang if present, else top |
+| YAML (non-lockfile) | `# ⚠️ GENERATED FILE ...` | Top of file |
+| JSON | *(no header — recorded in `.generated-manifest.json`)* | — |
+| `pnpm-lock.yaml` | *(no header — managed by pnpm)* | — |
+| JSONC | `// ⚠️ GENERATED FILE ...` | Top of file |
+
+### `.generated-manifest.json` contract
+
+Each generated root contains a `.generated-manifest.json` that:
+
+- Has a `$schema` marker and a `generator` field for identification.
+- Lists every **other** generator-owned path in that root (relative path,
+  file mode, SHA-256 hash, kind).
+- **Does NOT list itself** (non-self-referential).
+- Is scoped to exactly **one** generated root — there is no manifest at
+  `skills/<skill>/`.
+- Is verified structurally (not byte-for-byte) by `verify:generated`.
+
+### Package metadata (M3 change)
+
+Each generated agent-variant `package.json` carries:
+
+- A **unique private name** in the form `@ai-coding-skills/<skill>-<agent>` (e.g.
+  `@ai-coding-skills/atlassian-claude-code`).
+- `"private": true` to prevent accidental npm publication.
+
+The pi-package mirrors (`pi-package/skills/atlassian/scripts` and
+`pi-package/skills/web-automation/scripts`) use the `-mirror` agent suffix
+(`@ai-coding-skills/atlassian-pi-mirror`, `@ai-coding-skills/web-automation-pi-mirror`)
+to distinguish them from the `pi` agent variants in the workspace.
+
+This replaces the previous non-unique `name` fields (`atlassian-skill-scripts`,
+`web-automation-scripts`).  These packages are private and are never published.
+See `CHANGELOG.md` for the full rename list.
+
+## Adding a new agent variant
+
+To add support for a brand-new agent (e.g. `mynewagent`) across all skills:
+
+1. **Create canonical source files** for every skill:
+
+   ```bash
+   mkdir -p skills/<skill>/_source/mynewagent
+   # Add SKILL.md (copy and adapt from an existing agent variant)
+   ```
+
+2. **Update the generator** (`scripts/generate-skills.mjs`):
+   - Add `"mynewagent"` to the `AGENTS` array (or the per-skill agent list).
+   - Ensure any agent-specific substitution logic is handled.
+
+3. **Regenerate all variants:**
+
+   ```bash
+   pnpm run sync:pi
+   pnpm run verify:generated     # must exit 0
+   ```
+
+4. **Update the workspace** (`pnpm-workspace.yaml`):
+   - Add entries for every `skills/<skill>/mynewagent/scripts` package that
+     has a `package.json` (e.g. atlassian and web-automation).
+
+5. **Update documentation:**
+   - Add a row to the skills table in `README.md`.
+   - Add a row to the reviewer matrix in `docs/REVIEWERS.md` if the agent
+     exposes a reviewer CLI.
+   - Add an install guide at `docs/MYNEWAGENT.md` and link it from
+     `docs/README.md`.
+   - Add the doc file to the `files` list in root `package.json`.
+
+6. **Verify the full suite:**
+
+   ```bash
+   pnpm install
+   pnpm run check
+   ```
+
+## Adding a new skill
+
+To add an entirely new skill (e.g. `my-skill`):
+
+1. **Create the canonical source tree:**
+
+   ```bash
+   mkdir -p skills/my-skill/_source/{claude-code,codex,cursor,opencode,pi}
+   # Add SKILL.md to each _source/<agent>/ directory
+   ```
+
+2. **Update the generator** (`scripts/generate-skills.mjs`):
+   - Add `"my-skill"` to the `SKILLS` array.
+   - Provide a `getSkillMeta(skill)` entry with agents list, source resolver,
+     and any extra files to copy (templates, scripts, etc.).
+
+3. **Regenerate:**
+
+   ```bash
+   pnpm run sync:pi
+   pnpm run verify:generated
+   ```
+
+4. **Add the pi-package entry** if the skill should be distributed via the
+   Pi package:
+   - Update `package.json` → `pi.skills` with the new path.
+   - Update the `files` array to include the new skill directory.
+
+5. **Write documentation** in `docs/MY-SKILL.md` and link from `docs/README.md`
+   and the skills table in `README.md`.
+
+6. **Run the full suite:** `pnpm run check`.
+
+## CI
+
+Two GitHub Actions workflows live in `.github/workflows/`:
+
+| File | Trigger | Purpose |
+|------|---------|---------|
+| `check.yml` | push, pull_request (all branches) | Full `pnpm run check` on `ubuntu-latest` **and** `macos-latest`. Offline link-checking (no network dependency). |
+| `check-online.yml` | weekly schedule (Mon 09:00 UTC) + `workflow_dispatch` | `pnpm run verify:docs:online` with full external link checking. |
+
+### What the `check` workflow does
+
+1. Checks out the repository.
+2. Installs `shellcheck` via `apt-get` (Ubuntu) or `brew` (macOS).
+3. Installs `ripgrep` via `apt-get` (Ubuntu only; pre-installed on macOS runners).
+4. Installs Node.js 22 via `actions/setup-node`.
+5. Installs pnpm via `pnpm/action-setup@v4` — **no `version:` key is set**; the action reads the version from
+   `package.json#packageManager` (currently `pnpm@10.18.1+sha512…`), which is the single source of truth.
+6. Runs `pnpm install --frozen-lockfile`.
+7. Runs `pnpm run check` (the same command contributors run locally).
+
+The matrix runs both `ubuntu-latest` and `macos-latest` to guard against
+platform-specific regressions. Because M2 made all shell scripts portable
+across BSD and GNU coreutils, both runners should stay green.
+
+### pnpm version pinning
+
+The pnpm version is pinned **exclusively** in `package.json#packageManager`:
+
+```json
+"packageManager": "pnpm@10.18.1+sha512.77a884a..."
+```
+
+This field carries an exact version *and* an integrity hash, giving stronger
+reproducibility than a floating major like `version: "10"`.  The
+`pnpm/action-setup@v4` step in `check.yml` reads this field automatically;
+do **not** add a `with.version` key to that step.
+
+`pnpm run verify:ci` (backed by `scripts/lib/assert-no-pnpm-version-pin.mjs`)
+greps every `.github/workflows/*.yml` for `pnpm/action-setup` blocks that
+carry a `version:` key and fails if any are found.  This prevents
+reintroduction of the conflict that caused `pnpm/action-setup@v4` to error.
+
+### Adding new prerequisites to CI
+
+If a new tool is required (e.g. a new binary called by a verify script),
+add the install step to **both** OS branches in `check.yml`. Document the
+new prerequisite in the Prerequisites table at the top of this file.
+
+## Links
+
+- [CLEANUP-BASELINE.md](./CLEANUP-BASELINE.md) — as-is quality baseline
+- [README.md](../README.md) — project overview
+- [docs/README.md](./README.md) — documentation index
+- [CHANGELOG.md](../CHANGELOG.md) — milestone-by-milestone change record
+- [.github/workflows/check.yml](../.github/workflows/check.yml) — CI workflow
+- [.github/workflows/check-online.yml](../.github/workflows/check-online.yml) — weekly online link check