Skip to content

fix: pass adapter command from UI form for model discovery #1920

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mjaverto wants to merge 3 commits into
base: master
Choose a base branch
from
+72 −30
Open

fix: pass adapter command from UI form for model discovery #1920

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fc8755
fix: thread command option through listAdapterModels for opencode mod…
mjaverto Mar 27, 2026
f41a8d3
fix: correct getById call signature and add cross-company guard
mjaverto Mar 27, 2026
dbe6fca
fix: pass adapter command from UI form for model discovery
mjaverto Mar 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/adapter-utils/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ export interface ServerAdapterModule {
sessionManagement?: import("./session-compaction.js").AdapterSessionManagement;
supportsLocalAgentJwt?: boolean;
models?: AdapterModel[];
listModels?: () => Promise<AdapterModel[]>;
listModels?: (opts?: { command?: string }) => Promise<AdapterModel[]>;
agentConfigurationDoc?: string;
/**
* Optional lifecycle hook when an agent is approved/hired (join-request or hire_agent approval).
Expand Down
6 changes: 6 additions & 0 deletions packages/adapters/opencode-local/src/server/models.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ describe("openCode models", () => {
await expect(listOpenCodeModels()).resolves.toEqual([]);
});

it("uses command option when provided", async () => {
await expect(
listOpenCodeModels({ command: "__paperclip_missing_opencode_command__" }),
).resolves.toEqual([]);
});

it("rejects when model is missing", async () => {
await expect(
ensureOpenCodeModelConfiguredAndAvailable({ model: "" }),
Expand Down
4 changes: 2 additions & 2 deletions packages/adapters/opencode-local/src/server/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,9 @@ export async function ensureOpenCodeModelConfiguredAndAvailable(input: {
return models;
}

export async function listOpenCodeModels(): Promise<AdapterModel[]> {
export async function listOpenCodeModels(opts?: { command?: string }): Promise<AdapterModel[]> {
try {
return await discoverOpenCodeModelsCached();
return await discoverOpenCodeModelsCached({ command: opts?.command });
} catch {
return [];
}
Expand Down
4 changes: 2 additions & 2 deletions server/src/adapters/registry.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,11 +205,11 @@ export function getServerAdapter(type: string): ServerAdapterModule {
return adapter;
}

export async function listAdapterModels(type: string): Promise<{ id: string; label: string }[]> {
export async function listAdapterModels(type: string, opts?: { command?: string }): Promise<{ id: string; label: string }[]> {
const adapter = adaptersByType.get(type);
if (!adapter) return [];
if (adapter.listModels) {
const discovered = await adapter.listModels();
const discovered = await adapter.listModels(opts);
if (discovered.length > 0) return discovered;
}
return adapter.models ?? [];
Expand Down
9 changes: 8 additions & 1 deletion server/src/routes/agents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,14 @@ export function agentRoutes(db: Db) {
const companyId = req.params.companyId as string;
assertCompanyAccess(req, companyId);
const type = req.params.type as string;
const models = await listAdapterModels(type);
const command = typeof req.query.command === "string" ? req.query.command : undefined;
if (command !== undefined) {
// Reject shell metacharacters and empty/whitespace-only values
if (!command.trim() || /[;&|`$(){}[\]!#~]/.test(command)) {
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 27, 2026

Choose a reason for hiding this comment

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

P2 \ (backslash) and .. (path traversal) not rejected

The current regex blocks common shell metacharacters but does not block \ (Windows path separator) or relative path traversal sequences like ../../tmp/malicious. With shell: false none of these characters enable command injection, and the PR correctly acknowledges that reaching an arbitrary binary still requires pre-existing filesystem write access. However, if the goal is defense-in-depth, consider also rejecting \ and possibly enforcing that the value is an absolute path.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: server/src/routes/agents.ts
Line: 673

Comment:
**`\` (backslash) and `..` (path traversal) not rejected**

The current regex blocks common shell metacharacters but does not block `\` (Windows path separator) or relative path traversal sequences like `../../tmp/malicious`. With `shell: false` none of these characters enable command injection, and the PR correctly acknowledges that reaching an arbitrary binary still requires pre-existing filesystem write access. However, if the goal is defense-in-depth, consider also rejecting `\` and possibly enforcing that the value is an absolute path.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

return res.status(400).json({ error: "Invalid command" });
}
}
const models = await listAdapterModels(type, command ? { command } : undefined);
res.json(models);
});

Expand Down
11 changes: 7 additions & 4 deletions ui/src/api/agents.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,10 +155,13 @@ export const agentsApi = {
api.get<AgentTaskSession[]>(agentPath(id, companyId, "/task-sessions")),
resetSession: (id: string, taskKey?: string | null, companyId?: string) =>
api.post<void>(agentPath(id, companyId, "/runtime-state/reset-session"), { taskKey: taskKey ?? null }),
adapterModels: (companyId: string, type: string) =>
api.get<AdapterModel[]>(
`/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/models`,
),
adapterModels: (companyId: string, type: string, opts?: { command?: string }) => {
const base = `/companies/${encodeURIComponent(companyId)}/adapters/${encodeURIComponent(type)}/models`;
const params = new URLSearchParams();
if (opts?.command) params.set("command", opts.command);
const qs = params.toString();
return api.get<AdapterModel[]>(qs ? `${base}?${qs}` : base);
},
testEnvironment: (
companyId: string,
type: string,
Expand Down
23 changes: 19 additions & 4 deletions ui/src/components/AgentConfigForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,16 +303,31 @@ export function AgentConfigForm(props: AgentConfigFormProps) {
isLocal && shouldShowLegacyWorkingDirectoryField({ isCreate, adapterConfig: config });
const uiAdapter = useMemo(() => getUIAdapter(adapterType), [adapterType]);

// Fetch adapter models for the effective adapter type
// Resolve the current command value from form state (create or edit overlay)
const currentCommand = isCreate
? props.values.command
: eff("adapterConfig", "command", String(config.command ?? ""));

// Debounce command so every keystroke doesn't trigger a new fetch (server-side child process)
const [debouncedCommand, setDebouncedCommand] = useState(currentCommand);
useEffect(() => {
const timer = setTimeout(() => setDebouncedCommand(currentCommand), 800);
return () => clearTimeout(timer);
}, [currentCommand]);

// Fetch adapter models for the effective adapter type, passing command so
// adapters like opencode can discover models from the configured binary.
const {
data: fetchedModels,
error: fetchedModelsError,
} = useQuery({
queryKey: selectedCompanyId
? queryKeys.agents.adapterModels(selectedCompanyId, adapterType)
: ["agents", "none", "adapter-models", adapterType],
queryFn: () => agentsApi.adapterModels(selectedCompanyId!, adapterType),
? queryKeys.agents.adapterModels(selectedCompanyId, adapterType, debouncedCommand || undefined)
: ["agents", "none", "adapter-models", adapterType, debouncedCommand || ""],
queryFn: () =>
agentsApi.adapterModels(selectedCompanyId!, adapterType, debouncedCommand ? { command: debouncedCommand } : undefined),
enabled: Boolean(selectedCompanyId),
staleTime: 60_000,
});
const models = fetchedModels ?? externalModels ?? [];

Expand Down
12 changes: 8 additions & 4 deletions ui/src/components/NewIssueDialog.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,9 @@ export function NewIssueDialog() {
const selectedAssigneeAgentId = selectedAssignee.assigneeAgentId;
const selectedAssigneeUserId = selectedAssignee.assigneeUserId;

const assigneeAdapterType = (agents ?? []).find((agent) => agent.id === selectedAssigneeAgentId)?.adapterType ?? null;
const assigneeAgent = (agents ?? []).find((agent) => agent.id === selectedAssigneeAgentId);
const assigneeAdapterType = assigneeAgent?.adapterType ?? null;
const assigneeCommand = typeof assigneeAgent?.adapterConfig?.command === "string" ? assigneeAgent.adapterConfig.command : undefined;
const supportsAssigneeOverrides = Boolean(
assigneeAdapterType && ISSUE_OVERRIDE_ADAPTER_TYPES.has(assigneeAdapterType),
);
Expand Down Expand Up @@ -387,10 +389,12 @@ export function NewIssueDialog() {
const { data: assigneeAdapterModels } = useQuery({
queryKey:
effectiveCompanyId && assigneeAdapterType
? queryKeys.agents.adapterModels(effectiveCompanyId, assigneeAdapterType)
: ["agents", "none", "adapter-models", assigneeAdapterType ?? "none"],
queryFn: () => agentsApi.adapterModels(effectiveCompanyId!, assigneeAdapterType!),
? queryKeys.agents.adapterModels(effectiveCompanyId, assigneeAdapterType, assigneeCommand)
: ["agents", "none", "adapter-models", assigneeAdapterType ?? "none", assigneeCommand ?? ""],
queryFn: () =>
agentsApi.adapterModels(effectiveCompanyId!, assigneeAdapterType!, assigneeCommand ? { command: assigneeCommand } : undefined),
enabled: Boolean(effectiveCompanyId) && newIssueOpen && supportsAssigneeOverrides,
staleTime: 60_000,
});

const createIssue = useMutation({
Expand Down
10 changes: 6 additions & 4 deletions ui/src/components/OnboardingWizard.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,10 +199,12 @@ export function OnboardingWizard() {
isFetching: adapterModelsFetching
} = useQuery({
queryKey: createdCompanyId
? queryKeys.agents.adapterModels(createdCompanyId, adapterType)
: ["agents", "none", "adapter-models", adapterType],
queryFn: () => agentsApi.adapterModels(createdCompanyId!, adapterType),
enabled: Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 2
? queryKeys.agents.adapterModels(createdCompanyId, adapterType, command || undefined)
: ["agents", "none", "adapter-models", adapterType, command || ""],
queryFn: () =>
agentsApi.adapterModels(createdCompanyId!, adapterType, command ? { command } : undefined),
enabled: Boolean(createdCompanyId) && effectiveOnboardingOpen && step === 2,
staleTime: 60_000,
});
Comment on lines 200 to 208
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 27, 2026

Choose a reason for hiding this comment

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

P1 No debounce on command — per-keystroke subprocess spawning

Like NewAgent.tsx, this query uses the raw command state value directly. Each character the user types changes the query key, triggering a new HTTP request and a new server-side opencode models subprocess immediately. The 800 ms debounce added in AgentConfigForm.tsx has no effect here because this component manages its own command state and query independently.

A debouncedCommand local state (same 800 ms pattern) should be introduced for the query key and queryFn to prevent the subprocess storm on each keystroke.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/components/OnboardingWizard.tsx
Line: 200-208

Comment:
**No debounce on `command` — per-keystroke subprocess spawning**

Like `NewAgent.tsx`, this query uses the raw `command` state value directly. Each character the user types changes the query key, triggering a new HTTP request and a new server-side `opencode models` subprocess immediately. The 800 ms debounce added in `AgentConfigForm.tsx` has no effect here because this component manages its own `command` state and query independently.

A `debouncedCommand` local state (same 800 ms pattern) should be introduced for the query key and queryFn to prevent the subprocess storm on each keystroke.

How can I resolve this? If you propose a fix, please make it concise.

const isLocalAdapter =
adapterType === "claude_local" ||
Expand Down
4 changes: 2 additions & 2 deletions ui/src/lib/queryKeys.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ export const queryKeys = {
["agents", "instructions-bundle", id, "file", relativePath] as const,
keys: (agentId: string) => ["agents", "keys", agentId] as const,
configRevisions: (agentId: string) => ["agents", "config-revisions", agentId] as const,
adapterModels: (companyId: string, adapterType: string) =>
["agents", companyId, "adapter-models", adapterType] as const,
adapterModels: (companyId: string, adapterType: string, command?: string) =>
["agents", companyId, "adapter-models", adapterType, command ?? ""] as const,
},
issues: {
list: (companyId: string) => ["issues", companyId] as const,
Expand Down
9 changes: 6 additions & 3 deletions ui/src/pages/AgentDetail.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1428,13 +1428,16 @@ function ConfigurationTab({
const [awaitingRefreshAfterSave, setAwaitingRefreshAfterSave] = useState(false);
const lastAgentRef = useRef(agent);

const savedCommand = typeof agent.adapterConfig?.command === "string" ? agent.adapterConfig.command : undefined;
const { data: adapterModels } = useQuery({
queryKey:
companyId
? queryKeys.agents.adapterModels(companyId, agent.adapterType)
: ["agents", "none", "adapter-models", agent.adapterType],
queryFn: () => agentsApi.adapterModels(companyId!, agent.adapterType),
? queryKeys.agents.adapterModels(companyId, agent.adapterType, savedCommand)
: ["agents", "none", "adapter-models", agent.adapterType, savedCommand ?? ""],
queryFn: () =>
agentsApi.adapterModels(companyId!, agent.adapterType, savedCommand ? { command: savedCommand } : undefined),
enabled: Boolean(companyId),
staleTime: 60_000,
});

const updateAgent = useMutation({
Expand Down
8 changes: 5 additions & 3 deletions ui/src/pages/NewAgent.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,12 @@ export function NewAgent() {
isFetching: adapterModelsFetching,
} = useQuery({
queryKey: selectedCompanyId
? queryKeys.agents.adapterModels(selectedCompanyId, configValues.adapterType)
: ["agents", "none", "adapter-models", configValues.adapterType],
queryFn: () => agentsApi.adapterModels(selectedCompanyId!, configValues.adapterType),
? queryKeys.agents.adapterModels(selectedCompanyId, configValues.adapterType, configValues.command || undefined)
: ["agents", "none", "adapter-models", configValues.adapterType, configValues.command || ""],
queryFn: () =>
agentsApi.adapterModels(selectedCompanyId!, configValues.adapterType, configValues.command ? { command: configValues.command } : undefined),
enabled: Boolean(selectedCompanyId),
staleTime: 60_000,
});
Comment on lines 89 to 96
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot Mar 27, 2026

Choose a reason for hiding this comment

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

P1 Debounce bypassed — subprocess storm still possible

NewAgent.tsx uses configValues.command directly in both the queryKey and queryFn, with no debouncing. AgentConfigForm.tsx (also rendered on this page) has its own debounced query using debouncedCommand, but that only gates AgentConfigForm.tsx's own fetch. Because NewAgent.tsx's query fires immediately on every configValues.command change, each keystroke in the command field produces a new query key → new HTTP request → new server-side subprocess.

Since the two queries share a key only once debouncedCommand has caught up (after 800 ms of no typing), the debounce protection added in AgentConfigForm.tsx provides no benefit for the NewAgent.tsx create flow. OnboardingWizard.tsx has the same problem since its command state is also used directly without a debounce.

Fix: introduce a locally debounced command value in NewAgent.tsx (matching the 800 ms useEffect timer pattern already used in AgentConfigForm.tsx), and use that debounced value in the queryKey and queryFn instead of configValues.command directly.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: ui/src/pages/NewAgent.tsx
Line: 89-96

Comment:
**Debounce bypassed — subprocess storm still possible**

`NewAgent.tsx` uses `configValues.command` directly in both the `queryKey` and `queryFn`, with no debouncing. `AgentConfigForm.tsx` (also rendered on this page) has its own debounced query using `debouncedCommand`, but that only gates `AgentConfigForm.tsx`'s own fetch. Because `NewAgent.tsx`'s query fires immediately on every `configValues.command` change, each keystroke in the command field produces a new query key → new HTTP request → new server-side subprocess.

Since the two queries share a key only once `debouncedCommand` has caught up (after 800 ms of no typing), the debounce protection added in `AgentConfigForm.tsx` provides no benefit for the `NewAgent.tsx` create flow. `OnboardingWizard.tsx` has the same problem since its `command` state is also used directly without a debounce.

**Fix**: introduce a locally debounced command value in `NewAgent.tsx` (matching the 800 ms `useEffect` timer pattern already used in `AgentConfigForm.tsx`), and use that debounced value in the `queryKey` and `queryFn` instead of `configValues.command` directly.

How can I resolve this? If you propose a fix, please make it concise.


const { data: companySkills } = useQuery({
Expand Down
Loading