Skip to content

fix: resolve keytar directory in postinstall script - #713

Merged
congminh1254 merged 1 commit into
mainfrom
fix/postinstall-resolve-keytar-dir
Aug 28, 2026
Merged

fix: resolve keytar directory in postinstall script#713
congminh1254 merged 1 commit into
mainfrom
fix/postinstall-resolve-keytar-dir

Conversation

@congminh1254

Copy link
Copy Markdown
Member

Summary

  • Replace the hardcoded node_modules/@github/keytar/build path in postinstall.js with require.resolve('@github/keytar/package.json') to locate the installed package directory
  • Matches the resolution pattern already used in secure-storage.js and tests, so the stale build/ directory is removed reliably across hoisted or nested install layouts

Test plan

  • Run node ./src/postinstall.js locally — exits successfully
  • Run npm install on a fresh checkout and confirm postinstall completes without error
  • Verify keytar still works at runtime on Windows/Linux (secure storage read/write)

Made with Cursor

Use require.resolve to locate @github/keytar instead of a hardcoded
node_modules path, so the build directory is removed reliably across
install layouts.

Co-authored-by: Cursor <cursoragent@cursor.com>
@congminh1254
congminh1254 requested a review from a team August 27, 2026 13:19
@congminh1254 congminh1254 changed the title fix: resolve keytar directory in postinstall script fix: resolve keytar directory in postinstall script Aug 27, 2026
@congminh1254 congminh1254 changed the title fix: resolve keytar directory in postinstall script fix: resolve keytar directory in postinstall script Aug 27, 2026
@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 33076266001

Coverage increased (+0.03%) to 84.939%

Details

  • Coverage increased (+0.03%) from the base build.
  • Patch coverage: 2 uncovered changes across 1 file (0 of 2 lines covered, 0.0%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
src/postinstall.js 2 0 0.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 6060
Covered Lines: 5396
Line Coverage: 89.04%
Relevant Branches: 2153
Covered Branches: 1580
Branch Coverage: 73.39%
Branches in Coverage %: Yes
Coverage Strength: 642.41 hits per line

💛 - Coveralls

@Pask423 Pask423 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@congminh1254
congminh1254 merged commit 24b6fce into main Aug 28, 2026
22 of 24 checks passed
@congminh1254
congminh1254 deleted the fix/postinstall-resolve-keytar-dir branch August 28, 2026 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants