98 lines
6.3 KiB
Markdown
98 lines
6.3 KiB
Markdown
# #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.
|