Skip to content

feat: support loading config from remote URL#87

Open
neilkuan wants to merge 3 commits intoopenabdev:mainfrom
neilkuan:feat/remote-config-url
Open

feat: support loading config from remote URL#87
neilkuan wants to merge 3 commits intoopenabdev:mainfrom
neilkuan:feat/remote-config-url

Conversation

@neilkuan
Copy link
Copy Markdown
Contributor

@neilkuan neilkuan commented Apr 6, 2026

Closes #80

What

Auto-detect URL in the existing positional config argument. If it starts with http:// or https://, fetch the TOML content via HTTP GET; otherwise treat it as a local file path (existing behavior unchanged).

# local (unchanged)
agent-broker config.toml

# remote (new — auto-detected)
agent-broker https://example.com/config.toml
Implementation
  • Added reqwest dependency with rustls-tls backend (consistent with serenity's rustls)
  • Extracted shared parse_config(raw, source) from load_config to avoid duplication
  • Added load_config_from_url(url) async function with 10s timeout and clear error messages for network failure / non-2xx / invalid TOML
  • main.rs checks if the first positional arg starts with http:// or https:// to decide the code path
  • Env-var expansion (${VAR}) works identically on both local and remote config content
Test plan
  • cargo build passes
  • Existing local config path behavior unchanged
  • agent-broker https://raw.githubusercontent.com/.../config.toml fetches and parses correctly
  • Invalid URL returns clear error message
  • Non-200 HTTP response returns clear error message
  • Env-var expansion works on remote config content

neilkuan added 2 commits April 7, 2026 00:04
Auto-detect URL in the positional config argument — if it starts with
http:// or https://, fetch the TOML via HTTP GET (10s timeout) instead
of reading a local file. Env-var expansion and parsing pipeline remain
the same for both paths.

Closes openabdev#80
Cover parse_config, expand_env_vars, load_config (file & missing file),
load_config_from_url (unreachable host), and env-var substitution in
TOML content.
thepagent
thepagent previously approved these changes Apr 6, 2026
Copy link
Copy Markdown
Collaborator

@thepagent thepagent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🙌 Clean and minimal implementation.

What I like:

  • Auto-detect URL approach (no new flag needed) — exactly the kubectl-like UX we discussed in #80
  • parse_config(raw, source) extraction is a nice refactor, avoids duplication between local and remote paths
  • reqwest with rustls-tls — consistent with serenity's TLS backend
  • 10s timeout + separate error messages for network failure / non-2xx / invalid TOML ✅
  • Env-var expansion works on remote content — verified in tests
  • main.rs change is only 8 lines — surgical

One minor nit (non-blocking):

  • std::env::set_var / remove_var in tests can race in multi-threaded test runs. Not a real issue at this scale, but if you ever want to harden it, temp_env crate or serial_test can help. Totally fine to leave as-is for now.

Ship it! 🚀

@thepagent thepagent dismissed their stale review April 6, 2026 16:11

needs further review

@thepagent thepagent added the p1 High — address this sprint label Apr 6, 2026
@masami-agent
Copy link
Copy Markdown
Contributor

Hi @neilkuan, this PR overlaps with #160 which also implements remote config loading. Would you be okay closing one of these so we don't have two PRs for the same feature? Happy to discuss which one to keep. Thanks 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: support --remote-config for loading config from URL

4 participants