Skip to content

fix: carry remaining #6723 change (picker CA persistence + log admission test) - #6847

Merged
lidge-jun merged 2 commits into
devfrom
cursor/carry-6723-remainder-20bc
Oct 9, 2026
Merged

lidge-jun merged 2 commits into
devfrom
cursor/carry-6723-remainder-20bc

Conversation

@lidge-jun

@lidge-jun lidge-jun commented Oct 9, 2026 •

Copy link
Copy Markdown
Owner

Carries the remaining commit from #6723 (rest landed via #6796/#6797, CI part in #6806).

Summary

Verification

  • bun test tests/server/server-management-log-admission.test.ts tests/server/server-new-model-policy-arrival.test.ts tests/server/server-management-auth.test.ts — 53 pass, 0 fail (Bun 1.4.0, the pinned test runner).
  • bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts — 19 pass, 0 fail.
  • bun run typecheck — pass (Bun 1.4.2).
  • bun run structure:check — pass.
  • bun run privacy:scan — pass.
  • bun scripts/file-size-ratchet.ts — pass after the layout line fold.
  • Full bun run test was not run locally. On head 490bbab24, Cross-platform CI passed, including all four test shards, all nine Windows shards, gates, structure gate, docker smoke, storage policy, and api usage.
  • The enforce-target job on that head was cancelled before it started: two pull_request_target events landed in the same second and the concurrency group pr-gate-comment-6847 dropped both. This description edit is the revalidation.

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.
Open in Web Open in Cursor 

Summary by CodeRabbit

  • Tests
    • Added isolated coverage for log-management admission, including authentication, origin checks, malformed cursors, and unavailable management authority.
    • Updated test grouping and ensured asynchronous cleanup completes before temporary test files are removed.
  • Documentation
    • Clarified that picker CA persistence compares device and inode identifiers using full-precision values.

39663ff's BigInt picker identity check is already on dev via #6772, so this
keeps that implementation. The rest of the commit that still applies moves the
log-cursor cases into a registered sibling under the current fixture drain,
waits for ACL work before model-arrival fixture removal, and records the
bigint file-id comparison in the Claude Desktop contract. The diagnostic-crate
hunks stay with #6806.

Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com>
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: dc700dfc-e0a1-42a6-8231-13ce9725122b

📥 Commits

Reviewing files that changed from the base of the PR and between d17a9f2 and 490bbab.


📒 Files selected for processing (6)
  • scripts/test-layout/layout.json
  • structure/clients/claude-desktop.md
  • tests/fixtures/test-layout-expected.json
  • tests/server/server-management-auth.test.ts
  • tests/server/server-management-log-admission.test.ts
  • tests/server/server-new-model-policy-arrival.test.ts

💤 Files with no reviewable changes (1)
  • tests/server/server-management-auth.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.



📝 Walkthrough

Walkthrough

The pull request adds dedicated /api/logs admission tests and updates their server test-layout mappings. It also extends test teardown cleanup and clarifies picker CA device and inode identity comparisons in the documentation.

Changes

Log Admission Tests

Layer / File(s) Summary
Isolated log-admission coverage
tests/server/server-management-log-admission.test.ts, tests/server/server-management-auth.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json
The dedicated test file checks authentication and origin admission, cursor responses, and unavailable management authority. It isolates test setup and cleanup. The corresponding tests are removed from the authentication test file, and both test-layout mappings include the new test. (New test: lines 1–112; removed tests: pre-image lines 1838–1896.)

Test Cleanup

Layer / File(s) Summary
Await cleanup helpers
tests/server/server-new-model-policy-arrival.test.ts
The test imports cleanup helpers for config-directory hardening, native main startup releases, and Windows secret ACL reaps. Teardown awaits these helpers before removing the temporary home. (Lines 4–6, 34–36.)

Picker CA Documentation

Layer / File(s) Summary
Document bigint identity comparisons
structure/clients/claude-desktop.md
The picker CA persistence documentation specifies bigint device and inode comparisons to avoid numeric-rounding aliasing of distinct full-width file IDs. (Lines 253–254.)

Priority: ⬇️ Low

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

Change: Other


Merge Risk: ⚪ Minimal · up to 490bb

This PR moves log-admission tests, extends test cleanup, and clarifies documentation. It does not change production behavior, and no merge-blocking risk was found.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly identifies the two main changes: picker CA persistence documentation and log-admission test updates. It is specific and related to the pull request.
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.


✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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 added the bug Something isn't working label Oct 9, 2026
The new log-admission entry shares a line with the management-auth entry so
scripts/test-layout/layout.json stays at 1999 lines.

Co-authored-by: mashfromband <matsumoto.yukuhashi@gmail.com>
@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

@lidge-jun
lidge-jun marked this pull request as ready for review October 9, 2026 20:58
@lidge-jun
lidge-jun requested a review from Ingwannu as a code owner October 9, 2026 20:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 9, 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-10-09T21:00:11.031618Z 490bbab 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 merged commit f091942 into dev Oct 9, 2026
45 of 52 checks passed
@lidge-jun
lidge-jun deleted the cursor/carry-6723-remainder-20bc branch October 9, 2026 21:06
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.

2 participants