fix(guardrails): diff-scope cli-flag-verify to the tool payload

[kyle-sexton]

Closes #543
Closes #544 (duplicate pair — same defect, fuller repro on #544)

The PostToolUse cli-flag-verify check re-read the entire edited file from disk, so any edit to a file already containing an unrecognized flag elsewhere (dotnet test --project in prose, 43 lines from the edit in #544's repro) re-fired the advisory about lines the edit never touched. The hook now scans the tool payload — an Edit's new_string hunk, a Write's full content (a PostToolUse Write payload cannot distinguish new-file from overwrite, so whole-content is the closest the payload allows) — landing the hook-precision convention's diff-scoping rule (rule 1) on its first member.

Repro-first per the convention: converted suite vs unmodified hook = PASS=39 FAIL=2 (exactly the two new diff-scope discriminators — pre-existing-flag stay-quiet fired, hunk-flag missed); fixed hook = PASS=41 FAIL=0. Harness converted to payload-carrying invocations with disk content kept equal to payload so the 39 pre-existing assertions prove the conversion behavior-preserving. shellcheck clean. Guardrails 0.9.3 → 0.9.4.

Documented residuals (in-code): markdown fence state derives from the hunk alone — a fence-straddling edit can mis-classify in either direction; the convention's accepted trade. Live matcher is Write|Edit; other tools exit 0.

Related

• guardrails: cli-flag-verify PostToolUse hook scans whole file, warns on pre-existing lines outside the edited hunk #543 / guardrails: cli-flag-verify PostToolUse hook flags pre-existing unrelated lines (not diff-scoped) #544 (closes both)
• guardrails: hooks ship false-positive over-fires with no per-hook fixtures or shared precision convention (6-issue class: diff-scope, quoted-arg, boolean-intent, stdin, path-branch) #547 (hook-precision umbrella; rule-1 first conforming member)

🤖 Generated with Claude Code

[claude]

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

---

Security Review — PR #780

• Load tools
• Fetch PR diff
• Read changed files (cli-flag-verify.sh, cli-flag-verify.test.sh, plugin.json, CHANGELOG.md)
• Analyze for security vulnerabilities
• Post findings

---

Scope: diff-scope fix that shifts cli-flag-verify.sh from reading $FILE from disk to reading the tool payload (Edit.new_string / Write.content). No workflow files, no secrets handling, no permissions changes.

---

No security issues found.

The three new data-flow paths introduced are all safe:

1. tool_name extraction → $TOOL

TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null | tr -d '\r')
case "$TOOL" in
Edit) ... ;;
Write) ... ;;
*) exit 0 ;;
esac

$TOOL is used only as a case discriminator. A crafted multi-line value (jq's -r can output embedded newlines) won't match Edit or Write and falls to *) exit 0. No injection surface.

2. Payload content extraction → $SCAN_CONTENT

SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.new_string // empty' 2>/dev/null | tr -d '\r')

$SCAN_CONTENT is consumed exclusively through piped printf '%s' "$SCAN_CONTENT" | cmd chains inside emit_fragments() — never via unquoted word-splitting or command substitution. Arbitrary content in the payload cannot escape into shell execution.

3. Verifier invocation with payload-derived tokens

The pipeline is emit_fragments | split_segments → extract_candidates → "$VERIFIER" --quiet "$bin" "${chainarr[@]}" "$flag". The three payload-derived arguments are each gated before the call:

• $bin must appear word-exactly in the $BINS whitelist (a [[ " $BINS " == *" $bin "* ]] check using [[…]], which performs no word-splitting on $bin)
• ${chainarr[@]} elements must match ^[a-z][a-z0-9-]*$ before joining the chain
• $flag must match ^(--[a-zA-Z][a-zA-Z0-9-]*) via BASH_REMATCH

All three are passed as properly double-quoted arguments. No command injection path.

Attack-surface reduction vs. the pre-fix code

The old hook passed "$FILE" — an attacker-controlled path from the payload — as a filename argument to grep, awk, and subsequent tools. A crafted path beginning with - could have been misinterpreted as an option by some of those tools (classic filename-as-option). The fix eliminates every cmd "$FILE" call in favor of printf '%s' "$SCAN_CONTENT" | cmd, removing that class of risk entirely.

