diff --git a/docs/design/session-writer-lease-p0a.md b/docs/design/session-writer-lease-p0a.md index 4a5f0079a5e..6f595f2efba 100644 --- a/docs/design/session-writer-lease-p0a.md +++ b/docs/design/session-writer-lease-p0a.md @@ -21,6 +21,8 @@ P0a establishes one cross-process writer for each ACP/daemon `(runtime base, ses P0a does not make session switching, rewind, branch/fork, working-directory migration, archive/delete/rename maintenance, or transcript repair transactional. It also does not introduce an initializing registry entry that serializes every same-daemon load/resume against close; a repeated load reuses the owner after that owner is registered, while the cross-process lease still rejects a second writer during initialization. Full load/close outcome coalescing belongs to P0b. Session switching and persistence-root migration fail closed while an ACP Config owns a lease. ACP's logical working-directory change remains supported because it keeps the recorder and SessionService bound to the original persistence root. Same-owner rewind loads through that Config-pinned SessionService under the recorder write barrier; rename and branch retain their existing recorder or flush-before-copy paths. Daemon archive/delete and maintenance of non-live sessions retain their existing semantics. Concurrent maintenance from outside the live owner remains unsupported and is part of the P0b boundary. Interactive and headless CLI recorders retain their existing unleased behavior so `/clear`, `/resume`, `/branch`, and `/cd` do not regress; they must not write the same session concurrently with an ACP owner until P0b broadens the protocol. +The protocol is gated by `experimental.sessionWriterLease` and is disabled by default. The effective value is snapshotted from the bootstrap Config when the ACP child starts and remains fixed for every session served by that process; per-session settings reloads cannot change it. Enabling it requires a process restart. The setting affects only ACP/daemon recorders; interactive and headless recorders continue to use the legacy path even when the setting is enabled. + ## Invariants 1. At most one cooperating ACP process owns a session writer lease under a runtime base. @@ -48,7 +50,7 @@ The lease snapshots whether the transcript exists, its file identity and metadat ## Activation and close -An ACP `Config.initialize()` acquires the lease before extension, hook, tool, model, or scheduler initialization. While holding the lease it resolves active/archive state, reloads the active transcript when one exists, verifies that the transcript did not change during the reload, replaces any pre-lock preview, and activates the recorder. Non-ACP Configs continue through the legacy recorder path without acquiring this P0a lease. +When the feature gate is enabled, an ACP `Config.initialize()` acquires the lease before extension, hook, tool, model, or scheduler initialization. While holding the lease it resolves active/archive state, reloads the active transcript when one exists, verifies that the transcript did not change during the reload, replaces any pre-lock preview, and activates the recorder. ACP Configs without the opt-in and all non-ACP Configs continue through the legacy recorder path without acquiring this P0a lease. Any later initialization failure closes the recorder and releases the lease. Normal shutdown and ACP session close finalize pending metadata, drain the recorder queue, release the owner token, and only then remove the live session entry. Cleanup is identity-checked so a failed older initialization cannot close a newer same-ID entry, and an unreturned Config whose first release fails is retried before the daemon creates another fresh session. A definitive child refusal leaves the session live so close can be retried. Close draining is bounded; a timeout or transport failure has an unknown result, so the bridge terminates the shared ACP channel and its process-owned leases become recoverable as stale. Other sessions on that channel are also reaped by that recovery action. @@ -65,7 +67,7 @@ External responses use fixed messages and `errorKind`; they do not expose PID, h ## Compatibility and rollout -The protocol only coordinates ACP binaries that understand it. Deployment and rollback must drain old ACP/daemon writer processes before the new version starts. Mixed-version ACP operation is not safe because an old writer ignores the lock. Concurrent interactive or headless access to the same persisted session remains outside P0a and is unsupported until P0b. +The protocol only coordinates ACP writers that have the feature enabled. Deployment and rollback must drain old ACP/daemon writer processes before enabling or disabling the setting. Mixed-version or mixed-configuration ACP operation is not safe because a legacy writer ignores the lock. Concurrent interactive or headless access to the same persisted session remains outside P0a and is unsupported until P0b. The runtime filesystem must support same-directory hard links with atomic no-replace behavior. If that prerequisite is unavailable, acquisition fails closed with `session_writer_unavailable`. @@ -73,8 +75,8 @@ Existing branched transcripts are not automatically repaired. P0a prevents a new ## Verification -Unit coverage exercises lock contention, dead-owner and crashed-reclaimer recovery, malformed and non-regular locks, concurrent and retryable owner-token release, truncated and externally changed transcripts, equal-length file replacement, UTF-8 byte accounting, recorder activation/fencing/close, authoritative reload, initialization cleanup, runtime-root pinning, turn admission, same-daemon replay reuse, disabled-recording compatibility, legacy interactive recorder behavior, and error sanitization. Darwin coverage also verifies that processes with different time zones derive the same owner identity. PID-reuse handling is implemented but is not claimed as test evidence because process-start probing is platform dependent. +Unit coverage exercises the default-off and explicit-opt-in gates, lock contention, dead-owner and crashed-reclaimer recovery, malformed and non-regular locks, concurrent and retryable owner-token release, truncated and externally changed transcripts, equal-length file replacement, UTF-8 byte accounting, recorder activation/fencing/close, authoritative reload, initialization cleanup, runtime-root pinning, turn admission, same-daemon replay reuse, disabled-recording compatibility, legacy interactive recorder behavior, and error sanitization. Darwin coverage also verifies that processes with different time zones derive the same owner identity. PID-reuse handling is implemented but is not claimed as test evidence because process-start probing is platform dependent. -A real two-process regression recreates the incident timing: process A holds the writer after a tool-result tail, process B is rejected before loading as a writer, A appends its final answer and closes, and B then acquires, reloads that final answer, and appends the next user record with the final answer as its parent. +With the feature gate enabled, a real two-process regression recreates the incident timing: process A holds the writer after a tool-result tail, process B is rejected before loading as a writer, A appends its final answer and closes, and B then acquires, reloads that final answer, and appends the next user record with the final answer as its parent. Desktop coverage verifies that a writer conflict is surfaced to the user instead of silently replacing the requested persisted session with a fresh session. Live history refresh is served through the owner's write barrier and Config-pinned SessionService, including after a logical `/cd`. diff --git a/docs/users/configuration/settings.md b/docs/users/configuration/settings.md index bcae5774711..5d95e268724 100644 --- a/docs/users/configuration/settings.md +++ b/docs/users/configuration/settings.md @@ -557,6 +557,7 @@ LSP server configuration is done through `.lsp.json` files in your project root | -------------------------------------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------- | | `experimental.cron` | boolean | Enable in-session cron/loop tools (`cron_create`, `cron_list`, `cron_delete`) so the model can create recurring prompts. Can be disabled via the `QWEN_CODE_DISABLE_CRON=1` environment variable. Requires restart. | `true` | | `experimental.todoStopGuard` | boolean | Allow daemon and ACP sessions to continue after a natural model stop when the current work chain successfully wrote an unfinished top-level Todo list. Adds at most two consecutive primary-model calls without new user input; mid-turn user input starts a fresh two-attempt stage. It is not restored after process restart and is forced off in safe, bare, and Approval `plan` modes. Requires restart. | `false` | +| `experimental.sessionWriterLease` | boolean | Enable cross-process write fencing for persisted ACP and daemon sessions. The value is frozen when the ACP or daemon process starts. All concurrent ACP writers must enable the setting; mixed versions or configurations remain unsafe. Interactive and headless recorders are unaffected. Requires process restart. | `false` | | `experimental.cronRecurringMaxAgeDays` | number | Days a recurring cron/loop job lives before auto-expiring (it fires one final time, then is deleted). Set to `0` to disable expiry so jobs run until deleted — useful for long-running daemon deployments. Can be overridden via the `QWEN_CODE_CRON_MAX_AGE_DAYS` environment variable. Requires restart. | `7` | | `experimental.agentTeam` | boolean | Enable agent-team collaboration tools (`team_create`, `task_create`, `task_update`, `send_message`, etc.) for multi-agent coordination. Can also be enabled via `QWEN_CODE_ENABLE_AGENT_TEAM=1`. Requires restart. | `false` | | `experimental.artifact` | boolean | Enable artifact tools. Enabled by default. In interactive, non-SDK sessions, the model can publish a self-contained HTML page as an interactive Artifact and open it in the browser. Non-SDK daemon sessions can use metadata-only `record_artifact`. Set this to `false` or use `QWEN_CODE_DISABLE_ARTIFACT=1` to disable both. Requires restart. | `true` | diff --git a/packages/cli/src/acp-integration/acpAgent.test.ts b/packages/cli/src/acp-integration/acpAgent.test.ts index f68b606df0f..9c003729a08 100644 --- a/packages/cli/src/acp-integration/acpAgent.test.ts +++ b/packages/cli/src/acp-integration/acpAgent.test.ts @@ -2391,6 +2391,50 @@ describe('QwenAgent MCP SSE/HTTP support', () => { await agentPromise; }); + it.each([ + ['disabled', false, true], + ['enabled', true, false], + ])( + 'keeps the session writer lease %s until the ACP process restarts', + async (_label, startupEnabled, requestEnabled) => { + await setupSessionMocks(`session-writer-lease-${startupEnabled}`); + mockConfig.isSessionWriterLeaseEnabled = vi + .fn() + .mockReturnValue(startupEnabled); + + const startupSettings = makeSessionSettings(); + startupSettings.merged.experimental = { + sessionWriterLease: startupEnabled, + }; + const requestSettings = makeSessionSettings(); + requestSettings.merged.experimental = { + sessionWriterLease: requestEnabled, + }; + vi.mocked(loadSettings).mockReturnValue(requestSettings); + + const agentPromise = runAcpAgent(mockConfig, startupSettings, mockArgv); + await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined()); + const agent = capturedAgentFactory!({ + get closed() { + return mockConnectionState.promise; + }, + }) as AgentLike; + + await agent.newSession({ cwd: '/tmp', mcpServers: [] }); + + const sessionSettings = vi.mocked(loadCliConfig).mock.calls[0]?.[0]; + expect(sessionSettings?.experimental?.sessionWriterLease).toBe( + startupEnabled, + ); + expect(requestSettings.merged.experimental?.sessionWriterLease).toBe( + requestEnabled, + ); + + mockConnectionState.resolve(); + await agentPromise; + }, + ); + it('profiles newSession stages under the daemon trace context', async () => { const parentContext = { trace: 'parent' }; mockExtractDaemonTraceContext.mockReturnValue(parentContext); diff --git a/packages/cli/src/acp-integration/acpAgent.ts b/packages/cli/src/acp-integration/acpAgent.ts index cdbcd3b8d81..8c85b185ad0 100644 --- a/packages/cli/src/acp-integration/acpAgent.ts +++ b/packages/cli/src/acp-integration/acpAgent.ts @@ -2766,6 +2766,13 @@ export async function runAcpAgent( argv: CliArgs, options?: { privateParentCapability?: string }, ) { + // Freeze the restart-required writer protocol before the first await. + // Per-request settings reloads must not mix leased and legacy writers + // within one ACP process lifetime. + const sessionWriterLeaseEnabledAtStartup = + typeof config.isSessionWriterLeaseEnabled === 'function' + ? config.isSessionWriterLeaseEnabled() + : settings.merged.experimental?.sessionWriterLease === true; const privateParentCapability = options === undefined ? process.env[PRIVATE_ACP_CAPABILITY_ENV] @@ -2897,6 +2904,7 @@ export async function runAcpAgent( argv, conn, privateParentCapability, + sessionWriterLeaseEnabledAtStartup, ); return agentInstance; }, stream); @@ -3831,6 +3839,7 @@ class QwenAgent implements Agent { private argv: CliArgs, private connection: AgentSideConnection, private readonly expectedPrivateParentCapability?: string, + private readonly sessionWriterLeaseEnabledAtStartup = false, ) { // Pool kill switch via env var so operators can A/B compare or // roll back without rebuilding. `run-qwen-serve.ts` sets this when @@ -10439,7 +10448,13 @@ class QwenAgent implements Agent { } } - const mergedSettings = settings.merged; + const mergedSettings = { + ...settings.merged, + experimental: { + ...settings.merged.experimental, + sessionWriterLease: this.sessionWriterLeaseEnabledAtStartup, + }, + }; const sessionArg = resume === true diff --git a/packages/cli/src/config/config.test.ts b/packages/cli/src/config/config.test.ts index 29ae5189857..ca451aff8aa 100644 --- a/packages/cli/src/config/config.test.ts +++ b/packages/cli/src/config/config.test.ts @@ -1347,6 +1347,52 @@ describe('loadCliConfig', () => { ); }); + it('should keep the session writer lease disabled by default', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + + await loadCliConfig({}, argv); + + expect(mockConfigConstructorParams).toHaveBeenCalledWith( + expect.objectContaining({ + sessionWriterLeaseEnabled: false, + }), + ); + }); + + it('should propagate the session writer lease opt-in', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + + await loadCliConfig({ experimental: { sessionWriterLease: true } }, argv); + + expect(mockConfigConstructorParams).toHaveBeenCalledWith( + expect.objectContaining({ + sessionWriterLeaseEnabled: true, + }), + ); + }); + + it('should not enable the session writer lease for invalid truthy values', async () => { + process.argv = ['node', 'script.js']; + const argv = await parseArguments(); + + await loadCliConfig( + { + experimental: { + sessionWriterLease: 'true', + }, + } as unknown as Settings, + argv, + ); + + expect(mockConfigConstructorParams).toHaveBeenCalledWith( + expect.objectContaining({ + sessionWriterLeaseEnabled: false, + }), + ); + }); + it('should propagate the image model selection', async () => { process.argv = ['node', 'script.js']; const argv = await parseArguments(); diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts index c18b5c25c87..0b190ce97f2 100755 --- a/packages/cli/src/config/config.ts +++ b/packages/cli/src/config/config.ts @@ -2144,6 +2144,8 @@ export async function loadCliConfig( // Undefined flows through to Config's default (5) and clamp logic. maxSubagentDepth: resolveMaxSubagentDepth(argv, settings), experimentalZedIntegration: argv.acp || argv.experimentalAcp || false, + sessionWriterLeaseEnabled: + settings.experimental?.sessionWriterLease === true, cronEnabled: settings.experimental?.cron ?? true, cronRecurringMaxAgeDays: settings.experimental?.cronRecurringMaxAgeDays, agentTeamEnabled: settings.experimental?.agentTeam ?? false, diff --git a/packages/cli/src/config/settingsSchema.test.ts b/packages/cli/src/config/settingsSchema.test.ts index 8c7ab5069fd..3418c51e0db 100644 --- a/packages/cli/src/config/settingsSchema.test.ts +++ b/packages/cli/src/config/settingsSchema.test.ts @@ -122,6 +122,17 @@ describe('SettingsSchema', () => { ).toBeDefined(); }); + it('should keep the ACP session writer lease opt-in', () => { + expect( + getSettingsSchema().experimental.properties.sessionWriterLease, + ).toMatchObject({ + type: 'boolean', + default: false, + requiresRestart: true, + showInDialog: true, + }); + }); + it('should expose cumulative tool result threshold in clearContextOnIdle', () => { const threshold = getSettingsSchema().context.properties.clearContextOnIdle.properties diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts index 990898fab1c..3c8996f1b9e 100644 --- a/packages/cli/src/config/settingsSchema.ts +++ b/packages/cli/src/config/settingsSchema.ts @@ -3351,6 +3351,16 @@ const SETTINGS_SCHEMA = { 'Allow daemon and ACP sessions to continue an unfinished top-level Todo list for at most two consecutive primary-model calls without new user input. Mid-turn user input starts a fresh two-attempt stage. Disabled in safe, bare, and Approval plan modes.', showInDialog: false, }, + sessionWriterLease: { + type: 'boolean', + label: 'Enable ACP Session Writer Lease', + category: 'Experimental', + requiresRestart: true, + default: false, + description: + 'Enable cross-process write fencing for persisted ACP and daemon sessions. The effective value is frozen when the ACP or daemon process starts. Every concurrent ACP or daemon writer must enable the setting; interactive and headless writers remain outside the protocol.', + showInDialog: true, + }, cronRecurringMaxAgeDays: { type: 'number', label: 'Recurring Cron Max Age (Days)', diff --git a/packages/core/src/config/config.test.ts b/packages/core/src/config/config.test.ts index da384624b67..c81616f41c6 100644 --- a/packages/core/src/config/config.test.ts +++ b/packages/core/src/config/config.test.ts @@ -2621,12 +2621,56 @@ describe('Server Config (config.ts)', () => { }); describe('initialize', () => { + it.each([ + [ + 'an ACP session without an opt-in', + { experimentalZedIntegration: true }, + ], + [ + 'a non-ACP session with the setting enabled', + { sessionWriterLeaseEnabled: true }, + ], + [ + 'an ACP session with an invalid truthy opt-in', + { + experimentalZedIntegration: true, + sessionWriterLeaseEnabled: 'true' as unknown as boolean, + }, + ], + ])( + 'uses the legacy recorder without acquiring a writer lease for %s', + async (_name, params) => { + const acquire = vi.spyOn(SessionWriterLease, 'acquire'); + const config = new Config({ + ...baseParams, + ...params, + chatRecording: true, + }); + + await ( + config as unknown as { activateChatRecording(): Promise } + ).activateChatRecording(); + + expect(acquire).not.toHaveBeenCalled(); + expect(config.isSessionWriterLeaseEnabled()).toBe(false); + expect(config.hasSessionWriteOwnership()).toBe(false); + await expect( + config + .getChatRecordingService() + ?.runWithWriteBarrier(async () => 'legacy'), + ).resolves.toBe('legacy'); + acquire.mockRestore(); + }, + ); + it('preserves activation and lease release failures', async () => { const config = new Config({ ...baseParams, chatRecording: true, experimentalZedIntegration: true, + sessionWriterLeaseEnabled: true, }); + expect(config.isSessionWriterLeaseEnabled()).toBe(true); const activationError = new SessionTranscriptChangedError(); const releaseError = new Error('lease release failed'); const release = vi.fn().mockRejectedValue(releaseError); diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 8c99912f27d..b9d614a29f0 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -1053,6 +1053,7 @@ export interface ConfigParameters { clearContextOnIdle?: ClearContextOnIdleSettings; sessionTokenLimit?: number; experimentalZedIntegration?: boolean; + sessionWriterLeaseEnabled?: boolean; cronEnabled?: boolean; /** * Days a recurring cron job lives before auto-expiring. `0` disables @@ -1848,6 +1849,7 @@ export class Config { private readonly cliVersion?: string; private runtimeStatusEnabled = false; private readonly experimentalZedIntegration: boolean = false; + private readonly sessionWriterLeaseEnabled: boolean = false; private readonly cronEnabled: boolean = true; /** Recurring cron max age in days, resolved once at construction * (the setting declares `requiresRestart`); `Infinity` = no expiry. */ @@ -2106,6 +2108,9 @@ export class Config { this.sessionTokenLimit = params.sessionTokenLimit ?? -1; this.experimentalZedIntegration = params.experimentalZedIntegration ?? false; + this.sessionWriterLeaseEnabled = + this.experimentalZedIntegration === true && + params.sessionWriterLeaseEnabled === true; this.cronEnabled = params.cronEnabled ?? true; this.cronRecurringMaxAgeDays = resolveCronRecurringMaxAgeDays( params.cronRecurringMaxAgeDays, @@ -2888,7 +2893,9 @@ export class Config { } private async activateChatRecording(): Promise { - if (!this.chatRecordingEnabled || !this.experimentalZedIntegration) return; + if (!this.chatRecordingEnabled || !this.sessionWriterLeaseEnabled) { + return; + } const recorder = this.chatRecordingService; if (!recorder) throw new SessionWriterUnavailableError(); let lease: SessionWriterLease | undefined; @@ -6155,6 +6162,10 @@ export class Config { return this.experimentalZedIntegration; } + isSessionWriterLeaseEnabled(): boolean { + return this.sessionWriterLeaseEnabled; + } + getListExtensions(): boolean { return this.listExtensions; } @@ -6696,7 +6707,7 @@ export class Config { (event) => { this.notifyChatRecordingFailure(event); }, - this.experimentalZedIntegration, + this.sessionWriterLeaseEnabled, ); } diff --git a/packages/core/src/services/chatRecordingService.test.ts b/packages/core/src/services/chatRecordingService.test.ts index b4ccc23add8..36be8fd45c9 100644 --- a/packages/core/src/services/chatRecordingService.test.ts +++ b/packages/core/src/services/chatRecordingService.test.ts @@ -1561,6 +1561,17 @@ describe('ChatRecordingService', () => { }); describe('legacy recorder', () => { + it('uses the effective session writer lease gate by default', async () => { + mockConfig.getExperimentalZedIntegration = vi.fn().mockReturnValue(true); + mockConfig.isSessionWriterLeaseEnabled = vi.fn().mockReturnValue(false); + const service = new ChatRecordingService(mockConfig); + + service.recordUserMessage([{ text: 'legacy' }]); + await service.flush(); + + expect(jsonl.writeLine).toHaveBeenCalledOnce(); + }); + it('retries directory setup after a synchronous failure', async () => { const mkdirSpy = vi.spyOn(fs, 'mkdirSync'); mkdirSpy.mockImplementationOnce(() => { diff --git a/packages/core/src/services/chatRecordingService.ts b/packages/core/src/services/chatRecordingService.ts index 61dac133dc8..608731d276e 100644 --- a/packages/core/src/services/chatRecordingService.ts +++ b/packages/core/src/services/chatRecordingService.ts @@ -686,7 +686,9 @@ export class ChatRecordingService { constructor( config: Config, private readonly onWriteFailure?: ChatRecordingFailureListener, - writerLeaseRequired = config.getExperimentalZedIntegration?.() ?? true, + writerLeaseRequired = config.isSessionWriterLeaseEnabled?.() ?? + config.getExperimentalZedIntegration?.() ?? + true, ) { this.config = config; this.writerLeaseRequired = writerLeaseRequired; diff --git a/packages/core/src/services/session-writer-lease.test.ts b/packages/core/src/services/session-writer-lease.test.ts index bbba98a008f..80bae2dae25 100644 --- a/packages/core/src/services/session-writer-lease.test.ts +++ b/packages/core/src/services/session-writer-lease.test.ts @@ -207,6 +207,7 @@ describe('SessionWriterLease', () => { model: 'test-model', chatRecording: true, experimentalZedIntegration: true, + sessionWriterLeaseEnabled: true, bareMode: true, telemetry: { enabled: false }, usageStatisticsEnabled: false, @@ -318,6 +319,7 @@ describe('SessionWriterLease', () => { model: 'test-model', chatRecording: true, experimentalZedIntegration: true, + sessionWriterLeaseEnabled: true, bareMode: true, telemetry: { enabled: false }, usageStatisticsEnabled: false, @@ -369,6 +371,7 @@ describe('SessionWriterLease', () => { model: 'test-model', chatRecording: true, experimentalZedIntegration: true, + sessionWriterLeaseEnabled: true, bareMode: true, telemetry: { enabled: false }, usageStatisticsEnabled: false, diff --git a/packages/vscode-ide-companion/schemas/settings.schema.json b/packages/vscode-ide-companion/schemas/settings.schema.json index 433335f78df..598839709a5 100644 --- a/packages/vscode-ide-companion/schemas/settings.schema.json +++ b/packages/vscode-ide-companion/schemas/settings.schema.json @@ -3130,6 +3130,11 @@ "type": "boolean", "default": false }, + "sessionWriterLease": { + "description": "Enable cross-process write fencing for persisted ACP and daemon sessions. The effective value is frozen when the ACP or daemon process starts. Every concurrent ACP or daemon writer must enable the setting; interactive and headless writers remain outside the protocol.", + "type": "boolean", + "default": false + }, "cronRecurringMaxAgeDays": { "type": "number", "minimum": 0,