docs(testing): correct issue 261 diagnosis and add upstream repro

[valthon]

Summary

• Correct the framework/testing guidance: the reproduced Zig 0.16.0 successful-child stderr newline produces a stale failed-command label, not an app-boot race. Preserve the supported zigbase.addTest setup.
• Keep one dependency-free Zig+C reproduction; remove the redundant ZigBase consumer package and load-matrix script. Preserve historical measurements in the upstream issue draft.
• Give the candidate compiler patch three-line context and an explicit completed-tests, exit-zero, successful-results guard. Preserve real assertion and post-test destructor-signal diagnostics.
• Add an eight-case stock/private-patched-library regression driver to CI. This does not modify the installed compiler or file an upstream issue.

Refs #261.

Verification

Rebased onto current main; one coherent commit, 64c1680. Clean detached-checkout gate passed:

• Eight stock/patched cases: whitespace-only success, meaningful-stderr success, assertion failure, and passing test followed by destructor SIGSEGV.
• 185 Python tests and 13 subtests, including documentation parity.
• Ruff, Zig formatting, generated skill-reference synchronization, and changelog assembly checks.
• Site build, doctor, static and generated tests.
• Both compiler patches apply against pinned Zig 0.16.0; installed compiler remains unchanged.

The historical 160-run native/baseline load matrix is retained as dated investigation evidence, not claimed as rerun during this cleanup.

[valthon]

Review (draft): issue 261 diagnosis and upstream repro

Verdict: the diagnosis is right, but the branch can't merge as-is. The changelog fragment uses a section the assembler rejects, the branch conflicts with main in docs/framework.md (which has since moved to zigbase.addTest), and the candidate Zig patch is zero-context so its safety-critical placement can't be reviewed.

Reviewed against the merge-base. The technical finding holds up: facil.io's fio_lib_destroy destructor unconditionally does fprintf(stderr, "\n") when fio_is_master(), and Zig's evalZigTest .no_poll arm assigns result_stderr = stderr_owned on a clean exit while result_failed_command (set in spawnChildAndCollect) is never cleared on that path, and build_runner.printErrorMessages prints failed command: whenever result_failed_command != null. A passing child that emits one newline at exit produces exactly the observed block. Demoting "race" / "CPU crash" to "cosmetic diagnostic" is correct. No edits to docs/superpowers/ or generated mirrors; run-matrix.sh passes bash -n, only kills its own yes workers, uses mktemp -d, and touches no network beyond the ordinary zig build dependency fetch. CLAUDE.md:29 and tests/admin/test_init.py:211 remain consistent with the new diagnosis.

Findings

1. blocker — ### Documentation is not a recognized changelog section and fails the release build (changelog.d/issue-261-diagnostics.md:1). scripts/assemble-changelog.sh:37 defines SECTIONS=(Breaking Features Fixes Changed Performance Deprecated Removed Security Internal) and exits 1 on any other heading. Split it: the consumer-visible docs correction under ### Changed (or ### Fixes), the reproduction/patch tooling under ### Internal.

2. major — the branch is stale and conflicts with main; it re-introduces guidance main deliberately retired and leaves the old "race" explanation live in two places the PR doesn't edit (docs/framework.md:4642-4648 on the branch vs docs/framework.md:4871-4898 on main; docs/testing.md:19-25). git merge-tree origin/main <branch> reports a content conflict in docs/framework.md. Main's section now says "addTest wires a .simple-mode test runner that ZigBase ships … It sidesteps an upstream Zig 0.16 build-runner race … the runner can mis-read that normal exit as a crash", and CHANGELOG.md:83 records that addTest replaced "the previous advice to copy Zig's own test runner", yet the branch's snippet still says "copy Zig 0.16's lib/compiler/test_runner.zig into your project". docs/testing.md:21-24 (added on main after the merge-base) repeats the crash wording. Rebase, rewrite the two addTest bullets and the docs/testing.md paragraph with the newline diagnosis, and drop the copy-the-runner snippet in favor of zigbase.addTest.

3. major — the candidate Zig patch is -U0, so whether the result_failed_command clear is guarded by the clean-exit condition can't be verified (diagnostics/issue-261/zig-runner-diagnostic-fix.patch:10-12). The hunk is @@ -1754,0 +1758,2 @@ with two added lines and no context. In upstream evalZigTest the .no_poll arm is shared between success and failure: after result_stderr = stderr_owned comes if (!tests_done or !termMatches(.{ .Exited = 0 }, term)) addError("test process unexpectedly …"). A child that passes all tests and then dies in an exit-time destructor (SIGSEGV after tests_done) takes this same arm, fails the step, and, if the clear is unconditional, loses the command line that build_runner.zig prints only when result_failed_command != null. The README's verification list covers an in-test failure, not this post-tests_done unclean exit. Regenerate with -U3, place the free/clear in an explicit success branch (tests_done and termMatches(.Exited = 0)), and add the "passes, then signal at exit" case to the verified list. The whitespace-only discard itself is safe, and options.gpa matches the Step.allocPrintCmd(options.gpa, …) allocation.

4. minor — a fourth standalone consumer package that CI never builds will rot (diagnostics/issue-261/build.zig:13, src/repro.zig:16-18). CI builds examples/blog|golfsim|plugins only; nothing references diagnostics/. The repro pins zigbase.testing.start(App, .{}), harness.request(.GET, …), zigbase.http.Response, and an extern fn fio_is_master(). The durable, dependency-free artifact is upstream-minimal/ + upstream-issue-draft.md + the patches. Keep only those (or move the whole tree into the upstream issue or a gist and link it from the docs), or add a CI step that at least compiles the package if it stays.

5. nit — the results table is a point-in-time host measurement sitting next to the durable reproducer (diagnostics/issue-261/README.md:24-37). Move the matrix into the upstream issue draft, where it's evidence, and keep the README to the mechanism and how to run.

Positives

The facil.io claim is exact (fio.c fio_lib_destroy → if (add_eol) fprintf(stderr, "\n");), and the standalone Zig+C reproduction removes every zigbase-specific variable from the argument, which is what makes the upstream report credible.

---

Generated by Claude Code

[valthon]

Addressed all five review-body findings in 64c1680 and rebased onto main. Changelog sections are valid; addTest guidance is preserved; the maintained reproduction is dependency-free; historical measurements moved to the upstream draft; the contextual candidate patch now requires completed successful tests and a clean child exit. The private-library regression preserves both assertion failures and post-test destructor SIGSEGV diagnostics. Clean detached gate passed all eight compiler cases, 185 Python tests plus 13 subtests, and docs/site checks. No installed compiler changes or upstream issue publication.

[valthon]

PR355 Claude review audit

Verified remote/current head 64c1680; OPEN, draft, CI passed.
Claude's formal review has no Claude review: prefix but is included in this audit.

1. Invalid changelog heading: addressed. Fragment uses Changed and Internal.
2. Stale branch/race explanation: addressed. Branch merge-base is current main 85baf31; docs/framework.md and docs/testing.md retain zigbase.addTest and describe the successful-test newline diagnostic, not a race/crash.
3. Zero-context candidate patch: addressed. Patch includes context and explicitly requires tests_done, clean exit and successful test results before clearing diagnostics. Destructor-signal case is included.
4. Unbuilt consumer fixture: addressed. Tracked standalone consumer package removed; only dependency-free upstream reproduction remains, with CI invoking verify.py.
5. Host measurements placement: addressed. README describes mechanism and reproduction; historical measurements are in upstream-issue-draft.md.

Fresh verification: mise exec zig@0.16.0 python@3.13 -- python diagnostics/issue-261/verify.py exited0, all8 stock/patched cases behaved as expected. Both assertion and post-test destructor signal retain failure commands; successful cases remove only stale labels with candidate patch. No code change needed. Preserve draft; no upstream issue filed.

[Copilot]

Copilot review overview

🟡 Changes recommended

Address the duplicate reproduction and stderr-buffer leak identified in review.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Free owned stderr before discarding whitespace-only output · New

What changed in this PR

This PR corrects the Zig 0.16.0 test-runner diagnosis and adds dependency-free reproduction, compiler-patch diagnostics, and CI regression coverage.

Changes:

• Updates testing and framework documentation.
• Adds Zig/C reproduction and verification tooling.
• Preserves investigation evidence and integrates regression tests into CI.

File	Summary
skills/​zigbase-app-genesis/​references/​testing.md	Updates testing guidance.
docs/​testing.md	Corrects the runner diagnosis.
docs/​framework.md	Clarifies supported test wiring.
diagnostics/​issue-261/​zig-runner-instrumentation.patch	Adds optional runner instrumentation.
diagnostics/​issue-261/​zig-runner-diagnostic-fix.patch	Candidate compiler fix; moderate issue (2 votes): the whitespace-only stderr path leaks the owned buffer.
diagnostics/​issue-261/​verify.py	Verifies stock and patched behavior.
diagnostics/​issue-261/​upstream-minimal/​test.zig	Defines the reproduction test.
diagnostics/​issue-261/​upstream-minimal/​newline_destructor.c	Generates stderr and signal cases.
diagnostics/​issue-261/​upstream-minimal/​build.zig.zon	Defines package metadata; moderate issue (1 vote): duplicates the existing dependency-free reproduction.
diagnostics/​issue-261/​upstream-minimal/​build.zig	Builds reproduction variants.
diagnostics/​issue-261/​upstream-issue-draft.md	Preserves investigation evidence.
diagnostics/​issue-261/​README.md	Documents reproduction and verification.
changelog.d/​issue-261-diagnostics.md	Records the documentation and diagnostic changes.
.github/​workflows/​ci.yml	Runs the new regression in CI.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.