From 6c450fd4f39eec3091d73e13b3314752b5cc0467 Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 05:39:55 +0800 Subject: [PATCH 1/4] feat: link frontend tool registry to generated tool_types.json manifest Generate static/tool_types.json from KNOWN_TOOL_TYPES and load it at SPA boot to cross-check TOOL_USE_RENDERERS with console.warn on drift. Improve unknown tool/result fallbacks to show raw type names and JSON payloads. Extend test_tool_dispatch_sync.py with manifest freshness and coverage assertions. Await manifest init before first route render; document the flow in CONTRIBUTING.md, Makefile, and docs/architecture.md. --- CONTRIBUTING.md | 5 +- Makefile | 3 ++ docs/architecture.md | 1 + scripts/gen_tool_types_manifest.py | 36 ++++++++++++++ static/js/app.js | 4 +- static/js/app.test.js | 4 ++ static/js/render/registry.js | 7 +-- static/js/render/registry.test.js | 16 +++++-- static/js/render/tool_result/fallback.js | 7 ++- static/js/render/tool_types_manifest.js | 39 ++++++++++++++++ static/js/render/tool_types_manifest.test.js | 49 ++++++++++++++++++++ static/js/render/tool_types_state.js | 15 ++++++ static/js/render/tool_use/fallback.js | 3 +- static/tool_types.json | 15 ++++++ tests/test_tool_dispatch_sync.py | 38 +++++++++++++++ 15 files changed, 227 insertions(+), 15 deletions(-) create mode 100644 scripts/gen_tool_types_manifest.py create mode 100644 static/js/render/tool_types_manifest.js create mode 100644 static/js/render/tool_types_manifest.test.js create mode 100644 static/js/render/tool_types_state.js create mode 100644 static/tool_types.json diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 5a9dace..fd27d94 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -149,8 +149,9 @@ Claude Code assistant `tool_use` blocks carry a `name` string (e.g. `"Read"`, `" 2. **`models/tool_results.py`** — add the name to `ToolNameLiteral` and, when the tool has a distinct result payload, add the TypedDict, type guard (`is_*_tool_result`), and union member on `ToolResultUnion`. 3. **`utils/md_exporter.py`** — add an `elif name == "…"` branch in `_render_tool_use` (sync test parses these branches). 4. **`static/js/render/registry.js`** — add a `TOOL_USE_RENDERERS` entry (and a `tool_use/*.js` renderer module). -5. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module. -6. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type. +5. Regenerate **`static/tool_types.json`**: `python scripts/gen_tool_types_manifest.py` +6. **Optional result UI** — if the backend emits a new `result_type`, add `TOOL_RESULT_RENDERERS` and a `tool_result/*.js` module. +7. Run `pytest tests/test_tool_dispatch_sync.py -v` — failure names the site missing the new type. ## Getting help diff --git a/Makefile b/Makefile index d3d6d9b..8d7ce6e 100644 --- a/Makefile +++ b/Makefile @@ -11,6 +11,9 @@ seed-baselines-local: # Deprecated alias — kept for muscle memory; see seed-baselines-local warning above. update-baselines: seed-baselines-local +gen-tool-types-manifest: + PYTHONPATH=. python scripts/gen_tool_types_manifest.py + check-benchmarks: PYTHONPATH=. pytest tests/benchmarks/ --benchmark-only --benchmark-json=benchmark-results.json -o addopts= PYTHONPATH=. python scripts/check_benchmark_regression.py benchmark-results.json benchmarks/baselines.json diff --git a/docs/architecture.md b/docs/architecture.md index 86fdb59..5fccda5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -106,6 +106,7 @@ The UI is a **hash-routed** SPA with ES modules under `static/js/`: - `app.js` — routing and boot - `projects.js`, `sessions.js`, `search.js`, `export.js` — route handlers - `render/registry.js` — **tool dispatch registry** for session UI: `TOOL_USE_RENDERERS` and `TOOL_RESULT_RENDERERS` map tool name / `result_type` → render function (one module per type under `render/tool_use/` and `render/tool_result/`). Parallels backend `utils/tool_dispatch.py` (backend uses ordered predicates; frontend uses direct key lookup + fallback). +- `static/tool_types.json` — generated manifest of backend tool-use names (`python scripts/gen_tool_types_manifest.py` from `KNOWN_TOOL_TYPES`). Loaded at boot by `render/tool_types_manifest.js`, which cross-checks `TOOL_USE_RENDERERS` and logs `console.warn` on drift. - `shared/markdown.js` — markdown + **DOMPurify** sanitization (do not render raw LLM HTML) - `shared/state.js`, `shared/utils.js`, `shared/theme.js` — shared UI state and helpers diff --git a/scripts/gen_tool_types_manifest.py b/scripts/gen_tool_types_manifest.py new file mode 100644 index 0000000..adde3ba --- /dev/null +++ b/scripts/gen_tool_types_manifest.py @@ -0,0 +1,36 @@ +#!/usr/bin/env python3 +"""Write ``static/tool_types.json`` from ``KNOWN_TOOL_TYPES``. + +Run after adding a tool type to ``utils/tool_dispatch.py``:: + + python scripts/gen_tool_types_manifest.py +""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).resolve().parents[1] +_MANIFEST_PATH = _REPO_ROOT / "static" / "tool_types.json" + + +def write_tool_types_manifest(path: Path | None = None) -> int: + if str(_REPO_ROOT) not in sys.path: + sys.path.insert(0, str(_REPO_ROOT)) + from utils.tool_dispatch import KNOWN_TOOL_TYPES + + dest = path or _MANIFEST_PATH + payload = {"tool_types": sorted(KNOWN_TOOL_TYPES)} + dest.write_text(json.dumps(payload, indent=2) + "\n", encoding="utf-8") + return len(KNOWN_TOOL_TYPES) + + +def main() -> None: + count = write_tool_types_manifest() + print(f"Wrote {count} tool types to {_MANIFEST_PATH}") + + +if __name__ == "__main__": + main() diff --git a/static/js/app.js b/static/js/app.js index d472670..20483e3 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -8,6 +8,7 @@ import { HLJS_THEME_SHEETS, applyTheme, toggleTheme } from './shared/theme.js'; import { showProjects } from './projects.js'; import { showWorkspace, loadSession } from './sessions.js'; import { showSearchPage } from './search.js'; +import { initToolTypesManifest } from './render/tool_types_manifest.js'; // ==================== Router ==================== @@ -48,10 +49,11 @@ function handleRoute() { // ==================== Bootstrap ==================== -document.addEventListener('DOMContentLoaded', () => { +document.addEventListener('DOMContentLoaded', async () => { applyTheme(localStorage.getItem('theme') || 'dark'); const yearEl = document.getElementById('footer-year'); if (yearEl) yearEl.textContent = new Date().getFullYear(); + await initToolTypesManifest(); handleRoute(); window.addEventListener('hashchange', handleRoute); diff --git a/static/js/app.test.js b/static/js/app.test.js index c2a255e..e95b10e 100644 --- a/static/js/app.test.js +++ b/static/js/app.test.js @@ -23,6 +23,9 @@ vi.mock('./shared/theme.js', () => ({ toggleTheme, setWorkspaceMode: vi.fn(), })); +vi.mock('./render/tool_types_manifest.js', () => ({ + initToolTypesManifest: vi.fn().mockResolvedValue(undefined), +})); describe('router (app.js)', () => { const origScrollTo = window.scrollTo; @@ -33,6 +36,7 @@ describe('router (app.js)', () => { Element.prototype.scrollIntoView = vi.fn(); await import('./app.js'); document.dispatchEvent(new Event('DOMContentLoaded')); + await Promise.resolve(); }); afterAll(() => { diff --git a/static/js/render/registry.js b/static/js/render/registry.js index 893a8dd..7179bb8 100644 --- a/static/js/render/registry.js +++ b/static/js/render/registry.js @@ -66,9 +66,10 @@ function getToolUseRenderer(name) { } function getToolResultRenderer(resultType) { - return Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, resultType) - ? TOOL_RESULT_RENDERERS[resultType] - : renderToolResultFallback; + if (Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, resultType)) { + return TOOL_RESULT_RENDERERS[resultType]; + } + return renderToolResultFallback; } export function renderToolUse(tool) { diff --git a/static/js/render/registry.test.js b/static/js/render/registry.test.js index 432225e..28ee6b8 100644 --- a/static/js/render/registry.test.js +++ b/static/js/render/registry.test.js @@ -133,12 +133,13 @@ describe('getToolSummary', () => { }); describe('renderToolUse fallback', () => { - it('uses JSON fallback for unknown tools', () => { + it('uses JSON fallback for unknown tools with raw type name', () => { const html = renderToolUse({ name: 'UnknownToolXYZ', input: { foo: 'bar' }, }); expect(html).toContain('tool-call'); + expect(html).toContain('Unknown tool: UnknownToolXYZ'); expect(html).toContain('"foo"'); expect(TOOL_USE_RENDERERS.UnknownToolXYZ).toBeUndefined(); }); @@ -188,15 +189,20 @@ describe('renderTodoWriteResult', () => { }); describe('renderToolResult fallback', () => { - it('renders summary-only for unknown result types', () => { - const html = renderToolResult({ result_type: 'custom_type' }); - expect(html).toContain('Tool result (custom_type)'); + it('renders raw result type and JSON payload for unknown result types', () => { + const html = renderToolResult({ + result_type: 'custom_type', + payload: { answer: 42 }, + }); + expect(html).toContain('Unknown tool result: custom_type'); expect(html).toContain('tool-result'); + expect(html).toContain('"answer"'); + expect(html).toContain('42'); }); it('uses fallback when result_type is an inherited property (e.g. constructor)', () => { const html = renderToolResult({ result_type: 'constructor' }); - expect(html).toContain('Tool result (constructor)'); + expect(html).toContain('Unknown tool result: constructor'); expect(html).toContain('tool-result'); expect(Object.prototype.hasOwnProperty.call(TOOL_RESULT_RENDERERS, 'constructor')).toBe(false); }); diff --git a/static/js/render/tool_result/fallback.js b/static/js/render/tool_result/fallback.js index bef7425..edba1c3 100644 --- a/static/js/render/tool_result/fallback.js +++ b/static/js/render/tool_result/fallback.js @@ -1,8 +1,11 @@ +import { esc, truncate } from '../../shared/utils.js'; import { finishToolResult } from './common.js'; import { UNKNOWN_DISPATCH_KEY } from '../constants.js'; export function renderToolResultFallback(parsed) { const rt = parsed.result_type || UNKNOWN_DISPATCH_KEY; - const summary = `Tool result (${rt})`; - return finishToolResult(summary, ''); + const summary = `Unknown tool result: ${rt}`; + const payload = JSON.stringify(parsed, null, 2); + const body = `
${esc(truncate(payload, 500))}
`; + return finishToolResult(summary, body); } diff --git a/static/js/render/tool_types_manifest.js b/static/js/render/tool_types_manifest.js new file mode 100644 index 0000000..378524e --- /dev/null +++ b/static/js/render/tool_types_manifest.js @@ -0,0 +1,39 @@ +import { TOOL_USE_RENDERERS } from './registry.js'; +import { setManifestToolTypes } from './tool_types_state.js'; + +const MANIFEST_URL = '/static/tool_types.json'; + +/** + * Load backend tool-type manifest and cross-check ``TOOL_USE_RENDERERS``. + * Logs ``console.warn`` when the backend list and frontend registry diverge. + */ +export async function initToolTypesManifest() { + try { + const res = await fetch(MANIFEST_URL); + if (!res.ok) { + console.warn(`[tool registry] Could not load ${MANIFEST_URL}: HTTP ${res.status}`); + return; + } + const data = await res.json(); + const types = Array.isArray(data.tool_types) ? data.tool_types : []; + const manifest = new Set(types.filter((t) => typeof t === 'string')); + setManifestToolTypes(manifest); + + for (const name of manifest) { + if (!Object.prototype.hasOwnProperty.call(TOOL_USE_RENDERERS, name)) { + console.warn( + `[tool registry] Backend tool type "${name}" has no TOOL_USE_RENDERERS entry`, + ); + } + } + for (const name of Object.keys(TOOL_USE_RENDERERS)) { + if (!manifest.has(name)) { + console.warn( + `[tool registry] TOOL_USE_RENDERERS entry "${name}" is missing from ${MANIFEST_URL}`, + ); + } + } + } catch (err) { + console.warn('[tool registry] Could not load tool types manifest:', err); + } +} diff --git a/static/js/render/tool_types_manifest.test.js b/static/js/render/tool_types_manifest.test.js new file mode 100644 index 0000000..3a6e96f --- /dev/null +++ b/static/js/render/tool_types_manifest.test.js @@ -0,0 +1,49 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { initToolTypesManifest } from './tool_types_manifest.js'; +import { getManifestToolTypes, setManifestToolTypes } from './tool_types_state.js'; +import { TOOL_USE_RENDERERS } from './registry.js'; + +// Registry drift warnings are asserted here (init-time cross-check only; no per-render warn). + +describe('initToolTypesManifest', () => { + beforeEach(() => { + setManifestToolTypes(null); + vi.restoreAllMocks(); + }); + + afterEach(() => { + setManifestToolTypes(null); + }); + + it('cross-checks manifest against TOOL_USE_RENDERERS and warns on drift', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const manifestTypes = [...Object.keys(TOOL_USE_RENDERERS), 'FutureToolXYZ']; + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ tool_types: manifestTypes }), + }), + ); + + await initToolTypesManifest(); + + expect(getManifestToolTypes()).toEqual(new Set(manifestTypes)); + expect(warn).toHaveBeenCalledWith( + '[tool registry] Backend tool type "FutureToolXYZ" has no TOOL_USE_RENDERERS entry', + ); + }); + + it('warns when fetch fails', async () => { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network'))); + + await initToolTypesManifest(); + + expect(getManifestToolTypes()).toBeNull(); + expect(warn).toHaveBeenCalledWith( + '[tool registry] Could not load tool types manifest:', + expect.any(Error), + ); + }); +}); diff --git a/static/js/render/tool_types_state.js b/static/js/render/tool_types_state.js new file mode 100644 index 0000000..5f27fd5 --- /dev/null +++ b/static/js/render/tool_types_state.js @@ -0,0 +1,15 @@ +/** Backend tool-use names from ``static/tool_types.json`` (set at SPA init). */ + +let manifestToolTypes = null; + +export function setManifestToolTypes(types) { + manifestToolTypes = types; +} + +export function getManifestToolTypes() { + return manifestToolTypes; +} + +export function isManifestToolType(name) { + return manifestToolTypes !== null && manifestToolTypes.has(name); +} diff --git a/static/js/render/tool_use/fallback.js b/static/js/render/tool_use/fallback.js index 1c7bbfb..72c2968 100644 --- a/static/js/render/tool_use/fallback.js +++ b/static/js/render/tool_use/fallback.js @@ -1,12 +1,11 @@ import { esc, truncate } from '../../shared/utils.js'; -import { getToolSummary } from './summary.js'; import { wrapToolUse } from './common.js'; import { UNKNOWN_DISPATCH_KEY } from '../constants.js'; export function renderToolUseFallback(tool) { const name = tool.name || UNKNOWN_DISPATCH_KEY; const inp = tool.input || {}; - const summary = getToolSummary(name, inp); + const summary = `Unknown tool: ${name}`; const s = JSON.stringify(inp, null, 2); const body = `
${esc(truncate(s, 500))}
`; return wrapToolUse(summary, body); diff --git a/static/tool_types.json b/static/tool_types.json new file mode 100644 index 0000000..b6670c1 --- /dev/null +++ b/static/tool_types.json @@ -0,0 +1,15 @@ +{ + "tool_types": [ + "AskUserQuestion", + "Bash", + "Edit", + "Glob", + "Grep", + "Read", + "Task", + "TodoWrite", + "WebFetch", + "WebSearch", + "Write" + ] +} diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 7063bc0..2c924bc 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/tests/test_tool_dispatch_sync.py @@ -1,6 +1,7 @@ """Contract test: ``KNOWN_TOOL_TYPES`` must match all four dispatch sites. Sites (each compared to ``KNOWN_TOOL_TYPES`` in ``utils/tool_dispatch.py``): +- ``static/tool_types.json`` — generated manifest - ``utils/md_exporter.py`` — ``_render_tool_use`` if/elif branches (parsed) - ``models/tool_results.py`` — ``ToolNameLiteral`` - ``static/js/render/registry.js`` — ``TOOL_USE_RENDERERS`` keys (parsed) @@ -8,6 +9,7 @@ from __future__ import annotations +import json import re from pathlib import Path from typing import get_args @@ -15,11 +17,13 @@ import pytest from models.tool_results import ToolNameLiteral +from scripts.gen_tool_types_manifest import write_tool_types_manifest from utils.tool_dispatch import KNOWN_TOOL_TYPES _REPO_ROOT = Path(__file__).resolve().parents[1] _FRONTEND_REGISTRY = _REPO_ROOT / "static" / "js" / "render" / "registry.js" _MD_EXPORTER = _REPO_ROOT / "utils" / "md_exporter.py" +_TOOL_TYPES_MANIFEST = _REPO_ROOT / "static" / "tool_types.json" def _format_set_diff(expected: frozenset[str], actual: frozenset[str], site: str) -> str: @@ -81,6 +85,36 @@ def _parse_md_exporter_tool_use_handlers(path: Path) -> frozenset[str]: return frozenset(names) +def _load_manifest_tool_types(path: Path) -> frozenset[str]: + if not path.is_file(): + msg = f"Missing manifest: {path} (run python scripts/gen_tool_types_manifest.py)" + raise ValueError(msg) + data = json.loads(path.read_text(encoding="utf-8")) + raw = data.get("tool_types") + if not isinstance(raw, list): + msg = f"Invalid tool_types in {path}: expected a JSON array" + raise ValueError(msg) + return frozenset(str(t) for t in raw) + + +def test_tool_types_manifest_matches_known_tool_types() -> None: + site = "static/tool_types.json" + try: + actual = _load_manifest_tool_types(_TOOL_TYPES_MANIFEST) + except ValueError as exc: + pytest.fail(f"{site}: {exc}") + if actual != KNOWN_TOOL_TYPES: + pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) + + +def test_tool_types_manifest_is_committed_and_current(tmp_path: Path) -> None: + """Regenerating the manifest must match the committed file.""" + expected = tmp_path / "tool_types.json" + write_tool_types_manifest(expected) + committed = _TOOL_TYPES_MANIFEST.read_text(encoding="utf-8") + assert expected.read_text(encoding="utf-8") == committed + + def test_md_exporter_handlers_match_known_tool_types() -> None: site = "utils/md_exporter.py (_render_tool_use branches)" try: @@ -99,13 +133,17 @@ def test_tool_name_literal_matches_known_tool_types() -> None: def test_frontend_registry_matches_known_tool_types() -> None: + """``TOOL_USE_RENDERERS`` keys must match ``KNOWN_TOOL_TYPES`` and the manifest.""" site = "static/js/render/registry.js (TOOL_USE_RENDERERS)" try: actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY) + manifest = _load_manifest_tool_types(_TOOL_TYPES_MANIFEST) except ValueError as exc: pytest.fail(f"{site}: {exc}") if actual != KNOWN_TOOL_TYPES: pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) + if actual != manifest: + pytest.fail(_format_set_diff(manifest, actual, site)) def test_known_tool_types_nonempty() -> None: From d6f4cfda0cde0ef33bf2924ea2295c65d5ac69d6 Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 06:02:40 +0800 Subject: [PATCH 2/4] fix: non-blocking manifest init, fetch timeout, stricter manifest parse --- Makefile | 2 +- static/js/app.js | 4 ++-- static/js/app.test.js | 1 - static/js/render/tool_types_manifest.js | 13 +++++++++- static/js/render/tool_types_manifest.test.js | 25 ++++++++++++++++++++ tests/test_tool_dispatch_sync.py | 8 ++++++- 6 files changed, 47 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 8d7ce6e..bfb00cc 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: seed-baselines-local update-baselines check-benchmarks clean-benchmark-artifacts +.PHONY: seed-baselines-local update-baselines gen-tool-types-manifest check-benchmarks clean-benchmark-artifacts # WARNING: captures timings on THIS machine. Production baselines must match ubuntu-latest CI. # Prefer downloading benchmark-results.json from a CI artifact, then: diff --git a/static/js/app.js b/static/js/app.js index 20483e3..f362138 100644 --- a/static/js/app.js +++ b/static/js/app.js @@ -49,11 +49,11 @@ function handleRoute() { // ==================== Bootstrap ==================== -document.addEventListener('DOMContentLoaded', async () => { +document.addEventListener('DOMContentLoaded', () => { applyTheme(localStorage.getItem('theme') || 'dark'); const yearEl = document.getElementById('footer-year'); if (yearEl) yearEl.textContent = new Date().getFullYear(); - await initToolTypesManifest(); + void initToolTypesManifest(); handleRoute(); window.addEventListener('hashchange', handleRoute); diff --git a/static/js/app.test.js b/static/js/app.test.js index e95b10e..c034155 100644 --- a/static/js/app.test.js +++ b/static/js/app.test.js @@ -36,7 +36,6 @@ describe('router (app.js)', () => { Element.prototype.scrollIntoView = vi.fn(); await import('./app.js'); document.dispatchEvent(new Event('DOMContentLoaded')); - await Promise.resolve(); }); afterAll(() => { diff --git a/static/js/render/tool_types_manifest.js b/static/js/render/tool_types_manifest.js index 378524e..e5cb5b3 100644 --- a/static/js/render/tool_types_manifest.js +++ b/static/js/render/tool_types_manifest.js @@ -2,14 +2,17 @@ import { TOOL_USE_RENDERERS } from './registry.js'; import { setManifestToolTypes } from './tool_types_state.js'; const MANIFEST_URL = '/static/tool_types.json'; +const MANIFEST_FETCH_TIMEOUT_MS = 5000; /** * Load backend tool-type manifest and cross-check ``TOOL_USE_RENDERERS``. * Logs ``console.warn`` when the backend list and frontend registry diverge. */ export async function initToolTypesManifest() { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), MANIFEST_FETCH_TIMEOUT_MS); try { - const res = await fetch(MANIFEST_URL); + const res = await fetch(MANIFEST_URL, { signal: controller.signal }); if (!res.ok) { console.warn(`[tool registry] Could not load ${MANIFEST_URL}: HTTP ${res.status}`); return; @@ -34,6 +37,14 @@ export async function initToolTypesManifest() { } } } catch (err) { + if (err?.name === 'AbortError') { + console.warn( + `[tool registry] Could not load ${MANIFEST_URL}: timed out after ${MANIFEST_FETCH_TIMEOUT_MS}ms`, + ); + return; + } console.warn('[tool registry] Could not load tool types manifest:', err); + } finally { + clearTimeout(timeoutId); } } diff --git a/static/js/render/tool_types_manifest.test.js b/static/js/render/tool_types_manifest.test.js index 3a6e96f..5c7fc07 100644 --- a/static/js/render/tool_types_manifest.test.js +++ b/static/js/render/tool_types_manifest.test.js @@ -46,4 +46,29 @@ describe('initToolTypesManifest', () => { expect.any(Error), ); }); + + it('warns when fetch times out', async () => { + vi.useFakeTimers(); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi.fn((_url, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); + }); + }), + ), + ); + + const promise = initToolTypesManifest(); + await vi.advanceTimersByTimeAsync(5000); + await promise; + + expect(getManifestToolTypes()).toBeNull(); + expect(warn).toHaveBeenCalledWith( + '[tool registry] Could not load /static/tool_types.json: timed out after 5000ms', + ); + vi.useRealTimers(); + }); }); diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 2c924bc..1245e48 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/tests/test_tool_dispatch_sync.py @@ -94,7 +94,13 @@ def _load_manifest_tool_types(path: Path) -> frozenset[str]: if not isinstance(raw, list): msg = f"Invalid tool_types in {path}: expected a JSON array" raise ValueError(msg) - return frozenset(str(t) for t in raw) + for i, item in enumerate(raw): + if not isinstance(item, str): + msg = ( + f"Invalid tool_types[{i}] in {path}: expected string, got {type(item).__name__}" + ) + raise ValueError(msg) + return frozenset(raw) def test_tool_types_manifest_matches_known_tool_types() -> None: From 8475dc3b9629078498258b63627c24c96b26753e Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 06:42:47 +0800 Subject: [PATCH 3/4] fix(test): ensure fake timers cleanup in manifest timeout test Wrap the initToolTypesManifest timeout test in try/finally so vi.useRealTimers() runs even when assertions fail. Also ruff-format test_tool_dispatch_sync.py manifest validation helper (CI gate). --- static/js/render/tool_types_manifest.test.js | 41 +++++++++++--------- tests/test_tool_dispatch_sync.py | 4 +- 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/static/js/render/tool_types_manifest.test.js b/static/js/render/tool_types_manifest.test.js index 5c7fc07..ade5ffb 100644 --- a/static/js/render/tool_types_manifest.test.js +++ b/static/js/render/tool_types_manifest.test.js @@ -49,26 +49,29 @@ describe('initToolTypesManifest', () => { it('warns when fetch times out', async () => { vi.useFakeTimers(); - const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); - vi.stubGlobal( - 'fetch', - vi.fn((_url, init) => - new Promise((_resolve, reject) => { - init?.signal?.addEventListener('abort', () => { - reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); - }); - }), - ), - ); + try { + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + vi.stubGlobal( + 'fetch', + vi.fn((_url, init) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(Object.assign(new Error('aborted'), { name: 'AbortError' })); + }); + }), + ), + ); - const promise = initToolTypesManifest(); - await vi.advanceTimersByTimeAsync(5000); - await promise; + const promise = initToolTypesManifest(); + await vi.advanceTimersByTimeAsync(5000); + await promise; - expect(getManifestToolTypes()).toBeNull(); - expect(warn).toHaveBeenCalledWith( - '[tool registry] Could not load /static/tool_types.json: timed out after 5000ms', - ); - vi.useRealTimers(); + expect(getManifestToolTypes()).toBeNull(); + expect(warn).toHaveBeenCalledWith( + '[tool registry] Could not load /static/tool_types.json: timed out after 5000ms', + ); + } finally { + vi.useRealTimers(); + } }); }); diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 1245e48..2eadec3 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/tests/test_tool_dispatch_sync.py @@ -96,9 +96,7 @@ def _load_manifest_tool_types(path: Path) -> frozenset[str]: raise ValueError(msg) for i, item in enumerate(raw): if not isinstance(item, str): - msg = ( - f"Invalid tool_types[{i}] in {path}: expected string, got {type(item).__name__}" - ) + msg = f"Invalid tool_types[{i}] in {path}: expected string, got {type(item).__name__}" raise ValueError(msg) return frozenset(raw) From e217de2227e3adb7ad50ecbf55929b338996f9e2 Mon Sep 17 00:00:00 2001 From: chen Date: Wed, 1 Jul 2026 07:13:06 +0800 Subject: [PATCH 4/4] fix(frontend): drop unused tool_types_state scaffolding --- docs/architecture.md | 2 +- static/js/render/tool_types_manifest.js | 2 -- static/js/render/tool_types_manifest.test.js | 9 +-------- static/js/render/tool_types_state.js | 15 --------------- tests/test_tool_dispatch_sync.py | 5 +---- 5 files changed, 3 insertions(+), 30 deletions(-) delete mode 100644 static/js/render/tool_types_state.js diff --git a/docs/architecture.md b/docs/architecture.md index 5fccda5..b2ea3bb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -106,7 +106,7 @@ The UI is a **hash-routed** SPA with ES modules under `static/js/`: - `app.js` — routing and boot - `projects.js`, `sessions.js`, `search.js`, `export.js` — route handlers - `render/registry.js` — **tool dispatch registry** for session UI: `TOOL_USE_RENDERERS` and `TOOL_RESULT_RENDERERS` map tool name / `result_type` → render function (one module per type under `render/tool_use/` and `render/tool_result/`). Parallels backend `utils/tool_dispatch.py` (backend uses ordered predicates; frontend uses direct key lookup + fallback). -- `static/tool_types.json` — generated manifest of backend tool-use names (`python scripts/gen_tool_types_manifest.py` from `KNOWN_TOOL_TYPES`). Loaded at boot by `render/tool_types_manifest.js`, which cross-checks `TOOL_USE_RENDERERS` and logs `console.warn` on drift. +- `static/tool_types.json` — generated manifest of backend tool-use names (`python scripts/gen_tool_types_manifest.py` from `KNOWN_TOOL_TYPES`). Fetched non-blocking at boot by `render/tool_types_manifest.js` (`void initToolTypesManifest()` in `app.js`), which cross-checks `TOOL_USE_RENDERERS` and logs `console.warn` on drift. - `shared/markdown.js` — markdown + **DOMPurify** sanitization (do not render raw LLM HTML) - `shared/state.js`, `shared/utils.js`, `shared/theme.js` — shared UI state and helpers diff --git a/static/js/render/tool_types_manifest.js b/static/js/render/tool_types_manifest.js index e5cb5b3..6e2b3fd 100644 --- a/static/js/render/tool_types_manifest.js +++ b/static/js/render/tool_types_manifest.js @@ -1,5 +1,4 @@ import { TOOL_USE_RENDERERS } from './registry.js'; -import { setManifestToolTypes } from './tool_types_state.js'; const MANIFEST_URL = '/static/tool_types.json'; const MANIFEST_FETCH_TIMEOUT_MS = 5000; @@ -20,7 +19,6 @@ export async function initToolTypesManifest() { const data = await res.json(); const types = Array.isArray(data.tool_types) ? data.tool_types : []; const manifest = new Set(types.filter((t) => typeof t === 'string')); - setManifestToolTypes(manifest); for (const name of manifest) { if (!Object.prototype.hasOwnProperty.call(TOOL_USE_RENDERERS, name)) { diff --git a/static/js/render/tool_types_manifest.test.js b/static/js/render/tool_types_manifest.test.js index ade5ffb..a94b0d1 100644 --- a/static/js/render/tool_types_manifest.test.js +++ b/static/js/render/tool_types_manifest.test.js @@ -1,18 +1,14 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { initToolTypesManifest } from './tool_types_manifest.js'; -import { getManifestToolTypes, setManifestToolTypes } from './tool_types_state.js'; import { TOOL_USE_RENDERERS } from './registry.js'; -// Registry drift warnings are asserted here (init-time cross-check only; no per-render warn). - describe('initToolTypesManifest', () => { beforeEach(() => { - setManifestToolTypes(null); vi.restoreAllMocks(); }); afterEach(() => { - setManifestToolTypes(null); + vi.unstubAllGlobals(); }); it('cross-checks manifest against TOOL_USE_RENDERERS and warns on drift', async () => { @@ -28,7 +24,6 @@ describe('initToolTypesManifest', () => { await initToolTypesManifest(); - expect(getManifestToolTypes()).toEqual(new Set(manifestTypes)); expect(warn).toHaveBeenCalledWith( '[tool registry] Backend tool type "FutureToolXYZ" has no TOOL_USE_RENDERERS entry', ); @@ -40,7 +35,6 @@ describe('initToolTypesManifest', () => { await initToolTypesManifest(); - expect(getManifestToolTypes()).toBeNull(); expect(warn).toHaveBeenCalledWith( '[tool registry] Could not load tool types manifest:', expect.any(Error), @@ -66,7 +60,6 @@ describe('initToolTypesManifest', () => { await vi.advanceTimersByTimeAsync(5000); await promise; - expect(getManifestToolTypes()).toBeNull(); expect(warn).toHaveBeenCalledWith( '[tool registry] Could not load /static/tool_types.json: timed out after 5000ms', ); diff --git a/static/js/render/tool_types_state.js b/static/js/render/tool_types_state.js deleted file mode 100644 index 5f27fd5..0000000 --- a/static/js/render/tool_types_state.js +++ /dev/null @@ -1,15 +0,0 @@ -/** Backend tool-use names from ``static/tool_types.json`` (set at SPA init). */ - -let manifestToolTypes = null; - -export function setManifestToolTypes(types) { - manifestToolTypes = types; -} - -export function getManifestToolTypes() { - return manifestToolTypes; -} - -export function isManifestToolType(name) { - return manifestToolTypes !== null && manifestToolTypes.has(name); -} diff --git a/tests/test_tool_dispatch_sync.py b/tests/test_tool_dispatch_sync.py index 2eadec3..004e79e 100644 --- a/tests/test_tool_dispatch_sync.py +++ b/tests/test_tool_dispatch_sync.py @@ -137,17 +137,14 @@ def test_tool_name_literal_matches_known_tool_types() -> None: def test_frontend_registry_matches_known_tool_types() -> None: - """``TOOL_USE_RENDERERS`` keys must match ``KNOWN_TOOL_TYPES`` and the manifest.""" + """``TOOL_USE_RENDERERS`` keys must match ``KNOWN_TOOL_TYPES``.""" site = "static/js/render/registry.js (TOOL_USE_RENDERERS)" try: actual = _parse_frontend_tool_use_renderers(_FRONTEND_REGISTRY) - manifest = _load_manifest_tool_types(_TOOL_TYPES_MANIFEST) except ValueError as exc: pytest.fail(f"{site}: {exc}") if actual != KNOWN_TOOL_TYPES: pytest.fail(_format_set_diff(KNOWN_TOOL_TYPES, actual, site)) - if actual != manifest: - pytest.fail(_format_set_diff(manifest, actual, site)) def test_known_tool_types_nonempty() -> None: