Skip to content

feat(007): generic credential rail — make defineCredential complete on the phone (#19)#42

Open
TheBlackBit wants to merge 6 commits into
mainfrom
007-generic-credential-rail
Open

feat(007): generic credential rail — make defineCredential complete on the phone (#19)#42
TheBlackBit wants to merge 6 commits into
mainfrom
007-generic-credential-rail

Conversation

@TheBlackBit

@TheBlackBit TheBlackBit commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

What this does

Lets you gate a purchase (or any action) with any credential you define via defineCredential — and actually prove it on the phone. Until now only the built-in age and membership credentials worked end-to-end; a custom credential showed up to the agent but couldn't be completed at checkout. Now it can. Closes #19.

What changed

  • The ceremony serves any credential. The checkout ceremony now builds the wallet request and runs the check from the credential's own definition — not just the two built-ins.
  • Checkout enforces it. An order can't complete until its required credential is proven, on every payment path — not by hiding a button.
  • No extra setup. You still just pass the credential by object into the policy; the gate wires it up internally.

Try it

node examples/professional-license.mjs
  • "Buy the contractor drill" → asks for a professional license and blocks checkout until it's proven.
  • "Buy the headphones" → no license needed.

Status

Design notes live in specs/007-generic-credential-rail/.

🤖 Generated with Claude Code

@TheBlackBit TheBlackBit changed the title docs(007): spec — generic credential rail (make defineCredential complete on the phone) (#19) feat(007): generic credential rail — make defineCredential complete on the phone (#19) Jul 9, 2026
Ever Morales and others added 2 commits July 9, 2026 12:53
Spec-only design deliverable for issue #19 — generalize the credential-gate rail
so `defineCredential`'s "no new code path" promise (ARCHITECTURE.md) becomes
literally true for an arbitrary mdoc credential, and enforce any applicable
`gate()` credential at completion (not only age).

Design (Approach 1 — register-on-resolve): an in-process credential registry
populated synchronously in `requirements()` (no public API change, so
Principle V's "no registration step" holds), injected at `mount()` into the
rail + `completeOrder`. Completion re-derives applicable `gate()` credentials
from the RE-PRICED order (invariant 2) and refuses any not in the per-order
`verifiedGates` map (invariant 1).

Scope: runtime generalization + one worked pack (professional-license gate()).
Out of scope (separate follow-ups): SD-JWT VC, broader pack catalog,
real-wallet e2e / dev test-issuer, issuer-verified trust (#14).

Artifacts under specs/007-generic-credential-rail/: spec, plan, research,
data-model, contract, quickstart, tasks, requirements checklist. A
/speckit-analyze pass was applied (F1 spec self-consistency on gate()-always-
blocks; F2 mount-level wiring test; F3 amount-binding; F4 trust_level asserts).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Ever Morales <ever.morales@koombea.com>
…s on the phone (#19)

Make ARCHITECTURE.md's "A new credential — no new code path" literally true for an
arbitrary mdoc credential, and enforce any applicable gate() credential at completion
(not only age). Implements specs/007-generic-credential-rail (T001–T031).

Registry (register-on-resolve): CredentAgent holds an in-process Map<string,Credential>
populated synchronously in requirements() — no public API change, Principle V's "no
registration step" preserved — and mount() injects it into the ceremony context +
publishes it on app.locals so a host's completion seam reaches it.

Rail: the credential-gate rail retires the closed "age" | "membership" dispatch. For a
custom id it builds the wallet request from the credential's OWN request DCQL
(buildSignedRequestForDcql / mdocDocSpecFromDcql) and runs its OWN verify (evaluateCustom)
across the instant-demo and OpenID4VP/mdoc paths; page.ts renders from ui + a derived
demo claim. Built-in age/membership stay on their existing order-parameterized path
(threshold/percent preserved — no regression).

Completion (security core, invariant 1): completeOrder sweeps the injected registry for
applicable gate()-effect credentials, re-derives applicability from the RE-PRICED order
(invariant 2), and refuses any not in the per-order verifiedGates map (invariant 4) with
reason "gate". gate() is the hard-block effect — enforced whenever it applies, independent
of required/optional (F1). Reserved built-ins excluded. Absent registry ⇒ additive no-op.

Storefront: the completion closure passes the published registry to completeOrder
(read lazily from app.locals) so the mounted flow enforces custom gates end-to-end.

Pack + docs: examples/professional-license.mjs (a gate() proven + enforced end-to-end);
custom-credential.mjs footer + examples/README + ARCHITECTURE.md + the gate README
corrected to the now-true behavior.

Tests (257 green: 188 gate + 69 storefront). New bypass suites, each verified RED when
its control is removed:
 - generic-credential.test.ts (US1): custom request/verify served; positive records,
   negative doesn't; unknown/reserved id → 404 (FR-013); presence-only-demo (F4).
 - completion.test.ts (US2): unverified applicable gate refused (reason "gate"); verify →
   complete; per-order scoping; non-applicable no-block; amount-binding with a custom gate
   (F3); additive no-op without a registry.
 - mount.test.ts (US2 F2): mount() publishes the registry so a host completion seam
   enforces — catches a registry-injection mis-wire the unit test (which seeds the
   registry directly) cannot.

Incidental: jose 6 dropped `KeyLike`; fixed the ES256 SignJWT cast in credential-gate/
and dc-payment/request.ts so `npm run build` (tsc) is green again.

Honesty: every custom surface stays trust_level "presence-only-demo" (no issuer/device
trust anchor yet — #14). Multi-instance: register-on-resolve needs each instance to
resolve the policy once (documented); single-server/demo always does.

Out of scope (separate follow-ups): SD-JWT VC, broader pack catalog, real-wallet e2e /
dev test-issuer, issuer-verified trust.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Ever Morales <ever.morales@koombea.com>
@TheBlackBit TheBlackBit force-pushed the 007-generic-credential-rail branch from 28f96a4 to d37fdce Compare July 9, 2026 18:53
@TheBlackBit TheBlackBit requested a review from dzuluaga July 9, 2026 18:58
@dzuluaga

dzuluaga commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔴 Blocker — the checkout hub renders a custom gate as an age gate

@TheBlackBit — running the shipped examples/professional-license.mjs and opening /checkout for the licensed drill, the gate card reads "This order contains age-restricted items. Verify you're 21 or older" with a "Verify age (21+)" button and a "Payment is locked · unlocks after age verification" banner. None of that is about the professional license the order actually requires.

⬆️ screenshot: /checkout?order=ORD-… for the ProForce Hammer Drill (licensed trade) — rendered as an age gate.

Why it happens — the backend was generalized, the page layer wasn't. The manifest is correct: the entry carries credential: "professional_license", label: "Professional license", and the right approveUrl (/credentagent/credential?cred=professional_license). But renderGate in packages/credentagent-gate/src/ceremony/checkout-page.ts:271 (a file this PR does not touch) has only two branches — discount, then everything else assumed to be age:

// gate effect (age):
const age = entry.minAge ?? 21;
...
🔒 This order contains age-restricted items. Verify you're ${age} or older to continue.
<a ...>Verify age (${age}+)</a>

It never reads entry.label. Any gate() that isn't a discount falls here, and with no minAge it defaults to 21. Same root cause hits railSteps (only emits age/membership steps → the rail shows just "Pay") and the lock banner copy.

It's not just cosmetic — it deadlocks. Even after the buyer proves the license, this page can never unlock: isSatisfied (checkout-page.ts:169) returns false for any non-age gate, and the storefront never passes verifiedGates into the render (packages/credentagent-storefront/src/server.ts:668 builds RenderVerification as only { ageVerified, loyaltyApplied }). So blocked stays true, the pay methods never render, and the flagship flow can't complete through its own UI. (The button link is correct — it does go to the license ceremony — only the visible copy and the lock are wrong.)

Fix is contained to the presentation layer — no backend/enforcement change needed:

  • renderGate should render entry.label + the credential's own copy instead of assuming age;
  • railSteps should emit a step for any gate() effect;
  • the storefront + isSatisfied + RenderVerification need to consult verifiedGates so a proven custom gate clears the lock.

This is the second of two blockers from a fuller review of this PR; the companion (the first) is the completion-time appliesTo projection dropping every OrderLine field except minimumAge/category, which fails open for a requiresRx-keyed gate — happy to post that separately with its repro.

(edited to fix #-number auto-links that pointed at unrelated issues/PRs — these are review findings, not issue references.)

@dzuluaga

dzuluaga commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔴 Follow-up to the checkout-hub blocker — running the example end-to-end, the license flow can't complete (and "Verify your license (instant demo)" appears to do nothing)

@TheBlackBit — I built and ran the shipped examples/professional-license.mjs and drove the full flow. The credential page renders correctly ("Professional license" / "Present your professional license"), but clicking Verify your license (instant demo) appears to do nothing. It isn't the button — it's the same hub gap as above, and here's the live trace.

The verify call succeeds server-side. Replaying the exact body the instant-demo button POSTs, against a real order:

POST /credentagent/credential/verify
  { "order":"ORD-2ghdkt", "cred":"professional_license", "claims":{"license_active":true} }
→ 200 {"verified":true,"gates":[{"gate":"Professional license","pass":true,"detail":"claim verified"}],"trust_level":"presence-only-demo"}

It also records verifiedGates.professional_license = true (via recordVerifiedGate). So the credential is proven and persisted.

Then done() redirects to /checkout?order=…, which ignores the proof. Fetching that checkout page for the same, now-verified order:

GET /checkout?order=ORD-2ghdkt   (verifiedGates.professional_license === true)
  contains "Payment is locked"       : yes
  contains "Verify age (21+)" button : yes
  contains any pay method            : NO
  contains a satisfied/✓ state       : NO

So the flow loops: prove license → redirect to checkout → checkout can't see verifiedGates → back to the locked "Verify age (21+)" screen. From the buyer's seat the instant-demo button "does nothing." Root cause is exactly the hub gap in the comment above — packages/credentagent-storefront/src/server.ts:668 builds RenderVerification as only { ageVerified, loyaltyApplied }, and checkout-page.ts isSatisfied/renderGate/railSteps have no custom-gate branch — so a proven custom gate can never clear blocked.

Second, separate wrinkle found while reproducing: hitting /credentagent/credential/verify on a process that hasn't resolved the policy yet returns 404 {"verified":false,"error":"unknown credential"}resolveCred looks the credential up in the register-on-resolve registry, so on a cold instance even a real presentation is rejected as unknown. (Fail-closed sibling of the registry lifecycle issue; matters for multi-instance/serverless.)

Repro (self-contained):

git worktree add ../credentagent-pr42 origin/007-generic-credential-rail
cd ../credentagent-pr42 && npm install
npm run build -w @openmobilehub/credentagent-gate -w @openmobilehub/credentagent-storefront
PORT=3011 GATE_SECRET=demo node examples/professional-license.mjs
# checkout the contractor-drill → open the credential link → click "Verify your license (instant demo)"
# → you land back on the locked "Verify age (21+)" checkout.

Ask: this is the flagship example, and it doesn't complete through its own UI. The backend seam is correct and well-tested, but nothing drives the browser hub — I couldn't find an end-to-end test that checkouts a custom-gate order, proves the credential, and asserts a pay button appears. One such test would have caught both this and the age-mislabel, and would keep the "257 tests green" from masking a broken demo. Fix is presentation-layer only (teach server.ts + isSatisfied/renderGate/railSteps to read verifiedGates + entry.label); happy to open a PR against your branch if useful.

@dzuluaga

dzuluaga commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🔴 Blocker (security) — completion enforcement fails open for any appliesTo that reads a field other than minimumAge/category

@TheBlackBit — this is the most severe finding: an applicable custom gate() can be surfaced to the buyer and then completed with no credential proof, violating security invariant 1. It hits the README's own prescription example directly.

Root cause. At completion, the custom-gate sweep re-derives appliesTo against a hand-built projection of the re-priced order that forwards only five fields — packages/credentagent-gate/src/ceremony/completion.ts:

lines: repriced.lines.map((l) => ({
  id: l.id, quantity: l.quantity, unitPrice: l.unitPrice,
  ...(l.minimumAge != null ? { minimumAge: l.minimumAge } : {}),
  ...(l.category   != null ? { category:   l.category   } : {}),
})),

But resolveRequirements (manifest.ts) runs the same appliesTo against the host's full GateOrder. OrderLine documents requiresRx as a first-class field (types.ts), and the published README keys its prescription gate on exactly that:

appliesTo: (order) => order.lines.some((l) => l.requiresRx)

So the gate evaluates appliesTo === true at manifest time (surfaced, required: true, given an approve link) and appliesTo === false at completion (field was stripped) → the sweep skips it → the order completes unproven.

Reproduced against this branch (drives the PR's real resolveRequirements + completeOrder):

[rx]       surfaced in manifest: true   required: true
[rx]       completeOrder, no proof presented: {"completed":true}     ← 🚨 should be refused
[category] completeOrder, no proof presented: {"completed":false,"reason":"gate"}   ← same code, forwarded field → correctly refused

The contrast is the proof: identical machinery, the only difference is whether appliesTo reads a field the projection kept (category) or dropped (requiresRx). The sweep effectively only enforces gates keyed on minimumAge/category.

Why the tests are green. Every sweep test (completion.test.ts, generic-credential.test.ts, mount.test.ts) keys appliesTo on category, which survives the projection. requiresRx is exercised only through resolveRequirements, never through completeOrder — so no test drives the mismatched path.

Fix. Evaluate appliesTo against a single shared order representation so manifest-time and completion-time see identical inputs (pass the re-priced CeremonyOrder/host order through one helper instead of a lossy hand-copy). Add a bypass test: a requiresRx-gated prescription order, unproven, asserted refused at completeOrder — and confirm it flips to green only with the control in place.

(Companion to the checkout-hub blocker in the comments above. "finding N" here are review items, not issue references.)

@dzuluaga

dzuluaga commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Remaining review findings (correctness + quality)

@TheBlackBit — rounding out the review so the record is complete. The two checkout-hub comments and the fail-open comment above are the blockers; these are the rest, ranked. All verified against this branch.

3 · Registry is instance-lifetime and unscoped → cross-policy stuck orders. requirements() (client.ts) permanently registers every credential it ever resolves, and the completion sweep iterates the whole registry, not the completing order's policy. With per-call policy variation (feature flag / multi-tenant — the API takes policy per call), an order whose own policy omitted a custom gate is still refused at completion if its cart matches appliesTo — but its manifest never surfaced the gate, so there's no approve URL and the order is permanently stuck (fail-closed). Same-id credentials with different .when() are last-write-wins. Fix: scope the sweep to the credentials the order actually resolved.

4 · optional(gate()) → authorized-but-silently-refused dead end. The sweep enforces every gate() regardless of the required flag (contract C5), but the checkout page only blocks on required gates. So optional(customGate) renders payment unlocked; the buyer completes the on-device ceremony, the mandate is built and all amount-binding gates pass — then completeOrder refuses with reason:"gate", which the rail page never renders (no banner, Pay silently re-enabled). Also: required() on a custom discount()/authorize() credential is a silent no-op at the seam. Consider rejecting optional(gate()) / required(non-gate) at policy-build time, or aligning the page's blocked with the seam.

5 · Cold-registry fail-open on multi-instance (register-on-resolve). Custom-gate enforcement is guarded by if (ctx.credentialRegistry) and the registry is populated only on a process that has run requirements(). On a multi-instance / serverless deploy with a shared VerificationStore, an instance that handles a /verify or completion without having resolved the policy enforces nothing — while the built-in age gate re-derives from the catalog unconditionally. plan.md tracks this honestly ("warm the registry at startup"); flagging because the asymmetry with age is easy to miss, and there's no storefront integration test covering the app.locals registry wiring. (I also hit its fail-closed twin live: /credentagent/credential/verify on a cold process returns 404 "unknown credential" from resolveCred.)

6 · Multi-credential DCQL silently truncates on iOS. mdocDocSpecFromDcql (doc-spec.ts) reads only dcql.credentials[0]; the OpenID4VP/Android path sends the full query. A hand-built multi-doc custom credential (type-legal, unvalidated) therefore works on Android and can never verify on iOS, with no error. Fails closed. Validate-and-reject multi-credential requests, or emit multiple docRequests, rather than truncating.

7 · Two parallel per-credential-id stores on VerificationRecord. The new verifiedGates?: Record<string,true> sits directly above the pre-existing [credentialId: string]: unknown index signature whose comment still reads "Custom credential results, keyed by credential id" — now dead (nothing writes it) and actively misleading. A future contributor following that comment would write record[id] and silently un-enforce the gate, since the sweep reads only verifiedGates. Drop or re-document the index signature so one representation wins.

8 · Reuse / test-hygiene. (a) The DCQL claim-leaf extraction (path[path.length-1]) is implemented twice by this PR — doc-spec.ts and routes.ts demoClaimsFor — extract one helper next to the dcql() builder that owns the path shape. (b) tasks.md T002 is checked off as done, but the shared fixture it mandates (credential-gate/__fixtures__/customCredential.ts) was never created; the professional_license fixture is copy-pasted in completion.test.ts, generic-credential.test.ts, and mount.test.ts.

("finding N" are review items, not issue references.)

…en (PR #42 review)

Addresses the blockers from Diego's review of #42.

Blocker — checkout hub deadlocked (rendered a custom gate as an age gate and
could never unlock). The backend was generalized but the shared checkout page
(checkout-page.ts) and the storefront were still age-only:
 - renderGate now renders a custom gate from its OWN `entry.label` (not the age
   copy); railSteps emits a step per gate() effect; the lock copy is generic.
 - isSatisfied + RenderVerification now consult `verifiedGates`, and the
   storefront passes it into the render — so a proven custom gate clears the
   lock and offers the pay rail instead of looping back to a locked page.

Blocker (security, fail-OPEN) — the completion sweep evaluated `appliesTo`
against a lossy 5-field projection of the re-priced order, so a gate keyed on
any other field (e.g. the README's `requiresRx` prescription gate) was surfaced
as applicable at manifest time yet SKIPPED at completion → the order completed
unproven (invariant 1). Fix: spread the full re-priced line (no allow-list) so
completion and the manifest resolver see identical inputs; add `requiresRx` to
CeremonyOrderLine as a catalog-derived attribute. New bypass test drives a
requiresRx gate through completeOrder and asserts refusal (red without the fix).

Missing end-to-end coverage — added a storefront test that checks out a
custom-gate order, proves the credential, and asserts the pay button appears
(and completes through dc-payment), plus a checkout-page render test for a
custom gate. Verified at runtime against examples/professional-license.mjs:
prove license → payment unlocks → completes.

Cleanups from the review:
 - #7: dropped the dead/misleading `[credentialId: string]: unknown` index
   signature on VerificationRecord; `verifiedGates` is the single source.
 - #8a: extracted `claimLeaf(path)` next to `dcql()`; used by doc-spec.ts and
   routes.ts (was duplicated).
 - #8b: created the shared `credential-gate/__fixtures__/customCredential.ts`
   (T002) and consumed it from the completion / generic-credential / mount
   suites (was copy-pasted three times).

Deferred to follow-ups (need a design call): #4 optional(gate())/required(non-gate)
policy validation, #5 multi-instance cold-registry asymmetry, #6 multi-credential
DCQL truncation on iOS.

266 tests green (193 gate + 73 storefront); build green. Bypass controls verified
red when removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: Ever Morales <ever.morales@koombea.com>
@TheBlackBit

TheBlackBit commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator Author

@dzuluaga — addressed in 6b0ef8f (pushed). Verified end-to-end against examples/professional-license.mjs (prove license → payment unlocks → completes through dc-payment) plus new bypass/e2e tests. 266 green (193 gate + 73 storefront).

Blockers — fixed

  • Checkout hub rendered a custom gate as an age gate. renderGate now renders from the credential's own entry.label (custom branch); railSteps emits a step per gate() effect; the lock copy is no longer age-specific. (checkout-page.ts)
  • Deadlock — a proven custom gate stayed locked. isSatisfied + RenderVerification now consult verifiedGates, and the storefront passes verifiedGates into the render (server.ts), so a proven gate clears blocked and offers the pay rail instead of looping to a locked page. Added the missing coverage you called out: a storefront e2e test that checks out a custom-gate order, proves the credential, asserts the pay button appears, and completes via dc-payment.
  • Fail-open at completion (invariant 1). The sweep now evaluates appliesTo against the full re-priced line ({ ...l }, no 5-field allow-list) so completion and resolveRequirements see identical inputs; requiresRx is added to CeremonyOrderLine. New bypass test drives a requiresRx prescription gate through completeOrder and asserts it is refused — confirmed red without the fix (it completes unproven, exactly your repro).

Findings 7 & 8 — fixed

  • 7 — dropped the dead/misleading [credentialId: string]: unknown index signature; verifiedGates is the single source.
  • 8a — extracted claimLeaf(path) next to dcql(); doc-spec.ts and routes.ts both use it. 8b — created the shared credential-gate/__fixtures__/customCredential.ts (T002) and consumed it from the completion / generic-credential / mount suites.

Findings 3–6 — deferred (need a design decision)

Scoped this round to the blockers + safe cleanups; these are tracked as follow-ups:

  • 3 — scope the completion sweep to the order's resolved policy rather than iterating the whole instance-lifetime registry (cross-policy stuck-order risk).
  • 4optional(gate()) authorized-but-silently-refused: reject optional(gate()) / required(non-gate) at policy-build time, or align the page's blocked with the seam.
  • 5 — cold-registry multi-instance fail-open + the cold /verify 404 (register-on-resolve lifecycle).
  • 6mdocDocSpecFromDcql reads only credentials[0], truncating a multi-credential DCQL on iOS.

Every new bypass test was verified red with its control removed. Separate note from testing: the credential rail has no cross-device/QR transport (same-device DC API + instant-demo only) — pre-existing on age/membership, unchanged here; worth its own enhancement.

Tested on browser

Screenshot 2026-07-09 at 7 07 53 PM

dzuluaga and others added 3 commits July 10, 2026 09:36
Two currently-RED regression tests pinning the confirmed blockers from the review.

finding 1 (fail-OPEN): a defineCredential whose id collides with a reserved
built-in (age/membership/payment) is silently shadowed — resolveCred routes it to
the built-in and the completion sweep skips it (RESERVED_CREDENTIAL_IDS), so a
declared hard gate() completes unproven with no error. Test asserts defineCredential
rejects a reserved id (credentials.test.ts). Red until the fail-fast guard lands.

finding 2: the flagship prescription example keys appliesTo on requiresRx, but
priceCart forwards a fixed field set and drops it, so the gate can never apply on
the reference storefront (an Rx product checks out unproven while the README says
"enforced end-to-end"). Test asserts priceCart forwards requiresRx onto the priced
line (index.test.ts). Red until Product/priceCart thread the field.

Both verified RED on this branch; the rest of each suite stays green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxYCu2K49F4o1CZguC2NF
Signed-off-by: Diego Zuluaga <dfzuluaga@gmail.com>
…ts green)

finding 1 (fail-OPEN): defineCredential now throws on a reserved built-in id
(age / membership / payment). A custom credential with a reserved id was silently
routed to the built-in (resolveCred) and skipped by the completion sweep
(RESERVED_CREDENTIAL_IDS), so a declared hard gate() completed unproven with no
error. Fail-fast at construction beats accepting a policy the seam cannot honor.

finding 2: thread requiresRx through the storefront so the flagship prescription
gate is enforceable end-to-end. Product and PricedCartLine carry requiresRx, and
priceCart forwards it onto the priced line — so a custom appliesTo sees the SAME
field at manifest time and at the completion sweep (never fail-open). The gate
README's `appliesTo: (o) => o.lines.some(l => l.requiresRx)` example now fires on
createStorefront instead of silently never applying.

Makes the two regression tests from c0c4e65 green. Gate suite 196 green; the
storefront priceCart/index suite green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019QxYCu2K49F4o1CZguC2NF
Signed-off-by: Diego Zuluaga <dfzuluaga@gmail.com>
@dzuluaga

Copy link
Copy Markdown
Contributor

Pushed fixes for review findings 1 & 2 (with regression tests)

@TheBlackBit — I turned the two blockers from the review into regression tests and fixed them. Two commits on the branch:

  • c0c4e65 — the two tests (red on the prior head, so they pin the bugs).
  • d92d0df — the fixes (turn them green).

Finding 1 — a custom credential with a reserved id silently fails open. defineCredential({ id: "membership" | "age" | "payment", effect: gate(), … }) was routed to the built-in path by resolveCred and skipped by the completion sweep (RESERVED_CREDENTIAL_IDS), so a declared hard gate completed unproven, with no error at define/mount time. Those are the natural names a developer reaches for, and id is an unconstrained string. Fix: defineCredential now throws on a reserved id (fail-fast — better than accepting a policy the seam can't honor). Test: credentials.test.ts.

Finding 2 — the prescription example never fires on the storefront. The gate README's flagship gate keys appliesTo on requiresRx, but priceCart forwarded a fixed field set and dropped it — so an Rx product checked out with no proof while the README says "enforced end-to-end", and the completion.test.ts bypass test passed only because its private catalog injects the field. Fix: Product and PricedCartLine now carry requiresRx, and priceCart forwards it onto the priced line, so a custom appliesTo sees the same field at manifest time and at the completion sweep. Test: index.test.ts.

One thing left open (deeper option on finding 2): priceCart still forwards a hardcoded field set, so a custom gate can only key on fields we happen to forward. Forwarding arbitrary catalog attributes onto the line would make defineCredential truly generic — worth a follow-up if we want custom gates to key on any product field.

Still open from the review and untouched here: the register-on-resolve cold-instance fail-open (your deferred item 5), optional(gate()) / required(discount()) policy validation (deferred item 4), and the multi-credential DCQL iOS truncation (item 6). Happy to pair on those.

@dzuluaga dzuluaga left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check the comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Define Credential — credential library beyond age/membership/payment (defineCredential extension)

2 participants