Skip to content

add ACP shim for communication with agents in substrate#2049

Open
peterj wants to merge 9 commits into
mainfrom
peterj/substrate-acp
Open

add ACP shim for communication with agents in substrate#2049
peterj wants to merge 9 commits into
mainfrom
peterj/substrate-acp

Conversation

@peterj

@peterj peterj commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

Adds an ACP shim that runs inside the base images for openclaw/hermes on substrate. this allows us to re-use the websocket connection to the substrate actor (shim) and the shim does the translation from ws -> stdio. Both opencalw and hermes support the ACP protocol.

I've also removed the openshell and agent-sandbox from the templates/code as substrate is the thing we'll go with.

@github-actions github-actions Bot added the enhancement-proposal Indicates that this PR is for an enhancement proposal label Jun 18, 2026
@chromatic-com

chromatic-com Bot commented Jun 18, 2026

Copy link
Copy Markdown

Warning

Testing paused

Monthly snapshot limit reached. Update your plan for additional snapshots and to resume testing.

@peterj peterj force-pushed the peterj/substrate-acp branch 3 times, most recently from 653c39e to 6a4466d Compare June 22, 2026 18:30
@peterj peterj marked this pull request as ready for review June 22, 2026 18:31
Copilot AI review requested due to automatic review settings June 22, 2026 18:31
@peterj peterj force-pushed the peterj/substrate-acp branch from 6a4466d to f826a5b Compare June 22, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@peterj peterj force-pushed the peterj/substrate-acp branch from f826a5b to 534b029 Compare June 22, 2026 18:45
Comment thread go/core/pkg/sandboxbackend/substrate/constants.go Outdated

@EItanya EItanya left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple of questions for a first pass

Comment thread go/api/v1alpha2/agentharness_types.go Outdated
Comment thread go/core/cmd/acp-shim/sandbox/hermes-gateway-ensure.sh
Comment thread go/core/internal/database/gen/models.go Outdated
@peterj peterj force-pushed the peterj/substrate-acp branch from e3e34e4 to 563d69c Compare June 22, 2026 23:10

@jmhbh jmhbh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

look good to me so far (I'm around hafway through). Posting intial comments and will continue reviewing

Comment on lines +26 to +29
if [ -f "${PIDFILE}" ] && kill -0 "$(cat "${PIDFILE}" 2>/dev/null)" 2>/dev/null; then
log "gateway already running (pid $(cat "${PIDFILE}" 2>/dev/null)); nothing to do"
exit 0
fi

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this makes sense, but feels a bit hacky. Ideally we could perform the necessary cleanup right before a suspend operation but I understand we might want to iterate on this.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, in general we'll have to figure out how do we deal with the openclaw/hermes gateways -- they are launched and there mostly for the Telegram/Slack connections to work.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Long term I think we'll have to take over connectors for all agents in our system FWIW

Comment thread go/core/cmd/acp-shim/main.go Outdated
Comment thread go/core/internal/controller/agentharness_substrate_controller.go Outdated
Comment thread design/EP-XXXX-acp-integration.md Outdated
Comment thread go/core/pkg/sandboxbackend/channels/channels.go Outdated
// actor, then waits for it to be reachable via atenet-router. The sessionID
// identifies the chat for logging only; chats are multiplexed as ACP sessions
// inside the one actor, so they all resolve to the same ActorID(ah).
func (b *AgentHarnessSessionActorBackend) EnsureSessionActor(ctx context.Context, ah *v1alpha2.AgentHarness, sessionID string) (sandboxbackend.EnsureResult, error) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is the acp shim's child process stateful? AFAICT we are only managing the gateway connection to the backend type so this may not be relevant but the reason I'm asking is because if the child process maintains state that pertains to a specific session multiplexing on the actor like this could leak that state to potentially irrelevant sessions.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the session specific state shouldn't leak across other sessions; however, the things that are per-actor -- e.g. skills or tools -- those will be (and should be) available across sessions.

@peterj peterj force-pushed the peterj/substrate-acp branch from d014ce3 to 1f68ad1 Compare June 24, 2026 19:39
Comment thread go/core/pkg/acpshim/server.go Outdated
Comment thread go/core/pkg/acpshim/server.go
Comment thread go/core/pkg/sandboxbackend/substrate/constants.go Outdated
Comment thread scripts/controller-digest-ldflags.sh Outdated
@peterj peterj force-pushed the peterj/substrate-acp branch from 1932d5b to d955684 Compare June 25, 2026 18:14
peterj added 5 commits June 25, 2026 11:18
openshell and agent-sandbox

Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>

remove openshell and agent-sandbox; add hermes and openclaw to substrate

Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
…agentharness)

Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
@peterj peterj force-pushed the peterj/substrate-acp branch from d955684 to 26917ca Compare June 25, 2026 18:19
peterj added 4 commits June 25, 2026 12:02
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
Signed-off-by: Peter Jausovec <peter.jausovec@solo.io>
@peterj peterj force-pushed the peterj/substrate-acp branch from 636cc0d to c60a247 Compare June 25, 2026 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement-proposal Indicates that this PR is for an enhancement proposal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants