test(desktop-notification): measure C1 fd1-leak invariant differentially

[kyle-sexton]

Summary

The desktop-notification.test.sh C1 fd1-leak detector false-failed on Windows Git Bash. It asserted the hook returns in a fixed < 2000ms against a sink that sleeps 3s — but the hook's own process-spawn overhead (jq/tr/awk subshells) is ~1.6s solo and 4-10s under parallel-suite load, so the bound had a thin-to-negative margin even though no fd1 leak exists. On this dev machine it already failed outright: 2023ms elapsed, pure overhead, no leak.

Fix

Measure the invariant differentially instead of against a fixed wall-clock bound. The real invariant is "the hook did not wait for the backgrounded telemetry sink." A fast-sink baseline run captures the machine's current spawn overhead under the same command-substitution capture; the slow-sink run's excess over that baseline isolates the leak signal. Ambient overhead cancels in the subtraction, so the check holds under sustained load. Under a leak the sink's whole sleep lands in the delta; with no leak the delta is ~0 (± scheduling jitter). Threshold is half the sink sleep — comfortably above observed jitter, comfortably below the leak signal.

Why differential over the issue's literal Option A/B (< sink_sleep, or a proportional bump): both are still fixed thresholds measured against variable overhead, and the actual trigger is sustained load (parallel suites), under which overhead rises on both the baseline and the measured run together. A differential cancels that; a fixed bound eventually loses. Fixed < sink_sleep would additionally need sink_sleep > ~10s to clear the observed 4-10s overhead — a sink that then lingers past the suite's EXIT cleanup and locks its stub file on Windows. The differential keeps the sleep small (6s), so it self-expires during the post-C1 cases before cleanup.

Verification

Windows Git Bash (Ubuntu CI spawns ~10x cheaper — always had a huge margin and is unaffected).

Before (origin/main, fixed < 2000ms) — false-fail with no leak:

  (C1 slow-sink elapsed: 2023ms)
FAIL: telemetry/slow-sink: 2023ms — fd1 leak blocks
PASS=53 FAIL=1

After — 5 back-to-back runs, all green (worst observed base/slow jitter ~0.85s vs 3000ms threshold):

  (C1 fd1-leak: base=1450ms slow=1624ms delta=174ms, threshold <3000ms, sink sleeps 6s) ... PASS=54 FAIL=0
  (C1 fd1-leak: base=2008ms slow=1162ms delta=-846ms, threshold <3000ms, sink sleeps 6s) ... PASS=54 FAIL=0
  (C1 fd1-leak: base=1056ms slow=1139ms delta=83ms, threshold <3000ms, sink sleeps 6s) ... PASS=54 FAIL=0
  (C1 fd1-leak: base=1104ms slow=1360ms delta=256ms, threshold <3000ms, sink sleeps 6s) ... PASS=54 FAIL=0
  (C1 fd1-leak: base=757ms slow=970ms delta=213ms, threshold <3000ms, sink sleeps 6s) ... PASS=54 FAIL=0

Detector still catches a real leak — temporarily dropped the sink-spawn >/dev/null redirect in hook::emit_telemetry (fd1 inherited by the backgrounded sink), C1 went RED, then reverted:

  (C1 fd1-leak: base=1876ms slow=8139ms delta=6263ms, threshold <3000ms, sink sleeps 6s)
FAIL: telemetry/slow-sink: delta 6263ms ≈ sink's 6s sleep — fd1 leak blocks $() until the sink exits
PASS=53 FAIL=1

Full suite green after the fix: PASS=54 FAIL=0. No EXIT-cleanup errors across runs (the 6s sink self-expires during the later cases). Only the test file changed behaviorally; hook-utils.sh is unmodified (leak simulation was reverted).

Closes #448

Related

#443 (the hook::buffer_stdin migration that thinned the margin). Epic #313 (closed).

[claude]

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

---

Security Review — PR #751

• Load tools
• Fetch PR diff / read changed files
• Analyze for security issues
• Post findings

---

Scope

Three files changed: plugin.json (version bump 0.4.1 → 0.4.2), CHANGELOG.md (docs only), and desktop-notification.test.sh (the substantive change). Security surface is effectively the test file additions.

Findings

No security issues found.

The PR is a test-only change. I examined the two new constructs introduced:

epoch_delta_ms (lines 102–107)

epoch_delta_ms() {
  local start="$1" end="$2" ss sf es ef
  ss="${start%[.,]*}" sf="${start#*[.,]}"
  es="${end%[.,]*}" ef="${end#*[.,]}"
  echo $(((es * 1000000 + 10#$ef - ss * 1000000 - 10#$sf) / 1000))
}

Both call-sites pass $EPOCHREALTIME captures directly (L239, L246). EPOCHREALTIME is a bash readonly built-in; it cannot be injected via the environment. The 10#$sf/10#$ef base-10 coercion is correct for locale-agnostic fractional-second parsing and carries no external injection path.

Case 13 — differential fd1-leak detector (lines 215–256)

SINK_SLEEP=6
BASE_SINK="$(make_sink "cat >/dev/null")"
SLOW_SINK="$(make_sink "cat >/dev/null; sleep $SINK_SLEEP")"

make_sink writes $1 literally into an executable under the private $WORK directory (created with mktemp -d, mode 700). Both call-sites use either a hardcoded literal or a hardcoded integer constant (SINK_SLEEP=6, assigned two lines above), so there is no injection surface. The sinks are confined to $WORK and cleaned up by the EXIT trap.

Notes

• The pre-existing sink pattern make_sink "cat >\"$TEL\"" (Cases 11, 12, 14) places $TEL/$FAIL_FILE outside $WORK (bare mktemp → system /tmp). This is unchanged by this PR and the risk is negligible in a local-test context, but noted for completeness.
• No production hook code, GitHub Actions workflows, credentials handling, or permission configuration was touched.

[claude]

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

---

PR Review — test(desktop-notification): measure C1 fd1-leak invariant differentially

• Gather context from PR
• Read changed files
• Review test logic and differential measurement approach
• Check plugin.json and CHANGELOG.md changes
• Post findings

---

Summary

This PR replaces a fixed < 2000ms wall-clock assertion in the C1 fd1-leak detector with a differential measurement. The motivation is sound and the fix is correct. No concerns with the approach; a few minor observations below.

---

epoch_delta_ms helper (line 102–107)

The extraction is well done. The locale-agnostic [.,] separator and the 10#$sf / 10#$ef prefix for base-10 coercion (preventing leading-zero microseconds like 012345 from being read as octal) are exactly the right guards for cross-platform bash arithmetic. These were already present inline before; good to have them named and reusable.

---

Differential design (lines 231–256)

The core invariant — "the hook's $() returns as soon as the hook exits, not after the sink closes fd1" — is correctly isolated. The baseline fast-sink run and the slow-sink run both go through the same run() wrapper and build_input capture, so their shared overhead cancels in the subtraction. Under a leak, DELTA_MS ≈ SINK_SLEEP * 1000; under no leak, DELTA_MS ≈ 0 ± jitter. The threshold of SINK_SLEEP * 1000 / 2 = 3000ms sits comfortably in the middle.

Negative delta is handled correctly. The PR body shows one run with delta=-846ms. Bash arithmetic treats DELTA_MS as a signed integer, so [[ $DELTA_MS -lt $THRESHOLD_MS ]] passes for negatives — correct, a negative delta means the slow run was actually faster than baseline (scheduler variance) and that is definitively not a leak.

Threshold margin. Observed no-leak jitter peaks at ~850ms vs 3000ms threshold; real-leak delta is ~6000ms. The 4:1 signal-to-noise ratio is strong.

Sequential timing risk. If a system hiccup happens to fall between the two runs and adds >3s to the slow run alone, you'd get a spurious failure. This is a known limitation of sequential differential timing and can't be avoided without running the two cases simultaneously (which would change the measurement). The 3000ms threshold makes this very unlikely in practice, and it's spelled out clearly in the comment block.

---

Minor observation: _TS0 / _TS1 reused across both runs (lines 235–246)

The same _TS0/_TS1 names are reused for both the baseline and slow-sink captures. There's no bug (sequential use), but a reader has to mentally track which assignment wins. _BASE_TS0/_BASE_TS1 and _SLOW_TS0/_SLOW_TS1 would make the two runs visually independent — low priority, purely style. Fix this →

---

plugin.json version bump (0.4.1 → 0.4.2)

Technically this is a test-only change (no consumer-observable behavior changes), so a version bump isn't strictly required by semver. That said, bumping patch on a test improvement is a reasonable convention if the project treats test changes as releasing a new version of the test suite alongside the plugin. The CHANGELOG entry is accurate and complete.

---

Overall

The approach is strictly more correct than the fixed threshold for this invariant, the comments explain the "why" clearly, and the verification evidence in the PR body is thorough (including a deliberate leak regression to confirm the detector still fires). Approved.

[claude]

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

---

Security Review — PR #751

• Load tools
• Fetch PR diff / read changed files
• Analyze for security issues
• Post findings

---

Scope

Three files changed: plugin.json (version bump 0.4.1 → 0.4.2), CHANGELOG.md (docs only), and desktop-notification.test.sh (the substantive change). Security surface is effectively the test file additions — the new epoch_delta_ms helper and Case 13 (the differential fd1-leak detector).

Findings

No security issues found.

I examined the two constructs introduced by this PR:

epoch_delta_ms (lines 102–107)

epoch_delta_ms() {
  local start="$1" end="$2" ss sf es ef
  ss="${start%[.,]*}" sf="${start#*[.,]}"
  es="${end%[.,]*}" ef="${end#*[.,]}"
  echo $(((es * 1000000 + 10#$ef - ss * 1000000 - 10#$sf) / 1000))
}

Both call-sites pass $EPOCHREALTIME captures directly (L239, L246). EPOCHREALTIME is a bash dynamic built-in; its value is always a well-formed decimal float generated by the shell itself and cannot be injected through the process environment. The 10#$sf/10#$ef coercion is the correct guard against leading-zero microseconds being read as octal, and carries no external injection path.

Case 13 — differential fd1-leak detector (lines 215–256)

SINK_SLEEP=6
BASE_SINK="$(make_sink "cat >/dev/null")"
SLOW_SINK="$(make_sink "cat >/dev/null; sleep $SINK_SLEEP")"

make_sink writes its argument literally into an executable under the private $WORK directory (created with mktemp -d). The two new call-sites here use either a hardcoded literal ("cat >/dev/null") or a hardcoded integer constant (SINK_SLEEP=6, set two lines above). Even if SINK_SLEEP were set in the caller's environment, the unconditional assignment on L231 overrides it before any use. No external injection surface is introduced.

Notes

• The pre-existing make_sink "cat >\"$TEL\"" pattern (Cases 11, 12, 14) — where $TEL/$FAIL_FILE are bare mktemp paths outside $WORK — is unchanged by this PR. Negligible risk in a local test context, noted for completeness.
• No production hook code, GitHub Actions workflows, credentials handling, or permission configuration was touched.

[claude]

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

---

PR Review — test(desktop-notification): measure C1 fd1-leak invariant differentially

• Read changed files and diff
• Review epoch_delta_ms helper extraction
• Review differential measurement design (Case 13)
• Verify edge cases: negative delta, threshold sizing, sequential timing
• Review plugin.json and CHANGELOG.md

---

epoch_delta_ms helper (lines 102–107)

The logic was already correct in the old inline form; extracting it as a named function is an improvement. Two guards are worth confirming:

• [.,] separator — handles locale-specific decimal points (Windows uses ,). Correct.
• 10#$sf / 10#$ef — forces base-10 so fractional strings like 012345 aren't misread as octal in $(( )). Correct and necessary.

One implicit assumption: $EPOCHREALTIME always has exactly 6 fractional digits (microseconds). This holds on bash 4+/5+ across Linux, macOS, and Windows Git Bash, and was already assumed in the old inline code, so this is not a new risk. Still worth a note in the docstring if the function ever moves somewhere more general.

---

Differential design (lines 231–256)

The invariant being tested — "the $() wrapping run() unblocks when the hook exits, not when the backgrounded sink closes fd1" — is correctly isolated. Both the baseline and slow-sink runs go through the same run() wrapper plus a nested $(build_input permission_prompt) call, so jq-spawn overhead appears in both BASE_MS and SLOW_MS and cancels in the subtraction.

Negative delta is handled correctly. The PR shows an observed run of delta=-846ms; [[ $DELTA_MS -lt $THRESHOLD_MS ]] passes for any negative signed integer — a negative delta means the slow run finished faster than baseline (scheduler variance) and is definitively not a leak. No explicit guard needed.

Threshold sizing is solid. SINK_SLEEP=6 → THRESHOLD_MS=3000. Observed no-leak jitter peaks at ~850ms; real-leak delta is ~6000ms. The 4:1 signal-to-noise ratio across 5 documented runs is convincing. The PR correctly notes why keeping SINK_SLEEP small (rather than >10s as a fixed-bound approach would require) matters: the 6s sink self-expires during later cases before the EXIT cleanup, avoiding stub-file lock issues on Windows.

Sequential timing limitation. If a system hiccup falls exclusively between the two runs and inflates SLOW_MS by >3s with no leak, you'd get a spurious failure. This is inherent to sequential differential timing and is clearly documented in the comment block. At 3000ms headroom it's negligible in practice.

---

Minor observation: _TS0 / _TS1 reused across both runs (lines 235–246)

The same names are reassigned sequentially — no correctness bug, but a reader must mentally track which assignment is "current." _BASE_TS0/_BASE_TS1 and _SLOW_TS0/_SLOW_TS1 would make the two measurement windows visually independent. Low priority, style only. Fix this →

---

plugin.json and CHANGELOG.md

Version bump 0.4.1 → 0.4.2 is appropriate for a test-improvement patch. The CHANGELOG entry is accurate and covers the motivation, the mechanism, and why a fixed bound was insufficient.

---

Overall

The fix is technically sound. The differential measurement correctly cancels ambient spawn overhead, the threshold has strong empirical margin, the verification evidence in the PR body includes a deliberate leak regression (confirming the detector still fires when it should), and the in-code comments explain the design fully. Approved.

---

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 806cae0fc9

ℹ️ 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 9s —— View job

---

PR Review — test(desktop-notification): measure C1 fd1-leak invariant differentially

• Load tools
• Read changed files and diff
• Review differential measurement design and merge-from-main state
• Check for open design questions (Codex finding / kyle-sexton response)
• Post findings

---

Post-merge state

The HEAD commit (37265ce) is a clean merge of origin/main into this branch. The desktop-notification test file was untouched by the merge — no conflicts, no regressions from the 20+ commits that came in from main. Prior approvals from the earlier review passes remain valid.

The version field in plugin.json bumped to 0.4.3 (was 0.4.2 when the PR was first authored) — 0.4.2 was already occupied by something from main. The CHANGELOG entry accurately reflects [0.4.3]. This is correct.

---

epoch_delta_ms helper (lines 102–107)

Correct. [.,] handles locale-specific decimal separators; 10#$sf/10#$ef prevents zero-padded microseconds (e.g. 012345) from being read as octal inside $(( )). The extraction from inline to a named function improves reuse across both timing windows. One unstated assumption: $EPOCHREALTIME always produces exactly 6 fractional digits — this holds on bash 4+/5+ on all three platforms, and was already the same assumption in the old inline form, so no new risk.

---

Open design question: Codex P2 sequential-timing false-negative (line 252)

The finding is real but the risk is low. The failure mode requires overhead_base − overhead_slow > 3000ms — the baseline run being more than 3s slower than the slow run without a leak present. Observed jitter across 5 runs peaked at ~850ms, giving a 3.5× safety margin. Under pathological parallel-suite load (4–10s overhead) the margin shrinks, but a >3s asymmetric swing between two immediately-consecutive runs is a genuinely tail-of-tails event.

Concrete options, smallest-change first:

1. Min-of-N baseline — run the baseline twice and take min(BASE_MS_A, BASE_MS_B). Adds one extra hook invocation (~1-2s overhead on Windows), directly targets the failure mode (a min reliably reflects true-minimal overhead so an inflated baseline doesn't produce a false-negative), keeps SINK_SLEEP=6 and the rest of the design unchanged. This is the recommended path if you want to close the gap.

2. Floor baseline at zero — (( BASE_MS < 0 )) && BASE_MS=0 after measuring, then compare SLOW_MS against THRESHOLD_MS directly. This doesn't help with a genuinely-inflated-baseline false-negative (an anomalously slow baseline still inflates BASE_MS), but it handles negative jitter edge cases defensively.

3. Accept the documented tradeoff — keep as-is; the comment block at lines 221–230 already explains the sequential-timing limitation clearly. The fix is a strict improvement over the false-failing fixed threshold, and the false-negative mode requires a tail event that hasn't been observed in testing.

If you accept option 3, this thread is resolved by the in-code documentation. If you want to close it, option 1 is the lowest-effort targeted fix.

---

Minor style note (unchanged from prior reviews)

_TS0/_TS1 are reused for both the baseline and slow-sink captures (lines 235–241) — no correctness bug, but a reader has to mentally track which assignment is current. _BASE_TS0/_BASE_TS1 and _SLOW_TS0/_SLOW_TS1 would make the two timing windows visually independent. Low priority. Fix this →

---

Overall

The differential design is correct, well-documented, and strictly more robust than the fixed threshold it replaces. The merge from main was clean. The one genuine open item is the Codex P2 sequential-timing false-negative — it's low-frequency and the in-code comment already discloses it, but a min-of-N baseline (option 1 above) would close it with minimal rework if you want belt-and-suspenders confidence. Approved as-is; the Codex thread is a judgement call for the author.
| Branch

[claude]

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

---

Security Review — PR #751

• Load tools
• Fetch PR diff / read changed files
• Analyze for security issues
• Post findings

---

Scope

Three files changed: plugin.json (version bump 0.4.2 → 0.4.3), CHANGELOG.md (docs only), and desktop-notification.test.sh (the substantive change). Security surface is the two new constructs in the test file: the epoch_delta_ms helper and the revised Case 13 differential fd1-leak detector.

Findings

No security issues found.

epoch_delta_ms (lines 102–107)

Both call-sites (L239, L246) pass $EPOCHREALTIME captures directly. EPOCHREALTIME is populated by bash itself and cannot be injected through the process environment. The 10#$sf/10#$ef coercion prevents octal misread of zero-padded microseconds; $((...)) evaluates arithmetic only, not arbitrary commands, so even a malformed timestamp would produce an arithmetic error rather than code execution.

Case 13 — differential fd1-leak detector (lines 231–256)

SINK_SLEEP=6
BASE_SINK="$(make_sink "cat >/dev/null")"
SLOW_SINK="$(make_sink "cat >/dev/null; sleep $SINK_SLEEP")"

Both new make_sink call-sites use either a hardcoded literal ("cat >/dev/null") or a string composed exclusively from the hardcoded integer SINK_SLEEP=6, set unconditionally on L231 — no external injection path. The sinks are written into the private $WORK directory (mktemp -d) and removed by the EXIT trap.

Notes

• The pre-existing make_sink "cat >\"$TEL\"" pattern (Cases 11, 12, 14) places $TEL/$FAIL_FILE (bare mktemp outputs) into script bodies outside $WORK. This is unchanged by this PR and the risk is negligible in a local-test context, but noted for completeness.
• No production hook code, GitHub Actions workflows, credential handling, or permission configuration was touched.

[claude]

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

---

Security Review — PR #751

• Load tools
• Fetch PR diff (new HEAD ffa565f)
• Read changed files
• Analyze for security issues
• Post findings

---

Scope

This pass reviews the HEAD commit ffa565f ("min-of-N baseline"), which is the change not covered by prior security reviews. Two files changed: CHANGELOG.md (docs only) and desktop-notification.test.sh (substantive). Security surface is the new min-of-N baseline loop in Case 13.

Findings

No security issues found.

I examined the two constructs introduced or modified by ffa565f:

New BASE_SAMPLES constant and loop (lines 242–254)

BASE_SAMPLES=3
...
for ((_i = 0; _i < BASE_SAMPLES; _i++)); do
  _TS0=$EPOCHREALTIME
  _OUT_BASE="$(run "$(build_input permission_prompt)" HOOK_TELEMETRY_SINK="$BASE_SINK")"
  _TS1=$EPOCHREALTIME
  _b=$(epoch_delta_ms "$_TS0" "$_TS1")
  if [[ -z "$BASE_MS" || $_b -lt $BASE_MS ]]; then BASE_MS=$_b; fi
done

BASE_SAMPLES=3 is a hardcoded integer literal; the C-style for loop body is pure arithmetic with no external input. The loop calls the same run() / build_input / epoch_delta_ms stack reviewed in prior passes — no new external inputs are introduced. _b is always the integer output of echo $(( ... )) inside epoch_delta_ms; that expression either yields a decimal integer or raises a bash arithmetic error, so the unquoted $_b and $BASE_MS in the [[ -lt ]] test carry no injection surface inside [[ ]] (no word splitting).

Updated make_sink call-site (line 244)

SLOW_SINK="$(make_sink "cat >/dev/null; sleep $SINK_SLEEP")"

SINK_SLEEP=6 is assigned unconditionally on line 241 immediately before use; no environment variable of that name can override it. This call-site was already present before this commit and is unchanged in its injection profile.

Updated diagnostic echo (line 265)

echo "  (C1 fd1-leak: base=${BASE_MS}ms (min of ${BASE_SAMPLES}) slow=${SLOW_MS}ms ...)"

BASE_MS, BASE_SAMPLES, SLOW_MS, DELTA_MS, THRESHOLD_MS, and SINK_SLEEP are all integers derived from arithmetic or hardcoded literals — no control characters, no expansion risk.

Notes

• The pre-existing make_sink "cat >\"$TEL\"" pattern (Cases 11, 12, 14) — where $TEL/$FAIL_FILE are bare mktemp outputs outside $WORK — is unchanged by this PR. Negligible risk in a local-test context; noted for completeness from prior reviews.
• No production hook code, GitHub Actions workflows, credential handling, or permission configuration was touched.

[claude]

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

---

PR Review — test(desktop-notification): measure C1 fd1-leak invariant differentially

• Load tools
• Read changed files at HEAD
• Review min-of-N baseline implementation
• Check plugin.json and CHANGELOG.md
• Post findings

---

New commit — min-of-N baseline (ffa565f)

This commit implements the "min-of-N baseline" recommendation from prior reviews to close the Codex P2 sequential-timing false-negative. The analysis is correct; the implementation matches the stated design.

---

False-negative fix — correctness (lines 246–254)

BASE_MS=""
for ((_i = 0; _i < BASE_SAMPLES; _i++)); do
  _TS0=$EPOCHREALTIME
  _OUT_BASE="$(run "$(build_input permission_prompt)" HOOK_TELEMETRY_SINK="$BASE_SINK")"
  _TS1=$EPOCHREALTIME
  _b=$(epoch_delta_ms "$_TS0" "$_TS1")
  if [[ -z "$BASE_MS" || $_b -lt $BASE_MS ]]; then BASE_MS=$_b; fi
done

The logic is correct on every axis:

• First-iteration guard — [[ -z "$BASE_MS" ]] short-circuits the comparison on the first sample so BASE_MS="" never enters an integer comparison. Correct.
• Min selection — $_b -lt $BASE_MS uses bash integer arithmetic on signed values; if overhead is somehow negative (clock granularity), it would be selected as the min, which is conservative. Non-issue in practice.
• Why min, not average — the comment (lines 232–240) correctly identifies the asymmetry: only an inflated baseline can mask a real leak (false-negative), whereas an inflated slow sample only false-fails a green run (fail-safe, re-runnable). The min directly targets the dangerous failure mode; an average would still be vulnerable to one extreme outlier.
• 3 samples — reasonable. On Windows (~1.5s per baseline run), this adds ~4.5s to Case 13 but strongly suppresses the inflated-baseline tail. No observable impact on the slow run path.

---

Slow run stays single (lines 256–261)

_TS0=$EPOCHREALTIME
_OUT_SLOW="$(run "$(build_input permission_prompt)" HOOK_TELEMETRY_SINK="$SLOW_SINK")"
RC_SLOW=$?
_TS1=$EPOCHREALTIME
SLOW_MS=$(epoch_delta_ms "$_TS0" "$_TS1")

Correct. The asymmetry argument holds: an inflated slow sample is fail-safe (produces a spurious failure that re-runs clean). Running the slow path N times would multiply the SINK_SLEEP cost by N (an extra N × 6s per suite run) — the single-sample slow run is the right trade-off.

RC_SLOW=$? correctly captures the exit code of the $() subshell before any other command runs.

---

Diagnostic output (line 265)

(C1 fd1-leak: base=${BASE_MS}ms (min of ${BASE_SAMPLES}) slow=${SLOW_MS}ms delta=${DELTA_MS}ms, threshold <${THRESHOLD_MS}ms, sink sleeps ${SINK_SLEEP}s)

The (min of 3) annotation is a meaningful addition — future diagnostic runs immediately show whether the baseline measurement is min-of-N or a one-shot, and BASE_SAMPLES being in the output means the annotation updates automatically if the constant changes.

---

Minor style note (inherited, still low priority)

_TS0/_TS1 are reused for both the baseline loop iterations and the slow-sink run — no correctness bug (sequential), but a reader has to track context. _BASE_TS0/_BASE_TS1 / _SLOW_TS0/_SLOW_TS1 would make the two measurement windows visually independent. This was noted in earlier reviews; the min-of-N change doesn't worsen it. Fix this →

---

plugin.json and CHANGELOG.md

Version 0.4.3 is correct (0.4.2 was already taken by a main-branch commit that merged in). The CHANGELOG entry accurately describes both the differential approach and the min-of-N baseline rationale in a single cohesive note.

---

Overall

The min-of-N baseline directly and correctly closes the Codex P2 false-negative. The asymmetry reasoning (min only on baseline, single slow run) is sound and clearly documented in the comment block. Approved.