Skip to content

Commit 5993a12

Browse files
benbrandtbenbrandt
authored
feat: support append-mode ACP system prompts (spacering-net#653)
## Summary - Advertise and implement standard, unstable `systemPrompt` support on ACP v1 and v2 session lifecycle requests. Support text-only **append** mode; reject override, custom modes, and non-text content before lifecycle side effects. - Preserve AIR's existing instruction baseline. Append standard client text after AIR `customInstructions` when present, otherwise after configured Codex developer instructions. Leave Codex base instructions and workspace context intact. Requests without standard prompts retain legacy AIR behavior. - Carry known instruction state through new/resume/load/fork and through provider restarts/app-server recovery. Omitted/null prompts preserve known instructions while the adapter session remains open; clients should resend after closing/restarting the adapter. - For changed prompts on an idle, persisted thread, detach/reload under the same ID. Verify an actual unload instead of acknowledging an override Codex silently ignored. Check persisted history (a planned rollout path alone is not proof), block racing inputs/config changes, and restore delivery after rejected/failed changes. - Do not interrupt foreground work or background terminals, force compaction, restart the global app-server, rewrite history, or repurpose collaboration-mode instructions. Standard subagent support remains separate; the AIR subagent contract is unchanged. ## Timing and limits **New sessions apply the prompt on their first turn. Changed prompts on existing sessions and forks become model-visible at the next compaction.** Reload updates configuration but retained history still contains the previous developer message until context is rebuilt. This is the deferral allowed by the ACP system-prompt specification, similar to the Claude adapter's documented behavior. Changed prompts on busy sessions, sessions with background terminals, or live sessions without a saved rollout are explicitly rejected with `-32602`; finish the work and retry, or create a new session for immediate instructions. A runtime that keeps the existing thread loaded also produces an error rather than false success. Busy-session changes are not automatically queued in this package. ## Validation - `npm run typecheck` — passed, including examples. - `npm test` — **1,859 passed**, 36 skipped; includes 38 system-prompt wire tests across v1/v2, concurrency guards, runtime refusal, restoration after failure, empty-content clearing, and provider-restart preservation. - `npm run build` — passed. - `npm run test:integration` — **7 passed** against real Codex app-server with a local mock model. New v1/v2 tests inspect actual developer messages for new, changed resume, fork, full-history replay, and compaction, preserving AIR and configured baselines and keeping rejected unused sessions usable. - `npm run codex-test` live-model legacy smoke — passed. - Additional live-model ACP smoke: standard system prompt specified `system-prompt-live-ok`, user prompt was `Greet me.`, and the model returned exactly the system-prompt marker with `end_turn`. - `git diff --check` — passed. - Verified all 33 AIR scenario snapshot changes add only the `systemPrompt` capability. The pinned compatibility baselines are unchanged; their allowed feature-delta rule accounts for the new advertisement. The full live-model E2E suite was not run. Specification checked at `agent-client-protocol@4034a8cbc99bc48fd4579f70a3407b07907e0eb6`; backend behavior verified on Codex 0.160.1. ## Follow-up observed during testing The existing `/compact` command can answer on compaction-item completion shortly before Codex releases the underlying turn. The new integration fixture waits for the separate runtime turn completion before issuing the next probe. Changing that command's completion semantics is outside this package.
1 parent 5e9b7c5 commit 5993a12

55 files changed

Lines changed: 815 additions & 64 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎readme-dev.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,30 @@ v1 `modes` field (v2 removed the modes API in favor of config options), and fork
152152
get an `available_commands_update`, matching v1. There are no new environment/config knobs specific
153153
to v2; it reuses the runtime environment variables above.
154154

155+
#### Client system prompts
156+
157+
Both ACP versions advertise `systemPrompt: {}`: text-only `append` mode. Override and non-text
158+
content are rejected with `-32602` before a session is created or a fork is made.
159+
Text blocks are joined with a blank line and appended to the existing developer-instruction
160+
baseline: AIR `customInstructions` when supplied, otherwise configured Codex developer instructions.
161+
Codex's base instructions and workspace context remain intact. Requests without `systemPrompt`
162+
keep the legacy AIR behavior.
163+
164+
New sessions use the instructions on their first turn. A changed prompt on an idle, persisted
165+
session detaches and reloads that thread under the same session ID. **The retained model context
166+
changes at the next compaction**, not immediately after resume; the adapter does not force a
167+
compaction. Forks with changed instructions have the same deferred behavior.
168+
169+
Changed prompts are rejected while session work or background terminals are running, when a live
170+
session has no saved rollout yet, or when Codex cannot confirm a reload. Existing work is not
171+
cancelled. Retry after work finishes, or create a new session for immediate instructions.
172+
New prompts, steering, and config changes cannot race an instruction reload.
173+
174+
Omitted/null prompts keep the instructions known to the currently open adapter session, including
175+
through app-server recovery and provider restarts. After closing a session or restarting the
176+
adapter, clients should resend their prompt on load/resume/fork; without it, Codex's retained
177+
context and configured defaults determine the instructions.
178+
155179
#### AIR v2 client contract
156180

157181
These are behaviors the AIR (JetBrains) ACP client relies on that aren't obvious from the wire

‎src/ClientSystemPrompt.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import {RequestError, type SystemPrompt} from "@agentclientprotocol/sdk/unstable";
2+
import {airCustomInstructions} from "./AirExtension";
3+
4+
/** Instructions to resend when a thread is reopened, including after an app-server restart. */
5+
export type SessionInstructions = {
6+
text?: string;
7+
airInstructions?: string;
8+
developerInstructions: string;
9+
};
10+
11+
export function clientSystemPromptText(value: unknown): string | undefined {
12+
if (value == null) return undefined;
13+
if (typeof value !== "object" || Array.isArray(value)
14+
|| !("mode" in value) || value.mode !== "append"
15+
|| !("content" in value) || !Array.isArray(value.content)) {
16+
throw RequestError.invalidParams(undefined, "systemPrompt requires append mode and text content");
17+
}
18+
const blocks: string[] = [];
19+
for (const block of value.content) {
20+
if (block === null || typeof block !== "object" || block.type !== "text" || typeof block.text !== "string") {
21+
throw RequestError.invalidParams(undefined, "systemPrompt content must contain only text blocks");
22+
}
23+
blocks.push(block.text);
24+
}
25+
return blocks.join("\n\n");
26+
}
27+
28+
/** The text-only subset has the same wire shape on both ACP versions. */
29+
export function toV1SystemPrompt(value: unknown): SystemPrompt | null {
30+
const text = clientSystemPromptText(value);
31+
return text === undefined ? null : {mode: "append", content: [{type: "text", text}]};
32+
}
33+
34+
export function changesSystemPrompt(
35+
request: {systemPrompt?: unknown; _meta?: unknown},
36+
previous: SessionInstructions | undefined,
37+
): boolean {
38+
const text = clientSystemPromptText(request.systemPrompt);
39+
const air = airCustomInstructions(request._meta);
40+
return text !== undefined && (text !== previous?.text
41+
|| (air !== undefined && air !== previous?.airInstructions));
42+
}

‎src/CodexAcpClient.ts‎

Lines changed: 115 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -68,11 +68,13 @@ import {forkSession as runForkSession} from "./SessionFork";
6868
import type {SessionMetadata, SessionMetadataWithThread} from "./SessionMetadata";
6969
import {
7070
isMissingRolloutError,
71+
isThreadNotMaterializedError,
7172
isThreadActiveWriterError,
7273
isUnknownThreadError,
7374
threadActiveWriterRequestError,
7475
} from "./CodexThreadErrors";
7576
import {airCustomInstructions} from "./AirExtension";
77+
import {changesSystemPrompt, clientSystemPromptText, type SessionInstructions} from "./ClientSystemPrompt";
7678
import {
7779
GatewayListingError,
7880
type GatewayModelIds,
@@ -680,18 +682,24 @@ export class CodexAcpClient {
680682
}
681683
}
682684

683-
async resumeSession(request: WithAcpMcpServers<acp.ResumeSessionRequest>, onSubscribed?: () => void): Promise<SessionMetadata> {
685+
async resumeSession(
686+
request: WithAcpMcpServers<acp.ResumeSessionRequest>,
687+
onSubscribed?: () => void,
688+
previousInstructions?: SessionInstructions,
689+
): Promise<SessionMetadata> {
690+
const instructions = await this.resolveSessionInstructions(request, previousInstructions);
684691
const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta);
685692
await this.refreshSkills(request.cwd, additionalDirectories);
686693

687694
const sessionConfig = await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
688-
const response = await this.resumeThread({
695+
const response = await this.resumeThreadWithInstructions({
689696
excludeTurns: true,
690697
config: sessionConfig.config,
691698
cwd: request.cwd,
692699
modelProvider: await this.getResumeModelProvider(),
693700
threadId: request.sessionId,
694-
});
701+
...(instructions?.text !== undefined ? {developerInstructions: instructions.developerInstructions} : {}),
702+
}, request, previousInstructions);
695703
onSubscribed?.();
696704
const codexModels = await this.fetchAvailableModels();
697705
const currentModelId = this.createModelId(codexModels, response.model, response.reasoningEffort).toString();
@@ -706,14 +714,17 @@ export class CodexAcpClient {
706714
currentServiceTier: response.serviceTier as ServiceTier ?? null,
707715
additionalDirectories,
708716
skippedMcpServers: sessionConfig.skippedMcpServers,
717+
...(instructions ? {instructions} : {}),
709718
}
710719
}
711720

712721
async forkSession(
713722
request: WithAcpMcpServers<acp.ForkSessionRequest>,
714723
releaseFailedFork: (threadId: string) => Promise<void> = threadId => this.closeSession(threadId),
715724
forkCreated: (threadId: string) => void = () => {},
725+
previousInstructions?: SessionInstructions,
716726
): Promise<SessionMetadata> {
727+
const instructions = await this.resolveSessionInstructions(request, previousInstructions);
717728
const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta);
718729
return await runForkSession(request, additionalDirectories, {
719730
codexClient: this.codexClient,
@@ -727,21 +738,27 @@ export class CodexAcpClient {
727738
getCollaborationMode: sessionId => this.getCollaborationMode(sessionId),
728739
releaseFailedFork,
729740
forkCreated,
730-
});
741+
}, instructions);
731742
}
732743

733-
async loadSession(request: WithAcpMcpServers<acp.LoadSessionRequest>, onSubscribed?: () => void): Promise<SessionMetadataWithThread> {
744+
async loadSession(
745+
request: WithAcpMcpServers<acp.LoadSessionRequest>,
746+
onSubscribed?: () => void,
747+
previousInstructions?: SessionInstructions,
748+
): Promise<SessionMetadataWithThread> {
749+
const instructions = await this.resolveSessionInstructions(request, previousInstructions);
734750
const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta);
735751
await this.refreshSkills(request.cwd, additionalDirectories);
736752

737753
const sessionConfig = await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
738-
const response = await this.resumeThread({
754+
const response = await this.resumeThreadWithInstructions({
739755
excludeTurns: true,
740756
config: sessionConfig.config,
741757
cwd: request.cwd,
742758
modelProvider: await this.getResumeModelProvider(),
743759
threadId: request.sessionId,
744-
});
760+
...(instructions?.text !== undefined ? {developerInstructions: instructions.developerInstructions} : {}),
761+
}, request, previousInstructions);
745762
onSubscribed?.();
746763
// Resume cursors bound durable history; later turns arrive through live events.
747764
// A null paginated cursor means there was no durable history at resume time.
@@ -779,6 +796,7 @@ export class CodexAcpClient {
779796
history,
780797
additionalDirectories,
781798
skippedMcpServers: sessionConfig.skippedMcpServers,
799+
...(instructions ? {instructions} : {}),
782800
};
783801
}
784802

@@ -809,8 +827,9 @@ export class CodexAcpClient {
809827
}
810828

811829
async newSession(request: WithAcpMcpServers<acp.NewSessionRequest>): Promise<SessionMetadata> {
830+
const instructions = await this.resolveSessionInstructions(request, undefined, true);
812831
const additionalDirectories = readAdditionalDirectories(request.cwd, request.additionalDirectories, request._meta);
813-
const developerInstructions = airCustomInstructions(request._meta);
832+
const developerInstructions = instructions?.developerInstructions;
814833
await this.refreshSkills(request.cwd, additionalDirectories);
815834

816835
const sessionConfig = await this.createSessionConfig(request.cwd, additionalDirectories, request.mcpServers ?? []);
@@ -835,9 +854,97 @@ export class CodexAcpClient {
835854
currentServiceTier: response.serviceTier as ServiceTier ?? null,
836855
additionalDirectories,
837856
skippedMcpServers: sessionConfig.skippedMcpServers,
857+
...(instructions ? {instructions} : {}),
858+
};
859+
}
860+
861+
private async resolveSessionInstructions(
862+
request: {cwd: string; systemPrompt?: unknown; _meta?: unknown},
863+
previous?: SessionInstructions,
864+
creating = false,
865+
): Promise<SessionInstructions | undefined> {
866+
const text = clientSystemPromptText(request.systemPrompt);
867+
if (text === undefined) {
868+
if (!creating) return previous;
869+
const airInstructions = airCustomInstructions(request._meta);
870+
return airInstructions === undefined ? undefined : {airInstructions, developerInstructions: airInstructions};
871+
}
872+
if (!changesSystemPrompt(request, previous)) return previous;
873+
// AIR already replaces configured developer instructions. Append to that same baseline.
874+
const airInstructions = airCustomInstructions(request._meta) ?? previous?.airInstructions;
875+
const baseline = airInstructions ?? this.config["developer_instructions"]
876+
?? (await this.codexClient.configRead({cwd: request.cwd, includeLayers: false}))?.config?.developer_instructions
877+
?? "";
878+
if (typeof baseline !== "string") {
879+
throw acp.RequestError.invalidParams(undefined, "Configured developer instructions must be text");
880+
}
881+
return {
882+
text,
883+
...(airInstructions !== undefined ? {airInstructions} : {}),
884+
developerInstructions: [baseline, text].filter(part => part.length > 0).join("\n\n"),
838885
};
839886
}
840887

