Skip to content

Commit 41b1aea

Browse files
DeusDataDeusData
committed
fix(mcp): resolve the root owner for unnamed index status polls
With #2134, an unnamed index_repository of a root that another indexed project already owns adopts that owner's name, and the daemon keys the job by it (application_index_project_key reads the forwarded "name"). The unnamed status poll that the #2144 async reply asks for ("the same repo_path and status: true") still derived its key from the path alone in index_status_project_key. For an owned root it never found the job it had just started and answered "no index job or index is known for project '<path-derived>'"; an ambiguous root got that same misleading answer instead of the loud error the start returns. index_status_project_key now runs the start path's index_root_owner_resolve on the same canonical repo_path: an explicit name still wins, a single other owner is the key, several owners fail with the same error naming them, otherwise the path-derived key stays. The status error buffer grows to hold that owner list. The daemon's other readers of the key (job dedupe in application_job_subscribe_locked, the synchronous cut-short marker set and taken around application_index_run_sync) derive it from the forwarded args, which already carry the owner name, so they agree with the start; status was the only reader that diverged. Refs #2134, #2144 Signed-off-by: Martin Vogel <[email protected]>
1 parent 364dd6a commit 41b1aea

2 files changed

Lines changed: 262 additions & 4 deletions

File tree

‎src/mcp/mcp.c‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11392,8 +11392,9 @@ static bool index_call_mode_arg(const char *args, const char *key, bool *value_o
1139211392

1139311393
/* The project key a status query names: an explicit name override wins (the
1139411394
* key the index call used), then repo_path resolved exactly as for indexing
11395-
* (session root, canonical form, workspace boundary), then a known project
11396-
* alias. NULL with *error_out set when none identifies a project. */
11395+
* (session root, canonical form, workspace boundary, and the #2134 root owner
11396+
* an unnamed start adopts), then a known project alias. NULL with *error_out
11397+
* set when none identifies a project. */
1139711398
static char *index_status_project_key(cbm_mcp_server_t *srv, const char *args, char *error_out,
1139811399
size_t error_size) {
1139911400
char *name = cbm_mcp_get_string_arg(args, "name");
@@ -11427,7 +11428,21 @@ static char *index_status_project_key(cbm_mcp_server_t *srv, const char *args, c
1142711428
safe_free(repo_path);
1142811429
return NULL;
1142911430
}
11430-
char *key = repo_path ? cbm_project_name_from_path(repo_path) : NULL;
11431+
/* An unnamed start of a root another project owns keys its job by that
11432+
* owner (#2134), so the poll naming the same repo_path must resolve the
11433+
* same owner, and fail on the same ambiguity, or it never finds the job. */
11434+
char *owner = NULL;
11435+
char *owner_error = NULL;
11436+
if (repo_path && repo_path[0] && !index_root_owner_resolve(repo_path, &owner, &owner_error)) {
11437+
(void)snprintf(error_out, error_size, "%s",
11438+
owner_error ? owner_error : "could not resolve index project name");
11439+
safe_free(owner_error);
11440+
safe_free(repo_path);
11441+
return NULL;
11442+
}
11443+
const char *key_source = owner ? owner : repo_path;
11444+
char *key = key_source ? cbm_project_name_from_path(key_source) : NULL;
11445+
safe_free(owner);
1143111446
safe_free(repo_path);
1143211447
if (!key) {
1143311448
(void)snprintf(error_out, error_size, "could not resolve index project name");
@@ -11445,7 +11460,7 @@ static char *handle_index_repository_status(cbm_mcp_server_t *srv, const char *a
1144511460
"this in-process server tracks no index jobs",
1144611461
true);
1144711462
}
11448-
char error[CBM_SZ_1K] = {0};
11463+
char error[CBM_SZ_4K] = {0}; /* the #2134 ambiguity error lists every owner */
1144911464
char *project = index_status_project_key(srv, args, error, sizeof(error));
1145011465
if (!project) {
1145111466
return cbm_mcp_text_result(error[0] ? error : "could not resolve index project name", true);

‎tests/test_daemon_application.c‎

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6170,6 +6170,246 @@ TEST(daemon_application_async_refused_for_lone_one_shot_client_of_temporary_daem
61706170
PASS();
61716171
}
61726172

6173+
/* ── #2134 x #2144: the unnamed status poll resolves the start's key ────────
6174+
*
6175+
* An unnamed index_repository of a root that another indexed project already
6176+
* owns adopts that owner's name (#2134), so the daemon keys the async job by
6177+
* it. The async reply tells the client to poll "with the same repo_path and
6178+
* status: true"; that unnamed poll must resolve the same key (owner first,
6179+
* then path-derived; several owners fail loudly), or it never finds the job
6180+
* it just started. Owners are real project databases in a private cache; the
6181+
* held fake worker never writes one, so the seeded state is all there is. */
6182+
6183+
enum { APP_OWNER_CANONICAL_CAP = 4096 }; /* cbm_canonical_path needs >= 4096 */
6184+
6185+
typedef struct {
6186+
char cache[APP_TEST_PATH_CAP];
6187+
char canonical_root[APP_OWNER_CANONICAL_CAP];
6188+
char *saved_cache;
6189+
bool had_cache;
6190+
bool cache_set;
6191+
app_async_fixture_t fixture;
6192+
cbm_daemon_runtime_application_session_t *session;
6193+
} app_owner_fixture_t;
6194+
6195+
typedef struct {
6196+
char *started;
6197+
char *running;
6198+
char *finished;
6199+
bool worker_running;
6200+
bool drained;
6201+
} app_owner_run_t;
6202+
6203+
static bool app_owner_fixture_init(app_owner_fixture_t *owner, const char *label) {
6204+
memset(owner, 0, sizeof(*owner));
6205+
const char *old_cache = getenv("CBM_CACHE_DIR");
6206+
owner->had_cache = old_cache != NULL;
6207+
owner->saved_cache = old_cache ? strdup(old_cache) : NULL;
6208+
(void)snprintf(owner->cache, sizeof(owner->cache), "%s/cbm-app-owner-cache-XXXXXX",
6209+
cbm_tmpdir());
6210+
if ((owner->had_cache && !owner->saved_cache) || !cbm_mkdtemp(owner->cache)) {
6211+
owner->cache[0] = '\0';
6212+
return false;
6213+
}
6214+
owner->cache_set = cbm_setenv("CBM_CACHE_DIR", owner->cache, 1) == 0;
6215+
if (!owner->cache_set || !app_async_fixture_init(&owner->fixture, label, true) ||
6216+
!cbm_canonical_path(owner->fixture.root, owner->canonical_root,
6217+
sizeof(owner->canonical_root))) {
6218+
return false;
6219+
}
6220+
/* Stored root_path values use forward slashes on every platform. */
6221+
cbm_normalize_path_sep(owner->canonical_root);
6222+
return true;
6223+
}
6224+
6225+
/* One indexed project whose stored root is this fixture's root. */
6226+
static bool app_owner_seed(const app_owner_fixture_t *owner, const char *project) {
6227+
char path[APP_TEST_PATH_CAP];
6228+
int written = snprintf(path, sizeof(path), "%s/%s.db", owner->cache, project);
6229+
if (written <= 0 || (size_t)written >= sizeof(path)) {
6230+
return false;
6231+
}
6232+
cbm_store_t *store = cbm_store_open_path(path);
6233+
bool seeded =
6234+
store && cbm_store_upsert_project(store, project, owner->canonical_root) == CBM_STORE_OK;
6235+
cbm_store_close(store);
6236+
return seeded;
6237+
}
6238+
6239+
/* An unnamed async start, then unnamed status polls naming the same
6240+
* repo_path: one while the held worker runs, one after it drained. */
6241+
static void app_owner_run(app_owner_fixture_t *owner, app_owner_run_t *run) {
6242+
memset(run, 0, sizeof(*run));
6243+
app_async_fixture_t *fixture = &owner->fixture;
6244+
run->started = app_async_call_now(fixture, owner->session, ",\"async\":true");
6245+
run->worker_running = run->started && !app_async_contains(run->started, "\"isError\":true") &&
6246+
app_wait_for_atomic_int(&fixture->fake.starts, 1);
6247+
run->running = app_async_call_now(fixture, owner->session, ",\"status\":true");
6248+
atomic_store(&fixture->fake.allow_completion, true);
6249+
run->drained = run->worker_running && app_wait_for_active_jobs(fixture->application, 0);
6250+
run->finished =
6251+
run->drained ? app_async_call_now(fixture, owner->session, ",\"status\":true") : NULL;
6252+
}
6253+
6254+
static void app_owner_run_free(app_owner_run_t *run) {
6255+
free(run->started);
6256+
free(run->running);
6257+
free(run->finished);
6258+
memset(run, 0, sizeof(*run));
6259+
}
6260+
6261+
static bool app_owner_fixture_finish(app_owner_fixture_t *owner) {
6262+
if (owner->session) {
6263+
owner->fixture.callbacks.session_close(owner->fixture.callbacks.context, owner->session);
6264+
owner->session = NULL;
6265+
}
6266+
bool stopped = app_async_fixture_finish(&owner->fixture);
6267+
if (owner->cache_set) {
6268+
if (owner->had_cache) {
6269+
(void)cbm_setenv("CBM_CACHE_DIR", owner->saved_cache, 1);
6270+
} else {
6271+
(void)cbm_unsetenv("CBM_CACHE_DIR");
6272+
}
6273+
}
6274+
free(owner->saved_cache);
6275+
owner->saved_cache = NULL;
6276+
if (owner->cache[0]) {
6277+
(void)th_rmtree(owner->cache);
6278+
}
6279+
return stopped;
6280+
}
6281+
6282+
static bool app_owner_names_project(const char *response, const char *project) {
6283+
char needle[APP_TEST_PATH_CAP];
6284+
int written = snprintf(needle, sizeof(needle), "\"project\":\"%s\"", project);
6285+
return written > 0 && (size_t)written < sizeof(needle) && app_async_contains(response, needle);
6286+
}
6287+
6288+
TEST(daemon_application_unnamed_status_finds_async_job_of_root_owner_issue2134) {
6289+
app_owner_fixture_t owner;
6290+
bool setup =
6291+
app_owner_fixture_init(&owner, "owner-poll") && app_owner_seed(&owner, "owned-2134s");
6292+
owner.session = setup ? app_async_session(&owner.fixture, 7801) : NULL;
6293+
bool session_opened = owner.session != NULL;
6294+
app_owner_run_t run;
6295+
memset(&run, 0, sizeof(run));
6296+
if (session_opened) {
6297+
app_owner_run(&owner, &run);
6298+
}
6299+
int cancels = atomic_load(&owner.fixture.fake.cancels);
6300+
bool stopped = app_owner_fixture_finish(&owner);
6301+
if (!app_async_contains(run.running, "\"state\":\"running\"")) {
6302+
fprintf(stderr, " [2134x2144] unnamed status missed the owner's job: %.400s\n",
6303+
run.running ? run.running : "(null)");
6304+
}
6305+
6306+
ASSERT_TRUE(setup);
6307+
ASSERT_TRUE(session_opened);
6308+
/* #2134: the unnamed start adopted the owner; the daemon keyed its job so. */
6309+
ASSERT_TRUE(app_owner_names_project(run.started, "owned-2134s"));
6310+
ASSERT_TRUE(run.worker_running);
6311+
/* The unnamed poll with the same repo_path reports that live job ... */
6312+
ASSERT_FALSE(app_async_contains(run.running, "\"isError\":true"));
6313+
ASSERT_TRUE(app_owner_names_project(run.running, "owned-2134s"));
6314+
ASSERT_TRUE(app_async_contains(run.running, "\"state\":\"running\""));
6315+
/* ... and, once drained, the outcome recorded under the same key. */
6316+
ASSERT_TRUE(run.drained);
6317+
ASSERT_TRUE(app_owner_names_project(run.finished, "owned-2134s"));
6318+
ASSERT_TRUE(app_async_contains(run.finished, "\"state\":\"succeeded\""));
6319+
ASSERT_EQ(cancels, 0);
6320+
ASSERT_TRUE(stopped);
6321+
app_owner_run_free(&run);
6322+
PASS();
6323+
}
6324+
6325+
/* Control: the path-derived key is untouched where #2134 keeps it - a root no
6326+
* other project owns, and a root whose path-derived project exists even though
6327+
* a named twin shares it. Start and poll name the same project either way. */
6328+
TEST(daemon_application_unnamed_status_keeps_path_derived_key_issue2134) {
6329+
app_owner_fixture_t fresh;
6330+
bool fresh_setup = app_owner_fixture_init(&fresh, "owner-fresh");
6331+
fresh.session = fresh_setup ? app_async_session(&fresh.fixture, 7811) : NULL;
6332+
char *fresh_project = fresh.fixture.project ? strdup(fresh.fixture.project) : NULL;
6333+
app_owner_run_t fresh_run;
6334+
memset(&fresh_run, 0, sizeof(fresh_run));
6335+
if (fresh.session) {
6336+
app_owner_run(&fresh, &fresh_run);
6337+
}
6338+
bool fresh_stopped = app_owner_fixture_finish(&fresh);
6339+
6340+
app_owner_fixture_t twin;
6341+
bool twin_setup = app_owner_fixture_init(&twin, "owner-twin") && twin.fixture.project &&
6342+
app_owner_seed(&twin, twin.fixture.project) &&
6343+
app_owner_seed(&twin, "twin-2134s");
6344+
twin.session = twin_setup ? app_async_session(&twin.fixture, 7812) : NULL;
6345+
char *twin_project = twin.fixture.project ? strdup(twin.fixture.project) : NULL;
6346+
app_owner_run_t twin_run;
6347+
memset(&twin_run, 0, sizeof(twin_run));
6348+
if (twin.session) {
6349+
app_owner_run(&twin, &twin_run);
6350+
}
6351+
bool twin_stopped = app_owner_fixture_finish(&twin);
6352+
6353+
ASSERT_TRUE(fresh_setup);
6354+
ASSERT_NOT_NULL(fresh_project);
6355+
ASSERT_TRUE(app_owner_names_project(fresh_run.started, fresh_project));
6356+
ASSERT_TRUE(fresh_run.worker_running);
6357+
ASSERT_TRUE(app_owner_names_project(fresh_run.running, fresh_project));
6358+
ASSERT_TRUE(app_async_contains(fresh_run.running, "\"state\":\"running\""));
6359+
ASSERT_TRUE(app_async_contains(fresh_run.finished, "\"state\":\"succeeded\""));
6360+
ASSERT_TRUE(fresh_stopped);
6361+
6362+
ASSERT_TRUE(twin_setup);
6363+
ASSERT_NOT_NULL(twin_project);
6364+
ASSERT_TRUE(app_owner_names_project(twin_run.started, twin_project));
6365+
ASSERT_FALSE(app_owner_names_project(twin_run.started, "twin-2134s"));
6366+
ASSERT_TRUE(twin_run.worker_running);
6367+
ASSERT_TRUE(app_owner_names_project(twin_run.running, twin_project));
6368+
ASSERT_TRUE(app_async_contains(twin_run.running, "\"state\":\"running\""));
6369+
ASSERT_TRUE(app_async_contains(twin_run.finished, "\"state\":\"succeeded\""));
6370+
ASSERT_TRUE(twin_stopped);
6371+
free(fresh_project);
6372+
free(twin_project);
6373+
app_owner_run_free(&fresh_run);
6374+
app_owner_run_free(&twin_run);
6375+
PASS();
6376+
}
6377+
6378+
/* Control: several owners and no path-derived project is ambiguous. The start
6379+
* still refuses and names them without spawning a worker, and the poll gives
6380+
* the same loud error instead of guessing (or pointing at a path-derived
6381+
* project nobody indexes). */
6382+
TEST(daemon_application_unnamed_async_and_status_refuse_ambiguous_root_issue2134) {
6383+
app_owner_fixture_t owner;
6384+
bool setup = app_owner_fixture_init(&owner, "owner-ambiguous") &&
6385+
app_owner_seed(&owner, "first-2134s") && app_owner_seed(&owner, "second-2134s");
6386+
owner.session = setup ? app_async_session(&owner.fixture, 7821) : NULL;
6387+
bool session_opened = owner.session != NULL;
6388+
app_owner_run_t run;
6389+
memset(&run, 0, sizeof(run));
6390+
if (session_opened) {
6391+
app_owner_run(&owner, &run);
6392+
}
6393+
int starts = atomic_load(&owner.fixture.fake.starts);
6394+
bool stopped = app_owner_fixture_finish(&owner);
6395+
6396+
ASSERT_TRUE(setup);
6397+
ASSERT_TRUE(session_opened);
6398+
ASSERT_TRUE(app_async_contains(run.started, "\"isError\":true"));
6399+
ASSERT_TRUE(app_async_contains(run.started, "several indexed projects share root_path"));
6400+
ASSERT_TRUE(app_async_contains(run.started, "first-2134s"));
6401+
ASSERT_TRUE(app_async_contains(run.started, "second-2134s"));
6402+
ASSERT_FALSE(run.worker_running);
6403+
ASSERT_EQ(starts, 0);
6404+
ASSERT_TRUE(app_async_contains(run.running, "\"isError\":true"));
6405+
ASSERT_TRUE(app_async_contains(run.running, "several indexed projects share root_path"));
6406+
ASSERT_TRUE(app_async_contains(run.running, "first-2134s"));
6407+
ASSERT_TRUE(app_async_contains(run.running, "second-2134s"));
6408+
ASSERT_TRUE(stopped);
6409+
app_owner_run_free(&run);
6410+
PASS();
6411+
}
6412+
61736413
SUITE(daemon_application) {
61746414
RUN_TEST(daemon_application_oversized_reply_is_a_jsonrpc_error_not_a_death);
61756415
RUN_TEST(daemon_application_new_session_does_not_retain_initial_store);
@@ -6197,6 +6437,9 @@ SUITE(daemon_application) {
61976437
RUN_TEST(daemon_application_cancelled_sync_index_reply_offers_async);
61986438
RUN_TEST(daemon_application_cut_short_sync_index_advice_reaches_next_call);
61996439
RUN_TEST(daemon_application_async_refused_for_lone_one_shot_client_of_temporary_daemon);
6440+
RUN_TEST(daemon_application_unnamed_status_finds_async_job_of_root_owner_issue2134);
6441+
RUN_TEST(daemon_application_unnamed_status_keeps_path_derived_key_issue2134);
6442+
RUN_TEST(daemon_application_unnamed_async_and_status_refuse_ambiguous_root_issue2134);
62006443
RUN_TEST(daemon_application_programmatic_index_injects_resource_policy);
62016444
RUN_TEST(daemon_application_auto_index_honors_tracked_file_limit);
62026445
RUN_TEST(daemon_application_auto_index_file_count_handles_literal_metacharacter_path);

0 commit comments

Comments
 (0)