Skip to content

test(layout): move windows, service, update into tests/<domain>/ (#3497) - #3509

Merged
lidge-jun merged 1 commit into
devfrom
codex/layout-slice1-windows-service-update
Sep 4, 2026
Merged

lidge-jun merged 1 commit into
devfrom
codex/layout-slice1-windows-service-update

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Summary

First move slice of #3497, driven by the tooling that landed in #3500: windows (20), service (20) and update (9) leave the flat tests/ root for tests/<domain>/. Every move is a git mv so history and blame survive. Three literal sites that name these paths (tests/codex-catalog-writer.test.ts, tests/native-main-claim.test.ts, tests/grok-lifecycle.test.ts, one docs plan) follow, scripts/test.ts gets update/update-stop-first.test.ts as its serial-lane entry, and layout.migrated records the three domains so tests/test-layout.test.ts now rejects a straggler at the root for them.

Eight lines needed a hand after the mover (all files that already bound their own repoRoot/root URL, which the rewriter refuses to shadow): they now use tests/helpers/repo-root.ts.

One tooling adjustment: devlog/ is dropped from the literal sweep. The first slice rewrote 59 old plan documents that only record where a file lived at the time, burying the real diff; nothing reads those paths.

Verification

  • bun scripts/test-layout/move.ts --domain windows --domain service --domain update then verify.ts: 0 stale literals, 0 unsuppressed escapes, 0 module-resolution errors, bun test --isolate tests/windows tests/service tests/update: 929 pass, 0 fail (49 files).
  • bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/test-runner.test.ts tests/fixture-dir-uniqueness.test.ts tests/ci-workflows.test.ts tests/codex-catalog-writer.test.ts tests/native-main-claim.test.ts: 214 pass, 0 fail.
  • bun x tsc --noEmit, bun run privacy:scan: green.
  • Exact-head ci (Linux 4 shards, storage/usage isolated jobs, macOS 1/2 and 2/2) is the proof that discovery, the batch script and the isolated jobs still find the moved files.

Checklist

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

Summary by CodeRabbit

  • Tests

    • Corrected test references and path resolution after reorganizing service, update, and Windows test suites.
    • Standardized repository-root file access across tests.
    • Updated the Windows popup timeout check to consistently verify a 30-second timeout.
    • Full verification now includes the service-scoped OAuth doctor test.
  • Documentation

    • Updated testing guidance and references to reflect current test locations and coverage.

@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 4, 2026 19:09
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 4, 2026

Copy link
Copy Markdown

Codex Review Summary

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

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-04T19:17:38.804345Z 23ca0f2 PR opened
ℹ️ 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

github-actions Bot commented Sep 4, 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 4, 2026
@coderabbitai

coderabbitai Bot commented Sep 4, 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: 095cb965-f85f-4a84-8efa-15de242228d7

📥 Commits

Reviewing files that changed from the base of the PR and between 5df664c and 23ca0f2.

📒 Files selected for processing (56)
  • docs/superpowers/plans/2026-07-26-oauth-reliability-integrity.md
  • scripts/test-layout/layout.json
  • scripts/test-layout/verify.ts
  • scripts/test.ts
  • tests/codex-catalog-writer.test.ts
  • tests/grok-lifecycle.test.ts
  • tests/native-main-claim.test.ts
  • tests/service/autostart-health.test.ts
  • tests/service/crash-guard.test.ts
  • tests/service/doctor-codex-envkey-readiness.test.ts
  • tests/service/doctor-oauth.test.ts
  • tests/service/doctor-provider-apikey.test.ts
  • tests/service/doctor-service-memory-contract.test.ts
  • tests/service/init-backup-cleanup.test.ts
  • tests/service/init-eof.test.ts
  • tests/service/process-state.test.ts
  • tests/service/service-probe-docker.test.ts
  • tests/service/service-secrets.test.ts
  • tests/service/service-stop-verification.test.ts
  • tests/service/service-tier-capability.test.ts
  • tests/service/service.test.ts
  • tests/service/shutdown-drain.test.ts
  • tests/service/shutdown-launcher.test.ts
  • tests/service/stale-state-purge.test.ts
  • tests/service/stop-deferred-teardown.test.ts
  • tests/service/systemd-install-cleanup-hardening.test.ts
  • tests/service/winsw.test.ts
  • tests/update/update-badge.test.ts
  • tests/update/update-job.test.ts
  • tests/update/update-notify.test.ts
  • tests/update/update-npm-cache-preflight.test.ts
  • tests/update/update-npm-invocation.test.ts
  • tests/update/update-stop-classification.test.ts
  • tests/update/update-stop-first.test.ts
  • tests/update/update-transactional.test.ts
  • tests/update/update-tray-handoff.test.ts
  • tests/windows/tray-proxy-deadline.test.ts
  • tests/windows/tray-proxy.test.ts
  • tests/windows/win-exec.test.ts
  • tests/windows/win-paths.test.ts
  • tests/windows/windows-atomic-replace.test.ts
  • tests/windows/windows-deploy-close-regressions.test.ts
  • tests/windows/windows-elevation-spawn.test.ts
  • tests/windows/windows-elevation.test.ts
  • tests/windows/windows-popup-fix.test.ts
  • tests/windows/windows-scheduler-install-verification.test.ts
  • tests/windows/windows-secret-acl.test.ts
  • tests/windows/windows-service-mutation-lock.test.ts
  • tests/windows/windows-service-wrappers.test.ts
  • tests/windows/windows-text-decoding.test.ts
  • tests/windows/windows-tray-restart-hardening.test.ts
  • tests/windows/windows-tray-run-limit.test.ts
  • tests/windows/windows-tray.test.ts
  • tests/windows/windows-user-principal-nonascii.test.ts
  • tests/windows/windows-user-principal.test.ts
  • tests/windows/winsw-stop-hardening.test.ts

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


📝 Walkthrough

Walkthrough

The change aligns service, update, and Windows tests with their relocated directories. It updates imports, repository-root path resolution, suite configuration, layout metadata, verification roots, and stale documentation references. Test logic remains unchanged.

Changes

Test tree alignment

Layer / File(s) Summary
Layout and suite wiring
docs/superpowers/plans/..., scripts/test-layout/*, scripts/test.ts, tests/codex-catalog-writer.test.ts, tests/grok-lifecycle.test.ts, tests/native-main-claim.test.ts
Updated test paths, migrated domains, verification roots, suite entries, and comments for the relocated test directories.
Service test path resolution
tests/service/*
Corrected source and helper imports. Replaced test-directory-relative fixture paths with repository-root helpers where needed.
Update test path resolution
tests/update/*
Corrected source and helper imports. Updated fixture and repository-root resolution for update tests.
Windows test path resolution
tests/windows/*
Corrected source and helper imports. Updated repository file, child-process, tray asset, and working-directory resolution.

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

Merge Risk: ⚪ Minimal · up to 23ca0

This change relocates service, update, and Windows tests while updating their imports, repository-root resolution, and suite references. No current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 50 files. (6 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the primary change: moving the windows, service, and update tests into domain-specific subdirectories under tests/.
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.
Full details: Docstring Coverage

Explanation

Docstring coverage is 11.11% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 50 files. (6 skipped: 2 unsupported, 4 over the file limit.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/layout-slice1-windows-service-update

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 66 / 80

설명

이 PR은 이슈 #3497의 첫 실제 이동 슬라이스다. 바로 직전에 dev에 들어온 #3500이 scripts/test-layout/(domain map · move · plan · verify)와 tests/helpers/repo-root.ts를 깔아 두었고, 그 도구로 windows(20개) · service(20개) · update(9개) 도메인 파일을 평평한 tests/ 루트에서 tests/<domain>/ 아래로 git mv 한다. 주장·로직은 건드리지 않고 경로와 import 깊이만 고친다.

지금 dev HEAD(5df664cda, package 2.43.0)는 테스트 레이아웃 도구(#3500)와 macOS CI 2-way shard(#3501)까지 끝난 상태다. 다만 도구만 있고 layout.migrated는 아직 빈 배열이라 세 도메인 파일은 여전히 루트에 남아 있다. 이 PR이 migratedservice/update/windows를 넣고, tests/test-layout.test.ts가 이후 루트 잔존 파일을 거절하게 만든다. #3497 전체(나머지 도메인 이동 + 원래 목표의 일부)를 끝내는 것은 아니고, 첫 칸만 채운다.

자동으로 같이 따라가는 곳도 있다. scripts/test.tsSERIAL_FULL_SUITE_FILES에서 update-stop-first.test.tsupdate/update-stop-first.test.ts로 바뀌는데, 이건 move.ts가 serial lane을 고치도록 이미 설계된 동작이다(basename으로 ignore/timeout을 잡고, ./tests/${file}로 실제 경로를 연다). 주석으로만 옛 경로를 가리키던 tests/codex-catalog-writer.test.ts · tests/native-main-claim.test.ts · tests/grok-lifecycle.test.ts 세 곳과 docs/superpowers/plans/2026-07-26-oauth-reliability-integrity.md도 새 경로로 맞춰졌다.

한 손으로 고친 부분은 로컬 repoRoot/import.meta.dir + "/.."를 쓰던 파일들이다. mover는 그걸 덮어쓰지 않기로 되어 있어서, 해당 파일들이 tests/helpers/repo-root.tsrepoRoot/repoPath/helperPath로 바뀌었다. 한 단계 깊어진 ../helpers../../src 패턴은 도메인 한 칸 이동과 맞다.

도구 쪽 조정 하나: verify.tsSWEEP_ROOTS에서 devlog/_plan을 포함해 devlog 전체를 빼 버렸다. 이유 설명은 타당하다(첫 슬라이스에서 옛 plan 문서 59개가 같이 바뀌면 진짜 diff가 묻힌다). 대신 앞으로 open plan 문서 안의 tests/<basename> 문자열은 STALE 검사에 안 잡힌다. docs/ 아래 문서는 여전히 sweep 대상이라 oauth plan은 정상적으로 고쳤다. 로컬 검증 요약(도메인 929 pass · layout/관련 214 pass · tsc · privacy:scan)은 PR 본문에 적혀 있고, 정확한 증명은 exact-head ci(Linux shard + macOS 1/2·2/2)다. 이 글을 쓰는 시점에는 일부 테스트 shard와 macOS가 아직 pending이다.

types.ts/config.ts 대규모 분할 캠페인과는 무관하다. 런타임 src/를 안 건드리는 테스트 레이아웃 PR이라 close-don't-rebase 대상이 아니다. 중복 PR로 보이지도 않는다.

scripts/test-layout/verify.ts SWEEP_ROOTS - devlog/_plan까지 통째로 빠져서, 열려 있는 plan 문서에 남은 tests/<옛파일> 경로는 이후 슬라이스의 STALE 검사에 안 걸린다. 의도된 타협이지만, 에이전트/사람이 plan을 그대로 따라가면 틀린 경로로 bun test를 칠 수 있다.
CI (macos 1/2 · macos 2/2 · test 2/4·3/4·4/4 등) - 본문이 exact-head ci를 증명으로 적었는데, 리뷰 시점에는 해당 job들이 아직 pending/진행 중이다. discovery·batch·isolated job이 이동 경로를 찾는지가 이 슬라이스의 진짜 게이트다.
이슈 #3497 - 이 PR만으로는 닫히지 않는다. macOS shard(#3501)와 도구(#3500)는 이미 dev에 있고, 남은 도메인 이동이 이슈에 그대로 남는다. 랜딩 후 이슈를 조기 닫지 말 것.
docs/superpowers/plans/2026-07-26-oauth-reliability-integrity.md - 한 파일만 docs sweep으로 갱신됐다. 문제는 아니고, devlog/를 뺀 정책과 대비되는 ‘살아 있는 문서만 고친다’는 경계를 보여 주는 사례다.
tests/windows/windows-tray.test.ts (파일 하단 import) - helperPath/repoPath/repoRoot import가 describe 블록 아래에 붙어 있다. 원래 ManagementRequest import가 아래에 있던 패턴을 이은 것이라 신규 버그는 아니지만, 나중에 읽는 사람 기준으로는 파일 상단으로 올리는 편이 더 읽기 쉽다(필수 수정은 아님).

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

  • devlog/를 sweep에서 영구히 뺄지, 아니면 _fin만 빼고 open _plan은 다시 넣을지 (문서 소음 vs plan 경로 정확도).
  • 첫 슬라이스를 CI 완전 그린 뒤에만 머지할지, 로컬 929+214와 gates 통과만으로 先行 머지할지.
  • 다음 도메인 슬라이스 순서(providers / server / responses 등)를 #3497에 미리 박아 둘지, 이번 슬라이스 랜딩 후 도구 피드백을 보고 정할지.

너의 추천

exact-head ci(특히 Linux shard와 macOS 1/2·2/2)가 초록이 되는 것을 확인한 뒤 merge 한다. #3497은 열린 채로 두고, 랜딩 코멘트에 ‘slice1 windows/service/update via #3509’만 남긴다. devlog/ sweep 제외는 이번 슬라이스 기준으로 수용하고, 다음 슬라이스부터 plan 경로가 헷갈리면 _plan만 다시 넣는 follow-up을 고려한다. 코드 추가 수정 요구는 없다.

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

@lidge-jun

Copy link
Copy Markdown
Owner Author

Exact-head CI note (run 33909679589 on 23ca0f2): test 2/4 failed on tests/update/update-stop-first.test.ts > "npm launcher restarts the stopped runtime after a staged update failure" (waitForProxy false at 91.6 s). This is the launcher-recovery flake already root-caused as runner load in devlog/_plan/260827_dev_hardening/060_wp8_launcher_flake.md and seen on PRs #2801 and #2724 and on dev itself; the file passes locally in 3 s at its new path, the batch that ran it lists the moved path correctly, and the failing assertion reads a detached proxy's /healthz, which no path change reaches. Rerunning the failed job once on the same SHA for a clean aggregate; not editing the test.

@Ingwannu Ingwannu left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved on exact head 23ca0f2db. I rechecked the complete migration diff: the test changes are path/import and repository-root reference updates, the layout manifest records only the three intended domains, and no unresolved review threads remain. The failed test 2/4 job was rerun on the unchanged head and the aggregate CI is now fully green; the moved update-stop-first path was discovered and executed correctly.

@lidge-jun
lidge-jun merged commit 8f02f24 into dev Sep 4, 2026
44 of 46 checks passed
@lidge-jun
lidge-jun deleted the codex/layout-slice1-windows-service-update branch September 4, 2026 19:29
agentHits pushed a commit to agentHits/opencodex that referenced this pull request Sep 17, 2026
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