Hide ignored base_url in public config output and lint internal-tagged code in CI#11
Hide ignored base_url in public config output and lint internal-tagged code in CI#11wilsonsilva wants to merge 3 commits into
base_url in public config output and lint internal-tagged code in CI#11Conversation
WalkthroughThis PR introduces build-tag-based separation of base URL configuration behavior to support internal testing and development. Public builds lock the API endpoint to production via Go build tags; internal builds allow override via environment variable ( 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches⚔️ Resolve merge conflicts
✨ Simplify code
Comment |
wilsonsilva
force-pushed
the
runware-base-url
branch
from
4f46464 to
8de1154
Compare
wilsonsilva
force-pushed
the
runware-base-url
branch
from
8de1154 to
103cb5d
Compare
danmrichards
left a comment
danmrichards
left a comment
There was a problem hiding this comment.
I think this is a great feature to have for internal use, but it's not appropriate for the public build. Customers all use https://api.runware.ai/v1 and we don't want them to be able to reverse engineer access to our staging or internal dev environments.
I'd suggest making use of golang build tags so that we can create a dedicated internal build of the app that has this feature.
Add support for overriding the default API endpoint via config and env. Introduces Config.BaseURL (yaml key base_url) and binds RUNWARE_BASE_URL in viper; GetBaseURL() now returns the env/config value before falling back to DefaultBaseURL. Update CLI to show and tab-complete the Base URL, add README docs for the env var and config key, and add tests covering default, config, and env precedence.
Add an `internal` build tag allowing an internal-only override of the API base URL while keeping public binaries locked to the production endpoint. Changes include: - Split base URL and env binding logic into build-tagged files under internal/config (baseurl_internal.go, baseurl_public.go, bindenv_internal.go, bindenv_public.go) and move GetBaseURL into the tagged files. - Add tests for both internal and public variants (baseurl_internal_test.go, baseurl_public_test.go). - Factor `config set` completion/validation into build-tagged cmd files so public builds omit/reject the `base_url` key (config_keys_internal.go, config_keys_public.go) and update config_cmd.go to use the new helpers. - Add docs describing the internal build and usage (docs/INTERNAL.md). - Update CI to build and test the internal variant, and add Makefile targets for build-internal and test-internal (produces runware-internal binary). Clean target now removes runware-internal. - Remove RUNWARE_BASE_URL mention from the public README table to reflect that the env override is not available in public builds. This change ensures the CLI cannot be redirected to staging/internal endpoints in public releases while providing an internal binary for testing and debugging. Internal binaries must not be published. Addresses: #11 (review)
wilsonsilva
force-pushed
the
runware-base-url
branch
from
103cb5d to
507ab4e
Compare
wilsonsilva
changed the title
|
@danmrichards Added the build tag in 507ab4e |
There was a problem hiding this comment.
Pull request overview
Adds a build-tag-gated override for the API base URL. The public release is hard-locked to https://api.runware.ai/v1, while a separate internal build (compiled with -tags internal) honors a RUNWARE_BASE_URL env var and a base_url config key (env > config > default). The split is implemented via paired _public.go / _internal.go files in internal/config and cmd, with new Make targets, a CI job, and dedicated docs.
Changes:
- New
internalbuild tag togglesGetBaseURL()between locked-default and env/config-aware resolution, and gates whetherbase_urlis bindable in viper and settable viaconfig set. - Build/test tooling:
make build-internal/make test-internal, CI compiles and tests the internal variant, anddocs/INTERNAL.mddocuments the workflow. config showalways displays aBase URLrow (resolved viaGetBaseURL());config setrejects unknown keys via the newisSettableKeyshim.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/config/config.go | Adds Config.BaseURL field, calls bindInternalEnv(), removes the inline GetBaseURL. |
| internal/config/baseurl_public.go | Public build: GetBaseURL always returns DefaultBaseURL. |
| internal/config/baseurl_internal.go | Internal build: GetBaseURL honors viper base_url. |
| internal/config/bindenv_public.go | Public build: bindInternalEnv no-op. |
| internal/config/bindenv_internal.go | Internal build: binds RUNWARE_BASE_URL to base_url in viper. |
| internal/config/baseurl_public_test.go | Verifies locked default ignores config/env in public build. |
| internal/config/baseurl_internal_test.go | Verifies default / config / env-over-config precedence in internal build. |
| cmd/config_cmd.go | Adds Base URL to config show, delegates completion to settableConfigKeys, rejects non-settable keys. |
| cmd/config_keys_public.go | Public build: omits base_url from completion; rejects it in isSettableKey. |
| cmd/config_keys_internal.go | Internal build: includes base_url; permits all keys. |
| Makefile | Adds build-internal, test-internal, extends clean. |
| .github/workflows/ci.yml | Adds Build/Test steps with -tags internal. |
| docs/INTERNAL.md | New doc describing the internal build, override behavior, and tagged-lint guidance. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
base_url in public config output and lint internal-tagged code in CI
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 33-41: The workflow uses mutable action tags (e.g.,
golangci/golangci-lint-action@v9) in the CI steps; replace each mutable tag
occurrence (both instances of golangci/golangci-lint-action@v9 in the provided
diff) with the corresponding immutable commit SHA for that action release,
updating both Lint steps to the same pinned SHA; also audit and pin other
mutable action refs in this repo (actions/checkout, actions/setup-go,
goreleaser/goreleaser-action, amannn/action-semantic-pull-request) to their
respective commit SHAs to ensure reproducible CI.
In `@cmd/config_keys_internal.go`:
- Around line 24-25: The current isSettableKey function unconditionally returns
true, disabling unknown-key validation; change it to validate against the
canonical set of allowed config keys instead of always returning true—e.g., have
isSettableKey look up the provided key in the existing map/slice of valid keys
(or call the shared validation helper used elsewhere) and return true only if
the key exists; update references to isSettableKey (the caller that prints "Set
…") so only recognized keys produce success output.
In `@cmd/config_keys_public.go`:
- Around line 23-24: isSettableKey currently allows any key except "base_url",
which is too permissive for the `config set` path; change isSettableKey to
validate against an explicit whitelist of allowed public config keys (e.g.,
define a slice or set like allowedPublicKeys or knownPublicKeys and check
membership) and return true only if the key is present in that set (and still
exclude "base_url" if that's required); update related callers (the `config set`
command) and add/update tests to cover rejected unknown keys and accepted
whitelisted keys.
In `@internal/config/baseurl_public_test.go`:
- Around line 35-37: The subtest mutates the package-global configDir and
doesn't restore it; wrap the change by saving the original (e.g., oldConfigDir
:= configDir), set configDir = tmpDir, and defer restoring it (defer func(){
configDir = oldConfigDir }()) immediately after creating tmpDir so global state
is returned after the test; update the subtest around tmpDir/configDir usage
(references: configDir, tmpDir, path) to ensure no state leakage to other tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f186ec75-731c-47f5-88c4-2e018e5b3f9d
📒 Files selected for processing (15)
.github/workflows/ci.ymlMakefilecmd/config_cmd.gocmd/config_keys_internal.gocmd/config_keys_public.gocmd/config_show_internal.gocmd/config_show_public.godocs/INTERNAL.mdinternal/config/baseurl_internal.gointernal/config/baseurl_internal_test.gointernal/config/baseurl_public.gointernal/config/baseurl_public_test.gointernal/config/bindenv_internal.gointernal/config/bindenv_public.gointernal/config/config.go
| func isSettableKey(string) bool { | ||
| return true |
There was a problem hiding this comment.
isSettableKey currently disables unknown-key validation in internal builds.
Returning true for all keys means typos/arbitrary keys pass validation and get “Set …” success output.
Suggested fix
-func isSettableKey(string) bool {
- return true
+func isSettableKey(key string) bool {
+ for _, allowed := range settableConfigKeys() {
+ if key == allowed {
+ return true
+ }
+ }
+ return false
}📝 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.
| func isSettableKey(string) bool { | |
| return true | |
| func isSettableKey(key string) bool { | |
| for _, allowed := range settableConfigKeys() { | |
| if key == allowed { | |
| return true | |
| } | |
| } | |
| return false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/config_keys_internal.go` around lines 24 - 25, The current isSettableKey
function unconditionally returns true, disabling unknown-key validation; change
it to validate against the canonical set of allowed config keys instead of
always returning true—e.g., have isSettableKey look up the provided key in the
existing map/slice of valid keys (or call the shared validation helper used
elsewhere) and return true only if the key exists; update references to
isSettableKey (the caller that prints "Set …") so only recognized keys produce
success output.
| func isSettableKey(key string) bool { | ||
| return key != "base_url" |
There was a problem hiding this comment.
Public key validation is too permissive (key != "base_url").
This accepts arbitrary unknown keys and undermines the new validation path in config set.
Suggested fix
func isSettableKey(key string) bool {
- return key != "base_url"
+ for _, allowed := range settableConfigKeys() {
+ if key == allowed {
+ return true
+ }
+ }
+ return false
}📝 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.
| func isSettableKey(key string) bool { | |
| return key != "base_url" | |
| func isSettableKey(key string) bool { | |
| for _, allowed := range settableConfigKeys() { | |
| if key == allowed { | |
| return true | |
| } | |
| } | |
| return false | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/config_keys_public.go` around lines 23 - 24, isSettableKey currently
allows any key except "base_url", which is too permissive for the `config set`
path; change isSettableKey to validate against an explicit whitelist of allowed
public config keys (e.g., define a slice or set like allowedPublicKeys or
knownPublicKeys and check membership) and return true only if the key is present
in that set (and still exclude "base_url" if that's required); update related
callers (the `config set` command) and add/update tests to cover rejected
unknown keys and accepted whitelisted keys.
| tmpDir := t.TempDir() | ||
| configDir = tmpDir | ||
| path := filepath.Join(tmpDir, "config.yaml") |
There was a problem hiding this comment.
Restore configDir after mutating global test state.
This subtest overwrites a package-global and does not reset it, which can leak state into later tests.
Suggested fix
tmpDir := t.TempDir()
- configDir = tmpDir
+ oldConfigDir := configDir
+ configDir = tmpDir
+ t.Cleanup(func() { configDir = oldConfigDir })
path := filepath.Join(tmpDir, "config.yaml")📝 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.
| tmpDir := t.TempDir() | |
| configDir = tmpDir | |
| path := filepath.Join(tmpDir, "config.yaml") | |
| tmpDir := t.TempDir() | |
| oldConfigDir := configDir | |
| configDir = tmpDir | |
| t.Cleanup(func() { configDir = oldConfigDir }) | |
| path := filepath.Join(tmpDir, "config.yaml") |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@internal/config/baseurl_public_test.go` around lines 35 - 37, The subtest
mutates the package-global configDir and doesn't restore it; wrap the change by
saving the original (e.g., oldConfigDir := configDir), set configDir = tmpDir,
and defer restoring it (defer func(){ configDir = oldConfigDir }()) immediately
after creating tmpDir so global state is returned after the test; update the
subtest around tmpDir/configDir usage (references: configDir, tmpDir, path) to
ensure no state leakage to other tests.
Support configurable API base URL
Summary
Adds a configurable API base URL so the CLI can point at endpoints other than the hard-coded default (
https://api.runware.ai/v1). The value can be set three ways, in increasing precedence:DefaultBaseURL)base_url(~/.runware/config.yaml)RUNWARE_BASE_URLGetBaseURL()now resolves env/config before falling back to the default.Changes
internal/config/config.go— addConfig.BaseURL(yamlbase_url), bindRUNWARE_BASE_URLin viper, and resolve the value inGetBaseURL().cmd/config_cmd.go— showBase URLinconfig showand addbase_urlto theconfig settab-completion list.README.md— document the env var and config key.internal/config/config_test.go—TestGetBaseURLcovers default, config, and env-over-config precedence.Why a configurable base URL is useful
A pinned endpoint forces every user onto one host. Making it configurable unlocks several common workflows:
https://api.staging.internal/v1) to validate changes before they hit production.This mirrors how the major AI SDKs and CLIs treat the endpoint as a first-class, overridable setting rather than a constant:
OPENAI_BASE_URLenv var /base_urlclient option, widely used to target Azure OpenAI, gateways, and local proxies.ANTHROPIC_BASE_URL, plus provider-specific bases for Amazon Bedrock and Google Vertex.OLLAMA_HOST), the AWS CLI (--endpoint-url), and most cloud SDKs all expose an endpoint override for the same proxy/region/testing reasons.Following the
RUNWARE_*env-var convention keeps the override consistent with the existingRUNWARE_API_KEYandRUNWARE_ENVsettings.