Skip to content

Commit c2fd69e

Browse files
yomybabyclaude
andauthored
feat(FR-3665): name the dev server after its issue, PR and what it does (#9049)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6548df9 commit c2fd69e

6 files changed

Lines changed: 647 additions & 39 deletions

File tree

‎.claude/skills/dev-server/SKILL.md‎

Lines changed: 37 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@ description: >
88
conversation history, set VITE_THEME_HEADER_COLOR to the matching hex so the dev
99
server's header reflects this Claude session's color. When `/rename <name>`
1010
is visible, slugify the name and pass it as PORTLESS_APP_NAME so the dev
11-
URL reflects the session name (falls back to FR-XXXX from the branch, then
12-
to the current PR number). When the current branch's PR description names
11+
URL reflects the session name; dev.mjs prepends the branch's FR number and
12+
the PR number itself, and derives a word from the PR title when there is no
13+
/rename, so pass the word only. When the current branch's PR description names
1314
a backend test server (bare IP, `host:port`, or full URL), set
1415
VITE_DEFAULT_API_ENDPOINT so the login screen pre-fills that endpoint; when a
1516
live session is connected to a different backend a dev-only banner flags the
@@ -77,22 +78,47 @@ Do not invent additional names or alternate hex values. If the user's `/color` a
7778

7879
## 2b. Decide the Portless app name (webui only)
7980

80-
`scripts/dev.mjs` reads `PORTLESS_APP_NAME` from the env and uses it as the Portless subdomain (`https://<name>.localhost:1355`). Pick a name with this priority:
81+
`PORTLESS_APP_NAME` supplies **only the descriptive word**. `scripts/dev.mjs` composes the
82+
full subdomain itself, putting the identifiers first and the word last:
8183

82-
1. **Most recent successful `/rename <name>` in conversation history** — slugify the arg (see rules below) and use that. This makes the dev URL match the human-readable session name (e.g. `iphoto-disk-cleanup` → `https://iphoto-disk-cleanup.localhost:1355`).
83-
2. **FR-XXXX in the current git branch** — `dev.mjs` already detects this when `PORTLESS_APP_NAME` is unset, so just **omit the env var** and let it derive `fr-XXXX` (e.g. `fr-2794`). Don't recompute and pass it back in.
84-
3. **Open PR number for the current branch** — only if (1) and (2) both miss and the current branch has an open PR. Use `gh pr view --json number -q '.number'` and pass `PORTLESS_APP_NAME=pr-<NNNN>`.
85-
4. **None of the above** — omit the env var; `dev.mjs` falls back to Portless's auto-derived name.
84+
```
85+
https://fr-3665-pr9049-statusline.localhost:1355
86+
\_____/ \____/ \________/
87+
branch looked PORTLESS_APP_NAME
88+
issue up by (this is the only part you supply)
89+
dev.mjs
90+
```
91+
92+
So there is exactly one question for you to answer: **is there a `/rename` to use?**
8693

87-
**Slug rules** (apply to `/rename` arg before passing as `PORTLESS_APP_NAME`):
94+
1. **Most recent successful `/rename <name>`** — slugify the arg (rules below) and pass it.
95+
The dev URL then carries the human-readable session name alongside the identifiers.
96+
2. **No `/rename`** — **omit the env var.** `dev.mjs` falls back to a few words from the PR
97+
title, then to the identifiers alone (`fr-3665-pr9049`), then to Portless's auto-derived
98+
name. Every fallback is already handled.
99+
100+
**Never pass the FR number or the PR number yourself.** `dev.mjs` derives the issue key from
101+
the branch and looks the PR up with one cached `gh` call, and it strips either identifier from
102+
your string if you pass it anyway — so `PORTLESS_APP_NAME=fr-3665` just yields `fr-3665-pr9049`,
103+
losing the descriptive part for nothing.
104+
105+
**Slug rules** (apply to the `/rename` arg before passing as `PORTLESS_APP_NAME`):
88106
- Lowercase.
89107
- Replace any character that isn't `[a-z0-9-]` with `-` (spaces, underscores, dots, slashes, non-ASCII all become `-`).
90108
- Collapse repeated `-` into a single `-`.
91109
- Trim leading/trailing `-`.
92-
- Cap at ~40 chars (Portless cert generation can choke on very long subdomains).
110+
- Keep it short — a word or three. `dev.mjs` caps the whole hostname at 50 chars and truncates
111+
the descriptive tail first, so a long name loses its own end, not the identifiers.
93112
- If the result is empty after sanitization, treat as unset.
94113

95-
`dev.mjs` re-applies the same sanitization defensively, so it's safe to pass a slightly imperfect string — but compute the clean form yourself so you can announce the right hostname to the user without re-reading Portless output.
114+
`dev.mjs` re-applies the same sanitization defensively, so it's safe to pass a slightly imperfect
115+
string. It is **not** safe to predict the hostname from your string alone any more — the issue and
116+
PR parts are added after you. Read the URL Portless prints, or the statusline's Portless link,
117+
before announcing it.
118+
119+
`PORTLESS_APP_NAME_EXACT=1` turns the composition off and uses your string verbatim. It exists for
120+
callers that own the whole hostname (a release preview, say); a dev server for a branch should not
121+
use it.
96122

97123
**Detecting `/rename` in history**: scan the current conversation for `<command-name>/rename</command-name>` blocks. Take the **most recent** one whose `<local-command-stdout>` does not look like an error (e.g. doesn't start with `Error` / `Invalid`). Use `<command-args>` as the raw input to the slug rules.
98124

@@ -206,7 +232,7 @@ Running without it costs no real type safety: `scripts/verify.sh`, the Husky pre
206232
pnpm dev
207233
```
208234

209-
If step **2b** picked a Portless app name from `/rename` or a PR number, also prefix `PORTLESS_APP_NAME='<slug>'`. If 2b selected the FR-XXXX branch fallback (option 2) or "none" (option 4), **omit** `PORTLESS_APP_NAME` — `dev.mjs` handles those itself.
235+
If step **2b** found a `/rename` to use, also prefix `PORTLESS_APP_NAME='<slug>'` — the descriptive word only. Otherwise **omit** it; `dev.mjs` derives the word from the PR title and adds the identifiers either way.
210236

211237
If step **2c** resolved a default API endpoint, also prefix `VITE_DEFAULT_API_ENDPOINT='<url>'`. If 2c resolved nothing, **omit** the variable entirely — do not pass an empty string.
212238

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
import { devWebUiOrigin } from './login.js';
2+
import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs';
3+
import { tmpdir } from 'node:os';
4+
import { join } from 'node:path';
5+
import { describe, expect, it } from 'vitest';
6+
7+
/**
8+
* A checkout `tryResolveRepoContext` accepts, sitting on `branch`. It needs the
9+
* root package name plus every REQUIRED_SOURCES entry to exist.
10+
*/
11+
function repoOnBranch(branch: string): string {
12+
const root = mkdtempSync(join(tmpdir(), 'bai-cli-login-'));
13+
writeFileSync(
14+
join(root, 'package.json'),
15+
JSON.stringify({ name: 'backend.ai-webui', version: '0.0.0-test' }),
16+
);
17+
mkdirSync(join(root, 'data'), { recursive: true });
18+
writeFileSync(join(root, 'data', 'schema.graphql'), 'type Query { a: Int }\n');
19+
mkdirSync(join(root, 'resources', 'i18n'), { recursive: true });
20+
mkdirSync(join(root, 'packages', 'backend.ai-webui-docs'), { recursive: true });
21+
mkdirSync(join(root, '.git'));
22+
writeFileSync(join(root, '.git', 'HEAD'), `ref: refs/heads/${branch}\n`);
23+
return root;
24+
}
25+
26+
/**
27+
* A Portless state dir. `pid` defaults to this test process, whose cwd is the
28+
* package directory — i.e. a live route that this checkout does NOT own, which is
29+
* what the liveness-but-not-ownership fallback is about.
30+
*/
31+
function portlessState(
32+
routes: Array<{ hostname: string; pid?: number }>,
33+
): string {
34+
const dir = mkdtempSync(join(tmpdir(), 'bai-cli-portless-'));
35+
writeFileSync(
36+
join(dir, 'routes.json'),
37+
JSON.stringify(
38+
routes.map((route, i) => ({
39+
hostname: route.hostname,
40+
port: 4000 + i,
41+
pid: route.pid ?? process.pid,
42+
})),
43+
),
44+
);
45+
return dir;
46+
}
47+
48+
/** A pid that is certainly not running, for the stale-entry cases. */
49+
const DEAD_PID = 2 ** 22 - 1;
50+
51+
describe('devWebUiOrigin', () => {
52+
const env = (stateDir?: string) => ({
53+
PORTLESS_PORT: '1357',
54+
...(stateDir ? { PORTLESS_STATE_DIR: stateDir } : {}),
55+
});
56+
57+
it('falls back to the bare issue key when no route is registered', () => {
58+
const cwd = repoOnBranch('fix/FR-3665-portless-app-name');
59+
expect(devWebUiOrigin(cwd, env(portlessState([])))).toBe(
60+
'https://fr-3665.localhost:1357',
61+
);
62+
});
63+
64+
// FR-3665: dev.mjs now names the server `<issue>-<pr>-<word>`, so the issue key
65+
// on its own is no longer a hostname once a PR exists.
66+
it('resolves to the composed route that is actually running', () => {
67+
const cwd = repoOnBranch('fix/FR-3665-portless-app-name');
68+
const state = portlessState([
69+
{ hostname: 'fr-3654.localhost' },
70+
{ hostname: 'fr-3665-pr9049-statusline.localhost' },
71+
]);
72+
expect(devWebUiOrigin(cwd, env(state))).toBe(
73+
'https://fr-3665-pr9049-statusline.localhost:1357',
74+
);
75+
});
76+
77+
it('ignores a route whose owning process is gone', () => {
78+
const cwd = repoOnBranch('FR-3665');
79+
const state = portlessState([
80+
{ hostname: 'fr-3665-pr9049-statusline.localhost', pid: DEAD_PID },
81+
]);
82+
expect(devWebUiOrigin(cwd, env(state))).toBe('https://fr-3665.localhost:1357');
83+
});
84+
85+
it('skips a dead route in favour of a live one', () => {
86+
const cwd = repoOnBranch('FR-3665');
87+
const state = portlessState([
88+
{ hostname: 'fr-3665-old.localhost', pid: DEAD_PID },
89+
{ hostname: 'fr-3665-pr9049-statusline.localhost' },
90+
]);
91+
expect(devWebUiOrigin(cwd, env(state))).toBe(
92+
'https://fr-3665-pr9049-statusline.localhost:1357',
93+
);
94+
});
95+
96+
it('ignores an alias route, which has no owning process', () => {
97+
const cwd = repoOnBranch('FR-3665');
98+
const state = portlessState([{ hostname: 'fr-3665-alias.localhost', pid: 0 }]);
99+
expect(devWebUiOrigin(cwd, env(state))).toBe('https://fr-3665.localhost:1357');
100+
});
101+
102+
it('prefers an exact match over a composed one', () => {
103+
const cwd = repoOnBranch('FR-3665');
104+
const state = portlessState([
105+
{ hostname: 'fr-3665-pr9049-statusline.localhost' },
106+
{ hostname: 'fr-3665.localhost' },
107+
]);
108+
expect(devWebUiOrigin(cwd, env(state))).toBe('https://fr-3665.localhost:1357');
109+
});
110+
111+
it('takes the shortest when several extend the issue key', () => {
112+
const cwd = repoOnBranch('FR-3665');
113+
const state = portlessState([
114+
{ hostname: 'fr-3665-pr9049-statusline-and-more.localhost' },
115+
{ hostname: 'fr-3665-alt.localhost' },
116+
]);
117+
expect(devWebUiOrigin(cwd, env(state))).toBe(
118+
'https://fr-3665-alt.localhost:1357',
119+
);
120+
});
121+
122+
it('does not match a different issue that merely shares a prefix', () => {
123+
const cwd = repoOnBranch('FR-366');
124+
const state = portlessState([
125+
{ hostname: 'fr-3665-pr9049-statusline.localhost' },
126+
]);
127+
expect(devWebUiOrigin(cwd, env(state))).toBe('https://fr-366.localhost:1357');
128+
});
129+
130+
it('survives a missing or malformed routes.json', () => {
131+
const cwd = repoOnBranch('FR-3665');
132+
const missing = mkdtempSync(join(tmpdir(), 'bai-cli-portless-'));
133+
expect(devWebUiOrigin(cwd, env(missing))).toBe('https://fr-3665.localhost:1357');
134+
135+
const bad = mkdtempSync(join(tmpdir(), 'bai-cli-portless-'));
136+
writeFileSync(join(bad, 'routes.json'), '{not json');
137+
expect(devWebUiOrigin(cwd, env(bad))).toBe('https://fr-3665.localhost:1357');
138+
139+
const wrongShape = mkdtempSync(join(tmpdir(), 'bai-cli-portless-'));
140+
writeFileSync(join(wrongShape, 'routes.json'), '{"hostname":"x"}');
141+
expect(devWebUiOrigin(cwd, env(wrongShape))).toBe(
142+
'https://fr-3665.localhost:1357',
143+
);
144+
145+
const noPid = mkdtempSync(join(tmpdir(), 'bai-cli-portless-'));
146+
writeFileSync(
147+
join(noPid, 'routes.json'),
148+
JSON.stringify([{ hostname: 'fr-3665-x.localhost', port: 1 }]),
149+
);
150+
expect(devWebUiOrigin(cwd, env(noPid))).toBe('https://fr-3665.localhost:1357');
151+
});
152+
153+
it('has no issue key to resolve on a branch without one', () => {
154+
const cwd = repoOnBranch('main');
155+
const state = portlessState([
156+
{ hostname: 'fr-3665-pr9049-statusline.localhost' },
157+
]);
158+
expect(devWebUiOrigin(cwd, env(state))).toBe('https://localhost:1357');
159+
});
160+
});

‎packages/backend.ai-agent-cli/src/commands/login.ts‎

Lines changed: 91 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ import {
1919
type StoredSession,
2020
} from '../session.js';
2121
import { execFileSync, spawn } from 'node:child_process';
22-
import { existsSync, readFileSync } from 'node:fs';
22+
import { existsSync, readFileSync, realpathSync } from 'node:fs';
23+
import { homedir } from 'node:os';
2324
import { join } from 'node:path';
2425
import { createInterface } from 'node:readline';
2526

@@ -36,20 +37,106 @@ export interface LoginData {
3637
user: WhoAmI;
3738
}
3839

40+
/** The working directory of a live process, or undefined if it is not running. */
41+
function processCwd(pid: number): string | undefined {
42+
try {
43+
// Linux: the symlink is also the liveness check — a dead pid throws.
44+
return realpathSync(`/proc/${pid}/cwd`);
45+
} catch {
46+
/* no /proc, or the pid is gone */
47+
}
48+
try {
49+
const out = execFileSync('lsof', ['-p', String(pid), '-a', '-d', 'cwd', '-Fn'], {
50+
encoding: 'utf8',
51+
stdio: ['ignore', 'pipe', 'ignore'],
52+
timeout: 2000,
53+
});
54+
const line = out.split('\n').find((l) => l.startsWith('n'));
55+
return line ? realpathSync(line.slice(1)) : undefined;
56+
} catch {
57+
return undefined;
58+
}
59+
}
60+
3961
/**
40-
* Same derivation `scripts/dev.mjs` uses for the Portless app name, so the
41-
* default WebUI origin matches the dev server started from this checkout.
62+
* The Portless route actually serving this checkout, if one is registered.
63+
*
64+
* `scripts/dev.mjs` names a dev server after its issue, its PR and what it does
65+
* (`fr-3665-pr9049-statusline`), so the issue key alone stops being a hostname the
66+
* moment a PR exists. Rather than duplicate that composition — it needs a `gh`
67+
* lookup, which does not belong in a CLI's default-origin path — read what is
68+
* actually running.
69+
*
70+
* `routes.json` alone is not enough to pick from: it keeps entries whose owner has
71+
* died, and a sibling worktree on the same issue registers a hostname with the same
72+
* prefix. So a candidate must be a live process whose cwd is this checkout, which is
73+
* the same ownership test the statusline uses. Where neither /proc nor lsof can
74+
* answer, fall back to any live route, then to the bare issue key.
75+
*/
76+
function liveAppName(
77+
issueApp: string,
78+
repoRoot: string | undefined,
79+
env: Record<string, string | undefined>,
80+
): string | undefined {
81+
const stateDir =
82+
env.PORTLESS_STATE_DIR?.trim() ||
83+
env.PORTLESS_HOME?.trim() ||
84+
join(homedir(), '.portless');
85+
let routes: unknown;
86+
try {
87+
routes = JSON.parse(readFileSync(join(stateDir, 'routes.json'), 'utf8'));
88+
} catch {
89+
return undefined; // no Portless state, unreadable, or malformed
90+
}
91+
if (!Array.isArray(routes)) return undefined;
92+
93+
const candidates = routes
94+
.map((route) => route as { hostname?: unknown; pid?: unknown })
95+
.filter(
96+
(route): route is { hostname: string; pid: number } =>
97+
typeof route?.hostname === 'string' &&
98+
typeof route?.pid === 'number' &&
99+
route.pid > 0,
100+
)
101+
.map((route) => ({ app: route.hostname.split('.')[0], pid: route.pid }))
102+
.filter(
103+
({ app }) => app === issueApp || app.startsWith(`${issueApp}-`),
104+
);
105+
if (candidates.length === 0) return undefined;
106+
107+
const owned: string[] = [];
108+
const live: string[] = [];
109+
for (const { app, pid } of candidates) {
110+
const cwd = processCwd(pid);
111+
if (cwd === undefined) continue; // dead pid, or unknowable — not a candidate
112+
live.push(app);
113+
if (repoRoot && cwd === repoRoot) owned.push(app);
114+
}
115+
// Exact match first, then shortest: a checkout also running `fr-3665-alt` still
116+
// resolves to its main server.
117+
const pick = (names: string[]) =>
118+
[...names].sort((a, b) =>
119+
a === issueApp ? -1 : b === issueApp ? 1 : a.length - b.length,
120+
)[0];
121+
return pick(owned) ?? pick(live);
122+
}
123+
124+
/**
125+
* The default WebUI origin for a dev server started from this checkout: the issue
126+
* key from the branch, resolved against the Portless routes that are actually up.
42127
*/
43128
export function devWebUiOrigin(
44129
cwd: string,
45130
env: Record<string, string | undefined> = process.env,
46131
): string {
47132
const port = env.PORTLESS_PORT?.trim() || DEFAULT_PORTLESS_PORT;
133+
const resolved = tryResolveRepoContext(cwd);
134+
const repoRoot = resolved.ok ? realpathSync(resolved.context.repoRoot) : undefined;
48135
const branch = currentBranch(cwd);
49136
const issue = branch ? /(?:^|[-_/])(fr-?\d+)/i.exec(branch) : null;
50137
if (!issue) return `https://localhost:${port}`;
51138
const app = issue[1].toLowerCase().replace(/^fr-?/, 'fr-');
52-
return `https://${app}.localhost:${port}`;
139+
return `https://${liveAppName(app, repoRoot, env) ?? app}.localhost:${port}`;
53140
}
54141

55142
function currentBranch(cwd: string): string | undefined {

0 commit comments

Comments
 (0)