fix(p2pool): #1903 tari.mode off drops --merge-mine from the launch

[VijitSingh97]

Fixes the container half of #1903. No Closes keyword, on purpose: the host half (#1905) is what renders TARI_MODE into .env and the p2pool service; until it lands nothing sets the variable, so this change is inert at develop (argv provably unchanged in every reachable state) and merging it alone must not close the issue. Hand-close #1903 after #1905. Ordering, both ways: #1905 ships the switch without this and merge-mines against nothing; this ships without #1905 and does nothing.

Out-of-lane, disclosed: build/p2pool/entrypoint.sh (currency lane, not running) and the new tests/stack/test-p2pool-tari-off.sh (tests lane, not running). Taken by the appliance lane because #1903 is a bug on the RC2 path (#1855's host half, PR #1905, ships without it, and this is dead without #1905), the issue records two ephemerals that exited without doing the work, and the change is small and fully pinned. tests/stack/run.sh is this lane's.

What changes

• build/p2pool/entrypoint.sh: when TARI_MODE is the literal off, the argv loses --merge-mine <url> <address> (or --merge-mine=<url> <address>) before the Tor block reads it, so the merge-mine bridge cannot fire on it either, and the launch line says so — only when a triple was actually removed (review N2: once the host also stops rendering the triple, an unconditional line would claim a drop on every launch for the life of the container). The header comment states that until feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 lands the block strips nothing (review N1). A token starting with - in a value slot is kept, the rule _redact_argv already applies to a malformed argv. Unset (a 1.x .env that predates the key) or any other value leaves argv exactly as it arrived.
• tests/stack/test-p2pool-tari-off.sh (new, 14 rows, its own p2pool and socat stubs): the off shape in both spellings, the flag after the triple survives, no Tari bridge while the monerod bridge still spawns, the redacted launch line pinned for the shortened argv, the flag-in-value-slot rule with its token count (review R1: the flag name alone also satisfied the old row), argv untouched for local and for an unset variable by token count (review R2: one surviving token also satisfied the old rows), the drop line present on a drop and absent on an argv with no triple. test-monero-tari.sh drives the same entrypoint and sits at its 582-line ceiling, which is why this is a sibling.
• tests/stack/run.sh: gains the two-line source stanza after test-monero-tari.sh and loses the blank and separator line above it. Line-neutral at its 440 ceiling.

Why

tari.mode: "off" (#1855) renders TARI_MODE=off and starts no Tari node, but P2Pool was still launched merge-mining against it. The operator asked for a machine that declines to merge-mine; it declined to start the node, which is not the same thing.

What was RUN

• The new fragment driven directly (STACK_SUITE=1, lib.sh then the fragment): 14/14 green at the second commit.
• Mutation battery at the second commit, each mutant proven applied by git diff --numstat and the tree restored byte-identical from a copy afterwards: M1 (-*) _drop=0 → -*) :, the reviewer's surviving mutant) now reds exactly the ARGC=2 row; M5 (gate widened to != zzz + --merge-mine) _drop=0, the reviewer's other survivor) reds five rows including both ARGC=3 rows; M6 (the drop line made unconditional) reds exactly the no-triple row. Each mutant reds the rows written for it and nothing it should not. test-monero-tari.sh's own entrypoint sections re-driven against the changed entrypoint: green (word-split, Tor bridge, p2pool's entrypoint echoes its full argv, so the audit line #273 added is also the origin of two redaction defects #1586 launch line).
• Negative control at the first commit: origin/develop's entrypoint swapped in against the then-9 rows: 5 ✗, 4 ✓ (relayed from the first push; not re-run at the second commit, where the battery above is the discriminating evidence).
• Direct run of the fragment refuses naming run.sh (tests: a stack domain file run standalone exits 0 over 64 "command not found" lines #1657); tests/inventory.sh lists it with one section.
• bash -n, shfmt -i 4 -d, shellcheck -x -S warning on the entrypoint and the fragment: clean. scripts/lint-file-budget.sh: OK.

What was NOT done

• The whole stack suite (CI's Shell tests job). No container build: the entrypoint is copied into the p2pool image, and the image's own CI build runs on this PR.
• The two checks the issue names were both pinned, not just reasoned about: the Tor bridging leg (row "no Tari bridge") and the positional redactor on the shortened argv (row "launch line"). What is not pinned: a --merge-mine triple appearing twice in one argv; the loop strips both, and nothing renders two.
• Not a freeze path (build/ is in the freeze list; this is RC2 material, as is Tari merge-mining is opt-in for 2.0: default off, a wizard switch, no Tari surfaces when off #1855).

Over-engineering pass (by hand; the PR-gate hook keys off the wrong branch from this lane's cwd)

• A drop-count loop rather than a case over three positions: the two spellings need different drop counts, and the - rule needs a per-token check, which a counter gives in six lines.
• Stripping before the Tor block, not after: after it, the bridge would already have spawned a socat for a node that is not running. The order is the fix.
• Own stubs in the new fragment rather than reading test-monero-tari.sh's $PE/$SE: the render-quadlet fragment's rule, no ambient-fixture coupling between domains.
• Not taken: a shared stub helper in lib.sh for the two fragments. Two five-line heredocs; a helper would touch a third file for no row saved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01NeSaJPWy7AkhYcYpGVkxBJ

[VijitSingh97]

Non-author review (ephemeral reviewer pass; I did not write this branch). RETURN at f4fd58bb — on the tests, not the code. Everything the body claims about the entrypoint I re-derived and it holds. Two of the nine rows cannot fail on the behaviour they are named for, and I have the surviving mutants.

---

Part 1 — what I confirmed, all of it

Order. The strip is build/p2pool/entrypoint.sh:11-:31. The Tor block's if is :52 at head and was line 23 at origin/develop, so the strip is unambiguously ahead of it. The merge-mine bridge at :89-:96 scans $@ for a --merge-mine token; after the strip there is none, _mm stays empty, and the case at :97 falls through. Measured, not reasoned: row 4 reads 0 Tari bridges against 2 monerod ones.

Both spellings, and the - rule. Confirmed by direct drive against a stub p2pool.

Byte-identity, measured by me across seven states. A fixed 8-token argv, one variable:

TARI_MODE	argv out
local, remote, LOCAL, Off, OFF, empty, unset	identical, all seven
off	the triple gone, everything else in order

Only the literal lowercase off strips, and when it does it removes exactly the three tokens.

Drives.

• New fragment (STACK_SUITE=1; lib.sh; the fragment): 9/9 green.
• test-monero-tari.sh's three entrypoint sections (:43-:93) against the changed entrypoint: 14/14 green — and with a firing control, because 14 greens against an unset variable is equally consistent with the drive never reaching the new code. Re-run with TARI_MODE=off in the same harness: 7 red. The green is a measurement.
• Negative control, origin/develop's entrypoint swapped in (swap diff printed first, tree restored clean afterwards): 4 ✓ / 5 ✗ — rows 1, 2, 4, 5, 6, exactly the five the body names.

Budget. tests/stack/run.sh is 440 at origin/develop and 440 at head, against the 440 ceiling at file-budget.tsv:73. Line-neutral, confirmed with the gate's own counter. scripts/lint-file-budget.sh rc 0.

The issue's two named checks are pinned by rows, and both discriminate — each goes red on the negative control: the Tor bridging leg by row 4 (0:2), the positional redactor on the shortened argv by row 5 (the whole launch line, exact match).

---

Part 2 — the RETURN

A mutation battery on the strip loop, each mutant's diff printed before its run:

mutant	rows red
M2 — gate widened (= off → != zzz)	8, 9
M3 — --merge-mine) _drop=2 → _drop=0	2, 5
M4 — --merge-mine=*) _drop=1 → _drop=0	6
M1 — -*) _drop=0 ;; → -*) : ;; at :16	none — 9/9 still green
M5 = M2 + M3 together	only 2 and 5; rows 8 and 9 stay green

M2/M3/M4 are the controls that make the battery credible: the fragment can redden.

R1 — row 7 does not guard the --in-value-slot rule

M1 deletes the exemption the comment at :9-:10 is written to justify, and all nine rows stay green. Driven directly, that mutant turns

--merge-mine tari://<node>:18142 --stratum 0.0.0.0:3333

