Expand x402 tracking with audit fields and ordered writes#39
Expand x402 tracking with audit fields and ordered writes#39ponderingdemocritus merged 2 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughThis PR adds x402 audit fields across schema, types, helpers, store, middleware, and app wiring; implements canonical JSON hashing and extractX402AuditFields; introduces per-record operation queuing in the tracking module; and includes schema/migration files and tests to persist and verify the new audit data. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant App as Facilitator App
participant Module as ResourceTrackingModule
participant Store as PostgresStore
Client->>App: request (payment payload + requirements)
App->>Module: startTracking(request)
Module->>Store: create(record)
Store-->>Module: created(id)
App->>Module: recordVerification(id, success, paymentDetails, undefined, x402Audit)
Module->>Module: enqueue per-record op (serialize)
Module->>Store: update(record with verification + x402Audit)
Store-->>Module: update result
App->>Module: finalizeTracking(id)
Module->>Module: enqueue per-record op (serialize)
Module->>Store: update(finalize)
Store-->>Module: update result
Module-->>App: done
App-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@examples/facilitator-server/src/app.ts`:
- Around line 226-239: The verify-error tracking call to safeTrack currently
passes undefined as the 3rd argument to module.recordVerification, which drops
payment details; update the safeTrack invocation that calls
module.recordVerification (the call with trackingId, false, undefined, ...) to
pass extractPaymentDetails(paymentPayload, paymentRequirements) instead of
undefined when both paymentPayload and paymentRequirements are present (i.e.,
replace the 3rd arg with a conditional that calls
extractPaymentDetails(paymentPayload, paymentRequirements) only when those two
variables are defined), keeping the existing error message and audit fields
logic intact.
In `@packages/core/src/tracking/helpers.ts`:
- Around line 164-186: canonicalizeJson currently serializes object properties
whose value is undefined as "key":null, which differs from JSON.stringify (it
omits such keys); update canonicalizeJson in
packages/core/src/tracking/helpers.ts to skip object keys whose raw value is ===
undefined when building the serialized array so objects with omitted-undefined
properties match JSON.stringify; specifically, inside canonicalizeJson where you
iterate Object.keys(value).sort(), check typeof value[key] !== "undefined" (or
value.hasOwnProperty(key) && value[key] !== undefined) before pushing the
`${keyPart}:${valuePart}` entry, leaving other logic (arrays, dates, primitives,
and isRecord checks) unchanged.
In `@packages/core/tests/unit/postgresTrackingStore.test.ts`:
- Line 5: The describe block name is incorrect: locate the describe call
currently using the string "PostgresResourceTrackingStore.update" in
packages/core/tests/unit/postgresTrackingStore.test.ts and change it to a more
accurate label such as "PostgresResourceTrackingStore" (or split into two
describes like "PostgresResourceTrackingStore.create" and
"PostgresResourceTrackingStore.update" so tests that exercise create vs update
are grouped correctly); update the describe title around the tests (the test
titled "persists expanded x402 audit columns during create" references create)
so the describe string matches the operations being tested.
🧹 Nitpick comments (6)
examples/facilitator-server/src/schema/tracking.ts (1)
56-59: Indexes are selective — verifypayment_valid_beforeandpayment_signature_hashare intentionally excluded.Four of the six new columns are indexed, but
payment_valid_beforeandpayment_signature_hashare not. This aligns with the PRD's guidance to "add only query-justified indexes," but worth confirming these two won't be used in filter/lookup queries. Ifpayment_valid_beforeis ever used for range queries (e.g., finding expired authorizations), an index would be beneficial.packages/core/src/middleware/core.ts (1)
265-274: Consider extracting partial audit fields on payment errors for forensic purposes.In the error path,
extractX402AuditFieldsis not called. While this is reasonable (the payload may be malformed), the PRD mentions forensic investigation as a goal. If the payload is parseable but verification fails (e.g., expired nonce, invalid signature), the audit fields could still be valuable for debugging. This is a low-priority consideration for a future iteration.packages/core/tests/unit/resourceTrackingModule.test.ts (1)
90-95: Sleep-based assertions are timing-sensitive.The
sleep(5)andsleep(10)delays work in practice but are inherently fragile in slow CI environments. If these ever flake, consider exposing a flush/drain mechanism on the module (or awaiting the internal queue directly) for deterministic assertions. Not blocking, just noting.examples/facilitator-server/drizzle/meta/0000_snapshot.json (1)
205-264: Consider whetherpayment_signature_hashshould also be indexed.Four of the six new columns have indexes (
x402_version,payment_nonce,payload_hash,requirements_hash), butpayment_valid_beforeandpayment_signature_hashdo not. If you anticipate querying by signature hash (e.g., for duplicate payment detection or forensic lookups), an index would be useful. Otherwise, the omission is fine to keep write overhead low.packages/core/src/tracking/helpers.ts (1)
161-162: MoveisRecordabove its first usage inextractPayerfor clarity.
isRecordis aconstarrow function defined at line 161 but referenced at line 98 insideextractPayer. This works at runtime (the function is called after module initialization), but placing the helper before its callers improves readability and avoids confusion about temporal dead zones.packages/core/src/tracking/postgres-store.ts (1)
334-340: Document the throwing behavior ofupdate()in JSDoc.The
RETURNING idcheck now throws when a tracking record is not found (line 338-340), which is a behavioral change from the previous silent no-op. This is intentional and properly tested (see postgresTrackingStore.test.ts), and all production callers in module.ts already wrap calls withsafeTrack(), which catches and logs exceptions without crashing. However, since the method's throwing behavior is now a contract, add JSDoc to document it explicitly for any future direct callers of the store.
1 participant
This PR expands resource tracking to persist additional x402 audit columns and hashes across core and facilitator-server schemas.
It threads the new audit extraction through verify/settle flows and tracking middleware.
It also adds per-record tracking operation ordering, clears stale verification errors on successful verification, and surfaces missing-row updates in the Postgres store.
Unit and server tests were added/updated for helper hashing, write ordering, Postgres updates, schema columns, and undefined-body handling.
Summary by CodeRabbit