mirror of
https://github.com/garrytan/gstack.git
synced 2026-05-18 18:32:28 +08:00
fix: verify tunnel is alive before returning URL to pair-agent
Root cause: when ngrok dies externally (pkill, crash, timeout), the server still reports tunnelActive=true with a dead URL. pair-agent prints an instruction block pointing at a dead tunnel. The remote agent gets "endpoint offline" and the user has to manually restart everything. Three-layer fix: - Server /pair endpoint: probes tunnel URL before returning it. If dead, resets tunnelActive/tunnelUrl and returns null (triggers CLI restart). - Server /tunnel/start: probes cached tunnel before returning already_active. If dead, falls through to restart ngrok automatically. - CLI pair-agent: double-checks tunnel URL from server before printing instruction block. Falls through to auto-start on failure. 4 regression tests verify all three probe points + CLI verification. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -622,6 +622,25 @@ async function handlePairAgent(state: ServerState, args: string[]): Promise<void
|
|||||||
|
|
||||||
// Determine the URL to use
|
// Determine the URL to use
|
||||||
let serverUrl: string;
|
let serverUrl: string;
|
||||||
|
if (pairData.tunnel_url) {
|
||||||
|
// Server already verified the tunnel is alive, but double-check from CLI side
|
||||||
|
// in case of race condition between server probe and our request
|
||||||
|
try {
|
||||||
|
const cliProbe = await fetch(`${pairData.tunnel_url}/health`, {
|
||||||
|
headers: { 'ngrok-skip-browser-warning': 'true' },
|
||||||
|
signal: AbortSignal.timeout(5000),
|
||||||
|
});
|
||||||
|
if (cliProbe.ok) {
|
||||||
|
serverUrl = pairData.tunnel_url;
|
||||||
|
} else {
|
||||||
|
console.warn(`[browse] Tunnel returned HTTP ${cliProbe.status}, attempting restart...`);
|
||||||
|
pairData.tunnel_url = null; // fall through to restart logic
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
console.warn('[browse] Tunnel unreachable from CLI, attempting restart...');
|
||||||
|
pairData.tunnel_url = null; // fall through to restart logic
|
||||||
|
}
|
||||||
|
}
|
||||||
if (pairData.tunnel_url) {
|
if (pairData.tunnel_url) {
|
||||||
serverUrl = pairData.tunnel_url;
|
serverUrl = pairData.tunnel_url;
|
||||||
} else if (!localHost) {
|
} else if (!localHost) {
|
||||||
|
|||||||
@@ -1445,11 +1445,34 @@ async function start() {
|
|||||||
domains: pairBody.domains,
|
domains: pairBody.domains,
|
||||||
rateLimit: pairBody.rateLimit,
|
rateLimit: pairBody.rateLimit,
|
||||||
});
|
});
|
||||||
|
// Verify tunnel is actually alive before reporting it (ngrok may have died externally)
|
||||||
|
let verifiedTunnelUrl: string | null = null;
|
||||||
|
if (tunnelActive && tunnelUrl) {
|
||||||
|
try {
|
||||||
|
const probe = await fetch(`${tunnelUrl}/health`, {
|
||||||
|
headers: { 'ngrok-skip-browser-warning': 'true' },
|
||||||
|
signal: AbortSignal.timeout(5000),
|
||||||
|
});
|
||||||
|
if (probe.ok) {
|
||||||
|
verifiedTunnelUrl = tunnelUrl;
|
||||||
|
} else {
|
||||||
|
console.warn(`[browse] Tunnel probe failed (HTTP ${probe.status}), marking tunnel as dead`);
|
||||||
|
tunnelActive = false;
|
||||||
|
tunnelUrl = null;
|
||||||
|
tunnelListener = null;
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
console.warn('[browse] Tunnel probe timed out or unreachable, marking tunnel as dead');
|
||||||
|
tunnelActive = false;
|
||||||
|
tunnelUrl = null;
|
||||||
|
tunnelListener = null;
|
||||||
|
}
|
||||||
|
}
|
||||||
return new Response(JSON.stringify({
|
return new Response(JSON.stringify({
|
||||||
setup_key: setupKey.token,
|
setup_key: setupKey.token,
|
||||||
expires_at: setupKey.expiresAt,
|
expires_at: setupKey.expiresAt,
|
||||||
scopes: setupKey.scopes,
|
scopes: setupKey.scopes,
|
||||||
tunnel_url: tunnelActive ? tunnelUrl : null,
|
tunnel_url: verifiedTunnelUrl,
|
||||||
server_url: `http://127.0.0.1:${server?.port || 0}`,
|
server_url: `http://127.0.0.1:${server?.port || 0}`,
|
||||||
}), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
}), { status: 200, headers: { 'Content-Type': 'application/json' } });
|
||||||
} catch {
|
} catch {
|
||||||
@@ -1466,10 +1489,24 @@ async function start() {
|
|||||||
status: 403, headers: { 'Content-Type': 'application/json' },
|
status: 403, headers: { 'Content-Type': 'application/json' },
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
if (tunnelActive) {
|
if (tunnelActive && tunnelUrl) {
|
||||||
return new Response(JSON.stringify({ url: tunnelUrl, already_active: true }), {
|
// Verify tunnel is still alive before returning cached URL
|
||||||
status: 200, headers: { 'Content-Type': 'application/json' },
|
try {
|
||||||
});
|
const probe = await fetch(`${tunnelUrl}/health`, {
|
||||||
|
headers: { 'ngrok-skip-browser-warning': 'true' },
|
||||||
|
signal: AbortSignal.timeout(5000),
|
||||||
|
});
|
||||||
|
if (probe.ok) {
|
||||||
|
return new Response(JSON.stringify({ url: tunnelUrl, already_active: true }), {
|
||||||
|
status: 200, headers: { 'Content-Type': 'application/json' },
|
||||||
|
});
|
||||||
|
}
|
||||||
|
} catch {}
|
||||||
|
// Tunnel is dead, reset and fall through to restart
|
||||||
|
console.warn('[browse] Cached tunnel is dead, restarting...');
|
||||||
|
tunnelActive = false;
|
||||||
|
tunnelUrl = null;
|
||||||
|
tunnelListener = null;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
// Read ngrok authtoken: env var > ~/.gstack/ngrok.env > ngrok native config
|
// Read ngrok authtoken: env var > ~/.gstack/ngrok.env > ngrok native config
|
||||||
|
|||||||
@@ -10,6 +10,7 @@ import * as fs from 'fs';
|
|||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
|
||||||
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
|
const SERVER_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/server.ts'), 'utf-8');
|
||||||
|
const CLI_SRC = fs.readFileSync(path.join(import.meta.dir, '../src/cli.ts'), 'utf-8');
|
||||||
|
|
||||||
// Helper: extract a block of source between two markers
|
// Helper: extract a block of source between two markers
|
||||||
function sliceBetween(source: string, startMarker: string, endMarker: string): string {
|
function sliceBetween(source: string, startMarker: string, endMarker: string): string {
|
||||||
@@ -188,4 +189,47 @@ describe('Server auth security', () => {
|
|||||||
const commandStartBlock = sliceBetween(SERVER_SRC, "Activity: emit command_start", "try {");
|
const commandStartBlock = sliceBetween(SERVER_SRC, "Activity: emit command_start", "try {");
|
||||||
expect(commandStartBlock).toContain('clientId: tokenInfo?.clientId');
|
expect(commandStartBlock).toContain('clientId: tokenInfo?.clientId');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// ─── Tunnel liveness verification ─────────────────────────────
|
||||||
|
|
||||||
|
// Test 11a: /pair endpoint probes tunnel before returning tunnel_url
|
||||||
|
test('/pair verifies tunnel is alive before returning tunnel_url', () => {
|
||||||
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
||||||
|
// Must probe the tunnel URL
|
||||||
|
expect(pairBlock).toContain('verifiedTunnelUrl');
|
||||||
|
expect(pairBlock).toContain('Tunnel probe failed');
|
||||||
|
expect(pairBlock).toContain('marking tunnel as dead');
|
||||||
|
// Must reset tunnel state on failure
|
||||||
|
expect(pairBlock).toContain('tunnelActive = false');
|
||||||
|
expect(pairBlock).toContain('tunnelUrl = null');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Test 11b: /pair returns null tunnel_url when tunnel is dead
|
||||||
|
test('/pair returns verified tunnel URL, not raw tunnelActive flag', () => {
|
||||||
|
const pairBlock = sliceBetween(SERVER_SRC, "url.pathname === '/pair'", "url.pathname === '/tunnel/start'");
|
||||||
|
// Should use verifiedTunnelUrl (probe result), not raw tunnelUrl
|
||||||
|
expect(pairBlock).toContain('tunnel_url: verifiedTunnelUrl');
|
||||||
|
// Must NOT use raw tunnelActive check for the response
|
||||||
|
expect(pairBlock).not.toContain('tunnel_url: tunnelActive ? tunnelUrl');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Test 11c: /tunnel/start probes cached tunnel before returning already_active
|
||||||
|
test('/tunnel/start verifies cached tunnel is alive before returning already_active', () => {
|
||||||
|
const tunnelBlock = sliceBetween(SERVER_SRC, "url.pathname === '/tunnel/start'", "url.pathname === '/refs'");
|
||||||
|
// Must probe before returning cached URL
|
||||||
|
expect(tunnelBlock).toContain('Cached tunnel is dead');
|
||||||
|
expect(tunnelBlock).toContain('tunnelActive = false');
|
||||||
|
// Must fall through to restart when dead
|
||||||
|
expect(tunnelBlock).toContain('restarting');
|
||||||
|
});
|
||||||
|
|
||||||
|
// Test 11d: CLI verifies tunnel_url from server before printing instruction block
|
||||||
|
test('CLI probes tunnel_url before using it in instruction block', () => {
|
||||||
|
const pairSection = sliceBetween(CLI_SRC, 'Determine the URL to use', 'local HOST: write config');
|
||||||
|
// Must probe the tunnel URL
|
||||||
|
expect(pairSection).toContain('cliProbe');
|
||||||
|
expect(pairSection).toContain('Tunnel unreachable from CLI');
|
||||||
|
// Must fall through to restart logic on failure
|
||||||
|
expect(pairSection).toContain('attempting restart');
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user