Skip to content

test(cli): make stale-port status fixture deterministic - #3980

Closed
yansigit wants to merge 1 commit into
lidge-jun:devfrom
yansigit:codex/upstream-cli-stale-port-20260908
Closed

yansigit wants to merge 1 commit into
lidge-jun:devfrom
yansigit:codex/upstream-cli-stale-port-20260908

Conversation

@yansigit

@yansigit yansigit commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Allocate the refused runtime-record port after binding the deliberately occupied configured-port listener, and assert the two ports differ. Reallocate per test so the occupied listener cannot reuse a previously released shared port and silently invert the fixture.

Test fixture only; no user documentation change needed.

Verification

bun test tests/cli/cli-status-json.test.ts: 47 passed.

All runtime checks used a fresh temporary OPENCODEX_HOME and alternate port; production config fingerprint and backup inventory remained unchanged. Full root-suite and review-readiness gates have not been completed for this head; this is intentionally a draft.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Added/updated regression coverage or verified existing coverage for the affected behavior.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness

  • Local CI green.
  • Branch on the latest dev commit.
  • All correct Codex and CodeRabbit findings fixed.
  • Ready-for-review confirmation.

Co-authored-by: SB Yoon 44089734+yansigit@users.noreply.github.com
Co-authored-by: Yumi automation@sbyoon.com

Review readiness checklist

This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:

  • All CI tests are green on my local testing.

  • I pushed my PR to the latest dev commit.

  • I resolved all correct Codex and CodeRabbit findings.

  • My PR is ready for review.

@coderabbitai

coderabbitai Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Deterministic PR hygiene checks passed.

@github-actions github-actions Bot added the chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • review readiness checklist open (0/4 boxes ticked).

What to do

  • Tick all four boxes in the PR description once you're done (currently 0/4).

Review readiness checklist

  • ⬜ All CI tests are green on my local testing.
  • ⬜ I pushed my PR to the latest dev commit.
  • ⬜ I resolved all correct Codex and CodeRabbit findings.
  • ⬜ My PR is ready for review.

0/4 boxes ticked.

This PR stays in draft until every box above is ticked.

Hygiene

Deterministic PR hygiene checks passed.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 47 / 80

이 PR은 tests/cli/cli-status-json.test.tsstale-port status fixture만 고칩니다. 지금 dev HEAD c15662855에서는 beforeAll로 한 번 잡은 freePort를 여러 테스트가 재사용합니다. occupied listener가 그 포트를 다시 bind하면, “거절되어야 하는 runtime-port”와 “일부러 점유한 configured-port”가 같아져 fixture가 조용히 뒤집힐 수 있습니다. 이 변경은 allocateFreePort()beforeEach마다 돌리고, occupied를 bind한 뒤에 recordedPort를 다시 할당하며, 두 포트가 다르다고 assert합니다. 제품 CLI/status 코드는 안 건드립니다. types/config 분할과 무관합니다.

라인 tests/cli/cli-status-json.test.ts · allocateFreePort - listen(0)으로 ephemeral port를 잡고 바로 close합니다. 공유 freePort 전역을 없앱니다.
라인 tests/cli/cli-status-json.test.ts · beforeEach - 테스트마다 새 freePort를 받습니다. 이전 케이스가 포트를 반납한 뒤의 재사용 독을 끊습니다.
라인 tests/cli/cli-status-json.test.ts · recordedPort after occupied - occupied bind 이후에 거절용 포트를 다시 고르고 not.toBe(occupiedPort)로 잠급니다. 이 assert가 없으면 레이스가 다시 숨어듭니다.
경로 본문 검증 - bun test tests/cli/cli-status-json.test.ts 47 passed를 보고했습니다. draft·readiness 미완입니다.

메인테이너의 판단이 필요한 지점

  • 같은 beforeAll freePort 패턴이 다른 CLI 테스트에 있으면 후속으로 묶을지
  • draft를 바로 머지 열에 태울지, 포커스 테스트 증거만으로 ready할지

너의 추천
fixture determinism 수정으로 방향이 맞습니다. 포커스 테스트가 초록이면 draft 해제 후 머지하세요. 런타임 동작 변경 없습니다.

이 댓글은 grok-bot이 작성했습니다

lidge-jun added a commit that referenced this pull request Sep 8, 2026
…lback test (#4053)

* test(cli): make stale-port status fixture deterministic

Carried from #3980 (author yansigit) for maintainer-side CI and sequential integration.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* refactor(router): isolate API-key selection capture

Carried from #3897 (author parkjs101) for maintainer-side CI and sequential integration.
Closes #3894

Co-authored-by: parkjs101 <93533648+parkjs101@users.noreply.github.com>

* docs: retire the historical dashboard capture pack

Carried from #3963 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* refactor(gui): stabilize model feedback callback dependencies

Carried from #3984 (author yansigit) for maintainer-side CI and sequential integration.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* test(gui): pin the publishFeedback callback identity

#3984 changes publishFeedback to a useCallback and adds it to saveDisplayName's
dependency array, but shipped without coverage, so the hygiene gate flags
missing_regression_test. This source-oracle test asserts both halves and fails
on dev without the fix (0 pass / 2 fail), so the suppression cannot come back
unnoticed.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* chore(assets): file the #3984 screenshot with the others

#3984 added its review screenshot under a new assets/pr-screenshots/ directory.
The repository's nineteen existing PR screenshots all live in
docs-site/public/pr-screenshots/, so this moves it there and keeps the new
top-level directory out of the tree.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

---------

Co-authored-by: t <a@b.com>
Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
Co-authored-by: parkjs101 <93533648+parkjs101@users.noreply.github.com>
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
@lidge-jun

Copy link
Copy Markdown
Owner

Landed on dev in 164bd2b via #4053, which carried four reviewed changes as one sequential integration branch. Your commit is preserved in the squash body with a Co-authored-by trailer, so the contribution stays attributed to you.

Verification at the merged head e52116316: Cross-platform CI run 34265891731 finished with 19 jobs successful and 0 failures, bun x tsc --noEmit exit 0, and the changed test files pass. An independent review confirmed the stacked tree is byte-identical to the union of the carried diffs.

Closing this PR because the change is already on dev. Thank you.

@lidge-jun lidge-jun closed this Sep 8, 2026
@lidge-jun

Copy link
Copy Markdown
Owner

Landed via #4053 at 164bd2b

@lidge-jun lidge-jun added the landed-via-maintainer Original PR closed after landing via a maintainer merge train label Sep 8, 2026
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
…lback test (lidge-jun#4053)

* test(cli): make stale-port status fixture deterministic

Carried from lidge-jun#3980 (author yansigit) for maintainer-side CI and sequential integration.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* refactor(router): isolate API-key selection capture

Carried from lidge-jun#3897 (author parkjs101) for maintainer-side CI and sequential integration.
Closes lidge-jun#3894

Co-authored-by: parkjs101 <93533648+parkjs101@users.noreply.github.com>

* docs: retire the historical dashboard capture pack

Carried from lidge-jun#3963 (author luvs01) for maintainer-side CI and sequential integration.

Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>

* refactor(gui): stabilize model feedback callback dependencies

Carried from lidge-jun#3984 (author yansigit) for maintainer-side CI and sequential integration.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* test(gui): pin the publishFeedback callback identity

lidge-jun#3984 changes publishFeedback to a useCallback and adds it to saveDisplayName's
dependency array, but shipped without coverage, so the hygiene gate flags
missing_regression_test. This source-oracle test asserts both halves and fails
on dev without the fix (0 pass / 2 fail), so the suppression cannot come back
unnoticed.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

* chore(assets): file the lidge-jun#3984 screenshot with the others

lidge-jun#3984 added its review screenshot under a new assets/pr-screenshots/ directory.
The repository's nineteen existing PR screenshots all live in
docs-site/public/pr-screenshots/, so this moves it there and keeps the new
top-level directory out of the tree.

Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>

---------

Co-authored-by: t <a@b.com>
Co-authored-by: yansigit <44089734+yansigit@users.noreply.github.com>
Co-authored-by: parkjs101 <93533648+parkjs101@users.noreply.github.com>
Co-authored-by: luvs01 <27862058+luvs01@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance, CI, tests, refactors, or build changes (not a user-facing bug or feature). landed-via-maintainer Original PR closed after landing via a maintainer merge train

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants