From d4202eddbc9df8c26fdc2585c65accf1986fe17b Mon Sep 17 00:00:00 2001 From: Agnik47 <140933190+Agnik47@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:29:47 +0530 Subject: [PATCH] fix(cli): name the owning Profile when a Session lookup misses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Session ID is only ever resolved inside the selected Profile, so a command that omits `--profile` reports `Session not found` for a Session that does exist. Cleanup runs hit this constantly: the agent had a valid ID from `session create`, dropped the flag on the follow-up `session close`, read the error as "already gone", and retried the same command or went inspecting unrelated default-Profile Sessions. Every Profile's Sessions share one `browser-sessions.json` keyed by `profileId`, so the owning Profile is already in hand when the scoped lookup misses — no daemon or provider round trip is needed to name it. `SessionNotFoundError` now takes an optional owner and, when it is known, reports which Profile holds the Session plus the exact retry command. Both throw sites are covered: `require` and the private `requireMutable` behind `remove`/`touch`/handoff updates. The daemon provider and the CLI local fallback resolve through this same store, so both paths gain the hint from the one change. With no owner the message and hint are unchanged. `session list` also grows a `profileId` table column. The rows were always scoped to the selected Profile, but the table did not say so, which is the other half of how agents ended up acting on Sessions that were never theirs. JSON and YAML already carried the field. --- docs/troubleshooting.mdx | 2 +- src/browser/sessions.test.ts | 38 ++++++++++++++++++++++++++++++++++++ src/browser/sessions.ts | 30 ++++++++++++++++++++++------ src/cli.test.ts | 27 +++++++++++++++++++++++++ src/cli.ts | 5 ++++- 5 files changed, 94 insertions(+), 8 deletions(-) diff --git a/docs/troubleshooting.mdx b/docs/troubleshooting.mdx index 2382c2ad..7981be14 100644 --- a/docs/troubleshooting.mdx +++ b/docs/troubleshooting.mdx @@ -48,7 +48,7 @@ Common Session codes: | `SESSION_REQUIRED` | A raw browser command needs a root Session selector. | Run `webcmd session create -f json`, then retry as `webcmd --session browser ...`. | | `INVALID_SESSION_SELECTOR` | The selector is not an opaque Webcmd Session ID. | Use an ID returned by `webcmd session create` or `webcmd session list`. | | `SESSION_SELECTOR_POSITION` | `--session` was placed after the command name. | Move it before the command: `webcmd --session browser ...`. | -| `SESSION_NOT_FOUND` | The selected Session is missing for the current Profile. | Run `webcmd session list -f json`; create a new Session if needed. | +| `SESSION_NOT_FOUND` | The selected Session is missing for the current Profile. | When the ID belongs to another Profile the hint names it — retry with `webcmd --profile session close `. Otherwise run `webcmd session list -f json`; create a new Session if needed. | | `INVALID_SESSION_LIMIT` | `session list --limit` is outside 1-100. | Retry with a limit from 1 to 100. | | `SESSION_BUSY` | Another command is writing in the same Session or site scope. | Wait for the holder to finish; if it is dead, use `webcmd session close --force` as the last resort. | | `SESSION_PAUSED_FOR_HUMAN_HANDOFF` | The Session is waiting for user action such as sign-in. | Finish the action in the browser, then run the returned verifier before retrying. | diff --git a/src/browser/sessions.test.ts b/src/browser/sessions.test.ts index 676a677d..f267ac0a 100644 --- a/src/browser/sessions.test.ts +++ b/src/browser/sessions.test.ts @@ -47,6 +47,44 @@ describe('LocalBrowserSessionStore', () => { expect(() => store.find('profile_work', 'work')).toThrowError(expect.objectContaining({ code: 'INVALID_SESSION_SELECTOR' })); }); + it('names the owning profile when the session exists under a different one', () => { + // A cleanup command that omits `--profile` looks up a real Session ID in + // the wrong Profile. A bare "not found" sent agents into retrying the same + // command; naming the owner and the retry shape ends that loop. + const store = new LocalBrowserSessionStore({ baseDir: tempDir(), idFactory: () => 'session_a' }); + const created = store.create('profile_work'); + + expect(() => store.require('profile_personal', created.id)).toThrowError(expect.objectContaining({ + code: 'SESSION_NOT_FOUND', + ownerProfileId: 'profile_work', + message: `Session not found in Profile profile_personal: ${created.id}`, + hint: `Session ${created.id} belongs to Profile profile_work. Re-run the same command with \`--profile profile_work\`, for example \`webcmd --profile profile_work session close ${created.id}\`.`, + })); + }); + + it('names the owning profile on mutating lookups too', () => { + // `remove`/`touch`/handoff updates resolve the record through a separate + // lookup, so the close path must not fall back to the anonymous message. + const store = new LocalBrowserSessionStore({ baseDir: tempDir(), idFactory: () => 'session_a' }); + const created = store.create('profile_work'); + + expect(() => store.remove('profile_personal', created.id)).toThrowError(expect.objectContaining({ + code: 'SESSION_NOT_FOUND', + ownerProfileId: 'profile_work', + })); + }); + + it('keeps the generic hint when no profile owns the session', () => { + const store = new LocalBrowserSessionStore({ baseDir: tempDir() }); + + expect(() => store.require('profile_work', 'session_missing')).toThrowError(expect.objectContaining({ + code: 'SESSION_NOT_FOUND', + ownerProfileId: undefined, + message: 'Session not found: session_missing', + hint: 'Run `webcmd --profile profile_work session list` to choose an existing Session.', + })); + }); + it('resolves one lazy adapter-default per profile without list side effects', () => { const store = new LocalBrowserSessionStore({ baseDir: tempDir(), diff --git a/src/browser/sessions.ts b/src/browser/sessions.ts index d046999b..a6d6beba 100644 --- a/src/browser/sessions.ts +++ b/src/browser/sessions.ts @@ -2,7 +2,7 @@ import fs from 'node:fs'; import os from 'node:os'; import path from 'node:path'; import { randomUUID } from 'node:crypto'; -import { CONFIG_DIR_NAME, ENV_PREFIX } from '../brand.js'; +import { CLI_COMMAND, CONFIG_DIR_NAME, ENV_PREFIX } from '../brand.js'; import { CliError, ConfigError, EXIT_CODES } from '../errors.js'; export interface BrowserSessionRecord { @@ -30,13 +30,24 @@ type StateFile = { version: 1; sessions: BrowserSessionRecord[] }; const SESSION_RETENTION_MS = 30 * 24 * 60 * 60 * 1000; export class SessionNotFoundError extends CliError { - constructor(sessionId: string, profileId: string) { + /** Profile that actually owns the Session, when the ID exists under a different one. */ + readonly ownerProfileId?: string; + + constructor(sessionId: string, profileId: string, ownerProfileId?: string) { + // A Session ID is only ever looked up inside the selected Profile, so a + // caller that omits `--profile` sees "not found" for a Session that does + // exist. Naming the owner turns a dead end into a one-step retry. super( 'SESSION_NOT_FOUND', - `Session not found: ${sessionId}`, - `Run \`webcmd --profile ${profileId} session list\` to choose an existing Session.`, + ownerProfileId + ? `Session not found in Profile ${profileId}: ${sessionId}` + : `Session not found: ${sessionId}`, + ownerProfileId + ? `Session ${sessionId} belongs to Profile ${ownerProfileId}. Re-run the same command with \`--profile ${ownerProfileId}\`, for example \`${CLI_COMMAND} --profile ${ownerProfileId} session close ${sessionId}\`.` + : `Run \`${CLI_COMMAND} --profile ${profileId} session list\` to choose an existing Session.`, EXIT_CODES.EMPTY_RESULT, ); + this.ownerProfileId = ownerProfileId; } } @@ -84,7 +95,7 @@ export class LocalBrowserSessionStore { requireSessionIdShape(id); const state = this.load(); const record = state.sessions.find((row) => row.id === id && row.profileId === profileId); - if (!record) throw new SessionNotFoundError(id, profileId); + if (!record) throw new SessionNotFoundError(id, profileId, findOwnerProfileId(state, id)); this.touchRecord(state, record); return { ...record }; } @@ -176,7 +187,7 @@ export class LocalBrowserSessionStore { private requireMutable(state: StateFile, profileId: string, sessionId: string): BrowserSessionRecord { requireSessionIdShape(sessionId); const record = state.sessions.find((row) => row.id === sessionId && row.profileId === profileId); - if (!record) throw new SessionNotFoundError(sessionId, profileId); + if (!record) throw new SessionNotFoundError(sessionId, profileId, findOwnerProfileId(state, sessionId)); return record; } @@ -225,6 +236,13 @@ export class LocalBrowserSessionStore { } } +// Every Profile's Sessions share one state file, so the owning Profile of a +// Session that missed the scoped lookup is already in hand — no daemon or +// provider round trip is needed to name it. +function findOwnerProfileId(state: StateFile, sessionId: string): string | undefined { + return state.sessions.find((row) => row.id === sessionId)?.profileId; +} + function validateState(value: unknown): StateFile { if (!value || typeof value !== 'object') throw new ConfigError('browser-sessions.json must contain an object.'); const state = value as { version?: unknown; sessions?: unknown }; diff --git a/src/cli.test.ts b/src/cli.test.ts index d8425e1f..ea46e2ac 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -2449,6 +2449,33 @@ describe('browser Session lifecycle commands', () => { }); }); + it('names the owning Profile when closing a Session without its --profile', async () => { + // The eval failure this guards: cleanup omitted `--profile`, the CLI said + // the Session did not exist, and the agent retried the same command. + mockSendCommand.mockRejectedValueOnce(new Error('daemon unavailable')); + const baseDir = path.join(isolatedCliTestHome, '.webcmd'); + fs.mkdirSync(baseDir, { recursive: true }); + fs.writeFileSync(path.join(baseDir, 'browser-sessions.json'), JSON.stringify({ + version: 1, + sessions: [{ + id: 'session_owned', + profileId: 'work', + kind: 'explicit', + createdAt: '2026-08-11T00:00:00.000Z', + updatedAt: '2026-08-11T00:00:00.000Z', + lastUsedAt: '2026-08-11T00:00:00.000Z', + }], + }), { mode: 0o600 }); + + await expect( + createProgram('', '').parseAsync(['node', 'webcmd', 'session', 'close', 'session_owned', '-f', 'json']), + ).rejects.toThrowError(expect.objectContaining({ + code: 'SESSION_NOT_FOUND', + ownerProfileId: 'work', + hint: 'Session session_owned belongs to Profile work. Re-run the same command with `--profile work`, for example `webcmd --profile work session close session_owned`.', + })); + }); + it.each([ ['create'], ['list'], diff --git a/src/cli.ts b/src/cli.ts index 440d76ad..a4f4adb3 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -892,7 +892,10 @@ export function createProgram(BUILTIN_CLIS: string, USER_CLIS: string, pluginsDi console.log(`No browser Sessions found for Profile ${profileId}.`); return; } - await renderOutput(output, { fmt, fmtExplicit: outputFormatIsExplicit(command), columns: ['id', 'kind', 'runtimeState', 'handoff'] }); + // `profileId` is a column because these rows are scoped to the selected + // Profile. Without it the table reads as every Session on the machine, + // which is what led agents to act on Sessions from unrelated Profiles. + await renderOutput(output, { fmt, fmtExplicit: outputFormatIsExplicit(command), columns: ['id', 'profileId', 'kind', 'runtimeState', 'handoff'] }); }); const sessionCloseCmd = addOutputFormatOption(sessionCmd