Skip to content

Commit 3d3deec

Browse files
clay-goodclaude
andcommitted
feat(memory): harden the memory-integrity invariant under concurrency + adversarial edits
Bulletproof OpenLore's one defensible promise — never serve an unverified or stale fact as authoritative — across durability, concurrency, and freshness. Invariant (tests): - Name and property-test the AuthoritativeRecallInvariant over recall + orient: no drifted/orphaned memory ever sits in an authoritative set unlabeled (memory-invariant.test.ts, generated memories × generated code mutations). - Adversarial freshness suite (anchor-adversarial.test.ts): rename/move/delete → orphaned; whitespace/comment-only edits → drifted; multibyte UTF-8 span boundaries hash byte-correctly; truncated-hash collision-freedom property. Governing rule: a false-fresh fails the suite; a false-orphaned is acceptable. Durability (atomic-store.ts): - Atomic temp-write + fsync + rename for notes.json and pending.json — a crash mid-write leaves the prior committed store intact, never a torn file. - Monotonic `sequence` field (additive, defaults 0) + compare-and-swap on save: on conflict, re-read and re-apply the id-keyed merge instead of clobbering. remember and record_decision now lose zero writes under concurrency. - Corrupt-store quarantine: a store that fails validation is moved to *.corrupt-<n> (deterministic suffix, not wall-clock) and signaled, never silently substituted with empty. Gate (gate-state.ts): - Extract the pre-commit gate reason machine into a pure classifier and property-test it: total (every state → one reason), deterministic/idempotent, deadlock-free. decisions.ts delegates to it (behavior-preserving). Specs: AuthoritativeRecallInvariant / FreshnessFailsSafeTowardDistrust / ConcurrentMemoryWriteSafety added to mcp-handlers; DurableAtomicStorePersistence / CorruptStoreQuarantineNotSilentEmpty added to architecture; linked from CODEBASE.md. No new dependency, no LLM, schema extended additively. Full suite: 3732 passing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3af3046 commit 3d3deec

15 files changed

Lines changed: 1473 additions & 125 deletions

File tree

openspec/specs/architecture/spec.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,44 @@ The system SHALL implement security via: API key-based authentication for LLM pr
3838
- **WHEN** accessing protected resources
3939
- **THEN** access is denied
4040

41+
### Requirement: DurableAtomicStorePersistence
42+
43+
The persisted memory and decision stores (`.openlore/memory/notes.json`,
44+
`.openlore/decisions/pending.json`) SHALL be written atomically: a write goes to a temporary
45+
file and is moved into place with an atomic rename (after `fsync`), so a crash or
46+
interruption mid-write leaves the previously committed store intact and never a partially
47+
written (torn) file. Each store SHALL carry a monotonic `sequence` field used to order writes
48+
and detect conflicts; the field defaults to `0` for legacy stores. Saves use compare-and-swap
49+
on `sequence`: on a conflict the save re-reads the current store and re-applies the pending
50+
append/upsert rather than overwriting a competing write. Implemented in
51+
`src/core/decisions/atomic-store.ts`; guarded by `atomic-store.test.ts`.
52+
53+
#### Scenario: A crash mid-write preserves the prior store
54+
- **GIVEN** a store write interrupted between writing the temporary file and the rename
55+
- **WHEN** the store is next loaded
56+
- **THEN** the previously committed store is returned intact, with no torn or partial content
57+
58+
#### Scenario: Save uses compare-and-swap on sequence
59+
- **GIVEN** a store loaded at sequence S
60+
- **WHEN** a save is attempted but the on-disk sequence is no longer S
61+
- **THEN** the save re-reads the current store, re-applies the pending change, and writes at
62+
the new sequence instead of overwriting
63+
64+
### Requirement: CorruptStoreQuarantineNotSilentEmpty
65+
66+
When a persisted store fails validation on load, the system SHALL move the unreadable file
67+
aside to a quarantine path (`*.corrupt-<n>`) and emit a recoverable signal. The system SHALL
68+
NOT silently substitute an empty store for a corrupt one, because silently losing persisted
69+
memory presents absence as current fact and violates the authoritative-recall invariant. The
70+
quarantine suffix SHALL be derived from on-disk state (the next free index), not wall-clock
71+
time, to keep recovery reproducible.
72+
73+
#### Scenario: A malformed store is quarantined, not silently emptied
74+
- **GIVEN** a store file that fails schema or JSON validation
75+
- **WHEN** the store is loaded
76+
- **THEN** the file is moved to `*.corrupt-<n>` and a recoverable signal is emitted, rather
77+
than an empty store being returned silently
78+
4179
## System Diagram
4280

