From 67af19e83baaef31ba64c7e7dcae1bf671093fdc Mon Sep 17 00:00:00 2001 From: Andy11-cpu Date: Thu, 27 Aug 2026 13:53:22 -0400 Subject: [PATCH 01/24] 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/24] 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/24] 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 e0a6133293556d87de6c2ca43de674e3a643e339 Mon Sep 17 00:00:00 2001 From: Vincent Koc <25068+vincentkoc@users.noreply.github.com> Date: Mon, 14 Sep 2026 10:26:17 +0200 Subject: [PATCH 04/24] fix(daemon): preserve disabled UI on manual start Signed-off-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com> --- docs/CONFIGURATION.md | 2 + src/main.c | 22 ++++++--- tests/test_daemon_open_readiness.py | 77 +++++++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 6 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 862d591c3..cd28ced1c 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -136,6 +136,8 @@ Current format: Notes: - If a UI-enabled binary finds its verified external asset pack and no UI config file exists yet, the UI auto-enables on first run. Missing or invalid assets leave the MCP/daemon service available and keep the UI disabled. +- A bare `daemon start` honors the saved `ui_enabled` value, including `false`. Enable the UI with `codebase-memory-mcp config set ui_enabled true`. +- On a cold start, `daemon start --open` or `daemon start --port=N` explicitly enables the UI and persists that choice. An already-active daemon keeps its current settings; stop it first to change ports with `--port=N`. - `CBM_CACHE_DIR` changes both the UI config location and the runtime settings database location. - CBM resolves `CBM_CACHE_DIR` to one canonical per-account cache root. A process configured with a different root fails while any CBM session or command is active; close them before switching roots. diff --git a/src/main.c b/src/main.c index 284a899cb..0e65ab5f2 100644 --- a/src/main.c +++ b/src/main.c @@ -2333,7 +2333,7 @@ static void main_daemon_ctl_print_ui_configuration(void) { "`daemon start --open` verifies it)\n", ui_config.ui_port); } else { - printf(" ui: disabled (enable with `daemon start` in a UI build)\n"); + printf(" ui: disabled (enable with `config set ui_enabled true`)\n"); } } @@ -2556,8 +2556,12 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo cbm_ui_config_t ui_config; cbm_ui_config_load(&ui_config); if (!ui_config.ui_enabled) { - (void)fprintf(stderr, "error: UI is disabled for the active daemon; browser was not " - "opened\n"); + main_daemon_ctl_print_ui_configuration(); + if (open_browser) { + (void)fprintf(stderr, + "error: UI is disabled for the active daemon; browser was not " + "opened\n"); + } return open_browser ? EXIT_FAILURE : EXIT_SUCCESS; } if (requested_port > 0 && requested_port != ui_config.ui_port) { @@ -2612,9 +2616,12 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo * startup window; configure the UI before departing. */ int ui_port = 0; bool ui_configured = false; - if ((CBM_EMBEDDED_FILE_COUNT > 0)) { - cbm_ui_config_t ui_config; - cbm_ui_config_load(&ui_config); + cbm_ui_config_t ui_config; + cbm_ui_config_load(&ui_config); + /* A bare start inherits the saved choice, just like automatic startup. + * Only an explicit UI request may re-enable a deliberately disabled UI. */ + bool ui_requested = ui_config.ui_enabled || requested_port > 0 || open_browser; + if (CBM_EMBEDDED_FILE_COUNT > 0 && ui_requested) { ui_port = requested_port > 0 ? requested_port : ui_config.ui_port; uint8_t update_mask = 0x03U; /* enabled + port */ bool context_set = @@ -2660,6 +2667,9 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo } printf("It survives idle periods and session ends; `codebase-memory-mcp daemon stop` " "retires it.\n"); + if (CBM_EMBEDDED_FILE_COUNT > 0 && !ui_requested) { + main_daemon_ctl_print_ui_configuration(); + } /* Skipped when the handshake was refused: that path announces the port as * warming, which would be a promise nothing is keeping. */ int ui_result = diff --git a/tests/test_daemon_open_readiness.py b/tests/test_daemon_open_readiness.py index 08f7d851a..b3177eb54 100644 --- a/tests/test_daemon_open_readiness.py +++ b/tests/test_daemon_open_readiness.py @@ -6,6 +6,8 @@ endpoint*, not merely for the daemon process, before it reports or opens the URL This guard uses a real UI build and isolated daemon generations: +* a saved disabled UI survives cold and repeated bare starts, while explicit + UI requests and saved enabled settings still start the listener; * a temporarily occupied port proves the command waits and then succeeds once the verified UI listener becomes available; * a foreign service returning the exact formerly accepted HTML markers proves @@ -22,6 +24,7 @@ performed and the two negative cases can use a short deterministic deadline: Exit code: 0 == green, 1 == behavior regression, 2 == fixture/setup error. """ +import json import os import re import shutil @@ -327,6 +330,76 @@ def assert_active_daemon_open(binary, work): stop_daemon(binary, env, daemon_pid) +def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): + name = "enabled" if enabled else "disabled" + name += "-" + ("-".join(flag.lstrip("-") for flag in flags) or "bare") + cache = os.path.join(work, "cache-" + name) + marker = os.path.join(work, "browser-" + name + ".txt") + probe, port = occupied_loopback_port() + probe.close() + os.makedirs(cache, exist_ok=True) + env = fixture_environment(work, cache, marker, 10000) + config_path = os.path.join(cache, "config.json") + daemon_pid = 0 + try: + for key, value in (("ui_enabled", str(enabled).lower()), ("ui_port", str(port))): + configured = subprocess.run([binary, "config", "set", key, value], + capture_output=True, timeout=10, env=env, cwd=work) + if configured.returncode != 0: + print("SETUP FAIL: could not persist UI setting:\n%s" % output_text(configured)) + return False + with open(config_path, "rb") as handle: + original_config = handle.read() + arguments = ["--port=%d" % port if flag == "--port" else flag for flag in flags] + first = subprocess.run([binary, "daemon", "start", *arguments], capture_output=True, + timeout=30, env=env, cwd=work) + first_text = output_text(first) + daemon_pid = pid_from(first_text) + if first.returncode != 0 or "daemon: started" not in first_text: + print("RED: daemon start failed with saved UI setting (%s):\n%s" % + (name, first_text[:700])) + return False + with open(config_path, "rb") as handle: + current_config = handle.read() + expected_enabled = enabled or bool(flags) + if (json.loads(current_config).get("ui_enabled") != expected_enabled or + (not expected_enabled and current_config != original_config)): + print("RED: daemon start rewrote the saved UI setting (%s): %r" % + (name, current_config)) + return False + if expected_enabled: + opened = subprocess.run([binary, "daemon", "start", "--open"], capture_output=True, + timeout=20, env=env, cwd=work) + if opened.returncode != 0 or not os.path.exists(marker): + print("RED: requested UI did not become ready (%s):\n%s" % + (name, output_text(opened)[:700])) + return False + else: + for arguments in (("start",), ("status",), ("start", "--open")): + result = subprocess.run([binary, "daemon", *arguments], capture_output=True, + timeout=10, env=env, cwd=work) + text = output_text(result) + expected_result = 1 if "--open" in arguments else 0 + if (result.returncode != expected_result or "disabled" not in text or + "warming" in text or os.path.exists(marker)): + print("RED: disabled UI was not preserved by %r:\n%s" % + (arguments, text[:700])) + return False + with open(config_path, "rb") as handle: + if handle.read() != original_config: + print("RED: repeated daemon start rewrote disabled UI config") + return False + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as connection: + connection.settimeout(0.5) + if connection.connect_ex(("127.0.0.1", port)) == 0: + print("RED: disabled UI opened a listener") + return False + print("PASS: daemon start respects saved settings and explicit UI requests (%s)" % name) + return True + finally: + stop_daemon(binary, env, daemon_pid) + + def main(): if len(sys.argv) != 2: print("usage: python3 test_daemon_open_readiness.py ") @@ -346,6 +419,10 @@ def main(): # Darwin temporary root so the product endpoint itself remains valid. short_temp_root = "/private/tmp" if sys.platform == "darwin" else tempfile.gettempdir() with tempfile.TemporaryDirectory(prefix="cbm_uiopen_", dir=short_temp_root) as work: + for enabled, flags in ((False, ()), (True, ()), + (False, ("--port",)), (False, ("--open",))): + if not assert_persisted_ui_setting(binary, work, enabled, flags): + return 1 if not assert_delayed_success(binary, work): return 1 if not assert_bounded_foreign_port_failure(binary, work): 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 05/24] 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 06/24] 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 07/24] 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 08/24] 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 09/24] 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 10/24] 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 11/24] 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 12/24] 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 65dd99d7e64da39155b3dbcdc10dc0db1a83299e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:56:08 +0100 Subject: [PATCH 13/24] refactor(daemon): extract the cold start UI rule into a tested helper The cold daemon start rule this PR introduces (a bare start honours a saved ui_enabled=false; only an explicit --port=N or --open turns the UI on and persists that choice) was covered only by tests/test_daemon_open_readiness.py, a real-process guard that no CI leg runs. Move the rule into a pure helper, cbm_ui_config_daemon_start_ui() in src/ui/config.c, and call it from the cold start path in main.c; the CBM_EMBEDDED_FILE_COUNT gate stays in main.c and the behaviour is unchanged. Add config_daemon_start_honours_saved_ui_choice to the ui suite, which CI runs. It fails if the old always-configure rule comes back (a bare start with a saved disabled UI) and if --port=N or --open stop re-enabling a disabled UI. Refs #2114 Signed-off-by: Martin Vogel --- src/main.c | 7 ++++--- src/ui/config.c | 16 ++++++++++++++++ src/ui/config.h | 9 +++++++++ tests/test_ui.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 3 deletions(-) diff --git a/src/main.c b/src/main.c index 402ee5f64..6c2eea758 100644 --- a/src/main.c +++ b/src/main.c @@ -2735,10 +2735,11 @@ static int main_run_daemon_ctl(int argc, char **argv, const cbm_daemon_ipc_endpo cbm_ui_config_t ui_config; cbm_ui_config_load(&ui_config); /* A bare start inherits the saved choice, just like automatic startup. - * Only an explicit UI request may re-enable a deliberately disabled UI. */ - bool ui_requested = ui_config.ui_enabled || requested_port > 0 || open_browser; + * Only an explicit UI request may re-enable a deliberately disabled UI + * (cbm_ui_config_daemon_start_ui; the ui suite pins the rule). */ + bool ui_requested = + cbm_ui_config_daemon_start_ui(&ui_config, requested_port, open_browser, &ui_port); if (CBM_EMBEDDED_FILE_COUNT > 0 && ui_requested) { - ui_port = requested_port > 0 ? requested_port : ui_config.ui_port; uint8_t update_mask = 0x03U; /* enabled + port */ bool context_set = main_set_client_context(start_result.client, ".", CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL, diff --git a/src/ui/config.c b/src/ui/config.c index 8fc1bbda4..9c1e6b969 100644 --- a/src/ui/config.c +++ b/src/ui/config.c @@ -168,6 +168,22 @@ void cbm_ui_config_load(cbm_ui_config_t *cfg) { cbm_runtime_settings_load_ui(&cfg->ui_enabled, &cfg->ui_port); } +/* ── Daemon start rule ───────────────────────────────────────── */ + +bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port, + bool open_browser, int *port_out) { + if (port_out) { + *port_out = 0; + } + if (!saved || !(saved->ui_enabled || requested_port > 0 || open_browser)) { + return false; + } + if (port_out) { + *port_out = requested_port > 0 ? requested_port : saved->ui_port; + } + return true; +} + /* ── Save ────────────────────────────────────────────────────── */ static bool config_parent_directory(const char *path, char *directory, size_t directory_size) { diff --git a/src/ui/config.h b/src/ui/config.h index 90d4ceabe..36b62222f 100644 --- a/src/ui/config.h +++ b/src/ui/config.h @@ -31,4 +31,13 @@ bool cbm_ui_config_save(const cbm_ui_config_t *cfg); * Exposed for testing. */ void cbm_ui_config_path(char *buf, int bufsz); +/* The cold `daemon start` UI rule (#2114). A bare start honours the saved + * choice, including a deliberately disabled UI; an explicit --port=N + * (requested_port > 0) or --open requests the UI, and the caller persists that + * choice. Returns whether the start configures the UI. *port_out (optional) + * receives the port to use, the requested port winning over the saved one, or + * 0 when no UI is requested. A NULL saved configuration requests nothing. */ +bool cbm_ui_config_daemon_start_ui(const cbm_ui_config_t *saved, int requested_port, + bool open_browser, int *port_out); + #endif /* CBM_UI_CONFIG_H */ diff --git a/tests/test_ui.c b/tests/test_ui.c index 8c866ff52..922363b06 100644 --- a/tests/test_ui.c +++ b/tests/test_ui.c @@ -267,6 +267,52 @@ TEST(config_missing_fields) { PASS(); } +/* #2114: a cold `daemon start` used to configure (and so re-enable) the UI on + * every start, overwriting a saved ui_enabled=false. The rule now: a bare start + * honours the saved choice; only an explicit --port=N or --open requests the UI + * (and the caller persists that choice). The real-process guard + * tests/test_daemon_open_readiness.py covers the same contract end to end, but + * no CI leg runs it, so this pure rule is what CI checks. */ +TEST(config_daemon_start_honours_saved_ui_choice) { + const cbm_ui_config_t disabled = {.ui_enabled = false, .ui_port = 9749}; + const cbm_ui_config_t enabled = {.ui_enabled = true, .ui_port = 9749}; + int port = -1; + + /* Bare start, saved disabled: no UI and nothing to persist (#2114). */ + ASSERT_FALSE(cbm_ui_config_daemon_start_ui(&disabled, 0, false, &port)); + ASSERT_EQ(port, 0); + + /* Bare start, saved enabled: the UI on the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 0, false, &port)); + ASSERT_EQ(port, 9749); + + /* --port=N re-enables a disabled UI on N. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, false, &port)); + ASSERT_EQ(port, 9800); + + /* --open re-enables a disabled UI on the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 0, true, &port)); + ASSERT_EQ(port, 9749); + + /* An explicit --port=N wins over the saved port. */ + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&enabled, 9800, false, &port)); + ASSERT_EQ(port, 9800); + port = -1; + ASSERT_TRUE(cbm_ui_config_daemon_start_ui(&disabled, 9800, true, &port)); + ASSERT_EQ(port, 9800); + + /* No saved configuration requests nothing. */ + port = -1; + ASSERT_FALSE(cbm_ui_config_daemon_start_ui(NULL, 9800, true, &port)); + ASSERT_EQ(port, 0); + + PASS(); +} + /* ── Embedded asset tests ─────────────────────────────────────── */ TEST(embedded_lookup_not_found) { @@ -2734,6 +2780,7 @@ SUITE(ui) { RUN_TEST(config_overwrite); RUN_TEST(config_corrupt_file); RUN_TEST(config_missing_fields); + RUN_TEST(config_daemon_start_honours_saved_ui_choice); /* Embedded assets (stub) */ RUN_TEST(embedded_lookup_not_found); From f31d7e82a0437fd11cb3f86c3769996d29cb289e Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:56:08 +0100 Subject: [PATCH 14/24] test(daemon): read the saved UI setting through config get config set now stores ui_enabled and ui_port in the shared settings database (_config.db), so the readiness guard read of config.json raised FileNotFoundError before its first assertion. Read the pair back with config get ui_enabled and config get ui_port instead; they load the same value every start path uses. The guard compares it after config set, after the first start, and, for a disabled UI, after the repeated start, status and start --open commands. Drop the now unused json import. Signed-off-by: Martin Vogel --- tests/test_daemon_open_readiness.py | 46 +++++++++++++++++++++-------- 1 file changed, 33 insertions(+), 13 deletions(-) diff --git a/tests/test_daemon_open_readiness.py b/tests/test_daemon_open_readiness.py index b3177eb54..f0ef1d6e2 100644 --- a/tests/test_daemon_open_readiness.py +++ b/tests/test_daemon_open_readiness.py @@ -24,7 +24,6 @@ performed and the two negative cases can use a short deterministic deadline: Exit code: 0 == green, 1 == behavior regression, 2 == fixture/setup error. """ -import json import os import re import shutil @@ -330,6 +329,25 @@ def assert_active_daemon_open(binary, work): stop_daemon(binary, env, daemon_pid) +def saved_ui_setting(binary, env, work): + """Return the persisted (ui_enabled, ui_port) pair as ``config get`` reports it. + + ``config set`` keeps the UI keys in the shared settings database + (``_config.db``), not in ``config.json``. Reading them back through the + product's own command checks the value every start path loads, whatever the + storage format. Returns None when a read fails. + """ + values = [] + for key in ("ui_enabled", "ui_port"): + result = subprocess.run([binary, "config", "get", key], capture_output=True, + timeout=10, env=env, cwd=work) + if result.returncode != 0: + print("diagnostic: config get %s failed:\n%s" % (key, output_text(result)[:700])) + return None + values.append(result.stdout.decode("utf-8", "replace").strip()) + return tuple(values) + + def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): name = "enabled" if enabled else "disabled" name += "-" + ("-".join(flag.lstrip("-") for flag in flags) or "bare") @@ -339,7 +357,6 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): probe.close() os.makedirs(cache, exist_ok=True) env = fixture_environment(work, cache, marker, 10000) - config_path = os.path.join(cache, "config.json") daemon_pid = 0 try: for key, value in (("ui_enabled", str(enabled).lower()), ("ui_port", str(port))): @@ -348,8 +365,11 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): if configured.returncode != 0: print("SETUP FAIL: could not persist UI setting:\n%s" % output_text(configured)) return False - with open(config_path, "rb") as handle: - original_config = handle.read() + original_setting = saved_ui_setting(binary, env, work) + if original_setting != (str(enabled).lower(), str(port)): + print("SETUP FAIL: config get does not report the saved UI setting (%s): %r" % + (name, original_setting)) + return False arguments = ["--port=%d" % port if flag == "--port" else flag for flag in flags] first = subprocess.run([binary, "daemon", "start", *arguments], capture_output=True, timeout=30, env=env, cwd=work) @@ -359,13 +379,12 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): print("RED: daemon start failed with saved UI setting (%s):\n%s" % (name, first_text[:700])) return False - with open(config_path, "rb") as handle: - current_config = handle.read() + current_setting = saved_ui_setting(binary, env, work) expected_enabled = enabled or bool(flags) - if (json.loads(current_config).get("ui_enabled") != expected_enabled or - (not expected_enabled and current_config != original_config)): + if (current_setting is None or current_setting[0] != str(expected_enabled).lower() or + (not expected_enabled and current_setting != original_setting)): print("RED: daemon start rewrote the saved UI setting (%s): %r" % - (name, current_config)) + (name, current_setting)) return False if expected_enabled: opened = subprocess.run([binary, "daemon", "start", "--open"], capture_output=True, @@ -385,10 +404,11 @@ def assert_persisted_ui_setting(binary, work, enabled=False, flags=()): print("RED: disabled UI was not preserved by %r:\n%s" % (arguments, text[:700])) return False - with open(config_path, "rb") as handle: - if handle.read() != original_config: - print("RED: repeated daemon start rewrote disabled UI config") - return False + repeated_setting = saved_ui_setting(binary, env, work) + if repeated_setting != original_setting: + print("RED: repeated daemon start rewrote disabled UI config: %r" % + (repeated_setting,)) + return False with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as connection: connection.settimeout(0.5) if connection.connect_ex(("127.0.0.1", port)) == 0: From 5216b70635beb936eebb1a0c6b26b4aa11ac3b5d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 02:34:09 +0100 Subject: [PATCH 15/24] 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 16/24] 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 17/24] 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 18/24] 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 19/24] 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 20/24] 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 a031122a4dece52db1b8a41b3944ccab43acad19 Mon Sep 17 00:00:00 2001 From: Divakant Pandey Date: Tue, 3 Nov 2026 16:36:43 +0530 Subject: [PATCH 21/24] fix(extract): emit HTTP_CALLS for Retrofit interface methods (#2723) Retrofit's @GET/@POST/... share their names with the JAX-RS verbs, so every annotated client method became a served Route with a HANDLES edge, and any path without a leading slash collapsed onto "/". JAX-RS verbs are marker annotations; a verb that carries an argument is a Retrofit request. Record its path as client_route_path and emit HTTP_CALLS to the Route instead. Relative paths are rooted, full URLs kept; constants, templates and concatenations have no static value and produce no edge. Signed-off-by: Divakant Pandey --- internal/cbm/cbm.h | 3 + internal/cbm/extract_defs.c | 85 ++++++++++++++++++++++ internal/cbm/result_compact.c | 1 + src/pipeline/pass_definitions.c | 2 + src/pipeline/pass_parallel.c | 2 + src/pipeline/pass_route_nodes.c | 28 +++++++ src/pipeline/pipeline.c | 7 +- tests/test_edge_types_probe.c | 125 ++++++++++++++++++++++++++++++-- tests/test_pipeline.c | 72 ++++++++++++++++++ 9 files changed, 315 insertions(+), 10 deletions(-) diff --git a/internal/cbm/cbm.h b/internal/cbm/cbm.h index 5d1abc215..7b6312314 100644 --- a/internal/cbm/cbm.h +++ b/internal/cbm/cbm.h @@ -241,6 +241,9 @@ typedef struct { const char **return_types; // NULL-terminated array (NULL if none) const char *route_path; // HTTP route path from decorator (e.g., "/api/users") or NULL const char *route_method; // HTTP method from decorator (e.g., "POST") or NULL + // Path a client interface method requests (Retrofit @GET("v1/x")) or NULL; + // route_method is then its verb. + const char *client_route_path; int complexity; // cyclomatic complexity int cognitive; // cognitive complexity (nesting-weighted) int loop_count; // number of loop constructs in the body diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index 76571c1c7..ab990b7ed 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3593,6 +3593,83 @@ static void extract_route_from_decorators(CBMArena *a, TSNode func_node, const c extract_route_from_annotations(a, func_node, source, spec, out_path, out_method); } +/* The string literal an annotation argument consists of, positional or as + * `value = "..."`. A concatenation, a constant or a template has no statically + * known value, so it yields a null node rather than one of its fragments. */ +static TSNode annotation_arg_literal(TSNode arg) { + TSNode null_node = {0}; + const char *kind = ts_node_type(arg); + if (strcmp(kind, "element_value_pair") == 0 || strcmp(kind, "value_argument") == 0) { + uint32_t nc = ts_node_named_child_count(arg); + if (nc == 0) { + return null_node; + } + arg = ts_node_named_child(arg, nc - SKIP_ONE); + } + if (!is_route_string_kind(ts_node_type(arg))) { + return null_node; + } + for (uint32_t i = 0; i < ts_node_named_child_count(arg); i++) { + const char *ck = ts_node_type(ts_node_named_child(arg, i)); + if (strcmp(ck, "interpolated_identifier") == 0 || + strcmp(ck, "interpolated_expression") == 0 || strcmp(ck, "string_interpolation") == 0) { + return null_node; + } + } + return arg; +} + +/* Retrofit declares a client's requests as @GET("v1/orders") / @POST(...) on an + * interface method. JAX-RS uses the same bare verb names, but its verbs are + * marker annotations (the path is a sibling @Path), so a verb that carries an + * argument is a Retrofit request, not a served endpoint. Returns true for such + * a method; *out_path is its path, or NULL when the argument is not a literal. + * Retrofit resolves a relative path against the base URL, so it is rooted the + * way JAX-RS paths are; a full URL is kept as written. */ +static bool retrofit_client_route(CBMArena *a, TSNode func_node, const char *source, + const CBMLangSpec *spec, const char **out_path, + const char **out_method) { + *out_path = NULL; + *out_method = NULL; + TSNode wrappers[MAX_ATTR_WRAPPERS]; + int wn = find_jvm_modifiers(func_node, spec->language, wrappers, MAX_ATTR_WRAPPERS_MINUS_1); + wrappers[wn++] = func_node; + for (int w = 0; w < wn; w++) { + uint32_t cc = ts_node_child_count(wrappers[w]); + for (uint32_t ci = 0; ci < cc; ci++) { + TSNode child = ts_node_child(wrappers[w], ci); + if (!cbm_kind_in_set(child, spec->decorator_node_types)) { + continue; + } + TSNode name_node = annotation_name_node(child); + if (ts_node_is_null(name_node)) { + continue; + } + char *name = cbm_node_text(a, name_node, source); + const char *method = annotation_route_method(name); + if (!method || strcmp(method, name) != 0) { + continue; + } + TSNode args = annotation_args_node(child); + if (ts_node_is_null(args) || ts_node_named_child_count(args) == 0) { + continue; + } + *out_method = method; + TSNode lit = annotation_arg_literal(ts_node_named_child(args, 0)); + char *text = ts_node_is_null(lit) ? NULL : cbm_node_text(a, lit, source); + size_t len = text ? strlen(text) : 0; + if (len > PAIR_CHARS && (text[0] == '"' || text[0] == '\'')) { + char *path = cbm_arena_strndup(a, text + SKIP_CHAR, len - PAIR_CHARS); + *out_path = (!path || path[0] == '/' || strstr(path, "://")) + ? path + : cbm_arena_sprintf(a, "/%s", path); + } + return true; + } + } + return false; +} + static const char *join_route_paths(CBMArena *a, const char *prefix, const char *path) { if (!path || !path[0]) { return prefix; @@ -9306,6 +9383,14 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node, def.decorators = extract_decorators(a, child, ctx->source, ctx->language, spec); extract_route_from_decorators(a, child, ctx->source, spec, &def.route_path, &def.route_method); + const char *client_path = NULL; + const char *client_method = NULL; + if ((ctx->language == CBM_LANG_JAVA || ctx->language == CBM_LANG_KOTLIN) && + retrofit_client_route(a, child, ctx->source, spec, &client_path, &client_method)) { + def.route_path = NULL; + def.client_route_path = client_path; + def.route_method = client_path ? client_method : NULL; + } if (def.route_path && (ctx->language == CBM_LANG_JAVA || ctx->language == CBM_LANG_KOTLIN || ctx->language == CBM_LANG_SCALA)) { const char *prefix = spring_class_route_prefix(a, class_node, ctx->source, spec); diff --git a/internal/cbm/result_compact.c b/internal/cbm/result_compact.c index 46bf6eae1..b0994690e 100644 --- a/internal/cbm/result_compact.c +++ b/internal/cbm/result_compact.c @@ -268,6 +268,7 @@ static void cr_walk_def(cr_ctx_t *c, CBMDefinition *d) { cr_list(c, &d->return_types); cr_str(c, &d->route_path); cr_str(c, &d->route_method); + cr_str(c, &d->client_route_path); cr_blob(c, (const void **)&d->fingerprint, d->fingerprint_k > 0 ? (size_t)d->fingerprint_k * sizeof(uint32_t) : 0); cr_str(c, &d->structural_profile); diff --git a/src/pipeline/pass_definitions.c b/src/pipeline/pass_definitions.c index 7c7aa438f..ca885604c 100644 --- a/src/pipeline/pass_definitions.c +++ b/src/pipeline/pass_definitions.c @@ -322,6 +322,7 @@ static void build_def_props(char *buf, size_t bufsize, const CBMDefinition *def) append_json_str_array(buf, bufsize, &pos, "param_types", def->param_types); append_json_string(buf, bufsize, &pos, "route_path", def->route_path); append_json_string(buf, bufsize, &pos, "route_method", def->route_method); + append_json_string(buf, bufsize, &pos, "client_route_path", def->client_route_path); append_json_string(buf, bufsize, &pos, "http_client", def->http_client); append_json_string(buf, bufsize, &pos, "http_base_url", def->http_base_url); /* A document definition's facts (an ADR's id, status, date, ...): key, value pairs. */ @@ -402,6 +403,7 @@ static char *pd_props_buf(const CBMDefinition *def, char *stack, size_t *size) { pd_arr_field_len("param_types", def->param_types) + pd_str_field_len("route_path", def->route_path) + pd_str_field_len("route_method", def->route_method) + + pd_str_field_len("client_route_path", def->client_route_path) + pd_str_field_len("http_client", def->http_client) + pd_str_field_len("http_base_url", def->http_base_url); for (const char **kv = def->extra_props; kv && kv[0] && kv[SKIP_ONE]; kv += PAIR_LEN) { diff --git a/src/pipeline/pass_parallel.c b/src/pipeline/pass_parallel.c index e02d68c67..df98068fd 100644 --- a/src/pipeline/pass_parallel.c +++ b/src/pipeline/pass_parallel.c @@ -552,6 +552,7 @@ static void build_def_props(char *buf, size_t bufsize, const CBMDefinition *def) append_json_str_array(buf, bufsize, &pos, "param_types", def->param_types); append_json_string(buf, bufsize, &pos, "route_path", def->route_path); append_json_string(buf, bufsize, &pos, "route_method", def->route_method); + append_json_string(buf, bufsize, &pos, "client_route_path", def->client_route_path); append_json_string(buf, bufsize, &pos, "http_client", def->http_client); append_json_string(buf, bufsize, &pos, "http_base_url", def->http_base_url); /* A document definition's facts (an ADR's id, status, date, ...): key, value pairs. */ @@ -780,6 +781,7 @@ static char *pp_props_buf(const CBMDefinition *def, char *stack, size_t *size) { pp_arr_field_len("param_types", def->param_types) + pp_str_field_len("route_path", def->route_path) + pp_str_field_len("route_method", def->route_method) + + pp_str_field_len("client_route_path", def->client_route_path) + pp_str_field_len("http_client", def->http_client) + pp_str_field_len("http_base_url", def->http_base_url); for (const char **kv = def->extra_props; kv && kv[0] && kv[SKIP_ONE]; kv += PAIR_LEN) { diff --git a/src/pipeline/pass_route_nodes.c b/src/pipeline/pass_route_nodes.c index 23dff8150..02d0478fd 100644 --- a/src/pipeline/pass_route_nodes.c +++ b/src/pipeline/pass_route_nodes.c @@ -397,8 +397,36 @@ static void match_infra_routes(cbm_gbuf_t *gb) { } } +/* A client interface method's declared request (Retrofit): an HTTP_CALLS edge + * to the Route, with the url_path/method properties a call site's edge carries, + * so the endpoint is matched to its server like any other outbound call. */ +static void emit_client_route_call(cbm_gbuf_t *gb, int64_t func_id, const char *path, + const char *method) { + const char *rm = method ? method : "ANY"; + char route_qn[CBM_ROUTE_QN_SIZE]; + char cpath[CBM_SZ_256]; + snprintf(route_qn, sizeof(route_qn), "__route__%s__%s", rm, + cbm_route_canon_path(path, cpath, sizeof(cpath))); + char esc_m[CBM_SZ_128]; + cbm_json_escape(esc_m, sizeof(esc_m), rm); + char rprops[CBM_SZ_256]; + (void)snprintf(rprops, sizeof(rprops), "{\"method\":\"%s\"}", esc_m); + int64_t route_id = cbm_gbuf_upsert_node(gb, "Route", path, route_qn, "", 0, 0, rprops); + char esc_p[CBM_SZ_512]; + cbm_json_escape(esc_p, sizeof(esc_p), path); + char eprops[CBM_SZ_1K]; + (void)snprintf(eprops, sizeof(eprops), + "{\"callee\":\"%s\",\"url_path\":\"%s\",\"method\":\"%s\"}", esc_m, esc_p, + esc_m); + cbm_gbuf_insert_edge(gb, func_id, route_id, "HTTP_CALLS", eprops); +} + void cbm_pipeline_emit_def_route(cbm_gbuf_t *gb, int64_t func_id, const CBMDefinition *def, const char *rel) { + if (def->client_route_path && def->client_route_path[0]) { + emit_client_route_call(gb, func_id, def->client_route_path, def->route_method); + return; + } if (!def->route_path || def->route_path[0] == '\0') { return; } diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index e492c2b8d..7ea1ff81a 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -2592,9 +2592,10 @@ static void log_result_census(const char *tag, CBMFileResult **cache, int file_c } str_def_fp += def->fingerprint ? (size_t)def->fingerprint_k * sizeof(uint32_t) : 0; str_def_misc += census_len(def->route_path) + census_len(def->route_method) + - census_len(def->impl_trait) + census_len(def->http_base_url) + - census_list(def->extra_props) + census_len(def->decl_namespace) + - census_len(def->variant_key) + census_len(def->alias_target); + census_len(def->client_route_path) + census_len(def->impl_trait) + + census_len(def->http_base_url) + census_list(def->extra_props) + + census_len(def->decl_namespace) + census_len(def->variant_key) + + census_len(def->alias_target); } for (int c = 0; c < r->calls.count; c++) { const CBMCall *call = &r->calls.items[c]; diff --git a/tests/test_edge_types_probe.c b/tests/test_edge_types_probe.c index 4bba54612..8d900af72 100644 --- a/tests/test_edge_types_probe.c +++ b/tests/test_edge_types_probe.c @@ -1140,8 +1140,9 @@ static int et_qn_has_tail(const char *qn, const char *tail) { /* route_qns (optional, NULL-terminated): Route qualified_names that must * exist, so a test asserting that a route gets NO handler cannot pass merely * because the route itself was never extracted. */ -static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHandles *want, - int parallel, const char *const *route_qns) { +static int et_edges_exact_routes(const EtFile *files, int nfiles, const char *edge, + const EtHandles *want, int parallel, + const char *const *route_qns) { EtProj lp; cbm_store_t *store = parallel ? et_index_parallel(&lp, files, nfiles) : et_index_files(&lp, files, nfiles); @@ -1153,7 +1154,7 @@ static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHand cbm_edge_t *edges = NULL; int n = 0; int ok = store != NULL && - cbm_store_find_edges_by_type(store, lp.project, "HANDLES", &edges, &n) == + cbm_store_find_edges_by_type(store, lp.project, edge, &edges, &n) == CBM_STORE_OK && n == wanted; for (int i = 0; store && i < n; i++) { @@ -1171,7 +1172,7 @@ static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHand } if (!matched) { ok = 0; - fprintf(stderr, " [ET-HANDLES] unexpected %s -> %s\n", + fprintf(stderr, " [ET-%s] unexpected %s -> %s\n", edge, have_src && src.qualified_name ? src.qualified_name : "", have_tgt && tgt.name ? tgt.name : ""); } @@ -1181,7 +1182,7 @@ static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHand for (int wi = 0; wi < wanted; wi++) { if (!found[wi]) { ok = 0; - fprintf(stderr, " [ET-HANDLES] missing %s -> %s\n", want[wi].handler_tail, + fprintf(stderr, " [ET-%s] missing %s -> %s\n", edge, want[wi].handler_tail, want[wi].route); } } @@ -1190,13 +1191,13 @@ static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHand if (!store || cbm_store_find_node_by_qn(store, lp.project, route_qns[ri], &route) != CBM_STORE_OK) { ok = 0; - fprintf(stderr, " [ET-HANDLES] missing Route %s\n", route_qns[ri]); + fprintf(stderr, " [ET-%s] missing Route %s\n", edge, route_qns[ri]); } else { cbm_node_free_fields(&route); } } if (!ok) { - fprintf(stderr, " [ET-HANDLES] FAIL (%s path) expected=%d actual=%d\n", + fprintf(stderr, " [ET-%s] FAIL (%s path) expected=%d actual=%d\n", edge, parallel ? "parallel" : "sequential", wanted, n); } if (edges) cbm_store_free_edges(edges, n); @@ -1204,10 +1205,117 @@ static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHand return ok; } +static int et_handles_exact_routes(const EtFile *files, int nfiles, const EtHandles *want, + int parallel, const char *const *route_qns) { + return et_edges_exact_routes(files, nfiles, "HANDLES", want, parallel, route_qns); +} + static int et_handles_exact(const EtFile *files, int nfiles, const EtHandles *want, int parallel) { return et_handles_exact_routes(files, nfiles, want, parallel, NULL); } +/* Retrofit (#2723): an interface method's @GET("...") declares a request the + * client makes. It was minted as a served Route with a HANDLES edge, and every + * path without a leading slash collapsed onto "/". The relative path is rooted + * (Retrofit resolves it against the base URL); a path from a constant or a + * template has no static value and yields no edge rather than a "/" Route. */ +static const EtFile et_retrofit_kotlin[] = { + {"src/main/kotlin/com/acme/client/OrderApi.kt", + "package com.acme.client\n\n" + "import retrofit2.http.Body\n" + "import retrofit2.http.GET\n" + "import retrofit2.http.POST\n" + "import retrofit2.http.Path\n\n" + "private const val ORDERS = \"/v1/orders\"\n\n" + "interface OrderApi {\n" + " @GET(\"/v1/orders/{id}\")\n" + " suspend fun getOrder(@Path(\"id\") id: String): String\n\n" + " @POST(\"v1/orders\")\n" + " suspend fun createOrder(@Body body: String): String\n\n" + " @GET(ORDERS)\n" + " suspend fun listOrders(): String\n\n" + " @GET(\"$ORDERS/{id}/status\")\n" + " suspend fun getOrderStatus(@Path(\"id\") id: String): String\n" + "}\n"}}; + +static const EtHandles et_retrofit_kotlin_calls[] = { + {"OrderApi.getOrder", "/v1/orders/{id}"}, {"OrderApi.createOrder", "/v1/orders"}, {NULL, NULL}}; + +static const EtHandles et_no_edges[] = {{NULL, NULL}}; + +TEST(http_calls_retrofit_kotlin_interface_issue2723) { + static const char *const qns[] = {"__route__GET__/v1/orders/{}", "__route__POST__/v1/orders", + NULL}; + for (int parallel = 0; parallel <= 1; parallel++) { + ASSERT_TRUE(et_edges_exact_routes(et_retrofit_kotlin, 1, "HTTP_CALLS", + et_retrofit_kotlin_calls, parallel, qns)); + ASSERT_TRUE(et_handles_exact(et_retrofit_kotlin, 1, et_no_edges, parallel)); + } + PASS(); +} + +/* Java Retrofit: the named `value =` form and a full URL are read; a + * concatenation is not, so none of its string fragments becomes a path. */ +TEST(http_calls_retrofit_java_interface_issue2723) { + static const EtFile f[] = { + {"src/main/java/com/acme/jclient/OrderApi.java", + "package com.acme.jclient;\n\n" + "import retrofit2.Call;\n" + "import retrofit2.http.Body;\n" + "import retrofit2.http.DELETE;\n" + "import retrofit2.http.GET;\n" + "import retrofit2.http.POST;\n" + "import retrofit2.http.Path;\n\n" + "public interface OrderApi {\n" + " String BASE = \"v1\";\n\n" + " @GET(\"v1/orders/{id}\")\n" + " Call getOrder(@Path(\"id\") String id);\n\n" + " @POST(value = \"v1/orders\")\n" + " Call createOrder(@Body String body);\n\n" + " @GET(\"https://status.acme.com/v1/health\")\n" + " Call health();\n\n" + " @DELETE(BASE + \"/orders/{id}\")\n" + " Call deleteOrder(@Path(\"id\") String id);\n" + "}\n"}}; + static const EtHandles calls[] = {{"OrderApi.getOrder", "/v1/orders/{id}"}, + {"OrderApi.createOrder", "/v1/orders"}, + {"OrderApi.health", "https://status.acme.com/v1/health"}, + {NULL, NULL}}; + for (int parallel = 0; parallel <= 1; parallel++) { + ASSERT_TRUE(et_edges_exact_routes(f, 1, "HTTP_CALLS", calls, parallel, NULL)); + ASSERT_TRUE(et_handles_exact(f, 1, et_no_edges, parallel)); + } + PASS(); +} + +/* Control: a JAX-RS interface keeps HANDLES. Its @GET is a marker and the path + * is a sibling @Path, so the Retrofit reading (a verb with an argument) does + * not depend on the declaring type being an interface. */ +TEST(handles_jaxrs_interface_not_retrofit_issue2723) { + static const EtFile f[] = { + {"src/main/java/com/acme/api/OrderResource.java", + "package com.acme.api;\n\n" + "import jakarta.ws.rs.GET;\n" + "import jakarta.ws.rs.POST;\n" + "import jakarta.ws.rs.Path;\n\n" + "@Path(\"/v1/orders\")\n" + "public interface OrderResource {\n" + " @GET\n" + " @Path(\"/{id}\")\n" + " String get(String id);\n\n" + " @POST\n" + " String create(String body);\n" + "}\n"}}; + static const EtHandles handles[] = {{"OrderResource.get", "/v1/orders/{id}"}, + {"OrderResource.create", "/v1/orders"}, + {NULL, NULL}}; + for (int parallel = 0; parallel <= 1; parallel++) { + ASSERT_TRUE(et_handles_exact(f, 1, handles, parallel)); + ASSERT_TRUE(et_edges_exact_routes(f, 1, "HTTP_CALLS", et_no_edges, parallel, NULL)); + } + PASS(); +} + /* The exact DATA_FLOWS set, caller -> handler by QN tail. route_qns (NULL-terminated) must * exist, so a test that expects NO flow on a path cannot pass because the caller's or the * handler's Route was never created; handles is the exact HANDLES count. */ @@ -2871,6 +2979,9 @@ SUITE(edge_types_probe) { RUN_TEST(handles_jaxrs_java_relative_path); RUN_TEST(handles_spring_java_relative_string_not_route); RUN_TEST(handles_jaxrs_scala); + RUN_TEST(http_calls_retrofit_kotlin_interface_issue2723); + RUN_TEST(http_calls_retrofit_java_interface_issue2723); + RUN_TEST(handles_jaxrs_interface_not_retrofit_issue2723); RUN_TEST(handles_aspnet_csharp); RUN_TEST(handles_aspnet_attribute_routes_issue1692); RUN_TEST(handles_aspnet_attribute_routes_issue1041); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index b8bff190a..4b890aa19 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -4919,6 +4919,77 @@ TEST(incremental_dockerfile_edit_keeps_build_edges) { PASS(); } +/* Editing a Retrofit interface keeps its requests as HTTP_CALLS (#2723): the + * incremental routes re-extract the file and must not mint HANDLES for it. + * `legacy`: through the full-load route, else the closure-repair route. Returns "full=calls/handles + * inc=calls/handles", or NULL; *route: the route taken. */ +static char *incremental_retrofit_keep(bool legacy, cbm_incremental_route_t *route) { + char tmp[256]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_retrofitkeep_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + return NULL; + } + write_temp_file(tmp, "src/OrderApi.kt", + "package com.acme\n\nimport retrofit2.http.GET\nimport retrofit2.http.POST\n\n" + "interface OrderApi {\n" + " @GET(\"/v1/orders/{id}\")\n suspend fun getOrder(id: String): String\n" + " @POST(\"v1/orders\")\n suspend fun createOrder(body: String): String\n" + "}\n"); + char db_path[512]; + snprintf(db_path, sizeof(db_path), "%s/retrofitkeep.db", tmp); + static const char calls[] = "SELECT COUNT(*) FROM edges e JOIN nodes r ON r.id=e.target_id " + "WHERE e.type='HTTP_CALLS' AND r.label='Route' " + "AND r.qualified_name IN ('__route__GET__/v1/orders/{}'," + "'__route__POST__/v1/orders')"; + static const char handles[] = "SELECT COUNT(*) FROM edges WHERE type='HANDLES'"; + int full_calls = -1; + int full_handles = -1; + if (k8s_inc_index(tmp, db_path) == 0) { + full_calls = k8s_inc_count(db_path, calls); + full_handles = k8s_inc_count(db_path, handles); + } + write_temp_file(tmp, "src/OrderApi.kt", + "package com.acme\n\nimport retrofit2.http.GET\nimport retrofit2.http.POST\n\n" + "// edited\ninterface OrderApi {\n" + " @GET(\"/v1/orders/{id}\")\n suspend fun getOrder(id: String): String\n" + " @POST(\"v1/orders\")\n suspend fun createOrder(body: String): String\n" + "}\n"); +#if defined(CBM_INCREMENTAL_TEST_API) && CBM_INCREMENTAL_TEST_API + if (legacy) { + cbm_pipeline_incremental_test_force_legacy_partial_once(); + } +#else + (void)legacy; +#endif + char *out = NULL; + if (k8s_inc_index(tmp, db_path) == 0) { + char buf[64]; + snprintf(buf, sizeof(buf), "full=%d/%d inc=%d/%d", full_calls, full_handles, + k8s_inc_count(db_path, calls), k8s_inc_count(db_path, handles)); + out = strdup(buf); + } + *route = cbm_pipeline_incremental_test_last_route(); + th_rmtree(tmp); + return out; +} + +TEST(incremental_edit_keeps_retrofit_http_calls_issue2723) { + cbm_incremental_route_t route = CBM_INCREMENTAL_ROUTE_NONE; + char *got = incremental_retrofit_keep(false, &route); + ASSERT_EQ(route, CBM_INCREMENTAL_ROUTE_CLOSURE_REPAIR); + ASSERT_NOT_NULL(got); + ASSERT_STR_EQ(got, "full=2/0 inc=2/0"); + free(got); +#if defined(CBM_INCREMENTAL_TEST_API) && CBM_INCREMENTAL_TEST_API + got = incremental_retrofit_keep(true, &route); + ASSERT_EQ(route, CBM_INCREMENTAL_ROUTE_LEGACY_PARTIAL); + ASSERT_NOT_NULL(got); + ASSERT_STR_EQ(got, "full=2/0 inc=2/0"); + free(got); +#endif + PASS(); +} + /* A compose file's edit replaces its build edges: the renamed image keeps * no edge (nor node), the new one has its edge. `legacy`: through the * full-load route, else the closure-repair route. Returns "old=N new=M" @@ -31946,6 +32017,7 @@ SUITE(pipeline_semantic_manifest_repro) { RUN_TEST(incremental_script_request_to_compose_service_keeps_host); RUN_TEST(incremental_deleted_dockerfile_drops_its_image); RUN_TEST(incremental_dockerfile_edit_keeps_build_edges); + RUN_TEST(incremental_edit_keeps_retrofit_http_calls_issue2723); RUN_TEST(incremental_compose_edit_replaces_build_edges); RUN_TEST(incremental_env_edit_replaces_scoped_envvars); RUN_TEST(incremental_image_split_change_rebuilds); From c8c4b6c7dbb0ff87d3183b443e8190a3d79f642d Mon Sep 17 00:00:00 2001 From: Divakant Pandey Date: Wed, 4 Nov 2026 13:41:55 +0530 Subject: [PATCH 22/24] fix(extract): read Retrofit verbs only on interface methods (#2723) Pippo and Rapidoid put the path in @GET("/x") on a controller class, and those methods serve the route. Retrofit services must be interfaces, so limit the client reading to Java and Kotlin interfaces. Signed-off-by: Divakant Pandey --- internal/cbm/extract_defs.c | 25 +++++++++++++++++++++++ tests/test_edge_types_probe.c | 38 +++++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 2 deletions(-) diff --git a/internal/cbm/extract_defs.c b/internal/cbm/extract_defs.c index ab990b7ed..069c8093a 100644 --- a/internal/cbm/extract_defs.c +++ b/internal/cbm/extract_defs.c @@ -3619,6 +3619,28 @@ static TSNode annotation_arg_literal(TSNode arg) { return arg; } +/* Java interface_declaration, or a Kotlin class_declaration spelled with the + * `interface` keyword (the grammar has no separate node kind for it). */ +static bool is_jvm_interface(TSNode class_node) { + if (ts_node_is_null(class_node)) { + return false; + } + const char *kind = ts_node_type(class_node); + if (strcmp(kind, "interface_declaration") == 0) { + return true; + } + if (strcmp(kind, "class_declaration") != 0) { + return false; + } + for (uint32_t i = 0; i < ts_node_child_count(class_node); i++) { + TSNode ch = ts_node_child(class_node, i); + if (!ts_node_is_named(ch) && strcmp(ts_node_type(ch), "interface") == 0) { + return true; + } + } + return false; +} + /* Retrofit declares a client's requests as @GET("v1/orders") / @POST(...) on an * interface method. JAX-RS uses the same bare verb names, but its verbs are * marker annotations (the path is a sibling @Path), so a verb that carries an @@ -9385,7 +9407,10 @@ static void push_method_def(CBMExtractCtx *ctx, TSNode child, TSNode class_node, extract_route_from_decorators(a, child, ctx->source, spec, &def.route_path, &def.route_method); const char *client_path = NULL; const char *client_method = NULL; + /* Retrofit services must be interfaces; Pippo and Rapidoid put the path in + * @GET("/x") on a controller class, and those methods serve the route. */ if ((ctx->language == CBM_LANG_JAVA || ctx->language == CBM_LANG_KOTLIN) && + is_jvm_interface(class_node) && retrofit_client_route(a, child, ctx->source, spec, &client_path, &client_method)) { def.route_path = NULL; def.client_route_path = client_path; diff --git a/tests/test_edge_types_probe.c b/tests/test_edge_types_probe.c index 8d900af72..060dcba80 100644 --- a/tests/test_edge_types_probe.c +++ b/tests/test_edge_types_probe.c @@ -1289,8 +1289,8 @@ TEST(http_calls_retrofit_java_interface_issue2723) { } /* Control: a JAX-RS interface keeps HANDLES. Its @GET is a marker and the path - * is a sibling @Path, so the Retrofit reading (a verb with an argument) does - * not depend on the declaring type being an interface. */ + * is a sibling @Path, so it is not read as a Retrofit request (a verb with an + * argument) even though it is declared on an interface. */ TEST(handles_jaxrs_interface_not_retrofit_issue2723) { static const EtFile f[] = { {"src/main/java/com/acme/api/OrderResource.java", @@ -1316,6 +1316,39 @@ TEST(handles_jaxrs_interface_not_retrofit_issue2723) { PASS(); } +/* Control: Pippo and Rapidoid put the path inside @GET("/x") on a controller + * class. Retrofit services must be interfaces, so these keep HANDLES. */ +TEST(handles_verb_path_controller_class_not_retrofit_issue2723) { + static const EtFile f[] = { + {"src/main/java/com/acme/web/ContactsController.java", + "package com.acme.web;\n\n" + "import ro.pippo.controller.Controller;\n" + "import ro.pippo.controller.GET;\n" + "import ro.pippo.controller.POST;\n\n" + "public class ContactsController extends Controller {\n" + " @GET(\"/contacts\")\n" + " public void index() {}\n\n" + " @POST(\"/contacts\")\n" + " public void create() {}\n" + "}\n"}, + {"src/main/kotlin/com/acme/web/StatusController.kt", + "package com.acme.web\n\n" + "import org.rapidoid.annotation.GET\n\n" + "class StatusController {\n" + " @GET(\"/status\")\n" + " fun status(): String = \"ok\"\n" + "}\n"}}; + static const EtHandles handles[] = {{"ContactsController.index", "/contacts"}, + {"ContactsController.create", "/contacts"}, + {"StatusController.status", "/status"}, + {NULL, NULL}}; + for (int parallel = 0; parallel <= 1; parallel++) { + ASSERT_TRUE(et_handles_exact(f, 2, handles, parallel)); + ASSERT_TRUE(et_edges_exact_routes(f, 2, "HTTP_CALLS", et_no_edges, parallel, NULL)); + } + PASS(); +} + /* The exact DATA_FLOWS set, caller -> handler by QN tail. route_qns (NULL-terminated) must * exist, so a test that expects NO flow on a path cannot pass because the caller's or the * handler's Route was never created; handles is the exact HANDLES count. */ @@ -2982,6 +3015,7 @@ SUITE(edge_types_probe) { RUN_TEST(http_calls_retrofit_kotlin_interface_issue2723); RUN_TEST(http_calls_retrofit_java_interface_issue2723); RUN_TEST(handles_jaxrs_interface_not_retrofit_issue2723); + RUN_TEST(handles_verb_path_controller_class_not_retrofit_issue2723); RUN_TEST(handles_aspnet_csharp); RUN_TEST(handles_aspnet_attribute_routes_issue1692); RUN_TEST(handles_aspnet_attribute_routes_issue1041); From e750c0531ae5c6f239703d990a73a32ac2c2f27f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 02:50:49 +0100 Subject: [PATCH 23/24] 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 24/24] 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