fix(fleet): make framework shell assertions SIGPIPE-safe (#1100)

2026-08-07 08:21:20 +00:00
parent 4fa2768962
commit df4c591ab4
3 changed files with 240 additions and 37 deletions
@@ -176,6 +176,38 @@ roster-derived part of the generated launch projection and prove it reaches the
 3. `AC-FGI-03`: Focused launcher and generated-environment tests, repository quality gates,
   independent review, and the required RED/green/R7 evidence are recorded before push.

+### Framework shell assertion portability (#1098)
+
+#### Problem and objective
+
+The blocking framework-shell chain can report that a pane command omitted `/usr/bin/env -i` even when
+`-i` matched successfully. A short-circuiting `grep -q` under `set -o pipefail` may close its pipe after
+the match and cause an upstream producer to exit with SIGPIPE, turning a valid semantic result into a
+nonzero aggregate pipeline. The objective is to inspect the captured NUL-delimited argv directly and
+make failures carry the observed records needed for diagnosis.
+
+#### Normative requirements
+
+1. `FSP-REQ-01`: The pane-boundary test SHALL validate an adjacent `/usr/bin/env`, `-i` argv pair from
+   the authoritative NUL-delimited tmux capture without a short-circuit pipeline whose upstream status
+   can override a successful match.
+2. `FSP-REQ-02`: Missing, reversed, or non-adjacent boundary tokens SHALL fail, while valid boundaries
+   SHALL remain valid regardless of trailing argv size, pipe capacity, process scheduling, or host/CI
+   utility implementation.
+3. `FSP-REQ-03`: A failed boundary check SHALL print stable indexed, shell-escaped observed argv records
+   before exiting nonzero; the fixture SHALL continue to contain generated non-secret launch data only.
+4. `FSP-REQ-04`: Verification SHALL include RED-first large-payload evidence, negative token-order
+   controls, the complete focused launcher suite, canonical Woodpecker CI, and independent review.
+
+#### Acceptance criteria
+
+1. `AC-FSP-01`: A large captured argv with adjacent `/usr/bin/env`, `-i` passes even when the former
+   `grep -q` pipeline returns nonzero from an upstream SIGPIPE.
+2. `AC-FSP-02`: Missing executable, missing flag, and detached/reversed flag fixtures return nonzero and
+   emit the indexed observed argv.
+3. `AC-FSP-03`: The focused suite passes on the development host and CI image, and the merged-main
+   Woodpecker pipeline is terminal green before #1098 closes.
+
 ---

 ## Exact Cross-Harness Fleet Communications Contract (#766)
@@ -0,0 +1,97 @@
+# #1098 — Framework shell portability / red main
+
+## Objective
+
+Restore terminal-green `main` by making the `test-start-agent-session.sh` clean-environment assertion semantic and portable without removing either newly enumerated framework-shell suite.
+
+## Scope
+
+- Tracking issue: `mosaicstack/stack#1098`
+- Branch: `fix/framework-shell-portability`
+- Base: `origin/main` at `4fa2768962702d53e16e8b67ee6ad52ebcb0910e`
+- Primary file: `packages/mosaic/framework/tools/fleet/test-start-agent-session.sh`
+- Requirements source: `docs/PRD.md` § Framework shell assertion portability (#1098)
+- Out of scope: deployed files under `~/.config/mosaic`, pnpm-store cleanup, checkout deletion, and changes to the launcher’s `/usr/bin/env -i` behavior.
+
+## Acceptance criteria
+
+1. The test inspects the captured NUL-delimited tmux argv semantically and accepts an adjacent `/usr/bin/env`, `-i` pair regardless of trailing payload size or pipe scheduling.
+2. Missing `/usr/bin/env`, missing `-i`, and non-adjacent `-i` remain failures.
+3. Failure output includes the observed argv records with stable indexes and shell escaping; it exposes no credentials because this fixture supplies only generated non-secret launch data.
+4. The focused suite passes on the dev host and in the repository CI image; the blocking PR/main pipeline returns terminal green.
+5. Independent review passes; PR is squash-merged and #1098 is closed only after merged-main CI is terminal green.
+
+## Budget
+
+- ASSUMPTION: 30K-token working budget; rationale: one shell-test defect plus full PR/CI lifecycle.
+- Auto-reduction: focused shell and package gates first; rely on canonical Woodpecker for the full monorepo suite rather than duplicating a dependency install under constrained `/home`.
+- Disk baseline before clone/build: `/home` 7.1G free (99% used), `/tmp` 2.4G free (92% used).
+
+## Investigation
+
+### First-hand CI evidence
+
+- Public log: `GET https://ci.mosaicstack.dev/api/repos/47/logs/2269/53041`
+- Decoded 1,436 entries (11 null `data` entries treated as empty log rows), 190,756 bytes.
+- Failure: `FAIL: pane command did not clear its environment` immediately after the expected pane-PID warning.
+- BusyBox primitives, complete assertion pipeline, real CI image, stale/current image digests, Turbo cache masking, gateway failure, and heartbeat-sidecar concurrent writing were independently excluded.
+
+### Root cause
+
+The assertion ends in:
+
+```bash
+printf '%s\n' "$pane_args" | tail -n +"$after_pane_env" | grep -qxF -- '-i'
+```
+
+The script has `set -o pipefail`. `grep -q` exits as soon as it finds the valid `-i` record. Upstream `tail`/`printf` can then receive SIGPIPE, making the aggregate pipeline nonzero even though grep returned 0 and the semantic property is true. This depends on payload size, pipe capacity, and scheduling, explaining a local/image pass with a CI failure.
+
+Discriminating stress control with `/usr/bin/env` followed immediately by `-i`:
+
+- 8,192-byte trailing payload: `printf=0 tail=0 grep=0`, aggregate 0.
+- 16,384-byte trailing payload: `printf=0 tail=141 grep=0`, aggregate 141.
+- 32,768+ bytes: `printf=141 tail=141 grep=0`, aggregate 141.
+- A full-reading `grep -xF` control remained 0 for every payload.
+
+This is a third branch omitted by the earlier present-vs-corrupted split: the pair can be present and intact while `pipefail` reports an upstream SIGPIPE.
+
+## TDD plan
+
+1. RED: preserve the one-off stress reproducer above and add an automated large-argv semantic regression that fails under the current pipeline implementation.
+2. GREEN: parse the authoritative NUL-delimited capture into a Bash array and search for an adjacent `/usr/bin/env`, `-i` pair without a short-circuit pipeline.
+3. Add negative controls for missing, detached, and reversed tokens.
+4. On failure, print indexed `%q` argv records before returning nonzero.
+5. Run focused suite, mutation controls, shell syntax/format checks, then repository baseline gates feasible without dependency installation.
+6. Independent review, queue guard, push, PR, CI, coordinator merge authorization, squash merge, merged-main CI, issue close.
+
+## Progress
+
+- [x] Checkout created and based on `origin/main` `4fa27689`.
+- [x] CI log decoded directly.
+- [x] Root-cause stress control reproduced semantic match + aggregate pipeline failure.
+- [x] RED evidence: intact `/usr/bin/env`, `-i` fixture produced component statuses `0/141/0` and aggregate 141 under the former `grep -q` pipeline; full-reading semantic control stayed 0.
+- [x] GREEN implementation: direct NUL-argv adjacency parser, indexed diagnostics, and full-reading scalar predicates replace all load-bearing early-exit pipelines in this test.
+- [x] Baseline/situational tests:
+  - focused launcher suite: PASS on GNU host and cached Alpine CI image;
+  - paired `test-fleet-units.sh`: PASS;
+  - enumeration guard: PASS (`population=53`, `enumerated=36`, `excluded=18`), 14/14 mutation needles;
+  - `bash -n`, ShellCheck, `git diff --check`: PASS;
+  - static denominator after change: zero load-bearing `grep -q`/`head`/`-m1` pipeline candidates in `test-start-agent-session.sh`;
+  - delete-the-subject mutation removing production `-i`: RED with 78 indexed argv records, byte count, and explicit boundary failure.
+- [x] Independent review:
+  - first Codex review: request changes — negative fixtures did not each assert diagnostics;
+  - remediation: centralized predicate + diagnostic wrapper and exercised all four negative fixtures;
+  - second Codex review: APPROVE, 0 blockers/should-fix/suggestions;
+  - Codex security review: risk none, 0 findings.
+- [ ] PR CI, formal fleet review, merge, merged-main CI, issue closure.
+
+## Documentation disposition
+
+- Updated canonical `docs/PRD.md` with FSP requirements and acceptance criteria.
+- This is an internal test/reliability change with no API, user workflow, deployment, navigation, or publishing-surface change; no user/admin/API/sitemap update is required.
+- `docs/TASKS.md` remains unchanged because the project contract makes it orchestrator-only.
+
+## Risks
+
+- The CI failure did not print its captured argv, so the exact CI payload is unavailable. The stress control proves the assertion is non-portable and can emit the exact false verdict; branch CI is the canonical confirmation that replacing it resolves pipeline 2269’s failure class.
+- Printing fixture argv is safe only while this test’s projection remains non-secret. The diagnostic must stay scoped to the test capture and shell-escaped.