Repository navigation
fix(installer): resolve the gateway config dir like the CLI does - #4063
Conversation
install.sh hardcoded $TARGET_HOME/.config/openshell in two places: the readiness check, which reads the client certificates it expects the gateway registration to have written, and the cleanup that removes a stale openshell entry before re-adding it. The CLI instead keeps both under $XDG_CONFIG_HOME/openshell whenever that variable is set, so with the variable pointing anywhere other than ~/.config the installer and the CLI looked in different directories: the readiness check waited for certificates that were never going to appear there, and the cleanup left the real stale entry in place. Resolve the directory through as_target_user instead, which answers from the same environment the CLI is invoked in. That covers both branches without duplicating their logic: the same-user branch inherits the caller's XDG_CONFIG_HOME, while the sudo and runuser branches reset the environment and fall back to the target user's home. An empty XDG_CONFIG_HOME falls back to $HOME/.config, following the XDG base directory specification. The CLI diverges here: xdg_config_dir treats a set-but-empty value as set and joins onto it, yielding the relative path "openshell", which no installer path can usefully match. Worth fixing separately in the CLI. Signed-off-by: fede-kamel <fkamelhar@gmail.com>
There was a problem hiding this comment.
The non-empty XDG_CONFIG_HOME fix is sound, but the empty-value case still leaves the installer and CLI using different directories.
target_openshell_config_dir deliberately treats XDG_CONFIG_HOME="" as unset and returns $HOME/.config/openshell. The current openshell_core::paths::xdg_config_dir() treats an empty value as set, so the CLI resolves its root to the relative path openshell. Because registration invokes the CLI with that same empty environment value, it can write the gateway entry under ./openshell while the readiness and cleanup paths introduced here look under $HOME/.config/openshell. That preserves the mismatch for an acceptance case in #4042.
Please make the two paths agree for an empty value. Updating xdg_config_dir() to treat empty as unset would match the XDG specification and the behavior chosen here. Please also add the requested set-with-spaces test; the current focused tests cover unset, non-empty, and empty values only. A focused assertion through registration cleanup or another consumer of the helper would ensure this does more than validate the helper in isolation.
I ran tasks/scripts/test-install-sh.sh from commit 96a589822af55cb85e7be0651aea1d0a6e0db2fc; it passes. No related agent-skill updates appear necessary for this installer-path correction.
Maintainer note: the linked issue currently has Outdatedstate:validated, not state:accepted, and the author has read-only repository access. The issue acceptance gate must also be satisfied before merge.
xdg_config_dir() joined onto an empty XDG_CONFIG_HOME and resolved the config root to the relative path "openshell", while the installer falls back to $HOME/.config/openshell for the same value. Registration runs the CLI with the caller's environment, so the two could disagree on where the gateway entry lives. Treat an empty value as unset, as the XDG base directory spec requires, so the CLI and installer resolve the same directory. Add tests for a value with spaces and drive registration cleanup through the helper for the spaces and empty cases. Signed-off-by: fede-kamel <fkamelhar@gmail.com>
|
/ok to test 9ef492f |
|
@TaylorMutch Thanks for the review. Addressed in 9ef492f:
|
|
/ok to test 9ef492f |
Summary
install.shhardcoded$TARGET_HOME/.config/openshellin two places while theCLI keeps gateway entries and client certificates under
$XDG_CONFIG_HOME/openshellwhenever that variable is set:wait_for_local_gateway_listener, which reads the mTLS client bundle that thegateway registration writes.
remove_local_gateway_registration, which clears a staleopenshellentrybefore re-adding it.
With
XDG_CONFIG_HOMEpointing anywhere other than~/.config, the installerand the CLI looked in different directories: the readiness check waited out its
timeout on certificates that were never going to appear there, and the cleanup
left the real stale entry untouched.
Related Issue
Fixes #4042
Changes
target_openshell_config_dir, which resolves the directory throughas_target_userso the answer comes from the same environment the CLI isinvoked in. This covers both branches without restating their logic: the
same-user branch inherits the caller's
XDG_CONFIG_HOME, while thesudoandrunuserbranches reset the environment and fall back to the target user'shome.
remove_snap_gateway_registrationis deliberately untouched: the snap confinesits own config root under
~/snap/openshell/common/.config, which is notXDG_CONFIG_HOME.Testing
Added three assertions to
tasks/scripts/test-install-sh.sh, which sourcesinstall.shand exercises the function directly. Full suite passes:They are a real guard, not a vacuous pass — reverting only the helper to the old
hardcoded path fails the suite:
Resolution probed against the CLI's own rule
(
openshell_core::paths::xdg_config_dirjoined withopenshell), running thereal
as_target_user:XDG_CONFIG_HOME$HOME/.config/openshell/home/u/cfg/home/u/cfg/openshellopenshell(relative)$HOME/.config/openshellA divergence worth a separate look
The empty case is the one place this does not match the CLI byte for byte. The
installer follows the XDG base directory specification, which says an empty
value is treated as unset.
xdg_config_dirinstead takesenv::var'sOk("")as set and joins onto it, so it yields the relative path
openshell,resolved against the process working directory. No installer path can usefully
match that, and it looks like a bug on the CLI side rather than something to
mirror here. Happy to file it separately if you agree.