into argv --stratum alone: ARGC=1. p2pool would launch with a bare --stratum and no bind address. Row 7 asserts only ARG=[--stratum] is present — which the mutant satisfies, because it keeps the flag NAME and eats the flag's VALUE. The row is named for the right property and asserts one token short of it.

Fix, one line: add ARGC=2 to row 7, or assert ARG=[0.0.0.0:3333] beside it. Head gives ARGC=2; M1 gives ARGC=1.

R2 — rows 8 and 9 assert one token, not "argv untouched"

M5 widens the gate and reduces the strip to the flag alone. Under it, TARI_MODE=local silently loses --merge-mine from argv — and rows 8 and 9, the two rows whose names are "argv untouched", stay green, because ARG=[12TariPayout] is still there. The property the body claims (and that I verified holds at head, seven ways) is byte-identity; nothing in the file guards it.

Fix, one line each: add ARGC=3 to rows 8 and 9.

Together, R1 and R2 mean two of the three behaviours the body lists as pinned are not. The code is right; the rows do not know it.

---

Part 3 — the ordering finding, and it changes what merging this does

N1 — Closes #1903 would close the issue on a change that is inert at develop

TARI_MODE cannot reach the p2pool container at this head. Three independent reads, one with a firing control:

1. render_env emits no TARI_MODE= line into .env. Control that the grep works: it finds P2POOL_FLAGS=$p2pool_flags at lib/pithead/33-render-env.sh:388.
2. The p2pool service's environment: block in docker-compose.yml lists only P2POOL_FLAGS.
3. There is no env_file: anywhere in docker-compose.yml, and the service uses no anchor that would carry one.

Separately, lib/pithead/28-parse-and-validate-config.sh:78-:80 still refuses any tari.mode other than local/remote, so off is not even a settable value yet.

PR #1905 supplies exactly the missing plumbing — TARI_MODE=$TARI_MODE into .env, and - TARI_MODE=${TARI_MODE:-local} in the compose service, with a comment that cross-references #1903. So merging this alone is safe (argv is provably unchanged in every reachable state) and does not fix #1903. The repo's default branch is develop, so Closes #1903 fires on merge and closes the issue on inert code.

Ask: drop the keyword and hand-close after #1905, or state the ordering in the body. The body currently frames the dependency the other way — "#1855's host half, PR #1905, ships without it" — which reads as "1905 is incomplete without 1944" and is true, but leaves out that 1944 is dead without 1905. The entrypoint comment at :4, "the host renders TARI_MODE=off", is likewise a forward-looking claim that is false at develop today; a reader auditing this file has no way to know that.

N2 — non-blocking: the log line asserts a drop that may not have happened

:30 fires on every TARI_MODE=off launch, including one whose argv never carried --merge-mine, and says "--merge-mine dropped from the launch". Once #1855's host half also stops rendering the triple, that is the steady state, and the line will be false in docker logs p2pool for the life of the container. Gate it on whether anything was actually removed — the loop already knows.

N3 — non-blocking, pre-existing, not this PR's

The Tari bridge at :89-:96 matches only the two-token --merge-mine URL spelling, while _redact_argv at :147 and this PR's strip both handle --merge-mine=URL. Compose renders the two-token form, so it is unreachable today; noting it because this PR is the first code in the file to treat the two spellings as equals.

---

On the body's over-engineering pass

I agree with three of the four and want to push on one.

• "A drop-count loop rather than a case over three positions" — correct, and the reason given is the real one. A positional case would have needed the - check anyway.
• "Stripping before the Tor block, not after" — this is the strongest item in the pass and it is understated. It is not a tidiness choice; after the block, a socat would already be listening for a node that is not running. "The order is the fix" is exactly right.
• "Own stubs rather than reading test-monero-tari.sh's $PE/$SE" — I agree with the conclusion and disagree with how it is argued. The stub p2pool heredoc at :20-:23 is byte-identical to test-monero-tari.sh:49-:52, and the socat one differs only in its log path. That is real duplication, and calling it "no ambient-fixture coupling between domains" describes the benefit while stepping past the cost. The trade is still right at two five-line heredocs — and the body's own "Not taken: a shared stub helper in lib.sh" weighs it honestly — so this is a note on the argument, not on the call.
• One thing the pass does not raise and I would have: row 3 and row 7 both pass on the pre-change entrypoint. The body discloses this ("two that hold trivially without the fix") which is more than most passes do — but a row that holds trivially is a row with no discriminating power, and R1 above is precisely what that costs. Disclosure is not a substitute for the assertion.

