feat: Add app metadata endpoint and fix instance naming #714
Conversation
- Create GET /api/apps/:slug endpoint for fetching app metadata - Returns public metadata only (title, remixOf, hasScreenshot, hasIcon) - Update vibe-viewer to fetch real app title instead of "Fresh Data" - Make sessionId optional in SessionSidebarProps - Remove hardcoded sessionId="" from BrutalistLayout Benefits: - Instances get actual app names instead of "Fresh Data" - Uses public slug endpoint (doesn't leak chatId) - SessionSidebar more flexible for non-chat pages - Fallback to slug if metadata fetch fails 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
✅ Deploy Preview for fireproof-ai-builder ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🎉 Hosting Preview DeploymentPreview URL: https://pr-714-vibes-hosting-v2.jchris.workers.dev Test the preview:
This preview will be automatically updated on each push to this PR |
![charliecreates[bot]](https://avatars.githubusercontent.com/in/1140780?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new AppGet endpoint is generally well-structured but parses KV data with JSON.parse without validation or error handling, which can lead to 500s and contract drift if stored data is malformed. The public metadata response would be more robust with nullish coalescing and/or a dedicated metadata schema stored in KV. In vibe-viewer, the asynchronous fetchAndCreateInstance lacks a cancellation/cleanup mechanism, which can introduce stale updates if dependencies change while a request is in flight, and the hardcoded preview API fallback is dangerous if VITE_API_BASE_URL is missing in production. The SessionSidebar prop changes and layout adjustments look clean and aligned with the described behavior.
Summary of changes
Summary of Changes
- Added a new
AppGetOpenAPI endpoint (GET /api/apps/:slug) that fetches app metadata from KV and returns a public-safe subset (slug, title, remixOf, hasScreenshot, hasIcon). - Registered the new endpoint in
hosting/pkg/src/index.tsand re-exportedAppGetfor testing. - Updated
SessionSidebarPropsto makesessionIdoptional and removed the hardcoded emptysessionIdprop fromBrutalistLayout. - Enhanced
vibe-viewerroute to lazily create instances using the real app title fetched from the new hosting API, with a slug fallback and error handling.
| const app = JSON.parse(appData); | ||
|
|
||
| // Return only public metadata - no code, userId, or chatId | ||
| return c.json({ | ||
| success: true, | ||
| app: { | ||
| slug: app.slug, | ||
| title: app.title || slug, | ||
| remixOf: app.remixOf || null, | ||
| hasScreenshot: app.hasScreenshot || false, | ||
| hasIcon: app.hasIcon || false, | ||
| }, |
There was a problem hiding this comment.
kv.get(slug) returns a string that is parsed directly with JSON.parse without any validation or defensive handling. If the KV entry is malformed JSON or not in the expected shape, this will throw, returning a 500 instead of a controlled 4xx/5xx response and bypassing your AppMetadataSchema. This is an example of unsafe but type-valid JSON parsing the instructions warn about.
Since this is a public-facing endpoint, consider validating the parsed value against AppMetadataSchema (or a backend version of it) and handling parse/validation errors gracefully so the API contract stays stable and you don’t accidentally expose unexpected fields if the stored schema changes.
Suggestion
You could wrap the JSON parse in a try/catch and validate with Zod before responding, e.g.:
let appJson: unknown;
try {
appJson = JSON.parse(appData);
} catch (err) {
console.error("Failed to parse app data from KV", { slug, err });
return c.json({ error: "App data corrupted" }, 500);
}
const parseResult = AppMetadataSchema.safeParse(appJson);
if (!parseResult.success) {
console.error("App data failed validation", { slug, issues: parseResult.error.issues });
return c.json({ error: "App data invalid" }, 500);
}
const app = parseResult.data;
return c.json({
success: true,
app: {
slug: app.slug,
title: app.title || slug,
remixOf: app.remixOf ?? null,
hasScreenshot: app.hasScreenshot ?? false,
hasIcon: app.hasIcon ?? false,
},
});This keeps the public response shape aligned with AppMetadataSchema and prevents runtime crashes due to bad KV contents. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const fetchAndCreateInstance = async () => { | ||
| try { | ||
| // Fetch app metadata from hosting API | ||
| const apiBaseUrl = | ||
| import.meta.env.VITE_API_BASE_URL || | ||
| "https://vibes-hosting-v2-preview.jchris.workers.dev"; | ||
| const response = await fetch(`${apiBaseUrl}/api/apps/${titleId}`); | ||
|
|
||
| let title = titleId; // Fallback to slug if fetch fails | ||
| if (response.ok) { | ||
| const data = (await response.json()) as { | ||
| app?: { title?: string }; | ||
| }; | ||
| title = data.app?.title || titleId; | ||
| } | ||
|
|
||
| // Create instance with real title | ||
| await createInstance(title, {}, installId); | ||
| } catch (error) { | ||
| // If fetch fails, use slug as title | ||
| console.warn("Failed to fetch app metadata, using slug:", error); | ||
| await createInstance(titleId, {}, installId); | ||
| } | ||
| }; | ||
|
|
||
| fetchAndCreateInstance(); |
There was a problem hiding this comment.
fetchAndCreateInstance is an async function invoked and then intentionally not awaited inside useEffect. That’s fine for fire-and-forget, but the current implementation doesn’t guard against out-of-order updates: if titleId or installId change while a fetch is in flight, a slow response could still call createInstance with a stale slug/title. This is a subtle correctness concern in the presence of navigation or rapid changes.
React’s recommended pattern is to either track an aborted flag inside the effect or use an AbortController so that only the latest effect run is allowed to call createInstance. Without this, you may end up with duplicate or misnamed instances in edge cases.
Suggestion
You can guard against stale async work by capturing the current titleId/installId and cancelling on cleanup, for example:
useEffect(() => {
if (!titleId || !installId || isCreating) return;
const fullId = `${titleId}-${installId}`;
const instanceExists = instances.some((inst) => inst._id === fullId);
if (instanceExists) return;
let cancelled = false;
const fetchAndCreateInstance = async () => {
try {
const apiBaseUrl =
import.meta.env.VITE_API_BASE_URL ||
"https://vibes-hosting-v2-preview.jchris.workers.dev";
const response = await fetch(`${apiBaseUrl}/api/apps/${titleId}`);
let title = titleId;
if (response.ok) {
const data = (await response.json()) as { app?: { title?: string } };
title = data.app?.title || titleId;
}
if (!cancelled) {
await createInstance(title, {}, installId);
}
} catch (error) {
console.warn("Failed to fetch app metadata, using slug:", error);
if (!cancelled) {
await createInstance(titleId, {}, installId);
}
}
};
void fetchAndCreateInstance();
return () => {
cancelled = true;
};
}, [titleId, installId, instances, createInstance, isCreating]);This keeps behavior deterministic even if dependencies change mid-flight. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
There was a problem hiding this comment.
Expand this to see my work.
- Updated the
VibeInstanceViewerContentuseEffectinvibes.diy/pkg/app/routes/vibe-viewer.tsxto add acancelledflag, gatecreateInstancecalls behind it, and return a cleanup function while preserving the existing app metadata fetch flow. - Verified the root
package.jsonscripts and confirmedpnpm checkis the correct monorepo verification command for changes undervibes.diy/pkg/app/routes. - Ran
pnpm checkand confirmed format, build, lint, and Vitest all pass except the existingvibes.diy/tests/app/remix.test.tsxfailure (Remix Route > should process vibe slug and navigate with prompt parameter), which is unrelated to thevibe-viewer.tsxchanges. - Confirmed there are no new TypeScript errors, lint issues, or snapshot changes attributable to the updated
useEffectand itsvoid fetchAndCreateInstance();usage. - Reviewed the updated diff for
vibes.diy/pkg/app/routes/vibe-viewer.tsxand ensured the cancellation cleanup function only runs when a new instance needs to be created, matching the original control flow. - Committed the
vibe-viewerasync guard as "fix: guard async instance creation in vibe viewer" and pushed it to thejchris/groups-sessionbranch for PR #714. - Validated that the new
cancelledguard behaves as a non-breaking safety improvement, so it’s appropriate to reply "@CharlieHelps yes please" and treat the review suggestion as addressed.
There was a problem hiding this comment.
Implemented the async cancellation guard around instance creation in vibe-viewer and pushed the change.
- The
useEffectthat lazily creates instances now:- Tracks a
cancelledflag. - Wraps
createInstancecalls inif (!cancelled)checks. - Returns a cleanup function that sets
cancelled = trueso any in-flightfetchAndCreateInstancerun won’t create an instance after unmount or dependency changes. - Uses
void fetchAndCreateInstance()to make the fire‑and‑forget intent explicit.
- Tracks a
No other behavior in the effect changed: it still fetches app metadata from the hosting API, falls back to the slug when the fetch fails, and only runs when the instance does not already exist.
I left the separate suggestions about tightening the API base URL fallback and adding URL encoding/timeout unchanged for now, since your “yes please” was on the concurrency comment. Happy to wire those up too if you’d like.
Verification
# Monorepo checks (format, build, lint, tests, hosting tests)
$ pnpm checkpnpm checkruns:prettier --write(root)core-cli tsc(monorepo build)eslint .plustsc --noEmitinhosting/pkgvitest --runacross all configured projects@vibes.diy/hosting-teststest suite
- Current status:
- All linting, build, and hosting tests pass.
- One browser test is failing locally:
vibes.diy/tests/app/remix.test.tsx- Case: “Remix Route > should process vibe slug and navigate with prompt parameter”
- Failure:
mockOnNavigateis never called.
- This failure is in the Remix route test and not in the vibe viewer path I touched in this commit; I have not adjusted that test or its implementation in this change.
| const apiBaseUrl = | ||
| import.meta.env.VITE_API_BASE_URL || | ||
| "https://vibes-hosting-v2-preview.jchris.workers.dev"; | ||
| const response = await fetch(`${apiBaseUrl}/api/apps/${titleId}`); |
There was a problem hiding this comment.
The API base URL is hardcoded to a preview workers domain as a fallback. That’s useful for development but risky in production: if VITE_API_BASE_URL is misconfigured or absent, production clients would start talking to the preview service silently. This is an environment/configuration correctness issue rather than a type error.
It’s usually better for production builds to fail fast (or at least log loudly) when critical configuration is missing instead of silently using a preview URL.
Suggestion
Consider moving the default into a clearly dev-only path or asserting that VITE_API_BASE_URL is defined in production, e.g.:
const apiBaseUrl = import.meta.env.VITE_API_BASE_URL;
if (!apiBaseUrl) {
if (import.meta.env.DEV) {
console.warn(
"VITE_API_BASE_URL not set; falling back to preview hosting endpoint for dev only.",
);
} else {
throw new Error("VITE_API_BASE_URL must be configured in production.");
}
}
const response = await fetch(`${apiBaseUrl ?? "https://vibes-hosting-v2-preview.jchris.workers.dev"}/api/apps/${titleId}`);This keeps the convenient dev fallback but prevents silent misrouting in production. Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.
| const apiBaseUrl = | ||
| import.meta.env.VITE_API_BASE_URL || | ||
| "https://vibes-hosting-v2-preview.jchris.workers.dev"; | ||
| const response = await fetch(`${apiBaseUrl}/api/apps/${titleId}`); | ||
|
|
||
| let title = titleId; // Fallback to slug if fetch fails | ||
| if (response.ok) { | ||
| const data = (await response.json()) as { | ||
| app?: { title?: string }; | ||
| }; | ||
| title = data.app?.title || titleId; | ||
| } | ||
|
|
||
| // Create instance with real title | ||
| await createInstance(title, {}, installId); | ||
| } catch (error) { | ||
| // If fetch fails, use slug as title | ||
| console.warn("Failed to fetch app metadata, using slug:", error); | ||
| await createInstance(titleId, {}, installId); |
There was a problem hiding this comment.
The fetch URL is constructed using template interpolation with titleId, which ultimately comes from the route. While that's normal, there are two subtle issues here:
- Lack of URL encoding: If
titleIdever contains characters that are not URL-safe (spaces,?,#, etc.), the request could break or be misinterpreted by the server. - Missing timeout / abort:
fetchis called without any timeout orAbortController, so a slow or hanging hosting API could block instance creation for an unbounded time and degrade UX on this page.
These are not correctness bugs today given typical slug shapes, but they are easy-to-hit edge cases and a performance/UX risk if the hosting API becomes flaky.
Suggestion
To make this more robust:
- URL-encode
titleIdwhen constructing the path segment, e.g.encodeURIComponent(titleId). - Optionally, add a basic timeout via
AbortControllerso a stuck API call doesn't block instance creation indefinitely.
For example:
const controller = new AbortController();
const timeoutId = setTimeout(() => controller.abort(), 8000); // 8s timeout
try {
const response = await fetch(
`${apiBaseUrl}/api/apps/${encodeURIComponent(titleId)}`,
{ signal: controller.signal },
);
// ... existing logic
} finally {
clearTimeout(timeoutId);
}Reply with "@CharlieHelps yes please" if you'd like me to add a commit that applies these changes.
Summary
Adds a public app metadata endpoint and fixes instance creation to use real app titles instead of "Fresh Data". Also makes sessionId optional in SessionSidebar for better flexibility.
Key Changes
1. App Metadata Endpoint
GET /api/apps/:slugendpoint in hosting2. Fix Instance Naming
3. SessionSidebar Improvements
sessionId=""from BrutalistLayoutFiles Changed
Created:
hosting/pkg/src/endpoints/appGet.ts- New GET endpoint for app metadataModified:
hosting/pkg/src/index.ts- Register GET endpointprompts/pkg/chat.ts- Make sessionId optionalvibes.diy/pkg/app/components/BrutalistLayout.tsx- Remove sessionId propvibes.diy/pkg/app/routes/vibe-viewer.tsx- Fetch app title from APITest Plan
API Example
GET /api/apps/my-vibe-slug Response: { "success": true, "app": { "slug": "my-vibe-slug", "title": "My Awesome App", "remixOf": null, "hasScreenshot": true, "hasIcon": false } }🤖 Generated with Claude Code