HOMELAB-2682: feat(sbx-gits): documented PRECLEAN=skip opt-out in emu smoke #847

Merged

pi-landing merged 2 commits from plane/HOMELAB-2682-preclean-optout into live 2026-11-05 16:48:35 +00:00

Conversation 4 Commits 2 Files changed 4 +552 -13

pi-landing commented 2026-11-05 15:56:57 +00:00

Collaborator

Copy link

The need (quoted, not narrowed)

From gits-hypervisor:

gits-sbx-emu-smoke.sh needs a documented opt-out of PRE-CLEAN (for example PRECLEAN=skip) for runs where the app under test keeps non-exportable state in AndroidKeyStore: uninstall before install destroys the hardware-backed identity, so the guest key cannot be enrolled between install and test. Today GITS-493 uses a suite under the proof tool instead.

The smoke's PRE-CLEAN (R171) uninstalls <app><suffix> and <app><suffix>.test immediately
before Gradle installs. That is correct for an app with no persistent state of its own, and it
stays the default — but for an app that keeps its identity in AndroidKeyStore, the uninstall
takes the hardware-backed key with it, and the guest key cannot be enrolled between install and
test. That suite ends up measuring a freshly cleaned emulator without the identity it needs, so
today its consumer dodges the smoke with its own suite under the proof tool.

The change

core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh only:

• New env opt-out, one exact value: PRECLEAN=skip.
• When skipped: no uninstall — the connected test installs over whatever is already on
  the emulator (the install keeps its flags; only the uninstall step is bypassed) — and the run
  prints one line saying so:
  PRE-CLEAN omitido por env: PRECLEAN=skip (no se desinstala nada, se instala ENCIMA; AndroidKeyStore no exportable: el uninstall destruye la identidad respaldada por hardware y el guest key no se puede inscribir entre install y test)
  so captures (and therefore the ledger artifact) cannot pretend the emulator was cleaned.
• Recorded as PRE_CLEAN_ST=SKIP, which already means "no cleaning happened here": it is not a
  failure, does not colour connected-test, does not change rc, and does not add a fourth
  CHECK INFRA line.
• Documentation: the script's header block and its --help; docs/gits-sbx/t7-emu-smoke.md
  (the phases section, with the why and the safety framing) and
  docs/gits-sbx/run-proof.md (the proof-tool interaction).
• Harness: scripts/gits-sbx-epic-test.sh §23 runs the real extracted smoke code against a
  stateful fake adb: default uninstalls; PRECLEAN=skip does not and the state survives the
  install; Skip / SKIP / 1 / yes keep the default. Two executed red mutations (a copy
  with no opt-out; a copy with a lax ${PRECLEAN,,} comparison that would accept Skip) must
  behave differently, so the green is not an assertion nobody has seen fail.
• T7[diff] / 17[R159] (d) pinned set updated to what this branch actually changes against
  origin/live (M tools/gits-sbx-emu-smoke.sh; the docs/harness live outside that prefix).
  kustomization.yaml and statefulset.yaml untouched.

Default unchanged

• Unset, or any other value, runs the current path: PRE-CLEAN stays ON, same output, same
  rc, same summary. The comparison is literal (== skip), so a typo cannot disable it.
• The CLEAN at the end (the cleaning for the next epic) is untouched. This is an opt-out for
  suites that need state to survive the install, not a way to skip cleaning between epics — and
  when it is used, the capture records that the uninstall did not happen.

Proof tool non-interaction

• gits-sbx-emu-proof.sh is not modified (and T7[diff] would go red if it were).
• No new ledger field, no capture-format change, no check semantic change: the only trace is
  the smoke's own warning line inside stdout, which lands in the capture and therefore in
  output_sha256. check does not look at PRECLEAN, and should not.
• run launches the suite in a subshell with the same environment, so the variable is
  inherited; a default run (no variable) executes exactly the same smoke as before.

Verification

• bash -n on the smoke, the harness and the (untouched) proof tool: clean.
• bash scripts/gits-sbx-epic-test.sh run solo (never concurrent — fixed /tmp paths):
  RESULT: FAIL (3 of 593 checks, 6 skipped, 0 pending) → 590 pass, 3 red, 6 skip. All
  three reds are pre-existing and their inputs are untouched here: the known
  probe uses an EMPTY pre-existing --dir red, and manifest[3] / manifest[3b], whose
  expectation literal (platform/sbx-fleet:eebdbe1@sha256:7e0513ca…) appears 3× in
  origin/live's own harness while origin/live's own statefulset.yaml carries
  6fa150a@sha256:0ee5e97… (PR #842) — git diff origin/live -- core/manifests/apps/sbx-gits/{statefulset,kustomization}.yaml
  is empty here.
• T7[diff] and 17[R159] (d) pass on the re-measured set (M tools/gits-sbx-emu-smoke.sh),
  and the set bites (the red mutation that removes the entry goes red).
• New harness §23 (real extracted smoke code + stateful fake adb, no pod): default →
  uninstall before install, PRE_CLEAN_ST=OK, connected-test: OK; PRECLEAN=skip →
  zero uninstall actions, install-over, state preserved, one marker line,
  PRE_CLEAN_ST=SKIP, connected-test: OK; Skip / SKIP / 1 / yes → default path.
  Both red mutations execute as designed. 21 checks, 0 fails. Existing §19[R171] re-run:
  17 checks, 0 fails.
• CI on the head: runs #3138 / #3139, every job success.

Files

M core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh, M docs/gits-sbx/run-proof.md,
M docs/gits-sbx/t7-emu-smoke.md, M scripts/gits-sbx-epic-test.sh. kustomization.yaml,
statefulset.yaml and gits-sbx-emu-proof.sh untouched.

## The need (quoted, not narrowed) From gits-hypervisor: > gits-sbx-emu-smoke.sh needs a documented opt-out of PRE-CLEAN (for example PRECLEAN=skip) for runs where the app under test keeps non-exportable state in AndroidKeyStore: uninstall before install destroys the hardware-backed identity, so the guest key cannot be enrolled between install and test. Today GITS-493 uses a suite under the proof tool instead. The smoke's `PRE-CLEAN` (R171) uninstalls `<app><suffix>` and `<app><suffix>.test` immediately before Gradle installs. That is correct for an app with no persistent state of its own, and it stays the default — but for an app that keeps its identity in **AndroidKeyStore**, the uninstall takes the hardware-backed key with it, and the guest key cannot be enrolled between install and test. That suite ends up measuring a freshly cleaned emulator without the identity it needs, so today its consumer dodges the smoke with its own suite under the proof tool. ## The change `core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh` only: - New env opt-out, **one exact value**: `PRECLEAN=skip`. - When skipped: **no uninstall** — the connected test installs **over** whatever is already on the emulator (the install keeps its flags; only the uninstall step is bypassed) — and the run prints **one** line saying so: `PRE-CLEAN omitido por env: PRECLEAN=skip (no se desinstala nada, se instala ENCIMA; AndroidKeyStore no exportable: el uninstall destruye la identidad respaldada por hardware y el guest key no se puede inscribir entre install y test)` so captures (and therefore the ledger artifact) cannot pretend the emulator was cleaned. - Recorded as `PRE_CLEAN_ST=SKIP`, which already means "no cleaning happened here": it is not a failure, does not colour `connected-test`, does not change `rc`, and does not add a fourth `CHECK INFRA` line. - Documentation: the script's header block and its `--help`; `docs/gits-sbx/t7-emu-smoke.md` (the phases section, with the *why* and the safety framing) and `docs/gits-sbx/run-proof.md` (the proof-tool interaction). - Harness: `scripts/gits-sbx-epic-test.sh` §23 runs the **real extracted** smoke code against a stateful fake `adb`: default uninstalls; `PRECLEAN=skip` does not and the state survives the install; `Skip` / `SKIP` / `1` / `yes` keep the default. Two **executed red mutations** (a copy with no opt-out; a copy with a lax `${PRECLEAN,,}` comparison that would accept `Skip`) must behave differently, so the green is not an assertion nobody has seen fail. - `T7[diff]` / `17[R159] (d)` pinned set updated to what this branch actually changes against `origin/live` (`M tools/gits-sbx-emu-smoke.sh`; the docs/harness live outside that prefix). `kustomization.yaml` and `statefulset.yaml` untouched. ## Default unchanged - Unset, or **any** other value, runs the current path: `PRE-CLEAN` stays ON, same output, same rc, same summary. The comparison is literal (`== skip`), so a typo cannot disable it. - The `CLEAN` at the end (the cleaning for the *next* epic) is untouched. This is an opt-out for suites that need state to survive the install, not a way to skip cleaning between epics — and when it is used, the capture records that the uninstall did not happen. ## Proof tool non-interaction - `gits-sbx-emu-proof.sh` is **not modified** (and `T7[diff]` would go red if it were). - No new ledger field, no capture-format change, no `check` semantic change: the only trace is the smoke's own warning line inside stdout, which lands in the capture and therefore in `output_sha256`. `check` does not look at `PRECLEAN`, and should not. - `run` launches the suite in a subshell with the same environment, so the variable is inherited; a default `run` (no variable) executes exactly the same smoke as before. ## Verification - `bash -n` on the smoke, the harness and the (untouched) proof tool: clean. - `bash scripts/gits-sbx-epic-test.sh` run **solo** (never concurrent — fixed /tmp paths): `RESULT: FAIL (3 of 593 checks, 6 skipped, 0 pending)` → **590 pass, 3 red, 6 skip**. All three reds are pre-existing and their inputs are untouched here: the known `probe uses an EMPTY pre-existing --dir` red, and `manifest[3]` / `manifest[3b]`, whose expectation literal (`platform/sbx-fleet:eebdbe1@sha256:7e0513ca…`) appears 3× in `origin/live`'s own harness while `origin/live`'s own `statefulset.yaml` carries `6fa150a@sha256:0ee5e97…` (PR #842) — `git diff origin/live -- core/manifests/apps/sbx-gits/{statefulset,kustomization}.yaml` is empty here. - `T7[diff]` and `17[R159] (d)` pass on the re-measured set (`M tools/gits-sbx-emu-smoke.sh`), and the set bites (the red mutation that removes the entry goes red). - New harness §23 (real extracted smoke code + stateful fake `adb`, no pod): default → uninstall **before** install, `PRE_CLEAN_ST=OK`, `connected-test: OK`; `PRECLEAN=skip` → zero uninstall actions, install-over, state preserved, one marker line, `PRE_CLEAN_ST=SKIP`, `connected-test: OK`; `Skip` / `SKIP` / `1` / `yes` → default path. Both red mutations execute as designed. 21 checks, 0 fails. Existing §19[R171] re-run: 17 checks, 0 fails. - CI on the head: runs #3138 / #3139, every job success. ## Files `M core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh`, `M docs/gits-sbx/run-proof.md`, `M docs/gits-sbx/t7-emu-smoke.md`, `M scripts/gits-sbx-epic-test.sh`. `kustomization.yaml`, `statefulset.yaml` and `gits-sbx-emu-proof.sh` untouched.

pi-landing added 1 commit 2026-11-05 15:56:57 +00:00

HOMELAB-2682: feat(sbx-gits): PRECLEAN=skip opt-out for AndroidKeyStore state ... 505f3b994b

The emu smoke's PRE-CLEAN (R171) uninstalls the variant's packages right
before Gradle installs. That is right for an app with no persistent state
of its own, and it stays the default -- but an app whose state is NOT
exportable keeps its identity in AndroidKeyStore, and the uninstall takes
the hardware-backed key with it. The guest key cannot be enrolled between
install and test, so the PRE-CLEAN leaves that suite measuring a clean
emulator without the identity it needs.

PRECLEAN=skip (literal value) skips only the uninstall: the app is
installed OVER whatever is already there, the install keeps its flags, and
the run prints ONE line saying pre-clean was skipped by env (so the capture
and the ledger artifact cannot pretend the emulator was cleaned). Unset or
any other value -- Skip, SKIP, 1, yes -- runs the current path unchanged;
the CLEAN at the end is untouched.

Docs: the smoke's header + --help, docs/gits-sbx/t7-emu-smoke.md (why, and
the safety framing: default stays ON) and docs/gits-sbx/run-proof.md (the
proof tool is not touched: no ledger field, capture format or check
semantics change). Harness: section 23 runs the REAL extracted smoke code
against a stateful fake adb -- default uninstalls, PRECLEAN=skip does not
and survives the install, four typos keep the default -- with two executed
red mutations (no opt-out; a lax comparison that would accept Skip).

aaron added 1 commit 2026-11-05 16:09:34 +00:00

HOMELAB-2682: test(sbx-gits): fix the R171 fixture extract + assert the real ordering ... f9870ce020

Two things the first harness run found, both in the harness, not in the
smoke:

- 19[R171] extracts the REAL clean/pre-clean code and runs it under
  `set -u`; phase_connected_test now reads PRECLEAN, so the extractor also
  pulls the real `PRECLEAN=` default line from the smoke (and refuses the
  extraction without it) instead of the fixture blowing up on an unbound
  variable.
- the R171 red mutation deletes the `clean_variant_packages 'PRE-CLEAN'`
  call with a sed anchored to its old 2-space indent; the call now lives
  one level deeper inside the opt-out's else branch. The deletion is
  indentation-agnostic, so it still mutilates THE CALL and not whitespace.

Section 23's own assertions: assert the ORDER (uninstall before install,
which is the point of R171 — the package is installed again by the
install), and count the opt-out marker in the run that has the opt-out
rather than in a later typo case.

pi-landing commented 2026-11-05 16:24:10 +00:00

Author

Collaborator

Copy link

Verification (head f9870ce, branch run solo)

bash -n: gits-sbx-emu-smoke.sh, scripts/gits-sbx-epic-test.sh,
gits-sbx-emu-proof.sh (untouched) — all clean.

bash scripts/gits-sbx-epic-test.sh (run alone, never concurrent — fixed /tmp paths):
RESULT: FAIL (3 of 593 checks failed, 6 skipped, 0 pending) → 590 pass, 3 red, 6 skip.
All three reds are pre-existing and none of their inputs is touched by this branch:

red	why it is not this branch
probe uses an EMPTY pre-existing --dir and leaves it in place (rc=2)	the known pre-existing red, red before this branch
manifest[3] imagenes = sbx-fleet@sha256:7e0513ca…	the check's own expectation literal (platform/sbx-fleet:eebdbe1@sha256:7e0513ca…) appears 3× in origin/live's own harness while origin/live's own statefulset.yaml carries 6fa150a@sha256:0ee5e97… (PR #842's pin). Live's harness already disagrees with live's manifest; git diff origin/live -- core/manifests/apps/sbx-gits/{statefulset,kustomization}.yaml is empty here and this branch adds 0 occurrences of that literal
manifest[3b] bump de imagen H5 (falta: …)	same stale pin, same evidence

The two guards named in the brief still pass, on the re-measured set:

PASS  T7[diff] sbx-gits cambia contra origin/live SOLO en lo esperado: M tools/gits-sbx-emu-smoke.sh (…)
PASS  17[R159] (d) T7[diff] sbx-gits cambia contra origin/live SOLO en lo esperado: M tools/gits-sbx-emu-smoke.sh (…)
PASS  17[R159] (d) mutacion roja: quitado 'M tools/gits-sbx-emu-smoke.sh' del conjunto permitido, ROJO (el conjunto muerde)

New §23 (real extracted smoke code + stateful fake adb, no pod):

PASS  23[HOMELAB-2682] bash -n del smoke
PASS  23[HOMELAB-2682] codigo REAL extraido del smoke (phase_connected_test ENTERO + la linea del DEFAULT de PRECLEAN)
PASS  23[HOMELAB-2682] script: el aviso del opt-out sale UNA sola vez en la corrida
PASS  23[HOMELAB-2682] la herramienta de prueba NO conoce PRECLEAN (linea, campos y check del ledger no cambian)
PASS  23[HOMELAB-2682] mutacion roja EJECUTADA 1: copia SIN el opt-out, con PRECLEAN=skip, desinstala igual
PASS  23[HOMELAB-2682] mutacion roja EJECUTADA 2: copia con comparacion LAXA se salta el PRE-CLEAN con 'Skip'
PASS  23[HOMELAB-2682] el smoke gana UN opt-out documentado … (21 checks, 0 fallos)