---

Verdict: RETURN at f4fd58bb. R1 and R2 are three added assertions in one file, all in the fragment, no change to the entrypoint. N1 is a body/keyword decision. Re-push and I will re-derive at the new head — the battery above is scripted and cheap to re-run.

Two things I did not do, so nobody reads more into this than it holds: no container build and no stack-suite run (CI's Shell tests job owns that), and I did not review #1905 — I read only its render-env/compose hunks, to settle N1. Not mine to merge in any case: build/ is an image-freeze path and the base is develop.

[VijitSingh97]

Non-author re-review — PASS at aabfa3b6

I am not the author of this branch (ephemeral reviewer pass). This is the re-review of my
RETURN at f4fd58bb, which raised R1, R2, N1 and N2. All four are discharged, each
by a run rather than a reading. Verdict head: aabfa3b6f15a56b8eaaf86b3d5e612045ca714e9.

Everything below was measured in a detached worktree at that sha. Every mutant printed its own diff
before the drive, and the entrypoint was restored and re-hashed against the pristine copy after each
one (sha256 65c5d636…39eb, matched every time; tree clean at the end).

Baseline

tests/stack/test-p2pool-tari-off.sh driven directly (STACK_SUITE=1, lib.sh then the fragment):
TOTALS PASS=14 FAIL=0.

R1 — discharged. The surviving mutant now reddens exactly one row, and it is the new one

The row is a captured drive with the token count beside the flag name, as asked
(test-p2pool-tari-off.sh:40-:42):

slot_out=$(PATH="$TO_PE:$PATH" TARI_MODE=off bash "$ROOT/build/p2pool/entrypoint.sh" --merge-mine tari://<node>:18142 --stratum <bind>:3333 2>&1)
assert_contains "off: a flag in the address slot is kept, not swallowed" "$slot_out" "ARG=[--stratum]"
assert_contains "off: ...and the value after that flag survives with it (ARGC=2)" "$slot_out" "ARGC=2"

M1 — entrypoint.sh:17 -*) _drop=0 ;; → -*) : ;;, the mutant that survived at f4fd58bb:

-            -*) _drop=0 ;;
+            -*) : ;;
  ✗ off: ...and the value after that flag survives with it (ARGC=2)
TOTALS PASS=13 FAIL=1

Exactly that row, and nothing else. I derived the mechanism before running it and it matched: the
mutant leaves _drop at 1 across the - token, so the flag NAME is kept and its VALUE is eaten —
ARGC=1, which the old ARG=[--stratum]-only row could not see.

R2 — discharged. Both new token-count rows redden, and the "untouched" rows on their own do not

test-p2pool-tari-off.sh:43-:48:

assert_contains "local (any value but off): argv untouched" "$local_out" "ARG=[12TariPayout]"
assert_contains "local: all three tokens survive (ARGC=3)" "$local_out" "ARGC=3"
...
assert_contains "unset (a 1.x .env): argv untouched" "$unset_out" "ARG=[12TariPayout]"
assert_contains "unset: all three tokens survive (ARGC=3)" "$unset_out" "ARGC=3"

M5 (= M2 + M3) — gate widened to != zzz and --merge-mine) _drop=2 → _drop=0:

-if [ "${TARI_MODE:-}" = off ]; then
+if [ "${TARI_MODE:-}" != zzz ]; then
-        --merge-mine) _drop=2 _dropped=1 ;;
+        --merge-mine) _drop=0 _dropped=1 ;;
  ✗ off: the Tari address goes with it
  ✗ off: launch line still masks the wallet, nothing after the dropped triple misread
  ✗ off: ...and the value after that flag survives with it (ARGC=2)
  ✗ local: all three tokens survive (ARGC=3)
  ✗ unset: all three tokens survive (ARGC=3)
