Docs: align contributor guidance with the Rust workspace (#732)

Point runtime contributions and rule changes at crates/, document testing a rebuilt local engine, and correct the published-engine CI gate guidance. No runtime or generated provider output changes.

AI assistance: prepared with Codex under maintainer pbakaus direction.
This commit is contained in:
Paul Bakaus
2026-09-04 14:53:52 -07:00
committed by GitHub
parent b077f6f0e4
commit 46ffe5caa2
2 changed files with 19 additions and 10 deletions
+15 -6
View File
@@ -2,11 +2,12 @@
## Project Structure & Module Organization
`skill/` is the source of truth for the Impeccable skill: `SKILL.src.md`, `reference/`, `scripts/`, and `agents/`. `skill/scripts/` holds the launcher (`impeccable`, `impeccable.cmd`), the pinned engine `VERSION`, `command-metadata.json`, and the in-page live-mode JS; every skill verb (`{{scripts_path}}/impeccable <verb>`) runs in the engine binary, which is built in a separate repo and pinned by the root `ENGINE_VERSION` file. Build logic lives in `scripts/`, with provider configs in `scripts/lib/transformers/`. `cli/` is the npm shim that runs the same binary, the browser extension lives in `extension/`, and regression coverage in `tests/` with fixtures under `tests/fixtures/` and the behavior goldens under `tests/oracle/`. `dist/` and `build/` are generated and gitignored. The root harness folders (`.agents/`, `.claude/`, `.cursor/`, etc.) and `plugin/` are generated distribution artifacts that are tracked for direct repo installs, not hand-authored source.
`skill/` is the source of truth for the Impeccable skill: `SKILL.src.md`, `reference/`, `scripts/`, and `agents/`. `skill/scripts/` holds the launcher (`impeccable`, `impeccable.cmd`), the pinned engine `VERSION`, `command-metadata.json`, and the in-page live-mode JS. Every skill verb (`{{scripts_path}}/impeccable <verb>`) runs in the engine binary, built from this repo's Cargo workspace under `crates/`; the root `ENGINE_VERSION` pins the released binary used by installs. Read `docs/ENGINE.md` before changing runtime code. Build logic lives in `scripts/`, with provider configs in `scripts/lib/transformers/`. `cli/` is the npm shim that runs the same binary, the browser extension lives in `extension/`, and regression coverage lives in the Rust crates and `tests/`, including fixtures under `tests/fixtures/` and behavior goldens under `tests/oracle/`. The website and service live in the separate private `impeccable-site` repo. `dist/` and `build/` are generated and gitignored. The root harness folders (`.agents/`, `.claude/`, `.cursor/`, etc.) and `plugin/` are generated distribution artifacts that are tracked for direct repo installs, not hand-authored source.
## Build, Test, and Development Commands
- `bun run dev` - start the local Bun server.
- `cargo build --release -p impeccable` - build this checkout's runtime into `target/release/impeccable`.
- `cargo test --workspace` - run the Rust workspace tests.
- `bun run build` - source-first build: regenerate `dist/`, derived site assets, and validation output without syncing tracked harness folders.
- `bun run build:release` - release/distribution build: run the full build and sync tracked root harness folders plus `plugin/`.
- `bun run rebuild` - clean and rebuild everything from scratch without syncing tracked harness folders.
@@ -25,7 +26,7 @@ Run `bun run build` after changing anything in `skill/`, transformer code, or us
The root harness folders (`.agents/skills/`, `.claude/skills/`, `.cursor/skills/`, `.gemini/skills/`, `.github/skills/`, `.grok/skills/`, `.hermes/skills/`, `.kiro/skills/`, `.opencode/skills/`, `.pi/skills/`, `.qoder/skills/`, `.rovodev/skills/`, `.trae*/skills/`, `.vibe/skills/`) and `plugin/` stay tracked so `main` remains installable for direct GitHub, `npx skills`, and submodule users. They are still generated artifacts.
Normal development should be source-first: stage changes in `skill/`, `scripts/`, `cli/`, `site/`, `extension/`, `functions/`, and `tests/`; leave generated harness churn unstaged unless the user asked for it. After source changes land on `main`, `.github/workflows/sync-generated-output.yml` runs `bun run build:release` and commits generated provider output directly back to `main`. Treat generated harness diffs as release artifacts and keep them out of feature PRs unless they are the point of the PR.
Normal development should be source-first: stage changes in `crates/`, `browser-bundle/`, `skill/`, `scripts/`, `cli/`, `extension/`, and `tests/`; leave generated harness churn unstaged unless the user asked for it. After source changes land on `main`, `.github/workflows/sync-generated-output.yml` runs `bun run build:release` and commits generated provider output directly back to `main`. Treat generated harness diffs as release artifacts and keep them out of feature PRs unless they are the point of the PR. The two tracked engine assets under `crates/live/assets/` follow the rule-change workflow below instead.
## Sandbox gotchas for Codex agents
@@ -39,9 +40,13 @@ Some repo workflows need to run outside the sandbox in the desktop app:
Use ESM, semicolons, and the existing two-space indentation style in JS, HTML, and CSS. Prefer small, single-purpose modules over large abstractions. Keep filenames descriptive and lowercase with hyphens where needed; skill entrypoints stay as `SKILL.md`, build and test helpers use `.js` or `.mjs`. In source frontmatter, use clear kebab-case names and concise descriptions. There is no dedicated formatter or linter configured here, so match surrounding code closely.
For Rust, follow the surrounding crate's conventions and workspace formatting configuration. Keep changes scoped; do not reformat unrelated modules.
## Testing Guidelines
Tests use Buns test runner plus Nodes built-in `--test`. Name tests `*.test.js` or `*.test.mjs` and place new fixtures near the behavior they cover, usually under `tests/fixtures/`. Prefer targeted test runs while iterating, then finish with `bun run test`. If you change generated outputs or provider transforms, verify both source parsing and at least one affected provider path in `dist/`.
Tests use Bun's test runner plus Node's built-in `--test`. Name tests `*.test.js` or `*.test.mjs` and place new fixtures near the behavior they cover, usually under `tests/fixtures/`. Prefer targeted test runs while iterating, then finish with `bun run test`. If you change generated outputs or provider transforms, verify both source parsing and at least one affected provider path in `dist/`.
For runtime changes under `crates/`, add a failing regression in the affected crate, run its focused tests, then `cargo test --workspace`. Rebuild with `cargo build --release -p impeccable` and run `IMPECCABLE_BIN="$PWD/target/release/impeccable" bun run test` so the oracle exercises the changed source, not an older downloaded release. Review intended oracle changes by hand; never overwrite goldens just to make a regression pass. `tests/oracle/vectors/calls/` contains frozen function-level vectors and must not be regenerated.
For changes to the live-mode page JS (`skill/scripts/live-browser*.js`) or an `ENGINE_VERSION` bump, also run `bun run test:live-e2e` (kept out of the default suite because it does real `npm install` per fixture and boots framework dev servers). Scope to one fixture with `IMPECCABLE_E2E_ONLY=<fixture-name>` while iterating; pass `IMPECCABLE_E2E_DEBUG=1` for page-DOM and dev-server-log dumps on failure. Schema and authoring guide for new fixtures live in `tests/framework-fixtures/README.md`.
@@ -53,7 +58,11 @@ Other area-to-suite obligations (the canonical mapping is the `triggers` lists i
## Anti-pattern detection rules
The rule engine lives in the engine repo, not here. What this repo owns is the behavior contract: `docs/CLI-CONTRACT.md` describes every verb, `tests/oracle/` holds the recorded goldens and replays them against the binary (`tests/oracle.test.mjs`), and `tests/fixtures/antipatterns/*.html` are the fixtures those goldens scan. A rule change lands in the engine, then here as a new oracle case (`node tests/oracle/record.mjs --bin <prefix>`, golden reviewed by hand) and, when it introduces new design guidance, an edit to `skill/SKILL.src.md` or `skill/reference/*.md`. Rule counts quoted in `README.md` and `README.npm.md` are checked by the build against `crates/live/assets/antipatterns.json`, the tracked registry `cargo xtask bundle` writes (it falls back to the gitignored `extension/detector/antipatterns.json`).
The rule engine lives in this workspace. `crates/core` holds the checks and browser adapters; `crates/foundation` holds the registry and shared types. `crates/html`, `crates/browser`, and `crates/detect` provide the static HTML, URL, and CLI/text paths. `crates/wasm` compiles the shared rules for the extension, live overlay, and site. See `docs/ENGINE.md` for the crate map and bundle flow, and `docs/CLI-CONTRACT.md` for observable behavior.
Add a fixture first under `tests/fixtures/antipatterns/` with should-flag and should-pass columns, at least four flag cases and five false-positive shapes, unique headings, and explicit pixel dimensions. Add failing Rust coverage before implementing the rule. Cover each affected engine path and add or update an oracle case (`node tests/oracle/record.mjs --bin <prefix>`, golden reviewed by hand). When a rule introduces design guidance, update `skill/SKILL.src.md` or `skill/reference/*.md` too.
Run `cargo xtask bundle` after rule or browser-bundle changes and commit its two tracked outputs: `crates/live/assets/detect-antipatterns-browser.js` and `crates/live/assets/antipatterns.json`. The generated `extension/detector/` remains gitignored. Rebuild the native binary after bundling, run the Rust and Bun/Node checks above, and run `bun run build` to validate distribution and rule counts. Verify browser-facing changes on the relevant live fixture; native and browser adapters can disagree.
## Commit & Pull Request Guidelines
@@ -77,4 +86,4 @@ Tags are per-component because the three components ship independently: `skill-v
## Contributor Notes
Do not edit generated provider files directly unless you are intentionally patching generated output as part of a build-system change. Prefer fixing the root source in `skill/`, `scripts/`, or `cli/` (or the engine repo for verb behavior), then regenerate artifacts for validation. Stage generated harness artifacts only for release/main-sync or build-system work.
Do not edit generated provider files directly unless you are intentionally patching generated output as part of a build-system change. Prefer fixing the root source in `skill/`, `scripts/`, or `cli/`, and `crates/` for runtime behavior, then regenerate artifacts for validation. Stage generated harness artifacts only for release/main-sync or build-system work.
+4 -4
View File
@@ -160,7 +160,7 @@ bun run test:plugin-e2e # Just the plugin loader E2E (also part of the def
bun run test:cleanup # Kill live servers a previous run of THIS checkout left behind
```
Unit tests (build orchestration, transformers, validators) run via `bun test`. Everything that spawns the engine binary (`tests/oracle.test.mjs`, `tests/framework-fixtures.test.mjs`) runs via `node --test`; both skip cleanly when no binary is found (`bun run fetch:engine` or `IMPECCABLE_BIN`). The `test` script handles this split automatically. Verb behavior is not unit-tested here at all: the oracle goldens and the engine repo's own tests own it.
Unit tests (build orchestration, transformers, validators) run via `bun test`. Everything that spawns the engine binary (`tests/oracle.test.mjs`, `tests/framework-fixtures.test.mjs`) runs via `node --test`; both skip cleanly when no binary is found (`bun run fetch:engine` or `IMPECCABLE_BIN`). The `test` script handles this split automatically. Runtime unit and integration tests live under `crates/` and run with `cargo test --workspace`; the oracle goldens pin observable verb behavior across the same workspace.
### Live servers must not outlive their test process
@@ -190,7 +190,7 @@ The default suite does not cover everything. When a change touches one of these
| `ENGINE_VERSION` bump | `bun run test:new-work-e2e` | Playwright, offline, no API cost |
| `plugin/`, `skill/agents/`, `scripts/build.js`, plugin manifest validator | `bun run test:plugin-e2e` | ~1 s; already in the default suite, needs the `claude` CLI |
Verb-level behavior changes happen in the engine repo; the check they owe here is `bun run test` with a binary present (the oracle), and a new oracle case when the contract grows.
For verb-level behavior changes in `crates/`, run focused crate tests and `cargo test --workspace`, then `cargo build --release -p impeccable`. Run `IMPECCABLE_BIN="$PWD/target/release/impeccable" bun run test` to exercise the changed source rather than an older downloaded release. Add a new oracle case when the contract grows and review golden changes by hand. See `docs/ENGINE.md` for browser-bundle checks and generated assets.
**Plugin loader E2E** (`tests/plugin-e2e.test.mjs`, in the default suite): installs the committed `./plugin` subtree into a real Claude Code, sandboxed via `CLAUDE_CONFIG_DIR` in a temp dir, and asserts the component inventory from `claude plugin details`: the skill parses, every `plugin/agents/*.md` is visible, hooks are discovered. This is the only check that catches loader-contract surprises the unit guards can't know about (PR #494 shipped an `agents` manifest key that silently loaded zero agents; `claude plugin validate` never flags plugin-manifest problems). Runs in about a second; skips cleanly when the `claude` CLI is not on PATH. The known contract itself (allowed manifest keys, no `agents` key, trailing-slash `skills` path, source agents shipped) is pinned deterministically by `scripts/lib/validate-plugin-manifest.js`, unit-tested in `tests/validate-plugin-manifest.test.js` and enforced as a `bun run build` gate. Never add a key to the generated plugin manifest without verifying it against a real install and extending `KNOWN_LOADER_KEYS`.
@@ -255,7 +255,7 @@ npx impeccable install # install skills
npx impeccable --help # show help
```
The package no longer exports a JS detector API (`main` / `exports` are gone); the in-page bundle for the extension and site comes from the engine repo.
The package no longer exports a JS detector API (`main` / `exports` are gone); the in-page bundle for the extension and site is built from this workspace by `cargo xtask bundle`.
## Versioning
@@ -313,7 +313,7 @@ The skill launcher, the npm shim (`cli/bin/cli.js`), and `impeccable install` al
2. Publish the five `@impeccable/cli-<os>-<arch>@<ENGINE_VERSION>` npm platform packages.
3. Only then tag/publish the skill or CLI release, and only then merge a branch that bumps `ENGINE_VERSION` (the `sync-generated-output.yml` workflow rewrites provider dirs on merge to `main`).
`scripts/check-engine-release.mjs` verifies step 1 and 2 for the pinned version (ranged-GET each release asset, registry-probe each npm package; honors `IMPECCABLE_DOWNLOAD_BASE`). It exits non-zero and names exactly which assets are missing. `scripts/release.mjs` runs it as a hard gate before tagging the **skill** and **CLI** components and refuses to proceed when any asset is absent; the **extension** release is exempt because it ships a vendored WASM detector and never execs the engine. `IMPECCABLE_SKIP_ENGINE_CHECK=1` bypasses the gate only for the case where the assets exist but the registry probe is unreachable. CI's `engine-release-ready` job runs the same script; it is `continue-on-error: true` with a loud `::warning` until the first engine release is published, at which point flip it to `false` so a mis-ordered merge fails CI.
`scripts/check-engine-release.mjs` verifies step 1 and 2 for the pinned version (ranged-GET each release asset, registry-probe each npm package; honors `IMPECCABLE_DOWNLOAD_BASE`). It exits non-zero and names exactly which assets are missing. `scripts/release.mjs` runs it as a hard gate before tagging the **skill** and **CLI** components and refuses to proceed when any asset is absent; the **extension** release is exempt because it ships a vendored WASM detector and never execs the engine. `IMPECCABLE_SKIP_ENGINE_CHECK=1` bypasses the gate only for the case where the assets exist but the registry probe is unreachable. CI's `engine-release-ready` job runs the same script as a hard gate, so missing release assets fail CI.
## Adding New Commands