Skip to content

Harden runtime: providers, transport, HTTP contract, security, metrics - #1

Merged
AndrewMoryakov merged 4 commits into
masterfrom
claude/beautiful-maxwell-bRROr
May 27, 2026
Merged

Harden runtime: providers, transport, HTTP contract, security, metrics#1
AndrewMoryakov merged 4 commits into
masterfrom
claude/beautiful-maxwell-bRROr

Conversation

@AndrewMoryakov

@AndrewMoryakov AndrewMoryakov commented May 27, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes from a multi-scope code review of the runtime. Each fix was scoped to a disjoint set of files so changes don't overlap. No data migration; existing mem0 stores and access paths are unaffected (qdrant config/read paths unchanged; the scope-registry is a separate, rebuildable index).

Correctness

  • HTTP add contract — the HTTP surface now accepts the documented text field and synthesizes messages like the MCP/CLI adapters (previously {"text": ...} returned a KeyError/400).
  • Uniform input validation — HTTP query/body now validate against each operation's JSON schema (limit/enum/type/unexpected-field checks), matching MCP. Shared validator extracted to runtime/schema_validation.py.
  • mem0 add fallback — when mem0's response lacks a record, the fallback now attributes the write by id/text instead of returning an arbitrary pre-existing row (which could sync the wrong id to the registry).
  • reconcile — conflict detection now partitions by scope (user, agent, run), eliminating cross-user false positives and reducing the O(n²) pass to per-group.

Reliability / concurrency

  • mem0 provider caching — replaced @lru_cache on instance methods with per-instance caches, so a reconfigured provider rebuilds Memory and re-reads the API key (no stale backend, no instance leak).
  • owner-process proxyshould_proxy_to_api() now confirms ownership by matching os.getpid() against the recorded API pid, so a child that inherits AGENTMEMORY_OWNER_PROCESS=1 no longer touches the embedded backend directly.
  • proxy startup lock — the cross-process lock now wraps only the spawn; the up-to-20s health-wait runs outside it, so parallel clients don't serialize.
  • proxy auth errors — proxied 401/403 now surface an actionable "set AGENTMEMORY_API_TOKEN" message instead of a misleading validation error.
  • scope_registry — SQLite now uses WAL + busy_timeout/synchronous=NORMAL; a failed needs_rebuild status write is logged instead of silently swallowed.
  • atomic_io — the containing directory is fsynced after os.replace (POSIX) for true crash durability; gracefully skipped where unsupported.
  • clients — batch connect/disconnect/status/doctor now isolate per-target failures; a malformed existing client config yields an actionable error without clobbering the file.

Security

  • OAuth — PLAIN PKCE is rejected (S256 only); redirect_uri is validated by exact hostname (127.0.0.1/::1/localhost, rejecting @/suffix spoofing); /oauth/authorize is rate-limited.

Metrics

  • Bounded label cardinality (error types per op, model names) folding overflow into __other__ to prevent unbounded memory/series growth; histogram le labels render as plain numbers.

Test plan

  • Full suite: python -m unittest discover -s tests — 422 tests, no new failures (2 pre-existing env-dependent failures confirmed against the base commit).
  • Added focused regression tests for each fix (http_client, transport, operation_adapters, reconcile, clients, scope_registry, mem0).
  • Optional: live mem0 smoke (add → search → get) where OPENROUTER_API_KEY is configured.

Behavioral notes (not breakage)

  • HTTP input is now strictly validated like MCP (extra fields / bad enum / limit < 1 → 400).
  • OAuth clients relying on PLAIN PKCE must switch to S256.
  • The scope-registry auto-upgrades to WAL on first open (creates -wal/-shm sidecars); no action required.

https://claude.ai/code/session_01BKjtun7hVwxh6Hv3YLdrww


Generated by Claude Code

claude added 4 commits May 25, 2026 21:59
Fixes from a multi-agent system review (isolated, non-overlapping scopes):

- mem0: drop instance-method lru_cache (leak + stale backend on reconfigure);
  attribute post-add fallback to the actual write instead of an arbitrary row
- metrics: bound error-type/model label cardinality with __other__ overflow
- atomic_io: fsync parent directory after os.replace for crash durability
- reconcile: partition claims by scope so cross-user matches aren't false conflicts
- clients: per-target error isolation; malformed client config no longer aborts
  the batch or clobbers an unreadable file
- scope_registry: enable WAL + busy_timeout; surface status-write failures
- api/oauth: reject PLAIN PKCE (S256 only); exact-hostname redirect_uri check;
  rate-limit the authorize endpoint
- http_client/transport: scope the start lock to spawn only; verify owner by pid;
  map 401/403 to an actionable auth error

https://claude.ai/code/session_01BKjtun7hVwxh6Hv3YLdrww
- operation_adapters: HTTP `add` now accepts the documented `text` field and
  synthesizes `messages` like the MCP/CLI adapters (fixes KeyError on the
  published contract)
- HTTP query/body sources now validate against each operation's input schema
  (limit/enum/type/unexpected-field checks), matching MCP behavior
- extract the shared validator into runtime/schema_validation.py; mcp.py reuses
  it with no behavior change; lazy OPERATIONS import avoids a cycle

https://claude.ai/code/session_01BKjtun7hVwxh6Hv3YLdrww
The test mocks a localjson-like direct provider and expects the dispatcher to
reach the mocked memory_search, but it never pinned the transport. should_proxy_
to_api() then read the ambient active provider (mem0 by default), so the call
leaked into the owner-process proxy and failed (rc=2) wherever no API is running
-- e.g. the provider-certification CI job, which runs with no config. Mock
should_proxy_to_api to False so the test exercises the in-process path it intends.

https://claude.ai/code/session_01BKjtun7hVwxh6Hv3YLdrww
These surfaced because this is the first PR to run the CI/certification
workflows (they trigger on pull_request and push:main, but the default branch
is master), not from the hardening changes.

- provider-certification: test_localjson_provider (and mempalace, claude_memory)
  imported the harness only via `from tests.provider_contract_harness`, which
  fails under the cert runner's discover when `tests` isn't an importable
  package (PEP 660 editable install exposes only `agentmemory`). Mirror the
  robust top-level-first fallback already used by test_mem0_provider.
- ubuntu/windows test job: test_text_config_status_detects_configured_launcher
  hardcoded an absolute launcher path that only matches the author's install
  root, reading as stale everywhere else. Build it from expected_launcher_path()
  so the assertion is host-portable.

https://claude.ai/code/session_01BKjtun7hVwxh6Hv3YLdrww
@AndrewMoryakov
AndrewMoryakov merged commit 52e42bd into master May 27, 2026
3 checks passed
@AndrewMoryakov
AndrewMoryakov deleted the claude/beautiful-maxwell-bRROr branch May 28, 2026 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants