Repository navigation
fix: correct stale SDK profile path in grant configure - #48
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes grant configure so it reports the actual SDK profile path by resolving it through the SDK (profiles.GetProfilesFolder()), keeping the user-facing output consistent with what LoadProfile / SaveProfile already use.
Changes:
- Updated
grant configurelong help text and success output to use the SDK’s profile-folder resolver (profiles.GetProfilesFolder()). - Added table-driven tests to pin the printed profile path across HOME /
IDSEC_PROFILES_FOLDERedge-cases and to ensure help text no longer mentions the legacy path. - Documented the resolver requirement in
CLAUDE.mdand added migration guidance to theCHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| cmd/configure.go | Uses profiles.GetProfilesFolder() for the “Profile saved to …” path and updates help text to the new default location. |
| cmd/configure_test.go | Adds coverage for resolver edge-cases and asserts the legacy path isn’t present in help/output. |
| CLAUDE.md | Documents why the SDK resolver must be used (including the HOME/Windows caveat). |
| CHANGELOG.md | Notes the fix in Unreleased and adds a v0.7.0 migration note for the silent profile-dir move. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
4
to
8
| "errors" | ||
| "fmt" | ||
| "net/url" | ||
| "os" | ||
| "path/filepath" | ||
| "strings" |
aaearon
force-pushed
the
fix/stale-profile-paths
branch
from
August 13, 2026 16:02
aaef594 to
57e7ec6
Compare
`grant configure` printed `~/.idsec_profiles/grant` in both its Long help
and its "Profile saved to" success message. That path has been wrong since
the idsec-sdk-golang upgrade shipped in v0.7.0: the SDK's
profiles.GetProfilesFolder() resolves IDSEC_PROFILES_FOLDER, falling back to
$HOME/.idsec/profiles.
The hand-rolled resolution is replaced by a direct call to
profiles.GetProfilesFolder(). It is deliberately NOT "improved" to use
os.UserHomeDir(): the SDK reads os.Getenv("HOME"), which on Windows is often
unset and yields a relative .idsec/profiles under the CWD. configure must
print exactly the path the loader will later read — diverging would be worse
than the stale string. The Windows caveat is documented in CLAUDE.md.
Tests (written first, confirmed failing):
- TestRunConfigurePrintsProfilePath — table-driven over the full resolution
matrix: custom IDSEC_PROFILES_FOLDER, override unset, override set-but-empty
(must fall back to HOME — the SDK only takes it when non-empty), empty HOME
(relative path, the Windows case) and relative HOME. Each case also asserts
the printed path equals profiles.GetProfilesFolder()/grant, pinning the
write path to what login/root read, and that the legacy path never appears.
- TestConfigureLongHelpHasNoLegacyPath — guards the help text.
CHANGELOG: the released [0.7.0] section is edited to add the migration note,
a deliberate exception to Keep-a-Changelog immutability — the directory move
was a user-visible side effect of that release's SDK upgrade and was never
recorded, so users upgrading from v0.6.x have no way to learn about it from
an [Unreleased] entry. The documented migration uses `mv -n` so a user who
already re-ran `grant configure` cannot clobber their current profile with
the stale one; `-n` is supported by both GNU and BSD/macOS mv. The help-text
fix itself is under [Unreleased]/Fixed.
aaearon
force-pushed
the
fix/stale-profile-paths
branch
from
August 13, 2026 19:52
57e7ec6 to
a648e4c
Compare
Owner
Author
|
Rebased onto |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
grant configuretold users their SDK profile lives at~/.idsec_profiles/grant. Since idsec-sdk-golang v0.2.3 — shipped in grant v0.7.0 — the real default is$HOME/.idsec/profiles, overridable viaIDSEC_PROFILES_FOLDER.The printed path is now resolved with
profiles.GetProfilesFolder(), the same function the write path (SaveProfile) and read path (LoadProfile) already use, so whatconfigureprints is by construction whatloginreads.Deliberately not "improved" to
os.UserHomeDir(): the SDK usesos.Getenv("HOME"), which on Windows is often unset and resolves to a relative.idsec/profilesunder the CWD. Matching the SDK exactly matters more than being right in isolation — diverging would makeconfigureandlogindisagree. The caveat is documented in CLAUDE.md.Retroactive CHANGELOG note
v0.7.0 moved everyone's profile silently. A migration note is added to the already-released
[0.7.0]section — a deliberate exception to Keep-a-Changelog immutability, justified in the commit body. The migration command usesmv -nso a user who already re-rangrant configurecannot clobber their newer profile with a stale one.Tests
TestRunConfigurePrintsProfilePath(5 cases: custom folder, default under HOME, empty-vs-unsetIDSEC_PROFILES_FOLDER, empty HOME, relative HOME) andTestConfigureLongHelpHasNoLegacyPath. Each case pins output tofilepath.Join(profiles.GetProfilesFolder(), "grant"), so a future regression toos.UserHomeDir()fails on the empty-HOME and relative-HOME cases specifically. Both tests were confirmed failing before the fix.Reviewed by Codex: verdict ship with fixes; both findings addressed.