diff --git a/src/cli/cli.c b/src/cli/cli.c index 010719a64..f85a55f25 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -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. diff --git a/src/cli/client_adapter.c b/src/cli/client_adapter.c index c6828ea12..be00cdb33 100644 --- a/src/cli/client_adapter.c +++ b/src/cli/client_adapter.c @@ -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]) { diff --git a/tests/test_agent_clients.c b/tests/test_agent_clients.c index 3c164b5cc..edfc3a803 100644 --- a/tests/test_agent_clients.c +++ b/tests/test_agent_clients.c @@ -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); diff --git a/tests/test_cli.c b/tests/test_cli.c index 9baf9eae5..a6628f667 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -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);