docs(rfc): cross-channel conversation memory + platform identity - #100
Merged
Conversation
Contributor
Author
|
Round-1 Codex-Review surfaced structural findings (mismatched ConversationTurn shapes, service-registry key form, missing ctx.tenantId on TurnContextValue, ctx.notifications used as ops/audit primitive, schema integrity gaps). Closing while iterating on the branch — will reopen once the review verdict reaches 'merge as-is' / 'merge after small revisions'. No premature merge candidates in the repo. |
Contributor
Author
|
Reopening after four review rounds. Codex final verdict: merge as-is (no remaining structural or cosmetic findings). Round summary:
All findings were verified against the actual code in middleware/packages/plugin-api/, middleware/packages/harness-channel-sdk/, middleware/packages/harness-knowledge-graph-neon/, and middleware/src/plugins/manifestLoader.ts before applying fixes. No code changes in this PR — the contract is locked and ready for implementation PR 2 (platform-identity provider) to start. |
Driven by omadia-ui Tier-2 orchestrator's hard dependency on crossChannelConversationMemory@1 and the "Telegram commute → desktop office" continuity scenario. Per-channel ephemeral history today (InMemoryConversationHistoryStore, 10 turns / 2h / 500 LRU) cannot carry state across channels or restarts. The RFC proposes two capabilities — platformIdentity@1 and crossChannelConversationMemory@1 — with four provider plugins (Neon + in-memory siblings per capability), modelled on the harness-knowledge-graph-neon / -inmemory pattern. A new DurableConversationHistoryStore adapter in harness-channel-sdk bridges the contract; the existing ConversationHistoryStore interface stays unchanged, channels opt in per PR, missing capability falls back to today's in-memory behaviour. Adds docs/cross-channel-memory.md, a Phase 13 entry in §13 of middleware-agent-handoff.md, and a CHANGELOG entry under [Unreleased].
Round 1 left structural contract bugs that Codex flagged. Round 2 addresses them against the actual code in middleware/packages/: - Service-registry keys are bare names, not capability refs. The RFC now declares constants (PLATFORM_IDENTITY_SERVICE = 'platformIdentity', CROSS_CHANNEL_CONVERSATION_MEMORY_SERVICE = 'crossChannelConversationMemory') and reserves the '@1' suffix for manifest provides/requires only. Matches pluginContext.ts:226-229 and the existing KG plugin's KNOWLEDGE_GRAPH_SERVICE = 'knowledgeGraph' pattern (harness-knowledge-graph-neon/src/plugin.ts:61). - TurnContextValue does not carry tenantId or user identity today (turnContext.ts:34). The RFC now spells out the three additive optional fields (tenantId?, originatorUserRef?, originatorUserId?) that PR 4 lands together with the adapter, and notes that this absorbs the Phase-12 tenantId work. - The two ConversationTurn shapes in the SDK (stores.ts:15 with timestampMs? vs. inMemoryConversationHistory.ts:23 with required at) are now called out as pre-existing tech-debt; the adapter implements the canonical stores.ts contract and bridges to the legacy internal type at delegation time. - ctx.notifications is the wrong surface for ops/audit (it broadcasts cross-channel user notifications; pluginContext.ts:98-102, 520-526). Failure logging now goes through ctx.log; raw-access audit is a durable ccm_audit_events table written in-transaction with the audited operation. - platformIdentity@1 auto-merge on email is opt-in per tenant (pi_auto_merge_on_email, default false) and only triggers on email_verified=true. UNIQUE partial index + INSERT ... ON CONFLICT makes the resolver race-safe. Shared mailbox, email rename, and recycled email are documented edges. - cross_channel_messages gains materialised byte counters and a redaction_state column ('pending' | 'clean' | 'redacted'). Default reads exclude 'pending' rows to bound the post-insert/pre-redaction exposure window. GC gains a third pass for ccm_user_byte_cap. - Outbox is now specified: durable ccm_outbox table in the destination DB, with a process-local bounded fallback queue when the destination is unreachable, drained by a dedicated ccm-outbox job with exponential backoff and a 5-attempt DLQ. - PR sequence rephrased to source-mergeable with explicit deploy-prerequisite ordering, matching the kernel's boot-time `requires` check (pluginContext.ts:224-225). Each implementation PR carries the AGENTS-mandated .env.example / migration-ID / security-architecture / middleware-agent-handoff doc updates. - New §13 (Observability) lists the counter names, performance targets (appendTurn p99 <100ms, getRecentByUser p99 <50ms at limit<=20, gc <5min/1M turns), and the per-PR eval/test matrix. Plugin-internal registry exposed via /ccm/metrics; migrates when ctx.metrics lands. - §3.1 spells out the inmemory sibling's operational limits (single-process, lost-on-restart, not multi-pod). §12.3 documents that assistant quotes of forgotten content survive unless ccm_redact_on_persist is enabled. Status changed from 'Proposed' to 'Proposed (Round 2)'.
Round 2 left three blocker findings the Codex reviewer flagged.
Round 3 closes them against the real plugin-api / manifestLoader code:
- §13/§14.1 no longer invents `permissions.routes.allowed`. The
manifest loader (middleware/src/plugins/manifestLoader.ts:465-487)
extracts permissions for memory, graph, network, subAgents and llm
— there is no `permissions.routes` key today, and `RoutesAccessor`
(pluginContext.ts:464-472) registers routers with no permission
gate. /ccm/metrics is registered via the normal
`ctx.routes.register('/ccm', router)` path, with auth handled by
the plugin itself (static `CCM_METRICS_TOKEN` or operator-managed
reverse-proxy allow-list). The manifest fragment no longer carries
the fictional key.
- Outbox correctness is now spec'd end-to-end:
- `cross_channel_messages` gains a `client_message_id` UNIQUE
column. `appendTurn` is INSERT ... ON CONFLICT DO NOTHING; retries
from the outbox reuse the same ULID and are idempotent at the
storage layer.
- §6 introduces a structured `CcmAppendError` with
code = 'committed' | 'rejected' | 'timeout' | 'transport' and
optional `messageId`. The adapter branches on `code`:
'committed' is treated as success (no retry), 'rejected' is
permanent failure (no retry), 'timeout' / 'transport' go to the
outbox.
- §5.6 names the process-local fallback buffer honestly: bounded
in-memory ring, crash-lossy, overflow drops oldest. Counter
`ccm_outbox_local_dropped_total` lets operators alert. The
durable outbox in the destination DB is named as THE durability
boundary; the local buffer is only a brief-outage recovery aid.
- Audit-event retention and PII minimization (§8.4):
- `ccm_audit_events.detail` is bounded by an enumerated allow-list
of keys per `op`. Free-form turn content is never stored. The
capability impl validates `detail` shape on insert.
- `ccm_audit_retention_days` config (default 365) drives a new
pass-4 in `ccm-gc` that deletes expired audit rows.
- `forgetByUser` interaction with audit: 'read_raw' rows for the
forgotten user are deleted in the same transaction; 'forget_user'
and 'merge_identities' rows have their PII identifiers nullified
but the event itself is retained for compliance. New return field
`deletedAuditRows` on `forgetByUser`.
Partial findings also addressed in this round:
- §10.1 TurnContextValue extension grows a 4th optional field
`canvasSessionId?` so §10.4's read call is self-consistent. All
four new fields documented as optional with explicit
"tools / sub-agents / routines see undefined → degrade gracefully"
guidance.
- §10.3 spells out the resolveUserId cache: per-turn AsyncLocalStorage
Map<string, string> keyed on (tenantId, platformId), dropped on
turn exit. Group-chat multi-sender turns get one entry per ref.
- §3.1 gains a parity matrix listing exactly which durable behaviors
the in-memory sibling mirrors and which it skips (outbox doesn't
apply because the destination IS the process; audit is a ring
buffer with no retention guarantee; multi-pod unsupported).
- §11 tenant-assertion language trimmed — defense-in-depth catches
caller bugs, structural binding prevents cross-tenant reads. New
startup warning if `pi_tenant_id` and `ccm_tenant_id` disagree.
- §5.3, §5.5, §8.4 schema columns gain CHECK constraints for enums
and non-negative counters.
- §15.1 migration-ID column now cites concrete filenames
(`pi/0001_init.sql`, `ccm/0001_init.sql`) with a note that
implementers update the table when actual migrations land.
CHANGELOG entry updated to reflect the multi-round review and the
specific fixes that came out of it, satisfying AGENTS' rule that
significant doc changes carry a changelog entry in the same step.
Status header changed from 'Proposed (Round 2)' to 'Proposed (Round 3)'.
Round-3 verdict was "merge after small revisions". Round 4 closes
the three remaining nits:
- §8.4 allowed-key enum for op='forget_user' adds 'nullifiedAuditRows'
(referenced in §8.4.1 but missing from the enum, would have made the
capability impl's validator reject its own audit insert).
- §13.1 ccm_gc_deletes_total{pass} label set adds 'audit' (the
pass-4 audit-retention GC added in Round 3 was not covered by the
counter label, so the GC sweep would have lacked observability for
that pass).
- §6 CcmAppendError gains a doc-comment requiring full constructor
initialisation so implementers don't ship partially-initialised
exception instances.
Status: 'Proposed (Round 4 — merge-ready pending final Codex pass)'.
iret77
force-pushed
the
docs/cross-channel-memory-rfc
branch
from
May 18, 2026 16:35
8c1dbd3 to
9968dde
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
docs/cross-channel-memory.mdproposing two omadia core capabilities —platformIdentity@1andcrossChannelConversationMemory@1— together with four provider plugins (Neon + in-memory siblings per capability), modelled onharness-knowledge-graph-neon/harness-knowledge-graph-inmemory.requires: crossChannelConversationMemory@1to deliver the "Telegram commute → desktop office" continuity scenario.harness-channel-sdk: today'sConversationHistoryStorecontract stays unchanged. A newDurableConversationHistoryStoreadapter bridges to the capability and degrades to the existingInMemoryConversationHistoryStorewhen the capability is not installed, so channel plugins migrate one-by-one without breaking changes.docs/middleware-agent-handoff.md§13 gains a "Phase 13 — Cross-Channel Conversation Memory" roadmap entry pointing at the RFC; CHANGELOG records the proposal under[Unreleased] → Added.Key design decisions, opinionated
platformIdentity@1is intentionally separate fromcrossChannelConversationMemory@1so Slice 2.5 can replace the identity implementation without touching CCM consumers. Four packages by default; pragmatic fallback to two packages (one per backend) is called out as v1.0-acceptable.ccm-gccron job with TTL (90 days default) + per-user cap (10000 turns default).egressWalkerpipeline stays as-is. Async metadata job populatesredaction_metadata; reads default to redacted projection. Tenant-scopedccm_redact_on_persistflag available for strict-compliance setups.includeRawreads are admin-only and audit-logged.tenant_idNOT NULLon every table, every query bindsWHERE tenant_id = $1, fail-closed if missing. No admin override. Same operational model as the KG plugin, no RLS.PR sequence (this PR is #1, docs-only)
harness-platform-identity-*packages (Neon + in-memory) provideplatformIdentity@1harness-cross-channel-conversation-memory-*packages (Neon + in-memory) providecrossChannelConversationMemory@1,ccm-gcjobDurableConversationHistoryStoreadapter inharness-channel-sdkEach PR is independently mergeable. Full details in §14 of the RFC.
Test plan (PR 1 is docs-only)
docs/cross-channel-memory.mdrenders cleanly on github.com (table, fenced code, schema blocks).docs/middleware-agent-handoff.mdPhase 13 entry inlined before the closing---; existing phases unchanged.docs/CHANGELOG.mdentry under[Unreleased] → Addedin Keep-a-Changelog format, preceding the engineering-standards entry.