From 9abc162394c5f8eea6bdc48b7c4ee633eca87093 Mon Sep 17 00:00:00 2001 From: archipelago Date: Fri, 7 Aug 2026 05:00:54 -0400 Subject: [PATCH] fix(aiui): the content surface renders what the assistant found MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four defects, one visible symptom: a correct prose answer beside an empty grid. 1. The assistant's curated RPC bridge had an arm only for `content.list-mine`. `tools.rs` mapped the `peers`, `purchased` and `films` scopes onto three real, dispatcher-registered handlers that `assistant_dispatch_tool` had never heard of, so every non-"own" scope died on its catch-all. Downstream that read as "the peers have no content" — it was a missing match arm, and the tool never ran. Regression test added: every scope the schema advertises must reach a real handler. 2. `content.browse-all-peers` wrapped its whole fan-out in one `timeout(..).unwrap_or_default()`, which DISCARDED every completed batch the moment the budget expired. One slow peer turned a partly-successful browse into "0 reached, 16 unreachable". Observed live on archi-dev-box: back-to-back calls returned real peer items, then nothing. Now accumulates per batch and checks a deadline between them, so partial results always survive. Budget 20s -> 45s: two batches of eight at a 10s per-peer timeout had no headroom at all. 3. `assistant.chat` returned only `{ text }`. The structured results of any content tool the turn ran were dropped inside the loop, so the surface had nothing to render. The turn now carries them through (captured raw, before the untrusted wrap, since they go to a renderer that treats every field as inert data, never back into the prompt). 4. The adapter classified images as 'excluded' and dropped them. A node sharing mostly photos rendered as an empty grid while AIUI's image grid sat unused. Images now have a bucket, with the paid-lock and extension-fallback handling audio and video already had. Also: the panel says "Loading…" while a turn is in flight and "Nothing found" when it comes back empty, instead of leaving the previous query's heading standing as though it answered this one; the system prompt tells the model to call the content tool and summarise rather than re-list what the cards already show; and a refused tool now names its permission category so the trusted chrome can offer the settings screen instead of leaving "I don't have a tool for that" as the only clue. Co-Authored-By: Claude Opus 5 (1M context) --- aiui/packages/app/src/composables/useAI.ts | 29 ++++- aiui/packages/app/src/composables/useArchy.ts | 18 ++- .../app/src/composables/useContentPanel.ts | 42 +++++- aiui/packages/app/src/services/archyBridge.ts | 30 ++++- .../archipelago/src/api/rpc/assistant_chat.rs | 21 ++- core/archipelago/src/api/rpc/content.rs | 61 +++++---- core/archipelago/src/assistant/loop_.rs | 21 ++- core/archipelago/src/assistant/mod.rs | 120 +++++++++++++++++- core/archipelago/src/assistant/tools.rs | 59 +++++++++ .../__tests__/archyContentAdapter.test.ts | 33 ++++- .../src/composables/archyContentAdapter.ts | 63 ++++++++- neode-ui/src/locales/en.json | 4 +- neode-ui/src/locales/es.json | 2 + .../services/__tests__/contextBroker.test.ts | 2 + .../services/__tests__/toolConfirm.test.ts | 9 +- neode-ui/src/services/contextBroker.ts | 67 +++++++++- neode-ui/src/types/aiui-protocol.ts | 15 +++ neode-ui/src/views/Chat.vue | 110 ++++++++++++++++ 18 files changed, 650 insertions(+), 56 deletions(-) diff --git a/aiui/packages/app/src/composables/useAI.ts b/aiui/packages/app/src/composables/useAI.ts index f3f698a0..99021375 100644 --- a/aiui/packages/app/src/composables/useAI.ts +++ b/aiui/packages/app/src/composables/useAI.ts @@ -6,7 +6,9 @@ import type { ImageAttachment } from '@aiui/core/types/message' import { usePersonaStore } from '@/stores/personas' import { useMemoryStore } from '@/stores/memory' import { useArchy } from '@/composables/useArchy' -import { archyBridge } from '@/services/archyBridge' +import { archyBridge, type ChatSurface } from '@/services/archyBridge' +import { useContentPanel } from '@/composables/useContentPanel' +import type { Film, Song, Podcast, ImageItem } from '@aiui/core/types/content' import { useCodeContext } from '@/composables/useCodeContext' import { apiFetch } from '@/utils/api-fetch' import { useSettingsStore } from '@/stores/settings' @@ -392,16 +394,41 @@ async function streamViaArchy( ): Promise { const lastUser = [...messages].reverse().find((m) => m.role === 'user') const text = lastUser?.content ?? '' + const { beginArchyContentLoad, setArchyContent } = useContentPanel() + beginArchyContentLoad() try { const result = await archyBridge.sendChat(text) if (signal?.aborted) return + // The turn's tool results, rendered — not just described. A node that + // listed twelve shared files used to produce a correct paragraph next + // to an empty grid, because this was the point the structured results + // were dropped. + setArchyContent(mergeChatSurfaces(result.surfaces)) onToken(result.text) } catch (err) { if (signal?.aborted) return + // Clear the 'Loading…' heading — an errored turn must not leave the + // panel claiming it is still working. + setArchyContent({}) onError(err instanceof Error ? err.message : 'Archy chat request failed') } } +/** + * Flatten a turn's surfaces into the one bundle the panel renders. A + * single turn can legitimately run `content_list` more than once (own + * files AND peer films, say); concatenating rather than letting the last + * call win is what keeps both visible. + */ +function mergeChatSurfaces(surfaces: ChatSurface[] = []) { + return { + films: surfaces.flatMap((s) => (s.bundle?.films ?? []) as Film[]), + songs: surfaces.flatMap((s) => (s.bundle?.songs ?? []) as Song[]), + podcasts: surfaces.flatMap((s) => (s.bundle?.podcasts ?? []) as Podcast[]), + images: surfaces.flatMap((s) => (s.bundle?.images ?? []) as ImageItem[]), + } +} + async function readSSE( res: Response, onData: (data: string) => void, diff --git a/aiui/packages/app/src/composables/useArchy.ts b/aiui/packages/app/src/composables/useArchy.ts index 83ec51a7..62979b57 100644 --- a/aiui/packages/app/src/composables/useArchy.ts +++ b/aiui/packages/app/src/composables/useArchy.ts @@ -2,7 +2,7 @@ import { ref, readonly } from 'vue' import { archyBridge } from '@/services/archyBridge' import { useTheme } from '@/composables/useTheme' import { useContentPanel } from '@/composables/useContentPanel' -import type { Film, Song, Podcast } from '@aiui/core/types/content' +import type { Film, Song, Podcast, ImageItem } from '@aiui/core/types/content' import { mockArchyApps, mockArchySystem, mockArchyNetwork, mockArchyWallet, mockArchyBitcoin, mockArchyFiles, @@ -244,6 +244,7 @@ export function useArchy() { films: res.films as Film[], songs: panel.panelSongs.value, podcasts: res.podcasts as Podcast[], + images: res.images as ImageItem[], }) } catch (err) { console.warn('[AIUI Archy] content fetch failed:', (err as Error)?.message ?? err) @@ -283,9 +284,10 @@ export function useArchy() { // they arrive. A scope that times out costs only its own results. const films: Film[] = [] const podcasts: Podcast[] = [] + const images: ImageItem[] = [] const seen = new Set() - const merge = (res: { films?: unknown; podcasts?: unknown } | null) => { + const merge = (res: { films?: unknown; podcasts?: unknown; images?: unknown } | null) => { if (!res) return false let added = false for (const f of (res.films ?? []) as Film[]) { @@ -298,12 +300,22 @@ export function useArchy() { if (seen.has(k)) continue seen.add(k); podcasts.push(p); added = true } + for (const im of (res.images ?? []) as ImageItem[]) { + const k = `img:${im.id}` + if (seen.has(k)) continue + seen.add(k); images.push(im); added = true + } return added } const paint = () => { const panel = useContentPanel() - panel.setArchyContent({ films: [...films], songs: panel.panelSongs.value, podcasts: [...podcasts] }) + panel.setArchyContent({ + films: [...films], + songs: panel.panelSongs.value, + podcasts: [...podcasts], + images: [...images], + }) } const fetchScope = (scope: 'own' | 'owned' | 'peers') => diff --git a/aiui/packages/app/src/composables/useContentPanel.ts b/aiui/packages/app/src/composables/useContentPanel.ts index 824efe2d..26198410 100644 --- a/aiui/packages/app/src/composables/useContentPanel.ts +++ b/aiui/packages/app/src/composables/useContentPanel.ts @@ -73,6 +73,12 @@ const mapPlaces = ref([]) * Archy source in this plan's scope and keeps using the regex path * unconditionally (13-PATTERNS.md: partial deprecation, not a removal). */ const archyContentActive = ref(false) +/** A chat turn that may produce content is in flight. Until it resolves, + * the panel heading must NOT keep advertising the previous query's + * results — the operator reads that as the answer to what they just + * asked. `beginArchyContentLoad` raises this and `setArchyContent` + * lowers it. */ +const archyContentLoading = ref(false) export interface DesignSystemItem { id: string @@ -195,17 +201,20 @@ export function useContentPanel() { const visibleApps = showApps ? apps : [] const visibleCodeBlocks = showCode ? codeBlocks : [] - // Archy-sourced films/songs/podcasts are the source of truth once a - // node has supplied them (D-12) — don't let this turn's regex - // extraction of the model's own reply text overwrite them. + // Archy-sourced films/songs/podcasts/images are the source of truth + // once a node has supplied them (D-12) — don't let this turn's regex + // extraction of the model's own reply text overwrite them. Images + // joined this set when the adapter started carrying shared photos; + // leaving them out here would have let the regex path immediately + // wipe the grid the node had just filled. if (!archyContentActive.value) { panelFilms.value = visibleFilms panelSongs.value = visibleSongs panelPodcasts.value = visiblePodcasts + panelImages.value = visibleImages } panelBooks.value = visibleBooks panelTVSeries.value = visibleTVSeries - panelImages.value = visibleImages panelPlaces.value = visiblePlaces panelWebResults.value = visibleNews panelWebsites.value = visibleWebsites @@ -292,10 +301,23 @@ export function useContentPanel() { * `coverUrl`; `FilmGrid`/`SongGrid` already render their existing * no-artwork fallback for that case (unchanged by this plan, D-12). */ - function setArchyContent(bundle: { films?: Film[]; songs?: Song[]; podcasts?: Podcast[] }) { + /** A content-capable chat turn just started. Clears the stale heading + * so the panel says what it is doing rather than what it last found. */ + function beginArchyContentLoad() { + archyContentLoading.value = true + panelTitle.value = 'Loading…' + } + + function setArchyContent(bundle: { + films?: Film[] + songs?: Song[] + podcasts?: Podcast[] + images?: ImageItem[] + }) { panelFilms.value = bundle.films ?? [] panelSongs.value = bundle.songs ?? [] panelPodcasts.value = bundle.podcasts ?? [] + panelImages.value = bundle.images ?? [] archyContentActive.value = true // 13-11 (GAP-FOUND 2026-08-03): `availableTabs`/`activeTab`/`panelOpen` @@ -311,6 +333,7 @@ export function useContentPanel() { if (panelFilms.value.length > 0) archyTabs.push('film') if (panelSongs.value.length > 0) archyTabs.push('song') if (panelPodcasts.value.length > 0) archyTabs.push('podcast') + if (panelImages.value.length > 0) archyTabs.push('image') if (archyTabs.length > 0) { availableTabs.value = [...archyTabs, 'prompt'] if (!archyTabs.includes(activeTab.value)) activeTab.value = archyTabs[0]! @@ -320,8 +343,15 @@ export function useContentPanel() { else if (panelSongs.value.length > 1) panelTitle.value = `${panelSongs.value.length} Songs` else if (panelPodcasts.value.length === 1) panelTitle.value = panelPodcasts.value[0]!.title else if (panelPodcasts.value.length > 1) panelTitle.value = `${panelPodcasts.value.length} Podcasts` + else if (panelImages.value.length > 0) panelTitle.value = `${panelImages.value.length} Images` panelOpen.value = true + } else if (archyContentLoading.value) { + // The turn asked the node for content and got none back. Leaving the + // previous query's heading up would claim those results answer THIS + // question; say plainly that there was nothing. + panelTitle.value = 'Nothing found' } + archyContentLoading.value = false } function setActiveTab(tab: ContentTab) { @@ -521,6 +551,8 @@ export function useContentPanel() { panelSongs, panelPodcasts, archyContentActive, + archyContentLoading, + beginArchyContentLoad, setArchyContent, panelWebResults, panelWebsites, diff --git a/aiui/packages/app/src/services/archyBridge.ts b/aiui/packages/app/src/services/archyBridge.ts index 0aa51a91..c4fb3f5f 100644 --- a/aiui/packages/app/src/services/archyBridge.ts +++ b/aiui/packages/app/src/services/archyBridge.ts @@ -23,6 +23,7 @@ interface ContentResponse { films: unknown[] songs: unknown[] podcasts: unknown[] + images: unknown[] permitted: boolean } @@ -31,6 +32,21 @@ export interface ActionResponse { error?: string } +/** One content-tool result from a chat turn, carrying the same + * `films`/`songs`/`podcasts` buckets `content:push` delivers — see + * neode-ui's `ArchyChatSurface`. `scope` names what was asked for + * (`own`/`peers`/`purchased`/`films`) so the surface can title itself. */ +export interface ChatSurface { + tool: string + scope?: string + bundle: { + films: unknown[] + songs: unknown[] + podcasts: unknown[] + images: unknown[] + } +} + interface ThemeInfo { accent: string mode: 'dark' @@ -93,7 +109,10 @@ function handleMessage(event: MessageEvent) { if (pending) { pendingRequests.delete(msg.id) if (msg.success) { - pending.resolve({ text: msg.text ?? '' }) + pending.resolve({ + text: msg.text ?? '', + surfaces: Array.isArray(msg.surfaces) ? msg.surfaces : [], + }) } else { pending.reject(new Error(msg.error || 'Chat request failed')) } @@ -109,6 +128,7 @@ function handleMessage(event: MessageEvent) { films: Array.isArray(msg.films) ? msg.films : [], songs: Array.isArray(msg.songs) ? msg.songs : [], podcasts: Array.isArray(msg.podcasts) ? msg.podcasts : [], + images: Array.isArray(msg.images) ? msg.images : [], permitted: msg.permitted !== false, }) } @@ -266,9 +286,13 @@ export const archyBridge = { /** * Send a chat turn to Archy's node-side assistant loop (D-01: the model * key and the tool-calling loop live on the node, never in this bundle). - * Returns the assistant's final text answer for this turn. + * Resolves with the turn's final text AND any content the tools it ran + * produced, already adapted to grid records by the host broker — this + * is what lets an answer be rendered as a surface instead of only + * described in prose. `surfaces` is `[]` for a turn that ran no content + * tool, so callers never branch on undefined. */ - sendChat(text: string): Promise<{ text: string }> { + sendChat(text: string): Promise<{ text: string; surfaces: ChatSurface[] }> { const id = generateId() return new Promise((resolve, reject) => { pendingRequests.set(id, { resolve: resolve as (v: unknown) => void, reject }) diff --git a/core/archipelago/src/api/rpc/assistant_chat.rs b/core/archipelago/src/api/rpc/assistant_chat.rs index f958a18a..898ea66a 100644 --- a/core/archipelago/src/api/rpc/assistant_chat.rs +++ b/core/archipelago/src/api/rpc/assistant_chat.rs @@ -279,8 +279,17 @@ impl RpcHandler { let caller = crate::assistant::CallerScope::LocalOperator { session_id }; - let answer = crate::assistant::chat(Arc::clone(self), caller, text).await?; - Ok(serde_json::json!({ "text": answer })) + let outcome = crate::assistant::chat_with_surfaces(Arc::clone(self), caller, text).await?; + // `surfaces` carries the structured results of any content tools + // this turn ran, so the browser can render them as a grid instead + // of leaving the surface empty beside a correct prose answer. + // Always present (possibly empty) so the caller never has to + // distinguish "no content" from "old node". + Ok(serde_json::json!({ + "text": outcome.text, + "surfaces": outcome.surfaces, + "refused_categories": outcome.refused_categories, + })) } /// Internal-only bridge: executes a curated assistant tool against the @@ -310,6 +319,14 @@ impl RpcHandler { "network.set-wifi-radio" => self.handle_network_set_wifi_radio(params).await, "mesh.status" => self.handle_mesh_status().await, "content.list-mine" => self.handle_content_list_mine().await, + // The other three `content_list` scopes (peers/purchased/films). + // Their absence here — while `tools.rs`'s scope match named them + // — made every non-"own" scope fail with "no such handler", which + // reads downstream as "the peers have no content" when in truth + // the tool never ran. Curated one-by-one, same as every arm above. + "content.browse-all-peers" => self.handle_content_browse_all_peers().await, + "content.owned-list" => self.handle_content_owned_list().await, + "content.indeehub-projects" => self.handle_content_indeehub_projects().await, "system.settings.get" => self.handle_system_settings_get(params).await, "system.settings.set" => self.handle_system_settings_set(params).await, "system.kiosk-display.get" => self.handle_system_kiosk_display_get().await, diff --git a/core/archipelago/src/api/rpc/content.rs b/core/archipelago/src/api/rpc/content.rs index 2daf9a70..eb36b875 100644 --- a/core/archipelago/src/api/rpc/content.rs +++ b/core/archipelago/src/api/rpc/content.rs @@ -1275,35 +1275,50 @@ impl RpcHandler { // inside the budget, which is the point. const BROWSE_PEER_CONCURRENCY: usize = 8; const PER_PEER_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(10); - let overall = std::time::Duration::from_secs(20); + // Headroom matters: 16 peers at concurrency 8 is two batches, and a + // batch only finishes when its SLOWEST peer does. At a 20s budget + // one slow peer in batch 1 left batch 2 no time at all. + let overall = std::time::Duration::from_secs(45); + let deadline = tokio::time::Instant::now() + overall; let mut items = Vec::new(); let mut reached = 0usize; let mut unreachable = 0usize; - let results = tokio::time::timeout(overall, async { - let mut out: Vec<(String, Option)> = Vec::new(); - for chunk in onions.chunks(BROWSE_PEER_CONCURRENCY) { - let mut set = Vec::new(); - for onion in chunk { - let params = Some(serde_json::json!({ "onion": onion })); - set.push(async move { - let v = tokio::time::timeout( - PER_PEER_TIMEOUT, - self.handle_content_browse_peer(params), - ) - .await - .ok() - .and_then(|r| r.ok()); - (onion.clone(), v) - }); - } - out.extend(futures_util::future::join_all(set).await); + // Accumulate per batch rather than wrapping the whole loop in one + // `timeout(..).unwrap_or_default()`. That construction DISCARDED + // every completed batch the moment the budget expired, so a single + // slow peer turned a partly-successful fan-out into "0 reached, 16 + // unreachable" — indistinguishable, downstream, from the peers + // having no content at all. Observed live on archi-dev-box: back to + // back calls returned real peer items and then nothing. + let mut results: Vec<(String, Option)> = Vec::new(); + for chunk in onions.chunks(BROWSE_PEER_CONCURRENCY) { + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + if remaining.is_zero() { + break; } - out - }) - .await - .unwrap_or_default(); + let mut set = Vec::new(); + for onion in chunk { + let params = Some(serde_json::json!({ "onion": onion })); + set.push(async move { + let v = tokio::time::timeout( + PER_PEER_TIMEOUT, + self.handle_content_browse_peer(params), + ) + .await + .ok() + .and_then(|r| r.ok()); + (onion.clone(), v) + }); + } + // No batch-level timeout: every future in `set` is ALREADY + // bounded by PER_PEER_TIMEOUT, so this join can't outrun it, and + // adding an outer timeout here would reintroduce exactly the + // discard-on-expiry bug above. The deadline check at the top of + // the loop is what stops a long peer list from running forever. + results.extend(futures_util::future::join_all(set).await); + } for (onion, v) in &results { match v { diff --git a/core/archipelago/src/assistant/loop_.rs b/core/archipelago/src/assistant/loop_.rs index b7acf65d..e8e60d2d 100644 --- a/core/archipelago/src/assistant/loop_.rs +++ b/core/archipelago/src/assistant/loop_.rs @@ -212,6 +212,11 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu let granted = ctx.caller.granted_categories(ctx.handler.data_dir()).await; if !granted.contains(&tool.category) { + // Remember WHICH category blocked this, so the trusted chrome can + // offer the operator a link to the toggle. Without it the only + // trace is the model's prose, and "I don't have a tool for that" + // gives no hint that the capability exists and is one switch away. + ctx.note_refused_category(tool.category); return ToolResult { call_id: call.id.clone(), is_error: true, @@ -292,7 +297,18 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu // `tools::dispatch` — never a generic pass-through of the model's tool // name onto the RPC surface. match super::tools::dispatch(&call.name, &args, ctx.handler.as_ref()).await { - Ok(v) => ToolResult { + Ok(v) => { + // Capture grid-ready results for the UI *here*, on the raw + // value, before the untrusted wrap below turns it into + // delimiter-fenced text. See `ToolExecCtx::surfaces`. + if super::tools::is_surface_tool(&call.name) { + ctx.note_surface( + &call.name, + super::tools::surface_scope(&args), + v.clone(), + ); + } + ToolResult { call_id: call.id.clone(), is_error: false, // D-10: peer-authored content (filenames, log lines, mesh/peer @@ -301,7 +317,8 @@ pub(crate) async fn execute_tool(call: &ToolCall, ctx: &ToolExecCtx) -> ToolResu // ToolResult is constructed. Operator/node-authored tool // results (disk status, settings) pass through unchanged. content: super::tools::wrap_tool_result_if_untrusted(&call.name, v.to_string()), - }, + } + } Err(msg) => ToolResult { call_id: call.id.clone(), is_error: true, diff --git a/core/archipelago/src/assistant/mod.rs b/core/archipelago/src/assistant/mod.rs index 96d3146c..7b32dadc 100644 --- a/core/archipelago/src/assistant/mod.rs +++ b/core/archipelago/src/assistant/mod.rs @@ -576,6 +576,41 @@ pub struct ToolExecCtx { /// mint a fresh confirmation and re-open the dialog until the human /// gives in, which is the habituation failure, mechanized. declined_actions: Mutex>, + /// Grid-ready results captured from the content-producing tools this + /// turn ran, in call order. This is what lets a chat answer *show* the + /// films/photos/files it just looked up instead of only describing + /// them: without it, `chat` returns prose and the tool's structured + /// result is discarded inside the loop, so AIUI's content surface has + /// nothing to render and stays empty next to a perfectly correct + /// paragraph. + /// + /// Deliberately the RAW dispatch value, captured BEFORE + /// `wrap_tool_result_if_untrusted`: the untrusted boundary exists to + /// stop peer-authored text being read as instructions by the MODEL. + /// This copy never re-enters the prompt — it goes to a renderer that + /// treats every field as inert data — and wrapping it would leave the + /// UI parsing delimiter noise instead of JSON. + surfaces: Mutex>, + /// Categories a tool call was refused for this turn, in first-refusal + /// order. The model can only report this as prose ("I don't have a + /// tool for that"), which leaves the operator with no idea that the + /// fix is one toggle away — and reads as the assistant being broken. + /// Surfacing the category lets the TRUSTED chrome offer a real link + /// to the setting; the grant itself is never changed from here. + refused_categories: Mutex>, +} + +/// One content-producing tool result, kept for the UI to render. +#[derive(Debug, Clone, serde::Serialize)] +pub struct Surface { + /// The tool that produced it (e.g. `content_list`). + pub tool: String, + /// The scope argument it was called with, when it had one — this is + /// what tells "my own shared content" apart from "films from peers" + /// on the receiving side, so the surface can title itself honestly. + pub scope: Option, + /// The dispatch result, exactly as the RPC handler returned it. + pub data: serde_json::Value, } impl ToolExecCtx { @@ -616,9 +651,52 @@ impl ToolExecCtx { counters, validation_failures: Mutex::new(HashMap::new()), declined_actions: Mutex::new(HashSet::new()), + surfaces: Mutex::new(Vec::new()), + refused_categories: Mutex::new(Vec::new()), } } + /// A tool was refused because `category` is not granted. Deduped so a + /// model retrying the same blocked tool three times still produces one + /// prompt for the operator, not three. + pub(crate) fn note_refused_category(&self, category: PermissionCategory) { + let mut refused = self + .refused_categories + .lock() + .expect("refused_categories mutex poisoned"); + if !refused.contains(&category) { + refused.push(category); + } + } + + /// Categories refused this turn, in first-refusal order. + pub(crate) fn refused_categories(&self) -> Vec { + self.refused_categories + .lock() + .expect("refused_categories mutex poisoned") + .clone() + } + + /// Record a content-producing tool's raw result for the UI to render. + pub(crate) fn note_surface(&self, tool: &str, scope: Option, data: serde_json::Value) { + self.surfaces + .lock() + .expect("surfaces mutex poisoned") + .push(Surface { + tool: tool.to_string(), + scope, + data, + }); + } + + /// Everything captured this turn, in call order. + pub(crate) fn surfaces(&self) -> Vec { + self.surfaces + .lock() + .expect("surfaces mutex poisoned") + .clone() + } + /// The human declined this exact action; remember it for the rest of /// the turn so it can never re-prompt. pub(crate) fn note_declined(&self, action_key: String) { @@ -696,7 +774,15 @@ owner's general assistant: answer ordinary questions, explain things, and give r assistant would. Those answers are what the content surfaces in this app render as cards, so \ declining to answer them leaves the owner staring at an empty panel. Only a request to CHANGE \ or READ something on the node needs a tool — and if no tool covers it, say so plainly rather \ -than pretending. Never let a general question be refused merely because no tool matches it."; +than pretending. Never let a general question be refused merely because no tool matches it.\n\n\ +When the question is about what actually EXISTS here — this node's own shared files, a peer's \ +catalogue, purchased items, the film catalogue — call the content tool rather than answering \ +from memory or from earlier in this conversation, and call it once per place the question spans \ +(asking about \"films\" covers both the catalogue and the peers). The app renders whatever that \ +tool returns as cards beside your reply, so your text should introduce and summarise the result \ +— how many, anything notable — rather than re-listing every title, size and price in prose. A \ +long enumeration duplicates the cards the operator is already looking at. If the tool comes \ +back empty, say so plainly; do not fill the gap with remembered or invented items."; pub fn build_system_prompt(visible_tools: &[tools::ToolDef]) -> String { let mut prompt = String::from(SYSTEM_PROMPT_PREAMBLE); @@ -715,6 +801,28 @@ pub fn build_system_prompt(visible_tools: &[tools::ToolDef]) -> String { prompt } +/// A completed chat turn: the model's prose answer, plus whatever +/// content-producing tools it ran along the way. Callers that only want +/// words (the mesh `!archy` path) use [`chat`]; the browser uses +/// [`chat_with_surfaces`] so it can render the results as a grid. +pub struct ChatOutcome { + pub text: String, + pub surfaces: Vec, + /// Permission categories that blocked a tool this turn — what the + /// chrome turns into an "enable it in Settings" offer. + pub refused_categories: Vec, +} + +/// Text-only wrapper over [`chat_with_surfaces`], for callers with no +/// surface to render (mesh replies, tests). +pub async fn chat( + handler: Arc, + caller: CallerScope, + user_text: String, +) -> Result { + Ok(chat_with_surfaces(handler, caller, user_text).await?.text) +} + /// Entry point: run one chat turn for `caller` through the shared loop. /// Builds the visible-tool set from the caller's granted categories only /// (D-16 — the model should never even see a tool it can't use), selects a @@ -732,11 +840,11 @@ pub fn build_system_prompt(visible_tools: &[tools::ToolDef]) -> String { /// way that stays correct against Claude's strict `tool_use`/`tool_result` /// id-pairing requirement needs its own test budget, and none of Task 2's /// `` bullets require it this plan. -pub async fn chat( +pub async fn chat_with_surfaces( handler: Arc, caller: CallerScope, user_text: String, -) -> Result { +) -> Result { let registry = tools::registry(); let grants = caller.granted_categories(handler.data_dir()).await; let visible_tools = registry.visible_to(&grants); @@ -803,7 +911,11 @@ pub async fn chat( ); } - Ok(answer) + Ok(ChatOutcome { + text: answer, + surfaces: ctx.surfaces(), + refused_categories: ctx.refused_categories(), + }) } #[cfg(test)] diff --git a/core/archipelago/src/assistant/tools.rs b/core/archipelago/src/assistant/tools.rs index 1c73aba2..f9f5cfe8 100644 --- a/core/archipelago/src/assistant/tools.rs +++ b/core/archipelago/src/assistant/tools.rs @@ -45,6 +45,30 @@ pub fn wrap_tool_result_if_untrusted(name: &str, content: String) -> String { } } +/// Tools whose results are grid-ready content the UI should RENDER, not +/// merely summarise in prose. Kept separate from +/// [`UNTRUSTED_CONTENT_TOOLS`] on purpose even though they overlap today: +/// that list answers "can this text manipulate the model?", this one +/// answers "does this result have a visual form?" — and the answers +/// diverge (`app_logs` is untrusted but has no grid; `apps_list` has a +/// grid but is node-authored). +const SURFACE_TOOLS: &[&str] = &["content_list", "apps_list"]; + +/// Whether this tool's result should be captured for the content surface. +pub fn is_surface_tool(name: &str) -> bool { + SURFACE_TOOLS.contains(&name) +} + +/// The scope a captured surface was produced under, when its tool has +/// one. Lets the receiving grid title itself with what was actually +/// asked for rather than guessing from the payload's shape. +pub fn surface_scope(args: &ToolArgs) -> Option { + match args { + ToolArgs::ContentList(a) => Some(a.scope.clone().unwrap_or_else(|| "own".to_string())), + _ => None, + } +} + /// The backend-agnostic in/out of a tool invocation — the same shape /// regardless of which adapter (Ollama/Claude/Routstr) produced it. #[derive(Debug, Clone)] @@ -1095,6 +1119,41 @@ mod tests { ); } + /// Every scope the `content_list` schema advertises must resolve to an + /// RPC method that `assistant_dispatch_tool` actually has an arm for. + /// + /// This is the test that was missing. `peers`/`purchased`/`films` each + /// named a real, dispatcher-registered handler, but the assistant's + /// curated bridge had an arm only for `own` — so those three died on + /// the bridge's catch-all. The model then reported, accurately from + /// where it stood, that it could find no peer content, and that read + /// like a fleet outage instead of a missing match arm. Validating the + /// scope (above) is not enough: the whole failure lived downstream of + /// validation. + #[tokio::test] + async fn every_content_scope_reaches_a_real_dispatch_handler() { + let (handler, _tmp) = test_rpc_handler().await; + grant_all(&handler).await; + let ctx = local_operator_ctx(handler); + + for scope in ["own", "peers", "purchased", "films"] { + let call = ToolCall { + id: format!("call-{scope}"), + name: "content_list".to_string(), + arguments: json!({ "scope": scope }), + }; + let result = execute_tool(&call, &ctx).await; + // A bare handler with no orchestrator may legitimately return an + // empty catalogue or an upstream error; what it must NEVER do is + // report that the method itself is unreachable. + assert!( + !result.content.contains("no such handler"), + "scope {scope} has no assistant_dispatch_tool arm — it never ran: {}", + result.content + ); + } + } + #[tokio::test] async fn app_restart_refuses_unknown_app_id() { let (handler, _tmp) = test_rpc_handler().await; diff --git a/neode-ui/src/composables/__tests__/archyContentAdapter.test.ts b/neode-ui/src/composables/__tests__/archyContentAdapter.test.ts index 4e73b964..8dc28291 100644 --- a/neode-ui/src/composables/__tests__/archyContentAdapter.test.ts +++ b/neode-ui/src/composables/__tests__/archyContentAdapter.test.ts @@ -30,11 +30,18 @@ describe('classifyByMime', () => { expect(classifyByMime(item({ mime_type: 'audio/mpeg', filename: 'song.mp3' }))).toBe('audio') }) - it('excludes image and document mimes rather than mis-typing them', () => { - expect(classifyByMime(item({ mime_type: 'image/jpeg', filename: 'photo.jpg' }))).toBe('excluded') + it('classifies images as image, and still excludes documents', () => { + expect(classifyByMime(item({ mime_type: 'image/jpeg', filename: 'photo.jpg' }))).toBe('image') expect(classifyByMime(item({ mime_type: 'application/pdf', filename: 'doc.pdf' }))).toBe('excluded') }) + it('classifies images by extension when the mime is generic', () => { + // A node whose catalog is mostly photos shared with an unidentified + // mime must not present as empty. + expect(classifyByMime(item({ mime_type: 'application/octet-stream', filename: 'p.webp' }))).toBe('image') + expect(classifyByMime(item({ mime_type: 'application/octet-stream', filename: 'p.heic' }))).toBe('image') + }) + it('classifies m4a, aac, opus and wma as audio via extension fallback (ShareModal blind spot)', () => { expect(classifyByMime(item({ mime_type: 'application/octet-stream', filename: 'track.m4a' }))).toBe('audio') expect(classifyByMime(item({ mime_type: 'application/octet-stream', filename: 'track.aac' }))).toBe('audio') @@ -74,7 +81,7 @@ describe('adaptContentItems', () => { expect(bundle.films).toHaveLength(0) }) - it('excludes image and document mimes from all three buckets', () => { + it('routes images to the images bucket and still excludes documents', () => { const bundle = adaptContentItems( [ item({ id: 'img-1', filename: 'photo.jpg', mime_type: 'image/jpeg' }), @@ -85,6 +92,20 @@ describe('adaptContentItems', () => { expect(bundle.films).toHaveLength(0) expect(bundle.songs).toHaveLength(0) expect(bundle.podcasts).toHaveLength(0) + // The photo renders; the PDF has no grid, so it stays out of every bucket. + expect(bundle.images).toHaveLength(1) + expect(bundle.images[0]!.id).toBe('img-1') + expect(bundle.images[0]!.url).toBe('/content/img-1') + }) + + it('locks a paid image: price carried, no URL to fetch bytes the user has not bought', () => { + const bundle = adaptContentItems( + [item({ id: 'p-1', filename: 'photo.jpg', mime_type: 'image/jpeg', access: { paid: { price_sats: 100 } } })], + { source: 'own' }, + ) + expect(bundle.images[0]!.locked).toBe(true) + expect(bundle.images[0]!.priceSats).toBe(100) + expect(bundle.images[0]!.url).toBe('') }) it('maps an access:Paid item with a price and a locked flag, and no playable source URL', () => { @@ -139,12 +160,12 @@ describe('adaptContentItems', () => { it('an empty input array produces empty films/songs/podcasts arrays, not undefined or an error', () => { const bundle = adaptContentItems([], { source: 'own' }) - expect(bundle).toEqual({ films: [], songs: [], podcasts: [] }) + expect(bundle).toEqual({ films: [], songs: [], podcasts: [], images: [] }) }) it('handles null/undefined input the same as an empty array', () => { - expect(adaptContentItems(null, { source: 'own' })).toEqual({ films: [], songs: [], podcasts: [] }) - expect(adaptContentItems(undefined, { source: 'own' })).toEqual({ films: [], songs: [], podcasts: [] }) + expect(adaptContentItems(null, { source: 'own' })).toEqual({ films: [], songs: [], podcasts: [], images: [] }) + expect(adaptContentItems(undefined, { source: 'own' })).toEqual({ films: [], songs: [], podcasts: [], images: [] }) }) it('maps a null/absent description to an empty string, never the literal "null"', () => { diff --git a/neode-ui/src/composables/archyContentAdapter.ts b/neode-ui/src/composables/archyContentAdapter.ts index 1367d81d..6cbf956e 100644 --- a/neode-ui/src/composables/archyContentAdapter.ts +++ b/neode-ui/src/composables/archyContentAdapter.ts @@ -106,6 +106,23 @@ export interface Podcast { priceSats?: number } +/** Structurally matches `aiui/packages/core/src/types/content.ts`'s + * `ImageItem` — declared locally for the same D-19 reason as the shapes + * above (neode-ui does not depend on `@aiui/core`). */ +export interface ImageItem { + id: string + url: string + title?: string + description?: string + alt?: string + width?: number + height?: number + source?: string + attribution?: string + locked?: boolean + priceSats?: number +} + // ─── Source shape: content_server.rs's ContentItem, as seen over RPC ─── /** `AccessControl` (`core/archipelago/src/content_server.rs`) serializes via @@ -138,6 +155,12 @@ export interface ArchyContentBundle { films: Film[] songs: Song[] podcasts: Podcast[] + /** Shared photos. Images were previously classified 'excluded' and + * dropped on the floor, so a node sharing mostly photos rendered as an + * empty grid — the single biggest gap between what the assistant could + * DESCRIBE and what the surface could SHOW. AIUI has had an image grid + * (`panelImages`/`ImageGrid`) the whole time; nothing fed it. */ + images: ImageItem[] } export interface AdaptContentOptions { @@ -153,7 +176,7 @@ export interface AdaptContentOptions { // ─── Classification ─────────────────────────────────────────────────────── -type ContentBucket = 'video' | 'audio' | 'excluded' +type ContentBucket = 'video' | 'audio' | 'image' | 'excluded' // `m4a`, `aac`, `opus` and `wma` classify as audio via this extension // fallback — `ShareModal.vue`'s mime map omits exactly these four today, so @@ -175,6 +198,11 @@ const AUDIO_EXT_FALLBACK = new Set([ const VIDEO_EXT_FALLBACK = new Set(['mp4', 'mkv', 'avi', 'mov', 'webm', 'm4v']) +// Same reasoning as the audio fallback above: a photo shared with a mime +// this node could not identify still has an unambiguous extension, and a +// node whose catalog is mostly photos should not present as empty. +const IMAGE_EXT_FALLBACK = new Set(['jpg', 'jpeg', 'png', 'gif', 'webp', 'avif', 'heic', 'bmp']) + function extensionOf(filename: string): string { const base = filename.includes('/') ? filename.slice(filename.lastIndexOf('/') + 1) : filename const idx = base.lastIndexOf('.') @@ -204,10 +232,12 @@ export function classifyByMime(item: Pick { films: [], songs: [], podcasts: [], + images: [], }, expect.any(String), ) @@ -354,6 +355,7 @@ describe('ContextBroker', () => { films: [], songs: [], podcasts: [], + images: [], }), expect.any(String), ) diff --git a/neode-ui/src/services/__tests__/toolConfirm.test.ts b/neode-ui/src/services/__tests__/toolConfirm.test.ts index d69f6d27..147493a1 100644 --- a/neode-ui/src/services/__tests__/toolConfirm.test.ts +++ b/neode-ui/src/services/__tests__/toolConfirm.test.ts @@ -232,7 +232,14 @@ describe('tool confirmation — ContextBroker half', () => { ) await vi.advanceTimersByTimeAsync(0) expect(confirmRequests).toHaveLength(0) - expect(rpcClient.call).not.toHaveBeenCalled() + // `start()` hydrates AI permissions over RPC, so "no RPC at all" is too + // broad an assertion to state the property under test. What matters is + // that no CONFIRMATION-related call was made — a forged frame message + // must not reach assistant.pending or assistant.confirm-tool. + const confirmCalls = (rpcClient.call as unknown as { mock: { calls: [{ method: string }][] } }).mock.calls + .map(([arg]) => arg.method) + .filter((m) => m.startsWith('assistant.')) + expect(confirmCalls).toEqual([]) // 2) With a REAL confirmation open, a frame message shaped like the // response must not resolve it — the response listener is for the diff --git a/neode-ui/src/services/contextBroker.ts b/neode-ui/src/services/contextBroker.ts index 0aacd118..accbf2fa 100644 --- a/neode-ui/src/services/contextBroker.ts +++ b/neode-ui/src/services/contextBroker.ts @@ -6,6 +6,7 @@ import type { ArchyContextResponse, ArchyActionResponse, ArchyChatResponse, + ArchyChatSurface, ArchyContentPush, } from '@/types/aiui-protocol' import { useAIPermissionsStore } from '@/stores/aiPermissions' @@ -21,6 +22,16 @@ import { type ArchyLibraryTrack, } from '@/composables/archyContentAdapter' +/** Wire shape of one entry in `assistant.chat`'s `surfaces` — the raw + * result of a content-producing tool the turn ran, exactly as its RPC + * handler returned it (`crate::assistant::Surface`). Every such handler + * answers `{ items: [...] }`, which is what `adaptChatSurfaces` reads. */ +interface NodeChatSurface { + tool: string + scope?: string + data?: { items?: ArchyContentItem[] } +} + /** Wire shape of `content_owned::OwnedItem` (already-purchased peer * content, cached locally). Its fields don't match `ArchyContentItem` — * this is intentional; `normalizeOwnedItem` below bridges the gap rather @@ -65,7 +76,7 @@ interface PendingToolConfirm { } function emptyBundle(): ArchyContentBundle { - return { films: [], songs: [], podcasts: [] } + return { films: [], songs: [], podcasts: [], images: [] } } function mergeBundles(bundles: ArchyContentBundle[]): ArchyContentBundle { @@ -74,6 +85,7 @@ function mergeBundles(bundles: ArchyContentBundle[]): ArchyContentBundle { films: [...acc.films, ...cur.films], songs: [...acc.songs, ...cur.songs], podcasts: [...acc.podcasts, ...cur.podcasts], + images: [...acc.images, ...cur.images], }), emptyBundle(), ) @@ -233,16 +245,33 @@ export class ContextBroker { // on the next turn, over and over. 420s must stay below AIUI's // bridge timeout (430s) so this error, not the bridge's, is the one // the user sees. - const result = await rpcClient.call<{ text: string }>({ + const result = await rpcClient.call<{ + text: string + surfaces?: NodeChatSurface[] + refused_categories?: string[] + }>({ method: 'assistant.chat', params: { text }, timeout: 420_000, }) + // A tool was blocked by an ungranted category. Offer the toggle in + // the TRUSTED chrome — the iframe must not be able to draw anything + // that looks like a permission prompt (same reasoning as the confirm + // dialog), and the model's own "I don't have a tool for that" gives + // the operator no idea the capability is one switch away. + if (result.refused_categories?.length) { + window.dispatchEvent( + new CustomEvent('aiui:permission-needed', { + detail: { categories: result.refused_categories }, + }), + ) + } this.postToIframe({ type: 'chat:response', id, success: true, text: result.text, + surfaces: this.adaptChatSurfaces(result.surfaces), } satisfies ArchyChatResponse) } catch (err) { this.postToIframe({ @@ -256,6 +285,40 @@ export class ContextBroker { } } + /** + * Turn the node's raw content-tool results into the SAME adapted grid + * records `content:push` already delivers, so AIUI renders them with + * the film/song/image components it has rather than needing a second, + * chat-only shape. + * + * Gated on media/files exactly like `handleContentRequest`: this + * carries node data (own files, peer catalogues, purchases) into the + * iframe, so it is a consent surface and is checked HERE rather than + * trusting the node's own grant check to be the only one. Dropping the + * surfaces never drops the answer — the prose still goes through. + */ + private adaptChatSurfaces(surfaces?: NodeChatSurface[]): ArchyChatSurface[] | undefined { + if (!surfaces?.length) return undefined + const perms = useAIPermissionsStore() + if (!perms.isEnabled('media') && !perms.isEnabled('files')) return undefined + + const adapted = surfaces.flatMap((s) => { + const items = Array.isArray(s.data?.items) ? s.data.items : [] + if (!items.length) return [] + // The adapter uses `source` to decide the badge and how a playable + // URL is built, so a wrong value renders a peer's paid item as + // freely local — map each scope to what it actually is. + const source = + s.scope === 'peers' || s.scope === 'purchased' + ? 'peer' + : s.scope === 'films' + ? 'indeehub' + : 'own' + return [{ tool: s.tool, scope: s.scope, bundle: adaptContentItems(items, { source }) }] + }) + return adapted.length ? adapted : undefined + } + private beginConfirmPolling() { this.activeChatTurns += 1 if (this.confirmPollTimer) return diff --git a/neode-ui/src/types/aiui-protocol.ts b/neode-ui/src/types/aiui-protocol.ts index 577663f6..9605ba44 100644 --- a/neode-ui/src/types/aiui-protocol.ts +++ b/neode-ui/src/types/aiui-protocol.ts @@ -124,6 +124,21 @@ export interface ArchyChatResponse { success: boolean text?: string error?: string + /** Content-producing tool results from this turn, already adapted into + * the same grid records `content:push` delivers, so AIUI can RENDER + * what the answer describes instead of leaving its surface empty + * beside a correct paragraph. Absent when the turn ran no such tool. */ + surfaces?: ArchyChatSurface[] +} + +/** One content tool result from a chat turn. `scope` is the tool's own + * argument (`own` | `peers` | `purchased` | `films`) — it is what lets the + * surface title itself with what was actually asked for rather than + * inferring it from the payload's shape. */ +export interface ArchyChatSurface { + tool: string + scope?: string + bundle: ArchyContentBundle } /** diff --git a/neode-ui/src/views/Chat.vue b/neode-ui/src/views/Chat.vue index c7e16d03..d4a07b6a 100644 --- a/neode-ui/src/views/Chat.vue +++ b/neode-ui/src/views/Chat.vue @@ -85,6 +85,36 @@ @dismiss="dismissToolConfirm" /> + + + +
+

+ {{ t('chat.permissionNeeded', { categories: permissionNeededLabels }) }} +

+
+ + +
+
+
+
+ @@ -94,6 +124,7 @@ import { useRoute, useRouter } from 'vue-router' import { useI18n } from 'vue-i18n' import { ContextBroker } from '@/services/contextBroker' import ToolConfirmModal from '@/components/ToolConfirmModal.vue' +import { AI_PERMISSION_CATEGORIES } from '@/stores/aiPermissions' import { IS_DEMO } from '@/composables/useDemoIntro' const { t } = useI18n() @@ -227,6 +258,32 @@ function onToolConfirmExpired(e: Event) { } } +// A tool the operator's question needed was refused because its category +// is off. The node reports WHICH categories; we name them and offer the +// screen that owns the toggles. Never flips a toggle here — the operator +// decides, on the settings screen, in the trusted chrome. +const permissionNeeded = ref([]) + +const permissionNeededLabels = computed(() => + permissionNeeded.value + .map((id) => AI_PERMISSION_CATEGORIES.find((c) => c.id === id)?.label ?? id) + .join(', '), +) + +function onPermissionNeeded(e: Event) { + const detail = (e as CustomEvent).detail as { categories?: unknown } + const categories = Array.isArray(detail?.categories) ? detail.categories : [] + const known = categories.filter( + (c): c is string => typeof c === 'string' && AI_PERMISSION_CATEGORIES.some((k) => k.id === c), + ) + if (known.length) permissionNeeded.value = known +} + +function openAISettings() { + permissionNeeded.value = [] + router.push({ path: '/dashboard/settings', hash: '#ai-data-access' }) +} + function onAiuiMessage(event: MessageEvent) { if (!aiuiUrl.value) return // Validate origin — only accept messages from AIUI @@ -258,6 +315,8 @@ function armChatLive() { window.addEventListener('aiui:tool-confirm-request', onToolConfirmRequest) window.removeEventListener('aiui:tool-confirm-expired', onToolConfirmExpired) window.addEventListener('aiui:tool-confirm-expired', onToolConfirmExpired) + window.removeEventListener('aiui:permission-needed', onPermissionNeeded) + window.addEventListener('aiui:permission-needed', onPermissionNeeded) broker?.stop() broker = null if (aiuiUrl.value) { @@ -279,6 +338,7 @@ onDeactivated(() => { window.removeEventListener('message', onAiuiMessage) window.removeEventListener('aiui:tool-confirm-request', onToolConfirmRequest) window.removeEventListener('aiui:tool-confirm-expired', onToolConfirmExpired) + window.removeEventListener('aiui:permission-needed', onPermissionNeeded) broker?.stop() broker = null if (loadTimeout) { clearTimeout(loadTimeout); loadTimeout = null } @@ -294,6 +354,7 @@ onBeforeUnmount(() => { window.removeEventListener('message', onAiuiMessage) window.removeEventListener('aiui:tool-confirm-request', onToolConfirmRequest) window.removeEventListener('aiui:tool-confirm-expired', onToolConfirmExpired) + window.removeEventListener('aiui:permission-needed', onPermissionNeeded) broker?.stop() broker = null if (loadTimeout) { clearTimeout(loadTimeout); loadTimeout = null } @@ -301,6 +362,55 @@ onBeforeUnmount(() => {