Close the value.c mutation survivors, and correct one that cannot be

Two new tests in value_arithmetic.c, each verified by hand-applying the mutant
rather than by assuming. The obvious maximum-length-string test does not work:
math_plus clones self into the scratch first, so joining a full string to an
empty one leaves the byte a short copy missed already correct. The operands
have to sum to the limit without either being the answer.

One of the five listed survivors is equivalent and cannot be killed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

TODO.md

@@ -1095,18 +1095,33 @@ What remains, in priority order:
    - **Item 17** is ours: a host cannot reliably create a script variable while a script is
      suspended. The fix is `akbasic_runtime_global()` and roughly fifteen lines. §6 describes
      it and names the one design question to settle first.
-6. **Mutation survivors in `src/value.c`.** The harness is in place (`scripts/mutation_test.py`,
+6. **Mutation survivors in `src/value.c`** — ~~closed~~, with one correction to what was
-   the `mutation` CMake target, and a CI job on `src/symtab.c`), and a partial run over
+   written here before. `tests/value_arithmetic.c` grew two functions,
-   `src/value.c` turned up test gaps worth closing. These are *not* equivalent mutants; each
+   `test_maximum_length_string()` and `test_pool_starts_empty()`, and each was verified by
-   is a real bug of that shape the suite would not notice:
+   hand-applying the mutant and watching the test fail rather than by assuming it would:
 
-   - `AKBASIC_MAX_STRING_LENGTH - 1` → `+ 1` and → `- 0` survive at `src/value.c:47-48`, in
+   | Mutant | Result |
-     `set_string`'s `strncpy` and its NUL terminator. Nothing in the suite writes a
+   |---|---|
-     *maximum-length* string, so the off-by-one that would truncate or overrun goes unseen.
+   | `strncpy(dest->stringval, src, AKBASIC_MAX_STRING_LENGTH - 1)` → `- 2` | killed |
-     One test that round-trips a 255-character string kills all five.
+   | `dest->stringval[AKBASIC_MAX_STRING_LENGTH - 1] = '\0'` → `- 2` | killed |
-   - `memset(obj, 0, ...)` → `memset(obj, 1, ...)` and its deletion survive in
+   | `strncpy(..., AKBASIC_MAX_STRING_LENGTH - 1)` → `- 0` | **equivalent — cannot be killed** |
-     `akbasic_valuepool_init`, as does `obj->next = 0` → `= 1`. Nothing asserts a freshly
+   | `memset(obj, 0, sizeof(*obj))` deleted from `akbasic_valuepool_init` | killed |
-     initialised pool is actually empty and zeroed.
+   | `obj->next = 0` → `= 1` | killed |
+
+   **The correction.** This entry used to say all five were real bugs and that "one test that
+   round-trips a 255-character string kills all five". Two things about that were wrong.
+
+   The `- 0` mutant is *equivalent*. `strncpy(dest, src, 256)` into a 256-byte buffer writes at
+   most 256 bytes, and `set_string()` has already refused anything longer than 255 two lines
+   above, so the extra byte is always the terminator `strncpy` would have written anyway. It
+   cannot be killed and should not be counted against the score.
+
+   And the obvious test does *not* kill the truncating mutant. Joining a full-length string to
+   an empty one passes with the mutant in place, because `akbasic_value_math_plus` clones
+   `self` into the scratch before writing and the byte a short copy fails to write is already
+   the right one. The operands have to sum to the limit **without either of them being the
+   answer** — the test uses 200 `X` plus 55 `Z`, and says so at the site, because this is
+   exactly the sort of thing that gets "simplified" back into a passing no-op.
 
    Two survivors there are genuinely equivalent and cannot be killed: `rval_as_int` and
    `rval_as_float` (`src/value.c:30,35`) tolerate `+` → `-` because the reference's habit of

tests/value_arithmetic.c

@@ -39,6 +39,113 @@ static void set_string(akbasic_Value *v, const char *s)
     strncpy(v->stringval, s, AKBASIC_MAX_STRING_LENGTH - 1);
 }
 
