Commit Graph

2 Commits

Author SHA1 Message Date
Hongming Wang
1e97fb9a16 Memory v2 fixup C1: backfill idempotency via MemoryWrite.id
Self-review (post-merge) flagged that the backfill claimed to be
idempotent on re-run but actually duplicates every row because the
plugin's INSERT uses gen_random_uuid() and ignores any id passed in.

Fix is contract-level: extend MemoryWrite with an optional `id`
idempotency key. When supplied, the plugin MUST treat the write as
upsert keyed on this id; when omitted, the plugin generates a fresh
UUID (production agent commits keep working unchanged).

Changes:
  * docs/api-protocol/memory-plugin-v1.yaml: add id field with
    description that flags it as idempotency key
  * internal/memory/contract/contract.go: add ID to MemoryWrite struct,
    update memory_write_minimal golden vector
  * internal/memory/pgplugin/store.go: split CommitMemory into two
    paths — upsert when body.ID set (INSERT ... ON CONFLICT (id) DO
    UPDATE), plain INSERT otherwise
  * cmd/memory-backfill/main.go: pass agent_memories.id to MemoryWrite,
    fix the false comment about 409 deduplication

New tests:
  * pgplugin: TestCommitMemory_WithIDUpserts pins the upsert SQL is
    used when id is set; TestCommitMemory_UpsertScanError covers the
    error branch
  * backfill: TestBackfill_PassesSourceUUIDAsIdempotencyKey pins the
    forwarding behavior; TestBackfill_RerunIsIdempotent simulates a
    retry and asserts both runs pass the same uuid (plugin upsert is
    what makes this safe)

Why this matters: operators retrying a failed backfill (which they
will — networks fail, transactions abort) would otherwise create N
duplicates per memory. The duplicates aren't visible until search
results show obvious dupes — debugging that under prod load is bad.

Production agent commits are unaffected: they leave id empty, the
plugin generates a fresh UUID via gen_random_uuid(), zero behavior
change for the hot path.
2026-05-04 08:54:13 -07:00
Hongming Wang
53d823e719 Memory v2 PR-1: OpenAPI plugin contract + Go bindings
First of 11 PRs implementing the memory-system plugin refactor (RFC #2728).
This PR is pure additive scaffolding — no behavior change, no integration
yet. It defines the wire shape between workspace-server and a memory
plugin so PR-2 (HTTP client) and PR-3 (built-in postgres plugin) can be
built against a single source of truth.

What ships:
  - docs/api-protocol/memory-plugin-v1.yaml: OpenAPI 3.0.3 spec covering
    /v1/health, namespace upsert/patch/delete, memory commit, search,
    forget. Auth-free (private network only); workspace-server is the
    only sanctioned client and the security perimeter.
  - workspace-server/internal/memory/contract: typed Go bindings with
    Validate() methods on every wire object so both client (PR-2) and
    server (PR-3) self-check at the boundary.
  - Round-trip JSON tests for every type (catch asymmetric tag bugs).
  - 5 golden vector files under testdata/ pinning the exact wire shape;
    update via UPDATE_GOLDENS=1.

Coverage: 100% of statements in contract.go.

The validation rules encode design decisions worth flagging in review:
  - SearchRequest with empty Namespaces is REJECTED at plugin level —
    workspace-server is required to intersect the readable set
    server-side; an empty list reaching the plugin is a bug.
  - NamespacePatch with no fields is REJECTED — empty patches are
    pointless round-trips.
  - MemoryWrite with whitespace-only Content is REJECTED — zero-info
    memories pollute search results.

No code yet calls into this package; integration starts in PR-2.
2026-05-04 06:45:52 -07:00