From 4709e46aa6897a0cde45c7f71db1f6f6e668b9c1 Mon Sep 17 00:00:00 2001 From: konghuanjun Date: Wed, 2 Sep 2026 16:34:43 +0800 Subject: [PATCH 1/2] fix(agent-core-v2): rework tower mode enter failures and config-flag assembly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IAgentTowerService.enter() returns a typed TowerEnterResult: the 4 former silent failure points now carry their reason (not-main-agent / experiment-off / feature-not-assembled / owned-by-live-session with the owner session id + title); kap-server and node-sdk map each reason to a distinct SESSION_TOWER_MODE_INVALID message via towerEnterFailureMessage() instead of one misleading catch-all. - Fix config-sourced [experimental] tower flag never assembling the tower feature: ConfigService seeds its state synchronously at construction with a best-effort readFileSync of the config document it owns, so the flag is already visible when TowerFeature's constructor runs during App-scope creation. No event machinery: a runtime config flip updates flag reads but does not re-run feature constructors — a restart is required, pinned by test. docs/flag.md + docs/features.md document the contract. - Consolidated changeset for @moonshot-ai/kimi-code. --- .changeset/tower-mode-fixes.md | 5 + packages/agent-core-v2/docs/features.md | 7 + packages/agent-core-v2/docs/flag.md | 3 +- .../src/app/config/configService.ts | 24 +++ .../agent-core-v2/src/features/tower/tower.ts | 34 +++- .../src/features/tower/towerService.ts | 30 ++- .../features/tower/tools/spawnTool.test.ts | 2 +- .../features/tower/tools/towerTools.test.ts | 1 + .../test/features/tower/towerFeature.test.ts | 87 +++++++++ .../test/features/tower/towerService.test.ts | 177 ++++++++++++++++-- .../src/routes/sessionAgentConfig.ts | 7 +- packages/kap-server/test/sessions.test.ts | 5 +- packages/node-sdk/src/sdk-rpc-client-v2.ts | 7 +- .../node-sdk/test/sdk-rpc-client-v2.test.ts | 10 +- 14 files changed, 364 insertions(+), 35 deletions(-) create mode 100644 .changeset/tower-mode-fixes.md diff --git a/.changeset/tower-mode-fixes.md b/.changeset/tower-mode-fixes.md new file mode 100644 index 00000000000..68f56cacf5f --- /dev/null +++ b/.changeset/tower-mode-fixes.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Tower mode (experimental, `KIMI_CODE_EXPERIMENTAL_TOWER=1`): fix tower mode never starting when enabled through `[experimental] tower = true` in `config.toml` instead of the environment variable. When tower mode cannot be enabled, the error now names the actual blocker — the disabled experiment, a required restart, or the owning session. When another live session owns the workspace tower, the message also names the owning session's title alongside its id. diff --git a/packages/agent-core-v2/docs/features.md b/packages/agent-core-v2/docs/features.md index 4f4c750c528..08b8ed9b456 100644 --- a/packages/agent-core-v2/docs/features.md +++ b/packages/agent-core-v2/docs/features.md @@ -63,6 +63,13 @@ compositions over the existing seams: feature contributes at Agent scope appears in every existing and future Agent scope, bound by the same cascade rules as a static registration. +A feature whose assembly is gated on an experimental flag decides inside its constructor +(e.g. `TowerFeature` returns early when `flags.enabled('tower')` is false). `ConfigService` +seeds its state synchronously at construction (a best-effort sync read of the config +file), so a config-sourced flag is already visible in that constructor. A flag flipped at +runtime does not re-run feature constructors: turning a flag-gated feature on or off from +config at runtime requires a restart. + ## Static channels vs Feature channels (the rule for built-in features) Some contribution kinds must stay on the **static import=register channels** even when diff --git a/packages/agent-core-v2/docs/flag.md b/packages/agent-core-v2/docs/flag.md index 62201081265..09c3f1f4e73 100644 --- a/packages/agent-core-v2/docs/flag.md +++ b/packages/agent-core-v2/docs/flag.md @@ -34,7 +34,8 @@ Highest wins; env is read live on every call (nothing cached): ## Config integration - `FlagService` registers the `[experimental]` section into `IConfigRegistry` at construction (`registerSection('experimental', ExperimentalConfigSchema)`) and reads overrides from `IConfigService`. -- It subscribes `IConfigService.onDidChangeConfiguration` and refreshes overrides whenever the `experimental` domain changes, so config edits apply live. +- It subscribes `IConfigService.onDidChangeConfiguration` and refreshes overrides whenever the `experimental` domain changes, so config edits apply live. `ConfigService` seeds its state synchronously at construction (a best-effort sync read of the config file), so overrides from `config.toml` are already visible during App-scope creation. +- Overrides are re-read on every change (`enabled()`/`explain()` are computed, not cached), so new values apply to all subsequent calls. Decisions already made from the old values are not revisited: in particular a flag-gated Feature's constructor runs once at assembly, and a runtime flip only takes effect for it after a restart. - `IConfigRegistry.registerSection` throws if a domain is registered twice — `experimental` is owned exclusively by `FlagService`. - `setConfigOverrides(overrides)` is an imperative escape hatch for tests and hosts without an `IConfigService`; hosts on `IConfigService` should set the `[experimental]` section instead. diff --git a/packages/agent-core-v2/src/app/config/configService.ts b/packages/agent-core-v2/src/app/config/configService.ts index 71a2577e4bf..d427c5d1997 100644 --- a/packages/agent-core-v2/src/app/config/configService.ts +++ b/packages/agent-core-v2/src/app/config/configService.ts @@ -1,3 +1,7 @@ +import { readFileSync } from 'node:fs'; + +import { parse as parseToml } from 'smol-toml'; + import { type CollectionView } from '#/_base/di/collection'; import { Disposable } from '#/_base/di/lifecycle'; import { LifecycleScope } from '#/app/scopes'; @@ -322,6 +326,7 @@ export class ConfigService extends Disposable implements IConfigService { this._register(this.registry.onDidRegisterOverlay(() => this.reapplyOverlays())); const { configKey } = this; const { homeDir } = this.bootstrap; + this.seedInitialLoad(); this.ready = (async () => { await migrateThinkingEffortMaxToHigh(this.documentStore, configKey, homeDir); await this.load('load'); @@ -519,6 +524,25 @@ export class ConfigService extends Disposable implements IConfigService { return run; } + private seedInitialLoad(): void { + let fileData: ResolvedConfig; + try { + const text = readFileSync(this.bootstrap.configPath, 'utf8'); + const data: unknown = text.trim().length === 0 ? {} : parseToml(text); + if (!isPlainObject(data)) return; + fileData = data; + } catch { + return; + } + this.rawSnake = cloneRecord(fileData); + this.raw = transformTomlData(fileData, this.registry); + this.validated = this.buildValidated(this.raw); + const next = { ...this.validated }; + this.applySectionEnvBindings(next, true); + this.applyEnvOverlay(next); + this.effective = next; + } + private async load(source: ConfigChangeSource): Promise { this.diagnosticsList.length = 0; let fileData: ResolvedConfig = {}; diff --git a/packages/agent-core-v2/src/features/tower/tower.ts b/packages/agent-core-v2/src/features/tower/tower.ts index a9fd8acf795..1e7e50f90d0 100644 --- a/packages/agent-core-v2/src/features/tower/tower.ts +++ b/packages/agent-core-v2/src/features/tower/tower.ts @@ -17,12 +17,44 @@ export const TOWER_WORKER_PROFILE = 'tower-worker'; export const TOWER_FLAG_ID = 'tower'; +export type TowerEnterFailure = + | { + readonly entered: false; + readonly reason: 'not-main-agent' | 'experiment-off' | 'feature-not-assembled'; + } + | { + readonly entered: false; + readonly reason: 'owned-by-live-session'; + readonly owner: string; + readonly ownerTitle?: string; + }; + +export type TowerEnterResult = { readonly entered: true } | TowerEnterFailure; + +export function towerEnterFailureMessage(failure: TowerEnterFailure): string { + switch (failure.reason) { + case 'not-main-agent': + return 'tower mode is only supported by the main agent'; + case 'experiment-off': + return 'the tower experiment is disabled; enable it with KIMI_CODE_EXPERIMENTAL_TOWER=1 or `[experimental] tower = true` in config.toml'; + case 'feature-not-assembled': + return 'the tower feature is not assembled in this process; a restart is required'; + case 'owned-by-live-session': { + const owner = + failure.ownerTitle === undefined + ? failure.owner + : `${failure.ownerTitle} (${failure.owner})`; + return `another live session owns the workspace tower (session ${owner})`; + } + } +} + export interface IAgentTowerService { readonly _serviceBrand: undefined; readonly isActive: boolean; readonly requestedBase: string | undefined; - enter(base?: string): Promise; + enter(base?: string): Promise; exit(): void; } diff --git a/packages/agent-core-v2/src/features/tower/towerService.ts b/packages/agent-core-v2/src/features/tower/towerService.ts index 796ec7609e8..6c771d09ff2 100644 --- a/packages/agent-core-v2/src/features/tower/towerService.ts +++ b/packages/agent-core-v2/src/features/tower/towerService.ts @@ -1,7 +1,7 @@ import { join } from 'node:path'; import { Disposable } from '#/_base/di/lifecycle'; -import { ScopeActivation, registerScopedService } from '#/_base/di/scope'; +import { ScopeActivation, registerScopedService, type ISessionScopeHandle } from '#/_base/di/scope'; import { IAgentReminderService } from '#/features/reminder/reminderService'; import { IAgentLifecycleService } from '#/session/agentLifecycle/agentLifecycle'; import { IAgentContextMemoryService } from '#/agent/contextMemory/contextMemory'; @@ -25,6 +25,8 @@ import { ISessionActivityView } from '#/session/sessionActivity/sessionActivity' import { isWithinDirectory } from '#/tool/path-access'; import type { ToolFileAccess } from '#/tool/toolContract'; import { ISessionContext } from '#/session/sessionContext/sessionContext'; +import { ISessionMetadata } from '#/session/sessionMetadata/sessionMetadata'; +import { isUntitled } from '#/session/sessionMetadata/promptMetadata'; import { SubagentStarted } from '#/session/subagent/mirrorAgentRun'; import { TowerModeInjection } from './injection/towerModeInjection'; import { @@ -43,6 +45,7 @@ import { TOWER_FLAG_ID, TOWER_TOOL_NAMES, TOWER_WORKER_PROFILE, + type TowerEnterResult, } from './tower'; import { isTowerFeatureAssembled } from './towerFeature'; import { TowerModeEnter, TowerModeExit, towerBaseKey, towerKey, towerOwnerKey } from './towerOps'; @@ -186,10 +189,10 @@ export class AgentTowerService extends Disposable implements IAgentTowerService ); } - async enter(base?: string): Promise { - if (this.agentCtx.agentId !== 'main') return; - if (!this.flags.enabled(TOWER_FLAG_ID)) return; - if (!isTowerFeatureAssembled(this.flags)) return; + async enter(base?: string): Promise { + if (this.agentCtx.agentId !== 'main') return { entered: false, reason: 'not-main-agent' }; + if (!this.flags.enabled(TOWER_FLAG_ID)) return { entered: false, reason: 'experiment-off' }; + if (!isTowerFeatureAssembled(this.flags)) return { entered: false, reason: 'feature-not-assembled' }; if (base !== undefined) { await this.prepareUserBase(base); } @@ -197,14 +200,17 @@ export class AgentTowerService extends Disposable implements IAgentTowerService if (base !== undefined && base !== this.agentState.get(towerBaseKey)) { this.dispatchEnter(base); } - return; + return { entered: true }; } const owner = await this.resolveTowerOwner(); if (owner !== undefined && owner !== this.sessionCtx.sessionId) { const ownerHandle = this.sessions.get(owner); if (ownerHandle !== undefined) { const activity = ownerHandle.accessor.get(ISessionActivityView).state(); - if (activity.busy || activity.pendingInteraction !== 'none') return; + if (activity.busy || activity.pendingInteraction !== 'none') { + const ownerTitle = await this.resolveOwnerTitle(ownerHandle); + return { entered: false, reason: 'owned-by-live-session', owner, ownerTitle }; + } ownerHandle.accessor .get(IAgentLifecycleService) .handleOf('main') @@ -215,6 +221,7 @@ export class AgentTowerService extends Disposable implements IAgentTowerService for (const name of TOWER_MODE_TOOLS) this.profile.addActiveTool(name); this.lastPublished = true; this.dispatchEnter(base); + return { entered: true }; } get requestedBase(): string | undefined { @@ -318,6 +325,15 @@ export class AgentTowerService extends Disposable implements IAgentTowerService return storeOwner ?? this.agentState.get(towerOwnerKey); } + private async resolveOwnerTitle(ownerHandle: ISessionScopeHandle): Promise { + try { + const meta = await ownerHandle.accessor.get(ISessionMetadata).read(); + return isUntitled(meta.title) ? undefined : meta.title; + } catch { + return undefined; + } + } + private async recordTowerAgentDeath(info: AgentTaskInfo): Promise { if (info.kind !== 'agent') return; if (info.agentId === undefined) return; diff --git a/packages/agent-core-v2/test/features/tower/tools/spawnTool.test.ts b/packages/agent-core-v2/test/features/tower/tools/spawnTool.test.ts index f4cdf4958f8..c6c56dd2dbd 100644 --- a/packages/agent-core-v2/test/features/tower/tools/spawnTool.test.ts +++ b/packages/agent-core-v2/test/features/tower/tools/spawnTool.test.ts @@ -132,7 +132,7 @@ describe('TowerSpawnTool', () => { get requestedBase() { return undefined; }, - enter: () => {}, + enter: () => Promise.resolve({ entered: true as const }), exit: () => {}, } as unknown as IAgentTowerService); ix.stub(ITowerRateLimitService, { diff --git a/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts b/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts index 196fe944e67..fe2e829d2c4 100644 --- a/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts +++ b/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts @@ -130,6 +130,7 @@ beforeEach(async () => { }, enter: () => { towerActive = true; + return Promise.resolve({ entered: true as const }); }, exit: () => { towerActive = false; diff --git a/packages/agent-core-v2/test/features/tower/towerFeature.test.ts b/packages/agent-core-v2/test/features/tower/towerFeature.test.ts index 8d540cbc854..c6032ea8de5 100644 --- a/packages/agent-core-v2/test/features/tower/towerFeature.test.ts +++ b/packages/agent-core-v2/test/features/tower/towerFeature.test.ts @@ -1,3 +1,7 @@ +import { mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; + +import { stringify as stringifyToml } from 'smol-toml'; import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { type CollectionToken, type CollectionView } from '#/_base/di/collection'; @@ -132,6 +136,89 @@ describe('TowerFeature — experimental flag gating', () => { }); }); +describe('TowerFeature — config-sourced flag assembly', () => { + let disposables: DisposableStore; + let homeDir: string; + + beforeEach(() => { + disposables = new DisposableStore(); + homeDir = `/tmp/kimi-code-tower-assembly-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`; + _clearScopedRegistryForTests(); + _clearFeatureRecipesForTests(); + registerScopedService( + LifecycleScope.App, + IFeatureManager, + FeatureManagerService, + ScopeActivation.OnScopeCreated, + 'feature', + ); + registerScopedService( + LifecycleScope.App, + IFeatureAssemblyService, + FeatureAssemblyService, + ScopeActivation.OnScopeCreated, + 'features', + ); + registerFeature(TowerFeature); + }); + afterEach(() => disposables.dispose()); + + async function makeRealFlags(preseed?: Record) { + const ix = disposables.add(new TestInstantiationService()); + ix.stub(IBootstrapService, stubBootstrap(homeDir)); + ix.stub(ILogService, stubLog()); + ix.stub(IFileSystemStorageService, new InMemoryStorageService()); + ix.set(IAtomicTomlDocumentStore, new SyncDescriptor(TomlAtomicDocumentStore)); + ix.set(IConfigRegistry, new SyncDescriptor(ConfigRegistry)); + ix.set(IConfigService, new SyncDescriptor(ConfigService)); + ix.set(IFlagRegistry, new SyncDescriptor(FlagRegistryService)); + ix.set(IFlagService, new SyncDescriptor(FlagService)); + if (preseed !== undefined) { + mkdirSync(homeDir, { recursive: true }); + writeFileSync(join(homeDir, 'config.toml'), `${stringifyToml(preseed)}\n`); + await ix.get(IAtomicTomlDocumentStore).set('', 'config.toml', preseed); + } + return { config: ix.get(IConfigService), flags: ix.get(IFlagService) }; + } + + it('assembles a config-sourced flag at startup', async () => { + const { flags } = await makeRealFlags({ experimental: { [TOWER_FLAG_ID]: true } }); + const host = createScopedTestHost([[IFlagService, flags]]); + + expect(isTowerFeatureAssembled(flags)).toBe(true); + expect(flags.explain(TOWER_FLAG_ID)).toMatchObject({ enabled: true, source: 'config' }); + const manager = host.app.accessor.get(IFeatureManager); + expect( + manager + .contributedServices() + .filter( + (entry) => entry.scope === LifecycleScope.App && entry.id === ITowerRateLimitService, + ), + ).toHaveLength(1); + const agent = host.child(LifecycleScope.Agent, 'agent-1'); + expect(collectionViewOf(agent, AgentToolContribution).items).toHaveLength(11); + host.dispose(); + }); + + it('does not assemble on a config flip after startup — a restart is required', async () => { + const { config, flags } = await makeRealFlags(); + const host = createScopedTestHost([[IFlagService, flags]]); + const manager = host.app.accessor.get(IFeatureManager); + expect(manager.units().map((unit) => unit.name)).toEqual(['tower']); + expect(isTowerFeatureAssembled(flags)).toBe(false); + + await config.set(EXPERIMENTAL_SECTION, { [TOWER_FLAG_ID]: true }); + await new Promise((resolve) => setTimeout(resolve, 0)); + + expect(flags.enabled(TOWER_FLAG_ID)).toBe(true); + expect(isTowerFeatureAssembled(flags)).toBe(false); + expect(manager.contributedServices()).toHaveLength(0); + const agent = host.child(LifecycleScope.Agent, 'agent-1'); + expect(collectionViewOf(agent, AgentToolContribution).items).toHaveLength(0); + host.dispose(); + }); +}); + describe('tower flag — resolution', () => { let disposables: DisposableStore; let homeDir: string; diff --git a/packages/agent-core-v2/test/features/tower/towerService.test.ts b/packages/agent-core-v2/test/features/tower/towerService.test.ts index b65efea3dea..6543f1b3bec 100644 --- a/packages/agent-core-v2/test/features/tower/towerService.test.ts +++ b/packages/agent-core-v2/test/features/tower/towerService.test.ts @@ -25,7 +25,12 @@ import type { ResolvedToolExecutionHookContext, } from '#/agent/toolExecutor/toolHooks'; import { TowerStore } from '#/features/tower/protocol/index'; -import { IAgentTowerService, TOWER_FLAG_ID } from '#/features/tower/tower'; +import { + IAgentTowerService, + TOWER_FLAG_ID, + towerEnterFailureMessage, + type TowerEnterFailure, +} from '#/features/tower/tower'; import { _setTowerFeatureAssembledForTests } from '#/features/tower/towerFeature'; import { AgentTowerService, TOWER_MODE_TOOLS } from '#/features/tower/towerService'; import { towerKey } from '#/features/tower/towerOps'; @@ -44,6 +49,7 @@ import { ISessionActivityView, type SessionPendingInteraction, } from '#/session/sessionActivity/sessionActivity'; +import { ISessionMetadata } from '#/session/sessionMetadata/sessionMetadata'; import type { ToolCall } from '#/kosong/contract/message'; import { AppendLogStore } from '#/persistence/backends/node-fs/appendLogStore'; import { InMemoryStorageService } from '#/persistence/backends/memory/inMemoryStorageService'; @@ -138,7 +144,7 @@ describe('AgentTowerService', () => { let addedTools: string[]; let removedTools: string[]; let activeTools: string[] | undefined; - let liveSessions: Map void> }>; + let liveSessions: Map void>; title?: string; metadataReadFails?: boolean }>; let fireUnitsChanged: () => void = () => {}; beforeEach(() => { @@ -183,6 +189,14 @@ describe('AgentTowerService', () => { }), }; } + if (token === (ISessionMetadata as unknown)) { + return { + read: async () => { + if (stub.metadataReadFails === true) throw new Error('metadata read failed'); + return { title: stub.title }; + }, + }; + } return undefined; }, }, @@ -251,7 +265,7 @@ describe('AgentTowerService', () => { ); expect(tower.isActive).toBe(false); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ entered: true }); expect(tower.isActive).toBe(true); tower.exit(); expect(tower.isActive).toBe(false); @@ -856,7 +870,7 @@ describe('AgentTowerService', () => { expect(formatDenyMessage).toHaveBeenCalledTimes(1); }); - it('enter() is a no-op while the tower flag is off', async () => { + it('enter() reports experiment-off while the tower flag is off', async () => { towerFlagOn = false; const tower = ix.get(IAgentTowerService); const events: { readonly type: string }[] = []; @@ -866,18 +880,18 @@ describe('AgentTowerService', () => { }), ); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ entered: false, reason: 'experiment-off' }); expect(tower.isActive).toBe(false); expect(events).toEqual([]); }); - it('enter() is a no-op until the feature is assembled — a live flag flip needs a restart', async () => { + it('enter() reports feature-not-assembled until the feature is assembled — a live flag flip needs a restart', async () => { _setTowerFeatureAssembledForTests(false); try { const tower = ix.get(IAgentTowerService); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ entered: false, reason: 'feature-not-assembled' }); expect(tower.isActive).toBe(false); expect(addedTools).toEqual([]); @@ -945,18 +959,20 @@ describe('AgentTowerService', () => { function stubLiveSession( id: string, - init: { busy?: boolean; pendingInteraction?: SessionPendingInteraction } = {}, + init: { busy?: boolean; pendingInteraction?: SessionPendingInteraction; title?: string; metadataReadFails?: boolean } = {}, ): Mock<() => void> { const exit = vi.fn(); liveSessions.set(id, { busy: init.busy ?? false, pendingInteraction: init.pendingInteraction ?? 'none', exit, + title: init.title, + metadataReadFails: init.metadataReadFails, }); return exit; } - it('enter() is a no-op while a busy foreign session owns the tower in this process', async () => { + it('enter() reports owned-by-live-session with the owner id while a busy foreign session owns the tower in this process', async () => { const repo = await mkdtemp(join(tmpdir(), 'tower-enter-foreign-')); try { await initGitRepo(repo); @@ -969,7 +985,11 @@ describe('AgentTowerService', () => { ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); const tower = ix.get(IAgentTowerService); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + }); expect(tower.isActive).toBe(false); expect(addedTools).toEqual([]); @@ -978,7 +998,7 @@ describe('AgentTowerService', () => { } }); - it('enter() is a no-op while the owning session waits on an interaction', async () => { + it('enter() reports owned-by-live-session while the owning session waits on an interaction', async () => { const repo = await mkdtemp(join(tmpdir(), 'tower-enter-pending-')); try { await initGitRepo(repo); @@ -991,7 +1011,11 @@ describe('AgentTowerService', () => { ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); const tower = ix.get(IAgentTowerService); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + }); expect(tower.isActive).toBe(false); expect(addedTools).toEqual([]); @@ -1000,6 +1024,99 @@ describe('AgentTowerService', () => { } }); + it('enter() includes the live owner session title in the owned-by-live-session result', async () => { + const repo = await mkdtemp(join(tmpdir(), 'tower-enter-owner-title-')); + try { + await initGitRepo(repo); + await writeFile(join(repo, 'README.md'), '# fixture\n'); + await execFileAsync('git', ['add', 'README.md'], { cwd: repo }); + await execFileAsync('git', ['commit', '-m', 'initial'], { cwd: repo }); + await new TowerStore(repo).init('session-original'); + + stubLiveSession('session-original', { busy: true, title: 'Tower docs polish' }); + ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + await expect(tower.enter()).resolves.toEqual({ + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + ownerTitle: 'Tower docs polish', + }); + + expect(tower.isActive).toBe(false); + expect(addedTools).toEqual([]); + } finally { + await rm(repo, { recursive: true, force: true }); + } + }); + + it('enter() degrades to the owner id when the live owner has only a placeholder title', async () => { + const repo = await mkdtemp(join(tmpdir(), 'tower-enter-owner-untitled-')); + try { + await initGitRepo(repo); + await writeFile(join(repo, 'README.md'), '# fixture\n'); + await execFileAsync('git', ['add', 'README.md'], { cwd: repo }); + await execFileAsync('git', ['commit', '-m', 'initial'], { cwd: repo }); + await new TowerStore(repo).init('session-original'); + + stubLiveSession('session-original', { busy: true, title: 'New Session' }); + ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + const result = await tower.enter(); + + expect(result).toEqual({ + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + ownerTitle: undefined, + }); + if (!result.entered) { + expect(towerEnterFailureMessage(result)).toBe( + 'another live session owns the workspace tower (session session-original)', + ); + } + expect(tower.isActive).toBe(false); + expect(addedTools).toEqual([]); + } finally { + await rm(repo, { recursive: true, force: true }); + } + }); + + it('enter() degrades to the owner id when the live owner metadata cannot be read', async () => { + const repo = await mkdtemp(join(tmpdir(), 'tower-enter-owner-unreadable-')); + try { + await initGitRepo(repo); + await writeFile(join(repo, 'README.md'), '# fixture\n'); + await execFileAsync('git', ['add', 'README.md'], { cwd: repo }); + await execFileAsync('git', ['commit', '-m', 'initial'], { cwd: repo }); + await new TowerStore(repo).init('session-original'); + + stubLiveSession('session-original', { busy: true, metadataReadFails: true }); + ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + const result = await tower.enter(); + + expect(result).toEqual({ + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + ownerTitle: undefined, + }); + if (!result.entered) { + expect(towerEnterFailureMessage(result)).toBe( + 'another live session owns the workspace tower (session session-original)', + ); + } + expect(tower.isActive).toBe(false); + expect(addedTools).toEqual([]); + } finally { + await rm(repo, { recursive: true, force: true }); + } + }); + it('enter() takes the tower over from a live but idle owner session', async () => { const repo = await mkdtemp(join(tmpdir(), 'tower-enter-takeover-')); try { @@ -1069,14 +1186,14 @@ describe('AgentTowerService', () => { expect(removedTools).toEqual([]); }); - it('enter is inert on a non-main agent', async () => { + it('enter reports not-main-agent and is inert on a non-main agent', async () => { ix.stub( IAgentScopeContext, makeAgentScopeContext({ agentId: 'test-agent', agentScope: testWireScope('wire', 'tower-test'), generation: 0 }), ); const tower = ix.get(IAgentTowerService); - await tower.enter(); + await expect(tower.enter()).resolves.toEqual({ entered: false, reason: 'not-main-agent' }); expect(tower.isActive).toBe(false); expect(addedTools).toEqual([]); @@ -2059,3 +2176,35 @@ describe('TowerModeInjection', () => { expect(lastTowerReminder(context)).toContain('Tower mode is active'); }); }); + +describe('towerEnterFailureMessage', () => { + it.each([ + [ + { entered: false, reason: 'not-main-agent' }, + 'tower mode is only supported by the main agent', + ], + [ + { entered: false, reason: 'experiment-off' }, + 'the tower experiment is disabled; enable it with KIMI_CODE_EXPERIMENTAL_TOWER=1 or `[experimental] tower = true` in config.toml', + ], + [ + { entered: false, reason: 'feature-not-assembled' }, + 'the tower feature is not assembled in this process; a restart is required', + ], + [ + { entered: false, reason: 'owned-by-live-session', owner: 'session-original' }, + 'another live session owns the workspace tower (session session-original)', + ], + [ + { + entered: false, + reason: 'owned-by-live-session', + owner: 'session-original', + ownerTitle: 'Tower docs polish', + }, + 'another live session owns the workspace tower (session Tower docs polish (session-original))', + ], + ] as [TowerEnterFailure, string][])('maps %o to its message', (failure, message) => { + expect(towerEnterFailureMessage(failure)).toBe(message); + }); +}); diff --git a/packages/kap-server/src/routes/sessionAgentConfig.ts b/packages/kap-server/src/routes/sessionAgentConfig.ts index f104609dd66..bcf1915b4c2 100644 --- a/packages/kap-server/src/routes/sessionAgentConfig.ts +++ b/packages/kap-server/src/routes/sessionAgentConfig.ts @@ -8,6 +8,7 @@ import { IAgentSwarmService, IAgentTowerService, resumeSessionById, + towerEnterFailureMessage, type PermissionMode, type Scope, } from '@moonshot-ai/agent-core-v2'; @@ -56,11 +57,11 @@ export async function applySessionAgentConfig( if (agentConfig.tower_mode !== undefined) { const tower = agent.accessor.get(IAgentTowerService); if (agentConfig.tower_mode) { - await tower.enter(agentConfig.tower_base); - if (!tower.isActive) { + const result = await tower.enter(agentConfig.tower_base); + if (!result.entered) { throw new Error2( ErrorCodes.SESSION_TOWER_MODE_INVALID, - 'tower mode could not be enabled — another live session owns the workspace tower', + towerEnterFailureMessage(result), ); } } else { diff --git a/packages/kap-server/test/sessions.test.ts b/packages/kap-server/test/sessions.test.ts index 82e4dd86148..96355f7502f 100644 --- a/packages/kap-server/test/sessions.test.ts +++ b/packages/kap-server/test/sessions.test.ts @@ -760,8 +760,9 @@ describe('server-v2 /api/v1/sessions', () => { const on = await postJson(`/api/v1/sessions/${id}/profile`, { agent_config: { tower_mode: true }, }); - expect(on.body.code).not.toBe(0); - expect(on.body.msg).toContain('tower mode could not be enabled'); + expect(on.body.code).toBe(50001); + expect(on.body.msg).toContain('the tower experiment is disabled'); + expect(on.body.msg).toContain('KIMI_CODE_EXPERIMENTAL_TOWER=1'); const after = await getJson<{ tower_mode?: boolean; }>(`/api/v1/sessions/${id}/status`); diff --git a/packages/node-sdk/src/sdk-rpc-client-v2.ts b/packages/node-sdk/src/sdk-rpc-client-v2.ts index 9da7370515a..5008cb71925 100644 --- a/packages/node-sdk/src/sdk-rpc-client-v2.ts +++ b/packages/node-sdk/src/sdk-rpc-client-v2.ts @@ -231,6 +231,7 @@ import { resolveLoggingConfig, resolvePrintBackgroundMode, summarizeSkill, + towerEnterFailureMessage, type IAgentScopeHandle, type IDisposable, type ISessionScopeHandle, @@ -2094,11 +2095,11 @@ export class SDKRpcClientV2 extends SDKRpcClientBase { const agent = await this.agentScope(input.sessionId); const tower = agent.accessor.get(IAgentTowerService); if (input.enabled) { - await tower.enter(input.base); - if (!tower.isActive) { + const result = await tower.enter(input.base); + if (!result.entered) { throw new V2Error2( V2ErrorCodes.SESSION_TOWER_MODE_INVALID, - 'tower mode could not be enabled — another live session owns the workspace tower', + towerEnterFailureMessage(result), ); } } else { diff --git a/packages/node-sdk/test/sdk-rpc-client-v2.test.ts b/packages/node-sdk/test/sdk-rpc-client-v2.test.ts index 5693ea3a5ad..7cfc67b62df 100644 --- a/packages/node-sdk/test/sdk-rpc-client-v2.test.ts +++ b/packages/node-sdk/test/sdk-rpc-client-v2.test.ts @@ -1055,8 +1055,8 @@ key = "${titleOAuthRef.key}" }; await client.setTowerMode({ sessionId: 'ses_tower', enabled: true }); - // The tower feature is flag-gated engine-side, so enter() may be a - // no-op; the wire must always mirror the engine truth. + // A refused enter() rejects with a typed reason, so a resolved call + // means the engine activated tower mode; the wire mirrors it. expect((await client.getStatus({ sessionId: 'ses_tower' })).towerMode).toBe( mainTower().isActive, ); @@ -1075,6 +1075,7 @@ key = "${titleOAuthRef.key}" }); it('rejects setTowerMode when the tower feature is unavailable', async () => { + vi.stubEnv('KIMI_CODE_EXPERIMENTAL_TOWER', '0'); vi.stubEnv('KIMI_CODE_EXPERIMENTAL_FLAG', '0'); const homeDir = await mkdtemp(join(tmpdir(), 'kimi-sdk-v2-')); tempDirs.push(homeDir); @@ -1085,7 +1086,10 @@ key = "${titleOAuthRef.key}" await client.createSession({ id: 'ses_tower_off', workDir }); await expect(client.setTowerMode({ sessionId: 'ses_tower_off', enabled: true })) - .rejects.toMatchObject({ code: 'session.tower_mode_invalid' }); + .rejects.toMatchObject({ + code: 'session.tower_mode_invalid', + message: expect.stringContaining('the tower experiment is disabled'), + }); expect((await client.getStatus({ sessionId: 'ses_tower_off' })).towerMode).toBe(false); await client.setTowerMode({ sessionId: 'ses_tower_off', enabled: false }); From 214d8a67482396f137bf47a3d1c8a3c8b9af5981 Mon Sep 17 00:00:00 2001 From: konghuanjun Date: Wed, 2 Sep 2026 18:49:55 +0800 Subject: [PATCH 2/2] fix(tower): release handler when teardown --- .changeset/tower-mode-fixes.md | 2 +- .../injection/tower-mode-full-reminder.md | 5 +- .../src/features/tower/protocol/git.ts | 27 +++- .../src/features/tower/protocol/store.ts | 39 ++++- .../src/features/tower/towerService.ts | 18 ++- .../test/features/tower/store.test.ts | 86 +++++++++++ .../features/tower/tools/towerTools.test.ts | 34 +++++ .../test/features/tower/towerService.test.ts | 136 ++++++++++++++++-- 8 files changed, 326 insertions(+), 21 deletions(-) diff --git a/.changeset/tower-mode-fixes.md b/.changeset/tower-mode-fixes.md index 68f56cacf5f..62e5e5c1beb 100644 --- a/.changeset/tower-mode-fixes.md +++ b/.changeset/tower-mode-fixes.md @@ -2,4 +2,4 @@ "@moonshot-ai/kimi-code": patch --- -Tower mode (experimental, `KIMI_CODE_EXPERIMENTAL_TOWER=1`): fix tower mode never starting when enabled through `[experimental] tower = true` in `config.toml` instead of the environment variable. When tower mode cannot be enabled, the error now names the actual blocker — the disabled experiment, a required restart, or the owning session. When another live session owns the workspace tower, the message also names the owning session's title alongside its id. +Tower mode (experimental, `KIMI_CODE_EXPERIMENTAL_TOWER=1`): fix tower mode never starting when enabled through `[experimental] tower = true` in `config.toml` instead of the environment variable. When tower mode cannot be enabled, the error now names the actual blocker — the disabled experiment, a required restart, or the owning session. When another live session owns the workspace tower, the message also names the owning session's title alongside its id. /tower now also works in a directory that is not a git repository — it runs git init and commits what is there (an empty initial commit for empty directories). diff --git a/packages/agent-core-v2/src/features/tower/injection/tower-mode-full-reminder.md b/packages/agent-core-v2/src/features/tower/injection/tower-mode-full-reminder.md index 0c02d356f74..896bbb650e5 100644 --- a/packages/agent-core-v2/src/features/tower/injection/tower-mode-full-reminder.md +++ b/packages/agent-core-v2/src/features/tower/injection/tower-mode-full-reminder.md @@ -16,10 +16,7 @@ Working principles: ## Prepare (only when the directory is not a tower-ready git repo) -`TowerInit` requires a git repository with at least one commit. If `git rev-parse --is-inside-work-tree` fails: - -- **Empty directory** → `git init` + `git commit --allow-empty -m "tower: init"`, then proceed. No confirmation needed. -- **Non-empty directory** → never `git add -A`: a blind initial commit can seal secrets, large binaries, or dependency directories into history irreversibly. Survey the directory (file count, largest files, secret-looking names like `.env` or `*.pem`), present the summary, and ask the human **exactly once** whether to initialize and commit the existing files — but only when asking is possible. Under auto permission mode `AskUserQuestion` is disabled: do not call it into a deny error. Default to the safe behavior instead — do NOT commit existing files; stop tower there and tell the human in your reply the two commands to run themselves (`git init` plus an initial commit of their choosing). If they agree to the commit, write a conservative `.gitignore` (dependencies, build output, secrets), show the staged list, commit, proceed. +`TowerInit` requires a git repository with at least one commit. If the session working directory is not inside one, the engine bootstraps it for you: `git init`, then an initial commit on the base branch — an empty directory gets `git commit --allow-empty -m "tower: init"`; a non-empty directory gets every present file committed as a dirty-base snapshot (`tower: snapshot of uncommitted base checkout changes (base )` — the same semantics as starting a tower over an uncommitted checkout). If the directory holds secrets or large files that must not enter history, move them out or add a `.gitignore` BEFORE starting the tower — the snapshot commits everything present. ## Tower workflow diff --git a/packages/agent-core-v2/src/features/tower/protocol/git.ts b/packages/agent-core-v2/src/features/tower/protocol/git.ts index 665f9e0a5c4..0fe3ddda838 100644 --- a/packages/agent-core-v2/src/features/tower/protocol/git.ts +++ b/packages/agent-core-v2/src/features/tower/protocol/git.ts @@ -76,10 +76,35 @@ export async function branchExists(cwd: string, branch: string): Promise { + await git(cwd, ['init']); +} + +async function gitCommit(cwd: string, args: readonly string[]): Promise { + try { + await git(cwd, args); + } catch (error) { + if (!(error instanceof GitError) || !/identity unknown/.test(error.stderr)) { + throw error; + } + await git(cwd, [ + '-c', + 'user.name=Kimi Tower', + '-c', + 'user.email=kimi-tower@localhost', + ...args, + ]); + } +} + export async function checkoutNewLocalBranch(cwd: string, branch: string): Promise { await git(cwd, ['checkout', '-b', branch]); } +export async function commitAllowEmpty(cwd: string, message: string): Promise { + await gitCommit(cwd, ['commit', '--allow-empty', '-m', message]); +} + export async function commitPaths( cwd: string, paths: readonly string[], @@ -88,7 +113,7 @@ export async function commitPaths( for (let i = 0; i < paths.length; i += ADD_PATHS_CHUNK) { await git(cwd, ['add', '-A', '--', ...paths.slice(i, i + ADD_PATHS_CHUNK)]); } - await git(cwd, ['commit', '-m', message]); + await gitCommit(cwd, ['commit', '-m', message]); } export async function isAncestor(cwd: string, ancestor: string, ref: string): Promise { diff --git a/packages/agent-core-v2/src/features/tower/protocol/store.ts b/packages/agent-core-v2/src/features/tower/protocol/store.ts index e196933364a..8825d49eaa5 100644 --- a/packages/agent-core-v2/src/features/tower/protocol/store.ts +++ b/packages/agent-core-v2/src/features/tower/protocol/store.ts @@ -9,9 +9,13 @@ import { parseFrontmatter, renderFrontmatter } from './frontmatter'; import { branchExists, branchTip, + checkoutNewLocalBranch, + commitAllowEmpty, + commitPaths, currentBranch, diffNameOnly, hasAnyCommit, + initRepository, isAncestor, isInsideRepo, isWorktreeDirty, @@ -155,12 +159,28 @@ export class TowerStore { } } - async init(sessionId?: string, base?: string): Promise { - if (!(await isInsideRepo(this.repoRoot))) { - throw new TowerProtocolError( - 'tower needs a git repository (the session working directory is not inside one)', - ); + async ensureRepository(base?: string): Promise { + if (await isInsideRepo(this.repoRoot)) return; + await initRepository(this.repoRoot); + const unborn = (await tryGit(this.repoRoot, ['symbolic-ref', '--short', 'HEAD'])) ?? 'main'; + const resolvedBase = base ?? unborn; + if (resolvedBase !== unborn) { + await checkoutNewLocalBranch(this.repoRoot, resolvedBase); } + const dirty = await listBaseDirtyEntries(this.repoRoot); + if (dirty.length === 0) { + await commitAllowEmpty(this.repoRoot, 'tower: init'); + return; + } + await commitPaths( + this.repoRoot, + dirty.map((entry) => entry.path), + `tower: snapshot of uncommitted base checkout changes (base ${resolvedBase})`, + ); + } + + async init(sessionId?: string, base?: string): Promise { + await this.ensureRepository(base); if (!(await hasAnyCommit(this.repoRoot))) { throw new TowerProtocolError( 'the repository has no commits yet — create an initial commit first', @@ -255,6 +275,15 @@ export class TowerStore { return stale.map((agent) => agent.name); } + async release(sessionId: string): Promise { + if (!(await this.isInitialized())) return; + const state = await this.load(); + if (state.sessionId !== sessionId) return; + state.sessionId = undefined; + await this.save(state); + await this.appendLog(TOWER_NAME, 'release', { session: sessionId }); + } + private async ensureGitExclude(): Promise { const gitDir = (await readGitDir(this.repoRoot)) ?? join(this.repoRoot, '.git'); const excludePath = join(gitDir, 'info', 'exclude'); diff --git a/packages/agent-core-v2/src/features/tower/towerService.ts b/packages/agent-core-v2/src/features/tower/towerService.ts index 6c771d09ff2..c2b7834e435 100644 --- a/packages/agent-core-v2/src/features/tower/towerService.ts +++ b/packages/agent-core-v2/src/features/tower/towerService.ts @@ -2,6 +2,7 @@ import { join } from 'node:path'; import { Disposable } from '#/_base/di/lifecycle'; import { ScopeActivation, registerScopedService, type ISessionScopeHandle } from '#/_base/di/scope'; +import { ILogService } from '#/_base/log/log'; import { IAgentReminderService } from '#/features/reminder/reminderService'; import { IAgentLifecycleService } from '#/session/agentLifecycle/agentLifecycle'; import { IAgentContextMemoryService } from '#/agent/contextMemory/contextMemory'; @@ -70,6 +71,7 @@ export class AgentTowerService extends Disposable implements IAgentTowerService @IAgentReminderService reminder: IAgentReminderService, @IAgentContextMemoryService context: IAgentContextMemoryService, @IEventBus eventBus: IEventBus, + @ILogService private readonly log: ILogService, ) { super(); this.agentState.contributeState(towerKey); @@ -231,6 +233,7 @@ export class AgentTowerService extends Disposable implements IAgentTowerService private async prepareUserBase(base: string): Promise { const repoRoot = resolveTowerRepoRoot(this.sessionCtx.cwd); const store = new TowerStore(repoRoot); + await store.ensureRepository(base); if (await store.isInitialized()) { const state = await store.load(); if (state.base === base) { @@ -296,6 +299,19 @@ export class AgentTowerService extends Disposable implements IAgentTowerService if (!this.agentState.get(towerKey)) return; this.lastPublished = false; void this.dispatcher.dispatch(new TowerModeExit({ agentId: this.agentCtx.agentId })); + void this.releaseTowerOwnership(); + } + + private async releaseTowerOwnership(): Promise { + const store = new TowerStore(resolveTowerRepoRoot(this.sessionCtx.cwd)); + await store.release(this.sessionCtx.sessionId).then( + () => undefined, + (error: unknown) => { + this.log.warn( + `failed to release tower workspace ownership: ${error instanceof Error ? error.message : String(error)}`, + ); + }, + ); } get isActive(): boolean { @@ -313,7 +329,7 @@ export class AgentTowerService extends Disposable implements IAgentTowerService const owner = await this.resolveTowerOwner(); if (owner === undefined || owner === this.sessionCtx.sessionId) return; if (this.sessions.get(owner) === undefined) return; - void this.dispatcher.dispatch(new TowerModeExit({ agentId: this.agentCtx.agentId })); + this.exit(); } private async resolveTowerOwner(): Promise { diff --git a/packages/agent-core-v2/test/features/tower/store.test.ts b/packages/agent-core-v2/test/features/tower/store.test.ts index 9b87d96e217..3f88083b766 100644 --- a/packages/agent-core-v2/test/features/tower/store.test.ts +++ b/packages/agent-core-v2/test/features/tower/store.test.ts @@ -90,6 +90,40 @@ async function cleanReview(reviewer: string, target: string): Promise { }); } +describe('init in a non-git directory', () => { + it('bootstraps an empty directory with git init and an empty initial commit', async () => { + const dir = await mkdtemp(join(tmpdir(), 'tower-store-nogit-empty-')); + try { + const result = await new TowerStore(dir).init('session-a'); + const branch = await git(dir, 'symbolic-ref', '--short', 'HEAD'); + expect(result).toMatchObject({ base: branch, created: true, checkout: branch }); + expect(await git(dir, 'rev-list', '--count', 'HEAD')).toBe('1'); + expect(await git(dir, 'log', '-1', '--format=%s')).toBe('tower: init'); + expect(await git(dir, 'status', '--porcelain')).toBe(''); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + + it('snapshots the existing files of a non-empty directory onto the requested base', async () => { + const dir = await mkdtemp(join(tmpdir(), 'tower-store-nogit-dirty-')); + try { + await mkdir(join(dir, 'src'), { recursive: true }); + await writeFile(join(dir, 'src', 'app.ts'), 'export {}\n'); + await writeFile(join(dir, 'README.md'), '# scratch\n'); + const result = await new TowerStore(dir).init('session-a', 'tower-base'); + expect(result).toMatchObject({ base: 'tower-base', created: true, checkout: 'tower-base' }); + expect(await git(dir, 'log', '-1', '--format=%s')).toBe( + 'tower: snapshot of uncommitted base checkout changes (base tower-base)', + ); + expect(await git(dir, 'ls-files')).toContain('src/app.ts'); + expect(await git(dir, 'status', '--porcelain')).toBe(''); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); +}); + describe('init', () => { it('creates the directory skeleton, state.json, and the git exclude entry', async () => { const result = await store.init(); @@ -243,6 +277,58 @@ describe('init', () => { }); }); +describe('release', () => { + it('clears the recorded owner and logs the release when the session matches', async () => { + await store.init('session-a'); + + await store.release('session-a'); + + const state = await store.load(); + expect(state.sessionId).toBeUndefined(); + const log = await store.recentLog(5); + expect(log.some((line) => line.includes(' release ') && line.includes('session=session-a'))).toBe(true); + }); + + it('keeps the recorded owner for a different session and logs nothing', async () => { + await store.init('session-a'); + + await store.release('session-b'); + + const state = await store.load(); + expect(state.sessionId).toBe('session-a'); + const log = await store.recentLog(5); + expect(log.some((line) => line.includes(' release '))).toBe(false); + }); + + it('is a no-op while the workspace is not initialized', async () => { + await store.release('session-a'); + + expect(await store.isInitialized()).toBe(false); + }); + + it('is idempotent once ownership is already released', async () => { + await store.init('session-a'); + await store.release('session-a'); + + await store.release('session-a'); + + expect((await store.load()).sessionId).toBeUndefined(); + const log = await store.recentLog(10); + expect(log.filter((line) => line.includes(' release '))).toHaveLength(1); + }); + + it('lets another session adopt the workspace after the owner released it', async () => { + await store.init('session-a'); + await store.release('session-a'); + + const result = await store.init('session-b'); + + expect(result.created).toBe(false); + expect(result.retiredAgents).toEqual([]); + expect((await store.load()).sessionId).toBe('session-b'); + }); +}); + describe('markAgentDied', () => { it('marks the roster entry and appends an activity log line', async () => { await store.init('session-a'); diff --git a/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts b/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts index fe2e829d2c4..e8a66627c86 100644 --- a/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts +++ b/packages/agent-core-v2/test/features/tower/tools/towerTools.test.ts @@ -299,6 +299,24 @@ describe('TowerInitTool', () => { const state = await new TowerStore(repo).load(); expect(state.sessionId).toBe('session-test'); }); + + it('adopts once the owning session released ownership, even while it is still live', async () => { + await initViaTool(); + liveSessionIds = ['session-test']; + currentSessionId = 'session-next'; + + const blocked = await run(ix.get(ITowerInitTool), {}); + expect(blocked.isError).toBe(true); + expect(blocked.output).toContain('owned by a live session (session-test)'); + + await new TowerStore(repo).release('session-test'); + + const adopted = await run(ix.get(ITowerInitTool), {}); + expect(adopted.isError).toBeFalsy(); + expect(adopted.output).toContain('tower workspace already initialized'); + const state = await new TowerStore(repo).load(); + expect(state.sessionId).toBe('session-next'); + }); }); describe('TowerPlanTool', () => { @@ -351,6 +369,22 @@ describe('TowerTeardownTool', () => { expect(result.output).toContain('dismantle that session'); expect((await new TowerStore(repo).load()).sessionId).toBe('session-test'); }); + + it('tears down once the owning session released ownership, even while it is still live', async () => { + await initViaTool(); + liveSessionIds = ['session-test']; + currentSessionId = 'session-next'; + + const blocked = await run(ix.get(ITowerTeardownTool), {}); + expect(blocked.isError).toBe(true); + expect(blocked.output).toContain('dismantle that session'); + + await new TowerStore(repo).release('session-test'); + + const result = await run(ix.get(ITowerTeardownTool), {}); + expect(result.isError).toBeFalsy(); + expect(result.output).toContain('tower teardown:'); + }); }); describe('TowerSendTool + TowerInboxTool', () => { diff --git a/packages/agent-core-v2/test/features/tower/towerService.test.ts b/packages/agent-core-v2/test/features/tower/towerService.test.ts index 6543f1b3bec..35b15446c18 100644 --- a/packages/agent-core-v2/test/features/tower/towerService.test.ts +++ b/packages/agent-core-v2/test/features/tower/towerService.test.ts @@ -9,6 +9,7 @@ import { afterEach, beforeEach, describe, expect, it, vi, type Mock } from 'vite import { SyncDescriptor } from '#/_base/di/descriptors'; import { DisposableStore } from '#/_base/di/lifecycle'; import { TestInstantiationService } from '#/_base/di/test'; +import { ILogService } from '#/_base/log/log'; import { IAgentLifecycleService } from '#/session/agentLifecycle/agentLifecycle'; import { IAgentReminderService } from '#/features/reminder/reminderService'; import { createReminderStub } from '../reminder/stubs'; @@ -61,6 +62,7 @@ import { AGENT_WIRE_RECORD_KEY, type WireRecord } from '#/wire/record'; import { stubToolExecutorEvents, type ToolExecutorEventStubs } from '../../agent/toolExecutor/stubs'; import { stubFlag } from '../../app/flag/stubs'; +import { stubLog } from '../../_base/log/stubs'; import { appService, createTestAgent, @@ -228,6 +230,7 @@ describe('AgentTowerService', () => { IAgentReminderService, createReminderStub(), ); + ix.stub(ILogService, stubLog()); ix.stub(IAgentContextMemoryService, { get: () => [], } as unknown as IAgentContextMemoryService); @@ -1051,6 +1054,34 @@ describe('AgentTowerService', () => { } }); + it('enter() bootstraps a non-git directory before preparing the requested base', async () => { + const dir = await mkdtemp(join(tmpdir(), 'tower-enter-nogit-')); + try { + await writeFile(join(dir, 'notes.md'), '# scratch\n'); + ix.stub(ISessionContext, { cwd: dir, sessionId: 'session-fresh' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + const result = await tower.enter('tower-base'); + + expect(result).toEqual({ entered: true }); + expect(tower.isActive).toBe(true); + const { stdout: subject } = await execFileAsync('git', ['log', '-1', '--format=%s'], { + cwd: dir, + }); + expect(subject.trim()).toBe( + 'tower: snapshot of uncommitted base checkout changes (base tower-base)', + ); + const { stdout: branch } = await execFileAsync('git', ['symbolic-ref', '--short', 'HEAD'], { + cwd: dir, + }); + expect(branch.trim()).toBe('tower-base'); + const { stdout: tracked } = await execFileAsync('git', ['ls-files'], { cwd: dir }); + expect(tracked).toContain('notes.md'); + } finally { + await rm(dir, { recursive: true, force: true }); + } + }); + it('enter() degrades to the owner id when the live owner has only a placeholder title', async () => { const repo = await mkdtemp(join(tmpdir(), 'tower-enter-owner-untitled-')); try { @@ -1161,6 +1192,74 @@ describe('AgentTowerService', () => { } }); + it('exit() releases workspace ownership recorded under this session', async () => { + const repo = await mkdtemp(join(tmpdir(), 'tower-exit-release-')); + try { + await initGitRepo(repo); + await writeFile(join(repo, 'README.md'), '# fixture\n'); + await execFileAsync('git', ['add', 'README.md'], { cwd: repo }); + await execFileAsync('git', ['commit', '-m', 'initial'], { cwd: repo }); + const store = new TowerStore(repo); + await store.init('session-main'); + ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-main' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + await tower.enter(); + tower.exit(); + + expect(tower.isActive).toBe(false); + await vi.waitFor(async () => { + expect((await store.load()).sessionId).toBeUndefined(); + }); + const log = await store.recentLog(5); + expect(log.some((line) => line.includes(' release ') && line.includes('session=session-main'))).toBe(true); + } finally { + await rm(repo, { recursive: true, force: true }); + } + }); + + it('exit() keeps workspace ownership recorded under another session', async () => { + const repo = await mkdtemp(join(tmpdir(), 'tower-exit-foreign-')); + try { + await initGitRepo(repo); + await writeFile(join(repo, 'README.md'), '# fixture\n'); + await execFileAsync('git', ['add', 'README.md'], { cwd: repo }); + await execFileAsync('git', ['commit', '-m', 'initial'], { cwd: repo }); + const store = new TowerStore(repo); + await store.init('session-original'); + ix.stub(ISessionContext, { cwd: repo, sessionId: 'session-fork' } as unknown as ISessionContext); + const tower = ix.get(IAgentTowerService); + + await tower.enter(); + expect(tower.isActive).toBe(true); + + let releaseSettled = Promise.resolve(); + const originalRelease = TowerStore.prototype.release; + const releaseSpy = vi + .spyOn(TowerStore.prototype, 'release') + .mockImplementation(function (this: TowerStore, sessionId) { + const pending = originalRelease.call(this, sessionId); + releaseSettled = pending.then( + () => undefined, + () => undefined, + ); + return pending; + }); + try { + tower.exit(); + + await vi.waitFor(() => expect(releaseSpy).toHaveBeenCalledWith('session-fork')); + await releaseSettled; + expect(tower.isActive).toBe(false); + expect((await store.load()).sessionId).toBe('session-original'); + } finally { + releaseSpy.mockRestore(); + } + } finally { + await rm(repo, { recursive: true, force: true }); + } + }); + it('does not veto TodoList while the tower flag is off, even with tower mode persisted active', async () => { const tower = ix.get(IAgentTowerService); await tower.enter(); @@ -1501,16 +1600,35 @@ describe('AgentTowerService', () => { ); const restored = ix2.get(IAgentTowerService); - await restoreTestEventDispatcher( - dispatcher, - ix2.get(IAppendLogStore), - testWireScope('wire', 'tower-fork-restore'), - records, - ); + let releaseSettled = Promise.resolve(); + const originalRelease = TowerStore.prototype.release; + const releaseSpy = vi + .spyOn(TowerStore.prototype, 'release') + .mockImplementation(function (this: TowerStore, sessionId) { + const pending = originalRelease.call(this, sessionId); + releaseSettled = pending.then( + () => undefined, + () => undefined, + ); + return pending; + }); + try { + await restoreTestEventDispatcher( + dispatcher, + ix2.get(IAppendLogStore), + testWireScope('wire', 'tower-fork-restore'), + records, + ); - expect(restored.isActive).toBe(false); - expect(restoredAdded).toEqual([]); - expect(events).toContainEqual({ type: 'agent.status.updated', towerMode: false }); + expect(restored.isActive).toBe(false); + expect(restoredAdded).toEqual([]); + expect(events).toContainEqual({ type: 'agent.status.updated', towerMode: false }); + await vi.waitFor(() => expect(releaseSpy).toHaveBeenCalledWith('session-fork')); + await releaseSettled; + expect((await new TowerStore(repo).load()).sessionId).toBe('session-original'); + } finally { + releaseSpy.mockRestore(); + } } finally { await rm(repo, { recursive: true, force: true }); }