diff --git a/.agents/skills/custom-codereview-guide.md b/.agents/skills/custom-codereview-guide.md index 76726b8e59..f405ba7d7e 100644 --- a/.agents/skills/custom-codereview-guide.md +++ b/.agents/skills/custom-codereview-guide.md @@ -168,6 +168,78 @@ pydantic_core.ValidationError: Extra inputs are not permitted **This is a production-breaking change.** Do not approve PRs that modify event types without proper backward compatibility handling and tests. +## Frontend API Access Conventions + +These two rules are enforced by the CI test `src/api/no-direct-agent-server-calls.test.ts`. +**Flag any PR that introduces a violation** -- these are correctness bugs, not style nits. + +### Rule 1 -- All agent-server calls must use `@openhands/typescript-client` + +**DO NOT APPROVE** a PR that introduces raw `axios`, `fetch`, or the shared `openHands` +axios instance to call an agent-server endpoint (`/api/*`, `/server_info`). All such +calls must go through typed client classes from `@openhands/typescript-client`, +instantiated with options from `getAgentServerClientOptions()` or +`getAgentServerHttpClientOptions()` in `src/api/agent-server-client-options.ts`. + +Forbidden patterns (caught by the CI guard): +- `openHands.(...)` -- shared axios instance +- `createHttpClient(...)` -- creates a raw HTTP client +- `axios(...)` / `axios.get/post/etc.(...)` (except in the two allowed files) +- `fetch('/api/...')` or `fetch(\`${host}/api/...\`)` + +Correct pattern: +```ts +new ConversationClient(getAgentServerClientOptions()).getConversation(id) +new FileClient(getAgentServerClientOptions()).downloadTextFile(path) +new ServerClient(getAgentServerHttpClientOptions()).getServerInfo() +new RemoteWorkspace(getAgentServerClientOptions()).gitChanges({ ref: "HEAD" }) +``` + +Allowed exceptions (explicitly listed in `ALLOWED_AD_HOC_HTTP_FILES`): +- `src/api/automation-service/automation-service.api.ts` +- `src/api/cloud/proxy.ts` + +If a PR adds a new file to `ALLOWED_AD_HOC_HTTP_FILES` without a strong reason, +flag it -- the allowlist should not grow casually. + +### Rule 2 -- All cloud backend calls must go through `callCloudProxy` + +**DO NOT APPROVE** a PR that issues a direct browser `fetch` or `axios` call to the +cloud SaaS backend (`app.all-hands.dev`) or a cloud runtime sandbox +(`*.prod-runtime.all-hands.dev`). Both origins block CORS from `localhost`. Cloud calls +must go through `callCloudProxy()` in `src/api/cloud/proxy.ts`, which routes them +server-side through `/api/cloud-proxy` on the local agent-server. + +Correct pattern -- cloud SaaS: +```ts +callCloudProxy({ backend, method: "GET", path: "/api/v1/app-conversations/search?..." }) +``` + +Correct pattern -- cloud runtime sandbox (use `hostOverride` + `authMode: "session-api-key"`): +```ts +callCloudProxy({ + backend, + method: "GET", + hostOverride: buildHttpBaseUrl(conversationUrl), + path: `/api/conversations/${id}`, + authMode: "session-api-key", + sessionApiKey, +}) +``` + +Standard branch structure every cloud-aware service method should follow: +```ts +if (getActiveBackend().backend.kind === "cloud") { + return callCloudProxy({ backend: active, ... }); +} +// local path: typed typescript-client +return new ConversationClient(getAgentServerClientOptions()).someMethod(...); +``` + +Missing the `hostOverride` on a runtime-sandbox call is a silent bug: the proxy +will target `backend.host` (the SaaS API) instead of the actual runtime URL. +Flag any `callCloudProxy` call that targets a runtime URL without `hostOverride`. + ## SDK Architecture Conventions These conventions codify patterns that are easy to violate when adding new features. Each was learned from a real bug. diff --git a/AGENTS.md b/AGENTS.md index 6b6035e499..b75ef69bd7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -159,6 +159,94 @@ you are running inside of — NOT the automation backend. - `@openhands/typescript-client` is currently pinned to commit `ef62e82fc3dfb03991a1c8025429caf354427263` because the package metadata needed by this PR has not been published as a consistent npm/tagged release yet. That commit ships the needed typed clients plus subpath exports for `client/http-client`, `events/remote-events-list`, and `workspace/remote-workspace`. `RemoteWorkspace.gitChanges`/`gitDiff` accept an optional `{ ref }` option; agent-canvas passes `'HEAD'` so the changes panel reflects working-tree + index versus the latest commit (i.e. staged + unstaged) instead of a diff against the upstream/default branch. - The `@openhands/typescript-client` git dep must be expressed as a `git+https://github.com/...` URL in both `package.json` and the top-level dep entry of `package-lock.json`; the `github:OpenHands/...` shorthand normalizes to `git+ssh://` inside the lockfile, and Vercel's build environment has no GitHub SSH key, so an ssh-pinned lockfile makes Vercel fall back to a stale cached tarball and the bundler then fails with `[MISSING_EXPORT] ConversationClient/FileClient/SharedClient is not exported by .../dist/clients.js`. `scripts/vercel-install.sh` (wired up via `vercel.json`'s `installCommand`) defensively rewrites any leftover `git+ssh://git@github.com/` resolved URLs to `git+https://github.com/` and adds matching `git config --global url..insteadOf` aliases before invoking `npm ci`, so a future regression that re-introduces an ssh-pinned lockfile entry still builds on Vercel. See GitHub issue #384 for the original failure and PR #382 for the prior single-shot lockfile fix that this generalizes. +## API Access Rules + +Two strict conventions govern every REST call in the frontend. Violations break CI +via `src/api/no-direct-agent-server-calls.test.ts`. + +### Rule 1 -- Agent-server calls must use `@openhands/typescript-client` + +All calls that target the local agent-server (`/api/*`, `/server_info`, `/sockets`) +**must** go through typed client classes from `@openhands/typescript-client`, **never** +raw `axios`, `fetch`, or the legacy shared `openHands` axios instance. + +Available clients and their subpath imports: +- `ConversationClient` -- `@openhands/typescript-client/clients` +- `FileClient` -- `@openhands/typescript-client/clients` +- `VSCodeClient` -- `@openhands/typescript-client/clients` +- `ServerClient` -- `@openhands/typescript-client/clients` +- `HttpClient` -- `@openhands/typescript-client/client/http-client` +- `RemoteWorkspace` -- `@openhands/typescript-client/workspace/remote-workspace` +- `RemoteEventsList` -- `@openhands/typescript-client/events/remote-events-list` + +Client options are always assembled via helpers in `src/api/agent-server-client-options.ts`: +- `getAgentServerClientOptions(overrides?)` -- for SDK client constructors +- `getAgentServerHttpClientOptions(overrides?)` -- for `HttpClient`-based callers + +These helpers read host, session API key, and working directory from the active backend +registry and env config, so callers never hardcode URLs or auth tokens. + +```ts +// CORRECT +const data = await new ConversationClient(getAgentServerClientOptions()).getConversation(id); +const file = await new FileClient(getAgentServerClientOptions()).downloadTextFile(path); + +// WRONG -- raw axios/fetch calls fail the no-direct-agent-server-calls.test.ts guard +const data = await axios.get(`${host}/api/conversations/${id}`); +const data = await fetch(`/api/conversations/${id}`); +``` + +**Allowed exceptions** (files that may use axios directly for infrastructure reasons): +- `src/api/automation-service/automation-service.api.ts` +- `src/api/cloud/proxy.ts` -- the proxy envelope POST itself + +### Rule 2 -- Cloud backend routes must go through `callCloudProxy` + +Any call from the browser to the cloud SaaS backend (`app.all-hands.dev`) or a cloud +runtime sandbox (`*.prod-runtime.all-hands.dev`) **must** go through `callCloudProxy()` +in `src/api/cloud/proxy.ts`. These origins do not permit CORS from `localhost`; +`callCloudProxy` POSTs the request envelope to `/api/cloud-proxy` on the local +agent-server, which forwards it server-side. + +```ts +import { callCloudProxy } from "../cloud/proxy"; + +// CORRECT -- cloud SaaS endpoint +const result = await callCloudProxy({ + backend, + method: "GET", + path: `/api/v1/app-conversations/search?${params}`, +}); + +// CORRECT -- cloud runtime sandbox, auth via session key +const result = await callCloudProxy({ + backend, + method: "GET", + hostOverride: buildHttpBaseUrl(conversationUrl), + path: `/api/git/changes?path=${path}`, + authMode: "session-api-key", + sessionApiKey, +}); + +// WRONG -- direct fetch/axios to a cloud host is blocked by CORS in the browser +const result = await axios.get(`${backend.host}/api/v1/app-conversations`); +``` + +`callCloudProxy` key options: +- `backend` -- the cloud `Backend` object (provides host and bearer token) +- `hostOverride` -- override for runtime-sandbox calls; replaces `backend.host` +- `authMode` -- `"bearer"` (default, cloud SaaS) | `"session-api-key"` (runtime sandbox) | `"none"` +- `sessionApiKey` -- required when `authMode === "session-api-key"` + +Standard cloud/local branch pattern used throughout the service layer: + +```ts +if (getActiveBackend().backend.kind === "cloud") { + return callCloudProxy({ backend: active, ... }); +} +return new ConversationClient(getAgentServerClientOptions()).someMethod(...); +``` + - Use `@openhands/typescript-client` classes directly for agent-server-backed REST/workspace/event/VS Code calls. Centralize host/session API key/working-directory option assembly through `src/api/agent-server-client-options.ts`; the backend fallback policy itself lives in `src/api/backend-registry/active-store.ts`. - Local verification/build gotchas: - `npm run typecheck` assumes generated translation types exist; run `npm run make-i18n` first if `src/i18n/declaration.ts` is missing.