feat(config): follow XDG Base Directory spec for config location - #1503
feat(config): follow XDG Base Directory spec for config location#1503jared-outpost[bot] wants to merge 13 commits into
Conversation
Resolve the config/data directory via the XDG Base Directory specification instead of always using `~/.sentry`. Precedence: 1. `SENTRY_CONFIG_DIR` override (unchanged, highest priority) 2. Legacy `~/.sentry` when it already exists (no breakage for existing installs) 3. `/sentry`, defaulting to `~/.config/sentry` A non-absolute `XDG_CONFIG_HOME` is ignored per the spec. This keeps credentials and caches out of the home directory root, which also unblocks environments (e.g. coding agents) that restrict writes to `/root`. Fixes #1502
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
CI green on the feature branch (check_suite success). Self-review clean. Marking ready and requesting review from the issue author. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…onfig.json A bare ~/.sentry/bin created by the curl installer should not prevent new XDG-based installs. The legacy check now requires the presence of the actual database or the old JSON config file. Fixes the Cursor Bugbot report on PR #1503.
|
Jared, we need to do the following:
|
|
ack — will update binary install paths to follow XDG (via resolveConfigDir) and add migration logic inside |
|
Jared, update the patch already? |
|
working on it — expanding to XDG-aware binary paths + migration inside setup. will push the update shortly. |
|
yep, expanding the patch now to also move binary install paths to XDG (via XDG_BIN_HOME / ~/.local/bin) and add migration inside setup. will push the update. |
|
expanding the patch now to cover XDG binary paths + migration in setup. pushing shortly. |
|
not yet — expanding determineInstallDir + setup migration for binary paths now. will push once the XDG-aware logic + migration is in. |
|
Jared, update the patch based on our earlier discussions |
…acy layout Extend the XDG work to the binary install directory and add automatic migration of the legacy ~/.sentry layout in `sentry cli setup`. - determineInstallDir now honors an absolute XDG_BIN_HOME (after SENTRY_INSTALL_DIR) and falls back to ~/.local/bin instead of ~/.sentry/bin. - setup migrates an existing ~/.sentry/bin binary and legacy config (cli.db, config.json) into the XDG locations on first run; skipped when the target already exists. - upgrade's known-curl-path detection and fallback install path track the same XDG-aware resolution. - install script and getCurlInstallPaths recognize XDG_BIN_HOME. - resolveXdgConfigDir exposes the XDG target (bypassing legacy detection) so migration doesn't no-op. - docs: document binary install location + migration. Addresses review feedback on #1503.
|
pushed in 90c9cb2 — expanded per your two points:
tests: new XDG cases in binary.test.ts + a migration suite in setup.test.ts; full run green except one pre-existing root-only failure ( |
Reevaluated after scope expanded to include binary install paths. - Close the SQLite DB before renaming cli.db/WAL sidecars. The DB is opened at startup (cleanup-old-binary), and an open file cannot be renamed on Windows — previously the whole migration threw and was swallowed. Split config and binary migration into separate functions with independent try/catch so one failing can't skip the other. - migrateLegacyBinary now returns the new path; setup adopts it as binaryPath/binaryDir so PATH modification and setInstallInfo point at the migrated binary instead of the deleted legacy path (a --method setInstallInfo no longer overwrites it with the old location). - docs: clarify that 'sentry upgrade' keeps the binary in place (pins SENTRY_INSTALL_DIR) and only 'sentry cli setup' relocates it; config data still migrates on upgrade. - test: record-new-path regression covering the stale-path fix. Addresses cursor[bot] review on #1503.
# Conflicts: # packages/cli/plugins/sentry-cli/skills/sentry-cli/references/dashboard.md
|
merged latest main into the branch in 4148c42 — the conflict (a generated skill reference doc) is resolved and generated files are regenerated against the merged sources. PR is mergeable again; tsc + setup/binary/config tests green locally. |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
copyFileSync already preserves the source mode, so a migrated ~/.sentry binary stays executable in practice. Add an explicit chmodSync(0o755) after the copy anyway — mirrors installBinary — so the exec bit is never in doubt even if the legacy copy's permissions were stripped. Covered by a new accessSync(X_OK) assertion in the migration test. Also drain the now-flagged silent catch around unlinkSync with a log.debug.
Previously 'sentry upgrade' hard-pinned SENTRY_INSTALL_DIR to the current install dir, so a legacy ~/.sentry/bin user could never migrate off it via upgrade — the docs' claim that upgrade keeps the binary in place was technically true but meant curl users never reached the XDG location. Now resolveUpgradeInstallDir relocates a legacy ~/.sentry/bin install to the XDG install dir when that dir is already on PATH (upgrade runs --no-modify-path, so it must not move a binary to a dir that isn't on PATH). setup's migrateLegacyBinary then moves the old binary and removes it before --install writes the new version; the legacy dir is left empty so nothing shadows. Non-legacy installs and off-PATH XDG dirs stay put. - resolveUpgradeInstallDir helper + unit tests (relocate / stay-put / undefined PATH / non-legacy). - docs: describe the conditional relocation on upgrade accurately.
|
good catch — you're right that the old design left curl users stuck on fixed in 5c07acb: added unit tests for the relocate / stay-put / off-PATH / undefined-PATH cases and updated the docs. (the windows log you pasted is from an unrelated app — 'Attaquer Taskbar' hitting a WinRT LimitedAccessFeatures COM error — nothing to do with sentry, so no action there.) |
resolveUpgradeInstallDir compared a stored install path against a freshly computed ~/.sentry/bin using strict ===. On case-insensitive filesystems (Windows, macOS) the two can differ only in casing (e.g. C:\Users\User vs C:\Users\user) yet point at the same directory, so a legacy install would fail the equality check and never relocate to the XDG dir. Add a samePath() helper that lowercases on win32/darwin and use it for both the legacy-dir and XDG-target comparisons. Test asserts a mixed-case legacy dir is still treated as legacy on case-insensitive platforms. Addresses sentry[bot] review on #1503.
|
fix-ci: attempt 1 — the only failure is |
BYK
left a comment
BYK
left a comment
There was a problem hiding this comment.
I think we also need one e2e migration test to make sure the migration is actually working.
- Binary migration is no longer hardcoded to ~/.sentry/bin: findMigratableBinary scans getKnownInstallDirs() (derived from KNOWN_CURL_DIRS) for a binary in any known install dir other than the resolved target. - Deduplicate path logic: samePath and LEGACY_INSTALL_SUBDIR now live in binary.ts and are shared by setup and upgrade (removed the duplicate samePath in upgrade.ts). - Use async node:fs/promises (mkdir/copyFile/chmod/rename/unlink) in the migration paths. - Report migration failures to Sentry: both migrations now run through bestEffort(), which warns the user and captureException()s while never aborting setup. - Add an e2e migration test (test/e2e/migration.test.ts) that spawns the real CLI and verifies both the config DB and a legacy ~/.sentry/bin binary migrate to the XDG locations. Plus unit tests for samePath / getKnownInstallDirs. Addresses BYK's CHANGES_REQUESTED review on #1503.
|
Thanks — all five review comments are addressed in b03db3b, and threads resolved:
PR description updated to match. |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit b03db3b. Configure here.
Address BugBot + Seer findings on the latest revision. - BugBot (high): findMigratableBinary scanned all known install dirs, so a stray binary in ~/.local/bin or ~/bin — both valid *current* XDG targets — could be moved out and the original deleted, leaving the real ~/.sentry/bin install behind. Replace getKnownInstallDirs with getLegacyInstallDirs, which is limited to the genuinely pre-XDG ~/.sentry/bin. Add setup tests asserting binaries in ~/.local/bin and ~/bin are never relocated. - Seer (medium): isInPath used a case-sensitive membership check; on Windows/ macOS a PATH entry can differ only in casing from a computed dir. Compare case-insensitively on those platforms, matching samePath. Add isInPath tests. Unit + e2e migration tests green; tsc + biome clean.
1 participant
Closes #1502
What
sentrystored its config/data (thecli.dbSQLite database with credentials and caches) and its installed binary under~/.sentry, cluttering the home directory and breaking in environments that block writes to$HOME(e.g. sandboxed coding agents).This makes both the config directory and the binary install directory follow the XDG Base Directory specification, matching how the CLI already resolves shell/completion paths, and migrates existing
~/.sentryinstalls into the new locations on firstsetup.Config directory
getConfigDir()resolves via a pureresolveConfigDir(env, home)helper:SENTRY_CONFIG_DIR— explicit override (highest priority)~/.sentry— used only when it actually holds config (cli.dborconfig.json), so a bare~/.sentry/binfrom the installer no longer blocks XDG$XDG_CONFIG_HOME/sentry— defaulting to~/.config/sentry. A non-absoluteXDG_CONFIG_HOMEis ignored per the spec.resolveXdgConfigDir()exposes the XDG target directly (bypassing legacy detection) so migration doesn't no-op whilecli.dbstill sits in~/.sentry.Binary install directory
determineInstallDir()now resolves:SENTRY_INSTALL_DIR— explicit override$XDG_BIN_HOME— when set to an absolute path, per the XDG spec~/.local/binor~/bin— when either exists and is already onPATH~/.local/bin— default fallback (previously~/.sentry/bin)upgrade's known-curl-path detection and its fallback install path track the same resolution, and the install script recognizesXDG_BIN_HOME.Migration (in
sentry cli setup)On first run,
setupmigrates the legacy~/.sentrylayout into the XDG locations. Config and binary migration are independent (a failure in one can't skip the other):cli.db+ WAL sidecars andconfig.jsoninto the XDG config dir. Skipped when a targetcli.dbalready exists.getLegacyInstallDirs(), currently just the pre-XDG~/.sentry/bin) and, when found, copies it into the resolved install target,chmods it executable, records the new path viasetInstallInfo, and removes the old copy.~/.local/binand~/binare valid current XDG targets, so they are never treated as migration sources.setupthen adopts the new path soPATHsetup and recorded install metadata point at the migrated binary, not the deleted legacy location. Skipped when a binary already exists at the target.Both migrations use async
node:fs/promisesand run through the sharedbestEffort()helper, so any failure is surfaced as a warning and reported to Sentry (captureException) without ever aborting setup.sentry upgraderunssetupon the new binary, so it migrates too — but conservatively, because upgrade never editsPATH(it runs--no-modify-path). A legacy~/.sentry/binbinary is relocated to the XDG install dir only when that dir is already onPATH(resolveUpgradeInstallDir), so the moved binary stays discoverable; setup's legacy-binary migration moves the old binary and removes it before--installwrites the new one. If the XDG dir isn't onPATH, upgrade leaves the binary in place — runsentry cli setupexplicitly to relocate it and updatePATH. Legacy config is migrated on upgrade regardless.Changes
packages/cli/src/lib/db/index.ts—resolveConfigDir+ newresolveXdgConfigDirhelper.packages/cli/src/lib/binary.ts— XDG-awaredetermineInstallDir(XDG_BIN_HOME,~/.local/binfallback).packages/cli/src/lib/upgrade.ts— known-curl-path detection + fallback install path track the XDG resolution.packages/cli/src/lib/binary.ts— sharedsamePath,LEGACY_INSTALL_SUBDIR, andgetLegacyInstallDirs()(used by both setup and upgrade).packages/cli/src/commands/cli/setup.ts—migrateLegacyConfig/migrateLegacyBinary(async,bestEffort-wrapped), adopted into the setup flow.packages/cli/install— install script recognizesXDG_BIN_HOME.test/lib/config.test.ts,test/lib/binary.test.ts(XDG cases +samePath/getLegacyInstallDirs),test/commands/cli/setup.test.ts(migration + records-new-path + executability),test/commands/cli/upgrade.test.ts(resolveUpgradeInstallDir), andtest/e2e/migration.test.ts(spawns the real CLI and verifies config-DB + legacy-binary migration end-to-end).apps/cli-docs/src/fragments/configuration.mddocuments config location, binary install location, and migration behavior.Testing
vitest runfor config / binary / setup suites is green (one pre-existing root-onlyacquireLockpermission test fails onmaintoo, since root ignoreschmod 0o000).tsc --noEmitclean; Biome clean on changed files.Review follow-ups addressed
copyFileSyncpermission concern verified (mode is preserved) and hardened with an explicitchmod+ test.