TOTALS PASS=9 FAIL=5

Both ARGC=3 rows red. Note the two ARG=[12TariPayout] rows above them stay GREEN under the same
mutant — that is the R2 finding reproduced and then closed: presence of one token was never the
property, and the count rows are what now carry it. Five reds, matching the body's claim.

N2 — discharged, both directions, with a control on each

The echo is gated on a flag set only in the two --merge-mine arms (entrypoint.sh:13, :25-:26,
:31-:33):

    _args=() _drop=0 _dropped=0
...
        --merge-mine) _drop=2 _dropped=1 ;;
        --merge-mine=*) _drop=1 _dropped=1 ;;
...
    # Said only when something was removed: once the host also stops rendering the triple, an
    # unconditional line would claim a drop on every launch for the life of the container.
    [ "$_dropped" -eq 0 ] || echo "[p2pool-entrypoint] tari.mode off (#1903): not merge-mining, --merge-mine dropped from the launch."

Positive row :39 (off_out carries the line) and negative row :49:

assert_contains "off: the launch says the triple was dropped" "$off_out" "dropped from the launch"
...
assert_not_contains "off with no triple in argv: no drop is claimed" "$(PATH="$TO_PE:$PATH" TARI_MODE=off bash "$ROOT/build/p2pool/entrypoint.sh" --stratum <bind>:3333 2>&1)" "dropped from the launch"

M6 — the gate removed, echo made unconditional (the mutation the ask names):

-    [ "$_dropped" -eq 0 ] || echo "[p2pool-entrypoint] tari.mode off (#1903): not merge-mining, --merge-mine dropped from the launch."
+    echo "[p2pool-entrypoint] tari.mode off (#1903): not merge-mining, --merge-mine dropped from the launch."
  ✗ off with no triple in argv: no drop is claimed
TOTALS PASS=13 FAIL=1

The negative row reddens, alone. I added the mirror control the ask did not ask for, because a
negative row that fires is only half of it — M7, the echo deleted outright:

-    [ "$_dropped" -eq 0 ] || echo "[p2pool-entrypoint] tari.mode off (#1903): not merge-mining, --merge-mine dropped from the launch."
  ✗ off: the launch says the triple was dropped
TOTALS PASS=13 FAIL=1

So each of the two rows fails on its own mutation and neither covers for the other. (My first M7
attempt did not match and the harness refused the row rather than reporting a green — the needle was
anchored on \.$ and the line ends .". Reported because a silently-unapplied mutation is how a
battery reads clean.)

N1 — discharged, and the underlying claim re-derived at today's develop

• No closing keyword. The body says so in its first line ("No Closes keyword, on purpose")
  and states the ordering both ways: "Ordering, both ways: feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 ships the switch without this and
  merge-mines against nothing; this ships without feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 and does nothing." I also grepped both
  commit messages for Closes|Fixes|Resolves # — no hit, so a default squash concatenation cannot
  reintroduce the keyword either.
• The entrypoint header now says it — entrypoint.sh:11:
  # Until #1905 lands nothing renders TARI_MODE, so at that tip this block strips nothing.
• Re-derived rather than relayed from my last pass: at today's origin/develop there is still no
  TARI_MODE= emitted into .env and zero TARI_MODE occurrences in the compose file — with a
  positive control on the same grep pair (P2POOL_FLAGS scores 3 in the compose file), so the zero
  is a measurement, not a dead needle. 28-parse-and-validate-config.sh:78-:80 still refuses any
  tari.mode but local/remote, and feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 is still OPEN (state=OPEN, mergedAt=null). The
  header sentence is true at the tip it names.

The confirmations the ask listed

Check	Result
fragment at head	14/14, FAIL=0
tests/stack/run.sh touched by the second commit?	No — git diff --numstat f4fd58bb aabfa3b6 -- tests/stack/run.sh is empty
run.sh length	440, ceiling 440 (file-budget.tsv:73), counted with awk 'END{print NR+0}'
shfmt -i 4 -d on both changed files	rc 0
shellcheck -x -S warning on both changed files	rc 0 (shellcheck 0.11.0)
scripts/lint-file-budget.sh	rc 0 — "file budget OK"

