Skip to content

fix(clients): compare Aside file identities without precision loss - #3842

Merged
lidge-jun merged 2 commits into
devfrom
codex/axis1-exact-file-identity
Sep 7, 2026
Merged

fix(clients): compare Aside file identities without precision loss#3842
lidge-jun merged 2 commits into
devfrom
codex/axis1-exact-file-identity

Conversation

@lidge-jun

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

Copy link
Copy Markdown
Owner

Summary

Use exact BigInt file identities in the private Aside profile boundary guard. Legal distinct 64-bit IDs can collapse to the same JavaScript Number, causing false alias detection or loss of a captured-directory identity distinction. Path, symlink, hardlink, manifest-size and directory-replacement checks retain their existing semantics; public IO contracts and serialized records are unchanged.

Supporting final layer of the manual axis1 delivery chain: #3825 quota → #3826 version guidance → #3827 recovery reasons → this exact-identity fix. This is a delivery order, not a new runtime dependency. No native GitHub stack.

Verification

  • Independent Astra high plan and source/security audits passed.
  • Semantic regression exercises distinct high IDs whose Number representations collide, captured-directory replacement, native BigInt API support, and real hardlink/symlink refusals. Controlled-stat read assertions prevent a missed spy binding from passing vacuously; spies restore in finally.
  • Existing Aside negative tests remain unchanged; both test-layout maps include the new file.
  • git diff --check and combined privacy scan passed. No local application suite/typecheck was run; pushes use --no-verify per owner instruction.
  • Final combined CI 34074350604 validates all four layers with dev bf85e67 at 9470fdb. Service lifecycle 34074351720 passed all three platforms at the same head. Final tree: 90a75118402d2f310393bef9ac3e4668cfcbdcfa. Lower CI remains deferred per owner instruction.
  • Repeated Windows profile failures motivated this support work, but their actual IDs were not logged. This patch corrects independently demonstrated precision loss; it does not claim those incident IDs or every failure cause were measured.

Final combined CI34074350604 passed all26 jobs and same-head service lifecycle34074351720 passed all3 jobs at9470fdb1. The controlled high-ID regression and native guards passed. This does not retroactively identify the raw IDs from earlier failed runs. Owner authorized admin merge with original commits preserved.

Checklist

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

t added 2 commits September 7, 2026 09:44
Distinguish HTTP refusal, timeout, shared abort, transport failure and invalid output through the existing shared-flight cache. Preserve strict admission, success-only cache, wrapper APIs and caller-local cancellation. Addresses the diagnostic residual of #3661; multipart/retry policy stays deferred.

Reported-by: Hu9956 <282876394+Hu9956@users.noreply.github.com>
Use private BigInt stat identities so distinct legal 64-bit file IDs are not collapsed by Number conversion. Preserve all path/link/hardlink and directory replacement checks, manifest bounds, public IO types and serialization. Add semantic high-ID and native link controls with fixture-reachability checks. No local suite was run; the earlier Windows incident IDs remain unmeasured.
@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (2)
  • ^dev$
  • ^preview$

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Team

Run ID: f4aad0c7-c76b-4b40-99f5-83b6b0ce08b8

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

@lidge-jun

Copy link
Copy Markdown
Owner Author

리뷰 · 우선순위 69 / 80

이 PR은 Aside 프로필 경계 검사에서 파일 신원(dev/ino)을 JavaScript Number로 다루던 것을 BigInt로 바꿔, 서로 다른 합법적인 64비트 inode가 같은 숫자로 뭉개지는 구멍을 막는 수정입니다. 지금 dev(HEAD bf85e6754, 2.46.0)의 src/clients/aside-profiles.ts를 보면 inspect가 기본 statSync/lstatSync를 쓰고, DirectoryIdentitysameIdentitynumber 신원으로 비교하며, leaf.nlink > 1도 숫자 비교입니다. Number의 정확한 정수 범위는 2^53까지라, 그 위에서 서로 다른 inode가 Number(a) === Number(b)로 같게 보이면 (1) 다른 계정 디렉터리를 같은 대상으로 오인하거나 (2) 캡처해 둔 디렉터리 신원이 바뀌었는데도 같다고 판단할 수 있습니다. Windows에서 반복된 프로필 실패가 계기였다고 본문에 적혀 있지만, 그때의 실제 ID는 로그에 없어서 **이 PR은 “측정된 사고 원인”이 아니라 “독립적으로 재현된 정밀도 손실”**을 고친다고 스스로 선을 그어 두었습니다. 그 서술은 정직하고 좋습니다.

