Merge pull request 'Move outstanding work from TODO.md into the issue tracker' (#17) from todo-to-issues into main

Reviewed-on: #17
2026-08-02 19:47:11 -04:00
parent 4cc2b71936 23df4069ab
commit 39f20def72
4 changed files with 111 additions and 150 deletions
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -133,11 +133,31 @@ anything that touches the pool, the registry, initialization, or the lock.
 Recent commits use short, imperative, sentence-case subjects, for example
 `Fix refcount leak and stack-trace buffer overflow`. Keep commits focused and
 describe the observable behavior changed. Pull requests should include a brief
-summary, tests run, and any compatibility impact for public macros, generated
+summary, tests run, any compatibility impact for public macros, generated
-headers, installation paths, or CMake/pkg-config consumers.
+headers, installation paths, or CMake/pkg-config consumers, and a link to the
 issue they close.
 ## Agent-Specific Instructions
 **Outstanding work goes in the issue tracker, not in a file.** Open an issue at
 <https://source.starfort.tech/andrew/libakerror/issues> — `tea issues create
 --repo andrew/libakerror` — naming the file and line, the functional
 consequence, and what closing it would touch. Label it by kind and blast radius
 and leave `status::grooming` on it until its scope and approach are settled.
 **Do not add outstanding items to `TODO.md`**: that file is the record of why
 the handler ladder is a major-version change, why a copied `akerr_ErrorContext`
 is a trap, why validating more inputs lowers branch coverage, and why the
 mutation score is a floor. A description of work still to do goes stale the
 moment somebody does it.
 **This library's defects are most often found by its consumers, so make filing
 them cheap.** Three defects in this library — `IGNORE()` leaking a context, the
 un-namespaced `coverage` target, and the missing `akerrorConfigVersion.cmake` —
 were written down in `libakstdlib`'s own notes and never here, so each was
 worked around once per consumer and nobody saw the pattern. A consumer filing
 upstream should cost them one issue instead of one workaround. `libakgl`,
 `libakstdlib` and `akbasic` are all on the same forge.
 Do not overwrite uncommitted user changes. Avoid editing generated files in
 `build/`; update `include/akerror.tmpl.h`, `src/error.c`, CMake files, tests,
 or scripts instead.
--- a/README.md
+++ b/README.md
@@ -33,7 +33,7 @@ This library has 6 guiding principles:
 | [docs/thread-safety.md](docs/thread-safety.md) | What thread safety here covers, what it does not, and how to hand an error to another thread |
 | [docs/building.md](docs/building.md) | Configure options, the generated header, and building without stdlib |
 | [UPGRADING.md](UPGRADING.md) | What changed in 1.0.0, 2.0.0 and 2.0.1, and how to migrate |
-| [TODO.md](TODO.md) | Known defects, ordered by blast radius |
+| [TODO.md](TODO.md) | The reasoning behind decisions and measurements. **Outstanding work is in [the issue tracker](https://source.starfort.tech/andrew/libakerror/issues).** |
 # Installation
--- a/TODO.md
+++ b/TODO.md
@@ -1,161 +1,103 @@
-# TODO
+# Record
-Working notes for `libakerror`. Outstanding items only.
+**Outstanding work is in the issue tracker, not in this file:**
 <https://source.starfort.tech/andrew/libakerror/issues>
-## 1. Only ThreadSanitizer is wired into CI, not ASan/UBSan
+Issues are labelled by kind and blast radius, and milestoned by what they can land
 in: `2.0.x` for anything that breaks no ABI, `2.1.0` for additive surface, `3.0.0`
 for the handler-ladder rewrite and the contract changes that go with it. Everything
 filed carries `status::grooming` until it has been through grooming.
-`AKERR_SANITIZE` builds the library and the tests with any sanitizer list, and
+What stays here is the reasoning a tracker has no place for.
 CI runs `-DAKERR_SANITIZE=thread` through `scripts/thread_test.sh`. Nothing runs
 `address,undefined` yet, and that is the one that covers the original
 motivation: mutation testing caught an out-of-bounds probe in the status-name
 hash table that the suite could not, because the failure mode was a write into
 adjacent BSS, which does not crash. Sharpening one test closed that instance;
 ASan would catch the whole class regardless of how sharp the assertions are.
-The machinery is in place — this is one more job in
+## Three defects were recorded only downstream
 `.gitea/workflows/ci.yaml` running
 `cmake -S . -B build/asan -DAKERR_SANITIZE=address,undefined`. Left separate
 because ASan and TSan cannot be combined in one build.
-## 2. `HANDLE`-level status aliasing is still undetectable
+Found while moving this file and `libakstdlib`'s into their trackers. Each is a
 defect **in this library**, each was written down in a *consumer's* TODO file, and
 none had an entry here:
-Two components can compile the same integer into a `case` label without ever
+| Issue | What it is | What it costs downstream |
-reserving a range or registering a name, and nothing sees it. Ownership
+|---|---|---|
-enforcement covers *naming*, which is the part the library mediates; the `case`
+| #14 | `IGNORE()` logs a context and never releases it | `libakstdlib`'s `aksl_tree_iterate` open-codes log-then-release by hand; every `IGNORE()` in `libakgl`'s `CLEANUP` blocks is a leaked slot |
-label never reaches it.
+| #15 | The `coverage` target is not namespaced when embedded, though `mutation` is | `libakstdlib` shadows `add_custom_target` to embed this library at all |
 | #16 | No `akerrorConfigVersion.cmake` is installed | No consumer can ask `find_dependency(akerror)` for a version floor |
-Closing this needs the `if`/`else if` handler ladder — rewriting
+**That is the finding worth keeping, more than the three defects.** A library whose
-`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` so status matching
+consumers record its defects in their own files has no way to see them: each
-is not restricted to integer constant expressions. That would also allow
+consumer knows one, nobody sees the pattern, and the workaround gets written three
-matching on ranges or predicates, and would let a handler resolve a code through
+times. The tracker is now the place, and a consumer filing upstream costs them one
-its owner. It touches the most load-bearing code in the library and every
+issue instead of one workaround.
 consumer's error handling at once, so it wants its own change.
-Note it would *not* by itself fix the "don't use `CATCH` or `FAIL_*_BREAK`
+## Why the handler ladder is a `3.0.0` change and not a patch
 inside a loop" hazard: that comes from exiting via `break`, not from `switch`.
-## 3. No registry introspection
+`PROCESS`/`HANDLE`/`HANDLE_GROUP`/`HANDLE_DEFAULT`/`FINISH` compile to a `switch`,
 which restricts status matching to integer constant expressions. That is what makes
 `HANDLE`-level aliasing undetectable (#4): two components can compile the same
 integer into a `case` label without reserving a range or registering a name, and
 **ownership enforcement never sees it, because it covers naming and the `case` label
 never reaches the library.**
-There is no way to ask who owns a status, or to enumerate reservations. The
+Rewriting it as an `if`/`else if` ladder would allow matching on ranges or
-"coordinate ranges at the dependency-stack level" advice in UPGRADING.md
+predicates and let a handler resolve a code through its owner. It also touches **the
-therefore has no tooling behind it.
+most load-bearing code in the library and every consumer's error handling at once.**
-A read-only accessor plus a dump through `akerr_log_method` would let a startup
+One thing it would *not* fix, recorded so nobody expects it to: the "don't use
-self-check or a CI job print the whole map for a linked stack. Cheap, additive,
+`CATCH` or `FAIL_*_BREAK` inside a loop" hazard comes from exiting via `break`, not
-and the natural next step for multi-component adoption.
+from `switch`.
-## 4. No way to release a reservation
+## Why a copied `akerr_ErrorContext` is a trap
-A plugin host that `dlopen`s many distinct plugins over a process lifetime
+Recorded because the struct looks copyable and is not, and #11 exists only if a
-accumulates ranges until the table fills. Reloading the *same* plugin is fine —
+consumer ever needs it.
 an identical repeat by the same owner is idempotent.
-## 5. Renaming a status is not safe against a concurrent lookup
+- **`stacktracebufptr` is self-referential.** `akerr_ErrorContext c = *src;` leaves
  the copy's cursor pointing into the source's buffer, so the copy logs correctly
  and then **corrupts a slot it does not own** the first time anything appends.
 - **`arrayid` is restored after the wipe** in `akerr_release_error()`
  (`src/error.c:395-398`), so a copied id makes the destination **impersonate the
  source's slot for the life of the process.**
 If it is ever built, the shape takes the destination as a parameter rather than
 allocating it: an allocating copy could fail on pool exhaustion, and **reporting
 that failure needs a pool slot**, so it would have to abort — a third `exit()` site
 in a library that deliberately has two.
 ## Why validating more inputs lowers branch coverage
 Branch coverage on `src/error.c` sits just above its 50% gate, and the gate is set
 where it is on purpose.
 Every `FAIL_*` site carries about **six** branch outcomes of error-construction
 machinery (`ENSURE_ERROR_READY`, `AKERR_STACKTRACE_APPEND`) that only run when that
 specific failure fires. **Every `PASS` site around a call that cannot fail carries
 about twenty-five.**
 So adding a defensive check lowers the ratio by construction. **Before adding one,
 expect to add a test that drives it**, as `tests/err_copy_string.c` does.
 ## Why the mutation score is a floor
 `scripts/mutation_test.py` configures each mutant with the default CMake options,
 so **a mutant that only breaks under concurrency is judged by a suite running
 without ThreadSanitizer.**
 Measured: deleting the pool's `akerr_mutex_lock()` call survives the run, **even
 though it is a real race.** Rebuilt and run directly, that mutant fails
 `tests/err_threads_pool.c` in 4 of 10 runs, and fails under `scripts/thread_test.sh`
 in 5 of 5.
 **81.2% is therefore a floor for that category, not a verdict.** #10 is the
 passthrough that would fix the measurement, and it belongs behind a flag: a
 TSan-instrumented suite per mutant costs roughly 6s instead of 0.4s.
 ## Why a registered name is returned by pointer
 `akerr_name_for_status(status, NULL)` returns a pointer into the registry rather
-than a copy, which is what makes it usable from inside `FAIL` — it needs no
+than a copy, **which is what makes it usable from inside `FAIL`** — it needs no
-buffer and no error context of its own. Registering a *second* name for a status
+buffer and no error context of its own.
 that already has one (`tests/err_name_ownership.c` covers that it is allowed)
 overwrites that buffer in place, so a thread reading the name at that moment can
 see a torn string. Every other registry operation is serialized; this one cannot
 be, because the reader is outside the lock by the time it reads the characters.
-Documented in docs/thread-safety.md and UPGRADING.md as "register names during
+The cost is that renaming a status is not safe against a concurrent lookup (#7):
-initialization". Closing it properly means making a registered name immutable —
+every other registry operation is serialized, and this one cannot be, because **the
-either refusing a rename outright (a behavior change, and
+reader is outside the lock by the time it reads the characters.** Documented in
-`tests/err_name_ownership.c` asserts the current contract), or copying names
+`docs/thread-safety.md` and `UPGRADING.md` as "register names during
-into a bump-allocated arena and publishing the pointer with a release store, so
+initialization".
 a rename allocates new storage instead of rewriting live storage. The arena is
 the better answer; it costs a second capacity limit and its exhaustion path.
 ## 6. Deprecate the two-argument name-registration path
 `akerr_name_for_status(status, name)` cannot identify its caller, so it can only
 check that *some* reservation covers the status, not that the caller owns it. It
 exists for migration. Once consumers have moved to
 `akerr_register_status_name()`, make the set path a no-op or remove it and leave
 `akerr_name_for_status()` as pure lookup.
 ## 7. `akerr_init()`'s own reservation failure is untested
 `tests/err_library_status_fatal.c` covers the terminal path in
 `__akerr_name_library_status()` by naming a status the library does not own. The
 band reservation in `akerr_init()` has no such handle: it can only fail in a
 build whose tables are too small for the library's own entries, and both sizes
 are `PRIVATE` to the library target, so a test executable cannot set them.
 Closing it means a second library target built with tiny tables plus a
 `WILL_FAIL` test linked against it. Nothing in the CMake does that yet: the
 sanitizer and coverage options vary the *flags* of the one library target, not
 its compile definitions.
 Related: branch coverage on `src/error.c` now sits just above its 50% gate.
 Every `FAIL_*` site carries about six branch outcomes of error-construction
 machinery (`ENSURE_ERROR_READY`, `AKERR_STACKTRACE_APPEND`) that only run when
 that specific failure fires, and every `PASS` site around a call that cannot
 fail carries about twenty-five. Validating more inputs therefore lowers the
 ratio by construction. Before adding defensive checks, expect to add a test that
 drives them, as `tests/err_copy_string.c` does.
 ## 8. Mutation testing judges concurrency mutants without a sanitizer
 `scripts/mutation_test.py` configures each mutant build with the default CMake
 options, so a mutant that only breaks under concurrency is judged by a suite
 running without ThreadSanitizer. Deleting the pool's `akerr_mutex_lock()` call
 survives the run even though it is a real race: rebuilt and run directly, that
 mutant fails `tests/err_threads_pool.c` in 4 of 10 runs, and fails under
 `scripts/thread_test.sh` in 5 of 5. So 81.2% is a floor for that category, not a
 verdict.
 Closing it means a `--cmake-arg` passthrough on the harness so the mutant build
 can be configured with `-DAKERR_SANITIZE=thread`. The whole run then costs a
 TSan-instrumented suite per mutant (roughly 6s instead of 0.4s), so it belongs
 behind a flag rather than in the default target or in CI.
 ## 9. No way to keep an error context and report it at the same time
 A context can be handed to another thread and released there --
 `docs/thread-safety.md` now documents that pattern and
 `tests/err_threads_handoff.c` proves it -- but it is a *move*. A thread that
 wants to both keep its error and report it upward has to read the fields out
 into its own record, and it loses the stack trace doing so, because
 `stacktracebuf` is the one thing that cannot be usefully summarized.
 Copying the struct is not a workaround. `stacktracebufptr` is self-referential
 (`include/akerror.tmpl.h`), so `akerr_ErrorContext c = *src;` leaves the copy's
 cursor pointing into the source's buffer -- the copy logs correctly and then
 corrupts a slot it does not own the first time anything appends to it. `arrayid`
 is restored after the wipe in `akerr_release_error()` (`src/error.c:395-398`), so
 a copied id makes the destination impersonate the source's slot for the life of
 the process.
 If this is ever worth an API, the shape is:
 ```c
 akerr_ErrorContext AKERR_NOIGNORE *akerr_copy_error(akerr_ErrorContext *source,
 						    akerr_ErrorContext *destination);
 ```
 taking the destination as a parameter rather than allocating it. An allocating
 copy could fail on pool exhaustion, and reporting *that* failure needs a pool
 slot, so it would have to abort -- adding a third `exit()` site to a library that
 deliberately has two. Caller-allocates puts the pool pressure where it can be
 managed. The copy must repair four fields: `arrayid` (the destination's own),
 `refcount` (set to 1, never inherited), `handled` (false -- a copy is a fresh
 obligation, or `FINISH_NORETURN` on the receiving side drops it silently), and
 `stacktracebufptr` (re-anchored to the destination's buffer at the *same offset*,
 so a later append continues the trace instead of overwriting it).
 Not worth building yet: no consumer needs it. The trigger is a consumer that
 needs a worker's stack *trace*, not just its status and message, at the join
 point -- `libakstdlib`'s planned `pthread_*` wrappers are the likely first.
 ## Unrelated pre-existing issues
 - The `AKERR_USE_STDLIB=OFF` build does not compile at all: `bool`, `PATH_MAX`
  and `NULL` are used unconditionally but only included under the stdlib branch.
  `docs/building.md`'s dependency list states what a replacement must provide,
  but the header still needs its includes untangled for that configuration to
  work.
 - `CMakeLists.txt` sets `main_lib_dest` from `MY_LIBRARY_VERSION`, which is never
  defined and never read. Dead line.
--- a/tests/MUTATION.md
+++ b/tests/MUTATION.md
@@ -149,9 +149,8 @@ Findings surfaced by mutation testing:
 * **Open:** the harness builds every mutant with the default CMake options, so a
  mutant that only breaks under concurrency is judged by a suite running without
  ThreadSanitizer. Mutating under `-DAKERR_SANITIZE=thread` would close that,
-  and needs a way to pass CMake options through to the mutant build. See
+  and needs a way to pass CMake options through to the mutant build. That is
-  "Mutation testing judges concurrency mutants without a sanitizer" in
+  issue #10; `TODO.md` records why the score is a floor rather than a verdict.
  `TODO.md`.
 * **Superseded:** status names now use a private sparse registry, so the old
  public `AKERR_MAX_ERR_VALUE` ceiling and its consumer ABI mismatch no longer