From b8ea62a3c397d60286216fc34a845796d52fb169 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 20 May 2026 07:50:57 -0400 Subject: [PATCH 1/4] =?UTF-8?q?docs(state):=20finalize=20=C2=A76=20step=20?= =?UTF-8?q?6=20+=20tier-2=20deferral=20for=20INT-310=20tag-before-close=20?= =?UTF-8?q?[MON-5375]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds §6.6 close-out retrospective: 5 of 6 port units shipped (jtk-cache, Atlassian shared config, gro, slck, nrq); sfdc parked and explicitly excluded from the v0.1.0 train (no cli-common dependency today). Commons API additions during the rollout: cache.WriteEnvelope (#20) and underscore in the cache-component regex (#21). Both additive; no breaking change required across 5 ports. Codifies the ten reusable patterns that emerged across the matrix: mutation-free Detect/Apply split, Load/LoadForRuntime + cfg!=nil contract, unexported xxxFromNewDir testable seam, malformed-old fails loud before CopyNeeded, companion-plaintext-file dual-probe with parsed-projection equality, 7-var statedirtest.Hermetic, the cleanup- command recovery contract (cleanup must not be blocked by the broken state it exists to wipe), init-gate ordering proof, reflect.DeepEqual on default-applied Config, path-identity dedup. Reaffirms §5b tier-2 deferral: only jtk needs the registry/DAG/fetcher shape today (gro uses tier-1 only; cfl has no cache). Promoting from one consumer would just relocate jtk code. Re-evaluate when cfl gains a cache. Closes #19 --- docs/working-with-state.md | 115 ++++++++++++++++++++++++++++++++++++- 1 file changed, 114 insertions(+), 1 deletion(-) diff --git a/docs/working-with-state.md b/docs/working-with-state.md index 87ae715..9c40652 100644 --- a/docs/working-with-state.md +++ b/docs/working-with-state.md @@ -324,6 +324,12 @@ not designed, until cfl forces the question (rule of three).** documented copied pattern*. **Tier-2 promotion criteria (record now):** ≥2 consumers need the same registry/DAG shape AND the API has been stable across one full port cycle without per-consumer special-casing. + **INT-310 close-out (MON-5375, 2026-05-20): continue deferral.** Only jtk + needs the tier-2 registry/DAG/fetcher/refresh shape today — gro uses + tier-1 primitives only, and cfl has no cache. Promoting from a single + consumer would just relocate jtk code without surfacing a shared + abstraction. Re-evaluate when cfl gains a cache (or any third consumer + needs the same shape). > **The commons API co-evolves during the ports — under a hard guardrail.** > Each port may surface a constraint that generalizes (a) or tier 1; the jtk @@ -415,6 +421,111 @@ act* — done together (not two horizontal sweeps). new `cli-common` state package + this doc (credstore itself is unchanged here); repin consumers per the §5 matrix rule. 6. Finalize this doc from what survived; make the tier-2 call. + **DONE 2026-05-20 (MON-5375):** see §6.6 below for the retrospective and §5b for the tier-2 call. + +### 6.6 INT-310 close-out retrospective (MON-5375, 2026-05-20) + +The state workstream shipped 5 of 6 planned port units against the +candidate cli-common SHA `e67b2fc81f9d7072679d8cd77098121ed6f15f47` (no +breaking changes required). With the matrix green per §5, **cli-common +`v0.1.0` is the INT-310 state baseline tag** — first stable for +`statedir` + `statedirtest` + `cache` (tier 1), alongside the +already-shipped `credstore` and both standards docs. + +**Ports shipped (all status Deployed):** +| Unit | Repo SHA | Ticket | PR | +|------|----------|--------|-----| +| jtk cache re-migration | atlassian-cli@`59a2fb9` | MON-5369 | #371 | +| Atlassian shared config (cfl+jtk combined) | atlassian-cli@`b867c0e` | MON-5370 | #372 | +| gro (config→statedir + cache→cli-common/cache + drop user-TTL) | google-readonly@`1f472eb` | MON-5371 | #134 | +| slck (resolver + atomic + perms) | slack-chat-api@`2619ba3` | MON-5372 | #165 | +| nrq (resolver + dual-probe credentials file) | newrelic-cli@`128c84d` | MON-5373 | #101 | + +**Parked:** MON-5374 sfdc (salesforce-cli#42). sfdc has no cli-common +dependency today, so it does not ride the v0.1.0 train; it will adopt +the tag when un-parked. This is the explicit "resolve or exclude" +disposition the GH issue called for. + +**Commons API additions during ports (both additive — Codex-cleared per §5):** +- `cache.WriteEnvelope` (cli-common#20, SHA `2c4a5b8`) — verbatim atomic writer; jtk-facade required it to relocate envelopes byte-for-byte from `~/.jtk/cache`. +- Underscore allowed in `cache` component regex `^[A-Za-z0-9][A-Za-z0-9._\-]*$` (cli-common#21, SHA `e67b2fc`) — jtk's pre-port instance keys contain underscores. + +No exported symbol changed behavior across the 5 ports; the API was +stable enough that no train-coordinated repin was needed mid-rollout. + +**Reusable port patterns (codified here so future state-touching work +inherits them):** + +1. **Mutation-free `DetectXxxRelocation` + gated `ApplyXxxRelocation`**: + detection runs anywhere (init AND runtime); copy runs only at init's + pre-write gate. Mixing the two surfaces was the MON-5371 plan-r1 + blocker — kept separate ever since. + +2. **`Load` / `LoadForRuntime` split with `cfg != nil` soft-degrade + contract** (MON-5371): mutating callers (init post-gate, `config + set`, migrating `keychain.Open`) use strict `Load`; pure readers + (`config show`, `OpenNoMigrate`, runtime resolution) use + `LoadForRuntime` which warns-and-soft-degrades ONLY when canonical + was readable. Malformed canonical under conflict MUST hard-fail — + warning + defaulting would silently swap `CredentialRef` back to + the default and mask the corruption. + +3. **Unexported `xxxFromNewDir` testable seam** (MON-5372 trap-fix): + production AND tests call the same path. A parallel test helper + that re-implements the public seam lets production regress while + tests still pass — the unexported `loadFromNewDir(newDir)` / + `loadForRuntimeFromNewDir(newDir)` pair makes that impossible. + +4. **Old-only-malformed fails loud BEFORE `CopyNeeded`** (MON-5371): + if the OLD path is unparseable, `ApplyConfigRelocation` would + propagate corrupt bytes to the new dir. Validate-then-mark-copy. + +5. **Companion-plaintext-file dual-probe** (MON-5373): when the + resolver switch also relocates a secret-bearing companion file + (gro: `token.json`; nrq: `credentials`), the migrator MUST probe + BOTH old and new locations via a shared `CredentialFileCandidates` + /`OldHandRolledXxxPath`/`CanonicalXxxPath` trio so migrator and + `clear --all` cannot drift. Equality is **parsed-projection**, not + byte-equal (ordering / trailing-newline differences are harmless). + +6. **7-var `statedirtest.Hermetic`** mandatory: HOME/XDG-only test + isolation leaks the developer's real `~/Library/Application + Support` / `%APPDATA%` paths on macOS/Windows. Caught a real-dir + leak in MON-5370 (cfl+jtk shared test was reading the dev's actual + bearer config). `t.Parallel`-unsafe — use sequentially. + +7. **Cleanup-command recovery contract** (MON-5372/5373): `clear + --all` (and equivalents) is the user's primary recovery path — + it must not itself be blocked by the broken state it exists to + wipe. Resolve canonical + old paths up front WITHOUT calling + `Load`; store-open is best-effort under `--all` so an unparseable + canonical / invalid `credential_ref` / invalid `keyring.backend` + does not stall the file scrub. Pinned by + `TestConfigClear_All_MalformedConfig_StillScrubsFiles` (nrq). + +8. **Init-gate ordering proof** (MON-5372/5373): a gate-vs-migration + ordering test seeds a legacy artifact (credentials file / token) + AND asserts the error message contains the gate-specific wrapper. + Either alone is insufficient — the artifact-untouched check passes + even if a downstream strict-`Load` rejected; the message-only check + passes if the gate ran but didn't run BEFORE migration. Both + assertions together prove ordering. + +9. **`reflect.DeepEqual` on default-applied Config**: material + equality compares the full default-applied struct — future Config + fields auto-trigger divergence with no manual comparator + maintenance. + +10. **Path-identity dedup**: Linux collapses old≡new (statedir ≡ + `$XDG_CONFIG_HOME`); operations on both paths must dedupe so + they don't double-act. The dedup happens at the candidate-list + level (`CredentialFileCandidates`, `configPathsForClear`) so + every consumer naturally inherits it. + +These ten patterns are the durable INT-310 deposits. Future +state-touching work (sfdc un-parking, cfl future cache, any new CLI +adopting the resolver) should inherit them by reading this section — +not by rediscovering them per port. --- @@ -453,7 +564,9 @@ act* — done together (not two horizontal sweeps). `XDG_CONFIG_HOME`, `XDG_CACHE_HOME`, `XDG_DATA_HOME`) — `HOME`-only is a Windows real-dir leak. Load-bearing; ships once in `cli-common`. - [x] **Tier-2 cache extraction (§5b): deferred** to post-cfl; promotion - criteria recorded in §5b. + criteria recorded in §5b. **Reaffirmed 2026-05-20 (MON-5375):** + tier-2 shape has only one consumer (jtk); promotion criteria + explicitly unsatisfied. Re-evaluate when cfl gains a cache. - [x] **Doc home:** `cli-common/docs/`, versioned + pinned with the code. - [x] **Cache location:** `os.UserCacheDir()/`; jtk re-migrates; gro (B2b) stays. From 5bc9a39d1bed618d414a2340041026df52f39f28 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 20 May 2026 07:54:04 -0400 Subject: [PATCH 2/4] fixup: Codex r1 factual corrections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Atlassian PR numbers: #371→#373 (MON-5369 jtk-cache), #372→#374 (MON-5370 shared config). SHAs were already correct. - Pattern 5 (companion-secret-file dual-probe): format-dependent equality, not a single rule — text key=value uses parsed projection (nrq); opaque blobs use trimmed raw serialized value (gro's token.json). Generic shape described as old/new candidate enumeration + path-identity dedup, not nrq's specific helper trio. - Pattern 9 (material equality): use whole-struct DeepEqual on default-applied Config ONLY when every field is directly comparable and user-meaningful (slck/nrq); build an explicit projection when fields need semantic normalization (gro: OAuthClientPath default equivalence, sorted GrantedScopes). - §5b tier-2 reaffirmation: "second tier-2-shape consumer," not "third" (the criteria are ≥2 consumers). --- docs/working-with-state.md | 44 +++++++++++++++++++++++++------------- 1 file changed, 29 insertions(+), 15 deletions(-) diff --git a/docs/working-with-state.md b/docs/working-with-state.md index 9c40652..c50c9df 100644 --- a/docs/working-with-state.md +++ b/docs/working-with-state.md @@ -328,8 +328,8 @@ not designed, until cfl forces the question (rule of three).** needs the tier-2 registry/DAG/fetcher/refresh shape today — gro uses tier-1 primitives only, and cfl has no cache. Promoting from a single consumer would just relocate jtk code without surfacing a shared - abstraction. Re-evaluate when cfl gains a cache (or any third consumer - needs the same shape). + abstraction. Re-evaluate when cfl gains a cache, or any second + tier-2-shape consumer (registry/DAG/fetcher/refresh) appears. > **The commons API co-evolves during the ports — under a hard guardrail.** > Each port may surface a constraint that generalizes (a) or tier 1; the jtk @@ -435,8 +435,8 @@ already-shipped `credstore` and both standards docs. **Ports shipped (all status Deployed):** | Unit | Repo SHA | Ticket | PR | |------|----------|--------|-----| -| jtk cache re-migration | atlassian-cli@`59a2fb9` | MON-5369 | #371 | -| Atlassian shared config (cfl+jtk combined) | atlassian-cli@`b867c0e` | MON-5370 | #372 | +| jtk cache re-migration | atlassian-cli@`59a2fb9` | MON-5369 | #373 | +| Atlassian shared config (cfl+jtk combined) | atlassian-cli@`b867c0e` | MON-5370 | #374 | | gro (config→statedir + cache→cli-common/cache + drop user-TTL) | google-readonly@`1f472eb` | MON-5371 | #134 | | slck (resolver + atomic + perms) | slack-chat-api@`2619ba3` | MON-5372 | #165 | | nrq (resolver + dual-probe credentials file) | newrelic-cli@`128c84d` | MON-5373 | #101 | @@ -480,13 +480,19 @@ inherits them):** if the OLD path is unparseable, `ApplyConfigRelocation` would propagate corrupt bytes to the new dir. Validate-then-mark-copy. -5. **Companion-plaintext-file dual-probe** (MON-5373): when the - resolver switch also relocates a secret-bearing companion file - (gro: `token.json`; nrq: `credentials`), the migrator MUST probe - BOTH old and new locations via a shared `CredentialFileCandidates` - /`OldHandRolledXxxPath`/`CanonicalXxxPath` trio so migrator and - `clear --all` cannot drift. Equality is **parsed-projection**, not - byte-equal (ordering / trailing-newline differences are harmless). +5. **Companion-secret-file dual-probe** (MON-5371 gro, MON-5373 nrq): + when the resolver switch also relocates a secret-bearing companion + file (gro: `token.json`; nrq: `credentials`), the migrator MUST + probe BOTH old and new locations — old/new candidate enumeration + with path-identity dedup — and the shared candidate helper must be + the SAME one `clear --all` consumes (so migrator and clear cannot + drift). Equality model is **format-dependent**: text-key=value + files (nrq's `credentials`) compare on parsed/effective projection + (api_key / account_id / region — harmless ordering or trailing- + newline differences must NOT false-conflict); opaque blobs (gro's + `token.json`) compare on the trimmed raw serialized value + (different bytes = different token = conflict). Pick the model + that matches the file format, not a single family rule. 6. **7-var `statedirtest.Hermetic`** mandatory: HOME/XDG-only test isolation leaks the developer's real `~/Library/Application @@ -511,10 +517,18 @@ inherits them):** passes if the gate ran but didn't run BEFORE migration. Both assertions together prove ordering. -9. **`reflect.DeepEqual` on default-applied Config**: material - equality compares the full default-applied struct — future Config - fields auto-trigger divergence with no manual comparator - maintenance. +9. **Material equality compares the user-meaningful default-applied + projection**: apply defaults on both sides BEFORE comparing (so an + omitted field that semantically defaults to X compares equal to an + explicit X). Use `reflect.DeepEqual` on the full default-applied + struct ONLY when every field is directly comparable AND semantically + user-meaningful after defaults (slck / nrq). When a field needs + semantic normalization (gro: `OAuthClientPath` default-old vs + default-new are equivalent paths; `GrantedScopes` sort order is + meaningless), build an explicit projection so legitimate + default-path relocations don't false-conflict. Either way, future + fields force a deliberate choice — silent silence is what hides + divergence. 10. **Path-identity dedup**: Linux collapses old≡new (statedir ≡ `$XDG_CONFIG_HOME`); operations on both paths must dedupe so From 64cf616b5b20b00e2979b1371c99426a468f1750 Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 20 May 2026 07:55:24 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fixup:=20Codex=20r2=20=E2=80=94=20pattern?= =?UTF-8?q?=205=20conditional=20cleanup-helper=20rule;=20pattern=209=20typ?= =?UTF-8?q?o?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/working-with-state.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/working-with-state.md b/docs/working-with-state.md index c50c9df..059b417 100644 --- a/docs/working-with-state.md +++ b/docs/working-with-state.md @@ -484,9 +484,18 @@ inherits them):** when the resolver switch also relocates a secret-bearing companion file (gro: `token.json`; nrq: `credentials`), the migrator MUST probe BOTH old and new locations — old/new candidate enumeration - with path-identity dedup — and the shared candidate helper must be - the SAME one `clear --all` consumes (so migrator and clear cannot - drift). Equality model is **format-dependent**: text-key=value + with path-identity dedup. **If `clear --all` (or equivalent cleanup) + also scrubs that companion file, migrator and cleanup must share + the SAME candidate-enumeration helper so they cannot drift** — + nrq's `CredentialFileCandidates()` is the exemplar (consumed by + both `internal/keychain/migrate.go` discover() and + `internal/cmd/configcmd/config.go` clear). gro's case is the other + shape: the token dual-probe lives in the keychain migrator via + `GetTokenPath()` + `OldHandRolledTokenPath()`, and `config clear + --all` does NOT consume that helper because gro's cleanup scope is + config + cache only (token.json is explicitly excluded from + `ApplyConfigRelocation` — token lives entirely in the keychain + layer). Equality model is **format-dependent**: text-key=value files (nrq's `credentials`) compare on parsed/effective projection (api_key / account_id / region — harmless ordering or trailing- newline differences must NOT false-conflict); opaque blobs (gro's @@ -527,8 +536,7 @@ inherits them):** default-new are equivalent paths; `GrantedScopes` sort order is meaningless), build an explicit projection so legitimate default-path relocations don't false-conflict. Either way, future - fields force a deliberate choice — silent silence is what hides - divergence. + fields force a deliberate choice; silence is what hides divergence. 10. **Path-identity dedup**: Linux collapses old≡new (statedir ≡ `$XDG_CONFIG_HOME`); operations on both paths must dedupe so From 60d0b66b0209d65394de215573687dc4c519ee8b Mon Sep 17 00:00:00 2001 From: Rian Stockbower Date: Wed, 20 May 2026 07:56:17 -0400 Subject: [PATCH 4/4] =?UTF-8?q?fixup:=20Codex=20r3=20nit=20=E2=80=94=20nar?= =?UTF-8?q?row=20gro=20cleanup-scope=20wording?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/working-with-state.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/working-with-state.md b/docs/working-with-state.md index 059b417..5c07a93 100644 --- a/docs/working-with-state.md +++ b/docs/working-with-state.md @@ -492,8 +492,10 @@ inherits them):** `internal/cmd/configcmd/config.go` clear). gro's case is the other shape: the token dual-probe lives in the keychain migrator via `GetTokenPath()` + `OldHandRolledTokenPath()`, and `config clear - --all` does NOT consume that helper because gro's cleanup scope is - config + cache only (token.json is explicitly excluded from + --all` does NOT consume that helper because gro's `--all` file-scrub + scope is config + cache only; OAuth token cleanup happens through + the keychain path, not the legacy `token.json` candidate + enumeration (token.json is also explicitly excluded from `ApplyConfigRelocation` — token lives entirely in the keychain layer). Equality model is **format-dependent**: text-key=value files (nrq's `credentials`) compare on parsed/effective projection