From a68bed4a8f710f195462187caf21890864de3a26 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Sat, 19 Sep 2026 11:49:32 +0200 Subject: [PATCH 1/2] fix(manager): show the subtree a context delete destroys, and ask before taking it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Deleting a context deletes everything below it. The delete panel said nothing about that, and the omission broke in both directions. A context holding nothing of its own rendered "Your agent reports it holds no keys and no DIDs", `needsForce` was false, the panel sent `force: false`, and the agent refused — because sub-contexts exist. The operator was shown a refusal for a deletion the screen had just called harmless. A context holding one key of its own rendered that one key, `force` went true on the strength of it, and the agent destroyed every context below and everything they held. The checkbox read "destroying the keys and DIDs listed above"; the list was one key and the deletion was a subtree. The panel now names the sub-contexts first — they reframe every list under them as the subtree's contents rather than this context's — and decides `force` from all six of the agent's lists, not two of them. `contextPreviewDelete` returned three of the six. `aclEntriesRemoved` is the one an operator most needs, since it names the subjects about to lose their authority outright rather than merely have it narrowed, and dropping it in the client meant no screen could show it however carefully it was written. It now returns the whole preview as `ContextDeletePreview`, and the panel renders removed-entirely and narrowed as the distinct events the agent already distinguishes. Sub-contexts are derived from the context list the pane already holds: `vta/contexts/preview-delete/1.0` has no member naming them (one is proposed upstream), and a delete that silently takes them is exactly what this prompt exists to prevent, so it is computed rather than left out. Matching is segment-wise, so `acme-corp` is not a child of `acme`. The three render tests were confirmed to fail against the old panel; the fourth — an empty leaf still says so and still needs no force — is the control that keeps the other three from passing vacuously. Signed-off-by: Glenn Gore --- packages/core/src/admin/contexts.ts | 37 +++- .../extension/src/manager/panes/contexts.tsx | 202 +++++++++++++----- .../contexts-pane-delete.render.test.mts | 114 ++++++++++ 3 files changed, 302 insertions(+), 51 deletions(-) create mode 100644 packages/extension/tests/contexts-pane-delete.render.test.mts diff --git a/packages/core/src/admin/contexts.ts b/packages/core/src/admin/contexts.ts index be537f4..c314bd7 100644 --- a/packages/core/src/admin/contexts.ts +++ b/packages/core/src/admin/contexts.ts @@ -32,17 +32,41 @@ export interface ContextDeleteParams { * `ext`, which SPEC §4.5.1 lets any agent send. */ export type ContextDeleteResult = VTAContextsDeleteResponsePayload; +/** What deleting a context would destroy, as the agent reports it. */ +export interface ContextDeletePreview { + id: string; + keys: string[]; + webvhDids: string[]; + /** Subjects whose ACL entry disappears entirely — this context (or the + * subtree with it) was the only scope they held. */ + aclEntriesRemoved: string[]; + /** Subjects who keep an entry, with this scope removed from it. */ + aclEntriesUpdated: string[]; + didTemplates: string[]; +} + /** * What deleting this context would destroy. * * Worth calling first, every time: the keys and DIDs a context holds do not * come back, and `force` exists precisely because the agent refuses to take * them with it by accident. Show the operator this list, then delete. + * + * **Every array covers the whole subtree**, because the deletion does — the + * agent counts the sub-contexts' keys, DIDs and grants alongside this + * context's own. What it does not yet report is *which* sub-contexts those + * are; `vta/contexts/preview-delete/1.0` has no member for that, so a + * consumer wanting to name them derives them from the context list. + * + * This returned three of the six arrays until now. `aclEntriesRemoved` in + * particular is the one an operator most needs — it names the subjects about + * to lose their authority outright — and dropping it here meant no consumer + * could show it however carefully it was rendered. */ export async function contextPreviewDelete( sender: TrustTaskSender, params: Omit, -): Promise<{ id: string; keys: string[]; webvhDids: string[] }> { +): Promise { const envelope = buildTrustTask( TASK_CONTEXTS_PREVIEW_DELETE, { id: params.id }, @@ -52,9 +76,15 @@ export async function contextPreviewDelete( id: string; keys?: string[]; webvhDids?: string[]; - /** Pre-fold spelling, still sent by an agent that has not taken the + aclEntriesRemoved?: string[]; + aclEntriesUpdated?: string[]; + didTemplates?: string[]; + /** Pre-fold spellings, still sent by an agent that has not taken the * camelCase change. Accepted on read; never emitted. */ webvh_dids?: string[]; + acl_entries_removed?: string[]; + acl_entries_updated?: string[]; + did_templates?: string[]; }>(envelope, { expectedResponseType: `${TASK_CONTEXTS_PREVIEW_DELETE}#response`, operationLabel: "vta/contexts/preview-delete/1.0", @@ -63,6 +93,9 @@ export async function contextPreviewDelete( id: payload.id, keys: payload.keys ?? [], webvhDids: payload.webvhDids ?? payload.webvh_dids ?? [], + aclEntriesRemoved: payload.aclEntriesRemoved ?? payload.acl_entries_removed ?? [], + aclEntriesUpdated: payload.aclEntriesUpdated ?? payload.acl_entries_updated ?? [], + didTemplates: payload.didTemplates ?? payload.did_templates ?? [], }; } diff --git a/packages/extension/src/manager/panes/contexts.tsx b/packages/extension/src/manager/panes/contexts.tsx index 902f806..86bef2c 100644 --- a/packages/extension/src/manager/panes/contexts.tsx +++ b/packages/extension/src/manager/panes/contexts.tsx @@ -12,7 +12,11 @@ import { contextsUpdateDid, type ContextRecord, } from "@openvtc/pnm-core"; -import { contextDelete, contextPreviewDelete } from "@openvtc/pnm-core/admin"; +import { + contextDelete, + contextPreviewDelete, + type ContextDeletePreview, +} from "@openvtc/pnm-core/admin"; import { Button, Did, Empty, Note, Panel } from "../../ui.js"; import { LoadError } from "../table.js"; import { c, t, font } from "../../theme.js"; @@ -48,11 +52,7 @@ function Label({ children }: { children: React.ReactNode }) { } /** What deleting a context would destroy, in the agent's own words. */ -interface DeletePreview { - id: string; - keys: string[]; - webvhDids: string[]; -} +type DeletePreview = ContextDeletePreview; function CreateContext({ parties, @@ -266,14 +266,44 @@ function EditContext({ ); } +/** A destroyed-things list: a count, a caption, and the items under it. */ +function Destroyed({ + count, + caption, + children, +}: { + count: number; + caption: string; + children: React.ReactNode; +}) { + if (count === 0) return null; + return ( +
+
+ {count} {caption} +
+
    {children}
+
+ ); +} + function DeleteContext({ parties, record, + subContexts, authority, onDeleted, }: { parties: Parties; record: ContextRecord; + /** The subtree going with it, deepest first. + * + * Derived by the caller from the context list rather than read off the + * preview, because `vta/contexts/preview-delete/1.0` has no member naming + * them. It still has to be shown: the preview's own arrays already *count* + * what the sub-contexts hold, so without this the operator sees keys and + * DIDs that belong to contexts the panel never mentions. */ + subContexts: string[]; authority: Authority | null; onDeleted: () => void; }) { @@ -284,54 +314,112 @@ function DeleteContext({ return ( label="Delete context" disabledReason={denied} preview={() => contextPreviewDelete(managerSender, { ...parties, id: record.id })} - needsForce={(p) => p.keys.length > 0 || p.webvhDids.length > 0} - forceLabel="Delete anyway, destroying the keys and DIDs listed above" - renderPreview={(p) => ( - <> - - Deleting {record.name || record.id} is irreversible. - - {p.keys.length === 0 && p.webvhDids.length === 0 ? ( - Your agent reports it holds no keys and no DIDs. - ) : ( - <> - {p.keys.length > 0 && ( -
-
- {p.keys.length} key{p.keys.length === 1 ? "" : "s"} destroyed: -
-
    - {p.keys.map((k) => ( -
  • {k}
    • - ))} -
-
- )} - {p.webvhDids.length > 0 && ( -
-
- {p.webvhDids.length} DID{p.webvhDids.length === 1 ? "" : "s"} destroyed: -
-
    - {p.webvhDids.map((d) => ( -
  • - -
    • - ))} -
-
- )} - - )} - - )} + // Every list, not two of them. A context whose only contents are a + // grant or a template — or whose contents are all in a sub-context — + // used to read as empty here, so the panel sent `force: false` and the + // agent refused with nothing on screen explaining why. + needsForce={(p) => + subContexts.length > 0 || + p.keys.length > 0 || + p.webvhDids.length > 0 || + p.aclEntriesRemoved.length > 0 || + p.aclEntriesUpdated.length > 0 || + p.didTemplates.length > 0 + } + forceLabel="Delete anyway, destroying everything listed above" + renderPreview={(p) => { + const nothing = + subContexts.length === 0 && + p.keys.length === 0 && + p.webvhDids.length === 0 && + p.aclEntriesRemoved.length === 0 && + p.aclEntriesUpdated.length === 0 && + p.didTemplates.length === 0; + const plural = (n: number, word: string) => `${word}${n === 1 ? "" : "s"}`; + return ( + <> + Deleting {record.name || record.id} is irreversible. + {nothing ? ( + Your agent reports it holds nothing, and has no sub-contexts. + ) : ( + <> + {/* First, because it changes what every list below means: + those are the subtree's contents, not this context's. */} + + {subContexts.map((id) => ( +
  • + {id} +
    • + ))} +     + + {p.keys.map((k) => ( +
  • + {k} +
    • + ))} +     + + {p.webvhDids.map((d) => ( +
  • + +
    • + ))} +     + {/* Losing every scope is a different event from losing one, + and the agent distinguishes them — so does this. */} + + {p.aclEntriesRemoved.map((d) => ( +
  • + +
    • + ))} +     + + {p.aclEntriesUpdated.map((d) => ( +
  • + +
    • + ))} +     + + {p.didTemplates.map((n) => ( +
  • + {n} +
    • + ))} +     + + )} + + ); + }} commit={async (force) => { await contextDelete(managerSender, { ...parties, id: record.id, force }); }} @@ -492,6 +580,21 @@ function ContextDid({ ); } +/** + * The contexts strictly below `id`, deepest first. + * + * Path-derived, because a context id *is* its path — `acme/eng/ci` is under + * `acme`, and the agent builds its own cascade the same way. Segment-wise so + * that `acme-corp` is not read as a child of `acme`. + */ +function descendantsOf(id: string, records: ContextRecord[]): string[] { + const prefix = `${id}/`; + return records + .map((r) => r.id) + .filter((cid) => cid !== id && cid.startsWith(prefix)) + .sort((a, b) => b.split("/").length - a.split("/").length || a.localeCompare(b)); +} + export function ContextsPane({ parties, authority, @@ -550,6 +653,7 @@ export function ContextsPane({ diff --git a/packages/extension/tests/contexts-pane-delete.render.test.mts b/packages/extension/tests/contexts-pane-delete.render.test.mts new file mode 100644 index 0000000..81a5587 --- /dev/null +++ b/packages/extension/tests/contexts-pane-delete.render.test.mts @@ -0,0 +1,114 @@ +// The delete-a-context panel, rendered. +// +// Deleting a context deletes its whole subtree. The panel used to say nothing +// about that, and the omission was not cosmetic in either direction: +// +// **A parent holding nothing of its own read as harmless.** The preview showed +// "no keys and no DIDs", `needsForce` was false, so the panel sent +// `force: false` — and the agent refused, because sub-contexts exist. The +// operator saw a refusal for a deletion the screen had just described as +// destroying nothing. +// +// **A parent holding one key of its own read as almost harmless.** `force` +// went true on the strength of that one key, the confirmation listed it, and +// the agent destroyed every context below it and everything they held. The +// checkbox said "destroying the keys and DIDs listed above"; the list was one +// key, and the deletion was a subtree. +// +// Both are the same missing fact, so both are pinned here. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { agent, h, render, PARTIES } from "./harness/dom.mjs"; +import { ContextsPane } from "../src/manager/panes/contexts.js"; + +const PREVIEW = "vta/contexts/preview-delete/1.0"; +const DELETE = "vta/contexts/delete/1.0"; +const LIST_DIDS = "vta/webvh/dids/list/1.0"; + +const ctx = (id: string) => ({ + id, + name: id, + createdAt: "2026-09-01T00:00:00Z", + updatedAt: "2026-09-01T00:00:00Z", + basePath: "m/26'/2'/0'", +}); + +/** A subtree: `acme` over `acme/eng` over `acme/eng/ci`, plus an unrelated + * `acme-corp` that must not be read as a child of `acme`. */ +const RECORDS = [ctx("acme"), ctx("acme/eng"), ctx("acme/eng/ci"), ctx("acme-corp")]; + +const mount = async (preview: Record, records = RECORDS) => { + const a = agent({ + [PREVIEW]: { id: "acme", keys: [], webvhDids: [], ...preview }, + [DELETE]: { id: "acme", deleted: true }, + [LIST_DIDS]: { dids: [] }, + }); + const screen = await render( + h(ContextsPane, { + parties: PARTIES, + authority: null, + records, + selected: "acme", + onChanged: () => {}, + } as never), + { chrome: { runtime: { sendMessage: a.sendMessage } } }, + ); + return { a, screen }; +}; + +/** Open the delete panel's preview step. */ +const openPreview = async (screen: { button: (s: string) => Element; click: (e: Element) => Promise; settle: () => Promise }) => { + await screen.click(screen.button("Delete context")); + await screen.settle(); +}; + +test("the sub-contexts that go with it are named, not merely implied", async () => { + const { screen } = await mount({}); + await openPreview(screen as never); + const text = (screen as never as { text: () => string }).text(); + assert.match(text, /acme\/eng\/ci/, "the deepest sub-context is not on screen"); + assert.match(text, /acme\/eng(?!\/)/, "the intermediate sub-context is not on screen"); + assert.doesNotMatch(text, /acme-corp/, "a sibling whose id merely shares a prefix is not a child"); +}); + +test("a context holding nothing itself still asks, because its children go too", async () => { + // The agent reports no keys and no DIDs — the exact answer that used to + // produce "holds nothing" and a `force: false` the agent then refused. + const { screen } = await mount({}); + await openPreview(screen as never); + assert.doesNotMatch( + (screen as never as { text: () => string }).text(), + /holds nothing, and has no sub-contexts/, + "a context with three sub-contexts was described as holding nothing", + ); + // The force control is offered, which is what makes the deletion possible + // at all: the agent refuses this subtree without it. + assert.match((screen as never as { text: () => string }).text(), /Delete anyway/); +}); + +test("a leaf holding nothing says so, and does not ask for force", async () => { + const { screen } = await mount({}, [ctx("acme"), ctx("acme-corp")]); + await openPreview(screen as never); + const text = (screen as never as { text: () => string }).text(); + assert.match(text, /holds nothing, and has no sub-contexts/); + assert.doesNotMatch(text, /Delete anyway/, "an empty leaf must not need force"); +}); + +test("grants and templates count as contents, not only keys and DIDs", async () => { + // A context whose only contents are a grant used to render empty and skip + // the force control entirely. + const { screen } = await mount( + { + aclEntriesRemoved: ["did:key:z6MkBankBot"], + didTemplates: ["bank-persona"], + }, + [ctx("acme"), ctx("acme-corp")], + ); + await openPreview(screen as never); + const text = (screen as never as { text: () => string }).text(); + assert.match(text, /losing access entirely/, "the subject losing all authority is not shown"); + assert.match(text, /z6MkBankBot|z6MkB/, "the subject is not named"); + assert.match(text, /bank-persona/, "the template is not named"); + assert.match(text, /Delete anyway/, "a context holding a grant must need force"); +}); From 8c4d4957d364df1c1874e3a1596a2c5f159a7684 Mon Sep 17 00:00:00 2001 From: Glenn Gore Date: Sat, 19 Sep 2026 13:40:01 +0200 Subject: [PATCH 2/2] test(core): pin the whole preview shape, not the three fields the client used to return `contextPreviewDelete` now returns all six of the agent's lists, so the two tests asserting the old three-field object were asserting the defect. They become: every snake_case list translates, every camelCase spelling is read (which is what was missing), and all six default when the context holds nothing. Signed-off-by: Glenn Gore --- packages/core/tests/admin.contexts.mjs | 50 +++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/packages/core/tests/admin.contexts.mjs b/packages/core/tests/admin.contexts.mjs index e7986bd..18082ab 100644 --- a/packages/core/tests/admin.contexts.mjs +++ b/packages/core/tests/admin.contexts.mjs @@ -51,14 +51,54 @@ test("a delete the agent refused reports deleted:false rather than throwing", as assert.equal(result.deleted, false); }); -test("preview translates webvh_dids across the casing boundary", async () => { - const channel = recorder({ id: "demo", keys: ["key-1"], webvh_dids: ["did:webvh:QmX:h"] }); +test("preview translates every snake_case list across the casing boundary", async () => { + const channel = recorder({ + id: "demo", + keys: ["key-1"], + webvh_dids: ["did:webvh:QmX:h"], + acl_entries_removed: ["did:key:zGone"], + acl_entries_updated: ["did:key:zNarrowed"], + did_templates: ["persona"], + }); const result = await contextPreviewDelete(channel, { holder: HOLDER, service: SERVICE, id: "demo" }); - assert.deepEqual(result, { id: "demo", keys: ["key-1"], webvhDids: ["did:webvh:QmX:h"] }); + assert.deepEqual(result, { + id: "demo", + keys: ["key-1"], + webvhDids: ["did:webvh:QmX:h"], + aclEntriesRemoved: ["did:key:zGone"], + aclEntriesUpdated: ["did:key:zNarrowed"], + didTemplates: ["persona"], + }); }); -test("preview defaults both lists when the context holds nothing", async () => { +test("preview reads the camelCase spellings the spec actually declares", async () => { + // The snake_case arms above are the compatibility path, for an agent that + // has not taken the camelCase change. These are what a conforming agent + // sends, and reading them was what the client was missing: three of the six + // lists were dropped on the floor, so no consumer could show them. + const channel = recorder({ + id: "demo", + keys: [], + webvhDids: [], + aclEntriesRemoved: ["did:key:zGone"], + aclEntriesUpdated: ["did:key:zNarrowed"], + didTemplates: ["persona"], + }); + const result = await contextPreviewDelete(channel, { holder: HOLDER, service: SERVICE, id: "demo" }); + assert.deepEqual(result.aclEntriesRemoved, ["did:key:zGone"]); + assert.deepEqual(result.aclEntriesUpdated, ["did:key:zNarrowed"]); + assert.deepEqual(result.didTemplates, ["persona"]); +}); + +test("preview defaults every list when the context holds nothing", async () => { const channel = recorder({ id: "demo" }); const result = await contextPreviewDelete(channel, { holder: HOLDER, service: SERVICE, id: "demo" }); - assert.deepEqual(result, { id: "demo", keys: [], webvhDids: [] }); + assert.deepEqual(result, { + id: "demo", + keys: [], + webvhDids: [], + aclEntriesRemoved: [], + aclEntriesUpdated: [], + didTemplates: [], + }); });