Add opt-in project adoption receipts#1912
Conversation
| const adoptionKey = `sha256:${sha256( | ||
| `dkg-adoption-receipt-v1\0${event.type}\0${contextGraphId}\0${nodeIdHash}`, | ||
| )}`; | ||
| const receiptId = `sha256:${sha256( |
There was a problem hiding this comment.
🔴 Bug: Adoption upsert keys collide across networks
What's wrong
The telemetry contract tells receivers to UPSERT by adoptionKey, but the key omits the receipt's network. If the same context graph id can exist on more than one network, receipts from different networks collapse into one adoption record.
Example
A node with the same Peer ID syncs example-project on testnet and mainnet. Both receipts have different network fields but the same adoptionKey, so a receiver following the documented UPSERT-by-adoptionKey contract merges the two network-specific adoptions into one record and can undercount or overwrite per-network activity.
Suggested direction
Include the network or a canonical chain/network id in the adoption key, or otherwise make the receiver contract explicitly network-agnostic and avoid storing network as a single per-key attribute.
Confidence note
This depends on whether contextGraphId is guaranteed globally unique across every DKG network. The receipt carries network, which suggests receivers are expected to distinguish networks, but the stable upsert key currently cannot.
For Agents
In packages/cli/src/adoption-telemetry.ts, update buildAdoptionTelemetryReceipt so the stable adoptionKey includes a normalized network/chain identifier when available, and preserve retry/idempotency behavior for the same occurrence. Add a focused test proving identical event/contextGraphId/node on two networks produces different adoption keys, while repeated occurrences on the same network keep the same adoption key and different receipt IDs.
| }; | ||
| const plan = planInstallImpl({ ...ctx.context, manifest }); | ||
| const written = await writeInstallImpl(plan); | ||
| const trackingQueued = written.length > 0 && (adoptionTelemetry?.enqueue({ |
There was a problem hiding this comment.
🟡 Issue: Adoption tracking is leaking into the manifest route layer
What's wrong
The route handler now knows the telemetry event names, queue result vocabulary, and sink shape. That tangles an already-large routing module with an optional reporting feature and duplicates the install-success side effect across two HTTP paths. The codebase already uses the daemon event bus for PROJECT_SYNCED; install telemetry can follow the same ownership model instead of pushing feature-specific branches through shared request context.
Example
The same domain fact, a manifest install completed for a context graph, now appears as inline route logic in /manifest/install and as a separate /manifest/install-receipt endpoint used by DkgClient.recordProjectInstall. Any future change to adoption receipt semantics has to touch the HTTP route layer and client surface instead of one telemetry/domain boundary.
Suggested direction
Treat install_completed as a domain event or a small dedicated recorder owned by adoption telemetry, then let the route only report that the install completed. That would remove telemetry-specific plumbing from RequestContext, keep the 2k-line context-graph route from accumulating another concern, and give the standalone join path a single boundary that records an install without knowing receipt semantics.
For Agents
Look at packages/cli/src/daemon/routes/context-graph.ts, packages/cli/src/daemon/routes/context.ts, and packages/cli/src/daemon/lifecycle.ts. Preserve the current responses if they are needed, but move adoption-specific event construction behind a dedicated install-completion recorder or daemon event listener. Add coverage proving both UI install and standalone dkg-mcp join still queue/disable tracking exactly as today.
| sharedMemorySynced?: number; | ||
| } | ||
|
|
||
| export interface ResolvedAdoptionTelemetryConfig { |
There was a problem hiding this comment.
🟡 Issue: Resolved telemetry config should encode the enabled invariant
What's wrong
The current type boundary allows impossible states, so the implementation compensates with repeated runtime checks and a non-null assertion. That makes the core contract harder to reason about and spreads resolver assumptions into the transport class.
Example
{ enabled: true, timeoutMs: 1000, maxAttempts: 1 } is representable as a ResolvedAdoptionTelemetryConfig, so the reporter has to defensively check for a state the resolver is meant to make impossible. That is why deliver() needs a non-null assertion.
Suggested direction
Make the resolved type closed: either disabled with optional warning, or enabled with a required endpoint. An even cleaner cut would be createAdoptionTelemetrySink(...) returning a no-op sink for disabled config and a reporter for enabled config. Then enqueue() and deliver() no longer need to rediscover or assert the endpoint invariant.
For Agents
In packages/cli/src/adoption-telemetry.ts, replace the boolean-plus-optional shape with a discriminated union, or expose a factory that returns either a no-op sink or a reporter configured with a required endpoint. Preserve env/config precedence and delivery behavior, and keep tests covering disabled, missing endpoint, and enabled cases.
|
|
||
| agent.eventBus.on(DKGEvent.PROJECT_SYNCED, (data: any) => { | ||
| try { | ||
| adoptionTelemetry.enqueue({ |
There was a problem hiding this comment.
🟡 Issue: PROJECT_SYNCED adoption telemetry wiring is unverified
What's wrong
One of the two new documented telemetry signals depends on this daemon event-bus hook, but the added tests do not exercise this hook. The current coverage proves the reporter can send a context_graph_synced receipt if called, but not that a clean Context Graph catch-up actually calls it.
Example
A regression that changes this listener to enqueue install_completed, omits dataSynced, or removes the enqueue call entirely would still leave the current adoption telemetry unit tests green because they never exercise the daemon PROJECT_SYNCED wiring.
Suggested direction
Cover the production event-bus integration, not just receipt construction, so the documented sync telemetry signal cannot silently disconnect.
For Agents
Add a lifecycle-level regression test or extract a small registration helper for this listener. Preserve the existing SSE/notification behavior, emit a fake DKGEvent.PROJECT_SYNCED payload, and prove adoption telemetry receives { type: 'context_graph_synced', contextGraphId, dataSynced, sharedMemorySynced }.
| try { | ||
| const headers: Record<string, string> = { | ||
| 'Content-Type': 'application/json', | ||
| 'Idempotency-Key': receipt.receiptId, |
There was a problem hiding this comment.
🟡 Issue: Retry idempotency is not asserted across attempts
What's wrong
The receipt contract says HTTP retries must reuse the same receipt so receivers can deduplicate retry traffic. The tests cover retry count and single-request header/body consistency, but they would still pass if each retry generated a new receipt and idempotency key.
Example
A failing-test sketch: make the first fetch return 503 and the second 204, capture both Idempotency-Key headers and JSON bodies, then assert both attempts use the same receiptId and identical Idempotency-Key.
Suggested direction
Add a retry-specific assertion that all attempts for one delivery reuse the same receiptId and Idempotency-Key.
For Agents
In packages/cli/test/adoption-telemetry.test.ts, extend the transient retry case or add a focused case that captures every fetch init during retries and proves the same occurrence receipt is reused across attempts.
Summary
Add an opt-in, fail-open adoption receipt pipeline for project manifest installs and verified Context Graph sync completions.
Why
A plain request counter cannot distinguish a new installation from a retry or a recurring sync. Nodes already have a persistent libp2p Peer ID, so this change derives a stable pseudonymous node key locally and gives the collector enough information to deduplicate nodes while retaining activity history.
What changed
install_completedandcontext_graph_syncedreceipt typesnodeIdHashfrom the persistent Peer ID without transmitting the raw IDadoptionKeyfor each event/Context Graph/node tuplereceiptIdas the HTTP idempotency keydkg-mcp joinpathsPROJECT_SYNCEDcompletion signalImpact
Repeated syncs refresh
lastSeenand may increment a sync occurrence counter without inflating the distinct participating-node count. Delivery failures never change install or sync success.This PR implements the node-side producer and wire contract. A central collector still needs to deploy the documented idempotent upsert endpoint and opt-in nodes must configure it.
Privacy and security
Receipts exclude the raw Peer ID, wallet address, workspace path, and daemon token. Remote collectors must use HTTPS; HTTP is accepted only for loopback development endpoints.
Validation
pnpm --filter @origintrail-official/dkg buildgit diff --check