From 35e2d2b939c6a4d1e0e92b71020a316e54eefac0 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Sun, 11 Oct 2026 20:00:05 +0200 Subject: [PATCH] chore: streamline shared agent guidance --- .agnostic-ai/AGNOSTIC_AI.md | 6 +- .agnostic-ai/rules/architecture-map.md | 93 ++-------- .agnostic-ai/rules/bash-style.md | 164 +++--------------- .agnostic-ai/rules/testing.md | 136 +++------------ .agnostic-ai/skills/add-assertion/SKILL.md | 58 +------ .agnostic-ai/skills/check-coverage/SKILL.md | 54 +----- .agnostic-ai/skills/commit/SKILL.md | 91 +--------- .../skills/commit/references/formatting.md | 4 +- .agnostic-ai/skills/fix-test/SKILL.md | 52 +----- .agnostic-ai/skills/gh-issue/SKILL.md | 119 +------------ .agnostic-ai/skills/gh-issues/SKILL.md | 162 ++--------------- .agnostic-ai/skills/pr/SKILL.md | 107 ++---------- .agnostic-ai/skills/pre-release/SKILL.md | 92 ++-------- .agnostic-ai/skills/release/SKILL.md | 77 +------- .agnostic-ai/skills/review/SKILL.md | 40 +---- .agnostic-ai/skills/tdd-cycle/SKILL.md | 46 +---- .../skills/tdd-cycle/references/workflow.md | 77 +++----- CHANGELOG.md | 2 +- 18 files changed, 194 insertions(+), 1186 deletions(-) diff --git a/.agnostic-ai/AGNOSTIC_AI.md b/.agnostic-ai/AGNOSTIC_AI.md index 76e0e34f..ff39728e 100644 --- a/.agnostic-ai/AGNOSTIC_AI.md +++ b/.agnostic-ai/AGNOSTIC_AI.md @@ -16,11 +16,11 @@ All paths are relative to the repository root, including from generated skill an 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. -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. +Use the TDD rule for runtime changes and its `tdd-cycle` skill for nontrivial plans. Preserve public APIs unless the task includes documentation and CHANGELOG updates; record architectural decisions in an ADR. ## Quality gate -Run once after the complete change set, before committing: +The main session runs this gate once after the complete change set, before committing. Targeted checks support RED/GREEN or diagnosis; reuse passing checks while content is unchanged: ```bash ./bashunit tests/ @@ -41,6 +41,6 @@ Workers share the checkout: assign separate files, preserve others' edits, and r 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`. +Keep unrelated changes out of the delivery. Use a PR unless the user requests a direct commit to main. Update CHANGELOG for user-visible changes. Use the `pr` skill for pull requests. Use Conventional Commits: `feat`, `fix`, `perf`, `docs`, `style`, `refactor`, `test`, or `chore`, with an optional domain scope such as `assert`, `runner`, `cli`, or `docs`. 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/rules/architecture-map.md b/.agnostic-ai/rules/architecture-map.md index cb6e8017..fe9fa32a 100644 --- a/.agnostic-ai/rules/architecture-map.md +++ b/.agnostic-ai/rules/architecture-map.md @@ -2,44 +2,18 @@ scope: src --- -# Architecture Map: modules and the life of a run +# Architecture map -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. +Repository paths are relative to the root. Module paths below are under `src/` unless stated otherwise. Use function names as anchors; verify the current caller before changing a path. -## The life of `./bashunit tests/` +## Run lifecycle -``` -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). +1. `bashunit` checks the Bash version, scans early flags, sources modules, and routes to `bashunit::main::cmd_test`. +2. `runner/discovery.sh` discovers and loads files, checks duplicates, builds metadata and filters selected functions. Header counting in `helper/discovery.sh` uses static metadata where safe and sources dynamic files only when needed. Sequential multi-file counting caches snapshots and metadata for unchanged files up to 512 bytes; `helper/provider.sh` validates the path and contents before reuse. +3. The header calls `find_total_tests` directly and reads its return slot. Do not wrap it in `$()`, which discards shell-state cache changes. +4. `runner/exec.sh` dispatches tests and providers, captures the test body in a subshell, and manages retries, timeouts and hooks. `runner/payload.sh` and `state/payload.sh` transport results; mutations inside a test do not update parent counters directly. +5. `runner/result.sh` handles synchronous results or writes parallel `.result` files. Under `--parallel`, file dispatch also runs in a background subshell; results and explicit files carry state back. `state/parallel.sh` aggregates worker results in the parent. +6. Console and report modules render results. Rerun persistence, temporary-output cleanup and exit status complete the run. ## Module ownership @@ -107,54 +81,23 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). | `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, 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 | +| `cli/{upgrade,watch,doc,init}.sh` | the non-`test` subcommand implementations; main subcommand routing calls them | | `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. +- Follow `.agnostic-ai/rules/bash-style.md` for Bash 3.0 parsing and return slots. `local` is dynamically scoped; helper locals can shadow caller-named outputs. +- Follow `.agnostic-ai/rules/perf-fork-budget.md` for process costs. Preserve the acceptance budgets rather than assuming every path has the same allowance. +- Background file dispatch cannot return counters or flags through shell state. Put whole-file validation before dispatch or return failures through explicit results. Always test sequential and parallel modes when changing this boundary. +- Use dispatcher ordinals for worker identity. Subshells share `$$`; `RANDOM` reseeding varies by platform and nesting and cannot guarantee uniqueness. +- Framework binaries are pinned at startup (`$GREP`, `$MKTEMP`, `$CAT` in `config/env.sh`) so test doubles and PATH changes cannot hijack internal plumbing. +- Snapshots use an 80-column non-TTY fallback. Rendering width changes affect `tests/acceptance/snapshots/`. +- `build.sh` emits a file's body before recursively flattening its source lines and deduplicates by repository-relative path. Module `index.sh` files must contain only source lines and comments; other statements can run in a different order in the built artifact. Every source file starts with a shebang, which the build strips. See ADR-010. diff --git a/.agnostic-ai/rules/bash-style.md b/.agnostic-ai/rules/bash-style.md index 307f94f3..0acc14af 100644 --- a/.agnostic-ai/rules/bash-style.md +++ b/.agnostic-ai/rules/bash-style.md @@ -4,143 +4,51 @@ paths: - "tests/**" --- -# 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 style and compatibility -```bash -## -# Brief description -# Arguments: $1 - desc, $2 - desc (optional, default: "x") -# Returns: 0 success, 1 failure -## -``` - -### File Structure - -Constants -> Globals -> Private functions -> Public functions +Runtime code must parse and work on Bash 3.0+. `bashunit` checks both version components. Use `tests/unit/project/bash_compatibility_test.sh` and existing version gates; do not ban a newer construct when an established gate safely selects a supported body. A runtime gate cannot protect a parse error. -Source deps relative to script: `"$(dirname "${BASH_SOURCE[0]}")/dep.sh"` +| Construct | Minimum Bash | Portable approach | +|-----------|--------------|-------------------| +| `printf -v`, string `+=` | 3.1 | Return slot; `var="$var$more"` | +| Array `+=(x)` | 3.1 (parse error on 3.0) | `arr[${#arr[@]}]=x` | +| `declare -A` | 4.0 | Indexed arrays | +| `${var,,}`, `${var^^}` | 4.0 | `tr` for case conversion | +| `&>>` | 4.0 | `>> file 2>&1` | +| `BASHPID` | 4.0 | Dispatcher ordinal or `mktemp` | +| `mapfile`, `readarray` | 4.0 | `while IFS= read -r` | +| `coproc` | 4.0 | Existing portable worker dispatch | +| Negative array indices | 4.3 | `${array[${#array[@]}-1]}` | +| `declare -n`, `local -n` | 4.3 | Return slot | +| `${var@Q}` | 4.4 | Existing argument transport helpers | -## Returning values from a helper (dynamic-scope safety) +`[[ =~ ]]` exists on 3.0, but regex quoting changed in 3.2. Follow existing `grep -E` helpers and the process-budget rules rather than introducing version-dependent matching. Prefer `[ ]` over `[[ ]]` for ordinary tests; that is house style, not a compatibility requirement. -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. +Use 2-space indentation, quoted variables, `$()` instead of backticks, and existing function namespaces. Public helpers use `bashunit::*`, private helpers `_name`, locals `lowercase_with_underscores`, and globals `UPPERCASE_WITH_UNDERSCORES`. Match sibling file structure and source dependencies relative to the script. `make lint` is the formatting authority; never run `shfmt -w`. Use a comment only for a constraint the code cannot express. ShellCheck suppressions need a specific reason. -We need **all three** properties on the hot path: +## Return values and dynamic scope -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). +Bash `local` is dynamically scoped. A callee's local can hide a caller variable accessed by name through `${!name}`, `eval`, `printf -v` or `export`. Hot-path `$()` captures also fork. -### Preferred: dedicated global return slot +Prefer one reserved global return slot per helper: ```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 + local input=$1 + _BASHUNIT_PKG_THING_OUT=${input%%]*} } -# 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_*`). +Use natural local names and `_BASHUNIT_*` slots; distinct slots avoid nested-call clobbering. See `src/runner/payload.sh` and `src/coverage/branches.sh`. -### Last-resort fallback: outvar by name + `__bu_` prefix on locals +For mock or spy state, namespace the constructed variable itself, for example `_BASHUNIT_SPY_${variable}_TIMES_FILE`. See `src/doubles/spy.sh`. -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): +Only when a dedicated slot or namespaced state cannot fit, accept an output variable name and prefix every helper local with `__bu_`: ```bash function bashunit::pkg::do_thing() { @@ -151,24 +59,6 @@ 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 - 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 +Test caller names that collide with helper locals. Do not introduce unprefixed callee locals in this 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 -``` +The `_branch_*` helpers in `src/coverage/branches.sh` deliberately mutate `extract_branches` locals. Keep them together (ADR-010); new intentional dynamic-scope mutation needs an explicit one-line constraint. diff --git a/.agnostic-ai/rules/testing.md b/.agnostic-ai/rules/testing.md index 31413fee..dfd81385 100644 --- a/.agnostic-ai/rules/testing.md +++ b/.agnostic-ai/rules/testing.md @@ -2,133 +2,45 @@ scope: tests --- -# Testing Guidelines +# Testing -## Organization +Read sibling tests before writing a new fixture. Repository paths are relative to the repository root. -| 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 | +| Layer | Directory | Scope | +|-------|-----------|-------| +| Unit | `tests/unit/` | Isolated helpers, mocked dependencies | +| Functional | `tests/functional/` | Real component interactions | +| Acceptance | `tests/acceptance/` | CLI and end-to-end behavior | -**Naming:** Files end with `_test.sh`. Functions: `test_should__when_` +Files end in `_test.sh`; name tests for the behavior and trigger. Use targeted RED/GREEN checks during implementation, then the root quality gate once the complete change set is ready. Test success, failure and cleanup paths that the change affects; verify parallel behavior when state or workers change. -## Assertions +## Assertions and doubles -```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: +Assertions are bare functions (`assert_equals`, `assert_same`, `assert_contains`, `assert_not_contains`, `assert_successful_code`, `assert_general_error`, `assert_file_exists`, `assert_file_not_exists`). Helpers and doubles use `bashunit::*`. Inspect the matching assertion before guessing its signature. There is no `assert_fails`. ```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_times 2 function_name +assert_have_been_called_with function_name "arg" +assert_have_been_called_with function_name "arg" 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 +assert_have_been_called_with_args function_name "a b" 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 -} +bashunit::mock uname <<< "Linux" ``` -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. +`_times` takes the count first; `_with` takes the spy first and an optional call index last. `_with_args` compares argument boundaries. To test a failing assertion, compare its captured output with the corresponding renderer, as sibling tests do. -It cannot be fixed by tracking quote state, an apostrophe in any comment -(`# doesn't work`) would flip it and mask real duplicates. +Providers emit one argument row per line and are selected with `# @data_provider `. Keep parser quoting, empty arguments and row isolation intact. Hooks are `set_up_before_script`, `set_up`, `tear_down`, and `tear_down_after_script`; the script hooks run once per file, the others per test. -## Test Isolation +Use `assert_match_snapshot` or `assert_match_named_snapshot`; update snapshots deliberately with `./bashunit --snapshot-update --filter tests/`. -- 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/` +## Isolation -## Reference Tests +- Use `$(bashunit::temp_file)` or `$(bashunit::temp_dir)` for framework-cleaned fixtures. No shared fixed temporary paths or global state between tests. +- Mock network calls and nondeterministic external dependencies. Tests must work under `./bashunit --parallel tests/`. +- Embedded test-function definitions are scanned textually by the duplicate check, even inside strings. Give every embedded definition a unique name across its containing file; quote-state heuristics cannot safely hide them. +- Strict CI uses `set -euo pipefail`: initialize arrays, guard optional elements, use `if` for conditional side effects, and isolate `extdebug` in a subshell. Simple output contains markers, so test rendering helpers rather than assuming full `print_line` output. -- **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` +Start with `tests/unit/assert/`, `tests/functional/doubles_test.sh`, `tests/functional/provider_test.sh`, `tests/unit/runner/hooks_test.sh`, and `tests/acceptance/bashunit_test.sh`. Use `rg` to locate the matching function tests before adding a pattern. diff --git a/.agnostic-ai/skills/add-assertion/SKILL.md b/.agnostic-ai/skills/add-assertion/SKILL.md index a48c0cc3..19ae8569 100644 --- a/.agnostic-ai/skills/add-assertion/SKILL.md +++ b/.agnostic-ai/skills/add-assertion/SKILL.md @@ -4,56 +4,12 @@ description: Add new assertion function with comprehensive tests following TDD allowed-tools: read, edit, write, shell, Grep, Glob --- -# Add Assertion Skill +# Add an assertion -Add a new assertion function to bashunit following strict TDD methodology. +Repository paths are relative to the repository root. Establish the assertion name, arguments, success and failure behavior, and messages from the request and sibling assertions; ask only for missing decisions. Record acceptance criteria and test inventory in `.tasks/YYYY-MM-DD-add-assertion-name.md` for nontrivial work. -## 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` - -## Workflow - -### 1. Plan - -Ask user: assertion name, parameters, success/failure behavior, error messages. - -Document acceptance criteria and test inventory in task file. - -### 2. Study Existing Patterns - -Read `src/assertions.sh` and `tests/unit/assert_test.sh` to understand: -- Parameter validation, assertion logic, error message format -- Return codes (0 = success, 1 = failure) -- How tests verify both success and failure cases - -### 3. TDD Cycles - -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 - `bashunit::console_results::print_failed_test` (there is no `assert_fails`) -3. **Edge cases**, empty input, special characters, nested structures, malformed data - -### 4. Integration - -- Source new file in `src/bashunit.sh` if created -- `export -f` the assertion function -- Run full test suite: `./bashunit tests/` -- Quality checks: `make sa && make lint` - -### 5. Documentation - -- Add function header with Arguments/Returns/Example -- Update CHANGELOG.md -- Update user-facing docs if applicable - -## Final Checklist - -- [ ] All tests passing (success, failure, edge cases) -- [ ] Function documented and exported -- [ ] Quality gate passes -- [ ] CHANGELOG updated -- [ ] Task file completed +1. Study the matching implementation under `src/assert/` and tests under `tests/unit/assert/`, including parameter validation, return codes, and message checks. `src/assert/index.sh` lists assertion modules. +2. Use focused RED, minimal GREEN, then REFACTOR for basic success, expected failure, and relevant edge cases (empty values, special characters, malformed input). Use `assert_assertion_passes`, `assert_assertion_fails`, or `assert_assertion_fails_with` from `src/assert/assertions.sh` where appropriate. Match existing output comparisons when needed; there is no `assert_fails`. +3. If adding a module, source it from `src/assert/index.sh`. Expose the public function through the existing module-loading pattern. Preserve Bash 3.0 parse compatibility and the existing public contract. +4. Document the public assertion's arguments, returns, and example in user-facing docs, update CHANGELOG, and complete the task file when used. Add code comments only for a constraint the code cannot express. +5. The main session runs the canonical quality gate once after the entire change set; delegates run only assigned focused checks. Verify success, failure, edge cases, module loading, and documentation before handing off. diff --git a/.agnostic-ai/skills/check-coverage/SKILL.md b/.agnostic-ai/skills/check-coverage/SKILL.md index d2be38c2..82895f7e 100644 --- a/.agnostic-ai/skills/check-coverage/SKILL.md +++ b/.agnostic-ai/skills/check-coverage/SKILL.md @@ -4,53 +4,11 @@ description: Analyze test coverage and identify untested code paths allowed-tools: read, shell, Grep, Glob --- -# Check Coverage +# Check coverage -Analyze test coverage and identify gaps. +Repository paths are relative to the repository root. Measure the requested scope with `./bashunit --coverage `. For broad coverage, follow `.github/workflows/coverage.yml`: exclude `tests/unit/coverage/` meta-tests and fixtures, then measure the selected unit tests with `--coverage-paths src`. Running coverage meta-tests under coverage double-instruments the engine and corrupts their assertions. Keep analysis separate from any later implementation gate. -## Workflow - -### 1. Run Coverage (if available) - -```bash -BASHUNIT_COVERAGE=true ./bashunit tests/ -``` - -### 2. Map Functions to Tests - -For each file in `src/`: - -```bash -# List all public functions -grep "^function bashunit::" src/module.sh - -# Find tests referencing each function -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 - -### 4. Identify Critical Gaps - -**Priority 1:** Public API functions (`export -f`) with no tests -**Priority 2:** Error handling paths (`return 1`, `exit 1`) not tested -**Priority 3:** Complex conditionals without branch coverage - -### 5. Generate Report - -Output a markdown report with: -- Summary (total functions, tested count, estimated %) -- Coverage by module -- Critical gaps with file:line references -- Recommended tests to add (specific test function names) - -## Coverage Goals - -- 90%+ for public functions -- All user-facing CLI commands tested -- All error paths tested -- Internal helpers: optional (if trivial) +1. Map functions under `src/` to direct tests and observed coverage. Search definitions with `rg '^function ' src/` and usages of each relevant name under `tests/`. Public assertions may lack the `bashunit::` prefix; inspect the public assertion and API surfaces and their loaded module entrypoints. A text match alone does not prove a test executes a function. +2. Classify each relevant function as well tested, partially tested, or untested using success, edge, and failure-path evidence. State instrumentation limits, including shell-version differences, before treating an uncovered line as a missing test. +3. Prioritize public APIs without tests, error paths, then complex conditionals. Aim for 90%+ public API coverage and tests for user-facing commands and error paths; trivial internal helpers are lower priority. +4. Report measured totals and coverage by module, gaps with file and line evidence, and specific proposed test cases. Separate measured percentages from estimates. Do not change production code or tests unless the request includes implementation. diff --git a/.agnostic-ai/skills/commit/SKILL.md b/.agnostic-ai/skills/commit/SKILL.md index cbc74a32..bcd16823 100644 --- a/.agnostic-ai/skills/commit/SKILL.md +++ b/.agnostic-ai/skills/commit/SKILL.md @@ -7,89 +7,12 @@ disable-model-invocation: true allowed-tools: shell, read, Grep, Glob --- -# Commit with Conventional Commits +# Commit -Read [the formatting constraints](references/formatting.md) before formatting or checking shell changes. Repository paths are relative to the repository root. +Use this skill for project commits. Read [formatting constraints](references/formatting.md) when checking shell changes; repository paths are relative to the repository root. Accept an optional message hint. -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. - -## Current State - -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` - -## Arguments -- User-supplied argument: Optional hint for the commit message (e.g., `fix the snapshot comparison`) - -## Instructions - -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.). - -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 - -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 - - Use the most specific scope that fits. Omit if changes span many areas. - -5. **Write the commit message**: - - Format: `(): ` - - Description: imperative mood, lowercase, no period, under 70 chars - - Focus on **why**, not what - - Add a body (separated by blank line) only if the why isn't obvious from the description - - Never mention AI, Claude, or automation in the message - -6. **Create the commit**: - ```bash - git commit -m "$(cat <<'EOF' - (): - - - EOF - )" - ``` - -7. **Verify** the commit was created: - ```bash - git log --oneline -1 - ``` - -## Examples - -``` -feat(assert): add assert_json_contains function -fix(runner): resolve parallel execution race condition -test(doubles): add spy verification edge cases -refactor(cli): extract option parsing into helper -docs: update installation instructions -chore(ci): upgrade shellcheck to v0.10 -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 -- **Never amend** unless the user explicitly asks -- **Always create a NEW commit**, even after a hook failure -- **Author**: use the git config identity (never commit as sandbox/default user) +1. Inspect `git status --short`, the current branch, relevant staged and unstaged diffs, and recent history. Reuse state already obtained during this task unless it may have changed. Understand the complete change and confirm the coordinator's canonical quality gate passed after the final edits; do not repeat a passing gate without new changes or unresolved evidence. +2. Stage only the relevant files by explicit name. Never use `git add .` or `git add -A`, and keep credentials out of Git. Review `git diff --cached` before committing; one commit covers one logical change. +3. Use `(): ` with the project's types: `feat`, `fix`, `perf`, `docs`, `style`, `refactor`, `test`, or `chore`. Choose a domain scope such as `assert`, `runner`, `cli`, `doubles`, `docs`, or `ci` when useful. Follow sibling history, use a lowercase imperative subject under 70 characters without a final period, and add a body only when the reason needs explanation. +4. Use the configured Git identity and `git commit` with the exact message. Write public text in the user's voice without attribution; tooling names are appropriate when tooling is the subject. Never skip hooks or amend published history. If a hook fails, fix the cause and retry a new commit rather than rewriting an existing one. +5. Verify the created commit with `git log --oneline -1` and inspect the remaining working state. A commit does not grant push authorization; use the `pr` skill for an authorized PR delivery. diff --git a/.agnostic-ai/skills/commit/references/formatting.md b/.agnostic-ai/skills/commit/references/formatting.md index ecfb557e..4c337474 100644 --- a/.agnostic-ai/skills/commit/references/formatting.md +++ b/.agnostic-ai/skills/commit/references/formatting.md @@ -5,11 +5,11 @@ no `shfmt` target, and **`shfmt -w .` must not be run**, it fights `.editorconfi 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, + `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`. + the payload record writers 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: diff --git a/.agnostic-ai/skills/fix-test/SKILL.md b/.agnostic-ai/skills/fix-test/SKILL.md index ee922a1c..f1ce74a7 100644 --- a/.agnostic-ai/skills/fix-test/SKILL.md +++ b/.agnostic-ai/skills/fix-test/SKILL.md @@ -4,51 +4,13 @@ description: Debug and fix failing tests systematically allowed-tools: read, edit, shell, Grep, Glob --- -# Fix Test +# Fix a failing test -Systematically debug and fix failing test(s). +Repository paths are relative to the repository root. Start with reported output and the failing file; use `./bashunit --filter "test_name" ` to reproduce narrowly. Discover with the full suite only when no narrower failure is known. -## Workflow +1. Trace the failure to its source and classify it: incorrect expectation/setup, implementation defect, environment/fixture issue, parallel race, or flaky external dependency. Check fixtures under `tests/fixtures/` and mocks with `assert_have_been_called ` where relevant. Temporary diagnostics go to stderr and must be removed. +2. Preserve a failing regression for the actual defect. Correct an expectation only when the intended behavior proves it wrong; otherwise make the smallest implementation fix with TDD. Repair fixtures or setup for environment defects, use the existing `$temp_dir` isolation and wait for asynchronous work for races, and mock external dependencies for flaky tests. +3. Run the focused test until it passes for the right reason. Reproduce relevant parallel behavior when investigating isolation. If the failure may predate the change, verify the same focused test on unchanged main in an isolated checkout. +4. Add a relevant edge case if it protects the root cause. After the complete change set, the coordinator runs the canonical quality gate once; delegates run only assigned targeted checks. -### 1. Identify Failures - -```bash -./bashunit tests/ 2>&1 -``` - -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 - -### 3. Fix - -- **Test bug:** correct assertion or setup -- **Implementation bug:** minimal fix in `src/`, follow TDD -- **Environment:** fix `set_up` / fixtures -- **Race condition:** use `$temp_dir` for isolation, `wait` for async -- **Flaky:** mock external dependencies - -### 4. Verify - -```bash -./bashunit path/to/fixed_test.sh # Specific test -./bashunit tests/ # Full suite -./bashunit --parallel tests/ # Isolation check -``` - -### 5. Prevent Regression - -Document root cause. Consider adding edge case tests to prevent recurrence. - -## Debugging Tips - -- Use `--filter "test_name"` to run single test -- Add `echo "DEBUG: $var" >&2` temporarily -- Check fixtures exist: `ls tests/fixtures/` -- Verify mocks: `assert_have_been_called mock_name` +Report the cause, fix, focused evidence, and any specific remaining environment limit. diff --git a/.agnostic-ai/skills/gh-issue/SKILL.md b/.agnostic-ai/skills/gh-issue/SKILL.md index fd9d5bee..c2ca4f7d 100644 --- a/.agnostic-ai/skills/gh-issue/SKILL.md +++ b/.agnostic-ai/skills/gh-issue/SKILL.md @@ -6,116 +6,15 @@ argument-hint: "" allowed-tools: shell, read, edit, write, Grep, Glob, Agent, WebFetch --- -# GitHub Issue Workflow +# Work a GitHub issue -Fetch a GitHub issue, create branch, implement following TDD, and open a PR. +Repository paths are relative to the repository root. Accept an issue number with or without `#`. -## Arguments -- User-supplied argument: Issue number (e.g., `42` or `#42`) +1. Fetch `gh issue view --json title,body,labels,assignees,milestone,state` and read every comment with `gh issue view --comments`. Inspect referenced requirements and current code before repeating prior findings. State one acceptance scenario and non-goals; do not invent decisions or mark external prerequisites complete. +2. Assign yourself if unassigned with `gh issue edit --add-assignee @me`. Apply an existing relevant label when missing: `bug`, `enhancement`, or `documentation`. Preserve another assignee's work. +3. Inspect the checkout before switching branches. Fetch main and start from current `origin/main` without discarding local changes. Use `fix/`, `feat/`, or `docs/` according to the issue labels, defaulting to `feat/`; name the branch `-` with a slug up to 50 characters. Reuse an existing issue branch when continuing its work. +4. Study related code and tests. Keep a short plan covering acceptance criteria, test strategy, owned files, and the smallest first step. Use a task file for nontrivial work and read `.agnostic-ai/rules/tdd-workflow.md`. +5. For each runtime change, prove a focused failing test, implement minimal GREEN, then refactor with focused tests green. Delegate only independent scopes; workers run assigned targeted checks. Update CHANGELOG for user-facing changes and docs for changed behavior. +6. Once the whole change set is complete, the main session runs the canonical quality gate in `.agnostic-ai/AGNOSTIC_AI.md` once. Use the `commit` skill with `Closes #` in the commit body, then the `pr` skill with the issue number. Push only within user authorization. -## Instructions - -### Phase 1: Setup - -1. **Parse the issue number** from the user-supplied argument (strip `#` if present) - -2. **Fetch issue details**: - ```bash - gh issue view --json title,body,labels,assignees,milestone,state - ``` - -3. **Assign yourself if unassigned**: - ```bash - gh issue edit --add-assignee @me - ``` - -4. **Add appropriate labels** if the issue doesn't have them: - - `bug` - Something isn't working - - `enhancement` - New feature or request - - `documentation` - Improvements or additions to documentation - -5. **Create a branch** from `main`: - - Determine prefix from labels: - - `bug` → `fix/` - - `enhancement` → `feat/` - - `documentation` → `docs/` - - Default → `feat/` - - Branch name: `-` (slug: lowercase title, spaces → `-`, max 50 chars) - - ```bash - git checkout main && git pull - git checkout -b - ``` - -### Phase 2: Plan - -6. **Analyze the issue**: requirements, labels, referenced issues, affected areas - -7. **Explore the codebase** for context: - - Find related code in `src/` - - Find related tests in `tests/` - - Study existing patterns - -8. **Create implementation plan**: - - Acceptance Criteria - - Test Strategy (which tests to write first) - - Files to Change - - Implementation Order (smallest first step) - -9. **Keep a short implementation plan** in the main session if implementation is non-trivial - -### Phase 3: Implement - -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 the full test suite once after the change set is complete**: - ```bash - ./bashunit tests/ - ./bashunit --parallel tests/ - ``` - -12. **Quality checks** before committing: - ```bash - make sa && make lint - ``` - -### Phase 4: Ship - -13. **Final verification**: - ```bash - ./bashunit tests/ && ./bashunit --parallel tests/ && make sa && make lint - ``` - -14. **Commit** using conventional commits with `Closes #` - -15. **Create PR** using the `pr` skill: - ``` - /pr # - ``` - -## Output Format - -After fetching, present: - -``` -## Issue #: - -**Labels:** <labels> -**State:** <state> -**Branch:** <branch-name> - -### Description -<body content> - -### Next Steps -1. Explore codebase for context -2. Create implementation plan -3. Begin TDD cycle -``` +Report the issue, acceptance scenario, branch, delivered behavior, validation, and any unresolved prerequisite. Do not paste the full issue or a generic plan back to the user. diff --git a/.agnostic-ai/skills/gh-issues/SKILL.md b/.agnostic-ai/skills/gh-issues/SKILL.md index 736179e2..2db7776d 100644 --- a/.agnostic-ai/skills/gh-issues/SKILL.md +++ b/.agnostic-ai/skills/gh-issues/SKILL.md @@ -7,161 +7,27 @@ disable-model-invocation: true allowed-tools: read, shell, Skill --- -# GitHub Issues Watcher +# Process an issue queue -## Purpose +Process open issues that are unassigned or assigned only to the authenticated user, oldest first, using `gh-issue` sequentially. Accept `--limit N`, `--label foo`, and `--dry-run` (discovery only). -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. +## Discover -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`. - -Strip leading `#` if user passes `#123` style. - -## Phase 1: Discover - -Fetch open issues that are unassigned **or** assigned to `@me`, oldest first. GitHub search does not OR these cleanly, so run two queries and merge: +Resolve the current login with `gh api user -q .login`. Fetch open issues with pagination: ```bash -# Unassigned -gh issue list \ - --state open \ - --search "no:assignee" \ - --json number,title,labels,assignees,createdAt \ - --limit 200 - -# Assigned to me -gh issue list \ - --state open \ - --assignee "@me" \ - --json number,title,labels,assignees,createdAt \ - --limit 200 +gh api --paginate 'repos/{owner}/{repo}/issues?state=open&per_page=100' ``` -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. -- Apply `--label` filter if given. -- Apply `--limit` if given. -- Sort ascending by `createdAt` (FIFO). - -Print the queue: `#<num> <title> [assignee]` per line, where `[assignee]` is `unassigned` or `@me`. If empty, exit cleanly. - -## Phase 2: Worktree Sanity - -Before touching any issue: - -```bash -git status --porcelain -git fetch origin main -git checkout main && git reset --hard origin/main -``` - -Abort if worktree dirty. Never auto-stash. - -## Phase 3: Process Loop - -For each issue in the queue: - -1. Re-check assignment state (someone else may have grabbed it): - ```bash - gh issue view <num> --json assignees -q '.assignees[].login' - me=$(gh api user -q .login) - ``` - - Empty output → unassigned, proceed. - - 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: - - self-assign via `gh issue edit <num> --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 #<num>` in the body - - PR opened via `pr #<num>` - -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/ - ``` - In `--simple` mode `print_line` emits only a one-char marker, so assert on - pure helper functions, not on `print_line` stdout. Under `--strict` - (`set -euo pipefail`): no `[ cond ] && assignment` (use `if`), initialise - 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: - ```bash - gh pr checks --watch - ``` - 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 <id> --failed`. - -5. Merge when allowed: - ```bash - 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 #<num>` there (never `Closes`), so the `Closes #<num>` in the branch commit body is lost. After merge: - ```bash - gh issue view <num> --json state -q .state # still OPEN? - gh issue close <num> --reason completed -c "Done in #<pr> (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 - `[ ]` as a glob (it matches a space, not literal brackets) and silently - does nothing: - ```bash - gh issue view <tracker> --json body -q .body \ - | sed 's/- \[ \] #<num>/- [x] #<num>/' | gh issue edit <tracker> --body-file - - ``` - -7. Sync `main` for next iteration: - ```bash - git checkout main && git fetch origin main && git reset --hard origin/main - ``` - -8. Continue with next issue. - -## Stop Conditions - -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). -- `make sa` (ShellCheck) or `make lint` (EditorConfig) fails. -- CI stays red after one fix attempt. -- Merge is blocked by branch protection beyond `--admin` bypass. -- `--limit` reached. -- Queue empty. - -Do **not** retry blindly. Report which issue failed and why. - -## Dry Run - -With `--dry-run`, only execute Phase 1 and print the queue. No assignment, no branching, no commits. - -## Preconditions +Exclude pull requests, issues assigned to anyone else, and tracker issues whose body lists child issues. Deduplicate by number, apply the label filter, sort by `created_at`, then apply the requested limit. Print `#<number> <title> [unassigned|@me]`; stop if empty. Dry-run stops here. -- `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. +## Process -## Notes +1. Check `git status --porcelain`. Preserve dirty work and stop or use an isolated worktree; never auto-stash or reset it. Fetch main and update a clean main with `git pull --ff-only`. A divergent branch needs a deliberate resolution. +2. Choose the validation mode in step 3 before invoking `gh-issue`. Re-read each issue's state and assignees before starting; skip closed or newly assigned issues. Invoke `gh-issue`, which owns implementation, the canonical quality gate, commit, and PR. The coordinator runs that gate once per complete issue; delegates run only focused checks. +3. If strict-mode coverage is required by CI, use `./bashunit --parallel --simple --strict tests/` in place of the ordinary parallel gate before committing. Inspect the current CI command first. Simple mode emits marker output; assert on helpers when testing full messages. Under strict mode, use `if` instead of failing `[ condition ] && assignment`, initialize arrays with `=()`, read potentially empty elements as `${arr[i]:-}`, and keep `extdebug` inside a capture subshell. +4. Wait for `gh pr checks --watch`. Investigate red checks, including Bash 3.0 compatibility. Re-run an identified infrastructure failure with `gh run rerun <id> --failed`; do not classify every registry failure as transient. If a local failure appears unrelated, compare the relevant test on unchanged main in an isolated checkout before claiming it predates the change. +5. Merge only when the user's authorization covers it and checks and branch protection allow it. Do not bypass protection. After an authorized merge, verify issue closure; close a completed issue if needed and update its parent's checklist. Escape Markdown brackets as literals when editing checkbox text and preserve the rest of the body. +6. Delete merged branches and worktrees according to the global cleanup rules, fetch and prune, and fast-forward main before the next issue. Keep branches with open PRs or unpushed commits. -- **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. -- Never split bundled changes into multiple PRs unless the issue explicitly demands it. +Stop and report the issue and cause on a hard failure, unresolved red check, dirty checkout after processing, blocked merge, or reached limit. Do not retry blindly. If merging is outside authorization, retain the open PR and report the remaining queue. No issue assignment, branch, or commit changes in dry-run mode. diff --git a/.agnostic-ai/skills/pr/SKILL.md b/.agnostic-ai/skills/pr/SKILL.md index a2e29dc7..2573f29f 100644 --- a/.agnostic-ai/skills/pr/SKILL.md +++ b/.agnostic-ai/skills/pr/SKILL.md @@ -6,103 +6,16 @@ argument-hint: "[#issue]" allowed-tools: shell, read, Grep, Glob --- -# Create Pull Request +# Create a pull request -Push branch and create a PR with a concise, issue-linked description. +Use this skill for PR creation. Repository paths are relative to the repository root. -> **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. +1. Inspect the current branch, `git status --short`, `git log main..HEAD --oneline`, and `git diff main..HEAD --stat`. Keep the user's edits and scope. +2. Verify `CHANGELOG.md` describes public behavior changes under `Unreleased`, with issue references where applicable. Contributor-only changes can update the existing contributor entry; do not add a self-referential changelog note for a changelog edit. +3. Complete the quality gate and commit workflow in `.agnostic-ai/AGNOSTIC_AI.md` and {{$SKILL:commit}}. Reuse passing checks for unchanged content. Do not rely on an optional Git hook being installed. Obtain the required diff review before pushing. +4. Push with `git push -u origin HEAD` when authorized. If it fails, diagnose and fix the cause; never bypass hooks or branch protection. +5. Read `.github/PULL_REQUEST_TEMPLATE.md` and follow its current headings and checklist. Use a conventional title under 70 characters. Link an actual issue with `Related #N`, never invent one. Explain the problem and resulting behavior in 1-2 background sentences and 2-4 change bullets, including concise validation. Avoid file inventories and conversational history. +6. Write the exact PR body to a temporary file; create with `gh pr create --title <title> --assignee @me --label <label> --body-file <file>`. Select one label: `bug`, `enhancement`, `documentation`, `refactor`, `ui`, or `investigation`, according to the change. Report the URL. +7. If an issue has a configured project in `.agnostic-ai/local/github-project.json`, use its actual IDs to move the issue to In Review. Do not assume project access or invent a missing configuration; report an unavailable update separately. -## Current Branch Context - -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` - -## 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 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.) - - Reference issue numbers where applicable (e.g., `(Issue #123)`) - - Commit the update: - ```bash - git add CHANGELOG.md && git commit -m "docs: update changelog" - ``` - -3. **Push branch**: - ```bash - git push -u origin HEAD - ``` - - The `pre-commit` hook runs tests, shellcheck, and linting automatically on commit. Ensure all checks passed before pushing. - - 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**: - - If the user-supplied argument contains an issue number, fetch the issue title: - ```bash - gh issue view <number> --json title -q '.title' - ``` - - PR title format: `<type>(<scope>): <short description>` (conventional commit style, under 70 chars) - - Derive the type from the branch prefix (`feat/` → feat, `fix/` → fix, `docs/` → docs) - -5. **Create PR** using the template from `.github/PULL_REQUEST_TEMPLATE.md`: - ```bash - gh pr create --title "<title>" --assignee @me --label "<label>" --body "$(cat <<'EOF' - ## 🤔 Background - - Related #<issue-number> - - <1-2 sentences: motivation and context for the changes> - - ## 💡 Changes - - - <bullet 1: what changed and why> - - <bullet 2> - - <bullet 3> (optional) - - <bullet 4> (optional) - EOF - )" - ``` - - **MANDATORY:** Always follow the PR template structure (`## 🤔 Background` + `## 💡 Changes`). Never use a different format. - - **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 - - **Body guidelines:** - - **Background**: Link the issue with `Related #<number>`, then 1-2 sentences of context. **NEVER use `Closes` or `Fixes`.** - - **Changes**: 2-4 short bullet points. Focus on *what* and *why*, not implementation details. - - **No file lists, no class names, no code snippets** in the body. - - Keep the entire body under 15 lines. - -6. **Move issue to "In Review"** in GitHub Project (if issue number provided): - ```bash - # 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') - - gh project item-edit \ - --id "$ITEM_ID" \ - --project-id "PROJECT_ID" \ - --field-id "STATUS_FIELD_ID" \ - --single-select-option-id "IN_REVIEW_OPTION_ID" - ``` - - **Note:** Requires `project` scope. Run `gh auth refresh -s project` if needed. - -7. **Report the PR URL** to the user. - -## Example Usage - -Use the `pr` skill alone, or pass an issue number such as `42` or `15`. +Respect existing publishing authorization. Creating a PR does not authorize merging it. diff --git a/.agnostic-ai/skills/pre-release/SKILL.md b/.agnostic-ai/skills/pre-release/SKILL.md index 0f7fec76..ab1f355a 100644 --- a/.agnostic-ai/skills/pre-release/SKILL.md +++ b/.agnostic-ai/skills/pre-release/SKILL.md @@ -4,87 +4,17 @@ description: Run comprehensive pre-release validation checklist allowed-tools: read, shell, Grep, Glob --- -# Pre-Release Validation +# Pre-release validation -Run all checks before releasing a new bashunit version. +Assess readiness without publishing. Repository paths are relative to the repository root. Use {{$SKILL:release}} for publishing; `release.sh` owns version changes, tags and uploads. -## Checklist +1. Start on clean, current `main`. Read `CHANGELOG.md` and `git log <latest-release>..HEAD`; check coverage of public changes and migration guidance for breaking changes. +2. Check current versions in `bashunit`, `package.json`, `docs/package.json`, and both lockfiles (top-level and `packages[""].version`). The latest dated changelog section must agree; `Unreleased` describes the next version. +3. Run the quality gate in `.agnostic-ai/AGNOSTIC_AI.md` once. Reuse passing results for unchanged content. Use targeted checks to investigate failures; repeat suites only for new changes or a specific flakiness concern. +4. Verify CI for the exact current main commit, not a previous run: tests, Bash 3.0 compatibility, build, docs, static analysis and formatting must pass. Check the full OS matrix for changes to paths, permissions or runtime behavior. Use `gh run list --branch main` and inspect each relevant run. +5. Inspect the changed paths for unsafe input handling, secrets and undocumented public behavior. Use `.agnostic-ai/rules/bash-style.md` and `tests/unit/project/bash_compatibility_test.sh` for compatibility; `[[ ]]` is a style choice, not a Bash 3.0 failure. +6. Check affected README examples and documentation links. If Docker is available and Linux behavior needs local verification, use `make docker/alpine` and `make docker/ubuntu`; otherwise report the platform coverage supplied by CI. +7. Smoke-test `./bashunit --help`, `./bashunit --version`, and a single assertion in a unique temporary test file. Clean up that file. +8. Preview the chosen version with `./release.sh <version> --dry-run`. For changes to the release process, also inspect `--sandbox` output and its release commit without publishing. -### 1. Version Consistency - -Verify version matches across: `package.json`, `CHANGELOG.md`, `src/bashunit.sh` - -### 2. Full Test Suite - -```bash -./bashunit tests/unit/ -./bashunit tests/functional/ -./bashunit tests/acceptance/ -./bashunit tests/ -./bashunit --parallel tests/ -``` - -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 -``` - -### 4. Documentation - -- `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 - -```bash -grep -rn '\[\[' src/ # Should not use [[ -grep -rn 'declare -A' src/ # No associative arrays -grep -rn '\${.*,,}' src/ # No case conversion -``` - -### 6. Cross-Platform (if Docker available) - -```bash -make docker/alpine -make docker/ubuntu -``` - -### 7. Security Scan - -No hardcoded secrets, no unsafe eval, input validation present. - -### 8. Breaking Changes - -Review `git log` since last release. Breaking changes must be documented with migration guide. - -### 9. Git & CI Status - -- Working directory clean -- All CI checks passing: `gh run list --limit 5` - -### 10. Smoke Test - -```bash -./bashunit --help -./bashunit --version -echo 'function test_smoke() { assert_equals "1" "1"; }' > /tmp/smoke_test.sh -./bashunit /tmp/smoke_test.sh -rm /tmp/smoke_test.sh -``` - -## Output - -Report each check as pass/fail/warning. Only proceed to release when ALL checks pass. - -## After Validation - -```bash -git tag -a vX.Y.Z -m "Release vX.Y.Z" -git push origin vX.Y.Z -``` +Report passed checks, failures and specific unverified items. Pending CI is pending, not passed. Stop publishing on failed checks. Do not create tags manually or run the release script in publishing mode from this skill. diff --git a/.agnostic-ai/skills/release/SKILL.md b/.agnostic-ai/skills/release/SKILL.md index f223ad58..9fc70e46 100644 --- a/.agnostic-ai/skills/release/SKILL.md +++ b/.agnostic-ai/skills/release/SKILL.md @@ -8,75 +8,14 @@ allowed-tools: shell, read, Grep, Glob # Release -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. +Use `./release.sh` from the repository root. It owns manifest and lockfile versions, build, checksum, changelog, commit, signed tags, GitHub release and `latest`. Fix the script when a step is missing; do not duplicate its flow. -## Current State +1. Run {{$SKILL:pre-release}} and reuse completed checks for unchanged content. Start on clean, current `main` and require CI success for that commit. +2. Use the user-selected version. Otherwise recommend a version from the changes and repository history; the script defaults to a minor bump. Performance and maintenance work can justify a minor release before 1.0. Confirm only if the version or publishing authorization is missing; an earlier selection and authorization remain valid. +3. Preview `./release.sh <version> --dry-run`. Finish validation and show the concrete preview before requesting any missing publishing approval. +4. Once authorized and green, run `./release.sh <version> --force`. +5. Verify `gh release view <version>` contains the binary and checksum; check the matching `npm-publish.yml` run, published npm version, installer checksum, and `origin/latest` release commit. Report the release URL and any unfinished verification. -Run the following commands before proceeding: +The dry run prints the previous dated changelog section because it does not rewrite files. The real run converts `Unreleased` first. Tags are annotated and respect `tag.gpgsign`; `v0` points to the release commit. npm publishes through `.github/workflows/npm-publish.yml` on the GitHub release event. -- 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 - -### 1. Pre-flight - -```bash -./bashunit tests/ # all green -make sa && make lint # static analysis + editorconfig -gh run list --limit 3 --branch main -``` - -Stop and report if anything fails. Don't release on a red main. - -### 2. Pick the version - -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. - -### 3. Preview, then publish - -```bash -./release.sh <version> --dry-run # preview; changes nothing -./release.sh <version> --force # publish (non-interactive) -``` - -Notes: -- `--dry-run` release notes look "off" (they show the previous version's - section) because the CHANGELOG isn't actually rewritten in a dry run. The - real run converts `## Unreleased` → `## [<version>]` first, so the published - notes are correct. Not a bug. -- Tagging is gpgsign-safe: `release::create_tags` makes annotated, `-m` - tags (signed when `tag.gpgsign=true`) and pins `v0` to the release commit - (`^{}`). No manual tagging needed. -- npm publishes automatically via `.github/workflows/npm-publish.yml` on the - GitHub `release: published` event. - -### 4. Verify the published artifacts - -```bash -gh release view <version> # assets: bin/bashunit + bin/checksum -gh run list --workflow npm-publish.yml --limit 1 -git log --oneline -1 origin/latest # latest branch advanced (docs deploy) -``` - -Confirm the npm version and the install.sh checksum match, then report the -release URL. - -## Recovery - -`./release.sh --rollback` restores files from the most recent backup if a run -fails mid-way. - -## Example Usage - -``` -/release -/release 0.40.0 -``` +If execution fails, inspect the completed steps before retrying. `./release.sh --rollback` restores files from its saved backup; it does not undo already-pushed tags, commits or published artifacts. diff --git a/.agnostic-ai/skills/review/SKILL.md b/.agnostic-ai/skills/review/SKILL.md index f262969e..ffa8c830 100644 --- a/.agnostic-ai/skills/review/SKILL.md +++ b/.agnostic-ai/skills/review/SKILL.md @@ -6,39 +6,15 @@ argument-hint: "[#N]" allowed-tools: shell, read, Grep, Glob --- -# Review a Pull Request +# Review a pull request -Review a GitHub PR. When no number is given, review the PR for the current branch. +Resolve the supplied PR number or the current branch's PR with `gh pr view`. If neither identifies one, list open PRs and request a selection. For an uncommitted diff, use `code-reviewer` instead. -> For reviewing your uncommitted working diff instead, assign that diff to the `code-reviewer` agent. +Read `gh pr view <number> --json number,title,body,state,headRefName,baseRefName,url` and `gh pr diff <number>`. Follow the applicable source rules in `.agnostic-ai/AGNOSTIC_AI.md`. -## Current Branch Context +- Verify changed behavior, failure paths, Bash compatibility, dynamic scope, subshell state, parallel isolation and cleanup. Prefer `[ ]` for ordinary tests; `[[ ]]` works on Bash 3.0 and is not a compatibility defect. +- Check tests exercise the failure and the requested outcome. Respect process budgets and public behavior; require changelog and documentation where needed. +- For specs, check canonical sources, generated references, model resolution and preserved target behavior. +- Review read-only. Do not run the full gate, edit files, or post comments without authorization. -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 -- User-supplied argument: PR number to review (optional, e.g., `#42` or `42`). - -## Instructions - -1. **Resolve which PR to review:** - - 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. - -2. **Load the PR:** - ```bash - gh pr view <number> --json number,title,body,state,headRefName,baseRefName,url - gh pr diff <number> - ``` - -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. +Report only verified defects, most severe first: `path:line, severity, failing input, smallest fix`. Do not add style nits or speculate. If none remain, say so and name the scope checked. diff --git a/.agnostic-ai/skills/tdd-cycle/SKILL.md b/.agnostic-ai/skills/tdd-cycle/SKILL.md index b447bc9c..9f7975aa 100644 --- a/.agnostic-ai/skills/tdd-cycle/SKILL.md +++ b/.agnostic-ai/skills/tdd-cycle/SKILL.md @@ -4,45 +4,15 @@ description: Run complete TDD red-green-refactor cycle for a test allowed-tools: read, edit, write, shell, Grep, Glob --- -# TDD Cycle +# TDD 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. +Read [the complete workflow](references/workflow.md) for task-file conventions. Repository paths are relative to the repository root. The canonical quality gate and coordinator responsibilities in `.agnostic-ai/AGNOSTIC_AI.md` apply. -## Workflow +Use `.tasks/YYYY-MM-DD-slug.md` for features, refactors, or multi-test work; skip it for small fixes or a single test. Choose the next smallest test from the acceptance scenario or task inventory and study matching existing tests. -### 1. Verify Task File +1. **RED:** Write Arrange-Act-Assert, run `./bashunit <test-path>`, and verify failure proves the requested behavior is missing, rather than a syntax or setup error. Record the red bar and logbook entry when using a task file. +2. **GREEN:** Implement only what this test requires. Run the focused test and require a pass. Check off the inventory entry and record the result. +3. **REFACTOR:** Improve naming, structure, or duplication without changing behavior. Keep affected focused tests green; preserve other workers' changes. Record meaningful refactoring notes. +4. Repeat until the acceptance scenario is complete. The main session runs the full quality gate once after the whole change set, before committing. Delegates run only assigned targeted checks. -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. 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 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 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. 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 - -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**, when present, after each phase +Never skip the demonstrated RED or add speculative behavior during GREEN. Keep the task inventory and logbook current when a task file is used. diff --git a/.agnostic-ai/skills/tdd-cycle/references/workflow.md b/.agnostic-ai/skills/tdd-cycle/references/workflow.md index 059b6206..aa90bf87 100644 --- a/.agnostic-ai/skills/tdd-cycle/references/workflow.md +++ b/.agnostic-ai/skills/tdd-cycle/references/workflow.md @@ -1,73 +1,44 @@ -# TDD Workflow +# TDD task file -**Test-Driven Development is mandatory** for all bashunit development. +Use the `tdd-cycle` skill for runtime RED/GREEN/REFACTOR steps. This reference supplies the planning record; repository paths are relative to the root. Do not create runtime tests for prose-only edits. -## 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. +For features, refactors or multi-test work, create `.tasks/YYYY-MM-DD-slug.md`; skip it for a small fix or a single test. Keep the record current as work progresses. ```markdown -# [Feature/Fix Name] +# Feature or fix -**Date:** YYYY-MM-DD **Status:** In Progress +Date: YYYY-MM-DD +Status: In Progress -## Acceptance Criteria -- [ ] Criterion 1 +## Acceptance scenario -## Test Inventory -- [ ] `test_should_handle_valid_input` -- [ ] `test_should_fail_with_invalid_input` +Given the concrete trigger, the user observes the expected result. -## Current Red Bar -Test: (none yet) +## Non-goals -## Logbook -### YYYY-MM-DD HH:MM -- Created task, analyzed existing code -``` +Work outside this outcome. -## RED Phase +## Acceptance criteria -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) +- [ ] Criterion -## GREEN Phase +## Test inventory -1. Write **minimal** code to pass, no extra features, no premature optimization -2. Run: `./bashunit path/to/test.sh`, **must pass** +- [ ] `test_should_handle_valid_input` +- [ ] `test_should_fail_with_invalid_input` -## REFACTOR Phase +## Current red bar -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`) +Test: none yet +Failure: none yet -## Quality Gate (Before Commit) +## Logbook + +### YYYY-MM-DD HH:MM -```bash -./bashunit tests/ # All tests -./bashunit --parallel tests/ # Parallel (isolation check) -make sa && make lint # Static analysis + linting (lint is the format gate) +- Scope, evidence, decision, or test result. ``` -## Definition of Done +Choose the smallest next test, prove its failure is behavioral, implement minimal GREEN, and refactor with affected tests passing. Use existing patterns under `tests/unit/assert/` or the matching module tests; no speculative behavior. Delegates run only assigned targeted checks. -- 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) +Done means the acceptance scenario works, its criteria are complete, runtime parsing works on Bash 3.0, and public changes have docs and a CHANGELOG entry. Record architectural decisions in an ADR. The main session runs the quality gate in `.agnostic-ai/AGNOSTIC_AI.md` once after the complete change set and before committing. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d705550..13a090c7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ - Parallel reports avoid per-row base64 processes while preserving values and completion order (#1371) - Data providers avoid per-argument base64 processes while preserving quoting, empty values and parser isolation (#1370) - JSON reports avoid per-field processes for ordinary filenames, test names and empty messages while preserving escaping (#1369) -- Contributor instructions share task-specific context and model tiers for Claude Code and Codex through a pinned agnostic-ai npm development dependency (#1387) +- Contributor instructions share task-specific context and model tiers for Claude Code and Codex through a pinned agnostic-ai npm development dependency; shorter workflows align review and release checks with the current code (#1387) ### Fixed