Skip to content

Commit 5a8b3e6

Browse files
sgwannabeclaude
andcommitted
fix(preview-server): apply PR #104 review feedback (codex + coderabbit)
- Detect scaffold under <run_dir>/generated/ in addition to <run_dir>/ so freshly-frozen runs (be-lead writes apps to runs/<id>/generated/) auto-launch instead of mis-aborting with exit 2 (codex P1). - Replace per-line `docker compose ps --format json` parser with read-all-stdin logic that correctly handles both NDJSON and single JSON-array formats (codex P2 + coderabbit minor). - Add cleanup_spawned trap so wait_tcp 60s timeout no longer leaks api/web pids; PID_FILE is removed and SIGTERM sent to both children before exit (coderabbit major). - Remove unused spawn() helper to eliminate dead code (coderabbit nitpick). - chief-engineer-pm.md: explicitly whitelist start-preview-server.sh in allowed_scope as a v1.7.0+ Phase 2 sanctioned exception, resolving the Rule 6 conflict between the H2 auto-launch block and the read-only Bash policy (coderabbit major). - verify-plugin.sh: sync header checklist comment 14 -> 15 to match the command-count assertion at line 80 (coderabbit minor, outside-diff). Refs PR #104 review comments Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 98d940e commit 5a8b3e6

3 files changed

Lines changed: 74 additions & 47 deletions

File tree

plugins/preview-forge/agents/meta/chief-engineer-pm.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ Auto-retro-trigger 훅이 Blackboard에 `retro.requested` 행을 기록하면:
171171
- `runs/<id>/design-approved.json` (Gate H1 수집 결과)
172172
- `/memories/m3-decisions/*.md` (자신의 reflection)
173173
- Task: 모든 department lead 호출 가능
174-
- Bash: **H1/H2 gate 지원용 read-only scripts만** 허용 (v1.6.0+). 구체적으로:
175-
- `scripts/generate-gallery.sh <run-dir>` (H1 gallery HTML 생성)
176-
- `scripts/open-browser.sh <path-or-url>` (H1 gallery auto-open, 비블로킹)
174+
- Bash: **H1/H2 gate 지원용 scripts만** 허용 (v1.6.0+). 구체적으로:
175+
- `scripts/generate-gallery.sh <run-dir>` (H1 gallery HTML 생성, read-only)
176+
- `scripts/open-browser.sh <path-or-url>` (H1 gallery auto-open, 비블로킹, read-only)
177+
- `scripts/start-preview-server.sh <run-dir>``start|stop|status` 형태 (v1.7.0+ Phase 2 sanctioned exception: stateful이지만 idempotent + run_dir-local 작용으로 한정 — H2 finalize 직후 single-shot으로만 호출. PID/URL/log 파일은 모두 `<run_dir>/.preview-*`에만 기록되며, factory-policy/Rule 6 enforcement에서 명시적으로 화이트리스트 처리됨.)
177178
- 그 외 destructive·stateful Bash는 차단 (Rule 6). 상태 변화는 Write 또는 sub-agent 위임.
178179

179180
## forbidden

scripts/start-preview-server.sh

