Remove keychain storage, store tokens in config file by jai-deepsource · Pull Request #269 · DeepSourceCorp/cli

jai-deepsource · 2026-03-01T18:07:54Z

Drop internal/secrets/ package and keychain integration
Simplify config manager to use TOML-only token storage
Default host when config is empty, backfill user from server
Fix subcommand help delegation

deepsource-io · 2026-03-01T18:08:47Z

DeepSource Code Review

We reviewed changes in e34608a...074fd63 on this pull request. Below is the summary for the review, and you can see the individual issues we found as inline review comments.

See full review on DeepSource ↗

Important

Some issues found as part of this review are outside of the diff in this pull request and aren't shown in the inline review comments due to GitHub's API limitations. You can see those issues on the DeepSource dashboard.

Code Review Summary

Analyzer	Updated (UTC)	Details
Go	Mar 1, 2026 8:12p.m.	Review ↗
Secrets	Mar 1, 2026 8:12p.m.	Review ↗
Test coverage	Mar 1, 2026 8:12p.m.	Review ↗

Code Coverage Summary

Language	Line Coverage (New Code)	Line Coverage (Overall)
Aggregate	18.8%	19.4%
Go	18.8% [⤫ below threshold]	19.4% [✓ above threshold]

➟ Additional coverage metrics may have been reported. See full coverage report ↗

- Drop internal/secrets/ package and keychain integration - Simplify config manager to use TOML-only token storage - Default host when config is empty, backfill user from server - Fix subcommand help delegation Signed-off-by: Jai Pradeesh <jai@deepsource.io>

- Remove redundant godoc and inline comments across codebase - Extract auth status into smaller helper methods - Add BackfillUser and default host logic to config manager - Add tests for login edge cases, auth status, and config manager

deepsource-io · 2026-03-01T19:00:16Z

command/report/git.go

 )

-// gitGetHead accepts a git directory and returns head commit OID / error
 func gitGetHead(workspaceDir string) (headOID string, warning string, err error) {


func gitGetHead is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:16Z

command/report/git.go


-// Fetches the latest commit hash using the command `git rev-parse HEAD`
-// through git
 func fetchHeadManually(directoryPath string) (string, error) {


func fetchHeadManually is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:16Z

command/report/git.go

 }

-// Handle special cases for GitHub Actions.
 func getGitHubActionsCommit(topCommit string) (headOID string, warning string, err error) {


func getGitHubActionsCommit is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:17Z

command/report/git.go

 }

-// Return PR's HEAD ref set as env variable manually by DeepSource's Test coverage action.
 func getTestCoverageActionCommit() (headOID string, err error) {


func getTestCoverageActionCommit is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:17Z

command/report/git.go

-	// Fetch value of pull request SHA. If this is a PR, it will return SHA of HEAD commit of the PR, else "".
-	// If prSHA is not empty, that means we got an SHA, which is HEAD. Return this.
+	// Travis creates a merge commit for PR forks; use the real PR head instead.
 	if prSHA := os.Getenv("TRAVIS_PULL_REQUEST_SHA"); len(prSHA) > 0 {


replace `len(prSHA) > 0` with `prSHA != ""`

It is not recommended to use len for empty string test.

deepsource-io · 2026-03-01T19:00:17Z

command/report/git.go

 }

-// Handle special case for TravisCI
 func getTravisCommit(topCommit string) (string, string, error) {


func getTravisCommit is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:17Z

command/report/query.go

 )

-// makeQuery makes a HTTP query with a specified body and returns the response
 func makeQuery(url string, body []byte, bodyMimeType string, skipCertificateVerification bool) ([]byte, error) {


func makeQuery is unused

It is recommended to keep the codebase clean by removing unused code.

deepsource-io · 2026-03-01T19:00:17Z

command/auth/login/tests/login_test.go

+	if err != nil {
+		// Config may still be corrupt from our test setup, but the PAT flow should have overwritten it
+		t.Logf("config load after PAT: %v (may be expected if overwrite failed)", err)
+	} else if cfg.Token != "dsp_fallback" {


Test passes on error when it should fail

The test logic in TestLoginConfigLoadError is flawed. If cfgMgr.Load() returns an error after cmd.Execute(), it means the application failed to overwrite the corrupt configuration file with a valid one. The test currently logs this error and passes, masking a potential bug.

The test should instead fail explicitly if the configuration cannot be loaded after the login command has run. Replace the if err != nil block with a check that fails the test, for example using t.Fatalf().

- Remove unused git.go and query.go from command/report - Fix flaky assertion in TestLoginConfigLoadError

deepsource-io · 2026-03-01T20:14:21Z

command/auth/status/tests/auth_status_test.go

+
+	// Mock client that returns a non-auth error
+	mock := graphqlclient.NewMockClient()
+	mock.QueryFunc = func(_ context.Context, query string, _ map[string]any, _ any) error {


Function parameter `query` is declared but unused

The function assigned to mock.QueryFunc declares a parameter named query which is never used inside the function implementation. Unused parameters can confuse readers or indicate incomplete refactoring.
Rename or replace the unused query parameter with _ (underscore) to mark it explicitly as unused and improve code clarity and maintainability.

jai-deepsource force-pushed the v2-improvements branch from 857674c to 2fa27df Compare March 1, 2026 18:17

jai-deepsource added 3 commits March 1, 2026 10:17

Bump version to 2.0.38

7ed50cf

Bump version to 2.0.39

3b81ade

deepsource-io bot reviewed Mar 1, 2026

View reviewed changes

jai-deepsource added 2 commits March 1, 2026 12:00

Clean up report package and fix login test

f6e4a4a

- Remove unused git.go and query.go from command/report - Fix flaky assertion in TestLoginConfigLoadError

Bump version to 2.0.40

074fd63

deepsource-io bot reviewed Mar 1, 2026

View reviewed changes

jai-deepsource merged commit 093ba87 into master Mar 1, 2026
3 of 5 checks passed

jai-deepsource deleted the v2-improvements branch March 1, 2026 20:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove keychain storage, store tokens in config file#269

Remove keychain storage, store tokens in config file#269
jai-deepsource merged 6 commits intomasterfrom
v2-improvements

jai-deepsource commented Mar 1, 2026

Uh oh!

deepsource-io bot commented Mar 1, 2026 •

edited

Loading

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

deepsource-io bot Mar 1, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Conversation

jai-deepsource commented Mar 1, 2026

Uh oh!

deepsource-io bot commented Mar 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

DeepSource Code Review

Code Review Summary

Code Coverage Summary

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func gitGetHead is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func fetchHeadManually is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func getGitHubActionsCommit is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func getTestCoverageActionCommit is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

replace `len(prSHA) > 0` with `prSHA != ""`

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func getTravisCommit is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

func makeQuery is unused

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

Test passes on error when it should fail

Uh oh!

deepsource-io bot Mar 1, 2026

Choose a reason for hiding this comment

Function parameter `query` is declared but unused

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

deepsource-io bot commented Mar 1, 2026 •

edited

Loading