feat(state): port slck to cli-common state components [MON-5372]

go.mod

@@ -3,7 +3,7 @@ module github.com/open-cli-collective/slack-chat-api
 go 1.24.0
 
 require (
-	github.com/open-cli-collective/cli-common v0.0.0-20260516182733-b753d5c62d14
+	github.com/open-cli-collective/cli-common v0.0.0-20260519134256-e67b2fc81f9d
 	github.com/spf13/cobra v1.8.0
 	github.com/stretchr/testify v1.11.1
 	golang.org/x/term v0.27.0

go.sum

@@ -25,6 +25,8 @@ github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e h1:fD57ERR4JtEqsWb
 github.com/niemeyer/pretty v0.0.0-20200227124842-a10e7caefd8e/go.mod h1:zD1mROLANZcx1PVRCS0qkT7pwLkGfwJo4zjcN/Tysno=
 github.com/open-cli-collective/cli-common v0.0.0-20260516182733-b753d5c62d14 h1:78EW5uCbAzbAO32+oY4HDaFOqS2sPYnc4AT+G5UjdL0=
 github.com/open-cli-collective/cli-common v0.0.0-20260516182733-b753d5c62d14/go.mod h1:5i4MkFToMVPLBW29O01lsHS9d1m9pC0BxSOYjFDz7ds=
+github.com/open-cli-collective/cli-common v0.0.0-20260519134256-e67b2fc81f9d h1:fiyfxc/Wvpuen/u6Mk3bCUG0DLyOylF0K+eol4y9C64=
+github.com/open-cli-collective/cli-common v0.0.0-20260519134256-e67b2fc81f9d/go.mod h1:5i4MkFToMVPLBW29O01lsHS9d1m9pC0BxSOYjFDz7ds=
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM=

internal/cmd/config/clear.go

@@ -54,18 +54,29 @@ func runClear(opts *clearOptions) error {
 	}
 
 	if opts.all {
-		// §1.7: --all returns the active profile to a pre-init state —
-		// keyring bundle (above) + config file + caches + empty dirs.
-		// slck has NO cache directory today (no command writes one); if
-		// one is ever added it must also be removed here.
-		p := appconfig.Path()
-		switch err := os.Remove(p); {
-		case err == nil:
-			output.Printf("Removed %s\n", p)
-		case os.IsNotExist(err):
-			// idempotent
-		default:
-			return err
+		// §1.7: --all returns the active profile to a pre-init state. We
+		// must remove BOTH the new canonical config and the pre-MON-5372
+		// hand-rolled path — runtime old-only fallback would otherwise let
+		// a stale old config silently resurrect post-clear. Path-identity
+		// dedupe (Linux: old == new).
+		paths := []string{}
+		newPath, perr := appconfig.Path()
+		if perr != nil {
+			return perr
+		}
+		paths = append(paths, newPath)
+		if oldPath, oerr := appconfig.OldConfigPath(); oerr == nil && oldPath != newPath {
+			paths = append(paths, oldPath)
+		}
+		for _, p := range paths {
+			switch err := os.Remove(p); {
+			case err == nil:
+				output.Printf("Removed %s\n", p)
+			case os.IsNotExist(err):
+				// idempotent
+			default:
+				return err
+			}
 		}
 	}
 	return nil

internal/cmd/config/config_test.go

@@ -2,6 +2,8 @@ package config
 
 import (
 	"bytes"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 
@@ -10,6 +12,7 @@ import (
 
 	"github.com/open-cli-collective/cli-common/credstore"
 
+	appconfig "github.com/open-cli-collective/slack-chat-api/internal/config"
 	"github.com/open-cli-collective/slack-chat-api/internal/keychain"
 	"github.com/open-cli-collective/slack-chat-api/internal/output"
 	"github.com/open-cli-collective/slack-chat-api/internal/testutil"
@@ -121,3 +124,34 @@ func TestRunClear_Idempotent(t *testing.T) {
 	_, err = captureOutput(t, func() error { return runClear(&clearOptions{}) })
 	require.NoError(t, err)
 }
+
+// TestRunClearAll_RemovesBothNewAndOldConfigPaths pins the MON-5372 contract:
+// --all must scrub BOTH the new canonical config.yml AND the pre-MON-5372
+// hand-rolled path. Otherwise a stale old file would silently resurrect the
+// config via runtime old-only fallback. Linux: paths dedupe to one.
+func TestRunClearAll_RemovesBothNewAndOldConfigPaths(t *testing.T) {
+	testutil.Setup(t)
+
+	// Plant a config.yml at the NEW canonical path.
+	newPath, err := appconfig.Path()
+	require.NoError(t, err)
+	require.NoError(t, os.MkdirAll(filepath.Dir(newPath), 0o700))
+	require.NoError(t, os.WriteFile(newPath, []byte("credential_ref: slack-chat-api/test\n"), 0o600))
+
+	// Plant another config.yml at the OLD hand-rolled path (distinct on
+	// macOS/Windows; identical to new on Linux — which the dedupe handles).
+	oldPath, err := appconfig.OldConfigPath()
+	require.NoError(t, err)
+	require.NoError(t, os.MkdirAll(filepath.Dir(oldPath), 0o700))
+	require.NoError(t, os.WriteFile(oldPath, []byte("credential_ref: slack-chat-api/stale\n"), 0o600))
+
+	_, err = captureOutput(t, func() error { return runClear(&clearOptions{all: true}) })
+	require.NoError(t, err)
+
+	if _, e := os.Stat(newPath); !os.IsNotExist(e) {
+		t.Errorf("new path must be removed by --all: stat err=%v", e)
+	}
+	if _, e := os.Stat(oldPath); !os.IsNotExist(e) {
+		t.Errorf("old path must be removed by --all: stat err=%v", e)
+	}
+}

internal/cmd/config/show.go

@@ -39,7 +39,7 @@ type showStatus struct {
 }
 
 func runShow(_ *showOptions) error {
-	cfg, err := appconfig.Load()
+	cfg, err := appconfig.LoadForRuntime()
 	if err != nil {
 		return err
 	}

internal/cmd/initcmd/init.go

@@ -77,6 +77,20 @@ func runInit(opts *initOptions) error {
 	output.Println("Slack CLI Setup")
 	output.Println()
 
+	// Step -1 (must precede the §1.8 keyring migration): the MON-5372
+	// config-dir relocation gate. If the old hand-rolled dir and the new
+	// statedir-resolved dir both exist with divergent settings, abort
+	// BEFORE any token migration / config write — otherwise we'd soft-read
+	// one side, migrate, then SaveConfig, papering over the conflict. On
+	// only-old we copy old→new (atomic, idempotent).
+	if reloc, err := appconfig.DetectConfigRelocation(); err != nil {
+		return fmt.Errorf("detecting config relocation: %w", err)
+	} else if reloc.CopyNeeded {
+		if err := appconfig.ApplyConfigRelocation(reloc); err != nil {
+			return fmt.Errorf("relocating config from %s to %s: %w", reloc.OldPath, reloc.NewPath, err)
+		}
+	}
+
 	// Opening the store runs the one-time legacy migration (§1.8). With
 	// --overwrite a legacy/keyring conflict is resolved by forcing legacy.
 	open := keychain.Open
@@ -158,7 +172,12 @@ func runInit(opts *initOptions) error {
 	}
 
 	// Persist non-secret config (credential_ref + workspace, §1.2/§2.4).
-	cfg, err := appconfig.Load()
+	// Use LoadForRuntime even here so the dual-return-on-conflict contract
+	// is never exposed to a `if err != nil` callsite: by this point the
+	// init relocation gate above has reconciled or aborted, so soft-degrade
+	// is moot — but routing through LoadForRuntime keeps the read pattern
+	// uniform and the contract harder to misuse.
+	cfg, err := appconfig.LoadForRuntime()
 	if err != nil {
 		return err
 	}

internal/cmd/initcmd/init_test.go

@@ -16,18 +16,18 @@ import (
 	"github.com/open-cli-collective/cli-common/credstore"
 
 	"github.com/open-cli-collective/slack-chat-api/internal/client"
+	appconfig "github.com/open-cli-collective/slack-chat-api/internal/config"
 	"github.com/open-cli-collective/slack-chat-api/internal/keychain"
 	"github.com/open-cli-collective/slack-chat-api/internal/testutil"
 )
 
 // writeLegacyCreds writes the legacy plaintext credentials file at the path
-// the one-time migration scans ($XDG_CONFIG_HOME/slack-chat-api/credentials,
-// per testutil.Setup's isolated XDG).
+// the keychain migrator's legacyCredentialsPath() scans (the hand-rolled
+// XDG/$HOME/.config path — distinct from the new canonical config dir on
+// macOS/Windows post-MON-5372).
 func writeLegacyCreds(t *testing.T, kv map[string]string) string {
 	t.Helper()
-	dir := filepath.Join(os.Getenv("XDG_CONFIG_HOME"), "slack-chat-api")
-	require.NoError(t, os.MkdirAll(dir, 0o700))
-	path := filepath.Join(dir, "credentials")
+	path := testutil.LegacyCredentialsPath(t)
 	var b strings.Builder
 	for k, v := range kv {
 		b.WriteString(k + "=" + v + "\n")
@@ -185,6 +185,99 @@ func TestRunInit_OverwriteResolvesMigrationConflict(t *testing.T) {
 	assert.True(t, os.IsNotExist(statErr), "legacy file must be removed after forced migrate")
 }
 
+// TestRunInit_RelocationGate_OldOnlyCopied pins the MON-5372 init-level
+// contract: when only the old hand-rolled config.yml exists, runInit must
+// copy it to the new canonical dir BEFORE any token migration / save runs.
+// Tests the actual gate ordering through runInit (not just the unit-level
+// Detect+Apply pair), so a regression that removed or reordered the gate
+// would surface here.
+func TestRunInit_RelocationGate_OldOnlyCopied(t *testing.T) {
+	testutil.Setup(t)
+	// On Linux old == new path; the gate is a no-op and the test simply
+	// confirms init still works. On macOS/Windows these diverge and the
+	// gate actually performs the copy.
+	configHome := os.Getenv("XDG_CONFIG_HOME")
+	if configHome == "" {
+		// statedirtest sets it; defensive.
+		t.Skip("hermetic env did not set XDG_CONFIG_HOME")
+	}
+	oldDir := filepath.Join(configHome, "slack-chat-api")
+	require.NoError(t, os.MkdirAll(oldDir, 0o700))
+	require.NoError(t, os.WriteFile(filepath.Join(oldDir, "config.yml"),
+		[]byte("credential_ref: slack-chat-api/from-old\nworkspace: T_OLD_PRE_INIT\n"), 0o600))
+
+	// Run init with no tokens — wizard short-circuits at "no tokens provided"
+	// AFTER the relocation gate has run (and AFTER EnsureMigrated). The gate
+	// must have copied old→new by then if they differ.
+	err := runInit(&initOptions{stdin: strings.NewReader("\nn\n"), noVerify: true})
+	require.NoError(t, err)
+
+	// New canonical path must exist with the old content (or, on Linux,
+	// remain at the same path — same assertion either way).
+	newDir, err := appconfig.Dir()
+	require.NoError(t, err)
+	data, err := os.ReadFile(filepath.Join(newDir, "config.yml"))
+	require.NoError(t, err, "new canonical config.yml must exist after init")
+	assert.Contains(t, string(data), "slack-chat-api/from-old",
+		"new canonical must carry the old config content")
+}
+
+// TestRunInit_RelocationGate_DivergentAbortsBeforeMutation pins the MON-5372
+// fail-loud contract through runInit: divergent old/new config aborts init
+// BEFORE any token migration or config write papers over the conflict. Skips
+// on Linux where old == new path (the gate short-circuits to relocNone).
+func TestRunInit_RelocationGate_DivergentAbortsBeforeMutation(t *testing.T) {
+	testutil.Setup(t)
+	configHome := os.Getenv("XDG_CONFIG_HOME")
+	if configHome == "" {
+		t.Skip("hermetic env did not set XDG_CONFIG_HOME")
+	}
+	// On Linux the resolved new dir IS the XDG-rooted dir — old == new, no
+	// way to construct divergence at the public seam. The unit-level test
+	// already covers this branch on every OS via the injectable newDir.
+	newDir, err := appconfig.Dir()
+	require.NoError(t, err)
+	oldDir := filepath.Join(configHome, "slack-chat-api")
+	if newDir == oldDir {
+		t.Skip("Linux: old path equals new path; divergence covered at unit level")
+	}
+
+	require.NoError(t, os.MkdirAll(oldDir, 0o700))
+	require.NoError(t, os.MkdirAll(newDir, 0o700))
+	require.NoError(t, os.WriteFile(filepath.Join(oldDir, "config.yml"),
+		[]byte("credential_ref: slack-chat-api/old\n"), 0o600))
+	require.NoError(t, os.WriteFile(filepath.Join(newDir, "config.yml"),
+		[]byte("credential_ref: slack-chat-api/new\n"), 0o600))
+
+	// Seed a legacy credentials file too. If the relocation gate were
+	// accidentally moved AFTER keychain.Open, the migrator would consume
+	// this file before the abort and the post-abort stat would fail. With
+	// the gate ordered correctly, the legacy file stays untouched.
+	legacy := writeLegacyCreds(t, map[string]string{"api_token": "xoxb-MUST-NOT-BE-MIGRATED"})
+
+	// Snapshot pre-init bytes to assert mutate-nothing.
+	oldBefore, _ := os.ReadFile(filepath.Join(oldDir, "config.yml"))
+	newBefore, _ := os.ReadFile(filepath.Join(newDir, "config.yml"))
+
+	err = runInit(&initOptions{botEnv: "BOT_TOK", noVerify: true})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "detecting config relocation",
+		"init must abort at the relocation gate")
+
+	// Nothing mutated by the abort.
+	oldAfter, _ := os.ReadFile(filepath.Join(oldDir, "config.yml"))
+	newAfter, _ := os.ReadFile(filepath.Join(newDir, "config.yml"))
+	assert.Equal(t, string(oldBefore), string(oldAfter), "old config must not be mutated by aborted init")
+	assert.Equal(t, string(newBefore), string(newAfter), "new config must not be mutated by aborted init")
+
+	// Critical proof the gate ran BEFORE keychain.Open / migration: the
+	// legacy credentials file must still exist. If migration had run, it
+	// would have consumed and removed this file.
+	if _, statErr := os.Stat(legacy); statErr != nil {
+		t.Errorf("legacy credentials file must still exist (gate must abort before migration); stat err=%v", statErr)
+	}
+}
+
 func TestRunInit_VerificationFailed(t *testing.T) {
 	testutil.Setup(t)
 	s := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {