feat(acp): add machine stdio transport for Acepe#2858
Open
flazouh wants to merge 2 commits intoantinomyhq:mainfrom
Open
feat(acp): add machine stdio transport for Acepe#2858flazouh wants to merge 2 commits intoantinomyhq:mainfrom
flazouh wants to merge 2 commits intoantinomyhq:mainfrom
Conversation
Expose a machine stdio entrypoint in Forge and route it through a real ACP stdio transport so Acepe can launch Forge as an installable provider instead of depending on an unpublished branch. Co-Authored-By: ForgeCode <noreply@forgecode.dev>
Comment on lines
+39
to
+42
| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | ||
| self.called.store(true, Ordering::SeqCst); | ||
| async { Ok(()) } | ||
| } |
Contributor
There was a problem hiding this comment.
The test implementation has incorrect async behavior. The self.called.store() executes immediately when the future is created, not when it's awaited. This means the test doesn't accurately verify that the async function is actually executed.
fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send {
let called = self.called.clone();
async move {
called.store(true, Ordering::SeqCst);
Ok(())
}
}
Suggested change
| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | |
| self.called.store(true, Ordering::SeqCst); | |
| async { Ok(()) } | |
| } | |
| fn acp_start_stdio(&self) -> impl std::future::Future<Output = Result<()>> + Send { | |
| let called = self.called.clone(); | |
| async move { | |
| called.store(true, Ordering::SeqCst); | |
| Ok(()) | |
| } | |
| } | |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
- Replace unbounded notification channel with bounded (1024) to apply backpressure when the client stalls - Add per-session model override to prevent concurrent sessions from interfering with each other - Replace From<Error> impl with explicit into_acp_error() per project guidelines - Extract classify_mcp_tool() and convert to free functions, removing the unnecessary ToolOutputConverter struct - Validate MCP server names (length, charset) to prevent injection - Add MAX_BLOB_SIZE (50 MB) guard on base64-decoded resources - Add I/O timeout (5 min) and graceful shutdown drain (5 s) to prevent indefinite hangs - Track cancellation via AtomicBool across loop iterations - Log warnings instead of silently ignoring reload/config errors - Add tests for tool kind mapping, file extraction, and edge cases Co-Authored-By: ForgeCode <noreply@forgecode.dev>
a2e5e58 to
8fe513f
Compare
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
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.
Abstract
This PR adds a real
forge machine stdioentrypoint backed by Forge's ACP stdio transport. It is primarily for our app, Acepe, which needs to launch Forge from a stable CLI surface without depending on an unpublished feature branch.Product Context
Acepe is our desktop app for running and managing coding agents across providers and protocols.
Product links:
Problem
Acepe integrates Forge as an installable local provider. That integration needed a released Forge command that can:
Before this change, the relevant command path was missing on
main. We could add parser coverage forforge machine stdio, but that would still leave the command non-functional at runtime, which does not unblock Acepe.Solution
This change wires the machine-facing CLI entrypoint all the way through to a working ACP stdio server:
machine stdioas a first-class top-level CLI commandforge_maininto a dedicated stdio runneracp_start_stdio()so the transport can be started through the current app/service stackforge_appThe intent is to keep the public invocation simple while making the underlying server path real and launchable by external applications.
ASCII Schemas
Before
After
Ownership Boundary
Why This Is For Acepe
Acepe needs Forge to expose a stable, installable machine transport from
mainso our desktop app can launch Forge directly as a provider. This PR moves that capability into the open-source Forge repo so Acepe no longer has to depend on branch-specific or unpublished transport work.Files Of Interest
crates/forge_main/src/cli.rscrates/forge_main/src/ui.rscrates/forge_main/src/acp_runner.rscrates/forge_api/src/api.rscrates/forge_api/src/forge_api.rscrates/forge_app/src/acp_app.rscrates/forge_app/src/acp/Verification
I verified the change with:
Notes
protocwas not available on PATH in the local environment, so verification used a staged temporary binary.