-
Notifications
You must be signed in to change notification settings - Fork 4
[SILO-713] feat: add Agent Runs API and related models, activities, and tests #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
📝 WalkthroughWalkthroughAdds Agent Runs: typed AgentRun models, API resources for runs and activities, PlaneClient integration, unit tests, and TEST_AGENT_SLUG test configuration entries. Changes
Sequence DiagramsequenceDiagram
autonumber
actor User
participant PlaneClient as PlaneClient
participant AgentRuns as AgentRuns
participant Activities as Activities
participant API as BackendAPI
rect rgb(235,245,255)
User->>PlaneClient: instantiate client(config)
PlaneClient->>AgentRuns: new AgentRuns(config)
PlaneClient->>Activities: new Activities(config)
AgentRuns->>Activities: this.activities = Activities
end
rect rgb(245,255,235)
User->>PlaneClient: agentRuns.create(workspaceSlug, data)
PlaneClient->>AgentRuns: create(workspaceSlug, data)
AgentRuns->>API: POST /workspaces/{ws}/runs/
API-->>AgentRuns: 201 AgentRun
AgentRuns-->>PlaneClient: Promise<AgentRun>
PlaneClient-->>User: AgentRun
end
rect rgb(255,250,235)
User->>PlaneClient: agentRuns.activities.create(ws, runId, data)
PlaneClient->>Activities: create(ws, runId, data)
Activities->>API: POST /workspaces/{ws}/runs/{runId}/activities/
API-->>Activities: 201 AgentRunActivity
Activities-->>PlaneClient: Promise<AgentRunActivity>
PlaneClient-->>User: AgentRunActivity
end
rect rgb(255,240,245)
User->>PlaneClient: agentRuns.activities.list(ws, runId, params?)
PlaneClient->>Activities: list(ws, runId, params?)
Activities->>API: GET /workspaces/{ws}/runs/{runId}/activities/?...
API-->>Activities: PaginatedResponse<AgentRunActivity>
Activities-->>PlaneClient: Promise<PaginatedResponse>
PlaneClient-->>User: Activities page
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/agent-runs/agent-runs.test.ts (1)
7-46: Avoid inter-test coupling by creating the agent run in shared setupThe suite gives good coverage of create/retrieve/resume, but
agentRunis initialized inside"should create an agent run"and is then reused by the retrieve/resume tests. This couples test outcomes and makes it harder to run tests in isolation or to debug when the first test fails.Consider moving the
client.agentRuns.create(...)call intobeforeAll(or a dedicated setup block) and having eachitonly assert behavior against that sharedagentRun. That keeps create exercised while making tests less order-dependent.tests/unit/agent-runs/activities.test.ts (1)
7-118: Reduce inter-test coupling around the sharedactivityinstanceThese tests nicely cover create/list/retrieve and multiple activity types, but the
"list"and"retrieve"cases depend onactivitybeing created in the first"should create an agent run activity"test. This implicit ordering can make tests brittle if run individually or reordered.Consider moving creation of the baseline
activityintobeforeAll(or a small nested describe with its own setup) so that each test’s expectations don’t rely on another test’s side effects.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
env.example(1 hunks)src/api/AgentRuns/Activities.ts(1 hunks)src/api/AgentRuns/index.ts(1 hunks)src/client/plane-client.ts(3 hunks)src/index.ts(2 hunks)src/models/AgentRun.ts(1 hunks)src/models/index.ts(1 hunks)tests/unit/agent-runs/activities.test.ts(1 hunks)tests/unit/agent-runs/agent-runs.test.ts(1 hunks)tests/unit/constants.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/client/plane-client.ts (2)
src/api/AgentRuns/index.ts (1)
AgentRuns(10-47)src/index.ts (1)
AgentRuns(30-30)
src/api/AgentRuns/index.ts (2)
src/api/AgentRuns/Activities.ts (1)
Activities(10-54)src/models/AgentRun.ts (2)
CreateAgentRunRequest(55-63)AgentRun(34-50)
tests/unit/agent-runs/agent-runs.test.ts (4)
tests/unit/constants.ts (1)
config(1-11)src/client/plane-client.ts (1)
PlaneClient(27-83)src/index.ts (1)
PlaneClient(2-2)src/models/AgentRun.ts (1)
AgentRun(34-50)
src/api/AgentRuns/Activities.ts (2)
src/index.ts (3)
Activities(36-36)Activities(47-47)BaseResource(8-8)src/models/AgentRun.ts (2)
AgentRunActivity(90-102)CreateAgentRunActivityRequest(107-114)
🪛 GitHub Actions: Build and Test Node SDK
src/models/AgentRun.ts
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🔇 Additional comments (16)
env.example (1)
25-27: Agent configuration env var matches test usageAdding
TEST_AGENT_SLUGunder a dedicated “Agent configuration” section keeps test config organized and matches the newconfig.agentSlugusage in tests. No issues from my side.src/client/plane-client.ts (1)
21-21: AgentRuns integration into PlaneClient is consistentThe
AgentRunsimport, publicagentRunsproperty, and constructor initialization mirror the existing resource patterns and keep the client surface coherent. Looks good.Also applies to: 48-48, 81-81
tests/unit/constants.ts (1)
10-10: agentSlug config wiring aligns with env.example and testsExposing
agentSlugfromTEST_AGENT_SLUGkeeps configuration consistent with other IDs/slugs and supports the new agent run tests. No further changes needed.src/models/index.ts (1)
17-17: Barrel export for AgentRun is correctRe-exporting
./AgentRunhere is consistent with the rest of the models index and allows clean imports in tests and consumers.src/index.ts (1)
30-30: Public exports for AgentRuns and AgentRunActivities look goodExposing
AgentRunsand aliasingActivitiesasAgentRunActivitiesmatches the existing export style for other resources/sub-resources and makes the new API surface discoverable from the package root.Also applies to: 47-47
src/api/AgentRuns/index.ts (1)
1-47: AgentRuns API resource is consistent and well-scopedThe
AgentRunsresource, itsactivitiessub-resource, and thecreate/retrieve/resumemethods all follow the established BaseResource pattern and use the expected endpoint shapes. The typing withAgentRunandCreateAgentRunRequestlooks appropriate. No changes requested.src/api/AgentRuns/Activities.ts (4)
1-13: LGTM! Class setup follows established patterns.The imports, class declaration, and constructor are correctly structured. The constructor simply delegates to
BaseResource, which is a common pattern in this codebase.
15-31: LGTM! Thelistmethod is well-implemented.Proper pagination support with optional
per_pageandcursorparameters, correct return type, and clear JSDoc documentation.
33-42: LGTM! Theretrievemethod is clean and correct.Single resource retrieval with proper typing and documentation.
44-54: LGTM! Thecreatemethod correctly posts activity data.The method properly uses
CreateAgentRunActivityRequestfor input and returns the createdAgentRunActivity.src/models/AgentRun.ts (6)
3-14: LGTM! Status type covers the agent run lifecycle well.The
AgentRunStatusunion type provides comprehensive coverage of possible run states including creation, execution, completion, and failure scenarios.
16-29: LGTM! Activity-related type definitions are clear.The signal and activity type unions are well-defined.
AgentRunTypebeing a single value currently is fine - it can be extended when new run types are added.
31-50: LGTM! AgentRun interface is comprehensive.All necessary fields for tracking agent runs are present, including timing fields, status, metadata, and relationships to workspace/project/issue.
52-63: LGTM! Create request interface is appropriately minimal.Only
agent_slugis required; other fields are optional, providing flexibility for different use cases.
65-85: LGTM! Discriminated union for activity content is well-designed.The
typefield serves as the discriminant, enabling TypeScript to narrow the type correctly when checkingcontent.type === "action"vs other types.
87-114: LGTM! Activity interfaces correctly model the domain.
AgentRunActivityextendsBaseModelappropriately, andCreateAgentRunActivityRequestcorrectly excludes"prompt"from allowed types, enforcing that prompts are system-generated rather than user-created.
| @@ -0,0 +1,115 @@ | |||
| import { BaseModel } from "./common"; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run Prettier to fix formatting issues.
The pipeline indicates a Prettier formatting check failure for this file. Run prettier --write src/models/AgentRun.ts to fix the code style issues.
🧰 Tools
🪛 GitHub Actions: Build and Test Node SDK
[error] 1-1: Prettier formatting check failed. Run 'prettier --write' to fix code style issues in this file.
🤖 Prompt for AI Agents
In src/models/AgentRun.ts at line 1, the file fails the repository Prettier
formatting check; run the formatter and apply the changes. Execute: prettier
--write src/models/AgentRun.ts (or run your editor/IDE Prettier integration),
stage the updated file, and commit the formatted changes so the CI Prettier
check passes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tests/unit/agent-runs/agent-runs.test.ts (1)
16-28: Consider adding cleanup in afterAll.The test creates a work item and comment in
beforeAllbut doesn't clean them up. This could lead to resource accumulation in the test environment.🔎 Add cleanup logic:
afterAll(async () => { if (workItem?.id) { await client.workItems.delete(workspaceSlug, projectId, workItem.id); } });src/api/AgentRuns/index.ts (1)
10-37: Consider adding list method for completeness.The AgentRuns resource currently only supports create and retrieve operations. Consider adding a list method to retrieve all agent runs for a workspace, similar to the Activities resource pattern.
Example implementation:
/** * List agent runs for a workspace * @param workspaceSlug - The workspace slug * @param params - Optional query parameters for pagination * @returns Paginated list of agent runs */ async list( workspaceSlug: string, params?: { per_page?: number; cursor?: string } ): Promise<PaginatedResponse<AgentRun>> { return this.get<PaginatedResponse<AgentRun>>( `/workspaces/${workspaceSlug}/runs/`, params ); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
env.example(2 hunks)src/api/AgentRuns/index.ts(1 hunks)tests/unit/agent-runs/activities.test.ts(1 hunks)tests/unit/agent-runs/agent-runs.test.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/agent-runs/activities.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
src/api/AgentRuns/index.ts (2)
src/api/AgentRuns/Activities.ts (1)
Activities(10-54)src/models/AgentRun.ts (2)
CreateAgentRunRequest(55-63)AgentRun(34-50)
tests/unit/agent-runs/agent-runs.test.ts (3)
tests/unit/constants.ts (1)
config(1-11)src/client/plane-client.ts (1)
PlaneClient(27-83)src/models/AgentRun.ts (1)
AgentRun(34-50)
🔇 Additional comments (6)
tests/unit/agent-runs/agent-runs.test.ts (3)
1-6: LGTM!Imports are well-organized and appropriate for the test suite. The conditional test pattern using
describeIfis a good practice.
30-41: LGTM!The test correctly verifies agent run creation with appropriate assertions on id, status, and workspace.
43-49: LGTM!The retrieval test correctly verifies that the agent run can be fetched by ID. The test dependency on the previous test is acceptable for integration tests.
src/api/AgentRuns/index.ts (3)
1-16: LGTM!The class structure follows the established pattern in the codebase. The nested Activities resource is properly initialized with the same configuration.
18-26: LGTM!The create method is well-documented and follows RESTful conventions. The URL pattern and types are consistent with the codebase.
28-36: LGTM!The retrieve method is well-implemented with proper documentation and type safety. The URL pattern correctly includes both workspace slug and run ID.
|
|
||
| # Agent configuration | ||
| TEST_AGENT_SLUG=your-agent-slug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove duplicate TEST_AGENT_SLUG declaration.
The TEST_AGENT_SLUG variable is already declared at lines 11-12. This duplication will confuse users and could lead to configuration errors.
🔎 Apply this diff to remove the duplicate:
-# Agent configuration
-TEST_AGENT_SLUG=your-agent-slug📝 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.
| # Agent configuration | |
| TEST_AGENT_SLUG=your-agent-slug |
🤖 Prompt for AI Agents
In env.example around lines 28 to 30, there's a duplicate TEST_AGENT_SLUG
declaration that repeats the one at lines 11-12; remove the second declaration
(lines 28-30) so TEST_AGENT_SLUG is defined only once, leaving the original
entry at lines 11-12 intact and ensuring no extra blank lines or stray
whitespace remain.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| TEST_CUSTOM_TEXT_PROPERTY_ID=your-custom-text-property-id | ||
|
|
||
| # Agent configuration | ||
| TEST_AGENT_SLUG=your-agent-slug |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate TEST_AGENT_SLUG environment variable in env.example
The TEST_AGENT_SLUG environment variable is defined twice in env.example with different placeholder values (your-installed-agent-slug at line 12 and your-agent-slug at line 30). This appears to be an accidental duplication. When developers copy this file to create their test configuration, the second value will override the first, leading to confusion about which placeholder description is correct.
Additional Locations (1)
| signal_metadata?: Record<string, any>; | ||
| type: Exclude<AgentRunActivityType, "prompt">; | ||
| project?: string; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Type inconsistency allows prompt content type in create request
The CreateAgentRunActivityRequest interface has a type mismatch. The type field excludes "prompt" via Exclude<AgentRunActivityType, "prompt">, but the content field uses AgentRunActivityContent which allows content.type to be "prompt" (via AgentRunActivityTextContent). This lets TypeScript accept requests where content.type is "prompt" while the activity type is something else. Based on test patterns, these values are expected to match, so the types don't properly enforce this constraint, leading to potential API validation errors at runtime.
Additional Locations (1)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/models/AgentRun.ts (1)
48-48: Consider usingunknowninstead ofanyfor metadata fields.Multiple metadata fields are typed as
Record<string, any>, which bypasses TypeScript's type checking. While dynamic metadata often requires flexible typing, consider usingRecord<string, unknown>to maintain some type safety—consumers would need to perform runtime checks or type assertions before using the values.This is a minor type safety improvement and can be deferred if the current approach meets your needs.
Also applies to: 93-93, 96-96, 109-109, 111-111
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
package.jsonsrc/models/AgentRun.ts
✅ Files skipped from review due to trivial changes (1)
- package.json
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
src/models/AgentRun.ts (3)
68-72: Consider whether string-only parameters are sufficient.Line 71 restricts
parameterstoRecord<string, string>, meaning all action parameter values must be strings. If actions need to pass numbers, booleans, arrays, or nested objects, they would require string serialization/deserialization.Consider whether this constraint is intentional or if a more flexible type like
Record<string, unknown>orRecord<string, string | number | boolean>would better serve the API's needs.
74-85: Well-designed discriminated union pattern.The use of
Exclude<AgentRunActivityType, "action">paired with the union type creates a type-safe discriminated union. This ensures that action-type activities useAgentRunActivityActionContentwhile all other types useAgentRunActivityTextContent.
107-114: Prompt activity type exclusion is intentional and correct.The exclusion of
"prompt"fromCreateAgentRunActivityRequest(line 112) is by design. The API prevents clients from creating prompt activities—they can only be generated by the backend in response to specific triggers. The fullAgentRunActivitymodel includes the"prompt"type, allowing clients to receive and read prompt activities, but the creation request excludes it, maintaining a clear separation between system-generated and user-created activities.No changes needed.
Description
Type of Change
Screenshots and Media (if applicable)
Test Scenarios
References
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.
Note
Introduces Agent Runs as a first-class API with nested Activities, plus supporting models, client wiring, tests, and config updates.
AgentRuns(create/retrieve) with sub-resourceAgentRunActivities(list/retrieve/create)AgentRun,AgentRunActivity, request payloads, and enums for statuses, types, and signalsagentRunstoPlaneClientand exportAgentRuns/AgentRunActivitiesinsrc/index.tsTEST_AGENT_SLUGinenv.exampleand test constants; bump package version to0.2.5Written by Cursor Bugbot for commit fb36274. This will update automatically on new commits. Configure here.