Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThree test changes adjust a non-Windows executable fixture, increase a compaction-routing test timeout, and update service-state path assertions. ChangesRemote workspace command runner test
Compaction routing test
Service claim test
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Other Merge Risk: 🟡 Moderate · up to The changes are test-only, but the required cross-platform run for this head has not succeeded, leaving platform-specific regressions unchecked. Complete the run before merging. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
|
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. This PR stays in draft until every box above is ticked. |
d9a37ce to
46418c9
Compare
리뷰 · 우선순위 34 / 80이 풀 리퀘스트는 테스트 세 곳만 고칩니다. 제품이 사용자 요청을 처리하는 방식은 그대로입니다. 테스트를 따로 돌릴 때 괜찮았는데도 실패로 보이던 자리를 막으려는 수정입니다. 기준 브랜치는 dev이고, dev보다 커밋 1개 뒤, 1개 앞입니다. 압축 결과를 다시 재생하는 테스트가 Bun의 기본 제한 5초를 넘겼습니다. 작성자가 본 시간은 5.37초입니다. 시간이 넘어가면 같은 파일의 다음 검사가 401처럼 잘못 보입니다. 그 테스트에만 20초를 줍니다. 서비스 소유권 테스트는 상태 파일 경로가 모두 이번 임시 폴더 안에 있는지 확인했습니다. 테스트 모음 실행기는 일부러 따로 둔 HOME을 씁니다. 예전 기본 집 경로는 그 임시 폴더 밖에 있어서 검사가 깨졌습니다. 이제는 지금 쓰는 경로가 임시 폴더로 시작하는지만 보고, 그 경로가 목록에 들어 있는지만 확인합니다. 원격 작업 공간 테스트는 bubblewrap을 실행하지 않습니다. 만들어질 명령 인자만 봅니다. Bun이 /tmp 아래에 있으면, 그 옆에 만든 가짜 실행 파일은 상위 폴더를 아무나 쓸 수 있어서 신뢰 검사에서 먼저 탈락합니다. 유닉스에서는 시스템 프로그램 /usr/bin/env를 대신 씁니다. 윈도우는 예전처럼 가짜 파일을 만듭니다. 라인 - tests/service/service-claim.test.ts 의 새 검사는 전보다 헐겁습니다. 이 테스트는 기록 함수를 가짜로 바꾸지 않아서, 실제 기록이 serviceStatePaths()에 있는 경로를 쓸 수 있습니다. 예전 검사는 그 목록이 전부 이번 임시 폴더 안인지 확인했습니다. 새 검사는 기본 경로 하나만 확인합니다. 둘째 줄은 목록에 기본 경로가 있는지만 보는데, 기본 경로는 원래 그 목록에 들어갑니다. 임시 폴더 밖 경로가 남아도 이 테스트는 통과합니다. 라인 - tests/clients/remote-workspace-command-runner.test.ts 의 /usr/bin/env는 같은 파일이 다른 이름으로 묶여 있으면 안 됩니다. 제품 함수 trustedBubblewrap은 링크 개수(nlink)가 1이 아니면 거절합니다. 이 리눅스 머신에서는 /usr/bin/env가 일반 파일이고 링크 개수가 1이라 통과합니다. env가 busybox 하나로 묶인 이미지에서는 인자 검사 전에 다시 실패합니다. 라인 - 같은 압축 테스트의 20초는 관측된 5.37초보다 넓습니다. 테스트가 정말 느려져도 20초가 되기 전에는 실패로 나오지 않습니다. 메인테이너의 판단이 필요한 지점 위생 봇 댓글은 #5743 작성자를 Co-authored-by로 적으라고 남아 있습니다. 최신 hygiene 체크는 통과입니다. 바뀐 파일은 테스트뿐이고, 본문은 에이전트 메시지 복구가 이미 dev에 있다고 합니다. 그 트레일러가 아직 필요한지, 댓글이 이전 실행의 결과인지를 정해야 합니다. serviceStatePaths()가 모음 실행기의 HOME에 있는 예전 경로를 남기는 것이 맞다면, 그 경로가 개발자의 진짜 집(~/.opencodex)은 아니라는 확인을 이 테스트에 남길지 정해야 합니다. 지금 수정은 그 확인이 빠져 있습니다. 이 풀 리퀘스트는 드래프트입니다. 준비 체크리스트 4칸은 모두 비어 있습니다. 작성자도 이 커밋의 여러 운영체제 CI가 끝나기 전에는 합치지 말라고 적었습니다. 너의 추천 실패 원인과 테스트 수정은 서로 맞습니다. 서비스 클레임 테스트에는 "목록의 경로가 이번 임시 폴더 안이거나, 가드가 허용한 모음용 격리 집이다"라는 한 줄을 다시 넣는 쪽을 권합니다. /usr/bin/env는 리눅스 CI에서 링크 개수가 1인지 확인하고, 1이 아니면 상위 폴더를 아무나 쓸 수 없는 다른 시스템 파일을 고르면 됩니다. 20초는 같은 파일의 거짓 401을 막는 용도로 받아도 됩니다. 체크리스트를 채우고 CI가 통과한 뒤에 합치면 됩니다. 이 댓글은 grok-bot이 작성했습니다 |
46418c9 to
d77851e
Compare
|
@lidge-jun Thanks for the review. I tightened the service-state assertion to accept only the fixture path or the exact legacy path when the test-home guard says it is safe, and the Unix argv fixture now selects a single-link system executable rather than assuming |
…et replacement, test stability) (#5918) * fix(adapters): remint duplicate tool call ids on the openai-chat lane (#5914) Carried from #5914 as one squashed commit. Co-authored-by: moseoridev <sjssjs1344@gmail.com> * fix(chat-native): refetch on zero-output mid-stream socket reset (#5882) Carried from #5882 as one squashed commit. Co-authored-by: Yum-wu <1172989563@qq.com> * test: stabilize full-suite isolation and integration budgets (#5849) Carried from #5849 as one squashed commit. The tests/service/service-claim.test.ts hunk is dropped: dev already sandboxes that case with a homedir spy and a stricter assertion. Co-authored-by: Zhaofeng Li <lzfxxx@gmail.com> * test(chat-native): prove the replacement request copy is released mid-relay The #5882 regression streamed a 64 KiB delta against a 32 MiB turn budget, so it passed with or without the release. The new case rotates the key between sends, holds the replacement body after its first frame, and reads the live translator charge while the stream is relayed: 1x the request size with the release, 2x without it (verified red by reverting 318520e). Also drops a trailing blank line in src/lib/upstream-retry.ts and adds the batch plan. * fix(upstream-retry): require the resend gate on the zero-output wrapper wrapWithZeroOutputRefetch forwarded its options object, so the source guard that proves every post-header replacement is authorized could not see a gate at the new call. The wrapper now requires authorize in its type and passes it explicitly, and the guard scans wrapWithZeroOutputRefetch call sites as well. Fixes the red tests/lib/ambiguous-resend-composition.test.ts on test 1/4. --------- Co-authored-by: moseoridev <sjssjs1344@gmail.com> Co-authored-by: Yum-wu <1172989563@qq.com> Co-authored-by: Zhaofeng Li <lzfxxx@gmail.com>
|
Thanks! This landed on |
Summary
/usr/bin/envis one.Only test code and the test runner's lane list change. Agent-message recovery is already on
dev.Verification
866406bc8(rebased onto82cb66e82): Bun 1.4.0, Node 22, macOSbun run test: 31,848 pass, 60 skip, 0 fail, exit 0. Parallel lane: 31,216 pass / 37 skip; all fresh-process lanes passed. This result is from the final code after the review fixes.bun run typecheck,bun run privacy:scan,git diff --check upstream/dev..HEAD: pass.action_requiredon this head and have not run to completion; a maintainer must approve the fork workflows. Keep this PR in draft and do not merge based only on local macOS evidence.Review readiness checklist
Required local validation passed; commands, results, and any full-suite exception are documented.
I pushed my PR to a recent dev commit (at most 10 behind; a maintainer may still ask for the exact tip before merge).
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.