diff --git a/CLAUDE.md b/CLAUDE.md index 22cc20d..d4a74a4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -29,9 +29,18 @@ Each module owns one thing, and `main.rs` owns none of them. | `src/out.rs` | `ls`, `issue`, `export`, `index` | | `src/view.rs` | The HTML page, and the only markdown rendering in the crate. `view/style.css`, `view/app.js` and the three `view/theme*`/`view/*.html` files are `include_str!`-ed, never built by `format!` | | `src/main.rs` | clap wiring, routing, exit codes. No domain logic | +| `src/lock.rs` | The write lock, which is the kernel's: `flock` on unix, `LockFileEx` on Windows. Specified with `workspace` | +| `src/promise.rs` | `#[cfg(test)]`. Random capture / retire / hand-edit sequences over files hi did not write, asserted on shape and section | Every module has a spec under `specs//`, and requirements there cite the hi criterion they -serve (`hi: CAPTURE-3`). If you change behavior, update the spec. `specsync check` is in the gate. +serve (`hi: CAPTURE-3`). If you change behavior, update the spec. `specsync check` is in the gate, +and it covers every line of `src/`. + +The two test targets are specified too, because both make claims nothing else can. +`specs/promise/` covers `src/promise.rs` and `tests/promise.rs` — the generated half, which looks +for the id defects nobody has found yet. `specs/cli/` covers `tests/cli.rs` — the black-box half, +which pins the ones that shipped, one fixture per defect, and is the only place argv routing, exit +codes and the stdout/stderr split are tested at all. ## Rules that are easy to break by accident diff --git a/specs/cli/cli.spec.md b/specs/cli/cli.spec.md new file mode 100644 index 0000000..5948bf3 --- /dev/null +++ b/specs/cli/cli.spec.md @@ -0,0 +1,277 @@ +--- +module: cli +version: 1 +status: active +files: + - tests/cli.rs + +db_tables: [] +depends_on: + - specs/capture/capture.spec.md + - specs/check/check.spec.md + - specs/doc/doc.spec.md + - specs/id/id.spec.md + - specs/main/main.spec.md + - specs/out/out.spec.md + - specs/view/view.spec.md + - specs/workspace/workspace.spec.md +--- + +# CLI + +## Purpose + +The black-box acceptance suite: sixty-seven tests that spawn the real `hi` binary against a real +temporary repository and assert on what a person would see. It is the only place in hi where the +thing under test is a *process*, and that is the whole reason it exists — argv routing, exit codes, +which stream a line lands on, and what is actually on disk afterwards are properties no unit test +can hold, because every one of them is decided outside the module that produced the value. + +Three kinds of claim live here and nowhere else. + +**What only a process has.** `looks_like_id` routing an id-shaped first argument to capture before +clap parses anything; `peel_root` taking `--root` only while it leads; `args_os` turning a non-UTF-8 +argument into a message instead of a panic; exit 0, 1 and clap's 2; stdout carrying the record of +what landed while stderr carries everything else. A unit test can call `peel_root`. Only a process +can prove that `hi SEND-2 the --root docs option should be documented` writes the word `--root` +into the file and `hi --root /tmp/repo SEND-2 "..."` does not. + +**What a file on disk has.** Several of hi's rules are about bytes rather than values: a refusal +leaves the destination byte-for-byte unchanged; a capture into a CRLF or BOM'd or block-frontmatter +file returns it in the person's own style; a criterion renders as a markdown list item rather than a +line in a run-together paragraph; a file hi could not decode is never a file hi replaces. The +assertions are `assert_eq!` over file contents and, in the strictest case, over a `listing` of every +path under the repository root with its bytes — so a refusal can be held to changing *nothing* +rather than to changing no criteria. + +**The defects that shipped.** `an_id_is_never_handed_out_twice` is five reproduced bugs as five +fixtures in one function, each one a way a write path broke the single promise hi makes +(hi: FILE-13, FILE-20, FILE-22, RETIRE-5, RETIRE-6), and three more have tests of their own for the +concurrent cases (hi: FILE-19, RETIRE-7). These are hand-written counterparts to `specs/promise/`, +which generates its inputs: that module looks for the bugs nobody found, this one pins the bugs that +were. Neither replaces the other, and the comment above each fixture says which defect it is. + +Almost every fixture here is a file **hi did not write**. That is a standing rule rather than a +stylistic preference: three id defects shipped because every write-path test asserted over files hi +itself had produced, so the writer was being tested against its own output and agreed with itself +(DECISIONS.md §26). `Repo::with_chat` seeds a bare `SEND-1 I hit enter and it shows up.` line with +no bullet and no emphasis, which is not what capture emits and is what the parser must keep reading +(hi: FILE-14). + +One nested module, `mod nudge`, is here rather than in its own file because it is the same kind of +claim about a different executable: `bin/fledge-hi-nudge`, the fledge lifecycle hook. Its one +load-bearing property is that no path through it can exit non-zero, because fledge propagates a +hook's exit code and a failing hook would abort the command that ran it. + +## Public API + +| Export | Description | +|--------|-------------| +| None. | An integration test target. Cargo builds it as its own binary against `CARGO_BIN_EXE_hi`; nothing in the crate links it and nothing is compiled into a release build. | + +### Structs & Enums + +| Type | Description | +|------|-------------| +| `Repo` (private) | A throwaway repository under `std::env::temp_dir()`, named `hi-cli--`. Three constructors, and which one a test picks is load-bearing — see Invariant 3. | + +### Traits + +| Trait | Description | +|-------|-------------| +| None. | This module defines no traits. | + +### Functions + +| Function | Signature | Description | +|----------|-----------|-------------| +| `Repo::new` | `fn new(name: &str) -> Repo` | A root with an empty `hi/` directory. The default fixture for a test that does not care how the workspace came to exist. | +| `Repo::bare` | `fn bare(name: &str) -> Repo` | A root with a `.git` and **no** `hi/`: the state every first capture in an adopting repository starts in, and the only state in which the bootstrap lock defect is reachable (DECISIONS.md §33). | +| `Repo::with_chat` | `fn with_chat(name: &str) -> Repo` | `Repo::new` plus a `hi/chat.md` declaring `families: [SEND]` with an `## Intent` and a **bare** `SEND-1 I hit enter and it shows up.` on line 10. The bare form is deliberate (hi: FILE-14) and line 10 is the number the duplicate-id message quotes. | +| `Repo::write` / `Repo::read` | `fn write(&self, rel: &str, body: &str)` / `fn read(&self, rel: &str) -> String` | Repository-relative file access, used both to seed a fixture and to assert on the result. | +| `Repo::run` | `fn run(&self, args: &[&str]) -> Output` | Spawn `CARGO_BIN_EXE_hi` with `current_dir` set to the root. Tests that need to prove discovery or `--root` behavior build their own `Command` with a different working directory instead. | +| `stdout` / `stderr` | `fn stdout(output: &Output) -> String` | `String::from_utf8_lossy` over the captured stream. Lossy on purpose: a test asserting that a non-UTF-8 argument did not panic must still be able to read what was printed. | +| `listing` | `fn listing(root: &Path) -> Vec` | Every path under the root paired with its bytes, sorted. The strongest assertion in the suite: it is what lets `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` require that a refusal changed nothing at all, `INTENT.md` and `hi/AGENTS.md` included. | +| `future_format` | `fn future_format(name: &str) -> Repo` | A repository whose one criteria file declares `hi: 2`, a format version this binary has never heard of (hi: FILE-25). | +| `nudge::nudge` / `run` / `run_from` / `repo` / `decoy` | (private, `#[cfg(unix)]`) | Helpers for driving `bin/fledge-hi-nudge` as fledge drives it: the moment as argv, the repository as `FLEDGE_REPO_ROOT`, and a working directory that is deliberately *not* the repository. | + +## Invariants + +1. **Every fixture path carries the process id.** `hi-cli--`, `hi-nudge--`, + `hi-nudge-decoy-`. A fixed scratch path shared between two `cargo test` runs is the defect + that made this suite flake and made bulk capture lose writes; it is the same mistake + `doc::write_atomically` had. The one exception is `outside_a_repository_it_says_so_rather_than_guessing`, + which uses a fixed `hi-cli-norepo` path — see Error Cases. +2. **The tests drive the binary, never the library.** `CARGO_BIN_EXE_hi` is resolved by cargo at + compile time, so the suite always runs the binary built from the current tree. A stale + `target/debug/hi` has twice made a passing change look like a code bug; `cargo clean -p human-intent` + is the answer when a result here disagrees with the release binary. +3. **The fixture constructor is part of the test's meaning.** `Repo::new` pre-creates `hi/`, so a + test built on it cannot exercise `lock::acquire` creating the directory, and cannot see the + bootstrap race at all. A write-path test about a *first* capture must use `Repo::bare`. +4. **A refusal is asserted to change nothing, and the strength of "nothing" is chosen per test.** + Most refusal tests snapshot the destination file and `assert_eq!` afterwards. + `a_refusal_over_the_format_version_writes_nothing_at_all` additionally asserts that `INTENT.md`, + `hi/AGENTS.md`, `hi/CLAUDE.md` and `hi/.hi.lock` do not exist and that `hi/` holds exactly one + entry. `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` compares a `listing` of the whole + tree. Every one of them exists because a refusal that wrote something did ship (hi: CAPTURE-5). +5. **stdout and stderr are asserted separately and the split is the contract.** A refusal must print + nothing to stdout; a note about a feature list that could not be refreshed must be on stderr while + the command still exits 0; the nudge hook must never write to stdout at all, because + `fledge work start --json` puts its envelope there (hi: CAPTURE-11, INDEX-4.a). +6. **Exit codes are asserted as numbers, not as success/failure.** `Some(0)`, `Some(1)` and `Some(2)` + are distinct claims: 1 is hi refusing, 2 is clap reporting a usage error, and a test that accepts + either cannot tell an id error from an unknown subcommand. +7. **Assertions quote the whole output on failure.** Nearly every `assert!` in the file carries + `"{}", stderr(&out)` or the file body. A black-box failure with no output attached is a failure + nobody can diagnose from CI. +8. **Fixtures are files hi did not write.** Bare criterion lines, CRLF, a BOM, block-style + frontmatter, an indented `## Retired`, a fenced example of the format, an `INTENT.md` whose + generated count is deliberately wrong, and one file that is not valid UTF-8. Where a test asserts + a *count* changed, the seeded count is wrong in a way the correct answer is not, so the assertion + cannot pass by the block never being touched (DECISIONS.md §26). +9. **`an_id_is_never_handed_out_twice` is a register of shipped defects and only grows.** Five blocks + today, each with the bug it reproduces named in a comment above it. A block is never deleted when + the bug is fixed; that is the only thing keeping it fixed. +10. **The concurrency tests assert on what survived, and a quiet refusal is a failure.** Eight + captures of distinct ids must all land; thirty-two of them must land in a repository that had no + `hi/`; two concurrent retires must both stay retired. Thirty-two rather than eight because eight + did not reproduce the bootstrap race reliably and thirty-two is the shape adoption actually has. +11. **No path through the fledge hook may exit non-zero.** `no_path_through_the_hook_can_abort_a_push` + crosses four moments (`start`, `push`, empty, `unknown-moment`) with four roots (present, with a + `hi/`, non-existent, unset) and requires exit 0 from all sixteen. fledge propagates the code, so + a non-zero exit here aborts `fledge work push` for a reason that has nothing to do with the push. +12. **The hook's working directory is a decoy.** `decoy()` is a repository with a `.git` and no + `hi/`, because hi's own repository *has* a `hi/`: a hook that ignored `FLEDGE_REPO_ROOT` and + guessed from its cwd would fall silent in the crate root and look correct. Mutation testing found + exactly that, passing for the wrong reason. +13. **The suite runs everywhere `cargo test` does.** Two tests are `#[cfg(unix)]` — + `a_non_utf8_argument_is_reported_not_panicked`, which needs `OsStrExt`, and `mod nudge`, which + runs a shell script. Everything else, the concurrency tests included, runs on Windows too. + +## Behavioral Examples + +#### Scenario: Capture is reached before clap + +- **Given** `Repo::with_chat` +- **When** `hi SEND-2 "it reaches them and the mark changes to sent"` runs +- **Then** exit 0, stdout contains `+SEND-2`, and `hi/chat.md` gains + `**SEND-2** it reaches them and the mark changes to sent` + (`an_id_shaped_argument_captures`) + +#### Scenario: A sentence is one line however long it is + +- **Given** a 150-character sentence +- **When** it is captured +- **Then** the line in the file equals `- **SEND-2** ` exactly, unwrapped and + untruncated (`a_long_sentence_stays_on_one_line`, hi: FILE-6) + +#### Scenario: A file renders as a list, not a wall of text + +- **Given** a bare repository, and three captures: `SEND-1`, `SEND-1.a`, `SEND-2` +- **Then** the three criterion lines are exactly `- **SEND-1** I hit enter and it shows up`, + ` - **SEND-1.a** If I have no connection it queues` and `- **SEND-2** It reaches them` — one + line each, the case indented two spaces — and `hi export` parses the result back to the same three + (`a_hi_file_renders_as_a_list_not_a_wall_of_text`, hi: FILE-1.b, FILE-1.c) + +#### Scenario: `--root` before the id is a flag; after it, a word + +- **When** `hi --root SEND-2 "it reaches them"` runs from elsewhere, and separately + `hi SEND-2 the --root docs option should be documented` runs inside the repo +- **Then** the first writes `**SEND-2** it reaches them` and the string `--root` appears nowhere in + the file; the second writes `**SEND-2** the --root docs option should be documented` + (`root_is_honored_by_capture_and_not_swallowed_into_the_sentence`, + `a_flag_looking_word_inside_a_sentence_stays_a_word`, hi: CAPTURE-8, CAPTURE-9) + +#### Scenario: Five ways an id was handed out twice + +- **Given** one test, five fixtures +- **Then** (1) retiring into a file whose `## Retired` is not the last section lands the criterion + *inside* that section rather than at EOF, and the id stays spent; (2) a retirement reason + containing `\n- **SEND-5** forged\n` never produces a line that reads as a criterion, and `SEND-5` + is still free afterwards; (3) a criterion inside a fence under `## Criteria` is unreadable and + still taken; (4) a properly closed `markdown` example containing `## Retired` and `- **SEND-4**` + is prose — the retirement goes to the *real* section, `hi ls --retired` finds it, the example is + untouched, and `SEND-4` is still free because drawing an id does not burn it; (5) a fence the + person never closed makes capture refuse twice rather than report the same id saved twice, and the + half-written file is byte-identical afterwards + (`an_id_is_never_handed_out_twice`, hi: FILE-13, FILE-20, FILE-22, FILE-22.a, FILE-22.b, + RETIRE-5, RETIRE-6) + +#### Scenario: Thirty-two first captures at once + +- **Given** `Repo::bare` — a `.git` and no `hi/` +- **When** thirty-two threads each capture a distinct `SEND-` +- **Then** every process that reported success is readable in `hi/send.md`, and **no** process + refused; each refusal's stderr would be printed rather than counted + (`concurrent_captures_into_a_repository_with_no_hi_directory_all_land`, hi: FILE-19, CAPTURE-14) + +#### Scenario: A file hi cannot decode is never a file hi replaces + +- **Given** an `INTENT.md` holding three years of prose and one Latin-1 byte +- **When** a capture runs +- **Then** the capture succeeds, the criterion is on disk, `INTENT.md` is byte-identical, and stderr + carries `not refreshed` rather than going quiet + (`a_root_file_hi_cannot_read_survives_a_capture_byte_for_byte`, hi: INDEX-2.c, INDEX-4.a). The same + file reached by `hi index`, which *is* allowed to install a section, makes the command refuse with + exit 1 and every byte intact (`hi_index_refuses_a_root_file_it_cannot_read_rather_than_replacing_it`) + +#### Scenario: A later format is refused by every verb + +- **Given** a `hi/chat.md` declaring `hi: 2` +- **When** each of eleven invocations runs — `check`, `check --json`, `ls`, `ls --retired`, + `export`, `issue SEND-1`, `index`, `view`, `retire`, a capture, and `seed` +- **Then** every one exits 1, names both `hi/chat.md` and `hi: 2` on stderr, and prints nothing to + stdout; and the refusal creates no `INTENT.md`, no `hi/AGENTS.md`, no `hi/CLAUDE.md` and no + `hi/.hi.lock` (`a_file_from_a_later_format_is_refused_by_every_verb`, + `a_refusal_over_the_format_version_writes_nothing_at_all`, hi: FILE-25, FILE-25.a) + +#### Scenario: The fledge hook cannot break a push + +- **When** `bin/fledge-hi-nudge` runs for every combination of four moments and four roots +- **Then** all sixteen exit 0; `start` and `push` say different things on stderr in a repository + with nothing written down; both go quiet once a `hi/` exists; neither ever writes to stdout; and + none of them speaks when no `FLEDGE_REPO_ROOT` was given (`mod nudge`) + +## Error Cases + +These are the failure modes of the suite itself — the ways a test here can be wrong or can fail for +a reason that is not the defect it names. + +| Condition | Behavior | +|-----------|----------| +| `target/debug/hi` is stale | Not detected. `CARGO_BIN_EXE_hi` makes cargo rebuild, but a result compared by hand against a stale release binary has twice read as a code bug. `cargo clean -p human-intent` when a result disagrees with what the binary does | +| Two `cargo test` runs at once | Safe. Every fixture path carries `std::process::id()` — except `outside_a_repository_it_says_so_rather_than_guessing`, which uses a fixed `/hi-cli-norepo` and would collide with a concurrent run of itself | +| A test is built on `Repo::new` when it means a first capture | Passes for the wrong reason. `hi/` already exists, so `lock::acquire` never creates it and the bootstrap path is never taken. Use `Repo::bare` | +| A fixture is "tidied" into the list form capture emits | Passes, and covers less. The bare `SEND-1 ...` lines are what the parser must keep accepting (hi: FILE-14), and a fixture hi wrote tests the writer against itself (DECISIONS.md §26) | +| An assertion about a generated count is seeded with the correct count | Passes even if the block is never touched. The `INTENT.md` fixtures deliberately carry `(7 criteria)` or `(9 criteria)` against a true answer of 2 | +| `assert!(out.status.success())` used where the exit code matters | Cannot distinguish hi's 1 from clap's 2. The suite asserts `out.status.code() == Some(n)` wherever the number is the claim | +| A hook test run from the crate root | `hi`'s own repository has a `hi/`, so a hook that ignored `FLEDGE_REPO_ROOT` would go quiet and look correct. `decoy()` is a repository with no `hi/` for exactly this reason | +| The nudge helpers on Windows | Not run. `mod nudge` is `#[cfg(unix)]`; `scripts/nudge-behaves.sh` covers the same ground in the local gate only | +| A concurrency test that asserts a count of winners rather than which ones | Loses the diagnosis. `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` joins every refusal's stderr into the message, because the one time it fired it was a lock handoff on Windows | +| A criterion fixture whose line number changes | `Repo::with_chat` puts `SEND-1` on line 10 and the duplicate-id message quotes it. Adding a line to that fixture changes a message another test asserts on | +| `hi check` exiting 0 over a broken file | Precisely what several of these tests exist to catch, and why so many assert on `check`'s output *before and after* the operation under test | + +## Dependencies + +### Consumes + +| Crate/Module | What is used | +|-------------|-------------| +| `std` | `fs`, `path::PathBuf`, `process::{Command, Output}`, `thread`, `env::temp_dir`, `process::id`, and `ffi::OsStr` / `os::unix::ffi::OsStrExt` on unix | +| `serde_json` | `Value` and `from_str`, for the `export` and `check --json` payload assertions | +| `CARGO_BIN_EXE_hi` | The binary under test, resolved by cargo at compile time | +| `CARGO_MANIFEST_DIR` | `mod nudge` only: locating `bin/fledge-hi-nudge` | +| `src/seed/agents_0_5.md` | `include_str!`ed by `seed_rewrites_a_template_hi_has_shipped_and_refuses_an_edit`, so the "a template hi has shipped" fixture is the shipped bytes rather than a copy | + +### Consumed By + +| Module | What is used | +|--------|-------------| +| None. | An integration test target. Nothing links it. | + +## Change Log + +| Date | Author | Change | +|------|--------|--------| +| 2026-09-19 | Claude | Initial specification. Written after `fledge atlas` reported `tests/cli.rs` as the largest file in the repository under no spec (1,819 lines). Documents the three kinds of claim only a process can make, the three fixture constructors and why the choice between them is load-bearing, the register of five reproduced id defects, the strength ladder for "a refusal writes nothing", and the nested fledge-hook module. | diff --git a/specs/cli/context.md b/specs/cli/context.md new file mode 100644 index 0000000..0c4c096 --- /dev/null +++ b/specs/cli/context.md @@ -0,0 +1,136 @@ +--- +spec: cli.spec.md +--- + +## Key Decisions + +- **The suite tests a process, and that is the whole division of labour.** Every module has unit + tests that call its functions. What cannot be reached that way is argv before clap sees it, an + exit code, which stream a line landed on, and the bytes of a file after the process ended. If an + assertion here could be written by calling a function, it belongs in that module instead — and + several here are deliberate duplicates anyway, because an error string shown to a person is a + contract and is worth pinning from both sides. +- **Almost every fixture is a file hi did not write.** `Repo::with_chat` seeds a bare + `SEND-1 I hit enter and it shows up.` with no bullet and no bold id, which is *not* what capture + emits. Three id defects shipped because every write-path test asserted over files hi had produced + itself, so the writer was tested against its own output and agreed with itself (DECISIONS.md §26). + A fixture "tidied" into the list form is a silent weakening of the suite, and it would still pass. +- **Where a count is asserted, the seeded count is wrong on purpose.** `HAND_WRITTEN_INTENT` says + `(1 criterion)` where the answer is 2, and `a_retire_refreshes_the_feature_list` rewrites it to + `(9 criteria)` — a number that is neither the count before nor the count after. Seeding the correct + value would let the assertion pass by the block never being touched at all. +- **The three `Repo` constructors are three different claims.** `Repo::new` gives an empty `hi/`; + `Repo::with_chat` adds a seeded criteria file; `Repo::bare` gives a `.git` and **no** `hi/`. The + third is the one that matters: the first capture in an adopting repository has no directory to + put a lock file in, and `lock::acquire` used to hand back a `Guard` it had not taken when the open + failed, so every bootstrap capture ran unlocked. A test built on `Repo::new` cannot see any of + that, because `hi/` already exists (DECISIONS.md §33). +- **Thirty-two concurrent captures, not eight.** `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` + uses thirty-two because eight did not reproduce the bootstrap race reliably, and because + thirty-two is the shape adoption actually has — fledge's own adoption was 197 captures. The + ordinary concurrency test above it uses eight, which is enough for a lock that either exists or + does not. +- **A quiet refusal in a concurrency test is a failure, and its stderr is printed.** An id nobody + else asked for is free, so a capture of `SEND-7` refusing while `SEND-8` succeeds is a defect. The + assertion joins every refusal's stderr into the message rather than counting them, because the one + time it fired it was a lock handoff on Windows and a count says nothing about that. +- **`an_id_is_never_handed_out_twice` is a register, not a test.** Five fixtures in one function, + each preceded by a comment naming the shipped bug it reproduces. Keeping them together is + deliberate: read end to end it is the list of every way hi's own verbs have broken the one thing + hi promises. A block is never deleted when its bug is fixed — that is the only thing keeping it + fixed. It grows; it does not shrink. +- **There is a strength ladder for "a refusal writes nothing", and the rung is chosen per test.** + Most refusal tests snapshot the destination file. `a_refusal_over_the_format_version_writes_nothing_at_all` + also asserts the *absence* of `INTENT.md`, `hi/AGENTS.md`, `hi/CLAUDE.md` and `hi/.hi.lock`, which + is how it proves the refusal happened before `lock::acquire` ran. + `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` compares a `listing` of every path under + the root with its bytes. Each rung exists because a refusal that wrote something did ship + (hi: CAPTURE-5). +- **Exit codes are asserted as numbers.** 1 is hi refusing and 2 is clap reporting a usage error, + and a test that accepts "not success" cannot tell an id error from an unknown subcommand. That + distinction is the whole content of `an_unknown_subcommand_is_a_usage_error` and of the + `SEND-a` row in `specs/main/`. +- **`stdout` and `stderr` use `from_utf8_lossy`.** A test asserting that a non-UTF-8 argument did not + panic still has to be able to read what was printed. +- **`mod nudge` is here rather than in its own file** because it is the same kind of claim about a + different executable. Its load-bearing property is exit 0 from every path: fledge propagates a + hook's exit code, so a non-zero exit aborts the command that ran it, and hi has no business + blocking somebody's `fledge work push`. `scripts/nudge-behaves.sh` covers the same ground but only + the local gate runs it; these run wherever `cargo test` does. +- **The hook's working directory is a decoy on purpose.** `decoy()` is a repository with a `.git` + and no `hi/`. Running the hook from the crate root would hide the bug it guards: hi's own + repository *has* a `hi/`, so a hook that ignored `FLEDGE_REPO_ROOT` and guessed from its cwd would + fall silent there and look correct. Mutation testing found exactly that, passing for the wrong + reason. +- **`Repo::with_chat` puts `SEND-1` on line 10, and that number is load-bearing.** The duplicate-id + refusal quotes the line number, and `specs/capture/` documents it. Adding a line to that fixture + changes a message other tests read. +- **The suite keeps running on Windows.** Only two things are `#[cfg(unix)]`: + `a_non_utf8_argument_is_reported_not_panicked`, which needs `OsStrExt` to build an invalid + argument, and `mod nudge`, which runs a shell script. The concurrency tests in particular run + everywhere, which is how the one Windows lock-handoff refusal was ever observed. + +## Files to Read First + +- `tests/cli.rs` itself, starting with `Repo` at the top and `listing` and `future_format` near the + bottom. The comment above each test says which defect or which criterion it is about; those + comments are the index. +- `src/main.rs` for what is being driven: `peel_root`, the `args_os` collection, `looks_like_id` + routing, and the exit-code mapping. `specs/main/` describes all of it, and the Error Cases table + there and the assertions here should always agree. +- `DECISIONS.md` §26 for why the fixtures are files hi did not write, §33 for the bootstrap lock and + the concurrent-retire defect, §35 for the resurrection fixture + (`a_capture_never_brings_a_retired_criterion_back_to_life`), §32 for the reservation lookup and + the `INTENT.md` over-write, and §36 for the unreadable-is-not-absent pair. +- `specs/promise/` for the generated counterpart. This suite pins named cases; that module looks for + unnamed ones. Read them together before adding a test to either. +- `bin/fledge-hi-nudge` and `scripts/nudge-behaves.sh` before touching `mod nudge`. +- `hi/file.md`, `hi/capture.md` and `hi/check.md` for the criteria the assertions cite. + +## Current Status + +Sixty-seven tests, all passing, run by `cargo test --test cli` and by the `test` step of +`fledge lanes run verify`. Sixty-two at the top level and five inside `#[cfg(unix)] mod nudge`. The +whole suite finishes in under two seconds despite spawning a process per assertion, because the +fixtures are small and the binary does very little. + +Coverage by area, roughly: thirteen tests on argv routing, exit codes and the stream split; eight on +discovery, refusals and the format version; eleven on `check`; thirteen on `out`'s four verbs and the +`INTENT.md` rules; seven on id permanence including the five-fixture register and the three +concurrency tests; three on retire; three on `seed` and the agent file; one on the page; and the +five hook tests. + +Two things a future agent should know: + +1. **`outside_a_repository_it_says_so_rather_than_guessing` uses a fixed path.** It builds + `/hi-cli-norepo/deep/nested` with no pid in it, which is the one place Invariant 1 is not + honoured. It would collide with a concurrent run of itself. Nothing else in the file does this. +2. **`view_writes_a_self_contained_page` asserts on the HTML going in, not on the page.** It checks + the criterion text, the dark-mode query, the absence of `src=` and `https://`, and that the search + input, the sort options, the filter rail and a deep link are present. What the page *does* — + whether filtering actually hides a row — is `scripts/view-behaves.sh` in headless Chrome, because + two bugs shipped past HTML-level assertions exactly like these. + +## Notes + +- `assert!(!page.contains("src="))` in `view_writes_a_self_contained_page` is a substring test over + the whole document, so it also forbids `src=` inside a criterion's own text. No fixture has one, + and a future fixture that did would fail for a reason that is not the rule being tested. +- `a_criterion_in_a_file_hi_skips_is_reported_rather_than_vanishing` asserts the word `not lowercase` + in `check`'s output. That string is `check`'s wording for `StrayPlace::UnreadFile`; changing it is + a contract change in two places. +- `what_hi_writes_passes_hi_own_check` is the self-consistency test: hi retire wrote the reason after + the cases while hi check looked for it under the parent, so hi produced files that failed its own + check. It also asserts the retired *cases* carry no reason of their own, through `hi export`. +- `hi_s_own_files_are_not_counted_as_features` is the counterpart to `hi/AGENTS.md` being written on + a first capture: those files sit inside `hi/` and are hi's, so they are neither criteria files nor + lines in the product's feature list (DECISIONS.md §27). +- `seed_rewrites_a_template_hi_has_shipped_and_refuses_an_edit` `include_str!`s + `../src/seed/agents_0_5.md`, so the "a template hi has shipped" fixture is the shipped bytes rather + than a copy that could drift. +- The concurrency tests spawn threads that each spawn a process. The thread is only a way to start + them at once; nothing is asserted about the threads themselves, and no test asserts an ordering + between processes. +- Several tests run `hi check` *before and after* the operation under test and assert on both. That + pattern is deliberate: the defects this suite was written for mostly left `hi check` exiting 0 on + both sides, so the before-value is part of the evidence that the after-value means something. diff --git a/specs/cli/requirements.md b/specs/cli/requirements.md new file mode 100644 index 0000000..a7fb640 --- /dev/null +++ b/specs/cli/requirements.md @@ -0,0 +1,155 @@ +--- +spec: cli.spec.md +--- + +## User Stories + +- As someone typing `hi SEND-2 "..."`, I want the id-shaped first argument to reach capture rather than clap, so that writing a criterion is one command with no subcommand to remember (hi: CAPTURE-1) +- As someone putting hi's own options before the id, I want them peeled off and the rest of the line kept whole, so that a sentence containing a flag-shaped word is still my sentence (hi: CAPTURE-8, CAPTURE-9) +- As someone piping `hi export` into a file, I want results on stdout and everything else on stderr, so that a note about a feature list never lands in my JSON (hi: CAPTURE-11, INDEX-4.a) +- As someone whose script checks `$?`, I want 1 to mean hi refused and 2 to mean I typed the command wrong, so that the two are distinguishable (hi: CHECK-1) +- As someone whose file hi refused to write to, I want the file byte-for-byte as it was, including the files hi normally starts on a first capture, so that a refusal is never a half-applied command (hi: CAPTURE-5) +- As someone who typed their own markdown, I want hi to read it and hand it back in my style, bare lines, CRLF, BOM, block frontmatter and all, so that adopting hi is not a reformatting of what I already have (hi: FILE-4, FILE-7, FILE-10, FILE-11, FILE-14) +- As someone who has been handed an id, I want it never handed out again, whichever verb wrote it and wherever in the file it ended up (hi: ID-5, FILE-22) +- As someone running `fledge work push`, I want hi's lifecycle hook never to abort my command, whatever state my repository is in +- As the author of the next fix, I want each reproduced defect to keep its fixture and its comment, so that a bug that shipped once cannot ship twice + +## Acceptance Criteria + +### REQ-cli-001 + +The integration suite SHALL drive the binary as a process, from a throwaway repository, and SHALL assert on exit code, on each output stream separately, and on the bytes on disk. + +Acceptance Criteria + +- Every test spawns `CARGO_BIN_EXE_hi`, so the binary under test is built from the current tree. +- `Repo` creates its root under `std::env::temp_dir()` named `hi-cli--` and removes it first, so two `cargo test` runs never share a fixture. +- Exit codes are asserted as `out.status.code() == Some(n)` wherever the number is the claim: 0 for success, 1 for a refusal, 2 for a clap usage error. +- `stdout` and `stderr` are separate helpers and are asserted separately. A refusal asserts `stdout(&out).is_empty()`. +- Assertions carry the output or the file body in their failure message. + +### REQ-cli-002 + +The integration suite SHALL cover the routing decision that no unit test can reach (hi: CAPTURE-1, CAPTURE-8, CAPTURE-9, CAPTURE-1.c, CHECK-2.d). + +Acceptance Criteria + +- `an_id_shaped_argument_captures` proves an id-shaped first argument reaches capture rather than clap. +- `an_unknown_subcommand_is_a_usage_error` proves a non-id first argument reaches clap, exit 2. +- `a_wrongly_cased_id_is_reported_rather_than_read_as_prose` and `an_ordinary_hyphenated_word_is_not_mistaken_for_an_id` pin the two halves of `looks_like_id`: case-insensitive on the family, and requiring a digit after the hyphen. +- `root_is_honored_by_capture_and_not_swallowed_into_the_sentence` runs from a different working directory, in both the `--root PATH` and `--root=PATH` forms, and asserts the string `--root` appears nowhere in the file. +- `a_flag_looking_word_inside_a_sentence_stays_a_word` is the mirror: the same token after the id is written into the criterion intact. +- `a_non_utf8_argument_is_reported_not_panicked` (unix) asserts exit 1, `not valid UTF-8` on stderr, and the absence of `panicked`. +- `bare_invocation_prints_help_and_writes_nothing` asserts help is printed and `hi/` is still empty. + +### REQ-cli-003 + +The integration suite SHALL hold every refusal to writing nothing, at a strength chosen to match what that refusal could have written (hi: CAPTURE-5). + +Acceptance Criteria + +- `an_existing_id_refuses_with_exit_1_and_writes_nothing`, `a_malformed_id_refuses_without_writing`, `a_zero_padded_id_is_refused_rather_than_silently_renamed`, `index_refuses_rather_than_guessing_when_a_marker_is_unclosed` and `capturing_into_a_family_two_files_claim_refuses_and_writes_nothing` each snapshot the file before the run and `assert_eq!` after it. +- `a_refusal_over_the_format_version_writes_nothing_at_all` additionally asserts that `INTENT.md`, `hi/AGENTS.md`, `hi/CLAUDE.md` and `hi/.hi.lock` do not exist, and that `hi/` holds exactly the one entry that was already there. +- `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` compares a `listing` of every path under the root with its bytes, before and after, so the refusal is held to changing nothing at all. +- Each of these strengths exists because a refusal that wrote something shipped. + +### REQ-cli-004 + +The integration suite SHALL keep a register of the reproduced ways an id was handed out twice, one fixture per defect, each naming the defect it reproduces (hi: ID-5, FILE-13, FILE-20, FILE-22, RETIRE-5, RETIRE-6). + +Acceptance Criteria + +- `an_id_is_never_handed_out_twice` holds five fixtures in one function, each preceded by a comment naming the bug: a `## Retired` that is not the last section; a retirement reason containing newlines; a criterion inside a fence under `## Criteria`; a properly closed fenced example containing both `## Retired` and a criterion; and a fence the person never closed. +- Each block asserts both halves of the rule where the rule has two: an id hi cannot read is still taken, *and* an id nobody wrote is still free. Fixture 4 asserts that the documented `SEND-4` is still capturable and that the example prose is untouched. +- Fixture 5 asserts capture refuses **twice** for the same id and that the half-written file is byte-identical, because the defect was reporting the same id saved twice with both lines invisible. +- A block is never removed when its bug is fixed. + +### REQ-cli-005 + +The integration suite SHALL cover the concurrent write paths, asserting on what survived rather than on a count (hi: FILE-19, RETIRE-7, CAPTURE-14). + +Acceptance Criteria + +- `concurrent_captures_all_land` runs eight captures of distinct ids against a seeded repository and requires all eight in the file. +- `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` runs thirty-two against `Repo::bare`, requires every reported success to be readable, and requires that nothing refused — printing each refusal's stderr rather than counting them. Thirty-two rather than eight because eight did not reproduce the bootstrap race reliably. +- `concurrent_retires_never_bring_a_criterion_back` runs two retires at once and requires both ids to sit below `## Retired` afterwards, with the third criterion untouched. +- None of these asserts an ordering between processes. + +### REQ-cli-006 + +The integration suite SHALL assert that hi returns a person's file in the person's own style, over fixtures hi did not write (hi: FILE-4, FILE-7, FILE-10, FILE-11, FILE-14, FILE-1.b, FILE-1.c, FILE-6). + +Acceptance Criteria + +- `Repo::with_chat` seeds a **bare** `SEND-1 I hit enter and it shows up.` line: no bullet, no emphasis, and not what capture emits. +- `block_style_frontmatter_is_understood_and_preserved` asserts a YAML block `families:` list is read, gains the new family as another ` - NAME` item, and is never collapsed to `families: [`. +- `a_bom_does_not_make_a_valid_file_look_broken` asserts a BOM'd file checks clean. +- `a_hi_file_renders_as_a_list_not_a_wall_of_text` asserts the three written lines exactly, including the two-space indent on the case, and then round-trips them through `hi export`. +- `a_long_sentence_stays_on_one_line` asserts the captured line equals `- **SEND-2** ` for a sentence far past terminal width. +- `a_ticket_unwraps_prose_and_leaves_the_file_alone` asserts a wrapped paragraph arrives whole in the ticket, that a blank line stays a break, and that the source file is byte-identical afterwards. + +### REQ-cli-007 + +The integration suite SHALL assert that a generated feature list is true after every verb that changes the count, and that hi never writes prose a person removed (hi: INDEX-2, INDEX-4, INDEX-4.a, INDEX-4.c, INDEX-2.b, INDEX-2.c). + +Acceptance Criteria + +- The `HAND_WRITTEN_INTENT` fixture is prose either side of a block whose count is *already wrong*, so an assertion that the count is now right cannot pass by the block never being touched. +- `a_capture_refreshes_the_feature_list` and `a_retire_refreshes_the_feature_list` assert the corrected count and that the prose above and below the markers is byte-identical. +- `a_capture_survives_an_index_it_cannot_refresh` asserts exit 0, the criterion on disk, `INTENT.md` unchanged, and `not refreshed` on stderr. +- `a_feature_list_i_deleted_stays_deleted` asserts a capture leaves a marker-less `INTENT.md` untouched with nothing on stderr, and that `hi index` is how a person asks for one back. +- `a_root_file_hi_cannot_read_survives_a_capture_byte_for_byte` and `hi_index_refuses_a_root_file_it_cannot_read_rather_than_replacing_it` cover the undecodable file on both paths: a note and exit 0 from a capture, a refusal and exit 1 from `hi index`, every byte intact in both. +- `index_rewrites_only_the_generated_block` and `index_leaves_a_marker_quoted_in_prose_alone` assert the person's sentences, including one quoting the marker itself, survive. +- `the_first_capture_starts_the_product_intent_file` asserts the starter file is born with a true list. + +### REQ-cli-008 + +The integration suite SHALL assert that `hi check` fails on structure and only on structure, and that its notes are data (hi: CHECK-1, CHECK-2, CHECK-6). + +Acceptance Criteria + +- `check_is_clean_on_unfinished_intent` asserts exit 0 over a criterion with nothing implementing it. +- `check_fails_on_a_structural_problem`, `check_fails_on_a_duplicate_id`, `check_fails_when_two_files_claim_the_same_family` and `a_criterion_in_a_file_hi_skips_is_reported_rather_than_vanishing` each assert exit 1 and the problem kind by name, with the file and, for the orphan, the line number. +- `check_says_the_feature_list_is_behind_without_failing` asserts exit 0, the note present, and the word `problem` absent — a note is not a seventh problem. +- `check_json_carries_its_notes_as_a_list_with_codes` asserts `notes` is an array, each entry has a `kind`, no message contains a newline, and the old joined `note` string is `null`. + +### REQ-cli-009 + +The integration suite SHALL assert that a file declaring a later format version is refused by every verb, and that a file declaring none is still this format (hi: FILE-25, FILE-25.a). + +Acceptance Criteria + +- `a_file_from_a_later_format_is_refused_by_every_verb` loops over eleven invocations covering every subcommand and the capture path, and requires exit 1, both `hi/chat.md` and `hi: 2` on stderr, and empty stdout from each. +- `a_refusal_over_the_format_version_writes_nothing_at_all` asserts the refusal happens before `lock::acquire`, by requiring that no `hi/.hi.lock` was ever created. +- `a_file_that_declares_nothing_is_still_this_format` asserts a file written before `hi:` existed still works, found through the repository boundary rather than through the key it does not have. + +### REQ-cli-010 + +The integration suite SHALL assert that no path through the fledge lifecycle hook can abort the command that ran it. + +Acceptance Criteria + +- `mod nudge` is `#[cfg(unix)]` and runs `bin/fledge-hi-nudge` as a process, passing the moment as argv and the repository as `FLEDGE_REPO_ROOT`, the way fledge does. +- `no_path_through_the_hook_can_abort_a_push` requires exit 0 from all sixteen combinations of four moments (`start`, `push`, empty, `unknown-moment`) and four roots (a repository, a repository with a `hi/`, a path that does not exist, and no variable at all). +- `nothing_reaches_stdout_so_a_json_envelope_stays_valid` requires stdout empty and stderr non-empty at both speaking moments. +- `a_repository_with_nothing_written_down_is_told_so_at_both_moments` requires the two moments to say different things. +- `it_goes_quiet_once_something_is_written_down` requires silence once a `hi/` exists. +- `it_never_guesses_at_a_repository_it_was_not_told_about` runs with the hook's working directory set to a repository that has **no** `hi/`, so a hook guessing from cwd would speak and be caught. + +## Constraints + +- The suite tests a process. Anything assertable by calling a function belongs in that module's own unit tests; what lives here is what needs argv, an exit code, a stream or a byte on disk. +- Fixtures are files hi did not write, and they stay that way. Tidying a bare criterion line into capture's list form makes the suite a round-trip of the writer against its own output, which is the failure DECISIONS.md §26 records. +- Where a test asserts that a generated value was corrected, the seeded value must be wrong in a way the correct answer is not. +- Every scratch path carries the process id. +- `Repo::new` pre-creates `hi/` and therefore cannot exercise a first capture. A write-path test about a repository that has never run hi must use `Repo::bare`. +- Error strings asserted here are part of hi's contract with a person, and duplicate the assertions in the module unit tests on purpose: `already exists`, `next free is SEND-2`, `leading zero`, `not valid UTF-8`, `not a repository`, `not refreshed`, `it is yours`, `lowercase`, `feature list is behind`. +- The suite must keep running on Windows. Only the two things that cannot — `OsStrExt` and a shell script — are behind `#[cfg(unix)]`. + +## Out of Scope + +- The behavior being asserted. Each verb's rules live in its own spec; this suite only observes them from outside. +- Generated sequences. Random capture / retire / hand-edit orderings are `specs/promise/`; this suite pins named, reproduced cases. +- The page's behavior in a browser. `view_writes_a_self_contained_page` asserts on the HTML going in; what the page *does* is `scripts/view-behaves.sh`, driven in headless Chrome, because two bugs shipped past HTML-level assertions. +- `hi check`'s own coverage of the seven problem kinds, which lives in `specs/check/`; only the exit code, the kind names and the JSON shape are asserted here. +- The fledge plugin manifest, validated by `fledge plugins validate . --strict` in the gate rather than by a test. diff --git a/specs/cli/tasks.md b/specs/cli/tasks.md new file mode 100644 index 0000000..6d40b3e --- /dev/null +++ b/specs/cli/tasks.md @@ -0,0 +1,37 @@ +--- +spec: cli.spec.md +--- + +## Tasks + +- [x] Cover the routing decision a unit test cannot reach: an id-shaped first argument, a wrongly cased one, an ordinary hyphenated word, and an unknown subcommand. Evidence: `an_id_shaped_argument_captures`, `a_wrongly_cased_id_is_reported_rather_than_read_as_prose`, `an_ordinary_hyphenated_word_is_not_mistaken_for_an_id`, `an_unknown_subcommand_is_a_usage_error`. +- [x] Cover `--root` in both directions, from a different working directory. Evidence: `root_is_honored_by_capture_and_not_swallowed_into_the_sentence`, `a_flag_looking_word_inside_a_sentence_stays_a_word`. +- [x] Keep a register of the reproduced ways an id was handed out twice. Evidence: `an_id_is_never_handed_out_twice`, five fixtures. +- [x] Cover the concurrent write paths from a repository that has never run hi. Evidence: `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` (32 processes), `concurrent_retires_never_bring_a_criterion_back`. +- [x] Hold the strongest refusals to a byte-level listing of the whole tree. Evidence: `listing`, used by `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it`. +- [x] Prove no path through the fledge hook can abort a push, from a working directory that is not the repository. Evidence: `nudge::no_path_through_the_hook_can_abort_a_push`, `nudge::it_never_guesses_at_a_repository_it_was_not_told_about`. +- [ ] Give `outside_a_repository_it_says_so_rather_than_guessing` a pid-qualified path. It is the one fixture in the file that uses a fixed `/hi-cli-norepo`, and it would collide with a concurrent run of itself. +- [ ] Narrow `view_writes_a_self_contained_page`'s `src=` and `https://` assertions to the document's own markup. As substring tests over the whole page they would also fail on a criterion whose *text* contained either, which is a fixture hazard rather than a rule. +- [ ] Add a test for a single capture into a repository with no `hi/`. The path is covered only by the two tests that run many at once and assert on what survived; the plain case is in `specs/capture/`'s Manual Testing and nowhere else. +- [ ] Add a test for capturing into a file with no trailing newline, which gains one. It is the one documented exception to hi: FILE-4.a's byte-for-byte promise and nothing guards it. +- [ ] Add a test for a capture into a CRLF file through the binary. Line-ending preservation is covered at the `doc` level only (hi: FILE-10). +- [ ] Add a test for `hi issue --create`, which shells out to `gh`. Nothing exercises the `--create` path or its error message. +- [ ] Consider splitting the file. At 1,819 lines it is the largest in the repository, and the seven areas in Current Status are natural seams — but the register in `an_id_is_never_handed_out_twice` should stay in one piece. + +## Gaps + +- **No coverage of `hi issue --create`.** The `gh` invocation, its arguments and its failure message are untested end to end. +- **No coverage of a `--root` that does not exist.** `specs/main/` documents `error: resolving ` from `canonicalize`; no test asserts it. +- **No coverage of `hi view --out `.** Only the default output path is exercised. +- **No coverage of a single bootstrap capture.** Every test that starts from `Repo::bare` runs several processes at once, so the ordinary first-capture-in-a-new-repository path is only ever observed under contention. +- **Nothing asserts the page's behavior.** `view_writes_a_self_contained_page` asserts on the HTML; `scripts/view-behaves.sh` is the only thing that opens it, and only the local gate and CI run that. +- **`mod nudge` does not run on Windows.** Coverage of the hook there is `scripts/nudge-behaves.sh` in the local gate, which is also unix. +- **No test asserts that stdout stays machine-readable across a verb that also writes a note.** `a_capture_survives_an_index_it_cannot_refresh` asserts the note is on stderr, but nothing parses stdout as a record in the presence of one. +- **No negative test for the fixture rule itself.** Nothing fails if a future fixture is written in capture's own list form, which is the weakening DECISIONS.md §26 is about. + +## Review Sign-offs + +- **Product**: pending +- **QA**: pending +- **Design**: n/a +- **Dev**: pending diff --git a/specs/cli/testing.md b/specs/cli/testing.md new file mode 100644 index 0000000..e71ef12 --- /dev/null +++ b/specs/cli/testing.md @@ -0,0 +1,87 @@ +--- +spec: cli.spec.md +--- + +## Automated Testing + +This module *is* the automated testing layer, so what follows is what runs, and what each area of +it asserts. Sixty-seven tests: sixty-two at the top level and five inside `#[cfg(unix)] mod nudge`. +Run them with `cargo test --test cli`, a single one with `cargo test --test cli `, or the +whole gate with `fledge lanes run verify`. The suite finishes in under two seconds despite spawning +a process per assertion. + +| Test File | Type | What It Covers | +|-----------|------|----------------| +| `tests/cli.rs` | Integration, spawns `CARGO_BIN_EXE_hi` | Everything only a process has: argv routing, exit codes, the stdout/stderr split, and the bytes on disk afterwards | +| `tests/cli.rs` (`Repo::new`) | Fixture helper | `/hi-cli--/` with an empty `hi/`. Cannot exercise a first capture — `hi/` already exists | +| `tests/cli.rs` (`Repo::with_chat`) | Fixture helper | `Repo::new` plus `hi/chat.md`: `families: [SEND]`, an `## Intent`, and a **bare** `SEND-1 I hit enter and it shows up.` on line 10. Both the bare form (hi: FILE-14) and the line number (quoted by the duplicate-id message) are load-bearing | +| `tests/cli.rs` (`Repo::bare`) | Fixture helper | A `.git` and no `hi/`. The only state the bootstrap lock defect is reachable from (DECISIONS.md §33) | +| `tests/cli.rs` (`listing`) | Assertion helper | Every path under the root with its bytes, sorted. The strongest "wrote nothing" assertion in the repository | +| `tests/cli.rs` (`future_format`) | Fixture helper | A repository whose criteria file declares `hi: 2` (hi: FILE-25) | +| `tests/cli.rs` (`HAND_WRITTEN_INTENT`) | Fixture constant | Prose either side of a generated block whose count is already wrong, so a correction cannot be confused with the block never being touched | +| `tests/cli.rs` (`mod nudge`) | Integration, unix only, spawns `bin/fledge-hi-nudge` | The fledge lifecycle hook, driven as fledge drives it, from a working directory that is deliberately not the repository | + +### Requirement Coverage + +| Requirement | Covering Tests | +|-------------|----------------| +| REQ-cli-001 (drive the binary; assert exit code, streams and bytes) | Structural, and exercised by all sixty-seven. The exit-code discipline is visible in `an_existing_id_refuses_with_exit_1_and_writes_nothing` (`Some(1)`) against `an_unknown_subcommand_is_a_usage_error` (`Some(2)`); the stream discipline in the `stdout(&out).is_empty()` assertions on every refusal | +| REQ-cli-002 (routing only a process can reach) | `an_id_shaped_argument_captures`, `bare_invocation_prints_help_and_writes_nothing`, `an_unknown_subcommand_is_a_usage_error`, `a_wrongly_cased_id_is_reported_rather_than_read_as_prose`, `an_ordinary_hyphenated_word_is_not_mistaken_for_an_id`, `root_is_honored_by_capture_and_not_swallowed_into_the_sentence`, `a_flag_looking_word_inside_a_sentence_stays_a_word`, `a_non_utf8_argument_is_reported_not_panicked` (unix) | +| REQ-cli-003 (a refusal writes nothing, at a chosen strength) | File-level: `an_existing_id_refuses_with_exit_1_and_writes_nothing`, `a_malformed_id_refuses_without_writing`, `a_zero_padded_id_is_refused_rather_than_silently_renamed`, `index_refuses_rather_than_guessing_when_a_marker_is_unclosed`, `capturing_into_a_family_two_files_claim_refuses_and_writes_nothing`, and fixture 5 of `an_id_is_never_handed_out_twice`. Absence-level: `a_refusal_over_the_format_version_writes_nothing_at_all`. Tree-level: `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` | +| REQ-cli-004 (the register of id defects) | `an_id_is_never_handed_out_twice`, five fixtures; plus `a_retired_id_in_a_file_hi_skips_is_never_handed_out_again` and `a_capture_never_brings_a_retired_criterion_back_to_life`, which are two more of the same family with tests of their own | +| REQ-cli-005 (concurrent write paths) | `concurrent_captures_all_land` (8), `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` (32, from `Repo::bare`), `concurrent_retires_never_bring_a_criterion_back` (2 retires). The capture/retire race is in `specs/promise/`'s `tests/promise.rs` rather than here | +| REQ-cli-006 (a person's file comes back in their style) | `block_style_frontmatter_is_understood_and_preserved`, `a_bom_does_not_make_a_valid_file_look_broken`, `a_hi_file_renders_as_a_list_not_a_wall_of_text`, `a_long_sentence_stays_on_one_line`, `a_ticket_unwraps_prose_and_leaves_the_file_alone`. **Not covered:** a capture into a CRLF file, and a file with no trailing newline | +| REQ-cli-007 (the generated feature list stays true, and hi writes no prose a person removed) | `index_rewrites_only_the_generated_block`, `index_leaves_a_marker_quoted_in_prose_alone`, `a_capture_refreshes_the_feature_list`, `a_retire_refreshes_the_feature_list`, `a_capture_survives_an_index_it_cannot_refresh`, `a_feature_list_i_deleted_stays_deleted`, `a_root_file_hi_cannot_read_survives_a_capture_byte_for_byte`, `hi_index_refuses_a_root_file_it_cannot_read_rather_than_replacing_it`, `the_first_capture_starts_the_product_intent_file`, `hi_s_own_files_are_not_counted_as_features` | +| REQ-cli-008 (`check` fails on structure and only on structure) | `check_is_clean_on_unfinished_intent`, `check_fails_on_a_structural_problem`, `check_fails_on_a_duplicate_id`, `check_fails_when_two_files_claim_the_same_family`, `a_criterion_in_a_file_hi_skips_is_reported_rather_than_vanishing`, `check_says_the_feature_list_is_behind_without_failing`, `check_json_carries_its_notes_as_a_list_with_codes` | +| REQ-cli-009 (a later format version is refused; an absent one is not) | `a_file_from_a_later_format_is_refused_by_every_verb` (eleven invocations), `a_refusal_over_the_format_version_writes_nothing_at_all`, `a_file_that_declares_nothing_is_still_this_format` | +| REQ-cli-010 (the fledge hook cannot abort a push) | `nudge::no_path_through_the_hook_can_abort_a_push`, `nudge::a_repository_with_nothing_written_down_is_told_so_at_both_moments`, `nudge::nothing_reaches_stdout_so_a_json_envelope_stays_valid`, `nudge::it_goes_quiet_once_something_is_written_down`, `nudge::it_never_guesses_at_a_repository_it_was_not_told_about`. **Unix only** | + +### What Each Test Asserts + +Grouped by area. Every assertion below carries the relevant output or file body in its failure +message. + +| Area | Tests and what they pin | +|------|-------------------------| +| Routing and argv | `an_id_shaped_argument_captures` (exit 0, `+SEND-2` on stdout, the line in the file); `bare_invocation_prints_help_and_writes_nothing` (`Usage` printed, `hi/` still empty); `an_unknown_subcommand_is_a_usage_error` (exit **2**); `a_wrongly_cased_id_is_reported_rather_than_read_as_prose` (`unparseable-id` and `send-2` in `check`'s output, exit 1); `an_ordinary_hyphenated_word_is_not_mistaken_for_an_id` (`spec-sync and well-formed are ordinary words.` leaves `check` clean); `root_is_honored_by_capture_and_not_swallowed_into_the_sentence` (both `--root` forms, run from elsewhere; `--root` appears nowhere in the file); `a_flag_looking_word_inside_a_sentence_stays_a_word` (the same token after the id survives intact); `a_non_utf8_argument_is_reported_not_panicked` (exit 1, `not valid UTF-8`, no `panicked`) | +| Capture and refusal | `a_long_sentence_stays_on_one_line` (the line equals `- **SEND-2** ` exactly); `a_new_family_starts_its_own_file` (`created` on stdout, `families: [BILLING]` in the new file); `an_existing_id_refuses_with_exit_1_and_writes_nothing` (exit 1, `already exists`, `next free is SEND-2`, empty stdout, file byte-identical); `a_malformed_id_refuses_without_writing`; `a_zero_padded_id_is_refused_rather_than_silently_renamed` (`leading zero`); `capturing_into_a_family_two_files_claim_refuses_and_writes_nothing` (both file names on stderr, both files unchanged) | +| Discovery | `outside_a_repository_it_says_so_rather_than_guessing` (`not a repository`, exit 1); `a_hindi_locale_directory_is_not_mistaken_for_a_workspace` (run from `public/locales`, the capture lands in the real workspace and nothing is created inside the locale directory); `a_repository_is_a_boundary_for_discovery` (`hi check` inside a nested `.git` reports `0 criteria`) | +| Markdown and the person's own style | `a_hi_file_renders_as_a_list_not_a_wall_of_text` (three exact lines including the two-space indent, then a round-trip through `hi export`); `block_style_frontmatter_is_understood_and_preserved` (` - SEND` survives, ` - RECEIPT` joins it, never `families: [`); `a_bom_does_not_make_a_valid_file_look_broken` (`1 criterion`, exit 0) | +| `check` | `check_is_clean_on_unfinished_intent` (exit 0); `check_fails_on_a_structural_problem` (`orphan-case`, the file, and the line number); `check_fails_on_a_duplicate_id` (`duplicate-id`); `check_fails_when_two_files_claim_the_same_family` (`duplicate-family` in both the text and the JSON `problems`); `a_criterion_in_a_file_hi_skips_is_reported_rather_than_vanishing` (`stray-criterion`, `SEND-9`, `hi/NOTES.md`, `not lowercase`); `check_says_the_feature_list_is_behind_without_failing` (exit 0, the note present, the word `problem` absent, and the note gone after `hi index`); `check_json_carries_its_notes_as_a_list_with_codes` (`notes` is an array, each entry has a `kind`, no message contains a newline, `note` is null) | +| `export`, `issue`, `view` | `export_stdout_is_parseable_json` (`hi == 1`, `scope == "repo"`, the first criterion's id); `export_rejects_a_scope_that_matches_nothing` (exit 1, empty stdout); `export_accepts_the_path_it_prints` (`chat`, `chat.md`, `hi/chat.md` all resolve); `export_says_which_shape_it_is_apart_from_which_format_it_read` (`hi == 1` and `export == 1` are different fields); `issue_prints_a_ticket_carrying_the_id` (`hi: SEND-1` and the intent prose); `a_ticket_unwraps_prose_and_leaves_the_file_alone` (the wrapped paragraph arrives whole, a blank line stays a break, the source file is byte-identical); `a_ticket_does_not_print_the_sentence_twice`; `view_writes_a_self_contained_page` (the criterion text, `prefers-color-scheme: dark`, no `src=`, no `https://`, and the search, sort, filter, deep-link and rail markup) | +| `INTENT.md` | `index_rewrites_only_the_generated_block`; `index_refuses_rather_than_guessing_when_a_marker_is_unclosed` (exit 1, file unchanged); `index_leaves_a_marker_quoted_in_prose_alone` (the person's sentence *about* the marker survives, and so does the blank line after the block); `a_capture_refreshes_the_feature_list` (`(2 criteria)`, with `starts_with` and `ends_with` pinning the prose either side); `a_retire_refreshes_the_feature_list` (seeded `(9 criteria)`, asserted `(2 criteria)`); `a_capture_survives_an_index_it_cannot_refresh` (exit 0, criterion on disk, file unchanged, `not refreshed` on stderr); `a_feature_list_i_deleted_stays_deleted` (a capture leaves a marker-less file untouched with nothing on stderr; `hi index` puts one back); `a_root_file_hi_cannot_read_survives_a_capture_byte_for_byte` (a Latin-1 byte: capture succeeds, `INTENT.md` byte-identical, note on stderr); `hi_index_refuses_a_root_file_it_cannot_read_rather_than_replacing_it` (the same file, exit 1, every byte intact); `the_first_capture_starts_the_product_intent_file` (`INTENT.md` exists, named on stdout, born with `## Features` and a true count, and `check` says `no product-level why` without failing); `hi_s_own_files_are_not_counted_as_features` (`1 file`, and neither `AGENTS` nor `CLAUDE` in the generated list) | +| Id permanence | `an_id_is_never_handed_out_twice`, five fixtures — a `## Retired` that is not the last section; a reason containing newlines (and `SEND-5` still free afterwards); a fenced criterion under `## Criteria`; a documented example whose `## Retired` and `SEND-4` are prose (the retirement reaches the real section, `hi ls --retired` finds it, the example is untouched, `SEND-4` is still capturable); and an unclosed fence (two refusals, file byte-identical). Plus `a_retired_id_in_a_file_hi_skips_is_never_handed_out_again` (`hi/Archive.md`, `lowercase` on stderr, `check` reports the same id, `SEND-5` still free) and `a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` (an undecodable byte: `check` exits 1 naming the file, the capture refuses, the whole tree is byte-identical) and `a_capture_never_brings_a_retired_criterion_back_to_life` (an indented `## Retired`: `1 criterion` and `1 retired` afterwards, the heading is not part of the sentence, and the retired id is still spent) | +| Concurrency | `concurrent_captures_all_land` (eight distinct ids, all present); `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` (thirty-two from `Repo::bare`; every reported success present, and **no** refusals, each printed with its stderr); `concurrent_retires_never_bring_a_criterion_back` (both retired ids below `## Retired`, the third untouched) | +| Retire | `retire_moves_a_criterion_and_its_cases_out_of_the_way` (the case goes with its parent, `check` still passes, the id stays spent); `retire_works_without_a_reason` (a `## Retired` section and no `retired:` note); `what_hi_writes_passes_hi_own_check` (the cases are *named* in stdout, `check` passes, and `hi export` shows the reason on the parent and not on the case) | +| Format version | `a_file_from_a_later_format_is_refused_by_every_verb` (eleven invocations: exit 1, `hi/chat.md` and `hi: 2` on stderr, empty stdout from each); `a_refusal_over_the_format_version_writes_nothing_at_all` (no `INTENT.md`, `hi/AGENTS.md`, `hi/CLAUDE.md` or `hi/.hi.lock`, and `hi/` holds exactly one entry); `a_file_that_declares_nothing_is_still_this_format` (`hi ls` finds `SEND-1`) | +| Seed and the agent file | `the_agent_file_says_to_check_the_ids_after_a_merge` (`hi check`, `merge` and `same id` all present); `seed_rewrites_a_template_hi_has_shipped_and_refuses_an_edit` (a shipped 0.5 template is `updated`; an edited file exits 1 with `it is yours` and is unchanged); `seed_writes_the_instruction_without_a_dummy_capture` (`hi/AGENTS.md` and `hi/CLAUDE.md` created, and **no** `INTENT.md`) | +| The fledge hook (unix) | `no_path_through_the_hook_can_abort_a_push` (sixteen combinations, all exit 0); `a_repository_with_nothing_written_down_is_told_so_at_both_moments` (`No hi/ here` at start, `before it ships` at push, and the two differ); `nothing_reaches_stdout_so_a_json_envelope_stays_valid` (stdout empty, stderr non-empty); `it_goes_quiet_once_something_is_written_down` (stderr empty once `hi/` exists); `it_never_guesses_at_a_repository_it_was_not_told_about` (silent with no `FLEDGE_REPO_ROOT`, with a non-existent one, and outside a repository — run from a decoy repository that has no `hi/`) | + +## Manual Testing + +- [ ] Run `cargo test --test cli` twice at once in two terminals and confirm nothing flakes. Every fixture carries a pid except `outside_a_repository_it_says_so_rather_than_guessing`, which is the one that would collide. +- [ ] `cargo clean -p human-intent && cargo test --test cli` whenever a result here disagrees with what `./target/release/hi` does by hand. A stale binary has twice made a passing change look like a code bug. +- [ ] Rewrite `Repo::with_chat`'s seeded criterion into capture's own list form and confirm the suite still passes. It will, and it covers less. That is the weakening DECISIONS.md §26 is about, and nothing automated catches it. +- [ ] Seed `HAND_WRITTEN_INTENT` with the correct count and confirm `a_capture_refreshes_the_feature_list` still passes. It will, without the block ever being touched. +- [ ] Change one fixture in `an_id_is_never_handed_out_twice` to use `Repo::new` where it used `Repo::bare` and confirm the bootstrap coverage disappears silently. +- [ ] Run the suite on Windows and confirm the concurrency tests pass. The one refusal ever observed in `concurrent_captures_into_a_repository_with_no_hi_directory_all_land` was a lock handoff there. +- [ ] Run `sh scripts/view-behaves.sh` after changing anything the page does. `view_writes_a_self_contained_page` asserts on the HTML going in and two bugs shipped past exactly that kind of assertion. +- [ ] Break `bin/fledge-hi-nudge` deliberately (make it `exit 1`) and confirm `no_path_through_the_hook_can_abort_a_push` fails. It is the one test here protecting somebody else's command. + +## Edge Cases & Boundary Conditions + +| Scenario | Expected Behavior | +|----------|-------------------| +| Two `cargo test` runs at once | Safe everywhere except `outside_a_repository_it_says_so_rather_than_guessing`, whose `/hi-cli-norepo` path carries no pid | +| `target/debug/hi` is stale | `CARGO_BIN_EXE_hi` makes cargo rebuild, so the suite is safe; a *manual* comparison against a stale binary is not | +| A fixture is written in capture's own list form | The test passes and covers less. Nothing automated notices | +| A generated count is seeded correctly | The refresh assertion passes without the block being touched. Nothing automated notices | +| A test about a first capture is built on `Repo::new` | Passes for the wrong reason; `hi/` exists, so `lock::acquire` never creates it | +| A line is added to `Repo::with_chat`'s fixture | The duplicate-id message quotes line 10; other assertions and `specs/capture/` both read that number | +| A criterion's *text* contains `src=` or `https://` | `view_writes_a_self_contained_page` would fail, because its assertions are substring tests over the whole page rather than over the markup | +| The `not lowercase` wording in `check` changes | `a_criterion_in_a_file_hi_skips_is_reported_rather_than_vanishing` fails. That string is `check`'s wording for `StrayPlace::UnreadFile` and is a contract in two places | +| `gh` is absent from the machine | Nothing fails. No test exercises `hi issue --create` | +| Running on Windows | Sixty-five of the sixty-seven run. `a_non_utf8_argument_is_reported_not_panicked` and `mod nudge` are `#[cfg(unix)]` | +| A concurrency test refuses one id and succeeds the rest | A failure, with every refusal's stderr in the message. An id nobody else asked for is free | +| The order in which two concurrent processes land | Unspecified and unasserted. Only the state afterwards is asserted | +| `hi check` exits 0 over a file that is actually broken | What several of these tests exist to catch; it is why they assert on `check` both before and after the operation | diff --git a/specs/promise/context.md b/specs/promise/context.md new file mode 100644 index 0000000..1741464 --- /dev/null +++ b/specs/promise/context.md @@ -0,0 +1,126 @@ +--- +spec: promise.spec.md +--- + +## Key Decisions + +- **The property is the specification, and the generator is the adversary.** Everything else in + `specs/` describes behavior a person asked for. This describes the one invariant that spans every + verb: an id, once seen, belongs to one sentence forever. It is the only thing hi promises + (DECISIONS.md §4), so it is the only thing that gets a test whose input hi chooses. +- **Twelve defects were found by a person writing a case; none by hi generating one.** That is the + sentence the module doc opens with and it is the reason the fixtures look the way they do. Every + generator this repository had written before produced files hi itself would have written, so the + write path was tested against its own output and agreed with itself. The six `generated_body` + shapes are all things a person types and hi never emits. +- **The postcondition compares shapes, not ids, and that distinction cost a release.** + `read_back` used to compare a sorted multiset of ids before and after a write. DECISIONS.md §35 + is the nine lines of markdown that walked through it: an indented `## Retired`, a capture above + it, and afterwards two active criteria and none retired — SEND-1 live again, still carrying + `retired: Dropped.`. Every id was still readable. *A postcondition is only as strong as the thing + it compares.* `Shape` is `(id, section, text, note)` for exactly that reason, and the four fields + are not negotiable. +- **A verb's report is the claim, so it is never the evidence.** `assert_promise` throws away the + `Workspace` the step was holding and reloads from disk. Capture returning `Ok` with an id in it + is precisely what is under test; checking that report against itself would have passed through + every one of the five id defects on record. +- **A clean `hi check` is an assertion, not a convenience.** Invariant 4 is the one that catches + silent corruption, and it is written as a conditional because `check` failing is a legitimate + state mid-sequence: a hand-added criterion can be an orphan, a hand-edit can break alternation. + What is not legitimate is `check` exiting 0 over a file where an id is written twice. Four of the + five defects had exactly that shape, before and after. +- **Refusals are steps.** A refusal must leave the workspace shape-identical, not merely + criterion-identical (hi: CAPTURE-5). The `Err` arms all run the postcondition with nothing named, + which is how the suite would catch a refusal that had already created `hi/`, written `INTENT.md`, + or left a lock file behind. +- **The split between the two halves is about what each can ask, and it is not redundancy.** + `src/promise.rs` is `#[cfg(test)] mod promise;` inside the binary crate, so it can call + `capture::capture` and `check::run` and can ask `Workspace::load` which *section* a criterion came + back in. It cannot fork. `tests/promise.rs` can spawn eight processes and can only see bytes on + disk. A concurrency defect invisible to a serial sequence, and a section defect invisible from + outside the binary, have both shipped. Moving either half to the other side loses one of them. +- **`Repo::bare` and not `Repo::new`.** The concurrent fixtures create a `.git` and no `hi/`, + because the first capture in an adopting repository has no directory to put a lock file in, and + `lock::acquire` used to hand back a `Guard` when the open failed. Every bootstrap capture then ran + unlocked. A fixture that pre-creates `hi/` cannot reach that path at all (DECISIONS.md §33). +- **Eight threads for the same-id case, thirty-two for the bootstrap case.** The number in + `tests/cli.rs` is thirty-two because eight did not reproduce the bootstrap race reliably and + because thirty-two is the shape adoption actually has. The numbers here are eight, which is enough + for a lock that either exists or does not. +- **A refusal in the concurrent capture test is a failure, and its stderr is printed.** An id nobody + else asked for is free, so `WANT-7` refusing while `WANT-8` succeeds is a defect. The assertion + joins every refusal's stderr into the message rather than counting them, because the one time it + fired it was a lock handoff on Windows and a count says nothing about that. +- **Splitmix64 by hand, eleven lines.** hi depends on `anyhow`, `clap` and `serde_json`. A + property-testing crate would be a fourth dependency for a generator, in a project whose stated + rule is minimal dependencies. The tradeoff accepted is that there is no shrinking: a failure gives + a seed and a step count, not a minimal counterexample. +- **The seeds are fixed and the fixture is chosen by the seed.** `[1, 7, 13, 29, 41, 99, 256, 1024]` + with `seed % 6` picking the starting body, so the eight runs do not all start from the same shape. + A clock-seeded generator would find more over time and would make every failure unreproducible, + which is a worse trade for a gate that has to be green or red. +- **The hand-edit step replaces a line, not a substring.** `step`'s arm 4 finds the line containing + both the target id and the old sentence before falling back to `body.replacen(&old_text, ..)`. + Two hand-typed criteria can share a prefix, and a naive substring replace would edit a different + criterion than the one it named — which would make the step *pass* while testing nothing, because + the criterion it actually changed would be in the unnamed set and the assertion would fire on the + wrong thing. + +## Files to Read First + +- `src/promise.rs`, top to bottom. It is 386 lines and the order is: `Rng`, `Shape` and the four + accessors over a `Workspace`, `temp_root`, `generated_body`, the id helpers, `assert_promise`, + `step`, and the two `#[test]` functions. `assert_promise` is the whole contract in 60 lines; read + it before changing any arm of `step`. +- `tests/promise.rs`, all 220 lines. `typed_file()` is the fixture; the three tests are the + concurrent half. +- `DECISIONS.md` §26 "The one promise, and the four ways hi broke it" for why a generated test + exists at all, §33 for the fifth way and the bootstrap lock, §35 for why the postcondition + compares shapes, and §36 for the reservation lookup the taken-id step leans on. +- `hi/id.md` for ID-5 and ID-5.a, which are the two sentences this module exists to hold true, and + `hi/file.md` for FILE-19, FILE-22 and FILE-22.c. +- `src/capture.rs` and `src/doc.rs`'s `retire` / `insert` / `read_back`, which are the write paths + under test. `specs/capture/` and `specs/doc/` describe them; this module only asserts about them. +- `tests/cli.rs`'s `an_id_is_never_handed_out_twice` (`specs/cli/`), which is the hand-written + counterpart: five named, reproduced defects as five fixed fixtures. The two are complements — + that test pins the bugs that were found, this module looks for the ones that were not. + +## Current Status + +Complete and passing. Two `#[test]` functions in `src/promise.rs` (eight seeds, 24 steps each, plus +an eight-step run that pins the fixture) and three in `tests/promise.rs` (eight threads, eight +threads, and a capture racing a retire). All five run under plain `cargo test` with no feature flag +and no environment variable, which is deliberate: a property test behind a flag is a property test +nobody runs. + +The suite is a regression net, not a search. It is sized to finish inside an ordinary `cargo test`, +so a long soak is something a person runs by editing the seed list, not something CI does. + +Two things a future agent should know: + +1. **`step` arm 3 can return early without asserting anything.** If `workspace.find_id(target)` + comes back `None` — which can happen because `live_ids` was computed from an earlier load — the + arm returns. That is correct but it means a sequence can silently do fewer steps than 24. Nothing + counts the steps that actually ran. +2. **The tail of `random_sequences_over_files_hi_did_not_write_keep_the_promise` is nearly a no-op.** + The loop over `seen` with the `if appearances.len() == 1 { let _ = (first, text); }` body asserts + nothing; the real assertion after it is that every id in `assigned` is still present. The dead + loop is harmless and reads as though it checks something. It is listed in `tasks.md`. + +## Notes + +- `check::run(&workspace).expect("check is operational")` panics rather than skipping if `check` + itself errors. That is intended: a workspace hi cannot check is already a failure of the promise. +- `shapes_of` drops criteria whose id did not parse, through the `filter_map` on `c.id.as_ref()?`. + So an unparseable line is not a `Shape` and cannot be compared before-and-after. The hand-edit + step never produces one, but a future arm that does would be invisible to Invariant 2. +- The `note` field of `Shape` is the retirement reason. It is the field DECISIONS.md §35 found had + moved to a criterion that did not own it, and it is the reason the comparison is four-field. +- `next_top_level` asks `Workspace::next_free`, so the generated ids follow the same rule a person + reading the file would. It does not attempt to guess a case's letter; arm 1 picks a letter from + `a..h` directly and is allowed to collide, because a collision must be refused. +- `tests/promise.rs` asserts `stdout` contains `+WANT-` rather than parsing it. The exact line + format is `specs/main/`'s contract; this only needs to know which id a process claimed. +- Neither half asserts anything about wall-clock time or ordering between processes. Whether the + capture or the retire lands first in `concurrent_retire_and_capture_keep_every_unnamed_criterion` + is not specified and must not be asserted; what is asserted is that both are true afterwards. diff --git a/specs/promise/promise.spec.md b/specs/promise/promise.spec.md new file mode 100644 index 0000000..a1c00f0 --- /dev/null +++ b/specs/promise/promise.spec.md @@ -0,0 +1,262 @@ +--- +module: promise +version: 1 +status: active +files: + - src/promise.rs + - tests/promise.rs + +db_tables: [] +depends_on: + - specs/capture/capture.spec.md + - specs/check/check.spec.md + - specs/doc/doc.spec.md + - specs/id/id.spec.md + - specs/main/main.spec.md + - specs/workspace/workspace.spec.md +--- + +# Promise + +## Purpose + +The property tests of the one promise hi makes: **an id a person has seen is never given to a +second sentence, a criterion hi said it saved is one hi can find again, and a step leaves every +criterion it did not name exactly as it was** (hi: ID-5, ID-5.a, FILE-22.c). + +Every other spec in this repository describes a module that ships. This one describes the +adversary. It owns no product behavior and exports nothing; what it owns is a *method*, and the +method is the reason it is specified rather than left as an unremarkable test file. Twelve +confirmed defects in the write path were found by a person sitting down and writing a case. None +were found by hi generating one, because every generator hi had written until then generated files +hi itself would have written (DECISIONS.md §26). So the fixtures here are deliberately files hi +would never produce — bare criterion lines with no bullet, CRLF from Notepad, a fenced example of +the format inside somebody's intent prose, an indented `## Retired`, a stray line below a `# ` +heading — and the sequences run over them are random. + +The module is in two halves, split by what each half can ask. + +`src/promise.rs` is compiled into the binary crate under `#[cfg(test)]`, so it can call +`capture::capture`, `Doc::retire` and `check::run` directly and can ask `Workspace::load` which +*section* a criterion came back in. That is what makes its postcondition a comparison of shapes — +`(id, section, sentence, reason)` — rather than a comparison of ids. A comparison of ids is what +`read_back` used to do, and DECISIONS.md §35 is the nine lines of markdown that walked through it: +every id was still readable afterwards, and a retired criterion had come back to life carrying +somebody else's retirement reason. *A postcondition is only as strong as the thing it compares.* + +`tests/promise.rs` runs the release binary in threads, which is the only way to reach the half of +the promise that is about two processes at once: a lock that is really the kernel's, a +read-modify-write that reloads under it, and an id that eight simultaneous writers all ask for +(hi: FILE-19, RETIRE-7). It asserts through the filesystem, because that is all a second process +leaves behind. + +Neither half is a unit test of anything. A unit test names a function and an expected answer; these +name a property and let the generator find the input. When one fails, the seed is in the assertion +message and the failing sequence is reproducible by running that seed alone. + +## Public API + +| Export | Description | +|--------|-------------| +| None. | `src/promise.rs` is a private `#[cfg(test)]` module of the binary crate and declares no `pub` item; `tests/promise.rs` is an integration test binary. Nothing in hi depends on either, and neither is compiled into a release build. | + +### Structs & Enums + +| Type | Description | +|------|-------------| +| `Rng` (private, `src/promise.rs`) | Splitmix64 over a single `u64`. `next` advances and mixes, `pick` chooses from a slice, `below(n)` bounds to `0..n` with `n.max(1)`. Hand-written on purpose: hi's dependency budget is `anyhow`, `clap` and `serde_json`, and a generator is not worth a fourth (DECISIONS.md §26). Deterministic, so a failing seed is a reproduction. | +| `Shape` (private, `src/promise.rs`) | `{ id: String, section: Section, text: String, note: Option }`. One criterion as a *reader of the file* would find it. Derives `PartialEq, Eq, PartialOrd, Ord` so a whole workspace can be compared as a sorted `Vec`. The four fields are exactly the four things §35 found a comparison of ids could not see. | +| `Repo` (private, `tests/promise.rs`) | A throwaway repository under `std::env::temp_dir()`. Only `bare` exists here: a `.git` and **no** `hi/`, because the first capture in an adopting repository starts there and a fixture that pre-creates `hi/` cannot see the bootstrap lock defect (DECISIONS.md §33). | + +### Traits + +| Trait | Description | +|-------|-------------| +| None. | This module defines no traits. | + +### Functions + +| Function | Signature | Description | +|----------|-----------|-------------| +| `shapes_of` | `fn shapes_of(workspace: &Workspace) -> Vec` | Every criterion in every loaded doc, active and retired alike, as a sorted `Vec`. Criteria whose id did not parse are dropped by the `filter_map`, so an unparseable line is not a shape and cannot be compared. | +| `ids_in` | `fn ids_in(workspace: &Workspace) -> BTreeMap>` | Every *appearance* of every id, keyed by id. A key with more than one entry is an id written twice, which is the promise's headline failure. | +| `readable_in` | `fn readable_in(workspace: &Workspace, id: &str, section: Section) -> bool` | Whether a freshly loaded workspace can find that id in that section. "Reported saved" is checked against this, never against the return value the verb gave. | +| `unnamed_shapes` | `fn unnamed_shapes(before: &[Shape], named: &BTreeSet) -> Vec` | The shapes whose id the step did not name. These must be identical before and after; the named ones are the only ones a verb is allowed to change. | +| `temp_root` | `fn temp_root(name: &str) -> PathBuf` | `/hi-promise--`, removed and recreated with a `hi/` inside. The pid is not decoration: a fixed scratch path shared by two `cargo test` runs is the bug that made this suite flake and made bulk capture lose writes. | +| `generated_body` | `fn generated_body(kind: u32) -> String` | Six starting files, chosen by `kind % 6`, **none of which capture would have written**. See Behavioral Examples for the list. | +| `live_ids` | `fn live_ids(workspace: &Workspace) -> Vec` | Parsed ids from `doc.criteria` only, so a step that wants something to retire or to hang a case off does not pick a retired one. | +| `all_parsed_ids` | `fn all_parsed_ids(workspace: &Workspace) -> Vec` | Parsed ids from `doc.all()`, active and retired, so the "capture a taken id" step can pick a retired one and must still be refused. | +| `next_top_level` | `fn next_top_level(workspace: &Workspace, family: &str) -> String` | `-`, the id a person reading the file would type next. | +| `assert_promise` | `fn assert_promise(root, reported: &[(String, Section)], before: &[Shape], named: &BTreeSet, assigned: &BTreeMap)` | The postcondition. Reloads the workspace from disk and asserts the four things in Invariants 1 to 4. Called after **every** step, including the ones that refuse. | +| `step` | `fn step(rng: &mut Rng, root: &Path, assigned: &mut BTreeMap)` | One random move against the repository: capture a new id, capture a case, capture a taken id, retire, hand-edit a sentence, or hand-add a criterion. Each arm computes what it named and calls `assert_promise`. | +| `typed_file` | `fn typed_file() -> &'static str` (`tests/promise.rs`) | A file a person typed: frontmatter, an `## Intent`, and `WANT-1` and `WANT-2` as **bare** lines. Capture would have written `- **WANT-1**`, and the tests assert that it does not contain that, so a run that only round-trips hi's own output cannot pass. | + +## Invariants + +1. **An id a verb reported saved is readable, from disk, in the section it was reported in.** Not + in the value the verb returned: `assert_promise` throws the in-memory workspace away and calls + `Workspace::load(root)` again. A verb's own report is the claim under test, so it is never also + the evidence. +2. **Every criterion the step did not name is unchanged in all four of its fields.** `before` and + `after` are filtered through `unnamed_shapes` and compared with `assert_eq!`. Id, section, + sentence and retirement reason are all in the comparison, because a criterion that keeps its id + while changing section, changing its words, or inheriting a neighbour's `retired:` note is a + criterion that was lost (DECISIONS.md §35, hi: FILE-22.c, ID-5.a). +3. **No id a verb assigned ever carries a second sentence.** `assigned` accumulates + `id -> sentence` across the whole run, and any later appearance of that id must carry the same + words. A *person* may write the same id twice — that is a duplicate, and `hi check` reports it — + but a verb doing it is the promise breaking (hi: ID-5). +4. **`hi check` exiting 0 is a claim, and the claim is checked.** When `check::run(..).ok()` is + true, `assert_promise` additionally requires that no id appears more than once anywhere, that + every reported id is readable, and that nothing unnamed moved. This is the assertion that makes + a silent corruption fail: four of the five id defects in DECISIONS.md §26, §33 and §35 left the + file wrong and `hi check` exiting 0 both before and after. +5. **A refusal is a step too.** Every arm that can fail calls `assert_promise` on the failure path + with an empty `reported` and an empty `named`, so a refused capture is held to changing + *nothing*, not merely to not writing a criterion (hi: CAPTURE-5). +6. **The sequence starts from a file hi did not write.** `generated_body` produces six shapes and + `a_file_hi_did_not_write_is_where_the_sequence_starts` asserts the starting text does not contain + `**WANT-1**`. `tests/promise.rs` asserts the same of `typed_file`. This is the guard against the + failure mode DECISIONS.md §26 names: a generator that round-trips the writer's own output tests + the writer against itself and finds nothing. +7. **The generator is deterministic and the seeds are fixed.** `[1, 7, 13, 29, 41, 99, 256, 1024]`, + 24 steps each, with the starting file chosen by `seed % 6` so the eight runs do not all begin + from the same shape. A failure names its seed, and running that seed alone reproduces it. Do not + make the seed a function of the clock: a property test that cannot be re-run is a bug report with + no repro. +8. **Nothing here is allowed to need a new dependency.** `Rng` is eleven lines because the + alternative is a proptest or quickcheck dependency in a crate whose whole dependency list is + three entries, two of which are clap and anyhow. +9. **The two halves do not overlap and neither is redundant.** `src/promise.rs` can see sections + and shapes and cannot see a second process; `tests/promise.rs` can see a second process and can + only see bytes. A concurrency defect that is invisible to a serial sequence, and a section + defect that is invisible from outside the binary, have both shipped. +10. **The concurrent half asserts on what survived, not on a count.** In + `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` every process that + *reported* success must be readable in the file, and no process may quietly refuse — an id + nobody else asked for is free, so a refusal is a failure and its stderr is printed rather than + counted. The one time that fired it was a lock handoff on Windows, and a count would have said + nothing about it. +11. **The temp roots carry the pid.** Both halves build their scratch directory from + `std::process::id()`. Two `cargo test` runs at once otherwise share a fixture, wipe each other's + files, and the failures read as flakes. + +## Behavioral Examples + +#### Scenario: The six starting files, none of which hi wrote + +- **Given** `generated_body(kind)` for `kind % 6` in `0..6` +- **Then** the sequence starts from, in order: bare criterion lines with no bullet and no emphasis + (hi: FILE-14); list items with a nested case; a fenced `markdown` example of the format inside + `## Intent`, plus a real `## Retired` below it (hi: FILE-9, FILE-22.b); two families declared in + one file; a file saved with CRLF line endings (hi: FILE-10); and a file with a criterion-shaped + line stranded below a `# Appendix` heading (hi: CHECK-2.e) + +#### Scenario: A capture that succeeds + +- **Given** any point in a sequence +- **When** `step` captures a new top-level id or a case of a live one and `capture` returns `Ok` +- **Then** the id is recorded in `assigned` with its sentence — panicking immediately if that id was + already assigned — and `assert_promise` requires it readable in `Section::Criteria`, every other + criterion unchanged, and, if `hi check` exits 0, no id written twice anywhere + +#### Scenario: A capture that refuses + +- **Given** a workspace holding `WANT-1`, active or retired +- **When** `step` captures `WANT-1` again with a different sentence +- **Then** the call must return `Err` — `assert!(err.is_err(), "a taken id must refuse")` — and + `assert_promise` runs with nothing named, so the whole workspace must be shape-identical + (hi: CAPTURE-3, CAPTURE-5) + +#### Scenario: A retire names its descendants + +- **Given** a live `WANT-1` with cases under it +- **When** `step` calls `Doc::retire` and saves +- **Then** the ids it returns as taken, plus the target, are the `named` set, every one of them must + be readable in `Section::Retired`, and every criterion outside that set is unchanged + (hi: RETIRE-2, ID-5.a) + +#### Scenario: A person edits a sentence by hand + +- **Given** any criterion in the file +- **When** `step` rewrites its sentence directly on disk +- **Then** only that id is `named`, and every other criterion must be untouched. The replacement + finds the *line* carrying both the id and the old text before falling back to a substring + replace, because two hand-typed criteria can share a prefix and a naive `replacen` would edit the + wrong one — the fixture bug that would have made this step silently test nothing + +#### Scenario: A person types a criterion hi did not write + +- **Given** a workspace whose next free id in `WANT` is `WANT-5` +- **When** `step` appends `- **WANT-5** a hand-typed line for WANT-5.` to the file itself, creating + a `## Criteria` heading if there is none +- **Then** `WANT-5` is `named`, everything else is unchanged, and the *next* capture must not hand + `WANT-5` out again (hi: FILE-14, CAPTURE-3) + +#### Scenario: Eight processes capture eight different ids + +- **Given** a bare repository whose `hi/want.md` is `typed_file()` +- **When** eight threads run the real binary for `WANT-3` through `WANT-10` at once +- **Then** every one exits 0, each prints `+WANT-`, every reported id is in the file afterwards, + and the two sentences the person typed are still there (hi: FILE-19) + +#### Scenario: Eight processes ask for the same id + +- **Given** the same repository +- **When** eight threads all run `hi WANT-3 "sentence from waiter "` +- **Then** exactly one exits 0 and `WANT-3` appears exactly once in the file. `wins` is asserted + equal to 1 and the winning stdout is printed on failure, so "two of them won" and "none of them + won" are different messages (hi: ID-5, FILE-19) + +#### Scenario: A retire and a capture at the same time + +- **Given** a repository holding `WANT-1`, `WANT-2` and `WANT-3` +- **When** one thread captures `WANT-4` while another retires `WANT-2` +- **Then** both exit 0, `WANT-4` is in the file, `WANT-2` is below `## Retired`, and `WANT-1` — + which neither named — still carries the words the person typed (hi: RETIRE-7, FILE-22.c) + +## Error Cases + +This module produces no user-facing errors. Its "error cases" are the failures it is built to +report, and each one names what it saw. + +| Condition | Behavior | +|-----------|----------| +| A verb reports an id saved and `Workspace::load` cannot find it in that section | Panic: ` was reported saved in
and Workspace::load cannot find it there` (hi: FILE-22) | +| A step changes a criterion it did not name | Panic: `a step moved a criterion it did not name`, with the two sorted `Vec` in the diff, so the field that changed is visible (hi: ID-5.a) | +| A verb writes a second sentence under an id it already assigned | Panic: ` was assigned a second sentence by a verb: first <..>, now <..>` (hi: ID-5) | +| `capture` returns `Ok` twice for one id | Panic from `step` before the postcondition runs: ` assigned twice: first <..>, then <..>` | +| `hi check` exits 0 while an id is written twice | Panic: `hi check exited 0 with written twice: `. This is the shape every silent corruption in DECISIONS.md §26, §33 and §35 had | +| `hi check` exits 0 while a reported id is unreadable, or an unnamed criterion moved | Panic naming which of the two it was | +| Capturing an id that is already taken returns `Ok` | Panic: `a taken id must refuse, got a write of ` | +| A concurrent capture of a distinct id refuses | Panic listing every refusal with its stderr: `every capture asked for an id of its own, and these did not land:` | +| Two concurrent captures of one id both succeed | Panic: `the same id was assigned times: ` | +| The starting fixture turns out to be something hi would have written | Panic from the fixture assertion itself, before any step runs | +| `Workspace::load` fails at any point in a sequence | Panic from `.expect("workspace still loads")`. A file hi wrote that hi can no longer read is a failure of the promise, not a skipped test | +| A retire fails mid-sequence | Not a failure. The arm calls `assert_promise` with nothing named and returns: a refusal that writes nothing is allowed, and is still held to writing nothing | + +## Dependencies + +### Consumes + +| Crate/Module | What is used | +|-------------|-------------| +| `std` | `collections::{BTreeMap, BTreeSet}`, `fs`, `path::{Path, PathBuf}`, `process::{Command, Output}`, `thread`, `env::temp_dir`, `process::id` | +| `crate::capture` | `capture`, and the `Captured` it returns | +| `crate::check` | `run`, `Report::ok` — the conditional in Invariant 4 | +| `crate::doc` | `Section`, and `Doc::retire` / `Doc::save` through the workspace's docs | +| `crate::id` | `Id`, `Id::parse` | +| `crate::workspace` | `Workspace::load`, `Workspace::find_id`, `Workspace::next_free` | +| `CARGO_BIN_EXE_hi` | `tests/promise.rs` only: the release binary under test, resolved by cargo at compile time | + +### Consumed By + +| Module | What is used | +|--------|-------------| +| None. | `#[cfg(test)]` and an integration target. Nothing in a release build references either file. | + +## Change Log + +| Date | Author | Change | +|------|--------|--------| +| 2026-09-19 | Claude | Initial specification. Written after `specsync coverage` reported `src/promise.rs` as the repository's one uncovered file (386 LOC), and `fledge atlas` reported `tests/promise.rs` alongside it. Documents the property, the two halves and why they are two, the six fixtures and why none of them is a file hi would write, and the eleven invariants that make a failure here reproducible. | diff --git a/specs/promise/requirements.md b/specs/promise/requirements.md new file mode 100644 index 0000000..7e73297 --- /dev/null +++ b/specs/promise/requirements.md @@ -0,0 +1,129 @@ +--- +spec: promise.spec.md +--- + +## User Stories + +- As someone who has written a hundred criteria, I want an id I have seen never to be given to a second sentence, whatever order I captured, retired and hand-edited in, so that the one thing hi promises is the one thing I never have to verify myself (hi: ID-5) +- As someone who keeps a file open in an editor while hi writes to it, I want every criterion to keep its id, its section and its words across a step that did not name it, so that a command about one criterion is a command about one criterion (hi: ID-5.a, FILE-22.c) +- As someone hi has just told "saved", I want that to mean hi can find it again, so that success on stdout is not a different claim from the file on disk (hi: FILE-22) +- As two people on one repository, I want two hi processes writing at once to both land, or one of them to refuse, so that a race is never resolved by silently losing somebody's thought (hi: FILE-19, RETIRE-7) +- As the author of the next change to the write path, I want the test that catches me to start from a file I did not write, so that a bug that only appears in somebody else's markdown is not invisible to the suite (DECISIONS.md §26) +- As the author of a failing build, I want the seed printed with the failure, so that a property test is a reproduction and not a rumour + +## Acceptance Criteria + +### REQ-promise-001 + +The promise tests SHALL assert, after every step of every sequence, that each id a verb reported as saved is readable from a freshly loaded workspace in the section it was reported in (hi: FILE-22, ID-5.a). + +Acceptance Criteria + +- `assert_promise` discards the in-memory `Workspace` the step used and calls `Workspace::load(root)` again, so the verb's own report is never also the evidence for it. +- Each `(id, section)` in `reported` is checked with `readable_in`, which scans `doc.all()` across every loaded doc and compares both the rendered id and the `Section`. +- The failure message names the id and the section: ` was reported saved in
and Workspace::load cannot find it there`. +- A capture reports `Section::Criteria`; a retire reports `Section::Retired` for the target and for every descendant `Doc::retire` returned as taken. + +### REQ-promise-002 + +The promise tests SHALL assert that every criterion a step did not name is unchanged in its id, its section, its sentence and its retirement reason (hi: ID-5.a, FILE-22.c). + +Acceptance Criteria + +- `Shape` carries all four fields and derives `Ord`, and the comparison is `assert_eq!` over two sorted `Vec` filtered by `unnamed_shapes`. +- The comparison is *not* over ids. A criterion that keeps its id while moving from `## Retired` to `## Criteria`, losing its words, or acquiring a neighbour's `retired:` note must fail (DECISIONS.md §35). +- `named` is computed per step: the captured id; the retired id plus every id `Doc::retire` reported taken; the hand-edited id; the hand-added id. A refusal names nothing. +- The failure message is `a step moved a criterion it did not name`, and the assertion prints both vectors so the changed field is visible in the diff. + +### REQ-promise-003 + +The promise tests SHALL assert that no id a verb assigned ever carries a second sentence (hi: ID-5). + +Acceptance Criteria + +- `assigned: BTreeMap` accumulates across the whole sequence and is never cleared. +- `step` panics with ` assigned twice` if `capture` returns `Ok` for an id already in the map, before the postcondition runs. +- `assert_promise` walks every id with more than one appearance and, when that id is in `assigned`, requires every appearance to carry the first sentence. +- A duplicate written by a *person* is permitted here and is `hi check`'s business (`duplicate-id`); only a verb assigning one twice is a failure of this property. + +### REQ-promise-004 + +The promise tests SHALL treat a clean `hi check` as a claim and check it (hi: CHECK-1, ID-5). + +Acceptance Criteria + +- When `check::run(&workspace).expect("check is operational").ok()` is true, three further assertions run: no id appears more than once anywhere in the workspace, every reported id is readable in its section, and the unnamed shapes are unchanged. +- The failure message for the first is `hi check exited 0 with written twice: `. +- This is the assertion that fails on a *silent* corruption. Four of the five id defects on record left the file wrong with `hi check` exiting 0 both before and after (DECISIONS.md §26, §33, §35). +- `check::run` returning `Err` is a panic, not a skip: a workspace hi cannot check is already a failure. + +### REQ-promise-005 + +The promise tests SHALL hold a refusal to writing nothing at all, not merely to writing no criterion (hi: CAPTURE-5). + +Acceptance Criteria + +- The `Err` arm of the capture step calls `assert_promise` with an empty `reported` and an empty `named`, so every shape in the workspace is compared. +- The taken-id step asserts the call failed before it asserts anything else: `a taken id must refuse, got a write of `. +- A `Doc::retire` that returns `Err` runs the same empty postcondition and returns without saving. + +### REQ-promise-006 + +The promise tests SHALL start every sequence from a file hi itself would not have written (DECISIONS.md §26). + +Acceptance Criteria + +- `generated_body` returns six distinct bodies chosen by `kind % 6`: bare lines with no bullet; list items with a nested case; a fenced `markdown` example plus a real `## Retired`; two families in one file; CRLF throughout; and a criterion-shaped line stranded below a `# Appendix` heading. +- `a_file_hi_did_not_write_is_where_the_sequence_starts` asserts `!body.contains("**WANT-1**")` before writing the fixture, so a fixture quietly "fixed" into capture's own list form fails the test rather than weakening it. +- `tests/promise.rs` makes the same assertion about `typed_file()` after writing it. +- The seeded sequence picks its body with `seed % 6`, so the eight runs do not all begin from the same shape. + +### REQ-promise-007 + +The promise tests SHALL be deterministic and reproducible from the value printed with a failure. + +Acceptance Criteria + +- `Rng` is splitmix64 over one `u64` and takes its seed from the fixed list `[1, 7, 13, 29, 41, 99, 256, 1024]`. +- Each seed runs 24 steps against its own temp root, named `serial-`, so a failure names the seed in the path and in `seed {seed}: ...` assertions. +- No step reads the clock, the environment, or the network. +- The generator adds no dependency to the crate. + +### REQ-promise-008 + +The promise tests SHALL exercise the concurrent half through the real binary, in a repository that has never run hi (hi: FILE-19, RETIRE-7, CAPTURE-5). + +Acceptance Criteria + +- `tests/promise.rs` spawns `CARGO_BIN_EXE_hi` in threads and passes `--root` rather than relying on a shared working directory. +- `Repo::bare` creates a `.git` and no `hi/`, which is the state a first capture starts in and the only state in which the bootstrap lock defect is reachable (DECISIONS.md §33). +- `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` requires every reported success to be readable afterwards and requires that nothing refused, printing each refusal's stderr rather than counting them. +- `concurrent_captures_of_the_same_id_assign_it_once` requires exactly one winner and exactly one occurrence of `WANT-3` in the file. +- `concurrent_retire_and_capture_keep_every_unnamed_criterion` requires both commands to exit 0, `WANT-4` present, `WANT-2` below `## Retired`, and `WANT-1` — which neither named — still carrying the person's words. + +### REQ-promise-009 + +The promise tests SHALL NOT share a scratch path between processes. + +Acceptance Criteria + +- Both `promise::temp_root` and `tests::promise::Repo::bare` build their directory from `std::process::id()`. +- Each root is `remove_dir_all`ed before it is created, so a sequence never inherits a previous run's file. +- This is the same defect `doc::write_atomically` had, and it is why the integration suite flaked and why bulk capture lost writes. + +## Constraints + +- This module ships nothing. It must never be reachable from a release build: `src/promise.rs` is behind `#[cfg(test)]` in `src/main.rs` and `tests/promise.rs` is an integration target. +- No new dependency. `Rng` exists because hi's dependency list is `anyhow`, `clap` and `serde_json`, and a property-testing crate would be a fourth for a generator that fits in eleven lines. +- The postcondition compares shapes, never ids. Weakening it back to a comparison of ids reopens DECISIONS.md §35, where every id was still readable and a retired criterion had come back to life. +- The fixtures are files hi did not write, and they stay that way. A fixture rewritten into the form capture emits turns the suite into a round-trip of the writer against itself, which is the failure DECISIONS.md §26 records: twelve defects found by hand, none by a generator. +- The serial half must keep asking `Workspace::load` about `Section`, which is why it lives inside the crate rather than in `tests/`. The concurrent half must keep spawning processes, which is why it does not. +- A failing seed must remain runnable on its own. Do not seed from the clock and do not shuffle the seed list. + +## Out of Scope + +- What a criterion, an id, a section or a retirement *is*: `specs/id/`, `specs/doc/`. +- The behavior being asserted. Capture's refusals live in `specs/capture/`, retire and the section rules in `specs/doc/`, the seven structural problems in `specs/check/`, the lock in `specs/workspace/`. +- Argv routing, exit codes and the stdout/stderr split, which `tests/cli.rs` covers (`specs/cli/`). The concurrent half here reads stdout only to confirm which id a process claimed. +- Coverage measurement. Whether a line of hi is executed is `fledge coverage`'s question; whether the promise holds is this one's. +- Performance. The sequences are sized to run inside `cargo test` without a flag, not to search the space exhaustively. diff --git a/specs/promise/tasks.md b/specs/promise/tasks.md new file mode 100644 index 0000000..172ee6a --- /dev/null +++ b/specs/promise/tasks.md @@ -0,0 +1,27 @@ +--- +spec: promise.spec.md +--- + +## Tasks + +- [x] Compare shapes rather than ids, so a criterion that changes section, sentence or retirement reason is caught. Evidence: `Shape`, `unnamed_shapes`, and the `a step moved a criterion it did not name` assertion (DECISIONS.md §35). +- [x] Reload from disk inside the postcondition, so a verb's report is never its own evidence. Evidence: `assert_promise`'s `Workspace::load(root)`. +- [x] Make a clean `hi check` an assertion rather than a convenience. Evidence: the `check::run(..).ok()` branch in `assert_promise`. +- [x] Start every sequence from a file hi did not write. Evidence: `generated_body`'s six shapes and `a_file_hi_did_not_write_is_where_the_sequence_starts`. +- [x] Hold a refusal to writing nothing at all. Evidence: the `Err` arms calling `assert_promise` with nothing named. +- [x] Cover the concurrent half through the real binary, from a repository that has never run hi. Evidence: the three tests in `tests/promise.rs` over `Repo::bare`. +- [ ] Remove the dead loop at the end of `random_sequences_over_files_hi_did_not_write_keep_the_promise`. The `if appearances.len() == 1 { let _ = (first, text); }` body asserts nothing and reads as though it does; the surviving assertion is the `assigned.keys()` loop below it. +- [ ] Count the steps that actually ran. `step` arm 3 returns early when `find_id` misses, so a 24-step sequence can perform fewer and nothing notices. A counter asserted at the end would make a sequence that degenerated into no-ops visible. +- [ ] Decide whether a `Shape` should exist for a criterion whose id did not parse. `shapes_of`'s `filter_map` drops them, so an unparseable line is outside Invariant 2 entirely. +- [ ] Add a step that hand-edits a *retirement reason* rather than a sentence. `Shape::note` is compared but no arm changes one, so the field DECISIONS.md §35 was actually about is only exercised through `Doc::retire`. +- [ ] Add a step that runs `hi index` or `hi seed` between writes. Both take the lock and both touch files, and neither is in any sequence. +- [ ] Consider a longer soak behind an explicit command rather than a feature flag, so the seed list can be widened without slowing `cargo test`. + +## Gaps + +- **No shrinking.** A failure gives a seed and a starting fixture, not a minimal counterexample. Reproducing means running that seed and reading the sequence. +- **No coverage of `hi issue`, `hi export`, `hi ls` or `hi view` inside a sequence.** They only read, so they cannot break the promise directly, but none of them is exercised against a file a sequence mutated. +- **The concurrent half never mixes a hand edit into a race.** A person saving the file in an editor while two hi processes write is the shape most likely to be met in practice and is not covered anywhere. +- **No Windows-specific assertion.** `tests/promise.rs` runs wherever `cargo test` does, and the one refusal ever observed in `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` was a lock handoff on Windows. The test prints the stderr it saw; nothing pins the platform behavior. +- **Nothing asserts that the two halves stay on their own side of the line.** A future serial test added to `tests/promise.rs` would quietly lose access to `Section`, and a future concurrent test added to `src/promise.rs` cannot fork at all. +- **The seed list is fixed at eight.** Widening it is a one-line edit nobody is prompted to make, and there is no record of how much of the step space eight seeds × 24 steps actually reaches. diff --git a/specs/promise/testing.md b/specs/promise/testing.md new file mode 100644 index 0000000..2017bbb --- /dev/null +++ b/specs/promise/testing.md @@ -0,0 +1,72 @@ +--- +spec: promise.spec.md +--- + +## Automated Testing + +This module *is* tests, so "automated testing" here means what runs and what each run asserts, +rather than a separate suite testing a separate implementation. Five `#[test]` functions across +two files, all under plain `cargo test` with no feature flag and no environment variable. Run them +with `cargo test promise` (both halves), `cargo test --bin hi promise::` (the serial half) or +`cargo test --test promise` (the concurrent half). `fledge lanes run verify` runs all of them +through its `test` step. + +| Test File | Type | What It Covers | +|-----------|------|----------------| +| `src/promise.rs` | Property, in-crate, real filesystem under `std::env::temp_dir()` | Random capture / retire / hand-edit sequences over files hi did not write, asserted on shape and section after every step | +| `src/promise.rs` (`temp_root`) | Fixture helper | `/hi-promise--`, wiped and recreated with a `hi/` inside. The pid is what keeps two `cargo test` runs from sharing a scratch directory | +| `src/promise.rs` (`generated_body`) | Fixture helper | Six starting files, none of which capture would have written: bare lines, nested list items, a fenced example beside a real `## Retired`, two families in one file, CRLF, and a stray criterion below a `# ` heading | +| `src/promise.rs` (`assert_promise`) | Postcondition | The whole property. Reloads from disk, then asserts reported-is-readable, unnamed-is-unchanged, no-second-sentence, and the `hi check` conditional | +| `tests/promise.rs` | Property, integration, spawns `CARGO_BIN_EXE_hi` in threads | The concurrent half: two processes writing the same file, an id eight processes all want, and a retire racing a capture | +| `tests/promise.rs` (`Repo::bare`, `typed_file`) | Fixture helpers | A `.git` and no `hi/`, seeded with a file whose criteria are bare lines. Both halves of the fixture are deliberate: `bare` is the only state the bootstrap lock defect is reachable from, and `typed_file` is asserted not to contain `**WANT-1**` | + +### Requirement Coverage + +| Requirement | Covering Tests | +|-------------|----------------| +| REQ-promise-001 (reported saved means readable, from disk, in that section) | `assert_promise`'s `readable_in` loop, reached by every step of both `random_sequences_over_files_hi_did_not_write_keep_the_promise` and `a_file_hi_did_not_write_is_where_the_sequence_starts`. Through the binary: `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` re-reads the file and requires every `+WANT-` it saw on stdout to be in it | +| REQ-promise-002 (unnamed criteria are unchanged in all four fields) | `assert_promise`'s `assert_eq!(after_unnamed, before_unnamed)`, after every step. Through the binary: `concurrent_retire_and_capture_keep_every_unnamed_criterion` asserts `WANT-1` still carries the sentence the person typed while a capture and a retire both landed | +| REQ-promise-003 (no id carries a second sentence) | `step`'s `assigned` panic on a repeat `Ok`, and `assert_promise`'s walk over `ids_in` for ids in `assigned`. Through the binary: `concurrent_captures_of_the_same_id_assign_it_once` asserts `wins == 1` and one occurrence of `WANT-3` in the file | +| REQ-promise-004 (a clean `hi check` is checked) | The `check::run(..).ok()` branch of `assert_promise`, reached whenever the workspace is structurally clean. Through the binary: the `check.status.success()` branch of `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write`, which then requires the saved ids to be unique | +| REQ-promise-005 (a refusal writes nothing at all) | The `Err` arms of `step` kinds 0/1 and 3, and kind 2 in full: `assert!(err.is_err(), "a taken id must refuse")` followed by a postcondition with nothing named. **Partly covered:** the assertion is over shapes, so a refusal that created `hi/`, wrote `INTENT.md` or left a lock file behind is not caught here. `cli::a_file_hi_cannot_read_never_frees_the_id_reserved_in_it` covers that with a byte-level `listing` of the whole tree | +| REQ-promise-006 (the sequence starts from a file hi did not write) | `a_file_hi_did_not_write_is_where_the_sequence_starts` asserts `!body.contains("**WANT-1**")` and then that the typed `WANT-1` is still findable after eight steps; `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` makes the same assertion about `typed_file()` | +| REQ-promise-007 (deterministic, reproducible from the printed seed) | Structural: `Rng` is splitmix64 over a fixed seed list, `temp_root` names the directory `serial-`, and the `assigned.keys()` assertion prints `seed {seed}: {id} was saved and is gone`. **Not covered by a test** — nothing asserts that two runs of one seed produce the same sequence | +| REQ-promise-008 (the concurrent half, through the binary, from a bare repository) | All three tests in `tests/promise.rs`. `Repo::bare` is the fixture in every one | +| REQ-promise-009 (no shared scratch path) | Structural: `std::process::id()` in both `temp_root` and `Repo::bare`. **Not covered by a test** — the failure mode is two concurrent `cargo test` runs, which the suite cannot stage against itself | + +### What Each Test Asserts + +| Test | Assertions | +|------|------------| +| `random_sequences_over_files_hi_did_not_write_keep_the_promise` | For each of eight seeds: a temp root seeded with `generated_body(seed % 6)`, 24 random steps, and `assert_promise` after every one. Then, at the end of the seed, every id in `assigned` is still present in the workspace — `seed {seed}: {id} was saved and is gone` | +| `a_file_hi_did_not_write_is_where_the_sequence_starts` | The fixture does not contain `**WANT-1**`; after eight steps from seed 3, `workspace.find_id(Id::parse("WANT-1"))` is still `Some`. The criterion the person typed survives a sequence of hi's own writes | +| `assert_promise` (every call) | Four things: each `(id, section)` in `reported` is `readable_in` a freshly loaded workspace; `unnamed_shapes(after) == unnamed_shapes(before)`; every appearance of an id in `assigned` carries that id's first sentence; and, when `check::run(..).ok()`, no id appears twice, every reported id is readable, and nothing unnamed moved | +| `concurrent_captures_of_their_own_ids_all_land_in_a_file_hi_did_not_write` | The starting file does not contain `**WANT-1**`. Eight threads capture `WANT-3`..`WANT-10`; each success prints `+WANT-`; any failure panics with its stderr. Afterwards every saved id is in the file, and both sentences the person typed are still there. If `hi check` exits 0, the saved ids are unique | +| `concurrent_captures_of_the_same_id_assign_it_once` | Eight threads all capture `WANT-3` with different sentences. Exactly one exits 0 — `the same id was assigned {wins} times: {sentences:?}` — `WANT-3` appears exactly once in the file, and `WANT-1`'s typed sentence survives | +| `concurrent_retire_and_capture_keep_every_unnamed_criterion` | After seeding `WANT-3`, one thread captures `WANT-4` and another retires `WANT-2`. Both exit 0. `WANT-1` still carries `The first thought they typed.`; `WANT-4` is in the file; `WANT-2` appears *after* the `## Retired` heading | + +## Manual Testing + +- [ ] Run a single seed on its own (`cargo test --bin hi promise::random_sequences -- --nocapture` after narrowing the seed list) and read the sequence it performs. A property test whose steps nobody has ever watched is a property test that may be doing nothing. +- [ ] Widen the seed list to a few hundred and run it once before a release. The committed list is sized for the gate, not for a search. +- [ ] Run `cargo test` twice at once, in two terminals, and confirm neither run's fixtures disappear. This is the one thing REQ-promise-009 is about and the suite cannot stage it against itself. +- [ ] Revert the `Shape` comparison to a comparison of ids and confirm the suite still passes over the DECISIONS.md §35 fixture. It should, which is the point: only the four-field comparison catches that bug. +- [ ] Rewrite one `generated_body` fixture into the list form capture emits, and confirm the suite still passes. It will. That is the failure mode DECISIONS.md §26 records, and it is invisible unless somebody goes looking. +- [ ] On Windows, run `cargo test --test promise` several times and confirm no capture of a distinct id ever refuses. The one observed refusal was a lock handoff there. + +## Edge Cases & Boundary Conditions + +| Scenario | Expected Behavior | +|----------|-------------------| +| A step captures a case whose letter collides with an existing case | The capture is refused, the `Err` arm runs, and the whole workspace must be shape-identical. Arm 1 picks a letter from `a..h` and is allowed to collide on purpose | +| A step captures a case of a criterion that is retired between load and write | Accepted by capture, because `find_id` spans `Doc::all()`. The line joins the family's active block, which `specs/capture/` documents as a known departure. The postcondition does not object: the id is readable in `Section::Criteria`, which is what was reported | +| `live_ids` names an id that `find_id` cannot resolve | Arm 3 returns without asserting. Correct, but it means a sequence can perform fewer than 24 steps and nothing counts them | +| The hand-edit step's target shares a sentence prefix with another criterion | The line-level replacement finds the line carrying both the id and the old text first. Without it the step would edit a criterion it did not name, and the assertion would fire on the wrong one | +| A hand-added criterion is an orphan, or breaks alternation | `hi check` exits non-zero and Invariant 4's conditional simply does not run. The first three assertions still do | +| A criterion's id does not parse | It is not a `Shape` at all, so it is outside the before/after comparison entirely. No arm currently produces one | +| A retire returns `Err` | Not a failure. The arm asserts the empty postcondition and returns without saving | +| Two concurrent processes both refuse | `concurrent_captures_of_the_same_id_assign_it_once` fails with `wins == 0`, which is a different message from `wins == 2`. Both are defects and the message says which | +| A concurrent capture of a *distinct* id refuses | A failure, and every refusal's stderr is printed. An id nobody else asked for is free | +| The order in which a racing capture and retire land | Unspecified and deliberately unasserted. Only the state afterwards is asserted | +| The sequence leaves the workspace structurally broken at the end | Fine. No test asserts a clean `hi check` at the end of a sequence; a hand-added orphan is a legitimate thing for a person to have typed | +| Two `cargo test` runs at once | Each has its own pid and therefore its own scratch tree. This is the whole of REQ-promise-009 |