Skip to content

Commit 81ebbf2

Browse files
committed
fix(runtime): reject a mixed tightening while a Turn is active (#3349)
Review finding: session.configuration.update is a full configuration operation, but the fast tightening path commits the entire record while the live backend keeps its frozen composition. For bypass/agent → ask/plan the permission half applies per dispatch while the stale agent collaboration keeps combining with the fresh ask boundary — so a write-capable dispatch can still be admitted after the stored configuration already says the session is read-only Plan. A tightening that also changes any backend-composed field (backend, connection, model, thinking level, collaboration mode, orchestration mode) now rejects session_busy while a run is live — never partially committed; the retry lands whole once the run ends and the queued path recomposes the backend with every field. Permission-only projections keep the immediate-revocation path. The regression drives the reviewer's scenario end to end: live rejection with zero partial commit, then a read-only dispatch (plan downgrades ask to explore) after the retry. Generated-by: ZCode (Z.ai GLM)
1 parent d63f8ca commit 81ebbf2

2 files changed

Lines changed: 127 additions & 10 deletions

File tree

packages/runtime/src/__tests__/session-manager.test.ts

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5341,6 +5341,87 @@ describe('SessionManager permission mode updates', () => {
53415341
expect(composedModes).toEqual(['bypass', 'ask']);
53425342
});
53435343

5344+
test('a mixed bypass/agent → ask/plan tightening rejects while a run is live and lands read-only on retry', async () => {
5345+
const store = new VersionedConfigurationMemorySessionStore();
5346+
const runStore = new MemoryAgentRunStore();
5347+
const backends = new BackendRegistry();
5348+
const gate = makeGate();
5349+
backends.register('ai-sdk', (ctx) => new TestBackend(ctx, gate));
5350+
const manager = new SessionManager({
5351+
store,
5352+
runStore,
5353+
runtimeEventStore: runStore,
5354+
backends,
5355+
shellRuns: {
5356+
async terminateSession() {
5357+
return undefined;
5358+
},
5359+
async commitSessionClose() {},
5360+
rollbackSessionClose() {},
5361+
resumeSession() {},
5362+
} as never,
5363+
planStore: {
5364+
readState: async () => ({ activeExecutionId: null, latestProposalId: null, proposals: [] }),
5365+
} as never,
5366+
newId: nextId(),
5367+
now: nextNow(8_000),
5368+
});
5369+
const session = await manager.createSession(
5370+
makeInput({ permissionMode: 'bypass', collaborationMode: 'agent' }),
5371+
);
5372+
const mixedConfiguration = {
5373+
backend: session.backend,
5374+
llmConnectionSlug: session.llmConnectionSlug,
5375+
connectionLocked: true,
5376+
model: session.model,
5377+
thinkingLevel: session.thinkingLevel,
5378+
permissionMode: 'ask',
5379+
collaborationMode: 'plan',
5380+
orchestrationMode: session.orchestrationMode ?? 'default',
5381+
} as const;
5382+
5383+
// The turn is live under bypass/agent when the one-shot tightening to
5384+
// ask/plan arrives. Only the permission half could apply per dispatch; the
5385+
// plan half is backend-composed, so committing now would publish a
5386+
// read-only configuration the live composition cannot enforce.
5387+
const turn = manager
5388+
.sendMessage(session.id, { turnId: 'turn-1', text: 'work' })
5389+
[Symbol.asyncIterator]();
5390+
expect((await turn.next()).value?.type).toBe('text_delta');
5391+
5392+
await expectRejects(
5393+
manager.transitionSessionConfiguration(session.id, {
5394+
expectedRevision: 1,
5395+
configuration: mixedConfiguration,
5396+
}),
5397+
/while a Turn is active/,
5398+
);
5399+
5400+
// Nothing partially committed: the record and a dispatch in this turn
5401+
// still agree on the old authority.
5402+
const header = await store.readHeader(session.id);
5403+
expect(header.permissionMode).toBe('bypass');
5404+
expect(header.collaborationMode).toBe('agent');
5405+
const dispatch = await dispatchProbeTool(store, session.id);
5406+
expect(dispatch.boundaryKind).toBe('bypass');
5407+
expect(dispatch.permissionMode).toBe('bypass');
5408+
5409+
gate.release();
5410+
while (!(await turn.next()).done) {}
5411+
5412+
// Once the run ended the retry lands whole, and a write-capable dispatch
5413+
// cannot receive writable authority: plan downgrades ask to explore.
5414+
const committed = await manager.transitionSessionConfiguration(session.id, {
5415+
expectedRevision: 1,
5416+
configuration: mixedConfiguration,
5417+
});
5418+
expect(committed.header.permissionMode).toBe('ask');
5419+
expect(committed.header.collaborationMode).toBe('plan');
5420+
const narrowed = await dispatchProbeTool(store, session.id);
5421+
expect(narrowed.boundaryKind).toBe('managed');
5422+
expect(narrowed.permissionMode).toBe('explore');
5423+
});
5424+
53445425
test('a switch queued behind a turn that pauses on an interaction rejects busy', async () => {
53455426
const store = new MemorySessionStore();
53465427
const runStore = new MemoryAgentRunStore();

packages/runtime/src/session-manager.ts

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1135,9 +1135,24 @@ export class SessionManager {
11351135
input: SessionConfigurationTransitionRequest,
11361136
): Promise<VersionedSessionHeader> {
11371137
const store = this.requireSessionConfigurationStore();
1138+
// A tightening that also changes any backend-composed field cannot use the
1139+
// immediate-commit path while a run is live: the frozen composition (for
1140+
// example a stale collaboration mode) would keep combining with the fresh
1141+
// boundary, so a plan/ask session could still admit writable dispatches.
1142+
const header = await this.deps.store.readHeader(sessionId);
1143+
const changesBackendComposition =
1144+
input.configuration.backend !== header.backend ||
1145+
input.configuration.llmConnectionSlug !== header.llmConnectionSlug ||
1146+
input.configuration.model !== header.model ||
1147+
(input.configuration.thinkingLevel ?? undefined) !== header.thinkingLevel ||
1148+
(input.configuration.collaborationMode ?? 'agent') !==
1149+
(header.collaborationMode ?? 'agent') ||
1150+
(input.configuration.orchestrationMode ?? 'default') !==
1151+
(header.orchestrationMode ?? 'default');
11381152
const next = await this.commitExecutionResourceTransition(
11391153
sessionId,
11401154
input.configuration.permissionMode,
1155+
changesBackendComposition,
11411156
async () => {
11421157
const current = await store.readHeaderRecordSnapshot(sessionId);
11431158
if (current.revision !== input.expectedRevision) {
@@ -1673,21 +1688,28 @@ export class SessionManager {
16731688
},
16741689
): Promise<ExecutionBoundary> {
16751690
const nextPermissionMode = projection?.permissionMode ?? (kind === 'bypass' ? 'bypass' : 'ask');
1676-
return this.commitExecutionResourceTransition(sessionId, nextPermissionMode, async () => {
1677-
const latest = await this.deps.store.readExecutionBoundary(sessionId);
1678-
if (latest.revision !== current.revision) {
1679-
throw new SessionConfigurationTransitionError(
1680-
'operation_conflict',
1681-
'Session execution boundary changed before the transition',
1682-
);
1683-
}
1684-
return () => this.deps.store.setExecutionBoundaryKind(sessionId, kind, projection);
1685-
});
1691+
// Permission-only projection: no backend-composed field rides along.
1692+
return this.commitExecutionResourceTransition(
1693+
sessionId,
1694+
nextPermissionMode,
1695+
false,
1696+
async () => {
1697+
const latest = await this.deps.store.readExecutionBoundary(sessionId);
1698+
if (latest.revision !== current.revision) {
1699+
throw new SessionConfigurationTransitionError(
1700+
'operation_conflict',
1701+
'Session execution boundary changed before the transition',
1702+
);
1703+
}
1704+
return () => this.deps.store.setExecutionBoundaryKind(sessionId, kind, projection);
1705+
},
1706+
);
16861707
}
16871708

16881709
private async commitExecutionResourceTransition<T>(
16891710
sessionId: string,
16901711
nextPermissionMode: PermissionMode,
1712+
changesBackendComposition: boolean,
16911713
prepareCommit: () => Promise<() => Promise<T>>,
16921714
): Promise<T> {
16931715
const initialBoundary = await this.deps.store.readExecutionBoundary(sessionId);
@@ -1698,6 +1720,7 @@ export class SessionManager {
16981720
return this.commitTighteningTransition<T>(
16991721
sessionId,
17001722
[sessionId, ...initialDescendants],
1723+
changesBackendComposition,
17011724
prepareCommit,
17021725
);
17031726
}
@@ -1734,6 +1757,7 @@ export class SessionManager {
17341757
private async commitTighteningTransition<T>(
17351758
sessionId: string,
17361759
fencedSessionIds: readonly string[],
1760+
changesBackendComposition: boolean,
17371761
prepareCommit: () => Promise<() => Promise<T>>,
17381762
): Promise<T> {
17391763
if (!this.runtimeKernel.runSessionAdmissionMutation) {
@@ -1755,6 +1779,18 @@ export class SessionManager {
17551779
'Session lineage changed before the configuration transition',
17561780
);
17571781
}
1782+
if (changesBackendComposition && this.runtimeKernel.hasActiveRuns(sessionId)) {
1783+
// The permission half of this update could apply per dispatch, but the
1784+
// composed half only refreshes on rebuild — committing both now would
1785+
// publish a read-only configuration the live backend cannot enforce
1786+
// (a stale agent composition keeps admitting writable dispatches).
1787+
// Reject the atomic update whole; the retry lands once the run ends
1788+
// and the queued path recomposes the backend with every field.
1789+
throw new SessionConfigurationTransitionError(
1790+
'session_busy',
1791+
'A mixed permission and configuration tightening cannot commit while a Turn is active',
1792+
);
1793+
}
17581794
if (!this.deps.shellRuns) {
17591795
throw new SessionConfigurationTransitionError(
17601796
'operation_unavailable',

0 commit comments

Comments
 (0)