fix(source-control): document/export PYTHONUTF8=1 for fetch-all-pr-comments.sh consumers

[kyle-sexton]

This was generated by AI during work-loop execution.

Closes #597

Summary

fetch-all-pr-comments.sh's UTF-8 JSON output commonly carries non-ASCII bytes (bot badge
images, reaction emoji) from bot review comments. Reproduced directly on this Windows machine: a
Python consumer that opens the output, or reads this script's stdout, WITHOUT an explicit UTF-8
encoding inherits the interpreter's default ANSI code page (cp1252) and raises
UnicodeDecodeError on those bytes — confirmed with a minimal repro (open(path) + json.load,
no encoding= kwarg, PYTHONUTF8 unset).

This repo's own consumer (babysit_findings.py, invoked from babysit-readiness-gate.sh) already
pins encoding="utf-8" explicitly on every file/subprocess read and reconfigures stdout/stderr to
UTF-8 at runtime (configure_stdio()), so it is unaffected — the real gap is external/downstream
Python consumers that don't pin the encoding themselves. Two changes, matching the issue's own
"implementer's call" framing (doc it, or set it internally — both are mechanical):

1. fetch-all-pr-comments.sh --help now documents the PYTHONUTF8=1 (PEP 540) requirement for a
   Windows Python consumer that doesn't pin encoding="utf-8" itself.
2. babysit-readiness-gate.sh — the one babysit_python caller that parses this script's
   comment-JSON schema and was missing the export PYTHONUTF8=1 convention the two bin/ babysit
   wrappers (source-control-babysit-merge, source-control-babysit-resolve-thread) already
   apply — now sets it too, closing the inconsistency (belt-and-suspenders; not required by the
   explicit encoding="utf-8" calls already in place, but matches the established pattern).

Docs-only + a defense-in-depth env export; no behavior change to the JSON schema or exit codes.

Test plan

• Reproduced the reported failure directly: open(path) (no encoding=) + json.load on an
  emoji-laden fixture with PYTHONUTF8 unset on this Windows machine raises
  UnicodeDecodeError: 'charmap' codec can't decode byte 0x8d ... — confirms the cp1252 root
  cause described in source-control:babysit-prs: fetch-all-pr-comments.sh output needs PYTHONUTF8=1 to parse on Windows (emoji/cp1252 mismatch) #597.
• Confirmed the in-repo Python path (babysit_findings.py via babysit-readiness-gate.sh) is
  NOT affected either way (it already pins encoding="utf-8"), by running the same emoji fixture
  through it with PYTHONUTF8 explicitly unset: findings=2 classified=1, no crash.
• bash plugins/source-control/scripts/fetch-all-pr-comments.sh --help — new "Windows Python
  consumers" section renders correctly; usage()'s sed range updated to include it.
• bash plugins/source-control/scripts/fetch-all-pr-comments.test.sh — 19/19 pass (17 pre-existing
  • 2 new: --help documents PYTHONUTF8=1 and UnicodeDecodeError).
• bash plugins/source-control/scripts/babysit-readiness-gate.test.sh — 65/65 pass (63
  pre-existing + 2 new: source pins export PYTHONUTF8=1, and a behavioral test with a stubbed
  py -3 interpreter proving the exported value actually reaches the Python child process, not
  just present as dead source text).
• shellcheck --rcfile .shellcheckrc on all four changed shell files — clean.
• typos, editorconfig-checker on all changed files — clean.
• markdownlint-cli2 on CHANGELOG.md — Summary: 0 issues in 0 files.
• bash scripts/validate-plugins.sh — all plugin manifests and the catalog validated (version
  bump 0.26.2 → 0.26.3 checked).

Related

Refs #568 — background: the bash fetch-all-pr-comments.sh <N> > file.json stdout-capture pattern
this issue's Windows repro builds on.

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 4m 8s —— View job

---

PR Review

• Gather context and understand the request
• Read changed files
• Review fetch-all-pr-comments.sh changes
• Review babysit-readiness-gate.sh changes
• Review test files
• Review plugin.json and CHANGELOG.md
• Post comprehensive review feedback

---

Summary: Docs-only + defense-in-depth export PYTHONUTF8=1 for Windows Python consumers. The change is minimal, well-reasoned, and backed by solid tests. No behavior change to JSON schema or exit codes.

---

fetch-all-pr-comments.sh

sed -n '2,27p' range update — correct. The 9 new comment lines are inserted at lines 20–28 (the Windows consumers block), shifting Exit codes from line 20 to line 29. Range 2,27p captures through line 27 (the last sentence of the new section) and stops before the empty # divider at line 28 and # Exit codes: at line 29 — exactly parallel to how the original 2,18p stopped before the empty # at line 19. Consistent behaviour across the change.

Documentation quality — the new --help section is accurate and actionable. It names the failure mode (UnicodeDecodeError), the root cause (cp1252 ANSI page), and both remedies (encoding="utf-8" or PYTHONUTF8=1). The issue reference (#597) helps future readers trace context.

---

babysit-readiness-gate.sh

export PYTHONUTF8=1 placement — appropriate. Placed immediately after set -uo pipefail and before any variable assignments or subshell work, so it's set before babysit_python is ever invoked (line ~301). babysit_python ultimately does exec py -3 "$@", which replaces the shell process — the inherited environment at that point includes this export.

The inline comment (lines 68–76) is unusually long for a single export, but the reasoning genuinely IS non-obvious (the Windows encoding trap isn't obvious to a reader not familiar with cp1252 / PEP 540), and the comment cross-references the two other callers that already apply this convention, which is valuable for the "why are we doing this here too?" question. Acceptable given the file's existing documentation density.

---

babysit-readiness-gate.test.sh

Two new tests — one static, one behavioral:

Static source check (line 217):

if [[ "$GATE_BODY" == *'export PYTHONUTF8=1'* ]]; then

Tests the source text via GATE_BODY=$(cat "$GATE") (line 202). This is the same pattern as the existing grep -owE portability check at line 203, so it's consistent with the file's established pattern. It functions as a regression guard — if the export is accidentally removed, this check fails. Not a substitute for the behavioral test below, but a useful complement.

Behavioral stub test (lines 299–323):

This is the stronger test. The design is clever and correct:

1. A py stub is created in $PYSTUB_BIN, prepended to PATH.
2. babysit-python.sh's babysit_python() always probes py -3 first; the stub returns exit 0 for any -c argument (version probe), so babysit_python always selects exec py -3 "$@" — the stub — regardless of what other Python interpreters exist on the system.
3. For a non-probe invocation, the stub writes ${PYTHONUTF8:-unset} to $PYUTF8_PROBE_FILE, then prints findings=0 classified=0 (valid gate output).
4. PYUTF8_PROBE_FILE is exported before bash "$GATE" runs, so the subshell and its eventual exec'd stub inherit it.
5. The assertion confirms the value is "1", proving the exported env var actually propagated through babysit_python's exec.
6. unset PYUTF8_PROBE_FILE cleans up to avoid polluting subsequent tests.

The skip guard (probe_py py -3 || probe_py python3 || probe_py python) runs on the real system Python before the stub exists — correct. On a system with only python3, the stub py is still the intercept point because it's first in PATH and passes the version probe, so babysit_python always selects it. No false positives.

---

fetch-all-pr-comments.test.sh

Two simple assertions (lines 232–234):

assert_contains "--help documents PYTHONUTF8=1 for Windows consumers" "$help_out" "PYTHONUTF8=1"
assert_contains "--help names the UnicodeDecodeError failure mode" "$help_out" "UnicodeDecodeError"

Straightforward and correct. They're pinned against the actual --help output at runtime, so if the sed range drifts and drops the Windows section, these fail.

---

plugin.json / CHANGELOG.md

Version bump 0.26.2 → 0.26.3 is appropriate for a patch-level defense-in-depth + docs change. Changelog entry is accurate and references #597.

---

Verdict

Approve. The changes are minimal, correctly scoped, and well-tested. The behavioral stub test is particularly well-designed — it proves the env var actually reaches the child process rather than relying solely on the static source check. No bugs, no security concerns, no architectural issues.

[claude]

Claude finished @kyle-sexton's task in 1m 55s —— View job

---

Security Review

• Gather context and understand the request
• Review PR diff for security issues
• Read changed shell scripts for injection surfaces and unsafe patterns
• Report findings

---

No security vulnerabilities found.

I reviewed all six changed files for injection surfaces, unsafe env-var handling, privilege escalation, credential exposure, and dangerous shell patterns. Here is what I examined and why each surface is clean:

babysit-readiness-gate.sh — export PYTHONUTF8=1
Hard-coded integer literal. No user input reaches this assignment; it propagates to child processes by design (PEP 540 UTF-8 mode). No injection surface.

fetch-all-pr-comments.sh — sed -n '2,27p'
The --help range expanded from line 18 to line 27. The range operands are numeric literals; the file being read is the script itself via ${BASH_SOURCE[0]}. No user input touches either operand and no shell expansion occurs inside sed's stream. Clean.

babysit-readiness-gate.test.sh — stub interpreter + PATH prepend
The behavioral test (lines 299–323) places a fake py script in $TEST_TMPDIR/bin-pystub/ — an mktemp -d directory cleaned up on EXIT. Three things worth checking:

• PYUTF8_PROBE_FILE write target: The stub writes ${PYTHONUTF8:-unset} to this file. The variable is assigned to $TEST_TMPDIR/pyutf8-probe.txt inside the test before export, so any pre-existing inherited value is overwritten before the stub sees it. The write destination is always within the secure temp directory.
• PATH scope: PATH="$PYSTUB_BIN:$PATH" bash "$GATE" … is a single-command prefix, not a shell-wide export. The modified PATH does not survive past that invocation.
• Stub contents: The heredoc delimiter is quoted (<<'STUB'), preventing any expansion during the write. The stub only writes the numeric env-var value and a fixed status line. No eval, no dynamic command construction.

babysit-readiness-gate.test.sh — static source grep
[[ "$GATE_BODY" == *'export PYTHONUTF8=1'* ]] reads the gate script from a resolved absolute path ($SCRIPT_DIR/babysit-readiness-gate.sh). Pattern is a fixed glob literal. No injection risk.

fetch-all-pr-comments.test.sh — --help content assertions
Both new tests call bash "$SCRIPT" --help with no arguments and assert the output contains fixed strings. No user-controlled input anywhere.

plugin.json / CHANGELOG.md
Version bump and changelog entry. No security surface.

[claude]

Claude finished @kyle-sexton's task in 2m 57s —— View job

---

PR Review

• Gather context and understand the request
• Read changed files
• Review fetch-all-pr-comments.sh changes
• Review babysit-readiness-gate.sh changes
• Review test files
• Review plugin.json and CHANGELOG.md
• Post comprehensive review feedback

---

Summary: Docs-only addition + defense-in-depth export PYTHONUTF8=1 for Windows Python consumers of fetch-all-pr-comments.sh. Minimal scope, well-tested. No behavior change to JSON schema or exit codes.

---

fetch-all-pr-comments.sh — sed range

sed -n '2,27p' is correct. From the file:

line 27  #   the consumer with `PYTHONUTF8=1` (PEP 540) set in the environment.
line 28  #
line 29  # Exit codes:

The range captures through line 27 (last content line of the new Windows section) and stops before the blank separator at line 28 and # Exit codes: at line 29. This is exactly parallel to the original 2,18p, which ended before the blank separator preceding # Exit codes: in the old layout. Consistent.

The new section is accurate and actionable: it names the failure mode (UnicodeDecodeError), root cause (cp1252 ANSI page), and both remedies (encoding="utf-8" or PYTHONUTF8=1).

---

babysit-readiness-gate.sh — export PYTHONUTF8=1

Line 85 — correct placement. It's set immediately after set -uo pipefail and before SCRIPT_DIR= or any subshell work, so the export is in the inherited environment when babysit_python eventually does exec py -3 "$@".

The 9-line comment block is dense for a single export, but the reasoning — Windows cp1252 / PEP 540 interaction and cross-reference to the two other callers that already apply this convention — is genuinely non-obvious. Proportionate to the file's existing documentation density.

---

babysit-readiness-gate.test.sh — behavioral stub test

Lines 295–323 — design is correct and precise.

The stub distinguishes probe from real invocation correctly:

for a in "${args[@]}"; do
  if [[ "$a" == "-c" ]]; then
    exit 0  # version probe (babysit_python's py -3 -c '...' check)
  fi
done
printf '%s' "${PYTHONUTF8:-unset}" >"$PYUTF8_PROBE_FILE"
printf 'findings=0 classified=0\n'

• Probe path: babysit_python calls py -3 -c '...' — stub sees -c, exits 0. Probe succeeds, py selected as interpreter.
• Real invocation: py -3 babysit_findings.py --comments-json <file> ... — no -c arg, stub writes the inherited $PYTHONUTF8 to the probe file and returns valid gate output.

The stub intercepts even on Linux (where py is usually absent) because $PYSTUB_BIN is prepended to PATH before bash "$GATE" runs. babysit_python's own discovery loop tries py -3 first and finds the stub.

Skip guard: probe_py py -3 || probe_py python3 || probe_py python runs before stub creation against the real system Python. This is correct: it gates the whole test on whether the gate would actually take the Python path (Python 3.11+ required). Without this guard, on Python-free environments the gate would fall back to its bash degraded path, and there would be no babysit_python exec to intercept.

PYUTF8_PROBE_FILE is exported before bash "$GATE" runs, so the gate's subshell and its eventual exec'd stub inherit it. The unset PYUTF8_PROBE_FILE cleanup at line 320 is good hygiene.

One thing to be aware of: if babysit_python has a real invocation path that also passes -c (not just version probing), the stub would misclassify it as a probe and exit 0 without writing the probe file. Based on the PR description, babysit_python only uses -c for the version probe, so this is safe. If babysit_python ever gains a -c non-probe invocation, this stub would need updating.

---

babysit-readiness-gate.test.sh — static source check

Line 217 — correct and consistent with the existing grep -owE portability check at line 203. Functions as a regression guard: if the export is accidentally removed, this fails independently of the behavioral test.

---

fetch-all-pr-comments.test.sh

Lines 234–236 — straightforward. Both assertions pin against the live --help output at runtime, so a drifting sed range that drops the Windows section causes these to fail. Correct.

---

plugin.json / CHANGELOG.md

Version bump 0.26.7 → 0.26.8 is consistent between both files. The PR description mentions 0.26.2 → 0.26.3 — that's stale text from when the PR was originally authored and has since rebased over several releases. The files themselves are correct.

Patch-level bump is appropriate: docs + defense-in-depth env export, no schema or exit-code change.

---

Verdict

Approve. The change is minimal, correctly scoped, and backed by solid tests. The behavioral stub test is the standout — it proves env var propagation through exec rather than relying solely on the static source check. No bugs, no security concerns, no CLAUDE.md violations.

[claude]

Claude finished @kyle-sexton's task in 1m 52s —— View job

---

Security Review

• Gather context and understand the request
• Get PR diff
• Read changed shell scripts for security issues
• Report findings

---

No security vulnerabilities found.

I reviewed all six changed files in this PR for injection surfaces, unsafe env-var handling, credential exposure, dangerous shell patterns, and PATH manipulation risks. Here is what I examined and why each surface is clean:

---

babysit-readiness-gate.sh — export PYTHONUTF8=1

Hardcoded integer literal. No user input reaches this assignment; it propagates to child processes by design (PEP 540 UTF-8 mode). No injection surface.

---

fetch-all-pr-comments.sh — sed -n '2,27p' "${BASH_SOURCE[0]}"

Range operands 2,27 are numeric literals. ${BASH_SOURCE[0]} is a Bash built-in that resolves to the script's own path — not user-controlled. The second sed 's/^# \{0,1\}//' pipe processes comment-stripped lines from the script itself. No user input touches either operand; no shell expansion occurs inside sed's stream. Clean.

---

babysit-readiness-gate.test.sh — static source check (line 217)

GATE_BODY=$(cat "$GATE") reads the gate script from a resolved absolute path. [[ "$GATE_BODY" == *'export PYTHONUTF8=1'* ]] is a fixed-pattern string comparison — no code execution. No injection risk.

---

babysit-readiness-gate.test.sh — behavioral stub test (lines 295–323)

Three surfaces worth examining:

• Temp directory: TEST_TMPDIR="$(mktemp -d)" at line 9, cleaned up with trap 'rm -rf "$TEST_TMPDIR"' EXIT. mktemp -d produces a directory with mode 700 and a random suffix — no predictable path an attacker could pre-create. All stub artifacts (PYSTUB_BIN, PYUTF8_PROBE_FILE) live under this directory.

• Stub write: cat >"$PYSTUB_BIN/py" <<'STUB' uses a single-quoted heredoc delimiter ('STUB'), which prevents any shell expansion during the write. The stub body is a fixed shell script — no dynamic content is injected at write time.

• PATH prepend: PATH="$PYSTUB_BIN:$PATH" bash "$GATE" ... is a single-command environment prefix, not a shell-wide export. The modified PATH does not survive past that one invocation. $PYSTUB_BIN resolves to an absolute path under the secure mktemp temp dir.

• Probe file write in stub: The stub executes printf '%s' "${PYTHONUTF8:-unset}" >"$PYUTF8_PROBE_FILE". $PYUTF8_PROBE_FILE is inherited from the test's exported env (set to $TEST_TMPDIR/pyutf8-probe.txt before the subshell runs). The written value is the env var's runtime value (1 or unset) — not user-controlled input.

All clean.

---

fetch-all-pr-comments.test.sh — --help content assertions (lines 232–234)

Both tests call bash "$SCRIPT" --help with no arguments and assert the output contains fixed strings. No user-controlled input anywhere in the pipeline.

---

plugin.json / CHANGELOG.md

Version bump and changelog entry. No security surface.