fix(cross-repo): stop a "::missed" shadow row making a project unresolvable
Indexing writes an internal "<name>::missed" miss-graph row into the SAME db as
the primary project whenever a file parses partially. cr_store_has_exact_project
required `count == 1` over ALL rows returned by cbm_store_list_projects, which
does not filter those rows - so any project that had ever recorded a parse miss
failed validation, as SOURCE and as TARGET, and the whole feature reported:
project is not indexed
for a project that plainly was. There is no user-level workaround: a partial
parse is not something the operator controls, and re-indexing reproduces the
shadow row.
This is the same defect mcp.c fixed for list_projects in #1044 ("requiring
n == 1 over ALL rows made every project with a miss graph vanish"); the
cross-repo site never learned it. The fix ports that primary-row filter.
The single-primary requirement itself is deliberately kept: it is what proves
the db belongs to the project we were asked about rather than being a shared or
mislabelled store. Only "::" shadow rows stop counting toward it.
Reported by vitaliy-shatskiy in #1609, whose diagnosis named the exact function
and the exact reason.
Reproduce-first, and revert-checked both ways:
- the new test fails on origin/main with `ASSERT(!(result.failed))`
(tests/test_cross_repo.c), for the behaviour under test rather than a setup
error;
- it passes with the fix;
- reverting ONLY src/pipeline/pass_cross_repo.c and keeping the test brings
the identical RED back, so the test binds to the production change.
The existing pair without shadow rows is the control: those tests already prove
that path returns edges, so this cannot pass vacuously on a fixture that never
matched.
cross_repo 8 passed; pipeline 249 passed; store_edges 25 and store_nodes 67
passed - no collateral change.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
@@ -123,9 +123,32 @@ static int cr_project_compare(const void *left, const void *right) {
|
||||
static bool cr_store_has_exact_project(cbm_store_t *store, const char *project) {
|
||||
cbm_project_t *projects = NULL;
|
||||
int count = 0;
|
||||
bool matches = store && cbm_store_check_integrity(store) &&
|
||||
cbm_store_list_projects(store, &projects, &count) == CBM_STORE_OK &&
|
||||
count == 1 && projects[0].name && strcmp(projects[0].name, project) == 0;
|
||||
if (!store || !cbm_store_check_integrity(store) ||
|
||||
cbm_store_list_projects(store, &projects, &count) != CBM_STORE_OK) {
|
||||
cbm_store_free_projects(projects, count);
|
||||
return false;
|
||||
}
|
||||
/* Ignore internal shadow projects ("<name>::missed" miss-graph rows): they
|
||||
* live in the SAME db as the primary project, so any project that has ever
|
||||
* recorded a parse miss carries two rows. Requiring count == 1 over ALL rows
|
||||
* therefore made such a project unresolvable as source AND as target, and
|
||||
* cross-repo reported it as not indexed when it plainly was (#1609). This is
|
||||
* the same defect mcp.c fixed for list_projects in #1044; this site never
|
||||
* learned it.
|
||||
*
|
||||
* The single-primary requirement itself is kept: it is what proves this db
|
||||
* belongs to the project we were asked about, rather than being a shared or
|
||||
* mislabelled store. Only the shadow rows stop counting toward it. */
|
||||
int primary_count = 0;
|
||||
bool primary_matches = false;
|
||||
for (int i = 0; i < count; i++) {
|
||||
const char *name = projects[i].name;
|
||||
if (name && name[0] && !strstr(name, "::")) {
|
||||
primary_count++;
|
||||
primary_matches = strcmp(name, project) == 0;
|
||||
}
|
||||
}
|
||||
bool matches = primary_count == 1 && primary_matches;
|
||||
cbm_store_free_projects(projects, count);
|
||||
return matches;
|
||||
}
|
||||
|
||||
@@ -469,7 +469,58 @@ TEST(cross_repo_pre_cancel_preserves_existing_cross_edges) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* Add the internal "<name>::missed" miss-graph row that indexing writes into
|
||||
* the SAME db whenever a file parses partially. */
|
||||
static bool cross_repo_add_missed_shadow(const cross_repo_fixture_t *fixture, const char *project) {
|
||||
char path[512];
|
||||
char shadow[256];
|
||||
if (!cross_repo_project_path(fixture, project, path, sizeof(path))) {
|
||||
return false;
|
||||
}
|
||||
snprintf(shadow, sizeof(shadow), "%s::missed", project);
|
||||
cbm_store_t *store = cbm_store_open_path(path);
|
||||
if (!store) {
|
||||
return false;
|
||||
}
|
||||
bool ok = cbm_store_upsert_project(store, shadow, fixture->cache) == CBM_STORE_OK;
|
||||
cbm_store_close(store);
|
||||
return ok;
|
||||
}
|
||||
|
||||
/* #1609: any project that has ever recorded a parse miss carries a
|
||||
* "<name>::missed" shadow row in its own db. cr_store_has_exact_project
|
||||
* demanded count == 1 over ALL rows, so that second row made the project
|
||||
* unresolvable — as source AND as target — and the whole feature failed with
|
||||
* "not indexed" for a project that plainly was. mcp.c already solved exactly
|
||||
* this shape for list_projects in #1044; this site never learned it.
|
||||
*
|
||||
* The control is the pair without shadow rows: the tests above already prove
|
||||
* that path returns edges, so a regression here cannot hide behind a fixture
|
||||
* that never matched in the first place. */
|
||||
TEST(cross_repo_accepts_project_with_missed_shadow_row_issue1609) {
|
||||
cross_repo_fixture_t fixture;
|
||||
bool setup = cross_repo_fixture_begin(&fixture) &&
|
||||
cross_repo_seed_http_pair(&fixture, "shadow-source", "shadow-target", "/orders",
|
||||
"s") &&
|
||||
cross_repo_add_missed_shadow(&fixture, "shadow-source") &&
|
||||
cross_repo_add_missed_shadow(&fixture, "shadow-target");
|
||||
if (!setup) {
|
||||
cross_repo_fixture_end(&fixture);
|
||||
FAIL("failed to seed shadow-row fixture");
|
||||
}
|
||||
|
||||
const char *target = "shadow-target";
|
||||
cbm_cross_repo_result_t result = cbm_cross_repo_match("shadow-source", &target, 1);
|
||||
cross_repo_fixture_end(&fixture);
|
||||
|
||||
ASSERT_FALSE(result.failed);
|
||||
ASSERT_EQ(result.projects_scanned, 1);
|
||||
ASSERT_EQ(result.http_edges, 1);
|
||||
PASS();
|
||||
}
|
||||
|
||||
SUITE(cross_repo) {
|
||||
RUN_TEST(cross_repo_accepts_project_with_missed_shadow_row_issue1609);
|
||||
RUN_TEST(cross_repo_null_target_fails_without_dereference);
|
||||
RUN_TEST(cross_repo_wildcard_keeps_projects_containing_internal_tokens);
|
||||
RUN_TEST(cross_repo_scan_bound_counts_examined_rows_not_matches);
|
||||
|
||||
Reference in New Issue
Block a user