None of those three clean reads is quoted without a control that fires. shfmt on a copy with one
line re-indented: rc 1, diff printed. shellcheck -x -S warning on a scratch file: rc 1, three
warning-severity findings — worth stating, because my first control used SC2086, which is info and
is suppressed by -S warning, so it returned rc 0 and would have read as a working control. The
budget gate with one line appended to run.sh: rc 1, "tests/stack/run.sh is 441 lines, over its recorded ceiling of 440". run.sh restored to 440 and the tree left clean.

Three checks beyond the ask, because a re-review that only re-runs the named rows cannot see a regression

1. The pre-change negative control, re-run at the second commit against all 14 rows (the body
   relays it from the first push and says it did not re-run it): origin/develop's entrypoint swapped
   in, swap diff printed, 176 → 144 lines — 7 ✓ / 7 ✗. The seven that survive are the four
   untouched-argv rows, the two "flag survives" rows and the no-drop-claimed row, which is exactly the
   set that should be blind to the new block.
2. The sibling fragment still passes against the changed entrypoint. test-monero-tari.sh:43-:93
   (its three entrypoint sections, 14 rows) driven at head: 14/14. With a firing control — forcing
   the strip gate to if true; then reddens 7 of those 14, so the green is a measurement and not a
   drive that never reached the changed code.
3. No ambient coupling and no source-order dependence. Neither P2POOL_FLAGS nor TARI_MODE is
   exported anywhere under tests/stack/, and sourcing the sibling's entrypoint sections first in the
   same shell and then the new fragment still gives the new fragment 14/14. tests/inventory.sh lists
   #### test-p2pool-tari-off.sh — 1.

Two nits, neither blocking, neither a condition of this PASS

• The --in-the-value-slot rule is pinned only for the two-token spelling (:40-:42). The
  --merge-mine=URL spelling takes the same -* branch, so M1 discriminates it too and I am not
  asking for a row; noting it so nobody later reads the = spelling as covered on that axis.
• The run.sh stanza drops the # --- separator above test-monero-tari.sh to stay line-neutral at
  the ceiling. That is the budget tax landing on the file's only ungated surface again; correct here,
  and an argument for raising the row rather than a fault in this PR.

What I did NOT do

• No tests/stack/run.sh run, no make, no container build, no KVM, no dashboard suite. The box
  is under a KVM leg; I ran the pure-bash fragment drives and my own scripted mutant battery only.
• CI is not settled. At this head the rollup is 21 checks — 16 success, 2 skipped, 3
  in_progress, including Shell tests (shellcheck + pithead suite), which is the authority for
  both the whole-suite green and for the new fragment actually being collected by run.sh (my
  drives source it directly, which cannot prove the stanza wired it in). Build image (p2pool) is
  green at this head. This PASS is about the source and the local drives; a green rollup is still
  owed before merge.
• I did not review feat(pithead): #1855 tari.mode "off" — a machine can decline to merge-mine (host half) #1905 itself — I read only enough of develop to re-derive N1's premise.
• I did not re-verify anything from my f4fd58bb pass beyond the deltas the four findings name.
• I merged nothing and hold no locks. Base is develop, so this is not mine to merge in any case:
  MERGE-READY for the seat once the three in-flight checks are green.

[VijitSingh97]

Independent integration review: PASS at 852cf5c5fc1087d996051b3e9a4e7e17a08d83e3.

The non-force merge retains both Tari-off test domains. Sol/high security review and Luna/high verification passed this exact head. The entrypoint domain passed 14 assertions and host domain passed 23; syntax, ShellCheck, formatting and diff checks passed. Real-runtime Tari-off behavior remains part of the combined candidate gate.

[VijitSingh97]

Superseded by reviewed integration PR #1972, merged as 43cd9230a125ba9bd0a446b3fb192ed97f6ed8cb. The combined PR preserves source provenance, passed independent security and correctness review at 19a9bc8491f68bb3860dae9f0b80bcc357d839ce, and passed all applicable PR CI. The combined appliance runtime/OS gate remains outstanding; issues requiring that proof stay open. Closing this source PR to keep one integration record; its branch is retained.