diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 0000000..c89ccd9 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,20 @@ +{ + "$schema": "https://json.schemastore.org/claude-code-settings.json", + "permissions": { + "allow": [ + "Bash(./gradlew build*)", + "Bash(./gradlew check*)", + "Bash(./gradlew test)", + "Bash(./gradlew test -Drabosh.memory.seed=*)", + "Bash(./gradlew test -Drabosh.memory.iterations=*)", + "Bash(./gradlew crashDemo*)", + "Bash(./gradlew publishToMavenLocal*)", + "Bash(./gradlew tasks*)", + "Bash(./gradlew --stop*)" + ], + "ask": [ + "Bash(./gradlew test -Prabosh.memory.smoke*)", + "Bash(./gradlew bundleForCentral*)" + ] + } +} diff --git a/CHANGELOG.md b/CHANGELOG.md index bc64405..dd04674 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,59 @@ under [CONTRACT.md](CONTRACT.md), and a change to one is a breaking change howev ## [Unreleased] +### Added + +- **`checkDeleteDoesNotCompact`, a build task for the one design rule nothing was watching.** `delete` + writes its batch and stops; reclaiming tombstones belongs to `expireBefore`, which pins a snapshot, + decides the set, deletes and only then compacts. A breach of that would make a command the model + issues mid-conversation unpredictably slow — latency rather than a wrong answer, which is precisely + what no assertion about behaviour can see. "Delete should free space" is also the intuitive + position, and the correct code for it lives in the same file as the code that must not do it. The + task fails `check` if `compact()` appears anywhere but `expireBefore`, and is narrow on purpose: one + that also forbade the correct call would be worse than none. + +- **How to pair the handler with context editing, in the README.** `clear_tool_uses_20250919` + alongside the memory tool is the configuration this store is built for, and enabling it is a change + to the message parameters rather than to the handler. Documented with the three consequences that + are specific to this implementation: the pre-clearing flush is bursty and will meet + `createOverwrites = false` head-on, `memory` does not belong in `exclude_tools` because its results + are the cheapest thing in a transcript to discard, and the burst is still one writing thread under + the same `maxMemoryBytes` cap. + +- **The manual tool-use loop, written out rather than referred to.** The README has always named + driving the loop yourself as the escape hatch for `is_error` fidelity, and never showed it. It now + carries the dispatch — a `when` over `command` onto the six methods, one `ToolResultBlockParam` + each — together with what keying `is_error` off the `Error: ` prefix does *not* catch, since + `view`'s missing-path string deliberately has no prefix. + +- **`-Drabosh.memory.smoke.model`.** `LiveSmokeTest` still defaults to the cheapest model that can + drive the tool, which is right for a canary whose subject is the SDK rather than anyone's + reasoning. A release that wants one run against the model the README recommends can now pass the + dial instead of editing the file. + +- **A project `.claude/settings.json`** allowlisting the `./gradlew` invocations this repository + documents. The live smoke test and `bundleForCentral` are deliberately *not* on that list — one + spends money and the other is the last step before an irreversible upload, and neither should + become frictionless by default. + +No behaviour changed and no public signature moved. + +### Changed + +- **The README now says which half of the tool is beta.** The tool itself is generally available; + what lives in the SDK's beta namespace is the *helper* surface this module implements — + `BetaMemoryToolHandler`, the runner, and the beta `MessageCreateParams` they need. A reader + looking at the example could reasonably have concluded that adopting this handler meant adopting a + beta API, and the non-beta `MemoryTool20250818` declaration is now named beside the manual loop + that can use it. + +- **A `.png` path is documented as a text memory**, under Deliberate divergences. Claude's tool + description promises that `view` renders image files; `create` takes `file_text`, so there are + none here and the model reads back what it wrote, with line numbers. + +- **JUnit 6.1.2 → 6.1.3.** Test-only, and inside the confirmation the catalogue already records — + which is of the dependency, not of a patch number. + ### Fixed - **The listing-index figures published in 0.1.1 were measured before the JIT had finished, and are diff --git a/CLAUDE.md b/CLAUDE.md index aa9f96e..9006e49 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -40,8 +40,9 @@ range and never a snapshot, because there is no shared CI matrix to catch skew. - **`app.oreshkov:rabosh-api`** at an exact version, pinned at `0.3.0`. - JetBrains and Kotlin libraries are pre-approved. **Everything else requires explicit confirmation from the user before it is written into `gradle/libs.versions.toml`**, with the trade against - writing it by hand stated first. JUnit `6.1.2` was confirmed on 2026-08-16, test-only, against a - hand-rolled harness; that confirmation covers JUnit and nothing else. + writing it by hand stated first. JUnit was confirmed on 2026-08-16, test-only, against a + hand-rolled harness; that confirmation covers JUnit and nothing else, and covers the dependency + rather than a patch number — a 6.1.x bump does not need a fresh one. ## Toolchain @@ -112,8 +113,10 @@ breach; where nothing catches it, that is said too. mixture, and that is the property worth pointing at. Caught by `CrashSafetyTest`, which asserts the child was still alive when it was killed so the instrument cannot pass vacuously. - `delete` does not call `compact()`. Tombstones are the retention job's business, not the - interactive command's. **Nothing catches a breach of this** — it would show up as latency, not as - a failure. + interactive command's, and a breach shows up as latency rather than as a wrong answer — so no + assertion about behaviour can see it. Caught by the `checkDeleteDoesNotCompact` build task, which + `check` depends on: `compact()` is legitimate inside `expireBefore` and nowhere else. The check is + narrow on purpose, because one that also forbade the correct call would be worse than none. - `scope` is a key prefix, not a security boundary. One directory per end user when the threat model needs one. `README.md` and `CONTRACT.md` both say so; `DifferentialMemoryTest` asserts only that two scopes cannot see each other, which is isolation and not security. diff --git a/README.md b/README.md index 606c770..11fff90 100644 --- a/README.md +++ b/README.md @@ -35,6 +35,14 @@ Rabosh.open(Path.of("memories")).use { db -> `RaboshMemoryToolHandler.open(Path.of("memories"))` is the one-liner for a host whose only use for rabosh is this handler; it opens the store, owns it, and closes it with the handler. +**The `Beta` prefixes above belong to the tool runner, not to the memory tool.** The tool itself is +generally available and needs no beta header — declared on an ordinary `client.messages()` call it is +`com.anthropic.models.messages.MemoryTool20250818`. What lives in the SDK's beta namespace is the +*helper* surface: `BetaMemoryToolHandler`, `BetaToolRunner` and the beta `MessageCreateParams` they +require. Using this handler with the runner therefore means depending on a beta helper, and driving +the loop yourself means not depending on one — see +[Errors, and one limitation of the tool runner](#errors-and-one-limitation-of-the-tool-runner). + ## Installation One coordinate. Both dependencies below are declared `api` rather than `implementation`, because @@ -147,7 +155,104 @@ specification is explicit that this is fine — *"Claude reads whatever text you reserved for genuine faults: the store closed, IO failed, the lock was lost. **If you need `is_error` fidelity on expected outcomes, drive the tool-use loop yourself** rather -than using the runner. The six methods are the whole surface; nothing here depends on the runner. +than using the runner. The six methods are the whole surface; nothing here depends on the runner — +and this is also the path that keeps you off the beta helper namespace entirely, since the tool +declaration on a plain `client.messages()` call is `MemoryTool20250818`. + +The loop is the ordinary one: call, check `stop_reason` for `tool_use`, answer every `tool_use` block +in a single user turn, repeat. The only part specific to this handler is the dispatch, which is a +`when` over `command` and one `ToolResultBlockParam` per call: + +```kotlin +fun memoryResult(block: ToolUseBlock, handler: RaboshMemoryToolHandler): ToolResultBlockParam { + val input = block._input().asObject().orElseThrow() + fun text(field: String): String = input[field]?.asString()?.orElse("").orEmpty() + fun number(field: String): Long = input[field]?.asNumber()?.orElseThrow()?.toLong() ?: 0L + + val viewRange: Optional> = Optional.ofNullable( + input["view_range"]?.asArray()?.orElse(null)?.map { it.asNumber().orElseThrow().toLong() }, + ) + + val answer = when (val command = text("command")) { + "view" -> handler.view(text("path"), viewRange) + "create" -> handler.create(text("path"), text("file_text")) + "str_replace" -> handler.strReplace(text("path"), text("old_str"), text("new_str")) + "insert" -> handler.insert(text("path"), number("insert_line"), text("insert_text")) + "delete" -> handler.delete(text("path")) + "rename" -> handler.rename(text("old_path"), text("new_path")) + else -> "Error: unknown command $command" + } + + return ToolResultBlockParam.builder() + .toolUseId(block.id()) + .content(answer) + .isError(answer.startsWith("Error: ")) // the half the runner cannot give you + .build() +} +``` + +`isError` is keyed off the prefix here because that is the cheapest rule that matches this handler's +strings, and it is worth knowing what it does *not* catch: `view`'s missing-path string has no +`Error: ` prefix — see [Deliberate divergences](#deliberate-divergences) — so that one outcome +arrives as a successful result whose text says otherwise. Key off the return value of the specific +command instead if that distinction matters to you. + +## Context editing, and what it does to this store + +A store that outlives the context window is only interesting once the context window is actually +being cleared, so the pairing Anthropic recommends — the memory tool together with +`clear_tool_uses_20250919` — is the case this module is built for. Enabling it is a change to the +message parameters, not to the handler: + +```kotlin +val createParams = MessageCreateParams.builder() // com.anthropic.models.beta.messages, as above + .model(Model.CLAUDE_OPUS_5) + .maxTokens(4096L) + .addTool(BetaMemoryTool20250818.builder().build()) + // AnthropicBeta is one package up, in com.anthropic.models.beta; the three builders below sit + // beside MessageCreateParams. + .addBeta(AnthropicBeta.CONTEXT_MANAGEMENT_2025_06_27) + .contextManagement( + BetaContextManagementConfig.builder() + .addEdit( + BetaClearToolUses20250919Edit.builder() + .trigger(BetaInputTokensTrigger.builder().value(30_000L).build()) + .keep(BetaToolUsesKeep.builder().value(3L).build()) + .build(), + ) + .build(), + ) + .addUserMessage("…") + .build() +``` + +`ToolRunnerCreateParams` takes those as its `initialMessageParams`, so the beta and the +configuration reach the API through the runner with no further plumbing and the handler is not +involved in the decision at all. `BetaCompact20260112Edit` is available the same way if you want +server-side compaction as well — the two solve different halves of the same problem, and the +specification suggests running both. + +Three things follow that are worth knowing before you turn it on. + +**The flush is bursty, and it will produce `create` errors.** As the clearing threshold approaches, +Claude is warned and writes what it wants to keep before its tool results disappear. That burst is +where this store's one deliberate divergence becomes visible: a flush that re-`create`s a path it +already wrote earlier in the same session gets `Error: File … already exists` rather than an +overwrite. This is the documented behaviour working — the model reads the file back and edits it — +and it is the shape to expect in a transcript rather than a bug to report. `MemoryOptions(createOverwrites = true)` +is the other choice, and the trade is unchanged by context editing: overwriting costs you the memory +whose existence the model had forgotten. + +**Do not put `memory` in `exclude_tools`.** That option exists for results that are expensive to +obtain again — a web search, a paid API call. Memory results are the opposite of that: they are the +cheapest thing in the transcript to throw away, because every one of them can be read back off disk +with a `view` whenever it is next needed. Excluding them keeps bytes in the context window that the +store already holds. + +**The burst is still one writing thread.** Several memories arriving at once is the same single-writer +path as any other sequence of commands — see [Requirements](#requirements) — and each is still capped +by `maxMemoryBytes`. Neither is a new constraint; the flush is simply where they are most likely to +be met for the first time. ## Deliberate divergences @@ -184,6 +289,12 @@ number. The reference implementation reports an inode size instead, which is why **An empty `old_str` is answered with "did not appear verbatim".** It matches at every position, and the reference implementation would report one line number per character of the file. +**A `.png` path is a text memory like any other.** Claude's tool description tells it that `view` +displays image files — `.jpg`, `.jpeg` and `.png` — so it may well `view` one. There are no image +files here: `create` takes `file_text`, so a memory whose path happens to end in `.png` is rendered +with line numbers like every other. Nothing is lost, since the model can only read back what it +wrote, but the promise its tool description makes is one this store has no way to keep. + **Four strings are additions**, because the specification asks integrators to enforce something and supplies no wording: @@ -289,6 +400,7 @@ indefinitely. It is a rollback *window*. It is not audit. ./gradlew test -Drabosh.memory.iterations=600 # more generated scripts ./gradlew test -Prabosh.memory.bench # the listing benchmark, excluded from build ANTHROPIC_API_KEY=… ./gradlew test -Prabosh.memory.smoke # one real conversation, excluded from build +ANTHROPIC_API_KEY=… ./gradlew test -Prabosh.memory.smoke -Drabosh.memory.smoke.model=claude-opus-5 ``` The suites, and what each is for: @@ -304,8 +416,10 @@ The suites, and what each is for: | `LiveSmokeTest` | One real conversation, to catch the SDK moving. It **fails** rather than skips without a key | `checkNoNioPath` fails the build if a main source outside the store-directory allowlist uses -`java.nio.file.Path`, and `checkKotlinAbi` fails it if the published surface changed without the -committed dump in `api/` changing with it. +`java.nio.file.Path`; `checkDeleteDoesNotCompact` fails it if `compact()` is called anywhere but +`expireBefore`, because an interactive `delete` that reclaimed tombstones inline would be slow rather +than wrong and no test would see it; and `checkKotlinAbi` fails it if the published surface changed +without the committed dump in `api/` changing with it. CI runs `build` on Linux **and** Windows, and both are load-bearing rather than a formality: `MemoryPathTest` covers the Windows path spellings that are the reason normalisation is a string diff --git a/build.gradle.kts b/build.gradle.kts index b54ec5d..7fe705a 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -112,6 +112,7 @@ tasks.withType().configureEach { "rabosh.memory.bench.warmup", "rabosh.memory.bench.iterations", "rabosh.memory.bench.runs", + "rabosh.memory.smoke.model", )) { providers.systemProperty(key).orNull?.let { systemProperty(key, it) } } @@ -219,8 +220,100 @@ val checkNoNioPath = tasks.register("checkNoNioPath") { } } +/* + * The second rule enforced as a build step, and the reason is the same as the first: it fails + * silently. + * + * `delete` must not call `compact()`. Tombstones are the retention job's business — `expireBefore` + * pins a snapshot, decides the set, deletes and *then* compacts, which is the order a drain has to + * use — while an interactive `delete` writes its batch and stops. Reclaiming inline would make a + * command the model issues mid-conversation unpredictably slow, and that is the whole failure: it + * shows up as latency, not as a wrong answer, so no assertion about behaviour can see it. + * + * "Delete should free space" is the intuitive position, the correct code for it lives in the same + * file, and until this task existed nothing stood between the two. The check is deliberately narrow — + * `compact()` is legitimate inside `expireBefore` and nowhere else — because a rule that also forbade + * the correct call would be worse than no rule. + */ +val checkDeleteDoesNotCompact = tasks.register("checkDeleteDoesNotCompact") { + group = "verification" + description = "Fails if compact() is called outside expireBefore." + + val sources = sourceSets["main"].allSource.matching { include("**/*.kt") } + val report = layout.buildDirectory.file("reports/delete-does-not-compact.txt") + + inputs.files(sources).withPathSensitivity(PathSensitivity.RELATIVE) + outputs.file(report) + + doLast { + // Comments go first, for the reason `checkNoNioPath` strips them: the paragraphs explaining + // why `delete` does not compact are themselves full of the word, and a rule that tripped over + // its own rationale would be unmaintainable. String literals are blanked in the same pass so + // that a brace inside one cannot move the function boundary found below. + fun code(line: String): String { + val trimmed = line.trimStart() + if (trimmed.startsWith("*") || trimmed.startsWith("//") || trimmed.startsWith("/*")) return "" + return line.substringBefore("//").replace(Regex("\"(\\\\.|[^\"\\\\])*\""), "\"\"") + } + + val offenders = mutableListOf() + + for (file in sources.files.sortedBy { it.path }) { + val lines = file.readLines().map(::code) + val calls = lines.indices.filter { "compact(" in lines[it] } + if (calls.isEmpty()) continue + + // The allowed window: from `expireBefore`'s declaration to the brace that closes it. + // Absent in every file but the handler, which is the point — a `compact()` anywhere else + // has no window to be inside of. + val declaration = lines.indexOfFirst { "fun expireBefore(" in it } + var window = IntRange.EMPTY + if (declaration >= 0) { + var depth = 0 + var opened = false + for (index in declaration until lines.size) { + depth += lines[index].count { it == '{' } - lines[index].count { it == '}' } + if (depth > 0) opened = true + if (opened && depth == 0) { + window = declaration..index + break + } + } + check(window != IntRange.EMPTY) { + "${file.path}: expireBefore's body is unbalanced, so this check cannot tell " + + "which calls are inside it. That is a bug in the check, not in the source." + } + } + + calls.filterNot { it in window } + .mapTo(offenders) { "${file.path}:${it + 1}: ${lines[it].trim()}" } + } + + report.get().asFile.apply { + parentFile.mkdirs() + writeText( + if (offenders.isEmpty()) { + "compact() is called only inside expireBefore\n" + } else { + offenders.joinToString("\n") + "\n" + }, + ) + } + + check(offenders.isEmpty()) { + "compact() is called outside expireBefore, in:\n" + + offenders.joinToString("\n") { " $it" } + + "\nAn interactive command writes its batch and stops; reclaiming tombstones is " + + "`expireBefore`'s job and runs on the host's schedule rather than in the middle of a " + + "conversation. See CLAUDE.md, \"Design rules that must not be quietly broken\". If a " + + "new declaration genuinely needs to compact, it belongs beside `expireBefore` in the " + + "retention section and this check needs widening deliberately." + } + } +} + tasks.named("check") { - dependsOn(checkNoNioPath) + dependsOn(checkNoNioPath, checkDeleteDoesNotCompact) } /* diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index e6c8936..57981fa 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -1,4 +1,4 @@ -# Centralised version catalogue. Versions verified 2026-08-16 against Maven Central: every entry is +# Centralised version catalogue. Versions verified 2026-08-17 against Maven Central: every entry is # the latest stable release. No pre-releases — those need asking first. # # Dependency policy (see CLAUDE.md): JetBrains and Kotlin libraries are pre-approved; everything @@ -7,7 +7,8 @@ # - `anthropic-java` is the reason this repository exists and is the one third-party *runtime* # dependency it may have. Confirmed. # - `junit` is test-only and never reaches the published POM. Confirmed 2026-08-16, against a -# hand-rolled harness, because rabosh already pins the same version for the same suites. +# hand-rolled harness, because rabosh already pins the same version for the same suites. The +# confirmation is of the dependency, not of a patch number: 6.1.2 → 6.1.3 needs no new one. [versions] kotlin = "2.4.10" @@ -25,7 +26,7 @@ anthropic = "2.54.0" rabosh = "0.3.0" # Test-only. -junit = "6.1.2" +junit = "6.1.3" dokka = "2.2.0" # The engine's floor, and therefore this module's. FFM (Arena, MemorySegment, FileChannel.map with diff --git a/src/test/kotlin/app/oreshkov/rabosh/memory/LiveSmokeTest.kt b/src/test/kotlin/app/oreshkov/rabosh/memory/LiveSmokeTest.kt index 0d31724..577dcc7 100644 --- a/src/test/kotlin/app/oreshkov/rabosh/memory/LiveSmokeTest.kt +++ b/src/test/kotlin/app/oreshkov/rabosh/memory/LiveSmokeTest.kt @@ -33,6 +33,12 @@ import org.junit.jupiter.api.Tag * * It **fails** rather than skips when the key is absent. A smoke test that quietly passes because it * did not run is worse than no smoke test, because it is reported as a green canary. + * + * The model is [DEFAULT_MODEL] and `-Drabosh.memory.smoke.model=` changes it. The default is the + * cheapest model that can drive the tool, which is the right default for a canary whose subject is + * the SDK rather than the model: what it watches is the runner's dispatch, not anyone's reasoning. A + * release that wants one run against the model its README recommends can pass the dial rather than + * edit this file. */ @Tag("smoke") class LiveSmokeTest { @@ -91,7 +97,7 @@ class LiveSmokeTest { val counting = CountingHandler(handler) val createParams = MessageCreateParams.builder() - .model(Model.CLAUDE_HAIKU_4_5) + .model(model()) .maxTokens(1024L) .addTool(BetaMemoryTool20250818.builder().build()) .addUserMessage(prompt) @@ -116,6 +122,14 @@ class LiveSmokeTest { private class Transcript(val commands: Int, val text: String) + private companion object { + /** Cheapest model that drives the tool; the canary's subject is the SDK, not the model. */ + const val DEFAULT_MODEL: String = "claude-haiku-4-5" + + fun model(): Model = + Model.of(System.getProperty("rabosh.memory.smoke.model").orEmpty().ifBlank { DEFAULT_MODEL }) + } + /** * Counts the commands the runner dispatched. *