diff --git a/docs/extending-codeman.md b/docs/extending-codeman.md index 5e0bb605..9d19ff0d 100644 --- a/docs/extending-codeman.md +++ b/docs/extending-codeman.md @@ -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 diff --git a/docs/versioning-policy.md b/docs/versioning-policy.md index 44845689..4e635d07 100644 --- a/docs/versioning-policy.md +++ b/docs/versioning-policy.md @@ -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=` link.** Opening the dashboard URL with a + `#session=` 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) diff --git a/src/web/public/app.js b/src/web/public/app.js index fcfa641d..f1d9da4a 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -567,6 +567,14 @@ const DEFAULT_SHORTCUTS = [ */ const SIDEBAR_RICH_CLOCK_MS = 20000; +/** + * How long a `#session=` 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=` 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=` 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=` 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=` + // 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=` 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 {} diff --git a/src/web/public/i18n.js b/src/web/public/i18n.js index bfa8bd7a..bf6deb26 100644 --- a/src/web/public/i18n.js +++ b/src/web/public/i18n.js @@ -571,6 +571,8 @@ 'Task Complete': '任务完成', 'Copied to clipboard': '已复制到剪贴板', 'Nothing to copy': '没有可复制的内容', + // A `#session=` 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: '复制', diff --git a/src/web/public/webview-tabs.js b/src/web/public/webview-tabs.js index 98d40188..01682157 100644 --- a/src/web/public/webview-tabs.js +++ b/src/web/public/webview-tabs.js @@ -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=` 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. diff --git a/test/url-session-fragment.test.ts b/test/url-session-fragment.test.ts index c75a8fe7..40e8ec6a 100644 --- a/test/url-session-fragment.test.ts +++ b/test/url-session-fragment.test.ts @@ -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; 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= link', () => { @@ -163,6 +171,15 @@ describe('dashboard handling of a #session= 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= 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= 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) { + 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'); + }); +});