Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0be59bb1f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const requestedFramework = body.framework?.find( | ||
| (framework) => typeof framework === "string" && framework.trim(), | ||
| ); |
There was a problem hiding this comment.
Handle non-array framework payloads safely
body.framework?.find(...) assumes framework is an array, so a request like {"framework":"claude-code"} will throw TypeError: ...find is not a function before any validation response is returned. That turns a client input mistake into a 500 and bypasses the intended fallback-to-defaults behavior. Please normalize/validate framework with Array.isArray before calling .find.
Useful? React with 👍 / 👎.
| const regions = body.regions?.length ? body.regions : ["global"]; | ||
| const slug = body.slug || slugify(body.name); | ||
| const aport = createAPortService(env); | ||
| const role = (body.role || frameworkPreset?.role || "agent").trim(); |
There was a problem hiding this comment.
Keep role enum stable when applying framework presets
Using frameworkPreset.role directly lets non-enum values like "Claude Code agent" be persisted as the passport role, but gallery role filters only query canonical values (agent|assistant|tool|service) via strict equality in functions/lib/gallery.ts. As a result, preset-created passports disappear when users filter to "Agents". Normalize preset roles to the existing enum (or store display labels separately) before issuing.
Useful? React with 👍 / 👎.
| console.warn("[issue] Framework preset unavailable; using defaults", { | ||
| framework: requestedFramework, | ||
| status: presetResult.error?.status, | ||
| message: presetResult.error?.message, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Return upstream error for preset-only quick issuance failures
When preset lookup fails, the handler logs a warning and falls back to validating name/description as required fields. In the new quick-hosted flow (framework + email only), any transient preset API failure now surfaces as a 400 "name required" client error instead of an upstream 5xx, so callers cannot distinguish bad input from backend outage. Short-circuit with a 502/503 when framework-only issuance depends on an unavailable preset.
Useful? React with 👍 / 👎.
| const slug = body.slug || slugify(name); | ||
|
|
||
| // Build capabilities and limits, optionally including deliverable enforcement | ||
| const capabilities = [...DEFAULT_CAPABILITIES]; | ||
| const limits: Record<string, any> = { ...DEFAULT_LIMITS }; | ||
| const capabilities = frameworkPreset?.capabilities?.length | ||
| ? frameworkPreset.capabilities.map((capability) => ({ |
There was a problem hiding this comment.
Fail closed when preset capabilities are absent
If a framework preset is found but its capabilities become empty (for example, an empty array or entries filtered out by getFrameworkPassportPreset), this branch silently falls back to DEFAULT_CAPABILITIES and issues a broader-permission passport than the preset intended. For quick-hosted issuance that should inherit preset policy, this is a fail-open privilege expansion; reject the preset or preserve the empty set instead of defaulting to generic capabilities.
Useful? React with 👍 / 👎.
| const name = (body.name || frameworkPreset?.name || "").trim(); | ||
| const description = ( | ||
| body.description || | ||
| frameworkPreset?.description || | ||
| "" | ||
| ).trim(); |
There was a problem hiding this comment.
Guard name/description normalization against non-string input
name and description are normalized with .trim() before runtime type checks, so JSON payloads like {"name":123,"description":456,...} raise TypeError: ...trim is not a function and return 500. This turns simple client input errors into server errors and bypasses the endpoint’s normal 400 validation path; coerce or validate types before calling string methods.
Useful? React with 👍 / 👎.
No description provided.