test(clients): assert the Pi override with join, not a POSIX separator

[ntdatt812]

Refs #2152.

What this fixes

tests/management-client-config-route.test.ts has one case that cannot pass on Windows:

(fail) GET /api/client-config > an accepted override still resolves through the route
Expected: "/tmp/opencodex-pi-route-fixture/models.json"
Received: "\tmp\opencodex-pi-route-fixture\models.json"

piConfigPath builds the destination with join(piAgentDir(env, home), "models.json"), and join emits \ on win32. The expectation was a hard-coded POSIX string, so the case asserted the host's path separator rather than the thing it was written to prove — that PI_CODING_AGENT_DIR took effect instead of falling back to ~/.pi/agent.

The production behaviour is right; only the assertion was platform-bound.

The change

The expectation is now built with join from a single binding shared with the env value, so the override and the assertion cannot drift apart:

const overrideDir = "/tmp/opencodex-pi-route-fixture";
process.env.PI_CODING_AGENT_DIR = overrideDir;
...
expect(body.destination).toBe(join(overrideDir, "models.json"));

join is identity for this input on POSIX, so the Linux and macOS legs see the same string they see today:

$ node -e "const p=require('node:path'); console.log(p.win32.join('/tmp/opencodex-pi-route-fixture','models.json')); console.log(p.posix.join('/tmp/opencodex-pi-route-fixture','models.json'))"
\tmp\opencodex-pi-route-fixture\models.json
/tmp/opencodex-pi-route-fixture/models.json

I used join rather than calling piConfigPath in the test. Deriving the expectation from the same resolver that produced the value would have made the case pass no matter what the resolver did.

Verification — Windows 11, Bun 1.3.14

tests/management-client-config-route.test.ts        14 pass, 0 fail   (13 pass / 1 fail before)
+ client-config-export, client-config-export-new-clients,
  client-config-new-clients                        111 pass, 0 fail
bun x tsc --noEmit -p tsconfig.json                  0 errors
bun run privacy:scan                                 Privacy scan passed

Mutation-checked. Making piAgentDir ignore PI_CODING_AGENT_DIR entirely, keeping the new assertion:

(fail) a refused path override answers 400 with the bounded message, not a thrown 500
(fail) an accepted override still resolves through the route
(fail) a refused override wins over a failing catalog, and skips the catalog work
 11 pass | 3 fail

This case is among them, so it still pins the override taking effect rather than merely matching whatever the resolver returns.

How I found it

Running the full local suite on Windows: 14029 tests, this was the only failure. It is not in the four remaining failures listed on #2152, so it is an additional one in the same platform class rather than a duplicate of any of them.

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.

Summary by CodeRabbit

• Tests
  • Improved cross-platform test coverage for custom configuration directory path resolution.
  • Updated expected paths to match platform-specific path formatting.

[github-actions]

✅ Deterministic PR hygiene checks passed.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 1261e9f5-ab9f-48d7-b628-dfddf5f7c25b

📥 Commits

Reviewing files that changed from the base of the PR and between 8575838 and 4fbfb27.

📒 Files selected for processing (1)

• tests/management-client-config-route.test.ts

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

---

📝 Walkthrough

Walkthrough

The route test now derives the expected models.json path with node:path.join, matching platform-specific path separators. No production code changes.

Changes

Route test path expectation

Layer / File(s)	Summary
Platform-aware expected destination tests/management-client-config-route.test.ts	The test imports join, stores the override directory in overrideDir, and computes the expected models.json path with join(overrideDir, "models.json").

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 4fbfb

This change makes the route test platform-neutral without changing production behavior, and the affected Windows test suite passes. No actionable merge-blocking risk remains beyond normal checks.

Suggested reviewers: lidge-jun

🚥 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 1 files.
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 clearly and concisely describes the test change from a hard-coded POSIX separator to join for the Pi override path.

✨ Finishing Touches

🧪 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

• X
• Mastodon
• Reddit
• LinkedIn

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

[github-actions]

✅ READY

• all PR quality gates passed; the review readiness checklist is complete.

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.

✅ 4/4 boxes ticked.

This pull request is already Ready for Review.
The review-ready label marks this PR as ready; review automation runs independently.
Maintainers: @lidge-jun @Ingwannu

[Ingwannu]

Reviewed exact head e580e7e0001b5451c91b578e670c7940765d3fc6. The test-only change correctly derives the expected Pi models.json destination with the same platform-aware node:path.join contract as production, while retaining a non-tautological assertion that the override was honored.

Independent validation passed: bun test ./tests/management-client-config-route.test.ts (14/14) and repository typecheck. I found no code blocker. The PR is still draft with the readiness checklist at 0/4 and is now one integration commit behind dev; finish the checklist, refresh onto current dev, and let exact-head CI run before merge. This change has no Go-runtime counterpart.

