refactor(maestro): delete the config projections and the dead knobs #46

2026-08-22T14:39:25+01:00

naps62-yolo commented

2026-08-22 14:39:25 +01:00

122 insertions, 459 deletions. Almost entirely removal.

maestro-cli/src/config.rs was the shallowest module in the workspace: two pass-throughs (one with zero callers), a newtype over one error, a two-field projection of MaestroConfigFile, and one four-line or_else with a single caller. resolve_daemon_url moves onto MaestroConfigFile — next to the schema it reads from — and the module is deleted. Its tests moved with it.

[web].dev_proxy_target is gone end to end: schema field, env var, trailing-slash normalisation, three tests, the README line, and the startup warning at main.rs:53 that admitted the server never honoured it.

MaestroConfigFile::save had no callers outside its own tests — the installer tells the user to hand-edit config.toml rather than writing it. Deleting it also removed every skip_serializing_if, all five is_empty impls, and the Serialize derives that existed solely to make its output tidy.

BroadcastTuning was a struct that was never instantiated: all three uses were ::DEFAULT.field in const position and there is no [broadcast] section in the schema. Now two plain consts.

$MAESTRO_CONFIG was honoured by two different mechanisms — resolve_config_path (TUI only) and clap's env = (everywhere else). Every binary already receives it through clap, including the TUI via the umbrella's global --config, so resolve_config_path and CONFIG_PATH_ENV are deleted and the TUI falls back to default_config_path like everything else. One mechanism, and it shows up in --help.

Finally, three layers of 1:1 forwarding collapse into one: the umbrella's cli_dispatch shim (which forwarded three arguments verbatim), and four near-identical 8-line functions in cli/dispatch.rs that differed only in which run_* they called. dispatch.rs goes 113 → 80 lines and now builds the client once instead of four times.

What I did not delete: maestro-daemon/src/config.rs and maestro-tui/src/config.rs. The review called all four config modules into question, but those two earn their keep — the daemon's does three-level env precedence, a gethostname syscall and derived paths; the TUI's compiles the keymap, including the rule that rebinding the leader moves its Insert-mode escape. The TUI one is arguably misnamed (keymap_builder.rs would be honest) but renaming it is churn without a payoff.

Verification

cargo fmt --all -- --check                   ok
cargo clippy --all-targets -- -D warnings    0
cargo test --all                             304 passed, 0 failed
pnpm -C web test                             36 passed

304 rather than 309: four save tests and one resolve_config_path test went with the code they covered, three CLI config tests moved to maestro-config, and the BroadcastTuning test became a two-line const check.

Seventh of nine from the architecture review (#40–#45 merged).

**122 insertions, 459 deletions.** Almost entirely removal. `maestro-cli/src/config.rs` was the shallowest module in the workspace: two pass-throughs (one with zero callers), a newtype over one error, a two-field projection of `MaestroConfigFile`, and one four-line `or_else` with a single caller. `resolve_daemon_url` moves onto `MaestroConfigFile` — next to the schema it reads from — and the module is deleted. Its tests moved with it. `[web].dev_proxy_target` is gone end to end: schema field, env var, trailing-slash normalisation, three tests, the README line, and the startup warning at `main.rs:53` that admitted the server never honoured it. `MaestroConfigFile::save` had no callers outside its own tests — the installer tells the user to hand-edit `config.toml` rather than writing it. Deleting it also removed every `skip_serializing_if`, all five `is_empty` impls, and the `Serialize` derives that existed solely to make its output tidy. `BroadcastTuning` was a struct that was never instantiated: all three uses were `::DEFAULT.field` in `const` position and there is no `[broadcast]` section in the schema. Now two plain consts. `$MAESTRO_CONFIG` was honoured by two different mechanisms — `resolve_config_path` (TUI only) and clap's `env =` (everywhere else). Every binary already receives it through clap, including the TUI via the umbrella's global `--config`, so `resolve_config_path` and `CONFIG_PATH_ENV` are deleted and the TUI falls back to `default_config_path` like everything else. One mechanism, and it shows up in `--help`. Finally, three layers of 1:1 forwarding collapse into one: the umbrella's `cli_dispatch` shim (which forwarded three arguments verbatim), and four near-identical 8-line functions in `cli/dispatch.rs` that differed only in which `run_*` they called. `dispatch.rs` goes 113 → 80 lines and now builds the client once instead of four times. **What I did not delete:** `maestro-daemon/src/config.rs` and `maestro-tui/src/config.rs`. The review called all four config modules into question, but those two earn their keep — the daemon's does three-level env precedence, a `gethostname` syscall and derived paths; the TUI's compiles the keymap, including the rule that rebinding the leader moves its Insert-mode escape. The TUI one is arguably misnamed (`keymap_builder.rs` would be honest) but renaming it is churn without a payoff. <details> <summary>Verification</summary> ``` cargo fmt --all -- --check ok cargo clippy --all-targets -- -D warnings 0 cargo test --all 304 passed, 0 failed pnpm -C web test 36 passed ``` 304 rather than 309: four `save` tests and one `resolve_config_path` test went with the code they covered, three CLI config tests moved to `maestro-config`, and the `BroadcastTuning` test became a two-line const check. </details> Seventh of nine from the architecture review (#40–#45 merged).

naps62-yolo added 1 commit 2026-08-22 14:39:25 +01:00

refactor(maestro): delete the config projections and the dead knobs

ci / web (pull_request) Successful in 28s

Details

ci / rust (pull_request) Successful in 1m49s

Details

ci / web (push) Successful in 39s

Details

ci / rust (push) Successful in 2m48s

Details

6596104789

maestro-cli/src/config.rs was two pass-throughs, a newtype error, a
two-field projection of MaestroConfigFile, and one four-line or_else with
a single caller. `resolve_daemon_url` moves onto MaestroConfigFile, next
to the schema it reads, and the module goes; its tests move with it.

`[web].dev_proxy_target` is deleted end to end — schema, env var,
normalisation, three tests, README line, and the startup warning that
admitted the server ignores it.

MaestroConfigFile::save had no callers outside its own tests. Deleting it
also removes every skip_serializing_if, the five is_empty impls, and the
Serialize derives that existed only to make its output tidy.

BroadcastTuning was a struct that was never instantiated: all three uses
were `::DEFAULT.field` in const position and there is no [broadcast]
section. Now two plain consts.

$MAESTRO_CONFIG was honoured twice, by resolve_config_path and by clap's
`env =`. Every binary already gets it through clap, so resolve_config_path
and CONFIG_PATH_ENV go and the TUI falls back to default_config_path like
everything else.

Also collapses three layers of 1:1 forwarding: the umbrella's cli_dispatch
shim, and four near-identical 8-line functions in cli/dispatch.rs that
differed only in which run_* they called.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

naps62-yolo merged commit 6596104789 into main

2026-08-22 14:41:48 +01:00

naps62-yolo deleted branch lithuanians

2026-08-22 14:41:48 +01:00

This repo is archived. You cannot comment on pull requests.

1 Participants

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: naps62/maestro#46