Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,11 @@ jobs:
run: npm run check:bundle
working-directory: web/upload-page

- name: Test MCP App widget

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing ran web/mcp-app before this, so the new tests could break without anyone noticing. A new test folder needs its CI step in the same PR. (9a44c96)

if: needs.scope.outputs.run_suite == 'true' && matrix.python-version == '3.14'
run: npm test
working-directory: web/mcp-app

- name: Test
if: needs.scope.outputs.run_suite == 'true'
run: uv run --no-sync python -m unittest discover -s tests -q
Expand Down
228 changes: 224 additions & 4 deletions src/vidxp/assets/mcp_app/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,8 @@
.notice { min-height: 20px; color: var(--muted); font-size: 13px; }
.sr-only { position: absolute; width: 1px; height: 1px; overflow: hidden; clip: rect(0, 0, 0, 0); white-space: nowrap; }
@media (max-width: 560px) { .app { padding: 12px; } .header { align-items: stretch; flex-direction: column; } .header .button { align-self: flex-start; } }
.track { height: 6px; border-radius: 999px; background: var(--surface-2, rgba(127, 127, 127, 0.2)); overflow: hidden; }
.track > span { display: block; height: 100%; border-radius: inherit; background: var(--accent, #4f7cff); transition: width 160ms ease; }
</style>
</head>
<body>
Expand Down Expand Up @@ -82,10 +84,13 @@ <h1 id="title">Preparing video workspace…</h1>
const selected = new Set();
let requestId = 1;
let latestResult = null;
let progressTimer = null;
let uploadPageUrl = null;
let connected = false;
let hostCapabilities = {};
let hostContext = {};
let progressGeneration = 0;
let toolInput = null;

const element = (tag, className, text) => {
const node = document.createElement(tag);
Expand Down Expand Up @@ -167,6 +172,14 @@ <h1 id="title">Preparing video workspace…</h1>
return normalizeResult(await request("tools/call", { name, arguments: args }));
};

const stopProgressPolling = () => {
if (progressTimer !== null) {
window.clearTimeout(progressTimer);
progressTimer = null;
}
progressGeneration += 1;
};

const imageBlocks = (result) => asArray(result?.content).filter((block) => {
const mime = String(block?.mimeType ?? block?.mime_type ?? "").toLowerCase();
return block?.type === "image" && (mime === "image/jpeg" || mime === "image/png") && typeof block?.data === "string";
Expand Down Expand Up @@ -403,12 +416,216 @@ <h1 id="title">Preparing video workspace…</h1>
content.replaceChildren(wrapper);
};

const render = (result) => {
const persistMediaSelection = (mediaId) => {
if (connected) {
void request("ui/update-model-context", {
structuredContent: { selectedMediaId: mediaId },
}).catch(() => undefined);
}
if (window.openai?.setWidgetState) {
void Promise.resolve(window.openai.setWidgetState({ selectedMediaId: mediaId })).catch(() => undefined);
}
};

const humanBytes = (value) => {
const bytes = Number(value);
if (!Number.isFinite(bytes) || bytes <= 0) return "";
const units = ["B", "KB", "MB", "GB", "TB"];
let size = bytes;
let unit = 0;
while (size >= 1024 && unit < units.length - 1) {
size /= 1024;
unit += 1;
}
return `${size < 10 && unit > 0 ? size.toFixed(1) : Math.round(size)} ${units[unit]}`;
};

const humanDuration = (value) => {
const total = Number(value);
if (!Number.isFinite(total) || total <= 0) return "";
const seconds = Math.round(total);
const minutes = Math.floor(seconds / 60);
return `${minutes}:${String(seconds % 60).padStart(2, "0")}`;
};

const renderLibrary = (data, query) => {
const items = asArray(data.items).map(asObject);
title.textContent = "Select a video";
lede.textContent = items.length
? "Choose a registered video to work with, or load more from the library."
: "No registered videos yet. Upload one first.";
const wrapper = document.createDocumentFragment();

const overview = panel("Library");
const metrics = element("div", "metrics");
metrics.append(
metric(data.total ?? items.length, "registered"),
metric(items.length, "shown"),
metric(items.filter((item) => item.state === "ready").length, "ready")
);
overview.append(metrics);
wrapper.append(overview);

if (items.length) {
const listing = panel("Videos");
const list = element("div", "tile-list");
for (const item of items) {
if (typeof item.media_id !== "string") continue;
const tile = element("div", "tile");
const copy = element("div", "tile-copy");
copy.append(element("p", "tile-title", item.original_filename || item.media_id));
const facts = [item.state, humanDuration(item.duration_seconds), humanBytes(item.byte_size), item.container]
.filter((fact) => typeof fact === "string" && fact.length > 0);
copy.append(element("p", "tile-meta", facts.join(" · ")));
tile.append(copy);

const choose = element("button", "button primary", "Use this video");
choose.type = "button";
choose.addEventListener("click", async () => {
choose.disabled = true;
notice.textContent = "";
try {
const next = await callTool("get_media", { media_id: item.media_id });
if (next.isError) throw new Error("VidXP could not find that video.");

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selection was saved before get_media answered, and an error reply counted as success, so a missing video still showed as selected. It gets the same isError check as the upload and evidence buttons, and is saved only after that passes. (99546a2)

persistMediaSelection(item.media_id);
Comment on lines +488 to +490

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Older replies undo newer choices

Each selection button disables only itself. A user can choose video A and then B while both get_media calls are pending. If A finishes last, persistMediaSelection replaces B with A.

The “Load more” handler has the same problem: its reply calls render even after a newer host result opens another view. That can replace job progress and stop its updates.

Ignore replies from an older view, and let only the latest selection request save selectedMediaId.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/vidxp/assets/mcp_app/index.html
Line: 488-490

Comment:
**Older replies undo newer choices**

Each selection button disables only itself. A user can choose video A and then B while both `get_media` calls are pending. If A finishes last, `persistMediaSelection` replaces B with A.

The “Load more” handler has the same problem: its reply calls `render` even after a newer host result opens another view. That can replace job progress and stop its updates.

Ignore replies from an older view, and let only the latest selection request save `selectedMediaId`.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

const detail = next.structuredContent;
lede.textContent = `Selected ${detail.original_filename || item.original_filename || item.media_id}.`;
} catch (_error) {
notice.textContent = "Could not select that video.";
} finally {
choose.disabled = false;
}
});
tile.append(choose);
list.append(tile);
}
listing.append(list);
wrapper.append(listing);
}

if (typeof data.next_cursor === "string" && data.next_cursor && query) {
Comment thread
greptile-apps[bot] marked this conversation as resolved.
const actions = element("div", "toolbar");
const more = element("button", "button", "Load more");
more.type = "button";
more.addEventListener("click", async () => {
more.disabled = true;
notice.textContent = "";
try {
const nextQuery = { ...query, cursor: data.next_cursor };
const next = await callTool("list_media", nextQuery);
if (next.isError) throw new Error("VidXP could not load more videos.");
const page = next.structuredContent;
render(
{ ...next, structuredContent: { ...page, items: [...items, ...asArray(page.items)] } },
{ mediaQuery: nextQuery }
);
} catch (_error) {
notice.textContent = "Could not load more videos.";
more.disabled = false;
}
});
actions.append(more);
wrapper.append(actions);
}

content.replaceChildren(wrapper);
};

const renderProgress = (data) => {
const generation = progressGeneration;
const progress = asObject(data.progress);
const state = typeof data.state === "string" ? data.state : "unknown";
const kind = typeof data.kind === "string" ? data.kind : "job";
title.textContent = kind === "index" ? "Indexing progress" : `VidXP ${kind}`;
lede.textContent = progress.message || `Job is ${state}.`;
const wrapper = document.createDocumentFragment();

const overview = panel("Status");
const metrics = element("div", "metrics");
metrics.append(metric(state, "state"), metric(progress.stage || "—", "stage"));
const current = Number(progress.current);
const total = Number(progress.total);
const measured = Number.isFinite(current) && Number.isFinite(total) && total > 0;
if (measured) metrics.append(metric(`${current}/${total}`, "steps"));
overview.append(metrics);
if (measured) {
const track = element("div", "track");
const fill = element("span");
fill.style.width = `${Math.min(100, Math.round((current / total) * 100))}%`;
track.append(fill);
overview.append(track);
}
wrapper.append(overview);

const error = asObject(data.error);
if (error.message) {
const failure = panel("Error");
failure.append(element("p", "error", error.message));
wrapper.append(failure);
}

const jobId = typeof data.job_id === "string" ? data.job_id : "";
if (jobId) {
const schedule = () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One failed status check used to end polling: the catch set the notice but never scheduled the next check, and an error result replaced the progress view and its Refresh button. Every path through poll reschedules now, and an error only updates the notice. (5fd0874)

if (data.terminal !== false) return;
if (progressTimer !== null) window.clearTimeout(progressTimer);
const delay = Number(data.poll_after_seconds);
progressTimer = window.setTimeout(() => {
progressTimer = null;
void poll();
}, (Number.isFinite(delay) && delay > 0 ? delay : 1) * 1000);
};
const poll = async () => {
if (generation !== progressGeneration) return;
let next;
try {
next = await callTool("get_job_status", { job_id: jobId });
} catch (_error) {
if (generation !== progressGeneration) return;
notice.textContent = "Could not refresh job status.";
schedule();
return;
}
if (generation !== progressGeneration) return;
if (next.isError) {
notice.textContent = "VidXP returned an error checking job status.";
schedule();
return;
}
render(next);
};
const actions = element("div", "toolbar");
const refresh = element("button", "button", "Refresh status");
refresh.type = "button";
refresh.addEventListener("click", async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the refresh request fails, poll() catches the error but this button stays disabled. The user cannot retry that refresh. Please restore the button after a failed request, for example through finally, and cover the failure-and-retry path.

refresh.disabled = true;
notice.textContent = "";
try {
await poll();
} finally {
refresh.disabled = false;
}
});
actions.append(refresh);
wrapper.append(actions);
schedule();
}

content.replaceChildren(wrapper);
};

const render = (result, context = {}) => {
latestResult = normalizeResult(result);
const data = latestResult.structuredContent;
stopProgressPolling();
notice.textContent = latestResult.isError ? "VidXP returned an error." : "";
if (data.view === "upload" || data.upload_session_url || data.aggregate_state) renderUpload(data);
else if (data.view === "evidence" || data.board) renderEvidence(data, latestResult);
else if (
data.view === "library" ||
(Array.isArray(data.items) && Number.isInteger(data.total) && (data.next_cursor === null || typeof data.next_cursor === "string"))
) renderLibrary(data, context.mediaQuery);
else if (data.view === "job" || typeof data.job_id === "string") renderProgress(data);
else {
title.textContent = "VidXP";
lede.textContent = "This tool result does not include an interactive view.";
Expand All @@ -428,10 +645,12 @@ <h1 id="title">Preparing video workspace…</h1>
else request.resolve(message.result);
return;
}
if (message.method === "ui/notifications/tool-result") render(message.params);
if (message.method === "ui/notifications/tool-input") toolInput = asObject(message.params?.arguments);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Load more never showed on the first page: it only appears when the widget knows the original query, and the first page comes from the host without one. The host already sends the tool arguments in ui/notifications/tool-input before the result (window.openai.toolInput in ChatGPT), so we keep them here and pass them to the first render. Load more also adds pages to the list now instead of replacing it. (659b88b)

if (message.method === "ui/notifications/tool-result") render(message.params, { mediaQuery: toolInput });
if (message.method === "ui/notifications/host-context-changed") updateHostContext(message.params);
if (message.method === "ui/notifications/tool-cancelled") notice.textContent = message.params?.reason || "The tool call was cancelled.";
if (message.method === "ui/resource-teardown" && message.id !== undefined) {
stopProgressPolling();
window.parent.postMessage({ jsonrpc: "2.0", id: message.id, result: {} }, "*");
}
}, { passive: true });
Expand All @@ -450,8 +669,9 @@ <h1 id="title">Preparing video workspace…</h1>
});
updateHostContext({});
const initialMetadata = window.openai?.toolResponseMetadata;
if (initialMetadata) render(initialMetadata);
else if (window.openai?.toolOutput) render({ structuredContent: window.openai.toolOutput, content: [] });
toolInput = window.openai?.toolInput ? asObject(window.openai.toolInput) : null;
if (initialMetadata) render(initialMetadata, { mediaQuery: toolInput });
else if (window.openai?.toolOutput) render({ structuredContent: window.openai.toolOutput, content: [] }, { mediaQuery: toolInput });
if (typeof ResizeObserver === "function") {
new ResizeObserver(() => {
if (connected) {
Expand Down
8 changes: 8 additions & 0 deletions src/vidxp/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1352,6 +1352,10 @@ async def get_runtime_readiness() -> RuntimeReadiness:
"that a video is present in the active index snapshot."
),
annotations=_READ_ONLY,
meta=_mcp_app_tool_meta(
"Listing VidXP media…",
"VidXP media library ready.",
),
structured_output=True,
)
async def list_media(
Expand Down Expand Up @@ -2060,6 +2064,10 @@ def completed_evidence_job(_actor: Principal) -> Job:
"initial observation."
),
annotations=_READ_ONLY,
meta=_mcp_app_tool_meta(
"Checking VidXP job status…",
"VidXP job status ready.",
),
structured_output=True,
)
async def get_job_status(job_id: JobId) -> JobSummary:
Expand Down
13 changes: 13 additions & 0 deletions web/mcp-app/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"name": "@vidxp/mcp-app",
"private": true,
"version": "0.0.0",
"description": "Behavioural tests for the MCP App widget. The widget itself is hand-authored at src/vidxp/assets/mcp_app/index.html and has no build step, so this package carries tests only.",
"type": "module",
"engines": {
"node": ">=22"
},
"scripts": {
"test": "node --test test/*.test.mjs"
}
}
Loading
Loading