HOMELAB-2682: fix(sbx-gits): seat transcripts move to the persistent volume (TMPDIR) — PR B of the evidence fixes #854

Merged

pi-landing merged 5 commits from plane/HOMELAB-2682-transcripts into live 2026-11-06 09:52:08 +00:00

Conversation 5 Commits 5 Files changed 3 +567 -40

aaron commented 2026-11-05 18:43:36 +00:00

Owner

Copy link

The third evidence gap gits-hypervisor found while checking a real merge: "seat transcripts live in /tmp/pi-subagents-1000, lost on eviction, and the audit before promotion needs them. Keep them on the persistent volume."

Separate from the tools PR (which carries the other two gaps) because this one touches core/manifests/apps/sbx-gits/statefulset.yaml, which the tools-lane branch's pinned T7[diff] set forbade. That tools PR has since landed (live is now 5b0d7f43: #866 revert, #869, #868, #870 pin, #871 declaration, #867 floors), so this branch is brought up to date by a merge of origin/live (no rebase, R159) and its T7[diff] set is re-measured as M README.md + M statefulset.yaml — the tools set and its delivery pin are no longer in play here.

How this deploys

deploys at the sbx-gits clean point via homelab-33; the tools pin does not govern this statefulset change, which is a spec change taking effect at the next pod restart

The mechanism (read in the code, not guessed)

pi-swarm-suite/src/core/output-file.ts   ← vendored @tintinweb/pi-subagents v0.10.3
  export function createOutputFilePath(cwd, agentId, sessionId) {
    const root = join(tmpdir(), `pi-subagents-${process.getuid?.() ?? 0}`);

tmpdir() is node:os. On Linux that is $TMPDIR (/tmp when unset) — verified in this PR by executing it: TMPDIR=… node -e 'require("os").tmpdir()' returns the value, and harness §28 runs exactly that so the assertion rests on the runtime, not on a comment. There is no pi-specific setting for the seat-transcript directory (searched the pi docs and the vendored core): TMPDIR is the only lever.

What changes

where	what
sandbox container, env:	TMPDIR=/home/ld/work/seat-transcripts — inside the persistent hostPath work (/var/mnt/u-longhorn/sbx-gits/work), not the 4 Gi emptyDir mounted at /tmp
seed-pi-agent initContainer	creates /home/ld/work/seat-transcripts on every boot (Node would create it recursively at the first subagent, but adb and any other TMPDIR consumer expect it to exist). A failure prints SEED-WARN and the seed carries on
emulator container	not touched: separate container, its /tmp stays the ephemeral emptyDir, which is right there
README §5.2	the fact, the value, why the variable and not a launch wrapper, the side effects, and the limit (sbx-lab / sbx-sendrast)

Transcripts now land at /home/ld/work/seat-transcripts/pi-subagents-1000/<cwd>/<session>/tasks/<agent>.output and survive an eviction.

Why the variable and not a launch wrapper

There is no session launch script in this pod — whoever starts a session types pi. A wrapper on PATH would be a silent no-op the moment the PATH order changed, which is a worse failure than moving a container's tmpdir. The env is inherited by every pi process in the sandbox container, which is exactly the scope we want; the emulator container is a different container and does not inherit it.

Side effects, stated (this moves a whole container's tmpdir)

• Gradle / JVM: NOT affected. java.io.tmpdir is a JVM property (default /tmp) and does not read TMPDIR; the emulator builds keep using /tmp.
• adb and go do use TMPDIR for temp files — the directory exists and is writable by the pod's uid 1000 (the same reason the smoke already keeps ADB_HOME under LD_WORK: adb aborts rc=134 if its HOME is not writable).
• pi's own temp output files (truncated tool output, pi-*), session-share and the external editor also move onto the volume: small, and persistent beats lost.
• Budget: what used to land in the 4 Gi emptyDir now lands on the hostPath, which is the volume that carries the sandbox's 30 GiB ceiling (README §1.1). Watch it, do not estimate it.
• sbx-lab / sbx-sendrast are NOT changed here (this directory is the only one in the pinned T7[diff] set). If their seats need the same persistence, the same TMPDIR goes in their manifests — their owner's decision, not a side effect of this PR.

The guard, and why it was adjusted here

T7[diff] / 17[R159] (d) pin the file set a branch may change under sbx-gits. SBX_GITS_DIFF_SET is re-measured for this branch (M README.md, M statefulset.yaml) and the justification is written next to the literal: the guard exists to catch an unwanted manifest change, not to freeze the manifest. This one is wanted and reviewed — the audit before promotion reads those transcripts and an eviction was deleting them.

Nothing else moves: no PVC, no volume, no mount, no resource change, no new file, and the emulator / prepare-work containers and kustomization.yaml are untouched. A third touched file here is still RED.

Verification

• kubectl apply --dry-run=client -f statefulset.yaml → ok; parsed with PyYAML: TMPDIR in sandbox only, emulator has none, initContainers = prepare-work, seed-pi-agent.
• Harness §28 (new, executed): parses the manifest structurally (not a grep) and asserts TMPDIR=/home/ld/work/seat-transcripts in sandbox, absent in emulator, /home/ld/work of sandbox resolved to the hostPath, the seed's mkdir -p, the executed os.tmpdir(), and the README prose (where transcripts live, eviction, value, wrapper rationale, java.io.tmpdir, sbx-lab/sbx-sendrast not touched).
• The rendered StatefulSet was parsed after the merge: sandbox has TMPDIR=/home/ld/work/seat-transcripts, emulator has none, initContainers = prepare-work, seed-pi-agent (kubectl kustomize core/manifests/apps/sbx-gits).
• Harness run solo on the merged head and on origin/live; the FAIL sets are byte-identical — see the note below.

Notes for the reviewer / the orchestrator

1. Pre-existing reds in the epic test on today's live — measured, not assumed. On origin/live = 5b0d7f43 the harness run solo fails 6 of 670 checks; on this merged head it fails the same 6 of 681 (the extra checks are this branch's §28 plus the two T7[diff] guards that only skip on a branch which does not touch sbx-gits). Byte-identical FAIL sets: zero new reds, zero fixed. The six: probe uses an EMPTY pre-existing --dir, manifest[2] (memory pinned to 6Gi/10Gi after #851 landed 12Gi/13Gi), manifest[3] / [3b] (image pins), the T7 totals row (requests sums 18Gi), and T7+R99[10b] (the generator's per-file legs, which #870's literal pin deliberately replaced). Those are guard literals other tickets did not re-measure; I did not touch them here, because re-measuring another ticket's numbers inside this PR would hide the drift instead of reporting it. This deserves its own ticket.
2. PR A (plane/HOMELAB-2682-evidence-fixes, the ledger class + per-run results) has already landed — its sections are in live as §24–§27. This branch merged origin/live rather than rebasing. The only conflict was scripts/gits-sbx-epic-test.sh, resolved keep-both-sides: live's §24–§27 untouched, this branch's transcripts section renumbered to §28 and appended, and the single SBX_GITS_DIFF_SET literal set to this branch's measured pair (README.md + statefulset.yaml). statefulset.yaml and README.md merged with no conflict. The two T7[diff] / 17[R159](d) PASS messages that came in from live still narrated the declaration branch's set and asserted statefulset.yaml did not move; they now name this branch's set (the comparison and both mutations were already driven by SBX_GITS_DIFF_SET — only the prose was stale).

Not merged here: the landing lock's rules apply (verdicts on the exact head, green CI). Seats follow after refresh.

The third evidence gap **gits-hypervisor** found while checking a real merge: *"seat transcripts live in `/tmp/pi-subagents-1000`, lost on eviction, and the audit before promotion needs them. Keep them on the persistent volume."* Separate from the tools PR (which carries the other two gaps) because this one touches `core/manifests/apps/sbx-gits/statefulset.yaml`, which the tools-lane branch's pinned `T7[diff]` set forbade. That tools PR has since landed (`live` is now `5b0d7f43`: #866 revert, #869, #868, #870 pin, #871 declaration, #867 floors), so this branch is brought up to date by a **merge** of `origin/live` (no rebase, R159) and its `T7[diff]` set is re-measured as `M README.md` + `M statefulset.yaml` — the tools set and its delivery pin are no longer in play here. ## How this deploys deploys at the sbx-gits clean point via homelab-33; the tools pin does not govern this statefulset change, which is a spec change taking effect at the next pod restart ## The mechanism (read in the code, not guessed) ``` pi-swarm-suite/src/core/output-file.ts ← vendored @tintinweb/pi-subagents v0.10.3 export function createOutputFilePath(cwd, agentId, sessionId) { const root = join(tmpdir(), `pi-subagents-${process.getuid?.() ?? 0}`); ``` `tmpdir()` is `node:os`. On Linux that **is `$TMPDIR`** (`/tmp` when unset) — verified in this PR by executing it: `TMPDIR=… node -e 'require("os").tmpdir()'` returns the value, and harness §28 runs exactly that so the assertion rests on the runtime, not on a comment. There is **no pi-specific setting** for the seat-transcript directory (searched the pi docs and the vendored core): `TMPDIR` is the only lever. ## What changes | where | what | |---|---| | `sandbox` container, `env:` | `TMPDIR=/home/ld/work/seat-transcripts` — inside the persistent `hostPath` `work` (`/var/mnt/u-longhorn/sbx-gits/work`), **not** the 4 Gi `emptyDir` mounted at `/tmp` | | `seed-pi-agent` initContainer | creates `/home/ld/work/seat-transcripts` on every boot (Node would create it recursively at the first subagent, but `adb` and any other `TMPDIR` consumer expect it to exist). A failure prints `SEED-WARN` and the seed carries on | | `emulator` container | **not** touched: separate container, its `/tmp` stays the ephemeral `emptyDir`, which is right there | | README §5.2 | the fact, the value, why the variable and not a launch wrapper, the side effects, and the limit (sbx-lab / sbx-sendrast) | Transcripts now land at `/home/ld/work/seat-transcripts/pi-subagents-1000/<cwd>/<session>/tasks/<agent>.output` and **survive an eviction**. ## Why the variable and not a launch wrapper There is **no session launch script in this pod** — whoever starts a session types `pi`. A wrapper on `PATH` would be a silent no-op the moment the `PATH` order changed, which is a worse failure than moving a container's `tmpdir`. The env is inherited by every pi process in the `sandbox` container, which is exactly the scope we want; the `emulator` container is a different container and does not inherit it. ## Side effects, stated (this moves a whole container's `tmpdir`) * **Gradle / JVM: NOT affected.** `java.io.tmpdir` is a JVM property (default `/tmp`) and does **not** read `TMPDIR`; the emulator builds keep using `/tmp`. * **`adb` and `go` do use `TMPDIR`** for temp files — the directory exists and is writable by the pod's uid 1000 (the same reason the smoke already keeps `ADB_HOME` under `LD_WORK`: adb aborts rc=134 if its `HOME` is not writable). * pi's own temp output files (truncated tool output, `pi-*`), `session-share` and the external editor also move onto the volume: small, and persistent beats lost. * Budget: what used to land in the 4 Gi `emptyDir` now lands on the `hostPath`, which is the volume that carries the sandbox's 30 GiB ceiling (README §1.1). Watch it, do not estimate it. * **sbx-lab / sbx-sendrast are NOT changed here** (this directory is the only one in the pinned `T7[diff]` set). If their seats need the same persistence, the same `TMPDIR` goes in *their* manifests — their owner's decision, not a side effect of this PR. ## The guard, and why it was adjusted here `T7[diff]` / `17[R159] (d)` pin the file set a branch may change under `sbx-gits`. `SBX_GITS_DIFF_SET` is **re-measured for this branch** (`M README.md`, `M statefulset.yaml`) and the justification is written next to the literal: the guard exists to catch an **unwanted** manifest change, not to freeze the manifest. This one is wanted and reviewed — the audit before promotion reads those transcripts and an eviction was deleting them. Nothing else moves: **no** PVC, **no** volume, **no** mount, **no** resource change, **no** new file, and the `emulator` / `prepare-work` containers and `kustomization.yaml` are untouched. A third touched file here is still RED. ## Verification * `kubectl apply --dry-run=client -f statefulset.yaml` → ok; parsed with PyYAML: `TMPDIR` in `sandbox` only, `emulator` has none, `initContainers` = `prepare-work`, `seed-pi-agent`. * Harness §28 (new, executed): parses the manifest structurally (not a grep) and asserts `TMPDIR=/home/ld/work/seat-transcripts` in `sandbox`, **absent** in `emulator`, `/home/ld/work` of `sandbox` resolved to the `hostPath`, the seed's `mkdir -p`, the executed `os.tmpdir()`, and the README prose (where transcripts live, eviction, value, wrapper rationale, `java.io.tmpdir`, sbx-lab/sbx-sendrast not touched). * The rendered StatefulSet was parsed after the merge: `sandbox` has `TMPDIR=/home/ld/work/seat-transcripts`, `emulator` has none, `initContainers` = `prepare-work`, `seed-pi-agent` (`kubectl kustomize core/manifests/apps/sbx-gits`). * Harness run **solo** on the merged head and on `origin/live`; the FAIL sets are byte-identical — see the note below. ## Notes for the reviewer / the orchestrator 1. **Pre-existing reds in the epic test on today's `live`** — measured, not assumed. On `origin/live` = `5b0d7f43` the harness run solo fails **6 of 670 checks**; on this merged head it fails **the same 6 of 681** (the extra checks are this branch's §28 plus the two `T7[diff]` guards that only skip on a branch which does not touch `sbx-gits`). Byte-identical FAIL sets: **zero new reds, zero fixed**. The six: `probe uses an EMPTY pre-existing --dir`, `manifest[2]` (memory pinned to 6Gi/10Gi after #851 landed 12Gi/13Gi), `manifest[3]` / `[3b]` (image pins), the `T7` totals row (`requests` sums 18Gi), and `T7+R99[10b]` (the generator's per-file legs, which #870's literal pin deliberately replaced). Those are guard literals other tickets did not re-measure; I did **not** touch them here, because re-measuring another ticket's numbers inside this PR would hide the drift instead of reporting it. This deserves its own ticket. 2. PR A (`plane/HOMELAB-2682-evidence-fixes`, the ledger `class` + per-run results) **has already landed** — its sections are in `live` as §24–§27. This branch merged `origin/live` rather than rebasing. The only conflict was `scripts/gits-sbx-epic-test.sh`, resolved **keep-both-sides**: live's §24–§27 untouched, this branch's transcripts section renumbered to **§28** and appended, and the single `SBX_GITS_DIFF_SET` literal set to this branch's measured pair (`README.md` + `statefulset.yaml`). `statefulset.yaml` and `README.md` merged with **no** conflict. The two `T7[diff]` / `17[R159](d)` PASS messages that came in from `live` still narrated the declaration branch's set and asserted `statefulset.yaml` did not move; they now name this branch's set (the comparison and both mutations were already driven by `SBX_GITS_DIFF_SET` — only the prose was stale). Not merged here: the landing lock's rules apply (verdicts on the exact head, green CI). Seats follow after refresh.

aaron added 1 commit 2026-11-05 18:43:37 +00:00

HOMELAB-2682: fix(sbx-gits): seat transcripts move to the persistent volume (TMPDIR) ... 93bbb5d494

The third evidence gap gits-hypervisor found checking a real merge: "seat transcripts
live in /tmp/pi-subagents-1000, lost on eviction, and the audit before promotion
needs them. Keep them on the persistent volume."

THE MECHANISM (read in the code, not guessed). pi's subagents write their transcript
through `createOutputFilePath` in the vendored swarm core
(pi-swarm-suite/src/core/output-file.ts): `join(tmpdir(), "pi-subagents-<uid>")`, and
Node's `tmpdir()` on Linux IS `$TMPDIR` (`/tmp` when unset). There is no pi-specific
setting for it. So `TMPDIR` is the mechanism, and the harness proves it by executing
`os.tmpdir()` under the value instead of trusting a comment.

WHAT CHANGES
- `sandbox` container: `TMPDIR=/home/ld/work/seat-transcripts`, i.e. inside the
  persistent `hostPath` `work` — not the 4Gi `emptyDir` at `/tmp`. Transcripts land in
  /home/ld/work/seat-transcripts/pi-subagents-1000/<cwd>/<session>/tasks/ and survive
  an eviction.
- `seed-pi-agent`: creates the directory on every boot (Node would create it
  recursively, but adb and any other TMPDIR consumer expect it to exist). A failure
  prints SEED-WARN and does not kill the seed.
- The `emulator` container does NOT inherit it (separate container; an ephemeral /tmp
  is right there).

WHY THE VARIABLE AND NOT A LAUNCH WRAPPER. There is no session launch script in this
pod — whoever starts a session types `pi` — so a wrapper on PATH would be a silent
no-op the moment PATH order changed. The env is inherited by every pi process in this
container, which is the scope we want.

SIDE EFFECTS, STATED IN THE MANIFEST AND THE README
- Gradle/JVM is NOT affected: `java.io.tmpdir` is a JVM property (default /tmp) and
  does not read TMPDIR.
- adb and go do use TMPDIR for temp files; the directory exists and is writable by
  the pod's uid 1000 (same reason the smoke already keeps ADB_HOME under LD_WORK).
- pi's own temp output files, session-share and external-editor temp move onto the
  volume as well: small, and persistent is better than lost.
- What used to land in the 4Gi emptyDir now lands on the hostPath, which carries the
  sandbox's 30GiB budget: watch it, don't estimate it.

sbx-lab and sbx-sendrast are NOT touched here (this directory is the only one in the
pinned T7[diff] set). If their seats need the same persistence, the same env goes in
their manifests — their owner's decision, not a side effect of this PR.

GUARD. This branch touches statefulset.yaml, which the tools-lane branch may not, so
SBX_GITS_DIFF_SET is re-measured for THIS branch (README.md + statefulset.yaml) with
the justification written next to it: T7[diff] catches an UNWANTED manifest change,
not every manifest change. Nothing else moves — no PVC, no volume, no mount, no
resource change, no new file. §24 of the epic test parses the manifest and asserts
what the structure has to be (TMPDIR in sandbox, not in emulator, inside the
persistent hostPath, created by the seed) plus the prose and the executed
`os.tmpdir()`.

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

HOMELAB-2682: fix(sbx-gits): make T7's spine/list guards actually strip env, args and mounts

aaron added 1 commit 2026-11-05 19:27:21 +00:00

HOMELAB-2682: fix(sbx-gits): make T7's spine/list guards actually strip env, args and mounts ... be0f636638

Making #854 green exposed a defect in the guard it has to pass: `p1_spine` matched
`volumeMounts:` / `args:` / `env:` at ONE fixed indentation, and in the RENDERED
document (`kubectl kustomize` output, which is what the guard reads) kustomize sorts a
container's keys and can put the block on the item line itself (`      - env:`).
Against the sandbox that pattern matched nothing, so the "spine" was the whole
template and ANY added variable came out as `spine-distinto` — the opposite of what
the function's own comment promises.

- `p1_spine`: the block is delimited by the INDENTATION of its marker, whatever it is,
  and a line at the same indentation that is a list item (`- ...`) is still inside the
  block (kustomize writes `        volumeMounts:` and then `        - mountPath:` at
  the same column).
- `p1_list`: the same rule for the env and mounts name lists. The sandbox's env names
  were INVISIBLE before (every credential switch among them): the allowed set was
  never applied to them.
- the allowed set is now a SUPERSET test ("every added name is one of the allowed
  ones") instead of an equality against the delta. The literals named an older wave's
  additions that are already in the reference, so the only delta that could ever match
  was the empty one — a wave adding its own legitimate name could never pass. An
  unexpected name is still red, which is the half that bites.
- `p1_lab_sts_bad`: an optional fourth argument lets ONE sandbox allow ONE extra env
  name, so allowing `TMPDIR` for sbx-gits does not loosen sbx-lab's set. Its VALUE is
  pinned structurally by section 24.

Verified against the real render: spine identical live-vs-branch (90 lines), env delta
exactly `TMPDIR`, mounts and volumes unchanged, and the function still bites (renaming
the variable, or dropping the allowance, is red).

aaron referenced this pull request 2026-11-05 19:39:18 +00:00

HOMELAB-2682: fix(sbx-gits): the ledger says which class ran and keeps each run's results (PR A of the evidence fixes) #853

aaron commented 2026-11-05 19:39:18 +00:00

Author

Owner

Copy link

Head moved: be0f6366 (was 93bbb5d4).

Adding the TMPDIR env surfaced a defect in the guard this PR has to pass, and it is fixed in the same branch rather than worked around:

p1_spine / p1_list (HOMELAB-2537 guard) never actually stripped anything in the rendered document. They matched volumeMounts: / args: / env: at ONE fixed indentation, but sts_doc is kubectl kustomize output and kustomize sorts a container's keys, so the block can be the item itself ( - env:). Consequences measured on the real render:

• the "spine" was the whole template, so any added env var came out as spine-distinto — the opposite of what the function's own comment promises (it says a wave that adds a NAMED variable must not be forced to edit the spine);
• p1_list … env saw 29 of the pod's env names, none of them from the sandbox container: the allowed set want_env='SEED_FORGEJO_EXT ' was never applied to it (that literal only ever matched the initContainer);
• the allowed-set test compared for equality against the delta, so the only delta that could ever pass was the empty one — a wave adding its own legitimate name could never be green.

What changed:

1. p1_spine: the block is delimited by the indentation of its marker, whatever it is, and a line at the same indentation that is a list item (- …) is still inside the block (kustomize writes volumeMounts: and then - mountPath: at the same column).
2. p1_list: same rule for the env and mounts lists, so the name sets really cover every container.
3. the allowed set is a superset test (every added name must be one of the allowed ones), which is what the comments describe; an unexpected name is still red.
4. p1_lab_sts_bad takes an optional fourth argument so one sandbox can allow one extra name: TMPDIR for sbx-gits only. sbx-lab's set is not loosened. The value is not left free — §24 pins TMPDIR=/home/ld/work/seat-transcripts structurally.

Verified on the real render (not by eye): spine identical live-vs-branch (90 lines), env delta exactly TMPDIR, mounts 16→16 and volumes 16→16 unchanged, and the function still bites — renaming the variable to something else is env-anadidos: OTRA_COSA, and dropping the allowance is env-anadidos: TMPDIR.

Harness, solo, on be0f6366: 603 checks, 5 failed, 0 pending. The five are the pre-existing reds and the #851 drift, none of them from this branch:

FAIL  probe uses an EMPTY pre-existing --dir and leaves it in place (rc=2)      ← known
FAIL  manifest[2] memoria requests 6Gi / limit 10Gi                             ← #851 (live: 12Gi/13Gi)
FAIL  manifest[3]  imagenes = sbx-fleet@sha256:7e0513ca…                        ← pre-existing
FAIL  manifest[3b] bump de imagen H5 (falta: …)                                 ← pre-existing
FAIL  T7 emulador dentro del pod (requests-memoria=18, limits-memoria=21, limits-cpu=24)  ← #851: 12Gi+6Gi

T7[diff] and 17[R159] (d) are green (set = M README.md, M statefulset.yaml). All 8 of §24's transcript checks pass.

Two honest notes: (a) the guard fix governs the names of env vars — an env value change is not caught by it; for TMPDIR the value is pinned by §24, and for any other variable that was already true before this PR; (b) neither PR touches scripts/gits-sbx-epic-test.sh sections 5/10 numbers that #851 invalidated — that needs its own ticket, see the PR body.

Head moved: `be0f6366` (was `93bbb5d4`). Adding the `TMPDIR` env surfaced a defect in the guard this PR has to pass, and it is fixed in the same branch rather than worked around: **`p1_spine` / `p1_list` (HOMELAB-2537 guard) never actually stripped anything in the rendered document.** They matched `volumeMounts:` / `args:` / `env:` at ONE fixed indentation, but `sts_doc` is `kubectl kustomize` output and kustomize sorts a container's keys, so the block can be the item itself (` - env:`). Consequences measured on the real render: * the "spine" was the whole template, so **any** added env var came out as `spine-distinto` — the opposite of what the function's own comment promises (it says a wave that adds a NAMED variable must not be forced to edit the spine); * `p1_list … env` saw **29 of the pod's env names, none of them from the `sandbox` container**: the allowed set `want_env='SEED_FORGEJO_EXT '` was never applied to it (that literal only ever matched the initContainer); * the allowed-set test compared for **equality against the delta**, so the only delta that could ever pass was the empty one — a wave adding its own legitimate name could never be green. What changed: 1. `p1_spine`: the block is delimited by the **indentation of its marker**, whatever it is, and a line at the same indentation that is a list item (`- …`) is still inside the block (kustomize writes ` volumeMounts:` and then ` - mountPath:` at the same column). 2. `p1_list`: same rule for the `env` and `mounts` lists, so the name sets really cover every container. 3. the allowed set is a **superset** test (every added name must be one of the allowed ones), which is what the comments describe; an unexpected name is still red. 4. `p1_lab_sts_bad` takes an optional fourth argument so **one** sandbox can allow **one** extra name: `TMPDIR` for `sbx-gits` only. `sbx-lab`'s set is not loosened. The *value* is not left free — §24 pins `TMPDIR=/home/ld/work/seat-transcripts` structurally. Verified on the real render (not by eye): spine **identical** live-vs-branch (90 lines), env delta exactly `TMPDIR`, mounts 16→16 and volumes 16→16 unchanged, and the function still bites — renaming the variable to something else is `env-anadidos: OTRA_COSA`, and dropping the allowance is `env-anadidos: TMPDIR`. Harness, solo, on `be0f6366`: **603 checks, 5 failed, 0 pending**. The five are the pre-existing reds and the #851 drift, none of them from this branch: ``` FAIL probe uses an EMPTY pre-existing --dir and leaves it in place (rc=2) ← known FAIL manifest[2] memoria requests 6Gi / limit 10Gi ← #851 (live: 12Gi/13Gi) FAIL manifest[3] imagenes = sbx-fleet@sha256:7e0513ca… ← pre-existing FAIL manifest[3b] bump de imagen H5 (falta: …) ← pre-existing FAIL T7 emulador dentro del pod (requests-memoria=18, limits-memoria=21, limits-cpu=24) ← #851: 12Gi+6Gi ``` `T7[diff]` and `17[R159] (d)` are green (set = `M README.md`, `M statefulset.yaml`). All 8 of §24's transcript checks pass. Two honest notes: (a) the guard fix governs the *names* of env vars — an env **value** change is not caught by it; for `TMPDIR` the value is pinned by §24, and for any other variable that was already true before this PR; (b) neither PR touches `scripts/gits-sbx-epic-test.sh` sections 5/10 numbers that #851 invalidated — that needs its own ticket, see the PR body.

aaron added 1 commit 2026-11-06 08:21:38 +00:00

Merge remote-tracking branch 'origin/live' into plane/HOMELAB-2682-transcripts ... ab54cc5bee

# Conflicts:
#	scripts/gits-sbx-epic-test.sh

aaron added 1 commit 2026-11-06 08:40:41 +00:00

HOMELAB-2682: fix(sbx-gits): T7[diff] and 17[R159](d) name THIS branch's set (README + statefulset.yaml) ... 8180f4eb8c

The two PASS messages merged in from origin/live still narrated the declaration
branch's set (tools/gits-sbx-emu-proof.sh + README.md) and asserted that
statefulset.yaml did NOT move. After the merge that sentence is false for this
branch: the comparison itself was already driven by SBX_GITS_DIFF_SET (both
mutations bite), but the message a reviewer reads named the wrong files.

No behaviour change: the literal, the mutations and the check stay as they are.

ld-reviewer commented 2026-11-06 08:53:49 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 8180f4eb8c: REQUEST_CHANGES
model: deepseek-flash
session: sess-2682-s2-a7f3e1

Seat 2 of 2 (adversarial). The TMPDIR change itself is right and I verified it; what blocks is the harness rewrite that came with it.

Verified good (independently, not from the PR prose)

• Render diff is exactly the env line. kubectl kustomize core/manifests/apps/sbx-gits at head (8180f4eb) and at origin/live (5b0d7f43): pure addition, zero removals — - name: TMPDIR / value: /home/ld/work/seat-transcripts in the sandbox env, plus the seed's mkdir -p and its comment. securityContext, resources, nodeSelector, volumes, volumeMounts, kustomization.yaml byte-identical; the pod has no probes at all, so none can have drifted.
• Container isolation holds. In the head render the emulator mounts only avd (emptyDir 16Gi), has no work mount and no TMPDIR env. No cross-container collision on the hostPath is possible, and the emulator's /tmp stays the emptyDir.
• The mechanism is real: node's tmpdir() honours TMPDIR and createOutputFilePath joins it — the suite executes that rather than quoting the manifest.
• bash -n clean on scripts/gits-sbx-epic-test.sh at head and at base.

Blockers

• S2-1: blocker — the p1_spine rewrite silently drops whole container items from the compared document. The head spine contains no name: sandbox, no name: prepare-work, no capabilities for prepare-work (add:/CHOWN), and only 2 of the render's 4 image: harbor… and 2 of 4 terminationMessagePath lines; at base all of them are present. Cause: when kustomize writes the marker as the item itself ( - env:), base is the container list's own indentation, so the RLENGTH > base rule swallows every following sibling key of that item instead of ending at the first non-list line at item-key depth. Demonstrated on the rendered document: memory: 13Gi → 20Gi inside sandbox leaves the head spine byte-identical (0 diff lines) while the base spine flags it. The loss is in fields no other section guards: the sandbox's image:, its readOnlyRootFilesystem / allowPrivilegeEscalation / capabilities.drop, and prepare-work's root securityContext and CHOWN/FOWNER/DAC_OVERRIDE — in the one pod whose sandbox container holds every credential. (§10(b) still pins "privileged" and §10(d) the resource totals, so those two classes survive; nothing pins the rest.) Fix is bounded: end the strip at the first line at base+2 that is not a list item when the marker is the - key: form, and add both mutations (sandbox capabilities, prepare-work runAsUser) to the harness's own mutation list so this cannot regress silently.

Concerns

• C-1 disk growth, measured: no rotation is added and the target volume is already at 39G (kubectl exec -n sbx-gits sbx-gits-0 -c sandbox -- du -sh /home/ld/work: epics 27G, tmp 1.9G, .gradle 2.0G, android-sdk 2.8G) against the 30 GiB cap README §1.1 publishes (alert 25). The only enforced bound I can find lives inside tools/gits-sbx-android-install.sh (30 GiB warn, 40 GiB hard stop) and runs only when a human runs it; core/manifests/monitoring/rules/sbx-gits-alerts.yaml has exactly two rules (CPU throttle, emulator restart) and no volume rule. The disk has room (/dev/vdb1 500G, 381G free, 24%), so this is a cap-and-observability gap, not a full disk today. Size estimate: I could not measure a seat transcript — the live pod has no /tmp/pi-subagents-* (0 .output files) — so "se vigila, no se estima" currently rests on no automated check.
• C-2 credentials now persist. TMPDIR catches every mktemp in the container: the pinned tools ConfigMap's sbx-ci-log.sh does WORK=$(mktemp -d) and writes the trimmed ci-reader password ($WORK/pw, 0600) and the session cookie jar there, removed only by its EXIT trap. A SIGKILL or an eviction mid-run used to take those bytes with the emptyDir; now they stay on the persistent volume. gits-sbx-emu-lock.sh --selftest likewise.
• C-3 mode/ownership, honest limit. The seed runs as pod-level uid 1000 (no runAsUser override) with umask 0022 (measured in the live pod) and prepare-work has already made /home/ld/work 1000:1000 0755, so the new directory is 0755 1000:1000: every orchestrator and every worker in this pod is uid 1000 and can read every other seat's transcript, and the path is readable at node level on prod-wk-01 by anything that can reach /var/mnt/u-longhorn (the privileged emulator container cannot — it has no work mount). redact (HOMELAB-2680) is pattern-based and cannot be proven complete over a file that now survives eviction. One-line hardening if wanted: install -d -m 0700 … in the seed.
• C-4 blast radius, for the record: TMPDIR is read by node/pi (seat transcripts, pi tool-output files, session-share, external editor), adb/sdkmanager (the installer already forces its own TMPDIR under /home/ld/work for exactly that reason), Go's build temp (GOCACHE/GOMODCACHE are already on the volume) and the pod's mktemp users; not Gradle/JVM (java.io.tmpdir never reads TMPDIR and the pod sets no JAVA_TOOL_OPTIONS). I found no unix socket under TMPDIR anywhere in this pod's tooling, so the longer path carries no AF_UNIX 108-byte risk. Nothing breaks against a writable uid-1000 directory.
• C-5 nit: scripts/gits-sbx-epic-test.sh around line 3428 — the comment is corrupted to "kustomize sorts the keys of a container: keys, so - name: is NOT the first line of". Restore the original sentence.

Scope note: I did not run the full 14.6k-line suite (outside a review round's budget). I ran bash -n on both revisions and drove p1_spine/p1_list against the real kustomize renders of both heads — which is how S2-1 was found.

**Verdict of record — ld-reviewer — head 8180f4eb8cb8ede07670c83f93b60e595eec35e8: REQUEST_CHANGES** model: deepseek-flash session: sess-2682-s2-a7f3e1 Seat 2 of 2 (adversarial). The TMPDIR change itself is right and I verified it; what blocks is the harness rewrite that came with it. ## Verified good (independently, not from the PR prose) - **Render diff is exactly the env line.** `kubectl kustomize core/manifests/apps/sbx-gits` at head (`8180f4eb`) and at `origin/live` (`5b0d7f43`): **pure addition, zero removals** — `- name: TMPDIR` / `value: /home/ld/work/seat-transcripts` in the `sandbox` env, plus the seed's `mkdir -p` and its comment. `securityContext`, `resources`, `nodeSelector`, `volumes`, `volumeMounts`, `kustomization.yaml` byte-identical; the pod has no probes at all, so none can have drifted. - **Container isolation holds.** In the head render the `emulator` mounts **only** `avd` (emptyDir 16Gi), has **no** `work` mount and **no** `TMPDIR` env. No cross-container collision on the hostPath is possible, and the emulator's `/tmp` stays the emptyDir. - **The mechanism is real**: node's `tmpdir()` honours TMPDIR and `createOutputFilePath` joins it — the suite executes that rather than quoting the manifest. - **`bash -n` clean** on `scripts/gits-sbx-epic-test.sh` at head and at base. ## Blockers - S2-1: blocker — the `p1_spine` rewrite silently drops **whole container items** from the compared document. The head spine contains no `name: sandbox`, no `name: prepare-work`, no `capabilities` for prepare-work (`add:`/`CHOWN`), and only 2 of the render's 4 `image: harbor…` and 2 of 4 `terminationMessagePath` lines; at base all of them are present. Cause: when kustomize writes the marker as the item itself (` - env:`), `base` is the container list's own indentation, so the `RLENGTH > base` rule swallows every following sibling key of that item instead of ending at the first non-list line at item-key depth. Demonstrated on the rendered document: `memory: 13Gi → 20Gi` inside `sandbox` leaves the head spine byte-identical (0 diff lines) while the base spine flags it. The loss is in fields no other section guards: the `sandbox`'s `image:`, its `readOnlyRootFilesystem` / `allowPrivilegeEscalation` / `capabilities.drop`, and prepare-work's root `securityContext` and `CHOWN`/`FOWNER`/`DAC_OVERRIDE` — in the one pod whose `sandbox` container holds every credential. (§10(b) still pins "privileged" and §10(d) the resource totals, so those two classes survive; nothing pins the rest.) Fix is bounded: end the strip at the first line at `base+2` that is not a list item when the marker is the `- key:` form, and add both mutations (`sandbox` capabilities, `prepare-work` `runAsUser`) to the harness's own mutation list so this cannot regress silently. ## Concerns - C-1 disk growth, measured: no rotation is added and the target volume is **already at 39G** (`kubectl exec -n sbx-gits sbx-gits-0 -c sandbox -- du -sh /home/ld/work`: `epics` 27G, `tmp` 1.9G, `.gradle` 2.0G, `android-sdk` 2.8G) against the **30 GiB** cap README §1.1 publishes (alert 25). The only enforced bound I can find lives inside `tools/gits-sbx-android-install.sh` (30 GiB warn, 40 GiB hard stop) and runs only when a human runs it; `core/manifests/monitoring/rules/sbx-gits-alerts.yaml` has exactly two rules (CPU throttle, emulator restart) and **no** volume rule. The disk has room (`/dev/vdb1` 500G, 381G free, 24%), so this is a cap-and-observability gap, not a full disk today. Size estimate: I could **not** measure a seat transcript — the live pod has no `/tmp/pi-subagents-*` (0 `.output` files) — so "se vigila, no se estima" currently rests on no automated check. - C-2 credentials now persist. TMPDIR catches every `mktemp` in the container: the pinned tools ConfigMap's `sbx-ci-log.sh` does `WORK=$(mktemp -d)` and writes the **trimmed ci-reader password** (`$WORK/pw`, 0600) and the session cookie jar there, removed only by its EXIT trap. A SIGKILL or an eviction mid-run used to take those bytes with the emptyDir; now they stay on the persistent volume. `gits-sbx-emu-lock.sh --selftest` likewise. - C-3 mode/ownership, honest limit. The seed runs as pod-level uid 1000 (no `runAsUser` override) with umask **0022** (measured in the live pod) and `prepare-work` has already made `/home/ld/work` 1000:1000 0755, so the new directory is **0755 1000:1000**: every orchestrator and every worker in this pod is uid 1000 and can read every other seat's transcript, and the path is readable at node level on `prod-wk-01` by anything that can reach `/var/mnt/u-longhorn` (the privileged `emulator` container cannot — it has no `work` mount). `redact` (HOMELAB-2680) is pattern-based and cannot be proven complete over a file that now survives eviction. One-line hardening if wanted: `install -d -m 0700 …` in the seed. - C-4 blast radius, for the record: TMPDIR is read by node/pi (seat transcripts, pi tool-output files, session-share, external editor), `adb`/`sdkmanager` (the installer already forces its own TMPDIR under `/home/ld/work` for exactly that reason), Go's build temp (GOCACHE/GOMODCACHE are already on the volume) and the pod's `mktemp` users; **not** Gradle/JVM (`java.io.tmpdir` never reads TMPDIR and the pod sets no `JAVA_TOOL_OPTIONS`). I found no unix socket under TMPDIR anywhere in this pod's tooling, so the longer path carries no `AF_UNIX` 108-byte risk. Nothing breaks against a writable uid-1000 directory. - C-5 nit: `scripts/gits-sbx-epic-test.sh` around line 3428 — the comment is corrupted to "kustomize sorts the keys of a container: keys, so `- name:` is NOT the first line of". Restore the original sentence. Scope note: I did not run the full 14.6k-line suite (outside a review round's budget). I ran `bash -n` on both revisions and drove `p1_spine`/`p1_list` against the real `kustomize` renders of both heads — which is how S2-1 was found.

ld-reviewer commented 2026-11-06 08:59:31 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head 8180f4eb8c: APPROVE
model: deepseek-flash
session: 01a1b008-bdf4-757a-b84d-27fb16a5264f

Independent review seat 1 of 2. Reviewed from primary sources: git diff 5b0d7f43...8180f4eb, a side-by-side kubectl kustomize core/manifests/apps/sbx-gits render of origin/live (5b0d7f43) vs this head, structural (PyYAML) inspection of both renders, and my own solo harness runs on both trees.

What I verified

Render diff vs origin/live is exactly two hunks, nothing else.

• + - name: TMPDIR / value: /home/ld/work/seat-transcripts in the sandbox container (containers[0]), and nowhere else.
• + mkdir -p /home/ld/work/seat-transcripts || printf 'SEED-WARN: …' in seed-pi-agent.
• Everything else byte-identical in the render: sandbox resources requests 2 / 12Gi / 1Gi + limits 12 / 13Gi / 10Gi (live's floors, unchanged), the same 16 volume names both sides, hostPath work = /var/mnt/u-longhorn/sbx-gits/work (DirectoryOrCreate), initContainers = [prepare-work, seed-pi-agent] both sides, emulator env still 3 names with no TMPDIR, kustomization.yaml untouched. No PVC, no volume, no mount, no resource change.

The target is the persistent volume, not the ephemeral one. volumeMounts: /home/ld/work <- work (the hostPath) and /tmp <- tmp (the 4 Gi emptyDir). TMPDIR points into the hostPath mount, so the value is reachable and not on emptyDir.

The seed creates it before any session, as the right user. seed-pi-agent is an initContainer running after prepare-work and before the containers start; pod securityContext is runAsUser/runAsGroup/fsGroup: 1000 and the seed carries no runAsUser override, so mkdir -p lands 1000:1000. The sandbox container runs as the same uid 1000 → writable by pi, adb and go. set -uo pipefail (no -e) plus || printf means a failure prints SEED-WARN and the seed continues, as documented.

The mechanism is executed, not asserted in prose. Harness §28 reads the manifest structurally (PyYAML, not a grep) and asserts the sandbox value, the emulator's absence, that /home/ld/work resolves to the hostPath, and the seed's mkdir; it also runs TMPDIR=… node -e 'require("os").tmpdir()' so the assertion rests on the runtime. It passed on this head (executed, not skipped).

Scope survived the revival. Three files vs live: core/manifests/apps/sbx-gits/statefulset.yaml, core/manifests/apps/sbx-gits/README.md, scripts/gits-sbx-epic-test.sh. No tools/ change, so the #870 ConfigMap delivery pin is not traversed and needs no bump PR. The TMPDIR value and the seed line are the same ones the branch's original commit carried (93bbb5d4); the origin/live merge kept this branch's addition and live's later floor changes (memory 12Gi/13Gi), which is the resolution the PR claims. sbx-lab / sbx-sendrast manifests are not in the diff.

Harness solo: no new reds. My own run on this head: RESULT: FAIL (6 of 681 checks failed, 6 skipped, 0 pending), and the six are the pre-existing ones this PR documents (probe uses an EMPTY pre-existing --dir, manifest[2] 6Gi/10Gi, manifest[3], manifest[3b], the T7 resource totals row, T7+R99[10b] generator legs). §28's nine assertions PASS.

Blockers

None.

Concerns

• Baseline-solo comparison is partial at the time of writing (disclosed, not assumed away). My origin/live (5b0d7f43) solo run was still in section 14 of 28 when I posted this; the sorted FAIL set it had produced by then is byte-identical to the head's six (diff clean), and those six are exactly the set the PR documents as pre-existing. I did not let the baseline finish, so treat "zero new reds" as "the six documented reds are reproduced on both trees, and the head adds none among the sections the baseline reached". The full byte-identical comparison should be completed by the landing worker or the second seat.
• The harness that gates this change is edited by it. p1_spine / p1_list now delimit env: / args: / volumeMounts: by marker indentation (at the rendered indentation the old fixed ^ env: matched nothing, so the "spine" was the whole template and any added variable read as spine-distinto), and p1_lab_sts_bad moved from delta-equality to a superset test. The intent is defensible and the file set stays exact (T7[diff] is still red on a third file, verified in the diff), but the net effect is that the guard on this very PR is looser than on live: any subset of SEED_FORGEJO_EXT TMPDIR may now be added under sbx-gits, where live effectively only accepted the empty delta. A name outside that set is still red. Flagging it because a reviewer should not have to derive that from the awk.
• The seed cannot detect a wrong-owner directory. mkdir -p returns 0 on an existing-but-unwritable directory, so if /home/ld/work/seat-transcripts ever exists owned by another uid, the seed reports success and pi's transcripts silently fail to land. Nothing in the pod creates it root-owned today (the emulator container mounts only /avd, not work), so this is robustness rather than a defect; install -d -o 1000 -g 1000 -m 0755 in the root prepare-work (which already holds CHOWN/FOWNER) would make it self-healing, at the cost of touching a second initContainer.
• Mode and sensitivity. The directory inherits the seed's umask (0755, 1000:1000). That is no regression versus /tmp, where Node's own mkdir also produced 0755, and it must be writable by 1000 to be useful; worth stating only because the content is agent transcripts on a node-local volume (readable by root on prod-wk-01).
• TMPDIR is inherited by the whole sandbox container, not just pi. The tools scripts are affected too: tools/gits-sbx-emu-proof.sh uses mktemp "${TMPDIR:-/tmp}/…" for its normalisation/declaration temp files, and tools/sbx-ci-log.sh does WORK=$(mktemp -d) — those now land on the hostPath under the sandbox's 30 GiB budget. gits-sbx-android-install.sh already sets its own TMPDIR under $WORK, and gits-sbx-emu-smoke.sh sets FIXTURE_TMP explicitly, so both are unaffected. This matches the "watch it, don't estimate it" note; it is not free, and /home/ld/work usage is the thing to watch after the roll.
• Cosmetic: scripts/gits-sbx-epic-test.sh:3431 now reads "kustomize sorts the keys of a container: keys, so - name: is NOT the first line of" — a garbled rewrite of the original comment.

**Verdict of record — ld-reviewer — head 8180f4eb8cb8ede07670c83f93b60e595eec35e8: APPROVE** model: deepseek-flash session: 01a1b008-bdf4-757a-b84d-27fb16a5264f Independent review seat 1 of 2. Reviewed from primary sources: `git diff 5b0d7f43...8180f4eb`, a side-by-side `kubectl kustomize core/manifests/apps/sbx-gits` render of `origin/live` (`5b0d7f43`) vs this head, structural (PyYAML) inspection of both renders, and my own solo harness runs on both trees. ## What I verified **Render diff vs `origin/live` is exactly two hunks, nothing else.** - `+ - name: TMPDIR / value: /home/ld/work/seat-transcripts` in the **`sandbox`** container (`containers[0]`), and nowhere else. - `+ mkdir -p /home/ld/work/seat-transcripts || printf 'SEED-WARN: …'` in `seed-pi-agent`. - Everything else byte-identical in the render: `sandbox` resources `requests 2 / 12Gi / 1Gi` + `limits 12 / 13Gi / 10Gi` (live's floors, unchanged), the same 16 volume names both sides, `hostPath work = /var/mnt/u-longhorn/sbx-gits/work` (DirectoryOrCreate), `initContainers = [prepare-work, seed-pi-agent]` both sides, `emulator` env still 3 names with **no** `TMPDIR`, `kustomization.yaml` untouched. No PVC, no volume, no mount, no resource change. **The target is the persistent volume, not the ephemeral one.** `volumeMounts`: `/home/ld/work <- work` (the hostPath) and `/tmp <- tmp` (the 4 Gi `emptyDir`). `TMPDIR` points into the hostPath mount, so the value is reachable and not on `emptyDir`. **The seed creates it before any session, as the right user.** `seed-pi-agent` is an initContainer running after `prepare-work` and before the containers start; pod `securityContext` is `runAsUser/runAsGroup/fsGroup: 1000` and the seed carries no `runAsUser` override, so `mkdir -p` lands `1000:1000`. The `sandbox` container runs as the same uid 1000 → writable by pi, `adb` and `go`. `set -uo pipefail` (no `-e`) plus `|| printf` means a failure prints `SEED-WARN` and the seed continues, as documented. **The mechanism is executed, not asserted in prose.** Harness §28 reads the manifest structurally (PyYAML, not a grep) and asserts the sandbox value, the emulator's absence, that `/home/ld/work` resolves to the `hostPath`, and the seed's `mkdir`; it also runs `TMPDIR=… node -e 'require("os").tmpdir()'` so the assertion rests on the runtime. It passed on this head (executed, not skipped). **Scope survived the revival.** Three files vs `live`: `core/manifests/apps/sbx-gits/statefulset.yaml`, `core/manifests/apps/sbx-gits/README.md`, `scripts/gits-sbx-epic-test.sh`. No `tools/` change, so the #870 ConfigMap delivery pin is not traversed and needs no bump PR. The `TMPDIR` value and the seed line are the same ones the branch's original commit carried (`93bbb5d4`); the `origin/live` merge kept this branch's addition **and** live's later floor changes (memory 12Gi/13Gi), which is the resolution the PR claims. `sbx-lab` / `sbx-sendrast` manifests are not in the diff. **Harness solo: no new reds.** My own run on this head: `RESULT: FAIL (6 of 681 checks failed, 6 skipped, 0 pending)`, and the six are the pre-existing ones this PR documents (`probe uses an EMPTY pre-existing --dir`, `manifest[2]` 6Gi/10Gi, `manifest[3]`, `manifest[3b]`, the `T7` resource totals row, `T7+R99[10b]` generator legs). §28's nine assertions PASS. ## Blockers None. ## Concerns - **Baseline-solo comparison is partial at the time of writing (disclosed, not assumed away).** My `origin/live` (`5b0d7f43`) solo run was still in section 14 of 28 when I posted this; the sorted FAIL set it had produced by then is *byte-identical* to the head's six (`diff` clean), and those six are exactly the set the PR documents as pre-existing. I did not let the baseline finish, so treat "zero new reds" as "the six documented reds are reproduced on both trees, and the head adds none among the sections the baseline reached". The full byte-identical comparison should be completed by the landing worker or the second seat. - **The harness that gates this change is edited by it.** `p1_spine` / `p1_list` now delimit `env:` / `args:` / `volumeMounts:` by marker indentation (at the rendered indentation the old fixed `^ env:` matched nothing, so the "spine" was the whole template and any added variable read as `spine-distinto`), and `p1_lab_sts_bad` moved from delta-equality to a superset test. The intent is defensible and the file set stays exact (`T7[diff]` is still red on a third file, verified in the diff), but the net effect is that the guard on this very PR is looser than on live: any *subset* of `SEED_FORGEJO_EXT TMPDIR` may now be added under `sbx-gits`, where live effectively only accepted the empty delta. A name outside that set is still red. Flagging it because a reviewer should not have to derive that from the awk. - **The seed cannot detect a wrong-owner directory.** `mkdir -p` returns 0 on an existing-but-unwritable directory, so if `/home/ld/work/seat-transcripts` ever exists owned by another uid, the seed reports success and pi's transcripts silently fail to land. Nothing in the pod creates it root-owned today (the `emulator` container mounts only `/avd`, not `work`), so this is robustness rather than a defect; `install -d -o 1000 -g 1000 -m 0755` in the root `prepare-work` (which already holds `CHOWN`/`FOWNER`) would make it self-healing, at the cost of touching a second initContainer. - **Mode and sensitivity.** The directory inherits the seed's umask (0755, `1000:1000`). That is no regression versus `/tmp`, where Node's own `mkdir` also produced 0755, and it must be writable by 1000 to be useful; worth stating only because the content is agent transcripts on a node-local volume (readable by root on `prod-wk-01`). - **`TMPDIR` is inherited by the whole `sandbox` container, not just pi.** The tools scripts are affected too: `tools/gits-sbx-emu-proof.sh` uses `mktemp "${TMPDIR:-/tmp}/…"` for its normalisation/declaration temp files, and `tools/sbx-ci-log.sh` does `WORK=$(mktemp -d)` — those now land on the `hostPath` under the sandbox's 30 GiB budget. `gits-sbx-android-install.sh` already sets its own `TMPDIR` under `$WORK`, and `gits-sbx-emu-smoke.sh` sets `FIXTURE_TMP` explicitly, so both are unaffected. This matches the "watch it, don't estimate it" note; it is not free, and `/home/ld/work` usage is the thing to watch after the roll. - **Cosmetic:** `scripts/gits-sbx-epic-test.sh:3431` now reads "kustomize sorts the keys of a container: keys, so `- name:` is NOT the first line of" — a garbled rewrite of the original comment.

pi-landing added 1 commit 2026-11-06 09:20:39 +00:00

HOMELAB-2682: fix(sbx-gits): guard spine covers every container (names, images, securityContext) ... a29e9775c1

p1_spine delimited a stripped block by the INDENTATION OF ITS MARKER. When
kustomize writes the marker as the item itself (`      - env:`, six spaces =
the indentation of the CONTAINER LIST ITEM, body at eight) every sibling key
of that container is deeper than the marker, so the strip swallowed the whole
item: the head spine carried NO `name: sandbox`, NO `name: prepare-work`, 2 of
the pod's 4 `image:` lines, and lost the sandbox's image /
readOnlyRootFilesystem / capabilities.drop and prepare-work's root
securityContext (CHOWN/FOWNER/DAC_OVERRIDE) — fields no other section pins.
Proven by mutation: `memory: 13Gi -> 20Gi` inside `sandbox` left the head
spine byte-identical while `prepare-work`'s `runAsUser: 0` and the sandbox's
`readOnlyRootFilesystem` were equally invisible.

1. p1_spine: a block now ends at the first line, at the indentation of the
   block's FIRST BODY line, that is not a list item — so a container's next
   sibling key ends the block and is PRINTED. Every container keeps its name,
   full `image:` line, securityContext (runAs*, readOnlyRootFilesystem,
   capabilities) and resources; nothing is re-narrowed (the spine went 90 ->
   144 lines: it now also keeps `command:`).
2. p1_list: same rule for the `env` and `mounts` lists (the same latent bug),
   and a line that ends a block is re-tested as the next marker.
3. p1_containers/p1_census_missing: an INDEPENDENT, strip-free census (per
   container: name, full image line, whole securityContext block).
   p1_lab_sts_bad now requires live's census == now's AND every census line to
   appear verbatim in the spine — so a strip that goes lossy again reports
   spine-perdio-campos/contenedores-distintos instead of "identical to live".
4. H8[8][censo]/H8[8][mut]: the guard is now proved to BITE on the real
   render — 10 mutations, each applied inside ONE container of
   `kubectl kustomize` output: sandbox memory/image/readOnlyRootFilesystem/
   capabilities, prepare-work capabilities (CHOWN) and root runAsUser,
   emulator memory, and an unpermitted env NAME in sandbox, emulator and
   prepare-work (which pins that the env-name set really covers every
   container). Each must be red for the right reason.

Verified on the real render: `kubectl kustomize core/manifests/apps/sbx-gits`
at this head vs origin/live differs by exactly two hunks (the sandbox TMPDIR
env name and the seed's mkdir line), the new spine is identical live-vs-head
and carries all four container names and all four image lines.

ld-reviewer commented 2026-11-06 09:45:33 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head a29e9775c1: APPROVE
model: deepseek-flash
session: s2-r2-2682-6b1f4d92

Seat 2 of 2, round 2 (adversarial delta). Read from primary sources: git diff 8180f4eb..a29e9775, my own kubectl kustomize core/manifests/apps/sbx-gits renders of head and origin/live (5b0d7f43), the four p1_* functions cut out of scripts/gits-sbx-epic-test.sh and driven standalone against 40+ mutations of the REAL render, and a solo harness run on the head.

The blocker from round 1 (S2-1) is closed. Verified by mutation, not by prose: the head spine now carries every container.

claim	evidence
spine is identical live↔head	p1_spine live == p1_spine head, 144 lines both
spine is not LOSSY (the round-1 blocker)	per-field audit of the render: every container key is in the spine except args/env/volumeMounts (stripped by design), volumes at pod level; name:, the full image: line, resources:, securityContext: (4 container + 1 pod), imagePullPolicy, terminationMessagePath/Policy all present for all four containers
the round-1 counter-example now bites	memory: 13Gi -> 20Gi inside sandbox → spine-distinto (was byte-identical)
the census is a real second derivation	p1_containers head == live, image count spine(4) == census(4), all four names present; p1_census_missing empty on both; initContainers ORDER swapped → spine-distinto contenedores-distintos

Blockers

None.

Resolutions

• S2-1 (round 1: "p1_spine silently drops whole container items"). Verified fixed on the head render. I re-ran the exact round-1 attack (memory: 13Gi -> 20Gi in sandbox) and it is now spine-distinto; I also confirmed structurally that sandbox and prepare-work are back in the spine with their image: lines and their whole securityContext: blocks (readOnlyRootFilesystem, capabilities.drop: [ALL], capabilities.add: [CHOWN, FOWNER, DAC_OVERRIDE], runAsUser: 0), which is what the old strip swallowed. The end-of-block rule (first body-line indentation, plus "a line that ends a block is re-tested as the next marker") holds for both layouts kustomize emits ( - env: and volumeMounts:).
• The census/p1_census_missing backstop is genuinely independent. It is a second derivation that does not use the strip at all, and the two new failure reasons (contenedores-distintos, spine-perdio-campos) are what make a re-lossy spine impossible to hide: I could not construct a mutation that drops a container field from the spine without tripping at least one of the two. That is the structural answer to round 1, and it is better than a fixed regex would have been.
• The guard bites on the real render, on fields nothing else pins. I reproduced the harness's own 10-row mutation table independently (all 10 RED for the stated reason: resources, image, readOnlyRootFilesystem, capabilities, CHOWN, runAsUser, emulator memory, three env-name rows) and extended it — 27 further mutations, all RED: ephemeral-storage, requests, probes ADDED (liveness and readiness — the pod has none, so none can silently lose one), lifecycle, envFrom: secretRef, imagePullPolicy, command, stdin, terminationMessagePath, nodeSelector, tolerations ADDED, affinity ADDED, hostNetwork, shareProcessNamespace, serviceAccountName, automountServiceAccountToken, dnsPolicy, terminationGracePeriodSeconds, restartPolicy, replicas, updateStrategy, serviceName, selector matchLabels, template labels, a fifth container (spine-imagenes-5/censo-4), and initContainers order.
• p1_list really reads every container now. The round-1 bug's signature is gone: an env name added to sandbox, to emulator and to prepare-work (the harness's own three rows) all surface as env-anadidos: MUT_ENV; new mountPaths surface as mounts-anadidos. The superset semantics are as documented, and the per-sandbox exception is scoped: [[ $d == sbx-gits ]] && d_extra_env='TMPDIR', so sbx-lab/sbx-sendrast keep the empty allowance.
• env VALUE handling is exactly as claimed (your item 2). A value change does NOT flag the spine — I mutated HOME=/home/ld -> /root in sandbox and ANDROID_AVD_HOME in emulator: green, by design, because env: is stripped. For TMPDIR the value is caught, and caught structurally: §28 parses the manifest with PyYAML and asserts sandbox TMPDIR == /home/ld/work/seat-transcripts, emulator has none, and the mounted volume for /home/ld/work resolves to a hostPath under /var/mnt/u-longhorn/*; it also executes node's tmpdir(). So the one value this PR introduces is pinned in one place and the spine does not have to carry values.
• Runtime / flakiness (your item 3). The delta's own cost is small and I measured it: p1_lab_sts_bad + p1_census_missing over the 901-line render take ~0.38 s per pair, and the H8[8][mut] block runs ~12 of them → ~5 s added to a run that is otherwise ~10 min. My solo run on the head: 470 PASS, 6 FAIL, and the six are byte-for-byte the pre-existing set this PR documents (probe uses an EMPTY pre-existing --dir, manifest[2] 6Gi/10Gi, manifest[3], manifest[3b], the T7 totals row, T7+R99[10b] generator legs). Zero new reds. H8[8][sbx-gits][censo] and all 12 [mut] rows PASS in the run itself. No ordering dependency found in the new code: the mutation heredoc cannot steal the outer loop's stdin (the outer iteration is a for, and the heredoc is scoped to the inner loop); $R8/mut.sts is written and consumed inside the same iteration; the counts are read from FILES with grep -c precisely to dodge the SIGPIPE/141 race under set -uo pipefail (a real hazard, correctly avoided); d/d_extra_env are assigned before every use on the green path. The block skips (never falsely fails) when either .sts render is missing. I did not let the run finish (it was in section 22/28 when I posted), so treat the "zero new reds" as covering sections 1–22 plus the whole of §8 including every new row.

Concerns

• C-1 the allowed sets are POD-WIDE UNIONS, so an already-allowed name can move into another container unseen. Verified: adding FORGEJO_API_KEY as a secretKeyRef to the emulator env, and mounting /run/secrets/ld/git into that same privileged container, both leave p1_lab_sts_bad empty (env-anadidos/mounts-anadidos are computed with comm over p1_list, which unions every container). Same shape for removal: dropping sandbox's /home/ld/work mount is green because prepare-work still carries it. §28 catches the retarget case (and the removal of that mount, via its mapping lookup), so this PR's own subject is double-covered; but "which container holds which name/path" is pinned nowhere, and the delta's own new comment says the name set "really covers every container". A container-scoped comparison — the same shape as the new census, name -> sorted(list) instead of a union — would close it. Pre-existing, not introduced here, so not a blocker.
• C-2 the persistent mount is not pinned writable. readOnly: true added to sandbox's /home/ld/work mount is green everywhere I could find (manifest[5b] pins readOnly only on the identity mounts). That is the exact write path this PR exists to protect, and "the directory exists" is pinned while "the directory is writable" is not.
• C-3 /tmp: mount removal and size are unpinned. Removing sandbox's /tmp mount, or changing the tmp emptyDir's sizeLimit: 4Gi, is green in p1_*; the 4Gi the README §1.1 budget depends on is asserted in prose only. Same class: args: is stripped for EVERY container (the strip is not container-scoped), so args: ["sleep 100000"] added to the sandbox is green in p1_* — the emulator's args are pinned by T7 and the seed's by §28/seed_of_d, the sandbox's by nothing.
• C-4 env values other than TMPDIR remain unpinned. As designed (and the round-1 note is honest about it), but the visible remainder is now larger: HOME, GOPROXY, GOTOOLCHAIN, LD_MODEL_GATEWAY_URL, FORGEJO_LEGACY_PANEL_OWNERS, PI_LANDING_IDENTITY could each be changed with everything green. Worth one deliberate ticket ("pin the values that are security-relevant") rather than a comment.
• C-5 volume BODIES are outside every p1_* guard (the spine exits at volumes:, and only names are compared). hostPath + DirectoryOrCreate are pinned by manifest[6], the identity/ci secretNames by H4/H2v3 — so the surviving hole is the tmp emptyDir above and secretName rotation of a volume whose secret another section does not name.

**Verdict of record — ld-reviewer — head a29e9775c1eece50e8cd5c5c817f7f9ee174d9d2: APPROVE** model: deepseek-flash session: s2-r2-2682-6b1f4d92 Seat 2 of 2, round 2 (adversarial delta). Read from primary sources: `git diff 8180f4eb..a29e9775`, my own `kubectl kustomize core/manifests/apps/sbx-gits` renders of head and `origin/live` (5b0d7f43), the four `p1_*` functions cut out of `scripts/gits-sbx-epic-test.sh` and driven standalone against 40+ mutations of the REAL render, and a solo harness run on the head. **The blocker from round 1 (S2-1) is closed.** Verified by mutation, not by prose: the head spine now carries every container. | claim | evidence | |---|---| | spine is identical live↔head | `p1_spine live` == `p1_spine head`, 144 lines both | | spine is not LOSSY (the round-1 blocker) | per-field audit of the render: every container key is in the spine except `args`/`env`/`volumeMounts` (stripped by design), `volumes` at pod level; `name:`, the full `image:` line, `resources:`, `securityContext:` (4 container + 1 pod), `imagePullPolicy`, `terminationMessagePath/Policy` all present for all four containers | | the round-1 counter-example now bites | `memory: 13Gi -> 20Gi` inside `sandbox` → `spine-distinto` (was byte-identical) | | the census is a real second derivation | `p1_containers` head == live, image count spine(4) == census(4), all four names present; `p1_census_missing` empty on both; initContainers ORDER swapped → `spine-distinto contenedores-distintos` | ## Blockers None. ## Resolutions - **S2-1 (round 1: "`p1_spine` silently drops whole container items").** Verified fixed on the head render. I re-ran the exact round-1 attack (`memory: 13Gi -> 20Gi` in `sandbox`) and it is now `spine-distinto`; I also confirmed structurally that `sandbox` and `prepare-work` are back in the spine with their `image:` lines and their whole `securityContext:` blocks (`readOnlyRootFilesystem`, `capabilities.drop: [ALL]`, `capabilities.add: [CHOWN, FOWNER, DAC_OVERRIDE]`, `runAsUser: 0`), which is what the old strip swallowed. The end-of-block rule (first body-line indentation, plus "a line that ends a block is re-tested as the next marker") holds for both layouts kustomize emits (` - env:` and ` volumeMounts:`). - **The census/`p1_census_missing` backstop is genuinely independent.** It is a second derivation that does not use the strip at all, and the two new failure reasons (`contenedores-distintos`, `spine-perdio-campos`) are what make a re-lossy spine impossible to hide: I could not construct a mutation that drops a container field from the spine without tripping at least one of the two. That is the structural answer to round 1, and it is better than a fixed regex would have been. - **The guard bites on the real render, on fields nothing else pins.** I reproduced the harness's own 10-row mutation table independently (all 10 RED for the stated reason: resources, image, `readOnlyRootFilesystem`, `capabilities`, `CHOWN`, `runAsUser`, emulator memory, three env-name rows) and extended it — 27 further mutations, all RED: `ephemeral-storage`, `requests`, probes ADDED (liveness and readiness — the pod has none, so none can silently lose one), `lifecycle`, `envFrom: secretRef`, `imagePullPolicy`, `command`, `stdin`, `terminationMessagePath`, `nodeSelector`, tolerations ADDED, affinity ADDED, `hostNetwork`, `shareProcessNamespace`, `serviceAccountName`, `automountServiceAccountToken`, `dnsPolicy`, `terminationGracePeriodSeconds`, `restartPolicy`, `replicas`, `updateStrategy`, `serviceName`, selector `matchLabels`, template labels, a fifth container (`spine-imagenes-5/censo-4`), and initContainers order. - **`p1_list` really reads every container now.** The round-1 bug's signature is gone: an env name added to `sandbox`, to `emulator` and to `prepare-work` (the harness's own three rows) all surface as `env-anadidos: MUT_ENV`; new mountPaths surface as `mounts-anadidos`. The superset semantics are as documented, and the per-sandbox exception is scoped: `[[ $d == sbx-gits ]] && d_extra_env='TMPDIR'`, so `sbx-lab`/`sbx-sendrast` keep the empty allowance. - **env VALUE handling is exactly as claimed (your item 2).** A value change does NOT flag the spine — I mutated `HOME=/home/ld -> /root` in `sandbox` and `ANDROID_AVD_HOME` in `emulator`: green, by design, because `env:` is stripped. For `TMPDIR` the value is caught, and caught structurally: §28 parses the manifest with PyYAML and asserts `sandbox` `TMPDIR == /home/ld/work/seat-transcripts`, `emulator` has none, and the mounted volume for `/home/ld/work` resolves to a `hostPath` under `/var/mnt/u-longhorn/*`; it also executes node's `tmpdir()`. So the one value this PR introduces is pinned in one place and the spine does not have to carry values. - **Runtime / flakiness (your item 3).** The delta's own cost is small and I measured it: `p1_lab_sts_bad` + `p1_census_missing` over the 901-line render take ~0.38 s per pair, and the H8[8][mut] block runs ~12 of them → ~5 s added to a run that is otherwise ~10 min. My solo run on the head: **470 PASS, 6 FAIL**, and the six are byte-for-byte the pre-existing set this PR documents (`probe uses an EMPTY pre-existing --dir`, `manifest[2]` 6Gi/10Gi, `manifest[3]`, `manifest[3b]`, the `T7` totals row, `T7+R99[10b]` generator legs). **Zero new reds.** `H8[8][sbx-gits][censo]` and all 12 `[mut]` rows PASS in the run itself. No ordering dependency found in the new code: the mutation heredoc cannot steal the outer loop's stdin (the outer iteration is a `for`, and the heredoc is scoped to the inner loop); `$R8/mut.sts` is written and consumed inside the same iteration; the counts are read from FILES with `grep -c` precisely to dodge the SIGPIPE/141 race under `set -uo pipefail` (a real hazard, correctly avoided); `d`/`d_extra_env` are assigned before every use on the green path. The block skips (never falsely fails) when either `.sts` render is missing. I did not let the run finish (it was in section 22/28 when I posted), so treat the "zero new reds" as covering sections 1–22 plus the whole of §8 including every new row. ## Concerns - **C-1 the allowed sets are POD-WIDE UNIONS, so an already-allowed name can move into another container unseen.** Verified: adding `FORGEJO_API_KEY` as a `secretKeyRef` to the **`emulator`** env, and mounting `/run/secrets/ld/git` into that same **privileged** container, both leave `p1_lab_sts_bad` empty (`env-anadidos`/`mounts-anadidos` are computed with `comm` over `p1_list`, which unions every container). Same shape for removal: dropping `sandbox`'s `/home/ld/work` mount is green because `prepare-work` still carries it. §28 catches the retarget case (and the removal of that mount, via its mapping lookup), so this PR's own subject is double-covered; but "which container holds which name/path" is pinned nowhere, and the delta's own new comment says the name set "really covers every container". A container-scoped comparison — the same shape as the new census, `name -> sorted(list)` instead of a union — would close it. Pre-existing, not introduced here, so not a blocker. - **C-2 the persistent mount is not pinned writable.** `readOnly: true` added to `sandbox`'s `/home/ld/work` mount is green everywhere I could find (manifest[5b] pins `readOnly` only on the identity mounts). That is the exact write path this PR exists to protect, and "the directory exists" is pinned while "the directory is writable" is not. - **C-3 `/tmp`: mount removal and size are unpinned.** Removing `sandbox`'s `/tmp` mount, or changing the `tmp` emptyDir's `sizeLimit: 4Gi`, is green in `p1_*`; the `4Gi` the README §1.1 budget depends on is asserted in prose only. Same class: `args:` is stripped for EVERY container (the strip is not container-scoped), so `args: ["sleep 100000"]` added to the `sandbox` is green in `p1_*` — the emulator's args are pinned by T7 and the seed's by §28/`seed_of_d`, the sandbox's by nothing. - **C-4 env values other than `TMPDIR` remain unpinned.** As designed (and the round-1 note is honest about it), but the visible remainder is now larger: `HOME`, `GOPROXY`, `GOTOOLCHAIN`, `LD_MODEL_GATEWAY_URL`, `FORGEJO_LEGACY_PANEL_OWNERS`, `PI_LANDING_IDENTITY` could each be changed with everything green. Worth one deliberate ticket ("pin the values that are security-relevant") rather than a comment. - **C-5 volume BODIES are outside every `p1_*` guard** (the spine exits at `volumes:`, and only names are compared). `hostPath` + `DirectoryOrCreate` are pinned by `manifest[6]`, the identity/ci `secretName`s by H4/H2v3 — so the surviving hole is the `tmp` emptyDir above and `secretName` rotation of a volume whose secret another section does not name.

ld-reviewer commented 2026-11-06 09:48:29 +00:00

Collaborator

Copy link

Verdict of record — ld-reviewer — head a29e9775c1: APPROVE
model: deepseek-flash
session: sess-2682-r2-s1-9c41d7

Independent review seat 1 of 2, round 2 (delta). Primary sources only: git diff 8180f4eb a29e9775, git diff origin/live...a29e9775, my own kubectl kustomize renders of the head tree and of origin/live (5b0d7f43), the harness's own helper functions extracted verbatim from the head blob and driven against my render, and my own harness runs of both trees.

The delta is the guard and nothing else

git diff --stat 8180f4eb a29e9775 = scripts/gits-sbx-epic-test.sh | 292 ++++… , 1 file, +256/−36, exactly as declared. The head still differs from origin/live in three files (statefulset.yaml, README.md, the harness), and the render diff of core/manifests/apps/sbx-gits, live vs head, is still exactly two hunks, pure addition, zero removals: - name: TMPDIR / value: /home/ld/work/seat-transcripts in the sandbox env, and the seed's mkdir -p … || printf 'SEED-WARN: …'. Nothing else in the render moved.

1. The spine now carries every container (and the loss is provably not silent)

• Head spine 144 lines, origin/live spine 144 lines, diff empty.
• All four container names present: sandbox, emulator, prepare-work, seed-pi-agent.
• All four image: lines present: sbx-fleet:6fa150a@sha256:0ee5e97e… ×3 and android-emulator:230756a@sha256:66430940….
• The fields seat 2 said were unguarded are now in the spine: prepare-work's root securityContext (runAsUser: 0, capabilities.add: CHOWN/FOWNER/DAC_OVERRIDE, readOnlyRootFilesystem: true) and the sandbox's image, readOnlyRootFilesystem, capabilities.drop, requests/limits.
• The round-1 loss reproduced on the old code, on the same head render: running the p1_spine extracted from 8180f4eb over my head render yields 90 lines with only 2 of 4 container names (emulator, seed-pi-agent) and 2 of 4 images — byte-for-byte the failure seat 2 measured — while the delta's new independent census (p1_containers + p1_census_missing, which do not exist at 8180f4eb: zero occurrences) flags 6 missing lines against that old spine. So the new assertion is not vacuous: it detects the exact defect it was written for. At this head p1_census_missing(now) and p1_census_missing(live) are both empty, and p1_containers counts 4 images = 4 spine images, with census sections containers/sandbox, containers/emulator, initContainers/prepare-work, initContainers/seed-pi-agent.

2. The mutation table bites — executed against my own render

Baseline first (the ceiling): p1_lab_sts_bad(live, head, '', 'TMPDIR') = empty (green), and with the allowance removed = env-anadidos: TMPDIR. Rows, each applied inside the named container by p1_mutate (never a hand-patched doc) and then re-judged:

mutation	judged
sandbox memory 13Gi → 20Gi	spine-distinto
sandbox image digest mutated	spine-distinto contenedores-distintos
sandbox readOnlyRootFilesystem: true → false	spine-distinto contenedores-distintos
sandbox capabilities.drop: - ALL → - NET_ADMIN	spine-distinto contenedores-distintos
prepare-work capabilities.add minus CHOWN	spine-distinto contenedores-distintos
prepare-work runAsUser: 0 → 1000	spine-distinto contenedores-distintos
emulator memory 8Gi → 9Gi	spine-distinto
env name added in sandbox / emulator / prepare-work	env-anadidos: MUT_ENV

Extra rows I added because they are the fields the round-1 strip swallowed: sandbox requests memory 12Gi → 13Gi, sandbox imagePullPolicy, sandbox terminationMessagePolicy, prepare-work's command interpreter, a pod-level nodeSelector change, and deleting the emulator item outright — all red (spine-distinto, the last one also contenedores-distintos mounts-quitados:/avd env-quitados:…). The env-name addition leaves the spine unchanged by design and is caught by the name set; the p1_list env set now covers every container: head 35 names vs live 34, delta exactly TMPDIR, which is present in the head set.

The harness's own table (§10 H8[8][sbx-gits][censo] + 11 [mut] rows) is PASS on this head, and the allowance is scoped: d_extra_env is TMPDIR for sbx-gits only, '' for sbx-lab, and the other p1_lab_sts_bad call sites (10b[R99], 11[H11-render], 17[R159]) are sbx-lab/sbx-sendrast only, so the exception cannot leak.

3. Seat 2's original proof now flags

The sandbox memory: 13Gi → 20Gi mutation that left the round-1 spine byte-identical now changes the spine and yields p1_lab_sts_bad = spine-distinto. That is the precise demonstration the round-1 comment asked for, at this head, on the real render.

4. Harness solo — zero new reds, zero fixed

I ran each tree solo (see the note below on why not simultaneously):

• head a29e9775: RESULT: FAIL (5 of 693 checks failed, 6 skipped, 0 pending)
• origin/live 5b0d7f43: RESULT: FAIL (5 of 670 checks failed, 8 skipped, 0 pending)
• The sorted FAIL sets are byte-identical (diff clean) and are the pre-existing ones: manifest[2] (memory 6Gi/10Gi), manifest[3] and manifest[3b] (image pins), the T7 resource totals row, and T7+R99[10b] (the generator's per-file legs). The head adds 12 checks (§28's 8, the census/mutation rows, the T7[diff] re-measure) and adds no red — the guard rewrite did not turn the tree red anywhere.
• T7[diff] and 17[R159] (d) are green with this branch's re-measured set (M README.md, M statefulset.yaml), and all of §24 (27 rows) and §28 (8 rows) pass, including the executed os.tmpdir().

Resolutions

• resolved B1: the round-1 finding that the spine silently dropped whole container items is fixed and mutation-proof at a29e9775 — four container names and four image: lines retained, prepare-work's root securityContext and capabilities.add retained, p1_census_missing empty on both sides, and the sandbox memory mutation that used to slip through now goes red (spine-distinto).

Blockers

None.

Concerns

• Env values are still outside the guard (disclosed in the PR body, and I confirm it). For TMPDIR the exposure is closed elsewhere — §28 pins the value structurally through PyYAML — so the disclosure is compensated here; for any other already-present variable the value was unguarded before this delta too, so the delta neither widens nor narrows that.
• The harness's scratch state is shared, so it is not safe to run two copies at once. I hit this myself: a shared /tmp makes H1b's /tmp/h1-objects-<d>-{before,after}.txt, the 20/22[R172*] lock smokes and the "EMPTY pre-existing --dir" probe interfere, producing FAIL rows that belong to neither tree (on the head, a lock-release row; on live, an H1b[sbx-lab] object-set row whose renders I verified are identical). Re-running each tree in its own mount namespace with a private /tmp made both sets identical. The GITS loop runs it solo, so this is a robustness note for anyone running it twice in parallel — worth its own ticket, not this PR.
• Seat 2's C-1 (disk growth against the 30 GiB cap, no rotation, no volume alert) is unaffected by this delta: it is a property of the TMPDIR change itself, unchanged between 8180f4eb and a29e9775, and I did not re-measure it here.

**Verdict of record — ld-reviewer — head a29e9775c1eece50e8cd5c5c817f7f9ee174d9d2: APPROVE** model: deepseek-flash session: sess-2682-r2-s1-9c41d7 Independent review seat 1 of 2, round 2 (delta). Primary sources only: `git diff 8180f4eb a29e9775`, `git diff origin/live...a29e9775`, my own `kubectl kustomize` renders of the head tree and of `origin/live` (`5b0d7f43`), the harness's own helper functions extracted verbatim from the head blob and driven against **my** render, and my own harness runs of both trees. ## The delta is the guard and nothing else `git diff --stat 8180f4eb a29e9775` = `scripts/gits-sbx-epic-test.sh | 292 ++++… `, **1 file, +256/−36**, exactly as declared. The head still differs from `origin/live` in three files (`statefulset.yaml`, `README.md`, the harness), and the **render diff of `core/manifests/apps/sbx-gits`, live vs head, is still exactly two hunks, pure addition, zero removals**: `- name: TMPDIR` / `value: /home/ld/work/seat-transcripts` in the `sandbox` env, and the seed's `mkdir -p … || printf 'SEED-WARN: …'`. Nothing else in the render moved. ## 1. The spine now carries every container (and the loss is provably not silent) - Head spine **144 lines**, `origin/live` spine **144 lines**, `diff` **empty**. - All four container names present: `sandbox`, `emulator`, `prepare-work`, `seed-pi-agent`. - All four `image:` lines present: `sbx-fleet:6fa150a@sha256:0ee5e97e…` ×3 and `android-emulator:230756a@sha256:66430940…`. - The fields seat 2 said were unguarded are now in the spine: `prepare-work`'s root `securityContext` (`runAsUser: 0`, `capabilities.add: CHOWN/FOWNER/DAC_OVERRIDE`, `readOnlyRootFilesystem: true`) and the `sandbox`'s `image`, `readOnlyRootFilesystem`, `capabilities.drop`, `requests`/`limits`. - **The round-1 loss reproduced on the old code, on the same head render:** running the `p1_spine` extracted from `8180f4eb` over my head render yields **90 lines with only 2 of 4 container names** (`emulator`, `seed-pi-agent`) and **2 of 4 images** — byte-for-byte the failure seat 2 measured — while the delta's **new independent census** (`p1_containers` + `p1_census_missing`, which do not exist at `8180f4eb`: zero occurrences) flags **6 missing lines** against that old spine. So the new assertion is not vacuous: it detects the exact defect it was written for. At this head `p1_census_missing(now)` and `p1_census_missing(live)` are both **empty**, and `p1_containers` counts 4 images = 4 spine images, with census sections `containers/sandbox`, `containers/emulator`, `initContainers/prepare-work`, `initContainers/seed-pi-agent`. ## 2. The mutation table bites — executed against my own render Baseline first (the ceiling): `p1_lab_sts_bad(live, head, '', 'TMPDIR')` = **empty (green)**, and with the allowance removed = `env-anadidos: TMPDIR`. Rows, each applied inside the named container by `p1_mutate` (never a hand-patched doc) and then re-judged: | mutation | judged | |---|---| | `sandbox` memory 13Gi → 20Gi | `spine-distinto` | | `sandbox` image digest mutated | `spine-distinto contenedores-distintos` | | `sandbox` `readOnlyRootFilesystem: true → false` | `spine-distinto contenedores-distintos` | | `sandbox` `capabilities.drop: - ALL → - NET_ADMIN` | `spine-distinto contenedores-distintos` | | `prepare-work` `capabilities.add` minus `CHOWN` | `spine-distinto contenedores-distintos` | | `prepare-work` `runAsUser: 0 → 1000` | `spine-distinto contenedores-distintos` | | `emulator` memory 8Gi → 9Gi | `spine-distinto` | | env name added in `sandbox` / `emulator` / `prepare-work` | `env-anadidos: MUT_ENV` | Extra rows I added because they are the fields the round-1 strip swallowed: `sandbox` `requests` memory 12Gi → 13Gi, `sandbox` `imagePullPolicy`, `sandbox` `terminationMessagePolicy`, `prepare-work`'s command interpreter, a pod-level `nodeSelector` change, and deleting the `emulator` item outright — **all red** (`spine-distinto`, the last one also `contenedores-distintos mounts-quitados:/avd env-quitados:…`). The env-name addition leaves the spine unchanged by design and is caught by the name set; the `p1_list env` set now covers every container: head 35 names vs live 34, delta exactly `TMPDIR`, which is present in the head set. The harness's own table (§10 `H8[8][sbx-gits][censo]` + 11 `[mut]` rows) is **PASS on this head**, and the allowance is scoped: `d_extra_env` is `TMPDIR` for `sbx-gits` only, `''` for `sbx-lab`, and the other `p1_lab_sts_bad` call sites (10b[R99], 11[H11-render], 17[R159]) are `sbx-lab`/`sbx-sendrast` only, so the exception cannot leak. ## 3. Seat 2's original proof now flags The `sandbox` `memory: 13Gi → 20Gi` mutation that left the round-1 spine byte-identical now **changes the spine** and yields `p1_lab_sts_bad` = `spine-distinto`. That is the precise demonstration the round-1 comment asked for, at this head, on the real render. ## 4. Harness solo — zero new reds, zero fixed I ran each tree **solo** (see the note below on why not simultaneously): - head `a29e9775`: `RESULT: FAIL (5 of 693 checks failed, 6 skipped, 0 pending)` - `origin/live` `5b0d7f43`: `RESULT: FAIL (5 of 670 checks failed, 8 skipped, 0 pending)` - The sorted FAIL sets are **byte-identical** (`diff` clean) and are the pre-existing ones: `manifest[2]` (memory 6Gi/10Gi), `manifest[3]` and `manifest[3b]` (image pins), the `T7` resource totals row, and `T7+R99[10b]` (the generator's per-file legs). The head adds 12 checks (§28's 8, the census/mutation rows, the `T7[diff]` re-measure) and adds **no** red — the guard rewrite did not turn the tree red anywhere. - `T7[diff]` and `17[R159] (d)` are green with this branch's re-measured set (`M README.md`, `M statefulset.yaml`), and all of §24 (27 rows) and §28 (8 rows) pass, including the executed `os.tmpdir()`. ## Resolutions - resolved B1: the round-1 finding that the spine silently dropped whole container items is fixed and mutation-proof at `a29e9775` — four container names and four `image:` lines retained, `prepare-work`'s root `securityContext` and `capabilities.add` retained, `p1_census_missing` empty on both sides, and the `sandbox` memory mutation that used to slip through now goes red (`spine-distinto`). ## Blockers None. ## Concerns - **Env *values* are still outside the guard** (disclosed in the PR body, and I confirm it). For `TMPDIR` the exposure is closed elsewhere — §28 pins the value structurally through PyYAML — so the disclosure is compensated *here*; for any other already-present variable the value was unguarded before this delta too, so the delta neither widens nor narrows that. - **The harness's scratch state is shared, so it is not safe to run two copies at once.** I hit this myself: a shared `/tmp` makes `H1b`'s `/tmp/h1-objects-<d>-{before,after}.txt`, the `20/22[R172*]` lock smokes and the "EMPTY pre-existing --dir" probe interfere, producing FAIL rows that belong to neither tree (on the head, a lock-release row; on live, an `H1b[sbx-lab]` object-set row whose renders I verified are identical). Re-running each tree in its own mount namespace with a private `/tmp` made both sets identical. The GITS loop runs it solo, so this is a robustness note for anyone running it twice in parallel — worth its own ticket, not this PR. - Seat 2's `C-1` (disk growth against the 30 GiB cap, no rotation, no volume alert) is unaffected by this delta: it is a property of the `TMPDIR` change itself, unchanged between `8180f4eb` and `a29e9775`, and I did not re-measure it here.

pi-landing merged commit b9b5c2beff into live 2026-11-06 09:52:08 +00:00

pi-landing referenced this pull request from a commit 2026-11-06 09:52:09 +00:00

Merge pull request 'HOMELAB-2682: fix(sbx-gits): seat transcripts move to the persistent volume (TMPDIR) — PR B of the evidence fixes' (#854) from plane/HOMELAB-2682-transcripts into live

Sign in to join this conversation.

Reviewers

No reviewers

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!854

No description provided.