Skip to content

Commit 8836aab

Browse files
luca-iachiniLukeMathWalker
authored andcommitted
feat(cli)!: unify config env to FIRMA_CONFIG (#627)
## Why `firma sidecar` bound `--config` to `FIRMA_SIDECAR_CONFIG_FILE`, `authority` bound no env, and the stack-inspection commands bound `FIRMA_CONFIG` — all selecting the same unified `firma.toml`. The `FIRMA_SIDECAR_CONFIG_FILE` name was also overloaded as a `firma run` autostart sidecar-template source, so setting it for one purpose silently affected the other. This unifies unified-config selection under one canonical flag and env. ## What Changed - Add a global `-c` / `--config` bound to `FIRMA_CONFIG` on the top-level `firma` command (accepted before or after any subcommand); `main.rs` threads the resolved path into each consumer. - Remove the per-command unified-config `--config` from `control`, `doctor`, `monitor`, `sidecar` (serve/start/stop), and `authority`. `firma run --config` keeps working via the global (it was already the unified-config selector). - Fully retire `FIRMA_SIDECAR_CONFIG_FILE`, including the `firma run` autostart template fallback (`TemplateSource::Env` removed; selection is now `--sidecar-config` → `./firma_sidecar.toml` → synthesized minimal). - Update docs (cli.md, README, manage-the-stack, firma-run, examples) and extend the `--help` contract test to sidecar/authority. ## Risks / Notes - **Breaking (`feat!`)**: `FIRMA_SIDECAR_CONFIG_FILE` is removed. Migration: export `FIRMA_CONFIG` to select the unified `firma.toml`; use `--sidecar-config <path>` (or `./firma_sidecar.toml`) for a `firma run` autostart template. - Config resolution precedence and fail-closed behavior are unchanged (still owned by `firma-config-loader` `ConfigResolver`); the path — not a pre-resolved config — is threaded because `doctor` reports resolution, `run` may scaffold, and `sidecar start` uses a different resolver. - As a clap global, `--config` is also accepted (and ignored) by commands that do not consume the unified config (`token`, `policy`, internal `__*`). - Design/rationale: `docs/architecture/config-env-unification-plan.md`. - Stacked on top of #619 (`amp/canonical-config-discovery`); base retargets to `main` once that merges. ## AI Assistance Claude Opus 4.8 (claude-opus-4-8).
1 parent ae67384 commit 8836aab

26 files changed

Lines changed: 218 additions & 157 deletions

File tree

crates/firma-run/src/routing.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -901,10 +901,6 @@ fn prepare_run_components(
901901
})?;
902902
let firma_exe = std::env::current_exe()
903903
.map_err(|error| RunError::Internal(format!("resolve current executable: {error}")))?;
904-
let env_template = std::env::var("FIRMA_SIDECAR_CONFIG_FILE")
905-
.ok()
906-
.filter(|value| !value.trim().is_empty())
907-
.map(PathBuf::from);
908904
let cwd_template = std::env::current_dir()
909905
.ok()
910906
.map(|cwd| cwd.join("firma_sidecar.toml"));
@@ -1006,7 +1002,6 @@ fn prepare_run_components(
10061002
session_id: &identity.session_id,
10071003
marker_dir: marker_dir.clone(),
10081004
template_path: component_flags.template_path.as_deref(),
1009-
env_template: env_template.clone(),
10101005
cwd_template: cwd_template.clone(),
10111006
firma_exe: firma_exe.clone(),
10121007
authority_url: component_flags.authority_url.as_deref(),

crates/firma-run/src/sidecar/config.rs

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -303,8 +303,6 @@ pub struct SynthesizeRequest<'a> {
303303
pub session_id: &'a str,
304304
/// Highest-priority template path (typically `--sidecar-config`).
305305
pub explicit_template: Option<&'a Path>,
306-
/// Fallback template path from `FIRMA_SIDECAR_CONFIG_FILE`.
307-
pub env_template: Option<PathBuf>,
308306
/// Fallback template path from the current working directory.
309307
pub cwd_template: Option<PathBuf>,
310308
/// UDS path the spawned sidecar must bind.
@@ -352,7 +350,6 @@ pub struct SynthesizeRequest<'a> {
352350
#[derive(Debug, Clone, PartialEq, Eq)]
353351
pub enum TemplateSource {
354352
Explicit(PathBuf),
355-
Env(PathBuf),
356353
Cwd(PathBuf),
357354
Minimal,
358355
}
@@ -372,7 +369,7 @@ pub enum TemplateSource {
372369
pub fn synthesize(req: SynthesizeRequest<'_>) -> Result<TemplateSource, RunError> {
373370
let source = select_template(&req);
374371
let (mut value, template_dir) = match &source {
375-
TemplateSource::Explicit(path) | TemplateSource::Env(path) | TemplateSource::Cwd(path) => {
372+
TemplateSource::Explicit(path) | TemplateSource::Cwd(path) => {
376373
let abs = std::path::absolute(path).unwrap_or_else(|_| path.clone());
377374
(parse_template(path)?, abs.parent().map(Path::to_path_buf))
378375
}
@@ -489,11 +486,6 @@ fn select_template(req: &SynthesizeRequest<'_>) -> TemplateSource {
489486
{
490487
return TemplateSource::Explicit(path.to_path_buf());
491488
}
492-
if let Some(path) = req.env_template.as_deref()
493-
&& path.is_file()
494-
{
495-
return TemplateSource::Env(path.to_path_buf());
496-
}
497489
if let Some(path) = req.cwd_template.as_deref()
498490
&& path.is_file()
499491
{
@@ -1058,7 +1050,6 @@ mod tests {
10581050
execution_profile: firma_config_loader::AgentProfile::Vscode,
10591051
session_id: "sess_001",
10601052
explicit_template: None,
1061-
env_template: None,
10621053
cwd_template: None,
10631054
socket_path: &tmp.path().join("sidecar.sock"),
10641055
listen_addr: Some(

crates/firma-run/src/sidecar/prepare.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,6 @@ pub struct PrepareRequest<'a> {
6161
pub session_id: &'a str,
6262
pub marker_dir: PathBuf,
6363
pub template_path: Option<&'a Path>,
64-
pub env_template: Option<PathBuf>,
6564
pub cwd_template: Option<PathBuf>,
6665
pub firma_exe: PathBuf,
6766
pub authority_url: Option<&'a str>,
@@ -104,7 +103,6 @@ pub fn prepare(req: PrepareRequest<'_>) -> Result<PreparedSidecarLaunch, RunErro
104103
execution_profile: req.execution_profile,
105104
session_id: req.session_id,
106105
explicit_template: req.template_path,
107-
env_template: req.env_template,
108106
cwd_template: req.cwd_template,
109107
socket_path: &socket_path,
110108
listen_addr,

crates/firma-run/tests/integration/sidecar_config_merge.rs

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,6 @@ fn req<'a>(sock: &'a Path, out: &'a Path) -> SynthesizeRequest<'a> {
5151
execution_profile: AgentProfile::Generic,
5252
session_id: "sess",
5353
explicit_template: None,
54-
env_template: None,
5554
cwd_template: None,
5655
socket_path: sock,
5756
listen_addr: None,
@@ -269,13 +268,12 @@ paths = ["/etc/firma/cap.toml"]
269268
}
270269

271270
#[test]
272-
fn priority_order_explicit_over_env_over_cwd() {
271+
fn priority_order_explicit_over_cwd() {
273272
let tmp = TempDir::new().expect("tmp");
274273

275274
let explicit = tmp.path().join("explicit.toml");
276-
let env = tmp.path().join("env.toml");
277275
let cwd = tmp.path().join("cwd.toml");
278-
for path in [&explicit, &env, &cwd] {
276+
for path in [&explicit, &cwd] {
279277
fs::write(path, "[interceptor]\nmode = \"http_proxy\"\n").expect("write");
280278
}
281279

@@ -284,21 +282,12 @@ fn priority_order_explicit_over_env_over_cwd() {
284282

285283
let source = synthesize(SynthesizeRequest {
286284
explicit_template: Some(&explicit),
287-
env_template: Some(env.clone()),
288285
cwd_template: Some(cwd.clone()),
289286
..req(&sock, &out)
290287
})
291288
.expect("synthesize");
292289
assert_eq!(source, TemplateSource::Explicit(explicit));
293290

294-
let source = synthesize(SynthesizeRequest {
295-
env_template: Some(env.clone()),
296-
cwd_template: Some(cwd.clone()),
297-
..req(&sock, &out)
298-
})
299-
.expect("synthesize");
300-
assert_eq!(source, TemplateSource::Env(env));
301-
302291
let source = synthesize(SynthesizeRequest {
303292
cwd_template: Some(cwd.clone()),
304293
..req(&sock, &out)
@@ -477,7 +466,6 @@ fn nonexistent_template_paths_fall_through_to_minimal() {
477466
let explicit = PathBuf::from("/does/not/exist/explicit.toml");
478467
let source = synthesize(SynthesizeRequest {
479468
explicit_template: Some(&explicit),
480-
env_template: Some(PathBuf::from("/does/not/exist/env.toml")),
481469
cwd_template: Some(PathBuf::from("/does/not/exist/cwd.toml")),
482470
..req(&sock, &out)
483471
})

crates/firma-run/tests/integration/sidecar_prepare.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ fn request(
2121
session_id: "prepared-session",
2222
marker_dir,
2323
template_path: None,
24-
env_template: None,
2524
cwd_template: None,
2625
firma_exe: "/bin/false".into(),
2726
authority_url: Some("http://127.0.0.1:50051"),

crates/firma/README.md

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ firma sidecar --config /etc/firma/firma.toml
5151

5252
| Flag | Env | Default |
5353
| -------------------- | -------------------------------- | ---------------- |
54-
| `-c, --config` | `FIRMA_SIDECAR_CONFIG_FILE` | discovered |
54+
| `-c, --config` | `FIRMA_CONFIG` | discovered |
5555
| `--health-bind-addr` | `FIRMA_SIDECAR_HEALTH_BIND_ADDR` | `127.0.0.1:9000` |
5656

5757
When `--config` is omitted, a shared `firma.toml` is discovered from
@@ -222,12 +222,12 @@ Subcommands:
222222

223223
`start` / `stop` flags:
224224

225-
| Flag | Env | Default |
226-
| ------------- | --------------------------- | -------------------------------------------- |
227-
| `--config` | `FIRMA_SIDECAR_CONFIG_FILE` | discovered `firma.toml` (`start` only) |
228-
| `--state-dir` | `FIRMA_STATE_DIR` | `$XDG_RUNTIME_DIR/firma``/tmp/firma-$UID` |
229-
| `--detach` | | _off_ (`start` only) |
230-
| `--timeout` | | `2` seconds (`stop` only) |
225+
| Flag | Env | Default |
226+
| ------------- | ----------------- | ---------------------------------------------- |
227+
| `--config` | `FIRMA_CONFIG` | discovered `firma.toml` (`start` only, global) |
228+
| `--state-dir` | `FIRMA_STATE_DIR` | `$XDG_RUNTIME_DIR/firma``/tmp/firma-$UID` |
229+
| `--detach` || _off_ (`start` only) |
230+
| `--timeout` || `2` seconds (`stop` only) |
231231

232232
State-dir resolution order: `--state-dir` flag → `FIRMA_STATE_DIR` env →
233233
`$XDG_RUNTIME_DIR/firma``/tmp/firma-$UID` on Unix;

crates/firma/src/args/authority.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,6 @@ use firma_identifiers::TokenId;
77

88
#[derive(Debug, clap::Args)]
99
pub struct Args {
10-
/// Path to the Authority TOML config (issuer identity, key paths, bundle dir,
11-
/// listen address). When unset, falls back to platform discovery.
12-
#[arg(short, long)]
13-
pub config: Option<PathBuf>,
14-
1510
/// Internal path for writing the one-shot startup report.
1611
#[arg(long, hide = true)]
1712
pub startup_report: Option<PathBuf>,

crates/firma/src/args/control.rs

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,6 @@ use clap::Args as ClapArgs;
77
/// Parsed `firma control` command-line arguments.
88
#[derive(Debug, ClapArgs)]
99
pub struct Args {
10-
/// Stack config used to discover the sidecar audit log and Authority
11-
/// policy directory.
12-
#[arg(long, env = "FIRMA_CONFIG")]
13-
pub config: Option<PathBuf>,
1410
/// Override the runtime state directory containing the audit log.
1511
#[arg(long, env = "FIRMA_STATE_DIR")]
1612
pub state_dir: Option<PathBuf>,

crates/firma/src/args/doctor.rs

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,7 @@ use clap::Args as ClapArgs;
66

77
/// Parsed `firma doctor` arguments.
88
///
9-
/// `--config` is the unified `firma.toml`. It's optional; when unset,
10-
/// `doctor` resolves it via the standard config discovery precedence.
9+
/// The unified `firma.toml` is selected by the global `--config` / `FIRMA_CONFIG`.
1110
///
1211
/// `--state-dir` overrides the resolved state directory.
1312
///
@@ -18,10 +17,6 @@ use clap::Args as ClapArgs;
1817
/// already report unreachable as a structured `FAIL`, not a hang.
1918
#[derive(Debug, ClapArgs)]
2019
pub struct Args {
21-
/// Path to the unified `firma.toml`. When unset, resolved via standard
22-
/// config discovery (`FIRMA_CONFIG` / nearest `.firma/firma.toml`).
23-
#[arg(long, env = "FIRMA_CONFIG")]
24-
pub config: Option<PathBuf>,
2520
/// Override the runtime state directory inspected for pid files, sockets,
2621
/// and component logs.
2722
#[arg(long, env = "FIRMA_STATE_DIR")]
@@ -55,7 +50,6 @@ mod tests {
5550
#[test]
5651
fn defaults_parse() {
5752
let args = try_parse(&["firma-doctor"]).expect("parse default");
58-
assert!(args.config.is_none());
5953
assert!(args.state_dir.is_none());
6054
assert!(!args.json);
6155
assert_eq!(args.timeout_ms, 500);

crates/firma/src/args/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,11 @@ fn configure_help(command: &mut ClapCommand) {
7171
firma doctor — diagnose setup issues"
7272
)]
7373
pub struct Cli {
74+
/// Unified `firma.toml` selecting stack configuration. When unset, resolved
75+
/// via `FIRMA_CONFIG`, then the nearest `.firma/firma.toml`. Ignored by
76+
/// subcommands that do not consume the unified config.
77+
#[arg(long, short = 'c', global = true, env = "FIRMA_CONFIG")]
78+
pub config: Option<PathBuf>,
7479
/// Write logs to this file instead of stderr.
7580
#[arg(long, global = true, env = "FIRMA_LOG_FILE")]
7681
pub log_file: Option<PathBuf>,

0 commit comments

Comments
 (0)