+/**
+ * @brief A string that exactly fills the value's inline buffer round-trips.
+ *
+ * Written because mutation testing said nothing would have noticed if it did
+ * not: `AKBASIC_MAX_STRING_LENGTH - 1` mutated to `+ 1` and to `- 0` survived at
+ * both the strncpy and the NUL terminator in set_string(), and no test in the
+ * suite wrote a maximum-length string. Both mutants are a real bug of that shape
+ * -- one truncates a string that fits, the other writes one byte past the end of
+ * a 256-byte buffer -- and the boundary is the only place either shows.
+ *
+ * Concatenation rather than a direct set, because set_string() is static and
+ * concatenation is how every string in a BASIC program actually gets built.
+ *
+ * **The two operands are deliberately different lengths and different letters.**
+ * A first attempt joined a full-length string to an empty one, and the
+ * truncating mutant survived it: math_plus clones self into the scratch before
+ * writing, so the byte a short copy failed to write was already the right one.
+ * The parts have to sum to the limit without either of them being the answer.
+ */
+static void test_maximum_length_string(void)
+{
+    enum { LIMIT = AKBASIC_MAX_STRING_LENGTH - 1, HEAD = LIMIT - 55, TAIL = 55 };
+    akbasic_Value *out = NULL;
+    char head[AKBASIC_MAX_STRING_LENGTH];
+    char tail[AKBASIC_MAX_STRING_LENGTH];
+    char joined[AKBASIC_MAX_STRING_LENGTH];
+
+    memset(head, 'X', HEAD);
+    head[HEAD] = '\0';
+    memset(tail, 'Z', TAIL);
+    tail[TAIL] = '\0';
+    /* memcpy rather than snprintf: the lengths are known here and -Wformat-truncation
+     * cannot see that HEAD + TAIL is exactly the buffer's capacity. */
+    memcpy(joined, head, HEAD);
+    memcpy(joined + HEAD, tail, TAIL);
+    joined[HEAD + TAIL] = '\0';
+
+    /* Exactly the limit: every byte lands, and the terminator is where it should be. */
+    set_string(&A, head);
+    set_string(&B, tail);
+    TEST_REQUIRE_OK(akbasic_value_math_plus(&A, &B, &SCRATCH, &out));
+    TEST_REQUIRE_INT(strlen(out->stringval), LIMIT);
+    TEST_REQUIRE_STR(out->stringval, joined);
+    TEST_REQUIRE_INT(out->stringval[LIMIT - 1], 'Z');
+    TEST_REQUIRE_INT(out->stringval[LIMIT], '\0');
+
+    /* One character past it is an error, not a silent truncation. */
+    set_string(&A, joined);
+    set_string(&B, "!");
+    TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, &B, &SCRATCH, &out), AKBASIC_ERR_VALUE);
+
+    /* The same boundary through the repeat operator, which has its own copy. */
+    memset(head, 'Q', LIMIT / 2);
+    head[LIMIT / 2] = '\0';
+    set_string(&A, head);
+    set_int(&B, 2);
+    TEST_REQUIRE_OK(akbasic_value_math_multiply(&A, &B, &SCRATCH, &out));
+    TEST_REQUIRE_INT(strlen(out->stringval), (LIMIT / 2) * 2);
+    TEST_REQUIRE_INT(out->stringval[((LIMIT / 2) * 2) - 1], 'Q');
+}
+
+/**
+ * @brief A freshly initialised value pool is empty, and its storage is zeroed.
+ *
+ * Also a mutation-driven test: `memset(obj, 0, ...)` mutated to `memset(obj, 1,
+ * ...)`, the memset deleted outright, and `obj->next = 0` mutated to `= 1` all
+ * survived, because nothing asserted what an initialised pool looks like. The
+ * deleted-memset mutant is the one that matters -- a pool over stale stack
+ * memory hands a BASIC program somebody else's array contents.
+ */
+static void test_pool_starts_empty(void)
+{
+    static akbasic_ValuePool pool;
+    akbasic_Value *slice = NULL;
+    int i = 0;
+
+    /* Dirty every byte first, so a missing memset cannot pass by luck. */
+    memset(&pool, 0xAB, sizeof(pool));
+    TEST_REQUIRE_OK(akbasic_valuepool_init(&pool));
+
+    TEST_REQUIRE_INT(pool.next, 0);
+    for ( i = 0; i < AKBASIC_MAX_ARRAY_VALUES; i++ ) {
+	if ( pool.values[i].valuetype != 0 || pool.values[i].intval != 0 ||
+	     pool.values[i].stringval[0] != '\0' ) {
+	    TEST_REQUIRE(false, "pool slot %d was not zeroed by init", i);
+	    break;
+	}
+    }
+
+    /* The first take starts at slot zero and the bump advances by exactly count. */
+    TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 4, &slice));
+    TEST_REQUIRE(slice == &pool.values[0], "the first take must start at slot zero");
+    TEST_REQUIRE_INT(pool.next, 4);
+    TEST_REQUIRE_OK(akbasic_valuepool_take(&pool, 1, &slice));
+    TEST_REQUIRE(slice == &pool.values[4], "the second take must start where the first ended");
+    TEST_REQUIRE_INT(pool.next, 5);
+
+    /* Re-initialising takes it back to empty rather than merely rewinding. */
+    TEST_REQUIRE_OK(akbasic_valuepool_init(&pool));
+    TEST_REQUIRE_INT(pool.next, 0);
+
+    TEST_REQUIRE_STATUS(akbasic_valuepool_init(NULL), AKERR_NULLPOINTER);
+    TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, 0, &slice), AKBASIC_ERR_BOUNDS);
+    TEST_REQUIRE_STATUS(akbasic_valuepool_take(&pool, AKBASIC_MAX_ARRAY_VALUES + 1, &slice),
+			AKBASIC_ERR_BOUNDS);
+}
+
 int main(void)
 {
     akbasic_Value *out = NULL;
@@ -122,5 +229,8 @@ int main(void)
     /* nil rval is rejected everywhere. */
     TEST_REQUIRE_STATUS(akbasic_value_math_plus(&A, NULL, &SCRATCH, &out), AKERR_NULLPOINTER);
 
+    test_maximum_length_string();
+    test_pool_starts_empty();
+
     return akbasic_test_failures;
 }