Implement SkillStudio app with local storage safety and Vercel AI SDK v5 - #1
Conversation
…safety and Vercel AI SDK v5
Code Review by Qodo
1. skill-card-grid uses gradients
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| /* Custom grid background for signature cards */ | ||
| .skill-card-grid { | ||
| background-image: | ||
| linear-gradient(rgba(99, 102, 241, 0.06) 1px, transparent 1px), | ||
| linear-gradient(90deg, rgba(99, 102, 241, 0.06) 1px, transparent 1px); | ||
| background-size: 24px 24px; |
There was a problem hiding this comment.
1. skill-card-grid uses gradients 📘 Rule violation ⚙ Maintainability
The .skill-card-grid background is built with linear-gradient(...), which violates the design-system rule forbidding any CSS gradient usage. This can cause repeated visual regressions against the strict SkillStudio styling constraints.
Agent Prompt
## Issue description
`app/globals.css` uses `linear-gradient(...)` to render the signature card grid, but the design system forbids any CSS gradients.
## Issue Context
The grid effect can be implemented without CSS gradients (e.g., using an SVG background image or a data-URI) while preserving the required `#6366F1` @ 6% opacity and `24px 24px` sizing.
## Fix Focus Areas
- app/globals.css[29-35]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| <Button | ||
| variant="secondary" | ||
| onClick={handleClear} | ||
| className="flex items-center gap-2 text-rose-600 border-rose-200 hover:bg-rose-50 hover:text-rose-700" | ||
| > |
There was a problem hiding this comment.
2. clear keys uses rose accent 📘 Rule violation ⚙ Maintainability
The Clear Keys button introduces rose colors (text-rose-600, border-rose-200, hover:bg-rose-50), which violates the rule that #6366F1 is the single visual accent color. This breaks the strict SkillStudio design system constraints.
Agent Prompt
## Issue description
A non-indigo accent color (`rose-*`) is used for the destructive `Clear Keys` button.
## Issue Context
The design system requires `#6366F1` as the single visual accent (with `#22C55E` reserved only for positive/ready indicators). Destructive actions should still be represented using approved neutrals/indigo styling patterns.
## Fix Focus Areas
- app/(app)/settings/page.tsx[98-109]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const model = getModelInstance(provider as ModelProvider, apiKey, modelName) as any | ||
|
|
||
| // Call streamText with automatic tool execution (up to 5 steps) | ||
| const result = streamText({ | ||
| model, | ||
| system: SYSTEM_PROMPT, | ||
| messages, | ||
| tools: agentTools, | ||
| maxSteps: 5, | ||
| } as any) | ||
|
|
||
| return (result as any).toDataStreamResponse() | ||
| } catch (error: any) { |
There was a problem hiding this comment.
3. Chat route uses any casts 📘 Rule violation ⚙ Maintainability
app/api/chat/route.ts introduces multiple any casts and an error: any catch parameter without any justification comment. This erases type safety and violates the strict TypeScript compliance requirement.
Agent Prompt
## Issue description
The API route uses `as any` and `catch (error: any)` without documented necessity.
## Issue Context
Compliance requires avoiding `any` unless unavoidable and explicitly documented. This route can typically be typed by:
- Validating/parsing the JSON body (e.g., with Zod) into a concrete type
- Using `unknown` in catch and narrowing
- Avoiding `as any` around `streamText` options and the returned result
## Fix Focus Areas
- app/api/chat/route.ts[8-41]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // Parse conversation to extract skills and documents | ||
| let skillName = '' | ||
| let skillSlug = '' | ||
| let skillPlatform: any = 'all' | ||
| let skillMd = '' | ||
| const extraFiles: { path: string; content: string }[] = [] | ||
| let isReadyToPackage = false | ||
|
|
||
| messages.forEach((message: any) => { | ||
| if (message.toolInvocations) { | ||
| message.toolInvocations.forEach((tool: any) => { | ||
| const { toolName, args } = tool | ||
|
|
||
| if (toolName === 'generateSkillMd') { | ||
| const { name, slug, platform, content } = args as any | ||
| if (name) skillName = name | ||
| if (slug) skillSlug = slug | ||
| if (platform) skillPlatform = platform | ||
| if (content) skillMd = content | ||
| } | ||
|
|
||
| if (toolName === 'generateScript') { | ||
| const { path, content } = args as any | ||
| if (path && content) { | ||
| const idx = extraFiles.findIndex(f => f.path === path) | ||
| if (idx >= 0) { | ||
| extraFiles[idx].content = content | ||
| } else { | ||
| extraFiles.push({ path, content }) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (toolName === 'packageSkill') { | ||
| const { ready } = args as any | ||
| if (ready) { | ||
| isReadyToPackage = true | ||
| } | ||
| } | ||
| }) | ||
| } | ||
| }) | ||
|
|
||
| // Auto-save generated skill into local list when packaged successfully | ||
| useEffect(() => { | ||
| if (isReadyToPackage && skillName && skillMd) { | ||
| const saved = localStorage.getItem('ss_skills') | ||
| let list: any[] = [] |
There was a problem hiding this comment.
4. Chatinterface uses any tool parsing 📘 Rule violation ⚙ Maintainability
components/chat/chat-interface.tsx introduces any types for skillPlatform, messages, tool invocation objects, and tool args without justification comments. This defeats strict typing in a core UI flow and violates the compliance requirement.
Agent Prompt
## Issue description
Tool invocation parsing in `ChatInterface` relies on `any` for message/tool/args/list types.
## Issue Context
The tool shapes are already described by Zod schemas in `lib/agent/tools.ts`. Use those schemas to derive types (e.g., `z.infer<...>`) and type the tool invocations/messages accordingly, removing `any` casts.
## Fix Focus Areas
- components/chat/chat-interface.tsx[74-151]
- lib/agent/tools.ts[3-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| return new Response(new Blob([zipBuffer as any]), { | ||
| status: 200, | ||
| headers: { | ||
| 'Content-Type': 'application/zip', | ||
| 'Content-Disposition': `attachment; filename="${slug}.skill.zip"`, | ||
| 'Cache-Control': 'no-store, max-age=0', | ||
| }, | ||
| }) | ||
| } catch (error: any) { | ||
| console.error('Error in API Export Route:', error) |
There was a problem hiding this comment.
7. Export route uses any 📘 Rule violation ⚙ Maintainability
app/api/export/route.ts introduces both zipBuffer as any and catch (error: any) without any justification comment. This violates the strict TypeScript compliance requirement and weakens type safety for a core export API.
Agent Prompt
## Issue description
The export route uses `zipBuffer as any` and `catch (error: any)`.
## Issue Context
You can return the `Uint8Array` directly in the `Response` body (no `Blob` cast needed) and use `unknown` in the catch block with narrowing, preserving strict typing.
## Fix Focus Areas
- app/api/export/route.ts[16-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const getModelInstance = (provider: ModelProvider, apiKey: string, modelName?: string) => { | ||
| if (provider === 'gemini') { | ||
| const googleInstance = createGoogle({ | ||
| apiKey: apiKey || process.env.GEMINI_API_KEY || '', | ||
| }) | ||
| // Gemini 2.0 Flash is the default development model | ||
| return googleInstance(modelName || 'gemini-2.0-flash') | ||
| } else if (provider === 'claude') { | ||
| const anthropicInstance = createAnthropic({ | ||
| apiKey: apiKey || process.env.ANTHROPIC_API_KEY || '', | ||
| }) | ||
| return anthropicInstance(modelName || 'claude-3-5-sonnet-20241022') | ||
| } else if (provider === 'gpt') { | ||
| const openaiInstance = createOpenAI({ | ||
| apiKey: apiKey || process.env.OPENAI_API_KEY || '', | ||
| }) | ||
| return openaiInstance(modelName || 'gpt-4o-mini') |
There was a problem hiding this comment.
8. Server key fallback abuse 🐞 Bug ⛨ Security
/api/chat accepts an empty x-api-key and getModelInstance falls back to server environment API keys, allowing unauthenticated callers to spend server-owned LLM credits. Unsupported/invalid provider values also currently bubble into a thrown error path rather than a clean 4xx validation response.
Agent Prompt
### Issue description
`getModelInstance()` falls back to `process.env.*` keys when the request does not provide `x-api-key`, and `/api/chat` does not enforce that header. This makes the endpoint abusable to drain server/provider quotas.
### Issue Context
The UI stores keys in localStorage and forwards them via `x-api-key`, so server-side env fallback is not necessary for the intended design.
### Fix Focus Areas
- Validate and require `x-api-key` for all providers (return 400/401 when missing).
- Remove (or strictly gate) `process.env.*` API key fallback in `getModelInstance`.
- Validate `provider` to return 400 on unsupported values (avoid throwing into 500).
### Fix Focus Areas (code pointers)
- app/api/chat/route.ts[10-35]
- lib/models/providers.ts[7-25]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| extraFiles.forEach((file) => { | ||
| // Normalize path separators to forward slashes for zip | ||
| const normalizedPath = file.path.replace(/\\/g, '/') | ||
| skillFolder.file(normalizedPath, file.content) | ||
| }) |
There was a problem hiding this comment.
9. Unsafe zip entry paths 🐞 Bug ⛨ Security
/api/export passes caller-controlled files[].path directly into JSZip entry names without
sanitizing .. segments or absolute paths, creating archives that can perform Zip-Slip style
overwrites when extracted with a vulnerable/unhardened unzipper. The endpoint also does not validate
that files is an array of {path, content} strings, so malformed inputs can crash packaging
(500).
Agent Prompt
### Issue description
The export pipeline writes arbitrary `files[].path` into ZIP entries after only normalizing path separators. Paths like `../.ssh/config` or `/etc/passwd` (or Windows drive paths) can end up as unsafe archive entry names.
### Issue Context
- `app/api/export` accepts `files` from the request body.
- `lib/packager` writes those paths verbatim into the archive.
### Fix Focus Areas
- Validate request body shape (`skillName`, `skillMd`, and `files` array) before packaging.
- Reject absolute paths and any path containing `..` segments after normalization.
- Consider enforcing an allowlist of top-level directories (e.g., `scripts/`, `references/`, `assets/`, `templates/`).
- Optionally normalize to a safe prefix (e.g., always write under `generated/` inside the skill folder).
### Fix Focus Areas (code pointers)
- app/api/export/route.ts[6-22]
- lib/packager.ts[18-37]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| export const createClient = async () => { | ||
| const cookieStore = await cookies() | ||
| const supabaseUrl = process.env.NEXT_PUBLIC_SUPABASE_URL || 'https://placeholder-url.supabase.co' | ||
| const supabaseAnonKey = process.env.NEXT_PUBLIC_SUPABASE_ANON_KEY || 'placeholder-anon-key' | ||
|
|
There was a problem hiding this comment.
10. Supabase placeholder fallbacks 🐞 Bug ☼ Reliability
Supabase client/server helpers silently fall back to hard-coded placeholder URL/key values when env vars are missing, which hides configuration errors and can lead to confusing runtime behavior (requests directed at an invalid host/key). This should fail fast or be explicitly mocked per environment instead of silently defaulting.
Agent Prompt
### Issue description
`createClient()` for Supabase uses placeholder values when `NEXT_PUBLIC_SUPABASE_URL` / `NEXT_PUBLIC_SUPABASE_ANON_KEY` are missing. This masks misconfiguration and can cause non-obvious failures.
### Issue Context
Both browser and server Supabase clients use the same placeholder fallback behavior.
### Fix Focus Areas
- Replace placeholder fallbacks with explicit runtime checks and throw a clear error when env vars are missing.
- If local mock behavior is desired, gate it behind `NODE_ENV !== 'production'` and an explicit `SUPABASE_MOCK=true` flag.
### Fix Focus Areas (code pointers)
- lib/supabase/server.ts[4-8]
- lib/supabase/client.ts[3-7]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
PR Summary by QodoImplement SkillStudio app with BYOK local key storage and AI SDK v5 streaming
AI Description
Diagram
High-Level Assessment
Files changed (160)
|
This pull request adds comprehensive reference documentation for accessibility and Bun usage, as well as a WCAG 2.2 quick reference guide. These additions provide practical code patterns, standards checklists, and decision guidance to support accessible and efficient JavaScript/TypeScript development.
Accessibility Reference Additions:
.agents/skills/accessibility/references/A11Y-PATTERNS.mdto serve as ready-to-use examples for developers..agents/skills/accessibility/references/WCAG.md, summarizing success criteria by level, ARIA patterns, recent changes from WCAG 2.1 to 2.2, and recommended accessibility testing tools.Bun Reference Addition:
.agents/skills/bun/SKILL.md, covering product overview, essential commands, configuration, workflow steps, common pitfalls, and a verification checklist to streamline Bun adoption for JavaScript/TypeScript projects.…safety and Vercel AI SDK v5