4381
```mermaid

openspec/specs/mcp-handlers/spec.md

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,72 @@ The system SHALL The recall tool SHALL partition returned memories into authorit
116116

117117
> Decision recorded: dbe6a95e
118118
> Date: 2026-06-16
119+
120+
### Requirement: AuthoritativeRecallInvariant
121+
122+
The system SHALL guarantee, as a single named and test-enforced invariant, that **no
123+
memory whose freshness verdict is `drifted` or `orphaned` ever appears in an authoritative
124+
recall path unlabeled**. The authoritative recall paths are the `recall` tool and the
125+
memory (decision) section of `orient`. An `orphaned` memory SHALL be fully withheld from
126+
the authoritative set (surfaced only under `needsReanchoring` / `staleDecisions`); a
127+
`drifted` memory MAY remain in the authoritative set only when it carries an explicit
128+
`verify` label. This invariant is the operational definition of the project promise:
129+
*OpenLore never serves an unverified or stale fact as authoritative.* It SHALL be enforced
130+
by a property-based test (`memory-invariant.test.ts`) that generates arbitrary memories and
131+
arbitrary code mutations and asserts the property holds for every generated case.
132+
133+
#### Scenario: A drifted memory is excluded from the authoritative set unlabeled
134+
135+
- **GIVEN** a memory whose anchor verdict is `drifted`
136+
- **WHEN** `recall` or `orient` produces its response
137+
- **THEN** the memory does not appear in the authoritative set unlabeled; it is withheld or
138+
carries an explicit verify/non-authoritative label
139+
140+
#### Scenario: The invariant holds under generated mutation
141+
142+
- **GIVEN** an arbitrary memory and an arbitrary mutation to the code it anchors
143+
- **WHEN** the authoritative recall path is computed
144+
- **THEN** the authoritative set contains only `fresh` memories and explicitly-labeled
145+
`drifted` ones, never an `orphaned` memory
146+
147+
### Requirement: FreshnessFailsSafeTowardDistrust
148+
149+
The freshness computation (`anchorFreshness`, `hashSpan`) SHALL fail safe toward distrust:
150+
any ambiguity, hash collision, or boundary error SHALL bias the verdict toward `drifted` or
151+
`orphaned`, never toward a false `fresh`. A renamed, moved, or deleted symbol SHALL yield
152+
`orphaned` (or `drifted` only when a confident relocation is established). `hashSpan` SHALL
153+
slice spans by byte offset so multibyte UTF-8 boundaries hash correctly. A test that
154+
produces a false `fresh` SHALL be treated as a correctness failure; a false `orphaned` is
155+
acceptable. This is guarded by the adversarial suite (`anchor-adversarial.test.ts`).
156+
157+
#### Scenario: A forced collision does not produce false fresh
158+
159+
- **GIVEN** two distinct source spans
160+
- **WHEN** freshness is computed for a memory anchored to one after the other replaces it
161+
- **THEN** the verdict is `drifted` or `orphaned`, never `fresh` (distinct spans do not
162+
collide on the truncated content hash; a collision would fail the suite loudly)
163+
164+
#### Scenario: A multibyte span boundary hashes correctly
165+
166+
- **GIVEN** an anchored span whose start or end falls on a multibyte UTF-8 boundary
167+
- **WHEN** `hashSpan` computes the content hash before and after an unrelated edit elsewhere
168+
- **THEN** the hash is byte-correct and stable, producing `fresh` only when the span bytes
169+
are unchanged
170+
171+
### Requirement: ConcurrentMemoryWriteSafety
172+
173+
The `remember` and `record_decision` tools SHALL be safe under concurrent invocation: two
174+
concurrent writes to the same store SHALL NOT cause either write to be lost. On a write
175+
conflict the system SHALL re-read the current store and re-apply the pending
176+
append/upsert (compare-and-swap on a monotonic `sequence`), rather than overwrite the
177+
competing write.
178+
179+
#### Scenario: Concurrent remember calls lose no write
180+
181+
- **GIVEN** N concurrent `remember` calls against the same memory store
182+
- **WHEN** all calls complete
183+
- **THEN** the persisted store contains all N memories
184+
119185
### Requirement: DecisionsCarryStructuralAnchorsForSelfinvalidation
120186

121187
The system SHALL resolve structural anchors against the call graph when recording a decision, falling back to file-level anchors when no analysis is available.

src/cli/commands/decisions.ts

Lines changed: 79 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
INACTIVE_STATUSES,
3030
} from '../../core/decisions/store.js';
3131
import { consolidateDrafts } from '../../core/decisions/consolidator.js';
32+
import { classifyGateState } from '../../core/decisions/gate-state.js';
3233
import { acquireDecisionsLock } from '../../core/decisions/lock.js';
3334
import { extractFromDiff } from '../../core/decisions/extractor.js';
3435
import { verifyDecisions } from '../../core/decisions/verifier.js';
@@ -769,9 +770,49 @@ Examples:
769770

770771
// ── Gate only (no consolidation — consolidation happens on record_decision) ──
771772
if (options.gate && !options.consolidate) {
772-
// Block if approved decisions haven't been synced to spec files yet.
773773
const approved = getDecisionsByStatus(store, 'approved');
774-
if (approved.length > 0) {
774+
const verified = getDecisionsByStatus(store, 'verified');
775+
const drafts = getDecisionsByStatus(store, 'draft');
776+
const missing: Array<{ file: string; description: string }> = [];
777+
778+
// Phantom decisions ("recorded but no code evidence") are excluded — stale
779+
// phantoms from previous sessions would otherwise silently bypass the gate.
780+
const activeCount = store.decisions.filter((d) => !INACTIVE_STATUSES.has(d.status)).length;
781+
const consolidatedRecently = !!store.lastConsolidatedAt
782+
&& (Date.now() - new Date(store.lastConsolidatedAt).getTime()) < CONSOLIDATION_GRACE_PERIOD_MS;
783+
784+
// The staged-source check is the only input requiring git; resolve it lazily,
785+
// only in the state where it can change the outcome (nothing else gates).
786+
let isGitRepo = false;
787+
let hasStagedSourceChanges = false;
788+
if (approved.length === 0 && verified.length === 0 && drafts.length === 0
789+
&& !consolidatedRecently && activeCount === 0) {
790+
isGitRepo = await isGitRepository(rootPath);
791+
if (isGitRepo) {
792+
try {
793+
const { stdout } = await execFileAsync(
794+
'git', ['diff', '--cached', '--name-only', '--diff-filter=ACDMR'],
795+
{ cwd: rootPath },
796+
);
797+
const stagedFiles = stdout.trim().split('\n').filter(Boolean);
798+
const SOURCE_EXTS = /\.(ts|js|tsx|jsx|py|go|rs|rb|java|cpp|cc|swift)$/;
799+
hasStagedSourceChanges = stagedFiles.some((f) => SOURCE_EXTS.test(f));
800+
} catch { /* git unavailable — skip */ }
801+
}
802+
}
803+
804+
// The pure reason machine is the single arbiter of which reason applies.
805+
const outcome = classifyGateState({
806+
approvedCount: approved.length,
807+
verifiedCount: verified.length,
808+
draftCount: drafts.length,
809+
consolidatedRecently,
810+
activeCount,
811+
isGitRepo,
812+
hasStagedSourceChanges,
813+
});
814+
815+
if (outcome.reason === GATE_REASONS.APPROVED_NOT_SYNCED) {
775816
const payload = {
776817
gated: true,
777818
reason: GATE_REASONS.APPROVED_NOT_SYNCED,
@@ -784,74 +825,48 @@ Examples:
784825
return;
785826
}
786827

787-
const verified = getDecisionsByStatus(store, 'verified');
788-
const missing: Array<{ file: string; description: string }> = [];
789-
790-
if (verified.length === 0) {
791-
const drafts = getDecisionsByStatus(store, 'draft');
792-
if (drafts.length > 0) {
793-
// Drafts recorded but consolidation never completed.
794-
// Output structured JSON so the agent can relay to the user and act on the answer.
795-
const payload = {
796-
gated: true,
797-
reason: GATE_REASONS.DRAFTS_PENDING_CONSOLIDATION,
798-
message: `${drafts.length} draft decision(s) were recorded but never consolidated.`,
799-
drafts: drafts.map((d) => ({ id: d.id, title: d.title, recordedAt: d.recordedAt })),
800-
actions: {
801-
consolidate: 'openlore decisions --consolidate',
802-
consolidateAndGate: 'openlore decisions --consolidate --gate',
803-
skip: 'git commit --no-verify',
804-
},
805-
};
806-
process.stdout.write(JSON.stringify(payload, null, 2) + '\n');
807-
process.exitCode = 1;
808-
return;
809-
}
828+
if (outcome.reason === GATE_REASONS.DRAFTS_PENDING_CONSOLIDATION) {
829+
// Drafts recorded but consolidation never completed.
830+
// Output structured JSON so the agent can relay to the user and act on the answer.
831+
const payload = {
832+
gated: true,
833+
reason: GATE_REASONS.DRAFTS_PENDING_CONSOLIDATION,
834+
message: `${drafts.length} draft decision(s) were recorded but never consolidated.`,
835+
drafts: drafts.map((d) => ({ id: d.id, title: d.title, recordedAt: d.recordedAt })),
836+
actions: {
837+
consolidate: 'openlore decisions --consolidate',
838+
consolidateAndGate: 'openlore decisions --consolidate --gate',
839+
skip: 'git commit --no-verify',
840+
},
841+
};
842+
process.stdout.write(JSON.stringify(payload, null, 2) + '\n');
843+
process.exitCode = 1;
844+
return;
845+
}
810846

811-
// If consolidation already ran recently, trust it found nothing — skip the warning.
812-
const consolidatedRecently = store.lastConsolidatedAt
813-
&& (Date.now() - new Date(store.lastConsolidatedAt).getTime()) < CONSOLIDATION_GRACE_PERIOD_MS;
814-
if (consolidatedRecently) {
815-
process.exitCode = 0;
816-
return;
817-
}
847+
if (outcome.reason === GATE_REASONS.NO_DECISIONS_RECORDED) {
848+
// Source files staged but nothing recorded — output JSON for agent to relay.
849+
const payload = {
850+
gated: true,
851+
reason: GATE_REASONS.NO_DECISIONS_RECORDED,
852+
message: 'Source files are staged but no architectural decisions were recorded.',
853+
actions: {
854+
consolidateAndGate: 'openlore decisions --consolidate --gate',
855+
skip: 'git commit --no-verify',
856+
},
857+
};
858+
process.stdout.write(JSON.stringify(payload, null, 2) + '\n');
859+
process.exitCode = 1;
860+
return;
861+
}
818862

819-
// If source files are staged but nothing was recorded, offer to run the fallback extractor.
820-
// Phantom decisions ("recorded but no code evidence") are excluded — stale phantoms from
821-
// previous sessions would otherwise silently bypass the gate for all future commits.
822-
const activeDecisions = store.decisions.filter(
823-
(d) => !INACTIVE_STATUSES.has(d.status),
824-
);
825-
if (activeDecisions.length === 0 && await isGitRepository(rootPath)) {
826-
try {
827-
const { stdout } = await execFileAsync(
828-
'git', ['diff', '--cached', '--name-only', '--diff-filter=ACDMR'],
829-
{ cwd: rootPath },
830-
);
831-
const stagedFiles = stdout.trim().split('\n').filter(Boolean);
832-
const SOURCE_EXTS = /\.(ts|js|tsx|jsx|py|go|rs|rb|java|cpp|cc|swift)$/;
833-
const hasSourceChanges = stagedFiles.some((f) => SOURCE_EXTS.test(f));
834-
if (hasSourceChanges) {
835-
// Source files staged but nothing recorded — output JSON for agent to relay.
836-
const payload = {
837-
gated: true,
838-
reason: GATE_REASONS.NO_DECISIONS_RECORDED,
839-
message: 'Source files are staged but no architectural decisions were recorded.',
840-
actions: {
841-
consolidateAndGate: 'openlore decisions --consolidate --gate',
842-
skip: 'git commit --no-verify',
843-
},
844-
};
845-
process.stdout.write(JSON.stringify(payload, null, 2) + '\n');
846-
process.exitCode = 1;
847-
return;
848-
}
849-
} catch { /* git unavailable — skip */ }
850-
}
863+
if (!outcome.gated) {
864+
// Clean commit — nothing to review.
851865
process.exitCode = 0;
852866
return;
853867
}
854868

869+
// outcome.reason === GATE_REASONS.VERIFIED — verified decisions await review.
855870
// TTY: interactive TUI
856871
if (process.stdin.isTTY && process.stdout.isTTY && verified.length > 0) {
857872
const results = await runTuiApproval(verified);

0 commit comments

Comments
 (0)