[lidge-jun]

리뷰 · 우선순위 29 / 80

프로덕션은 이미 맞음. 지금 dev HEAD 7881319e7 src/clients/config-export.ts:206-215 piAgentDir / piConfigPath가 join(piAgentDir(...), "models.json")임. win32에선 \\ 임. 깨진 건 테스트 한 줄임. tests/management-client-config-route.test.ts:333-340이 destination을 POSIX 문자열 "/tmp/opencodex-pi-route-fixture/models.json"이랑 비교함. Windows에서 \\tmp\\opencodex-pi-route-fixture\\models.json이 나와서 기대가 틀림. #2152 본문 4개 실패 목록에 없는 추가 한 장임. 그 네 개 중복으로 닫지 말 것. Refs #2152는 플랫폼 클래스 표시임.

이 PR은 오버라이드 디렉터리 하나를 바인딩하고 기대를 join(overrideDir, "models.json")으로 만듦. POSIX에선 join이 아이덴티티라 리눅스/맥 기대 문자열이 오늘이랑 같음. piConfigPath를 테스트에서 다시 호출하지 않음. 리졸버가 뭘 반환하든 통과하는 순환 어서션을 피한 거임. ㅇㅇ 그거 맞음. 뮤테이션도 봄. piAgentDir가 PI_CODING_AGENT_DIR을 무시하면 이 케이스가 다시 실패함. 오버라이드가 먹는다는 걸 잠금.

draft=false. 체크리스트 4/4. 파일 하나. +8/-2. Windows 11 + Bun 1.3.14에서 해당 파일 14 pass. 로컬 풀수트 14029 중 이 한 장이 추가 실패였음. types.ts/config.ts 안 만짐. 스플릿 안 씹힘. 리베이스하지 말고 닫으라는 케이스 아님.

#2188 사이드카 이미 dev. x_search 안 넣음. Grok/Responses 레인 아님. 프리뷰 배포 아님. 2.28 블로커 아님. 핫 크래시 아님. Windows CI 샤드 한 장이 플랫폼 구분자 때문에 빨갱이 되는 거임. 그래서 29.

상대 경로 거절 케이스(:314-330, :347)는 이미 PI_CODING_AGENT_DIR 에러 문구를 봄. 이번 장은 accepted override만 고침. 다른 클라(MCode/ZCode/OMP) 테스트를 POSIX로 박아 둔 게 더 있는지는 이 PR 범위 밖임. 있으면 #2152 후속으로 따로 열어라.

해결방안: 머지해라. 프로덕션 경로 손대지 말 것. 기대를 join으로 두는 거 유지. piConfigPath를 기대에 쓰지 말 것. #2152를 이 한 장으로 닫지 말 것. 스플릿이 클라이언트 익스포트 테스트를 옮기면 그때는 리베이스하지 말고 닫고 다시 짜라. 지금은 테스트 한 파일이라 그 정도 아님.

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

[Ingwannu]

Approved exact head 4fbfb27d1da1e18d09cf3b8c521d0b2ec771f3cc.

I independently verified the owner/Grok recommendation. The change fixes only the Windows-specific test expectation by deriving the expected Pi destination with node:path.join; it does not call the production resolver from the assertion, so the override behavior remains genuinely tested rather than tautological. Production code is unchanged.

Local exact-head validation passed: 15/15 focused client-config route tests and repository typecheck. Cross-platform CI and React Doctor are green on this SHA. This test-only change has no Go-runtime counterpart.

[ntdatt812]

Heads-up: this one is merged here but is not on dev any more, so the failure it fixed is back.

$ git fetch origin && git log --oneline -1 origin/dev
7881319e7 Merge pull request #2283 from lidge-jun/codex/backfill-xai-web-search-capability

$ git merge-base --is-ancestor 34f18c1e7 origin/dev ; echo $?
1                       # 34f18c1e7 is this PR's merge commit

tests/management-client-config-route.test.ts:335 is back to the hard-coded string, and the case is red again on Windows:

(fail) GET /api/client-config > an accepted override still resolves through the route
  Expected: "/tmp/opencodex-pi-route-fixture/models.json"
  Received: "\tmp\opencodex-pi-route-fixture\models.json"

I checked the rest before saying anything — it looks like a one-off rather than something systematic. Every other merge of mine is still reachable from dev:

#1780 #1788 #1805 #1806 #2042 #2059 #2085 #2086 #2129 #2167 #2265   ON-DEV
#2272                                                              MISSING

There is no revert commit for it, and #2265 merged the same morning is intact, so this reads as a force-push that dropped the merge rather than a decision to back it out.

Nothing needed from you if you would rather restore it yourself. Otherwise I have the same eight lines ready to re-send against current dev — say the word and I will open it.