Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR removes CI deprecation warnings, fixes frontend lint issues, and expands backend hardening coverage around control-plane auth, health endpoints, and OpenCode proxy behavior.
Changes:
- Update the validate GitHub Actions workflow to newer action versions and Node 22.
- Address frontend lint/type issues (hook deps, narrower types, safer config section handling, remove unused exports).
- Add/expand backend tests for health routes, control-plane auth behavior, and proxy path/repo validation.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
.github/workflows/validate.yml |
Updates action versions/Node and changes pnpm setup in the validation workflow. |
frontend/src/components/ui/virtualized-text-view.tsx |
Adjusts hook dependencies usage to satisfy linting. |
frontend/src/components/ui/form.tsx |
Removes an unused export. |
frontend/src/components/ui/button.tsx |
Stops exporting unused variant helper. |
frontend/src/components/ui/badge.tsx |
Stops exporting unused variant helper. |
frontend/src/components/settings/OpenCodeConfigManager.tsx |
Adds safer typed extraction of config sections to avoid any/invalid shapes. |
frontend/src/components/settings/McpManager.tsx |
Tightens MCP config typing and adjusts delete logic. |
frontend/src/components/settings/KeyboardShortcuts.tsx |
Memoizes/closures for stable hook deps and event listeners. |
frontend/src/components/settings/AddMcpServerDialog.tsx |
Tightens MCP config typing and parses timeout more explicitly. |
frontend/src/components/message/TextPart.tsx |
Narrows React element typing to avoid any. |
frontend/src/components/file-browser/FilePreview.tsx |
Refactors save button handler to be clearer and avoid floating promises. |
frontend/src/components/file-browser/FileBrowser.tsx |
Makes loadFiles stable via useCallback and fixes effect deps. |
frontend/src/api/providers.ts |
Adds normalization for provider model configs (capabilities → model fields) and removes any casts. |
backend/test/services/proxy.test.ts |
Adds hardening tests for proxy path/repo validation and header filtering. |
backend/test/routes/health.test.ts |
Adds coverage for readiness/build/process health behavior and error paths. |
backend/test/middleware/control-plane.test.ts |
Expands auth middleware tests (nested health public, WWW-Authenticate, query token restrictions, missing token behavior). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Set up Node.js | ||
| uses: actions/setup-node@v5 | ||
| with: | ||
| version: 9.15.0 | ||
| node-version: 22 | ||
| package-manager-cache: false | ||
|
|
||
| - name: Set up Node.js | ||
| uses: actions/setup-node@v4 | ||
| - name: Set up pnpm | ||
| run: | | ||
| corepack enable |
There was a problem hiding this comment.
actions/setup-node is configured with cache: pnpm before pnpm is installed. setup-node expects pnpm to already be on PATH when caching is enabled (it runs pnpm store path), so this can fail the workflow. Install pnpm first (e.g. pnpm/action-setup@v5 with run_install: false), or remove cache: pnpm / switch to an explicit cache step after installing pnpm.
| const artifactsRoot = path.resolve(process.cwd(), 'test-artifacts/proxy') | ||
| const reposRoot = path.resolve(process.cwd(), 'workspace/repos') | ||
|
|
||
| describe('proxyRequest hardening', () => { | ||
| beforeEach(async () => { | ||
| vi.restoreAllMocks() | ||
| vi.mocked(repoQueries.listRepos).mockReturnValue([]) | ||
| await fs.rm(artifactsRoot, { recursive: true, force: true }) | ||
| await fs.rm(reposRoot, { recursive: true, force: true }) | ||
| await fs.mkdir(artifactsRoot, { recursive: true }) | ||
| await fs.mkdir(reposRoot, { recursive: true }) |
There was a problem hiding this comment.
This test suite deletes and recreates ${process.cwd()}/workspace/repos in beforeEach/afterEach. That path is the default workspace location and may contain real developer data; it also risks cross-test interference when Vitest runs files in parallel. Prefer using a test-only temp directory under backend/test-artifacts/... and make getReposPath() resolve to it (e.g. by mocking @opencode-webui/shared or setting WORKSPACE_PATH before importing the module under test).
Summary
pnpm lintpasses againValidation