QA orchestrator: trap/cleanup for background Vite in start-qa-local-full-stack.sh (151-2) #153

Closed
opened 2026-05-05 05:45:53 +00:00 by PlasticDigits · 8 comments
PlasticDigits commented 2026-05-05 05:45:53 +00:00 (Migrated from gitlab.com)

Problem

scripts/start-qa-local-full-stack.sh function start_frontend_dev backgrounds npm run dev (Vite), writes PID to /tmp/yieldomega_frontend_qa.pid, but installs no trap on EXIT / INT / TERM to tear down that child when the orchestrator exits or is Ctrl-C’d.

Impact: If the script dies after Vite starts (or the user interrupts mid–readiness loop), the Vite process can orphan and keep binding FRONTEND_DEV_PORT (default 5173). The next run may either fail to bind (loud) or, worse, pass the curl readiness probe against a stale server from a previous run — the reviewer in #151 observed the same class of bug with Anvil on 8545; the shape matches Vite here.

Split from GitLab #151 (Finding 2 — Low). Parent orchestrator story: GitLab #104.

Area Location
Orchestrator scripts/start-qa-local-full-stack.sh — start_frontend_dev (~lines 92–131): background subshell + PID file, no trap
Stack (delegated) scripts/start-local-anvil-stack.sh
QA procedure docs/testing/qa-local-full-stack.md — GitLab #104
Invariant docs/testing/invariants-and-business-logic.md — QA full stack / #104
Guardrails skill .cursor/skills/yieldomega-guardrails/SKILL.md (item 36)
Manual QA docs/testing/manual-qa-checklists.md — #104 if present

Contracts / indexer: Not applicable.

Acceptance criteria

  1. Cleanup on exit: When start_frontend_dev launches Vite, a trap (or equivalent) runs on EXIT, INT, and TERM that kills the recorded Vite PID (from the pid file or the background job’s $!) and avoids double-kill / noisy errors (|| true where appropriate).
  2. Ctrl+C during readiness: Interrupting the script while the 60×0.5s loop is running still tears down Vite or documents a single documented manual step if bash limitations require it (prefer automatic cleanup).
  3. Optional hardening (from #151): Readiness loop is fail-closed where feasible — e.g. detect “curl succeeded before we actually started our child” is hard; at minimum document that starting a second run kills prior PID (already partially at L102–109) and trap handles post-start aborts. Acceptable alternative: trap-only if behavior is clearly verified.
  4. --no-frontend: No regression — no Vite process, no spurious trap firing.

Verification checklist

  • Happy path: bash scripts/start-qa-local-full-stack.sh (default, with frontend) → browser loads http://127.0.0.1:5173 → quit script via documented “stopping” flow in qa-local-full-stack.md → no stray node/vite listening on 5173 (ss -ltnp | grep 5173 or equivalent).
  • Ctrl+C after Vite PID logged: Start script, wait until log shows Vite PID, Ctrl+C before or during readiness loop → confirm port 5173 freed or prior PID dead.
  • Second run: Run orchestrator twice in a row; second run should not serve stale bundle from an un-killed first Vite (spot-check: change a visible string in app, restart stack, confirm update — or kill -0 on expected PID).
  • --no-frontend: bash scripts/start-qa-local-full-stack.sh --no-frontend → no /tmp/yieldomega_frontend_qa.pid hot-leak from this change (n/a if Vite not started).
  • Update docs/testing/qa-local-full-stack.md Stopping the stack if trap changes how operators should stop Vite.
  • Re-read #105 --help behavior: no regression to usage() / banner-only help.
## Problem [`scripts/start-qa-local-full-stack.sh`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/scripts/start-qa-local-full-stack.sh) function `start_frontend_dev` backgrounds **`npm run dev`** (Vite), writes PID to `/tmp/yieldomega_frontend_qa.pid`, but **installs no `trap`** on `EXIT` / `INT` / `TERM` to tear down that child when the orchestrator exits or is Ctrl-C’d. **Impact:** If the script dies after Vite starts (or the user interrupts mid–readiness loop), the Vite process can **orphan** and keep binding **`FRONTEND_DEV_PORT`** (default **5173**). The next run may either fail to bind (loud) or, worse, **pass the `curl` readiness probe against a stale server** from a previous run — the reviewer in #151 observed the same class of bug with Anvil on **8545**; the shape matches Vite here. Split from [GitLab #151](https://gitlab.com/PlasticDigits/yieldomega/-/issues/151) (Finding 2 — Low). Parent orchestrator story: [GitLab #104](https://gitlab.com/PlasticDigits/yieldomega/-/issues/104). ## Related code & docs | Area | Location | |------|----------| | **Orchestrator** | [`scripts/start-qa-local-full-stack.sh`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/scripts/start-qa-local-full-stack.sh) — `start_frontend_dev` (~lines 92–131): background subshell + PID file, no trap | | **Stack (delegated)** | [`scripts/start-local-anvil-stack.sh`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/scripts/start-local-anvil-stack.sh) | | **QA procedure** | [`docs/testing/qa-local-full-stack.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/qa-local-full-stack.md) — [GitLab #104](https://gitlab.com/PlasticDigits/yieldomega/-/issues/104) | | **Invariant** | [`docs/testing/invariants-and-business-logic.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/invariants-and-business-logic.md) — QA full stack / #104 | | **Guardrails skill** | [`.cursor/skills/yieldomega-guardrails/SKILL.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/.cursor/skills/yieldomega-guardrails/SKILL.md) (item 36) | | **Manual QA** | [`docs/testing/manual-qa-checklists.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/manual-qa-checklists.md) — #104 if present | **Contracts / indexer:** Not applicable. ## Acceptance criteria 1. **Cleanup on exit:** When `start_frontend_dev` launches Vite, a **`trap`** (or equivalent) runs on **`EXIT`**, **`INT`**, and **`TERM`** that kills the **recorded** Vite PID (from the pid file or the background job’s `$!`) and avoids double-kill / noisy errors (`|| true` where appropriate). 2. **Ctrl+C during readiness:** Interrupting the script while the 60×0.5s loop is running still tears down Vite **or** documents a single documented manual step if bash limitations require it (prefer automatic cleanup). 3. **Optional hardening (from #151):** Readiness loop is **fail-closed** where feasible — e.g. detect “curl succeeded before we actually started our child” is hard; at minimum document that starting a second run kills prior PID (already partially at L102–109) **and** trap handles post-start aborts. Acceptable alternative: **trap-only** if behavior is clearly verified. 4. **`--no-frontend`:** No regression — no Vite process, no spurious trap firing. ## Verification checklist - [ ] **Happy path:** `bash scripts/start-qa-local-full-stack.sh` (default, with frontend) → browser loads `http://127.0.0.1:5173` → quit script via documented “stopping” flow in [`qa-local-full-stack.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/qa-local-full-stack.md) → **no** stray `node`/`vite` listening on 5173 (`ss -ltnp | grep 5173` or equivalent). - [ ] **Ctrl+C after Vite PID logged:** Start script, wait until log shows Vite PID, **Ctrl+C** before or during readiness loop → confirm port **5173** freed or prior PID dead. - [ ] **Second run:** Run orchestrator twice in a row; second run should not serve **stale** bundle from an un-killed first Vite (spot-check: change a visible string in app, restart stack, confirm update — or kill -0 on expected PID). - [ ] **`--no-frontend`:** `bash scripts/start-qa-local-full-stack.sh --no-frontend` → no `/tmp/yieldomega_frontend_qa.pid` hot-leak from this change (n/a if Vite not started). - [ ] Update [`docs/testing/qa-local-full-stack.md`](https://gitlab.com/PlasticDigits/yieldomega/-/blob/main/docs/testing/qa-local-full-stack.md) **Stopping the stack** if trap changes how operators should stop Vite. - [ ] Re-read **#105** `--help` behavior: no regression to `usage()` / banner-only help.
PlasticDigits commented 2026-05-05 05:45:54 +00:00 (Migrated from gitlab.com)

marked this issue as related to #151

marked this issue as related to #151
PlasticDigits commented 2026-05-05 05:46:12 +00:00 (Migrated from gitlab.com)

mentioned in issue #151

mentioned in issue #151
PlasticDigits commented 2026-05-05 05:48:02 +00:00 (Migrated from gitlab.com)

marked this issue as related to #104

marked this issue as related to #104
PlasticDigits commented 2026-05-05 05:48:18 +00:00 (Migrated from gitlab.com)

marked this issue as related to #105

marked this issue as related to #105
PlasticDigits commented 2026-05-05 06:58:37 +00:00 (Migrated from gitlab.com)

mentioned in commit 3b3bd0c6c5

mentioned in commit 3b3bd0c6c501f9cf908667d304146b7b63302b87
PlasticDigits commented 2026-05-05 07:00:33 +00:00 (Migrated from gitlab.com)

Implemented GitLab #153 on main (commit 3b3bd0c).

Summary

  • scripts/lib/qa_local_full_stack_frontend.sh: start_frontend_dev arms INT/TERM + EXIT traps during the readiness loop only. INT/TERM disarms traps first, kills the PID from the pid file, then exit 130 so bash does not resume a blocking loop after the trap (critical for Ctrl+C). EXIT covers errexit without clobbering exit codes. exec npm inside the subshell so $! is the Node/Vite process. Readiness curl uses --connect-timeout 1 --max-time 3. If the probe never succeeds in ~30s, the script kills Vite, removes the pid file, and exit 1 (fail-closed vs leaving a wedged server).
  • scripts/start-qa-local-full-stack.sh: sources the lib module; **--help / #105unchanged** (verified: noset -euo pipefail` in usage output).
  • scripts/verify-qa-orchestrator-frontend-trap.sh: hermetic smoke (no Docker/Anvil). Optional override QA_FRONTEND_PID_FILE for tests.
  • Docs / crosslinks: docs/testing/qa-local-full-stack.md, invariants-and-business-logic.md (INV-QA-FULLSTACK-153 + §153), manual-qa-checklists.md, e2e-anvil.md, skills/README.md, skills/script-with-timecurve-local/SKILL.md, .cursor/skills/yieldomega-guardrails/SKILL.md.

Verification checklist for @brouie

  • bash scripts/start-qa-local-full-stack.sh --help — still no shell setup lines (set -euo pipefail, etc.).
  • bash scripts/verify-qa-orchestrator-frontend-trap.sh — prints OK: and exits 0.
  • Full stack (optional): start orchestrator with frontend → load http://127.0.0.1:5173 → Ctrl+C during readiness (or right after Vite PID logged) → ss -ltnp (or lsof -i :5173) shows no stray listener on the dev port.
  • --no-frontend: no regression; no Vite pid file from this path.
  • Second consecutive run: no stale bundle from an orphan prior Vite (or confirm old pid is dead per script banner).

Issue left open for your sign-off. Thanks!

Implemented **GitLab #153** on `main` (commit `3b3bd0c`). ### Summary - **`scripts/lib/qa_local_full_stack_frontend.sh`:** `start_frontend_dev` arms **INT/TERM** + **EXIT** traps during the readiness loop only. **INT/TERM** disarms traps first, kills the PID from the pid file, then **`exit 130`** so bash does not resume a blocking loop after the trap (critical for Ctrl+C). **EXIT** covers **errexit** without clobbering exit codes. **`exec npm`** inside the subshell so **`$!`** is the Node/Vite process. Readiness **`curl`** uses **`--connect-timeout 1 --max-time 3`**. If the probe never succeeds in ~30s, the script kills Vite, removes the pid file, and **`exit 1`** (fail-closed vs leaving a wedged server). - **`scripts/start-qa-local-full-stack.sh`:** sources the lib module; **`--help` / #105` unchanged** (verified: no `set -euo pipefail` in usage output). - **`scripts/verify-qa-orchestrator-frontend-trap.sh`:** hermetic smoke (no Docker/Anvil). Optional override **`QA_FRONTEND_PID_FILE`** for tests. - **Docs / crosslinks:** `docs/testing/qa-local-full-stack.md`, `invariants-and-business-logic.md` (**`INV-QA-FULLSTACK-153`** + §153), `manual-qa-checklists.md`, `e2e-anvil.md`, `skills/README.md`, `skills/script-with-timecurve-local/SKILL.md`, `.cursor/skills/yieldomega-guardrails/SKILL.md`. ### Verification checklist for @brouie - [ ] `bash scripts/start-qa-local-full-stack.sh --help` — still **no** shell setup lines (`set -euo pipefail`, etc.). - [ ] `bash scripts/verify-qa-orchestrator-frontend-trap.sh` — prints **`OK:`** and exits **0**. - [ ] Full stack (optional): start orchestrator with frontend → load `http://127.0.0.1:5173` → **Ctrl+C during readiness** (or right after Vite PID logged) → **`ss -ltnp`** (or `lsof -i :5173`) shows **no** stray listener on the dev port. - [ ] **`--no-frontend`:** no regression; no Vite pid file from this path. - [ ] Second consecutive run: no stale bundle from an orphan prior Vite (or confirm old pid is dead per script banner). Issue left **open** for your sign-off. Thanks!
Brouie commented 2026-05-06 00:00:31 +00:00 (Migrated from gitlab.com)

@PlasticDigits — verification PASS on 3b3bd0c.

ran your hermetic verify script:

bash scripts/verify-qa-orchestrator-frontend-trap.sh

result: OK: verify-qa-orchestrator-frontend-trap (signal trap exits + _qa_kill_pid_file_process).

trap fires on interrupt-before-ready, kill + exit path executes cleanly. INV-QA-FULLSTACK-153 satisfied.

ready for close on your side.

@PlasticDigits — verification PASS on `3b3bd0c`. ran your hermetic verify script: ``` bash scripts/verify-qa-orchestrator-frontend-trap.sh ``` result: `OK: verify-qa-orchestrator-frontend-trap (signal trap exits + _qa_kill_pid_file_process)`. trap fires on interrupt-before-ready, kill + exit path executes cleanly. INV-QA-FULLSTACK-153 satisfied. ready for close on your side.
PlasticDigits (Migrated from gitlab.com) closed this issue 2026-05-06 08:24:50 +00:00
PlasticDigits commented 2026-05-13 15:13:33 +00:00 (Migrated from gitlab.com)

mentioned in commit ed51a62362

mentioned in commit ed51a62362773bd86491b0bd6c69d464f35a2eb8
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
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
code/yieldomega#153
No description provided.