From 43345e97e23d5441936b7a919aa3338fa196ba75 Mon Sep 17 00:00:00 2001 From: valentin Date: Fri, 31 Jul 2026 10:33:45 +0200 Subject: [PATCH 1/2] fix(cli): config set writes the value instead of printing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin shim's `config` case forwarded everything to `cmd_config show` as trailing arguments, which ignores them. So `temper config set auto_nudge_lines 300` printed the config and exited 0 having set nothing, and `cairn config reset local` did the same. A silent success, in the CLI of a tool whose whole argument is that silent successes are the dangerous failure. The shim now forwards the subcommand and namespaces the key by looking up the section the manifest schema declares for it — which is the point of having a shim: you should not need to know that `trailers` lives in [git] rather than [cairn]. temper config set auto_nudge_lines 300 → [temper] auto_nudge_lines cairn config set trailers Signed-off-by → [git] trailers cairn config set pr.base develop → [cairn] pr.base, dots intact Prefixing is also what makes dotted keys work: _split_key splits on the first dot, so a bare `pr.base` would resolve to a section called `pr`. A key given already qualified is passed through, or `temper config set temper.auto_nudge_lines` would write temper.temper.auto_nudge_lines. get, unset and explain are namespaced the same way. doctor, path and edit pass through — they answer for the whole file, not a section. An unrecognised subcommand exits 1 and names the real ones. Trust re-hashing, scope parsing and the unknown-key warning all come free, because they live in cmd_config, which is now actually reached. Also: the `trailers` doc string suggested Co-Authored-By, and that text gets written into every config file the tool generates. 16 new assertions in tests/test_cli.sh, the regression first. --- CHANGELOG.md | 26 +++++++++++++++ README.md | 9 ++++++ bin/aether | 56 +++++++++++++++++++++++++++++++- plugins/cairn/README.md | 17 ++++++---- plugins/cairn/aether.plugin | 2 +- plugins/temper/README.md | 12 +++++-- plugins/whetstone/README.md | 7 ++-- tests/test_cli.sh | 64 +++++++++++++++++++++++++++++++++++++ 8 files changed, 181 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 46aaff8..c3f308d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -64,6 +64,32 @@ number. Their individual histories are preserved under and signposts the config layer, which was useful with zero gates installed and started at line 340 of 878. +## [Unreleased] + +### Fixed + +- **` config set` printed the config and changed nothing.** The plugin shim's + `config` case forwarded everything to `cmd_config show` as trailing arguments, which + ignores them — so `temper config set auto_nudge_lines 300` exited 0 having set + nothing, and `cairn config reset local` did the same. That is the *reports success, + did nothing* failure mode, in the CLI of a tool built to catch it. + + The shim now forwards the subcommand and resolves the key's section from the manifest + schema, which is the point of having a shim at all: + + ```bash + temper config set auto_nudge_lines 300 # → [temper] auto_nudge_lines + cairn config set trailers Signed-off-by # → [git] trailers — cairn declares it there + cairn config set pr.base develop # → [cairn] pr.base, dots intact + ``` + + `get`, `unset` and `explain` are namespaced the same way; `doctor`, `path` and `edit` + pass through unchanged; an already-qualified key is not prefixed twice; and an + unrecognised subcommand now exits 1 naming the real ones instead of silently showing. + +- The `trailers` doc string suggested `Co-Authored-By`, and that text is written into + every config file the tool generates. It now suggests `Signed-off-by`. + ## [1.5.0] — 2026-07-31 ### Changed diff --git a/README.md b/README.md index da6623e..b9a229a 100644 --- a/README.md +++ b/README.md @@ -719,6 +719,15 @@ Every plugin answers to its own name — `aether cairn status` — and the `cair `temper`, `whetstone` and `bonsai` binaries are 24-line shims that exec exactly that, so there is one implementation. +A plugin's `config` subcommand takes the same verbs as `aether config` and resolves the +section from the schema, so you do not have to know which one a key lives in — including +the keys cairn declares under `[git]`: + +``` +cairn config set trailers Signed-off-by ≡ aether config set git.trailers Signed-off-by +temper config set auto_nudge_lines 300 ≡ aether config set temper.auto_nudge_lines 300 +``` + `enable`/`disable` writes `enabled: false` for the gate to read; `hook enable`/`disable` stops the gate being loaded. The soft mute is usually what you want. diff --git a/bin/aether b/bin/aether index edd6a29..83d38cb 100755 --- a/bin/aether +++ b/bin/aether @@ -1435,6 +1435,26 @@ _cfg_origin() { printf 'default||' } +# _plugin_section_for -> the [section] that plugin declares under +# +# Not always the plugin's own section: cairn declares `trailers` under [git], because a +# house rule about commit text is not a cairn setting. The schema already records that +# (`config..section:`), so this is a lookup rather than a second place to encode it. +_plugin_section_for() { + local s + s=$(_schema_all | awk -F'|' -v p="$1" -v k="$2" '$3 == p && $2 == k { print $1; exit }') + [ -n "$s" ] && { printf '%s' "$s"; return 0; } + # Unknown key: fall back to the plugin's own section so `cmd_config set` reaches its + # "no plugin declares this" warning instead of us inventing a section here. + s=$(_mf "$1" config_section); printf '%s' "${s:-$1}" +} + +# _is_known_section -> 0 when some manifest declares keys under it +_is_known_section() { + [ -n "$1" ] || return 1 + _schema_all | awk -F'|' -v s="$1" '$1 == s { found = 1 } END { exit !found }' +} + _split_key() { # temper.auto_nudge_lines -> "temper auto_nudge_lines" case "$1" in *.*) printf '%s %s' "${1%%.*}" "${1#*.}" ;; @@ -2976,7 +2996,41 @@ case "$COMMAND" in # `aether uninstall` and the three plugin updates before it. update) cmd_install "$_PLUGIN" "$(_manifest_get scope)" "$@" ;; hook) cmd_hook "$@" "$_PLUGIN" ;; - config) cmd_config show "$(_mf "$_PLUGIN" config_section || printf '%s' "$_PLUGIN")" "$@" ;; + # `temper config set auto_nudge_lines 300` used to reach `cmd_config show` with + # the rest as trailing arguments, which ignores them — so it printed the config + # and exited 0 having changed nothing. Forward the subcommand instead, and + # namespace the key, since the shim exists precisely so you do not have to know + # which section a key lives in. + config) + _csub="${1:-show}" + case "$_csub" in + show|'') + shift 2>/dev/null || true + cmd_config show "$(_mf "$_PLUGIN" config_section || printf '%s' "$_PLUGIN")" "$@" ;; + doctor|path|edit) + # Not section-scoped — these answer for the whole config file. + shift; cmd_config "$_csub" "$@" ;; + get|explain|set|unset) + shift + _ckey="${1:-}" + if [ -z "$_ckey" ]; then + printf 'Usage: %s config %s %s\n' "$_PLUGIN" "$_csub" \ + "$([ "$_csub" = set ] && printf ' ' )[global]" + exit 1 + fi + shift + # A key given fully qualified is passed through: prefixing again would + # write temper.temper.auto_nudge_lines. + if _is_known_section "$(_split_key "$_ckey" | cut -d' ' -f1)"; then + cmd_config "$_csub" "$_ckey" "$@" + else + cmd_config "$_csub" "$(_plugin_section_for "$_PLUGIN" "$_ckey").$_ckey" "$@" + fi ;; + *) + printf 'Unknown %s config subcommand: %s\n\n' "$_PLUGIN" "$_csub" + printf 'Try: show, get, set, unset, explain, doctor, path, edit\n' + exit 1 ;; + esac ;; help|--help|-h) usage ;; *) printf 'Unknown %s subcommand: %s\n' "$_PLUGIN" "$sub"; exit 1 ;; esac diff --git a/plugins/cairn/README.md b/plugins/cairn/README.md index 8429da6..1784429 100644 --- a/plugins/cairn/README.md +++ b/plugins/cairn/README.md @@ -306,6 +306,8 @@ A global install also provides a `cairn` command for managing your setup: ```bash cairn status # install state + resolved config cairn config # show the resolved [cairn] section, with sources +cairn config set style plain # set a key, section resolved for you +cairn config get|unset style # read or remove one cairn disable local # silence /draft-commit for this project cairn disable global # silence everywhere @@ -315,15 +317,18 @@ cairn update # reinstall cairn from the clone cairn uninstall global --claude-md # full removal ``` -`cairn` is a shim that execs `aether cairn …`. **Changing a value is `aether config set -
. `** — namespaced, space-separated, no `=`. The plugin shim's -`config` only shows: +`cairn` is a shim that execs `aether cairn …`. Values are space-separated, never `=`. +`cairn config set` resolves the section from the schema — including the `[git]` keys, +which are cairn's to declare but do not live in `[cairn]`: ```bash -aether config set cairn.style plain -aether config set cairn.pr.base develop +cairn config set style plain # → [cairn] style +cairn config set trailers Signed-off-by # → [git] trailers (resolved from the schema) +cairn config set pr.base develop # → [cairn] pr.base +cairn config unset summary.window + +aether config set cairn.style plain # the explicit form, always available aether config set git.ticket 'TK-[0-9]+' -aether config unset cairn.summary.window ``` Run `aether help` for the full reference. diff --git a/plugins/cairn/aether.plugin b/plugins/cairn/aether.plugin index 4d3d038..3da2db6 100644 --- a/plugins/cairn/aether.plugin +++ b/plugins/cairn/aether.plugin @@ -118,7 +118,7 @@ config.ticket.used_by: /draft-commit /draft-pr /critique-pr config.trailers.section: git config.trailers.default: -config.trailers.doc: Trailers every commit message must carry, e.g. Co-Authored-By +config.trailers.doc: Trailers every commit message must carry, e.g. Signed-off-by config.trailers.used_by: /draft-commit config.base.section: git diff --git a/plugins/temper/README.md b/plugins/temper/README.md index 420c1f8..e84a591 100644 --- a/plugins/temper/README.md +++ b/plugins/temper/README.md @@ -189,13 +189,21 @@ temper status Install state and resolved config temper enable [local|global] Enable temper temper disable [local|global] Disable temper temper config Show the resolved [temper] section +temper config set Set a key, without naming the section +temper config get|unset Read or remove one temper update Reinstall temper from the clone temper uninstall [global] [--claude-md] ``` `temper` is a shim that execs `aether temper …`, so `aether temper status` is the same -command. **Changing a value is `aether config set`**, not `temper config set` — the -plugin shim's `config` only shows. +command. `temper config set` resolves the section for you from the schema, which is the +point of the shim: + +```bash +temper config set auto_nudge_lines 300 # → [temper] auto_nudge_lines +temper config set auto_nudge_lines 300 global # → ~/.aether/config +aether config set temper.auto_nudge_lines 300 # the explicit form, always available +``` --- diff --git a/plugins/whetstone/README.md b/plugins/whetstone/README.md index ef8e648..5ede2d1 100644 --- a/plugins/whetstone/README.md +++ b/plugins/whetstone/README.md @@ -209,6 +209,8 @@ A global install also provides a `whetstone` command for managing your setup: ```bash whetstone status # install state + resolved config whetstone config # show the resolved [whetstone] section +whetstone config set severity red # set a key, section resolved for you +whetstone config get|unset severity # read or remove one whetstone disable local # silence for this project whetstone disable global # silence everywhere @@ -218,8 +220,9 @@ whetstone update # reinstall whetstone from the clone whetstone uninstall global --claude-md # full removal ``` -`whetstone` is a shim that execs `aether whetstone …`. **Changing a value is `aether -config set `** — the plugin shim's `config` only shows. +`whetstone` is a shim that execs `aether whetstone …`. `whetstone config set` resolves +the section from the schema; `aether config set whetstone. ` is the explicit +form and always works. Append `global` to either to write `~/.aether/config`. Run `aether help` for the full reference. diff --git a/tests/test_cli.sh b/tests/test_cli.sh index 127fd1a..98b964b 100644 --- a/tests/test_cli.sh +++ b/tests/test_cli.sh @@ -29,6 +29,70 @@ assert_contains "$out" "aether version" "help documents version" out=$(bash "$CLI" nonsense 2>&1); e=$? assert_exit 1 "$e" "unknown subcommand exits 1" +# ── plugin config subcommands ──────────────────────────────────────────────── +# `temper config set auto_nudge_lines 300` used to reach `cmd_config show` with the rest +# as trailing arguments, which ignores them: it printed the config and exited 0 having +# changed nothing. The regression assertion is the first one, because a silent success is +# the failure mode this project exists to catch, and it was in its own CLI. +suite "plugin config subcommands" + +PC_HOME=$(new_home) +PC_DIR=$(mktemp -d); FAKE_HOMES+=("$PC_DIR") +( cd "$PC_DIR" && git init -q . && mkdir -p .aether ) + +pc() { ( cd "$PC_DIR" && env HOME="$PC_HOME" AETHER_REPO="$REPO" bash "$CLI" "$@" 2>&1 ); } +pc_cfg() { cat "$PC_DIR/.aether/config" 2>/dev/null; } + +out=$(pc temper config set auto_nudge_lines 300); e=$? +assert_exit 0 "$e" "temper config set exits 0" +assert_contains "$(pc_cfg)" "auto_nudge_lines: 300" "…and actually writes the value" +assert_contains "$out" "temper.auto_nudge_lines" "…naming the resolved section.key" + +# cairn declares `trailers` under [git], not [cairn] — the shim has to read the schema +# rather than assume the plugin's own section. +pc cairn config set trailers Signed-off-by >/dev/null +assert_contains "$(pc_cfg)" "[git]" "a key declared under [git] lands in [git]" + +# `_split_key` splits on the first dot, so a bare `pr.base` would resolve to section +# `pr`. Prefixing is what makes dotted keys work. +pc cairn config set pr.base develop >/dev/null +assert_contains "$(pc_cfg)" "pr.base: develop" "a dotted key keeps its dots" +case "$(pc_cfg)" in + *"[pr]"*) fail "a dotted key does not invent a [pr] section" "found [pr]" ;; + *) pass "a dotted key does not invent a [pr] section" ;; +esac + +# Already qualified: prefixing again would write temper.temper.auto_nudge_lines. +pc temper config set temper.auto_nudge_lines 400 >/dev/null +case "$(pc_cfg)" in + *temper.temper*) fail "an already-qualified key is not prefixed twice" "found temper.temper" ;; + *) pass "an already-qualified key is not prefixed twice" ;; +esac +assert_eq "400" "$(pc temper config get auto_nudge_lines)" "get reads the value back" + +pc temper config unset auto_nudge_lines >/dev/null +case "$(pc_cfg)" in + *auto_nudge_lines*) fail "unset removes the key" "still present" ;; + *) pass "unset removes the key" ;; +esac + +# Scope still resolves through cmd_config, which the shim now actually reaches. +pc temper config set auto_nudge_lines 500 global >/dev/null +assert_contains "$(cat "$PC_HOME/.aether/config" 2>/dev/null)" "auto_nudge_lines: 500" \ + "the global scope argument still works" + +out=$(pc temper config); e=$? +assert_exit 0 "$e" 'bare temper config still shows' +assert_contains "$out" "[temper]" "…the plugin's own section" + +out=$(pc temper config bogus); e=$? +assert_exit 1 "$e" "an unknown config subcommand exits 1" +assert_contains "$out" "Try: show" "…and names the real ones" + +out=$(pc temper config set); e=$? +assert_exit 1 "$e" "set with no key exits 1" +assert_contains "$out" "Usage" "…with a usage line" + # ── status against a real install ──────────────────────────────────────────── suite "status" H=$(new_home) From fd53469a19c20820f8e72a803c4a4cfc3e688c64 Mon Sep 17 00:00:00 2001 From: valentin Date: Fri, 31 Jul 2026 10:45:30 +0200 Subject: [PATCH 2/2] fix(cli): no early awk exit in the section lookup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _plugin_section_for piped _schema_all into awk with `exit` after the first match, closing the pipe while the writer was still going. Bash on Linux reports the EPIPE as `printf: write error: Broken pipe` into whatever the caller captured, so `temper config get` returned the value with an error message glued to it. This is the same bug the file already carries a warning about, three lines above the function I added — "No `exit` in the awk". I wrote one anyway. _schema_plugin_for's `!f { print; f = 1 }` is the pattern; use it. Six stderr assertions added, following the precedent in test_config.sh: macOS dies from SIGPIPE silently, so a value comparison does not reliably catch this and it has to be asserted directly. That is why it passed locally and failed on both CI platforms. --- bin/aether | 6 +++++- tests/test_cli.sh | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/bin/aether b/bin/aether index 83d38cb..8ff0dd1 100755 --- a/bin/aether +++ b/bin/aether @@ -1441,8 +1441,11 @@ _cfg_origin() { # house rule about commit text is not a cairn setting. The schema already records that # (`config..section:`), so this is a lookup rather than a second place to encode it. _plugin_section_for() { + # No `exit` in the awk — see the note under _schema_plugin_for. Exiting early closes + # the pipe while _schema_all is still writing, and bash on Linux reports the EPIPE as + # `printf: write error: Broken pipe` into whatever the caller captured. local s - s=$(_schema_all | awk -F'|' -v p="$1" -v k="$2" '$3 == p && $2 == k { print $1; exit }') + s=$(_schema_all | awk -F'|' -v p="$1" -v k="$2" '$3 == p && $2 == k && !f { print $1; f = 1 }') [ -n "$s" ] && { printf '%s' "$s"; return 0; } # Unknown key: fall back to the plugin's own section so `cmd_config set` reaches its # "no plugin declares this" warning instead of us inventing a section here. @@ -1450,6 +1453,7 @@ _plugin_section_for() { } # _is_known_section -> 0 when some manifest declares keys under it +# The `exit` here is in END, after all input is read, so it cannot close the pipe early. _is_known_section() { [ -n "$1" ] || return 1 _schema_all | awk -F'|' -v s="$1" '$1 == s { found = 1 } END { exit !found }' diff --git a/tests/test_cli.sh b/tests/test_cli.sh index 98b964b..d8e38ed 100644 --- a/tests/test_cli.sh +++ b/tests/test_cli.sh @@ -93,6 +93,20 @@ out=$(pc temper config set); e=$? assert_exit 1 "$e" "set with no key exits 1" assert_contains "$out" "Usage" "…with a usage line" +# The shim's section lookup pipes _schema_all into awk, which is the shape that +# produced `printf: write error: Broken pipe` once before: an `exit` in the awk closes +# the pipe mid-write and bash on Linux reports the EPIPE into whatever the caller +# captured. macOS dies from SIGPIPE silently, so this must be asserted directly rather +# than left for a value comparison to notice — it passed locally and failed in CI. +for c in "temper config set auto_nudge_lines 300" "temper config get auto_nudge_lines" \ + "temper config unset auto_nudge_lines" "cairn config set trailers Signed-off-by" \ + "cairn config explain pr.base" "temper config"; do + # shellcheck disable=SC2086 + err=$( cd "$PC_DIR" && env HOME="$PC_HOME" AETHER_REPO="$REPO" bash "$CLI" $c 2>&1 >/dev/null ) + if [ -z "$err" ]; then pass "no stderr from: $c" + else fail "no stderr from: $c" "${err%%$'\n'*}"; fi +done + # ── status against a real install ──────────────────────────────────────────── suite "status" H=$(new_home)