Twinned from https://github.com/cfsh/ninefold/issues/703 by tools/beads/import_github.py's reconcile pass.
Suggestions from ninefold-reviewer's review of #699. All three are residuals from the CONVERGED verdict at 18a3480 — advisory, none blocking, none reachable with any branch name this repo has ever had. Filed unclaimed.
Context: #699's C1 fix routed every value in manifest.json through json.dumps, because S4 introduced the first free-form string the file ever emits (run.tree, a filesystem path). These are the edges that fix left.
in_build() decodes what rows() no longer encodes that waytools/playtest/manifest.py
rows() now emits json.dumps(branch). in_build() reads those same rows back with re.search(r'"branch": "([^"]*)"', row) — a raw-text regex, unchanged by the fix.
They agree for every real ref, since json.dumps("feat/x") is "feat/x". They diverge for the two characters json.dumps escapes that a git ref name may legally hold:
ensure_ascii replaces every non-ASCII character with a six-character escape: a backslash, a u, and four hex digits. A branch named feat/naïve therefore reaches the row with that escape standing in for the accented letter, and the regex captures the escaped text rather than the branch name.git check-ref-format forbids backslash, space, ~^:?*[ and control characters, but not "); "weird\"quote" makes the regex stop at the escaped quote and capture weird\.Either way branch in in_build fails inside excluded_rows, so a branch git says is in the build could appear in branches and excluded simultaneously — the self-contradictory document excluded_rows' own docstring rules out.
Fix: parse the row instead of matching it — json.loads(row)["branch"] — or compare against json.dumps(branch).
tools/playtest/manifest.py, in manifest():
```python f' "built": "{built_stamp()}",', f' "branch": "{branch}",', f' "f5_sha": "{head_sha}",', f' "base_sha": "{base_sha}",', ```
Safe by exactly the accident #699's own commit message declared insufficient for rows() — *"safe by ACCIDENT, not design … so it is routed through too"*. built is generated, the two shas are hex, and branch is git branch --show-current, which is f5/integration in practice — but a " is legal in a ref name, and the reason the rows() pair was converted was consistency rather than a live exploit.
Consistency-only, and cheap: four json.dumps calls, byte-identical output.
tools/tests/test_manifest.py
ManifestEncodingTest was inserted mid-class, so ProvenanceReadTest's last three methods ended up as members of it:
test_the_stamp_is_found_when_something_lands_ON_TOP_of_ittest_an_unparseable_or_absent_stamp_degrades_to_nothingtest_a_failed_git_log_degrades_rather_than_raising⚠ No coverage is lost — all 31 methods still run, which is why the count checks out. But they sit under a class docstring about encoding that says nothing about them, and the next encoding test added at the end of the class lands after three unrelated ones. Move the new class below ProvenanceReadTest.
None.
No comments.