chore(expert): tool-permissions review follow-ups (comments + generic JsonViewer) (#7730)

Follow-up cleanups agreed during review of #7639 (human-in-the-loop tool permissions). No behaviour change.

## Comments cleanup (#7728)
- Shorten the `/mcp/tools` handler and `curatePlatformTool` doc blocks in `forge/ee/routes/expert/index.js`.
- Drop the stale `TODO: decide with the team whether this belongs on the branch` in the catalog catch block, keeping the rationale for why an upstream 401/403 must not be forwarded as our own 401.
- Trim the `size` prop comment in `ToggleButtonGroup.vue`.

## Generic JsonViewer (#7729)
- Move `JsonViewer.vue` out of the expert message tree into `frontend/src/components` so it can be reused (its only consumer, `ToolApprovalCard`, is updated to the new import).
- Move its `safeStringify` helper into the shared strings composable (`frontend/src/composables/strings/String.js`) and add a unit test.

## Testing
- New `safeStringify` unit tests pass.
- eslint and colour lint green (pre-commit hook).

Closes #7728
Closes #7729

Co-authored-by: andypalmi <andrea@flowfuse.com>

Author: andypalmi

forge/ee/routes/expert/index.js

@@ -16,14 +16,9 @@ const getDeviceComms = (app) => { return app.comms?.devices }
 
 /**
  * Maps a platform automation tool's wire definition into a catalog entry for the
- * Expert permissions UI (#421). Platform tools carry standard MCP annotations
- * (readOnlyHint / destructiveHint), which give the read/write/delete class. They
- * run on the platform, not in Node-RED, so they have no nr-assistant version window
- * (no minVersion/maxVersion — the UI treats their absence as always-available).
- * `group: 'platform'` routes them to the FlowFuse Platform Tools section (groupOf()
- * in the product-assistant store). The friendly label is the tool's own `title`; if a
- * tool ever lacks one, fall back to deriving it from the name (strip the platform_
- * prefix and title-case the rest).
+ * Expert permissions UI. The read/write/delete class comes from the MCP annotations
+ * (readOnlyHint / destructiveHint), and `group: 'platform'` routes it to the platform
+ * section. The label is the tool's own `title`, falling back to a name-derived label.
  */
 const curatePlatformTool = (def) => {
     const annotations = def.annotations || {}
@@ -586,16 +581,10 @@ module.exports = async function (app) {
     })
 
     /**
-     * Retrieve the curated tool catalog for the Expert's human-in-the-loop permissions UI
-     * (#421). Returns the merged catalog for both sections the UI shows:
-     * - flow-building tools, proxied from the agent service's /mcp/flow-tools endpoint
-     *   (friendly catalog entries only — raw MCP identifiers never leave the backend);
-     * - FlowFuse platform tools, curated here from the platform automation handler
-     *   (app.comms.platformAutomation) and tagged group:'platform'.
-     * A `hash` fingerprint of the flow-building catalog rides along so the browser refetches
-     * only when it changes. Team access + feature gating are enforced by the shared
-     * preHandler above; read/write classification on each entry is what the client uses to
-     * decide which tools a role may enable.
+     * Returns the merged tool catalog for the Expert permissions UI: flow-building tools
+     * proxied from the agent's /mcp/flow-tools endpoint, plus curated platform tools. A
+     * `hash` of the flow-building catalog rides along so the browser refetches only when
+     * it changes. Team access and feature gating are enforced by the shared preHandler.
      */
     app.get('/mcp/tools', {
         schema: {
@@ -656,11 +645,10 @@ module.exports = async function (app) {
 
             reply.send({ catalog, hash: response.data?.hash || null })
         } catch (error) {
-            // TODO: decide with the team whether this belongs on the branch. The tool catalog
-            // is a non-fatal enhancement (the client swallows failures and gates safely with
-            // defaults). Never forward an upstream auth failure as our own 401 — the SPA's
-            // axios interceptor treats any 401 as session-expiry and logs the user out, which
-            // an unrelated expert-service token rejection must not trigger.
+            // The tool catalog is a non-fatal enhancement (the client swallows failures and
+            // gates safely with defaults). Never forward an upstream auth failure as our own
+            // 401. The SPA's axios interceptor treats any 401 as session-expiry and logs the
+            // user out, which an unrelated expert-service token rejection must not trigger.
             const upstreamStatus = error.response?.status
             app.log.warn(`[expert/mcp/tools] upstream tool-catalog fetch failed: status=${upstreamStatus} msg=${error.message}`)
             if (upstreamStatus === 401 || upstreamStatus === 403) {

…ages/components/resources/JsonViewer.vue frontend/src/components/JsonViewer.vuefrontend/src/components/expert/components/messages/components/resources/JsonViewer.vue renamed to frontend/src/components/JsonViewer.vue frontend/src/components/expert/components/messages/components/resources/JsonViewer.vue renamed to frontend/src/components/JsonViewer.vue

@@ -40,22 +40,13 @@
 </template>
 
 <script>
-// Read-only single-value JSON viewer with prettify, word-wrap and optional collapse,
-// following the presentation of SnapshotDiffChangePanel without its two-sided diff
-// machinery. Used for payloads such as the tool approval card's call parameters.
+// Read-only single-value JSON viewer with prettify, word-wrap and optional collapse.
+// Generic component: renders any object, array or scalar as a payload block.
 import { ChevronRightIcon } from '@heroicons/vue/20/solid'
 
-const LONG_LINE_THRESHOLD = 50
+import { safeStringify } from '@/composables/strings/String.js'
 
-// Stringify that never throws — if the payload can't be serialised for any
-// reason, show a plain error instead of breaking the surrounding card.
-function safeStringify (value) {
-    try {
-        return JSON.stringify(value, null, 2)
-    } catch (err) {
-        return 'Could not display the payload.'
-    }
-}
+const LONG_LINE_THRESHOLD = 50
 
 export default {
     name: 'JsonViewer',

frontend/src/components/elements/ToggleButtonGroup.vue

@@ -62,8 +62,7 @@ export default {
             required: false,
             default: 'value'
         },
-        // Size of the underlying buttons (passed through to ff-button). Defaults to
-        // 'medium' to match existing usages; 'small' suits dense contexts like tables.
+        // Button size passed through to ff-button: 'small' or 'medium'.
         size: {
             type: String,
             required: false,

frontend/src/components/expert/components/messages/components/resources/ToolApprovalCard.vue

@@ -47,7 +47,7 @@
 </template>
 
 <script>
-import JsonViewer from './JsonViewer.vue'
+import JsonViewer from '@/components/JsonViewer.vue'
 
 export default {
     name: 'ToolApprovalCard',

frontend/src/composables/strings/String.js

@@ -70,6 +70,22 @@ export const hashString = (str) => {
     return (h >>> 0).toString(16).padStart(8, '0')
 }
 
+/**
+ * Stringify a value as prettified JSON, never throwing. If the value can't be
+ * serialised (e.g. a circular reference), returns a plain fallback message so the
+ * caller can display something instead of breaking.
+ * @param {*} value - The value to stringify
+ * @param {String} fallback - Message returned when serialisation fails
+ * @returns {String} - Prettified JSON, or the fallback on failure
+ */
+export const safeStringify = (value, fallback = 'Could not display the payload.') => {
+    try {
+        return JSON.stringify(value, null, 2)
+    } catch (err) {
+        return fallback
+    }
+}
+
 /**
  * Convert a string to slug format
  * @param {String} str - The string to convert to slug format

test/unit/frontend/composables/String.spec.js

@@ -0,0 +1,26 @@
+import { describe, expect, test } from 'vitest'
+
+import { safeStringify } from '../../../../frontend/src/composables/strings/String.js'
+
+describe('safeStringify', () => {
+    test('prettifies objects with two-space indentation', () => {
+        expect(safeStringify({ a: 1, b: [2, 3] })).toBe('{\n  "a": 1,\n  "b": [\n    2,\n    3\n  ]\n}')
+    })
+
+    test('stringifies scalars', () => {
+        expect(safeStringify(42)).toBe('42')
+        expect(safeStringify('hi')).toBe('"hi"')
+    })
+
+    test('returns the fallback when the value cannot be serialised', () => {
+        const circular = {}
+        circular.self = circular
+        expect(safeStringify(circular)).toBe('Could not display the payload.')
+    })
+
+    test('accepts a custom fallback message', () => {
+        const circular = {}
+        circular.self = circular
+        expect(safeStringify(circular, 'nope')).toBe('nope')
+    })
+})