Advisory-only blast radius

The hook exits 0 in every branch. Even a complete logic failure produces, at worst, a missed detection or a spurious advisory message — not a block, credential leak, or code execution.

---

No findings to report.

[claude]

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

---

PR Review: fix(guardrails): diff-scope cli-flag-verify to the tool payload

• Gather context (PR diff, changed files)
• Review cli-flag-verify.sh hook logic
• Review cli-flag-verify.test.sh test suite
• Review plugin.json version bump
• Review CHANGELOG.md entry
• Post final review

---

Summary

Clean, well-motivated fix. The defect is real and well-described: the old hook re-read the whole file on every PostToolUse event, so a pre-existing unknown flag in an unrelated part of the file fired the advisory on every subsequent edit regardless of what the edit touched. The fix correctly sources content from the tool payload (new_string for Edit, content for Write) and never touches disk for scanning.

No correctness bugs found. A few observations below.

---

cli-flag-verify.sh

Order of guards (lines 56–76)

The extension check (line 56–60) still runs before the new TOOL block (lines 70–76). This is correct — exiting early on non-target extensions avoids unnecessary jq work. The sequence is now:

1. Extension gate → exit 0 for .cs, etc.
2. TOOL extraction → exit 0 for anything that isn't Write or Edit
3. Empty-content guard → exit 0 if payload had no content
4. Verifier binary presence check → fail-open

The positioning of step 4 after steps 1–3 means a missing verifier is never reached for non-target tools or extensions, which is correct.

tool_name absent → silent skip

If the Claude runtime ever sends a payload without tool_name, TOOL will be empty, the *) exit 0 ;; branch fires, and the hook silently skips. This is the right fail-open choice for an advisory hook. The PR description and comment both document this. No action needed, just worth noting as an explicit invariant.

Three separate jq calls on $INPUT

Line 54, 70, 72–73 each parse $INPUT separately. Three jq forks on a small JSON blob is negligible in practice. A single read -r FILE TOOL CONTENT < <(jq ...) extraction would be marginally faster but would complicate null-handling and isn't worth the complexity here.

FILE variable is still needed

Even though we no longer read file content from disk, FILE is still used for:

• Extension detection (IS_MD)
• Telemetry path (REPO_ROOT, file_rel)
• The advisory message ("unknown flag(s) in $FILE")

This is all correct.

printf '%s' "$SCAN_CONTENT" without trailing newline

For multi-line SCAN_CONTENT values that lack a trailing \n, grep and awk will still process the last line correctly. No issue.

---

cli-flag-verify.test.sh

run_fake conversion is behavior-preserving

The comment at line 49–53 accurately explains the invariant: disk == payload bytes, so the pre-fix hook (reading disk) and the fixed hook (reading payload) see the same content, and all 39 pre-existing cases remain valid coverage of the extraction logic.

Minor: disk vs. payload trailing newline mismatch in run_fake

printf '%s\n' "$content" >"$target" writes content + \n to disk, while --arg c "$content" in jq does not include the trailing newline. This is a cosmetic inconsistency — both grep and awk handle unterminated final lines correctly, so it doesn't affect pass/fail. The comment's claim that "disk == payload" is very slightly imprecise, but the test correctness is unaffected.

run_edit design is exactly right

Lines 71–82: disk and payload deliberately differ — the whole point of the helper. The two new test cases at lines 152–159 directly encode the bug-repro shape and its counterpart MUST-fire.

Test label on MUST-fire case

assert_exit "diff-scope: unknown flag in the hunk → exit 0" 0 "$RC"

The label → exit 0 is factually correct (advisory hook always exits 0) but could be read as "nothing happened." The accompanying ctx_contains makes the intent clear, and the pattern is consistent with all other advisory-fire cases in the suite. Not worth changing.

Kill-switch and telemetry inline inputs (lines 165, 194)

Both now include tool_name:"Write" and content:$c. Without this, the new TOOL-dispatch block would exit 0 before reaching the disabled-check or telemetry path, making those cases vacuously pass. The updates are necessary and correct.

Directory naming (fake-$((PASS+FAIL+1)) vs edit-$((PASS+FAIL+1)))

Different prefixes prevent collision even when PASS+FAIL+1 is the same numeric value between run_fake and run_edit invocations.

---

plugin.json / CHANGELOG.md

Version bump from 0.9.3 → 0.9.4 is appropriate for a bug fix that changes hook behavior. CHANGELOG entry at 0.9.4 accurately describes the fix, the repro-first approach, and the accepted markdown fence trade-off.

---

Verdict

Approve. The fix is correct, scoped tightly to the defect, repro-first, and the test harness conversion is behavior-preserving. The documented residuals (fence-straddling edits, Write new-vs-overwrite indistinguishability) are genuine limitations of payload-only scoping and are correctly accepted trade-offs per the hook-precision convention.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: db5760e7b3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

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

---

PR #780 Review

• Load tools
• Read changed files (cli-flag-verify.sh, cli-flag-verify.test.sh, CHANGELOG.md)
• Evaluate Codex P2 finding (partial Edit context loss)
• Assess correctness, edge cases, and test coverage
• Post review

---

Summary

The core fix is correct and sound: moving from grep "$FILE" / awk "$FILE" to printf '%s' "$SCAN_CONTENT" | grep / printf '%s' "$SCAN_CONTENT" | awk eliminates the whole-file re-read and properly implements the hook-precision convention's diff-scoping rule. The test conversion is behavior-preserving and the two new diff-scope discriminators directly encode the bug repro. No regressions.

One residual (below) is not yet documented in-code or tested.

---

Finding: Codex P2 — partial-hunk Edit misses flag-only replacements (undocumented residual)

Status: Real, inherent to the approach, but should be documented alongside the fence-straddling note.

When an Edit replaces only a flag token — old_string="--real", new_string="--fake" — the payload's new_string is just --fake. In extract_candidates, the first token of that segment is --fake, which doesn't match any entry in $BINS, so the segment is silently skipped and no advisory fires for the flag the edit introduced.

This is not a regression (the pre-fix hook would have scanned the whole file and potentially fired on unrelated lines); it's a new false-negative class that is inherent to payload-only scoping. It belongs in the documented residuals block at line 118–122 alongside the fence-straddling trade-off:

# A flag-only hunk (new_string="--fake" without the leading bin/subcommand)
# yields no bin match in extract_candidates → no advisory. This is the
# same accepted trade as fence-straddling: the payload cannot recover
# context that falls outside the hunk.

The test at line 157 only covers the full-command hunk shape (hunk='faketool sub --fake'). A complementary case for the pure-flag shape would make this explicit:

# flag-only hunk: no binary context → silent skip (accepted residual)
OUT=$(run_edit 'faketool sub --real' '--fake'); RC=$?
assert_exit "diff-scope: flag-only hunk → exit 0" 0 "$RC"
assert_silent "diff-scope: flag-only hunk → no advisory (no bin context)" "$OUT"

Without this test, a future attempt to "fix" the miss by reading from disk would break the diff-scope contract without an explicit signal that the silence is intentional.

Consider adding the comment + test →

---

Everything else looks correct

Dispatch block (lines 70–75): jq -r '.tool_name // empty' correctly produces empty string (not "null") when the field is absent, so the *) exit 0 ;; branch fires cleanly. CRLF stripping via tr -d '\r' on $TOOL is defensive and harmless.

Empty-content guard (line 76): [[ -n "$SCAN_CONTENT" ]] || exit 0 correctly handles deletes (where new_string or content is empty) without attempting to scan nothing.

Three separate jq forks on $INPUT: Noted in the prior review as negligible for a small JSON blob. The separation keeps null-handling clean; no change warranted.

FILE variable correctly retained: Extension detection (IS_MD), telemetry path, and the advisory message all still reference $FILE, which is correct since these are not payload-derived.

Test helpers updated correctly: run_fake (line 64) now passes tool_name:"Write" + content:$c; run_edit (line 81) passes tool_name:"Edit" + new_string:$s. Kill-switch (line 165) and telemetry (line 194) inline inputs updated to carry the new payload shape — without this, those cases would exit 0 at the TOOL dispatch and the assertions would vacuously pass.

run_edit design: Disk and payload deliberately differ. This is exactly right for encoding the diff-scope contract.

CHANGELOG / version bump: 0.9.3 → 0.9.4 is appropriate for a bug fix that changes scanning behavior.

---

Verdict

Approve with the suggestion to add one comment and one test case to document the partial-hunk residual. The fix itself is correct, and the overall improvement (no more whole-file re-reads, no more pre-existing-flag false positives) is a clear win. The undocumented residual is a known limitation of payload-only scoping — the only question is whether future maintainers will know it's intentional.

[claude]

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

---

Security Review — PR #780

• Load tools
• Fetch PR diff
• Read changed files (cli-flag-verify.sh, cli-flag-verify.test.sh, plugin.json, CHANGELOG.md)
• Analyze for security vulnerabilities
• Post findings

---

Scope: Two commits — db5760e (diff-scope fix: payload-read replacing disk-read) and e056bb1 (typo mis-classify → misclassify in a comment and CHANGELOG). No workflow files, no secrets handling, no permissions changes.

---

No security issues found.

I independently verified the four data-flow paths introduced or changed by this PR:

1. TOOL extraction and dispatch

TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null | tr -d '\r')
case "$TOOL" in
Edit) ... ;;
Write) ... ;;
*) exit 0 ;;
esac

tr -d '\r' strips carriage returns but not newlines. A crafted tool_name containing an embedded newline (jq -r can emit these) produces a multi-line $TOOL. A multi-line value does not match the literal patterns Edit or Write in bash case, so it falls to *) exit 0. No injection surface; safe fail-open.

2. SCAN_CONTENT extraction

SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.new_string // empty' 2>/dev/null | tr -d '\r')

$INPUT is double-quoted in printf; jq parses it as data. $SCAN_CONTENT is consumed exclusively through printf '%s' "$SCAN_CONTENT" | cmd pipelines in emit_fragments() — never through unquoted expansion, eval, or as a filename argument. Arbitrary payload content cannot escape into shell execution.

3. Attack-surface reduction vs. the pre-fix code

The old hook passed "$FILE" — the attacker-controlled tool_input.file_path value — as a filename argument to grep and awk. A path beginning with - can be misinterpreted as an option flag by those tools (the classic filename-as-option class). The fix eliminates every cmd "$FILE" call in emit_fragments(), replacing them with printf '%s' "$SCAN_CONTENT" | cmd. The risk class is gone.

4. Downstream verifier invocation

Line 262: "$VERIFIER" --quiet "$bin" "${chainarr[@]}" "$flag" — all three payload-derived arguments are gated before the call:

• $bin must appear word-exactly in " $BINS " via [[...]] pattern match (no word-splitting on $bin; the [[ compound command is safe even if $bin contained metacharacters)
• Each element of ${chainarr[@]} originates from chain entries validated against ^[a-z][a-z0-9-]*$ — alphanumeric and hyphens only
• $flag must match ^(--[a-zA-Z][a-zA-Z0-9-]*) via BASH_REMATCH before it enters the candidates map

All three are passed as separately double-quoted arguments. No command injection path.

5. Typo commit (e056bb1) — comment/text only

Changes mis-classify → misclassify in one comment in cli-flag-verify.sh and one line in CHANGELOG.md. No logic change; no security relevance.

Advisory-only blast radius

The hook exits 0 in every branch. Even a complete logic failure produces, at worst, a missed detection or a spurious advisory message — no block, credential leak, or code execution.

---

No findings to report. The diff-scope change is correct and strictly reduces the pre-existing attack surface.

[claude]

Claude finished @kyle-sexton's task in 3m 18s —— View job

---

Security Review — PR #780 (commit a3b4942)

• Load tools
• Fetch PR diff
• Read cli-flag-verify.sh (full current state)
• Analyze new reconstruct_partial_edit() code path
• Post findings

---

Scope: Commit a3b4942 adds reconstruct_partial_edit() — a bounded context reconstruction path that re-introduces on-disk reads for the specific case where an Edit hunk contains only a bare flag token. This is the only change not previously reviewed. The prior two security reviews covered commits db5760e and e056bb1 and found no issues; this review covers the new commit only.

---

No security issues found.

The single new data-flow path (reconstruct_partial_edit, lines 267–300) is implemented securely across every attack surface:

1. grep -F -- "$tok" "$FILE" — the re-introduced disk read (line 281)

This is the heart of the new path. Three concerns to audit:

• $tok as a pattern: Tokens in hunk_flags are extracted from the payload via grep -oE '(^|[[:space:]])--?[A-Za-z0-9][A-Za-z0-9-]*' — strictly alphanumeric-plus-hyphens after the leading hyphen(s). No shell metacharacters can survive into $tok. Combined with -F (fixed string, not regex) and double-quoting, $tok is inert.

• $FILE as a filename: The prior security reviews praised the elimination of cmd "$FILE" calls (classic filename-as-option risk). This commit re-adds one such call. However, -- is used correctly: grep -F -- "$tok" "$FILE" — everything after -- is treated as positional arguments, so a $FILE beginning with - cannot be misinterpreted as an option. The double-quoting prevents word-splitting and glob expansion. This is safe.

• Special file guard: [[ -f "$FILE" ]] at line 268 tests for a regular file. -f returns false for character devices (/dev/zero), named pipes (FIFOs), and block devices — the classes that would cause a blocking or infinite read. Symlinks to regular files follow through correctly.

2. Content from disk flows through a safe pipeline

After grep returns, disk lines are accumulated into $ctx, then:

$ctx → SCAN_CONTENT → printf '%s' "$SCAN_CONTENT" | emit_fragments() | split_segments() → extract_candidates()

Every step uses piped printf '%s' "$SCAN_CONTENT" | cmd — no eval, no unquoted expansion, no filename arguments carrying disk content. The same downstream validation gates then apply before the verifier is invoked: $bin must whitelist-match, subcommand elements must satisfy ^[a-z][a-z0-9-]*$, and $flag must satisfy ^(--[a-zA-Z][a-zA-Z0-9-]*). Arbitrary disk content cannot escape into shell execution.

3. Diff-scope filter is safe (lines 288–299)

[[ "$flag" == "$t" ]]

This is a bash [[ string equality comparison, not a pattern match. Since $t values are constrained to [A-Za-z0-9-] characters (via the hunk_flags regex), there are no glob characters that could widen the match. Correct and safe.

4. head -20 bound (line 284)

Applied to the concatenated $ctx after all per-token greps complete. Individual grep -F -- "$tok" "$FILE" calls are not bounded before accumulation — if $FILE were very large with many matches, $lines could be large before truncation. Not a security vulnerability (the content is inert), but worth noting as a worst-case performance characteristic. In practice, $FILE is a source file the agent just edited.

5. Advisory-only blast radius

reconstruct_partial_edit exits 0 in every branch (via return 0). The enclosing hook exits 0 in all branches. Even a complete logic failure in the new code path produces, at worst, a missed advisory or a spurious one — never a block, credential leak, or code execution path.

---

No findings to report. The reconstruct_partial_edit() implementation is careful and correct: disk reads are guarded by -f, protected against option-injection by --, and the content flows through the same safe validated pipeline as the payload-only path.

[claude]

Claude finished @kyle-sexton's task in 3m 15s —— View job

---

PR #780 Review — Commit a3b4942 (Bounded Context Reconstruction)

• Read current state of changed files
• Review bounded context reconstruction implementation (a3b4942)
• Check test coverage for new logic
• Verify SCAN_CONTENT mutation safety and data-flow
• Post findings

---

Summary

a3b4942 addresses the Codex P2 finding correctly. The implementation is well-bounded, correctly preserves the diff-scope contract through reconstruction, and the four new test cases directly encode the repro shapes. Approve.

---

reconstruct_partial_edit — Correctness Analysis

Trigger condition (line 271)

((${#CANDIDATES[@]} == 0)) || return 0

Correct. Only fires when the hunk produced zero candidates (the bare-flag case). A full-command hunk (faketool sub --fake) populates CANDIDATES in the first extract_candidates call and reconstruction is suppressed — no double-scan from disk. This is verified implicitly by test case 2 (run_edit 'faketool sub --real' 'faketool sub --fake'), which fires via the normal path with no reconstruction.

grep -F -- "$tok" on the file (line 281)

The -- correctly prevents flag-shaped tokens like --fake from being misinterpreted as grep options. Fixed-string match is correct here: substring matching is intentional — --fake-more on disk would be pulled, but the filter below correctly excludes it since --fake-more ≠ --fake.

CANDIDATES filter loop (lines 289–299)

Exact [[ "$flag" == "$t" ]] comparison, not a substring or pattern check. A pre-existing unrelated --otherbogus sharing a disk line with the hunk's --real will never match any $t in hunk_flags and is correctly pruned. Test case 4 (run_edit 'faketool sub --real --otherbogus' '--real') directly encodes this.

SCAN_CONTENT mutation (line 286)

SCAN_CONTENT="$ctx" overwrites the global with disk-derived context. Verified safe: SCAN_CONTENT is not referenced after reconstruct_partial_edit in the main flow — emit_tel and the advisory block both operate only on CANDIDATES, FAILURES, and FILE.

Re-calling extract_candidates into existing CANDIDATES (line 287)

Since the trigger requires CANDIDATES is empty before reconstruction, the second extract_candidates call accumulates into an empty map — no contamination from a prior call.

---

One Minor Observation — Short Flags in hunk_flags Extraction

The hunk_flags regex (line 275):

grep -oE '(^|[[:space:]])--?[A-Za-z0-9][A-Za-z0-9-]*'

The --? captures both --long-flags AND -s short flags. For a bare short-flag hunk (e.g., new_string="-f"):

• hunk_flags = ["-f"]
• grep -F -- "-f" "$FILE" runs and may match many lines (substring match, including paths containing -f)
• extract_candidates finds long-flag candidates from those lines
• The filter [[ "$flag" == "-f" ]] fails for all long flags (--something) → all pruned → silent skip

The silent skip is correct (short flags can't be verified against --help in the current framework), but the unnecessary disk grep work for a short-flag hunk is a minor inefficiency. Worth a one-liner comment in the residuals block if this surface ever grows, but not a bug.

No candidate mis-firings are possible: the filter is exact-match, so short flags in hunk_flags can only produce silence, never a false positive.

---

Test Coverage

The four new cases are well-chosen and necessary:

Test	Shape	Expected
run_edit 'faketool sub --fake' 'faketool sub --real'	Unknown flag on disk, clean hunk	Silent (regression guard for original bug)
run_edit 'faketool sub --real' 'faketool sub --fake'	Unknown flag introduced by hunk	Fires
run_edit 'faketool sub --fake' '--fake'	Bare-flag hunk, flag already on disk (PostToolUse)	Fires (reconstruction path)
run_edit 'faketool sub --real --otherbogus' '--real'	Bare-flag hunk; unrelated flag shares disk line	Silent (diff-scope contract through reconstruction)

The repro-first claim holds: test 3 is the MUST-FIRE that fails against the pre-reconstruction hook and passes after. Test 4 is the MUST-STAY-QUIET counterpart that directly encodes the filter invariant.

---

Everything Else

• head -20 bound on disk context — Limits blast radius for a hunk flag that appears on many disk lines (e.g., a common flag like --verbose). Correct.
• mapfile -t — Requires bash 4.0+, which declare -A already mandates. No new dependency.
• CHANGELOG / version — 0.9.3 → 0.9.4 is correct for a bug-fix that changes scanning behavior. Entry accurately describes both the diff-scope fix and bounded context reconstruction.
• is_skipped "$bin" && continue — Pre-existing per-binary opt-out is respected through reconstruction since extract_candidates already gates on this.

---

Verdict

Approve. The bounded context reconstruction correctly addresses the P2 gap — a swapped-in unknown bare flag now fires — while preserving the diff-scope contract: a pre-existing unrelated flag sharing the disk line stays quiet. The implementation is tight, the tests are repro-first, and the accepted trade-offs are documented in-code.