feat(acp): experimental ACP agent mode (Codex) via the Agent Client Protocol#380
Draft
pjdoland wants to merge 4 commits into
Draft
feat(acp): experimental ACP agent mode (Codex) via the Agent Client Protocol#380pjdoland wants to merge 4 commits into
pjdoland wants to merge 4 commits into
Conversation
…col (plmbr#378) Add an opt-in Codex (OpenAI) agent mode that drives the chat panel over the Agent Client Protocol (ACP), the way Claude mode drives it today: streaming replies, tool-call cards with diffs, and per-tool approval. An NBI MCP tool is invoked end to end to prove the path. - ACP client backend (acp_agent.py) on a persistent worker thread, mapping ACP events onto NBI's chat surfaces, with single-flight turns and a hardened subprocess lifecycle. A small stdio MCP server (acp_mcp_server.py) exposes an NBI tool to the agent. - codex_settings + a codex_mode admin policy that defaults to force-off, plus OPENAI_API_KEY / OPENAI_BASE_URL / NBI_CODEX_CHAT_MODEL overrides, following the existing seven-place policy and settings-override conventions. - Frontend: a Codex settings tab (enable, model, key, base URL) mirroring the Claude tab, and chat-panel awareness so the input and routing follow Codex. - An in-chat agent picker (shown when more than one agent mode is enabled) that resolves a single active agent from the enabled modes and a persisted preference; with no preference the historical Claude-wins default holds. The picker is a custom icon dropdown, and the agent footer is unified (Ask/Agent and tools stay on the native model path; agent modes show only their badge). - Consistent branding: the OpenAI mark for the Codex participant and tab. - Admin gating parity with Claude: a codex_full_access policy (default force-off) pins Codex to approval_policy=untrusted so it asks before risky actions, clamped server-side; user-choice exposes a "Full access" toggle that flips the launch to approval_policy=never. README and admin-guide document the controls, the CODEX_HOME isolation, and the residual precedence caveat. Unit tests cover the ACP-to-NBI mapping, approval, the policy clamps, the active-agent resolution, the approval-arg mapping, and the credential scrub. The live path was verified in a running JupyterLab.
6c12439 to
f842fdf
Compare
Collaborator
|
hi @pjdoland thanks, I started doing some testing.
- New session button and session history are not available in Codex mode
|
plmbr#378) Response to the PR plmbr#380 review feedback. Rename Codex mode to ACP mode with selectable agent types. The mode is named after the protocol, not the first agent: acp_settings gains an `agent` field backed by a registry (acp_registry.py, Codex today; Pi and OpenCode slot in once their adapters are validated), and the chat header and message avatars keep showing the selected agent's name and icon. All policy plumbing follows: acp_mode / acp_full_access policies, NBI_ACP_* env vars, and the acp_* setting locks. Claude mode and ACP mode are now mutually exclusive. Enabling one turns the other off at the settings boundary (most-recent selection wins, Claude wins ties per AGENT_MODE_PRIORITY), so the chat-header agent dropdown and the active_chat_agent preference are gone. Auto-disabling Claude also disconnects its live SDK client; the isinstance-gated disconnect at the end of the handler would otherwise skip because the default participant has already switched to ACP. Fix streamed replies rendering one word per line. ACP delivers token-sized deltas; each was streamed as MarkdownData, which the sidebar renders as its own paragraph. MarkdownPartData concatenates consecutive parts into one block. Fix attached files never reaching the agent. The websocket handler appends the turn's context lines (attachments, current-file pointer, output context) to the chat history exactly like Claude mode, but the ACP client sent only request.prompt. assemble_query now joins the turn's user-role lines, and extension.py gives ACP mode the same context framing and per-turn history slicing as Claude mode (is_agent_session_mode). Add new-session and session history. The header's new-chat button now shows in ACP mode and starts a fresh session on the running agent (ClearChatHistory), serialized against turns via the turn lock. The history button lists the agent's own stored sessions through ACP session/list and resumes via session/load (capability-gated), with NBI context preambles stripped from previews. A timed-out load forces a client restart so a delayed session swap cannot leak a replayed conversation into a later turn. Verified live in JupyterLab against codex-acp 0.16.0 with a real OpenAI key: single-paragraph rendering, attachment read via the agent's own tool, fresh-session isolation, resume recall, and the settings-panel exclusivity flip.
Live verification caught the session picker still showing "Additional context: Current directory open in Jupyter is: ..." as previews: codex stores session titles with newlines collapsed to spaces (and truncated), so the line-based strip never matched. Peel the directory pointer off the joined form structurally by its quoted segments, keeping the line-based path for agents that preserve newlines.
Collaborator
Author
|
Thanks for testing, Mehmet, this was really helpful feedback. All five items are addressed in the latest push:
All verified live against codex-acp with an OpenAI API key (rendering, attachment read, new-session isolation, resume recall, and the settings exclusivity flip). The PR description is updated to match. Happy to adjust anything. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
This adds an opt-in, experimental ACP agent mode that drives the chat panel over the Agent Client Protocol, the way Claude mode drives it today, with Codex (OpenAI) as the first selectable agent type. It is off by default and gated behind an admin policy that defaults to force-off, so it has no effect unless explicitly enabled. Claude mode and ACP mode are mutually exclusive: enabling one turns the other off.
The goal of this first pass is to prove that an external coding agent can run the chat loop end to end (streaming replies, tool-call cards with diffs, per-tool approval) and invoke an NBI MCP tool, using ACP as the integration layer.
What's included
acp_agent.py): a persistent worker thread that launches the agent's ACP adapter and maps ACP events onto NBI's existing chat surfaces (markdown, tool-call cards, diffs, approvals). Turns are single-flight and the subprocess lifecycle is hardened. A small stdio MCP server (acp_mcp_server.py) exposes an NBI tool to the agent so the end-to-end tool path is exercised.acp_registry.py): agent types are registry entries (adapter package, display metadata, auth convention). Codex is the first entry; further agents (Pi, OpenCode) slot in once their adapters are validated. The chat header and message avatars show the selected agent's name and icon.session/list/session/load(capability-gated), mirroring the Claude-mode UX.acp_settingsblock (with an agent-type dropdown) and anacp_modeadmin policy that defaults to force-off, plusOPENAI_API_KEY/OPENAI_BASE_URL/NBI_ACP_CHAT_MODELoverrides, following the existing policy and settings-override conventions. Mutual exclusivity with Claude mode is enforced at the settings boundary (most-recent selection wins; a hand-edited config that enables both resolves to Claude).acp_full_accesspolicy (default force-off) pins Codex toapproval_policy=untrustedso it asks before anything beyond trusted read-only commands, clamped server-side on read and write.user-choiceexposes a "Full access" toggle that flips the launch toapproval_policy=never. Documented in the README admin table and a new admin-guide section.Testing
acp_mode/acp_full_accesspolicy clamps, the mode-exclusivity resolution, the query assembly (attachment context), the approval-arg mapping, the session-preview cleanup, and the credential scrub.Risks / follow-ups
session/listis an optional protocol extension (codex-acp implements it); agents without it get a friendly notice in the picker. Pagination (nextCursor) is not consumed yet.-c approval_policycommand-line override. In the API-key path NBI also isolatesCODEX_HOME, so the config base is NBI-controlled; in the ChatGPT-auth path codex uses the user's own~/.codex. The pin relies on codex honoring the-coverride above any config it loads. On shared deployments, prefer API-key auth and keepNBI_ACP_MODE_POLICY=force-offunless needed.Related
Relates to #378.