mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-03 05:59:43 +02:00
fix(web): bound a pending #session= link and retire it on Home or a web tab (#507 review)
- A #session=<id> link whose session never appears (closed, a typo, or another user's session in multi-user mode) is dropped after URL_SESSION_WAIT_MS (30 s) with a "Session not found" toast instead of waiting forever. One stored timer per link, cleared whenever the link is followed, replaced by a newer link, or retired. - goHome() and opening a web tab now retire a waiting link, so a session that turns up later no longer takes the screen. App-made web tab opens (frame self-recovery, the fallback after the active web tab closes) pass auto: true and keep it, as selectSession() does. - zh-CN translation for the new toast. - selectSession's auto: true comment now lists the #session=<id> link. - docs: the 30 s bound, a win.location.replace() tip that avoids piling up history entries, and the fragment declared a stable SemVer surface in versioning-policy.md. - Tests: timeout drops and toasts, an early arrival is still selected, the wait does not restart, goHome and a web tab retire it, an auto web tab open keeps it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -355,7 +355,16 @@ When that window already shows the dashboard, only the fragment differs. The
|
||||
browser therefore keeps the page loaded, and the dashboard switches tabs without
|
||||
reloading it. A session the window has shown before appears at once. A session
|
||||
your page has only just created may not be listed yet, so the dashboard waits
|
||||
for its `session:created` event and selects it then.
|
||||
for its `session:created` event and selects it then. That wait lasts at most 30
|
||||
seconds: a link whose session never appears (a closed session, a typo, or in
|
||||
multi-user mode another user's session) is dropped with a "Session not found"
|
||||
notice. Clicking another tab, going Home or opening a web tab also ends the
|
||||
wait, so a session that turns up later never takes the screen from the person.
|
||||
|
||||
When your page holds the window reference (`const win = window.open(...)`),
|
||||
prefer `win.location.replace(url)` for later links: it still fires `hashchange`
|
||||
without a reload, but adds no history entry, so Back in the dashboard window
|
||||
does not turn into a silent no-op.
|
||||
|
||||
Following a link does not count as someone looking at the session, so it
|
||||
leaves the session's idle alert in place. The alert clears when the person
|
||||
|
||||
@@ -40,6 +40,10 @@ A **MAJOR** bump is required to break any of these after 1.0:
|
||||
optional fields, new error codes, new SSE events) are non-breaking; breaking
|
||||
changes ship under a new prefix (`/api/v2`). The unversioned `/api/...` alias
|
||||
is kept working for the bundled UI.
|
||||
5. **The dashboard's `#session=<id>` link.** Opening the dashboard URL with a
|
||||
`#session=<id>` fragment selects that session if this client can see it. The
|
||||
fragment name and that meaning are stable; see
|
||||
[Opening a session from your own page](extending-codeman.md#opening-a-session-from-your-own-page).
|
||||
|
||||
## What SemVer does NOT cover (internal surfaces — may change in any release)
|
||||
|
||||
|
||||
+65
-11
@@ -567,6 +567,14 @@ const DEFAULT_SHORTCUTS = [
|
||||
*/
|
||||
const SIDEBAR_RICH_CLOCK_MS = 20000;
|
||||
|
||||
/**
|
||||
* How long a `#session=<id>` link waits for the session list to name its id
|
||||
* before the dashboard drops it with a "Session not found" toast (see
|
||||
* _armUrlSessionWait). Long enough for a page that has just created the
|
||||
* session to see its session:created land here.
|
||||
*/
|
||||
const URL_SESSION_WAIT_MS = 30000;
|
||||
|
||||
class CodemanApp {
|
||||
constructor() {
|
||||
this.sessions = new Map();
|
||||
@@ -605,6 +613,7 @@ class CodemanApp {
|
||||
// A session another page asked for with a `#session=<id>` link. It waits
|
||||
// here until the session list has that id (see _selectUrlSession).
|
||||
this._urlSessionId = this.isSoloWindow ? null : this._takeUrlSession();
|
||||
this._urlSessionWaitTimer = null; // bounds that wait (_armUrlSessionWait)
|
||||
this.detachedSessions = new Set(); // dashboard-side: ids currently popped out
|
||||
this.detachedWindows = new Map(); // dashboard-side: id -> WindowProxy
|
||||
this._detachWatchTimers = new Map(); // dashboard-side: id -> setInterval handle
|
||||
@@ -1006,6 +1015,8 @@ class CodemanApp {
|
||||
window.addEventListener('hashchange', () => {
|
||||
const id = this._takeUrlSession();
|
||||
if (!id) return;
|
||||
// A new link replaces one still waiting, and gets a wait of its own.
|
||||
this._retireUrlSession();
|
||||
this._urlSessionId = id;
|
||||
this._selectUrlSession();
|
||||
});
|
||||
@@ -1394,20 +1405,56 @@ class CodemanApp {
|
||||
|
||||
/** Show the session a `#session=<id>` link asked for, once the session list
|
||||
* has it. A page that has just created a session can link to it before
|
||||
* session:created arrives here, so an unknown id stays pending and
|
||||
* _onSessionCreated tries again.
|
||||
* session:created arrives here, so an unknown id stays pending (for at most
|
||||
* URL_SESSION_WAIT_MS) and _onSessionCreated tries again.
|
||||
*
|
||||
* ⚠️ The selection is `auto`. The page that set the fragment may be a
|
||||
* script, and this window may not even be in front, so following a link is
|
||||
* not a human looking at the session and must not spend its idle alert. */
|
||||
_selectUrlSession() {
|
||||
const id = this._urlSessionId;
|
||||
if (!id || !this.sessions.has(id)) return false;
|
||||
this._urlSessionId = null;
|
||||
if (!id) return false;
|
||||
if (!this.sessions.has(id)) {
|
||||
this._armUrlSessionWait(id);
|
||||
return false;
|
||||
}
|
||||
this._retireUrlSession();
|
||||
this.selectSession(id, { auto: true });
|
||||
return true;
|
||||
}
|
||||
|
||||
/** Bound the wait for a link whose id the session list does not have. A
|
||||
* stale link (that session is closed), a typo, or in multi-user mode another
|
||||
* user's session (never in this client's list) would otherwise wait with
|
||||
* nothing on screen, and take the tab whenever a matching session turned up.
|
||||
* One timer per link: handleInit running again (an SSE reconnect) does not
|
||||
* restart it, and every way a link ends goes through _retireUrlSession. */
|
||||
_armUrlSessionWait(id) {
|
||||
if (this._urlSessionWaitTimer) return;
|
||||
this._urlSessionWaitTimer = setTimeout(() => {
|
||||
this._urlSessionWaitTimer = null;
|
||||
if (this._urlSessionId !== id) return;
|
||||
// Listed by a path other than session:created (a session:updated): select it.
|
||||
if (this.sessions.has(id)) {
|
||||
this._selectUrlSession();
|
||||
return;
|
||||
}
|
||||
this._retireUrlSession();
|
||||
this.showToast?.('Session not found', 'warning');
|
||||
}, URL_SESSION_WAIT_MS);
|
||||
}
|
||||
|
||||
/** Drop a waiting `#session=<id>` link and its timer: the link was followed,
|
||||
* replaced by a newer one, timed out, or the user chose something else
|
||||
* (another tab, Home, a web tab). */
|
||||
_retireUrlSession() {
|
||||
this._urlSessionId = null;
|
||||
if (this._urlSessionWaitTimer) {
|
||||
clearTimeout(this._urlSessionWaitTimer);
|
||||
this._urlSessionWaitTimer = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Pop a session out into its own browser window. SINGLE, idempotent entry
|
||||
* point: the tab's pop-out icon calls this, and a future gesture layer
|
||||
@@ -4421,6 +4468,9 @@ class CodemanApp {
|
||||
this._selectUrlSession();
|
||||
return;
|
||||
}
|
||||
// Not listed yet: its wait starts now that the list has loaded, and the
|
||||
// last active tab is restored meanwhile.
|
||||
if (this._urlSessionId) this._armUrlSessionWait(this._urlSessionId);
|
||||
|
||||
const previousActiveId = this.activeSessionId;
|
||||
if (this.sessionOrder.length === 0) {
|
||||
@@ -6619,7 +6669,7 @@ class CodemanApp {
|
||||
// waiting for its session, which would otherwise take the tab from you
|
||||
// whenever that session turned up (see _selectUrlSession).
|
||||
if (options?.auto !== true && this._urlSessionId && this._urlSessionId !== sessionId) {
|
||||
this._urlSessionId = null;
|
||||
this._retireUrlSession();
|
||||
}
|
||||
// If this session is popped out into its own window, raise that window
|
||||
// instead of showing it inline (focus-on-click for detached tabs). If we
|
||||
@@ -6630,12 +6680,13 @@ class CodemanApp {
|
||||
}
|
||||
const forceReload = options?.forceReload === true;
|
||||
// ⚠️ `auto: true` marks a selection the APP made rather than the human:
|
||||
// the boot restore, a solo window opening its target, the fallback after
|
||||
// the active session is deleted. Those must NOT spend a pending idle alert
|
||||
// (the yellow survives until a real tap), because "the app put this on
|
||||
// screen" is not "I checked it". The DEFAULT is user-initiated, so a call
|
||||
// site nobody tagged fails toward acknowledging rather than toward an
|
||||
// alert that can never be cleared.
|
||||
// the boot restore, a solo window opening its target, a `#session=<id>`
|
||||
// link from another page, the fallback after the active session is
|
||||
// deleted. Those must NOT spend a pending idle alert (the yellow survives
|
||||
// until a real tap), because "the app put this on screen" is not "I
|
||||
// checked it". The DEFAULT is user-initiated, so a call site nobody tagged
|
||||
// fails toward acknowledging rather than toward an alert that can never be
|
||||
// cleared.
|
||||
const userInitiated = options?.auto !== true;
|
||||
if (this.activeSessionId === sessionId && !forceReload) {
|
||||
// Tapping the tab you are already on is still "I checked it". The alert
|
||||
@@ -7598,6 +7649,9 @@ class CodemanApp {
|
||||
// ═══════════════════════════════════════════════════════════════
|
||||
|
||||
goHome() {
|
||||
// Going Home is choosing something else, so a `#session=<id>` link still
|
||||
// waiting for its session must not take the screen later.
|
||||
this._retireUrlSession();
|
||||
// Deselect active session and show welcome screen
|
||||
this.activeSessionId = null;
|
||||
try { localStorage.removeItem('codeman-active-session'); } catch {}
|
||||
|
||||
@@ -571,6 +571,8 @@
|
||||
'Task Complete': '任务完成',
|
||||
'Copied to clipboard': '已复制到剪贴板',
|
||||
'Nothing to copy': '没有可复制的内容',
|
||||
// A `#session=<id>` link whose session never appeared (app.js _armUrlSessionWait).
|
||||
'Session not found': '未找到会话',
|
||||
// Terminal touch-selection bar (long-press to select). The bar is a sibling of
|
||||
// `.xterm`, not a descendant, so SKIP_SELECTOR does not cover it and these apply.
|
||||
Copy: '复制',
|
||||
|
||||
@@ -274,7 +274,10 @@ Object.assign(CodemanApp.prototype, {
|
||||
// one `/`, and refuse whatever still opens a second one. The proxied form
|
||||
// is refused server-side as well (resolveUpstreamUrl).
|
||||
const path = data.path.replace(/[\t\n\r]/g, '').replace(/^[/\\]+/, '/');
|
||||
void this.openWebview(id, { path: path.startsWith('/') && !/^\/[/\\]/.test(path) ? path : '/' });
|
||||
void this.openWebview(id, {
|
||||
path: path.startsWith('/') && !/^\/[/\\]/.test(path) ? path : '/',
|
||||
auto: true,
|
||||
});
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -356,15 +359,23 @@ Object.assign(CodemanApp.prototype, {
|
||||
*/
|
||||
/**
|
||||
* @param {string} id
|
||||
* @param {{path?: string}} [options] `path` (pathname+search+hash) opens a
|
||||
* @param {{path?: string, auto?: boolean}} [options] `path` (pathname+search+hash) opens a
|
||||
* deep link inside the dashboard: appended to the proxy prefix, or resolved
|
||||
* against the real URL in direct mode. A mounted frame is navigated there
|
||||
* rather than left on whatever page it was showing.
|
||||
* rather than left on whatever page it was showing. `auto: true` marks an
|
||||
* open the APP made (a frame recovering itself, the fallback after the
|
||||
* active web tab closes), as on selectSession().
|
||||
*/
|
||||
async openWebview(id, options = {}) {
|
||||
const webview = this.webviews.get(id);
|
||||
if (!webview) return;
|
||||
|
||||
// Opening a web tab yourself is choosing something else, so a
|
||||
// `#session=<id>` link still waiting for its session must not take the
|
||||
// screen from this tab later. Retired before the await below, which a
|
||||
// session:created could otherwise land inside.
|
||||
if (options.auto !== true) this._retireUrlSession?.();
|
||||
|
||||
if (!this.webviewOrder.includes(id)) {
|
||||
this.webviewOrder.push(id);
|
||||
this._persistWebviewOrder();
|
||||
@@ -513,7 +524,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.activeWebviewId = null;
|
||||
const next = this.webviewOrder[0];
|
||||
if (next) {
|
||||
this.openWebview(next);
|
||||
this.openWebview(next, { auto: true });
|
||||
} else {
|
||||
this._hideWebviewLayer();
|
||||
// Fall back to whatever session was last shown, or the welcome screen.
|
||||
|
||||
@@ -7,7 +7,7 @@ import { readFileSync } from 'node:fs';
|
||||
import { resolve } from 'node:path';
|
||||
import vm from 'node:vm';
|
||||
import { performance } from 'node:perf_hooks';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
function loadHelper() {
|
||||
const context = vm.createContext({ window: {}, globalThis: {}, URLSearchParams });
|
||||
@@ -50,10 +50,12 @@ describe('CodemanUrlSession.sessionIdFromFragment', () => {
|
||||
|
||||
// The dashboard side: reading the link, holding an id it does not list yet,
|
||||
// and handing the selection over. Loaded like session-select-ack-gate.test.ts,
|
||||
// on a bare instance whose DOM-touching methods are stubbed.
|
||||
// on a bare instance whose DOM-touching methods are stubbed. webview-tabs.js
|
||||
// rides along because opening a web tab is one of the ways a waiting link ends.
|
||||
function loadApp() {
|
||||
const constants = readFileSync(resolve(import.meta.dirname, '../src/web/public/constants.js'), 'utf8');
|
||||
const app = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const webviewTabs = readFileSync(resolve(import.meta.dirname, '../src/web/public/webview-tabs.js'), 'utf8');
|
||||
const location = { hash: '', pathname: '/', search: '' };
|
||||
const history = {
|
||||
state: null,
|
||||
@@ -80,8 +82,12 @@ function loadApp() {
|
||||
window: { addEventListener: vi.fn(), removeEventListener: vi.fn() },
|
||||
MobileDetection: { isTouchDevice: () => false },
|
||||
});
|
||||
vm.runInContext(`${constants}\n${app}\nglobalThis.__CodemanApp = CodemanApp;`, context);
|
||||
vm.runInContext(
|
||||
`${constants}\n${app}\n${webviewTabs}\nglobalThis.__CodemanApp = CodemanApp;\nglobalThis.__waitMs = URL_SESSION_WAIT_MS;`,
|
||||
context
|
||||
);
|
||||
const CodemanApp = (context as { __CodemanApp: { prototype: object } }).__CodemanApp;
|
||||
const waitMs = (context as { __waitMs: number }).__waitMs;
|
||||
const make = (ids: string[]) => {
|
||||
const inst = Object.create(CodemanApp.prototype) as Record<string, any>;
|
||||
inst.sessions = new Map(ids.map((id) => [id, { id, name: id }]));
|
||||
@@ -90,7 +96,9 @@ function loadApp() {
|
||||
inst.detachedWindows = new Map();
|
||||
inst.isSoloWindow = false;
|
||||
inst._urlSessionId = null;
|
||||
inst._urlSessionWaitTimer = null;
|
||||
inst.selectSession = vi.fn();
|
||||
inst.showToast = vi.fn();
|
||||
for (const stub of [
|
||||
'saveSessionOrder',
|
||||
'markSessionTabEntering',
|
||||
@@ -103,7 +111,7 @@ function loadApp() {
|
||||
}
|
||||
return inst;
|
||||
};
|
||||
return { make, location, history, CodemanApp };
|
||||
return { make, location, history, CodemanApp, waitMs };
|
||||
}
|
||||
|
||||
describe('dashboard handling of a #session=<id> link', () => {
|
||||
@@ -163,6 +171,15 @@ describe('dashboard handling of a #session=<id> link', () => {
|
||||
expect(app._urlSessionId).toBe('later');
|
||||
});
|
||||
|
||||
it('starts the wait for an unlisted link once the page has loaded its session list', () => {
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const link = source.indexOf('if (this._urlSessionId && this.sessions.has(this._urlSessionId))');
|
||||
const wait = source.indexOf('if (this._urlSessionId) this._armUrlSessionWait(this._urlSessionId);');
|
||||
const restore = source.indexOf("restoreId = localStorage.getItem('codeman-active-session')");
|
||||
expect(wait).toBeGreaterThan(link);
|
||||
expect(wait).toBeLessThan(restore);
|
||||
});
|
||||
|
||||
it('puts the link ahead of restoring the last active tab when the page loads', () => {
|
||||
const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/app.js'), 'utf8');
|
||||
const link = source.indexOf('if (this._urlSessionId && this.sessions.has(this._urlSessionId))');
|
||||
@@ -177,3 +194,119 @@ describe('dashboard handling of a #session=<id> link', () => {
|
||||
expect(source).toMatch(/if \(!this\.isSoloWindow\) \{\s*window\.addEventListener\('hashchange'/);
|
||||
});
|
||||
});
|
||||
|
||||
// A link whose session never turns up: a stale link (the session is closed), a
|
||||
// typo, or in multi-user mode another user's session, which is never in this
|
||||
// client's list. It must not wait forever with nothing on screen, and choosing
|
||||
// something else must end it, or a session turning up later takes the screen.
|
||||
describe('a #session=<id> link that is still waiting', () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it('waits 30 seconds', () => {
|
||||
expect(loadApp().waitMs).toBe(30_000);
|
||||
});
|
||||
|
||||
it('is dropped with a toast when its session has not appeared in time', () => {
|
||||
vi.useFakeTimers();
|
||||
const { make, waitMs } = loadApp();
|
||||
const app = make([]);
|
||||
app._urlSessionId = 'gone';
|
||||
expect(app._selectUrlSession()).toBe(false);
|
||||
vi.advanceTimersByTime(waitMs - 1);
|
||||
expect(app._urlSessionId).toBe('gone');
|
||||
expect(app.showToast).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(1);
|
||||
expect(app._urlSessionId).toBeNull();
|
||||
expect(app._urlSessionWaitTimer).toBeNull();
|
||||
expect(app.showToast).toHaveBeenCalledWith('Session not found', 'warning');
|
||||
// Retired for good: the session turning up afterwards does not take the tab.
|
||||
app._onSessionCreated({ id: 'gone', name: 'gone' });
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('still selects a session that arrives before the wait runs out, and stops the timer', () => {
|
||||
vi.useFakeTimers();
|
||||
const { make, waitMs } = loadApp();
|
||||
const app = make([]);
|
||||
app._urlSessionId = 'new';
|
||||
app._selectUrlSession();
|
||||
vi.advanceTimersByTime(waitMs - 1);
|
||||
app._onSessionCreated({ id: 'new', name: 'new' });
|
||||
expect(app.selectSession).toHaveBeenCalledWith('new', { auto: true });
|
||||
expect(app._urlSessionWaitTimer).toBeNull();
|
||||
vi.advanceTimersByTime(waitMs);
|
||||
expect(app.showToast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not restart its wait when handleInit asks again', () => {
|
||||
vi.useFakeTimers();
|
||||
const { make, waitMs } = loadApp();
|
||||
const app = make([]);
|
||||
app._urlSessionId = 'gone';
|
||||
app._armUrlSessionWait('gone');
|
||||
vi.advanceTimersByTime(waitMs - 1000);
|
||||
app._armUrlSessionWait('gone');
|
||||
vi.advanceTimersByTime(1000);
|
||||
expect(app._urlSessionId).toBeNull();
|
||||
expect(app.showToast).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('is retired by going Home', () => {
|
||||
vi.useFakeTimers();
|
||||
const { make, waitMs } = loadApp();
|
||||
const app = make([]);
|
||||
app.terminal = { clear: vi.fn() };
|
||||
app.showWelcome = vi.fn();
|
||||
app.renderRalphStatePanel = vi.fn();
|
||||
app._urlSessionId = 'later';
|
||||
app._selectUrlSession();
|
||||
app.goHome();
|
||||
expect(app._urlSessionId).toBeNull();
|
||||
expect(app._urlSessionWaitTimer).toBeNull();
|
||||
app._onSessionCreated({ id: 'later', name: 'later' });
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(waitMs);
|
||||
expect(app.showToast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
function withWebTab(app: Record<string, any>) {
|
||||
const webview = { id: 'dash', name: 'Dash', url: 'http://127.0.0.1:8080/' };
|
||||
app.webviews = new Map([['dash', webview]]);
|
||||
app.webviewOrder = ['dash'];
|
||||
app._persistWebviewOrder = vi.fn();
|
||||
app._apiJson = vi.fn(async () => ({ webview, embedUrl: '/webview/cap/' }));
|
||||
app._mountWebviewFrame = vi.fn();
|
||||
app.hideWelcome = vi.fn();
|
||||
app._updateActiveWebviewTab = vi.fn();
|
||||
app.closeSessionSidebarOnHandheld = vi.fn();
|
||||
return app;
|
||||
}
|
||||
|
||||
it('is retired by opening a web tab', async () => {
|
||||
vi.useFakeTimers();
|
||||
const { make, waitMs } = loadApp();
|
||||
const app = withWebTab(make([]));
|
||||
app._urlSessionId = 'later';
|
||||
app._selectUrlSession();
|
||||
const opening = app.openWebview('dash');
|
||||
// Before the open's await: a session:created landing inside it finds no link.
|
||||
expect(app._urlSessionId).toBeNull();
|
||||
expect(app._urlSessionWaitTimer).toBeNull();
|
||||
await opening;
|
||||
expect(app.activeWebviewId).toBe('dash');
|
||||
app._onSessionCreated({ id: 'later', name: 'later' });
|
||||
expect(app.selectSession).not.toHaveBeenCalled();
|
||||
vi.advanceTimersByTime(waitMs);
|
||||
expect(app.showToast).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('survives a web tab the app opens itself', async () => {
|
||||
const { make } = loadApp();
|
||||
const app = withWebTab(make([]));
|
||||
app._urlSessionId = 'later';
|
||||
await app.openWebview('dash', { auto: true });
|
||||
expect(app._urlSessionId).toBe('later');
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user