mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
Switch OpenCode to explicit static/local-aware model selection (#5117)
> **Stacked PR (part 4 of 7).** Depends on: - PR #5114 - PR #5115 - PR #5116 > Diff against `master` includes commits from earlier PRs in the stack — the new commit in this PR is the topmost one. ## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - When creating an OpenCode-local agent, Paperclip currently validates > `adapterConfig.model` against the *Paperclip host's* `opencode models` output > - SSH testing surfaced that this blocks creating an OpenCode agent for an SSH > environment: the model that exists on the SSH target isn't visible to the > host, so creation fails with "OpenCode requires `adapterConfig.model` in > provider/model format" even when the operator picked a real remote model > - The initial direction was environment-aware model discovery; the final > decision was to keep OpenCode on the same explicit-model pattern as other > adapters (default + curated list + manual override) and stop blocking > creation on host-side discovery > - This PR does both: the adapter-models endpoint now accepts `environmentId` and > probes against the target environment, and the create-time hard gate is > replaced by `requireOpenCodeModelId` which validates `provider/model` *format* > without requiring host-local discovery. Test/run-time still surfaces real > auth/availability problems > - The benefit is that operators can create OpenCode agents for remote > environments without out-of-band setup, and the model picker in the UI > reflects the actually-targeted environment ## What Changed - Added `requireOpenCodeModelId(input)` in `opencode-local/src/server/models.ts`, exported it from the adapter index - `ensureOpenCodeModelConfiguredAndAvailable` now delegates the format check to `requireOpenCodeModelId` - `agentsApi.adapterModels(companyId, adapterType, { environmentId })` now accepts an environment ID and passes it as a query parameter - `queryKeys.agents.adapterModels` now keys on `(companyId, adapterType, environmentId)` - `server/src/routes/agents.ts` reads and validates the new query parameter, forwarding it to the adapter's model probe - `AgentConfigForm.tsx` and `OnboardingWizard.tsx` build the model query key from the currently selected default environment ID and disable autodetect for `opencode_local` (model selection is explicit) - `NewAgent.tsx` simplified — no longer special-cases OpenCode autodetect - `company-portability.ts` no longer needs OpenCode-specific autodetect handling - Tests added/updated: `adapter-model-refresh-routes.test.ts`, `adapter-models.test.ts`, `agent-permissions-routes.test.ts`, `opencode-local/src/server/models.test.ts` ## Verification - `pnpm --filter @paperclipai/server test -- adapter-models adapter-model-refresh agent-permissions` - `pnpm --filter @paperclipai/adapter-opencode-local test` - `pnpm --filter @paperclipai/ui test -- AgentConfigForm OnboardingWizard NewAgent` - Manual QA in browser: 1. Boot Paperclip on Tailscale-bound port (so it's reachable from another machine), create an OpenCode-local agent, switch the default environment between two installed sandboxes, and confirm the model list refreshes per-environment 2. Submit with a malformed `provider/model` string and verify the new `requireOpenCodeModelId` error surfaces - Before/after screenshots attached for `AgentConfigForm` model picker ## Risks - Behavioural shift: switching default environment now triggers a model refetch. Should be cheap but introduces a new UI loading state for OpenCode users. - Removing dynamic autodetect for OpenCode: if any user configured an agent without specifying `model` and relied on autodetect populating it, that agent will now fail at submit time. Mitigation: validation error is explicit and actionable. - New query string parameter on `/api/companies/:id/adapter-models` — older clients that omit it still work (parameter is optional and defaults to null). ## Model Used - OpenAI GPT-5.4 (reasoning effort: high) via Codex CLI - Provider: OpenAI - Used to author the code changes in this PR ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [ ] I have updated relevant documentation to reflect my changes — N/A - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
Devin Foley
GitHub
parent
076067865f
commit
bb7d040894
14 changed files with 281 additions and 139 deletions
|
|
@ -1,8 +1,16 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { models as openCodeFallbackModels } from "@paperclipai/adapter-opencode-local";
|
||||
import type { ServerAdapterModule } from "../adapters/index.js";
|
||||
|
||||
vi.mock("acpx/runtime", () => ({
|
||||
createAcpRuntime: vi.fn(),
|
||||
createAgentRegistry: vi.fn(),
|
||||
createRuntimeStore: vi.fn(),
|
||||
isAcpRuntimeError: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
const mockAccessService = vi.hoisted(() => ({
|
||||
canUser: vi.fn(),
|
||||
hasPermission: vi.fn(),
|
||||
|
|
@ -19,6 +27,10 @@ const mockSecretService = vi.hoisted(() => ({
|
|||
normalizeAdapterConfigForPersistence: vi.fn(async (_companyId: string, config: Record<string, unknown>) => config),
|
||||
resolveAdapterConfigForRuntime: vi.fn(async (_companyId: string, config: Record<string, unknown>) => ({ config })),
|
||||
}));
|
||||
const mockEnvironmentService = vi.hoisted(() => ({
|
||||
getById: vi.fn(),
|
||||
}));
|
||||
const mockListOpenCodeModels = vi.hoisted(() => vi.fn());
|
||||
|
||||
const mockAgentInstructionsService = vi.hoisted(() => ({
|
||||
materializeManagedBundle: vi.fn(),
|
||||
|
|
@ -55,6 +67,14 @@ const mockInstanceSettingsService = vi.hoisted(() => ({
|
|||
const mockLogActivity = vi.hoisted(() => vi.fn());
|
||||
|
||||
function registerModuleMocks() {
|
||||
vi.doMock("@paperclipai/adapter-opencode-local/server", async () => {
|
||||
const actual = await vi.importActual<typeof import("@paperclipai/adapter-opencode-local/server")>("@paperclipai/adapter-opencode-local/server");
|
||||
return {
|
||||
...actual,
|
||||
listOpenCodeModels: mockListOpenCodeModels,
|
||||
};
|
||||
});
|
||||
|
||||
vi.doMock("../services/index.js", () => ({
|
||||
agentService: () => ({}),
|
||||
agentInstructionsService: () => mockAgentInstructionsService,
|
||||
|
|
@ -74,6 +94,10 @@ function registerModuleMocks() {
|
|||
vi.doMock("../services/instance-settings.js", () => ({
|
||||
instanceSettingsService: () => mockInstanceSettingsService,
|
||||
}));
|
||||
|
||||
vi.doMock("../services/environments.js", () => ({
|
||||
environmentService: () => mockEnvironmentService,
|
||||
}));
|
||||
}
|
||||
|
||||
const refreshableAdapterType = "refreshable_adapter_route_test";
|
||||
|
|
@ -147,6 +171,10 @@ describe("adapter model refresh route", () => {
|
|||
mockAccessService.ensureMembership.mockResolvedValue(undefined);
|
||||
mockAccessService.setPrincipalPermission.mockResolvedValue(undefined);
|
||||
mockLogActivity.mockResolvedValue(undefined);
|
||||
mockEnvironmentService.getById.mockReset();
|
||||
mockEnvironmentService.getById.mockResolvedValue(null);
|
||||
mockListOpenCodeModels.mockReset();
|
||||
mockListOpenCodeModels.mockResolvedValue([{ id: "dynamic-opencode-model", label: "dynamic-opencode-model" }]);
|
||||
await unregisterTestAdapter(refreshableAdapterType);
|
||||
});
|
||||
|
||||
|
|
@ -182,4 +210,42 @@ describe("adapter model refresh route", () => {
|
|||
expect(refreshModels).toHaveBeenCalledTimes(1);
|
||||
expect(listModels).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("skips OpenCode model discovery for non-local environments", async () => {
|
||||
mockEnvironmentService.getById.mockResolvedValue({
|
||||
id: "env-1",
|
||||
companyId: "company-1",
|
||||
name: "Remote SSH",
|
||||
driver: "ssh",
|
||||
config: {},
|
||||
});
|
||||
|
||||
const app = await createApp();
|
||||
const res = await requestApp(app, (baseUrl) =>
|
||||
request(baseUrl).get("/api/companies/company-1/adapters/opencode_local/models?environmentId=env-1"),
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toEqual(openCodeFallbackModels);
|
||||
expect(mockListOpenCodeModels).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps OpenCode model discovery enabled for local environments", async () => {
|
||||
mockEnvironmentService.getById.mockResolvedValue({
|
||||
id: "env-1",
|
||||
companyId: "company-1",
|
||||
name: "Local",
|
||||
driver: "local",
|
||||
config: {},
|
||||
});
|
||||
|
||||
const app = await createApp();
|
||||
const res = await requestApp(app, (baseUrl) =>
|
||||
request(baseUrl).get("/api/companies/company-1/adapters/opencode_local/models?environmentId=env-1"),
|
||||
);
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(200);
|
||||
expect(res.body).toEqual([{ id: "dynamic-opencode-model", label: "dynamic-opencode-model" }]);
|
||||
expect(mockListOpenCodeModels).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -7,6 +7,13 @@ import { listAdapterModels, refreshAdapterModels } from "../adapters/index.js";
|
|||
import { resetCodexModelsCacheForTests } from "../adapters/codex-models.js";
|
||||
import { resetCursorModelsCacheForTests, setCursorModelsRunnerForTests } from "../adapters/cursor-models.js";
|
||||
|
||||
vi.mock("acpx/runtime", () => ({
|
||||
createAcpRuntime: vi.fn(),
|
||||
createAgentRegistry: vi.fn(),
|
||||
createRuntimeStore: vi.fn(),
|
||||
isAcpRuntimeError: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
describe("adapter model listing", () => {
|
||||
beforeEach(() => {
|
||||
delete process.env.OPENAI_API_KEY;
|
||||
|
|
|
|||
|
|
@ -1,6 +1,14 @@
|
|||
import express from "express";
|
||||
import request from "supertest";
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
import { DEFAULT_OPENCODE_LOCAL_MODEL } from "@paperclipai/adapter-opencode-local";
|
||||
|
||||
vi.mock("acpx/runtime", () => ({
|
||||
createAcpRuntime: vi.fn(),
|
||||
createAgentRegistry: vi.fn(),
|
||||
createRuntimeStore: vi.fn(),
|
||||
isAcpRuntimeError: vi.fn(() => false),
|
||||
}));
|
||||
|
||||
const agentId = "11111111-1111-4111-8111-111111111111";
|
||||
const companyId = "22222222-2222-4222-8222-222222222222";
|
||||
|
|
@ -908,6 +916,78 @@ describe.sequential("agent permission routes", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("seeds opencode agent creation with the static default model without live discovery", async () => {
|
||||
mockEnsureOpenCodeModelConfiguredAndAvailable.mockRejectedValue(
|
||||
new Error("`opencode models` should not be called during creation"),
|
||||
);
|
||||
|
||||
const app = await createApp({
|
||||
type: "board",
|
||||
userId: "board-user",
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: true,
|
||||
companyIds: [companyId],
|
||||
});
|
||||
|
||||
const res = await requestApp(app, (baseUrl) => request(baseUrl)
|
||||
.post(`/api/companies/${companyId}/agents`)
|
||||
.send({
|
||||
name: "OpenCode Builder",
|
||||
role: "engineer",
|
||||
adapterType: "opencode_local",
|
||||
adapterConfig: {},
|
||||
}));
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockEnsureOpenCodeModelConfiguredAndAvailable).not.toHaveBeenCalled();
|
||||
expect(mockAgentService.create).toHaveBeenCalledWith(
|
||||
companyId,
|
||||
expect.objectContaining({
|
||||
adapterType: "opencode_local",
|
||||
adapterConfig: expect.objectContaining({
|
||||
model: DEFAULT_OPENCODE_LOCAL_MODEL,
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("accepts manual opencode provider/model values without host-side discovery", async () => {
|
||||
mockEnsureOpenCodeModelConfiguredAndAvailable.mockRejectedValue(
|
||||
new Error("`opencode models` should not be called during creation"),
|
||||
);
|
||||
|
||||
const app = await createApp({
|
||||
type: "board",
|
||||
userId: "board-user",
|
||||
source: "local_implicit",
|
||||
isInstanceAdmin: true,
|
||||
companyIds: [companyId],
|
||||
});
|
||||
|
||||
const res = await requestApp(app, (baseUrl) => request(baseUrl)
|
||||
.post(`/api/companies/${companyId}/agents`)
|
||||
.send({
|
||||
name: "OpenCode Builder",
|
||||
role: "engineer",
|
||||
adapterType: "opencode_local",
|
||||
adapterConfig: {
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
},
|
||||
}));
|
||||
|
||||
expect(res.status, JSON.stringify(res.body)).toBe(201);
|
||||
expect(mockEnsureOpenCodeModelConfiguredAndAvailable).not.toHaveBeenCalled();
|
||||
expect(mockAgentService.create).toHaveBeenCalledWith(
|
||||
companyId,
|
||||
expect.objectContaining({
|
||||
adapterType: "opencode_local",
|
||||
adapterConfig: expect.objectContaining({
|
||||
model: "anthropic/claude-sonnet-4-5",
|
||||
}),
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("normalizes hire requests to disable timer heartbeats by default", async () => {
|
||||
const app = await createApp({
|
||||
type: "board",
|
||||
|
|
|
|||
|
|
@ -84,7 +84,8 @@ import {
|
|||
} from "@paperclipai/adapter-codex-local";
|
||||
import { DEFAULT_CURSOR_LOCAL_MODEL } from "@paperclipai/adapter-cursor-local";
|
||||
import { DEFAULT_GEMINI_LOCAL_MODEL } from "@paperclipai/adapter-gemini-local";
|
||||
import { ensureOpenCodeModelConfiguredAndAvailable } from "@paperclipai/adapter-opencode-local/server";
|
||||
import { DEFAULT_OPENCODE_LOCAL_MODEL } from "@paperclipai/adapter-opencode-local";
|
||||
import { requireOpenCodeModelId } from "@paperclipai/adapter-opencode-local/server";
|
||||
import {
|
||||
loadDefaultAgentInstructionsBundle,
|
||||
resolveDefaultAgentInstructionsBundleRole,
|
||||
|
|
@ -767,7 +768,6 @@ export function agentRoutes(
|
|||
{ strictMode: strictSecretsMode },
|
||||
);
|
||||
await assertAdapterConfigConstraints(
|
||||
input.companyId,
|
||||
input.adapterType,
|
||||
input.constraintAdapterConfig
|
||||
? { ...input.constraintAdapterConfig, ...normalizedAdapterConfig }
|
||||
|
|
@ -864,7 +864,10 @@ export function agentRoutes(
|
|||
next.model = DEFAULT_GEMINI_LOCAL_MODEL;
|
||||
return ensureGatewayDeviceKey(adapterType, next);
|
||||
}
|
||||
// OpenCode requires explicit model selection — no default
|
||||
if (adapterType === "opencode_local" && !asNonEmptyString(next.model)) {
|
||||
next.model = DEFAULT_OPENCODE_LOCAL_MODEL;
|
||||
return ensureGatewayDeviceKey(adapterType, next);
|
||||
}
|
||||
if (adapterType === "cursor" && !asNonEmptyString(next.model)) {
|
||||
next.model = DEFAULT_CURSOR_LOCAL_MODEL;
|
||||
}
|
||||
|
|
@ -872,20 +875,12 @@ export function agentRoutes(
|
|||
}
|
||||
|
||||
async function assertAdapterConfigConstraints(
|
||||
companyId: string,
|
||||
adapterType: string | null | undefined,
|
||||
adapterConfig: Record<string, unknown>,
|
||||
) {
|
||||
if (adapterType !== "opencode_local") return;
|
||||
const { config: runtimeConfig } = await secretsSvc.resolveAdapterConfigForRuntime(companyId, adapterConfig);
|
||||
const runtimeEnv = asRecord(runtimeConfig.env) ?? {};
|
||||
try {
|
||||
await ensureOpenCodeModelConfiguredAndAvailable({
|
||||
model: runtimeConfig.model,
|
||||
command: runtimeConfig.command,
|
||||
cwd: runtimeConfig.cwd,
|
||||
env: runtimeEnv,
|
||||
});
|
||||
requireOpenCodeModelId(adapterConfig.model);
|
||||
} catch (err) {
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
throw unprocessable(`Invalid opencode_local adapterConfig: ${reason}`);
|
||||
|
|
@ -1194,6 +1189,17 @@ export function agentRoutes(
|
|||
const refresh = typeof req.query.refresh === "string"
|
||||
? ["1", "true", "yes"].includes(req.query.refresh.toLowerCase())
|
||||
: false;
|
||||
const environmentId = asNonEmptyString(req.query.environmentId);
|
||||
const environment = environmentId ? await environmentsSvc.getById(environmentId) : null;
|
||||
if (environmentId && (!environment || environment.companyId !== companyId)) {
|
||||
res.status(404).json({ error: "Environment not found" });
|
||||
return;
|
||||
}
|
||||
if (type === "opencode_local" && environment && environment.driver !== "local") {
|
||||
const adapter = requireServerAdapter(type);
|
||||
res.json(adapter.models ?? []);
|
||||
return;
|
||||
}
|
||||
const models = refresh
|
||||
? await refreshAdapterModels(type)
|
||||
: await listAdapterModels(type);
|
||||
|
|
|
|||
|
|
@ -48,7 +48,7 @@ import {
|
|||
readPaperclipSkillSyncPreference,
|
||||
writePaperclipSkillSyncPreference,
|
||||
} from "@paperclipai/adapter-utils/server-utils";
|
||||
import { ensureOpenCodeModelConfiguredAndAvailable } from "@paperclipai/adapter-opencode-local/server";
|
||||
import { requireOpenCodeModelId } from "@paperclipai/adapter-opencode-local/server";
|
||||
import { findServerAdapter } from "../adapters/index.js";
|
||||
import { forbidden, notFound, unprocessable } from "../errors.js";
|
||||
import { ghFetch, gitHubApiBase, resolveRawGitHubUrl } from "./github-fetch.js";
|
||||
|
|
@ -2781,20 +2781,12 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
|||
}
|
||||
|
||||
async function assertImportAdapterConfigConstraints(
|
||||
companyId: string,
|
||||
adapterType: string,
|
||||
adapterConfig: Record<string, unknown>,
|
||||
) {
|
||||
if (adapterType !== "opencode_local") return;
|
||||
const { config: runtimeConfig } = await secrets.resolveAdapterConfigForRuntime(companyId, adapterConfig);
|
||||
const runtimeEnv = isPlainRecord(runtimeConfig.env) ? runtimeConfig.env : {};
|
||||
try {
|
||||
await ensureOpenCodeModelConfiguredAndAvailable({
|
||||
model: runtimeConfig.model,
|
||||
command: runtimeConfig.command,
|
||||
cwd: runtimeConfig.cwd,
|
||||
env: runtimeEnv,
|
||||
});
|
||||
requireOpenCodeModelId(adapterConfig.model);
|
||||
} catch (err) {
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
throw unprocessable(`Invalid opencode_local adapterConfig: ${reason}`);
|
||||
|
|
@ -2824,7 +2816,7 @@ export function companyPortabilityService(db: Db, storage?: StorageService) {
|
|||
nextAdapterConfig,
|
||||
{ strictMode: strictSecretsMode },
|
||||
);
|
||||
await assertImportAdapterConfigConstraints(companyId, effectiveAdapterType, normalizedAdapterConfig);
|
||||
await assertImportAdapterConfigConstraints(effectiveAdapterType, normalizedAdapterConfig);
|
||||
return {
|
||||
adapterType: effectiveAdapterType,
|
||||
adapterConfig: normalizedAdapterConfig,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue