refactor: delete dead codex-core subsystems and inert picker pagination#522
Merged
Conversation
The single page producer has hardcoded next_cursor=None and reached_scan_cap=false since the picker moved to one-shot loading via TranscriptLoader (#518), making the background page-loader channel, LoadingState/SearchState machinery, and all load-more paths unreachable. Removing them is behavior-identical and drops the picker's dependency on codex-core's rollout Cursor.
…bsystems Nothing in acp/tui/cli references rollout (nori-acp has its own transcript recorder), turn_diff_tracker, command_safety, event_mapping, user_instructions, or flags. exec/shell/bash/truncate/text_encoding stay: they back parse_command and the nori-sandbox path, and linux-sandbox's integration tests drive codex_core::exec::process_exec_tool_call directly. Slice B of docs/specs/crate-layering.md §6.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
The feature was default-on and the shipped binary's only config path; the not(nori-config) branches were a dead legacy codex-config fallback. Config source is now unconditional (~/.nori/cli/config.toml via nori-acp), removing a compile-time API-boundary switch. Slice C of docs/specs/crate-layering.md §6.
Drop codex-core's protocol/config_types/models re-exports and rewire all consumers (tui, cli, common, linux-sandbox, core-internal) to import from codex_protocol. Ends the detour that pulled the whole utility crate in for what are pure protocol structs. Slice D of docs/specs/crate-layering.md §6.
Move the harness-domain helpers nori-acp consumed from core into nori-acp itself: user_notification, custom_prompts (dead default_prompts_dir dropped), parse_command + shell/bash/powershell, compact constants (+ templates), and create_patch_with_context (new patch.rs, out of util.rs). Hoist McpServerConfig/McpServerTransportConfig into codex_protocol::config_types; core keeps an intra-crate re-export shim. nori-acp's Cargo.toml no longer lists codex-core; cargo tree confirms the edge is gone. tui/cli reach the moved helpers via nori_acp::. Slice E of docs/specs/crate-layering.md §6.
…nment) 🤖 Generated with [Nori](https://noriagentic.com) Co-Authored-By: Nori <contact@tilework.tech>
…x-sandbox New crate codex-sandbox owns exec, exec_env, spawn, safety, sandboxing, seatbelt (+ policies), landlock, text_encoding, truncate, and error (CodexErr/SandboxErr). Its integration tests move with it. Policy types (ShellEnvironmentPolicy*) hoist to codex_protocol::config_types alongside the Mcp types. codex-core keeps config/auth/model-metadata and now depends on codex-sandbox (matching the existing call direction); tui, cli, linux-sandbox, and core_test_support import codex_sandbox directly. Dead Config-coupled truncate ctor and uncalled helpers deleted. Slice F of docs/specs/crate-layering.md §6.
acp's config module was already self-contained (anyhow/serde/toml/ codex-protocol only). Move it verbatim to a new nori-config crate — Layer 1 of the crate-layering target. nori-acp aliases it back (pub use nori_config as config) so no consumer paths change yet. Slice G part 1 of docs/specs/crate-layering.md §6.
Pull the agent-agnostic ACP hosting machinery out of nori-acp into a new Layer-0 crate, nori-acp-host, per docs/specs/crate-layering.md §3: - connection/ (agent subprocess spawn, AcpConnection wire client, MCP wiring) - translator.rs (ACP wire ↔ codex-protocol event bridge) - registry.rs (agent registry / distribution resolution) - patch.rs (file-change/diff helpers) - error_category.rs (AcpErrorCategory + categorize_acp_error, split out of backend/mod.rs; enhanced_error_message stays in the backend — it is UX wording, not a wire concern) nori-acp re-exports the moved modules (`pub use nori_acp_host::connection;` etc.), so downstream consumers are untouched in this slice; the TUI is rewired onto direct crate deps in slice H. Validation: cargo test -p nori-acp-host standalone-clean (106 tests; caught two feature gaps masked by workspace unification — tokio-util "compat", agent-client-protocol-schema "unstable"); full workspace suite green; tui-pty-e2e green; elizacp close-the-loop TUI drive green. Part of the crate-layering refactor (docs/specs/crate-layering.md).
Rewire the Layer-2 frontends off nori-acp's config re-exports and onto the nori-config crate directly, per docs/specs/crate-layering.md §3: frontends take leaf crates directly; the harness crate is not a config pass-through. - tui/, cli/: `nori_acp::config::X` → `nori_config::X` (nori-config re-exports its surface flat at the crate root), plus the six top-level conveniences (ApprovalPolicy, FileManager, HistoryPersistence, NoriConfig, NoriConfigOverrides, find_nori_home) - nori-acp: public config surface sealed — the six `pub use config::*` re-exports deleted, `pub use nori_config as config` demoted to pub(crate) (53 internal uses unchanged); stale crate doc comment updated - tui/Cargo.toml: drop stale comment for the cargo feature removed in slice C Validation: full workspace suite green; tui-pty-e2e green (23 suites); elizacp close-the-loop TUI drive green; just fmt + just fix clean. Part of the crate-layering refactor (docs/specs/crate-layering.md).
…(slice H2)
Move everything about launching and driving an ACP session out of the TUI
and into a new `nori_acp::runtime` module, per docs/specs/crate-layering.md
slice H ("TUI consumes harness events only"):
- `SessionLaunchSpec` — the frontend's inputs (agent, cwd, policies, MCP
servers, version, session/fork context, optional `SessionResume`)
- `launch_session(spec) -> LaunchedSession { op_tx, handle, events }` —
owns NoriConfig loading and AcpBackendConfig assembly (hooks,
notifications, auto-worktree derivation, history, proxy), the
connect/shutdown/timeout race (8s warning, 30s abort), the op-forwarding
loop, and the session-control command loop
- `SessionEvent { Backend(Box<BackendEvent>), SpawnFailed, ShutdownRequested }`
- `AcpAgentCommand`, `AcpAgentHandle`, `drain_until_shutdown` move here
from the TUI
tui/src/chatwidget/agent.rs shrinks 602 → 170 lines: registry precheck,
AgentConnecting emission, spec construction from the codex `Config`, and a
SessionEvent → AppEvent mapping loop. The two near-identical ~250-line
spawn/resume orchestration functions collapse into the single runtime path
(spawn-vs-resume error labels preserved).
Any frontend (headless exec mode, alternative UIs) can now launch sessions
through the harness without reimplementing orchestration.
Validation: full workspace suite green; tui-pty-e2e green (23 suites);
elizacp close-the-loop TUI drive green; just fmt + just fix clean.
Part of the crate-layering refactor (docs/specs/crate-layering.md).
Adopt the Layer-1 name from docs/specs/crate-layering.md §3. With the low-level ACP hosting machinery extracted to nori-acp-host (slice G2) and session orchestration moved into the crate's runtime module (slice H2), this crate is the headless harness — transcript recording, session reducer, undo, worktrees, hooks, session runtime — not "the ACP crate". - directory acp/ → harness/, package nori-acp → nori-harness, lib nori_acp → nori_harness; all workspace deps and use-paths rewired - docs.md path references nori-rs/acp/ → nori-rs/harness/ - deliberately unchanged: the on-disk log prefix nori-acp (~/.nori/cli/log/nori-acp.*.log) — a runtime artifact external tooling greps for — and the nori-acp-host crate name - tui-pty-e2e tracing filter now names nori_harness and nori_acp_host explicitly; the old nori_acp directive had covered nori_acp_host only via string-prefix matching, and the exit-cleanup tests grep the acp-host "agent spawned (pid)" debug line Validation: full workspace suite green; tui-pty-e2e green (23 suites); elizacp close-the-loop TUI drive green; just fmt + just fix clean. Part of the crate-layering refactor (docs/specs/crate-layering.md).
Also add nori-acp-host and nori-config to the CI test list — they were never added when extracted, so their suites (106 tests in acp-host) did not run in CI at all; only the workspace-local battery covered them. 🤖 Generated with [Nori](https://noriagentic.com) Co-Authored-By: Nori <contact@tilework.tech>
…pt format (#533) Make the extracted crates usable by external ACP and harness projects: - Add publish metadata to the Layer-0 crates: nori-acp-host and mock-acp-agent gain description, license, repository, keywords, and categories, and the workspace gains a repository key. Actual crates.io publication is deferred — the workspace dep tree (codex-protocol, codex-rmcp-client, nori-config) must publish first. - Add docs/reference/transcript-format.md, the on-disk session transcript format for external builders: the nori-home layout, the JSONL envelope, all seven entry types with examples, and the compatibility rules (unknown fields ignored, corrupt lines skipped, first line must be session_meta). Verified against harness/src/transcript/. - Cleanups surfaced by writing the spec: delete the unused nested mock-acp-agent/Cargo.lock, delete the deprecated discover_transcript_for_agent stub (always errored, zero callers), and fix transcript/project.rs doc comments that claimed project ids are SHA-256 (the implementation uses DefaultHasher; the format doc documents ids as opaque). - Make cargo-deny green: add the missing workspace license field to codex-sandbox and nori-config, bump anyhow to 1.0.103 (lockfile-only) for RUSTSEC-2026-0190, and ignore the rmcp and quick-xml advisories with reasons (server-transport-only and semver-major-pinned, respectively). Headless exec/RPC mode is deferred as a new feature; this train is logic-preserving. Net −659 lines. --------- Co-authored-by: Nori <contact@tilework.tech>
CSRessel
added a commit
that referenced
this pull request
Jul 6, 2026
…on (#522) Two logic-preserving deletions, net −4,719 lines: - tui: excise the resume picker's inert pagination scaffolding. Since the picker moved to one-shot loading via TranscriptLoader, the sole page producer hardcodes next_cursor: None, which makes the background page-loader channel, the LoadingState/SearchState machinery, and every load-more path provably unreachable. Removing them is behavior-identical and drops the picker's dependency on codex-core's rollout Cursor. - core: delete dead subsystems — rollout/ (the ACP backend has its own transcript recorder), command_safety/, turn_diff_tracker, event_mapping, user_instructions, flags, and the test file exercising them. Kept deliberately: exec, shell, bash, truncate, text_encoding, terminal, model_provider_info, and openai_model_info. They back parse_command, config, and the nori sandbox path, and linux-sandbox's integration tests drive codex_core::exec::process_exec_tool_call directly.
CSRessel
added a commit
that referenced
this pull request
Jul 6, 2026
…on (#522) Two logic-preserving deletions, net −4,719 lines: - tui: excise the resume picker's inert pagination scaffolding. Since the picker moved to one-shot loading via TranscriptLoader, the sole page producer hardcodes next_cursor: None, which makes the background page-loader channel, the LoadingState/SearchState machinery, and every load-more path provably unreachable. Removing them is behavior-identical and drops the picker's dependency on codex-core's rollout Cursor. - core: delete dead subsystems — rollout/ (the ACP backend has its own transcript recorder), command_safety/, turn_diff_tracker, event_mapping, user_instructions, flags, and the test file exercising them. Kept deliberately: exec, shell, bash, truncate, text_encoding, terminal, model_provider_info, and openai_model_info. They back parse_command, config, and the nori sandbox path, and linux-sandbox's integration tests drive codex_core::exec::process_exec_tool_call directly.
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.
Two logic-preserving deletions, net −4,719 lines:
picker moved to one-shot loading via TranscriptLoader, the sole page
producer hardcodes next_cursor: None, which makes the background
page-loader channel, the LoadingState/SearchState machinery, and every
load-more path provably unreachable. Removing them is behavior-identical
and drops the picker's dependency on codex-core's rollout Cursor.
transcript recorder), command_safety/, turn_diff_tracker, event_mapping,
user_instructions, flags, and the test file exercising them.
Kept deliberately: exec, shell, bash, truncate, text_encoding, terminal,
model_provider_info, and openai_model_info. They back parse_command, config,
and the nori sandbox path, and linux-sandbox's integration tests drive
codex_core::exec::process_exec_tool_call directly.