[code-simplifier] Remove redundant refreshCookieName guard in issueTokens#337
Merged
veverkap merged 2 commits intoMay 23, 2026
Conversation
The early return at the top of issueTokens already guarantees that refreshCookieName is non-empty when sessions is non-nil, making the inner 'if refreshCookieName != ""' check unreachable-false. Remove it to make the invariant clearer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…kie-check-91796afecc562c47
Contributor
There was a problem hiding this comment.
Pull request overview
This PR simplifies issueTokens in handler/helpers.go by removing a redundant refreshCookieName != "" conditional before setting the refresh cookie. The function already fails fast when sessions != nil and refreshCookieName == "", so the removed condition could never be false in the sessions != nil branch.
Changes:
- Remove the redundant inner guard around
SetRefreshCookieinside thesessions != nilflow. - Make the “refresh cookie name is non-empty when sessions are enabled” invariant explicit by always calling
SetRefreshCookiein that branch.
Show a summary per file
| File | Description |
|---|---|
handler/helpers.go |
Removes an unreachable conditional and always sets the refresh cookie when sessions are enabled (protected by the existing early misconfiguration guard). |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Code Simplification - 2026-05-23
This PR simplifies recently modified code to improve clarity while preserving all functionality.
Files Simplified
handler/helpers.go- Removed a redundantif refreshCookieName != ""guard insideissueTokensImprovement
The
issueTokensfunction already contains an early return at the top:Because of this guard, when execution reaches the
SetRefreshCookiecall (inside thesessions != nilbranch),refreshCookieNameis provably non-empty. The innerif refreshCookieName != """ check was therefore alwaystrueand could never befalse`. Removing it makes the invariant explicit and the code easier to read.Changes Based On
Recent changes from:
Testing
go test ./handler/...)Automated by Code Simplifier Agent - analyzing code from the last 24 hours
Add this agentic workflows to your repo
To install this agentic workflow, run
Greptile Summary
This PR removes a dead-code guard (
if refreshCookieName != "") that wrappedSetRefreshCookieinsideissueTokens. Because the function already returns early wheneversessions != nil && refreshCookieName == "", any code path that reaches thesessions != nilbranch is guaranteed to have a non-emptyrefreshCookieName, making the inner check always-true and therefore unnecessary.sessions != nil && refreshCookieName == "") is the sole enforcement point, and it remains untouched.h.RefreshCookieName(validated at startup); the test athelpers_test.go:586explicitly covers the misconfiguration path.Confidence Score: 5/5
Safe to merge — the removed guard was provably dead code and the change is purely cosmetic with no behavioral impact.
The early-return at the top of
issueTokensalready covers the only case whererefreshCookieNamecould be empty whilesessionsis non-nil, so the deleted inner guard could never have beenfalse. No logic paths change, no new error conditions are introduced, and existing tests confirm the misconfiguration path is still handled correctly.No files require special attention.
Important Files Changed
if refreshCookieName != ""guard aroundSetRefreshCookie; the invariant is fully enforced by the existing early return at line 45 (sessions != nil && refreshCookieName == ""), making the inner check provably dead code.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[issueTokens called] --> B{sessions != nil\nAND\nrefreshCookieName == ?} B -- "sessions != nil\n&& refreshCookieName == ''" --> C[Return early\nHTTP 500 + false] B -- "sessions == nil\nOR refreshCookieName != ''" --> D{sessions != nil?} D -- No --> E[CreateToken\nSetAuthCookie\nReturn token] D -- Yes\nrefreshCookieName guaranteed non-empty --> F[GenerateRandomHex\nCreateSession\nCreateTokenWithSession] F --> G[SetRefreshCookie\n✅ called directly\nno inner guard needed] G --> H[SetAuthCookie\nReturn tokens]Reviews (1): Last reviewed commit: "Merge branch 'main' into code-simplifier..." | Re-trigger Greptile