Skip to content

Commit da3c14f

Browse files
Your Nameclaude
andcommitted
fix: close 13 real bugs from the architecture audit + split common.rs hotspot
Batch fix for CALM's own maintenance/graph/diff/memory/release subsystems, following a line-by-line verification of a 28-finding external audit against actual source (not docs/comments). Each item below was independently confirmed as a real, reproducing bug before being fixed and covered by a new regression test. Transaction/graph correctness: - txn::advance: close the check-then-act race (BEGIN IMMEDIATE before reading current_state); advance_many now surfaces a failed outer COMMIT instead of silently discarding it (Vec<Result> -> Result<Vec<Result>>). - SCIP overlay: a discarded (stale-generation) ingest pass no longer commits its cache key, which was silently disabling re-runs of that provider. - path tool: route confidence is no longer collapsed into a bare exists:true — routes now carry per-hop confidence and a `certain` flag. - transitive_bfs: a node first seen via an ambiguous (unexpandable) edge can now be promoted to expandable by a later confirmed encounter, instead of being permanently poisoned by a flat visited-set. - coreness: split into `coreness` (confirmed edges only, feeds is_hub/gate) and `possible_coreness` (old undifferentiated behavior, uncertainty-only). Maintenance outbox (biggest item): - run_all_coalesced now reports whether the caller actually led the coalesced pass or deferred to an in-flight one — only the leader may mark the durable job "done", closing a race where a deferred/no-op completion could mark real in-flight work as finished. - mark_completed no longer silently drops a later, genuinely-real completion just because an earlier one already reached a terminal state. - mark_running/reconcile_stale_at_startup now use a real lease (lease_owner/lease_expires_at, columns the schema already reserved) so a second process's startup no longer kills a sibling process's still-live job on a shared project. diff_impact: - A deleted file whose index row already converged away no longer defaults to "low" risk when there's no surviving evidence of its former callers. - Unified-diff parsing now handles Git's C-style quoted paths (spaces, tabs, escaped quotes/backslashes, octal-escaped non-ASCII). - Signature-change comparison is now language-aware: Python/Kotlin keyword arguments and Swift's one-identifier external-label shorthand are no longer stripped as "just a param rename". - The syntax gate now rejects any parse error intersecting the edited region outright, instead of a gameable bare error-count comparison. - reference_impact no longer suppresses independent textual hits in a file that also has an import edge. Memory/session safety: - remember() note upsert and ref replacement now share one transaction — a crash or ref-write failure can no longer leave new content paired with stale/partial refs. - Ambient note surfacing fails closed (not open) when the MAC key is unavailable or the injection scan didn't finish. - HTTP sessions (http.rs) now clean up their active_sessions registry entry on disconnect via an Arc-refcounted guard, closing a leak the unix-socket daemon already had a fix for but HTTP never got. Release/CI hygiene: - action.yml's default `version: latest` now resolves to the exact npm version matching the action's own tag instead of npm's floating latest, closing a real past release-skew incident. - action.yml's shallow base-ref fetch (depth=1) shared no history with the checkout's own depth=1 HEAD, so `git merge-base` failed outright — reproduced live in calm-guard-dogfood CI; fixed with a deeper fetch plus an --unshallow fallback. Documented explicit promotion criteria for taking that job out of shadow mode (not done yet — no clean track record with the fix in place). - Fixed 2 stale KNOWN_LIMITATIONS.md entries already resolved in code. Also: split tools/common.rs (15 methods -> new tools/session_state.rs) to bring its own hotspot_risk score back under the CI gate threshold, and fixed a self-perpetuating state-corruption bug in the project's own calm-nudge.sh dev-tooling hook (unvalidated JSON read before jq --argjson). 1351 unit tests pass (1000 calm-core + 351 calm-server), clippy -D warnings clean across calm-core/calm-server/calm-cli. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent b581b57 commit da3c14f

25 files changed

Lines changed: 2576 additions & 443 deletions

File tree

.claude/hooks/calm-nudge.sh

