diff --git a/TESTING_COVERAGE.md b/TESTING_COVERAGE.md index dc3c3a192c..58258284ba 100644 --- a/TESTING_COVERAGE.md +++ b/TESTING_COVERAGE.md @@ -312,14 +312,21 @@ installation's agent). `browser/lib/src/plugin-route-grant.test.ts` checks `installRelease` produces exactly that, signed by the installer; that a `config:` target that doesn't resolve refuses before anything is committed; that revoking takes the agent off the parent and leaves other writers; and -that an upgrade replaces the grant and moves the rights. +that an upgrade replaces the grant and moves the rights. A config edit on the +Installation page that moves a `config:` target (#1758): the same file checks +`routeWriteConfigChange` finds the moved targets and refuses an unresolved one, +and that `saveInstallationConfig` moves `write` to the new parent when +approved, drops the grant and the rights when declined, commits nothing for an +unresolved target and only saves when nothing moves. +`RouteWriteMoveDialog.test.tsx` checks the dialog marks the new parent New, +starts unchecked and saves with the choice. `route_writes::http_tests::a_page_shaped_install_stores_a_post` installs that fixture and POSTs (`2xx`). `RouteWriteApproval.test.tsx` covers the review: unchecked by default, the targets passed only once approved, install disabled while a target doesn't resolve, not offered below `read-write`, and an update kept approved or asked again. Not covered: a Playwright run of install, POST -and revoke (done by hand for #1717's follow-up), and the rights following a -config edit on the Installation page. +and revoke (done by hand for #1717's follow-up), and a Playwright run of the +config edit (done by hand for #1758). The data-browser no longer connects or syncs LocalThought platforms: that code was removed, and plugins will run in their own iframe and make proxy calls diff --git a/browser/data-browser/src/chunks/Plugins/PublicEndpoints.test.tsx b/browser/data-browser/src/chunks/Plugins/PublicEndpoints.test.tsx index 02bf3c6bd8..aa070b1247 100644 --- a/browser/data-browser/src/chunks/Plugins/PublicEndpoints.test.tsx +++ b/browser/data-browser/src/chunks/Plugins/PublicEndpoints.test.tsx @@ -19,12 +19,7 @@ import activitypub from '../../../../../testdata/plugin-manifest/v3-activitypub. // The dialog's chrome needs a real and the app's providers; what is // under test is its content and its Install button. -const dialog = vi.hoisted(() => [ - {}, - () => undefined, - () => undefined, - true, -]); +const dialog = vi.hoisted(() => [{}, () => undefined, () => undefined, true]); vi.mock('@components/Dialog', async original => { const Pass = ({ children }: { children?: React.ReactNode }) => ( diff --git a/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.test.tsx b/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.test.tsx new file mode 100644 index 0000000000..6b8004be69 --- /dev/null +++ b/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.test.tsx @@ -0,0 +1,93 @@ +// @vitest-environment jsdom +// @wc-ignore-file +import { cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { ThemeProvider } from 'styled-components'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + grantsWithRouteWrites, + routeWriteConfigChange, + type DeclaredWriteTarget, +} from '@tomic/react'; +import { buildTheme } from '../../styling'; +import { RouteWriteMoveDialog } from './RouteWriteMoveDialog'; +import inbox from '../../../../../testdata/plugin-routes/inbox/manifest.json'; + +const dialog = vi.hoisted(() => [{}, () => undefined, () => undefined, true]); + +vi.mock('@components/Dialog', async original => { + const Pass = ({ children }: { children?: React.ReactNode }) => ( +
{children}
+ ); + + return { + ...(await original()), + Dialog: Object.assign(Pass, { + Title: Pass, + Content: Pass, + Actions: Pass, + }), + useDialog: () => dialog, + }; +}); +vi.mock('@views/ResourceInline', () => ({ + ResourceInline: ({ subject }: { subject: string }) => {subject}, +})); + +afterEach(cleanup); + +// #1754's fixture: `inbox-items` under `config:inbox`. +const TARGETS = inbox.http.writeTargets as DeclaredWriteTarget[]; +const OLD = 'https://atomic.test/inbox'; +const NEW = 'https://atomic.test/elsewhere'; + +function renderMove() { + const change = routeWriteConfigChange( + grantsWithRouteWrites(['storage'], TARGETS), + { inbox: OLD }, + { inbox: NEW }, + )!; + const onSave = vi.fn(async (_approve: boolean) => undefined); + render( + + undefined} + /> + , + ); + + return onSave; +} + +describe('moving a write target in the config', () => { + it('shows the new parent as New and asks for approval, unchecked', () => { + renderMove(); + + const target = screen.getByTestId('route-write-target-inbox-items'); + expect(target.textContent).toContain(NEW); + expect(screen.getByTestId('route-write-new')).toBeTruthy(); + expect( + (screen.getByTestId('route-write-approval') as HTMLInputElement).checked, + ).toBe(false); + }); + + it('saves with the approval when it is given', async () => { + const onSave = renderMove(); + + fireEvent.click(screen.getByTestId('route-write-approval')); + fireEvent.click(screen.getByRole('button', { name: 'Save config' })); + + await vi.waitFor(() => expect(onSave).toHaveBeenCalledWith(true)); + }); + + it('saves declined when it is left unchecked', async () => { + const onSave = renderMove(); + + fireEvent.click(screen.getByRole('button', { name: 'Save config' })); + + await vi.waitFor(() => expect(onSave).toHaveBeenCalledWith(false)); + }); +}); diff --git a/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.tsx b/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.tsx new file mode 100644 index 0000000000..2733383ef9 --- /dev/null +++ b/browser/data-browser/src/chunks/Plugins/RouteWriteMoveDialog.tsx @@ -0,0 +1,91 @@ +import { Button } from '@components/Button'; +import { Dialog, useDialog } from '@components/Dialog'; +import { Column } from '@components/Row'; +import type { JSONValue, RouteWriteConfigChange } from '@tomic/react'; +import { useEffect, useState } from 'react'; +import { RouteWriteApproval } from './RouteWriteApproval'; + +interface RouteWriteMoveDialogProps { + plugin: string; + /** The move to approve; the dialog opens while it is set. */ + change: RouteWriteConfigChange | undefined; + config: JSONValue | undefined; + /** Saves the config; `approve` says whether the moved targets were approved. */ + onSave: (approve: boolean) => Promise; + /** Called after the dialog closed, whether or not anything was saved. */ + onClose: () => void; +} + +/** + * Asks again for the route grant when a config change points a write target + * somewhere else (#1758), the way an upgrade review asks for new targets: + * the moved targets are marked New and nothing is approved by default. + * Approving moves the plugin's write rights to the new parent; saving + * unchecked drops the route grant and the rights it came with. + */ +export function RouteWriteMoveDialog({ + plugin, + change, + config, + onSave, + onClose, +}: RouteWriteMoveDialogProps) { + const [approve, setApprove] = useState(false); + const [busy, setBusy] = useState(false); + const [dialogProps, show, hide] = useDialog({ + onCancel: onClose, + onSuccess: onClose, + }); + + useEffect(() => { + if (!change) return; + setApprove(false); + show(); + }, [change, show]); + + if (!change) return null; + + const save = async () => { + setBusy(true); + + try { + await onSave(approve); + hide(true); + } finally { + setBusy(false); + } + }; + + return ( + + +

Move incoming items

+ + + +

+ This config stores what other servers send somewhere else. Approve + the new place to move the plugin’s write rights there, or save + without it to stop accepting these items. +

+ + + + + + + + + ); +} diff --git a/browser/data-browser/src/locales/de.po b/browser/data-browser/src/locales/de.po index d27fe8ffb3..017d5f8494 100644 --- a/browser/data-browser/src/locales/de.po +++ b/browser/data-browser/src/locales/de.po @@ -117,6 +117,7 @@ msgstr "" #: src/chunks/DashboardPage/BlockConfigDialog.tsx #: src/chunks/PluginRuns/RunPluginDialog.tsx #: src/chunks/Plugins/InstallationReviewDialog.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/chunks/TablePage/BreakdownDialog.tsx #: src/chunks/TablePage/DerivedColumnDialog.tsx #: src/chunks/TablePage/Issues/IssuesView.tsx @@ -1165,6 +1166,7 @@ msgstr "" #: src/chunks/PluginRuns/AutomationWorkspace.tsx #: src/chunks/PluginRuns/IntegrationConnection.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/components/TeamProfileStep.tsx #: src/views/Canvas/CanvasPage.tsx msgid "Saving…" @@ -9590,3 +9592,27 @@ msgstr "" #: src/chunks/Plugins/RouteWriteApproval.tsx msgid "Route writes" msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved, rights moved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Set <0/> to the resource that should receive incoming items. The plugin's route grant stores them there, so this config can't be saved without it." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Move incoming items" +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "This config stores what other servers send somewhere else. Approve the new place to move the plugin’s write rights there, or save without it to stop accepting these items." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Save config" +msgstr "" diff --git a/browser/data-browser/src/locales/en.po b/browser/data-browser/src/locales/en.po index 25b0d0bd7e..2c147640c2 100644 --- a/browser/data-browser/src/locales/en.po +++ b/browser/data-browser/src/locales/en.po @@ -117,6 +117,7 @@ msgstr "AI Agents" #: src/chunks/DashboardPage/BlockConfigDialog.tsx #: src/chunks/PluginRuns/RunPluginDialog.tsx #: src/chunks/Plugins/InstallationReviewDialog.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/chunks/TablePage/BreakdownDialog.tsx #: src/chunks/TablePage/DerivedColumnDialog.tsx #: src/chunks/TablePage/Issues/IssuesView.tsx @@ -1165,6 +1166,7 @@ msgstr "Save failed: {0}" #: src/chunks/PluginRuns/AutomationWorkspace.tsx #: src/chunks/PluginRuns/IntegrationConnection.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/components/TeamProfileStep.tsx #: src/views/Canvas/CanvasPage.tsx msgid "Saving…" @@ -9608,3 +9610,27 @@ msgstr "The plugin gets write rights on these resources, and may only change or #: src/chunks/Plugins/RouteWriteApproval.tsx msgid "Route writes" msgstr "Route writes" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved, rights moved" +msgstr "Config saved, rights moved" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved" +msgstr "Config saved" + +#: src/views/Installation/InstallationPage.tsx +msgid "Set <0/> to the resource that should receive incoming items. The plugin's route grant stores them there, so this config can't be saved without it." +msgstr "Set <0/> to the resource that should receive incoming items. The plugin's route grant stores them there, so this config can't be saved without it." + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Move incoming items" +msgstr "Move incoming items" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "This config stores what other servers send somewhere else. Approve the new place to move the plugin’s write rights there, or save without it to stop accepting these items." +msgstr "This config stores what other servers send somewhere else. Approve the new place to move the plugin’s write rights there, or save without it to stop accepting these items." + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Save config" +msgstr "Save config" diff --git a/browser/data-browser/src/locales/es.po b/browser/data-browser/src/locales/es.po index 3c6583dedc..d479853124 100644 --- a/browser/data-browser/src/locales/es.po +++ b/browser/data-browser/src/locales/es.po @@ -117,6 +117,7 @@ msgstr "Agentes de IA" #: src/chunks/DashboardPage/BlockConfigDialog.tsx #: src/chunks/PluginRuns/RunPluginDialog.tsx #: src/chunks/Plugins/InstallationReviewDialog.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/chunks/TablePage/BreakdownDialog.tsx #: src/chunks/TablePage/DerivedColumnDialog.tsx #: src/chunks/TablePage/Issues/IssuesView.tsx @@ -1165,6 +1166,7 @@ msgstr "Error al guardar: {0}" #: src/chunks/PluginRuns/AutomationWorkspace.tsx #: src/chunks/PluginRuns/IntegrationConnection.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/components/TeamProfileStep.tsx #: src/views/Canvas/CanvasPage.tsx msgid "Saving…" @@ -9590,3 +9592,27 @@ msgstr "" #: src/chunks/Plugins/RouteWriteApproval.tsx msgid "Route writes" msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved, rights moved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Set <0/> to the resource that should receive incoming items. The plugin's route grant stores them there, so this config can't be saved without it." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Move incoming items" +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "This config stores what other servers send somewhere else. Approve the new place to move the plugin’s write rights there, or save without it to stop accepting these items." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Save config" +msgstr "" diff --git a/browser/data-browser/src/locales/fr.po b/browser/data-browser/src/locales/fr.po index b9041e994e..92bd1c0b38 100644 --- a/browser/data-browser/src/locales/fr.po +++ b/browser/data-browser/src/locales/fr.po @@ -117,6 +117,7 @@ msgstr "Agents IA" #: src/chunks/DashboardPage/BlockConfigDialog.tsx #: src/chunks/PluginRuns/RunPluginDialog.tsx #: src/chunks/Plugins/InstallationReviewDialog.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/chunks/TablePage/BreakdownDialog.tsx #: src/chunks/TablePage/DerivedColumnDialog.tsx #: src/chunks/TablePage/Issues/IssuesView.tsx @@ -1165,6 +1166,7 @@ msgstr "Échec de l'enregistrement : {0}" #: src/chunks/PluginRuns/AutomationWorkspace.tsx #: src/chunks/PluginRuns/IntegrationConnection.tsx +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx #: src/components/TeamProfileStep.tsx #: src/views/Canvas/CanvasPage.tsx msgid "Saving…" @@ -9590,3 +9592,27 @@ msgstr "" #: src/chunks/Plugins/RouteWriteApproval.tsx msgid "Route writes" msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved, rights moved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Config saved" +msgstr "" + +#: src/views/Installation/InstallationPage.tsx +msgid "Set <0/> to the resource that should receive incoming items. The plugin's route grant stores them there, so this config can't be saved without it." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Move incoming items" +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "This config stores what other servers send somewhere else. Approve the new place to move the plugin’s write rights there, or save without it to stop accepting these items." +msgstr "" + +#: src/chunks/Plugins/RouteWriteMoveDialog.tsx +msgid "Save config" +msgstr "" diff --git a/browser/data-browser/src/views/Installation/InstallationPage.tsx b/browser/data-browser/src/views/Installation/InstallationPage.tsx index b4b01143ef..108777c6d4 100644 --- a/browser/data-browser/src/views/Installation/InstallationPage.tsx +++ b/browser/data-browser/src/views/Installation/InstallationPage.tsx @@ -14,6 +14,8 @@ import { publishZipRelease, readInstallationReview, routeGrantOf, + routeWriteConfigChange, + saveInstallationConfig, server, updateInstallationRelease, withdrawRouteWriteRights, @@ -25,6 +27,7 @@ import { type DeclaredWriteTarget, type InstallationStatus, type JSONValue, + type RouteWriteConfigChange, type Server, } from '@tomic/react'; import type { ResourcePageProps } from '@views/ResourcePage'; @@ -49,12 +52,15 @@ import { import { ConfigReference } from './ConfigReference'; import { AssignRights } from './AssignRights'; import { useInstallationConfigSchema } from './useInstallationConfigSchema'; +import { useConfigDraft } from './useConfigDraft'; import { ResourceInline } from '@views/ResourceInline/ResourceInline'; import { useCustomViews } from '@components/CustomViewProvider'; import { InstallationReviewDialog, type PendingInstallation, } from '@chunks/Plugins/InstallationReviewDialog'; +import { unresolvedWriteTarget } from '@chunks/Plugins/RouteWriteApproval'; +import { RouteWriteMoveDialog } from '@chunks/Plugins/RouteWriteMoveDialog'; const UPDATE_VERB = { title: 'Update plugin', @@ -101,7 +107,16 @@ export const InstallationPage: React.FC< const [pluginAgent] = useString(resource, server.properties.pluginAgent); const [configValid, setConfigValid] = useState(true); const [configSyntaxValid, setConfigSyntaxValid] = useState(true); - const [configEdited, setConfigEdited] = useState(false); + // The config as saved, which the route grant's rights follow. The editor + // writes into the resource as you type, so this is kept apart. + const { + edited: configEdited, + savedConfig, + markEdited: markConfigEdited, + commit: commitConfigDraft, + } = useConfigDraft(config); + const [routeWriteMove, setRouteWriteMove] = + useState(); const saveState = useSaveState(resource); const [changing, setChanging] = useState(false); const [pending, setPending] = useState(); @@ -130,6 +145,43 @@ export const InstallationPage: React.FC< : []), ]; + // A target the edited config leaves unresolved refuses the save: the route + // grant would let other servers send items the plugin can't store. + const unresolved = routeGrant + ? unresolvedWriteTarget(routeGrant, config as JSONValue | undefined) + : undefined; + + const commitConfig = async (approveRouteWrites: boolean) => { + await commitConfigDraft(config, previousConfig => + saveInstallationConfig(store, resource.subject, { + config: config as JSONValue | undefined, + previousConfig, + approveRouteWrites, + }), + ); + }; + + const saveConfig = async () => { + try { + const move = routeWriteConfigChange( + grants, + savedConfig, + config as JSONValue | undefined, + ); + + // A moved write target is approved again first, as in an upgrade. + if (move) { + setRouteWriteMove(move); + + return; + } + + await commitConfig(true); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + } + }; + const changeStatus = async (next: InstallationStatus) => { setChanging(true); @@ -345,15 +397,12 @@ export const InstallationPage: React.FC< disabled={ !configValid || !configSyntaxValid || + !!unresolved || saveState.kind === 'saving' || saveState.kind === 'scheduled' || (!configEdited && saveState.kind !== 'dirty') } - onClick={() => { - setConfigEdited(false); - - return resource.save(); - }} + onClick={saveConfig} > Save @@ -366,7 +415,7 @@ export const InstallationPage: React.FC< onChange={v => { try { setConfig(JSON.parse(v)); - setConfigEdited(true); + markConfigEdited(); setConfigSyntaxValid(true); } catch { setConfigSyntaxValid(false); @@ -376,6 +425,7 @@ export const InstallationPage: React.FC< showErrorStyling={!configValid} onValidationChange={setConfigValid} /> + {unresolved && } {schema && } {declared.length > 0 && ( @@ -388,6 +438,23 @@ export const InstallationPage: React.FC< onInstall={applyUpdate} verb={UPDATE_VERB} /> + { + try { + await commitConfig(approve); + toast.success( + approve ? 'Config saved, rights moved' : 'Config saved', + ); + } catch (err) { + toast.error(err instanceof Error ? err.message : String(err)); + throw err; + } + }} + onClose={() => setRouteWriteMove(undefined)} + /> + Set {keyName} to the resource that should receive incoming + items. The plugin's route grant stores them there, so this config + can't be saved without it. + + ); +} + +const ConfigAlert = styled.p` + color: ${p => p.theme.colors.alert}; + margin: 0; +`; + const PluginName = styled.span` font-weight: bold; font-size: 1.2rem; diff --git a/browser/data-browser/src/views/Installation/useConfigDraft.test.ts b/browser/data-browser/src/views/Installation/useConfigDraft.test.ts new file mode 100644 index 0000000000..a51a32b7c0 --- /dev/null +++ b/browser/data-browser/src/views/Installation/useConfigDraft.test.ts @@ -0,0 +1,79 @@ +// @vitest-environment jsdom +// @wc-ignore-file +import { act, cleanup, renderHook } from '@testing-library/react'; +import { afterEach, expect, it } from 'vitest'; +import { useConfigDraft } from './useConfigDraft'; + +afterEach(cleanup); + +type Config = { folderPrefix: string }; + +// Stable, as `useValue` returns it: a new object each render would be a new +// config each render. +const DRAFT: Config = { folderPrefix: 'Draft' }; +const FINAL: Config = { folderPrefix: 'Final' }; + +function deferred() { + let resolve!: () => void; + const promise = new Promise(r => (resolve = r)); + + return { promise, resolve }; +} + +it('clears the edit once the latest draft is saved', async () => { + // The editor writes into the resource, so `config` is the draft. + const { result } = renderHook(() => useConfigDraft(FINAL)); + + act(() => result.current.markEdited()); + expect(result.current.edited).toBe(true); + + await act(() => result.current.commit(FINAL, () => Promise.resolve())); + + expect(result.current.edited).toBe(false); + expect(result.current.savedConfig).toEqual({ folderPrefix: 'Final' }); +}); + +// plugin.spec "install a plugin": a plugin with no write targets, saved +// offline. The first save settles after the next edit; Save must stay +// available for that edit. +it('keeps a later edit when an earlier save settles after it', async () => { + const { result } = renderHook(() => useConfigDraft(FINAL)); + const first = deferred(); + let previous: Config | undefined; + + act(() => result.current.markEdited()); + let saving!: Promise; + act(() => { + saving = result.current.commit({ folderPrefix: 'Offline' }, prev => { + previous = prev; + + return first.promise; + }); + }); + act(() => result.current.markEdited()); + + await act(async () => { + first.resolve(); + await saving; + }); + + expect(previous).toEqual({ folderPrefix: 'Final' }); + expect(result.current.edited).toBe(true); + expect(result.current.savedConfig).toEqual({ folderPrefix: 'Offline' }); +}); + +it('keeps the edit when the save fails', async () => { + const { result } = renderHook(() => useConfigDraft(DRAFT)); + + act(() => result.current.markEdited()); + await act(async () => { + await expect( + result.current.commit({ folderPrefix: 'Final' }, () => + Promise.reject(new Error('refused')), + ), + ).rejects.toThrow('refused'); + }); + + expect(result.current.edited).toBe(true); + expect(result.current.savedConfig).toEqual({ folderPrefix: 'Draft' }); +}); diff --git a/browser/data-browser/src/views/Installation/useConfigDraft.ts b/browser/data-browser/src/views/Installation/useConfigDraft.ts new file mode 100644 index 0000000000..fea9218c20 --- /dev/null +++ b/browser/data-browser/src/views/Installation/useConfigDraft.ts @@ -0,0 +1,47 @@ +import { useEffect, useRef, useState } from 'react'; + +export interface ConfigDraft { + /** Whether the editor holds changes no save has covered yet. */ + edited: boolean; + /** The config as last saved, which the route grant's rights follow. */ + savedConfig: T; + /** Call on every editor change. */ + markEdited: () => void; + /** + * Saves `draft` through `write`, which gets the config saved before it. + * Only a save of the latest draft clears `edited`: an offline save can + * settle after the next edit, and must not take that edit's Save away. + */ + commit: (draft: T, write: (previous: T) => Promise) => Promise; +} + +/** + * The Installation page's config draft. The editor writes into the resource + * as you type, so the saved config is kept apart from it. + */ +export function useConfigDraft(config: T): ConfigDraft { + const [edited, setEdited] = useState(false); + const [savedConfig, setSavedConfig] = useState(config); + // Counts edits, so a save that settles late can tell whether it still + // covers what the editor holds. + const edits = useRef(0); + + useEffect(() => { + if (!edited) setSavedConfig(config); + }, [config, edited]); + + const markEdited = () => { + edits.current++; + setEdited(true); + }; + + const commit = async (draft: T, write: (previous: T) => Promise) => { + const editsAtSave = edits.current; + await write(savedConfig); + setSavedConfig(draft); + + if (edits.current === editsAtSave) setEdited(false); + }; + + return { edited, savedConfig, markEdited, commit }; +} diff --git a/browser/lib/src/index.ts b/browser/lib/src/index.ts index e0ae9b9bc4..ce9a3ea194 100644 --- a/browser/lib/src/index.ts +++ b/browser/lib/src/index.ts @@ -287,6 +287,8 @@ export { installRelease, updateInstallationRelease, withdrawRouteWriteRights, + saveInstallationConfig, + type SaveConfigOptions, publishZipRelease, readInstallationReview, grantsFor, @@ -315,7 +317,9 @@ export { routeGrantOf, routeWriteParentsOf, routeWriteRightsDiff, + routeWriteConfigChange, type InstallationGrant, + type RouteWriteConfigChange, } from './plugin-route-grant.js'; export * from './integration-actions.js'; diff --git a/browser/lib/src/plugin-install.ts b/browser/lib/src/plugin-install.ts index 7d45dd8cc6..4aac37f5fd 100644 --- a/browser/lib/src/plugin-install.ts +++ b/browser/lib/src/plugin-install.ts @@ -20,11 +20,13 @@ import type { DeclaredWriteTarget, } from './plugin-manifest-http.js'; import { + capabilityGrantNames, fetchPluginAgent, giveRouteWriteRights, grantsWithRouteWrites, removeRouteWriteRights, resolveWriteTargetParents, + routeGrantOf, routeWriteParentsOf, routeWriteRightsDiff, } from './plugin-route-grant.js'; @@ -453,6 +455,68 @@ export async function withdrawRouteWriteRights( await removeRouteWriteRights(store, agent, parents); } +export interface SaveConfigOptions { + config: JSONValue | undefined; + /** The config the Installation had before this edit (its saved value). */ + previousConfig: unknown; + /** + * Whether the installer approved the route-write targets at the parents + * `config` resolves them to. Declining drops the route grant and the rights + * it came with, as declining an upgrade review's new targets does. Ignored + * without a route grant. + */ + approveRouteWrites: boolean; +} + +/** + * Saves an Installation's config and moves the route-write rights with it. + * When the config points a `config:` write target at another resource, the + * plugin's agent gets `write` on the new parent and loses it on the old one, + * in commits the signed-in agent signs, and `route-writes` in `grants` is + * rewritten: kept when approved, dropped when declined. A target the new + * config leaves unresolved refuses the save before anything is committed. + */ +export async function saveInstallationConfig( + store: Store, + installation: string, + options: SaveConfigOptions, +): Promise { + const { config, previousConfig, approveRouteWrites } = options; + const resource = await store.getResource(installation); + const grants = resource.get(server.properties.grants); + const targets = routeGrantOf(grants); + const keep = !!targets && targets.length > 0 && approveRouteWrites; + const before = targets ? routeWriteParentsOf(grants, previousConfig) : []; + // Throws before anything is committed. + const after = keep ? resolveWriteTargetParents(targets, config) : []; + const { give, remove } = routeWriteRightsDiff(before, after); + const agent = + give.length > 0 || remove.length > 0 + ? await fetchPluginAgent(store, installation) + : undefined; + + await resource.set(server.properties.config, config, false, Datatype.JSON); + + if (targets) { + await resource.set( + server.properties.grants, + grantsWithRouteWrites( + capabilityGrantNames(grants), + keep ? targets : undefined, + ) as JSONValue, + false, + Datatype.JSON, + ); + } + + await resource.save(); + + if (agent) { + await giveRouteWriteRights(store, agent, give); + await removeRouteWriteRights(store, agent, remove); + } +} + type InstallStore = Pick; /** diff --git a/browser/lib/src/plugin-manifest-http.ts b/browser/lib/src/plugin-manifest-http.ts index df2e62b7e5..f482b94ad4 100644 --- a/browser/lib/src/plugin-manifest-http.ts +++ b/browser/lib/src/plugin-manifest-http.ts @@ -747,11 +747,13 @@ export function requiresGate( entry.slice(entry.indexOf(':') + 1), ]; if (kind === 'plugin-routes') top = Math.max(top, rank(value)); + // Only nodes at `read-write` configure listeners and sidecars. if (kind === 'operator-listener') { listeners.push(value); top = Math.max(top, rank('read-write')); } + if (kind === 'operator-sidecar') { sidecars.push(value); top = Math.max(top, rank('read-write')); diff --git a/browser/lib/src/plugin-route-grant.test.ts b/browser/lib/src/plugin-route-grant.test.ts index 3f7b089476..0072cf14b6 100644 --- a/browser/lib/src/plugin-route-grant.test.ts +++ b/browser/lib/src/plugin-route-grant.test.ts @@ -4,6 +4,7 @@ import { core } from './ontologies/core.js'; import { server } from './ontologies/server.js'; import { installRelease, + saveInstallationConfig, updateInstallationRelease, withdrawRouteWriteRights, } from './plugin-install.js'; @@ -14,6 +15,7 @@ import { grantsWithRouteWrites, newWriteTargets, routeGrantOf, + routeWriteConfigChange, } from './plugin-route-grant.js'; import type { Store } from './store.js'; import { testStore } from './test-store.js'; @@ -251,6 +253,129 @@ describe('revoking route writes', () => { }); }); +describe('a config change that moves a write target', () => { + async function installed() { + const t = await testStore(); + const { drive, inbox } = await driveWithInbox(t.store); + const fetchAgent = serveAgent(t.store); + const subject = await installRelease(t.store, { + drive, + release: RELEASE, + name: 'inbox', + config: { inbox }, + grants: ['storage'], + routeWrites: TARGETS, + }); + const elsewhere = await t.store.newResource({ + parent: drive, + propVals: { [core.properties.name]: 'Elsewhere' }, + }); + await elsewhere.save(); + fetchAgent.mockClear(); + + return { ...t, subject, inbox, elsewhere: elsewhere.subject, fetchAgent }; + } + + it('is detected, and only when a resolved parent changes', () => { + const grants = grantsWithRouteWrites(['storage'], TARGETS); + + expect( + routeWriteConfigChange(grants, { inbox: 'a' }, { inbox: 'b' }), + ).toEqual({ targets: TARGETS, moved: TARGETS }); + // A config stored as a JSON string reads the same. + expect( + routeWriteConfigChange(grants, JSON.stringify({ inbox: 'a' }), { + inbox: 'b', + })?.moved, + ).toEqual(TARGETS); + expect( + routeWriteConfigChange(grants, { inbox: 'a' }, { inbox: 'a', x: 1 }), + ).toBeUndefined(); + expect( + routeWriteConfigChange(['storage'], { inbox: 'a' }, { inbox: 'b' }), + ).toBeUndefined(); + expect(() => + routeWriteConfigChange(grants, { inbox: 'a' }, { inbox: '' }), + ).toThrow(UnresolvedWriteTargetError); + }); + + it('moves the plugin agent’s write to the new parent when approved', async () => { + const { store, posted, agentDID, subject, inbox, elsewhere } = + await installed(); + const before = posted.length; + + await saveInstallationConfig(store, subject, { + config: { inbox: elsewhere }, + previousConfig: { inbox }, + approveRouteWrites: true, + }); + + const installation = store.getResourceLoading(subject); + const config = installation.get(server.properties.config); + expect(typeof config === 'string' ? JSON.parse(config) : config).toEqual({ + inbox: elsewhere, + }); + expect(routeGrantOf(installation.get(server.properties.grants))).toEqual( + TARGETS, + ); + expect(writers(store, elsewhere)).toEqual([PLUGIN_AGENT]); + expect(writers(store, inbox)).toEqual([OTHER_WRITER]); + // The Installation, then the new parent's grant, then the old one's + // withdrawal, all signed by the installer. + const commits = posted.slice(before); + expect(commits.map(c => c.subject)).toEqual([subject, elsewhere, inbox]); + expect(commits.every(c => c.signer === agentDID)).toBe(true); + }); + + it('drops the route grant and the rights when the approval is declined', async () => { + const { store, posted, subject, inbox, elsewhere } = await installed(); + const before = posted.length; + + await saveInstallationConfig(store, subject, { + config: { inbox: elsewhere }, + previousConfig: { inbox }, + approveRouteWrites: false, + }); + + const installation = store.getResourceLoading(subject); + expect(installation.get(server.properties.grants)).toEqual(['storage']); + expect(writers(store, inbox)).toEqual([OTHER_WRITER]); + expect(writers(store, elsewhere)).toEqual([]); + expect(posted.slice(before).map(c => c.subject)).toEqual([subject, inbox]); + }); + + it('refuses a config that leaves a target unresolved, committing nothing', async () => { + const { store, posted, subject, inbox, fetchAgent } = await installed(); + const before = posted.length; + + await expect( + saveInstallationConfig(store, subject, { + config: { inbox: '' }, + previousConfig: { inbox }, + approveRouteWrites: true, + }), + ).rejects.toThrow(UnresolvedWriteTargetError); + expect(posted.length).toBe(before); + expect(fetchAgent).not.toHaveBeenCalled(); + expect(writers(store, inbox)).toContain(PLUGIN_AGENT); + }); + + it('just saves the config when no target moves', async () => { + const { store, posted, subject, inbox, fetchAgent } = await installed(); + const before = posted.length; + + await saveInstallationConfig(store, subject, { + config: { inbox, note: 'hi' }, + previousConfig: { inbox }, + approveRouteWrites: true, + }); + + expect(posted.slice(before).map(c => c.subject)).toEqual([subject]); + expect(fetchAgent).not.toHaveBeenCalled(); + expect(writers(store, inbox)).toContain(PLUGIN_AGENT); + }); +}); + describe('upgrading with route writes', () => { it('replaces the grant with the widened targets and gives rights on the new parent', async () => { const { store } = await testStore(); diff --git a/browser/lib/src/plugin-route-grant.ts b/browser/lib/src/plugin-route-grant.ts index c0f66a75a1..94d4ba21a6 100644 --- a/browser/lib/src/plugin-route-grant.ts +++ b/browser/lib/src/plugin-route-grant.ts @@ -324,3 +324,42 @@ export function routeWriteRightsDiff( remove: before.filter(p => !after.includes(p)), }; } + +/** What a config change does to an Installation's route grant. */ +export interface RouteWriteConfigChange { + /** The targets the route grant approves, unchanged by the config. */ + targets: DeclaredWriteTarget[]; + /** + * The targets whose parent the new config resolves elsewhere: the ones to + * approve again, as an upgrade review asks for new targets. + */ + moved: DeclaredWriteTarget[]; +} + +/** + * Whether saving `after` over `before` moves a route-write target. Undefined + * when the Installation has no route grant, or every target keeps its parent. + * Throws {@link UnresolvedWriteTargetError} when `after` leaves a target + * unresolved: that save is refused, since the plugin could not store what the + * grant still lets other servers send. + */ +export function routeWriteConfigChange( + grants: unknown, + before: unknown, + after: JSONValue | undefined, +): RouteWriteConfigChange | undefined { + const targets = routeGrantOf(grants); + if (!targets || targets.length === 0) return undefined; + const previous = parseGrants(before) as JSONValue | undefined; + const moved = targets.filter(target => { + const next = resolveWriteTargetParent(target, after); + + try { + return resolveWriteTargetParent(target, previous) !== next; + } catch { + return true; + } + }); + + return moved.length > 0 ? { targets, moved } : undefined; +}