Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@ export const KNOWN_COMMANDS = [
const args = process.argv.slice(2);
const command = args[0];

// Global flags — take priority over subcommands
if (args.includes("--help") || args.includes("-h")) {
// Global flags — only when in the command position, so a subcommand argument
// named `--help`/`-v` (or a flag passed to a subcommand) isn't hijacked.
if (command === "--help" || command === "-h") {
await import("./commands/help.js");
} else if (args.includes("--version") || args.includes("-v")) {
} else if (command === "--version" || command === "-v") {
await import("./commands/version.js");
} else {
switch (command) {
Expand Down
4 changes: 2 additions & 2 deletions src/cli/config-io.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,9 @@ describe("getServicePort", () => {
expect(getServicePort(entry)).toBe(3000);
});

it("returns first port when service not found", () => {
it("returns null when a named service is not found (no silent fallback)", () => {
const entry = { ports: { web: 3000 } };
expect(getServicePort(entry, "missing")).toBe(3000);
expect(getServicePort(entry, "missing")).toBeNull();
});

it("returns port from legacy entry regardless of service param", () => {
Expand Down
6 changes: 4 additions & 2 deletions src/cli/config-io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,10 @@ export function getEntryPorts(entry: WorktreeEntry): number[] {
/** Get port for a specific service, with legacy fallback */
export function getServicePort(entry: WorktreeEntry, service?: string): number | null {
if ("ports" in entry) {
if (service && service in entry.ports) return entry.ports[service] ?? null;
// Fallback: first port
// A named service that isn't registered returns null rather than
// silently misrouting to another service's port. Only fall back to the
// first port when no service is requested.
if (service) return entry.ports[service] ?? null;
const values = Object.values(entry.ports);
return values[0] ?? null;
}
Expand Down
107 changes: 53 additions & 54 deletions src/commands/doctor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,21 +259,23 @@ describe("checkWorktreeConfig", () => {
});

it("validates portRange (min < max)", () => {
// worktrees come from the instance file (readProjectConfig); worktreeConfig
// and routes are resolved from dev-proxy.config.mjs into the ProjectConfig.
readProjectConfigMock.mockReturnValue({
worktrees: { feat1: { port: 4001 } },
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: {},
worktrees: {},
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
},
},
];

Expand All @@ -286,19 +288,19 @@ describe("checkWorktreeConfig", () => {
it("reports invalid portRange when min >= max", () => {
readProjectConfigMock.mockReturnValue({
worktrees: { feat1: { port: 4001 } },
worktreeConfig: {
portRange: [5000, 4000],
directory: "../{branch}",
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: {},
worktrees: {},
worktreeConfig: {
portRange: [5000, 4000],
directory: "../{branch}",
},
},
];

Expand All @@ -310,19 +312,19 @@ describe("checkWorktreeConfig", () => {
it("reports invalid portRange when min === max", () => {
readProjectConfigMock.mockReturnValue({
worktrees: { feat1: { port: 4001 } },
worktreeConfig: {
portRange: [5000, 5000],
directory: "../{branch}",
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: {},
worktrees: {},
worktreeConfig: {
portRange: [5000, 5000],
directory: "../{branch}",
},
},
];

Expand All @@ -333,25 +335,24 @@ describe("checkWorktreeConfig", () => {

it("cross-checks services against routes — warns for service not in routes", () => {
readProjectConfigMock.mockReturnValue({
routes: { app: "http://localhost:3000" },
worktrees: { feat1: { ports: { app: 4001, api: 4002 } } },
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
api: { env: "PORT_API" },
},
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
routes: {},
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: { app: "http://localhost:3000" },
worktrees: {},
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
api: { env: "PORT_API" },
},
},
},
];

Expand All @@ -364,25 +365,24 @@ describe("checkWorktreeConfig", () => {

it("reports valid when all services are in routes", () => {
readProjectConfigMock.mockReturnValue({
routes: { app: "http://localhost:3000", api: "http://localhost:4000" },
worktrees: { feat1: { ports: { app: 4001, api: 4002 } } },
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
api: { env: "PORT_API" },
},
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
routes: {},
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: { app: "http://localhost:3000", api: "http://localhost:4000" },
worktrees: {},
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
api: { env: "PORT_API" },
},
},
},
];

Expand Down Expand Up @@ -467,25 +467,24 @@ describe("checkWorktreeConfig", () => {

it("handles wildcard service in cross-check (does not warn for '*')", () => {
readProjectConfigMock.mockReturnValue({
routes: { app: "http://localhost:3000" },
worktrees: { feat1: { ports: { app: 4001 } } },
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
"*": { env: "PORT_DEFAULT" },
},
},
});

const projects: ProjectConfig[] = [
{
path: "/p1",
configPath: "/p1/.dev-proxy.json",
configType: "json",
routes: {},
configPath: "/p1/dev-proxy.config.mjs",
configType: "js",
routes: { app: "http://localhost:3000" },
worktrees: {},
worktreeConfig: {
portRange: [4000, 5000],
directory: "../{branch}",
services: {
app: { env: "PORT_APP" },
"*": { env: "PORT_DEFAULT" },
},
},
},
];

Expand Down
14 changes: 11 additions & 3 deletions src/commands/doctor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,11 @@ import * as net from "node:net";
import { Box, Text, render, useApp } from "ink";
import { config, CONFIG_DIR, GLOBAL_CONFIG_PATH } from "../proxy/config.js";
import type { ProjectConfig } from "../proxy/config.js";
import { getEntryPorts, readProjectConfig } from "../cli/config-io.js";
import {
getEntryPorts,
readProjectConfig,
type WorktreeConfig,
} from "../cli/config-io.js";
import { Header, Check, Section } from "../cli/output.js";

interface CheckResult {
Expand Down Expand Up @@ -182,7 +186,11 @@ function checkWorktreeConfig(projects: ProjectConfig[]): CheckResult[] {
for (const project of projects) {
const cfg = readProjectConfig(project.path);
const worktrees = cfg.worktrees ?? {};
const wtConfig = cfg.worktreeConfig;
// worktreeConfig and routes come from the resolved singleton, which merges
// dev-proxy.config.mjs (the standard format). readProjectConfig reads JSON
// only, so cfg.worktreeConfig is always undefined for mjs-based projects.
const wtConfig = project.worktreeConfig as WorktreeConfig | undefined;
const routes = project.routes;
const entries = Object.entries(worktrees);

if (entries.length === 0) continue;
Expand Down Expand Up @@ -222,7 +230,7 @@ function checkWorktreeConfig(projects: ProjectConfig[]): CheckResult[] {

// services vs routes cross-check
if (wtConfig.services) {
const routeKeys = new Set(Object.keys(cfg.routes ?? {}));
const routeKeys = new Set(Object.keys(routes));
for (const svc of Object.keys(wtConfig.services)) {
if (!routeKeys.has(svc) && svc !== "*") {
results.push({
Expand Down
11 changes: 7 additions & 4 deletions src/commands/init.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,11 @@ type Step =
// ── Validators ──────────────────────────────────────────────

function validatePort(value: string): string | null {
const num = parseInt(value, 10);
if (!isValidPort(num)) return `Invalid port "${value}" — must be 1-65535`;
// Number() rejects trailing garbage ("4000abc" → NaN); parseInt would not.
const num = Number(value.trim());
if (!value.trim() || !isValidPort(num)) {
return `Invalid port "${value}" — must be 1-65535`;
}
return null;
}

Expand Down Expand Up @@ -78,8 +81,8 @@ function parseRouteInput(value: string, existing: Route[]): ParseRouteResult {
}

const portStr = trimmed.slice(eq + 1).trim();
const portNum = parseInt(portStr, 10);
if (!isValidPort(portNum)) {
const portNum = Number(portStr);
if (!portStr || !isValidPort(portNum)) {
return { ok: false, error: `Invalid port "${portStr}" — must be 1-65535` };
}

Expand Down
16 changes: 16 additions & 0 deletions src/commands/migrate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,22 @@ describe("migrateProject", () => {
expect(mockUnlinkSync).toHaveBeenCalled();
});

it('returns "cleanup-failed" when the legacy file cannot be deleted', () => {
resolveProjectConfigFileMock.mockReturnValue(null);
mockExistsSync.mockReturnValue(true);
readProjectConfigMock.mockReturnValue({
routes: { app: "http://localhost:3000" },
});
mockUnlinkSync.mockImplementation(() => {
throw new Error("EACCES");
});

const result = migrateProject("/p");
// Contents were migrated, but the stale legacy file remains — surface it.
expect(result).toEqual({ path: "/p", status: "cleanup-failed" });
expect(writeJsConfigMock).toHaveBeenCalled();
});

it("moves worktreeConfig into mjs as the third argument", () => {
resolveProjectConfigFileMock.mockReturnValue(null);
mockExistsSync.mockReturnValue(true);
Expand Down
35 changes: 25 additions & 10 deletions src/commands/migrate.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface MigrateResult {
status:
| "migrated"
| "cleaned-legacy"
| "cleanup-failed"
| "skipped-no-legacy"
| "skipped-no-json"
| "skipped-no-routes";
Expand All @@ -39,8 +40,8 @@ function migrateProject(projectPath: string): MigrateResult {
const hasLegacyWtConfig = cfg.worktreeConfig !== undefined;

if (!hasWorktrees && !hasLegacyWtConfig) {
cleanupLegacyFile(legacyPath);
return { path: projectPath, status: "cleaned-legacy" };
const cleaned = cleanupLegacyFile(legacyPath);
return { path: projectPath, status: cleaned ? "cleaned-legacy" : "cleanup-failed" };
}

if (hasWorktrees) {
Expand All @@ -54,8 +55,8 @@ function migrateProject(projectPath: string): MigrateResult {
writeJsConfig(projectPath, routes, cfg.worktreeConfig);
}

cleanupLegacyFile(legacyPath);
return { path: projectPath, status: "cleaned-legacy" };
const cleaned = cleanupLegacyFile(legacyPath);
return { path: projectPath, status: cleaned ? "cleaned-legacy" : "cleanup-failed" };
}

if (!existsSync(legacyPath)) {
Expand All @@ -78,17 +79,19 @@ function migrateProject(projectPath: string): MigrateResult {
}

// .dev-proxy.json no longer holds anything — delete it.
cleanupLegacyFile(legacyPath);
const cleaned = cleanupLegacyFile(legacyPath);

return { path: projectPath, status: "migrated" };
return { path: projectPath, status: cleaned ? "migrated" : "cleanup-failed" };
}

function cleanupLegacyFile(legacyPath: string): void {
if (!existsSync(legacyPath)) return;
/** Delete the legacy file. Returns false if it still exists afterwards. */
function cleanupLegacyFile(legacyPath: string): boolean {
if (!existsSync(legacyPath)) return true;
try {
unlinkSync(legacyPath);
return true;
} catch {
/* best-effort cleanup */
return false;
}
}

Expand All @@ -113,8 +116,12 @@ function Migrate() {
const migrated = results.filter(
(r) => r.status === "migrated" || r.status === "cleaned-legacy",
);
const failed = results.filter((r) => r.status === "cleanup-failed");
const skipped = results.filter(
(r) => r.status !== "migrated" && r.status !== "cleaned-legacy",
(r) =>
r.status !== "migrated" &&
r.status !== "cleaned-legacy" &&
r.status !== "cleanup-failed",
);

return (
Expand All @@ -133,6 +140,14 @@ function Migrate() {
/>
))}

{failed.map((r) => (
<ErrorMessage
key={r.path}
message={`Migrated contents but failed to delete legacy .dev-proxy.json: ${r.path}`}
hint="Remove it manually to avoid config precedence ambiguity"
/>
))}

{skipped.map((r) => (
<Text key={r.path} dimColor>
{" "}
Expand Down
Loading