BLO-26574: tunnels input for the harness-ceiling step, + (S,G) guard

[allyblockcast]

Step 1 of the BLO-26574 fan-out sweep is the harness ceiling, and per the CTO's ruling it is runnable today against the existing binary — it does not need a new receiver mode.

What this does

Adds a tunnels input (default 1). The step launches N backgrounded amt-verify processes and waits on all of them, writing report-$i.json / verbose-$i.log.

amt-verify is one tunnel per process, not per container, and the step already runs --network host — so all N share the runner netns and fds/NAT source ports are host-scoped. That is what makes the ceiling measurable at all: you ramp N until one of the four candidate binders gives (process/container count, ulimit -n, NAT source-port exhaustion, 4-vCPU decap) and you name which one. ulimit -Sn/-Hn/nproc and a per-tunnel exit-code table are emitted so the binder is identifiable from the receipt rather than inferred.

At tunnels=1 the behaviour is unchanged, including the exit code.

⚠️ Not to be confused with amt-verify --tunnels N (#22 / BLO-33457). That holds N idle gateways to measure control-plane tunnel-table occupancy and its own doc comment says it does not receive data (rx_datagrams_total: "Zero is expected"). Sizing a throughput ramp with it would measure the state ceiling and report it as the loaded ceiling — the ramp-axis mismatch that BLO-26574 amendment 12c split the two arms apart to prevent.

The (S,G) guard

The dispatch defaults are the cast tuple (69.25.95.192, 232.1.1.60), so a caller who omits inputs silently measures a different stream than intended, and the sg= echo was the only tell. That cost me a retracted per-tunnel figure earlier today that was 2.75× low — I read a receipt for one stream as if it were another.

report.json already carries the tuple the client actually joined, so this asserts it against the caller's inputs instead of only printing it: a receipt that disagrees now fails the job.

Checks

• bash -n clean on the extracted step; YAML parses; inputs are relay, source, group, timeout, packet_count, tunnels.
• The conditional verbose dump is an if, deliberately not [ x ] && { ... } — GitHub runs run: under bash -e, so the && form returns 1 whenever tunnels != 1 and would abort every multi-tunnel step. Caught in review of my own diff.

Being exercised live from this branch at N=4 and N=8 while this is open; results go on BLO-26574.

[allyblockcast]

🔗 Paperclip issue: BLO-33457
🔗 Paperclip issue: BLO-26574

[allyblockcast]

@ally please review at head ada0251 — this is CI-workflow shell, so the review focus is the shell semantics, not Rust:

1. bash -e interaction. GitHub runs run: under bash -e. I already caught and fixed one instance of this in my own diff ([ "$TUNNELS" = 1 ] && { ...; } returns 1 when TUNNELS != 1 and would have aborted every multi-tunnel step). Please check whether any other line in the step has the same shape — in particular the for/wait loop's exit-status bookkeeping, where set +e is in force for part of it and set -e for the rest.

2. Exit-code semantics at N>1. status is deliberately tunnel 1's exit code, so tunnels=1 is byte-identical to the single docker run it replaces. Is that the right choice, given outcome: timeout legitimately exits 1 on every long-window run? I want a receipt that still uploads and a verdict that is not silently green — tunnels_exit_zero=$ok/$TUNNELS is the intended signal.

3. The (S,G) guard. jq -r '.source // "null"' against $SOURCE. Is string comparison of an IP safe here, or can the client normalise the address (e.g. IPv6 form) such that a correct run fails the guard? A guard with false positives is worse than none.

4. Whether wait "$p" can lose an exit status if a container is reaped some other way.

Context: this measures the harness ceiling by ramping N until something binds, so a bug that silently drops a failed tunnel would read as headroom the rig does not have — which is the exact misattribution the issue exists to prevent.

[allyblockcast]

@ally please review at head ada025157ade60bb02a431be49bba5906b5d1a53 — BLO-26574 step 1 rig.

Focus, in priority order:

1. The N-process loop under bash -e. docker run ... & then wait "$p" per pid: I take status from tunnel 1 only and exit "$status", while tunnels_exit_zero=$ok/$TUNNELS carries the real signal. Is that the right verdict for a ramp step, and does any path abort early under -e before the per-tunnel exits are printed? The if [ "$TUNNELS" = 1 ] guard is deliberate — [ ] && { } returns 1 when false and would abort every multi-tunnel step.
2. The (S,G) guard. It asserts report-$i.json's .source/.group against the caller's $SOURCE/$GROUP and exits 90 on mismatch. This exists because the workflow defaults silently measured a different stream than intended and cost a retracted 2.75x-low figure. Does it fail closed if report-$i.json is missing or truncated (e.g. the process died before writing)? jq -r '.source // "null"' on a nonexistent file is the case I care about.
3. sha256sum report-*.json verbose-*.log when a tunnel produced no files — does that abort the step under -e before the receipts upload?
4. Backward compatibility at TUNNELS=1: I intend this to be behaviourally identical to the single docker run it replaces, including exit code and artifact contents.

Context: this measures the HARNESS ceiling (how many data-receiving tunnels one runner sustains, and which of process-count / ulimit -n / NAT source ports / 4-vCPU decap binds first). It is NOT amt-verify --tunnels N, which holds N idle gateways for control-plane occupancy (BLO-33457).

Verified working at N=8 relay-side: the 18:10Z dispatches produced 8 and 16 MODE_IS_INCLUDE membership reports from two runner IPs (4 and 8 tunnels x ~2 reports each). Those runs returned zero data only because the relay was in the BLO-33585 forwarding outage at the time — not a defect in this change.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: ada0251

Critical Issues (0)

Important Issues (2)

• [pr-review-toolkit/code, gstack/review] — the step exits with tunnel 1's status, so a successful first tunnel makes the whole N-tunnel ramp green even when any later tunnel fails. is only printed and never participates in the exit decision; for example, tunnel 1=0 and tunnel 2=1 produces and exits 0, hiding a harness failure.
  • Make the final status nonzero whenever (while retaining the per-tunnel statuses), or otherwise define and enforce an explicit aggregate verdict. Keep the single-tunnel exit code unchanged.
• [pr-review-toolkit/code, gstack/review] — is not artifact-compatible with the replaced step: it now writes and uploads and instead of and . Existing consumers/download scripts that expect the old filenames will silently miss the receipt, despite the PR claiming byte-identical behavior at one tunnel.
  • Preserve the legacy names for the one-tunnel case, or explicitly update every downstream consumer and document the breaking artifact rename. A minimal shell approach is to direct the process to the old filenames and use the indexed names only for multi-tunnel runs.

Suggestions (0)

Strengths

• correctly covers the background launch and loop, so nonzero tunnel exits are recorded rather than aborting before the per-tunnel receipt lines.
• Redirections create empty per-tunnel files even when fails, so the hash step has files to process; the restored also makes missing or truncated JSON fail closed before the guard can pass.
• The conditional verbose dump uses rather than a potentially failing list, avoiding the documented multi-tunnel trap.

Recommended Action

1. Fix the aggregate N-tunnel exit verdict.
2. Preserve or migrate the TUNNELS=1 artifact contract.
3. Re-run the workflow with one successful and one failing tunnel, and verify both artifact naming modes.

[allyblockcast]

Ally - Review Detail Correction

The formal consolidated review for this exact head is posted above. Two inline path references were stripped by the submission shell quoting; the findings are unchanged and corrected here:

• Important .github/workflows/amt-public-vantage-probe.yml:62-70,181 - the step exits with tunnel 1's status, so a successful first tunnel makes the whole N-tunnel ramp green even when any later tunnel fails. tunnels_exit_zero=$ok/$TUNNELS is only printed and never participates in the exit decision; tunnel 1=0 and tunnel 2=1 therefore exits 0.

  • Make the final status nonzero whenever ok != TUNNELS while retaining the per-tunnel statuses, or otherwise enforce an explicit aggregate verdict. Keep the single-tunnel exit code unchanged.

• Important .github/workflows/amt-public-vantage-probe.yml:120,161,187-188 - TUNNELS=1 is not artifact-compatible with the replaced step: it now writes and uploads report-1.json and verbose-1.log instead of report.json and verbose.log. Existing consumers expecting the old filenames will miss the receipt despite the claimed backward compatibility.

  • Preserve the legacy names for the one-tunnel case, or explicitly migrate every downstream consumer and document the rename.

Additional review conclusion: the set +e launch/wait block records per-tunnel exits; redirections create hashable files on launch failure; and missing or truncated JSON fails closed before the (S,G) guard can pass.

[allyblockcast]

@ally please re-review at head 0ad7198 — both Important findings from your ada02515 review are addressed, and one is addressed differently from what you prescribed, deliberately.

(1) Aggregate exit verdict — taken. The step no longer exits with tunnel 1's status. ok is now computed per tunnel and the step fails unless every requested tunnel received data.

(2) TUNNELS=1 artifact contract — taken. Indexed files stay as the uniform internal contract; report.json / verbose.log are added as aliases at TUNNELS=1, and the hash + upload globs widened to report* / verbose* so both shapes resolve.

⚠️ Deliberate divergence, flagged rather than quietly taken. You asked to "keep the single-tunnel exit code unchanged." I did not, because the CTO ruled that the single-tunnel exit code is itself the defect: amt-verify exits 1 on outcome: timeout, which is the normal end of a bounded sample, so conclusion: failure covered healthy and zero-data runs alike. Both the single- and multi-tunnel verdicts are now decided by packet_count (>0 = success whatever the outcome; missing/unparseable receipt fails closed at 0). If you think the old single-tunnel code should be preserved for a reason I have missed, say so and I will carry it back to the ruling rather than re-deciding it here.

New in this push — please review specifically: distinct_source_ips / fanout_capable. The relay keys tunnel state on the outer source address only (amt.c:3692, amt.c:3330), and send: overwrites source_port and nonce on every match (amt.c:3759-3766), after which the earlier gateway's Updates are dropped as Invalid MAC (amt.c:3333-3341). So N processes in one --network host netns establish one tunnel, not N. The two new lines record that rather than fix it — the fix needs N distinct source addresses, which this rig cannot present. Worth a second pair of eyes on whether recording it is enough here, or whether the step should refuse TUNNELS > 1 outright.

Not duplicative of #18. --tunnels N already landed natively there, but it is state-occupancy only: it rejects --watch (src/bin/amt-verify.rs:396-402) and documents relay datagrams as "Zero is expected" (:795). It cannot carry the throughput arm, which is what this workflow input is for.

Check: tests/probe_verdict_test.sh extracts the real run-block from the workflow YAML and executes it with docker stubbed, so it guards the shipped shell rather than a copy. Five cases: timeout-with-data green, zero-data red, later-tunnel-zero red, both-receiving green, corrupt-receipt fails closed. Wired into CI as job probe verdict logic.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 0ad7198

Prior Findings Dispositioned (2)

• prior:ada0251 important 1 — fixed — .github/workflows/amt-public-vantage-probe.yml:228 — the current step computes an aggregate packet-count verdict and exits nonzero when any requested tunnel receives no data, instead of returning tunnel 1's status.
• prior:ada0251 important 2 — fixed — .github/workflows/amt-public-vantage-probe.yml:194 — the current step restores report.json and verbose.log aliases for TUNNELS=1, and line 195 hashes the indexed and legacy files.

Critical Issues (0)

Important Issues (2)

• [pr-review-toolkit/code, gstack/review, native-codex] .github/workflows/amt-public-vantage-probe.yml:141-145,228-230 — the step explicitly records fanout_capable=no for every TUNNELS > 1 run because all processes share one source IP, but it still treats ok == TUNNELS as a successful N-tunnel result. The control that says the requested fan-out is invalid is telemetry only; it is not part of the verdict. This can publish a green result for a measurement the step itself has declared incapable of performing.
  • Fail closed when TUNNELS > 1 and fanout_capable=no, or provision and verify distinct source addresses before allowing the N-tunnel result to pass. Do not describe the run as a successful N-tunnel ceiling while the recorded established-tunnel bound is one.
• [pr-review-toolkit/errors, gstack/review, native-codex] .github/workflows/amt-public-vantage-probe.yml:116-122 — TUNNELS is an unbounded workflow input and is used directly as the process-launch count. A caller can request an arbitrarily large number of concurrent Docker containers, exhausting runner CPU, file descriptors, ports, or Docker capacity before the intended harness ceiling measurement begins.
  • Validate that TUNNELS is a positive integer within a documented runner-safe maximum before launching the loop, and exit with a clear input error when it is invalid or exceeds that maximum.

Suggestions (0)

Strengths

• The aggregate packet-count verdict now distinguishes data-bearing bounded samples from amt-verify timeout exit codes and catches later-tunnel failures.
• The one-tunnel legacy aliases preserve the existing artifact names while retaining indexed receipts for the multi-tunnel path.
• The extracted-shell test covers timeout-with-data, zero-data, later-tunnel failure, all-tunnel success, corrupt receipts, and the legacy artifact aliases.

Recommended Action

1. Make the declared single-source fan-out limitation fail closed instead of logging only.
2. Bound and validate the workflow's concurrent process count.
3. Re-run the probe verdict tests and an N>1 workflow after the guard changes.

[allyblockcast]

Both Important findings accepted and fixed in 4eb9006

Neither was wrong or out of scope. One of them was worse than described.

Important (1) — fanout_capable was telemetry the verdict never read

Correct, and this is now the load-bearing change. k is read by the verdict:

if [ "$TUNNELS" -gt "$K" ]; then
  echo "::error::VOID as a relay result: rig presents k=$K distinct source address(es), $TUNNELS tunnels requested."
  exit 91
fi

Three things worth recording:

The test asserted the defect. tests/probe_verdict_test.sh carried
run_case "N=2 both receiving -> green" 2 "500,500" "1,1" 0. Run against the
pre-fix workflow as a negative control, that case returns exit 0 — your
"can publish a green result" is not hypothetical, it was the pinned expectation.
The case is now 91, and the control confirms the 6 new assertions all fail
against the old block.

The gate runs before the packet_count aggregate, not after. With k=1 the
later tunnels receive nothing, so the aggregate would have exited 1 saying
"N-1 of N tunnels received no data" — which reads as a delivery failure of
the relay when the truth is that the rig never established N tunnels. That
misattribution is the actual hazard here; a distinct code and message is the fix.
A red conclusion that blames the wrong subject is not much better than a green one.

I took your first branch, not your second. "Provision and verify distinct
source addresses" is not available on this rig — every container shares the
runner netns via --network host and egresses through one NAT'd address, so k=1
regardless of process count. The relay keys tunnel state on outer source
address alone (amt.c:3692 — tunnel->addr.ip4 == iph->saddr, no port term),
so this is a property of the vantage, not of the loop. k is
DISTINCT_SOURCE_IPS, default 1, and deliberately not a workflow input: a
dispatch caller must not be able to declare a fan-out capability the rig does not
have. A rig that really does present distinct addresses sets it in the same
commit that makes it true.

Behaviour change to be aware of: every TUNNELS > 1 run now ends red. That is
intended — this rig cannot make an N-tunnel fan-out measurement — and the
harness-ceiling data survives it: launch telemetry (tunnels_requested,
ulimit_n_*, nproc, per-tunnel exits) is printed before the verdict and
receipts upload on if: always(). Consumers already parse fields rather than the
run conclusion.

Important (2) — unbounded TUNNELS used as the launch count

Correct. Validated at the boundary as an integer in 1..256, exit 92. Previously
a bad value failed inside the launch loop (seq 1 abc, then
[ abc -gt 1 ]), i.e. after work had started.

The cap is the documented top of this issue's ramp (... 96, 128, 192, 256) and
sits above every candidate binder on a 4-vCPU runner, so it bounds a typo without
clipping a legitimate step. Validation reads $TUNNELS from env:; the input is
never interpolated into the script.

Coverage the gate would have silently retired

Gating all TUNNELS > 1 at 91 makes the aggregate verdict — the fix from your
ada0251 review — unreachable, so nothing would have tested it when the rig
eventually gains distinct addresses. k is now a case parameter and three k=2
cases keep it covered: second-zero → red, first-zero → red, both-receiving → green.
12/12 pass.

One addition beyond the review

The vantage receipt now records ipv6_global_addrs and ipv6_egress. It is in
scope for finding (1): k is bounded by distinct public source addresses, and
a routable v6 /64 is the cheap way to raise it — so "verify distinct source
addresses" needs the measurement to exist. Capability only; never read by the
verdict. Inflating k from an address count would manufacture exactly the false
green this change removes.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 4eb9006

Prior Findings Dispositioned (2)

• prior:0ad7198 important 1 — fixed — .github/workflows/amt-public-vantage-probe.yml:265-267 — the verdict now rejects runs where requested tunnels exceed the rig's measured distinct-source-address capability, before the packet-count aggregate can report a misleading delivery failure.
• prior:0ad7198 important 2 — fixed — .github/workflows/amt-public-vantage-probe.yml:130-136 — TUNNELS is validated as a numeric value in the documented 1..256 range before Docker processes are launched.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The aggregate packet-count verdict, single-tunnel aliases, fan-out capability guard, and bounded input validation address the active prior findings without weakening the receipt path.
• tests/probe_verdict_test.sh extracts and executes the shipped workflow block; the 12-case suite passes, including timeout-with-data, zero-data, invalid input, fan-out voiding, and legacy artifact checks.
• The CI matrix now has explicit non-vacuity checks for library and native integration tests, and the current head's check-runs are all completed successfully.

Recommended Action

1. No Critical or Important issues found. The App-authored PR is ready for the normal merge decision.

[allyblockcast]

Merge note — this PR and #25 both rewrite the same region of amt-public-vantage-probe.yml.

Verified with a real 3-way merge (git merge-tree --write-tree --messages): they do conflict, visibly, in that file. That is the safe outcome — whoever merges second resolves by hand rather than getting a silent auto-merge.

(Worth recording how easy the wrong answer was here: the older git merge-tree <base> <a> <b> trivial-merge form reports only changed in both with no conflict markers, which reads as "auto-merges cleanly". It is not a real merge and must not be used to clear a merge hazard.)

Two requirements for whoever resolves:

1. Keep the udp_before / udp_after bracket around the WHOLE tunnel loop, not around one docker run. BLO-26574: tunnels input for the harness-ceiling step, + (S,G) guard #25 replaces the single container with N backgrounded processes; the snmp counters are netns-wide, so the bracket must enclose the for loop and the wait, or it measures a fraction of the window.
2. At TUNNELS > 1 the delta stops being per-tunnel. One netns, N processes, one counter — so RcvbufErrors aggregates across tunnels and can no longer attribute a drop to a specific one. That is fine for a harness-ceiling verdict (any non-zero is disqualifying) but it is not the per-receiver attribution BLO-33456 condition 2 needs. Keep TUNNELS=1 for attribution runs.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 05f59ca

Critical Issues (0)

Important Issues (1)

• [pr-review-toolkit/code, gstack/review, native-codex] .github/workflows/amt-public-vantage-probe.yml:235-243,360-363 — DISTINCT_SOURCE_IPS is not defined by this workflow's env or inputs, so K always defaults to 1 for normal dispatches. Every requested tunnels > 1 run therefore exits 91 as VOID, making the advertised multi-tunnel harness-ceiling ramp impossible to execute; the new tests only exercise k=2 by injecting an environment variable that the workflow itself never supplies.
  • Make the capability a real, measured/configured workflow input or job environment value, and validate it before allowing a multi-tunnel run. Otherwise remove the multi-tunnel path until the runner can actually present distinct source addresses. Keep the void result for requests exceeding the measured capability.

Suggestions (0)

Strengths

• The aggregate packet-count verdict, single-tunnel legacy aliases, (S,G) receipt guard, fan-out voiding, and bounded tunnels validation address the prior review findings.
• The extracted-shell test covers timeout-with-data, zero-data, malformed receipts, fan-out voiding, aggregate tunnel failures, legacy artifacts, and invalid inputs.
• CI now runs the shell verdict test as a dedicated non-matrix job.

Recommended Action

1. Make the workflow's distinct-source-address capability available and measured before enabling tunnels > 1.
2. Re-run the probe verdict tests and an actual multi-tunnel workflow after that wiring is in place.

[allyblockcast]

@ally please re-review at head 73c16a16ecd554b52f21abd943920517e391fdc7.

On the Important — the observation is right, the remedy is not, and the cause is a version boundary

You are correct on the mechanics: DISTINCT_SOURCE_IPS is set by nothing, so K is always 1, every tunnels > 1 dispatch exits 91 VOID, and the k=2 cases in probe_verdict_test.sh only reach that branch by injecting a variable the workflow never supplies. I am not disputing any of that.

But the VOID is the finding, not a defect — and I verified why from source rather than from my own comment, which turned out to be the thing that was wrong.

linux-amt 7ec53567 (2026-09-13, BLO-33636 / linux-amt#223) changed main to key tunnel state on the (address, port) endpoint per RFC 7450 §4.2.2 — amt_request_handler amt.c:3736, amt_update_handler amt.c:3333. My comment asserted address-only keying as a timeless property of "the relay" and cited line numbers that no longer resolve. That was stale.

It is still operationally correct, because every relay actually deployed is pre-fix — confirmed by ancestry rather than by commit date:

relay	image	vs 7ec53567
prod 69.25.95.128	amtr:sha-f21f60e6	behind_by: 24
staging 69.25.95.96	amtr:sha-d20b2e04	behind_by: 14

So k=1 holds today and the multi-tunnel path is not dead code to delete — it is pending a relay deploy.

On making it an input: I am declining that specifically. A dispatch caller must not be able to declare a fan-out capability the rig and relay do not have; that converts a loud VOID into a clean-looking receipt for fan-out that never happened, which is the exact misattribution this gate exists to prevent.

What changed in 73c16a1

Comment and error-string only — no behaviour change, tests/probe_verdict_test.sh 12/12, YAML parses.

The old comment pointed the consequence the wrong way: it implied the lever was more distinct source addresses at the rig (a runner matrix, a routable v6 /64). It is not. Against a post-fix relay, one runner address presents many endpoints via source ports, so the lever is a relay deploy and the v6 path stops mattering.

⚠ And the ceiling moves rather than disappearing: AMT_MAX_TUNNELS_PER_SOURCE defaults to 16 (amt.h:573; global AMT_MAX_TUNNELS 128). A one-runner ramp against a post-fix relay that read the 17th failure as the relay's fan-out knee would be measuring the per-source cap — the same misattribution as above wearing a different number. That is now recorded at the K= site and in the VOID message, so whoever lifts the gate sets k to the deployed relay's actual cap in the same commit that deploys it.

Review focus, ranked: (1) is the version-scoping accurate against 7ec53567; (2) does the VOID message now give a reader the correct next action; (3) anything in the k=2 test cases that should change given they remain unreachable in production until that deploy.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 73c16a1

73c16a1 is comment + error-string only over 05f59ca2 (one commit, 50+/26-, no behaviour change) — confirmed by compare, not by the description.

Review Focus

(1) Is the version-scoping accurate against 7ec53567? Yes — every load-bearing claim verified from source, not inferred. 7ec53567 is real (2026-09-13T18:26:52Z, "amt: key relay tunnel state on the (address, port) endpoint (BLO-33636) (#223)"). amt.c:3736 lands on if (tunnel->source_port == udph->source) goto send; inside amt_request_handler (def. 3663); amt.c:3333 lands on if (tunnel->source_port != sport) continue; inside amt_update_handler (def. 3282), which carries the RFC 7450 §4.2.2 citation in-source. amt.h:573 is exactly #define AMT_MAX_TUNNELS_PER_SOURCE 16, with AMT_MAX_TUNNELS 128 at :562 and IFLA_AMT_MAX_TUNNELS_PER_SOURCE present in the uapi header. f21f60e6 and d20b2e04 are both strict ancestors of 7ec53567 (ahead_by 24 and 14, behind_by: 0, status: ahead) — no divergence, so the pre-fix claim holds for those two images. Not verified by me: that those digests are what is actually running on .128/.96. I have no reach to those hosts, so the image→host mapping rests on your deployment observation; everything downstream of it checks out.

(2) Does the VOID message give the correct next action? Today yes; in the transitional state it is built for, no. See Suggestion 1.

(3) Should the k=2 cases change? No — keep them exactly as they are. They are the only path to the aggregate-verdict branch, which is live regression cover for the prior:ada0251 important 1 repair, and deleting them because production cannot currently reach that branch would silently retire that cover — the failure mode the file's own header warns about. They become live the moment k is raised, with no test rewrite. The coverage gap is in the k=1 cases instead (Important 1).

Prior Findings Dispositioned (1)

• prior:05f59ca important 1 — no-longer-applicable — .github/workflows/amt-public-vantage-probe.yml:230-263,385 — I am retiring this on verified evidence rather than on a code change, so the basis should be explicit: the mechanics I described are unchanged at this head (K="${DISTINCT_SOURCE_IPS:-1}" at :266 is byte-identical, and every tunnels > 1 dispatch still exits 91). What the finding got wrong was the framing — it treated a permanently-VOID multi-tunnel path as an unintended dead path needing either a real input or removal. The version-scoping block now demonstrates it is a deliberate fail-closed gate pending a relay deploy, and I independently confirmed the two claims that decide it (endpoint keying introduced by 7ec53567; both deployed images strict ancestors of it). Your refusal to make k a dispatch input is also correct and I withdraw that half of the recommendation: a caller able to declare a capability the rig lacks converts a loud VOID into a clean-looking receipt for fan-out that never happened. The other half of my own recommendation — "keep the void result for requests exceeding the measured capability" — is what shipped.

Critical Issues (0)

Important Issues (1)

• [pr-review-toolkit/tests, gstack/review, native-codex] tests/probe_verdict_test.sh:77 — the suite never exercises the production value of k. run_case always exports DISTINCT_SOURCE_IPS=$k (defaulting to 1), so the :-1 fallback at .github/workflows/amt-public-vantage-probe.yml:266 — the only value any real dispatch ever uses, and the single line the whole VOID gate hangs on — is unreachable from the tests. Measured by mutation rather than argued: with :-1 changed to :-99, tests/probe_verdict_test.sh still reports 12/12 ALL PASS, while the same shell run in production shape (variable absent, TUNNELS=2, one receiving tunnel and one not) goes from exit 91 + the VOID message to exit 1 + ::error::1 of 2 tunnel(s) received no data (packet_count=0) — a delivery failure blaming the relay, which is exactly the misattribution the comment at :376-380 says the gate ordering exists to prevent. The gate is correct today and I verified that directly; it is simply undefended, and this is the one line where a silent regression reintroduces the retracted-figure class of error.
  • Add one case that runs with DISTINCT_SOURCE_IPS absent from the environment (e.g. a sentinel k=unset that run_case honours by not exporting it), asserting 91 for TUNNELS=2. That case fails under the mutation above, which is the property the existing twelve lack.

Suggestions (3)

• .github/workflows/amt-public-vantage-probe.yml:385 — the VOID string hard-codes two time-bound claims: "every relay deployed today is pre-fix" and "the lever is a relay deploy carrying 7ec53567". Both go false in precisely the state this PR is shepherding toward. An operator who deploys the fixed relay but has not yet edited this repo dispatches tunnels=2, and is told the deploy they just completed is the remedy — while the action actually required (set k at :266) appears nowhere in the message, only in a shell comment 120 lines up that they never see. It is the same staleness class 73c16a1 just repaired one layer up, re-created in a string. Cheapest form: branch the message on K — at K=1 keep today's text, otherwise say the per-source cap is the binder and name :266 as the site.
• .github/workflows/amt-public-vantage-probe.yml:39-41 — the dispatch-facing description still reads "Ramp this until one of the four candidate binders gives" with no indication that every value > 1 currently VOIDs. That description is the only surface a caller sees in the dispatch form; the gate's rationale lives 190 lines away in shell. One clause ("values > 1 are VOID until a relay carrying 7ec53567 is deployed — see the K= comment") closes the gap.
• tests/probe_verdict_test.sh:35,41-47 — the suite continues when the YAML extraction fails, and the nonzero expectation then passes vacuously. Observed directly: with pyyaml missing, probe.sh was never written and "N=1 unparseable receipt -> red" scored PASS on exit 127 from a missing file. The suite still fails overall so CI stays closed, but a guard case passing for the wrong reason is worth one line — assert probe.sh is non-empty immediately after the heredoc and abort if not.

Strengths

• The version-scoping is the right correction and is sourced properly: it names the commit, the date, the ticket and the PR, pins the line numbers to that commit, and carries a Verified from source 2026-09-18 marker — so the next reader can re-derive it instead of trusting it. Every citation I checked resolved first try.
• The ⚠ AMT_MAX_TUNNELS_PER_SOURCE note anticipates the next misattribution rather than only repairing the last one: reading the 17th failure as the relay's fan-out knee is the identical error wearing a different number, and recording it at the K= site is where the person raising k will actually be standing.
• Redirecting the unblock lever from rig topology (v6 /64, runner matrix) to a relay deploy is the substantive change here, and the v6 telemetry is correctly left in place as capability-only rather than deleted — it stays meaningful against a pre-fix relay.
• Declining the tunnels-declared-k input, and saying so explicitly at :264 with the reason, is the correct call and the right place to record it.
• Baseline suite re-run at this head: 12/12 ALL PASS. Non-Ally CI at this head is green; the only red contexts are the Ally review gates themselves, which this review is what clears.

Recommended Action

1. Add the one unset-DISTINCT_SOURCE_IPS case so the production k default is defended by the suite.
2. Consider the three suggestions opportunistically — the VOID-message branch is the one with a real transitional cost.