feat(cli): Comprehensive erst.toml configuration parsing by daveades · Pull Request #646 · dotandev/hintents

daveades · 2026-02-25T17:40:10Z

==============================================================================
TITLE:
feat(cli): Comprehensive erst.toml configuration parsing - Issue #409

=============================================================================
DESCRIPTION:

Overview

Consolidates CLI configuration so .erst.toml (system, home, local) is loaded first, then environment variables
override it. This unifies configuration precedence for CLI commands.

Changes

Core Implementation

Config precedence: /etc/erst/config.toml → ~/.erst.toml → ./.erst.toml, then env overrides.
Env overrides: Added support for ERST_SIM_PATH and ERST_CACHE_DIR aliases.
Parsing: Handles inline comments and boolean values in TOML, improves list parsing.
CLI usage: Debug/explain now read tokens via unified config loader.

Testing

Added precedence test for local overriding home config.
Updated config priority test to use TOML config file.

Verification

Tests not run in environment (go: command not found).

Related Issues

Closes #409

Type of Change

New feature
Bug fix
Breaking change
Documentation update

Checklist

Code follows project style guidelines
Self-review completed
Tests added/updated
Documentation updated
No new linting issues
Changes verified locally (Go toolchain unavailable)

================================================================================

Copilot

Pull request overview

