feat: add --env-file flag to develop.sh by johnstcn · Pull Request #25621 · coder/coder

johnstcn · 2026-05-22T14:48:24Z

Adds opt-in .env file support to the develop orchestrator (scripts/develop) via a new
--env-file flag. This makes it easier to configure things like external auth providers,
access URLs, and other dev-time settings without exporting a wall of environment variables
in every shell session.

Behavior

Pass --env-file <path> or set CODER_DEV_ENV_FILE to load a .env file.
Variables in the file do not override existing environment variables.
Loading happens before serpent option parsing, so develop-script flags like
CODER_DEV_ACCESS_URL and CODER_DEV_PORT work from the env file.
Uses github.com/joho/godotenv for parsing (supports all standard .env syntax).
Prints develop: loaded N variable(s) from <path> to stderr when active.
Errors immediately if the specified file does not exist.
Note: unquoted and double-quoted values undergo $VAR expansion;
use single quotes for literal dollar signs.

Usage

# Via flag
./scripts/develop.sh --env-file .env

# Via environment variable (set once in shell profile)
export CODER_DEV_ENV_FILE=.env
./scripts/develop.sh

Example `.env`

CODER_DEV_ACCESS_URL=http://dev.myworkspace.me.coder:3000
CODER_EXTERNAL_AUTH_0_ID=primary-gitlab
CODER_EXTERNAL_AUTH_0_TYPE=gitlab
CODER_EXTERNAL_AUTH_0_CLIENT_ID=abc123
CODER_EXTERNAL_AUTH_0_CLIENT_SECRET='secret$pecial'

Design decisions

Decision	Rationale
Opt-in flag, not auto-load	No magic; explicit is better than implicit; CWD-independent
`godotenv` library	Zero deps, 10k stars, eliminates hand-rolled parser bugs
Pre-parse from os.Args	Solves chicken-and-egg: file vars must be visible to serpent's Env-tag resolution
No-override semantics	Existing env always wins; consistent with direnv behavior
Error on missing file	Explicit request for missing file = mistake, fail fast

Generated with Coder Agents

johnstcn · 2026-05-22T14:48:59Z

/coder-agents-review

coder-agents-review · 2026-05-22T14:49:03Z

Review posted | Chat
Requested: 2026-05-22 14:49 UTC by @johnstcn

Review history

R1 (2026-05-22): 15 reviewers, 3 Nit, 1 P2, 6 P3, REQUEST_CHANGES. Review
R2 (2026-05-22): 15 reviewers, 5 Nit, 2 P0, 2 P2, 9 P3, REQUEST_CHANGES. Review
R3 (2026-05-22): 9 reviewers, 5 Nit, 2 P0, 2 P2, 10 P3, COMMENT. Review

deep-review v0.5.0 | Round 3 | 0d9718e..263bde8

Last posted: Round 3, 19 findings (2 P0, 2 P2, 10 P3, 5 Nit), COMMENT. Review

Finding inventory

Findings

