Skip to content

feat(acp): AcpClient.getAuthStatus for /api/acp/auth-status#196

Open
simonrosenberg wants to merge 1 commit into
mainfrom
feat-acp-auth-status-client
Open

feat(acp): AcpClient.getAuthStatus for /api/acp/auth-status#196
simonrosenberg wants to merge 1 commit into
mainfrom
feat-acp-auth-status-client

Conversation

@simonrosenberg
Copy link
Copy Markdown
Member

Summary

TypeScript-client side of the ACP subscription/login detection feature (OpenHands/agent-canvas#964). Adds a thin client for the new agent-server endpoint introduced in software-agent-sdk#3452, so the agent-canvas onboarding can show "✓ you're already logged in" instead of unconditionally asking for an API key.

Changes

  • AcpClient (src/client/acp-client.ts) with getAuthStatus(server)GET /api/acp/auth-status?server=<key>, returning the typed ACPAuthStatusResponse.
  • Types ACPAuthStatusValue ('authenticated' | 'unauthenticated' | 'unknown') and ACPAuthStatusResponse ({ server, status, auth_methods, agent_name, agent_version, detail }) in src/models/api.ts, mirroring the server model.
  • Exported from clients.ts (AcpClient + AcpClientOptions) and index.ts (the two types).
  • Unit tests in api-clients.test.ts: authenticated probe (asserts URL/query param + session-key header), unknown returned without throwing, and a 422 for an unknown provider.

The endpoint always responds 200 with a status of authenticated / unauthenticated / unknown (a probe that can't run is unknown, not an HTTP error), so a caller can map status 1:1 onto the canvas useAcpAuthStatus hook.

Consumed by

agent-canvas PR (wires useAcpAuthStatus to this endpoint). The agent-server endpoint lands in software-agent-sdk#3452.

Refs OpenHands/agent-canvas#964

🤖 Generated with Claude Code

Adds a thin client for the agent server's ACP auth-status probe endpoint
(software-agent-sdk #3452), so the agent-canvas onboarding can detect an
existing subscription/login instead of unconditionally asking for an API key.

- `AcpClient.getAuthStatus(server)` → `GET /api/acp/auth-status?server=<key>`,
  returning `ACPAuthStatusResponse { server, status, auth_methods, agent_name,
  agent_version, detail }` where `status ∈ {authenticated, unauthenticated,
  unknown}`.
- `ACPAuthStatusValue` / `ACPAuthStatusResponse` types mirror the server model.
- Exported from `clients.ts` (+ `AcpClientOptions`) and `index.ts`.
- Unit tests cover authenticated, unknown-without-throw, and the 422
  unknown-provider path.

Refs OpenHands/agent-canvas#964

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

all-hands-bot commented Jun 1, 2026

Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

Copy link
Copy Markdown

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

This is a clean, focused addition that follows the established client pattern faithfully. The types are well-documented, the implementation is minimal and correct, and the three test cases cover the meaningful behavioral branches. A couple of minor observations inline.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation


const client = new AcpClient({ host: 'http://example.com' });
await expect(client.getAuthStatus('nope')).rejects.toMatchObject({ status: 422 });
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Suggestion: The three tested cases cover authenticated, unknown, and 422. Consider adding a test for the unauthenticated status to complete the coverage of all three ACPAuthStatusValue variants — its behaviour at the client layer is identical to authenticated (the response object is returned as-is), but it would confirm the enum round-trip and make the test suite exhaustive:

Suggested change
});
});
it('AcpClient.getAuthStatus returns unauthenticated without throwing', async () => {
global.fetch = jest.fn().mockResolvedValue(
new Response(
JSON.stringify({
server: 'claude-code',
status: 'unauthenticated',
auth_methods: ['login'],
agent_name: 'claude-code-acp',
agent_version: '1.2.3',
detail: null,
}),
{ status: 200, headers: { 'content-type': 'application/json' } }
)
) as typeof fetch;
const client = new AcpClient({ host: 'http://example.com' });
const result = await client.getAuthStatus('claude-code');
expect(result.status).toBe('unauthenticated');
expect(result.auth_methods).toEqual(['login']);
});

Comment thread src/client/acp-client.ts
baseUrl: this.host,
apiKey: this.apiKey,
timeout: options.timeout || 60000,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Suggestion: options.timeout || 60000 is consistent with the pattern used across all other clients in this codebase (e.g. VSCodeClient, DesktopClient). If the project ever adds a lint rule preferring ?? over || for default-value assignments, this and its siblings would need updating together — but that is out of scope here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants