-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: enhance external CLI integrations with registry pattern and streaming support #1394
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
Changes from all commits
d95fc92
e298009
bd33c84
d522247
52ed8a6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,11 @@ export interface ExternalAgentResult { | |||||||
| duration: number; | ||||||||
| } | ||||||||
|
|
||||||||
| export type StreamEvent = | ||||||||
| | { type: 'text'; content: string } | ||||||||
| | { type: 'json'; data: unknown } | ||||||||
| | { type: 'error'; error: string }; | ||||||||
|
|
||||||||
| /** | ||||||||
| * Base class for external agent integrations | ||||||||
| */ | ||||||||
|
|
@@ -42,6 +47,11 @@ export abstract class BaseExternalAgent { | |||||||
| */ | ||||||||
| abstract execute(prompt: string): Promise<ExternalAgentResult>; | ||||||||
|
|
||||||||
| /** | ||||||||
| * Stream output from the external agent | ||||||||
| */ | ||||||||
| abstract stream(prompt: string): AsyncGenerator<StreamEvent, void, unknown>; | ||||||||
|
Comment on lines
+50
to
+53
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Avoid making Line 53 turns Suggested non-breaking shape- abstract stream(prompt: string): AsyncGenerator<StreamEvent, void, unknown>;
+ /**
+ * Stream output from the external agent.
+ * `@param` prompt Prompt to send to the external agent.
+ * `@returns` Async stream of external-agent events.
+ * `@example`
+ * for await (const event of agent.stream("hello")) {
+ * console.log(event);
+ * }
+ */
+ async *stream(prompt: string): AsyncGenerator<StreamEvent, void, unknown> {
+ const result = await this.execute(prompt);
+ if (!result.success) {
+ throw new Error(result.error || `${this.getName()} execution failed`);
+ }
+ yield { type: 'text', content: result.output };
+ }As per coding guidelines, "All public APIs must include JSDoc comments with 🤖 Prompt for AI Agents |
||||||||
|
|
||||||||
| /** | ||||||||
| * Get the agent name | ||||||||
| */ | ||||||||
|
|
@@ -97,6 +107,65 @@ export abstract class BaseExternalAgent { | |||||||
| }); | ||||||||
| } | ||||||||
|
|
||||||||
| /** | ||||||||
| * Stream command output line by line | ||||||||
| */ | ||||||||
| protected async *streamCommand(args: string[]): AsyncGenerator<StreamEvent, void, unknown> { | ||||||||
| const { spawn } = await import('child_process'); | ||||||||
|
|
||||||||
| const proc = spawn(this.config.command, args, { | ||||||||
| cwd: this.config.cwd || process.cwd(), | ||||||||
| env: { ...process.env, ...this.config.env }, | ||||||||
|
||||||||
| env: { ...process.env, ...this.config.env }, | |
| env: { ...process.env, ...this.config.env }, | |
| timeout: this.config.timeout, |
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.
The spawn call is missing an error listener. If the command is not found or fails to start, an error event will be emitted. If there are no listeners for the error event, the Node.js process will throw an unhandled exception and crash. Adding a simple error listener prevents this.
const proc = spawn(this.config.command, args, {
cwd: this.config.cwd || process.cwd(),
env: { ...process.env, ...this.config.env },
stdio: ['pipe', 'pipe', 'pipe']
});
proc.on('error', () => {}); // Prevent unhandled exception if command not found
Copilot
AI
Apr 16, 2026
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.
streamCommand() only reads from proc.stdout and never consumes proc.stderr. If the child process writes enough to stderr, the buffer can fill and block the process, stalling the stream. Drain stderr (e.g., with a separate listener/interface) or redirect it to stdout when streaming.
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.
Preserve blank lines in streamed text output.
The if (line.trim()) guard drops intentional empty lines, which means markdown paragraphs and code blocks cannot be reconstructed faithfully by callers consuming Claude/Gemini/Codex/Generic streams. Please preserve empty text chunks here and let the consumer decide whether to ignore them.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-ts/src/cli/features/external-agents.ts` around lines 144 - 155,
Remove the "if (line.trim())" guard so blank lines are not dropped; inside the
loop that iterates "for await (const line of rl)" still attempt JSON.parse in
the existing try/catch and on error yield the text variant (yield { type:
'text', content: line }) — this ensures empty lines (line === "" or whitespace)
are preserved and emitted for consumers to handle while keeping the existing
JSON parsing behavior.
Copilot
AI
Apr 16, 2026
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.
streamCommand() always proc.kill() in finally and never checks the subprocess exit code. This can terminate a still-running process prematurely and also makes failures (non-zero exit) invisible to callers. Consider awaiting the close/exit event, surfacing non-zero codes (throw or emit an error/exit event), and only killing the process on cancellation/timeouts.
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.
Aider’s fallback stream also strips meaningful blank lines.
Line 297 applies the same trim() filter, so streamed aider output loses paragraph and code-block separation too. This path should preserve empty lines just like the shared streaming helper.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/praisonai-ts/src/cli/features/external-agents.ts` around lines 296 - 299,
The loop over result.output is trimming and skipping blank lines (if
(line.trim())), which removes meaningful empty lines; change it to preserve
empty lines like the shared streaming helper by removing the trim-based filter
and yield every line (including empty strings) — update the loop that iterates
result.output and the yield { type: 'text', content: line } so it no longer uses
line.trim() to skip lines.
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 implementation of stream is not actually streaming; it waits for the entire command to finish before yielding any output. Since streamCommand already handles non-JSON output by yielding text events, you can use it here to provide real-time streaming of Aider's output.
async *stream(prompt: string): AsyncGenerator<StreamEvent, void, unknown> {
yield* this.streamCommand(['--message', prompt, '--yes']);
}| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -285,19 +285,47 @@ def get_available_integrations() -> Dict[str, bool]: | |||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| Get a dictionary of all integrations and their availability status. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Backward compatibility wrapper. Use ExternalAgentRegistry for new code. | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| Returns: | ||||||||||||||||||||||||||||||||||||
| dict: Mapping of integration name to availability (True/False) | ||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||
| from .claude_code import ClaudeCodeIntegration | ||||||||||||||||||||||||||||||||||||
| from .gemini_cli import GeminiCLIIntegration | ||||||||||||||||||||||||||||||||||||
| from .codex_cli import CodexCLIIntegration | ||||||||||||||||||||||||||||||||||||
| from .cursor_cli import CursorCLIIntegration | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| integrations = { | ||||||||||||||||||||||||||||||||||||
| 'claude': ClaudeCodeIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'gemini': GeminiCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'codex': CodexCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'cursor': CursorCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| # Import here to avoid circular imports | ||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||
| from .registry import get_registry | ||||||||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| registry = get_registry() | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| # Handle async call in sync context | ||||||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||||||
| # Try to get existing event loop | ||||||||||||||||||||||||||||||||||||
| loop = asyncio.get_event_loop() | ||||||||||||||||||||||||||||||||||||
| if loop.is_running(): | ||||||||||||||||||||||||||||||||||||
| # We're in an async context, use create_task | ||||||||||||||||||||||||||||||||||||
| import concurrent.futures | ||||||||||||||||||||||||||||||||||||
| with concurrent.futures.ThreadPoolExecutor() as executor: | ||||||||||||||||||||||||||||||||||||
| future = executor.submit(asyncio.run, registry.get_available()) | ||||||||||||||||||||||||||||||||||||
| return future.result() | ||||||||||||||||||||||||||||||||||||
| else: | ||||||||||||||||||||||||||||||||||||
| # No running loop, safe to use asyncio.run | ||||||||||||||||||||||||||||||||||||
| return asyncio.run(registry.get_available()) | ||||||||||||||||||||||||||||||||||||
| except RuntimeError: | ||||||||||||||||||||||||||||||||||||
| # No event loop, safe to use asyncio.run | ||||||||||||||||||||||||||||||||||||
| return asyncio.run(registry.get_available()) | ||||||||||||||||||||||||||||||||||||
|
Comment on lines
+300
to
+315
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This block introduces a complex and potentially fragile mechanism to call an asynchronous method from a synchronous context. If
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return {name: integration.is_available for name, integration in integrations.items()} | ||||||||||||||||||||||||||||||||||||
| except ImportError: | ||||||||||||||||||||||||||||||||||||
| # Fallback to original implementation | ||||||||||||||||||||||||||||||||||||
| from .claude_code import ClaudeCodeIntegration | ||||||||||||||||||||||||||||||||||||
| from .gemini_cli import GeminiCLIIntegration | ||||||||||||||||||||||||||||||||||||
| from .codex_cli import CodexCLIIntegration | ||||||||||||||||||||||||||||||||||||
| from .cursor_cli import CursorCLIIntegration | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| integrations = { | ||||||||||||||||||||||||||||||||||||
| 'claude': ClaudeCodeIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'gemini': GeminiCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'codex': CodexCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| 'cursor': CursorCLIIntegration(), | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| return {name: integration.is_available for name, integration in integrations.items()} | ||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -45,35 +45,55 @@ class CodexCLIIntegration(BaseCLIIntegration): | |
| output_schema: Path to JSON schema for structured output | ||
| """ | ||
|
|
||
| VALID_APPROVAL_MODES = {"suggest", "auto-edit", "full-auto"} | ||
|
|
||
| def __init__( | ||
| self, | ||
| workspace: str = ".", | ||
| timeout: int = 300, | ||
| full_auto: bool = False, | ||
| approval_mode: str = "suggest", # suggest, auto-edit, full-auto | ||
| sandbox: str = "default", | ||
| json_output: bool = False, | ||
| output_schema: Optional[str] = None, | ||
| output_file: Optional[str] = None, | ||
| provider: Optional[str] = None, # OpenAI, OpenRouter, Azure, Gemini, etc. | ||
| # Backward compatibility | ||
| full_auto: Optional[bool] = None, | ||
| ): | ||
|
Comment on lines
+54
to
62
|
||
| """ | ||
| Initialize Codex CLI integration. | ||
|
|
||
| Args: | ||
| workspace: Working directory for CLI execution | ||
| timeout: Timeout in seconds for CLI execution | ||
| full_auto: Whether to allow file modifications (--full-auto) | ||
| approval_mode: Approval mode ("suggest", "auto-edit", "full-auto") | ||
| sandbox: Sandbox mode ("default", "danger-full-access") | ||
| json_output: Whether to use JSON streaming output (--json) | ||
| output_schema: Path to JSON schema for structured output | ||
| output_file: Path to save the final output (-o) | ||
| provider: Model provider ("openai", "openrouter", "azure", "gemini", "ollama", etc.) | ||
| """ | ||
| super().__init__(workspace=workspace, timeout=timeout) | ||
|
|
||
| self.full_auto = full_auto | ||
| # Handle backward compatibility | ||
| if full_auto is not None: | ||
| approval_mode = "full-auto" if full_auto else "suggest" | ||
|
|
||
| if approval_mode not in self.VALID_APPROVAL_MODES: | ||
| raise ValueError( | ||
| f"Invalid approval_mode: '{approval_mode}'. " | ||
| f"Must be one of: {', '.join(sorted(self.VALID_APPROVAL_MODES))}" | ||
| ) | ||
|
|
||
| self.approval_mode = approval_mode | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| self.sandbox = sandbox | ||
| self.json_output = json_output | ||
| self.output_schema = output_schema | ||
| self.output_file = output_file | ||
| self.provider = provider | ||
|
|
||
| # Backward compatibility | ||
| self.full_auto = approval_mode == "full-auto" | ||
|
|
||
| @property | ||
| def cli_command(self) -> str: | ||
|
|
@@ -99,9 +119,12 @@ def _build_command(self, task: str, **options) -> List[str]: | |
| # Add task | ||
| cmd.append(task) | ||
|
|
||
| # Add full auto flag if enabled | ||
| if self.full_auto: | ||
| # Add approval mode | ||
| if self.approval_mode == "full-auto": | ||
| cmd.append("--full-auto") | ||
| elif self.approval_mode == "auto-edit": | ||
| cmd.append("--auto-edit") | ||
| # suggest is the default, no flag needed | ||
|
Comment on lines
+122
to
+127
|
||
|
|
||
| # Add sandbox mode if not default | ||
| if self.sandbox and self.sandbox != "default": | ||
|
|
@@ -119,6 +142,10 @@ def _build_command(self, task: str, **options) -> List[str]: | |
| if self.output_file: | ||
| cmd.extend(["-o", self.output_file]) | ||
|
|
||
| # Add provider if specified | ||
| if self.provider: | ||
| cmd.extend(["--provider", self.provider]) | ||
|
|
||
| return cmd | ||
|
|
||
| async def execute(self, prompt: str, **options) -> str: | ||
|
|
||
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.
StreamEventadvertises an error variant that the implementations never emit.Right now failures reject the async generator instead of yielding
{ type: 'error' }, so consumers branching onevent.typecannot rely on the exported contract. Either emit the error variant before terminating, or remove it fromStreamEventand document that stream errors are surfaced viathrow.🤖 Prompt for AI Agents