[SILO-713] feat: add Agent Runs API and related models, activities, and tests #22
Conversation
|
Linked to Plane Work Item(s) This comment was auto-generated by Plane |
WalkthroughAdds a new Agent Runs feature: typed AgentRun models, API resources for agent runs and their activities, PlaneClient integration, unit tests, and a TEST_AGENT_SLUG configuration entry. Changes
Sequence DiagramsequenceDiagram
actor User
participant PlaneClient
participant AgentRuns
participant Activities
participant API
User->>PlaneClient: instantiate client (config)
PlaneClient->>AgentRuns: new AgentRuns(config)
PlaneClient->>Activities: new Activities(config)
AgentRuns->>Activities: this.activities = new Activities(config)
User->>PlaneClient: agentRuns.create(workspaceSlug, data)
PlaneClient->>AgentRuns: create(workspaceSlug, data)
AgentRuns->>API: POST /workspaces/{workspaceSlug}/runs/
API-->>AgentRuns: AgentRun
AgentRuns-->>PlaneClient: Promise<AgentRun>
PlaneClient-->>User: AgentRun
User->>PlaneClient: agentRuns.retrieve(workspaceSlug, runId)
PlaneClient->>AgentRuns: retrieve(workspaceSlug, runId)
AgentRuns->>API: GET /workspaces/{workspaceSlug}/runs/{runId}/
API-->>AgentRuns: AgentRun
AgentRuns-->>PlaneClient: Promise<AgentRun>
PlaneClient-->>User: AgentRun
User->>PlaneClient: agentRuns.activities.list(workspaceSlug, runId)
PlaneClient->>Activities: list(workspaceSlug, runId, params?)
Activities->>API: GET /workspaces/{workspaceSlug}/runs/{runId}/activities/
API-->>Activities: PaginatedResponse<AgentRunActivity>
Activities-->>PlaneClient: Promise<PaginatedResponse<AgentRunActivity>>
PlaneClient-->>User: Activities list
User->>PlaneClient: agentRuns.activities.create(workspaceSlug, runId, data)
PlaneClient->>Activities: create(workspaceSlug, runId, data)
Activities->>API: POST /workspaces/{workspaceSlug}/runs/{runId}/activities/
API-->>Activities: AgentRunActivity
Activities-->>PlaneClient: Promise<AgentRunActivity>
PlaneClient-->>User: AgentRunActivity
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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.
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.
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.
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.
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.