Skip to content

Commit e059863

Browse files
Shubh-RajShubh-Raj
committed
fix: address code quality issues across multiple files
- README.md: fix license badge URLs to reference template-playground - e2e/navigation.spec.ts: use Playwright auto-waiting toHaveURL matcher - AIConfigPopup.tsx: use debouncedApiKey in fetches, add isEncrypting try-finally in handleSave, validate maxTokens bounds, unify isSaveDisabled, surface save errors to user - store.ts: fix agreementHtml type mismatch (undefined -> empty string) - ErrorBoundary.test.tsx: wrap window.location mock in try-finally - secureKeyStorage.ts: fix salt.buffer for HKDF, fix PRF error message Signed-off-by: Shubh-Raj <shubhraj625@gmail.com>
1 parent 169b7e8 commit e059863

File tree

6 files changed

+115
-95
lines changed

6 files changed

+115
-95
lines changed

‎README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
<h1 align="center">Accord Project Template Playground</h1>
22
<p align="center">
3-
<a href="https://github.com/accordproject/models/blob/master/LICENSE"><img src="https://img.shields.io/github/license/accordproject/models" alt="GitHub license"></a>
3+
<a href="https://github.com/accordproject/template-playground/blob/main/LICENSE"><img src="https://img.shields.io/github/license/accordproject/template-playground" alt="GitHub license"></a>
44
<a href="https://github.com/accordproject/template-playground/actions/workflows/test.yml?query=branch%3Amain"><img src="https://img.shields.io/badge/coverage-view%20artifacts-blue" alt="Coverage: view artifacts"></a>
55
<a href="https://www.accordproject.org/">
66
<img src="https://img.shields.io/badge/powered%20by-accord%20project-19C6C8.svg" alt="Accord Project" />

‎e2e/navigation.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ test.describe('Navigation', () => {
2929
await homeLink.click();
3030

3131
// Should be back at the playground
32-
await expect(page).toHaveURL(/\/$/);
32+
await expect(page).toHaveURL(/^https?:\/\/[^/]+\/?$/);
3333
});
3434

3535
test('should have Help dropdown menu', async ({ page }) => {

‎src/components/AIConfigPopup.tsx‎

Lines changed: 94 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -141,15 +141,15 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
141141
switch (provider) {
142142
case 'openai':
143143
case 'openai-compatible':
144-
if (!apiKey) return;
144+
if (!debouncedApiKey) return;
145145

146146
let endpoint = provider === 'openai-compatible' ? customEndpoint : 'https://api.openai.com/v1';
147147
if (!endpoint) return;
148148
endpoint = endpoint.replace(/\/$/, '');
149149
const url = `${endpoint}/models`;
150150

151151
const res = await fetch(url, {
152-
headers: { Authorization: `Bearer ${apiKey}` },
152+
headers: { Authorization: `Bearer ${debouncedApiKey}` },
153153
signal,
154154
});
155155

@@ -163,11 +163,11 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
163163
break;
164164

165165
case 'anthropic':
166-
if (!apiKey) return;
166+
if (!debouncedApiKey) return;
167167
{
168168
const res = await fetch('https://api.anthropic.com/v1/models', {
169169
headers: {
170-
'x-api-key': apiKey,
170+
'x-api-key': debouncedApiKey,
171171
'anthropic-version': '2023-06-01',
172172
'content-type': 'application/json',
173173
},
@@ -183,10 +183,10 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
183183
break;
184184

185185
case 'google':
186-
if (!apiKey) return;
186+
if (!debouncedApiKey) return;
187187
{
188188
const res = await fetch('https://generativelanguage.googleapis.com/v1beta2/models', {
189-
headers: { 'x-goog-api-key': apiKey },
189+
headers: { 'x-goog-api-key': debouncedApiKey },
190190
signal,
191191
});
192192
if (!res.ok) {
@@ -199,10 +199,10 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
199199
break;
200200

201201
case 'mistral':
202-
if (!apiKey) return;
202+
if (!debouncedApiKey) return;
203203
{
204204
const res = await fetch('https://api.mistral.ai/v1/models', {
205-
headers: { Authorization: `Bearer ${apiKey}` },
205+
headers: { Authorization: `Bearer ${debouncedApiKey}` },
206206
signal,
207207
});
208208
if (!res.ok) {
@@ -227,10 +227,10 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
227227
break;
228228

229229
case 'openrouter':
230-
if (!apiKey) return;
230+
if (!debouncedApiKey) return;
231231
{
232232
const res = await fetch('https://openrouter.ai/api/v1/models', {
233-
headers: { Authorization: `Bearer ${apiKey}` },
233+
headers: { Authorization: `Bearer ${debouncedApiKey}` },
234234
signal,
235235
});
236236
if (!res.ok) {
@@ -261,71 +261,78 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
261261
}, [provider, debouncedApiKey, customEndpoint]);
262262

263263
const handleSave = async () => {
264-
localStorage.setItem('aiProvider', provider);
265-
localStorage.setItem('aiModel', model);
266-
267-
if (provider === 'openai-compatible') {
268-
localStorage.setItem('aiCustomEndpoint', customEndpoint);
269-
} else {
270-
localStorage.removeItem('aiCustomEndpoint');
271-
}
264+
setIsEncrypting(true);
265+
try {
266+
localStorage.setItem('aiProvider', provider);
267+
localStorage.setItem('aiModel', model);
268+
269+
if (provider === 'openai-compatible') {
270+
localStorage.setItem('aiCustomEndpoint', customEndpoint);
271+
} else {
272+
localStorage.removeItem('aiCustomEndpoint');
273+
}
272274

273-
if (maxTokens) {
274-
localStorage.setItem('aiResMaxTokens', maxTokens);
275-
} else {
276-
localStorage.removeItem('aiResMaxTokens');
277-
}
275+
if (maxTokens) {
276+
localStorage.setItem('aiResMaxTokens', maxTokens);
277+
} else {
278+
localStorage.removeItem('aiResMaxTokens');
279+
}
278280

279-
localStorage.setItem('aiShowFullPrompt', showFullPrompt.toString());
280-
localStorage.setItem('aiEnableCodeSelectionMenu', enableCodeSelectionMenu.toString());
281-
localStorage.setItem('aiEnableInlineSuggestions', enableInlineSuggestions.toString());
282-
283-
// Securely store the API key
284-
let protectionLevel: KeyProtectionLevel = 'memory-only';
285-
286-
if (apiKey && provider !== 'ollama') {
287-
if (webauthnAvailable) {
288-
try {
289-
const success = await encryptAndStoreApiKey(apiKey);
290-
if (success) {
291-
protectionLevel = 'webauthn';
292-
// Only remove legacy plaintext key when encryption succeeds
293-
localStorage.removeItem('aiApiKey');
281+
localStorage.setItem('aiShowFullPrompt', showFullPrompt.toString());
282+
localStorage.setItem('aiEnableCodeSelectionMenu', enableCodeSelectionMenu.toString());
283+
localStorage.setItem('aiEnableInlineSuggestions', enableInlineSuggestions.toString());
284+
285+
// Securely store the API key
286+
let protectionLevel: KeyProtectionLevel = 'memory-only';
287+
288+
if (apiKey && provider !== 'ollama') {
289+
if (webauthnAvailable) {
290+
try {
291+
const success = await encryptAndStoreApiKey(apiKey);
292+
if (success) {
293+
protectionLevel = 'webauthn';
294+
// Only remove legacy plaintext key when encryption succeeds
295+
localStorage.removeItem('aiApiKey');
296+
}
297+
} catch {
298+
// Encryption failed — fall through to memory-only
294299
}
295-
} catch {
296-
// Encryption failed — fall through to memory-only
297300
}
298301
}
299-
}
300302

301-
// Build the config from the current form values and set it directly
302-
// in the Zustand store. This avoids calling loadConfigFromLocalStorage()
303-
// which would re-trigger a WebAuthn prompt to decrypt the key we just saved.
304-
const config: AIConfig = {
305-
provider,
306-
model,
307-
apiKey: provider === 'ollama' ? '' : apiKey,
308-
includeTemplateMarkContent: localStorage.getItem('aiIncludeTemplateMark') === 'true',
309-
includeConcertoModelContent: localStorage.getItem('aiIncludeConcertoModel') === 'true',
310-
includeDataContent: localStorage.getItem('aiIncludeData') === 'true',
311-
showFullPrompt,
312-
enableCodeSelectionMenu,
313-
enableInlineSuggestions,
314-
};
303+
// Build the config from the current form values and set it directly
304+
// in the Zustand store. This avoids calling loadConfigFromLocalStorage()
305+
// which would re-trigger a WebAuthn prompt to decrypt the key we just saved.
306+
const config: AIConfig = {
307+
provider,
308+
model,
309+
apiKey: provider === 'ollama' ? '' : apiKey,
310+
includeTemplateMarkContent: localStorage.getItem('aiIncludeTemplateMark') === 'true',
311+
includeConcertoModelContent: localStorage.getItem('aiIncludeConcertoModel') === 'true',
312+
includeDataContent: localStorage.getItem('aiIncludeData') === 'true',
313+
showFullPrompt,
314+
enableCodeSelectionMenu,
315+
enableInlineSuggestions,
316+
};
317+
318+
if (provider === 'openai-compatible' && customEndpoint) {
319+
config.customEndpoint = customEndpoint;
320+
}
315321

316-
if (provider === 'openai-compatible' && customEndpoint) {
317-
config.customEndpoint = customEndpoint;
318-
}
322+
if (maxTokens) {
323+
const parsed = parseInt(maxTokens, 10);
324+
if (Number.isFinite(parsed) && parsed >= 1 && parsed <= 32000) {
325+
config.maxTokens = parsed;
326+
}
327+
}
319328

320-
if (maxTokens) {
321-
config.maxTokens = parseInt(maxTokens);
329+
const { setAIConfig } = useAppStore.getState();
330+
setAIConfig(config);
331+
setKeyProtectionLevel(protectionLevel);
332+
onClose();
333+
} finally {
334+
setIsEncrypting(false);
322335
}
323-
324-
const { setAIConfig } = useAppStore.getState();
325-
setAIConfig(config);
326-
setKeyProtectionLevel(protectionLevel);
327-
setIsEncrypting(false);
328-
onClose();
329336
};
330337

331338
const handleReset = () => {
@@ -612,16 +619,27 @@ const AIConfigPopup = ({ isOpen, onClose }: AIConfigPopupProps) => {
612619
)}
613620
</div>
614621

615-
<button
616-
onClick={() => { handleSave().catch(console.warn); }}
617-
disabled={isEncrypting || !provider || !model || (availableModels.length > 0 && !availableModels.includes(model)) || (provider !== 'ollama' && !apiKey) || (provider === 'openai-compatible' && !customEndpoint)}
618-
className={`w-full py-2 rounded-lg transition-colors disabled:cursor-not-allowed ${isEncrypting || !provider || !model || (provider !== 'ollama' && !apiKey) || (provider === 'openai-compatible' && !customEndpoint)
619-
? theme.saveButton.disabled
620-
: theme.saveButton.enabled
621-
}`}
622-
>
623-
{isEncrypting ? 'Encrypting & Saving...' : 'Save Configuration'}
624-
</button>
622+
{(() => {
623+
const isSaveDisabled = isEncrypting || !provider || !model || (availableModels.length > 0 && !availableModels.includes(model)) || (provider !== 'ollama' && !apiKey) || (provider === 'openai-compatible' && !customEndpoint);
624+
return (
625+
<button
626+
onClick={() => {
627+
setSecurityMessage('');
628+
handleSave().catch((err) => {
629+
console.warn(err);
630+
setSecurityMessage(`Save failed: ${err instanceof Error ? err.message : String(err)}`);
631+
});
632+
}}
633+
disabled={isSaveDisabled}
634+
className={`w-full py-2 rounded-lg transition-colors disabled:cursor-not-allowed ${isSaveDisabled
635+
? theme.saveButton.disabled
636+
: theme.saveButton.enabled
637+
}`}
638+
>
639+
{isEncrypting ? 'Encrypting & Saving...' : 'Save Configuration'}
640+
</button>
641+
);
642+
})()}
625643

626644
<button
627645
onClick={handleReset}

‎src/store/store.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,7 @@ const useAppStore = create<AppState>()(
243243
if (sample) {
244244
set(() => ({
245245
sampleName: sample.NAME,
246-
agreementHtml: undefined,
246+
agreementHtml: "",
247247
error: undefined,
248248
templateMarkdown: sample.TEMPLATE,
249249
editorValue: sample.TEMPLATE,

‎src/tests/components/ErrorBoundary.test.tsx‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -55,21 +55,23 @@ describe("ErrorBoundary", () => {
5555
delete (window as any).location;
5656
window.location = { ...originalLocation, reload: reloadMock } as any;
5757

58-
render(
59-
<ErrorBoundary>
60-
<ThrowError shouldThrow={true} />
61-
</ErrorBoundary>
62-
);
63-
64-
const reloadButton = screen.getByRole("button", { name: /Reload Page/i });
65-
expect(reloadButton).toBeInTheDocument();
58+
try {
59+
render(
60+
<ErrorBoundary>
61+
<ThrowError shouldThrow={true} />
62+
</ErrorBoundary>
63+
);
6664

67-
reloadButton.click();
68-
expect(reloadMock).toHaveBeenCalledTimes(1);
65+
const reloadButton = screen.getByRole("button", { name: /Reload Page/i });
66+
expect(reloadButton).toBeInTheDocument();
6967

70-
// Restore original location
71-
delete (window as any).location;
72-
window.location = originalLocation as any;
68+
reloadButton.click();
69+
expect(reloadMock).toHaveBeenCalledTimes(1);
70+
} finally {
71+
// Restore original location
72+
delete (window as any).location;
73+
window.location = originalLocation as any;
74+
}
7375
});
7476

7577
it("should display error details in development mode", () => {

‎src/utils/secureKeyStorage.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,8 @@ export async function registerCredential(): Promise<string> {
110110
const prfResult = (credential.getClientExtensionResults() as Record<string, unknown>)?.prf as { enabled?: boolean } | undefined;
111111
if (!prfResult?.enabled) {
112112
throw new Error(
113-
'Your authenticator does not support the PRF extension. ' +
114-
'API key will be stored in memory only for this session.'
113+
'Authenticator does not support PRF extension; operation aborted — ' +
114+
'no API key was stored; caller must handle this exception.'
115115
);
116116
}
117117

@@ -184,7 +184,7 @@ export async function deriveEncryptionKey(
184184
{
185185
name: 'HKDF',
186186
hash: 'SHA-256',
187-
salt: salt.buffer as ArrayBuffer,
187+
salt: new Uint8Array(salt).buffer as ArrayBuffer,
188188
info: HKDF_INFO,
189189
},
190190
hkdfKey,

0 commit comments

Comments
 (0)