diff --git a/shared/credstore/credstore.go b/shared/credstore/credstore.go index 0c3fae48..d8e03c51 100644 --- a/shared/credstore/credstore.go +++ b/shared/credstore/credstore.go @@ -167,6 +167,9 @@ type Source string const ( SourceUnset Source = "unset" SourceDefault Source = "shared default" + // SourceJTK labels a value read from the per-tool jtk section + // (jtk.default_project), which is not part of the shared default. + SourceJTK Source = "shared " + ToolJTK ) // ResolveWithSource returns the resolved value and where it came from. diff --git a/shared/credstore/relocate.go b/shared/credstore/relocate.go index 6763be95..f14a568b 100644 --- a/shared/credstore/relocate.go +++ b/shared/credstore/relocate.go @@ -263,9 +263,36 @@ func LoadSharedRuntime() (*Store, error) { // branches hermetically testable on Linux (where the resolver would // otherwise collapse old≡new). func loadSharedRuntime(newPath string) (*Store, error) { + s, _, err := resolveSharedRuntime(newPath) + return s, err +} + +// SharedRuntimePath returns the shared config file that +// LoadSharedRuntime reads: the canonical path, or the prior location on +// the old-only read fallback. "" means no shared file is in effect +// (absent, or corrupt, in which case the error is ErrCorruptStore). On +// an old/new divergence it returns the canonical path together with +// ErrRelocationConflict, matching the store LoadSharedRuntime returns. +func SharedRuntimePath() (string, error) { + newPath, err := DefaultPath() + if err != nil { + return "", err + } + return sharedRuntimePath(newPath) +} + +func sharedRuntimePath(newPath string) (string, error) { + _, path, err := resolveSharedRuntime(newPath) + return path, err +} + +// resolveSharedRuntime is the single runtime resolution behind +// loadSharedRuntime and sharedRuntimePath, returning the store and the +// file it was read from ("" when no shared file exists). +func resolveSharedRuntime(newPath string) (*Store, string, error) { st, err := classifyRelocation(newPath) if err != nil { - return nil, err + return nil, "", err } switch st.kind { case relocOldOnly: @@ -273,15 +300,18 @@ func loadSharedRuntime(newPath string) (*Store, error) { // store IS the effective config until init materializes it. oldStore, oerr := Load(st.oldPath) if oerr != nil { - return nil, oerr + return nil, "", oerr } - return oldStore, nil + return oldStore, st.oldPath, nil case relocBothDivergent: - return st.newStore, relocationConflict(st.oldPath, newPath, "run init to reconcile") + return st.newStore, newPath, relocationConflict(st.oldPath, newPath, "run init to reconcile") case relocNone, relocBothEqual: - return st.newStore, nil + if _, serr := os.Stat(newPath); serr != nil { + return st.newStore, "", nil + } + return st.newStore, newPath, nil } - return st.newStore, nil + return st.newStore, "", nil } // OldSharedConnCandidates yields the origin-labeled connection diff --git a/shared/credstore/relocate_test.go b/shared/credstore/relocate_test.go index 1492cef6..c8ead0c2 100644 --- a/shared/credstore/relocate_test.go +++ b/shared/credstore/relocate_test.go @@ -313,6 +313,57 @@ func TestLoadSharedRuntime_PathIdentityUsesCanonical(t *testing.T) { } } +func TestSharedRuntimePath(t *testing.T) { + body := "default:\n url: https://acme.atlassian.net\n" + tests := []struct { + name string + writeOld bool + newBody string // "" leaves the canonical file absent + wantOld bool // expect the prior location instead of newPath + wantNone bool + wantErr error + }{ + {name: "none present", wantNone: true}, + {name: "old only reads the prior file", writeOld: true, wantOld: true}, + {name: "new only", newBody: body}, + {name: "both equal uses canonical", writeOld: true, newBody: body}, + {name: "divergent names canonical with conflict", writeOld: true, + newBody: "default:\n url: https://NEW.atlassian.net\n", wantErr: ErrRelocationConflict}, + {name: "corrupt canonical", newBody: "default: [unclosed\n", wantNone: true, wantErr: ErrCorruptStore}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + oldPath := oldBase(t) + newPath := filepath.Join(t.TempDir(), "new", "config.yml") + if tc.writeOld { + writeFile(t, oldPath, body) + } + if tc.newBody != "" { + writeFile(t, newPath, tc.newBody) + } + + got, err := sharedRuntimePath(newPath) + if tc.wantErr != nil { + if !errors.Is(err, tc.wantErr) { + t.Fatalf("want %v, got %v", tc.wantErr, err) + } + } else if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := newPath + switch { + case tc.wantNone: + want = "" + case tc.wantOld: + want = oldPath + } + if got != want { + t.Fatalf("path = %q, want %q", got, want) + } + }) + } +} + func TestApplySharedRelocation_NoOpWhenNotNeeded(t *testing.T) { if err := ApplySharedRelocation(nil); err != nil { t.Fatalf("nil ⇒ no-op, got %v", err) diff --git a/tools/jtk/CHANGELOG.md b/tools/jtk/CHANGELOG.md index 0ead9aab..b3d65dd0 100644 --- a/tools/jtk/CHANGELOG.md +++ b/tools/jtk/CHANGELOG.md @@ -58,6 +58,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- `config show` now reports values set in the shared `atlassian-cli/config.yml` store (url, email, default project) with their shared source, and its path row names the config file the runtime actually reads, including the prior shared location on macOS before `jtk init` relocates it. Previously these rows showed `-` and the legacy per-tool path even though jtk used the shared values. Thanks to @jurij for the fix. ([#483](https://github.com/open-cli-collective/atlassian-cli/pull/483)) - Smart links and mentions no longer disappear from `issues get` descriptions, comment output, and rich-text custom field values. Inline and block cards now render as their URL and mentions as their display text; previously they rendered as empty text. ([#495](https://github.com/open-cli-collective/atlassian-cli/pull/495)) - `issues update --description` no longer deletes images and attachments embedded in the stored description. Text input now keeps the existing media blocks, appended after the new text, and the update stops with an error instead of writing if the current description cannot be read. Raw ADF input still replaces the description exactly. Thanks to @tio-foosy, who proposed media preservation in [#407](https://github.com/open-cli-collective/atlassian-cli/pull/407). ([#488](https://github.com/open-cli-collective/atlassian-cli/issues/488)) - Rich-text (paragraph) custom fields now render in `issues get --fields`, `issues get --custom-fields`, `issues fields `, and `issues list/search --fields`, and count as populated in `issues check`. The v3 API returns these fields as ADF documents, which were previously shown as `-`, omitted, or reported as missing. Thanks to @tio-foosy, who proposed the same fix in [#407](https://github.com/open-cli-collective/atlassian-cli/pull/407). ([#493](https://github.com/open-cli-collective/atlassian-cli/pull/493)) diff --git a/tools/jtk/internal/cmd/configcmd/configcmd.go b/tools/jtk/internal/cmd/configcmd/configcmd.go index fb69577f..2130b860 100644 --- a/tools/jtk/internal/cmd/configcmd/configcmd.go +++ b/tools/jtk/internal/cmd/configcmd/configcmd.go @@ -44,10 +44,10 @@ func newShowCmd(opts *root.Options) *cobra.Command { The API token is shown as a presence status only (its value lives in the OS keyring and is never displayed); token/keyring reporting is -authoritative. The non-secret rows reflect environment variables and the -legacy per-tool file ONLY — a value set solely in the shared -~/.config/atlassian-cli/config.yml is shown as "-" here even though jtk -resolves and uses it at runtime.`, +authoritative. The non-secret rows reflect the same precedence the +runtime uses: environment variables, then the shared +atlassian-cli/config.yml, then the legacy per-tool file. The path row +names the file the runtime reads.`, RunE: func(_ *cobra.Command, _ []string) error { cfg := config.GetValuesWithSources() diff --git a/tools/jtk/internal/config/source.go b/tools/jtk/internal/config/source.go index 034f65e7..65c08b7b 100644 --- a/tools/jtk/internal/config/source.go +++ b/tools/jtk/internal/config/source.go @@ -5,6 +5,7 @@ import ( "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/keyring" + "github.com/open-cli-collective/atlassian-go/url" ) // ValuesWithSources holds all config values with their source information. @@ -64,10 +65,22 @@ func GetValuesWithSources() ValuesWithSources { AuthMethodSrc: authMethodSrc, CloudID: cloudID, CloudIDSrc: cloudIDSrc, - Path: Path(), + Path: activeConfigPath(), } } +// activeConfigPath returns the config file the resolvers actually read: +// the shared store credstore resolves at runtime (including the prior +// location on the relocation read fallback), else the legacy per-tool +// file. `config show` used to always print the legacy path, which is +// misleading once `jtk init` has written the shared store. +func activeConfigPath() string { + if sp, _ := credstore.SharedRuntimePath(); sp != "" { + return sp + } + return Path() +} + // keyringBackendLabel renders the backend and how it was selected, e.g. // "keychain (auto)". Empty when the keyring could not be opened. func keyringBackendLabel(kr keyring.Info) string { @@ -81,7 +94,7 @@ func keyringBackendLabel(kr keyring.Info) string { } // GetURLWithSource returns the URL and its source. -// Precedence: JIRA_URL → ATLASSIAN_URL → config url → JIRA_DOMAIN (legacy) → config domain (legacy) +// Precedence: JIRA_URL → ATLASSIAN_URL → shared default → config url → JIRA_DOMAIN (legacy) → config domain (legacy) func GetURLWithSource() (value, source string) { if os.Getenv("JIRA_URL") != "" { return GetURL(), "env (JIRA_URL)" @@ -89,6 +102,9 @@ func GetURLWithSource() (value, source string) { if os.Getenv("ATLASSIAN_URL") != "" { return GetURL(), "env (ATLASSIAN_URL)" } + if v, src := jtkSectionWithSource("url"); v != "" { + return url.NormalizeURL(v), string(src) + } cfg, err := Load() if err != nil { return "", "-" @@ -107,7 +123,7 @@ func GetURLWithSource() (value, source string) { } // GetEmailWithSource returns the email and its source. -// Precedence: JIRA_EMAIL → ATLASSIAN_EMAIL → config email +// Precedence: JIRA_EMAIL → ATLASSIAN_EMAIL → shared default → config email func GetEmailWithSource() (value, source string) { if os.Getenv("JIRA_EMAIL") != "" { return GetEmail(), "env (JIRA_EMAIL)" @@ -115,6 +131,9 @@ func GetEmailWithSource() (value, source string) { if os.Getenv("ATLASSIAN_EMAIL") != "" { return GetEmail(), "env (ATLASSIAN_EMAIL)" } + if v, src := jtkSectionWithSource("email"); v != "" { + return v, string(src) + } cfg, err := Load() if err != nil { return "", "-" @@ -126,11 +145,14 @@ func GetEmailWithSource() (value, source string) { } // GetDefaultProjectWithSource returns the default project and its source. -// Precedence: JIRA_DEFAULT_PROJECT → config default_project +// Precedence: JIRA_DEFAULT_PROJECT → shared jtk.default_project → config default_project func GetDefaultProjectWithSource() (value, source string) { if os.Getenv("JIRA_DEFAULT_PROJECT") != "" { return GetDefaultProject(), "env (JIRA_DEFAULT_PROJECT)" } + if v := loadShared().JTK.DefaultProject; v != "" { + return v, string(credstore.SourceJTK) + } cfg, err := Load() if err != nil { return "", "-" diff --git a/tools/jtk/internal/config/source_test.go b/tools/jtk/internal/config/source_test.go index 3e1919a9..6713d230 100644 --- a/tools/jtk/internal/config/source_test.go +++ b/tools/jtk/internal/config/source_test.go @@ -1,8 +1,11 @@ package config import ( + "os" + "path/filepath" "testing" + "github.com/open-cli-collective/atlassian-go/credstore" "github.com/open-cli-collective/atlassian-go/keyring" "github.com/open-cli-collective/atlassian-go/testutil" ) @@ -151,6 +154,84 @@ func TestGetDefaultProjectWithSource_EnvPrecedence(t *testing.T) { testutil.Equal(t, source, "env (JIRA_DEFAULT_PROJECT)") } +// Values set only in the shared store must show up in `config show` +// with the shared source label — they are what the runtime resolves +// (this used to render as empty "-" rows, hiding a working config). +func TestGetValuesWithSources_SharedStore(t *testing.T) { + sharedPath, cleanup := setupTestConfig(t) + defer cleanup() + + store := &credstore.Store{ + Default: credstore.Section{ + URL: "https://shared.atlassian.net", + Email: "shared@example.com", + }, + JTK: credstore.ToolSection{DefaultProject: "SHARED"}, + } + testutil.RequireNoError(t, store.Save(sharedPath)) + + result := GetValuesWithSources() + + testutil.Equal(t, result.URL, "https://shared.atlassian.net") + testutil.Equal(t, result.URLSource, string(credstore.SourceDefault)) + + testutil.Equal(t, result.Email, "shared@example.com") + testutil.Equal(t, result.EmailSource, string(credstore.SourceDefault)) + + testutil.Equal(t, result.DefaultProject, "SHARED") + testutil.Equal(t, result.ProjectSource, string(credstore.SourceJTK)) + + // The path row names the file actually read. + testutil.Equal(t, result.Path, sharedPath) + + // Env still outranks shared. + t.Setenv("JIRA_URL", "https://env.atlassian.net") + value, source := GetURLWithSource() + testutil.Equal(t, value, "https://env.atlassian.net") + testutil.Equal(t, source, "env (JIRA_URL)") +} + +// Shared store outranks the legacy per-tool file in the source column, +// matching runtime resolution. +func TestGetValuesWithSources_SharedBeatsLegacy(t *testing.T) { + sharedPath, cleanup := setupTestConfig(t) + defer cleanup() + + testutil.RequireNoError(t, Save(&Config{ + URL: "https://legacy.atlassian.net", + Email: "legacy@example.com", + })) + store := &credstore.Store{ + Default: credstore.Section{URL: "https://shared.atlassian.net"}, + } + testutil.RequireNoError(t, store.Save(sharedPath)) + + value, source := GetURLWithSource() + testutil.Equal(t, value, "https://shared.atlassian.net") + testutil.Equal(t, source, string(credstore.SourceDefault)) + + // Email is unset in shared, so the legacy file still supplies it. + value, source = GetEmailWithSource() + testutil.Equal(t, value, "legacy@example.com") + testutil.Equal(t, source, "config") +} + +// A corrupt shared file makes the resolvers fall back to the legacy +// per-tool file, so the path row must name the legacy file too. +func TestGetValuesWithSources_CorruptSharedShowsLegacyPath(t *testing.T) { + sharedPath, cleanup := setupTestConfig(t) + defer cleanup() + + testutil.RequireNoError(t, Save(&Config{URL: "https://legacy.atlassian.net"})) + testutil.RequireNoError(t, os.MkdirAll(filepath.Dir(sharedPath), 0o700)) + testutil.RequireNoError(t, os.WriteFile(sharedPath, []byte("default: [unclosed\n"), 0o600)) + + result := GetValuesWithSources() + testutil.Equal(t, result.URL, "https://legacy.atlassian.net") + testutil.Equal(t, result.URLSource, "config") + testutil.Equal(t, result.Path, Path()) +} + func TestGetValuesWithSources_AllFields(t *testing.T) { _, cleanup := setupTestConfig(t) defer cleanup()