diff --git a/CLAUDE.md b/CLAUDE.md index ee7a841..53a7589 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -36,7 +36,9 @@ Two harness details are load-bearing rather than incidental: **Placeholders are baked at sync time, not read at runtime.** `prompts:` in `techpack.yaml` declares `KB_GATE_MODE`; `hooks/kb-gate.mts` carries `const MODE: string = "__KB_GATE_MODE__"`, substituted during install. Changing the mode means re-running `mcs sync` — there is no runtime setting. The test suite injects modes the same way (string replacement in the temporary installed copy). -**One dispatcher, four hook events.** `hooks/kb-gate.mts` is registered four times in `techpack.yaml` (UserPromptSubmit, PostToolUse, PreToolUse, SubagentStart) and branches on `hook_event_name`. Matchers are broad on purpose; which agent types count as "discovery" is decided in exactly one place, `GATED_AGENTS`. `hooks/sync-memories.mts` is likewise registered twice, on SessionStart and UserPromptSubmit. +**One dispatcher, several registrations.** `hooks/kb-gate.mts` is registered once per event and matcher in `techpack.yaml` and branches on `hook_event_name`, then on `tool_name`. Matchers are broad on purpose; which agent types count as "discovery" is decided in exactly one place, `GATED_AGENTS`, and which skills authorize memory writes, and for how long, in `MEMORY_SKILL_SCOPES`. `hooks/sync-memories.mts` is likewise registered twice, on SessionStart and UserPromptSubmit. + +**Memory writes are gated by skill, per turn.** A file-tool write into `.claude/memories/` is denied unless a memory skill authorizes it. A Bash `rm`, `unlink`, `mv` or `git rm` whose command mentions `memories` is denied unless memory-audit ran, because only the audit deletes, and only after per-batch approval. That check exists for auto mode, where no permission prompt would stop the command. It matches the whole command rather than parsing arguments, since variables, globs and `cd` make parsing easy to evade. The scope is the turn for capture because a session that loaded the skill turns earlier edits from a stale copy and skips its checks. A typed skill command produces no `Skill` tool call, only prompt text, which is why the `UserPromptSubmit` branch scans the prompt as well. This gate ignores `KB_GATE_MODE` except `off`, so its own per-turn denial budget is what keeps a broken `Skill` record from blocking writes forever. **Both hooks fail open.** Unexpected filesystem or payload errors exit 0 without output. The gate never exits 2, never calls qmd, and logs every evaluation except when disabled or when an uninteresting sub-agent starts. Preserve the append-only query and denial files, monotonic turn counter, and separate per-state/per-turn denial budgets. Gate state is compatible with previous shell installs. diff --git a/README.md b/README.md index 7c221b3..e9666b8 100644 --- a/README.md +++ b/README.md @@ -106,6 +106,8 @@ During installation, the pack asks how strictly it should enforce searching the `enforce` cannot wedge a session: denials are budgeted per turn, so a spawn eventually proceeds even if the requirement is never satisfied. Every mode except `off` also briefs discovery sub-agents at startup and records decisions in `.claude/.kb-gate.log`. +Writes to `.claude/memories/` are gated separately, and in every mode except `off` they are denied unless a memory skill is active. That means `continuous-learning` in the same turn, or `memory-audit` earlier in the session. Deleting or moving a memory from the shell is allowed only under `memory-audit`. This keeps every save behind the skill's search and pre-write checks. + To change the mode, run `mcs sync` again. The selection is baked into the installed hook rather than read from a runtime setting. ## What's included @@ -117,7 +119,7 @@ To change the mode, run `mcs sync` again. The selection is baked into the instal | **memory-audit** (skill) | Reviews existing memories and flags stale or duplicate entries | | **sync-memories.mts** (hook) | Indexes memories at session start and re-indexes them when they change | | **memory-loop-activator.sh** (hook) | Reminds Claude to check for knowledge worth capturing after each prompt | -| **kb-gate.mts** (hook) | Keeps knowledge-base lookup ahead of delegated discovery work | +| **kb-gate.mts** (hook) | Keeps knowledge-base lookup ahead of delegated discovery work, and memory writes behind the memory skills | | `autoMemoryEnabled: false` (setting) | Disables Claude Code's built-in memory in favor of this system | ## Upgrading from the Ollama version diff --git a/hooks/kb-gate.mts b/hooks/kb-gate.mts index 4e817fb..88aadba 100755 --- a/hooks/kb-gate.mts +++ b/hooks/kb-gate.mts @@ -1,5 +1,6 @@ #!/usr/bin/env -S node --experimental-strip-types --disable-warning=ExperimentalWarning -import { appendFileSync, existsSync, lstatSync, mkdirSync, readdirSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { appendFileSync, existsSync, lstatSync, mkdirSync, readdirSync, realpathSync, renameSync, rmSync, statSync, writeFileSync } from "node:fs"; +import { basename, dirname, join, resolve } from "node:path"; import { attempt, isDirectory, projectRoot, read } from "./shared.mts"; const MODE: string = "__KB_GATE_MODE__"; @@ -10,6 +11,20 @@ const LOG_KEEP_LINES = 1000; const STATE_MAX_AGE_DAYS = 7; const KB_MARKER = "KB context:"; const GATED_AGENTS = ["Explore", "general-purpose", "Plan"]; +const MEMORY_SKILL_SCOPES: Record = { "continuous-learning": "turn", "memory-audit": "session" }; +const MEMORY_WRITE_TOOLS = ["Write", "Edit", "NotebookEdit"]; +const MAX_MEMORY_WRITE_DENIALS_PER_TURN = 3; +const MEMORY_DELETE = /(?:^|[^\w-])(?:rm|unlink|mv)(?=\s)/; +const SLASH_SKILL = new RegExp(`(?:^|[\\s>])/(?:[\\w-]+:)?(${Object.keys(MEMORY_SKILL_SCOPES).join("|")})\\b`, "g"); + +function memorySkill(name: unknown): string | undefined { + const skill = String(name ?? "").replace(/^\//, "").split(":").at(-1) ?? ""; + return Object.hasOwn(MEMORY_SKILL_SCOPES, skill) ? skill : undefined; +} + +function realPath(path: string): string { + return attempt(() => realpathSync(path)) ?? join(attempt(() => realpathSync(dirname(path))) ?? dirname(path), basename(path)); +} function gate(): void { if (MODE === "off") return; @@ -21,6 +36,14 @@ function gate(): void { const hadKBBlock = (payload.tool_input?.prompt || "").includes(KB_MARKER); if (event === "SubagentStart" && !GATED_AGENTS.includes(agentType)) return; if (!["UserPromptSubmit", "PostToolUse", "PreToolUse", "SubagentStart"].includes(event)) return; + const isSkillCall = event === "PostToolUse" && payload.tool_name === "Skill"; + if (isSkillCall && !memorySkill(payload.tool_input?.skill)) return; + const isMemoryWrite = event === "PreToolUse" && MEMORY_WRITE_TOOLS.includes(payload.tool_name); + const target = payload.tool_input?.file_path || payload.tool_input?.notebook_path || ""; + if (isMemoryWrite && !`${resolve(target)}\n${realPath(resolve(target))}`.includes("memories")) return; + const command = String(payload.tool_input?.command ?? ""); + const isMemoryDelete = event === "PreToolUse" && payload.tool_name === "Bash" && command.includes("memories") && MEMORY_DELETE.test(command); + if (event === "PreToolUse" && payload.tool_name === "Bash" && !isMemoryDelete) return; const root = projectRoot(); const memories = `${root}/.claude/memories`; @@ -29,6 +52,7 @@ function gate(): void { const turnFile = `${state}/${session}.turn`; const queriesFile = `${state}/${session}.queries`; const denialsFile = `${state}/${session}.denials`; + const skillsFile = `${state}/${session}.skills`; const turnText = (attempt(() => read(turnFile)) ?? "").split("\n")[0]?.trim() ?? ""; const turn = /^\d+$/.test(turnText) ? Number(turnText) : 0; @@ -40,6 +64,22 @@ function gate(): void { process.stdout.write(`${JSON.stringify({ hookSpecificOutput: { hookEventName: event, ...fields } })}\n`); } + function rows(file: string): string[][] | undefined { + const content = existsSync(file) ? attempt(() => read(file)) : ""; + return content?.split("\n").filter(Boolean).map(line => line.split("\t")); + } + + function denials(phase: string): string[][] | undefined { + return rows(denialsFile)?.filter(fields => Number(fields[0]) === turn && fields[1] === phase); + } + + function append(file: string, fields: (string | number)[]): void { + attempt(() => { + mkdirSync(state, { recursive: true }); + appendFileSync(file, `${fields.join("\t")}\n`); + }); + } + function sweep(directory: string): void { for (const entry of readdirSync(directory)) { attempt(() => { @@ -65,6 +105,17 @@ function gate(): void { attempt(() => writeFileSync(denialsFile, "")); attempt(() => writeFileSync(turnFile, `${turn + 1}\n`)); logEvent({ decision: "turn_start", turn: turn + 1 }); + for (const [, skill] of String(payload.prompt ?? "").matchAll(SLASH_SKILL)) { + append(skillsFile, [turn + 1, skill!]); + logEvent({ decision: "recorded_skill", skill, source: "prompt" }); + } + return; + } + + if (isSkillCall) { + const skill = memorySkill(payload.tool_input.skill)!; + append(skillsFile, [turn, skill]); + logEvent({ decision: "recorded_skill", skill, source: "tool" }); return; } @@ -102,6 +153,46 @@ mcp__memory-loop__query. return; } + if (isMemoryWrite || isMemoryDelete) { + const phase = isMemoryWrite ? "memory_write" : "memory_delete"; + let subject = command.slice(0, 200); + if (isMemoryWrite) { + const lexical = resolve(root, target); + subject = realPath(lexical); + const memoriesReal = attempt(() => realpathSync(memories)) ?? memories; + const inside = (path: string, directory: string) => path.startsWith(`${directory}/`); + if (!inside(lexical, memories) && !inside(subject, memoriesReal)) return; + } + function logMemory(decision: string, extra = {}): void { + logEvent({ phase, tool: payload.tool_name, target: subject, agent_type: agentType, decision, ...extra }); + } + if (agentId) { + logMemory("deny", { skip_reason: "subagent" }); + output({ permissionDecision: "deny", permissionDecisionReason: "Sub-agents cannot change .claude/memories/. Report the proposed memory change back instead; the main thread applies it through the continuous-learning or memory-audit skill, which run the search, pre-write and approval checks." }); + return; + } + const skill = rows(skillsFile)?.find(([at, name]) => MEMORY_SKILL_SCOPES[name!] === "session" || (isMemoryWrite && MEMORY_SKILL_SCOPES[name!] === "turn" && Number(at) === turn))?.[1]; + if (skill) { + logMemory("allow", { skill }); + return; + } + const spent = denials(phase); + if (spent === undefined) { + logMemory("allow", { skip_reason: "budget_unreadable" }); + return; + } + if (spent.length >= MAX_MEMORY_WRITE_DENIALS_PER_TURN) { + logMemory("allow", { skip_reason: "budget_turn", denials_turn: spent.length }); + return; + } + append(denialsFile, [turn, phase]); + logMemory("deny", { denials_turn: spent.length + 1 }); + output({ permissionDecision: "deny", permissionDecisionReason: isMemoryWrite + ? "Memory files are written only through the continuous-learning or memory-audit skill, and continuous-learning has not been invoked in this turn. Invoke Skill(continuous-learning) now and run its search and pre-write checks before retrying. Working from a copy loaded in an earlier turn is what skips them." + : "Memory files are deleted or moved only by the memory-audit skill, after the user approves the batch. Run Skill(memory-audit) and follow its approval step before retrying; to correct a memory instead, edit it through Skill(continuous-learning)." }); + return; + } + if (agentId) { logEvent({ decision: "skip", skip_reason: "nested_subagent" }); return; @@ -133,20 +224,19 @@ mcp__memory-loop__query. ].join(", and "); if (MODE === "enforce") { - const content = existsSync(denialsFile) ? attempt(() => read(denialsFile)) : ""; - if (content === undefined) { + const discovery = denials("discovery"); + if (discovery === undefined) { logDecision(false, "allow", { skip_reason: "budget_unreadable" }); return; } - const denials = content.split("\n").map(line => line.split("\t")).filter(fields => Number(fields[0]) === turn && fields[1] === "discovery"); - const denialsTurn = denials.length; - const denialsState = denials.filter(fields => (Number.parseFloat(fields[2] ?? "") || 0) === queries).length; + const denialsTurn = discovery.length; + const denialsState = discovery.filter(fields => (Number.parseFloat(fields[2] ?? "") || 0) === queries).length; const spent = denialsTurn >= MAX_DENIALS_PER_TURN ? "budget_turn" : denialsState >= MAX_DENIALS_PER_STATE ? "budget_no_progress" : ""; if (spent) { logDecision(false, "allow", { skip_reason: spent, denials_turn: denialsTurn, denials_state: denialsState }); return; } - attempt(() => appendFileSync(denialsFile, `${turn}\tdiscovery\t${queries}\n`)); + append(denialsFile, [turn, "discovery", queries]); logDecision(false, "deny", { denials_turn: denialsTurn + 1, denials_state: denialsState + 1 }); output({ permissionDecision: "deny", permissionDecisionReason: `Prerequisite missing: ${missing}. Search the KB with mcp__memory-loop__query first, then re-issue this exact call with a "${KB_MARKER}" block (1-5 bullets of findings, or "${KB_MARKER} none relevant.") at the top of the prompt. Spawning several agents at once: write the findings to a scratchpad file and open each prompt with "${KB_MARKER} see " instead of repeating them. Sub-agents cannot see your KB results — unpasted context is rediscovered from scratch.` }); return; diff --git a/techpack.yaml b/techpack.yaml index 29adf66..14eff03 100644 --- a/techpack.yaml +++ b/techpack.yaml @@ -251,11 +251,12 @@ components: destination: sync-memories.mts # ── KB-first delegation ───────────────────────────────────────────────── - # Four registrations of one dispatcher script, which branches on - # `hook_event_name`. Together they keep KB lookups ahead of delegated discovery, - # so sub-agents don't rediscover what a memory already recorded. Matchers stay - # broad on purpose — which agent types count as "discovery" is decided in one - # place, by `GATED_AGENTS` in the script. + # Several registrations of one dispatcher script, which branches on + # `hook_event_name` and then `tool_name`. Together they keep KB lookups ahead of delegated discovery, + # so sub-agents don't rediscover what a memory already recorded, and keep memory + # writes behind the skills that run the save checks. Matchers stay broad on + # purpose — which agent types count as "discovery" is decided in one place, by + # `GATED_AGENTS` in the script. - id: hook-kb-gate-turn displayName: KB gate — turn boundary description: Marks each new prompt so the KB check is required again per turn @@ -308,6 +309,26 @@ components: section: Hooks event: PreToolUse + - id: hook-kb-gate-skill + displayName: KB gate — record memory skills + description: Records continuous-learning and memory-audit invocations so memory writes can be checked against them + hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning + hookEvent: PostToolUse + hookMatcher: "Skill" + hook: + source: hooks/kb-gate.mts + destination: kb-gate.mts + + - id: hook-kb-gate-memory-write + displayName: KB gate — check memory writes + description: Blocks writes, deletes and moves in .claude/memories/ unless a memory skill is active + hookInterpreter: node --experimental-strip-types --disable-warning=ExperimentalWarning + hookEvent: PreToolUse + hookMatcher: "Write|Edit|NotebookEdit|Bash" + hook: + source: hooks/kb-gate.mts + destination: kb-gate.mts + - id: hook-kb-subagent-brief displayName: Sub-agent KB brief description: Tells discovery sub-agents that the KB exists, so they search it instead of sweeping files diff --git a/tests/contracts.test.mts b/tests/contracts.test.mts index 3362a11..ad7c964 100644 --- a/tests/contracts.test.mts +++ b/tests/contracts.test.mts @@ -23,7 +23,7 @@ for (const type of ["commonjs", "module"]) { const f = fixture(t); writeFileSync(`${f.project}/package.json`, JSON.stringify({ type })); const hooks = manifest.split(/^ - id: /m).filter(component => /\n hookEvent:/.test(component)); - assert.equal(hooks.length, 7); + assert.equal(hooks.length, 9); assert.match(manifest, /hook:\n source: hooks\/shared.mts\n destination: shared.mts/); const installed = `${f.project}/.claude/hooks/memory`; mkdirSync(installed, { recursive: true }); @@ -41,6 +41,8 @@ for (const type of ["commonjs", "module"]) { } assert.match(manifest, /hookMatcher: "Agent\|Task"/); assert.match(manifest, /hookMatcher: "mcp__memory-loop__query"/); + assert.match(manifest, /hookMatcher: "Skill"/); + assert.match(manifest, /hookMatcher: "Write\|Edit\|NotebookEdit\|Bash"/); assert.equal((manifest.match(/hookAsync: true/g) ?? []).length, 2); assert.equal((manifest.match(/hookTimeout: 120/g) ?? []).length, 2); }); diff --git a/tests/helpers.mts b/tests/helpers.mts index fe54543..6d0595c 100644 --- a/tests/helpers.mts +++ b/tests/helpers.mts @@ -37,15 +37,20 @@ export function gateFixture(t: TestContext, mode = "enforce") { const state = `${f.project}/.claude/.kb-gate`; const log = `${f.project}/.claude/.kb-gate.log`; const event = (hook_event_name: string, extra = {}) => f.run(script, { hook_event_name, session_id: "session", ...extra }); - const turn = () => event("UserPromptSubmit"); + const turn = (prompt = "do the thing") => event("UserPromptSubmit", { prompt }); const search = (input: unknown = { query: "some topic" }) => event("PostToolUse", { tool_input: input }); const spawn = (prompt = "find the thing", subagent_type = "Explore", agent_id = "") => event("PreToolUse", { agent_id, tool_input: { subagent_type, prompt } }); + const skill = (name = "continuous-learning") => event("PostToolUse", { tool_name: "Skill", tool_input: { skill: name } }); + const write = ({ file_path = `${f.project}/.claude/memories/learning_x.md`, agent_id = "", tool_name = "Write" } = {}) => event("PreToolUse", { tool_name, agent_id, tool_input: { file_path, content: "x" } }); + const bash = (command: string, agent_id = "") => event("PreToolUse", { tool_name: "Bash", agent_id, tool_input: { command } }); const lastLog = () => JSON.parse(readFileSync(log, "utf8").trim().split("\n").at(-1)!); - return { ...f, script, state, log, event, turn, search, spawn, lastLog }; + return { ...f, script, state, log, event, turn, search, spawn, skill, write, bash, lastLog }; } -export function denied(output: string): void { - assert.equal(JSON.parse(output).hookSpecificOutput.permissionDecision, "deny"); +export function denied(output: string): { permissionDecisionReason: string } { + const decision = JSON.parse(output).hookSpecificOutput; + assert.equal(decision.permissionDecision, "deny"); + return decision; } export function qmdStub(work: string): string { diff --git a/tests/kb-gate.test.mts b/tests/kb-gate.test.mts index 5d7558c..581f04e 100644 --- a/tests/kb-gate.test.mts +++ b/tests/kb-gate.test.mts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import { existsSync, readFileSync, readdirSync, rmSync, statSync, utimesSync, writeFileSync } from "node:fs"; +import { existsSync, mkdirSync, readFileSync, readdirSync, rmSync, statSync, symlinkSync, utimesSync, writeFileSync } from "node:fs"; import test from "node:test"; import { denied, gateFixture, root } from "./helpers.mts"; @@ -157,3 +157,125 @@ test("an unavailable state directory fails open", t => { assert.equal(f.turn(), ""); assert.equal(f.search(), ""); }); + +test("memory writes need continuous-learning in the same turn", t => { + const f = gateFixture(t); + f.turn(); + denied(f.write()); + assert.equal(f.lastLog().phase, "memory_write"); + f.skill(); + assert.equal(f.write(), ""); + assert.equal(f.write({ tool_name: "Edit" }), ""); + assert.equal(f.lastLog().decision, "allow"); + f.turn(); + assert.match(denied(f.write()).permissionDecisionReason, /Invoke Skill\(continuous-learning\)/); +}); + +test("memory-audit authorizes writes for the rest of the session", t => { + const f = gateFixture(t); + f.turn(); + f.skill("memory-audit"); + f.turn("approved"); + f.turn("apply batch 2"); + assert.equal(f.write(), ""); + assert.equal(f.lastLog().skill, "memory-audit"); +}); + +test("slash commands anywhere in the prompt count; paths and other skills do not", t => { + const f = gateFixture(t); + f.turn("/continuous-learning"); + assert.equal(f.write(), ""); + f.turn("fix the parser, then /memory:continuous-learning please"); + assert.equal(f.write(), ""); + f.turn("continuous-learning\n/continuous-learning"); + assert.equal(f.write(), ""); + f.turn("read skills/continuous-learning/SKILL.md"); + denied(f.write()); + f.turn(); + f.skill("simplify"); + denied(f.write()); + f.skill("memory:continuous-learning"); + assert.equal(f.write(), ""); +}); + +test("writes through a symlinked memories dir are gated; outside paths pass silently", t => { + const f = gateFixture(t); + rmSync(`${f.project}/.claude/memories`, { recursive: true }); + mkdirSync(`${f.work}/repo/memories`, { recursive: true }); + symlinkSync(`${f.work}/repo/memories`, `${f.project}/.claude/memories`); + f.turn(); + denied(f.write({ file_path: `${f.work}/repo/memories/decision_y.md` })); + denied(f.write({ file_path: `${f.project}/.claude/memories/decision_y.md`, tool_name: "NotebookEdit" })); + const before = readFileSync(f.log, "utf8"); + assert.equal(f.write({ file_path: `${f.project}/src/app.ts` }), ""); + assert.equal(f.write({ file_path: `${f.project}/.claude/memories-notes.md` }), ""); + assert.equal(readFileSync(f.log, "utf8"), before); +}); + +test("sub-agents are always denied memory writes", t => { + const f = gateFixture(t); + f.turn(); + f.skill("memory-audit"); + for (let i = 0; i < 5; i++) denied(f.write({ agent_id: "agent-7" })); + assert.equal(f.lastLog().skip_reason, "subagent"); +}); + +test("the memory-write budget releases after three denials, independent of discovery", t => { + const f = gateFixture(t); + f.turn(); + denied(f.spawn()); + for (let i = 0; i < 3; i++) denied(f.write()); + assert.equal(f.write(), ""); + assert.equal(f.lastLog().skip_reason, "budget_turn"); + denied(f.spawn()); + f.turn(); + denied(f.write()); +}); + +for (const mode of ["warn", "observe", "__KB_GATE_MODE__"]) { + test(`${mode} still denies memory writes`, t => { + const f = gateFixture(t, mode); + f.turn(); + denied(f.write()); + }); +} + +test("off skips memory writes too", t => { + const f = gateFixture(t, "off"); + f.turn(); + assert.equal(f.write(), ""); + assert.equal(existsSync(f.state), false); +}); + +test("deleting or moving memories through Bash needs memory-audit", t => { + const f = gateFixture(t); + f.turn(); + f.skill(); + assert.match(denied(f.bash("rm .claude/memories/learning_x.md")).permissionDecisionReason, /memory-audit/); + assert.equal(f.lastLog().phase, "memory_delete"); + denied(f.bash("git -C .claude/.memories-repo rm -- memories/learning_x.md")); + denied(f.bash("mv .claude/memories/a.md .claude/memories/b.md")); + f.turn(); + f.skill("memory-audit"); + f.turn("approved"); + assert.equal(f.bash("rm .claude/memories/learning_x.md"), ""); + assert.equal(f.lastLog().skill, "memory-audit"); +}); + +test("unrelated Bash commands pass without evaluation", t => { + const f = gateFixture(t); + f.turn(); + const before = readFileSync(f.log, "utf8"); + for (const command of ["ls .claude/memories", "rm build/tmp.txt", "docker run --rm memories-image", "grep -rn memories . | wc -l", "cat .claude/memories/a.md | grep rm"]) { + assert.equal(f.bash(command), ""); + } + assert.equal(readFileSync(f.log, "utf8"), before); +}); + +test("sub-agents cannot delete memories even under memory-audit", t => { + const f = gateFixture(t); + f.turn(); + f.skill("memory-audit"); + denied(f.bash("rm .claude/memories/learning_x.md", "agent-3")); + assert.equal(f.lastLog().skip_reason, "subagent"); +});