test(orchestrator): isolate service test pids

[moncher-dev]

Issues — Closed #762

TL;DR

Makes every reviewed host-PID-sensitive service test independent of the real process table.

Change-point diagram

mock worker/persisted PID
    │
    ├─ injected isProcessRunning → false
    │      └─ deterministic retry and cleanup reconciliation
    └─ explicit PID assertion guards the injected seam

Start here

• packages/orchestrator/src/service.test.ts injects false liveness for the duplicate-lease, verbose retry-log, continuation restart, and after-run hook test paths.
• Each affected test asserts the exact mocked or persisted PID was evaluated.

Risks & rollback

• Risk is limited to test scaffolding; production process-liveness behavior is unchanged.
• Revert commits 129e3f16 and 516f7a63 to restore the prior setup.

Changed files

• packages/orchestrator/src/service.test.ts

Evidence

• pnpm --filter @gh-symphony/orchestrator test -- service.test.ts — 193 passed, 10 consecutive runs.
• pnpm lint — passed.
• pnpm test — passed.
• pnpm typecheck — passed.
• pnpm build — passed.

Post-merge / human validation

• No manual validation required; this is a deterministic unit-test-only change.

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-08-29T10:22:59.038721Z	129e3f1	Draft marked ready

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	40.64%	43724 / 107566
🔵	Statements	40.64%	43724 / 107566
🔵	Functions	72.49%	2151 / 2967
🔵	Branches	78.25%	9928 / 12687

File Coverage

No changed files found.

Generated in workflow #1135 for commit efc5bcd by the Vitest Coverage Report Action

[hojinzs]

검토 결과: 방향은 정확, 적용 범위가 부족합니다 (라운드 1/6)

주입 방식 자체는 이슈 #762 가 권장한 그대로이고, 프로덕션 코드를 건드리지 않은 최소 변경입니다. 다만 같은 원인의 테스트가 2건 더 남아 있어 완료 기준을 충족하지 못합니다.

요구사항 대조 (#762)

완료 기준	상태
가짜 pid 가 살아 있어도 해당 테스트들이 결정적으로 통과	⚠️ 이슈에 명시된 2건은 해결, 동일 원인 1건 잔존
pnpm --filter @gh-symphony/orchestrator test 반복 통과	✅ 확인
동일 패턴을 쓰는 다른 테스트(dispatch.test.ts, recovery 관련)도 점검	❌ 미충족 — 아래 P1
changeset 불필요(테스트만)	✅ 올바름

스모크 테스트

Preview 환경이 없어 로컬에서 검증했습니다. 이슈가 말하는 "가짜 pid 와 같은 번호의 프로세스가 살아 있는 호스트"를 vitest.setup.ts 에서 process.kill(pid, 0) 을 가짜 pid 집합에 대해 성공시키는 방식으로 재현했습니다(검증 후 원복).

실행	base a77cc34	PR head 129e3f1
평시 (service.test.ts)	193 passed	193 passed
가짜 pid 생존 시뮬레이션 (service.test.ts)	3 failed / 190 passed	1 failed / 192 passed
가짜 pid 생존 시뮬레이션 (orchestrator 전체 14파일)	—	1 failed / 360 passed
pid 집합을 파일 내 전체 리터럴로 확대	—	2 failed / 359 passed

base 에서 실패한 3건 = 이슈에 적힌 2건 + renders the persisted attempt when restarting a continuation. 이 PR 은 앞의 2건만 고쳤습니다.

dispatch.test.ts 와 나머지 12개 파일은 시뮬레이션에서 모두 통과했습니다 — 이 부분은 문제 없습니다.

게이트

pnpm lint / pnpm typecheck / pnpm build / pnpm test 모두 통과, CI(Test, Container Smoke) 그린 확인.

지적사항

• P1 — renders the persisted attempt when restarting a continuation (L7202, pid 4310) 가 여전히 호스트 프로세스 테이블에 의존. 실패 재현 로그를 인라인에 첨부했습니다.
• P2 — passes issue workspace root to after_run hook environment (L15563, processId: 999999) 도 같은 노출면. spawnImpl pid 뿐 아니라 store 에 저장하는 processId 도 대상입니다.
• P2(제안, 선택) — 개별 주입 대신 vitest.setup.ts 에서 기본 프로브를 막아 클래스를 닫는 방안. 이 PR 범위 밖이므로 별도 이슈 분리도 괜찮습니다.

오버엔지니어링 / 임의 결정

없습니다. 변경은 6줄이고 프로덕션 동작 변화가 없으며, PR 본문의 리스크 서술도 실제와 일치합니다. changeset 을 넣지 않은 판단도 CLAUDE.md 기준에 맞습니다.

다음 라운드 기준

P1(필수) + P2 첫 항목(권장)을 같은 주입 방식으로 닫아주시면 Approve 하겠습니다. P2 제안 항목은 반영/미반영 어느 쪽이든 스레드에 판단만 남겨주시면 충분합니다.

---

Generated by Claude Code

[hojinzs]

리뷰 보완 (독립 검증, 1회차 Request changes와 결론 일치) — 접근(isProcessRunning 주입 + 호출 단언)은 맞고 두 테스트는 고쳐졌습니다. 증명: (A) process.kill(-pid,0)가 4000–5999에서 성공하도록 임시 shim → main service.test.ts 3 실패(:1849, :3615, :7196) vs PR 1 실패(:7202); (B) 실제 detached 프로세스 pid로 치환 → main 3 실패(이슈와 동일 메시지), PR은 :1849·:3618 통과하나 :7202 실패(retryWorkerEnv undefined, :7258). 남은 결함: service.test.ts:7223 worker.pid = 4310 + workers.length에 isProcessRunning 미주입(:7229) — 같은 패턴이라 #762 완료 기준("살아 있어도 결정적으로 통과", "동일 패턴 점검") 미충족. 5회 순차 361/361, typecheck·lint OK.

[hojinzs]

Approve — 1라운드 지적사항 전부 해소, 완료 기준 충족 (라운드 2/6, 종결)

1라운드 대조

항목	상태
P1 — renders the persisted attempt when restarting a continuation (pid 4310)	✅ 516f7a63 에서 동일 주입 방식으로 수정, 재현 실패 소멸 확인
P2 — passes issue workspace root to after_run hook environment (processId: 999999)	✅ 수정, 재현 실패 소멸 확인
P2(제안) — suite-wide 프로세스 테이블 차단	✅ 스레드에 판단 기록됨. 제가 "반영/미반영 어느 쪽이든 판단만 남겨달라"고 했으므로 요건 충족

세 번째 항목의 "#762 범위를 넘으므로 개별 주입을 유지한다"는 결정에 동의합니다. 실제로 이번에 orchestrator 전체를 시뮬레이션했을 때 남은 실패가 0이라, 구조적 가드 없이도 이 이슈가 말한 결함은 닫힙니다.

#762 완료 기준 검증 (독립 재현)

시뮬레이션 방식은 1라운드와 동일합니다 — setup 파일에서 process.kill(pid, 0) 을 테스트용 가짜 pid 집합에 대해 성공시켜 "가짜 pid 와 같은 번호의 프로세스가 살아 있는 호스트"를 만들고, 검증 후 원복했습니다. 서비스가 process.kill(-pid, 0) 으로 프로세스 그룹을 보므로 음수 pid 도 함께 가로챘습니다.

완료 기준	결과
가짜 pid 가 살아 있어도 결정적으로 통과	✅ service.test.ts 193 passed / 0 failed (base a77cc34 는 동일 조건에서 4 failed)
동일 패턴을 쓰는 다른 테스트(dispatch.test.ts, recovery) 점검	✅ orchestrator 14개 파일, pid 리터럴 78개 전부 생존 조건에서 361 passed / 0 failed
pnpm --filter @gh-symphony/orchestrator test 10회 연속 통과	✅ 10/10, 매회 361 passed
changeset 불필요(테스트만)	✅ 프로덕션 코드 변경 없음, CLAUDE.md 기준에 맞음

base 대조가 핵심입니다. 같은 shim 아래 base 는 정확히 4건이 깨지고 PR head 는 0건이며, 그 4건이 이 PR 이 손댄 4곳과 일치합니다. 시뮬레이션이 실제 결함을 재현하고 있고 이 PR 이 그것만 정확히 닫았다는 뜻입니다.

스모크 테스트

Preview 환경이 없어 로컬에서 수행했습니다. 프로덕션 경로 변경이 없는 테스트 전용 PR이라 API·화면 대상은 없고, 검증 대상은 "테스트가 호스트 프로세스 테이블에 의존하지 않는가" 자체입니다. 위 표가 그 결과입니다.

게이트

pnpm lint ✅ / pnpm typecheck ✅ / 변경 파일 Prettier ✅ / CI Test·Container Smoke ✅ (516f7a6)

pnpm format 은 저장소 전역 90개 파일에서 실패하지만 pnpm-lock.yaml·prettier.config.mjs 까지 포함된 기존 상태이고, 이 PR 의 변경 파일은 깨끗합니다. 이 PR 과 무관합니다.

요구사항 외 변경 / 오버엔지니어링

없습니다. 최종 diff 는 1개 파일 12줄 추가이고 전부 isProcessRunning 주입과 그 seam 을 지키는 단언입니다. 프로덕션 동작 변화 0, 임의 결정 없음. 주입값을 항상 false 로 둔 것이 "살아 있는 워커는 재디스패치하지 않는다"는 커버리지를 지우지 않는지도 확인했는데, 그 경로는 service.test.ts 의 다른 테스트들(isProcessRunning: (pid) => pid === 4410, livePids 기반 등)이 별도로 덮고 있어 손실이 없습니다.

참고 (블로킹 아님)

• 인라인에 남긴 packages/cli/src/commands/lifecycle.test.ts:585 건은 이 PR 범위 밖입니다. 제가 별도 이슈로 올리겠습니다.
• 브랜치가 main 보다 6 커밋 뒤에 있지만 mergeable_state: clean 이고 CI 그린이라 그대로 머지 가능합니다.

지적사항이 모두 닫혔고 완료 기준이 재현 가능한 근거로 충족되어 Approve 하고 리뷰를 종결합니다. 추가 라운드는 불필요합니다.

---

Generated by Claude Code

[hojinzs]

P1 해소 확인 — 1라운드에서 재현했던 실패가 사라졌습니다.

동일한 시뮬레이션(vitest.setup 에서 process.kill(pid, 0) 을 이 파일의 pid 리터럴 전체에 대해 성공시킴, 검증 후 원복)을 다시 돌린 결과입니다.

대상	service.test.ts 결과
base a77cc34	4 failed / 189 passed
PR head 516f7a63	193 passed (0 failed)

base 에서 깨진 4건이 정확히 이 PR 이 손댄 4건(이슈 명시 2건 + P1 + P2)과 일치합니다. 이 테스트도 그중 하나였고 이제 통과합니다.

worker.pid = 4310 + workers.length 로 워커가 여러 개 생기는데 첫 워커(4310)만 단언한 것도 적절합니다 — 재시도 판정이 걸리는 지점이 첫 워커의 종료이므로 이 단언이 주입 seam 을 정확히 가리킵니다.

---

Generated by Claude Code

[hojinzs]

P2 해소 확인 + 같은 부류의 잔존 노출면 1건 (이 PR 범위 밖, 별도 이슈 제안 — nit)

영속 processId 쪽도 닫혔습니다. 위 시뮬레이션에서 base 는 이 테스트가 ENOENT: .after_run_workspace_path 로 깨졌고 PR head 는 통과합니다.

추가로 orchestrator 패키지 전체를 같은 방식으로(패키지 내 테스트의 pid 리터럴 78개 전부 생존 처리) 돌려봤습니다.

범위	결과
orchestrator 14개 파일 전체, 가짜 pid 전부 생존	361 passed (0 failed)

dispatch.test.ts, recovery 계열 모두 깨끗합니다. #762 의 "동일 패턴을 쓰는 다른 테스트도 점검" 기준은 이걸로 충족된 것으로 봅니다.

다만 패키지 밖에 같은 부류가 하나 남아 있어 기록만 남깁니다. packages/cli/src/commands/lifecycle.test.ts:585 의 processId: 999_999 가 여기와 완전히 동일한 패턴이고, recover --dry-run 이 실제 프로세스 테이블을 조회합니다.

FAIL  src/commands/lifecycle.test.ts > reports recoverable runs without invoking recovery in dry-run mode
AssertionError: expected false to be true
 ❯ src/commands/lifecycle.test.ts:611:7

(cli 패키지 pid 리터럴 생존 시뮬레이션: 1 failed / 590 passed. pid 999999 가 살아 있으면 stale run 이 recoverable 로 잡히지 않아 실패합니다.)

이 PR 에서 고치실 필요 없습니다. main 에 이미 있던 것이고 #762 의 명시 범위(orchestrator)도 아닙니다. pnpm test 전체를 게이트로 쓰는 로컬 에이전트 파이프라인 관점에서만 남은 구멍이라 별도 이슈로 올려두겠습니다.

---

Generated by Claude Code