feat: add AgentRunActivityAction type and action field to activity in…#41
feat: add AgentRunActivityAction type and action field to activity in…#41
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a nullable ChangesAgent Run models + API update
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
🧹 Nitpick comments (1)
src/models/AgentRun.ts (1)
110-123: ⚡ Quick winAdd unit coverage for
actionround-trip behavior.Nice type alignment on both request/response. Please add a focused unit test that creates an activity with top-level
actionand asserts it is sent/parsed as expected (current activity-create test covers content action type, but not this new top-level field).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/models/AgentRun.ts` around lines 110 - 123, Add a unit test that exercises the top-level action round-trip: construct a CreateAgentRunActivityRequest with its action field set to a non-null AgentRunActivityAction, call the same activity creation helper used by the existing activity-create test, assert the outgoing request payload contains the top-level action, and assert the parsed response includes the same action value (verify both serialization and deserialization). Target the code that builds/parses agent activities (use CreateAgentRunActivityRequest, AgentRunActivityAction, and the activity create/parse helpers used in the current tests) and keep the test focused only on the top-level action behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/models/AgentRun.ts`:
- Around line 110-123: Add a unit test that exercises the top-level action
round-trip: construct a CreateAgentRunActivityRequest with its action field set
to a non-null AgentRunActivityAction, call the same activity creation helper
used by the existing activity-create test, assert the outgoing request payload
contains the top-level action, and assert the parsed response includes the same
action value (verify both serialization and deserialization). Target the code
that builds/parses agent activities (use CreateAgentRunActivityRequest,
AgentRunActivityAction, and the activity create/parse helpers used in the
current tests) and keep the test focused only on the top-level action behavior.
…terfaces Adds AgentRunActivityAction interface (name + redirect_url) and an optional action field to AgentRunActivity and CreateAgentRunActivityRequest, so agents can attach CTA buttons to response activities. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
0cb4f1a to
0d29ff7
Compare
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/models/AgentRun.ts (1)
68-70: ⚡ Quick winDerive
UpdateAgentRunRequestfrom the model to reduce DTO drift.This DTO is small now, but deriving it via
Pick/Partialkeeps it aligned withAgentRunover time and follows the models convention.Proposed refactor
-export interface UpdateAgentRunRequest { - external_link?: string | null; -} +export interface UpdateAgentRunRequest extends Partial<Pick<AgentRun, "external_link">> { + external_link?: string | null; +}As per coding guidelines,
src/models/**/*.ts: Use TypeScript interfaces for entity models with separate Create/Update DTOs usingPick,Omit, andPartial.🤖 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 `@src/models/AgentRun.ts` around lines 68 - 70, UpdateAgentRunRequest is defined manually and may drift from the AgentRun model; change its definition to derive from the AgentRun interface using TypeScript utility types (e.g., Pick and Partial) so it stays aligned. Locate the AgentRun interface and replace the explicit UpdateAgentRunRequest fields with a derived type such as Partial<Pick<AgentRun, 'external_link'>> (or the appropriate property set) so the DTO follows the models convention and will update automatically if AgentRun changes.
🤖 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 `@src/models/AgentRun.ts`:
- Line 117: Change the top-level property from an array to a nullable singular
field: replace occurrences of actions?: AgentRunActivityAction[] with action?:
AgentRunActivityAction | null in the AgentRun model so the type matches the API
contract; update the declarations referenced (the property at/around the
AgentRun interface/class and the other occurrence currently named actions) to
use the singular name "action" and allow nullability instead of an array so
callers passing a single action will type-check correctly.
---
Nitpick comments:
In `@src/models/AgentRun.ts`:
- Around line 68-70: UpdateAgentRunRequest is defined manually and may drift
from the AgentRun model; change its definition to derive from the AgentRun
interface using TypeScript utility types (e.g., Pick and Partial) so it stays
aligned. Locate the AgentRun interface and replace the explicit
UpdateAgentRunRequest fields with a derived type such as Partial<Pick<AgentRun,
'external_link'>> (or the appropriate property set) so the DTO follows the
models convention and will update automatically if AgentRun changes.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 06a07e8b-07d7-4f54-a67e-28fd06eabf03
📒 Files selected for processing (2)
src/api/AgentRuns/index.tssrc/models/AgentRun.ts
| type: AgentRunActivityType; | ||
| project?: string; | ||
| workspace: string; | ||
| actions?: AgentRunActivityAction[]; |
There was a problem hiding this comment.
Use singular nullable action here to match the intended API contract.
Line 117 and Line 130 currently expose actions?: AgentRunActivityAction[], but the feature goal is a single top-level CTA (action?: AgentRunActivityAction | null). With the current shape, callers passing action still won’t type-check as intended.
Proposed fix
export interface AgentRunActivity extends BaseModel {
@@
- actions?: AgentRunActivityAction[];
+ action?: AgentRunActivityAction | null;
}
@@
export interface CreateAgentRunActivityRequest {
@@
- actions?: AgentRunActivityAction[];
+ action?: AgentRunActivityAction | null;
}Also applies to: 130-130
🤖 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 `@src/models/AgentRun.ts` at line 117, Change the top-level property from an
array to a nullable singular field: replace occurrences of actions?:
AgentRunActivityAction[] with action?: AgentRunActivityAction | null in the
AgentRun model so the type matches the API contract; update the declarations
referenced (the property at/around the AgentRun interface/class and the other
occurrence currently named actions) to use the singular name "action" and allow
nullability instead of an array so callers passing a single action will
type-check correctly.
…t top-level fields Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
AgentRunActivityActioninterface withnameandredirect_urlfieldsaction?: AgentRunActivityAction | nulltoAgentRunActivity(response shape from API)action?: AgentRunActivityAction | nulltoCreateAgentRunActivityRequest(request shape to API)Context
Agents need to attach a CTA button to response activities — for example, nudging a user to connect their account when a personal API key is missing. This button is a top-level
actionon the activity, separate fromcontent, so the frontend can render it independently of the response body.Without this type, passing
actiontoplaneClient.agentRuns.activities.create(...)is rejected as an unknown property.Changes