Lines changed: 69 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,14 @@
88
# DEMO-STORYBOARD.md L1:50–2:00 expectation that a new tab pops up at
99
# http://localhost:18080 automatically.
1010
#
11-
# Profiles auto-detected from <run_dir> contents:
12-
# 1. pro / max — `<run_dir>/docker-compose.yml` exists.
11+
# Profiles auto-detected from <run_dir> contents (also probes <run_dir>/generated/
12+
# since be-lead.md and the QA leads write apps to runs/<id>/generated/, while
13+
# /pf:freeze and /pf:preview pass `runs/<id>/`):
14+
# 1. pro / max — docker-compose.yml at run_dir/ or run_dir/generated/.
1315
# → `docker compose up -d`, wait for any service Up,
1416
# extract first published port, open browser.
15-
# 2. standard — `<run_dir>/apps/api/package.json` AND
16-
# `<run_dir>/apps/web/package.json` exist.
17+
# 2. standard — apps/api/package.json AND apps/web/package.json at
18+
# run_dir/ or run_dir/generated/.
1719
# → install (pnpm > npm), pick free port from 18080+,
1820
# spawn api + web `pnpm dev` in background, persist
1921
# PIDs, wait for web TCP, open browser.
@@ -108,7 +110,7 @@ pids_alive() {
108110

109111
docker_project_up() {
110112
[ -f "$ID_FILE" ] || return 1
111-
local compose_file="$run_dir/docker-compose.yml"
113+
local compose_file="${scaffold_root:-$run_dir}/docker-compose.yml"
112114
[ -f "$compose_file" ] || return 1
113115
command -v docker >/dev/null 2>&1 || return 1
114116
# Any service in `running` state?
@@ -157,12 +159,19 @@ open_url() {
157159
}
158160

159161
# ---- profile detection ----
162+
# Engineering teams (be-lead.md) write apps to `runs/<id>/generated/`, but
163+
# `/pf:freeze` and `/pf:preview` instruct callers to pass `runs/<id>/`. So if
164+
# the scaffold isn't directly under run_dir, transparently fall through to
165+
# `<run_dir>/generated/` before declaring "scaffold missing".
166+
scaffold_root=""
160167
profile=""
161-
if [ -f "$run_dir/docker-compose.yml" ]; then
162-
profile="docker"
163-
elif [ -f "$run_dir/apps/api/package.json" ] && [ -f "$run_dir/apps/web/package.json" ]; then
164-
profile="standard"
165-
fi
168+
for cand in "$run_dir" "$run_dir/generated"; do
169+
if [ -f "$cand/docker-compose.yml" ]; then
170+
scaffold_root="$cand"; profile="docker"; break
171+
elif [ -f "$cand/apps/api/package.json" ] && [ -f "$cand/apps/web/package.json" ]; then
172+
scaffold_root="$cand"; profile="standard"; break
173+
fi
174+
done
166175

167176
# ---- action: status ----
168177
if [ "$action" = "status" ]; then
@@ -207,9 +216,15 @@ if [ "$action" = "stop" ]; then
207216
fi
208217
rm -f "$PID_FILE"
209218
fi
210-
# Docker-based stop.
211-
if [ -f "$ID_FILE" ] && [ -f "$run_dir/docker-compose.yml" ] && command -v docker >/dev/null 2>&1; then
212-
docker compose -f "$run_dir/docker-compose.yml" down >/dev/null 2>&1 || true
219+
# Docker-based stop. Scaffold may live under either run_dir or run_dir/generated.
220+
stop_compose=""
221+
for cand in "$run_dir" "$run_dir/generated"; do
222+
if [ -f "$cand/docker-compose.yml" ]; then
223+
stop_compose="$cand/docker-compose.yml"; break
224+
fi
225+
done
226+
if [ -f "$ID_FILE" ] && [ -n "$stop_compose" ] && command -v docker >/dev/null 2>&1; then
227+
docker compose -f "$stop_compose" down >/dev/null 2>&1 || true
213228
fi
214229
rm -f "$ID_FILE" "$URL_FILE"
215230
echo "preview server stopped (run_dir=$run_dir)"
@@ -220,7 +235,7 @@ fi
220235

221236
# No profile detected → caller has not run TestDD freeze yet.
222237
if [ -z "$profile" ]; then
223-
echo "neither apps/{api,web}/package.json nor docker-compose.yml found in $run_dir; cannot start preview server" >&2
238+
echo "neither apps/{api,web}/package.json nor docker-compose.yml found in $run_dir or $run_dir/generated; cannot start preview server" >&2
224239
exit 2
225240
fi
226241

@@ -252,7 +267,7 @@ fi
252267

253268
# ---- profile: docker (pro / max) ----
254269
if [ "$profile" = "docker" ]; then
255-
compose_file="$run_dir/docker-compose.yml"
270+
compose_file="$scaffold_root/docker-compose.yml"
256271
if [ "${PF_PREVIEW_DRY_RUN:-0}" = "1" ]; then
257272
echo "[dry-run] profile=docker compose_file=$compose_file"
258273
echo "[dry-run] would: docker compose -f $compose_file up -d --quiet-pull"
@@ -284,23 +299,24 @@ if [ "$profile" = "docker" ]; then
284299
port="$(docker compose -f "$compose_file" ps --format json 2>/dev/null \
285300
| python3 -c '
286301
import json, sys
287-
for line in sys.stdin:
288-
line = line.strip()
289-
if not line:
290-
continue
302+
raw = sys.stdin.read().strip()
303+
records = []
304+
if raw:
305+
# Older docker emits a single JSON array; newer ones emit NDJSON.
291306
try:
292-
rec = json.loads(line)
307+
parsed = json.loads(raw)
308+
records = parsed if isinstance(parsed, list) else [parsed]
293309
except json.JSONDecodeError:
294-
# Some docker versions emit a single JSON array instead of NDJSON.
295-
try:
296-
arr = json.loads(line)
297-
except Exception:
298-
continue
299-
for rec in arr if isinstance(arr, list) else []:
300-
for p in rec.get("Publishers") or []:
301-
pub = p.get("PublishedPort")
302-
if pub:
303-
print(pub); sys.exit(0)
310+
for line in raw.splitlines():
311+
line = line.strip()
312+
if not line:
313+
continue
314+
try:
315+
records.append(json.loads(line))
316+
except json.JSONDecodeError:
317+
continue
318+
for rec in records:
319+
if not isinstance(rec, dict):
304320
continue
305321
for p in rec.get("Publishers") or []:
306322
pub = p.get("PublishedPort")
@@ -321,8 +337,8 @@ for line in sys.stdin:
321337
fi
322338

323339
# ---- profile: standard (apps/api + apps/web) ----
324-
api_dir="$run_dir/apps/api"
325-
web_dir="$run_dir/apps/web"
340+
api_dir="$scaffold_root/apps/api"
341+
web_dir="$scaffold_root/apps/web"
326342

327343
# Pick free ports: web on 18080+, api on 18180+ (offset of 100 keeps logs scannable).
328344
web_port="$(pick_free_port 18080 11)" || {
@@ -346,9 +362,9 @@ fi
346362

347363
# Install deps. Prefer pnpm if pnpm-lock.yaml exists; else npm.
348364
pkg_mgr=""
349-
if command -v pnpm >/dev/null 2>&1 && [ -f "$run_dir/pnpm-lock.yaml" ]; then
365+
if command -v pnpm >/dev/null 2>&1 && [ -f "$scaffold_root/pnpm-lock.yaml" ]; then
350366
pkg_mgr="pnpm"
351-
elif command -v pnpm >/dev/null 2>&1 && [ -f "$run_dir/pnpm-workspace.yaml" ]; then
367+
elif command -v pnpm >/dev/null 2>&1 && [ -f "$scaffold_root/pnpm-workspace.yaml" ]; then
352368
pkg_mgr="pnpm"
353369
elif command -v npm >/dev/null 2>&1; then
354370
pkg_mgr="npm"
@@ -358,28 +374,37 @@ else
358374
fi
359375
case "$pkg_mgr" in
360376
pnpm )
361-
(cd "$run_dir" && pnpm install --frozen-lockfile >/dev/null 2>&1) || \
362-
(cd "$run_dir" && pnpm install >/dev/null 2>&1) || {
363-
echo "start-preview-server.sh: pnpm install failed in $run_dir" >&2
377+
(cd "$scaffold_root" && pnpm install --frozen-lockfile >/dev/null 2>&1) || \
378+
(cd "$scaffold_root" && pnpm install >/dev/null 2>&1) || {
379+
echo "start-preview-server.sh: pnpm install failed in $scaffold_root" >&2
364380
exit 1
365381
}
366382
dev_cmd="pnpm dev"
367383
;;
368384
npm )
369-
(cd "$run_dir" && npm install >/dev/null 2>&1) || true
385+
(cd "$scaffold_root" && npm install >/dev/null 2>&1) || true
370386
(cd "$api_dir" && npm install >/dev/null 2>&1) || true
371387
(cd "$web_dir" && npm install >/dev/null 2>&1) || true
372388
dev_cmd="npm run dev"
373389
;;
374390
esac
375391

376-
# Spawn api + web in background, redirecting output. setsid (if available)
377-
# detaches them from the controlling tty so they survive shell exit.
378-
spawn() {
379-
local dir="$1" port="$2" log="$3" extra_env="$4"
380-
( cd "$dir" && eval "$extra_env PORT=$port nohup $dev_cmd >'$log' 2>&1 &" echo $! )
392+
# Cleanup spawned api/web on early exit so a wait_tcp timeout does not leak
393+
# zombie processes that would still hold the port and force the next retry to
394+
# pick a higher port (causing PID_FILE drift).
395+
cleanup_spawned() {
396+
local pid
397+
for pid in "${api_pid:-}" "${web_pid:-}"; do
398+
case "$pid" in
399+
''|*[!0-9]*) continue ;;
400+
esac
401+
kill -TERM "$pid" 2>/dev/null || true
402+
done
403+
rm -f "$PID_FILE"
381404
}
382405

406+
# Spawn api + web in background, redirecting output. nohup detaches them
407+
# from the controlling tty so they survive shell exit.
383408
api_pid="$( ( cd "$api_dir" && PORT="$api_port" nohup $dev_cmd >"$API_LOG" 2>&1 & echo $! ) )"
384409
web_pid="$( ( cd "$web_dir" && PORT="$web_port" NEXT_PUBLIC_API_URL="http://localhost:$api_port" nohup $dev_cmd >"$WEB_LOG" 2>&1 & echo $! ) )"
385410

@@ -394,6 +419,7 @@ if ! wait_tcp 127.0.0.1 "$web_port" 60; then
394419
echo "start-preview-server.sh: web server did not start on :$web_port within 60s" >&2
395420
echo " api log: $API_LOG"
396421
echo " web log: $WEB_LOG"
422+
cleanup_spawned
397423
exit 1
398424
fi
399425

scripts/verify-plugin.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
# Checks:
66
# 1. Manifest JSON syntax (marketplace.json + plugin.json)
77
# 2. All 144 agents present with valid frontmatter
8-
# 3. 14 slash commands present
8+
# 3. 15 slash commands present
99
# 4. 3 hooks + hooks.json valid
1010
# 5. Memory seed + methodology + assets + schemas + seeds present
1111

0 commit comments

Comments
 (0)