diff --git a/.changeset/action-url-identifies-by-name.md b/.changeset/action-url-identifies-by-name.md new file mode 100644 index 0000000000..4ced444e07 --- /dev/null +++ b/.changeset/action-url-identifies-by-name.md @@ -0,0 +1,26 @@ +--- +'@object-ui/app-shell': major +--- + +**[ADR-0110 D1] The server-action URL identifies an action by `name`, not `target`.** + +`serverActionHandler` posted `action.target || action.name` — the handler's +registration KEY — to `/api/v1/actions/:object/:action`. For a target-bound +action (`{ name: 'complete_task', target: 'completeTask' }`) the server resolves +the declaration by name, so posting the target meant it resolved **no** +declaration and silently skipped both the ADR-0066 D4 capability gate and the +ADR-0104 param contract: a Console button correctly hidden from users without +the capability posted to an endpoint that accepted anyone (framework#3935). + +`target` is a binding expression — a handler key here, a flow id for +`type: 'flow'`, a URL for `type: 'url'`, `${param.X}`-interpolatable, and +legitimately non-unique — so it can never identify a declaration. The URL now +carries `action.name`, and the server derives the handler key from the +declaration it resolves. An action with no `name` is refused rather than +falling back to `target`. + +`apiHandler` and `flowHandler` are unchanged: their `target` genuinely is the +endpoint / flow id they dispatch on. + +Requires a framework with the ADR-0110 handler-key rotation (protocol 17); the +two ship in lockstep. diff --git a/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx b/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx index f6a1740f60..aff52c7deb 100644 --- a/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx +++ b/packages/app-shell/src/hooks/__tests__/useConsoleActionRuntime.test.tsx @@ -282,6 +282,45 @@ describe('useConsoleActionRuntime — authenticated handlers', () => { expect(res).toMatchObject({ success: true }); }); + // [ADR-0110 D1] The action URL identifies the action by `name`. It used to + // post `target || name` — the handler's REGISTRATION KEY — so for a + // target-bound action the server resolved no declaration and silently + // skipped the ADR-0066 D4 capability gate and the ADR-0104 param contract + // (framework#3935). `target` is a binding expression, not an identity. + it('serverActionHandler posts the action NAME, not its target', async () => { + authFetchSpy.mockResolvedValue({ ok: true, json: async () => ({ success: true, data: {} }) }); + const { result } = renderHook(() => + useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'todo_task' }), + ); + + await act(async () => { + // app-todo's real shape — declarative name ≠ handler registration key. + await result.current.serverActionHandler( + { type: 'script', name: 'complete_task', target: 'completeTask' } as any, + { selectedRecords: [{ id: 'task_1' }] } as any, + ); + }); + + const url = String(authFetchSpy.mock.calls[0][0]); + expect(url).toContain('/api/v1/actions/todo_task/complete_task'); + expect(url).not.toContain('completeTask'); + }); + + it('serverActionHandler refuses an action with no name rather than falling back to target', async () => { + const { result } = renderHook(() => + useConsoleActionRuntime({ dataSource: {}, objects: [], objectName: 'todo_task' }), + ); + + let res: any; + await act(async () => { + res = await result.current.serverActionHandler({ type: 'script', target: 'completeTask' } as any); + }); + + expect(res).toMatchObject({ success: false }); + expect(String(res.error)).toMatch(/no name/i); + expect(authFetchSpy).not.toHaveBeenCalled(); + }); + it('serverActionHandler returns a failed action error WITHOUT toasting it (the ActionRunner owns the error toast — no double toast)', async () => { // A script action that throws (e.g. lead_apply_convert validation) returns // { success:false, error } from the server. The handler must NOT toast it — diff --git a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx index 43499737fd..ecea08390c 100644 --- a/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx +++ b/packages/app-shell/src/hooks/useConsoleActionRuntime.tsx @@ -494,9 +494,18 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons // `context` is the shared ActionRunner context (registered handlers are // invoked as `handler(action, runnerContext)`). const serverActionHandler = useCallback(async (action: ActionDef, context?: ActionContext): Promise => { - const targetName = action.target || action.name; - if (!targetName) { - return { success: false, error: 'No action target provided' }; + // [ADR-0110 D1] The URL identifies the action by its declarative `name`, + // never by `target`. `target` is a BINDING EXPRESSION — the handler's + // registration key here, but a URL / flow id / FormView name for other + // types, `${param.X}`-interpolatable, and legitimately non-unique — so it + // cannot identify a declaration. Posting it (the old `target || name`) + // meant the server resolved NO declaration for a target-bound action and + // silently skipped both the ADR-0066 D4 capability gate and the ADR-0104 + // param contract (#3935). The server derives the handler key from the + // declaration it resolves by name. + const actionName = action.name; + if (!actionName) { + return { success: false, error: 'Action has no name — a server action must declare one' }; } const params = (action.params && !Array.isArray(action.params)) ? { ...(action.params as Record) } @@ -523,7 +532,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons } // Re-entrancy guard. - const inflightKey = `${targetName}:${resolvedRecordId ?? ''}`; + const inflightKey = `${actionName}:${resolvedRecordId ?? ''}`; if (serverActionInFlight.current.has(inflightKey)) { return { success: false, error: 'Action already in progress' }; } @@ -580,7 +589,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons } const obj = action.objectName || objApiName || 'global'; const res = await authFetch( - `${baseUrl}/api/v1/actions/${encodeURIComponent(obj)}/${encodeURIComponent(targetName)}`, + `${baseUrl}/api/v1/actions/${encodeURIComponent(obj)}/${encodeURIComponent(actionName)}`, { method: 'POST', headers: { 'Content-Type': 'application/json' }, @@ -591,7 +600,7 @@ export function useConsoleActionRuntime(opts: ConsoleActionRuntimeOptions): Cons // Single source for the `/actions` envelope rule — shared with // RecordDetailView, whose copy of this handler drifted from it and caused // objectstack#3913's console symptom. See utils/actionResponse. - const outcome = interpretActionResponse(res, json, `Action "${targetName}"`); + const outcome = interpretActionResponse(res, json, `Action "${actionName}"`); if (!outcome.ok) { if (preOpenedTab) { try { preOpenedTab.close(); } catch { /* ignore */ } } // Don't toast here — the ActionRunner's post-execution hook surfaces