Skip to content

feat: storage consolidation resilience (OAuth session, project binding, gitignore)#512

Closed
kelsonpw wants to merge 0 commit into
mainfrom
feat/storage-consolidation-resilience
Closed

feat: storage consolidation resilience (OAuth session, project binding, gitignore)#512
kelsonpw wants to merge 0 commit into
mainfrom
feat/storage-consolidation-resilience

Conversation

@kelsonpw

@kelsonpw kelsonpw commented May 1, 2026

Copy link
Copy Markdown
Member

Summary

Consolidates wizard-owned storage under predictable paths: OAuth/session persistence, canonical project binding under .amplitude/project-binding.json, dual-read from legacy root ampli.json, and a narrowed .gitignore so only intended artifacts are ignored.

Includes a fix to narrow AmpliConfigParseResult before reading .error so TypeScript discriminated unions are respected in readAmpliConfig merge/migrate logic.

Coordination

This branch may touch the same areas as #237, #504, #498, #207, and #209 — worth a quick overlap check before merge.

Made with Cursor


Note

Medium Risk
Touches credential/session persistence and project binding files, so regressions could affect login continuity or selecting the correct Amplitude project across runs, but changes are scoped and mostly additive/migration-focused.

Overview
Improves wizard storage resilience by consolidating persisted state under predictable paths (including OAuth/session data) and introducing a canonical per-repo binding file at .amplitude/project-binding.json, while still dual-reading legacy root ampli.json for backward compatibility.

Hardens auth/config handling by tightening readAmpliConfig’s discriminated-union narrowing, adding clearAuthFieldsInAmpliConfig to strip auth-scoped fields from ampli.json on logout/reset, and making getStoredToken zone-aware (and rejecting tokens with mismatched issuer/audience) to avoid reusing wrong-region sessions.

Narrows .gitignore rules so only intended wizard artifacts are ignored (anchoring patterns and explicitly allowing shipped .claude/skills).

Reviewed by Cursor Bugbot for commit e3af45a. Bugbot is set up for automated code reviews on this repo. Configure here.

@kelsonpw kelsonpw requested a review from a team as a code owner May 1, 2026 19:03

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Stale OAuth data returned after failed credential clear
    • Removed the try/catch around the primary (canonical) file write in writeConfig so failures propagate to callers, preventing the inconsistent state where stale tokens remain in the primary file after a failed clear while the legacy file is successfully wiped.

Create PR

Or push these changes by commenting:

@cursor push 81c4c96bba
Preview (81c4c96bba)
diff --git a/src/utils/ampli-settings.ts b/src/utils/ampli-settings.ts
--- a/src/utils/ampli-settings.ts
+++ b/src/utils/ampli-settings.ts
@@ -104,15 +104,9 @@
   }
   const primaryPath = getOAuthSettingsFile();
   const legacyPath = getLegacyAmpliHomeOAuthPath();
+  ensureDir(getCacheRoot());
+  atomicWriteJSON(primaryPath, data, 0o600);
   try {
-    ensureDir(getCacheRoot());
-    atomicWriteJSON(primaryPath, data, 0o600);
-  } catch (err) {
-    log.warn('writeConfig: failed to write canonical OAuth session file', {
-      'error message': err instanceof Error ? err.message : String(err),
-    });
-  }
-  try {
     atomicWriteJSON(legacyPath, data, 0o600);
   } catch (err) {
     log.debug('writeConfig: legacy OAuth mirror write failed (non-fatal)', {

You can send follow-ups to the cloud agent here.

Reviewed by Cursor Bugbot for commit ef9a654. Configure here.

Comment thread src/utils/ampli-settings.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant