Skip to content

test: stabilize clock-dependent auth and usage fixtures - #3950

Closed
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:agent/gui-timezone-fixture-20260908
Closed

test: stabilize clock-dependent auth and usage fixtures#3950
luvs01 wants to merge 2 commits into
lidge-jun:devfrom
luvs01:agent/gui-timezone-fixture-20260908

Conversation

@luvs01

@luvs01 luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Two clock-dependent fixtures can fail without a product regression.

The Santiago DST case in usage-custom-range.test.tsx changes process.env.TZ inside the shared test process. On Windows with Bun 1.4.0, restoring an initially absent variable can change the effective timezone from the one used to calculate the module's date-bound expectations. Four later URL assertions then differ by nine hours, even when this file runs alone.

Run the unchanged Santiago UI assertions in a child Bun process started with TZ=America/Santiago. The parent retains the presence and value of TZ and its local Date epoch. The child is limited to this exact test name, has a 12-second process deadline, and reports timeout, signal and nonzero-exit failures with captured diagnostics. The admission-substitution test also generated its expected JWT a second time after storing the original. Crossing a wall-clock second changes its expiry claim and therefore its bytes. Capture one token for both storage and comparison.

No product code or date interpretation changes.

Verification

Current standalone head 9b9e4d28bebf58dcb07c161914b3b9e324c55713.

  • Bun 1.4.0 on Windows: unchanged baseline reproduced 20 pass / 4 fail; the fixed file passed 24 tests with 331 assertions.
  • Parent timezone presence/value and local epoch are asserted unchanged after the child returns. The original skipped-midnight, final-day activity and tooltip assertions remain in the child.
  • Full dashboard validation passed 1,933 tests across 242 files with 15,655 assertions in the dependent proactive-preset validation tree. That tree includes this exact fixture change plus an unrelated Subagents change; this is not a claim of a separate full-suite run on the test-only tree.
  • Dashboard lint, i18n lint and production build, root typecheck, privacy scan and diff checks passed in that validation tree. Rebasing onto dev 514350e6f79ed4539378388bc39d3fc79ff2c70c changed only devlog files; test and build inputs remained identical.
  • Independent read-only review checked subprocess bounds and preservation of the parent's timezone.
  • The authentication failure occurred in a macOS CI job across a logged second boundary. A deterministic advancing-expiry control failed on the old assertion and passed with the captured token; the normal focused test also passed (4 assertions). Exact source bytes and SHA-256 were restored after the control, and typecheck/privacy/diff checks passed.
  • Cross-platform CI on the updated standalone head passed 24 jobs but failed Windows 4/6 on an unchanged storage-scanner test exceeding its explicit 15-second deadline. The same test, scanner and cleanup/watchdog inputs passed in the dependent preset run. Attempt 2 reran only the failed jobs and completed successfully; the precise I/O delay cause remains unestablished. All cross-platform jobs are now green on this head. The current CodeRabbit review has no actionable comments. The test-only screenshot waiver remains pending before undrafting.

This changes two test files and has no visible UI change to capture. Please apply the documented maintainer screenshot waiver for this test-only change if the path-based gate requests an image.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed (no product behavior change).
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

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.

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: a4eff372-8872-44f4-be03-6585d17aa33b

📥 Commits

Reviewing files that changed from the base of the PR and between 514350e and 9b9e4d2.

📒 Files selected for processing (2)
  • gui/tests/usage-custom-range.test.tsx
  • tests/codex-integration/codex-auth-context.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The changes update two tests. The DST test isolates the Santiago timezone in a child process. The authentication test reuses the credential written to auth.json when validating the authorization header.

Changes

DST test isolation

Layer / File(s) Summary
Santiago timezone test execution
gui/tests/usage-custom-range.test.tsx:157-195
When the timezone is not America/Santiago, the test runs the filtered test in a Bun child process with a timeout and verifies the parent timezone and local time. When the timezone matches, the DST assertions run directly.

Authentication credential consistency

Layer / File(s) Summary
Persisted credential and header assertion
tests/codex-integration/codex-auth-context.test.ts:1450-1462
The test stores liveJwt() in storedCredential, writes that value to auth.json, and compares the substituted authorization header with the same value.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 9b9e4

This test-only change prevents timezone leakage and JWT timing mismatches, improving test stability without changing product behavior. No current merge-blocking risk remains.

Suggested reviewers: invalid-email-address

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 2…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two test-only changes: stabilizing clock-dependent usage fixtures and authentication test behavior. It is concise, specific, and clearly related to the main changes…
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 7, 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 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

⏳ DRAFT

  • UI screenshot required.

What to do

  • Add a screenshot of the UI change to the PR description.
  • Tick all four boxes in the PR description once you're done (currently 3/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.

3/4 boxes ticked.

This pull request was already a draft. Its draft status will be preserved after every issue above is resolved.
@luvs01 Tick the boxes once your local CI is green, your branch is on the latest dev commit, and every correct Codex and CodeRabbit finding is resolved.

@lidge-jun

Copy link
Copy Markdown
Owner

리뷰 · 우선순위 48 / 80

지금 dev 끝은 514350e6f입니다. 바로 위는 #3948(C 트랙 config/init 스택 문서 마감)이고, 런타임 끝은 여전히 #3941(portable exclusive config temps + init publication recovery)입니다. 패키지는 2.48.0 라인입니다. 이 PR은 그 릴리스/문서 스택과 무관한 GUI 테스트 하니스 고침입니다.

이 PR(#3950)이 고치는 문제는 한 줄로 말하면 “산티아고 서머타임 테스트가 공유 프로세스의 TZ를 건드리면, 윈도우 Bun에서 원래 TZ가 없었을 때 복구가 깨져서 뒤에 오는 URL 단언이 9시간씩 어긋난다”입니다. 지금 HEAD의 gui/tests/usage-custom-range.test.tsx 156–181줄은 process.env.TZ = "America/Santiago"로 바꿨다가 finally에서 delete/restore합니다. 제품 코드(날짜 해석, Usage UI)는 건드리지 않고, 그 테스트만 TZ=America/Santiago로 띄운 자식 Bun 프로세스에서 돌리게 바꿉니다. 부모는 TZ 존재 여부와 로컬 Date epoch이 그대로인지 확인하고, 자식은 테스트 이름 정규식·10초 테스트 타임아웃·12초 프로세스 데드라인으로 묶여 있습니다.

왜 지금 dev에 넣을 만한가. Usage 커스텀 레인지와 히트맵 툴팁은 이미 HEAD에 있고(#3508/#3712 계열 로그/필터와는 다른 표면), 이 케이스는 DST 자정 경계에서 마지막 날 활동/툴팁이 남는지를 지키는 회귀입니다. 윈도우 CI나 로컬 Bun 1.4.0에서 이 파일만 돌려도 4개가 깨진다는 재현이 있으면, 제품 버그가 아니라 테스트 오염이 릴리스 신호에 잡음을 넣는 상황입니다. types.ts/config.ts 대형 분할과 겹치지 않고, 손대는 파일도 테스트 하나뿐입니다. 닫을 대상이 아니라 독립적으로 넣을 수 있는 chore입니다.

다만 아직 draft이고 enforce-target이 fail로 보이며 mergeState는 BLOCKED입니다. 작성자도 Cross-platform CI가 끝날 때까지 draft로 두겠다고 적었습니다. #3951이 같은 Santiago 커밋을 선행으로 얹고 있으니, 이 PR을 먼저 dev에 넣고 #3951에서 그 커밋을 빼는 순서가 맞습니다. 스크린샷 면제는 테스트-only라 문서화된 waiver 경로를 쓰면 됩니다.

라인 / 심볼 문제:

라인 156–181 (HEAD 기준, PR에서는 자식 분기로 교체) - 부모에서 TZ를 더 이상 바꾸지 않는 방향은 맞음. 다만 자식이 실패하면 부모 expect에 stdout/stderr를 붙이는 형태라, CI 로그가 길어질 수 있음.
Bun.spawnSync + process.execPath test import.meta.path -t ^…$ - 같은 파일을 재귀 실행함. 필터가 빗나가면 부모 스위트가 통째로 다시 돌거나, 이미 TZ=America/Santiago인 환경에서는 자식 분기 없이 본문이 바로 실행됨(의도된 분기).
--timeout 10000과 spawn timeout: 12000 / 테스트 함수 , 15000 - 세 층이 겹침. 느린 윈도우 에이전트에서 간헐 timeout이 나면 숫자를 맞출 필요가 생김.
Object.hasOwn(process.env, "TZ") - Bun/현대 런타임에서는 괜찮음. 다만 env 프록시에서 hasOwn이 예상과 다르면 “존재 여부” 단언만 깨질 수 있음.
제품 경로 없음 - Usage UI·날짜 파서 변경 없음. 회귀는 테스트 격리 품질에만 달림.
draft / enforce-target fail - Ready 전 CI 원인 확인. draft만으로 blocked일 가능성 큼.

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

  • #3950을 #3951보다 먼저 머지할지, 아니면 #3951에 묶인 채로 두고 Santiago 커밋만 중복으로 둘지.
  • 윈도우 전용 재현을 메인테이너가 한 번 더 볼지, 작성자 20pass/4fail → 24pass 보고만으로 충분한지.
  • 자식 프로세스 방식이 스위트 전체 시간에 미치는 영향을 허용할지, 나중에 파일 분리(usage-custom-range-santiago.test.tsx + env)로 바꿀지.
  • draft 해제 시점을 Cross-platform CI 초록으로 묶을지.

너의 추천
CI(특히 윈도우 관련 체크) 초록 확인 후 draft 해제하고 dev에 단독 머지하세요. #3951에서는 이 PR이 랜딩된 뒤 Santiago 선행 커밋을 빼라고 이미 적혀 있으니 그 순서를 지키면 됩니다. types/config 분할로 닫을 대상은 아닙니다.

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

@luvs01 luvs01 changed the title test(gui): isolate the Santiago timezone fixture test: stabilize clock-dependent auth and usage fixtures Sep 7, 2026
@luvs01

luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@luvs01

luvs01 commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

The current head 9b9e4d28bebf58dcb07c161914b3b9e324c55713 has green cross-platform CI after one failed-job rerun, and CodeRabbit reviewed both changed files with no actionable comments. The body distinguishes the local fixture/dashboard checks from the linked CI run and records the initial unrelated storage-test timeout.

Following the earlier recommendation to land this prerequisite independently, could a maintainer confirm the screenshot waiver? The diff changes only gui/tests/usage-custom-range.test.tsx and tests/codex-integration/codex-auth-context.test.ts; there is no visible UI change to capture. The gate requires a maintainer-authored statement that this does not change the GUI. I will undraft once that documented waiver clears the remaining image gate.

lidge-jun added a commit that referenced this pull request Sep 7, 2026
Preserve the stored JWT as the comparison oracle across clock boundaries. Carries only the JWT slice of #3950.

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

Copy link
Copy Markdown
Owner

Both independently delivered fixes are now on dev: JWT fixture via #3962 (eb4188a) and timezone isolation via #3967 (c46c22f). Original author luvs01 is preserved in both landed commits. The timezone carry adds explicit child identity/cwd, completion proof and aligned timeout boundaries; exact final candidate passed ten scenarios each on hosted Linux, Windows and macOS (34170111719). Standard timezone PR CI34170093095 completed on attempt2 (19success/2skip); attempt1 macOS root client-test timeout is retained as an unresolved CI reliability residual, not claimed fixed. Both actual landing trees, files and dev ancestry were checked before closing this mixed source PR as carried.

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).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants