mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-09 03:17:54 +00:00
fix(wiki): added testcase and corrected self-review points
- Export safeJSON as a named function with JSDoc for testability - Add console.warn when max_tokens -> max_completion_tokens auto-switch triggers, consistent with existing Azure warning pattern - Prevent auto-switch from consuming a retry attempt (attempt--) - Add unit tests for safeJSON escaping (XSS vector, round-trip, order) - Add unit tests for max_tokens auto-switch (happy path, guard clauses)
This commit is contained in:
parent
ae93a902de
commit
505228b851
4 changed files with 236 additions and 10 deletions
|
|
@ -61,19 +61,31 @@ function esc(text: string): string {
|
|||
.replace(/"/g, '"');
|
||||
}
|
||||
|
||||
/**
|
||||
* Serialize a value to JSON with HTML-significant characters Unicode-escaped.
|
||||
* `<`, `>`, and `&` are replaced with `\u003c`, `\u003e`, `\u0026` so the
|
||||
* result is safe to embed inside a `<script>` tag without the HTML parser
|
||||
* mis-interpreting the content (e.g. a literal `</script>` in a string).
|
||||
*
|
||||
* Replacement order matters: `<` and `>` are replaced first because their
|
||||
* replacements (`\u003c`, `\u003e`) contain no `&`, so the final `&` pass
|
||||
* cannot double-escape them.
|
||||
*/
|
||||
export function safeJSON(v: unknown): string {
|
||||
const s = JSON.stringify(v);
|
||||
if (s === undefined) return 'null';
|
||||
return s
|
||||
.replace(/</g, '\\u003c')
|
||||
.replace(/>/g, '\\u003e')
|
||||
.replace(/&/g, '\\u0026');
|
||||
}
|
||||
|
||||
function buildHTML(
|
||||
projectName: string,
|
||||
moduleTree: ModuleTreeNode[],
|
||||
pages: Record<string, string>,
|
||||
meta: Record<string, unknown> | null,
|
||||
): string {
|
||||
// Embed data as JSON inside the HTML
|
||||
// Unicode-escape HTML-significant chars so the HTML parser can't misinterpret them
|
||||
const safeJSON = (v: unknown) =>
|
||||
JSON.stringify(v)
|
||||
.replace(/</g, '\\u003c')
|
||||
.replace(/>/g, '\\u003e')
|
||||
.replace(/&/g, '\\u0026');
|
||||
const pagesJSON = safeJSON(pages);
|
||||
const treeJSON = safeJSON(moduleTree);
|
||||
const metaJSON = safeJSON(meta);
|
||||
|
|
@ -86,7 +98,7 @@ function buildHTML(
|
|||
parts.push('<head>');
|
||||
parts.push('<meta charset="UTF-8">');
|
||||
parts.push('<meta name="viewport" content="width=device-width, initial-scale=1.0">');
|
||||
parts.push('<meta http-equiv="Content-Security-Policy" content="default-src \'none\'; script-src \'unsafe-inline\' https://cdn.jsdelivr.net; style-src \'unsafe-inline\'; img-src data: https:;">');
|
||||
parts.push('<meta http-equiv="Content-Security-Policy" content="default-src \'none\'; script-src \'unsafe-inline\' \'unsafe-eval\' https://cdn.jsdelivr.net; style-src \'unsafe-inline\'; img-src data: https:;">');
|
||||
parts.push('<title>' + esc(projectName) + ' — Wiki</title>');
|
||||
parts.push('<script src="https://cdn.jsdelivr.net/npm/marked@11.0.0/marked.min.js"><\/script>');
|
||||
parts.push(
|
||||
|
|
|
|||
|
|
@ -169,6 +169,7 @@ export async function callLLM(
|
|||
|
||||
const MAX_RETRIES = 3;
|
||||
let lastError: Error | null = null;
|
||||
let switchedTokenParam = false;
|
||||
|
||||
for (let attempt = 0; attempt < MAX_RETRIES; attempt++) {
|
||||
try {
|
||||
|
|
@ -212,13 +213,18 @@ export async function callLLM(
|
|||
|
||||
// Auto-switch max_tokens → max_completion_tokens when the model rejects max_tokens
|
||||
if (
|
||||
!switchedTokenParam &&
|
||||
response.status === 400 &&
|
||||
errorText.includes('max_completion_tokens') &&
|
||||
(errorText.includes("'max_tokens'") || errorText.includes('"max_tokens"')) &&
|
||||
body.max_tokens !== undefined
|
||||
) {
|
||||
console.warn(
|
||||
'[gitnexus] Warning: model rejected max_tokens; retrying with max_completion_tokens.',
|
||||
);
|
||||
switchedTokenParam = true;
|
||||
body.max_completion_tokens = body.max_tokens;
|
||||
delete body.max_tokens;
|
||||
// Retry immediately with the corrected parameter
|
||||
attempt--;
|
||||
continue;
|
||||
}
|
||||
|
||||
|
|
|
|||
70
gitnexus/test/unit/wiki-html-viewer.test.ts
Normal file
70
gitnexus/test/unit/wiki-html-viewer.test.ts
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
import { describe, it, expect } from 'vitest';
|
||||
import fs from 'fs/promises';
|
||||
import os from 'os';
|
||||
import path from 'path';
|
||||
import { safeJSON, generateHTMLViewer } from '../../src/core/wiki/html-viewer.js';
|
||||
|
||||
describe('safeJSON', () => {
|
||||
it('escapes < to \\u003c', () => {
|
||||
expect(safeJSON('a < b')).toContain('\\u003c');
|
||||
expect(safeJSON('a < b')).not.toContain('<');
|
||||
});
|
||||
|
||||
it('escapes > to \\u003e', () => {
|
||||
expect(safeJSON('a > b')).toContain('\\u003e');
|
||||
expect(safeJSON('a > b')).not.toContain('>');
|
||||
});
|
||||
|
||||
it('escapes & to \\u0026', () => {
|
||||
expect(safeJSON('a & b')).toContain('\\u0026');
|
||||
expect(safeJSON('a & b')).not.toContain('&');
|
||||
});
|
||||
|
||||
it('escapes </script> — the primary XSS vector', () => {
|
||||
const result = safeJSON({ content: '</script><img onerror=alert(1)>' });
|
||||
expect(result).not.toContain('</script>');
|
||||
expect(result).not.toContain('<');
|
||||
expect(result).not.toContain('>');
|
||||
});
|
||||
|
||||
it('round-trips through JSON.parse to the original value', () => {
|
||||
const values = [
|
||||
'hello',
|
||||
'<script>alert(1)</script>',
|
||||
{ key: 'a < b & c > d' },
|
||||
['</script>', '&', '<div>'],
|
||||
null,
|
||||
42,
|
||||
];
|
||||
for (const v of values) {
|
||||
expect(JSON.parse(safeJSON(v))).toEqual(v);
|
||||
}
|
||||
});
|
||||
|
||||
it('does not double-escape — replacement order is safe', () => {
|
||||
const result = safeJSON('<');
|
||||
const parsed = JSON.parse(result);
|
||||
expect(parsed).toBe('<');
|
||||
});
|
||||
|
||||
it('returns "null" for non-serializable values like undefined', () => {
|
||||
expect(safeJSON(undefined)).toBe('null');
|
||||
expect(safeJSON(() => {})).toBe('null');
|
||||
});
|
||||
});
|
||||
|
||||
describe('generateHTMLViewer — CSP meta tag', () => {
|
||||
it('includes a Content-Security-Policy meta tag in generated HTML', async () => {
|
||||
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'wiki-csp-'));
|
||||
try {
|
||||
await fs.writeFile(path.join(tmpDir, 'overview.md'), '# Hello');
|
||||
await fs.writeFile(path.join(tmpDir, 'module_tree.json'), '[]');
|
||||
const outputPath = await generateHTMLViewer(tmpDir, 'TestProject');
|
||||
const html = await fs.readFile(outputPath, 'utf-8');
|
||||
expect(html).toContain('Content-Security-Policy');
|
||||
expect(html).toContain("'unsafe-eval'");
|
||||
} finally {
|
||||
await fs.rm(tmpDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
@ -285,6 +285,144 @@ describe('callLLM — Azure content_filter error', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('callLLM — max_tokens auto-switch', () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it('retries with max_completion_tokens when model rejects max_tokens', async () => {
|
||||
const fetchSpy = vi
|
||||
.fn()
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
error: {
|
||||
message: "Unsupported parameter: 'max_tokens'. Use 'max_completion_tokens' instead.",
|
||||
},
|
||||
}),
|
||||
{ status: 400, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
)
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({ choices: [{ message: { content: 'ok' } }], usage: {} }),
|
||||
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal('fetch', fetchSpy);
|
||||
|
||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
const { callLLM } = await import('../../src/core/wiki/llm-client.js');
|
||||
const result = await callLLM('test', {
|
||||
apiKey: 'sk-test',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'gpt-4.1',
|
||||
maxTokens: 1000,
|
||||
temperature: 0,
|
||||
});
|
||||
|
||||
expect(result.content).toBe('ok');
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
|
||||
// Second request should have max_completion_tokens, not max_tokens
|
||||
const secondBody = JSON.parse(fetchSpy.mock.calls[1][1].body as string);
|
||||
expect(secondBody.max_completion_tokens).toBe(1000);
|
||||
expect(secondBody.max_tokens).toBeUndefined();
|
||||
|
||||
expect(warnSpy).toHaveBeenCalledWith(
|
||||
expect.stringContaining('max_completion_tokens'),
|
||||
);
|
||||
});
|
||||
|
||||
it('does not trigger on unrelated 400 errors', async () => {
|
||||
const fetchSpy = vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({ error: { message: 'Invalid model: no-such-model' } }),
|
||||
{ status: 400, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal('fetch', fetchSpy);
|
||||
|
||||
const { callLLM } = await import('../../src/core/wiki/llm-client.js');
|
||||
await expect(
|
||||
callLLM('test', {
|
||||
apiKey: 'sk-test',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'no-such-model',
|
||||
maxTokens: 100,
|
||||
temperature: 0,
|
||||
}),
|
||||
).rejects.toThrow('LLM API error (400)');
|
||||
|
||||
// Should only have been called once — no retry
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not trigger when already using max_completion_tokens (reasoning model)', async () => {
|
||||
const fetchSpy = vi.fn().mockResolvedValue(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
error: { message: "Unsupported parameter: use 'max_completion_tokens'" },
|
||||
}),
|
||||
{ status: 400, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal('fetch', fetchSpy);
|
||||
|
||||
const { callLLM } = await import('../../src/core/wiki/llm-client.js');
|
||||
await expect(
|
||||
callLLM('test', {
|
||||
apiKey: 'sk-test',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'o3-mini',
|
||||
maxTokens: 100,
|
||||
temperature: 0,
|
||||
}),
|
||||
).rejects.toThrow('LLM API error (400)');
|
||||
|
||||
// Reasoning models already send max_completion_tokens — guard prevents switch
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not consume a retry attempt — full retries remain after switch', async () => {
|
||||
const fetchSpy = vi
|
||||
.fn()
|
||||
// 1st call: 400 triggers auto-switch
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({
|
||||
error: { message: "Unsupported parameter: 'max_tokens'. Use 'max_completion_tokens' instead." },
|
||||
}),
|
||||
{ status: 400, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
)
|
||||
// 2nd call: succeeds
|
||||
.mockResolvedValueOnce(
|
||||
new Response(
|
||||
JSON.stringify({ choices: [{ message: { content: 'done' } }], usage: {} }),
|
||||
{ status: 200, headers: { 'Content-Type': 'application/json' } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal('fetch', fetchSpy);
|
||||
vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
|
||||
const { callLLM } = await import('../../src/core/wiki/llm-client.js');
|
||||
const result = await callLLM('test', {
|
||||
apiKey: 'sk-test',
|
||||
baseUrl: 'https://api.openai.com/v1',
|
||||
model: 'gpt-4.1',
|
||||
maxTokens: 500,
|
||||
temperature: 0,
|
||||
});
|
||||
|
||||
expect(result.content).toBe('done');
|
||||
// Only 2 fetch calls: the rejected one + the successful retry
|
||||
expect(fetchSpy).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
});
|
||||
|
||||
describe('readSSEStream — content_filter handling', () => {
|
||||
afterEach(() => vi.unstubAllGlobals());
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue