Skip to content

Commit 259a3c0

Browse files
committed
fix(gateway): ride out port release race on restart; add machine assignment to API key clients
- restart_gateway: wait for the released port to actually free before rebinding, mirroring autostart's port-wait race handling - useGatewayControl.restart: skip the pre-probe (we still hold the port at probe time) and go straight to restartGateway, falling back to the bind-failure dialog on genuine conflict - RegisterApiKeyClientModal: optional machine assignment (existing or new) when registering an API key client - migration 040: rename gateway.public_url setting key to gateway.public_base_url Unrelated pre-existing WIP on this branch, committed separately before starting the workspace-binding-popup-loop-fix plan. Signed-off-by: crimsonsunset <jsangio1@gmail.com>
1 parent 0b52f0f commit 259a3c0

6 files changed

Lines changed: 277 additions & 15 deletions

File tree

apps/desktop/src-tauri/src/commands/gateway.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,9 @@
55
use crate::commands::server_manager::ServerManagerState;
66
use crate::services::ui_events::OAUTH_CONSENT_REQUEST_CHANNEL;
77
use crate::AppState;
8-
use mcpmux_core::service::{allocate_dynamic_port, is_port_available};
8+
use mcpmux_core::service::{
9+
allocate_dynamic_port, is_port_available, wait_for_port_available, AUTOSTART_PORT_WAIT,
10+
};
911
use mcpmux_core::DomainEvent;
1012
use mcpmux_gateway::admin::ui_events::AdminUiEventBus;
1113
use mcpmux_gateway::{
@@ -1623,6 +1625,21 @@ pub async fn restart_gateway(
16231625
shutdown_gateway_handle(h).await;
16241626
}
16251627

1628+
// The shutdown above is graceful (or force-aborted after a 2s timeout),
1629+
// but the OS may hold the socket a moment longer than the in-process
1630+
// handle takes to resolve. Ride out that brief window the same way
1631+
// auto-start rides out the self-update restart race, so start_gateway's
1632+
// single is_port_available() check below doesn't spuriously report the
1633+
// port we just released as taken.
1634+
let (preferred_port, _source) = resolve_preferred_port(&app_state, port).await;
1635+
if !wait_for_port_available(preferred_port, AUTOSTART_PORT_WAIT).await {
1636+
warn!(
1637+
"[Gateway] Restart: port {} still unavailable after waiting — \
1638+
deferring to start_gateway's normal conflict handling",
1639+
preferred_port
1640+
);
1641+
}
1642+
16261643
// Start with new config
16271644
start_gateway(
16281645
port,

apps/desktop/src/features/clients/RegisterApiKeyClientModal.tsx

Lines changed: 115 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,27 @@
1010
* never display it again — if lost, revoke it and issue a new one.
1111
*/
1212

13-
import { useState } from 'react';
13+
import { useEffect, useState } from 'react';
1414
import { AlertTriangle, Check, Copy, KeyRound, Loader2, ShieldCheck, X } from 'lucide-react';
1515
import { Button, Card, CardContent, CardDescription, CardHeader, CardTitle } from '@mcpmux/ui';
1616
import { registerApiKeyClient, type RegisteredApiKeyClient } from '@/lib/api/gateway';
17+
import {
18+
createMachine,
19+
getHostname,
20+
listMachines,
21+
setClientMachineId,
22+
type Machine,
23+
} from '@/lib/api/machines';
24+
import {
25+
getMissingMachineProfileField,
26+
toMachineProfilePayload,
27+
} from '@/lib/machine-profile.helpers';
28+
import { EmojiPickerButton } from '@/components/emoji-picker-button.component';
1729
import { useSpaces } from '@/stores';
1830

31+
/** Sentinel `<select>` value that reveals the "create new machine" sub-form. */
32+
const NEW_MACHINE_OPTION = '__new__';
33+
1934
interface RegisterApiKeyClientModalProps {
2035
onClose: () => void;
2136
/** Called once the client + key are created, so the page can refresh. */
@@ -37,19 +52,62 @@ export function RegisterApiKeyClientModal({
3752
const [result, setResult] = useState<RegisteredApiKeyClient | null>(null);
3853
const [copied, setCopied] = useState(false);
3954

55+
const [machines, setMachines] = useState<Machine[]>([]);
56+
const [selectedMachineId, setSelectedMachineId] = useState('');
57+
const [machineName, setMachineName] = useState('');
58+
const [machineIcon, setMachineIcon] = useState('');
59+
const [machineHostname, setMachineHostname] = useState('');
60+
const isCreatingMachine = selectedMachineId === NEW_MACHINE_OPTION;
61+
62+
useEffect(() => {
63+
void listMachines()
64+
.then(setMachines)
65+
.catch(() => undefined);
66+
}, []);
67+
4068
const handleGenerate = async () => {
4169
const trimmed = name.trim();
4270
if (!trimmed) {
4371
setError('Give the client a name so you can recognise it later.');
4472
return;
4573
}
74+
if (isCreatingMachine) {
75+
const missingField = getMissingMachineProfileField({
76+
name: machineName,
77+
icon: machineIcon,
78+
hostname: machineHostname,
79+
});
80+
if (missingField) {
81+
setError(`New machine ${missingField} is required, or switch back to "No machine".`);
82+
return;
83+
}
84+
}
85+
4686
setIsSubmitting(true);
4787
setError(null);
4888
try {
4989
const client = await registerApiKeyClient(
5090
trimmed,
5191
lockedSpaceId.trim() ? lockedSpaceId.trim() : null
5292
);
93+
94+
let machineId: string | null = null;
95+
if (isCreatingMachine) {
96+
const created = await createMachine(
97+
toMachineProfilePayload({
98+
name: machineName,
99+
icon: machineIcon,
100+
hostname: machineHostname,
101+
})
102+
);
103+
machineId = created.id;
104+
} else if (selectedMachineId) {
105+
machineId = selectedMachineId;
106+
}
107+
if (machineId) {
108+
await setClientMachineId(client.clientId, machineId);
109+
}
110+
53111
setResult(client);
54112
} catch (e) {
55113
setError(e instanceof Error ? e.message : String(e));
@@ -169,10 +227,7 @@ export function RegisterApiKeyClientModal({
169227
</div>
170228

171229
<div>
172-
<label
173-
htmlFor="api-key-locked-space"
174-
className="mb-1.5 block text-sm font-medium"
175-
>
230+
<label htmlFor="api-key-locked-space" className="mb-1.5 block text-sm font-medium">
176231
Lock to a Space
177232
</label>
178233
<select
@@ -196,6 +251,61 @@ export function RegisterApiKeyClientModal({
196251
</p>
197252
</div>
198253

254+
<div>
255+
<label htmlFor="api-key-machine" className="mb-1.5 block text-sm font-medium">
256+
Machine
257+
</label>
258+
<select
259+
id="api-key-machine"
260+
data-testid="register-api-key-machine"
261+
value={selectedMachineId}
262+
onChange={(e) => setSelectedMachineId(e.target.value)}
263+
className="w-full rounded-xl border border-[rgb(var(--border))] bg-[rgb(var(--surface))] px-3.5 py-2.5 text-sm transition-all focus:border-[rgb(var(--accent))] focus:outline-none focus:ring-2 focus:ring-[rgb(var(--accent))]/40"
264+
>
265+
<option value="">No machine — assign later if needed</option>
266+
{machines.map((machine) => (
267+
<option key={machine.id} value={machine.id}>
268+
{machine.icon ? `${machine.icon} ` : ''}
269+
{machine.name}
270+
</option>
271+
))}
272+
<option value={NEW_MACHINE_OPTION}>+ New machine…</option>
273+
</select>
274+
{isCreatingMachine && (
275+
<div className="mt-3 space-y-3 rounded-xl border border-[rgb(var(--border))] p-4">
276+
<div className="flex items-center gap-2">
277+
<EmojiPickerButton value={machineIcon} onChange={setMachineIcon} />
278+
<input
279+
type="text"
280+
value={machineName}
281+
onChange={(e) => setMachineName(e.target.value)}
282+
placeholder="e.g. Cursor Web"
283+
className="h-10 min-w-0 flex-1 rounded-lg border border-[rgb(var(--border))] bg-[rgb(var(--background))] px-3 text-sm"
284+
/>
285+
</div>
286+
<input
287+
type="text"
288+
value={machineHostname}
289+
onChange={(e) => setMachineHostname(e.target.value)}
290+
onFocus={() => {
291+
if (!machineHostname) {
292+
void getHostname()
293+
.then(setMachineHostname)
294+
.catch(() => undefined);
295+
}
296+
}}
297+
placeholder="Hostname"
298+
className="w-full rounded-lg border border-[rgb(var(--border))] bg-[rgb(var(--background))] px-3 py-2 font-mono text-sm"
299+
/>
300+
</div>
301+
)}
302+
<p className="mt-1.5 text-xs text-[rgb(var(--muted))]">
303+
Optional. Tags the client so tunneled routing (the{' '}
304+
<code>X-Mcpmux-Machine-Id</code> header) and the Connections list can identify
305+
which device it's connecting from.
306+
</p>
307+
</div>
308+
199309
<div className="flex items-start gap-3 rounded-xl border border-[rgb(var(--border-subtle))] bg-[rgb(var(--surface))] p-3.5">
200310
<ShieldCheck className="mt-0.5 h-5 w-5 flex-shrink-0 text-[rgb(var(--accent))]" />
201311
<p className="text-xs text-[rgb(var(--muted))]">

apps/desktop/src/features/gateway/useGatewayControl.tsx

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,7 @@ export function useGatewayControl() {
9090
const start = async (opts?: { port?: number }): Promise<GatewayStartOutcome> => {
9191
try {
9292
return await runStart(
93-
(allowFallback) =>
94-
startGateway({ port: opts?.port, allowDynamicFallback: allowFallback }),
93+
(allowFallback) => startGateway({ port: opts?.port, allowDynamicFallback: allowFallback }),
9594
opts?.port
9695
);
9796
} catch (err) {
@@ -104,12 +103,18 @@ export function useGatewayControl() {
104103
};
105104

106105
const restart = async (opts?: { port?: number }): Promise<GatewayStartOutcome> => {
106+
// Unlike start(), restart never pre-probes the port: the gateway we're
107+
// about to replace is still holding it at probe time, so
108+
// probeGatewayStart() would see our own listener and spuriously report
109+
// the port as taken. Restart owns the port by definition — go straight
110+
// to restartGateway() and only surface the confirm dialog on a genuine
111+
// bind failure (handleBindFailure), same as a real conflict from start().
112+
console.log('[Gateway] restart → skipping pre-probe, going straight to restartGateway');
107113
try {
108-
return await runStart(
109-
(allowFallback) =>
110-
restartGateway({ port: opts?.port, allowDynamicFallback: allowFallback }),
111-
opts?.port
112-
);
114+
const url = await restartGateway({ port: opts?.port, allowDynamicFallback: false });
115+
const port = parsePortFromUrl(url) ?? opts?.port ?? 0;
116+
console.log('[Gateway] restart ok →', url);
117+
return { status: 'started', url, port, fellBackToDynamic: false };
113118
} catch (err) {
114119
return await handleBindFailure(err, opts?.port, (allowFallback) =>
115120
restartGateway({ port: opts?.port, allowDynamicFallback: allowFallback })

crates/mcpmux-core/src/domain/workspace_binding.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -925,7 +925,10 @@ mod tests {
925925
#[test]
926926
fn normalize_expands_home_tilde() {
927927
let home = dirs::home_dir().expect("test environment has a home dir");
928-
let home_str = home.to_string_lossy().trim_end_matches(['/', '\\']).to_string();
928+
let home_str = home
929+
.to_string_lossy()
930+
.trim_end_matches(['/', '\\'])
931+
.to_string();
929932

930933
assert_eq!(
931934
normalize_workspace_root("~/Desktop/proj"),

crates/mcpmux-storage/src/database.rs

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,11 @@ const MIGRATIONS: &[Migration] = &[
228228
name: "inbound_client_icon",
229229
sql: include_str!("migrations/039_inbound_client_icon.sql"),
230230
},
231+
Migration {
232+
version: 40,
233+
name: "public_url_rename",
234+
sql: include_str!("migrations/040_public_url_rename.sql"),
235+
},
231236
];
232237

233238
/// SQLite database wrapper.
@@ -768,7 +773,7 @@ mod tests {
768773
|row| row.get(0),
769774
)
770775
.unwrap();
771-
assert_eq!(version, 38);
776+
assert_eq!(version, 40);
772777

773778
let v16_name: String = db
774779
.conn
@@ -863,4 +868,113 @@ mod tests {
863868

864869
assert_eq!(name, "Test");
865870
}
871+
872+
/// Migration 40 must carry an old `gateway.public_url` row over to
873+
/// `gateway.public_base_url` (the key current code actually reads) and
874+
/// remove the superseded key, without clobbering an already-current row.
875+
#[test]
876+
fn migration_40_renames_public_url_setting_key() {
877+
use rusqlite::{params, Connection};
878+
879+
let conn = Connection::open_in_memory().unwrap();
880+
conn.pragma_update(None, "foreign_keys", "ON").unwrap();
881+
let db = Database { conn };
882+
db.ensure_migrations_table().unwrap();
883+
884+
// Apply migrations up to v39, recording each as applied.
885+
const PRE_RENAME: i64 = 39;
886+
for m in MIGRATIONS.iter().filter(|m| m.version <= PRE_RENAME) {
887+
db.conn.execute_batch(m.sql).unwrap();
888+
db.conn
889+
.execute(
890+
"INSERT OR REPLACE INTO schema_migrations (version, name, applied_at) \
891+
VALUES (?1, ?2, datetime('now'))",
892+
params![m.version, m.name],
893+
)
894+
.unwrap();
895+
}
896+
897+
// Seed the OLD key, simulating an install that predates the rename.
898+
db.conn
899+
.execute(
900+
"INSERT INTO app_settings (key, value, updated_at) \
901+
VALUES ('gateway.public_url', 'https://mcp.example.com', datetime('now'))",
902+
[],
903+
)
904+
.unwrap();
905+
906+
db.run_migrations().unwrap();
907+
908+
let new_value: String = db
909+
.conn
910+
.query_row(
911+
"SELECT value FROM app_settings WHERE key = 'gateway.public_base_url'",
912+
[],
913+
|row| row.get(0),
914+
)
915+
.unwrap();
916+
assert_eq!(new_value, "https://mcp.example.com");
917+
918+
let old_key_count: i64 = db
919+
.conn
920+
.query_row(
921+
"SELECT COUNT(*) FROM app_settings WHERE key = 'gateway.public_url'",
922+
[],
923+
|row| row.get(0),
924+
)
925+
.unwrap();
926+
assert_eq!(old_key_count, 0, "old key must be removed");
927+
}
928+
929+
/// If the new key is already present (e.g. the user re-saved the setting
930+
/// as a workaround), migration 40 must not overwrite it with the old
931+
/// value — `INSERT OR IGNORE` should be a no-op for that row.
932+
#[test]
933+
fn migration_40_does_not_overwrite_existing_new_key() {
934+
use rusqlite::{params, Connection};
935+
936+
let conn = Connection::open_in_memory().unwrap();
937+
conn.pragma_update(None, "foreign_keys", "ON").unwrap();
938+
let db = Database { conn };
939+
db.ensure_migrations_table().unwrap();
940+
941+
const PRE_RENAME: i64 = 39;
942+
for m in MIGRATIONS.iter().filter(|m| m.version <= PRE_RENAME) {
943+
db.conn.execute_batch(m.sql).unwrap();
944+
db.conn
945+
.execute(
946+
"INSERT OR REPLACE INTO schema_migrations (version, name, applied_at) \
947+
VALUES (?1, ?2, datetime('now'))",
948+
params![m.version, m.name],
949+
)
950+
.unwrap();
951+
}
952+
953+
db.conn
954+
.execute(
955+
"INSERT INTO app_settings (key, value, updated_at) \
956+
VALUES ('gateway.public_url', 'https://stale.example.com', datetime('now'))",
957+
[],
958+
)
959+
.unwrap();
960+
db.conn
961+
.execute(
962+
"INSERT INTO app_settings (key, value, updated_at) \
963+
VALUES ('gateway.public_base_url', 'https://current.example.com', datetime('now'))",
964+
[],
965+
)
966+
.unwrap();
967+
968+
db.run_migrations().unwrap();
969+
970+
let new_value: String = db
971+
.conn
972+
.query_row(
973+
"SELECT value FROM app_settings WHERE key = 'gateway.public_base_url'",
974+
[],
975+
|row| row.get(0),
976+
)
977+
.unwrap();
978+
assert_eq!(new_value, "https://current.example.com");
979+
}
866980
}
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
-- Migration 040: gateway.public_url -> gateway.public_base_url key rename.
2+
--
3+
-- Two branches independently introduced this setting under different key
4+
-- names on 2026-05-27; current code only reads gateway.public_base_url, so
5+
-- installs that still carry the old key silently lose their public URL
6+
-- (allowed_hosts drops the tunnel hostname -> remote requests 403).
7+
8+
INSERT OR IGNORE INTO app_settings (key, value, updated_at)
9+
SELECT 'gateway.public_base_url', value, datetime('now')
10+
FROM app_settings
11+
WHERE key = 'gateway.public_url';
12+
13+
DELETE FROM app_settings WHERE key = 'gateway.public_url';

0 commit comments

Comments
 (0)