fix: two self-inflicted regressions, evidence parity in json, and the quarantine verdict tests (v0.10.3) - #1552
Open
DeusData wants to merge 11 commits into
Open
fix: two self-inflicted regressions, evidence parity in json, and the quarantine verdict tests (v0.10.3)#1552DeusData wants to merge 11 commits into
DeusData wants to merge 11 commits into
Conversation
DeusData
force-pushed
the
fix/batch2-regressions-and-determinism
branch
from
August 12, 2026 01:34
8598c7b to
cf6fe28
Compare
Two regressions we shipped ourselves, both with the same shape: a change that was correct in the abstract broke a concrete thing users had working. **`update --ui` became a hard error (#1544).** v0.10.2 removed the ui/standard chooser and deleted the flags that drove it, so a command in scripts, aliases and muscle memory started failing with "unknown update option" where v0.10.1 had printed the install-script handoff. Retiring a CHOICE is fine; retiring the WORDS is a break users get nothing for. Both flags are accepted again, do nothing, and print one note explaining that every build now includes the UI. A genuinely unknown flag is still rejected — the point is compatibility, not silence. Reported by @ehendrix23. **The generated Pi extension could not load (#1550).** Since #1392 the extension has been generated from the tool registry, emitting `export function register(pi)`. Pi loads an extension by calling its DEFAULT export as a factory, so the file failed to load — and a Pi extension that fails to load takes every `pi` command with it, `pi doctor` included, not just cbm's tools. It now default-exports the factory. Reported by @musichen. The existing generator test passed throughout: it asserted WHICH tools the file lists, never that the file Pi loads is loadable. The new test pins the export shape, which is the property that actually matters to the consumer. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…#1037) #1545 fixed the defect that let concurrent instances quarantine each other's healthy databases, but shipped without tests — and this is a data-loss path, so it gets coverage before it ships. Three cases, one per verdict, chosen so a regression in either direction fails: healthy -> OK (the baseline the other two are measured against) bad root_path-> CORRUPT (a verdict that never says CORRUPT would protect broken databases instead of users) no handle -> TRANSIENT (a file we could not open tells us NOTHING about its contents; answering CORRUPT there is how a database nobody had read got renamed and rebuilt) Co-Authored-By: LynxBay <LynxBay@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`trace_path(include_evidence:true)` added `strategy` and `confidence` per hop in the default tree output, and returned `cols: ["name","hop"]` under format:"json" — no error, no warning, just two fields missing from the response shape the schema and `--help` both promise. include_evidence was only ever implemented in bfs_to_tree_table; bfs_to_tree_json never took the flag. So the callers most likely to want the evidence — the ones asking for structured output to consume programmatically — were exactly the ones who silently got none of it. Reported by @asemraza. The json path now emits both columns with the same semantics as the tree path, including the placeholder case: the root hop has no inbound edge and non-CALLS edges record no strategy, so those rows carry nulls rather than dropping fields and leaving callers with ragged rows. Tree says "-", json says null — the same promise in the form each consumer can test. The existing evidence test covered only the tree path, which is why this went unnoticed; it now asserts both formats. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ause `posix_spawnp` and `fork` both fail with EAGAIN when the process table or a per-user limit is momentarily full. Both paths treated that as a hard failure, so a burst of process creation turned into CBM_PROC_SPAWN_FAILED — a git probe, an LSP server, or an index worker refusing to run for a reason the user can neither see nor act on, on a machine that is merely busy. This is the production half of a flake we have been re-running for two days: `subprocess_run_clean` failed spawning `/bin/sh -c "exit 0"` on macOS runners in three separate CI runs, always under a loaded release matrix, always green on a quiet rerun. Chasing it as a test race would have been chasing the symptom — the test is right, and a user on a loaded laptop hits the same edge. EAGAIN/ENOMEM now retry three times over ~30ms; everything else keeps failing immediately, and an exhausted system still fails after the backoff rather than spinning. exec-class errors (ENOENT/EACCES/ENOEXEC/...) are untouched, so the documented "missing binary exits 127, spawn_failed means the mechanism failed" contract still holds. The fork fallback gets the same treatment: it fails for the same reason under the same pressure, and the fallback must not be less robust than the primary path. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
A reporter ran `uninstall`, then could not install again: every attempt said "active CBM sessions and operations could not be stopped safely". A reboot proved no such session existed, and the remedy line v0.10.2 added made it worse — it told them to run `codebase-memory-mcp daemon status`, a binary they had just uninstalled. Both halves were wrong for their case: The guard funneled two unrelated outcomes into one message. A cohort that is BUSY means real sessions hold it and closing something is the fix. A reservation that FAILED — lock I/O, leftover coordination state, permissions — means nothing is running at all, and telling that reader to close sessions sends them after processes that do not exist. Same misattribution class as #1416 and #1535: a non-session failure wearing a session costume. They are now separate messages, and the failure one says plainly that nothing needs to be closed. The remedy also has to survive the situation it prints in. Install, and any retry after uninstall, runs with no cbm on PATH; advice that starts with `codebase-memory-mcp ...` is unusable exactly when it is needed. The busy message now leads with the actionable step (close the editor/agent holding it) and mentions the command only as an aid where a binary still exists; the failure message asks for a cache listing, which needs no cbm at all. A recorded transaction refusal still outranks both generic messages, so the #1416 attribution is unchanged. This does NOT yet explain why their reservation fails with nothing running — that needs their environment, which is being requested on the issue. What it does is stop sending them after the wrong thing while we find out. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
`test-msan` failed the two backpressure cases while every other lane was green, and the same suite passes under MSan locally (12/12). The wait that fired is a LIVENESS BACKSTOP, not a race budget: the daemon child either announces on its ready pipe or it does not, and the test asserts the announcement itself, never a timing window. A backstop only does its job if it never fires on a legitimately slow start. 30s was already the second value here — the first was below the daemon's own 12s cohort-claim budget and SIGKILLed healthy startups on oversubscribed runners. Under MemorySanitizer, with instrumented libc++ and origin tracking, startup runs several times slower than that same budget it has to clear, so 30s stopped being a backstop and started being a verdict. Sanitized builds now get 180s, matching the existing sanitizer-aware pattern in test_cs_lsp_bench.c. This costs nothing when the daemon is healthy: a passing run returns the moment the byte arrives, whatever the ceiling. Nothing about the asserted state changed, and the non-sanitized bound is untouched. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ollow-up) The local Windows leg caught this within hours of merging #1532: on a machine that is not domain-joined, USERDOMAIN is the literal string WORKGROUP, and `WORKGROUP\test` is not a resolvable principal — WARN: build-dir DACL stamp (pre-wave) failed (user=WORKGROUP\test ...): WORKGROUP\test: No mapping between account names and security IDs was done. so the grant fails outright and the tree keeps its inherited `Authenticated Users:(M)`. The guard then correctly reports that cross-account mutation is still possible, and the whole leg stops. Since most Windows machines are not domain-joined, that is the common case, not the exotic one. #1532 was fixing a real bug in the other direction (a bare name resolves against the machine first, so a host named like its user grants to an empty principal). Both failures are name RESOLUTION failures, so this stops resolving names: the account is identified by SID, which is unambiguous everywhere. The SYSTEM and Administrators grants in these same commands have always used that form — the current user was the odd one out. Name lookup survives only as a fallback where PowerShell is unavailable. Applied to all three scripts carrying the stamp (run-tests-parallel, soak-test, memlab), since they share the failure and would otherwise drift apart. Co-Authored-By: Kiborgik <Kiborgik@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/batch2-regressions-and-determinism
branch
from
August 12, 2026 09:57
99ad0b7 to
a654acb
Compare
…rrno (#1537) The reporter came back with the output that named this: exact executable identity could not be verified (cache-private) - /Users/.../.cache/codebase-memory-mcp: ancestry component validation failed (errno 2) Two separate defects, and the second is why nobody could see the first. **The refusal.** The daemon's private-directory ancestry walk rejected any group-write bit on any ancestor — `(mode & 0022) == 0`. That is the identical rule #1535 removed on the activation side, in the sibling gate the consolidated strictness decision covers but that never got changed. A group-writable ~ or ~/.cache is ordinary: WSL2 ships 0775, so do several distro skeletons and any site with a shared primary group. So the daemon refused to run, permanently, with nothing the reader could close or restart. World-writable is still refused (any local user could swap a path component), and the private directory itself is still chmod'd to 0700 and verified after the walk — which is what makes admitting the ancestor safe: the thing that actually holds data stays owner-private either way. Group-writable ancestors are logged, so the weaker check is never silent. Both directions are tested; a fix that admitted everything would "fix" this by deleting the protection. **The invented errno.** When that check failed, the branch set no detail, so the caller fell back to printing `errno` — which nothing on the path sets. The reporter was handed ENOENT for a permission refusal and went looking for a missing file that was there all along; it cost a round trip to find out the number meant nothing. The branch now names the component and the rule. Reported by @listepo, who ran the diagnostics that turned this from a guess into a one-line root cause. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ment Two contributors independently sent the same fix for #1550 while ours was already in CI (#1553 by @musichen, who also reported the bug, and #1564 by @henri-edh). Rather than close both as duplicates, this takes the part of theirs that ours was missing. @henri-edh's comment names the actual mechanism: Pi loads an extension via jiti.import(path, { default: true }) and rejects anything that is not a function, so a named-only export hands the loader the module namespace object and the install fails with "Extension does not export a valid factory function". Ours said "Pi calls the default export" — true, but it does not tell the next reader why a named export cannot simply be added alongside, which is exactly the change someone would try. Their wording is now ours. The assertion is tightened the same way, from `export default function` to `export default function (pi)`, so a default export with the wrong shape cannot satisfy it. Co-Authored-By: musichen <musichen@users.noreply.github.com> Co-Authored-By: henri-edh <henri-edh@users.noreply.github.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
#1554) The installed SKILL.md carried an unquoted `description` whose value contains "Triggers on: ". In YAML a colon-space inside an unquoted scalar is a nested-mapping indicator, so strict readers — js-yaml's `load`, the frontmatter parser in `npx skills` — reject the whole document. The skill does not load wrongly; it does not load at all, and nothing in the install output says why. Quoting the scalar fixes it without touching a word of the text. The test checks the RULE rather than this string: for every skill, any frontmatter value containing ": " must be quoted. Pinning the current wording would let the next skill someone adds reintroduce exactly this bug. Verified by reverting the fix — the test goes red, and green again with it restored. Reported by @xezzon. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
Codex sanitizes the environment of stdio MCP subprocesses and passes through only the names listed in `env_vars`. Our generated block had `command` and `args` and nothing else, so a user with a custom CBM_CACHE_DIR got a Codex-spawned server reading the DEFAULT cache while their account daemon used the configured one. The two disagree during initialization, the connection closes, and Codex exposes no cbm tools at all — a total failure that looks like a broken MCP server rather than a missing environment variable. The name is listed unconditionally rather than only when the variable happens to be set during install: `env_vars` names variables to forward IF PRESENT, so listing it costs nothing when unset, and it keeps working for someone who sets CBM_CACHE_DIR after installing — a case install-time detection would silently miss, producing exactly this bug again with no way to tell. Reported by @anton-matosov, who verified the corrected block end-to-end before filing. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second batch of the post-v0.10.0 fix program. Five community PRs were merged directly to main alongside this branch; what remains here is our own regressions plus the coverage a merged fix needed.
In this branch
update --uihard-errored ("unknown update option") after v0.10.2 removed the variant flags — a command people had in scriptsregister(pi); Pi wants a default-exported factory, so the file broke everypicommand (pi doctorincluded)trace_path(include_evidence)returnedcols:["name","hop"]underformat:"json"— implemented on the tree path onlystrategy/confidencewith the same semantics (nulls where the tree prints-); the test now asserts both formatsBoth regressions share a shape worth naming: the existing tests passed throughout. The Pi generator test asserted which tools the file listed, never that the file Pi loads is loadable; the evidence test covered the tree path only.
Merged to main in the same batch
#1545 @LynxBay (data-loss: healthy DBs quarantined on lock contention, fixes #1206+#1037) · #1471 + #1470 @Studnicky (edge-property merge made a total order; candidate sort on content keys — two concrete sources of the MT edge-jitter tracked since the RC) · #1531 @Kiborgik (inheritable DACL, fixes #1351 empty-DACL children) · #1532 @Kiborgik (USERDOMAIN-qualified ACL grants).
Deferred to the next batch, with analysis
#1546 (C/C++ CRLF + backslash continuation swallows the rest of the file) needs either a vendored-grammar patch or a pre-parse normalization layer whose byte-offset mapping is handled deliberately. #1548 (Python/JS defs nested inside a function never extracted) wants nested defs as first-class nodes carrying enclosing scope, not a special case. Both reporters told.
Verification
Local suites green with the account daemon stopped: cli 263/0, and mcp + store_nodes + agent_clients + graph_buffer + configlink 351/0. Full 3-OS ladder running. (Note for the record: a live permanent daemon fails 24 agent-config install/uninstall tests — the tests are refused by the daemon, not broken; recorded so it is not re-diagnosed.)
Ships as v0.10.3.