Cases actually executed: default (PRECLEAN unset) → uninstall before install, PRE_CLEAN_ST=OK,
connected-test: OK; PRECLEAN=skip → zero uninstall actions, install-over, state survives,
one marker line, PRE_CLEAN_ST=SKIP, connected-test: OK; Skip / SKIP / 1 / yes → the
default path, PRE_CLEAN_ST=OK.

R171 section re-run after the fixture fix (unchanged semantics): 17 checks, 0 fails — the
existing PRE-CLEAN green/mutation proofs still bite (its red mutation now deletes the call
independently of indentation).

CI on this head: run #3138 and #3139 — all jobs success (shellcheck, terraform-validate,
yaml-lint, helm-validate, runner-manifest-test, record-sql-test, dispatcher/cockpit/receiver…).

Files vs origin/live: M core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh,
M docs/gits-sbx/run-proof.md, M docs/gits-sbx/t7-emu-smoke.md,
M scripts/gits-sbx-epic-test.sh. kustomization.yaml, statefulset.yaml and
gits-sbx-emu-proof.sh: untouched.

## Verification (head f9870ce, branch run solo) **`bash -n`**: `gits-sbx-emu-smoke.sh`, `scripts/gits-sbx-epic-test.sh`, `gits-sbx-emu-proof.sh` (untouched) — all clean. **`bash scripts/gits-sbx-epic-test.sh`** (run alone, never concurrent — fixed /tmp paths): `RESULT: FAIL (3 of 593 checks failed, 6 skipped, 0 pending)` → **590 pass, 3 red, 6 skip**. All three reds are pre-existing and none of their inputs is touched by this branch: | red | why it is not this branch | |---|---| | `probe uses an EMPTY pre-existing --dir and leaves it in place (rc=2)` | the known pre-existing red, red before this branch | | `manifest[3] imagenes = sbx-fleet@sha256:7e0513ca…` | the check's own expectation literal (`platform/sbx-fleet:eebdbe1@sha256:7e0513ca…`) appears **3× in `origin/live`'s own harness** while `origin/live`'s own `statefulset.yaml` carries `6fa150a@sha256:0ee5e97…` (PR #842's pin). Live's harness already disagrees with live's manifest; `git diff origin/live -- core/manifests/apps/sbx-gits/{statefulset,kustomization}.yaml` is **empty** here and this branch adds **0** occurrences of that literal | | `manifest[3b] bump de imagen H5 (falta: …)` | same stale pin, same evidence | **The two guards named in the brief still pass, on the re-measured set:** ``` PASS T7[diff] sbx-gits cambia contra origin/live SOLO en lo esperado: M tools/gits-sbx-emu-smoke.sh (…) PASS 17[R159] (d) T7[diff] sbx-gits cambia contra origin/live SOLO en lo esperado: M tools/gits-sbx-emu-smoke.sh (…) PASS 17[R159] (d) mutacion roja: quitado 'M tools/gits-sbx-emu-smoke.sh' del conjunto permitido, ROJO (el conjunto muerde) ``` **New §23 (real extracted smoke code + stateful fake `adb`, no pod):** ``` PASS 23[HOMELAB-2682] bash -n del smoke PASS 23[HOMELAB-2682] codigo REAL extraido del smoke (phase_connected_test ENTERO + la linea del DEFAULT de PRECLEAN) PASS 23[HOMELAB-2682] script: el aviso del opt-out sale UNA sola vez en la corrida PASS 23[HOMELAB-2682] la herramienta de prueba NO conoce PRECLEAN (linea, campos y check del ledger no cambian) PASS 23[HOMELAB-2682] mutacion roja EJECUTADA 1: copia SIN el opt-out, con PRECLEAN=skip, desinstala igual PASS 23[HOMELAB-2682] mutacion roja EJECUTADA 2: copia con comparacion LAXA se salta el PRE-CLEAN con 'Skip' PASS 23[HOMELAB-2682] el smoke gana UN opt-out documentado … (21 checks, 0 fallos) ``` Cases actually executed: default (`PRECLEAN` unset) → uninstall **before** install, `PRE_CLEAN_ST=OK`, `connected-test: OK`; `PRECLEAN=skip` → **zero** uninstall actions, install-over, state survives, one marker line, `PRE_CLEAN_ST=SKIP`, `connected-test: OK`; `Skip` / `SKIP` / `1` / `yes` → the default path, `PRE_CLEAN_ST=OK`. **R171 section re-run after the fixture fix (unchanged semantics):** 17 checks, 0 fails — the existing PRE-CLEAN green/mutation proofs still bite (its red mutation now deletes the call independently of indentation). **CI on this head:** run #3138 and #3139 — all jobs success (shellcheck, terraform-validate, yaml-lint, helm-validate, runner-manifest-test, record-sql-test, dispatcher/cockpit/receiver…). **Files vs `origin/live`:** `M core/manifests/apps/sbx-gits/tools/gits-sbx-emu-smoke.sh`, `M docs/gits-sbx/run-proof.md`, `M docs/gits-sbx/t7-emu-smoke.md`, `M scripts/gits-sbx-epic-test.sh`. `kustomization.yaml`, `statefulset.yaml` and `gits-sbx-emu-proof.sh`: untouched.

ld-reviewer commented 2026-11-05 16:41:40 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head f9870ce020: APPROVE
model: deepseek-flash
session: 01a1ac8a-5333-757a-b84d-27614526925e

Seat 2 of 2 (adversarial/tooling angle). Everything below was read off the head and, where it
could be executed, executed against stubs in /tmp — the real opt-out block and the real
clean_variant_packages extracted from gits-sbx-emu-smoke.sh at this sha. The parallel
correctness seat is not a dependency of anything here.

Blockers

None.

Concerns

• C1: low — the comment directly above PRECLEAN=${PRECLEAN:-} in the smoke claims the
  comparison is "sin distinguir mayusculas" (case-insensitive). The code
  ([[ $PRECLEAN == skip ]]) is case-sensitive, and every other surface says so: the header
  block, the --help text, both docs, harness case (3) and the second red mutation. Behaviour
  is correct and safe; the sentence is wrong, and it sits at the definition site, so a
  maintainer following it would write ${PRECLEAN,,} — precisely the lax comparison the
  harness's second red mutation exists to catch. Dropping those four words in this PR would
  keep the one artifact that is read next to the code honest.
• C2: low — the skip is invisible in the copyable == RESUMEN / == CHECK INFRA block. The
  resumen prints clean (CLEAN_ST) only; PRE_CLEAN_ST is never printed there, and in the
  skip branch clean_variant_packages is not called at all, so there is no
  PRE-CLEAN resultado: line either. The single trace is the marker line in the body (and so
  in the capture). That is consistent with "no output change", but someone reading only the
  three copyable lines cannot tell. Not for this PR necessarily: a pre-clean=SKIP(env) token
  in the resumen line would surface it without adding a fourth CHECK INFRA line or a ledger
  field.
• C3: low — clarity for the next consumer: the trailing CLEAN still uninstalls, so the
  hardware-backed identity does not survive the run. t7-emu-smoke.md almost says it ("sólo
  afecta al estado que esta corrida deja antes de instalar"), but the new §PRECLEAN=skip
  would be safer stating once that the suite must re-enrol inside its own run, so the opt-out
  cannot be read as "the identity now persists between runs".

What I verified (evidence, seat 2)

• Value matching, executed. I extracted the real PRECLEAN=${PRECLEAN:-} line and the real
  if/else block from the head into a harness with set -uo pipefail and a stub
  clean_variant_packages. skip → no uninstall call, PRE_CLEAN_ST=SKIP. Skip, SKIP,
  1, yes, true, " skip", "skip ", sk, sk?p → the uninstall runs,
  PRE_CLEAN_ST=OK. Unset → uninstall runs and does not trip set -u (the :- default
  defines it). So: no trimming, no case folding (C1's sentence aside), a glob in the value
  cannot match because the pattern is the literal right-hand token, and every typo fails safe
  to PRE-CLEAN ON.
• rc, XML and capture unchanged. The skip branch falls through to the same gradle_task,
  the same single check_line connected-test, the same end-of-run phase_clean and the same
  on_exit; xml_scan reads the outputs directory by mtime, untouched. PRECLEAN appears
  nowhere in gits-sbx-emu-proof.sh, and cmd_check's required-field list has no notion of
  cleaning, so the gate is semantically identical. The only delta is the one marker line in
  stdout, which lands in the capture and therefore in output_sha256 — a new artifact identity
  for a run that skipped the uninstall, which is the intended, honest outcome.
• Env trust boundary (the sharp question), assessed. run launches the suite as
  ( PROJECT_DIR=… CONNECTED_TASK=… bash "$real" … ), so every other variable, PRECLEAN
  included, is inherited from whoever invokes the proof tool; the runbook has the driver typing
  that command by hand, so no wrapper pins the environment. A caller who can run run can
  therefore obtain a check-green line from a run that installed over stale state. Assessment:
  a concern at the gate's trust boundary, not a defect of this diff — (a) that boundary is
  already defeatable before this PR and with less visibility: I executed it,
  EMU_APP_PACKAGE=com.bogus turns every pre-clean target into "no instalado (ok)", ends in
  PRE-CLEAN resultado: OK and prints no marker at all; (b) the feature was requested by the
  gate's own consumer (gits-hypervisor) for a case it currently dodges completely outside the
  smoke; (c) the marker lands inside the hashed capture, so the weakening is legible and
  non-repudiable in the artifact. If the owner wants it hard, two named mitigations: pin
  PRECLEAN= explicitly at the epic/gate invocation site (no code), or teach check to
  refuse a line whose capture carries the marker unless the epic declared the opt-out (a
  schema decision this PR deliberately did not take, and both docs are honest about that).
• Second commit f9870ce matches its message and its scope. It touches
  scripts/gits-sbx-epic-test.sh only — no production code, no docs, no smoke change.
  t171_extract now also extracts the real PRECLEAN= default line and refuses the extraction
  without a grep -q '^PRECLEAN=' guard, so a missing default fails loudly instead of passing
  by luck; the R171 red mutation sed is anchored ^[[:space:]]*, and I checked the target text
  occurs exactly once in the head smoke (the call inside the opt-out's else), with the
  mutation verified as applied via cmp plus grep -c 0, so it cannot silently no-op; §23's
  default case now asserts ORDER (uninstall line before install line — R171's actual point)
  instead of end state, counts the marker in the run that has the opt-out rather than in a
  later typo case, and drops the old source-text grep -c == 1 assertion in favour of the
  executed count. Fixtures and assertions only, as the message says.

**Verdict of record — ld-reviewer — head f9870ce020b464eba34d1087c5379693bfc3785e: APPROVE** model: deepseek-flash session: 01a1ac8a-5333-757a-b84d-27614526925e Seat 2 of 2 (adversarial/tooling angle). Everything below was read off the head and, where it could be executed, executed against stubs in /tmp — the real opt-out block and the real `clean_variant_packages` extracted from `gits-sbx-emu-smoke.sh` at this sha. The parallel correctness seat is not a dependency of anything here. ## Blockers None. ## Concerns - C1: low — the comment directly above `PRECLEAN=${PRECLEAN:-}` in the smoke claims the comparison is "sin distinguir mayusculas" (case-insensitive). The code (`[[ $PRECLEAN == skip ]]`) is case-sensitive, and every other surface says so: the header block, the `--help` text, both docs, harness case (3) and the second red mutation. Behaviour is correct and safe; the sentence is wrong, and it sits at the definition site, so a maintainer following it would write `${PRECLEAN,,}` — precisely the lax comparison the harness's second red mutation exists to catch. Dropping those four words in this PR would keep the one artifact that is read next to the code honest. - C2: low — the skip is invisible in the copyable `== RESUMEN` / `== CHECK INFRA` block. The resumen prints `clean` (CLEAN_ST) only; `PRE_CLEAN_ST` is never printed there, and in the skip branch `clean_variant_packages` is not called at all, so there is no `PRE-CLEAN resultado:` line either. The single trace is the marker line in the body (and so in the capture). That is consistent with "no output change", but someone reading only the three copyable lines cannot tell. Not for this PR necessarily: a `pre-clean=SKIP(env)` token in the resumen line would surface it without adding a fourth CHECK INFRA line or a ledger field. - C3: low — clarity for the next consumer: the trailing CLEAN still uninstalls, so the hardware-backed identity does not survive the run. `t7-emu-smoke.md` almost says it ("sólo afecta al estado que esta corrida deja antes de instalar"), but the new §`PRECLEAN=skip` would be safer stating once that the suite must re-enrol inside its own run, so the opt-out cannot be read as "the identity now persists between runs". ## What I verified (evidence, seat 2) - **Value matching, executed.** I extracted the real `PRECLEAN=${PRECLEAN:-}` line and the real `if/else` block from the head into a harness with `set -uo pipefail` and a stub `clean_variant_packages`. `skip` → no uninstall call, `PRE_CLEAN_ST=SKIP`. `Skip`, `SKIP`, `1`, `yes`, `true`, `" skip"`, `"skip "`, `sk`, `sk?p` → the uninstall runs, `PRE_CLEAN_ST=OK`. Unset → uninstall runs and does not trip `set -u` (the `:-` default defines it). So: no trimming, no case folding (C1's sentence aside), a glob in the value cannot match because the pattern is the literal right-hand token, and every typo fails safe to PRE-CLEAN ON. - **rc, XML and capture unchanged.** The skip branch falls through to the same `gradle_task`, the same single `check_line connected-test`, the same end-of-run `phase_clean` and the same `on_exit`; `xml_scan` reads the outputs directory by mtime, untouched. `PRECLEAN` appears nowhere in `gits-sbx-emu-proof.sh`, and `cmd_check`'s required-field list has no notion of cleaning, so the gate is semantically identical. The only delta is the one marker line in stdout, which lands in the capture and therefore in `output_sha256` — a new artifact identity for a run that skipped the uninstall, which is the intended, honest outcome. - **Env trust boundary (the sharp question), assessed.** `run` launches the suite as `( PROJECT_DIR=… CONNECTED_TASK=… bash "$real" … )`, so every other variable, `PRECLEAN` included, is inherited from whoever invokes the proof tool; the runbook has the driver typing that command by hand, so no wrapper pins the environment. A caller who can run `run` can therefore obtain a check-green line from a run that installed over stale state. Assessment: a concern at the gate's trust boundary, not a defect of this diff — (a) that boundary is already defeatable before this PR and with less visibility: I executed it, `EMU_APP_PACKAGE=com.bogus` turns every pre-clean target into "no instalado (ok)", ends in `PRE-CLEAN resultado: OK` and prints no marker at all; (b) the feature was requested by the gate's own consumer (gits-hypervisor) for a case it currently dodges completely outside the smoke; (c) the marker lands inside the hashed capture, so the weakening is legible and non-repudiable in the artifact. If the owner wants it hard, two named mitigations: pin `PRECLEAN=` explicitly at the epic/gate invocation site (no code), or teach `check` to refuse a line whose capture carries the marker unless the epic declared the opt-out (a schema decision this PR deliberately did not take, and both docs are honest about that). - **Second commit f9870ce matches its message and its scope.** It touches `scripts/gits-sbx-epic-test.sh` only — no production code, no docs, no smoke change. `t171_extract` now also extracts the real `PRECLEAN=` default line and refuses the extraction without a `grep -q '^PRECLEAN='` guard, so a missing default fails loudly instead of passing by luck; the R171 red mutation sed is anchored `^[[:space:]]*`, and I checked the target text occurs exactly once in the head smoke (the call inside the opt-out's `else`), with the mutation verified as applied via `cmp` plus `grep -c` 0, so it cannot silently no-op; §23's default case now asserts ORDER (uninstall line before install line — R171's actual point) instead of end state, counts the marker in the run that has the opt-out rather than in a later typo case, and drops the old source-text `grep -c == 1` assertion in favour of the executed count. Fixtures and assertions only, as the message says.

ld-reviewer commented 2026-11-05 16:43:00 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head f9870ce020: APPROVE

model: deepseek-flash
session: 01a1ac8a-5304-757a-b84d-275e298774f1

Blockers

• None.

Concerns

• C1: low — the inline comment above PRECLEAN=${PRECLEAN:-} (smoke lines 329-331) says the
  comparison is "sin recortar y sin distinguir mayusculas", i.e. case-insensitive, which
  contradicts the code ([[ $PRECLEAN == skip ]], literal), the file header (lines 152-153),
  the --help block and both docs. Executed: Skip, SKIP, 1, yes, skip, skip all
  take the default path. Suggest dropping that clause so a later reader cannot "align" the code
  with the comment.
• C2: low, no code change required — the smoke's bytes change, so a ledger line recorded with
  the previous suite no longer satisfies check (script_sha256). That is the documented
  pre-existing semantics (the new run-proof.md section says exactly this); the only follow-up
  is that the GITS-493 consumer's next official run is on the merged head.

What I verified (primary sources at this head)

• Default path, executed. I extracted the real
  say/hms/check_line/gradle_shim_check/app_id/variant_suffix/clean_pkg_installed/clean_variant_packages/set_clean_result/pre_clean_hand_check/phase_clean/phase_connected_test
  from the base smoke (911810dd) and from the head smoke and ran both against the same
  stateful fake adb with PRECLEAN unset: stdout identical apart from my own fixture paths,
  action sequence identical — uninstall <app>.dev, then the three "no instalado (ok)" probes,
  then the install. Same order, same adb -s "$ADB_SERIAL" uninstall form, same flags;
  PRE_CLEAN_ST=OK, connected-test: OK.
• Default FAIL path, executed. With a fake adb answering
  Failure [DELETE_FAILED_USER_RESTRICTED], base and head both yield
  PRE-CLEAN resultado: FAIL (1 paquete(s)) → connected-test: FAIL (PRE-CLEAN no pudo dejar sin instalar …) plus the manual-check line. The guard inside the else is intact.
• Skip path, executed. PRECLEAN=skip → zero uninstall actions, first action is
  install com.ejemplo.app.dev over the existing package, exactly one marker line,
  PRE_CLEAN_ST=SKIP, connected-test: OK, no extra check_line (so CHECK INFRA keeps its
  three lines and rc is untouched).
• Final CLEAN, executed. phase_clean with PRECLEAN=skip set is byte-identical to
  base: the end-of-run clean is not affected.
• Literal comparison. skip → skip path; Skip, SKIP, 1, yes, sk*, *,
  skip x, skip, skip , unset → default. No globbing, no trim, no case folding.
• No downstream coupling. PRE_CLEAN_ST/PRE_CLEAN_PKGS/PRE_CLEAN_FAILS are read only by
  the FAIL guard that the skip branch bypasses; nothing else in the smoke or the proof tool
  reads them, and pm path/pm list packages appear only inside
  clean_variant_packages/pre_clean_hand_check.
• Proof tool non-interaction. gits-sbx-emu-proof.sh is byte-identical to base
  (sha256 b9d098e8…). It runs the suite as
  ( PROJECT_DIR=… CONNECTED_TASK=… bash "$real" … ), so the variable is inherited; run
  derives result from the real rc plus the XML and never parses the smoke's check lines, so
  the marker can only reach output_sha256 (documented). No new field, no capture-format or
  check change.
• Docs. Header block (lines 135-161) and --help (rendered locally with
  gits-sbx-emu-queue.sh present) state the single exact value, the AndroidKeyStore reason and
  that the default stays on; docs/gits-sbx/t7-emu-smoke.md (R171 pointer + the new
  §PRECLEAN=skip) and docs/gits-sbx/run-proof.md match the executed behaviour.
• Harness, executed by me on the head files. 23[HOMELAB-2682] = 21 checks, 0 fails, and
  both red mutations execute (the no-opt-out copy behaves exactly like the default; the lax
  ${PRECLEAN,,} copy accepts Skip), so the green is not an assertion nobody has seen fail.
  Commit 2 touches only scripts/gits-sbx-epic-test.sh: it extracts the real PRECLEAN= line
  into the §19 fixture (needed now that set -u reaches the new reference), de-anchors the
  mutation sed from a fixed indent, and replaces an incorrect "package must not be installed
  after the run" assertion with an uninstall-before-install ordering assertion plus a
  marker-count assertion — fixture correctness, no behaviour smuggled in.
• Scope. kustomization.yaml and statefulset.yaml untouched; the sbx-gits diff set is
  exactly M tools/gits-sbx-emu-smoke.sh; four files changed in total.
• CI on this head: runs #3138 and #3139, every job success.

**Verdict of record — ld-reviewer — head f9870ce020b464eba34d1087c5379693bfc3785e: APPROVE** model: deepseek-flash session: 01a1ac8a-5304-757a-b84d-275e298774f1 ## Blockers - None. ## Concerns - C1: low — the inline comment above `PRECLEAN=${PRECLEAN:-}` (smoke lines 329-331) says the comparison is "sin recortar y sin distinguir mayusculas", i.e. case-insensitive, which contradicts the code (`[[ $PRECLEAN == skip ]]`, literal), the file header (lines 152-153), the `--help` block and both docs. Executed: `Skip`, `SKIP`, `1`, `yes`, ` skip`, `skip ` all take the default path. Suggest dropping that clause so a later reader cannot "align" the code with the comment. - C2: low, no code change required — the smoke's bytes change, so a ledger line recorded with the previous suite no longer satisfies `check` (`script_sha256`). That is the documented pre-existing semantics (the new `run-proof.md` section says exactly this); the only follow-up is that the GITS-493 consumer's next official `run` is on the merged head. ## What I verified (primary sources at this head) - **Default path, executed.** I extracted the real `say/hms/check_line/gradle_shim_check/app_id/variant_suffix/clean_pkg_installed/clean_variant_packages/set_clean_result/pre_clean_hand_check/phase_clean/phase_connected_test` from the base smoke (`911810dd`) and from the head smoke and ran both against the same stateful fake `adb` with `PRECLEAN` unset: stdout identical apart from my own fixture paths, action sequence identical — `uninstall <app>.dev`, then the three "no instalado (ok)" probes, then the install. Same order, same `adb -s "$ADB_SERIAL" uninstall` form, same flags; `PRE_CLEAN_ST=OK`, `connected-test: OK`. - **Default FAIL path, executed.** With a fake adb answering `Failure [DELETE_FAILED_USER_RESTRICTED]`, base and head both yield `PRE-CLEAN resultado: FAIL (1 paquete(s))` → `connected-test: FAIL (PRE-CLEAN no pudo dejar sin instalar …)` plus the manual-check line. The guard inside the `else` is intact. - **Skip path, executed.** `PRECLEAN=skip` → zero uninstall actions, first action is `install com.ejemplo.app.dev` over the existing package, exactly one marker line, `PRE_CLEAN_ST=SKIP`, `connected-test: OK`, no extra `check_line` (so `CHECK INFRA` keeps its three lines and `rc` is untouched). - **Final `CLEAN`, executed.** `phase_clean` with `PRECLEAN=skip` set is byte-identical to base: the end-of-run clean is not affected. - **Literal comparison.** `skip` → skip path; `Skip`, `SKIP`, `1`, `yes`, `sk*`, `*`, `skip x`, ` skip`, `skip `, unset → default. No globbing, no trim, no case folding. - **No downstream coupling.** `PRE_CLEAN_ST`/`PRE_CLEAN_PKGS`/`PRE_CLEAN_FAILS` are read only by the FAIL guard that the skip branch bypasses; nothing else in the smoke or the proof tool reads them, and `pm path`/`pm list packages` appear only inside `clean_variant_packages`/`pre_clean_hand_check`. - **Proof tool non-interaction.** `gits-sbx-emu-proof.sh` is byte-identical to base (sha256 `b9d098e8…`). It runs the suite as `( PROJECT_DIR=… CONNECTED_TASK=… bash "$real" … )`, so the variable is inherited; `run` derives `result` from the real rc plus the XML and never parses the smoke's check lines, so the marker can only reach `output_sha256` (documented). No new field, no capture-format or `check` change. - **Docs.** Header block (lines 135-161) and `--help` (rendered locally with `gits-sbx-emu-queue.sh` present) state the single exact value, the AndroidKeyStore reason and that the default stays on; `docs/gits-sbx/t7-emu-smoke.md` (R171 pointer + the new §`PRECLEAN=skip`) and `docs/gits-sbx/run-proof.md` match the executed behaviour. - **Harness, executed by me on the head files.** `23[HOMELAB-2682]` = 21 checks, 0 fails, and both red mutations execute (the no-opt-out copy behaves exactly like the default; the lax `${PRECLEAN,,}` copy accepts `Skip`), so the green is not an assertion nobody has seen fail. Commit 2 touches only `scripts/gits-sbx-epic-test.sh`: it extracts the real `PRECLEAN=` line into the §19 fixture (needed now that `set -u` reaches the new reference), de-anchors the mutation `sed` from a fixed indent, and replaces an incorrect "package must not be installed after the run" assertion with an uninstall-before-install ordering assertion plus a marker-count assertion — fixture correctness, no behaviour smuggled in. - **Scope.** `kustomization.yaml` and `statefulset.yaml` untouched; the sbx-gits diff set is exactly `M tools/gits-sbx-emu-smoke.sh`; four files changed in total. - **CI on this head:** runs #3138 and #3139, every job success.

ld-reviewer approved these changes 2026-11-05 16:47:13 +00:00

ld-reviewer left a comment

Collaborator

Copy link

Verdict: APPROVED

model: omniroute/deepseek-flash
head: f9870ce020
session: 01a1ac99-07e3-757a-b84d-27621b17dbcd

The blockers raised in review 22087 are resolved on head f9870ce020: its only Blockers bullet is the empty-statement - None., which states the absence of blockers; both independent gate-tooling seats approved this exact head.

## Verdict: APPROVED model: omniroute/deepseek-flash head: f9870ce020b464eba34d1087c5379693bfc3785e session: 01a1ac99-07e3-757a-b84d-27621b17dbcd The blockers raised in review 22087 are resolved on head f9870ce020b464eba34d1087c5379693bfc3785e: its only Blockers bullet is the empty-statement `- None.`, which states the absence of blockers; both independent gate-tooling seats approved this exact head.

pi-landing merged commit b5c8498cd3 into live 2026-11-05 16:48:35 +00:00

pi-landing referenced this pull request from a commit 2026-11-05 16:48:36 +00:00

Merge pull request 'HOMELAB-2682: feat(sbx-gits): documented PRECLEAN=skip opt-out in emu smoke' (#847) from plane/HOMELAB-2682-preclean-optout into live

aaron referenced this pull request from a commit 2026-11-05 21:58:36 +00:00

HOMELAB-2682: fix(sbx-gits): check anchors the canonical smoke to the blob committed at the head

pi-landing referenced this pull request 2026-11-05 22:32:35 +00:00

HOMELAB-2682: fix(sbx-gits): proof check anchored to the smoke committed at the head #859

Sign in to join this conversation.

Reviewers

No reviewers

ld-reviewer

Labels

Clear labels   

operator-review

Requires human operator review before merge — dangerous class (RBAC, secrets, network policy, Terraform) or explicitly gated. Never auto-merge.

No labels

operator-review

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

3 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

Homelab/infra-core!847

No description provided.