From 9527b550ec7aa97dd94ce0af08f4053bf345c70a Mon Sep 17 00:00:00 2001 From: Dmitry Teryaev Date: Mon, 13 Jul 2026 15:44:25 +0300 Subject: [PATCH] feat: auto-load .env alongside agctl.yaml (--env-file / AGCTL_ENV_FILE) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit agctl's \${VAR} interpolation read only os.environ, so using a .env file required manually sourcing it first. This adds first-class .env support: - Auto-load .env next to the resolved agctl.yaml as env defaults, with real environment winning (.env fills blanks only — docker-compose/dotenv convention; CI/prod inject real env over committed defaults). - Explicit sources: --env-file (global flag + per-command wherever --config is declared) and AGCTL_ENV_FILE. Precedence: --env-file > AGCTL_ENV_FILE > sibling .env. Missing sibling = no-op; missing explicit source = ConfigError (exit 2). - python-dotenv parses with its own \${VAR} expansion disabled, so agctl's single interpolate() owns all \${...} resolution (chained/nested). AGCTL_* lines in .env apply as overrides (real env still wins). New module agctl/config/env_file.py; merge step in compose_config after config discovery, before interpolation. Adds python-dotenv>=1.0 dep. Reviewed via a 4-reviewer parallel team; all Critical/Important findings fixed and guarded by regression tests (incl. mock-start daemon argv forwarding and malformed-UTF8 error wrapping). Tests: 24 in tests/unit/test_env_file.py + a mock-start daemon-argv regression test; full suite green (1101 passed, 18 skipped). Co-Authored-By: Claude --- README.md | 8 +- agctl/cli.py | 9 +- agctl/command.py | 6 +- agctl/commands/check_commands.py | 8 +- agctl/commands/config_commands.py | 12 +- agctl/commands/db_commands.py | 28 +- agctl/commands/discover_commands.py | 27 +- agctl/commands/grpc_commands.py | 8 +- agctl/commands/http_commands.py | 15 +- agctl/commands/kafka_commands.py | 16 +- agctl/commands/logs_commands.py | 19 +- agctl/commands/mock_commands.py | 19 +- agctl/config/env_file.py | 92 +++++ agctl/config/loader.py | 30 +- docs/ARCHITECTURE.md | 25 +- docs/DESIGN.md | 14 +- pyproject.toml | 1 + skills/agctl-config/SKILL.md | 5 +- skills/agctl-config/reference/init-config.md | 7 +- skills/agctl/SKILL.md | 6 +- tests/unit/test_env_file.py | 349 +++++++++++++++++++ tests/unit/test_mock_commands.py | 63 ++++ tests/unit/test_overlay.py | 12 +- 23 files changed, 708 insertions(+), 71 deletions(-) create mode 100644 agctl/config/env_file.py create mode 100644 tests/unit/test_env_file.py diff --git a/README.md b/README.md index 5f244db..abe3f8f 100644 --- a/README.md +++ b/README.md @@ -203,7 +203,7 @@ AGCTL_SERVICES__ORDER_SERVICE__BASE_URL=http://order-svc:8080 `agctl config init` writes exactly this file — shown here for reference and for browsing on GitHub without installing. It has **concrete localhost values and no required env vars**, so `agctl config validate` passes as-is. (The production -version is the same file with secrets/hosts moved into `${...}` and sourced from a +version is the same file with secrets/hosts moved into `${...}` and loaded from a `.env` — see the note after it.) ```yaml @@ -400,10 +400,12 @@ logs: > **Note:** `charge-payment` uses the `${PAYMENT_SERVICE_TOKEN:-change-me}` form — > an *optional* env var with a literal default — so `config validate` passes even > with nothing exported. For production, `export PAYMENT_SERVICE_TOKEN=` -> (or move the whole value into `${...}` sourced from a `.env`); see below. +> (or move the whole value into `${...}` loaded from a `.env`); see below. **Moving to environment-driven config** — replace the concrete values above with -`${...}` and source them from a `.env` (agctl resolves them at load time): +`${...}` and put them in a `.env`. agctl auto-loads the `.env` next to the resolved +`agctl.yaml` at config load time (real env wins, so CI/prod can override committed +defaults); point at a different location with `--env-file ` or `AGCTL_ENV_FILE`: ```bash # .env — never commit real secrets diff --git a/agctl/cli.py b/agctl/cli.py index 4c5761e..c2e5379 100644 --- a/agctl/cli.py +++ b/agctl/cli.py @@ -112,13 +112,20 @@ def _ensure_utf8_streams() -> None: @click.version_option(version=__version__, message="agctl %(version)s") @click.option("--config", "config_path", default=None, help="Path to agctl.yaml") @click.option("--overlay", "overlay_paths", multiple=True, help="Overlay config fragment (repeatable; later wins)") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context -def cli(ctx: click.Context, config_path: str | None, overlay_paths: tuple[str, ...]) -> None: +def cli( + ctx: click.Context, + config_path: str | None, + overlay_paths: tuple[str, ...], + env_file: str | None, +) -> None: """agctl — agent-facing CLI harness for testing distributed systems.""" _ensure_utf8_streams() ctx.ensure_object(dict) ctx.obj["config_path"] = config_path ctx.obj["overlay_paths"] = tuple(overlay_paths) or None + ctx.obj["env_file"] = env_file @cli.group(name="config") diff --git a/agctl/command.py b/agctl/command.py index bf53895..8b496c4 100644 --- a/agctl/command.py +++ b/agctl/command.py @@ -68,9 +68,11 @@ def wrapper(*args: Any, **kwargs: Any) -> Any: def load_config_or_raise( - config_path: str | None = None, overlay_paths: list[str] | None = None + config_path: str | None = None, + overlay_paths: list[str] | None = None, + env_file: str | None = None, ): """Load config, letting ConfigError propagate to the envelope wrapper (exit 2).""" from .config import load_config - return load_config(config_path, overlays=overlay_paths) + return load_config(config_path, overlays=overlay_paths, env_file=env_file) diff --git a/agctl/commands/check_commands.py b/agctl/commands/check_commands.py index 050de6f..4d1f28c 100644 --- a/agctl/commands/check_commands.py +++ b/agctl/commands/check_commands.py @@ -76,8 +76,9 @@ def _check_ready_core( all_services: bool, timeout: float | None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) return _check_ready_with_config(cfg, service, all_services, timeout) @@ -100,9 +101,11 @@ def _check_ready_with_config( @click.option("--all", "all_services", is_flag=True, default=False, help="Check all services") @click.option("--timeout", "timeout", type=float, default=None) @click.option("--config", "config_path", default=None) +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def check_ready( ctx: click.Context, + env_file: str | None, service: str | None, all_services: bool, timeout: float | None, @@ -111,7 +114,8 @@ def check_ready( """Probe configured service health endpoints.""" path = config_path or (ctx.obj.get("config_path") if ctx.obj else None) ovs = ctx.obj.get("overlay_paths") if ctx.obj else None - _check_ready_envelope(path, service, all_services, timeout, overlay_paths=list(ovs) if ovs else None) + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) + _check_ready_envelope(path, service, all_services, timeout, overlay_paths=list(ovs) if ovs else None, env_file=env_file) _check_ready_envelope = envelope("check.ready")(_check_ready_core) diff --git a/agctl/commands/config_commands.py b/agctl/commands/config_commands.py index 8fe536c..5c7be12 100644 --- a/agctl/commands/config_commands.py +++ b/agctl/commands/config_commands.py @@ -129,15 +129,17 @@ def _emit_config_error(command: str, err: ConfigError, start: float) -> None: @click.command("validate") @click.option("--config", "config_path", default=None) @click.option("--overlay", "overlay_paths", multiple=True, default=None) +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context -def config_validate(ctx: click.Context, config_path: str | None, overlay_paths: tuple[str, ...] | None) -> None: +def config_validate(ctx: click.Context, config_path: str | None, overlay_paths: tuple[str, ...] | None, env_file: str | None) -> None: """Parse and validate agctl.yaml. Exit 2 on any error.""" start = time.monotonic() path = config_path or ctx.obj.get("config_path") # Resolve overlay paths: own option takes precedence over ctx.obj ovs = tuple(overlay_paths) or ctx.obj.get("overlay_paths") + resolved_env_file = env_file or ctx.obj.get("env_file") try: - composed = compose_config(path, list(ovs) if ovs else None) + composed = compose_config(path, list(ovs) if ovs else None, env_file=resolved_env_file) except ConfigError as err: _emit_config_error("config.validate", err, start) raise SystemExit(2) @@ -186,16 +188,18 @@ def config_validate(ctx: click.Context, config_path: str | None, overlay_paths: @click.command("show") @click.option("--config", "config_path", default=None) @click.option("--overlay", "overlay_paths", multiple=True, default=None) +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.option("--unmask", is_flag=True, default=False) @click.pass_context -def config_show(ctx: click.Context, config_path: str | None, overlay_paths: tuple[str, ...] | None, unmask: bool) -> None: +def config_show(ctx: click.Context, config_path: str | None, overlay_paths: tuple[str, ...] | None, env_file: str | None, unmask: bool) -> None: """Dump the resolved config as JSON, secrets masked.""" start = time.monotonic() path = config_path or ctx.obj.get("config_path") # Resolve overlay paths: own option takes precedence over ctx.obj ovs = tuple(overlay_paths) or ctx.obj.get("overlay_paths") + resolved_env_file = env_file or ctx.obj.get("env_file") try: - composed = compose_config(path, list(ovs) if ovs else None) + composed = compose_config(path, list(ovs) if ovs else None, env_file=resolved_env_file) except ConfigError as err: _emit_config_error("config.show", err, start) raise SystemExit(2) diff --git a/agctl/commands/db_commands.py b/agctl/commands/db_commands.py index f1f3095..b401f6f 100644 --- a/agctl/commands/db_commands.py +++ b/agctl/commands/db_commands.py @@ -168,8 +168,9 @@ def _db_query_core( param: tuple[str, ...], connection: str | None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) sql_text, params, conn_name = resolve_db_request( cfg, template=template, @@ -198,7 +199,8 @@ def db_query( """Run a DB query and return the rows.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None - _db_query_envelope(config_path, template, sql, param, connection, overlay_paths=list(ovs) if ovs else None) + env_file = ctx.obj.get("env_file") if ctx.obj else None + _db_query_envelope(config_path, template, sql, param, connection, overlay_paths=list(ovs) if ovs else None, env_file=env_file) _db_query_envelope = envelope("db.query")(_db_query_core) @@ -246,8 +248,9 @@ def _db_assert_core( equals: str | None, assertion: str | None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) sql_text, params, conn_name = resolve_db_request( cfg, template=template, @@ -372,6 +375,7 @@ def db_assert( """Run a DB query and assert on its result.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _db_assert_envelope( config_path, template, @@ -384,6 +388,7 @@ def db_assert( equals, assertion, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) @@ -403,9 +408,10 @@ def _db_execute_core( connection: str | None, write: bool, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: # Step 1: Load config - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # Step 2: Resolve SQL, params, and connection (enforces template XOR sql, # neither-given, unknown template, unknown connection) @@ -471,7 +477,8 @@ def db_execute( """Execute a write SQL statement and return affected row count.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None - _db_execute_envelope(config_path, template, sql, param, connection, write, overlay_paths=list(ovs) if ovs else None) + env_file = ctx.obj.get("env_file") if ctx.obj else None + _db_execute_envelope(config_path, template, sql, param, connection, write, overlay_paths=list(ovs) if ovs else None, env_file=env_file) _db_execute_envelope = envelope("db.execute")(_db_execute_core) @@ -531,9 +538,10 @@ def _db_schema_tables_core( connection: str | None, schema: str | None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: """Level 1: list relations (tables/views) visible in the connection.""" - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # Schema has no template, so resolve inline with no template_connection. conn_name = resolve_connection_name(cfg, connection_name=connection) raw = _probe_and_describe( @@ -555,9 +563,10 @@ def _db_schema_table_core( schema: str | None, table: str, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: """Level 2: return one relation's columns + keys, disambiguating matches.""" - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) conn_name = resolve_connection_name(cfg, connection_name=connection) raw = _probe_and_describe( cfg.database.connections[conn_name], table=table, schema=schema @@ -619,11 +628,12 @@ def db_schema( """Discover live DB schema: list relations, then columns/keys for one.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None overlay_paths = list(ovs) if ovs else None if table is None: - _db_schema_tables_envelope(config_path, connection, schema, overlay_paths=overlay_paths) + _db_schema_tables_envelope(config_path, connection, schema, overlay_paths=overlay_paths, env_file=env_file) else: - _db_schema_table_envelope(config_path, connection, schema, table, overlay_paths=overlay_paths) + _db_schema_table_envelope(config_path, connection, schema, table, overlay_paths=overlay_paths, env_file=env_file) _db_schema_tables_envelope = envelope("db.schema.tables")(_db_schema_tables_core) diff --git a/agctl/commands/discover_commands.py b/agctl/commands/discover_commands.py index 7298c00..4c2e19c 100644 --- a/agctl/commands/discover_commands.py +++ b/agctl/commands/discover_commands.py @@ -194,8 +194,8 @@ def _mock_kafka_example(reactor) -> str: # --------------------------------------------------------------------------- # -def _summary_core(config_path: str | None, overlay_paths: list[str] | None = None) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths) +def _summary_core(config_path: str | None, overlay_paths: list[str] | None = None, env_file: str | None = None) -> dict: + cfg = load_config_or_raise(config_path, overlay_paths, env_file=env_file) return { "services": len(cfg.services), "http_templates": len(cfg.templates), @@ -213,8 +213,8 @@ def _summary_core(config_path: str | None, overlay_paths: list[str] | None = Non _summary_envelope = envelope("discover.summary")(_summary_core) -def _category_core(config_path: str | None, category: str, overlay_paths: list[str] | None = None) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths) +def _category_core(config_path: str | None, category: str, overlay_paths: list[str] | None = None, env_file: str | None = None) -> dict: + cfg = load_config_or_raise(config_path, overlay_paths, env_file=env_file) if category not in _VALID_CATEGORIES: raise ConfigError(f"Unknown category: {category}", {"category": category}) @@ -287,8 +287,8 @@ def _category_core(config_path: str | None, category: str, overlay_paths: list[s _category_envelope = envelope("discover.category")(_category_core) -def _item_core(config_path: str | None, category: str, name: str, overlay_paths: list[str] | None = None) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths) +def _item_core(config_path: str | None, category: str, name: str, overlay_paths: list[str] | None = None, env_file: str | None = None) -> dict: + cfg = load_config_or_raise(config_path, overlay_paths, env_file=env_file) if category not in _VALID_CATEGORIES: raise ConfigError(f"Unknown category: {category}", {"category": category}) @@ -530,8 +530,8 @@ def _item_core(config_path: str | None, category: str, name: str, overlay_paths: _item_envelope = envelope("discover.item")(_item_core) -def _search_core(config_path: str | None, term: str, overlay_paths: list[str] | None = None) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths) +def _search_core(config_path: str | None, term: str, overlay_paths: list[str] | None = None, env_file: str | None = None) -> dict: + cfg = load_config_or_raise(config_path, overlay_paths, env_file=env_file) needle = term.lower() def _matches(haystack: str | None) -> bool: @@ -673,9 +673,11 @@ def _emit_argument_error(message: str) -> None: @click.option("--search", "search", default=None, help="Substring to search for") @click.option("--config", "config_path", default=None, help="Path to agctl.yaml") @click.option("--overlay", "overlay_paths", multiple=True, default=None, help="Overlay config paths") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def discover( ctx: click.Context, + env_file: str | None, category: str | None, name: str | None, search: str | None, @@ -686,6 +688,7 @@ def discover( resolved_config = config_path or (ctx.obj.get("config_path") if ctx.obj else None) ovs = tuple(overlay_paths) or (ctx.obj.get("overlay_paths") if ctx.obj else None) resolved_overlay = list(ovs) if ovs else None + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) # Mutual exclusion: --category + --search together is an error. if category is not None and search is not None: @@ -697,16 +700,16 @@ def discover( return if search is not None: - _search_envelope(resolved_config, search, resolved_overlay) + _search_envelope(resolved_config, search, resolved_overlay, env_file=env_file) return if category is not None and name is None: - _category_envelope(resolved_config, category, resolved_overlay) + _category_envelope(resolved_config, category, resolved_overlay, env_file=env_file) return if category is not None and name is not None: - _item_envelope(resolved_config, category, name, resolved_overlay) + _item_envelope(resolved_config, category, name, resolved_overlay, env_file=env_file) return # No flags — summary. - _summary_envelope(resolved_config, resolved_overlay) + _summary_envelope(resolved_config, resolved_overlay, env_file=env_file) diff --git a/agctl/commands/grpc_commands.py b/agctl/commands/grpc_commands.py index 5bc9aee..6dd2052 100644 --- a/agctl/commands/grpc_commands.py +++ b/agctl/commands/grpc_commands.py @@ -367,6 +367,7 @@ def _grpc_healthcheck_core( service: str | None, all_: bool, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: """Core gRPC healthcheck logic. @@ -383,7 +384,7 @@ def _grpc_healthcheck_core( Raises: ConfigError: If target is unknown. """ - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # Select targets: if all_ OR neither target given → all targets; else [target] if all_ or target is None: @@ -449,6 +450,7 @@ def grpc_healthcheck( """ config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _grpc_healthcheck_envelope( config_path, @@ -456,6 +458,7 @@ def grpc_healthcheck( service, all_, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) @@ -601,6 +604,7 @@ def grpc_call( """ config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None start = time.monotonic() # Step 1: Resolve EXACTLY ONCE — config load, target/template resolution, @@ -608,7 +612,7 @@ def grpc_call( # resulting context is handed to both the single-result path # (``_grpc_call_core``) and the streaming path; neither re-resolves. try: - cfg = load_config_or_raise(config_path, overlay_paths=list(ovs) if ovs else None) + cfg = load_config_or_raise(config_path, overlay_paths=list(ovs) if ovs else None, env_file=env_file) resolved = _resolve_grpc_call( cfg, target_name=target, diff --git a/agctl/commands/http_commands.py b/agctl/commands/http_commands.py index 19ac23f..23cc78b 100644 --- a/agctl/commands/http_commands.py +++ b/agctl/commands/http_commands.py @@ -117,8 +117,9 @@ def _http_call_core( jq_path: str | None = None, equals: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) if template_name not in cfg.templates: raise TemplateNotFound( @@ -233,6 +234,7 @@ def http_call( """Resolve and send a named HTTP template.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _http_call_envelope( config_path, template_name, @@ -246,6 +248,7 @@ def http_call( jq_path, equals, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) @@ -269,8 +272,9 @@ def _http_request_core( jq_path: str | None = None, equals: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) if url is not None: # URL mode: a full request URL, no configured service required. This is @@ -395,6 +399,7 @@ def http_request( """Send a free-form HTTP request against a configured service or a full URL.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _http_request_envelope( config_path, service, @@ -410,6 +415,7 @@ def http_request( jq_path, equals, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) @@ -498,12 +504,13 @@ def _resolve_ping_request( timeout: float | None, url: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ): """Resolve the request components for a ping (template, URL, or free-form). Returns ``(client, method, path, headers, body_dict_or_None)``. """ - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # --url is mutually exclusive with the template positional and --service/--path. if url is not None and ( @@ -638,6 +645,7 @@ def http_ping( """Repeatedly send an HTTP request, streaming NDJSON ping lines.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None start = time.monotonic() if duration is not None and until_stopped: @@ -667,6 +675,7 @@ def http_ping( timeout, url=url, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) except AgctlError as err: # Startup config/template errors (ConfigError, TemplateNotFound) -> structured diff --git a/agctl/commands/kafka_commands.py b/agctl/commands/kafka_commands.py index 4ac7b9a..2f6a07c 100644 --- a/agctl/commands/kafka_commands.py +++ b/agctl/commands/kafka_commands.py @@ -161,8 +161,9 @@ def _kafka_produce_core( header: tuple[str, ...], cluster: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) value = json.loads(message) headers = parse_params(header) if header else None @@ -191,7 +192,8 @@ def kafka_produce( """Produce one message to a Kafka topic.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None - _kafka_produce_envelope(config_path, topic, message, key, header, cluster, overlay_paths=list(ovs) if ovs else None) + env_file = ctx.obj.get("env_file") if ctx.obj else None + _kafka_produce_envelope(config_path, topic, message, key, header, cluster, overlay_paths=list(ovs) if ovs else None, env_file=env_file) _kafka_produce_envelope = envelope("kafka.produce")(_kafka_produce_core) @@ -214,8 +216,9 @@ def _kafka_consume_core( consumer_group: str | None, cluster: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # --filter-key is a deprecated alias of --match; both given -> error. if match is not None and filter_key is not None: @@ -327,6 +330,7 @@ def kafka_consume( """Consume messages from a Kafka topic window.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _kafka_consume_envelope( config_path, topic, @@ -339,6 +343,7 @@ def kafka_consume( consumer_group, cluster, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) @@ -430,8 +435,9 @@ def _kafka_assert_core( assertion: str | None, cluster: str | None = None, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) params = parse_params(param) # DESIGN §9.3: a custom assertion mode is mutually exclusive with the @@ -629,6 +635,7 @@ def kafka_assert( """Assert a matching message exists in a Kafka window.""" config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = ctx.obj.get("env_file") if ctx.obj else None _kafka_assert_envelope( config_path, topic, @@ -644,6 +651,7 @@ def kafka_assert( assertion, cluster, overlay_paths=list(ovs) if ovs else None, + env_file=env_file, ) diff --git a/agctl/commands/logs_commands.py b/agctl/commands/logs_commands.py index 2e4ab03..1ed3bde 100644 --- a/agctl/commands/logs_commands.py +++ b/agctl/commands/logs_commands.py @@ -160,8 +160,9 @@ def _logs_query_core( since: str | None, until: str | None, limit: int | None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path) + cfg = load_config_or_raise(config_path, env_file=env_file) src = _resolve_source(cfg, source) params = parse_params(param) filt = _build_log_filter( @@ -209,9 +210,11 @@ def _logs_query_core( @click.option("--until", "until", default=None, help="End time (ISO-8601 or duration)") @click.option("--limit", "limit", type=int, default=None, help="Max entries to return") @click.option("--config", "config_path", default=None, help="Path to agctl.yaml") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def logs_query( ctx: click.Context, + env_file: str | None, source: str, level: str | None, logger: str | None, @@ -228,6 +231,7 @@ def logs_query( config_path_resolved = config_path if ctx.obj and ctx.obj.get("config_path"): config_path_resolved = ctx.obj.get("config_path") + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) _logs_query_envelope( config_path_resolved, source, @@ -239,6 +243,7 @@ def logs_query( since, until, limit, + env_file=env_file, ) @@ -261,8 +266,9 @@ def _logs_assert_core( since: str | None, not_: bool, timeout: float | None, + env_file: str | None = None, ) -> dict: - cfg = load_config_or_raise(config_path) + cfg = load_config_or_raise(config_path, env_file=env_file) src = _resolve_source(cfg, source) params = parse_params(param) filt = _build_log_filter( @@ -343,9 +349,11 @@ def _logs_assert_core( @click.option("--not", "not_", is_flag=True, default=False, help="Invert: fail if a match IS found") @click.option("--timeout", "timeout", type=float, default=None, help="Poll timeout (seconds); omit for one-shot") @click.option("--config", "config_path", default=None, help="Path to agctl.yaml") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def logs_assert( ctx: click.Context, + env_file: str | None, source: str, level: str | None, logger: str | None, @@ -362,6 +370,7 @@ def logs_assert( config_path_resolved = config_path if ctx.obj and ctx.obj.get("config_path"): config_path_resolved = ctx.obj.get("config_path") + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) _logs_assert_envelope( config_path_resolved, source, @@ -373,6 +382,7 @@ def logs_assert( since, not_, timeout, + env_file=env_file, ) @@ -433,9 +443,11 @@ def _tail_run( @click.option("--duration", "duration", type=float, default=None, help="Stop after N seconds") @click.option("--until-stopped", "until_stopped", is_flag=True, default=False) @click.option("--config", "config_path", default=None, help="Path to agctl.yaml") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def logs_tail( ctx: click.Context, + env_file: str | None, source: str, level: str | None, logger: str | None, @@ -450,6 +462,7 @@ def logs_tail( config_path_resolved = config_path if ctx.obj and ctx.obj.get("config_path"): config_path_resolved = ctx.obj.get("config_path") + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) start = time.monotonic() @@ -468,7 +481,7 @@ def logs_tail( raise SystemExit(2) try: - cfg = load_config_or_raise(config_path_resolved) + cfg = load_config_or_raise(config_path_resolved, env_file=env_file) src = _resolve_source(cfg, source) params = parse_params(param) filt = _build_log_filter( diff --git a/agctl/commands/mock_commands.py b/agctl/commands/mock_commands.py index 47d1a9f..f72ad0d 100644 --- a/agctl/commands/mock_commands.py +++ b/agctl/commands/mock_commands.py @@ -244,6 +244,7 @@ def _mock_start_core( duration: float | None, state_dir: str, overlay_paths: list[str] | None = None, + env_file: str | None = None, ) -> dict: """Core logic for `mock start` (Task 4). @@ -257,7 +258,7 @@ def _mock_start_core( """ _require_posix_daemon() # Step 1: Load config - cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths) + cfg = load_config_or_raise(config_path, overlay_paths=overlay_paths, env_file=env_file) # Step 2: Resolve which engines to run run_http, run_kafka = _resolve_engines(only, cfg.mocks) @@ -321,6 +322,12 @@ def _mock_start_core( if overlay_paths is not None: for ov in overlay_paths: daemon_argv.extend(["--overlay", str(Path(ov).absolute())]) + # Forward --env-file: the daemon re-loads config from scratch, so without + # this it would silently fall back to the default .env sibling and ignore + # the user's flag (the parent's readiness load uses the right file, the + # server that actually serves traffic would not). + if env_file is not None: + daemon_argv.extend(["--env-file", str(Path(env_file).absolute())]) # Subcommand and mock-run-specific options daemon_argv.extend(["mock", "run"]) if run_http: @@ -420,9 +427,11 @@ def _mock_start_core( @click.option("--fail-fast", "fail_fast", is_flag=True, default=False, help="Exit on first reactor error") @click.option("--duration", "duration", type=float, default=None, help="Stop after N seconds") @click.option("--state-dir", "state_dir", default="./.agctl", help="Directory for mock state (pidfiles, logs)") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def mock_start( ctx: click.Context, + env_file: str | None, config_path: str | None, http_listen: str | None, only: str | None, @@ -435,8 +444,9 @@ def mock_start( if config_path is None: config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) - _mock_start_envelope(config_path, http_listen, only, fail_fast, duration, state_dir, overlay_paths=list(ovs) if ovs else None) + _mock_start_envelope(config_path, http_listen, only, fail_fast, duration, state_dir, overlay_paths=list(ovs) if ovs else None, env_file=env_file) _mock_start_envelope = envelope("mock.start")(_mock_start_core) @@ -455,9 +465,11 @@ def mock_start( @click.option("--fail-fast", "fail_fast", is_flag=True, default=False, help="Exit on first reactor error") @click.option("--duration", "duration", type=float, default=None, help="Stop after N seconds") @click.option("--until-stopped", "until_stopped", is_flag=True, default=False, help="Run until stopped") +@click.option("--env-file", "env_file", default=None, help="Path to .env file (default: .env next to agctl.yaml)") @click.pass_context def mock_run( ctx: click.Context, + env_file: str | None, config_path: str | None, http_listen: str | None, only: str | None, @@ -470,6 +482,7 @@ def mock_run( if config_path is None: config_path = ctx.obj.get("config_path") if ctx.obj else None ovs = ctx.obj.get("overlay_paths") if ctx.obj else None + env_file = env_file or (ctx.obj.get("env_file") if ctx.obj else None) start = time.monotonic() @@ -489,7 +502,7 @@ def mock_run( try: # Guard 1: Load config (ConfigError → envelope + exit 2) - cfg = load_config_or_raise(config_path, overlay_paths=list(ovs) if ovs else None) + cfg = load_config_or_raise(config_path, overlay_paths=list(ovs) if ovs else None, env_file=env_file) # Guard 2+3: Resolve engines to run run_http, run_kafka = _resolve_engines(only, cfg.mocks) diff --git a/agctl/config/env_file.py b/agctl/config/env_file.py new file mode 100644 index 0000000..86af3b7 --- /dev/null +++ b/agctl/config/env_file.py @@ -0,0 +1,92 @@ +"""Optional `.env` file loading — env defaults before interpolation (DESIGN §2.2). + +agctl reads `${VAR}` from the process environment. A `.env` file lets a project +commit those values as **defaults** for local dev / CI without forcing users to +`source` the file first. We merge the parsed values into the config env dict +with **real environment winning** (`.env` only fills keys not already set) — the +universal convention (docker compose, python-dotenv default, rails, django), so +CI/production can inject a real env var to override a committed default. + +Resolution order (highest precedence first): + +1. `--env-file ` (explicit). +2. `AGCTL_ENV_FILE` env var. +3. `.env` sitting next to the **resolved** `agctl.yaml` (best-effort: a missing + sibling is a normal no-op, not an error). + +Explicit sources (1, 2) are *required*: a missing file is a user-intent error +and raises ``ConfigError`` (mirrors `--config`). Only the auto-load sibling (3) +is allowed to be absent silently. + +We disable python-dotenv's *own* ``${VAR}`` expansion (``interpolate=False``) so +the raw values flow into the env dict and agctl's single ``interpolate()`` engine +(``loader.py``) owns all ``${...}`` resolution — including chains, e.g. a `.env` +line ``FOO=a-${B}`` plus env ``B=b`` yields ``a-b`` through agctl's multi-pass +interpolator. One engine, no double-substitution ambiguity. +""" + +import pathlib + +from dotenv import dotenv_values + +from ..errors import ConfigError + + +def load_env_file(path: pathlib.Path, *, required: bool) -> dict[str, str]: + """Parse a `.env` file into a ``{KEY: VALUE}`` dict. + + Args: + path: Path to the `.env` file. + required: If True, a missing file raises ``ConfigError`` (used for + explicit ``--env-file`` / ``AGCTL_ENV_FILE``). If False, a missing + file returns ``{}`` (used for the best-effort sibling auto-load). + + Returns: + Parsed key→value mapping. Bare keys (``KEY`` with no ``=``) are dropped + — python-dotenv returns ``None`` for them and they carry no value to + interpolate. ``KEY=`` is kept as the empty string. + + Raises: + ConfigError: ``required`` is True and the file is missing, or the file + exists but cannot be read. + """ + if not path.is_file(): + if required: + raise ConfigError(f"Env file not found: {path}", {"path": str(path)}) + return {} + try: + # interpolate=False: raw values — agctl's interpolate() owns ${...}. + raw = dotenv_values(path, interpolate=False) + except (OSError, UnicodeDecodeError) as exc: + # OSError: unreadable / permission / etc. + # UnicodeDecodeError: file is not valid UTF-8 (dotenv decodes internally; + # a ValueError subclass, NOT an OSError, so it needs explicit catching). + raise ConfigError( + f"Could not read env file {path}: {exc}", {"path": str(path)} + ) from exc + # dotenv_values -> Dict[str, str | None]; drop bare-key Nones. + return {key: value for key, value in raw.items() if value is not None} + + +def resolve_dotenv_values( + explicit: str | None, env: dict[str, str], base_path: pathlib.Path +) -> dict[str, str]: + """Pick the `.env` source per the precedence order and return its values. + + Args: + explicit: Value of ``--env-file`` (None if not given). + env: The config env dict (real environment, pre-merge). ``AGCTL_ENV_FILE`` + is read from here — so it must be set in the *real* environment, not + inside the `.env` itself (that would be circular and is ignored). + base_path: The resolved ``agctl.yaml`` path; its parent dir holds the + auto-load sibling `.env`. + + Returns: + Parsed values from the chosen source (may be empty). Real-env-wins + merging is the caller's responsibility (``{**dotenv, **env}``). + """ + if explicit: + return load_env_file(pathlib.Path(explicit), required=True) + if "AGCTL_ENV_FILE" in env: + return load_env_file(pathlib.Path(env["AGCTL_ENV_FILE"]), required=True) + return load_env_file(base_path.parent / ".env", required=False) diff --git a/agctl/config/loader.py b/agctl/config/loader.py index ffceb7b..fdf95d5 100644 --- a/agctl/config/loader.py +++ b/agctl/config/loader.py @@ -9,6 +9,7 @@ from pydantic import ValidationError from ..errors import ConfigError +from .env_file import resolve_dotenv_values from .models import Config, PartialConfig from .resolver import apply_env_overrides @@ -231,28 +232,32 @@ def compose_config( path: str | None = None, overlays: list[str] | None = None, env: dict[str, str] | None = None, + env_file: str | None = None, ) -> ComposedConfig: """Compose base config with overlay fragments. Pipeline: 1. Resolve env (defaults to os.environ). 2. Discover base config path. - 3. Load and interpolate base YAML. - 4. Check base version (must be v3). - 5. For each overlay (in order): + 3. Merge `.env` defaults into env (real env wins; DESIGN §2.2). + 4. Load and interpolate base YAML. + 5. Check base version (must be v3). + 6. For each overlay (in order): - Verify file exists. - Load and interpolate overlay YAML. - Validate fragment with PartialConfig. - Check overlay version major matches TOOL_MAJOR_VERSION if present. - Deep merge into base, recording overrides. - 6. Apply environment variable overrides to merged dict. - 7. Validate final Config. - 8. Return ComposedConfig(config, overrides). + 7. Apply environment variable overrides to merged dict. + 8. Validate final Config. + 9. Return ComposedConfig(config, overrides). Args: path: Explicit base config path (discovery if None). overlays: List of overlay file paths to apply in order. env: Environment dict (defaults to os.environ). + env_file: Explicit `.env` path (``--env-file``). None → fall back to + ``AGCTL_ENV_FILE``, then the `.env` sibling of the resolved config. Returns: ComposedConfig with final Config and override records. @@ -262,6 +267,15 @@ def compose_config( """ env = env if env is not None else os.environ base_path = discover_config_path(explicit=path, env=env) + # Merge `.env` defaults BEFORE interpolation so ${VAR} can resolve from it. + # Done after config discovery so the sibling .env sits next to the RESOLVED + # agctl.yaml, and so AGCTL_CONFIG / AGCTL_ENV_FILE set inside .env cannot + # steer their own resolution (only real-env values do — no cycle). Real env + # wins: .env only fills keys not already set. We build a NEW dict rather than + # mutate os.environ, so config loading has no process-env side effects. + dotenv = resolve_dotenv_values(env_file, env, base_path) + if dotenv: + env = {**dotenv, **env} base_raw = interpolate(yaml.safe_load(base_path.read_text()) or {}, env) _check_version(base_raw) @@ -314,6 +328,7 @@ def load_config( path: str | None = None, env: dict[str, str] | None = None, overlays: list[str] | None = None, + env_file: str | None = None, ) -> Config: """Full pipeline: discover -> parse -> interpolate -> merge overlays -> override -> validate. @@ -321,6 +336,7 @@ def load_config( path: Explicit config path (discovery if None). env: Environment dict (defaults to os.environ). overlays: Optional list of overlay file paths to compose. + env_file: Explicit `.env` path (``--env-file``); None → AGCTL_ENV_FILE → sibling. Returns: Validated Config object. @@ -328,7 +344,7 @@ def load_config( Raises: ConfigError: On any pipeline error. """ - return compose_config(path, overlays, env).config + return compose_config(path, overlays, env, env_file).config def _check_version(data: dict) -> None: diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 1ed49f9..5cae9bc 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -108,7 +108,8 @@ agctl/ ├── assertion_registry.py # pluggable assertion-mode registry + entry-point discovery ├── plugin_protocol.py # Protocol contract for protocol plugins ├── config/ -│ ├── loader.py # discover → parse → interpolate → override → version → validate +│ ├── loader.py # discover → merge .env → parse → interpolate → version → overlay-merge → override → validate +│ ├── env_file.py # .env auto-load + precedence (--env-file > AGCTL_ENV_FILE > sibling) │ ├── resolver.py # AGCTL_
__ override layer │ ├── validator.py # cross-reference checks + description warnings │ └── models.py # Pydantic v2 typed config models @@ -250,7 +251,7 @@ discovers, parses, and validates `agctl.yaml` from scratch. This is what makes `config/loader.py::load_config` → `compose_config`, fixed order; any stage may fail the load with a `ConfigError`: ``` -discover_config_path → yaml.safe_load → interpolate → _check_version (base must be v3) +discover_config_path → merge .env defaults into env (real env wins) → yaml.safe_load → interpolate → _check_version (base must be v3) ↓ for each overlay (in flag order): discover_config_path (explicit only) → yaml.safe_load → interpolate @@ -272,6 +273,24 @@ discover_config_path → yaml.safe_load → interpolate → _check_version (base `.git` or the filesystem root; first match wins. 4. None found → `ConfigError`. +**`.env` auto-load** (`config/env_file.py`, new) — after config discovery and +*before* interpolation, a `.env` file is merged into the config env dict as +**defaults**: real env wins (`env = {**dotenv, **env}` — a fresh dict, `os.environ` +is never mutated). Source precedence: `--env-file` (explicit, `required=True`) > +`AGCTL_ENV_FILE` (read from the *real* env only — a value set inside the `.env` +itself would be circular and is ignored) > `.env` sibling of the resolved +`agctl.yaml` (`required=False`: a missing sibling is a no-op, not an error). An +explicit source that is missing raises `ConfigError` (mirrors `--config`). +`load_env_file` parses with `dotenv_values(path, interpolate=False)` and drops +bare-key `None` values, so raw `${...}` flows into agctl's single `interpolate()` +— one engine owns all `${...}` resolution (chains, nested). Placement is +deliberately post-discovery (the sibling sits next to the *resolved* config) and +pre-interpolation (so `${VAR}` can resolve from `.env`); a consequence is that +`AGCTL_CONFIG`/`AGCTL_ENV_FILE` set *inside* the `.env` cannot steer their own +resolution — only real-env values do. `compose_config`, `load_config`, and +`load_config_or_raise` gained an `env_file` param threaded from the CLI's global +`--env-file` flag (DESIGN §3). + **Env interpolation** (`interpolate`, DESIGN §2.2), in every string scalar: - `${VAR}` — required; missing → collected and the load raises one `ConfigError` @@ -859,7 +878,7 @@ extras, so a user installs only what they need and the package imports fast: | Group | Dependencies | Needed for | |---|---|---| -| core (always) | `click`, `pyyaml`, `pydantic` | CLI, config loading, schema | +| core (always) | `click`, `pyyaml`, `pydantic`, `python-dotenv` | CLI, config loading, schema, `.env` parsing | | `http` | `httpx` | `http *`, `check ready` | | `jq` | `jq` | HTTP response assertions (`--match`/`--jq-path` on `http call`/`request`), mock HTTP `match.jq` (and mock startup pre-compile of stub `match.jq` / reactor `match`) | | `kafka` | `confluent-kafka`, `jq` | `kafka *` | diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 072ab31..0ea1278 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -435,6 +435,13 @@ Any YAML string value containing `${...}` is resolved at load time. Three forms The `${...}` syntax is only supported in string scalar values, not in keys. +**`.env` defaults.** Env vars for `${...}` interpolation and `AGCTL_*` overrides may also come from a `.env` file, loaded as **defaults** — a value already in the real environment overrides `.env`, so CI/production can inject real secrets over committed defaults (the universal convention: docker compose, python-dotenv, rails, django): + +- A `.env` next to the **resolved** `agctl.yaml` is loaded automatically — no need to `source` it in the shell first. +- Point at a different file with `--env-file ` (global flag) or `AGCTL_ENV_FILE`. Precedence: `--env-file` > `AGCTL_ENV_FILE` > sibling `.env`; an explicit source *replaces* the auto-load (like `--config` replaces walk-up). +- A missing sibling `.env` is a silent no-op; a missing explicit `--env-file` / `AGCTL_ENV_FILE` is a config error (exit 2), mirroring `--config`. +- Raw `.env` values flow into the same `${...}` interpolation as real env vars — agctl's single engine owns all `${...}` resolution (chained, nested), so a line like `FOO=a-${BAR}` resolves consistently. The `.env` itself does not do its own `${VAR}` expansion. + ### 2.3 Path Parameter Syntax HTTP template `path` and `body` values use `{placeholder}` (single braces) for runtime substitution via `--param key=value`. SQL (templates and free-form) uses `:paramName` (JDBC-style) instead — `{...}` is avoided in SQL to prevent collisions with JSON literals. Both are distinct from env var interpolation (`${...}`), which is resolved at config load time. @@ -467,6 +474,7 @@ All commands share these global flags: | Flag | Default | Description | |---|---|---| | `--config ` | auto-discovered | Explicit path to `agctl.yaml` | +| `--env-file ` | `.env` next to resolved config | Explicit path to a `.env` file; values are defaults, real env wins (precedence: `--env-file` > `AGCTL_ENV_FILE` > sibling `.env`) | | `--overlay ` | — | Overlay config fragment (repeatable; later wins); layered on base config | | `--timeout ` | from config `defaults` | Override request/operation timeout | | `--version` | — | Print version and exit | @@ -2288,7 +2296,7 @@ Refused to overwrite (without `--force`): 1. **`--config ` CLI flag** — if provided, only this file is loaded; no discovery walk is performed. 2. **`AGCTL_CONFIG` environment variable** — if set, used as the config file path. Ignored when `--config` is explicitly passed. 3. **Auto-discovery** — search for `agctl.yaml` starting in the current working directory, walking up parent directories until the filesystem root or a `.git` directory is found (whichever is first). The first `agctl.yaml` found wins. -4. **`${ENV_VAR}` interpolation within YAML values** — after the file is located and parsed, all `${VAR}` references in string values are resolved from the process environment. +4. **`${ENV_VAR}` interpolation within YAML values** — after the file is located and parsed, all `${VAR}` references in string values are resolved from the process environment, supplemented by `.env` defaults (real env wins; see §2.2 for `.env` source precedence). `AGCTL_*` overrides (step 6) likewise read from this merged env, so an `AGCTL_*` line in `.env` applies unless overridden by the real env. 5. **`--overlay ` CLI flags (repeatable)** — after base interpolation, each overlay file is loaded, interpolated, and deep-merged into the base config in flag order (later overlays win on conflict). Overlay version (if present) must match the base config's major version. 6. **`AGCTL_
_` environment variable overrides** — after overlays are merged, specific values can be overridden by structured env vars (see §8 for the exact convention). These have the highest precedence and win over both base and overlay values. @@ -2328,7 +2336,9 @@ that code changes work correctly against running services. ### Setup `agctl` reads `agctl.yaml` at the project root. Required environment variables -are listed in `.env.example`. Ensure they are set before running any `agctl` command. +are listed in `.env.example` — copy it to `.env` and fill in the values. agctl +auto-loads a `.env` next to `agctl.yaml` (real env wins), so no shell sourcing +is needed before running `agctl` commands. ### Discovering Available Resources diff --git a/pyproject.toml b/pyproject.toml index 672a53a..b68310d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -28,6 +28,7 @@ dependencies = [ "click>=8.1", "pyyaml>=6.0", "pydantic>=2.0", + "python-dotenv>=1.0", ] [project.optional-dependencies] diff --git a/skills/agctl-config/SKILL.md b/skills/agctl-config/SKILL.md index 788c97f..203c16c 100644 --- a/skills/agctl-config/SKILL.md +++ b/skills/agctl-config/SKILL.md @@ -88,7 +88,10 @@ field (`{customer_id}`, `:orderId`) — never kebab-case inside a `{}` or `:`. changes, and ask before overwriting. Never duplicate a key or silently clobber. **6. Secrets → env.** Header values, DB passwords, tokens → `${ENV_VAR}` (or `${ENV:-}` if -optional), and add the var to `.env.example`. Never inline a real secret. +optional), and add the var to `.env.example`. Never inline a real secret. agctl auto-loads a +`.env` next to the resolved `agctl.yaml` (real env wins), so those vars resolve at `config +validate` time with no shell sourcing — reach for `--env-file`/`AGCTL_ENV_FILE` only to point +at a different location. **7. Clarify, don't guess.** Ask only about genuine gaps (which connection? replace or append? what's the service's base URL?). Each question carries a recommended default. diff --git a/skills/agctl-config/reference/init-config.md b/skills/agctl-config/reference/init-config.md index f79894f..bdcc1a5 100644 --- a/skills/agctl-config/reference/init-config.md +++ b/skills/agctl-config/reference/init-config.md @@ -68,8 +68,11 @@ Never put real secret values in it. - Put `version: "3"` at the top. - Write `.env.example` (above), then have the user copy & fill it: `cp .env.example .env` (edit the secrets). -- Source it before validating, so required `${VAR}`s resolve: `set -a; . ./.env; set +a`. +- agctl auto-loads a `.env` next to the resolved `agctl.yaml` (real env wins) — required + `${VAR}`s resolve at `config validate` with **no shell sourcing**. `--env-file`/`AGCTL_ENV_FILE` + point at a different location. (`set -a; . ./.env; set +a` is only needed if other shell + tooling reads the same vars.) - **Then** run the mandatory verify from `SKILL.md`: `agctl config validate` (expect `ok:true`) followed by `agctl discover` (summary → a category → a sample item). -- A bare-required `${VAR}` makes `config validate` exit 2 *until `.env` is sourced* — that's +- A bare-required `${VAR}` makes `config validate` exit 2 *until `.env` provides it* — that's expected, not a failure. Never declare done while it still exits 2. diff --git a/skills/agctl/SKILL.md b/skills/agctl/SKILL.md index f891707..aefb1db 100644 --- a/skills/agctl/SKILL.md +++ b/skills/agctl/SKILL.md @@ -51,8 +51,10 @@ flags: see `--help`.) | Impersonate a dependency | `mock run` (foreground) / `mock start\|stop\|status` (daemon) | | Are services up? / validate config / migrate v1/v2→v3 | `check ready --all` / `config validate` / `config migrate` | -`--config ` and `--overlay ` (repeatable) are global; **`--timeout` is -not global**. `` for kafka = `--contains '{…}' | --match '' | --pattern `. +`--config `, `--overlay ` (repeatable), and `--env-file ` are global; +**`--timeout` is not global**. A `.env` next to the resolved `agctl.yaml` is auto-loaded as +env defaults (real env wins) — reach for `--env-file`/`AGCTL_ENV_FILE` only to point at a +different one. `` for kafka = `--contains '{…}' | --match '' | --pattern `. `kafka produce|consume|assert` take `--cluster ` (default: the pattern's bound cluster for `assert --pattern`, else `kafka.default_cluster`, else the single defined cluster; `--cluster` always wins) — set it only when a command must target a non-default diff --git a/tests/unit/test_env_file.py b/tests/unit/test_env_file.py new file mode 100644 index 0000000..b6cdf71 --- /dev/null +++ b/tests/unit/test_env_file.py @@ -0,0 +1,349 @@ +"""Tests for `.env` auto-loading and explicit `--env-file` / `AGCTL_ENV_FILE` (DESIGN §2.2). + +Covers: +- ``load_env_file`` parsing (quotes, comments, ``export`` prefix, bare keys, empty values). +- Real-environment-wins semantics (``.env`` provides defaults only). +- Auto-load of the ``.env`` sibling next to the resolved ``agctl.yaml``. +- Explicit sources: ``--env-file`` / ``AGCTL_ENV_FILE`` (missing -> ConfigError). +- Precedence: ``--env-file`` > ``AGCTL_ENV_FILE`` > sibling ``.env``. +- ``AGCTL_*`` overrides carried via ``.env``. +- Single interpolation engine: dotenv raw values + agctl's chained interpolator. +- CLI wiring (CliRunner): post-subcommand ``--env-file``, global flag, env var. +""" + +import json +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from agctl.cli import cli +from agctl.config import ConfigError, load_config +from agctl.config.env_file import load_env_file + + +# --- minimal valid config with a single ${VAR} to resolve ------------------ + +_CFG = """version: "3" +services: + s1: + base_url: "${BASE}" + health_path: /h + timeout_seconds: 5 +""" + + +def _write_cfg(d: Path, text: str = _CFG) -> Path: + p = d / "agctl.yaml" + p.write_text(text) + return p + + +# --- load_env_file parsing -------------------------------------------------- + + +def test_load_env_file_parses_common_forms(tmp_path): + envf = tmp_path / ".env" + envf.write_text( + "# a comment\n" + "\n" + "PLAIN=value\n" + "export EXPORTED=qux\n" # `export` prefix stripped + 'QUOTED="hello world"\n' # double quotes removed + "EQUAL=empty\n" # value with content + "EMPTYVAL=\n" # empty value -> "" + ) + vals = load_env_file(envf, required=False) + assert vals["PLAIN"] == "value" + assert vals["EXPORTED"] == "qux" + assert vals["QUOTED"] == "hello world" + assert vals["EQUAL"] == "empty" + assert vals["EMPTYVAL"] == "" + + +def test_load_env_file_drops_bare_keys(tmp_path): + """A bare KEY (no '=') carries no value and is dropped (dotenv returns None).""" + envf = tmp_path / ".env" + envf.write_text("GOOD=1\nBARE\nALSO=2\n") + vals = load_env_file(envf, required=False) + assert vals == {"GOOD": "1", "ALSO": "2"} + + +def test_load_env_file_missing_required_raises(tmp_path): + """An explicitly requested file that is missing -> ConfigError (mirrors --config).""" + missing = tmp_path / "nope.env" + with pytest.raises(ConfigError) as exc: + load_env_file(missing, required=True) + assert str(missing) in exc.value.message + + +def test_load_env_file_missing_optional_is_noop(tmp_path): + """A missing sibling auto-load file is a silent no-op (normal).""" + assert load_env_file(tmp_path / ".env", required=False) == {} + + +def test_dotenv_values_are_raw_not_expanded(tmp_path): + """Headline invariant: dotenv_values is called with interpolate=False, so a + value like ``FOO=a-${B}`` stays LITERAL in the env dict — agctl's own + ``interpolate()`` owns all ``${...}`` resolution. This assertion FAILS if + someone flips ``interpolate=False`` -> ``True`` (then vals["FOO"] == "a-b").""" + envf = tmp_path / ".env" + envf.write_text("B=b\nFOO=a-${B}\n") + vals = load_env_file(envf, required=False) + assert vals["FOO"] == "a-${B}" # literal, NOT expanded + + +def test_malformed_utf8_env_file_raises_config_error(tmp_path): + """A .env that isn't valid UTF-8 surfaces as ConfigError, not InternalError. + UnicodeDecodeError is a ValueError (not an OSError), so it must be caught + explicitly alongside OSError in load_env_file.""" + envf = tmp_path / "bad.env" + envf.write_bytes(b"KEY=bad\xff\xfe\n") + with pytest.raises(ConfigError): + load_env_file(envf, required=True) + + +# --- pipeline semantics: real-env-wins, auto-load, explicit ---------------- + + +def test_sibling_dotenv_provides_default(tmp_path): + """A bare required ${VAR} resolves from the .env sitting next to agctl.yaml.""" + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=from-dotenv\n") + cfg = load_config(str(tmp_path / "agctl.yaml"), env={}) + assert cfg.services["s1"].base_url == "from-dotenv" + + +def test_real_env_wins_over_dotenv(tmp_path): + """Values already in the real env override .env (.env is defaults only).""" + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=from-dotenv\n") + cfg = load_config(str(tmp_path / "agctl.yaml"), env={"BASE": "from-real"}) + assert cfg.services["s1"].base_url == "from-real" + + +def test_missing_sibling_dotenv_is_silent(tmp_path): + """No sibling .env -> no error; the bare ${VAR} simply stays unresolved.""" + _write_cfg(tmp_path) + with pytest.raises(ConfigError) as exc: + load_config(str(tmp_path / "agctl.yaml"), env={}) + assert "BASE" in exc.value.detail["variables"] + + +def test_bootstrap_var_in_dotenv_does_not_steer_resolution(tmp_path): + """AGCTL_ENV_FILE set INSIDE the .env cannot redirect its own resolution + (no cycle): resolution reads the real env BEFORE the dotenv merge, so only a + real-env AGCTL_ENV_FILE is honored. Here the sibling .env tries to point at + another file via AGCTL_ENV_FILE — it is ignored, and the sibling's own + BASE wins.""" + _write_cfg(tmp_path) + other = tmp_path / "other.env" + other.write_text("BASE=from-other\n") + (tmp_path / ".env").write_text("AGCTL_ENV_FILE=" + str(other) + "\nBASE=from-sibling\n") + cfg = load_config(str(tmp_path / "agctl.yaml"), env={}) + assert cfg.services["s1"].base_url == "from-sibling" + + +def test_auto_load_is_sibling_only_no_parent_walk(tmp_path): + """Auto-load looks ONLY at the .env next to the resolved config, not parent + dirs (asymmetric with config discovery, which walks up). A .env in a parent + dir is NOT loaded — BASE stays unresolved.""" + sub = tmp_path / "sub" + sub.mkdir() + _write_cfg(sub) + (tmp_path / ".env").write_text("BASE=from-parent-dir\n") # parent of config — ignored + with pytest.raises(ConfigError) as exc: + load_config(str(sub / "agctl.yaml"), env={}) + assert "BASE" in exc.value.detail["variables"] + + +def test_explicit_env_file_overrides_sibling(tmp_path): + """--env-file (explicit) replaces the sibling auto-load, like --config replaces walk-up.""" + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=from-sibling\n") + explicit = tmp_path / "explicit.env" + explicit.write_text("BASE=from-explicit\n") + cfg = load_config(str(tmp_path / "agctl.yaml"), env={}, env_file=str(explicit)) + assert cfg.services["s1"].base_url == "from-explicit" + + +def test_agctl_env_file_env_var(tmp_path): + """AGCTL_ENV_FILE (real env) points at the .env to load.""" + _write_cfg(tmp_path) + envf = tmp_path / "via-env.env" + envf.write_text("BASE=via-env-var\n") + cfg = load_config(str(tmp_path / "agctl.yaml"), env={"AGCTL_ENV_FILE": str(envf)}) + assert cfg.services["s1"].base_url == "via-env-var" + + +def test_precedence_explicit_beats_envvar_beats_sibling(tmp_path): + """--env-file > AGCTL_ENV_FILE > sibling .env.""" + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=sibling\n") + via_env = tmp_path / "via_env.env" + via_env.write_text("BASE=env-var\n") + explicit = tmp_path / "explicit.env" + explicit.write_text("BASE=explicit\n") + + # explicit beats env var + cfg = load_config( + str(tmp_path / "agctl.yaml"), + env={"AGCTL_ENV_FILE": str(via_env)}, + env_file=str(explicit), + ) + assert cfg.services["s1"].base_url == "explicit" + + # env var beats sibling + cfg = load_config( + str(tmp_path / "agctl.yaml"), env={"AGCTL_ENV_FILE": str(via_env)} + ) + assert cfg.services["s1"].base_url == "env-var" + + +def test_explicit_env_file_missing_raises(tmp_path): + """A missing explicit --env-file is a user error -> ConfigError (not a no-op).""" + _write_cfg(tmp_path) + with pytest.raises(ConfigError): + load_config( + str(tmp_path / "agctl.yaml"), env={}, env_file=str(tmp_path / "nope.env") + ) + + +def test_missing_agctl_env_file_raises(tmp_path): + """A missing AGCTL_ENV_FILE target is a user error -> ConfigError.""" + _write_cfg(tmp_path) + with pytest.raises(ConfigError): + load_config( + str(tmp_path / "agctl.yaml"), + env={"AGCTL_ENV_FILE": str(tmp_path / "nope.env")}, + ) + + +# --- emergent: AGCTL_* overrides via .env, and single interpolation engine -- + + +def test_agctl_override_via_dotenv(tmp_path): + """An AGCTL_
__ line in .env applies as an env override.""" + cfg_yaml = tmp_path / "agctl.yaml" + cfg_yaml.write_text( + 'version: "3"\n' + "services:\n" + " s1:\n" + ' base_url: "http://x"\n' + " health_path: /h\n" + " timeout_seconds: 5\n" + "defaults:\n" + " timeout_seconds: 10\n" + " database_connection: none\n" + ) + (tmp_path / ".env").write_text("AGCTL_DEFAULTS__TIMEOUT_SECONDS=99\n") + cfg = load_config(str(cfg_yaml), env={}) + assert cfg.defaults.timeout_seconds == 99 + + +def test_single_interpolation_engine_chained(tmp_path): + """dotenv's own ${VAR} expansion is OFF; agctl's chained interpolator owns it. + + .env: B=b, FOO=a-${B} -> raw 'a-${B}' enters env -> ${FOO} resolves to 'a-b' + via agctl's multi-pass interpolate(). + """ + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("B=b\nFOO=a-${B}\n") + # Switch the config to reference ${FOO} instead of ${BASE}. + _write_cfg(tmp_path, _CFG.replace("${BASE}", "${FOO}")) + cfg = load_config(str(tmp_path / "agctl.yaml"), env={"B": "b"}) + assert cfg.services["s1"].base_url == "a-b" + + +# --- CLI wiring ------------------------------------------------------------ + + +def _valid(res) -> None: + assert res.exit_code == 0, res.output + + +def test_cli_auto_load_sibling_dotenv(tmp_path): + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=from-cli\n") + res = CliRunner().invoke( + cli, ["--config", str(tmp_path / "agctl.yaml"), "config", "validate"], env={} + ) + _valid(res) + + +def test_cli_post_subcommand_env_file(tmp_path): + """`agctl config validate --env-file X` works (per-command flag).""" + _write_cfg(tmp_path) + envf = tmp_path / "x.env" + envf.write_text("BASE=from-flag\n") + res = CliRunner().invoke( + cli, + [ + "--config", + str(tmp_path / "agctl.yaml"), + "config", + "validate", + "--env-file", + str(envf), + ], + env={}, + ) + _valid(res) + + +def test_cli_global_env_file_flag(tmp_path): + """`agctl --env-file X config validate` works (global flag before subcommand).""" + _write_cfg(tmp_path) + envf = tmp_path / "x.env" + envf.write_text("BASE=from-global\n") + res = CliRunner().invoke( + cli, + [ + "--env-file", + str(envf), + "--config", + str(tmp_path / "agctl.yaml"), + "config", + "validate", + ], + env={}, + ) + _valid(res) + + +def test_cli_agctl_env_file_var(tmp_path): + _write_cfg(tmp_path) + envf = tmp_path / "x.env" + envf.write_text("BASE=from-var\n") + res = CliRunner().invoke( + cli, + ["--config", str(tmp_path / "agctl.yaml"), "config", "validate"], + env={"AGCTL_ENV_FILE": str(envf)}, + ) + _valid(res) + + +def test_cli_real_env_wins(tmp_path): + """Real env wins over .env at the CLI layer — verify the VALUE, not just + exit-0 (exit-0 would also pass if .env clobbered the real env).""" + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=from-file\n") + res = CliRunner().invoke( + cli, + ["--config", str(tmp_path / "agctl.yaml"), "config", "show", "--unmask"], + env={"BASE": "from-real-env"}, + ) + out = json.loads(res.output) + assert out["result"]["services"]["s1"]["base_url"] == "from-real-env" + + +def test_cli_show_reflects_dotenv(tmp_path): + _write_cfg(tmp_path) + (tmp_path / ".env").write_text("BASE=shown\n") + res = CliRunner().invoke( + cli, + ["--config", str(tmp_path / "agctl.yaml"), "config", "show", "--unmask"], + env={}, + ) + out = json.loads(res.output) + assert out["result"]["services"]["s1"]["base_url"] == "shown" diff --git a/tests/unit/test_mock_commands.py b/tests/unit/test_mock_commands.py index 1383c83..4f4d7f6 100644 --- a/tests/unit/test_mock_commands.py +++ b/tests/unit/test_mock_commands.py @@ -723,6 +723,69 @@ def fake_is_alive(pid): # Next item should be the absolute path to the overlay assert daemon_argv[overlay_idx + 1] == str(Path(ov).absolute()) + +# Regression: mock start daemon argv forwards --env-file (the daemon re-loads +# config from scratch, so it must receive the flag or it silently falls back to +# the default .env sibling — the parent's readiness load used the right file, +# the server that serves traffic would not). +@pytest.mark.skipif( + os.name == "nt", + reason="managed daemon is POSIX-only; gated by _require_posix_daemon on Windows", +) +def test_mock_start_includes_env_file_in_daemon_argv(tmp_path, monkeypatch): + """mock start forwards --env-file to the daemon argv so the spawned daemon + loads the same .env as the parent (otherwise it silently uses the default).""" + base = tmp_path / "agctl.yaml" + base.write_text("""version: "3" +services: + orders: + base_url: http://localhost:8081 +mocks: + http: + listen: "0.0.0.0:18080" + stubs: + stub1: + method: GET + path: /test + response: + status: 200 + body: '{}' +""") + envf = tmp_path / "secrets.env" + envf.write_text("BASE=http://from-envfile\n") + + captured_argv = [] + + def fake_spawn_daemon(argv, log_path, env=None): + captured_argv.append(argv) + return 12345 # Fake PID + + monkeypatch.setattr("agctl.commands.mock_commands.spawn_daemon", fake_spawn_daemon) + monkeypatch.setattr("agctl.commands.mock_commands.is_alive", lambda pid: True) + + from unittest.mock import MagicMock + + fake_parsed = MagicMock() + fake_parsed.started = {"http": {"listen": "0.0.0.0:18080", "stubs": 1}} + fake_parsed.startup_error = None + monkeypatch.setattr("agctl.commands.mock_commands.parse_log", lambda log_path: fake_parsed) + monkeypatch.setattr("agctl.commands.mock_commands.read_pidfile", lambda pidfile: None) + + result = CliRunner().invoke( + cli, + ["--env-file", str(envf), "--config", str(base), "mock", "start"], + ) + + assert len(captured_argv) == 1, result.output + daemon_argv = captured_argv[0] + + # Global flags (--env-file) must appear BEFORE the "mock" subcommand. + mock_idx = daemon_argv.index("mock") + assert "--env-file" in daemon_argv + env_file_idx = daemon_argv.index("--env-file") + assert env_file_idx < mock_idx, "--env-file must appear before the 'mock' subcommand" + assert daemon_argv[env_file_idx + 1] == str(Path(envf).absolute()) + # Verify --config also appears BEFORE the "mock" subcommand (if provided) if "--config" in daemon_argv: config_idx = daemon_argv.index("--config") diff --git a/tests/unit/test_overlay.py b/tests/unit/test_overlay.py index 5cdad85..899a0bc 100644 --- a/tests/unit/test_overlay.py +++ b/tests/unit/test_overlay.py @@ -598,7 +598,7 @@ def test_http_call_forwards_overlay(tmp_path, monkeypatch): # Track what overlay_paths was passed to load_config_or_raise captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) from agctl.config.models import HttpTemplate return Config( @@ -656,7 +656,7 @@ def test_db_query_forwards_overlay(tmp_path, monkeypatch): captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) from agctl.config.models import DatabaseConfig, ConnectionConfig return Config( @@ -702,7 +702,7 @@ def test_kafka_produce_forwards_overlay(tmp_path, monkeypatch): captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) from agctl.config.models import KafkaCluster, KafkaConfig return Config( @@ -741,7 +741,7 @@ def test_check_ready_forwards_overlay(tmp_path, monkeypatch): captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) return Config( version="2", @@ -775,7 +775,7 @@ def test_mock_run_forwards_overlay(tmp_path, monkeypatch): captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) # Return minimal valid config (no mocks needed for this test) return Config( @@ -827,7 +827,7 @@ def test_http_ping_forwards_overlay(tmp_path, monkeypatch): captured_overlays = [] - def fake_load_config(config_path, overlay_paths=None): + def fake_load_config(config_path, overlay_paths=None, env_file=None): captured_overlays.append(overlay_paths) from agctl.config.models import HttpTemplate return Config(