fix(daemon): keep one separator spelling for the session root on Windows - #2070
Conversation
On Windows cbm_canonical_path answers in the platform's native backslash form, and the daemon stored that spelling verbatim for the client's session_root and allowed_root (cbm_mcp_server_set_session_context) and for the auto-index job's repo_path (application.c canonicalizes with the bare call at three sites). Every tool handler, however, normalizes separators after canonicalizing. One directory therefore had two names, and the exact comparison in application_index_args_equal never matched: an explicit index_repository request for the session root was refused as OPTIONS_CONFLICT instead of joining the auto-index job already running for it. Normalize the separator spelling of both session roots in set_session_context, and canonicalize plus normalize at the three application.c sites through one helper, application_canonical_root(). Separators fold on every platform, so the new test daemon_session_context_keeps_one_spelling_of_a_root binds wherever the suite runs; it fails on main with "C:\repos\cbm" != "C:/repos/cbm". Distilled from #1726 with co-author credit. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com> Co-authored-by: 刘冲 <mail@liuchong.dev>
…pelling the policy root The first cut on this branch (c1b9c45) respelled the session root and the allowed root with forward slashes on the way into the session policy (cbm_mcp_server_set_session_context) and at the three application.c sites that canonicalize a root. CI's Windows shard then failed two tests: tests/test_daemon_application.c:2256: ASSERT(sensitive_blocked) tests/test_daemon_application.c:2380: ASSERT(sensitive_blocked) Both set HOME to the cbm_canonical_path spelling of a directory - the platform's native form, backslashes on Windows - and expect a session rooted there to be refused for auto-index and for watch. The sensitive-root and allowed-root containment walk (ws_is_ancestor_or_equal in src/foundation/workspace.c) compares prefixes byte-exact, so a root that now read C:/Users/... no longer matched a HOME of C:\Users\...: the sensitive refusal was lost and $HOME was admitted. Respelling the policy root was the wrong layer. This commit keeps the policy roots in their canonical native spelling (mcp.c and the three application.c sites are back to what main runs) and folds the separator spelling at the one comparison the original defect lives in: application_index_args_equal, where the running job's args are compared with an explicit index_repository request's. The auto-index job spells repo_path the way the session policy holds it; the handler spells it with forward slashes. On Windows the two never matched, and the request was refused as OPTIONS_CONFLICT instead of joining the job already running for its root. The fold acts on the parsed copies only (yyjson_mut_obj_replace with a cbm_normalize_path_sep'd string), so nothing the daemon stores changes spelling and every other option stays exact. Why this is correct on Windows without a local Windows run: the policy spelling is untouched, so the containment path is byte-for-byte what main runs there today; cbm_normalize_path_sep folds backslashes on every platform, so the new fold is exercised by the new test wherever the suite runs. Tests: daemon_session_context_keeps_one_spelling_of_a_root asserted that the policy respells a root with forward slashes. That was the wrong contract: the policy's containment checks compare its spelling byte-exact against HOME and the granted roots, so a respelled root is exactly what stops matching them. It is replaced by daemon_session_context_keeps_the_policy_spelling_of_a_root, which binds the opposite: the policy stores the root as given. The comparison is exposed through application_internal.h as cbm_daemon_application_index_args_equal_for_test and bound by daemon_application_index_args_compare_repo_path_separator_equivalently: a backslash and a forward-slash repo_path compare equal; a different path, a sub-path, and a different mode still do not. Refs #1726 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
Windows red, and what changed.
The follow-up commit (9846c3f) keeps the policy roots in their canonical native spelling ( Tests: |
1 participant
What
On Windows the daemon kept the client's
session_root/allowed_rootand the auto-index job'srepo_pathin the platform's native backslash spelling:cbm_mcp_server_set_session_contextstored the roots verbatim, andapplication.ccanonicalized with the barecbm_canonical_pathat three sites. Every tool handler, however, normalizes separators after canonicalizing. One directory therefore had two names inside the daemon.Symptom
application_index_args_equalcompares roots exactly, so an explicitindex_repositoryrequest for the session root never equalled the running auto-index job's args and was refused asOPTIONS_CONFLICTinstead of joining that job.Fix
cbm_mcp_server_set_session_contextnormalizes the separator spelling of both roots before storing them.application.ccanonicalizes and normalizes at the three sites through one helper,application_canonical_root()(=cbm_canonical_path+cbm_normalize_path_sep).daemon_session_context_keeps_one_spelling_of_a_rootintests/test_daemon.c. Separators fold on every platform, so it binds wherever the suite runs.Distilled from #1726 with co-author credit to @liuchong -- the fix only, nothing from the feature.
Verification
tests/test_daemon.c:483: "C:\repos\cbm" != "C:/repos/cbm"(daemon suite 12 passed, 1 failed). Fix re-applied byte-identical: green.daemon daemon_application mcp daemon_ipc cli-- 728 passed, 4 skipped.make -f Makefile.cbm lint-ci(cppcheck, clang-format, NOLINT whitelist) green;scripts/check-no-test-skips.shOK.Refs #1726