Skip to content
Closed
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
c6dc809
feat(serve): add runtime context injection for per-turn system-reminders
callmeYe Jun 25, 2026
8ca0e0c
fix(sdk): bump browser bundle size limit for runtime context APIs
callmeYe Jun 25, 2026
8eda5fe
fix(test): update test snapshots for runtime context capability
callmeYe Jun 25, 2026
419f3d5
fix: address review feedback for runtime context
callmeYe Jun 25, 2026
71172e7
fix(test): prefix unused context param with underscore
callmeYe Jun 25, 2026
5a78b44
fix: address wenshao review — prototype isolation, trusted client ID,…
callmeYe Jun 25, 2026
887a2ea
fix: address wenshao round-2 suggestions
callmeYe Jun 25, 2026
cf78b57
test(cli): add handler tests for sessionRuntimeContext ext-method
callmeYe Jun 25, 2026
f8d4162
fix(test): use runAcpAgent instead of non-existent runAcpMode
callmeYe Jun 25, 2026
ec0428e
fix(test): fix arrow-body-style lint error in Session mock
callmeYe Jun 25, 2026
e60fe0a
fix(test): pass cwd and mcpServers to newSession in runtime context t…
callmeYe Jun 25, 2026
7fb5ff3
fix(test): resolve connection.closed in afterEach to prevent shutdown…
callmeYe Jun 25, 2026
5c33b68
ci: trigger fresh CI run (flaky no-AK smoke test)
callmeYe Jun 25, 2026
421635b
fix(test): add session_runtime_context to integration test capabiliti…
callmeYe Jun 26, 2026
0d7c2f0
fix(test): restore missing closing braces lost during conflict resolu…
callmeYe Jul 1, 2026
6367d5c
fix(serve): move runtime-context route to routes/session.ts after ser…
callmeYe Jul 1, 2026
02c14cb
merge: sync with main
callmeYe Jul 1, 2026
d2fa328
merge: sync with main, resolve conflicts in routes/session.ts and bui…
callmeYe Jul 1, 2026
2e8192c
fix: remove accidental pnpm-lock.yaml from tracked files
callmeYe Jul 1, 2026
48ccb80
merge: sync with main, resolve config.test.ts conflict
callmeYe Jul 1, 2026
2626aa0
fix: remove pnpm-lock.yaml and add to .gitignore
callmeYe Jul 1, 2026
8b8c780
merge: sync with main (MCP test fix #6120)
callmeYe Jul 1, 2026
541dba6
merge: sync with main
callmeYe Jul 1, 2026
0677a38
merge: sync with main
qwen-code-dev-bot Jul 5, 2026
22a19e0
merge: sync with main
qwen-code-dev-bot Jul 7, 2026
0fd3502
Merge branch 'main' into worktree-feat+runtime-context
wenshao Jul 7, 2026
9de95a8
Merge branch 'main' into worktree-feat+runtime-context
wenshao Jul 10, 2026
a1c3b8e
Merge branch 'main' into worktree-feat+runtime-context
wenshao Jul 10, 2026
ce0767a
Merge branch 'main' into worktree-feat+runtime-context
wenshao Jul 11, 2026
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
1 change: 1 addition & 0 deletions integration-tests/cli/qwen-serve-routes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ describe('qwen serve — capabilities envelope', () => {
'permission_mediation',
'non_blocking_prompt',
'session_language',
'session_runtime_context',
'session_rewind',
'workspace_hooks',
'session_hooks',
Expand Down
29 changes: 29 additions & 0 deletions packages/acp-bridge/src/bridge.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4808,6 +4808,35 @@ export function createAcpSessionBridge(opts: BridgeOptions): AcpSessionBridge {
}
},

async setSessionRuntimeContext(sessionId, entries, context) {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged. The bridge follows the exact same pattern as setSessionLanguage — same SessionNotFoundError paths, same withTimeout + getTransportClosedReject race. Adding bridge integration tests is a valid follow-up but out of scope for this PR (the existing setSessionLanguage bridge tests don't exist either). Will track as a follow-up.

const entry = byId.get(sessionId);
if (!entry) throw new SessionNotFoundError(sessionId);
const info = channelInfoForEntry(entry);
if (!info || info.isDying) throw new SessionNotFoundError(sessionId);
resolveTrustedClientId(entry, context?.clientId);

const response = (await Promise.race([
withTimeout(
entry.connection.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
{ sessionId, entries },
),
initTimeoutMs,
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
),
getTransportClosedReject(entry),
])) as {
keys: string[];
rejected?: Array<{ key: string; reason: string }>;
};

return {
sessionId,
keys: response.keys,
rejected: response.rejected ?? [],
};
},

async generateSessionRecap(sessionId, _context) {
// Thin pass-through to `qwen/control/session/
// recap` — the ACP child runs `generateSessionRecap` against the
Expand Down
15 changes: 15 additions & 0 deletions packages/acp-bridge/src/bridgeTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,21 @@ export interface AcpSessionBridge {
persisted: boolean;
}>;

/**
* Set, update, or remove runtime context entries on a live session.
* Entries are injected as per-turn <system-reminder> blocks on the
* next model call. Passing an empty string for a value removes that key.
*/
setSessionRuntimeContext(
sessionId: string,
entries: Record<string, string>,
context?: BridgeClientRequestContext,
): Promise<{
sessionId: string;
keys: string[];
rejected: Array<{ key: string; reason: string }>;
}>;

/**
* Generate a one-sentence "where did I leave off" recap of a live
* session. Forwards through `qwen/control/session/recap`, which
Expand Down
1 change: 1 addition & 0 deletions packages/acp-bridge/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export const SERVE_CONTROL_EXT_METHODS = {
sessionGoalClear: 'qwen/control/session/goal/clear',
workspaceMcpRuntimeAdd: 'qwen/control/workspace/mcp/runtime-add',
workspaceMcpRuntimeRemove: 'qwen/control/workspace/mcp/runtime-remove',
sessionRuntimeContext: 'qwen/control/session/runtime_context',
workspaceReload: 'qwen/control/workspace/reload',
workspaceExtensionsRefresh: 'qwen/control/workspace/extensions/refresh',
/**
Expand Down
225 changes: 225 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8028,3 +8028,228 @@ describe('deliverClientMcpMessage — reverse tool channel (#5626)', () => {
});
});
});

describe('sessionRuntimeContext handler', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Missing closing braces for the it('returns the parent reply payload on success') block and the enclosing describe('deliverClientMcpMessage ...') block. The new describe('sessionRuntimeContext handler', ...) block was appended directly after the last expect() call without first closing the preceding it and describe blocks. This causes tsc error TS1005 ('}' expected) at EOF (line 8194) and esbuild Unexpected end of file — the entire packages/cli test file fails to compile.

Suggested change
describe('sessionRuntimeContext handler', () => {
});
});
});
describe('sessionRuntimeContext handler', () => {

— qwen3.7-max via Qwen Code /review

let capturedAgentFactory:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] No mixed-bag request test. Each of the 6 existing tests isolates one scenario (all valid, all non-string, all invalid-key, single empty-value deletion, missing sessionId, invalid shape). No test sends a single request combining valid entries, invalid keys, non-string values, and empty-value deletions simultaneously.

A bug that short-circuits the loop on the first rejection (e.g. accidental return instead of continue) or corrupts accumulation state would pass all existing tests. Partial-success behavior is the most common real-world usage pattern.

Suggested test:

it('handles mixed valid, invalid, and delete entries', async () => {
  runtimeCtxMap.set('existing', 'stale');
  const agent = await setupAgent();
  const result = await agent.extMethod(
    SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
    {
      sessionId: 'rt-sid',
      entries: { valid: 'ok', 'bad key!': 'val', num: 42, existing: '' },
    },
  );
  expect(result['keys']).toEqual(['valid', 'existing']);
  expect(result['rejected']).toEqual(
    expect.arrayContaining([
      { key: 'bad key!', reason: 'invalid_key' },
      { key: 'num', reason: 'value_not_string' },
    ]),
  );
});

— qwen3.7-max via Qwen Code /review

| ((conn: { closed: Promise<void> }) => {
initialize: (args: Record<string, unknown>) => Promise<unknown>;
newSession: (args: Record<string, unknown>) => Promise<unknown>;
extMethod: (
method: string,
args: Record<string, unknown>,
) => Promise<Record<string, unknown>>;
})
| undefined;

let processExitSpy: MockInstance<typeof process.exit>;
let stdinDestroySpy: MockInstance<typeof process.stdin.destroy>;
let stdoutDestroySpy: MockInstance<typeof process.stdout.destroy>;

const mockConnectionState = {
promise: undefined as unknown as Promise<void>,
resolve: undefined as unknown as () => void,
reset() {
this.promise = new Promise<void>((r) => {
this.resolve = r;
});
},
};

const runtimeCtxMap = new Map<string, string>();

function makeRuntimeCtxConfig(overrides: Record<string, unknown> = {}) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Test mock re-implements Config validation, creating 3 copies of magic constants

makeRuntimeCtxConfig() duplicates Config.setRuntimeContextEntry's validation logic inline (key regex /^[a-zA-Z0-9_-]{1,64}$/, 32 KiB byte limit, 16-entry capacity). Combined with the handler's own re-validation at acpAgent.ts:6215-6222, there are now 3 copies of each constant.

Handler tests exercise the mock's validation, not the real Config method. If Config changes its regex, both the mock and the handler would silently stay in sync with each other but diverge from Config, and all tests would still pass.

Suggested fix: Use a real Config instance in the test setup for setRuntimeContextEntry, or make the mock delegate to the real method.

— qwen3.7-max via Qwen Code /review

return {
initialize: vi.fn().mockResolvedValue(undefined),
waitForMcpReady: vi.fn().mockResolvedValue(undefined),
getModel: vi.fn().mockReturnValue('m'),
getModelsConfig: vi.fn().mockReturnValue({
getCurrentAuthType: vi.fn().mockReturnValue('api-key'),
syncAfterAuthRefresh: vi.fn(),
}),
reloadModelProvidersConfig: vi.fn(),
refreshAuth: vi.fn().mockResolvedValue(undefined),
getTargetDir: vi.fn().mockReturnValue('/tmp'),
getContentGeneratorConfig: vi.fn().mockReturnValue({}),
getAvailableModels: vi.fn().mockReturnValue([]),
getModes: vi.fn().mockReturnValue([]),
getApprovalMode: vi.fn().mockReturnValue('default'),
getSessionId: vi.fn().mockReturnValue('rt-sid'),
getAuthType: vi.fn().mockReturnValue('api-key'),
getAllConfiguredModels: vi.fn().mockReturnValue([]),
getGeminiClient: vi.fn().mockReturnValue({
isInitialized: vi.fn().mockReturnValue(true),
initialize: vi.fn().mockResolvedValue(undefined),
waitForMcpReady: vi.fn().mockResolvedValue(undefined),
refreshSystemInstruction: vi.fn().mockResolvedValue(undefined),
}),
getFileSystemService: vi.fn().mockReturnValue(undefined),
setFileSystemService: vi.fn(),
getHookSystem: vi.fn().mockReturnValue(undefined),
getDisableAllHooks: vi.fn().mockReturnValue(true),
hasHooksForEvent: vi.fn().mockReturnValue(false),
getWorkspaceContext: vi.fn().mockReturnValue({}),
getDebugMode: vi.fn().mockReturnValue(false),
getRuntimeContext: vi.fn().mockReturnValue(runtimeCtxMap),
setRuntimeContextEntry: vi
.fn()
.mockImplementation((key: string, value: string) => {
if (!/^[a-zA-Z0-9_-]{1,64}$/.test(key)) return false;
if (Buffer.byteLength(value, 'utf8') > 32 * 1024) return false;
if (!runtimeCtxMap.has(key) && runtimeCtxMap.size >= 16) return false;
runtimeCtxMap.set(key, value);
return true;
}),
removeRuntimeContextEntry: vi.fn().mockImplementation((key: string) => {
runtimeCtxMap.delete(key);
}),
...overrides,
};
}

beforeEach(() => {
vi.clearAllMocks();
runtimeCtxMap.clear();
mockConnectionState.reset();
capturedAgentFactory = undefined;

vi.mocked(AgentSideConnection).mockImplementation((factory: unknown) => {
capturedAgentFactory = factory as typeof capturedAgentFactory;
return {
get closed() {
return mockConnectionState.promise;
},
} as unknown as InstanceType<typeof AgentSideConnection>;
});

processExitSpy = vi
.spyOn(process, 'exit')
.mockImplementation((() => undefined) as unknown as typeof process.exit);
stdinDestroySpy = vi
.spyOn(process.stdin, 'destroy')
.mockImplementation(() => process.stdin);
stdoutDestroySpy = vi
.spyOn(process.stdout, 'destroy')
.mockImplementation(() => process.stdout);
});

afterEach(() => {
mockConnectionState.resolve();
processExitSpy.mockRestore();
stdinDestroySpy.mockRestore();
stdoutDestroySpy.mockRestore();
});

async function setupAgent() {
const cfg = makeRuntimeCtxConfig();

vi.mocked(loadSettings).mockReturnValue({
merged: { mcpServers: {} },
getUserHooks: vi.fn().mockReturnValue({}),
getProjectHooks: vi.fn().mockReturnValue({}),
} as unknown as LoadedSettings);

vi.mocked(loadCliConfig).mockResolvedValue(cfg as unknown as Config);

vi.mocked(Session).mockImplementation(
() =>
({
getId: vi.fn().mockReturnValue('rt-sid'),
getConfig: vi.fn().mockReturnValue(cfg),
sendAvailableCommandsUpdate: vi.fn().mockResolvedValue(undefined),
installRewriter: vi.fn(),
startCronScheduler: vi.fn(),
dispose: vi.fn(),
}) as unknown as Session,
);

vi.mocked(buildAvailableCommandsSnapshot).mockResolvedValue({
availableCommands: [],
availableSkills: [],
});

const bootConfig = makeRuntimeCtxConfig();
runAcpAgent(
bootConfig as unknown as Config,
{ merged: { mcpServers: {} } } as unknown as LoadedSettings,
{} as CliArgs,
);
await vi.waitFor(() => expect(capturedAgentFactory).toBeDefined());
const agent = capturedAgentFactory!({
closed: mockConnectionState.promise,
});
await agent.initialize({});
await agent.newSession({ cwd: '/tmp', mcpServers: [] });
return agent;
}

it('sets entries and returns applied keys', async () => {
const agent = await setupAgent();
const result = await agent.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
{
sessionId: 'rt-sid',
entries: { operator: 'Alice', rules: 'no-prod' },
},
);
expect(result['keys']).toEqual(['operator', 'rules']);
expect(result['rejected']).toEqual([]);
expect(runtimeCtxMap.get('operator')).toBe('Alice');
});

it('removes entries with empty string values', async () => {
runtimeCtxMap.set('old-key', 'stale');
const agent = await setupAgent();
const result = await agent.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
{ sessionId: 'rt-sid', entries: { 'old-key': '' } },
);
expect(result['keys']).toEqual(['old-key']);
expect(runtimeCtxMap.has('old-key')).toBe(false);
});

it('rejects non-string values', async () => {
const agent = await setupAgent();
const result = await agent.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
{ sessionId: 'rt-sid', entries: { bad: 123 } },
);
expect(result['keys']).toEqual([]);
expect(
(result['rejected'] as Array<{ key: string; reason: string }>)[0],
).toEqual({
key: 'bad',
reason: 'value_not_string',
});
});

it('rejects invalid keys', async () => {
const agent = await setupAgent();
const result = await agent.extMethod(
SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext,
{ sessionId: 'rt-sid', entries: { 'invalid key!': 'value' } },
);
expect(result['keys']).toEqual([]);
expect(
(result['rejected'] as Array<{ key: string; reason: string }>)[0]?.reason,
).toBe('invalid_key');
});

it('throws on missing sessionId', async () => {
const agent = await setupAgent();
await expect(
agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext, {
entries: { a: 'b' },
}),
).rejects.toThrow(/sessionId/);
});

it('throws on invalid entries shape', async () => {
const agent = await setupAgent();
await expect(
agent.extMethod(SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext, {
sessionId: 'rt-sid',
entries: 'not-an-object',
}),
).rejects.toThrow(/entries/);
});
});
49 changes: 49 additions & 0 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6175,6 +6175,55 @@ class QwenAgent implements Agent {

return { language: resolvedLanguage, outputLanguage, refreshed };
}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 0660660. Added 6 handler tests following the sessionLanguage pattern: happy-path set, empty-string removal, non-string value rejection, invalid key rejection, missing sessionId error, and invalid entries shape error. Tests use a mock Config with a real Map<string, string> backing store to verify state changes.

case SERVE_CONTROL_EXT_METHODS.sessionRuntimeContext: {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d594328. The handler now returns { keys: appliedKeys, rejected: [...] } where each rejected entry includes { key, reason }. Non-string values get reason: 'value_not_string', and failed Config validations get reason: 'invalid_key_or_value'. Callers can now distinguish successful from rejected entries.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed — 6 handler tests added in commit 0660660, covering: happy-path set, empty-string removal, non-string rejection, invalid key rejection, missing sessionId, and invalid entries shape. See also my reply to the later duplicate of this comment.

const sessionId = params['sessionId'];
const entries = params['entries'];
if (typeof sessionId !== 'string' || sessionId.length === 0) {
throw RequestError.invalidParams(
undefined,
'Invalid or missing sessionId',
);
}
if (
typeof entries !== 'object' ||
entries === null ||
Array.isArray(entries)
) {
throw RequestError.invalidParams(
undefined,
'`entries` must be a non-null object',
);
}
const session = this.sessionOrThrow(sessionId);
const config = session.getConfig();
const appliedKeys: string[] = [];
const rejected: Array<{ key: string; reason: string }> = [];
for (const [key, value] of Object.entries(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Processing the batch strictly in object order makes capacity handling depend on caller key order. If the store already has 16 entries and a caller sends { newKey: 'v', oldKey: '' }, newKey is rejected as capacity_full before oldKey is removed; the same logical update succeeds if the deletion appears first. Apply empty-string removals before non-empty sets, or compute capacity against the post-removal store, so a single batch has deterministic semantics.

— GPT-5 via Qwen Code /review

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Batch entry processing order makes capacity-full behavior non-deterministic and surprising. If a caller sends { newKey: 'v', oldKey: '' } when the store is already at the 16-entry limit, newKey is processed first and rejected as capacity_full before oldKey: '' is processed to free a slot. Reordering the keys in the same batch (e.g. { oldKey: '', newKey: 'v' }) succeeds. A single logical atomic update — swap one entry — either works or fails purely based on JS object key enumeration order, which is insertion-order for string keys. This produces inconsistent results for callers who cannot predict Object.entries order.

Suggested change
for (const [key, value] of Object.entries(
Apply all empty-string removals first, then process non-empty insertions:
```ts
const toRemove = Object.entries(entries as Record<string, unknown>).filter(([, v]) => typeof v === 'string' && v === '');
const toSet = Object.entries(entries as Record<string, unknown>).filter(([, v]) => typeof v === 'string' && v !== '');
const nonString = Object.entries(entries as Record<string, unknown>).filter(([, v]) => typeof v !== 'string');
for (const [key] of nonString) rejected.push({ key, reason: 'value_not_string' });
for (const [key] of toRemove) {
if (config.getRuntimeContext().has(key)) { config.removeRuntimeContextEntry(key); appliedKeys.push(key); }
}
for (const [key, value] of toSet) {
// ... set/reject logic
}

_— claude-sonnet-4-6 via Qwen Code /review_

---
_Generated by [Claude Code](https://claude.ai/code)_

entries as Record<string, unknown>,
)) {
if (typeof value !== 'string') {
rejected.push({ key, reason: 'value_not_string' });
continue;
}
if (value === '') {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in d594328. The removal path now checks config.getRuntimeContext().has(key) before calling removeRuntimeContextEntry — only existing keys are removed and reported as applied. Non-existing keys are silently ignored (no rejected entry needed since removing a non-existent key is a no-op).

if (config.getRuntimeContext().has(key)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Silent no-op removal. When value === '' and getRuntimeContext().has(key) is false (this line), the key is silently skipped — it appears in neither appliedKeys nor rejected. The caller receives no signal that the key did not exist.

Suggested: either always push to appliedKeys (idempotent delete), or report in rejected:

if (value === '') {
  config.removeRuntimeContextEntry(key);
  appliedKeys.push(key); // idempotent: always report as applied
}

config.removeRuntimeContextEntry(key);
appliedKeys.push(key);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 2dc0034 — the removal path now checks config.getRuntimeContext().has(key) before calling removeRuntimeContextEntry. Non-existing keys are silently skipped (no-op removal doesn't need to appear in either keys or rejected).

}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] When value === '' (deletion semantics) and the key does NOT currently exist in the store, the handler silently skips it — the key appears in neither keys nor rejected. A caller issuing { oldKey: '' } receives { keys: [], rejected: [] } and cannot distinguish "key was already absent (idempotent no-op)" from "request was lost". Consider making deletion idempotent: always push to appliedKeys when value === '', since the caller's desired end-state (key absent) is already satisfied.

Suggested change
}
if (value === '') {
if (config.getRuntimeContext().has(key)) {
config.removeRuntimeContextEntry(key);
}
appliedKeys.push(key);
} else if (config.setRuntimeContextEntry(key, value)) {

— qwen3.7-max via Qwen Code /review

} else if (config.setRuntimeContextEntry(key, value)) {
appliedKeys.push(key);
} else if (!/^[a-zA-Z0-9_-]{1,64}$/.test(key)) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] The rejection-reason cascade re-implements Config's internal validation checks using inlined literals (/^[a-zA-Z0-9_-]{1,64}$/ and 32 * 1024) that duplicate Config.RUNTIME_CONTEXT_KEY_RE and Config.RUNTIME_CONTEXT_MAX_VALUE_BYTES (both private static readonly). Because the cascade runs AFTER setRuntimeContextEntry already applied Config's constants, if Config's key pattern or byte limit ever changes, the handler will report the wrong rejection reason — e.g., reporting invalid_key for a key that Config actually accepted (or vice versa), and value_too_large for a size that Config no longer enforces. The constants are not exported, so there is no single source of truth.

Suggested change
} else if (!/^[a-zA-Z0-9_-]{1,64}$/.test(key)) {
Change `Config.setRuntimeContextEntry` to return a discriminated result rather than a bare boolean:
```ts
// config.ts
setRuntimeContextEntry(key: string, value: string): { ok: true } | { ok: false; reason: 'invalid_key' | 'value_too_large' | 'capacity_full' } {
if (!Config.RUNTIME_CONTEXT_KEY_RE.test(key)) return { ok: false, reason: 'invalid_key' };
// ...
if (byteLen > Config.RUNTIME_CONTEXT_MAX_VALUE_BYTES) return { ok: false, reason: 'value_too_large' };
if (!entries.has(key) && entries.size >= Config.RUNTIME_CONTEXT_MAX_ENTRIES) return { ok: false, reason: 'capacity_full' };
entries.set(key, value);
return { ok: true };
}
// acpAgent.ts handler
const result = config.setRuntimeContextEntry(key, value);
if (result.ok) {
appliedKeys.push(key);
} else {
rejected.push({ key, reason: result.reason });
}

This eliminates the duplicated literals and makes the rejection reason authoritative from Config.


_— claude-sonnet-4-6 via Qwen Code /review_

---
_Generated by [Claude Code](https://claude.ai/code)_

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rejection-reason classification re-implements Config's validation constants (/^[a-zA-Z0-9_-]{1,64}$/ and 32 * 1024) that already exist as Config.RUNTIME_CONTEXT_KEY_RE / RUNTIME_CONTEXT_MAX_VALUE_BYTES. If those limits change in config.ts, this handler will silently misclassify the reason (e.g. report capacity_full for a value that is actually too large, or invalid_key under a widened regex). Consider having setRuntimeContextEntry return a typed reason instead of a boolean so there's a single source of truth for both the accept/reject decision and its cause.

rejected.push({ key, reason: 'invalid_key' });
} else if (
Buffer.byteLength(value, 'utf8') > 32 * 1024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Key regex /^[a-zA-Z0-9_-]{1,64}$/ and 32 * 1024 are hardcoded inline, duplicating Config.RUNTIME_CONTEXT_KEY_RE and RUNTIME_CONTEXT_MAX_VALUE_BYTES. If Config constants change, this handler's validation silently diverges.

Consider exporting the constants from Config or adding a Config.classifyRuntimeContextRejection(key, value) method that returns the reason string, so the handler doesn't re-derive it.

— qwen3.7-max via Qwen Code /review

) {
rejected.push({ key, reason: 'value_too_large' });

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Untested rejection branches. The 6 handler tests cover value_not_string and invalid_key rejections, but value_too_large (this line) and capacity_full (L6222) have zero test coverage.

If the rejection classification logic has a bug (wrong reason string, misordered checks), it silently returns incorrect rejected metadata to callers.

Suggested tests:

it('rejects values exceeding 32 KiB', async () => {
  // ...entries: { big: 'x'.repeat(32 * 1024 + 1) }
  // expect rejected: [{ key: 'big', reason: 'value_too_large' }]
});

it('rejects new keys when capacity is full', async () => {
  // pre-fill 16 entries, then try a 17th
  // expect rejected: [{ key: 'overflow', reason: 'capacity_full' }]
});

} else {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 4af5086 — the handler now returns granular rejection reasons: invalid_key, value_too_large, or capacity_full instead of the generic invalid_key_or_value.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Critical] Three rejection branches lack test coverage: value_too_large (this line), capacity_full (line 6222), and Array.isArray(entries) handler-level check. These are part of the public API contract — SDK clients depend on exact reason strings ('value_too_large', 'capacity_full') for error handling.

Suggested tests:

  • Fill 16 entries, attempt 17th → assert rejected[0].reason === 'capacity_full'
  • Send 32 KiB+ value → assert rejected[0].reason === 'value_too_large'
  • Send array input → assert throws

— qwen3.7-max via Qwen Code /review

rejected.push({ key, reason: 'capacity_full' });
}
}
return { keys: appliedKeys, rejected };
}
case SERVE_CONTROL_EXT_METHODS.sessionRecap: {
// Generate a one-sentence "where did I leave off" summary.
// Best-effort: returns `null` on short history or model failure.
Expand Down
1 change: 1 addition & 0 deletions packages/cli/src/serve/capabilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ export const SERVE_CAPABILITY_REGISTRY = {
writer_idle_timeout: { since: 'v1' },
non_blocking_prompt: { since: 'v1' },
session_language: { since: 'v1' },
session_runtime_context: { since: 'v1' },
session_rewind: { since: 'v1' },
workspace_hooks: { since: 'v1' },
session_hooks: { since: 'v1' },
Expand Down
Loading
Loading