Fix base path encoding by nick-the-nuke · Pull Request #3 · prokube/opencode

nick-the-nuke · 2026-02-05T20:36:38Z

What does this PR do?

Fixes an issue where the CWD path gets corrupted because it tries to base64 decode the wrong part of the URL

If the CWD gets corrupted, the agents can't run bash commands.

This also fixes asset loading issues.

How did you verify your code works?

Merged into anomolyco dev locally, and ran it behind a reverse-proxy.

Fonts (inter, BlexMono) and audio files reference /assets/ directly in string literals. These need to be rewritten to include the basePath. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

…e.ai The proxy approach fails because app.opencode.ai serves the anomalyco frontend which lacks basePath support in the Router. Even with __OPENCODE_BASE_PATH__ injected, the Router doesn't use it. This change: - Add generate-app-manifest.ts to embed frontend assets at build time - Add app.ts with serveApp() to serve embedded assets with basePath rewriting - Update server.ts to use serveApp() instead of proxy() The embedded frontend includes the basePath-aware Router, so the session directory is correctly parsed from the URL. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR refactors the application serving mechanism to use embedded assets instead of proxying to app.opencode.ai, which fixes issues with base path encoding that caused CWD path corruption and asset loading problems. The change transitions from a proxy-based approach to serving assets directly from files embedded in the binary at build time.

Changes:

Added regex pattern to rewrite hardcoded "/assets/..." paths in JavaScript for fonts and audio files
Replaced proxy-based app serving with embedded asset serving using Bun's file embedding
Created a new script to generate an app manifest that embeds frontend assets at build time

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.

File	Description
packages/opencode/src/util/base-path.ts	Added regex replacement for hardcoded "/assets/" paths in string literals to support basePath
packages/opencode/src/server/server.ts	Removed proxy logic and unused imports, replaced with serveApp function call for embedded assets
packages/opencode/src/server/app.ts	New file implementing embedded asset serving with basePath rewriting, cache headers, and SPA routing
packages/opencode/script/generate-app-manifest.ts	New build script to scan and generate import statements for all frontend assets using Bun's file embedding

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

Copilot · 2026-02-05T21:05:23Z

packages/opencode/script/generate-app-manifest.ts

