fix: invalidate repo filter cache when GitHub token auth state changes#494
fix: invalidate repo filter cache when GitHub token auth state changes#494Nsanjayboruds wants to merge 8 commits intofossasia:mainfrom
Conversation
Reviewer's GuideAdds GitHub-auth-state–aware cache keys and invalidation for repository filtering, ensuring repo caches are not shared between authenticated and unauthenticated sessions, and updates related scrum helper cache keys plus formatting and Biome config changes. Sequence diagram for GitHub repo filtering with auth-aware cachesequenceDiagram
actor User
participant Popup
participant Storage as browser_storage_local
participant GitHubAPI
User->>Popup: Toggle useRepoFilter to enabled
Popup->>Storage: get platform, username, orgName, githubToken, repoCache
Popup-->>Popup: check platform is github
Popup-->>Popup: repoCacheKey = getRepoCacheKey(username, orgName, githubToken)
Popup-->>Popup: compute cacheAge
alt Fresh cache with matching repoCacheKey
Popup-->>Popup: use cached repos
Popup-->>User: Show filtered repo list from cache
else No cache or stale cache or mismatched repoCacheKey
Popup->>GitHubAPI: fetchUserRepositories(username, orgName, githubToken)
GitHubAPI-->>Popup: repos
Popup-->>Popup: repoCacheKey = getRepoCacheKey(username, orgName, githubToken)
Popup->>Storage: set repoCache { data, timestamp, cacheKey: repoCacheKey }
Popup-->>User: Show filtered repo list from fresh fetch
end
Note over Popup: getRepoCacheKey(username, orgName, token) -> "repos-username-org-tokenMarker" where tokenMarker is auth or noauth
Sequence diagram for GitHub token change invalidating repo cachesequenceDiagram
actor User
participant Popup
participant Storage as browser_storage_local
User->>Popup: Open popup UI
Popup->>Storage: get githubToken, repoCache
Storage-->>Popup: githubToken, repoCache
Popup-->>Popup: previousGithubTokenMarker = githubToken ? auth : noauth
User->>Popup: Edit githubTokenInput
Popup-->>Popup: nextTokenMarker = githubTokenInput.value ? auth : noauth
Popup-->>Popup: shouldInvalidateRepoCache = previousGithubTokenMarker != nextTokenMarker
Popup-->>Popup: previousGithubTokenMarker = nextTokenMarker
alt shouldInvalidateRepoCache is true
Popup-->>Popup: payload = { githubToken, repoCache: null }
else shouldInvalidateRepoCache is false
Popup-->>Popup: payload = { githubToken }
end
Popup->>Storage: set payload
Note over Popup: Changing between auth and noauth forces repoCache to be cleared so future loads refetch repos
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
auth/noauthmarker logic is duplicated and slightly inconsistent (trim()inpopup.jsbut not inscrumHelper.js); consider extracting a small shared helper (e.g.,getGithubTokenMarker(token)) and reusing it to avoid future divergence in cache key behavior. - The
auth/noauthstrings are now part of multiple cache key formats; pulling them into a shared constant or enum-like structure would make it less error‑prone to evolve these markers or search for all usages later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `auth`/`noauth` marker logic is duplicated and slightly inconsistent (`trim()` in `popup.js` but not in `scrumHelper.js`); consider extracting a small shared helper (e.g., `getGithubTokenMarker(token)`) and reusing it to avoid future divergence in cache key behavior.
- The `auth`/`noauth` strings are now part of multiple cache key formats; pulling them into a shared constant or enum-like structure would make it less error‑prone to evolve these markers or search for all usages later.
## Individual Comments
### Comment 1
<location path="src/scripts/popup.js" line_range="711-713" />
<code_context>
});
});
githubTokenInput.addEventListener('input', () => {
- browser.storage.local.set({ githubToken: githubTokenInput.value });
+ const nextTokenMarker = githubTokenInput.value.trim() ? 'auth' : 'noauth';
+ const shouldInvalidateRepoCache = previousGithubTokenMarker !== nextTokenMarker;
+ previousGithubTokenMarker = nextTokenMarker;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** Repo cache invalidation only distinguishes auth/noauth, not actual token changes.
Because both `previousGithubTokenMarker` and `nextTokenMarker` map any non-empty token to `'auth'`, switching between different valid tokens will not invalidate `repoCache`, even when the repo set differs (different user, scopes, orgs, etc.). Since `getRepoCacheKey` also only uses the auth/noauth marker, cached results from one token can be incorrectly reused for another. Please key the cache on a more specific, non-reversible token fingerprint (e.g., a hash) so any token change invalidates the cache without storing the raw token.
</issue_to_address>
### Comment 2
<location path="src/scripts/scrumHelper.js" line_range="918" />
<code_context>
return [];
}
- const repoCacheKey = `repos-${platformUsernameLocal}-${orgName}-${startDateForCache}-${endDateForCache}`;
+ const tokenMarker = githubToken ? 'auth' : 'noauth';
+ const repoCacheKey = `repos-${platformUsernameLocal}-${orgName}-${startDateForCache}-${endDateForCache}-${tokenMarker}`;
const now = Date.now();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Token marker logic here is inconsistent with `getRepoCacheKey`’s trimming behavior.
`getRepoCacheKey` uses `token && token.trim() ? 'auth' : 'noauth'`, while this code uses `githubToken ? 'auth' : 'noauth'`. A whitespace-only token will be treated as `'noauth'` in the popup but `'auth'` here, leading to inconsistent cache keys between flows. Align the logic (e.g. `githubToken && githubToken.trim()`) so both paths define “authenticated” the same way.
```suggestion
const tokenMarker = githubToken && githubToken.trim() ? 'auth' : 'noauth';
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
Fixes stale GitHub repository filter results being reused across unauthenticated vs authenticated sessions by making repo-cache keys auth-state aware and invalidating cached repo data when token auth state changes.
Changes:
- Add an
auth/noauthmarker to GitHub repo cache keys used by popup repo filtering and scrum generation repo fetching. - Invalidate
repoCachein popup when the GitHub token transitions between empty and non-empty. - Update Biome schema URL to match the installed Biome CLI version and apply formatting updates.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/scripts/popup.js | Adds token auth-state marker to repo cache keys and invalidates repo cache on auth-state transitions. |
| src/scripts/scrumHelper.js | Updates internal repo cache key to include auth-state marker; formatting adjustments. |
| src/scripts/gitlabHelper.js | Formatting-only changes (no intended logic changes). |
| biome.json | Updates $schema to Biome 2.4.9 to match installed CLI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mariobehling
left a comment
There was a problem hiding this comment.
Please address AI reviews or mention why they are not relevant if so.
|
Also fix linting issues. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Addressed the security review findings.
Validation:
|
|
Hi @mariobehling and @vedansh-5, I’ve implemented all the requested changes and updates. Could you please take a moment to review the PR? Your feedback would be greatly appreciated. Thank you! |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,5 +1,5 @@ | |||
| { | |||
| "$schema": "https://biomejs.dev/schemas/2.3.13/schema.json", | |||
| "$schema": "https://biomejs.dev/schemas/2.4.9/schema.json", | |||
There was a problem hiding this comment.
biome.json schema is set to 2.4.9, but package.json pins @biomejs/biome to 2.4.11. Either bump the schema URL to 2.4.11 or align the dependency/version claim so tooling and editor validation match.
There was a problem hiding this comment.
Fixed. Schema is aligned to Biome 2.4.11 to match the dependency version and avoid tooling mismatch.
📌 Fixes
Fixes #493
📝 Summary of Changes
auth/noauth) so cache is not reused across unauthenticated and authenticated sessions.biome.jsonto match installed CLI version (2.4.9) and formatted affected files to pass checks.Files touched:
src/scripts/popup.jssrc/scripts/scrumHelper.jssrc/scripts/gitlabHelper.js(format only)biome.json✅ Checklist
👀 Reviewer Notes
npm installnpm run check(passes)src/scripts/gitlabHelper.jswas auto-formatted to satisfy repository-wide Biome checks; no logic change intended there.Summary by Sourcery
Ensure GitHub repository filtering cache respects authentication state and is invalidated when the GitHub token changes.
Bug Fixes:
Enhancements:
Build: