From e985c6efb0b41e354249546adefe003818b74533 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Tue, 8 Sep 2026 14:01:47 +0200 Subject: [PATCH] fix(persona): a decision does not wait for the claim-type table MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported from a live wallet, and three defects behind one screen. The agent does not answer `persona/claim-types/list`, so the console holds no table — and that state was handled three different ways, each of them wrong. **The holder's decision was discarded with it.** `treatmentFor` returned the fail-closed floor for a missing registry *before* reading the override, on the reasoning that a decision "cannot be applied over an answer that has not arrived". That reads well and is backwards: §4 rule 1 makes the holder's answer win over the registry's, so where they gave one there is nothing to combine and nothing to wait for. An agent that serves no table — or failed to answer once — was silently overruling every choice its owner had made about their own values, and the screen attributed that to the registry. The override is now applied first. The mask axis still keeps a *declared* token's registry mask, which is exactly what a missing table cannot know, so there the holder is the only evidence there is. **The screen said the table had declined the tokens.** With no table every attribute grouped as `unregistered`, whose words are "your agent's claim-type table does not declare these" — a claim about the tokens that nobody had checked, and the same error as reporting a context the agent would not answer for as a context that holds nothing. `unknown` is now its own family and its own `source`, and says *your agent has not said*. **The failure was swallowed.** `persona.tsx` loaded the table with a note saying it "needs no failure branch of its own, because the same agent answers both". They are different tasks: the live wallet listed its pool perfectly and served no table. `registry.error` is now shown, with what it means for the screen, and `reloadAll` reloads it — left out, one failure kept every value masked for the life of the tab. Also the wording a person asked about twice: a withheld value read "not on this page", and pressing *Show* then fetched and displayed it, so the card said the value was not here and then produced it. It now says **with your agent**, which the next press does not contradict. 7 new tests (366 extension, 576 core), `tsc -b` clean, `npm run build` clean. Signed-off-by: Glenn Gore --- CLAUDE.md | 20 ++++++ .../extension/src/manager/attribute-family.ts | 33 +++++++-- .../src/manager/claim-sensitivity.ts | 64 ++++++++++++------ .../src/manager/panes/persona-editors.tsx | 16 +++-- .../extension/src/manager/panes/persona.tsx | 25 +++++-- .../extension/src/manager/reveal-value.ts | 2 +- .../tests/manager-attribute-family.test.mts | 14 +++- .../tests/manager-claim-sensitivity.test.mts | 37 ++++++++++ .../tests/manager-reveal-value.test.mts | 2 +- .../tests/persona-pane.render.test.mts | 67 ++++++++++++++++++- 10 files changed, 240 insertions(+), 40 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b80c945..bde4a2d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -461,6 +461,26 @@ per §3.3; `maskedFact` gated on `high` anyway, so `email.*` (`normal` / `emailLocal`) was called hidden by `isSensitive` and drawn in full by the renderer — a promised *Show* button that never appeared. +**A decision does not wait for the table.** `treatmentFor` applies the holder's +`sensitivity` first, before it looks at the registry at all — §4 rule 1 makes +their answer win, so where they gave one there is nothing to combine and nothing +to wait for. It used to return the fail-closed floor for a missing registry +*before* reading the override, which meant an agent that does not implement +`persona/claim-types/list`, or failed to answer once, silently overruled every +choice the holder had made about their own values. The mask axis still keeps a +*declared* token's registry mask; with no table, whether the token is declared is +unknowable and the holder is the only evidence there is. + +**A missing table is not a statement about a token.** `source: "unknown"` and the +`unknown` family exist so the screen can say *your agent has not said* rather +than *your agent's table does not declare these* — the second is a claim about +the tokens that nobody checked, the same error as reporting an unreadable +context as an empty one. `persona.tsx` surfaces `registry.error` in a note (it +used to swallow it on the reasoning that "the same agent answers both", which is +false: they are different tasks and a live wallet listed its pool perfectly while +serving no table), and `reloadAll` reloads it — left out, one failure kept every +value masked for the life of the tab. + **An editor may not write a value it never held.** The pane lists without `includeSensitive` — the point of #194 — so an existing sensitive attribute reaches the editor with `value: undefined`, `rawValue` turns that into `""`, and diff --git a/packages/extension/src/manager/attribute-family.ts b/packages/extension/src/manager/attribute-family.ts index 97a8067..bd0aef1 100644 --- a/packages/extension/src/manager/attribute-family.ts +++ b/packages/extension/src/manager/attribute-family.ts @@ -41,13 +41,20 @@ import { registeredRoots, type ClaimTypeRegistry } from "@openvtc/pnm-core/perso * fallback bucket to be tidied away: it is the answer for every token the * registry does not declare, which today includes most of what a holder * invents for themselves. */ -export type Family = "identity" | "contact" | "public" | "gated" | "unregistered"; +export type Family = "identity" | "contact" | "public" | "gated" | "unregistered" | "unknown"; /** Top to bottom, the order the map lays the groups out in — roughly how * closely a value identifies the person, so the row reads as a gradient rather * than an alphabet. `unregistered` sits last because it is the group whose * size is a question rather than a fact about the holder. */ -export const FAMILY_ORDER: readonly Family[] = ["identity", "contact", "public", "gated", "unregistered"]; +export const FAMILY_ORDER: readonly Family[] = [ + "identity", + "contact", + "public", + "gated", + "unregistered", + "unknown", +]; export interface FamilyStyle { /** The group heading, in the vocabulary of `design-docs/persona-vocabulary.md`. */ @@ -89,6 +96,16 @@ const STYLES: Readonly> = { note: "your agent's claim-type table does not declare these, so they are treated as the most private kind", hue: "var(--m-fam-unregistered)", }, + unknown: { + // Not a family at all, and the words must not read as one. "Your agent's + // table does not declare these" is a statement *about the tokens*, and + // printing it when no table arrived says something nobody checked — the + // same error as reporting a context the agent would not answer for as a + // context that holds nothing. + label: "Your agent has not said", + note: "it did not answer with a claim-type table, so everything here is treated as the most private kind until it does", + hue: "var(--m-fam-unregistered)", + }, }; export function familyStyle(family: Family): FamilyStyle { @@ -123,13 +140,15 @@ export const PLACED_ROOTS = [ ] as const; export function familyOf(type: string, registry: ClaimTypeRegistry | null): Family { + // No table. Every token here is unclassified, but *why* it is unclassified is + // a different sentence and the heading says it out loud — `unknown` rather + // than `unregistered`. Colouring by a compiled-in guess would put a family on + // something this agent may never have declared, which is the copy this + // replaced; claiming the agent *declined* the token is the copy that replaced + // it and was wrong in the other direction. + if (!registry) return "unknown"; if (type.startsWith("x:")) return "unregistered"; const root = type.split(".")[0] ?? ""; - // No table yet: `unregistered` is what a token nobody has classified gets, - // and while the registry is in flight that is exactly what every token is - // from here. Colouring by a compiled-in guess would put a family on - // something this agent may never have declared. - if (!registry) return "unregistered"; if (!registeredRoots(registry).has(root)) return "unregistered"; switch (root) { case "name": diff --git a/packages/extension/src/manager/claim-sensitivity.ts b/packages/extension/src/manager/claim-sensitivity.ts index 4e52560..11e002b 100644 --- a/packages/extension/src/manager/claim-sensitivity.ts +++ b/packages/extension/src/manager/claim-sensitivity.ts @@ -189,30 +189,56 @@ export function treatmentFor( registry: ClaimTypeRegistry | null, type: string, override?: Sensitivity | undefined, -): { treatment: ClaimTreatment; source: "holder" | "registry" } { - // No table yet. Everything is drawn masked and attributed to the registry, - // which is the fail-closed answer *and* the honest one: the holder's decision - // cannot be applied over an answer that has not arrived, and claiming - // `source: "holder"` here would put their name on a default. +): { treatment: ClaimTreatment; source: TreatmentSource } { + // **The holder's decision does not need the table.** §4 rule 1 makes it win + // over the registry's answer, so where they gave one there is nothing to + // combine and nothing to wait for. This branch is first for that reason. + // + // It used to be last, behind a fail-closed return for a missing registry, on + // the reasoning that a decision "cannot be applied over an answer that has not + // arrived". That reads well and is wrong: it made an agent that does not serve + // `persona/claim-types/list` — or one that failed to answer once — silently + // overrule every choice the holder had made about their own values. The + // console showed four bullets on a value its owner had explicitly marked + // *show it*, and attributed that to the registry. + // + // The mask axis still needs to know whether the token is *declared*, because a + // declared token's mask is the registry's own statement and this console does + // not overrule it. With no table that is unknowable, and the only evidence to + // hand is the holder — so their answer governs, which is also the answer they + // asked for. + if (override !== undefined) { + const declared = registry !== null && isRegisteredType(registry, type); + return { + treatment: { + sensitivity: override, + mask: declared ? resolveTreatment(registry!, type).mask : override === "high" ? "full" : "none", + }, + source: "holder", + }; + } + // No table, and no decision to fall back on: mask everything. `unknown` is a + // third source and not a synonym for `registry` — a surface that says "your + // agent's table does not declare this" when the table never arrived is + // stating a fact it does not have, which is the same error as reporting an + // unreadable context as an empty one. if (!registry) { - return { treatment: { sensitivity: "high", mask: "full" }, source: "registry" }; + return { treatment: { sensitivity: "high", mask: "full" }, source: "unknown" }; } - const declared = resolveTreatment(registry, type); - if (override === undefined) return { treatment: declared, source: "registry" }; - return { - treatment: { - sensitivity: override, - mask: isRegisteredType(registry, type) - ? declared.mask - : override === "high" - ? "full" - : "none", - }, - source: "holder", - }; + return { treatment: resolveTreatment(registry, type), source: "registry" }; } +/** + * Whose answer a treatment came from. + * + * `unknown` exists so a surface can tell "the table says nothing about this + * token" apart from "there is no table" — the same distinction the contexts + * band makes between a context that holds nothing and one the agent would not + * answer for. Both mask; only one of them is a statement about the token. + */ +export type TreatmentSource = "holder" | "registry" | "unknown"; + /** Whether this attribute's value is hidden until asked for, the holder's own * decision included. The `type`-only {@link isSensitive} is the registry's * answer alone and stays that way — a call site holding a whole attribute diff --git a/packages/extension/src/manager/panes/persona-editors.tsx b/packages/extension/src/manager/panes/persona-editors.tsx index f3ec514..d2792aa 100644 --- a/packages/extension/src/manager/panes/persona-editors.tsx +++ b/packages/extension/src/manager/panes/persona-editors.tsx @@ -80,10 +80,18 @@ export function Label({ children }: { children: React.ReactNode }) { * printing a passport number in full, and it would look like ordinary code. */ function formatValue(value: unknown): { text: string; withheld: boolean } { - // "not on this page" rather than "not requested": the second described the - // request that was made, which is a fact about the console, while the person - // reading it wants to know where the value is. It is with their agent. - if (value === undefined) return { text: "not on this page", withheld: true }; + // **"with your agent"**, because the two earlier attempts each described + // something other than what the reader needs. + // + // "not requested" described the request the console made — true, and about + // the console rather than the value. "not on this page" fixed that and + // introduced its own contradiction: pressing *Show* fetches the value and + // displays it, so the card had said the value was not here and then produced + // it, which reads as the screen not knowing its own mind. + // + // This says where the value is, and the control beside it says what pressing + // it does. Neither is a claim the next press disproves. + if (value === undefined) return { text: "with your agent", withheld: true }; if (value === null) return { text: "null", withheld: false }; if (typeof value === "string") return { text: value, withheld: false }; if (typeof value === "number" || typeof value === "boolean") { diff --git a/packages/extension/src/manager/panes/persona.tsx b/packages/extension/src/manager/panes/persona.tsx index f3ec731..4c1d4f9 100644 --- a/packages/extension/src/manager/panes/persona.tsx +++ b/packages/extension/src/manager/panes/persona.tsx @@ -151,9 +151,16 @@ export function PersonaPane({ async () => loadContexts(parties, records), [parties.holder.did, parties.service.did, records.map((r) => r.id).join(" ")], ); - // The claim-type table, read from THIS agent rather than compiled in. Loaded - // beside the pool because the same agent answers both: if this refuses there - // are no attributes to mask either, so it needs no failure branch of its own. + // The claim-type table, read from THIS agent rather than compiled in. + // + // **It needs a failure branch of its own, and the note here used to say it + // did not.** The reasoning was that the same agent answers both, so a refusal + // would take the attributes with it — but they are two different tasks, and an + // agent that lists a pool perfectly while declining or not implementing + // `persona/claim-types/list` is exactly what a live wallet hit. Every value + // then falls to the floor and is masked, which is the right *behaviour* and a + // silent one: the screen said the table did not declare these tokens, which is + // a claim about the tokens that nobody had checked. const registry = useAsync( async () => listClaimTypes(managerSender, parties), [parties.holder.did, parties.service.did], @@ -168,7 +175,10 @@ export function PersonaPane({ profiles.reload(); contexts.reload(); history.reload(); - }, [attributes, profiles, contexts, history]); + // Reloaded with the rest. Left out, a table that failed once stayed failed + // for the life of the tab, and every value stayed masked with it. + registry.reload(); + }, [attributes, profiles, contexts, history, registry]); // Guide or map — derived, with two flags that each fix a different way the // naive version is wrong. @@ -240,6 +250,13 @@ export function PersonaPane({
{profiles.error && } {contexts.error && } + {registry.error && ( + + Your agent would not give its claim-type table — {registry.error}. Until it does, every + value here is hidden as the most private kind, whatever kind it actually is. What you have + decided for yourself still stands. + + )} { - const families: Family[] = ["identity", "contact", "public", "gated", "unregistered"]; + const families: Family[] = ["identity", "contact", "public", "gated", "unregistered", "unknown"]; assert.deepEqual([...FAMILY_ORDER].sort(), [...families].sort(), "a family with no place in the order never draws"); for (const family of families) { const style = familyStyle(family); @@ -115,3 +115,15 @@ test("no family's words claim the colour protects anything", () => { } } }); + +test("no table is its own answer, and it does not accuse the tokens", () => { + // `unregistered` says the agent's table declines to declare this token. + // Saying that when no table arrived is a claim about the token that nobody + // checked — the same error as reporting a context the agent would not answer + // for as a context that holds nothing. + assert.equal(familyOf("name.legal", null), "unknown"); + assert.equal(familyOf("x:whatever", null), "unknown", "even the one case that is unregistered by construction"); + const { label, note } = familyStyle("unknown"); + assert.doesNotMatch(`${label} ${note}`.toLowerCase(), /does not declare/); + assert.match(note.toLowerCase(), /did not answer/); +}); diff --git a/packages/extension/tests/manager-claim-sensitivity.test.mts b/packages/extension/tests/manager-claim-sensitivity.test.mts index eab9d67..8b746f7 100644 --- a/packages/extension/tests/manager-claim-sensitivity.test.mts +++ b/packages/extension/tests/manager-claim-sensitivity.test.mts @@ -327,3 +327,40 @@ test("a type with no mask style is drawn plainly and offers no control", () => { assert.equal(drawn("name.legal", "Glenn Gore").masked, false); assert.equal(isSensitiveFor(REGISTRY, "name.legal"), false); }); + +// ── A decision does not wait for the table ───────────────────────────────── +// +// Reported from a live wallet: every value masked, including `name.legal`, +// including one the holder had explicitly marked *show it*, with a heading +// saying the agent's table did not declare them. The agent had simply not +// answered `persona/claim-types/list` — and `treatmentFor` returned the floor +// for a missing table BEFORE looking at the holder's own decision, so an absent +// answer silently overruled every choice they had made about their own values. + +test("the holder's decision outranks a table that never arrived", () => { + const { treatment, source } = treatmentFor(null, "profile.github", "normal"); + assert.equal(treatment.sensitivity, "normal"); + assert.equal(treatment.mask, "none", "they said show it; there is nothing to wait for"); + assert.equal(source, "holder"); +}); + +test("with no table and no decision, everything is masked — and says why", () => { + const { treatment, source } = treatmentFor(null, "name.legal"); + assert.equal(treatment.mask, "full", "fail closed"); + assert.equal(source, "unknown", "not `registry` — no registry answered"); +}); + +test("a decision to keep something back survives a missing table too", () => { + // The direction that fails safe is not the only one that has to work: a + // holder who marked something private must not have that quietly ignored + // either, even though the outcome happens to match the floor. + const { treatment, source } = treatmentFor(null, "name.legal", "high"); + assert.equal(treatment.mask, "full"); + assert.equal(source, "holder", "their decision, not a default wearing their name"); +}); + +test("with a table, a declared token still keeps the registry's mask", () => { + // The narrowness this PR must not lose: knowing the token is declared is + // exactly what the missing-table case cannot know. + assert.equal(treatmentFor(REGISTRY, "phone.mobile", "normal").treatment.mask, "last2"); +}); diff --git a/packages/extension/tests/manager-reveal-value.test.mts b/packages/extension/tests/manager-reveal-value.test.mts index cdedb37..5315c73 100644 --- a/packages/extension/tests/manager-reveal-value.test.mts +++ b/packages/extension/tests/manager-reveal-value.test.mts @@ -69,7 +69,7 @@ test("the answer is matched by id, because a type can have siblings", async () = }); test("an agent that still withholds the value is an error, not another blank", async () => { - // Returning `undefined` would redraw "not on this page" and the person would + // Returning `undefined` would redraw "with your agent" and the person would // press Show again, learning nothing about why. const s = sender([attr()]); await assert.rejects( diff --git a/packages/extension/tests/persona-pane.render.test.mts b/packages/extension/tests/persona-pane.render.test.mts index 85abd5d..c0605a8 100644 --- a/packages/extension/tests/persona-pane.render.test.mts +++ b/packages/extension/tests/persona-pane.render.test.mts @@ -17,6 +17,7 @@ import { agent, h, render, PARTIES } from "./harness/dom.mjs"; import { GuidedSetup } from "../src/manager/panes/persona-setup.js"; import { IdentityMap } from "../src/manager/panes/persona-map.js"; import { AttributeEditor, BindingForm, ResolvedProfile } from "../src/manager/panes/persona-editors.js"; +import { PersonaPane } from "../src/manager/panes/persona.js"; import { buildGraph } from "../src/manager/identity-graph.js"; const HOLDER = { session: { id: "s" }, roles: ["admin"], scopes: [] }; @@ -610,7 +611,7 @@ test("a value the agent withheld is said to be missing, not masked", async () => const a = agent({}); const ui = await render(withheldMap(), { chrome: { runtime: { sendMessage: a.sendMessage } } }); const text = ui.text(); - assert.match(text, /not on this page/, "the card says where the value is: with the agent"); + assert.match(text, /with your agent/, "the card says where the value is: with the agent"); assert.doesNotMatch(text, /•/, "a mask over a value nobody sent claims one is being held back"); await ui.unmount(); }); @@ -642,7 +643,7 @@ test("Hide drops a fetched value rather than covering it over", async () => { assert.match(ui.text(), /octocat/); await ui.click(ui.button("Hide")); assert.doesNotMatch(ui.text(), /octocat/, "hidden means gone from the page, not greyed"); - assert.match(ui.text(), /not on this page/); + assert.match(ui.text(), /with your agent/); await ui.unmount(); }); @@ -654,7 +655,7 @@ test("an agent that refuses says why, in place, and does not blank the card", as ); await ui.click(ui.button("Show")); assert.match(ui.text(), /held the value back/); - assert.match(ui.text(), /not on this page/, "the card still says what it knows"); + assert.match(ui.text(), /with your agent/, "the card still says what it knows"); await ui.unmount(); }); @@ -975,3 +976,63 @@ test("without the pool the claim falls back to the registry, which is weaker and assert.match(ui.text(), /Glenn Gore/, "a registered normal type is unaffected either way"); await ui.unmount(); }); + +// ── When the agent serves no claim-type table ────────────────────────────── +// +// Reported from a live wallet, and three separate defects in one screen: every +// value masked including `name.legal`, a value the holder had explicitly marked +// *show it* masked with the rest, and a heading saying the agent's table "does +// not declare these" — about an agent that had not answered at all. + +const DECIDED = [ + attribute("f1", "name.legal", "Glenn Gore"), + { ...attribute("f9", "profile.github", "octocat"), sensitivity: "normal" }, +]; + +test("a decision the holder made still holds when no table arrived", async () => { + const a = agent({}); + const ui = await render( + h(IdentityMap, { + parties: PARTIES, + authority: HOLDER, + // `null` is the state a failed or unimplemented `claim-types/list` leaves + // behind, and it used to discard the holder's own answer with it. + registry: null, + graph: buildGraph(DECIDED, [], []), + attributes: DECIDED, + profiles: [], + records: CONTEXTS, + history: [], + onReveal: async () => "never asked", + onChanged: () => {}, + }), + { chrome: { runtime: { sendMessage: a.sendMessage } } }, + ); + const text = ui.text(); + assert.match(text, /octocat/, "they said show it, and that does not depend on a table"); + assert.doesNotMatch(text, /does not declare these/, "no table answered, so nothing declined anything"); + assert.match(text, /Your agent has not said/); + await ui.unmount(); +}); + +test("the pane says the table did not arrive, rather than letting it look like a rule", async () => { + // The fake agent throws on a task the test did not name, which is exactly + // what an agent that does not implement `persona/claim-types/list` does to + // this pane. Everything else answers, so the pool is on screen and only the + // table is missing — the shape the live wallet was in. + const a = agent({ + "persona/attribute/list/1.0": { attributes: DECIDED }, + "persona/profile/list/1.0": { profiles: [face("p1", "OSS Developer", ["f1"])] }, + "persona/binding/list/1.0": { personas: [] }, + "persona/disclosure/history/1.0": { disclosures: [] }, + }); + const ui = await render( + h(PersonaPane, { parties: PARTIES, authority: HOLDER, records: CONTEXTS }), + { chrome: { runtime: { sendMessage: a.sendMessage } } }, + ); + await ui.settle(); + const text = ui.text(); + assert.match(text, /would not give its claim-type table/); + assert.match(text, /What you have decided for yourself still stands/); + await ui.unmount(); +});