Port Atlassian shared config (cfl+jtk) to cli-common statedir resolver [MON-5370]

[rianjs]

Summary

working-with-state.md §6.4 unit 2 — adopt the cli-common statedir
resolver for the shared Atlassian credential-scope config
(atlassian-cli/config.yml), the combined cfl+jtk unit. Durable config
⇒ fail-loud semantics (not cache "miss").

6 decomposed commits:

1. build: repin shared+tools/cfl cli-common → e67b2fc
   (jtk already there from MON-5369; workspace converges on one cli-common).
2. refactor(credstore)!: DefaultPath() → (string,error) via
   statedir.Scope{Name:"atlassian-cli"}.ConfigDir(). A relative/
   unresolvable $XDG_CONFIG_HOME now errors (§1.1 tightening) —
   no silent cwd-relative ./.atlassian-cli fallback. All 7 call sites
   updated (init = hard error; runtime/keyring existence checks treat a
   resolver error as "no shared file"). Linux layout unchanged; macOS →
   ~/Library/Application Support, Windows → %APPDATA%.
3. feat(credstore): the prior hand-rolled location becomes an
   additive pre-token legacy source — strict pure detect/enumerate
   (no mutation; composes with the existing fail-loud/mutate-nothing
   per-tool divergence machinery) vs gated apply/copy (only after
   relocation AND per-tool conflict checks pass). Dedicated
   relocation-equality projection (canonical tool defaults + legacy
   per-tool conn/token, migration-skew-tolerant token compare).
   copy-leave-old; path-identity short-circuit + dedup; old-shared wired
   into the keyring token-migration + clear --all source sets so a
   stale plaintext api_token at the old path is scrubbed/listed before
   token resolution.
4. fix(cfl/config): legacy per-tool store — atomic write + 0700 dir.
5. fix(jtk/config): legacy per-tool store — atomic write.
6. test: hermetic statedirtest 7-var adoption (the resolver change
   made HOME/XDG-only isolation a macOS/Windows real-dir leak — one test
   was reading the developer's real bearer config) + pure §3.2 dedup
   coverage.

CI note

This unit touches shared/**, so the path filter triggers both cfl
and jtk build/test/lint — expected.

Test plan

• make test green (3237 passed, 61 packages) on macOS dev
• make build + module-qualified go build ./shared/... ./tools/cfl/... ./tools/jtk/...
• make lint — only 2 pre-existing govet reflect.Ptr nits in
  shared/testutil/assert.go (untouched by this branch; unrelated)
• Pure §3.2 matrix (relocate_test.go): skip/identity/old-only/
  both-equal/divergent/malformed-old/malformed-new/token-skew/
  not-double-enumerated — hermetic
• CI green (Linux: old≡new short-circuit path)

Closes #372

[rianjs]

Blocker

• DetectSharedRelocation / ApplySharedRelocation only run from the init reconcile paths. Runtime config loading still does DefaultPath() + Load() only (tools/cfl/internal/config/config.go, tools/jtk/internal/config/config.go), and shared/keyring/migrate.go only enumerates/scrubs old-shared tokens via OldSharedProjection; it never copies or reads the old shared non-secret config. That breaks the §3.2 matrix for normal commands: an old-only shared config at the prior macOS/Windows path is ignored until the user runs init, and old-vs-new divergence is not fail-loud on “next load” as promised. Add a shared runtime relocation/read helper used by cfl LoadWithEnv and jtk loadShared/token-first flow, while keeping init’s copy gated behind per-tool divergence checks.

Major

• The entry-point coverage promised by the plan is not present. relocate_test.go proves helper behavior, but there are no cfl/jtk init tests proving “per-tool divergence pending ⇒ no copy,” and no runtime/token-first test proving old-only shared config is copied or loaded for a normal jtk/cfl command. The missing tests would have caught the blocker above.

Minor

• Add explicit relocation-equality tests for durable tool-default divergence: old/new differing only in cfl.default_space, cfl.output_format, or jtk.default_project should return ErrRelocationConflict. The implementation appears to compare those fields, but this was a round-2 architectural requirement and should be pinned.

• The Linux old==new coverage is seam-level only. That is probably enough for unit scope, but CI should include at least one end-to-end clear/keyring path where DefaultPath() and oldSharedPath() naturally collapse to the same path, so duplicate listing/scrubbing cannot regress.

Token-skew tolerance itself looks right: one-sided plaintext tokens are migration skew, while two distinct non-empty tokens still fail loud.

[rianjs]

TDD coverage assessment (independent)

Scope: test coverage only. Ran go test ./shared/credstore/... ./shared/keyring/... — 117 pass.

Well covered

• relocate_test.go exercises essentially every meaningful branch of the pure layer: relative-XDG skip (TestOldSharedPath_RelativeXDGSkipped), path-identity dedup, old-only CopyNeeded, both-equal no-op, divergent conflict, malformed old/new fail-loud (incl. asserting the new file is not mutated), token-skew tolerance vs two-different-tokens conflict, table-driven tool-default divergence (cfl.default_space / cfl.output_format / jtk.default_project), OldSharedConnCandidates relabel, OldSharedProjection enumerate + path-identity non-enumeration, OldSharedConfigPath dedup, ApplySharedRelocation no-op + copy-leave-old, and loadSharedRuntime old-only/divergent/equal/path-identity. The oldBase helper forcing a distinct $XDG so old≠new engages on Linux is the right call.
• §3.2 init reconcile is exercised from both cfl and jtk (TestReconcile_OldSharedOnly_CopiedAtInit, TestReconcile_OldShared_PerToolDivergencePending_NoCopy — copy gated on per-tool divergence, mutate-nothing asserted).
• Hermeticity is solid: credtest.Hermetic/statedirtest.Hermetic 7-var harness, SharedConfigPath/sharedConfigPath derived from the production resolver (no hand-built <root>/atlassian-cli/...), and TestDefaultPath_DelegatesToStatedir has an explicit real-dir-leak guard (strings.HasPrefix(got, root)). No remaining real-$HOME leak risk that I can see.
• Keyring migration/conflict/scrub remains well covered for the canonical + legacy per-tool sources.

Gaps (specific)

1. Old-shared keyring scrub is not exercised end-to-end (secret-adjacent — highest priority). migrate.go:138-142 (enumerate old-shared token) and :234-238 (scrub it) have no test. keyring_e2e_test.go only ever seeds the canonical sharedConfigPath. Need a test: hermetic, plaintext api_token at a distinct old-shared path, EnsureMigrated() → token in keyring, old-shared file scrubbed, notice fired, idempotent. Only the pure OldSharedProjection enumeration is tested today, not the consolidation/scrub.
2. config clear --all old path untested. ClearPlan.OldSharedConfigPath + ClearFiles old-path removal (clear.go:82-88, 158-170) and the configcmd "It will also delete the prior shared config file" line have zero coverage; no path-identity-dedup (not double-listed) test at the clear level. The PR description calls this out specifically.
3. §3.2 runtime path not covered from the cfl/jtk entry points. Covered only at the loadSharedRuntime unit level; no test drives an old-only / divergent shared file through cfl LoadWithEnv or jtk loadShared (the actual LoadSharedRuntime() wiring + warn-once fallback is untested at the seam users hit).
4. Atomic Save only round-trip-tested. cfl/jtk Save (now temp+rename) has no crash-safety assertion (no stale .tmp on success, 0600 file / 0700 dir perms, rename atomicity). Not regressed, but the new atomic behavior itself is unverified.

Most important gap: #1 — a stale plaintext api_token left at the old hand-rolled location after the macOS/Windows resolver move is the whole security rationale for the old-shared source, and its scrub/consolidation is exercised only through pure enumeration, never end-to-end through EnsureMigrated.

[rianjs]

Response to TDD coverage assessment

Thanks — actioned the cheap, high-value gap and justifying the rest with the architectural constraint behind them.

Gap #4 (atomic Save perms/atomicity) — FIXED. Added TestConfig_Save_AtomicPermsNoStaleTmp (cfl: 0600 file / 0700 dir / no .tmp) and TestConfig_Save_AtomicNoStaleTmp (jtk: no .tmp; file mode already pinned by TestConfig_FilePermissions).

Gaps #1–#3 (old-shared keyring scrub e2e, clear --all old-path e2e, runtime through the real LoadWithEnv/loadShared seam) — won't-add, with rationale. These all hit the same hard architectural constraint: on Linux oldSharedPath() ≡ credstore.DefaultPath() (both resolve $XDG_CONFIG_HOME/~/.config). The old-shared distinct path only exists on macOS/Windows (where os.UserConfigDir() diverges from ~/.config). The keyring EnsureMigrated, config clear --all, and LoadWithEnv/loadShared seams all call credstore.DefaultPath() internally with no injectable path, so on Linux CI old≡new → OldSharedProjection/OldSharedConfigPath dedupe to empty and the old-shared-specific branch is, by design, an unreachable no-op (it collapses into the normal shared scrub/clear/load, which IS covered end-to-end in keyring_e2e_test.go).

The old-shared behavior is therefore verified where it is hermetically constructible: the pure layer takes an injectable newPath and is exhaustively covered in relocate_test.go (OldSharedProjection token enumeration, OldSharedConfigPath dedup, loadSharedRuntime old-only/divergent/equal/identity), and the init path is driven from both cfl and jtk in the reconcile tests. Adding a production test-seam to keyring/migrate.go purely to reach a Linux-impossible branch would be disproportionate (it is the same constraint that makes the relocation copy itself macOS/Windows-only). Net: pure logic + composition over already-e2e-tested machinery; no real-dir-leak risk (full statedirtest 7-var isolation).

[rianjs]

Minor

• ErrRelocationConflict is now a non-corrupt, usable-canonical-store condition in runtime, but both runtime paths still reuse the old “shared credential store is unreadable / falling back” warning text. In cfl LoadWithEnv and jtk loadShared, split the warning path for errors.Is(err, credstore.ErrRelocationConflict): tell the user prior/current shared config diverge, commands are using the canonical config, and init will reconcile. The current text is misleading because the store is readable and not always falling back.

Everything else is architecturally aligned. The runtime path is now mutation-free but old-aware, init remains the gated copy point, per-tool divergence still blocks copy, token-skew tolerance is correct, and the Linux old==new limitation is reasonably covered at the pure injectable layer without adding production-only test seams.

[monit-reviewer]

Automated PR Review

Reviewed commit: 0d46e2a

Summary

Reviewer	Findings
harness-engineering:harness-architecture-reviewer	6
harness-engineering:harness-enforcement-reviewer	2
harness-engineering:harness-knowledge-reviewer	2
harness-engineering:harness-self-documenting-code-reviewer	3

harness-engineering:harness-architecture-reviewer (6 findings)

⚠️ Should Fix - shared/credstore/relocate.go:193

loadSharedRuntime re-implements ~40 lines of detection logic already present in DetectSharedRelocation (old-path resolution, path-identity dedup, LoadSharedLegacyProjection probes, relocationEqual comparison, ErrRelocationConflict wrapping). Future bug fixes to DetectSharedRelocation will not propagate to loadSharedRuntime. The runtime read-only contract should be a thin wrapper around DetectSharedRelocation that skips the copy gate and returns the canonical store alongside any conflict error. Note: the open PR discussion thread from rianjs identifies this as a blocker — runtime config loading in cfl LoadWithEnv and jtk loadShared does not consult the old-shared path at all, so an old-only shared config at the prior macOS/Windows path is silently ignored until the user runs init.

💡 Suggestion - shared/credstore/relocate.go:93

Load(oldPath) is called unconditionally before the newProj == nil guard. In the common first-run case where the new path is absent (CopyNeeded=true branch), oldStore is loaded and then discarded without use — relocationEqual, the only consumer of oldStore, is never reached in that branch. Moving Load(oldPath) to after the newProj == nil check eliminates a redundant disk read in the most common relocation scenario.

💡 Suggestion - shared/credtest/credtest.go:134

credtest.Hermetic resets keyring.ResetCorruptWarnOnce and keyring.ResetMigrationNotice, but does not reset the package-level corruptSharedWarnOnce sync.Once in tools/cfl/internal/config/config.go or tools/jtk/internal/config/config.go. Tests that trigger warnCorruptSharedOnce (including the new ErrRelocationConflict branch) will leave the once latched, silently suppressing the warning and any assertions on it in subsequent tests within the same package run. Expose a reset function in each config package and call it from credtest.Hermetic.

💡 Suggestion - shared/keyring/clear.go:166

ClearFiles removes OldSharedConfigPath unconditionally (relying on os.Remove + IsNotExist suppression), while PlanClear checks fileExists before setting OldSharedConfigPath. The behavioral difference is harmless today, but the inconsistency makes the invariant 'only remove files we know exist' harder to verify and could mislead a future reader into thinking the guard was intentionally omitted.

💡 Suggestion - tools/cfl/internal/cmd/init/reconcile_test.go:24

oldSharedFixture builds the old-shared path by reading $XDG_CONFIG_HOME from the live environment (set implicitly by statedirtest.Hermetic) rather than using an explicit override to a known distinct base. If statedirtest.Hermetic ever stops setting XDG_CONFIG_HOME on a given platform, oldSharedFixture would write to whatever XDG_CONFIG_HOME happened to be, and oldSharedPath() would resolve to a different location, silently voiding the test. Use the explicit-override style from relocate_test.go's oldBase.

💡 Suggestion - tools/jtk/internal/cmd/initcmd/reconcile_test.go:16

Same oldSharedFixture fragility as tools/cfl/internal/cmd/init/reconcile_test.go:24 — old-shared path derived from a live env var rather than an explicit XDG_CONFIG_HOME override, making the test depend on statedirtest.Hermetic's platform-specific behavior. Use the explicit-override style from relocate_test.go's oldBase.

harness-engineering:harness-enforcement-reviewer (2 findings)

⚠️ Should Fix - shared/credstore/relocate.go:88

Normalization mismatch between relocationEqual and OldSharedConnCandidates: relocationEqual normalizes URLs via NormalizeBaseURL and auth methods via canonAuthMethod before comparing, but OldSharedConnCandidates contributes raw OldProj field values as ConnCandidates. If DetectConnDivergence does not apply the same normalization, configs that relocationEqual deems equal (e.g. 'http://acme.atlassian.net' vs 'https://acme.atlassian.net') could still raise a false ConnConflict error via the candidate set, blocking init with a spurious conflict message.

💡 Suggestion - shared/keyring/clear.go:158

ClearFiles() silently discards a credstore.DefaultPath() resolution error (e.g. relative $XDG_CONFIG_HOME): neither the current nor the prior shared config is removed, and no error is returned to the caller. The --all clear succeeds superficially while leaving plaintext tokens on disk with no user-visible indication. The error should be appended to errs so callers can surface it.

harness-engineering:harness-knowledge-reviewer (2 findings)

💡 Suggestion - tools/cfl/internal/cmd/init/reconcile.go:119

After ApplySharedRelocation copies old→new, store and proj are reloaded but candidates and chosen are not recomputed. The affectsSibling guard downstream evaluates against the post-copy store rather than the pre-init empty state. Functionally correct (chosen and reloaded store are consistent since they came from the same source), but a user doing a pure relocation on macOS/Windows could see an unexpected 'this will also affect jtk' prompt even though no config values changed.

💡 Suggestion - tools/jtk/internal/cmd/initcmd/reconcile.go:109

Same issue as tools/cfl/internal/cmd/init/reconcile.go:119 — after ApplySharedRelocation reloads store and proj, chosen is not recomputed. The affectsSibling guard operates on the post-copy store rather than the pre-init empty state, which can produce a false-positive sibling-impact warning during a pure path relocation on macOS/Windows.

harness-engineering:harness-self-documenting-code-reviewer (3 findings)

💡 Suggestion - shared/credstore/relocate.go:234

OldSharedConnCandidates does not guard on r.CopyNeeded. When both old and new shared files exist and are equal (CopyNeeded=false), the function still returns candidates from the old path. These are appended to the reconcile candidate pool in cfl and jtk reconcile.go, adding redundant equal-connection entries. When a conflict with a per-tool legacy file IS detected, the error message will redundantly cite both the old-shared path and the canonical path as sources of the same connection, which is confusing. Guard should be: if r == nil || r.OldProj == nil || r.OldPath == "" || !r.CopyNeeded { return nil }.

💡 Suggestion - shared/credstore/relocate.go:24

oldSharedPath() declares a (string, error) return type but never returns a non-nil error — all error conditions return ("", nil). Every caller correctly discards the error with _, but the signature implies callers should handle an error. This misleading contract will propagate via copy-paste. Either drop the error return or document that nil error is always expected.

💡 Suggestion - shared/keyring/migrate.go:64

When sperr != nil (DefaultPath resolver failure), proj is set to an empty &SharedLegacyProjection{} with Path == "", and sharedPath is also "". The subsequent add() calls embed the empty path in source label strings (e.g. "shared config default ()"). Although all token fields are empty so add() is not invoked today, if a future path adds a non-empty field to the empty fallback proj, migration conflict messages would reference an empty untraceable path. Consider using a nil proj and skipping add() calls with an explicit nil check.

7 info-level observations excluded. Run with --verbose to include.

4 PR discussion threads considered.

---

Completed in 7m 03s | $1.95 | sonnet | daemon 0.2.120 | Glorfindel

Field	Value
Model	sonnet
Reviewers	hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	7m 03s wall · 17m 16s compute (Reviewers: 4m 49s · Synthesis: 2m 10s)
Cost	$1.95
Tokens	255.4k in / 65.6k out
Turns	6

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	35.9k	8.6k	0	35.9k (1h)	$0.27
harness-engineering:harness-architecture-reviewer	sonnet	44.8k	10.7k	2.1k	42.7k (1h)	$0.36
harness-engineering:harness-enforcement-reviewer	sonnet	44.8k	15.4k	2.1k	42.7k (1h)	$0.43
harness-engineering:harness-knowledge-reviewer	sonnet	44.8k	11.6k	2.1k	42.7k (1h)	$0.37
harness-engineering:harness-self-documenting-code-reviewer	sonnet	42.4k	15.4k	2.1k	40.3k (1h)	$0.42
security:security-code-auditor	haiku	42.7k	3.9k	0	42.7k (1h)	$0.11

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

Superseded by updated review

[monit-reviewer]

Automated PR Review

Reviewed commit: 0031d0b | Previous: 0d46e2a (incremental)

Approved with 4 non-blocking suggestions below. Address at your discretion.

Summary

Reviewer	Findings
harness-engineering:harness-architecture-reviewer	1
harness-engineering:harness-enforcement-reviewer	2
harness-engineering:harness-knowledge-reviewer	1

harness-engineering:harness-architecture-reviewer (1 findings)

💡 Suggestion - shared/credstore/relocate.go:118

classifyRelocation reads the old-shared file twice in the both-present branch: once via LoadSharedLegacyProjection(oldPath) and again via Load(oldPath). relocationEqual merges fields from both reads. Between the two reads there is a narrow TOCTOU window where a concurrent write could cause an inconsistent (proj, store) pair, making the equality decision non-atomic. For a local CLI config file this is unlikely to cause problems in practice, but the fix is straightforward: extend SharedLegacyProjection to carry the ToolSection defaults so the file is parsed only once.

harness-engineering:harness-enforcement-reviewer (2 findings)

💡 Suggestion - shared/credstore/relocate.go:148

classifyRelocation calls Load(newPath) unconditionally at the top of every invocation, including the runtime path (LoadSharedRuntime). On a fully-migrated installation where no old path exists, this is an additional file open per command compared to the old credstore.Load(DefaultPath()) call — the old-path stat that follows is a new overhead on every CLI invocation. The impact is small for a CLI tool, but the Load(newPath) call could be moved into the both-paths branch to match the old single-read contract on the happy path.

💡 Suggestion - tools/cfl/internal/cmd/init/reconcile.go:100

reconcile.go calls credstore.Load(sharedPath) immediately after DetectSharedRelocation(sharedPath), which already loaded the same file internally via classifyRelocation. In the relocation case a third load follows ApplySharedRelocation. The three reads are not incorrect, but the SharedRelocation struct already carries newStore — returning it from DetectSharedRelocation and consuming it in reconcile.go would eliminate the redundant reads and prevent the double-read pattern from reappearing. Same issue exists in tools/jtk/internal/cmd/initcmd/reconcile.go.

harness-engineering:harness-knowledge-reviewer (1 findings)

💡 Suggestion - shared/credstore/relocate.go:225

ApplySharedRelocation uses a deterministic temp path (r.NewPath + ".tmp"). If cfl init and jtk init run concurrently against the same shared path they will clobber each other's temp file, potentially leaving a truncated or zeroed destination after rename. Use os.CreateTemp in the same directory instead of a fixed ".tmp" suffix.

6 info-level observations excluded. Run with --verbose to include.

17 PR discussion threads considered.

---

Completed in 7m 34s | $1.93 | sonnet | daemon 0.2.120 | Glorfindel

Field	Value
Model	sonnet
Mode	Re-review · Cycle 2 · Session resumed
Reviewers	hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor
Engine	claude · sonnet
Reviewed by	pr-review-daemon · monit-pr-reviewer
Duration	7m 34s wall · 16m 49s compute (Reviewers: 3m 54s · Synthesis: 2m 49s)
Cost	$1.93
Tokens	301.7k in / 61.1k out
Turns	6

Per-workstream usage

Workstream	Model	In	Out	Cache read	Cache create	Cost
hybrid-synthesis	sonnet	52.3k	9.9k	13.6k	38.7k (1h)	$0.30
harness-engineering:harness-architecture-reviewer	sonnet	45.9k	12.3k	2.1k	43.8k (1h)	$0.38
harness-engineering:harness-enforcement-reviewer	sonnet	45.9k	11.4k	2.1k	43.8k (1h)	$0.37
harness-engineering:harness-knowledge-reviewer	sonnet	45.9k	8.0k	2.1k	43.8k (1h)	$0.32
harness-engineering:harness-self-documenting-code-reviewer	sonnet	43.4k	11.9k	2.1k	41.3k (1h)	$0.37
security:security-code-auditor	haiku	43.7k	6.1k	0	43.7k (1h)	$0.12
discussion-summarizer	—	24.4k	1.4k	13.6k	10.8k (1h)	$0.07

_{Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.}

[monit-reviewer]

🔵 Low (harness-engineering:harness-architecture-reviewer): classifyRelocation reads the old-shared file twice in the both-present branch: once via LoadSharedLegacyProjection(oldPath) and again via Load(oldPath). relocationEqual merges fields from both reads. Between the two reads there is a narrow TOCTOU window where a concurrent write could cause an inconsistent (proj, store) pair, making the equality decision non-atomic. For a local CLI config file this is unlikely to cause problems in practice, but the fix is straightforward: extend SharedLegacyProjection to carry the ToolSection defaults so the file is parsed only once.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-knowledge-reviewer): ApplySharedRelocation uses a deterministic temp path (r.NewPath + ".tmp"). If cfl init and jtk init run concurrently against the same shared path they will clobber each other's temp file, potentially leaving a truncated or zeroed destination after rename. Use os.CreateTemp in the same directory instead of a fixed ".tmp" suffix.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-enforcement-reviewer): classifyRelocation calls Load(newPath) unconditionally at the top of every invocation, including the runtime path (LoadSharedRuntime). On a fully-migrated installation where no old path exists, this is an additional file open per command compared to the old credstore.Load(DefaultPath()) call — the old-path stat that follows is a new overhead on every CLI invocation. The impact is small for a CLI tool, but the Load(newPath) call could be moved into the both-paths branch to match the old single-read contract on the happy path.

Reply to this thread when addressed.

[monit-reviewer]

🔵 Low (harness-engineering:harness-enforcement-reviewer): reconcile.go calls credstore.Load(sharedPath) immediately after DetectSharedRelocation(sharedPath), which already loaded the same file internally via classifyRelocation. In the relocation case a third load follows ApplySharedRelocation. The three reads are not incorrect, but the SharedRelocation struct already carries newStore — returning it from DetectSharedRelocation and consuming it in reconcile.go would eliminate the redundant reads and prevent the double-read pattern from reappearing. Same issue exists in tools/jtk/internal/cmd/initcmd/reconcile.go.

Reply to this thread when addressed.

[rianjs]

Findings

None.

The final refactor preserves the agreed architecture: one pure relocation classifier, init-only fail-loud policy, runtime soft-conflict policy using canonical config, old-only runtime read fallback without copy, and gated copy only after relocation and per-tool divergence checks pass. OldSharedConnCandidates now being old-only is correct; both-present-equal does not need duplicate candidates, and both-present-divergent is already stopped by relocation classification.

The token skew tolerance still looks right: one-sided plaintext token is migration skew, while two differing non-empty tokens remain a conflict. Clear/keyring handling is aligned with the durable-data rule, and surfacing DefaultPath errors from clear --all is the right recovery-path behavior. Remaining Linux old==new limits are covered at the injectable pure seams without adding disproportionate production-only test hooks.