fix(cmc): readOffer must omit streams filter — :_cmc:_internal:offer is not a real parent#68
fix(cmc): readOffer must omit streams filter — :_cmc:_internal:offer is not a real parent#68perki wants to merge 1 commit into
streams filter — :_cmc:_internal:offer is not a real parent#68Conversation
…r is not a real parent After the previous `streamIds → streams` fix, readOffer still fails against any real api-server with `unknown-referenced-resource`: the api-server validates that streams referenced in events.get actually exist, and `:_cmc:_internal:offer` is NOT auto-provisioned on every user account — only the per-capability children `:_cmc:_internal:offer:<capId>` are minted by capabilityMintHook. The accepter doesn't know <capId> from the capabilityUrl alone, so a specific-stream filter isn't possible either. The plugin's own implementation of the same read (`readOfferViaCapability` in acceptOrchestration.ts:71-78) handles this correctly: omit the streams filter entirely and rely on the capability access's permissions to narrow the response to the single offer event this token can read. Mirror that approach. Keep the `types: [ET_REQUEST]` filter as defense in case future plugin revisions place additional event types on the offer stream. Verified end-to-end on demo: readOffer now returns the offer with `requestedPermissions`, `consent`, etc. No new tests in this PR — the existing cmc.test.js mocks Connection.apiOne, so neither the field-name bug nor this parent- stream-doesn't-exist bug is caught at unit-test time. Both surface only against a real api-server.
The contract test was asserting the buggy intermediate shape from PR #67 (`streams: [':_cmc:_internal:offer']`). PR #68 corrected that to omit the streams filter entirely (parent stream isn't auto-provisioned). Update the assertion to match the final correct wire-shape: no `streams`, no `streamIds`, with `types: ['consent/request-cmc']` as the only filter.
|
Merged into master at e11d080 (3 commits: 7b7fe2a fix + f84b663 test + e11d080 release). Also updated the existing [CMCL1OA] contract test (added in 1.0.1 — which asserted the buggy intermediate shape from PR #67) to match the final correct shape: no End-to-end validation against deployed pryv.me: Patch release just published: Thanks for the rapid follow-up + the clean repro. |
|
Correction to my previous comment: the patch release commit is on master (e11d080) but npm publish hasn't happened yet — handing back to the user for that step. Sorry for the confusion. |
Follow-up to #67. After fixing
streamIds → streams,cmc.readOfferstill throws against any real api-server:Root cause
The api-server validates that streams referenced in
events.getactually exist on the user's account.:_cmc:_internal:offeris not auto-provisioned — only the per-capability children:_cmc:_internal:offer:<capId>are minted bycapabilityMintHook. The accepter doesn't know<capId>from thecapabilityUrlalone, so a specific-stream filter isn't possible either.The plugin's own implementation of the same read —
readOfferViaCapabilityincomponents/cmc/src/acceptOrchestration.ts:71-78of open-pryv.io — handles this correctly: omit thestreamsfilter entirely and rely on the capability access's permissions to narrow the response to the single offer event this token can read.Fix
readOffermirrors that pattern: drop thestreamsfilter, keeptypes: [ET_REQUEST]as defense.Test
No new tests in this PR — the existing
cmc.test.jssuite mocksConnection.apiOne, so neither the field-name bug (PR #67) nor this parent-stream-doesn't-exist bug is caught at unit-test time. Both surface only against a real api-server. The downstream behaviour is exercised end-to-end via HDS plan-59 Phase-3 scenario tests against a deployed open-pryv.io —cmc.readOfferreturns the offer correctly post-fix.Happy to add a wire-shape contract test (asserting
events.getis called with nostreamsfield and withtypes: ['consent/request-cmc']) if you'd like it landed alongside the fix.