feat: add OpenCode extension - #402
AdityaZxxx wants to merge 5 commits into
Conversation
|
Thanks for contributing an extension to Vicinae! 👋 Before publication, this pull request receives two reviews:
✅ Ready for human review. The automated reviewer approved the latest commit and a maintainer has been notified. No blocking findings remain on the latest commit. The automated reviewer examines only the current commit. New commits invalidate its previous decision and start another review. |
clankus-aurelius
left a comment
There was a problem hiding this comment.
Found one publication-blocking credential exposure and four functional/UX defects.
Automated review found 1 publication-blocking issue.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
| export function parseServerUrl(raw: string): string | null { | ||
| const trimmed = raw.trim(); | ||
| if (!trimmed || !HTTP_URL_PATTERN.test(trimmed)) return null; | ||
| try { | ||
| const url = new URL(trimmed); | ||
| if (!url.hostname || url.username || url.password) return null; | ||
| // Strip any path/query so the endpoint is a bare origin as the client expects. | ||
| return `${url.protocol}//${url.host}`; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| function basicAuthHeaders(username?: string, password?: string): Record<string, string> | undefined { | ||
| if (!password) return undefined; | ||
| const user = username?.trim() || "opencode"; | ||
| const credentials = Buffer.from(`${user}:${password}`).toString("base64"); | ||
| return { authorization: `Basic ${credentials}` }; |
There was a problem hiding this comment.
🔴 Blocking — Basic-auth credentials can be sent over plaintext HTTP
Rule: SECURITY-003
Configured servers may use any http:// host, and the password is then placed in an Authorization header. A non-loopback HTTP server therefore receives the credential without transport encryption.
Suggested resolution: Require HTTPS whenever authentication is configured, except for explicitly recognized loopback hosts, or refuse to attach credentials to plaintext remote endpoints.
| readonly onRetry?: () => void; | ||
| }): ReactNode { | ||
| const [starting, setStarting] = useState(false); | ||
| const startable = props.error.kind === "unreachable"; |
There was a problem hiding this comment.
🟠 Warning — Remote connection errors offer an ineffective local start action
Rule: CORRECTNESS-001
Every unreachable error enables “Start OpenCode,” including failures from a configured serverUrl. Starting a local service cannot fix that connection because retry still resolves the configured remote URL.
Suggested resolution: Only offer the start action when local auto-discovery is in use.
| const startable = props.error.kind === "unreachable"; | |
| const startable = props.error.kind === "unreachable" && !props.config.serverUrl; |
| const columns = line.split("\t"); | ||
| if (columns.length < 3) continue; | ||
| const [addedRaw, deletedRaw, ...rest] = columns as [string, string, ...string[]]; | ||
| const additions = addedRaw === "-" ? -1 : Number.parseInt(addedRaw, 10); | ||
| const deletions = deletedRaw === "-" ? -1 : Number.parseInt(deletedRaw, 10); | ||
| if (Number.isNaN(additions) || Number.isNaN(deletions)) continue; | ||
| files.push({ path: rest.join("\t"), additions, deletions, untracked: false }); |
There was a problem hiding this comment.
🟠 Warning — Renamed files are parsed as nonexistent paths
Rule: CORRECTNESS-001
git --numstat represents renames using rename notation rather than a directly usable filesystem path. Joining all columns and later passing that display value after -- causes per-file diff loading and file icons to fail for renamed files.
Suggested resolution: Request a machine-readable rename format and parse old/new paths correctly, or disable rename detection so each emitted entry contains a real path.
| (modelId: string) => { | ||
| setSelectedModelId(modelId); | ||
| const model = parseModelRef(modelId); | ||
| if (sessionID && model) void service.switchModel(sessionID, model); | ||
| }, | ||
| [service, sessionID], | ||
| ); |
There was a problem hiding this comment.
🟠 Warning — Model-switch failures are silently ignored
Rule: UX-001
The UI records the selected model before firing switchModel without awaiting or catching it. If the request fails, the UI continues showing and tagging responses with a model the session never adopted, with no error feedback.
Suggested resolution: Await the switch for existing sessions, update selectedModelId only after success, and show a failure toast while retaining the prior selection on error.
| filtering | ||
| searchBarPlaceholder="Search models" |
There was a problem hiding this comment.
🟠 Warning — An empty model list remains in a permanent loading state
Rule: UX-002
After a successful request returning no models, failed remains false and models.length remains zero, so the picker displays an indefinite spinner rather than explaining that no models are configured.
Suggested resolution: Track loading separately from the loaded model array and render an actionable empty state after a successful empty response.
clankus-aurelius
left a comment
There was a problem hiding this comment.
The prior credential-exposure blocker remains because the new loopback check also trusts remote hostnames beginning with 127.. The other previous findings are resolved.
Automated review found 1 publication-blocking issue.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
|
|
||
| /** Loopback hosts accept plaintext HTTP; anything else needs HTTPS for credentials. */ | ||
| function isLoopbackHost(hostname: string): boolean { | ||
| const host = hostname.toLowerCase().replace(/^\[|\]$/g, "").replace(/\.$/, ""); |
There was a problem hiding this comment.
🔴 Blocking — Remote 127.* hostnames bypass the HTTPS requirement
Rule: SECURITY-003
/^127\./ accepts hostnames such as 127.attacker.example, not only IPv4 loopback addresses. A password configured for such an HTTP URL is therefore still sent in a plaintext Basic Authorization header.
Suggested resolution: Recognize only a normalized four-octet address in the 127/8 loopback range, in addition to localhost and ::1.
| const host = hostname.toLowerCase().replace(/^\[|\]$/g, "").replace(/\.$/, ""); | |
| return host === "localhost" || host.endsWith(".localhost") || host === "::1" || /^127(?:\.\d{1,3}){3}$/.test(host); |
clankus-aurelius
left a comment
There was a problem hiding this comment.
The prior credential-exposure blocker is resolved. The incremental changes introduce one command-injection risk and two user-visible state issues.
Automated review found 1 publication-blocking issue.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
| : `${props.config.openCodePath || "opencode"} --session ${props.session.id}`; | ||
| return ( | ||
| <ActionPanel> |
There was a problem hiding this comment.
🔴 Blocking — Copied resume command embeds remote values without shell quoting
Rule: SECURITY-002
The directory and session ID originate from OpenCode server responses, including supported remote servers, and are interpolated directly into a shell command. A crafted value containing shell syntax will execute unintended commands when the user pastes the advertised resume command.
Suggested resolution: Shell-quote the directory, executable, and session ID as individual arguments when constructing the copied command, using the same safe argument semantics as the actual terminal launch.
| searchText={searchText} | ||
| onSearchTextChange={setSearchText} | ||
| searchBarPlaceholder="Search sessions" | ||
| navigationTitle={props.navigationTitle} | ||
| searchBarAccessory={ | ||
| directories.length > 1 ? ( | ||
| <List.Dropdown | ||
| tooltip="Project" | ||
| value={directoryFilter} | ||
| onChange={(value) => setDirectoryFilter(value)} | ||
| > | ||
| <List.Dropdown.Item title="All Projects" value="all" /> | ||
| {directories.map((directory) => ( | ||
| <List.Dropdown.Item key={directory} title={tildify(directory) ?? directory} value={directory} /> | ||
| ))} | ||
| </List.Dropdown> | ||
| ) : undefined |
There was a problem hiding this comment.
🟠 Warning — Active project filter can disappear and trap the session list
Rule: CORRECTNESS-001
The dropdown is hidden whenever the current result set has at most one directory, but directoryFilter remains active. Searching or refreshing can therefore hide all returned sessions while also removing the control needed to select “All Projects.”
Suggested resolution: Keep the dropdown visible while a non-default filter is active, or reset directoryFilter to all when its directory is no longer available.
| : messages && messages.length > 0 | ||
| ? transcriptMarkdown(messages) | ||
| : "…"; | ||
|
|
||
| // Loading feedback: Detail has no isLoading prop, so an animated toast | ||
| // covers the wait and is hidden once the transcript arrives. |
There was a problem hiding this comment.
🟠 Warning — Empty transcripts remain displayed as loading
Rule: UX-002
A successful response containing zero transcript messages produces the same ellipsis as the loading state, even after the loading toast is hidden. Empty sessions therefore appear to load indefinitely.
Suggested resolution: Render the ellipsis only while messages is undefined and show an explicit empty-transcript message for an empty loaded array.
| : messages && messages.length > 0 | |
| ? transcriptMarkdown(messages) | |
| : "…"; | |
| // Loading feedback: Detail has no isLoading prop, so an animated toast | |
| // covers the wait and is hidden once the transcript arrives. | |
| const markdown = failed | |
| ? `**Transcript could not be loaded.**\n\n${failed}` | |
| : messages === undefined | |
| ? "…" | |
| : messages.length > 0 | |
| ? transcriptMarkdown(messages) | |
| : "**No transcript messages.**"; |
clankus-aurelius
left a comment
There was a problem hiding this comment.
All three prior findings are resolved. The incremental changes introduce no new actionable issues.
Automated extension review passed. A maintainer review is still required.
This is an AI-generated first pass and may be mistaken. If a finding is unclear or incorrect, reply in the relevant thread and mention @aurelleb.
|
@aurelleb automated review passed for |
Overview
This adds an OpenCode extension for Vicinae. It lets you use OpenCode from the launcher: browse sessions, start new work, ask questions, and review changes. It does not replace the OpenCode. OpenCode runs the agent and its tools. This extension covers finding things and quick actions.
It talks to OpenCode through the official
@opencode/client(V2) only. It finds a running local server on its own, or connects to a server you set in preferences, with optional basic auth.Commands
Notes
opencode2). V1 is not supported.Testing
bun test58 tests, all passing (status logic, API contract, error mapping, config defaults, terminal quoting, prompt building: used in the review command)npm run typecheckclean (strict mode,exactOptionalPropertyTypes)vici lint/vici buildmanifest valid, builds and installs cleanly