#	Sev	Status	Location	Summary	Round	Reviewer	Posted
CRF-1	P3	Author fixed (`b42d851`)	scripts/develop/main.go:491	Blank line splits filterEnv doc comment	R1	Netero, Mafu-san	Yes
CRF-2	Nit	Author fixed (`b42d851`)	scripts/develop/main.go:453	os.IsNotExist instead of errors.Is(err, fs.ErrNotExist)	R1	Netero	Yes
CRF-3	P3	Author fixed (`b42d851`)	scripts/develop/main_test.go:874	Test cleanup: 2 of 3 env vars lack t.Setenv protection	R1	Netero Nit, Mafu-san P2	Yes
CRF-4	P2	Author fixed (`b42d851`)	scripts/develop/main.go:477	strings.Trim strips individual chars not matching pairs, silent value corruption	R1	Bisky P2, Mafuuu P3, Pariston P2, Hisoka Note, Zoro P3, Knov P2	Yes
CRF-5	P3	Author fixed (`b42d851`)	scripts/develop/main.go:470	Inline comments silently become part of value	R1	Chopper P3, Hisoka P3, Ryosuke P3, Zoro P2, Knov Note	Yes
CRF-6	P3	Author fixed (`b42d851`)	scripts/develop/main.go:474	Empty key after TrimSpace crashes os.Setenv with cryptic error	R1	Kite	Yes
CRF-7	P3	Author fixed (`b42d851`)	scripts/develop/main.go:471	Malformed lines (no =) silently dropped with no diagnostic	R1	Chopper	Yes
CRF-8	P3	Author fixed (`b42d851`)	scripts/develop/main.go:75	All-malformed .env: zero output indistinguishable from missing file	R1	Leorio	Yes
CRF-9	Nit	Author fixed (`b42d851`)	scripts/develop/main_test.go:869	Single-quoted values untested despite being a claimed feature	R1	Bisky	Yes
CRF-10	Nit	Dropped by orchestrator (style preference, comments serve as section markers in 35-line parser)	scripts/develop/main.go:468	Three inline comments restate code	R1	Gon P2	No
CRF-11	Nit	Author fixed (`b42d851`)	scripts/develop/main.go:448	Doc comment doesn't document accepted line formats	R1	Leorio	Yes
CRF-12	P0	Author fixed (`cccdb91`)	scripts/develop/main_test.go:920	TestParseEnvFileFlag missing //nolint:paralleltest	R2	Netero, Mafu-san	Yes
CRF-13	P0	Author fixed (`cccdb91`)	go.mod:598	godotenv marked // indirect but directly imported	R2	Netero, Mafu-san	Yes
CRF-14	P2	Author fixed (`263bde8`)	PR description	PR description describes R1 auto-load behavior, not R2 opt-in --env-file	R2	Leorio, Mafu-san	Yes
CRF-15	P3	Author fixed (`263bde8`)	scripts/develop/main.go:464	Dangling --env-file (last arg, no value) silently ignored	R2	Bisky, Chopper, Meruem	Yes
CRF-16	P3	Author fixed (`263bde8`)	scripts/develop/main.go:478	godotenv expands $VAR in unquoted/double-quoted values, undocumented	R2	Hisoka	Yes
CRF-17	P3	Author fixed (`263bde8`)	scripts/develop/main_test.go:912	EmptyPathNoAction test name misleading (asserts error, not no-action)	R2	Gon	Yes
CRF-18	Nit	Author fixed (`263bde8`)	scripts/develop/main.go:83	%q in error message vs %s in success message inconsistency	R2	Chopper, Gon, Leorio	Yes
CRF-19	Nit	Author fixed (`263bde8`)	scripts/develop/main.go:246	cfg.envFile write-only field needs comment	R2	Ryosuke Nit, Meruem P3	Yes
CRF-20	Nit	Dropped by orchestrator (standard Go doc comments, not bloat)	scripts/develop/main.go:459	Four doc comments restate function behavior	R2	Gon P2	No
CRF-21	P3	Open	scripts/develop/main.go:202	$VAR expansion docs omit that godotenv.Read resolves only against file-local vars, not process env	R3	Hisoka, Razor	Yes

Contested and acknowledged

(none)

Round log

Round 1

Panel. Netero first pass: 1 P3, 2 Nit. Panel: 15 reviewers. 1 P2, 5 P3, 3 Nit posted. 1 Nit dropped. Reviewed against 0d9718e..69f5ae4.

Round 2

Churn guard: PROCEED. All R1 findings addressed. Netero: 2 P0. Panel: 15 reviewers. 2 P0, 1 P2, 3 P3, 2 Nit new. 1 Nit dropped. Reviewed against 0d9718e..b42d851.

Round 3

Churn guard: PROCEED. All R2 findings addressed (cccdb91, 263bde8). PR description rewritten. Netero: no findings. Panel: 9 reviewers (Bisky, Chopper, Ging-Go, Hisoka, Kite, Mafu-san, Mafuuu, Pariston + Razor wildcard). 5 no findings. 1 P3 new. Reviewed against 0d9718e..263bde8.

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer	Focus
Bisky	tests
Chopper	ops/errors
Churn-guard	change verification
Ging	language modernization
Gon	naming
Hisoka	edge cases
Killua	perf
Kite	change integrity
Knov	contracts
Knuckle	SQL
Kurapika	security
Leorio	docs
Luffy	product
Mafu-san	process
Mafuuu	contracts
Melody	dispatch/pairing
Meruem	structural
Nami	frontend
Netero	mechanical checks
Pariston	premise testing
Pen-botter	product gaps
Razor	verification
Robin	duplication
Ryosuke	Go arch
Takumi	concurrency
Zoro	shape

🤖 Managed by Coder Agents.

Copilot

Pull request overview

Adds .env file loading to the scripts/develop orchestrator so local dev configuration can be provided via a repo-root .env file before serpent parses Env-tagged options.

Changes:

Load .env at process startup and report how many variables were applied (without overriding existing env).
Add a small .env parser (loadDotEnv) that supports comments, export prefix, and quoted values.
Add unit tests covering basic loading, non-overriding behavior, and missing-file behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
scripts/develop/main.go	Loads `.env` before CLI option parsing; introduces `loadDotEnv` helper for parsing and applying variables.
scripts/develop/main_test.go	Adds tests validating `.env` loading behavior and override semantics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

coder-agents-review

Clean, well-scoped addition. Zero dependencies, correct sequencing before serpent parsing, correct no-override semantics, and proportional test coverage. Pariston traced four alternative framings of the problem (direnv, full config format, .env.example) and concluded the in-process loader is the right call.

1 P2, 5 P3, 3 Nit. The P2 is the strings.Trim quote-stripping behavior (CRF-4): six reviewers converged on it independently, and empirical verification confirms strings.Trim(val, "\"'") silently corrupts values with mixed quote characters at boundaries.

CRF-4 and CRF-5 interact: KEY="value" # comment produces value" # comment because Trim strips the leading " but the trailing " is no longer at the string edge. If inline comments are stripped first (CRF-5), the quote-matching fix for CRF-4 works correctly on commented lines too. Fix order: inline comment stripping before quote matching.

"The premise of hand-rolling to avoid a dependency is fine for a dev tool. But the parser has to match the common expectations of .env syntax, and inline comments are part of that contract." (Zoro)

🤖 This review was automatically generated with Coder Agents.

Replace the automatic .env loading with an explicit --env-file flag backed by CODER_DEV_ENV_FILE env var. Uses godotenv for parsing instead of a hand-rolled parser. The flag is pre-parsed from os.Args before serpent runs so that loaded variables are visible to serpent's Env-tag resolution for other options.

johnstcn · 2026-05-22T17:18:46Z

/coder-agents-review

coder-agents-review

Good response to R1. All 10 findings addressed. The switch from a hand-rolled parser to godotenv eliminates the entire class of parsing bugs from R1 (quote stripping, inline comments, empty keys, malformed lines). The new --env-file flag with pre-serpent parsing is a cleaner design than the hardcoded .env path.

2 P0, 1 P2, 3 P3, 2 Nit. The P0s are CI-breaking: missing //nolint:paralleltest on TestParseEnvFileFlag and godotenv listed as // indirect in go.mod despite being directly imported.

R1 findings CRF-1 through CRF-11: all verified fixed. The godotenv switch was the right call.

"The agent knew the right pattern, applied it to one variable, and didn't generalize to the other two in the same block. This round: fixed all three. Self-correction landed." (Mafu-san, observing the R1-to-R2 delta)

🤖 This review was automatically generated with Coder Agents.

coder-agents-review · 2026-05-22T17:48:44Z

+	})
+}
+
+func TestParseEnvFileFlag(t *testing.T) {


P0 [CRF-12] TestParseEnvFileFlag mutates os.Args (process-global) but lacks the //nolint:paralleltest annotation that TestLoadEnvFile (line 859) correctly has. The linter flags this:

scripts/develop/main_test.go:921: Function TestParseEnvFileFlag missing the call to method parallel (paralleltest)

Fix: add //nolint:paralleltest // parseEnvFileFlag reads os.Args, a process-global. before the function declaration.

(Netero, Mafu-san)

🤖

Fixed in cccdb91.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:44Z

 	github.com/inconshreveable/mousetrap v1.1.0 // indirect
 	github.com/jackmordaunt/icns/v3 v3.0.1 // indirect
 	github.com/jbenet/go-context v0.0.0-20150711004518-d14ea06fba99 // indirect
+	github.com/joho/godotenv v1.5.1 // indirect


P0 [CRF-13] github.com/joho/godotenv is imported directly in scripts/develop/main.go:30 but listed as // indirect here. Running go mod tidy moves it to the direct dependencies block, producing a dirty diff that fails CI gen/fmt checks.

(Netero, Mafu-san)

🤖

Fixed in cccdb91. Removed // indirect marker.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:44Z

 )

 func main() {
+	// Pre-parse --env-file before serpent runs so that variables from


P2 [CRF-14] The PR description still describes R1's auto-load behavior: "On startup, the develop binary reads .env from the repo root." The code now requires explicit --env-file or CODER_DEV_ENV_FILE. The title, behavior section, and example .env block all need updating to reflect the opt-in --env-file design shown here.

(Leorio P2, Mafu-san P2)

🤖

Fixed. PR title and description updated to reflect the opt-in --env-file design.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:44Z

+// are visible to serpent's Env-tag resolution for other options.
+func parseEnvFileFlag() string {
+	for i, arg := range os.Args[1:] {
+		if arg == "--env-file" && i+2 < len(os.Args) {


P3 [CRF-15] When --env-file is the last argument with no value (os.Args = ["develop", "--env-file"]), the bounds check i+2 < len(os.Args) fails and the function silently falls through to os.Getenv("CODER_DEV_ENV_FILE"). The developer typed --env-file intending to pass a path, but no diagnostic indicates the flag was ignored.

"The developer sees either 'develop: error loading env file' (if the env var is set to a bad path) or nothing at all (if the env var is unset). Neither message mentions the dangling flag." (Chopper)

Consider either erroring when --env-file appears without a value, or adding a test that pins the current fall-through behavior as intentional.

(Bisky P3, Chopper P3, Meruem P3)

🤖

Fixed in 263bde8. parseEnvFileFlag now prints "develop: --env-file requires a value" and exits 1 when --env-file is the last argument. Added subprocess test ErrorsWhenValueMissing to cover this.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:44Z

+// not already present in the process environment. It returns the number of
+// variables set.
+func loadEnvFile(path string) (int, error) {
+	vars, err := godotenv.Read(path)


P3 [CRF-16] godotenv.Read expands $VAR and ${VAR} references in unquoted and double-quoted values. Undefined variables resolve to empty string. Single-quoted values are not expanded.

This is new behavior vs. the R1 hand-rolled parser, which did not expand variables. A developer writing CODER_EXTERNAL_AUTH_0_CLIENT_SECRET=$ecret_value will get an empty secret (godotenv interprets $ecret_value as variable reference ecret_value, which is undefined). The --env-file help text or a code comment should note this. Workaround: single-quote values containing $.

Verified against godotenv's test suite (TestExpanding): "expands undefined variables to an empty string."

(Hisoka)

🤖

Documented in 263bde8. The --env-file option description now notes: "unquoted and double-quoted values undergo $VAR expansion; use single quotes for literal dollar signs." This is standard .env behavior and we're keeping it.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:45Z

+		require.Error(t, err)
+	})
+
+	t.Run("EmptyPathNoAction", func(t *testing.T) {


P3 [CRF-17] Test name EmptyPathNoAction implies the function takes no action, but the test asserts require.Error(t, err). The name should match the assertion: ErrorsOnEmptyPath.

(Gon)

🤖

Fixed in 263bde8. Renamed to ErrorsOnEmptyPath.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:45Z

+			_, _ = fmt.Fprintf(os.Stderr, "develop: error loading env file %q: %v\n", envFile, err)
+			os.Exit(1)
+		}
+		_, _ = fmt.Fprintf(os.Stderr, "develop: loaded %d variable(s) from %s\n", n, envFile)


Nit [CRF-18] Line 80 uses %q (quoted path) in the error message; line 83 uses %s (bare path) in the success message. Same variable, same stderr, 3 lines apart. Pick one format for consistency.

(Chopper, Gon, Leorio)

🤖

Fixed in 263bde8. Both messages now use %s.

🤖 Generated with Coder Agents

coder-agents-review · 2026-05-22T17:48:45Z

 	dbRollback        bool
 	dbReset           bool
 	dbContinue        bool
+	envFile           string


Nit [CRF-19] envFile is written by serpent but never read. The caller comment at lines 73-76 explains this design (pre-parse + --help discoverability), but a reader looking at the struct definition has no signal that the field is intentionally write-only. A brief comment on the field would help:

// envFile is populated by serpent for --help; the actual loading // uses parseEnvFileFlag() before serpent runs. envFile string

(Ryosuke Nit, Meruem P3)

🤖

Fixed in 263bde8. Added comment on the struct field explaining the pre-parse design.

🤖 Generated with Coder Agents

…ct marker

… CRF-19)

johnstcn · 2026-05-22T17:58:57Z

/coder-agents-review

coder-agents-review

All R2 findings addressed and verified. Good work across three rounds: the hand-rolled parser bugs from R1 are gone, CI-breaking issues from R2 are fixed, the PR description matches the implementation, and the dangling-flag edge case is handled.

1 P3 from the panel. 20 of 20 prior findings are resolved. The remaining item is a documentation refinement.

CI note: the title check is still failing. Worth resolving before merge.

"godotenv.Read returns a map. It never calls os.Getenv. The expansion scope is file-local only, and the docs don't say that." (Hisoka)

🤖 This review was automatically generated with Coder Agents.

coder-agents-review · 2026-05-22T18:18:13Z

+			{
+				Flag:        "env-file",
+				Env:         "CODER_DEV_ENV_FILE",
+				Description: "Path to a .env file to load before starting. Variables in the file do not override existing environment variables. Note: unquoted and double-quoted values undergo $VAR expansion; use single quotes for literal dollar signs.",


P3 [CRF-21] The Description documents $VAR expansion and the single-quote workaround, but omits the expansion scope: godotenv.Read() resolves $VAR references only against other variables defined earlier in the same file, not the process environment.

A developer writing MYPATH=$HOME/.config expects shell-like behavior ($HOME from the process env). With godotenv.Read(), $HOME resolves to empty string because it's not defined in the file. No error, no warning.

"godotenv.Read returns a map and does not call os.Getenv; only godotenv.Load (which this PR does not use) consults the process environment." (Razor, verified against parser.go:157,177)

Suggested addition to the Description: "Variables are expanded against other entries in the same file, not the process environment."

(Hisoka P3, Razor P3)

🤖

…s.Exit Satisfies revive deep-exit rule by removing os.Exit from non-main function.

johnstcn added 2 commits May 22, 2026 14:46

feat(scripts/develop): load .env file for local dev configuration

4801b6d

fix(scripts/develop): load .env before option parsing

69f5ae4

Copilot AI review requested due to automatic review settings May 22, 2026 14:48

Copilot started reviewing on behalf of johnstcn May 22, 2026 14:48 View session

github-actions Bot assigned johnstcn May 22, 2026

Copilot AI reviewed May 22, 2026

View reviewed changes

Comment thread scripts/develop/main.go Outdated

Comment thread scripts/develop/main.go Outdated

Comment thread scripts/develop/main_test.go

coder-agents-review Bot suggested changes May 22, 2026

View reviewed changes

johnstcn changed the title ~~feat(scripts/develop): load .env file for local dev configuration~~ feat: load .env file for local dev configuration May 22, 2026

coder-agents-review Bot suggested changes May 22, 2026

View reviewed changes

johnstcn added 2 commits May 22, 2026 17:52

fix(scripts/develop): add paralleltest nolint and fix godotenv indire…

cccdb91

…ct marker

fix(scripts/develop): address Round 2 review findings (CRF-15 through…

263bde8

… CRF-19)

johnstcn changed the title ~~feat: load .env file for local dev configuration~~ feat(scripts/develop): add --env-file flag for local dev configuration May 22, 2026

johnstcn changed the title ~~feat(scripts/develop): add --env-file flag for local dev configuration~~ feat: add --env-file flag for local dev configuration May 22, 2026

johnstcn changed the title ~~feat: add --env-file flag for local dev configuration~~ feat: add --env-file flag to develop.sh May 22, 2026

coder-agents-review Bot reviewed May 22, 2026

View reviewed changes

fix(scripts/develop): return error from parseEnvFileFlag instead of o…

f6f5245

…s.Exit Satisfies revive deep-exit rule by removing os.Exit from non-main function.

Conversation

johnstcn commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Behavior

Usage

Example .env

Uh oh!

johnstcn commented May 22, 2026

Uh oh!

coder-agents-review Bot commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Findings

Contested and acknowledged

Round log

Round 1

Round 2

Round 3

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

johnstcn commented May 22, 2026

Uh oh!

coder-agents-review Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

coder-agents-review Bot May 22, 2026

johnstcn commented May 22, 2026 •

edited

Loading

Example `.env`

coder-agents-review Bot commented May 22, 2026 •

edited

Loading