Conversation
RohitR311
added
Type: Enhancement
WalkthroughThis PR extends run source tracking from a boolean Changes
Sequence DiagramsequenceDiagram
actor Client as Client<br/>(API/SDK/MCP/CLI)
participant API as API Handler<br/>(record.ts/sdk.ts)
participant DB as Database<br/>(Run Model)
participant UI as Frontend<br/>(RunsTable/ColapsibleRow)
Client->>API: Request with x-run-source header
API->>API: Parse runSource from header<br/>(default: 'api')
API->>DB: createWorkflowAndStoreMetadata()<br/>with runSource discriminant
DB->>DB: Set runByAPI/SDK/MCP/CLI<br/>based on runSource
DB->>DB: Persist to database
API-->>Client: Response with run metadata
UI->>DB: Fetch run records
DB-->>UI: Return runs with runByMCP/runByCLI flags
UI->>UI: Render RunTypeChip<br/>displaying MCP/CLI labels
UI-->>Client: Display run type in table
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/components/run/ColapsibleRow.tsx (1)
92-104: Consider refactoring the deeply nested ternary for readability.The
runByLabelcomputation has grown to 7 levels of nesting. While functionally correct, this could be simplified for maintainability:♻️ Optional refactor suggestion
- const runByLabel = row.runByScheduleId - ? `${row.runByScheduleId}` - : row.runByUserId - ? `${userEmail}` - : row.runByCLI - ? 'CLI' - : row.runByMCP - ? 'MCP' - : row.runBySDK - ? 'SDK' - : row.runByAPI - ? 'API' - : 'Unknown'; + const getRunByLabel = (): string => { + if (row.runByScheduleId) return row.runByScheduleId; + if (row.runByUserId) return userEmail ?? 'Unknown'; + if (row.runByCLI) return 'CLI'; + if (row.runByMCP) return 'MCP'; + if (row.runBySDK) return 'SDK'; + if (row.runByAPI) return 'API'; + return 'Unknown'; + }; + const runByLabel = getRunByLabel();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/run/ColapsibleRow.tsx` around lines 92 - 104, The nested ternary used to compute runByLabel is hard to read; replace the multi-level conditional in runByLabel with a clearer control structure (e.g., an if/else chain or a small helper function like getRunByLabel(row, userEmail)) that checks row.runByScheduleId, row.runByUserId, row.runByCLI, row.runByMCP, row.runBySDK, and row.runByAPI in order and returns the appropriate string (using userEmail when runByUserId is true, otherwise 'CLI'/'MCP'/'SDK'/'API'/'Unknown') so the logic is explicit and maintainable.server/src/api/record.ts (1)
1385-1386: Implementation is correct; consider adding a clarifying comment.CLI requests are properly routed through
sdk.ts(which mapsx-run-source: 'cli'correctly), so this API endpoint never receives CLI traffic. The POST handler only needs to distinguish MCP from regular API calls, and the current logic achieves this. Add a comment above line 1385 documenting this design:// CLI traffic uses the SDK endpoint (sdk.ts); this API endpoint only handles API and MCP requests const runSource = req.headers['x-run-source'] === 'mcp' ? 'mcp' : 'api';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@server/src/api/record.ts` around lines 1385 - 1386, Add a clarifying comment above the runSource assignment to explain that CLI traffic is routed through sdk.ts and therefore this endpoint only needs to distinguish MCP vs API; specifically, place a comment before the line that assigns runSource (const runSource = ...) referencing that CLI uses the SDK endpoint (sdk.ts) and then keep the existing logic that sets runSource and calls handleRunRecording(req.params.id, req.user.id, runSource, requestedFormats).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/api/sdk.ts`:
- Around line 492-493: The runSource derivation currently maps only 'cli' to
'cli' and forces every other header value to 'sdk', causing MCP runs to be
misclassified; update the logic that sets runSource (the line reading
req.headers['x-run-source'] and the value passed into handleRunRecording) to
explicitly detect 'mcp' and set runSource to 'mcp' (in addition to preserving
'cli'), so handleRunRecording receives 'mcp' for MCP requests and downstream
code in record handling (e.g., runByMCP in server/src/api/record.ts) is set
correctly.
In `@server/src/models/Run.ts`:
- Around line 126-133: The Run model now defines two new nullable boolean fields
runByMCP and runByCLI but there is no DB migration; add a new migration that
alters the run table to ADD COLUMN "runByMCP" BOOLEAN and ADD COLUMN "runByCLI"
BOOLEAN (nullable) and implement the corresponding down migration to DROP those
columns so existing databases get the new columns; reference the Run model
(Run.ts) and the run table/column names when creating the migration.
---
Nitpick comments:
In `@server/src/api/record.ts`:
- Around line 1385-1386: Add a clarifying comment above the runSource assignment
to explain that CLI traffic is routed through sdk.ts and therefore this endpoint
only needs to distinguish MCP vs API; specifically, place a comment before the
line that assigns runSource (const runSource = ...) referencing that CLI uses
the SDK endpoint (sdk.ts) and then keep the existing logic that sets runSource
and calls handleRunRecording(req.params.id, req.user.id, runSource,
requestedFormats).
In `@src/components/run/ColapsibleRow.tsx`:
- Around line 92-104: The nested ternary used to compute runByLabel is hard to
read; replace the multi-level conditional in runByLabel with a clearer control
structure (e.g., an if/else chain or a small helper function like
getRunByLabel(row, userEmail)) that checks row.runByScheduleId, row.runByUserId,
row.runByCLI, row.runByMCP, row.runBySDK, and row.runByAPI in order and returns
the appropriate string (using userEmail when runByUserId is true, otherwise
'CLI'/'MCP'/'SDK'/'API'/'Unknown') so the logic is explicit and maintainable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a995ecc-7c7a-4d9a-ab82-e7963830bb21
📒 Files selected for processing (12)
public/locales/de.jsonpublic/locales/en.jsonpublic/locales/es.jsonpublic/locales/ja.jsonpublic/locales/tr.jsonpublic/locales/zh.jsonserver/src/api/record.tsserver/src/api/sdk.tsserver/src/mcp-worker.tsserver/src/models/Run.tssrc/components/run/ColapsibleRow.tsxsrc/components/run/RunsTable.tsx
| const runSource = req.headers['x-run-source'] === 'cli' ? 'cli' : 'sdk'; | ||
| const runId = await handleRunRecording(robotId, user.id.toString(), runSource); |
There was a problem hiding this comment.
Handle mcp explicitly when deriving runSource.
Line 492 currently maps only 'cli'; every other value (including 'mcp') is forced to 'sdk'. That causes MCP runs (sent with x-run-source: mcp from server/src/mcp-worker.ts, Line 41) to be persisted as SDK, so runByMCP never gets set in server/src/api/record.ts.
💡 Proposed fix
- const runSource = req.headers['x-run-source'] === 'cli' ? 'cli' : 'sdk';
+ const headerRunSource = Array.isArray(req.headers['x-run-source'])
+ ? req.headers['x-run-source'][0]
+ : req.headers['x-run-source'];
+ const runSource: 'sdk' | 'cli' | 'mcp' =
+ headerRunSource === 'cli' || headerRunSource === 'mcp'
+ ? headerRunSource
+ : 'sdk';
const runId = await handleRunRecording(robotId, user.id.toString(), runSource);📝 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.
| const runSource = req.headers['x-run-source'] === 'cli' ? 'cli' : 'sdk'; | |
| const runId = await handleRunRecording(robotId, user.id.toString(), runSource); | |
| const headerRunSource = Array.isArray(req.headers['x-run-source']) | |
| ? req.headers['x-run-source'][0] | |
| : req.headers['x-run-source']; | |
| const runSource: 'sdk' | 'cli' | 'mcp' = | |
| headerRunSource === 'cli' || headerRunSource === 'mcp' | |
| ? headerRunSource | |
| : 'sdk'; | |
| const runId = await handleRunRecording(robotId, user.id.toString(), runSource); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/api/sdk.ts` around lines 492 - 493, The runSource derivation
currently maps only 'cli' to 'cli' and forces every other header value to 'sdk',
causing MCP runs to be misclassified; update the logic that sets runSource (the
line reading req.headers['x-run-source'] and the value passed into
handleRunRecording) to explicitly detect 'mcp' and set runSource to 'mcp' (in
addition to preserving 'cli'), so handleRunRecording receives 'mcp' for MCP
requests and downstream code in record handling (e.g., runByMCP in
server/src/api/record.ts) is set correctly.
| runByMCP: { | ||
| type: DataTypes.BOOLEAN, | ||
| allowNull: true, | ||
| }, | ||
| runByCLI: { | ||
| type: DataTypes.BOOLEAN, | ||
| allowNull: true, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for migration files that might add runByMCP or runByCLI columns
fd -e js -e ts migration | xargs -I{} grep -l -E 'runByMCP|runByCLI' {} 2>/dev/null || echo "No migration found"
# Also check for any migration files in common locations
fd -t f -e js -e ts . -p 'migrations|migrate' 2>/dev/null | head -20Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# First, let's check the actual Run.ts file at the specified lines
head -140 server/src/models/Run.ts | tail -20Repository: getmaxun/maxun
Length of output: 452
🏁 Script executed:
# Check if migrations directory exists and what's in it
find . -type d -name migrations -o -type d -name migrate 2>/dev/null | head -10Repository: getmaxun/maxun
Length of output: 84
🏁 Script executed:
# Look for any SQL migration files
find . -type f -name "*.sql" 2>/dev/null | head -20Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Check for Sequelize configuration and migration patterns
find . -type f \( -name ".sequelizerc" -o -name "sequelize.config.*" -o -name "config.json" \) 2>/dev/nullRepository: getmaxun/maxun
Length of output: 100
🏁 Script executed:
# List all TypeScript/JS files in common migration paths
fd -t f -e js -e ts -e sql . -p '(migration|migrate|db/|database/)' 2>/dev/null | head -30Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Check what's in the migrations directory
ls -la server/src/db/migrations/Repository: getmaxun/maxun
Length of output: 432
🏁 Script executed:
# Count and list migration files
find server/src/db/migrations -type f | sortRepository: getmaxun/maxun
Length of output: 248
🏁 Script executed:
# Check .sequelizerc content to understand migration setup
cat .sequelizercRepository: getmaxun/maxun
Length of output: 349
🏁 Script executed:
# Check the config.json to see Sequelize setup
cat server/config/config.jsonRepository: getmaxun/maxun
Length of output: 634
🏁 Script executed:
# Search the entire migrations directory for any reference to runByMCP or runByCLI
rg -i 'runByMCP|runByCLI' server/src/db/migrations/Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# List all files in migrations directory with more details
find server/src/db/migrations -type f -exec basename {} \; | sortRepository: getmaxun/maxun
Length of output: 173
🏁 Script executed:
# Check the most recent migration file
cat server/src/db/migrations/20250612000000-add-robot-name-unique-index.jsRepository: getmaxun/maxun
Length of output: 469
🏁 Script executed:
# Check the first migration which appears to add columns
cat server/src/db/migrations/20250327111003-add-airtable-columns.jsRepository: getmaxun/maxun
Length of output: 3263
🏁 Script executed:
# Check the Run.ts model to see all attributes and when they might have been added
grep -n "runBy\|type: DataTypes.BOOLEAN" server/src/models/Run.ts | head -30Repository: getmaxun/maxun
Length of output: 685
🏁 Script executed:
# Check the webhooks migration
cat server/src/db/migrations/20250527105655-add-webhooks.jsRepository: getmaxun/maxun
Length of output: 853
🏁 Script executed:
# Verify which table the Run model uses
grep -n "tableName\|table:" server/src/models/Run.ts | head -5Repository: getmaxun/maxun
Length of output: 83
🏁 Script executed:
# Search across all migrations for any reference to 'run' table
rg -i "addColumn.*'run'|addColumn.*\"run\"" server/src/db/migrations/Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Check git status to see what files are in this PR
git status --shortRepository: getmaxun/maxun
Length of output: 71
🏁 Script executed:
# Check if there are any uncommitted migration files
git diff --name-onlyRepository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Look for any migration files that might be timestamped with recent dates (April 2026)
ls -lt server/src/db/migrations/ | head -10Repository: getmaxun/maxun
Length of output: 329
🏁 Script executed:
# Get the full Run.ts file to understand the context better
wc -l server/src/models/Run.tsRepository: getmaxun/maxun
Length of output: 86
🏁 Script executed:
# Check if runBySDK was already in the model previously or if it's new
git log -1 --name-only server/src/models/Run.ts 2>/dev/null || echo "Not in git history"Repository: getmaxun/maxun
Length of output: 257
🏁 Script executed:
# Check recent commits to the Run model
git log --oneline -n 10 -- server/src/models/Run.ts 2>/dev/null || echo "Unable to get git log"Repository: getmaxun/maxun
Length of output: 99
🏁 Script executed:
# Let's check if there's a pending migration file that hasn't been added yet
find . -name "*runBy*" -o -name "*run-*.js" -o -name "*run-*.ts" 2>/dev/null | grep -i migratRepository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Get the complete Run.ts model around the new columns
sed -n '110,140p' server/src/models/Run.tsRepository: getmaxun/maxun
Length of output: 683
🏁 Script executed:
# Check if runBySDK exists in migrations (if it does, it shows a pattern)
rg -i 'runBySDK' server/src/db/migrations/Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Verify the table name one more time and check git diff for this file
git diff HEAD -- server/src/models/Run.ts | head -100Repository: getmaxun/maxun
Length of output: 40
🏁 Script executed:
# Check what's actually staged for commit
git status -sRepository: getmaxun/maxun
Length of output: 71
🏁 Script executed:
# Check the current HEAD to see what's committed vs what's being reviewed
git show --stat HEAD | head -30Repository: getmaxun/maxun
Length of output: 1698
🏁 Script executed:
# Final verification: list all migration files with timestamps to see if any cover runBy* columns
ls -la server/src/db/migrations/ && echo "---" && grep -r "runByMCP\|runByCLI" server/src/db/migrations/ || echo "No runBy* columns in migrations"Repository: getmaxun/maxun
Length of output: 468
Missing database migration for new columns.
The model defines runByMCP and runByCLI columns in the run table (lines 126-133), but no corresponding migration exists. Existing databases will throw a "column does not exist" error at runtime when attempting to write these fields.
Add a migration to alter the run table:
ALTER TABLE run ADD COLUMN "runByMCP" BOOLEAN;
ALTER TABLE run ADD COLUMN "runByCLI" BOOLEAN;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/models/Run.ts` around lines 126 - 133, The Run model now defines
two new nullable boolean fields runByMCP and runByCLI but there is no DB
migration; add a new migration that alters the run table to ADD COLUMN
"runByMCP" BOOLEAN and ADD COLUMN "runByCLI" BOOLEAN (nullable) and implement
the corresponding down migration to DROP those columns so existing databases get
the new columns; reference the Run model (Run.ts) and the run table/column names
when creating the migration.
2 participants
What this PR does?
Tracks the run status for MCP and CLI robot runs by updating the database accordingly.
Summary by CodeRabbit
New Features
Chores