From 53ebc1e50463b7eab49e4fa1d446a91e32b9863e Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 12:33:37 +0200 Subject: [PATCH 1/5] chore: centralize coding agent setup --- .../CLAUDE.md => .agnostic-ai/AGNOSTIC_AI.md | 36 ++++++--- .agnostic-ai/agents/aragorn.md | 8 ++ .agnostic-ai/agents/gandalf.md | 10 +++ .agnostic-ai/agents/legolas.md | 10 +++ .../hooks/posttooluse-edit-write-if.yaml | 6 ++ .../overlays/claude.settings.hook-events.json | 6 ++ .agnostic-ai/overlays/claude.settings.json | 4 + .agnostic-ai/overlays/codex.config.toml | 2 + .../rules/architecture-map.md | 53 ++++++------- {.claude => .agnostic-ai}/rules/bash-style.md | 19 ++--- .../rules/perf-fork-budget.md | 57 +++++++------- .../rules/tdd-workflow.md | 11 +-- {.claude => .agnostic-ai}/rules/testing.md | 19 ++--- .agnostic-ai/settings/models.yaml | 1 + .../skills/add-assertion/SKILL.md | 12 +-- .../skills/check-coverage/SKILL.md | 8 +- .../skills/commit/SKILL.md | 78 ++++++++++++++----- .../skills/fix-test/SKILL.md | 12 +-- .../skills/gh-issue/SKILL.md | 26 ++++++- .../skills/gh-issues/SKILL.md | 22 +++--- {.claude => .agnostic-ai}/skills/pr/SKILL.md | 50 +++++++++--- .../skills/pre-release/SKILL.md | 12 +-- .../skills/release/SKILL.md | 34 +++++++- .../skills/review/SKILL.md | 30 ++++++- .../skills/tdd-cycle/SKILL.md | 22 +++--- .claude/settings.json | 17 ---- .editorconfig | 2 +- .github/CONTRIBUTING.md | 19 +++++ .github/workflows/check-agent-specs.yml | 42 ++++++++++ .gitignore | 23 ++++++ .worktreeinclude | 18 +++++ CHANGELOG.md | 1 + agnostic-ai.yaml | 19 +++++ 33 files changed, 501 insertions(+), 188 deletions(-) rename .claude/CLAUDE.md => .agnostic-ai/AGNOSTIC_AI.md (62%) create mode 100644 .agnostic-ai/agents/aragorn.md create mode 100644 .agnostic-ai/agents/gandalf.md create mode 100644 .agnostic-ai/agents/legolas.md create mode 100644 .agnostic-ai/hooks/posttooluse-edit-write-if.yaml create mode 100644 .agnostic-ai/overlays/claude.settings.hook-events.json create mode 100644 .agnostic-ai/overlays/claude.settings.json create mode 100644 .agnostic-ai/overlays/codex.config.toml rename {.claude => .agnostic-ai}/rules/architecture-map.md (84%) rename {.claude => .agnostic-ai}/rules/bash-style.md (91%) rename {.claude => .agnostic-ai}/rules/perf-fork-budget.md (89%) rename {.claude => .agnostic-ai}/rules/tdd-workflow.md (80%) rename {.claude => .agnostic-ai}/rules/testing.md (89%) create mode 100644 .agnostic-ai/settings/models.yaml rename {.claude => .agnostic-ai}/skills/add-assertion/SKILL.md (74%) rename {.claude => .agnostic-ai}/skills/check-coverage/SKILL.md (85%) rename {.claude => .agnostic-ai}/skills/commit/SKILL.md (52%) rename {.claude => .agnostic-ai}/skills/fix-test/SKILL.md (74%) rename {.claude => .agnostic-ai}/skills/gh-issue/SKILL.md (80%) rename {.claude => .agnostic-ai}/skills/gh-issues/SKILL.md (88%) rename {.claude => .agnostic-ai}/skills/pr/SKILL.md (69%) rename {.claude => .agnostic-ai}/skills/pre-release/SKILL.md (82%) rename {.claude => .agnostic-ai}/skills/release/SKILL.md (78%) rename {.claude => .agnostic-ai}/skills/review/SKILL.md (65%) rename {.claude => .agnostic-ai}/skills/tdd-cycle/SKILL.md (62%) delete mode 100644 .claude/settings.json create mode 100644 .github/workflows/check-agent-specs.yml create mode 100644 .worktreeinclude create mode 100644 agnostic-ai.yaml diff --git a/.claude/CLAUDE.md b/.agnostic-ai/AGNOSTIC_AI.md similarity index 62% rename from .claude/CLAUDE.md rename to .agnostic-ai/AGNOSTIC_AI.md index 14e65cd8b..f6e4d5b7f 100644 --- a/.claude/CLAUDE.md +++ b/.agnostic-ai/AGNOSTIC_AI.md @@ -9,29 +9,29 @@ ## Core Principles ### TDD by Default -**RED → GREEN → REFACTOR** — every change starts from a failing test. No exceptions. +**RED → GREEN → REFACTOR**, every change starts from a failing test. No exceptions. ### Bash 3.0+ Compatible Works on macOS default bash (3.2) and older. **Prohibited features:** - `printf -v` (Bash 3.1+) - `x+=y` string append assignment (Bash 3.1+) -- `arr+=(x)` array append (a Bash 3.0 **parse** error — it kills the file even in dead code) +- `arr+=(x)` array append (a Bash 3.0 **parse** error, it kills the file even in dead code) - `declare -A` (associative arrays - Bash 4.0+) -- `[[ ]]` (use `[ ]` instead — house style, not a version limit) +- `[[ ]]` (use `[ ]` instead, house style, not a version limit) - `[[ =~ ]]` (exists on 3.0, but 3.2 changed quoted-pattern semantics) - `${var,,}` (case conversion - Bash 4.0+) - `${array[-1]}` (negative indexing - Bash 4.3+) - `&>>` redirect (Bash 4.0+) -See `.claude/rules/bash-style.md` for complete compatibility guide (auto-loaded when editing `src/` or `tests/`). +See `.agnostic-ai/rules/bash-style.md` for complete compatibility guide (auto-loaded when editing `src/` or `tests/`). ### Quality Standards Every change must pass: ```bash make sa # ShellCheck static analysis -make lint # EditorConfig linting (the formatting authority — 2-space indent) +make lint # EditorConfig linting (the formatting authority, 2-space indent) ./bashunit tests/ # All tests passing ``` @@ -42,13 +42,13 @@ breaks `make lint`. See "Formatting" below. ## Formatting Match the surrounding 2-space style by hand and verify with `make lint`. There is -no `shfmt` target, and **`shfmt -w .` must not be run** — it fights `.editorconfig` +no `shfmt` target, and **`shfmt -w .` must not be run**, it fights `.editorconfig` two ways: - `.editorconfig` sets `indent_size = unset` for `tests/acceptance/**.sh` and `src/console_header.sh`; shfmt reads `unset` as "use my default", which is tabs, while `[*]` sets `indent_style = space`. Any new file under `tests/acceptance/` - therefore shows up shfmt-dirty — that is expected, not a defect. + therefore shows up shfmt-dirty, that is expected, not a defect. - shfmt collapses `\`-continued strings onto one line, which would push `src/state.sh`'s record writer past `max_line_length = 120`. @@ -70,9 +70,27 @@ Study existing tests before writing new ones: - **Lifecycle Hooks**: `tests/unit/setup_teardown_test.sh` - **CLI Testing**: `tests/acceptance/bashunit_test.sh` -## Path-Scoped Guidelines +## Task Context -Rules auto-load based on file paths being edited (via `paths:` frontmatter in each rule file). +All paths below are relative to the repository root, including from a generated skill or agent directory. Read only the rules relevant to the task before editing. Claude also loads them automatically through their path selectors. + +| Task | Read | +|------|------| +| Changes to `src/` or `bashunit` | `.agnostic-ai/rules/architecture-map.md` | +| Shell changes under `src/` or `tests/` | `.agnostic-ai/rules/bash-style.md` | +| Changes to `src/` or fork census acceptance tests | `.agnostic-ai/rules/perf-fork-budget.md` | +| Shell development or task plans | `.agnostic-ai/rules/tdd-workflow.md` | +| Test changes | `.agnostic-ai/rules/testing.md` | + +## Delegation + +Keep one acceptance scenario and a short plan in the main session. Handle small edits directly. Delegate independent lookups to `legolas`, a bounded implementation to `aragorn`, and defect review to `gandalf` before pushing. Give each agent a specific question or file scope, a base revision, and a validation budget. Reuse its findings instead of repeating its searches. + +Keep at most two delegates active. Workers share the checkout: assign separate files and never revert another worker's edits. Run the full quality gate once in the main session after the change set is complete. Delegates run only a targeted check when needed to prove a finding. Return concise evidence with file and line references; put longer reports under the ignored `.agnostic-ai/local/` directory. + +## Editing Instructions + +Edit `.agnostic-ai/` and `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Native instruction files, skills, agents, and settings are generated and ignored by Git. Never edit them directly. ## Guardrails diff --git a/.agnostic-ai/agents/aragorn.md b/.agnostic-ai/agents/aragorn.md new file mode 100644 index 000000000..fcfe37003 --- /dev/null +++ b/.agnostic-ai/agents/aragorn.md @@ -0,0 +1,8 @@ +--- +name: aragorn +description: Implement a bounded change in assigned files with Bash 3.0 compatibility and targeted validation. +model: balanced +can: [read, Grep, Glob, shell, edit, write] +--- + +You own only the files assigned by the main agent. Other workers share the checkout; preserve their edits and adapt to them. Read the applicable source rules listed in .agnostic-ai/AGNOSTIC_AI.md before editing. Follow the existing test patterns and the requested acceptance scenario. Use TDD for runtime changes. Keep hot paths within the fork budget. Run only the assigned targeted validation, never the full gate. Do not commit, push, publish, or install dependencies. Return changed files, the check result, and any unresolved limit. diff --git a/.agnostic-ai/agents/gandalf.md b/.agnostic-ai/agents/gandalf.md new file mode 100644 index 000000000..14bcc839a --- /dev/null +++ b/.agnostic-ai/agents/gandalf.md @@ -0,0 +1,10 @@ +--- +name: gandalf +description: Review a completed diff for verified defects before a push or merge. +model: strong +readonly: true +x-codex: {sandbox_mode: null} +can: [read, Grep, Glob, shell] +--- + +Review without editing files, committing, or posting to GitHub. Read the diff against the supplied base and the applicable source rules listed in .agnostic-ai/AGNOSTIC_AI.md. Trace changed behavior through callers and existing tests. Check Bash 3.0 parsing, dynamic scope, subshell state, parallel isolation, cleanup, and hot-path process costs when relevant. For configuration changes, check clean-clone generation, canonical references, model resolution, and preserved native behavior. Do not run the full quality gate. Return only verified findings as file:line, severity, concrete failing input, and the smallest fix. If none remain, say so and list the scope checked. diff --git a/.agnostic-ai/agents/legolas.md b/.agnostic-ai/agents/legolas.md new file mode 100644 index 000000000..4c94c5598 --- /dev/null +++ b/.agnostic-ai/agents/legolas.md @@ -0,0 +1,10 @@ +--- +name: legolas +description: Locate definitions, callers, and references before planning a nontrivial change. +model: fast +readonly: true +x-codex: {sandbox_mode: null} +can: [read, Grep, Glob, shell] +--- + +Find code without editing it or proposing a redesign. Use rg, then read only enough surrounding code to verify each match. Return a file:line table and the searches used when nothing matches. Do not run tests, builds, installs, or Git mutations. diff --git a/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml b/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml new file mode 100644 index 000000000..533f5b53a --- /dev/null +++ b/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml @@ -0,0 +1,6 @@ +target: claude +command: file="$CLAUDE_FILE_PATH"; if [ -n "$file" ] && echo "$file" | grep -q '\.sh$'; then shellcheck -x "$file" 2>&1 | head -20; fi +description: Runs `file="$CLAUDE_FILE_PATH"; if [ -n "$file" ] && echo "$file" ...` on PostToolUse for Edit|Write. +event: PostToolUse +matcher: Edit|Write +name: posttooluse-edit-write-if diff --git a/.agnostic-ai/overlays/claude.settings.hook-events.json b/.agnostic-ai/overlays/claude.settings.hook-events.json new file mode 100644 index 000000000..5078d82b7 --- /dev/null +++ b/.agnostic-ai/overlays/claude.settings.hook-events.json @@ -0,0 +1,6 @@ +{ + "after": "includeCoAuthoredBy", + "events": [ + "PostToolUse" + ] +} diff --git a/.agnostic-ai/overlays/claude.settings.json b/.agnostic-ai/overlays/claude.settings.json new file mode 100644 index 000000000..4fa8942f2 --- /dev/null +++ b/.agnostic-ai/overlays/claude.settings.json @@ -0,0 +1,4 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "includeCoAuthoredBy": false +} diff --git a/.agnostic-ai/overlays/codex.config.toml b/.agnostic-ai/overlays/codex.config.toml new file mode 100644 index 000000000..b345ee4f5 --- /dev/null +++ b/.agnostic-ai/overlays/codex.config.toml @@ -0,0 +1,2 @@ +[agents] +max_concurrent_threads_per_session = 2 diff --git a/.claude/rules/architecture-map.md b/.agnostic-ai/rules/architecture-map.md similarity index 84% rename from .claude/rules/architecture-map.md rename to .agnostic-ai/rules/architecture-map.md index cb05c1290..4f33273b8 100644 --- a/.claude/rules/architecture-map.md +++ b/.agnostic-ai/rules/architecture-map.md @@ -1,4 +1,5 @@ --- +target: claude paths: - "src/**/*.sh" - "bashunit" @@ -6,7 +7,7 @@ paths: # Architecture Map: modules and the life of a run -Orientation for working in `src/` — which file owns what, and the exact call +Orientation for working in `src/`, which file owns what, and the exact call flow of a test run. Line numbers drift; function names are the stable anchors. ## The life of `./bashunit tests/` @@ -15,7 +16,7 @@ flow of a test run. Line numbers drift; function names are the stable anchors. bashunit (entry) sources all src/*.sh; version gate; early flag scan └─ bashunit::main::cmd_test (main.sh: flag parsing, env exports) └─ bashunit::runner::load_test_files (runner/discovery.sh: the per-file loop) - ├─ console_header::print_header "Running N tests" — captures + ├─ console_header::print_header "Running N tests", captures │ └─ helper::find_total_tests $() SUBSHELL: sources each file in a │ nested subshell just to count tests ├─ source "$test_file" in the MAIN shell (workers inherit) @@ -48,12 +49,12 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). | Module | Owns | |--------|------| | `bashunit` | entry: version gate, early flag scan, sources every module, subcommand routing | -| `main/index.sh` | aggregator only — sources the `src/main/` module below | +| `main/index.sh` | aggregator only, sources the `src/main/` module below | | `main/validate.sh` | shared parser validation: unknown options, integer/path checks, shard parsing | | `main/test.sh` / `main/bench.sh` / `main/subcommands.sh` | flag parsing per subcommand | | `main/assert.sh` | the standalone `bashunit assert` path | | `main/watch.sh` / `main/run.sh` | the `--watch` loop; the run lifecycle, exit codes and cleanup | -| `runner/index.sh` | aggregator only — sources the `src/runner/` module below | +| `runner/index.sh` | aggregator only, sources the `src/runner/` module below | | `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes | | `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload | | `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers | @@ -65,22 +66,22 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). | `runner/list.sh` | `--list`/`--dry-run`: emits selected tests instead of running them; hooks in after all filtering, before any hook | | `runner/discovery.sh` | `load_test_files` (the per-file loop), `functions_for_script` | | `runner/bench.sh` | benchmark file loop and bench function dispatch | -| `helper/index.sh` | aggregator only — sources the `src/helper/` module below | +| `helper/index.sh` | aggregator only, sources the `src/helper/` module below | | `helper/naming.sh` | resolving and normalising test-function and variable names | | `helper/discovery.sh` | `find_files_recursive`, `find_total_tests`, fn filtering, duplicate check, line lookup | | `helper/provider.sh` / `helper/tags.sh` | `@data_provider` map; `@tag` extraction and matching | | `helper/encoding.sh` / `helper/functions.sh` / `helper/git.sh` | base64 + ids; generic fn utilities; remote tag lookup for `upgrade` | -| `state/index.sh` | aggregator only — sources the `src/state/` module below | +| `state/index.sh` | aggregator only, sources the `src/state/` module below | | `state/counters.sh` | test and assertion tallies and their get/add accessors | | `state/duplicates.sh` | duplicate test-function detection state | | `state/context.sh` | per-test output buffer, exit code, title, hook failure, and the per-test reset | | `state/payload.sh` | the encoded per-test result payload and the base64 capability probe | | `state/parallel.sh` | aggregating per-test result files after a `--parallel` run | -| `config/index.sh` | aggregator only — sources the `src/config/` module below | +| `config/index.sh` | aggregator only, sources the `src/config/` module below | | `config/env.sh` | all `BASHUNIT_*` defaults/config files, scratch dirs (`_BASHUNIT_RUN_OUTPUT_DIR` + EXIT-trap cleanup); executes at source time | | `config/rerun.sh` | the `.bashunit/last-failed` cache for `--rerun-failed` | | `config/parallel.sh` | is this run parallel; worker temp tree and stop-on-failure flag file (not `runner/parallel.sh`, which waits on job slots) | -| `console/index.sh` | aggregator only — sources the `src/console/` module below | +| `console/index.sh` | aggregator only, sources the `src/console/` module below | | `console/colors.sh` | the `_BASHUNIT_COLOR_*` palette and `bashunit::sgr` | | `console/header.sh` | the "Running N tests" header | | `console/line.sh` | `print_line`, the primitive every result line goes through, and its TAP variant | @@ -88,62 +89,62 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). | `console/test_line.sh` | the per-test result lines (passed/failed/skipped/incomplete/snapshot/risky/error) | | `console/deferred.sh` | end-of-run blocks buffered during the run (scratch files under the run dir) | | `console/summary.sh` | run totals, execution time, hook completion | -| `assert/index.sh` | aggregator only — sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | +| `assert/index.sh` | aggregator only, sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | | `assert/core.sh` | `assert::should_skip`, `assert::fail_with`, `assert::join_to_slot` and the comparison assertions the other files build on | | `assert/{arrays,assertions,dates,duration,files,folders,json,once,snapshot}.sh` | the per-topic assertions; the per-assertion path must stay fork-free | -| `util/index.sh` | aggregator only — sources the `src/util/` module below | +| `util/index.sh` | aggregator only, sources the `src/util/` module below | | `util/clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads | | `util/str.sh` / `util/math.sh` | pure-bash string and arithmetic helpers | -| `api/index.sh` | aggregator only — sources the `src/api/` module below | -| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone — the entrypoint already set it | +| `api/index.sh` | aggregator only, sources the `src/api/` module below | +| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone, the entrypoint already set it | | `api/skip_todo.sh` / `api/test_title.sh` | `skip`/`todo` markers; `set_test_title` | | `api/bashunit.sh` | the custom-assert facade (`assert_that`, `assertion_failed`/`passed`) | -| `doubles/index.sh` | aggregator only — sources the `src/doubles/` module below | +| `doubles/index.sh` | aggregator only, sources the `src/doubles/` module below | | `doubles/mock.sh` | `mock`/`unmock` and the `_BASHUNIT_MOCKED_FUNCTIONS` registry `runner/hooks.sh` unwinds per test | | `doubles/spy.sh` | `spy` and its `_BASHUNIT_SPY_*` state slots + files | | `doubles/assertions.sh` | the `assert_have_been_called*` family | -| `coverage/index.sh` | aggregator only — sources the `src/coverage/` modules below | +| `coverage/index.sh` | aggregator only, sources the `src/coverage/` modules below | | `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | | `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | | `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | | `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | | `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | | `coverage/functions.sh` | function definitions and their line spans | -| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose** — the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | +| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose**, the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | | `coverage/report_text.sh` / `report_lcov.sh` / `report_html.sh` | the three renderers | | `coverage/html_index.sh` / `html_file.sh` | HTML page emitters; the only two files exempt from `max_line_length` | | `reports.sh` | JUnit/HTML/TAP/JSON writers | -| `system/index.sh` | aggregator only — sources the `src/system/` module below | +| `system/index.sh` | aggregator only, sources the `src/system/` module below | | `system/check_os.sh` / `system/dependencies.sh` | one-fork OS detect; `command -v` probes (builtins, not forks) | | `system/io.sh` | small I/O helpers | -| `cli/index.sh` | aggregator only — sources the `src/cli/` module below | +| `cli/index.sh` | aggregator only, sources the `src/cli/` module below | | `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; `main.sh` is their only caller | | `benchmark/index.sh` | the remaining non-`test` subcommand; shared with `runner/bench.sh` | ## Cross-cutting invariants -- **Bash 3.0 floor** (`.claude/rules/bash-style.md`): no `[[`, `declare -A`, - `${var,,}`, `BASHPID`, negative indices. Subshells share `$$` — you cannot +- **Bash 3.0 floor** (`.agnostic-ai/rules/bash-style.md`): no `[[`, `declare -A`, + `${var,,}`, `BASHPID`, negative indices. Subshells share `$$`, you cannot make a per-worker unique token without a fork (`mktemp`) or an ordinal assigned before the fork. `RANDOM` is no help either: whether a subshell reseeds it depends on the platform and the nesting depth (measured in perf-fork-budget.md), so it is unreliable in both directions (#1354). - **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` - captures on hot paths — bash-style.md documents it; `local` is dynamically + captures on hot paths, bash-style.md documents it; `local` is dynamically scoped, so helpers must not write caller-named variables. -- **Fork budget** (`.claude/rules/perf-fork-budget.md`): per-test paths are +- **Fork budget** (`.agnostic-ai/rules/perf-fork-budget.md`): per-test paths are fork-free (sequential) or mktemp-only (parallel); budgets are enforced by - `tests/acceptance/bashunit_*forks*_test.sh` on three platforms — a "harmless" + `tests/acceptance/bashunit_*forks*_test.sh` on three platforms, a "harmless" `echo | sed` in a per-test path will fail CI. - **`call_test_functions` is a background subshell under `--parallel`** - (`discovery.sh` backgrounds it per file). Anything it sets in shell state — - counters, flags, error records — dies with the worker. Only two things cross + (`discovery.sh` backgrounds it per file). Anything it sets in shell state, + counters, flags, error records, dies with the worker. Only two things cross back: per-test `.result` payloads, and files written to disk. This produced two silent-green bugs in one day: a data-provider error printed while the run exited 0 (#1145), and duplicate-test-function detection being entirely off in the mode CI uses (#1147). Before adding a check anywhere under that call, ask - which side of the fork it belongs on — a whole-file check almost always + which side of the fork it belongs on, a whole-file check almost always belongs in the parent loop, before dispatch. **Verify both modes.** Sequential and `--parallel` disagreeing is the signature of this bug, and it is invisible to a sequential-only test. @@ -155,7 +156,7 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). `build::process_file`): a file's body is emitted, *then* its `source` lines are recursed into. That equals dev-mode order only if a module aggregator (`src//index.sh`, ADR-010) contains **nothing but `source` - lines and comments** — any other top-level statement would run before its + lines and comments**, any other top-level statement would run before its dependencies in the built artifact but after them in dev mode. Files are deduped by repo-relative path, so `src/` may hold module dirs and two files may share a basename. Every src file's **first line must be the shebang**: diff --git a/.claude/rules/bash-style.md b/.agnostic-ai/rules/bash-style.md similarity index 91% rename from .claude/rules/bash-style.md rename to .agnostic-ai/rules/bash-style.md index ffa88305d..402565eb5 100644 --- a/.claude/rules/bash-style.md +++ b/.agnostic-ai/rules/bash-style.md @@ -1,4 +1,5 @@ --- +target: claude paths: - "src/**/*.sh" - "tests/**/*.sh" @@ -29,14 +30,14 @@ enforced mechanically by `tests/unit/bash_compatibility_test.sh`, which greps | `BASHPID` | 4.0+ | fork (`mktemp`) or an assigned ordinal | | `mapfile` / `readarray` | 4.0+ | `while IFS= read -r` | | `declare -n` / `local -n` (nameref) | 4.3+ | Return-slot pattern (below) | -| `coproc` | 4.0+ | — | -| `${var@Q}` (transformations) | 4.4+ | — | +| `coproc` | 4.0+ |, | +| `${var@Q}` (transformations) | 4.4+ |, | `[[ =~ ]]` is a subtler case: it **exists** on 3.0 (verified against a real build), but 3.2 changed whether a quoted right-hand side is a regex or a literal. At a 3.0 floor the same match therefore behaves differently across -supported versions, so regex matching goes through `grep -E` — a real -constraint, not a style preference. See `.claude/rules/perf-fork-budget.md`. +supported versions, so regex matching goes through `grep -E`, a real +constraint, not a style preference. See `.agnostic-ai/rules/perf-fork-budget.md`. ### Style, not compatibility @@ -46,7 +47,7 @@ Bash 3 requirement, and do not add it to the compatibility gate. ## Coding Conventions -- **2 spaces** indent, no tabs — enforced by `make lint` (EditorConfig). Do **not** +- **2 spaces** indent, no tabs, enforced by `make lint` (EditorConfig). Do **not** run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses `\`-continued strings past the 120-char limit, breaking `make lint` - **120 chars** max line length (soft) @@ -82,14 +83,14 @@ Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee shadows the caller's same-named variable for the duration of the call. Same trap fires with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...` -writes — they all resolve against the dynamic scope. +writes, they all resolve against the dynamic scope. We need **all three** properties on the hot path: 1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork per call, which dominates per-test cost. 2. **No collision with caller locals.** A helper that silently overwrites or reads - the wrong variable is the worst kind of bug — no error, just stale data. + the wrong variable is the worst kind of bug, no error, just stale data. 3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and it has the same dynamic-scope shadowing as `eval "$name=..."` anyway). @@ -152,7 +153,7 @@ function bashunit::pkg::do_thing() { ``` - All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing - silently breaks the caller's outvar (see PR #672 — that exact bug caused 12 parallel + silently breaks the caller's outvar (see PR #672, that exact bug caused 12 parallel test failures in #662). - Include a regression test that calls the helper with one of its own documented internal names as the outvar. @@ -161,7 +162,7 @@ function bashunit::pkg::do_thing() { The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends) deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). -That is documented inline above the helpers in that file and is **not** the outvar pattern — +That is documented inline above the helpers in that file and is **not** the outvar pattern, the helper has no `$1`-named outvar argument; the caller agrees to share state by convention. It is also why those helpers and `extract_branches` must stay in one file (ADR-010). Don't introduce new instances of this pattern without an inline justification comment. diff --git a/.claude/rules/perf-fork-budget.md b/.agnostic-ai/rules/perf-fork-budget.md similarity index 89% rename from .claude/rules/perf-fork-budget.md rename to .agnostic-ai/rules/perf-fork-budget.md index 559abc961..7b27e398e 100644 --- a/.claude/rules/perf-fork-budget.md +++ b/.agnostic-ai/rules/perf-fork-budget.md @@ -1,4 +1,5 @@ --- +target: claude paths: - "src/**/*.sh" - "tests/acceptance/bashunit_*forks*_test.sh" @@ -27,18 +28,18 @@ grep -c . /tmp/count ``` Caveats: binaries pinned at startup via `command -v` (`$GREP`, `$MKTEMP`, -`$CAT`) bypass PATH shims — trace those instead; and shims are unreliable on +`$CAT`) bypass PATH shims, trace those instead; and shims are unreliable on Git Bash (skip such tests on Windows). **A shim census only sees forks that exec a binary.** `$(some_shell_function)` forks a subshell and execs nothing, so it is invisible to every shim and to the -`^\++ +/path/to/binary` trace patterns above — yet it costs the same ~0.6ms. +`^\++ +/path/to/binary` trace patterns above, yet it costs the same ~0.6ms. That blind spot hid sixteen of them in the colour palette (`$(bashunit::sgr N)` per colour, ~9ms, the largest single cost in a cold start) straight through the #801-#851 campaign that pinned everything else on this page. To count them, match the *function name* in the trace (`^\++ +bashunit::fn( |$)`), or profile by injecting `$EPOCHREALTIME` echoes at the `# src/` markers the build -emits — that attributes startup cost per source file and is how this one +emits, that attributes startup cost per source file and is how this one surfaced. **`bash -x` trace census (cheap but inflated).** `PS4='+ ' bash -x ./bashunit …` @@ -71,17 +72,17 @@ fork class; they are the RED test of the TDD cycle. | `printf \| grep -cE` per line as a classifier fallback | `case` globs translating each regex alternation | #1005 (286 lines cost 1055 forks) | `shopt -s extdebug` for `declare -F`: enable it **inside the capture subshell -only** — toggling it in the caller's shell clobbers caller state (#808). +only**, toggling it in the caller's shell clobbers caller state (#808). -## Where pure bash LOSES — measured, do not "optimize" these +## Where pure bash LOSES, measured, do not "optimize" these - **`… | awk` over ≳100 lines**: a `while read` loop was 5x slower than the awk fork (19ms vs 3.5ms for 600 `declare -F` lines). File scans (provider map, duplicate check) stay awk. -- **`${var//pattern/}` on large strings**: quadratic on Bash 3.2 — 2.7 s where +- **`${var//pattern/}` on large strings**: quadratic on Bash 3.2, 2.7 s where awk takes 3.5ms. Never string-replace over big captures. - **Regex assertions** (`assert_matches`): the `grep -E` fork is **not** because - `[[ =~ ]]` is missing — it was verified working on a real Bash 3.0 build. It is + `[[ =~ ]]` is missing, it was verified working on a real Bash 3.0 build. It is because 3.2 changed whether a quoted right-hand side is a regex or a literal, so at the 3.0 floor the same pattern would match differently across supported versions. Removing the fork means picking one semantic and breaking the other. @@ -109,7 +110,7 @@ believing a number. `>>` appends; holding the files open on persistent descriptors recovered **184 ms** of it, not 705 (#1110). - **A benchmark path must be the path that occurs.** `${file//[^a-zA-Z0-9]/_}` - costs 7.8 µs at 7 characters, 131 µs at 59 and 549 µs at 121 — so measuring + costs 7.8 µs at 7 characters, 131 µs at 59 and 549 µs at 121, so measuring it with an absolute path when the trap actually sees `./src/x.sh` inflated the estimate by 4x (#1102). @@ -128,7 +129,7 @@ diff: /dev/fd/62: Bad file descriptor `diff` then produces no comparison and writes that to stderr, which a caller collecting output reads as a real difference. Both coverage differentials did -this once per repo file — 473 iterations — and failed roughly 1 full suite run +this once per repo file, 473 iterations, and failed roughly 1 full suite run in 3, each time reporting "the classifiers disagree" over a file that agreed perfectly (#1152). Two earlier attempts blamed `awk`; the `AWK-FAILED` marker from #1143 is what finally ruled it out. @@ -139,7 +140,7 @@ shell opens the descriptor and dups it to stdin, so no external process opens So: never put `<(...)` in an external command's **argument list** inside a loop that runs per file or per test. Write real files (concurrently, so the two -sides still overlap — serialising them was measurably slower) and compare with +sides still overlap, serialising them was measurably slower) and compare with `[ "$a" != "$b" ]`, which the shell does without a fork. ## Where pure bash LOSES on some Bash versions but not others @@ -148,7 +149,7 @@ sides still overlap — serialising them was measurably slower) and compare with REPLACEMENT mean "the matched text", so `${line//` marker technique: 41.6ms of executed top-level code, against 10ms to parse the whole 600KB artifact and define all 915 functions. @@ -230,9 +231,9 @@ Lazy-loading whole modules was still rejected in #798. Measuring a change this small needs ~200 invocations per sample: the two forks are ~4ms against a ~65ms startup, and single runs vary by ±10ms. The acceptance -suite is useless for it — consecutive runs there differ by 5s. +suite is useless for it, consecutive runs there differ by 5s. `--coverage` adds ~3 forks per unique file first seen by the DEBUG trap -(decision-cache miss: grep|head + dirname) — bounded by file count, nightly +(decision-cache miss: grep|head + dirname), bounded by file count, nightly non-gating workflow, not worth chasing. **Don't assume the engine is the cost.** Profiling `--coverage` for #1005 found @@ -243,7 +244,7 @@ twice per run (`precompute_file_stats`, then `report_lcov`). Budget guarded by `tests/acceptance/bashunit_coverage_forks_test.sh`, expressed as "fork count does not grow with source-line count" rather than an absolute number. -Two traps when replacing a regex classifier with `case` globs — both silently +Two traps when replacing a regex classifier with `case` globs, both silently change coverage numbers rather than erroring: - Inside a POSIX bracket expression a backslash is a **literal member of the @@ -254,16 +255,16 @@ change coverage numbers rather than erroring: Verify such a rewrite by running both implementations over the same input (`git show HEAD:` + `declare -f | sed` to alias the old one) across every -`git ls-files '*.sh'` line, and mutation-test the harness itself — a differential +`git ls-files '*.sh'` line, and mutation-test the harness itself, a differential that cannot fail proves nothing. -**Parallel 10-test file run (CI's mode):** ~11 forks — 3 `mkdir`, 4 `rm`, +**Parallel 10-test file run (CI's mode):** ~11 forks, 3 `mkdir`, 4 `rm`, 3 `awk` (#813; was 61). That count was taken with a sequential census fixture and so missed the one cost that only exists in this mode: every worker used to re-probe the clock, because the probe resolved the implementation inside a `$( )` and the resolved value died with that subshell. On a shell without `EPOCHREALTIME` the probe forks `perl`, so it scaled one-for-one with the tests -— 502 execs for a 500-test file against 2 — and it fired even with per-test +, 502 execs for a 500-test file against 2, and it fired even with per-test timing off, since deciding that timing is off is what asks whether the clock is expensive (#1353). **Measure the parallel budget with a parallel fixture**: a per-worker cost is invisible to a sequential census by construction. The per-test result file is named by a per-suite @@ -271,7 +272,7 @@ ordinal the single-threaded dispatcher assigns just before each `&` (the fork inherits it), so it costs **no** `mktemp` + `mv` per test (#851; was 10 `mktemp` + 10 `mv`). This replaced the old sanitized-test-name scheme, whose deterministic names could collide (different provider args sanitize identically) -because Bash 3 workers can't mint a unique token — subshells inherit `$$`, and +because Bash 3 workers can't mint a unique token, subshells inherit `$$`, and `BASHPID` is 4.0+; an ordinal sidesteps that entirely. This file used to add "and the `RANDOM` state" to that list. Measured, `RANDOM` @@ -279,21 +280,21 @@ is neither reliably shared nor reliably reseeded (#1354): | Context | three consecutive `$( )` reads | |---|---| -| Plain shell — 3.00.22, 3.2.57, 4.4, 5.2, 5.3 | differ | -| `--parallel` worker — Linux 3.0, 5.2 | differ | -| `--parallel` worker — macOS 3.2.57 | **identical** | +| Plain shell, 3.00.22, 3.2.57, 4.4, 5.2, 5.3 | differ | +| `--parallel` worker, Linux 3.0, 5.2 | differ | +| `--parallel` worker, macOS 3.2.57 | **identical** | So it depends on the platform *and* on how deeply nested the subshell is. Do not build on it in either direction, and do not reuse "`RANDOM` is shared" as a -premise — it is right about the conclusion for the wrong reason. `$$` is the +premise, it is right about the conclusion for the wrong reason. `$$` is the part that genuinely is inherited. `wait_for_job_slot` already uses `wait -n` on Bash 4.3+ and an adaptive -sleep-poll fallback — don't "fix" it. The spinner forks `sleep` ~1/s on +sleep-poll fallback, don't "fix" it. The spinner forks `sleep` ~1/s on non-tty; not worth chasing. **Reports are on the per-test path too.** Any `--log-*`/`--report-*` flag makes each worker spool a result row, and that row used to base64 each of its nine -fields on its own — 14 `base64` forks per test counting the parent's decode, +fields on its own, 14 `base64` forks per test counting the parent's decode, with a `tr` on each encode. `--log-junit` therefore cost more than the run it reported on (300 tests: 1.2s → 9.2s wall, 5.8s → 27.4s CPU). Now only the four fields that can hold arbitrary text are encoded and the row is joined with diff --git a/.claude/rules/tdd-workflow.md b/.agnostic-ai/rules/tdd-workflow.md similarity index 80% rename from .claude/rules/tdd-workflow.md rename to .agnostic-ai/rules/tdd-workflow.md index 73c916ea4..1cedcb537 100644 --- a/.claude/rules/tdd-workflow.md +++ b/.agnostic-ai/rules/tdd-workflow.md @@ -1,4 +1,5 @@ --- +target: claude paths: - "**/*.sh" - ".tasks/**/*.md" @@ -47,19 +48,19 @@ Test: (none yet) 1. Pick the **smallest next test** from inventory 2. Study patterns from existing tests (unit -> `assert_test.sh`, doubles -> `doubles_test.sh`) 3. Write test following Arrange-Act-Assert -4. Run: `./bashunit path/to/test.sh` — **must fail** +4. Run: `./bashunit path/to/test.sh`, **must fail** 5. Verify failure reason: function missing or wrong output (**not** syntax error) ## GREEN Phase -1. Write **minimal** code to pass — no extra features, no premature optimization -2. Run: `./bashunit path/to/test.sh` — **must pass** +1. Write **minimal** code to pass, no extra features, no premature optimization +2. Run: `./bashunit path/to/test.sh`, **must pass** ## REFACTOR Phase -1. Improve readability, naming, extract duplication — **no behavior changes** +1. Improve readability, naming, extract duplication, **no behavior changes** 2. Run tests after each change -3. Run quality checks: `make sa && make lint` (never `shfmt -w` — it breaks `make lint`) +3. After the change set is complete, run quality checks once: `make sa && make lint` (never `shfmt -w`, it breaks `make lint`) ## Quality Gate (Before Commit) diff --git a/.claude/rules/testing.md b/.agnostic-ai/rules/testing.md similarity index 89% rename from .claude/rules/testing.md rename to .agnostic-ai/rules/testing.md index d8a6aeb18..9dd4186a5 100644 --- a/.claude/rules/testing.md +++ b/.agnostic-ai/rules/testing.md @@ -1,4 +1,5 @@ --- +target: claude paths: - "tests/**/*_test.sh" --- @@ -63,7 +64,7 @@ bashunit::mock date echo "2024-05-01" bashunit::mock uname <<< "Linux" # heredoc form ignores the call's arguments ``` -Note `_times` takes the count first while `_with` takes the spy first — a swapped pair +Note `_times` takes the count first while `_with` takes the spy first, a swapped pair fails the assertion rather than erroring, so it reads like a real defect. ## Data Providers @@ -82,10 +83,10 @@ function test_should_add() { ## Lifecycle Hooks -- `set_up_before_script()` — once before all tests in file -- `set_up()` — before each test -- `tear_down()` — after each test -- `tear_down_after_script()` — once after all tests in file +- `set_up_before_script()`, once before all tests in file +- `set_up()`, before each test +- `tear_down()`, after each test +- `tear_down_after_script()`, once after all tests in file ## Snapshot Testing @@ -114,16 +115,16 @@ function this file never defines. Give every embedded fixture function a name unique across the file. The report includes the line of each definition (`test_x (lines 30, 40)`), which is what identifies the strings. -It cannot be fixed by tracking quote state — an apostrophe in any comment +It cannot be fixed by tracking quote state, an apostrophe in any comment (`# doesn't work`) would flip it and mask real duplicates. ## Test Isolation - Use `$(bashunit::temp_file)` / `$(bashunit::temp_dir)` (auto-cleaned) for file - operations — these are functions, not variables + operations, these are functions, not variables - No shared global state between tests -- No network calls — mock external commands -- No time dependencies — mock `date` if needed +- No network calls, mock external commands +- No time dependencies, mock `date` if needed - Tests must be safe for `./bashunit --parallel tests/` ## Reference Tests diff --git a/.agnostic-ai/settings/models.yaml b/.agnostic-ai/settings/models.yaml new file mode 100644 index 000000000..c0dc68256 --- /dev/null +++ b/.agnostic-ai/settings/models.yaml @@ -0,0 +1 @@ +model: balanced diff --git a/.claude/skills/add-assertion/SKILL.md b/.agnostic-ai/skills/add-assertion/SKILL.md similarity index 74% rename from .claude/skills/add-assertion/SKILL.md rename to .agnostic-ai/skills/add-assertion/SKILL.md index 810a9bf44..a48c0cc3c 100644 --- a/.claude/skills/add-assertion/SKILL.md +++ b/.agnostic-ai/skills/add-assertion/SKILL.md @@ -1,7 +1,7 @@ --- name: add-assertion description: Add new assertion function with comprehensive tests following TDD -allowed-tools: Read, Edit, Write, Bash, Grep, Glob +allowed-tools: read, edit, write, shell, Grep, Glob --- # Add Assertion Skill @@ -10,8 +10,8 @@ Add a new assertion function to bashunit following strict TDD methodology. ## Prerequisites -1. **Task file must exist** — create `.tasks/YYYY-MM-DD-add-assertion-name.md` -2. **Study patterns** — read `tests/unit/assert_test.sh` and `src/assertions.sh` +1. **Task file must exist**, create `.tasks/YYYY-MM-DD-add-assertion-name.md` +2. **Study patterns**, read `tests/unit/assert_test.sh` and `src/assertions.sh` ## Workflow @@ -32,10 +32,10 @@ Read `src/assertions.sh` and `tests/unit/assert_test.sh` to understand: For each test in inventory, follow RED -> GREEN -> REFACTOR: -1. **Basic success case** — assertion passes with valid input -2. **Failure case** — assertion fails correctly: capture its output and compare it with +1. **Basic success case**, assertion passes with valid input +2. **Failure case**, assertion fails correctly: capture its output and compare it with `bashunit::console_results::print_failed_test` (there is no `assert_fails`) -3. **Edge cases** — empty input, special characters, nested structures, malformed data +3. **Edge cases**, empty input, special characters, nested structures, malformed data ### 4. Integration diff --git a/.claude/skills/check-coverage/SKILL.md b/.agnostic-ai/skills/check-coverage/SKILL.md similarity index 85% rename from .claude/skills/check-coverage/SKILL.md rename to .agnostic-ai/skills/check-coverage/SKILL.md index f6096926a..d2be38c29 100644 --- a/.claude/skills/check-coverage/SKILL.md +++ b/.agnostic-ai/skills/check-coverage/SKILL.md @@ -1,7 +1,7 @@ --- name: check-coverage description: Analyze test coverage and identify untested code paths -allowed-tools: Read, Bash, Grep, Glob +allowed-tools: read, shell, Grep, Glob --- # Check Coverage @@ -30,9 +30,9 @@ grep -r "function_name" tests/ ### 3. Categorize Coverage -- **Well tested** — multiple tests, edge cases covered -- **Partially tested** — basic test exists, missing edge cases -- **Not tested** — no tests found +- **Well tested**, multiple tests, edge cases covered +- **Partially tested**, basic test exists, missing edge cases +- **Not tested**, no tests found ### 4. Identify Critical Gaps diff --git a/.claude/skills/commit/SKILL.md b/.agnostic-ai/skills/commit/SKILL.md similarity index 52% rename from .claude/skills/commit/SKILL.md rename to .agnostic-ai/skills/commit/SKILL.md index c3cfa166b..992b54ef5 100644 --- a/.claude/skills/commit/SKILL.md +++ b/.agnostic-ai/skills/commit/SKILL.md @@ -4,48 +4,84 @@ description: Stage and commit changes using conventional commits format user-invocable: true argument-hint: "[message hint]" disable-model-invocation: true -allowed-tools: Bash, Read, Grep, Glob +allowed-tools: shell, read, Grep, Glob --- # Commit with Conventional Commits Stage and commit current changes using the conventional commits format. -> **IMPORTANT:** This skill MUST be used for ALL commits — even when the user says "commit" without `/commit`. Never commit without following these steps. +> **IMPORTANT:** This skill MUST be used for ALL commits, even when the user says "commit" without `/commit`. Never commit without following these steps. ## Current State +::target claude - Branch: !`git branch --show-current` +::end + +::target codex +- Run Branch: `git branch --show-current` +::end +::target claude - Status: !`git status --short` +::end + +::target codex +- Run Status: `git status --short` +::end +::target claude - Staged diff: !`git diff --cached --stat 2>/dev/null` +::end + +::target codex +- Run Staged diff: `git diff --cached --stat 2>/dev/null` +::end +::target claude - Unstaged diff: !`git diff --stat 2>/dev/null` +::end + +::target codex +- Run Unstaged diff: `git diff --stat 2>/dev/null` +::end +::target claude - Recent commits: !`git log --oneline -5 2>/dev/null` +::end + +::target codex +- Run Recent commits: `git log --oneline -5 2>/dev/null` +::end ## Arguments +::target claude - `$ARGUMENTS` - Optional hint for the commit message (e.g., `fix the snapshot comparison`) +::end + +::target codex +- the argument in the user request - Optional hint for the commit message (e.g., `fix the snapshot comparison`) +::end ## Instructions -1. **Review the state above** — understand what changed and why. +1. **Review the state above**, understand what changed and why. -2. **Stage files** — add only the relevant changed files by name. Never use `git add -A` or `git add .`. Never stage files that contain secrets (`.env`, credentials, etc.). +2. **Stage files**, add only the relevant changed files by name. Never use `git add -A` or `git add .`. Never stage files that contain secrets (`.env`, credentials, etc.). 3. **Determine the commit type** from the nature of the changes: - - `feat` — new feature or capability - - `fix` — bug fix - - `docs` — documentation only - - `style` — formatting, whitespace (no logic change) - - `refactor` — code restructuring (no behavior change) - - `test` — adding or updating tests - - `chore` — maintenance, tooling, config - - `perf` — performance improvement + - `feat`, new feature or capability + - `fix`, bug fix + - `docs`, documentation only + - `style`, formatting, whitespace (no logic change) + - `refactor`, code restructuring (no behavior change) + - `test`, adding or updating tests + - `chore`, maintenance, tooling, config + - `perf`, performance improvement 4. **Determine the scope** from the area of the codebase affected: - - `assert` — assertions (`src/assert*.sh`) - - `runner` — test runner - - `cli` — CLI entry point, flags, options - - `doubles` — mocks, spies - - `docs` — documentation site - - `ci` — CI/CD, GitHub Actions + - `assert`, assertions (`src/assert*.sh`) + - `runner`, test runner + - `cli`, CLI entry point, flags, options + - `doubles`, mocks, spies + - `docs`, documentation site + - `ci`, CI/CD, GitHub Actions - Use the most specific scope that fits. Omit if changes span many areas. 5. **Write the commit message**: @@ -84,8 +120,8 @@ perf(runner): reduce subshell usage in test discovery ## Rules -- **One logical change per commit** — don't mix unrelated changes -- **Never use `--no-verify`** — if hooks fail, fix the issue +- **One logical change per commit**, don't mix unrelated changes +- **Never use `--no-verify`**, if hooks fail, fix the issue - **Never amend** unless the user explicitly asks -- **Always create a NEW commit** — even after a hook failure +- **Always create a NEW commit**, even after a hook failure - **Author**: use the git config identity (never commit as sandbox/default user) diff --git a/.claude/skills/fix-test/SKILL.md b/.agnostic-ai/skills/fix-test/SKILL.md similarity index 74% rename from .claude/skills/fix-test/SKILL.md rename to .agnostic-ai/skills/fix-test/SKILL.md index 653967951..ee922a1c0 100644 --- a/.claude/skills/fix-test/SKILL.md +++ b/.agnostic-ai/skills/fix-test/SKILL.md @@ -1,7 +1,7 @@ --- name: fix-test description: Debug and fix failing tests systematically -allowed-tools: Read, Edit, Bash, Grep, Glob +allowed-tools: read, edit, shell, Grep, Glob --- # Fix Test @@ -20,11 +20,11 @@ Parse: which files, which functions, error messages. ### 2. Categorize Each Failure -- **Test bug** — test itself is wrong (wrong expected value, bad setup) -- **Implementation bug** — code doesn't match expected behavior -- **Environment issue** — missing dependency, wrong fixture path -- **Race condition** — passes sequential, fails parallel -- **Flaky test** — network/time/random dependency +- **Test bug**, test itself is wrong (wrong expected value, bad setup) +- **Implementation bug**, code doesn't match expected behavior +- **Environment issue**, missing dependency, wrong fixture path +- **Race condition**, passes sequential, fails parallel +- **Flaky test**, network/time/random dependency ### 3. Fix diff --git a/.claude/skills/gh-issue/SKILL.md b/.agnostic-ai/skills/gh-issue/SKILL.md similarity index 80% rename from .claude/skills/gh-issue/SKILL.md rename to .agnostic-ai/skills/gh-issue/SKILL.md index 48d91e858..5804c5dd7 100644 --- a/.claude/skills/gh-issue/SKILL.md +++ b/.agnostic-ai/skills/gh-issue/SKILL.md @@ -3,7 +3,7 @@ name: gh-issue description: Fetch GitHub issue, create branch, plan and implement with TDD, then open PR user-invocable: true argument-hint: "" -allowed-tools: Bash, Read, Edit, Write, Grep, Glob, Agent, WebFetch +allowed-tools: shell, read, edit, write, Grep, Glob, Agent, WebFetch --- # GitHub Issue Workflow @@ -11,13 +11,25 @@ allowed-tools: Bash, Read, Edit, Write, Grep, Glob, Agent, WebFetch Fetch a GitHub issue, create branch, implement following TDD, and open a PR. ## Arguments +::target claude - `$ARGUMENTS` - Issue number (e.g., `42` or `#42`) +::end + +::target codex +- the argument in the user request - Issue number (e.g., `42` or `#42`) +::end ## Instructions ### Phase 1: Setup +::target claude 1. **Parse the issue number** from `$ARGUMENTS` (strip `#` if present) +::end + +::target codex +1. **Parse the issue number** from the argument in the user request (strip `#` if present) +::end 2. **Fetch issue details**: ```bash @@ -64,24 +76,30 @@ Fetch a GitHub issue, create branch, implement following TDD, and open a PR. - Files to Change - Implementation Order (smallest first step) +::target claude 9. **Use EnterPlanMode** if implementation is non-trivial +::end + +::target codex +9. **Keep a short implementation plan** in the main session if implementation is non-trivial +::end ### Phase 3: Implement -10. **Follow strict TDD workflow** (see @.claude/rules/tdd-workflow.md): +10. **Follow strict TDD workflow** (see `.agnostic-ai/rules/tdd-workflow.md`): For each test: - **RED** - Write failing test, verify it fails for the RIGHT reason - **GREEN** - Minimal code to pass - **REFACTOR** - Improve while keeping tests green -11. **Run full test suite frequently**: +11. **Run the full test suite once after the change set is complete**: ```bash ./bashunit tests/ ./bashunit --parallel tests/ ``` -12. **Quality checks** after each refactor: +12. **Quality checks** before committing: ```bash make sa && make lint ``` diff --git a/.claude/skills/gh-issues/SKILL.md b/.agnostic-ai/skills/gh-issues/SKILL.md similarity index 88% rename from .claude/skills/gh-issues/SKILL.md rename to .agnostic-ai/skills/gh-issues/SKILL.md index db667a57f..e33b1ae30 100644 --- a/.claude/skills/gh-issues/SKILL.md +++ b/.agnostic-ai/skills/gh-issues/SKILL.md @@ -4,7 +4,7 @@ description: Walk over all open GitHub issues that are unassigned or assigned to user-invocable: true argument-hint: "[--limit N] [--label foo] [--dry-run]" disable-model-invocation: true -allowed-tools: Read, Bash, Skill +allowed-tools: read, shell, Skill --- # GitHub Issues Watcher @@ -17,9 +17,9 @@ Driven from inside the Claude session rather than a polling shell script. ## Args -- `--limit N` — process at most N issues this run (default: all). -- `--label foo` — only issues carrying label `foo`. -- `--dry-run` — list issues that would be processed; do not invoke `/gh-issue`. +- `--limit N`, process at most N issues this run (default: all). +- `--label foo`, only issues carrying label `foo`. +- `--dry-run`, list issues that would be processed; do not invoke `/gh-issue`. Strip leading `#` if user passes `#123` style. @@ -47,7 +47,7 @@ Merge: - Deduplicate by `number`. - Keep only issues whose `assignees` array is empty **or** contains the current user (`gh api user -q .login`). - Drop issues assigned to anyone else (defensive). -- **Skip tracker/epic issues** — those whose body is a checklist of other issues (e.g. `- [ ] #123 …`). Process the child issues directly, not the parent. +- **Skip tracker/epic issues**, those whose body is a checklist of other issues (e.g. `- [ ] #123 …`). Process the child issues directly, not the parent. - Apply `--label` filter if given. - Apply `--limit` if given. - Sort ascending by `createdAt` (FIFO). @@ -88,7 +88,7 @@ For each issue in the queue: - commit with conventional message + `Closes #` in the body - PR opened via `/pr #` -3. **Before pushing**, run the exact command CI's strict job uses — plain +3. **Before pushing**, run the exact command CI's strict job uses, plain sequential runs miss simple-mode and strict-mode bugs that CI rejects: ```bash ./bashunit --parallel --simple --strict tests/ @@ -103,7 +103,7 @@ For each issue in the queue: ```bash gh pr checks --watch ``` - Fix red checks on the branch before moving on. Bash 3.0 CI fails often — + Fix red checks on the branch before moving on. Bash 3.0 CI fails often, check that job specifically. A red `docker`/registry step is usually a transient Docker Hub timeout, not your code: `gh run rerun --failed`. @@ -118,7 +118,7 @@ For each issue in the queue: gh issue close --reason completed -c "Done in # (merged)." ``` If the issue belongs to a tracker/epic, tick its checkbox in the parent - issue body. Use `sed` for the tick — bash `${body/- [ ] #N/...}` treats + issue body. Use `sed` for the tick, bash `${body/- [ ] #N/...}` treats `[ ]` as a glob (it matches a space, not literal brackets) and silently does nothing: ```bash @@ -138,7 +138,7 @@ For each issue in the queue: Halt the loop and surface the failure when: - `/gh-issue` errors out or leaves the worktree dirty. -- `./bashunit tests/` or `./bashunit --parallel tests/` fails after implementation (for the right reason — distinguish pre-existing failures; see Notes). +- `./bashunit tests/` or `./bashunit --parallel tests/` fails after implementation (for the right reason, distinguish pre-existing failures; see Notes). - `make sa` (ShellCheck) or `make lint` (EditorConfig) fails. - CI stays red after one fix attempt. - Merge is blocked by branch protection beyond `--admin` bypass. @@ -160,8 +160,8 @@ With `--dry-run`, only execute Phase 1 and print the queue. No assignment, no br ## Notes -- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.claude/rules/bash-style.md`. +- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.agnostic-ai/rules/bash-style.md`. - **Quality gate is `make sa` + `make lint`, not bare `shfmt`.** `shfmt -w .` without project flags rewrites the whole tree (collapses line-continuations, flips binary-op style, tabs the `indent_size = unset` files). Match the surrounding 2-space style by hand and rely on `make lint` (EditorConfig) to verify. There is no `shfmt` make target. -- **Know your baseline.** `tests/unit/coverage_subshell_test.sh` fails on some macOS setups regardless of the change — confirm a failure is new (`git stash` + re-run) before treating it as a regression. +- **Know your baseline.** `tests/unit/coverage_subshell_test.sh` fails on some macOS setups regardless of the change, confirm a failure is new (`git stash` + re-run) before treating it as a regression. - Treat GitHub CI as the full quality gate; locally run focused tests during implementation, the full sequential + parallel suite once before commit. - Never split bundled changes into multiple PRs unless the issue explicitly demands it. diff --git a/.claude/skills/pr/SKILL.md b/.agnostic-ai/skills/pr/SKILL.md similarity index 69% rename from .claude/skills/pr/SKILL.md rename to .agnostic-ai/skills/pr/SKILL.md index 48b023bbb..343dd30aa 100644 --- a/.claude/skills/pr/SKILL.md +++ b/.agnostic-ai/skills/pr/SKILL.md @@ -3,28 +3,52 @@ name: pr description: Push branch and create a GitHub PR with concise, issue-linked description user-invocable: true argument-hint: "[#issue]" -allowed-tools: Bash, Read, Grep, Glob +allowed-tools: shell, read, Grep, Glob --- # Create Pull Request Push branch and create a PR with a concise, issue-linked description. -> **IMPORTANT:** This skill MUST be used for ALL pull request creation — even when the user says "create pr" without `/pr`. Never create a PR without following these steps. +> **IMPORTANT:** This skill MUST be used for ALL pull request creation, even when the user says "create pr" without `/pr`. Never create a PR without following these steps. ## Current Branch Context +::target claude - Branch: !`git branch --show-current` +::end + +::target codex +- Run Branch: `git branch --show-current` +::end +::target claude - Commits: !`git log main..HEAD --oneline 2>/dev/null` +::end + +::target codex +- Run Commits: `git log main..HEAD --oneline 2>/dev/null` +::end +::target claude - Changed files: !`git diff main..HEAD --stat 2>/dev/null` +::end + +::target codex +- Run Changed files: `git diff main..HEAD --stat 2>/dev/null` +::end ## Arguments +::target claude - `$ARGUMENTS` - Issue reference (optional, e.g., `#42` or `42`). If provided, the PR will be linked to this issue. +::end + +::target codex +- the argument in the user request - Issue reference (optional, e.g., `#42` or `42`). If provided, the PR will be linked to this issue. +::end ## Instructions -1. **Review the branch context above** — the commits and changed files are already loaded. +1. **Review the branch context above**, the commits and changed files are already loaded. -2. **MANDATORY: Update CHANGELOG.md** — Read `CHANGELOG.md` and check the `## Unreleased` section. If the changes from this branch are NOT already listed there, you MUST update it before proceeding. **Do NOT skip this step. Do NOT proceed to push without verifying.** +2. **MANDATORY: Update CHANGELOG.md**, Read `CHANGELOG.md` and check the `## Unreleased` section. If the changes from this branch are NOT already listed there, you MUST update it before proceeding. **Do NOT skip this step. Do NOT proceed to push without verifying.** - Add entries under the appropriate subsection (`### Added`, `### Changed`, `### Fixed`, etc.) - Reference issue numbers where applicable (e.g., `(Issue #123)`) - Commit the update: @@ -40,7 +64,13 @@ Push branch and create a PR with a concise, issue-linked description. - If push fails, read the output, fix the issue, commit the fix, and retry. Do NOT use `--no-verify` to bypass. 4. **Generate PR title**: +::target claude - If `$ARGUMENTS` contains an issue number, fetch the issue title: +::end + +::target codex + - If the argument in the user request contains an issue number, fetch the issue title: +::end ```bash gh issue view --json title -q '.title' ``` @@ -71,12 +101,12 @@ Push branch and create a PR with a concise, issue-linked description. **Assignee:** Always assign to `@me` (the PR creator). **Labels:** Add the single most relevant label based on the branch prefix and change context: - - `bug` — branch starts with `fix/` and addresses a defect - - `enhancement` — branch starts with `feat/` or adds new functionality - - `documentation` — branch starts with `docs/` or only changes docs - - `refactor` — code restructuring with no behavior change - - `ui` — visual/frontend-only changes - - `investigation` — spikes, research, or exploratory work + - `bug`, branch starts with `fix/` and addresses a defect + - `enhancement`, branch starts with `feat/` or adds new functionality + - `documentation`, branch starts with `docs/` or only changes docs + - `refactor`, code restructuring with no behavior change + - `ui`, visual/frontend-only changes + - `investigation`, spikes, research, or exploratory work **Body guidelines:** - **Background**: Link the issue with `Related #`, then 1-2 sentences of context. **NEVER use `Closes` or `Fixes`.** diff --git a/.claude/skills/pre-release/SKILL.md b/.agnostic-ai/skills/pre-release/SKILL.md similarity index 82% rename from .claude/skills/pre-release/SKILL.md rename to .agnostic-ai/skills/pre-release/SKILL.md index a425f0720..0f7fec76e 100644 --- a/.claude/skills/pre-release/SKILL.md +++ b/.agnostic-ai/skills/pre-release/SKILL.md @@ -1,7 +1,7 @@ --- name: pre-release description: Run comprehensive pre-release validation checklist -allowed-tools: Read, Bash, Grep, Glob +allowed-tools: read, shell, Grep, Glob --- # Pre-Release Validation @@ -29,15 +29,15 @@ All must pass. Run 3-5 times to catch flaky tests. ### 3. Static Analysis & Formatting ```bash -make sa # ShellCheck — zero warnings -make lint # EditorConfig — clean -# NB: no shfmt gate — it conflicts with .editorconfig here; make lint is the authority +make sa # ShellCheck, zero warnings +make lint # EditorConfig, clean +# NB: no shfmt gate, it conflicts with .editorconfig here; make lint is the authority ``` ### 4. Documentation -- `CHANGELOG.md` — version section complete with all changes -- `README.md` — examples current, no broken links +- `CHANGELOG.md`, version section complete with all changes +- `README.md`, examples current, no broken links - No remaining TODOs/FIXMEs in docs ### 5. Bash 3.0+ Compatibility diff --git a/.claude/skills/release/SKILL.md b/.agnostic-ai/skills/release/SKILL.md similarity index 78% rename from .claude/skills/release/SKILL.md rename to .agnostic-ai/skills/release/SKILL.md index 36c526bad..245218e7c 100644 --- a/.claude/skills/release/SKILL.md +++ b/.agnostic-ai/skills/release/SKILL.md @@ -3,7 +3,7 @@ name: release description: Run pre-release validation and execute the release process user-invocable: true argument-hint: "[version]" -allowed-tools: Bash, Read, Grep, Glob +allowed-tools: shell, read, Grep, Glob --- # Release @@ -11,14 +11,38 @@ allowed-tools: Bash, Read, Grep, Glob Thin reminder around `./release.sh`. The release script owns the whole end-to-end flow (version bumps, build, checksum, CHANGELOG, commit, signed tags, push, GitHub release, `latest` branch). Don't reimplement those steps -here — fix `release.sh` if something is missing. +here, fix `release.sh` if something is missing. ## Current State +::target claude - Current version: !`grep -o 'BASHUNIT_VERSION="[^"]*"' bashunit | cut -d'"' -f2` +::end + +::target codex +- Run Current version: `grep -o 'BASHUNIT_VERSION="[^"]*"' bashunit | cut -d'"' -f2` +::end +::target claude - Branch: !`git branch --show-current` +::end + +::target codex +- Run Branch: `git branch --show-current` +::end +::target claude - Working tree: !`git status --short` +::end + +::target codex +- Run Working tree: `git status --short` +::end +::target claude - Unreleased changes: !`awk '/^## Unreleased$/,/^## \[/' CHANGELOG.md | head -30` +::end + +::target codex +- Run Unreleased changes: `awk '/^## Unreleased$/,/^## \[/' CHANGELOG.md | head -30` +::end ## Steps @@ -34,7 +58,13 @@ Stop and report if anything fails. Don't release on a red main. ### 2. Pick the version +::target claude `$ARGUMENTS` overrides; otherwise the script auto-increments the minor. +::end + +::target codex +the argument in the user request overrides; otherwise the script auto-increments the minor. +::end Bump by the Unreleased section: a `### Added`/feat → minor, only `### Fixed` → patch. Confirm the version with the user before publishing. diff --git a/.claude/skills/review/SKILL.md b/.agnostic-ai/skills/review/SKILL.md similarity index 65% rename from .claude/skills/review/SKILL.md rename to .agnostic-ai/skills/review/SKILL.md index 9a77db4c3..ff1825ad1 100644 --- a/.claude/skills/review/SKILL.md +++ b/.agnostic-ai/skills/review/SKILL.md @@ -3,7 +3,7 @@ name: review description: Review a GitHub pull request; defaults to the current branch's PR when no number is given user-invocable: true argument-hint: "[#N]" -allowed-tools: Bash, Read, Grep, Glob +allowed-tools: shell, read, Grep, Glob --- # Review a Pull Request @@ -13,16 +13,40 @@ Review a GitHub PR. When no number is given, review the PR for the current branc > For reviewing your uncommitted working diff instead, use `/code-review`. ## Current Branch Context +::target claude - Branch: !`git branch --show-current` +::end + +::target codex +- Run Branch: `git branch --show-current` +::end +::target claude - PR for this branch: !`gh pr view --json number,title,state,url --jq '"#\(.number) \(.title) [\(.state)] \(.url)"' 2>/dev/null || echo "none"` +::end + +::target codex +- Run PR for this branch: `gh pr view --json number,title,state,url --jq '"#\(.number) \(.title) [\(.state)] \(.url)"' 2>/dev/null || echo "none"` +::end ## Arguments +::target claude - `$ARGUMENTS` - PR number to review (optional, e.g., `#42` or `42`). +::end + +::target codex +- the argument in the user request - PR number to review (optional, e.g., `#42` or `42`). +::end ## Instructions 1. **Resolve which PR to review:** +::target claude - If `$ARGUMENTS` contains a number, review that PR. +::end + +::target codex + - If the argument in the user request contains a number, review that PR. +::end - Otherwise, use the current branch's PR shown in the context above. - If neither yields a PR (no argument and the branch has no open PR), run `gh pr list` and ask the user which one to review. Do NOT guess. @@ -32,11 +56,11 @@ Review a GitHub PR. When no number is given, review the PR for the current branc gh pr diff ``` -3. **Review the diff** against this project's standards (see `.claude/CLAUDE.md` and `.claude/rules/`): +3. **Review the diff** against this project's standards (see `.agnostic-ai/AGNOSTIC_AI.md` and `.agnostic-ai/rules/`): - Bash 3.0+ compatibility (no `declare -A`, `[[ ]]`, `${var,,}`, negative indexing, `&>>`) - Tests exist and follow TDD; both success and failure paths covered - Naming, namespacing (`bashunit::*` / `_private`), and dynamic-scope safety - ShellCheck cleanliness and 2-space indentation (`make lint`, not `shfmt`) - CHANGELOG.md updated for user-facing changes -4. **Report findings** grouped by severity (blocker / suggestion / nit), each as `path:line — problem. fix.`. No praise, no scope creep. State plainly if the PR looks good. +4. **Report findings** grouped by severity (blocker / suggestion / nit), each as `path:line, problem. fix.`. No praise, no scope creep. State plainly if the PR looks good. diff --git a/.claude/skills/tdd-cycle/SKILL.md b/.agnostic-ai/skills/tdd-cycle/SKILL.md similarity index 62% rename from .claude/skills/tdd-cycle/SKILL.md rename to .agnostic-ai/skills/tdd-cycle/SKILL.md index 8f0a4b65f..cc12562e4 100644 --- a/.claude/skills/tdd-cycle/SKILL.md +++ b/.agnostic-ai/skills/tdd-cycle/SKILL.md @@ -1,7 +1,7 @@ --- name: tdd-cycle description: Run complete TDD red-green-refactor cycle for a test -allowed-tools: Read, Edit, Write, Bash, Grep, Glob +allowed-tools: read, edit, write, shell, Grep, Glob --- # TDD Cycle @@ -14,24 +14,24 @@ Execute a complete Red -> Green -> Refactor cycle. Check for `.tasks/YYYY-MM-DD-*.md`. If missing, create one before proceeding. -### 2. RED — Write Failing Test +### 2. RED, Write Failing Test 1. Show test inventory from task file, ask which test to implement 2. Study patterns from existing tests (unit -> `assert_test.sh`, doubles -> `doubles_test.sh`) 3. Write the failing test following Arrange-Act-Assert -4. Run: `./bashunit path/to/test.sh` — **must fail** +4. Run: `./bashunit path/to/test.sh`, **must fail** 5. Verify failure is for the RIGHT reason (not syntax error or wrong setup) 6. Update task file: current red bar + logbook entry -### 3. GREEN — Make It Pass +### 3. GREEN, Make It Pass -1. Write **minimal** code in `src/` — only enough for THIS test -2. Run: `./bashunit path/to/test.sh` — **must pass** +1. Write **minimal** code in `src/`, only enough for THIS test +2. Run: `./bashunit path/to/test.sh`, **must pass** 3. Update task file: check off test + logbook entry -### 4. REFACTOR — Improve Code +### 4. REFACTOR, Improve Code -1. Improve readability, naming, extract duplication — no behavior changes +1. Improve readability, naming, extract duplication, no behavior changes 2. Run tests after each change 3. Quality checks: `make sa && make lint` 4. Full suite: `./bashunit tests/` @@ -43,7 +43,7 @@ Show updated test inventory. Ask if continuing or done. ## Critical Rules -- **Never skip RED** — always verify test fails first -- **Minimal code in GREEN** — resist feature creep -- **All tests green during REFACTOR** — revert if broken +- **Never skip RED**, always verify test fails first +- **Minimal code in GREEN**, resist feature creep +- **All tests green during REFACTOR**, revert if broken - **Update task file** after each phase diff --git a/.claude/settings.json b/.claude/settings.json deleted file mode 100644 index 4e4daa43a..000000000 --- a/.claude/settings.json +++ /dev/null @@ -1,17 +0,0 @@ -{ - "$schema": "https://json.schemastore.org/claude-code-settings.json", - "includeCoAuthoredBy": false, - "hooks": { - "PostToolUse": [ - { - "matcher": "Edit|Write", - "hooks": [ - { - "type": "command", - "command": "file=\"$CLAUDE_FILE_PATH\"; if [ -n \"$file\" ] && echo \"$file\" | grep -q '\\.sh$'; then shellcheck -x \"$file\" 2>&1 | head -20; fi" - } - ] - } - ] - } -} diff --git a/.editorconfig b/.editorconfig index a7a0edca9..cb1993de5 100644 --- a/.editorconfig +++ b/.editorconfig @@ -29,7 +29,7 @@ indent_style = tab [{tests/acceptance/**.sh,src/console/header.sh,docs/command-line.md}] indent_size = unset -[.claude/**.md] +[{.claude/**.md,.agnostic-ai/**.md}] indent_size = unset # Vendored verbatim (Cobertura's coverage-04.dtd); not ours to reformat. diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 03aff3da4..228955fe5 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -53,6 +53,25 @@ make pre_commit/install make test && make sa && make lint ``` +### Coding Agent Setup (Optional) + +The project uses [agnostic-ai](https://agnostic-ai.org/) 0.82.0 for Claude Code and Codex. Install the pinned version, then generate the ignored native configuration from the repository root: + +```bash +curl -fsSL https://raw.githubusercontent.com/Chemaclass/agnostic-ai/v0.82.0/scripts/install.sh -o /tmp/bashunit-agnostic-ai-install.sh +AGNOSTIC_AI_VERSION=v0.82.0 bash /tmp/bashunit-agnostic-ai-install.sh +agnostic-ai sync +agnostic-ai sync --check +``` + +On Windows, use the [Windows installer](https://agnostic-ai.org/docs/installation/) with version `v0.82.0`. If the CLI is already installed, `agnostic-ai upgrade --version v0.82.0` installs the pinned release for a standalone binary; package-manager users pin through their package manager. + +Edit `.agnostic-ai/` or `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Generated files are ignored. Run sync after cloning and after pulling changes to the specs. Keep personal overrides in `.agnostic-ai/local/` or `agnostic-ai.local.yaml`, which are also ignored. + +The root instructions list the detailed rules to read for each task. Claude keeps its automatic path selectors and existing ShellCheck hook. Codex reads the same rule sources on demand to keep its initial context small. Skills keep Claude's automatic command snippets and provide explicit command instructions for Codex. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. + +The default model tier is `balanced`: Sonnet for Claude and `gpt-6.1-sol` at medium effort for Codex. `legolas` locates code on Haiku or `gpt-6-luna` at low effort, `aragorn` implements bounded changes on the balanced tier, and `gandalf` reviews on Opus or `gpt-6.1-sol` at high effort. These are task assignments, not speed or quality benchmarks. Codex permits at most two concurrent delegates. The coordinator runs the full gate once after all edits. Read-only agent prompts prohibit writes; Codex agents inherit the session sandbox, so these prompts do not impose a separate filesystem restriction. Override model tiers locally when a model is unavailable in your account; do not commit personal model or permission choices. + ### Documentation Setup (Optional) Docs live in their own workspace under `docs/` with an isolated `package.json`. diff --git a/.github/workflows/check-agent-specs.yml b/.github/workflows/check-agent-specs.yml new file mode 100644 index 000000000..aed127c21 --- /dev/null +++ b/.github/workflows/check-agent-specs.yml @@ -0,0 +1,42 @@ +name: Agent Specs + +on: + pull_request: + paths: + - '.agnostic-ai/**' + - 'agnostic-ai.yaml' + - '.gitignore' + - '.worktreeinclude' + - '.github/workflows/check-agent-specs.yml' + push: + branches: [main] + paths: + - '.agnostic-ai/**' + - 'agnostic-ai.yaml' + - '.gitignore' + - '.worktreeinclude' + - '.github/workflows/check-agent-specs.yml' + +permissions: + contents: read + +jobs: + validate: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install pinned agnostic-ai + env: + AGNOSTIC_AI_VERSION: v0.82.0 + AGNOSTIC_AI_INSTALL_DIR: ${{ runner.temp }}/agnostic-ai-bin + run: | + curl -fsSL https://raw.githubusercontent.com/Chemaclass/agnostic-ai/v0.82.0/scripts/install.sh -o "$RUNNER_TEMP/agnostic-ai-install.sh" + bash "$RUNNER_TEMP/agnostic-ai-install.sh" + echo "$AGNOSTIC_AI_INSTALL_DIR" >> "$GITHUB_PATH" + - name: Validate and generate both targets + run: | + agnostic-ai validate + agnostic-ai lint --strict + agnostic-ai sync + agnostic-ai sync --check + git diff --exit-code diff --git a/.gitignore b/.gitignore index f8fdfb16a..29c85d4a1 100644 --- a/.gitignore +++ b/.gitignore @@ -32,3 +32,26 @@ dev.log .claude/plans/ .claude/projects/ .claude/scheduled_tasks.lock + +# >>> agnostic-ai (managed) >>> +# Generated paths. Edit specs, not this block. Run `agnostic-ai sync` to refresh. +# Not committed: a fresh clone or `git worktree` lacks these until `agnostic-ai sync` runs (e.g. a post-checkout hook). +/.agents/skills/ +/.agnostic-ai/.command-lock +/.agnostic-ai/.sync-state +/.agnostic-ai/local/ +/.agnostic-ai/packs/ +/.claude/agent-memory-local/ +/.claude/agents/ +/.claude/rules/ +/.claude/scheduled_tasks.lock +/.claude/settings.json +/.claude/settings.local.json +/.claude/skills/ +/.claude/worktrees/ +/.codex/agents/ +/.codex/config.toml +/AGENTS.md +/CLAUDE.md +/agnostic-ai.local.yaml +# <<< agnostic-ai (managed) <<< diff --git a/.worktreeinclude b/.worktreeinclude new file mode 100644 index 000000000..7489f57a9 --- /dev/null +++ b/.worktreeinclude @@ -0,0 +1,18 @@ +# >>> agnostic-ai (managed) >>> +# Generated paths. Edit specs, not this block. Run `agnostic-ai sync` to refresh. +# Copied into each Claude Code worktree: the gitignored paths sync writes, and the local layer it reads. +/.agents/skills/ +/.agnostic-ai/local/ +/.agnostic-ai/packs/ +/.claude/agent-memory-local/ +/.claude/agents/ +/.claude/rules/ +/.claude/settings.json +/.claude/settings.local.json +/.claude/skills/ +/.codex/agents/ +/.codex/config.toml +/AGENTS.md +/CLAUDE.md +/agnostic-ai.local.yaml +# <<< agnostic-ai (managed) <<< diff --git a/CHANGELOG.md b/CHANGELOG.md index e749a887a..b0eed0061 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased ### Changed +- Contributor instructions use shared agnostic-ai specs for Claude Code and Codex, with task-specific context and model tiers - Sequential multi-file runs reuse provider and annotation metadata from header counting for unchanged files up to 512 characters (#1372) - Parallel reports collect worker results without per-row base64 processes, preserving values and completion order (#1371) - Data providers pass arguments without per-argument base64 processes, preserving quoting, empty values and parser isolation (#1370) diff --git a/agnostic-ai.yaml b/agnostic-ai.yaml new file mode 100644 index 000000000..2d1b7c9f4 --- /dev/null +++ b/agnostic-ai.yaml @@ -0,0 +1,19 @@ +# yaml-language-server: $schema=https://raw.githubusercontent.com/Chemaclass/agnostic-ai/v0.82.0/docs/schemas/config.schema.json +version: 1 +requires: "0.82.0" +builtins: [] + +targets: + - claude + - codex + +# Set to error to fail sync when a target cannot represent a spec. +on-unsupported: warn + +gitignore: + enabled: true + +models: + fast: {claude: haiku, codex: luna, effort: low} + balanced: {claude: sonnet, codex: sol, effort: medium} + strong: {claude: opus, codex: sol, effort: high} From af70ac0bf75142c5bfeec3d00e23903f8c95347a Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 13:29:14 +0200 Subject: [PATCH 2/5] refactor: make agent specs portable --- .agnostic-ai/AGNOSTIC_AI.md | 18 +- .../{aragorn.md => code-implementer.md} | 4 +- .../agents/{legolas.md => code-locator.md} | 6 +- .../agents/{gandalf.md => code-reviewer.md} | 6 +- .agnostic-ai/context/architecture-map.md | 156 +++++++++ .agnostic-ai/context/bash-style.md | 168 ++++++++++ .agnostic-ai/context/perf-fork-budget.md | 296 +++++++++++++++++ .agnostic-ai/context/tdd-workflow.md | 73 +++++ .agnostic-ai/context/testing.md | 130 ++++++++ .../hooks/posttooluse-edit-write-if.yaml | 6 - .agnostic-ai/hooks/shellcheck-after-edit.yaml | 5 + .agnostic-ai/rules/architecture-map.md | 158 +--------- .agnostic-ai/rules/bash-style.md | 170 +--------- .agnostic-ai/rules/perf-fork-budget.md | 298 +----------------- .agnostic-ai/rules/tdd-workflow.md | 75 +---- .agnostic-ai/rules/testing.md | 132 +------- .agnostic-ai/scripts/shellcheck-after-edit.sh | 17 + .agnostic-ai/skills/commit/SKILL.md | 52 +-- .agnostic-ai/skills/gh-issue/SKILL.md | 26 +- .agnostic-ai/skills/gh-issues/SKILL.md | 18 +- .agnostic-ai/skills/pr/SKILL.md | 54 +--- .agnostic-ai/skills/release/SKILL.md | 42 +-- .agnostic-ai/skills/review/SKILL.md | 36 +-- .github/CONTRIBUTING.md | 4 +- .github/workflows/check-agent-specs.yml | 24 ++ .gitignore | 3 + .worktreeinclude | 3 + 27 files changed, 944 insertions(+), 1036 deletions(-) rename .agnostic-ai/agents/{aragorn.md => code-implementer.md} (92%) rename .agnostic-ai/agents/{legolas.md => code-locator.md} (80%) rename .agnostic-ai/agents/{gandalf.md => code-reviewer.md} (89%) create mode 100644 .agnostic-ai/context/architecture-map.md create mode 100644 .agnostic-ai/context/bash-style.md create mode 100644 .agnostic-ai/context/perf-fork-budget.md create mode 100644 .agnostic-ai/context/tdd-workflow.md create mode 100644 .agnostic-ai/context/testing.md delete mode 100644 .agnostic-ai/hooks/posttooluse-edit-write-if.yaml create mode 100644 .agnostic-ai/hooks/shellcheck-after-edit.yaml create mode 100644 .agnostic-ai/scripts/shellcheck-after-edit.sh diff --git a/.agnostic-ai/AGNOSTIC_AI.md b/.agnostic-ai/AGNOSTIC_AI.md index f6e4d5b7f..af15838db 100644 --- a/.agnostic-ai/AGNOSTIC_AI.md +++ b/.agnostic-ai/AGNOSTIC_AI.md @@ -24,7 +24,7 @@ Works on macOS default bash (3.2) and older. **Prohibited features:** - `${array[-1]}` (negative indexing - Bash 4.3+) - `&>>` redirect (Bash 4.0+) -See `.agnostic-ai/rules/bash-style.md` for complete compatibility guide (auto-loaded when editing `src/` or `tests/`). +See `.agnostic-ai/context/bash-style.md` for the complete compatibility guide. ### Quality Standards @@ -72,19 +72,19 @@ Study existing tests before writing new ones: ## Task Context -All paths below are relative to the repository root, including from a generated skill or agent directory. Read only the rules relevant to the task before editing. Claude also loads them automatically through their path selectors. +All paths below are relative to the repository root, including from a generated skill or agent directory. Read only the guides relevant to the task before editing. Rules select the matching tasks and point to these guides. Follow the table even when the client cannot load path-scoped rules automatically. | Task | Read | |------|------| -| Changes to `src/` or `bashunit` | `.agnostic-ai/rules/architecture-map.md` | -| Shell changes under `src/` or `tests/` | `.agnostic-ai/rules/bash-style.md` | -| Changes to `src/` or fork census acceptance tests | `.agnostic-ai/rules/perf-fork-budget.md` | -| Shell development or task plans | `.agnostic-ai/rules/tdd-workflow.md` | -| Test changes | `.agnostic-ai/rules/testing.md` | +| Changes to `src/` or `bashunit` | `.agnostic-ai/context/architecture-map.md` | +| Shell changes under `src/` or `tests/` | `.agnostic-ai/context/bash-style.md` | +| Changes to `src/` or fork census acceptance tests | `.agnostic-ai/context/perf-fork-budget.md` | +| Shell development or task plans | `.agnostic-ai/context/tdd-workflow.md` | +| Test changes | `.agnostic-ai/context/testing.md` | ## Delegation -Keep one acceptance scenario and a short plan in the main session. Handle small edits directly. Delegate independent lookups to `legolas`, a bounded implementation to `aragorn`, and defect review to `gandalf` before pushing. Give each agent a specific question or file scope, a base revision, and a validation budget. Reuse its findings instead of repeating its searches. +Keep one acceptance scenario and a short plan in the main session. Handle small edits directly. Delegate independent lookups to `code-locator`, a bounded implementation to `code-implementer`, and defect review to `code-reviewer` before pushing. Give each agent a specific question or file scope, a base revision, and a validation budget. Reuse its findings instead of repeating its searches. Keep at most two delegates active. Workers share the checkout: assign separate files and never revert another worker's edits. Run the full quality gate once in the main session after the change set is complete. Delegates run only a targeted check when needed to prove a finding. Return concise evidence with file and line references; put longer reports under the ignored `.agnostic-ai/local/` directory. @@ -101,7 +101,7 @@ Edit `.agnostic-ai/` and `agnostic-ai.yaml`, then run `agnostic-ai validate`, `a - Change public API without docs/CHANGELOG - Commit without tests passing - Batch unrelated changes in one PR -- Create a PR without using the `/pr` skill +- Create a PR without using the `pr` skill ### Always: - Write tests before implementation diff --git a/.agnostic-ai/agents/aragorn.md b/.agnostic-ai/agents/code-implementer.md similarity index 92% rename from .agnostic-ai/agents/aragorn.md rename to .agnostic-ai/agents/code-implementer.md index fcfe37003..390c2cae3 100644 --- a/.agnostic-ai/agents/aragorn.md +++ b/.agnostic-ai/agents/code-implementer.md @@ -1,8 +1,8 @@ --- -name: aragorn +name: code-implementer description: Implement a bounded change in assigned files with Bash 3.0 compatibility and targeted validation. model: balanced -can: [read, Grep, Glob, shell, edit, write] +can: [read, shell, edit, write] --- You own only the files assigned by the main agent. Other workers share the checkout; preserve their edits and adapt to them. Read the applicable source rules listed in .agnostic-ai/AGNOSTIC_AI.md before editing. Follow the existing test patterns and the requested acceptance scenario. Use TDD for runtime changes. Keep hot paths within the fork budget. Run only the assigned targeted validation, never the full gate. Do not commit, push, publish, or install dependencies. Return changed files, the check result, and any unresolved limit. diff --git a/.agnostic-ai/agents/legolas.md b/.agnostic-ai/agents/code-locator.md similarity index 80% rename from .agnostic-ai/agents/legolas.md rename to .agnostic-ai/agents/code-locator.md index 4c94c5598..3e9d8f43c 100644 --- a/.agnostic-ai/agents/legolas.md +++ b/.agnostic-ai/agents/code-locator.md @@ -1,10 +1,8 @@ --- -name: legolas +name: code-locator description: Locate definitions, callers, and references before planning a nontrivial change. model: fast -readonly: true -x-codex: {sandbox_mode: null} -can: [read, Grep, Glob, shell] +can: [read, shell] --- Find code without editing it or proposing a redesign. Use rg, then read only enough surrounding code to verify each match. Return a file:line table and the searches used when nothing matches. Do not run tests, builds, installs, or Git mutations. diff --git a/.agnostic-ai/agents/gandalf.md b/.agnostic-ai/agents/code-reviewer.md similarity index 89% rename from .agnostic-ai/agents/gandalf.md rename to .agnostic-ai/agents/code-reviewer.md index 14bcc839a..7372fa315 100644 --- a/.agnostic-ai/agents/gandalf.md +++ b/.agnostic-ai/agents/code-reviewer.md @@ -1,10 +1,8 @@ --- -name: gandalf +name: code-reviewer description: Review a completed diff for verified defects before a push or merge. model: strong -readonly: true -x-codex: {sandbox_mode: null} -can: [read, Grep, Glob, shell] +can: [read, shell] --- Review without editing files, committing, or posting to GitHub. Read the diff against the supplied base and the applicable source rules listed in .agnostic-ai/AGNOSTIC_AI.md. Trace changed behavior through callers and existing tests. Check Bash 3.0 parsing, dynamic scope, subshell state, parallel isolation, cleanup, and hot-path process costs when relevant. For configuration changes, check clean-clone generation, canonical references, model resolution, and preserved native behavior. Do not run the full quality gate. Return only verified findings as file:line, severity, concrete failing input, and the smallest fix. If none remain, say so and list the scope checked. diff --git a/.agnostic-ai/context/architecture-map.md b/.agnostic-ai/context/architecture-map.md new file mode 100644 index 000000000..5b2673359 --- /dev/null +++ b/.agnostic-ai/context/architecture-map.md @@ -0,0 +1,156 @@ +# Architecture Map: modules and the life of a run + +Orientation for working in `src/`, which file owns what, and the exact call +flow of a test run. Line numbers drift; function names are the stable anchors. + +## The life of `./bashunit tests/` + +``` +bashunit (entry) sources all src/*.sh; version gate; early flag scan +└─ bashunit::main::cmd_test (main.sh: flag parsing, env exports) + └─ bashunit::runner::load_test_files (runner/discovery.sh: the per-file loop) + ├─ console_header::print_header "Running N tests", captures + │ └─ helper::find_total_tests $() SUBSHELL: sources each file in a + │ nested subshell just to count tests + ├─ source "$test_file" in the MAIN shell (workers inherit) + ├─ helper::check_duplicate_functions one awk pass per file + ├─ runner::functions_for_script pure bash: filter + sort by def line + ├─ helper::build_provider_map one awk pass per file (cached by path, + │ but the count subshell's build dies + │ with it → runs twice per file) + └─ runner::call_test_functions per-fn loop + └─ runner::run_test [--parallel: spawned as a & worker per test] + ├─ clock::now_to_slot duration start (if needed) + ├─ runner::execute_test_body $() subshell: the actual test fn + │ └─ state::export_subshell_context printf payload "##K=V##…" + ├─ runner::parse_result sync: parse counts from payload + │ parallel: + write .result via mktemp + └─ console_results::print_successful_test / _failed_ / … + └─ str::rpad + strip_ansi align per-test time (pure bash) + └─ [--parallel] wait; state::aggregate_parallel_results over *.result files + └─ console_results::render_result totals; deferred failure/skip blocks + └─ rerun::persist; env::cleanup_run_output_dir; exit code +``` + +Key inversion to remember: **tests run inside `$()` subshells; state comes back +as an encoded single-line payload** (`##ASSERTIONS_FAILED=…##TEST_OUTPUT=##`), +parsed by `runner::parse_result`/`state.sh`. Counters only exist in the main +shell (or, in parallel, in per-test `.result` files aggregated at the end). + +## Module ownership + +| Module | Owns | +|--------|------| +| `bashunit` | entry: version gate, early flag scan, sources every module, subcommand routing | +| `main/index.sh` | aggregator only, sources the `src/main/` module below | +| `main/validate.sh` | shared parser validation: unknown options, integer/path checks, shard parsing | +| `main/test.sh` / `main/bench.sh` / `main/subcommands.sh` | flag parsing per subcommand | +| `main/assert.sh` | the standalone `bashunit assert` path | +| `main/watch.sh` / `main/run.sh` | the `--watch` loop; the run lifecycle, exit codes and cleanup | +| `runner/index.sh` | aggregator only, sources the `src/runner/` module below | +| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes | +| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload | +| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers | +| `runner/result.sh` | `parse_result{,_sync,_parallel}`, failure source context, failed/skipped/incomplete/risky writers | +| `runner/parallel.sh` | job-slot waiting (`wait -n` or poll), running-job count, spinner | +| `runner/hooks.sh` | set_up/tear_down (test + script scope), hook failure records, mock clearing, EXIT cleanup | +| `runner/provider.sh` | `@data_provider` argument parsing | +| `runner/exec.sh` | `run_test`, the capture-subshell body, retry, timeout watchdog, per-file dispatch, `order_functions_for_script` (the run order a seed produces) | +| `runner/list.sh` | `--list`/`--dry-run`: emits selected tests instead of running them; hooks in after all filtering, before any hook | +| `runner/discovery.sh` | `load_test_files` (the per-file loop), `functions_for_script` | +| `runner/bench.sh` | benchmark file loop and bench function dispatch | +| `helper/index.sh` | aggregator only, sources the `src/helper/` module below | +| `helper/naming.sh` | resolving and normalising test-function and variable names | +| `helper/discovery.sh` | `find_files_recursive`, `find_total_tests`, fn filtering, duplicate check, line lookup | +| `helper/provider.sh` / `helper/tags.sh` | `@data_provider` map; `@tag` extraction and matching | +| `helper/encoding.sh` / `helper/functions.sh` / `helper/git.sh` | base64 + ids; generic fn utilities; remote tag lookup for `upgrade` | +| `state/index.sh` | aggregator only, sources the `src/state/` module below | +| `state/counters.sh` | test and assertion tallies and their get/add accessors | +| `state/duplicates.sh` | duplicate test-function detection state | +| `state/context.sh` | per-test output buffer, exit code, title, hook failure, and the per-test reset | +| `state/payload.sh` | the encoded per-test result payload and the base64 capability probe | +| `state/parallel.sh` | aggregating per-test result files after a `--parallel` run | +| `config/index.sh` | aggregator only, sources the `src/config/` module below | +| `config/env.sh` | all `BASHUNIT_*` defaults/config files, scratch dirs (`_BASHUNIT_RUN_OUTPUT_DIR` + EXIT-trap cleanup); executes at source time | +| `config/rerun.sh` | the `.bashunit/last-failed` cache for `--rerun-failed` | +| `config/parallel.sh` | is this run parallel; worker temp tree and stop-on-failure flag file (not `runner/parallel.sh`, which waits on job slots) | +| `console/index.sh` | aggregator only, sources the `src/console/` module below | +| `console/colors.sh` | the `_BASHUNIT_COLOR_*` palette and `bashunit::sgr` | +| `console/header.sh` | the "Running N tests" header | +| `console/line.sh` | `print_line`, the primitive every result line goes through, and its TAP variant | +| `console/duration.sh` / `console/diff.sh` | duration formatting; unified and line-by-line diffs under a failure | +| `console/test_line.sh` | the per-test result lines (passed/failed/skipped/incomplete/snapshot/risky/error) | +| `console/deferred.sh` | end-of-run blocks buffered during the run (scratch files under the run dir) | +| `console/summary.sh` | run totals, execution time, hook completion | +| `assert/index.sh` | aggregator only, sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | +| `assert/core.sh` | `assert::should_skip`, `assert::fail_with`, `assert::join_to_slot` and the comparison assertions the other files build on | +| `assert/{arrays,assertions,dates,duration,files,folders,json,once,snapshot}.sh` | the per-topic assertions; the per-assertion path must stay fork-free | +| `util/index.sh` | aggregator only, sources the `src/util/` module below | +| `util/clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads | +| `util/str.sh` / `util/math.sh` | pure-bash string and arithmetic helpers | +| `api/index.sh` | aggregator only, sources the `src/api/` module below | +| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone, the entrypoint already set it | +| `api/skip_todo.sh` / `api/test_title.sh` | `skip`/`todo` markers; `set_test_title` | +| `api/bashunit.sh` | the custom-assert facade (`assert_that`, `assertion_failed`/`passed`) | +| `doubles/index.sh` | aggregator only, sources the `src/doubles/` module below | +| `doubles/mock.sh` | `mock`/`unmock` and the `_BASHUNIT_MOCKED_FUNCTIONS` registry `runner/hooks.sh` unwinds per test | +| `doubles/spy.sh` | `spy` and its `_BASHUNIT_SPY_*` state slots + files | +| `doubles/assertions.sh` | the `assert_have_been_called*` family | +| `coverage/index.sh` | aggregator only, sources the `src/coverage/` modules below | +| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | +| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | +| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | +| `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | +| `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | +| `coverage/functions.sh` | function definitions and their line spans | +| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose**, the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | +| `coverage/report_text.sh` / `report_lcov.sh` / `report_html.sh` | the three renderers | +| `coverage/html_index.sh` / `html_file.sh` | HTML page emitters; the only two files exempt from `max_line_length` | + +| `reports.sh` | JUnit/HTML/TAP/JSON writers | +| `system/index.sh` | aggregator only, sources the `src/system/` module below | +| `system/check_os.sh` / `system/dependencies.sh` | one-fork OS detect; `command -v` probes (builtins, not forks) | +| `system/io.sh` | small I/O helpers | +| `cli/index.sh` | aggregator only, sources the `src/cli/` module below | +| `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; `main.sh` is their only caller | +| `benchmark/index.sh` | the remaining non-`test` subcommand; shared with `runner/bench.sh` | + +## Cross-cutting invariants + +- **Bash 3.0 floor** (`.agnostic-ai/rules/bash-style.md`): no `[[`, `declare -A`, + `${var,,}`, `BASHPID`, negative indices. Subshells share `$$`, you cannot + make a per-worker unique token without a fork (`mktemp`) or an ordinal + assigned before the fork. `RANDOM` is no help either: whether a subshell + reseeds it depends on the platform and the nesting depth (measured in + perf-fork-budget.md), so it is unreliable in both directions (#1354). +- **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` + captures on hot paths, bash-style.md documents it; `local` is dynamically + scoped, so helpers must not write caller-named variables. +- **Fork budget** (`.agnostic-ai/rules/perf-fork-budget.md`): per-test paths are + fork-free (sequential) or mktemp-only (parallel); budgets are enforced by + `tests/acceptance/bashunit_*forks*_test.sh` on three platforms, a "harmless" + `echo | sed` in a per-test path will fail CI. +- **`call_test_functions` is a background subshell under `--parallel`** + (`discovery.sh` backgrounds it per file). Anything it sets in shell state, + counters, flags, error records, dies with the worker. Only two things cross + back: per-test `.result` payloads, and files written to disk. This produced + two silent-green bugs in one day: a data-provider error printed while the run + exited 0 (#1145), and duplicate-test-function detection being entirely off in + the mode CI uses (#1147). Before adding a check anywhere under that call, ask + which side of the fork it belongs on, a whole-file check almost always + belongs in the parent loop, before dispatch. + **Verify both modes.** Sequential and `--parallel` disagreeing is the + signature of this bug, and it is invisible to a sequential-only test. +- **Binaries pinned at startup** (`$GREP`, `$MKTEMP`, `$CAT` in env.sh) so test + doubles/PATH games can't hijack the framework's own plumbing. +- **Snapshots assume 80-col non-tty width** (`tput cols` fallback); anything + that changes rendering widths breaks `tests/acceptance/snapshots/`. +- **The build flattens the source graph in DFS order** (`build.sh` + `build::process_file`): a file's body is emitted, *then* its `source` lines are + recursed into. That equals dev-mode order only if a module aggregator + (`src//index.sh`, ADR-010) contains **nothing but `source` + lines and comments**, any other top-level statement would run before its + dependencies in the built artifact but after them in dev mode. Files are + deduped by repo-relative path, so `src/` may hold module dirs and two files may + share a basename. Every src file's **first line must be the shebang**: + `tail -n +2` strips it when embedding. diff --git a/.agnostic-ai/context/bash-style.md b/.agnostic-ai/context/bash-style.md new file mode 100644 index 000000000..054e58e63 --- /dev/null +++ b/.agnostic-ai/context/bash-style.md @@ -0,0 +1,168 @@ +# Bash Style & Compatibility Rules + +## Bash 3.0+ Compatibility (Critical) + +bashunit must work on **Bash 3.0+**. The floor was deliberately lowered from 3.2 +to 3.0 in 0.33 to reach older systems; macOS's 3.2 is the common case, not the +floor. The runtime gate in `./bashunit` compares major **and** minor against +`BASHUNIT_MIN_BASH_VERSION`, so the declared floor is the enforced one. + +These are **version-breaking** above the floor and are prohibited. They are +enforced mechanically by `tests/unit/bash_compatibility_test.sh`, which greps +`src/` regardless of test coverage: + +| Feature | Bash ver | Alternative | +|---------|----------|-------------| +| `printf -v` | 3.1+ | Return-slot pattern (below) | +| `x+=y` (string append) | 3.1+ | `var="$var$more"` | +| `arr+=(x)` (array append) | 3.1+, and a 3.0 **parse** error | `arr[${#arr[@]}]=x` | +| `declare -A` (associative arrays) | 4.0+ | Parallel indexed arrays | +| `${var,,}` / `${var^^}` (case) | 4.0+ | `tr '[:upper:]' '[:lower:]'` | +| `${array[-1]}` (negative index) | 4.3+ | `${array[${#array[@]}-1]}` | +| `&>>` (append both) | 4.0+ | `>> file 2>&1` | +| `BASHPID` | 4.0+ | fork (`mktemp`) or an assigned ordinal | +| `mapfile` / `readarray` | 4.0+ | `while IFS= read -r` | +| `declare -n` / `local -n` (nameref) | 4.3+ | Return-slot pattern (below) | +| `coproc` | 4.0+ |, | +| `${var@Q}` (transformations) | 4.4+ |, | + +`[[ =~ ]]` is a subtler case: it **exists** on 3.0 (verified against a real +build), but 3.2 changed whether a quoted right-hand side is a regex or a +literal. At a 3.0 floor the same match therefore behaves differently across +supported versions, so regex matching goes through `grep -E`, a real +constraint, not a style preference. See `.agnostic-ai/rules/perf-fork-budget.md`. + +### Style, not compatibility + +Prefer `[ ]` over `[[ ]]` for ordinary tests. This is **house style**: `[[ ]]` +itself works on every supported bash, including 3.0. Do not justify it as a +Bash 3 requirement, and do not add it to the compatibility gate. + +## Coding Conventions + +- **2 spaces** indent, no tabs, enforced by `make lint` (EditorConfig). Do **not** + run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses + `\`-continued strings past the 120-char limit, breaking `make lint` +- **120 chars** max line length (soft) +- Follow [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html) +- Always quote variables unless explicit word splitting is needed +- Use `$()` for command substitution, never backticks + +### Naming + +- **Public functions:** `bashunit::function_name` +- **Private functions:** `_function_name` (leading underscore) +- **Local variables:** `lowercase_with_underscores` +- **Globals/exports:** `UPPERCASE_WITH_UNDERSCORES` + +### Function Docs (public functions) + +```bash +## +# Brief description +# Arguments: $1 - desc, $2 - desc (optional, default: "x") +# Returns: 0 success, 1 failure +## +``` + +### File Structure + +Constants -> Globals -> Private functions -> Public functions + +Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` + +## Returning values from a helper (dynamic-scope safety) + +Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee +shadows the caller's same-named variable for the duration of the call. Same trap fires +with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...` +writes, they all resolve against the dynamic scope. + +We need **all three** properties on the hot path: + +1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork + per call, which dominates per-test cost. +2. **No collision with caller locals.** A helper that silently overwrites or reads + the wrong variable is the worst kind of bug, no error, just stale data. +3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and + it has the same dynamic-scope shadowing as `eval "$name=..."` anyway). + +### Preferred: dedicated global return slot + +```bash +_BASHUNIT_PKG_THING_OUT="" + +# Writes the result into _BASHUNIT_PKG_THING_OUT. +# Arguments: $1 input +function bashunit::pkg::do_thing() { + local input=$1 # natural name, no prefix needed + local val="${input%%]*}" + val="${val#[}" + _BASHUNIT_PKG_THING_OUT=$val +} + +# Caller +bashunit::pkg::do_thing "$payload" +local thing=$_BASHUNIT_PKG_THING_OUT +``` + +- Zero forks per call. +- Helper has natural local names; no `__bu_` noise. +- Caller cannot accidentally shadow the slot because the `_BASHUNIT_*` namespace + is reserved for the framework. +- A dedicated slot per helper (rather than one shared `_BASHUNIT_OUT`) means + adjacent or nested calls can't clobber each other. Cheap: globals are free. + +Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`, +`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`), +`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`). + +### When the helper builds dynamic variable names (mock/spy state) + +Use a clear namespace prefix on the **constructed** name, not on the helper's locals: + +```bash +# Spy state lives in _BASHUNIT_SPY_${variable}_TIMES_FILE, not ${variable}_times_file. +# A caller doing `local foo_times_file=...` is therefore harmless: the helper +# resolves a different global. +export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file" +``` + +Example in tree: `src/test_doubles.sh` (`_BASHUNIT_SPY_*`). + +### Last-resort fallback: outvar by name + `__bu_` prefix on locals + +Only use this when neither a fixed return slot nor a namespaced constructed name is +practical (e.g. a generic helper called from many call sites with different output +variables and no shared global is appropriate): + +```bash +function bashunit::pkg::do_thing() { + local __bu_out=$1 + local __bu_in=$2 + local __bu_val="${__bu_in%%]*}" + eval "$__bu_out=\$__bu_val" +} +``` + +- All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing + silently breaks the caller's outvar (see PR #672, that exact bug caused 12 parallel + test failures in #662). +- Include a regression test that calls the helper with one of its own documented + internal names as the outvar. + +### Intentional dynamic-scope mutation is a separate pattern + +The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends) +deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). +That is documented inline above the helpers in that file and is **not** the outvar pattern, +the helper has no `$1`-named outvar argument; the caller agrees to share state by convention. +It is also why those helpers and `extract_branches` must stay in one file (ADR-010). +Don't introduce new instances of this pattern without an inline justification comment. + +## ShellCheck + +All code must pass `make sa`. Use directives sparingly with reason: +```bash +# shellcheck disable=SC2034 # Variable used by caller +``` diff --git a/.agnostic-ai/context/perf-fork-budget.md b/.agnostic-ai/context/perf-fork-budget.md new file mode 100644 index 000000000..f6c81d2b1 --- /dev/null +++ b/.agnostic-ai/context/perf-fork-budget.md @@ -0,0 +1,296 @@ +# Performance: Fork Budget & Census Method + +bashunit's dominant runtime cost on Bash 3.2 (macOS) is **process forks**, not +shell execution. A fork costs ~1-3ms; the acceptance suite spawns ~258 nested +`./bashunit` runs, so one avoidable fork per file or per test multiplies fast. +PRs #801-#817 removed the per-test, per-file and cold-start forks (the full +`--parallel` acceptance suite went from ~61s to ~17s wall, CPU time halved); +this file records how to measure and the traps found, so future work starts +from evidence. + +## Measuring: shim census is ground truth, traces overcount + +**PATH-shim census (authoritative).** Wrap a binary, count invocations: + +```bash +real_awk="$(command -v awk)" # resolve BEFORE prepending the shim dir +printf '#!/usr/bin/env bash\necho x >> /tmp/count\nexec "%s" "$@"\n' "$real_awk" > shim/awk +chmod +x shim/awk +PATH="shim:$PATH" ./bashunit --no-parallel fixture_test.sh +grep -c . /tmp/count +``` + +Caveats: binaries pinned at startup via `command -v` (`$GREP`, `$MKTEMP`, +`$CAT`) bypass PATH shims, trace those instead; and shims are unreliable on +Git Bash (skip such tests on Windows). + +**A shim census only sees forks that exec a binary.** `$(some_shell_function)` +forks a subshell and execs nothing, so it is invisible to every shim and to the +`^\++ +/path/to/binary` trace patterns above, yet it costs the same ~0.6ms. +That blind spot hid sixteen of them in the colour palette (`$(bashunit::sgr N)` +per colour, ~9ms, the largest single cost in a cold start) straight through the +#801-#851 campaign that pinned everything else on this page. To count them, +match the *function name* in the trace (`^\++ +bashunit::fn( |$)`), or profile +by injecting `$EPOCHREALTIME` echoes at the `# src/` markers the build +emits, that attributes startup cost per source file and is how this one +surfaced. + +**`bash -x` trace census (cheap but inflated).** `PS4='+ ' bash -x ./bashunit …` +also counts trace lines **re-echoed inside captured test output**, so it can +overcount 10-20x (one real `grep` appeared 24 times). Use it to *locate* fork +sites (`PS4='+X ${BASH_SOURCE##*/}:$LINENO> '`), never to *count* them. It also +misses forks inside `--parallel` workers. Anchor real execs with +`grep -cE '^\++ +/[^ ]*mktemp$'`-style patterns (absolute path, exact argv +shape) to skip `command -v` probes and variable assignments. + +**Regression tests.** Budgets are enforced in +`tests/acceptance/bashunit_coldstart_forks_test.sh` (cold start: mktemp/perl/ +grep/mkdir) and `tests/acceptance/bashunit_run_forks_test.sh` (run path: +grep/cat/sed/sort/awk budget, run-dir cleanup). Extend these when you remove a +fork class; they are the RED test of the TDD cycle. + +## Replacement patterns that worked + +| Fork | Replacement | Example | +|------|-------------|---------| +| `echo x \| grep -c fixed` | `case` glob match | #803, #804 | +| `cat <>` creation | #801, #811 | +| probe fork + first-use fork | probe does the real work and seeds the return slot | #802 (clock perl) | +| per-worker `mkdir -p` | parent pre-creates before spawning workers (`[ -d ]` inside a worker races its siblings) | #813 | +| sanitize-args pipeline on empty input | guard: skip the pipeline when the input is empty | #813 | +| `printf \| grep -cE` per line as a classifier fallback | `case` globs translating each regex alternation | #1005 (286 lines cost 1055 forks) | + +`shopt -s extdebug` for `declare -F`: enable it **inside the capture subshell +only**, toggling it in the caller's shell clobbers caller state (#808). + +## Where pure bash LOSES, measured, do not "optimize" these + +- **`… | awk` over ≳100 lines**: a `while read` loop was 5x slower than the awk + fork (19ms vs 3.5ms for 600 `declare -F` lines). File scans (provider map, + duplicate check) stay awk. +- **`${var//pattern/}` on large strings**: quadratic on Bash 3.2, 2.7 s where + awk takes 3.5ms. Never string-replace over big captures. +- **Regex assertions** (`assert_matches`): the `grep -E` fork is **not** because + `[[ =~ ]]` is missing, it was verified working on a real Bash 3.0 build. It is + because 3.2 changed whether a quoted right-hand side is a regex or a literal, + so at the 3.0 floor the same pattern would match differently across supported + versions. Removing the fork means picking one semantic and breaking the other. + Measured cost of keeping it, so the trade is at least quantified: 2000 + `assert_same` run in ~130 ms (0.065 ms each), 500 `assert_matches` in ~1.25 s + (**2.5 ms each, ~38x**). That is the fork, and it is the price of the 3.0 + floor -- not a regression to chase. +- **Single-file build artifact**: sourcing `bin/bashunit` is *not* faster than + sourcing `src/*.sh` (parse time dominates, file opens don't). +- **`tput cols` at startup**: returns 80 on non-tty; snapshots depend on that + width. Not removable. + +## Estimating: three ways an estimate lied, all in one day + +The coverage work in #1084-#1112 produced three wrong predictions, each of +which cost a full implement-and-measure cycle. Check against these before +believing a number. + +- **A micro-benchmark of a function the DEBUG trap calls overstates the win.** + A same-file memo for `record_line`'s path lookups predicted 5x from a loop + benchmark and delivered **4%** in situ (#1102). Whatever the trap costs + around the call swamps what the call itself costs. +- **Stubbing tells you what a stage costs, not what removing a syscall from it + saves.** Returning early from `record_line` attributed 705 ms to its two + `>>` appends; holding the files open on persistent descriptors recovered + **184 ms** of it, not 705 (#1110). +- **A benchmark path must be the path that occurs.** `${file//[^a-zA-Z0-9]/_}` + costs 7.8 µs at 7 characters, 131 µs at 59 and 549 µs at 121, so measuring + it with an absolute path when the trap actually sees `./src/x.sh` inflated + the estimate by 4x (#1102). + +**A/B in the same tree.** Comparing a worktree under `/private/tmp` against the +repo under `/Users` made a 70 ms *improvement* read as a 40 ms regression. +Check out the two versions of the file into one tree and alternate rounds. + +## Process substitution as an argument fails under load + +`cmd <(a) <(b)` passes `/dev/fd/N` paths that **`cmd` itself must open**. Under +a loaded parallel suite that open fails: + +``` +diff: /dev/fd/62: Bad file descriptor +``` + +`diff` then produces no comparison and writes that to stderr, which a caller +collecting output reads as a real difference. Both coverage differentials did +this once per repo file, 473 iterations, and failed roughly 1 full suite run +in 3, each time reporting "the classifiers disagree" over a file that agreed +perfectly (#1152). Two earlier attempts blamed `awk`; the `AWK-FAILED` marker +from #1143 is what finally ruled it out. + +The redirect form `while read …; done < <(cmd)` is **not** the same shape: the +shell opens the descriptor and dups it to stdin, so no external process opens +`/dev/fd/N`. It is used throughout `src/` and has never been seen to fail. + +So: never put `<(...)` in an external command's **argument list** inside a loop +that runs per file or per test. Write real files (concurrently, so the two +sides still overlap, serialising them was measurably slower) and compare with +`[ "$a" != "$b" ]`, which the shell does without a fork. + +## Where pure bash LOSES on some Bash versions but not others + +- **`${var//&/…}` is not portable.** Bash 5.2 made a bare `&` in a substitution + REPLACEMENT mean "the matched text", so `${line//` marker technique: 41.6ms of executed top-level code, +against 10ms to parse the whole 600KB artifact and define all 915 functions. +Parsing is cheap; what runs at source time is not. The current shape is +`config/env.sh` ~12.6ms (config files, `tput`, `mkdir`), `state/payload.sh` +~2.9ms (the `base64 --help` probe) and `system/check_os.sh` ~2.4ms (`uname`). +Lazy-loading whole modules was still rejected in #798. + +Measuring a change this small needs ~200 invocations per sample: the two forks +are ~4ms against a ~65ms startup, and single runs vary by ±10ms. The acceptance +suite is useless for it, consecutive runs there differ by 5s. +`--coverage` adds ~3 forks per unique file first seen by the DEBUG trap +(decision-cache miss: grep|head + dirname), bounded by file count, nightly +non-gating workflow, not worth chasing. + +**Don't assume the engine is the cost.** Profiling `--coverage` for #1005 found +the *report* phase was ~50-60% of wall time and identical for both engines: the +per-line executable/non-executable classifier fell back to a `grep -E` fork for +every line it could not decide in pure Bash, and every tracked line is classified +twice per run (`precompute_file_stats`, then `report_lcov`). Budget guarded by +`tests/acceptance/bashunit_coverage_forks_test.sh`, expressed as "fork count does +not grow with source-line count" rather than an absolute number. + +Two traps when replacing a regex classifier with `case` globs, both silently +change coverage numbers rather than erroring: + +- Inside a POSIX bracket expression a backslash is a **literal member of the + set**, so `[^\)]` excludes `\` as well as `)`. `x=$(foo)` and + `x=$(printf '%s\n')` therefore classify differently. +- `[\{\}]` likewise matches a lone `\`, so a bare line continuation counts as a + brace-only line. + +Verify such a rewrite by running both implementations over the same input +(`git show HEAD:` + `declare -f | sed` to alias the old one) across every +`git ls-files '*.sh'` line, and mutation-test the harness itself, a differential +that cannot fail proves nothing. + +**Parallel 10-test file run (CI's mode):** ~11 forks, 3 `mkdir`, 4 `rm`, +3 `awk` (#813; was 61). That count was taken with a sequential census fixture +and so missed the one cost that only exists in this mode: every worker used to +re-probe the clock, because the probe resolved the implementation inside a +`$( )` and the resolved value died with that subshell. On a shell without +`EPOCHREALTIME` the probe forks `perl`, so it scaled one-for-one with the tests +, 502 execs for a 500-test file against 2, and it fired even with per-test +timing off, since deciding that timing is off is what asks whether the clock is +expensive (#1353). **Measure the parallel budget with a parallel fixture**: a +per-worker cost is invisible to a sequential census by construction. The per-test result file is named by a per-suite +ordinal the single-threaded dispatcher assigns just before each `&` (the fork +inherits it), so it costs **no** `mktemp` + `mv` per test (#851; was 10 +`mktemp` + 10 `mv`). This replaced the old sanitized-test-name scheme, whose +deterministic names could collide (different provider args sanitize identically) +because Bash 3 workers can't mint a unique token, subshells inherit `$$`, and +`BASHPID` is 4.0+; an ordinal sidesteps that entirely. + +This file used to add "and the `RANDOM` state" to that list. Measured, `RANDOM` +is neither reliably shared nor reliably reseeded (#1354): + +| Context | three consecutive `$( )` reads | +|---|---| +| Plain shell, 3.00.22, 3.2.57, 4.4, 5.2, 5.3 | differ | +| `--parallel` worker, Linux 3.0, 5.2 | differ | +| `--parallel` worker, macOS 3.2.57 | **identical** | + +So it depends on the platform *and* on how deeply nested the subshell is. Do +not build on it in either direction, and do not reuse "`RANDOM` is shared" as a +premise, it is right about the conclusion for the wrong reason. `$$` is the +part that genuinely is inherited. +`wait_for_job_slot` already uses `wait -n` on Bash 4.3+ and an adaptive +sleep-poll fallback, don't "fix" it. The spinner forks `sleep` ~1/s on +non-tty; not worth chasing. + +**Reports are on the per-test path too.** Any `--log-*`/`--report-*` flag makes +each worker spool a result row, and that row used to base64 each of its nine +fields on its own, 14 `base64` forks per test counting the parent's decode, +with a `tr` on each encode. `--log-junit` therefore cost more than the run it +reported on (300 tests: 1.2s → 9.2s wall, 5.8s → 27.4s CPU). Now only the four +fields that can hold arbitrary text are encoded and the row is joined with +0x1F. The main workflow passes no report flag, so CI never showed this; the +budget is guarded by `bashunit_run_forks_test.sh`. When adding a field to that +row, ask whether it can hold arbitrary text before reaching for base64. diff --git a/.agnostic-ai/context/tdd-workflow.md b/.agnostic-ai/context/tdd-workflow.md new file mode 100644 index 000000000..059b6206b --- /dev/null +++ b/.agnostic-ai/context/tdd-workflow.md @@ -0,0 +1,73 @@ +# TDD Workflow + +**Test-Driven Development is mandatory** for all bashunit development. + +## The Cycle + +``` +RED -> Write a failing test (must fail for the RIGHT reason) +GREEN -> Write minimal code to make it pass (nothing extra) +REFACTOR -> Improve code while keeping all tests green +REPEAT -> Until acceptance criteria are met +``` + +## Task File (Recommended for non-trivial changes) + +For features, refactors, or multi-test work, create `.tasks/YYYY-MM-DD-slug.md`. +Skip for small bug fixes or single-test changes. + +```markdown +# [Feature/Fix Name] + +**Date:** YYYY-MM-DD **Status:** In Progress + +## Acceptance Criteria +- [ ] Criterion 1 + +## Test Inventory +- [ ] `test_should_handle_valid_input` +- [ ] `test_should_fail_with_invalid_input` + +## Current Red Bar +Test: (none yet) + +## Logbook +### YYYY-MM-DD HH:MM +- Created task, analyzed existing code +``` + +## RED Phase + +1. Pick the **smallest next test** from inventory +2. Study patterns from existing tests (unit -> `assert_test.sh`, doubles -> `doubles_test.sh`) +3. Write test following Arrange-Act-Assert +4. Run: `./bashunit path/to/test.sh`, **must fail** +5. Verify failure reason: function missing or wrong output (**not** syntax error) + +## GREEN Phase + +1. Write **minimal** code to pass, no extra features, no premature optimization +2. Run: `./bashunit path/to/test.sh`, **must pass** + +## REFACTOR Phase + +1. Improve readability, naming, extract duplication, **no behavior changes** +2. Run tests after each change +3. After the change set is complete, run quality checks once: `make sa && make lint` (never `shfmt -w`, it breaks `make lint`) + +## Quality Gate (Before Commit) + +```bash +./bashunit tests/ # All tests +./bashunit --parallel tests/ # Parallel (isolation check) +make sa && make lint # Static analysis + linting (lint is the format gate) +``` + +## Definition of Done + +- All tests green for the **right reason** +- All acceptance criteria met +- Quality gate passes +- Bash 3.0+ compatible +- CHANGELOG updated (if user-facing) +- ADR created (if architectural decision) diff --git a/.agnostic-ai/context/testing.md b/.agnostic-ai/context/testing.md new file mode 100644 index 000000000..f5ef6d91e --- /dev/null +++ b/.agnostic-ai/context/testing.md @@ -0,0 +1,130 @@ +# Testing Guidelines + +## Organization + +| Directory | Purpose | Pattern | +|-----------|---------|---------| +| `tests/unit/` | Isolated function tests | Mocks/spies for deps | +| `tests/functional/` | Multi-component integration | Real interactions | +| `tests/acceptance/` | CLI/end-to-end workflows | Full user scenarios | + +**Naming:** Files end with `_test.sh`. Functions: `test_should__when_` + +## Assertions + +```bash +assert_equals "expected" "$actual" +assert_not_equals "not_this" "$actual" +assert_contains "substring" "$haystack" +assert_not_contains "substring" "$haystack" +assert_matches "regex" "$string" +assert_not_matches "regex" "$string" +assert_empty "$var" +assert_not_empty "$var" +assert_successful_code "$?" +assert_general_error "$?" +assert_file_exists "$path" +assert_file_not_exists "$path" +assert_directory_exists "$path" +assert_array_contains "value" "${array[@]}" +assert_array_not_contains "value" "${array[@]}" +``` + +There is no `assert_fails`. To assert that an assertion *fails*, capture its output and +compare it with the renderer: + +```bash +assert_same "$(bashunit::console_results::print_failed_test "My test" "a" "but got " "b")" \ + "$(assert_same "a" "b")" +``` + +## Test Doubles + +Helpers are namespaced (`bashunit::*`); assertions are bare. The unprefixed helper name +is a `command not found`, not an alias. + +```bash +# Spies - track calls without changing behavior +bashunit::spy function_name +assert_have_been_called function_name +assert_have_been_called_times 2 function_name # count first, then spy +assert_have_been_called_with function_name "arg" # spy first, then expected +assert_have_been_called_with function_name "arg" 1 # ...of call #1 +assert_have_been_called_nth_with 1 function_name "arg" +assert_have_been_called_with_args function_name "a b" # boundary-exact, no call index + +# Mocks - replace behavior +bashunit::mock date echo "2024-05-01" +bashunit::mock uname <<< "Linux" # heredoc form ignores the call's arguments +``` + +Note `_times` takes the count first while `_with` takes the spy first, a swapped pair +fails the assertion rather than erroring, so it reads like a real defect. + +## Data Providers + +```bash +function data_provider_inputs() { + echo "5 3 8" + echo "0 0 0" +} + +# @data_provider data_provider_inputs +function test_should_add() { + assert_equals "$3" "$(add "$1" "$2")" +} +``` + +## Lifecycle Hooks + +- `set_up_before_script()`, once before all tests in file +- `set_up()`, before each test +- `tear_down()`, after each test +- `tear_down_after_script()`, once after all tests in file + +## Snapshot Testing + +```bash +assert_match_snapshot "$output" +assert_match_named_snapshot "stderr" "$stderr" # multiple snapshots in one test +# Re-record deliberately: ./bashunit --snapshot-update --filter "test name" tests/ +``` + +## Writing a test that generates test files + +The duplicate-test-function check scans a file **textually**, so a fixture body +embedded in a string counts as a definition of the *enclosing* file: + +```bash +function test_one() { + run_fixture 'function test_x() { assert_same 1 1; }' # ← counted +} +function test_two() { + run_fixture 'function test_x() { assert_same 2 2; }' # ← counted: duplicate +} +``` + +That fails the whole run before any test executes, and the message names a +function this file never defines. Give every embedded fixture function a name +unique across the file. The report includes the line of each definition +(`test_x (lines 30, 40)`), which is what identifies the strings. + +It cannot be fixed by tracking quote state, an apostrophe in any comment +(`# doesn't work`) would flip it and mask real duplicates. + +## Test Isolation + +- Use `$(bashunit::temp_file)` / `$(bashunit::temp_dir)` (auto-cleaned) for file + operations, these are functions, not variables +- No shared global state between tests +- No network calls, mock external commands +- No time dependencies, mock `date` if needed +- Tests must be safe for `./bashunit --parallel tests/` + +## Reference Tests + +- **Assertions:** `tests/unit/assert_test.sh` +- **Doubles:** `tests/functional/doubles_test.sh` +- **Providers:** `tests/functional/provider_test.sh` +- **Hooks:** `tests/unit/setup_teardown_test.sh` +- **CLI/snapshots:** `tests/acceptance/bashunit_test.sh` diff --git a/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml b/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml deleted file mode 100644 index 533f5b53a..000000000 --- a/.agnostic-ai/hooks/posttooluse-edit-write-if.yaml +++ /dev/null @@ -1,6 +0,0 @@ -target: claude -command: file="$CLAUDE_FILE_PATH"; if [ -n "$file" ] && echo "$file" | grep -q '\.sh$'; then shellcheck -x "$file" 2>&1 | head -20; fi -description: Runs `file="$CLAUDE_FILE_PATH"; if [ -n "$file" ] && echo "$file" ...` on PostToolUse for Edit|Write. -event: PostToolUse -matcher: Edit|Write -name: posttooluse-edit-write-if diff --git a/.agnostic-ai/hooks/shellcheck-after-edit.yaml b/.agnostic-ai/hooks/shellcheck-after-edit.yaml new file mode 100644 index 000000000..45211dffa --- /dev/null +++ b/.agnostic-ai/hooks/shellcheck-after-edit.yaml @@ -0,0 +1,5 @@ +name: shellcheck-after-edit +description: Show ShellCheck diagnostics after editing shell files. +on: after-edit +command: 'bash "$(git rev-parse --show-toplevel)/.agnostic-ai/scripts/shellcheck-after-edit.sh"' +commandWindows: "bash ((git rev-parse --show-toplevel) + '/.agnostic-ai/scripts/shellcheck-after-edit.sh') codex; exit $LASTEXITCODE" diff --git a/.agnostic-ai/rules/architecture-map.md b/.agnostic-ai/rules/architecture-map.md index 4f33273b8..ced87b2c5 100644 --- a/.agnostic-ai/rules/architecture-map.md +++ b/.agnostic-ai/rules/architecture-map.md @@ -1,163 +1,7 @@ --- -target: claude paths: - "src/**/*.sh" - "bashunit" --- -# Architecture Map: modules and the life of a run - -Orientation for working in `src/`, which file owns what, and the exact call -flow of a test run. Line numbers drift; function names are the stable anchors. - -## The life of `./bashunit tests/` - -``` -bashunit (entry) sources all src/*.sh; version gate; early flag scan -└─ bashunit::main::cmd_test (main.sh: flag parsing, env exports) - └─ bashunit::runner::load_test_files (runner/discovery.sh: the per-file loop) - ├─ console_header::print_header "Running N tests", captures - │ └─ helper::find_total_tests $() SUBSHELL: sources each file in a - │ nested subshell just to count tests - ├─ source "$test_file" in the MAIN shell (workers inherit) - ├─ helper::check_duplicate_functions one awk pass per file - ├─ runner::functions_for_script pure bash: filter + sort by def line - ├─ helper::build_provider_map one awk pass per file (cached by path, - │ but the count subshell's build dies - │ with it → runs twice per file) - └─ runner::call_test_functions per-fn loop - └─ runner::run_test [--parallel: spawned as a & worker per test] - ├─ clock::now_to_slot duration start (if needed) - ├─ runner::execute_test_body $() subshell: the actual test fn - │ └─ state::export_subshell_context printf payload "##K=V##…" - ├─ runner::parse_result sync: parse counts from payload - │ parallel: + write .result via mktemp - └─ console_results::print_successful_test / _failed_ / … - └─ str::rpad + strip_ansi align per-test time (pure bash) - └─ [--parallel] wait; state::aggregate_parallel_results over *.result files - └─ console_results::render_result totals; deferred failure/skip blocks - └─ rerun::persist; env::cleanup_run_output_dir; exit code -``` - -Key inversion to remember: **tests run inside `$()` subshells; state comes back -as an encoded single-line payload** (`##ASSERTIONS_FAILED=…##TEST_OUTPUT=##`), -parsed by `runner::parse_result`/`state.sh`. Counters only exist in the main -shell (or, in parallel, in per-test `.result` files aggregated at the end). - -## Module ownership - -| Module | Owns | -|--------|------| -| `bashunit` | entry: version gate, early flag scan, sources every module, subcommand routing | -| `main/index.sh` | aggregator only, sources the `src/main/` module below | -| `main/validate.sh` | shared parser validation: unknown options, integer/path checks, shard parsing | -| `main/test.sh` / `main/bench.sh` / `main/subcommands.sh` | flag parsing per subcommand | -| `main/assert.sh` | the standalone `bashunit assert` path | -| `main/watch.sh` / `main/run.sh` | the `--watch` loop; the run lifecycle, exit codes and cleanup | -| `runner/index.sh` | aggregator only, sources the `src/runner/` module below | -| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes | -| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload | -| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers | -| `runner/result.sh` | `parse_result{,_sync,_parallel}`, failure source context, failed/skipped/incomplete/risky writers | -| `runner/parallel.sh` | job-slot waiting (`wait -n` or poll), running-job count, spinner | -| `runner/hooks.sh` | set_up/tear_down (test + script scope), hook failure records, mock clearing, EXIT cleanup | -| `runner/provider.sh` | `@data_provider` argument parsing | -| `runner/exec.sh` | `run_test`, the capture-subshell body, retry, timeout watchdog, per-file dispatch, `order_functions_for_script` (the run order a seed produces) | -| `runner/list.sh` | `--list`/`--dry-run`: emits selected tests instead of running them; hooks in after all filtering, before any hook | -| `runner/discovery.sh` | `load_test_files` (the per-file loop), `functions_for_script` | -| `runner/bench.sh` | benchmark file loop and bench function dispatch | -| `helper/index.sh` | aggregator only, sources the `src/helper/` module below | -| `helper/naming.sh` | resolving and normalising test-function and variable names | -| `helper/discovery.sh` | `find_files_recursive`, `find_total_tests`, fn filtering, duplicate check, line lookup | -| `helper/provider.sh` / `helper/tags.sh` | `@data_provider` map; `@tag` extraction and matching | -| `helper/encoding.sh` / `helper/functions.sh` / `helper/git.sh` | base64 + ids; generic fn utilities; remote tag lookup for `upgrade` | -| `state/index.sh` | aggregator only, sources the `src/state/` module below | -| `state/counters.sh` | test and assertion tallies and their get/add accessors | -| `state/duplicates.sh` | duplicate test-function detection state | -| `state/context.sh` | per-test output buffer, exit code, title, hook failure, and the per-test reset | -| `state/payload.sh` | the encoded per-test result payload and the base64 capability probe | -| `state/parallel.sh` | aggregating per-test result files after a `--parallel` run | -| `config/index.sh` | aggregator only, sources the `src/config/` module below | -| `config/env.sh` | all `BASHUNIT_*` defaults/config files, scratch dirs (`_BASHUNIT_RUN_OUTPUT_DIR` + EXIT-trap cleanup); executes at source time | -| `config/rerun.sh` | the `.bashunit/last-failed` cache for `--rerun-failed` | -| `config/parallel.sh` | is this run parallel; worker temp tree and stop-on-failure flag file (not `runner/parallel.sh`, which waits on job slots) | -| `console/index.sh` | aggregator only, sources the `src/console/` module below | -| `console/colors.sh` | the `_BASHUNIT_COLOR_*` palette and `bashunit::sgr` | -| `console/header.sh` | the "Running N tests" header | -| `console/line.sh` | `print_line`, the primitive every result line goes through, and its TAP variant | -| `console/duration.sh` / `console/diff.sh` | duration formatting; unified and line-by-line diffs under a failure | -| `console/test_line.sh` | the per-test result lines (passed/failed/skipped/incomplete/snapshot/risky/error) | -| `console/deferred.sh` | end-of-run blocks buffered during the run (scratch files under the run dir) | -| `console/summary.sh` | run totals, execution time, hook completion | -| `assert/index.sh` | aggregator only, sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | -| `assert/core.sh` | `assert::should_skip`, `assert::fail_with`, `assert::join_to_slot` and the comparison assertions the other files build on | -| `assert/{arrays,assertions,dates,duration,files,folders,json,once,snapshot}.sh` | the per-topic assertions; the per-assertion path must stay fork-free | -| `util/index.sh` | aggregator only, sources the `src/util/` module below | -| `util/clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads | -| `util/str.sh` / `util/math.sh` | pure-bash string and arithmetic helpers | -| `api/index.sh` | aggregator only, sources the `src/api/` module below | -| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone, the entrypoint already set it | -| `api/skip_todo.sh` / `api/test_title.sh` | `skip`/`todo` markers; `set_test_title` | -| `api/bashunit.sh` | the custom-assert facade (`assert_that`, `assertion_failed`/`passed`) | -| `doubles/index.sh` | aggregator only, sources the `src/doubles/` module below | -| `doubles/mock.sh` | `mock`/`unmock` and the `_BASHUNIT_MOCKED_FUNCTIONS` registry `runner/hooks.sh` unwinds per test | -| `doubles/spy.sh` | `spy` and its `_BASHUNIT_SPY_*` state slots + files | -| `doubles/assertions.sh` | the `assert_have_been_called*` family | -| `coverage/index.sh` | aggregator only, sources the `src/coverage/` modules below | -| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | -| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | -| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | -| `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | -| `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | -| `coverage/functions.sh` | function definitions and their line spans | -| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose**, the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | -| `coverage/report_text.sh` / `report_lcov.sh` / `report_html.sh` | the three renderers | -| `coverage/html_index.sh` / `html_file.sh` | HTML page emitters; the only two files exempt from `max_line_length` | - -| `reports.sh` | JUnit/HTML/TAP/JSON writers | -| `system/index.sh` | aggregator only, sources the `src/system/` module below | -| `system/check_os.sh` / `system/dependencies.sh` | one-fork OS detect; `command -v` probes (builtins, not forks) | -| `system/io.sh` | small I/O helpers | -| `cli/index.sh` | aggregator only, sources the `src/cli/` module below | -| `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; `main.sh` is their only caller | -| `benchmark/index.sh` | the remaining non-`test` subcommand; shared with `runner/bench.sh` | - -## Cross-cutting invariants - -- **Bash 3.0 floor** (`.agnostic-ai/rules/bash-style.md`): no `[[`, `declare -A`, - `${var,,}`, `BASHPID`, negative indices. Subshells share `$$`, you cannot - make a per-worker unique token without a fork (`mktemp`) or an ordinal - assigned before the fork. `RANDOM` is no help either: whether a subshell - reseeds it depends on the platform and the nesting depth (measured in - perf-fork-budget.md), so it is unreliable in both directions (#1354). -- **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` - captures on hot paths, bash-style.md documents it; `local` is dynamically - scoped, so helpers must not write caller-named variables. -- **Fork budget** (`.agnostic-ai/rules/perf-fork-budget.md`): per-test paths are - fork-free (sequential) or mktemp-only (parallel); budgets are enforced by - `tests/acceptance/bashunit_*forks*_test.sh` on three platforms, a "harmless" - `echo | sed` in a per-test path will fail CI. -- **`call_test_functions` is a background subshell under `--parallel`** - (`discovery.sh` backgrounds it per file). Anything it sets in shell state, - counters, flags, error records, dies with the worker. Only two things cross - back: per-test `.result` payloads, and files written to disk. This produced - two silent-green bugs in one day: a data-provider error printed while the run - exited 0 (#1145), and duplicate-test-function detection being entirely off in - the mode CI uses (#1147). Before adding a check anywhere under that call, ask - which side of the fork it belongs on, a whole-file check almost always - belongs in the parent loop, before dispatch. - **Verify both modes.** Sequential and `--parallel` disagreeing is the - signature of this bug, and it is invisible to a sequential-only test. -- **Binaries pinned at startup** (`$GREP`, `$MKTEMP`, `$CAT` in env.sh) so test - doubles/PATH games can't hijack the framework's own plumbing. -- **Snapshots assume 80-col non-tty width** (`tput cols` fallback); anything - that changes rendering widths breaks `tests/acceptance/snapshots/`. -- **The build flattens the source graph in DFS order** (`build.sh` - `build::process_file`): a file's body is emitted, *then* its `source` lines are - recursed into. That equals dev-mode order only if a module aggregator - (`src//index.sh`, ADR-010) contains **nothing but `source` - lines and comments**, any other top-level statement would run before its - dependencies in the built artifact but after them in dev mode. Files are - deduped by repo-relative path, so `src/` may hold module dirs and two files may - share a basename. Every src file's **first line must be the shebang**: - `tail -n +2` strips it when embedding. +Before changing `src/` or the `bashunit` entry point, read `.agnostic-ai/context/architecture-map.md` for module ownership, the run lifecycle, and cross-cutting constraints. diff --git a/.agnostic-ai/rules/bash-style.md b/.agnostic-ai/rules/bash-style.md index 402565eb5..e50738a90 100644 --- a/.agnostic-ai/rules/bash-style.md +++ b/.agnostic-ai/rules/bash-style.md @@ -1,175 +1,7 @@ --- -target: claude paths: - "src/**/*.sh" - "tests/**/*.sh" --- -# Bash Style & Compatibility Rules - -## Bash 3.0+ Compatibility (Critical) - -bashunit must work on **Bash 3.0+**. The floor was deliberately lowered from 3.2 -to 3.0 in 0.33 to reach older systems; macOS's 3.2 is the common case, not the -floor. The runtime gate in `./bashunit` compares major **and** minor against -`BASHUNIT_MIN_BASH_VERSION`, so the declared floor is the enforced one. - -These are **version-breaking** above the floor and are prohibited. They are -enforced mechanically by `tests/unit/bash_compatibility_test.sh`, which greps -`src/` regardless of test coverage: - -| Feature | Bash ver | Alternative | -|---------|----------|-------------| -| `printf -v` | 3.1+ | Return-slot pattern (below) | -| `x+=y` (string append) | 3.1+ | `var="$var$more"` | -| `arr+=(x)` (array append) | 3.1+, and a 3.0 **parse** error | `arr[${#arr[@]}]=x` | -| `declare -A` (associative arrays) | 4.0+ | Parallel indexed arrays | -| `${var,,}` / `${var^^}` (case) | 4.0+ | `tr '[:upper:]' '[:lower:]'` | -| `${array[-1]}` (negative index) | 4.3+ | `${array[${#array[@]}-1]}` | -| `&>>` (append both) | 4.0+ | `>> file 2>&1` | -| `BASHPID` | 4.0+ | fork (`mktemp`) or an assigned ordinal | -| `mapfile` / `readarray` | 4.0+ | `while IFS= read -r` | -| `declare -n` / `local -n` (nameref) | 4.3+ | Return-slot pattern (below) | -| `coproc` | 4.0+ |, | -| `${var@Q}` (transformations) | 4.4+ |, | - -`[[ =~ ]]` is a subtler case: it **exists** on 3.0 (verified against a real -build), but 3.2 changed whether a quoted right-hand side is a regex or a -literal. At a 3.0 floor the same match therefore behaves differently across -supported versions, so regex matching goes through `grep -E`, a real -constraint, not a style preference. See `.agnostic-ai/rules/perf-fork-budget.md`. - -### Style, not compatibility - -Prefer `[ ]` over `[[ ]]` for ordinary tests. This is **house style**: `[[ ]]` -itself works on every supported bash, including 3.0. Do not justify it as a -Bash 3 requirement, and do not add it to the compatibility gate. - -## Coding Conventions - -- **2 spaces** indent, no tabs, enforced by `make lint` (EditorConfig). Do **not** - run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses - `\`-continued strings past the 120-char limit, breaking `make lint` -- **120 chars** max line length (soft) -- Follow [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html) -- Always quote variables unless explicit word splitting is needed -- Use `$()` for command substitution, never backticks - -### Naming - -- **Public functions:** `bashunit::function_name` -- **Private functions:** `_function_name` (leading underscore) -- **Local variables:** `lowercase_with_underscores` -- **Globals/exports:** `UPPERCASE_WITH_UNDERSCORES` - -### Function Docs (public functions) - -```bash -## -# Brief description -# Arguments: $1 - desc, $2 - desc (optional, default: "x") -# Returns: 0 success, 1 failure -## -``` - -### File Structure - -Constants -> Globals -> Private functions -> Public functions - -Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` - -## Returning values from a helper (dynamic-scope safety) - -Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee -shadows the caller's same-named variable for the duration of the call. Same trap fires -with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...` -writes, they all resolve against the dynamic scope. - -We need **all three** properties on the hot path: - -1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork - per call, which dominates per-test cost. -2. **No collision with caller locals.** A helper that silently overwrites or reads - the wrong variable is the worst kind of bug, no error, just stale data. -3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and - it has the same dynamic-scope shadowing as `eval "$name=..."` anyway). - -### Preferred: dedicated global return slot - -```bash -_BASHUNIT_PKG_THING_OUT="" - -# Writes the result into _BASHUNIT_PKG_THING_OUT. -# Arguments: $1 input -function bashunit::pkg::do_thing() { - local input=$1 # natural name, no prefix needed - local val="${input%%]*}" - val="${val#[}" - _BASHUNIT_PKG_THING_OUT=$val -} - -# Caller -bashunit::pkg::do_thing "$payload" -local thing=$_BASHUNIT_PKG_THING_OUT -``` - -- Zero forks per call. -- Helper has natural local names; no `__bu_` noise. -- Caller cannot accidentally shadow the slot because the `_BASHUNIT_*` namespace - is reserved for the framework. -- A dedicated slot per helper (rather than one shared `_BASHUNIT_OUT`) means - adjacent or nested calls can't clobber each other. Cheap: globals are free. - -Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`, -`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`), -`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`). - -### When the helper builds dynamic variable names (mock/spy state) - -Use a clear namespace prefix on the **constructed** name, not on the helper's locals: - -```bash -# Spy state lives in _BASHUNIT_SPY_${variable}_TIMES_FILE, not ${variable}_times_file. -# A caller doing `local foo_times_file=...` is therefore harmless: the helper -# resolves a different global. -export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file" -``` - -Example in tree: `src/test_doubles.sh` (`_BASHUNIT_SPY_*`). - -### Last-resort fallback: outvar by name + `__bu_` prefix on locals - -Only use this when neither a fixed return slot nor a namespaced constructed name is -practical (e.g. a generic helper called from many call sites with different output -variables and no shared global is appropriate): - -```bash -function bashunit::pkg::do_thing() { - local __bu_out=$1 - local __bu_in=$2 - local __bu_val="${__bu_in%%]*}" - eval "$__bu_out=\$__bu_val" -} -``` - -- All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing - silently breaks the caller's outvar (see PR #672, that exact bug caused 12 parallel - test failures in #662). -- Include a regression test that calls the helper with one of its own documented - internal names as the outvar. - -### Intentional dynamic-scope mutation is a separate pattern - -The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends) -deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). -That is documented inline above the helpers in that file and is **not** the outvar pattern, -the helper has no `$1`-named outvar argument; the caller agrees to share state by convention. -It is also why those helpers and `extract_branches` must stay in one file (ADR-010). -Don't introduce new instances of this pattern without an inline justification comment. - -## ShellCheck - -All code must pass `make sa`. Use directives sparingly with reason: -```bash -# shellcheck disable=SC2034 # Variable used by caller -``` +Before changing shell code under `src/` or `tests/`, read `.agnostic-ai/context/bash-style.md`. Keep Bash 3.0 parsing valid and use the documented version gates and return-slot pattern. diff --git a/.agnostic-ai/rules/perf-fork-budget.md b/.agnostic-ai/rules/perf-fork-budget.md index 7b27e398e..d9caf12a7 100644 --- a/.agnostic-ai/rules/perf-fork-budget.md +++ b/.agnostic-ai/rules/perf-fork-budget.md @@ -1,303 +1,7 @@ --- -target: claude paths: - "src/**/*.sh" - "tests/acceptance/bashunit_*forks*_test.sh" --- -# Performance: Fork Budget & Census Method - -bashunit's dominant runtime cost on Bash 3.2 (macOS) is **process forks**, not -shell execution. A fork costs ~1-3ms; the acceptance suite spawns ~258 nested -`./bashunit` runs, so one avoidable fork per file or per test multiplies fast. -PRs #801-#817 removed the per-test, per-file and cold-start forks (the full -`--parallel` acceptance suite went from ~61s to ~17s wall, CPU time halved); -this file records how to measure and the traps found, so future work starts -from evidence. - -## Measuring: shim census is ground truth, traces overcount - -**PATH-shim census (authoritative).** Wrap a binary, count invocations: - -```bash -real_awk="$(command -v awk)" # resolve BEFORE prepending the shim dir -printf '#!/usr/bin/env bash\necho x >> /tmp/count\nexec "%s" "$@"\n' "$real_awk" > shim/awk -chmod +x shim/awk -PATH="shim:$PATH" ./bashunit --no-parallel fixture_test.sh -grep -c . /tmp/count -``` - -Caveats: binaries pinned at startup via `command -v` (`$GREP`, `$MKTEMP`, -`$CAT`) bypass PATH shims, trace those instead; and shims are unreliable on -Git Bash (skip such tests on Windows). - -**A shim census only sees forks that exec a binary.** `$(some_shell_function)` -forks a subshell and execs nothing, so it is invisible to every shim and to the -`^\++ +/path/to/binary` trace patterns above, yet it costs the same ~0.6ms. -That blind spot hid sixteen of them in the colour palette (`$(bashunit::sgr N)` -per colour, ~9ms, the largest single cost in a cold start) straight through the -#801-#851 campaign that pinned everything else on this page. To count them, -match the *function name* in the trace (`^\++ +bashunit::fn( |$)`), or profile -by injecting `$EPOCHREALTIME` echoes at the `# src/` markers the build -emits, that attributes startup cost per source file and is how this one -surfaced. - -**`bash -x` trace census (cheap but inflated).** `PS4='+ ' bash -x ./bashunit …` -also counts trace lines **re-echoed inside captured test output**, so it can -overcount 10-20x (one real `grep` appeared 24 times). Use it to *locate* fork -sites (`PS4='+X ${BASH_SOURCE##*/}:$LINENO> '`), never to *count* them. It also -misses forks inside `--parallel` workers. Anchor real execs with -`grep -cE '^\++ +/[^ ]*mktemp$'`-style patterns (absolute path, exact argv -shape) to skip `command -v` probes and variable assignments. - -**Regression tests.** Budgets are enforced in -`tests/acceptance/bashunit_coldstart_forks_test.sh` (cold start: mktemp/perl/ -grep/mkdir) and `tests/acceptance/bashunit_run_forks_test.sh` (run path: -grep/cat/sed/sort/awk budget, run-dir cleanup). Extend these when you remove a -fork class; they are the RED test of the TDD cycle. - -## Replacement patterns that worked - -| Fork | Replacement | Example | -|------|-------------|---------| -| `echo x \| grep -c fixed` | `case` glob match | #803, #804 | -| `cat <>` creation | #801, #811 | -| probe fork + first-use fork | probe does the real work and seeds the return slot | #802 (clock perl) | -| per-worker `mkdir -p` | parent pre-creates before spawning workers (`[ -d ]` inside a worker races its siblings) | #813 | -| sanitize-args pipeline on empty input | guard: skip the pipeline when the input is empty | #813 | -| `printf \| grep -cE` per line as a classifier fallback | `case` globs translating each regex alternation | #1005 (286 lines cost 1055 forks) | - -`shopt -s extdebug` for `declare -F`: enable it **inside the capture subshell -only**, toggling it in the caller's shell clobbers caller state (#808). - -## Where pure bash LOSES, measured, do not "optimize" these - -- **`… | awk` over ≳100 lines**: a `while read` loop was 5x slower than the awk - fork (19ms vs 3.5ms for 600 `declare -F` lines). File scans (provider map, - duplicate check) stay awk. -- **`${var//pattern/}` on large strings**: quadratic on Bash 3.2, 2.7 s where - awk takes 3.5ms. Never string-replace over big captures. -- **Regex assertions** (`assert_matches`): the `grep -E` fork is **not** because - `[[ =~ ]]` is missing, it was verified working on a real Bash 3.0 build. It is - because 3.2 changed whether a quoted right-hand side is a regex or a literal, - so at the 3.0 floor the same pattern would match differently across supported - versions. Removing the fork means picking one semantic and breaking the other. - Measured cost of keeping it, so the trade is at least quantified: 2000 - `assert_same` run in ~130 ms (0.065 ms each), 500 `assert_matches` in ~1.25 s - (**2.5 ms each, ~38x**). That is the fork, and it is the price of the 3.0 - floor -- not a regression to chase. -- **Single-file build artifact**: sourcing `bin/bashunit` is *not* faster than - sourcing `src/*.sh` (parse time dominates, file opens don't). -- **`tput cols` at startup**: returns 80 on non-tty; snapshots depend on that - width. Not removable. - -## Estimating: three ways an estimate lied, all in one day - -The coverage work in #1084-#1112 produced three wrong predictions, each of -which cost a full implement-and-measure cycle. Check against these before -believing a number. - -- **A micro-benchmark of a function the DEBUG trap calls overstates the win.** - A same-file memo for `record_line`'s path lookups predicted 5x from a loop - benchmark and delivered **4%** in situ (#1102). Whatever the trap costs - around the call swamps what the call itself costs. -- **Stubbing tells you what a stage costs, not what removing a syscall from it - saves.** Returning early from `record_line` attributed 705 ms to its two - `>>` appends; holding the files open on persistent descriptors recovered - **184 ms** of it, not 705 (#1110). -- **A benchmark path must be the path that occurs.** `${file//[^a-zA-Z0-9]/_}` - costs 7.8 µs at 7 characters, 131 µs at 59 and 549 µs at 121, so measuring - it with an absolute path when the trap actually sees `./src/x.sh` inflated - the estimate by 4x (#1102). - -**A/B in the same tree.** Comparing a worktree under `/private/tmp` against the -repo under `/Users` made a 70 ms *improvement* read as a 40 ms regression. -Check out the two versions of the file into one tree and alternate rounds. - -## Process substitution as an argument fails under load - -`cmd <(a) <(b)` passes `/dev/fd/N` paths that **`cmd` itself must open**. Under -a loaded parallel suite that open fails: - -``` -diff: /dev/fd/62: Bad file descriptor -``` - -`diff` then produces no comparison and writes that to stderr, which a caller -collecting output reads as a real difference. Both coverage differentials did -this once per repo file, 473 iterations, and failed roughly 1 full suite run -in 3, each time reporting "the classifiers disagree" over a file that agreed -perfectly (#1152). Two earlier attempts blamed `awk`; the `AWK-FAILED` marker -from #1143 is what finally ruled it out. - -The redirect form `while read …; done < <(cmd)` is **not** the same shape: the -shell opens the descriptor and dups it to stdin, so no external process opens -`/dev/fd/N`. It is used throughout `src/` and has never been seen to fail. - -So: never put `<(...)` in an external command's **argument list** inside a loop -that runs per file or per test. Write real files (concurrently, so the two -sides still overlap, serialising them was measurably slower) and compare with -`[ "$a" != "$b" ]`, which the shell does without a fork. - -## Where pure bash LOSES on some Bash versions but not others - -- **`${var//&/…}` is not portable.** Bash 5.2 made a bare `&` in a substitution - REPLACEMENT mean "the matched text", so `${line//` marker technique: 41.6ms of executed top-level code, -against 10ms to parse the whole 600KB artifact and define all 915 functions. -Parsing is cheap; what runs at source time is not. The current shape is -`config/env.sh` ~12.6ms (config files, `tput`, `mkdir`), `state/payload.sh` -~2.9ms (the `base64 --help` probe) and `system/check_os.sh` ~2.4ms (`uname`). -Lazy-loading whole modules was still rejected in #798. - -Measuring a change this small needs ~200 invocations per sample: the two forks -are ~4ms against a ~65ms startup, and single runs vary by ±10ms. The acceptance -suite is useless for it, consecutive runs there differ by 5s. -`--coverage` adds ~3 forks per unique file first seen by the DEBUG trap -(decision-cache miss: grep|head + dirname), bounded by file count, nightly -non-gating workflow, not worth chasing. - -**Don't assume the engine is the cost.** Profiling `--coverage` for #1005 found -the *report* phase was ~50-60% of wall time and identical for both engines: the -per-line executable/non-executable classifier fell back to a `grep -E` fork for -every line it could not decide in pure Bash, and every tracked line is classified -twice per run (`precompute_file_stats`, then `report_lcov`). Budget guarded by -`tests/acceptance/bashunit_coverage_forks_test.sh`, expressed as "fork count does -not grow with source-line count" rather than an absolute number. - -Two traps when replacing a regex classifier with `case` globs, both silently -change coverage numbers rather than erroring: - -- Inside a POSIX bracket expression a backslash is a **literal member of the - set**, so `[^\)]` excludes `\` as well as `)`. `x=$(foo)` and - `x=$(printf '%s\n')` therefore classify differently. -- `[\{\}]` likewise matches a lone `\`, so a bare line continuation counts as a - brace-only line. - -Verify such a rewrite by running both implementations over the same input -(`git show HEAD:` + `declare -f | sed` to alias the old one) across every -`git ls-files '*.sh'` line, and mutation-test the harness itself, a differential -that cannot fail proves nothing. - -**Parallel 10-test file run (CI's mode):** ~11 forks, 3 `mkdir`, 4 `rm`, -3 `awk` (#813; was 61). That count was taken with a sequential census fixture -and so missed the one cost that only exists in this mode: every worker used to -re-probe the clock, because the probe resolved the implementation inside a -`$( )` and the resolved value died with that subshell. On a shell without -`EPOCHREALTIME` the probe forks `perl`, so it scaled one-for-one with the tests -, 502 execs for a 500-test file against 2, and it fired even with per-test -timing off, since deciding that timing is off is what asks whether the clock is -expensive (#1353). **Measure the parallel budget with a parallel fixture**: a -per-worker cost is invisible to a sequential census by construction. The per-test result file is named by a per-suite -ordinal the single-threaded dispatcher assigns just before each `&` (the fork -inherits it), so it costs **no** `mktemp` + `mv` per test (#851; was 10 -`mktemp` + 10 `mv`). This replaced the old sanitized-test-name scheme, whose -deterministic names could collide (different provider args sanitize identically) -because Bash 3 workers can't mint a unique token, subshells inherit `$$`, and -`BASHPID` is 4.0+; an ordinal sidesteps that entirely. - -This file used to add "and the `RANDOM` state" to that list. Measured, `RANDOM` -is neither reliably shared nor reliably reseeded (#1354): - -| Context | three consecutive `$( )` reads | -|---|---| -| Plain shell, 3.00.22, 3.2.57, 4.4, 5.2, 5.3 | differ | -| `--parallel` worker, Linux 3.0, 5.2 | differ | -| `--parallel` worker, macOS 3.2.57 | **identical** | - -So it depends on the platform *and* on how deeply nested the subshell is. Do -not build on it in either direction, and do not reuse "`RANDOM` is shared" as a -premise, it is right about the conclusion for the wrong reason. `$$` is the -part that genuinely is inherited. -`wait_for_job_slot` already uses `wait -n` on Bash 4.3+ and an adaptive -sleep-poll fallback, don't "fix" it. The spinner forks `sleep` ~1/s on -non-tty; not worth chasing. - -**Reports are on the per-test path too.** Any `--log-*`/`--report-*` flag makes -each worker spool a result row, and that row used to base64 each of its nine -fields on its own, 14 `base64` forks per test counting the parent's decode, -with a `tr` on each encode. `--log-junit` therefore cost more than the run it -reported on (300 tests: 1.2s → 9.2s wall, 5.8s → 27.4s CPU). Now only the four -fields that can hold arbitrary text are encoded and the row is joined with -0x1F. The main workflow passes no report flag, so CI never showed this; the -budget is guarded by `bashunit_run_forks_test.sh`. When adding a field to that -row, ask whether it can hold arbitrary text before reaching for base64. +Before changing `src/` or a fork census acceptance test, read `.agnostic-ai/context/perf-fork-budget.md`. Keep per-test paths within the existing process budget; measure with the matching sequential or parallel fixture. diff --git a/.agnostic-ai/rules/tdd-workflow.md b/.agnostic-ai/rules/tdd-workflow.md index 1cedcb537..e8f6d49bf 100644 --- a/.agnostic-ai/rules/tdd-workflow.md +++ b/.agnostic-ai/rules/tdd-workflow.md @@ -1,80 +1,7 @@ --- -target: claude paths: - "**/*.sh" - ".tasks/**/*.md" --- -# TDD Workflow - -**Test-Driven Development is mandatory** for all bashunit development. - -## The Cycle - -``` -RED -> Write a failing test (must fail for the RIGHT reason) -GREEN -> Write minimal code to make it pass (nothing extra) -REFACTOR -> Improve code while keeping all tests green -REPEAT -> Until acceptance criteria are met -``` - -## Task File (Recommended for non-trivial changes) - -For features, refactors, or multi-test work, create `.tasks/YYYY-MM-DD-slug.md`. -Skip for small bug fixes or single-test changes. - -```markdown -# [Feature/Fix Name] - -**Date:** YYYY-MM-DD **Status:** In Progress - -## Acceptance Criteria -- [ ] Criterion 1 - -## Test Inventory -- [ ] `test_should_handle_valid_input` -- [ ] `test_should_fail_with_invalid_input` - -## Current Red Bar -Test: (none yet) - -## Logbook -### YYYY-MM-DD HH:MM -- Created task, analyzed existing code -``` - -## RED Phase - -1. Pick the **smallest next test** from inventory -2. Study patterns from existing tests (unit -> `assert_test.sh`, doubles -> `doubles_test.sh`) -3. Write test following Arrange-Act-Assert -4. Run: `./bashunit path/to/test.sh`, **must fail** -5. Verify failure reason: function missing or wrong output (**not** syntax error) - -## GREEN Phase - -1. Write **minimal** code to pass, no extra features, no premature optimization -2. Run: `./bashunit path/to/test.sh`, **must pass** - -## REFACTOR Phase - -1. Improve readability, naming, extract duplication, **no behavior changes** -2. Run tests after each change -3. After the change set is complete, run quality checks once: `make sa && make lint` (never `shfmt -w`, it breaks `make lint`) - -## Quality Gate (Before Commit) - -```bash -./bashunit tests/ # All tests -./bashunit --parallel tests/ # Parallel (isolation check) -make sa && make lint # Static analysis + linting (lint is the format gate) -``` - -## Definition of Done - -- All tests green for the **right reason** -- All acceptance criteria met -- Quality gate passes -- Bash 3.0+ compatible -- CHANGELOG updated (if user-facing) -- ADR created (if architectural decision) +For shell development and task plans, read `.agnostic-ai/context/tdd-workflow.md`. Write a failing test for runtime changes, implement the smallest passing change, then refactor. Run the full quality gate once after the change set is complete. diff --git a/.agnostic-ai/rules/testing.md b/.agnostic-ai/rules/testing.md index 9dd4186a5..3f50dbd38 100644 --- a/.agnostic-ai/rules/testing.md +++ b/.agnostic-ai/rules/testing.md @@ -1,136 +1,6 @@ --- -target: claude paths: - "tests/**/*_test.sh" --- -# Testing Guidelines - -## Organization - -| Directory | Purpose | Pattern | -|-----------|---------|---------| -| `tests/unit/` | Isolated function tests | Mocks/spies for deps | -| `tests/functional/` | Multi-component integration | Real interactions | -| `tests/acceptance/` | CLI/end-to-end workflows | Full user scenarios | - -**Naming:** Files end with `_test.sh`. Functions: `test_should__when_` - -## Assertions - -```bash -assert_equals "expected" "$actual" -assert_not_equals "not_this" "$actual" -assert_contains "substring" "$haystack" -assert_not_contains "substring" "$haystack" -assert_matches "regex" "$string" -assert_not_matches "regex" "$string" -assert_empty "$var" -assert_not_empty "$var" -assert_successful_code "$?" -assert_general_error "$?" -assert_file_exists "$path" -assert_file_not_exists "$path" -assert_directory_exists "$path" -assert_array_contains "value" "${array[@]}" -assert_array_not_contains "value" "${array[@]}" -``` - -There is no `assert_fails`. To assert that an assertion *fails*, capture its output and -compare it with the renderer: - -```bash -assert_same "$(bashunit::console_results::print_failed_test "My test" "a" "but got " "b")" \ - "$(assert_same "a" "b")" -``` - -## Test Doubles - -Helpers are namespaced (`bashunit::*`); assertions are bare. The unprefixed helper name -is a `command not found`, not an alias. - -```bash -# Spies - track calls without changing behavior -bashunit::spy function_name -assert_have_been_called function_name -assert_have_been_called_times 2 function_name # count first, then spy -assert_have_been_called_with function_name "arg" # spy first, then expected -assert_have_been_called_with function_name "arg" 1 # ...of call #1 -assert_have_been_called_nth_with 1 function_name "arg" -assert_have_been_called_with_args function_name "a b" # boundary-exact, no call index - -# Mocks - replace behavior -bashunit::mock date echo "2024-05-01" -bashunit::mock uname <<< "Linux" # heredoc form ignores the call's arguments -``` - -Note `_times` takes the count first while `_with` takes the spy first, a swapped pair -fails the assertion rather than erroring, so it reads like a real defect. - -## Data Providers - -```bash -function data_provider_inputs() { - echo "5 3 8" - echo "0 0 0" -} - -# @data_provider data_provider_inputs -function test_should_add() { - assert_equals "$3" "$(add "$1" "$2")" -} -``` - -## Lifecycle Hooks - -- `set_up_before_script()`, once before all tests in file -- `set_up()`, before each test -- `tear_down()`, after each test -- `tear_down_after_script()`, once after all tests in file - -## Snapshot Testing - -```bash -assert_match_snapshot "$output" -assert_match_named_snapshot "stderr" "$stderr" # multiple snapshots in one test -# Re-record deliberately: ./bashunit --snapshot-update --filter "test name" tests/ -``` - -## Writing a test that generates test files - -The duplicate-test-function check scans a file **textually**, so a fixture body -embedded in a string counts as a definition of the *enclosing* file: - -```bash -function test_one() { - run_fixture 'function test_x() { assert_same 1 1; }' # ← counted -} -function test_two() { - run_fixture 'function test_x() { assert_same 2 2; }' # ← counted: duplicate -} -``` - -That fails the whole run before any test executes, and the message names a -function this file never defines. Give every embedded fixture function a name -unique across the file. The report includes the line of each definition -(`test_x (lines 30, 40)`), which is what identifies the strings. - -It cannot be fixed by tracking quote state, an apostrophe in any comment -(`# doesn't work`) would flip it and mask real duplicates. - -## Test Isolation - -- Use `$(bashunit::temp_file)` / `$(bashunit::temp_dir)` (auto-cleaned) for file - operations, these are functions, not variables -- No shared global state between tests -- No network calls, mock external commands -- No time dependencies, mock `date` if needed -- Tests must be safe for `./bashunit --parallel tests/` - -## Reference Tests - -- **Assertions:** `tests/unit/assert_test.sh` -- **Doubles:** `tests/functional/doubles_test.sh` -- **Providers:** `tests/functional/provider_test.sh` -- **Hooks:** `tests/unit/setup_teardown_test.sh` -- **CLI/snapshots:** `tests/acceptance/bashunit_test.sh` +Before changing tests, read `.agnostic-ai/context/testing.md`. Follow the existing assertion, test-double, fixture, and lifecycle patterns. Tests must remain isolated under parallel execution. diff --git a/.agnostic-ai/scripts/shellcheck-after-edit.sh b/.agnostic-ai/scripts/shellcheck-after-edit.sh new file mode 100644 index 000000000..353f564f3 --- /dev/null +++ b/.agnostic-ai/scripts/shellcheck-after-edit.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +set -eu + +if [ -n "${1:-}" ]; then + export AGNOSTIC_AI_TARGET="$1" +fi + +files=$(agnostic-ai hook paths) || exit 1 +printf '%s\n' "$files" | while IFS= read -r file; do + case "$file" in + *.sh) + if [ -f "$file" ]; then + shellcheck -x "$file" 2>&1 | head -20 + fi + ;; + esac +done diff --git a/.agnostic-ai/skills/commit/SKILL.md b/.agnostic-ai/skills/commit/SKILL.md index 992b54ef5..3b8ea439e 100644 --- a/.agnostic-ai/skills/commit/SKILL.md +++ b/.agnostic-ai/skills/commit/SKILL.md @@ -11,53 +11,19 @@ allowed-tools: shell, read, Grep, Glob Stage and commit current changes using the conventional commits format. -> **IMPORTANT:** This skill MUST be used for ALL commits, even when the user says "commit" without `/commit`. Never commit without following these steps. +> **IMPORTANT:** This skill MUST be used for ALL commits, even when the user says "commit" without `commit`. Never commit without following these steps. ## Current State -::target claude -- Branch: !`git branch --show-current` -::end - -::target codex -- Run Branch: `git branch --show-current` -::end -::target claude -- Status: !`git status --short` -::end - -::target codex -- Run Status: `git status --short` -::end -::target claude -- Staged diff: !`git diff --cached --stat 2>/dev/null` -::end - -::target codex -- Run Staged diff: `git diff --cached --stat 2>/dev/null` -::end -::target claude -- Unstaged diff: !`git diff --stat 2>/dev/null` -::end - -::target codex -- Run Unstaged diff: `git diff --stat 2>/dev/null` -::end -::target claude -- Recent commits: !`git log --oneline -5 2>/dev/null` -::end - -::target codex -- Run Recent commits: `git log --oneline -5 2>/dev/null` -::end -## Arguments -::target claude -- `$ARGUMENTS` - Optional hint for the commit message (e.g., `fix the snapshot comparison`) -::end +Run the following commands before proceeding: +- Branch: `git branch --show-current` +- Status: `git status --short` +- Staged diff: `git diff --cached --stat 2>/dev/null` +- Unstaged diff: `git diff --stat 2>/dev/null` +- Recent commits: `git log --oneline -5 2>/dev/null` -::target codex -- the argument in the user request - Optional hint for the commit message (e.g., `fix the snapshot comparison`) -::end +## Arguments +- User-supplied argument: Optional hint for the commit message (e.g., `fix the snapshot comparison`) ## Instructions diff --git a/.agnostic-ai/skills/gh-issue/SKILL.md b/.agnostic-ai/skills/gh-issue/SKILL.md index 5804c5dd7..5509c54a6 100644 --- a/.agnostic-ai/skills/gh-issue/SKILL.md +++ b/.agnostic-ai/skills/gh-issue/SKILL.md @@ -11,25 +11,13 @@ allowed-tools: shell, read, edit, write, Grep, Glob, Agent, WebFetch Fetch a GitHub issue, create branch, implement following TDD, and open a PR. ## Arguments -::target claude -- `$ARGUMENTS` - Issue number (e.g., `42` or `#42`) -::end - -::target codex -- the argument in the user request - Issue number (e.g., `42` or `#42`) -::end +- User-supplied argument: Issue number (e.g., `42` or `#42`) ## Instructions ### Phase 1: Setup -::target claude -1. **Parse the issue number** from `$ARGUMENTS` (strip `#` if present) -::end - -::target codex -1. **Parse the issue number** from the argument in the user request (strip `#` if present) -::end +1. **Parse the issue number** from the user-supplied argument (strip `#` if present) 2. **Fetch issue details**: ```bash @@ -76,17 +64,11 @@ Fetch a GitHub issue, create branch, implement following TDD, and open a PR. - Files to Change - Implementation Order (smallest first step) -::target claude -9. **Use EnterPlanMode** if implementation is non-trivial -::end - -::target codex 9. **Keep a short implementation plan** in the main session if implementation is non-trivial -::end ### Phase 3: Implement -10. **Follow strict TDD workflow** (see `.agnostic-ai/rules/tdd-workflow.md`): +10. **Follow strict TDD workflow** (see `.agnostic-ai/context/tdd-workflow.md`): For each test: - **RED** - Write failing test, verify it fails for the RIGHT reason @@ -113,7 +95,7 @@ Fetch a GitHub issue, create branch, implement following TDD, and open a PR. 14. **Commit** using conventional commits with `Closes #` -15. **Create PR** using the `/pr` skill: +15. **Create PR** using the `pr` skill: ``` /pr # ``` diff --git a/.agnostic-ai/skills/gh-issues/SKILL.md b/.agnostic-ai/skills/gh-issues/SKILL.md index e33b1ae30..ff8951e78 100644 --- a/.agnostic-ai/skills/gh-issues/SKILL.md +++ b/.agnostic-ai/skills/gh-issues/SKILL.md @@ -11,7 +11,7 @@ allowed-tools: read, shell, Skill ## Purpose -Process every open GitHub issue that is **unassigned** or **assigned to the current user (`@me`)**, one after another, by delegating each to the `/gh-issue` skill. Stop on first hard failure so it can be inspected. +Process every open GitHub issue that is **unassigned** or **assigned to the current user (`@me`)**, one after another, by delegating each to the `gh-issue` skill. Stop on first hard failure so it can be inspected. Driven from inside the Claude session rather than a polling shell script. @@ -19,7 +19,7 @@ Driven from inside the Claude session rather than a polling shell script. - `--limit N`, process at most N issues this run (default: all). - `--label foo`, only issues carrying label `foo`. -- `--dry-run`, list issues that would be processed; do not invoke `/gh-issue`. +- `--dry-run`, list issues that would be processed; do not invoke `gh-issue`. Strip leading `#` if user passes `#123` style. @@ -79,14 +79,14 @@ For each issue in the queue: - Only `$me` listed → already mine, proceed (skip self-assign step). - Any other login present → skip this issue. -2. Invoke the `/gh-issue` skill with the issue number. That skill owns: +2. Invoke the `gh-issue` skill with the issue number. That skill owns: - self-assign via `gh issue edit --add-assignee @me` (no-op if already assigned) - branch from fresh `main` (prefix from labels: `fix/`, `feat/`, `docs/`) - strict TDD: RED → GREEN → REFACTOR (no production code without a failing test first) - full suite green locally (`./bashunit tests/` **and** `./bashunit --parallel tests/`) - `CHANGELOG.md` entry under `## Unreleased` for user-facing changes - commit with conventional message + `Closes #` in the body - - PR opened via `/pr #` + - PR opened via `pr #` 3. **Before pushing**, run the exact command CI's strict job uses, plain sequential runs miss simple-mode and strict-mode bugs that CI rejects: @@ -99,7 +99,7 @@ For each issue in the queue: arrays with `=()` and read elements as `${arr[i]:-}`, and never enable `shopt -s extdebug` in the parent shell (isolate it inside a `$()` subshell). -4. After `/gh-issue` returns, wait for CI green on the PR: +4. After `gh-issue` returns, wait for CI green on the PR: ```bash gh pr checks --watch ``` @@ -112,7 +112,7 @@ For each issue in the queue: gh pr merge --auto --squash --admin ``` -6. **Close the issue if the squash-merge did not.** GitHub builds the squash commit from the **PR body**, and `/pr` writes `Related #` there (never `Closes`), so the `Closes #` in the branch commit body is lost. After merge: +6. **Close the issue if the squash-merge did not.** GitHub builds the squash commit from the **PR body**, and `pr` writes `Related #` there (never `Closes`), so the `Closes #` in the branch commit body is lost. After merge: ```bash gh issue view --json state -q .state # still OPEN? gh issue close --reason completed -c "Done in # (merged)." @@ -137,7 +137,7 @@ For each issue in the queue: Halt the loop and surface the failure when: -- `/gh-issue` errors out or leaves the worktree dirty. +- `gh-issue` errors out or leaves the worktree dirty. - `./bashunit tests/` or `./bashunit --parallel tests/` fails after implementation (for the right reason, distinguish pre-existing failures; see Notes). - `make sa` (ShellCheck) or `make lint` (EditorConfig) fails. - CI stays red after one fix attempt. @@ -156,11 +156,11 @@ With `--dry-run`, only execute Phase 1 and print the queue. No assignment, no br - `gh` authenticated, can read issues, open and merge PRs. - Worktree clean. - `main` exists and tracks `origin/main`. -- `/gh-issue` and `/pr` skills available in this session. +- `gh-issue` and `pr` skills available in this session. ## Notes -- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.agnostic-ai/rules/bash-style.md`. +- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.agnostic-ai/context/bash-style.md`. - **Quality gate is `make sa` + `make lint`, not bare `shfmt`.** `shfmt -w .` without project flags rewrites the whole tree (collapses line-continuations, flips binary-op style, tabs the `indent_size = unset` files). Match the surrounding 2-space style by hand and rely on `make lint` (EditorConfig) to verify. There is no `shfmt` make target. - **Know your baseline.** `tests/unit/coverage_subshell_test.sh` fails on some macOS setups regardless of the change, confirm a failure is new (`git stash` + re-run) before treating it as a regression. - Treat GitHub CI as the full quality gate; locally run focused tests during implementation, the full sequential + parallel suite once before commit. diff --git a/.agnostic-ai/skills/pr/SKILL.md b/.agnostic-ai/skills/pr/SKILL.md index 343dd30aa..a2e29dc70 100644 --- a/.agnostic-ai/skills/pr/SKILL.md +++ b/.agnostic-ai/skills/pr/SKILL.md @@ -10,43 +10,21 @@ allowed-tools: shell, read, Grep, Glob Push branch and create a PR with a concise, issue-linked description. -> **IMPORTANT:** This skill MUST be used for ALL pull request creation, even when the user says "create pr" without `/pr`. Never create a PR without following these steps. +> **IMPORTANT:** This skill MUST be used for ALL pull request creation, even when the user says "create pr" without `pr`. Never create a PR without following these steps. ## Current Branch Context -::target claude -- Branch: !`git branch --show-current` -::end - -::target codex -- Run Branch: `git branch --show-current` -::end -::target claude -- Commits: !`git log main..HEAD --oneline 2>/dev/null` -::end - -::target codex -- Run Commits: `git log main..HEAD --oneline 2>/dev/null` -::end -::target claude -- Changed files: !`git diff main..HEAD --stat 2>/dev/null` -::end - -::target codex -- Run Changed files: `git diff main..HEAD --stat 2>/dev/null` -::end -## Arguments -::target claude -- `$ARGUMENTS` - Issue reference (optional, e.g., `#42` or `42`). If provided, the PR will be linked to this issue. -::end +Run the following commands before proceeding: +- Branch: `git branch --show-current` +- Commits: `git log main..HEAD --oneline 2>/dev/null` +- Changed files: `git diff main..HEAD --stat 2>/dev/null` -::target codex -- the argument in the user request - Issue reference (optional, e.g., `#42` or `42`). If provided, the PR will be linked to this issue. -::end +## Arguments +- User-supplied argument: Issue reference (optional, e.g., `#42` or `42`). If provided, the PR will be linked to this issue. ## Instructions -1. **Review the branch context above**, the commits and changed files are already loaded. +1. **Review the branch context above**, the commits and changed files come from the commands above. 2. **MANDATORY: Update CHANGELOG.md**, Read `CHANGELOG.md` and check the `## Unreleased` section. If the changes from this branch are NOT already listed there, you MUST update it before proceeding. **Do NOT skip this step. Do NOT proceed to push without verifying.** - Add entries under the appropriate subsection (`### Added`, `### Changed`, `### Fixed`, etc.) @@ -64,13 +42,7 @@ Push branch and create a PR with a concise, issue-linked description. - If push fails, read the output, fix the issue, commit the fix, and retry. Do NOT use `--no-verify` to bypass. 4. **Generate PR title**: -::target claude - - If `$ARGUMENTS` contains an issue number, fetch the issue title: -::end - -::target codex - - If the argument in the user request contains an issue number, fetch the issue title: -::end + - If the user-supplied argument contains an issue number, fetch the issue title: ```bash gh issue view --json title -q '.title' ``` @@ -116,7 +88,7 @@ Push branch and create a PR with a concise, issue-linked description. 6. **Move issue to "In Review"** in GitHub Project (if issue number provided): ```bash - # Read project config from .claude/github-project.json + # Read project config from .agnostic-ai/local/github-project.json ITEM_ID=$(gh project item-list PROJECT_NUMBER --owner OWNER --format json \ | jq -r '.items[] | select(.content.number == ISSUE_NUMBER) | .id') @@ -133,8 +105,4 @@ Push branch and create a PR with a concise, issue-linked description. ## Example Usage -``` -/pr -/pr #42 -/pr 15 -``` +Use the `pr` skill alone, or pass an issue number such as `42` or `15`. diff --git a/.agnostic-ai/skills/release/SKILL.md b/.agnostic-ai/skills/release/SKILL.md index 245218e7c..f223ad582 100644 --- a/.agnostic-ai/skills/release/SKILL.md +++ b/.agnostic-ai/skills/release/SKILL.md @@ -15,34 +15,12 @@ here, fix `release.sh` if something is missing. ## Current State -::target claude -- Current version: !`grep -o 'BASHUNIT_VERSION="[^"]*"' bashunit | cut -d'"' -f2` -::end - -::target codex -- Run Current version: `grep -o 'BASHUNIT_VERSION="[^"]*"' bashunit | cut -d'"' -f2` -::end -::target claude -- Branch: !`git branch --show-current` -::end - -::target codex -- Run Branch: `git branch --show-current` -::end -::target claude -- Working tree: !`git status --short` -::end - -::target codex -- Run Working tree: `git status --short` -::end -::target claude -- Unreleased changes: !`awk '/^## Unreleased$/,/^## \[/' CHANGELOG.md | head -30` -::end - -::target codex -- Run Unreleased changes: `awk '/^## Unreleased$/,/^## \[/' CHANGELOG.md | head -30` -::end +Run the following commands before proceeding: + +- Current version: `grep -o 'BASHUNIT_VERSION="[^"]*"' bashunit | cut -d'"' -f2` +- Branch: `git branch --show-current` +- Working tree: `git status --short` +- Unreleased changes: `awk '/^## Unreleased$/,/^## \[/' CHANGELOG.md | head -30` ## Steps @@ -58,13 +36,7 @@ Stop and report if anything fails. Don't release on a red main. ### 2. Pick the version -::target claude -`$ARGUMENTS` overrides; otherwise the script auto-increments the minor. -::end - -::target codex -the argument in the user request overrides; otherwise the script auto-increments the minor. -::end +the user-supplied argument overrides; otherwise the script auto-increments the minor. Bump by the Unreleased section: a `### Added`/feat → minor, only `### Fixed` → patch. Confirm the version with the user before publishing. diff --git a/.agnostic-ai/skills/review/SKILL.md b/.agnostic-ai/skills/review/SKILL.md index ff1825ad1..3ac8be6b1 100644 --- a/.agnostic-ai/skills/review/SKILL.md +++ b/.agnostic-ai/skills/review/SKILL.md @@ -10,43 +10,21 @@ allowed-tools: shell, read, Grep, Glob Review a GitHub PR. When no number is given, review the PR for the current branch. -> For reviewing your uncommitted working diff instead, use `/code-review`. +> For reviewing your uncommitted working diff instead, assign that diff to the `code-reviewer` agent. ## Current Branch Context -::target claude -- Branch: !`git branch --show-current` -::end -::target codex -- Run Branch: `git branch --show-current` -::end -::target claude -- PR for this branch: !`gh pr view --json number,title,state,url --jq '"#\(.number) \(.title) [\(.state)] \(.url)"' 2>/dev/null || echo "none"` -::end - -::target codex -- Run PR for this branch: `gh pr view --json number,title,state,url --jq '"#\(.number) \(.title) [\(.state)] \(.url)"' 2>/dev/null || echo "none"` -::end +Run the following commands before proceeding: +- Branch: `git branch --show-current` +- PR for this branch: `gh pr view --json number,title,state,url --jq '"#\(.number) \(.title) [\(.state)] \(.url)"' 2>/dev/null || echo "none"` ## Arguments -::target claude -- `$ARGUMENTS` - PR number to review (optional, e.g., `#42` or `42`). -::end - -::target codex -- the argument in the user request - PR number to review (optional, e.g., `#42` or `42`). -::end +- User-supplied argument: PR number to review (optional, e.g., `#42` or `42`). ## Instructions 1. **Resolve which PR to review:** -::target claude - - If `$ARGUMENTS` contains a number, review that PR. -::end - -::target codex - - If the argument in the user request contains a number, review that PR. -::end + - If the user-supplied argument contains a number, review that PR. - Otherwise, use the current branch's PR shown in the context above. - If neither yields a PR (no argument and the branch has no open PR), run `gh pr list` and ask the user which one to review. Do NOT guess. @@ -56,7 +34,7 @@ Review a GitHub PR. When no number is given, review the PR for the current branc gh pr diff ``` -3. **Review the diff** against this project's standards (see `.agnostic-ai/AGNOSTIC_AI.md` and `.agnostic-ai/rules/`): +3. **Review the diff** against this project's standards (see `.agnostic-ai/AGNOSTIC_AI.md` and `.agnostic-ai/context/`): - Bash 3.0+ compatibility (no `declare -A`, `[[ ]]`, `${var,,}`, negative indexing, `&>>`) - Tests exist and follow TDD; both success and failure paths covered - Naming, namespacing (`bashunit::*` / `_private`), and dynamic-scope safety diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 228955fe5..4b61fb960 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -68,9 +68,9 @@ On Windows, use the [Windows installer](https://agnostic-ai.org/docs/installatio Edit `.agnostic-ai/` or `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Generated files are ignored. Run sync after cloning and after pulling changes to the specs. Keep personal overrides in `.agnostic-ai/local/` or `agnostic-ai.local.yaml`, which are also ignored. -The root instructions list the detailed rules to read for each task. Claude keeps its automatic path selectors and existing ShellCheck hook. Codex reads the same rule sources on demand to keep its initial context small. Skills keep Claude's automatic command snippets and provide explicit command instructions for Codex. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. +The root instructions list the detailed rules to read for each task. Short shared rules point to detailed guides under `.agnostic-ai/context/`, which agents read only when relevant. The root task table also works for sessions whose client cannot automatically load scoped rules. A portable after-edit hook extracts edited paths from either tool and runs ShellCheck on shell files. Skills use explicit commands and request arguments in a shared body. Hook commands require agnostic-ai and ShellCheck on the client's PATH. Review and trust the generated hook through `/hooks` in Codex before relying on automatic checks. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. -The default model tier is `balanced`: Sonnet for Claude and `gpt-6.1-sol` at medium effort for Codex. `legolas` locates code on Haiku or `gpt-6-luna` at low effort, `aragorn` implements bounded changes on the balanced tier, and `gandalf` reviews on Opus or `gpt-6.1-sol` at high effort. These are task assignments, not speed or quality benchmarks. Codex permits at most two concurrent delegates. The coordinator runs the full gate once after all edits. Read-only agent prompts prohibit writes; Codex agents inherit the session sandbox, so these prompts do not impose a separate filesystem restriction. Override model tiers locally when a model is unavailable in your account; do not commit personal model or permission choices. +The default model tier is `balanced`: Sonnet for Claude and `gpt-6.1-sol` at medium effort for Codex. `code-locator` locates code on Haiku or `gpt-6-luna` at low effort, `code-implementer` implements bounded changes on the balanced tier, and `code-reviewer` reviews on Opus or `gpt-6.1-sol` at high effort. These are task assignments, not speed or quality benchmarks. Codex permits at most two concurrent delegates. The coordinator runs the full gate once after all edits. Lookup and review agents prohibit writes in their prompts and request only read and shell capabilities. Each client applies its own capability support and session sandbox; these prompts do not impose a separate filesystem restriction. Override model tiers locally when a model is unavailable in your account; do not commit personal model or permission choices. ### Documentation Setup (Optional) diff --git a/.github/workflows/check-agent-specs.yml b/.github/workflows/check-agent-specs.yml index aed127c21..d5cd31a09 100644 --- a/.github/workflows/check-agent-specs.yml +++ b/.github/workflows/check-agent-specs.yml @@ -40,3 +40,27 @@ jobs: agnostic-ai sync agnostic-ai sync --check git diff --exit-code + - name: Verify shared ShellCheck hook + run: | + python3 - <<'PY' + import json + import subprocess + import tempfile + from pathlib import Path + + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "edited shell.sh" + path.write_text("#!/usr/bin/env bash\necho $1\n") + result = subprocess.run( + [ + "agnostic-ai", "hook", "run", "shellcheck-after-edit", + "--edit", str(path), "--format", "json", + ], + check=True, capture_output=True, text=True, + ) + targets = json.loads(result.stdout)["targets"] + assert {target["target"] for target in targets} == {"claude", "codex"} + for target in targets: + assert target["counted"] and not target["warnings"], target + assert any("SC2086" in command["stdout"] for command in target["commands"]), target + PY diff --git a/.gitignore b/.gitignore index 29c85d4a1..55e003ca9 100644 --- a/.gitignore +++ b/.gitignore @@ -43,6 +43,7 @@ dev.log /.agnostic-ai/packs/ /.claude/agent-memory-local/ /.claude/agents/ +/.claude/hooks/ /.claude/rules/ /.claude/scheduled_tasks.lock /.claude/settings.json @@ -51,6 +52,8 @@ dev.log /.claude/worktrees/ /.codex/agents/ /.codex/config.toml +/.codex/hooks.json +/.codex/hooks/ /AGENTS.md /CLAUDE.md /agnostic-ai.local.yaml diff --git a/.worktreeinclude b/.worktreeinclude index 7489f57a9..b029198cc 100644 --- a/.worktreeinclude +++ b/.worktreeinclude @@ -6,12 +6,15 @@ /.agnostic-ai/packs/ /.claude/agent-memory-local/ /.claude/agents/ +/.claude/hooks/ /.claude/rules/ /.claude/settings.json /.claude/settings.local.json /.claude/skills/ /.codex/agents/ /.codex/config.toml +/.codex/hooks.json +/.codex/hooks/ /AGENTS.md /CLAUDE.md /agnostic-ai.local.yaml From dbeb15546d3c06f44c673c56821457feac1cd91d Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 14:05:45 +0200 Subject: [PATCH 3/5] refactor: use standard agent spec layout --- .agnostic-ai/AGNOSTIC_AI.md | 147 +++------------ .agnostic-ai/context/architecture-map.md | 156 ---------------- .agnostic-ai/context/bash-style.md | 168 ----------------- .agnostic-ai/context/testing.md | 130 ------------- .agnostic-ai/rules/architecture-map.md | 161 +++++++++++++++- .agnostic-ai/rules/bash-style.md | 173 +++++++++++++++++- .agnostic-ai/rules/perf-fork-budget.md | 15 +- .agnostic-ai/rules/tdd-workflow.md | 8 +- .agnostic-ai/rules/testing.md | 134 +++++++++++++- .agnostic-ai/skills/commit/SKILL.md | 2 + .../skills/commit/references/formatting.md | 20 ++ .agnostic-ai/skills/fork-census/SKILL.md | 15 ++ .../fork-census/references/fork-budget.md} | 0 .agnostic-ai/skills/gh-issue/SKILL.md | 2 +- .agnostic-ai/skills/gh-issues/SKILL.md | 2 +- .agnostic-ai/skills/review/SKILL.md | 2 +- .agnostic-ai/skills/tdd-cycle/SKILL.md | 19 +- .../tdd-cycle/references/workflow.md} | 0 .github/CONTRIBUTING.md | 2 +- .gitignore | 3 + .worktreeinclude | 3 + CHANGELOG.md | 1 + tests/unit/project/bash_compatibility_test.sh | 28 ++- 23 files changed, 578 insertions(+), 613 deletions(-) delete mode 100644 .agnostic-ai/context/architecture-map.md delete mode 100644 .agnostic-ai/context/bash-style.md delete mode 100644 .agnostic-ai/context/testing.md create mode 100644 .agnostic-ai/skills/commit/references/formatting.md create mode 100644 .agnostic-ai/skills/fork-census/SKILL.md rename .agnostic-ai/{context/perf-fork-budget.md => skills/fork-census/references/fork-budget.md} (100%) rename .agnostic-ai/{context/tdd-workflow.md => skills/tdd-cycle/references/workflow.md} (100%) diff --git a/.agnostic-ai/AGNOSTIC_AI.md b/.agnostic-ai/AGNOSTIC_AI.md index af15838db..3121600f7 100644 --- a/.agnostic-ai/AGNOSTIC_AI.md +++ b/.agnostic-ai/AGNOSTIC_AI.md @@ -1,141 +1,46 @@ -# bashunit - Bash Testing Framework +# bashunit -## Project Overview +bashunit is a Bash 3.0+ testing framework with assertions, spies, mocks, data providers, and snapshots. Documentation: https://bashunit.com. -**bashunit** is a lightweight Bash testing framework (Bash 3.0+) focused on developer experience. Provides assertions, test doubles (spies/mocks), data providers, snapshots, and more. +## Required guidance -**Documentation:** https://bashunit.com +All paths are relative to the repository root, including from generated skill and agent directories. Read the applicable source rules before editing. Directory scopes generate native instructions for that directory. Sessions started at the root must also read the relevant sources below; some clients do not load nested instructions during later edits. -## Core Principles - -### TDD by Default -**RED → GREEN → REFACTOR**, every change starts from a failing test. No exceptions. - -### Bash 3.0+ Compatible - -Works on macOS default bash (3.2) and older. **Prohibited features:** -- `printf -v` (Bash 3.1+) -- `x+=y` string append assignment (Bash 3.1+) -- `arr+=(x)` array append (a Bash 3.0 **parse** error, it kills the file even in dead code) -- `declare -A` (associative arrays - Bash 4.0+) -- `[[ ]]` (use `[ ]` instead, house style, not a version limit) -- `[[ =~ ]]` (exists on 3.0, but 3.2 changed quoted-pattern semantics) -- `${var,,}` (case conversion - Bash 4.0+) -- `${array[-1]}` (negative indexing - Bash 4.3+) -- `&>>` redirect (Bash 4.0+) - -See `.agnostic-ai/context/bash-style.md` for the complete compatibility guide. - -### Quality Standards - -Every change must pass: -```bash -make sa # ShellCheck static analysis -make lint # EditorConfig linting (the formatting authority, 2-space indent) -./bashunit tests/ # All tests passing -``` - -**Never run `shfmt -w`.** shfmt and `.editorconfig` disagree on this repo: 146 -tracked files are already shfmt-dirty on a clean `main`, and satisfying shfmt -breaks `make lint`. See "Formatting" below. +| Task | Read | +|------|------| +| Changes to `src/` or `bashunit` | `.agnostic-ai/rules/architecture-map.md` | +| Shell changes under `src/` or `tests/`, including entrypoint changes | `.agnostic-ai/rules/bash-style.md` | +| Changes to `src/`, `bashunit`, or fork census acceptance tests | `.agnostic-ai/rules/perf-fork-budget.md` | +| Runtime development or task plans | `.agnostic-ai/rules/tdd-workflow.md` | +| Test changes | `.agnostic-ai/rules/testing.md` | -## Formatting +Bash 3.0 is the compatibility floor, including parse-time behavior. Use existing patterns. Study the relevant tests before adding assertions, doubles, providers, hooks, or CLI behavior. Do not invent commands or features. -Match the surrounding 2-space style by hand and verify with `make lint`. There is -no `shfmt` target, and **`shfmt -w .` must not be run**, it fights `.editorconfig` -two ways: +Use TDD for runtime changes: RED for the right reason, minimal GREEN, then REFACTOR with tests green. Complete the acceptance criteria. Use a task file for nontrivial work; the `tdd-cycle` skill carries the full workflow. Preserve public APIs unless the requested change includes documentation and CHANGELOG updates. Record architectural decisions in an ADR. -- `.editorconfig` sets `indent_size = unset` for `tests/acceptance/**.sh` and - `src/console_header.sh`; shfmt reads `unset` as "use my default", which is tabs, - while `[*]` sets `indent_style = space`. Any new file under `tests/acceptance/` - therefore shows up shfmt-dirty, that is expected, not a defect. -- shfmt collapses `\`-continued strings onto one line, which would push - `src/state.sh`'s record writer past `max_line_length = 120`. +## Quality gate -To check you introduced no new drift, compare the dirty-file *list* against main -rather than requiring it to be empty: +Run once after the complete change set, before committing: ```bash -git ls-files -z '*.sh' bashunit | xargs -0 shfmt -d 2>/dev/null \ - | grep '^--- ' | sed 's|--- ||; s|\.orig.*||' | sort +./bashunit tests/ +./bashunit --parallel tests/ +make sa +make lint ``` -## Test Patterns - -Study existing tests before writing new ones: - -- **Assertions**: `tests/unit/assert_test.sh` -- **Test Doubles**: `tests/functional/doubles_test.sh` -- **Data Providers**: `tests/functional/provider_test.sh` -- **Lifecycle Hooks**: `tests/unit/setup_teardown_test.sh` -- **CLI Testing**: `tests/acceptance/bashunit_test.sh` - -## Task Context - -All paths below are relative to the repository root, including from a generated skill or agent directory. Read only the guides relevant to the task before editing. Rules select the matching tasks and point to these guides. Follow the table even when the client cannot load path-scoped rules automatically. - -| Task | Read | -|------|------| -| Changes to `src/` or `bashunit` | `.agnostic-ai/context/architecture-map.md` | -| Shell changes under `src/` or `tests/` | `.agnostic-ai/context/bash-style.md` | -| Changes to `src/` or fork census acceptance tests | `.agnostic-ai/context/perf-fork-budget.md` | -| Shell development or task plans | `.agnostic-ai/context/tdd-workflow.md` | -| Test changes | `.agnostic-ai/context/testing.md` | +`make lint` is the formatting authority. Match surrounding 2-space indentation by hand. Never run `shfmt -w`; it conflicts with EditorConfig. The `commit` skill bundles the detailed formatting constraints and optional drift comparison. ## Delegation -Keep one acceptance scenario and a short plan in the main session. Handle small edits directly. Delegate independent lookups to `code-locator`, a bounded implementation to `code-implementer`, and defect review to `code-reviewer` before pushing. Give each agent a specific question or file scope, a base revision, and a validation budget. Reuse its findings instead of repeating its searches. - -Keep at most two delegates active. Workers share the checkout: assign separate files and never revert another worker's edits. Run the full quality gate once in the main session after the change set is complete. Delegates run only a targeted check when needed to prove a finding. Return concise evidence with file and line references; put longer reports under the ignored `.agnostic-ai/local/` directory. - -## Editing Instructions - -Edit `.agnostic-ai/` and `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Native instruction files, skills, agents, and settings are generated and ignored by Git. Never edit them directly. - -## Guardrails - -### Never: -- Invent commands/features not in the codebase -- Break Bash 3.0+ compatibility -- Skip tests or quality checks -- Change public API without docs/CHANGELOG -- Commit without tests passing -- Batch unrelated changes in one PR -- Create a PR without using the `pr` skill - -### Always: -- Write tests before implementation -- Use existing patterns from `tests/**` and `src/**` -- Minimal code in GREEN phase -- Keep tests passing during REFACTOR -- Update CHANGELOG.md for user-visible changes -- Run quality checks before committing -- Maintain Bash 3.0+ compatibility - -## Definition of Done - -- All tests green for the **right reason** -- `make sa` passes (ShellCheck) -- `make lint` passes (EditorConfig) -- Code formatted to 2-space indent by hand (verified by `make lint`, never `shfmt -w`) -- Bash 3.0+ compatible -- Parallel tests passing (`./bashunit --parallel tests/`) -- CHANGELOG.md updated (if user-facing changes) -- ADR created/updated (if architectural decision) +Handle small edits directly. Keep one acceptance scenario and a short plan in the main session. Delegate independent lookups to `code-locator`, bounded edits to `code-implementer`, and verified defect review to `code-reviewer` before pushing. Assign a question or file scope, a base revision, and a validation budget. Reuse findings. Keep at most two delegates active. -## Commit Message Format +Workers share the checkout: assign separate files, preserve others' edits, and run only assigned targeted checks. The coordinator runs the full gate. Return concise file and line evidence; save longer reports under ignored `.agnostic-ai/local/`. -[Conventional Commits](https://conventionalcommits.org/): `(): ` +## Specs and delivery -**Types:** `feat`, `fix`, `docs`, `style`, `refactor`, `test`, `chore` -**Scopes:** `assert`, `runner`, `cli`, `docs`, etc. +Edit `.agnostic-ai/` or `agnostic-ai.yaml`, never generated instructions, agents, skills, hooks, or settings. Run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. -## Prohibited Actions +Keep unrelated changes out of the PR. Update CHANGELOG for user-visible changes. Use the `pr` skill for pull requests. Use Conventional Commits: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, or `chore`, with an optional domain scope such as `assert`, `runner`, `cli`, or `docs`. -**Never without explicit user request:** -- Commit secrets or sensitive data -- Force push to main/master -- Skip git hooks (--no-verify) -- Amend published commits -- Use destructive git commands (reset --hard, clean -f) -- Push to remote without confirmation +Never commit secrets, skip Git hooks, amend published commits, or use destructive Git commands. Pushing requires user authorization; force-pushing main requires an explicit request. All local gates must pass before committing. diff --git a/.agnostic-ai/context/architecture-map.md b/.agnostic-ai/context/architecture-map.md deleted file mode 100644 index 5b2673359..000000000 --- a/.agnostic-ai/context/architecture-map.md +++ /dev/null @@ -1,156 +0,0 @@ -# Architecture Map: modules and the life of a run - -Orientation for working in `src/`, which file owns what, and the exact call -flow of a test run. Line numbers drift; function names are the stable anchors. - -## The life of `./bashunit tests/` - -``` -bashunit (entry) sources all src/*.sh; version gate; early flag scan -└─ bashunit::main::cmd_test (main.sh: flag parsing, env exports) - └─ bashunit::runner::load_test_files (runner/discovery.sh: the per-file loop) - ├─ console_header::print_header "Running N tests", captures - │ └─ helper::find_total_tests $() SUBSHELL: sources each file in a - │ nested subshell just to count tests - ├─ source "$test_file" in the MAIN shell (workers inherit) - ├─ helper::check_duplicate_functions one awk pass per file - ├─ runner::functions_for_script pure bash: filter + sort by def line - ├─ helper::build_provider_map one awk pass per file (cached by path, - │ but the count subshell's build dies - │ with it → runs twice per file) - └─ runner::call_test_functions per-fn loop - └─ runner::run_test [--parallel: spawned as a & worker per test] - ├─ clock::now_to_slot duration start (if needed) - ├─ runner::execute_test_body $() subshell: the actual test fn - │ └─ state::export_subshell_context printf payload "##K=V##…" - ├─ runner::parse_result sync: parse counts from payload - │ parallel: + write .result via mktemp - └─ console_results::print_successful_test / _failed_ / … - └─ str::rpad + strip_ansi align per-test time (pure bash) - └─ [--parallel] wait; state::aggregate_parallel_results over *.result files - └─ console_results::render_result totals; deferred failure/skip blocks - └─ rerun::persist; env::cleanup_run_output_dir; exit code -``` - -Key inversion to remember: **tests run inside `$()` subshells; state comes back -as an encoded single-line payload** (`##ASSERTIONS_FAILED=…##TEST_OUTPUT=##`), -parsed by `runner::parse_result`/`state.sh`. Counters only exist in the main -shell (or, in parallel, in per-test `.result` files aggregated at the end). - -## Module ownership - -| Module | Owns | -|--------|------| -| `bashunit` | entry: version gate, early flag scan, sources every module, subcommand routing | -| `main/index.sh` | aggregator only, sources the `src/main/` module below | -| `main/validate.sh` | shared parser validation: unknown options, integer/path checks, shard parsing | -| `main/test.sh` / `main/bench.sh` / `main/subcommands.sh` | flag parsing per subcommand | -| `main/assert.sh` | the standalone `bashunit assert` path | -| `main/watch.sh` / `main/run.sh` | the `--watch` loop; the run lifecycle, exit codes and cleanup | -| `runner/index.sh` | aggregator only, sources the `src/runner/` module below | -| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes | -| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload | -| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers | -| `runner/result.sh` | `parse_result{,_sync,_parallel}`, failure source context, failed/skipped/incomplete/risky writers | -| `runner/parallel.sh` | job-slot waiting (`wait -n` or poll), running-job count, spinner | -| `runner/hooks.sh` | set_up/tear_down (test + script scope), hook failure records, mock clearing, EXIT cleanup | -| `runner/provider.sh` | `@data_provider` argument parsing | -| `runner/exec.sh` | `run_test`, the capture-subshell body, retry, timeout watchdog, per-file dispatch, `order_functions_for_script` (the run order a seed produces) | -| `runner/list.sh` | `--list`/`--dry-run`: emits selected tests instead of running them; hooks in after all filtering, before any hook | -| `runner/discovery.sh` | `load_test_files` (the per-file loop), `functions_for_script` | -| `runner/bench.sh` | benchmark file loop and bench function dispatch | -| `helper/index.sh` | aggregator only, sources the `src/helper/` module below | -| `helper/naming.sh` | resolving and normalising test-function and variable names | -| `helper/discovery.sh` | `find_files_recursive`, `find_total_tests`, fn filtering, duplicate check, line lookup | -| `helper/provider.sh` / `helper/tags.sh` | `@data_provider` map; `@tag` extraction and matching | -| `helper/encoding.sh` / `helper/functions.sh` / `helper/git.sh` | base64 + ids; generic fn utilities; remote tag lookup for `upgrade` | -| `state/index.sh` | aggregator only, sources the `src/state/` module below | -| `state/counters.sh` | test and assertion tallies and their get/add accessors | -| `state/duplicates.sh` | duplicate test-function detection state | -| `state/context.sh` | per-test output buffer, exit code, title, hook failure, and the per-test reset | -| `state/payload.sh` | the encoded per-test result payload and the base64 capability probe | -| `state/parallel.sh` | aggregating per-test result files after a `--parallel` run | -| `config/index.sh` | aggregator only, sources the `src/config/` module below | -| `config/env.sh` | all `BASHUNIT_*` defaults/config files, scratch dirs (`_BASHUNIT_RUN_OUTPUT_DIR` + EXIT-trap cleanup); executes at source time | -| `config/rerun.sh` | the `.bashunit/last-failed` cache for `--rerun-failed` | -| `config/parallel.sh` | is this run parallel; worker temp tree and stop-on-failure flag file (not `runner/parallel.sh`, which waits on job slots) | -| `console/index.sh` | aggregator only, sources the `src/console/` module below | -| `console/colors.sh` | the `_BASHUNIT_COLOR_*` palette and `bashunit::sgr` | -| `console/header.sh` | the "Running N tests" header | -| `console/line.sh` | `print_line`, the primitive every result line goes through, and its TAP variant | -| `console/duration.sh` / `console/diff.sh` | duration formatting; unified and line-by-line diffs under a failure | -| `console/test_line.sh` | the per-test result lines (passed/failed/skipped/incomplete/snapshot/risky/error) | -| `console/deferred.sh` | end-of-run blocks buffered during the run (scratch files under the run dir) | -| `console/summary.sh` | run totals, execution time, hook completion | -| `assert/index.sh` | aggregator only, sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | -| `assert/core.sh` | `assert::should_skip`, `assert::fail_with`, `assert::join_to_slot` and the comparison assertions the other files build on | -| `assert/{arrays,assertions,dates,duration,files,folders,json,once,snapshot}.sh` | the per-topic assertions; the per-assertion path must stay fork-free | -| `util/index.sh` | aggregator only, sources the `src/util/` module below | -| `util/clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads | -| `util/str.sh` / `util/math.sh` | pure-bash string and arithmetic helpers | -| `api/index.sh` | aggregator only, sources the `src/api/` module below | -| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone, the entrypoint already set it | -| `api/skip_todo.sh` / `api/test_title.sh` | `skip`/`todo` markers; `set_test_title` | -| `api/bashunit.sh` | the custom-assert facade (`assert_that`, `assertion_failed`/`passed`) | -| `doubles/index.sh` | aggregator only, sources the `src/doubles/` module below | -| `doubles/mock.sh` | `mock`/`unmock` and the `_BASHUNIT_MOCKED_FUNCTIONS` registry `runner/hooks.sh` unwinds per test | -| `doubles/spy.sh` | `spy` and its `_BASHUNIT_SPY_*` state slots + files | -| `doubles/assertions.sh` | the `assert_have_been_called*` family | -| `coverage/index.sh` | aggregator only, sources the `src/coverage/` modules below | -| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | -| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | -| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | -| `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | -| `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | -| `coverage/functions.sh` | function definitions and their line spans | -| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose**, the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | -| `coverage/report_text.sh` / `report_lcov.sh` / `report_html.sh` | the three renderers | -| `coverage/html_index.sh` / `html_file.sh` | HTML page emitters; the only two files exempt from `max_line_length` | - -| `reports.sh` | JUnit/HTML/TAP/JSON writers | -| `system/index.sh` | aggregator only, sources the `src/system/` module below | -| `system/check_os.sh` / `system/dependencies.sh` | one-fork OS detect; `command -v` probes (builtins, not forks) | -| `system/io.sh` | small I/O helpers | -| `cli/index.sh` | aggregator only, sources the `src/cli/` module below | -| `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; `main.sh` is their only caller | -| `benchmark/index.sh` | the remaining non-`test` subcommand; shared with `runner/bench.sh` | - -## Cross-cutting invariants - -- **Bash 3.0 floor** (`.agnostic-ai/rules/bash-style.md`): no `[[`, `declare -A`, - `${var,,}`, `BASHPID`, negative indices. Subshells share `$$`, you cannot - make a per-worker unique token without a fork (`mktemp`) or an ordinal - assigned before the fork. `RANDOM` is no help either: whether a subshell - reseeds it depends on the platform and the nesting depth (measured in - perf-fork-budget.md), so it is unreliable in both directions (#1354). -- **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` - captures on hot paths, bash-style.md documents it; `local` is dynamically - scoped, so helpers must not write caller-named variables. -- **Fork budget** (`.agnostic-ai/rules/perf-fork-budget.md`): per-test paths are - fork-free (sequential) or mktemp-only (parallel); budgets are enforced by - `tests/acceptance/bashunit_*forks*_test.sh` on three platforms, a "harmless" - `echo | sed` in a per-test path will fail CI. -- **`call_test_functions` is a background subshell under `--parallel`** - (`discovery.sh` backgrounds it per file). Anything it sets in shell state, - counters, flags, error records, dies with the worker. Only two things cross - back: per-test `.result` payloads, and files written to disk. This produced - two silent-green bugs in one day: a data-provider error printed while the run - exited 0 (#1145), and duplicate-test-function detection being entirely off in - the mode CI uses (#1147). Before adding a check anywhere under that call, ask - which side of the fork it belongs on, a whole-file check almost always - belongs in the parent loop, before dispatch. - **Verify both modes.** Sequential and `--parallel` disagreeing is the - signature of this bug, and it is invisible to a sequential-only test. -- **Binaries pinned at startup** (`$GREP`, `$MKTEMP`, `$CAT` in env.sh) so test - doubles/PATH games can't hijack the framework's own plumbing. -- **Snapshots assume 80-col non-tty width** (`tput cols` fallback); anything - that changes rendering widths breaks `tests/acceptance/snapshots/`. -- **The build flattens the source graph in DFS order** (`build.sh` - `build::process_file`): a file's body is emitted, *then* its `source` lines are - recursed into. That equals dev-mode order only if a module aggregator - (`src//index.sh`, ADR-010) contains **nothing but `source` - lines and comments**, any other top-level statement would run before its - dependencies in the built artifact but after them in dev mode. Files are - deduped by repo-relative path, so `src/` may hold module dirs and two files may - share a basename. Every src file's **first line must be the shebang**: - `tail -n +2` strips it when embedding. diff --git a/.agnostic-ai/context/bash-style.md b/.agnostic-ai/context/bash-style.md deleted file mode 100644 index 054e58e63..000000000 --- a/.agnostic-ai/context/bash-style.md +++ /dev/null @@ -1,168 +0,0 @@ -# Bash Style & Compatibility Rules - -## Bash 3.0+ Compatibility (Critical) - -bashunit must work on **Bash 3.0+**. The floor was deliberately lowered from 3.2 -to 3.0 in 0.33 to reach older systems; macOS's 3.2 is the common case, not the -floor. The runtime gate in `./bashunit` compares major **and** minor against -`BASHUNIT_MIN_BASH_VERSION`, so the declared floor is the enforced one. - -These are **version-breaking** above the floor and are prohibited. They are -enforced mechanically by `tests/unit/bash_compatibility_test.sh`, which greps -`src/` regardless of test coverage: - -| Feature | Bash ver | Alternative | -|---------|----------|-------------| -| `printf -v` | 3.1+ | Return-slot pattern (below) | -| `x+=y` (string append) | 3.1+ | `var="$var$more"` | -| `arr+=(x)` (array append) | 3.1+, and a 3.0 **parse** error | `arr[${#arr[@]}]=x` | -| `declare -A` (associative arrays) | 4.0+ | Parallel indexed arrays | -| `${var,,}` / `${var^^}` (case) | 4.0+ | `tr '[:upper:]' '[:lower:]'` | -| `${array[-1]}` (negative index) | 4.3+ | `${array[${#array[@]}-1]}` | -| `&>>` (append both) | 4.0+ | `>> file 2>&1` | -| `BASHPID` | 4.0+ | fork (`mktemp`) or an assigned ordinal | -| `mapfile` / `readarray` | 4.0+ | `while IFS= read -r` | -| `declare -n` / `local -n` (nameref) | 4.3+ | Return-slot pattern (below) | -| `coproc` | 4.0+ |, | -| `${var@Q}` (transformations) | 4.4+ |, | - -`[[ =~ ]]` is a subtler case: it **exists** on 3.0 (verified against a real -build), but 3.2 changed whether a quoted right-hand side is a regex or a -literal. At a 3.0 floor the same match therefore behaves differently across -supported versions, so regex matching goes through `grep -E`, a real -constraint, not a style preference. See `.agnostic-ai/rules/perf-fork-budget.md`. - -### Style, not compatibility - -Prefer `[ ]` over `[[ ]]` for ordinary tests. This is **house style**: `[[ ]]` -itself works on every supported bash, including 3.0. Do not justify it as a -Bash 3 requirement, and do not add it to the compatibility gate. - -## Coding Conventions - -- **2 spaces** indent, no tabs, enforced by `make lint` (EditorConfig). Do **not** - run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses - `\`-continued strings past the 120-char limit, breaking `make lint` -- **120 chars** max line length (soft) -- Follow [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html) -- Always quote variables unless explicit word splitting is needed -- Use `$()` for command substitution, never backticks - -### Naming - -- **Public functions:** `bashunit::function_name` -- **Private functions:** `_function_name` (leading underscore) -- **Local variables:** `lowercase_with_underscores` -- **Globals/exports:** `UPPERCASE_WITH_UNDERSCORES` - -### Function Docs (public functions) - -```bash -## -# Brief description -# Arguments: $1 - desc, $2 - desc (optional, default: "x") -# Returns: 0 success, 1 failure -## -``` - -### File Structure - -Constants -> Globals -> Private functions -> Public functions - -Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` - -## Returning values from a helper (dynamic-scope safety) - -Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee -shadows the caller's same-named variable for the duration of the call. Same trap fires -with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...` -writes, they all resolve against the dynamic scope. - -We need **all three** properties on the hot path: - -1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork - per call, which dominates per-test cost. -2. **No collision with caller locals.** A helper that silently overwrites or reads - the wrong variable is the worst kind of bug, no error, just stale data. -3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and - it has the same dynamic-scope shadowing as `eval "$name=..."` anyway). - -### Preferred: dedicated global return slot - -```bash -_BASHUNIT_PKG_THING_OUT="" - -# Writes the result into _BASHUNIT_PKG_THING_OUT. -# Arguments: $1 input -function bashunit::pkg::do_thing() { - local input=$1 # natural name, no prefix needed - local val="${input%%]*}" - val="${val#[}" - _BASHUNIT_PKG_THING_OUT=$val -} - -# Caller -bashunit::pkg::do_thing "$payload" -local thing=$_BASHUNIT_PKG_THING_OUT -``` - -- Zero forks per call. -- Helper has natural local names; no `__bu_` noise. -- Caller cannot accidentally shadow the slot because the `_BASHUNIT_*` namespace - is reserved for the framework. -- A dedicated slot per helper (rather than one shared `_BASHUNIT_OUT`) means - adjacent or nested calls can't clobber each other. Cheap: globals are free. - -Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`, -`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`), -`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`). - -### When the helper builds dynamic variable names (mock/spy state) - -Use a clear namespace prefix on the **constructed** name, not on the helper's locals: - -```bash -# Spy state lives in _BASHUNIT_SPY_${variable}_TIMES_FILE, not ${variable}_times_file. -# A caller doing `local foo_times_file=...` is therefore harmless: the helper -# resolves a different global. -export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file" -``` - -Example in tree: `src/test_doubles.sh` (`_BASHUNIT_SPY_*`). - -### Last-resort fallback: outvar by name + `__bu_` prefix on locals - -Only use this when neither a fixed return slot nor a namespaced constructed name is -practical (e.g. a generic helper called from many call sites with different output -variables and no shared global is appropriate): - -```bash -function bashunit::pkg::do_thing() { - local __bu_out=$1 - local __bu_in=$2 - local __bu_val="${__bu_in%%]*}" - eval "$__bu_out=\$__bu_val" -} -``` - -- All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing - silently breaks the caller's outvar (see PR #672, that exact bug caused 12 parallel - test failures in #662). -- Include a regression test that calls the helper with one of its own documented - internal names as the outvar. - -### Intentional dynamic-scope mutation is a separate pattern - -The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends) -deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). -That is documented inline above the helpers in that file and is **not** the outvar pattern, -the helper has no `$1`-named outvar argument; the caller agrees to share state by convention. -It is also why those helpers and `extract_branches` must stay in one file (ADR-010). -Don't introduce new instances of this pattern without an inline justification comment. - -## ShellCheck - -All code must pass `make sa`. Use directives sparingly with reason: -```bash -# shellcheck disable=SC2034 # Variable used by caller -``` diff --git a/.agnostic-ai/context/testing.md b/.agnostic-ai/context/testing.md deleted file mode 100644 index f5ef6d91e..000000000 --- a/.agnostic-ai/context/testing.md +++ /dev/null @@ -1,130 +0,0 @@ -# Testing Guidelines - -## Organization - -| Directory | Purpose | Pattern | -|-----------|---------|---------| -| `tests/unit/` | Isolated function tests | Mocks/spies for deps | -| `tests/functional/` | Multi-component integration | Real interactions | -| `tests/acceptance/` | CLI/end-to-end workflows | Full user scenarios | - -**Naming:** Files end with `_test.sh`. Functions: `test_should__when_` - -## Assertions - -```bash -assert_equals "expected" "$actual" -assert_not_equals "not_this" "$actual" -assert_contains "substring" "$haystack" -assert_not_contains "substring" "$haystack" -assert_matches "regex" "$string" -assert_not_matches "regex" "$string" -assert_empty "$var" -assert_not_empty "$var" -assert_successful_code "$?" -assert_general_error "$?" -assert_file_exists "$path" -assert_file_not_exists "$path" -assert_directory_exists "$path" -assert_array_contains "value" "${array[@]}" -assert_array_not_contains "value" "${array[@]}" -``` - -There is no `assert_fails`. To assert that an assertion *fails*, capture its output and -compare it with the renderer: - -```bash -assert_same "$(bashunit::console_results::print_failed_test "My test" "a" "but got " "b")" \ - "$(assert_same "a" "b")" -``` - -## Test Doubles - -Helpers are namespaced (`bashunit::*`); assertions are bare. The unprefixed helper name -is a `command not found`, not an alias. - -```bash -# Spies - track calls without changing behavior -bashunit::spy function_name -assert_have_been_called function_name -assert_have_been_called_times 2 function_name # count first, then spy -assert_have_been_called_with function_name "arg" # spy first, then expected -assert_have_been_called_with function_name "arg" 1 # ...of call #1 -assert_have_been_called_nth_with 1 function_name "arg" -assert_have_been_called_with_args function_name "a b" # boundary-exact, no call index - -# Mocks - replace behavior -bashunit::mock date echo "2024-05-01" -bashunit::mock uname <<< "Linux" # heredoc form ignores the call's arguments -``` - -Note `_times` takes the count first while `_with` takes the spy first, a swapped pair -fails the assertion rather than erroring, so it reads like a real defect. - -## Data Providers - -```bash -function data_provider_inputs() { - echo "5 3 8" - echo "0 0 0" -} - -# @data_provider data_provider_inputs -function test_should_add() { - assert_equals "$3" "$(add "$1" "$2")" -} -``` - -## Lifecycle Hooks - -- `set_up_before_script()`, once before all tests in file -- `set_up()`, before each test -- `tear_down()`, after each test -- `tear_down_after_script()`, once after all tests in file - -## Snapshot Testing - -```bash -assert_match_snapshot "$output" -assert_match_named_snapshot "stderr" "$stderr" # multiple snapshots in one test -# Re-record deliberately: ./bashunit --snapshot-update --filter "test name" tests/ -``` - -## Writing a test that generates test files - -The duplicate-test-function check scans a file **textually**, so a fixture body -embedded in a string counts as a definition of the *enclosing* file: - -```bash -function test_one() { - run_fixture 'function test_x() { assert_same 1 1; }' # ← counted -} -function test_two() { - run_fixture 'function test_x() { assert_same 2 2; }' # ← counted: duplicate -} -``` - -That fails the whole run before any test executes, and the message names a -function this file never defines. Give every embedded fixture function a name -unique across the file. The report includes the line of each definition -(`test_x (lines 30, 40)`), which is what identifies the strings. - -It cannot be fixed by tracking quote state, an apostrophe in any comment -(`# doesn't work`) would flip it and mask real duplicates. - -## Test Isolation - -- Use `$(bashunit::temp_file)` / `$(bashunit::temp_dir)` (auto-cleaned) for file - operations, these are functions, not variables -- No shared global state between tests -- No network calls, mock external commands -- No time dependencies, mock `date` if needed -- Tests must be safe for `./bashunit --parallel tests/` - -## Reference Tests - -- **Assertions:** `tests/unit/assert_test.sh` -- **Doubles:** `tests/functional/doubles_test.sh` -- **Providers:** `tests/functional/provider_test.sh` -- **Hooks:** `tests/unit/setup_teardown_test.sh` -- **CLI/snapshots:** `tests/acceptance/bashunit_test.sh` diff --git a/.agnostic-ai/rules/architecture-map.md b/.agnostic-ai/rules/architecture-map.md index ced87b2c5..cb6e80175 100644 --- a/.agnostic-ai/rules/architecture-map.md +++ b/.agnostic-ai/rules/architecture-map.md @@ -1,7 +1,160 @@ --- -paths: - - "src/**/*.sh" - - "bashunit" +scope: src --- -Before changing `src/` or the `bashunit` entry point, read `.agnostic-ai/context/architecture-map.md` for module ownership, the run lifecycle, and cross-cutting constraints. +# Architecture Map: modules and the life of a run + +Orientation for working in `src/`, which file owns what, and the exact call +flow of a test run. Line numbers drift; function names are the stable anchors. + +## The life of `./bashunit tests/` + +``` +bashunit (entry) sources all src/*.sh; version gate; early flag scan +└─ bashunit::main::cmd_test (main.sh: flag parsing, env exports) + └─ bashunit::runner::load_test_files (runner/discovery.sh: the per-file loop) + ├─ console_header::print_header "Running N tests", captures + │ └─ helper::find_total_tests $() SUBSHELL: sources each file in a + │ nested subshell just to count tests + ├─ source "$test_file" in the MAIN shell (workers inherit) + ├─ helper::check_duplicate_functions one awk pass per file + ├─ runner::functions_for_script pure bash: filter + sort by def line + ├─ helper::build_provider_map one awk pass per file (cached by path, + │ but the count subshell's build dies + │ with it → runs twice per file) + └─ runner::call_test_functions per-fn loop + └─ runner::run_test [--parallel: spawned as a & worker per test] + ├─ clock::now_to_slot duration start (if needed) + ├─ runner::execute_test_body $() subshell: the actual test fn + │ └─ state::export_subshell_context printf payload "##K=V##…" + ├─ runner::parse_result sync: parse counts from payload + │ parallel: + write .result via mktemp + └─ console_results::print_successful_test / _failed_ / … + └─ str::rpad + strip_ansi align per-test time (pure bash) + └─ [--parallel] wait; state::aggregate_parallel_results over *.result files + └─ console_results::render_result totals; deferred failure/skip blocks + └─ rerun::persist; env::cleanup_run_output_dir; exit code +``` + +Key inversion to remember: **tests run inside `$()` subshells; state comes back +as an encoded single-line payload** (`##ASSERTIONS_FAILED=…##TEST_OUTPUT=##`), +parsed by `runner::parse_result`/`state.sh`. Counters only exist in the main +shell (or, in parallel, in per-test `.result` files aggregated at the end). + +## Module ownership + +| Module | Owns | +|--------|------| +| `bashunit` | entry: version gate, early flag scan, sources every module, subcommand routing | +| `main/index.sh` | aggregator only, sources the `src/main/` module below | +| `main/validate.sh` | shared parser validation: unknown options, integer/path checks, shard parsing | +| `main/test.sh` / `main/bench.sh` / `main/subcommands.sh` | flag parsing per subcommand | +| `main/assert.sh` | the standalone `bashunit assert` path | +| `main/watch.sh` / `main/run.sh` | the `--watch` loop; the run lifecycle, exit codes and cleanup | +| `runner/index.sh` | aggregator only, sources the `src/runner/` module below | +| `runner/context.sh` | workdir restore, test identity/location exports, title interpolation, capability probes | +| `runner/payload.sh` | the `_BASHUNIT_RUNNER_*_OUT` return slots; encode/decode of the per-test result payload | +| `runner/diagnostics.sh` | runtime-error detection, kill-signal classification, profiling, verbose/file headers | +| `runner/result.sh` | `parse_result{,_sync,_parallel}`, failure source context, failed/skipped/incomplete/risky writers | +| `runner/parallel.sh` | job-slot waiting (`wait -n` or poll), running-job count, spinner | +| `runner/hooks.sh` | set_up/tear_down (test + script scope), hook failure records, mock clearing, EXIT cleanup | +| `runner/provider.sh` | `@data_provider` argument parsing | +| `runner/exec.sh` | `run_test`, the capture-subshell body, retry, timeout watchdog, per-file dispatch, `order_functions_for_script` (the run order a seed produces) | +| `runner/list.sh` | `--list`/`--dry-run`: emits selected tests instead of running them; hooks in after all filtering, before any hook | +| `runner/discovery.sh` | `load_test_files` (the per-file loop), `functions_for_script` | +| `runner/bench.sh` | benchmark file loop and bench function dispatch | +| `helper/index.sh` | aggregator only, sources the `src/helper/` module below | +| `helper/naming.sh` | resolving and normalising test-function and variable names | +| `helper/discovery.sh` | `find_files_recursive`, `find_total_tests`, fn filtering, duplicate check, line lookup | +| `helper/provider.sh` / `helper/tags.sh` | `@data_provider` map; `@tag` extraction and matching | +| `helper/encoding.sh` / `helper/functions.sh` / `helper/git.sh` | base64 + ids; generic fn utilities; remote tag lookup for `upgrade` | +| `state/index.sh` | aggregator only, sources the `src/state/` module below | +| `state/counters.sh` | test and assertion tallies and their get/add accessors | +| `state/duplicates.sh` | duplicate test-function detection state | +| `state/context.sh` | per-test output buffer, exit code, title, hook failure, and the per-test reset | +| `state/payload.sh` | the encoded per-test result payload and the base64 capability probe | +| `state/parallel.sh` | aggregating per-test result files after a `--parallel` run | +| `config/index.sh` | aggregator only, sources the `src/config/` module below | +| `config/env.sh` | all `BASHUNIT_*` defaults/config files, scratch dirs (`_BASHUNIT_RUN_OUTPUT_DIR` + EXIT-trap cleanup); executes at source time | +| `config/rerun.sh` | the `.bashunit/last-failed` cache for `--rerun-failed` | +| `config/parallel.sh` | is this run parallel; worker temp tree and stop-on-failure flag file (not `runner/parallel.sh`, which waits on job slots) | +| `console/index.sh` | aggregator only, sources the `src/console/` module below | +| `console/colors.sh` | the `_BASHUNIT_COLOR_*` palette and `bashunit::sgr` | +| `console/header.sh` | the "Running N tests" header | +| `console/line.sh` | `print_line`, the primitive every result line goes through, and its TAP variant | +| `console/duration.sh` / `console/diff.sh` | duration formatting; unified and line-by-line diffs under a failure | +| `console/test_line.sh` | the per-test result lines (passed/failed/skipped/incomplete/snapshot/risky/error) | +| `console/deferred.sh` | end-of-run blocks buffered during the run (scratch files under the run dir) | +| `console/summary.sh` | run totals, execution time, hook completion | +| `assert/index.sh` | aggregator only, sources the `src/assert/` module below, plus `skip_todo.sh` and `test_doubles.sh` | +| `assert/core.sh` | `assert::should_skip`, `assert::fail_with`, `assert::join_to_slot` and the comparison assertions the other files build on | +| `assert/{arrays,assertions,dates,duration,files,folders,json,once,snapshot}.sh` | the per-topic assertions; the per-assertion path must stay fork-free | +| `util/index.sh` | aggregator only, sources the `src/util/` module below | +| `util/clock.sh` | time impl selection (EPOCHREALTIME > date > perl > …), return-slot reads | +| `util/str.sh` / `util/math.sh` | pure-bash string and arithmetic helpers | +| `api/index.sh` | aggregator only, sources the `src/api/` module below | +| `api/globals.sh` | `temp_file`/`temp_dir`/`current_dir`/`data_set`; repeats `set -euo pipefail` at file scope for anything sourcing it alone, the entrypoint already set it | +| `api/skip_todo.sh` / `api/test_title.sh` | `skip`/`todo` markers; `set_test_title` | +| `api/bashunit.sh` | the custom-assert facade (`assert_that`, `assertion_failed`/`passed`) | +| `doubles/index.sh` | aggregator only, sources the `src/doubles/` module below | +| `doubles/mock.sh` | `mock`/`unmock` and the `_BASHUNIT_MOCKED_FUNCTIONS` registry `runner/hooks.sh` unwinds per test | +| `doubles/spy.sh` | `spy` and its `_BASHUNIT_SPY_*` state slots + files | +| `doubles/assertions.sh` | the `assert_have_been_called*` family | +| `coverage/index.sh` | aggregator only, sources the `src/coverage/` modules below | +| `coverage/config.sh` | data-file locations, tracked-file roots, engine selection (`init` resets state owned by several modules) | +| `coverage/paths.sh` | `normalize_path`, `should_track` and the hot-path track/path caches | +| `coverage/engine.sh` | DEBUG-trap and xtrace capture, buffering, `finalize`/`cleanup`, parallel merge; only active under `--coverage` | +| `coverage/lines.sh` | executable-line classification (fork-free; the report phase's hot path) and hit-data reading | +| `coverage/stats.sh` | percentages, the precomputed per-file stats cache, threshold gate | +| `coverage/functions.sh` | function definitions and their line spans | +| `coverage/branches.sh` | branch extraction + hit computation; **one file on purpose**, the `_branch_*` helpers mutate `extract_branches`'s locals via dynamic scoping | +| `coverage/report_text.sh` / `report_lcov.sh` / `report_html.sh` | the three renderers | +| `coverage/html_index.sh` / `html_file.sh` | HTML page emitters; the only two files exempt from `max_line_length` | + +| `reports.sh` | JUnit/HTML/TAP/JSON writers | +| `system/index.sh` | aggregator only, sources the `src/system/` module below | +| `system/check_os.sh` / `system/dependencies.sh` | one-fork OS detect; `command -v` probes (builtins, not forks) | +| `system/io.sh` | small I/O helpers | +| `cli/index.sh` | aggregator only, sources the `src/cli/` module below | +| `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; `main.sh` is their only caller | +| `benchmark/index.sh` | the remaining non-`test` subcommand; shared with `runner/bench.sh` | + +## Cross-cutting invariants + +- **Bash 3.0 floor** (`.agnostic-ai/rules/bash-style.md`): no `[[`, `declare -A`, + `${var,,}`, `BASHPID`, negative indices. Subshells share `$$`, you cannot + make a per-worker unique token without a fork (`mktemp`) or an ordinal + assigned before the fork. `RANDOM` is no help either: whether a subshell + reseeds it depends on the platform and the nesting depth (measured in + perf-fork-budget.md), so it is unreliable in both directions (#1354). +- **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` + captures on hot paths, bash-style.md documents it; `local` is dynamically + scoped, so helpers must not write caller-named variables. +- **Fork budget** (`.agnostic-ai/rules/perf-fork-budget.md`): per-test paths are + fork-free (sequential) or mktemp-only (parallel); budgets are enforced by + `tests/acceptance/bashunit_*forks*_test.sh` on three platforms, a "harmless" + `echo | sed` in a per-test path will fail CI. +- **`call_test_functions` is a background subshell under `--parallel`** + (`discovery.sh` backgrounds it per file). Anything it sets in shell state, + counters, flags, error records, dies with the worker. Only two things cross + back: per-test `.result` payloads, and files written to disk. This produced + two silent-green bugs in one day: a data-provider error printed while the run + exited 0 (#1145), and duplicate-test-function detection being entirely off in + the mode CI uses (#1147). Before adding a check anywhere under that call, ask + which side of the fork it belongs on, a whole-file check almost always + belongs in the parent loop, before dispatch. + **Verify both modes.** Sequential and `--parallel` disagreeing is the + signature of this bug, and it is invisible to a sequential-only test. +- **Binaries pinned at startup** (`$GREP`, `$MKTEMP`, `$CAT` in env.sh) so test + doubles/PATH games can't hijack the framework's own plumbing. +- **Snapshots assume 80-col non-tty width** (`tput cols` fallback); anything + that changes rendering widths breaks `tests/acceptance/snapshots/`. +- **The build flattens the source graph in DFS order** (`build.sh` + `build::process_file`): a file's body is emitted, *then* its `source` lines are + recursed into. That equals dev-mode order only if a module aggregator + (`src//index.sh`, ADR-010) contains **nothing but `source` + lines and comments**, any other top-level statement would run before its + dependencies in the built artifact but after them in dev mode. Files are + deduped by repo-relative path, so `src/` may hold module dirs and two files may + share a basename. Every src file's **first line must be the shebang**: + `tail -n +2` strips it when embedding. diff --git a/.agnostic-ai/rules/bash-style.md b/.agnostic-ai/rules/bash-style.md index e50738a90..307f94f3c 100644 --- a/.agnostic-ai/rules/bash-style.md +++ b/.agnostic-ai/rules/bash-style.md @@ -1,7 +1,174 @@ --- +scope: src paths: - - "src/**/*.sh" - - "tests/**/*.sh" + - "tests/**" --- -Before changing shell code under `src/` or `tests/`, read `.agnostic-ai/context/bash-style.md`. Keep Bash 3.0 parsing valid and use the documented version gates and return-slot pattern. +# Bash Style & Compatibility Rules + +## Bash 3.0+ Compatibility (Critical) + +bashunit must work on **Bash 3.0+**. The floor was deliberately lowered from 3.2 +to 3.0 in 0.33 to reach older systems; macOS's 3.2 is the common case, not the +floor. The runtime gate in `./bashunit` compares major **and** minor against +`BASHUNIT_MIN_BASH_VERSION`, so the declared floor is the enforced one. + +These are **version-breaking** above the floor and are prohibited. They are +enforced mechanically by `tests/unit/bash_compatibility_test.sh`, which greps +`src/` regardless of test coverage: + +| Feature | Bash ver | Alternative | +|---------|----------|-------------| +| `printf -v` | 3.1+ | Return-slot pattern (below) | +| `x+=y` (string append) | 3.1+ | `var="$var$more"` | +| `arr+=(x)` (array append) | 3.1+, and a 3.0 **parse** error | `arr[${#arr[@]}]=x` | +| `declare -A` (associative arrays) | 4.0+ | Parallel indexed arrays | +| `${var,,}` / `${var^^}` (case) | 4.0+ | `tr '[:upper:]' '[:lower:]'` | +| `${array[-1]}` (negative index) | 4.3+ | `${array[${#array[@]}-1]}` | +| `&>>` (append both) | 4.0+ | `>> file 2>&1` | +| `BASHPID` | 4.0+ | fork (`mktemp`) or an assigned ordinal | +| `mapfile` / `readarray` | 4.0+ | `while IFS= read -r` | +| `declare -n` / `local -n` (nameref) | 4.3+ | Return-slot pattern (below) | +| `coproc` | 4.0+ |, | +| `${var@Q}` (transformations) | 4.4+ |, | + +`[[ =~ ]]` is a subtler case: it **exists** on 3.0 (verified against a real +build), but 3.2 changed whether a quoted right-hand side is a regex or a +literal. At a 3.0 floor the same match therefore behaves differently across +supported versions, so regex matching goes through `grep -E`, a real +constraint, not a style preference. See `.agnostic-ai/rules/perf-fork-budget.md`. + +### Style, not compatibility + +Prefer `[ ]` over `[[ ]]` for ordinary tests. This is **house style**: `[[ ]]` +itself works on every supported bash, including 3.0. Do not justify it as a +Bash 3 requirement, and do not add it to the compatibility gate. + +## Coding Conventions + +- **2 spaces** indent, no tabs, enforced by `make lint` (EditorConfig). Do **not** + run `shfmt -w`: it wants tabs for `tests/acceptance/**` and collapses + `\`-continued strings past the 120-char limit, breaking `make lint` +- **120 chars** max line length (soft) +- Follow [Google Shell Style Guide](https://google.github.io/styleguide/shellguide.html) +- Always quote variables unless explicit word splitting is needed +- Use `$()` for command substitution, never backticks + +### Naming + +- **Public functions:** `bashunit::function_name` +- **Private functions:** `_function_name` (leading underscore) +- **Local variables:** `lowercase_with_underscores` +- **Globals/exports:** `UPPERCASE_WITH_UNDERSCORES` + +### Function Docs (public functions) + +```bash +## +# Brief description +# Arguments: $1 - desc, $2 - desc (optional, default: "x") +# Returns: 0 success, 1 failure +## +``` + +### File Structure + +Constants -> Globals -> Private functions -> Public functions + +Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` + +## Returning values from a helper (dynamic-scope safety) + +Bash `local` is **dynamically scoped**, not lexically scoped. A `local` inside a callee +shadows the caller's same-named variable for the duration of the call. Same trap fires +with `${!name}` reads and `eval "$name=..."`/`printf -v "$name"`/`export "$name"=...` +writes, they all resolve against the dynamic scope. + +We need **all three** properties on the hot path: + +1. **No subshell fork.** Returning via stdout and capturing with `$(...)` costs a fork + per call, which dominates per-test cost. +2. **No collision with caller locals.** A helper that silently overwrites or reads + the wrong variable is the worst kind of bug, no error, just stale data. +3. **Bash 3.0+ portable.** Rules out `declare -n` (4.3+) and `printf -v` (3.1+, and + it has the same dynamic-scope shadowing as `eval "$name=..."` anyway). + +### Preferred: dedicated global return slot + +```bash +_BASHUNIT_PKG_THING_OUT="" + +# Writes the result into _BASHUNIT_PKG_THING_OUT. +# Arguments: $1 input +function bashunit::pkg::do_thing() { + local input=$1 # natural name, no prefix needed + local val="${input%%]*}" + val="${val#[}" + _BASHUNIT_PKG_THING_OUT=$val +} + +# Caller +bashunit::pkg::do_thing "$payload" +local thing=$_BASHUNIT_PKG_THING_OUT +``` + +- Zero forks per call. +- Helper has natural local names; no `__bu_` noise. +- Caller cannot accidentally shadow the slot because the `_BASHUNIT_*` namespace + is reserved for the framework. +- A dedicated slot per helper (rather than one shared `_BASHUNIT_OUT`) means + adjacent or nested calls can't clobber each other. Cheap: globals are free. + +Examples in tree: `src/runner/payload.sh` (`_BASHUNIT_RUNNER_FIELD_OUT`, +`_BASHUNIT_RUNNER_TOTAL_OUT`, `_BASHUNIT_RUNNER_TYPE_OUT`, `_BASHUNIT_RUNNER_OUTPUT_OUT`), +`src/coverage/branches.sh` (`_BASHUNIT_BRANCH_ARMS_OUT`). + +### When the helper builds dynamic variable names (mock/spy state) + +Use a clear namespace prefix on the **constructed** name, not on the helper's locals: + +```bash +# Spy state lives in _BASHUNIT_SPY_${variable}_TIMES_FILE, not ${variable}_times_file. +# A caller doing `local foo_times_file=...` is therefore harmless: the helper +# resolves a different global. +export "_BASHUNIT_SPY_${variable}_TIMES_FILE"="$times_file" +``` + +Example in tree: `src/test_doubles.sh` (`_BASHUNIT_SPY_*`). + +### Last-resort fallback: outvar by name + `__bu_` prefix on locals + +Only use this when neither a fixed return slot nor a namespaced constructed name is +practical (e.g. a generic helper called from many call sites with different output +variables and no shared global is appropriate): + +```bash +function bashunit::pkg::do_thing() { + local __bu_out=$1 + local __bu_in=$2 + local __bu_val="${__bu_in%%]*}" + eval "$__bu_out=\$__bu_val" +} +``` + +- All internal locals MUST be `__bu_`-prefixed; otherwise dynamic-scope shadowing + silently breaks the caller's outvar (see PR #672, that exact bug caused 12 parallel + test failures in #662). +- Include a regression test that calls the helper with one of its own documented + internal names as the outvar. + +### Intentional dynamic-scope mutation is a separate pattern + +The coverage branch helpers in `src/coverage/branches.sh` (`_branch_push_if` and friends) +deliberately mutate caller locals (`if_decision_line`, `if_arms`, `if_depth`, `if_arm_start`). +That is documented inline above the helpers in that file and is **not** the outvar pattern, +the helper has no `$1`-named outvar argument; the caller agrees to share state by convention. +It is also why those helpers and `extract_branches` must stay in one file (ADR-010). +Don't introduce new instances of this pattern without an inline justification comment. + +## ShellCheck + +All code must pass `make sa`. Use directives sparingly with reason: +```bash +# shellcheck disable=SC2034 # Variable used by caller +``` diff --git a/.agnostic-ai/rules/perf-fork-budget.md b/.agnostic-ai/rules/perf-fork-budget.md index d9caf12a7..eb24bc339 100644 --- a/.agnostic-ai/rules/perf-fork-budget.md +++ b/.agnostic-ai/rules/perf-fork-budget.md @@ -1,7 +1,16 @@ --- +scope: src paths: - - "src/**/*.sh" - - "tests/acceptance/bashunit_*forks*_test.sh" + - "tests/acceptance/**" --- -Before changing `src/` or a fork census acceptance test, read `.agnostic-ai/context/perf-fork-budget.md`. Keep per-test paths within the existing process budget; measure with the matching sequential or parallel fixture. +# Process budgets + +Keep startup, per-file, and per-test process costs within the existing fork census budgets. Read and use {{$SKILL:fork-census}} before changing those paths, parallel workers, report serialization, or coverage classification. + +- Measure the path that runs, including flags and lifecycle hooks. A sequential fixture cannot detect per-worker costs; parallel changes need a parallel census. +- Count binary invocations with PATH shims where reliable. Pinned binaries bypass shims, and shell-function command substitutions fork without executing a binary. Raw `bash -x` totals overcount captured output. +- Benchmark before replacing an external tool with pure Bash. Large shell loops and string substitutions can be slower, and behavior varies across supported Bash versions. Keep regex assertions and HTML escaping semantics portable. +- Preserve fork-free per-test hooks, worker result naming by dispatcher ordinal, and the shared clock implementation. Do not use inherited `$$` or platform-dependent `RANDOM` behavior as unique worker identifiers. +- Never pass process substitutions as external-command arguments in per-file or per-test loops. Compare real files; redirecting process substitution into stdin is a different case. +- Extend the matching cold-start, run, or coverage fork regression when removing a fork class. Prove comparison harnesses can fail before relying on them, and repeat noisy or anomalous measurements. diff --git a/.agnostic-ai/rules/tdd-workflow.md b/.agnostic-ai/rules/tdd-workflow.md index e8f6d49bf..fe4cf71bb 100644 --- a/.agnostic-ai/rules/tdd-workflow.md +++ b/.agnostic-ai/rules/tdd-workflow.md @@ -1,7 +1,7 @@ --- -paths: - - "**/*.sh" - - ".tasks/**/*.md" +alwaysApply: true --- -For shell development and task plans, read `.agnostic-ai/context/tdd-workflow.md`. Write a failing test for runtime changes, implement the smallest passing change, then refactor. Run the full quality gate once after the change set is complete. +# TDD + +Use test-driven development for runtime changes: prove the smallest test fails for the right reason, make it pass with minimal code, then refactor while keeping tests green. Use {{$SKILL:tdd-cycle}} for the full workflow and task-file template. Run the complete quality gate once after the change set is complete, before committing. diff --git a/.agnostic-ai/rules/testing.md b/.agnostic-ai/rules/testing.md index 3f50dbd38..31413fee2 100644 --- a/.agnostic-ai/rules/testing.md +++ b/.agnostic-ai/rules/testing.md @@ -1,6 +1,134 @@ --- -paths: - - "tests/**/*_test.sh" +scope: tests --- -Before changing tests, read `.agnostic-ai/context/testing.md`. Follow the existing assertion, test-double, fixture, and lifecycle patterns. Tests must remain isolated under parallel execution. +# Testing Guidelines + +## Organization + +| Directory | Purpose | Pattern | +|-----------|---------|---------| +| `tests/unit/` | Isolated function tests | Mocks/spies for deps | +| `tests/functional/` | Multi-component integration | Real interactions | +| `tests/acceptance/` | CLI/end-to-end workflows | Full user scenarios | + +**Naming:** Files end with `_test.sh`. Functions: `test_should__when_` + +## Assertions + +```bash +assert_equals "expected" "$actual" +assert_not_equals "not_this" "$actual" +assert_contains "substring" "$haystack" +assert_not_contains "substring" "$haystack" +assert_matches "regex" "$string" +assert_not_matches "regex" "$string" +assert_empty "$var" +assert_not_empty "$var" +assert_successful_code "$?" +assert_general_error "$?" +assert_file_exists "$path" +assert_file_not_exists "$path" +assert_directory_exists "$path" +assert_array_contains "value" "${array[@]}" +assert_array_not_contains "value" "${array[@]}" +``` + +There is no `assert_fails`. To assert that an assertion *fails*, capture its output and +compare it with the renderer: + +```bash +assert_same "$(bashunit::console_results::print_failed_test "My test" "a" "but got " "b")" \ + "$(assert_same "a" "b")" +``` + +## Test Doubles + +Helpers are namespaced (`bashunit::*`); assertions are bare. The unprefixed helper name +is a `command not found`, not an alias. + +```bash +# Spies - track calls without changing behavior +bashunit::spy function_name +assert_have_been_called function_name +assert_have_been_called_times 2 function_name # count first, then spy +assert_have_been_called_with function_name "arg" # spy first, then expected +assert_have_been_called_with function_name "arg" 1 # ...of call #1 +assert_have_been_called_nth_with 1 function_name "arg" +assert_have_been_called_with_args function_name "a b" # boundary-exact, no call index + +# Mocks - replace behavior +bashunit::mock date echo "2024-05-01" +bashunit::mock uname <<< "Linux" # heredoc form ignores the call's arguments +``` + +Note `_times` takes the count first while `_with` takes the spy first, a swapped pair +fails the assertion rather than erroring, so it reads like a real defect. + +## Data Providers + +```bash +function data_provider_inputs() { + echo "5 3 8" + echo "0 0 0" +} + +# @data_provider data_provider_inputs +function test_should_add() { + assert_equals "$3" "$(add "$1" "$2")" +} +``` + +## Lifecycle Hooks + +- `set_up_before_script()`, once before all tests in file +- `set_up()`, before each test +- `tear_down()`, after each test +- `tear_down_after_script()`, once after all tests in file + +## Snapshot Testing + +```bash +assert_match_snapshot "$output" +assert_match_named_snapshot "stderr" "$stderr" # multiple snapshots in one test +# Re-record deliberately: ./bashunit --snapshot-update --filter "test name" tests/ +``` + +## Writing a test that generates test files + +The duplicate-test-function check scans a file **textually**, so a fixture body +embedded in a string counts as a definition of the *enclosing* file: + +```bash +function test_one() { + run_fixture 'function test_x() { assert_same 1 1; }' # ← counted +} +function test_two() { + run_fixture 'function test_x() { assert_same 2 2; }' # ← counted: duplicate +} +``` + +That fails the whole run before any test executes, and the message names a +function this file never defines. Give every embedded fixture function a name +unique across the file. The report includes the line of each definition +(`test_x (lines 30, 40)`), which is what identifies the strings. + +It cannot be fixed by tracking quote state, an apostrophe in any comment +(`# doesn't work`) would flip it and mask real duplicates. + +## Test Isolation + +- Use `$(bashunit::temp_file)` / `$(bashunit::temp_dir)` (auto-cleaned) for file + operations, these are functions, not variables +- No shared global state between tests +- No network calls, mock external commands +- No time dependencies, mock `date` if needed +- Tests must be safe for `./bashunit --parallel tests/` + +## Reference Tests + +- **Assertions:** `tests/unit/assert_test.sh` +- **Doubles:** `tests/functional/doubles_test.sh` +- **Providers:** `tests/functional/provider_test.sh` +- **Hooks:** `tests/unit/setup_teardown_test.sh` +- **CLI/snapshots:** `tests/acceptance/bashunit_test.sh` diff --git a/.agnostic-ai/skills/commit/SKILL.md b/.agnostic-ai/skills/commit/SKILL.md index 3b8ea439e..cbc74a322 100644 --- a/.agnostic-ai/skills/commit/SKILL.md +++ b/.agnostic-ai/skills/commit/SKILL.md @@ -9,6 +9,8 @@ allowed-tools: shell, read, Grep, Glob # Commit with Conventional Commits +Read [the formatting constraints](references/formatting.md) before formatting or checking shell changes. Repository paths are relative to the repository root. + Stage and commit current changes using the conventional commits format. > **IMPORTANT:** This skill MUST be used for ALL commits, even when the user says "commit" without `commit`. Never commit without following these steps. diff --git a/.agnostic-ai/skills/commit/references/formatting.md b/.agnostic-ai/skills/commit/references/formatting.md new file mode 100644 index 000000000..ecfb557e2 --- /dev/null +++ b/.agnostic-ai/skills/commit/references/formatting.md @@ -0,0 +1,20 @@ +# Formatting + +Match the surrounding 2-space style by hand and verify with `make lint`. There is +no `shfmt` target, and **`shfmt -w .` must not be run**, it fights `.editorconfig` +two ways: + +- `.editorconfig` sets `indent_size = unset` for `tests/acceptance/**.sh` and + `src/console_header.sh`; shfmt reads `unset` as "use my default", which is tabs, + while `[*]` sets `indent_style = space`. Any new file under `tests/acceptance/` + therefore shows up shfmt-dirty, that is expected, not a defect. +- shfmt collapses `\`-continued strings onto one line, which would push + `src/state.sh`'s record writer past `max_line_length = 120`. + +To check you introduced no new drift, compare the dirty-file *list* against main +rather than requiring it to be empty: + +```bash +git ls-files -z '*.sh' bashunit | xargs -0 shfmt -d 2>/dev/null \ + | grep '^--- ' | sed 's|--- ||; s|\.orig.*||' | sort +``` diff --git a/.agnostic-ai/skills/fork-census/SKILL.md b/.agnostic-ai/skills/fork-census/SKILL.md new file mode 100644 index 000000000..72e349fab --- /dev/null +++ b/.agnostic-ai/skills/fork-census/SKILL.md @@ -0,0 +1,15 @@ +--- +name: fork-census +description: Measure process costs and fork budgets when changing startup, per-file, per-test, parallel workers, reporting, or coverage paths; use for fork census and performance comparisons. +allowed-tools: read, shell +--- + +# Fork census + +Read [the fork-budget reference](references/fork-budget.md) before changing process costs. Its historical timings are evidence of earlier measurements, not current baselines. All repository paths below are relative to the repository root. + +1. Identify the actual path: cold start, sequential run, parallel worker, lifecycle hook, report, or coverage. Read the matching existing fixture in `tests/acceptance/bashunit_coldstart_forks_test.sh`, `tests/acceptance/bashunit_run_forks_test.sh`, or `tests/acceptance/bashunit_coverage_forks_test.sh`. Include the options and hooks the change exercises. +2. For a timing claim, measure current main before editing and record its commit, Bash version, OS, fixture, options, and sampling method. Land any new benchmark harness before the PR that measures a delta. Alternate baseline and candidate rounds in the same location and repeat anomalous readings. +3. Use the reference's PATH-shim census for binary invocations. Resolve real binaries before adding shims; startup-pinned binaries bypass them. Account separately for shell-function subshells. Use traces to locate sites, not raw trace-line totals to count forks. Skip unreliable PATH-shim checks on Git Bash. +4. Add a failing budget regression using the existing fixture patterns, then make the smallest change. Measure parallel costs with a parallel fixture and compare supported Bash versions. Never replace a fork with pure Bash based only on a microbenchmark. +5. Run the matching acceptance file with `./bashunit --no-parallel tests/acceptance/`. These tests select their own internal sequential or parallel census. Preserve the checked budgets; report counts, timing samples, and limits. The main session runs the full quality gate after the complete change set. diff --git a/.agnostic-ai/context/perf-fork-budget.md b/.agnostic-ai/skills/fork-census/references/fork-budget.md similarity index 100% rename from .agnostic-ai/context/perf-fork-budget.md rename to .agnostic-ai/skills/fork-census/references/fork-budget.md diff --git a/.agnostic-ai/skills/gh-issue/SKILL.md b/.agnostic-ai/skills/gh-issue/SKILL.md index 5509c54a6..fd9d5beeb 100644 --- a/.agnostic-ai/skills/gh-issue/SKILL.md +++ b/.agnostic-ai/skills/gh-issue/SKILL.md @@ -68,7 +68,7 @@ Fetch a GitHub issue, create branch, implement following TDD, and open a PR. ### Phase 3: Implement -10. **Follow strict TDD workflow** (see `.agnostic-ai/context/tdd-workflow.md`): +10. **Follow strict TDD workflow** (see `.agnostic-ai/rules/tdd-workflow.md`): For each test: - **RED** - Write failing test, verify it fails for the RIGHT reason diff --git a/.agnostic-ai/skills/gh-issues/SKILL.md b/.agnostic-ai/skills/gh-issues/SKILL.md index ff8951e78..736179e22 100644 --- a/.agnostic-ai/skills/gh-issues/SKILL.md +++ b/.agnostic-ai/skills/gh-issues/SKILL.md @@ -160,7 +160,7 @@ With `--dry-run`, only execute Phase 1 and print the queue. No assignment, no br ## Notes -- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.agnostic-ai/context/bash-style.md`. +- **Bash 3.0+ compatibility is mandatory.** No `declare -A`, `[[ ]]`, `${var,,}`, negative array indexing, or `&>>` in `src/`. See `.agnostic-ai/rules/bash-style.md`. - **Quality gate is `make sa` + `make lint`, not bare `shfmt`.** `shfmt -w .` without project flags rewrites the whole tree (collapses line-continuations, flips binary-op style, tabs the `indent_size = unset` files). Match the surrounding 2-space style by hand and rely on `make lint` (EditorConfig) to verify. There is no `shfmt` make target. - **Know your baseline.** `tests/unit/coverage_subshell_test.sh` fails on some macOS setups regardless of the change, confirm a failure is new (`git stash` + re-run) before treating it as a regression. - Treat GitHub CI as the full quality gate; locally run focused tests during implementation, the full sequential + parallel suite once before commit. diff --git a/.agnostic-ai/skills/review/SKILL.md b/.agnostic-ai/skills/review/SKILL.md index 3ac8be6b1..f262969e3 100644 --- a/.agnostic-ai/skills/review/SKILL.md +++ b/.agnostic-ai/skills/review/SKILL.md @@ -34,7 +34,7 @@ Run the following commands before proceeding: gh pr diff ``` -3. **Review the diff** against this project's standards (see `.agnostic-ai/AGNOSTIC_AI.md` and `.agnostic-ai/context/`): +3. **Review the diff** against this project's standards (see `.agnostic-ai/AGNOSTIC_AI.md` and `.agnostic-ai/rules/`): - Bash 3.0+ compatibility (no `declare -A`, `[[ ]]`, `${var,,}`, negative indexing, `&>>`) - Tests exist and follow TDD; both success and failure paths covered - Naming, namespacing (`bashunit::*` / `_private`), and dynamic-scope safety diff --git a/.agnostic-ai/skills/tdd-cycle/SKILL.md b/.agnostic-ai/skills/tdd-cycle/SKILL.md index cc12562e4..b447bc9c9 100644 --- a/.agnostic-ai/skills/tdd-cycle/SKILL.md +++ b/.agnostic-ai/skills/tdd-cycle/SKILL.md @@ -6,44 +6,43 @@ allowed-tools: read, edit, write, shell, Grep, Glob # TDD Cycle -Execute a complete Red -> Green -> Refactor cycle. +Execute a complete Red -> Green -> Refactor cycle. Read [the complete workflow](references/workflow.md) for task-file conventions and the quality gate. Repository paths are relative to the repository root. ## Workflow ### 1. Verify Task File -Check for `.tasks/YYYY-MM-DD-*.md`. If missing, create one before proceeding. +Use `.tasks/YYYY-MM-DD-slug.md` for features, refactors, or multi-test work. Skip it for small fixes or single-test changes. Choose the next test from the acceptance scenario when no task file is needed. ### 2. RED, Write Failing Test -1. Show test inventory from task file, ask which test to implement +1. Pick the smallest next test from the task file or acceptance scenario 2. Study patterns from existing tests (unit -> `assert_test.sh`, doubles -> `doubles_test.sh`) 3. Write the failing test following Arrange-Act-Assert 4. Run: `./bashunit path/to/test.sh`, **must fail** 5. Verify failure is for the RIGHT reason (not syntax error or wrong setup) -6. Update task file: current red bar + logbook entry +6. Update the task file, when present: current red bar + logbook entry ### 3. GREEN, Make It Pass 1. Write **minimal** code in `src/`, only enough for THIS test 2. Run: `./bashunit path/to/test.sh`, **must pass** -3. Update task file: check off test + logbook entry +3. Update the task file, when present: check off test + logbook entry ### 4. REFACTOR, Improve Code 1. Improve readability, naming, extract duplication, no behavior changes 2. Run tests after each change -3. Quality checks: `make sa && make lint` -4. Full suite: `./bashunit tests/` -5. Update task file with refactoring notes +3. Run `make sa && make lint` and both full test modes once after the change set is complete +4. Update the task file, when present, with refactoring notes ### 5. Next Test -Show updated test inventory. Ask if continuing or done. +Continue until the acceptance scenario is complete. Update the test inventory when using a task file. ## Critical Rules - **Never skip RED**, always verify test fails first - **Minimal code in GREEN**, resist feature creep - **All tests green during REFACTOR**, revert if broken -- **Update task file** after each phase +- **Update task file**, when present, after each phase diff --git a/.agnostic-ai/context/tdd-workflow.md b/.agnostic-ai/skills/tdd-cycle/references/workflow.md similarity index 100% rename from .agnostic-ai/context/tdd-workflow.md rename to .agnostic-ai/skills/tdd-cycle/references/workflow.md diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 4b61fb960..63ea3324e 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -68,7 +68,7 @@ On Windows, use the [Windows installer](https://agnostic-ai.org/docs/installatio Edit `.agnostic-ai/` or `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Generated files are ignored. Run sync after cloning and after pulling changes to the specs. Keep personal overrides in `.agnostic-ai/local/` or `agnostic-ai.local.yaml`, which are also ignored. -The root instructions list the detailed rules to read for each task. Short shared rules point to detailed guides under `.agnostic-ai/context/`, which agents read only when relevant. The root task table also works for sessions whose client cannot automatically load scoped rules. A portable after-edit hook extracts edited paths from either tool and runs ShellCheck on shell files. Skills use explicit commands and request arguments in a shared body. Hook commands require agnostic-ai and ShellCheck on the client's PATH. Review and trust the generated hook through `/hooks` in Codex before relying on automatic checks. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. +Standing conventions live in `.agnostic-ai/rules/`. Directory scopes produce path-scoped Claude rules and nested Codex `AGENTS.md` files. Start a session in the relevant directory to load its native instructions; sessions started at the root must read the applicable source rules listed in the root task table. Longer procedures and reference material live in skills and their bundled files, including the fork census skill. A portable after-edit hook extracts edited paths from either tool and runs ShellCheck on shell files. Skills use explicit commands and request arguments in a shared body. Hook commands require agnostic-ai and ShellCheck on the client's PATH. Review and trust the generated hook through `/hooks` in Codex before relying on automatic checks. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. The default model tier is `balanced`: Sonnet for Claude and `gpt-6.1-sol` at medium effort for Codex. `code-locator` locates code on Haiku or `gpt-6-luna` at low effort, `code-implementer` implements bounded changes on the balanced tier, and `code-reviewer` reviews on Opus or `gpt-6.1-sol` at high effort. These are task assignments, not speed or quality benchmarks. Codex permits at most two concurrent delegates. The coordinator runs the full gate once after all edits. Lookup and review agents prohibit writes in their prompts and request only read and shell capabilities. Each client applies its own capability support and session sandbox; these prompts do not impose a separate filesystem restriction. Override model tiers locally when a model is unavailable in your account; do not commit personal model or permission choices. diff --git a/.gitignore b/.gitignore index 55e003ca9..e2b5499aa 100644 --- a/.gitignore +++ b/.gitignore @@ -57,4 +57,7 @@ dev.log /AGENTS.md /CLAUDE.md /agnostic-ai.local.yaml +/src/AGENTS.md +/tests/AGENTS.md +/tests/acceptance/AGENTS.md # <<< agnostic-ai (managed) <<< diff --git a/.worktreeinclude b/.worktreeinclude index b029198cc..b358a2f02 100644 --- a/.worktreeinclude +++ b/.worktreeinclude @@ -18,4 +18,7 @@ /AGENTS.md /CLAUDE.md /agnostic-ai.local.yaml +/src/AGENTS.md +/tests/AGENTS.md +/tests/acceptance/AGENTS.md # <<< agnostic-ai (managed) <<< diff --git a/CHANGELOG.md b/CHANGELOG.md index b0eed0061..8f3e44e1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - JSON reports write ordinary filenames, test names and empty messages without per-field processes, preserving existing escaping (#1369) ### Fixed +- Bash compatibility checks scan shell source files without treating generated contributor instructions as executable code - Concurrent provider rows have separate temporary-file owners, including rows with identical arguments (#1379) - Skipped macOS and Windows acceptance CI jobs have readable names, with shard labels in their test steps - Cancellation regression checks distinguish exited bodies awaiting reaping from running cleanup (#1385) diff --git a/tests/unit/project/bash_compatibility_test.sh b/tests/unit/project/bash_compatibility_test.sh index 315404afb..ebc05d8a5 100644 --- a/tests/unit/project/bash_compatibility_test.sh +++ b/tests/unit/project/bash_compatibility_test.sh @@ -11,11 +11,24 @@ # example preferring `[ ]` over `[[ ]]`) are not compatibility rules and are not # enforced by these tests -- `[[ ]]` works on every bash we support. -# Returns offending "file:line: text" for a pattern under a directory, skipping -# comment lines so a rule quoted in documentation or in a comment is not an -# error. +# Return offending shell source lines, excluding comments and documentation. function bashunit::compat::offenders_in() { - grep -rnE "$2" "$1" 2>/dev/null | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' || true + find "$1" -name '*.sh' -type f -print0 | + xargs -0 grep -nHE "$2" 2>/dev/null | grep -vE '^[^:]+:[0-9]+:[[:space:]]*#' || true +} + +function test_compatibility_scan_ignores_documentation_and_checks_nested_shell_files() { + local dir + dir="$(bashunit::temp_dir)" + mkdir -p "$dir/nested sources" + printf '%s\n' 'declare -A documented' >"$dir/AGENTS.md" + printf '%s\n' 'declare -A unsupported' >"$dir/nested sources/runtime.sh" + + local hits + hits="$(bashunit::compat::offenders_in "$dir" 'declare -A')" + + assert_not_contains "AGENTS.md" "$hits" + assert_contains "nested sources/runtime.sh:1:declare -A unsupported" "$hits" } function bashunit::compat::offenders() { @@ -133,7 +146,7 @@ function test_src_has_no_bare_hash_or_percent_substitution_pattern() { } # `printf -v` is Bash 3.1+. Use the return-slot pattern documented in -# .claude/rules/bash-style.md instead (which also avoids its dynamic-scope trap). +# .agnostic-ai/rules/bash-style.md instead (which also avoids its dynamic-scope trap). function test_src_has_no_printf_assignment() { assert_empty "$(bashunit::compat::ungated_offenders 'printf[[:space:]]+(-[a-zA-Z]*v)' 31)" } @@ -282,7 +295,7 @@ function test_src_has_no_mapfile_or_readarray() { } # declare -n / local -n (namerefs) are Bash 4.3+. Use the return-slot pattern -# documented in .claude/rules/bash-style.md instead. +# documented in .agnostic-ai/rules/bash-style.md instead. function test_src_has_no_namerefs() { assert_empty "$(bashunit::compat::ungated_offenders \ '(declare|local|typeset)[[:space:]]+(-[a-zA-Z]*n)[[:space:]]' 43)" @@ -461,7 +474,8 @@ function test_a_gate_does_not_carry_over_into_the_next_file() { # on every shell -- green, silently slower, and impossible to see in review. function test_every_gate_in_src_names_a_declared_flag() { local used declared missing - used="$(grep -rhoE '_BASHUNIT_BASH_GE_[0-9]+' src/ | LC_ALL=C sort -u)" + used="$(find src -name '*.sh' -type f -print0 | + xargs -0 grep -hoE '_BASHUNIT_BASH_GE_[0-9]+' | LC_ALL=C sort -u)" declared="$(grep -hoE '^_BASHUNIT_BASH_GE_[0-9]+' src/system/bash.sh | LC_ALL=C sort -u)" missing="" From 26b335983dd27f76afd81d93b74d6dfc40c11875 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 15:01:32 +0200 Subject: [PATCH 4/5] chore: pin project agent tooling --- .agnostic-ai/AGNOSTIC_AI.md | 2 +- .agnostic-ai/scripts/shellcheck-after-edit.sh | 3 +- .github/CONTRIBUTING.md | 23 ++- .github/workflows/check-agent-specs.yml | 27 ++-- CHANGELOG.md | 2 +- package-lock.json | 146 ++++++++++++++++++ package.json | 9 +- tests/unit/project/package_json_test.sh | 10 +- 8 files changed, 191 insertions(+), 31 deletions(-) create mode 100644 package-lock.json diff --git a/.agnostic-ai/AGNOSTIC_AI.md b/.agnostic-ai/AGNOSTIC_AI.md index 3121600f7..76e0e34f3 100644 --- a/.agnostic-ai/AGNOSTIC_AI.md +++ b/.agnostic-ai/AGNOSTIC_AI.md @@ -39,7 +39,7 @@ Workers share the checkout: assign separate files, preserve others' edits, and r ## Specs and delivery -Edit `.agnostic-ai/` or `agnostic-ai.yaml`, never generated instructions, agents, skills, hooks, or settings. Run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. +Edit `.agnostic-ai/` or `agnostic-ai.yaml`, never generated instructions, agents, skills, hooks, or settings. Install the pinned contributor dependencies with `npm ci` (use `npm ci --force` for a Windows source checkout because the published npm package restricts its OS). Run `npm run agents:sync` and `npm run agents:check`, which use the project-local CLI. Keep unrelated changes out of the PR. Update CHANGELOG for user-visible changes. Use the `pr` skill for pull requests. Use Conventional Commits: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, or `chore`, with an optional domain scope such as `assert`, `runner`, `cli`, or `docs`. diff --git a/.agnostic-ai/scripts/shellcheck-after-edit.sh b/.agnostic-ai/scripts/shellcheck-after-edit.sh index 353f564f3..c1e0d622d 100644 --- a/.agnostic-ai/scripts/shellcheck-after-edit.sh +++ b/.agnostic-ai/scripts/shellcheck-after-edit.sh @@ -5,7 +5,8 @@ if [ -n "${1:-}" ]; then export AGNOSTIC_AI_TARGET="$1" fi -files=$(agnostic-ai hook paths) || exit 1 +project_root=$(git rev-parse --show-toplevel) +files=$("$project_root/node_modules/.bin/agnostic-ai" hook paths) || exit 1 printf '%s\n' "$files" | while IFS= read -r file; do case "$file" in *.sh) diff --git a/.github/CONTRIBUTING.md b/.github/CONTRIBUTING.md index 63ea3324e..4ad7c4c23 100644 --- a/.github/CONTRIBUTING.md +++ b/.github/CONTRIBUTING.md @@ -34,6 +34,7 @@ Contributions are licensed under the [MIT License](https://github.com/TypedDevs/ - Bash 3.0+ - Git - Make +- Node.js (use the version in `.nvmrc`) and npm for contributor tooling - [ShellCheck](https://github.com/koalaman/shellcheck#installing) - [editorconfig-checker](https://github.com/editorconfig-checker/editorconfig-checker#installation) - [shfmt](https://github.com/mvdan/sh) and [jq](https://jqlang.org/) when building the standalone binary @@ -46,6 +47,11 @@ git clone https://github.com/YOUR_USERNAME/bashunit.git cd bashunit git remote add upstream https://github.com/TypedDevs/bashunit.git +# Install pinned contributor dependencies and generate agent configuration +npm ci +npm run agents:sync +npm run agents:check + # Install pre-commit hooks make pre_commit/install @@ -53,22 +59,15 @@ make pre_commit/install make test && make sa && make lint ``` -### Coding Agent Setup (Optional) - -The project uses [agnostic-ai](https://agnostic-ai.org/) 0.82.0 for Claude Code and Codex. Install the pinned version, then generate the ignored native configuration from the repository root: +For a Windows source checkout in Git Bash, replace `npm ci` with `npm ci --force`: the root manifest restricts the published npm package to macOS and Linux. -```bash -curl -fsSL https://raw.githubusercontent.com/Chemaclass/agnostic-ai/v0.82.0/scripts/install.sh -o /tmp/bashunit-agnostic-ai-install.sh -AGNOSTIC_AI_VERSION=v0.82.0 bash /tmp/bashunit-agnostic-ai-install.sh -agnostic-ai sync -agnostic-ai sync --check -``` +### Coding Agent Setup -On Windows, use the [Windows installer](https://agnostic-ai.org/docs/installation/) with version `v0.82.0`. If the CLI is already installed, `agnostic-ai upgrade --version v0.82.0` installs the pinned release for a standalone binary; package-manager users pin through their package manager. +The root `package.json` pins [agnostic-ai](https://agnostic-ai.org/) 0.82.0 as a development dependency. `npm ci` installs it from the committed lockfile. The setup commands above generate Claude Code and Codex configuration using that local version; a global installation is not required. -Edit `.agnostic-ai/` or `agnostic-ai.yaml`, then run `agnostic-ai validate`, `agnostic-ai lint --strict`, `agnostic-ai sync`, and `agnostic-ai sync --check`. Generated files are ignored. Run sync after cloning and after pulling changes to the specs. Keep personal overrides in `.agnostic-ai/local/` or `agnostic-ai.local.yaml`, which are also ignored. +Edit `.agnostic-ai/` or `agnostic-ai.yaml`, then run `npm run agents:sync` and `npm run agents:check`. Generated files are ignored. Run sync after cloning and after pulling changes to the specs. Keep personal overrides in `.agnostic-ai/local/` or `agnostic-ai.local.yaml`, which are also ignored. -Standing conventions live in `.agnostic-ai/rules/`. Directory scopes produce path-scoped Claude rules and nested Codex `AGENTS.md` files. Start a session in the relevant directory to load its native instructions; sessions started at the root must read the applicable source rules listed in the root task table. Longer procedures and reference material live in skills and their bundled files, including the fork census skill. A portable after-edit hook extracts edited paths from either tool and runs ShellCheck on shell files. Skills use explicit commands and request arguments in a shared body. Hook commands require agnostic-ai and ShellCheck on the client's PATH. Review and trust the generated hook through `/hooks` in Codex before relying on automatic checks. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. +Standing conventions live in `.agnostic-ai/rules/`. Directory scopes produce path-scoped Claude rules and nested Codex `AGENTS.md` files. Start a session in the relevant directory to load its native instructions; sessions started at the root must read the applicable source rules listed in the root task table. Longer procedures and reference material live in skills and their bundled files, including the fork census skill. A portable after-edit hook extracts edited paths from either tool and runs ShellCheck on shell files. Skills use explicit commands and request arguments in a shared body. The hook uses the project's installed agnostic-ai dependency and requires Node.js and ShellCheck on the client's PATH. Review and trust the generated hook through `/hooks` in Codex before relying on automatic checks. Claude's skill tool restrictions and invocation metadata have no equivalent in Codex; both tools still require the full quality gate before committing. The default model tier is `balanced`: Sonnet for Claude and `gpt-6.1-sol` at medium effort for Codex. `code-locator` locates code on Haiku or `gpt-6-luna` at low effort, `code-implementer` implements bounded changes on the balanced tier, and `code-reviewer` reviews on Opus or `gpt-6.1-sol` at high effort. These are task assignments, not speed or quality benchmarks. Codex permits at most two concurrent delegates. The coordinator runs the full gate once after all edits. Lookup and review agents prohibit writes in their prompts and request only read and shell capabilities. Each client applies its own capability support and session sandbox; these prompts do not impose a separate filesystem restriction. Override model tiers locally when a model is unavailable in your account; do not commit personal model or permission choices. diff --git a/.github/workflows/check-agent-specs.yml b/.github/workflows/check-agent-specs.yml index d5cd31a09..6e171b2c9 100644 --- a/.github/workflows/check-agent-specs.yml +++ b/.github/workflows/check-agent-specs.yml @@ -5,6 +5,9 @@ on: paths: - '.agnostic-ai/**' - 'agnostic-ai.yaml' + - 'package.json' + - 'package-lock.json' + - '.nvmrc' - '.gitignore' - '.worktreeinclude' - '.github/workflows/check-agent-specs.yml' @@ -13,6 +16,9 @@ on: paths: - '.agnostic-ai/**' - 'agnostic-ai.yaml' + - 'package.json' + - 'package-lock.json' + - '.nvmrc' - '.gitignore' - '.worktreeinclude' - '.github/workflows/check-agent-specs.yml' @@ -25,20 +31,19 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 - - name: Install pinned agnostic-ai - env: - AGNOSTIC_AI_VERSION: v0.82.0 - AGNOSTIC_AI_INSTALL_DIR: ${{ runner.temp }}/agnostic-ai-bin + - uses: actions/setup-node@v4 + with: + node-version-file: .nvmrc + cache: npm + cache-dependency-path: package-lock.json + - name: Install contributor dependencies run: | - curl -fsSL https://raw.githubusercontent.com/Chemaclass/agnostic-ai/v0.82.0/scripts/install.sh -o "$RUNNER_TEMP/agnostic-ai-install.sh" - bash "$RUNNER_TEMP/agnostic-ai-install.sh" - echo "$AGNOSTIC_AI_INSTALL_DIR" >> "$GITHUB_PATH" + npm ci --ignore-scripts + echo "$GITHUB_WORKSPACE/node_modules/.bin" >> "$GITHUB_PATH" - name: Validate and generate both targets run: | - agnostic-ai validate - agnostic-ai lint --strict - agnostic-ai sync - agnostic-ai sync --check + npm run agents:sync + npm run agents:check git diff --exit-code - name: Verify shared ShellCheck hook run: | diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f3e44e1c..9856aaa3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ ## Unreleased ### Changed -- Contributor instructions use shared agnostic-ai specs for Claude Code and Codex, with task-specific context and model tiers +- Contributor instructions use shared agnostic-ai specs for Claude Code and Codex, with a pinned npm development dependency, task-specific context, and model tiers - Sequential multi-file runs reuse provider and annotation metadata from header counting for unchanged files up to 512 characters (#1372) - Parallel reports collect worker results without per-row base64 processes, preserving values and completion order (#1371) - Data providers pass arguments without per-argument base64 processes, preserving quoting, empty values and parser isolation (#1370) diff --git a/package-lock.json b/package-lock.json new file mode 100644 index 000000000..c8639d201 --- /dev/null +++ b/package-lock.json @@ -0,0 +1,146 @@ +{ + "name": "bashunit", + "version": "0.51.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "bashunit", + "version": "0.51.0", + "license": "MIT", + "os": [ + "darwin", + "linux" + ], + "bin": { + "bashunit": "bin/bashunit" + }, + "devDependencies": { + "agnostic-ai": "0.82.0" + } + }, + "node_modules/@agnostic-ai/darwin-arm64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/darwin-arm64/-/darwin-arm64-0.82.0.tgz", + "integrity": "sha512-i2H3gz0AXZ8cAkYbEHSIBAGqvxvm79ELtZKhKpT9T8bt5cqzIruWkfbXodCdbFRl3VF/49LQqclXJz8a3OnrEw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@agnostic-ai/darwin-x64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/darwin-x64/-/darwin-x64-0.82.0.tgz", + "integrity": "sha512-qBn5GEOT3y2wRSR2Mzy98cpzJcDlAZTucat5EJDqPCrfqjl50tP95/+oHB8/YevfrJtcch/ZXWKXoAf4WvIbQQ==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "darwin" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@agnostic-ai/linux-arm64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/linux-arm64/-/linux-arm64-0.82.0.tgz", + "integrity": "sha512-SRafUWRqmcH+E+Zl4IfmbydZ8iCg7vjHvTdPZVgcz1XPMLX4mBJr1vzBPbqgRiZqR5vl+3laZCUVbgS992YUEw==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@agnostic-ai/linux-x64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/linux-x64/-/linux-x64-0.82.0.tgz", + "integrity": "sha512-M8MfvrEZCswAivA3Gdn68cl2t7bsWVb7qZ/YNDodEpEwFgcwyM27cgZ2KcPEdJm2elRZrHgAJU1olhQT4/MyxA==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "linux" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@agnostic-ai/win32-arm64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/win32-arm64/-/win32-arm64-0.82.0.tgz", + "integrity": "sha512-nTq6+ODuRRMxHQAxTHPLYKOZ2kBxp2VP6JgdkkFqAT8c7VDVEr2JN0enNbeftzVdniKgtDGkWjpLKbulRbI8ow==", + "cpu": [ + "arm64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/@agnostic-ai/win32-x64": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/@agnostic-ai/win32-x64/-/win32-x64-0.82.0.tgz", + "integrity": "sha512-p3BcBvlxDsUga2Lw1WDTdqihz/m3MFGB2oNRLNA86jjFMm5KkObQ9uGHF/dDbjY5zFj1p5wabhgIstpfjyi7mw==", + "cpu": [ + "x64" + ], + "dev": true, + "license": "MIT", + "optional": true, + "os": [ + "win32" + ], + "engines": { + "node": ">=18" + } + }, + "node_modules/agnostic-ai": { + "version": "0.82.0", + "resolved": "https://registry.npmjs.org/agnostic-ai/-/agnostic-ai-0.82.0.tgz", + "integrity": "sha512-wvsD5lhKpvv5bkEYjBBM99uNR0B27lPsgMSI15CjzZjNszV2UsV33OyR4l1LfcNrCEHmJrq2MXvJ6B2xGbnNIw==", + "dev": true, + "license": "MIT", + "bin": { + "agnostic-ai": "bin/agnostic-ai.js" + }, + "engines": { + "node": ">=18" + }, + "optionalDependencies": { + "@agnostic-ai/darwin-arm64": "0.82.0", + "@agnostic-ai/darwin-x64": "0.82.0", + "@agnostic-ai/linux-arm64": "0.82.0", + "@agnostic-ai/linux-x64": "0.82.0", + "@agnostic-ai/win32-arm64": "0.82.0", + "@agnostic-ai/win32-x64": "0.82.0" + } + } + } +} diff --git a/package.json b/package.json index 4f7365b1e..18f213cbb 100644 --- a/package.json +++ b/package.json @@ -35,5 +35,12 @@ "bashunit", "tdd", "assertions" - ] + ], + "devDependencies": { + "agnostic-ai": "0.82.0" + }, + "scripts": { + "agents:sync": "agnostic-ai sync", + "agents:check": "agnostic-ai validate && agnostic-ai lint --strict && agnostic-ai sync --check" + } } diff --git a/tests/unit/project/package_json_test.sh b/tests/unit/project/package_json_test.sh index 918ba2412..069bde77f 100644 --- a/tests/unit/project/package_json_test.sh +++ b/tests/unit/project/package_json_test.sh @@ -81,18 +81,20 @@ function test_package_json_excludes_docs_dependencies() { assert_not_contains '"vue"' "$pkg" } -function test_package_json_declares_no_dependencies_block() { +function test_package_json_declares_no_runtime_dependencies() { local pkg pkg=$(cat "$PKG_FILE") assert_not_matches '"dependencies"[[:space:]]*:' "$pkg" - assert_not_matches '"devDependencies"[[:space:]]*:' "$pkg" + assert_not_matches '"optionalDependencies"[[:space:]]*:' "$pkg" assert_not_matches '"peerDependencies"[[:space:]]*:' "$pkg" } -function test_package_json_declares_no_scripts_block() { +function test_package_json_declares_no_install_lifecycle_scripts() { local pkg pkg=$(cat "$PKG_FILE") - assert_not_matches '"scripts"[[:space:]]*:' "$pkg" + local lifecycle + lifecycle='"(preinstall|install|postinstall|prepublish|prepare|preprepare|postprepare)"[[:space:]]*:' + assert_not_matches "$lifecycle" "$pkg" } function test_docs_package_json_marked_private() { From 096601a567490ac9d6e6dc6416cbcdf191a9a875 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 15:18:07 +0200 Subject: [PATCH 5/5] fix: keep edit hooks working in new worktrees --- .worktreeinclude | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.worktreeinclude b/.worktreeinclude index b358a2f02..86886535f 100644 --- a/.worktreeinclude +++ b/.worktreeinclude @@ -1,3 +1,6 @@ +# Generated edit hooks require the pinned contributor dependency in each worktree. +/node_modules/ + # >>> agnostic-ai (managed) >>> # Generated paths. Edit specs, not this block. Run `agnostic-ai sync` to refresh. # Copied into each Claude Code worktree: the gitignored paths sync writes, and the local layer it reads.