Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions docs/design/session-writer-lease-p0a.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.

Expand All @@ -65,16 +67,16 @@ 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`.

Existing branched transcripts are not automatically repaired. P0a prevents a new stale-load branch after rollout; repair and explicit branch semantics remain separate work.

## 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`.
1 change: 1 addition & 0 deletions docs/users/configuration/settings.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down
44 changes: 44 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
17 changes: 16 additions & 1 deletion packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -2897,6 +2904,7 @@ export async function runAcpAgent(
argv,
conn,
privateParentCapability,
sessionWriterLeaseEnabledAtStartup,
);
return agentInstance;
}, stream);
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
46 changes: 46 additions & 0 deletions packages/cli/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 2 additions & 0 deletions packages/cli/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Comment thread
doudouOUC marked this conversation as resolved.
settings.experimental?.sessionWriterLease === true,
Comment thread
doudouOUC marked this conversation as resolved.
cronEnabled: settings.experimental?.cron ?? true,
cronRecurringMaxAgeDays: settings.experimental?.cronRecurringMaxAgeDays,
agentTeamEnabled: settings.experimental?.agentTeam ?? false,
Expand Down
11 changes: 11 additions & 0 deletions packages/cli/src/config/settingsSchema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Comment thread
doudouOUC marked this conversation as resolved.
},
cronRecurringMaxAgeDays: {
type: 'number',
label: 'Recurring Cron Max Age (Days)',
Expand Down
44 changes: 44 additions & 0 deletions packages/core/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Comment thread
doudouOUC marked this conversation as resolved.
{
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<void> }
).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);
Expand Down
Loading
Loading