888+
private async resumeThreadWithInstructions(
889+
params: ThreadResumeParams,
890+
request: {sessionId: string; systemPrompt?: unknown; _meta?: unknown},
891+
previous?: SessionInstructions,
892+
): Promise<ResumedThread> {
893+
if (!changesSystemPrompt(request, previous)) return this.resumeThread(params);
894+
const {thread} = await this.codexClient.threadRead({threadId: request.sessionId});
895+
if (thread.status.type === "notLoaded") return this.resumeThread(params);
896+
if (thread.status.type !== "idle" || thread.path === null) {
897+
throw acp.RequestError.invalidParams(
898+
undefined,
899+
"Changing systemPrompt requires an idle, persisted session; finish the turn or create a new session",
900+
);
901+
}
902+
const terminals = await this.codexClient.threadBackgroundTerminalsList({threadId: request.sessionId, limit: 1});
903+
if (terminals.data.length > 0) {
904+
throw RequestError.invalidParams(undefined, "Cannot change systemPrompt while background terminals are running");
905+
}
906+
// A start response contains a planned rollout path even before the file exists.
907+
try {
908+
await this.codexClient.threadTurnsList({threadId: request.sessionId, limit: 1, itemsView: "notLoaded"});
909+
} catch (error) {
910+
if (!isMissingRolloutError(error) && !isThreadNotMaterializedError(error)) throw error;
911+
throw RequestError.invalidParams(undefined, "Changing systemPrompt requires a persisted session; create a new session instead");
912+
}
913+
// Subscribed threads ignore resume overrides. Idle, detached threads can be rebuilt.
914+
// Retained developer messages are replaced at the next compaction.
915+
let unloaded = false;
916+
let detached = false;
917+
let resumed = false;
918+
const release = this.codexClient.onClientTransportEvent(event => {
919+
if (event.eventType === "notification" && event.method === "thread/status/changed"
920+
&& event.params.threadId === request.sessionId && event.params.status.type === "notLoaded") {
921+
unloaded = true;
922+
}
923+
});
924+
try {
925+
await this.codexClient.threadUnsubscribe({threadId: request.sessionId});
926+
detached = true;
927+
const response = await this.resumeThread(params);
928+
resumed = true;
929+
if (!unloaded) {
930+
throw RequestError.invalidParams(
931+
undefined,
932+
"Codex kept the existing thread loaded; systemPrompt could not be changed. Retry when idle",
933+
);
934+
}
935+
return response;
936+
} catch (error) {
937+
if (detached && !resumed) {
938+
await this.resumeThread({threadId: request.sessionId, excludeTurns: true}).catch(restoreError => {
939+
logger.error(`Failed to restore session ${request.sessionId} after an instruction update`, restoreError);
940+
});
941+
}
942+
throw error;
943+
} finally {
944+
release();
945+
}
946+
}
947+
841948
async closeSession(sessionId: string): Promise<void> {
842949
try {
843950
await this.codexClient.threadUnsubscribe({threadId: sessionId});

0 commit comments

Comments
 (0)