Skip to content

fix(server): standalone CLI builds again with Cursor usage - #71

Merged
kalvenschraut merged 1 commit into
rtvisionfrom
fix/cli-exe-keyring-require
Sep 25, 2026
Merged

kalvenschraut merged 1 commit into
rtvisionfrom
fix/cli-exe-keyring-require

Conversation

@kalvenschraut

@kalvenschraut kalvenschraut commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

The RTVision 0.0.63 release run failed in linux-cli: build-exe rejected dist-exe/bin.mjs because it imports @napi-rs/keyring. Upstream's Cursor usage history (pingdotgg#10409) added a dynamic import("@napi-rs/keyring") in cursorCredentialStore.ts, and a Node single executable can only import built-ins.

This loads the package through createRequire, the same way NodePtyAdapter.ts and WorkspaceSearchIndex.ts load their native packages. Upstream main has the same import, so its next standalone build will hit this too.

Validation

  • Server typecheck passes. cursorCredentialStore.test.ts and src/usage pass (109 tests).
  • vp pack of the server bundle followed by findEsmImportsOfExternalPackages (the build-exe check) finds no file-backed imports. Exe mode uses the same externals. This host (musl, Node 24) cannot run the SEA step itself.

Failed run: https://github.com/RTVision/t3code/actions/runs/36180001882

Written by Claude Opus 5.5 through Claude Code in T3 Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • Improved loading of macOS Cursor credentials from the system keyring.

Upstream's Cursor usage reader (pingdotgg#10409) loads @napi-rs/keyring with a
dynamic import(). A Node single executable can only import built-ins, so
the release's build-exe check rejects the bundle. Load it through
createRequire like node-pty and fff-node.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XS labels Sep 25, 2026
@github-actions

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 13.5 KiB 13.5 KiB −74 B (−0.5%) 15.1 KiB ✅
Codex Thread snapshot wire 7.1 KiB 7.1 KiB +3 B (+0.0%) 7.3 KiB ✅
Codex Live turn WebSocket wire 6.5 KiB 6.4 KiB −77 B (−1.2%) 7.8 KiB ✅
Codex Live turn WebSocket decoded 56.3 KiB 56.2 KiB −88 B (−0.2%) 66.4 KiB ✅
Codex Live turn messages 10 8 −2 (−20.0%) 21 ✅
Claude Total thread wire 13.5 KiB 13.5 KiB −34 B (−0.2%) 15.1 KiB ✅
Claude Thread snapshot wire 7.1 KiB 7.1 KiB +2 B (+0.0%) 7.3 KiB ✅
Claude Live turn WebSocket wire 6.4 KiB 6.4 KiB −36 B (−0.5%) 7.8 KiB ✅
Claude Live turn WebSocket decoded 57.0 KiB 57.0 KiB −44 B (−0.1%) 66.4 KiB ✅
Claude Live turn messages 9 8 −1 (−11.1%) 21 ✅

Baseline: 563c178 · PR result: ccca49e · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@coderabbitai

coderabbitai Bot commented Sep 25, 2026

Copy link
Copy Markdown

Review in Change Stack →

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 configuration

Configuration used: Repository: RTVision/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 5bfaa8a2-4b98-4bd3-93a6-9145be3873e7

📥 Commits

Reviewing files that changed from the base of the PR and between 563c178 and ccca49e.

📒 Files selected for processing (1)
  • apps/server/src/provider/cursorCredentialStore.ts

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

The Cursor credential store now uses NodeModule.createRequire to load @napi-rs/keyring when it reads the macOS Cursor credential. This replaces the asynchronous dynamic import.

Changes

Cursor credential loading

Layer / File(s) Summary
Load the keyring module
apps/server/src/provider/cursorCredentialStore.ts
The store creates a filesystem-based require and uses it to load AsyncEntry from @napi-rs/keyring instead of using a dynamic import.

Priority: ⬇️ Low

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

Change: Bug fix

Suggested reviewers: maria-rcks

Merge Risk: ⚪ Minimal · up to ccca4

The loader change has no demonstrated blocker to merging. The standalone executable check remains unrun on the reported host.

Security Architecture Review

Security architecture risk: 🔵 Low · up to ccca4

The change appears limited to loading the same Cursor keyring package; it does not appear to broaden who can request or receive the credential. Risk remains low rather than minimal because keyring loading has not been exercised in the finished executable.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The affected asset is the local Cursor keychain credential and its existing server usage paths. The identified change does not expand the independently reachable credential-reading interface.

Trust Boundaries and Controls

  • observed — The package name and keychain entry are fixed in the credential store. The provider usage-limits path retains its existing conditions before reading and forwarding the token.

Resilience and Maintainability Implications

  • observed — One identified consumer converts a keychain-loading failure into an error result rather than proceeding with a token. This does not validate that native loading succeeds in the executable.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the server fix and its purpose: restoring standalone CLI builds when Cursor usage is enabled.
Description check ✅ Passed The description explains what changed, why the change was needed, the validation performed, and that no UI changes are involved. It omits the formal Checklist and UI Changes headings, but the required…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 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.
✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

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.

@kalvenschraut
kalvenschraut merged commit 0a5124e into rtvision Sep 25, 2026
19 checks passed
@kalvenschraut
kalvenschraut deleted the fix/cli-exe-keyring-require branch September 25, 2026 19:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant