From 67af19e83baaef31ba64c7e7dcae1bf671093fdc Mon Sep 17 00:00:00 2001 From: Andy11-cpu Date: Thu, 27 Aug 2026 13:53:22 -0400 Subject: [PATCH 01/19] feat(cli): persist install hook opt-out Signed-off-by: Andy11-cpu --- README.md | 2 + docs/CONFIGURATION.md | 4 + scripts/smoke-test.sh | 15 +++ src/cli/cli.c | 250 ++++++++++++++++++++++++++++++++++++++---- src/cli/cli.h | 1 + src/main.c | 2 +- tests/test_cli.c | 233 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 482 insertions(+), 25 deletions(-) diff --git a/README.md b/README.md index dfd969044..8d4a3b092 100644 --- a/README.md +++ b/README.md @@ -109,6 +109,8 @@ The `install` command automatically strips macOS quarantine attributes and ad-ho The `install` command auto-detects installed coding agents and configures their documented MCP entries plus durable instructions, skills, and lifecycle hooks where supported. +Hooks are enabled by default. Run `codebase-memory-mcp install --no-hooks` to remove CBM-managed hooks while keeping MCP registration and other integration files; the choice persists for later installs and updates. Run `codebase-memory-mcp install --hooks` to re-enable them, or inspect the current preference with `codebase-memory-mcp config get install_hooks`. + ### Session Coordination Daemon CBM automatically shares one per-account coordination daemon across Claude Code, Codex, OpenCode, and every other configured client. There is no opt-in setting for MCP servers or hook clients: the first daemon-backed CBM session starts it, each session registers its own work, and the final session shuts it down. The daemon owns long-lived background services such as watchers, shared indexing jobs, and the optional UI. Closing one session cancels work owned only by that session, while work still needed by another session continues. diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index fe9c24a7b..d795fed9d 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -79,6 +79,7 @@ codebase-memory-mcp config set auto_index true codebase-memory-mcp config set auto_index_limit 50000 codebase-memory-mcp config set watcher_enabled false codebase-memory-mcp config reset auto_index +codebase-memory-mcp config get install_hooks ``` Current keys: @@ -89,6 +90,9 @@ Current keys: | `auto_index_limit` | `50000` | Maximum file count allowed for automatic indexing of a new project. | | `auto_watch` | `true` | Register the session's project with the background git watcher on connect. Set `false` to keep a session from registering its project (the watcher still runs for other projects). | | `watcher_enabled` | `true` | Master switch for the background watcher subsystem. Set `false` to stop the watcher from starting at all — no poll thread and no project registration. Reindex manually with `index_repository` when disabled. | +| `install_hooks` | `true` | Install CBM-managed client hooks. `install --no-hooks` persists `false`; `install --hooks` restores `true`. | + +Bare installs and updates respect the stored hook preference. MCP registration, skills, instructions, and profiles remain installed when hooks are disabled. > **`watcher_enabled` vs `auto_watch`.** `watcher_enabled` controls whether the > watcher *subsystem* starts at all (the background poll thread). `auto_watch` is diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index fb7fe3dd3..5fd4c84f5 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -1691,6 +1691,21 @@ if ! grep -q 'hook-augment' "$GATE_SCRIPT"; then fi echo "OK 8e: shim installed, non-blocking, delegates to hook-augment" +HOME="$FAKE_HOME" XDG_CONFIG_HOME="$FAKE_HOME/.config" APPDATA="$FAKE_HOME/AppData/Roaming" LOCALAPPDATA="$FAKE_HOME/AppData/Local" PATH="$FAKE_HOME/.local/bin:$PATH" \ + "$BINARY" install --no-hooks --skip-binary --clients=claude -y >/dev/null +if grep -qE 'cbm-(code-discovery-gate|session-reminder|subagent-reminder)' "$FAKE_HOME/.claude/settings.json" 2>/dev/null || + ! grep -q 'codebase-memory-mcp' "$FAKE_HOME/.claude.json" 2>/dev/null; then + echo "FAIL 8e-i: --no-hooks did not remove owned Claude hooks while preserving MCP" + exit 1 +fi +HOME="$FAKE_HOME" XDG_CONFIG_HOME="$FAKE_HOME/.config" APPDATA="$FAKE_HOME/AppData/Roaming" LOCALAPPDATA="$FAKE_HOME/AppData/Local" PATH="$FAKE_HOME/.local/bin:$PATH" \ + "$BINARY" install --hooks --skip-binary --clients=claude -y >/dev/null +if ! grep -q 'cbm-code-discovery-gate' "$FAKE_HOME/.claude/settings.json" 2>/dev/null; then + echo "FAIL 8e-i: --hooks did not restore owned Claude hooks" + exit 1 +fi +echo "OK 8e-i: Claude hook opt-out and re-enable preserve MCP" + # 8f-8h: Codex TOML if ! grep -q '\[mcp_servers.codebase-memory-mcp\]' "$FAKE_HOME/.codex/config.toml"; then echo "FAIL 8f: Codex TOML missing MCP section" diff --git a/src/cli/cli.c b/src/cli/cli.c index 56af212d1..cd1837260 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -6841,6 +6841,7 @@ static const config_key_def_t CONFIG_KEYS[] = { {CBM_CONFIG_AUTO_WATCH, "true", "Register background git watcher on session connect"}, {CBM_CONFIG_WATCHER_ENABLED, "true", "Run the background watcher thread (auto-reindex); false to disable"}, + {CBM_CONFIG_INSTALL_HOOKS, "true", "Install managed client hooks"}, {CBM_CONFIG_UI_LANG, "auto", "Pin graph UI language: en, zh, or auto"}, {CBM_CONFIG_UI_ENABLED, "false", "Serve the graph UI on a loopback HTTP port"}, {CBM_CONFIG_UI_PORT, "9749", "Port for the graph UI listener when enabled"}, @@ -7535,11 +7536,19 @@ typedef struct { static cbm_install_plan_t *g_install_plan = NULL; static int g_agent_install_errors = 0; static int g_agent_uninstall_errors = 0; +static bool g_install_hooks = true; + +static void print_hooks_skipped(void) { + printf(" hooks: skipped (install --hooks to enable)\n"); +} static void plan_record(const char *agent, const char *kind, const char *path) { if (!g_install_plan || !path || !path[0]) { return; } + if (!g_install_hooks && strcmp(kind, "hook") == 0) { + return; + } cbm_install_plan_t *pl = g_install_plan; if (pl->count >= pl->cap) { int ncap = pl->cap ? pl->cap * 2 : CLI_BUF_16; @@ -7732,6 +7741,21 @@ static void install_claude_code_config(const char *home, const char *binary_path char settings_path[CLI_BUF_1K]; snprintf(settings_path, sizeof(settings_path), "%s/settings.json", config_dir); + if (!g_install_hooks) { + if (!dry_run) { + if (cbm_remove_claude_hooks(settings_path) != CLI_OK) { + record_agent_config_error(false, "Claude Code", "hook_disable", settings_path); + } + if (cbm_remove_session_hooks(settings_path) != CLI_OK) { + record_agent_config_error(false, "Claude Code", "hook_disable", settings_path); + } + if (cbm_remove_claude_subagent_hooks(settings_path) != CLI_OK) { + record_agent_config_error(false, "Claude Code", "hook_disable", settings_path); + } + } + print_hooks_skipped(); + goto claude_hooks_done; + } bool gate_ok = dry_run; bool session_ok = dry_run; bool subagent_ok = dry_run; @@ -7810,6 +7834,7 @@ static void install_claude_code_config(const char *home, const char *binary_path } } +claude_hooks_done: /* Migration nudge: when CLAUDE_CONFIG_DIR is set and a legacy ~/.claude tree * still exists, mention it so users can clean up stale artifacts. */ if (home && home[0]) { @@ -8171,6 +8196,14 @@ static void install_copilot_durable_context(const char *home, const char *binary plan_record("Copilot", "hook", hook_path); return; } + if (!g_install_hooks) { + if (!dry_run && cbm_file_exists(hook_path) && + cbm_remove_copilot_hooks(hook_path, binary_path) != CLI_OK) { + record_agent_config_error(false, "Copilot", "hook_disable", hook_path); + } + print_hooks_skipped(); + return; + } bool hook_ok = true; if (!dry_run && (!cbm_mkdir_p(hooks_dir, CLI_OCTAL_PERM) || cbm_upsert_copilot_hooks(binary_path, hook_path) != CLI_OK)) { @@ -8343,6 +8376,14 @@ static void install_qoder_durable_context(const char *home, const char *binary_p } return; } + if (!g_install_hooks) { + if (!dry_run && config_resolved && cbm_file_exists(settings_path) && + cbm_remove_qoder_context_hook(settings_path, binary_path) != CLI_OK) { + record_agent_config_error(false, "Qoder CLI", "hook_disable", settings_path); + } + print_hooks_skipped(); + return; + } if (!hook_supported) { printf(" hooks: withheld because no documented executor is available\n"); return; @@ -8402,6 +8443,14 @@ static void install_gitlab_durable_context(const cbm_agent_registry_context_t *r } return; } + if (!g_install_hooks) { + if (!dry_run && cbm_file_exists(hooks_path) && + cbm_remove_gitlab_session_hook(hooks_path, binary_path) != CLI_OK) { + record_agent_config_error(false, "GitLab Duo CLI", "hook_disable", hooks_path); + } + print_hooks_skipped(); + return; + } if (!hook_supported) { printf(" hook: withheld on Windows (vendor hook shell is undocumented)\n"); return; @@ -8439,6 +8488,14 @@ static void install_devin_durable_context(const cbm_agent_registry_context_t *re } return; } + if (!g_install_hooks) { + if (!dry_run && config_resolved && cbm_file_exists(config_path) && + cbm_remove_devin_context_hooks(config_path, binary_path) != CLI_OK) { + record_agent_config_error(false, "Devin CLI / Local", "hook_disable", config_path); + } + print_hooks_skipped(); + return; + } if (!hook_supported) { printf(" hooks: withheld on Windows (vendor hook shell is undocumented)\n"); return; @@ -8547,6 +8604,14 @@ static void install_kimi_durable_context(const cbm_agent_registry_context_t *reg plan_record("Kimi Code CLI", "hook", config_path); return; } + if (!g_install_hooks) { + if (!dry_run && cbm_file_exists(config_path) && + cbm_remove_kimi_context_hook(config_path) != CLI_OK) { + record_agent_config_error(false, "Kimi Code CLI", "hook_disable", config_path); + } + print_hooks_skipped(); + return; + } bool installed = true; if (!dry_run && (!prepare_config_parent(config_path) || cbm_upsert_kimi_context_hook(config_path, binary_path) != CLI_OK)) { @@ -8763,6 +8828,19 @@ static void install_gemini_config(const char *home, const char *binary_path, boo plan_record("Gemini CLI", "hook", cp); /* BeforeTool + SessionStart in settings.json */ return; } + if (!g_install_hooks) { + if (!dry_run) { + int tool_result = cbm_remove_gemini_hooks(cp); + int coverage_result = cbm_remove_gemini_coverage_hook(cp, binary_path); + int session_result = cbm_remove_gemini_session_hooks(cp); + if (tool_result != CLI_OK || coverage_result != CLI_OK || session_result != CLI_OK) { + record_agent_config_error(false, "Gemini CLI", "hook_disable", cp); + } + } + print_hooks_skipped(); + printf(" subagents: Scout + Verify + Auditor\n"); + return; + } if (!dry_run) { if (cbm_upsert_gemini_hooks(cp) != CLI_OK) { record_agent_config_error(false, "Gemini CLI", "before_tool_hook_install", cp); @@ -8806,8 +8884,9 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const cbm_build_augment_command(binary_path, command, sizeof(command)) == CLI_OK && cbm_build_augment_command_windows(binary_path, command_windows, sizeof(command_windows)) == CLI_OK; - cbm_toml_codex_hook_action_t preflight_action = - use_hooks_json ? CBM_TOML_CODEX_HOOK_REMOVE : CBM_TOML_CODEX_HOOK_UPSERT; + cbm_toml_codex_hook_action_t preflight_action = !g_install_hooks || use_hooks_json + ? CBM_TOML_CODEX_HOOK_REMOVE + : CBM_TOML_CODEX_HOOK_UPSERT; cbm_toml_codex_hook_failure_t preflight_failure = CBM_TOML_CODEX_HOOK_FAILURE_NONE; int preflight_result = commands_ok ? cbm_reconcile_codex_hooks_command_detailed( cp, command, command_windows, preflight_action, @@ -8848,7 +8927,14 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const plan_record("Codex CLI", "hook", hook_target); } else { bool hook_ok = true; - if (!dry_run && use_hooks_json) { + if (!g_install_hooks && !dry_run) { + int toml_result = cbm_reconcile_codex_hooks_command( + cp, command, command_windows, CBM_TOML_CODEX_HOOK_REMOVE, false); + int json_result = !cbm_file_exists(hooks_json) + ? CLI_OK + : cbm_remove_paired_lifecycle_hooks_json(hooks_json, command); + hook_ok = toml_result == CLI_OK && json_result == CLI_OK; + } else if (!dry_run && use_hooks_json) { hook_ok = cbm_upsert_paired_lifecycle_hooks_json(hooks_json, command, command_windows, NULL, CMM_HOOK_TIMEOUT_SEC) == CLI_OK && @@ -8858,7 +8944,11 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const hook_ok = cbm_upsert_codex_hooks_command(cp, command, command_windows) == CLI_OK; } if (!hook_ok) { - record_agent_config_error(false, "Codex CLI", "hook_install", hook_target); + record_agent_config_error(false, "Codex CLI", + g_install_hooks ? "hook_install" : "hook_disable", + hook_target); + } else if (!g_install_hooks) { + print_hooks_skipped(); } else { printf(" hooks: SessionStart + SubagentStart (dynamic graph context)\n"); printf(" note: non-managed hooks require /hooks trust; definition changes " @@ -8899,8 +8989,17 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const char plugin_path[CLI_BUF_1K]; snprintf(plugin_path, sizeof(plugin_path), "%s/.config/opencode/plugins/cbm-augment.ts", home); - install_generated_client_extension("OpenCode", plugin_path, binary_path, - cbm_client_adapter_opencode, dry_run); + if (g_install_hooks) { + install_generated_client_extension("OpenCode", plugin_path, binary_path, + cbm_client_adapter_opencode, dry_run); + } else if (!g_install_plan) { + if (!dry_run && cbm_file_exists(plugin_path) && + cbm_text_remove_managed_block(plugin_path, CBM_ADAPTER_MARKER_START, + CBM_ADAPTER_MARKER_END) != 0) { + record_agent_config_error(false, "OpenCode", "hook_disable", plugin_path); + } + print_hooks_skipped(); + } } if (agents->antigravity) { char cp[CLI_BUF_1K]; @@ -9141,6 +9240,13 @@ static void install_editor_agent_configs(const cbm_detected_agents_t *agents, co bool workspace_ok = cbm_openclaw_workspace_path(home, cp, workspace, sizeof(workspace)); (void)install_generic_agent_config("OpenClaw", binary_path, cp, NULL, dry_run, cbm_install_openclaw_mcp); + if (!g_install_hooks && !g_install_plan) { + if (!dry_run && cbm_file_exists(cp) && + cbm_remove_openclaw_compaction(cp) != CLI_OK) { + record_agent_config_error(false, "OpenClaw", "hook_disable", cp); + } + print_hooks_skipped(); + } if (workspace_ok) { char agents_path[CLI_BUF_1K]; char tools_path[CLI_BUF_1K]; @@ -9163,16 +9269,16 @@ static void install_editor_agent_configs(const cbm_detected_agents_t *agents, co record_agent_config_error(false, "OpenClaw", "tools_context_install", tools_path); } - if (cbm_upsert_openclaw_compaction(cp) != CLI_OK) { + if (g_install_hooks && cbm_upsert_openclaw_compaction(cp) != CLI_OK) { compaction_installed = false; record_agent_config_error(false, "OpenClaw", "compaction_install", cp); } } printf(" instructions: %s\n", agents_path); printf(" tools context: %s\n", tools_path); - if (compaction_installed) { + if (g_install_hooks && compaction_installed) { printf(" compaction: reinjects Codebase Memory\n"); - } else { + } else if (g_install_hooks) { printf(" compaction: could not update exact-owned augmentation\n"); } } @@ -9259,14 +9365,19 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents } else { int hook_result = CBM_YAML_IDENTITY_EDIT_OK; if (!dry_run) { - hook_result = prepare_config_parent(cp) - ? cbm_upsert_hermes_context_hook(cp, binary_path) - : CBM_YAML_IDENTITY_EDIT_ERROR; + hook_result = g_install_hooks + ? (prepare_config_parent(cp) + ? cbm_upsert_hermes_context_hook(cp, binary_path) + : CBM_YAML_IDENTITY_EDIT_ERROR) + : cbm_remove_hermes_context_hook(cp, binary_path); } if (hook_result == CBM_YAML_IDENTITY_EDIT_FOREIGN) { record_agent_config_error(false, "Hermes", "pre_llm_hook_foreign", cp); } else if (hook_result != CBM_YAML_IDENTITY_EDIT_OK) { - record_agent_config_error(false, "Hermes", "pre_llm_hook_install", cp); + record_agent_config_error( + false, "Hermes", g_install_hooks ? "pre_llm_hook_install" : "hook_disable", cp); + } else if (!g_install_hooks) { + print_hooks_skipped(); } else { printf(" hook: %s (pre_llm_call)\n", cp); } @@ -9312,7 +9423,11 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents } else { bool hook_ok = true; if (!dry_run) { - if (!cbm_install_augment_session_script(binary_path, session_hp)) { + if (!g_install_hooks) { + int session_result = cbm_remove_augment_session_hook(cp, session_hp); + int coverage_result = cbm_remove_augment_coverage_hook(cp, coverage_hp); + hook_ok = session_result == CLI_OK && coverage_result == CLI_OK; + } else if (!cbm_install_augment_session_script(binary_path, session_hp)) { hook_ok = false; record_agent_config_error(false, "Augment/Auggie", "session_script_install", session_hp); @@ -9320,16 +9435,23 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents hook_ok = false; record_agent_config_error(false, "Augment/Auggie", "session_hook_install", cp); } - if (!cbm_install_augment_coverage_script(binary_path, coverage_hp)) { - hook_ok = false; - record_agent_config_error(false, "Augment/Auggie", "coverage_script_install", - coverage_hp); - } else if (cbm_upsert_augment_coverage_hook(cp, coverage_hp) != CLI_OK) { - hook_ok = false; - record_agent_config_error(false, "Augment/Auggie", "coverage_hook_install", cp); + if (g_install_hooks) { + if (!cbm_install_augment_coverage_script(binary_path, coverage_hp)) { + hook_ok = false; + record_agent_config_error(false, "Augment/Auggie", + "coverage_script_install", coverage_hp); + } else if (cbm_upsert_augment_coverage_hook(cp, coverage_hp) != CLI_OK) { + hook_ok = false; + record_agent_config_error(false, "Augment/Auggie", "coverage_hook_install", + cp); + } } } - if (hook_ok) { + if (!hook_ok && !g_install_hooks) { + record_agent_config_error(false, "Augment/Auggie", "hook_disable", cp); + } else if (!g_install_hooks) { + print_hooks_skipped(); + } else if (hook_ok) { printf(" hooks: SessionStart + PostToolUse view coverage\n"); } } @@ -9390,11 +9512,20 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents #else bool windows = false; #endif - if (cbm_upsert_qwen_lifecycle_hooks(cp, binary_path, windows) != CLI_OK) { - record_agent_config_error(false, "Qwen Code", "lifecycle_hook_install", cp); + int hook_result = g_install_hooks + ? cbm_upsert_qwen_lifecycle_hooks(cp, binary_path, windows) + : cbm_remove_qwen_lifecycle_hooks(cp, binary_path, windows); + if (hook_result != CLI_OK) { + record_agent_config_error( + false, "Qwen Code", g_install_hooks ? "lifecycle_hook_install" : "hook_disable", + cp); + } else if (!g_install_hooks) { + print_hooks_skipped(); } else { printf(" hooks: SessionStart + SubagentStart + PostToolUse ReadFile\n"); } + } else if (!g_install_hooks) { + print_hooks_skipped(); } } if (agents->copilot_cli) { @@ -9439,6 +9570,12 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents if (hook_supported) { plan_record("Factory Droid", "hook", hp); } + } else if (!g_install_hooks) { + if (!dry_run && cbm_file_exists(hp) && + cbm_remove_factory_hooks(hp, binary_path) != CLI_OK) { + record_agent_config_error(false, "Factory Droid", "hook_disable", hp); + } + print_hooks_skipped(); } else if (!hook_supported) { printf(" hooks: withheld on Windows (vendor documents Bash only)\n"); } else { @@ -9914,6 +10051,44 @@ static bool cli_binary_is_externally_managed(const char *self_path, bool self_pa return cli_external_manager_name(self_path) != NULL; } +static int cli_resolve_install_hooks(bool override_set, bool override_value) { + bool enabled = true; + if (!override_set) { + const char *cache_dir = cbm_resolve_cache_dir(); + char db_path[CLI_BUF_1K]; + int written = + cache_dir ? snprintf(db_path, sizeof(db_path), "%s/_config.db", cache_dir) : CLI_ERR; + if (written <= 0 || (size_t)written >= sizeof(db_path)) { + (void)fprintf(stderr, "error: config database path is unavailable\n"); + return CLI_ERR; + } + if (cbm_file_exists(db_path)) { + cbm_config_t *cfg = cbm_config_open(cache_dir); + if (!cfg) { + (void)fprintf(stderr, "error: cannot read install_hooks preference\n"); + return CLI_ERR; + } + enabled = cbm_config_get_bool(cfg, CBM_CONFIG_INSTALL_HOOKS, true); + cbm_config_close(cfg); + } + } else { + enabled = override_value; + } + g_install_hooks = enabled; + return CLI_OK; +} + +static int cli_persist_install_hooks(void) { + const char *cache_dir = cbm_resolve_cache_dir(); + cbm_config_t *cfg = cache_dir ? cbm_config_open(cache_dir) : NULL; + if (!cfg) { + return CLI_ERR; + } + int result = cbm_config_set(cfg, CBM_CONFIG_INSTALL_HOOKS, g_install_hooks ? "true" : "false"); + cbm_config_close(cfg); + return result; +} + /* Build the agent.install.plan.v1 receipt (#388): a machine-readable list of * the config / instruction / skill / agent / hook files `install` WOULD write, produced by * running the real install dispatch in record-only mode (no mutation, no @@ -10051,6 +10226,7 @@ typedef struct { bool skip_binary; bool delete_indexes; bool skip_config; + bool persist_install_hooks; bool force; bool dry_run; } cli_install_activation_t; @@ -10104,6 +10280,13 @@ static int cli_install_activate(void *opaque) { } printf("Installed binary -> %s\n\n", activation->bin_target); } + if (!activation->dry_run && activation->persist_install_hooks && + cli_persist_install_hooks() != CLI_OK) { + cli_activation_transaction_finalize_committed_or_fail_stop( + &activation->binary_transaction, "install_transaction_config_failure_finalize"); + (void)fprintf(stderr, "error: cannot persist install_hooks preference\n"); + return CLI_ACTIVATION_PARTIAL; + } /* Config and PATH refreshes are install mutations too. Keep them in this * callback so the startup lock covers the complete filesystem window, * including same-binary and non-force installs. */ @@ -10175,6 +10358,7 @@ static int cli_install_activate(void *opaque) { int cbm_cmd_install(int argc, char **argv) { parse_auto_answer(argc, argv); + g_client_selection = NULL; bool dry_run = false; bool force = false; bool plan = false; @@ -10182,6 +10366,8 @@ int cbm_cmd_install(int argc, char **argv) { bool skip_config = false; bool skip_binary = false; bool force_binary = false; + bool hooks_flag = false; + bool no_hooks_flag = false; const char *requested_clients = NULL; const char *requested_bin_dir = NULL; for (int i = 0; i < argc; i++) { @@ -10195,6 +10381,10 @@ int cbm_cmd_install(int argc, char **argv) { reset_indexes = true; } else if (strcmp(argv[i], "--skip-config") == 0) { skip_config = true; + } else if (strcmp(argv[i], "--hooks") == 0) { + hooks_flag = true; + } else if (strcmp(argv[i], "--no-hooks") == 0) { + no_hooks_flag = true; } else if (strncmp(argv[i], "--clients=", SLEN("--clients=")) == 0) { requested_clients = argv[i] + SLEN("--clients="); if (!requested_clients[0]) { @@ -10232,11 +10422,19 @@ int cbm_cmd_install(int argc, char **argv) { } } + if (hooks_flag && no_hooks_flag) { + (void)fprintf(stderr, "error: --hooks and --no-hooks cannot be used together\n"); + return CLI_TRUE; + } + const char *home = cbm_get_home_dir(); if (!home) { (void)fprintf(stderr, "error: HOME not set (use USERPROFILE on Windows)\n"); return CLI_TRUE; } + if (cli_resolve_install_hooks(hooks_flag || no_hooks_flag, hooks_flag) != CLI_OK) { + return CLI_TRUE; + } char bin_dir[CLI_BUF_1K]; int bin_dir_length = requested_bin_dir @@ -10487,6 +10685,7 @@ int cbm_cmd_install(int argc, char **argv) { .copy_binary = do_copy, .delete_indexes = delete_indexes, .skip_config = skip_config, + .persist_install_hooks = hooks_flag || no_hooks_flag, .skip_binary = skip_binary, .force = force, .dry_run = dry_run, @@ -12444,6 +12643,9 @@ int cbm_cmd_update(int argc, char **argv) { "v0.10.0 there is one build per platform and it always includes the " "graph UI.\n"); } + if (cli_resolve_install_hooks(false, true) != CLI_OK) { + return CLI_TRUE; + } /* #1566: we cannot update a binary a package manager owns, and pretending * otherwise is the dishonest-success pattern this project keeps fixing: diff --git a/src/cli/cli.h b/src/cli/cli.h index da2c4512a..295883c55 100644 --- a/src/cli/cli.h +++ b/src/cli/cli.h @@ -423,6 +423,7 @@ int cbm_config_delete(cbm_config_t *cfg, const char *key); #define CBM_CONFIG_AUTO_INDEX "auto_index" #define CBM_CONFIG_AUTO_INDEX_LIMIT "auto_index_limit" #define CBM_CONFIG_AUTO_WATCH "auto_watch" +#define CBM_CONFIG_INSTALL_HOOKS "install_hooks" #define CBM_CONFIG_UI_LANG "ui-lang" #define CBM_CONFIG_WATCHER_ENABLED "watcher_enabled" /* #1558: the graph UI's loopback listener. Stored in the UI config file rather diff --git a/src/main.c b/src/main.c index 1e06d86ff..7851ab3ec 100644 --- a/src/main.c +++ b/src/main.c @@ -901,7 +901,7 @@ static void print_help(void) { printf(" codebase-memory-mcp cli [--progress] [--json] [args]\n"); printf(" Run one tool locally, then exit\n"); printf(" codebase-memory-mcp install [-y|-n] [--force] [--dry-run] " - "[--dir=] [--skip-config]\n"); + "[--dir=] [--skip-config] [--hooks|--no-hooks]\n"); printf(" codebase-memory-mcp uninstall [-y|-n] [--dry-run]\n"); printf(" codebase-memory-mcp update [-y|-n]\n"); printf(" codebase-memory-mcp config \n"); diff --git a/tests/test_cli.c b/tests/test_cli.c index a72ed6474..e5c113abb 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -1158,6 +1158,223 @@ TEST(cli_activation_commands_reject_malformed_and_unknown_flags) { PASS(); } +TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s/cli-install-hooks-XXXXXX", cbm_tmpdir()); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *const env_names[] = {"HOME", "CBM_CACHE_DIR", "CLAUDE_CONFIG_DIR", "PATH"}; + char *saved_env[sizeof(env_names) / sizeof(env_names[0])]; + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + saved_env[i] = save_test_env(env_names[i]); + cbm_unsetenv(env_names[i]); + } + + char cache_dir[640]; + char claude_dir[640]; + char settings_path[768]; + char mcp_path[768]; + char db_path[768]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + snprintf(claude_dir, sizeof(claude_dir), "%s/.claude", tmpdir); + snprintf(settings_path, sizeof(settings_path), "%s/settings.json", claude_dir); + snprintf(mcp_path, sizeof(mcp_path), "%s/.claude.json", tmpdir); + snprintf(db_path, sizeof(db_path), "%s/_config.db", cache_dir); + test_mkdirp(claude_dir); + write_test_file(settings_path, + "{\"hooks\":{\"PreToolUse\":[{\"matcher\":\"Bash\",\"hooks\":[{\"type\":" + "\"command\",\"command\":\"/usr/bin/foreign-hook\"}]}]}}\n"); + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + cbm_setenv("PATH", tmpdir, 1); + + cli_activation_fake_t fake = {.mutation_reserve_result = 1}; + cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake); + cbm_cli_set_activation_ops_for_test(&ops); + + char *conflict_a[] = {"--hooks", "--no-hooks"}; + char *conflict_b[] = {"--no-hooks", "--hooks"}; + int conflict_a_rc = cli_test_cmd_install(2, conflict_a); + int conflict_b_rc = cli_test_cmd_install(2, conflict_b); + + char *default_args[] = {"--skip-binary", "--clients=claude", "--yes"}; + int default_rc = cli_test_cmd_install(3, default_args); + char *default_state = read_test_file_alloc(settings_path); + bool default_hook = default_state && strstr(default_state, "cbm-code-discovery-gate") && + strstr(default_state, "/usr/bin/foreign-hook"); + free(default_state); + + char *no_hooks_plan_args[] = {"--plan", "--no-hooks"}; + int no_hooks_plan_rc = cli_test_cmd_install(2, no_hooks_plan_args); + char *disabled_plan = cbm_build_install_plan_json(tmpdir, "/opt/codebase-memory-mcp"); + yyjson_doc *disabled_doc = + disabled_plan ? yyjson_read(disabled_plan, strlen(disabled_plan), 0) : NULL; + yyjson_val *disabled_root = disabled_doc ? yyjson_doc_get_root(disabled_doc) : NULL; + yyjson_val *disabled_hooks = + disabled_root ? yyjson_obj_get(disabled_root, "hooks_planned") : NULL; + yyjson_val *disabled_configs = + disabled_root ? yyjson_obj_get(disabled_root, "config_files_planned") : NULL; + bool disabled_plan_ok = disabled_hooks && yyjson_is_arr(disabled_hooks) && + yyjson_arr_size(disabled_hooks) == 0U && disabled_configs && + yyjson_is_arr(disabled_configs) && + yyjson_arr_size(disabled_configs) > 0U; + yyjson_doc_free(disabled_doc); + free(disabled_plan); + + char *no_hooks_dry_args[] = {"--dry-run", "--no-hooks", "--skip-binary", "--clients=claude", + "--yes"}; + int no_hooks_dry_rc = cli_test_cmd_install(5, no_hooks_dry_args); + struct stat db_status; + bool previews_read_only = stat(db_path, &db_status) != 0; + + char *no_hooks_args[] = {"--no-hooks", "--skip-binary", "--clients=claude", "--yes"}; + int no_hooks_rc = cli_test_cmd_install(4, no_hooks_args); + cbm_config_t *cfg = cbm_config_open(cache_dir); + bool stored_false = cfg && !cbm_config_get_bool(cfg, CBM_CONFIG_INSTALL_HOOKS, true); + cbm_config_close(cfg); + char *disabled_state = read_test_file_alloc(settings_path); + char *disabled_mcp = read_test_file_alloc(mcp_path); + bool disabled_ok = disabled_state && strstr(disabled_state, "/usr/bin/foreign-hook") && + !strstr(disabled_state, "cbm-code-discovery-gate") && disabled_mcp && + strstr(disabled_mcp, "codebase-memory-mcp"); + free(disabled_state); + free(disabled_mcp); + + char *bare_args[] = {"--skip-binary", "--clients=claude", "--yes"}; + int bare_disabled_rc = cli_test_cmd_install(3, bare_args); + disabled_state = read_test_file_alloc(settings_path); + bool bare_disabled = disabled_state && strstr(disabled_state, "/usr/bin/foreign-hook") && + !strstr(disabled_state, "cbm-code-discovery-gate"); + free(disabled_state); + + char *hooks_plan_args[] = {"--plan", "--hooks"}; + char *hooks_dry_args[] = {"--dry-run", "--hooks", "--skip-binary", "--clients=claude", "--yes"}; + int hooks_plan_rc = cli_test_cmd_install(2, hooks_plan_args); + int hooks_dry_rc = cli_test_cmd_install(5, hooks_dry_args); + cfg = cbm_config_open(cache_dir); + bool previews_retained_false = cfg && !cbm_config_get_bool(cfg, CBM_CONFIG_INSTALL_HOOKS, true); + cbm_config_close(cfg); + + char *hooks_args[] = {"--hooks", "--skip-binary", "--clients=claude", "--yes"}; + int hooks_rc = cli_test_cmd_install(4, hooks_args); + cfg = cbm_config_open(cache_dir); + bool stored_true = cfg && cbm_config_get_bool(cfg, CBM_CONFIG_INSTALL_HOOKS, false); + cbm_config_close(cfg); + char *enabled_state = read_test_file_alloc(settings_path); + bool enabled_ok = enabled_state && strstr(enabled_state, "cbm-code-discovery-gate") && + strstr(enabled_state, "/usr/bin/foreign-hook"); + free(enabled_state); + int bare_enabled_rc = cli_test_cmd_install(3, bare_args); + bool results_ok = + conflict_a_rc == 1 && conflict_b_rc == 1 && default_rc == 0 && default_hook && + no_hooks_plan_rc == 0 && disabled_plan_ok && no_hooks_dry_rc == 0 && previews_read_only && + no_hooks_rc == 0 && stored_false && disabled_ok && bare_disabled_rc == 0 && bare_disabled && + hooks_plan_rc == 0 && hooks_dry_rc == 0 && previews_retained_false && hooks_rc == 0 && + stored_true && enabled_ok && bare_enabled_rc == 0 && fake.mutation_reserve_count > 0; + + cbm_cli_set_activation_ops_for_test(NULL); + cbm_set_auto_answer_for_test(0); + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + restore_test_env(env_names[i], saved_env[i]); + } + test_rmdir_r(tmpdir); + + ASSERT_TRUE(results_ok); + PASS(); +} + +TEST(cli_no_hooks_plan_covers_every_hook_installer_family) { + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s/cli-hook-families-XXXXXX", cbm_tmpdir()); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *const env_names[] = {"HOME", "CBM_CACHE_DIR", "PATH", "XDG_CONFIG_HOME"}; + char *saved_env[sizeof(env_names) / sizeof(env_names[0])]; + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + saved_env[i] = save_test_env(env_names[i]); + cbm_unsetenv(env_names[i]); + } + + const char *const dirs[] = {".claude", ".codex", ".gemini", + ".config/opencode", ".augment", ".openclaw/workspace", + ".hermes", ".qwen", ".copilot", + ".factory", ".qoder", ".kimi-code", + ".config/devin", ".gitlab/duo", "bin"}; + char path[768]; + for (size_t i = 0U; i < sizeof(dirs) / sizeof(dirs[0]); i++) { + snprintf(path, sizeof(path), "%s/%s", tmpdir, dirs[i]); + test_mkdirp(path); + } + snprintf(path, sizeof(path), "%s/.openclaw/openclaw.json", tmpdir); + write_test_file(path, "{}\n"); + snprintf(path, sizeof(path), "%s/.copilot/mcp-config.json", tmpdir); + write_test_file(path, "{}\n"); + + const char *const commands[] = {"qodercli", "kimi", "duo", "devin", "gemini", + "copilot", "auggie", "hermes", "droid"}; + char bin_dir[640]; + snprintf(bin_dir, sizeof(bin_dir), "%s/bin", tmpdir); + for (size_t i = 0U; i < sizeof(commands) / sizeof(commands[0]); i++) { +#ifdef _WIN32 + snprintf(path, sizeof(path), "%s/%s.exe", bin_dir, commands[i]); +#else + snprintf(path, sizeof(path), "%s/%s", bin_dir, commands[i]); +#endif + write_test_file(path, "exit 0\n"); + chmod(path, 0700); + } + + char cache_dir[640]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + cbm_setenv("PATH", bin_dir, 1); + char *no_hooks_args[] = {"--plan", "--no-hooks"}; + int no_hooks_rc = cli_test_cmd_install(2, no_hooks_args); + char *plan = cbm_build_install_plan_json(tmpdir, "/opt/codebase-memory-mcp"); + yyjson_doc *doc = plan ? yyjson_read(plan, strlen(plan), 0) : NULL; + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *hooks = root ? yyjson_obj_get(root, "hooks_planned") : NULL; + yyjson_val *configs = root ? yyjson_obj_get(root, "config_files_planned") : NULL; + yyjson_val *instructions = root ? yyjson_obj_get(root, "instruction_files_planned") : NULL; + const char *const detected[] = {"claude-code", "codex", "gemini", "opencode", + "augment-auggie", "openclaw", "hermes", "qwen", + "copilot-cli", "factory-droid", "qoder", "kimi", + "gitlab-duo", "devin"}; + bool families = no_hooks_rc == 0 && hooks && yyjson_is_arr(hooks) && + yyjson_arr_size(hooks) == 0U && configs && yyjson_is_arr(configs) && + yyjson_arr_size(configs) > 0U && instructions && yyjson_is_arr(instructions) && + yyjson_arr_size(instructions) > 0U && plan && !strstr(plan, "cbm-augment.ts"); + for (size_t i = 0U; families && i < sizeof(detected) / sizeof(detected[0]); i++) { + families = test_json_string_array_contains(root, "agents_detected", detected[i]); + } + yyjson_doc_free(doc); + free(plan); + + char *hooks_args[] = {"--plan", "--hooks"}; + int hooks_rc = cli_test_cmd_install(2, hooks_args); + char *enabled_plan = cbm_build_install_plan_json(tmpdir, "/opt/codebase-memory-mcp"); + yyjson_doc *enabled_doc = + enabled_plan ? yyjson_read(enabled_plan, strlen(enabled_plan), 0) : NULL; + yyjson_val *enabled_root = enabled_doc ? yyjson_doc_get_root(enabled_doc) : NULL; + yyjson_val *enabled_hooks = enabled_root ? yyjson_obj_get(enabled_root, "hooks_planned") : NULL; + bool enabled_plan_ok = + enabled_hooks && yyjson_is_arr(enabled_hooks) && yyjson_arr_size(enabled_hooks) > 0U; + yyjson_doc_free(enabled_doc); + free(enabled_plan); + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + restore_test_env(env_names[i], saved_env[i]); + } + test_rmdir_r(tmpdir); + + ASSERT_TRUE(families); + ASSERT_EQ(hooks_rc, 0); + ASSERT_TRUE(enabled_plan_ok); + PASS(); +} + TEST(cli_install_reset_deletion_waits_for_final_activation_guard) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-daemon-install-reset-XXXXXX"); @@ -12724,18 +12941,32 @@ TEST(cli_config_cmd_get_prints_defaults_and_rejects_unknown_keys) { char *get_limit[] = {"get", "auto_index_limit"}; ASSERT_EQ(cli_config_cmd_capture(2, get_limit, out, sizeof(out)), 0); ASSERT_STR_EQ(out, "50000\n"); + char *get_install_hooks[] = {"get", "install_hooks"}; + ASSERT_EQ(cli_config_cmd_capture(2, get_install_hooks, out, sizeof(out)), 0); + ASSERT_STR_EQ(out, "true\n"); + char *list[] = {"list"}; + ASSERT_EQ(cli_config_cmd_capture(1, list, out, sizeof(out)), 0); + ASSERT_NOT_NULL(strstr(out, "install_hooks")); /* Stored value round-trips. */ char *set_watch[] = {"set", "auto_watch", "false"}; ASSERT_EQ(cli_config_cmd_capture(3, set_watch, out, sizeof(out)), 0); ASSERT_EQ(cli_config_cmd_capture(2, get_watch, out, sizeof(out)), 0); ASSERT_STR_EQ(out, "false\n"); + char *set_install_hooks[] = {"set", "install_hooks", "false"}; + ASSERT_EQ(cli_config_cmd_capture(3, set_install_hooks, out, sizeof(out)), 0); + ASSERT_EQ(cli_config_cmd_capture(2, get_install_hooks, out, sizeof(out)), 0); + ASSERT_STR_EQ(out, "false\n"); /* Reset returns the key to its default. */ char *reset_watch[] = {"reset", "auto_watch"}; ASSERT_EQ(cli_config_cmd_capture(2, reset_watch, out, sizeof(out)), 0); ASSERT_EQ(cli_config_cmd_capture(2, get_watch, out, sizeof(out)), 0); ASSERT_STR_EQ(out, "true\n"); + char *reset_install_hooks[] = {"reset", "install_hooks"}; + ASSERT_EQ(cli_config_cmd_capture(2, reset_install_hooks, out, sizeof(out)), 0); + ASSERT_EQ(cli_config_cmd_capture(2, get_install_hooks, out, sizeof(out)), 0); + ASSERT_STR_EQ(out, "true\n"); /* Unknown keys are errors on every subcommand, with clean stdout. */ char *get_bogus[] = {"get", "totally_bogus_key"}; @@ -13669,6 +13900,8 @@ SUITE(cli) { RUN_TEST(cli_install_force_quiesces_active_cohort_before_replacing_binary); RUN_TEST(cli_install_dir_and_skip_config_stage_first_install_safely); RUN_TEST(cli_activation_commands_reject_malformed_and_unknown_flags); + RUN_TEST(cli_install_hooks_preference_persists_and_migrates_owned_state); + RUN_TEST(cli_no_hooks_plan_covers_every_hook_installer_family); RUN_TEST(cli_install_reset_deletion_waits_for_final_activation_guard); RUN_TEST(cli_install_config_only_waits_for_cohort_drain); RUN_TEST(cli_install_config_and_path_finish_before_guard_release); From b0713cc24dcd2d4f35c5cce1988b1b4b6b04907e Mon Sep 17 00:00:00 2001 From: Andy11-cpu Date: Fri, 28 Aug 2026 00:37:48 -0400 Subject: [PATCH 02/19] fix(cli): remove owned Claude scripts with no-hooks Signed-off-by: Andy11-cpu --- scripts/smoke-test.sh | 1 + src/cli/cli.c | 169 ++++++++++++++++++++++-------------------- tests/test_cli.c | 47 +++++++++++- 3 files changed, 134 insertions(+), 83 deletions(-) diff --git a/scripts/smoke-test.sh b/scripts/smoke-test.sh index 5fd4c84f5..aa192261e 100755 --- a/scripts/smoke-test.sh +++ b/scripts/smoke-test.sh @@ -1694,6 +1694,7 @@ echo "OK 8e: shim installed, non-blocking, delegates to hook-augment" HOME="$FAKE_HOME" XDG_CONFIG_HOME="$FAKE_HOME/.config" APPDATA="$FAKE_HOME/AppData/Roaming" LOCALAPPDATA="$FAKE_HOME/AppData/Local" PATH="$FAKE_HOME/.local/bin:$PATH" \ "$BINARY" install --no-hooks --skip-binary --clients=claude -y >/dev/null if grep -qE 'cbm-(code-discovery-gate|session-reminder|subagent-reminder)' "$FAKE_HOME/.claude/settings.json" 2>/dev/null || + [ -e "$FAKE_HOME/.claude/hooks/cbm-code-discovery-gate" ] || ! grep -q 'codebase-memory-mcp' "$FAKE_HOME/.claude.json" 2>/dev/null; then echo "FAIL 8e-i: --no-hooks did not remove owned Claude hooks while preserving MCP" exit 1 diff --git a/src/cli/cli.c b/src/cli/cli.c index cd1837260..0fd9aca4e 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -7663,6 +7663,93 @@ static void uninstall_tiered_profile_prompts(const char *label, const char *veri cbm_graph_profile_dialect_t dialect, const char *legacy_verify_content, bool dry_run); +static int remove_claude_owned_hook_scripts(const char *config_dir, const char *binary_path, + bool uninstall) { + char current_gate[CLI_BUF_8K]; + char current_session[CLI_BUF_8K]; + char current_subagent[CLI_BUF_8K]; + char released_gate[CLI_BUF_8K]; + const char *const gate_legacy[] = {released_gate}; + const char *const session_legacy[] = {cmm_released_session_script}; + const char *const subagent_legacy[] = {cmm_released_subagent_script}; + size_t gate_legacy_count = + cbm_build_released_gate_script(binary_path, released_gate, sizeof(released_gate)) == CLI_OK + ? 1U + : 0U; + static const struct { + const char *name; + const char *legacy_name; + const char *prefix; + } hook_types[] = { + {CMM_HOOK_GATE_SCRIPT, CMM_HOOK_GATE_SCRIPT_LEGACY, cmm_gate_script_prefix}, + {CMM_SESSION_REMINDER_SCRIPT, CMM_SESSION_REMINDER_SCRIPT_LEGACY, + cmm_session_script_prefix}, + {CMM_SUBAGENT_REMINDER_SCRIPT, CMM_SUBAGENT_REMINDER_SCRIPT_LEGACY, + cmm_subagent_script_prefix}, + }; + struct { + const char *name; + const char *legacy_name; + const char *current; + const char *const *legacy; + size_t legacy_count; + bool current_valid; + } owned_scripts[] = { + {hook_types[0].name, hook_types[0].legacy_name, current_gate, gate_legacy, + gate_legacy_count, + cbm_build_current_hook_script(hook_types[0].prefix, binary_path, current_gate, + sizeof(current_gate)) == CLI_OK}, + {hook_types[1].name, hook_types[1].legacy_name, current_session, session_legacy, 1U, + cbm_build_current_hook_script(hook_types[1].prefix, binary_path, current_session, + sizeof(current_session)) == CLI_OK}, + {hook_types[2].name, hook_types[2].legacy_name, current_subagent, subagent_legacy, 1U, + cbm_build_current_hook_script(hook_types[2].prefix, binary_path, current_subagent, + sizeof(current_subagent)) == CLI_OK}, + }; + char hooks_dir[CLI_BUF_1K]; + int hooks_written = snprintf(hooks_dir, sizeof(hooks_dir), "%s/hooks", config_dir); + bool hooks_dir_valid = hooks_written > 0 && (size_t)hooks_written < sizeof(hooks_dir); + int rc = CLI_OK; + const char *op = uninstall ? "hook_script_uninstall" : "hook_script_disable"; + for (size_t i = 0; i < sizeof(owned_scripts) / sizeof(owned_scripts[0]); i++) { + char script_path[CLI_BUF_1K]; + int script_written = hooks_dir_valid ? snprintf(script_path, sizeof(script_path), "%s/%s", + hooks_dir, owned_scripts[i].name) + : CLI_ERR; + bool script_path_valid = script_written > 0 && (size_t)script_written < sizeof(script_path); + if (!owned_scripts[i].current_valid) { + record_agent_config_error(uninstall, "Claude Code", op, owned_scripts[i].name); + rc = CLI_ERR; + continue; + } + if (!script_path_valid || + cbm_remove_owned_hook_script(script_path, owned_scripts[i].current, + owned_scripts[i].legacy, + owned_scripts[i].legacy_count) < CLI_OK) { + record_agent_config_error(uninstall, "Claude Code", op, + script_path_valid ? script_path : owned_scripts[i].name); + rc = CLI_ERR; + } +#ifdef _WIN32 + if (!hooks_dir_valid || + cbm_remove_owned_legacy_hook_script(hooks_dir, owned_scripts[i].legacy_name, + owned_scripts[i].current, owned_scripts[i].legacy, + owned_scripts[i].legacy_count) != CLI_OK) { + char legacy_path[CLI_BUF_1K]; + int written = hooks_dir_valid ? snprintf(legacy_path, sizeof(legacy_path), "%s/%s", + hooks_dir, owned_scripts[i].legacy_name) + : CLI_ERR; + record_agent_config_error(uninstall, "Claude Code", "legacy_hook_script_uninstall", + written > 0 && (size_t)written < sizeof(legacy_path) + ? legacy_path + : owned_scripts[i].legacy_name); + rc = CLI_ERR; + } +#endif + } + return rc; +} + static void install_claude_code_config(const char *home, const char *binary_path, bool force, bool dry_run) { char config_dir[CLI_BUF_1K]; @@ -7752,6 +7839,7 @@ static void install_claude_code_config(const char *home, const char *binary_path if (cbm_remove_claude_subagent_hooks(settings_path) != CLI_OK) { record_agent_config_error(false, "Claude Code", "hook_disable", settings_path); } + remove_claude_owned_hook_scripts(config_dir, binary_path, false); } print_hooks_skipped(); goto claude_hooks_done; @@ -10787,86 +10875,7 @@ static void uninstall_claude_code(const char *home, bool dry_run) { record_agent_config_error(true, "Claude Code", "subagent_hook_uninstall", settings_path); } - char current_gate[CLI_BUF_8K]; - char current_session[CLI_BUF_8K]; - char current_subagent[CLI_BUF_8K]; - char released_gate[CLI_BUF_8K]; - const char *const gate_legacy[] = {released_gate}; - const char *const session_legacy[] = {cmm_released_session_script}; - const char *const subagent_legacy[] = {cmm_released_subagent_script}; - size_t gate_legacy_count = cbm_build_released_gate_script(installed_binary, released_gate, - sizeof(released_gate)) == CLI_OK - ? 1U - : 0U; - static const struct { - const char *name; - const char *legacy_name; - const char *prefix; - } hook_types[] = { - {CMM_HOOK_GATE_SCRIPT, CMM_HOOK_GATE_SCRIPT_LEGACY, cmm_gate_script_prefix}, - {CMM_SESSION_REMINDER_SCRIPT, CMM_SESSION_REMINDER_SCRIPT_LEGACY, - cmm_session_script_prefix}, - {CMM_SUBAGENT_REMINDER_SCRIPT, CMM_SUBAGENT_REMINDER_SCRIPT_LEGACY, - cmm_subagent_script_prefix}, - }; - struct { - const char *name; - const char *legacy_name; - const char *current; - const char *const *legacy; - size_t legacy_count; - bool current_valid; - } owned_scripts[] = { - {hook_types[0].name, hook_types[0].legacy_name, current_gate, gate_legacy, - gate_legacy_count, - cbm_build_current_hook_script(hook_types[0].prefix, installed_binary, current_gate, - sizeof(current_gate)) == CLI_OK}, - {hook_types[1].name, hook_types[1].legacy_name, current_session, session_legacy, 1U, - cbm_build_current_hook_script(hook_types[1].prefix, installed_binary, current_session, - sizeof(current_session)) == CLI_OK}, - {hook_types[2].name, hook_types[2].legacy_name, current_subagent, subagent_legacy, 1U, - cbm_build_current_hook_script(hook_types[2].prefix, installed_binary, current_subagent, - sizeof(current_subagent)) == CLI_OK}, - }; - char hooks_dir[CLI_BUF_1K]; - int hooks_written = snprintf(hooks_dir, sizeof(hooks_dir), "%s/hooks", config_dir); - bool hooks_dir_valid = hooks_written > 0 && (size_t)hooks_written < sizeof(hooks_dir); - for (size_t i = 0; i < sizeof(owned_scripts) / sizeof(owned_scripts[0]); i++) { - char script_path[CLI_BUF_1K]; - int script_written = hooks_dir_valid - ? snprintf(script_path, sizeof(script_path), "%s/%s", - hooks_dir, owned_scripts[i].name) - : CLI_ERR; - bool script_path_valid = - script_written > 0 && (size_t)script_written < sizeof(script_path); - if (!owned_scripts[i].current_valid) { - record_agent_config_error(true, "Claude Code", "hook_script_uninstall", - owned_scripts[i].name); - continue; - } - if (!script_path_valid || - cbm_remove_owned_hook_script(script_path, owned_scripts[i].current, - owned_scripts[i].legacy, - owned_scripts[i].legacy_count) < CLI_OK) { - record_agent_config_error(true, "Claude Code", "hook_script_uninstall", - script_path_valid ? script_path : owned_scripts[i].name); - } -#ifdef _WIN32 - if (!hooks_dir_valid || - cbm_remove_owned_legacy_hook_script( - hooks_dir, owned_scripts[i].legacy_name, owned_scripts[i].current, - owned_scripts[i].legacy, owned_scripts[i].legacy_count) != CLI_OK) { - char legacy_path[CLI_BUF_1K]; - int written = hooks_dir_valid ? snprintf(legacy_path, sizeof(legacy_path), "%s/%s", - hooks_dir, owned_scripts[i].legacy_name) - : CLI_ERR; - record_agent_config_error(true, "Claude Code", "legacy_hook_script_uninstall", - written > 0 && (size_t)written < sizeof(legacy_path) - ? legacy_path - : owned_scripts[i].legacy_name); - } -#endif - } + remove_claude_owned_hook_scripts(config_dir, installed_binary, true); } printf(" removed PreToolUse + SessionStart + SubagentStart hooks\n"); } diff --git a/tests/test_cli.c b/tests/test_cli.c index e5c113abb..b9790fdce 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -1176,11 +1176,23 @@ TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { char settings_path[768]; char mcp_path[768]; char db_path[768]; + char gate_path[768]; + char session_path[768]; + char subagent_path[768]; snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); snprintf(claude_dir, sizeof(claude_dir), "%s/.claude", tmpdir); snprintf(settings_path, sizeof(settings_path), "%s/settings.json", claude_dir); snprintf(mcp_path, sizeof(mcp_path), "%s/.claude.json", tmpdir); snprintf(db_path, sizeof(db_path), "%s/_config.db", cache_dir); +#ifdef _WIN32 + snprintf(gate_path, sizeof(gate_path), "%s/hooks/cbm-code-discovery-gate.cmd", claude_dir); + snprintf(session_path, sizeof(session_path), "%s/hooks/cbm-session-reminder.cmd", claude_dir); + snprintf(subagent_path, sizeof(subagent_path), "%s/hooks/cbm-subagent-reminder.cmd", claude_dir); +#else + snprintf(gate_path, sizeof(gate_path), "%s/hooks/cbm-code-discovery-gate", claude_dir); + snprintf(session_path, sizeof(session_path), "%s/hooks/cbm-session-reminder", claude_dir); + snprintf(subagent_path, sizeof(subagent_path), "%s/hooks/cbm-subagent-reminder", claude_dir); +#endif test_mkdirp(claude_dir); write_test_file(settings_path, "{\"hooks\":{\"PreToolUse\":[{\"matcher\":\"Bash\",\"hooks\":[{\"type\":" @@ -1203,6 +1215,8 @@ TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { char *default_state = read_test_file_alloc(settings_path); bool default_hook = default_state && strstr(default_state, "cbm-code-discovery-gate") && strstr(default_state, "/usr/bin/foreign-hook"); + bool default_scripts = cbm_file_exists(gate_path) && cbm_file_exists(session_path) && + cbm_file_exists(subagent_path); free(default_state); char *no_hooks_plan_args[] = {"--plan", "--no-hooks"}; @@ -1238,6 +1252,8 @@ TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { bool disabled_ok = disabled_state && strstr(disabled_state, "/usr/bin/foreign-hook") && !strstr(disabled_state, "cbm-code-discovery-gate") && disabled_mcp && strstr(disabled_mcp, "codebase-memory-mcp"); + bool no_hooks_scripts_gone = !cbm_file_exists(gate_path) && !cbm_file_exists(session_path) && + !cbm_file_exists(subagent_path); free(disabled_state); free(disabled_mcp); @@ -1246,6 +1262,9 @@ TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { disabled_state = read_test_file_alloc(settings_path); bool bare_disabled = disabled_state && strstr(disabled_state, "/usr/bin/foreign-hook") && !strstr(disabled_state, "cbm-code-discovery-gate"); + bool bare_scripts_gone = + !cbm_file_exists(gate_path) && !cbm_file_exists(session_path) && + !cbm_file_exists(subagent_path); free(disabled_state); char *hooks_plan_args[] = {"--plan", "--hooks"}; @@ -1264,14 +1283,36 @@ TEST(cli_install_hooks_preference_persists_and_migrates_owned_state) { char *enabled_state = read_test_file_alloc(settings_path); bool enabled_ok = enabled_state && strstr(enabled_state, "cbm-code-discovery-gate") && strstr(enabled_state, "/usr/bin/foreign-hook"); + bool hooks_scripts_back = cbm_file_exists(gate_path) && cbm_file_exists(session_path) && + cbm_file_exists(subagent_path); free(enabled_state); int bare_enabled_rc = cli_test_cmd_install(3, bare_args); + bool bare_scripts_enabled = cbm_file_exists(gate_path) && cbm_file_exists(session_path) && + cbm_file_exists(subagent_path); + + const char *foreign_session_script = +#ifdef _WIN32 + "@echo off\r\nC:\\tmp\\user-owned-session-hook.cmd\r\n"; +#else + "#!/bin/sh\n/usr/bin/user-owned-session-hook\n"; +#endif + write_test_file(session_path, foreign_session_script); + int foreign_no_hooks_rc = cli_test_cmd_install(4, no_hooks_args); + char *preserved_session = read_test_file_alloc(session_path); + bool foreign_session_preserved = + foreign_no_hooks_rc == 0 && preserved_session && + strcmp(preserved_session, foreign_session_script) == 0 && !cbm_file_exists(gate_path) && + !cbm_file_exists(subagent_path); + free(preserved_session); + bool results_ok = conflict_a_rc == 1 && conflict_b_rc == 1 && default_rc == 0 && default_hook && - no_hooks_plan_rc == 0 && disabled_plan_ok && no_hooks_dry_rc == 0 && previews_read_only && - no_hooks_rc == 0 && stored_false && disabled_ok && bare_disabled_rc == 0 && bare_disabled && + default_scripts && no_hooks_plan_rc == 0 && disabled_plan_ok && no_hooks_dry_rc == 0 && + previews_read_only && no_hooks_rc == 0 && stored_false && disabled_ok && + no_hooks_scripts_gone && bare_disabled_rc == 0 && bare_disabled && bare_scripts_gone && hooks_plan_rc == 0 && hooks_dry_rc == 0 && previews_retained_false && hooks_rc == 0 && - stored_true && enabled_ok && bare_enabled_rc == 0 && fake.mutation_reserve_count > 0; + stored_true && enabled_ok && hooks_scripts_back && bare_enabled_rc == 0 && + bare_scripts_enabled && foreign_session_preserved && fake.mutation_reserve_count > 0; cbm_cli_set_activation_ops_for_test(NULL); cbm_set_auto_answer_for_test(0); From 0e8fc16abce48346d07af78acf3293a805085e23 Mon Sep 17 00:00:00 2001 From: Andy11-cpu Date: Wed, 2 Sep 2026 00:22:06 -0400 Subject: [PATCH 03/19] fix(cli): guard Gemini coverage cleanup on Windows Signed-off-by: Andy11-cpu --- src/cli/cli.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index 0fd9aca4e..bb4a4cd32 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -8919,7 +8919,10 @@ static void install_gemini_config(const char *home, const char *binary_path, boo if (!g_install_hooks) { if (!dry_run) { int tool_result = cbm_remove_gemini_hooks(cp); - int coverage_result = cbm_remove_gemini_coverage_hook(cp, binary_path); + int coverage_result = CLI_OK; +#ifndef _WIN32 + coverage_result = cbm_remove_gemini_coverage_hook(cp, binary_path); +#endif int session_result = cbm_remove_gemini_session_hooks(cp); if (tool_result != CLI_OK || coverage_result != CLI_OK || session_result != CLI_OK) { record_agent_config_error(false, "Gemini CLI", "hook_disable", cp); From 6b6106fa936de96eb7d6e2432e79303f003bacb2 Mon Sep 17 00:00:00 2001 From: tahodev <85001874+tahodev@users.noreply.github.com> Date: Wed, 21 Oct 2026 13:28:53 +0900 Subject: [PATCH 04/19] refactor(search): run POSIX unscoped scan as argv stages, no shell (#2634) Signed-off-by: tahodev <85001874+tahodev@users.noreply.github.com> --- src/mcp/mcp.c | 327 ++++++++++++++++++++++++++++++++++++++++++++++- tests/test_mcp.c | 138 ++++++++++++++++++++ 2 files changed, 461 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 5bd02e4e3..6b79ba94f 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -16474,6 +16474,28 @@ static mcp_scan_cause_t mcp_run_shell_command_cancellable_bounded( uint64_t deadline_ms, bool deadline_enabled, bool deadline_latched, bool exit_one_is_no_match, bool git_child, cbm_proc_result_t *result_out); +#ifndef _WIN32 +/* POSIX-only: supervised run of one argv stage (no shell). Same latch and + * containment contract as the shell runner; the measured file's size plus + * output_base counts toward output_limit so chained stages share one budget. + * tolerate_exit_one maps a plain exit-1 (grep's no-match) to SUCCESS. */ +static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( + cbm_mcp_server_t *srv, const char *bin, const char *const *argv, + const char *const *env_overrides, const char *stdout_capture, uint64_t output_base, + size_t output_limit, uint64_t deadline_ms, bool deadline_enabled, bool tolerate_exit_one, + bool *cancellation_latched, bool *deadline_latched, bool *output_limit_latched, + cbm_proc_result_t *result_out); + +/* POSIX-only: the unscoped search_code scan as find/sort/grep argv stages. */ +static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, + const char *file_pattern, + const char *pattern_file, const char *filelist, + const char *root_path, + char output_path[CBM_SZ_2K], size_t output_limit, + uint64_t deadline_ms, bool deadline_latched, + cbm_proc_result_t *result_out); +#endif + static char *search_code_timeout_result(void) { static const char message[] = "search_code scan exceeded its execution deadline"; static const char fallback[] = @@ -16873,10 +16895,22 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { scan_cause = MCP_SCAN_COMMAND_FAILURE; } } else { - scan_cause = mcp_run_shell_command_cancellable_bounded( - srv, scan_command, output_path, scan_output_limit, scan_deadline_ms, true, - scan_deadline_latched, /*exit_one_is_no_match=*/false, /*git_child=*/false, - &scan_result); +#ifndef _WIN32 + if (!srv->search_scan_command_override) { + /* POSIX unscoped scans run as find/sort/grep argv stages: no + * shell parses search scopes or file patterns anymore (#2634). + * The command override keeps the shell runner for tests. */ + scan_cause = mcp_run_search_scan_argv( + srv, use_regex, file_pattern, tmpfile, filelist, root_path, output_path, + scan_output_limit, scan_deadline_ms, scan_deadline_latched, &scan_result); + } else +#endif + { + scan_cause = mcp_run_shell_command_cancellable_bounded( + srv, scan_command, output_path, scan_output_limit, scan_deadline_ms, true, + scan_deadline_latched, /*exit_one_is_no_match=*/false, /*git_child=*/false, + &scan_result); + } } /* Both POSIX commands wrap grep and map its no-match status to 0, so any * non-zero exit (a failed find/sort, an unreadable operand, a broken @@ -17395,6 +17429,291 @@ static int mcp_run_shell_command_cancellable(cbm_mcp_server_t *srv, const char * return cause == MCP_SCAN_COMMAND_FAILURE || cause == MCP_SCAN_SUPERVISION_FAILURE ? -1 : 0; } +#ifndef _WIN32 +/* Latch poll shared by argv stages: request-cancellation, wall-clock deadline, + * and the cumulative measured-file output budget. Mirrors the latch updates of + * the shell runner so both paths fail the same way. */ +static void mcp_argv_stage_poll_latches(cbm_mcp_server_t *srv, const char *measured_path, + uint64_t output_base, size_t output_limit, + uint64_t deadline_ms, bool deadline_enabled, + bool *cancellation_latched, bool *deadline_latched, + bool *output_limit_latched) { + *cancellation_latched = *cancellation_latched || mcp_request_cancelled(srv); + *deadline_latched = *deadline_latched || (deadline_enabled && cbm_now_ms() >= deadline_ms); + if (!*output_limit_latched && output_limit > 0 && measured_path && measured_path[0]) { + int64_t measured_size = cbm_file_size(measured_path); + *output_limit_latched = + measured_size > 0 && output_base + (uint64_t)measured_size > output_limit; + } +} + +static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( + cbm_mcp_server_t *srv, const char *bin, const char *const *argv, + const char *const *env_overrides, const char *stdout_capture, uint64_t output_base, + size_t output_limit, uint64_t deadline_ms, bool deadline_enabled, bool tolerate_exit_one, + bool *cancellation_latched, bool *deadline_latched, bool *output_limit_latched, + cbm_proc_result_t *result_out) { + memset(result_out, 0, sizeof(*result_out)); + mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms, + deadline_enabled, cancellation_latched, deadline_latched, + output_limit_latched); + if (*cancellation_latched || *deadline_latched || *output_limit_latched) { + result_out->tree_quiesced = true; /* no child was spawned */ + return *cancellation_latched ? MCP_SCAN_CANCELLED + : *deadline_latched ? MCP_SCAN_DEADLINE + : MCP_SCAN_OUTPUT_LIMIT; + } + cbm_proc_opts_t options = { + .bin = bin, + .argv = argv, + .env_overrides = env_overrides, + .log_file = NULL, + .quiet_timeout_ms = 0, + .cancel_grace_ms = CBM_SUBPROCESS_DEFAULT_CANCEL_GRACE_MS, + .delete_log_on_exit = false, + .strip_git_repo_env = false, + .stdout_file = stdout_capture, + }; + cbm_subprocess_t *process = NULL; + if (cbm_subprocess_spawn(&options, &process) != 0) { + result_out->tree_quiesced = true; + return MCP_SCAN_COMMAND_FAILURE; + } + cbm_proc_poll_t state; + for (;;) { + mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms, + deadline_enabled, cancellation_latched, deadline_latched, + output_limit_latched); + if (*cancellation_latched || *deadline_latched || *output_limit_latched) { + (void)cbm_subprocess_request_cancel(process); + } + state = cbm_subprocess_poll(process, result_out); + if (state != CBM_PROC_POLL_RUNNING) { + break; + } + cbm_usleep(10000); + } + bool contained = state == CBM_PROC_POLL_TERMINAL && result_out->tree_quiesced && + !result_out->supervision_failed; + cbm_subprocess_destroy(process); + mcp_argv_stage_poll_latches(srv, stdout_capture, output_base, output_limit, deadline_ms, + deadline_enabled, cancellation_latched, deadline_latched, + output_limit_latched); + if (!contained) { + return MCP_SCAN_SUPERVISION_FAILURE; + } + if (*cancellation_latched) { + return MCP_SCAN_CANCELLED; + } + if (*deadline_latched) { + return MCP_SCAN_DEADLINE; + } + if (*output_limit_latched) { + return MCP_SCAN_OUTPUT_LIMIT; + } + if (tolerate_exit_one && result_out->outcome == CBM_PROC_EXIT_NONZERO && + result_out->exit_code == 1) { + return MCP_SCAN_SUCCESS; /* ordinary grep no-match */ + } + return result_out->outcome == CBM_PROC_CLEAN ? MCP_SCAN_SUCCESS + : MCP_SCAN_CONTAINED_COMMAND_FAILURE; +} + +/* Number of file operands per grep spawn. Bounded so argv stays far below + * ARG_MAX even with long paths, like the xargs batching it replaces. */ +#define MCP_ARGV_SCAN_GREP_CHUNK 256 + +/* The unscoped (unindexed or not-fully-indexed root) search_code scan as three + * argv stages, with no shell anywhere: find writes the NUL-delimited candidate + * list, LC_ALL=C sort orders it in place, and grep scans it in chunks. Search + * scopes, file patterns and the pattern file now reach the tools as plain + * arguments instead of shell-string interpolations (#2634). Exit codes keep + * the old pipeline's contract: find/sort failures fail closed, grep's exit 1 + * is an ordinary no-match, and an empty list is a successful empty scan. */ +static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, + const char *file_pattern, + const char *pattern_file, const char *filelist, + const char *root_path, + char output_path[CBM_SZ_2K], size_t output_limit, + uint64_t deadline_ms, bool deadline_latched, + cbm_proc_result_t *result_out) { + bool cancellation_latched = false; + bool output_limit_latched = false; + + const char *find_argv[9]; + int fa = 0; + find_argv[fa++] = "find"; + find_argv[fa++] = root_path; + find_argv[fa++] = "-type"; + find_argv[fa++] = "f"; + if (file_pattern) { + find_argv[fa++] = "-name"; + find_argv[fa++] = file_pattern; + } + find_argv[fa++] = "-print0"; + find_argv[fa] = NULL; + mcp_scan_cause_t cause = mcp_run_argv_stage_cancellable_bounded( + srv, "find", find_argv, NULL, filelist, 0, 0, deadline_ms, true, false, + &cancellation_latched, &deadline_latched, &output_limit_latched, result_out); + if (cause != MCP_SCAN_SUCCESS) { + return cause; + } + + const char *sort_argv[] = {"sort", "-z", "-o", filelist, filelist, NULL}; + const char *sort_env[] = {"LC_ALL=C", NULL}; + cause = mcp_run_argv_stage_cancellable_bounded( + srv, "sort", sort_argv, sort_env, NULL, 0, 0, deadline_ms, true, false, + &cancellation_latched, &deadline_latched, &output_limit_latched, result_out); + if (cause != MCP_SCAN_SUCCESS) { + return cause; + } + + if (!mcp_command_output_path(output_path)) { + return MCP_SCAN_COMMAND_FAILURE; + } + FILE *out = cbm_fopen(output_path, "wb"); + if (!out) { + (void)cbm_unlink(output_path); + output_path[0] = '\0'; + return MCP_SCAN_COMMAND_FAILURE; + } + FILE *list = cbm_fopen(filelist, "rb"); + if (!list) { + (void)fclose(out); + (void)cbm_unlink(output_path); + output_path[0] = '\0'; + return MCP_SCAN_COMMAND_FAILURE; + } + + const char *grep_flag = use_regex ? "-E" : "-F"; + const char **chunk_argv = + malloc(((size_t)MCP_ARGV_SCAN_GREP_CHUNK + 7) * sizeof(*chunk_argv)); + char **chunk_paths = malloc((size_t)MCP_ARGV_SCAN_GREP_CHUNK * sizeof(*chunk_paths)); + char *pathbuf = malloc(CBM_SZ_4K); + if (!chunk_argv || !chunk_paths || !pathbuf) { + free(chunk_argv); + free(chunk_paths); + free(pathbuf); + (void)fclose(list); + (void)fclose(out); + (void)cbm_unlink(output_path); + output_path[0] = '\0'; + return MCP_SCAN_COMMAND_FAILURE; + } + size_t pathcap = CBM_SZ_4K; + size_t pathlen = 0; + uint64_t output_bytes = 0; + size_t seq = 0; + cause = MCP_SCAN_SUCCESS; + bool io_error = false; + while (cause == MCP_SCAN_SUCCESS) { + size_t n = 0; + while (n < MCP_ARGV_SCAN_GREP_CHUNK) { + int c = fgetc(list); + if (c == EOF) { + if (ferror(list)) { + io_error = true; + } + break; + } + if (c == '\0') { + if (pathlen > 0) { + pathbuf[pathlen] = '\0'; + chunk_paths[n] = cbm_strdup(pathbuf); + if (!chunk_paths[n]) { + io_error = true; + break; + } + n++; + pathlen = 0; + } + continue; + } + if (pathlen + 1 >= pathcap) { + size_t newcap = pathcap * 2; + char *grown = realloc(pathbuf, newcap); + if (!grown) { + io_error = true; + break; + } + pathbuf = grown; + pathcap = newcap; + } + pathbuf[pathlen++] = (char)c; + } + if (io_error || n == 0) { + for (size_t i = 0; i < n; i++) { + free(chunk_paths[i]); + } + break; + } + + char chunk_path[CBM_SZ_2K]; + int written = snprintf(chunk_path, sizeof(chunk_path), "%s.grep%zu", output_path, seq++); + if (written <= 0 || (size_t)written >= sizeof(chunk_path)) { + io_error = true; + } else { + chunk_argv[0] = "grep"; + chunk_argv[1] = "-Hn"; + chunk_argv[2] = grep_flag; + chunk_argv[3] = "-f"; + chunk_argv[4] = pattern_file; + chunk_argv[5] = "--"; + for (size_t i = 0; i < n; i++) { + chunk_argv[6 + i] = chunk_paths[i]; + } + chunk_argv[6 + n] = NULL; + cause = mcp_run_argv_stage_cancellable_bounded( + srv, "grep", chunk_argv, NULL, chunk_path, output_bytes, output_limit, + deadline_ms, true, true, &cancellation_latched, &deadline_latched, + &output_limit_latched, result_out); + if (cause == MCP_SCAN_SUCCESS) { + FILE *chunk = cbm_fopen(chunk_path, "rb"); + if (!chunk) { + cause = MCP_SCAN_COMMAND_FAILURE; + } else { + char copybuf[CBM_SZ_16K]; + size_t got; + while ((got = fread(copybuf, 1, sizeof(copybuf), chunk)) > 0) { + if (fwrite(copybuf, 1, got, out) != got) { + cause = MCP_SCAN_COMMAND_FAILURE; + break; + } + output_bytes += got; + } + if (ferror(chunk) && cause == MCP_SCAN_SUCCESS) { + cause = MCP_SCAN_COMMAND_FAILURE; + } + (void)fclose(chunk); + } + } + (void)cbm_unlink(chunk_path); + } + for (size_t i = 0; i < n; i++) { + free(chunk_paths[i]); + } + } + free(chunk_argv); + free(chunk_paths); + free(pathbuf); + (void)fclose(list); + if (fclose(out) != 0 && cause == MCP_SCAN_SUCCESS) { + cause = MCP_SCAN_COMMAND_FAILURE; + } + if (io_error && cause == MCP_SCAN_SUCCESS) { + cause = MCP_SCAN_COMMAND_FAILURE; + } + if (cause == MCP_SCAN_SUCCESS && output_limit > 0 && output_bytes > output_limit) { + cause = MCP_SCAN_OUTPUT_LIMIT; + } + if (cause != MCP_SCAN_SUCCESS) { + (void)cbm_unlink(output_path); + output_path[0] = '\0'; + } + return cause; +} +#endif + /* Does `node`'s line range overlap any recorded hunk for `file`? Used to scope * seed detection to the actually-changed lines rather than the whole file. * Non-static (declared in mcp_internal.h) so tests can exercise the overlap diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 80a320ca5..57262d8a6 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -12420,6 +12420,141 @@ TEST(search_code_output_limit_fails_closed_and_cleans_scan) { PASS(); } +/* #2634: the POSIX unscoped scan runs find/sort/grep as argv stages with no + * shell, so file names carrying shell metacharacters are plain operands, not + * command text. Pin that such files are discovered and scanned. */ +TEST(search_code_unscoped_argv_scans_metachar_filenames_issue2634) { +#ifdef _WIN32 + SKIP_PLATFORM("argv scan stages are POSIX-only"); +#else + char tmp[CBM_SZ_512]; + snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-meta-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + + char hostile[CBM_SZ_1K]; + snprintf(hostile, sizeof(hostile), "%s/we'ird ;na`me$.go", tmp); + ASSERT_EQ(th_write_file(hostile, + "package main\n\nfunc HandleHostile2634() error {\n\treturn nil\n}\n"), + 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + const char *project = "search-argv-metachar"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK); + + char *resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"HandleHostile2634\"," + "\"project\":\"search-argv-metachar\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "we'ird ;na`me$.go")); + ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":1")); + free(inner); + free(resp); + cbm_mcp_server_free(srv); + ASSERT_EQ(th_rmtree(tmp), 0); + PASS(); +#endif +} + +/* #2634: grep's exit 1 (no match in a chunk) is an ordinary empty result, not + * a scan failure — the same contract the retired xargs wrapper encoded. */ +TEST(search_code_unscoped_argv_no_match_is_success_issue2634) { +#ifdef _WIN32 + SKIP_PLATFORM("argv scan stages are POSIX-only"); +#else + char tmp[CBM_SZ_512]; + snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-nomatch-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + + char src_path[CBM_SZ_1K]; + snprintf(src_path, sizeof(src_path), "%s/handler.go", tmp); + ASSERT_EQ(th_write_file(src_path, + "package main\n\nfunc HandlePresent2634() error {\n\treturn nil\n}\n"), + 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + const char *project = "search-argv-nomatch"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK); + + char *resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"NoSuchSymbol2634\"," + "\"project\":\"search-argv-nomatch\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":0")); + ASSERT_NOT_NULL(strstr(inner, "\"total_relation\":\"eq\"")); + free(inner); + free(resp); + cbm_mcp_server_free(srv); + ASSERT_EQ(th_rmtree(tmp), 0); + PASS(); +#endif +} + +/* #2634: the unscoped file_pattern reaches find as a -name operand, so a + * non-matching extension is excluded from the scan without any shell glob. */ +TEST(search_code_unscoped_argv_honours_file_pattern_issue2634) { +#ifdef _WIN32 + SKIP_PLATFORM("argv scan stages are POSIX-only"); +#else + char tmp[CBM_SZ_512]; + snprintf(tmp, sizeof(tmp), "%s/cbm-search-argv-glob-XXXXXX", cbm_tmpdir()); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + + char py[CBM_SZ_1K]; + snprintf(py, sizeof(py), "%s/handler.py", tmp); + ASSERT_EQ(th_write_file(py, "def handle_python2634():\n return 1\n"), 0); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *store = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(store); + const char *project = "search-argv-glob"; + cbm_mcp_server_set_project(srv, project); + ASSERT_EQ(cbm_store_upsert_project(store, project, tmp), CBM_STORE_OK); + + char *resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"handle_python2634\"," + "\"project\":\"search-argv-glob\"," + "\"file_pattern\":\"*.go\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_TRUE(strstr(inner, "handler.py") == NULL); + ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":0")); + free(inner); + free(resp); + + resp = cbm_mcp_handle_tool(srv, "search_code", + "{\"pattern\":\"handle_python2634\"," + "\"project\":\"search-argv-glob\",\"format\":\"json\"}"); + ASSERT_NOT_NULL(resp); + ASSERT_TRUE(strstr(resp, "\"isError\":true") == NULL); + inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + ASSERT_NOT_NULL(strstr(inner, "handler.py")); + ASSERT_NOT_NULL(strstr(inner, "\"total_grep_matches\":1")); + free(inner); + free(resp); + cbm_mcp_server_free(srv); + ASSERT_EQ(th_rmtree(tmp), 0); + PASS(); +#endif +} + TEST(search_code_scan_deadline_fails_closed_and_resets) { mcp_search_cache_t cache; ASSERT_TRUE(mcp_search_cache_open(&cache, "cbm-search-deadline")); @@ -25224,6 +25359,9 @@ SUITE(mcp) { RUN_TEST(search_code_file_pattern_prefilter_boundaries); RUN_TEST(search_code_windows_scope_prefilter_removes_pipeline_filter); RUN_TEST(search_code_cancel_cleans_supervised_scan); + RUN_TEST(search_code_unscoped_argv_scans_metachar_filenames_issue2634); + RUN_TEST(search_code_unscoped_argv_no_match_is_success_issue2634); + RUN_TEST(search_code_unscoped_argv_honours_file_pattern_issue2634); RUN_TEST(search_code_output_limit_fails_closed_and_cleans_scan); RUN_TEST(search_code_scan_deadline_fails_closed_and_resets); RUN_TEST(search_code_scan_deadline_override_is_per_server); From 808100974cf89324bafdf03a3278fa83bd697ac1 Mon Sep 17 00:00:00 2001 From: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:06:43 +0200 Subject: [PATCH 05/19] feat(java): no name guess for calls on a proven-external receiver When the Java resolver resolves a call's receiver to a type outside the project (a JDK package, or a package no project file declares), the call gets strategy lsp_external. Both pipeline paths (pass_calls.c and pass_parallel.c) then look the target up strictly by QN through cbm_pipeline_java_external_target and skip the registry's name guess, so no edge is created to an unrelated project method of the same name. This is the rule the Rust path follows since #2053. Positive proof only: a receiver of unknown type keeps the existing guess, and a seeded JDK type that lacks the called method stays no_method_match. Type variables (T, ID) are never taken for package-qualified types. A package counts as the project's when any registry layer holds a project type in it (the layer above the shared registry is a per-file overlay), and a path-derived spelling counts as the project's also for a file at the repository root. java_receiver_is_external memoizes its answer per package. A type of the caller's own package, or of its own file, now shadows java.lang (JLS 6.4.1): java_resolve_type_name looks the JVM package up before the java.lang auto-import. Before the gate that wrong first answer was harmless, since the name guess still found the project class; with the gate a project `Enum` or `Process` would read as external. Only the parallel path builds the Java cross registry, so on the sequential path the gate does not fire yet; the next commit adds it. Tests: java_typed_calls_parallel (external receivers get no edge; a project receiver keeps its edge, also when its type is missing from a project package or shares its simple name with a java.lang type), parallel_java_external_target_contract. Signed-off-by: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> --- Makefile.cbm | 3 +- internal/cbm/lsp/java_lsp.c | 180 ++++++++++++++++++-- internal/cbm/lsp/java_lsp.h | 8 + src/pipeline/lsp_resolve.h | 12 ++ src/pipeline/pass_calls.c | 16 +- src/pipeline/pass_parallel.c | 9 +- tests/test_java_typed_calls.c | 300 ++++++++++++++++++++++++++++++++++ tests/test_main.c | 2 + tests/test_parallel.c | 31 ++++ 9 files changed, 540 insertions(+), 21 deletions(-) create mode 100644 tests/test_java_typed_calls.c diff --git a/Makefile.cbm b/Makefile.cbm index 3b93146f1..84137eb6e 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -846,7 +846,8 @@ TEST_PY_LSP_STRESS_SRCS = tests/test_py_lsp_stress.c TEST_PY_LSP_SCALE_SRCS = tests/test_py_lsp_scale.c TEST_TS_LSP_SRCS = tests/test_ts_lsp.c -TEST_JAVA_LSP_SRCS = tests/test_java_lsp.c tests/test_java_lsp_coverage.c +TEST_JAVA_LSP_SRCS = tests/test_java_lsp.c tests/test_java_lsp_coverage.c \ + tests/test_java_typed_calls.c TEST_KOTLIN_LSP_SRCS = tests/test_kotlin_lsp.c tests/test_kotlin_typed_calls.c TEST_RUST_LSP_SRCS = tests/test_rust_lsp.c diff --git a/internal/cbm/lsp/java_lsp.c b/internal/cbm/lsp/java_lsp.c index cf221f06e..437dd8285 100644 --- a/internal/cbm/lsp/java_lsp.c +++ b/internal/cbm/lsp/java_lsp.c @@ -510,7 +510,19 @@ const char *java_resolve_type_name(JavaLSPContext *ctx, const char *name) { } } - /* 3. java.lang auto-import. Try registered first, then the conventional QN. */ + /* 3. Java package prefix. A type of the own package shadows every + * on-demand import, java.lang's included (JLS 6.4.1). */ + if (ctx->package_name && ctx->package_name[0]) { + char buf[JAVA_LSP_BUF]; + int n = snprintf(buf, sizeof(buf), "%s.%s", ctx->package_name, name); + if (n > 0 && (size_t)n < sizeof(buf)) { + if (cbm_registry_lookup_type(ctx->registry, buf)) { + return cbm_arena_strdup(ctx->arena, buf); + } + } + } + + /* 4. java.lang auto-import. Try registered first, then the conventional QN. */ for (int i = 0; JAVA_LANG_TYPES[i]; i++) { if (strcmp(JAVA_LANG_TYPES[i], name) == 0) { char buf[JAVA_LSP_BUF]; @@ -521,7 +533,7 @@ const char *java_resolve_type_name(JavaLSPContext *ctx, const char *name) { } } - /* 4. On-demand imports. */ + /* 5. On-demand imports. */ for (int i = 0; i < ctx->import_count; i++) { if (ctx->import_kinds[i] != CBM_JAVA_IMPORT_ON_DEMAND) continue; @@ -534,17 +546,6 @@ const char *java_resolve_type_name(JavaLSPContext *ctx, const char *name) { } } - /* 5. Java package prefix. */ - if (ctx->package_name && ctx->package_name[0]) { - char buf[JAVA_LSP_BUF]; - int n = snprintf(buf, sizeof(buf), "%s.%s", ctx->package_name, name); - if (n > 0 && (size_t)n < sizeof(buf)) { - if (cbm_registry_lookup_type(ctx->registry, buf)) { - return cbm_arena_strdup(ctx->arena, buf); - } - } - } - /* Exact already-qualified name. */ if (cbm_registry_lookup_type(ctx->registry, name)) { return cbm_arena_strdup(ctx->arena, name); @@ -969,6 +970,12 @@ const CBMRegisteredFunc *java_lookup_method(JavaLSPContext *ctx, const char *cla return fallback; } +/* A type declared in the project: the cross registrar and the own-file pass + * stamp namespace_qn on every project type; the JDK seed leaves it NULL. */ +static bool java_is_project_type(const CBMRegisteredType *rt) { + return rt && rt->namespace_qn && rt->namespace_qn[0]; +} + /* ── Method-invocation evaluation ─────────────────────────────────── */ static const CBMType *java_return_type_of(const CBMRegisteredFunc *f) { @@ -1797,11 +1804,135 @@ void java_lsp_process_file(JavaLSPContext *ctx, TSNode root) { /* ── Call-edge resolution ─────────────────────────────────────────── */ +/* True when `callee_qn` names a member (or a constructor) of a registered type + * the project does not declare — a JDK seed type. Such a target has no graph + * node, and the pipeline must not hand it to a same-named project method. */ +static bool java_callee_is_external(const JavaLSPContext *ctx, const char *callee_qn) { + if (!ctx->registry || !callee_qn) + return false; + const CBMRegisteredType *self = cbm_registry_lookup_type(ctx->registry, callee_qn); + if (self) + return !java_is_project_type(self); + const char *dot = strrchr(callee_qn, '.'); + if (!dot || dot == callee_qn) + return false; + char owner[JAVA_LSP_BUF]; + size_t n = (size_t)(dot - callee_qn); + if (n >= sizeof(owner)) + return false; + memcpy(owner, callee_qn, n); + owner[n] = '\0'; + const CBMRegisteredType *rt = cbm_registry_lookup_type(ctx->registry, owner); + return rt && !java_is_project_type(rt); +} + +/* `T`, `ID`, `SELF`: a type-variable spelling — one or two characters, or + * capitals, digits and '_' only — that the file imports neither by name nor + * through its package (`import org.jooq.impl.*;` makes `DSL` a type). A type + * variable the resolver failed to bind can reach a receiver qualified by some + * package; it is unknown, not a library type. */ +static bool java_is_type_var_spelling(const JavaLSPContext *ctx, const char *pkg, size_t pkg_len, + const char *simple) { + size_t n = strlen(simple); + bool caps = n > 0; + for (size_t i = 0; i < n && caps; i++) { + caps = isupper((unsigned char)simple[i]) || isdigit((unsigned char)simple[i]) || + simple[i] == '_'; + } + if (!caps && n > 2) { + return false; + } + for (int i = 0; i < ctx->import_count; i++) { + const char *target = ctx->import_target_qns[i]; + if (ctx->import_kinds[i] == CBM_JAVA_IMPORT_TYPE && ctx->import_local_names[i] && + strcmp(ctx->import_local_names[i], simple) == 0) { + return false; + } + if (ctx->import_kinds[i] == CBM_JAVA_IMPORT_ON_DEMAND && target && + strlen(target) == pkg_len && strncmp(target, pkg, pkg_len) == 0) { + return false; + } + } + return true; +} + +/* Whether the shared registry holds no project type under `prefix` + * ("org.jooq."), memoised per package. */ +static bool java_package_is_external(JavaLSPContext *ctx, const CBMTypeRegistry *base, + const char *prefix) { + for (int i = 0; i < JAVA_EXT_MEMO; i++) { + if (ctx->ext_memo_pkg[i] && strcmp(ctx->ext_memo_pkg[i], prefix) == 0) { + return ctx->ext_memo_val[i]; + } + } + CBMArena scratch; + cbm_arena_init(&scratch); + CBMQNPrefixIter it; + bool project = false; + /* The prefix search reads one layer, and the layer above the shared + * registry is a per-file overlay that is often empty: walk the chain. */ + for (const CBMTypeRegistry *reg = base; reg && !project; reg = reg->fallback) { + if (!cbm_registry_types_with_qn_prefix(reg, prefix, true, &scratch, &it)) + continue; + for (int ti = cbm_qn_prefix_iter_next(&it); ti >= 0 && !project; + ti = cbm_qn_prefix_iter_next(&it)) { + project = java_is_project_type(®->types[ti]); + } + } + cbm_arena_destroy(&scratch); + int slot = ctx->ext_memo_next++ % JAVA_EXT_MEMO; + ctx->ext_memo_pkg[slot] = prefix; + ctx->ext_memo_val[slot] = !project; + return !project; +} + +/* A receiver type the registry does not know, spelled with a package the + * project declares no type in (`org.jooq.DSLContext` from an import): a + * library type. The package is the segments before the first capitalised + * one, so a nested type (`com.acme.Order.OrderBuilder`, registered flattened + * as `com.acme.OrderBuilder`) is judged by `com.acme`. Cross-file runs only — + * the shared registry is the one that knows every project package, and its + * type order makes the probe a binary search. A path-derived QN + * (project-prefixed) is an unresolved project spelling, never a library. */ +static bool java_receiver_is_external(JavaLSPContext *ctx, const char *recv_qn) { + const CBMTypeRegistry *base = ctx->registry ? ctx->registry->fallback : NULL; + if (!base || !recv_qn || !islower((unsigned char)recv_qn[0])) + return false; + const char *type = NULL; + for (const char *p = recv_qn; (p = strchr(p, '.')) != NULL; p++) { + if (isupper((unsigned char)p[1])) { + type = p + 1; + break; + } + } + if (!type) + return false; + size_t pkg_len = (size_t)(type - 1 - recv_qn); + const char *last = strrchr(recv_qn, '.'); + if (java_is_type_var_spelling(ctx, recv_qn, pkg_len, last + 1)) + return false; + /* The project segment; a file at the repository root has a module QN + * that is the project name alone. */ + const char *mod_dot = ctx->module_qn ? strchr(ctx->module_qn, '.') : NULL; + size_t mod_len = mod_dot ? (size_t)(mod_dot - ctx->module_qn) + : (ctx->module_qn ? strlen(ctx->module_qn) : 0); + if (mod_len && strncmp(recv_qn, ctx->module_qn, mod_len) == 0 && recv_qn[mod_len] == '.') + return false; + if (ctx->package_name && strlen(ctx->package_name) == pkg_len && + strncmp(ctx->package_name, recv_qn, pkg_len) == 0) + return false; + /* "org.jooq." */ + return java_package_is_external(ctx, base, cbm_arena_strndup(ctx->arena, recv_qn, pkg_len + 1)); +} + static void java_emit_resolved_orig_kind(JavaLSPContext *ctx, const char *callee_qn, const char *orig, const char *strategy, float confidence, CBMResolvedKind kind) { if (!ctx->resolved_calls || !ctx->enclosing_method_qn || !callee_qn) return; + if (kind == CBM_RESOLVED_INVOCATION && java_callee_is_external(ctx, callee_qn)) { + strategy = "lsp_external"; + } CBMResolvedCall rc = {0}; rc.caller_qn = ctx->enclosing_method_qn; rc.callee_qn = callee_qn; @@ -2145,6 +2276,15 @@ static void resolve_method_call(JavaLSPContext *ctx, TSNode call) { "lsp_interface_dispatch", 0.80f); return; } + /* A library type the registry does not hold: no method of it can be + * looked up, but no project method is its target either. A seeded + * JDK type with no such method stays a diagnostic below, as Rust's + * gate (#2053) needs a positively resolved std method. */ + if (!rt && java_receiver_is_external(ctx, recv_qn)) { + java_emit_resolved(ctx, cbm_arena_sprintf(ctx->arena, "%s.%s", recv_qn, mname), + "lsp_external", 0.90f); + return; + } java_emit_unresolved(ctx, cbm_arena_sprintf(ctx->arena, "%s.%s", recv_qn, mname), "no_method_match"); return; @@ -3214,6 +3354,9 @@ static void register_local_func_or_type_from_file(JavaLSPContext *ctx, CBMTypeRe stub.qualified_name = d->qualified_name; stub.short_name = d->name; stub.is_interface = (strcmp(d->label, "Interface") == 0); + stub.namespace_qn = (ctx->package_name && ctx->package_name[0]) ? ctx->package_name + : (module_qn && module_qn[0]) ? module_qn + : d->qualified_name; cbm_registry_add_type(reg, stub); } @@ -3740,6 +3883,11 @@ void cbm_java_register_lsp_defs(CBMArena *arena, CBMTypeRegistry *reg, const CBM rt.qualified_name = d->qualified_name; rt.short_name = d->short_name; rt.is_interface = (strcmp(d->label, "Interface") == 0) || d->is_interface; + /* Project marker (java_is_project_type) and the package that + * qualifies the type's own member signatures. */ + rt.namespace_qn = (d->namespace_name && d->namespace_name[0]) ? d->namespace_name + : (d->def_module_qn && d->def_module_qn[0]) ? d->def_module_qn + : d->qualified_name; /* Embedded types from "|"-separated text. */ if (d->embedded_types && d->embedded_types[0]) { int n = 1; @@ -3883,6 +4031,9 @@ CBMTypeRegistry *cbm_java_build_cross_registry(CBMArena *arena, CBMLSPDef *defs, cbm_java_register_lsp_defs(arena, reg, jvm + type_count, total - type_count); } cbm_registry_finalize(reg); + /* java_receiver_is_external asks "does the project declare a type in + * this package?" per unresolved receiver: a binary search on this order. */ + cbm_registry_build_type_qn_order(reg, arena); reg->read_only = true; /* seal: shared Tier-2 registry is read-only during resolve */ return reg; } @@ -3958,6 +4109,9 @@ void cbm_run_java_lsp_cross_with_registry(CBMArena *arena, CBMFileResult *result } } +/* The per-file fallback when no shared cross registry was built. Its + * registry has no fallback layer, so java_receiver_is_external never fires + * here: library calls keep the textual registry fallback, as before. */ void cbm_run_java_lsp_cross(CBMArena *arena, const char *source, int source_len, const char *module_qn, CBMLSPDef *defs, int def_count, const char **import_names, const char **import_qns, int import_count, diff --git a/internal/cbm/lsp/java_lsp.h b/internal/cbm/lsp/java_lsp.h index ea304c64b..18d064235 100644 --- a/internal/cbm/lsp/java_lsp.h +++ b/internal/cbm/lsp/java_lsp.h @@ -35,6 +35,8 @@ #include "../cbm.h" #include "go_lsp.h" /* CBMLSPDef, CBMResolvedCallArray reused across languages */ +#define JAVA_EXT_MEMO 8 + /* Java `use`-style import kinds. Mirrors PHP's enum with Java semantics. */ enum { CBM_JAVA_IMPORT_TYPE = 0, /* import com.foo.Bar; — type import */ @@ -90,6 +92,12 @@ typedef struct { /* Debug mode (CBM_LSP_DEBUG env). */ bool debug; + + /* java_receiver_is_external's answers for the packages probed so far + * (a few per file), so each costs one registry walk. */ + const char *ext_memo_pkg[JAVA_EXT_MEMO]; + bool ext_memo_val[JAVA_EXT_MEMO]; + int ext_memo_next; } JavaLSPContext; /* ── Initialization / configuration ───────────────────────────────── */ diff --git a/src/pipeline/lsp_resolve.h b/src/pipeline/lsp_resolve.h index 419f29e26..10df40a80 100644 --- a/src/pipeline/lsp_resolve.h +++ b/src/pipeline/lsp_resolve.h @@ -635,6 +635,18 @@ static inline bool cbm_pipeline_kotlin_external_target(CBMLanguage lang, const c strncmp(callee_qn, "javax.", strlen("javax.")) == 0; } +/* Java: the LSP typed the receiver as a type OUTSIDE the project (a JDK or + * library class, `lsp_external`), so the callee is `java.util.Optional.map` + * and no project method. Without this gate the tail/unique-leaf node policy + * and then the textual registry bound such a call to whatever project method + * shares the leaf (`Optional.map` -> `Filters.map`). Such a call gets no + * CALLS edge, as #2053 does for Rust. Pure; unit-tested. */ +static inline bool cbm_pipeline_java_external_target(CBMLanguage lang, const CBMResolvedCall *lsp) { + return lang == CBM_LANG_JAVA && lsp && lsp->strategy && + strcmp(lsp->strategy, "lsp_external") == 0 && lsp->callee_qn && + strchr(lsp->callee_qn, '.') != NULL; +} + /* Rust (#2053): true when the Rust LSP positively resolved a call to a * registered symbol that lives OUTSIDE the project — a std/core/alloc seed * method (`root.join()` on `root: &Path` -> std.path.Path.join) or a seeded diff --git a/src/pipeline/pass_calls.c b/src/pipeline/pass_calls.c index f3570904d..e087b6073 100644 --- a/src/pipeline/pass_calls.c +++ b/src/pipeline/pass_calls.c @@ -892,14 +892,20 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBM * Service/route classification below still runs. Mirrors pass_parallel.c. */ bool php_vendor_call = cbm_pipeline_php_vendor_call(php_vendor, result, call) || cbm_pipeline_lsp_external_receiver(lsp); + /* A Java call on a JDK/library type is never bound to a project method + * found by name (no tail match, no registry guess). MUST match + * pass_parallel.c. */ + bool java_external = cbm_pipeline_java_external_target(lang, lsp); if (lsp) { bool exact_external_target = call->requires_lsp_resolution && cbm_pipeline_kotlin_external_target(lang, lsp->callee_qn); const cbm_gbuf_node_t *target_node = - exact_external_target ? cbm_pipeline_lsp_target_node_strict( - ctx->gbuf, ctx->project_name, lsp->callee_qn, allow_tail) - : cbm_pipeline_lsp_target_node(ctx->gbuf, ctx->project_name, - lsp->callee_qn, allow_tail); + java_external ? cbm_pipeline_lsp_target_node_strict(ctx->gbuf, ctx->project_name, + lsp->callee_qn, false) + : exact_external_target ? cbm_pipeline_lsp_target_node_strict( + ctx->gbuf, ctx->project_name, lsp->callee_qn, allow_tail) + : cbm_pipeline_lsp_target_node(ctx->gbuf, ctx->project_name, + lsp->callee_qn, allow_tail); if (target_node && source_node->id != target_node->id) { cbm_resolution_t res = {0}; /* Use the gbuf node's QN so downstream edge props show the canonical @@ -988,7 +994,7 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call, const CBM bool rust_external = lsp && cbm_pipeline_rust_external_target( lang, lsp->strategy, lsp->callee_qn, ctx->project_name); cbm_resolution_t res = {0}; - if (!rust_external && !php_vendor_call) { + if (!rust_external && !php_vendor_call && !java_external) { /* Python LEGB fallback first (MUST match pass_parallel.c). */ res = cbm_registry_resolve_call_scoped(ctx->registry, call->callee_name, module_qn, imp_keys, diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index 73ce742ad..fd8565c48 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -3271,13 +3271,18 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB memory_order_relaxed); _rc_t0 = extract_now_ns(); const cbm_gbuf_node_t *lsp_target = NULL; + /* Mirrors pass_calls.c: a Java call on a JDK/library type is never + * bound to a project method found by name. */ + bool java_external = cbm_pipeline_java_external_target(lang, lsp); if (lsp) { /* Canonicalise to the gbuf node's QN so res.qualified_name matches * the gbuf even when the cross-file fallback had to prefix the * project name. */ bool exact_external_target = call->requires_lsp_resolution && cbm_pipeline_kotlin_external_target(lang, lsp->callee_qn); - lsp_target = exact_external_target + lsp_target = java_external ? cbm_pipeline_lsp_target_node_strict( + rc->main_gbuf, rc->project_name, lsp->callee_qn, false) + : exact_external_target ? cbm_pipeline_lsp_target_node_strict(rc->main_gbuf, rc->project_name, lsp->callee_qn, allow_tail) : cbm_pipeline_lsp_target_node(rc->main_gbuf, rc->project_name, @@ -3314,7 +3319,7 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB bool rust_external = lsp && cbm_pipeline_rust_external_target( lang, lsp->strategy, lsp->callee_qn, rc->project_name); if ((!res.qualified_name || !res.qualified_name[0]) && !call->requires_lsp_resolution && - !rust_external && !php_vendor_call) { + !rust_external && !php_vendor_call && !java_external) { /* Python LEGB fallback first (MUST match pass_calls.c). */ res = cbm_registry_resolve_call_scoped(rc->registry, call->callee_name, module_qn, imp_keys, imp_vals, imp_count, lang, diff --git a/tests/test_java_typed_calls.c b/tests/test_java_typed_calls.c new file mode 100644 index 000000000..8f478a5ad --- /dev/null +++ b/tests/test_java_typed_calls.c @@ -0,0 +1,300 @@ +/* + * test_java_typed_calls.c — Java CALLS edges follow the receiver's type + * through the whole pipeline (definitions → cross-file LSP → calls pass). + * + * One small project is indexed with cbm_pipeline_run and the CALLS edges of + * InvoiceService are read back from the store. Each project method carries a + * same-named decoy in an unrelated class (Filters), so a name-only guess is + * visible as an edge to Filters. Covered: + * - JDK and library receivers (Stream, List, org.jooq.DSLContext), whose + * calls get no edge instead of a project method of the same name; + * - a project receiver, whose call keeps its edge, also when its type is + * spelled with its package; + * - a receiver whose type is missing from the graph but whose package is + * the project's (a generated class that was not indexed, spelled with + * its package): unknown, not external, so its call keeps the name + * fallback; + * - a project type in the caller's own package that shares its simple + * name with a java.lang type (Process), or such a type declared in the + * caller's own file (Record): the own package wins. + * The fixture is padded past MIN_FILES_FOR_PARALLEL: only the parallel path + * builds the Java cross registry that proves a receiver external. + */ +#include "../src/foundation/compat.h" +#include "test_framework.h" +#include "test_helpers.h" +#include "pipeline/pipeline.h" +#include "sqlite3.h" + +#include +#include +#include + +/* ── Fixture ─────────────────────────────────────────────────────── */ + +static const char *const JT_CUSTOMER = "package com.acme.model;\n\n" + "public class Customer {\n" + " public String getName() { return \"\"; }\n" + "}\n"; + +/* Every method here shares a name with a real target elsewhere. */ +static const char *const JT_FILTERS = "package com.acme.other;\n\n" + "public class Filters {\n" + " public Filters filter(Object p) { return this; }\n" + " public Filters map(Object f) { return this; }\n" + " public String getName() { return \"\"; }\n" + " public Object select(Object a) { return a; }\n" + " public Object build() { return null; }\n" + " public String getCity() { return \"\"; }\n" + " public int size() { return 0; }\n" + " public int start() { return 0; }\n" + " public Filters number(String n) { return this; }\n" + " public void setPaid(boolean p) {}\n" + " public Object getCustomer() { return null; }\n" + " public String label() { return \"\"; }\n" + "}\n"; + +/* Process and Record share their simple names with java.lang types: the own + * package and the file's own types win (JLS 6.4.1), so these are the types + * InvoiceService and Process.start mean. */ +static const char *const JT_PROCESS = "package com.acme.svc;\n\n" + "public class Process {\n" + " public Process(String name) {}\n" + " public int start() { return new Record().id(); }\n" + "}\n\n" + "class Record {\n" + " int id() { return 0; }\n" + "}\n"; + +static const char *const JT_SERVICE = + "package com.acme.svc;\n\n" + "import com.acme.model.Customer;\n" + "import java.util.List;\n" + "import java.util.stream.Collectors;\n" + "import org.jooq.DSLContext;\n\n" + "public class InvoiceService {\n" + " private final DSLContext dsl;\n\n" + " public InvoiceService(DSLContext dsl) {\n" + " this.dsl = dsl;\n" + " }\n\n" + " public List numbers(List names) {\n" + " return names.stream().filter(n -> n.isEmpty()).map(n -> n.trim())\n" + " .collect(Collectors.toList());\n" + " }\n\n" + " public int touch(List names) {\n" + " dsl.select(\"a\");\n" + " return names.size();\n" + " }\n\n" + " public String customerName(Customer c) {\n" + " return c.getName();\n" + " }\n\n" + " public int run() {\n" + " Process p = new Process(\"job\");\n" + " return p.start();\n" + " }\n\n" + " public String qualifiedName(com.acme.other.Filters f) {\n" + " return f.label();\n" + " }\n\n" + " public String label() {\n" + " return \"\";\n" + " }\n\n" + " public String branchCity(com.acme.model.Branch b) {\n" + " return b.getCity();\n" + " }\n" + "}\n"; + +static void jt_write_fixture(const char *tmp, int filler) { + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/model/Customer.java"), JT_CUSTOMER); + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/other/Filters.java"), JT_FILTERS); + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/svc/InvoiceService.java"), JT_SERVICE); + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/svc/Process.java"), JT_PROCESS); + for (int i = 0; i < filler; i++) { + char rel[128]; + char body[256]; + snprintf(rel, sizeof(rel), "src/main/java/com/acme/fill/Fill%d.java", i); + snprintf(body, sizeof(body), + "package com.acme.fill;\n\npublic class Fill%d {\n" + " public int value%d() { return %d; }\n}\n", + i, i, i); + th_write_file(TH_PATH(tmp, rel), body); + } +} + +/* ── Store reads ─────────────────────────────────────────────────── */ + +typedef struct { + char tmp[256]; + char db_path[512]; + char project[512]; + sqlite3 *db; +} jt_index_t; + +static int jt_index(jt_index_t *ix, int filler) { + memset(ix, 0, sizeof(*ix)); + snprintf(ix->tmp, sizeof(ix->tmp), "/tmp/cbm_jtyped_XXXXXX"); + if (!cbm_mkdtemp(ix->tmp)) { + return -1; + } + jt_write_fixture(ix->tmp, filler); + snprintf(ix->db_path, sizeof(ix->db_path), "%s/jt.db", ix->tmp); + cbm_pipeline_t *p = cbm_pipeline_new(ix->tmp, ix->db_path, CBM_MODE_FULL); + if (!p) { + return -1; + } + int rc = cbm_pipeline_run(p); + snprintf(ix->project, sizeof(ix->project), "%s", cbm_pipeline_project_name(p)); + cbm_pipeline_free(p); + if (rc != 0 || sqlite3_open_v2(ix->db_path, &ix->db, SQLITE_OPEN_READONLY, NULL) != SQLITE_OK) { + return -1; + } + return 0; +} + +static void jt_close(jt_index_t *ix) { + if (ix->db) { + sqlite3_close(ix->db); + } + th_cleanup(ix->tmp); +} + +/* Targets of the CALLS edges from method `caller` whose textual callee ends + * in `.leaf` or is `leaf`, as "|" lines. */ +static int jt_targets(jt_index_t *ix, const char *caller, const char *leaf, char *out, + size_t out_len) { + out[0] = '\0'; + const char *sql = "SELECT t.qualified_name, json_extract(e.properties,'$.strategy'), " + "json_extract(e.properties,'$.callee') FROM edges e " + "JOIN nodes s ON s.id = e.source_id JOIN nodes t ON t.id = e.target_id " + "WHERE e.type = 'CALLS' AND s.name = ?1"; + sqlite3_stmt *st = NULL; + if (sqlite3_prepare_v2(ix->db, sql, -1, &st, NULL) != SQLITE_OK) { + return -1; + } + sqlite3_bind_text(st, 1, caller, -1, SQLITE_STATIC); + int n = 0; + size_t plen = strlen(ix->project); + size_t llen = strlen(leaf); + while (sqlite3_step(st) == SQLITE_ROW) { + const char *qn = (const char *)sqlite3_column_text(st, 0); + const char *strategy = (const char *)sqlite3_column_text(st, 1); + const char *callee = (const char *)sqlite3_column_text(st, 2); + if (!qn || !callee) { + continue; + } + size_t clen = strlen(callee); + bool leaf_match = (clen == llen && strcmp(callee, leaf) == 0) || + (clen > llen && callee[clen - llen - 1] == '.' && + strcmp(callee + clen - llen, leaf) == 0); + if (!leaf_match) { + continue; + } + if (strncmp(qn, ix->project, plen) == 0 && qn[plen] == '.') { + qn += plen + 1; + } + size_t used = strlen(out); + snprintf(out + used, out_len - used, "%s|%s\n", qn, strategy ? strategy : ""); + n++; + } + sqlite3_finalize(st); + return n; +} + +/* Exactly one CALLS edge for (caller, leaf), and its target QN ends in + * `want_suffix`. Prints what the graph has on failure. */ +static int jt_expect(jt_index_t *ix, const char *caller, const char *leaf, + const char *want_suffix) { + char got[4096]; + int n = jt_targets(ix, caller, leaf, got, sizeof(got)); + if (n == 1) { + const char *bar = strchr(got, '|'); + size_t qlen = bar ? (size_t)(bar - got) : strlen(got); + size_t wlen = strlen(want_suffix); + if (qlen >= wlen && strncmp(got + qlen - wlen, want_suffix, wlen) == 0) { + return 1; + } + } + printf("\n %s -> %s: want one edge to ...%s, have %d:\n%s", caller, leaf, want_suffix, n, got); + return 0; +} + +/* No CALLS edge for (caller, leaf) reaches a node whose QN contains `bad`. */ +static int jt_expect_none_to(jt_index_t *ix, const char *caller, const char *leaf, + const char *bad) { + char got[4096]; + (void)jt_targets(ix, caller, leaf, got, sizeof(got)); + if (!strstr(got, bad)) { + return 1; + } + printf("\n %s -> %s: an edge reaches %s:\n%s", caller, leaf, bad, got); + return 0; +} + +/* At least one CALLS edge for (caller, leaf). */ +static int jt_expect_some_edge(jt_index_t *ix, const char *caller, const char *leaf) { + char got[4096]; + if (jt_targets(ix, caller, leaf, got, sizeof(got)) > 0) { + return 1; + } + printf("\n %s -> %s: want an edge, have none\n", caller, leaf); + return 0; +} + +/* No CALLS edge at all for (caller, leaf). */ +static int jt_expect_no_edge(jt_index_t *ix, const char *caller, const char *leaf) { + char got[4096]; + int n = jt_targets(ix, caller, leaf, got, sizeof(got)); + if (n == 0) { + return 1; + } + printf("\n %s -> %s: want no edge, have %d:\n%s", caller, leaf, n, got); + return 0; +} + +/* ── Tests ───────────────────────────────────────────────────────── */ + +static int jt_check_external(jt_index_t *ix) { + int ok = 1; + /* JDK and library receivers never bind a project method by name... */ + ok &= jt_expect_none_to(ix, "numbers", "filter", "Filters"); + ok &= jt_expect_none_to(ix, "numbers", "map", "Filters"); + ok &= jt_expect_none_to(ix, "touch", "select", "Filters"); + ok &= jt_expect_none_to(ix, "touch", "size", "Filters"); + /* ...and get no edge: the graph holds no node for a library method. */ + ok &= jt_expect_no_edge(ix, "numbers", "filter"); + ok &= jt_expect_no_edge(ix, "touch", "select"); + ok &= jt_expect_no_edge(ix, "touch", "size"); + /* A project receiver keeps its edge. */ + ok &= jt_expect(ix, "customerName", "getName", "com.acme.model.Customer.getName"); + /* A project type spelled with its package and not imported binds by its + * type, where the name guess picks the caller's own label(). */ + ok &= jt_expect(ix, "qualifiedName", "label", "com.acme.other.Filters.label"); + /* An unknown type in a project package is not proven external. */ + ok &= jt_expect_some_edge(ix, "branchCity", "getCity"); + /* A project type named like a java.lang one, in the caller's package. */ + /* `new` on a project type binds its class node, as for any project type. */ + ok &= jt_expect(ix, "run", "Process", "com.acme.svc.Process"); + ok &= jt_expect(ix, "run", "start", "com.acme.svc.Process.start"); + ok &= jt_expect(ix, "start", "id", "Record.id"); + return ok; +} + +static int jt_run_all(int filler) { + jt_index_t ix; + if (jt_index(&ix, filler) != 0) { + printf("\n indexing the fixture failed\n"); + jt_close(&ix); + return 0; + } + int ok = jt_check_external(&ix); + jt_close(&ix); + return ok; +} + +TEST(java_typed_calls_parallel) { + ASSERT_TRUE(jt_run_all(60)); + PASS(); +} + +SUITE(java_typed_calls) { + RUN_TEST(java_typed_calls_parallel); +} diff --git a/tests/test_main.c b/tests/test_main.c index e0526cf85..e461ed45b 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -1811,6 +1811,7 @@ extern void suite_py_lsp_scale(void); extern void suite_ts_lsp(void); extern void suite_java_lsp(void); extern void suite_java_lsp_coverage(void); +extern void suite_java_typed_calls(void); extern void suite_kotlin_lsp(void); extern void suite_kotlin_typed_calls(void); extern void suite_rust_lsp(void); @@ -2403,6 +2404,7 @@ int main(int argc, char **argv) { RUN_SELECTED_SUITE(ts_lsp); RUN_SELECTED_SUITE(java_lsp); RUN_SELECTED_SUITE(java_lsp_coverage); + RUN_SELECTED_SUITE(java_typed_calls); /* Architecture + ADR + Louvain */ RUN_SELECTED_SUITE(store_arch); diff --git a/tests/test_parallel.c b/tests/test_parallel.c index a641e7e0e..7206f72c1 100644 --- a/tests/test_parallel.c +++ b/tests/test_parallel.c @@ -3730,6 +3730,36 @@ TEST(parallel_rust_external_target_contract) { PASS(); } +/* Contract for cbm_pipeline_java_external_target: only a Java row the + * resolver marked lsp_external, naming a qualified member, is external. The + * end-to-end probes live in test_java_typed_calls.c. */ +TEST(parallel_java_external_target_contract) { + CBMResolvedCall rc = {0}; + rc.callee_qn = "org.jooq.DSLContext.select"; + rc.strategy = "lsp_external"; + ASSERT_TRUE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + /* Every other strategy keeps the registry fallback. */ + rc.strategy = "lsp_type_dispatch"; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + rc.strategy = "lsp_unresolved"; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + /* A bare name is no member of any type. */ + rc.strategy = "lsp_external"; + rc.callee_qn = "select"; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + /* Per-language: no other language is affected. */ + rc.callee_qn = "org.jooq.DSLContext.select"; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_KOTLIN, &rc)); + /* Defensive NULL inputs. */ + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, NULL)); + rc.callee_qn = NULL; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + rc.callee_qn = "org.jooq.DSLContext.select"; + rc.strategy = NULL; + ASSERT_FALSE(cbm_pipeline_java_external_target(CBM_LANG_JAVA, &rc)); + PASS(); +} + /* Gate guard for the JVM-only unique-tail fallbacks (lsp_resolve.h). * * The tail fallbacks join LSP overrides across QN drift by unique @@ -5587,6 +5617,7 @@ SUITE(parallel) { RUN_TEST(parallel_cross_file_reread_preserves_unretained_edges); RUN_TEST(parallel_java_kotlin_lsp_override_cross_file_emits_lsp_strategy_edges); RUN_TEST(parallel_rust_external_target_contract); + RUN_TEST(parallel_java_external_target_contract); RUN_TEST(parallel_lsp_tail_match_fallbacks_gated_to_jvm); RUN_TEST(parallel_calls_parity); RUN_TEST(parallel_defines_parity); From 8e9caf941adfffe8791a2dfee1640420f159ff20 Mon Sep 17 00:00:00 2001 From: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:06:43 +0200 Subject: [PATCH 06/19] feat(java): build the Java cross registry on the sequential path pass_lsp_cross.c built the shared Java registry only on the parallel path (more than 50 files). On the sequential path every Java file was resolved against its own definitions alone, so a call on a type from another file fell back to the name guess, and the external-receiver gate of the previous commit never fired. This is a behaviour change for every repository of 50 files or fewer: calls whose receiver type is declared in another file now resolve through the LSP, as they do on the parallel path, and calls on JDK and library types lose their guessed edge. It also fixes the package-qualified parameter case measured in #2382. Tests: java_typed_calls_sequential (the parallel fixture on the sequential path, plus a parameter typed with its package-qualified name), java_typed_calls_paths_agree (both paths produce the same edges). Signed-off-by: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> --- src/pipeline/pass_lsp_cross.c | 1 + tests/test_java_typed_calls.c | 58 +++++++++++++++++++++++++++++++++-- 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/src/pipeline/pass_lsp_cross.c b/src/pipeline/pass_lsp_cross.c index 5aa955112..e69c07c8f 100644 --- a/src/pipeline/pass_lsp_cross.c +++ b/src/pipeline/pass_lsp_cross.c @@ -2592,6 +2592,7 @@ int cbm_pipeline_pass_lsp_cross(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t * cross_registries.c = cbm_c_build_cross_registry(xa, all_defs, def_count); cross_registries.cs = cbm_cs_build_cross_registry(xa, all_defs, def_count); cross_registries.ts = cbm_ts_build_cross_registry(xa, all_defs, def_count); + cross_registries.java = cbm_java_build_cross_registry(xa, all_defs, def_count); } int processed = 0; diff --git a/tests/test_java_typed_calls.c b/tests/test_java_typed_calls.c index 8f478a5ad..1fc5d6076 100644 --- a/tests/test_java_typed_calls.c +++ b/tests/test_java_typed_calls.c @@ -17,8 +17,8 @@ * - a project type in the caller's own package that shares its simple * name with a java.lang type (Process), or such a type declared in the * caller's own file (Record): the own package wins. - * The fixture is padded past MIN_FILES_FOR_PARALLEL: only the parallel path - * builds the Java cross registry that proves a receiver external. + * The same fixture runs on the sequential path and, padded past + * MIN_FILES_FOR_PARALLEL, on the parallel one: both must agree. */ #include "../src/foundation/compat.h" #include "test_framework.h" @@ -250,6 +250,33 @@ static int jt_expect_no_edge(jt_index_t *ix, const char *caller, const char *lea return 0; } +/* The CALLS edges out of the svc package, one sorted line each, with the + * project prefix dropped so two indexes compare. The Java resolver's + * strategies are kept; the textual registry's are one label, as its names + * already differ between the two paths before this resolver runs (a + * constructor is import_map on one and unique_name on the other). */ +static char *jt_edge_set(jt_index_t *ix) { + const char *sql = + "SELECT group_concat(x, char(10)) FROM (SELECT s.name || '|' || " + "coalesce(json_extract(e.properties,'$.callee'),'') || '|' || " + "substr(t.qualified_name, length(?1) + 2) || '|' || " + "CASE WHEN json_extract(e.properties,'$.strategy') LIKE 'lsp%' " + "THEN json_extract(e.properties,'$.strategy') ELSE 'registry' END AS x FROM edges e " + "JOIN nodes s ON s.id = e.source_id JOIN nodes t ON t.id = e.target_id " + "WHERE e.type = 'CALLS' AND s.file_path LIKE '%/svc/%' ORDER BY x)"; + sqlite3_stmt *st = NULL; + if (sqlite3_prepare_v2(ix->db, sql, -1, &st, NULL) != SQLITE_OK) { + return NULL; + } + sqlite3_bind_text(st, 1, ix->project, -1, SQLITE_STATIC); + char *out = NULL; + if (sqlite3_step(st) == SQLITE_ROW && sqlite3_column_text(st, 0)) { + out = strdup((const char *)sqlite3_column_text(st, 0)); + } + sqlite3_finalize(st); + return out; +} + /* ── Tests ───────────────────────────────────────────────────────── */ static int jt_check_external(jt_index_t *ix) { @@ -290,11 +317,38 @@ static int jt_run_all(int filler) { return ok; } +TEST(java_typed_calls_sequential) { + ASSERT_TRUE(jt_run_all(0)); + PASS(); +} + TEST(java_typed_calls_parallel) { ASSERT_TRUE(jt_run_all(60)); PASS(); } +/* The sequential and the parallel path produce the same svc edges. */ +TEST(java_typed_calls_paths_agree) { + jt_index_t seq; + jt_index_t par; + int seq_rc = jt_index(&seq, 0); + int par_rc = jt_index(&par, 60); + char *a = seq_rc == 0 ? jt_edge_set(&seq) : NULL; + char *b = par_rc == 0 ? jt_edge_set(&par) : NULL; + int same = a && b && strcmp(a, b) == 0; + if (!same) { + printf("\n sequential:\n%s\n parallel:\n%s\n", a ? a : "(none)", b ? b : "(none)"); + } + free(a); + free(b); + jt_close(&seq); + jt_close(&par); + ASSERT_TRUE(same); + PASS(); +} + SUITE(java_typed_calls) { + RUN_TEST(java_typed_calls_sequential); RUN_TEST(java_typed_calls_parallel); + RUN_TEST(java_typed_calls_paths_agree); } From 7e0f021bf35b9d808ba43435de3984bf7b8c205d Mon Sep 17 00:00:00 2001 From: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> Date: Thu, 22 Oct 2026 02:02:25 +0200 Subject: [PATCH 07/19] test(java): compare the two paths on edges found by type On Windows the parallel path binds `c.getName()` (Customer comes from an import) to Filters.getName: the textual registry's name guess picks another same-named method there. This PR does not change that guess, so the test no longer pins its target: - on the parallel path, customerName only has to keep an edge; the sequential path still checks Customer.getName; - java_typed_calls_paths_agree compares only the edges the Java resolver found by type (lsp_*), not the registry's. Signed-off-by: Timur Nizametdinov <7229951+tfactor2@users.noreply.github.com> --- tests/test_java_typed_calls.c | 43 +++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 19 deletions(-) diff --git a/tests/test_java_typed_calls.c b/tests/test_java_typed_calls.c index 1fc5d6076..0849f3acd 100644 --- a/tests/test_java_typed_calls.c +++ b/tests/test_java_typed_calls.c @@ -18,7 +18,9 @@ * name with a java.lang type (Process), or such a type declared in the * caller's own file (Record): the own package wins. * The same fixture runs on the sequential path and, padded past - * MIN_FILES_FOR_PARALLEL, on the parallel one: both must agree. + * MIN_FILES_FOR_PARALLEL, on the parallel one: both must agree on every edge + * found by type. A name guess is left out of that comparison: on Windows the + * parallel path can pick another same-named method for it. */ #include "../src/foundation/compat.h" #include "test_framework.h" @@ -250,20 +252,20 @@ static int jt_expect_no_edge(jt_index_t *ix, const char *caller, const char *lea return 0; } -/* The CALLS edges out of the svc package, one sorted line each, with the - * project prefix dropped so two indexes compare. The Java resolver's - * strategies are kept; the textual registry's are one label, as its names - * already differ between the two paths before this resolver runs (a - * constructor is import_map on one and unique_name on the other). */ +/* The CALLS edges out of the svc package that the Java resolver found by + * type, one sorted line each, with the project prefix dropped so two indexes + * compare. Edges of the textual registry are left out: its strategy names + * already differ between the two paths (a constructor is import_map on one + * and unique_name on the other), and so can its pick among same-named + * methods. */ static char *jt_edge_set(jt_index_t *ix) { - const char *sql = - "SELECT group_concat(x, char(10)) FROM (SELECT s.name || '|' || " - "coalesce(json_extract(e.properties,'$.callee'),'') || '|' || " - "substr(t.qualified_name, length(?1) + 2) || '|' || " - "CASE WHEN json_extract(e.properties,'$.strategy') LIKE 'lsp%' " - "THEN json_extract(e.properties,'$.strategy') ELSE 'registry' END AS x FROM edges e " - "JOIN nodes s ON s.id = e.source_id JOIN nodes t ON t.id = e.target_id " - "WHERE e.type = 'CALLS' AND s.file_path LIKE '%/svc/%' ORDER BY x)"; + const char *sql = "SELECT group_concat(x, char(10)) FROM (SELECT s.name || '|' || " + "coalesce(json_extract(e.properties,'$.callee'),'') || '|' || " + "substr(t.qualified_name, length(?1) + 2) || '|' || " + "json_extract(e.properties,'$.strategy') AS x FROM edges e " + "JOIN nodes s ON s.id = e.source_id JOIN nodes t ON t.id = e.target_id " + "WHERE e.type = 'CALLS' AND s.file_path LIKE '%/svc/%' " + "AND json_extract(e.properties,'$.strategy') LIKE 'lsp%' ORDER BY x)"; sqlite3_stmt *st = NULL; if (sqlite3_prepare_v2(ix->db, sql, -1, &st, NULL) != SQLITE_OK) { return NULL; @@ -279,7 +281,7 @@ static char *jt_edge_set(jt_index_t *ix) { /* ── Tests ───────────────────────────────────────────────────────── */ -static int jt_check_external(jt_index_t *ix) { +static int jt_check_external(jt_index_t *ix, int sequential) { int ok = 1; /* JDK and library receivers never bind a project method by name... */ ok &= jt_expect_none_to(ix, "numbers", "filter", "Filters"); @@ -290,8 +292,11 @@ static int jt_check_external(jt_index_t *ix) { ok &= jt_expect_no_edge(ix, "numbers", "filter"); ok &= jt_expect_no_edge(ix, "touch", "select"); ok &= jt_expect_no_edge(ix, "touch", "size"); - /* A project receiver keeps its edge. */ - ok &= jt_expect(ix, "customerName", "getName", "com.acme.model.Customer.getName"); + /* A project receiver keeps its edge. Its type comes from an import, which + * only the name guess reads here, as on main; on Windows the parallel + * path's guess can pick Filters.getName. */ + ok &= sequential ? jt_expect(ix, "customerName", "getName", "com.acme.model.Customer.getName") + : jt_expect_some_edge(ix, "customerName", "getName"); /* A project type spelled with its package and not imported binds by its * type, where the name guess picks the caller's own label(). */ ok &= jt_expect(ix, "qualifiedName", "label", "com.acme.other.Filters.label"); @@ -312,7 +317,7 @@ static int jt_run_all(int filler) { jt_close(&ix); return 0; } - int ok = jt_check_external(&ix); + int ok = jt_check_external(&ix, filler == 0); jt_close(&ix); return ok; } @@ -327,7 +332,7 @@ TEST(java_typed_calls_parallel) { PASS(); } -/* The sequential and the parallel path produce the same svc edges. */ +/* The sequential and the parallel path find the same svc edges by type. */ TEST(java_typed_calls_paths_agree) { jt_index_t seq; jt_index_t par; From 5c54201ef8ed8903ffdaf8f417308c9f679aa09c Mon Sep 17 00:00:00 2001 From: Shane McCarron Date: Thu, 29 Oct 2026 11:46:21 -0500 Subject: [PATCH 08/19] fix(watcher): pace re-indexes by the cost of the last successful run After a successful re-index the next poll waits max(interval, 3 x index duration), capped at 30 min, counted from when the index finished. A project whose full rebuild takes minutes no longer re-indexes back-to-back under continuous writes; indexing spends at most ~1/4 of wall time. Sub-second indexes keep the existing cadence; failure backoff and busy-skip are unchanged. Logs watcher.cooldown when the delay is extended. Refs #2835 Signed-off-by: Shane McCarron --- src/watcher/watcher.c | 44 +++++++++++++-- src/watcher/watcher.h | 9 ++++ tests/test_watcher.c | 123 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 172 insertions(+), 4 deletions(-) diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index ac7c47b7f..cef4fd482 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -154,6 +154,16 @@ struct cbm_watcher { * daemon log names the stuck project instead of only repeating the warning. */ #define INDEX_FAIL_SUSTAINED 10 +/* Success cooldown (#2835): after a successful re-index the next poll waits + * at least INDEX_COOLDOWN_FACTOR x the index's own duration, counted from when + * the index finished, so the watcher spends at most ~1/(K+1) of wall time + * indexing one project — a 5-minute rebuild idles 15 minutes. Sub-second + * indexes stay at the plain cadence. The ceiling bounds staleness after one + * pathological run; it is NOT INDEX_FAIL_CEILING_MS (5 min), which would cut + * the cooldown of any rebuild over 100 s below K x its duration. */ +#define INDEX_COOLDOWN_FACTOR 3 +#define INDEX_COOLDOWN_CEILING_MS 1800000 /* 30 min */ + /* Stale-root pruning (#286): a watched project whose root directory stays * missing is pruned — its cached DB is deleted and the watch entry removed. * Deletion is destructive (the DB can hold user-authored data such as the @@ -204,6 +214,20 @@ int cbm_watcher_index_backoff_ms(int interval_ms, int consecutive_failures) { return (int)(delay < interval_ms ? interval_ms : delay); } +int cbm_watcher_success_delay_ms(int interval_ms, int64_t index_elapsed_ms) { + if (interval_ms < 0) { + interval_ms = 0; + } + if (index_elapsed_ms <= 0) { + return interval_ms; + } + /* Divide instead of multiply so a huge elapsed cannot overflow. */ + int64_t cooldown = index_elapsed_ms > INDEX_COOLDOWN_CEILING_MS / INDEX_COOLDOWN_FACTOR + ? INDEX_COOLDOWN_CEILING_MS + : index_elapsed_ms * INDEX_COOLDOWN_FACTOR; + return (int)(cooldown > interval_ms ? cooldown : interval_ms); +} + int cbm_watcher_poll_interval_ms(int file_count) { int ms = POLL_BASE_MS + ((file_count / POLL_FILE_STEP) * CBM_MSEC_PER_SEC); if (ms > POLL_MAX_MS) { @@ -1693,12 +1717,22 @@ static void poll_project(const char *key, void *val, void *ud) { } cbm_log_info("watcher.changed", "project", s->project_name, "strategy", s->is_git ? "git" : "tree"); + int delay_ms = -1; /* < 0: use the failure-backoff cadence below */ + int64_t anchor_ns = ctx->now; /* success re-anchors to when the index finished */ if (ctx->w->index_fn) { + int64_t index_start = now_ns(); int rc = ctx->w->index_fn(s->project_name, s->root_path, ctx->w->user_data); if (rc == 0) { ctx->reindexed++; s->index_failures = 0; commit_baselines(ctx->w, s); + anchor_ns = now_ns(); + delay_ms = + cbm_watcher_success_delay_ms(s->interval_ms, (anchor_ns - index_start) / US_PER_MS); + if (delay_ms > s->interval_ms) { + cbm_log_info("watcher.cooldown", "project", s->project_name, "delay_ms", + itoa_buf(delay_ms)); + } } else if (rc > 0) { /* Busy-skip: baseline stays uncommitted, next poll retries. */ cbm_log_info("watcher.index.retry", "project", s->project_name); @@ -1721,10 +1755,12 @@ static void poll_project(const char *key, void *val, void *ud) { } } - /* Failures back off; success and busy-skip keep the adaptive cadence. */ - s->next_poll_ns = - ctx->now + - ((int64_t)cbm_watcher_index_backoff_ms(s->interval_ms, s->index_failures) * US_PER_MS); + /* Success pays back the index's cost; failures back off; busy-skip keeps + * the adaptive cadence. */ + if (delay_ms < 0) { + delay_ms = cbm_watcher_index_backoff_ms(s->interval_ms, s->index_failures); + } + s->next_poll_ns = anchor_ns + ((int64_t)delay_ms * US_PER_MS); } /* Callback to snapshot project state pointers into an array. */ diff --git a/src/watcher/watcher.h b/src/watcher/watcher.h index 2986cb57d..eb7e92915 100644 --- a/src/watcher/watcher.h +++ b/src/watcher/watcher.h @@ -195,6 +195,15 @@ int cbm_watcher_poll_interval_ms(int file_count); * Negative inputs are treated as zero. */ int cbm_watcher_index_backoff_ms(int interval_ms, int consecutive_failures); +/* Return the delay (ms) before the next poll after a SUCCESSFUL re-index that + * took `index_elapsed_ms`: max(interval_ms, INDEX_COOLDOWN_FACTOR (3) x + * index_elapsed_ms), with the cooldown term capped at INDEX_COOLDOWN_CEILING_MS + * (30 min). The caller counts it from when the index finished, so the watcher + * spends at most ~1/4 of wall time indexing a project; sub-second indexes keep + * `interval_ms`. + * Never below `interval_ms`; overflow-safe; negative inputs act as zero. */ +int cbm_watcher_success_delay_ms(int interval_ms, int64_t index_elapsed_ms); + /* Classify a stat() errno observed on a watched project root: returns true * only for values that mean the root itself is gone (ENOENT, ENOTDIR) and * may count toward stale-root pruning (#286). Any other failure (EACCES, diff --git a/tests/test_watcher.c b/tests/test_watcher.c index 8be483ca8..c2796e3a6 100644 --- a/tests/test_watcher.c +++ b/tests/test_watcher.c @@ -2260,6 +2260,124 @@ TEST(watcher_sustained_failure_logs_once_issue2015) { PASS(); } +/* ── Success cooldown (#2835) ─────────────────────────────────────── */ + +TEST(success_delay_fast_index_keeps_interval) { + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 0), 5000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 1), 5000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 1666), 5000); /* 3x < interval */ + ASSERT_EQ(cbm_watcher_success_delay_ms(60000, 20000), 60000); + PASS(); +} + +TEST(success_delay_slow_index_scales_by_factor) { + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 1667), 5001); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 10000), 30000); + /* 5-minute rebuild → next poll 15 min after it finishes. */ + ASSERT_EQ(cbm_watcher_success_delay_ms(60000, 300000), 900000); + PASS(); +} + +TEST(success_delay_clamped_and_degenerate) { + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 600000), 1800000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 600001), 1800000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, INT64_MAX), 1800000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, -1), 5000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, INT64_MIN), 5000); + ASSERT_EQ(cbm_watcher_success_delay_ms(-1, 0), 0); + /* Never below the caller's own cadence, even above the ceiling. */ + ASSERT_EQ(cbm_watcher_success_delay_ms(INT_MAX, INT64_MAX), INT_MAX); + PASS(); +} + +TEST(success_delay_is_monotonic) { + int previous = cbm_watcher_success_delay_ms(5000, 0); + for (int64_t e = 0; e <= 2000000; e += 997) { + int delay = cbm_watcher_success_delay_ms(5000, e); + ASSERT_TRUE(delay >= previous); + ASSERT_TRUE(delay >= 5000 && delay <= 1800000); + previous = delay; + } + PASS(); +} + +/* Wire-up: the cooldown must be measured around the real index_fn call. An + * instant callback keeps the plain cadence (no cooldown line); a callback + * slower than interval/3 (5000/3 ms) extends it and logs once. */ +static int cooldown_log_hits = 0; +static void cooldown_log_sink(const char *line) { + if (line && strstr(line, "watcher.cooldown") != NULL) { + cooldown_log_hits++; + } +} +static int slow_index_sleep_ms = 0; +static int slow_index_calls = 0; +static int slow_index_callback(const char *name, const char *path, void *ud) { + (void)name; + (void)path; + (void)ud; + slow_index_calls++; + if (slow_index_sleep_ms > 0) { + cbm_usleep((unsigned)slow_index_sleep_ms * 1000U); + } + return 0; +} + +TEST(watcher_success_cooldown_measures_index_issue2835) { + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_cool_XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + if (wt_git(tmpdir, "init -q") != 0) { + th_rmtree(tmpdir); + FAIL("git init failed"); + } + char p[300]; + th_write_file(wt_path(p, sizeof(p), tmpdir, "file.txt"), "hello\n"); + wt_git(tmpdir, "add file.txt"); + wt_git(tmpdir, "commit -q -m init"); + + cbm_store_t *store = cbm_store_open_memory(); + cbm_watcher_t *w = cbm_watcher_new(store, slow_index_callback, NULL); + cbm_watcher_watch(w, "cool-repo", tmpdir); + slow_index_calls = 0; + slow_index_sleep_ms = 0; + cbm_watcher_poll_once(w); /* baseline */ + + CBMLogLevel old_level = cbm_log_get_level(); + cbm_log_set_level(CBM_LOG_INFO); + cooldown_log_hits = 0; + cbm_log_set_sink(cooldown_log_sink); + + /* Instant index: no cooldown. Loop on the attempt count (a transient git + * probe failure makes a poll a no-op, see the sustained-failure test). */ + th_append_file(wt_path(p, sizeof(p), tmpdir, "file.txt"), "a\n"); + while (slow_index_calls < 1) { + cbm_watcher_touch(w, "cool-repo"); + cbm_watcher_poll_once(w); + } + int fast_hits = cooldown_log_hits; + + /* 1.8 s index → 3 x 1800 = 5400 ms > 5000 ms interval: cooldown. */ + slow_index_sleep_ms = 1800; + th_append_file(wt_path(p, sizeof(p), tmpdir, "file.txt"), "b\n"); + while (slow_index_calls < 2) { + cbm_watcher_touch(w, "cool-repo"); + cbm_watcher_poll_once(w); + } + + cbm_log_set_sink(NULL); + cbm_log_set_level(old_level); + + ASSERT_EQ(fast_hits, 0); + ASSERT_EQ(cooldown_log_hits, 1); + + cbm_watcher_free(w); + cbm_store_close(store); + th_rmtree(tmpdir); + PASS(); +} + TEST(watcher_index_failure_count_unknown_project) { cbm_store_t *store = cbm_store_open_memory(); cbm_watcher_t *w = cbm_watcher_new(store, index_callback, NULL); @@ -4043,6 +4161,11 @@ SUITE(watcher) { RUN_TEST(index_backoff_never_schedules_sooner_than_the_interval); RUN_TEST(watcher_index_failure_streak_increments_and_resets_issue2015); RUN_TEST(watcher_index_failure_backoff_gates_repolling_issue2015); + RUN_TEST(success_delay_fast_index_keeps_interval); + RUN_TEST(success_delay_slow_index_scales_by_factor); + RUN_TEST(success_delay_clamped_and_degenerate); + RUN_TEST(success_delay_is_monotonic); + RUN_TEST(watcher_success_cooldown_measures_index_issue2835); RUN_TEST(watcher_sustained_failure_logs_once_issue2015); RUN_TEST(watcher_index_failure_count_unknown_project); RUN_TEST(watcher_project_info_publishes_scan_issue2167); From eaf5b865d5af9cc6938d6076052937474a2654f5 Mon Sep 17 00:00:00 2001 From: tahodev <85001874+tahodev@users.noreply.github.com> Date: Mon, 2 Nov 2026 05:27:48 +0900 Subject: [PATCH 09/19] Feed the unscoped search file list to search_scan_in_process instead of chunked grep Signed-off-by: tahodev <85001874+tahodev@users.noreply.github.com> --- src/mcp/mcp.c | 165 ++++++-------------------------------------------- 1 file changed, 20 insertions(+), 145 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 504e1d83a..5c1ee41e1 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -17326,10 +17326,11 @@ static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( bool *cancellation_latched, bool *deadline_latched, bool *output_limit_latched, cbm_proc_result_t *result_out); -/* POSIX-only: the unscoped search_code scan as find/sort/grep argv stages. */ +/* POSIX-only: the unscoped search_code scan as find/sort argv stages feeding the + * in-process scanner. */ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, const char *file_pattern, - const char *pattern_file, const char *filelist, + const char *pattern, const char *filelist, const char *root_path, char output_path[CBM_SZ_2K], size_t output_limit, uint64_t deadline_ms, bool deadline_latched, @@ -17741,7 +17742,7 @@ static char *handle_search_code(cbm_mcp_server_t *srv, const char *args) { * shell parses search scopes or file patterns anymore (#2634). * The command override keeps the shell runner for tests. */ scan_cause = mcp_run_search_scan_argv( - srv, use_regex, file_pattern, tmpfile, filelist, root_path, output_path, + srv, use_regex, file_pattern, pattern, filelist, root_path, output_path, scan_output_limit, scan_deadline_ms, scan_deadline_latched, &scan_result); } else #endif @@ -18360,20 +18361,16 @@ static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( : MCP_SCAN_CONTAINED_COMMAND_FAILURE; } -/* Number of file operands per grep spawn. Bounded so argv stays far below - * ARG_MAX even with long paths, like the xargs batching it replaces. */ -#define MCP_ARGV_SCAN_GREP_CHUNK 256 - -/* The unscoped (unindexed or not-fully-indexed root) search_code scan as three - * argv stages, with no shell anywhere: find writes the NUL-delimited candidate - * list, LC_ALL=C sort orders it in place, and grep scans it in chunks. Search - * scopes, file patterns and the pattern file now reach the tools as plain - * arguments instead of shell-string interpolations (#2634). Exit codes keep - * the old pipeline's contract: find/sort failures fail closed, grep's exit 1 - * is an ordinary no-match, and an empty list is a successful empty scan. */ +/* The unscoped (unindexed or not-fully-indexed root) search_code scan with no + * shell anywhere: find writes the NUL-delimited candidate list and LC_ALL=C + * sort orders it in place, both as argv stages, then the list feeds the + * in-process scanner (search_scan_in_process). Search scopes and file patterns + * reach find as plain arguments instead of shell-string interpolations (#2634). + * find/sort failures fail closed, and an empty list is a successful empty + * scan. */ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, const char *file_pattern, - const char *pattern_file, const char *filelist, + const char *pattern, const char *filelist, const char *root_path, char output_path[CBM_SZ_2K], size_t output_limit, uint64_t deadline_ms, bool deadline_latched, @@ -18418,140 +18415,18 @@ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use output_path[0] = '\0'; return MCP_SCAN_COMMAND_FAILURE; } - FILE *list = cbm_fopen(filelist, "rb"); - if (!list) { - (void)fclose(out); - (void)cbm_unlink(output_path); - output_path[0] = '\0'; - return MCP_SCAN_COMMAND_FAILURE; + /* The sorted, NUL-separated file list feeds the same in-process scanner an + * indexed project uses, so matching has one implementation. */ + mcp_scan_cause_t scan_cause = search_scan_in_process(srv, filelist, pattern, use_regex, out, + output_limit, deadline_ms); + if (fclose(out) != 0 && scan_cause == MCP_SCAN_SUCCESS) { + scan_cause = MCP_SCAN_COMMAND_FAILURE; } - - const char *grep_flag = use_regex ? "-E" : "-F"; - const char **chunk_argv = - malloc(((size_t)MCP_ARGV_SCAN_GREP_CHUNK + 7) * sizeof(*chunk_argv)); - char **chunk_paths = malloc((size_t)MCP_ARGV_SCAN_GREP_CHUNK * sizeof(*chunk_paths)); - char *pathbuf = malloc(CBM_SZ_4K); - if (!chunk_argv || !chunk_paths || !pathbuf) { - free(chunk_argv); - free(chunk_paths); - free(pathbuf); - (void)fclose(list); - (void)fclose(out); - (void)cbm_unlink(output_path); - output_path[0] = '\0'; - return MCP_SCAN_COMMAND_FAILURE; - } - size_t pathcap = CBM_SZ_4K; - size_t pathlen = 0; - uint64_t output_bytes = 0; - size_t seq = 0; - cause = MCP_SCAN_SUCCESS; - bool io_error = false; - while (cause == MCP_SCAN_SUCCESS) { - size_t n = 0; - while (n < MCP_ARGV_SCAN_GREP_CHUNK) { - int c = fgetc(list); - if (c == EOF) { - if (ferror(list)) { - io_error = true; - } - break; - } - if (c == '\0') { - if (pathlen > 0) { - pathbuf[pathlen] = '\0'; - chunk_paths[n] = cbm_strdup(pathbuf); - if (!chunk_paths[n]) { - io_error = true; - break; - } - n++; - pathlen = 0; - } - continue; - } - if (pathlen + 1 >= pathcap) { - size_t newcap = pathcap * 2; - char *grown = realloc(pathbuf, newcap); - if (!grown) { - io_error = true; - break; - } - pathbuf = grown; - pathcap = newcap; - } - pathbuf[pathlen++] = (char)c; - } - if (io_error || n == 0) { - for (size_t i = 0; i < n; i++) { - free(chunk_paths[i]); - } - break; - } - - char chunk_path[CBM_SZ_2K]; - int written = snprintf(chunk_path, sizeof(chunk_path), "%s.grep%zu", output_path, seq++); - if (written <= 0 || (size_t)written >= sizeof(chunk_path)) { - io_error = true; - } else { - chunk_argv[0] = "grep"; - chunk_argv[1] = "-Hn"; - chunk_argv[2] = grep_flag; - chunk_argv[3] = "-f"; - chunk_argv[4] = pattern_file; - chunk_argv[5] = "--"; - for (size_t i = 0; i < n; i++) { - chunk_argv[6 + i] = chunk_paths[i]; - } - chunk_argv[6 + n] = NULL; - cause = mcp_run_argv_stage_cancellable_bounded( - srv, "grep", chunk_argv, NULL, chunk_path, output_bytes, output_limit, - deadline_ms, true, true, &cancellation_latched, &deadline_latched, - &output_limit_latched, result_out); - if (cause == MCP_SCAN_SUCCESS) { - FILE *chunk = cbm_fopen(chunk_path, "rb"); - if (!chunk) { - cause = MCP_SCAN_COMMAND_FAILURE; - } else { - char copybuf[CBM_SZ_16K]; - size_t got; - while ((got = fread(copybuf, 1, sizeof(copybuf), chunk)) > 0) { - if (fwrite(copybuf, 1, got, out) != got) { - cause = MCP_SCAN_COMMAND_FAILURE; - break; - } - output_bytes += got; - } - if (ferror(chunk) && cause == MCP_SCAN_SUCCESS) { - cause = MCP_SCAN_COMMAND_FAILURE; - } - (void)fclose(chunk); - } - } - (void)cbm_unlink(chunk_path); - } - for (size_t i = 0; i < n; i++) { - free(chunk_paths[i]); - } - } - free(chunk_argv); - free(chunk_paths); - free(pathbuf); - (void)fclose(list); - if (fclose(out) != 0 && cause == MCP_SCAN_SUCCESS) { - cause = MCP_SCAN_COMMAND_FAILURE; - } - if (io_error && cause == MCP_SCAN_SUCCESS) { - cause = MCP_SCAN_COMMAND_FAILURE; - } - if (cause == MCP_SCAN_SUCCESS && output_limit > 0 && output_bytes > output_limit) { - cause = MCP_SCAN_OUTPUT_LIMIT; - } - if (cause != MCP_SCAN_SUCCESS) { + if (scan_cause != MCP_SCAN_SUCCESS) { (void)cbm_unlink(output_path); output_path[0] = '\0'; } - return cause; + return scan_cause; } #endif From 9a0742ed4521156575063692bade8b088bd79037 Mon Sep 17 00:00:00 2001 From: tahodev <85001874+tahodev@users.noreply.github.com> Date: Mon, 2 Nov 2026 05:29:05 +0900 Subject: [PATCH 10/19] Lower the memory-core baseline for src/mcp/mcp.c Signed-off-by: tahodev <85001874+tahodev@users.noreply.github.com> --- scripts/memory-core-baseline.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/memory-core-baseline.txt b/scripts/memory-core-baseline.txt index d9f632e7c..7e979160d 100644 --- a/scripts/memory-core-baseline.txt +++ b/scripts/memory-core-baseline.txt @@ -51,7 +51,7 @@ src/git/git_context.c 18 src/main.c 32 src/mcp/compact_out.c 39 src/mcp/index_supervisor.c 8 -src/mcp/mcp.c 769 +src/mcp/mcp.c 765 src/pipeline/artifact.c 38 src/pipeline/fqn.c 23 src/pipeline/lsp_resolve.h 4 From 55200a6cfd9da24acdad9f18207efdbdb37b9b9b Mon Sep 17 00:00:00 2001 From: tahodev <85001874+tahodev@users.noreply.github.com> Date: Mon, 2 Nov 2026 07:25:32 +0900 Subject: [PATCH 11/19] Format the new search scan hunks for clang-format-20 Signed-off-by: tahodev <85001874+tahodev@users.noreply.github.com> --- src/mcp/mcp.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 5c1ee41e1..463aa8d80 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -17329,9 +17329,8 @@ static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( /* POSIX-only: the unscoped search_code scan as find/sort argv stages feeding the * in-process scanner. */ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, - const char *file_pattern, - const char *pattern, const char *filelist, - const char *root_path, + const char *file_pattern, const char *pattern, + const char *filelist, const char *root_path, char output_path[CBM_SZ_2K], size_t output_limit, uint64_t deadline_ms, bool deadline_latched, cbm_proc_result_t *result_out); @@ -18301,9 +18300,10 @@ static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( output_limit_latched); if (*cancellation_latched || *deadline_latched || *output_limit_latched) { result_out->tree_quiesced = true; /* no child was spawned */ - return *cancellation_latched ? MCP_SCAN_CANCELLED - : *deadline_latched ? MCP_SCAN_DEADLINE - : MCP_SCAN_OUTPUT_LIMIT; + if (*cancellation_latched) { + return MCP_SCAN_CANCELLED; + } + return *deadline_latched ? MCP_SCAN_DEADLINE : MCP_SCAN_OUTPUT_LIMIT; } cbm_proc_opts_t options = { .bin = bin, @@ -18369,9 +18369,8 @@ static mcp_scan_cause_t mcp_run_argv_stage_cancellable_bounded( * find/sort failures fail closed, and an empty list is a successful empty * scan. */ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use_regex, - const char *file_pattern, - const char *pattern, const char *filelist, - const char *root_path, + const char *file_pattern, const char *pattern, + const char *filelist, const char *root_path, char output_path[CBM_SZ_2K], size_t output_limit, uint64_t deadline_ms, bool deadline_latched, cbm_proc_result_t *result_out) { @@ -18417,8 +18416,8 @@ static mcp_scan_cause_t mcp_run_search_scan_argv(cbm_mcp_server_t *srv, bool use } /* The sorted, NUL-separated file list feeds the same in-process scanner an * indexed project uses, so matching has one implementation. */ - mcp_scan_cause_t scan_cause = search_scan_in_process(srv, filelist, pattern, use_regex, out, - output_limit, deadline_ms); + mcp_scan_cause_t scan_cause = + search_scan_in_process(srv, filelist, pattern, use_regex, out, output_limit, deadline_ms); if (fclose(out) != 0 && scan_cause == MCP_SCAN_SUCCESS) { scan_cause = MCP_SCAN_COMMAND_FAILURE; } From 5216b70635beb936eebb1a0c6b26b4aa11ac3b5d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 02:34:09 +0100 Subject: [PATCH 12/19] test(cli): adapt the no-hooks plan fixture to the Claude detection rule Since #1180 a bare ~/.claude directory no longer counts as a Claude Code install. The fixture of cli_no_hooks_plan_covers_every_hook_installer_family now also writes .claude/settings.json. Without it claude-code drops out of agents_detected and the test fails on the tree merged with current main. Signed-off-by: Martin Vogel --- tests/test_cli.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/test_cli.c b/tests/test_cli.c index 484b30dfa..4e750d314 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -2676,6 +2676,9 @@ TEST(cli_no_hooks_plan_covers_every_hook_installer_family) { write_test_file(path, "{}\n"); snprintf(path, sizeof(path), "%s/.copilot/mcp-config.json", tmpdir); write_test_file(path, "{}\n"); + /* #1180: a bare ~/.claude is not a Claude Code install; its settings.json is. */ + snprintf(path, sizeof(path), "%s/.claude/settings.json", tmpdir); + write_test_file(path, "{}\n"); const char *const commands[] = {"qodercli", "kimi", "duo", "devin", "gemini", "copilot", "auggie", "hermes", "droid"}; From ebe26ba2b5ab0f0229492dc9e1159ff24bd87029 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 02:34:09 +0100 Subject: [PATCH 13/19] fix(cli): remove the Codex PreToolUse hook under the install-hook opt-out #2719 added a PreToolUse Bash hook for Codex after this opt-out was written. In config.toml it lives inside the managed block, which the opt-out already removes. In hooks.json it is a separate entry beside the lifecycle pair, and the opt-out removed only the pair, so after install --no-hooks, and on every later bare install or update that respects the stored preference, our PreToolUse hook stayed registered. The --no-hooks branch now also calls cbm_remove_codex_pretool_hooks_json, the exact-command remover that uninstall uses since #2719. A PreToolUse hook the user owns is left alone. Test: cli_no_hooks_removes_codex_pretool_hook drives the real install dispatch through both hook representations. A default install writes ours beside a foreign hook; --no-hooks and a bare install with the stored preference remove ours and keep the foreign hook and the MCP entry; and --hooks restores ours. Without the fix it fails: ours is still in hooks.json after --no-hooks. Signed-off-by: Martin Vogel --- src/cli/cli.c | 8 ++- tests/test_cli.c | 149 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index df29e41ea..cf69fe13b 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -10755,7 +10755,13 @@ static void install_cli_agent_configs(const cbm_detected_agents_t *agents, const int json_result = !cbm_file_exists(hooks_json) ? CLI_OK : cbm_remove_paired_lifecycle_hooks_json(hooks_json, command); - hook_ok = toml_result == CLI_OK && json_result == CLI_OK; + /* #2719: the PreToolUse Bash hook shares hooks.json with the + * lifecycle pair, so the opt-out removes it the way uninstall does. */ + int pretool_result = !cbm_file_exists(hooks_json) + ? CLI_OK + : cbm_remove_codex_pretool_hooks_json(hooks_json, command); + hook_ok = + toml_result == CLI_OK && json_result == CLI_OK && pretool_result == CLI_OK; } else if (!dry_run && use_hooks_json) { hook_ok = cbm_upsert_paired_lifecycle_hooks_json(hooks_json, command, command_windows, diff --git a/tests/test_cli.c b/tests/test_cli.c index 4e750d314..569c9353d 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -2743,6 +2743,154 @@ TEST(cli_no_hooks_plan_covers_every_hook_installer_family) { PASS(); } +/* Counts the PreToolUse hooks in a Codex hooks.json: ours run hook-augment, the + * foreign one is matched by its exact command. */ +static void test_codex_pretool_hook_counts(const char *path, const char *foreign_command, + size_t *owned, size_t *foreign) { + *owned = 0U; + *foreign = 0U; + char *text = read_test_file_alloc(path); + yyjson_doc *doc = text ? yyjson_read(text, strlen(text), 0) : NULL; + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *hooks = root ? yyjson_obj_get(root, "hooks") : NULL; + yyjson_val *entries = hooks ? yyjson_obj_get(hooks, "PreToolUse") : NULL; + size_t entry_index; + size_t entry_count; + yyjson_val *entry; + if (entries && yyjson_is_arr(entries)) { + yyjson_arr_foreach(entries, entry_index, entry_count, entry) { + yyjson_val *entry_hooks = yyjson_obj_get(entry, "hooks"); + size_t hook_index; + size_t hook_count; + yyjson_val *hook; + if (!entry_hooks || !yyjson_is_arr(entry_hooks)) { + continue; + } + yyjson_arr_foreach(entry_hooks, hook_index, hook_count, hook) { + const char *command = yyjson_get_str(yyjson_obj_get(hook, "command")); + if (command && strstr(command, "hook-augment")) { + (*owned)++; + } else if (command && strcmp(command, foreign_command) == 0) { + (*foreign)++; + } + } + } + } + yyjson_doc_free(doc); + free(text); +} + +/* The Codex PreToolUse Bash hook (#2719) follows the persisted opt-out in both + * hook representations: --no-hooks removes ours from hooks.json and from the + * config.toml managed block, a bare install keeps it off, the user's own + * PreToolUse hook and the MCP entry stay, and --hooks restores ours. */ +TEST(cli_no_hooks_removes_codex_pretool_hook) { + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s/cli-codex-no-hooks-XXXXXX", cbm_tmpdir()); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *const env_names[] = {"HOME", "CBM_CACHE_DIR", "CODEX_HOME", "PATH"}; + char *saved_env[sizeof(env_names) / sizeof(env_names[0])]; + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + saved_env[i] = save_test_env(env_names[i]); + cbm_unsetenv(env_names[i]); + } + + char cache_dir[640]; + char codex_dir[640]; + char hooks_path[768]; + char config_path[768]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + snprintf(codex_dir, sizeof(codex_dir), "%s/.codex", tmpdir); + snprintf(hooks_path, sizeof(hooks_path), "%s/hooks.json", codex_dir); + snprintf(config_path, sizeof(config_path), "%s/config.toml", codex_dir); + test_mkdirp(codex_dir); + /* An installed binary at the default target names the Codex hook command; + * without it --skip-binary would configure the test runner itself. */ + char binary_dir[640]; + char binary_path[768]; + snprintf(binary_dir, sizeof(binary_dir), "%s/.local/bin", tmpdir); + test_mkdirp(binary_dir); +#ifdef _WIN32 + snprintf(binary_path, sizeof(binary_path), "%s/codebase-memory-mcp.exe", binary_dir); +#else + snprintf(binary_path, sizeof(binary_path), "%s/codebase-memory-mcp", binary_dir); +#endif + write_test_file(binary_path, "installed binary\n"); + const char *foreign_command = "/usr/bin/foreign-shell-audit"; + write_test_file(hooks_path, + "{\"hooks\":{\"PreToolUse\":[{\"matcher\":\"Bash\",\"hooks\":[{\"type\":" + "\"command\",\"command\":\"/usr/bin/foreign-shell-audit\"}]}]}}\n"); + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + cbm_setenv("PATH", tmpdir, 1); + + cli_activation_fake_t fake = {.mutation_reserve_result = 1}; + cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake); + cbm_cli_set_activation_ops_for_test(&ops); + + size_t owned = 0U; + size_t foreign = 0U; + char *default_args[] = {"--skip-binary", "--clients=codex", "--yes"}; + int default_rc = cli_test_cmd_install(3, default_args); + test_codex_pretool_hook_counts(hooks_path, foreign_command, &owned, &foreign); + bool default_json = default_rc == 0 && owned == 1U && foreign == 1U; + + char *no_hooks_args[] = {"--no-hooks", "--skip-binary", "--clients=codex", "--yes"}; + int no_hooks_rc = cli_test_cmd_install(4, no_hooks_args); + test_codex_pretool_hook_counts(hooks_path, foreign_command, &owned, &foreign); + char *json_state = read_test_file_alloc(hooks_path); + char *toml_state = read_test_file_alloc(config_path); + bool json_disabled = no_hooks_rc == 0 && owned == 0U && foreign == 1U && json_state && + !strstr(json_state, "hook-augment"); + bool json_mcp_kept = toml_state && strstr(toml_state, "[mcp_servers.codebase-memory-mcp]") && + !strstr(toml_state, "hook-augment"); + free(json_state); + free(toml_state); + + int bare_rc = cli_test_cmd_install(3, default_args); + test_codex_pretool_hook_counts(hooks_path, foreign_command, &owned, &foreign); + bool bare_disabled = bare_rc == 0 && owned == 0U && foreign == 1U; + + /* Without hooks.json the config.toml managed block carries the hooks. */ + (void)remove(hooks_path); + char *hooks_args[] = {"--hooks", "--skip-binary", "--clients=codex", "--yes"}; + int toml_enable_rc = cli_test_cmd_install(4, hooks_args); + toml_state = read_test_file_alloc(config_path); + bool toml_enabled = toml_enable_rc == 0 && toml_state && + test_count_substring(toml_state, "[[hooks.PreToolUse]]") == 1U; + free(toml_state); + int toml_disable_rc = cli_test_cmd_install(4, no_hooks_args); + toml_state = read_test_file_alloc(config_path); + bool toml_disabled = toml_disable_rc == 0 && toml_state && + !strstr(toml_state, "hooks.PreToolUse") && + !strstr(toml_state, "hook-augment") && + strstr(toml_state, "[mcp_servers.codebase-memory-mcp]"); + free(toml_state); + int restore_rc = cli_test_cmd_install(4, hooks_args); + toml_state = read_test_file_alloc(config_path); + bool restored = restore_rc == 0 && toml_state && + test_count_substring(toml_state, "[[hooks.PreToolUse]]") == 1U; + free(toml_state); + + cbm_cli_set_activation_ops_for_test(NULL); + cbm_set_auto_answer_for_test(0); + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + restore_test_env(env_names[i], saved_env[i]); + } + test_rmdir_r(tmpdir); + + ASSERT_TRUE(default_json); + ASSERT_TRUE(json_mcp_kept); + ASSERT_TRUE(toml_enabled); + ASSERT_TRUE(toml_disabled); + ASSERT_TRUE(restored); + ASSERT_TRUE(json_disabled); + ASSERT_TRUE(bare_disabled); + PASS(); +} + TEST(cli_install_reset_deletion_waits_for_final_activation_guard) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-daemon-install-reset-XXXXXX"); @@ -20433,6 +20581,7 @@ SUITE(cli) { RUN_TEST(cli_activation_commands_reject_malformed_and_unknown_flags); RUN_TEST(cli_install_hooks_preference_persists_and_migrates_owned_state); RUN_TEST(cli_no_hooks_plan_covers_every_hook_installer_family); + RUN_TEST(cli_no_hooks_removes_codex_pretool_hook); RUN_TEST(cli_install_reset_deletion_waits_for_final_activation_guard); RUN_TEST(cli_remove_indexes_deletes_orphan_sqlite_sidecars_issue2054); RUN_TEST(cli_index_enumeration_skips_internal_cache_dbs); From 7fd658497715065b132c9e54fe1e16bd4fd8bdff Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 03:15:43 +0100 Subject: [PATCH 14/19] test(java): pin the own-package Enum case from the review The review of this change named a literal case: a project class Enum in package com.acme.protogen, and new Enum(cls, "parent") in a sibling class of that package (Netflix conductor has this shape: protogen.Enum). java.lang is an on-demand import, and an on-demand import never shadows a type of the own package (JLS 6.4.1), so the call means the project class. java_typed_calls_own_package_enum_sequential and _parallel index that pair on both pipeline paths and assert one CALLS edge from addNestedEnum to the Class node com.acme.protogen.Enum, as new does for every project type here. RED with only the type-name order of this branch reverted (java.lang before the own package): Enum reads as java.lang.Enum, the call counts as external, and both new tests find no edge. GREEN with this branch. Signed-off-by: Martin Vogel --- tests/test_java_typed_calls.c | 94 +++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 3 deletions(-) diff --git a/tests/test_java_typed_calls.c b/tests/test_java_typed_calls.c index 0849f3acd..93ad9fbd4 100644 --- a/tests/test_java_typed_calls.c +++ b/tests/test_java_typed_calls.c @@ -16,7 +16,9 @@ * fallback; * - a project type in the caller's own package that shares its simple * name with a java.lang type (Process), or such a type declared in the - * caller's own file (Record): the own package wins. + * caller's own file (Record): the own package wins; + * - the same rule on a project class named Enum (com.acme.protogen): + * `new Enum(..)` there binds that class node, not java.lang.Enum. * The same fixture runs on the sequential path and, padded past * MIN_FILES_FOR_PARALLEL, on the parallel one: both must agree on every edge * found by type. A name guess is left out of that comparison: on Windows the @@ -105,11 +107,30 @@ static const char *const JT_SERVICE = " }\n" "}\n"; +/* JLS 6.4.1: java.lang is an on-demand import, and an on-demand import never + * shadows a type of the compilation unit's own package. So `Enum` in + * Message means com.acme.protogen.Enum (the shape of Netflix conductor's + * protogen.Enum). Read as java.lang.Enum, the call counts as external and + * loses its edge. */ +static const char *const JT_PROTO_ENUM = "package com.acme.protogen;\n\n" + "public class Enum {\n" + " public Enum(Class cls, String parent) {}\n" + "}\n"; + +static const char *const JT_PROTO_MESSAGE = "package com.acme.protogen;\n\n" + "public class Message {\n" + " public void addNestedEnum(Class cls) {\n" + " Enum e = new Enum(cls, \"parent\");\n" + " }\n" + "}\n"; + static void jt_write_fixture(const char *tmp, int filler) { th_write_file(TH_PATH(tmp, "src/main/java/com/acme/model/Customer.java"), JT_CUSTOMER); th_write_file(TH_PATH(tmp, "src/main/java/com/acme/other/Filters.java"), JT_FILTERS); th_write_file(TH_PATH(tmp, "src/main/java/com/acme/svc/InvoiceService.java"), JT_SERVICE); th_write_file(TH_PATH(tmp, "src/main/java/com/acme/svc/Process.java"), JT_PROCESS); + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/protogen/Enum.java"), JT_PROTO_ENUM); + th_write_file(TH_PATH(tmp, "src/main/java/com/acme/protogen/Message.java"), JT_PROTO_MESSAGE); for (int i = 0; i < filler; i++) { char rel[128]; char body[256]; @@ -310,18 +331,73 @@ static int jt_check_external(jt_index_t *ix, int sequential) { return ok; } -static int jt_run_all(int filler) { +/* The one CALLS edge for (caller, leaf) reaches a node labelled `label`. */ +static int jt_expect_target_label(jt_index_t *ix, const char *caller, const char *leaf, + const char *label) { + const char *sql = "SELECT t.label, json_extract(e.properties,'$.callee') FROM edges e " + "JOIN nodes s ON s.id = e.source_id JOIN nodes t ON t.id = e.target_id " + "WHERE e.type = 'CALLS' AND s.name = ?1"; + sqlite3_stmt *st = NULL; + if (sqlite3_prepare_v2(ix->db, sql, -1, &st, NULL) != SQLITE_OK) { + return 0; + } + sqlite3_bind_text(st, 1, caller, -1, SQLITE_STATIC); + int hits = 0; + int others = 0; + size_t llen = strlen(leaf); + while (sqlite3_step(st) == SQLITE_ROW) { + const char *have = (const char *)sqlite3_column_text(st, 0); + const char *callee = (const char *)sqlite3_column_text(st, 1); + if (!have || !callee) { + continue; + } + size_t clen = strlen(callee); + bool leaf_match = (clen == llen && strcmp(callee, leaf) == 0) || + (clen > llen && callee[clen - llen - 1] == '.' && + strcmp(callee + clen - llen, leaf) == 0); + if (!leaf_match) { + continue; + } + if (strcmp(have, label) == 0) { + hits++; + } else { + others++; + } + } + sqlite3_finalize(st); + if (hits == 1 && others == 0) { + return 1; + } + printf("\n %s -> %s: want one edge to a %s node, have %d to one and %d to others\n", caller, + leaf, label, hits, others); + return 0; +} + +/* `new Enum(..)` in com.acme.protogen binds the project class Enum (its + * class node, as `new` does for every project type), not java.lang.Enum. */ +static int jt_check_own_package_enum(jt_index_t *ix) { + int ok = 1; + ok &= jt_expect(ix, "addNestedEnum", "Enum", "com.acme.protogen.Enum"); + ok &= jt_expect_target_label(ix, "addNestedEnum", "Enum", "Class"); + return ok; +} + +static int jt_run_check(int filler, int enum_only) { jt_index_t ix; if (jt_index(&ix, filler) != 0) { printf("\n indexing the fixture failed\n"); jt_close(&ix); return 0; } - int ok = jt_check_external(&ix, filler == 0); + int ok = enum_only ? jt_check_own_package_enum(&ix) : jt_check_external(&ix, filler == 0); jt_close(&ix); return ok; } +static int jt_run_all(int filler) { + return jt_run_check(filler, 0); +} + TEST(java_typed_calls_sequential) { ASSERT_TRUE(jt_run_all(0)); PASS(); @@ -332,6 +408,16 @@ TEST(java_typed_calls_parallel) { PASS(); } +TEST(java_typed_calls_own_package_enum_sequential) { + ASSERT_TRUE(jt_run_check(0, 1)); + PASS(); +} + +TEST(java_typed_calls_own_package_enum_parallel) { + ASSERT_TRUE(jt_run_check(60, 1)); + PASS(); +} + /* The sequential and the parallel path find the same svc edges by type. */ TEST(java_typed_calls_paths_agree) { jt_index_t seq; @@ -355,5 +441,7 @@ TEST(java_typed_calls_paths_agree) { SUITE(java_typed_calls) { RUN_TEST(java_typed_calls_sequential); RUN_TEST(java_typed_calls_parallel); + RUN_TEST(java_typed_calls_own_package_enum_sequential); + RUN_TEST(java_typed_calls_own_package_enum_parallel); RUN_TEST(java_typed_calls_paths_agree); } From cbf379472f2358c6d3ca26fa64d5969779ae07e8 Mon Sep 17 00:00:00 2001 From: Shane McCarron Date: Mon, 2 Nov 2026 07:52:34 -0600 Subject: [PATCH 15/19] fix(watcher): cap success cooldown at 5 min; test the schedule without sleeping Review feedback on #2840: - Cap the success cooldown at INDEX_FAIL_CEILING_MS (5 min) instead of a separate 30-minute ceiling, so under continuous writes a project falls at most one rebuild plus five minutes behind. The 3x factor is unchanged below the cap. - Add two CBM_ENABLE_TEST_SEAMS hooks: freeze the watcher's scheduling clock, and read a project's next_poll_ns. The wire-up test now advances the frozen clock inside index_fn instead of sleeping 1.8 s, and asserts the exact scheduled poll time for an instant index, a 60 s index (180 s, counted from the END of the index) and a 200 s index (capped at 300 s). Refs #2835 Signed-off-by: Shane McCarron --- src/watcher/watcher.c | 42 +++++++++++++---- src/watcher/watcher.h | 17 +++++-- tests/test_watcher.c | 103 ++++++++++++++++++++++-------------------- 3 files changed, 99 insertions(+), 63 deletions(-) diff --git a/src/watcher/watcher.c b/src/watcher/watcher.c index cef4fd482..3bdd1a6c3 100644 --- a/src/watcher/watcher.c +++ b/src/watcher/watcher.c @@ -155,14 +155,13 @@ struct cbm_watcher { #define INDEX_FAIL_SUSTAINED 10 /* Success cooldown (#2835): after a successful re-index the next poll waits - * at least INDEX_COOLDOWN_FACTOR x the index's own duration, counted from when - * the index finished, so the watcher spends at most ~1/(K+1) of wall time - * indexing one project — a 5-minute rebuild idles 15 minutes. Sub-second - * indexes stay at the plain cadence. The ceiling bounds staleness after one - * pathological run; it is NOT INDEX_FAIL_CEILING_MS (5 min), which would cut - * the cooldown of any rebuild over 100 s below K x its duration. */ + * INDEX_COOLDOWN_FACTOR x the index's own duration, counted from when the + * index finished, so short rebuilds spend at most ~1/(K+1) of wall time + * indexing one project. Sub-second indexes stay at the plain cadence. The + * cooldown is capped at INDEX_FAIL_CEILING_MS (5 min) so that, under + * continuous writes, a project never falls more than one rebuild plus five + * minutes behind. */ #define INDEX_COOLDOWN_FACTOR 3 -#define INDEX_COOLDOWN_CEILING_MS 1800000 /* 30 min */ /* Stale-root pruning (#286): a watched project whose root directory stays * missing is pruned — its cached DB is deleted and the watch entry removed. @@ -185,7 +184,17 @@ struct cbm_watcher { /* ── Time helper ────────────────────────────────────────────────── */ +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS +/* Test seam: nonzero freezes the watcher's scheduling clock at this value. */ +static int64_t test_clock_ns = 0; +#endif + static int64_t now_ns(void) { +#if defined(CBM_ENABLE_TEST_SEAMS) && CBM_ENABLE_TEST_SEAMS + if (test_clock_ns != 0) { + return test_clock_ns; + } +#endif struct timespec ts; cbm_clock_gettime(CLOCK_MONOTONIC, &ts); return ((int64_t)ts.tv_sec * NS_PER_SEC) + ts.tv_nsec; @@ -222,8 +231,8 @@ int cbm_watcher_success_delay_ms(int interval_ms, int64_t index_elapsed_ms) { return interval_ms; } /* Divide instead of multiply so a huge elapsed cannot overflow. */ - int64_t cooldown = index_elapsed_ms > INDEX_COOLDOWN_CEILING_MS / INDEX_COOLDOWN_FACTOR - ? INDEX_COOLDOWN_CEILING_MS + int64_t cooldown = index_elapsed_ms > INDEX_FAIL_CEILING_MS / INDEX_COOLDOWN_FACTOR + ? INDEX_FAIL_CEILING_MS : index_elapsed_ms * INDEX_COOLDOWN_FACTOR; return (int)(cooldown > interval_ms ? cooldown : interval_ms); } @@ -1912,4 +1921,19 @@ int cbm_watcher_test_pending_free_count(cbm_watcher_t *w) { cbm_mutex_unlock(&w->projects_lock); return count; } + +void cbm_watcher_test_set_clock_ns(int64_t ns) { + test_clock_ns = ns; +} + +int64_t cbm_watcher_test_next_poll_ns(cbm_watcher_t *w, const char *project) { + if (!w || !project) { + return -1; + } + cbm_mutex_lock(&w->projects_lock); + project_state_t *s = cbm_ht_get(w->projects, project); + int64_t next = s ? s->next_poll_ns : -1; + cbm_mutex_unlock(&w->projects_lock); + return next; +} #endif diff --git a/src/watcher/watcher.h b/src/watcher/watcher.h index eb7e92915..571b20123 100644 --- a/src/watcher/watcher.h +++ b/src/watcher/watcher.h @@ -145,6 +145,15 @@ int cbm_watcher_index_failure_count(cbm_watcher_t *w, const char *project_name); * otherwise unobservable — cbm_watcher_free frees the same list, so a poll * that stopped draining would neither crash nor leak. Test builds only. */ int cbm_watcher_test_pending_free_count(cbm_watcher_t *w); + +/* Test seam: freeze the watcher's monotonic scheduling clock at `ns` + * (nonzero), or restore the real clock (0). Process-global; single-threaded + * tests only. Lets a test advance time inside index_fn without sleeping. */ +void cbm_watcher_test_set_clock_ns(int64_t ns); + +/* Test seam: a project's scheduled next poll (monotonic ns), read under + * projects_lock; -1 when it is not watched. */ +int64_t cbm_watcher_test_next_poll_ns(cbm_watcher_t *w, const char *project); #endif /* Per-project watch status, for index_status (#2167). */ @@ -197,10 +206,10 @@ int cbm_watcher_index_backoff_ms(int interval_ms, int consecutive_failures); /* Return the delay (ms) before the next poll after a SUCCESSFUL re-index that * took `index_elapsed_ms`: max(interval_ms, INDEX_COOLDOWN_FACTOR (3) x - * index_elapsed_ms), with the cooldown term capped at INDEX_COOLDOWN_CEILING_MS - * (30 min). The caller counts it from when the index finished, so the watcher - * spends at most ~1/4 of wall time indexing a project; sub-second indexes keep - * `interval_ms`. + * index_elapsed_ms), with the cooldown term capped at INDEX_FAIL_CEILING_MS + * (5 min). The caller counts it from when the index finished, so short + * rebuilds spend at most ~1/4 of wall time indexing a project; sub-second + * indexes keep `interval_ms`. * Never below `interval_ms`; overflow-safe; negative inputs act as zero. */ int cbm_watcher_success_delay_ms(int interval_ms, int64_t index_elapsed_ms); diff --git a/tests/test_watcher.c b/tests/test_watcher.c index c2796e3a6..670d4f5a2 100644 --- a/tests/test_watcher.c +++ b/tests/test_watcher.c @@ -2273,15 +2273,17 @@ TEST(success_delay_fast_index_keeps_interval) { TEST(success_delay_slow_index_scales_by_factor) { ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 1667), 5001); ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 10000), 30000); - /* 5-minute rebuild → next poll 15 min after it finishes. */ - ASSERT_EQ(cbm_watcher_success_delay_ms(60000, 300000), 900000); + /* 90 s rebuild → next poll 4.5 min after it finishes. */ + ASSERT_EQ(cbm_watcher_success_delay_ms(60000, 90000), 270000); PASS(); } TEST(success_delay_clamped_and_degenerate) { - ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 600000), 1800000); - ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 600001), 1800000); - ASSERT_EQ(cbm_watcher_success_delay_ms(5000, INT64_MAX), 1800000); + /* Capped at 5 min (INDEX_FAIL_CEILING_MS) from 100 s of indexing up. */ + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 100000), 300000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, 100001), 300000); + ASSERT_EQ(cbm_watcher_success_delay_ms(60000, 300000), 300000); + ASSERT_EQ(cbm_watcher_success_delay_ms(5000, INT64_MAX), 300000); ASSERT_EQ(cbm_watcher_success_delay_ms(5000, -1), 5000); ASSERT_EQ(cbm_watcher_success_delay_ms(5000, INT64_MIN), 5000); ASSERT_EQ(cbm_watcher_success_delay_ms(-1, 0), 0); @@ -2295,34 +2297,45 @@ TEST(success_delay_is_monotonic) { for (int64_t e = 0; e <= 2000000; e += 997) { int delay = cbm_watcher_success_delay_ms(5000, e); ASSERT_TRUE(delay >= previous); - ASSERT_TRUE(delay >= 5000 && delay <= 1800000); + ASSERT_TRUE(delay >= 5000 && delay <= 300000); previous = delay; } PASS(); } -/* Wire-up: the cooldown must be measured around the real index_fn call. An - * instant callback keeps the plain cadence (no cooldown line); a callback - * slower than interval/3 (5000/3 ms) extends it and logs once. */ -static int cooldown_log_hits = 0; -static void cooldown_log_sink(const char *line) { - if (line && strstr(line, "watcher.cooldown") != NULL) { - cooldown_log_hits++; - } -} -static int slow_index_sleep_ms = 0; -static int slow_index_calls = 0; -static int slow_index_callback(const char *name, const char *path, void *ud) { +/* Wire-up: the cooldown must be measured around the real index_fn call and + * counted from when the index finished. The watcher's clock is frozen via a + * test seam and the callback advances it, so an index "takes" a chosen time + * with no sleep and the scheduled poll time can be asserted exactly. */ +#define COOL_NS_PER_MS 1000000LL +static int64_t cool_clock_ns = 0; +static int64_t cool_index_ms = 0; +static int cool_index_calls = 0; +static int cool_index_callback(const char *name, const char *path, void *ud) { (void)name; (void)path; (void)ud; - slow_index_calls++; - if (slow_index_sleep_ms > 0) { - cbm_usleep((unsigned)slow_index_sleep_ms * 1000U); - } + cool_index_calls++; + cool_clock_ns += cool_index_ms * COOL_NS_PER_MS; + cbm_watcher_test_set_clock_ns(cool_clock_ns); return 0; } +/* Re-index once (touch + poll until the callback runs; a transient git probe + * failure makes a poll a no-op, see the sustained-failure test) and return + * the scheduled next poll relative to when the index finished, in ms. */ +static int64_t cool_reindex_delay_ms(cbm_watcher_t *w, const char *file, const char *line, + int64_t index_ms) { + cool_index_ms = index_ms; + th_append_file(file, line); + int target = cool_index_calls + 1; + while (cool_index_calls < target) { + cbm_watcher_touch(w, "cool-repo"); + cbm_watcher_poll_once(w); + } + return (cbm_watcher_test_next_poll_ns(w, "cool-repo") - cool_clock_ns) / COOL_NS_PER_MS; +} + TEST(watcher_success_cooldown_measures_index_issue2835) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cbm_watcher_cool_XXXXXX"); @@ -2337,40 +2350,30 @@ TEST(watcher_success_cooldown_measures_index_issue2835) { wt_git(tmpdir, "add file.txt"); wt_git(tmpdir, "commit -q -m init"); + cool_clock_ns = 1000LL * 1000 * COOL_NS_PER_MS; /* any nonzero start */ + cbm_watcher_test_set_clock_ns(cool_clock_ns); + cool_index_calls = 0; + cbm_store_t *store = cbm_store_open_memory(); - cbm_watcher_t *w = cbm_watcher_new(store, slow_index_callback, NULL); + cbm_watcher_t *w = cbm_watcher_new(store, cool_index_callback, NULL); cbm_watcher_watch(w, "cool-repo", tmpdir); - slow_index_calls = 0; - slow_index_sleep_ms = 0; cbm_watcher_poll_once(w); /* baseline */ - CBMLogLevel old_level = cbm_log_get_level(); - cbm_log_set_level(CBM_LOG_INFO); - cooldown_log_hits = 0; - cbm_log_set_sink(cooldown_log_sink); + wt_path(p, sizeof(p), tmpdir, "file.txt"); + /* Instant index: plain cadence. */ + int64_t fast = cool_reindex_delay_ms(w, p, "a\n", 0); + /* 60 s index → 3 x 60 s, counted from the END of the index (anchoring at + * the start would give 120 s here). */ + int64_t slow = cool_reindex_delay_ms(w, p, "b\n", 60000); + /* 200 s index → 600 s, capped at 5 min. */ + int64_t capped = cool_reindex_delay_ms(w, p, "c\n", 200000); - /* Instant index: no cooldown. Loop on the attempt count (a transient git - * probe failure makes a poll a no-op, see the sustained-failure test). */ - th_append_file(wt_path(p, sizeof(p), tmpdir, "file.txt"), "a\n"); - while (slow_index_calls < 1) { - cbm_watcher_touch(w, "cool-repo"); - cbm_watcher_poll_once(w); - } - int fast_hits = cooldown_log_hits; + /* Restore the real clock before any assert can return early. */ + cbm_watcher_test_set_clock_ns(0); - /* 1.8 s index → 3 x 1800 = 5400 ms > 5000 ms interval: cooldown. */ - slow_index_sleep_ms = 1800; - th_append_file(wt_path(p, sizeof(p), tmpdir, "file.txt"), "b\n"); - while (slow_index_calls < 2) { - cbm_watcher_touch(w, "cool-repo"); - cbm_watcher_poll_once(w); - } - - cbm_log_set_sink(NULL); - cbm_log_set_level(old_level); - - ASSERT_EQ(fast_hits, 0); - ASSERT_EQ(cooldown_log_hits, 1); + ASSERT_EQ(fast, cbm_watcher_poll_interval_ms(1)); + ASSERT_EQ(slow, 180000); + ASSERT_EQ(capped, 300000); cbm_watcher_free(w); cbm_store_close(store); From e1be4c4eaf6ef77fb5e55bacbe04e47bbb13d71d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 18:52:14 +0100 Subject: [PATCH 16/19] fix(cli): remove the owned Augment scripts under the install-hook opt-out install --no-hooks and the persisted install_hooks=false preference removed the Augment SessionStart and PostToolUse view registrations from ~/.augment/settings.json but left the two scripts they pointed at, ~/.augment/hooks/codebase-memory-session.sh and codebase-memory-coverage.sh (.ps1 on Windows), on disk. The opt-out now deletes them the way uninstall does. Both paths share remove_augment_owned_hook_scripts(): each script is rebuilt from the binary path the install embedded and removed with cbm_text_remove_owned_document() only on a byte-for-byte match, so a script the user edited, or a foreign file with the same name, is kept. Uninstall keeps its error operation names; the opt-out records session_script_disable and coverage_script_disable. install --hooks writes both scripts again. Test: cli_no_hooks_removes_augment_owned_scripts drives the install command with fake activation ops in a temporary HOME: default install, --no-hooks (both scripts gone, MCP entry kept), a bare install with the stored preference (still gone), --hooks (restored), and a user-edited session script that --no-hooks keeps while it deletes the untouched coverage script. The test ends on --hooks because the opt-out is process state that later tests rely on. Signed-off-by: Martin Vogel --- src/cli/cli.c | 55 ++++++++++++--------- tests/test_cli.c | 126 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 24 deletions(-) diff --git a/src/cli/cli.c b/src/cli/cli.c index cf69fe13b..44c0861df 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -9140,6 +9140,35 @@ static int remove_claude_owned_hook_scripts(const char *config_dir, const char * return rc; } +/* Augment's SessionStart and PostToolUse view scripts are exact-owned: each is + * rebuilt from the binary path the install embedded and deleted only on a + * byte-for-byte match, so a script the user edited stays. Uninstall and the + * install-hook opt-out share this rule. */ +static void remove_augment_owned_hook_scripts(const char *binary_path, const char *session_path, + const char *coverage_path, bool uninstall) { + const struct { + const char *path; + int (*build)(const char *, char *, size_t); + const char *build_op; + const char *remove_op; + } scripts[] = { + {session_path, cbm_build_augment_session_script, "session_script_build", + uninstall ? "session_script_uninstall" : "session_script_disable"}, + {coverage_path, cbm_build_augment_coverage_script, "coverage_script_build", + uninstall ? "coverage_script_uninstall" : "coverage_script_disable"}, + }; + for (size_t i = 0; i < sizeof(scripts) / sizeof(scripts[0]); i++) { + char script[CLI_BUF_8K]; + if (scripts[i].build(binary_path, script, sizeof(script)) != CLI_OK) { + record_agent_config_error(uninstall, "Augment/Auggie", scripts[i].build_op, + scripts[i].path); + } else if (cbm_text_remove_owned_document(scripts[i].path, script) < 0) { + record_agent_config_error(uninstall, "Augment/Auggie", scripts[i].remove_op, + scripts[i].path); + } + } +} + static void install_claude_code_config(const char *home, const char *binary_path, bool force, bool dry_run) { char config_dir[CLI_BUF_1K]; @@ -11326,6 +11355,7 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents int session_result = cbm_remove_augment_session_hook(cp, session_hp); int coverage_result = cbm_remove_augment_coverage_hook(cp, coverage_hp); hook_ok = session_result == CLI_OK && coverage_result == CLI_OK; + remove_augment_owned_hook_scripts(binary_path, session_hp, coverage_hp, false); } else if (!cbm_install_augment_session_script(binary_path, session_hp)) { hook_ok = false; record_agent_config_error(false, "Augment/Auggie", "session_script_install", @@ -13701,30 +13731,7 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con if (cbm_remove_augment_coverage_hook(cp, coverage_hp) != CLI_OK) { record_agent_config_error(true, "Augment/Auggie", "coverage_hook_uninstall", cp); } - char session_script[CLI_BUF_8K]; - if (cbm_build_augment_session_script(binary_path, session_script, - sizeof(session_script)) != CLI_OK) { - record_agent_config_error(true, "Augment/Auggie", "session_script_build", - session_hp); - } else { - int script_rc = cbm_text_remove_owned_document(session_hp, session_script); - if (script_rc < 0) { - record_agent_config_error(true, "Augment/Auggie", "session_script_uninstall", - session_hp); - } - } - char coverage_script[CLI_BUF_8K]; - if (cbm_build_augment_coverage_script(binary_path, coverage_script, - sizeof(coverage_script)) != CLI_OK) { - record_agent_config_error(true, "Augment/Auggie", "coverage_script_build", - coverage_hp); - } else { - int script_rc = cbm_text_remove_owned_document(coverage_hp, coverage_script); - if (script_rc < 0) { - record_agent_config_error(true, "Augment/Auggie", "coverage_script_uninstall", - coverage_hp); - } - } + remove_augment_owned_hook_scripts(binary_path, session_hp, coverage_hp, true); } printf(" removed SessionStart + PostToolUse hooks + dedicated subagent\n"); } diff --git a/tests/test_cli.c b/tests/test_cli.c index 569c9353d..82b65975e 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -2891,6 +2891,131 @@ TEST(cli_no_hooks_removes_codex_pretool_hook) { PASS(); } +/* The Augment SessionStart and PostToolUse view scripts follow the persisted + * opt-out the way uninstall treats them: --no-hooks deletes both exact-owned + * scripts and keeps the MCP entry, a bare install keeps them off, --hooks + * restores them, and a script the user edited is kept. */ +TEST(cli_no_hooks_removes_augment_owned_scripts) { + char tmpdir[512]; + snprintf(tmpdir, sizeof(tmpdir), "%s/cli-augment-no-hooks-XXXXXX", cbm_tmpdir()); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + + const char *const env_names[] = {"HOME", "CBM_CACHE_DIR", "PATH"}; + char *saved_env[sizeof(env_names) / sizeof(env_names[0])]; + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + saved_env[i] = save_test_env(env_names[i]); + cbm_unsetenv(env_names[i]); + } + + char cache_dir[640]; + char augment_dir[640]; + char settings_path[768]; + char session_path[768]; + char coverage_path[768]; + snprintf(cache_dir, sizeof(cache_dir), "%s/cache", tmpdir); + snprintf(augment_dir, sizeof(augment_dir), "%s/.augment", tmpdir); + snprintf(settings_path, sizeof(settings_path), "%s/settings.json", augment_dir); +#ifdef _WIN32 + snprintf(session_path, sizeof(session_path), "%s/hooks/codebase-memory-session.ps1", + augment_dir); + snprintf(coverage_path, sizeof(coverage_path), "%s/hooks/codebase-memory-coverage.ps1", + augment_dir); +#else + snprintf(session_path, sizeof(session_path), "%s/hooks/codebase-memory-session.sh", + augment_dir); + snprintf(coverage_path, sizeof(coverage_path), "%s/hooks/codebase-memory-coverage.sh", + augment_dir); +#endif + test_mkdirp(augment_dir); + /* An installed binary at the default target is the path the scripts embed; + * without it --skip-binary would configure the test runner itself. */ + char binary_dir[640]; + char binary_path[768]; + snprintf(binary_dir, sizeof(binary_dir), "%s/.local/bin", tmpdir); + test_mkdirp(binary_dir); +#ifdef _WIN32 + snprintf(binary_path, sizeof(binary_path), "%s/codebase-memory-mcp.exe", binary_dir); +#else + snprintf(binary_path, sizeof(binary_path), "%s/codebase-memory-mcp", binary_dir); +#endif + write_test_file(binary_path, "installed binary\n"); + cbm_setenv("HOME", tmpdir, 1); + cbm_setenv("CBM_CACHE_DIR", cache_dir, 1); + cbm_setenv("PATH", tmpdir, 1); + + cli_activation_fake_t fake = {.mutation_reserve_result = 1}; + cbm_cli_activation_ops_t ops = cli_activation_fake_ops(&fake); + cbm_cli_set_activation_ops_for_test(&ops); + + char *default_args[] = {"--skip-binary", "--clients=augment", "--yes"}; + int default_rc = cli_test_cmd_install(3, default_args); + bool default_scripts = + default_rc == 0 && cbm_file_exists(session_path) && cbm_file_exists(coverage_path); + + char *no_hooks_args[] = {"--no-hooks", "--skip-binary", "--clients=augment", "--yes"}; + int no_hooks_rc = cli_test_cmd_install(4, no_hooks_args); + bool scripts_removed = + no_hooks_rc == 0 && !cbm_file_exists(session_path) && !cbm_file_exists(coverage_path); + char *settings = read_test_file_alloc(settings_path); + bool mcp_kept = settings && strstr(settings, "mcpServers") && + strstr(settings, "codebase-memory-mcp") && + !strstr(settings, "codebase-memory-session") && + !strstr(settings, "codebase-memory-coverage"); + free(settings); + + int bare_rc = cli_test_cmd_install(3, default_args); + bool bare_disabled = + bare_rc == 0 && !cbm_file_exists(session_path) && !cbm_file_exists(coverage_path); + + char *hooks_args[] = {"--hooks", "--skip-binary", "--clients=augment", "--yes"}; + int hooks_rc = cli_test_cmd_install(4, hooks_args); + settings = read_test_file_alloc(settings_path); + bool restored = hooks_rc == 0 && cbm_file_exists(session_path) && + cbm_file_exists(coverage_path) && settings && + strstr(settings, "codebase-memory-session") && + strstr(settings, "codebase-memory-coverage"); + free(settings); + + /* A script the user edited is no longer the owned document: the opt-out + * keeps it, as uninstall does, and still deletes the untouched one. */ + char *owned_session = read_test_file_alloc(session_path); + char user_session[4096]; + snprintf(user_session, sizeof(user_session), "%s# local edit\n", + owned_session ? owned_session : ""); + free(owned_session); + write_test_file(session_path, user_session); + int user_rc = cli_test_cmd_install(4, no_hooks_args); + char *kept_session = read_test_file_alloc(session_path); + bool user_script_kept = user_rc == 0 && kept_session && + strcmp(kept_session, user_session) == 0 && + !cbm_file_exists(coverage_path); + free(kept_session); + + /* End on --hooks, as the Codex case does: the opt-out is process state, + * and later tests install without the command line. */ + (void)remove(session_path); + int final_rc = cli_test_cmd_install(4, hooks_args); + bool final_restored = + final_rc == 0 && cbm_file_exists(session_path) && cbm_file_exists(coverage_path); + + cbm_cli_set_activation_ops_for_test(NULL); + cbm_set_auto_answer_for_test(0); + for (size_t i = 0U; i < sizeof(env_names) / sizeof(env_names[0]); i++) { + restore_test_env(env_names[i], saved_env[i]); + } + test_rmdir_r(tmpdir); + + ASSERT_TRUE(default_scripts); + ASSERT_TRUE(mcp_kept); + ASSERT_TRUE(restored); + ASSERT_TRUE(scripts_removed); + ASSERT_TRUE(bare_disabled); + ASSERT_TRUE(user_script_kept); + ASSERT_TRUE(final_restored); + PASS(); +} + TEST(cli_install_reset_deletion_waits_for_final_activation_guard) { char tmpdir[256]; snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-daemon-install-reset-XXXXXX"); @@ -20582,6 +20707,7 @@ SUITE(cli) { RUN_TEST(cli_install_hooks_preference_persists_and_migrates_owned_state); RUN_TEST(cli_no_hooks_plan_covers_every_hook_installer_family); RUN_TEST(cli_no_hooks_removes_codex_pretool_hook); + RUN_TEST(cli_no_hooks_removes_augment_owned_scripts); RUN_TEST(cli_install_reset_deletion_waits_for_final_activation_guard); RUN_TEST(cli_remove_indexes_deletes_orphan_sqlite_sidecars_issue2054); RUN_TEST(cli_index_enumeration_skips_internal_cache_dbs); From 79a5734112d58523f3632403515f4d1fa940f127 Mon Sep 17 00:00:00 2001 From: Tanishq Date: Tue, 3 Nov 2026 07:32:40 +0530 Subject: [PATCH 17/19] fix(pipeline): skip a file named nul in the control-file walk instead of aborting the index (#2646) semantic_manifest_control_entry() reads the metadata of every entry the control-file walk lists, and one failed read fails the whole manifest. On Win32 the plain path \nul names the NUL device, so a real file named nul (Git Bash leaves one behind after `cmd > nul` or `2>nul`) cannot be read: GetFileAttributesExW fails with ERROR_INVALID_PARAMETER. The manifest returns CBM_NOT_FOUND and every index run on the repository ends in CBM_PIPELINE_ABORT_PRESERVE_DB (aborted_previous_preserved). Listing nul in .gitignore does not help, because the walk visits every entry. Only directories and control files can contribute to the manifest. Decide the control name first and skip an entry that the listing already shows to be neither, without reading it. A Windows listing always carries the attributes; on POSIX an entry whose d_type is DT_UNKNOWN is still read. Directories and control-named entries are read exactly as before. Signed-off-by: Tanishq --- src/pipeline/pipeline_incremental.c | 51 +++++++++---- tests/test_pipeline.c | 106 ++++++++++++++++++++++++++++ 2 files changed, 144 insertions(+), 13 deletions(-) diff --git a/src/pipeline/pipeline_incremental.c b/src/pipeline/pipeline_incremental.c index ac2c253af..3bf321107 100644 --- a/src/pipeline/pipeline_incremental.c +++ b/src/pipeline/pipeline_incremental.c @@ -379,10 +379,37 @@ static int semantic_manifest_walk_controls(semantic_manifest_builder_t *builder, const char *rel_dir, int depth, char **excluded_dirs, int excluded_count); +/* Control files are the only regular files the walk records. */ +static bool semantic_manifest_control_name(const char *rel_dir, const char *name) { + bool root_control = (!rel_dir || !rel_dir[0]) && (strcmp(name, ".cbmignore") == 0 || + strcmp(name, ".codebase-memory.json") == 0); + /* A build context's ignore file (or BuildKit's .dockerignore) + * shapes which files an image holds (pass_docker.c image_ignore), and + * discovery indexes neither. */ + static const char DOCKERIGNORE[] = ".dockerignore"; + size_t nl = strlen(name); + bool dockerignore = nl >= sizeof(DOCKERIGNORE) - SKIP_ONE && + strcmp(name + nl - (sizeof(DOCKERIGNORE) - SKIP_ONE), DOCKERIGNORE) == 0; + return strcmp(name, ".gitignore") == 0 || root_control || + semantic_manifest_package_control(name) || dockerignore; +} + +/* True when the directory listing alone shows that the entry is not a + * directory. A Windows listing always carries the attributes; on POSIX a + * d_type of DT_UNKNOWN (0), which readdir could not resolve, proves nothing. */ +static bool semantic_manifest_listed_non_directory(const cbm_dirent_t *entry) { +#ifdef _WIN32 + return !entry->is_dir; +#else + return entry->d_type != 0 && !entry->is_dir; +#endif +} + static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder, const char *project, const char *abs_dir, const char *rel_dir, int depth, char **excluded_dirs, - int excluded_count, const char *name) { + int excluded_count, const cbm_dirent_t *entry) { + const char *name = entry->name; if (strcmp(name, ".") == 0 || strcmp(name, "..") == 0) return 0; char abs_path[CBM_SZ_4K], rel_path[CBM_SZ_4K]; @@ -392,6 +419,15 @@ static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder, if (abs_n < 0 || (size_t)abs_n >= sizeof(abs_path) || rel_n < 0 || (size_t)rel_n >= sizeof(rel_path)) return CBM_NOT_FOUND; + /* Only directories and control files contribute, so an entry the listing + * already shows to be neither is skipped without reading its metadata. + * That read can fail on such an entry: on Windows the plain path of a file + * named nul names the NUL device (#2646), and the plain path of a name + * ending in a dot or a space does not reach the file. */ + bool control = semantic_manifest_control_name(rel_dir, name); + if (!control && semantic_manifest_listed_non_directory(entry)) { + return 0; + } cbm_path_info_t info; if (cbm_path_info_utf8(abs_path, &info) != 0) return CBM_NOT_FOUND; @@ -404,17 +440,6 @@ static int semantic_manifest_control_entry(semantic_manifest_builder_t *builder, return semantic_manifest_walk_controls(builder, project, abs_path, rel_path, depth + 1, excluded_dirs, excluded_count); } - bool root_control = (!rel_dir || !rel_dir[0]) && (strcmp(name, ".cbmignore") == 0 || - strcmp(name, ".codebase-memory.json") == 0); - /* A build context's ignore file (or BuildKit's .dockerignore) - * shapes which files an image holds (pass_docker.c image_ignore), and - * discovery indexes neither. */ - static const char DOCKERIGNORE[] = ".dockerignore"; - size_t nl = strlen(name); - bool dockerignore = nl >= sizeof(DOCKERIGNORE) - SKIP_ONE && - strcmp(name + nl - (sizeof(DOCKERIGNORE) - SKIP_ONE), DOCKERIGNORE) == 0; - bool control = strcmp(name, ".gitignore") == 0 || root_control || - semantic_manifest_package_control(name) || dockerignore; return info.is_regular && control ? semantic_manifest_add(builder, project, rel_path, abs_path) : 0; } @@ -439,7 +464,7 @@ static int semantic_manifest_walk_controls(semantic_manifest_builder_t *builder, rc = cbm_pipeline_frozen_checkpoint(builder->frozen); if (rc == 0) rc = semantic_manifest_control_entry(builder, project, abs_dir, rel_dir, depth, - excluded_dirs, excluded_count, entry->name); + excluded_dirs, excluded_count, entry); } cbm_closedir(dir); return rc; diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 1958fdb5e..58c6d796d 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -9657,6 +9657,108 @@ TEST(pipeline_semantic_manifest_skips_unreadable_subdirectory) { } #endif /* !_WIN32 */ +#ifdef _WIN32 +/* Create or delete a real file named nul in dir, as a Git Bash redirect such + * as `cmd > nul` leaves behind (#2646). The plain Win32 path /nul names + * the NUL device, so the file is reached only through the extended-length + * namespace, which needs an absolute path with backslashes (CI's TEMP uses + * forward slashes). */ +static bool pipeline_test_nul_file(const char *dir, bool create) { + enum { NUL_PATH_CAP = 1024, PREFIX_LEN = 4, LEAF_LEN = 4 }; + static const wchar_t prefix[] = L"\\\\?\\"; + static const wchar_t leaf[] = L"\\nul"; + wchar_t path[NUL_PATH_CAP]; + const DWORD room = NUL_PATH_CAP - PREFIX_LEN - LEAF_LEN; + wchar_t *wide_dir = cbm_utf8_to_wide(dir); + DWORD length = wide_dir ? GetFullPathNameW(wide_dir, room, path + PREFIX_LEN, NULL) : 0; + free(wide_dir); + if (length == 0 || length >= room) { + return false; + } + wmemcpy(path, prefix, PREFIX_LEN); + wmemcpy(path + PREFIX_LEN + length, leaf, LEAF_LEN + 1); + if (!create) { + return DeleteFileW(path) != 0; + } + HANDLE file = + CreateFileW(path, GENERIC_WRITE, 0, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL); + if (file == INVALID_HANDLE_VALUE) { + return false; + } + CloseHandle(file); + return true; +} + +/* #2646: the control-file walk cannot inspect a file named nul through the + * plain path, and it never needs to: only directories and control files can + * contribute to the manifest. Listing nul in .gitignore does not help, since + * the walk visits every entry. */ +TEST(pipeline_semantic_manifest_skips_nul_device_file_issue2646) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_manifest_nul_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + write_temp_file(tmp, "app.py", "def NulNeighbor():\n return 1\n"); + write_temp_file(tmp, ".gitignore", "nul\n"); + bool created = pipeline_test_nul_file(tmp, true); + + cbm_file_hash_t *manifest = NULL; + int manifest_count = -1; + int rc = cbm_pipeline_build_semantic_manifest(NULL, "manifest-skip-nul", tmp, NULL, 0, NULL, 0, + NULL, NULL, &manifest, &manifest_count); + bool found_gitignore = false; + for (int i = 0; i < manifest_count; i++) { + if (manifest[i].rel_path && strcmp(manifest[i].rel_path, ".gitignore") == 0) { + found_gitignore = true; + } + } + cbm_pipeline_free_semantic_manifest(manifest, manifest_count > 0 ? manifest_count : 0); + bool deleted = created && pipeline_test_nul_file(tmp, false); + th_rmtree(tmp); + + ASSERT_TRUE(created); + ASSERT_EQ(rc, 0); + ASSERT_TRUE(found_gitignore); + ASSERT_TRUE(deleted); + PASS(); +} + +/* The user-visible half of #2646: one such file made every index run return + * CBM_PIPELINE_ABORT_PRESERVE_DB (aborted_previous_preserved), so a new + * project never got an index. */ +TEST(pipeline_full_index_survives_nul_device_file_issue2646) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_index_nul_XXXXXX"); + ASSERT_NOT_NULL(cbm_mkdtemp(tmp)); + write_temp_file(tmp, "app.py", "def NulNeighbor():\n return 1\n"); + write_temp_file(tmp, ".gitignore", "nul\n"); + bool created = pipeline_test_nul_file(tmp, true); + + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/generation.db", tmp); + cbm_pipeline_t *pipeline = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL); + int run_rc = pipeline ? cbm_pipeline_run(pipeline) : -1; + char project[256] = ""; + if (pipeline) { + snprintf(project, sizeof(project), "%s", cbm_pipeline_project_name(pipeline)); + cbm_pipeline_free(pipeline); + } + int indexed = -1; + cbm_store_t *store = cbm_store_open_path_query(db_path); + if (store) { + indexed = named_node_count(store, project, "NulNeighbor"); + cbm_store_close(store); + } + bool deleted = created && pipeline_test_nul_file(tmp, false); + th_rmtree(tmp); + + ASSERT_TRUE(created); + ASSERT_EQ(run_rc, 0); + ASSERT_EQ(indexed, 1); + ASSERT_TRUE(deleted); + PASS(); +} +#endif /* _WIN32 */ + /* A fully validated staged graph must be able to recover from a definitely * non-SQLite destination without deleting evidence or overwriting an earlier * quarantine. The replacement happens only after the corrupt bytes are moved. */ @@ -31994,6 +32096,10 @@ SUITE(pipeline_semantic_manifest_repro) { RUN_TEST(pipeline_semantic_manifest_rejects_non_directory_root); #ifndef _WIN32 RUN_TEST(pipeline_semantic_manifest_skips_unreadable_subdirectory); +#endif +#ifdef _WIN32 + RUN_TEST(pipeline_semantic_manifest_skips_nul_device_file_issue2646); + RUN_TEST(pipeline_full_index_survives_nul_device_file_issue2646); #endif RUN_TEST(pipeline_full_reindex_quarantines_corrupt_destination_without_overwrite); RUN_TEST(pipeline_full_reindex_replaces_legacy_schema_without_quarantine); From e750c0531ae5c6f239703d990a73a32ac2c2f27f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 02:50:49 +0100 Subject: [PATCH 18/19] ci(freebsd): add a dispatch-only, informational FreeBSD build workflow Add .github/workflows/freebsd.yml, a standalone workflow triggered only by workflow_dispatch. It builds the UI-embedded binary inside FreeBSD 15.1 and 14.4 x86_64 VMs (vmactions/freebsd-vm, full-SHA pin to v1.5.9) with the canonical scripts/build.sh and runs the binary with --version. It declares read-only permissions, uploads and publishes nothing, is not part of ci-ok and is not called by release.yml or dry-run.yml, so a FreeBSD red never gates a pull request or a release. scripts/env.sh now detects FreeBSD and selects gmake there (BSD make cannot read the GNU make syntax of Makefile.cbm) and make everywhere else; the environment summary line reports the choice. scripts/build.sh runs $MAKE at all three build call sites. macOS, Linux and Windows still run make, so their builds are unchanged. Distilled from #1422 by PR9000: the release matrix, the x86_64-host choice, the aarch64 exclusion, the vmactions VM approach with a SHA pin, fail-fast: false and the gmake detection all come from that pull request, which built on the FreeBSD groundwork of #1093, #1423 and #1467. Co-authored-by: Pedro Ramos <131530838+pr9000@users.noreply.github.com> Signed-off-by: Martin Vogel --- .github/workflows/freebsd.yml | 66 +++++++++++++++++++++++++++++++++++ scripts/build.sh | 9 ++--- scripts/env.sh | 17 +++++++-- 3 files changed, 86 insertions(+), 6 deletions(-) create mode 100644 .github/workflows/freebsd.yml diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml new file mode 100644 index 000000000..ad3de23dd --- /dev/null +++ b/.github/workflows/freebsd.yml @@ -0,0 +1,66 @@ +# FreeBSD build, INFORMATIONAL ONLY. Builds the UI-embedded binary (the one +# every platform ships) inside a FreeBSD VM with the canonical scripts/build.sh, +# then checks that the binary starts (--version). +# +# NON-GATING: workflow_dispatch only. This workflow is not part of the ci-ok +# aggregate and is not called by release.yml or dry-run.yml, so a FreeBSD red +# never blocks a pull request or a release. Nothing from it is uploaded or +# published, and it runs with read-only permissions. +name: FreeBSD build (informational) + +on: + workflow_dispatch: + +permissions: + contents: read + +jobs: + build-freebsd: + name: build (freebsd-${{ matrix.release }}, ${{ matrix.arch }}, clang) + strategy: + fail-fast: false + matrix: + release: ['15.1', '14.4'] + arch: ['x86_64'] + # arch: ['x86_64', 'aarch64'] + # aarch64 is disabled for now: lightningcss (a native Rust dependency + # of the Vite/CSS build) publishes no prebuilt binary for freebsd-arm64, + # so `npm run build` fails with "Cannot find module + # '../lightningcss.freebsd-arm64.node'". That is a gap in the package's + # platform coverage, not something fixable on our side without + # vendoring a Rust build of it. Re-enable once lightningcss ships that + # target, or once the frontend build runs natively on a Linux host + # instead of inside the FreeBSD VM. + include: + - arch: x86_64 + sync: rsync + # - arch: aarch64 + # sync: scp + # Must be an x86_64 host (ubuntu-latest). QEMU guest emulation on the arm + # runners (ubuntu-*-arm) degrades so badly that runs hit 3+ hour timeouts. + runs-on: ubuntu-latest + timeout-minutes: 90 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false + + - name: Build and run inside a FreeBSD ${{ matrix.release }} (${{ matrix.arch }}) VM + # https://github.com/vmactions/freebsd-vm + uses: vmactions/freebsd-vm@c46abacb49f09938ca4e1702d15d836285d694cc # v1.5.9 + with: + release: ${{ matrix.release }} + arch: ${{ matrix.arch }} + # Sync options: rsync, sshfs, nfs, scp + sync: ${{ matrix.sync }} + # Nothing from the VM is used after the run, so skip the copy back. + copyback: false + mem: 6144 + cpu: 4 + prepare: | + pkg update -f + pkg install -y gmake sqlite3 pkgconf node22 npm-node22 bash + which python3 >/dev/null 2>&1 || ln -sf $(which python3.15 python3.14 python3.13 python3.12 python3.11 python3.10 2>/dev/null | head -n 1) /usr/local/bin/python3 + run: | + scripts/build.sh --with-ui CC=clang CXX=clang++ + build/c/codebase-memory-mcp --version diff --git a/scripts/build.sh b/scripts/build.sh index ad646c5de..a8035f9cd 100755 --- a/scripts/build.sh +++ b/scripts/build.sh @@ -193,17 +193,18 @@ else cbm_remove_build_dir "$ROOT" "$BUILD_DIR" fi -# Step 2: Build (Makefile applies $ARCHFLAGS for the target arch on macOS) +# Step 2: Build (Makefile applies $ARCHFLAGS for the target arch on macOS; +# $MAKE comes from env.sh: gmake on FreeBSD, make elsewhere) if [[ "$PHASE" == vendored ]]; then - make -j"$NPROC" -f Makefile.cbm cbm-third-party \ + "$MAKE" -j"$NPROC" -f Makefile.cbm cbm-third-party \ CFLAGS_EXTRA="$CFLAGS_EXTRA" "${EXTRA_MAKE_ARGS[@]+"${EXTRA_MAKE_ARGS[@]}"}" echo "=== Third-party objects built in ${BUILD_DIR} (no binary) ===" exit 0 elif $WITH_UI; then - make -j"$NPROC" -f Makefile.cbm cbm-with-ui \ + "$MAKE" -j"$NPROC" -f Makefile.cbm cbm-with-ui \ CFLAGS_EXTRA="$CFLAGS_EXTRA" "${EXTRA_MAKE_ARGS[@]+"${EXTRA_MAKE_ARGS[@]}"}" else - make -j"$NPROC" -f Makefile.cbm cbm "${RETRACE_ARGS[@]+"${RETRACE_ARGS[@]}"}" \ + "$MAKE" -j"$NPROC" -f Makefile.cbm cbm "${RETRACE_ARGS[@]+"${RETRACE_ARGS[@]}"}" \ CFLAGS_EXTRA="$CFLAGS_EXTRA" "${EXTRA_MAKE_ARGS[@]+"${EXTRA_MAKE_ARGS[@]}"}" fi diff --git a/scripts/env.sh b/scripts/env.sh index e23affe4c..13ece00b5 100755 --- a/scripts/env.sh +++ b/scripts/env.sh @@ -6,8 +6,9 @@ # Exports: # ARCH — target architecture (arm64 / x86_64) # ARCHFLAGS — "-arch " on macOS (target slice for clang/ld), empty elsewhere +# MAKE — make program for Makefile.cbm (gmake on FreeBSD, make elsewhere) # NPROC — number of CPU cores -# OS — darwin / linux / windows +# OS — darwin / linux / windows / freebsd set -euo pipefail @@ -16,10 +17,22 @@ OS="$(uname -s | tr '[:upper:]' '[:lower:]')" case "$OS" in darwin) OS="darwin" ;; linux) OS="linux" ;; + freebsd*) OS="freebsd" ;; mingw*|msys*|cygwin*) OS="windows" ;; *) OS="unknown" ;; esac +# ── Select the make program ──────────────────────────────────── +# Makefile.cbm is written in GNU make syntax. FreeBSD ships BSD make as its +# default make, so builds there run gmake (pkg install gmake); every other +# platform runs make. Not exported: build.sh sources this file and runs +# $MAKE in the same shell. +if [[ "$OS" == "freebsd" ]]; then + MAKE="gmake" +else + MAKE="make" +fi + # ── Detect / override architecture ───────────────────────────── # Default: native HARDWARE architecture (not Rosetta-translated). # On macOS under Rosetta, uname -m returns x86_64 even on Apple Silicon. @@ -128,5 +141,5 @@ fi # ── Print environment summary ────────────────────────────────── print_env() { local context="$1" - echo "=== $context: os=$OS arch=$ARCH cores=$NPROC cc=${CC:-default} cxx=${CXX:-default} ===" + echo "=== $context: os=$OS arch=$ARCH cores=$NPROC cc=${CC:-default} cxx=${CXX:-default} make=$MAKE ===" } From f13c84735518ec5e468245a4b93cef1a013a91c7 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 18:11:59 +0100 Subject: [PATCH 19/19] ci(freebsd): build the headless binary in the FreeBSD VM The first proof run failed in both FreeBSD legs at `npm ci`: the UI dependency tree pulls in onnxruntime-node, which publishes builds for win32, darwin and linux only, so npm stops with EBADPLATFORM before the frontend is built. The VM boot, the pkg prepare step, the gmake detection and the source sync all worked. The job now builds the headless binary (scripts/build.sh without --with-ui) and drops node22/npm-node22 from the prepare step. That still proves the C core builds and starts on FreeBSD; the UI bundle is platform-independent JavaScript and is built on every other leg. The aarch64 comment now says what is actually unproven. Signed-off-by: Martin Vogel --- .github/workflows/freebsd.yml | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/.github/workflows/freebsd.yml b/.github/workflows/freebsd.yml index ad3de23dd..88daf2545 100644 --- a/.github/workflows/freebsd.yml +++ b/.github/workflows/freebsd.yml @@ -1,6 +1,9 @@ -# FreeBSD build, INFORMATIONAL ONLY. Builds the UI-embedded binary (the one -# every platform ships) inside a FreeBSD VM with the canonical scripts/build.sh, -# then checks that the binary starts (--version). +# FreeBSD build, INFORMATIONAL ONLY. Builds the headless binary (the C core, +# no embedded UI) inside a FreeBSD VM with the canonical scripts/build.sh, +# then checks that the binary starts (--version). The UI build is not run here: +# its npm dependency tree pulls in onnxruntime-node, which publishes no FreeBSD +# build, so `npm ci` stops with EBADPLATFORM. The UI bundle itself is +# platform-independent JavaScript and is built on every other leg. # # NON-GATING: workflow_dispatch only. This workflow is not part of the ci-ok # aggregate and is not called by release.yml or dry-run.yml, so a FreeBSD red @@ -23,14 +26,10 @@ jobs: release: ['15.1', '14.4'] arch: ['x86_64'] # arch: ['x86_64', 'aarch64'] - # aarch64 is disabled for now: lightningcss (a native Rust dependency - # of the Vite/CSS build) publishes no prebuilt binary for freebsd-arm64, - # so `npm run build` fails with "Cannot find module - # '../lightningcss.freebsd-arm64.node'". That is a gap in the package's - # platform coverage, not something fixable on our side without - # vendoring a Rust build of it. Re-enable once lightningcss ships that - # target, or once the frontend build runs natively on a Linux host - # instead of inside the FreeBSD VM. + # aarch64 is disabled for now: it has not been proven with this + # workflow. Its earlier blocker was the frontend build (lightningcss + # publishes no freebsd-arm64 binary), which this headless job no + # longer runs. Re-enable after a green aarch64 proof run. include: - arch: x86_64 sync: rsync @@ -59,8 +58,8 @@ jobs: cpu: 4 prepare: | pkg update -f - pkg install -y gmake sqlite3 pkgconf node22 npm-node22 bash + pkg install -y gmake sqlite3 pkgconf bash which python3 >/dev/null 2>&1 || ln -sf $(which python3.15 python3.14 python3.13 python3.12 python3.11 python3.10 2>/dev/null | head -n 1) /usr/local/bin/python3 run: | - scripts/build.sh --with-ui CC=clang CXX=clang++ + scripts/build.sh CC=clang CXX=clang++ build/c/codebase-memory-mcp --version