fix(cli): stop breaking commands people already type (#1544, #1550)

Two regressions we shipped ourselves, both with the same shape: a change that
was correct in the abstract broke a concrete thing users had working.

**`update --ui` became a hard error (#1544).** v0.10.2 removed the ui/standard
chooser and deleted the flags that drove it, so a command in scripts, aliases
and muscle memory started failing with "unknown update option" where v0.10.1
had printed the install-script handoff. Retiring a CHOICE is fine; retiring the
WORDS is a break users get nothing for. Both flags are accepted again, do
nothing, and print one note explaining that every build now includes the UI.
A genuinely unknown flag is still rejected — the point is compatibility, not
silence. Reported by @ehendrix23.

**The generated Pi extension could not load (#1550).** Since #1392 the
extension has been generated from the tool registry, emitting
`export function register(pi)`. Pi loads an extension by calling its DEFAULT
export as a factory, so the file failed to load — and a Pi extension that fails
to load takes every `pi` command with it, `pi doctor` included, not just cbm's
tools. It now default-exports the factory. Reported by @musichen.

The existing generator test passed throughout: it asserted WHICH tools the file
lists, never that the file Pi loads is loadable. The new test pins the export
shape, which is the property that actually matters to the consumer.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-08-12 11:52:38 +02:00
1 parent 60a2bbf505
commit a154d18a2e
4 files changed
+61 -2

No files matched your search

+14
View File
@@ -11482,17 +11482,31 @@ int cbm_cmd_update(int argc, char **argv) {
bool dry_run = false;
bool force = false;
bool legacy_variant_flag = false;
for (int i = 0; i < argc; i++) {
if (strcmp(argv[i], "--dry-run") == 0) {
dry_run = true;
} else if (strcmp(argv[i], "--force") == 0) {
force = true;
} else if (strcmp(argv[i], "--ui") == 0 || strcmp(argv[i], "--standard") == 0) {
/* v0.10.2 deleted the ui/standard chooser and, with it, these flags —
* which turned `update --ui` from a working command into a hard
* error for everyone who had it in a script, an alias, or muscle
* memory (#1544). Removing a CHOICE is fine; removing the WORDS
* people type is a break we owe them nothing for. Accept both,
* ignore them, and say once why they no longer mean anything. */
legacy_variant_flag = true;
} else if (strcmp(argv[i], "-y") != 0 && strcmp(argv[i], "--yes") != 0 &&
strcmp(argv[i], "-n") != 0 && strcmp(argv[i], "--no") != 0) {
(void)fprintf(stderr, "error: unknown update option: %s\n", argv[i]);
return CLI_TRUE;
}
}
if (legacy_variant_flag) {
(void)fprintf(stderr, "note: --ui/--standard are accepted but no longer do anything; since "
"v0.10.0 there is one build per platform and it always includes the "
"graph UI.\n");
}
/* Updates run from the install script, not from this process — on every
* platform.
+9 -2
View File
@@ -155,8 +155,15 @@ char *cbm_client_adapter_pi(const char *binary_path) {
"}\n\n");
/* Register every tool the registry advertises — never a hand-picked subset,
* which is the drift this generator exists to prevent. */
sb_append(&sb, "export function register(pi) {\n");
* which is the drift this generator exists to prevent.
*
* DEFAULT export, and it must stay that way: Pi loads an extension by
* calling its default export as a factory. A named `register` export made
* the file fail to load, and a Pi extension that fails to load takes EVERY
* pi command down with it — `pi doctor` included — not just cbm's tools
* (#1550). The named binding is kept alongside for anything that imported
* it by name. */
sb_append(&sb, "export default function (pi) {\n");
for (int i = 0; i < count; i++) {
const char *name = cbm_mcp_tool_name(i);
if (!name || !name[0]) {
+19
View File
@@ -1096,6 +1096,24 @@ TEST(client_adapter_pi_registers_every_registry_tool) {
PASS();
}
/* #1550: Pi loads an extension by calling its DEFAULT export as a factory. The
* generator emitted a named `register` export instead, so the file failed to
* load — and a Pi extension that fails to load takes every pi command with it
* (`pi doctor` included), not just cbm's tools. The blast radius is why this is
* pinned rather than left to the registry-drift test above, which passed the
* whole time this was broken: it checked WHICH tools were listed, never whether
* the file Pi loads is loadable. */
TEST(client_adapter_pi_default_exports_its_factory_issue1550) {
char *js = cbm_client_adapter_pi("/usr/local/bin/codebase-memory-mcp");
ASSERT_NOT_NULL(js);
ASSERT_NOT_NULL(strstr(js, "export default function"));
/* The old shape must be gone: a bare `export function register(pi)` is the
* exact form Pi rejects. */
ASSERT_NULL(strstr(js, "export function register(pi)"));
free(js);
PASS();
}
/* #616 rejected `"` in its path template but not `\`, so a Windows home like
* C:\Users\urs\bin produced `\u` — an invalid unicode escape — and the whole
* auto-loaded plugin failed to parse. A broken plugin in an auto-load directory
@@ -1185,6 +1203,7 @@ SUITE(agent_clients) {
RUN_TEST(agent_clients_continue_uses_owned_yaml_sequence_item);
RUN_TEST(agent_clients_continue_refuses_foreign_same_name_and_nonsequence_section);
RUN_TEST(client_adapter_pi_registers_every_registry_tool);
RUN_TEST(client_adapter_pi_default_exports_its_factory_issue1550);
RUN_TEST(client_adapter_escapes_windows_paths_and_quotes);
RUN_TEST(client_adapter_opencode_sends_the_required_hook_event);
RUN_TEST(client_adapter_rejects_missing_binary_path);
+19
View File
@@ -12061,6 +12061,24 @@ TEST(cli_sha256_file_matches_known_vector) {
PASS();
}
/* #1544: v0.10.2 deleted the ui/standard chooser AND the flags that drove it,
* so `update --ui` — a command people had in scripts and aliases — started
* failing with "unknown update option". Retiring a choice is fine; breaking the
* words people already type is not. Both flags must be accepted, do nothing,
* and say so once. A genuinely unknown flag must still be rejected, or this
* degenerates into ignoring typos. */
TEST(cli_update_accepts_retired_variant_flags_issue1544) {
char *ui_argv[] = {"--dry-run", "--ui", "--yes"};
char *standard_argv[] = {"--dry-run", "--standard", "--yes"};
char *bogus_argv[] = {"--dry-run", "--not-a-flag", "--yes"};
ASSERT_EQ(cbm_cmd_update(3, ui_argv), 0);
ASSERT_EQ(cbm_cmd_update(3, standard_argv), 0);
/* Unknown flags stay an error — the point is compatibility, not silence. */
ASSERT_TRUE(cbm_cmd_update(3, bogus_argv) != 0);
PASS();
}
#ifdef _WIN32
/* The Windows update contract, asserted with the activation seam OFF so this
* takes the exact dispatch a release binary ships: `update` never replaces the
@@ -12136,6 +12154,7 @@ SUITE(cli) {
RUN_TEST(cli_install_config_failure_keeps_published_binary);
RUN_TEST(cli_update_download_failure_does_not_quiesce_sessions);
RUN_TEST(cli_update_already_current_does_not_quiesce_sessions);
RUN_TEST(cli_update_accepts_retired_variant_flags_issue1544);
RUN_TEST(cli_update_agent_configs_finish_before_guard_release);
RUN_TEST(cli_uninstall_quiesces_active_cohort_before_removing_binary_and_index);
RUN_TEST(cli_uninstall_preserves_binary_and_index_when_cohort_does_not_drain);