Execute direct MCP steps in test pipeline#69
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors the test-pipeline route to unify execution of all MCP steps (both code generation and non-code actions) through a single ChangesUnified pipeline step execution
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
🚥 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/api/test-pipeline/route.ts`:
- Around line 295-299: preparedSteps currently uses model-produced step.action
directly; enforce a fail-closed allowlist by validating step.action against the
MCP_TOOLS set before adding it to preparedSteps (in the block that constructs
the step object where planStepNumber, step.action, step.parameters, and
step.rationale/expected_outcome are used). If step.action is not present in
MCP_TOOLS, replace the action with a safe no-op identifier (e.g., "noop" or
"disallowed_action") and annotate params or description with a warning (or
alternatively throw/reject depending on desired behavior) so no unallowlisted
tool will be invoked at runtime; apply the same validation where the other
prepared step construction occurs to cover both occurrences.
- Around line 351-365: The logging call can throw when JSON.stringify encounters
circular refs or returns undefined, causing a successful step to be treated as
an error; fix by precomputing a safe, non-throwing preview string before calling
agentMonitor.log: create a short helper or inline safe serialization for
stepResult.output (e.g., try to JSON.stringify and slice, but catch exceptions
and fallback to a string like "[unserializable]" or String(value), and treat
undefined as "undefined"), then pass that safe preview to agentMonitor.log
(references: agentMonitor.log, stepResult, executable.planStepNumber,
executionResults.push) so the stringify cannot escape into the step try/catch
and falsely mark the step as failed.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 35ebb7d6-aec0-461c-8be9-b09344a07508
📒 Files selected for processing (2)
app/api/test-pipeline/route.tsscripts/test/test-test-pipeline-direct-step-execution.ts
Summary
Validation
Summary by CodeRabbit
Bug Fixes
Tests