Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ serde_json = { workspace = true }
surrealdb = { workspace = true }
surrealdb_types = { workspace = true }
anyhow = { workspace = true }
arc-swap = { workspace = true }
thiserror = { workspace = true }
sha2 = { workspace = true }
tracing = { workspace = true }
Expand Down
15 changes: 15 additions & 0 deletions crates/surreal-memory/src/storage/surreal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,6 +924,21 @@ DEFINE INDEX IF NOT EXISTS memory_embedding_hnsw
self.live_db()
}

/// Return a connection for the durable operation ledger.
///
/// Server mode opens an independent transport so cancelling a ledger query
/// cannot strand unrelated storage work on the same WebSocket. Embedded
/// mode clones the in-process SDK handle because RocksDB cannot be opened a
/// second time at the same path.
pub async fn operation_ledger_connection(&self) -> Result<Surreal<Any>> {
match self.connection_info.config.mode {
SurrealMode::Embedded => self.live_db(),
SurrealMode::Server => {
Self::connect_with_attempts(&self.connection_info.config, 0).await
}
}
}

/// Persist a fully planned and embedded logical memory under a stable key.
///
/// The durable-operation coordinator derives `record_key` from its caller
Expand Down
1 change: 1 addition & 0 deletions docs/lessons.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ Format: `YYYY-MM-DD — Rule — *(context: what went wrong)*`
- 2026-08-26 — Never stack a PR onto another PR's branch when the base will merge first. PR #9 targeted PR #8's branch; #8 merged to main, then #9 merged into an orphaned branch and GitHub reported "MERGED" while main had none of it. Target `main` and rebase, or the fix silently never ships.
- 2026-08-26 — When adding a typed field to a struct that derives Serde, derive the same Serde direction on the field type before the first compile. *(Context: `LocalEmbeddingBackend` was added to deserializable `Config` without `Deserialize`.)*
- 2026-08-26 — Do not run the aggregate `prometheus-rust-auditor audit` command in this repository: its pipeline generates a GitHub Actions workflow, which violates the local-only validation policy. Run the read-only enforcement, format, dependency, inventory, and partition checks individually.
- 2026-09-21 — Never launch `cargo fmt` and `cargo test` in parallel in this repository; they share one target directory, so even a read-only format check violates the single-writer build rule when paired with compilation.
- 2026-08-26 — A copied SwiftPM executable is not self-contained when a C target ships resources: install `mlx-swift_Cmlx.bundle` beside the executable and smoke-test the copied path, because the build-tree binary can hide a missing `default.metallib` deployment.
- 2026-08-26 — Persisted executor generations span server processes, but a newly pre-warmed child starts with process-local numbering. Adopt the healthy child into the durable generation sequence; do not kill it merely because its initial number is lower, or queue recovery turns into an unnecessary cold model launch.
- 2026-08-26 — A liveness heartbeat must run independently of the work it supervises. An async Swift `Task` heartbeat can be starved while MLX/Metal synchronously occupies a cooperative executor; use a dedicated Dispatch timer and synchronously drain it before writing the terminal protocol message.
Expand Down
54 changes: 54 additions & 0 deletions openspec/changes/bound-operation-query-deadlines/evidence.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
# Verification evidence

Date: 2026-09-21

## Source and review

- PR #20: https://github.com/Prometheus-AGS/surreal-memory-server/pull/20
- source: `93372f820438dc485277530e8d5b23c21711090d`
- merge: `b000bced73f4f626625b4379d26cda657999e080`
- Critic round 1 returned BLOCK because the recovery assertion used a second
coordinator. The regression was repaired to use the production coordinator.
- Critic round 2 returned BLOCK because the fixture could race with coordinator
processing. The terminal receipt is now seeded before the coordinator starts.
- Follow-up change `complete-operation-ledger-recovery` addressed the recorded
blocker in a new review cycle. Its round 1 critic required a direct
coordinator retry regression and removal of a redundant legacy-spec edit;
both were repaired. Round 2 returned PASS with no blocking findings.

## Local verification

- `RUSTC_WRAPPER= cargo check --locked --package surreal-memory-server --no-default-features --features server-only`
— exit 0; compilation completed in 1 minute 16 seconds.
- `RUSTC_WRAPPER= cargo test --locked --test operation_query_deadline --no-default-features --features server-only`
— exit 0; 1 test passed. A real isolated SurrealDB server held a large
receipt query past the 50 ms application deadline, and the same production
coordinator then committed a subsequent operation.
- `RUSTC_WRAPPER= cargo test --locked --test executor_recovery` — exit 0; 4
tests passed.
- `RUSTFLAGS='-Dwarnings' cargo build --release --locked --no-default-features --features embedded,metal,local-embeddings`
— exit 0; release build completed in 37.71 seconds.
- `cargo fmt --all --check` — exit 0 on the merged tree.
- `openspec validate bound-operation-query-deadlines --strict` — exit 0; the
change is valid on the merged tree.
- `RUSTC_WRAPPER= cargo test --locked --lib operations::tests:: -- --nocapture`
— exit 0; 17 tests passed, including independent initialization,
generation-safe overlap, replacement failure, startup retry, and the actual
coordinator drain retry branch.
- `prometheus-rust-auditor enforce`, `format`, and `inventory` — exit 0; no
findings. The dependency phase remains a repository baseline limitation:
`cargo-deny` is not installed, while `cargo audit` reports the same three
advisories in both the committed and working lockfiles.

## Deployment state

The signed installed binary has SHA-256
`63d4b297a9e4b1ebd2cb61ebac0752a00532eaf611b5b0a967ed9dc875942046`
at both owned install paths, and both copies pass `codesign --verify`.

The deployed server recorded
`operation database reconciliation discovery timed out after 10000ms` instead
of leaving the startup future pending indefinitely. The service stayed ready,
and the durable queue continued to advance after the learning worker loaded.
Backlog recovery is still running, so task 1.3 remains open until the accepted
count reaches zero and the final doctor check exits successfully.
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Verification evidence

Date: 2026-09-21

## Source and review

- PR #18: https://github.com/Prometheus-AGS/surreal-memory-server/pull/18
- source: `16db67a94230f93a73be6a608cb4b357ec0231c4`
- merge: `199651d8ec632a1918f64d3e9f015b5a18f84111`
- PR #19: https://github.com/Prometheus-AGS/surreal-memory-server/pull/19
- source: `9e9c180d4f9bc8c88b96770ea413e9a1c2bcc233`
- merge: `c88144158d8e7bfc6f690d88586e5e68183521d7`
- The PR #19 isolated artifact critic returned PASS. PR #18 also passed the
repository Rust audit gate with no blocking finding.

## Local verification

- `RUSTC_WRAPPER= cargo test --locked operations::tests::startup_reconciliation -- --nocapture`
— exit 0; 2 tests passed.
- `RUSTC_WRAPPER= cargo test --locked operations::tests::startup_reconciliation_query_projects_only_operation_identity -- --nocapture`
— exit 0; 1 test passed.
- `cargo fmt --all --check` — exit 0 on the merged tree.
- `openspec validate bound-operation-reconciliation-projection --strict` —
exit 0; the change is valid on the merged tree.

## Deployment state

The projection changes are installed through the signed
`surreal-memory-server` binary whose SHA-256 is
`63d4b297a9e4b1ebd2cb61ebac0752a00532eaf611b5b0a967ed9dc875942046`
at both `~/.local/bin` and `/usr/local/bin`. Both copies pass
`codesign --verify`.

Backlog recovery is still running. The installed service reduced the local
durable queue from 271 accepted and 2,286 completed operations to 261 accepted
and 2,296 completed operations while both database health and memory readiness
returned HTTP 200. Task 1.3 remains open until the accepted count reaches zero
and the final doctor check exits successfully.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-09-20
95 changes: 95 additions & 0 deletions openspec/changes/complete-operation-ledger-recovery/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
## Context

