fix(core): send contextId, the name the schema gives it - #124
Merged
Conversation
`vtaListDids` sent `{ context_id }` for `vta/webvh/dids/list/1.0`. That schema
names `contextId` and sets `additionalProperties: false`, so the old spelling
was never a tolerated synonym — it made the whole payload malformed, and a
conforming agent refuses it as `malformedRequest`.
The failure mode before the schema existed is why this is worth a test of its
own. Nothing rejected the payload, so the filter was simply ignored: callers
asking for one context got every context, and an unfiltered list looks exactly
like a working one until you count the rows.
`vault.ops.mjs` asserted the old spelling, so it was pinning the drift rather
than catching it. Updated — and its stubbed REPLY stays snake_case on purpose,
because the read path folds both while agents migrate. Emitting the canonical
spelling and accepting either are separate moves; dropping the fold would leave
`contextId` undefined against an agent that has not taken it.
Audited the other 107 `buildTrustTask` call sites for the same class and found
none: 106 modules build payloads from the generated bindings, so they are
camelCase by construction. The audit was checked against the pre-fix tree first
— it reports this defect there and nothing after, so "no other hits" means the
check works rather than that it looked in the wrong place. Remaining snake_case
in `src/` is externally-owned names carried verbatim per SPEC §4.10 (`vp_token`,
`dcql_query`, `credential_offer`, `id_token`) and the DIDComm pickup fields
`live_delivery` / `message_id_list`.
Also takes `@openvtc/trust-tasks` ^0.10.0 -> ^0.12.1. That is the version whose
schema index would begin catching this class client-side; the same move on the
Rust side exposed four defects of exactly this shape (VTI #1015).
Lint clean; 530 tests passing.
Signed-off-by: Glenn Gore <glenn.g@affinidi.com>
11 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The defect
vtaListDidssent{ context_id }forvta/webvh/dids/list/1.0. That schemanames
contextIdand setsadditionalProperties: false, so the oldspelling was never a tolerated synonym — it made the whole payload malformed,
and a conforming agent refuses it as
malformedRequest.The failure mode before the schema existed is the part worth pausing on.
Nothing rejected the payload, so the filter was simply ignored: a caller
asking for one context got every context. An unfiltered list looks exactly like
a working one until you count the rows, so no amount of "does it return DIDs"
testing would have found it.
The test was pinning the bug
vault.ops.mjsasserted{ context_id: "work" }— so the suite was holding thedrift in place rather than catching it.
Its stubbed reply stays snake_case on purpose. Emitting the canonical
spelling and accepting either are separate moves: the read path folds both while
agents migrate, and dropping that fold would leave
contextIdundefined againstan agent that hasn't taken it.
Audited the rest, and checked the audit
The other 107
buildTrustTaskcall sites carry no keys of this class. 106modules build payloads from the generated bindings, so they're camelCase by
construction.
I ran the audit against the pre-fix tree first:
So "no other hits" means the check works, rather than that it was looking in the
wrong place.
Remaining snake_case in
src/is externally-owned names carried verbatim perSPEC §4.10 —
vp_token,dcql_query,credential_offer,id_token— plus theDIDComm pickup fields
live_delivery/message_id_list. Correct as-is.Library bump
Takes
@openvtc/trust-tasks^0.10.0→^0.12.1. That's the version whoseschema index would begin catching this class client-side. The equivalent move on
the Rust side (
trust-tasks-rs0.9 → 0.11) exposed four defects of exactlythis shape — see OpenVTC/verifiable-trust-infrastructure#1015 — which is what
prompted looking here.
Typechecks clean on 0.12.1 with no other source changes needed.
Test
Lint (
tsc -b) clean. 530 passing, 0 failing. Three new tests cover the sendpath, the no-filter case, and that the read-path fold still works.
Checklist (stack guide §9)
fetch(); timeouts applied wherefetchis injected (R1.2)(R3.*) — the whole PR; the send path now matches the published schema
while intake keeps tolerating the old spelling