변경 자체는 작습니다. inspect{ bigint: true }를 넣고 반환 타입을 BigIntStats로 바꾸며, MAX_MANIFEST_BYTESnlink 비교도 bigint 리터럴로 맞춥니다. 공개 IO 계약·직렬화·경로/심볼릭/하드링크/디렉터리 교체 거절 의미는 그대로입니다. 새 테스트 tests/clients/aside-profile-identity.test.ts는 (a) Number로는 충돌하지만 BigInt로는 다른 두 inode를 심어 경계가 오인하지 않는지, (b) 캡처 후 detectDir 신원이 바뀌면 거절하는지, (c) 실제 hardlink/symlink는 계속 거절하는지, (d) 스파이 바인딩이 빠지면 그냥 통과하지 않도록 경계 호출 중에만 읽기를 기록하는지를 잠급니다. layout 맵에도 새 파일이 들어가 있습니다. types/config 분할과는 무관합니다.

다만 이 PR은 단독 dev 착지물이 아닙니다. base가 codex/axis1-recovery-reasons이고, 작성자가 적은 수동 axis1 배달 순서(#3825 quota → #3826 version guidance → #3827 recovery reasons → 이번 exact-identity)의 마지막 층입니다. 지금 #3825/#3826/#3827도 아직 열려 있고, 비교상 그 축 브랜치는 dev보다 앞서면서도 뒤로 많이 벌어진 상태입니다. 게다가 이 PR은 draft이며, 로컬 스위트/typecheck는 돌리지 않았고 --no-verify push라고 본문에 명시되어 있습니다. 결합 CI 링크는 있지만 lower CI는 owner 지시로 미뤄 둔 상태입니다. 코드 방향은 맞고 회귀도 잘 짜였지만, 머지 타이밍은 스택과 draft 해제에 묶입니다.

라인 src/clients/aside-profiles.ts · inspect - { bigint: true }로 신원만 정확히 읽고, 공개 API 타입은 건드리지 않습니다. 의도한 최소 변경입니다.
라인 MAX_MANIFEST_BYTES = 4n * 1024n * 1024n - manifestStat.size(bigint)와 비교하기 위한 맞춤입니다. 동작은 맞습니다.
라인 leaf.nlink > 1n - BigIntStats에서 nlink가 bigint라 필수입니다. hardlink 거절 의미는 그대로입니다.
경로 tests/clients/aside-profile-identity.test.ts - FIRST_INODE = 2n ** 53n+1n으로 Number 충돌을 직접 증명하고, observingBoundary로 스파이 miss를 막는 설계가 좋습니다. 이 테스트만으로도 패치 가치가 설명됩니다.
경로 base codex/axis1-recovery-reasons - 현재 dev tip(bf85e6754)과 직접 비교 대상이 아닙니다. #3825→#3827이 dev에 먼저(또는 함께) 착지해야 이 층이 의미가 있습니다.
경로 draft / CI - draft라 CodeRabbit도 skip이고, label/resolve 등은 아직 불완전해 보입니다. ready로 올리기 전에 스택 앞단과 CI 정책을 맞출 필요가 있습니다.
경로 Windows 사고 - 본문이 “그 사고의 inode를 측정하진 않았다”고 했으므로, 이 PR만으로 Windows 프로필 실패 이슈를 닫으면 안 됩니다. 별도 관측/후속이 남습니다.

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

  • axis1 네 층을 한 열차로 dev에 올릴지, #3842만 따로 dev에 리타깃해서 먼저 넣을지(코드 의존은 약하고 배달 순서만 있다고 본문에 적힘)
  • draft를 언제 ready로 올릴지, lower CI를 이 층에서 돌릴지
  • Windows 프로필 실패를 이 PR과 분리된 이슈로 남길지(원인 미측정이므로 권장)

너의 추천
코드는 머지 가치가 있습니다. 다만 지금 당장 dev에 단독 머지하지는 말고, #3825#3827 스택 착지 계획에 맞춰 draft를 ready로 올린 뒤 게이트를 초록으로 만든 다음 착지하세요. 만약 axis1 앞단이 늦어지면, 본문대로 런타임 의존이 없으니 이 커밋만 dev로 리타깃해 독립 착지하는 편이 더 안전합니다. Windows 실패 이슈는 이 PR로 닫지 마세요. types/config 분할 때문에 닫을 필요도 없습니다.

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

@lidge-jun
lidge-jun marked this pull request as ready for review September 7, 2026 02:13
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner September 7, 2026 02:13
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 7, 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-07T02:16:27.256224Z b29bbb4 Draft marked ready
ℹ️ 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.

@lidge-jun
lidge-jun changed the base branch from codex/axis1-recovery-reasons to dev September 7, 2026 02:19
@lidge-jun
lidge-jun merged commit 5fdf9bb into dev Sep 7, 2026
19 checks passed
@lidge-jun
lidge-jun deleted the codex/axis1-exact-file-identity branch September 7, 2026 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant