Support status checks via workflows. #152

wes-smith · 2025-06-04T16:17:30Z

This PR adds two features:

Added support for verifyPresentationOptions to appear on exchange steps,
allowing for more extensible and general step option expression.
Added support for verifyPresentationResponseSchema to appear on exchange
steps, allowing for enforcing validation constraints at the workflow layer.

codecov-commenter · 2025-06-04T16:21:20Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.69%. Comparing base (78fb952) to head (36ca606).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #152      +/-   ##
==========================================
+ Coverage   82.38%   82.69%   +0.31%     
==========================================
  Files          15       15              
  Lines        4529     4611      +82     
==========================================
+ Hits         3731     3813      +82     
  Misses        798      798

Files with missing lines	Coverage Δ
lib/helpers.js	`85.32% <100.00%> (+1.60%)`	⬆️
lib/oid4/oid4vp.js	`83.97% <100.00%> (+0.25%)`	⬆️
lib/vcapi.js	`76.89% <100.00%> (+0.44%)`	⬆️
lib/verify.js	`69.12% <100.00%> (+1.71%)`	⬆️
schemas/bedrock-vc-workflow.js	`100.00% <100.00%> (ø)`

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 78fb952...36ca606. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

tminard

A few questions / suggestions.

The new tests look a little gnarly but I do not have specific suggestions yet.

tminard · 2025-06-04T16:47:17Z

lib/helpers.js

+  if(verifyPresentationOptions.checks !== undefined) {
+    for(const check in verifyPresentationOptions.checks) {
+      if(verifyPresentationOptions.checks[check] == true) {
+        if(!checks.includes(check)) {
+          checks.push(check);
+        }
+      }
+    }
+  }
+  options.checks = checks;


Suggested change

if(verifyPresentationOptions.checks !== undefined) {

for(const check in verifyPresentationOptions.checks) {

if(verifyPresentationOptions.checks[check] == true) {

if(!checks.includes(check)) {

checks.push(check);

}

}

}

}

options.checks = checks;

const checksSet = new Set(checks);

if(verifyPresentationOptions.checks) {

for(const [check, enabled] of Object.entries(verifyPresentationOptions.checks)) {

if(enabled) {

checksSet.add(check);

}

}

}

options.checks = Array.from(checksSet);

I'm not sure what's being checked with verifyPresentationOptions.checks[check].

tminard · 2025-06-04T16:48:46Z

lib/verify.js

    result = await zcapClient.write({
      capability,
      json: {
-        options: {
-          // FIXME: support multi-proof presentations?


Does this FIXME need to be preserved?

Hmm, no, I'd say we can remove it. If we want to support that in the future we can figure out where to add it (which may even be in a different spot or something).

tminard · 2025-06-04T16:50:12Z

lib/verify.js

+  // validate against the verify presentation result schema, if applicable
+  if(verifyPresentationResultSchema) {
+    const {jsonSchema: schema} = verifyPresentationResultSchema;
+    const validate = compile({schema});


Should this be compiled prior to make the zcap call to ensure it fails as early as possible if there's something invalid here?

I'm not sure how much earlier it can actually be compiled. Only once the step executes are you guaranteed to have all the variables to construct the step (if it is a template itself, which is common). Therefore, there might not be any earlier time this can happen. We do want, in the future, some better "test" tools for this sort of thing if we can figure out how to accomplish that nicely.

tminard · 2025-06-04T16:50:41Z

lib/verify.js

+    const validate = compile({schema});
+    const {valid, error} = validate(result.data);
+    if(!valid) {
+      throw error;


Should this be wrapped or provide some logging in case of failure here?

It's hard to tell from the diff so I'll need to go look at the source, but I believe this follows the pattern we have with the presentation schema, which is wrapped/handled properly now (IIRC). If this is in the same code path/outer wrapper somewhere then this is ok. Will need to go look and confirm.

dlongley

Reviewed most of this modulo the test, will take a look at that later, LGTM so far.

dlongley · 2025-06-05T02:04:08Z

lib/helpers.js

+  // update 'checks' with anything additional from 'verifyPresentationOptions'
+  if(verifyPresentationOptions.checks !== undefined) {
+    for(const check in verifyPresentationOptions.checks) {
+      if(verifyPresentationOptions.checks[check] == true) {
+        if(!checks.includes(check)) {
+          checks.push(check);
+        }
+      }
+    }
+  }
+  options.checks = checks;


I think I like Tyler's suggestion better (seems more readable), but if you were looking for a functional way to do it, it could look like this:

Suggested change

// update 'checks' with anything additional from 'verifyPresentationOptions'

if(verifyPresentationOptions.checks !== undefined) {

for(const check in verifyPresentationOptions.checks) {

if(verifyPresentationOptions.checks[check] == true) {

if(!checks.includes(check)) {

checks.push(check);

}

}

}

}

options.checks = checks;

options.checks = checks;

// update 'checks' with anything additional from 'verifyPresentationOptions'

if(verifyPresentationOptions.checks) {

options.checks = [...new Set([

...options.checks,

...Object.entries(verifyPresentationOptions.checks)

.filter(([, v]) => v).map(([k]) => k)]

])];

}

dlongley · 2025-06-05T02:06:19Z

lib/verify.js

    result = await zcapClient.write({
      capability,
      json: {
-        options: {
-          // FIXME: support multi-proof presentations?


Hmm, no, I'd say we can remove it. If we want to support that in the future we can figure out where to add it (which may even be in a different spot or something).

dlongley · 2025-06-05T02:08:20Z

lib/verify.js

+  // validate against the verify presentation result schema, if applicable
+  if(verifyPresentationResultSchema) {
+    const {jsonSchema: schema} = verifyPresentationResultSchema;
+    const validate = compile({schema});


I'm not sure how much earlier it can actually be compiled. Only once the step executes are you guaranteed to have all the variables to construct the step (if it is a template itself, which is common). Therefore, there might not be any earlier time this can happen. We do want, in the future, some better "test" tools for this sort of thing if we can figure out how to accomplish that nicely.

dlongley · 2025-06-05T02:09:29Z

lib/verify.js

+    const validate = compile({schema});
+    const {valid, error} = validate(result.data);
+    if(!valid) {
+      throw error;


It's hard to tell from the diff so I'll need to go look at the source, but I believe this follows the pattern we have with the presentation schema, which is wrapped/handled properly now (IIRC). If this is in the same code path/outer wrapper somewhere then this is ok. Will need to go look and confirm.

dlongley

Minor nits here -- I think all style / maintenance related. Will approve and merge in once those are addressed. Thanks!

dlongley · 2025-06-05T18:00:43Z