fix(evi): inline dynamic tool executes so resumed sessions keep their tools - #551
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
4 Skipped Deployments
|
|
|
Thank you for following the naming conventions! 🙏 |
|
Warning Review limit reached
Next review available in: 42 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR updates scheduled GitHub approval and prompt handling. It moves dynamic tools from session-startup factories to authorization-gated definitions resolved during ChangesScheduled execution and tool access
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Session
participant TurnStarted
participant canAccessAdminTools
participant DynamicTools
Session->>TurnStarted: Start turn
TurnStarted->>canAccessAdminTools: Check session access
canAccessAdminTools-->>TurnStarted: Return authorization
TurnStarted->>DynamicTools: Define authorized tools inline
DynamicTools->>canAccessAdminTools: Recheck access during execution
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/evi/agent/tools/capture.ts (1)
72-84: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire approval when
approvalCtx.toolInputis missing.
toolInputis optional in the Eve approval context, soapprovalCtx.toolInput?.beforeUrlcan skip sensitivity inspection for both URLs whentoolInputisundefined. That path returns'not-applicable'and can publish public-captured screens without an approval card. Fail closed by returning'user-approval'before inspectingbeforeUrl/afterUrl.</verification Result>
🤖 Prompt for 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. In `@apps/evi/agent/tools/capture.ts` around lines 72 - 84, Update the approval callback around approvalCtx.toolInput to return 'user-approval' immediately when toolInput is missing, before iterating over beforeUrl and afterUrl. Preserve the existing URL sensitivity checks and 'not-applicable' result when toolInput is present and neither URL requires approval.
🤖 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 `@apps/evi/agent/tools/ai-gateway.ts`:
- Around line 79-86: Remove the implementation-history clause about
factory-built maps breaking resumed-session tool execution from the comments at
apps/evi/agent/tools/ai-gateway.ts lines 79-86, apps/evi/agent/tools/blob.ts
lines 9-13, apps/evi/agent/tools/capture.ts lines 48-54, and
apps/evi/agent/tools/turbo.ts lines 10-14. Preserve the durable
inline-definition constraint in all four comments, the bare-tool-name note in
ai-gateway.ts, and the public-URL constraint in capture.ts.
In `@apps/evi/agent/tools/blob.ts`:
- Around line 24-31: Re-check canAccessAdminTools inside execute for every
turn-scoped admin tool, preserving the existing checks in capture.ts and
turbo.ts. In apps/evi/agent/tools/blob.ts lines 24-31, make it the first execute
statement and return the specified unavailable-session error when unauthorized;
in apps/evi/agent/tools/ai-gateway.ts lines 92-98, add the same check to
ai_gateway__credits, ai_gateway__report, and ai_gateway__generation, adding
toolCtx to each execute signature.
In `@apps/evi/agent/tools/turbo.ts`:
- Line 40: Wrap the exchangeTurboToken call in the tool function’s existing
error-handling flow so rejected exchanges do not expose the upstream response
body. Return the same structured { success: false } result used by the other
failure branches, matching the handling already applied to getVercelOidcToken.
---
Outside diff comments:
In `@apps/evi/agent/tools/capture.ts`:
- Around line 72-84: Update the approval callback around approvalCtx.toolInput
to return 'user-approval' immediately when toolInput is missing, before
iterating over beforeUrl and afterUrl. Preserve the existing URL sensitivity
checks and 'not-applicable' result when toolInput is present and neither URL
requires approval.
🪄 Autofix
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 Plus
Run ID: ea25ff60-e714-4fab-ad58-13bca31bfb50
📒 Files selected for processing (8)
apps/evi/agent/extensions/github.tsapps/evi/agent/schedules/digest.tsapps/evi/agent/schedules/upstream-sync.tsapps/evi/agent/tools/ai-gateway.tsapps/evi/agent/tools/blob.tsapps/evi/agent/tools/capture.tsapps/evi/agent/tools/git.tsapps/evi/agent/tools/turbo.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@apps/evi/agent/tools/git.ts`:
- Line 18: Update the comment near the inline resolver execution to state the
durable requirement directly: instruct maintainers to keep execute inline in the
resolver, while retaining the reference to docs/notes.md. Remove the
implementation-history wording “on purpose.”
🪄 Autofix
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 Plus
Run ID: 49090d7a-da72-4cf1-bbd4-79053f8f7be4
📒 Files selected for processing (4)
apps/evi/agent/tools/ai-gateway.tsapps/evi/agent/tools/blob.tsapps/evi/agent/tools/git.tsapps/evi/agent/tools/turbo.ts
1 participant
Production logs from this morning's scheduled runs show the failure eve's docs warn about:
All five dynamic tool files built their tool maps through factory functions; eve's bundler transform only registers
executestep functions it finds inline in the resolver body, so on a resumed session (the long-lived iMessage thread permanently is one) the tools stopped executing and the model degraded to answering around them.ai-gateway,git,blob,capture,turbo: the tool maps now sit inline in a singleturn.startedresolver (which also covers whatsession.starteddid, once per turn instead of once per session).createPullRequestpolicy: a schedule-app turn creating a draft PR no longer parks on an approval card — a draft cannot merge, and marking one ready stays a human act. Non-draft keeps the card. This matches the original upstream-sync contract ("one or more draft PRs, nothing auto-applicable").No changeset: confined to
apps/evi. Verified:tsc, 63 unit tests,eve build. The step-registration warning disappearing from production logs after deploy is the end-to-end check.Summary by CodeRabbit
Bug Fixes
Security
Documentation