test: cover remaining §17 conformance rows - #765
Conversation
Coverage Report
File CoverageNo changed files found. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 615a358805
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
hojinzs
left a comment
There was a problem hiding this comment.
리뷰 1회차 — Request changes
대상 head: 615a358805455c50fd3e63641dace9c399c43e4f (4 commits, 6 files, +188/−17). 소스 변경 없음(test + docs), changeset 없음 — 적절.
요구사항 반영 (이슈 #678 체크리스트 행별 매핑)
| #678 행 | 이 PR의 대응 | 판정 |
|---|---|---|
§17.3 listIssuesByStates([]) / fetchIssueStatesByIds([]) → provider 호출 없음 (GitHub/Linear/file) |
tracker-github.test.ts:2383 / tracker-linear.test.ts:1153 — fetchImpl spy not.toHaveBeenCalled(); file-tracker-adapter.test.ts:235 — 존재하지 않는 missing-issues.json 경로로 호출해 [] 반환(읽었다면 ENOENT) |
충족 |
| §17.3 ID refresh malformed 레코드 → 실패 | Linear 신규 tracker-linear.test.ts:1233 — state: null 노드로 fetchIssueStatesByIds → rejects.toThrow("Linear issue state name is required."), 실제 throw는 orchestrator-adapter.ts:600 requireString(...). GitHub은 기존 tracker-github.test.ts:5416("fails loudly when a refreshed Project item omits the state field")에 매핑, 중복 추가 없음 |
충족 |
| §17.2 워크스페이스 경로에 일반 파일 → 안전 처리 | service.test.ts:1963 — resolveIssueWorkspaceDirectory 위치에 일반 파일을 만들고 runOnce(): dispatched 0, spawnImpl 미호출, 파일 내용 보존, 레코드 retry_queued. 프로덕션 경로는 service.ts:2809 mkdir(recursive)가 EEXIST로 throw → dispatch 실패로 분류되는 실제 동작이며 mock으로 가려진 부분 없음. 정책은 "fail"로 docs/architecture.md에 명시 |
충족 |
§17.2 after_create 재사용 디렉터리 미재실행 |
기존 service.test.ts:14874("runs after_create only for a new workspace …")에 매핑만 함, 중복 없음 |
충족 |
| §17.4 running 없을 때 reconcile no-op | service.test.ts:389 — loadAllRuns → []인 store로 실제 reconcileProject 호출, reconcileRun 미호출·recovered 0 확인. (selectCurrentRunsForReconciliation spy는 loop가 activeRuns를 쓰므로 사실상 불필요하지만 무해) |
충족 |
| §17.5 stderr 분리 / 미지원 dynamic tool 거부 | dynamic tool: codex-dynamic-tools.test.ts:45("structured error for an unknown tool and keeps the session usable") 매핑 OK. stderr 분리는 매핑이 실제 요구사항을 검증하지 않음 (아래 지적 2) |
부분 |
§17.7/§13.7 start.ts port/host/bind |
기존 start.test.ts:1625(loopback 127.0.0.1 / 4680), :1658("passes an explicit port and --bind-all") 매핑. 리포트의 "0건" 기록은 이미 stale이었고, 테스트는 실제 존재함. 라벨은 §13.7이 맞음 (아래 지적 3) |
충족(라벨 수정 필요) |
docs/architecture.md 행↔테스트 매핑 |
"§17 conformance test matrix" 섹션 추가, AGENT_TEST.md에 "§17 conformance coverage" 행 추가 |
충족(내용 정확성은 아래) |
검증한 것
- 격리 worktree에서
origin/feat/678-spec17-conformance-testscheckout →pnpm install,pnpm -r build성공. - 터치된 4개 패키지(
orchestrator,tracker-file,tracker-github,tracker-linear) 테스트 3회 연속 실행: 매회 363 / 21 / 130 / 41 passed, 실패·flake 0. pnpm typecheck,pnpm lint,pnpm format통과.- 결정성: 신규 테스트에 고정 포트·실시간 타이머·하드코딩 OS pid 없음(#758/#762 패턴 해당 없음).
mkdtemp+ 고정now,fetchImpl주입만 사용. - 프로덕션 코드 변경 없음, 스펙 파일 미수정.
- CI: head
615a358에 대한 GitHub Actions run이 하나도 없음(gh pr checks→ "No checks reported").mergeable: false / mergeable_state: dirty상태라 pull_request merge ref를 만들 수 없어 CI가 트리거되지 않은 것으로 보임. - 머지 가능성:
git merge-tree origin/main HEAD→AGENT_TEST.mdCONFLICT. main의 #756(50e7313, retry scheduling events)이 같은 표의 3행을 수정했는데 이 PR은 표 전체 컬럼 폭을 재포맷해서 충돌.
지적사항
-
[Blocking]
AGENT_TEST.mdorigin/main과 충돌, CI 미실행. main을 merge/rebase해서 충돌을 해소하고 CI가 head에서 녹색으로 도는 것을 확인해 주세요. 재포맷은 Prettier가 시키는 것이라 어쩔 수 없지만, #756 변경 3행(Restart failure isolation/retry 관련 문구)이 유실되지 않게 확인 필요. -
[Blocking — 권위 매핑 표의 과장 주장]
docs/architecture.md:110(§17.3 행): "GitHub and Linear suites assert that malformed requested records fail while polling-list malformed items are omitted". Linear는 해당 없음 —- 코드:
packages/tracker-linear/src/orchestrator-adapter.ts:456-461listLinearIssues는 모든 노드를 throw하는normalizeLinearIssue로 매핑하므로 polling 리스트에 malformed 노드가 하나라도 있으면 전체 read가 실패하고, 생략+로그가 아닙니다. - 테스트: Linear 스위트에 리스트 생략을 검증하는 테스트가 없습니다(신규 테스트는 ID refresh 실패 경로만). GitHub만
tracker-github.test.ts:556, 588에 있음. - 이 표를 "authoritative"로 선언했으니 생략 주장은 GitHub으로 한정하고, Linear 리스트 생략은 미구현/미검증 gap으로 명시해 주세요(#678 본문대로 코드 변경은 별도 이슈로). Codex 코멘트와 동일 지적.
- 코드:
-
[Should fix]
docs/architecture.md:112(§17.5 행): "worker-protocol.test.tscovers protocol writes on stderr"가 스펙 행 "diagnostic stderr handling is kept separate from the protocol stream"의 증거로 제시되어 있는데, 참조되는 테스트(worker-protocol.test.ts:2394, 2502)는 worker→orchestrator 채널 writer의 backpressure/순서 테스트이지, app-server 자식의 stderr가 JSON-RPC 스트림과 분리되어 파싱되지 않음을 검증하지 않습니다. 실제로packages/runtime-codex/src/launcher.ts:83은child.stderr?.pipe(process.stderr)로 자식 stderr를 worker의 orchestrator 채널(stderr)에 합류시킵니다. 이 행은 실제 검증 테스트를 가리키거나, #678 본문("S21 결정 후")대로 gap으로 표기해 주세요. dynamic tool 거부 매핑(codex-dynamic-tools.test.ts:45)은 정확합니다. -
[Minor]
docs/architecture.md:113행 라벨 "§17.7 host port and bind lifecycle": upstream §17.7 행은 positional workflow path / 기본./WORKFLOW.md/ 종료 코드이고, port/host/bind는 §13.7입니다(리포트 B19가 §17.7 positional path 미구현을 이미 divergence로 기록). 라벨을 "§13.7"로 바꾸고 §17.7은 documented divergence임을 한 줄 적어 주세요.--portalias는 #751이 소유하므로 이 PR에서 더 건드릴 필요 없음. -
[Minor] Prettier 미적용 —
packages/orchestrator/src/service.test.ts:1965,:1992-1994(신규 hunk).npx prettier --check가 이 파일만 경고하며 두 곳 모두 이 PR이 추가한 줄입니다(mkdtemp(join(...))줄바꿈,expect(await readFile(...)).toBe(...)한 줄화). CI는pnpm format을 돌리지 않지만 저장소 컨벤션이므로pnpm format:write한 번 부탁드립니다. 다른 5개 변경 파일은 통과. -
[Nit]
service.test.ts:404-407selectCurrentRunsForReconciliationspy는 이 테스트에서 결과에 영향이 없음(loop는activeRuns기준). 앞 테스트와 대칭 유지 목적이면 그대로 둬도 됩니다.
판단
Request changes. 신규 테스트 5건은 모두 실제 프로덕션 경로를 검증하고(mock-away/tautology 없음), 결정적이며, 3회 연속 녹색·typecheck/lint/build 통과입니다. 그러나 (1) AGENT_TEST.md가 main과 충돌해 머지 불가 + CI가 head에서 실행되지 않았고, (2) 이 PR이 "authoritative"로 추가한 docs/architecture.md 매핑 표가 Linear polling 생략(§17.3)과 stderr 분리(§17.5) 두 항목에서 존재하지 않는 검증을 인증하고 있습니다. 충돌 해소 + 표 2행 정정(+§17.7 라벨) 후 CI 녹색이면 2회차에서 승인하겠습니다.
615a358 to
1af1ad6
Compare
hojinzs
left a comment
There was a problem hiding this comment.
리뷰 2회차 — Approve
대상 head: 1af1ad64c54d1dc7ef4d81a10bae080e01aa8595 (5 commits, 6 files, +195/−21). 1회차 지적 6건 전부 확인했고, 남은 blocking 이슈 없습니다.
1회차 지적사항 처리 확인
| # | 지적 | 처리 | 판정 |
|---|---|---|---|
| 1 | [Blocking] AGENT_TEST.md main 충돌 + CI 미실행 |
브랜치를 45bb54e(당시 main) 위로 rebase. git merge-tree --write-tree origin/main 1af1ad6 → 충돌 없음 (main이 이후 c549379까지 진행됐지만 여전히 clean). git diff -w --word-diff로 확인한 결과 #756의 3행을 포함해 기존 표 내용은 한 글자도 변경 없음, 컬럼 폭 재포맷 + 신규 §17 conformance coverage 1행 추가뿐. CI는 1af1ad6에서 Test·Container Smoke 모두 success |
해소 |
| 2 | [Blocking] architecture.md §17.3 Linear polling 생략 과장 주장 |
"GitHub and Linear suites assert that malformed requested records fail; GitHub alone covers omission of malformed polling-list items. Linear polling-list omission is a documented implementation gap."로 정정. listLinearIssues의 전량 throw 동작과 문서가 이제 일치 |
해소 |
| 3 | [Should fix] §17.5 stderr 분리 주장 | worker-protocol.test.ts 참조를 제거하고 "Stderr isolation from the protocol stream remains a documented implementation gap pending the S21 decision."로 변경. 존재하지 않는 검증을 인증하지 않음 |
해소 |
| 4 | [Minor] §17.7 라벨 | 행 라벨을 §13.7 host, port, and bind lifecycle로 교체하고 §17.7 positional workflow-path는 documented divergence로 명시 |
해소 |
| 5 | [Minor] Prettier 미적용 | npx prettier --check 6개 변경 파일 전부 통과. service.test.ts:9141 추가 hunk는 origin/main에서 이미 깨져 있던 기존 위반을 함께 고친 것으로 확인(main 원본 파일 단독 --check → warn). 무해 |
해소 |
| 6 | [Nit] selectCurrentRunsForReconciliation spy |
유지. 1회차에 "그대로 둬도 됨"이라 했으므로 이견 없음 | — |
이번 회차에 직접 검증한 것
pnpm install→pnpm build→ 터치된 4개 패키지 테스트:tracker-file21,tracker-linear41,tracker-github130,orchestrator366 전부 green.orchestrator는 연속 3회 실행 모두 366 passed, flake 0.pnpm lint,pnpm typecheck통과. 워킹 트리 clean.- 비어있지 않은 테스트인지(mutation check):
packages/tracker-github/src/orchestrator-adapter.ts의states.length === 0/issueIds.length === 0가드 2개를 제거한 뒤 신규 테스트를 돌리면does not call GitHub for empty state or ID lookups가 실패합니다. 항진명제가 아니라 실제 회귀를 잡습니다. (검증 후 원복) - §17.4 no-op 테스트의 타당성 재확인: 1회차 이후 다시 본 결과
resolveTrackerAdapter를 강제로 throw시키는 구성이 run 루프를 우회시키는 것 아닌지 의심했으나,service.ts:1162의for (const run of activeRuns)루프가service.ts:1204의resolveTrackerAdapter앞에 있으므로reconcileRun미호출은 순수하게activeRuns === []때문입니다. throw는 이후 dispatch 경로만 차단하며lastError단언이 도달 지점을 문서화합니다. 매핑 정확. - 매트릭스가 가리키는 테스트 실재 여부(authoritative 표이므로 전수 확인):
core/src/workspace-safety.test.ts(path containment 4건),service.test.ts:14893(after_create only for a new workspace),worker/src/codex-dynamic-tools.test.ts:45(unknown tool 구조화 거부),start.test.ts:1625/1658(loopback 127.0.0.1:4680, explicit port +--bind-all),e2e/host-dynamic-tool-e2e.mjs— 5개 참조 전부 존재하며 설명과 실제 테스트 내용이 일치합니다. - Smoke: 이 PR은 프로덕션 코드 변경이 없어 노출된 API·화면이 없습니다. Preview 환경 없음, 로컬에 Docker 미제공이라
run-e2e.sh는 직접 돌리지 못했고, 대신 head1af1ad6의 CIContainer Smokesuccess로 대체 확인했습니다. - 스코프: 프로덕션 코드 변경 0,
docs/symphony-spec.md미수정, changeset 없음(test/docs-only이므로 규약상 정확). 요구사항 밖 오버엔지니어링 없음.
이슈 #678 수용 조건
체크리스트 8행 모두 대응됩니다. 신규 테스트 5건(§17.3 empty lookup ×3, §17.3 Linear malformed refresh, §17.2 workspace 일반 파일, §17.4 reconcile no-op)은 pnpm test에 포함되고, 나머지 4행(§17.2 after_create, §17.5 dynamic tool, §13.7 host/port/bind, GitHub malformed refresh)은 기존 테스트에 중복 없이 매핑만 했으며 매핑이 정확합니다. AGENT_TEST.md에 §17 시나리오 행이 등록되었습니다. 검증되지 않은 2건(Linear polling 생략, stderr 분리)은 주장 대신 gap으로 정직하게 표기됐고, 코드 변경은 #678 본문대로 별도 이슈 몫입니다.
판단
Approve. 머지 가능하고 CI green입니다. 남은 documented gap 2건은 이 PR의 범위가 아니며, 후속 이슈로 다루면 됩니다.
Generated by Claude Code
|
검증 통과 → Land로 이동 (프로젝트 상태 관리) — 테스트 전용 PR. head |
1af1ad6 to
8b28386
Compare
8b28386 to
260ae4e
Compare
|
독립 검증 (Land 전) — 1회차 6개 항목 모두 해소 확인(리베이스 clean, Linear 생략 주장 → 문서화된 gap + 실패 테스트, stderr 인용 정정, §13.7 라벨, prettier). orchestrator 366/tracker-* 3회 연속 동일 통과, 결정성 OK. 비차단 1건(머지 후 정리): |
Issues — Closed #678
TL;DR
Adds focused tests for the §17 conformance rows that are not owned by other sub-issues, then documents the exact coverage boundaries and existing upstream divergences.
Change-point diagram
Start here
packages/tracker-{github,linear,file}/src/*test.tscovers adapter lookup and malformed-refresh behavior.packages/orchestrator/src/service.test.tscovers workspace-file safety and no-running reconciliation.docs/architecture.mdis the row-to-test matrix;AGENT_TEST.mdrecords the applicable black-box scenarios.Risks & rollback
Changed files
packages/tracker-file/src/file-tracker-adapter.test.tspackages/tracker-github/src/tracker-github.test.tspackages/tracker-linear/src/tracker-linear.test.tspackages/orchestrator/src/service.test.tsdocs/architecture.mdAGENT_TEST.mdEvidence
pnpm lintpnpm testpnpm typecheckpnpm build./e2e/run-e2e.sh happy 60TestandContainer Smokepassed on1af1ad64.Post-merge / human validation