diff --git a/apps/claude-sdk-cli/src/createAppTools.ts b/apps/claude-sdk-cli/src/createAppTools.ts index 2d2c60d6..ffdc1a21 100644 --- a/apps/claude-sdk-cli/src/createAppTools.ts +++ b/apps/claude-sdk-cli/src/createAppTools.ts @@ -141,7 +141,7 @@ export function createAppTools({ fs, tsServer, toolsConfig, rulesProvider, objec // A real process-lifetime singleton, not just per-call: createAppTools is only ever invoked once // (see AppToolsService's factory registration in container.ts — core-di-lite memoizes by the // registration itself), so this is constructed exactly once for the process's lifetime. - const azSessionCache = new AzSessionCache(clock, logger); + const azSessionCache = new AzSessionCache(fs, clock, logger); tools.push(...createAdoPrTools(azDeps, getAzAccounts, azSessionCache)); tools.push(...createAzTools(azDeps, getAzAccounts, azSessionCache)); diff --git a/apps/claude-sdk-cli/test/MemoryFileSystem.ts b/apps/claude-sdk-cli/test/MemoryFileSystem.ts index a685f171..cf3e59bc 100644 --- a/apps/claude-sdk-cli/test/MemoryFileSystem.ts +++ b/apps/claude-sdk-cli/test/MemoryFileSystem.ts @@ -105,6 +105,25 @@ export class MemoryFileSystem extends IFileSystem { // Directories are implicit \u2014 nothing to remove when empty } + public async deleteDirectoryRecursive(path: string): Promise { + const prefix = path.endsWith('/') ? path : `${path}/`; + for (const p of [...this.files.keys()]) { + if (p.startsWith(prefix)) { + this.files.delete(p); + } + } + } + + public async mkdir(): Promise { + // Directories are implicit — nothing to create + } + + #mkdtempCounter = 0; + + public async mkdtemp(prefix: string): Promise { + return `${prefix}${this.#mkdtempCounter++}`; + } + public async stat(path: string): Promise { const content = this.files.get(path); if (content === undefined) { diff --git a/packages/claude-core/CHANGELOG.md b/packages/claude-core/CHANGELOG.md index 7097b3b7..3da69c67 100644 --- a/packages/claude-core/CHANGELOG.md +++ b/packages/claude-core/CHANGELOG.md @@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add the conversation-history model: store types, read/write interfaces, and near-duplicate detection (shingle, minhash, LSH) for the sweep - Add the Memory tool: a persistent, shared, relevance-searchable memory Claude reads and writes across sessions - Condition attached images: resize to a 2000px PNG long edge via sips, downscaling only, and log each outcome to the debug log; a missing or failing sips passes the image through unchanged +- IFileSystem gained mkdir, mkdtemp, and deleteDirectoryRecursive, and writeFile now accepts an optional mode - Parse mouse-wheel events from stdin into scroll_up/scroll_down key actions; add enableMouse/disableMouse escape sequences - Support binary file reads through encoding parameter on IFileSystem.readFile diff --git a/packages/claude-core/changes.jsonl b/packages/claude-core/changes.jsonl index 55888d78..34efc7c9 100644 --- a/packages/claude-core/changes.jsonl +++ b/packages/claude-core/changes.jsonl @@ -23,3 +23,4 @@ {"description":"Fix version metadata","category":"fixed"} {"description":"Depend on @shellicar/core-di instead of @shellicar/core-di-lite","category":"changed"} {"description":"Config merge now recurses arbitrarily deep instead of stopping after one nested level, so a local override several levels down (e.g. one entry of a nested record) no longer silently replaces its whole containing object and drops its siblings","category":"fixed"} +{"description":"IFileSystem gained mkdir, mkdtemp, and deleteDirectoryRecursive, and writeFile now accepts an optional mode","category":"added"} diff --git a/packages/claude-core/src/fs/interfaces.ts b/packages/claude-core/src/fs/interfaces.ts index e039f8aa..439ffb0f 100644 --- a/packages/claude-core/src/fs/interfaces.ts +++ b/packages/claude-core/src/fs/interfaces.ts @@ -10,9 +10,17 @@ export abstract class IFileSystem { public abstract homedir(): string; public abstract exists(path: string): Promise; public abstract readFile(path: string, encoding?: BufferEncoding): Promise; - public abstract writeFile(path: string, content: string): Promise; + public abstract writeFile(path: string, content: string, options?: { mode?: number }): Promise; public abstract deleteFile(path: string): Promise; public abstract deleteDirectory(path: string): Promise; + /** Recursive, force delete — for internal housekeeping (e.g. a session cache's own temp dirs), + * never exposed by a tool. `deleteDirectory` above stays non-recursive; that is the tool-facing + * safety contract. */ + public abstract deleteDirectoryRecursive(path: string): Promise; + /** Ensures a directory exists, creating any missing parents — no content, unlike `writeFile`. */ + public abstract mkdir(path: string): Promise; + /** Creates a fresh, uniquely-named directory inside the OS temp directory, named with `prefix`, and returns its path. */ + public abstract mkdtemp(prefix: string): Promise; public abstract rename(oldPath: string, newPath: string): Promise; public async find(path: string, options?: FindOptions): Promise { const re = options?.pattern ? new RegExp(options.pattern) : undefined; diff --git a/packages/claude-core/test/MemoryFileSystem.ts b/packages/claude-core/test/MemoryFileSystem.ts index 5c28f684..d88467ab 100644 --- a/packages/claude-core/test/MemoryFileSystem.ts +++ b/packages/claude-core/test/MemoryFileSystem.ts @@ -58,6 +58,18 @@ export class MemoryFileSystem extends IFileSystem { throw new Error('MemoryFileSystem: deleteDirectory() not supported'); } + public deleteDirectoryRecursive(): Promise { + throw new Error('MemoryFileSystem: deleteDirectoryRecursive() not supported'); + } + + public mkdir(): Promise { + throw new Error('MemoryFileSystem: mkdir() not supported'); + } + + public mkdtemp(): Promise { + throw new Error('MemoryFileSystem: mkdtemp() not supported'); + } + public appendFile(): Promise { throw new Error('MemoryFileSystem: appendFile() not supported'); } diff --git a/packages/claude-sdk-tools/CHANGELOG.md b/packages/claude-sdk-tools/CHANGELOG.md index 1888e2d0..4b1b7af5 100644 --- a/packages/claude-sdk-tools/CHANGELOG.md +++ b/packages/claude-sdk-tools/CHANGELOG.md @@ -91,6 +91,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - A failed tsserver request now throws instead of returning an empty result that was indistinguishable from a clean file - An interactive az identity no longer gets a silent, unattended background relogin; the browser/MFA prompt only ever appears attached to a real caller's call +- An interactive az identity now reuses an existing signed-in session found on disk after a CLI restart instead of always prompting for a fresh sign-in - AzCli, EscalatedAzCli, and every AzureDevOps.PullRequest.* tool now honor cancellation — an in-progress az login or command can be aborted instead of blocking until the process crashes or restarts - AzureDevOps.PullRequest.* tools accept an account field, matching AzCli/EscalatedAzCli - Binary files are blocked from text reads when the format is recognised; unrecognised formats are still treated as text diff --git a/packages/claude-sdk-tools/changes.jsonl b/packages/claude-sdk-tools/changes.jsonl index 0276a049..5a376765 100644 --- a/packages/claude-sdk-tools/changes.jsonl +++ b/packages/claude-sdk-tools/changes.jsonl @@ -87,3 +87,4 @@ {"description":"AzCli, EscalatedAzCli, and every AzureDevOps.PullRequest.* tool now honor cancellation — an in-progress az login or command can be aborted instead of blocking until the process crashes or restarts","category":"fixed"} {"description":"An interactive az identity no longer gets a silent, unattended background relogin; the browser/MFA prompt only ever appears attached to a real caller's call","category":"fixed"} {"description":"The az session's own login and command env now strips the same ambient Azure credential vars ExecV3 strips, so the CLI's own environment can no longer steer a login it believes it fully controls","category":"security"} +{"description":"An interactive az identity now reuses an existing signed-in session found on disk after a CLI restart instead of always prompting for a fresh sign-in","category":"fixed"} diff --git a/packages/claude-sdk-tools/src/Az/AzSessionCache.ts b/packages/claude-sdk-tools/src/Az/AzSessionCache.ts index 0874b5a7..331d1b1b 100644 --- a/packages/claude-sdk-tools/src/Az/AzSessionCache.ts +++ b/packages/claude-sdk-tools/src/Az/AzSessionCache.ts @@ -1,9 +1,8 @@ import { rmSync } from 'node:fs'; -import { mkdtemp, writeFile } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; import { join } from 'node:path'; import type { Clock } from '@js-joda/core'; import { Instant } from '@js-joda/core'; +import type { IFileSystem } from '@shellicar/claude-core/fs/interfaces'; import type { ILogger } from '@shellicar/claude-core/logging/ILogger'; import type { IExecutor } from '@shellicar/exec-core'; import { ensureAzExtensionDir, ensureAzInteractiveSessionDir, type RunResult, removeConfigDir, runOnce, stripAmbientAzureEnv } from '../az-shared'; @@ -47,12 +46,17 @@ export class AzSessionCache { readonly #entries = new Map(); readonly #allConfigDirs = new Set(); readonly #onExit: () => void; + readonly #fs: IFileSystem; readonly #clock: Clock; readonly #logger?: ILogger; - public constructor(clock: Clock, logger?: ILogger) { + public constructor(fs: IFileSystem, clock: Clock, logger?: ILogger) { + this.#fs = fs; this.#clock = clock; this.#logger = logger; + // A synchronous, direct node:fs call is a deliberate exception to the IFileSystem abstraction + // used everywhere else in this class: this runs on the 'exit' event, where async work cannot + // complete, so there is no IFileSystem call this could route through. this.#onExit = () => { for (const dir of this.#allConfigDirs) { try { @@ -151,7 +155,7 @@ export class AzSessionCache { // path every login for this account/identity reuses, so discarding it here would destroy the // persistence the whole interactive path exists for. if (deps.getIdentity(account, identity).type === 'cert') { - await removeConfigDir(result.configDir); + await removeConfigDir(this.#fs, result.configDir); this.#allConfigDirs.delete(result.configDir); } return; @@ -163,21 +167,28 @@ export class AzSessionCache { async #doLogin(deps: AzDeps, identity: 'reader' | 'holder', account: string, cwd: string, key: string, signal?: AbortSignal): Promise { const identityConfig = deps.getIdentity(account, identity); const tenantId = deps.getTenantId(account); - const extensionDir = await ensureAzExtensionDir(); + const extensionDir = await ensureAzExtensionDir(this.#fs); // Cert-SP gets a fresh throwaway dir per login (cheap, silent relogin — no reason to keep it // around, and #onExit sweeps it). Interactive gets a stable dir reused across restarts, so its // MSAL token cache survives a CLI restart without forcing MFA again — never swept on exit. - const configDir = identityConfig.type === 'cert' ? await mkdtemp(join(tmpdir(), 'az-')) : await ensureAzInteractiveSessionDir(account, identity); + const configDir = identityConfig.type === 'cert' ? await this.#fs.mkdtemp('az-') : await ensureAzInteractiveSessionDir(this.#fs, account, identity); if (identityConfig.type === 'cert') { this.#allConfigDirs.add(configDir); } const env = { ...stripAmbientAzureEnv(process.env), AZURE_CONFIG_DIR: configDir, AZURE_EXTENSION_DIR: extensionDir }; + if (identityConfig.type === 'interactive') { + const reused = await this.#tryReuseInteractiveSession(deps.executor, cwd, env, tenantId, key); + if (reused != null) { + return reused; + } + } + const loginArgs = ['login', '--tenant', tenantId]; if (identityConfig.type === 'cert') { const certPath = join(configDir, 'cert.pem'); - await writeFile(certPath, deps.getCert(account, identity), { mode: 0o600 }); + await this.#fs.writeFile(certPath, deps.getCert(account, identity), { mode: 0o600 }); loginArgs.push('--service-principal', '-u', identityConfig.clientId, '--certificate', certPath); if (identityConfig.subscriptionIds.length === 0) { loginArgs.push('--allow-no-subscriptions'); @@ -200,13 +211,12 @@ export class AzSessionCache { } if (login.exitCode !== 0) { if (identityConfig.type === 'cert') { - await removeConfigDir(configDir); + await removeConfigDir(this.#fs, configDir); this.#allConfigDirs.delete(configDir); } this.#logger?.warn('az_login_failed', { key, exitCode: login.exitCode, durationMs: this.#clock.millis() - loginStartedAt }); return { loginFailed: login }; } - const loginAt = this.#clock.millis(); const lifetimeMs = await this.#tokenLifetimeMs(deps.executor, cwd, env); const refreshAt = loginAt + lifetimeMs * REFRESH_FRACTION; @@ -222,6 +232,45 @@ export class AzSessionCache { return { configDir, extensionDir, refreshAt, hardExpireAt }; } + /** Cold-start reuse check for an interactive identity only: the session dir from + * `ensureAzInteractiveSessionDir` is stable across CLI restarts, so a prior process may already + * have left a good login there. Queries the local account cache for an entry in the configured + * tenant — no network call, no token minted — and if one exists, skips `az login` entirely and + * reuses the existing session, computing `refreshAt`/`hardExpireAt` from the token's real + * lifetime exactly as a fresh login would. Returns null when nothing reusable is found, so the + * caller falls through to the normal `az login` flow unchanged. */ + async #tryReuseInteractiveSession(executor: IExecutor, cwd: string, env: NodeJS.ProcessEnv, tenantId: string, key: string): Promise { + const probe = await runOnce(executor, 'az', ['account', 'list', '--output', 'json'], cwd, env); + const hasTenant = probe.exitCode === 0 && this.#accountListHasTenant(probe.stdout, tenantId); + if (!hasTenant) { + this.#logger?.debug('az_session_reuse_miss', { key }); + return null; + } + const loginAt = this.#clock.millis(); + const lifetimeMs = await this.#tokenLifetimeMs(executor, cwd, env); + const refreshAt = loginAt + lifetimeMs * REFRESH_FRACTION; + const hardExpireAt = loginAt + lifetimeMs * HARD_EXPIRE_FRACTION; + this.#logger?.info('az_session_reused_from_disk', { + key, + tokenLifetimeMs: lifetimeMs, + tokenLifetimeMinutes: Math.round(lifetimeMs / 60_000), + refreshAt: Instant.ofEpochMilli(refreshAt).toString(), + hardExpireAt: Instant.ofEpochMilli(hardExpireAt).toString(), + }); + return { configDir: env.AZURE_CONFIG_DIR as string, extensionDir: env.AZURE_EXTENSION_DIR as string, refreshAt, hardExpireAt }; + } + + /** Parses `az account list`'s own JSON rather than filtering it server-side with a `--query` string + * built from `tenantId` — a tenant id containing `'` would otherwise break the JMESPath expression. */ + #accountListHasTenant(stdout: string, tenantId: string): boolean { + try { + const accounts = JSON.parse(stdout) as Array<{ tenantId?: string }>; + return accounts.some((account) => account.tenantId === tenantId); + } catch { + return false; + } + } + /** Reads the token's real lifetime so refresh/expiry are bounded by fact, not an assumption. Falls * back to a conservative default if `az` can't report an expiry (older CLI, transient failure) — * the fallback only ever makes the cache refresh sooner than a longer-lived real token needed. */ diff --git a/packages/claude-sdk-tools/src/az-shared.ts b/packages/claude-sdk-tools/src/az-shared.ts index 4aefba48..142b6854 100644 --- a/packages/claude-sdk-tools/src/az-shared.ts +++ b/packages/claude-sdk-tools/src/az-shared.ts @@ -1,7 +1,7 @@ -import { mkdir, rm } from 'node:fs/promises'; import { homedir } from 'node:os'; import { join } from 'node:path'; import { PassThrough } from 'node:stream'; +import type { IFileSystem } from '@shellicar/claude-core/fs/interfaces'; import type { IExecutor } from '@shellicar/exec-core'; /** Azure CLI installs extensions (e.g. `azure-devops`) under AZURE_EXTENSION_DIR. Each escalated @@ -11,8 +11,8 @@ import type { IExecutor } from '@shellicar/exec-core'; * once ever instead of once per call, which was the real cost behind every call being slow. */ const AZ_EXTENSION_DIR = join(homedir(), '.claude', 'az-extensions'); -export async function ensureAzExtensionDir(): Promise { - await mkdir(AZ_EXTENSION_DIR, { recursive: true }); +export async function ensureAzExtensionDir(fs: IFileSystem): Promise { + await fs.mkdir(AZ_EXTENSION_DIR); return AZ_EXTENSION_DIR; } @@ -38,9 +38,9 @@ function platformDataDir(appName: string): string { * without forcing a fresh interactive sign-in (MFA/CA prompt) every time. Cert-SP identities never * use this: their relogin is silent and cheap, so they get a fresh throwaway dir per login instead * (see `AzSessionCache`). */ -export async function ensureAzInteractiveSessionDir(account: string, identity: 'reader' | 'holder'): Promise { +export async function ensureAzInteractiveSessionDir(fs: IFileSystem, account: string, identity: 'reader' | 'holder'): Promise { const dir = join(platformDataDir('claude-sdk-cli'), 'az-sessions', `${account}-${identity}`); - await mkdir(dir, { recursive: true }); + await fs.mkdir(dir); return dir; } @@ -81,10 +81,10 @@ const REMOVE_RETRY_BASE_MS = 100; * for a moment after the foreground command exits, racing a plain rm with ENOTEMPTY. Retry a few * times with a short linear backoff before giving up — this is a timing issue on our side, not a * reason to leave the temp dir behind. */ -export async function removeConfigDir(dir: string): Promise { +export async function removeConfigDir(fs: IFileSystem, dir: string): Promise { for (let attempt = 1; attempt <= REMOVE_ATTEMPTS; attempt++) { try { - await rm(dir, { recursive: true, force: true }); + await fs.deleteDirectoryRecursive(dir); return; } catch (err) { if (attempt === REMOVE_ATTEMPTS) { diff --git a/packages/claude-sdk-tools/src/fs/NodeFileSystem.ts b/packages/claude-sdk-tools/src/fs/NodeFileSystem.ts index 9e5590ab..1f590566 100644 --- a/packages/claude-sdk-tools/src/fs/NodeFileSystem.ts +++ b/packages/claude-sdk-tools/src/fs/NodeFileSystem.ts @@ -1,7 +1,7 @@ import { createWriteStream, existsSync } from 'node:fs'; -import { appendFile, readdir as fsReaddir, readlink as fsReadlink, realpath as fsRealpath, rename as fsRename, stat as fsStat, mkdir, readFile, rm, rmdir, writeFile } from 'node:fs/promises'; -import { homedir as osHomedir } from 'node:os'; -import { dirname } from 'node:path'; +import { appendFile, chmod, mkdtemp as fsMkdtemp, readdir as fsReaddir, readlink as fsReadlink, realpath as fsRealpath, rename as fsRename, stat as fsStat, mkdir, readFile, rm, rmdir, writeFile } from 'node:fs/promises'; +import { homedir as osHomedir, tmpdir } from 'node:os'; +import { dirname, join } from 'node:path'; import type { Writable } from 'node:stream'; import { IFileSystem } from '@shellicar/claude-core/fs/interfaces'; import type { IFileEntry, StatResult } from '@shellicar/claude-core/fs/types'; @@ -34,9 +34,14 @@ export class NodeFileSystem extends IFileSystem { return readFile(path, encoding); } - public async writeFile(path: string, content: string): Promise { + public async writeFile(path: string, content: string, options?: { mode?: number }): Promise { await mkdir(dirname(path), { recursive: true }); - await writeFile(path, content, 'utf-8'); + await writeFile(path, content, { encoding: 'utf-8', mode: options?.mode }); + // writeFile's mode option only applies when creating a new file, so an explicit chmod is needed + // to also lock down a file that already existed before this write with looser permissions. + if (options?.mode != null) { + await chmod(path, options.mode); + } } public async deleteFile(path: string): Promise { @@ -47,6 +52,18 @@ export class NodeFileSystem extends IFileSystem { await rmdir(path); } + public async deleteDirectoryRecursive(path: string): Promise { + await rm(path, { recursive: true, force: true }); + } + + public async mkdir(path: string): Promise { + await mkdir(path, { recursive: true }); + } + + public async mkdtemp(prefix: string): Promise { + return fsMkdtemp(join(tmpdir(), prefix)); + } + public async appendFile(path: string, content: string): Promise { await mkdir(dirname(path), { recursive: true }); await appendFile(path, content, 'utf-8'); diff --git a/packages/claude-sdk-tools/test/integration/AzSessionCache.mechanism.spec.ts b/packages/claude-sdk-tools/test/Az/AzSessionCache.mechanism.spec.ts similarity index 64% rename from packages/claude-sdk-tools/test/integration/AzSessionCache.mechanism.spec.ts rename to packages/claude-sdk-tools/test/Az/AzSessionCache.mechanism.spec.ts index 27acb5c0..9f31c2c0 100644 --- a/packages/claude-sdk-tools/test/integration/AzSessionCache.mechanism.spec.ts +++ b/packages/claude-sdk-tools/test/Az/AzSessionCache.mechanism.spec.ts @@ -1,12 +1,10 @@ -import { mkdtemp, rm } from 'node:fs/promises'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; import { Clock, ZoneOffset } from '@js-joda/core'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; import { AzSessionCache } from '../../src/Az/AzSessionCache'; import type { AzDeps } from '../../src/Az/runAz'; import type { AzIdentityConfig } from '../../src/Az/tools'; import { FakeExecutor } from '../FakeExecutor'; +import { MemoryFileSystem } from '../MemoryFileSystem'; class FixedClock extends Clock { public instant() { @@ -70,33 +68,10 @@ function tokenJson(expiresAtMs: number): string { return JSON.stringify({ expires_on: Math.floor(expiresAtMs / 1000) }); } -// The interactive mechanism creates a real, stable data-dir entry, and the cert mechanism writes a -// real cert.pem into a real mkdtemp dir — both genuine disk operations, not fakeable without an -// injected filesystem seam, so this suite lives in the integration tier. describe('AzSessionCache — mechanism branching', () => { - let previousXdgDataHome: string | undefined; - let scratchDataDir: string; - const ephemeralConfigDirs: string[] = []; - - beforeEach(async () => { - previousXdgDataHome = process.env.XDG_DATA_HOME; - scratchDataDir = await mkdtemp(join(tmpdir(), 'az-session-cache-test-')); - process.env.XDG_DATA_HOME = scratchDataDir; - }); - - afterEach(async () => { - if (previousXdgDataHome == null) { - delete process.env.XDG_DATA_HOME; - } else { - process.env.XDG_DATA_HOME = previousXdgDataHome; - } - await rm(scratchDataDir, { recursive: true, force: true }); - await Promise.all(ephemeralConfigDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }).catch(() => {}))); - }); - it('passes --tenant but no --service-principal for an interactive identity', async () => { const executor = new FakeExecutor(() => ({ exitCode: 0 })); - const cache = new AzSessionCache(new FixedClock()); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); await cache.getSession(deps, 'reader', 'acct', '/cwd'); @@ -107,13 +82,10 @@ describe('AzSessionCache — mechanism branching', () => { it('passes --service-principal and --certificate for a cert identity with no subscriptionIds', async () => { const executor = new FakeExecutor(() => ({ exitCode: 0 })); - const cache = new AzSessionCache(new FixedClock()); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); const deps = makeDeps({ type: 'cert', clientId: 'client-1', subscriptionIds: [] }, executor); - const session = await cache.getSession(deps, 'reader', 'acct', '/cwd'); - if ('configDir' in session) { - ephemeralConfigDirs.push(session.configDir); - } + await cache.getSession(deps, 'reader', 'acct', '/cwd'); const [call] = loginCalls(executor); expect(call.args).toEqual(['login', '--tenant', 'tenant-id', '--service-principal', '-u', 'client-1', '--certificate', expect.stringContaining('cert.pem'), '--allow-no-subscriptions']); @@ -121,7 +93,7 @@ describe('AzSessionCache — mechanism branching', () => { it('loops one login per configured subscription id, skipping discovery, without --allow-no-subscriptions', async () => { const executor = new FakeExecutor(() => ({ exitCode: 0 })); - const cache = new AzSessionCache(new FixedClock()); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); const deps = makeDeps({ type: 'interactive', subscriptionIds: ['sub-1', 'sub-2'] }, executor); await cache.getSession(deps, 'reader', 'acct', '/cwd'); @@ -134,12 +106,15 @@ describe('AzSessionCache — mechanism branching', () => { }); it('stops the subscription loop on the first failing login and reports it', async () => { - let calls = 0; - const executor = new FakeExecutor(() => { - calls += 1; - return { exitCode: calls === 1 ? 1 : 0 }; + let loginAttempts = 0; + const executor = new FakeExecutor((cmd) => { + if (cmd.program === 'az' && cmd.args?.[0] === 'login') { + loginAttempts += 1; + return { exitCode: loginAttempts === 1 ? 1 : 0 }; + } + return { exitCode: 0 }; }); - const cache = new AzSessionCache(new FixedClock()); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); const deps = makeDeps({ type: 'interactive', subscriptionIds: ['sub-1', 'sub-2'] }, executor); const session = await cache.getSession(deps, 'reader', 'acct', '/cwd'); @@ -150,7 +125,7 @@ describe('AzSessionCache — mechanism branching', () => { it('strips ambient Azure credential env vars from the login env', async () => { const executor = new FakeExecutor(() => ({ exitCode: 0 })); - const cache = new AzSessionCache(new FixedClock()); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); const previous = process.env.AZURE_CLIENT_SECRET; process.env.AZURE_CLIENT_SECRET = 'ambient-secret'; @@ -172,7 +147,7 @@ describe('AzSessionCache — mechanism branching', () => { it('never starts a background refresh for an interactive identity crossing refreshAt', async () => { const executor = new FakeExecutor(() => ({ exitCode: 0, stdout: tokenJson(1000) })); const clock = new MutableClock(0); - const cache = new AzSessionCache(clock); + const cache = new AzSessionCache(new MemoryFileSystem(), clock); const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); await cache.getSession(deps, 'reader', 'acct', '/cwd'); @@ -187,4 +162,52 @@ describe('AzSessionCache — mechanism branching', () => { const loginsAfterRefreshWindow = loginCalls(executor).length; expect(loginsAfterRefreshWindow).toBe(loginsAfterColdStart); }); + + it('reuses an existing interactive session from disk instead of logging in again', async () => { + const executor = new FakeExecutor((cmd) => { + if (cmd.program === 'az' && cmd.args?.[0] === 'account' && cmd.args?.[1] === 'list') { + return { exitCode: 0, stdout: JSON.stringify([{ tenantId: 'tenant-id' }]) }; + } + if (cmd.program === 'az' && cmd.args?.[0] === 'account' && cmd.args?.[1] === 'get-access-token') { + return { exitCode: 0, stdout: tokenJson(1000) }; + } + return { exitCode: 0 }; + }); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); + const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); + + await cache.getSession(deps, 'reader', 'acct', '/cwd'); + + expect(loginCalls(executor)).toHaveLength(0); + }); + + it('logs in when the interactive account-list probe finds nothing for the configured tenant', async () => { + const executor = new FakeExecutor((cmd) => { + if (cmd.program === 'az' && cmd.args?.[0] === 'account' && cmd.args?.[1] === 'list') { + return { exitCode: 0, stdout: JSON.stringify([{ tenantId: 'other-tenant' }]) }; + } + return { exitCode: 0 }; + }); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); + const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); + + await cache.getSession(deps, 'reader', 'acct', '/cwd'); + + expect(loginCalls(executor)).toHaveLength(1); + }); + + it('logs in when the interactive account-list probe itself fails', async () => { + const executor = new FakeExecutor((cmd) => { + if (cmd.program === 'az' && cmd.args?.[0] === 'account' && cmd.args?.[1] === 'list') { + return { exitCode: 1 }; + } + return { exitCode: 0 }; + }); + const cache = new AzSessionCache(new MemoryFileSystem(), new FixedClock()); + const deps = makeDeps({ type: 'interactive', subscriptionIds: [] }, executor); + + await cache.getSession(deps, 'reader', 'acct', '/cwd'); + + expect(loginCalls(executor)).toHaveLength(1); + }); }); diff --git a/packages/claude-sdk-tools/test/integration/AzSessionCache.spec.ts b/packages/claude-sdk-tools/test/AzSessionCache.spec.ts similarity index 89% rename from packages/claude-sdk-tools/test/integration/AzSessionCache.spec.ts rename to packages/claude-sdk-tools/test/AzSessionCache.spec.ts index 2175530c..01c29476 100644 --- a/packages/claude-sdk-tools/test/integration/AzSessionCache.spec.ts +++ b/packages/claude-sdk-tools/test/AzSessionCache.spec.ts @@ -1,10 +1,10 @@ -import { rm } from 'node:fs/promises'; import type { PassThrough } from 'node:stream'; import { Clock, Instant, ZoneOffset } from '@js-joda/core'; import type { CommandSpec, ExitStatus, IExecutor, SpawnOpts } from '@shellicar/exec-core'; -import { afterEach, describe, expect, it } from 'vitest'; -import { AzSessionCache } from '../../src/Az/AzSessionCache'; -import type { AzDeps } from '../../src/Az/runAz'; +import { describe, expect, it } from 'vitest'; +import { AzSessionCache } from '../src/Az/AzSessionCache'; +import type { AzDeps } from '../src/Az/runAz'; +import { MemoryFileSystem } from './MemoryFileSystem'; // A settable fake clock, injected exactly the way CLAUDE.md's "never read the system clock // directly" convention expects: the test moves time by hand between steps instead of faking @@ -88,16 +88,7 @@ async function waitForCallCount(executor: ControllableExecutor, n: number): Prom } } -// Cert-mechanism logins write a real cert.pem into a real mkdtemp dir — the actual disk write is -// what's under test here (config-dir lifecycle across concurrent/racing logins), so this lives in -// the integration tier rather than being faked. describe('AzSessionCache — background refresh vs hard-expiry relogin race', () => { - const configDirs: string[] = []; - - afterEach(async () => { - await Promise.all(configDirs.splice(0).map((dir) => rm(dir, { recursive: true, force: true }).catch(() => {}))); - }); - // The cache entry is written to the map synchronously, inside #login, before its own first // await — so two calls issued back-to-back (as Promise.all does: each runs to its first await // before the next starts) can never both observe a cold miss. The second always finds the @@ -107,7 +98,7 @@ describe('AzSessionCache — background refresh vs hard-expiry relogin race', () const clock = new MutableClock(0); const executor = new ControllableExecutor(); const deps = makeDeps(executor); - const cache = new AzSessionCache(clock); + const cache = new AzSessionCache(new MemoryFileSystem(), clock); const [promiseA, promiseB] = [cache.getSession(deps, 'reader', 'acct', '/cwd'), cache.getSession(deps, 'reader', 'acct', '/cwd')]; @@ -124,7 +115,6 @@ describe('AzSessionCache — background refresh vs hard-expiry relogin race', () if ('loginFailed' in sessionA || 'loginFailed' in sessionB) { throw new Error('unreachable'); } - configDirs.push(sessionA.configDir); const expected = sessionA.configDir; const actual = sessionB.configDir; @@ -141,7 +131,7 @@ describe('AzSessionCache — background refresh vs hard-expiry relogin race', () const clock = new MutableClock(0); const executor = new ControllableExecutor(); const deps = makeDeps(executor); - const cache = new AzSessionCache(clock); + const cache = new AzSessionCache(new MemoryFileSystem(), clock); // Cold start at t=0: a 1000ms-lifetime token, so refreshAt=500, hardExpireAt=750. const coldPromise = cache.getSession(deps, 'reader', 'acct', '/cwd'); @@ -153,7 +143,6 @@ describe('AzSessionCache — background refresh vs hard-expiry relogin race', () if ('loginFailed' in sessionA) { throw new Error('unreachable'); } - configDirs.push(sessionA.configDir); // t=600: past refreshAt (500), before hardExpireAt (750) — starts a background refresh. // Its login/token calls (indices 2 and 3) are deliberately left unresolved: this is the slow @@ -181,7 +170,6 @@ describe('AzSessionCache — background refresh vs hard-expiry relogin race', () if ('loginFailed' in sessionC) { throw new Error('unreachable'); } - configDirs.push(sessionC.configDir); // Now let the earlier, slower background refresh land, after the hard-expiry relogin already // replaced the cache entry. Its token call only registers once this login resolves, landing at diff --git a/packages/claude-sdk-tools/test/MemoryFileSystem.ts b/packages/claude-sdk-tools/test/MemoryFileSystem.ts index 1b8e5748..62104eda 100644 --- a/packages/claude-sdk-tools/test/MemoryFileSystem.ts +++ b/packages/claude-sdk-tools/test/MemoryFileSystem.ts @@ -93,6 +93,25 @@ export class MemoryFileSystem extends IFileSystem { // Directories are implicit \u2014 nothing to remove when empty } + public async deleteDirectoryRecursive(path: string): Promise { + const prefix = path.endsWith('/') ? path : `${path}/`; + for (const p of [...this.files.keys()]) { + if (p.startsWith(prefix)) { + this.files.delete(p); + } + } + } + + public async mkdir(): Promise { + // Directories are implicit — nothing to create + } + + #mkdtempCounter = 0; + + public async mkdtemp(prefix: string): Promise { + return `${prefix}${this.#mkdtempCounter++}`; + } + public async stat(path: string): Promise { const content = this.files.get(path); if (content === undefined) { diff --git a/packages/claude-sdk-tools/test/find-symlinks.spec.ts b/packages/claude-sdk-tools/test/find-symlinks.spec.ts index 37029756..e7747a79 100644 --- a/packages/claude-sdk-tools/test/find-symlinks.spec.ts +++ b/packages/claude-sdk-tools/test/find-symlinks.spec.ts @@ -87,6 +87,18 @@ class SymlinkMockFileSystem extends IFileSystem { throw new Error('not implemented'); } + public async deleteDirectoryRecursive(): Promise { + throw new Error('not implemented'); + } + + public async mkdir(): Promise { + throw new Error('not implemented'); + } + + public async mkdtemp(): Promise { + throw new Error('not implemented'); + } + public async rename(): Promise { throw new Error('not implemented'); }