Lines changed: 33 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,21 @@ save_state() {
266266
# them, silently zeroing the tally.
267267
acquire_state_lock
268268
local prev='{}'
269-
[ -f "$state_file" ] && prev=$(cat "$state_file" 2>/dev/null || echo '{}')
269+
if [ -f "$state_file" ]; then
270+
prev=$(cat "$state_file" 2>/dev/null || echo '{}')
271+
# Self-heal a corrupted/empty state file instead of perpetuating it:
272+
# bash's `>` redirect below truncates $state_file the instant the `jq
273+
# -n` command starts, BEFORE jq has produced any output -- so if that
274+
# jq call ever fails (bad $1/$2/$3 JSON, OOM, killed mid-run) the file
275+
# is left at 0 bytes with nothing written back. Every later
276+
# save_state/bump call then reads that empty file into $prev, which is
277+
# NOT valid JSON, so --argjson/<<< again fails the exact same way —
278+
# once corrupted, permanently corrupted for the rest of the session
279+
# (confirmed live: a real session's state file was still 0 bytes after
280+
# 200+ tool calls). Validating $prev here and falling back to '{}' on
281+
# a parse failure makes the very next successful call recover instead.
282+
jq -e . >/dev/null 2>&1 <<<"$prev" || prev='{}'
283+
fi
270284
jq -n --argjson prev "$prev" --argjson ecf "$1" --argjson nd "$2" --argjson nc "$3" \
271285
'$prev + {edit_context_files: $ecf, needs_diff_impact: $nd, nudge_counts: $nc}' \
272286
>"$state_file" 2>/dev/null || true
@@ -279,7 +293,12 @@ save_state() {
279293
bump() {
280294
acquire_state_lock
281295
local prev='{}'
282-
[ -f "$state_file" ] && prev=$(cat "$state_file" 2>/dev/null || echo '{}')
296+
if [ -f "$state_file" ]; then
297+
prev=$(cat "$state_file" 2>/dev/null || echo '{}')
298+
# Same self-heal as save_state — see its comment for the corruption
299+
# mechanism this guards against.
300+
jq -e . >/dev/null 2>&1 <<<"$prev" || prev='{}'
301+
fi
283302
jq -c --arg k "$1" '.[$k] = ((.[$k] // 0) + 1)' <<<"$prev" >"$state_file" 2>/dev/null || true
284303
release_state_lock
285304
}
@@ -465,6 +484,18 @@ maybe_nudge_session_context() {
465484
acquire_state_lock
466485
local prev
467486
prev=$(cat "$state_file" 2>/dev/null || echo '{}')
487+
# Same self-heal as save_state/bump (see save_state's comment for the
488+
# corruption mechanism): this function is its own independent
489+
# read-modify-write path on $state_file, so it needs its own guard, not
490+
# just save_state's/bump's -- a corrupted/empty $prev here used to make
491+
# `n=$(jq -r ... <<<"$prev")` fail silently (empty $n), which bash's
492+
# arithmetic then read as 0, so `$((n % SESSION_CONTEXT_REMINDER_EVERY))
493+
# -eq 0` was ALWAYS true -- the "possibly_stuck" reminder fired on every
494+
# single call instead of every Nth one, and the write right after it
495+
# re-truncated $state_file to empty again on every call, permanently
496+
# undoing save_state's/bump's own fix moments after either one ran (this
497+
# function runs last in every dispatch branch).
498+
jq -e . >/dev/null 2>&1 <<<"$prev" || prev='{}'
468499
if [ "$tool_name" = "mcp__calm__session_context" ]; then
469500
if [ "$(jq -r '.since_session_context // 0' <<<"$prev")" != "0" ]; then
470501
jq -c '.since_session_context = 0' <<<"$prev" >"$state_file" 2>/dev/null || true

.claude/hooks/test-calm-nudge.sh

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,23 @@ fi
9494
# must still unlock that file for a later native Edit, by falling back to
9595
# a read-only lookup against .calm/index.db mirroring resolve_symbol's own
9696
# "exactly one row named X" criterion. Uses a real symbol from this repo
97-
# (crates/calm-server/src/tools/common.rs::resolve_symbol_candidates) so
97+
# (crates/calm-server/src/tools/common.rs::apply_personalization_boost) so
9898
# the DB lookup has something real to find -- this is the exact regression
9999
# a prior session hit editing crates/calm-cli/src/main.rs (see memory
100-
# calm-two-tooling-bugs-root-cause-2026-07-14).
100+
# calm-two-tooling-bugs-root-cause-2026-07-14). NOTE 2026-08-06: this used
101+
# to reference resolve_symbol_candidates, but that symbol moved to
102+
# outcome.rs in the 2026-07-28 hotspot split and this fixture was never
103+
# updated -- it was silently testing "does the DB fallback correctly
104+
# resolve to wherever the symbol REALLY is" (outcome.rs) while asserting
105+
# the OLD location (common.rs), which fails now that the two disagree.
106+
# apply_personalization_boost is verified to still live in common.rs.
101107
run_hook_symbol_only() {
102108
jq -nc --arg session "$session_id_test" --arg tool "$1" --arg symbol "$2" \
103109
'{session_id: $session, tool_name: $tool, tool_input: {symbol: $symbol}}' \
104110
| bash .claude/hooks/calm-nudge.sh
105111
}
106112
if [ -f .calm/index.db ]; then
107-
run_hook_symbol_only "mcp__calm__edit_context" "resolve_symbol_candidates" >/dev/null
113+
run_hook_symbol_only "mcp__calm__edit_context" "apply_personalization_boost" >/dev/null
108114
out=$(run_hook "Edit" "crates/calm-server/src/tools/common.rs")
109115
if echo "$out" | jq -e '.hookSpecificOutput.permissionDecision == "deny"' >/dev/null 2>&1; then
110116
fail "expected allow for common.rs after edit_context(symbol-only) resolved it via the DB fallback, got deny: $out"

.github/workflows/ci.yml

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,31 @@ jobs:
3636
# end, not just that `cargo test` passes against the Rust source directly.
3737
# continue-on-error: shadow mode while this is new (matches this repo's
3838
# own rollout convention for new gates -- elicit_hub_confirm, verification,
39-
# etc. all default off/observe-only at first) -- promote to blocking once
40-
# it's run clean across enough real PRs.
39+
# etc. all default off/observe-only at first).
40+
#
41+
# Audit 11.4 (2026-08-06): reviewed this job's run history before
42+
# considering promotion. Found two real infra failure modes, not false
43+
# positives from the guard's own risk analysis -- neither is "the gate
44+
# doing its job":
45+
# 1. `error: unrecognized subcommand 'guard'` -- ran against a commit
46+
# from before this job's build-from-source+alias steps existed
47+
# (older npm-install-only version of this job). Self-resolved by
48+
# the current job definition; not expected to recur.
49+
# 2. `fatal: origin/main...HEAD: no merge base` -- action.yml's
50+
# shallow-fetch fix only fetched the base ref at depth=1, sharing no
51+
# history with this checkout's own depth=1 HEAD. Fixed in the same
52+
# commit as this comment (action.yml's "Resolve review scope" step
53+
# now fetches depth=100 + falls back to `--unshallow`).
54+
# Promotion criteria (remove `continue-on-error` once ALL of these hold):
55+
# - 10 consecutive PRs where this job actually ran (not skipped) with
56+
# zero infra-category failures (subcommand/merge-base/install/
57+
# timeout) -- a real "aggregate_risk >= fail-on" block is NOT an
58+
# infra failure and doesn't reset this count.
59+
# - At least one of those 10 was a PR this job correctly blocked or
60+
# would have blocked (fail-on triggered), confirmed by manual review
61+
# to be a genuine high-risk change, not a false positive.
62+
# - No open false-positive report against calm guard's risk model
63+
# older than 7 days.
4164
calm-guard-dogfood:
4265
runs-on: ubuntu-latest
4366
timeout-minutes: 15

KNOWN_LIMITATIONS.md

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,6 @@ genuinely absent: per-IP rate limiting, backoff, or request queueing.
9494
`docs/http-transport.md` is explicit this remains defense-in-depth only;
9595
put a real reverse proxy in front if you need actual rate limiting.
9696

97-
## Shared daemon has one capability ceiling for every connection
98-
99-
A daemon's tool-preset ceiling is fixed at whichever process first
100-
spawned it (`crates/calm-server/src/daemon.rs`); `calm connect --preset`
101-
only takes effect if that connection is the one doing the spawning. Two
102-
MCP clients attached to the same project daemon share the same ceiling —
103-
there's no per-connection handshake negotiating a narrower profile per
104-
client. Each connection *does* get its own session state (`oriented`,
105-
`enabled_toolsets`, `session_log`), just not its own ceiling.
106-
10797
## Malicious/pathological-repo indexing DoS has partial mitigations
10898

10999
`SECURITY.md` still calls resource-exhaustion-via-huge-repo out of scope
@@ -129,22 +119,3 @@ native/GitHub-release path doesn't. Renaming the native binary is a
129119
breaking change for anyone who's already scripted against `calm` and
130120
needs a deliberate decision (and probably a compatibility-alias
131121
transition period), not a silent rename — not done here.
132-
133-
## No Git/CI-native integration path
134-
135-
Everything above mostly assumes an MCP client calling CALM's tools
136-
directly. A first step now exists: `calm guard --project-root .`
137-
(`crates/calm-cli/src/main.rs`) runs the exact same `diff_impact` tool
138-
an MCP agent's own Stage-7 pre-commit gate uses, against the staged diff
139-
(`git diff --cached`) by default, and exits non-zero when the resulting
140-
`aggregate_risk` is at or above `--fail-on` (default `high`) — usable
141-
directly as a pre-commit hook or CI step for a change made outside any
142-
MCP session (a teammate's native editor, a bot PR), which was previously
143-
invisible to CALM entirely. `calm guard --base origin/main`-style
144-
PR-range analysis now also exists (`--base <ref>`, sugar for the
145-
merge-base-relative `<ref>...HEAD` commits range; `--commits <range>`
146-
for raw passthrough when that convention isn't what's wanted) — this
147-
was indeed mostly CLI plumbing onto `diff_impact`'s pre-existing
148-
`commits` param, as this section previously predicted. What's still
149-
missing: no publishable GitHub Action wrapping `calm guard` for
150-
one-line CI adoption.

action.yml

Lines changed: 60 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,13 @@ inputs:
4242
required: false
4343
default: 'false'
4444
version:
45-
description: 'Which @eilodon/calm-mcp npm version to install. Defaults to the latest published release.'
45+
description: >-
46+
Which @eilodon/calm-mcp npm version to install. Defaults to 'latest',
47+
which the install step resolves to the exact npm version matching
48+
THIS action instance's own tag (release.yml publishes the action and
49+
the npm package from the same tag in one coordinated workflow) rather
50+
than npm's floating `latest` dist-tag -- set this explicitly only to
51+
deliberately override that pinning.
4652
required: false
4753
default: 'latest'
4854
skip-install:
@@ -60,11 +66,47 @@ inputs:
6066
runs:
6167
using: 'composite'
6268
steps:
69+
# Audit 11.3 (release-skew hazard): `action.yml`'s implementation comes
70+
# from whatever ref the caller's `uses: eilodon/calm-mcp-action@X`
71+
# checked out, but a bare npm `latest` install is a SEPARATE, floating
72+
# identity that can drift out of sync with it -- a real past incident
73+
# (see .github/workflows/ci.yml's calm-guard-dogfood job comment): this
74+
# action called a subcommand that existed in source but hadn't been
75+
# published to npm yet under `latest`, so CI had to switch to building
76+
# calm-cli from source + skip-install to work around it. Since
77+
# release.yml stamps the crate version from the release tag and
78+
# publishes the matching npm version from that SAME tag in one
79+
# coordinated workflow, `github.action_ref` (the tag THIS action
80+
# instance was invoked at, e.g. "v0.6.0") is exactly the npm version
81+
# guaranteed to match what this copy of action.yml expects -- resolved
82+
# here instead of trusting npm's independently-moving `latest` dist-tag.
83+
- name: Resolve calm version
84+
if: inputs.skip-install != 'true'
85+
id: calm_version
86+
shell: bash
87+
env:
88+
INPUT_VERSION: ${{ inputs.version }}
89+
ACTION_REF: ${{ github.action_ref }}
90+
run: |
91+
set -euo pipefail
92+
version="$INPUT_VERSION"
93+
if [ "$version" = "latest" ]; then
94+
if [[ "$ACTION_REF" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then
95+
version="${ACTION_REF#v}"
96+
else
97+
# Not invoked at a release tag (e.g. a branch ref during this
98+
# action's own development) -- no fixed release point to pin
99+
# to, so fall back to real npm latest rather than guessing.
100+
version="latest"
101+
fi
102+
fi
103+
echo "version=$version" >> "$GITHUB_OUTPUT"
104+
63105
- name: Install calm
64106
if: inputs.skip-install != 'true'
65107
shell: bash
66108
env:
67-
CALM_VERSION: ${{ inputs.version }}
109+
CALM_VERSION: ${{ steps.calm_version.outputs.version }}
68110
run: npm install --global "@eilodon/calm-mcp@$CALM_VERSION"
69111

70112
# All `github.*`/`inputs.*` context values are threaded in via `env:`
@@ -105,8 +147,23 @@ runs:
105147
# so `calm guard --base` would fail to resolve it. Fetch it
106148
# explicitly rather than requiring every caller to remember
107149
# `fetch-depth: 0` in their own checkout step.
150+
#
151+
# Audit 11.4: a plain `--depth=1` fetch of the base ref (the prior
152+
# version of this fix) has NO history shared with this checkout's
153+
# OWN also-depth=1 HEAD -- `calm guard --base` builds a
154+
# merge-base-relative `<base>...HEAD` range, and `git merge-base`
155+
# then fails outright ("fatal: <base>...HEAD: no merge base")
156+
# unless the two single commits happen to coincide. Reproduced for
157+
# real in calm-guard-dogfood CI. `--depth=100` covers the
158+
# overwhelming common case (most branches diverge by a handful of
159+
# commits from their base); the `--unshallow` fallback guarantees a
160+
# merge-base is found for anything that diverged further, at the
161+
# cost of a slower fetch only in that rarer case.
108162
if [ -n "$fetch_ref" ]; then
109-
git fetch --no-tags --depth=1 origin "$fetch_ref" || true
163+
git fetch --no-tags --depth=100 origin "$fetch_ref" || true
164+
if ! git merge-base "origin/$fetch_ref" HEAD >/dev/null 2>&1; then
165+
git fetch --no-tags --unshallow origin || true
166+
fi
110167
fi
111168
112169
echo "commits=$commits" >> "$GITHUB_OUTPUT"

0 commit comments

Comments
 (0)