Repository navigation
feat(cli): migrate ACP runtime to acp-kit with agent management - #8
Merged
Merged
Conversation
Move OpenSpec slash commands to flat opsx-*.md files for dotagents deploy, add Cursor as a deploy target, and ignore generated .agents/ and .cursor/ dirs. Co-authored-by: Cursor <cursoragent@cursor.com>
Define the process manager, session router, agent config, and CLI requirements for running ACP agents in the Cyrus worker. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace hello with listAgents and add agentName, threadId, and cwd to the chat input schema so controllers can target registered ACP agents. Co-authored-by: Cursor <cursoragent@cursor.com>
Relocate device config persistence to store/config, extract name and agent validators, and add small io/error helpers used by ACP runtime. Co-authored-by: Cursor <cursoragent@cursor.com>
Persist registered ACP agents in agents.yml and expose add, list, update, rm, and doctor subcommands under cyrusd agents. Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce the ACP SDK integration with subprocess lifecycle management, health pings, and per-thread session routing for agent prompts. Co-authored-by: Cursor <cursoragent@cursor.com>
Route chat prompts through the session router and expose listAgents so controllers can discover registered agents on the local worker. Co-authored-by: Cursor <cursoragent@cursor.com>
…input Extend the controller contract with model/mode/effort/persona getters and setters plus listProjects, and define shared chat event schemas for streaming agent output. Co-authored-by: Cursor <cursoragent@cursor.com>
Introduce acp-kit as the dependency for subprocess transport, session management, and normalized agent runtime events. Co-authored-by: Cursor <cursoragent@cursor.com>
Replace the hand-rolled ACP client and process manager with acp-kit-backed
modules under core/{acp,agents,threads}, including pool lifecycle, event mapping,
thread coordination, and a mock projectId-to-cwd resolver.
Co-authored-by: Cursor <cursoragent@cursor.com>
Connect controller RPC handlers to the new runtime, route chat through threadCoordinator, and update doctor ping to use the acp-kit health check. Co-authored-by: Cursor <cursoragent@cursor.com>
Align the web thread caller with the updated RTC chat input schema. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 15 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (28)
📝 WalkthroughWalkthroughThis PR introduces an ACP (Agent Client Protocol) provider runtime for the Cyrus CLI: new schemas and controller contract endpoints for agents/projects/models/modes, a YAML-backed agent registry, an AgentPool managing subprocess lifecycles, AgentRuntime/ThreadCoordinator for session prompting, new CLI agent commands, controller router rewiring, web chat client updates to typed AgentEvents, config-import path migrations, and OpenSpec documentation. ChangesACP Provider Runtime
Repository tooling and config tweaks
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Resolve modify/delete conflict by dropping the legacy /threads route; main replaced it with the workspace shell that already routes by projectId. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Nitpick comments (9)
apps/cli/src/validators/acp.ts (1)
7-25: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDuplicated zod→InvalidArgumentError pattern.
commandArgParserandnameArgParserrepeat identical safeParse/throw boilerplate. Consider a small generic helper to reduce duplication.♻️ Suggested consolidation
+function zodArgParser<T>(schema: z.ZodType<T>, fallbackMessage: string) { + return (value: string): T => { + const result = schema.safeParse(value); + if (!result.success) { + throw new InvalidArgumentError( + result.error.issues[0]?.message ?? fallbackMessage + ); + } + return result.data; + }; +} + +export const commandArgParser = zodArgParser(commandSchema, "invalid command"); +export const nameArgParser = zodArgParser(nameSchema, "invalid name");🤖 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/cli/src/validators/acp.ts` around lines 7 - 25, Both commandArgParser and nameArgParser duplicate the same safeParse-to-InvalidArgumentError flow. Extract the repeated validation/throw logic into a small shared helper that accepts a schema and fallback message, then have commandArgParser and nameArgParser delegate to it while preserving their current messages and return types. Use the existing symbols commandSchema, nameSchema, and InvalidArgumentError to keep the refactor localized.apps/cli/src/commands/agents/update.ts (1)
22-38: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant re-merge in
updateAgent.
entryis already fully merged withexistingbefore being passed toupdateAgent, which merges again (partial.command ?? existing.command) per the store snippet. Not a bug, but the double merge is unnecessary — mainly needed here becauseentrydoubles as the display payload for the success message.🤖 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/cli/src/commands/agents/update.ts` around lines 22 - 38, The `updateAgent` call in `agents/update.ts` is redundantly merging data twice because `entry` is already fully resolved from `existing`. Adjust the `update` command so `entry` is only used as the display payload for the success message, and pass a partial patch object into `updateAgent` instead of the already merged `AgentEntry`. Keep the success output logic tied to the local `entry` variable, and ensure the `saved.match` handling remains unchanged.apps/cli/src/commands/config/index.ts (1)
2-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueCross-context reuse of
nameArgParserfromvalidators/acp.ts.Device rename names now share validation with agent names via
nameArgParser, which is defined inacp.ts— a file otherwise scoped to ACP agent command/name validation. Not incorrect, but the naming/location may confuse future readers since this validator is now used outside the ACP domain.🤖 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/cli/src/commands/config/index.ts` around lines 2 - 9, The `registerConfigCommands` rename flow is reusing `nameArgParser` from `validators/acp`, but that validator is now shared outside ACP and its current name/location is misleading. Move the shared name validation to a more generic validator module or rename/export it to reflect broader device-name use, and update the `registerConfigCommands` import so the `rename` command clearly points to a cross-domain validator.apps/cli/src/commands/agents/add.ts (1)
1-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate result-handling pattern across agent commands.
The
saved.match({ ok: ..., err: (message) => { print.error...; process.exit(1); } })pattern is repeated verbatim inrm.ts(and likelyupdate.ts). Consider extracting a shared helper to avoid drift.♻️ Suggested helper
// apps/cli/src/utils/cli.ts import { print } from "`@/utils/style`"; import type { Result } from "better-result"; export function printResultOrExit<T>( result: Result<T, string>, onOk: (value: T) => void ): void { result.match({ ok: onOk, err: (message) => { print.error`${message}`; process.exit(1); }, }); }- saved.match({ - ok: () => { - const args = entry.args.length > 0 ? ` ${entry.args.join(" ")}` : ""; - print.success`✓ added agent "${name}" with command ${entry.command}${args}`; - }, - err: (message) => { - print.error`${message}`; - process.exit(1); - }, - }); + printResultOrExit(saved, () => { + const args = entry.args.length > 0 ? ` ${entry.args.join(" ")}` : ""; + print.success`✓ added agent "${name}" with command ${entry.command}${args}`; + });🤖 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/cli/src/commands/agents/add.ts` around lines 1 - 17, The result-handling in add() duplicates the same ok/err pattern used by other agent commands, so extract it into a shared helper to keep behavior consistent. Create a reusable function like printResultOrExit in a common CLI utility module, then update add() to use it with the addAgent result and keep the success callback for the success message while centralizing the error printing and process.exit(1) path.apps/cli/src/core/acp/pool.ts (1)
99-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueType the placeholder
runtimefield as optional instead of an unsafe cast.
runtime: undefined as unknown as AcpRuntime(line 67, referenced fromgetRuntime) silences the type checker rather than modeling the actual optionality. Considerruntime?: AcpRuntimeonManagedAgentto make the "not yet booted" state type-safe.🤖 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/cli/src/core/acp/pool.ts` around lines 99 - 149, The ManagedAgent placeholder runtime is being modeled with an unsafe cast instead of the actual “not yet booted” state. Update the ManagedAgent shape used by getRuntime and bootRuntime so runtime is optional (runtime?: AcpRuntime), then adjust the initialization and assignment sites in bootRuntime, resetIdleTimer, and stopAgent to work with the optional field safely without using undefined as unknown as AcpRuntime.apps/cli/src/core/acp/events.ts (1)
54-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMerge identical
tool.update/tool.endcases.Both branches are byte-for-byte identical. Combine the case labels to avoid future divergence.
♻️ Proposed refactor
- case "tool.update": - return [ - ToolCallUpdateEventSchema.parse({ - type: "tool_call_update", - toolCallId: event.toolCallId, - title: event.title, - status: mapToolStatus(event.status), - content: event.content, - rawOutput: event.output, - }), - ]; - case "tool.end": + case "tool.update": + case "tool.end": return [ ToolCallUpdateEventSchema.parse({ type: "tool_call_update", toolCallId: event.toolCallId, title: event.title, status: mapToolStatus(event.status), content: event.content, rawOutput: event.output, }), ];🤖 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/cli/src/core/acp/events.ts` around lines 54 - 75, The `tool.update` and `tool.end` branches in `events.ts` are identical, so merge them into a single shared handler to prevent drift. Update the `switch` in the event-mapping logic so both case labels fall through to the same `ToolCallUpdateEventSchema.parse` return path, keeping the existing `mapToolStatus`, `toolCallId`, `title`, `content`, and `rawOutput` mapping unchanged.apps/cli/src/core/agents/catalog.ts (1)
82-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPersona-matching fallback duplicated with
runtime.ts.The category/id substring fallback here (
option.category?.includes("persona") || option.id.toLowerCase().includes("persona")) is duplicated verbatim inAgentRuntime.setPersona(apps/cli/src/core/agents/runtime.ts, shown in context snippet 3). If the heuristic ever needs a tweak, one copy will silently drift from the other.Consider exporting a single
findPersonaConfigOption/findPersonaConfigOptionIdhelper from this file and havingruntime.tsconsume it instead of re-implementing the match.♻️ Suggested consolidation
+export function findPersonaConfigOption( + options: SessionConfigOption[] +): Extract<SessionConfigOption, { type: "select" }> | undefined { + const match = + findSelectConfigOption(options, "persona") ?? + options.find( + (option) => + option.type === "select" && + (option.category?.includes("persona") || + option.id.toLowerCase().includes("persona")) + ); + return match?.type === "select" ? match : undefined; +} + export function configOptionsToPersonas( options: SessionConfigOption[] ): SelectOption[] { - const config = - findSelectConfigOption(options, "persona") ?? - options.find( - (option) => - option.type === "select" && - (option.category?.includes("persona") || - option.id.toLowerCase().includes("persona")) - ); - if (!config || config.type !== "select") return []; + const config = findPersonaConfigOption(options); + if (!config) return []; return selectOptionsToList(config.options); }Then in
runtime.ts, replace the inline fallback withfindPersonaConfigOption(probe.transcript.session.configOptions)?.id.🤖 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/cli/src/core/agents/catalog.ts` around lines 82 - 95, The persona fallback matching logic is duplicated between configOptionsToPersonas and AgentRuntime.setPersona, so the heuristic can drift over time. Extract the shared category/id matching into a single helper such as findPersonaConfigOption or findPersonaConfigOptionId in catalog.ts, then update runtime.ts to call that helper instead of re-implementing the inline fallback. Keep the existing configOptionsToPersonas flow intact and reuse the shared helper wherever persona config lookup is needed.apps/cli/src/handlers/controller.ts (2)
77-95: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winNo error handling around the prompt generator.
If
runtime.threadCoordinator.prompt(...)throws mid-stream (agent crash,resolveProjectCwdfailure for an unknown project, etc.), the generator just rejects with no broadcast/cleanup — subscribed peers (context.broadcaster) get no notice that the stream ended abnormally. Consider wrapping the loop in try/catch (or try/finally) to broadcast a terminal error event before rethrowing.🤖 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/cli/src/handlers/controller.ts` around lines 77 - 95, The chat handler in controller.ts does not handle failures from runtime.threadCoordinator.prompt(...) or the async iteration, so a mid-stream exception can end the generator silently without notifying subscribers. Wrap the prompt/gen loop in try/catch (and optionally finally) inside the chat handler, catch errors from the prompt generator or for-await loop, broadcast a terminal error/abort event through context.broadcaster before rethrowing, and ensure the stream exits cleanly for peers.
77-95: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winUnconditional per-event sleep may add significant latency to chatty streams.
Every yielded
AgentEvent(including per-token/thought chunks) is followed byawait Bun.sleep(env.CYRUS_STREAM_THROTTLING_MS)— for a response with hundreds of token events at the default 25ms, that's several seconds of added latency purely from throttling. Consider throttling by wall-clock interval (e.g., only sleep if less than N ms elapsed since the last flush) rather than sleeping after every single yield, so throttling scales with event volume instead of compounding linearly.🤖 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/cli/src/handlers/controller.ts` around lines 77 - 95, The chat handler in os.chat.handler applies an unconditional Bun.sleep after every yielded event, which adds latency linearly for chatty streams. Update the streaming loop around runtime.threadCoordinator.prompt and context.broadcaster.broadcast to throttle by elapsed wall-clock time instead of per event, so the delay only happens when the last flush was too recent. Keep the existing stream behavior, but gate the sleep using a last-flush timestamp and env.CYRUS_STREAM_THROTTLING_MS.
🤖 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/cli/src/commands/service/worker.ts`:
- Around line 52-60: The shutdown handler exits the process before agent
termination finishes, so AgentPool.shutdown() needs to be awaited instead of
fire-and-forget. Update runtime.agentPool.shutdown() in the shutdown flow to
return a Promise<void> from AgentPool.shutdown() by waiting for all stopAgent
calls to settle, then make the shutdown handler async and await
runtime.agentPool.shutdown() before calling device.close(),
signalingSession.close(), and process.exit(0). Use the AgentPool.shutdown and
stopAgent symbols to locate the affected flow.
In `@apps/cli/src/core/acp/host.ts`:
- Around line 1-23: The requestPermission logic in createDefaultHost is
incorrectly falling through to AllowOnce when the first available option is a
reject kind. Update the option selection so it only chooses allow_once or
allow_always from request.options, and return PermissionDecision.Deny when
neither exists. Keep the fix contained within createDefaultHost and its
requestPermission handler so reject-only option lists cannot be treated as
allowed.
In `@apps/cli/src/core/acp/ping.ts`:
- Around line 18-29: Wrap the createAcpRuntime setup in ping() so it cannot
throw synchronously and instead returns a Result.err string on failure. The
current construction path for createAcpRuntime in ping() can escape the
Result<AcpPingSuccess, string> contract that doctor.ts consumes via .match(), so
catch any exception around the runtime creation and convert it into an err
result with a descriptive message.
- Around line 31-36: The bare method references in ping.ts can lose the
AcpRuntime instance context when passed to Result.tryPromise. Update the
acp.ready and acp.shutdown calls in the ping flow to use thunks so the methods
are invoked with acp as the receiver, and apply the same safe pattern used
elsewhere in the ACP code (for example, the pool logic).
In `@apps/cli/src/core/acp/pool.ts`:
- Around line 93-97: `shutdown()` in `AgentPool` is fire-and-forget, so callers
cannot wait for `stopAgent` to finish terminating spawned processes. Update
`AgentPool.shutdown()` to return a Promise that resolves only after all agents
have been stopped, and make the implementation await the async `stopAgent(name)`
calls for every entry in `this.agents` instead of ignoring the returned
promises. This will let the worker/service shutdown path wait for the pool to
fully terminate before resolving.
- Around line 46-91: The getRuntime flow allows duplicate startups because the
agent is not marked in this.agents until after await this.getEntry(name)
completes. In AcpRuntimePool.getRuntime, register a placeholder entry for the
name immediately before any await, then reuse that entry’s startPromise for
concurrent calls so only one bootRuntime runs per agent. Keep the existing state
transitions and updates to runtime, state, and startPromise in the same
getRuntime/bootRuntime path.
In `@apps/cli/src/core/agents/runtime.ts`:
- Around line 37-117: The config/session methods in Runtime still use stale
cached sessions after an agent crash because only prompt() checks pool state and
recoverSessions() does not refresh probeSession. Update Runtime’s
getProbeSession() and requireSession() (or add a shared crash-check helper) so
they verify this.pool.getState(this.agentName) and call recoverSessions() before
returning any cached RuntimeSession, and ensure probeSession is rebuilt
alongside sessions when recovery happens. Apply the same guard to getModels,
getModes, getEfforts, getPersonas, setModel, setMode, setEffort, and setPersona
so they always bind to a live subprocess.
In `@apps/cli/src/handlers/controller.ts`:
- Around line 78-83: The chat flow in controller.ts generates a fallback
threadId server-side when it is omitted, but that value is never returned to the
caller, so the client cannot resume or manage the thread later. Update the
handling around the chat/controller path (including the chat result construction
and any response type/schema used by controller.ts and AgentEventSchema) so the
generated threadId is included in the returned payload or initial event, while
preserving existing AgentEvent emission behavior for the rest of the stream.
In `@apps/cli/src/mocks/projects.ts`:
- Around line 9-34: The mock project registry and cwd resolver need to ensure
the default project directory exists before it is returned, because
ThreadCoordinator uses resolveProjectCwd and runtime.newSession({ cwd }) can
fail if /tmp/cyrus-agent-test is missing. Update the projects mock around
DEFAULT_PROJECT_ID, listProjects, and resolveProjectCwd to create the cwd on
demand (or initialize it during module setup) before returning it, and make sure
any lookup for non-default project IDs is handled consistently with the mock’s
intended behavior.
In `@apps/cli/src/store/agents.ts`:
- Around line 16-36: `readRegistry()` is hiding file/parse failures by returning
an empty registry, which lets `addAgent`, `updateAgent`, and `removeAgent`
overwrite `agents.yml` after a bad read. Change `readRegistry` to surface errors
instead of swallowing them (for both YAML/IO failures and invalid data), and
update the mutators to check that result and abort/report without calling
`writeRegistry()` when the read fails. Use the existing `readRegistry`,
`writeRegistry`, `addAgent`, `updateAgent`, and `removeAgent` flow to preserve
data on corrupted or unreadable files.
In `@apps/cli/src/utils/io.ts`:
- Around line 1-13: The stdinWritable helper currently writes to a Bun.FileSink
without forcing buffered data out, so incremental pipe messages may stay
buffered until close. Update stdinWritable to flush the sink after each write in
the WritableStream write handler, and keep the existing close/abort behavior
using stdin.end() so the child receives data promptly.
In `@apps/web/src/routes/threads/index.tsx`:
- Around line 135-153: The streaming handler in the threads route creates a new
broadcast message even when appendAgentEventText("", event) returns an empty
string, which leads to blank “…” bubbles for no-op events. Update the for await
event processing in the stream handler to skip events whose appended text is
empty before pushing a broadcast message, and keep the existing update path for
the last broadcast bubble when present.
- Around line 31-33: The event handling in the threads text accumulation
currently merges both "token" and "thought" into the same visible reply text,
which leaks internal thought content; update the switch in the thread rendering
logic to keep "thought" events separate or ignore them in the user-facing text,
while preserving the normal accumulation for assistant token events only. Use
the existing event-processing path in the threads route to ensure the final
bubble is built from tokens alone and thought content is handled elsewhere or
omitted.
In `@openspec/changes/acp-provider-runtime/design.md`:
- Around line 72-88: Add a language tag to the fenced code block in the ACP
provider runtime design doc so markdownlint no longer flags MD040. Update the
fence in the section listing src/acp and src/providers files to use a text-like
language identifier, keeping the content unchanged.
In `@openspec/changes/acp-provider-runtime/specs/acp-provider-cli/spec.md`:
- Around line 3-42: The CLI namespace is inconsistent across this spec and the
rest of the ACP docs, splitting the contract for the same provider feature set.
Update the requirement scenarios and command names to use a single namespace
consistently, matching the agreed CLI surface already used elsewhere (for
example, the provider list/detect behavior and worker capability advertising
docs), and ensure all references to the command group are aligned throughout
this spec.
In `@openspec/specs/acp-process-manager/spec.md`:
- Around line 83-95: The agent availability requirement is too narrow because it
only validates PATH membership, which would reject valid configured command
paths. Update the availability logic in the relevant worker/doctor check for the
agent command so it accepts executable absolute or relative paths when they
resolve successfully, and only emit the external-install hint for bare command
names that cannot be resolved on PATH. Use the existing agent command validation
flow to distinguish between path resolution and PATH lookup before reporting
availability.
---
Nitpick comments:
In `@apps/cli/src/commands/agents/add.ts`:
- Around line 1-17: The result-handling in add() duplicates the same ok/err
pattern used by other agent commands, so extract it into a shared helper to keep
behavior consistent. Create a reusable function like printResultOrExit in a
common CLI utility module, then update add() to use it with the addAgent result
and keep the success callback for the success message while centralizing the
error printing and process.exit(1) path.
In `@apps/cli/src/commands/agents/update.ts`:
- Around line 22-38: The `updateAgent` call in `agents/update.ts` is redundantly
merging data twice because `entry` is already fully resolved from `existing`.
Adjust the `update` command so `entry` is only used as the display payload for
the success message, and pass a partial patch object into `updateAgent` instead
of the already merged `AgentEntry`. Keep the success output logic tied to the
local `entry` variable, and ensure the `saved.match` handling remains unchanged.
In `@apps/cli/src/commands/config/index.ts`:
- Around line 2-9: The `registerConfigCommands` rename flow is reusing
`nameArgParser` from `validators/acp`, but that validator is now shared outside
ACP and its current name/location is misleading. Move the shared name validation
to a more generic validator module or rename/export it to reflect broader
device-name use, and update the `registerConfigCommands` import so the `rename`
command clearly points to a cross-domain validator.
In `@apps/cli/src/core/acp/events.ts`:
- Around line 54-75: The `tool.update` and `tool.end` branches in `events.ts`
are identical, so merge them into a single shared handler to prevent drift.
Update the `switch` in the event-mapping logic so both case labels fall through
to the same `ToolCallUpdateEventSchema.parse` return path, keeping the existing
`mapToolStatus`, `toolCallId`, `title`, `content`, and `rawOutput` mapping
unchanged.
In `@apps/cli/src/core/acp/pool.ts`:
- Around line 99-149: The ManagedAgent placeholder runtime is being modeled with
an unsafe cast instead of the actual “not yet booted” state. Update the
ManagedAgent shape used by getRuntime and bootRuntime so runtime is optional
(runtime?: AcpRuntime), then adjust the initialization and assignment sites in
bootRuntime, resetIdleTimer, and stopAgent to work with the optional field
safely without using undefined as unknown as AcpRuntime.
In `@apps/cli/src/core/agents/catalog.ts`:
- Around line 82-95: The persona fallback matching logic is duplicated between
configOptionsToPersonas and AgentRuntime.setPersona, so the heuristic can drift
over time. Extract the shared category/id matching into a single helper such as
findPersonaConfigOption or findPersonaConfigOptionId in catalog.ts, then update
runtime.ts to call that helper instead of re-implementing the inline fallback.
Keep the existing configOptionsToPersonas flow intact and reuse the shared
helper wherever persona config lookup is needed.
In `@apps/cli/src/handlers/controller.ts`:
- Around line 77-95: The chat handler in controller.ts does not handle failures
from runtime.threadCoordinator.prompt(...) or the async iteration, so a
mid-stream exception can end the generator silently without notifying
subscribers. Wrap the prompt/gen loop in try/catch (and optionally finally)
inside the chat handler, catch errors from the prompt generator or for-await
loop, broadcast a terminal error/abort event through context.broadcaster before
rethrowing, and ensure the stream exits cleanly for peers.
- Around line 77-95: The chat handler in os.chat.handler applies an
unconditional Bun.sleep after every yielded event, which adds latency linearly
for chatty streams. Update the streaming loop around
runtime.threadCoordinator.prompt and context.broadcaster.broadcast to throttle
by elapsed wall-clock time instead of per event, so the delay only happens when
the last flush was too recent. Keep the existing stream behavior, but gate the
sleep using a last-flush timestamp and env.CYRUS_STREAM_THROTTLING_MS.
In `@apps/cli/src/validators/acp.ts`:
- Around line 7-25: Both commandArgParser and nameArgParser duplicate the same
safeParse-to-InvalidArgumentError flow. Extract the repeated validation/throw
logic into a small shared helper that accepts a schema and fallback message,
then have commandArgParser and nameArgParser delegate to it while preserving
their current messages and return types. Use the existing symbols commandSchema,
nameSchema, and InvalidArgumentError to keep the refactor localized.
🪄 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: 1fae5a2f-56b4-487d-b531-a8311ca8a02d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
.dotagents/commands/opsx-apply.md.dotagents/commands/opsx-archive.md.dotagents/commands/opsx-continue.md.dotagents/commands/opsx-explore.md.dotagents/commands/opsx-new.md.dotagents/commands/opsx-propose.md.dotagents/commands/opsx-sync.md.dotagents/commands/opsx-verify.md.dotagents/config.toml.gitignoreapps/cli/package.jsonapps/cli/src/commands/agents/add.tsapps/cli/src/commands/agents/doctor.tsapps/cli/src/commands/agents/index.tsapps/cli/src/commands/agents/list.tsapps/cli/src/commands/agents/rm.tsapps/cli/src/commands/agents/update.tsapps/cli/src/commands/auth/login.tsapps/cli/src/commands/auth/logout.tsapps/cli/src/commands/auth/whoami.tsapps/cli/src/commands/config/index.tsapps/cli/src/commands/config/rename.tsapps/cli/src/commands/service/status.tsapps/cli/src/commands/service/worker.tsapps/cli/src/constants/file.tsapps/cli/src/core/acp/config.tsapps/cli/src/core/acp/events.tsapps/cli/src/core/acp/host.tsapps/cli/src/core/acp/ping.tsapps/cli/src/core/acp/pool.tsapps/cli/src/core/acp/transport.tsapps/cli/src/core/agents/catalog.tsapps/cli/src/core/agents/profile.tsapps/cli/src/core/agents/runtime.tsapps/cli/src/core/index.tsapps/cli/src/core/threads/coordinator.tsapps/cli/src/handlers/controller.tsapps/cli/src/index.tsapps/cli/src/lib/auth.tsapps/cli/src/lib/env.tsapps/cli/src/mocks/projects.tsapps/cli/src/store/agents.tsapps/cli/src/store/config.tsapps/cli/src/utils/error.tsapps/cli/src/utils/io.tsapps/cli/src/validators/acp.tsapps/cli/src/validators/agent.tsapps/cli/src/validators/name.tsapps/web/src/routes/threads/index.tsxopenspec/changes/acp-provider-runtime/.openspec.yamlopenspec/changes/acp-provider-runtime/design.mdopenspec/changes/acp-provider-runtime/proposal.mdopenspec/changes/acp-provider-runtime/specs/acp-process-manager/spec.mdopenspec/changes/acp-provider-runtime/specs/acp-provider-cli/spec.mdopenspec/changes/acp-provider-runtime/specs/acp-provider-config/spec.mdopenspec/changes/acp-provider-runtime/specs/acp-session-router/spec.mdopenspec/changes/acp-provider-runtime/tasks.mdopenspec/specs/acp-process-manager/spec.mdopenspec/specs/acp-provider-cli/spec.mdopenspec/specs/acp-provider-config/spec.mdopenspec/specs/acp-session-router/spec.mdshared/connections/src/contracts/controller.tsshared/connections/src/schemas/agents.tsshared/connections/src/schemas/chat.tsshared/connections/src/schemas/rtc.ts
Make agent pool shutdown awaitable, harden permission handling and session recovery, propagate agents.yml read errors via better-result, return threadId from chat, and archive the completed OpenSpec change. Co-authored-by: Cursor <cursoragent@cursor.com>
This was referenced Jul 8, 2026
This was referenced Jul 13, 2026
4 tasks done
This branch was successfully deployed
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
@acp-kit/core, organized undercore/{acp,agents,threads}cyrusd agents add/list/update/rm/doctor) backed by~/.cyrus/agents.ymllistProjects, shared chat event schemas, andprojectId-based chat inputThreadCoordinator+AgentPoolruntimeTest plan
bun checkandbun check:typespasscyrusd agents add <name> --cmd <agent>registers an agentcyrusd agents doctor <name>completes ACP initialize successfullycyrusd startworker connects and serves controller RPCsprojectId: "default"and receive streamed eventsgetModels,setMode, etc.) work against a live agentMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes