Skip to content

Write-approval durability across multiple worker processes (gate 2: process-local store) #49

Description

@Xavier-SDK

Summary

execute_approved_write is gated by two independent checks. PR #48 fixes the first
(stateless hash — integral floats hashing differently). This issue is about the second,
distinct root cause of intermittent approval failures: the stateful session check relies
on process-local memory and breaks under multi-worker deployments.

This is a proposal for discussion, not a PR — the fix is larger and the trade-offs are
worth agreeing on before implementing.

Problem

server_core.require_validated_write_approval requires the token to be present in
AppContext.write_approvals — a plain dict populated by validate_write and consulted by
execute_approved_write. It lives in one process's memory, TTL 10 min
(WRITE_APPROVAL_TTL_SECONDS), never shared or persisted.

That's correct and sufficient for the common case (one process, one stdio/HTTP session). It
breaks for a deployment running multiple workers behind a load balancer without session
affinity
— plausible for any shared, multi-user MCP gateway. validate_write registers the
token on worker A; execute_approved_write for the same token lands on worker B, which never
saw it:

approval token has not been validated in this server session or has expired; call validate_write first

…on a fresh, unchanged token. It's a routing/topology problem, intermittent by nature (whichever
worker handles each call), easy to misdiagnose as a client bug.

Observed in practice on a multi-user Odoo 19 (JSON-2) production connector.

Two candidate designs (complementary, not exclusive)

Option 1 — Self-verifying token (HMAC-signed canonical payload) — recommended first

Make the token carry its own proof instead of being a lookup key into per-process state.

  • Server-side secret (ODOO_MCP_APPROVAL_SIGNING_KEY, generated+persisted, or operator-supplied
    for multi-worker).
  • validate_write folds hmac_sha256(key, canonical_json(payload)) + expires_at into the
    token, e.g. odoo-write:{payload_hash}:{expires_at}:{hmac_sig}.
  • execute_approved_write recomputes the HMAC from the canonical payload + embedded expires_at
    and compares — no dict lookup. Expiry still bounded by WRITE_APPROVAL_TTL_SECONDS.

Pros: identical on 1 or 100 workers; no shared infra, no new dependency; no cross-process race;
small local change living in agent_tools.py (pure functions, consistent with its no-side-effects
contract).
Cons / open questions: loses consume-once (a valid token can be replayed within its TTL —
matters for create, which would duplicate a record; write replays are effectively no-ops).
Needs key management (rotation invalidates outstanding tokens — 10-min blast radius; key stays
server-side, same trust boundary as Odoo creds). Longer token (cosmetic).

Option 2 — Shared approval store (Redis / SQLite) with in-memory fallback

Move AppContext.write_approvals into a store every worker can reach, keeping today's dict as the
default (no behavior change for single-process/local).

  • WriteApprovalStore protocol: put(token, record, ttl) / get(token) / pop(token).
  • InMemoryWriteApprovalStore (default, wraps current dict) + RedisWriteApprovalStore
    (SETEX/GET/DEL) or SqliteWriteApprovalStore (table + expires_at), behind an optional
    extra (pip install odoo-mcp[redis]).
  • Selected via env, e.g. ODOO_MCP_APPROVAL_STORE_URL (unset → in-memory; redis://…;
    sqlite:///…) — mirrors existing env-gated optional behavior.

Pros: preserves consume-once; gives an inspectable record of outstanding approvals (audit,
complements ODOO_MCP_AUDIT_LOG); smallest mental leap from today's design; zero risk to the
default path.
Cons / open questions: new optional dependency + failure mode (store unreachable → fail closed
vs. fall back to in-memory with a loud warning? fail-closed matches the project's write posture);
ops burden (provision/monitor/secure); larger test surface (partitions, restarts mid-TTL).

Recommendation for discussion

Option 1 is the smaller, lower-risk change and solves the reported multi-worker failure with no new
infrastructure — worth doing first regardless. Option 2 is worth it separately if consume-once /
auditable-store semantics matter for a deployment. They compose: option 1 could even become the
signed transport for option 2's record.

A fuller write-up (with code sketches) is in docs/write-approval-durability.md on my branch; happy
to open a follow-up PR for whichever direction you prefer. Filed separately from #48 so the two
independent root causes of intermittent approval-token failures — payload-hash instability (fixed in
#48) vs. cross-process approval visibility (this issue) — stay tracked apart.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions