feat(daemon): let a product build relocate the runtime directory - #1576
feat(daemon): let a product build relocate the runtime directory#1576tmonestudio wants to merge 5 commits into
Conversation
A live checkout SHA is not proof of the generation that produced graph content, so the freshness verdict must come from the indexed-checkout identity recorded with the DB. Legacy DBs record no such identity: verbose index_status now exposes the graph generation separately (indexed_generation) and fails closed with verdict=unknown, indexed_checkout_sha=null and reason=indexed_checkout_unavailable. The default status call stays lean and the freshness block is report-only — it never triggers indexing. Add focused coverage for the fail-closed verdict and for its omission from the default output. Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Record the frozen git HEAD at the same successful staged-generation boundary as graph coverage. cbm_coverage_meta_t gains indexed_checkout_sha; new writable DBs create a nullable indexed_checkout_sha column and existing writable DBs are migrated idempotently in init_schema, while read-only legacy DBs without the column still open, read and report no identity instead of erroring. coverage_replace_ex writes/updates the SHA inside its existing transaction, so failure/rollback retains the prior identity. Full, delta-incremental and legacy-incremental generation templates set it from the pipeline's refreshed git context (NULL/empty for non-git repos). index_status freshness now derives from the recorded identity: no indexed SHA -> unknown/indexed_checkout_unavailable; differs from the live git HEAD -> stale/indexed_checkout_mismatch; equal -> current. Emits indexed_generation, indexed_checkout_sha, checkout_sha, a stable reasons array and recommended_action; stays verbose-only and read-only. Add store round-trip/rollback coverage and MCP verdict tests for matching current and mismatched stale (git fixtures, platform-skipped on Windows). Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Add cbm_git_worktree_status: parse git status --porcelain=v1 -z
--untracked-files=all record-by-record over a bounded chunk buffer
(4K chunks, 16K field cap). Counts advance for the whole stream while
only max_samples paths per class (tracked/untracked) are retained;
exceeding the cap sets *_truncated. Rename/copy records consume their
second NUL-separated source path without counting it, so one rename
counts once. available is true ONLY when git exited 0 and every record
parsed cleanly; any shell-unsafe path, spawn failure, nonzero exit,
malformed/oversized/trailing record leaves available=false with zero
counts - callers must never read that as a clean worktree.
Index the snapshot into the verbose freshness block (status_available,
tracked_changes{count,paths,truncated}, untracked_source{...}). Verdict
composition: clean matching SHA -> current; tracked changes or SHA
mismatch -> stale; untracked-only, unavailable status or missing live
HEAD -> unknown. Reasons array keeps every applicable code in stable
order with the dominant reason first; recommended_action derives from
the verdict. Report-only, verbose-only, read-only: no indexing or
mutation is ever triggered.
Owner gates corrected: restored missing #endif around canonical_root
tests; rename/copy detection checks both XY columns; introduced
cbm_pipeline_indexed_checkout_sha accessor because cbm_pipeline_t is
opaque in pipeline_incremental.c (HEAD accessed git_ctx.head_sha
directly and could not compile); Windows tests force git add -f so a
user's global excludes cannot silently drop the tracked fixture;
rollback test now mutates row detail so the shadow rebuild actually
executes before the replace is rolled back.
Tests now shell out through cbm_popen (isolated spawn) instead of
system(), so the git-backed freshness/status tests run on Windows too;
only a genuinely missing git skips. Prior SHA persistence (staged-
generation boundary) is the anchor the new worktree status refines.
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
The daemon/CLI rendezvous is created under %LOCALAPPDATA% (Windows) or the POSIX runtime/home directory, resolved with no override in a product build. When that ancestry carries a mutation-granting ACE for an untrusted identity -- an AppContainer capability SID, for instance -- it fails cbm_daemon_ipc_private_directory_secure() and the binary cannot start at all. config list is unreachable too, because it needs the same endpoint, so the operator cannot even inspect settings to diagnose it. The only relocation hook, CBM_TEST_DAEMON_RUNTIME_PARENT, is compiled out unless CBM_ENABLE_TEST_SEAMS is defined, so a test build starts while the product build does not. CBM_RUNTIME_DIR does not relax the check: the named directory goes through exactly the same validation, and an unusable value is refused rather than ignored. The operator only chooses an ancestry that passes. The test seam keeps precedence so existing lifecycle tests keep their isolation, and the override is applied at every product endpoint call site -- daemon, local CLI, tool execution and index worker -- so the CLI path cannot silently keep the default. Refs DeusData#1574 Signed-off-by: Leonardo trindade miranda <tmonestudio@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
DeusData
added
bug
|
Thank you @tmonestudio for turning #1574 into a fail-closed proposal rather than weakening the ancestry check. The issue is grounded on current Before substantive review, this branch needs to be unstacked. Its first four commits ( Please rebase/cherry-pick the runtime commit onto current |
…IME_DIR
The daemon/CLI rendezvous directory is created under %LOCALAPPDATA% (Windows) or
/tmp -- /private/tmp on macOS -- and every ancestor of it must pass the
private-directory walk. That ancestry is not always acceptable, and when it is
not, EVERY invocation fails, `config list` included, so the settings surface
cannot be reached either:
codebase-memory-mcp: secure daemon endpoint could not be created
DeusData#1623 narrowed the Windows side of this by admitting AppContainer package and
capability SIDs on ancestors, and named the remainder explicitly: a live local
group, Authenticated Users inherited from a secondary volume root, and orphaned
unresolvable SIDs still refuse, and "those need CBM_RUNTIME_DIR or a separate
change". DeusData#1621 is the POSIX shape of the same dead end -- /private/tmp/cbm-daemon-<uid>
refused with no way to move it.
There was no way to move it in a shipped build. The only relocation hook,
CBM_TEST_DAEMON_RUNTIME_PARENT, is compiled out unless CBM_ENABLE_TEST_SEAMS is
defined, so a test build started while the shipped build did not; CBM_CACHE_DIR
is no help either, because it moves the cache and never the rendezvous.
CBM_RUNTIME_DIR names the parent directory the rendezvous is created under. It
does NOT relax the check: the directory it names goes through exactly the same
validation as the default -- ancestors owned by you or root, not world-writable,
no allow-ACL; the rendezvous directory itself still forced to owner-only -- and a
value that fails is refused rather than silently replaced by the default. The
operator only chooses an ancestry that passes. cbm_safe_getenv never truncates,
so no half of an over-long value can become a runtime parent.
The override is resolved in cbm_daemon_bootstrap_endpoint_new(), the one function
every product endpoint goes through: the daemon, the MCP client, the local CLI,
the index worker, and the install/update/uninstall activation path in cli.c. No
call site can silently keep the default, and the detached daemon inherits the
value with the rest of its environment. An explicit parent still wins, so the
compile-time test seam and the lifecycle guards' isolated namespace behave
exactly as before.
Approach and variable name from DeusData#1576 by Leonardo trindade miranda, resolved one
layer lower so the activation path is covered too.
Refs DeusData#1574
Refs DeusData#1621
Co-Authored-By: Leonardo trindade miranda <tmonestudio@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
|
Thank you for this — your idea shipped, and you are named in the commit.
The one change is where it is resolved. Yours sat in the daemon path; the merged version resolves it in It keeps your semantics exactly: it does not relax the check. The directory you name goes through the same validation as the default, and a value that fails is refused rather than ignored — the operator chooses an ancestry that passes, they do not get to switch the rule off. It ships in v0.10.5 today, and it is the escape hatch several people have needed for weeks — #1537, #1574 and #1621 all reduce to "the rendezvous is not relocatable in a shipped build". Two things I'm sorry about: this PR stayed open and unanswered while its idea went into the product, and your #1561 is still open and blocked on nothing but |
2 participants
Closes the blocker reported in #1574.
Problem
The daemon/CLI rendezvous directory is created under
%LOCALAPPDATA%(Windows)or the POSIX runtime/home directory, and a product build has no way to point it
elsewhere. When that ancestry carries a mutation-granting ACE for an untrusted
identity, it fails
cbm_daemon_ipc_private_directory_secure()and the binarycannot start at all —
config listincluded, since it needs the same endpoint,so the operator cannot even inspect settings to diagnose it.
The only relocation hook,
CBM_TEST_DAEMON_RUNTIME_PARENT, is compiled outunless
CBM_ENABLE_TEST_SEAMSis defined. That is why a test build starts on anaffected machine while the product build does not.
Approach
CBM_RUNTIME_DIRdoes not relax the check. The named directory goes throughexactly the same validation, and an unusable value is refused rather than
silently ignored. The operator only chooses an ancestry that passes.
Two details worth reviewing:
lifecycle tests keep their isolated rendezvous namespace.
local CLI, tool execution and index worker. Wiring only
main_daemon_endpoint_new()would have left theclipath on the default andlooked fixed while it was not.
Evidence
Measured on the affected machine (Windows 11, single user), same environment,
same cache, only the patch differing:
config listmainsecure CLI coordination could not be created (endpoint)CBM_RUNTIME_DIRset to a clean-ancestry parentTwo non-obvious properties this surfaced, which may be worth documenting
regardless of this PR:
operator. A pre-created directory inherits the volume's ACEs and then fails as
an ancestor; the tool hardens the final directory itself.
a volume root works.
Tests
tests/test_product_runtime_dir_override.py, a process-level guard in the styleof
tests/test_daemon_open_readiness.py. It asserts that the rendezvous iscreated under the named directory, that leaving the variable unset creates
nothing there, and that an unusable value is refused rather than ignored.
Honest disclosure about what I could and could not run locally:
directory lives under the very
%LOCALAPPDATA%whose ACE causes the bug, sothe parent it creates is itself rejected. On a clean runner it should pass;
your CI will be the first to actually execute it.
make -f Makefile.cbm test-focusedfails to compile
tests/test_daemon_ipc.cwith-Werror=stringop-overflowunder w64devkit GCC 2.8.0. I verified this ispre-existing and unrelated by building the same target on the unpatched
base commit — identical failure, same line. Your CI uses LLVM 20, which does
not raise it.
Happy to adjust the variable name, move the guard into the C suite, or rework
the approach entirely if you would rather solve this differently.