Skip to content

fix(mcp): report freshness with the indexer's own mtime source (#1714) #1787

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
umi008 wants to merge 1 commit into
base: main
Choose a base branch
from
+77 −43
Open

fix(mcp): report freshness with the indexer's own mtime source (#1714) #1787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 8 additions & 14 deletions src/mcp/mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4204,17 +4204,6 @@ static coverage_path_result_t coverage_normalize_rel(const char *input, bool all
return written > 0U || allow_root ? COVERAGE_PATH_OK : COVERAGE_PATH_INVALID;
}

static int64_t coverage_stat_mtime_ns(const struct stat *st) {
#ifdef __APPLE__
return ((int64_t)st->st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) +
(int64_t)st->st_mtimespec.tv_nsec;
#elif defined(_WIN32)
return (int64_t)st->st_mtime * (int64_t)CBM_NSEC_PER_SEC;
#else
return ((int64_t)st->st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) + (int64_t)st->st_mtim.tv_nsec;
#endif
}

static const char *coverage_path_freshness(cbm_store_t *store, const char *project,
const char *root_path, const char *rel_path,
bool *outside) {
Expand All @@ -4228,8 +4217,13 @@ static const char *coverage_path_freshness(cbm_store_t *store, const char *proje
if (n < 0 || (size_t)n >= sizeof(abs_path)) {
return "unavailable";
}
struct stat st;
if (stat(abs_path, &st) != 0) {
/* #1714: mtime_ns must come from the SAME source the indexer recorded it
* with — cbm_path_info_utf8. Recomputing from struct stat truncates to
* seconds on Windows (st_mtime), while the file_hashes record carries
* FILETIME-resolution nanoseconds, so a byte-identical file never matched
* and every path was reported metadata_changed. */
cbm_path_info_t info;
if (cbm_path_info_utf8(abs_path, &info) != 0) {
return "missing";
}
if (!cbm_path_within_root(root_path, abs_path)) {
Expand All @@ -4245,7 +4239,7 @@ static const char *coverage_path_freshness(cbm_store_t *store, const char *proje
if (rc != CBM_STORE_OK) {
return "unavailable";
}
bool matches = hash.mtime_ns == coverage_stat_mtime_ns(&st) && hash.size == st.st_size;
bool matches = hash.mtime_ns == info.mtime_ns && hash.size == info.size;
cbm_store_clear_file_hash(&hash);
return matches ? "metadata_match" : "metadata_changed";
}
Expand Down
24 changes: 9 additions & 15 deletions src/pipeline/pipeline_incremental.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,18 +102,6 @@ static const char *itoa_buf(int v) {
return buf[idx];
}

/* ── Platform-portable mtime_ns ──────────────────────────────────── */

static int64_t stat_mtime_ns(const struct stat *st) {
#ifdef __APPLE__
return ((int64_t)st->st_mtimespec.tv_sec * CBM_NS_PER_SEC) + (int64_t)st->st_mtimespec.tv_nsec;
#elif defined(_WIN32)
return (int64_t)st->st_mtime * CBM_NS_PER_SEC;
#else
return ((int64_t)st->st_mtim.tv_sec * CBM_NS_PER_SEC) + (int64_t)st->st_mtim.tv_nsec;
#endif
}

static const char *incr_mode_name(int mode) {
switch (mode) {
case CBM_MODE_FULL:
Expand Down Expand Up @@ -683,14 +671,20 @@ static bool *classify_files(cbm_file_info_t *files, int file_count, cbm_file_has
continue;
}

struct stat st;
if (stat(files[i].path, &st) != 0) {
/* #1714: compare against the SAME source the hash writer used. The
* manifest hash is written from cbm_path_info_utf8 (see
* semantic_manifest_hash_file), so a stat()-based comparison is
* internally inconsistent: on Windows stat() truncates mtime to
* seconds while the recorded value carries FILETIME nanoseconds, which
* made every file look changed on each incremental pass. */
cbm_path_info_t info;
if (cbm_path_info_utf8(files[i].path, &info) != 0) {
changed[i] = true;
n_changed++;
continue;
}

if (stat_mtime_ns(&st) != h->mtime_ns || st.st_size != h->size) {
if (info.mtime_ns != h->mtime_ns || info.size != h->size) {
changed[i] = true;
n_changed++;
} else {
Expand Down
74 changes: 60 additions & 14 deletions tests/test_mcp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2727,20 +2727,15 @@ TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_
ASSERT_NOT_NULL(store);
char source_path[512];
snprintf(source_path, sizeof(source_path), "%s/project/main.go", tmp);
struct stat source_stat;
ASSERT_EQ(stat(source_path, &source_stat), 0);
#ifdef __APPLE__
int64_t source_mtime_ns =
((int64_t)source_stat.st_mtimespec.tv_sec * (int64_t)CBM_NSEC_PER_SEC) +
(int64_t)source_stat.st_mtimespec.tv_nsec;
#elif defined(_WIN32)
int64_t source_mtime_ns = (int64_t)source_stat.st_mtime * (int64_t)CBM_NSEC_PER_SEC;
#else
int64_t source_mtime_ns = ((int64_t)source_stat.st_mtim.tv_sec * (int64_t)CBM_NSEC_PER_SEC) +
(int64_t)source_stat.st_mtim.tv_nsec;
#endif
ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", source_mtime_ns,
source_stat.st_size),
/* The fixture must record the hash with the SAME mtime source the indexer
* writes with — cbm_path_info_utf8 — not struct stat. On Windows stat
* truncates to seconds while the stored record carries FILETIME-derived
* nanoseconds, so a stat-written fixture would never compare equal and the
* metadata_match contract below would fail. */
cbm_path_info_t path_info;
ASSERT_EQ(cbm_path_info_utf8(source_path, &path_info), 0);
ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", path_info.mtime_ns,
path_info.size),
CBM_STORE_OK);
cbm_project_t project = {0};
ASSERT_EQ(cbm_store_get_project(store, "test-project", &project), CBM_STORE_OK);
Expand Down Expand Up @@ -2782,6 +2777,56 @@ TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_
PASS();
}

/* #1714: coverage freshness must compare mtime_ns at the SAME precision the
* indexer records it. The pipeline records cbm_path_info_utf8's value (which
* on Windows derives from FILETIME — nanosecond), while the freshness reader
* used to recompute from struct stat, which on Windows truncates to seconds
* (st_mtime). A byte-identical file therefore never matched and every path was
* reported metadata_changed. The reader now uses the indexer's own source. */
TEST(tool_check_index_coverage_freshness_uses_indexer_mtime_source_issue1714) {
char tmp[256];
cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp));
ASSERT_NOT_NULL(srv);
cbm_store_t *store = cbm_mcp_server_store(srv);
ASSERT_NOT_NULL(store);

char source_path[512];
snprintf(source_path, sizeof(source_path), "%s/project/main.go", tmp);
cbm_path_info_t info;
ASSERT_EQ(cbm_path_info_utf8(source_path, &info), 0);

/* The hash exactly as the indexer writes it: same source, ns precision. */
ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "", info.mtime_ns,
info.size),
CBM_STORE_OK);

char *response = cbm_mcp_handle_tool(
srv, "check_index_coverage", "{\"project\":\"test-project\",\"paths\":[\"main.go\"]}");
ASSERT_NOT_NULL(response);
ASSERT_NOT_NULL(strstr(response, "\"freshness\":\"metadata_match\""));
free(response);

/* A hash stored at seconds precision — what a stat-based reader previously
* compared against — must NOT match an unchanged file: the comparison must
* stay nanosecond-exact, or part of mtime resolution is silently dropped. */
int64_t seconds_mtime_ns = (info.mtime_ns / (int64_t)CBM_NSEC_PER_SEC) *
(int64_t)CBM_NSEC_PER_SEC;
if (seconds_mtime_ns != info.mtime_ns) {
ASSERT_EQ(cbm_store_upsert_file_hash(store, "test-project", "main.go", "",
seconds_mtime_ns, info.size),
CBM_STORE_OK);
response = cbm_mcp_handle_tool(
srv, "check_index_coverage", "{\"project\":\"test-project\",\"paths\":[\"main.go\"]}");
ASSERT_NOT_NULL(response);
ASSERT_NOT_NULL(strstr(response, "\"freshness\":\"metadata_changed\""));
free(response);
}

cleanup_snippet_dir(tmp);
cbm_mcp_server_free(srv);
PASS();
}

TEST(tool_check_index_coverage_rejects_stale_generation) {
char tmp[256];
cbm_mcp_server_t *srv = setup_snippet_server(tmp, sizeof(tmp));
Expand Down Expand Up @@ -11332,6 +11377,7 @@ SUITE(mcp) {
RUN_TEST(tool_check_index_coverage_preserves_multiple_scope_labels);
RUN_TEST(tool_check_index_coverage_accepts_truncated_ignored_catalog_for_fresh_path_issue1613);
RUN_TEST(tool_check_index_coverage_rejects_stale_generation);
RUN_TEST(tool_check_index_coverage_freshness_uses_indexer_mtime_source_issue1714);
RUN_TEST(tool_check_index_coverage_requires_source_when_file_metadata_changed);
RUN_TEST(tool_check_index_coverage_surfaces_lookup_errors);
RUN_TEST(tool_index_status_includes_git_metadata);
Expand Down
Loading