diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 0000000..f5af1e4 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,85 @@ +# Copilot Instructions + +## Commit Conventions + +All commits in this repository are validated by `@savvy-web/commitlint` (Silk preset). + +### Format + +```text +type(scope): subject + +Optional body — plain prose only, max 300 chars per line. + +Signed-off-by: Name +``` + +### Allowed Types + +| Type | When to use | +| :--- | :---------- | +| `feat` | New feature or capability | +| `fix` | Bug fix | +| `docs` | Documentation-only change | +| `style` | Formatting, whitespace — no logic change | +| `refactor` | Code restructuring without behaviour change | +| `perf` | Performance improvement | +| `test` | Adding or updating tests | +| `build` | Build system or dependency changes | +| `ci` | CI/CD workflow changes | +| `chore` | Maintenance tasks (version bumps, configs) | +| `revert` | Reverts a previous commit | +| `release` | Release preparation commit | +| `ai` | AI-generated or AI-assisted change | + +### Rules + +- **Subject line**: imperative mood, no trailing period, max ~72 chars +- **Body**: optional; plain prose only — no Markdown headers (`##`), no fenced + code blocks (` ``` `), no bare URLs, no numbered lists +- **Body line length**: max 300 characters per line +- **DCO sign-off**: required on every commit — a `Signed-off-by:` trailer must + be present because this repository has a `DCO` file and enforces the check in + CI via `dco.yml` + +### DCO Sign-Off + +The Developer Certificate of Origin (DCO) certifies that you have the right to +submit your contribution. Add the trailer automatically with: + +```bash +git commit -s -m "feat(biome): bump default version to 2.5.0" +``` + +Or configure git to add it by default: + +```bash +git config --global trailer.sign.key "Signed-off-by" +git config --global trailer.sign.ifmissing add +git config --global trailer.sign.ifexists doNothing +git config --global trailer.sign.command 'echo "$(git config user.name) <$(git config user.email)>"' +``` + +### Examples + +```text +feat(biome): bump default version to 2.5.0 + +Signed-off-by: C. Spencer Beggs +``` + +```text +fix(package-manager): handle missing packageManager field in package.json + +When no packageManager field is present and the option is set to 'auto', +the install script now exits cleanly instead of forwarding an empty spec +to corepack. + +Signed-off-by: C. Spencer Beggs +``` + +```text +ci: switch feature tests to devcontainer features test CLI + +Signed-off-by: C. Spencer Beggs +``` diff --git a/.github/instructions/FEATURE.instructions.md b/.github/instructions/FEATURE.instructions.md index a0f05b7..dde5e3a 100644 --- a/.github/instructions/FEATURE.instructions.md +++ b/.github/instructions/FEATURE.instructions.md @@ -36,14 +36,15 @@ affected files together in the same commit: 1. `src//devcontainer-feature.json` — `"version"` field and option `"default"` value 2. `test//test.sh` — `grep ""` assertions -3. `test//scenarios.json` — step descriptions mentioning the version -4. `docs/features/.md` — usage snippet and options default listing +3. `test//scenarios.json` — scenario option values mentioning the version +4. `test//.sh` — `grep ""` assertions in scenario scripts +5. `docs/features/.md` — usage snippet and options default listing Use the `bump-feature` skill for guided step-by-step assistance. ## Five-File Completeness Rule -Every feature must have exactly five files. Before committing, verify all are +Every feature must have at least these five files. Before committing, verify all are present: ```text @@ -54,8 +55,16 @@ test//scenarios.json docs/features/.md ``` +If `scenarios.json` is non-empty (contains scenario keys), each key also +requires a matching `test//.sh` assertion script. + Run `pnpm run validate-feature ` to check completeness. +**Note on executable bits:** scripts are stored as `100644` (non-executable) +in git. The Husky `post-checkout`/`post-merge` hooks set the bits locally and +CI workflows set them before running. Do **not** run `chmod +x` or +`git update-index --chmod=+x` — the bits are intentionally absent in git. + ## `documentationURL` Must Match Actual Doc File The `documentationURL` field in `devcontainer-feature.json` must always point diff --git a/.github/instructions/SHELL.instructions.md b/.github/instructions/SHELL.instructions.md index 6b5dde9..3f5f01e 100644 --- a/.github/instructions/SHELL.instructions.md +++ b/.github/instructions/SHELL.instructions.md @@ -94,6 +94,20 @@ fi For tools installed to a fixed path where overwriting is safe, no guard is needed. +## Executable Bits + +Shell scripts are stored in git **without** the executable bit. Do **not** +run `chmod +x` on any `.sh` file, and do **not** use +`git update-index --chmod=+x`. + +Executable bits are managed outside of git: + +- **Local dev** — Husky `post-checkout`/`post-merge` hooks apply + `chmod +x` on all tracked `*.sh` files automatically. +- **CI** — the test and publish workflows set the bits before invoking scripts. + +Agents must never treat a missing executable bit as a bug to fix. + ## Linting After editing a shell script, run shellcheck if it is available: diff --git a/.github/scripts/discover-features.js b/.github/scripts/discover-features.js new file mode 100644 index 0000000..60627a3 --- /dev/null +++ b/.github/scripts/discover-features.js @@ -0,0 +1,31 @@ +#!/usr/bin/env node +// Outputs a JSON matrix of { id, skipAutogenerated } entries for every +// feature that has both a devcontainer-feature.json and a test/test.sh file. +// Used by the "Discover features" step in test.yml. +import { existsSync, readFileSync, readdirSync } from "node:fs"; +import { join, resolve } from "node:path"; + +const srcRoot = resolve("src"); +const testRoot = resolve("test"); + +if (!existsSync(srcRoot)) { + process.stdout.write("[]"); + process.exit(0); +} + +const entries = readdirSync(srcRoot) + .filter((f) => { + const featureJson = join(srcRoot, f, "devcontainer-feature.json"); + const testSh = join(testRoot, f, "test.sh"); + return existsSync(featureJson) && existsSync(testSh); + }) + .map((id) => { + const featureJson = JSON.parse(readFileSync(join(srcRoot, id, "devcontainer-feature.json"), "utf8")); + const installsAfter = featureJson.installsAfter || []; + // Skip auto-generated test for features that depend on another local + // savvy-web feature (e.g. package-manager requires node). + const skipAutogenerated = installsAfter.some((dep) => dep.includes("ghcr.io/savvy-web/")); + return { id, skipAutogenerated }; + }); + +process.stdout.write(JSON.stringify(entries)); diff --git a/.github/scripts/parse-test-results.js b/.github/scripts/parse-test-results.js new file mode 100644 index 0000000..48ab7d3 --- /dev/null +++ b/.github/scripts/parse-test-results.js @@ -0,0 +1,41 @@ +#!/usr/bin/env node +// Parses the TEST REPORT section emitted by `devcontainer features test` and +// writes a structured JSON results file. +// +// Usage: +// node parse-test-results.js [logPath] [outPath] +// +// Defaults: +// logPath /tmp/test.log +// outPath /tmp/test-results.json +// +// Output shape: +// { passed: string[], failed: string[], total: number, allPassed: boolean } +import { readFileSync, writeFileSync } from "node:fs"; + +const logPath = process.argv[2] ?? "/tmp/test.log"; +const outPath = process.argv[3] ?? "/tmp/test-results.json"; + +let log = ""; +try { + log = readFileSync(logPath, "utf8"); +} catch { + // log file not yet written; treat as empty +} + +const idx = log.indexOf("TEST REPORT"); +// If the CLI crashed or changed output format, treat as a parse failure +// rather than silently succeeding with an empty result set. +const reportFound = idx >= 0; +const section = reportFound ? log.slice(idx) : ""; +const passed = [...section.matchAll(/Passed:\s+'([^']+)'/g)].map((m) => m[1]); +const failed = [...section.matchAll(/Failed:\s+'([^']+)'/g)].map((m) => m[1]); + +const result = { + passed, + failed, + total: passed.length + failed.length, + allPassed: reportFound && failed.length === 0, +}; + +writeFileSync(outPath, JSON.stringify(result, null, 2)); diff --git a/.github/skills/debug-feature/SKILL.md b/.github/skills/debug-feature/SKILL.md index 4b3bcc0..ddff531 100644 --- a/.github/skills/debug-feature/SKILL.md +++ b/.github/skills/debug-feature/SKILL.md @@ -17,7 +17,7 @@ Systematic triage guide for diagnosing failing `install.sh` scripts, Use this skill when: -- `install.sh` exits with a non-zero code during `pnpm run test:feature` +- `install.sh` exits with a non-zero code during `pnpm run feature:test` - A `test.sh` assertion fails (`[FAIL]` in output or non-zero exit) - The publish workflow's `test` job fails - A feature installs successfully locally but fails in CI @@ -55,12 +55,11 @@ The most common failure categories, in order of frequency: 1. **Env var not injected** — option value is empty or unset 2. **Architecture mismatch** — wrong binary for the platform 3. **Broken download URL** — upstream URL pattern changed or version not found -4. **Missing execute permission** — `install.sh` is not executable -5. **Missing `set -euo pipefail`** — silent failures masked by `|| true` -6. **Idempotency failure** — re-running the script breaks an existing install -7. **Version assertion mismatch** — `test.sh` checks a different version than +4. **Missing `set -euo pipefail`** — silent failures masked by `|| true` +5. **Idempotency failure** — re-running the script breaks an existing install +6. **Version assertion mismatch** — `test.sh` checks a different version than the default in `devcontainer-feature.json` -8. **Missing `installsAfter`** — the feature depends on another feature +7. **Missing `installsAfter`** — the feature depends on another feature (e.g. Node.js) being installed first but does not declare it ### Step 3 — Apply the fix @@ -75,7 +74,7 @@ After applying a fix: 1. Run the feature locally with act: ```bash - pnpm run test:feature + pnpm run feature:test ``` 2. Run the validation script to catch any remaining structural issues: @@ -119,7 +118,7 @@ If the error is not immediately obvious, work through these questions: - Does the script end with `command -v ` or ` --version`? - If the binary is installed to a non-standard path, is that path on `$PATH`? -- Is the binary installed with execute permission (`chmod +x`)? +- Is the binary installed with execute permission (`chmod +x` on the installed binary in `/usr/local/bin`, not the `install.sh` source file)? **Idempotency:** diff --git a/.github/skills/debug-feature/references/common-failure-modes.md b/.github/skills/debug-feature/references/common-failure-modes.md index 3d96486..3c4b017 100644 --- a/.github/skills/debug-feature/references/common-failure-modes.md +++ b/.github/skills/debug-feature/references/common-failure-modes.md @@ -149,27 +149,36 @@ Always include an `else` branch that errors on unsupported architectures. --- -## 4. Missing Execute Permission +## 4. Execute Permission on Installed Binaries ### Detection -- Error: `"Permission denied"` when running `install.sh` -- `ls -la src//install.sh` shows `-rw-r--r--` (no `x` bit) +- Error: `"Permission denied"` when running a binary the feature installed + (e.g. `/usr/local/bin/biome`) +- `ls -la /usr/local/bin/` shows `-rw-r--r--` (no `x` bit) ### Why it happens -Files created with a text editor or copied without preserving permissions may -not have the execute bit set. The devcontainer CLI and CI workflows run -`install.sh` directly. +The feature's `install.sh` downloaded or copied a binary but did not mark it +executable with `chmod +x`. ### Fix +Add `chmod +x` or use `install -m 0755` in `install.sh` when placing the +binary: + ```bash -chmod +x src//install.sh +chmod +x "$BINARY_PATH" +# or +install -m 0755 "$TMP/binary" /usr/local/bin/binary ``` -The `validate-feature` script (`pnpm run validate-feature`) checks for this -and reports it as an error. +**Note:** this is about the *installed binary*, not `install.sh` itself. +`install.sh` and all other `*.sh` files in the repository are stored **without** +the executable bit in git (`100644`). The Husky `post-checkout`/`post-merge` +hooks set the bit locally; CI workflows set it before running the scripts. +Never run `chmod +x src//install.sh` or +`git update-index --chmod=+x src//install.sh`. --- @@ -318,7 +327,7 @@ Then bump the feature version using the `bump-feature` skill, since ### Detection -- `pnpm run test:feature` exits with `"act is not installed"` +- `pnpm run feature:test` exits with `"act is not installed"` - `docker: command not found` ### Fix diff --git a/.github/skills/devcontainer/SKILL.md b/.github/skills/devcontainer/SKILL.md index 0d01808..5ea6fa9 100644 --- a/.github/skills/devcontainer/SKILL.md +++ b/.github/skills/devcontainer/SKILL.md @@ -126,6 +126,10 @@ docs/ - End with a validation step: `command -v ` or ` --version` - Print `[ERROR]` to stderr and `exit 1` on unrecoverable failures - Print `[INFO]` for progress steps +- Use `_REMOTE_USER` (not `_CONTAINER_USER`) to transfer directory ownership to + the devcontainer user after installing to user-writable paths. Always guard + for the unset and root cases — see `references/feature-anatomy.md` §User + Environment Variables for the canonical pattern ### Feature Bootstrap Checklist @@ -173,8 +177,12 @@ looking for `test//test.sh` matching the feature's directory name. - **Env var not set** — the devcontainer CLI injects option values as env vars but they may be empty strings, not unset. Always use `VAR="${OPTION_VAR:-default}"` to handle both the unset and empty-string case. -- **Missing execute permission** — `install.sh` must be executable - (`chmod +x`). The publish workflow runs it directly. +- **Executable bits not committed** — shell scripts are stored in git without + the executable bit (`100644`). Do **not** run `chmod +x` or + `git update-index --chmod=+x`. Husky hooks set the bits on checkout/merge + locally; CI workflows set them before running scripts. The + `validate-feature` script may note missing bits — this is expected and safe + to ignore. - **Semver ranges** — option defaults like `">=20"` or `"^24"` are not reproducible. Always use an exact version string. - **Missing `documentationURL`** — all features must link to their doc page. diff --git a/.github/skills/devcontainer/references/feature-anatomy.md b/.github/skills/devcontainer/references/feature-anatomy.md index 806378b..360746c 100644 --- a/.github/skills/devcontainer/references/feature-anatomy.md +++ b/.github/skills/devcontainer/references/feature-anatomy.md @@ -181,6 +181,39 @@ biome --version Errors go to stderr (`>&2`). Info and success go to stdout. +### User Environment Variables + +Feature scripts run as root. The devcontainer CLI injects these variables so +scripts can transfer ownership or run commands as the actual container user: + +| Variable | Value | +| :------- | :---- | +| `_REMOTE_USER` | The `remoteUser` setting in `devcontainer.json`. If `remoteUser` is not set, this equals `_CONTAINER_USER`. This is the user VS Code / Codespaces will run as. | +| `_CONTAINER_USER` | The container's user (set via `USER` in the Dockerfile, `user:` in `docker-compose.yml`, or `containerUser` in `devcontainer.json`). | +| `_REMOTE_USER_HOME` | Home directory of `_REMOTE_USER`. | +| `_CONTAINER_USER_HOME` | Home directory of `_CONTAINER_USER`. | + +**Use `_REMOTE_USER`** whenever you need to transfer ownership of a directory +so the devcontainer user can write to it (e.g. `RUSTUP_HOME`, `CARGO_HOME`). +Always guard for the unset and root cases: + +```bash +REMOTE_USER="${_REMOTE_USER:-}" +if [[ -n "$REMOTE_USER" && "$REMOTE_USER" != "root" ]] && id -u "$REMOTE_USER" &>/dev/null; then + chown -R "$REMOTE_USER" /some/dir + echo "[INFO] Transferred ownership to $REMOTE_USER." +fi +``` + +This guard handles three edge cases: + +- `_REMOTE_USER` is unset (container doesn't use feature user env vars) +- `_REMOTE_USER` is `root` (no chown needed; root already owns everything) +- The user account doesn't exist yet at install time (defensive `id -u` check) + +For tools that install to system-wide paths owned by root (e.g. +`/usr/local/bin/biome`), no chown is needed — the binary is world-executable. + ### Idempotency Guard When re-running the installer would break an existing installation, guard it: diff --git a/.github/skills/devcontainer/references/test-patterns.md b/.github/skills/devcontainer/references/test-patterns.md index 72d5c87..08a7069 100644 --- a/.github/skills/devcontainer/references/test-patterns.md +++ b/.github/skills/devcontainer/references/test-patterns.md @@ -1,7 +1,10 @@ # Test Patterns -Reference for the two-file test convention used in this repository: -`test.sh` and `scenarios.json`. +Reference for the three-file test convention used in this repository, +following the official `devcontainer features test` CLI pattern: +`test.sh`, `scenarios.json`, and per-scenario `.sh` scripts. + +See also: [devcontainer features test CLI docs](https://github.com/devcontainers/cli/blob/main/docs/features/test.md) ## File Locations @@ -11,190 +14,205 @@ directory structure: ```text test/ / - test.sh # executable shell assertions - scenarios.json # human-readable scenario descriptions + test.sh # auto-generated default test (default options) + scenarios.json # map of scenario names → devcontainer.json fragments + .sh # one assertion script per entry in scenarios.json ``` -Both files are required. The CI publish workflow discovers tests by resolving -`test//test.sh` from the feature id. - -## `scenarios.json` +`test.sh` and `scenarios.json` are always required (five-file rule). Each key +in `scenarios.json` requires a matching `.sh` assertion script. -An array of scenario objects. Each object describes one logical test scenario. +## `test.sh` — Default (Auto-Generated) Test -### Schema +The auto-generated test builds a container that installs the feature with +**default options** and then runs `test.sh` inside it. It must use +`dev-container-features-test-lib` for `check` and `reportResults`. -```json -[ - { - "name": "Human-readable scenario name", - "steps": [ - "Step one description", - "Step two description" - ] - } -] -``` +### Structure -### Rules +```bash +#!/usr/bin/env bash +# Brief comment describing what this test verifies +set -e -- At least one scenario per feature -- `name` — short, title-case, describes what the scenario verifies -- `steps` — ordered list of what the test checks; mirrors the assertions in - `test.sh` -- Not executed by CI — this file is documentation and a checklist for humans - reviewing the test +# Import test library provided by the devcontainer CLI +source dev-container-features-test-lib -### Example +check "binary is installed" --version +check "version matches default" bash -c " --version | grep ''" -```json -[ - { - "name": "Biome global install", - "steps": ["Install feature", "Check Biome version is 2.4.12"] - } -] +reportResults ``` -## `test.sh` +### Rules -An executable Bash script that runs assertions after the feature is installed -in the devcontainer. +- `#!/usr/bin/env bash` + `set -e` (not `set -euo pipefail` — the test lib + uses unset variables internally) +- `source dev-container-features-test-lib` before any `check` calls +- `check "