feat(agents): expand create/list/show/stop (2/8)#8281
Open
VaibhavAcharya wants to merge 1 commit into
Open
Conversation
Contributor
📝 WalkthroughWalkthroughThis PR converts the agents CLI to operate on "agent runs" using a typed agents API client, replaces direct fetch calls with API methods, and introduces: agents:create with prompt/agent/branch resolvers, attachment uploads, model/deploy/parent options; agents:list with server-side filters, pagination, ndjson, and colored statuses; agents:show with run view, session detail, and --watch live polling; agents:stop with confirmation gating and terminal-state handling. Documentation and integration tests were updated to match the new terminology and behaviors. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 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)
Comment |
VaibhavAcharya
changed the title
VaibhavAcharya
force-pushed
the
agent/cli/pr8237-base
branch
from
c4a30f1 to
5549de6
Compare
VaibhavAcharya
force-pushed
the
agent/cli/pr8237-existing-cmds
branch
from
79a1657 to
c47c842
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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)
docs/commands/agents.md (1)
132-133:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUsage blocks omit required
idargument.Line 132 and Line 165 show bare
agents:show/agents:stop, but both sections now requireid(Line 137 and Line 170). Please update usage examples to include<id>so docs don’t suggest invalid commands.Suggested doc fix
**Usage** ```bash -netlify agents:show +netlify agents:show <id>@@
Usage-netlify agents:stop +netlify agents:stop <id></details> Also applies to: 165-166 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.In
@docs/commands/agents.mdaround lines 132 - 133, Update the usage examples
for the agent commands to include the required id argument: change the bare
"netlify agents:show" and "netlify agents:stop" examples so they read "netlify
agents:show " and "netlify agents:stop " respectively (update the usage
blocks that show the commands for agents:show and agents:stop to include the
placeholder).</details> </blockquote></details> </blockquote></details>🤖 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/commands/agents/agents-list.ts`: - Around line 85-97: The code warns that --branch/--since/--until are ignored when options.account is set but still passes the prebuilt filters to the API; update the logic around filters so that when options.account is truthy you do not send branch/from/to filters to api.listAgentRunnersForAccount: either build a separate empty/clean filters object for account mode or filter out those keys from the existing filters before calling api.listAgentRunnersForAccount (referencing options.account, droppedFilters, filters, and the two API calls api.listAgentRunnersForAccount / api.listAgentRunners) and keep the spinner/startSpinner usage intact. In `@src/commands/agents/agents-stop.ts`: - Around line 33-36: The early-return for terminal runs prints a human message and skips the JSON output path, breaking the --json contract; update the terminal-state branch around runner and TERMINAL_AGENT_STATES so that if the CLI --json flag is set you emit the same JSON representation of the runner that non-terminal flows produce (reuse the same serialization/emit logic used later for successful stops) and then return runner, otherwise keep the existing human-readable chalk.yellow log and return; ensure you reference the existing JSON/emit helper used elsewhere so both paths produce identical JSON structure. In `@tests/integration/commands/agents/agents-show.test.ts`: - Around line 72-76: The tests for the agents:show flow are missing explicit mocked routes that include the query string used by the code (GET /agent_runners/:id/sessions?page=1&per_page=100&order_by=desc), so add mocks that match the full requested URL (including query params) for both the successful case returning [mockAgentSession] and the 404/error case currently omitting sessions; update the existing mocked route with path 'agent_runners/test_id/sessions' to include the exact query parameters (page=1, per_page=100, order_by=desc) and mirror that change for the other test block around the mock that covers lines 94-103 so the sessions endpoint is fully mocked during agents:show tests. --- Outside diff comments: In `@docs/commands/agents.md`: - Around line 132-133: Update the usage examples for the agent commands to include the required id argument: change the bare "netlify agents:show" and "netlify agents:stop" examples so they read "netlify agents:show <id>" and "netlify agents:stop <id>" respectively (update the usage blocks that show the commands for agents:show and agents:stop to include the <id> placeholder).🪄 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: CHILL
Plan: Pro
Run ID:
f018ff25-2583-40be-bd4c-d975293204e4⛔ Files ignored due to path filters (1)
tests/integration/commands/help/__snapshots__/help.test.ts.snapis excluded by!**/*.snap📒 Files selected for processing (11)
docs/commands/agents.mddocs/index.mdsrc/commands/agents/agents-create.tssrc/commands/agents/agents-list.tssrc/commands/agents/agents-show.tssrc/commands/agents/agents-stop.tssrc/commands/agents/agents.tstests/integration/commands/agents/agents-create.test.tstests/integration/commands/agents/agents-list.test.tstests/integration/commands/agents/agents-show.test.tstests/integration/commands/agents/agents-stop.test.ts
Update the existing commands to use the new typed API and shared infra: - create: live model validation, --from-deploy/--parent/--attach, prompt/title sanitization matching the UI - list: --status/--branch/--user/--title/--since/--until/--page/ --per-page/--account filters, --ndjson output, ISO dates, drops the AGENT column to avoid an N+1 session lookup - show: --watch (bounded, rate-limit resilient), --session, richer output (start sha, per-session user/published/discarded badges) - stop: unchanged behavior, new infra Rename user-facing "agent task" to "agent run" throughout, refresh the help snapshot and regenerated docs. Part 2/8 of the agents CLI revamp split.
VaibhavAcharya
force-pushed
the
agent/cli/pr8237-existing-cmds
branch
from
c47c842 to
8a0f826
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/commands/agents/agents-list.ts (1)
85-97:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
--accountmode warns filters are ignored but still sends them.Line 88 warns that
--branch/--since/--untilare ignored with--account, but line 96 still passes the prebuiltfiltersobject (containingbranch/from/to) to the API. This can silently return filtered data while telling users the filters are ignored.Proposed fix
if (options.account) { const droppedFilters = ['branch', 'since', 'until'].filter((key) => options[key as keyof AgentListOptions]) if (droppedFilters.length > 0) { log(chalk.yellow(`⚠ --${droppedFilters.join(', --')} are ignored when --account is set.`)) } + delete filters.branch + delete filters.from + delete filters.to }🤖 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/commands/agents/agents-list.ts` around lines 85 - 97, The warning about ignored filters is correct but the code still passes the prebuilt filters object to api.listAgentRunnersForAccount; update the call site so that when options.account is set you pass an empty/undefined filters payload instead of the existing filters object. Locate the branch where result is assigned (the ternary using options.account and the functions api.listAgentRunnersForAccount and api.listAgentRunners) and change the account branch to call api.listAgentRunnersForAccount(options.account, undefined) or an empty filtered object so branch/from/to are not sent; keep the non-account branch unchanged.
🧹 Nitpick comments (1)
docs/commands/agents.md (1)
68-68: 💤 Low valueMinor wording improvement.
The phrase "off of" could be simplified to "from" for cleaner documentation.
Suggested change
-- `parent` (*string*) - chain this agent run off of another agent run +- `parent` (*string*) - chain this agent run from another agent run🤖 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 `@docs/commands/agents.md` at line 68, Update the docs line describing the `parent` parameter to replace the phrase "chain this agent run off of another agent run" with clearer wording such as "chain this agent run from another agent run" and ensure the `parent` parameter remains marked as (*string*) for consistency.
🤖 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/commands/agents/agents-show.ts`:
- Line 248: The header construction in agents-show.ts can crash because
session.prompt may be undefined; update the header assignment (the const header
using session.title and session.prompt.slice) to safely handle missing
prompt/title by using a fallback string and safe slicing (e.g., use
(session.prompt ?? '').slice(0,80) or optional chaining plus a default like
session.title ?? (session.prompt ?? '')) so the template never calls slice on
undefined and displays a sensible fallback when both title and prompt are
absent.
---
Duplicate comments:
In `@src/commands/agents/agents-list.ts`:
- Around line 85-97: The warning about ignored filters is correct but the code
still passes the prebuilt filters object to api.listAgentRunnersForAccount;
update the call site so that when options.account is set you pass an
empty/undefined filters payload instead of the existing filters object. Locate
the branch where result is assigned (the ternary using options.account and the
functions api.listAgentRunnersForAccount and api.listAgentRunners) and change
the account branch to call api.listAgentRunnersForAccount(options.account,
undefined) or an empty filtered object so branch/from/to are not sent; keep the
non-account branch unchanged.
---
Nitpick comments:
In `@docs/commands/agents.md`:
- Line 68: Update the docs line describing the `parent` parameter to replace the
phrase "chain this agent run off of another agent run" with clearer wording such
as "chain this agent run from another agent run" and ensure the `parent`
parameter remains marked as (*string*) for consistency.
🪄 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: CHILL
Plan: Pro
Run ID: c87c73fa-4722-4078-9c81-da92dfbbad21
⛔ Files ignored due to path filters (1)
tests/integration/commands/help/__snapshots__/help.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
docs/commands/agents.mddocs/index.mdsrc/commands/agents/agents-create.tssrc/commands/agents/agents-list.tssrc/commands/agents/agents-show.tssrc/commands/agents/agents-stop.tssrc/commands/agents/agents.tstests/integration/commands/agents/agents-create.test.tstests/integration/commands/agents/agents-list.test.tstests/integration/commands/agents/agents-show.test.tstests/integration/commands/agents/agents-stop.test.ts
✅ Files skipped from review due to trivial changes (2)
- docs/index.md
- tests/integration/commands/agents/agents-show.test.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- tests/integration/commands/agents/agents-stop.test.ts
- src/commands/agents/agents.ts
- tests/integration/commands/agents/agents-list.test.ts
- tests/integration/commands/agents/agents-create.test.ts
- src/commands/agents/agents-stop.ts
Contributor
Author
|
Link EX-2195 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Linear: https://linear.app/netlify/issue/EX-2195
Part 2 of the agents CLI revamp split. Updates the original
create/list/show/stopcommands to use the new typed API, and renames user-facing "agent task" to "agent run" throughout (help, docs, output).create: live model validation,--from-deploy/--parent/--attach, title and prompt sanitization matching the UI.list:--status/--branch/--user/--title/--since/--until/--page/--per-page/--accountfilters,--ndjsonoutput, ISO dates, and drops the AGENT column to remove an N+1 session lookup. Status filter values match the UI dropdown (running | done | error | archived).show:--watch(bounded retries, rate-limit resilient),--session, and richer output (start sha, per-session user/published/discarded badges).stop: same behavior on the new infra.Sample output:
Stacked on the base infra PR.
For us to review and ship your PR efficiently, please perform the following steps:
passes our tests.