feat: Separate treb.toml config from foundry.toml#37
Merged
solofberlin merged 11 commits intomainfrom Feb 27, 2026
Merged
Conversation
📝 WalkthroughWalkthroughThis PR makes treb.toml the preferred sender config, adds loading/merging and env expansion for treb.toml with foundry.toml fallback, introduces an interactive/non-interactive migrate-config command and deprecation warnings, updates init to create treb.toml, propagates config source/profile through runtime and run flows, and adds tests, fixtures, and docs. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI
participant Provider
participant TrebLoader
participant FoundryLoader
participant Runtime
participant Forge
User->>CLI: run command
CLI->>Provider: initialize
Provider->>TrebLoader: loadTrebConfig(projectRoot)
alt treb.toml present
TrebLoader-->>Provider: TrebFileConfig
Provider->>Provider: mergeTrebFileConfig(ns, senders)
Provider->>Runtime: set ConfigSource="treb.toml", FoundryProfile
else treb.toml absent
TrebLoader-->>Provider: nil
Provider->>FoundryLoader: load foundry.toml
FoundryLoader-->>Provider: FoundryConfig
Provider->>Provider: mergeFoundryTrebConfig(profile)
Provider->>Runtime: set ConfigSource="foundry.toml", FoundryProfile
end
CLI->>Runtime: run command (runtime config)
Runtime->>Forge: execute (env includes FOUNDRY_PROFILE)
Forge-->>CLI: result
CLI->>User: show results (includes "📦 Config source")
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
scripts/ralph/progress.txt (1)
1-3: Remove timestamped progress-log artifacts from source control.This file looks like local progress metadata and is unrelated to runtime behavior; keeping it in-repo will create churn on every update. Please remove it from the PR (or move to ignored/local-only artifacts).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/ralph/progress.txt` around lines 1 - 3, Delete the local progress artifact (the timestamped progress.txt) from the PR and stop tracking it: remove the file from the commit (unstage/delete it) and add an appropriate ignore rule for progress.txt in .gitignore so future local updates don’t get committed; then amend the commit or create a new one that removes the file from version control and updates .gitignore accordingly.internal/domain/config/runtime.go (1)
27-30: Consider constants (or a small enum-like type) forConfigSource.Using raw string values for source labels is typo-prone across provider/render/usecase paths; central constants in
internal/domain/configwould harden this.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/domain/config/runtime.go` around lines 27 - 30, Replace the raw string usage for ConfigSource with a small typed constant set: define a new type (e.g., ConfigSourceType string) and declare constants (e.g., ConfigSourceTreb = "treb.toml", ConfigSourceFoundry = "foundry.toml") in the internal/domain/config package, change the struct field ConfigSource string to ConfigSourceType, and update call sites to use these constants instead of string literals to avoid typos and centralize valid values (leave FoundryProfile unchanged).internal/usecase/show_config.go (1)
38-42:ConfigSourcefield is not populated by the use case.The
ShowConfigResultstruct includesConfigSource(line 14), but theRunmethod doesn't set it. Based on the PR context, the CLI layer appears to enrich this field fromapp.Config.ConfigSourcebefore rendering. This works but creates a subtle contract where the use case returns a partially populated result.Consider either:
- Documenting this expectation with a comment in the struct
- Injecting a config source provider into the use case to fully populate the result
📝 Optional: Add documentation comment
// ShowConfigResult contains the result of showing configuration type ShowConfigResult struct { Config *config.LocalConfig ConfigPath string Exists bool - ConfigSource string // "treb.toml" or "foundry.toml" + ConfigSource string // "treb.toml" or "foundry.toml" - populated by CLI layer from RuntimeConfig }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/usecase/show_config.go` around lines 38 - 42, The ShowConfigResult returned by the Run method is missing the ConfigSource field; update the use case so Run populates ConfigSource before returning (set ShowConfigResult.ConfigSource = <source>) by either injecting a config source provider into the use case (e.g., add a field like configSourceProvider with a method GetConfigSource() and call it in Run) or, if available, read it from the app config already accessible to the use case (e.g., uc.app.Config.ConfigSource) and assign that value to ShowConfigResult.ConfigSource; alternatively, if you prefer not to change behavior, add a comment on the ShowConfigResult struct documenting that the CLI enriches ConfigSource.test/integration/init_test.go (1)
38-64: Consider addingOutputArtifactsfor consistency.The
init_and_deploytest pre-creates atreb.tomlinPreSetupbut doesn't include it inOutputArtifacts, unlike the other two tests. While this may be intentional (focusing on deployment artifacts), addingOutputArtifacts: []string{"treb.toml"}would ensure the golden file captures the expected treb.toml state for this scenario as well.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/init_test.go` around lines 38 - 64, The test case "init_and_deploy" pre-creates a treb.toml in its PreSetup but omits it from the test's OutputArtifacts, so add OutputArtifacts: []string{"treb.toml"} to the init_and_deploy test definition to ensure the generated treb.toml is captured by the golden output; update the test struct next to Name/TestCmds to include the OutputArtifacts field so the harness records the file produced in PreSetup.internal/config/provider.go (1)
196-205: Asymmetric handling may cause unintended mutation.When
mergedis nil,profile.Trebis assigned directly without copying. Ifmerged.Senderswere later modified, it would mutate the originalFoundryConfig. For consistency and safety, consider always copying into a new map.♻️ Proposed fix for consistent copying
// If requesting a specific profile, merge it with default if namespace != "default" { if profile, ok := foundryConfig.Profile[namespace]; ok { - if merged == nil { - merged = profile.Treb - } else if profile.Treb != nil && profile.Treb.Senders != nil { + if profile.Treb != nil && profile.Treb.Senders != nil { + if merged == nil { + merged = &config.TrebConfig{ + Senders: make(map[string]config.SenderConfig), + } + } for k, v := range profile.Treb.Senders { merged.Senders[k] = v } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/provider.go` around lines 196 - 205, The code assigns profile.Treb directly to merged when merged is nil, which can cause later mutations of merged.Senders to affect the original FoundryConfig; update the logic in the block handling namespace/profile to always allocate a new Treb instance and deep-copy the Senders map instead of assigning profile.Treb directly: when merged is nil create merged = &Treb{} (or equivalent new struct), copy scalar fields from profile.Treb, and if profile.Treb.Senders != nil allocate a new map and copy each key/value into merged.Senders so no references to the original profile.Treb or its Senders map remain.internal/cli/migrate_config.go (2)
131-138: Consider preserving original file permissions.The cleanup writes with hardcoded
0644permissions, which may differ from the original file. Consider usingos.Statto preserve the original mode.♻️ Proposed fix
func cleanupFoundryToml(path string) error { + info, err := os.Stat(path) + if err != nil { + return err + } data, err := os.ReadFile(path) if err != nil { return err } cleaned := removeTrebFromFoundryToml(string(data)) - return os.WriteFile(path, []byte(cleaned), 0644) + return os.WriteFile(path, []byte(cleaned), info.Mode()) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/migrate_config.go` around lines 131 - 138, The cleanupFoundryToml function currently writes back the cleaned file with a hardcoded 0644 mode; change it to preserve the original file permissions by calling os.Stat(path) (or os.Lstat) to get the file's Mode().Perm() before writing, handle any Stat error, and pass that mode into os.WriteFile instead of 0644 so cleanupFoundryToml writes the file with its original permissions.
51-128: Consider moving migration business logic to usecase layer.The
runMigrateConfigfunction and its helpers (extractTrebProfiles,generateTrebToml,removeTrebFromFoundryToml) contain business logic directly in the CLI layer. Per the hexagonal architecture pattern, this logic should reside ininternal/usecase/with the CLI layer only handling command parsing and I/O orchestration.This keeps the CLI thin and makes the migration logic independently testable without Cobra dependencies.
Based on learnings: "CLI commands must be implemented using Cobra framework in internal/cli/ with corresponding use case implementations in internal/usecase/"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/migrate_config.go` around lines 51 - 128, The CLI currently contains migration business logic; extract that logic into a usecase under internal/usecase (e.g., a MigrateConfig usecase) and have runMigrateConfig delegate to it: move functions extractTrebProfiles, generateTrebToml and cleanupFoundryToml into the new usecase, change them to accept domainconfig.RuntimeConfig (or plain inputs) and return the generated TOML string and cleanup results/errors instead of printing or prompting, and keep all user interaction (confirmPrompt, color printing, file writes) in runMigrateConfig so it merely calls usecase.MigrateConfig(...) and acts on its outputs; update runMigrateConfig to call the usecase, handle returned content/error, and perform I/O (write treb.toml, show preview, prompt and call cleanup when requested).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cli/migrate_config_test.go`:
- Around line 473-476: The test fixture contains a real private-key-like value
under the TOML section [profile.default.treb.senders.anvil] with the field
private_key that trips secret scanners; replace that concrete key string with a
clearly non-sensitive placeholder (e.g. "REPLACE_WITH_TEST_PRIVATE_KEY" or
"0xPRIVATE_KEY_PLACEHOLDER") in the test data so the intent remains but no real
secret-like material is stored, and ensure the replacement still conforms to the
expected format consumed by the tests that reference the private_key field.
In `@internal/config/provider.go`:
- Around line 163-173: Foundry profile assignment incorrectly accepts empty
strings from activeNs.Profile/defaultNs.Profile causing loss of the documented
fallback to the namespace name; update the logic in provider.go where
foundryProfile is set (references: namespace, trebFile.Ns, activeNs.Profile,
defaultNs.Profile, merged.Senders) so that after retrieving activeNs.Profile or
defaultNs.Profile you check for an empty string and if empty assign the
namespace name (for the active branch use the variable namespace, for the
default branch use "default") instead of the empty value.
In `@internal/config/trebfile.go`:
- Around line 22-24: The TOML decoder call in trebfile.go (and similarly in
foundry.go) currently discards the DecodeFile metadata so unknown/misspelled
keys pass silently; update the call to capture the metadata (e.g., md, err :=
toml.DecodeFile(trebPath, &cfg)), keep the existing parse error handling, then
check md.Undecoded() and return a descriptive error (e.g., fmt.Errorf("unknown
keys in treb.toml: %v", undecoded)) when any undecoded keys exist; apply the
identical change for the DecodeFile usage in foundry.go so both functions
validate and reject unknown TOML keys.
In `@internal/usecase/init_project.go`:
- Around line 83-88: The createTrebToml failure is currently ignored in Execute;
update Execute (in init_project.go) to propagate createTrebToml failures by
checking the returned step from i.createTrebToml(ctx) and, if step.Success is
false, mark the overall result as failed (e.g., set result.Success = false and
result.TrebTomlCreated = false) and return the result (or otherwise stop further
success processing) so Execute does not report initialization as successful when
createTrebToml fails; refer to the createTrebToml call, the local variable step,
and result.TrebTomlCreated/result.Steps for where to apply the change.
In `@scripts/ralph/CLAUDE.md`:
- Around line 21-30: The Markdown file has fenced code blocks without language
identifiers which triggers MD040; update each triple-backtick fence that
surrounds the sections starting with "## [Date/Time] - [Story ID]" and "##
Codebase Patterns" (and the other similar fences around lines 38-43) to include
a language tag such as text (e.g., change ``` to ```text) so the linter
recognizes the fence language; ensure every opening and closing fence pair is
updated consistently.
In `@test/helpers/test_context.go`:
- Around line 350-373: The Clean routine currently ignores errors from file
operations which can leave test state dirty; capture and handle errors from
copyFile(fixtureTrebToml, workTrebToml), os.Remove(workTrebToml),
copyFile(fixtureFoundryToml, workFoundryToml) and
updateFoundryConfig(workFoundryToml, port1, port2): make copyFile and
updateFoundryConfig return error if they don't already, check each returned
error and either propagate it (change Clean to return error) or fail the test
immediately (call ctx.T.Fatalf / t.Fatalf) so failures are surfaced rather than
ignored, and log contextual messages including the target filenames and
operation names to aid debugging.
---
Nitpick comments:
In `@internal/cli/migrate_config.go`:
- Around line 131-138: The cleanupFoundryToml function currently writes back the
cleaned file with a hardcoded 0644 mode; change it to preserve the original file
permissions by calling os.Stat(path) (or os.Lstat) to get the file's
Mode().Perm() before writing, handle any Stat error, and pass that mode into
os.WriteFile instead of 0644 so cleanupFoundryToml writes the file with its
original permissions.
- Around line 51-128: The CLI currently contains migration business logic;
extract that logic into a usecase under internal/usecase (e.g., a MigrateConfig
usecase) and have runMigrateConfig delegate to it: move functions
extractTrebProfiles, generateTrebToml and cleanupFoundryToml into the new
usecase, change them to accept domainconfig.RuntimeConfig (or plain inputs) and
return the generated TOML string and cleanup results/errors instead of printing
or prompting, and keep all user interaction (confirmPrompt, color printing, file
writes) in runMigrateConfig so it merely calls usecase.MigrateConfig(...) and
acts on its outputs; update runMigrateConfig to call the usecase, handle
returned content/error, and perform I/O (write treb.toml, show preview, prompt
and call cleanup when requested).
In `@internal/config/provider.go`:
- Around line 196-205: The code assigns profile.Treb directly to merged when
merged is nil, which can cause later mutations of merged.Senders to affect the
original FoundryConfig; update the logic in the block handling namespace/profile
to always allocate a new Treb instance and deep-copy the Senders map instead of
assigning profile.Treb directly: when merged is nil create merged = &Treb{} (or
equivalent new struct), copy scalar fields from profile.Treb, and if
profile.Treb.Senders != nil allocate a new map and copy each key/value into
merged.Senders so no references to the original profile.Treb or its Senders map
remain.
In `@internal/domain/config/runtime.go`:
- Around line 27-30: Replace the raw string usage for ConfigSource with a small
typed constant set: define a new type (e.g., ConfigSourceType string) and
declare constants (e.g., ConfigSourceTreb = "treb.toml", ConfigSourceFoundry =
"foundry.toml") in the internal/domain/config package, change the struct field
ConfigSource string to ConfigSourceType, and update call sites to use these
constants instead of string literals to avoid typos and centralize valid values
(leave FoundryProfile unchanged).
In `@internal/usecase/show_config.go`:
- Around line 38-42: The ShowConfigResult returned by the Run method is missing
the ConfigSource field; update the use case so Run populates ConfigSource before
returning (set ShowConfigResult.ConfigSource = <source>) by either injecting a
config source provider into the use case (e.g., add a field like
configSourceProvider with a method GetConfigSource() and call it in Run) or, if
available, read it from the app config already accessible to the use case (e.g.,
uc.app.Config.ConfigSource) and assign that value to
ShowConfigResult.ConfigSource; alternatively, if you prefer not to change
behavior, add a comment on the ShowConfigResult struct documenting that the CLI
enriches ConfigSource.
In `@scripts/ralph/progress.txt`:
- Around line 1-3: Delete the local progress artifact (the timestamped
progress.txt) from the PR and stop tracking it: remove the file from the commit
(unstage/delete it) and add an appropriate ignore rule for progress.txt in
.gitignore so future local updates don’t get committed; then amend the commit or
create a new one that removes the file from version control and updates
.gitignore accordingly.
In `@test/integration/init_test.go`:
- Around line 38-64: The test case "init_and_deploy" pre-creates a treb.toml in
its PreSetup but omits it from the test's OutputArtifacts, so add
OutputArtifacts: []string{"treb.toml"} to the init_and_deploy test definition to
ensure the generated treb.toml is captured by the golden output; update the test
struct next to Name/TestCmds to include the OutputArtifacts field so the harness
records the file produced in PreSetup.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (45)
CLAUDE.mdinternal/adapters/forge/forge.gointernal/cli/config.gointernal/cli/migrate_config.gointernal/cli/migrate_config_test.gointernal/cli/render/config.gointernal/cli/render/init.gointernal/cli/root.gointernal/cli/root_test.gointernal/config/provider.gointernal/config/provider_test.gointernal/config/trebfile.gointernal/config/trebfile_test.gointernal/domain/config/runtime.gointernal/domain/config/trebfile.gointernal/usecase/init_project.gointernal/usecase/ports.gointernal/usecase/run_script.gointernal/usecase/show_config.goscripts/ralph/CLAUDE.mdscripts/ralph/progress.txtscripts/ralph/ralph.shtasks/prd-separate-config.mdtest/helpers/test_context.gotest/integration/init_test.gotest/integration/migrate_config_test.gotest/testdata/golden/integration/TestConfigCommand/config_remove_namespace/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_remove_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_remove_with_ns_alias/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_both_namespace_and_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_namespace/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_namespace_with_ns_alias/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_show_with_existing_config/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_show_with_network_not_set/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_update_existing_value/commands.goldentest/testdata/golden/integration/TestInitCommand/init_and_deploy/commands.goldentest/testdata/golden/integration/TestInitCommand/init_existing_project/commands.goldentest/testdata/golden/integration/TestInitCommand/init_existing_project/treb.toml.goldentest/testdata/golden/integration/TestInitCommand/init_new_project/commands.goldentest/testdata/golden/integration/TestInitCommand/init_new_project/treb.toml.goldentest/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/commands.goldentest/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/treb.toml.goldentest/testdata/project/foundry.tomltest/testdata/project/treb.toml
💤 Files with no reviewable changes (1)
- test/testdata/project/foundry.toml
Comment on lines
+473
to
+476
| [profile.default.treb.senders.anvil] | ||
| type = "private_key" # anvil user 0 | ||
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | ||
|
|
There was a problem hiding this comment.
Avoid real secret-like key material in test fixtures.
Line [475] contains a concrete private-key-looking value and is already flagged by secret scanning. Replace it with a non-sensitive placeholder so tests keep intent without tripping gitleaks.
Proposed fix
[profile.default.treb.senders.anvil]
type = "private_key" # anvil user 0
-private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
+private_key = "${ANVIL_PRIVATE_KEY}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [profile.default.treb.senders.anvil] | |
| type = "private_key" # anvil user 0 | |
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | |
| [profile.default.treb.senders.anvil] | |
| type = "private_key" # anvil user 0 | |
| private_key = "${ANVIL_PRIVATE_KEY}" | |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 475-475: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/migrate_config_test.go` around lines 473 - 476, The test fixture
contains a real private-key-like value under the TOML section
[profile.default.treb.senders.anvil] with the field private_key that trips
secret scanners; replace that concrete key string with a clearly non-sensitive
placeholder (e.g. "REPLACE_WITH_TEST_PRIVATE_KEY" or
"0xPRIVATE_KEY_PLACEHOLDER") in the test data so the intent remains but no real
secret-like material is stored, and ensure the replacement still conforms to the
expected format consumed by the tests that reference the private_key field.
Comment on lines
+163
to
+173
| // Overlay active namespace senders (if not "default") | ||
| if namespace != "default" { | ||
| if activeNs, ok := trebFile.Ns[namespace]; ok { | ||
| for k, v := range activeNs.Senders { | ||
| merged.Senders[k] = v | ||
| } | ||
| foundryProfile = activeNs.Profile | ||
| } | ||
| } else if defaultNs, ok := trebFile.Ns["default"]; ok { | ||
| foundryProfile = defaultNs.Profile | ||
| } |
There was a problem hiding this comment.
Empty Profile field causes incorrect fallback.
When activeNs.Profile or defaultNs.Profile is an empty string (field omitted in treb.toml), foundryProfile is set to "" instead of falling back to the namespace name. This breaks the documented behavior where profile "defaults to namespace name."
🐛 Proposed fix
// Overlay active namespace senders (if not "default")
if namespace != "default" {
if activeNs, ok := trebFile.Ns[namespace]; ok {
for k, v := range activeNs.Senders {
merged.Senders[k] = v
}
- foundryProfile = activeNs.Profile
+ if activeNs.Profile != "" {
+ foundryProfile = activeNs.Profile
+ }
}
} else if defaultNs, ok := trebFile.Ns["default"]; ok {
- foundryProfile = defaultNs.Profile
+ if defaultNs.Profile != "" {
+ foundryProfile = defaultNs.Profile
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Overlay active namespace senders (if not "default") | |
| if namespace != "default" { | |
| if activeNs, ok := trebFile.Ns[namespace]; ok { | |
| for k, v := range activeNs.Senders { | |
| merged.Senders[k] = v | |
| } | |
| foundryProfile = activeNs.Profile | |
| } | |
| } else if defaultNs, ok := trebFile.Ns["default"]; ok { | |
| foundryProfile = defaultNs.Profile | |
| } | |
| // Overlay active namespace senders (if not "default") | |
| if namespace != "default" { | |
| if activeNs, ok := trebFile.Ns[namespace]; ok { | |
| for k, v := range activeNs.Senders { | |
| merged.Senders[k] = v | |
| } | |
| if activeNs.Profile != "" { | |
| foundryProfile = activeNs.Profile | |
| } | |
| } | |
| } else if defaultNs, ok := trebFile.Ns["default"]; ok { | |
| if defaultNs.Profile != "" { | |
| foundryProfile = defaultNs.Profile | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/config/provider.go` around lines 163 - 173, Foundry profile
assignment incorrectly accepts empty strings from
activeNs.Profile/defaultNs.Profile causing loss of the documented fallback to
the namespace name; update the logic in provider.go where foundryProfile is set
(references: namespace, trebFile.Ns, activeNs.Profile, defaultNs.Profile,
merged.Senders) so that after retrieving activeNs.Profile or defaultNs.Profile
you check for an empty string and if empty assign the namespace name (for the
active branch use the variable namespace, for the default branch use "default")
instead of the empty value.
Comment on lines
+22
to
+24
| if _, err := toml.DecodeFile(trebPath, &cfg); err != nil { | ||
| return nil, fmt.Errorf("failed to parse treb.toml: %w", err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the actual file and the code in question
cat -n internal/config/trebfile.go | head -40Repository: trebuchet-org/treb-cli
Length of output: 1492
🏁 Script executed:
# Search for all TOML decode usages in the codebase
rg -n 'toml\.(DecodeFile|Unmarshal|Decode|NewDecoder)' --type go internalRepository: trebuchet-org/treb-cli
Length of output: 249
🏁 Script executed:
# Search for Undecoded() usage in the codebase
rg -n 'Undecoded\(' --type goRepository: trebuchet-org/treb-cli
Length of output: 48
🏁 Script executed:
# Let's also check if there's a go.mod or go.sum to verify the toml library version
cat go.mod | grep -i tomlRepository: trebuchet-org/treb-cli
Length of output: 152
🏁 Script executed:
# Find the TrebFileConfig struct definition
rg -n 'type TrebFileConfig' --type go -A 15Repository: trebuchet-org/treb-cli
Length of output: 683
🏁 Script executed:
# Check for tests related to config loading and validation
fd -e 'test.go' --path '*config*' | head -10Repository: trebuchet-org/treb-cli
Length of output: 299
🏁 Script executed:
# Look at test files for config validation patterns
rg -n 'treb\.toml|DecodeFile|Undecoded' --type go testRepository: trebuchet-org/treb-cli
Length of output: 3386
Reject unknown keys in TOML config files to prevent silent typos.
toml.DecodeFile() returns metadata including unknown/unrecognized keys via Undecoded(), but line 22 in trebfile.go (and line 34 in foundry.go) discard this metadata, allowing misspelled config keys to pass silently.
Use the metadata to validate and error on unknown keys:
md, err := toml.DecodeFile(trebPath, &cfg)
if err != nil {
return nil, fmt.Errorf("failed to parse treb.toml: %w", err)
}
if undecoded := md.Undecoded(); len(undecoded) > 0 {
return nil, fmt.Errorf("unknown keys in treb.toml: %v", undecoded)
}Apply the same fix to foundry.go.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/config/trebfile.go` around lines 22 - 24, The TOML decoder call in
trebfile.go (and similarly in foundry.go) currently discards the DecodeFile
metadata so unknown/misspelled keys pass silently; update the call to capture
the metadata (e.g., md, err := toml.DecodeFile(trebPath, &cfg)), keep the
existing parse error handling, then check md.Undecoded() and return a
descriptive error (e.g., fmt.Errorf("unknown keys in treb.toml: %v", undecoded))
when any undecoded keys exist; apply the identical change for the DecodeFile
usage in foundry.go so both functions validate and reject unknown TOML keys.
Comment on lines
+83
to
+88
| // Create treb.toml | ||
| step = i.createTrebToml(ctx) | ||
| result.Steps = append(result.Steps, step) | ||
| if step.Success { | ||
| result.TrebTomlCreated = true | ||
| } |
There was a problem hiding this comment.
Propagate createTrebToml failures from Execute.
If createTrebToml fails, Execute still returns success. That can report initialization as successful without a valid treb.toml.
Proposed fix
// Create treb.toml
step = i.createTrebToml(ctx)
result.Steps = append(result.Steps, step)
if step.Success {
result.TrebTomlCreated = true
+ } else {
+ return result, step.Error
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Create treb.toml | |
| step = i.createTrebToml(ctx) | |
| result.Steps = append(result.Steps, step) | |
| if step.Success { | |
| result.TrebTomlCreated = true | |
| } | |
| // Create treb.toml | |
| step = i.createTrebToml(ctx) | |
| result.Steps = append(result.Steps, step) | |
| if step.Success { | |
| result.TrebTomlCreated = true | |
| } else { | |
| return result, step.Error | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/usecase/init_project.go` around lines 83 - 88, The createTrebToml
failure is currently ignored in Execute; update Execute (in init_project.go) to
propagate createTrebToml failures by checking the returned step from
i.createTrebToml(ctx) and, if step.Success is false, mark the overall result as
failed (e.g., set result.Success = false and result.TrebTomlCreated = false) and
return the result (or otherwise stop further success processing) so Execute does
not report initialization as successful when createTrebToml fails; refer to the
createTrebToml call, the local variable step, and
result.TrebTomlCreated/result.Steps for where to apply the change.
Comment on lines
+350
to
+373
| if _, err := os.Stat(fixtureTrebToml); err == nil { | ||
| copyFile(fixtureTrebToml, workTrebToml) | ||
| } else { | ||
| os.Remove(workTrebToml) | ||
| } | ||
|
|
||
| // Restore foundry.toml from fixture and re-apply port config | ||
| // (tests may modify foundry.toml via PreSetup, e.g., migrate-config tests) | ||
| fixtureFoundryToml := filepath.Join(GetFixtureDir(), "foundry.toml") | ||
| workFoundryToml := filepath.Join(ctx.WorkDir, "foundry.toml") | ||
| copyFile(fixtureFoundryToml, workFoundryToml) | ||
| var port1, port2 int | ||
| for name, node := range ctx.AnvilNodes { | ||
| p := 0 | ||
| fmt.Sscanf(node.Port, "%d", &p) | ||
| if strings.Contains(name, "31337") { | ||
| port1 = p | ||
| } else { | ||
| port2 = p | ||
| } | ||
| } | ||
| if port1 > 0 && port2 > 0 { | ||
| updateFoundryConfig(workFoundryToml, port1, port2) | ||
| } |
There was a problem hiding this comment.
Handle restore/update errors in Clean to prevent state leakage across tests.
Line [351], Line [360], and Line [372] ignore errors. If copy/update fails, the context may be returned in a dirty state and cause non-deterministic test failures.
Proposed fix
fixtureTrebToml := filepath.Join(GetFixtureDir(), "treb.toml")
workTrebToml := filepath.Join(ctx.WorkDir, "treb.toml")
- if _, err := os.Stat(fixtureTrebToml); err == nil {
- copyFile(fixtureTrebToml, workTrebToml)
- } else {
- os.Remove(workTrebToml)
- }
+ if _, err := os.Stat(fixtureTrebToml); err == nil {
+ if err := copyFile(fixtureTrebToml, workTrebToml); err != nil {
+ return fmt.Errorf("failed to restore treb.toml: %w", err)
+ }
+ } else if os.IsNotExist(err) {
+ if err := os.Remove(workTrebToml); err != nil && !os.IsNotExist(err) {
+ return fmt.Errorf("failed to remove treb.toml: %w", err)
+ }
+ } else {
+ return fmt.Errorf("failed to stat fixture treb.toml: %w", err)
+ }
fixtureFoundryToml := filepath.Join(GetFixtureDir(), "foundry.toml")
workFoundryToml := filepath.Join(ctx.WorkDir, "foundry.toml")
- copyFile(fixtureFoundryToml, workFoundryToml)
+ if err := copyFile(fixtureFoundryToml, workFoundryToml); err != nil {
+ return fmt.Errorf("failed to restore foundry.toml: %w", err)
+ }
...
if port1 > 0 && port2 > 0 {
- updateFoundryConfig(workFoundryToml, port1, port2)
+ if err := updateFoundryConfig(workFoundryToml, port1, port2); err != nil {
+ return fmt.Errorf("failed to re-apply foundry ports: %w", err)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if _, err := os.Stat(fixtureTrebToml); err == nil { | |
| copyFile(fixtureTrebToml, workTrebToml) | |
| } else { | |
| os.Remove(workTrebToml) | |
| } | |
| // Restore foundry.toml from fixture and re-apply port config | |
| // (tests may modify foundry.toml via PreSetup, e.g., migrate-config tests) | |
| fixtureFoundryToml := filepath.Join(GetFixtureDir(), "foundry.toml") | |
| workFoundryToml := filepath.Join(ctx.WorkDir, "foundry.toml") | |
| copyFile(fixtureFoundryToml, workFoundryToml) | |
| var port1, port2 int | |
| for name, node := range ctx.AnvilNodes { | |
| p := 0 | |
| fmt.Sscanf(node.Port, "%d", &p) | |
| if strings.Contains(name, "31337") { | |
| port1 = p | |
| } else { | |
| port2 = p | |
| } | |
| } | |
| if port1 > 0 && port2 > 0 { | |
| updateFoundryConfig(workFoundryToml, port1, port2) | |
| } | |
| if _, err := os.Stat(fixtureTrebToml); err == nil { | |
| if err := copyFile(fixtureTrebToml, workTrebToml); err != nil { | |
| return fmt.Errorf("failed to restore treb.toml: %w", err) | |
| } | |
| } else if os.IsNotExist(err) { | |
| if err := os.Remove(workTrebToml); err != nil && !os.IsNotExist(err) { | |
| return fmt.Errorf("failed to remove treb.toml: %w", err) | |
| } | |
| } else { | |
| return fmt.Errorf("failed to stat fixture treb.toml: %w", err) | |
| } | |
| // Restore foundry.toml from fixture and re-apply port config | |
| // (tests may modify foundry.toml via PreSetup, e.g., migrate-config tests) | |
| fixtureFoundryToml := filepath.Join(GetFixtureDir(), "foundry.toml") | |
| workFoundryToml := filepath.Join(ctx.WorkDir, "foundry.toml") | |
| if err := copyFile(fixtureFoundryToml, workFoundryToml); err != nil { | |
| return fmt.Errorf("failed to restore foundry.toml: %w", err) | |
| } | |
| var port1, port2 int | |
| for name, node := range ctx.AnvilNodes { | |
| p := 0 | |
| fmt.Sscanf(node.Port, "%d", &p) | |
| if strings.Contains(name, "31337") { | |
| port1 = p | |
| } else { | |
| port2 = p | |
| } | |
| } | |
| if port1 > 0 && port2 > 0 { | |
| if err := updateFoundryConfig(workFoundryToml, port1, port2); err != nil { | |
| return fmt.Errorf("failed to re-apply foundry ports: %w", err) | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/helpers/test_context.go` around lines 350 - 373, The Clean routine
currently ignores errors from file operations which can leave test state dirty;
capture and handle errors from copyFile(fixtureTrebToml, workTrebToml),
os.Remove(workTrebToml), copyFile(fixtureFoundryToml, workFoundryToml) and
updateFoundryConfig(workFoundryToml, port1, port2): make copyFile and
updateFoundryConfig return error if they don't already, check each returned
error and either propagate it (change Clean to return error) or fail the test
immediately (call ctx.T.Fatalf / t.Fatalf) so failures are surfaced rather than
ignored, and log contextual messages including the target filenames and
operation names to aid debugging.
Add TrebFileConfig domain type with namespace-based sender config structure and loadTrebConfig parser that handles TOML parsing, profile defaulting, and environment variable expansion. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…pter Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…and write treb.toml Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After writing treb.toml, migrate-config now offers to remove [profile.*.treb.*] sections from foundry.toml using a line-based approach that preserves user formatting and comments. Non-interactive mode safely skips foundry.toml modification. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Update config show command to display treb.toml source info, add treb.toml to test project, update all config golden files, and add migrate-config integration tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
solofberlin
force-pushed
the
ralph/separate-config
branch
from
cf33b18 to
1f62d7f
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
internal/usecase/init_project.go (1)
83-88:⚠️ Potential issue | 🟠 Major
createTrebTomlfailures should be propagated.If
createTrebTomlfails,Executecontinues and returnsnilerror, potentially reporting successful initialization without a validtreb.toml. This differs fromcheckTrebSolLibrary(line 70) which returns early on failure.Proposed fix
// Create treb.toml step = i.createTrebToml(ctx) result.Steps = append(result.Steps, step) if step.Success { result.TrebTomlCreated = true + } else { + return result, step.Error }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/usecase/init_project.go` around lines 83 - 88, The Execute flow currently ignores failures from createTrebToml and proceeds returning nil, so modify Execute to treat createTrebToml failures like checkTrebSolLibrary: after calling i.createTrebToml(ctx) and appending the step to result.Steps, if step.Success is false (or step.Error is non-nil) return that error immediately and set appropriate result flags; ensure the check mirrors the early return behavior used for checkTrebSolLibrary so a failed createTrebToml stops execution and surfaces the error to the caller.internal/cli/migrate_config_test.go (1)
473-476:⚠️ Potential issue | 🟠 MajorAvoid private-key-like literal in test fixture.
Line [475] still uses a concrete private-key-looking value and is likely to trigger secret scanning.
🔐 Proposed fix
[profile.default.treb.senders.anvil] type = "private_key" # anvil user 0 -private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" +private_key = "${ANVIL_PRIVATE_KEY}"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/cli/migrate_config_test.go` around lines 473 - 476, The test fixture contains a concrete private-key-looking literal under the [profile.default.treb.senders.anvil] section (private_key) which triggers secret scanning; replace that value with a non-secret placeholder or clearly dummy string (e.g., "0xDEADBEEF..." or "REDACTED_PRIVATE_KEY") and update any tests that expect that exact string to accept the placeholder or a test-only generated key, ensuring changes reference the private_key field in the fixture so the secret-like literal is removed.internal/config/provider.go (1)
163-173:⚠️ Potential issue | 🟠 MajorPreserve namespace fallback when
profileis omitted.Line [169] and Line [172] overwrite
foundryProfilewith""whenprofileis unset intreb.toml. That loses the intended fallback to namespace/default.🐛 Proposed fix
if namespace != "default" { if activeNs, ok := trebFile.Ns[namespace]; ok { for k, v := range activeNs.Senders { merged.Senders[k] = v } - foundryProfile = activeNs.Profile + if activeNs.Profile != "" { + foundryProfile = activeNs.Profile + } } } else if defaultNs, ok := trebFile.Ns["default"]; ok { - foundryProfile = defaultNs.Profile + if defaultNs.Profile != "" { + foundryProfile = defaultNs.Profile + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/config/provider.go` around lines 163 - 173, The code overwrites foundryProfile with an empty string when a namespace entry has no Profile set; change the assignment so you only set foundryProfile from activeNs.Profile or defaultNs.Profile if the Profile value is non-empty (e.g., check activeNs.Profile != "" and defaultNs.Profile != "" before assigning), leaving the previous foundryProfile untouched to preserve fallback; locate the logic around trebFile.Ns, the namespace branch that merges Senders into merged.Senders and conditionally assigns foundryProfile, and guard those assignments accordingly.
🧹 Nitpick comments (1)
internal/usecase/show_config.go (1)
11-14: VerifyConfigSourceis initialized on everyShowConfigResultpath.
ConfigSourceis now part of the result contract, butRundoes not populate it. Please confirm no render path can emit an empty source.#!/bin/bash # Verify where ShowConfigResult is constructed and whether ConfigSource is set. rg -nP --type=go 'ShowConfigResult\s*{' rg -nP --type=go 'ConfigSource\s*:'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/usecase/show_config.go` around lines 11 - 14, ShowConfigResult.ConfigSource is not being filled on all return paths from the Run method, so ensure every construction/return of ShowConfigResult sets ConfigSource explicitly; find all places in the Run method (and any helper that constructs ShowConfigResult) and populate ConfigSource based on which config was used (e.g., set "treb.toml" when you loaded the local treb config, "foundry.toml" when you loaded the foundry config, or a deterministic default like "unknown" if no file was present). Update all branches that currently return a ShowConfigResult (including error/empty/existence branches) to assign ConfigSource together with Config, ConfigPath, and Exists so no render path emits an empty ConfigSource.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cli/migrate_config.go`:
- Around line 51-289: The CLI file contains full migration business logic
(extraction, transformation, file I/O and cleanup) that must be moved into an
internal/usecase so the CLI remains thin; create a usecase (e.g., type
MigrateConfigUsecase with a method Run/Execute) that encapsulates the behavior
currently implemented by runMigrateConfig, cleanupFoundryToml,
extractTrebProfiles, generateTrebToml, writeSenderConfig and
removeTrebFromFoundryToml (and the trebSectionHeaderRe/anySectionHeaderRe
regexes), have the usecase accept inputs (project root, NonInteractive flag, io
prompts or a Prompt interface) and return structured results/errors, then
refactor internal/cli/migrate_config.go to only wire Cobra flags, call the
usecase, and handle printing/prompts by delegating to the usecase via the Prompt
interface; ensure all file mutation (reading/writing foundry.toml and writing
treb.toml) and deterministic formatting live inside the new usecase.
In `@tasks/prd-separate-config.md`:
- Around line 51-54: The fenced warning block containing the text "Warning: treb
config detected in foundry.toml — this is deprecated. Run `treb migrate-config`
to move your config to treb.toml." lacks a language tag and triggers
markdownlint MD040; update the opening fence from ``` to ```text (or another
appropriate language like console) so the block becomes fenced with a language
tag to satisfy the linter.
- Line 144: The PRD example embeds a raw private key string in the private_key
variable which will trigger secret scanners and encourage unsafe copy/paste;
replace the literal value with an environment placeholder (e.g., reference an
env var like PRIVATE_KEY or a clearly redacted token placeholder) and update any
docs/comments to instruct users to set PRIVATE_KEY in their environment or
secrets manager instead of committing a raw key.
In `@test/integration/init_test.go`:
- Line 51: The TOML test fixture contains a hard-coded private_key literal
(private_key = "..."); replace it with an environment-interpolated placeholder
(e.g. private_key = "${TEST_PRIVATE_KEY}") and update the test setup code that
constructs the fixture to inject the value from os.Getenv("TEST_PRIVATE_KEY")
(or generate a key as a safe default) so no raw secret appears in the fixture
content; ensure the fixture-building function/method that writes the TOML uses
the env value and that tests set or mock TEST_PRIVATE_KEY appropriately.
- Around line 15-18: In the PreSetup closure ensure os.Remove errors are checked
and only ignored when the file doesn't exist: after calling
os.Remove(filepath.Join(ctx.WorkDir, "treb.toml")) capture the returned error
and if err != nil and !os.IsNotExist(err) call t.Fatalf (or t.Fatalf equivalent)
with a clear message including the error; apply the same robust error handling
change to the other PreSetup occurrence around the referenced lines so
unexpected removal failures fail the test setup instead of being silently
ignored.
In `@test/integration/migrate_config_test.go`:
- Around line 44-47: The fixture contains a private_key literal under the TOML
table profile.default.treb.senders.anvil which looks like a real secret; replace
that value with a non-sensitive placeholder (e.g. "PRIVATE_KEY_PLACEHOLDER" or a
clearly fake hex like "0xdeadbeef...") and update any assertions in
migrate_config_test.go that expect the exact original string to instead accept
or normalize the placeholder, ensuring the key is no longer a real-looking
private key and avoids secret-scan failures.
In
`@test/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/treb.toml.golden`:
- Around line 33-34: The golden treb.toml is missing the required private_key
for the sender "signer0": update the migration logic that builds ns.live.senders
(the code exercised by MigrateConfigCommand / migrateConfig path and the fixture
helper readFixtureFoundryTomlWithTreb()) to copy the private_key field from the
source foundry.toml for any sender with type = "private_key" (ensure the mapper
that processes sender entries preserves and writes private_key for key
"signer0"), or if the test intent is different, update the test fixture/golden
to include private_key = "${BASE_SEPOLIA_SIGNER0_PK}" so the generated treb.toml
is valid.
---
Duplicate comments:
In `@internal/cli/migrate_config_test.go`:
- Around line 473-476: The test fixture contains a concrete private-key-looking
literal under the [profile.default.treb.senders.anvil] section (private_key)
which triggers secret scanning; replace that value with a non-secret placeholder
or clearly dummy string (e.g., "0xDEADBEEF..." or "REDACTED_PRIVATE_KEY") and
update any tests that expect that exact string to accept the placeholder or a
test-only generated key, ensuring changes reference the private_key field in the
fixture so the secret-like literal is removed.
In `@internal/config/provider.go`:
- Around line 163-173: The code overwrites foundryProfile with an empty string
when a namespace entry has no Profile set; change the assignment so you only set
foundryProfile from activeNs.Profile or defaultNs.Profile if the Profile value
is non-empty (e.g., check activeNs.Profile != "" and defaultNs.Profile != ""
before assigning), leaving the previous foundryProfile untouched to preserve
fallback; locate the logic around trebFile.Ns, the namespace branch that merges
Senders into merged.Senders and conditionally assigns foundryProfile, and guard
those assignments accordingly.
In `@internal/usecase/init_project.go`:
- Around line 83-88: The Execute flow currently ignores failures from
createTrebToml and proceeds returning nil, so modify Execute to treat
createTrebToml failures like checkTrebSolLibrary: after calling
i.createTrebToml(ctx) and appending the step to result.Steps, if step.Success is
false (or step.Error is non-nil) return that error immediately and set
appropriate result flags; ensure the check mirrors the early return behavior
used for checkTrebSolLibrary so a failed createTrebToml stops execution and
surfaces the error to the caller.
---
Nitpick comments:
In `@internal/usecase/show_config.go`:
- Around line 11-14: ShowConfigResult.ConfigSource is not being filled on all
return paths from the Run method, so ensure every construction/return of
ShowConfigResult sets ConfigSource explicitly; find all places in the Run method
(and any helper that constructs ShowConfigResult) and populate ConfigSource
based on which config was used (e.g., set "treb.toml" when you loaded the local
treb config, "foundry.toml" when you loaded the foundry config, or a
deterministic default like "unknown" if no file was present). Update all
branches that currently return a ShowConfigResult (including
error/empty/existence branches) to assign ConfigSource together with Config,
ConfigPath, and Exists so no render path emits an empty ConfigSource.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (49)
CLAUDE.mdinternal/adapters/forge/forge.gointernal/adapters/repository/contracts/repository.gointernal/cli/config.gointernal/cli/migrate_config.gointernal/cli/migrate_config_test.gointernal/cli/render/config.gointernal/cli/render/init.gointernal/cli/root.gointernal/cli/root_test.gointernal/cli/verify.gointernal/config/provider.gointernal/config/provider_test.gointernal/config/trebfile.gointernal/config/trebfile_test.gointernal/domain/config/runtime.gointernal/domain/config/trebfile.gointernal/usecase/init_project.gointernal/usecase/list_deployments.gointernal/usecase/ports.gointernal/usecase/run_script.gointernal/usecase/show_config.gointernal/usecase/verify_deployment_test.goscripts/ralph/CLAUDE.mdscripts/ralph/progress.txtscripts/ralph/ralph.shtasks/prd-separate-config.mdtest/helpers/test_context.gotest/integration/init_test.gotest/integration/migrate_config_test.gotest/testdata/golden/integration/TestConfigCommand/config_remove_namespace/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_remove_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_remove_with_ns_alias/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_both_namespace_and_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_namespace/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_namespace_with_ns_alias/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_set_network/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_show_with_existing_config/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_show_with_network_not_set/commands.goldentest/testdata/golden/integration/TestConfigCommand/config_update_existing_value/commands.goldentest/testdata/golden/integration/TestInitCommand/init_and_deploy/commands.goldentest/testdata/golden/integration/TestInitCommand/init_existing_project/commands.goldentest/testdata/golden/integration/TestInitCommand/init_existing_project/treb.toml.goldentest/testdata/golden/integration/TestInitCommand/init_new_project/commands.goldentest/testdata/golden/integration/TestInitCommand/init_new_project/treb.toml.goldentest/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/commands.goldentest/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/treb.toml.goldentest/testdata/project/foundry.tomltest/testdata/project/treb.toml
💤 Files with no reviewable changes (1)
- test/testdata/project/foundry.toml
✅ Files skipped from review due to trivial changes (5)
- internal/usecase/list_deployments.go
- internal/cli/verify.go
- internal/usecase/verify_deployment_test.go
- internal/adapters/repository/contracts/repository.go
- scripts/ralph/progress.txt
🚧 Files skipped from review as they are similar to previous changes (21)
- internal/cli/config.go
- scripts/ralph/ralph.sh
- test/testdata/golden/integration/TestConfigCommand/config_remove_namespace/commands.golden
- internal/config/trebfile.go
- test/helpers/test_context.go
- internal/cli/root_test.go
- internal/adapters/forge/forge.go
- internal/domain/config/trebfile.go
- internal/cli/render/config.go
- test/testdata/golden/integration/TestConfigCommand/config_set_both_namespace_and_network/commands.golden
- internal/domain/config/runtime.go
- test/testdata/golden/integration/TestConfigCommand/config_remove_network/commands.golden
- test/testdata/golden/integration/TestConfigCommand/config_remove_with_ns_alias/commands.golden
- test/testdata/golden/integration/TestConfigCommand/config_set_network/commands.golden
- test/testdata/golden/integration/TestInitCommand/init_existing_project/commands.golden
- test/testdata/golden/integration/TestConfigCommand/config_set_namespace_with_ns_alias/commands.golden
- test/testdata/golden/integration/TestConfigCommand/config_set_namespace/commands.golden
- test/testdata/golden/integration/TestConfigCommand/config_update_existing_value/commands.golden
- test/testdata/golden/integration/TestInitCommand/init_existing_project/treb.toml.golden
- test/testdata/golden/integration/TestInitCommand/init_and_deploy/commands.golden
- test/testdata/golden/integration/TestInitCommand/init_new_project/treb.toml.golden
Comment on lines
+51
to
+289
| // runMigrateConfig performs the config migration from foundry.toml to treb.toml. | ||
| func runMigrateConfig(cfg *domainconfig.RuntimeConfig) error { | ||
| // Check if there's any treb config in foundry.toml to migrate | ||
| profiles := extractTrebProfiles(cfg.FoundryConfig) | ||
| if len(profiles) == 0 { | ||
| fmt.Println("No treb config found in foundry.toml — nothing to migrate.") | ||
| return nil | ||
| } | ||
|
|
||
| // Generate treb.toml content | ||
| content := generateTrebToml(profiles) | ||
|
|
||
| trebTomlPath := filepath.Join(cfg.ProjectRoot, "treb.toml") | ||
| foundryTomlPath := filepath.Join(cfg.ProjectRoot, "foundry.toml") | ||
|
|
||
| // Check if treb.toml already exists | ||
| if _, err := os.Stat(trebTomlPath); err == nil { | ||
| if cfg.NonInteractive { | ||
| // In non-interactive mode, overwrite | ||
| fmt.Fprintln(os.Stderr, "Warning: treb.toml already exists and will be overwritten.") | ||
| } else { | ||
| yellow := color.New(color.FgYellow) | ||
| yellow.Fprintln(os.Stderr, "Warning: treb.toml already exists.") | ||
| if !confirmPrompt("Overwrite existing treb.toml?") { | ||
| fmt.Println("Migration cancelled.") | ||
| return nil | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Show preview | ||
| if !cfg.NonInteractive { | ||
| fmt.Println("Generated treb.toml:") | ||
| fmt.Println() | ||
| fmt.Println(content) | ||
|
|
||
| if !confirmPrompt("Write this to treb.toml?") { | ||
| fmt.Println("Migration cancelled.") | ||
| return nil | ||
| } | ||
| } | ||
|
|
||
| // Write treb.toml | ||
| if err := os.WriteFile(trebTomlPath, []byte(content), 0644); err != nil { | ||
| return fmt.Errorf("failed to write treb.toml: %w", err) | ||
| } | ||
|
|
||
| green := color.New(color.FgGreen, color.Bold) | ||
| green.Printf("✓ treb.toml written successfully\n") | ||
|
|
||
| // Offer to clean up foundry.toml | ||
| cleanedUp := false | ||
| if !cfg.NonInteractive { | ||
| fmt.Println() | ||
| if confirmPrompt("Remove [profile.*.treb.*] sections from foundry.toml?") { | ||
| if err := cleanupFoundryToml(foundryTomlPath); err != nil { | ||
| return fmt.Errorf("failed to clean up foundry.toml: %w", err) | ||
| } | ||
| green.Printf("✓ foundry.toml cleaned up\n") | ||
| cleanedUp = true | ||
| } else { | ||
| fmt.Println("Skipped foundry.toml cleanup — you can remove [profile.*.treb.*] sections manually.") | ||
| } | ||
| } | ||
|
|
||
| // Print next steps | ||
| fmt.Println() | ||
| fmt.Println("Next steps:") | ||
| fmt.Println(" 1. Review the generated treb.toml") | ||
| if !cleanedUp { | ||
| fmt.Println(" 2. Remove [profile.*.treb.*] sections from foundry.toml") | ||
| fmt.Println(" 3. Run `treb config show` to verify your config is loaded correctly") | ||
| } else { | ||
| fmt.Println(" 2. Run `treb config show` to verify your config is loaded correctly") | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| // cleanupFoundryToml reads foundry.toml and removes [profile.*.treb.*] sections. | ||
| func cleanupFoundryToml(path string) error { | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| cleaned := removeTrebFromFoundryToml(string(data)) | ||
| return os.WriteFile(path, []byte(cleaned), 0644) | ||
| } | ||
|
|
||
| // trebProfile holds a foundry profile name and its treb sender config. | ||
| type trebProfile struct { | ||
| Name string | ||
| Senders map[string]domainconfig.SenderConfig | ||
| } | ||
|
|
||
| // extractTrebProfiles extracts foundry profiles that have treb sender configs. | ||
| func extractTrebProfiles(fc *domainconfig.FoundryConfig) []trebProfile { | ||
| if fc == nil { | ||
| return nil | ||
| } | ||
|
|
||
| var profiles []trebProfile | ||
| for name, profile := range fc.Profile { | ||
| if profile.Treb != nil && len(profile.Treb.Senders) > 0 { | ||
| profiles = append(profiles, trebProfile{ | ||
| Name: name, | ||
| Senders: profile.Treb.Senders, | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| // Sort by name for deterministic output (default first) | ||
| sort.Slice(profiles, func(i, j int) bool { | ||
| if profiles[i].Name == "default" { | ||
| return true | ||
| } | ||
| if profiles[j].Name == "default" { | ||
| return false | ||
| } | ||
| return profiles[i].Name < profiles[j].Name | ||
| }) | ||
|
|
||
| return profiles | ||
| } | ||
|
|
||
| // generateTrebToml generates well-formatted treb.toml content from foundry profiles. | ||
| func generateTrebToml(profiles []trebProfile) string { | ||
| var b strings.Builder | ||
|
|
||
| b.WriteString("# treb.toml — Treb sender configuration\n") | ||
| b.WriteString("#\n") | ||
| b.WriteString("# Each [ns.<name>] section defines a namespace with sender configs.\n") | ||
| b.WriteString("# The optional 'profile' field maps to a foundry.toml profile (defaults to namespace name).\n") | ||
| b.WriteString("#\n") | ||
| b.WriteString("# Migrated from foundry.toml [profile.*.treb.*] sections.\n") | ||
|
|
||
| for i, p := range profiles { | ||
| if i > 0 { | ||
| b.WriteString("\n") | ||
| } | ||
| b.WriteString("\n") | ||
|
|
||
| // Namespace header | ||
| fmt.Fprintf(&b, "[ns.%s]\n", p.Name) | ||
| // Always set profile explicitly so it's clear where it maps | ||
| fmt.Fprintf(&b, "profile = %q\n", p.Name) | ||
|
|
||
| // Sort sender names for deterministic output | ||
| senderNames := make([]string, 0, len(p.Senders)) | ||
| for name := range p.Senders { | ||
| senderNames = append(senderNames, name) | ||
| } | ||
| sort.Strings(senderNames) | ||
|
|
||
| for _, senderName := range senderNames { | ||
| sender := p.Senders[senderName] | ||
| b.WriteString("\n") | ||
| fmt.Fprintf(&b, "[ns.%s.senders.%s]\n", p.Name, senderName) | ||
| writeSenderConfig(&b, sender) | ||
| } | ||
| } | ||
|
|
||
| return b.String() | ||
| } | ||
|
|
||
| // writeSenderConfig writes a sender config's fields to the builder. | ||
| func writeSenderConfig(b *strings.Builder, s domainconfig.SenderConfig) { | ||
| fmt.Fprintf(b, "type = %q\n", string(s.Type)) | ||
|
|
||
| if s.Address != "" { | ||
| fmt.Fprintf(b, "address = %q\n", s.Address) | ||
| } | ||
| if s.PrivateKey != "" { | ||
| fmt.Fprintf(b, "private_key = %q\n", s.PrivateKey) | ||
| } | ||
| if s.Safe != "" { | ||
| fmt.Fprintf(b, "safe = %q\n", s.Safe) | ||
| } | ||
| if s.Signer != "" { | ||
| fmt.Fprintf(b, "signer = %q\n", s.Signer) | ||
| } | ||
| if s.DerivationPath != "" { | ||
| fmt.Fprintf(b, "derivation_path = %q\n", s.DerivationPath) | ||
| } | ||
| if s.Governor != "" { | ||
| fmt.Fprintf(b, "governor = %q\n", s.Governor) | ||
| } | ||
| if s.Timelock != "" { | ||
| fmt.Fprintf(b, "timelock = %q\n", s.Timelock) | ||
| } | ||
| if s.Proposer != "" { | ||
| fmt.Fprintf(b, "proposer = %q\n", s.Proposer) | ||
| } | ||
| } | ||
|
|
||
| // trebSectionHeaderRe matches [profile.<name>.treb] and [profile.<name>.treb.senders.<sender>] headers. | ||
| var trebSectionHeaderRe = regexp.MustCompile(`^\[profile\.[^]]+\.treb(?:\.[^]]+)?\]\s*$`) | ||
|
|
||
| // anySectionHeaderRe matches any TOML section header like [something] or [a.b.c]. | ||
| var anySectionHeaderRe = regexp.MustCompile(`^\[.+\]\s*$`) | ||
|
|
||
| // removeTrebFromFoundryToml removes [profile.*.treb.*] sections from foundry.toml content | ||
| // using a line-based approach to preserve user formatting and comments. | ||
| func removeTrebFromFoundryToml(content string) string { | ||
| lines := strings.Split(content, "\n") | ||
| var result []string | ||
| inTrebSection := false | ||
|
|
||
| for _, line := range lines { | ||
| trimmed := strings.TrimSpace(line) | ||
|
|
||
| if anySectionHeaderRe.MatchString(trimmed) { | ||
| if trebSectionHeaderRe.MatchString(trimmed) { | ||
| inTrebSection = true | ||
| continue | ||
| } | ||
| inTrebSection = false | ||
| } | ||
|
|
||
| if inTrebSection { | ||
| // Skip key-value lines, comments, and blank lines within treb sections | ||
| continue | ||
| } | ||
|
|
||
| result = append(result, line) | ||
| } | ||
|
|
||
| // Clean up excess trailing blank lines (collapse to single trailing newline) | ||
| output := strings.Join(result, "\n") | ||
| for strings.HasSuffix(output, "\n\n") { | ||
| output = strings.TrimSuffix(output, "\n") | ||
| } | ||
| if !strings.HasSuffix(output, "\n") { | ||
| output += "\n" | ||
| } | ||
|
|
||
| return output | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move migration business logic to internal/usecase and keep CLI thin.
This file currently owns orchestration, extraction, transformation, and file mutation logic. Keep internal/cli/migrate_config.go focused on Cobra wiring and delegate migration behavior to a use case in internal/usecase/.
As per coding guidelines: CLI commands must be implemented using Cobra framework in internal/cli/ with corresponding use case implementations in internal/usecase/.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/cli/migrate_config.go` around lines 51 - 289, The CLI file contains
full migration business logic (extraction, transformation, file I/O and cleanup)
that must be moved into an internal/usecase so the CLI remains thin; create a
usecase (e.g., type MigrateConfigUsecase with a method Run/Execute) that
encapsulates the behavior currently implemented by runMigrateConfig,
cleanupFoundryToml, extractTrebProfiles, generateTrebToml, writeSenderConfig and
removeTrebFromFoundryToml (and the trebSectionHeaderRe/anySectionHeaderRe
regexes), have the usecase accept inputs (project root, NonInteractive flag, io
prompts or a Prompt interface) and return structured results/errors, then
refactor internal/cli/migrate_config.go to only wire Cobra flags, call the
usecase, and handle printing/prompts by delegating to the usecase via the Prompt
interface; ensure all file mutation (reading/writing foundry.toml and writing
treb.toml) and deterministic formatting live inside the new usecase.
Comment on lines
+51
to
+54
| ``` | ||
| Warning: treb config detected in foundry.toml — this is deprecated. | ||
| Run `treb migrate-config` to move your config to treb.toml. | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced warning block.
The fenced block at Line 51 has no language tag and will trip markdownlint (MD040).
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 51-51: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tasks/prd-separate-config.md` around lines 51 - 54, The fenced warning block
containing the text "Warning: treb config detected in foundry.toml — this is
deprecated. Run `treb migrate-config` to move your config to treb.toml." lacks a
language tag and triggers markdownlint MD040; update the opening fence from ```
to ```text (or another appropriate language like console) so the block becomes
fenced with a language tag to satisfy the linter.
|
|
||
| [ns.default.senders.anvil] | ||
| type = "private_key" | ||
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" |
There was a problem hiding this comment.
Avoid embedding a raw private key in the PRD example.
Even test/dev keys trigger secret scanners and encourage unsafe copy/paste. Prefer an env placeholder.
Suggested doc-safe change
-private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
+private_key = "${ANVIL_PRIVATE_KEY}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | |
| private_key = "${ANVIL_PRIVATE_KEY}" |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 144-144: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tasks/prd-separate-config.md` at line 144, The PRD example embeds a raw
private key string in the private_key variable which will trigger secret
scanners and encourage unsafe copy/paste; replace the literal value with an
environment placeholder (e.g., reference an env var like PRIVATE_KEY or a
clearly redacted token placeholder) and update any docs/comments to instruct
users to set PRIVATE_KEY in their environment or secrets manager instead of
committing a raw key.
Comment on lines
+15
to
+18
| PreSetup: func(t *testing.T, ctx *helpers.TestContext) { | ||
| // Remove fixture treb.toml so init creates a fresh one | ||
| os.Remove(filepath.Join(ctx.WorkDir, "treb.toml")) | ||
| }, |
There was a problem hiding this comment.
Handle unexpected os.Remove errors in PreSetup.
Right now all remove failures are ignored. If deletion fails for reasons other than non-existence, setup can silently drift and produce misleading test failures.
Suggested hardening
import (
+ "errors"
"os"
"path/filepath"
"testing"
@@
- os.Remove(filepath.Join(ctx.WorkDir, "treb.toml"))
+ if err := os.Remove(filepath.Join(ctx.WorkDir, "treb.toml")); err != nil && !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("failed to remove prebaked treb.toml: %v", err)
+ }
@@
- os.Remove(filepath.Join(ctx.WorkDir, "treb.toml"))
+ if err := os.Remove(filepath.Join(ctx.WorkDir, "treb.toml")); err != nil && !errors.Is(err, os.ErrNotExist) {
+ t.Fatalf("failed to remove prebaked treb.toml: %v", err)
+ }Also applies to: 26-29
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integration/init_test.go` around lines 15 - 18, In the PreSetup closure
ensure os.Remove errors are checked and only ignored when the file doesn't
exist: after calling os.Remove(filepath.Join(ctx.WorkDir, "treb.toml")) capture
the returned error and if err != nil and !os.IsNotExist(err) call t.Fatalf (or
t.Fatalf equivalent) with a clear message including the error; apply the same
robust error handling change to the other PreSetup occurrence around the
referenced lines so unexpected removal failures fail the test setup instead of
being silently ignored.
|
|
||
| [ns.default.senders.anvil] | ||
| type = "private_key" | ||
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" |
There was a problem hiding this comment.
Replace the raw private key literal in test fixture content.
Line 51 is flagged by secret scanning and can break CI/security gates. Prefer env interpolation in the TOML snippet.
Suggested change
-private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
+private_key = "${ANVIL_PRIVATE_KEY}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | |
| private_key = "${ANVIL_PRIVATE_KEY}" |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integration/init_test.go` at line 51, The TOML test fixture contains a
hard-coded private_key literal (private_key = "..."); replace it with an
environment-interpolated placeholder (e.g. private_key = "${TEST_PRIVATE_KEY}")
and update the test setup code that constructs the fixture to inject the value
from os.Getenv("TEST_PRIVATE_KEY") (or generate a key as a safe default) so no
raw secret appears in the fixture content; ensure the fixture-building
function/method that writes the TOML uses the env value and that tests set or
mock TEST_PRIVATE_KEY appropriately.
Comment on lines
+44
to
+47
| [profile.default.treb.senders.anvil] | ||
| type = "private_key" | ||
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | ||
|
|
There was a problem hiding this comment.
Replace private-key-like fixture literal to avoid secret-scan failures.
Line [46] embeds a real-looking private key value. Even in tests, this is high-friction for security scanning and repo hygiene.
🔐 Proposed fixture-safe replacement
[profile.default.treb.senders.anvil]
type = "private_key"
-private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80"
+private_key = "${ANVIL_PRIVATE_KEY}"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [profile.default.treb.senders.anvil] | |
| type = "private_key" | |
| private_key = "0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80" | |
| [profile.default.treb.senders.anvil] | |
| type = "private_key" | |
| private_key = "${ANVIL_PRIVATE_KEY}" |
🧰 Tools
🪛 Gitleaks (8.30.0)
[high] 46-46: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/integration/migrate_config_test.go` around lines 44 - 47, The fixture
contains a private_key literal under the TOML table
profile.default.treb.senders.anvil which looks like a real secret; replace that
value with a non-sensitive placeholder (e.g. "PRIVATE_KEY_PLACEHOLDER" or a
clearly fake hex like "0xdeadbeef...") and update any assertions in
migrate_config_test.go that expect the exact original string to instead accept
or normalize the placeholder, ensuring the key is no longer a real-looking
private key and avoids secret-scan failures.
Comment on lines
+33
to
+34
| [ns.live.senders.signer0] | ||
| type = "private_key" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check source fixture for signer0 configuration
echo "=== Source fixture signer0 config ==="
sed -n '/\[ns\.live\.senders\.signer0\]/,/^\[/p' test/testdata/project/treb.toml | head -10
echo ""
echo "=== Golden output signer0 config ==="
sed -n '/\[ns\.live\.senders\.signer0\]/,/^\[/p' test/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/treb.toml.golden | head -10
echo ""
echo "=== Migration logic - search for private_key field handling ==="
find internal -name "*.go" -exec rg -l "migrate" {} \;Repository: trebuchet-org/treb-cli
Length of output: 474
🏁 Script executed:
#!/bin/bash
# Examine the migration logic for sender configuration handling
echo "=== Searching for sender-related migration code ==="
rg -n "private_key|signer|sender" internal/cli/migrate_config.go | head -40
echo ""
echo "=== Look for sender struct definitions ==="
rg -B2 -A5 "type.*Sender|signer0" internal/cli/migrate_config.go | head -50Repository: trebuchet-org/treb-cli
Length of output: 1232
🏁 Script executed:
#!/bin/bash
# Get the full writeSenderConfig function
echo "=== Full writeSenderConfig function ==="
sed -n '217,250p' internal/cli/migrate_config.goRepository: trebuchet-org/treb-cli
Length of output: 1175
🏁 Script executed:
#!/bin/bash
# Find the code that extracts sender configuration from foundry.toml
echo "=== Search for sender extraction logic ==="
sed -n '147,220p' internal/cli/migrate_config.goRepository: trebuchet-org/treb-cli
Length of output: 2255
🏁 Script executed:
#!/bin/bash
# Find the test that generates the golden file
echo "=== Search for test files related to migrate_config ==="
find . -name "*test.go" -path "*/cli/*" -exec rg -l "migrate_config_creates_treb_toml|TestMigrateConfigCommand" {} \;
echo ""
echo "=== Check if there's a test that validates golden files ==="
rg -l "golden" internal/cli/*test.goRepository: trebuchet-org/treb-cli
Length of output: 182
🏁 Script executed:
#!/bin/bash
# Search for references to the golden file path
echo "=== Search for references to the golden file ==="
rg "migrate_config_creates_treb_toml" --type go
echo ""
echo "=== Check root_test.go for migrate config test ==="
rg -B5 -A20 "TestMigrateConfigCommand" internal/cli/root_test.goRepository: trebuchet-org/treb-cli
Length of output: 251
🏁 Script executed:
#!/bin/bash
# Examine the migrate config test
echo "=== Full migrate_config_test.go ==="
wc -l test/integration/migrate_config_test.go
echo ""
echo "=== Content of migrate_config_test.go ==="
cat test/integration/migrate_config_test.goRepository: trebuchet-org/treb-cli
Length of output: 6240
signer0 sender config in the golden file is invalid: type = "private_key" without the required private_key field.
The test fixture at readFixtureFoundryTomlWithTreb() includes private_key="${BASE_SEPOLIA_SIGNER0_PK}" for the signer0 sender, but the generated golden file omits this field entirely. According to the config schema, a sender with type = "private_key" must include a private_key field. The golden file should contain:
[ns.live.senders.signer0]
type = "private_key"
private_key = "${BASE_SEPOLIA_SIGNER0_PK}"
This indicates either the migration code is not parsing/preserving the field from foundry.toml, or the golden file was corrupted. The generated treb.toml is invalid as-is.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@test/testdata/golden/integration/TestMigrateConfigCommand/migrate_config_creates_treb_toml/treb.toml.golden`
around lines 33 - 34, The golden treb.toml is missing the required private_key
for the sender "signer0": update the migration logic that builds ns.live.senders
(the code exercised by MigrateConfigCommand / migrateConfig path and the fixture
helper readFixtureFoundryTomlWithTreb()) to copy the private_key field from the
source foundry.toml for any sender with type = "private_key" (ensure the mapper
that processes sender entries preserves and writes private_key for key
"signer0"), or if the test intent is different, update the test fixture/golden
to include private_key = "${BASE_SEPOLIA_SIGNER0_PK}" so the generated treb.toml
is valid.
The test helper baseRunScriptConfig() was missing the FoundryProfile field, which now needs to be explicitly set since buildEnv uses config.FoundryProfile instead of config.Namespace. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bump Go test timeout to 15m and workflow job timeout to 20m to reduce flaky timeout failures in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/adapters/forge/forge_test.go`:
- Around line 25-28: One of the tests initializes both Namespace and
FoundryProfile to "default", which masks bugs that copy Namespace into
FOUNDRY_PROFILE; update at least one test case in
internal/adapters/forge/forge_test.go to use distinct values (e.g., Namespace =
"namespace-x", FoundryProfile = "foundry-profile-x") when constructing the
struct that contains Namespace and FoundryProfile, and add an assertion that the
produced environment/configuration sets FOUNDRY_PROFILE (or the value returned
by the code under test) equal to the FoundryProfile field to ensure the two are
not accidentally aliased.
Comment on lines
+25
to
+28
| Namespace: "default", | ||
| FoundryProfile: "default", | ||
| Script: &models.Contract{Name: "DeployCounter", Path: "script/deploy/DeployCounter.s.sol"}, | ||
| Parameters: map[string]string{"LABEL": "v1"}, |
There was a problem hiding this comment.
Differentiate Namespace and FoundryProfile in at least one test case
Initializing both fields to "default" weakens regression coverage: a bug that sources FOUNDRY_PROFILE from Namespace would still pass current assertions. Use distinct values in one test and assert FOUNDRY_PROFILE matches FoundryProfile.
Proposed test hardening
func TestBuildEnv_WithoutForkOverride(t *testing.T) {
adapter := newTestForgeAdapter()
cfg := baseRunScriptConfig()
+ cfg.Namespace = "ns-default"
+ cfg.FoundryProfile = "ci-profile"
env := adapter.buildEnv(cfg)
envMap := envToMap(env)
_, hasForkVar := envMap["SEPOLIA_RPC_URL"]
assert.False(t, hasForkVar, "should not have fork RPC override when no fork is active")
- assert.Equal(t, "default", envMap["FOUNDRY_PROFILE"])
+ assert.Equal(t, "ci-profile", envMap["FOUNDRY_PROFILE"])
assert.Equal(t, "sepolia", envMap["NETWORK"])
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| Namespace: "default", | |
| FoundryProfile: "default", | |
| Script: &models.Contract{Name: "DeployCounter", Path: "script/deploy/DeployCounter.s.sol"}, | |
| Parameters: map[string]string{"LABEL": "v1"}, | |
| func TestBuildEnv_WithoutForkOverride(t *testing.T) { | |
| adapter := newTestForgeAdapter() | |
| cfg := baseRunScriptConfig() | |
| cfg.Namespace = "ns-default" | |
| cfg.FoundryProfile = "ci-profile" | |
| env := adapter.buildEnv(cfg) | |
| envMap := envToMap(env) | |
| _, hasForkVar := envMap["SEPOLIA_RPC_URL"] | |
| assert.False(t, hasForkVar, "should not have fork RPC override when no fork is active") | |
| assert.Equal(t, "ci-profile", envMap["FOUNDRY_PROFILE"]) | |
| assert.Equal(t, "sepolia", envMap["NETWORK"]) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@internal/adapters/forge/forge_test.go` around lines 25 - 28, One of the tests
initializes both Namespace and FoundryProfile to "default", which masks bugs
that copy Namespace into FOUNDRY_PROFILE; update at least one test case in
internal/adapters/forge/forge_test.go to use distinct values (e.g., Namespace =
"namespace-x", FoundryProfile = "foundry-profile-x") when constructing the
struct that contains Namespace and FoundryProfile, and add an assertion that the
produced environment/configuration sets FOUNDRY_PROFILE (or the value returned
by the code under test) equal to the FoundryProfile field to ensure the two are
not accidentally aliased.
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
1 participant
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.
Summary
treb.tomlas the dedicated config file for treb sender profiles, replacing the legacy[profile.*.treb.*]sections infoundry.tomltreb migrate-configcommand to automatically migrate sender config fromfoundry.tomltotreb.tomland clean up legacy entriestreb initto generatetreb.tomlinstead of writing tofoundry.tomlfoundry.tomlUser Stories Implemented
Test plan
make unit-test)make integration-test)make lint)treb initgeneratestreb.tomltreb migrate-configmigrates from foundry.toml to treb.toml🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Improvements
Tests