Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
joshsny
left a comment
joshsny
left a comment
There was a problem hiding this comment.
Looks good - left some comments :)
| this.clientCapabilities = request.clientCapabilities; | ||
|
|
||
| // Default authMethod | ||
| const authMethod: any = { |
|
|
||
| const options: Options = { | ||
| systemPrompt, | ||
| settingSources: ["user", "project", "local"], |
There was a problem hiding this comment.
how will this behave if the user has claude code installed locally and has settings there, how do we want it to behave?
Should we inherit their settings, or set this to local and make sure we control the settings in Array?
There was a problem hiding this comment.
We should inherit user settings.
There was a problem hiding this comment.
I am presuming most of this is copied from zed's existing adapter?
The code is not great, but if we're copying it over let's not change it as we know it works reasonably well.
Can we leave a comment saying where the code came from if that's the case?
| }, | ||
| agentInfo: { | ||
| name: packageJson.name, | ||
| title: "Claude Code", |
There was a problem hiding this comment.
Shall we give it a different name?
| } | ||
|
|
||
| async authenticate(_params: AuthenticateRequest): Promise<void> { | ||
| throw new Error("Method not implemented."); |
There was a problem hiding this comment.
Why? Is this because we are passing the API key and we don't want to allow the user to do their own authentication?
| } | ||
|
|
||
| async prompt(params: PromptRequest): Promise<PromptResponse> { | ||
| if (!this.sessions[params.sessionId]) { |
There was a problem hiding this comment.
Will this work with handing off cloud -> local - I guess we can hydrate the session locally in that case
There was a problem hiding this comment.
Yes, we'll call loadSession first usually, this is just a safety check
| } | ||
|
|
||
| async function getAvailableModels(query: Query): Promise<SessionModelState> { | ||
| const models = await query.supportedModels(); |
There was a problem hiding this comment.
Do you know what request this makes? We'll need to support it in the llm gateway
| }, | ||
| }; | ||
| break; | ||
| case "thinking": |
There was a problem hiding this comment.
can we get loading states via this? would be nice to get those messages claude code gives
There was a problem hiding this comment.
For sure, will add it to the list.
| } | ||
| } | ||
|
|
||
| export function runAcp() { |
There was a problem hiding this comment.
Do you know what this does?
There was a problem hiding this comment.
Yeah this sets up the communication stream between the ACP agent and the ACP client, and is used to kick off the agent connection
| completed: validatedData.metadata?.completed ?? 0, | ||
| }); | ||
|
|
||
| await this.writeTaskFile(taskId, { |
There was a problem hiding this comment.
out of scope of these PRs, but I think this and any other changes that aren't code related should be persisted in S3 and not locally for resumability and handoff
There was a problem hiding this comment.
Yeah, we'll start persisting these by reading the agent plan notifications from the S3 logs instead. https://agentclientprotocol.com/protocol/agent-plan
jonathanlab
force-pushed
the
12-01-feat_set_up_acp_and_claude_adapter
branch
from
9063af6 to
b72851b
Compare
Merge activity
|
2 participants
Based on Zed`s ACP claude adapter: https://github.com/zed-industries/claude-code-acp/tree/main