refactor: validate and parse the endpoint and proxy at program load by peterguy · Pull Request #1267 · sourcegraph/src-cli

peterguy · 2026-03-07T02:37:45Z

Summary

Parse the endpoint URL at program load instead of passing around an unparsed string and needing to check/parse it in multiple places.

Parse the proxy url at program load, supporting HTTP_PROXY, HTTPS_PROXY and NO_PROXY in addition to SRC_PROXY. SRC_PROXY takes precedence.

Changes

`config` struct (`cmd/src/main.go`)

Split out a configFromFile struct containing the JSON elements
Un-export fields (all consumers are in package main)
Use endpointURL - Endpoint and Proxy are now in configFromFile
Parse endpointURL in readConfig - consumers use the parsed URL

`api.ClientOpts` (`internal/api/api.go`)

Endpoint string → EndpointURL *url.URL
Use JoinPath for URL construction instead of string concatenation with TrimRight

`login` command (`cmd/src/login.go`)

loginCmd now uses the config's endpointURL
Endpoint conflict detection moved from loginCmd into the handler, where the CLI arg is parsed and compared before entering the command

`code_intel_upload` (`cmd/src/code_intel_upload.go`)

makeCodeIntelUploadURL uses a stack copy (u := *cfg.endpointURL) instead of re-parsing the URL
Error return removed since there's nothing that can fail

Tests

Updated all test call sites to pass *url.URL

Testing

All affected packages pass:

ok  github.com/sourcegraph/src-cli/cmd/src
ok  github.com/sourcegraph/src-cli/internal/api
ok  github.com/sourcegraph/src-cli/internal/batches/repozip
ok  github.com/sourcegraph/src-cli/internal/batches/executor

cmd/src/code_intel_upload.go

cmd/src/login.go

peterguy · 2026-03-07T23:44:48Z

cmd/src/login.go

+	if cfg.configFilePath != "" {
 		fmt.Fprintln(out)
-		fmt.Fprintf(out, "⚠️  Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.ConfigFilePath)
+		fmt.Fprintf(out, "⚠️  Warning: Configuring src with a JSON file is deprecated. Please migrate to using the env vars SRC_ENDPOINT, SRC_ACCESS_TOKEN, and SRC_PROXY instead, and then remove %s. See https://github.com/sourcegraph/src-cli#readme for more information.\n", cfg.configFilePath)


The config file has been deprecated for a long time, but if it ever is finally removed, the ability to set additional headers will go also. Do we need to keep the ability to set additional headers around? If so, we should probably introduce yet another environment variable for those and delay the config file removal. If not, let's complete the config file deprecation!

See https://github.com/sourcegraph/src-cli/pull/1267/changes#r2903847768

peterguy · 2026-03-08T03:32:27Z

This change is part of the following stack:

refactor: validate and parse the endpoint and proxy at program load #1267 ◀
- Add support for HTTP_PROXY, HTTPS_PROXY, and NO_PROXY #1260

_{Change managed by git-spice.}

internal/api/api.go

bahrmichael

I think there's a bug in https://github.com/sourcegraph/src-cli/pull/1267/changes#r2903861064. But apart from that good to go, so I'll approve to unblock.

keegancsmith · 2026-03-09T12:51:49Z

cmd/src/batch_remote.go

 		executionURL := fmt.Sprintf(
 			"%s/%s/batch-changes/%s/executions/%s",
-			strings.TrimSuffix(cfg.Endpoint, "/"),
+			cfg.endpointURL,


switch to using JoinPath?

Maybe? It is a pretty complex concatenation. Here are a few options for comparison:
Current:

executionURL := fmt.Sprintf( "%s/%s/batch-changes/%s/executions/%s", cfg.endpointURL, strings.TrimPrefix(namespace.URL, "/"), batchChangeName, batchSpecID, )

Proposal A:

executionURL := cfg.endpointURL.JoinPath( fmt.Sprintf( "%s/batch-changes/%s/executions/%s", namespace.URL, batchChangeName, batchSpecID, ), ).String()

Proposal B:

executionURL := endpointURL. JoinPath(namespace.URL). JoinPath("batch-changes"). JoinPath(batchChangeName). JoinPath("executions"). JoinPath(batchSpecID). String()

I think proposal A is the easiest to read, but also it's nearly the same as the original fmt.Sprintf.

I'm going to use proposal A unless someone has a strong opinion otherwise.

keegancsmith

approving, but agree with Michael's catch on the breaking change.

internal/api/api.go

- config struct: unexport fields, replace Endpoint/Proxy strings with endpointURL/proxyURL (*url.URL) and proxyPath, parse once in readConfig - api.ClientOpts: Endpoint string → EndpointURL *url.URL - login: accept *url.URL, move endpoint conflict check to handler - code_intel_upload: simplify makeCodeIntelUploadURL with stack copy, remove error return - api.go: use JoinPath for URL construction instead of string concat - Update all call sites and tests Amp-Thread-ID: https://ampcode.com/threads/T-019cc5da-fb05-71f8-962b-689cfc5b494d Co-authored-by: Amp <amp@ampcode.com>

…tain a query string

Amp-Thread-ID: https://ampcode.com/threads/T-019cd85e-222f-769f-9950-b0ed936a38dd Co-authored-by: Amp <amp@ampcode.com> # Conflicts: # cmd/src/login.go # cmd/src/login_oauth.go # cmd/src/login_test.go # cmd/src/login_validate.go # cmd/src/main.go # cmd/src/main_test.go # internal/api/api.go

peterguy changed the title ~~refactor: use parsed *url.URL types instead of string endpoints~~ refactor: validate and parse the endpoint up front instead of keeping it as a string Mar 7, 2026

peterguy commented Mar 7, 2026

View reviewed changes

cmd/src/code_intel_upload.go Outdated Show resolved Hide resolved

peterguy commented Mar 7, 2026

View reviewed changes

cmd/src/login.go Show resolved Hide resolved

peterguy commented Mar 7, 2026

View reviewed changes

peterguy changed the title ~~refactor: validate and parse the endpoint up front instead of keeping it as a string~~ refactor: validate and parse the endpoint and proxy at program load Mar 8, 2026

peterguy self-assigned this Mar 8, 2026

peterguy marked this pull request as ready for review March 8, 2026 01:34

peterguy requested a review from a team March 8, 2026 01:34

peterguy mentioned this pull request Mar 8, 2026

Add support for HTTP_PROXY, HTTPS_PROXY, and NO_PROXY #1260

Open

bahrmichael reviewed Mar 9, 2026

View reviewed changes

internal/api/api.go Outdated Show resolved Hide resolved

bahrmichael approved these changes Mar 9, 2026

View reviewed changes

keegancsmith self-requested a review March 9, 2026 09:50

keegancsmith reviewed Mar 9, 2026

View reviewed changes

keegancsmith approved these changes Mar 9, 2026

View reviewed changes

internal/api/api.go Outdated Show resolved Hide resolved

peterguy and others added 12 commits March 9, 2026 18:51

fix formatting and error messages

7cf81b8

clear any auth from the endpoint url at program load

f004487

rename variables

2d77f7b

fixup main tests

921b34b

restore indirection

c45082e

fix capitalization

26a0b64

standardize url parsing

4bfcd6f

standardize url parsing

39be826

remove support for HTTP(S)_PROXY - that will come later

f42f16a

revert JoinPath in createHTTPRequest because the input path could con…

ed31465

…tain a query string

added a comment about the URL shallow copy

97c1b0a

peterguy force-pushed the peterguy/clean-up-config branch from cab95e8 to 97c1b0a Compare March 10, 2026 01:53

peterguy added 2 commits March 9, 2026 19:13

fix typo in comment

e62a619

peterguy added 2 commits March 10, 2026 09:20

re-apply JoinPath lost during merge

8ecf5d6

rename variable

f976adb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: validate and parse the endpoint and proxy at program load#1267

refactor: validate and parse the endpoint and proxy at program load#1267
peterguy wants to merge 16 commits intomainfrom
peterguy/clean-up-config

peterguy commented Mar 7, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

peterguy Mar 7, 2026 •

edited

Loading

Uh oh!

bahrmichael Mar 9, 2026

Uh oh!

peterguy commented Mar 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

bahrmichael left a comment

Uh oh!

keegancsmith Mar 9, 2026

Uh oh!

peterguy Mar 9, 2026 •

edited

Loading

Uh oh!

peterguy Mar 10, 2026

Uh oh!

keegancsmith left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

peterguy commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

config struct (cmd/src/main.go)

api.ClientOpts (internal/api/api.go)

login command (cmd/src/login.go)

code_intel_upload (cmd/src/code_intel_upload.go)

Tests

Testing

Uh oh!

Uh oh!

Uh oh!

peterguy Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bahrmichael Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

peterguy commented Mar 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

bahrmichael left a comment

Choose a reason for hiding this comment

Uh oh!

keegancsmith Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

peterguy Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

peterguy Mar 10, 2026

Choose a reason for hiding this comment

Uh oh!

keegancsmith left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

peterguy commented Mar 7, 2026 •

edited

Loading

`config` struct (`cmd/src/main.go`)

`api.ClientOpts` (`internal/api/api.go`)

`login` command (`cmd/src/login.go`)

`code_intel_upload` (`cmd/src/code_intel_upload.go`)

peterguy Mar 7, 2026 •

edited

Loading

peterguy commented Mar 8, 2026 •

edited

Loading

peterguy Mar 9, 2026 •

edited

Loading