Skip to content

fix: harden opencode webui flows#1

Open
diamondplated wants to merge 1 commit intothreehymns:mainfrom
diamondplated:copilot/opencode-webui-hardening
Open

fix: harden opencode webui flows#1
diamondplated wants to merge 1 commit intothreehymns:mainfrom
diamondplated:copilot/opencode-webui-hardening

Conversation

@diamondplated
Copy link
Copy Markdown

Summary

  • fix Copilot auth, session routing, command handling, keyboard shortcuts, and dialog/accessibility issues
  • harden settings/provider handling so tokens are redacted and OAuth providers are represented truthfully
  • expose build provenance metadata from the built image and health endpoint

Validation

  • docker compose build app && docker compose up -d app on Harbor
  • backend build inside container
  • backend tests inside container
  • live smoke checks against /api/settings, /api/opencode/config/providers, /api/health, and /api/health/build

- fix Copilot auth/session/provider UX and command handling
- repair session routing, SSE refresh, keyboard shortcuts, and accessibility issues
- expose build provenance metadata from the running image

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 29, 2026 14:50
@roomote-v0
Copy link
Copy Markdown

roomote-v0 bot commented Mar 29, 2026

Rooviewer Clock   See task

Found 3 issues across the hardening changes:

  • Baseten provider template corrupted (frontend/src/lib/providerTemplates.ts) -- copy-paste error set npm to @opencode/github-copilot and requiresApiKey: false
  • Keyboard shortcuts broken on Mac (shared/src/schemas/settings.ts) -- defaults use ctrl which maps to literal Control, not Cmd; all Cmd-based shortcuts will fail to match on macOS
  • Proxy enriches /config/providers on error responses (backend/src/services/proxy.ts) -- response.json() called without status check; non-JSON error bodies will throw and mask the original error

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Comment on lines +203 to +204
npm: '@opencode/github-copilot',
requiresApiKey: false,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks like a copy-paste error from the GitHub Copilot changes. Baseten's npm was changed to @opencode/github-copilot and requiresApiKey to false, which will break Baseten provider setup for anyone using the add-provider dialog.

Suggested change
npm: '@opencode/github-copilot',
requiresApiKey: false,
npm: '@ai-sdk/openai-compatible',
requiresApiKey: true,

Fix it with Roo Code or mention @roomote and request a fix.

Comment on lines +29 to +31
compact: "ctrl+K",
newSession: "ctrl+N",
toggleMode: "Tab",
toggleMode: "ctrl+shift+P",
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

On Mac, these defaults will break keyboard shortcuts. The frontend's normalizeShortcut maps ctrl to literal Ctrl (the Control key), not Cmd (the Meta/Command key). So on Mac, pressing Cmd+Shift+P produces Cmd+Shift+P from parseEventShortcut, but the stored default normalizes to Ctrl+Shift+P -- they never match. The same issue affects compact, newSession, selectModel, undo, and redo.

The frontend's own DEFAULT_KEYBOARD_SHORTCUTS in types/settings.ts handles this correctly with a platform-aware CMD_KEY variable, but the backend overrides those with these shared defaults on every settings fetch.

Consider using a platform-neutral token like CmdOrCtrl or mod here, and mapping it in normalizeShortcut to the correct modifier per platform.

Fix it with Roo Code or mention @roomote and request a fix.

