watcher_poll_only_watched_projects failed on the macOS LSan job of PR
#2295, whose diff was Java-only, with
`FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1`
(8211 passed, 1 failed). The LSan report that followed points into the
aborted test body (watcher and store never freed after the early FAIL):
a consequence, not a separate leak.
The assert expected ONE touch+poll after a dirty change to reindex
exactly once. That needs every git probe of the poll to succeed. A poll
whose git spawn is refused under runner load is a fail-closed no-op by
design (#937: the baseline stays uncommitted and the next poll retries),
and that path returns without a log line. This is the same test race
that #2376 fixed in watcher_unwatch_drains_pending_free and
watcher_sustained_failure_logs_once_issue2015, not a production defect.
Holding one exhausted spawn with
cbm_subprocess_force_spawn_eagain_for_testing at that poll reproduces
the CI output exactly: watch, baseline, then
`FAIL tests/test_watcher.c:2971: index_call_count == 0, expected 1 == 1`,
with no watcher.changed and no watcher.git.failed.
The old test was also partly inert. The Go original (TestPollAllOnlyWatched,
from the #49 fix that replaced scan-all with an explicit watch list)
registered BOTH projects in the store and asserted that the unwatched
one gets no watcher state. The C port dropped the store registration, so
nothing let the watcher learn about B, and the B half could not fail.
The test now:
- registers A and B in the store, and watches only A;
- asserts that the baseline poll indexes nothing and does not adopt B;
- makes both worktrees dirty (a dirty change, not a commit, so a late
baseline still sees it);
- polls until A's reindex has happened, asserting after EVERY poll that B
has no watcher state (cbm_watcher_index_failure_count == -1), was never
reindexed, and that the watch count is still 1;
- asserts that A was reindexed exactly once.
No timer or deadline decides the verdict. A watcher that never reindexes
A hangs, which the runner reports as a failure, never a pass.
Evidence (macOS arm64, ASan/UBSan test-runner, CBM_SANITIZED, so one
exhausted spawn = 10 injected refusals), with temporary injection that
has since been removed:
- CI-shaped (baseline OK, first change poll refused): consumed=10 at
loop poll 1, watcher.changed at loop poll 2, PASS.
- Heavy (baseline poll plus loop polls 1-3 refused): loop poll 4 takes
the baseline, poll 5 reindexes A; loop polls=5 watched=1 other=0, PASS.
- Mutation, the pre-#49 scan-all (poll_once adopts every stored project):
`FAIL tests/test_watcher.c:3001: cbm_watcher_index_failure_count(w,
"projB-ow") == 0, expected -1 == -1`, both unforced and with the heavy
refusals (A's and B's baseline spawns both refused). The other 82
watcher tests stay green under that mutation, so this is the only
test guarding #49.
Final tree: build/c/test-runner watcher, 5/5 runs, 83 passed each.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
watcher_sustained_failure_logs_once_issue2015 failed on macos-14 CI with
`failing_index_calls == 13, expected 14`. Its loop ran exactly 14
touch+poll rounds and assumed each one reached the index callback. A poll
whose git probe fails transiently (spawn refused under runner load) is a
fail-closed no-op by design (#937): the baseline stays uncommitted and the
next poll retries. So one refused spawn costs one attempt, and the count
ends at 13. This is the same test race as watcher_unwatch_drains_pending_free,
not a production defect. Holding one exhausted spawn mid-loop with
cbm_subprocess_force_spawn_eagain_for_testing reproduces the CI line
exactly.
The loop now polls until the 14th failed attempt instead of counting
polls. Each poll runs at most one attempt for the single project, so it
stops at exactly 14. A watcher that never reindexes hangs, and the runner
reports that as a failure, never a pass. The log-once contract means the
same thing as before: watcher.index.sustained_failure is emitted only on a
failed index attempt when the streak equals the threshold, so extra no-op
polls cannot add lines. An `==` to `>=` mutation in watcher.c still turns
it red (`sustained_log_hits == 5, expected 1`).
The change is now a dirty worktree instead of a commit. If the baseline
poll itself failed, the old commit would be adopted as the baseline HEAD
and never reindexed, so the wait would never end. The baseline keeps the
dirty signature at "clean known", so a dirty change is seen whenever the
baseline lands. With spawns forced to fail at the baseline poll and again
mid-loop, the test passes after 16 polls for 14 attempts.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
watcher_unwatch_drains_pending_free failed three times on macos-14 CI
with `index_call_count == 0, expected 1` at its dirty-poll precondition.
That precondition needs every git subprocess of one poll (rev-parse
HEAD, status, submodule foreach) to succeed. A transient probe failure
(spawn refused under runner load, a non-zero exit) makes that poll a
fail-closed no-op by design: #937 leaves the baseline uncommitted and
the next poll retries. Several of those paths return without a log
line, which matches the CI output exactly: watch, baseline, FAIL, with
no watcher.changed and no watcher.git.failed. Holding a spawn failure
with cbm_subprocess_force_spawn_eagain_for_testing reproduces that
output deterministically. This is a test race, not a production defect.
The precondition was also unrelated to the contract under test. A poll
that stopped draining would neither crash nor leak, because
cbm_watcher_free frees the same deferred list. So the old test could not
see a broken drain at all.
Add a test-only seam, cbm_watcher_test_pending_free_count, and assert
the stable states the watcher holds: 0 parked before unwatch, 1 after,
0 after the next poll with no index work, and still 0 after a further
poll. No assertion depends on a git probe succeeding. Dirty-poll
reindexing stays covered by watcher_detects_dirty_worktree and
watcher_modify_tracked_file.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
After every publish the pipeline re-exports <root>/.codebase-memory/ (graph.db.zst + artifact.json) whenever an artifact already lives there: once a session indexed with persistence, or when the team committed it and every linked worktree checks it out. The watcher's dirty signature folded those files' (size, mtime) in, so each successful reindex rewrote them, the next poll saw a new dirty state, and the daemon re-triggered itself forever: index.supervisor.reap outcome=clean immediately followed by watcher.changed strategy=git, 100+ times in 15 minutes with index workers pinned (#1953). Nothing under .git moved and no source changed, which is why the reporters blamed their long hyphenated worktree branch names (the #1254 theory its own reporter retracted); the regression test builds that exact worktree and shows the branch is irrelevant.
Scope git status with the same :(exclude).codebase-memory pathspec the exporter's own clean-tree probe already uses for the same reason. Nothing under that directory is ever an index input (discovery skips it), so a change there can never require a reindex; the pathspec is CWD-relative, so a monorepo sub-package watch excludes its own artifact directory. Untracked and committed artifacts both looped before; a real edit and a real HEAD move are still detected.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Documentation and claim accuracy only — no behavioural change. Two of the
three findings are corrections to inaccurate statements the round-2 commit
(61ccd905) introduced.
The gating test's framing was overstated, and this time the overstatement was
measured rather than argued. Replacing the backoff call with the pre-change
`ctx->now + interval_ms * US_PER_MS` leaves the watcher suite green at 82/82:
the index_backoff_* tests pass because they exercise the pure function
directly, which the reverted scheduler no longer calls, and the streak tests
pass because the counter is untouched. So no test in this suite fails when
this change's behaviour is removed. The test now records that, with the
figure, instead of claiming it demonstrates the backoff. It is a regression
guard on a deadline being assigned at all — real, but narrower than round 2
said.
Closing that gap needs either a controllable clock or a further accessor
exposing next_poll_ns. Both were judged out of scope: a third exported symbol
purely for testing, on a change already carrying two, is the API widening
earlier rounds flagged. The gap is recorded in the test, the PR description
and the round-3 note rather than papered over.
The memory-visibility comment claimed index_failures follows "the same
discipline every other per-project field in this struct already follows".
That is false: active_git is serialized by projects_lock and registered is an
atomic_bool. Narrowed to the poll-mutated fields, which are now enumerated.
The round-2 decision to document rather than lock still stands — the panel
verified the underlying premise — but it was resting on an overbroad claim.
watcher.h said the delay "doubles up to a fixed ceiling", which skips the
shift cap: the delay plateaus at interval_ms << INDEX_FAIL_SHIFT_MAX, and
that reaches the ceiling only for interval_ms >= 4688 ms. Every interval this
watcher generates is >= POLL_BASE_MS so the ceiling is always reached in
practice, but the function is exported and a caller may pass less.
Refs #2015
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbrM52pmB1m2SR1vRRoNnf
Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
Retracts an incorrect justification from the round-1 commit (4fc5999a) and
closes the coverage it was used to excuse.
MAJOR — the delay wire-up had no coverage, and the reason given for that was
wrong. Round 1 claimed the gating half was "structurally unobservable without
an injectable clock". It is not. cbm_watcher_touch zeroes next_poll_ns, so a
wire-up that never sets a deadline leaves it at zero and the callback fires on
every poll: simply NOT calling touch between polls observes gating
deterministically, no clock involved. Only the delay's MAGNITUDE needs one.
watcher_index_failure_backoff_gates_repolling_issue2015 covers it — one hard
failure, then five polls without touch asserting no re-fork, then a touch
proving the retry still gets through (so the test cannot pass by observing a
dead watcher). Mutation-verified: deleting the wire-up yields
"failing_index_calls == 6, expected 1".
MINOR — round 1's F1 was half-fixed. Of the two single-token mutations that
finding named, only the streak reset was covered; widening the
sustained-failure "==" to ">=" still passed. watcher_sustained_failure_logs_
once_issue2015 drives 14 consecutive failures through a cbm_log_set_sink and
asserts exactly one emission. Mutation-verified: ">=" yields
"sustained_log_hits == 5, expected 1".
All three mutations F1 named are now demonstrated caught rather than argued
about: reset removal (round 1), wire-up deletion, and the "==" widening.
MINOR — cbm_watcher_index_failure_count reads index_failures under
projects_lock while poll_project writes it lock-free from a state snapshot.
That is the discipline every other per-project field here already follows, so
this documents the visibility contract rather than making one field atomic in
isolation: the accessor is a diagnostic and test seam, not a synchronisation
point, and must not carry scheduling decisions without atomic accessors first.
MINOR — watcher.h still described the pre-F3 backoff contract. It now states
the clamp is a floor as well as a cap, why the floor exists for an exported
function whose reachable inputs cannot currently hit it, and that negative
inputs are treated as zero.
Still uncovered, now stated precisely rather than hand-waved: the delay's
MAGNITUDE. Nothing asserts that ten failures produce a five-minute delay
rather than a five-second one. Gating is proven; the numbers are not, and
that does need a controllable clock.
Refs #2015
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbrM52pmB1m2SR1vRRoNnf
Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
F1 (major) — the failure state machine the backoff feeds had no test
coverage. Deleting the streak reset, or widening the sustained-failure
comparison, left all 7788 tests green: the index_backoff_* tests cover the
delay arithmetic, and nothing covered the counter that selects which delay
applies. A streak that never increments silently degrades the backoff to the
unbounded retry it exists to prevent.
Adds cbm_watcher_index_failure_count(), alongside the existing
cbm_watcher_watch_count() in watcher.h's "Introspection (for testing)"
section, and a test that drives a real git repo through fail/fail/succeed on
the failing_index_callback seam already used by the #937 test, asserting the
streak goes 0 -> 1 -> 2 -> 0. Verified by mutation: removing the reset turns
it red at the reset assertion while every index_backoff_* test stays green.
The delay-GATING half stays uncovered on purpose. Every integration test
calls cbm_watcher_touch first, which zeroes next_poll_ns, so whether the
scheduler honours the computed deadline is unobservable without an
injectable clock the watcher does not have. That harness is a larger change
than this fix.
F3 (minor) — cbm_watcher_index_backoff_ms was non-monotonic for an interval
above the ceiling: zero failures returned the interval, one failure returned
the smaller ceiling. Unreachable today (POLL_MAX_MS < the ceiling), but the
function is exported, so the clamp is now a floor as well as a cap and
backing off can never schedule sooner than the project's own cadence.
F2 (minor) needed no code change: a busy-skip following an unresolved
failure streak keeps the backed-off delay, which is deliberate — only a
success clears the streak, and treating rc > 0 as recovery would let a
project alternating fail/busy never back off at all. The PR description's
over-strong "busy-skip is behaviourally unchanged" claim is corrected there.
Refs #2015
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbrM52pmB1m2SR1vRRoNnf
Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
A hard index failure (index_fn < 0) left the retry cadence untouched, so a
persistently failing project re-forked an index worker every poll for as
long as the daemon lived. Observed on 0.10.8/macOS: 2233 consecutive failed
workers over 4h43m, ~8/min, every one dying at the same worker-side
coordination seal. Read queries stayed healthy throughout, so nothing
surfaced to the user while no project could be indexed at all.
#937 deliberately leaves the baseline uncommitted on a failed reindex so the
change is retried rather than lost. That guarantee is kept; what changes is
the cadence. Each consecutive hard failure doubles the delay, capped at five
minutes, taking a permanently failing project from ~480 attempts/hour to
~12 while still recovering on its own once the cause clears. Busy-skip
(rc > 0) and success paths are untouched.
The delay arithmetic is a pure exported helper so it is unit-testable
without a clock. watcher.index.err now carries rc and the consecutive-
failure count, and a distinct watcher.index.sustained_failure names a
project whose failures are clearly not transient.
Note: itoa_buf returns a single shared per-thread buffer, so the two-value
log line uses local buffers rather than two itoa_buf calls.
Refs #2015
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FbrM52pmB1m2SR1vRRoNnf
Signed-off-by: Shane McCarron <shane.mccarron@corvexconnect.com>
Three related defects in how the watcher decides what counts as a change.
1. `git rev-parse --git-dir` walks UP the tree, so an ordinary folder that
merely sits under an unrelated repository answered yes. The watcher then
inherited that ancestor's dirty state -- permanently non-empty and about
entirely different files -- and reindexed on every poll forever. A
directory is now git-managed only if it carries its own .git (directory
or gitlink file) or the ancestor actually tracks something inside it, so
a real monorepo sub-package still qualifies while a scratch or ignored
folder does not.
2. `git status` reports the whole repository regardless of -C, so every
package in a monorepo reindexed whenever any sibling was edited. The
status call is now scoped with a `-- .` pathspec.
3. Porcelain paths are REPOSITORY-relative but were stat'ed against
root_path. For a project watched at the repository root the two coincide
and the bug is invisible; for a subdirectory project every stat missed,
silently degrading the dirty signature to text-only and losing the
size/mtime component that makes an edit to an already-dirty file
detectable. Paths now resolve through `rev-parse --show-cdup`.
--show-cdup rather than --show-toplevel because MSYS/Cygwin git returns a
translated absolute path from the latter that will not join onto the
native path we hold; a relative hop composes on every platform.
Distilled from #1001 by bethzyy, whose diagnosis identified all three.
Main independently fixed that PR's two headline problems (the unbounded
non-git file count, and per-poll churn via a dirty-state signature) while
it waited, and the watcher was rewritten onto a supervised argv-spawn
harness in between, so this is a reimplementation against the new code
rather than a rebase of theirs.
Both new tests are verified binding: reverting the classification guard
reddens the nested-directory test, and removing the pathspec reddens the
monorepo one, each without disturbing the other.
Addresses the remaining parts of #713, #841 and #937.
Co-authored-by: bethzyy <bethzyy@users.noreply.github.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The blocked-git child's PID-marker wait was a fixed 5s. On the 4-vCPU
windows-11-arm release runners that budget was missed twice in a row
(run 30300256783, attempts 1+2: test_watcher.c:1038 ASSERT(marker_ready))
while every faster leg stayed green — spawn latency under CPU starvation,
not a watcher defect. Raise the four spawn-marker waits to a shared 20s
budget (WATCHER_TEST_SPAWN_MARKER_BUDGET_MS). The wait stays bounded: a
genuinely wedged spawn still fails, at 20s instead of 5s. Termination,
completion, and join budgets are untouched.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Daemon lifecycle and Windows correctness, verified on a real Windows 11
ARM64 VM through the maintained test-infrastructure/vm drivers, plus the
macOS and Linux arm64 suites and the container lint gate.
Daemon lifecycle:
- daemon start/stop/status subcommands. `daemon start` launches a
PERMANENT daemon (spawn shape is byte-exact argv; survives idle
periods and session ends) and reports an already-active daemon
instead of failing. Permanence is honored at every stop latch:
last-committed-client disconnect, host initial-client window,
coordinator release, and application final-session close — a
permanent daemon also keeps admitting new sessions after its last
one closes.
- daemon stop refuses while sessions are active and lists the blocking
peers (pid/role) that must finish first; an idle daemon drains
through the activation-shutdown machinery with the ACK ordered after
connection interrupts. A second stop is idempotent. The wire ops are
no-cohort first-frame requests with peer fingerprint authentication,
so stop/status never conflict with an exact-build admission gate.
- One-shot CLI commands now execute through the daemon (index workers
keep their local supervised path). A cold CLI run that had to spawn a
temporary daemon prints a hint that `daemon start` removes the
per-command startup tax; a warm daemon is recycled silently.
- Hooks are connect-only fail-open: with no daemon present the hook
emits a visible, rate-limited notice (Claude-dialect systemMessage
plus stderr for other dialects) and always exits 0 — augmentation is
never allowed to block the caller's tool use.
- Version skew: a newer-build client automatically drains an
older-build permanent daemon (strict semantic-version triples only;
dev builds never auto-drain) and the build-conflict message names
`cbm daemon stop` as the manual escape hatch.
Windows IPC/runtime (real-VM verified):
- ipc(win): persistent pending overlapped ConnectNamedPipe. The accept
path used to destroy its listening pipe instance on every 20 ms poll
timeout; a client attaching in the teardown window was severed or left
on an orphaned pipe object whose HELLO no server handle could ever
read, absorbing the connect until the client's own timeout expired.
The pending connect now survives poll timeouts and nothing is
destroyed while a client could be attaching.
- ipc(win): drain-before-close for final responses. Closing a named-pipe
server handle can discard a just-sent response before the peer reads
it (POSIX stream sockets never lose buffered data on close). A bounded
cbm_daemon_ipc_connection_drain (read-until-peer-EOF; no-op on POSIX,
immediate on interrupted connections) now precedes close in
runtime_worker_finish and runtime_reject_inline, so hello-conflict,
capacity and disconnect acknowledgements reliably reach the peer.
- runtime: CLOSE_INTENT wire frame. A Windows named-pipe client has no
transport half-close, so close_begin now announces departure with an
explicit frame (ordered after APPLICATION_CANCEL, before the local
interrupt); the server releases the client's admission on receipt
instead of waiting for the handle to close. Admission-drop timing is
now identical to POSIX shutdown() semantics on every platform.
- runtime(win): client close cancellation. close_begin serializes with
request publication under the send lock, best-effort sends the active
token's APPLICATION_CANCEL frame, then interrupts local I/O; the
server cancels MCP/subprocess work promptly. Contract tests accept
both correct outcomes (interrupted transport or decoded CANCELLED).
- runtime: activation acknowledgement ordering. The activation ACK is
the requester's license to act on "snapshotted and draining", so every
connection interrupt is now initiated before the ACK is sent; a
session could previously get one more request serviced after the
requester observed the ACK.
- service(win): deadline-bounded private-file prepare. The conflict-log
prepare retry loop (100 x Sleep(2), which rounds up to the ~16 ms
timer granularity) burned ~1.6 s against permanently obstructed paths,
stalling hello rejections past the client's timeout. The retry budget
is now a 250 ms deadline; transient share collisions still retry.
- subprocess(win): cmd.exe /C payload encoder quotes metacharacters
correctly (root cause of the git-on-Windows failure cluster).
- watcher: SHA-256 buffer sizing (CBM_SZ_64 -> CBM_SZ_128) and a native
Windows stop/unwatch cancellation test with exact-image verification.
- httpd: send_all writes in bounded 64 KiB slices. A single giant
nonblocking send() on Windows is absorbed wholesale into AFD kernel
buffering regardless of SO_SNDBUF, so send deadlines and interrupts
could never engage against a slow peer (and the full payload was
pinned in nonpaged pool). Slicing restores a deterministic
backpressure point; a test hook pins SO_SNDBUF for the deadline and
interrupt tests.
- ui/http: shutdown lifecycle — interrupt checks, response-wide send
deadline, explicit connection states, refusal to free a server while
a listener-owned connection is active.
Windows long-path support:
- Central path-aware wide conversion (canonicalize via GetFullPathNameW
and prepend the extended-length prefix for absolute paths >=240) at
the compat chokepoints (cbm_fopen/compat_fs/mkstemp/mkdtemp), sqlite
store opens, and the daemon build-fingerprint/log paths. Deep managed
installs (a 64-hex generation directory routinely exceeds MAX_PATH)
now index, stage and activate correctly.
- activation transaction: its own file APIs and the component-walking
ancestry validators now operate in the extended-length namespace;
the launcher path is canonicalized (and prefixed when deep) once at
entry so every downstream exact-string comparison stays
form-consistent.
- Executable self-resolution uses the wide APIs (GetModuleFileNameW,
GetFileAttributesW) so non-ASCII install paths survive argv[0]
resolution.
Windows launcher install/uninstall transaction:
- FileRenameInfoEx names are NUL-terminated in an over-allocated
buffer. FileNameLength governs per the contract, but filter drivers
read FileName as NUL-terminated and appended adjacent heap bytes to
created names — a flaky, garbage-suffixed rename target. Both the CLI
and the launcher rename helpers are fixed.
- Uninstall retires state via rename-aside (.cbm ->
.cbm-retired-v1-<tag>-<pid>) with the retired tag shortened to 16 hex
chars so the bare rename target stays under the FileRenameInfoEx
NT-conversion ceiling at guard depths; 64 bits still uniquely
identify the generation.
- When the running launcher's mapped generation backings pin .cbm
against rename, the backings are relocated to activation-<pid>-N
.retired tombstones beside the install (a mapped image may be renamed,
never deleted; the launcher's liveness-guarded sweep reclaims stale
tombstones). Every relocation is recorded, and a FAILED uninstall
reverses the moves after restoring .cbm — via MoveFileExW with
extended-length paths on both arguments, since the deep generation
target is beyond the handle-based rename's bare-path reach — so a
restored install keeps its generation backings and stays runnable.
- After a committed uninstall the retired tree's backings are relocated
out so the tree is shallow enough for the detached cleanup's rd, and
the cleanup's working directory strips the extended-length prefix
(CreateProcessW lpCurrentDirectory silently ignores prefixed paths).
- Files created under Administrators-default-owner directories
(CopyFileW destinations, CREATE_NEW tombstones, probe directories)
are explicitly owner-stamped so the exact-owner validators hold on
runner images; guard fixtures stamp hand-built trees the same way.
Diagnostics, tests and infra:
- diagnostics: discovery is now an always-delivered JSON control record
(new cbm_log_control) that survives CBM_LOG_LEVEL suppression and
paths containing spaces; placement honors $TMPDIR with /tmp fallback
via a diagnostics-local helper; the soak parser reads the JSON record;
documented in docs/CONFIGURATION.md. Red-first coverage for suppressed
log levels, TMPDIR-with-spaces, and native Windows output-contract
assertions.
- tests(win): daemon_ipc/daemon_frontend fixtures now build endpoint
parents with production-shaped ancestry (LocalAppData on Windows, via
th_secure_runtime_parent_new) — the runtime ancestry validation
correctly refuses temp roots whose ancestors grant mutation rights to
Authenticated Users (C:/msys64/tmp, GitHub-runner work dirs) — and
drive the documented startup-owner publication flow before reading
generation-bound endpoint addresses. This turns the 26 Windows
failures previously visible in CI's full-test job green without
weakening any validation.
- tests(win): the launcher guard covers the full permanent-launcher
contract including failed-uninstall restore and immediate reinstall
after uninstall; new daemon lifecycle and reworked hook-augment
guards run the start/recycle/stop flow end to end.
- tests: CBM_SKIP_PERF is now actually consumed by the test runner
(it was set by CI but never read, so perf suites ran everywhere);
four throughput/bench suites are classified as perf, the heavy
store_arch suite moved to the slow-timeout tier, and two
wall-clock-sensitive assertions were rewritten as invariant checks
with coarse hang-detector backstops.
- build/test infra: build-dir safety contract, UI dev-proxy security
contract, soak daemon-recovery contract, path-safety helper, the
Windows VM worktree-sync contract wired into scripts/test.sh, and
vm/win.sh guards building its clean embedded-UI product in an
isolated BUILD_DIR so it cannot clobber the incremental test build.
provision-windows.sh now installs Node.js for the guards UI build.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A persistently dirty or untracked worktree re-triggered a full reindex
(and its DB/artifact rewrite) on every poll cycle - git_is_dirty()
answered 'is the tree dirty' where the watcher needs 'did the tree
CHANGE'. Idle repos with one uncommitted file produced sustained disk
write amplification (reported at 1 TB/day across several repos).
The watcher now tracks a dirty-state signature: FNV-1a over the
entries of git status --porcelain -uall -z plus each listed path's
(size, mtime_ns). Same signature -> no reindex; editing a dirty file,
adding/removing one, or reverting the tree to clean each produce a new
signature and trigger exactly one reindex. Submodule status is folded
in on POSIX, mirroring the old submodule dirty probe. 0 is reserved
for a clean tree; a dirty-at-baseline tree still reindexes once (the
watcher cannot know whether that state reached the DB, e.g. after a
server restart with a stale artifact).
Baselines (HEAD and signature) are now committed only after a
SUCCESSFUL reindex: check_changes stages observations in pending_*
fields, and busy-skips (index_fn > 0, e.g. pipeline lock held) or
failed runs leave the baselines untouched so the change is retried on
the next poll instead of being recorded as seen and silently lost -
the old code stored the new HEAD at check time, dropping any commit
that arrived while the pipeline was busy.
Closes#937
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Distilled from #738: the watcher now prunes a watched project whose root
directory has genuinely disappeared - deleting the cached DB (+wal/shm,
validated name, cache-dir-only paths) and removing the watch entry - so
vanished worktrees stop being watched forever (#286).
Hardened beyond the original PR to remove a data-loss hazard (the cached
DB can hold user-authored data such as the ADR, unrecoverable once
deleted):
- Only ENOENT/ENOTDIR stat failures count as missing. Any other failure
(EACCES, EIO, transient mounts, macOS TCC revocation) resets the
streak and logs watcher.root_stat_error with the errno; Windows
(mingw/UCRT) maps not-found to ENOENT so the check holds there.
- Pruning requires BOTH >=3 consecutive missing polls AND a sustained-
absence grace window since the streak's first miss (default 600s,
override via CBM_WATCHER_PRUNE_GRACE_S), tracked with monotonic
cbm_now_ms.
- Root reappearance resets streak + timestamp (watcher.root_restored).
- The pruned entry is released via the deferred-free list poll_once
drains (one freeing model shared with cbm_watcher_unwatch).
Limitation: only currently-watched projects are pruned; stale DBs left
by older sessions are out of scope.
Co-authored-by: pcristin <xxxokzxxx@protonmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The watcher tests built their git repos with POSIX bash via system() (export ...; cd ... && git ...), which the native Windows test binary runs through cmd.exe and cannot execute — so they were SKIP_PLATFORM'd there. Replace the shell with wt_git() (git -C "<dir>" -c user.* ..., no shell metacharacters) plus th_write_file/th_append_file, so all 19 git-setup blocks run identically on POSIX and Windows. Removes the SKIP_PLATFORM.
The watcher tests build their git repo via system() with POSIX shell syntax (export ...; cd ... && git ...), which the native Windows test binary runs through cmd.exe and cannot execute. Skip them as a platform limitation there (they still run on the POSIX legs) instead of failing.
Add scripts/check-no-test-skips.sh (run from lint) which fails the lint phase on any plain SKIP() or direct tf_skip_count manipulation; only SKIP_PLATFORM() (for genuinely platform-specific tests) is tolerated. Add FAIL() and SKIP_PLATFORM() helpers to the test framework and convert the remaining SKIP()/perf-gated skips across the suite into pass-or-fail assertions, so a suite that cannot meet its preconditions reports a red failure instead of a silent skip.
Previous tests ran `cd <tmpdir> && git config user.email test@test && ...`
to set up author info for test commits. If the tmpdir path was ever
interpreted differently (or if the test process inherited a cwd inside
a real git repo), the `git config` call could write to a real repo's
.git/config instead of the temp repo's.
Replace with `export GIT_AUTHOR_NAME=t GIT_AUTHOR_EMAIL=t@t ...` which
are ephemeral env vars that can never leak into persistent config.
The /tmp/ → %TEMP%\ translation produces longer paths (~50 chars)
than the original templates (~25 chars). Changed all 68 test
declarations from char tmpdir[] = "..." (sized to literal) to
char tmpdir[256] with snprintf initialization.
- cbm_mkdtemp: _mktemp + _mkdir on Windows
- cbm_setenv/cbm_unsetenv: _putenv_s on Windows
- cbm_pipe: _pipe on Windows
- Replace bare POSIX calls in 12 test files with compat wrappers
- All test files now include compat.h
Enforce WarningsAsErrors: '*' with zero suppressions of entire check categories.
All non-FP warnings fixed at source; true false positives get per-instance NOLINT
with reasoning. Fixes include: braces around statements (1679), isolate declarations
(75), explicit casts for multilevel pointer conversions, missing includes, function
parameter documentation via NOLINT, and proper NOLINTBEGIN/NOLINTEND nesting.
Complete C implementation of the indexing pipeline (src/), parallel worker
pool, graph buffer with merge support, SQLite writer, and 1893-test suite.
Linter setup: zero warnings from clang-tidy (all checks enabled, no NOLINT
suppressions), cppcheck, and clang-format. All issues fixed at source —
proper headers for external linkage, GROW_ARRAY macro restructured to
eliminate type parameter, null-deref paths guarded, named intermediates
for suspicious-argument checks.
Martin Vogel