This pull request implements comprehensive TOML configuration parsing for the erst CLI tool (Issue #409), establishing a hierarchical configuration system that loads settings from multiple sources with proper precedence. The PR description incorrectly includes extensive AWS KMS signing content (Issue #393), which is not implemented in this PR.

Changes:

Implemented TOML configuration file loading with precedence: /etc/erst/config.toml → ~/.erst.toml → ./.erst.toml → environment variables
Added support for environment variable aliases (ERST_SIM_PATH for ERST_SIMULATOR_PATH, ERST_CACHE_DIR for ERST_CACHE_PATH)
Migrated debug and explain commands from config.LoadConfig() to unified config.Load() function

Reviewed changes

Copilot reviewed 19 out of 21 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
internal/config/config.go	Refactored configuration loading to support TOML files with hierarchical precedence, added helper functions for parsing booleans and stripping inline comments, implemented environment variable override system
internal/config/config_test.go	Added test for local config overriding home config to verify precedence behavior
internal/cmd/debug.go	Updated to use new unified config.Load() function for token resolution
internal/cmd/explain.go	Updated to use new unified config.Load() function for token resolution
tests/config_priority_test.go	Updated test to use TOML config format instead of JSON

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

Copilot · 2026-02-25T17:46:35Z

internal/config/config.go

 	paths := []string{
-		".erst.toml",
-		filepath.Join(os.ExpandEnv("$HOME"), ".erst.toml"),
 		"/etc/erst/config.toml",


The hardcoded path "/etc/erst/config.toml" is Unix-specific and will not work correctly on Windows. On Windows, this path would be invalid and could cause issues. Consider using a platform-appropriate system config path or making this path configurable, or at least document that system-level configuration is only supported on Unix-like systems.

Copilot · 2026-02-25T17:46:35Z

tests/config_priority_test.go

+	// Create a dummy TOML config file
+	configFile := filepath.Join(tmpDir, ".erst.toml")
+	configData := []byte(`rpc_token = "CONFIG_TOKEN"`)
+	err := os.WriteFile(configFile, configData, 0600)


The PR description focuses heavily on AWS KMS signing features (Issue #393), but the actual code changes only implement TOML configuration parsing (Issue #409). The PR title correctly mentions "erst.toml configuration parsing", but the description should be updated to accurately reflect what this PR actually implements. The AWS KMS content appears to be copied from a different PR.

@copilot open a new pull request to apply changes based on this feedback

Copilot · 2026-02-25T17:46:35Z

internal/config/config_test.go

+
+	for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {
+		os.Unsetenv(key)
+	}


The test should restore the original values of the unset environment variables after the test completes. Currently, it only unsets them (line 332-334) but doesn't restore them in a defer block. This could cause test pollution if other tests depend on these environment variables. Consider saving the original values and restoring them in a defer function, similar to how HOME is handled on lines 320-322.

Suggested change

for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {

os.Unsetenv(key)

}

origEnv := make(map[string]struct {

value string

set bool

})

for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {

val, ok := os.LookupEnv(key)

origEnv[key] = struct {

value string

set bool

}{value: val, set: ok}

os.Unsetenv(key)

}

defer func() {

for key, state := range origEnv {

if state.set {

_ = os.Setenv(key, state.value)

} else {

_ = os.Unsetenv(key)

}

}

}()

Copilot · 2026-02-25T17:46:36Z

internal/config/config.go

+func parseBool(value string) bool {
+	switch strings.ToLower(strings.TrimSpace(value)) {
+	case "1", "true", "yes", "on":
+		return true
+	default:
+		return false
+	}
+}


The new helper function parseBool has been introduced but lacks unit test coverage. Given that it handles multiple boolean representations (1, true, yes, on), it would be beneficial to add explicit tests for this function to ensure all cases are properly handled, especially edge cases like "TRUE", "Yes", "ON" (uppercase variants) and invalid inputs like "false", "0", "no", "off".

Copilot · 2026-02-25T17:46:36Z

internal/config/config.go

+func stripInlineComment(line string) string {
+	inSingle := false
+	inDouble := false
+	for i, ch := range line {
+		if ch == '"' && !inSingle {
+			inDouble = !inDouble
+			continue
+		}
+		if ch == '\'' && !inDouble {
+			inSingle = !inSingle
+			continue
+		}
+		if ch == '#' && !inSingle && !inDouble {
+			return strings.TrimSpace(line[:i])
+		}
 	}
-	return defaultValue
+	return line
 }


The stripInlineComment function lacks unit test coverage. This function handles complex quote-aware comment parsing logic, including nested quote handling for single and double quotes. Given its complexity and importance for correct TOML parsing, dedicated unit tests should be added to verify behavior with edge cases like unmatched quotes, escaped quotes, and comments within strings.

Copilot · 2026-02-25T17:46:36Z

internal/config/config.go

+func stripInlineComment(line string) string {
+	inSingle := false
+	inDouble := false
+	for i, ch := range line {
+		if ch == '"' && !inSingle {
+			inDouble = !inDouble
+			continue
+		}
+		if ch == '\'' && !inDouble {
+			inSingle = !inSingle
+			continue
+		}
+		if ch == '#' && !inSingle && !inDouble {
+			return strings.TrimSpace(line[:i])
+		}
 	}
-	return defaultValue
+	return line
 }


The stripInlineComment function does not handle escaped quotes. For example, a string like key = "value with \" quote" would incorrectly toggle the inDouble state when encountering the escaped quote, potentially causing comments to be parsed incorrectly. Consider adding escape character handling by checking if the previous character is a backslash before toggling quote states.

Copilot · 2026-02-25T17:46:36Z

internal/config/config.go

 	lines := strings.Split(content, "\n")
 	for _, line := range lines {
-		line = strings.TrimSpace(line)
+		line = strings.TrimSpace(stripInlineComment(line))


The stripInlineComment function is called on every line of the TOML file, but the result is only used after TrimSpace. This means that if stripInlineComment returns a line with trailing spaces before a comment, those spaces will be trimmed anyway. Consider calling stripInlineComment before the initial TrimSpace to avoid unnecessary whitespace processing, or ensuring stripInlineComment returns an already-trimmed result for better performance.

Suggested change

line = strings.TrimSpace(stripInlineComment(line))

line = stripInlineComment(strings.TrimSpace(line))

Copilot · 2026-02-25T17:46:36Z

internal/config/config_test.go

+	for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {
+		os.Unsetenv(key)
+	}


The test should also unset or save/restore other environment variables that could affect the configuration loading, such as ERST_NETWORK, ERST_LOG_LEVEL, ERST_CACHE_PATH, etc. Currently, if any of these are set in the test environment, they could interfere with the test's ability to verify that local config properly overrides home config. Consider either unsetting all ERST_* environment variables or saving and restoring them similar to how HOME is handled.

Suggested change

for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {

os.Unsetenv(key)

}

envKeys := []string{

"ERST_RPC_URL",

"ERST_RPC_URLS",

"STELLAR_RPC_URLS",

"ERST_NETWORK",

"ERST_LOG_LEVEL",

"ERST_CACHE_PATH",

}

origEnv := make(map[string]string, len(envKeys))

for _, key := range envKeys {

if val, ok := os.LookupEnv(key); ok {

origEnv[key] = val

}

os.Unsetenv(key)

}

defer func() {

for _, key := range envKeys {

if val, ok := origEnv[key]; ok {

_ = os.Setenv(key, val)

} else {

_ = os.Unsetenv(key)

}

}

}()

dotandev · 2026-02-25T19:54:58Z

fix conflicts, please.

Copilot AI review requested due to automatic review settings February 25, 2026 17:40

Copilot started reviewing on behalf of daveades February 25, 2026 17:40 View session

Copilot AI reviewed Feb 25, 2026

View reviewed changes

daveades force-pushed the feat/cli-issue-93 branch from 8446922 to d42777b Compare February 25, 2026 22:27

daveades added 2 commits February 27, 2026 10:31

feat(cli): Comprehensive erst.toml configuration parsing

ceff17f

chore(license): fix missing headers

e226a26

daveades force-pushed the feat/cli-issue-93 branch from d42777b to e226a26 Compare February 27, 2026 09:34

chore: remove redundant emojis

d9ae616

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): Comprehensive erst.toml configuration parsing#646

feat(cli): Comprehensive erst.toml configuration parsing#646
daveades wants to merge 3 commits intodotandev:mainfrom
daveades:feat/cli-issue-93

daveades commented Feb 25, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

daveades Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

Copilot AI Feb 25, 2026

Uh oh!

dotandev commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-	for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {
-		os.Unsetenv(key)
-	}
+	origEnv := make(map[string]struct {
+		value string
+		set   bool
+	})
+	for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {
+		val, ok := os.LookupEnv(key)
+		origEnv[key] = struct {
+			value string
+			set   bool
+		}{value: val, set: ok}
+		os.Unsetenv(key)
+	}
+	defer func() {
+		for key, state := range origEnv {
+			if state.set {
+				_ = os.Setenv(key, state.value)
+			} else {
+				_ = os.Unsetenv(key)
+			}
+		}
+	}()

	line = strings.TrimSpace(stripInlineComment(line))
	line = stripInlineComment(strings.TrimSpace(line))

-	for _, key := range []string{"ERST_RPC_URL", "ERST_RPC_URLS", "STELLAR_RPC_URLS"} {
-		os.Unsetenv(key)
-	}
+	envKeys := []string{
+		"ERST_RPC_URL",
+		"ERST_RPC_URLS",
+		"STELLAR_RPC_URLS",
+		"ERST_NETWORK",
+		"ERST_LOG_LEVEL",
+		"ERST_CACHE_PATH",
+	}
+	origEnv := make(map[string]string, len(envKeys))
+	for _, key := range envKeys {
+		if val, ok := os.LookupEnv(key); ok {
+			origEnv[key] = val
+		}
+		os.Unsetenv(key)
+	}
+	defer func() {
+		for _, key := range envKeys {
+			if val, ok := origEnv[key]; ok {
+				_ = os.Setenv(key, val)
+			} else {
+				_ = os.Unsetenv(key)
+			}
+		}
+	}()

Conversation

daveades commented Feb 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes

Core Implementation

Testing

Verification

Related Issues

Type of Change

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

daveades Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 25, 2026

Choose a reason for hiding this comment

Uh oh!

dotandev commented Feb 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

daveades commented Feb 25, 2026 •

edited

Loading