Comment on lines +82 to +92
if (request.method === 'GET' && cleanPath === '/config/providers') {
const payload = await response.json()
return new Response(JSON.stringify(enrichProviderPayload(payload)), {
status: response.status,
statusText: response.statusText,
headers: {
...responseHeaders,
'content-type': 'application/json',
},
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This calls response.json() unconditionally, regardless of the upstream HTTP status. If the OpenCode server returns a non-JSON error response (e.g., an HTML 502 from a reverse proxy), response.json() will throw and the outer catch will return a generic 502 with Proxy request failed, losing the original status code and error details. Consider guarding with if (!response.ok) and falling through to the normal Response passthrough for error statuses.

Fix it with Roo Code or mention @roomote and request a fix.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens OpenCode WebUI end-to-end flows (session routing, commands, keyboard shortcuts, dialogs/accessibility), improves provider/auth handling (including OAuth vs API-key representation and credential status), and adds build provenance metadata surfaced via health endpoints.

Changes:

  • Updated session routing to work both with and without a repo route param, and improved UI behavior around “waiting for input” tool states.
  • Reworked command handling (supported commands list + argument passing) and keyboard shortcut defaults/normalization.
  • Redacted sensitive settings fields (git token), enriched provider metadata/credential status (OAuth-aware), and exposed build metadata via /api/health and /api/health/build.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
shared/src/schemas/settings.ts Updates shared default keyboard shortcuts (compact/toggleMode).
frontend/src/pages/SessionDetail.tsx Improves session/repo resolution and removes Tab key mode toggle effect in favor of shortcut hook.
frontend/src/lib/providerTemplates.ts Updates provider templates (notably Copilot/OAuth-related fields).
frontend/src/hooks/useSSE.ts Adds repo-state invalidation on additional SSE events.
frontend/src/hooks/useKeyboardShortcuts.ts Improves shortcut normalization for cross-platform consistency.
frontend/src/hooks/useContextUsage.ts Adjusts current model inference from latest assistant message metadata.
frontend/src/hooks/useCommands.ts Narrows built-in commands to those supported and updates descriptions.
frontend/src/hooks/useCommandHandler.ts Adds command argument support and reworks special-command dispatch.
frontend/src/components/ui/dialog.tsx Improves dialog a11y prop handling (aria-describedby).
frontend/src/components/settings/SettingsDialog.tsx Adds an accessible dialog title.
frontend/src/components/settings/ProviderSettings.tsx Switches to per-provider credential status queries and OAuth-aware UI.
frontend/src/components/settings/GeneralSettings.tsx Reworks git token UX to explicit save/clear (token no longer echoed from settings).
frontend/src/components/session/ContextUsageIndicator.tsx Prefers live/session model when displaying model name.
frontend/src/components/model/ModelSelectDialog.tsx Removes per-session model command sending; improves search robustness and adds description.
frontend/src/components/message/ToolCallPart.tsx Adds specialized rendering for question tool input.
frontend/src/components/message/PromptInput.tsx Adds slash-command argument passing; improves “waiting for input” handling and model selection precedence.
frontend/src/components/message/MessageThread.tsx Adds “Waiting for input…” status display for question tool states.
frontend/src/components/file-browser/GitChangesSheet.tsx Adds accessible dialog title.
frontend/src/components/file-browser/FilePreviewDialog.tsx Adds accessible dialog title.
frontend/src/api/types/settings.ts Updates frontend default shortcut for toggleMode.
frontend/src/api/types.ts Extends SSE event type union to include repo-affecting events.
frontend/src/api/providers.ts Loads live providers from /api/opencode/config/providers, normalizes provider/model shapes, and adds credential-status shape.
frontend/index.html Adjusts PWA/meta tags (manifest link removed; capability meta changed).
backend/src/services/settings.ts Normalizes legacy shortcuts and redacts gitToken in client responses.
backend/src/services/proxy.ts Enriches /config/providers payload with authMethod and redacts OAuth apiKey option.
backend/src/services/build-info.ts Adds build-info loader with caching and sane defaults.
backend/src/services/auth.ts Adds credential status details and ensures auth file permissions on write.
backend/src/routes/settings.ts Ensures settings responses are redacted via toClientSettings().
backend/src/routes/providers.ts Adds credential status endpoint details and blocks API-key config for OAuth-only providers.
backend/src/routes/health.ts Adds build metadata to health response and exposes /api/health/build.
Dockerfile Generates and ships build-info.json into the runtime image; uses COPY --chmod.
.dockerignore Stops ignoring .git (affects build context/content).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 124 to 136
<CardTitle className="text-base flex items-center gap-2">
{provider.name || provider.id}
{hasKey ? (
<Badge variant="default" className="bg-green-600 hover:bg-green-700">
<Check className="h-3 w-3 mr-1" />
Configured
{statusLabel}
</Badge>
) : (
<Badge variant="secondary">
<X className="h-3 w-3 mr-1" />
No Key
{statusLabel}
</Badge>
)}
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

For OAuth providers, an expired token can still report hasCredentials=true (refresh token present), which makes the badge render with the “configured” (green) styling even when statusLabel is Expired. Consider basing the badge variant/color on credentialStatus.expired (and possibly type) instead of only hasKey, so “Expired”/“Not Connected” states aren’t shown as healthy.

Copilot uses AI. Check for mistakes.
Comment on lines 111 to +118
const commandMatch = prompt.match(/^\/([a-zA-Z0-9_-]+)(?:\s+(.*))?$/)
if (commandMatch) {
const [, commandName] = commandMatch
const command = filterCommands(commandName)[0]
const [, commandName, commandArguments = ''] = commandMatch
const command = commands.find((candidate) => candidate.name === commandName)

if (command) {

executeCommand(command)
executeCommand(command, commandArguments)
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

Command execution now requires an exact, case-sensitive name match (commands.find(... === commandName)), whereas the command search/filtering logic is case-insensitive and supports partial matches. This makes inputs like /Help or other casing variants silently fall through to sending a normal prompt. Consider normalizing commandName to lowercase (and/or reusing filterCommands(commandName) with an exact-preferred fallback) so slash-commands behave consistently with the suggestions UI.

Copilot uses AI. Check for mistakes.
Comment on lines +203 to +204
npm: '@opencode/github-copilot',
requiresApiKey: false,
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

The Baseten provider template was changed to use the GitHub Copilot package and marked as not requiring an API key. This looks incorrect for Baseten (docsUrl still points to Baseten) and will mislead users and/or generate invalid config. Revert Baseten to its appropriate provider package (likely the same OpenAI-compatible template used by other hosted providers) and keep requiresApiKey=true unless Baseten truly supports OAuth/device auth.

Suggested change
npm: '@opencode/github-copilot',
requiresApiKey: false,
npm: '@ai-sdk/openai-compatible',
options: {
baseURL: 'https://api.baseten.co/v1',
},
requiresApiKey: true,

Copilot uses AI. Check for mistakes.
temp

.git
.github
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

.git is no longer ignored in the Docker build context. This can significantly increase build time/context size and risks shipping repository history (and potentially sensitive data) into intermediate build layers/caches. Prefer keeping .git ignored and pass git SHA/dirty state via build args/labels (or CI env) to generate build-info.json without including the full repo history in the context.

Suggested change
.github
.github
.git

Copilot uses AI. Check for mistakes.
Comment on lines 5 to 11
<link rel="icon" type="image/svg+xml" href="/favicon.svg" />
<link rel="manifest" href="/manifest.json" />
<meta name="viewport" content="width=device-width, initial-scale=1.0, viewport-fit=cover" />
<meta name="theme-color" content="#0a0a0a" />
<meta name="description" content="AI-powered coding assistant interface" />
<meta name="apple-mobile-web-app-capable" content="yes" />
<meta name="mobile-web-app-capable" content="yes" />
<meta name="apple-mobile-web-app-status-bar-style" content="black-translucent" />
<meta name="apple-mobile-web-app-title" content="OpenCode" />
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

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

Removing the web app manifest link and replacing apple-mobile-web-app-capable with mobile-web-app-capable will break iOS PWA/standalone behavior and disables install metadata for browsers that rely on the manifest. If the intent is still to support PWA install/standalone mode, keep the manifest link and include apple-mobile-web-app-capable (you can also keep mobile-web-app-capable for non-iOS).

Copilot uses AI. Check for mistakes.
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