See `proposal.md` for the failure. `OperationService` currently stores a
generation-tagged SDK handle, but it seeds generation zero by cloning the
general `SurrealStorage` handle. Its timeout error is also erased into
`anyhow::Error`, so the coordinator retries every processing failure rather
than the single recovery case the spec permits.

The Surreal SDK handle is clone-safe, but server-mode clones multiplex one
physical WebSocket. An outer Tokio timeout can drop a query future while the
SDK is still unwinding that transport. The server-mode ledger therefore needs
a separately opened SDK connection. Embedded mode is different: opening the
same RocksDB path twice is invalid, and its clone is an in-process handle rather
than a shared remote transport.

## Goals / Non-Goals

**Goals:**

- Make lazy first-use initialization and later replacement use the same
serialized independent-connection path.
- Preserve the existing public HTTP deadline error text.
- Keep retry classification inspectable after errors acquire context.
- Prove the retry and generation decisions deterministically, with a real
server integration proving isolation from general storage.

**Non-Goals:**

- Change the Surreal SDK's internal query timeout or general storage reconnect
policy.
- Retry executor work, payload validation, or arbitrary database failures.
- Add a third connection pool or configurable recovery knobs.

## Decisions

### Lazy independent initialization

`OperationService` starts with no ledger connection. Its async accessor locks
the existing replacement mutex, checks again after acquiring the lock, asks
`SurrealStorage` for an operation-ledger handle, and publishes generation zero.
That storage method opens a fresh authenticated transport in server mode and
clones the live in-process handle in embedded mode. This keeps the synchronous
router builder unchanged while ensuring a server query cannot fall back to the
shared WebSocket.

Making the whole router builder async was rejected because it would broaden
every construction site and still require concurrency control for the first
request. Pre-opening a connection by blocking inside the synchronous builder
was rejected because it can deadlock a Tokio runtime.

### One typed deadline error with recovery state

The deadline error retains the stage, elapsed milliseconds, and whether a
newer ledger generation was available after replacement. Its display remains
the existing API error. The coordinator inspects this type through the error
chain and retries only when recovery succeeded.

String matching was rejected because repository lessons prohibit retry
classification from rendered messages. Retrying every `anyhow::Error` was
rejected because it repeats executor work.

### Generation check under one async mutex

Initialization and replacement share one Tokio mutex. A replacement checks the
currently published generation after locking; if another caller already
advanced it, the replacement succeeds without opening or publishing another
connection. This prevents an older recovery from overwriting a newer handle.

### Evidence split

Pure decision helpers and generation transitions receive focused unit tests,
including connector failure and non-ledger errors. The server integration
holds a large ledger receipt query past the application deadline while a
general storage health query completes, then proves the same production
coordinator accepts and commits later work.

## Risks / Trade-offs

- **[Risk] The first server-mode operation request now pays one connection
setup.** → The setup happens once and is serialized; embedded mode retains
its existing in-process cost.
- **[Risk] A failed initial connection leaves the slot empty.** → A later call
may attempt initialization again; no shared fallback is allowed.
- **[Risk] Real-server timing evidence can be affected by host pressure.** →
The integration uses an isolated fixture and deterministic large result,
while retry classification and generation behavior use non-timing tests.

## Migration Plan

1. Merge and build the repaired binary from a clean committed tree.
2. Install and sign both owned binary copies.
3. Restart the database, memory server, and learning worker in dependency order.
4. Observe accepted receipts reach zero and run `prometheus doctor --json`.
5. Roll back to the preceding signed binary if readiness or backlog progress
regresses; stored receipts require no schema migration.
59 changes: 59 additions & 0 deletions openspec/changes/complete-operation-ledger-recovery/evidence.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
# Verification evidence

Date: 2026-09-21

## Behavior

- Server-mode operation-ledger initialization opens a separately authenticated
transport before the first query; embedded mode clones its in-process handle
and does not reopen RocksDB.
- Initialization and replacement share one async mutex and check the published
generation after acquiring it, so overlapping stale callers publish only one
next generation.
- A database deadline is retryable only when replacement succeeded or another
caller already published a newer generation. Replacement failure preserves
the existing API error but is not classified as recovered.
- Startup discovery and the coordinator drain each retry at most once only for
that typed recovered condition. Executor and ordinary errors are not retried.

## Local verification

- `cargo fmt --all --check` — exit 0.
- `git diff --check` — exit 0.
- `RUSTC_WRAPPER= cargo check --locked --package surreal-memory-server --no-default-features --features server-only`
— exit 0.
- `RUSTC_WRAPPER= cargo test --locked --lib operations::tests:: -- --nocapture`
— exit 0; 17 passed, 0 failed.
- `RUSTC_WRAPPER= cargo test --locked --test operation_query_deadline --no-default-features --features server-only -- --nocapture`
— exit 0; 1 passed, 0 failed. The real isolated server kept general storage
responsive while four ledger receipt queries timed out, then the same
production coordinator committed later work.
- `RUSTC_WRAPPER= cargo test --locked --test executor_recovery -- --nocapture`
— exit 0; 4 passed, 0 failed.
- `openspec validate complete-operation-ledger-recovery --strict` — exit 0.
- `openspec validate bound-operation-query-deadlines --strict` — exit 0.
- `openspec validate bound-operation-reconciliation-projection --strict` —
exit 0.
- `prometheus-rust-auditor format`, `enforce`, and `inventory` — exit 0; no
findings. `partition` exited 0 with six informational AI-loop-pending rows.
- `prometheus-rust-auditor deps` could not pass because `cargo-deny` is absent
and the committed lockfile already contains the same three `cargo audit`
advisories as the working lockfile: RUSTSEC-2026-0235, RUSTSEC-2023-0071,
and RUSTSEC-2026-0285. This change adds only the already workspace-pinned
`arc-swap` package dependency and introduces none of those advisory paths.

## Isolated review

- Round 1: BLOCK. It required exercising the actual coordinator drain retry
branch and removing the duplicate mutation to the prior deadline spec.
- Repairs: added a deterministic `drain_pending` regression for recovered,
repeated, and executor errors; restored the prior change's spec to committed
bytes.
- Round 2: PASS. No blocking findings.

## Deployment state

The repaired source is not yet the installed binary. Deployment certification
requires the merged commit to be built, signed, installed at both owned paths,
and observed draining every accepted receipt before `prometheus doctor --json`
can pass.
41 changes: 41 additions & 0 deletions openspec/changes/complete-operation-ledger-recovery/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
## Why

The first operation-ledger query still uses the shared storage connection, so
cancelling that query can poison unrelated memory persistence before rotation
begins. The current recovery retry also catches executor failures, which can
repeat non-database work and violates the intended retry boundary.

## What Changes

- Open the server-mode operation ledger on an independent connection before
its first query, including startup reconciliation and concurrent API
requests; retain one clone-safe in-process handle for embedded mode, where a
second RocksDB open on the same path is invalid.
- Serialize connection initialization and replacement so an older recovery
cannot overwrite a newer healthy generation.
- Classify a database deadline as retryable only after the stale ledger
generation has been replaced.
- Retry startup discovery and an interrupted coordinator operation once only
for that typed stale-ledger condition.
- Add deterministic regressions for initialization, overlapping replacement,
replacement failure, retry classification, and startup recovery.

## Capabilities

### New Capabilities

- `operation-ledger-connection-recovery`: Independent connection ownership,
generation-safe replacement, and narrowly classified retry behavior for the
durable operation ledger.

### Modified Capabilities

None.

## Impact

This changes `src/operations.rs`, the `SurrealStorage` independent-connection
API, focused operation tests, and the installed server runtime. The
uncomfortable constraint is that the deployed backlog cannot be certified
until the repaired binary is installed and processes every accepted receipt;
passing isolated tests alone does not close the release task.
Loading
Loading