+  const assetEntries: string[] = []
+
+  for (const file of files) {
+    // Skip source maps in production


The comment on line 82 says "Skip source maps in production", but the code unconditionally skips all .map files regardless of environment. Either update the comment to say "Skip source maps" (without "in production"), or make the skipping conditional on environment if source maps should be included in development builds.

Suggested change

// Skip source maps in production

// Skip source maps

Copilot · 2026-02-05T21:05:24Z

packages/opencode/src/server/app.ts

+ * Assets are embedded into the binary at build time using Bun's file embedding.
+ */
+
+import { assets, indexHtmlPath, type Asset } from "./app-manifest"


The indexHtmlPath is imported from app-manifest but never used in this file. The code retrieves index.html from the assets Map directly instead. Consider removing this unused import or using it if it was intended for a specific purpose.

Suggested change

import { assets, indexHtmlPath, type Asset } from "./app-manifest"

import { assets, type Asset } from "./app-manifest"

Copilot · 2026-02-05T21:05:24Z

packages/opencode/src/util/base-path.ts


+  // Rewrite hardcoded "/assets/..." paths in string literals
+  // These are used for fonts (inter, BlexMono, etc.) and audio files (staplebops, nope, etc.)
+  result = result.replace(/"\/assets\//g, `"${basePath}/assets/`)


The regex pattern /"\/assets\//g will replace all occurrences of "/assets/ in the JavaScript code, including in comments and potentially in regex literals. While unlikely in minified production code, this could theoretically cause issues if the string appears in an unexpected context. Consider whether a more specific pattern is needed, or if the current broad approach is acceptable given that this targets minified Vite output where such edge cases are unlikely.

Suggested change

result = result.replace(/"\/assets\//g, `"${basePath}/assets/`)

result = result.replace(/"\/assets\/([^"]*)"/g, `"${basePath}/assets/$1"`)

Copilot · 2026-02-05T21:05:25Z

packages/opencode/src/util/base-path.ts

+  // Rewrite hardcoded "/assets/..." paths in string literals
+  // These are used for fonts (inter, BlexMono, etc.) and audio files (staplebops, nope, etc.)
+  result = result.replace(/"\/assets\//g, `"${basePath}/assets/`)


The new regex replacement for hardcoded "/assets/..." paths in JavaScript (line 107) lacks test coverage. Consider adding a test case in the existing test file (test/util/base-path.test.ts) to verify this functionality works correctly. Example test case: verify that a string like const fontPath = "/assets/fonts/Inter.woff2" is correctly rewritten to const fontPath = "/myapp/assets/fonts/Inter.woff2" when basePath is "/myapp".

Copilot · 2026-02-05T21:05:25Z

packages/opencode/src/server/app.ts

+  let content: string | ArrayBuffer = await file.arrayBuffer()
+
+  // Apply basePath rewriting if needed
+  if (basePath) {
+    const mime = asset.mime
+    if (mime.includes("text/html")) {
+      content = rewriteHtmlForBasePath(await file.text(), basePath)
+    } else if (mime.includes("javascript") || path.endsWith(".js")) {
+      content = rewriteJsForBasePath(await file.text(), basePath)
+    } else if (mime.includes("text/css") || path.endsWith(".css")) {
+      content = rewriteCssForBasePath(await file.text(), basePath)
+    }


The file content is read twice when basePath rewriting is needed. First, file.arrayBuffer() is called on line 75, then file.text() is called within the basePath rewriting blocks (lines 81, 83, 85). This results in reading the same file from disk twice. Consider reading the file only once by checking if basePath rewriting is needed before reading, or by converting the ArrayBuffer to text when needed instead of re-reading.

Suggested change

let content: string | ArrayBuffer = await file.arrayBuffer()

// Apply basePath rewriting if needed

if (basePath) {

const mime = asset.mime

if (mime.includes("text/html")) {

content = rewriteHtmlForBasePath(await file.text(), basePath)

} else if (mime.includes("javascript") || path.endsWith(".js")) {

content = rewriteJsForBasePath(await file.text(), basePath)

} else if (mime.includes("text/css") || path.endsWith(".css")) {

content = rewriteCssForBasePath(await file.text(), basePath)

}

const mime = asset.mime

let content: string | ArrayBuffer

// Apply basePath rewriting if needed

if (basePath && (mime.includes("text/html") || mime.includes("javascript") || mime.includes("text/css") || path.endsWith(".js") || path.endsWith(".css"))) {

const textContent = await file.text()

if (mime.includes("text/html")) {

content = rewriteHtmlForBasePath(textContent, basePath)

} else if (mime.includes("javascript") || path.endsWith(".js")) {

content = rewriteJsForBasePath(textContent, basePath)

} else if (mime.includes("text/css") || path.endsWith(".css")) {

content = rewriteCssForBasePath(textContent, basePath)

} else {

// Fallback: if conditions somehow don't match, serve original content

content = await file.arrayBuffer()

}

} else {

content = await file.arrayBuffer()

Copilot · 2026-02-05T21:05:25Z

packages/opencode/src/server/app.ts

+import { assets, indexHtmlPath, type Asset } from "./app-manifest"
+import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"
+


The generate-app-manifest.ts script creates the app-manifest.ts file that app.ts imports, but this script doesn't appear to be integrated into the build process. The build script (script/build.ts) or package.json should include a step to run this script before building, otherwise the build will fail with a missing module error. Consider adding this script to the build pipeline or documenting when it should be run.

Suggested change

import { assets, indexHtmlPath, type Asset } from "./app-manifest"

import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"

import type { Asset } from "./app-manifest"

import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"

// Lazily load the generated app-manifest at runtime so that the build

// does not fail if the manifest hasn't been generated yet. When the

// manifest is missing, we fall back to an empty asset map.

let assets = new Map<string, Asset>()

let indexHtmlPath = "/index.html"

// The dynamic import is wrapped in a try/catch so that missing manifest

// files don't cause runtime failures in environments where the embedded

// app is not built.

void (async () => {

try {

const manifest = await import("./app-manifest")

assets = manifest.assets as Map<string, Asset>

indexHtmlPath = manifest.indexHtmlPath as string

} catch {

// No embedded app manifest available; keep defaults.

}

})()

Copilot · 2026-02-05T21:05:26Z

packages/opencode/src/server/app.ts

+  if (staticRootFiles.includes(path)) return true
+
+  // Check by extension
+  const ext = path.substring(path.lastIndexOf("."))


When extracting the file extension, if a path has no dot (e.g., "/README"), lastIndexOf(".") returns -1 and path.substring(-1) returns the entire path. While this edge case is unlikely with Vite-built assets, the logic should handle it more explicitly. Consider using const ext = path.includes(".") ? path.substring(path.lastIndexOf(".")) : "" to handle paths without extensions more clearly.

Suggested change

const ext = path.substring(path.lastIndexOf("."))

const ext = path.includes(".") ? path.substring(path.lastIndexOf(".")) : ""

hsteude · 2026-02-05T21:17:46Z

@nick-the-nuke, thanks! Could you ask you AI what it thinks about copilots review points?

nick-the-nuke · 2026-02-06T08:22:09Z

@nick-the-nuke, thanks! Could you ask you AI what it thinks about copilots review points?

when I can catch a breather, sure. very busy at work this week.

nick-the-nuke and others added 2 commits February 5, 2026 13:06

fix(base-path): rewrite hardcoded /assets/ paths in JS

0ca4db0

Fonts (inter, BlexMono) and audio files reference /assets/ directly in string literals. These need to be rewritten to include the basePath. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

nick-the-nuke mentioned this pull request Feb 5, 2026

feat: base path support anomalyco/opencode#7625

Open

hsteude requested a review from Copilot February 5, 2026 21:00

Copilot started reviewing on behalf of hsteude February 5, 2026 21:00 View session

Copilot AI reviewed Feb 5, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix base path encoding#3

Fix base path encoding#3
nick-the-nuke wants to merge 2 commits intoprokube:feature/base-path-supportfrom
nick-the-nuke:base-path-support

nick-the-nuke commented Feb 5, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

Copilot AI Feb 5, 2026

Uh oh!

hsteude commented Feb 5, 2026 •

edited

Loading

Uh oh!

nick-the-nuke commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	import { assets, indexHtmlPath, type Asset } from "./app-manifest"
	import { assets, type Asset } from "./app-manifest"

	result = result.replace(/"\/assets\//g, `"${basePath}/assets/`)
	result = result.replace(/"\/assets\/([^"]*)"/g, `"${basePath}/assets/$1"`)

		import { assets, indexHtmlPath, type Asset } from "./app-manifest"
		import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"

-import { assets, indexHtmlPath, type Asset } from "./app-manifest"
-import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"
+import type { Asset } from "./app-manifest"
+import { rewriteHtmlForBasePath, rewriteJsForBasePath, rewriteCssForBasePath } from "../util/base-path"
+// Lazily load the generated app-manifest at runtime so that the build
+// does not fail if the manifest hasn't been generated yet. When the
+// manifest is missing, we fall back to an empty asset map.
+let assets = new Map<string, Asset>()
+let indexHtmlPath = "/index.html"
+// The dynamic import is wrapped in a try/catch so that missing manifest
+// files don't cause runtime failures in environments where the embedded
+// app is not built.
+void (async () => {
+  try {
+    const manifest = await import("./app-manifest")
+    assets = manifest.assets as Map<string, Asset>
+    indexHtmlPath = manifest.indexHtmlPath as string
+  } catch {
+    // No embedded app manifest available; keep defaults.
+  }
+})()

	const ext = path.substring(path.lastIndexOf("."))
	const ext = path.includes(".") ? path.substring(path.lastIndexOf(".")) : ""

Conversation

nick-the-nuke commented Feb 5, 2026

What does this PR do?

How did you verify your code works?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

hsteude commented Feb 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nick-the-nuke commented Feb 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hsteude commented Feb 5, 2026 •

edited

Loading