mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-18 19:50:38 +09:00
[codex] fix worktree dev dependency ergonomics (#3743)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - Local development needs to work cleanly across linked git worktrees because Paperclip itself leans on worktree-based engineering workflows > - Dev-mode asset routing, Vite watch behavior, and workspace package links are part of that day-to-day control-plane ergonomics > - The current branch had a small but coherent set of worktree/dev-tooling fixes that are independent from both the issue UI changes and the heartbeat runtime changes > - This pull request isolates those environment fixes into a standalone branch that can merge without carrying unrelated product work > - The benefit is a smoother multi-worktree developer loop with fewer stale links and less noisy dev watching ## What Changed - Serve dev public assets before the HTML shell and add a routing test that locks that behavior in. - Ignore UI test files in the Vite dev watch helper so the dev server does less unnecessary work. - Update `ensure-workspace-package-links.ts` to relink stale workspace dependencies whenever a workspace `node_modules` directory exists, instead of only inside linked-worktree detection paths. ## Verification - `pnpm vitest run server/src/__tests__/app-vite-dev-routing.test.ts ui/src/lib/vite-watch.test.ts` - `node cli/node_modules/tsx/dist/cli.mjs scripts/ensure-workspace-package-links.ts` ## Risks - The asset routing change is low risk but sits near app shell behavior, so a regression would show up as broken static assets in dev mode. - The workspace-link repair now runs in more cases, so the main risk is doing unexpected relinks when a checkout has intentionally unusual workspace symlink state. ## Model Used - OpenAI Codex, GPT-5-based coding agent in the Codex CLI environment. Exact backend model deployment ID was not exposed in-session. Tool-assisted editing and shell execution were used. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge
This commit is contained in:
parent
390502736c
commit
c1a02497b0
8 changed files with 101 additions and 18 deletions
|
|
@ -1,6 +1,6 @@
|
||||||
#!/usr/bin/env -S node --import tsx
|
#!/usr/bin/env -S node --import tsx
|
||||||
import fs from "node:fs/promises";
|
import fs from "node:fs/promises";
|
||||||
import { existsSync, lstatSync, readdirSync, readFileSync, realpathSync } from "node:fs";
|
import { existsSync, readdirSync, readFileSync, realpathSync } from "node:fs";
|
||||||
import path from "node:path";
|
import path from "node:path";
|
||||||
import { repoRoot } from "./dev-service-profile.ts";
|
import { repoRoot } from "./dev-service-profile.ts";
|
||||||
|
|
||||||
|
|
@ -43,20 +43,6 @@ function discoverWorkspacePackagePaths(rootDir: string): Map<string, string> {
|
||||||
return packagePaths;
|
return packagePaths;
|
||||||
}
|
}
|
||||||
|
|
||||||
function isLinkedGitWorktreeCheckout(rootDir: string) {
|
|
||||||
const gitMetadataPath = path.join(rootDir, ".git");
|
|
||||||
if (!existsSync(gitMetadataPath)) return false;
|
|
||||||
|
|
||||||
const stat = lstatSync(gitMetadataPath);
|
|
||||||
if (!stat.isFile()) return false;
|
|
||||||
|
|
||||||
return readFileSync(gitMetadataPath, "utf8").trimStart().startsWith("gitdir:");
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!isLinkedGitWorktreeCheckout(repoRoot)) {
|
|
||||||
process.exit(0);
|
|
||||||
}
|
|
||||||
|
|
||||||
const workspacePackagePaths = discoverWorkspacePackagePaths(repoRoot);
|
const workspacePackagePaths = discoverWorkspacePackagePaths(repoRoot);
|
||||||
const workspaceDirs = Array.from(
|
const workspaceDirs = Array.from(
|
||||||
new Set(
|
new Set(
|
||||||
|
|
@ -67,6 +53,11 @@ const workspaceDirs = Array.from(
|
||||||
).sort();
|
).sort();
|
||||||
|
|
||||||
function findWorkspaceLinkMismatches(workspaceDir: string): WorkspaceLinkMismatch[] {
|
function findWorkspaceLinkMismatches(workspaceDir: string): WorkspaceLinkMismatch[] {
|
||||||
|
const nodeModulesDir = path.join(repoRoot, workspaceDir, "node_modules");
|
||||||
|
if (!existsSync(nodeModulesDir)) {
|
||||||
|
return [];
|
||||||
|
}
|
||||||
|
|
||||||
const packageJson = readJsonFile(path.join(repoRoot, workspaceDir, "package.json"));
|
const packageJson = readJsonFile(path.join(repoRoot, workspaceDir, "package.json"));
|
||||||
const dependencies = {
|
const dependencies = {
|
||||||
...(packageJson.dependencies as Record<string, unknown> | undefined),
|
...(packageJson.dependencies as Record<string, unknown> | undefined),
|
||||||
|
|
|
||||||
27
server/src/__tests__/app-vite-dev-routing.test.ts
Normal file
27
server/src/__tests__/app-vite-dev-routing.test.ts
Normal file
|
|
@ -0,0 +1,27 @@
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import type { Request } from "express";
|
||||||
|
import { shouldServeViteDevHtml } from "../app.js";
|
||||||
|
|
||||||
|
function createRequest(path: string, acceptsResult: string | false): Request {
|
||||||
|
return {
|
||||||
|
path,
|
||||||
|
accepts: () => acceptsResult,
|
||||||
|
} as unknown as Request;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe("shouldServeViteDevHtml", () => {
|
||||||
|
it("serves HTML shell for document requests", () => {
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/", "html"))).toBe(true);
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/issues/abc", "html"))).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips public assets even when the client accepts */*", () => {
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/sw.js", "html"))).toBe(false);
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/site.webmanifest", "html"))).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("skips vite asset requests", () => {
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/@vite/client", "html"))).toBe(false);
|
||||||
|
expect(shouldServeViteDevHtml(createRequest("/src/main.tsx", "html"))).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -56,6 +56,7 @@ describe("shouldSilenceHttpSuccessLog", () => {
|
||||||
expect(shouldSilenceHttpSuccessLog("GET", "/@fs/Users/dotta/paperclip/ui/src/main.tsx", 200)).toBe(true);
|
expect(shouldSilenceHttpSuccessLog("GET", "/@fs/Users/dotta/paperclip/ui/src/main.tsx", 200)).toBe(true);
|
||||||
expect(shouldSilenceHttpSuccessLog("GET", "/src/App.tsx?t=123", 200)).toBe(true);
|
expect(shouldSilenceHttpSuccessLog("GET", "/src/App.tsx?t=123", 200)).toBe(true);
|
||||||
expect(shouldSilenceHttpSuccessLog("GET", "/site.webmanifest", 200)).toBe(true);
|
expect(shouldSilenceHttpSuccessLog("GET", "/site.webmanifest", 200)).toBe(true);
|
||||||
|
expect(shouldSilenceHttpSuccessLog("GET", "/sw.js", 200)).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("keeps normal successful application requests", () => {
|
it("keeps normal successful application requests", () => {
|
||||||
|
|
|
||||||
|
|
@ -70,6 +70,7 @@ const VITE_DEV_STATIC_PATHS = new Set([
|
||||||
"/favicon.ico",
|
"/favicon.ico",
|
||||||
"/favicon.svg",
|
"/favicon.svg",
|
||||||
"/site.webmanifest",
|
"/site.webmanifest",
|
||||||
|
"/sw.js",
|
||||||
]);
|
]);
|
||||||
|
|
||||||
export function resolveViteHmrPort(serverPort: number): number {
|
export function resolveViteHmrPort(serverPort: number): number {
|
||||||
|
|
@ -79,7 +80,7 @@ export function resolveViteHmrPort(serverPort: number): number {
|
||||||
return Math.max(1_024, serverPort - 10_000);
|
return Math.max(1_024, serverPort - 10_000);
|
||||||
}
|
}
|
||||||
|
|
||||||
function shouldServeViteDevHtml(req: ExpressRequest): boolean {
|
export function shouldServeViteDevHtml(req: ExpressRequest): boolean {
|
||||||
const pathname = req.path;
|
const pathname = req.path;
|
||||||
if (VITE_DEV_STATIC_PATHS.has(pathname)) return false;
|
if (VITE_DEV_STATIC_PATHS.has(pathname)) return false;
|
||||||
if (VITE_DEV_ASSET_PREFIXES.some((prefix) => pathname.startsWith(prefix))) return false;
|
if (VITE_DEV_ASSET_PREFIXES.some((prefix) => pathname.startsWith(prefix))) return false;
|
||||||
|
|
@ -347,6 +348,7 @@ export async function createApp(
|
||||||
|
|
||||||
if (opts.uiMode === "vite-dev") {
|
if (opts.uiMode === "vite-dev") {
|
||||||
const uiRoot = path.resolve(__dirname, "../../ui");
|
const uiRoot = path.resolve(__dirname, "../../ui");
|
||||||
|
const publicUiRoot = path.resolve(uiRoot, "public");
|
||||||
const hmrPort = resolveViteHmrPort(opts.serverPort);
|
const hmrPort = resolveViteHmrPort(opts.serverPort);
|
||||||
const { createServer: createViteServer } = await import("vite");
|
const { createServer: createViteServer } = await import("vite");
|
||||||
const vite = await createViteServer({
|
const vite = await createViteServer({
|
||||||
|
|
@ -369,6 +371,9 @@ export async function createApp(
|
||||||
});
|
});
|
||||||
const renderViteHtml = viteHtmlRenderer;
|
const renderViteHtml = viteHtmlRenderer;
|
||||||
|
|
||||||
|
if (fs.existsSync(publicUiRoot)) {
|
||||||
|
app.use(express.static(publicUiRoot, { index: false }));
|
||||||
|
}
|
||||||
app.get(/.*/, async (req, res, next) => {
|
app.get(/.*/, async (req, res, next) => {
|
||||||
if (!shouldServeViteDevHtml(req)) {
|
if (!shouldServeViteDevHtml(req)) {
|
||||||
next();
|
next();
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@ const SILENCED_SUCCESS_STATIC_PREFIXES = [
|
||||||
const SILENCED_SUCCESS_STATIC_PATHS = new Set([
|
const SILENCED_SUCCESS_STATIC_PATHS = new Set([
|
||||||
"/favicon.ico",
|
"/favicon.ico",
|
||||||
"/site.webmanifest",
|
"/site.webmanifest",
|
||||||
|
"/sw.js",
|
||||||
]);
|
]);
|
||||||
|
|
||||||
function normalizePath(url: string): string {
|
function normalizePath(url: string): string {
|
||||||
|
|
|
||||||
29
ui/src/lib/vite-watch.test.ts
Normal file
29
ui/src/lib/vite-watch.test.ts
Normal file
|
|
@ -0,0 +1,29 @@
|
||||||
|
import { describe, expect, it } from "vitest";
|
||||||
|
import { createUiDevWatchOptions, shouldIgnoreUiDevWatchPath } from "./vite-watch";
|
||||||
|
|
||||||
|
describe("shouldIgnoreUiDevWatchPath", () => {
|
||||||
|
it("ignores test-only files and folders", () => {
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/src/components/IssuesList.test.tsx")).toBe(true);
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/src/lib/issue-tree.spec.ts")).toBe(true);
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/src/__tests__/helpers.ts")).toBe(true);
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/tests/helpers.ts")).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps runtime source files watchable", () => {
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/src/components/IssuesList.tsx")).toBe(false);
|
||||||
|
expect(shouldIgnoreUiDevWatchPath("/repo/ui/src/pages/IssueDetail.tsx")).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("createUiDevWatchOptions", () => {
|
||||||
|
it("preserves the WSL /mnt polling fallback", () => {
|
||||||
|
expect(createUiDevWatchOptions("/mnt/c/paperclip")).toMatchObject({
|
||||||
|
usePolling: true,
|
||||||
|
interval: 1000,
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
it("always includes the ignored-path predicate", () => {
|
||||||
|
expect(createUiDevWatchOptions("/Users/dotta/paperclip")).toHaveProperty("ignored");
|
||||||
|
});
|
||||||
|
});
|
||||||
29
ui/src/lib/vite-watch.ts
Normal file
29
ui/src/lib/vite-watch.ts
Normal file
|
|
@ -0,0 +1,29 @@
|
||||||
|
const TEST_DIRECTORY_NAMES = new Set([
|
||||||
|
"__tests__",
|
||||||
|
"_tests",
|
||||||
|
"test",
|
||||||
|
"tests",
|
||||||
|
]);
|
||||||
|
|
||||||
|
const TEST_FILE_BASENAME_RE = /\.(test|spec)\.[^/]+$/i;
|
||||||
|
|
||||||
|
export function shouldIgnoreUiDevWatchPath(watchedPath: string): boolean {
|
||||||
|
const normalizedPath = String(watchedPath).replaceAll("\\", "/");
|
||||||
|
if (normalizedPath.length === 0) return false;
|
||||||
|
|
||||||
|
const segments = normalizedPath.split("/");
|
||||||
|
const basename = segments.at(-1) ?? normalizedPath;
|
||||||
|
|
||||||
|
return segments.some((segment) => TEST_DIRECTORY_NAMES.has(segment))
|
||||||
|
|| TEST_FILE_BASENAME_RE.test(basename);
|
||||||
|
}
|
||||||
|
|
||||||
|
export function createUiDevWatchOptions(currentWorkingDirectory: string) {
|
||||||
|
return {
|
||||||
|
ignored: shouldIgnoreUiDevWatchPath,
|
||||||
|
// WSL2 /mnt/ drives don't support inotify — fall back to polling so HMR works.
|
||||||
|
...(currentWorkingDirectory.startsWith("/mnt/")
|
||||||
|
? { usePolling: true, interval: 1000 }
|
||||||
|
: {}),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
@ -2,6 +2,7 @@ import path from "path";
|
||||||
import { defineConfig } from "vite";
|
import { defineConfig } from "vite";
|
||||||
import react from "@vitejs/plugin-react";
|
import react from "@vitejs/plugin-react";
|
||||||
import tailwindcss from "@tailwindcss/vite";
|
import tailwindcss from "@tailwindcss/vite";
|
||||||
|
import { createUiDevWatchOptions } from "./src/lib/vite-watch";
|
||||||
|
|
||||||
export default defineConfig(({ mode }) => ({
|
export default defineConfig(({ mode }) => ({
|
||||||
plugins: [react(), tailwindcss()],
|
plugins: [react(), tailwindcss()],
|
||||||
|
|
@ -23,8 +24,7 @@ export default defineConfig(({ mode }) => ({
|
||||||
},
|
},
|
||||||
server: {
|
server: {
|
||||||
port: 5173,
|
port: 5173,
|
||||||
// WSL2 /mnt/ drives don't support inotify — fall back to polling so HMR works
|
watch: createUiDevWatchOptions(process.cwd()),
|
||||||
watch: process.cwd().startsWith("/mnt/") ? { usePolling: true, interval: 1000 } : undefined,
|
|
||||||
proxy: {
|
proxy: {
|
||||||
"/api": {
|
"/api": {
|
||||||
target: "http://localhost:3100",
|
target: "http://localhost:3100",
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue