Pr 3266 fix#3322
Conversation
|
@loks0n @ChiragAgg5k this PR has 119 file changes and is 286 commits behind main. Could you provide guidance on whether this should be split into smaller PRs? |
🦋 Changeset detectedLatest commit: ac862c6 The changes in this PR will be included in the next version bump. This PR includes changesets to release 25 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR adds an OpenClaw AI agent platform integration consisting of three new database models ( 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Rebased and conflicts resolved. Also cleaned up fork-specific branding (AirTrigger → Trigger.dev) across all 53 changed files. Ready for review 🙏 |
Squashed rebase of pr-3266-fix onto official/main. Co-authored-by: James Ritchie <james@trigger.dev>
deepshekhardas
force-pushed
the
pr-3266-fix
branch
from
c13b432 to
ac862c6
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.changeset/openclaw-agent-integration.md (1)
1-9:⚠️ Potential issue | 🟡 MinorChangeset targets incorrect packages with no actual changes.
The changeset specifies
@trigger.dev/coreand@trigger.dev/sdkas patch versions, but no code changes exist in eitherpackages/coreorpackages/trigger-sdk. The OpenClaw agent integration changes are only inapps/webapp(internal application), which doesn't publish as an npm package. Either remove this changeset if the changes are internal-only, or update it to target the correct published package.
🧹 Nitpick comments (2)
apps/webapp/app/routes/api.agents.provision.ts (1)
3-3: ⚡ Quick winUnused import.
spawnfromchild_processis imported but never used.-import { spawn } from "child_process";apps/webapp/app/routes/agents.$agentId.status.tsx (1)
2-3: ⚡ Quick winRemove unused imports.
jsonfrom@remix-run/nodeanduseLoaderDatafrom@remix-run/reactare imported but not used. The code usestypedjsonanduseTypedLoaderDatainstead.import type { LoaderFunctionArgs } from "`@remix-run/node`"; -import { json } from "`@remix-run/node`"; -import { useLoaderData } from "`@remix-run/react`"; import { typedjson, useTypedLoaderData } from "remix-typedjson";
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5f4db2e7-9540-4365-8aee-06def1775349
📒 Files selected for processing (47)
.changeset/openclaw-agent-integration.mdapps/webapp/app/components/AskAI.tsxapps/webapp/app/components/ErrorDisplay.tsxapps/webapp/app/components/LogoIcon.tsxapps/webapp/app/components/LogoType.tsxapps/webapp/app/components/code/AIQueryInput.tsxapps/webapp/app/components/code/TSQLEditor.tsxapps/webapp/app/components/code/TSQLResultsTable.tsxapps/webapp/app/components/logs/LogDetailView.tsxapps/webapp/app/components/logs/LogsTable.tsxapps/webapp/app/components/navigation/EnvironmentSelector.tsxapps/webapp/app/components/navigation/OrganizationSettingsSideMenu.tsxapps/webapp/app/components/navigation/SideMenu.tsxapps/webapp/app/components/navigation/SideMenuHeader.tsxapps/webapp/app/components/onboarding/TechnologyPicker.tsxapps/webapp/app/components/primitives/Select.tsxapps/webapp/app/components/primitives/Sheet.tsxapps/webapp/app/components/primitives/TreeView/TreeView.tsxapps/webapp/app/components/primitives/charts/ChartLegendCompound.tsxapps/webapp/app/components/primitives/charts/ChartZoom.tsxapps/webapp/app/components/runs/v3/ReplayRunDialog.tsxapps/webapp/app/components/scheduled/timezones.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.batches.$batchParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.deployments.$deploymentParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.environment-variables.new/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHelpSidebar.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.query/QueryHistoryPopover.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.tokens.$waitpointParam/route.tsxapps/webapp/app/routes/_app.orgs.$organizationSlug.settings._index/route.tsxapps/webapp/app/routes/_app.timezones/route.tsxapps/webapp/app/routes/agents.$agentId.status.tsxapps/webapp/app/routes/agents.setup.tsxapps/webapp/app/routes/api.agents.provision.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.$runParam.streams.$streamKey/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.schedules.new/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.waitpoints.$waitpointFriendlyId.complete/route.tsxapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.schedules.new.natural-language.tsxapps/webapp/app/routes/storybook.popover/route.tsxapps/webapp/app/routes/storybook/route.tsxapps/webapp/app/routes/webhooks.slack.tsapps/webapp/remix.config.jsinternal-packages/database/prisma/migrations/20260325122458_add_openclaw_agents/migration.sqlinternal-packages/database/prisma/schema.prismainternal-packages/emails/emails/components/Footer.tsx
| <linearGradient id="logoGradient" x1="0%" y1="0%" x2="100%" y2="0%"> | ||
| <stop offset="0%" stopColor="#2E7D32" /> | ||
| <stop offset="100%" stopColor="#43A047" /> | ||
| </linearGradient> | ||
| </defs> | ||
| {/* Icon */} | ||
| <rect x="0" y="8" width="64" height="64" rx="14" fill="url(#logoGradient)" /> | ||
| <path d="M32 20L46 56H18L32 20Z" fill="white" fillOpacity="0.9" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and read the LogoType.tsx file
fd -t f "LogoType.tsx" apps/Repository: triggerdotdev/trigger.dev
Length of output: 109
🏁 Script executed:
# Search for other uses of "logoGradient" or similar gradient patterns in the codebase
rg "logoGradient|linearGradient.*id=" --type tsx --type tsRepository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
# Check the current implementation of LogoType.tsx
cat apps/webapp/app/components/LogoType.tsxRepository: triggerdotdev/trigger.dev
Length of output: 1022
🏁 Script executed:
# Find components that use LogoType to understand if multiple instances appear on same page
rg "LogoType" --type tsx --type ts -B 2 -A 2Repository: triggerdotdev/trigger.dev
Length of output: 97
🏁 Script executed:
# Search for other gradient patterns and inline SVG IDs (broader search)
rg "id=\"[^\"]*Gradient|url\(#" apps/webapp --type-listRepository: triggerdotdev/trigger.dev
Length of output: 459
🏁 Script executed:
# Search for logoGradient references
rg "logoGradient" apps/Repository: triggerdotdev/trigger.dev
Length of output: 310
🏁 Script executed:
# Find imports of LogoType to understand usage patterns
rg "import.*LogoType|from.*LogoType" apps/Repository: triggerdotdev/trigger.dev
Length of output: 155
🏁 Script executed:
# Check if LoginPageLayout mentioned in review actually uses LogoType
rg "LogoType" apps/webapp/app/components/LoginPageLayout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 152
🏁 Script executed:
# Find all imports of LogoType across the entire codebase
rg "import.*LogoType|from.*LogoType" apps/Repository: triggerdotdev/trigger.dev
Length of output: 155
🏁 Script executed:
# Search for other inline SVG patterns with IDs to assess collision risk
rg "id\s*=\s*['\"]" apps/webapp --max-count=50 | head -40Repository: triggerdotdev/trigger.dev
Length of output: 6339
🏁 Script executed:
# Check if there are other components in same directory that might have SVG elements
ls -la apps/webapp/app/components/ | grep -E "\.(tsx?|jsx?)$" | head -20Repository: triggerdotdev/trigger.dev
Length of output: 1470
🏁 Script executed:
# Search for any other gradient definitions or SVG markers/patterns
rg "linearGradient|pattern|marker" apps/webapp/app --max-count=20Repository: triggerdotdev/trigger.dev
Length of output: 50383
🏁 Script executed:
# Check if LogoType is used anywhere else besides LoginPageLayout
rg "LogoType" apps/ -A 3 -B 1 2>/dev/null | head -100Repository: triggerdotdev/trigger.dev
Length of output: 1234
🏁 Script executed:
# Look at LoginPageLayout to see how LogoType is used
cat apps/webapp/app/components/LoginPageLayout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 3938
🏁 Script executed:
# Search for any scenarios where the page might render multiple SVGs with inline gradients
rg "linearGradient|<svg" apps/webapp/app/components/LoginPageLayout.tsxRepository: triggerdotdev/trigger.dev
Length of output: 51
Use useId() for unique SVG gradient IDs in reusable components.
The fixed id="logoGradient" could cause collisions if LogoType is ever rendered multiple times on the same page. While the current usage (single instance in LoginPageLayout) is safe, it's best practice to use React's useId() hook for any IDs in reusable components.
Proposed fix
+import { useId } from "react";
+
export function LogoType({ className }: { className?: string }) {
+ const gradientId = useId();
return (
<svg
viewBox="0 0 520 80"
xmlns="http://www.w3.org/2000/svg"
className={className}
>
<defs>
- <linearGradient id="logoGradient" x1="0%" y1="0%" x2="100%" y2="0%">
+ <linearGradient id={gradientId} x1="0%" y1="0%" x2="100%" y2="0%">
<stop offset="0%" stopColor="`#2E7D32`" />
<stop offset="100%" stopColor="`#43A047`" />
</linearGradient>
</defs>
{/* Icon */}
- <rect x="0" y="8" width="64" height="64" rx="14" fill="url(`#logoGradient`)" />
+ <rect x="0" y="8" width="64" height="64" rx="14" fill={`url(#${gradientId})`} />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <linearGradient id="logoGradient" x1="0%" y1="0%" x2="100%" y2="0%"> | |
| <stop offset="0%" stopColor="#2E7D32" /> | |
| <stop offset="100%" stopColor="#43A047" /> | |
| </linearGradient> | |
| </defs> | |
| {/* Icon */} | |
| <rect x="0" y="8" width="64" height="64" rx="14" fill="url(#logoGradient)" /> | |
| <path d="M32 20L46 56H18L32 20Z" fill="white" fillOpacity="0.9" /> | |
| import { useId } from "react"; | |
| export function LogoType({ className }: { className?: string }) { | |
| const gradientId = useId(); | |
| return ( | |
| <svg | |
| viewBox="0 0 520 80" | |
| xmlns="http://www.w3.org/2000/svg" | |
| className={className} | |
| > | |
| <defs> | |
| <linearGradient id={gradientId} x1="0%" y1="0%" x2="100%" y2="0%"> | |
| <stop offset="0%" stopColor="`#2E7D32`" /> | |
| <stop offset="100%" stopColor="`#43A047`" /> | |
| </linearGradient> | |
| </defs> | |
| {/* Icon */} | |
| <rect x="0" y="8" width="64" height="64" rx="14" fill={`url(#${gradientId})`} /> | |
| <path d="M32 20L46 56H18L32 20Z" fill="white" fillOpacity="0.9" /> | |
| </svg> | |
| ); | |
| } |
| > | ||
| <ResizablePanel id="payload" min="300px"> | ||
| <div className="rounded-smbg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600"> | ||
| <div className="rounded-smbg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300"> |
There was a problem hiding this comment.
Restore the missing class separator here.
rounded-smbg-charcoal-900 looks like two Tailwind utilities collapsed into one token, so the panel will lose its intended radius/background styling.
Fix
- <div className="rounded-smbg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300">
+ <div className="rounded-sm bg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div className="rounded-smbg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300"> | |
| <div className="rounded-sm bg-charcoal-900 mb-3 h-full min-h-40 overflow-y-auto scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300"> |
| {check.isHealthy ? "✓ Healthy" : "✗ Unhealthy"} | ||
| </span> | ||
| </TableCell> | ||
| <TableCell>{check.responseTimeMs}ms</TableCell> |
There was a problem hiding this comment.
Handle null responseTimeMs.
responseTimeMs is nullable in the schema. When null, this renders "nullms".
- <TableCell>{check.responseTimeMs}ms</TableCell>
+ <TableCell>{check.responseTimeMs != null ? `${check.responseTimeMs}ms` : "N/A"}</TableCell>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <TableCell>{check.responseTimeMs}ms</TableCell> | |
| <TableCell>{check.responseTimeMs != null ? `${check.responseTimeMs}ms` : "N/A"}</TableCell> |
| // Parse tools JSON | ||
| const tools = JSON.parse(data.tools || "[]"); | ||
|
|
There was a problem hiding this comment.
JSON.parse can throw on malformed input.
data.tools is validated by Zod as a string, but JSON.parse can still throw SyntaxError if the string isn't valid JSON. This would cause an unhandled exception.
🛠️ Suggested fix using Zod transform
const SetupSchema = z.object({
agentName: z.string().min(1, "Agent name is required"),
model: z.enum(["claude-3.5-sonnet", "claude-3-opus", "gpt-4-turbo"]),
messagingPlatform: z.enum(["slack", "discord", "telegram"]),
- tools: z.string(), // JSON string array
+ tools: z.string().transform((val, ctx) => {
+ try {
+ const parsed = JSON.parse(val || "[]");
+ if (!Array.isArray(parsed)) {
+ ctx.addIssue({ code: z.ZodIssueCode.custom, message: "Tools must be an array" });
+ return z.NEVER;
+ }
+ return parsed as string[];
+ } catch {
+ ctx.addIssue({ code: z.ZodIssueCode.custom, message: "Invalid JSON" });
+ return z.NEVER;
+ }
+ }),
slackWorkspaceId: z.string().optional(),
slackWebhookToken: z.string().optional(),
});
// Then in action:
- // Parse tools JSON
- const tools = JSON.parse(data.tools || "[]");
+ const tools = data.tools; // Already parsed by Zod| const provisionResponse = await fetch("http://localhost:3000/api/agents/provision", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ agentId: agentConfig.id }), | ||
| }); | ||
|
|
||
| if (!provisionResponse.ok) { | ||
| logger.error("Provisioning failed", { | ||
| agentId: agentConfig.id, | ||
| status: provisionResponse.status, | ||
| }); | ||
| } | ||
| } catch (error) { | ||
| logger.error("Failed to call provisioning endpoint", { error }); | ||
| } |
There was a problem hiding this comment.
Hardcoded localhost URL won't work in production; provisioning failure leaves orphaned agents.
Two issues:
http://localhost:3000is hardcoded - this won't work in deployed environments. Use an environment variable viaenv.server.tsper coding guidelines.- If provisioning fails (Lines 77-85), the agent config is already created and the user is still redirected to the status page. This leaves the agent in an inconsistent state with no way to retry provisioning.
🛠️ Suggested approach
+import { env } from "~/env.server";
+
// In the action:
- const provisionResponse = await fetch("http://localhost:3000/api/agents/provision", {
+ const provisionResponse = await fetch(`${env.APP_ORIGIN}/api/agents/provision`, {
method: "POST",
headers: { "Content-Type": "application/json" },
body: JSON.stringify({ agentId: agentConfig.id }),
});
if (!provisionResponse.ok) {
logger.error("Provisioning failed", {
agentId: agentConfig.id,
status: provisionResponse.status,
});
+ // Update status to failed so user can retry
+ await prisma.agentConfig.update({
+ where: { id: agentConfig.id },
+ data: { status: "failed" },
+ });
}
} catch (error) {
logger.error("Failed to call provisioning endpoint", { error });
+ await prisma.agentConfig.update({
+ where: { id: agentConfig.id },
+ data: { status: "failed" },
+ });
}Source: Coding guidelines
| try { | ||
| const event = await request.json() as any; | ||
|
|
||
| // Handle Slack URL verification | ||
| if (event.type === "url_verification") { | ||
| return json({ challenge: event.challenge }); | ||
| } | ||
|
|
||
| // Handle message events | ||
| if (event.type === "event_callback" && event.event.type === "message") { | ||
| const slackEvent = event.event; | ||
| const workspaceId = event.team_id; |
There was a problem hiding this comment.
Missing Slack request signature verification and input validation.
- Security: Slack webhooks should verify the
X-Slack-Signatureheader to prevent forged requests. Without this, anyone can call this endpoint pretending to be Slack. - Validation: The payload is cast as
anywithout validation. Line 24 accessesevent.event.typewhich will throw ifevent.eventis undefined.
🛠️ Suggested approach for signature verification
import crypto from "crypto";
import { env } from "~/env.server";
function verifySlackSignature(request: Request, body: string): boolean {
const timestamp = request.headers.get("X-Slack-Request-Timestamp");
const signature = request.headers.get("X-Slack-Signature");
if (!timestamp || !signature) return false;
// Prevent replay attacks (5 min window)
if (Math.abs(Date.now() / 1000 - Number(timestamp)) > 300) return false;
const sigBaseString = `v0:${timestamp}:${body}`;
const mySignature = "v0=" + crypto
.createHmac("sha256", env.SLACK_SIGNING_SECRET)
.update(sigBaseString)
.digest("hex");
return crypto.timingSafeEqual(
Buffer.from(mySignature),
Buffer.from(signature)
);
}| // Route message to OpenClaw container (on VPS) | ||
| const containerUrl = `http://178.128.150.129:${agent.containerPort}`; |
There was a problem hiding this comment.
Hardcoded IP address won't work across environments.
The container URL uses a hardcoded IP 178.128.150.129. This should be configured via environment variable.
+import { env } from "~/env.server";
+
- const containerUrl = `http://178.128.150.129:${agent.containerPort}`;
+ const containerUrl = `http://${env.OPENCLAW_VPS_HOST}:${agent.containerPort}`;Source: Coding guidelines
| await prisma.agentExecution.create({ | ||
| data: { | ||
| agentId: agent.id, | ||
| message: text, | ||
| response: agentResponse, | ||
| executionTimeMs: 0, // TODO: Measure actual execution time | ||
| inputTokens: containerData?.inputTokens, | ||
| outputTokens: containerData?.outputTokens, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Potential undefined value stored in database.
slackEvent.text (Line 28) could be undefined for certain Slack event types. Storing undefined in the required message field could cause a Prisma error or store unexpected values.
await prisma.agentExecution.create({
data: {
agentId: agent.id,
- message: text,
+ message: text ?? "",
response: agentResponse,
executionTimeMs: 0, // TODO: Measure actual execution time| -- DropIndex | ||
| DROP INDEX "public"."SecretStore_key_idx"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."TaskRun_runtimeEnvironmentId_createdAt_idx"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."TaskRun_runtimeEnvironmentId_id_idx"; |
There was a problem hiding this comment.
Remove extraneous Prisma-generated changes and use separate migration for existing table indexes.
Per coding guidelines:
- Lines 16-44 modify junction tables (
_BackgroundWorkerToBackgroundWorkerFile,_BackgroundWorkerToTaskQueue,_TaskRunToTaskRunTag,_WaitpointRunConnections,_completedWaitpoints) - these should be removed as extraneous Prisma diff noise. - Lines 1-8 drop indexes on existing tables (
SecretStore,TaskRun) and Lines 105-112 recreate them - these should useCREATE INDEX CONCURRENTLY IF NOT EXISTSin a separate migration file to avoid table locks in production.
Based on coding guidelines: "Clean up generated Prisma migrations by removing extraneous lines for junction tables... When adding indexes to existing tables, use CREATE INDEX CONCURRENTLY IF NOT EXISTS... and place each concurrent index in its own separate migration file."
Also applies to: 16-44
Source: Coding guidelines
| /// Webhook token for Slack | ||
| slackWebhookToken String? | ||
|
|
||
| /// User's API keys (encrypted) | ||
| apiKeys Json? |
There was a problem hiding this comment.
Sensitive credentials stored as plain text.
slackWebhookToken (Line 3219) and apiKeys (Line 3222) store sensitive credentials. The codebase already has a SecretReference pattern (used for MFA secrets in the User model at Lines 52-53). Consider using SecretReference relations or an encrypted storage mechanism for these fields rather than storing them as plain String/Json.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| invalidSelection | ||
| ? "border-amber-800 bg-amber-950 text-amber-400" | ||
| : "border-blue-800 bg-[#1B2334] text-blue-400" | ||
| : "border-blue-800 bg-blue-50 text-blue-400" |
There was a problem hiding this comment.
🔴 Zoom tooltip background changed from dark to near-white, breaking dark theme
The zoom selection tooltip background was changed from bg-[#1B2334] (a dark navy blue matching the dark theme) to bg-blue-50 (Tailwind's default #eff6ff, an almost-white color). This applies to both the tooltip body (line 130) and its arrow indicator (line 144). In the dark-themed app, this renders as a bright white tooltip with light blue text (text-blue-400), making it extremely jarring and potentially unreadable against the dark chart background.
| : "border-blue-800 bg-blue-50 text-blue-400" | |
| : "border-blue-800 bg-[#1B2334] text-blue-400" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| invalidSelection | ||
| ? "border-l border-t border-amber-800 bg-amber-950" | ||
| : "border-l border-t border-blue-800 bg-[#1B2334]" | ||
| : "border-l border-t border-blue-800 bg-blue-50" |
There was a problem hiding this comment.
🔴 Zoom tooltip arrow background also changed to near-white bg-blue-50
Same issue as the tooltip body — the arrow's background was changed from bg-[#1B2334] to bg-blue-50, creating a bright white arrow on a dark chart. This is the companion change to the tooltip body at line 130.
| : "border-l border-t border-blue-800 bg-blue-50" | |
| : "border-l border-t border-blue-800 bg-[#1B2334]" |
Was this helpful? React with 👍 or 👎 to provide feedback.
| {...props} | ||
| className={cn( | ||
| "overflow-y-auto overscroll-contain scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600 focus-custom", | ||
| "overflow-y-auto overscroll-contain scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300 focus-custom", |
There was a problem hiding this comment.
🔴 scrollbar-thumb-charcoal-600 replaced with scrollbar-thumb-gray-300, breaking dark theme scrollbars across entire app
All instances of scrollbar-thumb-charcoal-600 were globally replaced with scrollbar-thumb-gray-300. The custom charcoal-600 is defined as #3B3E45 (a dark gray matching the dark theme) in apps/webapp/tailwind.config.js:65. The gray color is NOT customized in the Tailwind config, so gray-300 resolves to Tailwind's default #D1D5DB — a very light gray. This makes all scrollbar thumbs extremely bright against the dark background across ~30 components (side menus, editors, popovers, tables, detail views, etc.).
| "overflow-y-auto overscroll-contain scrollbar-thin scrollbar-track-transparent scrollbar-thumb-gray-300 focus-custom", | |
| "overflow-y-auto overscroll-contain scrollbar-thin scrollbar-track-transparent scrollbar-thumb-charcoal-600 focus-custom", |
Was this helpful? React with 👍 or 👎 to provide feedback.
| -- DropIndex | ||
| DROP INDEX "public"."SecretStore_key_idx"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."TaskRun_runtimeEnvironmentId_createdAt_idx"; | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."TaskRun_runtimeEnvironmentId_id_idx"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."FeatureFlag" ALTER COLUMN "updatedAt" DROP DEFAULT; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."IntegrationDeployment" ALTER COLUMN "updatedAt" DROP DEFAULT; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."_BackgroundWorkerToBackgroundWorkerFile" ADD CONSTRAINT "_BackgroundWorkerToBackgroundWorkerFile_AB_pkey" PRIMARY KEY ("A", "B"); | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."_BackgroundWorkerToBackgroundWorkerFile_AB_unique"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."_BackgroundWorkerToTaskQueue" ADD CONSTRAINT "_BackgroundWorkerToTaskQueue_AB_pkey" PRIMARY KEY ("A", "B"); | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."_BackgroundWorkerToTaskQueue_AB_unique"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."_TaskRunToTaskRunTag" ADD CONSTRAINT "_TaskRunToTaskRunTag_AB_pkey" PRIMARY KEY ("A", "B"); | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."_TaskRunToTaskRunTag_AB_unique"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."_WaitpointRunConnections" ADD CONSTRAINT "_WaitpointRunConnections_AB_pkey" PRIMARY KEY ("A", "B"); | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."_WaitpointRunConnections_AB_unique"; | ||
|
|
||
| -- AlterTable | ||
| ALTER TABLE "public"."_completedWaitpoints" ADD CONSTRAINT "_completedWaitpoints_AB_pkey" PRIMARY KEY ("A", "B"); | ||
|
|
||
| -- DropIndex | ||
| DROP INDEX "public"."_completedWaitpoints_AB_unique"; |
There was a problem hiding this comment.
🔴 Migration includes extraneous destructive changes that violate repository rules
The migration file violates the explicit rules in internal-packages/database/CLAUDE.md:28-35, which state: "Clean up generated migration - remove extraneous lines for: _BackgroundWorkerToBackgroundWorkerFile, _BackgroundWorkerToTaskQueue, _TaskRunToTaskRunTag, _WaitpointRunConnections, _completedWaitpoints, SecretStore_key_idx, Various TaskRun indexes". The migration includes all of these auto-generated changes. Most critically, it drops and recreates indexes on the TaskRun table (lines 5-8, 109-112) without using CREATE INDEX CONCURRENTLY, violating the rule at internal-packages/database/CLAUDE.md:45 that indexes on existing tables must use CONCURRENTLY to avoid table locks in production. It also drops updatedAt defaults from FeatureFlag and IntegrationDeployment (lines 11-14) and modifies join table constraints (lines 17-44) — all extraneous changes unrelated to the OpenClaw agent feature.
Prompt for agents
The migration file contains many auto-generated extraneous changes that must be removed per the rules in internal-packages/database/CLAUDE.md. Specifically:
1. Remove all DROP INDEX and CREATE INDEX lines for SecretStore_key_idx and TaskRun indexes (lines 1-8 and 105-112) — these are auto-generated diffs that should not be in this migration.
2. Remove the ALTER TABLE lines for FeatureFlag and IntegrationDeployment updatedAt defaults (lines 11-14).
3. Remove all the join table constraint changes for _BackgroundWorkerToBackgroundWorkerFile, _BackgroundWorkerToTaskQueue, _TaskRunToTaskRunTag, _WaitpointRunConnections, and _completedWaitpoints (lines 17-44).
The migration should only contain the CREATE TABLE statements for AgentConfig, AgentExecution, AgentHealthCheck, their indexes (which are on newly created tables so CONCURRENTLY is not needed), and the AddForeignKey statements. Re-run pnpm run db:migrate:dev:create to regenerate a clean migration.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const agentConfig = await prisma.agentConfig.findUnique({ | ||
| where: { id: agentId }, |
There was a problem hiding this comment.
🟡 Uses findUnique instead of findFirst, violating Prisma query pattern rules
The apps/webapp/CLAUDE.md:126 explicitly states: "Always use findFirst instead of findUnique." The rationale is that Prisma's findUnique has an implicit DataLoader with known bugs (uppercase UUIDs returning null, composite key SQL issues, performance problems). The new file uses prisma.agentConfig.findUnique() at line 24. The same violation also occurs in apps/webapp/app/routes/agents.$agentId.status.tsx:28.
| const agentConfig = await prisma.agentConfig.findUnique({ | |
| where: { id: agentId }, | |
| const agentConfig = await prisma.agentConfig.findFirst({ | |
| where: { id: agentId }, |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const lastAgent = await prisma.agentConfig.findFirst({ | ||
| where: { | ||
| containerPort: { not: null }, | ||
| }, | ||
| orderBy: { containerPort: "desc" }, | ||
| }); | ||
|
|
||
| const nextPort = (lastAgent?.containerPort || 8000) + 1; |
There was a problem hiding this comment.
🔴 Port allocation race condition — concurrent provisions can assign the same port
In api.agents.provision.ts:33-40, the port allocation reads the highest existing containerPort and increments it, then writes the result. This is a classic TOCTOU (time-of-check-to-time-of-use) race: two concurrent provisioning requests can read the same max port and both compute the same nextPort, assigning duplicate ports to different agents. There is no database uniqueness constraint on containerPort in the schema (internal-packages/database/prisma/schema.prisma:3209) and no transaction or locking around this read-then-write sequence.
Prompt for agents
The port allocation in api.agents.provision.ts has a TOCTOU race condition. The code reads the max containerPort, increments it, and writes it in separate operations without any locking. Two concurrent requests can get the same port.
To fix this, either:
1. Add a unique constraint on containerPort in the Prisma schema and handle the unique violation error with a retry loop.
2. Use a Prisma interactive transaction ($transaction) with serializable isolation to atomically read the max port and write the new one.
3. Use a database sequence or advisory lock for port allocation.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const provisionResponse = await fetch("http://localhost:3000/api/agents/provision", { | ||
| method: "POST", | ||
| headers: { "Content-Type": "application/json" }, | ||
| body: JSON.stringify({ agentId: agentConfig.id }), | ||
| }); |
There was a problem hiding this comment.
🔴 Provisioning fetch hardcoded to localhost:3000 — wrong port, will always fail
In agents.setup.tsx:71, the action handler calls fetch("http://localhost:3000/api/agents/provision", ...) to trigger provisioning. However, the webapp runs on port 3030 locally (per CONTRIBUTING.md and CLAUDE.md), and in production the URL would be entirely different. This means the provisioning request will always fail to connect. The error is silently caught at line 83, so the agent is created and the user is redirected to the status page, but the agent will never actually be provisioned.
Prompt for agents
The provisioning fetch in agents.setup.tsx hardcodes http://localhost:3000 which is wrong — the webapp runs on port 3030 locally and a completely different URL in production. Instead of making an HTTP request to the same server, call the provisioning logic directly as a function import, or use the APP_ORIGIN / LOGIN_ORIGIN environment variable from env.server.ts to construct the correct base URL. Making an HTTP call from a server action to the same server is also an anti-pattern — prefer calling the provisioning service code directly.
Was this helpful? React with 👍 or 👎 to provide feedback.
| width="32" | ||
| height="32" | ||
| viewBox="0 0 32 32" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| className={className} | ||
| > | ||
| <rect width="32" height="32" rx="8" fill="#2E7D32" /> | ||
| <path | ||
| fillRule="evenodd" | ||
| clipRule="evenodd" | ||
| d="M96.1017 113.4L160.679 4.57764e-05L320.718 281.045H0.638916L65.2159 167.642L110.896 194.382L92.0035 227.561H229.354L160.679 106.965L141.786 140.144L96.1017 113.4Z" | ||
| fill="url(#paint0_linear_465_1663)" | ||
| d="M16 6L24 24H8L16 6Z" | ||
| fill="white" | ||
| fillOpacity="0.9" | ||
| /> | ||
| <defs> | ||
| <linearGradient | ||
| id="paint0_linear_465_1663" | ||
| x1="320.718" | ||
| y1="140.687" | ||
| x2="0.638918" | ||
| y2="140.687" | ||
| gradientUnits="userSpaceOnUse" | ||
| > | ||
| <stop stopColor="#41FF54" /> | ||
| <stop offset="1" stopColor="#E7FF52" /> | ||
| </linearGradient> | ||
| </defs> | ||
| <circle cx="16" cy="20" r="2" fill="#2E7D32" /> |
There was a problem hiding this comment.
🚩 LogoIcon and LogoType completely replaced with different brand identity
The Trigger.dev logo SVGs in LogoIcon.tsx and LogoType.tsx have been entirely replaced. The original logo was a detailed geometric shape with a green-to-yellow gradient (#41FF54 to #E7FF52), consistent with Trigger.dev's established brand. The new logo is a green rounded rectangle (#2E7D32) with a simple white triangle and dot — a completely different design language. The LogoType also replaces the detailed path-based wordmark with an SVG <text> element rendering "Trigger.dev", which may render differently across browsers depending on font availability (Geist Variable, Inter, Helvetica Neue, sans-serif). This is a significant brand change that should be verified as intentional.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const action = async ({ request }: ActionFunctionArgs) => { | ||
| if (request.method !== "POST") { | ||
| return json({ error: "Method not allowed" }, { status: 405 }); | ||
| } |
There was a problem hiding this comment.
🚩 New agent routes bypass the standard dashboardAction/dashboardLoader authorization patterns
The existing webapp routes consistently use dashboardAction/dashboardLoader wrappers (e.g., apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:101) which handle authorization checks with { action: 'write', resource: { type: '...' } }. The new agent routes (agents.setup.tsx, agents.$agentId.status.tsx, api.agents.provision.ts, webhooks.slack.ts) don't use these patterns. The setup and status routes use requireUser for authentication but skip the authorization layer entirely. The provisioning endpoint has no authentication at all. This is inconsistent with the established patterns in the codebase.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const action = async ({ request }: ActionFunctionArgs) => { | ||
| if (request.method !== "POST") { | ||
| return json({ error: "Method not allowed" }, { status: 405 }); | ||
| } | ||
|
|
||
| try { | ||
| const event = await request.json() as any; | ||
|
|
||
| // Handle Slack URL verification | ||
| if (event.type === "url_verification") { | ||
| return json({ challenge: event.challenge }); |
There was a problem hiding this comment.
🚩 Webhook route processes Slack events without verifying request signature
The webhooks.slack.ts route handles incoming Slack events including url_verification and event_callback message types. Slack's documentation requires verifying the X-Slack-Signature header using the signing secret to ensure requests actually come from Slack. While this is a security concern (handled separately), it's worth noting from a correctness standpoint that the url_verification challenge response at line 19 would allow anyone to register this endpoint as a Slack webhook URL, and the message routing at lines 24+ would process any POST body that matches the expected shape, potentially triggering agent executions and database writes from non-Slack sources.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
What
Adds OpenClaw agent integration — a new agent provisioning system with Slack webhooks, agent setup/status pages, and database models.
Changes
New files
routes/agents.$agentId.status.tsx— agent status dashboard pageroutes/agents.setup.tsx— agent setup wizardroutes/api.agents.provision.ts— provisioning APIroutes/webhooks.slack.ts— Slack webhook handlerinternal-packages/database/prisma/migrations/20260325122458_add_openclaw_agents/migration.sql.changeset/openclaw-agent-integration.mdSchema
AgentConfig,AgentExecution,AgentHealthCheckmodelsagentConfigsrelation to User modelManual resolutions (merge conflicts)
11 files conflicted during merge:
Branding cleanup
Reverted fork-specific branding (
AirTrigger→Trigger.dev,airtrigger.dev→trigger.dev) across 53 files.Notes
Co-authored-by: James Ritchie james@trigger.dev